Commit 19f8c98 introduced VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, with the intent that omitting the flag makes undefine fail, and including the flag deletes metadata. But it used the wrong logic. Also, hoist the transient domain sooner, so that we don't accidentally remove metadata of a transient domain. * src/qemu/qemu_driver.c (qemuDomainUndefineFlags): Check correct flag value. --- src/qemu/qemu_driver.c | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 67c43ab..4a24019 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5095,7 +5095,7 @@ cleanup: static int qemuDomainUndefineFlags(virDomainPtr dom, - unsigned int flags) + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; @@ -5118,11 +5118,17 @@ qemuDomainUndefineFlags(virDomainPtr dom, goto cleanup; } + if (!vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cannot undefine transient domain")); + goto cleanup; + } + if (!virDomainObjIsActive(vm) && (nsnapshots = virDomainSnapshotObjListNum(&vm->snapshots, 0))) { struct snap_remove rem; - if (flags & VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA) { + if (!(flags & VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA)) { qemuReportError(VIR_ERR_OPERATION_INVALID, _("cannot delete inactive domain with %d " "snapshots"), @@ -5139,12 +5145,6 @@ qemuDomainUndefineFlags(virDomainPtr dom, goto cleanup; } - if (!vm->persistent) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cannot undefine transient domain")); - goto cleanup; - } - name = qemuDomainManagedSavePath(driver, vm); if (name == NULL) goto cleanup; -- 1.7.4.4

