- Devel - libvirt List Archives

[libvirt] [PATCH] qemu: Initialize GnuTLS
by Michal Privoznik 17 Aug '11

17 Aug '11

When spice_tls is set but listen_tls is not, we don't initialize GnuTLS library. So any later gnutls call (e.g. during migration, where we initialize a certificate) will access uninitialized GnuTLS internal structs and throws an error. Although, we might now initialize GnuTLS twice, it is safe according to the documentation: This function can be called many times, but will only do something the first time. --- src/qemu/qemu_driver.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 421a98e..5fe20b6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -44,6 +44,7 @@ #include <sys/ioctl.h> #include <sys/un.h> #include <byteswap.h> +#include <gnutls/gnutls.h> #include "qemu_driver.h" @@ -537,6 +538,15 @@ qemudStartup(int privileged) { } VIR_FREE(driverConf); + if (qemu_driver->spiceTLS) { + /* Initialize GnuTLS. If it was initialized before, + * it doesn't hurt. From GnuTLS documentation: + * This function can be called many times, + * but will only do something the first time. + */ + gnutls_global_init(); + } + /* We should always at least have the 'nop' manager, so * NULLs here are a fatal error */ @@ -754,6 +764,9 @@ qemudShutdown(void) { qemuProcessAutoDestroyShutdown(qemu_driver); + if (qemu_driver->spiceTLS) + gnutls_global_deinit(); + VIR_FREE(qemu_driver->configDir); VIR_FREE(qemu_driver->autostartDir); VIR_FREE(qemu_driver->logDir); -- 1.7.3.4

2 1

[libvirt] [PATCH 0/2] Libvirt-java event handling
by js＠alien8.de 17 Aug '11

17 Aug '11

From: Julian Stecklina <jsteckli(a)os.inf.tu-dresden.de> I noticed that event handling in the libvirt Java bindings is incomplete. The first patch adds required data types and constants. The second relaxes the visibility of the Domain constructor, otherwise Callbacks are kind of useless. Regards, Julian Julian Stecklina (2): Allow different types of callbacks (just as the C API). Allow to construct Domain objects from inside callbacks. src/main/java/org/libvirt/Connect.java | 3 ++- src/main/java/org/libvirt/Domain.java | 2 +- src/main/java/org/libvirt/jna/Libvirt.java | 22 ++++++++++++++++++++-- 3 files changed, 23 insertions(+), 4 deletions(-) -- 1.7.3.4

3 4

Re: [libvirt] patch for disk migration verbose progress
by Eric Blake 17 Aug '11

17 Aug '11

[re-adding the list] On 08/17/2011 12:30 AM, Tom Vijlbrief wrote: > Hi Eric > > This is the complete patch including json (not tested) and a typo correction > in an error message... > > Tom > ---------- Doorgestuurd bericht ---------- > Van: "Tom Vijlbrief"<tom(a)nomadbsd.v7f.eu> > Datum: 17 aug. 2011 08:23 > Onderwerp: mon.patch > Aan:<tvijlbrief(a)gmail.com> > > > mon.patch > Thanks again for taking the time to contribute. It is easier to work with patches generated by 'git format-patch' (and 'git send-email' is a nice wrapper that both formats the patch and mails it to the list); as written, your patch had no commit message, so I had to invent one. I also added you to AUTHORS; let me know if I need to update any preferred spellings. > > # NB, must pass the --listen flag to the libvirtd process for this to Is this a stray line in your email, or is it pointing out a further issue? > diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c > index 2a9a078..618a45c 100644 > --- a/src/qemu/qemu_monitor_json.c > +++ b/src/qemu/qemu_monitor_json.c > @@ -1894,6 +1894,31 @@ qemuMonitorJSONGetMigrationStatusReply(virJSONValuePtr reply, > _("migration was active, but RAM 'total' data was missing")); > return -1; > } > + > + virJSONValuePtr disk = virJSONValueObjectGet(ret, "disk"); > + if (!disk) { > + return 0; > + } > + > + unsigned long long t; Our style tends to float declarations to the top, C89 style, even though this is valid as written (we require C99 for other reasons). > + if (virJSONValueObjectGetNumberUlong(disk, "transferred",&t)< 0) { > + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("disk migration was active, but DISK 'transferred' data was missing")); > + return -1; > + } > + (*transferred)+= t; > + if (virJSONValueObjectGetNumberUlong(disk, "remaining",&t)< 0) { > + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("disk migration was active, but DISK 'remaining' data was missing")); > + return -1; > + } > + (*remaining)+= t; I tweaked spacing between operators (yeah, it doesn't help that thunderbird corrupted spacing even worse in my reply to your mail). And the () are redundant; this can safely be: *remaining += t; > + if (virJSONValueObjectGetNumberUlong(disk, "total",&t)< 0) { > + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("disk migration was active, but DISK 'total' data was missing")); > + return -1; > + } > + (*total)+= t; > } > > return 0; > diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c > index 7bf733d..fef6f36 100644 > --- a/src/qemu/qemu_monitor_text.c > +++ b/src/qemu/qemu_monitor_text.c > @@ -149,7 +149,7 @@ int qemuMonitorTextIOProcess(qemuMonitorPtr mon ATTRIBUTE_UNUSED, > passwd = strstr(start, PASSWORD_PROMPT); > if (passwd) { > #if DEBUG_IO > - VIR_DEBUG("Seen a passwowrd prompt [%s]", data + used); > + VIR_DEBUG("Seen a password prompt [%s]", data + used); > #endif > if (msg->passwordHandler) { > int i; > @@ -1183,6 +1183,9 @@ cleanup: > #define MIGRATION_TRANSFER_PREFIX "transferred ram: " > #define MIGRATION_REMAINING_PREFIX "remaining ram: " > #define MIGRATION_TOTAL_PREFIX "total ram: " > +#define MIGRATION_DISK_TRANSFER_PREFIX "transferred disk: " > +#define MIGRATION_DISK_REMAINING_PREFIX "remaining disk: " > +#define MIGRATION_DISK_TOTAL_PREFIX "total disk: " > > int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr mon, > int *status, > @@ -1246,6 +1249,7 @@ int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr mon, > goto cleanup; > } > *remaining *= 1024; > + tmp = end; > > if (!(tmp = strstr(tmp, MIGRATION_TOTAL_PREFIX))) > goto done; > @@ -1257,7 +1261,53 @@ int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr mon, > goto cleanup; > } > *total *= 1024; > + tmp = end; > + > + /* > + * Check for Optional Disk Migration status > + */ > + > + unsigned long long disk_transferred= 0; > + unsigned long long disk_remaining= 0; > + unsigned long long disk_total= 0; > + > + if (!(tmp = strstr(tmp, MIGRATION_DISK_TRANSFER_PREFIX))) > + goto done; > + tmp += strlen(MIGRATION_DISK_TRANSFER_PREFIX); > > + if (virStrToLong_ull(tmp,&end, 10,&disk_transferred)< 0) { > + qemuReportError(VIR_ERR_INTERNAL_ERROR, > + _("cannot parse disk migration data transferred statistic %s"), tmp); > + goto cleanup; > + } > + disk_transferred *= 1024; > + (*transferred)+= disk_transferred; For one less assignment, I merged these two lines into: *transferred += disk_transferred * 1024; > + tmp = end; > + > + if (!(tmp = strstr(tmp, MIGRATION_DISK_REMAINING_PREFIX))) > + goto done; > + tmp += strlen(MIGRATION_DISK_REMAINING_PREFIX); > + > + if (virStrToLong_ull(tmp,&end, 10,&disk_remaining)< 0) { > + qemuReportError(VIR_ERR_INTERNAL_ERROR, > + _("cannot parse disk migration data remaining statistic %s"), tmp); > + goto cleanup; > + } > + disk_remaining *= 1024; > + (*remaining)+= disk_remaining; > + tmp = end; > + > + if (!(tmp = strstr(tmp, MIGRATION_DISK_TOTAL_PREFIX))) > + goto done; > + tmp += strlen(MIGRATION_DISK_TOTAL_PREFIX); > + > + if (virStrToLong_ull(tmp,&end, 10,&disk_total)< 0) { > + qemuReportError(VIR_ERR_INTERNAL_ERROR, > + _("cannot parse disk migration data total statistic %s"), tmp); > + goto cleanup; > + } > + disk_total *= 1024; > + (*total)+= disk_total; > } > } ACK and pushed with those tweaks. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

1 0

Re: [libvirt] Patch for verbose Disk transfer
by Eric Blake 17 Aug '11

17 Aug '11

[re-adding the list] On 08/16/2011 11:52 PM, Tom Vijlbrief wrote: > Hi Eric, > > Yes json has similar info and I also patched it, but I don't know how to > configure vir-sh so that it uses the json channel for at least testing the > patch. I can send you the patch anyway, it's a simple change... Use of JSON depends on which version of qemu you are running; if you are using Fedora 15 or newer, then JSON should automatically be in use. It's not something you can configure at the virsh command line. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

1 0

[libvirt] [PATCH] qemu: Init reattaching related members of pciDevice before reattach
by Osier Yang 17 Aug '11

17 Aug '11

Otherwise the device will still be bound to pci-stub driver even it's set as "managed=yes" when do detaching. Of course, it won't triger any driver reprobing too. --- src/qemu/qemu_hotplug.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5f449fb..b7fdfa0 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1761,6 +1761,7 @@ int qemuDomainDetachHostPciDevice(struct qemud_driver *driver, pciDeviceListDel(driver->activePciHostdevs, pci); if (pciResetDevice(pci, driver->activePciHostdevs, NULL) < 0) ret = -1; + pciDeviceReAttachInit(pci); qemuReattachPciDevice(pci, driver); pciFreeDevice(pci); } -- 1.7.6

2 2

[libvirt] [PATCH] conf: Do not load domain if the domain with same already exists
by Osier Yang 16 Aug '11

16 Aug '11

We don't allow to define domain with same name and different UUID, or with same UUID, so it's reasonable to not load the domain config if there is domain with same name already exists. Otherwise it can cause problem like: 1) % cp /etc/libvirt/qemu/dom.xml /etc/libvirt/qemu/dom_diffuuid.xml 2) remove the line with "uuid" in the "dom_diffuuid.xml" 3) % for i in {1..10}; do kill -SIGHUP $(pidof libvirtd); done 4) % virsh list --all There will be 11 domains listed with the same name, as if there is no UUID specified in domain XML, libvirt will generate one for it, which will be definitely different with the original one. --- src/conf/domain_conf.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ce1f3c5..0bed929 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10535,7 +10535,8 @@ static virDomainObjPtr virDomainLoadConfig(virCapsPtr caps, /* if the domain is already in our hashtable, we only need to * update the autostart flag */ - if ((dom = virDomainFindByUUID(doms, def->uuid))) { + if ((dom = virDomainFindByUUID(doms, def->uuid)) || + (dom = virDomainFindByName(doms, def->name))) { dom->autostart = autostart; VIR_FREE(configFile); -- 1.7.6

2 2

[libvirt] [PATCH] Ensure stream is aborted when exiting console
by Daniel P. Berrange 16 Aug '11

16 Aug '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> After running 'virsh console' in interactive mode, there was a missing call to virStreamAbort, which meant the server kept the stream resources open * tools/console.c: Abort stream when exiting --- tools/console.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/console.c b/tools/console.c index 171ebc9..82ef537 100644 --- a/tools/console.c +++ b/tools/console.c @@ -90,9 +90,11 @@ static void virConsoleShutdown(virConsolePtr con) { con->quit = true; - virStreamEventRemoveCallback(con->st); - if (con->st) + if (con->st) { + virStreamEventRemoveCallback(con->st); + virStreamAbort(con->st); virStreamFree(con->st); + } if (con->stdinWatch != -1) virEventRemoveHandle(con->stdinWatch); if (con->stdoutWatch != -1) -- 1.7.6

3 6

[libvirt] [PATCH] Ensure async packets never get marked for sync replies
by Daniel P. Berrange 16 Aug '11

16 Aug '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> If a client had initiated a stream abort, it will have a call waiting for a reply in the queue. If more data continues to arrive on the stream, the abort command could mistakenly get signalled as complete. Remove the code from async data processing that looked for waiting calls. Add a sanity check to ensure no async call can ever be marked as needing a reply * src/rpc/virnetclient.c: Ensure async data packets can't trigger a reply --- src/rpc/virnetclient.c | 12 +++++++----- 1 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index efaac88..055361d 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -627,11 +627,6 @@ static int virNetClientCallDispatchStream(virNetClientPtr client) case VIR_NET_CONTINUE: { if (virNetClientStreamQueuePacket(st, &client->msg) < 0) return -1; - - if (thecall && thecall->expectReply) { - VIR_DEBUG("Got sync data packet completion"); - thecall->mode = VIR_NET_CLIENT_MODE_COMPLETE; - } return 0; } @@ -1193,6 +1188,13 @@ int virNetClientSend(virNetClientPtr client, virNetClientCallPtr call; int ret = -1; + if (expectReply && + (msg->header.status == VIR_NET_CONTINUE)) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Attempt to send an asynchronous message with a synchronous reply")); + return -1; + } + if (VIR_ALLOC(call) < 0) { virReportOOMError(); return -1; -- 1.7.6

2 2

[libvirt] [PATCH] Don't attempt to read from a stream if it is closed
by Daniel P. Berrange 16 Aug '11

16 Aug '11

From: "Daniel P. Berrange" <berrange(a)redhat.com> The I/O event callback processes incoming packets first, and then does outgoing packets. If the incoming packet caused the stream to close, then the attempt to process outgoing data resulted in an error. This caused libvirt to then send an error back to the client, but the stream had already been stopped. This confused the client since it sees 2 error events. * daemon/stream.c: Don't attempt read if stream is closed --- daemon/stream.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/daemon/stream.c b/daemon/stream.c index 7dd9ae7..7d2b367 100644 --- a/daemon/stream.c +++ b/daemon/stream.c @@ -678,7 +678,15 @@ daemonStreamHandleRead(virNetServerClientPtr client, size_t bufferLen = VIR_NET_MESSAGE_PAYLOAD_MAX; int ret; - VIR_DEBUG("client=%p, stream=%p", client, stream); + VIR_DEBUG("client=%p, stream=%p tx=%d closed=%d", + client, stream, stream->tx, stream->closed); + + /* We might have had an event pending before we shut + * down the stream, so if we're marked as closed, + * then do nothing + */ + if (stream->closed) + return 0; /* Shouldn't ever be called unless we're marked able to * transmit, but doesn't hurt to check */ -- 1.7.6

2 1

[libvirt] [RFC] Host CPU passthrough
by Jiri Denemark 16 Aug '11

16 Aug '11

Hi, AFAIK this topic is not new but I think we still do not have a good solution for it. Libvirt already supports specifying what CPU and its features a guest should see but imagine one wants to run a guest on the best possible CPU. The current way is to copy the <cpu> element from capabilities XML into domain XML. This approach is fine since it provides reproducible environment and such domain can even be migrated to a different host. But the CPU shown provided to a guest is not the same as the real host CPU. It's based on a model which doesn't reflect all aspects of real CPUs. Ideally, we would model everything but that's quite complicated and may need updating anytime a new CPU is introduced. I think we should add a possibility to passthrough host CPU into a domain no matter what the CPU looks like. Domains configured in such a way will not be allowed to be migrated since we cannot guarantee the same CPU on destination host but they would be able to see the real CPU. Another benefit is that a single XML configuration would work on any CPU without modifications. Since possible issues with such domains will be harder to debug we can mark them as tainted just like we do if someone calls qemu monitor commands directly. I'm not sure what XML representation makes the most sense so I'll mention several options which I'm thinking about: - "host" ("native", "passthrough", whatever) special model; same name space as real CPU models, I don't like this too much: <cpu match='exact'> <model>host</model> </cpu> - "host" match policy; has nice first-look semantics that the guest CPU matches host's but doesn't play very nice with the defined semantics of "match" attribute: <cpu match='host'/> - new "mode" attribute; IMHO the best one, similar to what we have for SMBIOS: <cpu mode='host'/> vs. <cpu mode='custom' match='...'> <model>...</model. ... </cpu> What do you think? Jirka

3 6