June 2010 - Devel - libvirt List Archives

[libvirt] [PATCH] vbox: Let configure detect/set the XPCOMC directory
by Matthias Bolte 29 Jun '10

29 Jun '10

This allows the user to give an explicit path to configure ./configure --with-vbox=/path/to/virtualbox instead of having the VirtualBox driver probe a set of possible paths at runtime. If no explicit path is specified then configure probes the set of "known" paths. https://bugzilla.redhat.com/show_bug.cgi?id=609185 --- configure.ac | 73 +++++++++++++++++++++++++++++++++++++++++++- src/Makefile.am | 1 + src/vbox/vbox_XPCOMCGlue.c | 22 ++----------- 3 files changed, 76 insertions(+), 20 deletions(-) diff --git a/configure.ac b/configure.ac index 691ef69..a1ba2f3 100644 --- a/configure.ac +++ b/configure.ac @@ -232,7 +232,7 @@ AC_ARG_WITH([phyp], AC_ARG_WITH([xenapi], AC_HELP_STRING([--with-xenapi], [add XenAPI support @<:@default=check@:>@]),[],[with_xenapi=check]) AC_ARG_WITH([vbox], - AC_HELP_STRING([--with-vbox], [add VirtualBox support @<:@default=yes@:>@]),[],[with_vbox=yes]) + AC_HELP_STRING([--with-vbox=@<:@PFX@:>@], [VirtualBox XPCOMC location @<:@default=check@:>@]),[],[with_vbox=check]) AC_ARG_WITH([lxc], AC_HELP_STRING([--with-lxc], [add Linux Container support @<:@default=check@:>@]),[],[with_lxc=check]) AC_ARG_WITH([one], @@ -315,6 +315,77 @@ if test "$with_openvz" = "yes"; then fi AM_CONDITIONAL([WITH_OPENVZ], [test "$with_openvz" = "yes"]) + +dnl +dnl check for VirtualBox XPCOMC location +dnl + +vbox_xpcomc_dir= + +if test "x$with_vbox" = "xyes" || test "x$with_vbox" = "xcheck"; then + AC_MSG_CHECKING([for VirtualBox XPCOMC location]) + + if test -f /usr/lib/virtualbox/VBoxXPCOMC.so; then + vbox_xpcomc_dir=/usr/lib/virtualbox + else + if test -f /usr/lib/VirtualBox/VBoxXPCOMC.so; then + vbox_xpcomc_dir=/usr/lib/VirtualBox + else + if test -f /opt/virtualbox/VBoxXPCOMC.so; then + vbox_xpcomc_dir=/opt/virtualbox + else + if test -f /opt/virtualbox/VBoxXPCOMC.so; then + vbox_xpcomc_dir=/opt/virtualbox + else + if test -f /opt/VirtualBox/i386/VBoxXPCOMC.so; then + vbox_xpcomc_dir=/opt/VirtualBox/i386 + else + if test -f /opt/VirtualBox/amd64/VBoxXPCOMC.so; then + vbox_xpcomc_dir=/opt/VirtualBox/amd64 + else + if test -f /usr/local/lib/virtualbox/VBoxXPCOMC.so; then + vbox_xpcomc_dir=/usr/local/lib/virtualbox + else + if test -f /usr/lib/local/VirtualBox/VBoxXPCOMC.so; then + vbox_xpcomc_dir=/usr/lib/local/VirtualBox + else + if test -f /Application/VirtualBox.app/Contents/MacOS/VBoxXPCOMC.dylib; then + vbox_xpcomc_dir=/Application/VirtualBox.app/Contents/MacOS + fi + fi + fi + fi + fi + fi + fi + fi + fi + + if test -n "$vbox_xpcomc_dir"; then + AC_MSG_RESULT([$vbox_xpcomc_dir]) + with_vbox=yes + else + if test "x$with_vbox" = "xcheck"; then + AC_MSG_RESULT([not found, disabling VirtualBox driver]) + with_vbox=no + else + AC_MSG_RESULT([not found]) + AC_MSG_ERROR([VirtualBox XPCOMC is required for the VirtualBox driver]) + fi + fi +else + if test "x$with_vbox" != "xno"; then + if test -f ${with_vbox}/VBoxXPCOMC.so || test -f ${with_vbox}/VBoxXPCOMC.dylib; then + vbox_xpcomc_dir=$with_vbox + with_vbox=yes + else + AC_MSG_ERROR([$with_vbox does not contain VirtualBox XPCOMC]) + fi + fi +fi + +AC_SUBST([vbox_xpcomc_dir]) + if test "x$with_vbox" = "xyes"; then AC_SEARCH_LIBS([dlopen], [dl], [], [AC_MSG_ERROR([Unable to find dlopen()])]) case $ac_cv_search_dlopen in diff --git a/src/Makefile.am b/src/Makefile.am index ece18a6..d3c7087 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -19,6 +19,7 @@ INCLUDES = \ -DPKGDATADIR=\""$(pkgdatadir)"\" \ -DLOCAL_STATE_DIR=\""$(localstatedir)"\" \ -DGETTEXT_PACKAGE=\"$(PACKAGE)\" \ + -DVBOX_XPCOMC_DIR=\"$(vbox_xpcomc_dir)\" \ $(WARN_CFLAGS) \ $(LOCK_CHECKING_CFLAGS) \ -DIN_LIBVIRT \ diff --git a/src/vbox/vbox_XPCOMCGlue.c b/src/vbox/vbox_XPCOMCGlue.c index 0987c1b..fcae0cb 100644 --- a/src/vbox/vbox_XPCOMCGlue.c +++ b/src/vbox/vbox_XPCOMCGlue.c @@ -192,28 +192,12 @@ int VBoxCGlueInit(void) return tryLoadOne(pszHome, 0); /* - * Try the known standard locations. + * Try the configured location. */ g_szVBoxErrMsg[0] = '\0'; -#if defined(__gnu__linux__) || defined(__linux__) - if (tryLoadOne("/opt/VirtualBox", 1) == 0) - return 0; - if (tryLoadOne("/usr/lib/virtualbox", 1) == 0) - return 0; -#elif defined(__sun__) - if (tryLoadOne("/opt/VirtualBox/amd64", 1) == 0) - return 0; - if (tryLoadOne("/opt/VirtualBox/i386", 1) == 0) - return 0; -#elif defined(__APPLE__) - if (tryLoadOne("/Application/VirtualBox.app/Contents/MacOS", 1) == 0) - return 0; -#elif defined(__FreeBSD__) - if (tryLoadOne("/usr/local/lib/virtualbox", 1) == 0) + + if (tryLoadOne(VBOX_XPCOMC_DIR, 1) == 0) return 0; -#else -# error "port me" -#endif /* * Finally try the dynamic linker search path. -- 1.7.0.4

