- Devel - libvirt List Archives

[libvirt] [PATCH] conf: Fix libvirtd crash and memory leak caused by virDomainVcpuPinDel()
by Zhang Bo 11 Feb '15

11 Feb '15

The function virDomainVcpuPinDel() used vcpupin_list to stand for def->cputune.vcpupin, which made the codes more readable. However, in this function, it will realloc vcpupin_list later. As the definition of realloc(), it may free vcpupin_list and then points it to a new-realloced address, but def->cputune.vcpupin doesn't point to the new address(it's freed however). Thus, 1) When we refer to the def->cputune.vcpupin afterwards, which was freed by realloc(), an INVALID READ occurs, and libvirtd may crash. 2) As no one will use vcpupin_list any more, and no one frees it(it's just alloced by realloc()), memory leak occurs. Part of the valgrind logs are shown as below: ==1837== Thread 15: ==1837== Invalid read of size 8 ==1837== at 0x5367337: virDomainDefFormatInternal (domain_conf.c:18392) which is : virBufferAsprintf(buf, "<vcpupin vcpu='%u' ", def->cputune.vcpupin[i]->vcpuid); ==1837== by 0x536966C: virDomainObjFormat (domain_conf.c:18970) ==1837== by 0x5369743: virDomainSaveStatus (domain_conf.c:19166) ==1837== by 0x117B26DC: qemuDomainPinVcpuFlags (qemu_driver.c:4586) ==1837== by 0x53EA313: virDomainPinVcpuFlags (libvirt.c:9803) ==1837== by 0x14CB7D: remoteDispatchDomainPinVcpuFlags (remote_dispatch.h:6762) ==1837== by 0x14CC81: remoteDispatchDomainPinVcpuFlagsHelper (remote_dispatch.h:6740) ==1837== by 0x5464C30: virNetServerProgramDispatchCall (virnetserverprogram.c:437) ==1837== by 0x546507A: virNetServerProgramDispatch (virnetserverprogram.c:307) ==1837== by 0x171B83: virNetServerProcessMsg (virnetserver.c:172) ==1837== by 0x171E6E: virNetServerHandleJob (virnetserver.c:193) ==1837== by 0x5318E78: virThreadPoolWorker (virthreadpool.c:145) ==1837== Address 0x12ea2870 is 0 bytes inside a block of size 16 free'd ==1837== at 0x4C291AC: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==1837== by 0x52A3D14: virReallocN (viralloc.c:245) ==1837== by 0x52A3DFB: virShrinkN (viralloc.c:372) ==1837== by 0x52A3F57: virDeleteElementsN (viralloc.c:503) ==1837== by 0x533939E: virDomainVcpuPinDel (domain_conf.c:15405) //doReset为true时才会进到。 ==1837== by 0x117B2642: qemuDomainPinVcpuFlags (qemu_driver.c:4573) ==1837== by 0x53EA313: virDomainPinVcpuFlags (libvirt.c:9803) ==1837== by 0x14CB7D: remoteDispatchDomainPinVcpuFlags (remote_dispatch.h:6762) ==1837== by 0x14CC81: remoteDispatchDomainPinVcpuFlagsHelper (remote_dispatch.h:6740) ==1837== by 0x5464C30: virNetServerProgramDispatchCall (virnetserverprogram.c:437) ==1837== by 0x546507A: virNetServerProgramDispatch (virnetserverprogram.c:307) ==1837== by 0x171B83: virNetServerProcessMsg (virnetserver.c:172) Steps to reproduce the problem: 1) use virDomainPinVcpuFlags() to pin a guest's vcpu to all the pcpus of the host. This patch uses def->cputune.vcpupin instead of vcpupin_list to do the realloc() job, to avoid invalid read or memory leaking. Signed-off-by: Zhang Bo <oscar.zhangbo(a)huawei.com> Signed-off-by: Yue Wenyuan <yuewenyuan@huawei.com@huawei.com> --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f0b715d..e1a3024 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16276,7 +16276,7 @@ virDomainVcpuPinDel(virDomainDefPtr def, int vcpu) if (vcpupin_list[n]->vcpuid == vcpu) { virBitmapFree(vcpupin_list[n]->cpumask); VIR_FREE(vcpupin_list[n]); - VIR_DELETE_ELEMENT(vcpupin_list, n, def->cputune.nvcpupin); + VIR_DELETE_ELEMENT(def->cputune.vcpupin, n, def->cputune.nvcpupin); return; } } -- 1.7.12.4

2 1

[libvirt] [PATCH 0/2] Two fixes for graphics listen
by Laine Stump 11 Feb '15

11 Feb '15

These area a followup to Luyao Huang's patch to fix incorrect reporting from virsh domdisplay: https://www.redhat.com/archives/libvir-list/2015-February/msg00298.html While his patch works when the remote libvirtd is 0.9.4 or greater, it won't work when talking to older libvirt. Also the code as written isn't what was intended. Patch 1 takes care of that. Patch 2 fixes two theoretical memory leaks (which never happen in real life, but could some day if the functions are used differently). Laine Stump (2): domain: backfill listen address to parent <graphics> listen attribute domain: avoid potential memory leak in virDomainGraphicsListenSet*() src/conf/domain_conf.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) -- 2.1.0

2 3

[libvirt] [PATCH] qemu: do upfront check for vcpupids being null when querying pinning
by Daniel P. Berrange 11 Feb '15

11 Feb '15

The qemuDomainHelperGetVcpus attempted to report an error when the vcpupids info was NULL. Unfortunately earlier code would clamp the value of 'maxinfo' to 0 when nvcpupids was 0, so the error reporting would end up being skipped. This lead to 'virsh vcpuinfo <dom>' just returning an empty list instead of giving the user a clear error. --- src/qemu/qemu_driver.c | 47 +++++++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e3ca437..cd30d9a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1376,6 +1376,12 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int maxinfo, if ((hostcpus = nodeGetCPUCount()) < 0) return -1; + if (priv->vcpupids == NULL) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cpu affinity is not supported")); + return -1; + } + maxcpu = maplen * 8; if (maxcpu > hostcpus) maxcpu = hostcpus; @@ -1391,8 +1397,7 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int maxinfo, info[i].number = i; info[i].state = VIR_VCPU_RUNNING; - if (priv->vcpupids != NULL && - qemuGetProcessInfo(&(info[i].cpuTime), + if (qemuGetProcessInfo(&(info[i].cpuTime), &(info[i].cpu), NULL, vm->pid, @@ -1406,28 +1411,22 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm, virVcpuInfoPtr info, int maxinfo, if (cpumaps != NULL) { memset(cpumaps, 0, maplen * maxinfo); - if (priv->vcpupids != NULL) { - for (v = 0; v < maxinfo; v++) { - unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v); - virBitmapPtr map = NULL; - unsigned char *tmpmap = NULL; - int tmpmapLen = 0; - - if (virProcessGetAffinity(priv->vcpupids[v], - &map, maxcpu) < 0) - return -1; - virBitmapToData(map, &tmpmap, &tmpmapLen); - if (tmpmapLen > maplen) - tmpmapLen = maplen; - memcpy(cpumap, tmpmap, tmpmapLen); - - VIR_FREE(tmpmap); - virBitmapFree(map); - } - } else { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cpu affinity is not available")); - return -1; + for (v = 0; v < maxinfo; v++) { + unsigned char *cpumap = VIR_GET_CPUMAP(cpumaps, maplen, v); + virBitmapPtr map = NULL; + unsigned char *tmpmap = NULL; + int tmpmapLen = 0; + + if (virProcessGetAffinity(priv->vcpupids[v], + &map, maxcpu) < 0) + return -1; + virBitmapToData(map, &tmpmap, &tmpmapLen); + if (tmpmapLen > maplen) + tmpmapLen = maplen; + memcpy(cpumap, tmpmap, tmpmapLen); + + VIR_FREE(tmpmap); + virBitmapFree(map); } } } -- 2.1.0

