February 2012 - Devel - libvirt List Archives

[libvirt] [PATCH] command: allow merging stdout and stderr in string capture
by Eric Blake 03 Feb '12

03 Feb '12

Sometimes, its easier to run children with 2>&1 in shell notation, and just deal with stdout and stderr interleaved. This was already possible for fd handling; extend it to also work when doing string capture of a child process. * docs/internals/command.html.in: Document this. * src/util/command.c (virCommandSetErrorBuffer): Likewise. (virCommandRun, virExecWithHook): Implement it. * tests/commandtest.c (test14): Test it. * daemon/remote.c (remoteDispatchAuthPolkit): Use new command feature. --- In response to: https://www.redhat.com/archives/libvir-list/2012-January/msg01262.html Hmm, I thought I sent this earlier, but can't find it on the archives. daemon/remote.c | 11 +++-------- docs/internals/command.html.in | 5 ++++- src/util/command.c | 16 ++++++++++++++-- tests/commandtest.c | 26 +++++++++++++++++++++++++- 4 files changed, 46 insertions(+), 12 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 26ac4a6..1cea942 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2477,7 +2477,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, int status = -1; char *ident = NULL; bool authdismissed = 0; - char *pkout = NULL, *pkerr = NULL; + char *pkout = NULL; struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); virCommandPtr cmd = NULL; @@ -2489,7 +2489,7 @@ remoteDispatchAuthPolkit(virNetServerPtr server ATTRIBUTE_UNUSED, cmd = virCommandNewArgList(PKCHECK_PATH, "--action-id", action, NULL); virCommandSetOutputBuffer(cmd, &pkout); - virCommandSetErrorBuffer(cmd, &pkerr); + virCommandSetErrorBuffer(cmd, &pkout); VIR_DEBUG("Start PolicyKit auth %d", virNetServerClientGetFD(client)); if (virNetServerClientGetAuth(client) != VIR_NET_SERVER_SERVICE_AUTH_POLKIT) { @@ -2548,17 +2548,12 @@ error: if (authdismissed) { virNetError(VIR_ERR_AUTH_CANCELLED, "%s", _("authentication cancelled by user")); - } else if (pkout || pkerr) { - virNetError(VIR_ERR_AUTH_FAILED, "%s %s", - pkerr ? pkerr : "", - pkout ? pkout : ""); } else { virNetError(VIR_ERR_AUTH_FAILED, "%s", - _("authentication failed")); + pkout && *pkout ? pkout : _("authentication failed")); } VIR_FREE(pkout); - VIR_FREE(pkerr); virNetMessageSaveError(rerr); virMutexUnlock(&priv->lock); return -1; diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in index 7dcf462..7bb9aa3 100644 --- a/docs/internals/command.html.in +++ b/docs/internals/command.html.in @@ -373,7 +373,10 @@ allocation of collected information (however, on an out-of-memory condition, the buffer may still be NULL). The caller is responsible for freeing registered buffers, since the - buffers are designed to persist beyond virCommandFree. + buffers are designed to persist beyond virCommandFree. It + is possible to pass the same pointer to both + virCommandSetOutputBuffer and virCommandSetErrorBuffer, in which + case the child process interleaves output into a single string. </p> <h3><a name="directory">Setting working directory</a></h3> diff --git a/src/util/command.c b/src/util/command.c index 6f51fbb..a2d5f84 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -464,7 +464,9 @@ virExecWithHook(const char *const*argv, } if (errfd != NULL) { - if (*errfd == -1) { + if (errfd == outfd) { + childerr = childout; + } else if (*errfd == -1) { if (pipe2(pipeerr, O_CLOEXEC) < 0) { virReportSystemError(errno, "%s", _("Failed to create pipe")); @@ -1428,7 +1430,10 @@ virCommandSetOutputBuffer(virCommandPtr cmd, char **outbuf) * guaranteed to be allocated after successful virCommandRun or * virCommandWait, and is best-effort allocated after failed * virCommandRun; caller is responsible for freeing *errbuf. - * This requires the use of virCommandRun. + * This requires the use of virCommandRun. It is possible to + * pass the same pointer as for virCommandSetOutputBuffer(), in + * which case the child process will interleave all output into + * a single string. */ void virCommandSetErrorBuffer(virCommandPtr cmd, char **errbuf) @@ -1940,6 +1945,13 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) cmd->inpipe = infd[1]; } + /* If caller requested the same string for stdout and stderr, then + * merge those into one string. */ + if (cmd->outbuf && cmd->outbuf == cmd->errbuf) { + cmd->errfdptr = &cmd->outfd; + cmd->errbuf = NULL; + } + /* If caller hasn't requested capture of stdout/err, then capture * it ourselves so we can log it. But the intermediate child for * a daemon has no expected output, and we don't want our diff --git a/tests/commandtest.c b/tests/commandtest.c index 9b9130c..3997d2c 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -508,6 +508,14 @@ static int test14(const void *unused ATTRIBUTE_UNUSED) const char *errexpect = "BEGIN STDERR\n" "Hello World\n" "END STDERR\n"; + + char *jointactual = NULL; + const char *jointexpect = "BEGIN STDOUT\n" + "BEGIN STDERR\n" + "Hello World\n" + "Hello World\n" + "END STDOUT\n" + "END STDERR\n"; int ret = -1; virCommandSetInputBuffer(cmd, "Hello World\n"); @@ -523,7 +531,18 @@ static int test14(const void *unused ATTRIBUTE_UNUSED) goto cleanup; virCommandFree(cmd); - cmd = NULL; + + cmd = virCommandNew(abs_builddir "/commandhelper"); + virCommandSetInputBuffer(cmd, "Hello World\n"); + virCommandSetOutputBuffer(cmd, &jointactual); + virCommandSetErrorBuffer(cmd, &jointactual); + if (virCommandRun(cmd, NULL) < 0) { + virErrorPtr err = virGetLastError(); + printf("Cannot run child %s\n", err->message); + goto cleanup; + } + if (!jointactual) + goto cleanup; if (!STREQ(outactual, outexpect)) { virtTestDifference(stderr, outexpect, outactual); @@ -533,6 +552,10 @@ static int test14(const void *unused ATTRIBUTE_UNUSED) virtTestDifference(stderr, errexpect, erractual); goto cleanup; } + if (!STREQ(jointactual, jointexpect)) { + virtTestDifference(stderr, jointexpect, jointactual); + goto cleanup; + } ret = checkoutput("test14"); @@ -540,6 +563,7 @@ cleanup: virCommandFree(cmd); VIR_FREE(outactual); VIR_FREE(erractual); + VIR_FREE(jointactual); return ret; } -- 1.7.7.6