2 1

[libvirt] [PATCH 0/2]: Small fixes for PCI device assignment
by Chris Lalancette 29 Jun '10

29 Jun '10

This series fixes a couple of bugs found while testing libvirt PCI device assignment.

3 8

[libvirt] [PATCH] virFileResolveLink: guarantee an absolute path
by Eric Blake 29 Jun '10

29 Jun '10

https://bugzilla.redhat.com/show_bug.cgi?id=608092 * src/util/util.c (virFileResolveLink): Use canonicalize_file_name, rather than areadlink. --- For this patch, I chose to avoid any symlink resolution if an lstat shows the last element doesn't need any; but if we know we need symlink resolution, it is much easier to use existing interfaces to correctly resolve relative paths, even if it ends up doing more resolution work than what we strictly need. src/util/util.c | 26 ++++++++++++++++---------- 1 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 445fd4e..d058113 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -63,7 +63,7 @@ # include <mntent.h> #endif -#include "areadlink.h" +#include "dirname.h" #include "virterror_internal.h" #include "logging.h" #include "event.h" @@ -1178,8 +1178,9 @@ int virFileLinkPointsTo(const char *checkLink, /* - * Attempt to resolve a symbolic link, returning the - * real path + * Attempt to resolve a symbolic link, returning an + * absolute path where only the last component is guaranteed + * not to be a symlink. * * Return 0 if path was not a symbolic, or the link was * resolved. Return -1 with errno set upon error @@ -1191,16 +1192,21 @@ int virFileResolveLink(const char *linkpath, *resultpath = NULL; - if (lstat(linkpath, &st) < 0) - return -1; - - if (!S_ISLNK(st.st_mode)) { - if (!(*resultpath = strdup(linkpath))) + /* We don't need the full canonicalization of intermediate + * directories, if linkpath is absolute and the basename is + * already a non-symlink. */ + if (IS_ABSOLUTE_FILE_NAME(linkpath)) { + if (lstat(linkpath, &st) < 0) return -1; - return 0; + + if (!S_ISLNK(st.st_mode)) { + if (!(*resultpath = strdup(linkpath))) + return -1; + return 0; + } } - *resultpath = areadlink (linkpath); + *resultpath = canonicalize_file_name(linkpath); return *resultpath == NULL ? -1 : 0; } -- 1.7.0.1

