Re: [libvirt] [PATCH] qemu: Don't fail qemuProcessAttach for IOThreads if no JSON

On 09/18/2014 04:39 AM, Daniel P. Berrange wrote: > On Wed, Sep 17, 2014 at 03:07:47PM -0400, John Ferlan wrote: >> While doing some investigation for another bug I found that I could >> not qemu-attach to the process and got the following: >> >> error: Operation not supported: JSON monitor is required >> >> while running thru qemuProcessAttach. Since we can only get the data >> using the JSON parser and if the guest to be attached to doesn't have >> it we shouldn't just fail. See example in virsh qemu-attach for sample >> command that failed. > > It isn't simply qemu-attach that's affected. If you merely try to > start a guest normally with a QEMU that predates JSON support this > would fail too. > >> Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; >> --- >> >> I also considered removing the call from qemuProcessAttach rather than >> this approach. >> >> src/qemu/qemu_monitor.c | 8 +++----- >> 1 file changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c >> index 8927dbb..4342088 100644 >> --- a/src/qemu/qemu_monitor.c >> +++ b/src/qemu/qemu_monitor.c >> @@ -4112,11 +4112,9 @@ qemuMonitorGetIOThreads(qemuMonitorPtr mon, >> return -1; >> } >> >> - if (!mon->json) { >> - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", >> - _("JSON monitor is required")); >> - return -1; >> - } >> + /* Requires JSON to make the query */ >> + if (!mon->json) >> + return 0; > > I think you need should do '*iothreads = NULL' for safety too. > OK with the following squashed in? diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 4342088..10f51c5 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4113,8 +4113,10 @@ qemuMonitorGetIOThreads(qemuMonitorPtr mon, } /* Requires JSON to make the query */ - if (!mon->json) + if (!mon->json) { + *iothreads = NULL; return 0; + } return qemuMonitorJSONGetIOThreads(mon, iothreads); }