February 2011 - Devel - libvirt List Archives

[libvirt] [PATCH] build: address clang reports about virCommand
by Eric Blake 15 Feb '11

15 Feb '11

clang had 5 reports against virCommand; three were false positives (a NULL deref in ProcessIO solved by sa_assert, and two uninitialized memory operations solved by adding an initializer), but two were real. * src/util/command.c (virCommandProcessIO): Fix real bug of possible NULL dereference. Teach clang that buf is never NULL. (virCommandRun): Teach clang that infd is only ever accessed when initialized. --- src/util/command.c | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index abd2dc4..0845db4 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -1,7 +1,7 @@ /* * command.c: Child command execution * - * Copyright (C) 2010 Red Hat, Inc. + * Copyright (C) 2010-2011 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -881,6 +881,8 @@ virCommandProcessIO(virCommandPtr cmd) buf = cmd->errbuf; len = &errlen; } + /* Silence a false positive from clang. */ + sa_assert(buf); done = read(fds[i].fd, data, sizeof(data)); if (done < 0) { @@ -930,9 +932,9 @@ virCommandProcessIO(virCommandPtr cmd) ret = 0; cleanup: - if (*cmd->outbuf) + if (cmd->outbuf && *cmd->outbuf) (*cmd->outbuf)[outlen] = '\0'; - if (*cmd->errbuf) + if (cmd->errbuf && *cmd->errbuf) (*cmd->errbuf)[errlen] = '\0'; return ret; } @@ -950,7 +952,7 @@ virCommandRun(virCommandPtr cmd, int *exitstatus) int ret = 0; char *outbuf = NULL; char *errbuf = NULL; - int infd[2]; + int infd[2] = { -1, -1 }; struct stat st; bool string_io; bool async_io = false; -- 1.7.4

2 2

[libvirt] [PATCH] qemu: don't mask real error with oom report
by Eric Blake 15 Feb '11

15 Feb '11

* src/qemu/qemu_command.c (qemuBuildCommandLine): Don't report oom after qemuBuildControllerDevStr, which reported its own errors. --- src/qemu/qemu_command.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3ba0950..63d5edc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3102,7 +3102,7 @@ qemuBuildCommandLine(virConnectPtr conn, char *devstr; if (!(devstr = qemuBuildControllerDevStr(def->controllers[i], qemuCmdFlags))) - goto no_memory; + goto error; virCommandAddArg(cmd, devstr); VIR_FREE(devstr); -- 1.7.4

2 2

[libvirt] [PATCH] virsh: Fix improper freecell --cellno command behavior
by Michal Privoznik 15 Feb '11

15 Feb '11

Non-digit characters were accepted in --cellno resulting in unwanted behavior: 'virsh freecell --cellno foo' wasn't rejected. --- This fix addresses bug 639587 tools/virsh.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index c2d165d..765566d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2283,7 +2283,7 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd) { int func_ret = FALSE; int ret; - int cell, cell_given; + int cell, cell_given, cellStr_given; unsigned long long memory; unsigned long long *nodes = NULL; int all_given; @@ -2295,6 +2295,13 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd) cell = vshCommandOptInt(cmd, "cellno", &cell_given); all_given = vshCommandOptBool(cmd, "all"); + vshCommandOptString(cmd, "cellno", &cellStr_given); + + if (cellStr_given && !cell_given) { + vshError(ctl, "%s", _("cell number must not contain any " + "non-digit characters.")); + goto cleanup; + } if (all_given && cell_given) { vshError(ctl, "%s", _("--cellno and --all are mutually exclusive. " -- 1.7.3.5

2 1

[libvirt] [PATCH] qemu: avoid NULL derefs
by Eric Blake 15 Feb '11

15 Feb '11

