[Engine-devel] Floating Disks implementation in REST-API

[Ayal Baron]

----- Original Message -----
> On 10/04/12 14:35, Geert Jansen wrote:
> > 
> > 
> > On 04/09/2012 05:14 PM, Ori Liel wrote:
> >> The "Floating Disks" feature makes disks into stand-alone
> >> entities: a
> >> given disk may be attached to a VM (as all disks are today), or it
> >> may
> >> be not attached to any VM, which makes it a floating disk
> >> (http://www.ovirt.org/wiki/Features/FloatingDisk)
> >>
> >> To implement attach/detach of disk to/from VM in REST-API, we
> >> intend
> >> to introduce two new actions:
> >>
> >>    POST .../api/vms/{vm:id}/disks/{disk:id}/attach
> >>    POST .../api/vms/{vm:id}/disks/{disk:id}/detach
> >>
> >> Since we try not to add new actions unless we have to, I want to
> >> explain why I believe that these actions are necessary.
> >> The other implementation would use existing add/remove flows:
> >>
> >>    POST   .../api/vms/{vm:id}/disks               - if the disk
> >>    was
> >> passed with an ID, attach it to this VM. If no id - create a new
> >> disk.
> >>    DELETE .../api/vms/{vm:id}/disks/{disk:id}     - *ambiguity
> >> problem, need to add a flag*
> >>
> >> We can't break existing API, so regular DELETE must remove the
> >> disk,
> >> as it does today. To detach a disk using DELETE we'd have to add a
> >> flag to the DETELE command. This is quite risky, because if the
> >> user
> >> forgets to pass this flag, the disk which he wanted to detach will
> >> actually be deleted.
> >>
> >> Theoretically, if we could break the API, the following modelling
> >> would resolve the ambiguity and perhaps be ideal:
> >> - POST/DELETE disk in root context means create or delete it.
> >> - POST/DELETE disk in VM context means attach or detach it.
> >> But we don't have the privilege of breaking the API.
> >>
> >> Considering all of the above - and the fact that attach/detach
> >> nics
> >> to/from host is also implemented using actions - I believe that
> >> the
> >> new actions are justifiable.
> >>
> >> Any comments?
> > 
> > I assume that a floating disk can be attached to 0 VMs as well,
> > right?
> 
> floating == attached to 0 VMs
> 
> > So i would assume we get a top-level /disks collection correct??
> 
> Yes
> 
> > And I assume that collection would only list floating disks?
> 
> IMO the /disks collection includes all disks in the setup regardless
> if
> they are floating or attached to VM/s
> 
> > 
> > In my view, backwards compatible DELETE semantics for a floating
> > disk
> > aren't that bad:
> > 
> >  DELETE /vms/{vm:id}/disks/{disk:id}
> > 
> > => Accept a <detach>true|false</detach> argument.
> > => Defaults to "false" for compatibility.
> > 
> 
> I think that ideally :
> 
> Detach a disk from VM (becomes floating):
> DELETE api/vms/{vm:id}/disks/{disk:id}
> 
> Delete a disk ('real' delete)"
> DELETE api/disks/{disk:id}

Assuming this also works when the disk is attached to a VM then the above seems to me like the simplest and clearest path. i.e.
DELETE in VM context detaches
DELETE in DISKS context really deletes

POST in VM context attaches (and creates if does not yet exist)
POST in DISKS context creates floating.

Only thing is that to create and attach you post to VM but the complementary detach and delete is delete from disks (i.e. not symmetrical).

> 
> I understand that it is changing the current API behavior, it means
> that
> a user who deleted disks in 3.0 will end up only detaching them in
> 3.1,
> is that something we can't 'live' with?
> 
> For attach, I would go with Eoghan approach:
> 
> POST /api/vms/{vm:id}/disks
>    <disk id="{disk:id}"/>                       => attach disk
> 
> 
> 
> > This means that DELETE will by default really DELETE any
> > non-floating
> > disk. That is compatibility with today. For safety, implement this:
> > 
> > => When deleting a disk that is floating, fail unless <force> is
> > also
> > set to "true". And always fail if one of the other VMs is running
> > (obviously).
> > 
> > To attach a disk and make it float, use POST and you and Eoghan
> > described.
> > 
> > To detach a disk, use DELETE with <detach>true</detach>.
> > 
> > To delete a non-attached, floating disks, use:
> > 
> >  DELETE /disks/{disk:id}
> > 
> > To delete an attached, floating disk, use:
> > 
> >  DELETE /vm/{vm:id}/disks/{disk:id} with <force>true</force>; OR
> 
> ^^ isn't that considered breaking the API? we require force flag for
> deleting disk which we did not require before.
> 
> >  DELETE /disks/{disk:id}
> > 
> > To create a non-attached, floating, disk, use:
> > 
> >  POST /disks
> > 
> > Regards,
> > Geert
> > _______________________________________________
> > Engine-devel mailing list
> > Engine-devel at ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/engine-devel
> 
> _______________________________________________
> Engine-devel mailing list
> Engine-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>