- Devel - libvirt List Archives

[libvirt] [PATCH] virsh: fix a typo in the memtune help description
by Justin Clift 08 Dec '10

08 Dec '10

--- tools/virsh.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 6c48a62..4e37f2d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2894,8 +2894,9 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd) * "memtune" command */ static const vshCmdInfo info_memtune[] = { - {"help", N_("Get/Set memory paramters")}, - {"desc", N_("Get/Set the current memory paramters for the guest domain.\n" \ + {"help", N_("Get or set memory parameters")}, + {"desc", N_("Get or set the current memory parameters for a guest" \ + " domain.\n" \ " To get the memory parameters use following command: \n\n" \ " virsh # memtune <domain>")}, {NULL, NULL} -- 1.7.3.2

3 4

[libvirt] [RFC] cgroups net_cls controller implementation
by D. Herrendoerfer 08 Dec '10

08 Dec '10

This a basic implemantation to support the net_cls feature of cgroups. It adds the setting of a net_cls.classid value to the existing cgroups setup in the qemu driver. The classid is specified in the qemu.conf file. This enables the use of the tc utility to manage traffic from/to vitual machines based on the setting combination of classid and network interface. Signed-off-by: D.Herrendoerfer <d.herrendoerfer [at] herrendoerfer [dot] name > src/libvirt_private.syms | 1 + src/qemu/qemu.conf | 6 +++++- src/qemu/qemu_conf.c | 7 ++++++- src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 12 ++++++++++++ src/util/cgroup.c | 18 +++++++++++++++++- src/util/cgroup.h | 3 +++ 7 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f251c94..771911e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -80,6 +80,7 @@ virCgroupSetFreezerState; virCgroupSetMemory; virCgroupSetMemoryHardLimit; virCgroupSetMemorySoftLimit; +virCgroupSetNetworkClassID; virCgroupSetSwapHardLimit; diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index f4f965e..591d8dc 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -157,7 +157,7 @@ # can be mounted in different locations. libvirt will detect # where they are located. # -# cgroup_controllers = [ "cpu", "devices", "memory" ] +# cgroup_controllers = [ "cpu", "devices", "memory", "net_cls" ] # This is the basic set of devices allowed / required by # all virtual machines. @@ -175,6 +175,10 @@ # "/dev/rtc", "/dev/hpet", "/dev/net/tun", #] +# This is the default classid that will be assigned +# to all virtual machines. +# cgroup_net_cls_classid = 4096 + # The default format for Qemu/KVM guest save images is raw; that is, the # memory from the domain is dumped out directly to a file. If you have diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 7cd0603..46ac040 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -326,7 +326,8 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, driver->cgroupControllers = (1 << VIR_CGROUP_CONTROLLER_CPU) | (1 << VIR_CGROUP_CONTROLLER_DEVICES) | - (1 << VIR_CGROUP_CONTROLLER_MEMORY); + (1 << VIR_CGROUP_CONTROLLER_MEMORY) | + (1 << VIR_CGROUP_CONTROLLER_NETWORK); } for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { if (driver->cgroupControllers & (1 << i)) { @@ -364,6 +365,10 @@ int qemudLoadDriverConfig(struct qemud_driver *driver, driver->cgroupDeviceACL[i] = NULL; } + p = virConfGetValue (conf, "cgroup_net_cls_classid"); + CHECK_TYPE ("cgroup_net_cls_classid", VIR_CONF_LONG); + if (p) driver->cgroupNetClsClassid = p->l; + p = virConfGetValue (conf, "save_image_format"); CHECK_TYPE ("save_image_format", VIR_CONF_STRING); if (p && p->str) { diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index aba64d6..961c6cd 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -119,6 +119,7 @@ struct qemud_driver { virCgroupPtr cgroup; int cgroupControllers; char **cgroupDeviceACL; + int cgroupNetClsClassid; virDomainObjList domains; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1a7c1ad..42448b5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -186,6 +186,8 @@ static int qemudVMFiltersInstantiate(virConnectPtr conn, static struct qemud_driver *qemu_driver = NULL; +#include "interface.h" + static void *qemuDomainObjPrivateAlloc(void) { @@ -3597,6 +3599,16 @@ static int qemuSetupCgroup(struct qemud_driver *driver, vm->def->name); } + if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_NETWORK)) { + + if (driver->cgroupNetClsClassid != 0) { + rc = virCgroupSetNetworkClassID(cgroup, driver- >cgroupNetClsClassid); + if (rc != 0) { + VIR_WARN("Cannot set net_cls.classid for: %s", + vm->def->name); + } + } + } done: virCgroupFree(&cgroup); return 0; diff --git a/src/util/cgroup.c b/src/util/cgroup.c index 2758a8f..a2ed0ed 100644 --- a/src/util/cgroup.c +++ b/src/util/cgroup.c @@ -37,7 +37,7 @@ VIR_ENUM_IMPL(virCgroupController, VIR_CGROUP_CONTROLLER_LAST, "cpu", "cpuacct", "cpuset", "memory", "devices", - "freezer"); + "freezer", "net_cls"); struct virCgroupController { int type; @@ -851,6 +851,22 @@ int virCgroupForDomain(virCgroupPtr driver ATTRIBUTE_UNUSED, #endif /** + * virCgroupSetNetworkClassID: + * + * @group: The cgroup to change memory for + * @classid: The classID number + * + * Returns: 0 on success + */ +int virCgroupSetNetworkClassID(virCgroupPtr group, unsigned long classid) +{ + return virCgroupSetValueU64(group, + VIR_CGROUP_CONTROLLER_NETWORK, + "net_cls.classid", + classid); +} + +/** * virCgroupSetMemory: * * @group: The cgroup to change memory for diff --git a/src/util/cgroup.h b/src/util/cgroup.h index 9e1c61f..9626e82 100644 --- a/src/util/cgroup.h +++ b/src/util/cgroup.h @@ -22,6 +22,7 @@ enum { VIR_CGROUP_CONTROLLER_MEMORY, VIR_CGROUP_CONTROLLER_DEVICES, VIR_CGROUP_CONTROLLER_FREEZER, + VIR_CGROUP_CONTROLLER_NETWORK, VIR_CGROUP_CONTROLLER_LAST }; @@ -40,6 +41,8 @@ int virCgroupForDomain(virCgroupPtr driver, int virCgroupAddTask(virCgroupPtr group, pid_t pid); +int virCgroupSetNetworkClassID(virCgroupPtr group, unsigned long classid); + int virCgroupSetMemory(virCgroupPtr group, unsigned long kb); int virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb);

3 4

[libvirt] cancel cloning
by Zdenek Styblik 08 Dec '10

08 Dec '10

Hello, please, is there some way how to cancel VM cloning in progress? To be more specific, to cancel volume allocation. And since we are on the topic, why is volume allocation so slow even for VM that's shut off? Cloning has been initiated via virt-tools resp. virt-manager. Thank you for reply, Zdenek -- Zdenek Styblik Net/Linux admin OS TurnovFree.net email: stybla(a)turnovfree.net jabber: stybla(a)jabber.turnovfree.net

1 1

[libvirt] [PATCH] docs: updated virsh command reference download links to new version
by Justin Clift 08 Dec '10

08 Dec '10

The new version lists every virsh command, its description, and the version of libvirt where it became available. --- docs/virshcmdref.html.in | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/virshcmdref.html.in b/docs/virshcmdref.html.in index 0a19ca7..352c6b3 100644 --- a/docs/virshcmdref.html.in +++ b/docs/virshcmdref.html.in @@ -49,21 +49,21 @@ <ul> <li> Standard HTML format, multiple pages - (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html…">.tar.gz</a>) - (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html…">.tar.bz2</a>) - (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html…">.zip</a>) + (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html…">.tar.gz</a>) + (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html…">.tar.bz2</a>) + (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html…">.zip</a>) </li> <li> HTML format, one long page - (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html…">.tar.gz</a>) - (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html…">.tar.bz2</a>) - (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0-html…">.zip</a>) + (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html…">.tar.gz</a>) + (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html…">.tar.bz2</a>) + (<a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1-html…">.zip</a>) </li> <li> - <a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0.pdf">PDF format</a> + <a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1.pdf">PDF format</a> </li> <li> - <a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-0.epub">ePub format</a> + <a href="http://libvirt.org/sources/virshcmdref/Virsh_Command_Reference-0.8.6-1.epub">ePub format</a> </li> </ul> -- 1.7.3.2

1 1

[libvirt] [PATCH] configure: improve misleading libnl missing error message
by Justin Clift 08 Dec '10

08 Dec '10

This fixes a misleading error message saying the libnl package needs to be installed, when it's really the libnl-devel package needing to be installed. --- configure.ac | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/configure.ac b/configure.ac index d26bc68..dde2cde 100644 --- a/configure.ac +++ b/configure.ac @@ -2215,7 +2215,7 @@ LIBNL_LIBS="" if test "$with_macvtap" = "yes"; then PKG_CHECK_MODULES([LIBNL], [libnl-1 >= $LIBNL_REQUIRED], [ ], [ - AC_MSG_ERROR([libnl >= $LIBNL_REQUIRED is required for macvtap support]) + AC_MSG_ERROR([libnl-devel >= $LIBNL_REQUIRED is required for macvtap support]) ]) fi -- 1.7.3.2

2 2

[libvirt] [PATCH] maint: update to latest gnulib
by Eric Blake 08 Dec '10

08 Dec '10

* .gnulib: Update to latest, for at least a stdint.h fix * src/storage/storage_driver.c (storageVolumeZeroSparseFile) (storageWipeExtent): Use better type, although it still triggers spurious -Wformat warning on MacOS's gcc. --- * .gnulib 48b1a1a...6491120 (47): > unistr/u8-strcmp: Avoid collision with libc function on Solaris 11. > Update internal documentation. > Put more information about failed tests into the test return codes. > Update for Solaris 11 2010-11. > nproc: Relax license. > maint: restore executable bit > utimecmp: fine-grained src to nearby coarse-grained dest > strerror_r-posix: Fix autoconf test. > Tests for module 'getdomainname'. > getdomainname: Use the system function when possible. > sys_socket: Ensure ss_family field on AIX. > readline: Improve configure output. > *printf-posix: Detect a bug on Solaris 10/x86. > Fix link error when module libunistring-optional is in use. > regex: Mention link dependencies. > ftoastr: Fix compilation error on Solaris. > Oops, fix typo in last commit. > autoupdate > autoupdate > getloadavg: Update documentation. > sys_socket: Fix test whether the functions are declared. > getpass: Make sure to get system declaration on some platforms. > iconv-h: Fix test-iconv-h-c++ failure on Solaris 11 2010-11. > ftoastr: comment fix > stdint: port to GCC 4.3 + OSX + Octave > doc: Corrections regarding MacOS X 10.4 and 10.5. > autoupdate > Uninstall ".bin" files installed by relocwrapper. > Update for NetBSD 5.0. > Update for HP-UX 11.23 and HP-UX 11.31. > Update for MacOS X 10.5. > bootstrap: add bootstrap_sync option. > Oops, fix incomplete ChangeLog entry. > Ensure that <features.h> is included before __GLIBC__ is tested. > memmem: Fix autoconf test. > Port to uClibc. > nproc: Fix condition. > Fix a comment. * lib/vasnprintf.c (VASNPRINTF): Fix comment. > ftoastr: don't assume snprintf > test-rename.h: fix compilation failure > maint.mk: add a URL discussing the no-@acronym policy > ftoastr: depend on snprintf, improve comments > ftoastr: port to hosts lacking strtof and strtold > c-strtold: Avoid link error on AIX 7. > intprops: new macro INT_BITS_STRLEN_BOUND > ftoastr: new module, for lossless conversion of floats to short strings > autoupdate .gnulib | 2 +- src/storage/storage_driver.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.gnulib b/.gnulib index 48b1a1a..6491120 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 48b1a1ae7d5b0f1494aabd4c8a02fbfcec531026 +Subproject commit 64911207854610668b480939469282fdaeb96f74 diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 0f099f0..67d043b 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1575,7 +1575,7 @@ storageVolumeZeroSparseFile(virStorageVolDefPtr vol, virReportSystemError(errno, _("Failed to truncate volume with " "path '%s' to %ju bytes"), - vol->target.path, (intmax_t)size); + vol->target.path, (uintmax_t)size); } out: @@ -1597,13 +1597,13 @@ storageWipeExtent(virStorageVolDefPtr vol, size_t write_size = 0; VIR_DEBUG("extent logical start: %ju len: %ju", - (intmax_t)extent_start, (intmax_t)extent_length); + (uintmax_t)extent_start, (uintmax_t)extent_length); if ((ret = lseek(fd, extent_start, SEEK_SET)) < 0) { virReportSystemError(errno, _("Failed to seek to position %ju in volume " "with path '%s'"), - (intmax_t)extent_start, vol->target.path); + (uintmax_t)extent_start, vol->target.path); goto out; } -- 1.7.3.2

2 2

[libvirt] [PATCH] Moved the nodeinfo command to the 'host' help keyword group
by Justin Clift 08 Dec '10

08 Dec '10

--- tools/virsh.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 9ce3202..6c48a62 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -10062,7 +10062,6 @@ static const vshCmdDef nodedevCmds[] = { {"nodedev-list", cmdNodeListDevices, opts_node_list_devices, info_node_list_devices}, {"nodedev-reattach", cmdNodeDeviceReAttach, opts_node_device_reattach, info_node_device_reattach}, {"nodedev-reset", cmdNodeDeviceReset, opts_node_device_reset, info_node_device_reset}, - {"nodeinfo", cmdNodeinfo, NULL, info_nodeinfo}, {NULL, NULL, NULL, NULL} }; @@ -10127,6 +10126,7 @@ static const vshCmdDef hostAndHypervisorCmds[] = { {"connect", cmdConnect, opts_connect, info_connect}, {"freecell", cmdFreecell, opts_freecell, info_freecell}, {"hostname", cmdHostname, NULL, info_hostname}, + {"nodeinfo", cmdNodeinfo, NULL, info_nodeinfo}, {"qemu-monitor-command", cmdQemuMonitorCommand, opts_qemu_monitor_command, info_qemu_monitor_command}, {"uri", cmdURI, NULL, info_uri}, {NULL, NULL, NULL, NULL} -- 1.7.3.2

2 2

[libvirt] [PATCH] openvz: convert popen to virCommand
by Eric Blake 08 Dec '10

08 Dec '10

popen must be matched with pclose (not fclose), or it will leak resources. Furthermore, it is a lousy interface when it comes to signal handling. We're much better off using our decent command wrapper. * src/openvz/openvz_conf.c (openvzLoadDomains, openvzGetVEID): Replace popen with virCommand usage. --- src/openvz/openvz_conf.c | 54 +++++++++++++++++++++++---------------------- 1 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 863af93..9d2689a 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -51,6 +51,7 @@ #include "util.h" #include "nodeinfo.h" #include "files.h" +#include "command.h" #define VIR_FROM_THIS VIR_FROM_OPENVZ @@ -433,26 +434,26 @@ openvzFreeDriver(struct openvz_driver *driver) int openvzLoadDomains(struct openvz_driver *driver) { - FILE *fp; int veid, ret; char status[16]; char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainObjPtr dom = NULL; char temp[50]; + char *outbuf = NULL; + char *line; + virCommandPtr cmd = NULL; if (openvzAssignUUIDs() < 0) return -1; - if ((fp = popen(VZLIST " -a -ovpsid,status -H 2>/dev/null", "r")) == NULL) { - openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("popen failed")); - return -1; - } - - while (!feof(fp)) { - if (fscanf(fp, "%d %s\n", &veid, status) != 2) { - if (feof(fp)) - break; + cmd = virCommandNewArgList(VZLIST, "-a", "-ovpsid,status", "-H", NULL); + virCommandSetOutputBuffer(cmd, &outbuf); + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + line = *outbuf ? outbuf : NULL; + while (line) { + if (sscanf(line, "%d %s\n", &veid, status) != 2) { openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to parse vzlist output")); goto cleanup; @@ -526,9 +527,14 @@ int openvzLoadDomains(struct openvz_driver *driver) { virDomainObjUnlock(dom); dom = NULL; + + line = strchr(line, '\n'); + if (line) + line++; } - VIR_FORCE_FCLOSE(fp); + virCommandFree(cmd); + VIR_FREE(outbuf); return 0; @@ -536,7 +542,8 @@ int openvzLoadDomains(struct openvz_driver *driver) { virReportOOMError(); cleanup: - VIR_FORCE_FCLOSE(fp); + virCommandFree(cmd); + VIR_FREE(outbuf); if (dom) virDomainObjUnref(dom); return -1; @@ -978,27 +985,22 @@ static int openvzAssignUUIDs(void) */ int openvzGetVEID(const char *name) { - char *cmd; + virCommandPtr cmd; + char *outbuf; int veid; - FILE *fp; bool ok; - if (virAsprintf(&cmd, "%s %s -ovpsid -H", VZLIST, name) < 0) { - openvzError(VIR_ERR_INTERNAL_ERROR, "%s", - _("virAsprintf failed")); + cmd = virCommandNewArgList(VZLIST, name, "-ovpsid", "-H", NULL); + virCommandSetOutputBuffer(cmd, &outbuf); + if (virCommandRun(cmd, NULL) < 0) { + virCommandFree(cmd); return -1; } - fp = popen(cmd, "r"); - VIR_FREE(cmd); - - if (fp == NULL) { - openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("popen failed")); - return -1; - } + virCommandFree(cmd); + ok = sscanf(outbuf, "%d\n", &veid) == 1; + VIR_FREE(outbuf); - ok = fscanf(fp, "%d\n", &veid ) == 1; - VIR_FORCE_FCLOSE(fp); if (ok && veid >= 0) return veid; -- 1.7.3.2

2 7

[libvirt] [PATCH 1/2] virCommand: document behavior on no output
by Eric Blake 07 Dec '10

07 Dec '10

Option 1: This patch (all callers have to worry about NULL buffers, but checking for output is a simple pointer check). Option 2: Guarantee that outbuf/errbuf are allocated, even if to the empty string. Caller always has to free the result, and empty output check requires checking if *outbuf=='\0'. Personally, I prefer option 2. Thoughts? * docs/internals/command.html.in: Update documentation. * src/util/command.c (virCommandSetOutputBuffer) (virCommandSetErrorBuffer) Guarantee NULL buffer on no output. --- docs/internals/command.html.in | 4 ++-- src/util/command.c | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in index c4fa800..ea9ec64 100644 --- a/docs/internals/command.html.in +++ b/docs/internals/command.html.in @@ -349,8 +349,8 @@ <p> Once the command has finished executing, these buffers - will contain the output. It is the callers responsibility - to free these buffers. + will contain the output, or be NULL if there was no output. It + is the callers responsibility to free these buffers. </p> <h3><a name="directory">Setting working directory</a></h3> diff --git a/src/util/command.c b/src/util/command.c index aa43f76..1923799 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -549,6 +549,7 @@ virCommandSetOutputBuffer(virCommandPtr cmd, char **outbuf) return; } + VIR_FREE(*outbuf); cmd->outbuf = outbuf; cmd->outfdptr = &cmd->outfd; } @@ -569,6 +570,7 @@ virCommandSetErrorBuffer(virCommandPtr cmd, char **errbuf) return; } + VIR_FREE(*errbuf); cmd->errbuf = errbuf; cmd->errfdptr = &cmd->errfd; } -- 1.7.3.2

