- Devel - libvirt List Archives

[libvirt] [PATCH] daemon: Plug memory leaks
by Alex Jia 12 Apr '12

12 Apr '12

Detected by valgrind. Leaks are introduced in commit 6e6e9be. * daemon/libvirtd-config.c (macro GET_CONF_STR): fix memory leaks. How to reproduce? % make && make -C tests check TESTS=libvirtdconftest % cd tests && valgrind -v --leak-check=full ./libvirtdconftest actual result: ==11008== 185 bytes in 5 blocks are definitely lost in loss record 3 of 5 ==11008== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==11008== by 0x39CF07F6E1: strdup (strdup.c:43) ==11008== by 0x406626: daemonConfigLoadOptions (libvirtd-config.c:438) ==11008== by 0x406800: daemonConfigLoadData (libvirtd-config.c:492) ==11008== by 0x403CCF: testCorrupt (libvirtdconftest.c:110) ==11008== by 0x404FAD: virtTestRun (testutils.c:145) ==11008== by 0x403A34: mymain (libvirtdconftest.c:219) ==11008== by 0x404687: virtTestMain (testutils.c:700) ==11008== by 0x39CF01ECDC: (below main) (libc-start.c:226) ==11008== ==11008== LEAK SUMMARY: ==11008== definitely lost: 185 bytes in 5 blocks Signed-off-by: Alex Jia <ajia(a)redhat.com> --- daemon/libvirtd-config.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/daemon/libvirtd-config.c b/daemon/libvirtd-config.c index 4d041f0..0a4323a 100644 --- a/daemon/libvirtd-config.c +++ b/daemon/libvirtd-config.c @@ -152,6 +152,7 @@ checkType (virConfValuePtr p, const char *filename, virReportOOMError(); \ goto error; \ } \ + VIR_FREE(data->var_name); \ } \ } while (0) -- 1.7.1

2 2

[libvirt] [test-API PATCH 0/5] Destroy class definition in utils.py
by Osier Yang 11 Apr '12

11 Apr '12

These five patches are to change utils.py into a collection of functions. And cleanup all of existing testcase to use the functions in it directly. (1) Destroy Utils class definition (2) Remove "util = utils.Utils()" (3) Substitue "util." with "utils." (4) Remove useless utils import "from utils import utils" in testcases where none of functions in utils.py is called In testcase from utils import utils ... utils.functions(...) ...

2 10

[libvirt] [PATCH 0/5] fix block job bugs relative to upstream qemu
by Eric Blake 11 Apr '12

11 Apr '12

The following 5 patches are ready for review and application to libvirt now, since they correspond to features that are currently in the qemu 1.1 tree. (I've split these less-controversial parts from v4 of my larger storage migration series [1]; so that this series has no dependency on the yet-to-be-determined 'drive-mirror' semantics). [1]https://www.redhat.com/archives/libvir-list/2012-April/msg00330.html Patch 1 is new, and matches a qemu patch taken just today. Patch 2 matches 2/18 in the block migration series, with some tweaks to defer any qemu changes till later, as well as wording improvements in the documentation to match the behavior implemented by this series. Patch 3 matches an independent patch I sent earlier today, updated to resolve review comments from that version. Patch 4 matches 3/18 in the block migration series, with modifications to now account for both RHEL 6.2 and qemu 1.1 behaviors. I've altered this so much from Adam's original that I claimed authorship, although I've left his Signed-off-by for the initial implementation. Patch 5 matches 4/18 in the block migration series, with minor tweaks due to rebasing. Later, I will repost a v5 of my storage migration series based on top of this one. Adam Litke (1): blockjob: add API for async virDomainBlockJobAbort Eric Blake (4): blockjob: add qemu capabilities related to block pull jobs blockjob: optimize JSON event handler lookup blockjob: wire up qemu async virDomainBlockJobAbort blockjob: allow for fast-finishing job include/libvirt/libvirt.h.in | 10 +++ src/libvirt.c | 13 +++- src/qemu/qemu_capabilities.c | 3 + src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_driver.c | 106 ++++++++++++++++++++----- src/qemu/qemu_monitor.c | 11 ++- src/qemu/qemu_monitor.h | 8 +- src/qemu/qemu_monitor_json.c | 179 +++++++++++++++++++++++++++--------------- src/qemu/qemu_monitor_json.h | 4 +- tools/virsh.c | 51 ++++++++----- tools/virsh.pod | 35 ++++++--- 11 files changed, 300 insertions(+), 122 deletions(-) -- 1.7.7.6

2 14

[libvirt] [PATCH] qemu: optimize JSON event handler lookup
by Eric Blake 11 Apr '12

11 Apr '12

Probably in the noise, but this will let us scale more efficiently as we recognize ever more qemu events. * src/qemu/qemu_monitor_json.c (eventHandlers): Sort. (eventSearch): New helper function. (qemuMonitorJSONIOProcessEvent): Optimize event lookup. --- In reply to: https://www.redhat.com/archives/libvir-list/2012-April/msg00416.html src/qemu/qemu_monitor_json.c | 55 +++++++++++++++++++++++------------------ 1 files changed, 31 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7c893d4..d142517 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -67,37 +67,45 @@ static void qemuMonitorJSONHandlePMSuspend(qemuMonitorPtr mon, virJSONValuePtr d static void qemuMonitorJSONHandleBlockJobCompleted(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleBlockJobCanceled(qemuMonitorPtr mon, virJSONValuePtr data); -static struct { +typedef struct { const char *type; void (*handler)(qemuMonitorPtr mon, virJSONValuePtr data); -} eventHandlers[] = { - { "SHUTDOWN", qemuMonitorJSONHandleShutdown, }, - { "RESET", qemuMonitorJSONHandleReset, }, +} qemuEventHandler; +static qemuEventHandler eventHandlers[] = { + { "BLOCK_IO_ERROR", qemuMonitorJSONHandleIOError, }, + { "BLOCK_JOB_CANCELLED", qemuMonitorJSONHandleBlockJobCanceled, }, + { "BLOCK_JOB_COMPLETED", qemuMonitorJSONHandleBlockJobCompleted, }, + { "DEVICE_TRAY_MOVED", qemuMonitorJSONHandleTrayChange, }, { "POWERDOWN", qemuMonitorJSONHandlePowerdown, }, - { "STOP", qemuMonitorJSONHandleStop, }, + { "RESET", qemuMonitorJSONHandleReset, }, { "RTC_CHANGE", qemuMonitorJSONHandleRTCChange, }, - { "WATCHDOG", qemuMonitorJSONHandleWatchdog, }, - { "BLOCK_IO_ERROR", qemuMonitorJSONHandleIOError, }, - { "VNC_CONNECTED", qemuMonitorJSONHandleVNCConnect, }, - { "VNC_INITIALIZED", qemuMonitorJSONHandleVNCInitialize, }, - { "VNC_DISCONNECTED", qemuMonitorJSONHandleVNCDisconnect, }, + { "SHUTDOWN", qemuMonitorJSONHandleShutdown, }, { "SPICE_CONNECTED", qemuMonitorJSONHandleSPICEConnect, }, - { "SPICE_INITIALIZED", qemuMonitorJSONHandleSPICEInitialize, }, { "SPICE_DISCONNECTED", qemuMonitorJSONHandleSPICEDisconnect, }, - { "DEVICE_TRAY_MOVED", qemuMonitorJSONHandleTrayChange, }, - { "WAKEUP", qemuMonitorJSONHandlePMWakeup, }, + { "SPICE_INITIALIZED", qemuMonitorJSONHandleSPICEInitialize, }, + { "STOP", qemuMonitorJSONHandleStop, }, { "SUSPEND", qemuMonitorJSONHandlePMSuspend, }, - { "BLOCK_JOB_COMPLETED", qemuMonitorJSONHandleBlockJobCompleted, }, - { "BLOCK_JOB_CANCELLED", qemuMonitorJSONHandleBlockJobCanceled, }, + { "VNC_CONNECTED", qemuMonitorJSONHandleVNCConnect, }, + { "VNC_DISCONNECTED", qemuMonitorJSONHandleVNCDisconnect, }, + { "VNC_INITIALIZED", qemuMonitorJSONHandleVNCInitialize, }, + { "WAKEUP", qemuMonitorJSONHandlePMWakeup, }, + { "WATCHDOG", qemuMonitorJSONHandleWatchdog, }, + /* We use bsearch, so keep this list sorted. */ }; +static int +eventSearch(const void *key, const void *elt) { + const char *type = key; + const qemuEventHandler *handler = elt; + return strcmp(type, handler->type); +} static int qemuMonitorJSONIOProcessEvent(qemuMonitorPtr mon, virJSONValuePtr obj) { const char *type; - int i; + qemuEventHandler *handler; VIR_DEBUG("mon=%p obj=%p", mon, obj); type = virJSONValueObjectGetString(obj, "event"); @@ -107,14 +115,13 @@ qemuMonitorJSONIOProcessEvent(qemuMonitorPtr mon, return -1; } - for (i = 0 ; i < ARRAY_CARDINALITY(eventHandlers) ; i++) { - if (STREQ(eventHandlers[i].type, type)) { - virJSONValuePtr data = virJSONValueObjectGet(obj, "data"); - VIR_DEBUG("handle %s handler=%p data=%p", type, - eventHandlers[i].handler, data); - (eventHandlers[i].handler)(mon, data); - break; - } + handler = bsearch(type, eventHandlers, ARRAY_CARDINALITY(eventHandlers), + sizeof(eventHandlers[0]), eventSearch); + if (handler) { + virJSONValuePtr data = virJSONValueObjectGet(obj, "data"); + VIR_DEBUG("handle %s handler=%p data=%p", type, + handler->handler, data); + (handler->handler)(mon, data); } return 0; } -- 1.7.7.6

2 2

[libvirt] [libvirt-glib] libvirt-gconfig: Move symbols introduced post 0.0.4 to the correct versions
by Guido Günther 11 Apr '12

11 Apr '12

Triggered by http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=667683 --- If this looks correct I'll have a look at libvirt-glib and libvirt-gobject too. -- Guido libvirt-gconfig/libvirt-gconfig.sym | 50 +++++++++++++++++++++++------------ 1 files changed, 33 insertions(+), 17 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index 717c3c9..405ad98 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -14,10 +14,6 @@ LIBVIRT_GCONFIG_0.0.4 { gvir_config_domain_new; gvir_config_domain_new_from_xml; gvir_config_domain_set_clock; - gvir_config_domain_set_custom_xml; - gvir_config_domain_get_custom_xml; - gvir_config_domain_get_description; - gvir_config_domain_set_description; gvir_config_domain_get_devices; gvir_config_domain_set_devices; gvir_config_domain_get_features; @@ -51,10 +47,6 @@ LIBVIRT_GCONFIG_0.0.4 { gvir_config_domain_chardev_source_pty_new_from_xml; gvir_config_domain_source_pty_set_path; - gvir_config_domain_chardev_source_spicevmc_get_type; - gvir_config_domain_chardev_source_spicevmc_new; - gvir_config_domain_chardev_source_spicevmc_new_from_xml; - gvir_config_domain_clock_get_type; gvir_config_domain_clock_offset_get_type; gvir_config_domain_clock_new; @@ -79,8 +71,6 @@ LIBVIRT_GCONFIG_0.0.4 { gvir_config_domain_disk_type_get_type; gvir_config_domain_disk_new; gvir_config_domain_disk_new_from_xml; - gvir_config_domain_disk_get_driver_cache; - gvir_config_domain_disk_set_driver_cache; gvir_config_domain_disk_get_driver_name; gvir_config_domain_disk_set_driver_name; gvir_config_domain_disk_get_driver_type; @@ -96,7 +86,6 @@ LIBVIRT_GCONFIG_0.0.4 { gvir_config_domain_disk_get_target_dev; gvir_config_domain_disk_set_target_dev; gvir_config_domain_disk_get_disk_type; - gvir_config_domain_disk_set_readonly; gvir_config_domain_disk_set_type; gvir_config_domain_filesys_get_type; @@ -146,13 +135,9 @@ LIBVIRT_GCONFIG_0.0.4 { gvir_config_domain_interface_get_type; gvir_config_domain_interface_link_state_get_type; gvir_config_domain_interface_set_ifname; - gvir_config_domain_interface_get_ifname; gvir_config_domain_interface_set_link_state; - gvir_config_domain_interface_get_link_state; gvir_config_domain_interface_set_mac; - gvir_config_domain_interface_get_mac; gvir_config_domain_interface_set_model; - gvir_config_domain_interface_get_model; gvir_config_domain_interface_bridge_get_type; gvir_config_domain_interface_bridge_new; @@ -327,8 +312,39 @@ LIBVIRT_GCONFIG_0.0.4 { gvir_config_xml_doc_get_type; gvir_config_xml_doc_new; - local: - *; }; +LIBVIRT_GCONFIG_0.0.5 { + global: + gvir_config_domain_disk_get_driver_cache; + gvir_config_domain_disk_set_driver_cache; + + gvir_config_domain_disk_set_readonly; + + gvir_config_domain_set_custom_xml; + gvir_config_domain_get_custom_xml; + gvir_config_domain_get_description; + gvir_config_domain_set_description; +} LIBVIRT_GCONFIG_0.0.4; + +LIBVIRT_GCONFIG_0.0.6 { + global: + gvir_config_domain_interface_get_ifname; + gvir_config_domain_interface_get_link_state; + gvir_config_domain_interface_get_mac; + gvir_config_domain_interface_get_model; +} LIBVIRT_GCONFIG_0.0.5; + +LIBVIRT_GCONFIG_0.0.7 { + global: + gvir_config_domain_chardev_source_spicevmc_get_type; + gvir_config_domain_chardev_source_spicevmc_new; + gvir_config_domain_chardev_source_spicevmc_new_from_xml; +} LIBVIRT_GCONFIG_0.0.6; + # .... define new API here using predicted next version number .... + +LIBVIRT_GCONFIG_PRIVATE_0.0.7 { + local: + *; +}; -- 1.7.9.1

3 9

[libvirt] USB redirection support
by Christophe Fergeau 11 Apr '12

11 Apr '12

Hey, This patch series adds the needed classes for apps to be able to use USB redirection. I've followed http://hansdegoede.livejournal.com/11084.html and added all the API needed to be able to get the same XML. However, I'm not 100% sure all these attributes are required, in particular the various addresses. Given that for now libvirt-gconfig users tend to reuse what test-domain-create does, it would be useful to know which parts are required and which are not to get working USB redirection. Thanks, Christophe

2 15

[libvirt] [PATCH] qemu: Fix deadlock when qemuDomainOpenConsole cleans up a connection
by Peter Krempa 11 Apr '12

11 Apr '12

The new safe console handling introduced a possibility to deadlock the qemu driver when a new console connection forcibly disconnects a previous console stream that belongs to an already closed connection. The virStreamFree function calls subsequently a the virReleaseConnect function that tries to lock the driver while discarding the connection, but the driver was already locked in qemuDomainOpenConsole. Backtrace of the deadlocked thread: 0 0x00007f66e5aa7f14 in __lll_lock_wait () from /lib64/libpthread.so.0 1 0x00007f66e5aa3411 in _L_lock_500 () from /lib64/libpthread.so.0 2 0x00007f66e5aa322a in pthread_mutex_lock () from/lib64/libpthread.so.0 3 0x0000000000462bbd in qemudClose () 4 0x00007f66e6e178eb in virReleaseConnect () from/usr/lib64/libvirt.so.0 5 0x00007f66e6e19c8c in virUnrefStream () from /usr/lib64/libvirt.so.0 6 0x00007f66e6e3d1de in virStreamFree () from /usr/lib64/libvirt.so.0 7 0x00007f66e6e09a5d in virConsoleHashEntryFree () from/usr/lib64/libvirt.so.0 8 0x00007f66e6db7282 in virHashRemoveEntry () from/usr/lib64/libvirt.so.0 9 0x00007f66e6e09c4e in virConsoleOpen () from /usr/lib64/libvirt.so.0 10 0x00000000004526e9 in qemuDomainOpenConsole () 11 0x00007f66e6e421f1 in virDomainOpenConsole () from/usr/lib64/libvirt.so.0 12 0x00000000004361e4 in remoteDispatchDomainOpenConsoleHelper () 13 0x00007f66e6e80375 in virNetServerProgramDispatch () from/usr/lib64/libvirt.so.0 14 0x00007f66e6e7ae11 in virNetServerHandleJob () from/usr/lib64/libvirt.so.0 15 0x00007f66e6da897d in virThreadPoolWorker () from/usr/lib64/libvirt.so.0 16 0x00007f66e6da7ff6 in virThreadHelper () from/usr/lib64/libvirt.so.0 17 0x00007f66e5aa0c5c in start_thread () from /lib64/libpthread.so.0 18 0x00007f66e57e7fcd in clone () from /lib64/libc.so.6 * src/qemu/qemu_driver.c: qemuDomainOpenConsole() -- unlock the qemu driver right after acquiring the domain object --- Found while writing tests for the libvirt-test-API, its 100% reproducible: 1: Start a domain with serial console and run "cat" on this console 2: download python reproducer: http://files.pipo.sk/console_deadlock.py 3: set guest name in the script 4: run the reproducer and "virsh console 'guestname' --force" after that 5: <deadlocked> src/qemu/qemu_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d9e35be..e18e72d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11494,6 +11494,7 @@ qemuDomainOpenConsole(virDomainPtr dom, qemuDriverLock(driver); virUUIDFormat(dom->uuid, uuidstr); vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); if (!vm) { qemuReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); @@ -11558,7 +11559,6 @@ qemuDomainOpenConsole(virDomainPtr dom, cleanup: if (vm) virDomainObjUnlock(vm); - qemuDriverUnlock(driver); return ret; } -- 1.7.3.4

2 2

[libvirt] Etsy uses Libvirt... :)
by Justin Clift 10 Apr '12

