- Devel - libvirt List Archives

[libvirt] [PATCH] Make an error message in PCI util code clearer.
by Chris Lalancette 19 Feb '10

19 Feb '10

Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- src/util/pci.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/pci.c b/src/util/pci.c index aa8344e..83ed461 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -540,7 +540,7 @@ pciTrySecondaryBusReset(pciDevice *dev, */ if (pciRead(dev, 0, config_space, PCI_CONF_LEN) < 0) { pciReportError(VIR_ERR_NO_SUPPORT, - _("Failed to save PCI config space for %s"), + _("Failed to read PCI config space for %s"), dev->name); goto out; } @@ -586,7 +586,7 @@ pciTryPowerManagementReset(pciDevice *dev) /* Save and restore the device's config space. */ if (pciRead(dev, 0, &config_space[0], PCI_CONF_LEN) < 0) { pciReportError(VIR_ERR_NO_SUPPORT, - _("Failed to save PCI config space for %s"), + _("Failed to read PCI config space for %s"), dev->name); return -1; } -- 1.6.6

2 6

[libvirt] [PATCH] [FIX] macvtap mac_filter support
by Stefan Berger 19 Feb '10

19 Feb '10

Previous posting had an unused variable in the compile case of --without-macvtap. This patch adds the mac_filter support to the macvtap device. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>

2 1

[libvirt] [PATCH 1/4] qemu: Make Set*Mem commands hotplug only
by Cole Robinson 19 Feb '10

19 Feb '10

SetMem and SetMaxMem are hotplug only APIs, any persistent config changes are supposed to go via XML definition. The original implementation of these calls were incorrect and had the nasty side effect of making a psuedo persistent change that would be lost after libvirtd restart (I didn't know any better). Fix these APIs to rightly reject non running domains. --- src/qemu/qemu_driver.c | 54 ++++++++++++++++++++++++++++------------------- 1 files changed, 32 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a2be1a3..56a450c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3625,14 +3625,21 @@ static int qemudDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) { goto cleanup; } + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + if (newmax < vm->def->memory) { - qemuReportError(VIR_ERR_INVALID_ARG, - "%s", _("cannot set max memory lower than current memory")); - goto cleanup;; + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("cannot set max memory lower than current memory")); + goto cleanup; } - vm->def->maxmem = newmax; - ret = 0; + /* There isn't any way to change this value for a running qemu guest */ + qemuReportError(VIR_ERR_NO_SUPPORT, + "%s", _("cannot set max memory of an active domain")); cleanup: if (vm) @@ -3643,8 +3650,9 @@ cleanup: static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { struct qemud_driver *driver = dom->conn->privateData; + qemuDomainObjPrivatePtr priv; virDomainObjPtr vm; - int ret = -1; + int ret = -1, r; qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -3657,6 +3665,12 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { goto cleanup; } + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + if (newmem > vm->def->maxmem) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", _("cannot set memory higher than max memory")); @@ -3666,25 +3680,21 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) { if (qemuDomainObjBeginJob(vm) < 0) goto cleanup; - if (virDomainObjIsActive(vm)) { - qemuDomainObjPrivatePtr priv = vm->privateData; - qemuDomainObjEnterMonitor(vm); - int r = qemuMonitorSetBalloon(priv->mon, newmem); - qemuDomainObjExitMonitor(vm); - if (r < 0) - goto endjob; + priv = vm->privateData; + qemuDomainObjEnterMonitor(vm); + r = qemuMonitorSetBalloon(priv->mon, newmem); + qemuDomainObjExitMonitor(vm); + if (r < 0) + goto endjob; - /* Lack of balloon support is a fatal error */ - if (r == 0) { - qemuReportError(VIR_ERR_NO_SUPPORT, - "%s", _("cannot set memory of an active domain")); - goto endjob; - } - } else { - vm->def->memory = newmem; + /* Lack of balloon support is a fatal error */ + if (r == 0) { + qemuReportError(VIR_ERR_NO_SUPPORT, + "%s", _("cannot set memory of an active domain")); + goto endjob; } - ret = 0; + ret = 0; endjob: if (qemuDomainObjEndJob(vm) == 0) vm = NULL; -- 1.6.5.2

4 6

[libvirt] [PATCH] virBufferVSprintf: do not skip va_end
by Jim Meyering 19 Feb '10

19 Feb '10

This fixes the last of the varargs problems reported by coverity: va_end(argptr) was never called, and va_end(locarg) would have been skipped upon OOM. >From 7a75b9da0d08a54e9f256dd26cca061b59c32c6d Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 18 Feb 2010 21:25:01 +0100 Subject: [PATCH] virBufferVSprintf: do not skip va_end * src/util/buf.c (virBufferVSprintf): Do not omit or skip va_end calls. --- src/util/buf.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/util/buf.c b/src/util/buf.c index cc0a087..caf8ee0 100644 --- a/src/util/buf.c +++ b/src/util/buf.c @@ -246,14 +246,17 @@ virBufferVSprintf(const virBufferPtr buf, const char *format, ...) grow_size = (count > 1000) ? count : 1000; if (virBufferGrow(buf, grow_size) < 0) - return; + goto cleanup; size = buf->size - buf->use - 1; va_copy(locarg, argptr); } - va_end(locarg); buf->use += count; buf->content[buf->use] = '\0'; + + cleanup: + va_end(argptr); + va_end(locarg); } /** -- 1.7.0.233.g05e1a

3 8

[libvirt] [PATCH] macvtap mac_filter support
by Stefan Berger 19 Feb '10

19 Feb '10

This patch adds the mac_filter support to the macvtap device. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>

1 0

[libvirt] [PATCH] virsh: be careful to return "FALSE" upon OOM
by Jim Meyering 19 Feb '10

19 Feb '10

Clang reported this as a "dead store" to "ret". Here's one way to fix it. Or just "return FALSE;" and remove the preceding assignment. >From debb6f5198027a056aa24dca95ea4930184ad3fe Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 18 Feb 2010 11:05:38 +0100 Subject: [PATCH] virsh: be careful to return "FALSE" upon OOM * tools/virsh.c (cmdCPUBaseline): Add an explicit "return" statement after the "no_memory:" label. --- tools/virsh.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index e1d1300..dd916f3 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -7141,6 +7141,7 @@ cleanup: no_memory: vshError(ctl, "%s", _("Out of memory")); ret = FALSE; + return ret; } /* Common code for the edit / net-edit / pool-edit functions which follow. */ -- 1.7.0.233.g05e1a

2 1

[libvirt] [PATCH] xenDaemonDomainSetAutostart: avoid appearance of impropriety
by Jim Meyering 19 Feb '10

19 Feb '10

Coverity noticed that of the 13 uses of sexpr_lookup, only this one was unchecked, and here, the result is dereferenced. It happens to be a false positive, due to the preceding sexpr_node check, but worth fixing: sexpr_node is just a very thin wrapper around sexpr_lookup so there's little justification for looking up the same string twice. >From a9ab34214cf9d247d39731563dcc70b8f1dc73b5 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 17 Feb 2010 22:14:25 +0100 Subject: [PATCH] xenDaemonDomainSetAutostart: avoid appearance of impropriety * src/xen/xend_internal.c (xenDaemonDomainSetAutostart): Rewrite to avoid dereferencing the result of sexpr_lookup. While in this particular case, it was guaranteed never to be NULL, due to the preceding "if sexpr_node(...)" guard, it's cleaner to skip the sexpr_node call altogether, and also saves a lookup. --- src/xen/xend_internal.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 88923c8..1f8b106 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -4383,7 +4383,6 @@ xenDaemonDomainSetAutostart(virDomainPtr domain, int autostart) { struct sexpr *root, *autonode; - const char *autostr; char buf[4096]; int ret = -1; xenUnifiedPrivatePtr priv; @@ -4408,16 +4407,17 @@ xenDaemonDomainSetAutostart(virDomainPtr domain, return (-1); } - autostr = sexpr_node(root, "domain/on_xend_start"); - if (autostr) { - if (!STREQ(autostr, "ignore") && !STREQ(autostr, "start")) { + autonode = sexpr_lookup(root, "domain/on_xend_start"); + if (autonode) { + const char *val = (autonode->u.s.car->kind == SEXPR_VALUE + ? autonode->u.s.car->u.value : NULL); + if (!STREQ(val, "ignore") && !STREQ(val, "start")) { virXendError(domain->conn, VIR_ERR_INTERNAL_ERROR, "%s", _("unexpected value from on_xend_start")); goto error; } // Change the autostart value in place, then define the new sexpr - autonode = sexpr_lookup(root, "domain/on_xend_start"); VIR_FREE(autonode->u.s.car->u.value); autonode->u.s.car->u.value = (autostart ? strdup("start") : strdup("ignore")); -- 1.7.0.219.g6bb57

2 1

[libvirt] [PATCH] macvtap IFF_VNET_HDR configuration
by Stefan Berger 19 Feb '10

19 Feb '10

This patch sets or unsets the IFF_VNET_HDR flag depending on what device is used in the VM. The manipulation of the flag is done in the open function and is only fatal if the IFF_VNET_HDR flag could not be cleared although it has to be (or if an ioctl generally fails). In that case the macvtap tap is closed again and the macvtap interface torn. This patch also passes 'make syntax-check' :-). Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>

2 1

[libvirt] [PATCH] virBufferStrcat: do not skip va_end
by Jim Meyering 19 Feb '10

19 Feb '10

Another missed va_end: >From b2e727f9dd25f427d634ef4d79733b37af2b29dd Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 18 Feb 2010 20:46:24 +0100 Subject: [PATCH] virBufferStrcat: do not skip va_end * src/util/buf.c (virBufferStrcat): Do not skip va_end due to an early return. --- src/util/buf.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/buf.c b/src/util/buf.c index e683928..cc0a087 100644 --- a/src/util/buf.c +++ b/src/util/buf.c @@ -1,21 +1,21 @@ /* * buf.c: buffers for libvirt * - * Copyright (C) 2005-2008 Red Hat, Inc. + * Copyright (C) 2005-2008, 2010 Red Hat, Inc. * * See COPYING.LIB for the License of this software * * Daniel Veillard <veillard(a)redhat.com> */ #include <config.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include <stdarg.h> #include "c-ctype.h" #define __VIR_BUFFER_C__ #include "buf.h" @@ -410,25 +410,25 @@ virBufferURIEncodeString (virBufferPtr buf, const char *str) void virBufferStrcat(virBufferPtr buf, ...) { va_list ap; char *str; if (buf->error) return; va_start(ap, buf); while ((str = va_arg(ap, char *)) != NULL) { unsigned int len = strlen(str); unsigned int needSize = buf->use + len + 2; if (needSize > buf->size) { if (virBufferGrow(buf, needSize - buf->use) < 0) - return; + break; } memcpy(&buf->content[buf->use], str, len); buf->use += len; buf->content[buf->use] = 0; } va_end(ap); } -- 1.7.0.233.g05e1a

2 1

[libvirt] [PATCH] qparams.c: do not skip va_end, twice
by Jim Meyering 19 Feb '10

19 Feb '10

More coverity-prompted fixes: >From 56d339d99b09c5943fa36600ca39939080cc64f4 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 18 Feb 2010 20:27:22 +0100 Subject: [PATCH] qparams.c: do not skip va_end, twice * src/util/qparams.c (new_qparam_set, append_qparams): Do not skip va_end due to an early return. --- src/util/qparams.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/util/qparams.c b/src/util/qparams.c index 9535ca4..f6d0713 100644 --- a/src/util/qparams.c +++ b/src/util/qparams.c @@ -1,6 +1,6 @@ -/* Copyright (C) 2007, 2009 Red Hat, Inc. +/* Copyright (C) 2007, 2009-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 * License as published by the Free Software Foundation; either * version 2.1 of the License, or (at your option) any later version. @@ -60,11 +60,12 @@ new_qparam_set (int init_alloc, ...) while ((pname = va_arg (args, char *)) != NULL) { pvalue = va_arg (args, char *); if (append_qparam (ps, pname, pvalue) == -1) { free_qparam_set (ps); - return NULL; + ps = NULL; + break; } } va_end (args); return ps; @@ -73,21 +74,24 @@ new_qparam_set (int init_alloc, ...) int append_qparams (struct qparam_set *ps, ...) { va_list args; const char *pname, *pvalue; + int ret = 0; va_start (args, ps); while ((pname = va_arg (args, char *)) != NULL) { pvalue = va_arg (args, char *); - if (append_qparam (ps, pname, pvalue) == -1) - return -1; + if (append_qparam (ps, pname, pvalue) == -1) { + ret = -1; + break; + } } va_end (args); - return 0; + return ret; } /* Ensure there is space to store at least one more parameter * at the end of the set. */ -- 1.7.0.233.g05e1a

2 1