[PATCH] rpm: Enable KVM for riscv64 on RHEL 10+
by Andrea Bolognani
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
1 month, 2 weeks
Re: [PATCH] qemuSnapshotDeleteValidate: Fix crash when disk is
not found in VM definition
by jungleman759
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
>
1 month, 2 weeks
Invoke QMP commands using native libvirt API
by Blade Liu
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]
1 month, 2 weeks
[PATCH v2] meson: Add back prefix path for runstatedir
by Zhenzhong Duan
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
1 month, 2 weeks
Re: [PATCH] qemuSnapshotDeleteValidate: Fix crash when disk is
not found in VM definition
by jungleman759
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 month, 3 weeks
[PATCH 0/3] remote: improve missing socket diagnostics
by Daniel P. Berrangé
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
1 month, 3 weeks
[PATCH 0/3] Fix CI, build, and a typo
by Martin Kletzander
Each in its respecive commit, obviously.
Martin Kletzander (3):
nwfilter: Fix erroneous pointer passing to g_clear_pointer
python: Do not explicitly state variables are global when only read
qemu_rdp: Fix a typo existance -> existence
scripts/apibuild.py | 6 ------
src/nwfilter/nwfilter_gentech_driver.c | 4 ++--
src/qemu/qemu_rdp.c | 2 +-
tools/virt-qemu-qmp-proxy | 1 -
4 files changed, 3 insertions(+), 10 deletions(-)
--
2.49.0
1 month, 4 weeks
[PATCH] virbitmap: Change return type of virBitmapToData to void
by Alexander Kuznetsov
This function return value is invariant since e59b8f9, so change
its type and remove all dependent checks.
Found by Linux Verification Center (linuxtesting.org) with Svace.
Signed-off-by: Artem Chernyshev <artem.chernyshev(a)red-soft.ru>
Signed-off-by: Alexander Kuznetsov <kuznetsovam(a)altlinux.org>
---
src/ch/ch_monitor.c | 5 +----
src/hypervisor/domain_driver.c | 4 +---
src/qemu/qemu_driver.c | 3 +--
src/util/virbitmap.c | 6 +-----
src/util/virbitmap.h | 2 +-
src/util/virhostcpu.c | 4 ++--
tests/virbitmaptest.c | 3 +--
tools/virsh-completer-domain.c | 6 ++----
tools/virsh-domain.c | 3 +--
9 files changed, 11 insertions(+), 25 deletions(-)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 0ba927a194..b772eec8a2 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -1196,10 +1196,7 @@ virCHMonitorGetIOThreads(virCHMonitor *mon,
if (!(map = virProcessGetAffinity(iothreadinfo->iothread_id)))
goto error;
- if (virBitmapToData(map, &(iothreadinfo->cpumap),
- &(iothreadinfo->cpumaplen)) < 0) {
- goto error;
- }
+ virBitmapToData(map, &(iothreadinfo->cpumap), &(iothreadinfo->cpumaplen));
/* Append to iothreadinfolist */
iothreadinfolist[niothreads] = g_steal_pointer(&iothreadinfo);
diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c
index e2a08d737e..35966a5a8d 100644
--- a/src/hypervisor/domain_driver.c
+++ b/src/hypervisor/domain_driver.c
@@ -652,9 +652,7 @@ virDomainDriverGetIOThreadsConfig(virDomainDef *targetDef,
cpumask = bitmap;
}
}
- if (virBitmapToData(cpumask, &info_ret[i]->cpumap,
- &info_ret[i]->cpumaplen) < 0)
- goto cleanup;
+ virBitmapToData(cpumask, &info_ret[i]->cpumap, &info_ret[i]->cpumaplen);
}
*info = g_steal_pointer(&info_ret);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6ce949dd07..59ba6cfbf3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4787,8 +4787,7 @@ qemuDomainGetIOThreadsLive(virDomainObj *vm,
if (!(map = virProcessGetAffinity(iothreads[i]->thread_id)))
goto endjob;
- if (virBitmapToData(map, &info_ret[i]->cpumap, &info_ret[i]->cpumaplen) < 0)
- goto endjob;
+ virBitmapToData(map, &info_ret[i]->cpumap, &info_ret[i]->cpumaplen);
}
*info = g_steal_pointer(&info_ret);
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index e42f5b872c..8a3f33c806 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -637,10 +637,8 @@ virBitmapNewData(const void *data,
* Convert a bitmap to a chunk of data containing bits information.
* Data consists of sequential bytes, with lower bytes containing
* lower bits. This function allocates @data.
- *
- * Returns 0 on success, -1 otherwise.
*/
-int
+void
virBitmapToData(virBitmap *bitmap,
unsigned char **data,
int *dataLen)
@@ -656,8 +654,6 @@ virBitmapToData(virBitmap *bitmap,
*dataLen = len;
virBitmapToDataBuf(bitmap, *data, *dataLen);
-
- return 0;
}
diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h
index a9f9d97fd0..a9c0a1a307 100644
--- a/src/util/virbitmap.h
+++ b/src/util/virbitmap.h
@@ -91,7 +91,7 @@ virBitmap *virBitmapNewCopy(virBitmap *src) ATTRIBUTE_NONNULL(1);
virBitmap *virBitmapNewData(const void *data, int len) ATTRIBUTE_NONNULL(1);
-int virBitmapToData(virBitmap *bitmap, unsigned char **data, int *dataLen)
+void virBitmapToData(virBitmap *bitmap, unsigned char **data, int *dataLen)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
void virBitmapToDataBuf(virBitmap *bitmap, unsigned char *data, size_t len)
diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index 3b4a11effb..80819d83e6 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -1110,8 +1110,8 @@ virHostCPUGetMap(unsigned char **cpumap,
if (!(cpus = virHostCPUGetOnlineBitmap()))
goto cleanup;
- if (cpumap && virBitmapToData(cpus, cpumap, &dummy) < 0)
- goto cleanup;
+ if (cpumap)
+ virBitmapToData(cpus, cpumap, &dummy);
if (online)
*online = virBitmapCountBits(cpus);
diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
index 27b6c13114..709c62ab54 100644
--- a/tests/virbitmaptest.c
+++ b/tests/virbitmaptest.c
@@ -339,8 +339,7 @@ test5(const void *v G_GNUC_UNUSED)
ignore_value(virBitmapSetBit(bitmap, 2));
ignore_value(virBitmapSetBit(bitmap, 15));
- if (virBitmapToData(bitmap, &data2, &len2) < 0)
- return -1;
+ virBitmapToData(bitmap, &data2, &len2);
if (len2 != sizeof(data) ||
data2[0] != 0x05 ||
diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c
index 0a3a113dca..b020d87f49 100644
--- a/tools/virsh-completer-domain.c
+++ b/tools/virsh-completer-domain.c
@@ -794,8 +794,7 @@ virshDomainVcpulistViaAgentCompleter(vshControl *ctl,
if (!(onlineVcpus = virBitmapParseUnlimited(onlineVcpuStr)))
goto cleanup;
- if (virBitmapToData(onlineVcpus, &onlineVcpumap, &dummy) < 0)
- goto cleanup;
+ virBitmapToData(onlineVcpus, &onlineVcpumap, &dummy);
if (enable) {
offlinableVcpuStr = vshGetTypedParamValue(ctl, ¶ms[2]);
@@ -803,8 +802,7 @@ virshDomainVcpulistViaAgentCompleter(vshControl *ctl,
if (!(offlinableVcpus = virBitmapParseUnlimited(offlinableVcpuStr)))
goto cleanup;
- if (virBitmapToData(offlinableVcpus, &offlinableVcpumap, &dummy) < 0)
- goto cleanup;
+ virBitmapToData(offlinableVcpus, &offlinableVcpumap, &dummy);
lastcpu = virBitmapLastSetBit(offlinableVcpus);
cpulist = g_new0(char *, nvcpus - virBitmapCountBits(onlineVcpus) + 1);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index ceff6789d8..a2e30a0fa9 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7420,8 +7420,7 @@ virshParseCPUList(vshControl *ctl, int *cpumaplen,
}
}
- if (virBitmapToData(map, &cpumap, cpumaplen) < 0)
- return NULL;
+ virBitmapToData(map, &cpumap, cpumaplen);
return cpumap;
}
--
2.42.4
1 month, 4 weeks
[PATCH] nss: Fix memory leak in findLease
by Alexander Kuznetsov
path is allocated by asprintf() and must be freed later if realloc() fails or at
the end of each while() iteration
Move the free() call out of LIBVIRT_NSS_GUEST macro and add another one if
realloc() fails
Found by Linux Verification Center (linuxtesting.org) with Svace.
Reported-by: Dmitry Fedin <d.fedin(a)fobos-nt.ru>
Signed-off-by: Alexander Kuznetsov <kuznetsovam(a)altlinux.org>
---
tools/nss/libvirt_nss.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c
index d79a00a1b0..190cc7a3dd 100644
--- a/tools/nss/libvirt_nss.c
+++ b/tools/nss/libvirt_nss.c
@@ -141,8 +141,11 @@ findLease(const char *name,
goto cleanup;
tmpLease = realloc(leaseFiles, sizeof(char *) * (nleaseFiles + 1));
- if (!tmpLease)
+ if (!tmpLease) {
+ free(path);
goto cleanup;
+ }
+
leaseFiles = tmpLease;
leaseFiles[nleaseFiles++] = path;
#if defined(LIBVIRT_NSS_GUEST)
@@ -155,8 +158,8 @@ findLease(const char *name,
free(path);
goto cleanup;
}
- free(path);
#endif /* LIBVIRT_NSS_GUEST */
+ free(path);
}
errno = 0;
--
2.42.4
1 month, 4 weeks
[PATCH] virNWFilterIncludeDefToRuleInst: Prevent potential double g_free
by Alexander Kuznetsov
If virNWFilterDefToInst returns -1, it has already called virNWFilterInstReset.
Remove the additional call to prevent a double g_free
Found by Linux Verification Center (linuxtesting.org) with Svace.
Reported-by: Dmitry Fedin <d.fedin(a)fobos-nt.ru>
Signed-off-by: Alexander Kuznetsov <kuznetsovam(a)altlinux.org>
---
src/nwfilter/nwfilter_gentech_driver.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
index 41f270bb7c..f7a909bdc0 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -293,10 +293,8 @@ virNWFilterIncludeDefToRuleInst(virNWFilterDriverState *driver,
tmpvars,
useNewFilter,
foundNewFilter,
- inst) < 0) {
- virNWFilterInstReset(inst);
+ inst) < 0)
return -1;
- }
return 0;
}
--
2.42.4
1 month, 4 weeks
