Re: [libvirt] [PATCH] qemu: Don't fail qemuProcessAttach for IOThreads if no JSON
On Thu, Sep 18, 2014 at 06:18:22AM -0400, John Ferlan wrote:
>
>
> 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
> >> ---
> >>
> >> 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);
> }
ACK
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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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
>> ---
>>
>> 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);
}
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Don't fail qemuProcessAttach for IOThreads if no JSON
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
> ---
>
> 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.
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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Don't fail qemuProcessAttach for IOThreads if no JSON
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.
Signed-off-by: John Ferlan
---
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;
return qemuMonitorJSONGetIOThreads(mon, iothreads);
}
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