10 Apr '12

Hey everyone, Just noticed that Etsy uses libvirt: http://codeascraft.etsy.com/2012/03/13/making-it-virtually-easy-to-deploy-o… Of specific note, there's this bit: "Libvirt supports live migrations across non-shared storage (in QEMU 0.12.2+) with zero downtime which makes it easy to allocate and balance VM’s across hosts if adjustments need to be made throughout the pool." Seems like a win, eh? :) Regards and best wishes, Justin Clift -- Aeolus Community Manager http://www.aeolusproject.org

2 3

[libvirt] [PATCH 1/2] qemu: Track job owner for better debugging
by Jiri Denemark 10 Apr '12

10 Apr '12

In case an API fails with "cannot acquire state change lock", searching for the API that possibly forgot to end its job is not always easy. Let's keep track of the job owner and print it out for easier identification. --- src/qemu/qemu_domain.c | 53 +++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 4 +++ src/qemu/qemu_migration.c | 1 + src/qemu/qemu_process.c | 6 +--- 4 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index da1f57f..7f1f8ee 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -147,6 +147,7 @@ qemuDomainObjResetJob(qemuDomainObjPrivatePtr priv) struct qemuDomainJobObj *job = &priv->job; job->active = QEMU_JOB_NONE; + job->owner = 0; } static void @@ -155,6 +156,7 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv) struct qemuDomainJobObj *job = &priv->job; job->asyncJob = QEMU_ASYNC_JOB_NONE; + job->asyncOwner = 0; job->phase = 0; job->mask = DEFAULT_JOB_MASK; job->start = 0; @@ -169,13 +171,25 @@ qemuDomainObjRestoreJob(virDomainObjPtr obj, memset(job, 0, sizeof(*job)); job->active = priv->job.active; + job->owner = priv->job.owner; job->asyncJob = priv->job.asyncJob; + job->asyncOwner = priv->job.asyncOwner; job->phase = priv->job.phase; qemuDomainObjResetJob(priv); qemuDomainObjResetAsyncJob(priv); } +void +qemuDomainObjTransferJob(virDomainObjPtr obj) +{ + qemuDomainObjPrivatePtr priv = obj->privateData; + + VIR_DEBUG("Changing job owner from %d to %d", + priv->job.owner, virThreadSelfID()); + priv->job.owner = virThreadSelfID(); +} + static void qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv) { @@ -664,11 +678,23 @@ qemuDomainObjSetJobPhase(struct qemud_driver *driver, int phase) { qemuDomainObjPrivatePtr priv = obj->privateData; + int me = virThreadSelfID(); if (!priv->job.asyncJob) return; + VIR_DEBUG("Setting '%s' phase to '%s'", + qemuDomainAsyncJobTypeToString(priv->job.asyncJob), + qemuDomainAsyncJobPhaseToString(priv->job.asyncJob, phase)); + + if (priv->job.asyncOwner && me != priv->job.asyncOwner) { + VIR_WARN("'%s' async job is owned by thread %d", + qemuDomainAsyncJobTypeToString(priv->job.asyncJob), + priv->job.asyncOwner); + } + priv->job.phase = phase; + priv->job.asyncOwner = me; qemuDomainObjSaveJob(driver, obj); } @@ -695,6 +721,22 @@ qemuDomainObjDiscardAsyncJob(struct qemud_driver *driver, virDomainObjPtr obj) qemuDomainObjSaveJob(driver, obj); } +void +qemuDomainObjReleaseAsyncJob(virDomainObjPtr obj) +{ + qemuDomainObjPrivatePtr priv = obj->privateData; + + VIR_DEBUG("Releasing ownership of '%s' async job", + qemuDomainAsyncJobTypeToString(priv->job.asyncJob)); + + if (priv->job.asyncOwner != virThreadSelfID()) { + VIR_WARN("'%s' async job is owned by thread %d", + qemuDomainAsyncJobTypeToString(priv->job.asyncJob), + priv->job.asyncOwner); + } + priv->job.asyncOwner = 0; +} + static bool qemuDomainNestedJobAllowed(qemuDomainObjPrivatePtr priv, enum qemuDomainJob job) { @@ -764,11 +806,13 @@ retry: qemuDomainJobTypeToString(job), qemuDomainAsyncJobTypeToString(priv->job.asyncJob)); priv->job.active = job; + priv->job.owner = virThreadSelfID(); } else { VIR_DEBUG("Starting async job: %s", qemuDomainAsyncJobTypeToString(asyncJob)); qemuDomainObjResetAsyncJob(priv); priv->job.asyncJob = asyncJob; + priv->job.asyncOwner = virThreadSelfID(); priv->job.start = now; } @@ -784,6 +828,15 @@ retry: return 0; error: + VIR_WARN("Cannot start job (%s, %s) for domain %s;" + " current job is (%s, %s) owned by (%d, %d)", + qemuDomainJobTypeToString(job), + qemuDomainAsyncJobTypeToString(asyncJob), + obj->def->name, + qemuDomainJobTypeToString(priv->job.active), + qemuDomainAsyncJobTypeToString(priv->job.asyncJob), + priv->job.owner, priv->job.asyncOwner); + if (errno == ETIMEDOUT) qemuReportError(VIR_ERR_OPERATION_TIMEOUT, "%s", _("cannot acquire state change lock")); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index b3eecd3..ce52569 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -96,9 +96,11 @@ VIR_ENUM_DECL(qemuDomainAsyncJob) struct qemuDomainJobObj { virCond cond; /* Use to coordinate jobs */ enum qemuDomainJob active; /* Currently running job */ + int owner; /* Thread which set current job */ virCond asyncCond; /* Use to coordinate with async jobs */ enum qemuDomainAsyncJob asyncJob; /* Currently active async job */ + int asyncOwner; /* Thread which set current async job */ int phase; /* Job phase (mainly for migrations) */ unsigned long long mask; /* Jobs allowed during async job */ unsigned long long start; /* When the async job started */ @@ -203,8 +205,10 @@ void qemuDomainObjSetAsyncJobMask(virDomainObjPtr obj, unsigned long long allowedJobs); void qemuDomainObjRestoreJob(virDomainObjPtr obj, struct qemuDomainJobObj *job); +void qemuDomainObjTransferJob(virDomainObjPtr obj); void qemuDomainObjDiscardAsyncJob(struct qemud_driver *driver, virDomainObjPtr obj); +void qemuDomainObjReleaseAsyncJob(virDomainObjPtr obj); void qemuDomainObjEnterMonitor(struct qemud_driver *driver, virDomainObjPtr obj) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index ff6f273..c1bb93a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3242,6 +3242,7 @@ qemuMigrationJobStartPhase(struct qemud_driver *driver, int qemuMigrationJobContinue(virDomainObjPtr vm) { + qemuDomainObjReleaseAsyncJob(vm); return virDomainObjUnref(vm); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 96f39e8..19569cf 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3038,8 +3038,8 @@ qemuProcessReconnect(void *opaque) priv = obj->privateData; - /* Set fake job so that EnterMonitor* doesn't want to start a new one */ - priv->job.active = QEMU_JOB_MODIFY; + /* Job was started by the caller for us */ + qemuDomainObjTransferJob(obj); /* Hold an extra reference because we can't allow 'vm' to be * deleted if qemuConnectMonitor() failed */ @@ -3119,8 +3119,6 @@ qemuProcessReconnect(void *opaque) if (qemuProcessRecoverJob(driver, obj, conn, &oldjob) < 0) goto error; - priv->job.active = QEMU_JOB_NONE; - /* update domain state XML with possibly updated state in virDomainObj */ if (virDomainSaveStatus(driver->caps, driver->stateDir, obj) < 0) goto error; -- 1.7.8.5

