[libvirt] [PATCH] Run 'qmp_capabilities' command at monitor startup
--- src/qemu/qemu_driver.c | 18 ++++++++++++++---- src/qemu/qemu_monitor.c | 13 +++++++++++++ src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_monitor_json.c | 20 ++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 2 ++ 5 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 03feaf4..e5b11bc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -829,9 +829,10 @@ static qemuMonitorCallbacks monitorCallbacks = { }; static int -qemuConnectMonitor(virDomainObjPtr vm) +qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; + int ret; /* Hold an extra reference because we can't allow 'vm' to be * deleted while the monitor is active */ @@ -845,7 +846,16 @@ qemuConnectMonitor(virDomainObjPtr vm) return -1; } - return 0; + qemuDomainObjEnterMonitorWithDriver(driver, vm); + ret = qemuMonitorSetCapabilities(priv->mon); + qemuDomainObjExitMonitorWithDriver(driver, vm); + + if (ret < 0) { + qemuMonitorClose(priv->mon); + priv->mon = NULL; + } + + return ret; } /* @@ -867,7 +877,7 @@ qemuReconnectDomain(void *payload, const char *name ATTRIBUTE_UNUSED, void *opaq priv = obj->privateData; /* XXX check PID liveliness & EXE path */ - if (qemuConnectMonitor(obj) < 0) + if (qemuConnectMonitor(driver, obj) < 0) goto error; if (qemuUpdateActivePciHostdevs(driver, obj->def) < 0) { @@ -1576,7 +1586,7 @@ qemudWaitForMonitor(struct qemud_driver* driver, return -1; VIR_DEBUG("Connect monitor to %p '%s'", vm, vm->def->name); - if (qemuConnectMonitor(vm) < 0) + if (qemuConnectMonitor(driver, vm) < 0) return -1; /* Try to get the pty path mappings again via the monitor. This is much more diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index c1d369b..b88532c 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -791,6 +791,19 @@ int qemuMonitorEmitStop(qemuMonitorPtr mon) } +int qemuMonitorSetCapabilities(qemuMonitorPtr mon) +{ + int ret; + DEBUG("mon=%p, fd=%d", mon, mon->fd); + + if (mon->json) + ret = qemuMonitorJSONSetCapabilities(mon); + else + ret = 0; + return ret; +} + + int qemuMonitorStartCPUs(qemuMonitorPtr mon, virConnectPtr conn) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 786ad7a..0ac3957 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -99,6 +99,8 @@ qemuMonitorPtr qemuMonitorOpen(virDomainObjPtr vm, int qemuMonitorClose(qemuMonitorPtr mon); +int qemuMonitorSetCapabilities(qemuMonitorPtr mon); + void qemuMonitorLock(qemuMonitorPtr mon); void qemuMonitorUnlock(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index c9b8d60..f0dcf81 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -477,6 +477,26 @@ static void qemuMonitorJSONHandleStop(qemuMonitorPtr mon, virJSONValuePtr data A int +qemuMonitorJSONSetCapabilities(qemuMonitorPtr mon) +{ + int ret; + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("qmp_capabilities", NULL); + virJSONValuePtr reply = NULL; + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + +int qemuMonitorJSONStartCPUs(qemuMonitorPtr mon, virConnectPtr conn ATTRIBUTE_UNUSED) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 65a70e3..c6a6d51 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -34,6 +34,8 @@ int qemuMonitorJSONIOProcess(qemuMonitorPtr mon, size_t len, qemuMonitorMessagePtr msg); +int qemuMonitorJSONSetCapabilities(qemuMonitorPtr mon); + int qemuMonitorJSONStartCPUs(qemuMonitorPtr mon, virConnectPtr conn); int qemuMonitorJSONStopCPUs(qemuMonitorPtr mon); -- 1.6.6
2010/2/15 Daniel P. Berrange <berrange@redhat.com>:
--- src/qemu/qemu_driver.c | 18 ++++++++++++++---- src/qemu/qemu_monitor.c | 13 +++++++++++++ src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_monitor_json.c | 20 ++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 2 ++ 5 files changed, 51 insertions(+), 4 deletions(-)
ACK. Why do we need to call qmp_capabilities at startup? Matthias
On Mon, Feb 15, 2010 at 04:14:01PM +0100, Matthias Bolte wrote:
2010/2/15 Daniel P. Berrange <berrange@redhat.com>:
--- src/qemu/qemu_driver.c | 18 ++++++++++++++---- src/qemu/qemu_monitor.c | 13 +++++++++++++ src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_monitor_json.c | 20 ++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 2 ++ 5 files changed, 51 insertions(+), 4 deletions(-)
ACK.
Why do we need to call qmp_capabilities at startup?
Because QEMU developers just decided to add it and make it mandatory :-) /me is happy the JSON code is still disabled until QEMU 0.13 comes out Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Mon, 15 Feb 2010 15:16:32 +0000 "Daniel P. Berrange" <berrange@redhat.com> wrote:
On Mon, Feb 15, 2010 at 04:14:01PM +0100, Matthias Bolte wrote:
2010/2/15 Daniel P. Berrange <berrange@redhat.com>:
--- src/qemu/qemu_driver.c | 18 ++++++++++++++---- src/qemu/qemu_monitor.c | 13 +++++++++++++ src/qemu/qemu_monitor.h | 2 ++ src/qemu/qemu_monitor_json.c | 20 ++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 2 ++ 5 files changed, 51 insertions(+), 4 deletions(-)
ACK.
Why do we need to call qmp_capabilities at startup?
Because QEMU developers just decided to add it and make it mandatory :-)
Actually, it's because when a client connects QMP is in "capabilities negotiation mode". In this mode only the qmp_capabilities command can be issued. When this is done QMP changes to "command mode", where all commands are accepted. This is QMP's capabilities negotiation support. It's a mandatory step because new capabilities are reserved for protocol changes or features which are not backward compatible. This way, it's important that server and clients agree on what is going to be on the wire before any message is exchanged. We don't have any capabilities yet, so qmp_capabilities is issued without arguments. Which is what clients should do if they don't support new features.
/me is happy the JSON code is still disabled until QEMU 0.13 comes out
Well, I was in favor of making QMP stable for 0.12, but today I realize that this would have been a mistake, as the protocol wasn't mature yet. I understand that having to keep up to date with an unstable protocol isn't always fun, on the other hand libvirt's early adoption and your inputs are crucial to guide QMP's unstable phase into the right direction. Please, let me know if there's anything that we could improve on our side.
participants (3)
-
Daniel P. Berrange -
Luiz Capitulino -
Matthias Bolte