March 2011 - Devel - libvirt List Archives

[libvirt] [PATCH] virsh: Insert error messages to avoid a quiet abortion of commands
by Michal Privoznik 16 Mar '11

16 Mar '11

in case of incorrect option parsing. --- My former patch introduced a regression: https://bugzilla.redhat.com/show_bug.cgi?id=605660 tools/virsh.c | 52 ++++++++++++++++++++++++++++++++++++++-------------- 1 files changed, 38 insertions(+), 14 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index b42aac4..42ebd55 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -706,8 +706,10 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) } VIR_FREE(ctl->name); - if (vshCommandOptString(cmd, "name", &name) <= 0) + if (vshCommandOptString(cmd, "name", &name) < 0) { + vshError(ctl, "%s", _("Please specify valid connection URI")); return FALSE; + } ctl->name = vshStrdup(ctl, name); if (!ro) { @@ -773,7 +775,7 @@ static int cmdConsole(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; - int ret; + int ret = FALSE; const char *name = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) @@ -782,11 +784,14 @@ cmdConsole(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return FALSE; - if (vshCommandOptString(cmd, "devname", &name) < 0) - return FALSE; + if (vshCommandOptString(cmd, "devname", &name) < 0) { + vshError(ctl, "%s", _("Invalid devname")); + goto cleanup; + } ret = cmdRunConsole(ctl, dom, name); +cleanup: virDomainFree(dom); return ret; } @@ -2410,8 +2415,10 @@ cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd) const char *type = NULL; int vcpus; - if (vshCommandOptString(cmd, "type", &type) < 0) + if (vshCommandOptString(cmd, "type", &type) < 0) { + vshError(ctl, "%s", _("Invalid type")); return FALSE; + } if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; @@ -2856,8 +2863,10 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return FALSE; - if (vshCommandOptInt(cmd, "count", &count) < 0) - return FALSE; + if (vshCommandOptInt(cmd, "count", &count) < 0) { + vshError(ctl, "%s", _("Invalid number of virtual CPUs")); + goto cleanup; + } if (!flags) { if (virDomainSetVcpus(dom, count) != 0) { @@ -3705,8 +3714,10 @@ doMigrate (void *opaque) if (vshCommandOptString(cmd, "desturi", &desturi) <= 0 || vshCommandOptString(cmd, "migrateuri", &migrateuri) < 0 || - vshCommandOptString(cmd, "dname", &dname) < 0) + vshCommandOptString(cmd, "dname", &dname) < 0) { + vshError(ctl, "%s", _("missing argument")); goto out; + } if (vshCommandOptBool (cmd, "live")) flags |= VIR_MIGRATE_LIVE; @@ -5509,8 +5520,10 @@ static int buildPoolXML(const vshCmd *cmd, const char **retname, char **xml) { vshCommandOptString(cmd, "source-dev", &srcDev) < 0 || vshCommandOptString(cmd, "source-name", &srcName) < 0 || vshCommandOptString(cmd, "source-format", &srcFormat) < 0 || - vshCommandOptString(cmd, "target", &target) < 0) + vshCommandOptString(cmd, "target", &target) < 0) { + vshError(NULL, "%s", _("missing argument")); goto cleanup; + } virBufferVSprintf(&buf, "<pool type='%s'>\n", type); virBufferVSprintf(&buf, " <name>%s</name>\n", name); @@ -6314,8 +6327,10 @@ cmdPoolDiscoverSourcesAs(vshControl * ctl, const vshCmd * cmd ATTRIBUTE_UNUSED) if (vshCommandOptString(cmd, "type", &type) <= 0 || vshCommandOptString(cmd, "host", &host) < 0 || - vshCommandOptString(cmd, "initiator", &initiator) < 0) + vshCommandOptString(cmd, "initiator", &initiator) < 0) { + vshError(ctl,"%s", _("missing argument")); return FALSE; + } if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; @@ -6386,8 +6401,10 @@ cmdPoolDiscoverSources(vshControl * ctl, const vshCmd * cmd ATTRIBUTE_UNUSED) if (vshCommandOptString(cmd, "type", &type) <= 0) return FALSE; - if (vshCommandOptString(cmd, "srcSpec", &srcSpecFile) < 0) + if (vshCommandOptString(cmd, "srcSpec", &srcSpecFile) < 0) { + vshError(ctl, "%s", _("missing option")); return FALSE; + } if (!vshConnectionUsability(ctl, ctl->conn)) return FALSE; @@ -8862,8 +8879,10 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd) vshCommandOptString(cmd, "target", &target) < 0 || vshCommandOptString(cmd, "mac", &mac) < 0 || vshCommandOptString(cmd, "script", &script) < 0 || - vshCommandOptString(cmd, "model", &model) < 0) + vshCommandOptString(cmd, "model", &model) < 0) { + vshError(ctl, "missing argument"); goto cleanup; + } /* check interface type */ if (STREQ(type, "network")) { @@ -8969,8 +8988,10 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptString(cmd, "type", &type) <= 0) goto cleanup; - if (vshCommandOptString(cmd, "mac", &mac) < 0) + if (vshCommandOptString(cmd, "mac", &mac) < 0) { + vshError(ctl, "%s", _("missing option")); goto cleanup; + } doc = virDomainGetXMLDesc(dom, 0); if (!doc) @@ -9120,6 +9141,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) vshCommandOptString(cmd, "type", &type) < 0 || vshCommandOptString(cmd, "mode", &mode) < 0 || vshCommandOptString(cmd, "sourcetype", &stype) < 0) { + vshError(ctl, "%s", _("missing option")); goto cleanup; } @@ -11272,8 +11294,10 @@ vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd, if (vshCommandOptString(cmd, optname, &n) <= 0) return NULL; - if (vshCommandOptString(cmd, pooloptname, &p) < 0) + if (vshCommandOptString(cmd, pooloptname, &p) < 0) { + vshError(ctl, "%s", _("missing option")); return NULL; + } if (p) pool = vshCommandOptPoolBy(ctl, cmd, pooloptname, name, flag); -- 1.7.4

