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

[Itamar Heim]

On 04/11/2012 11:50 AM, Ayal Baron wrote:
>
>
> ----- 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

this would break backward compatibility of the API, as today this 
deletes the disk as well.
geert - thoughts on this?

> 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
>>
> _______________________________________________
> Engine-devel mailing list
> Engine-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel