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.
On 04/16/2012 02:33 PM, Ori Liel wrote:
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).
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. I think it makes it less risky. And it is still backwards compatible, as 3.0 clients do not know how to create a floating disk, and therefore there is no risk that they are deleting one. Regards, Geert
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.
-- Geert Jansen Sr. Product Marketing Manager, Red Hat Enterprise Virtualization Red Hat S.r.L. O: +39 095 916287 Via G. Fara 26 C: +39 348 1980079 (when in US: 415-623-0542) Milan 20124, Italy E: gjansen@redhat.com
On 04/16/2012 02:33 PM, Ori Liel wrote:
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).
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'.
I think it makes it less risky. And it is still backwards compatible, as 3.0 clients do not know how to create a floating disk, and therefore there is no risk that they are deleting one.
Regards, Geert
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.
-- Geert Jansen Sr. Product Marketing Manager, Red Hat Enterprise Virtualization
Red Hat S.r.L. O: +39 095 916287 Via G. Fara 26 C: +39 348 1980079 (when in US: 415-623-0542) Milan 20124, Italy E: gjansen@redhat.com
On 04/17/2012 02:27 PM, Ori Liel wrote:
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.
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. Regarding the root context: you are right that DELETE in the root context is unambiguous, and therefore we don't need "detach" or "force" arguments there. And because in 3.0 we didn't have a disk collection in the root context, backwards compatibility isn't an issue here.
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'.
A 3.0 client does not know how to create a floating disk. So in my view, it does not need to know how to delete one. Is your concern about the case where a 3.1 client creates a floating disk, and you would like a 3.0 client to be able to delete that. I think this is a too strong interpretation of backwards compatibility. So far all ISV software that integrates with us and does provisioning of VMs, both creates and deletes VMs. And properly written software will issue an error message when a disk DELETE fails. In the end there could be many other reasons why a disk DELETE can fail, right? We are adding one more reason a disk DELETE will fail, namely that the disk is floating. Regards, Geert
On 04/17/2012 02:27 PM, Ori Liel wrote:
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.
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.
Regarding the root context: you are right that DELETE in the root context is unambiguous, and therefore we don't need "detach" or "force" arguments there. And because in 3.0 we didn't have a disk collection in the root context, backwards compatibility isn't an issue here.
correct, root context is not a problem
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'.
A 3.0 client does not know how to create a floating disk. So in my view, it does not need to know how to delete one.
Is your concern about the case where a 3.1 client creates a floating disk, and you would like a 3.0 client to be able to delete that. I think this is a too strong interpretation of backwards compatibility. So far all ISV software that integrates with us and does provisioning of VMs, both creates and deletes VMs. And properly written software will issue an error message when a disk DELETE fails. In the end there could be many other reasons why a disk DELETE can fail, right? We are adding one more reason a disk DELETE will fail, namely that the disk is floating.
Regards, Geert
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. - 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
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
On 04/17/2012 05:26 PM, Ori Liel wrote:
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.
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.
- 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@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
On 04/17/2012 05:26 PM, Ori Liel wrote:
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://{hostort}/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.
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)
- 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@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
On 04/17/2012 04:53 PM, Ori Liel wrote:
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? Regards,
On 04/17/2012 05:56 PM, Geert Jansen wrote:
On 04/17/2012 04:53 PM, Ori Liel wrote:
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
On 17/04/12 19:31, Geert Jansen wrote:
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
Hi Geert, UIUC we are trying to model two backend commands: - attach disk to VM - detach disk from VM If the disk property 'shareable' is checked it means the user can attach the disk to more than one VM, but the question is how the user does that (regardless if the disk is attached to other VMs or not). My understanding is that the root disks collections includes all disks in the setup (floating and attached) and the /api/vms/{vm:id}/disks/ includes all disks attached to the VM (can be disk that is attached to more than this VM, or only to this VM). Livnat
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
----- Original Message -----
From: "Ori Liel" <oliel@redhat.com> To: engine-devel@ovirt.org Cc: "Eoghan Glynn" <eglynn@redhat.com> Sent: Monday, April 16, 2012 8:33:27 AM 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.
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
No solution is perfect but we need to decide.
Thanks,
Ori. _______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
On 04/16/2012 02:39 PM, Andrew Cathrow wrote:
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
This was discussed and the problem with that is that it's not backwards compatible. Current clients and ISV software expects to be able to DELETE a disk, and after that it expects that the disk is gone. If we change behavior so that DELETE becomes detach by default, then old clients when they DELETE without any flags will instead create a floating disk. Regards, Geert
[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). - 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). Comments? ----- Original Message -----
From: "Ori Liel" <oliel@redhat.com> To: engine-devel@ovirt.org Cc: "Eoghan Glynn" <eglynn@redhat.com> 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@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@redhat.com> To: engine-devel@ovirt.org Cc: "Eoghan Glynn" <eglynn@redhat.com> 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@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________ Engine-devel mailing list Engine-devel@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
I'll try to describe your suggestion, if I understood it correctly. PUT is used for update. We could decide that user achieves attach/detach by an updating the entire disks collection: PUT .../api/vms/{vm:id}/disks The user must always pass all disks, and then: - If a disk, which is currently not attached to the VM, is passed - attach this disk. - if a disk, which is currently attached to the VM, is not passed - detach this disk. I'm against for two reasons: 1) We currently don't have update on collection anywhere else (although technically it's possible) 2) It's quite confusing. Intuitively, the user has no of knowing that updating the collection (which he's not used to anyway) will result in attach/detach, and not in create/delete. Another possibility of using PUT is on the root collection of disks, PUT .../api/disks Updating the vm-ids of this disk. But I don't like this solution as well (opinions?) If you had in mind a different way of using PUT, please explain it.
Comments?
----- Original Message -----
From: "Ori Liel" <oliel@redhat.com> To: engine-devel@ovirt.org Cc: "Eoghan Glynn" <eglynn@redhat.com> 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@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
On 04/18/2012 01:36 PM, 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).
- 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).
Comments?
I also like #1. maybe change the parameter name though from detach to something that explains the delete will only be from the VM ("preserve=true", or some better name). i.e., detach, like delete is "in the same direction" of removing something. preserve implies this is in the other direction from the delete. hope this makes sense
----- Original Message -----
From: "Ori Liel"<oliel@redhat.com> To: engine-devel@ovirt.org Cc: "Eoghan Glynn"<eglynn@redhat.com> 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@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
Current summary: There are four options for Floating-Disks modelling: two that were previously discussed, and two that were recently added by Livnat, using PUT. See full list of options below. Itamar, Geert, Einav & Ori are currently pro option #1 (if anyone changed his mind - please say so). Does anyone strongly prefer another option, or can we move forward with option #1? Thanks, Ori. 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 2) POST /api/vms/{vm:id}/disks <disk id="{disk:id}"/> => attach disk DELETE /api/vms/{vm:id}/disks/{disk:id}/detach => detach disk 3) PUT /api/vms/{vm:id}/disks (update on the disks collection) => attach + detach 4) PUT /api/disks/{disk:id} (update on the vm-id's of disk) => attach + detach
In the interest of moving forward, let's agree that, since there seems to be an overwhelming majority for option 1 (see below), then if by the start of next week (i.e Monday, April 23) no one will object to option 1, I will start the implementation. Thanks, Ori.
Current summary:
There are four options for Floating-Disks modelling: two that were previously discussed, and two that were recently added by Livnat, using PUT. See full list of options below.
Itamar, Geert, Einav & Ori are currently pro option #1 (if anyone changed his mind - please say so). Does anyone strongly prefer another option, or can we move forward with option #1?
Thanks,
Ori.
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
2)
POST /api/vms/{vm:id}/disks <disk id="{disk:id}"/> => attach disk
DELETE /api/vms/{vm:id}/disks/{disk:id}/detach => detach disk
3) PUT /api/vms/{vm:id}/disks (update on the disks collection) => attach + detach
4) PUT /api/disks/{disk:id} (update on the vm-id's of disk) => attach + detach
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
participants (6)
-
Andrew Cathrow -
Einav Cohen -
Geert Jansen -
Itamar Heim -
Livnat Peer -
Ori Liel