2 2

[libvirt] [PATCH] qemu: Avoid excessive calls to qemuDomainObjSaveJob()
by Jiri Denemark 10 Apr '12

10 Apr '12

As reported by Daniel Berrangé, we have a huge performance regression for virDomainGetInfo() due to the change which makes virDomainEndJob() save the XML status file every time it is called. Previous to that change, 2000 calls to virDomainGetInfo() took ~2.5 seconds. After that change, 2000 calls to virDomainGetInfo() take 2 *minutes* 45 secs. We made the change to be able to recover from libvirtd restart in the middle of a job. However, only destroy and async jobs are taken care of. Thus it makes more sense to only save domain state XML when these jobs are started/stopped. --- src/qemu/qemu_domain.c | 21 ++++++++++++++++++--- src/qemu/qemu_domain.h | 5 +++++ src/qemu/qemu_process.c | 4 ++++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 69d9e6e..da1f57f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -183,6 +183,12 @@ qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv) ignore_value(virCondDestroy(&priv->job.asyncCond)); } +static bool +qemuDomainTrackJob(enum qemuDomainJob job) +{ + return (QEMU_DOMAIN_TRACK_JOBS & JOB_MASK(job)) != 0; +} + static void *qemuDomainObjPrivateAlloc(void) { @@ -239,6 +245,7 @@ static int qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) { qemuDomainObjPrivatePtr priv = data; const char *monitorpath; + enum qemuDomainJob job; /* priv->monitor_chr is set only for qemu */ if (priv->monConfig) { @@ -284,6 +291,10 @@ static int qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) if (priv->lockState) virBufferAsprintf(buf, " <lockstate>%s</lockstate>\n", priv->lockState); + job = priv->job.active; + if (!qemuDomainTrackJob(job)) + priv->job.active = QEMU_JOB_NONE; + if (priv->job.active || priv->job.asyncJob) { virBufferAsprintf(buf, " <job type='%s' async='%s'", qemuDomainJobTypeToString(priv->job.active), @@ -295,6 +306,7 @@ static int qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) } virBufferAddLit(buf, "/>\n"); } + priv->job.active = job; if (priv->fakeReboot) virBufferAsprintf(buf, " <fakereboot/>\n"); @@ -766,7 +778,8 @@ retry: virDomainObjLock(obj); } - qemuDomainObjSaveJob(driver, obj); + if (qemuDomainTrackJob(job)) + qemuDomainObjSaveJob(driver, obj); return 0; @@ -862,15 +875,17 @@ int qemuDomainObjBeginAsyncJobWithDriver(struct qemud_driver *driver, int qemuDomainObjEndJob(struct qemud_driver *driver, virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; + enum qemuDomainJob job = priv->job.active; priv->jobs_queued--; VIR_DEBUG("Stopping job: %s (async=%s)", - qemuDomainJobTypeToString(priv->job.active), + qemuDomainJobTypeToString(job), qemuDomainAsyncJobTypeToString(priv->job.asyncJob)); qemuDomainObjResetJob(priv); - qemuDomainObjSaveJob(driver, obj); + if (qemuDomainTrackJob(job)) + qemuDomainObjSaveJob(driver, obj); virCondSignal(&priv->job.cond); return virDomainObjUnref(obj); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index adccfed..b3eecd3 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -53,6 +53,11 @@ JOB_MASK(QEMU_JOB_DESTROY) | \ JOB_MASK(QEMU_JOB_ABORT)) +/* Jobs which have to be tracked in domain state XML. */ +# define QEMU_DOMAIN_TRACK_JOBS \ + (JOB_MASK(QEMU_JOB_DESTROY) | \ + JOB_MASK(QEMU_JOB_ASYNC)) + /* Only 1 job is allowed at any time * A job includes *all* monitor commands, even those just querying * information, not merely actions */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 481b4f3..96f39e8 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2963,6 +2963,10 @@ qemuProcessRecoverJob(struct qemud_driver *driver, if (!virDomainObjIsActive(vm)) return -1; + /* In case any special handling is added for job type that has been ignored + * before, QEMU_DOMAIN_TRACK_JOBS (from qemu_domain.h) needs to be updated + * for the job to be properly tracked in domain state XML. + */ switch (job->active) { case QEMU_JOB_QUERY: /* harmless */ -- 1.7.8.5

2 1