[libvirt] [PATCH 0/2] Avoid touching monitor when fetching guest XML or info
The virDomainGetXMLDesc and virDomainGetInfo APIs for QEMU suffer from needing to run 'query-balloon' to update the balloon level. This has caused us performance problems and even worse caused us to lock up on a dead QEMU. The following two patches to QEMU add a BALLOON_EVENT and a query-events command to QMP. With those two parts, we can avoiding running the query-balloon command at all. https://lists.gnu.org/archive/html/qemu-devel/2012-05/msg02215.html https://lists.gnu.org/archive/html/qemu-devel/2012-05/msg02255.html
From: "Daniel P. Berrange" <berrange@redhat.com> When the guest changes its memory balloon applications may want to know what the new value is, without having to periodically poll on XML / domain info. Introduce a "balloon change" event to let apps see this * include/libvirt/libvirt.h.in: Define the virConnectDomainEventBalloonChangeCallback callback and VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE constant * python/libvirt-override-virConnect.py, python/libvirt-override.c: Wire up helpers for new event * daemon/remote.c: Helper for serializing balloon event * examples/domain-events/events-c/event-test.c, examples/domain-events/events-python/event-test.py: Add example of balloon event usage * src/conf/domain_event.c, src/conf/domain_event.h: Handling of balloon events * src/remote/remote_driver.c: Add handler of balloon events * src/remote/remote_protocol.x: Define wire protocol for balloon events Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/remote.c | 27 +++++++++++ examples/domain-events/events-c/event-test.c | 29 +++++++++++- examples/domain-events/events-python/event-test.py | 3 + include/libvirt/libvirt.h.in | 17 +++++++ python/libvirt-override-virConnect.py | 9 ++++ python/libvirt-override.c | 50 ++++++++++++++++++++ src/conf/domain_event.c | 34 +++++++++++++ src/conf/domain_event.h | 3 + src/libvirt_private.syms | 2 + src/remote/remote_driver.c | 31 ++++++++++++ src/remote/remote_protocol.x | 9 +++- 11 files changed, 212 insertions(+), 2 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 16a8a05..7826059 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -582,6 +582,32 @@ static int remoteRelayDomainEventPMSuspend(virConnectPtr conn ATTRIBUTE_UNUSED, return 0; } +static int remoteRelayDomainEventBalloonChange(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + unsigned long long actual, + void *opaque) +{ + virNetServerClientPtr client = opaque; + remote_domain_event_balloon_change_msg data; + + if (!client) + return -1; + + VIR_DEBUG("Relaying domain ballon change event %s %d %lld", dom->name, dom->id, actual); + + /* build return data */ + memset(&data, 0, sizeof(data)); + make_nonnull_domain(&data.dom, dom); + data.actual = actual; + + remoteDispatchDomainEventSend(client, remoteProgram, + REMOTE_PROC_DOMAIN_EVENT_BALLOON_CHANGE, + (xdrproc_t)xdr_remote_domain_event_balloon_change_msg, &data); + + return 0; +} + + static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventLifecycle), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventReboot), @@ -596,6 +622,7 @@ static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventTrayChange), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventPMWakeup), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventPMSuspend), + VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventBalloonChange), }; verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST); diff --git a/examples/domain-events/events-c/event-test.c b/examples/domain-events/events-c/event-test.c index c54778f..2991acb 100644 --- a/examples/domain-events/events-c/event-test.c +++ b/examples/domain-events/events-c/event-test.c @@ -222,6 +222,25 @@ static int myDomainEventRTCChangeCallback(virConnectPtr conn ATTRIBUTE_UNUSED, return 0; } +static int myDomainEventBalloonChangeCallback(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + unsigned long long actual, + void *opaque ATTRIBUTE_UNUSED) +{ + char *str = NULL; + /* HACK: use asprintf since we have gnulib's wrapper for %lld on Win32 + * but don't have a printf() replacement with %lld */ + if (asprintf(&str, "%s EVENT: Domain %s(%d) balloon change %lld KB\n", + __func__, virDomainGetName(dom), + virDomainGetID(dom), actual) < 0) + return 0; + + printf("%s", str); + free(str); + + return 0; +} + static int myDomainEventWatchdogCallback(virConnectPtr conn ATTRIBUTE_UNUSED, virDomainPtr dom, int action, @@ -391,6 +410,7 @@ int main(int argc, char **argv) int callback10ret = -1; int callback11ret = -1; int callback12ret = -1; + int callback13ret = -1; struct sigaction action_stop; memset(&action_stop, 0, sizeof(action_stop)); @@ -476,6 +496,11 @@ int main(int argc, char **argv) VIR_DOMAIN_EVENT_ID_PMSUSPEND, VIR_DOMAIN_EVENT_CALLBACK(myDomainEventPMSuspendCallback), strdup("pmsuspend"), myFreeFunc); + callback13ret = virConnectDomainEventRegisterAny(dconn, + NULL, + VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE, + VIR_DOMAIN_EVENT_CALLBACK(myDomainEventBalloonChangeCallback), + strdup("callback balloonchange"), myFreeFunc); if ((callback1ret != -1) && (callback2ret != -1) && (callback3ret != -1) && @@ -486,7 +511,8 @@ int main(int argc, char **argv) (callback9ret != -1) && (callback10ret != -1) && (callback11ret != -1) && - (callback12ret != -1)) { + (callback12ret != -1) && + (callback13ret != -1)) { if (virConnectSetKeepAlive(dconn, 5, 3) < 0) { virErrorPtr err = virGetLastError(); fprintf(stderr, "Failed to start keepalive protocol: %s\n", @@ -514,6 +540,7 @@ int main(int argc, char **argv) virConnectDomainEventDeregisterAny(dconn, callback10ret); virConnectDomainEventDeregisterAny(dconn, callback11ret); virConnectDomainEventDeregisterAny(dconn, callback12ret); + virConnectDomainEventDeregisterAny(dconn, callback13ret); if (callback8ret != -1) virConnectDomainEventDeregisterAny(dconn, callback8ret); } diff --git a/examples/domain-events/events-python/event-test.py b/examples/domain-events/events-python/event-test.py index 96dc268..bd42c76 100644 --- a/examples/domain-events/events-python/event-test.py +++ b/examples/domain-events/events-python/event-test.py @@ -483,6 +483,8 @@ def myDomainEventPMWakeupCallback(conn, dom, reason, opaque): def myDomainEventPMSuspendCallback(conn, dom, reason, opaque): print "myDomainEventPMSuspendCallback: Domain %s(%s) system pmsuspend" % ( dom.name(), dom.ID()) +def myDomainEventRTCChangeCallback(conn, dom, utcoffset, actual): + print "myDomainEventBalloonChangeCallback: Domain %s(%s) %d" % (dom.name(), dom.ID(), actual) def usage(out=sys.stderr): print >>out, "usage: "+os.path.basename(sys.argv[0])+" [-hdl] [uri]" print >>out, " uri will default to qemu:///system" @@ -544,6 +546,7 @@ def main(): vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_TRAY_CHANGE, myDomainEventTrayChangeCallback, None) vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_PMWAKEUP, myDomainEventPMWakeupCallback, None) vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_PMSUSPEND, myDomainEventPMSuspendCallback, None) + vc.domainEventRegisterAny(None, libvirt.VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE, myDomainEventBalloonChangeCallback, None) vc.setKeepAlive(5, 3) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index ac5df95..c1ee67b 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3780,6 +3780,22 @@ typedef void (*virConnectDomainEventPMSuspendCallback)(virConnectPtr conn, int reason, void *opaque); + +/** + * virConnectDomainEventBalloonChangeCallback: + * @conn: connection object + * @dom: domain on which the event occurred + * @actual: the new balloon level measured in kilobytes + * @opaque: application specified data + * + * The callback signature to use when registering for an event of type + * VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE with virConnectDomainEventRegisterAny() + */ +typedef void (*virConnectDomainEventBalloonChangeCallback)(virConnectPtr conn, + virDomainPtr dom, + unsigned long long actual, + void *opaque); + /** * VIR_DOMAIN_EVENT_CALLBACK: * @@ -3803,6 +3819,7 @@ typedef enum { VIR_DOMAIN_EVENT_ID_TRAY_CHANGE = 10, /* virConnectDomainEventTrayChangeCallback */ VIR_DOMAIN_EVENT_ID_PMWAKEUP = 11, /* virConnectDomainEventPMWakeupCallback */ VIR_DOMAIN_EVENT_ID_PMSUSPEND = 12, /* virConnectDomainEventPMSuspendCallback */ + VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE = 13, /* virConnectDomainEventBalloonChangeCallback */ #ifdef VIR_ENUM_SENTINELS /* diff --git a/python/libvirt-override-virConnect.py b/python/libvirt-override-virConnect.py index 811e16b..badd69d 100644 --- a/python/libvirt-override-virConnect.py +++ b/python/libvirt-override-virConnect.py @@ -161,6 +161,15 @@ cb(self, virDomain(self, _obj=dom), reason, opaque) return 0; + def _dispatchDomainEventBalloonChangeCallback(self, dom, actual, cbData): + """Dispatches events to python user domain balloon change event callbacks + """ + cb = cbData["cb"] + opaque = cbData["opaque"] + + cb(self, virDomain(self, _obj=dom), actual, opaque) + return 0 + def domainEventDeregisterAny(self, callbackID): """Removes a Domain Event Callback. De-registering for a domain callback will disable delivery of this event type """ diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 130e702..8ca3a91 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -5199,6 +5199,53 @@ libvirt_virConnectDomainEventPMSuspendCallback(virConnectPtr conn ATTRIBUTE_UNUS return ret; } +static int +libvirt_virConnectDomainEventBalloonChangeCallback(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + unsigned long long actual, + void *opaque) +{ + PyObject *pyobj_cbData = (PyObject*)opaque; + PyObject *pyobj_dom; + PyObject *pyobj_ret; + PyObject *pyobj_conn; + PyObject *dictKey; + int ret = -1; + + LIBVIRT_ENSURE_THREAD_STATE; + + /* Create a python instance of this virDomainPtr */ + virDomainRef(dom); + pyobj_dom = libvirt_virDomainPtrWrap(dom); + Py_INCREF(pyobj_cbData); + + dictKey = libvirt_constcharPtrWrap("conn"); + pyobj_conn = PyDict_GetItem(pyobj_cbData, dictKey); + Py_DECREF(dictKey); + + /* Call the Callback Dispatcher */ + pyobj_ret = PyObject_CallMethod(pyobj_conn, + (char*)"_dispatchDomainEventBalloonChangeCallback", + (char*)"OLO", + pyobj_dom, + (PY_LONG_LONG)actual, + pyobj_cbData); + + Py_DECREF(pyobj_cbData); + Py_DECREF(pyobj_dom); + + if(!pyobj_ret) { + DEBUG("%s - ret:%p\n", __FUNCTION__, pyobj_ret); + PyErr_Print(); + } else { + Py_DECREF(pyobj_ret); + ret = 0; + } + + LIBVIRT_RELEASE_THREAD_STATE; + return ret; +} + static PyObject * libvirt_virConnectDomainEventRegisterAny(ATTRIBUTE_UNUSED PyObject * self, PyObject * args) @@ -5268,6 +5315,9 @@ libvirt_virConnectDomainEventRegisterAny(ATTRIBUTE_UNUSED PyObject * self, case VIR_DOMAIN_EVENT_ID_PMSUSPEND: cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventPMSuspendCallback); break; + case VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE: + cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventBalloonChangeCallback); + break; } if (!cb) { diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 923c58d..2ff2cc1 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -121,6 +121,9 @@ struct _virDomainEvent { char *devAlias; int reason; } trayChange; + struct { + unsigned long long actual; + } balloonChange; } data; }; @@ -1144,6 +1147,31 @@ virDomainEventPMSuspendNewFromDom(virDomainPtr dom) return virDomainEventPMSuspendNew(dom->id, dom->name, dom->uuid); } +virDomainEventPtr virDomainEventBalloonChangeNewFromDom(virDomainPtr dom, + unsigned long long actual) +{ + virDomainEventPtr ev = + virDomainEventNewInternal(VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE, + dom->id, dom->name, dom->uuid); + + if (ev) + ev->data.balloonChange.actual = actual; + + return ev; +} +virDomainEventPtr virDomainEventBalloonChangeNewFromObj(virDomainObjPtr obj, + unsigned long long actual) +{ + virDomainEventPtr ev = + virDomainEventNewInternal(VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE, + obj->def->id, obj->def->name, obj->def->uuid); + + if (ev) + ev->data.balloonChange.actual = actual; + + return ev; +} + /** * virDomainEventQueuePush: * @evtQueue: the dom event queue @@ -1282,6 +1310,12 @@ virDomainEventDispatchDefaultFunc(virConnectPtr conn, ((virConnectDomainEventPMSuspendCallback)cb)(conn, dom, 0, cbopaque); break; + case VIR_DOMAIN_EVENT_ID_BALLOON_CHANGE: + ((virConnectDomainEventBalloonChangeCallback)cb)(conn, dom, + event->data.balloonChange.actual, + cbopaque); + break; + default: VIR_WARN("Unexpected event ID %d", event->eventID); break; diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index f7776c7..15745b5 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -125,6 +125,9 @@ virDomainEventPtr virDomainEventPMWakeupNewFromDom(virDomainPtr dom); virDomainEventPtr virDomainEventPMSuspendNewFromObj(virDomainObjPtr obj); virDomainEventPtr virDomainEventPMSuspendNewFromDom(virDomainPtr dom); +virDomainEventPtr virDomainEventBalloonChangeNewFromDom(virDomainPtr dom, unsigned long long actual); +virDomainEventPtr virDomainEventBalloonChangeNewFromObj(virDomainObjPtr obj, unsigned long long actual); + void virDomainEventFree(virDomainEventPtr event); void virDomainEventStateFree(virDomainEventStatePtr state); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f5c2184..b10beb5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -494,6 +494,8 @@ virDomainWatchdogModelTypeToString; # domain_event.h +virDomainEventBalloonChangeNewFromDom; +virDomainEventBalloonChangeNewFromObj; virDomainEventBlockJobNewFromObj; virDomainEventBlockJobNewFromDom; virDomainEventControlErrorNewFromDom; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 5c87561..63dcdc0 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -253,6 +253,10 @@ static void remoteDomainBuildEventPMSuspend(virNetClientProgramPtr prog, virNetClientPtr client, void *evdata, void *opaque); +static void +remoteDomainBuildEventBalloonChange(virNetClientProgramPtr prog, + virNetClientPtr client, + void *evdata, void *opaque); static virNetClientProgramEvent remoteDomainEvents[] = { { REMOTE_PROC_DOMAIN_EVENT_RTC_CHANGE, @@ -307,6 +311,10 @@ static virNetClientProgramEvent remoteDomainEvents[] = { remoteDomainBuildEventPMSuspend, sizeof(remote_domain_event_pmsuspend_msg), (xdrproc_t)xdr_remote_domain_event_pmsuspend_msg }, + { REMOTE_PROC_DOMAIN_EVENT_BALLOON_CHANGE, + remoteDomainBuildEventBalloonChange, + sizeof(remote_domain_event_balloon_change_msg), + (xdrproc_t)xdr_remote_domain_event_balloon_change_msg }, }; enum virDrvOpenRemoteFlags { @@ -3826,6 +3834,29 @@ remoteDomainBuildEventPMSuspend(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, remoteDomainEventQueue(priv, event); } + +static void +remoteDomainBuildEventBalloonChange(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, + virNetClientPtr client ATTRIBUTE_UNUSED, + void *evdata, void *opaque) +{ + virConnectPtr conn = opaque; + struct private_data *priv = conn->privateData; + remote_domain_event_balloon_change_msg *msg = evdata; + virDomainPtr dom; + virDomainEventPtr event = NULL; + + dom = get_nonnull_domain(conn, msg->dom); + if (!dom) + return; + + event = virDomainEventBalloonChangeNewFromDom(dom, msg->actual); + virDomainFree(dom); + + remoteDomainEventQueue(priv, event); +} + + static virDrvOpenStatus ATTRIBUTE_NONNULL (1) remoteSecretOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 2d57247..3e1204c 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2192,6 +2192,11 @@ struct remote_domain_event_pmsuspend_msg { remote_nonnull_domain dom; }; +struct remote_domain_event_balloon_change_msg { + remote_nonnull_domain dom; + unsigned hyper actual; +}; + struct remote_domain_managed_save_args { remote_nonnull_domain dom; unsigned int flags; @@ -2782,7 +2787,9 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_PM_WAKEUP = 267, /* autogen autogen */ REMOTE_PROC_DOMAIN_EVENT_TRAY_CHANGE = 268, /* autogen autogen */ REMOTE_PROC_DOMAIN_EVENT_PMWAKEUP = 269, /* autogen autogen */ - REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND = 270 /* autogen autogen */ + REMOTE_PROC_DOMAIN_EVENT_PMSUSPEND = 270, /* autogen autogen */ + + REMOTE_PROC_DOMAIN_EVENT_BALLOON_CHANGE = 271 /* autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? -- 1.7.7.6
On 05/16/2012 08:35 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
When the guest changes its memory balloon applications may want to know what the new value is, without having to periodically poll on XML / domain info. Introduce a "balloon change" event to let apps see this
+++ b/daemon/remote.c @@ -582,6 +582,32 @@ static int remoteRelayDomainEventPMSuspend(virConnectPtr conn ATTRIBUTE_UNUSED, return 0; }
+static int remoteRelayDomainEventBalloonChange(virConnectPtr conn ATTRIBUTE_UNUSED,
Formatting nit: should the function name start on a new line, leaving 'static int' on a line of its own?
+ virDomainPtr dom, + unsigned long long actual, + void *opaque) +{ + virNetServerClientPtr client = opaque; + remote_domain_event_balloon_change_msg data; + + if (!client) + return -1; + + VIR_DEBUG("Relaying domain ballon change event %s %d %lld", dom->name, dom->id, actual);
s/ballon/balloon/
+++ b/examples/domain-events/events-c/event-test.c @@ -222,6 +222,25 @@ static int myDomainEventRTCChangeCallback(virConnectPtr conn ATTRIBUTE_UNUSED, return 0; }
+static int myDomainEventBalloonChangeCallback(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + unsigned long long actual, + void *opaque ATTRIBUTE_UNUSED) +{ + char *str = NULL; + /* HACK: use asprintf since we have gnulib's wrapper for %lld on Win32 + * but don't have a printf() replacement with %lld */
Yep, I can see why you did this. And since our example code can't use virAsprintf (which is internal to libvirt, but our examples are supposed to be standalone), we definitely need to keep this comment. I'm a bit worried, though, that we are making this example harder than necessary to use on BSD (that is, avoiding our virAsprintf wrapper but then relying on our gnulib wrapper seems risky). Maybe we should do: #ifdef WIN32 use printf("%I64d") #else use printf("%lld") #endif so that the example code is self-standing without asprintf. I don't know which approach is worse, though, and at least your comment should be sufficient to give someone a hint of how to work around things on a system that lacks asprintf when trying to use the example code in a standalone manner.
+++ b/include/libvirt/libvirt.h.in @@ -3780,6 +3780,22 @@ typedef void (*virConnectDomainEventPMSuspendCallback)(virConnectPtr conn, int reason, void *opaque);
+ +/** + * virConnectDomainEventBalloonChangeCallback: + * @conn: connection object + * @dom: domain on which the event occurred + * @actual: the new balloon level measured in kilobytes
s/kilobytes/kibibytes (blocks of 1024 bytes)/ At any rate, I agree with keeping this in blocks of 1024, to match our XML for memory balloon sizing. If we had a time machine, I would have designed that value in bytes instead; oh well.
+++ b/src/conf/domain_event.c @@ -121,6 +121,9 @@ struct _virDomainEvent { char *devAlias; int reason; } trayChange; + struct { + unsigned long long actual;
add a comment, documenting that this is in units of 1024 (otherwise, we may end up with a future patch goofing and using bytes or megabytes instead of kilobytes, and messing things up) -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> If QEMU supports the BALLOON_EVENT QMP event, then we can avoid invoking 'query-balloon' when returning XML or the domain info. * src/qemu/qemu_capabilities.c, src/qemu/qemu_capabilities.h: Add QEMU_CAPS_BALLOON_EVENT * src/qemu/qemu_driver.c: Skip query-balloon in qemudDomainGetInfo and qemuDomainGetXMLDesc if we have QEMU_CAPS_BALLOON_EVENT set * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h: Check for BALLOON_EVENT at connect to monitor. Add callback for balloon change notifications * src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h: Add handling of BALLOON_EVENT and impl 'query-events' check Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 7 ++++- src/qemu/qemu_monitor.c | 18 +++++++++++- src/qemu/qemu_monitor.h | 5 +++ src/qemu/qemu_monitor_json.c | 65 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 2 + src/qemu/qemu_process.c | 31 ++++++++++++++++++++ 8 files changed, 129 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a3c87d1..223852a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -162,6 +162,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "scsi-cd", "ide-cd", "no-user-config", + + "balloon-event", /* 95 */ ); struct qemu_feature_flags { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 0e0899e..8c829dc 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -130,6 +130,7 @@ enum qemuCapsFlags { QEMU_CAPS_SCSI_CD = 92, /* -device scsi-cd */ QEMU_CAPS_IDE_CD = 93, /* -device ide-cd */ QEMU_CAPS_NO_USER_CONFIG = 94, /* -no-user-config */ + QEMU_CAPS_BALLOON_EVENT = 95, /* Async event for balloon changes */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0fd7de1..45eb16d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2226,6 +2226,8 @@ static int qemudDomainGetInfo(virDomainPtr dom, if ((vm->def->memballoon != NULL) && (vm->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE)) { info->memory = vm->def->mem.max_balloon; + } else if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_BALLOON_EVENT)) { + info->memory = vm->def->mem.cur_balloon; } else if (qemuDomainJobAllowed(priv, QEMU_JOB_QUERY)) { if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) goto cleanup; @@ -4500,6 +4502,7 @@ static char *qemuDomainGetXMLDesc(virDomainPtr dom, char *ret = NULL; unsigned long long balloon; int err = 0; + qemuDomainObjPrivatePtr priv; /* Flags checked by virDomainDefFormat */ @@ -4514,11 +4517,13 @@ static char *qemuDomainGetXMLDesc(virDomainPtr dom, goto cleanup; } + priv = vm->privateData; + /* Refresh current memory based on balloon info if supported */ if ((vm->def->memballoon != NULL) && (vm->def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_NONE) && + !qemuCapsGet(priv->qemuCaps, QEMU_CAPS_BALLOON_EVENT) && (virDomainObjIsActive(vm))) { - qemuDomainObjPrivatePtr priv = vm->privateData; /* Don't delay if someone's using the monitor, just use * existing most recent data instead */ if (qemuDomainJobAllowed(priv, QEMU_JOB_QUERY)) { diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 7d69c67..2b04240 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1087,6 +1087,16 @@ int qemuMonitorEmitBlockJob(qemuMonitorPtr mon, } +int qemuMonitorEmitBalloonChange(qemuMonitorPtr mon, + unsigned long long actual) +{ + int ret = -1; + VIR_DEBUG("mon=%p", mon); + + QEMU_MONITOR_CALLBACK(mon, ret, domainBalloonChange, mon->vm, actual); + return ret; +} + int qemuMonitorSetCapabilities(qemuMonitorPtr mon, virBitmapPtr qemuCaps) @@ -1103,11 +1113,17 @@ int qemuMonitorSetCapabilities(qemuMonitorPtr mon, if (mon->json) { ret = qemuMonitorJSONSetCapabilities(mon); - if (ret) + if (ret < 0) goto cleanup; ret = qemuMonitorJSONCheckCommands(mon, qemuCaps, &json_hmp); + if (ret < 0) + goto cleanup; mon->json_hmp = json_hmp > 0; + + ret = qemuMonitorJSONCheckEvents(mon, qemuCaps); + if (ret < 0) + goto cleanup; } else { ret = 0; } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index ffe8fe7..3803ea6 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -133,6 +133,9 @@ struct _qemuMonitorCallbacks { virDomainObjPtr vm); int (*domainPMSuspend)(qemuMonitorPtr mon, virDomainObjPtr vm); + int (*domainBalloonChange)(qemuMonitorPtr mon, + virDomainObjPtr vm, + unsigned long long actual); }; char *qemuMonitorEscapeArg(const char *in); @@ -208,6 +211,8 @@ int qemuMonitorEmitBlockJob(qemuMonitorPtr mon, const char *diskAlias, int type, int status); +int qemuMonitorEmitBalloonChange(qemuMonitorPtr mon, + unsigned long long actual); int qemuMonitorStartCPUs(qemuMonitorPtr mon, virConnectPtr conn); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e1f5453..e3bdf51 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -66,6 +66,7 @@ static void qemuMonitorJSONHandlePMWakeup(qemuMonitorPtr mon, virJSONValuePtr da static void qemuMonitorJSONHandlePMSuspend(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleBlockJobCompleted(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleBlockJobCanceled(qemuMonitorPtr mon, virJSONValuePtr data); +static void qemuMonitorJSONHandleBalloonChange(qemuMonitorPtr mon, virJSONValuePtr data); typedef struct { const char *type; @@ -73,6 +74,7 @@ typedef struct { } qemuEventHandler; static qemuEventHandler eventHandlers[] = { + { "BALLOON_CHANGE", qemuMonitorJSONHandleBalloonChange, }, { "BLOCK_IO_ERROR", qemuMonitorJSONHandleIOError, }, { "BLOCK_JOB_CANCELLED", qemuMonitorJSONHandleBlockJobCanceled, }, { "BLOCK_JOB_COMPLETED", qemuMonitorJSONHandleBlockJobCompleted, }, @@ -872,6 +874,19 @@ qemuMonitorJSONHandleBlockJobCanceled(qemuMonitorPtr mon, VIR_DOMAIN_BLOCK_JOB_CANCELED); } +static void +qemuMonitorJSONHandleBalloonChange(qemuMonitorPtr mon, + virJSONValuePtr data) +{ + unsigned long long actual = 0; + if (virJSONValueObjectGetNumberUlong(data, "actual", &actual) < 0) { + VIR_WARN("missing actual in balloon change event"); + return; + } + actual = (actual/1024); + qemuMonitorEmitBalloonChange(mon, actual); +} + int qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon, const char *cmd_str, @@ -999,6 +1014,56 @@ cleanup: int +qemuMonitorJSONCheckEvents(qemuMonitorPtr mon, + virBitmapPtr qemuCaps) +{ + int ret = -1; + virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-events", NULL); + virJSONValuePtr reply = NULL; + virJSONValuePtr data; + int i, n; + + if (!cmd) + return ret; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + ret = 0; + goto cleanup; + } + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + if (!(data = virJSONValueObjectGet(reply, "return")) || + data->type != VIR_JSON_TYPE_ARRAY || + (n = virJSONValueArraySize(data)) <= 0) + goto cleanup; + + for (i = 0; i < n; i++) { + virJSONValuePtr entry; + const char *name; + + if (!(entry = virJSONValueArrayGet(data, i)) || + !(name = virJSONValueObjectGetString(entry, "name"))) + goto cleanup; + + if (STREQ(name, "BALLOON_CHANGE")) + qemuCapsSet(qemuCaps, QEMU_CAPS_BALLOON_EVENT); + } + + ret = 0; + +cleanup: + 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 22a3adf..46b6ab3 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -45,6 +45,8 @@ int qemuMonitorJSONSetCapabilities(qemuMonitorPtr mon); int qemuMonitorJSONCheckCommands(qemuMonitorPtr mon, virBitmapPtr qemuCaps, int *json_hmp); +int qemuMonitorJSONCheckEvents(qemuMonitorPtr mon, + virBitmapPtr qemuCaps); int qemuMonitorJSONStartCPUs(qemuMonitorPtr mon, virConnectPtr conn); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 58ba5bf..3248dbd 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1139,6 +1139,36 @@ qemuProcessHandlePMSuspend(qemuMonitorPtr mon ATTRIBUTE_UNUSED, return 0; } +static int +qemuProcessHandleBalloonChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + unsigned long long actual) +{ + struct qemud_driver *driver = qemu_driver; + virDomainEventPtr event; + + virDomainObjLock(vm); + event = virDomainEventBalloonChangeNewFromObj(vm, actual); + + VIR_DEBUG("Updating balloon from %lld to %lld kb", + vm->def->mem.cur_balloon, actual); + vm->def->mem.cur_balloon = actual; + + if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) + VIR_WARN("unable to save domain status with balloon change"); + + virDomainObjUnlock(vm); + + if (event) { + qemuDriverLock(driver); + qemuDomainEventQueue(driver, event); + qemuDriverUnlock(driver); + } + + return 0; +} + + static qemuMonitorCallbacks monitorCallbacks = { .destroy = qemuProcessHandleMonitorDestroy, .eofNotify = qemuProcessHandleMonitorEOF, @@ -1155,6 +1185,7 @@ static qemuMonitorCallbacks monitorCallbacks = { .domainTrayChange = qemuProcessHandleTrayChange, .domainPMWakeup = qemuProcessHandlePMWakeup, .domainPMSuspend = qemuProcessHandlePMSuspend, + .domainBalloonChange = qemuProcessHandleBalloonChange, }; static int -- 1.7.7.6
On 05/16/2012 08:35 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
If QEMU supports the BALLOON_EVENT QMP event, then we can avoid invoking 'query-balloon' when returning XML or the domain info.
* src/qemu/qemu_capabilities.c, src/qemu/qemu_capabilities.h: Add QEMU_CAPS_BALLOON_EVENT * src/qemu/qemu_driver.c: Skip query-balloon in qemudDomainGetInfo and qemuDomainGetXMLDesc if we have QEMU_CAPS_BALLOON_EVENT set * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h: Check for BALLOON_EVENT at connect to monitor. Add callback for balloon change notifications * src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h: Add handling of BALLOON_EVENT and impl 'query-events' check
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 7 ++++- src/qemu/qemu_monitor.c | 18 +++++++++++- src/qemu/qemu_monitor.h | 5 +++ src/qemu/qemu_monitor_json.c | 65 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 2 + src/qemu/qemu_process.c | 31 ++++++++++++++++++++ 8 files changed, 129 insertions(+), 2 deletions(-)
+ QEMU_CAPS_BALLOON_EVENT = 95, /* Async event for balloon changes */
Yet another cap bit that can only be learned when the guest is live. We really need to start caching qemu capabilities per binary... But at least your patch only matters for live guests (unlike my recent patches for block copy, where I was giving a bad error message for offline guests until I added commit 8e532d34 to shuffle things around).
+static void +qemuMonitorJSONHandleBalloonChange(qemuMonitorPtr mon, + virJSONValuePtr data) +{ + unsigned long long actual = 0; + if (virJSONValueObjectGetNumberUlong(data, "actual", &actual) < 0) { + VIR_WARN("missing actual in balloon change event"); + return; + } + actual = (actual/1024);
I think you want to use VIR_DIV_UP(actual, 1024) here, just in case qemu ever reports a value with finer granularity than a kilobyte. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 05/16/2012 08:35 AM, Daniel P. Berrange wrote:
The virDomainGetXMLDesc and virDomainGetInfo APIs for QEMU suffer from needing to run 'query-balloon' to update the balloon level. This has caused us performance problems and even worse caused us to lock up on a dead QEMU.
The following two patches to QEMU add a BALLOON_EVENT and a query-events command to QMP. With those two parts, we can avoiding running the query-balloon command at all.
https://lists.gnu.org/archive/html/qemu-devel/2012-05/msg02215.html https://lists.gnu.org/archive/html/qemu-devel/2012-05/msg02255.html
I like the goal of this series, but we shouldn't apply it until this has been accepted in upstream qemu and we have a commit id to point to. That said, I'll still review the code. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Daniel P. Berrange -
Eric Blake