- Devel - libvirt List Archives

[libvirt] [PATCH] conf: avoid NULL deref for pmsuspended domain state
by Eric Blake 26 Jan '13

26 Jan '13

While working with a pmsuspend vs. snapshot issue, I noticed that the state file in /var/run/libvirt/qemu/dom.xml contained a rather suspicious "(null)" string, which does not round-trip well through a libvirtd restart. Had I been on a platform other than glibc where printf("%s",NULL) crashes instead of printing (null), we might have noticed the problem much sooner. And in fixing that problem, I also noticed that we had several missing states, because we were #defining several *_LAST names to a value _different_ than what they were already given as enums in libvirt.h. Yuck. I got rid of default: labels in the case statements, because they get in the way of gcc's -Wswitch helping us ensure we cover all enum values. * src/conf/domain_conf.c (virDomainStateReasonToString) (virDomainStateReasonFromString): Fill in missing domain states; rewrite case statement to let compiler enforce checking. (VIR_DOMAIN_NOSTATE_LAST, VIR_DOMAIN_RUNNING_LAST) (VIR_DOMAIN_BLOCKED_LAST, VIR_DOMAIN_PAUSED_LAST) (VIR_DOMAIN_SHUTDOWN_LAST, VIR_DOMAIN_SHUTOFF_LAST) (VIR_DOMAIN_CRASHED_LAST): Drop dead defines. (VIR_DOMAIN_PMSUSPENDED_LAST): Drop dead define. (virDomainPMSuspendedReason): Add missing enum function. (virDomainRunningReason, virDomainPausedReason): Add missing enum value. * src/conf/domain_conf.h (virDomainPMSuspendedReason): Declare missing functions. * src/libvirt_private.syms (domain_conf.h): Export them. --- src/conf/domain_conf.c | 32 +++++++++++++++++++------------- src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 6 ++++-- 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7273790..5782abb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -574,11 +574,9 @@ VIR_ENUM_IMPL(virDomainState, VIR_DOMAIN_LAST, "crashed", "pmsuspended") -#define VIR_DOMAIN_NOSTATE_LAST (VIR_DOMAIN_NOSTATE_UNKNOWN + 1) VIR_ENUM_IMPL(virDomainNostateReason, VIR_DOMAIN_NOSTATE_LAST, "unknown") -#define VIR_DOMAIN_RUNNING_LAST (VIR_DOMAIN_RUNNING_SAVE_CANCELED + 1) VIR_ENUM_IMPL(virDomainRunningReason, VIR_DOMAIN_RUNNING_LAST, "unknown", "booted", @@ -587,13 +585,12 @@ VIR_ENUM_IMPL(virDomainRunningReason, VIR_DOMAIN_RUNNING_LAST, "from snapshot", "unpaused", "migration canceled", - "save canceled") + "save canceled", + "wakeup") -#define VIR_DOMAIN_BLOCKED_LAST (VIR_DOMAIN_BLOCKED_UNKNOWN + 1) VIR_ENUM_IMPL(virDomainBlockedReason, VIR_DOMAIN_BLOCKED_LAST, "unknown") -#define VIR_DOMAIN_PAUSED_LAST (VIR_DOMAIN_PAUSED_SHUTTING_DOWN + 1) VIR_ENUM_IMPL(virDomainPausedReason, VIR_DOMAIN_PAUSED_LAST, "unknown", "user", @@ -603,14 +600,13 @@ VIR_ENUM_IMPL(virDomainPausedReason, VIR_DOMAIN_PAUSED_LAST, "ioerror", "watchdog", "from snapshot", - "shutdown") + "shutdown", + "snapshot") -#define VIR_DOMAIN_SHUTDOWN_LAST (VIR_DOMAIN_SHUTDOWN_USER + 1) VIR_ENUM_IMPL(virDomainShutdownReason, VIR_DOMAIN_SHUTDOWN_LAST, "unknown", "user") -#define VIR_DOMAIN_SHUTOFF_LAST (VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT + 1) VIR_ENUM_IMPL(virDomainShutoffReason, VIR_DOMAIN_SHUTOFF_LAST, "unknown", "shutdown", @@ -621,10 +617,12 @@ VIR_ENUM_IMPL(virDomainShutoffReason, VIR_DOMAIN_SHUTOFF_LAST, "failed", "from snapshot") -#define VIR_DOMAIN_CRASHED_LAST (VIR_DOMAIN_CRASHED_UNKNOWN + 1) VIR_ENUM_IMPL(virDomainCrashedReason, VIR_DOMAIN_CRASHED_LAST, "unknown") +VIR_ENUM_IMPL(virDomainPMSuspendedReason, VIR_DOMAIN_PMSUSPENDED_LAST, + "unknown") + VIR_ENUM_IMPL(virDomainSeclabel, VIR_DOMAIN_SECLABEL_LAST, "default", "none", @@ -15438,9 +15436,13 @@ virDomainStateReasonToString(virDomainState state, int reason) return virDomainShutoffReasonTypeToString(reason); case VIR_DOMAIN_CRASHED: return virDomainCrashedReasonTypeToString(reason); - default: - return NULL; + case VIR_DOMAIN_PMSUSPENDED: + return virDomainPMSuspendedReasonTypeToString(reason); + case VIR_DOMAIN_LAST: + break; } + VIR_WARN("Unexpected domain state: %d", state); + return NULL; } @@ -15462,9 +15464,13 @@ virDomainStateReasonFromString(virDomainState state, const char *reason) return virDomainShutoffReasonTypeFromString(reason); case VIR_DOMAIN_CRASHED: return virDomainCrashedReasonTypeFromString(reason); - default: - return -1; + case VIR_DOMAIN_PMSUSPENDED: + return virDomainPMSuspendedReasonTypeFromString(reason); + case VIR_DOMAIN_LAST: + break; } + VIR_WARN("Unexpected domain state: %d", state); + return -1; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index abbfe86..9a9e0b1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2315,6 +2315,7 @@ VIR_ENUM_DECL(virDomainPausedReason) VIR_ENUM_DECL(virDomainShutdownReason) VIR_ENUM_DECL(virDomainShutoffReason) VIR_ENUM_DECL(virDomainCrashedReason) +VIR_ENUM_DECL(virDomainPMSuspendedReason) const char *virDomainStateReasonToString(virDomainState state, int reason); int virDomainStateReasonFromString(virDomainState state, const char *reason); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fc23adc..57ecc36 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -487,10 +487,12 @@ virDomainObjSetState; virDomainObjTaint; virDomainPausedReasonTypeFromString; virDomainPausedReasonTypeToString; -virDomainPciRombarModeTypeFromString; -virDomainPciRombarModeTypeToString; virDomainPMStateTypeFromString; virDomainPMStateTypeToString; +virDomainPMSuspendedReasonTypeFromString; +virDomainPMSuspendedReasonTypeToString; +virDomainPciRombarModeTypeFromString; +virDomainPciRombarModeTypeToString; virDomainRedirdevBusTypeFromString; virDomainRedirdevBusTypeToString; virDomainRemoveInactive; -- 1.8.1

3 4

[libvirt] [PATCH] Fix performance & reliabilty of QMP probing
by Daniel P. Berrange 25 Jan '13

