February 2011 - Devel - libvirt List Archives

[libvirt] [PATCH 0/3 PHP] Misc changes to build / git
by Daniel P. Berrange 15 Feb '11

15 Feb '11

This series does some cleanups to GIT and introduces support for automated build testing

3 9

[libvirt] [PATCH] qemu: avoid NULL deref on error
by Eric Blake 15 Feb '11

15 Feb '11

* src/qemu/qemu_command.c (qemuParseCommandLineDisk): Report error before cleaning def. --- Pushing under the trivial rule, as this is a blatant NULL deref. src/qemu/qemu_command.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3ba0950..1687203 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4603,9 +4603,10 @@ qemuParseCommandLineDisk(virCapsPtr caps, host = def->src + strlen("nbd:"); port = strchr(host, ':'); if (!port) { - def = NULL; qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse nbd filename '%s'"), def->src); + _("cannot parse nbd filename '%s'"), + def->src); + def = NULL; goto cleanup; } *port++ = '\0'; -- 1.7.4

2 1

[libvirt] [PATCH] storage: Create enough volumes for mpath pool
by Osier Yang 15 Feb '11

15 Feb '11

"virStorageBackendCreateVols": "names->next" serves as condition expression for "do...while", however, "names" was shifted before, it then results in one less loop, and thus, one less volume will be created for mpath pool, the patch is to fix it. * src/storage/storage_backend_mpath.c --- src/storage/storage_backend_mpath.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index be4db78..2e85561 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -212,6 +212,7 @@ virStorageBackendCreateVols(virStoragePoolObjPtr pool, int retval = -1, is_mpath = 0; char *map_device = NULL; uint32_t minor = -1; + uint32_t next; do { is_mpath = virStorageBackendIsMultipath(names->name); @@ -243,9 +244,10 @@ virStorageBackendCreateVols(virStoragePoolObjPtr pool, /* Given the way libdevmapper returns its data, I don't see * any way to avoid this series of casts. */ - names = (struct dm_names *)(((char *)names) + names->next); + next = names->next; + names = (struct dm_names *)(((char *)names) + next); - } while (names->next); + } while (next); retval = 0; out: -- 1.7.4

2 2

[libvirt] [PATCH] maint: kill dead assignments
by Eric Blake 15 Feb '11

15 Feb '11

* src/network/bridge_driver.c (networkStartNetworkDaemon): Delete unused assignments. --- Detected by clang. Pushing under the trivial rule, as it's pretty easy to see. src/network/bridge_driver.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 08aaa36..c4ee1e8 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1,7 +1,7 @@ /* * bridge_driver.c: core driver methods for managing network * - * Copyright (C) 2006-2010 Red Hat, Inc. + * Copyright (C) 2006-2011 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1586,16 +1586,16 @@ networkStartNetworkDaemon(struct network_driver *driver, } /* Set bridge options */ - if ((err = brSetForwardDelay(driver->brctl, network->def->bridge, - network->def->delay))) { + if (brSetForwardDelay(driver->brctl, network->def->bridge, + network->def->delay)) { networkReportError(VIR_ERR_INTERNAL_ERROR, _("cannot set forward delay on bridge '%s'"), network->def->bridge); goto err1; } - if ((err = brSetEnableSTP(driver->brctl, network->def->bridge, - network->def->stp ? 1 : 0))) { + if (brSetEnableSTP(driver->brctl, network->def->bridge, + network->def->stp ? 1 : 0)) { networkReportError(VIR_ERR_INTERNAL_ERROR, _("cannot set STP '%s' on bridge '%s'"), network->def->stp ? "on" : "off", network->def->bridge); -- 1.7.4

1 0

[libvirt] [PATCH] build: silence some clang warnings
by Eric Blake 15 Feb '11

15 Feb '11

* tools/virsh.c (cmdHelp): Kill dead variables. --- Pushing under the trivial rule. tools/virsh.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index c2d165d..50d5e33 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -589,8 +589,6 @@ static const vshCmdOptDef opts_help[] = { static int cmdHelp(vshControl *ctl, const vshCmd *cmd) { - const vshCmdDef *c; - const vshCmdGrp *g; const char *name; name = vshCommandOptString(cmd, "command", NULL); @@ -615,9 +613,9 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd) return TRUE; } - if ((c = vshCmddefSearch(name))) { + if (vshCmddefSearch(name)) { return vshCmddefHelp(ctl, name); - } else if ((g = vshCmdGrpSearch(name))) { + } else if (vshCmdGrpSearch(name)) { return vshCmdGrpHelp(ctl, name); } else { vshError(ctl, _("command or command group '%s' doesn't exist"), name); -- 1.7.4

1 0

[libvirt] [PATCH] build: silence false positive clang report
by Eric Blake 15 Feb '11

15 Feb '11

clang complained that STREQ(group->controllers[i].mountPoint,...) was a NULL dereference when i==VIR_CGROUP_CONTROLLER_CPUSET, because it assumes the worst about virCgroupPathOfController. Marking the argument const doesn't yet have an effect, per this clang bug: http://llvm.org/bugs/show_bug.cgi?id=7758 So, we use sa_assert, which was designed to shut up false positives from tools like clang. * src/util/cgroup.c (virCgroupMakeGroup): Teach clang that there is no NULL dereference. --- I'm including enough context to show the STREQ that clang complained about. And yes, I'm plowing through clang reports right now - there were less than 20, so it seemed worth tackling before 0.8.8. This one is a one-liner fix (tested by re-running clang and no longer seeing a false positive, and sa_assert() is a no-op for gcc compilation), but I'd rather get an ACK before pushing. src/util/cgroup.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/util/cgroup.c b/src/util/cgroup.c index de1fd8e..47c4633 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -496,24 +496,27 @@ static int virCgroupMakeGroup(virCgroupPtr parent, virCgroupPtr group, VIR_DEBUG("Make group %s", group->path); for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { char *path = NULL; /* Skip over controllers that aren't mounted */ if (!group->controllers[i].mountPoint) continue; rc = virCgroupPathOfController(group, i, "", &path); if (rc < 0) return rc; + /* As of Feb 2011, clang can't see that the above function + * call did not modify group. */ + sa_assert(group->controllers[i].mountPoint); VIR_DEBUG("Make controller %s", path); if (access(path, F_OK) != 0) { if (!create || mkdir(path, 0755) < 0) { rc = -errno; VIR_FREE(path); break; } if (group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint != NULL && (i == VIR_CGROUP_CONTROLLER_CPUSET || STREQ(group->controllers[i].mountPoint, group->controllers[VIR_CGROUP_CONTROLLER_CPUSET].mountPoint))) { -- 1.7.4

2 2

[libvirt] [PATCH] qemu: ignore failure of qemu -M ? on older qemu
by Eric Blake 15 Feb '11

15 Feb '11

https://bugzilla.redhat.com/show_bug.cgi?id=676563 Regression introduced in commit 2211518. * src/qemu/qemu_capabilities.c (qemuCapsProbeMachineTypes): Allow non-zero exit status. --- src/qemu/qemu_capabilities.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ca7d842..cc5552c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -171,6 +171,7 @@ qemuCapsProbeMachineTypes(const char *binary, char *output; int ret = -1; virCommandPtr cmd; + int status; /* Make sure the binary we are about to try exec'ing exists. * Technically we could catch the exec() failure, but that's @@ -186,7 +187,8 @@ qemuCapsProbeMachineTypes(const char *binary, virCommandSetOutputBuffer(cmd, &output); virCommandClearCaps(cmd); - if (virCommandRun(cmd, NULL) < 0) + /* Ignore failure from older qemu that did not understand '-M ?'. */ + if (virCommandRun(cmd, &status) < 0) goto cleanup; if (qemuCapsParseMachineTypesStr(output, machines, nmachines) < 0) -- 1.7.4

2 2

[libvirt] [PATCH] xml: avoid compiler warning
by Eric Blake 15 Feb '11

15 Feb '11

Detected by clang. * src/util/xml.c (virXPathStringLimit): Use %zd, not obsolete %Zd. --- Pushing under the trivial rule; just because glibc treats %Zd as a synonym for %zd does not mean other platforms do likewise, nor that gettext() gracefully handles it. src/util/xml.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/xml.c b/src/util/xml.c index de5e9de..ff340d8 100644 --- a/src/util/xml.c +++ b/src/util/xml.c @@ -105,7 +105,7 @@ virXPathStringLimit(const char *xpath, if (tmp != NULL && strlen(tmp) >= maxlen) { virXMLError(VIR_ERR_INTERNAL_ERROR, - _("\'%s\' value longer than %Zd bytes in virXPathStringLimit()"), + _("\'%s\' value longer than %zd bytes in virXPathStringLimit()"), xpath, maxlen); return NULL; } -- 1.7.4

1 0

[libvirt] [PATCH] nwfilter: reorder match extensions relative to state match
by Stefan Berger 15 Feb '11

15 Feb '11

This patch reorders the connlimit and comment match extensions relative to the state match (-m state); connlimit being most useful if found after a -m state --state NEW and not before it. Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com> --- src/nwfilter/nwfilter_ebiptables_driver.c | 38 ++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c @@ -862,6 +862,7 @@ err_exit: static int iptablesHandleIpHdr(virBufferPtr buf, + virBufferPtr afterStateMatch, virNWFilterHashTablePtr vars, ipHdrDataDefPtr ipHdr, int directionIn, @@ -1005,7 +1006,9 @@ iptablesHandleIpHdr(virBufferPtr buf, &ipHdr->dataConnlimitAbove)) goto err_exit; - virBufferVSprintf(buf, + /* place connlimit after potential -m state --state ... + since this is the most useful order */ + virBufferVSprintf(afterStateMatch, " -m connlimit %s --connlimit-above %s", ENTRY_GET_NEG_SIGN(&ipHdr->dataConnlimitAbove), number); @@ -1016,7 +1019,9 @@ iptablesHandleIpHdr(virBufferPtr buf, if (HAS_ENTRY_ITEM(&ipHdr->dataComment)) { printCommentVar(prefix, ipHdr->dataComment.u.string); - virBufferAddLit(buf, + /* keep comments behind everything else -- they are packet eval. + no-ops */ + virBufferAddLit(afterStateMatch, " -m comment --comment \"$" COMMENT_VARNAME "\""); } @@ -1024,6 +1029,7 @@ iptablesHandleIpHdr(virBufferPtr buf, err_exit: virBufferFreeAndReset(buf); + virBufferFreeAndReset(afterStateMatch); return 1; } @@ -1148,6 +1154,7 @@ _iptablesCreateRuleInstance(int directio char number[20]; virBuffer prefix = VIR_BUFFER_INITIALIZER; virBuffer buf = VIR_BUFFER_INITIALIZER; + virBuffer afterStateMatch = VIR_BUFFER_INITIALIZER; virBufferPtr final = NULL; const char *target; const char *iptables_cmd = (isIPv6) ? ip6tables_cmd_path @@ -1188,6 +1195,7 @@ _iptablesCreateRuleInstance(int directio goto err_exit; if (iptablesHandleIpHdr(&buf, + &afterStateMatch, vars, &rule->p.tcpHdrFilter.ipHdr, directionIn, @@ -1234,6 +1242,7 @@ _iptablesCreateRuleInstance(int directio goto err_exit; if (iptablesHandleIpHdr(&buf, + &afterStateMatch, vars, &rule->p.udpHdrFilter.ipHdr, directionIn, @@ -1267,6 +1276,7 @@ _iptablesCreateRuleInstance(int directio goto err_exit; if (iptablesHandleIpHdr(&buf, + &afterStateMatch, vars, &rule->p.udpliteHdrFilter.ipHdr, directionIn, @@ -1295,6 +1305,7 @@ _iptablesCreateRuleInstance(int directio goto err_exit; if (iptablesHandleIpHdr(&buf, + &afterStateMatch, vars, &rule->p.espHdrFilter.ipHdr, directionIn, @@ -1323,6 +1334,7 @@ _iptablesCreateRuleInstance(int directio goto err_exit; if (iptablesHandleIpHdr(&buf, + &afterStateMatch, vars, &rule->p.ahHdrFilter.ipHdr, directionIn, @@ -1351,6 +1363,7 @@ _iptablesCreateRuleInstance(int directio goto err_exit; if (iptablesHandleIpHdr(&buf, + &afterStateMatch, vars, &rule->p.sctpHdrFilter.ipHdr, directionIn, @@ -1387,6 +1400,7 @@ _iptablesCreateRuleInstance(int directio goto err_exit; if (iptablesHandleIpHdr(&buf, + &afterStateMatch, vars, &rule->p.icmpHdrFilter.ipHdr, directionIn, @@ -1449,6 +1463,7 @@ _iptablesCreateRuleInstance(int directio goto err_exit; if (iptablesHandleIpHdr(&buf, + &afterStateMatch, vars, &rule->p.igmpHdrFilter.ipHdr, directionIn, @@ -1477,6 +1492,7 @@ _iptablesCreateRuleInstance(int directio goto err_exit; if (iptablesHandleIpHdr(&buf, + &afterStateMatch, vars, &rule->p.allHdrFilter.ipHdr, directionIn, @@ -1512,6 +1528,22 @@ _iptablesCreateRuleInstance(int directio rule, &buf); + if (virBufferError(&afterStateMatch)) { + virBufferFreeAndReset(&buf); + virBufferFreeAndReset(&prefix); + virBufferFreeAndReset(&afterStateMatch); + virReportOOMError(); + return -1; + } + + if (virBufferUse(&afterStateMatch)) { + char *s = virBufferContentAndReset(&afterStateMatch); + + virBufferAdd(&buf, s, -1); + + VIR_FREE(s); + } + virBufferVSprintf(&buf, " -j %s" CMD_DEF_POST CMD_SEPARATOR CMD_EXEC, @@ -1553,12 +1585,14 @@ _iptablesCreateRuleInstance(int directio err_exit: virBufferFreeAndReset(&buf); virBufferFreeAndReset(&prefix); + virBufferFreeAndReset(&afterStateMatch); return -1; exit_no_error: virBufferFreeAndReset(&buf); virBufferFreeAndReset(&prefix); + virBufferFreeAndReset(&afterStateMatch); return 0; }

2 2

[libvirt] [PATCH] conf: Fix XMl generation for smartcards
by Jiri Denemark 15 Feb '11

15 Feb '11

When formating XML for smartcard device with mode=host, libvirt generates invalid XML if the device has address info associated: <smartcard mode='host' <address type='ccid' controller='0' slot='1'/> --- src/conf/domain_conf.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 59adf36..f2bb3aa 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7058,6 +7058,7 @@ virDomainSmartcardDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); return 0; } + virBufferAddLit(buf, ">\n"); break; case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES: -- 1.7.4

2 2