[libvirt] [PATCH RFC 1/3] undefine: Add flags for removing storage files when undefining a domain.

Eric Blake eblake at redhat.com
Thu Jul 21 16:52:36 UTC 2011


On 07/21/2011 09:12 AM, Peter Krempa wrote:
> Adds option for virDomainUndefineFlags to remove storage associated with
> the domain while undefining it.
>
> VIR_DOMAIN_UNDEFINE_DISK_FILE - remove disk files, that aren't managed by
> storage pools.
>
> VIR_DOMAIN_UNDEFINE_DISK_BACKEND - remove storage devices that are managed
> using libvirt storage pools.
> ---
>   include/libvirt/libvirt.h.in |    2 ++
>   1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 40ce0fc..f99bc8b 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1203,6 +1203,8 @@ int                     virDomainUndefine       (virDomainPtr domain);
>
>   typedef enum {
>       VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = (1<<  0),
> +    VIR_DOMAIN_UNDEFINE_DISK_FILE = (1<<  1),
> +    VIR_DOMAIN_UNDEFINE_DISK_BACKEND = (1<<  2),

At first glance, I like the idea; however, it needs documentation in 
libvirt.c (that is, the official virDomainUndefineFlags must mention the 
semantic of these flags, so that we can uniformly implement those 
semantics across all hypervisors where we add support).  The names could 
be tweaked:

VIR_DOMAIN_UNDEFINE_REMOVE_UNMANAGED_DISKS - affect only disks not 
belonging to a pool
VIR_DOMAIN_UNDEFINE_REMOVE_MANAGED_DISKS - affect only disks belonging 
to a pool

But on thinking further...

We have to be careful - since undefine is supposed to work on running 
guests (turning persistent into transient), these flags must cause 
undefine to fail on a running guest.  Conversely, if we have a transient 
guest, we use virDomainShutdown/virDomainDestroy to end the domain, not 
virDomainUndefine, so those APIs would _also_ need the same flags 
argument, to give the same semantics of storage cleanup on death of the 
domain.

Are those flags sufficient?  What about the distinction in 
readonly/shared/readwrite disks - do you delete a readonly .iso file 
mounted as a virtual cdrom disk, or just readwrite storage?

Meanwhile, this is all-or-nothing, on the other hand, if you have a 
scenario where you want some, but not all, disks removed, then you still 
have access to the virStorageVol APIs to do that removal, at least for 
managed disks.  I don't know if there is any way to make it more 
fine-tuned, short of adding some XML attributes to each <disk> element 
of the domain definition on whether the backing storage of the disk gets 
auto-deleted on undefine.

And if you update XML to support auto-deletion of a disk on domain, then 
it can be argued that you no longer need this flag on the 
virDomainUndefineFlags path - instead, the undefine path inspects the 
XML and fails if the requested auto-deletion fails.  Or maybe we combine 
the aspects - the XML has an optional element to say what happens on 
undefine, but for every disk where that element is not present, then: if 
the disk is shared or readonly, the default is leave the storage alone, 
and if the disk is readwrite, the default is to raise an undefine error 
if no flags were passed.  Then you have two mutually exclusive flags to 
undefine - one says to change a default error to instead leave the 
storage untouched, and the other says to change a default error to 
instead remove the storage.  Neither flag is needed if the XML fully 
specifies the behavior on all disks.

Meanwhile, this points out that we are probably missing some API for 
unmanaged disks (that is, a volume in use by a domain, but not belonging 
to a storage pool).  Other holes - we don't have any way, given a 
virStorageVolPtr, to get back to the virStoragePoolPtr that contains it 
(or, if we use virStorageVolPtr for unmanaged disks, to return an error 
indicating that it is unmanaged).  Also, we don't have a way, given a 
virDomainPtr, to enumerate all the storage volumes associated with that 
domain as a list of virStorageVolPtr.

Adding more APIs for storage volume management would also be useful for 
my work with snapshots.  I guess we add things incrementally as we go.

Anyways, thanks for bringing up the ideas - there's lots of potential 
for improvement in this area!

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org




More information about the libvir-list mailing list