February 2010 - Devel - libvirt List Archives

[libvirt] [PATCH] Fix compliation of AppArmor related code
by Matthias Bolte 12 Feb '10

12 Feb '10

Broken by the latest commits to remove the virConnectPtr parameter from internal functions. I just pushed this one, as it is an obvious fix. --- src/security/security_apparmor.c | 4 ++-- src/security/virt-aa-helper.c | 8 +++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 23f40f8..db04d5c 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -262,7 +262,7 @@ use_apparmor(void) char *libvirt_daemon = NULL; if (virFileResolveLink("/proc/self/exe", &libvirt_daemon) < 0) { - virSecurityReportError(NULL, VIR_ERR_INTERNAL_ERROR, + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("could not find libvirtd")); return rc; } @@ -295,7 +295,7 @@ AppArmorSecurityDriverProbe(void) } if (!virFileExists(template)) { - virSecurityReportError(NULL, VIR_ERR_INTERNAL_ERROR, + virSecurityReportError(VIR_ERR_INTERNAL_ERROR, _("template \'%s\' does not exist"), template); goto clean; } diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 619c8c3..066e18b 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -690,7 +690,7 @@ get_definition(vahControl * ctl, const char *xmlStr) goto exit; } - ctl->def = virDomainDefParseString(NULL, ctl->caps, xmlStr, 0); + ctl->def = virDomainDefParseString(ctl->caps, xmlStr, 0); if (ctl->def == NULL) { vah_error(ctl, 0, "could not parse XML"); goto exit; @@ -767,8 +767,7 @@ vah_add_file(virBufferPtr buf, const char *path, const char *perms) } static int -file_iterate_cb(virConnectPtr conn ATTRIBUTE_UNUSED, - usbDevice *dev ATTRIBUTE_UNUSED, +file_iterate_cb(usbDevice *dev ATTRIBUTE_UNUSED, const char *file, void *opaque) { virBufferPtr buf = opaque; @@ -844,8 +843,7 @@ get_files(vahControl * ctl) if (usb == NULL) continue; - rc = usbDeviceFileIterate(NULL, usb, - file_iterate_cb, &buf); + rc = usbDeviceFileIterate(usb, file_iterate_cb, &buf); usbFreeDevice(usb); if (rc != 0) goto clean; -- 1.6.3.3

2 1

[libvirt] dname parameter ignored in qemudDomainMigratePrepareTunnel{, 2}
by Jim Meyering 12 Feb '10

12 Feb '10

clang warned about a dead-store in src/qemu/qemu_driver.c, since dname is never used thereafter: http://libvirt.org/git/?p=libvirt.git;a=blob;f=src/qemu/qemu_driver.c;h=0d7… /* Target domain name, maybe renamed. */ dname = dname ? dname : def->name; That stmt was initially added with a following use, if (!vm) vm = virDomainFindByName(&driver->domains, dname); But the use was removed by this commit, rendering the store "dead", and the dname parameter effectively unused: http://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=c26cb9234f4b9fa46d7caa… The same thing happened in both qemudDomainMigratePrepareTunnel and qemudDomainMigratePrepareTunnel2 Shouldn't they be doing something like this instead? if (dname) def->name = dname; But def->name is already allocated. Better free it first. And is dname "known" to be allocated from the heap? That's not clear from the little documentation I've read so far, so I've strdup'd it below: Here's a possible patch: >From b51a39aed62ee5c8db733cecfe6eef0c34a7f989 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 11 Feb 2010 16:46:21 +0100 Subject: [PATCH] qemu_driver.c: honor dname parameter once again Since c26cb9234f4b9fa46d7caa3385ae36704167c53f, the dname parameter has been ignored by these two functions. Use it. * src/qemu/qemu_driver.c (qemudDomainMigratePrepareTunnel): Honor dname parameter once again. (qemudDomainMigratePrepare2): Likewise. --- src/qemu/qemu_driver.c | 14 ++++++++++++-- 1 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0d77d57..9ff712c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7434,7 +7434,12 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, } /* Target domain name, maybe renamed. */ - dname = dname ? dname : def->name; + if (dname) { + VIR_FREE(def->name); + def->name = strdup(dname); + if (def->name == NULL) + goto cleanup; + } if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) goto cleanup; @@ -7660,7 +7665,12 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, } /* Target domain name, maybe renamed. */ - dname = dname ? dname : def->name; + if (dname) { + VIR_FREE(def->name); + def->name = strdup(dname); + if (def->name == NULL) + goto cleanup; + } if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) goto cleanup; -- 1.7.0.rc2.170.gbc565

2 2

[libvirt] [PATCH] Mark all error messages for translation
by Jiri Denemark 12 Feb '10

12 Feb '10

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/conf/storage_conf.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index eb1c9d9..2352637 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1464,7 +1464,7 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools, if (!virFileMatchesNameSuffix(file, def->name, ".xml")) { virStorageReportError(VIR_ERR_INVALID_STORAGE_POOL, - "Storage pool config filename '%s' does not match pool name '%s'", + _("Storage pool config filename '%s' does not match pool name '%s'"), path, def->name); virStoragePoolDefFree(def); return NULL; @@ -1524,7 +1524,7 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools, if (virFileBuildPath(configDir, entry->d_name, NULL, path, PATH_MAX) < 0) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, - "Config filename '%s/%s' is too long", + _("Config filename '%s/%s' is too long"), configDir, entry->d_name); continue; } @@ -1532,7 +1532,7 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools, if (virFileBuildPath(autostartDir, entry->d_name, NULL, autostartLink, PATH_MAX) < 0) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, - "Autostart link path '%s/%s' is too long", + _("Autostart link path '%s/%s' is too long"), autostartDir, entry->d_name); continue; } -- 1.6.6.1

