[libvirt] [PATCH] qemu: handle not supported writable devices

If the running VM contains a writable raw image, creating snapshot fails internally in QEMU, but the error is not detected by libvirt. Success is still reported to the user, who will see the snapshot in libvirt, even they are NOT created by qemu. virsh # qemu-monitor-command --hmp winxp-1 savevm \"test\" Device 'drive-fdc0-0-0' is writable but does not support snapshots. virsh # snapshot-create-as winxp-1 test Domain snapshot test created Since there is no QMP command in QEMU, libvirtd sends a HMP command to the running QEMU and parses the returned text. There only the following 4 strings are detected as errors: src/qemu/qemu_monitor_text.c:2822 # qemuMonitorTextCreateSnapshot()
"Error while creating snapshot" "No block device can accept snapshots" "Could not open VM state file" "Error" + "while writing VM"
Since none of them match the above message, libvirt thinks the command succeeded. Add "does not support snapshots" as an additional error condition. Signed-off-by: Philipp Hahn <hahn@univention.de> --- src/qemu/qemu_monitor_text.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index bc0a11d..5880ab9 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2854,6 +2854,10 @@ int qemuMonitorTextCreateSnapshot(qemuMonitorPtr mon, const char *name) virReportError(VIR_ERR_OPERATION_FAILED, "%s", reply); goto cleanup; } + else if (strstr(reply, "does not support snapshots") != NULL) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", reply); + goto cleanup; + } ret = 0; -- 1.7.10.4

On 02/15/2013 07:13 AM, Philipp Hahn wrote:
If the running VM contains a writable raw image, creating snapshot fails internally in QEMU, but the error is not detected by libvirt. Success is still reported to the user, who will see the snapshot in libvirt, even they are NOT created by qemu.
Libvirt is supposed to already have code that refuses an internal snapshot on non-qcow2 files. We must have broken this somewhere in the recent past. It would be nice to figure out why qemuDomainSnapshotPrepare() is failing to do a proper job before we take this patch. :(
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index bc0a11d..5880ab9 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2854,6 +2854,10 @@ int qemuMonitorTextCreateSnapshot(qemuMonitorPtr mon, const char *name) virReportError(VIR_ERR_OPERATION_FAILED, "%s", reply); goto cleanup; } + else if (strstr(reply, "does not support snapshots") != NULL) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", reply); + goto cleanup; + }
At any rate, this patch makes sense, once we _also_ solve the root cause of why we are getting here in the first place. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Hello Eric, Am Freitag 15 Februar 2013, 17:26:30 schrieb Eric Blake:
On 02/15/2013 07:13 AM, Philipp Hahn wrote:
If the running VM contains a writable raw image, creating snapshot fails internally in QEMU, but the error is not detected by libvirt. Success is still reported to the user, who will see the snapshot in libvirt, even they are NOT created by qemu.
Libvirt is supposed to already have code that refuses an internal snapshot on non-qcow2 files. We must have broken this somewhere in the recent past. It would be nice to figure out why qemuDomainSnapshotPrepare() is failing to do a proper job before we take this patch. :(
Okay, that might be my problem: I'm working on the old Debian version 0.9.12-5, where that code might not yet be available. I patched it there and forward-ported that patch to current git, because there the check was also missing. (Perhaps even better would be to return an error by default and only return rv=0, when _no_ output is returned by the command) Sincerely Philipp -- Philipp Hahn Open Source Software Engineer hahn@univention.de Univention GmbH be open. fon: +49 421 22 232- 0 Mary-Somerville-Str.1 D-28359 Bremen fax: +49 421 22 232-99 http://www.univention.de/
participants (2)
-
Eric Blake
-
Philipp Hahn