- Devel - libvirt List Archives

[libvirt] [PATCH] Fix stream procedure number for virDomainMigratePrepareTunnel3
by Daniel P. Berrange 30 Jun '11

30 Jun '11

The virDomainMigratePrepareTunnel3 impl in the remote driver was using the procedure number for the virDomainMigratePrepareTunnel method. This doesn't work out so well, because it makes the server ignore & drop all stream packets * src/remote/remote_driver.c: Fix procedure for PrepareTunnel3 --- src/remote/remote_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index ce9bcb1..f318740 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3691,7 +3691,7 @@ remoteDomainMigratePrepareTunnel3(virConnectPtr dconn, memset(&ret, 0, sizeof(ret)); if (!(netst = virNetClientStreamNew(priv->remoteProgram, - REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL, + REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL3, priv->counter))) goto done; -- 1.7.4.4

2 2

[libvirt] [PATCH] Send back an error if we get unexpected stream control message
by Daniel P. Berrange 30 Jun '11

30 Jun '11

We ignore any stream data packets which come in for streams which are not registered, since these packets are async and do not have a reply. If we get a stream control packet though we must send back an actual error, otherwise a (broken) client may hang forever making it hard to diagnose the client bug. * src/rpc/virnetserverprogram.c: Send back error for unexpected stream control messages --- src/rpc/virnetserverprogram.c | 25 +++++++++++++++++-------- 1 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c index ca80ae0..63a6b6d 100644 --- a/src/rpc/virnetserverprogram.c +++ b/src/rpc/virnetserverprogram.c @@ -257,14 +257,23 @@ int virNetServerProgramDispatch(virNetServerProgramPtr prog, * stream packets after we closed down a stream. Just drop & ignore * these. */ - VIR_INFO("Ignoring unexpected stream data serial=%d proc=%d status=%d", - msg->header.serial, msg->header.proc, msg->header.status); - /* Send a dummy reply to free up 'msg' & unblock client rx */ - memset(msg, 0, sizeof(*msg)); - msg->header.type = VIR_NET_REPLY; - if (virNetServerClientSendMessage(client, msg) < 0) { - ret = -1; - goto cleanup; + if (msg->header.status == VIR_NET_CONTINUE) { + VIR_INFO("Ignoring unexpected stream data serial=%d proc=%d status=%d", + msg->header.serial, msg->header.proc, msg->header.status); + /* Send a dummy reply to free up 'msg' & unblock client rx */ + memset(msg, 0, sizeof(*msg)); + msg->header.type = VIR_NET_REPLY; + if (virNetServerClientSendMessage(client, msg) < 0) { + ret = -1; + goto cleanup; + } + } else { + VIR_INFO("Unexpected stream control message serial=%d proc=%d status=%d", + msg->header.serial, msg->header.proc, msg->header.status); + virNetError(VIR_ERR_RPC, + _("Unexpected stream control message serial=%d proc=%d status=%d"), + msg->header.serial, msg->header.proc, msg->header.status); + goto error; } ret = 0; break; -- 1.7.4.4

2 1

[libvirt] [PATCH] Fix release of virNetMessagePtr instances in streams processing
by Daniel P. Berrange 30 Jun '11

30 Jun '11

If a message packet for a invalid stream is received it is just free'd. This is not good because it doesn't let the client RPC request counter decrement. If a stream is shutdown with pending packets the message also isn't released properly because of an incorrect header type * daemon/stream.c: Fix message header type * src/rpc/virnetserverprogram.c: Send dummy reply instead of free'ing ignored stream message --- daemon/stream.c | 1 + src/rpc/virnetserverprogram.c | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletions(-) diff --git a/daemon/stream.c b/daemon/stream.c index 685cee2..56d79c2 100644 --- a/daemon/stream.c +++ b/daemon/stream.c @@ -327,6 +327,7 @@ int daemonFreeClientStream(virNetServerClientPtr client, virNetMessagePtr tmp = msg->next; /* Send a dummy reply to free up 'msg' & unblock client rx */ memset(msg, 0, sizeof(*msg)); + msg->header.type = VIR_NET_REPLY; if (virNetServerClientSendMessage(client, msg) < 0) { virNetServerClientMarkClose(client); virNetMessageFree(msg); diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c index 4afed64..ca80ae0 100644 --- a/src/rpc/virnetserverprogram.c +++ b/src/rpc/virnetserverprogram.c @@ -259,7 +259,13 @@ int virNetServerProgramDispatch(virNetServerProgramPtr prog, */ VIR_INFO("Ignoring unexpected stream data serial=%d proc=%d status=%d", msg->header.serial, msg->header.proc, msg->header.status); - virNetMessageFree(msg); + /* Send a dummy reply to free up 'msg' & unblock client rx */ + memset(msg, 0, sizeof(*msg)); + msg->header.type = VIR_NET_REPLY; + if (virNetServerClientSendMessage(client, msg) < 0) { + ret = -1; + goto cleanup; + } ret = 0; break; @@ -275,6 +281,7 @@ int virNetServerProgramDispatch(virNetServerProgramPtr prog, error: ret = virNetServerProgramSendReplyError(prog, client, msg, &rerr, &msg->header); +cleanup: return ret; } -- 1.7.4.4

2 1

[libvirt] libvirt-0.9.3 week freeze, RC1 version to test
by Daniel Veillard 30 Jun '11

30 Jun '11

So we are entering the week freeze for the new version 0.9.3 I have made an rc1 tarball available at: ftp://libvirt.org/libvirt/libvirt-0.9.3-rc1.tar.gz along with rpms for those interested We pushed a lot of patches in the last few days, we know there is at least some cleanup issues, but I tried it locally and basic operations seems to work fine, please give it a try too and report issues, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

11 26

[libvirt] [PATCH 0/3] Fix stream upload in new RPC code
by Daniel P. Berrange 30 Jun '11

30 Jun '11

These patches do three small fixes to the RPC code in libvirtd to make sure stream upload works correctly again

2 7

[libvirt] [PATCH] Fix leak of virStreamPtr object with callback added in fdstream impl
by Daniel P. Berrange 30 Jun '11

30 Jun '11

When adding a callback to an FD stream, we take an extra reference on the virStreamPtr instance. We forgot to registered a free function with the callback, so when the callback was removed, the extra reference held on virStreamPtr was not released. * src/fdstream.c: Use a free callback to release reference on virStreamPtr when removing callback --- src/fdstream.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/src/fdstream.c b/src/fdstream.c index 182b6fa..54f8198 100644 --- a/src/fdstream.c +++ b/src/fdstream.c @@ -171,6 +171,13 @@ static void virFDStreamEvent(int watch ATTRIBUTE_UNUSED, } } +static void virFDStreamCallbackFree(void *opaque) +{ + virStreamPtr st = opaque; + virStreamFree(st); +} + + static int virFDStreamAddCallback(virStreamPtr st, int events, @@ -198,7 +205,7 @@ virFDStreamAddCallback(virStreamPtr st, events, virFDStreamEvent, st, - NULL)) < 0) { + virFDStreamCallbackFree)) < 0) { streamsReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot register file watch on stream")); goto cleanup; -- 1.7.4.4

2 1

[libvirt] [PATCH] Fix leak of mdnsGroupName in virNetServer object
by Daniel P. Berrange 30 Jun '11

30 Jun '11

* src/rpc/virnetserver.c: Free mdnsGroupName --- src/rpc/virnetserver.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 8a2b113..0ef81fd 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -746,6 +746,8 @@ void virNetServerFree(virNetServerPtr srv) } VIR_FREE(srv->clients); + VIR_FREE(srv->mdnsGroupName); + virNetServerUnlock(srv); virMutexDestroy(&srv->lock); VIR_FREE(srv); -- 1.7.4.4

2 1

[libvirt] [PATCH] Fix potential crash when saving guests
by Daniel P. Berrange 30 Jun '11

30 Jun '11

The qemudDomainSaveFlag method will call EndJob on the 'vm' object it is passed in. This can result in the 'vm' object being free'd if the last reference is removed. Thus no caller of 'qemudDomainSaveFlag' must *ever* reference 'vm' again upon return. Unfortunately qemudDomainSave and qemuDomainManagedSave both call 'virDomainObjUnlock', which can result in a crash. This is non-deterministic since it involves a race with the monitor I/O thread. Fix this by making qemudDomainSaveFlag responsible for calling virDomainObjUnlock instead. * src/qemu/qemu_driver.c: Fix potential use after free when saving guests --- src/qemu/qemu_driver.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d63f57d..363a361 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2091,7 +2091,10 @@ qemuCompressProgramName(int compress) } /* This internal function expects the driver lock to already be held on - * entry and the vm must be active. + * entry and the vm must be active + locked. Vm will be unlocked and + * potentially free'd after this returns (eg transient VMs are freed + * shutdown). So 'vm' must not be referenced by the caller after + * this returns (whether returning success or failure). */ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, virDomainObjPtr vm, const char *path, @@ -2318,6 +2321,8 @@ cleanup: unlink(path); if (event) qemuDomainEventQueue(driver, event); + if (vm) + virDomainObjUnlock(vm); return ret; } @@ -2380,6 +2385,7 @@ static int qemudDomainSave(virDomainPtr dom, const char *path) } ret = qemudDomainSaveFlag(driver, dom, vm, path, compressed); + vm = NULL; cleanup: if (vm) @@ -2436,6 +2442,7 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) compressed = QEMUD_SAVE_FORMAT_RAW; ret = qemudDomainSaveFlag(driver, dom, vm, name, compressed); + vm = NULL; cleanup: if (vm) -- 1.7.4.4