2 2

[libvirt] [PATCH] remote_driver.c: avoid leak on OOM error path
by Jim Meyering 12 Feb '10

12 Feb '10

Any "goto out_of_memory" taken between the vars = qparam_query_parse (query) allocation and the free_qparam_set call would result in a leak. Here's the fix: >From a3a21142e40962c960eae882ac9edcc3bd17cb8b Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 11 Feb 2010 12:51:31 +0100 Subject: [PATCH] remote_driver.c: avoid leak on OOM error path * src/remote/remote_driver.c (doRemoteOpen): Don't leak a qparam_set buffer upon OOM error. --- src/remote/remote_driver.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e17db3d..cc543fc 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2,7 +2,7 @@ * remote_internal.c: driver to provide access to libvirtd running * on a remote machine * - * Copyright (C) 2007-2009 Red Hat, Inc. + * Copyright (C) 2007-2010 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 @@ -370,6 +370,7 @@ doRemoteOpen (virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, int flags) { + struct qparam_set *vars = NULL; int wakeupFD[2] = { -1, -1 }; char *transport_str = NULL; enum { @@ -458,7 +459,6 @@ doRemoteOpen (virConnectPtr conn, * feasibly it might contain variables needed by the real driver, * although that won't be the case for now). */ - struct qparam_set *vars; struct qparam *var; int i; char *query; @@ -551,6 +551,7 @@ doRemoteOpen (virConnectPtr conn, } free_qparam_set (vars); + vars = NULL; } else { /* Probe URI server side */ name = strdup(""); @@ -933,6 +934,8 @@ doRemoteOpen (virConnectPtr conn, out_of_memory: virReportOOMError(); + if (vars) + free_qparam_set (vars); failed: /* Close the socket if we failed. */ -- 1.7.0.rc2.170.gbc565

2 2

[libvirt] [PATCH] Swap position of nmodels and models parameters in cpuDecode()
by Jiri Denemark 12 Feb '10

12 Feb '10

All other libvirt functions use array first and then number of elements in that array. Let's make cpuDecode follow this rule. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/cpu/cpu.c | 6 +++--- src/cpu/cpu.h | 8 ++++---- src/cpu/cpu_x86.c | 4 ++-- src/qemu/qemu_conf.c | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index ae8d37d..e9ecc98 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -119,8 +119,8 @@ cpuCompare(virCPUDefPtr host, int cpuDecode(virCPUDefPtr cpu, const union cpuData *data, - unsigned int nmodels, - const char **models) + const char **models, + unsigned int nmodels) { struct cpuArchDriver *driver; @@ -146,7 +146,7 @@ cpuDecode(virCPUDefPtr cpu, return -1; } - return driver->decode(cpu, data, nmodels, models); + return driver->decode(cpu, data, models, nmodels); } diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 1e04993..7ee0ce6 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -48,8 +48,8 @@ typedef virCPUCompareResult typedef int (*cpuArchDecode) (virCPUDefPtr cpu, const union cpuData *data, - unsigned int nmodels, - const char **models); + const char **models, + unsigned int nmodels); typedef int (*cpuArchEncode) (const virCPUDefPtr cpu, @@ -95,8 +95,8 @@ cpuCompare (virCPUDefPtr host, extern int cpuDecode (virCPUDefPtr cpu, const union cpuData *data, - unsigned int nmodels, - const char **models); + const char **models, + unsigned int nmodels); extern int cpuEncode (const char *arch, diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 7ae7fbf..c6ed078 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -940,8 +940,8 @@ x86GuestData(virCPUDefPtr host, static int x86Decode(virCPUDefPtr cpu, const union cpuData *data, - unsigned int nmodels, - const char **models) + const char **models, + unsigned int nmodels) { int ret = -1; struct x86_map *map; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 3988582..ca32dc0 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1016,7 +1016,7 @@ qemudCapsInitCPU(virCapsPtr caps, cpu->threads = nodeinfo.threads; if (!(data = cpuNodeData(arch)) - || cpuDecode(cpu, data, 0, NULL) < 0) + || cpuDecode(cpu, data, NULL, 0) < 0) goto error; caps->host.cpu = cpu; @@ -2959,7 +2959,7 @@ qemuBuildCpuArgStr(const struct qemud_driver *driver, if (VIR_ALLOC(guest) < 0 || !(guest->arch = strdup(ut->machine))) goto no_memory; - if (cpuDecode(guest, data, ncpus, cpus) < 0) + if (cpuDecode(guest, data, cpus, ncpus) < 0) goto cleanup; virBufferVSprintf(&buf, "%s", guest->model); -- 1.6.6.1

2 2

[libvirt] [PATCH] Fix <cpu> element in domain XML schema
by Jiri Denemark 12 Feb '10

12 Feb '10

The current schema is more permissive than the XML parsing code in libvirt. Precisely, 'match' attribute is optional in schema while in reality its use is bound to <model> element: - <cpu> element without 'match' attribute is allowed only if <topology> element is the only child element of <cpu> - <cpu> element with 'match' attribute requires <model> element to be present; <topology> and <feature> elements are optional Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- docs/schemas/domain.rng | 109 +++++++++++++++++++++++++++-------------------- 1 files changed, 63 insertions(+), 46 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index bb6d00d..c913a50 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1313,52 +1313,69 @@ --> <define name="cpu"> <element name="cpu"> - <optional> - <attribute name="match"> - <choice> - <value>minimum</value> - <value>exact</value> - <value>strict</value> - </choice> - </attribute> - </optional> - <interleave> - <optional> - <element name="model"> - <text/> - </element> - </optional> - <optional> - <element name="topology"> - <attribute name="sockets"> - <ref name="positiveInteger"/> - </attribute> - <attribute name="cores"> - <ref name="positiveInteger"/> - </attribute> - <attribute name="threads"> - <ref name="positiveInteger"/> - </attribute> - </element> - </optional> - <zeroOrMore> - <element name="feature"> - <attribute name="policy"> - <choice> - <value>force</value> - <value>require</value> - <value>optional</value> - <value>disable</value> - <value>forbid</value> - </choice> - </attribute> - <attribute name="name"> - <ref name="featureName"/> - </attribute> - <empty/> - </element> - </zeroOrMore> - </interleave> + <choice> + <ref name="cpuTopology"/> + <group> + <ref name="cpuMatch"/> + <interleave> + <ref name="cpuModel"/> + <optional> + <ref name="cpuTopology"/> + </optional> + <zeroOrMore> + <ref name="cpuFeature"/> + </zeroOrMore> + </interleave> + </group> + </choice> + </element> + </define> + + <define name="cpuMatch"> + <attribute name="match"> + <choice> + <value>minimum</value> + <value>exact</value> + <value>strict</value> + </choice> + </attribute> + </define> + + <define name="cpuModel"> + <element name="model"> + <text/> + </element> + </define> + + <define name="cpuFeature"> + <element name="feature"> + <attribute name="policy"> + <choice> + <value>force</value> + <value>require</value> + <value>optional</value> + <value>disable</value> + <value>forbid</value> + </choice> + </attribute> + <attribute name="name"> + <ref name="featureName"/> + </attribute> + <empty/> + </element> + </define> + + <define name="cpuTopology"> + <element name="topology"> + <attribute name="sockets"> + <ref name="positiveInteger"/> + </attribute> + <attribute name="cores"> + <ref name="positiveInteger"/> + </attribute> + <attribute name="threads"> + <ref name="positiveInteger"/> + </attribute> </element> </define> -- 1.6.6.1

2 2

[libvirt] [PATCH 0/9] New public API for computing baseline CPU
by Jiri Denemark 12 Feb '10

12 Feb '10

Hi, This series introduces a new virConnectBaselineCPU() API call for computing baseline CPU model from a set of host CPU definitions. The call accepts a list of <cpu>...</cpu> xml definitions taken from host capabilities and returns an xml description of a guest CPU which is supported by all hosts. virsh is little bit less picky and accepts a file containing host CPU definitions. It doesn't care what is outside of the <cpu/> tag, which allows for concatenating outputs of virsh capabilities from several hosts and feeding the result to virsh cpu-baseline. This series can also be found on cpu-baseline branch of git://gitorious.org/~jirka/libvirt/jirka-staging.git In practice it works like this: # virsh cpu-baseline /dev/stdin >guest-cpu <<EOF <capabilities> <host> <cpu> <arch>x86_64</arch> <model>core2duo</model> <topology sockets='1' cores='2' threads='1'/> <feature name='lahf_lm'/> <feature name='xtpr'/> <feature name='cx16'/> <feature name='tm2'/> <feature name='est'/> <feature name='vmx'/> </cpu> </host> </capabilities> <cpu> <arch>i686</arch> <model>pentiumpro</model> <topology sockets='1' cores='2' threads='1'/> <feature name='vme'/> <feature name='pse36'/> <feature name='pni'/> <feature name='monitor'/> <feature name='ssse3'/> </cpu> <cpu> <arch>x86_64</arch> <model>phenom</model> </cpu> EOF # cat guest-cpu <cpu match='exact'> <model>qemu32</model> <feature policy='require' name='monitor'/> <feature policy='require' name='pse36'/> </cpu> Jiri Denemark (9): Functions for computing baseline CPU from a set of host CPUs Implement cpuArchBaseline in generic CPU driver Implement cpuArchBaseline in x86 CPU driver virConnectBaselineCPU public API Internal driver API for virConnectBaselineCPU virConnectBaselineCPU public API implementation Wire protocol format and dispatcher for virConnectBaselineCPU Implement cpuBaseline in remote and qemu drivers cpu-baseline command for virsh daemon/remote.c | 25 +++++++ daemon/remote_dispatch_args.h | 1 + daemon/remote_dispatch_prototypes.h | 8 ++ daemon/remote_dispatch_ret.h | 1 + daemon/remote_dispatch_table.h | 5 ++ include/libvirt/libvirt.h.in | 18 +++++ src/cpu/cpu.c | 125 ++++++++++++++++++++++++++++++++++- src/cpu/cpu.h | 23 ++++++- src/cpu/cpu_generic.c | 104 ++++++++++++++++++++++++++++- src/cpu/cpu_x86.c | 88 ++++++++++++++++++++++++- src/driver.h | 6 ++ src/esx/esx_driver.c | 1 + src/libvirt.c | 57 ++++++++++++++++ src/libvirt_private.syms | 2 + src/libvirt_public.syms | 5 ++ src/lxc/lxc_driver.c | 1 + src/opennebula/one_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 1 + src/qemu/qemu_driver.c | 14 ++++ src/remote/remote_driver.c | 32 +++++++++ src/remote/remote_protocol.c | 24 +++++++- src/remote/remote_protocol.h | 20 ++++++ src/remote/remote_protocol.x | 18 +++++- src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vbox/vbox_tmpl.c | 1 + src/xen/xen_driver.c | 1 + tools/virsh.c | 72 ++++++++++++++++++++ tools/virsh.pod | 8 ++ 30 files changed, 658 insertions(+), 7 deletions(-)

3 15

[libvirt] [PATCH] Add cpu_generic.c to the list of translated files
by Jiri Denemark 12 Feb '10

12 Feb '10

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- po/POTFILES.in | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/po/POTFILES.in b/po/POTFILES.in index 3e6440e..6655005 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -12,6 +12,7 @@ src/conf/secret_conf.c src/conf/storage_conf.c src/conf/storage_encryption_conf.c src/cpu/cpu.c +src/cpu/cpu_generic.c src/cpu/cpu_map.c src/cpu/cpu_x86.c src/datatypes.c -- 1.6.6.1

2 1

[libvirt] [PATCH] Fix crash in LXC driver open method when URI has no path
by Daniel P. Berrange 12 Feb '10

12 Feb '10

If giving a lxc:// URI instead of lxc:/// the open method would crash ona NULL pointer * src/lxc/lxc_driver.c: Cope with a NULL URI path --- src/lxc/lxc_driver.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 1a73ca6..6d568a3 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -126,7 +126,8 @@ static virDrvOpenStatus lxcOpen(virConnectPtr conn, return VIR_DRV_OPEN_DECLINED; /* If path isn't '/' then they typoed, tell them correct path */ - if (STRNEQ(conn->uri->path, "/")) { + if (conn->uri->path != NULL && + STRNEQ(conn->uri->path, "/")) { lxcError(VIR_ERR_INTERNAL_ERROR, _("Unexpected LXC URI path '%s', try lxc:///"), conn->uri->path); -- 1.6.5.2

2 1

[libvirt] [PATCH] Fix USB device path formatting mixup
by Daniel P. Berrange 12 Feb '10

12 Feb '10

* src/util/hostusb.c: The device path for a USB device wants the bus/device IDs in decimal not octal --- src/util/hostusb.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/hostusb.c b/src/util/hostusb.c index 3cce66b..bf96539 100644 --- a/src/util/hostusb.c +++ b/src/util/hostusb.c @@ -184,7 +184,7 @@ usbGetDevice(unsigned bus, snprintf(dev->name, sizeof(dev->name), "%.3o:%.3o", dev->bus, dev->dev); snprintf(dev->path, sizeof(dev->path), - USB_DEVFS "%03o/%03o", dev->bus, dev->dev); + USB_DEVFS "%03d/%03d", dev->bus, dev->dev); /* XXX fixme. this should be product/vendor */ snprintf(dev->id, sizeof(dev->id), "%d %d", dev->bus, dev->dev); -- 1.6.5.2

2 1