The processWatchdogEvent fix is real, although it can only trigger on OOM, since bad things happen if doCoreDump is called with a NULL pathname argument. The other fixes silence clang, but aren't a real bug because virReportErrorHelper tolerates a NULL format string even though *printf does not. * src/qemu/qemu_driver.c (processWatchdogEvent): Exit on OOM. (qemuDomainIsActive, qemuDomainIsPersistent, qemuDomainIsUpdated): Provide valid message. --- Another valid clang finding. Maybe we whould mark virReportErrorHelper as ATTRIBUTE_NONNULL for the format string argument, and require all clients to pass in a valid error string (after all, that's what the printf attritube already assumes, and passing NULL just means that we aren't giving back any useful error information to the user); but that can be a separate cleanup. src/qemu/qemu_driver.c | 23 +++++++++++++++++------ 1 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c581cfe..82a2210 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3489,7 +3489,10 @@ static int qemuDomainIsActive(virDomainPtr dom) obj = virDomainFindByUUID(&driver->domains, dom->uuid); qemuDriverUnlock(driver); if (!obj) { - qemuReportError(VIR_ERR_NO_DOMAIN, NULL); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } ret = virDomainObjIsActive(obj); @@ -3510,7 +3513,10 @@ static int qemuDomainIsPersistent(virDomainPtr dom) obj = virDomainFindByUUID(&driver->domains, dom->uuid); qemuDriverUnlock(driver); if (!obj) { - qemuReportError(VIR_ERR_NO_DOMAIN, NULL); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } ret = obj->persistent; @@ -3531,7 +3537,10 @@ static int qemuDomainIsUpdated(virDomainPtr dom) obj = virDomainFindByUUID(&driver->domains, dom->uuid); qemuDriverUnlock(driver); if (!obj) { - qemuReportError(VIR_ERR_NO_DOMAIN, NULL); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } ret = obj->updated; @@ -4981,12 +4990,14 @@ static void processWatchdogEvent(void *data, void *opaque) case VIR_DOMAIN_WATCHDOG_ACTION_DUMP: { char *dumpfile; - int i; - i = virAsprintf(&dumpfile, "%s/%s-%u", + if (virAsprintf(&dumpfile, "%s/%s-%u", driver->autoDumpPath, wdEvent->vm->def->name, - (unsigned int)time(NULL)); + (unsigned int)time(NULL)) < 0) { + virReportOOMError(); + break; + } qemuDriverLock(driver); virDomainObjLock(wdEvent->vm); -- 1.7.4

2 2

[libvirt] [PATCH] virDomainMemoryStats: avoid null dereference
by Eric Blake 15 Feb '11

15 Feb '11

* src/libvirt.c (virDomainMemoryStats): Check domain before flags. --- Another valid bug found by clang. User's generally should't call virDomainMemoryStats(NULL,...), but we don't forbid it elsewhere, and doing so is not supposed to crash libvirt. All other flags checks in this file occur after validating the object first. src/libvirt.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 479a9b5..f65cc24 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -4411,12 +4411,6 @@ int virDomainMemoryStats (virDomainPtr dom, virDomainMemoryStatPtr stats, VIR_DOMAIN_DEBUG(dom, "stats=%p, nr_stats=%u", stats, nr_stats); - if (flags != 0) { - virLibDomainError(VIR_ERR_INVALID_ARG, - _("flags must be zero")); - goto error; - } - virResetLastError(); if (!VIR_IS_CONNECTED_DOMAIN (dom)) { @@ -4424,6 +4418,12 @@ int virDomainMemoryStats (virDomainPtr dom, virDomainMemoryStatPtr stats, virDispatchError(NULL); return -1; } + if (flags != 0) { + virLibDomainError(VIR_ERR_INVALID_ARG, + _("flags must be zero")); + goto error; + } + if (!stats || nr_stats == 0) return 0; -- 1.7.4

2 2

[libvirt] libvirt 0.8.7 tests failure on arm and ppc
by Serge E. Hallyn 15 Feb '11

15 Feb '11

Hi, as per the message after the tests fail, I'm reporting this on the list. Hopefully someone has seen this before. I've not yet tried this with the latest git snapshot. With 0.8.7, I get: TEST: qemuxml2argvtest ........................................ 40 ........................................ 80 .............................!.!!!!! 116 FAIL -serge

4 9

[libvirt] [PATCH] macvtap: Work-around failing nl_connect calls (weird problem)
by Stefan Berger 15 Feb '11

15 Feb '11