2 2

[libvirt] Storage segfaults after phyp patches
by Cole Robinson 29 Jun '10

29 Jun '10

Basically: > $ gdb virsh ... > (gdb) run --connect qemu:///session pool-list --all > Starting program: /usr/bin/virsh --connect qemu:///session pool-list --all > [Thread debugging using libthread_db enabled] > Detaching after fork from child process 29579. > > Program received signal SIGSEGV, Segmentation fault. ... > (gdb) bt > #0 _libssh2_channel_open (session=0x0, channel_type=0x7ffff7b82737 "session", > channel_type_len=7, window_size=65536, packet_size=32768, message=0x0, > message_len=0) at channel.c:139 > #1 0x00000030cb008a7c in libssh2_channel_open_ex (session=0x0, > type=0x7ffff7b82737 "session", type_len=7, window_size=65536, > packet_size=32768, msg=<value optimized out>, msg_len=0) at channel.c:338 > #2 0x00007ffff7ac9edb in phypExec (session=0x0, > cmd=0x63ba50 "viosvrcmd -m (null) --id 0 -c 'lsvg'|grep -c '^.*$'", > exit_status=0x7fffffffdbcc, conn=<value optimized out>) > at phyp/phyp_driver.c:124 > #3 0x00007ffff7acae1c in phypNumOfStoragePools (conn=0x6341b0) > at phyp/phyp_driver.c:3032 > #4 0x00007ffff7a771be in virConnectNumOfStoragePools (conn=0x6341b0) > at libvirt.c:7223 > #5 0x0000000000415f9e in cmdPoolList (ctl=0x7fffffffdf80, cmd=0x1) > at virsh.c:4911 > #6 0x0000000000409dcc in vshCommandRun (ctl=0x7fffffffdf80, cmd=0x62d500) > at virsh.c:9647 > #7 0x00000000004186ed in main (argc=5, argv=<value optimized out>) > at virsh.c:10656 Looks like the phyp storage driver is being used instead of the libvirtd storage driver (and then segfaulting). Looked briefly for a fix, but it wasn't obvious to me. Thanks, Cole

5 8

[libvirt] [PATCH] Avoid calling virStorageFileIsSharedFS with NULL
by Laine Stump 29 Jun '10

29 Jun '10

