[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]