2 2

[libvirt] [PATCH] virsh: extension of virsh attach-disk for rawio
by Taku Izumi 03 Feb '12

03 Feb '12

This patch extends "virsh attach-disk" command so that we can specify "rawio" attribute. Signed-off-by: Taku Izumi <izumi.taku(a)jp.fujitsu.com> --- tools/virsh.c | 3 +++ tools/virsh.pod | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) Index: libvirt/tools/virsh.c =================================================================== --- libvirt.orig/tools/virsh.c +++ libvirt/tools/virsh.c @@ -13883,6 +13883,7 @@ static const vshCmdOptDef opts_attach_di {"sourcetype", VSH_OT_STRING, 0, N_("type of source (block|file)")}, {"serial", VSH_OT_STRING, 0, N_("serial of disk device")}, {"shareable", VSH_OT_BOOL, 0, N_("shareable between domains")}, + {"rawio", VSH_OT_BOOL, 0, N_("needs rawio capability")}, {"address", VSH_OT_STRING, 0, N_("address of disk device")}, {"multifunction", VSH_OT_BOOL, 0, N_("use multifunction pci under specified address")}, @@ -14102,6 +14103,8 @@ cmdAttachDisk(vshControl *ctl, const vsh (isFile) ? "file" : "block"); if (type) virBufferAsprintf(&buf, " device='%s'", type); + if (vshCommandOptBool (cmd, "rawio")) + virBufferAddLit(&buf, " rawio='yes'"); virBufferAddLit(&buf, ">\n"); if (driver || subdriver) Index: libvirt/tools/virsh.pod =================================================================== --- libvirt.orig/tools/virsh.pod +++ libvirt/tools/virsh.pod @@ -1404,7 +1404,7 @@ the device does not use managed mode. =item B<attach-disk> I<domain-id> I<source> I<target> [I<--driver driver>] [I<--subdriver subdriver>] [I<--cache cache>] [I<--type type>] [I<--mode mode>] [I<--persistent>] [I<--sourcetype soucetype>] -[I<--serial serial>] [I<--shareable>] [I<--address address>] +[I<--serial serial>] [I<--shareable>] [I<--rawio>] [I<--address address>] [I<--multifunction>] Attach a new disk device to the domain. @@ -1421,6 +1421,7 @@ I<cache> can be one of "default", "none" "directsync" or "unsafe". I<serial> is the serial of disk device. I<shareable> indicates the disk device is shareable between domains. +I<rawio> indicates the disk is needs rawio capability. I<address> is the address of disk device in the form of pci:domain.bus.slot.function, scsi:controller.bus.unit or ide:controller.bus.unit. I<multifunction> indicates specified pci address is a multifunction pci device -- Taku Izumi <izumi.taku(a)jp.fujitsu.com>

