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

[Livnat Peer]

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}

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