docs/interop/qmp-spec.txt | 5 ++- include/monitor/monitor.h | 3 +- monitor.c | 82 ++++++++++++++++----------------------- qapi/misc.json | 40 ------------------- tests/libqtest.c | 10 ++--- tests/libqtest.h | 4 +- tests/qmp-test.c | 32 +++++++++++++-- tests/test-qmp-cmds.c | 16 ++++++++ vl.c | 5 --- 9 files changed, 89 insertions(+), 108 deletions(-)
Based-on: <20180828191048.29806-1-armbru@redhat.com> Based-on: <20180901111716.1675-1-armbru@redhat.com> (this series is based on Markus's monitor-next tree) v9: - add r-bs - release the qmp queue lock before resume [Marc-Andre] v8: - remove patch 1 & 2 since already in the QAPI pull - squash patch 3 & 4, use Markus's version of commit message (with some of my additions), make sure popping and reading queue length is in the same critical section [Markus] - add one patch to cover test for queue full [Markus] - add one patch to resume the monitor earlier when queue not full [Markus] v7: - use Markus's commit message for patch "qapi: Drop qapi_event_send_FOO()'s Error ** argument" [Markus] - update commit message for "qapi: remove COMMAND_DROPPED event" since QEMU 3.0 is released [Eric/Dave] - rebase to Markus's monitor-next tree: http://repo.or.cz/qemu/armbru.git/shortlog/refs/heads/monitor-next the patch "monitor: suspend monitor instead of send CMD_DROP" re-written since people prefer to drop need_resume flag so now I hand-made it. Dropped a few patches since not appliable any more. Please review. Thanks, Peter Xu (6): monitor: Suspend monitor instead dropping commands monitor: resume the monitor earlier if needed monitor: remove "x-oob", turn oob on by default Revert "tests: Add parameter to qtest_init_without_qmp_handshake" tests: add oob functional test for test-qmp-cmds tests: qmp-test: add queue full test docs/interop/qmp-spec.txt | 5 ++- include/monitor/monitor.h | 3 +- monitor.c | 82 ++++++++++++++++----------------------- qapi/misc.json | 40 ------------------- tests/libqtest.c | 10 ++--- tests/libqtest.h | 4 +- tests/qmp-test.c | 32 +++++++++++++-- tests/test-qmp-cmds.c | 16 ++++++++ vl.c | 5 --- 9 files changed, 89 insertions(+), 108 deletions(-) -- 2.17.1
On 10/9/18 1:27 AM, Peter Xu wrote:
> Based-on: <20180828191048.29806-1-armbru@redhat.com>
> Based-on: <20180901111716.1675-1-armbru@redhat.com>
>
> (this series is based on Markus's monitor-next tree)
>
> v9:
> - add r-bs
> - release the qmp queue lock before resume [Marc-Andre]
I haven't reviewed closely, but did want to report that I tested that
with your patches applied, there is no way to trigger OOB of the initial
capability handshake (good). It's a bit odd that the initial error
(input member unexpected) is different from the later error (does not
support OOB), but not a show-stopper, so I don't think you need to worry
about it:
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 0, "major": 3},
"package": "v3.0.0-1150-g7d932cd3d53"}, "capabilities": ["oob"]}}
{"exec-oob":"qmp_capabilities","arguments":{"enable":["oob"]}}
{"error": {"class": "GenericError", "desc": "QMP input member 'exec-oob'
is unexpected"}}
{"execute":"qmp_capabilities","arguments":{"enable":["oob"]}}
{"return": {}}
{"exec-oob":"qmp_capabilities"}
{"error": {"class": "GenericError", "desc": "The command
qmp_capabilities does not support OOB"}}
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
On 10/10/18 11:26 AM, Eric Blake wrote:
> On 10/9/18 1:27 AM, Peter Xu wrote:
>> Based-on: <20180828191048.29806-1-armbru@redhat.com>
>> Based-on: <20180901111716.1675-1-armbru@redhat.com>
>>
>> (this series is based on Markus's monitor-next tree)
>>
>> v9:
>> - add r-bs
>> - release the qmp queue lock before resume [Marc-Andre]
>
> I haven't reviewed closely, but did want to report that I tested that
> with your patches applied, there is no way to trigger OOB of the initial
> capability handshake (good). It's a bit odd that the initial error
> (input member unexpected) is different from the later error (does not
> support OOB), but not a show-stopper, so I don't think you need to worry
> about it:
>
> {"QMP": {"version": {"qemu": {"micro": 50, "minor": 0, "major": 3},
> "package": "v3.0.0-1150-g7d932cd3d53"}, "capabilities": ["oob"]}}
> {"exec-oob":"qmp_capabilities","arguments":{"enable":["oob"]}}
> {"error": {"class": "GenericError", "desc": "QMP input member 'exec-oob'
> is unexpected"}}
> {"execute":"qmp_capabilities","arguments":{"enable":["oob"]}}
> {"return": {}}
> {"exec-oob":"qmp_capabilities"}
> {"error": {"class": "GenericError", "desc": "The command
> qmp_capabilities does not support OOB"}}
>
On the other hand, when I'm trying to use a qemu binary with these
patches applied, libvirt is hanging when trying to probe the
capabilities of the binary, waiting for a response to
"qmp_capabilities". I'll try and bisect which patch is causing the
problem, and figure out why it is happening for libvirt and not running
by hand (perhaps is it a tty vs. Unix socket thing?)
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
On 10/10/18 2:26 PM, Eric Blake wrote:
>
> On the other hand, when I'm trying to use a qemu binary with these
> patches applied, libvirt is hanging when trying to probe the
> capabilities of the binary, waiting for a response to
> "qmp_capabilities". I'll try and bisect which patch is causing the
> problem, and figure out why it is happening for libvirt and not running
> by hand (perhaps is it a tty vs. Unix socket thing?)
Bisect didn't help much; it landed on:
monitor: remove "x-oob", turn oob on by default
as the cause of libvirt hanging. I didn't have time to investigate
further, other than the command line that is hanging:
/home/eblake/qemu/x86_64-softmmu/qemu-system-x86_64 -S -no-user-config
-nodefaults -nographic -machine none,accel=kvm:tcg -qmp
unix:/var/lib/libvirt/qemu/capabilities.monitor.sock,server,nowait
-pidfile /var/lib/libvirt/qemu/capabilities.pidfile -daemonize
And I suspect it is the -daemonize that is causing the hang I'm seeing
when run by libvirt.
> Based-on: <20180828191048.29806-1-armbru@redhat.com>
> Based-on: <20180901111716.1675-1-armbru@redhat.com>
>
> (this series is based on Markus's monitor-next tree)
Am I missing any prerequisite patches? Markus' monitor-next tree is
currently a subset of git master (merge efd1d522).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
On Wed, Oct 10, 2018 at 03:27:34PM -0500, Eric Blake wrote: > On 10/10/18 2:26 PM, Eric Blake wrote: > > > > > On the other hand, when I'm trying to use a qemu binary with these > > patches applied, libvirt is hanging when trying to probe the > > capabilities of the binary, waiting for a response to > > "qmp_capabilities". I'll try and bisect which patch is causing the > > problem, and figure out why it is happening for libvirt and not running > > by hand (perhaps is it a tty vs. Unix socket thing?) > > Bisect didn't help much; it landed on: > > monitor: remove "x-oob", turn oob on by default > > as the cause of libvirt hanging. I didn't have time to investigate further, > other than the command line that is hanging: > > /home/eblake/qemu/x86_64-softmmu/qemu-system-x86_64 -S -no-user-config > -nodefaults -nographic -machine none,accel=kvm:tcg -qmp > unix:/var/lib/libvirt/qemu/capabilities.monitor.sock,server,nowait -pidfile > /var/lib/libvirt/qemu/capabilities.pidfile -daemonize > > And I suspect it is the -daemonize that is causing the hang I'm seeing when > run by libvirt. > > > Based-on: <20180828191048.29806-1-armbru@redhat.com> > > Based-on: <20180901111716.1675-1-armbru@redhat.com> > > > > (this series is based on Markus's monitor-next tree) > > > Am I missing any prerequisite patches? Markus' monitor-next tree is > currently a subset of git master (merge efd1d522). Sorry for the confusion. I should have removed these lines from the old cover letter. It's very possible the daemonize thing, actually Wolfgang Bumiller has posted patches to fix this up (it's not the problem of this series, but it just exposed this to libvirt by the series since it only happens when oob and daemonize are both enabled). The fixes are: [PATCH v2 0/2] delay monitor iothread creation Since it cannot be applied cleanly onto this series, I resolved the conflicts and pushed a tree here in case you wanna try with these two extra patches applied: https://github.com/xzpeter/qemu/tree/test-oob Please feel free to test with libvirt again with that. Thanks for playing with the tree and further investigation! I will also mention this in the next version of cover letter (if there is one). Regards, -- Peter Xu
On 10/10/18 7:05 PM, Peter Xu wrote: >> other than the command line that is hanging: >> >> /home/eblake/qemu/x86_64-softmmu/qemu-system-x86_64 -S -no-user-config >> -nodefaults -nographic -machine none,accel=kvm:tcg -qmp >> unix:/var/lib/libvirt/qemu/capabilities.monitor.sock,server,nowait -pidfile >> /var/lib/libvirt/qemu/capabilities.pidfile -daemonize >> >> And I suspect it is the -daemonize that is causing the hang I'm seeing when >> run by libvirt. >> > > It's very possible the daemonize thing, actually Wolfgang Bumiller has > posted patches to fix this up (it's not the problem of this series, > but it just exposed this to libvirt by the series since it only > happens when oob and daemonize are both enabled). The fixes are: > > [PATCH v2 0/2] delay monitor iothread creation > > Since it cannot be applied cleanly onto this series, I resolved the > conflicts and pushed a tree here in case you wanna try with these two > extra patches applied: > > https://github.com/xzpeter/qemu/tree/test-oob Technically, we should apply the patches in the opposite order (Wolfgang's first, then yours), so that bisection does not land on a known-bad hang situation. > > Please feel free to test with libvirt again with that. > > Thanks for playing with the tree and further investigation! I will > also mention this in the next version of cover letter (if there is one). Confirmed that libvirt no longer hung with Wolfgang's patches added in. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Wed, Oct 10, 2018 at 08:17:41PM -0500, Eric Blake wrote: > On 10/10/18 7:05 PM, Peter Xu wrote: > > > > other than the command line that is hanging: > > > > > > /home/eblake/qemu/x86_64-softmmu/qemu-system-x86_64 -S -no-user-config > > > -nodefaults -nographic -machine none,accel=kvm:tcg -qmp > > > unix:/var/lib/libvirt/qemu/capabilities.monitor.sock,server,nowait -pidfile > > > /var/lib/libvirt/qemu/capabilities.pidfile -daemonize > > > > > > And I suspect it is the -daemonize that is causing the hang I'm seeing when > > > run by libvirt. > > > > > > > > It's very possible the daemonize thing, actually Wolfgang Bumiller has > > posted patches to fix this up (it's not the problem of this series, > > but it just exposed this to libvirt by the series since it only > > happens when oob and daemonize are both enabled). The fixes are: > > > > [PATCH v2 0/2] delay monitor iothread creation > > > > Since it cannot be applied cleanly onto this series, I resolved the > > conflicts and pushed a tree here in case you wanna try with these two > > extra patches applied: > > > > https://github.com/xzpeter/qemu/tree/test-oob > > Technically, we should apply the patches in the opposite order (Wolfgang's > first, then yours), so that bisection does not land on a known-bad hang > situation. Agreed, that patch actually fixes bug of current master (current master could possibly hit the same issue when with x-oob=on and -daemonize), so should be applied earlier when proper. > > > > > Please feel free to test with libvirt again with that. > > > > Thanks for playing with the tree and further investigation! I will > > also mention this in the next version of cover letter (if there is one). > > Confirmed that libvirt no longer hung with Wolfgang's patches added in. Thanks! -- Peter Xu
© 2016 - 2024 Red Hat, Inc.