3 2

[libvirt] [PATCH] qemu: fix my typo at commit 74e034964c32edb1732d0ff7642f3977f3587d72
by Taku Izumi 03 Feb '12

03 Feb '12

Fix my typo at commit 74e034964c32edb1732d0ff7642f3977f3587d72 "disk->rawio == -1" indicates that this value is not specified. So in case of this, domain must not be tainted. Signed-off-by: Taku Izumi <izumi.taku(a)jp.fujitsu.com> --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: libvirt/src/qemu/qemu_domain.c =================================================================== --- libvirt.orig/src/qemu/qemu_domain.c +++ libvirt/src/qemu/qemu_domain.c @@ -1260,7 +1260,7 @@ void qemuDomainObjCheckDiskTaint(struct driver->allowDiskFormatProbing) qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_DISK_PROBING, logFD); - if (disk->rawio) + if (disk->rawio == 1) qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, logFD); }

3 3

[libvirt] [PATCH] Fixed connection definition for non-SELinux builds
by Martin Kletzander 03 Feb '12

03 Feb '12

This patch fixes the access of variable "con" in two files where the variable was declared only on SELinux builds and thus the build failed without SELinux. It's a rather nasty fix but helps fix the build quickly and without any major changes to the code. --- src/lxc/lxc_container.c | 2 ++ src/lxc/lxc_controller.c | 2 ++ 2 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index d1156e4..1c148e2 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -449,6 +449,8 @@ static int lxcContainerMountBasicFS(const char *srcprefix, bool pivotRoot) char *opts = NULL; #if HAVE_SELINUX security_context_t con; +#else + bool con = false; #endif VIR_DEBUG("Mounting basic filesystems %s pivotRoot=%d", NULLSTR(srcprefix), pivotRoot); diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index fedb7c8..8f336f5 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -1438,6 +1438,8 @@ lxcControllerRun(virDomainDefPtr def, if (root) { #if HAVE_SELINUX security_context_t con; +#else + bool con = false; #endif char *opts; VIR_DEBUG("Setting up private /dev/pts"); -- 1.7.3.4

3 2

[libvirt] [PATCH 0/2] Added RSS reporting for qemu
by Martin Kletzander 03 Feb '12

03 Feb '12

This patch enables reporting of Resident Set Size for qemu process. It is available through qemudDomainMemoryStats and qemudGetProcessInfo. The reporting is also added into "virsh dommemstat" command. Martin Kletzander (2): Added RSS information gathering into qemudGetProcessInfo Added RSS reporting include/libvirt/libvirt.h.in | 7 +++++- src/qemu/qemu_driver.c | 46 +++++++++++++++++++++++++++++++---------- tools/virsh.c | 2 + 3 files changed, 43 insertions(+), 12 deletions(-) -- 1.7.3.4

2 7

[libvirt] [PATCH] Replace TAB with white spaces
by Osier Yang 03 Feb '12

03 Feb '12

