April 2011 - Devel - libvirt List Archives

[libvirt] [PATCH] maint: fix comment typos
by Eric Blake 28 Apr '11

28 Apr '11

* src/esx/esx_driver.c: Fix spelling of 'relative'. * src/util/util.c: Likewise. --- Pushing under the trivial rule. src/esx/esx_driver.c | 2 +- src/util/util.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 79c1360..81e3c8e 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2413,7 +2413,7 @@ esxDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) VIR_DEBUG("usedCpuTimeCounterId %d END", priv->usedCpuTimeCounterId); /* - * FIXME: Cannot map between realtive used-cpu-time and absolute + * FIXME: Cannot map between relative used-cpu-time and absolute * info->cpuTime */ } diff --git a/src/util/util.c b/src/util/util.c index 16454ed..a28af0b 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -2040,7 +2040,7 @@ cleanup: /* - * Creates an absolute path for a potentialy realtive path. + * Creates an absolute path for a potentially relative path. * Return 0 if the path was not relative, or on success. * Return -1 on error. * -- 1.7.4.4

1 0

[libvirt] [PATCH] build: Use pkg-config for libssh2 check
by Jiri Denemark 28 Apr '11

28 Apr '11

Currently the build fails if /usr/local/include does not exist since its use is hardcoded in configure.ac --- configure.ac | 86 ++++++++++++++------------------------------------------- 1 files changed, 21 insertions(+), 65 deletions(-) diff --git a/configure.ac b/configure.ac index 25cc15b..39e3a05 100644 --- a/configure.ac +++ b/configure.ac @@ -68,6 +68,7 @@ DEVMAPPER_REQUIRED=1.0.0 LIBCURL_REQUIRED="7.18.0" LIBPCAP_REQUIRED="1.0.0" LIBNL_REQUIRED="1.1" +LIBSSH2_REQUIRED="1.0" dnl Checks for C compiler. AC_PROG_CC @@ -258,7 +259,7 @@ AC_ARG_WITH([openvz], AC_ARG_WITH([vmware], AC_HELP_STRING([--with-vmware], [add VMware support @<:@default=yes@:>@]),[],[with_vmware=yes]) AC_ARG_WITH([libssh2], - AC_HELP_STRING([--with-libssh2=@<:@PFX@:>@], [libssh2 location @<:@default=/usr/local/lib@:>@]),[],[with_libssh2=yes]) + AC_HELP_STRING([--with-libssh2], [libssh2 location @<:@default=check@:>@]),[],[with_libssh2=check]) AC_ARG_WITH([phyp], AC_HELP_STRING([--with-phyp], [add PHYP support @<:@default=check@:>@]),[],[with_phyp=check]) AC_ARG_WITH([xenapi], @@ -1328,78 +1329,33 @@ AM_CONDITIONAL([WITH_UML], [test "$with_uml" = "yes"]) dnl -dnl libssh checks +dnl check for libssh2 (PHYP) dnl -if test "$with_libssh2" != "yes" && test "$with_libssh2" != "no"; then - libssh2_path="$with_libssh2" -elif test "$with_libssh2" = "yes"; then - libssh2_path="/usr/local/lib/" -elif test "$with_libssh2" = "no"; then - with_phyp="no"; -fi +LIBSSH2_CFLAGS="" +LIBSSH2_LIBS="" -if test "$with_phyp" = "check"; then - AC_CHECK_LIB([ssh2],[libssh2_session_startup],[ - LIBSSH2_LIBS="$LIBSSH2_LIBS -lssh2 -L$libssh2_path" - AC_SUBST([LIBSSH2_LIBS]) - ],[ - with_phyp="no" - with_libssh2="no"; - ],[]) - - if test "$with_phyp" != "no"; then - AC_CHECK_HEADERS([libssh2.h],[ - with_phyp="yes" - LIBSSH2_CFLAGS="-I/usr/local/include" - AC_SUBST([LIBSSH2_CFLAGS]) - ],[ - with_phyp="no" - with_libssh2="no"; - ],[ - ]) - fi - - if test "$with_phyp" != "no"; then - saved_libs="$LIBS" - LIBS="$LIBS -L$libssh2_path -lssh2" - AC_TRY_LINK([#include <libssh2.h>], [ - (void) libssh2_session_block_directions(NULL); - ], [ - AC_DEFINE_UNQUOTED([WITH_PHYP], 1, [whether IBM HMC / IVM driver is enabled]) - ], [ +if test "$with_phyp" = "yes" || test "$with_phyp" = "check"; then + PKG_CHECK_MODULES(LIBSSH2, libssh2 >= $LIBSSH2_REQUIRED, [ + with_phyp=yes + ], [ + if test "$with_phyp" = "check"; then with_phyp=no - AC_MSG_NOTICE([Function libssh2_session_block_directions() not present in your version of libssh2 but required for Phyp driver, disabling it]) - ]) - LIBS="$saved_libs" - fi -elif test "$with_phyp" = "yes"; then - AC_CHECK_LIB([ssh2],[libssh2_session_startup],[ - LIBSSH2_LIBS="$LIBSSH2_LIBS -lssh2 -L$libssh2_path" - AC_SUBST([LIBSSH2_LIBS])],[ - AC_MSG_ERROR([You must install the libssh2 to compile Phyp driver.]) - ]) - - AC_CHECK_HEADERS([libssh2.h],[ - LIBSSH2_CFLAGS="-I/usr/local/include" - AC_SUBST([LIBSSH2_CFLAGS])],[ - AC_MSG_ERROR([Cannot find libssh2 headers. Is libssh2 installed ?]) - ],[]) - - saved_libs="$LIBS" - LIBS="$LIBS -lssh2" - AC_TRY_LINK([#include <libssh2.h>], [ - (void) libssh2_session_block_directions(NULL); - ], [], [ - AC_MSG_ERROR([Function libssh2_session_block_directions() not present in your version of libssh2. Need >= 1.0]) - ]) - LIBS="$saved_libs" + AC_MSG_NOTICE([libssh2 is required for Phyp driver, disabling it]) + else + AC_MSG_ERROR([libssh2 >= $LIBSSH2_REQUIRED is required for Phyp driver]) + fi + ]) +fi - AC_DEFINE_UNQUOTED([WITH_PHYP], 1, - [whether IBM HMC / IVM driver is enabled]) +if test "$with_phyp" = "yes"; then + AC_DEFINE_UNQUOTED([WITH_PHYP], 1, [whether IBM HMC / IVM driver is enabled]) fi AM_CONDITIONAL([WITH_PHYP],[test "$with_phyp" = "yes"]) +AC_SUBST([LIBSSH2_CFLAGS]) +AC_SUBST([LIBSSH2_LIBS]) + dnl libcap-ng AC_ARG_WITH([capng], AC_HELP_STRING([--with-capng], [use libcap-ng to reduce libvirtd privileges @<:@default=check@:>@]), -- 1.7.5.rc1

2 2

[libvirt] [PATCH] build: Ignore old audit library
by Jiri Denemark 28 Apr '11

28 Apr '11

Check for audit_encode_nv_string in libaudit so that ancient audit library is ignored rather than trying to compile with libaudit support and failing. --- configure.ac | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/configure.ac b/configure.ac index 190bf40..25cc15b 100644 --- a/configure.ac +++ b/configure.ac @@ -1026,7 +1026,7 @@ if test "$with_audit" != "no" ; then LIBS="$LIBS $AUDIT_LIBS" fail=0 AC_CHECK_HEADER([libaudit.h], [], [fail=1]) - AC_CHECK_LIB([audit], [audit_is_enabled], [], [fail=1]) + AC_CHECK_LIB([audit], [audit_encode_nv_string], [], [fail=1]) if test $fail = 1 ; then if test "$with_audit" = "yes" ; then -- 1.7.5.rc1

2 2

[libvirt] RFC: virt-xml: command line tool for altering guest configuration
by Cole Robinson 28 Apr '11

28 Apr '11

Hi all, Attached is a patch to virtinst.git that provides a new tool named virt-xml. virt-xml provides a scriptable command line interface for editing libvirt guest XML. A couple examples: Change boot order and boot menu for 'myguest': $ ./virt-xml --guest myguest --boot network,hd,cdrom,menu=on Requested changes: --- Original XML +++ New XML @@ -10,6 +10,8 @@ <os> <type arch="i686">hvm</type> <loader>/usr/lib/xen/boot/hvmloader</loader> + <boot dev="network"/> + <bootmenu enable="yes"/> <boot dev="hd"/> + <boot dev="cdrom"/> </os> <features> <acpi/> Change disk #7 to use cache=none for 'myguest': $ ./virt-xml --guest myguest --device 7 --disk cache=none Requested changes: --- Original XML +++ New XML @@ -74,6 +74,7 @@ <disk type="file" device="disk"> <source file="/tmp/foobar3"/> <target dev="sdd" bus="usb"/> + <driver cache="none"/> </disk> <disk type="file" device="disk"> <driver name="tap" type="qcow" cache="writethrough"/> Change watchdog action for 'myguest': $ ./virt-xml --guest myguest --device 1 --watchdog action=pause Requested changes: --- Original XML +++ New XML @@ -220,7 +220,7 @@ <address domain="0x0003" bus="0x00" slot="0x19" function="0x0"/> </source> </hostdev> - <watchdog model="ib700" action="poweroff"/> + <watchdog model="ib700" action="pause"/> <memballoon model="xen"/> </devices> </domain> --boot, --disk, and --watchdog come straight from virt-install, see man virt-install for detailed docs of the option format. Those options shown are the only things wired up right now, but it's a straightforward task to wire up most of the relevant remaining virt-install opts. Also the tool doesn't actually apply the changes yet, but that is also a trivial addition. I think the interesting bit here is nailing down the scope of the tool and deciding on good CLI syntax. Not sure if we should do hotplug/hotunplug as a subcommand or just another option like: virt-xml ... --hotplug --disk /tmp/foo.img Also not really sure how to hotunplug using the --device syntax of the above examples: virt-xml ... --hotunplug --device 1 --disk <what goes here?> We could make --device just an option to whatever device string the user gives virt-xml ... --hotunplug --disk id=1 We can easily allow different XML input methods like: virt-xml --guestxml file.xml ... or even batch processing virt-xml --guest * ... Maybe even have allow more intelligence about what guests we edit, say we want to set cache=none for all disk devices, not CDROM or floppy (this would take a bit of work to sort out). virt-xml --guest * --disk match,device=disk --disk cache=none Then there is also the question of extending the tool to also edit other XML like storage, network, interface, etc. Any recommendations appreciated: option syntax, future capabilities, or even a better name for the tool :) A quick word about why I think we should do this with a separate tool rather than virsh. For starters, I think the need for this functionality on the command line is clear by now, so it's just a question of where to implement it. Why not virsh? Text and command line processing in C is a serious pain compared to a higher level language like python. I don't think this is a controversial statement, in fact I think it's partly responsible for why the virsh XML building commands have typically been in a pretty sorry state. virsh could conceivably use the internal domain_conf API to make this task much easier, but it would still require a lot of command line options for every XML parameter. In contrast, virt-install already needs to do most of this command line processing, and virt-manager needs to do all the XML building, parsing, and editing. Since this code already lives in one place (virtinst), it's almost trivial for us to reuse it. Supporting a new XML property on the command line is a very simple task and will immediately benefit both virt-install and virt-xml. We also already have over 75% of the domain XML schema available via existing CLI options that are already documented :) Questions or comments appreciated! Thanks, Cole

3 2

[libvirt] RFC: disconnecting guest/domain interface config from host config (aka migration with macvtap)
by Laine Stump 28 Apr '11

28 Apr '11

Abstraction of guest <--> host network connection in libvirt ===================================== The <interface> element of a guest's domain config in libvirt has a <source> element that describes what resources on a host will be used to connect the guest's network interface to the rest of the world. This is very flexible, allowing several different types of connection (virtual network, host bridge, direct macvtap connection to physical interface, qemu usermode, user-defined via an external script), but currently has the problem that unnecessary details of the host config are embedded into the guest's config; if the guest is migrated to a different host, and that host has a different hardware or network config (or possibly the same hardware, but that hardware is currently in use by a different guest), the migration will fail. I am proposing a change to libvirt's network XML that will allow us to (optionally - old configs will remain valid) remove the host details from the guest's domain XML (which can move around from host to host) and place them in the network XML (which remains with the host); the domain XML will then use existing config elements to associate each guest interface with a "network". The motivating use case for this change is the "direct" connection type (which uses macvtap for vepa and vnlink connections directly between a guest and a physical interface, rather than through a bridge), but it is applicable for all types of connection. (Another hopeful side effect of this change will be to make libvirt's network connection model easier to realize on non-Linux hypervisors (eg, VMWare ESX), so Mathias - please chime in!) Background -------------------- libvirt currently has 3 major types of guest interface connection (there are also "type='user'" and "type='ethernet'", but they probably wouldn't be used in a multi-host environment, so I'm not considering them here): 1) type='network' The guest's network interface is connected to a libvirt-created "virtual network", which is in reality (in the case of KVM or Xen) a Linux bridge device that isn't connected to any physical host interface - any connection to the outside goes through the host's IP routing stack. The network to use is indicated in the <source> element of the guest's interface xml: <source network='mynetwork'/>. Because the name 'mynetwork' is controlled by libvirt, it's perfectly reasonable to assume that the same network name could be available on another host that is accepting a migrated guest. 2) type='bridge' The guest's network interface is connected to a bridge device (eg "br0") that has already been configured in the host's network config files (eg, in /etc/sysconfig/network-scripts). This bridge is itself connected to the outside via a physical host interface, eg "eth0", *NOT* through the hosts IP routing stack. The bridge to use is indicated in the source with <source bridge='br0'/>. Although the naming of the bridge is outside the scope of libvirt, it is at least possible to setup all hosts to have the same bridge name (so that a guest could be migrated from one host to another). 3) type='direct' The guest's network interface is connected directly to a physical interface (eg "eth0") with macvtap, or sometimes to a virtual function ("VF") of a physical interface (which is also really just another interface, from the software point of view). The interface to use is indicated with <source interface='eth0' mode='something'/> In this case, the interface name is determined by the host OS and cannot be arbitrarily changed. Also a host will have multiple interfaces / VFs available to guests, and in some modes may allow only a single guest to connect to a given interface (implying that the interface used by a guest when on one host will probably not be available when migrating to another). So in order to have flexible migration from one host to another, an abstraction to allow the guest XML to use the same name on all hosts must be introduced. Three possible methods for providing this abstraction come to mind: Option 1 ----------- (Be forewarned that Option 1 & 2 are shown here mainly to illustrate my thought process while arriving at my preferred Option - 3 :-) In a manner similar to the way the vnet%d tap devices are created, name the interface with an embedded variable (eg "eth%d") (plus attributes for min and max %d) and let the underlying code in libvirt search for/reserve an appropriate device> This is the simplest to code/configure, but does not allow a) more complex names (eg, interface names as determined by biosdevname can be of the form "pci%dp%d_%d"), b) multiple ranges, c) oversubscribing of interfaces (it is possible, although sub-optimal, to connect multiple guest interfaces to a single host interface with macvtap). VERDICT: looks ugly, not flexible enough. Option 2 ----------- create a new class of libvirt XML config to describe a pool of network interfaces, and reference this pool in the guest interface element: <interface type='interfacePool'> <source pool='red-network'/> ... </interface> The problem with this is that it requires a new API for defining/undefining/etc management of "interface pools". Also, it wouldn't allow (for example) one host to use a pool of macvtap addresses to connect guests, and another host to use a host bridge for the same connection (obviously, such a non-uniform setup wouldn't be desirable in a large host farm, but may be encountered in some smaller setup) VERDICT: creates more API clutter (ie extra work *and* confusion for users). Is "flexible enough" for current motivation, but unnecessarily limiting, eg doesn't help the model to be more easily adapted to VMWare etc. Option 3 ----------- Up to now we've only discussed the need for separating the host-specific config (<source> element) in the case of type='direct' interfaces (well, in reality I've gone back and edited this document so many times that is no longer true, but play along with me! :-). But it really is a problem for all interface types - all of the information currently in the guest's interface <source> element really is tied to the host, and shouldn't be defined in detail in the guest XML; it should instead be defined once for each host, and only referenced by some name in the guest XML; that way as a guest moves from host to host, it will automatically adjust its connection to match the new environmant. As a more general solution, instead of having the special new "interfacePool" object in the config, what if the XML for "network was expanded to mean "any type of guest network connection" (with a new "type='xxx'" attribute at the toplevel to indicate which type), not just "a private bridge optionally connected to the real world via routing/NAT"? If this was the case, the guest interface XML could always be, eg: <interface type='network'> <source network='red-network'/> ... </interface> and depending on the network config of the host the guest was migrated to, this could be either a direct (macvtap) connection via an interface allocated from a pool (the pool being defined in the definition of 'red-network'), a bridge (again, pointed to by the definition of 'red-network', or a virtual network (using the current network definition syntax). This way the same guest could be migrated not only between macvtap-enabled hosts, but from there to a host using a bridge, or maybe a host in a remote location that used a virtual network with a secure tunnel to connect back to the rest of the red-network. (Part of the migration process would of course check that the destination host had a network of the proper name, and fail if it didn't; management software at a level above libvirt would probably filter a list of candidate migration destinations based on available networks, and only attempt migration to one that had the matching network available). Examples of 'red-network' for different types of connections (all of these would work with the interface XML given above):  <network>  <name>red-network</name> <bridge name='virbr0'/> <forward mode='route'/> ... </network>  <network type='bridge'> <name>red-network</name> <bridge name='br0'/> </network> <network type='direct'> <name>red-network</name> <source mode='vepa'>   </source>    </network> I know there may be some resistance to this expansion of the usage of <network>, but I think it does fit in with the current usage properly, and is preferable to adding an entire new class of API just to define a pool of interfaces. Open questions: 1) What should the <pool> element inside network/source look like. Making each interface in the pool a separate element, with possible attributed, would be the simplest to code, but would get tedious on a system with, for example, an ethernet card with 64 VFs. On the other hand, just parameterizing a string (eth%d) is inadequate, eg, when there are multiple non-contiguous ranges. 2) do we need a "max connections" for each interface in a pool of macvtap interfaces? Or should we just overload them in a round-robin fashion unless mode='passthru' (a new mode which requires only one guest per interface). 3) What about the parameters in the <virtualport> element that are currently used by vepa/vnlink. Do those belong with the host, or with the guest? 4) Are there other <network> types that we want? Perhaps the recent proposal for IPSec / secure tunnels could be incorporated as a new network type (or maybe it could just be the standard "virtual" type, with a tunnel as the forward device).

1 2

[libvirt] [PATCH] util: Initialize hooks at daemon shutdown if no hooks defined
by Osier Yang 28 Apr '11

28 Apr '11

We support to initialize the hooks at daemon reload if there is no hooks script is defined, should we also support initialize the hooks at daemon shutdown if no hooks is defined? To address bz: https://bugzilla.redhat.com/show_bug.cgi?id=688859 --- src/util/hooks.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/util/hooks.c b/src/util/hooks.c index a409d77..30e20ac 100644 --- a/src/util/hooks.c +++ b/src/util/hooks.c @@ -209,7 +209,8 @@ virHookCall(int driver, const char *id, int op, int sub_op, const char *extra, */ if ((virHooksFound == -1) || ((driver == VIR_HOOK_DRIVER_DAEMON) && - (op == VIR_HOOK_DAEMON_OP_RELOAD))) + (op == VIR_HOOK_DAEMON_OP_RELOAD || + op == VIR_HOOK_DAEMON_OP_SHUTDOWN))) virHookInitialize(); if ((virHooksFound & (1 << driver)) == 0) -- 1.7.4