2 1

[libvirt] [PATCH] Fix uninitialized value in QEMU monitor FD sending code
by Daniel P. Berrange 30 Jun '11

30 Jun '11

The 'char control[CMSG_SPACE(sizeof(int))];' was not being wiped, so could potentially contain uninitialized bytes. While this was harmless in this case, it caused complaints from valgrind * src/qemu/qemu_monitor.c: memset 'control' variable in qemuMonitorIOWriteWithFD --- src/qemu/qemu_monitor.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index e995d97..8573262 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -383,6 +383,7 @@ qemuMonitorIOWriteWithFD(qemuMonitorPtr mon, struct cmsghdr *cmsg; memset(&msg, 0, sizeof(msg)); + memset(control, 0, sizeof(control)); iov[0].iov_base = (void *)data; iov[0].iov_len = len; -- 1.7.4.4

2 1

[libvirt] [PATCH] Fix leak of JSON object for events
by Daniel P. Berrange 30 Jun '11

30 Jun '11

The event handler functions do not free the virJSONValuePtr object. Every event received from a VM thus caused a memory leak * src/qemu/qemu_monitor_json.c: Fix leak of event object --- src/qemu/qemu_monitor_json.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7d286d8..81b7f8c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -122,7 +122,6 @@ qemuMonitorJSONIOProcessLine(qemuMonitorPtr mon, if (virJSONValueObjectHasKey(obj, "QMP") == 1) { ret = 0; - virJSONValueFree(obj); } else if (virJSONValueObjectHasKey(obj, "event") == 1) { ret = qemuMonitorJSONIOProcessEvent(mon, obj); } else if (virJSONValueObjectHasKey(obj, "error") == 1 || @@ -130,6 +129,7 @@ qemuMonitorJSONIOProcessLine(qemuMonitorPtr mon, if (msg) { msg->rxObject = obj; msg->finished = 1; + obj = NULL; ret = 0; } else { qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -141,8 +141,7 @@ qemuMonitorJSONIOProcessLine(qemuMonitorPtr mon, } cleanup: - if (ret < 0) - virJSONValueFree(obj); + virJSONValueFree(obj); return ret; } -- 1.7.4.4

2 1