May 2009 - Devel - libvirt List Archives

[libvirt] [PATCH] virsh: Use consistent spacing for net-list
by Cole Robinson 29 May '09

29 May '09

There is different spacing when listing active vs. inactive networks. Ex: Name State Autostart ----------------------------------------- default active yes xxxxxx inactive no Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- src/virsh.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/virsh.c b/src/virsh.c index fe4e8e6..e6241e6 100644 --- a/src/virsh.c +++ b/src/virsh.c @@ -2798,7 +2798,7 @@ cmdNetworkList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) else autostartStr = autostart ? "yes" : "no"; - vshPrint(ctl, "%-20s %s %s\n", + vshPrint(ctl, "%-20s %-10s %-10s\n", inactiveNames[i], _("inactive"), autostartStr); -- 1.6.0.6

3 12

[libvirt] libvirt needs to dynamically support kvm-img and/or qemu-img
by Doug Goldstein 29 May '09

29 May '09

Several distros out there install "kvm-img" when they install KVM and do not install "qemu-img". The following patch checks for kvm-img and qemu-img and does the right thing. I've taken it a step further and made the check a run-time check instead of a build-time check since people are able to remove and install KVM, QEMU and Xen components as they please and as such may invalidate the build-time check. The patch prefers "kvm-img" over "qemu-img" over "qcow-create" since there was a comment that libvirt wanted to support a feature of qemu-img that wasn't present in upstream qemu releases, however the feature is present in kvm-img. The patch adds a new util function called "virFindFileInPath()" which merely searches PATH for the actual location of the binary. It might be worth-while to make some of the other build-time tool checks run-time with that function since it would allow users to get more features just by installing new tools and wouldn't require distros to disable features because they don't want libvirt to be too dependency heavy. -- Doug Goldstein

3 4

[libvirt] PATCH: Fix thread safety with HAL driver restart
by Daniel P. Berrange 29 May '09

29 May '09

The restart method of the HAL implementation of node device is not currently threadsafe, because it destroys and creates the entire driver, including its mutexes. This patch changes it to simply throw away all existing devices, and re-load them from dbus. Daniel diff -r 625ffe1918a4 src/node_device_hal.c --- a/src/node_device_hal.c Thu May 21 15:56:27 2009 +0100 +++ b/src/node_device_hal.c Thu May 21 15:56:33 2009 +0100 @@ -785,10 +785,28 @@ static int halDeviceMonitorShutdown(void static int halDeviceMonitorReload(void) { - /* XXX This isn't thread safe because its free'ing the thing - * we're locking */ - (void)halDeviceMonitorShutdown(); - return halDeviceMonitorStartup(); + DBusError err; + char **udi = NULL; + int num_devs, i; + LibHalContext *hal_ctx; + + nodeDeviceLock(driverState); + virNodeDeviceObjListFree(&driverState->devs); + nodeDeviceUnlock(driverState); + + hal_ctx = DRV_STATE_HAL_CTX(driverState); + udi = libhal_get_all_devices(hal_ctx, &num_devs, &err); + if (udi == NULL) { + fprintf(stderr, "%s: libhal_get_all_devices failed\n", __FUNCTION__); + return -1; + } + for (i = 0; i < num_devs; i++) { + dev_create(udi[i]); + VIR_FREE(udi[i]); + } + VIR_FREE(udi); + + return 0; } -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 1

[libvirt] PATCH: Fix double-free in daemon after client connection drop
by Daniel P. Berrange 29 May '09

29 May '09

If a client drops a connection unexpectedly there is a possiblity of a double free in the daemon if using SASL or TLS. This is because there is possibility for poll() on the socket, returns POLLIN and POLLHUP/ERR at the same time. Both the POLLIN and POLLHUP handling code will attempt to use qemudDispatchClientFailure to mark the client as dieing, doing a double free. It is hard to avoid this potential double-invocation of the cleanup function, so it is preferrable to make it safe Daniel diff -rup libvirt-0.6.2.orig/qemud/qemud.c libvirt-0.6.2.new/qemud/qemud.c --- libvirt-0.6.2.orig/qemud/qemud.c 2009-03-13 17:06:16.000000000 +0000 +++ libvirt-0.6.2.new/qemud/qemud.c 2009-05-28 17:58:44.000000000 +0100 @@ -1397,7 +1397,10 @@ static int qemudDispatchServer(struct qe * jobs have finished, then clean it up elsehwere */ void qemudDispatchClientFailure(struct qemud_client *client) { - virEventRemoveHandleImpl(client->watch); + if (client->watch != -1) { + virEventRemoveHandleImpl(client->watch); + client->watch = -1; + } /* Deregister event delivery callback */ if(client->conn) { @@ -1406,12 +1409,21 @@ void qemudDispatchClientFailure(struct q } #if HAVE_SASL - if (client->saslconn) sasl_dispose(&client->saslconn); + if (client->saslconn) { + sasl_dispose(&client->saslconn); + client->saslconn = NULL; + } free(client->saslUsername); + client->saslUsername = NULL; #endif - if (client->tlssession) gnutls_deinit (client->tlssession); - close(client->fd); - client->fd = -1; + if (client->tlssession) { + gnutls_deinit (client->tlssession); + client->tlssession = NULL; + } + if (client->fd != -1) { + close(client->fd); + client->fd = -1; + } } -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 1

[libvirt] PPC Qemu Machine Type Patch
by Thomas J. Baker 29 May '09

29 May '09

Here's a patch I was asked to post here that attempts to fix libvirt incorrectly specifying g3bw as the machine type when it calls qemu-system-ppc. The renamed machine type is g3beige. I searched for all occurrences of g3bw and replaced it with g3beige. Thanks, tjb -- ======================================================================= | Thomas Baker email: tjb(a)unh.edu | | Systems Programmer | | Research Computing Center voice: (603) 862-4490 | | University of New Hampshire fax: (603) 862-1761 | | 332 Morse Hall | | Durham, NH 03824 USA http://wintermute.sr.unh.edu/~tjb | =======================================================================

3 2

[libvirt] [PATCH] fix storage volume inconsistencies in schema and document
by Ryota Ozaki 29 May '09

29 May '09

Signed-off-by: Ryota Ozaki <ozaki.ryota(a)gmail.com> >From 23b096394eb15183a12c3277ac7a61e1eb73deb2 Mon Sep 17 00:00:00 2001 From: Ryota Ozaki <ozaki.ryota(a)gmail.com> Date: Sat, 23 May 2009 20:52:51 +0900 Subject: [PATCH] fix storage volume inconsistencies in schema and document This patch fixes the following inconsistencies: - The volume element does not take a type attribute, but the document says it can. - The capacity element can take a unit attribute optionally, but the schema does not reflect that. This patch also modifies a testcase to check the unit attribute. --- docs/formatstorage.html | 4 ++-- docs/formatstorage.html.in | 4 ++-- docs/schemas/storagevol.rng | 11 +++++++++++ tests/storagevolschemadata/vol-qcow2.xml | 2 +- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/docs/formatstorage.html b/docs/formatstorage.html index fc47991..e96712d 100644 --- a/docs/formatstorage.html +++ b/docs/formatstorage.html @@ -294,7 +294,7 @@ <a name="StorageVolFirst" id="StorageVolFirst">General metadata</a> </h3> <pre> - <volume type="file"> + <volume> <name>sparse.img</name> <key>/var/lib/xen/images/sparse.img</key> <allocation>0</allocation> @@ -432,7 +432,7 @@ <a name="exampleVol" id="exampleVol">Storage volume</a> </h3> <pre> - <volume type="file"> + <volume> <name>sparse.img</name> <allocation>0</allocation> <capacity unit="T">1</capacity> diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 60e2ebc..4878d72 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -183,7 +183,7 @@ <h3><a name="StorageVolFirst">General metadata</a></h3> <pre> - <volume type="file"> + <volume> <name>sparse.img</name> <key>/var/lib/xen/images/sparse.img</key> <allocation>0</allocation> @@ -353,7 +353,7 @@ <h3><a name="exampleVol">Storage volume</a></h3> <pre> - <volume type="file"> + <volume> <name>sparse.img</name> <allocation>0</allocation> <capacity unit="T">1</capacity> diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index c7bd3a7..7dc7876 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -29,6 +29,11 @@ <define name='sizing'> <optional> <element name='capacity'> + <optional> + <attribute name='unit'> + <ref name='unit'/> + </attribute> + </optional> <ref name='uint'/> </element> </optional> @@ -183,5 +188,11 @@ </data> </define> + <define name='unit'> + <data type='string'> + <param name="pattern">[kKmMgGtTpPyYzZ]</param> + </data> + </define> + </grammar> diff --git a/tests/storagevolschemadata/vol-qcow2.xml b/tests/storagevolschemadata/vol-qcow2.xml index a3f5cca..c1cf02f 100644 --- a/tests/storagevolschemadata/vol-qcow2.xml +++ b/tests/storagevolschemadata/vol-qcow2.xml @@ -3,7 +3,7 @@ <key>/var/lib/libvirt/images/OtherDemo.img</key> <source> </source> - <capacity>5242880000</capacity> + <capacity unit="G">5</capacity> <allocation>294912</allocation> <target> <path>/var/lib/libvirt/images/OtherDemo.img</path> -- 1.6.0.6

2 1

[libvirt] [PATCH 0/1] API extension tutorial
by David Allan 28 May '09

28 May '09

Here is a document describing the process for extending the libvirt API. Note that the patch includes 8 sample patches that go with the document that should be included in and not applied to the tree. Dave

4 7

[libvirt] PATCH: Fix misc bugs in QEMU ARGV -> XML convertor
by Daniel P. Berrange 28 May '09

28 May '09

Having just tried out the QEMU ARGV -> XML convertor on a random set of args posted on the mailing list I found a few bugs. It didn't cope with a -net arg without a vlan=XX option set - this should assume vlan=0 if not set. It didn't cope with quoting of args with spaces in them. It finally did not generate a random UUID or MAC address if omitted. Regards, Daniel diff -r eb060781cfb9 src/qemu_conf.c --- a/src/qemu_conf.c Wed May 27 21:07:37 2009 +0100 +++ b/src/qemu_conf.c Wed May 27 22:56:55 2009 +0100 @@ -1565,14 +1565,27 @@ static int qemuStringToArgvEnv(const cha /* Iterate over string, splitting on sequences of ' ' */ while (curr && *curr != '\0') { char *arg; - const char *next = strchr(curr, ' '); + const char *next; + if (*curr == '\'') { + curr++; + next = strchr(curr, '\''); + } else if (*curr == '"') { + curr++; + next = strchr(curr, '"'); + } else { + next = strchr(curr, ' '); + } if (!next) next = strchr(curr, '\n'); - if (next) + if (next) { arg = strndup(curr, next-curr); - else + if (*next == '\'' || + *next == '"') + next++; + } else { arg = strdup(curr); + } if (!arg) goto no_memory; @@ -1644,7 +1657,7 @@ static const char *qemuFindEnv(const cha int i; int len = strlen(name); - for (i = 0 ; progenv[i] ; i++) { + for (i = 0 ; progenv && progenv[i] ; i++) { if (STREQLEN(progenv[i], name, len) && progenv[i][len] == '=') return progenv[i] + len + 1; @@ -1883,8 +1896,10 @@ qemuFindNICForVLAN(virConnectPtr conn, int gotvlan; const char *tmp = strstr(nics[i], "vlan="); char *end; - if (tmp) - tmp += strlen("vlan="); + if (!tmp) + continue; + + tmp += strlen("vlan="); if (virStrToLong_i(tmp, &end, 10, &gotvlan) < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, @@ -1896,6 +1911,9 @@ qemuFindNICForVLAN(virConnectPtr conn, return nics[i]; } + if (wantvlan == 0 && nnics > 0) + return nics[0]; + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("cannot find NIC definition for vlan %d"), wantvlan); return NULL; @@ -1909,32 +1927,33 @@ qemuFindNICForVLAN(virConnectPtr conn, */ static virDomainNetDefPtr qemuParseCommandLineNet(virConnectPtr conn, + virCapsPtr caps, const char *val, int nnics, const char **nics) { virDomainNetDefPtr def = NULL; - char **keywords; - char **values; + char **keywords = NULL; + char **values = NULL; int nkeywords; const char *nic; int wantvlan = 0; const char *tmp; + int genmac = 1; int i; tmp = strchr(val, ','); - if (!tmp) { - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("cannot extract NIC type from '%s'"), val); - return NULL; + + if (tmp) { + if ((nkeywords = qemuParseCommandLineKeywords(conn, + tmp+1, + &keywords, + &values)) < 0) + return NULL; + } else { + nkeywords = 0; } - if ((nkeywords = qemuParseCommandLineKeywords(conn, - tmp+1, - &keywords, - &values)) < 0) - return NULL; - if (VIR_ALLOC(def) < 0) { virReportOOMError(conn); goto cleanup; @@ -1983,7 +2002,9 @@ qemuParseCommandLineNet(virConnectPtr co goto cleanup; } - if (!STRPREFIX(nic, "nic,")) { + if (!STRPREFIX(nic, "nic")) { + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("cannot parse NIC definition '%s'"), nic); virDomainNetDefFree(def); def = NULL; goto cleanup; @@ -1996,17 +2017,22 @@ qemuParseCommandLineNet(virConnectPtr co VIR_FREE(keywords); VIR_FREE(values); - if ((nkeywords = qemuParseCommandLineKeywords(conn, - nic + strlen("nic,"), - &keywords, - &values)) < 0) { - virDomainNetDefFree(def); - def = NULL; - goto cleanup; + if (STRPREFIX(nic, "nic,")) { + if ((nkeywords = qemuParseCommandLineKeywords(conn, + nic + strlen("nic,"), + &keywords, + &values)) < 0) { + virDomainNetDefFree(def); + def = NULL; + goto cleanup; + } + } else { + nkeywords = 0; } for (i = 0 ; i < nkeywords ; i++) { if (STREQ(keywords[i], "macaddr")) { + genmac = 0; virParseMacAddr(values[i], def->mac); } else if (STREQ(keywords[i], "model")) { def->model = values[i]; @@ -2014,6 +2040,9 @@ qemuParseCommandLineNet(virConnectPtr co } } + if (genmac) + virCapabilitiesGenerateMac(caps, def->mac); + cleanup: for (i = 0 ; i < nkeywords ; i++) { VIR_FREE(keywords[i]); @@ -2283,6 +2312,7 @@ error: * as is practical. This is not an exact science.... */ virDomainDefPtr qemuParseCommandLine(virConnectPtr conn, + virCapsPtr caps, const char **progenv, const char **progargv) { @@ -2303,6 +2333,8 @@ virDomainDefPtr qemuParseCommandLine(vir if (VIR_ALLOC(def) < 0) goto no_memory; + virUUIDGenerate(def->uuid); + def->id = -1; def->memory = def->maxmem = 64 * 1024; def->vcpus = 1; @@ -2604,7 +2636,7 @@ virDomainDefPtr qemuParseCommandLine(vir WANT_VALUE(); if (!STRPREFIX(val, "nic") && STRNEQ(val, "none")) { virDomainNetDefPtr net; - if (!(net = qemuParseCommandLineNet(conn, val, nnics, nics))) + if (!(net = qemuParseCommandLineNet(conn, caps, val, nnics, nics))) goto error; if (VIR_REALLOC_N(def->nets, def->nnets+1) < 0) { virDomainNetDefFree(net); @@ -2741,6 +2773,7 @@ error: virDomainDefPtr qemuParseCommandLineString(virConnectPtr conn, + virCapsPtr caps, const char *args) { const char **progenv = NULL; @@ -2751,7 +2784,7 @@ virDomainDefPtr qemuParseCommandLineStri if (qemuStringToArgvEnv(args, &progenv, &progargv) < 0) goto cleanup; - def = qemuParseCommandLine(conn, progenv, progargv); + def = qemuParseCommandLine(conn, caps, progenv, progargv); cleanup: for (i = 0 ; progargv && progargv[i] ; i++) diff -r eb060781cfb9 src/qemu_conf.h --- a/src/qemu_conf.h Wed May 27 21:07:37 2009 +0100 +++ b/src/qemu_conf.h Wed May 27 22:56:55 2009 +0100 @@ -127,9 +127,11 @@ int qemudBuildCommandLine const char *migrateFrom); virDomainDefPtr qemuParseCommandLine(virConnectPtr conn, + virCapsPtr caps, const char **progenv, const char **progargv); virDomainDefPtr qemuParseCommandLineString(virConnectPtr conn, + virCapsPtr caps, const char *args); #endif /* __QEMUD_CONF_H */ diff -r eb060781cfb9 src/qemu_driver.c --- a/src/qemu_driver.c Wed May 27 21:07:37 2009 +0100 +++ b/src/qemu_driver.c Wed May 27 22:56:55 2009 +0100 @@ -3418,6 +3418,7 @@ static char *qemuDomainXMLFromNative(vir const char *format, const char *config, unsigned int flags ATTRIBUTE_UNUSED) { + struct qemud_driver *driver = conn->privateData; virDomainDefPtr def = NULL; char *xml = NULL; @@ -3427,7 +3428,7 @@ static char *qemuDomainXMLFromNative(vir goto cleanup; } - def = qemuParseCommandLineString(conn, config); + def = qemuParseCommandLineString(conn, driver->caps, config); if (!def) goto cleanup; diff -r eb060781cfb9 tests/qemuargv2xmltest.c --- a/tests/qemuargv2xmltest.c Wed May 27 21:07:37 2009 +0100 +++ b/tests/qemuargv2xmltest.c Wed May 27 22:56:55 2009 +0100 @@ -49,7 +49,7 @@ static int testCompareXMLToArgvFiles(con if (virtTestLoadFile(xml, &expectxml, MAX_FILE) < 0) goto fail; - if (!(vmdef = qemuParseCommandLineString(NULL, cmd))) + if (!(vmdef = qemuParseCommandLineString(NULL, driver.caps, cmd))) goto fail; if (!(actualxml = virDomainDefFormat(NULL, vmdef, 0))) @@ -109,6 +109,8 @@ mymain(int argc, char **argv) if (!abs_srcdir) abs_srcdir = getcwd(cwd, sizeof(cwd)); + virRandomInitialize(0); + if ((driver.caps = testQemuCapsInit()) == NULL) return EXIT_FAILURE; if((driver.stateDir = strdup("/nowhere")) == NULL) -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 1

[libvirt] PATCH: Don't set ifname or script args for QEMU tap devices
by Daniel P. Berrange 28 May '09

28 May '09

When running QEMU with a type=bridge or type=network interface config, we pass a pre-created TAP device file descriptor, eg -net tap,fd=17,script=,vlan=0,ifname=vnet0 The ifname and script args have always been ignored by all QEMU versions we support. Latest QEMU will now error out if they are set. So this patch drops those two redundant options. NB, then are still set for type=ethernet inteface configs, because in that config we're not creating a TAP device. -net tap,fd=17,vlan=0 There is no impact on test cases, because live TAP devices is something that you can exercise in the test cases. Daniel diff -r b9dc4ea7f6aa src/qemu_conf.c --- a/src/qemu_conf.c Tue May 26 16:16:28 2009 +0100 +++ b/src/qemu_conf.c Tue May 26 22:56:09 2009 +0100 @@ -662,8 +662,8 @@ qemudNetworkIfaceConnect(virConnectPtr c } snprintf(tapfdstr, sizeof(tapfdstr), - "tap,fd=%d,script=,vlan=%d,ifname=%s", - tapfd, vlan, net->ifname); + "tap,fd=%d,vlan=%d", + tapfd, vlan); if (!(retval = strdup(tapfdstr))) goto no_memory; -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 1

[libvirt] PATCH: Always log source location if available.
by Daniel P. Berrange 28 May '09

28 May '09

Currently, even though all the logging functions get given the source function and line number, it is just discarded unless priority == DEBUG. As an example, run virsh with logging enabled, and a bogus URI LIBVIRT_DEBUG=1 LIBVIRT_LOG_OUTPUTS=1:stderr virsh -c foo://bar 13:12:06.252: debug : do_open:993 : trying driver 4 (remote) ... 13:12:06.252: debug : do_open:999 : driver 4 remote returned DECLINED 13:12:06.252: error : could not connect to foo://bar 13:12:06.252: debug : virUnrefConnect:232 : unref connection 0x99617b0 1 13:12:06.252: debug : virReleaseConnect:191 : release connection 0x99617b0 Notice how it doesn't tell the user where the log message with 'error' priority came from. The same happens for info & warn levels. With the following patch applied, we always report function if it was passed into virLogMessage() 13:15:00.456: debug : do_open:993 : trying driver 4 (remote) ... 13:15:00.456: debug : do_open:999 : driver 4 remote returned DECLINED 13:15:00.456: error : virLibConnError:390 : could not connect to foo://bar 13:15:00.456: debug : virUnrefConnect:232 : unref connection 0x8dd87b0 1 13:15:00.456: debug : virReleaseConnect:191 : release connection 0x8dd87b0 So, we now see that the error came from virLibConnError at line 390 Daniel diff -r b9dc4ea7f6aa src/logging.c --- a/src/logging.c Tue May 26 16:16:28 2009 +0100 +++ b/src/logging.c Wed May 27 13:15:55 2009 +0100 @@ -516,7 +516,7 @@ void virLogMessage(const char *category, gettimeofday(&cur_time, NULL); localtime_r(&cur_time.tv_sec, &time_info); - if ((funcname != NULL) && (priority == VIR_LOG_DEBUG)) { + if ((funcname != NULL)) { ret = virAsprintf(&msg, "%02d:%02d:%02d.%03d: %s : %s:%lld : %s\n", time_info.tm_hour, time_info.tm_min, time_info.tm_sec, (int) cur_time.tv_usec / 1000, -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2 1