- Devel - libvirt List Archives

[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

[libvirt] Just in case it's useful: virsh commands, their version, and their commits
by Justin Clift 07 Dec '10

07 Dec '10

Hi all, Just put a (temporary) page on the wiki showing all of the virsh commands, the version of libvirt each was introduced in, and the commit that did it. http://wiki.libvirt.org/page/VirshCmdAdditionCommits It's very "raw data" style, and at some point in the next week the info will get transferred somewhere better (Virsh Command Reference). But figured other people out there might have a use for it (no idea). Regards and best wishes, Justin Clift

1 0

[libvirt] [PATCH 0/4] Fix up the mess in commandtest
by Jiri Denemark 07 Dec '10

07 Dec '10

commandtest is quite messy with lots of memory leaks, some filedescriptor leaks and few other small issues. This patches fixes them. Jiri Denemark (4): tests: Fix code formating in commandtest tests: Fix leaks in commandtest tests: Fix commandtest in VPATH build tests: Fix detection of expected error tests/commandtest.c | 282 +++++++++++++++++++++++++++++++++------------------ 1 files changed, 184 insertions(+), 98 deletions(-) -- 1.7.3.2

3 16

[libvirt] [PATCH] virsh: Remove redundant commands in group domain
by Osier Yang 07 Dec '10

07 Dec '10

"echo" is already in group "virsh", "freecell" and "hostname" are already in group "host", so remove them from group "domain" * tools/virsh.c --- tools/virsh.c | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 441fd77..9ce3202 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -9956,10 +9956,7 @@ static const vshCmdDef domManagementCmds[] = { {"domxml-to-native", cmdDomXMLToNative, opts_domxmltonative, info_domxmltonative}, {"dump", cmdDump, opts_dump, info_dump}, {"dumpxml", cmdDumpXML, opts_dumpxml, info_dumpxml}, - {"echo", cmdEcho, opts_echo, info_echo}, {"edit", cmdEdit, opts_edit, info_edit}, - {"freecell", cmdFreecell, opts_freecell, info_freecell}, - {"hostname", cmdHostname, NULL, info_hostname}, {"managedsave", cmdManagedSave, opts_managedsave, info_managedsave}, {"managedsave-remove", cmdManagedSaveRemove, opts_managedsaveremove, info_managedsaveremove}, {"maxvcpus", cmdMaxvcpus, opts_maxvcpus, info_maxvcpus}, -- 1.7.3.2

3 2

[libvirt] RFC: Setting a permanent MAC address for virtual network bridges
by Laine Stump 07 Dec '10

07 Dec '10

This is in reference to the following bugs that are filed against RHEL, but applicable to all Linux distros: https://bugzilla.redhat.com/show_bug.cgi?id=560994 https://bugzilla.redhat.com/show_bug.cgi?id=609463 "MAC address of virtual network bridge changes when TAP devices are added/removed" The problem is that the MAC address of a bridge that hasn't had its address specifically set, will be set to the MAC address of the first interface attached to it. If that interface later goes away, it will be set to the MAC address of another attached interface. Daniel's comments in the bug 1) state that the MAC address of a bridge device must match the MAC address of one of the attached interfaces, or no traffic will pass, and 2) suggest creating a dummy interface that is created and used purely to set a fixed MAC address that won't go away when any guest is destroyed (because it's not associated with any guest. I did some quick experimentation and found that: 1) It is possible to set a bridge's MAC address to something that isn't the address of an attached interface (although the range of numbers seems to be limited, at least when setting it using "ip link set virbr0 address xx:xx:xx:xx:xx:xx - anything starting with less than "B2" was rejected). 2) When a bridge's MAC address is set in that way, as Daniel says, no traffic passes. (Why is that, anyway?) 3) If I create a tap device, set a MAC address on it, attach it to the bridge, and make sure the bridge is assigned that MAC address, traffic does pass, and when guests are destroyed and restarted, the MAC address of the bridge doesn't change. So it looks like this is a viable way to solve the problem. Before digging into it, though, I thought I should ask if there are any alternate ideas, or problems that anyone sees with this approach. Also, I'm wondering where the name and MAC address of the dummy interfaces should come from, and whether I need to worry about storing them in the network XML. So far, this is what I think on those topics: 1) the MAC address can be autogenerated if it's not in the XML, but needs to be saved in the XML so that it's persistent across libvirtd restarts. 2) the dummy interface name can just be some derivation of the bridge device name ("virbr0-mac"?), and probably doesn't need to be saved in the XML. (Also, I just noticed the code that sets the first byte of the MAC address of all tap interfaces being attached to virtual network bridges to 0xFE. What's the story behind that? Does the bridge always take the numerically lowest MAC that's currently connected?)

2 2

[libvirt] [Patch 0/2] Add new feature into live migration
by Wen Congyang 07 Dec '10

07 Dec '10

If the memory of guest OS is changed constantly, the live migration can not be ended ever for ever. We can use the command 'virsh migrate-setmaxdowntime' to control the live migration. But the value of maxdowntime is diffcult to calculate because it depends on the transfer speed of network and constantly changing memroy size. We need a easy way to control the live migration. This patch set add the support of auto cold migration fallback on timeout. With this patch set, when we migrate the guest OS, we can specify a timeout. If the live migration timeouts, the migration will fallback to cold migration. TODO: 1. The timer for Windows has not been implemented. Test of this patchset: Env: a. The size of guest OS's memory: 1GB b. The transfer speed of network: about 100Mb/s c. The size of constantly changing memory: more than 900MB 1. migrate without timeout # virsh migrate --live RHEL6RC qemu+ssh://<dest IP>/system tcp://<dest IP>:49152 The migration does not end after 12 hours. 2. migrate with timeout(30 minutes): # date Tue Dec 7 10:00:13 CST 2010 # virsh migrate --live --timeout 1800 RHEL6RC qemu+ssh://10.167.225.67/system tcp:10.167.225.67:49152 # date Tue Dec 7 10:43:28 CST 2010 Wen Congyang (2): timer impl auto cold migration fallback at timeout configure.ac | 4 ++ src/Makefile.am | 6 ++- src/libvirt.c | 2 + src/libvirt_private.syms | 6 ++ src/util/timer.c | 116 ++++++++++++++++++++++++++++++++++++++++++++ src/util/timer.h | 37 ++++++++++++++ src/util/timer_linux.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.c | 53 ++++++++++++++++++++ 8 files changed, 343 insertions(+), 2 deletions(-) create mode 100644 src/util/timer.c create mode 100644 src/util/timer.h create mode 100644 src/util/timer_linux.c

1 2

[libvirt] Virsh commands in two help groups?
by Justin Clift 07 Dec '10

07 Dec '10

Hi Osier, Just noticed that some of the virsh commands are now listed in two groups: freecell: listed in both domain and host groups hostname: listed in both domain and host groups echo: listed in both domain and virsh groups Wondering if that's on purpose, or if it needs to be changed? The "echo" one seems a bit wrong in the domain group, but not sure about the others. :)

2 1