April 2025 - Devel - libvirt List Archives

[PATCH] conf: Fix handling of vlan with one tagged vlan id
by Leigh Brown 25 Apr '25

25 Apr '25

Yanqiu Zhang reported [1] that when a vlan is defined with a single vlan id and native mode set to tagged, the vlan was not setup correctly using the standard Linux bridge functionality. This is due a conflict between the XML validation and the bridge functionality. The XML validation will allow a single vlan id to be configured with native mode set to tagged without setting the vlan mode to trunked. If the vlan is explicitly set to "trunk='no'" then the validation will correctly reject the configuration. Rather than force the user to specify "trunk='yes'", update the code to infer trunk='yes' if a single vlan id is configured with native mode set to a non-default value. It already infers trunk='yes' if more than one vlan id is specified. Reported-by: Yanqiu Zhang <yanqzhan(a)redhat.com> Closes: https://gitlab.com/libvirt/libvirt/-/issues/767 [1] Signed-off-by: Leigh Brown <leigh(a)solinno.co.uk> --- src/conf/netdev_vlan_conf.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/conf/netdev_vlan_conf.c b/src/conf/netdev_vlan_conf.c index 1ac66aec54..300d0d8e86 100644 --- a/src/conf/netdev_vlan_conf.c +++ b/src/conf/netdev_vlan_conf.c @@ -87,12 +87,13 @@ virNetDevVlanParse(xmlNodePtr node, xmlXPathContextPtr ctxt, virNetDevVlan *def) def->nTags = nTags; - /* now that we know how many tags there are, look for an explicit - * trunk setting. + /* In case the trunk attribute is not specified, infer it from + * the number of tags or use of native vlan mode. */ - if (nTags > 1) + if (nTags > 1 || def->nativeMode != 0) def->trunk = true; + /* Look for and validate an explicit trunk setting. */ ctxt->node = node; if ((trunk = virXPathString("string(./@trunk)", ctxt)) != NULL) { def->trunk = STRCASEEQ(trunk, "yes"); -- 2.49.0

1 0

[PATCH v2] virhostcpu: Fix potential use of unallocated memory
by Felix Huettner 24 Apr '25

24 Apr '25

In case of a host that has a large number of cpus offline the count of host cpus and the last bit set in the virHostCPUGetOnlineBitmap might diverge significantly. This can e.g. be the case when disabling smt via /sys/devices/system/cpu/smt/control. On the host this looks like: ``` $ cat /sys/devices/system/cpu/present 0-255 $ cat /sys/devices/system/cpu/online 0-127 ``` However in this case virBitmapToData previously only allocated 16 bytes for the output bitmap. This is becase the last set bit is on the 15th byte. Users of virHostCPUGetMap however rely on the "cpumap" containing enough space for all existing cpus (so they would expect 32 bytes in this case). E.g. cmdNodeCpuMap relies on this for its output. It will then actually read 32 bytes from the start of the "cpumap" address where in this case the last 16 of these bytes are uninitialized. This manifests itself in flapping outputs of "virsh nodecpumap --pretty" like: ``` $ virsh nodecpumap --pretty CPUs present: 256 CPUs online: 128 CPU map: 0-127,192,194,202 $ virsh nodecpumap --pretty CPUs present: 256 CPUs online: 128 CPU map: 0-127,192,194,197 $ virsh nodecpumap --pretty CPUs present: 256 CPUs online: 128 CPU map: 0-127,192,194,196-197 ``` This in turn potentially causes users of this data to report wrong cpu counts. Note that this only seems to happen with at least 256 physical cpus where at least 128 are offline. We fix this by preallocating the expected bitmap size. Signed-off-by: Felix Huettner <felix.huettner(a)stackit.cloud> --- src/util/virhostcpu.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 5dbcc8987c..4805484b1d 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1090,28 +1090,26 @@ virHostCPUGetMap(unsigned char **cpumap, unsigned int flags) { g_autoptr(virBitmap) cpus = NULL; - int ret = -1; - int dummy; + int ncpus = virHostCPUGetCount(); virCheckFlags(0, -1); if (!cpumap && !online) - return virHostCPUGetCount(); + return ncpus; if (!(cpus = virHostCPUGetOnlineBitmap())) - goto cleanup; + return -1; + + if (cpumap) { + int len = (ncpus + CHAR_BIT) / CHAR_BIT; + *cpumap = g_new0(unsigned char, len); + virBitmapToDataBuf(cpus, *cpumap, len); + } - if (cpumap) - virBitmapToData(cpus, cpumap, &dummy); if (online) *online = virBitmapCountBits(cpus); - ret = virHostCPUGetCount(); - - cleanup: - if (ret < 0 && cpumap) - VIR_FREE(*cpumap); - return ret; + return ncpus; } base-commit: c5a73f75bc6cae4f466d0a6344d5b3277ac9c2f4 -- 2.43.0