2 2

[libvirt] [PATCH] fix virsh's regression
by Wen Congyang 28 Apr '11

28 Apr '11

This patch does the following things: 1. The return value of cmdSchedInfoUpdate() can be -1, 0 and 1. So the type of return value should be int not bool.(This function is not a entry of a virsh command, but the name of this function likes cmdXXX) 2. The type of cmdSchedinfo()'s, cmdFreecell()'s, cmdPoolList()'s and cmdVolList()'s return value is bool not int, so change the type of variable ret_val, func_ret and functionReturn. 3. Add a variable functionReturn for cmdMigrate(), cmdAttachInterface(), cmdDetachInterface(), cmdAttachDisk() and cmdDetachDisk() to save the return value. 4. Change the type of variable ret in the function cmdAttachDevice(), cmdDetachDevice(), cmdUpdateDevice(), cmdAttachInterface(), cmdDetachInterface(), cmdAttachDisk() and cmdDetachDisk() to int, as we use it to save the return value of virXXX() and the type of virXXX()'s return value is int not bool. 5. Do some cleanup when virBuff.error is 1. The bug 1-4 were introduced by commit b56fa5bb. --- tools/virsh.c | 63 +++++++++++++++++++++++++++++--------------------------- 1 files changed, 33 insertions(+), 30 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 9ac27b3..27140f3 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1593,7 +1593,7 @@ static const vshCmdOptDef opts_schedinfo[] = { {NULL, 0, 0, NULL} }; -static bool +static int cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, virSchedParameterPtr param) { @@ -1696,7 +1696,7 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) int nparams = 0; int update = 0; int i, ret; - int ret_val = false; + bool ret_val = false; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -2288,7 +2288,7 @@ static const vshCmdOptDef opts_freecell[] = { static bool cmdFreecell(vshControl *ctl, const vshCmd *cmd) { - int func_ret = false; + bool func_ret = false; int ret; int cell = -1, cell_given; unsigned long long memory; @@ -3847,6 +3847,7 @@ cmdMigrate (vshControl *ctl, const vshCmd *cmd) virDomainPtr dom = NULL; int p[2] = {-1, -1}; int ret = -1; + bool functionReturn = false; virThread workerThread; struct pollfd pollfd; char retchar; @@ -3921,15 +3922,15 @@ repoll: if (ret > 0) { if (saferead(p[0], &retchar, sizeof(retchar)) > 0) { if (retchar == '0') { - ret = true; + functionReturn = true; if (verbose) { /* print [100 %] */ print_job_progress(0, 1); } } else - ret = false; + functionReturn = false; } else - ret = false; + functionReturn = false; break; } @@ -3937,11 +3938,11 @@ repoll: if (errno == EINTR) { if (intCaught) { virDomainAbortJob(dom); - ret = false; intCaught = 0; } else goto repoll; } + functionReturn = false; break; } @@ -3975,7 +3976,7 @@ cleanup: virDomainFree(dom); VIR_FORCE_CLOSE(p[0]); VIR_FORCE_CLOSE(p[1]); - return ret; + return functionReturn; } /* @@ -5952,7 +5953,8 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) { virStoragePoolInfo info; char **poolNames = NULL; - int i, functionReturn, ret; + int i, ret; + bool functionReturn; int numActivePools = 0, numInactivePools = 0, numAllPools = 0; size_t stringLength = 0, nameStrLength = 0; size_t autostartStrLength = 0, persistStrLength = 0; @@ -7525,7 +7527,8 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) double val; int details = vshCommandOptBool(cmd, "details"); int numVolumes = 0, i; - int ret, functionReturn; + int ret; + bool functionReturn; int stringLength = 0; size_t allocStrLength = 0, capStrLength = 0; size_t nameStrLength = 0, pathStrLength = 0; @@ -8904,7 +8907,7 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; const char *from = NULL; char *buffer; - bool ret = true; + int ret; unsigned int flags; if (!vshConnectionUsability(ctl, ctl->conn)) @@ -8969,7 +8972,7 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; const char *from = NULL; char *buffer; - bool ret = true; + int ret; unsigned int flags; if (!vshConnectionUsability(ctl, ctl->conn)) @@ -9035,7 +9038,7 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom; const char *from = NULL; char *buffer; - bool ret = true; + int ret; unsigned int flags; if (!vshConnectionUsability(ctl, ctl->conn)) @@ -9110,7 +9113,8 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) const char *mac = NULL, *target = NULL, *script = NULL, *type = NULL, *source = NULL, *model = NULL; int typ; - bool ret = false; + int ret; + bool functionReturn = false; unsigned int flags; virBuffer buf = VIR_BUFFER_INITIALIZER; char *xml; @@ -9165,7 +9169,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) if (virBufferError(&buf)) { vshPrint(ctl, "%s", _("Failed to allocate XML buffer")); - return false; + goto cleanup; } xml = virBufferContentAndReset(&buf); @@ -9183,17 +9187,16 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) if (ret != 0) { vshError(ctl, "%s", _("Failed to attach interface")); - ret = false; } else { vshPrint(ctl, "%s", _("Interface attached successfully\n")); - ret = true; + functionReturn = true; } cleanup: if (dom) virDomainFree(dom); virBufferFreeAndReset(&buf); - return ret; + return functionReturn; } /* @@ -9226,7 +9229,8 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) char *doc; char buf[64]; int i = 0, diff_mac; - bool ret = false; + int ret; + int functionReturn = false; unsigned int flags; if (!vshConnectionUsability(ctl, ctl->conn)) @@ -9322,10 +9326,9 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) if (ret != 0) { vshError(ctl, "%s", _("Failed to detach interface")); - ret = false; } else { vshPrint(ctl, "%s", _("Interface detached successfully\n")); - ret = true; + functionReturn = true; } cleanup: @@ -9337,7 +9340,7 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) xmlFreeDoc(xml); if (xml_buf) xmlBufferFree(xml_buf); - return ret; + return functionReturn; } /* @@ -9368,7 +9371,8 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) virDomainPtr dom = NULL; const char *source = NULL, *target = NULL, *driver = NULL, *subdriver = NULL, *type = NULL, *mode = NULL; - bool isFile = false, ret = false; + bool isFile = false, functionReturn = false; + int ret; unsigned int flags; const char *stype = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -9460,17 +9464,16 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) if (ret != 0) { vshError(ctl, "%s", _("Failed to attach disk")); - ret = false; } else { vshPrint(ctl, "%s", _("Disk attached successfully\n")); - ret = true; + functionReturn = true; } cleanup: if (dom) virDomainFree(dom); virBufferFreeAndReset(&buf); - return ret; + return functionReturn; } /* @@ -9501,7 +9504,8 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd) const char *target = NULL; char *doc; int i = 0, diff_tgt; - bool ret = false; + int ret; + bool functionReturn = false; unsigned int flags; if (!vshConnectionUsability(ctl, ctl->conn)) @@ -9582,10 +9586,9 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd) if (ret != 0) { vshError(ctl, "%s", _("Failed to detach disk")); - ret = false; } else { vshPrint(ctl, "%s", _("Disk detached successfully\n")); - ret = true; + functionReturn = true; } cleanup: @@ -9597,7 +9600,7 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd) xmlBufferFree(xml_buf); if (dom) virDomainFree(dom); - return ret; + return functionReturn; } /* -- 1.7.1