3 5

[libvirt] [PATCH] Ensure binary is resolved wrt $PATH in virExec
by Daniel P. Berrange 16 Mar '11

16 Mar '11

virExec would only resolved the binary to $PATH if no env variables were being set. Since there is no execvep() API in POSIX, we use virFindFileInPath to manually resolve the binary and then use execv() instead of execvp(). --- src/util/util.c | 16 ++++++++++++++-- 1 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index e573f4a..a30a542 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -475,6 +475,18 @@ __virExec(const char *const*argv, int childout = -1; int childerr = -1; int tmpfd; + const char *binary = NULL; + + if (argv[0][0] != '/') { + if (!(binary = virFindFileInPath(argv[0]))) { + virReportSystemError(ENOENT, + _("Cannot find '%s' in path"), + argv[0]); + return -1; + } + } else { + binary = argv[0]; + } if ((null = open("/dev/null", O_RDWR)) < 0) { virReportSystemError(errno, @@ -694,9 +706,9 @@ __virExec(const char *const*argv, virLogReset(); if (envp) - execve(argv[0], (char **) argv, (char**)envp); + execve(binary, (char **) argv, (char**)envp); else - execvp(argv[0], (char **) argv); + execv(binary, (char **) argv); virReportSystemError(errno, _("cannot execute binary %s"), -- 1.7.4

2 1

[libvirt] [PATCH] qemu: Fallback to HMP when cpu_set QMP command is not found
by Wen Congyang 16 Mar '11

16 Mar '11

--- src/qemu/qemu_monitor_json.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index a839ffe..13d12c8 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1450,7 +1450,14 @@ int qemuMonitorJSONSetCPU(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + goto cleanup; + + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + VIR_DEBUG0("cpu_set command not found, trying HMP"); + ret = qemuMonitorTextSetCPU(mon, cpu, online); + goto cleanup; + } if (ret == 0) { /* XXX See if CPU soft-failed due to lack of ACPI */ @@ -1468,10 +1475,7 @@ int qemuMonitorJSONSetCPU(qemuMonitorPtr mon, ret = 1; } -#if 0 cleanup: -#endif - virJSONValueFree(cmd); virJSONValueFree(reply); return ret; -- 1.7.1

2 1

[libvirt] [PATCH] qemu: Check the unsigned integer overflow
by Osier Yang 16 Mar '11

16 Mar '11

As perhaps other hypervisor drivers use different capacity units, do the checking in qemu driver instead of in conf/domain_conf.c. --- src/qemu/qemu_command.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 198a4e2..42be6ee 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1933,6 +1933,13 @@ qemuBuildVideoDevStr(virDomainVideoDefPtr video, virBufferVSprintf(&buf, ",id=%s", video->info.alias); if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { + if (video->vram > (UINT_MAX / 1024)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("value for 'vram' must be less than '%u'"), + UINT_MAX / 1024); + goto error; + } + /* QEMU accepts bytes for vram_size. */ virBufferVSprintf(&buf, ",vram_size=%u", video->vram * 1024); } -- 1.7.4

3 5

[libvirt] [PATCH] Add compat function for geteuid()
by Daniel P. Berrange 16 Mar '11

16 Mar '11

* configure.ac: Check for geteuid() * src/util/util.h: Compat for geteuid() --- configure.ac | 2 +- src/util/util.h | 4 ++++ 2 files changed, 5 insertions(+), 1 deletions(-) diff --git a/configure.ac b/configure.ac index a58ee4e..e2b2b24 100644 --- a/configure.ac +++ b/configure.ac @@ -120,7 +120,7 @@ AC_MSG_RESULT([$have_cpuid]) dnl Availability of various common functions (non-fatal if missing), dnl and various less common threadsafe functions AC_CHECK_FUNCS_ONCE([cfmakeraw regexec sched_getaffinity getuid getgid \ - initgroups posix_fallocate mmap kill \ + geteuid initgroups posix_fallocate mmap kill \ getmntent_r getgrnam_r getpwuid_r]) dnl Availability of pthread functions (if missing, win32 threading is diff --git a/src/util/util.h b/src/util/util.h index 31c3a33..276e259 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -269,6 +269,10 @@ const char *virEnumToString(const char *const*types, static inline int getuid (void) { return 0; } # endif +# ifndef HAVE_GETUID +static inline int geteuid (void) { return 0; } +# endif + # ifndef HAVE_GETGID static inline int getgid (void) { return 0; } # endif -- 1.7.4

3 3

[libvirt] [PATCH] Fix misc bugs in virCommandPtr
by Daniel P. Berrange 16 Mar '11

16 Mar '11

The virCommandNewArgs() method would free the virCommandPtr if it failed to add the args. This meant errors reported in virCommandAddArgSet() were lost. Simply removing the check for errors from the constructor means they can be reported correctly later The virCommandAddEnvPassCommon() method failed to check for errors before reallocating the cmd->env array, causing a potential SEGV if cmd was NULL The virCommandAddArgSet() method needs to validate that at least 1 element in 'val's parameter is non-NULL, otherwise code like cmd = virCommandNew(binary) virCommandAddAtg(cmd, "foo") Would end up trying todo execve("foo"), if binary was NULL. --- src/util/command.c | 13 ++++++++----- 1 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index ff2bd46..22f350b 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -109,11 +109,6 @@ virCommandNewArgs(const char *const*args) virCommandAddArgSet(cmd, args); - if (cmd->has_error) { - virCommandFree(cmd); - return NULL; - } - return cmd; } @@ -362,6 +357,9 @@ virCommandAddEnvPass(virCommandPtr cmd, const char *name) void virCommandAddEnvPassCommon(virCommandPtr cmd) { + if (!cmd || cmd->has_error) + return; + /* Attempt to Pre-allocate; allocation failure will be detected * later during virCommandAdd*. */ ignore_value(VIR_RESIZE_N(cmd->env, cmd->maxenv, cmd->nenv, 9)); @@ -478,6 +476,11 @@ virCommandAddArgSet(virCommandPtr cmd, const char *const*vals) if (!cmd || cmd->has_error) return; + if (vals[0] == NULL) { + cmd->has_error = EINVAL; + return; + } + while (vals[narg] != NULL) narg++; -- 1.7.4

3 3

[libvirt] [PATCH] Change message for VIR_FROM_RPC error domain
by Daniel P. Berrange 15 Mar '11

15 Mar '11

The VIR_FROM_RPC error domain is used generically for any RPC problem, not simply XML-RPC problems. * src/util/virterror.c: s/XML-RPC/RPC/ --- src/util/virterror.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/virterror.c b/src/util/virterror.c index dc00957..7c07c69 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -105,7 +105,7 @@ static const char *virErrorDomainName(virErrorDomain domain) { dom = "Domain "; break; case VIR_FROM_RPC: - dom = "XML-RPC "; + dom = "RPC "; break; case VIR_FROM_QEMU: dom = "QEMU "; -- 1.7.4

2 1

[libvirt] [PATCH] Add virSetBlocking() to allow O_NONBLOCK to be toggle on or off
by Daniel P. Berrange 15 Mar '11

15 Mar '11

The virSetNonBlock() API only allows enabling non-blocking operations. It doesn't allow turning blocking back on. Add a new API to allow arbitrary toggling. * src/libvirt_private.syms, src/util/util.h src/util/util.c: Add virSetBlocking --- src/libvirt_private.syms | 1 + src/util/util.c | 13 ++++++++++--- src/util/util.h | 1 + 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 09eb03a..c88d934 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -910,6 +910,7 @@ virRandom; virRandomInitialize; virRun; virRunWithHook; +virSetBlocking; virSetCloseExec; virSetNonBlock; virSetUIDGID; diff --git a/src/util/util.c b/src/util/util.c index f41e117..e573f4a 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -244,16 +244,19 @@ virArgvToString(const char *const *argv) return ret; } -int virSetNonBlock(int fd) { +int virSetBlocking(int fd, bool blocking) { #ifndef WIN32 int flags; if ((flags = fcntl(fd, F_GETFL)) < 0) return -1; - flags |= O_NONBLOCK; + if (blocking) + flags &= ~O_NONBLOCK; + else + flags |= O_NONBLOCK; if ((fcntl(fd, F_SETFL, flags)) < 0) return -1; #else - unsigned long flag = 1; + unsigned long flag = blocking ? 0 : 1; /* This is actually Gnulib's replacement rpl_ioctl function. * We can't call ioctlsocket directly in any case. @@ -264,6 +267,10 @@ int virSetNonBlock(int fd) { return 0; } +int virSetNonBlock(int fd) { + return virSetBlocking(fd, false); +} + #ifndef WIN32 diff --git a/src/util/util.h b/src/util/util.h index 5f6473c..31c3a33 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -49,6 +49,7 @@ enum { VIR_EXEC_CLEAR_CAPS = (1 << 2), }; +int virSetBlocking(int fd, bool blocking) ATTRIBUTE_RETURN_CHECK; int virSetNonBlock(int fd) ATTRIBUTE_RETURN_CHECK; int virSetCloseExec(int fd) ATTRIBUTE_RETURN_CHECK; -- 1.7.4

2 1

[libvirt] [PATCH 0/4] white space handling in libvirt-guest.init script
by Philipp Hahn 15 Mar '11

15 Mar '11

libvirt-guest doesn't correctly handle the case, that the domain name contains blanks, when it parses the output of "virsh dominfo | grep Name". While investigating this this bug, I took the opportunity to also fix several other potential quoting problems in libvirt-guest.init. Those further changes might be controversial, since most variables will never contain characters from IFS, but since I encountered several problems with scripts not quoting variables in the past, I personally tend to quote almost every variable which must contains exactly one single value. The last patch is not for application, but since I had to do it anyway, I'll include it for others for inspiration. So #1 is the most important, #2 and #3 depends on your preferred style, and #4 is just FYI only. Philipp Hahn (4): libvirt-guest.init: handle domain name with spaces libvirt-guest.init: quoting variables libvirt-guest.init: declare variables as local libvirt-guest.init: Use lsb-init functions tools/libvirt-guests.init.sh | 145 ++++++++++++++++++++++-------------------- 1 files changed, 77 insertions(+), 68 deletions(-) Sincerely Philipp Hahn -- Philipp Hahn Open Source Software Engineer hahn(a)univention.de Univention GmbH Linux for Your Business fon: +49 421 22 232- 0 Mary-Somerville-Str.1 28359 Bremen fax: +49 421 22 232-99 http://www.univention.de/

4 16

[libvirt] [PATCH] docs: update windows page for initial libvirt 0.8.8 installer
by Justin Clift 15 Mar '11

15 Mar '11

--- docs/windows.html.in | 9 +-------- 1 files changed, 1 insertions(+), 8 deletions(-) diff --git a/docs/windows.html.in b/docs/windows.html.in index 4850a54..4011cc3 100644 --- a/docs/windows.html.in +++ b/docs/windows.html.in @@ -18,7 +18,7 @@ version is available here: - <a href="http://libvirt.org/sources/win32_experimental/Libvirt-0.8.7-2.exe">http://libvirt.org/sources/win32_experimental/Libvirt-0.8.7-2.exe</a> + <a href="http://libvirt.org/sources/win32_experimental/Libvirt-0.8.8-0.exe">http://libvirt.org/sources/win32_experimental/Libvirt-0.8.8-0.exe</a> It is not production ready. @@ -70,13 +70,6 @@ </li> - <li> - Start menu shortcuts aren't being removed at uninstall time - - Not sure why yet. Needs to be investigated. - - </li> - </ul> <h2><a name="conntypes">Connection types</h2> -- 1.7.4.1

1 1