1 0

[PATCH] virhostcpu: Fix potential use of unallocated memory
by Felix Huettner 24 Apr '25

24 Apr '25

In case of a host that has a large number of cpus offline the count of host cpus and the last bit set in the virHostCPUGetOnlineBitmap might diverge significantly. This can e.g. be the case when disabeling smt via /sys/devices/system/cpu/smt/control. On the host this looks like: ``` $ cat /sys/devices/system/cpu/present 0-255 $ cat /sys/devices/system/cpu/online 0-127 ``` However in this case virBitmapToData previously only allocated 16 bytes for the output bitmap. This is becase the last set bit is on the 15th byte. Users of virHostCPUGetMap however rely on the "cpumap" containing enough space for all existing cpus (so they would expect 32 bytes in this case). E.g. cmdNodeCpuMap relies on this for its output. It will then actually read 32 bytes from the start of the "cpumap" address where in this case the last 16 of these bytes are uninitialized. This manifests itself in flapping outputs of "virsh nodecpumap --pretty" like: ``` $ virsh nodecpumap --pretty CPUs present: 256 CPUs online: 128 CPU map: 0-127,192,194,202 $ virsh nodecpumap --pretty CPUs present: 256 CPUs online: 128 CPU map: 0-127,192,194,197 $ virsh nodecpumap --pretty CPUs present: 256 CPUs online: 128 CPU map: 0-127,192,194,196-197 ``` This in turn potentially causes users of this data to report wrong cpu counts. Note that this only seems to happen with at least 256 phyiscal cpus where at least 128 are offline. We fix this by preallocating the expected bitmap size. Signed-off-by: Felix Huettner <felix.huettner(a)stackit.cloud> --- src/util/virhostcpu.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 5dbcc8987c..626faa88cf 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1091,22 +1091,26 @@ virHostCPUGetMap(unsigned char **cpumap, { g_autoptr(virBitmap) cpus = NULL; int ret = -1; - int dummy; virCheckFlags(0, -1); + ret = virHostCPUGetCount(); + if (!cpumap && !online) - return virHostCPUGetCount(); + return ret; if (!(cpus = virHostCPUGetOnlineBitmap())) goto cleanup; - if (cpumap) - virBitmapToData(cpus, cpumap, &dummy); + if (cpumap) { + int len = (ret + CHAR_BIT) / CHAR_BIT; + *cpumap = g_new0(unsigned char, len); + virBitmapToDataBuf(cpus, *cpumap, len); + } + if (online) *online = virBitmapCountBits(cpus); - ret = virHostCPUGetCount(); cleanup: if (ret < 0 && cpumap) base-commit: c5a73f75bc6cae4f466d0a6344d5b3277ac9c2f4 -- 2.43.0

2 2

[PATCH 0/2] qemucapabilitiestest: Update of x86_64 test data for qemu-10.0 release
by Peter Krempa 24 Apr '25

24 Apr '25

Peter Krempa (2): qemucapabilitiestest: Final update for qemu-10.0 release on x86_64 qemucapabilitiestest: Final update for qemu-10.0 release on x86_64 of the 'amdsev' variant .../caps_10.0.0_x86_64+amdsev.replies | 199 ++++++++++-------- .../caps_10.0.0_x86_64+amdsev.xml | 9 +- .../caps_10.0.0_x86_64.replies | 16 +- .../caps_10.0.0_x86_64.xml | 12 +- 4 files changed, 127 insertions(+), 109 deletions(-) -- 2.49.0

2 3

[PATCH] rpm: Enable KVM for riscv64 on RHEL 10+
by Andrea Bolognani 24 Apr '25

24 Apr '25

Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- libvirt.spec.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index cb41ea1de1..9217820137 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -8,7 +8,9 @@ %define arches_qemu_kvm %{ix86} x86_64 %{power64} %{arm} aarch64 s390x riscv64 %if 0%{?rhel} - %if 0%{?rhel} > 8 + %if 0%{?rhel} >= 10 + %define arches_qemu_kvm x86_64 aarch64 s390x riscv64 + %elif 0%{?rhel} >= 9 %define arches_qemu_kvm x86_64 aarch64 s390x %else %define arches_qemu_kvm x86_64 %{power64} aarch64 s390x -- 2.48.1

2 4

Re: [PATCH] qemuSnapshotDeleteValidate: Fix crash when disk is not found in VM definition
by jungleman759 24 Apr '25

24 Apr '25

Hi Thanks for following up, and sorry for the delay in getting back to you. You're right to suspect the issue might be related to device changes. Here’s how the crash can be triggered: The VM initially uses a SATA controller, with a disk defined as: xml 复制编辑 <controller type="scsi" index="0" model="lsilogic"></controller> <disk type='file' device='disk'> <driver name='qemu' type='qcow2'/> <source file='/var/lib/libvirt/images/Testguest.qcow2'/> <target dev='sda' bus='sata'/> </disk> A snapshot is created at this point — which records the disk as sda. Later, the VM is reconfigured to use a virtio controller, and the disk is now assigned as vda. When the VM is running and the snapshot is deleted, the snapshot code still expects to find a disk named sda in the current VM definition. Because of this mismatch, qemuDomainDiskByName() returns NULL, and the crash occurs when the result is used without a null check. This can easily happen during controller or disk bus reconfiguration between snapshot and deletion. The patch adds sanity checks to ensure we don’t dereference a null pointer in this situation. Let me know if you’d like me to adjust the wording in the error messages or add a reproducer for automated testing. Best regards, kaihuan Martin Kletzander <mkletzan(a)redhat.com> 于2025年1月22日周三 21:05写道： On Fri, Nov 29, 2024 at 10:56:45PM +0800, kaihuan wrote: >qemuDomainDiskByName() can return a NULL pointer on failure. >But this returned value in qemuSnapshotDeleteValidate is not checked.It will make libvirtd crash. > Hi, looking through old unreviewed patches I found this one. Sorry for the wait. Can you explain whether you found out how to trigger that? This looks like there is some other issue that causes a snapshot having a disk that is not in the domain. Does that happen when you want to delete a snapshot after unplugging a disk? >Signed-off-by: kaihuan <jungleman759(a)gmail.com> >--- > src/qemu/qemu_snapshot.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > >diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c >index 18b2e478f6..bcbd913073 100644 >--- a/src/qemu/qemu_snapshot.c >+++ b/src/qemu/qemu_snapshot.c >@@ -4242,8 +4242,19 @@ qemuSnapshotDeleteValidate(virDomainObj *vm, > virDomainDiskDef *vmdisk = NULL; > virDomainDiskDef *disk = NULL; > >- vmdisk = qemuDomainDiskByName(vm->def, snapDisk->name); >- disk = qemuDomainDiskByName(snapdef->parent.dom, snapDisk->name); The function calls already report an error when the disk is not found, so there is not need to rewrite that error in my opinion. >+ if (!(vmdisk = qemuDomainDiskByName(vm->def, snapDisk->name))) { >+ virReportError(VIR_ERR_OPERATION_FAILED, >+ _("disk '%1$s' referenced by snapshot '%2$s' not found in the current definition"), >+ snapDisk->name, snap->def->name); >+ return -1; >+ } >+ >+ if (!(disk = qemuDomainDiskByName(snapdef->parent.dom, snapDisk->name))) { >+ virReportError(VIR_ERR_OPERATION_FAILED, >+ _("disk '%1$s' referenced by snapshot '%2$s' not found in the VM definition of the deleted snapshot"), >+ snapDisk->name, snap->def->name); >+ return -1; >+ } > > if (!virStorageSourceIsSameLocation(vmdisk->src, disk->src)) { > virReportError(VIR_ERR_OPERATION_UNSUPPORTED, >-- >2.33.1.windows.1 >

2 1

Invoke QMP commands using native libvirt API
by Blade Liu 24 Apr '25

24 Apr '25

Hi all, I have a problem to execute qmp commands via qemu guest agent using libvirt-Python. In libivrt and qemu tutorial, qmp commands are invoked by terminal. In my project, I want to invoke the commands natively using libvirt-Python API, not using Python subprocess module. (my test shows using subprocess to invoke the commands have some issues) I'm not sure if libvirt-Python provides such API. If the API is not avaiable, as libvirt natively use C interface to interact with QEMU, do we have to implement it by hand? Another way to invoke qmp commands is communicate the socket of qemu guest agent. I tried it which did not work. Invoking qmp commands with virsh works fine. Sorry if this post should be posted in other libvirt mail lists. Thank you very much for feedbacks. Environment - OS: Centos 8.5(x86_64) - libvirt 6.0.0 - QEMU 6.0.0 === qemu guest socket $ss | grep libvirt /var/lib/libvirt/qemu/channel/target/domain-71-run-win7/org.qemu.guest_agent.0 8568953 /run/libvirt/libvirt-sock $sudo socat unix-connect:/var/lib/libvirt/qemu/channel/target/domain-71-run-win7/org.qemu.guest_agent.0 ${"execute":"guest-info"} // nothing shows === invoke qmp commands using Python import subprocess cmd = '{"execute": "guest-info"}' p = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE) result = p.communicate()[0]

2 2

[PATCH v2] meson: Add back prefix path for runstatedir
by Zhenzhong Duan 24 Apr '25

24 Apr '25

Currently libvirt favors /run instead of /var/run, but for local build run test, a prefix path is still needed to avoid interoperating with OS vendor provided binaries. When 'system' option is specified, fixed path /run is honored. Fixes: e5299ddf86121d3c792ca271ffcb54900eb19dc3 Signed-off-by: Zhenzhong Duan <zhenzhong.duan(a)intel.com> --- v2: Take option `system` into consideration (Pavel) meson.build | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/meson.build b/meson.build index bf4a245dd3..2762236f37 100644 --- a/meson.build +++ b/meson.build @@ -62,11 +62,16 @@ if get_option('system') endif localstatedir = '/var' sysconfdir = '/etc' + runstatedir = '/run' else prefix = get_option('prefix') libdir = prefix / get_option('libdir') localstatedir = prefix / get_option('localstatedir') sysconfdir = prefix / get_option('sysconfdir') + runstatedir = get_option('runstatedir') + if runstatedir == '' + runstatedir = prefix / 'run' + endif endif # if --prefix is /usr, don't use /usr/var for localstatedir or /usr/etc for @@ -80,11 +85,6 @@ if prefix == '/usr' endif endif -runstatedir = get_option('runstatedir') -if runstatedir == '' - runstatedir = '/run' -endif - initconfdir = get_option('initconfdir') if initconfdir == '' if (os_release.contains('alpine') or -- 2.34.1

2 1

Re: [PATCH] qemuSnapshotDeleteValidate: Fix crash when disk is not found in VM definition
by jungleman759 22 Apr '25

22 Apr '25

Hi Thanks for following up, and sorry for the delay in getting back to you. You're right to suspect the issue might be related to device changes. Here’s how the crash can be triggered: The VM initially uses a SATA controller, with a disk defined as: xml 复制编辑 <controller type="scsi" index="0" model="lsilogic"></controller> <disk type='file' device='disk'> <driver name='qemu' type='qcow2'/> <source file='/var/lib/libvirt/images/Testguest.qcow2'/> <target dev='sda' bus='sata'/> </disk> A snapshot is created at this point — which records the disk as sda. Later, the VM is reconfigured to use a virtio controller, and the disk is now assigned as vda. When the VM is running and try to delete the snapshot, the snapshot code still expects to find a disk named sda in the current VM definition. Because of this mismatch, qemuDomainDiskByName() returns NULL, and the crash occurs when the result is used without a null check. This can easily happen during controller or disk bus reconfiguration between snapshot and deletion. The patch adds sanity checks to ensure we don’t dereference a null pointer in this situation. Let me know if you’d like me to adjust the wording in the error messages or add a reproducer for automated testing. Best regards, kaihuan Martin Kletzander <mkletzan(a)redhat.com> 于2025年1月22日周三 21:05写道： On Fri, Nov 29, 2024 at 10:56:45PM +0800, kaihuan wrote: >qemuDomainDiskByName() can return a NULL pointer on failure. >But this returned value in qemuSnapshotDeleteValidate is not checked.It will make libvirtd crash. > Hi, looking through old unreviewed patches I found this one. Sorry for the wait. Can you explain whether you found out how to trigger that? This looks like there is some other issue that causes a snapshot having a disk that is not in the domain. Does that happen when you want to delete a snapshot after unplugging a disk? >Signed-off-by: kaihuan <jungleman759(a)gmail.com> >--- > src/qemu/qemu_snapshot.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > >diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c >index 18b2e478f6..bcbd913073 100644 >--- a/src/qemu/qemu_snapshot.c >+++ b/src/qemu/qemu_snapshot.c >@@ -4242,8 +4242,19 @@ qemuSnapshotDeleteValidate(virDomainObj *vm, > virDomainDiskDef *vmdisk = NULL; > virDomainDiskDef *disk = NULL; > >- vmdisk = qemuDomainDiskByName(vm->def, snapDisk->name); >- disk = qemuDomainDiskByName(snapdef->parent.dom, snapDisk->name); The function calls already report an error when the disk is not found, so there is not need to rewrite that error in my opinion. >+ if (!(vmdisk = qemuDomainDiskByName(vm->def, snapDisk->name))) { >+ virReportError(VIR_ERR_OPERATION_FAILED, >+ _("disk '%1$s' referenced by snapshot '%2$s' not found in the current definition"), >+ snapDisk->name, snap->def->name); >+ return -1; >+ } >+ >+ if (!(disk = qemuDomainDiskByName(snapdef->parent.dom, snapDisk->name))) { >+ virReportError(VIR_ERR_OPERATION_FAILED, >+ _("disk '%1$s' referenced by snapshot '%2$s' not found in the VM definition of the deleted snapshot"), >+ snapDisk->name, snap->def->name); >+ return -1; >+ } > > if (!virStorageSourceIsSameLocation(vmdisk->src, disk->src)) { > virReportError(VIR_ERR_OPERATION_UNSUPPORTED, >-- >2.33.1.windows.1 >

1 0

[PATCH 0/3] remote: improve missing socket diagnostics
by Daniel P. Berrangé 18 Apr '25

18 Apr '25

Daniel P. Berrangé (3): remote: improve error message when no URI is set kbase: update docs to account for changed error message remote: expand some debug messages for socket detection docs/kbase/failed_connection_after_install.rst | 10 ++++++++++ src/remote/remote_sockets.c | 9 +++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) -- 2.48.1

2 5