[libvirt] [PATCH] Drive hot-unplug: reliable parsing of HMP results
Hot-unplugging a disk from a guest that supports hot-unplugging generates an error in the libvirt log when running QEMU with the "-msg timestamp=on" flag. 2015-08-06 10:48:59.945+0000: 11662: error : qemuMonitorTextDriveDel:2594 : operation failed: deleting drive-virtio-disk4 drive failed: 2015-08-06T10:48:59.945058Z Device 'drive-virtio-disk4' not found This error is caused because the HMP results are getting prefixed with a timestamp. Parsing the output is not reliable with STRPREFIX as the results can be prefixed with a timestamp. Using strstr ensures that parsing the output works whether the results are prefixed or not. Cc: Stefan Hajnoczi <stefanha@redhat.com> Cc: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Frank Schreuder <fschreuder@transip.nl> --- src/qemu/qemu_monitor_text.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 2aa0460..d5ef089 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2586,7 +2586,7 @@ int qemuMonitorTextDriveDel(qemuMonitorPtr mon, /* (qemu) drive_del wark * Device 'wark' not found */ - } else if (STRPREFIX(reply, "Device '") && (strstr(reply, "not found"))) { + } else if (strstr(reply, "Device '") && strstr(reply, "not found")) { /* NB: device not found errors mean the drive was auto-deleted and we * ignore the error */ } else if (STRNEQ(reply, "")) { -- 2.4.5
On Mon, Aug 10, 2015 at 4:37 PM, Frank Schreuder <fschreuder@transip.nl> wrote:
Hot-unplugging a disk from a guest that supports hot-unplugging generates an error in the libvirt log when running QEMU with the "-msg timestamp=on" flag.
2015-08-06 10:48:59.945+0000: 11662: error : qemuMonitorTextDriveDel:2594 : operation failed: deleting drive-virtio-disk4 drive failed: 2015-08-06T10:48:59.945058Z Device 'drive-virtio-disk4' not found
This error is caused because the HMP results are getting prefixed with a timestamp. Parsing the output is not reliable with STRPREFIX as the results can be prefixed with a timestamp.
Using strstr ensures that parsing the output works whether the results are prefixed or not.
Cc: Stefan Hajnoczi <stefanha@redhat.com> Cc: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Frank Schreuder <fschreuder@transip.nl> ---
src/qemu/qemu_monitor_text.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 2aa0460..d5ef089 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2586,7 +2586,7 @@ int qemuMonitorTextDriveDel(qemuMonitorPtr mon,
/* (qemu) drive_del wark * Device 'wark' not found */ - } else if (STRPREFIX(reply, "Device '") && (strstr(reply, "not found"))) { + } else if (strstr(reply, "Device '") && strstr(reply, "not found")) { /* NB: device not found errors mean the drive was auto-deleted and we * ignore the error */ } else if (STRNEQ(reply, "")) {
I'm not very familiar with the libvirt codebase, but perhaps the timestamps on error messages should be stripped in the QEMU monitor processIO handler functions? That way any remaining bugs like this will be resolved too. Stefan
On Mon, Aug 10, 2015 at 04:49:03PM +0100, Stefan Hajnoczi wrote:
On Mon, Aug 10, 2015 at 4:37 PM, Frank Schreuder <fschreuder@transip.nl> wrote:
Hot-unplugging a disk from a guest that supports hot-unplugging generates an error in the libvirt log when running QEMU with the "-msg timestamp=on" flag.
2015-08-06 10:48:59.945+0000: 11662: error : qemuMonitorTextDriveDel:2594 : operation failed: deleting drive-virtio-disk4 drive failed: 2015-08-06T10:48:59.945058Z Device 'drive-virtio-disk4' not found
This error is caused because the HMP results are getting prefixed with a timestamp. Parsing the output is not reliable with STRPREFIX as the results can be prefixed with a timestamp.
Using strstr ensures that parsing the output works whether the results are prefixed or not.
Cc: Stefan Hajnoczi <stefanha@redhat.com> Cc: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Frank Schreuder <fschreuder@transip.nl> ---
src/qemu/qemu_monitor_text.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 2aa0460..d5ef089 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2586,7 +2586,7 @@ int qemuMonitorTextDriveDel(qemuMonitorPtr mon,
/* (qemu) drive_del wark * Device 'wark' not found */ - } else if (STRPREFIX(reply, "Device '") && (strstr(reply, "not found"))) { + } else if (strstr(reply, "Device '") && strstr(reply, "not found")) { /* NB: device not found errors mean the drive was auto-deleted and we * ignore the error */ } else if (STRNEQ(reply, "")) {
I'm not very familiar with the libvirt codebase, but perhaps the timestamps on error messages should be stripped in the QEMU monitor processIO handler functions?
That way any remaining bugs like this will be resolved too.
That would be right froma strict correctness POV, but looking at the existing monitor handling code it all uses strstr too, so this is matching existing practice. The text monitor code is scary enough that it is probably wise not to try changing it at this stage in its life. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Mon, Aug 10, 2015 at 05:01:22PM +0100, Daniel P. Berrange wrote:
On Mon, Aug 10, 2015 at 04:49:03PM +0100, Stefan Hajnoczi wrote:
On Mon, Aug 10, 2015 at 4:37 PM, Frank Schreuder <fschreuder@transip.nl> wrote:
Hot-unplugging a disk from a guest that supports hot-unplugging generates an error in the libvirt log when running QEMU with the "-msg timestamp=on" flag.
2015-08-06 10:48:59.945+0000: 11662: error : qemuMonitorTextDriveDel:2594 : operation failed: deleting drive-virtio-disk4 drive failed: 2015-08-06T10:48:59.945058Z Device 'drive-virtio-disk4' not found
This error is caused because the HMP results are getting prefixed with a timestamp. Parsing the output is not reliable with STRPREFIX as the results can be prefixed with a timestamp.
Using strstr ensures that parsing the output works whether the results are prefixed or not.
Cc: Stefan Hajnoczi <stefanha@redhat.com> Cc: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Frank Schreuder <fschreuder@transip.nl> ---
src/qemu/qemu_monitor_text.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 2aa0460..d5ef089 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2586,7 +2586,7 @@ int qemuMonitorTextDriveDel(qemuMonitorPtr mon,
/* (qemu) drive_del wark * Device 'wark' not found */ - } else if (STRPREFIX(reply, "Device '") && (strstr(reply, "not found"))) { + } else if (strstr(reply, "Device '") && strstr(reply, "not found")) { /* NB: device not found errors mean the drive was auto-deleted and we * ignore the error */ } else if (STRNEQ(reply, "")) {
I'm not very familiar with the libvirt codebase, but perhaps the timestamps on error messages should be stripped in the QEMU monitor processIO handler functions?
That way any remaining bugs like this will be resolved too.
That would be right froma strict correctness POV, but looking at the existing monitor handling code it all uses strstr too, so this is matching existing practice. The text monitor code is scary enough that it is probably wise not to try changing it at this stage in its life.
Fair enough. Stefan
On Mon, Aug 10, 2015 at 05:37:30PM +0200, Frank Schreuder wrote:
Hot-unplugging a disk from a guest that supports hot-unplugging generates an error in the libvirt log when running QEMU with the "-msg timestamp=on" flag.
2015-08-06 10:48:59.945+0000: 11662: error : qemuMonitorTextDriveDel:2594 : operation failed: deleting drive-virtio-disk4 drive failed: 2015-08-06T10:48:59.945058Z Device 'drive-virtio-disk4' not found
This error is caused because the HMP results are getting prefixed with a timestamp. Parsing the output is not reliable with STRPREFIX as the results can be prefixed with a timestamp.
Using strstr ensures that parsing the output works whether the results are prefixed or not.
Cc: Stefan Hajnoczi <stefanha@redhat.com> Cc: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Frank Schreuder <fschreuder@transip.nl> ---
src/qemu/qemu_monitor_text.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On Mon, Aug 10, 2015 at 05:37:30PM +0200, Frank Schreuder wrote:
Hot-unplugging a disk from a guest that supports hot-unplugging generates an error in the libvirt log when running QEMU with the "-msg timestamp=on" flag.
2015-08-06 10:48:59.945+0000: 11662: error : qemuMonitorTextDriveDel:2594 : operation failed: deleting drive-virtio-disk4 drive failed: 2015-08-06T10:48:59.945058Z Device 'drive-virtio-disk4' not found
This error is caused because the HMP results are getting prefixed with a timestamp. Parsing the output is not reliable with STRPREFIX as the results can be prefixed with a timestamp.
Using strstr ensures that parsing the output works whether the results are prefixed or not.
Cc: Stefan Hajnoczi <stefanha@redhat.com> Cc: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Frank Schreuder <fschreuder@transip.nl> ---
src/qemu/qemu_monitor_text.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK and pushed. Jan
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 2aa0460..d5ef089 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2586,7 +2586,7 @@ int qemuMonitorTextDriveDel(qemuMonitorPtr mon,
/* (qemu) drive_del wark * Device 'wark' not found */ - } else if (STRPREFIX(reply, "Device '") && (strstr(reply, "not found"))) { + } else if (strstr(reply, "Device '") && strstr(reply, "not found")) { /* NB: device not found errors mean the drive was auto-deleted and we * ignore the error */ } else if (STRNEQ(reply, "")) { -- 2.4.5
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (5)
-
Daniel P. Berrange -
Frank Schreuder -
Ján Tomko -
Stefan Hajnoczi -
Stefan Hajnoczi