From: Laine Stump <laine(a)redhat.com> This code was just recently added (by me) and didn't account for the fact that stdin_path is sometimes NULL. If it's NULL, and SetSecurityAllLabel fails, a segfault would result. --- src/qemu/qemu_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2a277a5..e8c5c35 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3429,7 +3429,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, if (driver->securityDriver && driver->securityDriver->domainSetSecurityAllLabel && driver->securityDriver->domainSetSecurityAllLabel(vm, stdin_path) < 0) { - if (virStorageFileIsSharedFS(stdin_path) != 1) + if (stdin_path && virStorageFileIsSharedFS(stdin_path) != 1) goto cleanup; } -- 1.7.1

3 3

[libvirt] [PATCHv3 0/4] phyp: add storage management
by Eric Blake 28 Jun '10

28 Jun '10

Here's my promised refactoring of the patches. Eduardo Otubo (2): phyp: add rudimentary storage driver phyp: add storage management driver Eric Blake (2): phyp: reduce scope of driver functions phyp: optimize use of sed src/phyp/phyp_driver.c | 3542 +++++++++++++++++++++++++++++++++++------------- src/phyp/phyp_driver.h | 76 +- 2 files changed, 2621 insertions(+), 997 deletions(-)

3 13

[libvirt] [PATCH] lxc: Fix virsh console doesn't work after restarting libvirtd
by Ryota Ozaki 28 Jun '10

28 Jun '10

Because tty path is unexpectedly not saved in the live configuration file of a domain, libvirtd cannot get the console of the domain back after restarting. The reason why the tty path isn't saved is that, to save the tty path, the save function, virDomainSaveConfig, requires that the target domain is running (pid != -1), however, lxc driver calls the function before starting the domain to pass the configuration to controller. To ensure to save the tty path, the patch lets lxc driver call the save function again after starting the domain. --- src/lxc/lxc_driver.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 19d4dcb..462bc9c 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1284,7 +1284,7 @@ static int lxcVmStart(virConnectPtr conn, if (lxcSetupInterfaces(conn, vm->def, &nveths, &veths) != 0) goto cleanup; - /* Persist the live configuration now we have veth & tty info */ + /* Save the configuration for the controller */ if (virDomainSaveConfig(driver->stateDir, vm->def) < 0) goto cleanup; @@ -1328,6 +1328,13 @@ static int lxcVmStart(virConnectPtr conn, goto cleanup; } + /* + * Again, need to save the live configuration, because the function + * requires vm->def->id != -1 to save tty info surely. + */ + if (virDomainSaveConfig(driver->stateDir, vm->def) < 0) + goto cleanup; + rc = 0; cleanup: -- 1.6.5.2

3 2

[libvirt] [PATCHv2 0/4] Fix domain restore problems when selinux is enforcing
by Laine Stump 28 Jun '10

28 Jun '10

Changes from V1: 1) Don't set context label of kernel *or* image file, set the context label of both. 2) Add a patch to enhance virStorageFileIsSharedFS to behave similarly to the ill-fated virFileIsOnNetworkShare. 3) Use virStorageFileIsSharedFS instead of virStorageFileIsSharedFS. Prior to this patch series, restoring a domain with selinux set to enforcing would fail, because the function that sets the label on the file to allow qemu to read it did not have the name of the file (see the comments in the individual patches). A patch from Jamie Stranboge (2b57478ef0a0a983cc6a47b98300c8359f9708d0) added the filename to the args passed down into the security driver; the first patch of this series takes advantage of that to properly set the label. Patches 2 - 4 solve a problem with restoring a domain from an NFS share - in this case the selinux functions will fail (as will functions trying to set the uid of the file, if it is a root-squashed share). The solution to this is just ignore the failure. If the security driver fails to set the label, and virStorageFileIsSharedFS() tells us that the file is on a network-shared FS, we ignore the failure, otherwise we behave as before. qemudDomainSaveFlag previously had a bit of code that detected if a particular path was on an NFS share; this code was replaced with a call to virStorageFileIsSharedFS, which is now functionally equivalent (better, even, since it detects a few other types of network filesystems).

3 10

[libvirt] [PATCH] Fix a reference leak for node devices.
by Chris Lalancette 28 Jun '10

28 Jun '10

