Re: [Engine-devel] Floating Disks implementation in REST-API
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}
diskdetachtrue/detachdisk = 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
Re: [Engine-devel] Floating Disks implementation in REST-API
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
sharedtrue|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
___
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
Re: [Engine-devel] Floating Disks implementation in REST-API
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 ol...@redhat.com
To: engine-devel@ovirt.org
Cc: Eoghan Glynn egl...@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}
diskdetachtrue/detachdisk=
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
___
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
Re: [Engine-devel] Floating Disks implementation in REST-API
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 Lielol...@redhat.com
To: engine-devel@ovirt.org
Cc: Eoghan Glynnegl...@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}
diskdetachtrue/detachdisk =
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
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}
diskdetachtrue/detachdisk= 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
Re: [Engine-devel] Floating Disks implementation in REST-API
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}
diskdetachtrue/detachdisk = 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 forcetrue|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: gjan...@redhat.com
___
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
Re: [Engine-devel] Floating Disks implementation in REST-API
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 ___ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
Re: [Engine-devel] Floating Disks implementation in REST-API
- 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 detachtrue|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
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 detachtrue/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 forcetrue/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@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
___
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
Re: [Engine-devel] Floating Disks implementation in REST-API
- Original Message -
From: Eoghan Glynn egl...@redhat.com
To: Ori Liel ol...@redhat.com
Cc: meyer...@redhat.com, jhern...@redhat.com, Geert Jansen
gjan...@redhat.com, Einav Cohen eco...@redhat.com, Michael Pasternak
mpast...@redhat.com, Michael Kublin mkub...@redhat.com,
engine-devel@ovirt.org
Sent: Tuesday, April 10, 2012 1:42:04 PM
Subject: Re: Floating Disks implementation in REST-API
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?
Hi Ori,
I tend to agree that overloading the DELETE verb to either delete or detach
the disk is error-prone, and justifies the addition of new detach action
in this case.
However, I'm wondering whether it would be better, if somewhat asymmetric,
to still avoid the attach action, e.g. instead use:
POST /api/vms/{vm:id}/disks
disk id={disk:id}/ = attach disk
POST /api/vms/{vm:id}/disks/{disk:id}/detach = detach disk
The reasoning here would be that:
POST /api/vms/{vm:id}/disks/{disk:id}/attach
would tend to break the sub-collection idiom in my mind (as the disk in
question
is not yet part of the disks sub-collection of that VM, prior to the attachment
actually occuring).
Eoghan, I agree with you; this is actually a crushing argument against 'attach'
action, which I overlooked. But I dislike the asymmetry of detaching using an
action,
and attaching using POST. So I'm now actually advocating:
POST /api/vms/{vm:id}/disks
disk id={disk:id}/ = attach disk
DELETE .../api/vms/{vm:id}/disks/{disk:id}
diskdetachtrue/detachdisk= detach disk
If you can agree to this then Geert, you and myself would be aligned (right
Geert?),
and I can go forward with implementation.
Thanks,
Ori.
Cheers,
Eoghan
___
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
Re: [Engine-devel] Floating Disks implementation in REST-API
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 adetachtrue|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 unlessforce 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 withdetachtrue/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} withforcetrue/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@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
___
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
Re: [Engine-devel] Floating Disks implementation in REST-API
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 detachtrue|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 detachtrue/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 forcetrue/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@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
Re: [Engine-devel] Floating Disks implementation in REST-API
In my view, backwards compatible DELETE semantics for a floating disk
aren't that bad:
DELETE /vms/{vm:id}/disks/{disk:id}
= Accept a detachtrue|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).
To attach a disk and make it float, use POST and you and Eoghan
described.
To detach a disk, use DELETE with detachtrue/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 forcetrue/force; OR
DELETE /disks/{disk:id}
While keeping to the DELETE pattern for detach is attractive, the use
of two separate flags (detach and force), plus the requirement to
special-case deletion logic in the floating disk case, seems to outweigh
the benefit.
This scenario would seem to fit in with the critera we applied when
choosing actions over standard verbs in the first place - i.e. a case
where a standard verb leads to awkward, counter-intuitive or unnatural
usage. In this case, the client-side logic to select the force flag
and the error handling for an inadvertent DELETE-delete when a
DELETE-detach was intended (409 status, I guess?), would seem to me to
justify going the action route.
Just my 2 cents ...
Cheers,
Eoghan
___
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
Re: [Engine-devel] Floating Disks implementation in REST-API
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 detachtrue|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 detachtrue/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 forcetrue/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
___
Engine-devel mailing list
Engine-devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
