- Devel - libvirt List Archives

[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 14 Feb '11

14 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 14 Feb '11

14 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 14 Feb '11

14 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 14 Feb '11

14 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 14 Feb '11

14 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

[libvirt] Currently in freeze for 0.8.8, rc2 available
by Daniel Veillard 14 Feb '11

14 Feb '11

Just a reminder that we are in feature freeze for 0.8.8, I just uploaded a new candidate version: ftp://libvirt.org/libvirt/libvirt-0.8.8-rc2.tar.gz it does fix the error messages I had on linux configure, but I'm not sure the issues on Win32 and OS-X are all sorted out yet, so please check. Note that I didn't tried to update the localizations from the fedora side yet so those tarballs may be a little off from that perspective. thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

7 17

[libvirt] Strange periodic failures of the commandtest case
by Daniel P. Berrange 14 Feb '11

14 Feb '11

I'm getting periodic failures of the 'commandtest' case where the diff is > DAEMON:no -- < DAEMON:yes For test cases 'test3' through to 'test15', except 'test'4. Tests 1, 2, 4, 16, 17, 18 are all unaffected. I can never reproduce it when I run just that one test case manually, but the automated builds I'm doing hit it > 50% of the time so it is clearly some kind of race condition. The line of code that's getting confused is this fprintf(log, "DAEMON:%s\n", getpgrp() == getsid(0) ? "yes" : "no"); but I'm not clear how this can be going wrong. Ideas ? 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 :|

2 2

[libvirt] [PATCH 0/2] Fix cpuMhz of virNodeGetInfo() don't returns maximum cpu frequency
by Minoru Usui 14 Feb '11

14 Feb '11

cpuMhz of virNodeGetInfo() should return maximum cpu frequency. (See. http://fossplanet.com/f13/%5Blibvirt%5D-%5Brfd%5D-add-modify-some-api-funct…) virNodeGetInfo() gets it from "cpu MHz" of /proc/cpuinfo, but this value is scaled by cpu power saving feature. This patch gets from /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq, if exist. And if this file doesn't exist, it gets from /proc/cpuinfo. -- Minoru Usui <usui(a)mxm.nes.nec.co.jp>

3 8

[libvirt] [PATCH] Fix cleanup on VM state after failed QEMU startup
by Daniel P. Berrange 14 Feb '11

14 Feb '11

Commit 9962e406c664ed5521f5aca500c860a331cb3979 introduced a problem where if the VM failed to startup, it would not be correctly cleaned up. Amongst other things the SELinux security label would not be removed, which prevents the VM from ever starting again. The virDomainIsActive() check at the start of qemudShutdownVMDaemon checks for vm->def->id not being -1. By moving the assignment of the VM id to the start of qemudStartVMDaemon, we can ensure cleanup will occur on failure * src/qemu/qemu_driver.c: Move initialization of 'vm->def->id' so that qemudShutdownVMDaemon() will process the shutdown --- src/qemu/qemu_driver.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 21d7779..d7c806b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2652,6 +2652,8 @@ static int qemudStartVMDaemon(virConnectPtr conn, if (virDomainObjSetDefTransient(driver->caps, vm, true) < 0) goto cleanup; + vm->def->id = driver->nextvmid++; + /* Must be run before security labelling */ DEBUG0("Preparing host devices"); if (qemuPrepareHostDevices(driver, vm->def) < 0) @@ -2818,7 +2820,6 @@ static int qemudStartVMDaemon(virConnectPtr conn, } DEBUG0("Building emulator command line"); - vm->def->id = driver->nextvmid++; if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig, priv->monJSON != 0, qemuCmdFlags, migrateFrom, stdin_fd, -- 1.7.4

3 3