There was one major, and a few minor bugs having to do with the reference counting of node devices in daemon/remote.c The major bug was that remoteDispatchNodeDeviceListCaps() was completely failing to unreference node devices; this would lead to many open file descriptors, which would eventually fail. The minor bugs were along the same lines, but were in rarely used error paths. Still, they should be corrected. Signed-off-by: Chris Lalancette <clalance(a)redhat.com> --- daemon/remote.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 91faa9a..ec5f85b 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -5012,11 +5012,13 @@ remoteDispatchNodeDeviceGetParent (struct qemud_server *server ATTRIBUTE_UNUSED, /* remoteDispatchClientRequest will free this. */ char **parent_p; if (VIR_ALLOC(parent_p) < 0) { + virNodeDeviceFree(dev); remoteDispatchOOMError(rerr); return -1; } *parent_p = strdup(parent); if (*parent_p == NULL) { + virNodeDeviceFree(dev); remoteDispatchOOMError(rerr); return -1; } @@ -5048,6 +5050,7 @@ remoteDispatchNodeDeviceNumOfCaps (struct qemud_server *server ATTRIBUTE_UNUSED, ret->num = virNodeDeviceNumOfCaps(dev); if (ret->num < 0) { + virNodeDeviceFree(dev); remoteDispatchConnError(rerr, conn); return -1; } @@ -5076,6 +5079,7 @@ remoteDispatchNodeDeviceListCaps (struct qemud_server *server ATTRIBUTE_UNUSED, } if (args->maxnames > REMOTE_NODE_DEVICE_NAME_LIST_MAX) { + virNodeDeviceFree(dev); remoteDispatchFormatError(rerr, "%s", _("maxnames > REMOTE_NODE_DEVICE_NAME_LIST_MAX")); return -1; @@ -5083,6 +5087,7 @@ remoteDispatchNodeDeviceListCaps (struct qemud_server *server ATTRIBUTE_UNUSED, /* Allocate return buffer. */ if (VIR_ALLOC_N(ret->names.names_val, args->maxnames) < 0) { + virNodeDeviceFree(dev); remoteDispatchOOMError(rerr); return -1; } @@ -5091,11 +5096,13 @@ remoteDispatchNodeDeviceListCaps (struct qemud_server *server ATTRIBUTE_UNUSED, virNodeDeviceListCaps (dev, ret->names.names_val, args->maxnames); if (ret->names.names_len == -1) { + virNodeDeviceFree(dev); remoteDispatchConnError(rerr, conn); VIR_FREE(ret->names.names_val); return -1; } + virNodeDeviceFree(dev); return 0; } -- 1.6.6.1

5 6

[libvirt] [PATCH] lxc: Fix error handlings in lxcContainerRenameAndEnableInterfaces
by Ryota Ozaki 28 Jun '10

28 Jun '10

The function is expected to return negative value on failure, however, it returns positive value when either setInterfaceName or vethInterfaceUpOrDown fails. Because the function returns the return value of either as is, however, the two functions may return positive value on failure. The patch fixes the defects and add error messages. --- src/lxc/lxc_container.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 018f4d5..4371dba 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -255,13 +255,20 @@ static int lxcContainerRenameAndEnableInterfaces(unsigned int nveths, DEBUG("Renaming %s to %s", veths[i], newname); rc = setInterfaceName(veths[i], newname); - if (0 != rc) + if (0 != rc) { + VIR_ERROR(_("Failed to rename %s to %s (%d)"), + veths[i], newname, rc); + rc = -1; goto error_out; + } DEBUG("Enabling %s", newname); - rc = vethInterfaceUpOrDown(newname, 1); - if (0 != rc) + rc = vethInterfaceUpOrDown(newname, 1); + if (0 != rc) { + VIR_ERROR(_("Failed to enable %s (%d)"), newname, rc); + rc = -1; goto error_out; + } VIR_FREE(newname); } -- 1.6.5.2

2 1