3 3

[libvirt] [PATCH] qemu: fix crash in migrate when migrateuri do not have a scheme
by Luyao Huang 11 Feb '15

11 Feb '15

https://bugzilla.redhat.com/show_bug.cgi?id=1191355 When we migrate a vm with migrateuri option with a uri do not have scheme like this: # virsh migrate test4 --live qemu+ssh://lhuang/system --migrateuri 127.0.0.1 target libvirtd will crashed because uri->scheme is NULL in qemuMigrationPrepareDirect this line: if (STRNEQ(uri->scheme, "tcp") && add a value check before this line. Signed-off-by: Luyao Huang <lhuang(a)redhat.com> --- src/qemu/qemu_migration.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 879b1bf..5c3b73e 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3281,6 +3281,13 @@ qemuMigrationPrepareDirect(virQEMUDriverPtr driver, if (!(uri = qemuMigrationParseURI(uri_in, &well_formed_uri))) goto cleanup; + if (uri->scheme == NULL) { + virReportError(VIR_ERR_INVALID_ARG, + _("missing scheme in migration URI: %s"), + uri_in); + goto cleanup; + } + if (STRNEQ(uri->scheme, "tcp") && STRNEQ(uri->scheme, "rdma")) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, -- 1.8.3.1

4 4

[libvirt] [PATCH] storage: rbd: Improve the error when start a pool based on non-exist rados object
by Shanzhi Yu 11 Feb '15