25 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> This previous commit commit 1a50ba2cb07d8bb2aa724062889deb9efd7ad9e9 Author: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com> Date: Mon Nov 26 15:17:13 2012 +0100 qemu: Fix QMP Capabability Probing Failure which attempted to make sure the QEMU process used for probing ran as the right user id, caused serious performance regression and unreliability in probing. The -daemonize switch in QEMU guarantees that the monitor socket is present before the parent process exits. This means libvirtd is guaranteed to be able to connect immediately. By switching from -daemonize to the virCommandDaemonize API libvirtd was no longer synchronized with QEMU's startup process. The result was that the QEMU monitor failed to open and went into its 200ms sleep loop. This happened for all 25 binaries resulting in 5 seconds worth of sleeping at libvirtd startup. In addition sometimes when libvirt connected, QEMU would be partially initialized and crash causing total failure to probe that binary. This commit reverts the previous change, ensuring we do use the -daemonize flag to QEMU. Startup delay is cut from 7 seconds to 2 seconds on my machine, which is on a par with what it was prior to the capabilities rewrite. To deal with the fact that QEMU needs to be able to create the pidfile, we switch pidfile location fron runDir to libDir, which QEMU is guaranteed to be able to write to. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/qemu/qemu_capabilities.c | 50 ++++++++++++++++++++++++++++++-------------- src/qemu/qemu_capabilities.h | 3 +-- 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 95fa3da..703179d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -38,6 +38,7 @@ #include "virbitmap.h" #include "virnodesuspend.h" #include "qemu_monitor.h" +#include "virtime.h" #include <sys/stat.h> #include <unistd.h> @@ -916,11 +917,19 @@ virCapsPtr qemuCapsInit(qemuCapsCachePtr cache) * so just probe for them all - we gracefully fail * if a qemu-system-$ARCH binary can't be found */ - for (i = 0 ; i < VIR_ARCH_LAST ; i++) + unsigned long long a, b; + for (i = 0 ; i < VIR_ARCH_LAST ; i++) { + unsigned long long start, end; + if (virTimeMillisNow(&start) < 0) + goto error; if (qemuCapsInitGuest(caps, cache, virArchFromHost(), i) < 0) goto error; + if (virTimeMillisNow(&end) < 0) + goto error; + VIR_DEBUG("Probed %s in %llums", virArchToString(i), end-start); + } /* QEMU Requires an emulator in the XML */ virCapabilitiesSetEmulatorRequired(caps); @@ -2291,7 +2300,6 @@ qemuCapsInitQMPBasic(qemuCapsPtr caps) static int qemuCapsInitQMP(qemuCapsPtr caps, const char *libDir, - const char *runDir, uid_t runUid, gid_t runGid) { @@ -2324,8 +2332,11 @@ qemuCapsInitQMP(qemuCapsPtr caps, /* ".pidfile" suffix is used rather than ".pid" to avoid a possible clash * with a qemu domain called "capabilities" + * Normally we'd use runDir for pid files, but because we're using + * -daemonize we need QEMU to be allowed to create them, rather + * than libvirtd. So we're using libDir which QEMU can write to */ - if (virAsprintf(&pidfile, "%s/%s", runDir, "capabilities.pidfile") < 0) { + if (virAsprintf(&pidfile, "%s/%s", libDir, "capabilities.pidfile") < 0) { virReportOOMError(); goto cleanup; } @@ -2337,6 +2348,13 @@ qemuCapsInitQMP(qemuCapsPtr caps, VIR_DEBUG("Try to get caps via QMP caps=%p", caps); + /* + * We explicitly need to use -daemonize here, rather than + * virCommandDaemonize, because we need to synchronize + * with QEMU creating its monitor socket API. Using + * daemonize guarnatees control won't return to libvirt + * until the socket is present. + */ cmd = virCommandNewArgList(caps->binary, "-S", "-no-user-config", @@ -2344,14 +2362,14 @@ qemuCapsInitQMP(qemuCapsPtr caps, "-nographic", "-M", "none", "-qmp", monarg, + "-pidfile", pidfile, + "-daemonize", NULL); virCommandAddEnvPassCommon(cmd); virCommandClearCaps(cmd); hookData.runUid = runUid; hookData.runGid = runGid; virCommandSetPreExecHook(cmd, qemuCapsHook, &hookData); - virCommandSetPidFile(cmd, pidfile); - virCommandDaemonize(cmd); if (virCommandRun(cmd, &status) < 0) goto cleanup; @@ -2472,7 +2490,6 @@ cleanup: qemuCapsPtr qemuCapsNewForBinary(const char *binary, const char *libDir, - const char *runDir, uid_t runUid, gid_t runGid) { @@ -2502,12 +2519,14 @@ qemuCapsPtr qemuCapsNewForBinary(const char *binary, goto error; } - if ((rv = qemuCapsInitQMP(caps, libDir, runDir, runUid, runGid)) < 0) + if ((rv = qemuCapsInitQMP(caps, libDir, runUid, runGid)) < 0) goto error; - if (!caps->usedQMP && - qemuCapsInitHelp(caps, runUid, runGid) < 0) - goto error; + if (!caps->usedQMP) { + VIR_ERROR("Falling back to help"); + if (qemuCapsInitHelp(caps, runUid, runGid) < 0) + goto error; + } return caps; @@ -2542,8 +2561,9 @@ qemuCapsHashDataFree(void *payload, const void *key ATTRIBUTE_UNUSED) qemuCapsCachePtr -qemuCapsCacheNew(const char *libDir, const char *runDir, - uid_t runUid, gid_t runGid) +qemuCapsCacheNew(const char *libDir, + uid_t runUid, + gid_t runGid) { qemuCapsCachePtr cache; @@ -2561,8 +2581,7 @@ qemuCapsCacheNew(const char *libDir, const char *runDir, if (!(cache->binaries = virHashCreate(10, qemuCapsHashDataFree))) goto error; - if (!(cache->libDir = strdup(libDir)) || - !(cache->runDir = strdup(runDir))) { + if (!(cache->libDir = strdup(libDir))) { virReportOOMError(); goto error; } @@ -2594,7 +2613,7 @@ qemuCapsCacheLookup(qemuCapsCachePtr cache, const char *binary) if (!ret) { VIR_DEBUG("Creating capabilities for %s", binary); - ret = qemuCapsNewForBinary(binary, cache->libDir, cache->runDir, + ret = qemuCapsNewForBinary(binary, cache->libDir, cache->runUid, cache->runGid); if (ret) { VIR_DEBUG("Caching capabilities %p for %s", @@ -2634,7 +2653,6 @@ qemuCapsCacheFree(qemuCapsCachePtr cache) return; VIR_FREE(cache->libDir); - VIR_FREE(cache->runDir); virHashFree(cache->binaries); virMutexDestroy(&cache->lock); VIR_FREE(cache); diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 089fa30..5279d07 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -179,7 +179,6 @@ qemuCapsPtr qemuCapsNew(void); qemuCapsPtr qemuCapsNewCopy(qemuCapsPtr caps); qemuCapsPtr qemuCapsNewForBinary(const char *binary, const char *libDir, - const char *runDir, uid_t runUid, gid_t runGid); @@ -219,7 +218,7 @@ int qemuCapsGetMachineTypesCaps(qemuCapsPtr caps, bool qemuCapsIsValid(qemuCapsPtr caps); -qemuCapsCachePtr qemuCapsCacheNew(const char *libDir, const char *runDir, +qemuCapsCachePtr qemuCapsCacheNew(const char *libDir, uid_t uid, gid_t gid); qemuCapsPtr qemuCapsCacheLookup(qemuCapsCachePtr cache, const char *binary); qemuCapsPtr qemuCapsCacheLookupCopy(qemuCapsCachePtr cache, const char *binary); -- 1.8.0.2

4 6

[libvirt] [Openstack][Sheepdog][Libvirt][Qemu]Add a new block storage driver by Libvirt/Qemu way for Openstack
by harryxiyou 25 Jan '13

25 Jan '13

Hi all, I wanna add a new block storage driver by Libvirt/Qemu way for Openstack, which is as same as Sheepdog driver for Openstack. So i think the theories are like this. 1, In the Openstack Nova branch, Openstck driver call libvirt client and send parameters to libvirt client.(From this point, i should modify Openstack Nova source codes. They are a, nova/nova/virt/libvirt/driver.py add new driver way b, /OpenStack/nova/nova/tests/test_libvirt.py add new driver test) 2, According to own protocol, libvirt client in Openstack Nova branch send parameters to Libvirt server.(From this point, i should modify libvirt library to let libvirt library support this new driver like Sheepdog). 3, Libvirt server call Qemu interfaces to send parameters to Qemu.(From this point, i should modify Qemu source codes to let Qemu support this new driver like Sheepdog). 4, In Openstack Cinder branch, Openstack driver use Qemu commands to create this new volumes to Qemu.(From this point, i should modify Openstack Cinder branch source codes like this. a, Add new driver file /OpenStack/cinder/cinder/volume/drivers/new_driver.py like Sheepdog.py b, Change file /OpenStack/cinder/cinder/tests/test_drivers_compatibility.py to test new driver). 5, At last, i should also modify /OpenStack/manuals/doc/src/docbkx/openstack-compute-admin/tables/hypervisors-nova-conf.xml to configure this new driver. Are my theories right? Should i do any other stuffs? Could anyone give me any other suggestions? Thanks in advance ;-) -- Thanks Harry Wei

2 5

[libvirt] [PATCH] virsh: Forbid usage of --memspec and --disk-only together
by Peter Krempa 25 Jan '13

25 Jan '13

virsh snapshot-create-as with --memspec and --disk only fails at the qemu driver. Forbid the creation at virsh too and document it. --- tools/virsh-snapshot.c | 6 ++++++ tools/virsh.pod | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index e4745d6..86fd1fd 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -457,6 +457,12 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) goto cleanup; } + if (memspec && (flags & VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY)) { + vshError(ctl, "%s", + _("Can't combine disk only snapshots with --memspec")); + goto cleanup; + } + if (memspec && vshParseSnapshotMemspec(ctl, &buf, memspec) < 0) goto cleanup; diff --git a/tools/virsh.pod b/tools/virsh.pod index e2a2aec..c5cf16c 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2746,7 +2746,8 @@ The I<--memspec> option can be used to control whether a checkpoint is internal or external. The I<--memspec> flag is mandatory, followed by a B<memspec> of the form B<[file=]name[,snapshot=type]>, where type can be B<none>, B<internal>, or B<external>. To include a literal -comma in B<file=name>, escape it with a second comma. +comma in B<file=name>, escape it with a second comma. I<--memspec> cannot +be used together with I<--disk-only>. The I<--diskspec> option can be used to control how I<--disk-only> and external checkpoints create external files. This option can occur -- 1.8.1.1

2 1

[libvirt] [PATCH] Fix bogus reporting of KVM support for non-native emulators
by Daniel P. Berrange 25 Jan '13

25 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> A logic bug meant we reported KVM was possible for every architecture, merely based on whether the query-kvm command exists. We should instead have been doing it based on whether the query-kvm command returns 'present: 1' Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/qemu/qemu_capabilities.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f86c28f..29693c3 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2117,11 +2117,16 @@ qemuCapsProbeQMPKVMState(qemuCapsPtr caps, return -1; /* The QEMU_CAPS_KVM flag was initially set according to the QEMU - * reporting the recognition of 'query-kvm' QMP command, but the - * flag means whether the KVM is enabled by default and should be - * disabled in case we want SW emulated machine, so let's fix that - * if it's true. */ - if (!enabled) { + * reporting the recognition of 'query-kvm' QMP command. That merely + * indicates existance of the command though, not whether KVM support + * is actually available, nor whether it is enabled by default. + * + * If it is not present we need to clear the flag, and if it is + * not enabled by default we need to change the flag. + */ + if (!present) { + qemuCapsClear(caps, QEMU_CAPS_KVM); + } else if (!enabled) { qemuCapsClear(caps, QEMU_CAPS_KVM); qemuCapsSet(caps, QEMU_CAPS_ENABLE_KVM); } -- 1.8.0.2

2 1

[libvirt] [PATCH] qemu: don't share kerberos caches between domains
by Eric Blake 25 Jan '13

25 Jan '13

https://bugzilla.redhat.com/show_bug.cgi?id=718377 complains that there were some SELinux AVCs when using vnc console over Kerberos. The root problem was that Kerberos tries to set up a cache file, and if we don't tell it where, then all domains use the same cache file, which violates sVirt protections. Setting the environment variable unconditionally should be safe, even for setups where Kerboros won't actually create a cache file. * src/qemu/qemu_process.c (qemuProcessStart): Set KRB5CACHEDIR for each domain. --- src/qemu/qemu_process.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 55d00e3..986e8ee 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1,7 +1,7 @@ /* * qemu_process.h: QEMU process management * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2013 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 @@ -3857,6 +3857,8 @@ int qemuProcessStart(virConnectPtr conn, virCommandSetPidFile(cmd, priv->pidfile); virCommandDaemonize(cmd); virCommandRequireHandshake(cmd); + virCommandAddEnvFormat(cmd, "KRB5CACHEDIR=%s/%s.krb", + driver->cacheDir, vm->def->name); ret = virCommandRun(cmd, NULL); -- 1.8.1

4 6

[libvirt] [PATCH] qemuDomainSendKey: Relax the qemu driver locking
by Michal Privoznik 25 Jan '13

25 Jan '13

Currently, there is no reason to hold qemu driver locked throughout whole API execution. Moreover, we can use the new qemuDomObjFromDomain() internal API to lookup domain then. --- src/qemu/qemu_driver.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6d4c1e9..100f10b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2407,19 +2407,12 @@ static int qemuDomainSendKey(virDomainPtr domain, } } - qemuDriverLock(driver); - vm = virDomainFindByUUID(&driver->domains, domain->uuid); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(domain->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = qemuDomObjFromDomain(domain))) goto cleanup; - } priv = vm->privateData; - if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0) + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -2428,9 +2421,9 @@ static int qemuDomainSendKey(virDomainPtr domain, goto endjob; } - qemuDomainObjEnterMonitorWithDriver(driver, vm); + qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorSendKey(priv->mon, holdtime, keycodes, nkeycodes); - qemuDomainObjExitMonitorWithDriver(driver, vm); + qemuDomainObjExitMonitor(driver, vm); endjob: if (qemuDomainObjEndJob(driver, vm) == 0) @@ -2439,7 +2432,6 @@ endjob: cleanup: if (vm) virObjectUnlock(vm); - qemuDriverUnlock(driver); return ret; } -- 1.8.0.2

5 7

[libvirt] [PATCH] node_memory: Add '\n' to help message
by Satoru Moriya 25 Jan '13

25 Jan '13

Linefeed is missed in the help of node-memory-tune. This patch just adds '\n' to get a correct help message. Signed-off-by: Satoru Moriya <satoru.moriya(a)hds.com> --- tools/virsh-host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-host.c b/tools/virsh-host.c index d05e435..b83c893 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -764,7 +764,7 @@ cmdVersion(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) static const vshCmdInfo info_node_memory_tune[] = { {"help", N_("Get or set node memory parameters")}, - {"desc", N_("Get or set node memory parameters" + {"desc", N_("Get or set node memory parameters\n" " To get the memory parameters, use following command: \n\n" " virsh # node-memory-tune")}, {NULL, NULL} -- 1.7.11.7

2 1

[libvirt] [PATCH] qemu: escape ipv6 for rbd network disk hosts
by Josh Durgin 25 Jan '13

25 Jan '13

Hosts for rbd are ceph monitor daemons. These have fixed IP addresses, so they are often referenced by IP rather than hostname for convenience, or to avoid relying on DNS. Using IPv4 addresses as the host name works already, but IPv6 addresses require rbd-specific escaping because the colon is used as an option separator in the string passed to qemu. Escape these colons, and enclose the IPv6 address in square brackets if a port is specified. Signed-off-by: Josh Durgin <josh.durgin(a)inktank.com> --- docs/schemas/domaincommon.rng | 5 ++- src/qemu/qemu_command.c | 34 +++++++++++++++---- tests/qemuargv2xmltest.c | 1 + .../qemuxml2argv-disk-drive-network-rbd-ipv6.args | 9 +++++ .../qemuxml2argv-disk-drive-network-rbd-ipv6.xml | 36 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 + 6 files changed, 79 insertions(+), 8 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.xml diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7f3320e..273e54c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1099,7 +1099,10 @@ </attribute> </optional> <attribute name="name"> - <ref name="dnsName"/> + <choice> + <ref name="dnsName"/> + <ref name="ipAddr"/> + </choice> </attribute> <attribute name="port"> <ref name="unsignedInt"/> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 02fe015..dfc042b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -34,6 +34,7 @@ #include "virerror.h" #include "virutil.h" #include "virfile.h" +#include "virstring.h" #include "viruuid.h" #include "c-ctype.h" #include "domain_nwfilter.h" @@ -1937,13 +1938,16 @@ qemuBuildRBDString(virConnectPtr conn, if (i) { virBufferAddLit(opt, "\\;"); } - if (disk->hosts[i].port) { - virBufferAsprintf(opt, "%s\\:%s", - disk->hosts[i].name, - disk->hosts[i].port); + + /* assume host containing : is ipv6 */ + if (strchr(disk->hosts[i].name, ':')) { + virBufferEscape(opt, '\\', ":", "[%s]", disk->hosts[i].name); } else { virBufferAsprintf(opt, "%s", disk->hosts[i].name); } + if (disk->hosts[i].port) { + virBufferAsprintf(opt, "\\:%s", disk->hosts[i].port); + } } } @@ -1961,15 +1965,26 @@ error: static int qemuAddRBDHost(virDomainDiskDefPtr disk, char *hostport) { char *port; + size_t skip; + char **parts; disk->nhosts++; if (VIR_REALLOC_N(disk->hosts, disk->nhosts) < 0) goto no_memory; - port = strstr(hostport, "\\:"); + if (strchr(hostport, ']')) { + /* ipv6, strip brackets */ + hostport += 1; + port = strstr(hostport, "]\\:"); + skip = 3; + } else { + port = strstr(hostport, "\\:"); + skip = 2; + } + if (port) { *port = '\0'; - port += 2; + port += skip; disk->hosts[disk->nhosts-1].port = strdup(port); if (!disk->hosts[disk->nhosts-1].port) goto no_memory; @@ -1978,7 +1993,12 @@ static int qemuAddRBDHost(virDomainDiskDefPtr disk, char *hostport) if (!disk->hosts[disk->nhosts-1].port) goto no_memory; } - disk->hosts[disk->nhosts-1].name = strdup(hostport); + + parts = virStringSplit(hostport, "\\:", 0); + if (!parts) + goto no_memory; + disk->hosts[disk->nhosts-1].name = virStringJoin((const char **)parts, ":"); + virStringFreeList(parts); if (!disk->hosts[disk->nhosts-1].name) goto no_memory; diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 2923324..e465f3d 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -185,6 +185,7 @@ mymain(void) DO_TEST("disk-drive-network-nbd"); DO_TEST("disk-drive-network-gluster"); DO_TEST("disk-drive-network-rbd"); + DO_TEST("disk-drive-network-rbd-ipv6"); /* older format using CEPH_ARGS env var */ DO_TEST("disk-drive-network-rbd-ceph-env"); DO_TEST("disk-drive-network-sheepdog"); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args new file mode 100644 index 0000000..0c67229 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.args @@ -0,0 +1,9 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \ +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -drive \ +file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \ +'file=rbd:pool/image:auth_supported=none:\ +mon_host=[\:\:1]\:6321\;example.org\:6789\;\ +[ffff\:1234\:567\:abc\:\:0f]\:6322\;\ +[2001\:db8\:\:ff00\:42\:8329]\:6322,\ +if=virtio,format=raw' -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.xml new file mode 100644 index 0000000..a2ca2d2 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-ipv6.xml @@ -0,0 +1,36 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw'/> + <source protocol='rbd' name='pool/image'> + <host name='::1' port='6321'/> + <host name='example.org' port='6789'/> + <host name='ffff:1234:567:abc::0f' port='6322'/> + <host name='2001:db8::ff00:42:8329' port='6322'/> + </source> + <target dev='vda' bus='virtio'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 31ed116..88e1c36 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -478,6 +478,8 @@ mymain(void) QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST("disk-drive-network-rbd-auth", QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); + DO_TEST("disk-drive-network-rbd-ipv6", + QEMU_CAPS_DRIVE, QEMU_CAPS_DRIVE_FORMAT); DO_TEST("disk-drive-no-boot", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_BOOTINDEX); DO_TEST("disk-usb", NONE); -- 1.7.2.5

2 4

[libvirt] [PATCH] snapshot: fix state after external snapshot of S3 domain
by Eric Blake 25 Jan '13

25 Jan '13

https://bugzilla.redhat.com/show_bug.cgi?id=876829 complains that if a guest is put into S3 state (such as via virsh dompmsuspend) and then an external snapshot is taken, qemu forcefully transitions the domain to paused, but libvirt doesn't reflect that change internally. Thus, a user has to use 'virsh suspend' to get libvirt back in sync with qemu state, and if the user doesn't know this trick, then the guest appears hung. * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateActiveExternal): Track fact that qemu wakes up a suspended domain on migration. --- src/qemu/qemu_driver.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6d4c1e9..da30dcb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1,7 +1,7 @@ /* * qemu_driver.c: core driver methods for managing qemu guests * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2013 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -11124,6 +11124,7 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn, bool atomic = !!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC); bool transaction = qemuCapsGet(priv->caps, QEMU_CAPS_TRANSACTION); int thaw = 0; /* 1 if freeze succeeded, -1 if freeze failed */ + bool pmsuspended = false; if (qemuDomainObjBeginAsyncJobWithDriver(driver, vm, QEMU_ASYNC_JOB_SNAPSHOT) < 0) @@ -11144,7 +11145,9 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn, } /* we need to resume the guest only if it was previously running */ - if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PMSUSPENDED) { + pmsuspended = true; + } else if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { resume = true; /* For external checkpoints (those with memory), the guest @@ -11226,6 +11229,18 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn, vm = NULL; if (event) qemuDomainEventQueue(driver, event); + } else if (memory && pmsuspended) { + /* qemu 1.3 is unable to save a domain in pm-suspended (S3) + * state; so we must emit an event stating that it was + * converted to paused. */ + virDomainEventPtr event; + + virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, + VIR_DOMAIN_PAUSED_FROM_SNAPSHOT); + event = virDomainEventNewFromObj(vm, VIR_DOMAIN_EVENT_SUSPENDED, + VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT); + if (event) + qemuDomainEventQueue(driver, event); } ret = 0; -- 1.8.1

2 2

[libvirt] [PATCH 00/10] Patches for various coverity issues
by John Ferlan 25 Jan '13

25 Jan '13

locking: Fixes Coverity OVERRUN warning due to the size of the source string being 19 bytes and the memcpy size of 48 bytes security: Fixes Coverity warning STRING_OVERFLOW about due to the copy being from a variable sized string to a fixed length string. Even though on read other checks ensure the source buffer is no longer than the number of bytes in the destination, Coverity doesn't know that. rpc: Fixes Coverity warning DEADCODE due to removal of code as specified in the commit message parallels: Fixes Coverity warning NULL_RETURNS from not checking result of virJSONValueObjectGetString() call as part of commit id: 8ce9e2ab util: Fixes Coverity warning NULL_RETURNS due to not checking the return value 'child' from a virJSONValueNewObject() call. qemu: Fixes Coverity warning NULL_RETURNS from not checking the return value 'activeDev' from a pciDeviceListFind() call. selinux: Fixes Coverity warning RESOURCE_LEAK as a result of code specified in the commit message esx: Fixes Coverity warning REVERSE_INULL as a result of needlessly checking if objectSpec != NULL. At the point in the code, objectSpec must be valid. virobject: Fixes Coverity warning CONSTANT_EXPRESSION_RESULT and DEADCODE as a result of commit id b545f65 using "if (!virObjectInitialize() < 0)" network: Fixes Coverity warning UNUSED_VALUE as specific in the commit message John Ferlan (10): locking: use virStrcpy instead of memcpy security: Use virStrcpy to move the label rpc: Remove call to virKeepAliveStop at init parallels: Need to handle virJSONValueObjectGetString error util: Need to check child JSON allocation before use qemu: Check valid activeDev before calling pciDeviceSetUsedBy selinux: Resolve resource leak using the default disk label esx: No need to check for objectSpec virobject: Remove the bogus ! from call to virObjectInitialize() network: Remove dead code getting, but not using ipdef src/conf/domain_conf.c | 29 +++++++++++++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/esx/esx_vi.c | 7 +++---- src/locking/lock_driver_sanlock.c | 9 ++++++++- src/network/bridge_driver.c | 3 --- src/parallels/parallels_driver.c | 5 ++++- src/qemu/qemu_hostdev.c | 3 ++- src/rpc/virnetserverclient.c | 2 -- src/security/security_dac.c | 8 +++++++- src/security/security_selinux.c | 6 +++--- src/util/virlockspace.c | 3 +++ src/util/virobject.c | 4 ++-- 12 files changed, 64 insertions(+), 18 deletions(-) -- 1.7.11.7

4 37

[libvirt] [PATCH] Move QEMU capabilities initialization later in QEMU startup
by Daniel P. Berrange 25 Jan '13

25 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> Currently QEMU capabilities are initialized before the QEMU driver sets ownership on its various directories. The upshot is that if you change the user/group in the qemu.conf file, libvirtd will fail to probe QEMU the first time it is run after the config change. Moving QEMU capabilities initialization to after the chown() calls fixes this Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/qemu/qemu_driver.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1f43d54..9ff920e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -825,13 +825,6 @@ qemuStartup(bool privileged, if (qemuSecurityInit(qemu_driver) < 0) goto error; - qemu_driver->capsCache = qemuCapsCacheNew(qemu_driver->libDir, - qemu_driver->stateDir, - qemu_driver->user, - qemu_driver->group); - if (!qemu_driver->capsCache) - goto error; - if ((qemu_driver->activePciHostdevs = pciDeviceListNew()) == NULL) goto error; @@ -871,6 +864,12 @@ qemuStartup(bool privileged, } } + qemu_driver->capsCache = qemuCapsCacheNew(qemu_driver->libDir, + qemu_driver->user, + qemu_driver->group); + if (!qemu_driver->capsCache) + goto error; + if ((qemu_driver->caps = qemuCreateCapabilities(qemu_driver)) == NULL) goto error; -- 1.8.0.2

2 1

[libvirt] [PATCH v3 0/2] selinux: Only create the selabel_handle once.
by Richard W.M. Jones 25 Jan '13

25 Jan '13

Version 3 of this patch. Because I now have to pass the 'mgr' pointer around, the patch is considerably more complicated than before. Patch 1/2 is required so that I can use virReportSystemError when I don't need to have any optional arguments, ie. the equivalent of: printf ("foo\n"); I have tested these two patches against the libguestfs test suite, and it works for me and removes the memory leak. Rich.

3 6

[libvirt] not able to stop vmware player node through libvirt API
by varun bhatnagar 25 Jan '13

25 Jan '13

Hi, I have started one node through libvirt API using its start method. I am trying to stop that node using the *destroy *method provided by the API but I am not able to do so. Instead, I am getting one message saying: *libvir: error: internal error Child process (vmrun -T player stop /root/test_folder/myImage.vmx soft) unexpected exit status 255* Can anybody tell me why I am not able to stop the node. Thanks in advance. :)

1 0

[libvirt] [libvirt-java] Add various block, snapshot and migrate methods
by Wido den Hollander 24 Jan '13

24 Jan '13

Hi, I've sent a series of patches like these about two weeks ago and got some great feedback from Claudio on those! The feedback from Claudio has been used for writing this series of patches. Backwards compatibility has been preserved by still using virDomainMigrate and virtDomainMigrateToUri for the exisiting methods, just to be sure. Thank you, Wido

2 17

[libvirt] libvirt -- add virtio-scsi disk with scsi-generic
by 韩诚 24 Jan '13

24 Jan '13

Hi, Paolo, All, I read "virtio-scsi support proposal v2"(http://permalink.gmane.org/gmane.comp.emulators.libvirt/50428) Try to add a virtio-scsi with scsi-generic like this: <hostdev type='scsi'> <source> <adapter name='scsi_host0'/> <address type='scsi' bus='0' target='0' unit='0'/> </source> <target> <address type='scsi' controller='0' bus='0' target='0' unit='2'/> </target> </hostdev> But It turn out to be wrong, showing: error: Failed to define domain from rhel63ga error: XML error: unknown host device source address type 'scsi_host' I'd like to know how shall to add a virtio-scsi disk with scsi-generic option. Thanks~ -- Best regards, Cheng

3 4

[libvirt] This patch adds the label to lxc-enter-namespace
by Daniel J Walsh 24 Jan '13

24 Jan '13

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 (Resend to the correct list) lxc-enter-namespace allows a process from outside a container to start a process inside a container. One problem with the current code is the process running within the container would run with the label of the process that created it. For example if the admin process is running as unconfined_t and executes the following command # virsh -c lxc:/// lxc-enter-namespace --nolabel dan -- /bin/ps -eZ LABEL PID TTY TIME CMD system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 29 ? 00:00:00 dhclient staff_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 47 ? 00:00:00 ps Note the ps command is running as unconfined_t, After this patch, virsh -c lxc:/// lxc-enter-namespace dan -- /bin/ps -eZ LABEL PID TTY TIME CMD system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 32 ? 00:00:00 dhclient system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 38 ? 00:00:00 ps I also add a --nolabel command to virsh, which can go back to the original behaviour. virsh -c lxc:/// lxc-enter-namespace --nolabel dan -- /bin/ps -eZ LABEL PID TTY TIME CMD system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 1 pts/0 00:00:00 systemd system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 3 pts/1 00:00:00 sh system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 24 ? 00:00:00 systemd-journal system_u:system_r:svirt_lxc_net_t:s0:c0.c1023 32 ? 00:00:00 dhclient staff_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 37 ? 00:00:00 ps One problem I had when I originally did the patch is lxcDomainGetSecurityLabel was returning the incorrect label, I needed the label of the initpid within the container not its parent process, so I changed this function to match OpenNamespaces function. One last strangeness, about half the time I run this, virsh hangs and never returns. Seems like if (conn->driver->domainGetSecurityLabel(domain, seclabel) == 0) { Gets hung up. I have attached the strace in out1.gz -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlEBO2EACgkQrlYvE4MpobMS5ACg3Ih4Iu0lD9BofF4iP0QXarAL jpQAoLyWWNhnnFw2TRDJsXqvrTTVujyZ =hUZ/ -----END PGP SIGNATURE-----

2 1

[libvirt] [PATCH 0/2] Two additional build fixes
by Jiri Denemark 24 Jan '13

24 Jan '13

Both pushed as trivial and build-breakers. Jiri Denemark (2): apparmor: Avoid freeing uninitialized pointer selinux: Properly indent preprocessor directives src/security/security_apparmor.c | 2 +- src/security/security_selinux.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) -- 1.8.1.1

1 2

[libvirt] [PATCH 0/2] Fix build after commit 87b4c10c6cf02251dd8c29b5b895bebc6ec297f9
by Peter Krempa 24 Jan '13

24 Jan '13

My gcc didn't whine about the uninitialized variable. This series tries to fix and then actually fixes the bug. Sorry for the inconvenience. I'm putting on my brown bag and upgrading my GCC. Series pushed as a build-breaker. Peter Krempa (2): xen: Initialize variable before using xen: Actually fix the uninitialized variable src/xen/xend_internal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 1.8.1.1

1 2

[libvirt] [libvirt-glib 1/2] Use g_strlcpy instead of strncpy
by Christophe Fergeau 24 Jan '13

24 Jan '13

This guarantees that the string will be nul-terminated. Coverity warned about this issue. --- libvirt-gobject/libvirt-gobject-connection.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index 91cc535..0525323 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -1583,7 +1583,7 @@ GVirNodeInfo *gvir_connection_get_node_info(GVirConnection *conn, } ret = g_slice_new(GVirNodeInfo); - strncpy (ret->model, info.model, sizeof (ret->model)); + g_strlcpy (ret->model, info.model, sizeof (ret->model)); ret->memory = info.memory; ret->cpus = info.cpus; ret->mhz = info.mhz; -- 1.8.0.2

2 6

[libvirt] [PATCH 0/5] Resolve parallels and virCommandPtr resource leaks
by John Ferlan 24 Jan '13

24 Jan '13

This is v3 of the parallels_driver.c changes. The most recent review pointed out that virCommandPtr's need to be free'd. I found a few other instances where they weren't free'd. I also found a few instances where the status check from virCommandRun was not < 0 and cleaned them to be more consistent. John Ferlan (5): parallels: Resolve some resource leaks security: Need to add virCommandFree() storage: Need to add virCommandFree() util: Need to add virCommandFree() parallels_utils: Check return status properly from virCommandRun() src/parallels/parallels_driver.c | 47 ++++++++++++++++++++++++---------------- src/parallels/parallels_utils.c | 2 +- src/security/security_apparmor.c | 1 + src/storage/storage_backend_fs.c | 2 ++ src/util/virnetdevopenvswitch.c | 2 ++ 5 files changed, 34 insertions(+), 20 deletions(-) -- 1.7.11.7

2 8

[libvirt] [PATCHv3 0/6] Add additional data to NUMA topology info
by Peter Krempa 24 Jan '13

24 Jan '13

Another spin of the series. Difference to previous version: - remove redundant init of some structures - add graceful behavior if topology cannot be discovered - fix incorrect usage of virBitmapParse I'm working on adding more docs for the capabilities XML and the support for the xend driver. Peter Krempa (6): schema: Make the cpuset type reusable across schema files schemas: Add schemas for more CPU topology information in the caps XML conf: Split out NUMA topology formatting to simplify access to data capabilities: Switch CPU data in NUMA topology to a struct capabilities: Add additional data to the NUMA topology info test: Add support for thread and core information for the test driver docs/schemas/basictypes.rng | 6 +++ docs/schemas/capability.rng | 11 +++++ docs/schemas/domaincommon.rng | 5 --- src/conf/capabilities.c | 94 ++++++++++++++++++++++++++++++------------- src/conf/capabilities.h | 16 +++++++- src/libvirt_private.syms | 1 + src/nodeinfo.c | 85 ++++++++++++++++++++++++++++++++------ src/qemu/qemu_process.c | 2 +- src/test/test_driver.c | 24 +++++++++-- src/xen/xend_internal.c | 21 +++++----- 10 files changed, 204 insertions(+), 61 deletions(-) -- 1.8.1.1

2 9

[libvirt] [PATCH v4] selinux: Only create the selabel_handle once.
by Richard W.M. Jones 24 Jan '13

24 Jan '13

From: "Richard W.M. Jones" <rjones(a)redhat.com> According to Eric Paris this is slightly more efficient because it only loads the regular expressions in libselinux once. --- src/security/security_selinux.c | 129 ++++++++++++++++++++++++++-------------- 1 file changed, 83 insertions(+), 46 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index a3ef728..2affe69 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -63,6 +63,9 @@ struct _virSecuritySELinuxData { char *content_context; virHashTablePtr mcs; bool skipAllLabel; +#if HAVE_SELINUX_LABEL_H + struct selabel_handle *label_handle; +#endif }; struct _virSecuritySELinuxCallbackData { @@ -367,12 +370,21 @@ virSecuritySELinuxLXCInitialize(virSecurityManagerPtr mgr) data->skipAllLabel = true; +#if HAVE_SELINUX_LABEL_H + data->label_handle = selabel_open(SELABEL_CTX_FILE, NULL, 0); + if (!data->label_handle) { + virReportSystemError(errno, "%s", + _("cannot open SELinux label_handle")); + return -1; + } +#endif + selinux_conf = virConfReadFile(selinux_lxc_contexts_path(), 0); if (!selinux_conf) { virReportSystemError(errno, _("cannot open SELinux lxc contexts file '%s'"), selinux_lxc_contexts_path()); - return -1; + goto error; } scon = virConfGetValue(selinux_conf, "process"); @@ -418,6 +430,9 @@ virSecuritySELinuxLXCInitialize(virSecurityManagerPtr mgr) return 0; error: +#if HAVE_SELINUX_LABEL_H + selabel_close(data->label_handle); +#endif virConfFree(selinux_conf); VIR_FREE(data->domain_context); VIR_FREE(data->file_context); @@ -444,6 +459,15 @@ virSecuritySELinuxQEMUInitialize(virSecurityManagerPtr mgr) data->skipAllLabel = false; +#if HAVE_SELINUX_LABEL_H + data->label_handle = selabel_open(SELABEL_CTX_FILE, NULL, 0); + if (!data->label_handle) { + virReportSystemError(errno, "%s", + _("cannot open SELinux label_handle")); + return -1; + } +#endif + if (virFileReadAll(selinux_virtual_domain_context_path(), MAX_CONTEXT, &(data->domain_context)) < 0) { virReportSystemError(errno, _("cannot read SELinux virtual domain context file '%s'"), @@ -499,6 +523,9 @@ virSecuritySELinuxQEMUInitialize(virSecurityManagerPtr mgr) return 0; error: +#if HAVE_SELINUX_LABEL_H + selabel_close(data->label_handle); +#endif VIR_FREE(data->domain_context); VIR_FREE(data->alt_domain_context); VIR_FREE(data->file_context); @@ -763,6 +790,10 @@ virSecuritySELinuxSecurityDriverClose(virSecurityManagerPtr mgr) if (!data) return 0; +#if HAVE_SELINUX_LABEL_H + selabel_close(data->label_handle); +#endif + virHashFree(data->mcs); VIR_FREE(data->domain_context); @@ -937,18 +968,13 @@ virSecuritySELinuxFSetFilecon(int fd, char *tcon) /* Set fcon to the appropriate label for path and mode, or return -1. */ static int -getContext(const char *newpath, mode_t mode, security_context_t *fcon) +getContext(virSecurityManagerPtr mgr, + const char *newpath, mode_t mode, security_context_t *fcon) { #if HAVE_SELINUX_LABEL_H - struct selabel_handle *handle = selabel_open(SELABEL_CTX_FILE, NULL, 0); - int ret; - - if (handle == NULL) - return -1; + virSecuritySELinuxDataPtr data = virSecurityManagerGetPrivateData(mgr); - ret = selabel_lookup_raw(handle, fcon, newpath, mode); - selabel_close(handle); - return ret; + return selabel_lookup_raw(data->label_handle, fcon, newpath, mode); #else return matchpathcon(newpath, mode, fcon); #endif @@ -958,7 +984,8 @@ getContext(const char *newpath, mode_t mode, security_context_t *fcon) /* This method shouldn't raise errors, since they'll overwrite * errors that the caller(s) are already dealing with */ static int -virSecuritySELinuxRestoreSecurityFileLabel(const char *path) +virSecuritySELinuxRestoreSecurityFileLabel(virSecurityManagerPtr mgr, + const char *path) { struct stat buf; security_context_t fcon = NULL; @@ -980,7 +1007,7 @@ virSecuritySELinuxRestoreSecurityFileLabel(const char *path) goto err; } - if (getContext(newpath, buf.st_mode, &fcon) < 0) { + if (getContext(mgr, newpath, buf.st_mode, &fcon) < 0) { /* Any user created path likely does not have a default label, * which makes this an expected non error */ @@ -997,7 +1024,7 @@ err: } static int -virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, +virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainDiskDefPtr disk, int migrated) @@ -1044,7 +1071,7 @@ virSecuritySELinuxRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr ATTRIBU } } - return virSecuritySELinuxRestoreSecurityFileLabel(disk->src); + return virSecuritySELinuxRestoreSecurityFileLabel(mgr, disk->src); } @@ -1301,22 +1328,27 @@ virSecuritySELinuxSetSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UN static int virSecuritySELinuxRestoreSecurityPCILabel(pciDevice *dev ATTRIBUTE_UNUSED, const char *file, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { - return virSecuritySELinuxRestoreSecurityFileLabel(file); + virSecurityManagerPtr mgr = opaque; + + return virSecuritySELinuxRestoreSecurityFileLabel(mgr, file); } static int virSecuritySELinuxRestoreSecurityUSBLabel(usbDevice *dev ATTRIBUTE_UNUSED, const char *file, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { - return virSecuritySELinuxRestoreSecurityFileLabel(file); + virSecurityManagerPtr mgr = opaque; + + return virSecuritySELinuxRestoreSecurityFileLabel(mgr, file); } static int -virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(virDomainHostdevDefPtr dev, +virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(virSecurityManagerPtr mgr, + virDomainHostdevDefPtr dev, const char *vroot) { @@ -1335,7 +1367,7 @@ virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(virDomainHostdevDefPtr dev, if (!usb) goto done; - ret = usbDeviceFileIterate(usb, virSecuritySELinuxRestoreSecurityUSBLabel, NULL); + ret = usbDeviceFileIterate(usb, virSecuritySELinuxRestoreSecurityUSBLabel, mgr); usbFreeDevice(usb); break; @@ -1350,7 +1382,7 @@ virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(virDomainHostdevDefPtr dev, if (!pci) goto done; - ret = pciDeviceFileIterate(pci, virSecuritySELinuxRestoreSecurityPCILabel, NULL); + ret = pciDeviceFileIterate(pci, virSecuritySELinuxRestoreSecurityPCILabel, mgr); pciFreeDevice(pci); break; @@ -1367,7 +1399,8 @@ done: static int -virSecuritySELinuxRestoreSecurityHostdevCapsLabel(virDomainHostdevDefPtr dev, +virSecuritySELinuxRestoreSecurityHostdevCapsLabel(virSecurityManagerPtr mgr, + virDomainHostdevDefPtr dev, const char *vroot) { int ret = -1; @@ -1387,7 +1420,7 @@ virSecuritySELinuxRestoreSecurityHostdevCapsLabel(virDomainHostdevDefPtr dev, return -1; } } - ret = virSecuritySELinuxRestoreSecurityFileLabel(path); + ret = virSecuritySELinuxRestoreSecurityFileLabel(mgr, path); VIR_FREE(path); break; } @@ -1405,7 +1438,7 @@ virSecuritySELinuxRestoreSecurityHostdevCapsLabel(virDomainHostdevDefPtr dev, return -1; } } - ret = virSecuritySELinuxRestoreSecurityFileLabel(path); + ret = virSecuritySELinuxRestoreSecurityFileLabel(mgr, path); VIR_FREE(path); break; } @@ -1420,7 +1453,7 @@ virSecuritySELinuxRestoreSecurityHostdevCapsLabel(virDomainHostdevDefPtr dev, static int -virSecuritySELinuxRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, +virSecuritySELinuxRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainHostdevDefPtr dev, const char *vroot) @@ -1437,10 +1470,10 @@ virSecuritySELinuxRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr ATTRIBUT switch (dev->mode) { case VIR_DOMAIN_HOSTDEV_MODE_SUBSYS: - return virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(dev, vroot); + return virSecuritySELinuxRestoreSecurityHostdevSubsysLabel(mgr, dev, vroot); case VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES: - return virSecuritySELinuxRestoreSecurityHostdevCapsLabel(dev, vroot); + return virSecuritySELinuxRestoreSecurityHostdevCapsLabel(mgr, dev, vroot); default: return 0; @@ -1522,7 +1555,8 @@ done: } static int -virSecuritySELinuxRestoreSecurityChardevLabel(virDomainDefPtr def, +virSecuritySELinuxRestoreSecurityChardevLabel(virSecurityManagerPtr mgr, + virDomainDefPtr def, virDomainChrDefPtr dev, virDomainChrSourceDefPtr dev_source) @@ -1545,14 +1579,14 @@ virSecuritySELinuxRestoreSecurityChardevLabel(virDomainDefPtr def, switch (dev_source->type) { case VIR_DOMAIN_CHR_TYPE_DEV: case VIR_DOMAIN_CHR_TYPE_FILE: - if (virSecuritySELinuxRestoreSecurityFileLabel(dev_source->data.file.path) < 0) + if (virSecuritySELinuxRestoreSecurityFileLabel(mgr, dev_source->data.file.path) < 0) goto done; ret = 0; break; case VIR_DOMAIN_CHR_TYPE_UNIX: if (!dev_source->data.nix.listen) { - if (virSecuritySELinuxRestoreSecurityFileLabel(dev_source->data.file.path) < 0) + if (virSecuritySELinuxRestoreSecurityFileLabel(mgr, dev_source->data.file.path) < 0) goto done; } ret = 0; @@ -1565,11 +1599,11 @@ virSecuritySELinuxRestoreSecurityChardevLabel(virDomainDefPtr def, goto done; } if (virFileExists(in) && virFileExists(out)) { - if ((virSecuritySELinuxRestoreSecurityFileLabel(out) < 0) || - (virSecuritySELinuxRestoreSecurityFileLabel(in) < 0)) { + if ((virSecuritySELinuxRestoreSecurityFileLabel(mgr, out) < 0) || + (virSecuritySELinuxRestoreSecurityFileLabel(mgr, in) < 0)) { goto done; } - } else if (virSecuritySELinuxRestoreSecurityFileLabel(dev_source->data.file.path) < 0) { + } else if (virSecuritySELinuxRestoreSecurityFileLabel(mgr, dev_source->data.file.path) < 0) { goto done; } ret = 0; @@ -1590,14 +1624,16 @@ done: static int virSecuritySELinuxRestoreSecurityChardevCallback(virDomainDefPtr def, virDomainChrDefPtr dev, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { + virSecurityManagerPtr mgr = opaque; + /* This is taken care of by processing of def->serials */ if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && dev->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) return 0; - return virSecuritySELinuxRestoreSecurityChardevLabel(def, dev, + return virSecuritySELinuxRestoreSecurityChardevLabel(mgr, def, dev, &dev->source); } @@ -1605,8 +1641,9 @@ virSecuritySELinuxRestoreSecurityChardevCallback(virDomainDefPtr def, static int virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr def, virDomainSmartcardDefPtr dev, - void *opaque ATTRIBUTE_UNUSED) + void *opaque) { + virSecurityManagerPtr mgr = opaque; const char *database; switch (dev->type) { @@ -1617,10 +1654,10 @@ virSecuritySELinuxRestoreSecuritySmartcardCallback(virDomainDefPtr def, database = dev->data.cert.database; if (!database) database = VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE; - return virSecuritySELinuxRestoreSecurityFileLabel(database); + return virSecuritySELinuxRestoreSecurityFileLabel(mgr, database); case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: - return virSecuritySELinuxRestoreSecurityChardevLabel(def, NULL, &dev->data.passthru); + return virSecuritySELinuxRestoreSecurityChardevLabel(mgr, def, NULL, &dev->data.passthru); default: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1670,21 +1707,21 @@ virSecuritySELinuxRestoreSecurityAllLabel(virSecurityManagerPtr mgr, if (virDomainChrDefForeach(def, false, virSecuritySELinuxRestoreSecurityChardevCallback, - NULL) < 0) + mgr) < 0) rc = -1; if (virDomainSmartcardDefForeach(def, false, virSecuritySELinuxRestoreSecuritySmartcardCallback, - NULL) < 0) + mgr) < 0) rc = -1; if (def->os.kernel && - virSecuritySELinuxRestoreSecurityFileLabel(def->os.kernel) < 0) + virSecuritySELinuxRestoreSecurityFileLabel(mgr, def->os.kernel) < 0) rc = -1; if (def->os.initrd && - virSecuritySELinuxRestoreSecurityFileLabel(def->os.initrd) < 0) + virSecuritySELinuxRestoreSecurityFileLabel(mgr, def->os.initrd) < 0) rc = -1; return rc; @@ -1737,7 +1774,7 @@ virSecuritySELinuxSetSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, static int -virSecuritySELinuxRestoreSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, +virSecuritySELinuxRestoreSavedStateLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, const char *savefile) { @@ -1750,7 +1787,7 @@ virSecuritySELinuxRestoreSavedStateLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNU if (secdef->norelabel) return 0; - return virSecuritySELinuxRestoreSecurityFileLabel(savefile); + return virSecuritySELinuxRestoreSecurityFileLabel(mgr, savefile); } @@ -2080,7 +2117,7 @@ virSecuritySELinuxSetImageFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, } static int -virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, +virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr, virDomainDefPtr def, int fd) { @@ -2108,7 +2145,7 @@ virSecuritySELinuxSetTapFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, goto cleanup; } - if (getContext("/dev/tap.*", buf.st_mode, &fcon) < 0) { + if (getContext(mgr, "/dev/tap.*", buf.st_mode, &fcon) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot lookup default selinux label for tap fd %d"), fd); goto cleanup; -- 1.8.1

2 1

[libvirt] [PATCHv2] qemu: Re-add driver unlock to qemuDomainSendKey
by Viktor Mihajlovski 24 Jan '13

24 Jan '13

Should have been done in commit 56fd513 already, but was missed due to oversight: qemuDomainSendKey didn't release the driver lock in its cleanup section. This fixes an issue introduced by commit 8c5d2ba. Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com> --- V2 Changes: - Removed bogus hunk for qemuDomainManagedSave - Enhanced subject line and patch description src/qemu/qemu_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 72907d2..6d4c1e9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2439,6 +2439,7 @@ endjob: cleanup: if (vm) virObjectUnlock(vm); + qemuDriverUnlock(driver); return ret; } -- 1.7.9.5

3 3

[libvirt] Non-blocking virStreamRecv always returns -2 (EAGAIN)?
by John Eckersberg 24 Jan '13

24 Jan '13

I'm running the following test program, and it works as written with a blocking stream. Inside the guest I'm running: [root@f17-minimal ~]# socat /usr/share/dict/words /dev/virtio-ports/org.libguestfs.channel.0 As expected on the client side, I get all the words dumped. However if I swap the virStreamNew lines and instead use the non-blocking stream, the virStreamRecv call always returns -2. >From http://libvirt.org/internals/rpc.html#apiclientdispatch, I see: When no thread is performing any RPC method call, or sending stream data there is still a need to monitor the socket for incoming I/O related to asynchronous events, or stream data receipt. For this task, a watch is registered with the event loop which triggers whenever the socket is readable. This watch is automatically disabled whenever any other thread grabs the buck, and re-enabled when the buck is released. If I understand that correctly, shouldn't the watch be responsible for reading the stream data in this case? Or am I just completely missing something? ----- #include <libvirt.h> int main() { virConnectPtr conn; virDomainPtr dom; virStreamPtr st; char buf[1024+1]; int got = 0; conn = virConnectOpen("qemu+ssh://root@localhost/system"); dom = virDomainLookupByName(conn, "f17-minimal"); /* st = virStreamNew(conn, VIR_STREAM_NONBLOCK); */ st = virStreamNew(conn, 0); virDomainOpenChannel(dom, "org.libguestfs.channel.0", st, 0); while (1) { got = virStreamRecv(st, buf, 1024); switch (got) { case 0: goto finish; case -1: goto free; case -2: puts("Retrying"); sleep(1); continue; } buf[got] = '\0'; puts(buf); } finish: virStreamFinish(st); free: virStreamFree(st); return 0; }

3 2

[libvirt] [PATCH] S390: Enhance memballoon handling for virtio-s390
by Viktor Mihajlovski 24 Jan '13

24 Jan '13

The way in that memory balloon suppression was handled for S390 is flawed for a number or reasons. 1. Just preventing the default balloon to be created in the case of VIR_ARCH_S390[X] is not sufficient. An explicit memballoon element in the guest definition will still be honored, resulting both in a -balloon option and the allocation of a PCI bus address, neither being supported. 2. Prohibiting balloon for S390 altogether at a domain_conf level is no good solution either as there's work in progress on the QEMU side to implement a virtio-balloon device, although in conjunction with a new machine type. Suppressing the balloon should therefore be done at the QEMU driver level depending on the present capabilities. Therefore we remove the conditional suppression of the default balloon in domain_conf.c. Further, we are claiming the memballoon device for virtio-s390 during device address assignment to prevent it from being considered as a PCI device. Finally, we suppress the generation of the balloon command line option if this is a virtio-s390 machine. Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com> --- src/conf/domain_conf.c | 4 +--- src/qemu/qemu_command.c | 9 ++++++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6feded4..325e1ee 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10285,9 +10285,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, def->memballoon = memballoon; VIR_FREE(nodes); - } else if (def->os.arch != VIR_ARCH_S390 && - def->os.arch != VIR_ARCH_S390X) { - /* TODO: currently no balloon support on s390 -> no default balloon */ + } else { if (def->virtType == VIR_DOMAIN_VIRT_XEN || def->virtType == VIR_DOMAIN_VIRT_QEMU || def->virtType == VIR_DOMAIN_VIRT_KQEMU || diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 981c692..3496dbf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -795,7 +795,7 @@ qemuDomainPrimeS390VirtioDevices(virDomainDefPtr def, { /* declare address-less virtio devices to be of address type 'type' - only disks, networks, consoles and controllers for now + only disks, networks, consoles, controllers and memballoon for now */ int i; @@ -825,6 +825,10 @@ qemuDomainPrimeS390VirtioDevices(virDomainDefPtr def, def->controllers[i]->info.type = type; } + if (def->memballoon && + def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO && + def->memballoon->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + def->memballoon->info.type = type; } static void @@ -6956,6 +6960,9 @@ qemuBuildCommandLine(virConnectPtr conn, * NB: Earlier we declared that VirtIO balloon will always be in * slot 0x3 on bus 0x0 */ + if (qemuCapsGet(caps, QEMU_CAPS_VIRTIO_S390) && (def->memballoon)) + def->memballoon->model = VIR_DOMAIN_MEMBALLOON_MODEL_NONE; + if ((def->memballoon) && (def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_NONE)) { if (def->memballoon->model != VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO) { -- 1.7.9.5

2 3

[libvirt] [PATCH] qemu: Move one misplaced driver unlock to the right function
by Viktor Mihajlovski 24 Jan '13

24 Jan '13

This should have been commit 56fd513 already, but was missed by initially. The driver unlock call in the cleanup section of DomainManagedSave does actually belong to DomainSendKey. Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com> --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c28c223..853e35d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2434,6 +2434,7 @@ endjob: cleanup: if (vm) virObjectUnlock(vm); + qemuDriverUnlock(driver); return ret; } @@ -3174,7 +3175,6 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) cleanup: if (vm) virObjectUnlock(vm); - qemuDriverUnlock(driver); VIR_FREE(name); return ret; -- 1.7.9.5

2 5

[libvirt] [PATCH v2] selinux: Only create the selabel_handle once.
by Richard W.M. Jones 24 Jan '13

24 Jan '13

From: "Richard W.M. Jones" <rjones(a)redhat.com> According to Eric Paris this is slightly more efficient because it only loads the regular expressions in libselinux once. --- src/security/security_selinux.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index a3ef728..d4f0595 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -935,20 +935,30 @@ virSecuritySELinuxFSetFilecon(int fd, char *tcon) return 0; } +#if HAVE_SELINUX_LABEL_H + +static struct selabel_handle *seLabelHandle = NULL; + +static int +seLabelHandleOnceInit (void) +{ + seLabelHandle = selabel_open(SELABEL_CTX_FILE, NULL, 0); + return seLabelHandle ? 0 : -1; +} + +VIR_ONCE_GLOBAL_INIT(seLabelHandle) + +#endif + /* Set fcon to the appropriate label for path and mode, or return -1. */ static int getContext(const char *newpath, mode_t mode, security_context_t *fcon) { #if HAVE_SELINUX_LABEL_H - struct selabel_handle *handle = selabel_open(SELABEL_CTX_FILE, NULL, 0); - int ret; - - if (handle == NULL) + if (seLabelHandleInitialize() < 0) return -1; - ret = selabel_lookup_raw(handle, fcon, newpath, mode); - selabel_close(handle); - return ret; + return selabel_lookup_raw(seLabelHandle, fcon, newpath, mode); #else return matchpathcon(newpath, mode, fcon); #endif -- 1.8.1

2 3

[libvirt] [PATCH] selinux: Only create the selabel_handle once.
by Richard W.M. Jones 24 Jan '13

24 Jan '13

From: "Richard W.M. Jones" <rjones(a)redhat.com> According to Eric Paris this is slightly more efficient because it only loads the regular expressions in libselinux once. --- src/security/security_selinux.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index a3ef728..8b88785 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -935,20 +935,26 @@ virSecuritySELinuxFSetFilecon(int fd, char *tcon) return 0; } +#if HAVE_SELINUX_LABEL_H +static struct selabel_handle *sehandle = NULL; +static virOnceControl sehandleonce = VIR_ONCE_CONTROL_INITIALIZER; + +static void +seHandleInit (void) +{ + sehandle = selabel_open(SELABEL_CTX_FILE, NULL, 0); +} +#endif + /* Set fcon to the appropriate label for path and mode, or return -1. */ static int getContext(const char *newpath, mode_t mode, security_context_t *fcon) { #if HAVE_SELINUX_LABEL_H - struct selabel_handle *handle = selabel_open(SELABEL_CTX_FILE, NULL, 0); - int ret; - - if (handle == NULL) + if (virOnce(&sehandleonce, seHandleInit) < 0 || sehandle == NULL) return -1; - ret = selabel_lookup_raw(handle, fcon, newpath, mode); - selabel_close(handle); - return ret; + return selabel_lookup_raw(sehandle, fcon, newpath, mode); #else return matchpathcon(newpath, mode, fcon); #endif -- 1.8.1

3 3

[libvirt] Network stops with CentOS 6.3 guest on Ubuntu 12.10
by Andy Howell 24 Jan '13

24 Jan '13

I'm still have not resolved this, any idea how to debug it? Thanks, Andy Hello, I'm trying to get CentOS 6.3 running under Ubuntu 12.10. When the interface comes up, I can ping another machine for 9 packets. After that it stops working. Bouncing the guest network interface gets it going again for another 10 packets. Monitoring the vnet0 interface on the host, I see ARP requests from the host to the guest are not getting replies. I cleared out iptables to eliminate that. No luck. I have Fedora 17 and Windows 7 running on the machine with no problems. Any ideas? Thanks, Andy -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

1 0

[libvirt] [PATCH] Ensure nodeinfo struct is initialized to zero
by Daniel P. Berrange 24 Jan '13

24 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> When linuxNodeInfoCPUPopulate() method triggered use of an uninitialize value, since it did not initialize the 'sockets' field in the virNodeInfoPtr struct: ==30020== Conditional jump or move depends on uninitialised value(s) ==30020== at 0x5125DBD: linuxNodeInfoCPUPopulate (nodeinfo.c:513) ==30020== by 0x51261A0: nodeGetInfo (nodeinfo.c:884) ==30020== by 0x149B9B10: qemuCapsInit (qemu_capabilities.c:846) ==30020== by 0x14A11B25: qemuCreateCapabilities (qemu_driver.c:424) ==30020== by 0x14A12426: qemuStartup (qemu_driver.c:874) ==30020== by 0x512A7AF: virStateInitialize (libvirt.c:822) ==30020== by 0x40DE04: daemonRunStateInit (libvirtd.c:877) ==30020== by 0x50ADCE5: virThreadHelper (virthreadpthread.c:161) ==30020== by 0x328CA07D14: start_thread (pthread_create.c:308) ==30020== by 0x328C6F246C: clone (clone.S:114) (happened twice) if (socks > nodeinfo->sockets) <--- here nodeinfo->sockets = socks; Rather than doing this for each field, just make the caller memset the entire struct to zero. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/nodeinfo.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 477104f..5b91a12 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -387,11 +387,6 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, char *sysfs_nodedir = NULL; char *sysfs_cpudir = NULL; - nodeinfo->cpus = 0; - nodeinfo->mhz = 0; - nodeinfo->cores = 0; - nodeinfo->nodes = 0; - /* Start with parsing CPU clock speed from /proc/cpuinfo */ while (fgets(line, sizeof(line), cpuinfo) != NULL) { # if defined(__x86_64__) || \ @@ -868,6 +863,8 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) { virArch hostarch = virArchFromHost(); + memset(nodeinfo, 0, sizeof(nodeinfo)); + if (virStrcpyStatic(nodeinfo->model, virArchToString(hostarch)) == NULL) return -1; -- 1.8.1

4 9

[libvirt] [PATCH] Avoid use of free'd memory in auto destroy callback
by Daniel P. Berrange 24 Jan '13

24 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> The autodestroy callback code has the following function called from a hash iterator qemuDriverCloseCallbackRun(void *payload, const void *name, void *opaque) { ... char *uuidstr = name ... dom = closeDef->cb(data->driver, dom, data->conn); if (dom) virObjectUnlock(dom); virHashRemoveEntry(data->driver->closeCallbacks, uuidstr); } The closeDef->cb function may well cause the current callback to be removed, if it shuts down 'dom'. As such the use of 'uuidstr' in virHashRemoveEntry is accessing free'd memory. We must make a copy of the uuid str before invoking the callback to be safe. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/qemu/qemu_conf.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index b21392e..ee48333 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -510,20 +510,25 @@ qemuDriverCloseCallbackRun(void *payload, { struct qemuDriverCloseCallbackData *data = opaque; qemuDriverCloseDefPtr closeDef = payload; - const char *uuidstr = name; unsigned char uuid[VIR_UUID_BUFLEN]; + char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainObjPtr dom; VIR_DEBUG("conn=%p, thisconn=%p, uuid=%s, cb=%p", - closeDef->conn, data->conn, uuidstr, closeDef->cb); + closeDef->conn, data->conn, (const char *)name, closeDef->cb); if (data->conn != closeDef->conn || !closeDef->cb) return; - if (virUUIDParse(uuidstr, uuid) < 0) { - VIR_WARN("Failed to parse %s", uuidstr); + if (virUUIDParse(name, uuid) < 0) { + VIR_WARN("Failed to parse %s", (const char *)name); return; } + /* We need to reformat uuidstr, because closeDef->cb + * might cause the current hash entry to be removed, + * which means 'name' will have been free()d + */ + virUUIDFormat(uuid, uuidstr); if (!(dom = virDomainFindByUUID(&data->driver->domains, uuid))) { VIR_DEBUG("No domain object with UUID %s", uuidstr); -- 1.8.1

3 2

[libvirt] [PATCH] Fix leak of securityDriverNames
by Daniel P. Berrange 24 Jan '13

24 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> When shutting down, the QEMU driver forgot to free the securityDriverNames string list Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/qemu/qemu_driver.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 72907d2..b3d6199 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1134,6 +1134,9 @@ qemuShutdown(void) { VIR_FREE(qemu_driver->saveImageFormat); VIR_FREE(qemu_driver->dumpImageFormat); + for (i = 0 ; qemu_driver->securityDriverNames[i] != NULL ; i++) + VIR_FREE(qemu_driver->securityDriverNames[i]); + VIR_FREE(qemu_driver->securityDriverNames); virSecurityManagerFree(qemu_driver->securityManager); ebtablesContextFree(qemu_driver->ebtables); -- 1.8.1

2 1

[libvirt] ssh not working with libvirt
by varun bhatnagar 24 Jan '13

24 Jan '13

Hi, I am having issue while doing *ssh* to a virtual node through libvirt. The nodes's ip address is 192.168.82.1. When I am giving the command it is giving the following error message: *virsh# connect vbox+ssh://192.168.82.1/session* *Password:* *error: Failed to connect to the hypervisor* *error: End of ifle while reading data: sh: nc: command not found: Input/output error.* Can anyone tell me why am I facing this problem and how to resolve it. Thanks in advance.

2 1

[libvirt] [PATCHv2 0/2] Fix docs for virBitmapParse
by Peter Krempa 23 Jan '13

23 Jan '13

Split out from the other series. Peter Krempa (2): util: Fix docs for virBitmapParse conf: Fix usage of virBitmapParse src/conf/network_conf.c | 2 +- src/util/virbitmap.c | 23 +++++++++++++++-------- 2 files changed, 16 insertions(+), 9 deletions(-) -- 1.8.1.1

2 5

[libvirt] [PATCH] Have a colorful nodedev.rng with vi
by Osier Yang 23 Jan '13

23 Jan '13

Just add the head line to let vi known it's a XML document. --- docs/schemas/nodedev.rng | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng index 7c85815..04d8ee9 100644 --- a/docs/schemas/nodedev.rng +++ b/docs/schemas/nodedev.rng @@ -1,3 +1,4 @@ +<?xml version="1.0"?>  <grammar xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> -- 1.7.7.6

2 3

[libvirt] [PATCH v2] qemu_agent: Ignore expected EOFs
by Michal Privoznik 23 Jan '13

23 Jan '13

https://bugzilla.redhat.com/show_bug.cgi?id=892079 One of my previous patches (f2a4e5f176c408) tried to fix crashing libvirtd on domain detroy. However, we need to copy pattern from qemuProcessHandleMonitorEOF() instead of decrementing reference counter. The rationale for this is, if qemu process is dying due to domain being destroyed, we obtain EOF on both the monitor and agent sockets. However, if the exit is expected, qemuProcessStop is called, which cleans both agent and monitor sockets up. We want qemuAgentClose() to be called iff the EOF is not expected, so we don't leak an FD and memory. --- diff to v1: -handle race with qemuProcessHandleMonitorEOF correctly src/qemu/qemu_process.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2f08215..013b2ca 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -133,14 +133,30 @@ qemuProcessHandleAgentEOF(qemuAgentPtr agent, virObjectLock(vm); priv = vm->privateData; - if (priv->agent == agent && - !virObjectUnref(priv->agent)) + + if (!priv->agent) { + VIR_DEBUG("Agent freed already"); + goto unlock; + } + + if (priv->beingDestroyed) { + VIR_DEBUG("Domain is being destroyed, agent EOF is expected"); + goto unlock; + } + + if (priv->agent == agent) priv->agent = NULL; virObjectUnlock(vm); qemuDriverUnlock(driver); qemuAgentClose(agent); + return; + +unlock: + virObjectUnlock(vm); + qemuDriverUnlock(driver); + return; } -- 1.8.0.2

2 2

[libvirt] [PATCH 0/6] More Coverity changes
by John Ferlan 23 Jan '13

23 Jan '13

Like a gift that keeps on giving... More Coverity changes. Adjust changes from earlier based on Eric's comments/feedback. nodeinfo.c - use sa_assert() instead of comment: https://www.redhat.com/archives/libvir-list/2013-January/msg01578.html virnetclient.c - remove change - the VIR_FREE change resolves this: https://www.redhat.com/archives/libvir-list/2013-January/msg01581.html Two new Coverity warnings popped up today (lxc_driver.c & lxc_process.c). No rhyme or reason, they just were new with no recent change to blame (very strange). I missed the virnettlscontexttest.c and it's VIR_FREE() warning this morning because it's before all the BAD_SIZEOF. The virbitmaptest.c change was on another branch waiting - I just moved it here. One more major change and some fixups still due on prior changes and the number of Coverity warnings goes below 50 of which 35 are related to the sdt macros (started at 297). John Ferlan (6): nodeinfo: Use sa_assert() instead of Coverity error tag tests: Remove VIR_FREE() on static/stack buffer (der.data) lxc_driver: Need to check for vm before calling virDomainUnlock(vm) lxc_process: Avoid passing NULL iface->iname rpc: Revert Coverity tag message virbitmaptest: Resolve Coverity errors src/lxc/lxc_driver.c | 3 ++- src/lxc/lxc_process.c | 14 ++++++++------ src/nodeinfo.c | 4 ++-- src/rpc/virnetclient.c | 1 - tests/virbitmaptest.c | 24 ++++++++++++++++-------- tests/virnettlscontexttest.c | 3 --- 6 files changed, 28 insertions(+), 21 deletions(-) -- 1.7.11.7

3 14

[libvirt] [PATCH v2] parallels: Resolve some resource leaks
by John Ferlan 23 Jan '13

23 Jan '13

Be sure to VIR_FREE(accel) and moved virDomainVideoDefFree() within no_memory label to be consistent Resolve resource leak in parallelsApplyIfaceParams() when the 'oldnet' is allocated locally. Also virCommandFree(cmd) as necessary. --- This is a followup patch to: https://www.redhat.com/archives/libvir-list/2013-January/msg01565.html Changes in v2: Change label from cleanup to error Add calls to virCommandFree() src/parallels/parallels_driver.c | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 1b47246..2b79939 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -271,17 +271,17 @@ parallelsAddVideoInfo(virDomainDefPtr def, virJSONValuePtr value) if (!(tmp = virJSONValueObjectGetString(value, "size"))) { parallelsParseError(); - goto cleanup; + goto error; } if (virStrToLong_ul(tmp, &endptr, 10, &mem) < 0) { parallelsParseError(); - goto cleanup; + goto error; } if (!STREQ(endptr, "Mb")) { parallelsParseError(); - goto cleanup; + goto error; } if (VIR_ALLOC(video) < 0) @@ -304,8 +304,9 @@ parallelsAddVideoInfo(virDomainDefPtr def, virJSONValuePtr value) no_memory: virReportOOMError(); -cleanup: + VIR_FREE(accel); virDomainVideoDefFree(video); +error: return -1; } @@ -1793,7 +1794,7 @@ static int parallelsApplyIfaceParams(parallelsDomObjPtr pdom, { bool create = false; bool is_changed = false; - virCommandPtr cmd; + virCommandPtr cmd = NULL; char strmac[VIR_MAC_STRING_BUFLEN]; int i; @@ -1808,58 +1809,58 @@ static int parallelsApplyIfaceParams(parallelsDomObjPtr pdom, if (!create && oldnet->type != newnet->type) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Changing network type is not supported")); - return -1; + goto error; } if (!STREQ_NULLABLE(oldnet->model, newnet->model)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Changing network device model is not supported")); - return -1; + goto error; } if (!STREQ_NULLABLE(oldnet->data.network.portgroup, newnet->data.network.portgroup)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Changing network portgroup is not supported")); - return -1; + goto error; } if (!virNetDevVPortProfileEqual(oldnet->virtPortProfile, newnet->virtPortProfile)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Changing virtual port profile is not supported")); - return -1; + goto error; } if (newnet->tune.sndbuf_specified) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Setting send buffer size is not supported")); - return -1; + goto error; } if (!STREQ_NULLABLE(oldnet->script, newnet->script)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Setting startup script is not supported")); - return -1; + goto error; } if (!STREQ_NULLABLE(oldnet->filter, newnet->filter)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Changing filter params is not supported")); - return -1; + goto error; } if (newnet->bandwidth != NULL) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Setting bandwidth params is not supported")); - return -1; + goto error; } for (i = 0; i < sizeof(newnet->vlan); i++) { if (((char *)&newnet->vlan)[i] != 0) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Setting vlan params is not supported")); - return -1; + goto error; } } @@ -1901,15 +1902,24 @@ static int parallelsApplyIfaceParams(parallelsDomObjPtr pdom, is_changed = true; } + if (create) + VIR_FREE(oldnet); + if (!create && !is_changed) { /* nothing changed - no need to run prlctl */ + virCommandFree(cmd); return 0; } if (virCommandRun(cmd, NULL)) - return -1; + goto error; return 0; +error: + if (create) + VIR_FREE(oldnet); + virCommandFree(cmd); + return -1; } static int -- 1.7.11.7

2 2

[libvirt] [PATCH] Fix nwfilter driver reload/shutdown handling when unprivileged
by Daniel P. Berrange 23 Jan '13

23 Jan '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> Although the nwfilter driver skips startup when running in a session libvirtd, it did not skip reload or shutdown. This caused errors to be reported when sending SIGHUP to libvirtd, and caused an abort() in libdbus on shutdown due to trying to remove a dbus filter that was never added NB, this was previously part of my large refactoring series https://www.redhat.com/archives/libvir-list/2013-January/msg01411.html but I've pulled it out since I noticed it has SEGV impact. --- src/conf/nwfilter_conf.h | 1 + src/nwfilter/nwfilter_driver.c | 31 +++++++++++++++++++------------ 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h index ca2c070..35f8dde 100644 --- a/src/conf/nwfilter_conf.h +++ b/src/conf/nwfilter_conf.h @@ -556,6 +556,7 @@ typedef struct _virNWFilterDriverState virNWFilterDriverState; typedef virNWFilterDriverState *virNWFilterDriverStatePtr; struct _virNWFilterDriverState { virMutex lock; + bool privileged; virNWFilterObjList nwfilters; diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index b67b704..19ebb03 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -165,7 +165,7 @@ nwfilterDriverInstallDBusMatches(DBusConnection *sysbus ATTRIBUTE_UNUSED) * Initialization function for the QEmu daemon */ static int -nwfilterDriverStartup(bool privileged ATTRIBUTE_UNUSED, +nwfilterDriverStartup(bool privileged, virStateInhibitCallback callback ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { @@ -185,6 +185,7 @@ nwfilterDriverStartup(bool privileged ATTRIBUTE_UNUSED, goto err_free_driverstate; driverState->watchingFirewallD = (sysbus != NULL); + driverState->privileged = privileged; if (!privileged) return 0; @@ -275,6 +276,9 @@ nwfilterDriverReload(void) { return -1; } + if (!driverState->privileged) + return 0; + conn = virConnectOpen("qemu:///system"); if (conn) { @@ -333,21 +337,24 @@ nwfilterDriverShutdown(void) { if (!driverState) return -1; - virNWFilterConfLayerShutdown(); - virNWFilterTechDriversShutdown(); - virNWFilterDHCPSnoopShutdown(); - virNWFilterLearnShutdown(); - virNWFilterIPAddrMapShutdown(); + if (driverState->privileged) { + virNWFilterConfLayerShutdown(); + virNWFilterTechDriversShutdown(); + virNWFilterDHCPSnoopShutdown(); + virNWFilterLearnShutdown(); + virNWFilterIPAddrMapShutdown(); - nwfilterDriverLock(driverState); + nwfilterDriverLock(driverState); - nwfilterDriverRemoveDBusMatches(); + nwfilterDriverRemoveDBusMatches(); - /* free inactive nwfilters */ - virNWFilterObjListFree(&driverState->nwfilters); + /* free inactive nwfilters */ + virNWFilterObjListFree(&driverState->nwfilters); + + VIR_FREE(driverState->configDir); + nwfilterDriverUnlock(driverState); + } - VIR_FREE(driverState->configDir); - nwfilterDriverUnlock(driverState); virMutexDestroy(&driverState->lock); VIR_FREE(driverState); -- 1.8.0.2

2 1

[libvirt] [PATCH 0/2] Fix parsing of vCPU counts from domain XMLs.
by Peter Krempa 23 Jan '13

23 Jan '13

These patches fix the parsing and the returned error if parsing of vCPUs failed. If a overflowing number was used for the count, the domain might disappear after the restart of the daemon. Peter Krempa (2): conf: Check if number of vCPUs fits in the storage variable conf: Improve error messages if parsing of vCPU count fails src/conf/domain_conf.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) -- 1.8.1.1

4 8

[libvirt] [RFC PATCH v1 0/7] virCgroup refactor
by Hu Tao 23 Jan '13

23 Jan '13

Hi, This series is posted for early review. This series refactors virCgroup. The changes are: - virCgroupItem is associated with a cgroup directory. The directory is created only when needed, and removed if no one is using it. - Anyone using cgroups creates instances of virCgroupItem and maintains their lifetime. Please focus on patch #5, which brings the main change(virCgroupItem), and qemu part in patch #6, which shows the usage of virCgroupItem(I've not tested lxc part yet). Hu Tao (7): call virstateCleanup to do the cleanup before libvirtd exits include util.h in cgroup.h include virterror_internal.h in threads.h refactor virCgroupDetectMounts and virCgroupDetectPlacement cgroup: refactor virCgroup deploy the newly introduced virCgroupItem. tests for virCgroup. daemon/libvirtd.c | 6 + src/conf/domain_audit.c | 16 +- src/conf/domain_audit.h | 6 +- src/conf/domain_conf.h | 5 + src/libvirt_private.syms | 17 +- src/lxc/lxc_cgroup.c | 91 ++- src/lxc/lxc_conf.h | 2 +- src/lxc/lxc_driver.c | 268 +++----- src/lxc/lxc_process.c | 56 +- src/qemu/qemu_cgroup.c | 287 +++------ src/qemu/qemu_cgroup.h | 17 +- src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_driver.c | 478 +++++--------- src/qemu/qemu_hotplug.c | 44 +- src/qemu/qemu_migration.c | 36 +- src/qemu/qemu_process.c | 29 +- src/util/vircgroup.c | 1570 ++++++++++++++++++++------------------------- src/util/vircgroup.h | 108 ++-- src/util/virthread.h | 1 + tests/Makefile.am | 5 + tests/vircgrouptest.c | 103 +++ 21 files changed, 1338 insertions(+), 1809 deletions(-) create mode 100644 tests/vircgrouptest.c -- 1.8.0.1.240.ge8a1f5a

3 21

[libvirt] [libvirt-designer 1/4] virtxml: Make global var local
by Christophe Fergeau 23 Jan '13

23 Jan '13

OsinfoLoader is declared as a global variable in virtxml.c but is only used in one function, so it can be made local. --- examples/virtxml.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/virtxml.c b/examples/virtxml.c index 2918ee0..18b0456 100644 --- a/examples/virtxml.c +++ b/examples/virtxml.c @@ -35,7 +35,6 @@ GList *disk_str_list = NULL; GList *iface_str_list = NULL; -OsinfoLoader *loader = NULL; OsinfoDb *db = NULL; #define print_error(...) \ @@ -65,6 +64,7 @@ load_osinfo(void) { GError *err = NULL; gboolean ret = FALSE; + OsinfoLoader *loader = NULL; loader = osinfo_loader_new(); osinfo_loader_process_default_path(loader, &err); -- 1.8.1

2 8

[libvirt] Error connecting to virConnectOpen("qemu:///system"); after 20 times
by Pankaj Rawat 23 Jan '13

23 Jan '13

Hi, I am using libvirt api to create an app, I am facing a problem. I can connect to connection object 20 number of times in programm after that it gives connection object null. Here is the sample code. Normally code should run 1000 times but it only runs 20 times Filename: list.cpp #include<pthread.h> #include<stdlib.h> #include<stdio.h> #include<malloc.h> #include <iostream> #include<string.h> #include <libvirt/libvirt.h> #include <libvirt/virterror.h> static int count; static void customErrorFunc(void *userdata, virErrorPtr err) { } int main() { virConnectPtr conn; for ( int i=0; i< 1000;i++) { conn = virConnectOpen("qemu:///system"); if (conn == NULL) { fprintf(stderr, "Failed to open connection to qemu:///system\n"); printf("Failed to open connection to qemu:///system\n"); virConnectClose(conn); return NULL; } printf("Hello %d\n",i); virError err; virSetErrorFunc(NULL, customErrorFunc); int j; int numDomains1,numDomains2,numDomains; int *activeDomains; numDomains1 = virConnectNumOfDomains(conn); activeDomains = (int*)malloc(sizeof(int) * numDomains1); numDomains1 = virConnectListDomains(conn, activeDomains, numDomains1); for (int j = 0 ; j < numDomains1 ; j++) { virDomainPtr dom; dom = virDomainLookupByID(conn,activeDomains[j]); free(dom); } free(activeDomains); count--; virConnectClose(conn); } } Output: Hello 0 Hello 1 Hello 2 Hello 3 Hello 4 Hello 5 Hello 6 Hello 7 Hello 8 Hello 9 Hello 10 Hello 11 Hello 12 Hello 13 Hello 14 Hello 15 Hello 16 Hello 17 Hello 18 Hello 19 Failed to open connection to qemu:///system Failed to open connection to qemu:///system Can any one tell me where I am going wrong. Regards, Pankaj Rawat NECHCL ST DISCLAIMER: ----------------------------------------------------------------------------------------------------------------------- The contents of this e-mail and any attachment(s) are confidential and intended for the named recipient(s) only. It shall not attach any liability on the originator or NECHCL or its affiliates. Any views or opinions presented in this email are solely those of the author and may not necessarily reflect the opinions of NECHCL or its affiliates. Any form of reproduction, dissemination, copying, disclosure, modification, distribution and / or publication of this message without the prior written consent of the author of this e-mail is strictly prohibited. If you have received this email in error please delete it and notify the sender immediately. . -----------------------------------------------------------------------------------------------------------------------

2 2

[libvirt] [PATCH] python: Fix bindings for virDomainSnapshotGetDomain
by Jiri Denemark 23 Jan '13

23 Jan '13

https://bugzilla.redhat.com/show_bug.cgi?id=895882 virDomainSnapshotGetDomain returns snapshot->domain without incrementing domain's reference counter. The autogenerated python wrapper around this API did not honor this fact and created a new virDomain object from the snapshot's domain. When this object is deleted, it calls virDomainFree. This caused python client to crash when the domain object is accessed after it has been freed. --- python/generator.py | 1 + python/libvirt-override.c | 24 ++++++++++++++++++++++++ src/libvirt.c | 6 +++++- 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/python/generator.py b/python/generator.py index f853d77..7e76c2a 100755 --- a/python/generator.py +++ b/python/generator.py @@ -379,6 +379,7 @@ skip_impl = ( 'virConnectListDefinedInterfaces', 'virConnectListNWFilters', 'virDomainSnapshotListNames', + 'virDomainSnapshotGetDomain', 'virDomainSnapshotListChildrenNames', 'virConnGetLastError', 'virGetLastError', diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 8154024..92dc939 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -2212,6 +2212,29 @@ cleanup: } static PyObject * +libvirt_virDomainSnapshotGetDomain(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + virDomainPtr dom; + virDomainSnapshotPtr snapshot; + PyObject *pyobj_snapshot; + + if (!PyArg_ParseTuple(args, (char *)"O:virDomainSnapshotGetDomain", + &pyobj_snapshot)) + return NULL; + snapshot = PyvirDomainSnapshot_Get(pyobj_snapshot); + + LIBVIRT_BEGIN_ALLOW_THREADS; + dom = virDomainSnapshotGetDomain(snapshot); + LIBVIRT_END_ALLOW_THREADS; + + if (dom) + virDomainRef(dom); + + return libvirt_virDomainPtrWrap(dom); +} + +static PyObject * libvirt_virDomainListAllSnapshots(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { @@ -6674,6 +6697,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virConnectBaselineCPU", libvirt_virConnectBaselineCPU, METH_VARARGS, NULL}, {(char *) "virDomainGetJobInfo", libvirt_virDomainGetJobInfo, METH_VARARGS, NULL}, {(char *) "virDomainSnapshotListNames", libvirt_virDomainSnapshotListNames, METH_VARARGS, NULL}, + {(char *) "virDomainSnapshotGetDomain", libvirt_virDomainSnapshotGetDomain, METH_VARARGS, NULL}, {(char *) "virDomainListAllSnapshots", libvirt_virDomainListAllSnapshots, METH_VARARGS, NULL}, {(char *) "virDomainSnapshotListChildrenNames", libvirt_virDomainSnapshotListChildrenNames, METH_VARARGS, NULL}, {(char *) "virDomainSnapshotListAllChildren", libvirt_virDomainSnapshotListAllChildren, METH_VARARGS, NULL}, diff --git a/src/libvirt.c b/src/libvirt.c index d5d561c..5f36934 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17850,7 +17850,11 @@ virDomainSnapshotGetName(virDomainSnapshotPtr snapshot) * virDomainSnapshotGetDomain: * @snapshot: a snapshot object * - * Get the domain that a snapshot was created for + * Get the domain that a snapshot was created for. The caller must not call + * virDomainFree() on the returned domain unless it calls virDomainRef() first + * as this function does not increment domain's reference counter. The returned + * object will be automatically freed with the end of life of the snapshot + * object. * * Returns the domain or NULL. */ -- 1.8.1.1

2 1

[libvirt] [PATCH] build: allow virObject to have no parent
by Eric Blake 23 Jan '13

23 Jan '13

When building with static analysis enabled, we turn on attribute nonnull checking. However, this caused the build to fail with: ../../src/util/virobject.c: In function 'virObjectOnceInit': ../../src/util/virobject.c:55:40: error: null argument where non-null required (argument 1) [-Werror=nonnull] Creation of the virObject class is the one instance where the parent class is allowed to be NULL, so making things conditional will let us keep static analysis checking for all other callers, without breaking the build on the one exception. * src/util/virobject.c: Define witness. * src/util/virobject.h (virClassNew): Use it to force most callers to pass non-null parameter. --- I couldn't bring myself to push this under the build breaker rule, so I'll wait for a review. src/util/virobject.c | 3 ++- src/util/virobject.h | 9 +++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/util/virobject.c b/src/util/virobject.c index 7f08a11..3102e5e 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -1,7 +1,7 @@ /* * virobject.c: libvirt reference counted object * - * Copyright (C) 2012 Red Hat, Inc. + * Copyright (C) 2012-2013 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 @@ -21,6 +21,7 @@ #include <config.h> +#define VIR_OBJECT_C #include "virobject.h" #include "virthread.h" #include "viralloc.h" diff --git a/src/util/virobject.h b/src/util/virobject.h index bb72a25..02c409f 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -1,7 +1,7 @@ /* * virobject.h: libvirt reference counted object * - * Copyright (C) 2012 Red Hat, Inc. + * Copyright (C) 2012-2013 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 @@ -51,11 +51,16 @@ struct _virObjectLockable { virClassPtr virClassForObject(void); virClassPtr virClassForObjectLockable(void); +#ifndef VIR_OBJECT_C +# define VIR_PARENT_REQUIRED ATTRIBUTE_NONNULL(1) +#else +# define VIR_PARENT_REQUIRED /* empty */ +#endif virClassPtr virClassNew(virClassPtr parent, const char *name, size_t objectSize, virObjectDisposeCallback dispose) - ATTRIBUTE_NONNULL(1); + VIR_PARENT_REQUIRED; const char *virClassName(virClassPtr klass) ATTRIBUTE_NONNULL(1); -- 1.8.1

2 2

[libvirt] [libvirt-glib][PATCH] Don't redefine _FORTIFY_SOURCE macro
by Michal Privoznik 23 Jan '13

23 Jan '13

If the _FORTIFY_SOURCE has been already defined, we unconditionally redefine it, leaving us with warning/error thrown at compilation time. --- m4/virt-compile-warnings.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 99d214e..d07872a 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -87,7 +87,7 @@ AC_DEFUN([LIBVIRT_GLIB_COMPILE_WARNINGS],[ AH_VERBATIM([FORTIFY_SOURCE], [/* Enable compile-time and run-time bounds-checking, and some warnings, without upsetting newer glibc. */ - #if defined __OPTIMIZE__ && __OPTIMIZE__ + #if defined __OPTIMIZE__ && __OPTIMIZE__ && !defined _FORTIFY_SOURCE # define _FORTIFY_SOURCE 2 #endif ]) -- 1.8.0.2

2 2

[libvirt] [PATCH] util: Remove ATTRIBUTE_NONNULL from virClassNew()
by John Ferlan 23 Jan '13

23 Jan '13

When compiling with Coverity, the STATIC_ANALYSIS was set to true which triggered the ATTRIBUTE_NONNULL check from src/internal.h. Since virClassNew now expects to be called with NULL under a special condition, this check is no longer valid. --- src/util/virobject.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virobject.h b/src/util/virobject.h index bb72a25..f58a950 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -54,8 +54,7 @@ virClassPtr virClassForObjectLockable(void); virClassPtr virClassNew(virClassPtr parent, const char *name, size_t objectSize, - virObjectDisposeCallback dispose) - ATTRIBUTE_NONNULL(1); + virObjectDisposeCallback dispose); const char *virClassName(virClassPtr klass) ATTRIBUTE_NONNULL(1); -- 1.7.11.7

2 1

[libvirt] [PATCH v2] qemu: Support ram bar size for qxl devices
by Alon Levy 23 Jan '13

23 Jan '13

Adds a "ram" attribute globally to the video.model element, that changes the resulting qemu command line only if video.type == "qxl". <video> <model type='qxl' ram='65536' vram='65536' heads='1'/> </video> That attribute gets a default value of 64*1024. The schema is unchanged for other video element types. The resulting qemu command line change is the addition of -global qxl-vga.ram_size=<ram>*1024 or -global qxl.ram_size=<ram>*1024 For the main and secondary qxl devices respectively. The default for the qxl ram bar is 64*1024 kilobytes (the same as the default qxl vram bar size). --- changes from v1: renamed attribute to ram (Eric Blake) fixes as given by Eric Blake Did not add any of the suggestions that Gerd made, I think it can be done in a separate patch and there was no discussion on it yet. docs/formatdomain.html.in | 5 +- docs/schemas/domaincommon.rng | 36 +++++++++----- src/conf/domain_conf.c | 19 +++++++- src/conf/domain_conf.h | 3 +- src/qemu/qemu_command.c | 57 +++++++++++++++------- .../qemuxml2argv-graphics-spice-compression.args | 2 +- .../qemuxml2argv-graphics-spice-compression.xml | 4 +- .../qemuxml2argv-graphics-spice-qxl-vga.args | 3 +- .../qemuxml2argv-graphics-spice-qxl-vga.xml | 4 +- .../qemuxml2argv-graphics-spice.args | 3 +- .../qemuxml2argv-graphics-spice.xml | 4 +- .../qemuxml2argv-video-device-pciaddr-default.args | 6 +-- 12 files changed, 102 insertions(+), 44 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index bb0b199..370de99 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3565,7 +3565,10 @@ qemu-kvm -net nic,model=? /dev/null video device in domain xml is the primary one, but the optional attribute <code>primary</code> (<span class="since">since 1.0.2</span>) with value 'yes' can be used to mark the primary in cases of mutiple - video device. The non-primary must be type of "qxl". + video device. The non-primary must be type of "qxl". The optional + attribute <code>ram</code> is allowed for "qxl" type only and specify + the size of the primary bar, while <code>vram</code> specifies the + secondary bar size. If "ram" is not supplied a default value is used. </dd> <dt><code>model</code></dt> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 67ae864..6aca97a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2251,22 +2251,34 @@ </define>  <define name="video"> <element name="video"> <optional> - <element name="model"> - <attribute name="type"> - <choice> - <value>vga</value> - <value>cirrus</value> - <value>vmvga</value> - <value>xen</value> - <value>vbox</value> - <value>qxl</value> - </choice> - </attribute> + <element name="model"> + <choice> + <attribute name="type"> + <choice> + <value>vga</value> + <value>cirrus</value> + <value>vmvga</value> + <value>xen</value> + <value>vbox</value> + </choice> + </attribute> + <group> + <attribute name="type"> + <value>qxl</value> + </attribute> + <optional> + <attribute name="ram"> + <ref name="unsignedInt"/> + </attribute> + </optional> + </group> + </choice> <optional> <attribute name="vram"> <ref name="unsignedInt"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 13c14e9..e490093 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7391,6 +7391,7 @@ virDomainVideoDefParseXML(const xmlNodePtr node, char *type = NULL; char *heads = NULL; char *vram = NULL; + char *ram = NULL; char *primary = NULL; if (VIR_ALLOC(def) < 0) { @@ -7401,9 +7402,10 @@ virDomainVideoDefParseXML(const xmlNodePtr node, cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { - if (!type && !vram && !heads && + if (!type && !vram && !ram && !heads && xmlStrEqual(cur->name, BAD_CAST "model")) { type = virXMLPropString(cur, "type"); + ram = virXMLPropString(cur, "ram"); vram = virXMLPropString(cur, "vram"); heads = virXMLPropString(cur, "heads"); @@ -7431,6 +7433,18 @@ virDomainVideoDefParseXML(const xmlNodePtr node, } } + if (ram) { + if (virStrToLong_ui(ram, NULL, 10, &def->ram) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse video ram '%s'"), ram); + goto error; + } + } else { + if (def->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { + def->ram = virDomainVideoDefaultRAM(dom, def->type); + } + } + if (vram) { if (virStrToLong_ui(vram, NULL, 10, &def->vram) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -7455,6 +7469,7 @@ virDomainVideoDefParseXML(const xmlNodePtr node, goto error; VIR_FREE(type); + VIR_FREE(ram); VIR_FREE(vram); VIR_FREE(heads); @@ -13383,6 +13398,8 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAddLit(buf, " <video>\n"); virBufferAsprintf(buf, " <model type='%s'", model); + if (def->ram) + virBufferAsprintf(buf, " ram='%u'", def->ram); if (def->vram) virBufferAsprintf(buf, " vram='%u'", def->vram); if (def->heads) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ce36003..af9a4bb 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1157,7 +1157,8 @@ struct _virDomainVideoAccelDef { struct _virDomainVideoDef { int type; - unsigned int vram; + unsigned int ram; /* kilobytes */ + unsigned int vram; /* kilobytes */ unsigned int heads; bool primary; virDomainVideoAccelDefPtr accel; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 981c692..712b54e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3558,11 +3558,20 @@ qemuBuildDeviceVideoStr(virDomainVideoDefPtr video, if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { if (video->vram > (UINT_MAX / 1024)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + virReportError(VIR_ERR_OVERFLOW, _("value for 'vram' must be less than '%u'"), UINT_MAX / 1024); goto error; } + if (video->ram > (UINT_MAX / 1024)) { + virReportError(VIR_ERR_OVERFLOW, + _("value for 'ram' must be less than '%u'"), + UINT_MAX / 1024); + goto error; + } + + /* QEMU accepts bytes for ram_size. */ + virBufferAsprintf(&buf, ",ram_size=%u", video->ram * 1024); /* QEMU accepts bytes for vram_size. */ virBufferAsprintf(&buf, ",vram_size=%u", video->vram * 1024); @@ -6568,24 +6577,36 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgList(cmd, "-vga", vgastr, NULL); - if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { - if (def->videos[0]->vram && - qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { - if (def->videos[0]->vram > (UINT_MAX / 1024)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("value for 'vram' must be less than '%u'"), - UINT_MAX / 1024); - goto error; - } + if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_QXL && + (def->videos[0]->vram || def->videos[0]->ram) && + qemuCapsGet(caps, QEMU_CAPS_DEVICE)) { + const char *dev = (qemuCapsGet(caps, QEMU_CAPS_DEVICE_QXL_VGA) + ? "qxl-vga" : "qxl"); + int ram = def->videos[0]->ram; + int vram = def->videos[0]->vram; + + if (vram > (UINT_MAX / 1024)) { + virReportError(VIR_ERR_OVERFLOW, + _("value for 'vram' must be less than '%u'"), + UINT_MAX / 1024); + goto error; + } + if (ram > (UINT_MAX / 1024)) { + virReportError(VIR_ERR_OVERFLOW, + _("value for 'ram' must be less than '%u'"), + UINT_MAX / 1024); + goto error; + } + if (ram) { virCommandAddArg(cmd, "-global"); - - if (qemuCapsGet(caps, QEMU_CAPS_DEVICE_QXL_VGA)) - virCommandAddArgFormat(cmd, "qxl-vga.vram_size=%u", - def->videos[0]->vram * 1024); - else - virCommandAddArgFormat(cmd, "qxl.vram_size=%u", - def->videos[0]->vram * 1024); + virCommandAddArgFormat(cmd, "%s.ram_size=%u", + dev, ram * 1024); + } + if (vram) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.vram_size=%u", + dev, vram * 1024); } } } @@ -9247,6 +9268,8 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, else vid->type = video; vid->vram = virDomainVideoDefaultRAM(def, vid->type); + vid->ram = vid->type == VIR_DOMAIN_VIDEO_TYPE_QXL ? + virDomainVideoDefaultRAM(def, vid->type) : 0; vid->heads = 1; if (VIR_REALLOC_N(def->videos, def->nvideos+1) < 0) { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args index 5c5912b..2f537df 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args @@ -5,5 +5,5 @@ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ x509-dir=/etc/pki/libvirt-spice,\ image-compression=auto_glz,jpeg-wan-compression=auto,zlib-glz-wan-compression=auto,\ playback-compression=on,streaming-video=filter -vga \ -qxl -global qxl.vram_size=18874368 -device qxl,id=video1,vram_size=33554432,bus=pci.0,addr=0x4 \ +qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368 -device qxl,id=video1,ram_size=67108864,vram_size=33554432,bus=pci.0,addr=0x4 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.xml index 52eb5b9..a8c4ad8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.xml @@ -31,10 +31,10 @@ <streaming mode='filter'/> </graphics> <video> - <model type='qxl' vram='18432' heads='1'/> + <model type='qxl' ram='65536' vram='18432' heads='1'/> </video> <video> - <model type='qxl' vram='32768' heads='1'/> + <model type='qxl' ram='65536' vram='32768' heads='1'/> </video> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args index 3954c03..ef499e6 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.args @@ -3,5 +3,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=spice \ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ /dev/HostVG/QEMUGuest1 -spice port=5903,tls-port=5904,addr=127.0.0.1,\ x509-dir=/etc/pki/libvirt-spice,tls-channel=main,plaintext-channel=inputs -vga \ -qxl -global qxl-vga.vram_size=33554432 -device qxl,id=video1,vram_size=67108864,bus=pci.0,addr=0x4 \ +qxl -global qxl-vga.ram_size=67108864 -global qxl-vga.vram_size=33554432 \ +-device qxl,id=video1,ram_size=67108864,vram_size=67108864,bus=pci.0,addr=0x4 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.xml index 49cb8cc..563d371 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-qxl-vga.xml @@ -28,10 +28,10 @@ <channel name='inputs' mode='insecure'/> </graphics> <video> - <model type='qxl' vram='32768' heads='1'/> + <model type='qxl' ram='65536' vram='32768' heads='1'/> </video> <video> - <model type='qxl' vram='65536' heads='1'/> + <model type='qxl' ram='65536' vram='65536' heads='1'/> </video> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args index 854e723..d7cfae0 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args @@ -5,5 +5,6 @@ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \ x509-dir=/etc/pki/libvirt-spice,tls-channel=default,tls-channel=main,plaintext-channel=inputs,\ image-compression=auto_glz,jpeg-wan-compression=auto,zlib-glz-wan-compression=auto,\ playback-compression=on,streaming-video=filter,disable-copy-paste -vga \ -qxl -global qxl.vram_size=18874368 -device qxl,id=video1,vram_size=33554432,bus=pci.0,addr=0x4 \ +qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368 \ +-device qxl,id=video1,ram_size=67108864,vram_size=33554432,bus=pci.0,addr=0x4 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml index d4939a4..9a36660 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml @@ -34,10 +34,10 @@ <clipboard copypaste='no'/> </graphics> <video> - <model type='qxl' vram='18432' heads='1'/> + <model type='qxl' ram='65536' vram='18432' heads='1'/> </video> <video> - <model type='qxl' vram='32768' heads='1'/> + <model type='qxl' ram='65536' vram='32768' heads='1'/> </video> <memballoon model='virtio'/> </devices> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-device-pciaddr-default.args b/tests/qemuxml2argvdata/qemuxml2argv-video-device-pciaddr-default.args index 9ce852f..4abd7c2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-video-device-pciaddr-default.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-device-pciaddr-default.args @@ -3,7 +3,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ -usb \ -hda /var/lib/libvirt/images/QEMUGuest1 -vnc 127.0.0.1:-5900 \ --device qxl-vga,id=video0,vram_size=67108864,bus=pci.0,addr=0x3 \ --device qxl,id=video1,vram_size=67108864,bus=pci.0,addr=0x4 \ --device qxl,id=video2,vram_size=67108864,bus=pci.0,addr=0x5 \ +-device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,bus=pci.0,addr=0x3 \ +-device qxl,id=video1,ram_size=67108864,vram_size=67108864,bus=pci.0,addr=0x4 \ +-device qxl,id=video2,ram_size=67108864,vram_size=67108864,bus=pci.0,addr=0x5 \ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 -- 1.8.0.1

2 1

[libvirt] [PATCH] virsh-domain: Refactor error paths for cmdCPUStats
by Peter Krempa 23 Jan '13

23 Jan '13

This patch fixes the following issues in the cpu-stats virsh command: 1) Renames label failed_params to no_memory to match coding style 2) Uses proper typed parameter cleanup in error paths to avoid leaks 3) Adds a ret variable and simplifies error labels 4) Changes error message to a slightly more descriptive one and gets rid of the newline at the end: Before: $ virsh cpu-stats tr error: Failed to virDomainGetCPUStats() error: Requested operation is not valid: domain is not running After: $ tools/virsh cpu-stats tr error: Failed to retrieve CPU statistics for domain 'tr' error: Requested operation is not valid: domain is not running --- tools/virsh-domain.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index f4b6622..026dac1 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6109,9 +6109,10 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; virTypedParameterPtr params = NULL; - int i, j, pos, max_id, cpu = -1, show_count = -1, nparams; + int i, j, pos, max_id, cpu = -1, show_count = -1, nparams = 0; bool show_total = false, show_per_cpu = false; unsigned int flags = 0; + bool ret = false; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; @@ -6151,7 +6152,7 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) } if (VIR_ALLOC_N(params, nparams * MIN(show_count, 128)) < 0) - goto failed_params; + goto no_memory; while (show_count) { int ncpus = MIN(show_count, 128); @@ -6199,8 +6200,8 @@ do_show_total: goto cleanup; } - if (VIR_ALLOC_N(params, nparams)) - goto failed_params; + if (VIR_ALLOC_N(params, nparams) < 0) + goto no_memory; /* passing start_cpu == -1 gives us domain's total status */ if ((nparams = virDomainGetCPUStats(dom, params, nparams, -1, 1, flags)) < 0) @@ -6222,22 +6223,22 @@ do_show_total: VIR_FREE(s); } } - virTypedParamsFree(params, nparams); + + ret = true; cleanup: + virTypedParamsFree(params, nparams); virDomainFree(dom); - return true; + return ret; -failed_params: +no_memory: virReportOOMError(); - virDomainFree(dom); - return false; + goto cleanup; failed_stats: - vshError(ctl, _("Failed to virDomainGetCPUStats()\n")); - VIR_FREE(params); - virDomainFree(dom); - return false; + vshError(ctl, _("Failed to retrieve CPU statistics for domain '%s'"), + virDomainGetName(dom)); + goto cleanup; } /* -- 1.8.1.1

2 2