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

----- Original Message ----- From: "Eoghan Glynn" <eglynn(a)redhat.com&gt; To: "Ori Liel" <oliel(a)redhat.com&gt; Cc: meyering(a)redhat.com, jhernand(a)redhat.com, "Geert Jansen" <gjansen(a)redhat.com&gt;, "Einav Cohen" <ecohen(a)redhat.com&gt;, "Michael Pasternak" <mpastern(a)redhat.com&gt;, "Michael Kublin" <mkublin(a)redhat.com&gt;, engine-devel(a)ovirt.org Sent: Tuesday, April 10, 2012 1:42:04 PM Subject: Re: Floating Disks implementation in REST-API > 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? Hi Ori, I tend to agree that overloading the DELETE verb to either delete or detach the disk is error-prone, and justifies the addition of new detach action in this case. However, I'm wondering whether it would be better, if somewhat asymmetric, to still avoid the attach action, e.g. instead use: POST /api/vms/{vm:id}/disks <disk id="{disk:id}"/> => attach disk POST /api/vms/{vm:id}/disks/{disk:id}/detach => detach disk The reasoning here would be that: POST /api/vms/{vm:id}/disks/{disk:id}/attach would tend to break the sub-collection idiom in my mind (as the disk in question is not yet part of the disks sub-collection of that VM, prior to the attachment actually occuring).

Cheers, Eoghan