When trying to start / stop a domain with macvtap device (direct type of interface) having a device description like this one here <interface type='direct'> <source dev='static' mode='vepa'/> </interface> then I see netlink related errors when a 'virsh edit' session is happening at the same time. So, to reproduce this error you should try the following (on a kernel supporting macvtap): virsh edit <macvtap domain> -> do not terminate the edit sessions virsh start <macvtap domain> -> works virsh destroy <macvtap domain> -> leaves a macvtap device due to nl_connect failing virsh start <macvtap domain> -> does not start anymore That should make it fail. The work-around basically keeps on allocating new netlink library handles until the nl_connect() succeeds. New netlink library handles cause the next available port (intern to libnl; nl_pid) to be allocated. For every ongoing virsh edit session, one new handle seems to be required. So for 2 virsh edit session, the 3rd one (usually) works. I do not know what in the system is causing this, but my guess it that 'something' is blocking the same port (nl_pid) -- it may not be in libvirt but in a dependent library that's not using libnl (?). Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com> Index: libvirt-acl/src/util/macvtap.c =================================================================== --- libvirt-acl.orig/src/util/macvtap.c +++ libvirt-acl/src/util/macvtap.c @@ -120,13 +120,43 @@ int nlComm(struct nl_msg *nl_msg, fd_set readfds; int fd; int n; - struct nl_handle *nlhandle = nl_handle_alloc(); + struct nl_handle **nlhandles = NULL; struct nlmsghdr *nlmsg = nlmsg_hdr(nl_msg); + unsigned int idx = 0, num_elms = 1, i; - if (!nlhandle) - return -1; +realloc: + if (VIR_REALLOC_N(nlhandles, num_elms * sizeof(struct nl_handle *)) < 0) { + virReportOOMError(); + rc = -1; + goto err_exit; + } + + for (i = idx; i < num_elms ; i++) + nlhandles[i] = NULL; + +next_handle: + nlhandles[idx] = nl_handle_alloc(); - if (nl_connect(nlhandle, NETLINK_ROUTE) < 0) { + if (nlhandles[idx] == NULL) { + virReportOOMError(); + rc = -1; + goto err_exit; + } + + if (nl_connect(nlhandles[idx], NETLINK_ROUTE) < 0) { + VIR_DEBUG0("Could not create netlink socket - trying a new one\n"); + /* get a new handle and keep the ones we have */ + idx++; + if (idx < num_elms) + goto next_handle; + /* need to reallocate */ + num_elms += 10; + if (idx < 500) + goto realloc; + + macvtapError(VIR_ERR_INTERNAL_ERROR, "%s [%s]", + _("Could not create netlink socket"), + nl_geterror()); rc = -1; goto err_exit; } @@ -135,15 +165,16 @@ int nlComm(struct nl_msg *nl_msg, nlmsg->nlmsg_pid = getpid(); - nbytes = nl_send_auto_complete(nlhandle, nl_msg); + nbytes = nl_send_auto_complete(nlhandles[idx], nl_msg); if (nbytes < 0) { virReportSystemError(errno, - "%s", _("cannot send to netlink socket")); + "%s [%s]", _("cannot send to netlink socket"), + nl_geterror()); rc = -1; goto err_exit; } - fd = nl_socket_get_fd(nlhandle); + fd = nl_socket_get_fd(nlhandles[idx]); FD_ZERO(&readfds); FD_SET(fd, &readfds); @@ -160,7 +191,7 @@ int nlComm(struct nl_msg *nl_msg, goto err_exit; } - *respbuflen = nl_recv(nlhandle, &nladdr, respbuf, NULL); + *respbuflen = nl_recv(nlhandles[idx], &nladdr, respbuf, NULL); if (*respbuflen <= 0) rc = -1; @@ -171,7 +202,12 @@ err_exit: *respbuflen = 0; } - nl_handle_destroy(nlhandle); + if (nlhandles) + for (idx = 0; idx < num_elms; idx++) + nl_handle_destroy(nlhandles[idx]); + + VIR_FREE(nlhandles); + return rc; }

2 5

[libvirt] [PATCH] docs: added link for nimbus to apps page
by Justin Clift 15 Feb '11

15 Feb '11

--- docs/apps.html.in | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/docs/apps.html.in b/docs/apps.html.in index d8bc265..8b949f3 100644 --- a/docs/apps.html.in +++ b/docs/apps.html.in @@ -197,6 +197,18 @@ </dd> </dl> + <h2><a name="iaas">Infrastructure as a Service (IaaS)</a></h2> + + <dl> + <dt><a href="http://www.nimbusproject.org">Nimbus</a></dt> + <dd> + Nimbus is an open-source toolkit focused on providing + Infrastructure-as-a-Service (IaaS) capabilities to the scientific + community. It uses libvirt for communication with all KVM and Xen + virtual machines. + </dd> + </dl> + <h2><a name="libraries">Libraries</a></h2> <dl> -- 1.7.4.1

4 6

[libvirt] Mails rejected by mailist?
by Lyre 15 Feb '11

15 Feb '11

Hi all: I've tried to send a patch for libvirt-php using git send-email yesterday. My user.email is configured as a gmail address, that is this one. But git sent patches from local postfix. git CC the patchs to my gmail and I can revice it, but not listed on the maillist. Is it rejected by the maillist? And what should I do to send the patches?

4 5

[libvirt] [PATCH] Output commandline on status != 0 in virCommandWait
by Matthias Bolte 15 Feb '11

15 Feb '11

I pushed this one as suggested by Daniel in https://www.redhat.com/archives/libvir-list/2011-February/msg00519.html Matthias

1 0