June 2011 - Devel - libvirt List Archives

[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

[libvirt] [PATCH] Remove bogus warning message in JSON code
by Daniel P. Berrange 30 Jun '11

30 Jun '11

* src/util/json.c: Remove warning message --- src/util/json.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/util/json.c b/src/util/json.c index 48521f2..a85f580 100644 --- a/src/util/json.c +++ b/src/util/json.c @@ -950,7 +950,6 @@ cleanup: if (parser.nstate) { int i; - VIR_WARN("cleanup state %d", parser.nstate); for (i = 0 ; i < parser.nstate ; i++) { VIR_FREE(parser.state[i].key); } -- 1.7.4.4

2 1

[libvirt] [PATCH] Fix use of uninitialized memory when releasing PCI slots
by Daniel P. Berrange 30 Jun '11

30 Jun '11

The 'function' field in the PCI address was not correctly initialized, so it was building the wrong address address string and so not removing all functions from the in use list. * src/qemu/qemu_command.c: Fix initialization of PCI function --- src/qemu/qemu_command.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7ac1faf..90a6653 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -931,14 +931,14 @@ int qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs, int slot) { virDomainDeviceInfo dev; char *addr; - int function; int ret = 0; + unsigned int *function = &dev.addr.pci.function; dev.addr.pci.domain = 0; dev.addr.pci.bus = 0; dev.addr.pci.slot = slot; - for (function = 0; function <= QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) { + for (*function = 0; *function <= QEMU_PCI_ADDRESS_LAST_FUNCTION; (*function)++) { addr = qemuPCIAddressAsString(&dev); if (!addr) return -1; @@ -950,7 +950,7 @@ int qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs, int slot) VIR_FREE(addr); - if (qemuDomainPCIAddressReleaseFunction(addrs, slot, function) < 0) + if (qemuDomainPCIAddressReleaseFunction(addrs, slot, *function) < 0) ret = -1; } -- 1.7.4.4

2 1

[libvirt] [PATCH] storage: Fix problem of creating a volume in LVM pool
by Osier Yang 30 Jun '11

30 Jun '11

LVM allows one specify the VG with VG path like "/dev/lv_pool", and it gets the correct VG name internally by skipping "/dev". <snip> vg_name = skip_dev_dir(cmd, argv[0], NULL); if (strrchr(vg_name, '/')) { log_error("Volume group name expected " "(no slash)"); return 0; } </snip> However, if the path is like "/dev/t/lv_pool", the VG name will be "/t/lv_pool" then, definitely it's not a valid VG name, and LVM will complain and fail. This patch change the codes to use "pool->def->source.name" instead of "pool->def->target.path" to avoid the problem. --- src/storage/storage_backend_logical.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 4de5442..8fe69ea 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -562,7 +562,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, char size[100]; const char *cmdargvnew[] = { LVCREATE, "--name", vol->name, "-L", size, - pool->def->target.path, NULL + pool->def->source.name, NULL }; const char *cmdargvsnap[] = { LVCREATE, "--name", vol->name, "-L", size, -- 1.7.4

2 2

[libvirt] [PATCH] virsh: Fix a problem of buildPoolXML
by Osier Yang 30 Jun '11

30 Jun '11

It doesn't generate "<name>" and "<format>" nodes for "<source>" even if they are explictly specified. This patch fixes it. --- tools/virsh.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index d15d206..e82ddae 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -6372,7 +6372,7 @@ static int buildPoolXML(const vshCmd *cmd, const char **retname, char **xml) { virBufferAsprintf(&buf, "<pool type='%s'>\n", type); virBufferAsprintf(&buf, " <name>%s</name>\n", name); - if (srcHost || srcPath || srcDev) { + if (srcHost || srcPath || srcDev || srcFormat || srcName) { virBufferAddLit(&buf, " <source>\n"); if (srcHost) -- 1.7.4

2 2