[libvirt] [RFC] Add flag for virsh undefine to remove/wipe the disk devices
Hi, All I'm thinking to introduce a new flag (something like --remove-disks, --wipe-disks) for "virsh undefine", so that the user can choose whether to remove/wipe the disk devices or not, have seen this requirement in many places, @libvirt-users, public #virt, and also we have a bug of this function. So, IMHO this is a reasonable requirement, following is the rough thoughts: 1) General idea. As we don't have a API which can get all the disk devices of a domain, perhaps need to write functions to parse domain xml to extract the disks' path (this is annoyed, but seems don't other way), and then lookup them by storage volume API (virStorageVolLookupByPath), and then can remove or wipe the volume by (virStorageVolDelete/virStorageVolWipe). And for the disk path which doesn't belong to any storage pool, simply remove it by "unlink()"? 2) Which type of devices can not be removed/wiped. * Can't delete/wipe ISCSI/SCSI vol. * Vol doesn't exists (which will throw an warning when do virStorageVolLookupByPath). * Have no write permission on the parent directory of the disk path. * Can't delete/wipe the disk device which is passthrough'ed from host, (e.g. /dev/sr0 as a CDROM device for guest) * The storage pool which the disk device belongs to as a vol is marked as "share" * The storage pool which the disk device belongs as a vol is readonly * can't delete disk device of network type. * Any others? For these situations, we need to do checking and throw straightforward warnings to tell user why it can't be removed/wiped. Any idea is welcomed. Thanks. CC'ed Eric, as I saw you talked about this somewhere. (#virt?), :-) Regards Osier
On Wed, Mar 30, 2011 at 09:39:14PM +0800, Osier Yang wrote:
Hi, All
I'm thinking to introduce a new flag (something like --remove-disks, --wipe-disks) for "virsh undefine", so that the user can choose whether to remove/wipe the disk devices or not, have seen this requirement in many places, @libvirt-users, public #virt, and also we have a bug of this function. So, IMHO this is a reasonable requirement, following is the rough thoughts:
1) General idea. As we don't have a API which can get all the disk devices of a domain, perhaps need to write functions to parse domain xml to extract the disks' path (this is annoyed, but seems don't other way), and then lookup them by storage volume API (virStorageVolLookupByPath), and then can remove or wipe the volume by (virStorageVolDelete/virStorageVolWipe).
And for the disk path which doesn't belong to any storage pool, simply remove it by "unlink()"?
Won't work for connection to remote hosts.
2) Which type of devices can not be removed/wiped.
* Can't delete/wipe ISCSI/SCSI vol. * Vol doesn't exists (which will throw an warning when do virStorageVolLookupByPath). * Have no write permission on the parent directory of the disk path. * Can't delete/wipe the disk device which is passthrough'ed from host, (e.g. /dev/sr0 as a CDROM device for guest) * The storage pool which the disk device belongs to as a vol is marked as "share" * The storage pool which the disk device belongs as a vol is readonly * can't delete disk device of network type. * Any others?
For these situations, we need to do checking and throw straightforward warnings to tell user why it can't be removed/wiped.
I would rather make this a flag of virDomainUndefine(), except there is no flag argument for it :( I think if we want this to work well tis should be based on API operations only, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
于 2011年03月30日 21:50, Daniel Veillard 写道:
On Wed, Mar 30, 2011 at 09:39:14PM +0800, Osier Yang wrote:
Hi, All
I'm thinking to introduce a new flag (something like --remove-disks, --wipe-disks) for "virsh undefine", so that the user can choose whether to remove/wipe the disk devices or not, have seen this requirement in many places, @libvirt-users, public #virt, and also we have a bug of this function. So, IMHO this is a reasonable requirement, following is the rough thoughts:
1) General idea. As we don't have a API which can get all the disk devices of a domain, perhaps need to write functions to parse domain xml to extract the disks' path (this is annoyed, but seems don't other way), and then lookup them by storage volume API (virStorageVolLookupByPath), and then can remove or wipe the volume by (virStorageVolDelete/virStorageVolWipe).
And for the disk path which doesn't belong to any storage pool, simply remove it by "unlink()"?
Won't work for connection to remote hosts.
Hmm, yes, :-)
2) Which type of devices can not be removed/wiped.
* Can't delete/wipe ISCSI/SCSI vol. * Vol doesn't exists (which will throw an warning when do virStorageVolLookupByPath). * Have no write permission on the parent directory of the disk path. * Can't delete/wipe the disk device which is passthrough'ed from host, (e.g. /dev/sr0 as a CDROM device for guest) * The storage pool which the disk device belongs to as a vol is marked as "share" * The storage pool which the disk device belongs as a vol is readonly * can't delete disk device of network type. * Any others?
For these situations, we need to do checking and throw straightforward warnings to tell user why it can't be removed/wiped.
I would rather make this a flag of virDomainUndefine(), except there is no flag argument for it :(
Yes, actually I also prefer to add new flag to API, but not in virsh instead, however, adding new flag argument is not workable, how about introduce a new API, something like "virDomainUndefineFlag"?
I think if we want this to work well tis should be based on API operations only,
Daniel
于 2011年03月30日 23:23, Osier Yang 写道:
于 2011年03月30日 21:50, Daniel Veillard 写道:
On Wed, Mar 30, 2011 at 09:39:14PM +0800, Osier Yang wrote:
Hi, All
I'm thinking to introduce a new flag (something like --remove-disks, --wipe-disks) for "virsh undefine", so that the user can choose whether to remove/wipe the disk devices or not, have seen this requirement in many places, @libvirt-users, public #virt, and also we have a bug of this function. So, IMHO this is a reasonable requirement, following is the rough thoughts:
1) General idea. As we don't have a API which can get all the disk devices of a domain, perhaps need to write functions to parse domain xml to extract the disks' path (this is annoyed, but seems don't other way), and then lookup them by storage volume API (virStorageVolLookupByPath), and then can remove or wipe the volume by (virStorageVolDelete/virStorageVolWipe).
And for the disk path which doesn't belong to any storage pool, simply remove it by "unlink()"?
Won't work for connection to remote hosts.
Hmm, yes, :-)
2) Which type of devices can not be removed/wiped.
* Can't delete/wipe ISCSI/SCSI vol. * Vol doesn't exists (which will throw an warning when do virStorageVolLookupByPath). * Have no write permission on the parent directory of the disk path. * Can't delete/wipe the disk device which is passthrough'ed from host, (e.g. /dev/sr0 as a CDROM device for guest) * The storage pool which the disk device belongs to as a vol is marked as "share" * The storage pool which the disk device belongs as a vol is readonly * can't delete disk device of network type. * Any others?
For these situations, we need to do checking and throw straightforward warnings to tell user why it can't be removed/wiped.
I would rather make this a flag of virDomainUndefine(), except there is no flag argument for it :(
Yes, actually I also prefer to add new flag to API, but not in virsh instead, however, adding new flag argument is not workable, how about introduce a new API, something like "virDomainUndefineFlag"?
And also a new API to get the disks of domain? so that we don't need to parse the XML directly. :-)
I think if we want this to work well tis should be based on API operations only,
Daniel
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wednesday 30 March 2011 17:23:51 Osier Yang wrote:
Yes, actually I also prefer to add new flag to API, but not in virsh instead, however, adding new flag argument is not workable, how about introduce a new API, something like "virDomainUndefineFlag"?
I wanted to suggest something similar because I'm annoyed that calling undefine on an OpenVZ domain destroys the container private area... -- Jean-Baptiste ROUAULT Ingénieur R&D - Diateam : Architectes de l'information Phone : +33 (0)9 53 16 02 70 Fax : +33 (0)2 98 050 051
于 2011年03月30日 23:56, Jean-Baptiste Rouault 写道:
On Wednesday 30 March 2011 17:23:51 Osier Yang wrote:
Yes, actually I also prefer to add new flag to API, but not in virsh instead, however, adding new flag argument is not workable, how about introduce a new API, something like "virDomainUndefineFlag"?
I wanted to suggest something similar because I'm annoyed that calling undefine on an OpenVZ domain destroys the container private area...
Hi, Jean Could you explain more? Wanna known if can do it incidentally, though I'm guessing it's caused by openvz driver destroying the private area internally, something like "vzctl --destroy". If it's right for your meaning, then IMHO we can't introduce new flag for this, as it's only openvz driver specificly. Regards Osier
On Thursday 31 March 2011 10:04:36 Osier Yang wrote:
于 2011年03月30日 23:56, Jean-Baptiste Rouault 写道:
On Wednesday 30 March 2011 17:23:51 Osier Yang wrote:
Yes, actually I also prefer to add new flag to API, but not in virsh instead, however, adding new flag argument is not workable, how about introduce a new API, something like "virDomainUndefineFlag"?
I wanted to suggest something similar because I'm annoyed that calling undefine on an OpenVZ domain destroys the container private area...
Hi, Jean
Could you explain more? Wanna known if can do it incidentally, though I'm guessing it's caused by openvz driver destroying the private area internally, something like "vzctl --destroy". If it's right for your meaning, then IMHO we can't introduce new flag for this, as it's only openvz driver specificly.
Regards Osier
Hi, Yes the problem is OpenVZ specific because "vzctl destroy" internally destroys the private area. If there is a new "virDomainUndefineFlag" API, I was thinking that maybe the openvz driver could check the value of that flag in "openvzDomainUndefine()", then only remove the container config file in one case, or use "vzctl destroy" in the other case. To make it cleaner, a patch could be send upstream so that vzctl destroy accepts a new argument like "--keep-private". Regards Jean-Baptiste -- Jean-Baptiste ROUAULT Ingénieur R&D - Diateam : Architectes de l'information Phone : +33 (0)9 53 16 02 70 Fax : +33 (0)2 98 050 051
On Wed, Mar 30, 2011 at 09:50:19PM +0800, Daniel Veillard wrote:
On Wed, Mar 30, 2011 at 09:39:14PM +0800, Osier Yang wrote:
Hi, All
I'm thinking to introduce a new flag (something like --remove-disks, --wipe-disks) for "virsh undefine", so that the user can choose whether to remove/wipe the disk devices or not, have seen this requirement in many places, @libvirt-users, public #virt, and also we have a bug of this function. So, IMHO this is a reasonable requirement, following is the rough thoughts:
1) General idea. As we don't have a API which can get all the disk devices of a domain, perhaps need to write functions to parse domain xml to extract the disks' path (this is annoyed, but seems don't other way), and then lookup them by storage volume API (virStorageVolLookupByPath), and then can remove or wipe the volume by (virStorageVolDelete/virStorageVolWipe).
And for the disk path which doesn't belong to any storage pool, simply remove it by "unlink()"?
Won't work for connection to remote hosts.
2) Which type of devices can not be removed/wiped.
* Can't delete/wipe ISCSI/SCSI vol. * Vol doesn't exists (which will throw an warning when do virStorageVolLookupByPath). * Have no write permission on the parent directory of the disk path. * Can't delete/wipe the disk device which is passthrough'ed from host, (e.g. /dev/sr0 as a CDROM device for guest) * The storage pool which the disk device belongs to as a vol is marked as "share" * The storage pool which the disk device belongs as a vol is readonly * can't delete disk device of network type. * Any others?
For these situations, we need to do checking and throw straightforward warnings to tell user why it can't be removed/wiped.
I would rather make this a flag of virDomainUndefine(), except there is no flag argument for it :(
I don't think this is a good idea. Applications should directly call the storage APIs for this, so that when we adding RBAC support to our APIs, we get correct access control checks on *each* volume being deleted / wiped. We won't want the 'undefine' API for a VM to be side-stepping the volume access controls. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Thu, Mar 31, 2011 at 11:30:03AM +0100, Daniel P. Berrange wrote:
On Wed, Mar 30, 2011 at 09:50:19PM +0800, Daniel Veillard wrote:
On Wed, Mar 30, 2011 at 09:39:14PM +0800, Osier Yang wrote:
For these situations, we need to do checking and throw straightforward warnings to tell user why it can't be removed/wiped.
I would rather make this a flag of virDomainUndefine(), except there is no flag argument for it :(
I don't think this is a good idea. Applications should directly call the storage APIs for this, so that when we adding RBAC support to our APIs, we get correct access control checks on *each* volume being deleted / wiped. We won't want the 'undefine' API for a VM to be side-stepping the volume access controls.
Well if the user don't have the resource to wipe all the volumes, fail the operation. Simple thing should stay simple even if we allow for more complex ones. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Thu, Mar 31, 2011 at 07:14:12PM +0800, Daniel Veillard wrote:
On Thu, Mar 31, 2011 at 11:30:03AM +0100, Daniel P. Berrange wrote:
On Wed, Mar 30, 2011 at 09:50:19PM +0800, Daniel Veillard wrote:
On Wed, Mar 30, 2011 at 09:39:14PM +0800, Osier Yang wrote:
For these situations, we need to do checking and throw straightforward warnings to tell user why it can't be removed/wiped.
I would rather make this a flag of virDomainUndefine(), except there is no flag argument for it :(
I don't think this is a good idea. Applications should directly call the storage APIs for this, so that when we adding RBAC support to our APIs, we get correct access control checks on *each* volume being deleted / wiped. We won't want the 'undefine' API for a VM to be side-stepping the volume access controls.
Well if the user don't have the resource to wipe all the volumes, fail the operation. Simple thing should stay simple even if we allow for more complex ones.
Error reporting is an even bigger problem, because if wiping some vols failed, but other succeeded, it is impossible for the caller to know what was actually done. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Wed, Mar 30, 2011 at 09:39:14PM +0800, Osier Yang wrote:
Hi, All
I'm thinking to introduce a new flag (something like --remove-disks, --wipe-disks) for "virsh undefine", so that the user can choose whether to remove/wipe the disk devices or not, have seen this requirement in many places, @libvirt-users, public #virt, and also we have a bug of this function. So, IMHO this is a reasonable requirement, following is the rough thoughts:
1) General idea. As we don't have a API which can get all the disk devices of a domain, perhaps need to write functions to parse domain xml to extract the disks' path (this is annoyed, but seems don't other way), and then lookup them by storage volume API (virStorageVolLookupByPath), and then can remove or wipe the volume by (virStorageVolDelete/virStorageVolWipe).
And for the disk path which doesn't belong to any storage pool, simply remove it by "unlink()"?
2) Which type of devices can not be removed/wiped.
Any device that's read/write by the guest should be wipeable.
* Can't delete/wipe ISCSI/SCSI vol. * Vol doesn't exists (which will throw an warning when do virStorageVolLookupByPath). * Have no write permission on the parent directory of the disk path. * Can't delete/wipe the disk device which is passthrough'ed from host, (e.g. /dev/sr0 as a CDROM device for guest) * The storage pool which the disk device belongs to as a vol is marked as "share" * The storage pool which the disk device belongs as a vol is readonly * can't delete disk device of network type. * Any others?
For these situations, we need to do checking and throw straightforward warnings to tell user why it can't be removed/wiped.
Any idea is welcomed. Thanks.
CC'ed Eric, as I saw you talked about this somewhere. (#virt?), :-)
Regards Osier
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
于 2011年03月30日 21:58, Dave Allan 写道:
On Wed, Mar 30, 2011 at 09:39:14PM +0800, Osier Yang wrote:
Hi, All
I'm thinking to introduce a new flag (something like --remove-disks, --wipe-disks) for "virsh undefine", so that the user can choose whether to remove/wipe the disk devices or not, have seen this requirement in many places, @libvirt-users, public #virt, and also we have a bug of this function. So, IMHO this is a reasonable requirement, following is the rough thoughts:
1) General idea. As we don't have a API which can get all the disk devices of a domain, perhaps need to write functions to parse domain xml to extract the disks' path (this is annoyed, but seems don't other way), and then lookup them by storage volume API (virStorageVolLookupByPath), and then can remove or wipe the volume by (virStorageVolDelete/virStorageVolWipe).
And for the disk path which doesn't belong to any storage pool, simply remove it by "unlink()"?
2) Which type of devices can not be removed/wiped.
Any device that's read/write by the guest should be wipeable.
Hum, make sense, :-)
* Can't delete/wipe ISCSI/SCSI vol. * Vol doesn't exists (which will throw an warning when do virStorageVolLookupByPath). * Have no write permission on the parent directory of the disk path. * Can't delete/wipe the disk device which is passthrough'ed from host, (e.g. /dev/sr0 as a CDROM device for guest) * The storage pool which the disk device belongs to as a vol is marked as "share" * The storage pool which the disk device belongs as a vol is readonly * can't delete disk device of network type. * Any others?
For these situations, we need to do checking and throw straightforward warnings to tell user why it can't be removed/wiped.
Any idea is welcomed. Thanks.
CC'ed Eric, as I saw you talked about this somewhere. (#virt?), :-)
Regards Osier
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 03/30/2011 07:39 AM, Osier Yang wrote:
Hi, All
I'm thinking to introduce a new flag (something like --remove-disks, --wipe-disks) for "virsh undefine", so that the user can choose whether to remove/wipe the disk devices or not, have seen this requirement in many places, @libvirt-users, public #virt, and also we have a bug of this function. So, IMHO this is a reasonable requirement, following is the rough thoughts:
I'm debating whether delete and wipe as two separate options make sense, or whether we only need one option. I guess for file-based volumes, there is a difference between leaving a wiped file behind and deleting the file altogether; but for most other volume types, wiping is the only option.
1) General idea. As we don't have a API which can get all the disk devices of a domain, perhaps need to write functions to parse domain xml to extract the disks' path (this is annoyed, but seems don't other way), and then lookup them by storage volume API (virStorageVolLookupByPath), and then can remove or wipe the volume by (virStorageVolDelete/virStorageVolWipe).
virt-manager has a gui option for deleting disks when deleting a domain; it achieves this by making multiple underlying API calls. You may want to use that implementation an example for a starting point. But it also has the advantage of listing all disks associated with the VM, and allowing fine-grain control over which volumes to keep or delete, whereas with virsh, it seems like a new --wipe-disks option would be all or none. At any rate, I think that this is all at the virsh level, and doesn't need any new API additions in include/libvirt/libvirt.h.in.
And for the disk path which doesn't belong to any storage pool, simply remove it by "unlink()"?
No, better would be to error out for any detected disk that cannot be resolved to a volume in a storage pool.
2) Which type of devices can not be removed/wiped.
* Can't delete/wipe ISCSI/SCSI vol.
Can't delete, but can wipe.
* Vol doesn't exists (which will throw an warning when do virStorageVolLookupByPath).
Can we tell the difference between a volume where the backing storage is located within an existing storage pool but the volume has already been deleted prior to deleting the guest XML, and the case of a guest XML that references a disk that does not belong to a storage pool? In the former case, requesting delete or wipe
* Have no write permission on the parent directory of the disk path. * Can't delete/wipe the disk device which is passthrough'ed from host, (e.g. /dev/sr0 as a CDROM device for guest)
/dev/sr0 would typically be a read-only volume. But I don't see why you can't wipe a read-write device passed through from the host (such as /dev/sda2).
* The storage pool which the disk device belongs to as a vol is marked as "share" * The storage pool which the disk device belongs as a vol is readonly * can't delete disk device of network type. * Any others?
For these situations, we need to do checking and throw straightforward warnings to tell user why it can't be removed/wiped.
Any idea is welcomed. Thanks.
Certainly post-0.9.0, whatever we come up with :) -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
于 2011年03月31日 01:11, Eric Blake 写道:
On 03/30/2011 07:39 AM, Osier Yang wrote:
Hi, All
I'm thinking to introduce a new flag (something like --remove-disks, --wipe-disks) for "virsh undefine", so that the user can choose whether to remove/wipe the disk devices or not, have seen this requirement in many places, @libvirt-users, public #virt, and also we have a bug of this function. So, IMHO this is a reasonable requirement, following is the rough thoughts:
I'm debating whether delete and wipe as two separate options make sense, or whether we only need one option. I guess for file-based volumes, there is a difference between leaving a wiped file behind and deleting the file altogether;
yes, for fs pool based volumes, they are different. So I guess some user will require both of them. but for most other volume types, wiping is the only
option.
1) General idea. As we don't have a API which can get all the disk devices of a domain, perhaps need to write functions to parse domain xml to extract the disks' path (this is annoyed, but seems don't other way), and then lookup them by storage volume API (virStorageVolLookupByPath), and then can remove or wipe the volume by (virStorageVolDelete/virStorageVolWipe).
virt-manager has a gui option for deleting disks when deleting a domain; it achieves this by making multiple underlying API calls. You may want to use that implementation an example for a starting point. But it also has the advantage of listing all disks associated with the VM, and allowing fine-grain control over which volumes to keep or delete, whereas with virsh, it seems like a new --wipe-disks option would be all or none.
Yes, it has dialog box which can ask user to choose. But for virsh, don't think ask user interactively is a good idea. :-)
At any rate, I think that this is all at the virsh level, and doesn't need any new API additions in include/libvirt/libvirt.h.in.
And for the disk path which doesn't belong to any storage pool, simply remove it by "unlink()"?
No, better would be to error out for any detected disk that cannot be resolved to a volume in a storage pool.
2) Which type of devices can not be removed/wiped.
* Can't delete/wipe ISCSI/SCSI vol.
Can't delete, but can wipe.
* Vol doesn't exists (which will throw an warning when do virStorageVolLookupByPath).
Can we tell the difference between a volume where the backing storage is located within an existing storage pool but the volume has already been deleted prior to deleting the guest XML, and the case of a guest XML that references a disk that does not belong to a storage pool?
In the former case, requesting delete or wipe
* Have no write permission on the parent directory of the disk path. * Can't delete/wipe the disk device which is passthrough'ed from host, (e.g. /dev/sr0 as a CDROM device for guest)
/dev/sr0 would typically be a read-only volume. But I don't see why you can't wipe a read-write device passed through from the host (such as /dev/sda2).
* The storage pool which the disk device belongs to as a vol is marked as "share" * The storage pool which the disk device belongs as a vol is readonly * can't delete disk device of network type. * Any others?
For these situations, we need to do checking and throw straightforward warnings to tell user why it can't be removed/wiped.
Any idea is welcomed. Thanks.
Certainly post-0.9.0, whatever we come up with :)
participants (6)
-
Daniel P. Berrange -
Daniel Veillard -
Dave Allan -
Eric Blake -
Jean-Baptiste Rouault -
Osier Yang