February 2010 - Devel - libvirt List Archives

[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

[libvirt] FW: [PATCH 1/4] Addition of XenAPI support to libvirt
by Sharadha Prabhakar (3P) 18 Feb '10

18 Feb '10

This is a patch to add XenAPI driver support for libvirt version 0.7.6. XenAPI can be used against XenCloud platform and managed through virsh and virt-manger. This patch supports domain related APIs in libvirt. It is possible to get domain information, list active and inactive domains, get Domain XML configuration and Start/stop/pause/shutdown/destroy VMs. There will be more patches after this review to support more libvirt APIs and add remote storage support to XenAPI. In order to run this patch you would require libxenserver library. The XenCloud platform can be downloaded from http://xen.org/products/cloudxen.html diff -ur ./libvirt_org/configure.ac ./libvirt/configure.ac --- ./libvirt_org/configure.ac 2010-02-17 17:39:21.000000000 +0000 +++ ./libvirt/configure.ac 2010-02-18 11:51:29.000000000 +0000 @@ -219,6 +219,8 @@ AC_HELP_STRING([--with-libssh2=@<:@PFX@:>@], [libssh2 location @<:@default=/usr/local/lib@:>@]),[],[with_libssh2=yes]) AC_ARG_WITH([phyp], AC_HELP_STRING([--with-phyp], [add PHYP support @<:@default=check@:>@]),[],[with_phyp=check]) +AC_ARG_WITH([xenapi], + AC_HELP_STRING([--with-xenapi], [add XenAPI support @<:@default=yes@:>@]),[],[with_xenapi=check]) AC_ARG_WITH([vbox], AC_HELP_STRING([--with-vbox], [add VirtualBox support @<:@default=yes@:>@]),[],[with_vbox=yes]) AC_ARG_WITH([lxc], @@ -307,6 +309,11 @@ fi AM_CONDITIONAL([WITH_VBOX], [test "$with_vbox" = "yes"]) +if test "$with_xenapi" = "yes"; then + AC_DEFINE_UNQUOTED([WITH_XENAPI], 1, [whether XenAPI driver is enabled]) +fi +AM_CONDITIONAL([WITH_XENAPI], [test "$with_xenapi" = "yes"]) + if test "$with_libvirtd" = "no" ; then with_qemu=no fi @@ -1894,6 +1901,7 @@ AC_MSG_NOTICE([ UML: $with_uml]) AC_MSG_NOTICE([ OpenVZ: $with_openvz]) AC_MSG_NOTICE([ VBox: $with_vbox]) +AC_MSG_NOTICE([ XenAPI: $with_xenapi]) AC_MSG_NOTICE([ LXC: $with_lxc]) AC_MSG_NOTICE([ PHYP: $with_phyp]) AC_MSG_NOTICE([ ONE: $with_one]) diff -ur ./libvirt_org/src/Makefile.am ./libvirt/src/Makefile.am --- ./libvirt_org/src/Makefile.am 2010-02-17 17:38:13.000000000 +0000 +++ ./libvirt/src/Makefile.am 2010-02-18 16:25:55.000000000 +0000 @@ -3,12 +3,19 @@ # No libraries with the exception of LIBXML should be listed # here. List them against the individual XXX_la_CFLAGS targets # that actually use them + +XENAPI_CFLAGS = -I@top_srcdir@/../libxenserver/include + INCLUDES = \ -I$(top_srcdir)/gnulib/lib \ -I../gnulib/lib \ -I../include \ + -I/usr/include \ -I@top_srcdir@/src/util \ + -I@top_srcdir@/src \ + -I@top_srcdir@/src/xenapi \ -I@top_srcdir@/include \ + $(XENAPI_CFLAGS) \ $(DRIVER_MODULE_CFLAGS) \ $(LIBXML_CFLAGS) \ -DLIBDIR=\""$(libdir)"\" \ @@ -42,6 +49,8 @@ augeastestdir = $(datadir)/augeas/lenses/tests augeastest_DATA = +XENAPI_LIBS = @top_srcdir@/../libxenserver/libxenserver.so + # These files are not related to driver APIs. Simply generic # helper APIs for various purposes UTIL_SOURCES = \ @@ -205,6 +214,10 @@ qemu/qemu_security_dac.h \ qemu/qemu_security_dac.c +XENAPI_DRIVER_SOURCES = \ + xenapi/xenapi_driver.c xenapi/xenapi_driver.h \ + xenapi/xenapi_utils.c xenapi/xenapi_utils.h + UML_DRIVER_SOURCES = \ uml/uml_conf.c uml/uml_conf.h \ uml/uml_driver.c uml/uml_driver.h @@ -466,6 +479,28 @@ libvirt_driver_vbox_la_SOURCES = $(VBOX_DRIVER_SOURCES) endif +if WITH_XENAPI +if WITH_DRIVER_MODULES +mod_LTLIBRARIES += libvirt_driver_xenapi.la +else +noinst_LTLIBRARIES += libvirt_driver_xenapi.la + +libvirt_la_LIBADD += libvirt_driver_xenapi.la \ + $(XENAPI_LIBS) +endif +#libvirt_driver_xenapi_la_LIBADD = $(XENAPI_LIBS) +libvirt_driver_xenapi_la_CFLAGS = $(XEN_CFLAGS) \ + $(shell xml2-config --cflags) \ + $(shell curl-config --cflags) +libvirt_driver_xenapi_la_LDFLAGS = -L@top_srcdir@/../libxenserver/ -lxenserver -g $(shell xml2-config --libs) \ + $(shell curl-config --libs) + +if WITH_DRIVER_MODULES +libvirt_driver_xenapi_la_LDFLAGS += -module -avoid-version +endif +libvirt_driver_xenapi_la_SOURCES = $(XENAPI_DRIVER_SOURCES) +endif + if WITH_QEMU if WITH_DRIVER_MODULES mod_LTLIBRARIES += libvirt_driver_qemu.la @@ -722,6 +757,7 @@ $(OPENVZ_DRIVER_SOURCES) \ $(PHYP_DRIVER_SOURCES) \ $(VBOX_DRIVER_SOURCES) \ + $(XENAPI_DRIVER_SOURCES) \ $(ESX_DRIVER_SOURCES) \ $(NETWORK_DRIVER_SOURCES) \ $(INTERFACE_DRIVER_SOURCES) \ diff -ur ./libvirt_org/tools/Makefile.am ./libvirt/tools/Makefile.am --- ./libvirt_org/tools/Makefile.am 2010-02-17 17:36:13.000000000 +0000 +++ ./libvirt/tools/Makefile.am 2010-02-18 11:56:30.000000000 +0000 @@ -40,7 +40,8 @@ $(WARN_CFLAGS) \ ../src/libvirt.la \ ../gnulib/lib/libgnu.la \ - $(VIRSH_LIBS) + $(VIRSH_LIBS) \ + ../../libxenserver/libxenserver.so virsh_CFLAGS = \ -I$(top_srcdir)/gnulib/lib -I../gnulib/lib \ -I../include -I$(top_srcdir)/include \

2 1

[libvirt] [PATCH 0/2] Make a wrapper for fork() - Take 2
by Laine Stump 18 Feb '10

18 Feb '10

This is an update to / deprecates the patchset I sent last night: https://www.redhat.com/archives/libvir-list/2010-February/msg00580.html I have corrected the problem found by Dan Berrange (neglecting to restore the signal mask when fork() fails). Aside from that, and corresponding comments in the commit logs, it is unchanged. Here's the original intro email: This was partly prompted by DV's suggestion last week. The first of these patches creates a new function called virFork() which behaves (almost) like fork() but takes care of some important details that pretty much any call to fork() should be doing. The 2nd switches three fork-calling functions in util.c over to using virFork() instead of fork(). In the future, except for odd circumstances, code that needs to fork should call virFork() instead, and if there is anything determined to be universally necessary at fork-time, it should be added to virFork() rather than to the callers of virFork(); hopefully this will ease maintenance and reduce replicated bugs. (Note that, while this is just an overall "code health" patch, a couple bug fix patches I'll be submitting either tomorrow or Thursday will assume it as a prerequisite).

2 3