On Tue, Oct 18, 2011 at 04:04:57PM -0600, Eric Blake wrote: > Noticed when testing new libvirt against old qemu that lacked the > snapshot_blkdev HMP command. Libvirt was mistakenly treating the > command as successful, and re-writing the domain XML to use the > just-created 0-byte file, rendering the domain broken on restart. > > * src/qemu/qemu_monitor_text.c (qemuMonitorTextDiskSnapshot): > Notice another possible error message. > * src/qemu/qemu_driver.c > (qemuDomainSnapshotCreateSingleDiskActive): Don't keep 0-byte file > on failure. > --- > > v2: clean up on failure > > src/qemu/qemu_driver.c | 2 +- > src/qemu/qemu_monitor_text.c | 3 ++- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index f833655..84ef4dd 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -9027,7 +9027,6 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver, > VIR_WARN("Unable to release lock on %s", source); > goto cleanup; > } > - need_unlink = false; > > disk->src = origsrc; > origsrc = NULL; > @@ -9041,6 +9040,7 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver *driver, > goto cleanup; > > /* Update vm in place to match changes. */ > + need_unlink = false; > VIR_FREE(disk->src); > disk->src = source; > source = NULL; > diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c > index 2f31d99..4774df9 100644 > --- a/src/qemu/qemu_monitor_text.c > +++ b/src/qemu/qemu_monitor_text.c > @@ -3064,7 +3064,8 @@ qemuMonitorTextDiskSnapshot(qemuMonitorPtr mon, const char *device, > goto cleanup; > } > > - if (strstr(reply, "error while creating qcow2") != NULL) { > + if (strstr(reply, "error while creating qcow2") != NULL || > + strstr(reply, "unknown command:") != NULL) { > qemuReportError(VIR_ERR_OPERATION_FAILED, > _("Failed to take snapshot: %s"), reply); > goto cleanup; > -- > 1.7.4.4 > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list ACK, snapshot still works correctly when qemu does support it, and after review with Eric, I think the code looks good. Dave -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list