Re: [PATCH] Drive hot-unplug: reliable parsing of HMP results

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Aug 10, 2015 at 4:37 PM, Frank Schreuder <fschreuder@xxxxxxxxxx> 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@xxxxxxxxxx>
> Cc: Daniel P. Berrange <berrange@xxxxxxxxxx>
> Signed-off-by: Frank Schreuder <fschreuder@xxxxxxxxxx>
> ---
>
>  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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]