- Devel - libvirt List Archives

[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

[libvirt] [PATCH] build: avoid s390 compiler warnings
by Eric Blake 10 Apr '12

10 Apr '12

I noticed compiler warnings when building for the s390 architecture. * src/node_device/node_device_udev.c (udevDeviceMonitorStartup): Mark unused variable. * src/nodeinfo.c (linuxNodeInfoCPUPopulate): Avoid unused variable. --- Pushing under the trivial rule. src/node_device/node_device_udev.c | 4 ++-- src/nodeinfo.c | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 438c2eb..6418015 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1,7 +1,7 @@ /* * node_device_udev.c: node device enumeration - libudev implementation * - * Copyright (C) 2009-2011 Red Hat, Inc. + * Copyright (C) 2009-2012 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -1604,7 +1604,7 @@ out: return ret; } -static int udevDeviceMonitorStartup(int privileged) +static int udevDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) { udevPrivate *priv = NULL; struct udev *udev = NULL; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 26f8dc1..7e993a9 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -231,10 +231,10 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, /* NOTE: hyperthreads are ignored here; they are parsed out of /sys */ while (fgets(line, sizeof(line), cpuinfo) != NULL) { - char *buf = line; # if defined(__x86_64__) || \ defined(__amd64__) || \ defined(__i386__) + char *buf = line; if (STRPREFIX(buf, "cpu MHz")) { char *p; unsigned int ui; @@ -253,6 +253,7 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, } # elif defined(__powerpc__) || \ defined(__powerpc64__) + char *buf = line; if (STRPREFIX(buf, "clock")) { char *p; unsigned int ui; -- 1.7.7.6

1 0

[libvirt] [PATCH] daemon: Add libvirtd-config.c to the list of files to translate
by Jiri Denemark 10 Apr '12

10 Apr '12

--- po/POTFILES.in | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index 5d5739a..4c49200 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -1,3 +1,4 @@ +daemon/libvirtd-config.c daemon/libvirtd.c daemon/qemu_dispatch.h daemon/remote.c -- 1.7.8.5

2 1

[libvirt] [PATCH] virsh: Clean up usage of boolean flag variables
by Peter Krempa 10 Apr '12

10 Apr '12

This patch cleans up variables used to store boolean command flags that are inquired by vshCommandOptBool to use the bool data type instead of an integer. Additionaly this patch cleans up flag variables that are inferred from existing flags. --- tools/virsh.c | 120 ++++++++++++++++++++++++++++---------------------------- 1 files changed, 60 insertions(+), 60 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index cfdd040..1a2e191 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -933,9 +933,9 @@ static const vshCmdOptDef opts_list[] = { static bool cmdList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { - int inactive = vshCommandOptBool(cmd, "inactive"); - int all = vshCommandOptBool(cmd, "all"); - int active = !inactive || all ? 1 : 0; + bool inactive = vshCommandOptBool(cmd, "inactive"); + bool all = vshCommandOptBool(cmd, "all"); + bool active = !inactive || all; int *ids = NULL, maxid = 0, i; char **names = NULL; int maxname = 0; @@ -1317,7 +1317,7 @@ cmdDomstate(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; bool ret = true; - int showReason = vshCommandOptBool(cmd, "reason"); + bool showReason = vshCommandOptBool(cmd, "reason"); int state, reason; if (!vshConnectionUsability(ctl, ctl->conn)) @@ -1966,9 +1966,9 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd) int nparams = 0; virTypedParameterPtr params = NULL; bool ret = false; - int current = vshCommandOptBool(cmd, "current"); - int config = vshCommandOptBool(cmd, "config"); - int live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); virNetDevBandwidthRate inbound, outbound; int i; @@ -2626,7 +2626,7 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd) bool ret = true; char *buffer; #ifndef WIN32 - int console = vshCommandOptBool(cmd, "console"); + bool console = vshCommandOptBool(cmd, "console"); #endif unsigned int flags = VIR_DOMAIN_NONE; @@ -3094,7 +3094,7 @@ cmdStart(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; bool ret = false; #ifndef WIN32 - int console = vshCommandOptBool(cmd, "console"); + bool console = vshCommandOptBool(cmd, "console"); #endif unsigned int flags = VIR_DOMAIN_NONE; int rc; @@ -3786,9 +3786,9 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) int i, ret; bool ret_val = false; unsigned int flags = 0; - int current = vshCommandOptBool(cmd, "current"); - int config = vshCommandOptBool(cmd, "config"); - int live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); if (current) { if (live || config) { @@ -4921,11 +4921,11 @@ cmdVcpucount(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; bool ret = true; - int maximum = vshCommandOptBool(cmd, "maximum"); - int active = vshCommandOptBool(cmd, "active"); - int config = vshCommandOptBool(cmd, "config"); - int live = vshCommandOptBool(cmd, "live"); - int current = vshCommandOptBool(cmd, "current"); + bool maximum = vshCommandOptBool(cmd, "maximum"); + bool active = vshCommandOptBool(cmd, "active"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); bool all = maximum + active + current + config + live == 0; int count; @@ -5246,9 +5246,9 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) int i, cpu, lastcpu, maxcpu, ncpus; bool unuse = false; const char *cur; - int config = vshCommandOptBool(cmd, "config"); - int live = vshCommandOptBool(cmd, "live"); - int current = vshCommandOptBool(cmd, "current"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); bool query = false; /* Query mode if no cpulist */ unsigned int flags = 0; @@ -5479,10 +5479,10 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; int count = 0; bool ret = true; - int maximum = vshCommandOptBool(cmd, "maximum"); - int config = vshCommandOptBool(cmd, "config"); - int live = vshCommandOptBool(cmd, "live"); - int current = vshCommandOptBool(cmd, "current"); + bool maximum = vshCommandOptBool(cmd, "maximum"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); unsigned int flags = 0; if (current) { @@ -5853,9 +5853,9 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd) unsigned long long max; unsigned long kibibytes = 0; bool ret = true; - int config = vshCommandOptBool(cmd, "config"); - int live = vshCommandOptBool(cmd, "live"); - int current = vshCommandOptBool(cmd, "current"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); unsigned int flags = 0; if (current) { @@ -5935,9 +5935,9 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd) unsigned long long max; unsigned long kibibytes = 0; bool ret = true; - int config = vshCommandOptBool(cmd, "config"); - int live = vshCommandOptBool(cmd, "live"); - int current = vshCommandOptBool(cmd, "current"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); unsigned int flags = VIR_DOMAIN_MEM_MAXIMUM; if (current) { @@ -6026,9 +6026,9 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) virTypedParameterPtr params = NULL, temp = NULL; bool ret = false; unsigned int flags = 0; - int current = vshCommandOptBool(cmd, "current"); - int config = vshCommandOptBool(cmd, "config"); - int live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); if (current) { if (live || config) { @@ -6201,9 +6201,9 @@ cmdMemtune(vshControl *ctl, const vshCmd *cmd) virTypedParameterPtr params = NULL, temp = NULL; bool ret = false; unsigned int flags = 0; - int current = vshCommandOptBool(cmd, "current"); - int config = vshCommandOptBool(cmd, "config"); - int live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); if (current) { if (live || config) { @@ -6365,9 +6365,9 @@ cmdNumatune(vshControl * ctl, const vshCmd * cmd) const char *nodeset = NULL; bool ret = false; unsigned int flags = 0; - int current = vshCommandOptBool(cmd, "current"); - int config = vshCommandOptBool(cmd, "config"); - int live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); const char *mode = NULL; if (current) { @@ -6831,9 +6831,9 @@ cmdDumpXML(vshControl *ctl, const vshCmd *cmd) bool ret = true; char *dump; unsigned int flags = 0; - int inactive = vshCommandOptBool(cmd, "inactive"); - int secure = vshCommandOptBool(cmd, "security-info"); - int update = vshCommandOptBool(cmd, "update-cpu"); + bool inactive = vshCommandOptBool(cmd, "inactive"); + bool secure = vshCommandOptBool(cmd, "security-info"); + bool update = vshCommandOptBool(cmd, "update-cpu"); if (inactive) flags |= VIR_DOMAIN_XML_INACTIVE; @@ -7800,9 +7800,9 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) virTypedParameterPtr params = NULL, temp = NULL; unsigned int flags = 0, i = 0; int rv = 0; - int current = vshCommandOptBool(cmd, "current"); - int config = vshCommandOptBool(cmd, "config"); - int live = vshCommandOptBool(cmd, "live"); + bool current = vshCommandOptBool(cmd, "current"); + bool config = vshCommandOptBool(cmd, "config"); + bool live = vshCommandOptBool(cmd, "live"); bool ret = false; if (current) { @@ -8336,9 +8336,9 @@ static const vshCmdOptDef opts_network_list[] = { static bool cmdNetworkList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { - int inactive = vshCommandOptBool(cmd, "inactive"); - int all = vshCommandOptBool(cmd, "all"); - int active = !inactive || all ? 1 : 0; + bool inactive = vshCommandOptBool(cmd, "inactive"); + bool all = vshCommandOptBool(cmd, "all"); + bool active = !inactive || all; int maxactive = 0, maxinactive = 0, i; char **activeNames = NULL, **inactiveNames = NULL; inactive |= all; @@ -8609,9 +8609,9 @@ static const vshCmdOptDef opts_interface_list[] = { static bool cmdInterfaceList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { - int inactive = vshCommandOptBool(cmd, "inactive"); - int all = vshCommandOptBool(cmd, "all"); - int active = !inactive || all ? 1 : 0; + bool inactive = vshCommandOptBool(cmd, "inactive"); + bool all = vshCommandOptBool(cmd, "all"); + bool active = !inactive || all; int maxactive = 0, maxinactive = 0, i; char **activeNames = NULL, **inactiveNames = NULL; inactive |= all; @@ -8786,7 +8786,7 @@ cmdInterfaceDumpXML(vshControl *ctl, const vshCmd *cmd) bool ret = true; char *dump; unsigned int flags = 0; - int inactive = vshCommandOptBool(cmd, "inactive"); + bool inactive = vshCommandOptBool(cmd, "inactive"); if (inactive) flags |= VIR_INTERFACE_XML_INACTIVE; @@ -10038,7 +10038,7 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd) virStoragePoolPtr pool; const char *name; char *xml; - int printXML = vshCommandOptBool(cmd, "print-xml"); + bool printXML = vshCommandOptBool(cmd, "print-xml"); if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -10126,7 +10126,7 @@ cmdPoolDefineAs(vshControl *ctl, const vshCmd *cmd) virStoragePoolPtr pool; const char *name; char *xml; - int printXML = vshCommandOptBool(cmd, "print-xml"); + bool printXML = vshCommandOptBool(cmd, "print-xml"); if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -10400,10 +10400,10 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) struct poolInfoText *poolInfoTexts = NULL; /* Determine the options passed by the user */ - int all = vshCommandOptBool(cmd, "all"); - int details = vshCommandOptBool(cmd, "details"); - int inactive = vshCommandOptBool(cmd, "inactive"); - int active = !inactive || all ? 1 : 0; + bool all = vshCommandOptBool(cmd, "all"); + bool details = vshCommandOptBool(cmd, "details"); + bool inactive = vshCommandOptBool(cmd, "inactive"); + bool active = !inactive || all; inactive |= all; /* Check the connection to libvirtd daemon is still working */ @@ -12062,7 +12062,7 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) char *outputStr = NULL; const char *unit; double val; - int details = vshCommandOptBool(cmd, "details"); + bool details = vshCommandOptBool(cmd, "details"); int numVolumes = 0, i; int ret; bool functionReturn; @@ -13006,7 +13006,7 @@ cmdNodeListDevices (vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) const char *cap = NULL; char **devices; int num_devices, i; - int tree = vshCommandOptBool(cmd, "tree"); + bool tree = vshCommandOptBool(cmd, "tree"); if (!vshConnectionUsability(ctl, ctl->conn)) return false; -- 1.7.3.4

2 2

[libvirt] [PATCH] virsh: Clarify use of the --managed-save flag for the list command
by Peter Krempa 10 Apr '12

10 Apr '12

The documentation for the flag doesn't clearly state that the flag only enhances the output and the user needs to specify other flags to list inactive domains, that are enhanced by this flag. --- tools/virsh.c | 2 +- tools/virsh.pod | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index cfdd040..7477d32 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -924,7 +924,7 @@ static const vshCmdOptDef opts_list[] = { {"name", VSH_OT_BOOL, 0, N_("list domain names only")}, {"table", VSH_OT_BOOL, 0, N_("list table (default)")}, {"managed-save", VSH_OT_BOOL, 0, - N_("mark domains with managed save state")}, + N_("mark inactive domains with managed save state")}, {"title", VSH_OT_BOOL, 0, N_("show short domain description")}, {NULL, 0, 0, NULL} }; diff --git a/tools/virsh.pod b/tools/virsh.pod index a60e667..c6e0ff3 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -381,8 +381,9 @@ into s3 state. =back If I<--managed-save> is specified, then domains that have managed save -state (only possible if they are in the B<shut off> state) will -instead show as B<saved> in the listing. This flag is usable only with the +state (only possible if they are in the B<shut off> state, so you need to +specify I<--inactive> or I<--all> to actually list them) will instead +show as B<saved> in the listing. This flag is usable only with the default I<--table> output. If I<--name> is specified, domain names are printed instead of the table -- 1.7.3.4

2 2

[libvirt] [RESEND] libvirt: xen: Shutdown detection fixes (RH-BZ#746007)
by Stefan Bader 10 Apr '12

10 Apr '12

Resending this as there was no feedback overall and at least the first change fixes the following bug: https://bugzilla.redhat.com/show_bug.cgi?id=746007 --- The issue is easy to observe when using virt-manager as a frontend. Its main window seems to open connections for the up/down main display once and then only repeats DomainGetInfo on the same connection to check whether the instance is up or shut down. The initial open on the connection sets the domain id to -1 for inactive managed domains. And once running, the hypervisor driver returns the status. But after shutdown, the domain cannot be found by the hypervisor driver and the domain info is obtained by the xend driver. However, that expects the domain id to be -1 for inactive domains. This is not true (anymore). In testing I found that domain/status seems to be always present and better suited to check whether a domain is running or not (0 for shutdown and 2 for running, there are probably other values, too). Beside the change, which does fix this problem, the question is whether there should be a mechanism that resets domain->id to -1 when the instance is found to be shut down. And if that should happen, should it be done by any sub-driver or by the unified driver. The second problem I observed when I shut down an instance from the vnc console provided by virt-manager. As soon as the instance was down, the log produced internal errors about every second. Found this again, to be related to the unified driver approach. The call that causes the many error messages is GetVcpus. While the instance is up, the hypervisor driver returns those. But after shutdown, the domain cannot be found by that driver. The xend driver works, though. So what happens is that the hypervisor driver logs an internal error but the call succeeds because the xend driver returns successful. This generally opens the question whether a sub-driver should log any errors (at least not on error level, maybe debug)? The second patch only quiets down the one message I saw repeated that often. There are some other errors logged without any obvious problems interacting with the instances, but those are by far less often. -Stefan Please cc: me on replies as I am (yeah still) not subscribed to this m-l. Thanks.

2 10

[libvirt] [PATCH] tests: Fix libvirtdconftest in VPATH build
by Jiri Denemark 10 Apr '12

10 Apr '12

Without this, libvirtdconftest fails to build with "fatal error: daemon/libvirtd-config.h: No such file or directory" --- tests/Makefile.am | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 57358e9..c4d550f 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -6,6 +6,7 @@ SHELL = $(PREFERABLY_POSIX_SHELL) INCLUDES = \ + -I$(top_builddir) -I$(top_srcdir) \ -I$(top_builddir)/gnulib/lib -I$(top_srcdir)/gnulib/lib \ -I$(top_builddir)/include -I$(top_srcdir)/include \ -I$(top_builddir)/src -I$(top_srcdir)/src \ -- 1.7.8.5

2 1

[libvirt] libvirt starts KVM domain without -enable-kvm
by Reinier Schoof 10 Apr '12

10 Apr '12

Hey, we're running a VM-pool based on libvirt and QEMU/KVM. The host machines run debian 6 (squeeze). All VM's have a similar libvirt XML-definition. Since we've moved to the combination of Linux kernel 3.2.0-1 and libvirt 0.9.9 (installed from testing/wheezy repo's), we periodically notice poor performance of new(ly started) VM's and after inspection they turn out to be started without the qemu option -enable-kvm. In /var/log/libvirt/qemu/$VM-name.log a line like this is shown: 20529: error : qemuBuildCommandLine:3681 : unsupported configuration: the QEMU binary /usr/bin/kvm does not support kvm After a virsh destroy/start, using the same XML-definition, the VM is started WITH -enable-kvm. Looking at the libvirt source of 0.9.9, I can't imagine this being the result of a race condition or other strange circumstance: it simply parse 'qemu -help', which always give exactly the same output. I've tried reproducing the problem by continuously destroy, undefine, define and start the same VM and then grepping the kvm-commandline for the absence of -enable-kvm, without any result. The XML-definition includes <domain type='kvm'> and <emulator>/usr/bin/kvm</emulator> Does anyone recognize this behaviour of libvirt? Thanks, Reinier Schoof # virsh version Compiled against library: libvir 0.9.9 Using library: libvir 0.9.9 Using API: QEMU 0.9.9 Running hypervisor: QEMU 0.15.0 Linux 3.2.0-1-amd64 #1 SMP Fri Feb 17 05:17:36 UTC 2012 x86_64 GNU/Linux -- TransIP BV | https://www.transip.nl/

1 0

[libvirt] [PATCH] Fix comment about GNUTLS initialization/cleanup
by Daniel P. Berrange 10 Apr '12

10 Apr '12

From: "Daniel P. Berrange" <berrange(a)redhat.com> --- src/rpc/virnettlscontext.c | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 74a61e0..7440c7a 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -1423,9 +1423,13 @@ void virNetTLSSessionFree(virNetTLSSessionPtr sess) * virNetTLS* because it initializes * underlying GnuTLS library. According to * it's documentation, it's safe to be called - * many times, but is not thread safe. Each - * call SHOULD be later followed by - * virNetTLSContextDeinit. + * many times, but is not thread safe. + * + * There is no corresponding "Deinit" / "Cleanup" + * function because there is no safe way to call + * 'gnutls_global_deinit' from a multi-threaded + * library, where other libraries linked into the + * application may also be using gnutls. */ void virNetTLSInit(void) { -- 1.7.7.6

2 1