11 Feb '15

When start/create a pool based on non-exist rados object, the error will be like $virsh pool-start p-c error: Failed to start pool p-c error: failed to create the RBD IoCTX. Does the pool 'libvirt-pool-clone' exist?: No such file or directory update it to error: Failed to start pool p-c error: internal error: failed to create the RBD IoCTX. the rados pool 'libvirt-pool-clone' is not available Signed-off-by: Shanzhi Yu <shyu(a)redhat.com> --- src/storage/storage_backend_rbd.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 57182de..98e7fe7 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -236,8 +236,10 @@ static int virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr ptr, virSt { int r = rados_ioctx_create(ptr->cluster, pool->def->source.name, &ptr->ioctx); if (r < 0) { - virReportSystemError(-r, _("failed to create the RBD IoCTX. Does the pool '%s' exist?"), - pool->def->source.name); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to create the RBD IoCTX. " + "the rados pool '%s' is not available"), + pool->def->source.name); } return r; } -- 2.1.0

2 3

[libvirt] [PATCH] virsh: fix show the wrong IP address for network type listen address graphic
by Luyao Huang 11 Feb '15

11 Feb '15

https://bugzilla.redhat.com/show_bug.cgi?id=1191016 We try to get the IP address in /domain/devices/graphics/@listen, howerver for the network type listen address donnot have this parameter, it will show the address in the /domain/devices/graphics/listen/@address, running XML like this: <graphics type='spice' port='5901' autoport='yes' keymap='en-us'> <listen type='network' address='192.168.122.1' network='default'/> </graphics> This patch will try to get the IP address in this path /domain/devices/graphics/listen/@address Signed-off-by: Luyao Huang <lhuang(a)redhat.com> --- tools/virsh-domain.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 358d61c..4a9b574 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10024,7 +10024,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) int tmp; int flags = 0; bool params = false; - const char *xpath_fmt = "string(/domain/devices/graphics[@type='%s']/@%s)"; + const char *xpath_fmt = "string(/domain/devices/graphics[@type='%s']/%s)"; virSocketAddr addr; if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) @@ -10054,7 +10054,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) continue; /* Create our XPATH lookup for the current display's port */ - if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "port") < 0) + if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "@port") < 0) goto cleanup; /* Attempt to get the port number for the current graphics scheme */ @@ -10068,7 +10068,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) /* Create our XPATH lookup for TLS Port (automatically skipped * for unsupported schemes */ - if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "tlsPort") < 0) + if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "@tlsPort") < 0) goto cleanup; /* Attempt to get the TLS port number */ @@ -10081,7 +10081,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) continue; /* Create our XPATH lookup for the current display's address */ - if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "listen") < 0) + if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "listen/@address") < 0) goto cleanup; /* Attempt to get the listening addr if set for the current @@ -10095,7 +10095,7 @@ cmdDomDisplay(vshControl *ctl, const vshCmd *cmd) * care of when getting the XML */ /* Create our XPATH lookup for the password */ - if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "passwd") < 0) + if (virAsprintf(&xpath, xpath_fmt, scheme[iter], "@passwd") < 0) goto cleanup; /* Attempt to get the password */ -- 1.8.3.1

3 3

[libvirt] [PATCH v2] qemu: fix setting of VM CPU affinity with TCG
by Daniel P. Berrange 10 Feb '15

10 Feb '15

If a previous commit I fixed the incorrect handling of vcpu pids for TCG mode QEMU: commit b07f3d821dfb11a118ee75ea275fd6ab737d9500 Author: Daniel P. Berrange <berrange(a)redhat.com> Date: Thu Dec 18 16:34:39 2014 +0000 Don't setup fake CPU pids for old QEMU The code assumes that def->vcpus == nvcpupids, so when we setup fake CPU pids for old QEMU with nvcpupids == 1, we cause the later code to read off the end of the array. This has fun results like sche_setaffinity(0, ...) which changes libvirtd's own CPU affinity, or even better sched_setaffinity($RANDOM, ...) which changes the affinity of a random OS process. The intent was that this would merely disable the ability to set per-vCPU affinity. It should still have been possible to set VM level host CPU affinity. Unfortunately, when you set <vcpu cpuset='0-1'>4</vcpu>, the XML parser will internally take this & initialize an entry in the def->cputune.vcpupin array for every VCPU. IOW this is implicitly being treated as <cputune> <vcpupin cpuset='0-1' vcpu='0'/> <vcpupin cpuset='0-1' vcpu='1'/> <vcpupin cpuset='0-1' vcpu='2'/> <vcpupin cpuset='0-1' vcpu='3'/> </cputune> Even more fun, the faked cputune elements are hidden from view when querying the live XML, because their cpuset mask is the same as the VM default cpumask. The upshot was that it was impossible to set VM level CPU affinity. To fix this we must update qemuProcessSetVcpuAffinities so that it only reports a fatal error if the per-VCPU cpu mask is different from the VM level cpu mask. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com> --- src/qemu/qemu_process.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) In v2: - Check for def->cpumask being NULL diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d5df60d..4c35f39 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2497,9 +2497,19 @@ qemuProcessSetVcpuAffinities(virDomainObjPtr vm) return 0; if (priv->vcpupids == NULL) { - virReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cpu affinity is not supported")); - return -1; + /* If any CPU has custom affinity that differs from the + * VM default affinity, we must reject it + */ + for (n = 0; n < def->vcpus; n++) { + if (!def->cpumask || + !virBitmapEqual(def->cpumask, + def->cputune.vcpupin[n]->cpumask)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cpu affinity is not supported")); + return -1; + } + } + return 0; } for (n = 0; n < def->vcpus; n++) { -- 2.1.0

1 0

[libvirt] virsh vcpuinfo with tcg
by Serge Hallyn 10 Feb '15

10 Feb '15

Hi, 'virsh vcpuinfo' in 1.2.12 returns an empty line for VMs using tcg. I assume this is due to commit b07f3d821dfb11 which explicitly sets nvcpupids to 0 now. Is 'virsh vcpuinfo' returning nothing just an unfortunate but expected side-effect, or is it a bug and it should return info anyway? My impression is that qemu_driver.c should be using vm->def->vcpus rather than priv->nvcpupids to determine the # of cpus to show? thanks, -serge

2 2

[libvirt] [PATCH] Fix qemu job handling in SetSchedulerParameters
by Ján Tomko 10 Feb '15

10 Feb '15

Commit c5ee5cf added a job to SetSchedulerParameters, but forgot to change one label in the SCHED_RANGE_CHECK macro. --- Not yet released. src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1614340..e3ca437 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9740,7 +9740,7 @@ qemuSetEmulatorBandwidthLive(virDomainObjPtr vm, virCgroupPtr cgroup, _("value of '%s' is out of range [%lld, %lld]"), \ NAME, MIN, MAX); \ rc = -1; \ - goto cleanup; \ + goto endjob; \ } static int -- 2.0.5

