February 2013 - Devel - libvirt List Archives

[libvirt] [PATCH] Remove some C99 variable decls in parallels driver
by Daniel P. Berrange 26 Feb '13

26 Feb '13

From: "Daniel P. Berrange" <berrange(a)redhat.com> The parallels storage driver declared some loop variables inside the for(;;). This is not allowed by libvirt coding standards Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> Pushed under build breaker rule - RHEL-5 gcc chokes on them --- src/parallels/parallels_storage.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/parallels/parallels_storage.c b/src/parallels/parallels_storage.c index f696fcb..ac65a1d 100644 --- a/src/parallels/parallels_storage.c +++ b/src/parallels/parallels_storage.c @@ -129,9 +129,11 @@ static char *parallelsMakePoolName(virConnectPtr conn, const char *path) { parallelsConnPtr privconn = conn->privateData; char *name; + unsigned int i; - for (unsigned int i = 0; i < UINT_MAX; i++) { + for (i = 0; i < UINT_MAX; i++) { bool found = false; + int j; if (!(name = strdup(path))) { virReportOOMError(); @@ -148,11 +150,11 @@ static char *parallelsMakePoolName(virConnectPtr conn, const char *path) return 0; } - for (int j = 0; j < strlen(name); j++) + for (j = 0; j < strlen(name); j++) if (name[j] == '/') name[j] = '-'; - for (int j = 0; j < privconn->pools.count; j++) { + for (j = 0; j < privconn->pools.count; j++) { if (STREQ(name, privconn->pools.objs[j]->def->name)) { found = true; break; @@ -226,6 +228,7 @@ parallelsPoolAddByDomain(virConnectPtr conn, virDomainObjPtr dom) virStoragePoolObjListPtr pools = &privconn->pools; char *poolPath; virStoragePoolObjPtr pool = NULL; + int j; if (!(poolPath = strdup(pdom->home))) { virReportOOMError(); @@ -234,7 +237,7 @@ parallelsPoolAddByDomain(virConnectPtr conn, virDomainObjPtr dom) poolPath = dirname(poolPath); - for (int j = 0; j < pools->count; j++) { + for (j = 0; j < pools->count; j++) { if (STREQ(poolPath, pools->objs[j]->def->target.path)) { pool = pools->objs[j]; break; -- 1.8.1.2

1 0

[libvirt] ANNOUNCE: Perl binding Sys-Virt release 1.0.1
by Daniel P. Berrange 26 Feb '13

26 Feb '13

I am pleased to announce that release 1.0.1 of Sys-Virt, the libvirt Perl API binding is now available for download: http://search.cpan.org/CPAN/authors/id/D/DA/DANBERR/Sys-Virt-1.0.1.tar.gz Changed in this release: - Add all new APIs and constants in libvirt 1.0.1 - Fix typo preventing listing of NWFilters - Add more testing of object list APIs - Fix some incorrect error handling tests in binding - Remove bogus compare < 0 for size_t variables - Fix const-ness of functions for populating constants - Add option to turn on more GCC warning flags - Fix typos in POD docs Further information, including online API documentation & previous release archives, is available from the CPAN home page http://search.cpan.org/dist/Sys-Virt/ Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

1 0

[libvirt] [PATCH 0/2] fix some RHEL 5 test failures
by Eric Blake 26 Feb '13

26 Feb '13

A couple of patches that I noticed while testing the release candidate on a RHEL 5 VM. Eric Blake (2): tests: consistent skip messages tests: old automake lacks abs_builddir tests/Makefile.am | 25 ++----------------------- tests/qemumonitorjsontest.c | 2 +- tests/virstoragetest.c | 4 ++-- 3 files changed, 5 insertions(+), 26 deletions(-) -- 1.8.1.2

2 3

[libvirt] [PATCH 0/4] Virtio-RNG followup
by Peter Krempa 26 Feb '13

26 Feb '13

This series adds rate limiting and multiple device support and fixes docs and adds test for XML entities and multiple RNG devices. Peter Krempa (4): virtio-rng: allow multiple RNG devices docs: Fix attribute name for virtio-rng backend virtio-rng: Add rate limiting options for virtio-RNG tests: Test multiple RNG devices and support for XML entities in paths docs/formatdomain.html.in | 14 +++- docs/schemas/domaincommon.rng | 18 ++++- src/conf/domain_conf.c | 78 +++++++++++++--------- src/conf/domain_conf.h | 6 +- src/qemu/qemu_command.c | 28 +++++--- .../qemuxml2argv-virtio-rng-random.args | 2 +- .../qemuxml2argv-virtio-rng-random.xml | 4 ++ 7 files changed, 106 insertions(+), 44 deletions(-) -- 1.8.1.1

2 8

[libvirt] [PATCH] vl.c: Support multiple CPU ranges on -numa option
by Eduardo Habkost 26 Feb '13

26 Feb '13

This allows "," to be used a separator between each CPU range. Note that commas inside key=value command-line options have to be escaped using ",,", so the command-line will look like: -numa node,cpus=A,,B,,C,,D Note that the following format, currently used by libvirt: -numa nodes,cpus=A,B,C,D will _not_ work yet, as "," is the option separator for the command-line option parser, and it will require changing the -numa option parsing code to handle "cpus" as a special case. Signed-off-by: Eduardo Habkost <ehabkost(a)redhat.com> --- vl.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/vl.c b/vl.c index 955d2ff..cd247be 100644 --- a/vl.c +++ b/vl.c @@ -1244,7 +1244,7 @@ char *get_boot_devices_list(size_t *size) return list; } -static void numa_node_parse_cpus(int nodenr, const char *cpus) +static void numa_node_parse_cpu_range(int nodenr, const char *cpus) { char *endptr; unsigned long long value, endvalue; @@ -1288,6 +1288,18 @@ error: exit(1); } +static void numa_node_parse_cpus(int nodenr, const char *option) +{ + char **parts; + int i; + + parts = g_strsplit(option, ",", 0); + for (i = 0; parts[i]; i++) { + numa_node_parse_cpu_range(nodenr, parts[i]); + } + g_strfreev(parts); +} + static void numa_add(const char *optarg) { char option[128]; -- 1.8.1

6 12

[libvirt] [PATCH] qemu: fix graphics port allocation
by Ján Tomko 26 Feb '13

26 Feb '13

Right now, we allocate a port or a TLS port for SPICE if it's set to -1, even if autoport is off. But we only free them if autoport is on. With autoport on, we only allocate TLS port if cfg->spiceTLS is set, but we free it even if it's not, leading to an error message. This patch separates the autoport=yes|no XML option into autoport and tlsAutoport in virDomainGraphicsDef: if autoport is yes, we set them both to 1 (and vice versa) if either of the port numbers is -1, the corresponding autoport is set to 1 and it's used as a condition for acquiring/releasing the port. For TLS ports, cfg->spiceTLS is checked before releasing the port as well. Also check the port number before trying to release it - the vnc port will be 0, the spice port will be -1 or 0 if it hasn't been allocated yet (if qemuProcessStart fails before port allocation). --- src/conf/domain_conf.c | 17 +++++++++++------ src/conf/domain_conf.h | 1 + src/qemu/qemu_hotplug.c | 3 ++- src/qemu/qemu_process.c | 26 ++++++++++++++------------ 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b65e52a..cb27f82 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7083,8 +7083,10 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, } if ((autoport = virXMLPropString(node, "autoport")) != NULL) { - if (STREQ(autoport, "yes")) + if (STREQ(autoport, "yes")) { def->data.spice.autoport = 1; + def->data.spice.tlsAutoport = 1; + } VIR_FREE(autoport); } @@ -7102,12 +7104,14 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, VIR_FREE(defaultMode); } - if (def->data.spice.port == -1 && def->data.spice.tlsPort == -1) { - /* Legacy compat syntax, used -1 for auto-port */ + /* Legacy compat syntax, used -1 for auto-port */ + if (def->data.spice.port == -1) def->data.spice.autoport = 1; - } + if (def->data.spice.tlsPort == -1) + def->data.spice.tlsAutoport = 1; - if (def->data.spice.autoport && (flags & VIR_DOMAIN_XML_INACTIVE)) { + if (def->data.spice.autoport && def->data.spice.tlsAutoport && + (flags & VIR_DOMAIN_XML_INACTIVE)) { def->data.spice.port = 0; def->data.spice.tlsPort = 0; } @@ -14201,7 +14205,8 @@ virDomainGraphicsDefFormat(virBufferPtr buf, def->data.spice.tlsPort); virBufferAsprintf(buf, " autoport='%s'", - def->data.spice.autoport ? "yes" : "no"); + (def->data.spice.autoport && + def->data.spice.tlsAutoport) ? "yes" : "no"); if (listenAddr) virBufferAsprintf(buf, " listen='%s'", listenAddr); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0828954..cc67716 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1356,6 +1356,7 @@ struct _virDomainGraphicsDef { char *keymap; virDomainGraphicsAuthDef auth; unsigned int autoport :1; + unsigned int tlsAutoport :1; int channels[VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST]; int defaultMode; /* enum virDomainGraphicsSpiceChannelMode */ int image; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 78961a7..d8f9007 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1879,9 +1879,10 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver, case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: if ((olddev->data.spice.autoport != dev->data.spice.autoport) || + (olddev->data.spice.tlsAutoport != dev->data.spice.tlsAutoport) || (!dev->data.spice.autoport && (olddev->data.spice.port != dev->data.spice.port)) || - (!dev->data.spice.autoport && + (!dev->data.spice.tlsAutoport && (olddev->data.spice.tlsPort != dev->data.spice.tlsPort))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot change port settings on spice graphics")); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 732964f..7c2a54e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3622,8 +3622,7 @@ int qemuProcessStart(virConnectPtr conn, graphics->data.vnc.port = port; } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { unsigned short port = 0; - if (graphics->data.spice.autoport || - graphics->data.spice.port == -1) { + if (graphics->data.spice.autoport) { if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0) goto cleanup; @@ -3635,9 +3634,7 @@ int qemuProcessStart(virConnectPtr conn, graphics->data.spice.port = port; } - if (cfg->spiceTLS && - (graphics->data.spice.autoport || - graphics->data.spice.tlsPort == -1)) { + if (cfg->spiceTLS && graphics->data.spice.tlsAutoport) { unsigned short tlsPort; if (virPortAllocatorAcquire(driver->remotePorts, &tlsPort) < 0) goto cleanup; @@ -4318,16 +4315,21 @@ retry: for (i = 0 ; i < vm->def->ngraphics; ++i) { virDomainGraphicsDefPtr graphics = vm->def->graphics[i]; if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && - graphics->data.vnc.autoport) { + graphics->data.vnc.autoport && graphics->data.vnc.port) { ignore_value(virPortAllocatorRelease(driver->remotePorts, graphics->data.vnc.port)); } - if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE && - graphics->data.spice.autoport) { - ignore_value(virPortAllocatorRelease(driver->remotePorts, - graphics->data.spice.port)); - ignore_value(virPortAllocatorRelease(driver->remotePorts, - graphics->data.spice.tlsPort)); + if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + if (graphics->data.spice.autoport && + graphics->data.spice.port > 0) { + ignore_value(virPortAllocatorRelease(driver->remotePorts, + graphics->data.spice.port)); + } + if (cfg->spiceTLS && graphics->data.spice.tlsAutoport && + graphics->data.spice.tlsPort > 0) { + ignore_value(virPortAllocatorRelease(driver->remotePorts, + graphics->data.spice.tlsPort)); + } } } -- 1.7.12.4

2 1

[libvirt] [PATCH] libvirt: fix error message when connection can't be opened
by Ján Tomko 26 Feb '13

26 Feb '13

VIR_ERR_NO_CONNECT already contains "no connection driver available". This patch changes: no connection driver available for No connection for URI hello to: no connection driver available for hello Bug: https://bugzilla.redhat.com/show_bug.cgi?id=851413 --- src/libvirt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libvirt.c b/src/libvirt.c index 934997a..8a28e4a 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1216,7 +1216,7 @@ do_open(const char *name, if (!ret->driver) { /* If we reach here, then all drivers declined the connection. */ virLibConnError(VIR_ERR_NO_CONNECT, - _("No connection for URI %s"), + "%s", NULLSTR(name)); goto failed; } -- 1.7.12.4

2 2

[libvirt] [PATCH 0/4] uid_t|gid_t fixes for 32 bit Linux
by Philipp Hahn 26 Feb '13

26 Feb '13

Hello, on 2012-11-23 I already reported a problem with running libvirt on 32 Bit Linuxes (libvirt 0.9.12 on Debian Squeeze in my case) on this ML: [BUG] storage.xml: owner|group=-1 → 2^32-1 on 32 Bit Due to multiple castings the initial -1 gets casted to (unsigned int)4294967295, which failes to be cased to an (signed int) on read-back. Thus the default storage pool is not launched: 2013-02-22 16:50:10.182+0000: 28550: info : libvirt version: 0.9.12 2013-02-22 16:50:10.182+0000: 28550: error : virStorageDefParsePerms:635 : XML error: malformed owner element This is caused by uid_t and gid_t being opaque types: their exact type ranges from s32 to u32 to u64. (Solaris until 2007, Linux, Windows64) The following patch series fixes several issues, which I found while investigating the storage pool startup problem: - mismatch between printf format and types - implicit casts of -1 to uid_t and gid_t - missing casts of uid_t and gid_t for printing - virXPathLong() failing on +(2^32-1) @Guido: for Debian only the 4th patch is relevant, which applies with some re-indention. Additional issues: - UID and GID 4294967295 happens to be the same as -1, so hopefully nobody uses it. Should this be documented somewhere? - Some regression test? Philipp Hahn (4): util: Fix printf format for uid_t|gid_t storage: Cast uid_t|gid_t to unsigned int storage: cast -1 for uid_t|gid_t storage: fix uid_t|gid_t handling on 32 bit Linux src/conf/storage_conf.c | 82 ++++++++++++++++++++++++++++++++--------- src/storage/storage_backend.c | 22 ++++++----- src/util/virutil.c | 10 ++--- 3 files changed, 81 insertions(+), 33 deletions(-) -- 1.7.10.4

4 11

[libvirt] [PATCH] qemu: do not set unpriv_sgio if neither supported nor requested
by Paolo Bonzini 26 Feb '13

26 Feb '13

Currently we call virSetDeviceUnprivSGIO with val == 0 if a block device has an sgio attribute. But for sgio='filtered', we know that a kernel with no unpriv_sgio support will always behave as the user wanted. In this case, there is no need to call the function and report a (bogus) error. Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com> --- src/qemu/qemu_process.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b560d2e..4c152b2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3455,36 +3455,35 @@ qemuProcessReconnectAll(virConnectPtr conn, virQEMUDriverPtr driver) int qemuSetUnprivSGIO(virDomainDiskDefPtr disk) { + char *sysfs_path = NULL; int val = -1; + int ret = 0; /* "sgio" is only valid for block disk; cdrom * and floopy disk can have empty source. */ if (disk->type != VIR_DOMAIN_DISK_TYPE_BLOCK || + disk->device != VIR_DOMAIN_DISK_DEVICE_LUN || !disk->src) return 0; - if (disk->sgio) - val = (disk->sgio == VIR_DOMAIN_DISK_SGIO_UNFILTERED); - - /* Ignore the setting if unpriv_sgio is not supported by the - * kernel, otherwise defaults to filter the SG_IO commands, - * I.E. Set unpriv_sgio to 0. - */ - if (disk->sgio == VIR_DOMAIN_DISK_SGIO_DEFAULT && - disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { - char *sysfs_path = NULL; + sysfs_path = virGetUnprivSGIOSysfsPath(disk->src, NULL); + if (sysfs_path == NULL) + return -1; - if ((sysfs_path = virGetUnprivSGIOSysfsPath(disk->src, NULL)) && - virFileExists(sysfs_path)) - val = 0; - VIR_FREE(sysfs_path); - } + /* By default, filter the SG_IO commands, i.e. set unpriv_sgio to 0. */ + val = (disk->sgio == VIR_DOMAIN_DISK_SGIO_UNFILTERED); - if (val >= 0 && virSetDeviceUnprivSGIO(disk->src, NULL, val) < 0) - return -1; + /* Do not do anything if unpriv_sgio is not supported by the kernel and the + * whitelist is enabled. But if requesting unfiltered access, always call + * virSetDeviceUnprivSGIO, to report an error for unsupported unpriv_sgio. + */ + if ((virFileExists(sysfs_path) || val == 1) && + virSetDeviceUnprivSGIO(disk->src, NULL, val) < 0) + ret = -1; - return 0; + VIR_FREE(sysfs_path); + return ret; } int qemuProcessStart(virConnectPtr conn, -- 1.8.1.2

2 1

[libvirt] RFC: APIs for managing resource groups
by Daniel P. Berrange 26 Feb '13

26 Feb '13

Historically for the QEMU/LXC drivers we've simply put each virtual instance in a dedicated cgroup, under the path $LIBVIRT_CGROUP_LOCATION | +- libvirt | +- qemu | | | +- vm1 | +- vm2 | +- vm3 | +- lxc | +- cont1 +- cont2 +- cont3 for a variety of reasons this nesting sucks. It is too deep causing kernel performance problems, its structure does not easily allow for calculating fixed % shares, it does not allow for grouping of VMs. We need to simplify our layout and also introduce some APIs for the grouping of VMs. I won't go into specifics of a new cgroups layout here, just focus on the question of defining a set of APIs that are generic to any hypervisor, for the purpose of setting up VM resource groups. I'm calling the resource cgroup a "partition", since this is all about partitioning workloads. I anticipate a new top level object and APIs for creating/defining it in the usual manner: typedef struct _virPartition virPartition; typedef virPartition *virPartitionPtr; int virConnectListAllPartitions(virConnectPtr conn, virPartitionPtr **partitions, unsigned int flags); virPartitionPtr virPartitionDefineXML(virConnectPtr conn, const char *xml, unsigned int flags); int virPartitionCreate(virPartitionPtr partition, unsigned int flags); int virPartitionCreateXML(virPartitionPtr partition, const char *xml, unsigned int flags); int virPartitionDestroy(virPartitionPtr partition, unsigned int flags); int virPartitionUndefine(virPartitionPtr partition, unsigned int flags); Then I think we'll duplicate all the APIs for setting resource tunables from virDomainPtr against the new object, so we get int virPartitionGetSchedulerParameters(virPartitionPtr partition, virTypedParameterPtr params, int *nparams, unsigned int flags); int virPartitionSetSchedulerParameters(virPartitionPtr partition, virTypedParameterPtr params, int nparams, unsigned int flags) int virDomainSetBlkioParameters(virDomainPtr domain, virTypedParameterPtr params, int nparams, unsigned int flags); int virDomainGetBlkioParameters(virDomainPtr domain, virTypedParameterPtr params, int *nparams, unsigned int flags); int virDomainSetMemoryParameters(virDomainPtr domain, virTypedParameterPtr params, int nparams, unsigned int flags); int virDomainGetMemoryParameters(virDomainPtr domain, virTypedParameterPtr params, int *nparams, unsigned int flags); int virDomainSetNumaParameters(virDomainPtr domain, virTypedParameterPtr params, int nparams, unsigned int flags); int virDomainGetNumaParameters(virDomainPtr domain, virTypedParameterPtr params, int *nparams, unsigned int flags); Finally we need a way to associate a domain with a partition virPartitionPtr virDomainGetPartition(virDomainPtr dom, unsigned int flags); void virDomainSetPartition(virDomainPtr dom, unsigned int flags); There'd also likely be a new VM XML element <partition name="..partition name..."/> which is what the Get/SetPartition methods would be touching. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

3 3