9:46 a.m.
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?
So i would assume we get a top-level /disks collection correct?? And I
assume that collection would only list floating disks?
Indeed, there will be a root collection. but I do not see a reason why this collection
should only show floating disks. I tend to think it should show all disks in the system.
Keep in mind that we're going towards Shared disks, meaning the same disk may be used
by two VMs, so it makes sense to show such a disk in the root collection, along with the
information of which VMs it's attached to.
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.
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).
I Agree with Eoghan that the drawbacks outweigh the benefits in this case,
for the reasons Eoghan said.
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
DELETE /disks/{disk:id}
To create a non-attached, floating, disk, use:
POST /disks
Worth mentioning that creation of disk that is attached to a VM is still possible by POST
to /vms/{vm:id}/disks/ without supplying disk-Id.
Regards,
Geert
Ori