Trivially no migration parameters are supported when
query-migrate-parameters QMP command is missing. There's no need to
report an error in such case especially when doing so breaks
compatibility with old QEMU.

https://bugzilla.redhat.com/show_bug.cgi?id=1441934

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/qemu/qemu_monitor_json.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 98e3c53f5..083729003 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2660,6 +2660,11 @@ qemuMonitorJSONGetMigrationParams(qemuMonitorPtr mon,
     if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
         goto cleanup;
 
+    if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
+        ret = 0;
+        goto cleanup;
+    }
+
     if (qemuMonitorJSONCheckError(cmd, reply) < 0)
         goto cleanup;
 
-- 
2.12.2

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

On Wed, Apr 26, 2017 at 11:25:46PM +0200, Jiri Denemark wrote:
> Trivially no migration parameters are supported when
> query-migrate-parameters QMP command is missing. There's no need to
> report an error in such case especially when doing so breaks
> compatibility with old QEMU.

I'd suggest adjusting the commit message a bit, since the important part is
that there's a regression, the migration should work regardless of the presence
of the QMP command, and without your patch, it doesn't.

ACK

Erik

>
> https://bugzilla.redhat.com/show_bug.cgi?id=1441934
>
> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> ---
>  src/qemu/qemu_monitor_json.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 98e3c53f5..083729003 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -2660,6 +2660,11 @@ qemuMonitorJSONGetMigrationParams(qemuMonitorPtr mon,
>      if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
>          goto cleanup;
>
> +    if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
> +        ret = 0;
> +        goto cleanup;
> +    }
> +
>      if (qemuMonitorJSONCheckError(cmd, reply) < 0)
>          goto cleanup;
>
> --
> 2.12.2
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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

On Thu, Apr 27, 2017 at 08:49:30 +0200, Erik Skultety wrote:
> On Wed, Apr 26, 2017 at 11:25:46PM +0200, Jiri Denemark wrote:
> > Trivially no migration parameters are supported when
> > query-migrate-parameters QMP command is missing. There's no need to
> > report an error in such case especially when doing so breaks
> > compatibility with old QEMU.
> 
> I'd suggest adjusting the commit message a bit, since the important part is
> that there's a regression, the migration should work regardless of the presence
> of the QMP command, and without your patch, it doesn't.

Migration with old QEMU which does not support query-migrate-parameters
would fail because the QMP command is called unconditionally since the
introduction of TLS migration. Previously it was only called if the user
explicitly requested a feature which uses QEMU migration parameters. And
even then the situation was not ideal, instead of reporting an
unsupported feature we'd just complain about missing QMP command.

Trivially no migration parameters are supported when
query-migrate-parameters QMP command is missing. There's no need to
report an error if it is missing, the callers will report better error
if needed.

Is this better?

Jirka

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

On Thu, Apr 27, 2017 at 09:22:14AM +0200, Jiri Denemark wrote:
> On Thu, Apr 27, 2017 at 08:49:30 +0200, Erik Skultety wrote:
> > On Wed, Apr 26, 2017 at 11:25:46PM +0200, Jiri Denemark wrote:
> > > Trivially no migration parameters are supported when
> > > query-migrate-parameters QMP command is missing. There's no need to
> > > report an error in such case especially when doing so breaks
> > > compatibility with old QEMU.
> >
> > I'd suggest adjusting the commit message a bit, since the important part is
> > that there's a regression, the migration should work regardless of the presence
> > of the QMP command, and without your patch, it doesn't.
>
> Migration with old QEMU which does not support query-migrate-parameters
> would fail because the QMP command is called unconditionally since the
> introduction of TLS migration. Previously it was only called if the user
> explicitly requested a feature which uses QEMU migration parameters. And
> even then the situation was not ideal, instead of reporting an
> unsupported feature we'd just complain about missing QMP command.
>
> Trivially no migration parameters are supported when
> query-migrate-parameters QMP command is missing. There's no need to
> report an error if it is missing, the callers will report better error
> if needed.
>
> Is this better?

Very much, yep.

Erik

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

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