--- src/libvirt.c | 28 ++++++++++++++-------------- 1 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index f58dfa6..e8473c2 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -8519,9 +8519,9 @@ error: * Bytes are stored in little-endian order: CPU0-7, 8-15... * In each byte, lowest CPU number is least significant bit. * @maplen: number of bytes in cpumap, from 1 up to size of CPU map in - * underlying virtualization system (Xen...). - * If maplen < size, missing bytes are set to zero. - * If maplen > size, failure code is returned. + * underlying virtualization system (Xen...). + * If maplen < size, missing bytes are set to zero. + * If maplen > size, failure code is returned. * * Dynamically change the real CPUs which can be allocated to a virtual CPU. * This function may require privileged access to the hypervisor. @@ -8730,15 +8730,15 @@ error: * @maxinfo: number of structures in info array * @cpumaps: pointer to a bit map of real CPUs for all vcpus of this * domain (in 8-bit bytes) (OUT) - * If cpumaps is NULL, then no cpumap information is returned by the API. - * It's assumed there is <maxinfo> cpumap in cpumaps array. - * The memory allocated to cpumaps must be (maxinfo * maplen) bytes - * (ie: calloc(maxinfo, maplen)). - * One cpumap inside cpumaps has the format described in + * If cpumaps is NULL, then no cpumap information is returned by the API. + * It's assumed there is <maxinfo> cpumap in cpumaps array. + * The memory allocated to cpumaps must be (maxinfo * maplen) bytes + * (ie: calloc(maxinfo, maplen)). + * One cpumap inside cpumaps has the format described in * virDomainPinVcpu() API. * @maplen: number of bytes in one cpumap, from 1 up to size of CPU map in - * underlying virtualization system (Xen...). - * Must be zero when cpumaps is NULL and positive when it is non-NULL. + * underlying virtualization system (Xen...). + * Must be zero when cpumaps is NULL and positive when it is non-NULL. * * Extract information about virtual CPUs of domain, store it in info array * and also in cpumaps if this pointer isn't NULL. This call may fail @@ -11160,7 +11160,7 @@ virStoragePoolGetConnect (virStoragePoolPtr pool) * Returns the number of pools found, or -1 on error */ int -virConnectNumOfStoragePools (virConnectPtr conn) +virConnectNumOfStoragePools(virConnectPtr conn) { VIR_DEBUG("conn=%p", conn); @@ -11200,9 +11200,9 @@ error: * Returns 0 on success, -1 on error */ int -virConnectListStoragePools (virConnectPtr conn, - char **const names, - int maxnames) +virConnectListStoragePools(virConnectPtr conn, + char **const names, + int maxnames) { VIR_DEBUG("conn=%p, names=%p, maxnames=%d", conn, names, maxnames); -- 1.7.7.3

2 2

[libvirt] [PATCH] lxc: Fix build with AppArmor
by Jiri Denemark 03 Feb '12

03 Feb '12

--- src/Makefile.am | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index b25a439..dd77eec 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1526,6 +1526,9 @@ endif if WITH_SECDRIVER_SELINUX libvirt_lxc_LDADD += $(SELINUX_LIBS) endif +if WITH_SECDRIVER_APPARMOR +libvirt_lxc_LDADD += $(APPARMOR_LIBS) +endif libvirt_lxc_CFLAGS = \ $(LIBPARTED_CFLAGS) \ $(NUMACTL_CFLAGS) \ @@ -1541,6 +1544,9 @@ endif if WITH_SECDRIVER_SELINUX libvirt_lxc_CFLAGS += $(SELINUX_CFLAGS) endif +if WITH_SECDRIVER_APPARMOR +libvirt_lxc_CFLAGS += $(APPARMOR_CFLAGS) +endif endif endif EXTRA_DIST += $(LXC_CONTROLLER_SOURCES) -- 1.7.8.4

2 2

[libvirt] [PATCH] conf: Plug memory on virDomainDiskDefParseXML
by ajia＠redhat.com 03 Feb '12

03 Feb '12

