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 losing the last reference to a domain (undefine on an inactive, or shutdown/destroy on a transient). The caller must first take care of snapshots, possible via the existing virDomainSnapshotDelete. This also documents some new flags that hypervisors can choose to support to shortcuts taking care of the metadata as part of the shutdown 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 affect shutdown/destroy (the persistent domain still remains); likewise they never store snapshot metadata, so one of the two flags is trivial. The bulk of the nontrivial work remaining is thus in the qemu driver. * include/libvirt/libvirt.h.in (VIR_DOMAIN_UNDEFINE_SNAPSHOTS) (VIR_DOMAIN_DESTROY_SNAPSHOTS): New flags. * src/libvirt.c (virDomainUndefine, virDomainUndefineFlags) (virDomainDestroy, virDomainDestroyFlags, virDomainShutdown): Document new limitations and flags. * src/esx/esx_driver.c (esxDomainUndefineFlags): Enforce the limitations. * src/vbox/vbox_tmpl.c (vboxDomainUndefineFlags): Likewise. * src/qemu/qemu_driver.c (qemuDomainUndefineFlags) (qemuDomainShutdown, qemuDomainDestroyFlags): Likewise. --- include/libvirt/libvirt.h.in | 25 ++++++++++++++++++--- src/esx/esx_driver.c | 11 ++++++++- src/libvirt.c | 47 +++++++++++++++++++++++++++++++++++------ src/qemu/qemu_driver.c | 26 +++++++++++++++++++++++ src/vbox/vbox_tmpl.c | 11 ++++++++- 5 files changed, 105 insertions(+), 15 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c672145..e6509ec 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -919,10 +919,20 @@ virConnectPtr virDomainGetConnect (virDomainPtr domain); * Domain creation and destruction */ -/* - * typedef enum { - * } virDomainDestroyFlagsValues; + +/* Counterparts to virDomainUndefineFlagsValues, but note that running + * domains have no managed save data, so no flag is provided for that. */ +typedef enum { + /* VIR_DOMAIN_DESTROY_MANAGED_SAVE = (1 << 0), */ /* Reserved */ + VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA = (1 << 1), /* If last use of domain, + then also remove any + snapshot metadata */ + VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL = (1 << 2), /* If last use of domain, + then also remove any + snapshot data */ +} virDomainDestroyFlagsValues; + virDomainPtr virDomainCreateXML (virConnectPtr conn, const char *xmlDesc, unsigned int flags); @@ -1240,7 +1250,14 @@ 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 */ + VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL = (1 << 2), /* If last use of domain, + then also remove any + snapshot data */ /* Future undefine control flags should come here. */ } virDomainUndefineFlagsValues; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 837b774..c71b1b3 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1950,7 +1950,9 @@ esxDomainDestroyFlags(virDomainPtr domain, esxVI_TaskInfoState taskInfoState; char *taskInfoErrorMessage = NULL; - virCheckFlags(0, -1); + /* No transient domains, so these flags are trivially ignored. */ + virCheckFlags(VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA | + VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL, -1); if (priv->vCenter != NULL) { ctx = priv->vCenter; @@ -3309,7 +3311,9 @@ 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. */ + virCheckFlags(VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1); if (priv->vCenter != NULL) { ctx = priv->vCenter; @@ -3337,6 +3341,9 @@ esxDomainUndefineFlags(virDomainPtr domain, goto cleanup; } + /* ESX snapshots maintain no metadata, so we can trivially ignore + * that flag. XXX Wire up VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL. */ + if (esxVI_UnregisterVM(ctx, virtualMachine->obj) < 0) { goto cleanup; } diff --git a/src/libvirt.c b/src/libvirt.c index 3e031b6..deb95b2 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2039,6 +2039,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 the destroy will fail. See + * virDomainDestroyFlags() for more control. + * * Returns 0 in case of success and -1 in case of failure. */ int @@ -2088,7 +2092,20 @@ 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. + * + * If the domain is transient and has any snapshot metadata (see + * virDomainSnapshotNum()), then including + * VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA in @flags will also remove + * that metadata, including VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL will + * remove the metadata and snapshot data contents, and omitting both + * of these flags will cause the destroy process to fail. + * VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA and + * VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL have no effect if the domain is + * persistent, and VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA has no effect + * if libvirt does not maintain snapshot metadata. * * Returns 0 in case of success and -1 in case of failure. */ @@ -2856,12 +2873,16 @@ 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 the shutdown will fail. * * Returns 0 in case of success and -1 in case of failure. */ @@ -6808,8 +6829,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 the domain is active 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 */ @@ -6860,6 +6882,17 @@ 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, including VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL will + * remove the metadata and snapshot data contents, and omitting both + * of these flags will cause the undefine process to fail. + * VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA and + * VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL have no effect if the domain is + * active, and VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA has no effect if + * libvirt does not maintain snapshot metadata. + * * 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 ffea714..678ac1b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1466,6 +1466,7 @@ static int qemuDomainShutdown(virDomainPtr dom) { virDomainObjPtr vm; int ret = -1; qemuDomainObjPrivatePtr priv; + int nsnapshots; qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -1479,6 +1480,14 @@ static int qemuDomainShutdown(virDomainPtr dom) { goto cleanup; } + if (!vm->persistent && + (nsnapshots = virDomainSnapshotObjListNum(&vm->snapshots))) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("cannot delete transient domain with %d snapshots"), + nsnapshots); + goto cleanup; + } + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; @@ -1574,6 +1583,7 @@ qemuDomainDestroyFlags(virDomainPtr dom, int ret = -1; virDomainEventPtr event = NULL; qemuDomainObjPrivatePtr priv; + int nsnapshots; virCheckFlags(0, -1); @@ -1587,6 +1597,14 @@ qemuDomainDestroyFlags(virDomainPtr dom, goto cleanup; } + if (!vm->persistent && + (nsnapshots = virDomainSnapshotObjListNum(&vm->snapshots))) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("cannot delete transient domain with %d snapshots"), + nsnapshots); + goto cleanup; + } + priv = vm->privateData; priv->fakeReboot = false; @@ -4706,6 +4724,7 @@ qemuDomainUndefineFlags(virDomainPtr dom, virDomainEventPtr event = NULL; char *name = NULL; int ret = -1; + int nsnapshots; virCheckFlags(VIR_DOMAIN_UNDEFINE_MANAGED_SAVE, -1); @@ -4720,10 +4739,17 @@ qemuDomainUndefineFlags(virDomainPtr dom, goto cleanup; } + /* XXX We should allow undefine to convert running persistent into + * transient domain, rather than reject things here. */ if (virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot delete active domain")); goto cleanup; + } else if ((nsnapshots = virDomainSnapshotObjListNum(&vm->snapshots))) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("cannot delete inactive domain with %d snapshots"), + nsnapshots); + goto cleanup; } if (!vm->persistent) { diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index fa19aaa..91a5423 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -1701,7 +1701,9 @@ vboxDomainDestroyFlags(virDomainPtr dom, PRBool isAccessible = PR_FALSE; nsresult rc; - virCheckFlags(0, -1); + /* No transient domains, so these flags are trivially ignored. */ + virCheckFlags(VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA | + VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL, -1); vboxIIDFromUUID(&iid, dom->uuid); rc = VBOX_OBJECT_GET_MACHINE(iid.value, &machine); @@ -4982,10 +4984,15 @@ 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. */ + virCheckFlags(VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, -1); vboxIIDFromUUID(&iid, dom->uuid); + /* VBox snapshots maintain no metadata, so we can trivially ignore + * that flag. XXX Wire up VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL. */ + #if VBOX_API_VERSION < 4000 /* Block for checking if HDD's are attched to VM. * considering just IDE bus for now. Also skipped -- 1.7.4.4

As more clients start to want to know this information, doing a PATH stat walk and malloc for every client adds up. * src/qemu/qemu_conf.h (qemud_driver): Add member. * src/qemu/qemu_driver.c (qemudShutdown): Cleanup. (qemuFindQemuImgBinary): Add an argument, and cache result. (qemuDomainSnapshotDiscard, qemuDomainSnapshotCreateInactive) (qemuDomainSnapshotRevertInactive, qemuDomainSnapshotCreateXML) (qemuDomainRevertToSnapshot): Update callers. --- src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 42 +++++++++++++++++++++--------------------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 0a60d32..5469a63 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -82,6 +82,7 @@ struct qemud_driver { char *cacheDir; char *saveDir; char *snapshotDir; + char *qemuImgBinary; unsigned int vncAutoUnixSocket : 1; unsigned int vncTLS : 1; unsigned int vncTLSx509verify : 1; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d9e88fe..08d5a27 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -762,6 +762,7 @@ qemudShutdown(void) { VIR_FREE(qemu_driver->cacheDir); VIR_FREE(qemu_driver->saveDir); VIR_FREE(qemu_driver->snapshotDir); + VIR_FREE(qemu_driver->qemuImgBinary); VIR_FREE(qemu_driver->autoDumpPath); VIR_FREE(qemu_driver->vncTLSx509certdir); VIR_FREE(qemu_driver->vncListen); @@ -1582,19 +1583,19 @@ struct snap_remove { }; /* Locate an appropriate 'qemu-img' binary. */ -static char * -qemuFindQemuImgBinary(void) +static const char * +qemuFindQemuImgBinary(struct qemud_driver *driver) { - char *ret; - - ret = virFindFileInPath("kvm-img"); - if (ret == NULL) - ret = virFindFileInPath("qemu-img"); - if (ret == NULL) - qemuReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("unable to find kvm-img or qemu-img")); + if (!driver->qemuImgBinary) { + driver->qemuImgBinary = virFindFileInPath("kvm-img"); + if (!driver->qemuImgBinary) + driver->qemuImgBinary = virFindFileInPath("qemu-img"); + if (!driver->qemuImgBinary) + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unable to find kvm-img or qemu-img")); + } - return ret; + return driver->qemuImgBinary; } /* Discard one snapshot (or its metadata), without reparenting any children. */ @@ -1613,7 +1614,7 @@ qemuDomainSnapshotDiscard(struct qemud_driver *driver, if (!metadata_only) { if (!virDomainObjIsActive(vm)) { - qemuimgarg[0] = qemuFindQemuImgBinary(); + qemuimgarg[0] = qemuFindQemuImgBinary(driver); if (qemuimgarg[0] == NULL) /* qemuFindQemuImgBinary set the error */ goto cleanup; @@ -1681,7 +1682,6 @@ qemuDomainSnapshotDiscard(struct qemud_driver *driver, cleanup: VIR_FREE(snapFile); - VIR_FREE(qemuimgarg[0]); return ret; } @@ -8505,14 +8505,15 @@ static int qemuDomainSnapshotIsAllowed(virDomainObjPtr vm) /* The domain is expected to be locked and inactive. */ static int -qemuDomainSnapshotCreateInactive(virDomainObjPtr vm, +qemuDomainSnapshotCreateInactive(struct qemud_driver *driver, + virDomainObjPtr vm, virDomainSnapshotObjPtr snap) { const char *qemuimgarg[] = { NULL, "snapshot", "-c", NULL, NULL, NULL }; int ret = -1; int i; - qemuimgarg[0] = qemuFindQemuImgBinary(); + qemuimgarg[0] = qemuFindQemuImgBinary(driver); if (qemuimgarg[0] == NULL) { /* qemuFindQemuImgBinary set the error */ goto cleanup; @@ -8545,7 +8546,6 @@ qemuDomainSnapshotCreateInactive(virDomainObjPtr vm, ret = 0; cleanup: - VIR_FREE(qemuimgarg[0]); return ret; } @@ -8643,7 +8643,7 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain, /* actually do the snapshot */ if (!virDomainObjIsActive(vm)) { - if (qemuDomainSnapshotCreateInactive(vm, snap) < 0) + if (qemuDomainSnapshotCreateInactive(driver, vm, snap) < 0) goto cleanup; } else { if (qemuDomainSnapshotCreateActive(domain->conn, driver, @@ -8880,14 +8880,15 @@ cleanup: /* The domain is expected to be locked and inactive. */ static int -qemuDomainSnapshotRevertInactive(virDomainObjPtr vm, +qemuDomainSnapshotRevertInactive(struct qemud_driver *driver, + virDomainObjPtr vm, virDomainSnapshotObjPtr snap) { const char *qemuimgarg[] = { NULL, "snapshot", "-a", NULL, NULL, NULL }; int ret = -1; int i; - qemuimgarg[0] = qemuFindQemuImgBinary(); + qemuimgarg[0] = qemuFindQemuImgBinary(driver); if (qemuimgarg[0] == NULL) { /* qemuFindQemuImgBinary set the error */ goto cleanup; @@ -8920,7 +8921,6 @@ qemuDomainSnapshotRevertInactive(virDomainObjPtr vm, ret = 0; cleanup: - VIR_FREE(qemuimgarg[0]); return ret; } @@ -9045,7 +9045,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, } } - if (qemuDomainSnapshotRevertInactive(vm, snap) < 0) + if (qemuDomainSnapshotRevertInactive(driver, vm, snap) < 0) goto endjob; } -- 1.7.4.4

Similar to 'undefine --managed-save' (commit 83e849c1), we must assume that the old API is unsafe, and emulate it ourselves. Additionally, we have the wrinkle that while virDomainUndefineFlags and managed save cleanup were introduced in 0.9.4, it wasn't until 0.9.5 that snapshots block undefine of a domain. Simulate as much as possible with older servers, but the --snapshots-metadata support requires a server >= 0.9.5. Same story for virDomainDestroyFlags, and virDomainShutdownFlags doesn't exist yet. Oh well. * tools/virsh.c (cmdUndefine, cmdDestroy, cmdShutdown): Add --snapshots-full and --snapshots-metadata flags. (vshRemoveAllSnapshots): New helper method. * tools/virsh.pod (undefine, destroy, shutdown): Document them. --- tools/virsh.c | 400 ++++++++++++++++++++++++++++++++++++++++++++++++------- tools/virsh.pod | 36 +++++- 2 files changed, 386 insertions(+), 50 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index c094911..7275e4c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1410,6 +1410,59 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd) return ret; } +/* Helper for undefine, shutdown, and destroy */ +static int +vshRemoveAllSnapshots(virDomainPtr dom, int nsnapshots, bool full) +{ + int ret = -1; + char **names; + int actual; + int i; + virDomainSnapshotPtr snapshot = NULL; + int flags = 0; + + /* It's likely that if virDomainUndefineFlags was unsupported to + * get us here in the first place, then + * VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY will also be + * unsupported. But better to hear that from the driver. + */ + if (!full) + flags |= VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY; + + if (VIR_ALLOC_N(names, nsnapshots) < 0) + goto cleanup; + + actual = virDomainSnapshotListNames(dom, names, nsnapshots, 0); + if (actual < 0) + goto cleanup; + + /* Sadly, this is an O(n) loop over a function call that is also a + * minimum of O(n), for a complexity of O(n^2), whereas newer + * servers that support the delete in the undefine action are + * O(n). Oh well. */ + for (i = 0; i < actual; i++) { + if (snapshot) + virDomainSnapshotFree(snapshot); + + snapshot = virDomainSnapshotLookupByName(dom, names[i], 0); + if (snapshot == NULL) + continue; + + if (virDomainSnapshotDelete(snapshot, flags) < 0) + goto cleanup; + } + + ret = 0; + +cleanup: + if (snapshot) + virDomainSnapshotFree(snapshot); + for (i = 0; i < actual; i++) + VIR_FREE(names[i]); + VIR_FREE(names); + return ret; +} + /* * "undefine" command */ @@ -1422,6 +1475,10 @@ static const vshCmdInfo info_undefine[] = { static const vshCmdOptDef opts_undefine[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name or uuid")}, {"managed-save", VSH_OT_BOOL, 0, N_("remove domain managed state file")}, + {"snapshots-metadata", VSH_OT_BOOL, 0, + N_("remove all domain snapshot metadata, if inactive")}, + {"snapshots-full", VSH_OT_BOOL, 0, + N_("remove all domain snapshot contents, if inactive")}, {NULL, 0, 0, NULL} }; @@ -1429,18 +1486,36 @@ static bool cmdUndefine(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; - bool ret = true; + bool ret = false; const char *name = NULL; + /* Flags to attempt. */ int flags = 0; - int managed_save = vshCommandOptBool(cmd, "managed-save"); + /* User-requested actions. */ + bool managed_save = vshCommandOptBool(cmd, "managed-save"); + bool snapshots_metadata = vshCommandOptBool(cmd, "snapshots-metadata"); + bool snapshots_full = vshCommandOptBool(cmd, "snapshots-full"); + /* Positive if these items exist. */ int has_managed_save = 0; + int has_snapshots_metadata = 0; + int has_snapshots = 0; + /* True if undefine will not strand data, even on older servers. */ + bool managed_save_safe = false; + bool snapshots_safe = false; int rc = -1; + int running; - if (managed_save) + if (managed_save) { flags |= VIR_DOMAIN_UNDEFINE_MANAGED_SAVE; - - if (!managed_save) - flags = -1; + managed_save_safe = true; + } + if (snapshots_metadata) { + flags |= VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA; + snapshots_safe = true; + } + if (snapshots_full) { + flags |= VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL; + snapshots_safe = true; + } if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -1448,61 +1523,124 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; - has_managed_save = virDomainHasManagedSaveImage(dom, 0); - if (has_managed_save < 0) { - if (last_error->code != VIR_ERR_NO_SUPPORT) { - virshReportError(ctl); - virDomainFree(dom); - return false; - } else { + /* Do some flag manipulation. The goal here is to disable bits + * from flags to reduce the likelihood of a server rejecting + * unknown flag bits, as well as to track conditions which are + * safe by default for the given hypervisor and server version. */ + running = virDomainIsActive(dom); + if (running < 0) { + virshReportError(ctl); + goto cleanup; + } + if (!running) { + /* Undefine with snapshots only fails for inactive domains, + * and managed save only exists on inactive domains; if + * running, then we don't want to remove anything. */ + has_managed_save = virDomainHasManagedSaveImage(dom, 0); + if (has_managed_save < 0) { + if (last_error->code != VIR_ERR_NO_SUPPORT) { + virshReportError(ctl); + goto cleanup; + } virFreeError(last_error); last_error = NULL; - } - } - - if (flags == -1) { - if (has_managed_save == 1) { - vshError(ctl, - _("Refusing to undefine while domain managed save " - "image exists")); - virDomainFree(dom); - return false; + has_managed_save = 0; } - rc = virDomainUndefine(dom); - } else { - rc = virDomainUndefineFlags(dom, flags); - - /* It might fail when virDomainUndefineFlags is not - * supported on older libvirt, try to undefine the - * domain with combo virDomainManagedSaveRemove and - * virDomainUndefine. - */ - if (rc < 0) { + has_snapshots = virDomainSnapshotNum(dom, 0); + if (has_snapshots < 0) { if (last_error->code != VIR_ERR_NO_SUPPORT) { virshReportError(ctl); - goto end; - } else { + goto cleanup; + } + virFreeError(last_error); + last_error = NULL; + has_snapshots = 0; + } + if (has_snapshots) { + has_snapshots_metadata + = virDomainSnapshotNum(dom, VIR_DOMAIN_SNAPSHOT_NUM_METADATA); + if (has_snapshots_metadata < 0) { + /* The server did not know the new flag, assume that all + snapshots have metadata. */ virFreeError(last_error); last_error = NULL; + has_snapshots_metadata = has_snapshots; + } else { + /* The server knew the new flag, all aspects of + * undefineFlags are safe. */ + managed_save_safe = snapshots_safe = true; } + } + } + if (!has_managed_save) { + flags &= ~VIR_DOMAIN_UNDEFINE_MANAGED_SAVE; + managed_save_safe = true; + } + if (has_snapshots == 0) { + flags &= ~VIR_DOMAIN_UNDEFINE_SNAPSHOTS_FULL; + snapshots_safe = true; + } + if (has_snapshots_metadata == 0) { + flags &= ~VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA; + snapshots_safe = true; + } - if ((has_managed_save == 1) && - virDomainManagedSaveRemove(dom, 0) < 0) - goto end; + /* Generally we want to try the new API first. However, while + * virDomainUndefineFlags was introduced at the same time as + * VIR_DOMAIN_UNDEFINE_MANAGED_SAVE in 0.9.4, the + * VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA flag was not present + * until 0.9.5; skip to piecewise emulation if we couldn't prove + * above that the new API is safe. */ + if (managed_save_safe && snapshots_safe) { + rc = virDomainUndefineFlags(dom, flags); + if (rc == 0 || (last_error->code != VIR_ERR_NO_SUPPORT && + last_error->code != VIR_ERR_INVALID_ARG)) + goto out; + virFreeError(last_error); + last_error = NULL; + } + + /* The new API is unsupported or unsafe; fall back to doing things + * piecewise. */ + if (has_managed_save) { + if (!managed_save) { + vshError(ctl, "%s", + _("Refusing to undefine while domain managed save " + "image exists")); + goto cleanup; + } + if (virDomainManagedSaveRemove(dom, 0) < 0) { + virshReportError(ctl); + goto cleanup; + } + } - rc = virDomainUndefine(dom); + if (has_snapshots) { + if (has_snapshots_metadata && + !(snapshots_metadata || snapshots_full)) { + vshError(ctl, + _("Refusing to undefine while %d snapshots exist"), + has_snapshots); + goto cleanup; } + + if ((snapshots_metadata || snapshots_full) && + vshRemoveAllSnapshots(dom, has_snapshots, snapshots_full) < 0) + goto cleanup; } -end: + rc = virDomainUndefine(dom); + +out: if (rc == 0) { vshPrint(ctl, _("Domain %s has been undefined\n"), name); + ret = true; } else { vshError(ctl, _("Failed to undefine domain %s"), name); - ret = false; } +cleanup: virDomainFree(dom); return ret; } @@ -2488,6 +2626,10 @@ static const vshCmdInfo info_shutdown[] = { static const vshCmdOptDef opts_shutdown[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"snapshots-metadata", VSH_OT_BOOL, 0, + N_("remove all domain snapshot metadata, if transient")}, + {"snapshots-full", VSH_OT_BOOL, 0, + N_("remove all domain snapshot contents, if transient")}, {NULL, 0, 0, NULL} }; @@ -2495,8 +2637,15 @@ static bool cmdShutdown(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; - bool ret = true; + bool ret = false; const char *name; + /* User-requested actions. */ + bool snapshots_metadata = vshCommandOptBool(cmd, "snapshots-metadata"); + bool snapshots_full = vshCommandOptBool(cmd, "snapshots-full"); + /* Positive if these items exist. */ + int has_snapshots_metadata = 0; + int has_snapshots = 0; + int persistent; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -2504,13 +2653,67 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; + /* Do some flag manipulation. The goal here is to track + * conditions which are safe by default for the given hyperviser + * and server version. */ + persistent = virDomainIsPersistent(dom); + if (persistent < 0) { + virshReportError(ctl); + goto cleanup; + } + if (!persistent) { + /* Snapshot safety is only needed for transient domains. */ + has_snapshots = virDomainSnapshotNum(dom, 0); + if (has_snapshots < 0) { + if (last_error->code != VIR_ERR_NO_SUPPORT) { + virshReportError(ctl); + goto cleanup; + } + virFreeError(last_error); + last_error = NULL; + has_snapshots = 0; + } + if (has_snapshots) { + has_snapshots_metadata + = virDomainSnapshotNum(dom, VIR_DOMAIN_SNAPSHOT_NUM_METADATA); + if (has_snapshots_metadata < 0) { + /* The server did not know the new flag, assume that all + snapshots have metadata. */ + virFreeError(last_error); + last_error = NULL; + has_snapshots_metadata = has_snapshots; + } + } + } + + /* XXX Once virDomainShutdownFlags is added, use it here. Since + * snapshot shutdown safety was introduced before the new API, we + * don't have to go through quite as many contortions as + * cmdDestroy to check for snapshot safety. */ + + /* Fall back to doing things piecewise. */ + if (has_snapshots) { + if (has_snapshots_metadata && + !(snapshots_metadata || snapshots_full)) { + vshError(ctl, + _("Refusing to shutdown while %d snapshots exist"), + has_snapshots); + goto cleanup; + } + + if ((snapshots_metadata || snapshots_full) && + vshRemoveAllSnapshots(dom, has_snapshots, snapshots_full) < 0) + goto cleanup; + } + if (virDomainShutdown(dom) == 0) { vshPrint(ctl, _("Domain %s is being shutdown\n"), name); + ret = true; } else { vshError(ctl, _("Failed to shutdown domain %s"), name); - ret = false; } +cleanup: virDomainFree(dom); return ret; } @@ -2565,6 +2768,10 @@ static const vshCmdInfo info_destroy[] = { static const vshCmdOptDef opts_destroy[] = { {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"snapshots-metadata", VSH_OT_BOOL, 0, + N_("remove all domain snapshot metadata, if transient")}, + {"snapshots-full", VSH_OT_BOOL, 0, + N_("remove all domain snapshot contents, if transient")}, {NULL, 0, 0, NULL} }; @@ -2572,8 +2779,29 @@ static bool cmdDestroy(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; - bool ret = true; + bool ret = false; const char *name; + /* Flags to attempt. */ + int flags = 0; + /* User-requested actions. */ + bool snapshots_metadata = vshCommandOptBool(cmd, "snapshots-metadata"); + bool snapshots_full = vshCommandOptBool(cmd, "snapshots-full"); + /* Positive if these items exist. */ + int has_snapshots_metadata = 0; + int has_snapshots = 0; + /* True if undefine will not strand data, even on older servers. */ + bool snapshots_safe = false; + int rc = -1; + int persistent; + + if (snapshots_metadata) { + flags |= VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA; + snapshots_safe = true; + } + if (snapshots_full) { + flags |= VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL; + snapshots_safe = true; + } if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -2581,13 +2809,93 @@ cmdDestroy(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; - if (virDomainDestroy(dom) == 0) { + /* Do some flag manipulation. The goal here is to disable bits + * from flags to reduce the likelihood of a server rejecting + * unknown flag bits, as well as to track conditions which are + * safe by default for the given hypervisor and server version. */ + persistent = virDomainIsPersistent(dom); + if (persistent < 0) { + virshReportError(ctl); + goto cleanup; + } + if (!persistent) { + /* Snapshot safety is only needed for transient domains. */ + has_snapshots = virDomainSnapshotNum(dom, 0); + if (has_snapshots < 0) { + if (last_error->code != VIR_ERR_NO_SUPPORT) { + virshReportError(ctl); + goto cleanup; + } + virFreeError(last_error); + last_error = NULL; + has_snapshots = 0; + } + if (has_snapshots) { + has_snapshots_metadata + = virDomainSnapshotNum(dom, VIR_DOMAIN_SNAPSHOT_NUM_METADATA); + if (has_snapshots_metadata < 0) { + /* The server did not know the new flag, assume that all + snapshots have metadata. */ + virFreeError(last_error); + last_error = NULL; + has_snapshots_metadata = has_snapshots; + } else { + /* The server knew the new flag, all aspects of + * destroyFlags are safe. */ + snapshots_safe = true; + } + } + } + if (has_snapshots == 0) { + flags &= ~VIR_DOMAIN_DESTROY_SNAPSHOTS_FULL; + snapshots_safe = true; + } + if (has_snapshots_metadata == 0) { + flags &= ~VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA; + snapshots_safe = true; + } + + /* Generally we want to try the new API first. However, while + * virDomainDestroyFlags was introduced in 0.9.4, the + * VIR_DOMAIN_DESTROY_SNAPSHOTS_METADATA flag was not present + * until 0.9.5; skip to piecewise emulation if we couldn't prove + * above that the new API is safe. */ + if (snapshots_safe) { + rc = virDomainDestroyFlags(dom, flags); + if (rc == 0 || (last_error->code != VIR_ERR_NO_SUPPORT && + last_error->code != VIR_ERR_INVALID_ARG)) + goto out; + virFreeError(last_error); + last_error = NULL; + } + + /* The new API is unsupported or unsafe; fall back to doing things + * piecewise. */ + if (has_snapshots) { + if (has_snapshots_metadata && + !(snapshots_metadata || snapshots_full)) { + vshError(ctl, + _("Refusing to destroy while %d snapshots exist"), + has_snapshots); + goto cleanup; + } + + if ((snapshots_metadata || snapshots_full) && + vshRemoveAllSnapshots(dom, has_snapshots, snapshots_full) < 0) + goto cleanup; + } + + rc = virDomainDestroy(dom); + +out: + if (rc == 0) { vshPrint(ctl, _("Domain %s destroyed\n"), name); + ret = true; } else { vshError(ctl, _("Failed to destroy domain %s"), name); - ret = false; } +cleanup: virDomainFree(dom); return ret; } diff --git a/tools/virsh.pod b/tools/virsh.pod index 11a13fd..c27e99d 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -405,7 +405,7 @@ B<Example> Define a domain from an XML <file>. The domain definition is registered but not started. -=item B<destroy> I<domain-id> +=item B<destroy> I<domain-id> [I<--snapshots-full>] [I<--snapshots-metadata>] Immediately terminate the domain domain-id. This doesn't give the domain OS any chance to react, and it's the equivalent of ripping the power @@ -414,6 +414,15 @@ the B<shutdown> command instead. However, this does not delete any storage volumes used by the guest, and if the domain is persistent, it can be restarted later. +The I<--snapshots-full> and I<--snapshots-metadata> flags guarantee +that any snapshots (see the B<snapshot-list> command) are also cleaned +up when stopping a transient domain. Without either flag, attempts +to destroy a transient domain with snapshot metadata will fail. The +I<--snapshots-full> flag works with older servers, but loses all +snapshot contents; the I<--snapshots-metadata> flag only removes +metadata that can later be recreated, but requires newer servers. If +the domain is persistent, these flags are ignored. + =item B<domblkstat> I<domain> I<block-device> Get device block stats for a running domain. @@ -867,7 +876,7 @@ The I<--maximum> flag controls the maximum number of virtual cpus that can be hot-plugged the next time the domain is booted. As such, it must only be used with the I<--config> flag, and not with the I<--live> flag. -=item B<shutdown> I<domain-id> +=item B<shutdown> I<domain-id> [I<--snapshots-full>] [I<--snapshots-metadata>] Gracefully shuts down a domain. This coordinates with the domain OS to perform graceful shutdown, so there is no guarantee that it will @@ -877,6 +886,15 @@ services must be shutdown in the domain. The exact behavior of a domain when it shuts down is set by the I<on_shutdown> parameter in the domain's XML definition. +The I<--snapshots-full> and I<--snapshots-metadata> flags guarantee +that any snapshots (see the B<snapshot-list> command) are also cleaned +up when shutting down a transient domain. Without either flag, attempts +to shutdown a transient domain with snapshot metadata will fail. The +I<--snapshots-full> flag works with older servers, but loses all +snapshot contents; the I<--snapshots-metadata> flag only removes +metadata that can later be recreated, but requires newer servers. If +the domain is persistent, these flags are ignored. + =item B<start> I<domain-name> [I<--console>] [I<--paused>] [I<--autodestroy>] [I<--bypass-cache>] @@ -907,16 +925,26 @@ hypervisor. Output the device used for the TTY console of the domain. If the information is not available the processes will provide an exit code of 1. -=item B<undefine> I<domain-id> [I<--managed-save>] +=item B<undefine> I<domain-id> [I<--managed-save>] [I<--snapshots-full>] +[I<--snapshots-metadata] Undefine a domain. If the domain is running, this converts it to a transient domain, without stopping it. If the domain is inactive, the domain configuration is removed. -The I<--managed-save> flag guarantees that any managed save image(see +The I<--managed-save> flag guarantees that any managed save image (see the B<managedsave> command) is also cleaned up. Without the flag, attempts to undefine a domain with a managed save image will fail. +The I<--snapshots-full> and I<--snapshots-metadata> flags guarantee +that any snapshots (see the B<snapshot-list> command) are also cleaned +up when undefining an inactive domain. Without either flag, attempts +to undefine an inactive domain with snapshot metadata will fail. The +I<--snapshots-full> flag works with older servers, but loses all +snapshot contents; the I<--snapshots-metadata> flag only removes +metadata that can later be recreated, but requires newer servers. If +the domain is active, these flags are ignored. + NOTE: For an inactive domain, the domain name or UUID must be used as the I<domain-id>. -- 1.7.4.4