2 2

[libvirt] [PATCH] free memory properly in cleanup patch
by Hu Tao 27 Apr '11

27 Apr '11

virsh schedinfo inactive-domain will trigger the problem. --- daemon/remote.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 1c98bba..eedbc77 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -945,8 +945,11 @@ remoteDispatchDomainGetSchedulerParameters(struct qemud_server *server ATTRIBUTE cleanup: if (rv < 0) { remoteDispatchError(rerr); - for (i = 0 ; i < nparams ; i++) - VIR_FREE(ret->params.params_val[i].field); + if (ret->params.params_val) { + for (i = 0 ; i < nparams ; i++) + VIR_FREE(ret->params.params_val[i].field); + VIR_FREE(ret->params.params_val); + } } if (dom) virDomainFree(dom); -- 1.7.3.1

3 2

[libvirt] [PATCH] release PCI address only when we have ensured it successfully
by Wen Congyang 27 Apr '11

27 Apr '11

Steps to reproduce this bug: 1. # cat net.xml # 00:03.0 has been used <interface type='network'> <mac address='52:54:00:04:72:f3'/> <source network='default'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </interface> 2. # virsh attach-device vm1 net.xml error: Failed to attach device from net.xml error: internal error unable to reserve PCI address 0:0:3 3. # virsh attach-device vm1 net.xml error: Failed to attach device from net.xml error: internal error unable to execute QEMU command 'device_add': Device 'rtl8139' could not be initialized The reason of this bug is that: we can not reserve PCI address 0:0:3 because it has been used, but we release PCI address when we reserve it failed. --- src/qemu/qemu_hotplug.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index b03f774..5fdb013 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -147,6 +147,7 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver, qemuDomainObjPrivatePtr priv = vm->privateData; char *devstr = NULL; char *drivestr = NULL; + bool releaseaddr = false; for (i = 0 ; i < vm->def->ndisks ; i++) { if (STREQ(vm->def->disks[i]->dst, disk->dst)) { @@ -163,6 +164,7 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver, if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &disk->info) < 0) goto error; + releaseaddr = true; if (qemuAssignDeviceDiskAlias(disk, qemuCaps) < 0) goto error; @@ -221,6 +223,7 @@ error: if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && + releaseaddr && qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &disk->info) < 0) VIR_WARN("Unable to release PCI address on %s", disk->src); @@ -242,6 +245,7 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver, const char* type = virDomainControllerTypeToString(controller->type); char *devstr = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; + bool releaseaddr = false; for (i = 0 ; i < vm->def->ncontrollers ; i++) { if ((vm->def->controllers[i]->type == controller->type) && @@ -256,6 +260,7 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver, if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &controller->info) < 0) goto cleanup; + releaseaddr = true; if (qemuAssignDeviceControllerAlias(controller) < 0) goto cleanup; @@ -288,6 +293,7 @@ cleanup: if ((ret != 0) && qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && + releaseaddr && qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &controller->info) < 0) VIR_WARN0("Unable to release PCI address on controller"); @@ -559,6 +565,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, int ret = -1; virDomainDevicePCIAddress guestAddr; int vlan; + bool releaseaddr = false; if (!qemuCapsGet(qemuCaps, QEMU_CAPS_HOST_NET_ADD)) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -595,6 +602,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &net->info) < 0) goto cleanup; + releaseaddr = true; + if (qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV) && qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { vlan = -1; @@ -694,6 +703,7 @@ cleanup: if ((ret != 0) && qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && + releaseaddr && qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &net->info) < 0) VIR_WARN0("Unable to release PCI address on NIC"); @@ -757,6 +767,7 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, char *devstr = NULL; int configfd = -1; char *configfd_name = NULL; + bool releaseaddr = false; if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0) { virReportOOMError(); @@ -771,6 +782,7 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, goto error; if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &hostdev->info) < 0) goto error; + releaseaddr = true; if (qemuCapsGet(qemuCaps, QEMU_CAPS_PCI_CONFIGFD)) { configfd = qemuOpenPCIConfig(hostdev); if (configfd >= 0) { @@ -823,6 +835,7 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, error: if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE) && (hostdev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && + releaseaddr && qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &hostdev->info) < 0) VIR_WARN0("Unable to release PCI address on host device"); -- 1.7.1

3 2

[libvirt] [PATCH V2] free buf->content when vsnprintf() failed
by Wen Congyang 27 Apr '11

27 Apr '11

When buf->error is 1, we do not return buf->content in the function virBufferContentAndReset(). So we should free buf->content when vsnprintf() failed. --- Changes v1->v2 - rename virBufferNoMemory() to virBufferSetError() and use it to free buf->content as Laine Stump suggested src/util/buf.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/util/buf.c b/src/util/buf.c index 7557ad1..a3cc063 100644 --- a/src/util/buf.c +++ b/src/util/buf.c @@ -39,7 +39,7 @@ struct _virBuffer { * freeing the content and setting the error flag. */ static void -virBufferNoMemory(const virBufferPtr buf) +virBufferSetError(const virBufferPtr buf) { VIR_FREE(buf->content); buf->size = 0; @@ -70,7 +70,7 @@ virBufferGrow(virBufferPtr buf, unsigned int len) size = buf->use + len + 1000; if (VIR_REALLOC_N(buf->content, size) < 0) { - virBufferNoMemory(buf); + virBufferSetError(buf); return -1; } buf->size = size; @@ -241,7 +241,7 @@ virBufferVSprintf(const virBufferPtr buf, const char *format, ...) size = buf->size - buf->use; if ((count = vsnprintf(&buf->content[buf->use], size, format, argptr)) < 0) { - buf->error = 1; + virBufferSetError(buf); goto err; } @@ -259,7 +259,7 @@ virBufferVSprintf(const virBufferPtr buf, const char *format, ...) size = buf->size - buf->use; if ((count = vsnprintf(&buf->content[buf->use], size, format, argptr)) < 0) { - buf->error = 1; + virBufferSetError(buf); goto err; } } @@ -299,7 +299,7 @@ virBufferEscapeString(const virBufferPtr buf, const char *format, const char *st } if (VIR_ALLOC_N(escaped, 6 * len + 1) < 0) { - virBufferNoMemory(buf); + virBufferSetError(buf); return; } @@ -386,7 +386,7 @@ virBufferEscapeSexpr(const virBufferPtr buf, } if (VIR_ALLOC_N(escaped, 2 * len + 1) < 0) { - virBufferNoMemory(buf); + virBufferSetError(buf); return; } -- 1.7.1

3 2