From: Alex Jia <ajia(a)redhat.com> Detected by valgrind. Leak is introduced in commit 397e6a7. * src/conf/domain_conf.c(virDomainDiskDefParseXML): fix memory leak. How to reproduce? % make -C tests check TESTS=qemuxml2argvtest % cd tests && valgrind -v --leak-check=full ./qemuxml2argvtest * Actual result: ==16352== 4 bytes in 1 blocks are definitely lost in loss record 12 of 147 ==16352== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==16352== by 0x39D90A67DD: xmlStrndup (xmlstring.c:45) ==16352== by 0x4E83D5: virDomainDiskDefParseXML (domain_conf.c:2894) ==16352== by 0x4F542D: virDomainDefParseXML (domain_conf.c:7626) ==16352== by 0x4F8683: virDomainDefParseNode (domain_conf.c:8390) ==16352== by 0x4F904E: virDomainDefParse (domain_conf.c:8340) ==16352== by 0x41C626: testCompareXMLToArgvHelper (qemuxml2argvtest.c:105) ==16352== by 0x41DED1: virtTestRun (testutils.c:142) ==16352== by 0x418172: mymain (qemuxml2argvtest.c:486) ==16352== by 0x41D5C7: virtTestMain (testutils.c:697) ==16352== by 0x39CF01ECDC: (below main) (in /lib64/libc-2.12.so) Signed-off-by: Alex Jia <ajia(a)redhat.com> --- 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 aeef9db..26e24f0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3413,6 +3413,7 @@ cleanup: VIR_FREE(bus); VIR_FREE(type); VIR_FREE(snapshot); + VIR_FREE(rawio); VIR_FREE(target); VIR_FREE(source); while (nhosts > 0) { -- 1.7.1

2 1

[libvirt] [PATCH 1/1] Add detail to documentation on storage pools and volumes.
by Dave Allan 02 Feb '12

02 Feb '12

The storage pools page contains details about the capabilities of the various pool types, but not an overview of how they are intended to be used. This patch adds some explanation of what pools and volumes can be used for and why an administrator might want to use them. --- docs/storage.html.in | 82 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 79 insertions(+), 3 deletions(-) diff --git a/docs/storage.html.in b/docs/storage.html.in index a9c7f1c..0e3e289 100644 --- a/docs/storage.html.in +++ b/docs/storage.html.in @@ -3,9 +3,85 @@ <body> <h1 >Storage Management</h1> <p> -This page describes the backends for the storage management capabilities in -libvirt. -</p> + Libvirt provides storage management on the physical host through + storage pools and volumes. + </p> + <p> + A storage pool is a quantity of storage set aside by an + administrator, often a dedicated storage administrator, for use + by virtual machines. Storage pools are divided into storage + volumes either by the storage administrator or the system + administrator, and the volumes are assigned to VMs as block + devices. + </p> + <p> + For example, the storage administrator responsible for an NFS + server creates a share to store virtual machines' data. The + system administrator defines a pool on the virtualization host + with the details of the share + (e.g. nfs.example.com:/path/to/share should be mounted on + /vm_data). When the pool is started, libvirt mounts the share + on the specified directory, just as if the system administrator + logged in and executed 'mount nfs.example.com:/path/to/share + /vmdata'. If the pool is configured to autostart, libvirt + ensures that the NFS share is mounted on the directory specified + when libvirt is started. + </p> + <p> + Once the pool is started, the files in the NFS share are + reported as volumes, and the storage volumes' paths may be + queried using the libvirt APIs. The volumes' paths can then be + copied into the section of a VM's XML definition describing the + source storage for the VM's block devices. In the case of NFS, + an application using the libvirt APIs can create and delete + volumes in the pool (files in the NFS share) up to the limit of + the size of the pool (the storage capacity of the share). Not + all pool types support creating and deleting volumes. Stopping + the pool (somewhat unfortunately referred to by virsh and the + API as "pool-destroy") undoes the start operation, in this case, + unmounting the NFS share. The data on the share is not modified + by the destroy operation, despite the name. See man virsh for + more details. + </p> + <p> + A second example is an iSCSI pool. A storage administrator + provisions an iSCSI target to present a set of LUNs to the host + running the VMs. When libvirt is configured to manage that + iSCSI target as a pool, libvirt will ensure that the host logs + into the iSCSI target and libvirt can then report the available + LUNs as storage volumes. The volumes' paths can be queried and + used in VM's XML definitions as in the NFS example. In this + case, the LUNs are defined on the iSCSI server, and libvirt + cannot create and delete volumes. + </p> + <p> + Storage pools and volumes are not required for the proper + operation of VMs. Pools and volumes provide a way for libvirt + to ensure that a particular piece of storage will be available + for a VM, but some administrators will prefer to manage their + own storage and VMs will operate properly without any pools or + volumes defined. On systems that do not use pools, system + administrators must ensure the availability of the VMs' storage + using whatever tools they prefer, for example, adding the NFS + share to the host's fstab so that the share is mounted at boot + time. + </p> + <p> + If at this point the value of pools and volumes over traditional + system administration tools is unclear, note that one of the + features of libvirt is its remote protocol, so it's possible to + manage all aspects of a virtual machine's lifecycle as well as + the configuration of the resources required by the VM. These + operations can be performed on a remote host entirely within the + libvirt API. In other words, a management application using + libvirt can enable a user to perform all the required tasks for + configuring the host for a VM: allocating resources, running the + VM, shutting it down and deallocating the resources, without + requiring shell access or any other control channel. + </p> + <p> + Libvirt supports the following storage pool types: + </p> <ul> <li> <a href="#StorageBackendDir">Directory backend</a> -- 1.7.7.6

2 1

[libvirt] [PATCH] tests: avoid test failure on rawhide gnutls
by Eric Blake 02 Feb '12

02 Feb '12

I hit a VERY weird testsuite failure on rawhide, which included _binary_ output to stderr, followed by a hang waiting for me to type something! (Here, using ^@ for NUL): $ ./commandtest TEST: commandtest WARNING: gnome-keyring:: couldn't send data: Bad file descriptor .WARNING: gnome-keyring:: couldn't send data: Bad file descriptor .WARNING: gnome-keyring:: couldn't send data: Bad file descriptor WARNING: gnome-keyring:: couldn't send data: Bad file descriptor .8^@^@^@8^@^@^@^A^@^@^@^Bay^A^@^@^@)PRIVATE-GNOME-KEYRING-PKCS11-PROTOCOL-V-1 I finally traced it to the fact that gnome-keyring, called via gnutls_global_init which is turn called by virNetTLSInit, opens an internal fd that it expects to communicate to via a pthread_atfork handler (never mind that it violates POSIX by using non-async-signal-safe functions in that handler: https://bugzilla.redhat.com/show_bug.cgi?id=772320) Our problem stems from the fact that we pulled the rug out from under the library's expectations by closing an fd that it had just opened. While we aren't responsible for fixing the bugs in that pthread_atfork handler, we can at least avoid the bugs by not closing the fd in the first place. * tests/commandtest.c (mymain): Avoid closing fds that were opened by virInitialize. --- Pushing under the build-breaker rule. It cost me the better part of a morning to track this one down, so I left a super-long comment to help the next person to read the file understand what we're fighting against. tests/commandtest.c | 20 ++++++++++++++++++-- 1 files changed, 18 insertions(+), 2 deletions(-) diff --git a/tests/commandtest.c b/tests/commandtest.c index efc48fe..b4b6044 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -784,6 +784,22 @@ mymain(void) setpgid(0, 0); setsid(); + /* Our test expects particular fd values; to get that, we must not + * leak fds that we inherited from a lazy parent. At the same + * time, virInitialize may open some fds (perhaps via third-party + * libraries that it uses), and we must not kill off an fd that + * this process opens as it might break expectations of a + * pthread_atfork handler, as well as interfering with our tests + * trying to ensure we aren't leaking to our children. The + * solution is to do things in two phases - reserve the fds we + * want by overwriting any externally inherited fds, then + * initialize, then clear the slots for testing. */ + if ((fd = open("/dev/null", O_RDONLY)) < 0 || + dup2(fd, 3) < 0 || + dup2(fd, 4) < 0 || + dup2(fd, 5) < 0 || + (fd > 5 && VIR_CLOSE(fd) < 0)) + return EXIT_FAILURE; /* Prime the debug/verbose settings from the env vars, * since we're about to reset 'environ' */ @@ -791,8 +807,8 @@ mymain(void) virTestGetVerbose(); virInitialize(); - /* Kill off any inherited fds that might interfere with our - * testing. */ + + /* Phase two of killing interfering fds; see above. */ fd = 3; VIR_FORCE_CLOSE(fd); fd = 4; -- 1.7.7.5

3 3