I'd like us to move forward with this. The option of using 'attach' action does not exist, because, as Eoghan observed, we would be executing an action on an entity which doesn't exist in the collection: (.../api/vms/{vm:id}/disks/???-no entity-???/attach) There are two options left on the table; let's see if we can agree on one of them: 1) 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 Pros: symmetrical Cons: risky (if user wants to detach but forgot to give parameter, disk will be deleted).

2) POST/api/vms/{vm:id}/disks <disk id="{disk:id}"/> => attach disk DELETE .../api/vms/{vm:id}/disks/{disk:id}/detach => detach disk Pros: not dangerous Cons: asymmetrical, attach done like add, but detach done using action. No solution is perfect but we need to decide. Thanks, Ori.

Is there a 3rd option based on #1 where we just detach the disk rather than detaching and deleting? That could be further extended by adding an optional delete flag

> My idea for that was to introduce<force>true|false</force> as well. > Without force=true, we won't delete a disk if it's currently floating. > If the disk is floating it will only appear in root collection (...api/disks), and DELETE from there is non-ambiguous, as there's no option to detach - so I don't see how adding 'force' there helps us.

In your previous mail you suggested the same thing for attached floating disk, so perhaps you meant that here as well? (sorry for only responding now, by the way). But that does break API - for example, a simple script deleting all disks from a VM would fail because suddenly deleting requires 'force=true'.

