Just as leaving managed save metadata behind can cause problems when creating a new domain that happens to collide with the name of the just-deleted domain, the same is true of leaving any snapshot metadata behind. For safety sake, extend the semantic change of commit b26a9fa9 to also cover snapshot metadata as a reason to reject undefining an inactive domain. A future patch will make sure that shutdown of a transient domain automatically deletes snapshot metadata (whether by destroy, shutdown, or guest-initiated action). Management apps of transient domains should take care to capture xml of snapshots, if it is necessary to recreate the snapshot metadata on a later transient domain with the same name and uuid. This also documents a new flag that hypervisors can choose to support as a shortcut for taking care of the metadata as part of the undefine process; however, nontrivial driver support for these flags will be deferred to future patches. Note that ESX and VBox can never be transient; therefore, they do not have to worry about automatic cleanup after shutdown (the persistent domain still remains); likewise they never store snapshot metadata, so the undefine flag is trivial. The nontrivial work remaining is thus in the qemu driver. * include/libvirt/libvirt.h.in (VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA): New flag. * src/libvirt.c (virDomainUndefine, virDomainUndefineFlags): Document new limitations and flag. * src/esx/esx_driver.c (esxDomainUndefineFlags): Trivial implementation. * src/vbox/vbox_tmpl.c (vboxDomainUndefineFlags): Likewise. * src/qemu/qemu_driver.c (qemuDomainUndefineFlags): Enforce the limitations. --- include/libvirt/libvirt.h.in | 8 +++++++- src/esx/esx_driver.c | 5 ++++- src/libvirt.c | 35 ++++++++++++++++++++++++++++------- src/qemu/qemu_driver.c | 9 +++++++++ src/vbox/vbox_tmpl.c | 5 ++++- 5 files changed, 52 insertions(+), 10 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 00b8350..4a5dbff 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -923,10 +923,12 @@ virConnectPtr virDomainGetConnect (virDomainPtr domain); * Domain creation and destruction */ + /* * typedef enum { * } virDomainDestroyFlagsValues; */ + virDomainPtr virDomainCreateXML (virConnectPtr conn, const char *xmlDesc, unsigned int flags); @@ -1247,7 +1249,11 @@ virDomainPtr virDomainDefineXML (virConnectPtr conn, int virDomainUndefine (virDomainPtr domain); typedef enum { - VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = (1 << 0), + VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = (1 << 0), /* Also remove any + managed save */ + VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA = (1 << 1), /* If last use of domain, + then also remove any + snapshot metadata */ /* Future undefine control flags should come here. */ } virDomainUndefineFlagsValues; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index e004324..fb5b1a2 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -3309,7 +3309,10 @@ esxDomainUndefineFlags(virDomainPtr domain, esxVI_String *propertyNameList = NULL; esxVI_VirtualMachinePowerState powerState; - virCheckFlags(0, -1); + /* No managed save, so we explicitly reject + * VIR_DOMAIN_UNDEFINE_MANAGED_SAVE. No snapshot metadata for + * ESX, so we can trivially ignore that flag. */ + virCheckFlags(VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1); if (priv->vCenter != NULL) { ctx = priv->vCenter; diff --git a/src/libvirt.c b/src/libvirt.c index e3188b7..68d469f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2058,6 +2058,10 @@ error: * does not free the associated virDomainPtr object. * This function may require privileged access * + * If the domain is transient and has any snapshot metadata (see + * virDomainSnapshotNum()), then that metadata will automatically + * be deleted when the domain quits. + * * Returns 0 in case of success and -1 in case of failure. */ int @@ -2107,7 +2111,9 @@ error: * This function may require privileged access. * * Calling this function with no @flags set (equal to zero) - * is equivalent to calling virDomainDestroy. + * is equivalent to calling virDomainDestroy. Using virDomainShutdown() + * may produce cleaner results for the guest's disks, but depends on guest + * support. * * Returns 0 in case of success and -1 in case of failure. */ @@ -2912,12 +2918,17 @@ error: * virDomainShutdown: * @domain: a domain object * - * Shutdown a domain, the domain object is still usable there after but + * Shutdown a domain, the domain object is still usable thereafter but * the domain OS is being stopped. Note that the guest OS may ignore the - * request. + * request. For guests that react to a shutdown request, the differences + * from virDomainDestroy() are that the guests disk storage will be in a + * stable state rather than having the (virtual) power cord pulled, and + * this command returns as soon as the shutdown request is issued rather + * than blocking until the guest is no longer running. * - * TODO: should we add an option for reboot, knowing it may not be doable - * in the general case ? + * If the domain is transient and has any snapshot metadata (see + * virDomainSnapshotNum()), then that metadata will automatically + * be deleted when the domain quits. * * Returns 0 in case of success and -1 in case of failure. */ @@ -6879,8 +6890,9 @@ error: * the domain configuration is removed. * * If the domain has a managed save image (see - * virDomainHasManagedSaveImage()), then the undefine will fail. See - * virDomainUndefineFlags() for more control. + * virDomainHasManagedSaveImage()), or if it is inactive and has any + * snapshot metadata (see virDomainSnapshotNum()), then the undefine will + * fail. See virDomainUndefineFlags() for more control. * * Returns 0 in case of success, -1 in case of error */ @@ -6931,6 +6943,15 @@ error: * then including VIR_DOMAIN_UNDEFINE_MANAGED_SAVE in @flags will also remove * that file, and omitting the flag will cause the undefine process to fail. * + * If the domain is inactive and has any snapshot metadata (see + * virDomainSnapshotNum()), then including + * VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA in @flags will also remove + * that metadata. Omitting the flag will cause the undefine of an + * inactive domain to fail. Active snapshots will retain snapshot + * metadata until the (now-transient) domain halts, regardless of + * whether this flag is present. On hypervisors where snapshots do + * not use libvirt metadata, this flag has no effect. + * * Returns 0 in case of success, -1 in case of error */ int diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ac71b04..38a21db 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4838,6 +4838,7 @@ qemuDomainUndefineFlags(virDomainPtr dom, virDomainEventPtr event = NULL; char *name = NULL; int ret = -1; + int nsnapshots; virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE, -1); @@ -4852,6 +4853,14 @@ qemuDomainUndefineFlags(virDomainPtr dom, goto cleanup; } + if (!virDomainObjIsActive(vm) && + (nsnapshots = virDomainSnapshotObjListNum(&vm->snapshots, 0))) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("cannot delete inactive domain with %d snapshots"), + nsnapshots); + goto cleanup; + } + if (!vm->persistent) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot undefine transient domain")); diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 636f5f2..ebed4d9 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -4982,7 +4982,10 @@ vboxDomainUndefineFlags(virDomainPtr dom, unsigned int flags) #if VBOX_API_VERSION >= 4000 vboxArray media = VBOX_ARRAY_INITIALIZER; #endif - virCheckFlags(0, -1); + /* No managed save, so we explicitly reject + * VIR_DOMAIN_UNDEFINE_MANAGED_SAVE. No snapshot metadata for + * VBox, so we can trivially ignore that flag. */ + virCheckFlags(VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1); vboxIIDFromUUID(&iid, dom->uuid); -- 1.7.4.4 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list