- Devel - libvirt List Archives

[libvirt] [PATCH] build: tolerate unlimited group size
by Eric Blake 17 May '11

17 May '11

POSIX allows sysconf(_SC_GETPW_R_SIZE_MAX) to return -1 if there is no fixed limit, and requires ERANGE errors to track real size. http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid_r.html Also, on error for get*_r functions, errno is undefined, and the real error was the return value. * src/util/util.c (virGetUserEnt, virGetUserID, virGetGroupID) (virSetUIDGID): Cope with sysconf failure or too small buffer. Reported by Matthias Bolte. --- This pretty much copies the example implementation straight from POSIX for getting a hint for initial size, then iterating until ERANGE no longer occurs. src/util/util.c | 74 +++++++++++++++++++++++++++++++++++++----------------- 1 files changed, 51 insertions(+), 23 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index f169e54..240f776 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -603,7 +603,7 @@ virExecWithHook(const char *const*argv, goto fork_error; } - openmax = sysconf (_SC_OPEN_MAX); + openmax = sysconf(_SC_OPEN_MAX); for (i = 3; i < openmax; i++) if (i != infd && i != null && @@ -2743,11 +2743,11 @@ static char *virGetUserEnt(uid_t uid, struct passwd *pw = NULL; long val = sysconf(_SC_GETPW_R_SIZE_MAX); size_t strbuflen = val; + int rc; - if (val < 0) { - virReportSystemError(errno, "%s", _("sysconf failed")); - return NULL; - } + /* sysconf is a hint; if it fails, fall back to a reasonable size */ + if (val < 0) + strbuflen = 1024; if (VIR_ALLOC_N(strbuf, strbuflen) < 0) { virReportOOMError(); @@ -2761,8 +2761,15 @@ static char *virGetUserEnt(uid_t uid, * 0 or ENOENT or ESRCH or EBADF or EPERM or ... * The given name or uid was not found. */ - if (getpwuid_r(uid, &pwbuf, strbuf, strbuflen, &pw) != 0 || pw == NULL) { - virReportSystemError(errno, + while ((rc = getpwuid_r(uid, &pwbuf, strbuf, strbuflen, &pw)) == ERANGE) { + if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) { + virReportOOMError(); + VIR_FREE(strbuf); + return NULL; + } + } + if (rc != 0 || pw == NULL) { + virReportSystemError(rc, _("Failed to find user record for uid '%u'"), (unsigned int) uid); VIR_FREE(strbuf); @@ -2800,11 +2807,11 @@ int virGetUserID(const char *name, struct passwd *pw = NULL; long val = sysconf(_SC_GETPW_R_SIZE_MAX); size_t strbuflen = val; + int rc; - if (val < 0) { - virReportSystemError(errno, "%s", _("sysconf failed")); - return -1; - } + /* sysconf is a hint; if it fails, fall back to a reasonable size */ + if (val < 0) + strbuflen = 1024; if (VIR_ALLOC_N(strbuf, strbuflen) < 0) { virReportOOMError(); @@ -2818,8 +2825,15 @@ int virGetUserID(const char *name, * 0 or ENOENT or ESRCH or EBADF or EPERM or ... * The given name or uid was not found. */ - if (getpwnam_r(name, &pwbuf, strbuf, strbuflen, &pw) != 0 || pw == NULL) { - virReportSystemError(errno, + while ((rc = getpwnam_r(name, &pwbuf, strbuf, strbuflen, &pw)) == ERANGE) { + if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) { + virReportOOMError(); + VIR_FREE(strbuf); + return -1; + } + } + if (rc != 0 || pw == NULL) { + virReportSystemError(rc, _("Failed to find user record for name '%s'"), name); VIR_FREE(strbuf); @@ -2842,11 +2856,11 @@ int virGetGroupID(const char *name, struct group *gr = NULL; long val = sysconf(_SC_GETGR_R_SIZE_MAX); size_t strbuflen = val; + int rc; - if (val < 0) { - virReportSystemError(errno, "%s", _("sysconf failed")); - return -1; - } + /* sysconf is a hint; if it fails, fall back to a reasonable size */ + if (val < 0) + strbuflen = 1024; if (VIR_ALLOC_N(strbuf, strbuflen) < 0) { virReportOOMError(); @@ -2860,8 +2874,15 @@ int virGetGroupID(const char *name, * 0 or ENOENT or ESRCH or EBADF or EPERM or ... * The given name or uid was not found. */ - if (getgrnam_r(name, &grbuf, strbuf, strbuflen, &gr) != 0 || gr == NULL) { - virReportSystemError(errno, + while ((rc = getgrnam_r(name, &grbuf, strbuf, strbuflen, &gr)) == ERANGE) { + if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) { + virReportOOMError(); + VIR_FREE(strbuf); + return -1; + } + } + if (rc != 0 || gr == NULL) { + virReportSystemError(rc, _("Failed to find group record for name '%s'"), name); VIR_FREE(strbuf); @@ -2897,6 +2918,7 @@ virSetUIDGID(uid_t uid, gid_t gid) struct passwd pwd, *pwd_result; char *buf = NULL; size_t bufsize; + int rc; bufsize = sysconf(_SC_GETPW_R_SIZE_MAX); if (bufsize == -1) @@ -2906,10 +2928,16 @@ virSetUIDGID(uid_t uid, gid_t gid) virReportOOMError(); return -1; } - getpwuid_r(uid, &pwd, buf, bufsize, &pwd_result); - if (!pwd_result) { - virReportSystemError(errno, - _("cannot getpwuid_r(%d)"), + while ((rc = getpwuid_r(uid, &pwd, buf, bufsize, + &pwd_result)) == ERANGE) { + if (VIR_RESIZE_N(buf, bufsize, bufsize, bufsize) < 0) { + virReportOOMError(); + VIR_FREE(buf); + return -1; + } + } + if (rc || !pwd_result) { + virReportSystemError(rc, _("cannot getpwuid_r(%d)"), (unsigned int) uid); VIR_FREE(buf); return -1; -- 1.7.4.4

2 2

[libvirt] 'attach-interface' failure and RHEL6 qemu-kvm
by Neil Wilson 17 May '11

17 May '11

I've run up against a small issue with the 'attach-interface' facility in later versions of libvirt. It doesn't appear to work with the vanilla qemu-kvm package in RHEL6 (which reports as version 0.12.1). For both attach-interface and attach-device I'm getting the same error. virsh attach-interface srv-vvh2o network vir0001 error: Failed to attach interface error: internal error unable to add host net: unknown command: 'host_net_add' virsh # attach-device srv-vvh2o fred.xml error: Failed to attach device from fred.xml error: internal error unable to add host net: unknown command: 'host_net_add' Looking at qemu monitor the 'qemu-kvm' is indeed short of the 'host_net_add' command and it all seems to be down to an entry in 'qemu_capabilities.c' which switches off 'netdev' for qemu versions less than 13. Is there a reason that the RHEL6 version - which seems to have functioning -netdev parameters and 'netdev_add' qemu commands - has been excluded? If its a RHEL6 special patched version is there a way to special case detect it in the code? Thanks Neil

1 0

[libvirt] [PATCH 00/12] esx: Generator improvments and general cleanups
by Matthias Bolte 17 May '11

17 May '11

As we're currently in feature freeze, this series is meant for 0.9.2. It includes mostly generator improvments and general cleanups. 10/12 is a race condition fix, but it's not critical because it's not that simple to trigger. Matthias

2 40

[libvirt] [PATCH v2] virsh: Correctly initialize libvirt
by Jiri Denemark 17 May '11

17 May '11

virsh didn't call virInitialize(), which (among other things) initializes virLastErr thread local variable. As a result of that, virsh could just segfault in virEventRegisterDefaultImpl() since that is the first call that touches (resets) virLastErr. I have no idea what lucky coincidence made this bug visible but I was able to reproduce it in 100% cases but only in one specific environment which included building in sandbox. --- src/libvirt.c | 3 +++ tools/virsh.c | 13 +++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 5a5439d..787908e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -369,6 +369,9 @@ static struct gcry_thread_cbs virTLSThreadImpl = { * in multithreaded applications to avoid potential race when initializing * the library. * + * Calling virInitialize is mandatory, unless your first API call is one of + * virConnectOpen*. + * * Returns 0 in case of success, -1 in case of error */ int diff --git a/tools/virsh.c b/tools/virsh.c index e35637d..b469e7a 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12983,6 +12983,10 @@ main(int argc, char **argv) char *defaultConn; bool ret = true; + memset(ctl, 0, sizeof(vshControl)); + ctl->imode = true; /* default is interactive mode */ + ctl->log_fd = -1; /* Initialize log file descriptor */ + if (!setlocale(LC_ALL, "")) { perror("setlocale"); /* failure to setup locale is not fatal */ @@ -12996,15 +13000,16 @@ main(int argc, char **argv) return EXIT_FAILURE; } + if (virInitialize() < 0) { + vshError(ctl, "%s", _("Failed to initialize libvirt")); + return EXIT_FAILURE; + } + if (!(progname = strrchr(argv[0], '/'))) progname = argv[0]; else progname++; - memset(ctl, 0, sizeof(vshControl)); - ctl->imode = true; /* default is interactive mode */ - ctl->log_fd = -1; /* Initialize log file descriptor */ - if ((defaultConn = getenv("VIRSH_DEFAULT_CONNECT_URI"))) { ctl->name = vshStrdup(ctl, defaultConn); } -- 1.7.5.rc3

2 2

[libvirt] [PATCH] mark mig->flags as QEMU_MIGRATION_COOKIE_GRAPHICS only when mig->graphics is not NULL
by Wen Congyang 17 May '11

17 May '11

Steps to reproduce this problem: 1. remove all <graphics ...> from xml. 2. start the domain 3. migrate the domain to another host, and libvirtd on the target host will crash. The reason of this bug is that we mark mig->flags as QEMU_MIGRATION_COOKIE_GRAPHICS even if we have no graphics. We will use mig->graphics when mig->flgas is QEMU_MIGRATION_COOKIE_GRAPHICS in the function qemuMigrationCookieXMLFormat() --- src/qemu/qemu_migration.c | 12 +++++++----- 1 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index cb408ac..3def14b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -252,11 +252,13 @@ qemuMigrationCookieAddGraphics(qemuMigrationCookiePtr mig, if (dom->def->ngraphics == 1 && (dom->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC || - dom->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) && - !(mig->graphics = qemuMigrationCookieGraphicsAlloc(driver, dom->def->graphics[0]))) - return -1; - - mig->flags |= QEMU_MIGRATION_COOKIE_GRAPHICS; + dom->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE)) { + mig->graphics = qemuMigrationCookieGraphicsAlloc(driver, + dom->def->graphics[0]); + if (mig->graphics == NULL) + return -1; + mig->flags |= QEMU_MIGRATION_COOKIE_GRAPHICS; + } return 0; } -- 1.7.1

1 0

[libvirt] [PATCHv4 0/6] Add virNodeGetCPUTimeParameters() API
by Minoru Usui 17 May '11

17 May '11

Hi, This is v4 of virNodeGetCpuTime() API. (This time, I rename it to virNodeGetCPUTimeParameters()) It returns cpu utilization or cumulative cpu time of the node from /proc/stat since node boots up. This patch only supports linux host. Changes v3->v4 - Rebase this patch like virDomainGetMemoryParameters() from v2 patches. (drop v3 patches except virsh subcommand) - Rename API name to virNodeGetCPUTimeParameters() v2->v3 - Change user I/F. It is able to request what the user want by the @flags. - Minor change of virsh nodecputime I/F. v1->v2 - Change user I/F like virDomainGetMemoryStats() - It can return either cpu utilization or cumulative cpu time of the node depends on each driver. Minoru Usui (6): [v4] virNodeGetCPUTimeParameters: Expose new API [v4] virNodeGetCPUTimeParameters: Define internal driver API [v4] virNodeGetCPUTimeParameters: Implement public API [v4] virNodeGetCPUTimeParameters: Implement remote protocol [v4] virNodeGetCPUTimeParameters: Implement virsh support [v4] virNodeGetCPUTimeParameters: Implement linux support daemon/remote.c | 81 +++++++++++++++++++++++++++ include/libvirt/libvirt.h.in | 65 +++++++++++++++++++++ src/driver.h | 8 +++ src/esx/esx_driver.c | 1 + src/libvirt.c | 87 ++++++++++++++++++++++++++++ src/libvirt_private.syms | 1 + src/libvirt_public.syms | 1 + src/libxl/libxl_driver.c | 1 + src/lxc/lxc_driver.c | 1 + src/nodeinfo.c | 127 ++++++++++++++++++++++++++++++++++++++++++ src/nodeinfo.h | 5 +- src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 1 + src/qemu/qemu_driver.c | 1 + src/remote/remote_driver.c | 65 +++++++++++++++++++++ src/remote/remote_protocol.x | 21 +++++++- src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vbox/vbox_tmpl.c | 1 + src/vmware/vmware_driver.c | 1 + src/xen/xen_driver.c | 1 + src/xenapi/xenapi_driver.c | 1 + tools/virsh.c | 107 +++++++++++++++++++++++++++++++++++ tools/virsh.pod | 4 + 24 files changed, 582 insertions(+), 2 deletions(-) -- Minoru Usui <usui(a)mxm.nes.nec.co.jp>

2 10

[libvirt] [PATCH] virsh: Improve editing
by Michal Privoznik 17 May '11

17 May '11

When users (pool-/net-)edit and they make a mistake, temporary file and thus all changes are gone. Better way is to let them decide if they want to get back to edit and correct what's wrong. However, this is suitable only in interactive mode. --- tools/virsh.c | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 40 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index fbeb7c8..0dccc17 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -338,6 +338,7 @@ static const char *vshDomainStateToString(int state); static const char *vshDomainVcpuStateToString(int state); static bool vshConnectionUsability(vshControl *ctl, virConnectPtr conn); +static char * vshReadline (vshControl *ctl, const char *prompt); static char *editWriteToTempFile (vshControl *ctl, const char *doc); static int editFile (vshControl *ctl, const char *filename); static char *editReadBackFile (vshControl *ctl, const char *filename); @@ -10099,6 +10100,7 @@ cmdEdit (vshControl *ctl, const vshCmd *cmd) char *doc_edited = NULL; char *doc_reread = NULL; int flags = VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INACTIVE; + char *response; if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; @@ -10116,6 +10118,7 @@ cmdEdit (vshControl *ctl, const vshCmd *cmd) tmp = editWriteToTempFile (ctl, doc); if (!tmp) goto cleanup; +again: /* Start the editor. */ if (editFile (ctl, tmp) == -1) goto cleanup; @@ -10148,8 +10151,43 @@ cmdEdit (vshControl *ctl, const vshCmd *cmd) /* Everything checks out, so redefine the domain. */ virDomainFree (dom); dom = virDomainDefineXML (ctl->conn, doc_edited); - if (!dom) - goto cleanup; + if (!dom) { + if (ctl->imode) { + virshReportError(ctl); + response = vshReadline(ctl, "Do you want to edit again [Y/n] "); + while (true) { + char *c = response; + while (c && *c) { + *c = tolower(*c); + c++; + } + + if (STREQ(response, "") || STREQ(response, "y") || + STREQ(response, "yes")) { + dom = vshCommandOptDomain (ctl, cmd, NULL); + if (!dom) + goto cleanup; + goto again; + } else if (!response || STREQ(response, "n") || + STREQ(response, "no")) { + /* Ok, this is a little hack. Command did not succeed, + * but the error was already reported. So we do not + * want it to get reported twice */ + ret = true; + goto cleanup; + } else { + char *prompt; + if (virAsprintf(&prompt, _("Sorry, response '%s' not " + "understood. [Y/n] "), response) < 0) + goto cleanup; + response = vshReadline(ctl, prompt); + VIR_FREE(prompt); + } + } + } else { + goto cleanup; + } + } vshPrint (ctl, _("Domain %s XML configuration edited.\n"), virDomainGetName(dom)); -- 1.7.5.rc3

7 8

[libvirt] FreeBSD and sysconf(_SC_GETPW_R_SIZE_MAX)
by Matthias Bolte 16 May '11

16 May '11

On FreeBSD virsh fails to enter interactive mode because vshReadlineInit fails, because virGetUserDirectory fails, because virGetUserEnt fails, because sysconf(_SC_GETPW_R_SIZE_MAX) returns -1 and sets errno to EINVAL. Is this something that gnulib should/could deal with, Eric? Or should we work around it in libvrt and fallback to PATH_MAX when sysconf(_SC_GETPW_R_SIZE_MAX) fails? Matthias

4 4

[libvirt] [PATCH] Fix leak of mime type string in screenshot dispatcher
by Daniel P. Berrange 16 May '11

16 May '11

* daemon/remote.c: Free mime string --- daemon/remote.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index bb01c89..ea36bf5 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1746,6 +1746,7 @@ remoteDispatchDomainScreenshot(struct qemud_server *server ATTRIBUTE_UNUSED, cleanup: if (rv < 0) remoteDispatchError(rerr); + VIR_FREE(mime); if (dom) virDomainFree(dom); if (stream && rv != 0) { -- 1.7.4.4

2 1

[libvirt] [PATCH] Remove obsolete remoteDispatchOOMError method
by Daniel P. Berrange 16 May '11

16 May '11

No new code should be using remoteDispatchOOMError() * daemon/dispatch.c, daemon/dispatch.h: Remove remoteDispatchOOMError --- daemon/dispatch.c | 8 -------- daemon/dispatch.h | 1 - 2 files changed, 0 insertions(+), 9 deletions(-) diff --git a/daemon/dispatch.c b/daemon/dispatch.c index 7453451..010be1e 100644 --- a/daemon/dispatch.c +++ b/daemon/dispatch.c @@ -104,14 +104,6 @@ void remoteDispatchGenericError (remote_error *rerr) } -void remoteDispatchOOMError (remote_error *rerr) -{ - remoteDispatchStringError(rerr, - VIR_ERR_NO_MEMORY, - "out of memory"); -} - - void remoteDispatchError(remote_error *rerr) { virErrorPtr verr = virGetLastError(); diff --git a/daemon/dispatch.h b/daemon/dispatch.h index 79d35ac..f24f494 100644 --- a/daemon/dispatch.h +++ b/daemon/dispatch.h @@ -45,7 +45,6 @@ void remoteDispatchFormatError (remote_error *rerr, void remoteDispatchAuthError (remote_error *rerr); void remoteDispatchGenericError (remote_error *rerr); -void remoteDispatchOOMError (remote_error *rerr); void remoteDispatchError(remote_error *rerr); -- 1.7.4.4

2 1