- Devel - libvirt List Archives

[libvirt] [PATCH] Enhance qemuParseCommandLineKeywords
by Jiri Denemark 19 Jan '10

19 Jan '10

Current version expects name=value,... list and when an incorrect string such as "a,b,c=d" would be parsed as "a,b,c" keyword with "d" value without reporting any error, which is probably not the expected behavior. This patch adds an extra argument called allowEmptyValue, which if non-zero will permit keywords with no value; "a,b=c,,d=" will be parsed as follows: keyword value "a" NULL "b" "c" "" NULL "d" "" In case allowEmptyValue is zero, the string is required to contain name=value pairs only; retvalues is guaranteed to contain non-NULL pointers. Now, "a,b,c=d" will result in an error. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com> --- src/qemu/qemu_conf.c | 51 ++++++++++++++++++++++++++++++------------------- 1 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index d3da776..9f35217 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -3066,47 +3066,58 @@ static const char *qemuFindEnv(const char **progenv, /* * Takes a string containing a set of key=value,key=value,key... * parameters and splits them up, returning two arrays with - * the individual keys and values + * the individual keys and values. If allowEmptyValue is nonzero, + * the "=value" part is optional and if a key with no value is found, + * NULL is be placed into corresponding place in retvalues. */ static int qemuParseCommandLineKeywords(virConnectPtr conn, const char *str, char ***retkeywords, - char ***retvalues) + char ***retvalues, + int allowEmptyValue) { int keywordCount = 0; int keywordAlloc = 0; char **keywords = NULL; char **values = NULL; const char *start = str; + const char *end; int i; *retkeywords = NULL; *retvalues = NULL; + end = start + strlen(str); while (start) { const char *separator; const char *endmark; char *keyword; - char *value; + char *value = NULL; - if (!(separator = strchr(start, '='))) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("malformed keyword arguments in '%s'"), str); - goto error; + if (!(endmark = strchr(start, ','))) + endmark = end; + if (!(separator = strchr(start, '='))) + separator = end; + + if (separator >= endmark) { + if (!allowEmptyValue) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("malformed keyword arguments in '%s'"), str); + goto error; + } + separator = endmark; } + if (!(keyword = strndup(start, separator - start))) goto no_memory; - separator++; - endmark = strchr(separator, ','); - - value = endmark ? - strndup(separator, endmark - separator) : - strdup(separator); - if (!value) { - VIR_FREE(keyword); - goto no_memory; + if (separator < endmark) { + separator++; + if (!(value = strndup(separator, endmark - separator))) { + VIR_FREE(keyword); + goto no_memory; + } } if (keywordAlloc == keywordCount) { @@ -3123,7 +3134,7 @@ qemuParseCommandLineKeywords(virConnectPtr conn, values[keywordCount] = value; keywordCount++; - start = endmark ? endmark + 1 : NULL; + start = endmark < end ? endmark + 1 : NULL; } *retkeywords = keywords; @@ -3163,7 +3174,7 @@ qemuParseCommandLineDisk(virConnectPtr conn, if ((nkeywords = qemuParseCommandLineKeywords(conn, val, &keywords, - &values)) < 0) + &values, 0)) < 0) return NULL; if (VIR_ALLOC(def) < 0) { @@ -3347,7 +3358,7 @@ qemuParseCommandLineNet(virConnectPtr conn, if ((nkeywords = qemuParseCommandLineKeywords(conn, tmp+1, &keywords, - &values)) < 0) + &values, 0)) < 0) return NULL; } else { nkeywords = 0; @@ -3420,7 +3431,7 @@ qemuParseCommandLineNet(virConnectPtr conn, if ((nkeywords = qemuParseCommandLineKeywords(conn, nic + strlen("nic,"), &keywords, - &values)) < 0) { + &values, 0)) < 0) { virDomainNetDefFree(def); def = NULL; goto cleanup; -- 1.6.6

3 2

[libvirt] [PATCH] storage_conf: plug a leak on OOM error path
by Jim Meyering 18 Jan '10

18 Jan '10

Obvious, for a change... >From 964f24e0724eda6bf12d115318ea9576bbd91f10 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 18 Jan 2010 18:40:13 +0100 Subject: [PATCH] storage_conf: plug a leak on OOM error path * src/conf/storage_conf.c (virStoragePoolSourceListNewSource): Free just-allocated "source" upon VIR_REALLOC_N failure. --- src/conf/storage_conf.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 0aefa06..ea49531 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1,7 +1,7 @@ /* * storage_conf.c: config handling for storage driver * - * Copyright (C) 2006-2009 Red Hat, Inc. + * Copyright (C) 2006-2010 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1695,6 +1695,7 @@ virStoragePoolSourceListNewSource(virConnectPtr conn, } if (VIR_REALLOC_N(list->sources, list->nsources+1) < 0) { + VIR_FREE(source); virReportOOMError(conn); return NULL; } -- 1.6.6.638.g2bc54

2 1

[libvirt] Cannot use console with 0.7.5, error: internal error no assigned pty for device serial0
by Marc Haber 18 Jan '10

18 Jan '10

[my apologies for this re-post. I stupidly managed to hide the first instance of this mail away in an unrelated patch-thread and am thus afraid that the people who could have answered didn't see it] Hi, I have one test host running Debian unstable (kernel 2.6.32.3), and I would like to virtualize on it using KVM and virsh. Debian unstable has libvirt 0.7.5. On this host, I cannot start any KVM domain using console with these XML parts: <serial type='pty'> <target port='0'/> </serial> <console type='pty'> <target port='0'/> </console> When I try virsh start $domain, I get the error message: error: internal error no assigned pty for device serial0 When I remove the serial and console stanza, the machine starts up. This is the configuration documented everywhere. At first, I suspected a change in the configuration syntax and filed Debian bug #565145, But on the #virt IRC channel, people suggested that there was a change in libvirt parsing KVM's output, which may be a genuine bug breaking the console. Is this already a known issue? Can I do anything to help debugging? Greetings Marc -- ----------------------------------------------------------------------------- Marc Haber | "I don't trust Computers. They | Mailadresse im Header Mannheim, Germany | lose things." Winona Ryder | Fon: *49 621 72739834 Nordisch by Nature | How to make an American Quilt | Fax: *49 3221 2323190

5 10

[libvirt] Current cpu and memory usage for Host and Domains
by anuj rampal 18 Jan '10

18 Jan '10

Hi, Is there a way to get the Current CPU and Memory usage of the Host machine and the Guest Domains..?? Regards Anuj

4 5

[libvirt] virsh list after libvirt restart "forgets" existing guests (0.7.5)
by Thomas Sjolshagen 18 Jan '10

18 Jan '10

After the upgrade to 0.7.5 from the preview repository, I've noticed that a restart of libvirt does not appear to "re-acquire" any guests already running on the system. # virsh list Compiled against library: libvir 0.7.5 Using library: libvir 0.7.5 Using API: QEMU 0.7.5 Running hypervisor: QEMU 0.12.1 # ps -ef | grep qemu-kvm qemu 1445 1 0 Jan17 ? 00:03:32 /usr/bin/qemu-kvm -S -M pc-0.11 -enable-kvm -m 512 -smp 1 -name imap [SNIP] qemu 1516 1 0 Jan17 ? 00:04:11 /usr/bin/qemu-kvm -S -M pc-0.11 -cpu qemu32 -enable-kvm -m 512 -smp 1 -name mail [SNIP] qemu 21409 1 16 Jan16 ? 07:29:32 /usr/bin/qemu-kvm -S -M pc-0.11 -enable-kvm -m 1640 -smp 1 -name imap2-cluster [SNIP] # virsh list Id Name State ---------------------------------- 1 imap running 2 mail running Additionally, something doesn't appear to be getting either parsed right or set correctly when the guest is started as the /var/log/libvirt/qemu/<guestname>.log file contains: "Option 'ipv4: Use 'on' or 'off'" "Failed to parse "yes" for "dummy.ipv4"

2 1

[libvirt] [PATCH] xen_driver: don't leak a parsed-config buffer
by Jim Meyering 18 Jan '10

18 Jan '10

Another leak: >From 10a428a2ed1ddbe4da2b4fbd7690da5a0fe0420b Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 18 Jan 2010 18:02:17 +0100 Subject: [PATCH] xen_driver: don't leak a parsed-config buffer * src/xen/xen_driver.c (xenUnifiedDomainXMLFromNative): Also free "conf" before returning. --- src/xen/xen_driver.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 4911c9e..72f56ae 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1,7 +1,7 @@ /* * xen_driver.c: Unified Xen driver. * - * Copyright (C) 2007, 2008, 2009 Red Hat, Inc. + * Copyright (C) 2007-2010 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -1199,6 +1199,8 @@ xenUnifiedDomainXMLFromNative(virConnectPtr conn, cleanup: virDomainDefFree(def); + if (conf) + virConfFree(conf); return ret; } -- 1.6.6.638.g2bc54

2 1

[libvirt] [PATCH] qemu_driver: don't leak a virDomainDeviceDef buffer
by Jim Meyering 18 Jan '10

18 Jan '10

At first I was going to call virDomainDeviceDefFree only "if (dev)", but saw that it handles a NULL "dev" just fine, so it's better to skip the test altogether, just as we do for many other free-like functions. >From ea8511d709492f5cdc152a1eaccbccd05f036648 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 18 Jan 2010 16:55:36 +0100 Subject: [PATCH] qemu_driver: don't leak a virDomainDeviceDef buffer * src/qemu/qemu_driver.c (qemudDomainAttachDevice): Don't leak "dev". --- src/qemu/qemu_driver.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 365921f..1aa8af6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6062,11 +6062,11 @@ cleanup: if (cgroup) virCgroupFree(&cgroup); - if (ret < 0 && dev != NULL) { + if (ret < 0 && dev != NULL) if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 1) < 0) VIR_WARN0("Fail to restore disk device ownership"); - virDomainDeviceDefFree(dev); - } + virDomainDeviceDefFree(dev); + if (vm) virDomainObjUnlock(vm); qemuDriverUnlock(driver); -- 1.6.6.638.g2bc54

3 4

[libvirt] 0.7.5 errors in configure script
by Avi Weit 18 Jan '10

18 Jan '10

Hi, I am using Ubuntu 9.10 and downloaded and extracted libvirt.0.7.5. I have issued: ./configure --prefix=$HOME/usr --without-xen but I get: You must install device-mapper-devel/libdevmapper >= 1.0.0 to compile libvirt Below is a snippet of the final configure output: checking for showmount... no checking for pvcreate... no checking for vgcreate... no checking for lvcreate... no checking for pvremove... no checking for vgremove... no checking for lvremove... no checking for vgchange... no checking for vgscan... no checking for pvs... no checking for vgs... no checking for lvs... no checking for iscsiadm... no checking for DEVMAPPER... no checking libdevmapper.h usability... no checking libdevmapper.h presence... no checking for libdevmapper.h... no checking for dm_task_run in -ldevmapper... no configure: error: You must install device-mapper-devel/libdevmapper >= 1.0.0 to compile libvirt /dev/mapper exists on the system. Any help is appreciated. Thanks, - Avi

4 4

[libvirt] [PATCH 00/34] Add device addressing and disk controller support
by Daniel P. Berrange 18 Jan '10

18 Jan '10

This series is a merge of two previous series I posted http://www.redhat.com/archives/libvir-list/2009-December/msg00232.html http://www.redhat.com/archives/libvir-list/2009-December/msg00392.html It accomplishes quite a lot of things, having major impact on the QEMU driver, hopefully all in a postive way :-) In particular it does * Add standard XML syntax for addressing of PCI devices, and disk drives * Add support for disk controllers as a managed device in XML * Add support for disk controller hotplug/unplug * Convert everything over to use QEMU's -device flag where available * Add PCI addressing when using -device * Introduce a way to give every device a unique 'alias' name in the XML format I can't promise it works perfectly, but i've done quite alot of positive testing with it now & believe all the back comptability stuff is working right.

4 85

[libvirt] [PATCH] storage_backend_fs.c: do not ignore probe failure
by Jim Meyering 18 Jan '10

18 Jan '10

When virStorageBackendProbeTarget fails, it returns -1 or -2. The two uses below obviously intended to handle those cases differently, but due to a wrong comparison, they always treated a "real" (e.g., open) failure (-1) just like an ignorable "wrong file type" failure (-2). FYI, coverity reported that the "goto cleanup" statement was unreachable, which was true in each case, but hardly the real problem. >From c9ba79daf124afbab351741192bbaf7110a199dd Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 18 Jan 2010 10:34:53 +0100 Subject: [PATCH] storage_backend_fs.c: do not ignore probe failure * src/storage/storage_backend_fs.c (virStorageBackendFileSystemRefresh): Correct two error-handling bugs. The documented intent is to ignore non-regular files, yet due to a comparison "< 0" (rather than "!= 0") all errors were handled that way. (vboxStorageVolDelete): Likewise. (vboxDomainDefineXML): Likewise. --- src/storage/storage_backend_fs.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 4fe40b3..fa72f9c 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1,7 +1,7 @@ /* * storage_backend_fs.c: storage backend for FS and directory handling * - * Copyright (C) 2007-2009 Red Hat, Inc. + * Copyright (C) 2007-2010 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -562,7 +562,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, &backingStore, &vol->allocation, &vol->capacity, - &vol->target.encryption) < 0)) { + &vol->target.encryption) != 0)) { if (ret == -1) goto cleanup; else { @@ -603,7 +603,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn, if ((ret = virStorageBackendProbeTarget(conn, &vol->backingStore, NULL, NULL, NULL, - NULL)) < 0) { + NULL)) != 0) { if (ret == -1) goto cleanup; else { -- 1.6.6.638.g2bc54

2 3