[libvirt] [RFC] Add flag for virsh undefine to remove/wipe the disk devices

[Daniel P. Berrange]

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 :|