5 8

[libvirt] [PATCH] command: enforce fd vs. buffer considerations
by Eric Blake 07 Dec '10

07 Dec '10

* docs/internals/command.html.in: Better documentation of buffer vs. fd considerations. * src/util/command.c (virCommandRunAsync): Reject raw execution with string io. (virCommandRun): Reject execution with user-specified fds not visiting a regular file. --- Perhaps we need to relax the fstat check to permit block devices in addition to regular files; but that can be a later patch if needed. docs/internals/command.html.in | 35 ++++++++++++++++++++++++++--------- src/util/command.c | 37 ++++++++++++++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in index c4fa800..259e68e 100644 --- a/docs/internals/command.html.in +++ b/docs/internals/command.html.in @@ -250,7 +250,7 @@ allow an open file handle to be passed into the child, while controlling whether that handle remains open in the parent or guaranteeing that the handle will be closed in the parent after - either virCommandRun or virCommandFree. + virCommandRun, virCommandRunAsync, or virCommandFree. </p> <pre> @@ -266,9 +266,16 @@ With this, both file descriptors sharedfd and childfd in the current process remain open as the same file descriptors in the child. Meanwhile, after the child is spawned, sharedfd remains - open in the parent, while childfd is closed. For stdin/out/err - it is usually necessary to map a file handle. To attach file - descriptor 7 in the current process to stdin in the child: + open in the parent, while childfd is closed. + </p> + + <p> + For stdin/out/err it is sometimes necessary to map a file + handle. If a mapped file handle is a pipe fed or consumed by + the caller, then the caller should use virCommandDaemonize or + virCommandRunAsync rather than virCommandRun to avoid deadlock + (mapping a regular file is okay with virCommandRun). To attach + file descriptor 7 in the current process to stdin in the child: </p> <pre> @@ -322,11 +329,21 @@ <h3><a name="buffers">Feeding & capturing strings to/from the child</a></h3> <p> - Often dealing with file handles for stdin/out/err - is unnecessarily complex. It is possible to specify - a string buffer to act as the data source for the - child's stdin, if there are no embedded NUL bytes, - and if the command will be run with virCommandRun: + Often dealing with file handles for stdin/out/err is + unnecessarily complex; an alternative is to let virCommandRun + perform the I/O and interact via string buffers. Use of a buffer + only works with virCommandRun, and cannot be mixed with pipe + file descriptors. That is, the choice is generally between + managing all I/O in the caller (any fds not specified are tied + to /dev/null), or letting virCommandRun manage all I/O via + strings (unspecified stdin is tied to /dev/null, and unspecified + output streams get logged but are otherwise discarded). + </p> + + <p> + It is possible to specify a string buffer to act as the data + source for the child's stdin, if there are no embedded NUL + bytes, and if the command will be run with virCommandRun: </p> <pre> diff --git a/src/util/command.c b/src/util/command.c index c0520ec..d1d8f6d 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -25,6 +25,7 @@ #include <stdarg.h> #include <stdbool.h> #include <stdlib.h> +#include <sys/stat.h> #include <sys/wait.h> #include "command.h" @@ -872,6 +873,9 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) char *outbuf = NULL; char *errbuf = NULL; int infd[2]; + struct stat st; + bool string_io; + bool async_io = false; if (!cmd ||cmd->has_error == ENOMEM) { virReportOOMError(); @@ -883,6 +887,28 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) return -1; } + /* Avoid deadlock, by requiring that any open fd not under our + * control must be visiting a regular file, or that we are + * daemonized and no string io is required. */ + string_io = cmd->inbuf || cmd->outbuf || cmd->errbuf; + if (cmd->infd != -1 && + (fstat(cmd->infd, &st) < 0 || !S_ISREG(st.st_mode))) + async_io = true; + if (cmd->outfdptr && cmd->outfdptr != &cmd->outfd && + (*cmd->outfdptr == -1 || + fstat(*cmd->outfdptr, &st) < 0 || !S_ISREG(st.st_mode))) + async_io = true; + if (cmd->errfdptr && cmd->errfdptr != &cmd->errfd && + (*cmd->errfdptr == -1 || + fstat(*cmd->errfdptr, &st) < 0 || !S_ISREG(st.st_mode))) + async_io = true; + if (async_io ? (!(cmd->flags & VIR_EXEC_DAEMON) || string_io) + : ((cmd->flags & VIR_EXEC_DAEMON) && string_io)) { + virCommandError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid use of command API")); + return -1; + } + /* If we have an input buffer, we need * a pipe to feed the data to the child */ if (cmd->inbuf) { @@ -921,7 +947,7 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) return -1; } - if (cmd->inbuf || cmd->outbuf || cmd->errbuf) + if (string_io) ret = virCommandProcessIO(cmd); if (virCommandWait(cmd, exitstatus) < 0) @@ -1001,6 +1027,15 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid) return -1; } + /* Buffer management can only be requested via virCommandRun. */ + if ((cmd->inbuf && cmd->infd == -1) || + (cmd->outbuf && cmd->outfdptr != &cmd->outfd) || + (cmd->errbuf && cmd->errfdptr != &cmd->errfd)) { + virCommandError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid use of command API")); + return -1; + } + if (cmd->pid != -1) { virCommandError(VIR_ERR_INTERNAL_ERROR, _("command is already running as pid %d"), -- 1.7.3.2

2 2