2 1

[libvirt] [PATCH 00/12] Implement random number generator hot/cold (un)plug
by Peter Krempa 10 Feb '15

10 Feb '15

Extension of Luyao's series with changes that were possible due to the prepare series for memory hotplug. Luyao Huang (7): qemu: Add helper to assign RNG device aliases qemu: refactor qemuBuildRNGDeviceArgs to allow reuse in RNG hotplug qemu: command: Make RNG backend device IDs unique audit: export virDomainAuditRNG conf: Add helpers to insert/remove/find RNG devices in domain def qemu: Implement random number generator hotplug qemu: Implement random number generator hotunplug Peter Krempa (5): conf: Introduce helper to find duplicate device address qemu: command: Shuffle around formatting of alias for RNG device backend qemu: command: Break some very long lines in qemuBuildRNGDevStr() qemu: command: Refactor creation of RNG device commandline qemu: Implement random number generator cold (un)plug src/conf/domain_audit.c | 2 +- src/conf/domain_audit.h | 7 + src/conf/domain_conf.c | 164 +++++++++++++++++ src/conf/domain_conf.h | 8 + src/libvirt_private.syms | 6 + src/qemu/qemu_command.c | 153 +++++++++++---- src/qemu/qemu_command.h | 9 + src/qemu/qemu_driver.c | 41 ++++- src/qemu/qemu_hotplug.c | 205 ++++++++++++++++++++- src/qemu/qemu_hotplug.h | 7 +- .../qemuxml2argv-aarch64-virt-virtio.args | 4 +- .../qemuxml2argv-arm-vexpressa9-virtio.args | 4 +- .../qemuxml2argv-arm-virt-virtio.args | 4 +- .../qemuxml2argv-s390-piix-controllers.args | 3 +- .../qemuxml2argv-s390-usb-none.args | 3 +- .../qemuxml2argv-virtio-rng-ccw.args | 4 +- .../qemuxml2argv-virtio-rng-default.args | 4 +- .../qemuxml2argv-virtio-rng-egd.args | 4 +- .../qemuxml2argv-virtio-rng-multiple.args | 8 +- .../qemuxml2argv-virtio-rng-random.args | 4 +- 20 files changed, 578 insertions(+), 66 deletions(-) -- 2.2.2

3 21