On 10/04/2011 07:39 PM, Eric Blake wrote:
Once we know which set of disks belong to a snapshot, reverting or
deleting that snapshot should visit just those disks, rather than
also visiting disks that were hot-plugged in the meantime or
skipping disks that were hot-unplugged in the meantime.
* src/qemu/qemu_domain.c (qemuDomainSnapshotForEachQcow2): Use
snapshot domain details when available. Avoid NULL deref.
---
Detected while actually testing patch 2/2 in the series. This
fixes the issue, but is worth a separate review before I push
the series.
src/qemu/qemu_domain.c | 24 ++++++++++++++++--------
1 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 65f721a..85bebd6 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1402,6 +1402,14 @@ qemuDomainSnapshotForEachQcow2(struct qemud_driver *driver,
const char *qemuimgarg[] = { NULL, "snapshot", NULL, NULL, NULL, NULL };
int i;
bool skipped = false;
+ virDomainDefPtr def;
+
+ /* Prefer action on the disks in use at the time the snapshot was
+ * created; but fall back to current definition if dealing with a
+ * snapshot created prior to libvirt 0.9.5. */
+ def = snap->def->dom;
+ if (!def)
+ def = vm->def;
qemuimgarg[0] = qemuFindQemuImgBinary(driver);
if (qemuimgarg[0] == NULL) {
@@ -1412,36 +1420,36 @@ qemuDomainSnapshotForEachQcow2(struct qemud_driver *driver,
qemuimgarg[2] = op;
qemuimgarg[3] = snap->def->name;
- for (i = 0; i< vm->def->ndisks; i++) {
+ for (i = 0; i< 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 (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
+ if (!def->disks[i]->driverType ||
+ STRNEQ(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);
+ def->disks[i]->dst);
skipped = true;
continue;
}
qemuReportError(VIR_ERR_OPERATION_INVALID,
_("Disk device '%s' does not support"
" snapshotting"),
- vm->def->disks[i]->dst);
+ def->disks[i]->dst);
return -1;
}
- qemuimgarg[4] = vm->def->disks[i]->src;
+ qemuimgarg[4] = 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);
+ def->disks[i]->dst);
skipped = true;
continue;
}
ACK.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list