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

[Ori Liel]

>
>
>----- Original Message -----
>From: "Eoghan Glynn" <eglynn at redhat.com>
>To: "Ori Liel" <oliel at redhat.com>
>Cc: meyering at redhat.com, jhernand at redhat.com, "Geert Jansen" <gjansen at redhat.com>, "Einav Cohen" <ecohen at redhat.com>, "Michael Pasternak" <mpastern at redhat.com>, "Michael Kublin" <mkublin at redhat.com>, engine-devel at 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).

Eoghan, I agree with you; this is actually a crushing argument against 'attach' 
action, which I overlooked. But I dislike the asymmetry of detaching using an action, 
and attaching using POST. So I'm now actually advocating:

      POST /api/vms/{vm:id}/disks        
      <disk id="{disk:id}"/>                       => attach disk

      DELETE .../api/vms/{vm:id}/disks/{disk:id}
      <disk><detach>true</detach><disk>            => detach disk

If you can agree to this then Geert, you and myself would be aligned (right Geert?), 
and I can go forward with implementation.

Thanks, 

Ori. 

>
>Cheers,
>Eoghan