> Floating disks should be both in the root context, and also in the VM > context for each VM the floating disk is attached to, right? So floating > disks would appear 1+N_attach times in the API, once in the root > context, and once for each VM they are attached to. > IIUC a floating disk is not associated with the VM in any way, and should not appear in that VM's collection of disks. There's a similar feature of activate/deactivate disk (http://www.ovirt.org/wiki/Features/HotPlug). A deactivated disk is still associated with the VM, and should appear in the VM's collection of disks. Perhaps this is the source of confusion.

> >>> Floating disks should be both in the root context, and also in the VM >>> context for each VM the floating disk is attached to, right? So floating >>> disks would appear 1+N_attach times in the API, once in the root >>> context, and once for each VM they are attached to. >>> >> >> IIUC a floating disk is not associated with the VM in any way, and should >> not appear in that VM's collection of disks. There's a similar feature of >> activate/deactivate disk (http://www.ovirt.org/wiki/Features/HotPlug). >> A deactivated disk is still associated with the VM, and should appear in >> the VM's collection of disks. Perhaps this is the source of confusion. > > No that isn't it. I think have a bit of terminology mismatch. According to: > > http://www.ovirt.org/wiki/Features/DetailedFloatingDisk > > i think your use of the word floating is correct. I got confused as i > thought that any disk could be floating, whether it was attached to 0 > VMs, 1 VM, or 2 or more VMs. > > So before we go further, let's get things straight: > > - disk without any VMs => floating. It appears in the root context. > - disk attached to 1 VM => normal state. It appears in the VM context. I think this type of disk should be shown in both contexts. If a user is looking at http://{host:port}/disks, I think he intuitively expects to see all disks (attached or unattached). A disk will contain reference to the VMs which it is attached to, and thus: - No VM-ids --> floating disk. - One VM-id --> regular disk. - Multiple VM-ids --> shared disk.

> - disk attached to 2 VMs => shared disk. It appears in both VM contexts. > > Is that correct? > > Also can someone explain on how floating and shared disks are actually > implemented in the BE? I imagine it's one of two ways: > > => The BE keep a global list of disks, each disk with zero or more > references to a VM. > => The BE keeps a special "floating" state independent of the number > of VMs that it is attached to. > > Thanks, > Geert _______________________________________________ Engine-devel mailing list Engine-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel

> iirc, shared disk is a flag on the disk to allow it to be shared. > doesn't mean it is shared yet, rather can be shared. Right, but there's no contradiction: a disk can have 'shared' flag and also show the list of VMs it's attached to. The flag tells you if the disk has the ability to be shared, while the list of VMs tells you which VMs are actually sharing the disk right now (if any at all)

> If we can expose the "shared" flag though, i think the API gets more > natural. We can then only show the shared disks in the root context. > > Itamar, can we expose this flag, or do you consider this flag is > internal to the BE? we have to expose this flag for shared disk functionality. this is the only way via ui/api to flag the disk as shared (which only then allows attaching it to more than a single vm - this is explicit to avoid disk corruptions)

On 04/17/2012 05:54 PM, Itamar Heim wrote: >> If we can expose the "shared" flag though, i think the API gets more >> natural. We can then only show the shared disks in the root context. >> >> Itamar, can we expose this flag, or do you consider this flag is >> internal to the BE? > > we have to expose this flag for shared disk functionality. this is the > only way via ui/api to flag the disk as shared (which only then allows > attaching it to more than a single vm - this is explicit to avoid disk > corruptions) OK, understood. Which then begs the question - why we are trying to make the setting / unsetting of this flag implicit via the various POST and DELETE calls we were trying to model. Maybe we ought to just allow a user to set the shared flag: PUT /api/vms/{vm:id}/disks/{disk:id} <disk> <shared>true|false</shared> </disk> When a disk is shared: - The disks shows up in the root context /api/disks. - You can POST this disk to another VM, making it available to that VM. - Detach it from a VM using DELETE. To delete a shared disk, you need to either: - Unshare it first. Then DELETE. - DELETE it from the root context. Regards, Geert

_______________________________________________ Engine-devel mailing list Engine-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel

On 18/04/12 13:36, Einav Cohen wrote: > [Top Posting] > > Let's try to concentrate on the subject of this thread (we can discuss shared disks in a separate thread): > > - I agree with Ori: Disks root collection should have all disks, not only floating (shared?) disks. Disk is an important business entity, which "deserves" a full root collection of its own - I think it is more aligned with the existing api structure (VMs are shown both in the VMs root collection as well as under their Cluster/DC). +1 > > - Regarding possibilities 1 vs. 2 below: I prefer #1 (again - agreeing with Ori, IIRC): Although risky in a sense, at least existing users/scripts won't be harmed because of this, since the behavior is backward compatible. Regarding new users - they should know what they are doing, and they should always be careful when using DELETE, no matter from which context. It also "looks" better (more symmetrical, adding a new POST action is avoided). > I find option one confusing, I prefer #2. Does anyone have an idea how to model detach and attach as PUT (update) operation instead of DELETE(remove) and POST(add). Pros: - we avoid confusing delete disk with detach disk - symmetrical (for attach detach) Cons: - not sure we have this approach in other places in the API - is it confusing for the user? Livnat

> Comments? > > > ----- Original Message ----- >> From: "Ori Liel" <oliel(a)redhat.com&gt; >> To: engine-devel(a)ovirt.org >> Cc: "Eoghan Glynn" <eglynn(a)redhat.com&gt; >> Sent: Monday, April 16, 2012 3:33:27 PM >> Subject: Re: [Engine-devel] Floating Disks implementation in REST-API >> >> I'd like us to move forward with this. >> >> The option of using 'attach' action does not exist, because, as >> Eoghan observed, we would be executing an action on an entity which >> doesn't exist in the collection: (.../api/vms/{vm:id}/disks/???-no >> entity-???/attach) >> >> There are two options left on the table; let's see if we can agree on >> one of them: >> >> 1) >> 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 >> >> Pros: symmetrical >> Cons: risky (if user wants to detach but forgot to give parameter, >> disk will be deleted). >> >> 2) >> >> POST/api/vms/{vm:id}/disks >> <disk id="{disk:id}"/> => >> attach disk >> >> DELETE .../api/vms/{vm:id}/disks/{disk:id}/detach => >> detach disk >> >> Pros: not dangerous >> Cons: asymmetrical, attach done like add, but detach done using >> action. >> >> >> No solution is perfect but we need to decide. >> >> Thanks, >> >> Ori. >> _______________________________________________ >> Engine-devel mailing list >> Engine-devel(a)ovirt.org >> http://lists.ovirt.org/mailman/listinfo/engine-devel >> >> >> > _______________________________________________ > Engine-devel mailing list > Engine-devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/engine-devel