This patch is mostly code motion - moving some functions out of qemu_driver and into qemu_domain so they can be reused by multiple qemu_* files (since qemu_driver.h must not grow). It also adds a new helper function, qemuDomainRemoveInactive, which will be used in the next patch. * src/qemu/qemu_domain.h (qemuFindQemuImgBinary) (qemuDomainSnapshotWriteMetadata, qemuDomainSnapshotForEachQcow2) (qemuDomainSnapshotDiscard, qemuDomainSnapshotDiscardAll) (qemuDomainRemoveInactive): New prototypes. (struct qemu_snap_remove): New struct. * src/qemu/qemu_domain.c (qemuDomainRemoveInactive) (qemuDomainSnapshotDiscardAllMetadata): New functions. (qemuFindQemuImgBinary, qemuDomainSnapshotWriteMetadata) (qemuDomainSnapshotForEachQcow2, qemuDomainSnapshotDiscard) (qemuDomainSnapshotDiscardAll): Move here... * src/qemu/qemu_driver.c (qemuFindQemuImgBinary) (qemuDomainSnapshotWriteMetadata, qemuDomainSnapshotForEachQcow2) (qemuDomainSnapshotDiscard, qemuDomainSnapshotDiscardAll): ...from here. (qemuDomainUndefineFlags): Update caller. * src/conf/domain_conf.c (virDomainRemoveInactive): Doc fixes. --- src/conf/domain_conf.c | 4 +- src/qemu/qemu_domain.c | 260 +++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 36 +++++++ src/qemu/qemu_driver.c | 249 +-------------------------------------------- src/qemu/qemu_process.c | 2 +- 5 files changed, 301 insertions(+), 250 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index eebcba0..7463d7c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1438,9 +1438,9 @@ virDomainObjGetPersistentDef(virCapsPtr caps, } /* - * The caller must hold a lock on the driver owning 'doms', + * The caller must hold a lock on the driver owning 'doms', * and must also have locked 'dom', to ensure no one else - * is either waiting for 'dom' or still usingn it + * is either waiting for 'dom' or still using it */ void virDomainRemoveInactive(virDomainObjListPtr doms, virDomainObjPtr dom) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d2cc2f0..9436245 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1296,3 +1296,263 @@ cleanup: VIR_FREE(message); return ret; } + +/* Locate an appropriate 'qemu-img' binary. */ +const char * +qemuFindQemuImgBinary(struct qemud_driver *driver) +{ + 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 driver->qemuImgBinary; +} + +int +qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, + virDomainSnapshotObjPtr snapshot, + char *snapshotDir) +{ + int fd = -1; + char *newxml = NULL; + int ret = -1; + char *snapDir = NULL; + char *snapFile = NULL; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + char *tmp; + + virUUIDFormat(vm->def->uuid, uuidstr); + newxml = virDomainSnapshotDefFormat(uuidstr, snapshot->def, + VIR_DOMAIN_XML_SECURE, 1); + if (newxml == NULL) { + virReportOOMError(); + return -1; + } + + if (virAsprintf(&snapDir, "%s/%s", snapshotDir, vm->def->name) < 0) { + virReportOOMError(); + goto cleanup; + } + if (virFileMakePath(snapDir) < 0) { + virReportSystemError(errno, _("cannot create snapshot directory '%s'"), + snapDir); + goto cleanup; + } + + if (virAsprintf(&snapFile, "%s/%s.xml", snapDir, snapshot->def->name) < 0) { + virReportOOMError(); + goto cleanup; + } + fd = open(snapFile, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR); + if (fd < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("failed to create snapshot file '%s'"), snapFile); + goto cleanup; + } + + if (virAsprintf(&tmp, "snapshot-edit %s", vm->def->name) < 0) { + virReportOOMError(); + goto cleanup; + } + virEmitXMLWarning(fd, snapshot->def->name, tmp); + VIR_FREE(tmp); + + if (safewrite(fd, newxml, strlen(newxml)) != strlen(newxml)) { + virReportSystemError(errno, _("Failed to write snapshot data to %s"), + snapFile); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(snapFile); + VIR_FREE(snapDir); + VIR_FREE(newxml); + VIR_FORCE_CLOSE(fd); + return ret; +} + +/* The domain is expected to be locked and inactive. Return -1 on normal + * failure, 1 if we skipped a disk due to try_all. */ +int +qemuDomainSnapshotForEachQcow2(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainSnapshotObjPtr snap, + const char *op, + bool try_all) +{ + const char *qemuimgarg[] = { NULL, "snapshot", NULL, NULL, NULL, NULL }; + int i; + bool skipped = false; + + qemuimgarg[0] = qemuFindQemuImgBinary(driver); + if (qemuimgarg[0] == NULL) { + /* qemuFindQemuImgBinary set the error */ + return -1; + } + + qemuimgarg[2] = op; + qemuimgarg[3] = snap->def->name; + + for (i = 0; i < vm->def->ndisks; i++) { + /* FIXME: we also need to handle LVM here */ + /* FIXME: if we fail halfway through this loop, we are in an + * inconsistent state. I'm not quite sure what to do about that + */ + if (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) { + if (!vm->def->disks[i]->driverType || + STRNEQ(vm->def->disks[i]->driverType, "qcow2")) { + if (try_all) { + /* Continue on even in the face of error, since other + * disks in this VM may have the same snapshot name. + */ + VIR_WARN("skipping snapshot action on %s", + vm->def->disks[i]->dst); + skipped = true; + continue; + } + qemuReportError(VIR_ERR_OPERATION_INVALID, + _("Disk device '%s' does not support" + " snapshotting"), + vm->def->disks[i]->dst); + return -1; + } + + qemuimgarg[4] = vm->def->disks[i]->src; + + if (virRun(qemuimgarg, NULL) < 0) { + if (try_all) { + VIR_WARN("skipping snapshot action on %s", + vm->def->disks[i]->info.alias); + skipped = true; + continue; + } + return -1; + } + } + } + + return skipped ? 1 : 0; +} + +/* Discard one snapshot (or its metadata), without reparenting any children. */ +int +qemuDomainSnapshotDiscard(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainSnapshotObjPtr snap, + bool update_current, + bool metadata_only) +{ + char *snapFile = NULL; + int ret = -1; + qemuDomainObjPrivatePtr priv; + virDomainSnapshotObjPtr parentsnap = NULL; + + if (!metadata_only) { + if (!virDomainObjIsActive(vm)) { + /* Ignore any skipped disks */ + if (qemuDomainSnapshotForEachQcow2(driver, vm, snap, "-d", + true) < 0) + goto cleanup; + } else { + priv = vm->privateData; + qemuDomainObjEnterMonitorWithDriver(driver, vm); + /* we continue on even in the face of error */ + qemuMonitorDeleteSnapshot(priv->mon, snap->def->name); + qemuDomainObjExitMonitorWithDriver(driver, vm); + } + } + + if (virAsprintf(&snapFile, "%s/%s/%s.xml", driver->snapshotDir, + vm->def->name, snap->def->name) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (snap == vm->current_snapshot) { + if (update_current && snap->def->parent) { + parentsnap = virDomainSnapshotFindByName(&vm->snapshots, + snap->def->parent); + if (!parentsnap) { + VIR_WARN("missing parent snapshot matching name '%s'", + snap->def->parent); + } else { + parentsnap->def->current = true; + if (qemuDomainSnapshotWriteMetadata(vm, parentsnap, + driver->snapshotDir) < 0) { + VIR_WARN("failed to set parent snapshot '%s' as current", + snap->def->parent); + parentsnap->def->current = false; + parentsnap = NULL; + } + } + } + vm->current_snapshot = parentsnap; + } + + if (unlink(snapFile) < 0) + VIR_WARN("Failed to unlink %s", snapFile); + virDomainSnapshotObjListRemove(&vm->snapshots, snap); + + ret = 0; + +cleanup: + VIR_FREE(snapFile); + + return ret; +} + +/* Hash iterator callback to discard multiple snapshots. */ +void qemuDomainSnapshotDiscardAll(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *data) +{ + virDomainSnapshotObjPtr snap = payload; + struct qemu_snap_remove *curr = data; + int err; + + if (snap->def->current) + curr->current = true; + err = qemuDomainSnapshotDiscard(curr->driver, curr->vm, snap, false, + curr->metadata_only); + if (err && !curr->err) + curr->err = err; +} + +int +qemuDomainSnapshotDiscardAllMetadata(struct qemud_driver *driver, + virDomainObjPtr vm) +{ + struct qemu_snap_remove rem; + + rem.driver = driver; + rem.vm = vm; + rem.metadata_only = true; + rem.err = 0; + virHashForEach(vm->snapshots.objs, qemuDomainSnapshotDiscardAll, &rem); + + /* XXX also do rmdir ? */ + return rem.err; +} + +/* + * The caller must hold a lock on both driver and vm, and there must + * be no remaining references to vm. + */ +void +qemuDomainRemoveInactive(struct qemud_driver *driver, + virDomainObjPtr vm) +{ + /* Remove any snapshot metadata prior to removing the domain */ + if (qemuDomainSnapshotDiscardAllMetadata(driver, vm) < 0) { + VIR_WARN("unable to remove all snapshots for domain %s", + vm->def->name); + } + virDomainRemoveInactive(&driver->domains, vm); +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index cde3ada..00cfa3a 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -237,4 +237,40 @@ int qemuDomainAppendLog(struct qemud_driver *driver, int logFD, const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(4, 5); +const char *qemuFindQemuImgBinary(struct qemud_driver *driver); + +int qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, + virDomainSnapshotObjPtr snapshot, + char *snapshotDir); + +int qemuDomainSnapshotForEachQcow2(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainSnapshotObjPtr snap, + const char *op, + bool try_all); + +int qemuDomainSnapshotDiscard(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainSnapshotObjPtr snap, + bool update_current, + bool metadata_only); + +struct qemu_snap_remove { + struct qemud_driver *driver; + virDomainObjPtr vm; + int err; + bool metadata_only; + bool current; +}; + +void qemuDomainSnapshotDiscardAll(void *payload, + const void *name, + void *data); + +int qemuDomainSnapshotDiscardAllMetadata(struct qemud_driver *driver, + virDomainObjPtr vm); + +void qemuDomainRemoveInactive(struct qemud_driver *driver, + virDomainObjPtr vm); + #endif /* __QEMU_DOMAIN_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4a24019..4ff3281 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1596,243 +1596,6 @@ cleanup: } -/* Locate an appropriate 'qemu-img' binary. */ -static const char * -qemuFindQemuImgBinary(struct qemud_driver *driver) -{ - 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 driver->qemuImgBinary; -} - -static int -qemuDomainSnapshotWriteMetadata(virDomainObjPtr vm, - virDomainSnapshotObjPtr snapshot, - char *snapshotDir) -{ - int fd = -1; - char *newxml = NULL; - int ret = -1; - char *snapDir = NULL; - char *snapFile = NULL; - char uuidstr[VIR_UUID_STRING_BUFLEN]; - char *tmp; - - virUUIDFormat(vm->def->uuid, uuidstr); - newxml = virDomainSnapshotDefFormat(uuidstr, snapshot->def, - VIR_DOMAIN_XML_SECURE, 1); - if (newxml == NULL) { - virReportOOMError(); - return -1; - } - - if (virAsprintf(&snapDir, "%s/%s", snapshotDir, vm->def->name) < 0) { - virReportOOMError(); - goto cleanup; - } - if (virFileMakePath(snapDir) < 0) { - virReportSystemError(errno, _("cannot create snapshot directory '%s'"), - snapDir); - goto cleanup; - } - - if (virAsprintf(&snapFile, "%s/%s.xml", snapDir, snapshot->def->name) < 0) { - virReportOOMError(); - goto cleanup; - } - fd = open(snapFile, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR); - if (fd < 0) { - qemuReportError(VIR_ERR_OPERATION_FAILED, - _("failed to create snapshot file '%s'"), snapFile); - goto cleanup; - } - - if (virAsprintf(&tmp, "snapshot-edit %s", vm->def->name) < 0) { - virReportOOMError(); - goto cleanup; - } - virEmitXMLWarning(fd, snapshot->def->name, tmp); - VIR_FREE(tmp); - - if (safewrite(fd, newxml, strlen(newxml)) != strlen(newxml)) { - virReportSystemError(errno, _("Failed to write snapshot data to %s"), - snapFile); - goto cleanup; - } - - ret = 0; - -cleanup: - VIR_FREE(snapFile); - VIR_FREE(snapDir); - VIR_FREE(newxml); - VIR_FORCE_CLOSE(fd); - return ret; -} - -/* The domain is expected to be locked and inactive. Return -1 on normal - * failure, 1 if we skipped a disk due to try_all. */ -static int -qemuDomainSnapshotForEachQcow2(struct qemud_driver *driver, - virDomainObjPtr vm, - virDomainSnapshotObjPtr snap, - const char *op, - bool try_all) -{ - const char *qemuimgarg[] = { NULL, "snapshot", NULL, NULL, NULL, NULL }; - int i; - bool skipped = false; - - qemuimgarg[0] = qemuFindQemuImgBinary(driver); - if (qemuimgarg[0] == NULL) { - /* qemuFindQemuImgBinary set the error */ - return -1; - } - - qemuimgarg[2] = op; - qemuimgarg[3] = snap->def->name; - - for (i = 0; i < vm->def->ndisks; i++) { - /* FIXME: we also need to handle LVM here */ - /* FIXME: if we fail halfway through this loop, we are in an - * inconsistent state. I'm not quite sure what to do about that - */ - if (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - if (!vm->def->disks[i]->driverType || - STRNEQ(vm->def->disks[i]->driverType, "qcow2")) { - if (try_all) { - /* Continue on even in the face of error, since other - * disks in this VM may have the same snapshot name. - */ - VIR_WARN("skipping snapshot action on %s", - vm->def->disks[i]->dst); - skipped = true; - continue; - } - qemuReportError(VIR_ERR_OPERATION_INVALID, - _("Disk device '%s' does not support" - " snapshotting"), - vm->def->disks[i]->dst); - return -1; - } - - qemuimgarg[4] = vm->def->disks[i]->src; - - if (virRun(qemuimgarg, NULL) < 0) { - if (try_all) { - VIR_WARN("skipping snapshot action on %s", - vm->def->disks[i]->info.alias); - skipped = true; - continue; - } - return -1; - } - } - } - - return skipped ? 1 : 0; -} - -/* Discard one snapshot (or its metadata), without reparenting any children. */ -static int -qemuDomainSnapshotDiscard(struct qemud_driver *driver, - virDomainObjPtr vm, - virDomainSnapshotObjPtr snap, - bool update_current, - bool metadata_only) -{ - char *snapFile = NULL; - int ret = -1; - qemuDomainObjPrivatePtr priv; - virDomainSnapshotObjPtr parentsnap = NULL; - - if (!metadata_only) { - if (!virDomainObjIsActive(vm)) { - /* Ignore any skipped disks */ - if (qemuDomainSnapshotForEachQcow2(driver, vm, snap, "-d", - true) < 0) - goto cleanup; - } else { - priv = vm->privateData; - qemuDomainObjEnterMonitorWithDriver(driver, vm); - /* we continue on even in the face of error */ - qemuMonitorDeleteSnapshot(priv->mon, snap->def->name); - qemuDomainObjExitMonitorWithDriver(driver, vm); - } - } - - if (virAsprintf(&snapFile, "%s/%s/%s.xml", driver->snapshotDir, - vm->def->name, snap->def->name) < 0) { - virReportOOMError(); - goto cleanup; - } - - if (snap == vm->current_snapshot) { - if (update_current && snap->def->parent) { - parentsnap = virDomainSnapshotFindByName(&vm->snapshots, - snap->def->parent); - if (!parentsnap) { - VIR_WARN("missing parent snapshot matching name '%s'", - snap->def->parent); - } else { - parentsnap->def->current = true; - if (qemuDomainSnapshotWriteMetadata(vm, parentsnap, - driver->snapshotDir) < 0) { - VIR_WARN("failed to set parent snapshot '%s' as current", - snap->def->parent); - parentsnap->def->current = false; - parentsnap = NULL; - } - } - } - vm->current_snapshot = parentsnap; - } - - if (unlink(snapFile) < 0) - VIR_WARN("Failed to unlink %s", snapFile); - virDomainSnapshotObjListRemove(&vm->snapshots, snap); - - ret = 0; - -cleanup: - VIR_FREE(snapFile); - - return ret; -} - -struct snap_remove { - struct qemud_driver *driver; - virDomainObjPtr vm; - int err; - bool metadata_only; - bool current; -}; - -/* Hash iterator callback to discard multiple snapshots. */ -static void -qemuDomainSnapshotDiscardAll(void *payload, - const void *name ATTRIBUTE_UNUSED, - void *data) -{ - virDomainSnapshotObjPtr snap = payload; - struct snap_remove *curr = data; - int err; - - if (snap->def->current) - curr->current = true; - err = qemuDomainSnapshotDiscard(curr->driver, curr->vm, snap, false, - curr->metadata_only); - if (err && !curr->err) - curr->err = err; -} - /* Count how many snapshots in a set have external disk snapshots. */ static void qemuDomainSnapshotCountExternal(void *payload, @@ -5126,8 +4889,6 @@ qemuDomainUndefineFlags(virDomainPtr dom, if (!virDomainObjIsActive(vm) && (nsnapshots = virDomainSnapshotObjListNum(&vm->snapshots, 0))) { - struct snap_remove rem; - if (!(flags & VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA)) { qemuReportError(VIR_ERR_OPERATION_INVALID, _("cannot delete inactive domain with %d " @@ -5135,13 +4896,7 @@ qemuDomainUndefineFlags(virDomainPtr dom, nsnapshots); goto cleanup; } - - rem.driver = driver; - rem.vm = vm; - rem.metadata_only = true; - rem.err = 0; - virHashForEach(vm->snapshots.objs, qemuDomainSnapshotDiscardAll, &rem); - if (rem.err < 0) + if (qemuDomainSnapshotDiscardAllMetadata(driver, vm) < 0) goto cleanup; } @@ -10132,7 +9887,7 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, int ret = -1; virDomainSnapshotObjPtr snap = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; - struct snap_remove rem; + struct qemu_snap_remove rem; struct snap_reparent rep; bool metadata_only = !!(flags & VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY); int external = 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index bd49b21..8797a56 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -106,7 +106,7 @@ qemuProcessRemoveDomainStatus(struct qemud_driver *driver, extern struct qemud_driver *qemu_driver; /* - * This is a callback registered with a qemuMonitorPtr instance, + * This is a callback registered with a qemuMonitorPtr instance, * and to be invoked when the monitor console hits an end of file * condition, or error, thus indicating VM shutdown should be * performed -- 1.7.4.4

The previous patch removed all snapshots, but not the directory where the snapshots lived, which is still a form of stale data. * src/qemu/qemu_domain.c (qemuDomainRemoveInactive): Wipe any snapshot directory. --- I could probably also just squash this into 2/3. src/qemu/qemu_domain.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9436245..4023648 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1537,7 +1537,6 @@ qemuDomainSnapshotDiscardAllMetadata(struct qemud_driver *driver, rem.err = 0; virHashForEach(vm->snapshots.objs, qemuDomainSnapshotDiscardAll, &rem); - /* XXX also do rmdir ? */ return rem.err; } @@ -1549,10 +1548,21 @@ void qemuDomainRemoveInactive(struct qemud_driver *driver, virDomainObjPtr vm) { + char *snapDir; + /* Remove any snapshot metadata prior to removing the domain */ if (qemuDomainSnapshotDiscardAllMetadata(driver, vm) < 0) { VIR_WARN("unable to remove all snapshots for domain %s", vm->def->name); } + else if (virAsprintf(&snapDir, "%s/%s", driver->snapshotDir, + vm->def->name) < 0) { + VIR_WARN("unable to remove snapshot directory %s/%s", + driver->snapshotDir, vm->def->name); + } else { + if (rmdir(snapDir) < 0 && errno != ENOENT) + VIR_WARN("unable to remove snapshot directory %s", snapDir); + VIR_FREE(snapDir); + } virDomainRemoveInactive(&driver->domains, vm); } -- 1.7.4.4