September 2023 - Devel - libvirt List Archives

[PATCH] src: Avoid needless checks before calling g_strdup()
by Michal Privoznik 18 Sep '23

18 Sep '23

There are few places where the following pattern occurs: if (var) other = g_strdup(var); where @other wasn't initialized before g_strdup(). Checking for var != NULL is useless in this case, as that's exactly what g_strdup() does (in which case it returns NULL). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/conf/domain_event.c | 3 +-- src/libxl/libxl_conf.c | 6 ++---- src/lxc/lxc_native.c | 3 +-- src/qemu/qemu_monitor_json.c | 3 +-- src/util/virconf.c | 3 +-- 5 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 75603a933a..09f3368064 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -1561,8 +1561,7 @@ virDomainEventMetadataChangeNew(int id, return NULL; ev->type = type; - if (nsuri) - ev->nsuri = g_strdup(nsuri); + ev->nsuri = g_strdup(nsuri); return (virObjectEvent *)ev; } diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 3ffb46fddd..4582126d19 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -583,8 +583,7 @@ libxlMakeDomBuildInfo(virDomainDef *def, #endif /* copy SLIC table path to acpi_firmware */ - if (def->os.slic_table) - b_info->u.hvm.acpi_firmware = g_strdup(def->os.slic_table); + b_info->u.hvm.acpi_firmware = g_strdup(def->os.slic_table); if (def->nsounds > 0) { /* @@ -1198,8 +1197,7 @@ libxlMakeDisk(virDomainDiskDef *l_disk, libxl_device_disk *x_disk) return -1; } - if (l_disk->domain_name) - x_disk->backend_domname = g_strdup(l_disk->domain_name); + x_disk->backend_domname = g_strdup(l_disk->domain_name); return 0; } diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 5a63efc35f..c0011e0600 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -67,8 +67,7 @@ lxcCreateFSDef(int type, def->type = type; def->accessmode = VIR_DOMAIN_FS_ACCESSMODE_PASSTHROUGH; - if (src) - def->src->path = g_strdup(src); + def->src->path = g_strdup(src); def->dst = g_strdup(dst); def->readonly = readonly; def->usage = usage; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 137cb4e293..8152eea9a0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2960,8 +2960,7 @@ qemuMonitorJSONGetMigrationStatsReply(virJSONValue *reply, case QEMU_MONITOR_MIGRATION_STATUS_ERROR: if (error) { tmp = virJSONValueObjectGetString(ret, "error-desc"); - if (tmp) - *error = g_strdup(tmp); + *error = g_strdup(tmp); } break; diff --git a/src/util/virconf.c b/src/util/virconf.c index 934632a35f..8fdf40e9d0 100644 --- a/src/util/virconf.c +++ b/src/util/virconf.c @@ -939,8 +939,7 @@ int virConfGetValueStringList(virConf *conf, case VIR_CONF_STRING: if (compatString) { *values = g_new0(char *, cval->str ? 2 : 1); - if (cval->str) - (*values)[0] = g_strdup(cval->str); + (*values)[0] = g_strdup(cval->str); break; } G_GNUC_FALLTHROUGH; -- 2.41.0

2 1

[libvirt PATCH 00/12] Extract the integration job commands to a shell script
by Erik Skultety 18 Sep '23

18 Sep '23

Now that we have our base GitLab jobs extracted to ci/jobs.sh file, let's stay consistent and do the same for the core integration tests job template. Technically a v2 of: https://listman.redhat.com/archives/libvir-list/2023-January/237201.html Despite the above, quite a few things have changed, so I'm proposing this as a new thing. Here are test pipelines (both integration as well as normal one) after the change: https://gitlab.com/eskultety/libvirt/-/pipelines/1008044353 https://gitlab.com/eskultety/libvirt/-/pipelines/1008098378 Erik Skultety (12): syntax-check: Drop the shell's 'check for minus' rule ci: integration: Extract the integration CI main recipe to jobs.sh ci: integration: Adjust the check for CentOS Stream version ci: integration: Drop the 'install-deps' hidden job and reference ci: jobs.sh: Drop comment about the need for Avocado 98.0 ci: jobs.sh: integration: Use --quiet with virsh ci: jobs.sh: run_integration: Add/Rewrite/Reformat commentaries ci: jobs: run_integration: Make POSIX-compliant ci: jobs.sh: run_cmd: Use eval to run commands ci: jobs: integration: Execute raw commands via 'run_cmd' helper ci: jobs: run_integration: Print DAEMONS variable for debugging ci: jobs.sh: Define and create SCRATCH_DIR for local executions build-aux/syntax-check.mk | 9 ------- ci/integration-template.yml | 42 +++-------------------------- ci/jobs.sh | 53 +++++++++++++++++++++++++++++++++++-- 3 files changed, 54 insertions(+), 50 deletions(-) -- 2.41.0

2 26

[PATCH 0/8] network: Add callback for metadata changes
by K Shiva Kiran 18 Sep '23

18 Sep '23

This patchset adds support to trigger an event upon changes in the content of `<title>`, `<description>` or `<metadata>` in the network. K Shiva Kiran (8): Define Network Metadata change callback function Define Network event struct for Metadata change Add Event ID, Server side dispatcher and virsh print function Add methods to create Metadata change events Implement RPC Client for Network Metadata change callbacks Test driver implementation for Network metadata change callbacks Add Bridge Driver implementation for Network metadata change callbacks examples: Add Testing for metadata change callbacks examples/c/misc/event-test.c | 85 ++++++++++++++++++++++++++--- include/libvirt/libvirt-network.h | 22 ++++++++ src/conf/network_event.c | 84 ++++++++++++++++++++++++++++ src/conf/network_event.h | 11 ++++ src/libvirt_private.syms | 2 + src/network/bridge_driver.c | 6 ++ src/remote/remote_daemon_dispatch.c | 38 +++++++++++++ src/remote/remote_driver.c | 31 +++++++++++ src/remote/remote_protocol.x | 15 ++++- src/remote_protocol-structs | 6 ++ src/test/test_driver.c | 7 +++ tools/virsh-network.c | 46 +++++++++++++++- 12 files changed, 342 insertions(+), 11 deletions(-) -- 2.42.0

2 12

Plans for 9.8.0 release (freeze on Tuesday 26 Sep)
by Jiri Denemark 18 Sep '23

18 Sep '23

We are getting close to 9.8.0 release of libvirt. To aim for the release on Monday 02 Oct I suggest entering the freeze on Tuesday 26 Sep and tagging RC2 on Friday 29 Sep. I hope this works for everyone. Jirka

1 0

[PATCH 00/18] RFC: Remove deprecated audio features
by Martin Kletzander 15 Sep '23

15 Sep '23

I wanted to deal with https://bugzilla.redhat.com/2043498 and I got a suggesstion that removing deprecated features could actually make it easier to propagate the error. In the end (last patch) it turns out the error is still just reported with error_fatal, so it probably is not really needed, but I really wanted to dig into QEMU more and learn some of the internals for quite some time now. So I used the opportunity. The one-liner ended up being an 18 patch series which was, for someone who has just one commit in QEMU codebase, a pretty challenging task. Although I tried my best to do things properly, I am not sure whether I handled everything correctly, hence the RFC. Any comments are very much appreciated. Thanks and have a nice day ;) Martin Kletzander (18): hw/audio: Remove -soundhw support hw/input/tsc210x: Extract common init code into new function hw/audio: Simplify hda audio init hw/audio/lm4549: Add errp error reporting to init function tests/qtest: Specify audiodev= and -audiodev ui/vnc: Require audiodev= Introduce machine's default-audiodev property audio: Add easy dummy audio initialiser hw/display/xlnx_dp.c: Add audiodev property hw/input/tsc210x.c: Support machine-default audiodev with fallback hw/arm: Support machine-default audiodev with fallback hw/ppc: Support machine-default audiodev with fallback audio: Make AUD_register_card fallible and require audiodev= audio: Require AudioState in AUD_add_capture audio: Be more strict during audio backend initialisation audio: Remove legacy audio environment variables and options audio: Remove unused can_be_default audio/spiceaudio: Fail initialisation when not using spice audio/alsaaudio.c | 1 - audio/audio.c | 204 +++---- audio/audio.h | 5 +- audio/audio_int.h | 1 - audio/audio_legacy.c | 555 ------------------ audio/coreaudio.m | 1 - audio/dbusaudio.c | 1 - audio/dsoundaudio.c | 1 - audio/jackaudio.c | 1 - audio/meson.build | 1 - audio/noaudio.c | 1 - audio/ossaudio.c | 1 - audio/paaudio.c | 1 - audio/sdlaudio.c | 1 - audio/spiceaudio.c | 3 +- audio/wavaudio.c | 1 - docs/about/deprecated.rst | 24 - docs/about/removed-features.rst | 27 + docs/qdev-device-use.txt | 21 +- docs/replay.txt | 2 +- hw/arm/integratorcp.c | 8 +- hw/arm/musicpal.c | 8 +- hw/arm/omap2.c | 11 +- hw/arm/realview.c | 3 + hw/arm/spitz.c | 10 +- hw/arm/versatilepb.c | 3 + hw/arm/vexpress.c | 3 + hw/arm/xlnx-zcu102.c | 4 + hw/arm/z2.c | 12 +- hw/audio/ac97.c | 9 +- hw/audio/adlib.c | 9 +- hw/audio/cs4231a.c | 8 +- hw/audio/es1370.c | 8 +- hw/audio/gus.c | 6 +- hw/audio/hda-codec.c | 37 +- hw/audio/intel-hda.c | 25 +- hw/audio/intel-hda.h | 2 +- hw/audio/lm4549.c | 7 +- hw/audio/lm4549.h | 3 +- hw/audio/meson.build | 1 - hw/audio/pcspk.c | 15 +- hw/audio/pl041.c | 2 +- hw/audio/sb16.c | 9 +- hw/audio/soundhw.c | 177 ------ hw/audio/wm8750.c | 5 +- hw/core/machine.c | 23 + hw/display/xlnx_dp.c | 12 +- hw/input/tsc210x.c | 79 ++- hw/ppc/prep.c | 4 + hw/usb/dev-audio.c | 5 +- include/hw/audio/soundhw.h | 15 - include/hw/boards.h | 1 + qemu-options.hx | 37 -- .../codeconverter/test_regexps.py | 1 - softmmu/qdev-monitor.c | 2 - softmmu/vl.c | 10 - tests/qtest/ac97-test.c | 3 +- tests/qtest/es1370-test.c | 3 +- tests/qtest/fuzz/generic_fuzz_configs.h | 6 +- tests/qtest/intel-hda-test.c | 15 +- ui/vnc.c | 15 +- 61 files changed, 329 insertions(+), 1140 deletions(-) delete mode 100644 audio/audio_legacy.c delete mode 100644 hw/audio/soundhw.c delete mode 100644 include/hw/audio/soundhw.h -- 2.35.1

5 43

[PATCH merged] bhyveBuildControllerArgStr: Fill in missing cases to switch() statement
by Michal Privoznik 15 Sep '23

15 Sep '23

When making 'type' member of virDomainControllerDef to be of virDomainControllerType rather than an int I forgot to update bhyve_command.c. Fixes: 27a653b8931051991d44b8776a8cdc08c666b729 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- Pushed as build breaker fix. src/bhyve/bhyve_command.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 82e7e96816..bcef836eca 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -367,6 +367,18 @@ bhyveBuildControllerArgStr(const virDomainDef *def, virCommandAddArgFormat(cmd, "%d:0,lpc", controller->info.addr.pci.slot); break; + case VIR_DOMAIN_CONTROLLER_TYPE_IDE: + case VIR_DOMAIN_CONTROLLER_TYPE_FDC: + case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: + case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: + case VIR_DOMAIN_CONTROLLER_TYPE_CCID: + case VIR_DOMAIN_CONTROLLER_TYPE_XENBUS: + case VIR_DOMAIN_CONTROLLER_TYPE_LAST: + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unsupported controller device")); + return -1; + } return 0; } -- 2.41.0

1 0

[PATCH v1 0/9] query & cache host-recommended CPU model
by Collin Walling 15 Sep '23

15 Sep '23

Notes: https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg02518.html - For information regarding the QEMU "static-recommended" (aka host-recommended) CPU model, please see the analogous QEMU patches: https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg02518.html - Patches include caps added to QEMU 8.1 test files. This is a stand-in for the interim and the appropriate caps files will be updated in the future pending QEMU acceptance. Description: Allow libvirt to query, cache, and report the "host-recommended" CPU model, which reports CPU features the hypervisor recommends older CPU models to disable in order to ensure migration to new-generation machines that will no longer support said features. The host-recommended CPU is a retrieved via a query to QEMU that is a combination of two parameters: the standard model name "host" and a new model type "static-recommended". The resulting CPU is similar to the host-model which now reports deprecated CPU features disabled (e.g. featurex=off). Once this model has been queried, the libvirt ABI will parse the feature on|off parameter and set the feature policy as appropriate. The resulting model is cached in the respective QEMU capabilities file. It can be reported via the virsh domcapabilities command Two "host models" are now represented: the original "host-model" that reports the hypervisor-supported features as-is, and the new "host-recommended" that reports the hypervisor-supported features with any deprecated features paired with the disabled policy. Additionally, hypervisor-cpu-baseline has been modified to output the feature policy and report disabled features. Baseline will also expand the resulting model with the host-recommended parameters if possible. The following s390x features are flagged as deprecated and will be dropped outright on future models: csske, te, cte, bpb. Examples: To enable this CPU model, define a guest with the following <cpu> element: <cpu mode='host-recommended' match='exact' check='partial'/> A guest will be generated with a CPU e.g.: <cpu mode='custom' match='exact' check='partial'> <model fallback='forbid'>gen16a-base</model> <feature policy='require' name='nnpa'/> <feature policy='require' name='aen'/> <feature policy='require' name='cmmnt'/> <feature policy='require' name='vxpdeh'/> <feature policy='require' name='aefsi'/> <feature policy='require' name='diag318'/> <feature policy='disable' name='csske'/> <feature policy='require' name='mepoch'/> <feature policy='require' name='msa9'/> <feature policy='require' name='msa8'/> <feature policy='require' name='msa7'/> <feature policy='require' name='msa6'/> <feature policy='require' name='msa5'/> <feature policy='require' name='msa4'/> <feature policy='require' name='msa3'/> <feature policy='require' name='msa2'/> <feature policy='require' name='msa1'/> <feature policy='require' name='sthyi'/> <feature policy='require' name='edat'/> <feature policy='require' name='ri'/> <feature policy='require' name='deflate'/> <feature policy='require' name='edat2'/> <feature policy='require' name='etoken'/> <feature policy='require' name='vx'/> <feature policy='require' name='ipter'/> <feature policy='require' name='pai'/> <feature policy='require' name='paie'/> <feature policy='require' name='mepochptff'/> <feature policy='require' name='ap'/> <feature policy='require' name='vxeh'/> <feature policy='require' name='vxpd'/> <feature policy='require' name='esop'/> <feature policy='require' name='msa9_pckmo'/> <feature policy='require' name='vxeh2'/> <feature policy='require' name='esort'/> <feature policy='require' name='apqi'/> <feature policy='require' name='apft'/> <feature policy='require' name='els'/> <feature policy='require' name='iep'/> <feature policy='require' name='apqci'/> <feature policy='disable' name='cte'/> <feature policy='require' name='ais'/> <feature policy='disable' name='bpb'/> <feature policy='require' name='gs'/> <feature policy='require' name='ppa15'/> <feature policy='require' name='zpci'/> <feature policy='require' name='rdp'/> <feature policy='require' name='sea_esop2'/> <feature policy='require' name='beareh'/> <feature policy='disable' name='te'/> <feature policy='require' name='cmm'/> <feature policy='require' name='vxpdeh2'/> </cpu> An example of the hypervisor-cpu-baseline command which will now invoke a CPU model expansion with the host-recommended parameters: # virsh hypervisor-cpu-baseline host-model.xml <cpu mode='custom' match='exact'> <model fallback='forbid'>z14.2-base</model> <feature policy='require' name='aen'/> <feature policy='require' name='cmmnt'/> <feature policy='require' name='aefsi'/> <feature policy='require' name='diag318'/> <feature policy='disable' name='csske'/> <feature policy='require' name='mepoch'/> <feature policy='require' name='msa8'/> <feature policy='require' name='msa7'/> <feature policy='require' name='msa6'/> <feature policy='require' name='msa5'/> <feature policy='require' name='msa4'/> <feature policy='require' name='msa3'/> <feature policy='require' name='msa2'/> <feature policy='require' name='msa1'/> <feature policy='require' name='sthyi'/> <feature policy='require' name='edat'/> <feature policy='require' name='ri'/> <feature policy='require' name='edat2'/> <feature policy='require' name='etoken'/> <feature policy='require' name='vx'/> <feature policy='require' name='ipter'/> <feature policy='require' name='mepochptff'/> <feature policy='require' name='ap'/> <feature policy='require' name='vxeh'/> <feature policy='require' name='vxpd'/> <feature policy='require' name='esop'/> <feature policy='require' name='apqi'/> <feature policy='require' name='apft'/> <feature policy='require' name='els'/> <feature policy='require' name='iep'/> <feature policy='require' name='apqci'/> <feature policy='disable' name='cte'/> <feature policy='require' name='ais'/> <feature policy='disable' name='bpb'/> <feature policy='require' name='gs'/> <feature policy='require' name='ppa15'/> <feature policy='require' name='zpci'/> <feature policy='require' name='sea_esop2'/> <feature policy='disable' name='te'/> <feature policy='require' name='cmm'/> </cpu> Collin Walling (9): qemu: monitor: enable query for static-recommended CPU model qemu: capabilities: add static-recommended capability qemu: capabilities: refactor virQEMUCapsLoadHostCPUModelInfo qemu: capabilities: query and cache host-recommended CPU model cpu: conf: add cpu mode for host-recommended domain: capabilities: report host-recommended CPU model qemu: process: allow guest to use host-recommended CPU model qemu_driver: report feature policy when baselining qemu_driver: expand baselined model to static_recommended for s390x src/conf/cpu_conf.c | 1 + src/conf/cpu_conf.h | 1 + src/conf/domain_capabilities.c | 16 ++ src/conf/domain_capabilities.h | 1 + src/conf/schemas/cputypes.rng | 1 + src/conf/schemas/domaincaps.rng | 24 +++ src/cpu/cpu.c | 2 + src/cpu/cpu_ppc64.c | 2 + src/qemu/qemu_capabilities.c | 144 +++++++++++++++--- src/qemu/qemu_capabilities.h | 4 + src/qemu/qemu_capspriv.h | 6 +- src/qemu/qemu_command.c | 5 + src/qemu/qemu_domain.c | 2 + src/qemu/qemu_driver.c | 18 ++- src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 4 +- src/qemu/qemu_process.c | 9 +- src/qemu/qemu_validate.c | 8 + tests/cputest.c | 4 +- tests/domaincapsdata/qemu_8.1.0.s390x.xml | 55 +++++++ .../caps_8.1.0_s390x.replies | 80 +++++++++- .../qemucapabilitiesdata/caps_8.1.0_s390x.xml | 55 +++++++ ...c-cpu-kvm-ccw-virtio-8.1.s390x-latest.args | 32 ++++ .../s390-host-rec-cpu-kvm-ccw-virtio-8.1.xml | 17 +++ tests/qemuxml2argvtest.c | 2 + ...ec-cpu-kvm-ccw-virtio-8.1.s390x-latest.xml | 25 +++ tests/qemuxml2xmltest.c | 1 + 27 files changed, 486 insertions(+), 34 deletions(-) create mode 100644 tests/qemuxml2argvdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.s390x-latest.args create mode 100644 tests/qemuxml2argvdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.xml create mode 100644 tests/qemuxml2xmloutdata/s390-host-rec-cpu-kvm-ccw-virtio-8.1.s390x-latest.xml -- 2.41.0

2 13

[sdl-qemu] [PATCH 1/1] No checks, dereferencing possible
by Миронов Сергей Владимирович 14 Sep '23

14 Sep '23

No checks, dereferencing possible. Return value of a function 'virDomainChrSourceDefNew' is dereferenced at qemu_command.c without checking for NULL, but it is usually checked for this function. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 1f85f0967b ("ci: jobs.sh: Add back '--no-suite syntax-check --print-errorlogs'") Signed-off-by: Sergey Mironov <mironov(a)fintech.ru> --- src/qemu/qemu_command.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e84374b4cf..8d11972c88 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4698,6 +4698,8 @@ qemuBuildVideoCommandLine(virCommand *cmd, g_autofree char *name = g_strdup_printf("%s-vhost-user", video->info.alias); qemuDomainChrSourcePrivate *chrsrcpriv = QEMU_DOMAIN_CHR_SOURCE_PRIVATE(chrsrc); + if (chrsrc == NULL) + return -1; chrsrc->type = VIR_DOMAIN_CHR_TYPE_UNIX; chrsrcpriv->directfd = qemuFDPassDirectNew(name, &videopriv->vhost_user_fd); -- 2.31.1

3 2

[sdl-qemu] [PATCH 0/1] There are no checks, virDomainChrSourceDefNew can return 0
by Миронов Сергей Владимирович 14 Sep '23

14 Sep '23

There are no checks, virDomainChrSourceDefNew can return 0. Return value of a function 'virDomainChrSourceDefNew' is dereferenced at qemu_hotplug.c without checking for NULL, but it is usually checked for this function. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 1f85f0967b ("ci: jobs.sh: Add back '--no-suite syntax-check --print-errorlogs'") Signed-off-by: Sergey Mironov <mironov(a)fintech.ru> --- src/qemu/qemu_hotplug.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 177ca87d11..09e16c2c7e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3207,6 +3207,8 @@ qemuDomainAttachFSDevice(virQEMUDriver *driver, qemuAssignDeviceFSAlias(vm->def, fs); chardev = virDomainChrSourceDefNew(priv->driver->xmlopt); + if (chardev == NULL) + goto cleanup; chardev->type = VIR_DOMAIN_CHR_TYPE_UNIX; chardev->data.nix.path = qemuDomainGetVHostUserFSSocketPath(priv, fs); -- 2.31.1

3 4

[PATCH] qemuxml2argvtest: Fix tests failing on none x86 host CPUs
by Boris Fiuczynski 14 Sep '23

14 Sep '23

Since commit 54257ed51b51 on S390x qemuxml2argvtest fails with the following errors: 144) QEMU XML-2-ARGV cpu-kvmclock.x86_64-latest ... libvirt: CPU Driver error : the CPU is incompatible with host CPU: Host CPU does not provide required features: monitor FAILED 2023-09-14 13:01:23.883+0000: 4113077: info : libvirt version: 9.8.0 2023-09-14 13:01:23.883+0000: 4113077: info : hostname: a46lp61.lnxne.boe 2023-09-14 13:01:23.883+0000: 4113077: error : virCPUx86Compare:1954 : the CPU is incompatible with host CPU: Host CPU does not provide required features: monitor 1059) QEMU XML-2-ARGV cpu-check-partial.x86_64-latest ... libvirt: CPU Driver error : the CPU is incompatible with host CPU: Host CPU does not provide required features: monitor FAILED 2023-09-14 13:01:23.885+0000: 4113077: error : virCPUx86Compare:1954 : the CPU is incompatible with host CPU: Host CPU does not provide required features: monitor 1064) QEMU XML-2-ARGV cpu-check-default-partial2.x86_64-latest ... libvirt: CPU Driver error : the CPU is incompatible with host CPU: Host CPU does not provide required features: monitor FAILED 2023-09-14 13:01:23.885+0000: 4113077: error : virCPUx86Compare:1954 : the CPU is incompatible with host CPU: Host CPU does not provide required features: monitor 3 tests failed. Fixes: 54257ed51b5132032cedb7e1e7b8c34b9ae52115 Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com> --- tests/qemuxml2argvtest.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 0dd10f1fab..c3f05e83b9 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1035,7 +1035,7 @@ mymain(void) DO_TEST_CAPS_VER("clock-hpet-off", "7.2.0"); DO_TEST_CAPS_LATEST("clock-hpet-off"); DO_TEST_CAPS_LATEST("clock-catchup"); - DO_TEST_CAPS_LATEST("cpu-kvmclock"); + DO_TEST_CAPS_ARCH_LATEST_FULL("cpu-kvmclock", "x86_64", ARG_CAPS_HOST_CPU_MODEL, QEMU_CPU_DEF_HASWELL); DO_TEST_CAPS_LATEST("cpu-host-kvmclock"); DO_TEST_CAPS_LATEST("kvmclock"); DO_TEST_CAPS_LATEST("clock-timer-hyperv-rtc"); @@ -2201,13 +2201,13 @@ mymain(void) DO_TEST_CAPS_LATEST("memfd-memory-default-hugepage"); DO_TEST_CAPS_LATEST("cpu-check-none"); - DO_TEST_CAPS_LATEST("cpu-check-partial"); + DO_TEST_CAPS_ARCH_LATEST_FULL("cpu-check-partial", "x86_64", ARG_CAPS_HOST_CPU_MODEL, QEMU_CPU_DEF_HASWELL); DO_TEST_CAPS_LATEST("cpu-check-full"); DO_TEST_CAPS_LATEST("cpu-check-default-none"); DO_TEST_CAPS_LATEST("cpu-check-default-none2"); /* this test case uses 'cpu="host-model"', run it with Haswell host cpu to prevent test case churn */ DO_TEST_CAPS_ARCH_LATEST_FULL("cpu-check-default-partial", "x86_64", ARG_CAPS_HOST_CPU_MODEL, QEMU_CPU_DEF_HASWELL); - DO_TEST_CAPS_LATEST("cpu-check-default-partial2"); + DO_TEST_CAPS_ARCH_LATEST_FULL("cpu-check-default-partial2", "x86_64", ARG_CAPS_HOST_CPU_MODEL, QEMU_CPU_DEF_HASWELL); DO_TEST_CAPS_LATEST("cpu-cache-disable"); /* this test case uses 'cpu="host-model"', run it with Haswell host cpu to prevent test case churn */ -- 2.41.0

2 1

[PATCH 00/20] Misc cleanups (pt. 1)
by Michal Privoznik 14 Sep '23

14 Sep '23

I had these sitting on a local branch for a while. My initial plan was to use virXMLPropEnum() more and while working on that I found one bug (patch 01/20), one improvement (patch 02/20) and a couple of useless typecats. Michal Prívozník (20): lxc_controller: Access correct union member in virLXCControllerSetupHostdevCaps() src: Access hostdev->source.subsys iff VIR_DOMAIN_HOSTDEV_MODE_SUBSYS qemu_domain_address: Drop needless typecast in qemuDomainDeviceCalculatePCIConnectFlags() qemu: Drop needless typecast to virDomainFSDriverType src: Drop needless typecast to virDomainDeviceType qemu_domain_address: Drop needless typecast to virDomainDiskModel src: Drop needless typecast to virDomainDiskBus virDomainHostdevSubsysSCSI: Convert 'protocol' field to proper enum type virDomainHostdevSubsysSCSIVHost: Convert 'protocol' field to proper enum type virDomainHostdevDef: Convert 'mode' field to proper enum type virDomainHostdevCaps: Convert 'type' field to proper enum type virDomainHostdevSubsys: Convert 'type' field to proper enum type virDomainControllerDef: Convert 'type' field to proper enum type virDomainRNGDef: Convert 'model' field to proper enum type virDomainRNGDef: Convert 'backend' field to proper enum type virDomainWatchdogDef: Convert 'model' field to proper enum type virDomainWatchdogDef: Convert 'action' field to proper enum type virDomainDeviceInfo: Convert 'type' field to proper enum type virStorageNetHostDef: Convert 'transport' field to proper enum type virDomainDiskSourceNVMeParse: Use virXMLPropULongLong() src/ch/ch_domain.c | 2 +- src/conf/device_conf.c | 12 +- src/conf/device_conf.h | 4 +- src/conf/domain_audit.c | 10 +- src/conf/domain_conf.c | 291 ++++++++---------- src/conf/domain_conf.h | 29 +- src/conf/domain_postparse.c | 2 +- src/conf/domain_validate.c | 9 +- src/conf/storage_source_conf.h | 2 +- src/libxl/libxl_driver.c | 12 + src/lxc/lxc_cgroup.c | 3 + src/lxc/lxc_controller.c | 9 +- src/lxc/lxc_driver.c | 16 + src/lxc/lxc_hostdev.c | 7 + src/qemu/qemu_alias.c | 2 +- src/qemu/qemu_backup.c | 2 +- src/qemu/qemu_block.c | 2 +- src/qemu/qemu_command.c | 26 +- src/qemu/qemu_domain.c | 18 +- src/qemu/qemu_domain_address.c | 28 +- src/qemu/qemu_driver.c | 6 +- src/qemu/qemu_hotplug.c | 22 +- src/qemu/qemu_migration.c | 4 +- src/qemu/qemu_monitor.c | 12 + src/qemu/qemu_monitor_json.c | 2 +- src/qemu/qemu_namespace.c | 2 +- src/qemu/qemu_validate.c | 18 +- src/security/security_apparmor.c | 2 +- src/security/security_dac.c | 4 +- src/security/security_selinux.c | 10 +- src/security/virt-aa-helper.c | 7 + .../storage_file_backend_gluster.c | 2 +- .../storage_source_backingstore.c | 15 +- src/vbox/vbox_common.c | 4 +- 34 files changed, 333 insertions(+), 263 deletions(-) -- 2.41.0

2 22

[libvirt PATCHv1 0/8] support running virtiofsd in session mode
by Ján Tomko 13 Sep '23

13 Sep '23

https://bugzilla.redhat.com/show_bug.cgi?id=2034630 https://gitlab.com/libvirt/libvirt/-/issues/535 https://gitlab.gnome.org/GNOME/gnome-boxes/-/issues/292 Ján Tomko (8): qemu: fix indentation conf: move idmap definition earlier conf: move idmap parsing earlier conf: add idmap element to filesystem qemu: format uid/gid map for virtiofs qemu: virtiofs: do not force UID 0 qemu: allow running virtiofsd in session mode docs: virtiofs: add section about ID remapping docs/formatdomain.rst | 7 + docs/kbase/virtiofs.rst | 29 ++++ src/conf/domain_conf.c | 149 ++++++++++++------ src/conf/domain_conf.h | 29 ++-- src/conf/schemas/domaincommon.rng | 3 + src/qemu/qemu_validate.c | 25 ++- src/qemu/qemu_virtiofs.c | 17 +- .../vhost-user-fs-fd-memory.xml | 4 + 8 files changed, 181 insertions(+), 82 deletions(-) -- 2.41.0

3 20

[libvirt PATCH] ci: jobs.sh: Add back '--no-suite syntax-check --print-errorlogs'
by Erik Skultety 13 Sep '23

13 Sep '23

Commit f688a53a converted .gitlab-ci.yml to the usage of ci/jobs.sh functions, but in doing that our test options '--no-suite syntax-check --print-errorlogs' got lost in the process and since commit 8e660c52 didn't introduce them in the first place, it caused a behavioral regression. This patch adds them back. Fixes: 8e660c5286d7e2d07dd61681074bf1555955552d Signed-off-by: Erik Skultety <eskultet(a)redhat.com> --- Technically this is a build breaker fix, but sending for review anyway to see if there's an agreement on this approach. ci/jobs.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ci/jobs.sh b/ci/jobs.sh index abd695e231..eb4a4e29cf 100644 --- a/ci/jobs.sh +++ b/ci/jobs.sh @@ -39,7 +39,10 @@ run_dist() { } run_test() { + TEST_ARGS="${TEST_ARGS:=--no-suite syntax-check --print-errorlogs}" + test -f $GIT_ROOT/build/build.ninja || run_meson_setup + run_cmd meson test -C build $TEST_ARGS } -- 2.41.0

2 1

[libvirt PATCH v2 0/5] Add support for vDPA block devices
by Jonathon Jongsma 13 Sep '23

13 Sep '23

see https://bugzilla.redhat.com/show_bug.cgi?id=1900770. Changes in v2: - Don't use virStorageSource->path for vdpa device path to avoid clashing with existing path functionality - Move vdpa device opening to the qemuProcessPrepareHostStorageSource() function rather than the qemuDomainPrepareStorageSource() function. This also required some additional support in the tests for setting up the objects properly for testing. - rebased to latest master branch Jonathon Jongsma (5): conf: add ability to configure a vdpa block disk device qemu: add virtio-blk-vhost-vdpa capability qemu: make vdpa connect function more generic qemu: consider vdpa block devices for memlock limits qemu: Implement support for vDPA block devices docs/formatdomain.rst | 19 +++++++++- src/ch/ch_monitor.c | 1 + src/conf/domain_conf.c | 8 ++++ src/conf/schemas/domaincommon.rng | 13 +++++++ src/conf/storage_source_conf.c | 7 +++- src/conf/storage_source_conf.h | 2 + src/libxl/xen_xl.c | 1 + src/qemu/qemu_block.c | 20 ++++++++++ src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 24 +++++++++++- src/qemu/qemu_command.h | 1 + src/qemu/qemu_domain.c | 12 +++++- src/qemu/qemu_interface.c | 23 ------------ src/qemu/qemu_interface.h | 2 - src/qemu/qemu_migration.c | 2 + src/qemu/qemu_process.c | 34 +++++++++++++++++ src/qemu/qemu_snapshot.c | 4 ++ src/qemu/qemu_validate.c | 33 +++++++++++++++-- src/storage_file/storage_source.c | 1 + .../caps_8.1.0_x86_64.xml | 1 + tests/qemuhotplugmock.c | 4 +- .../disk-vhostvdpa.x86_64-latest.args | 37 +++++++++++++++++++ tests/qemuxml2argvdata/disk-vhostvdpa.xml | 21 +++++++++++ tests/qemuxml2argvmock.c | 2 +- tests/qemuxml2argvtest.c | 34 +++++++++++++++++ tests/testutilsqemu.c | 11 ++++++ tests/testutilsqemu.h | 2 + 28 files changed, 285 insertions(+), 37 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-vhostvdpa.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-vhostvdpa.xml -- 2.41.0

4 11

[libvirt PATCH v2 00/35] ci: Unify the GitLab CI jobs with local executions && adopt lcitool container executions
by Erik Skultety 13 Sep '23

13 Sep '23

since v2 [1]: - moved the custom meson.build directory check before running setup to individual callers for better robustness, but actually let meson deal with an existing setup directory most of the time - drops the ci/Makefile - only set our verbose env variables when running without a TTY - switched the bright yellow (93m) color to green (32m) - added KeyboardInterrupt handling in the ci/helper Python code The changes above require the following lcitool MR to be merged first as they're built on top of it [2] [1] https://listman.redhat.com/archives/libvir-list/2023-August/241471.html [2] https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/431 Erik Skultety (35): ci: build.sh: Add variables from .gitlab-ci.yml ci: build.sh: Add GIT_ROOT env helper variable ci: build.sh: Don't mention that MESON_ARGS are available via CLI ci: build.sh: Add a wrapper function executing 'shell' commands ci: build.sh: Add a wrapper function over meson's setup ci: build.sh: Add a wrapper function over the 'build' job ci: build.sh: Add a helper function to create the dist tarball ci: build.sh: Add a wrapper function over the 'test' job ci: build.sh: Add a wrapper function over the 'codestyle' job ci: build.sh: Add a wrapper function over the 'potfile' job ci: build.sh: Add a wrapper function over the 'rpmbuild' job ci: build.sh: Add a wrapper function over the 'website' job ci: build.sh: Drop changing working directory to CI_CONT_DIR ci: build.sh: Drop direct invocation of meson/ninja commands ci: build.sh: Drop MESON_ARGS definition from global level ci: Rename build.sh -> jobs.sh .gitlab-ci.yml: Add 'after_script' stage to prep for artifact collection .gitlab-ci.yml: Convert the native build job to the build.sh usage .gitlab-ci.yml: Convert the cross build job to the build.sh usage .gitlab-ci.yml: Convert the website build job to the build.sh usage .gitlab-ci.yml: Convert the codestyle job to the build.sh usage .gitlab-ci.yml: Convert the potfile job to the build.sh usage ci: helper: Drop _lcitool_get_targets method ci: helper: Don't make ':' literal a static part of the image tag ci: helper: Add --lcitool-path CLI option ci: helper: Add a required_deps higher order helper/decorator ci: helper: Add Python code hangling git clones ci: helper: Add a helper to create a local repo clone Pythonic way ci: helper: Rework _lcitool_run method logic ci: helper: Add an action to run the container workload via lcitool ci: helper: Add a job argparse subparser ci: helper: Drop original actions ci: helper: Drop the --meson-args/--ninja-args CLI options ci: helper: Drop the _make_run method ci: Drop the now unused Makefile .gitlab-ci.yml | 47 +++++----- ci/Makefile | 245 ------------------------------------------------- ci/build.sh | 23 ----- ci/helper | 181 +++++++++++++++++++++--------------- ci/jobs.sh | 79 ++++++++++++++++ 5 files changed, 211 insertions(+), 364 deletions(-) delete mode 100644 ci/Makefile delete mode 100644 ci/build.sh create mode 100644 ci/jobs.sh -- 2.41.0

2 50

[PATCH (pushed)] virStorageSourceClear: Clear 'vdpadev' field
by Peter Krempa 13 Sep '23

13 Sep '23

Release memory for the recently added field. Fixes: 1df106cc20a4cc6417cfbaf01860f465ec3dd915 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/conf/storage_source_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c index f57cb3241d..f7f62c3966 100644 --- a/src/conf/storage_source_conf.c +++ b/src/conf/storage_source_conf.c @@ -1143,6 +1143,7 @@ virStorageSourceClear(virStorageSource *def) VIR_FREE(def->path); VIR_FREE(def->fdgroup); VIR_FREE(def->volume); + VIR_FREE(def->vdpadev); VIR_FREE(def->snapshot); VIR_FREE(def->configFile); VIR_FREE(def->query); -- 2.41.0

1 0

[PATCH 0/2] Check for capng_*() retvals
by Michal Privoznik 11 Sep '23

11 Sep '23

There's a commit inside of (yet-unreleased) libcap-ng which marks some functions as 'warned unused result'. Fedora rawhide already picked up the commit, but since we are not checking for all retvals we got a build failure on rawhide. https://src.fedoraproject.org/rpms/libcap-ng/c/fed9b23c8d0020e07c937a3ac0d6… Green pipeline: https://gitlab.com/MichalPrivoznik/libvirt/-/pipelines/999435445 Michal Prívozník (2): lxc_container: Check retval of capng_get_caps_process() virutil: Check retval of capng_apply() src/lxc/lxc_container.c | 8 +++++++- src/util/virutil.c | 8 ++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) -- 2.41.0

3 4

[libvirt PATCH v4] ch: Fix cloud-hypervisor version processing
by Praveen K Paladugu 11 Sep '23

11 Sep '23

Refactor the version processing logic in ch driver to support versions from non-release cloud-hypervisor binaries. This version also supports versions with branch prefixes in them. Signed-off-by: Praveen K Paladugu <prapal(a)linux.microsoft.com> --- src/ch/ch_conf.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/src/ch/ch_conf.c b/src/ch/ch_conf.c index a8565d9537..f421af5121 100644 --- a/src/ch/ch_conf.c +++ b/src/ch/ch_conf.c @@ -172,6 +172,29 @@ virCHDriverConfigDispose(void *obj) #define MIN_VERSION ((15 * 1000000) + (0 * 1000) + (0)) +/** + * chPreProcessVersionString: + * + * Returns: a pointer to numerical version without branch/commit info + */ +static char * +chPreProcessVersionString(char *version) +{ + char *tmp = strrchr(version, '/'); + + if (tmp) + version = tmp + 1; + + if (version[0] == 'v') + version++; + + tmp = strchr(version, '-'); + if (tmp) + *tmp = '\0'; + + return version; +} + int chExtractVersion(virCHDriver *driver) { @@ -193,13 +216,20 @@ chExtractVersion(virCHDriver *driver) tmp = help; - /* expected format: cloud-hypervisor v<major>.<minor>.<micro> */ - if ((tmp = STRSKIP(tmp, "cloud-hypervisor v")) == NULL) { + /* Below are example version formats and expected outputs: + * cloud-hypervisor v32.0.0 (expected: 32.0.0) + * cloud-hypervisor v33.0-104-ge0e3779e-dirty (expected: 33.0) + * cloud-hypervisor testing/v32.0.131-1-ga5d6db5c-dirty (expected: 32.0.131) + */ + if ((tmp = STRSKIP(tmp, "cloud-hypervisor ")) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unexpected output of cloud-hypervisor binary")); return -1; } + tmp = chPreProcessVersionString(tmp); + VIR_DEBUG("Cloud-Hypervisor version detected: %s", tmp); + if (virStringParseVersion(&version, tmp, true) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse cloud-hypervisor version: %1$s"), tmp); -- 2.41.0

2 1

[PATCH v2] interface: fix udev_device_get_sysattr_value return value check
by Dmitry Frolov 11 Sep '23

11 Sep '23

Reviewing the code I found that return value of function udev_device_get_sysattr_value() is dereferenced without a check. udev_device_get_sysattr_value() may return NULL by number of reasons. v2: VIR_DEBUG added, replaced STREQ(NULLSTR()) with STREQ_NULLABLE() Signed-off-by: Dmitry Frolov <frolov(a)swemel.ru> --- src/interface/interface_backend_udev.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index a0485ddd21..df7066727e 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -23,6 +23,7 @@ #include <dirent.h> #include <libudev.h> +#include "virlog.h" #include "virerror.h" #include "virfile.h" #include "datatypes.h" @@ -40,6 +41,8 @@ #define VIR_FROM_THIS VIR_FROM_INTERFACE +VIR_LOG_INIT("interface.interface_backend_udev"); + struct udev_iface_driver { struct udev *udev; /* pid file FD, ensures two copies of the driver can't use the same root */ @@ -355,10 +358,13 @@ udevConnectListAllInterfaces(virConnectPtr conn, g_autoptr(virInterfaceDef) def = NULL; path = udev_list_entry_get_name(dev_entry); - dev = udev_device_new_from_syspath(udev, path); + if (!(dev = udev_device_new_from_syspath(udev, path))) { + VIR_DEBUG("Skipping interface '%s'", NULLSTR(path)); + continue; + } name = udev_device_get_sysname(dev); macaddr = udev_device_get_sysattr_value(dev, "address"); - status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up"); + status = STREQ_NULLABLE(udev_device_get_sysattr_value(dev, "operstate"), "up"); def = udevGetMinimalDefForDevice(dev); if (!virConnectListAllInterfacesCheckACL(conn, def)) { @@ -964,9 +970,9 @@ udevGetIfaceDef(struct udev *udev, const char *name) /* MTU */ mtu_str = udev_device_get_sysattr_value(dev, "mtu"); - if (virStrToLong_ui(mtu_str, NULL, 10, &mtu) < 0) { + if (!mtu_str || virStrToLong_ui(mtu_str, NULL, 10, &mtu) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not parse MTU value '%1$s'"), mtu_str); + _("Could not parse MTU value '%1$s'"), NULLSTR(mtu_str)); goto error; } ifacedef->mtu = mtu; @@ -1089,7 +1095,7 @@ udevInterfaceIsActive(virInterfacePtr ifinfo) goto cleanup; /* Check if it's active or not */ - status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up"); + status = STREQ_NULLABLE(udev_device_get_sysattr_value(dev, "operstate"), "up"); udev_device_unref(dev); -- 2.34.1

2 1

[libvirt PATCH 0/5] Add support for vDPA block devices
by Jonathon Jongsma 09 Sep '23

09 Sep '23

see https://bugzilla.redhat.com/show_bug.cgi?id=1900770. Jonathon Jongsma (5): conf: add ability to configure a vdpa block disk device qemu: add virtio-blk-vhost-vdpa capability qemu: make vdpa connect function more generic qemu: consider vdpa block devices for memlock limits qemu: Implement support for vDPA block devices docs/formatdomain.rst | 19 ++++++++- src/ch/ch_monitor.c | 1 + src/conf/domain_conf.c | 7 ++++ src/conf/schemas/domaincommon.rng | 13 +++++++ src/conf/storage_source_conf.c | 6 ++- src/conf/storage_source_conf.h | 1 + src/libxl/xen_xl.c | 1 + src/qemu/qemu_block.c | 20 ++++++++++ src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 24 +++++++++++- src/qemu/qemu_command.h | 1 + src/qemu/qemu_domain.c | 37 +++++++++++++++++- src/qemu/qemu_interface.c | 23 ----------- src/qemu/qemu_interface.h | 2 - src/qemu/qemu_migration.c | 2 + src/qemu/qemu_snapshot.c | 4 ++ src/qemu/qemu_validate.c | 45 +++++++++++++++++++--- src/storage_file/storage_source.c | 1 + tests/qemuhotplugmock.c | 4 +- tests/qemuxml2argvdata/disk-vhostvdpa.args | 35 +++++++++++++++++ tests/qemuxml2argvdata/disk-vhostvdpa.xml | 21 ++++++++++ tests/qemuxml2argvmock.c | 2 +- tests/qemuxml2argvtest.c | 2 + 24 files changed, 235 insertions(+), 39 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-vhostvdpa.args create mode 100644 tests/qemuxml2argvdata/disk-vhostvdpa.xml -- 2.40.1

3 22

[PATCH] interface: fix udev_device_get_sysattr_value return value check
by Dmitry Frolov 08 Sep '23

08 Sep '23

Reviewing the code I found that return value of function udev_device_get_sysattr_value() is dereferenced without a check. udev_device_get_sysattr_value() may return NULL by number of reasons. Signed-off-by: Dmitry Frolov <frolov(a)swemel.ru> --- src/interface/interface_backend_udev.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c index a0485ddd21..c820b3ccdf 100644 --- a/src/interface/interface_backend_udev.c +++ b/src/interface/interface_backend_udev.c @@ -355,10 +355,11 @@ udevConnectListAllInterfaces(virConnectPtr conn, g_autoptr(virInterfaceDef) def = NULL; path = udev_list_entry_get_name(dev_entry); - dev = udev_device_new_from_syspath(udev, path); + if (!(dev = udev_device_new_from_syspath(udev, path))) + continue; name = udev_device_get_sysname(dev); macaddr = udev_device_get_sysattr_value(dev, "address"); - status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up"); + status = STREQ(NULLSTR(udev_device_get_sysattr_value(dev, "operstate")), "up"); def = udevGetMinimalDefForDevice(dev); if (!virConnectListAllInterfacesCheckACL(conn, def)) { @@ -964,9 +965,9 @@ udevGetIfaceDef(struct udev *udev, const char *name) /* MTU */ mtu_str = udev_device_get_sysattr_value(dev, "mtu"); - if (virStrToLong_ui(mtu_str, NULL, 10, &mtu) < 0) { + if (!mtu_str || virStrToLong_ui(mtu_str, NULL, 10, &mtu) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Could not parse MTU value '%1$s'"), mtu_str); + _("Could not parse MTU value '%1$s'"), NULLSTR(mtu_str)); goto error; } ifacedef->mtu = mtu; @@ -1089,7 +1090,7 @@ udevInterfaceIsActive(virInterfacePtr ifinfo) goto cleanup; /* Check if it's active or not */ - status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up"); + status = STREQ(NULLSTR(udev_device_get_sysattr_value(dev, "operstate")), "up"); udev_device_unref(dev); -- 2.34.1

2 1

[PATCH] virnetdevopenvswitch: Propagate OVS error messages
by Michal Privoznik 08 Sep '23

08 Sep '23

When configuring OVS interfaces/bridges we spawn 'ovs-vsctl' with appropriate arguments and if it exited with a non-zero status we report a generic error message, like "Unable to add port vnet0 to OVS bridge ovsbr0". This is all cool, but the real reason why operation failed is hidden in (debug) logs because that's where virCommandRun() reports it unless caller requested otherwise. This is a bit clumsy because then we have to ask users to turn on debug logs and reproduce the problem again, e.g. [1]. Therefore, in cases where an error is reported to the user - just read ovs-vsctl's stderr and include it in the error message. For other cases (like VIR_DEBUG/VIR_WARN) - well they are meant to end up in (debug) logs anyway. 1: https://mail.openvswitch.org/pipermail/ovs-discuss/2023-September/052640.ht… Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/util/virnetdevopenvswitch.c | 93 ++++++++++++++++++++------------- 1 file changed, 58 insertions(+), 35 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 8dad6ed2bd..d836d05845 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -54,10 +54,14 @@ virNetDevOpenvswitchSetTimeout(unsigned int timeout) } static virCommand * -virNetDevOpenvswitchCreateCmd(void) +virNetDevOpenvswitchCreateCmd(char **errbuf) { virCommand *cmd = virCommandNew(OVS_VSCTL); + virCommandAddArgFormat(cmd, "--timeout=%u", virNetDevOpenvswitchTimeout); + if (errbuf) + virCommandSetErrorBuffer(cmd, errbuf); + return cmd; } @@ -137,6 +141,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, char ifuuidstr[VIR_UUID_STRING_BUFLEN]; char vmuuidstr[VIR_UUID_STRING_BUFLEN]; g_autoptr(virCommand) cmd = NULL; + g_autofree char *errbuf = NULL; g_autofree char *attachedmac_ex_id = NULL; g_autofree char *ifaceid_ex_id = NULL; g_autofree char *profile_ex_id = NULL; @@ -157,7 +162,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, ovsport->profileID); } - cmd = virNetDevOpenvswitchCreateCmd(); + cmd = virNetDevOpenvswitchCreateCmd(&errbuf); virCommandAddArgList(cmd, "--", "--may-exist", "add-port", brname, ifname, NULL); @@ -185,8 +190,8 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to add port %1$s to OVS bridge %2$s"), - ifname, brname); + _("Unable to add port %1$s to OVS bridge %2$s: %3$s"), + ifname, brname, NULLSTR(errbuf)); return -1; } @@ -203,13 +208,15 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, */ int virNetDevOpenvswitchRemovePort(const char *brname G_GNUC_UNUSED, const char *ifname) { - g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(); + g_autofree char *errbuf = NULL; + g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(&errbuf); virCommandAddArgList(cmd, "--", "--if-exists", "del-port", ifname, NULL); if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to delete port %1$s from OVS"), ifname); + _("Unable to delete port %1$s from OVS: %2$s"), + ifname, NULLSTR(errbuf)); return -1; } @@ -228,7 +235,8 @@ int virNetDevOpenvswitchRemovePort(const char *brname G_GNUC_UNUSED, const char int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname) { size_t len; - g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(); + g_autofree char *errbuf = NULL; + g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(&errbuf); virCommandAddArgList(cmd, "--if-exists", "get", "Interface", ifname, "external_ids:PortData", NULL); @@ -238,8 +246,8 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname) /* Run the command */ if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to run command to get OVS port data for interface %1$s"), - ifname); + _("Unable to run command to get OVS port data for interface %1$s: %2$s"), + ifname, NULLSTR(errbuf)); return -1; } @@ -263,21 +271,22 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname) int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname) { g_autoptr(virCommand) cmd = NULL; + g_autofree char *errbuf = NULL; if (!migrate) { VIR_DEBUG("No OVS port data for interface %s", ifname); return 0; } - cmd = virNetDevOpenvswitchCreateCmd(); + cmd = virNetDevOpenvswitchCreateCmd(&errbuf); virCommandAddArgList(cmd, "set", "Interface", ifname, NULL); virCommandAddArgFormat(cmd, "external_ids:PortData=%s", migrate); /* Run the command */ if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to run command to set OVS port data for interface %1$s"), - ifname); + _("Unable to run command to set OVS port data for interface %1$s: %2$s"), + ifname, NULLSTR(errbuf)); return -1; } @@ -371,7 +380,8 @@ int virNetDevOpenvswitchInterfaceStats(const char *ifname, virDomainInterfaceStatsPtr stats) { - g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(); + g_autofree char *errbuf = NULL; + g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(&errbuf); g_autofree char *output = NULL; virCommandAddArgList(cmd, "--if-exists", "--format=list", "--data=json", @@ -390,8 +400,9 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname, if (virCommandRun(cmd, NULL) < 0 || STREQ_NULLABLE(output, "")) { /* no ovs-vsctl or interface 'ifname' doesn't exists in ovs */ - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Interface not found")); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Interface not found: %1$s"), + NULLSTR(errbuf)); return -1; } @@ -433,7 +444,8 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname, int virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master) { - g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(); + g_autofree char *errbuf = NULL; + g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(&errbuf); int exitstatus; *master = NULL; @@ -443,8 +455,8 @@ virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master) if (virCommandRun(cmd, &exitstatus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to run command to get OVS master for interface %1$s"), - ifname); + _("Unable to run command to get OVS master for interface %1$s: %2$s"), + ifname, NULLSTR(errbuf)); return -1; } @@ -546,7 +558,7 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path, return 0; } - cmd = virNetDevOpenvswitchCreateCmd(); + cmd = virNetDevOpenvswitchCreateCmd(NULL); if (server) { virCommandAddArgList(cmd, "--no-headings", "--columns=name", "find", @@ -600,7 +612,8 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path, int virNetDevOpenvswitchUpdateVlan(const char *ifname, const virNetDevVlan *virtVlan) { - g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(); + g_autofree char *errbuf = NULL; + g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(&errbuf); virCommandAddArgList(cmd, "--", "--if-exists", "clear", "Port", ifname, "tag", @@ -614,7 +627,8 @@ int virNetDevOpenvswitchUpdateVlan(const char *ifname, if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to set vlan configuration on port %1$s"), ifname); + _("Unable to set vlan configuration on port %1$s: %2$s"), + ifname, NULLSTR(errbuf)); return -1; } @@ -629,7 +643,7 @@ virNetDevOpenvswitchFindUUID(const char *table, g_autoptr(virCommand) cmd = NULL; char *uuid = NULL; - cmd = virNetDevOpenvswitchCreateCmd(); + cmd = virNetDevOpenvswitchCreateCmd(NULL); virCommandAddArgList(cmd, "--no-heading", "--columns=_uuid", "find", table, vmid_ex_id, ifname_ex_id, NULL); virCommandSetOutputBuffer(cmd, &uuid); @@ -673,7 +687,7 @@ virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname, if (!*line) { continue; } - listcmd = virNetDevOpenvswitchCreateCmd(); + listcmd = virNetDevOpenvswitchCreateCmd(NULL); virCommandAddArgList(listcmd, "--no-heading", "--columns=_uuid", "--if-exists", "list", "port", ifname, "qos", NULL); virCommandSetOutputBuffer(listcmd, &port_qos); @@ -681,13 +695,13 @@ virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname, VIR_WARN("Unable to remove port qos on port %s", ifname); } if (port_qos && *port_qos) { - g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(); + g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(NULL); virCommandAddArgList(cmd, "remove", "port", ifname, "qos", line, NULL); if (virCommandRun(cmd, NULL) < 0) { VIR_WARN("Unable to remove port qos on port %s", ifname); } } - destroycmd = virNetDevOpenvswitchCreateCmd(); + destroycmd = virNetDevOpenvswitchCreateCmd(NULL); virCommandAddArgList(destroycmd, "destroy", "qos", line, NULL); if (virCommandRun(destroycmd, NULL) < 0) { VIR_WARN("Unable to destroy qos on port %s", ifname); @@ -706,7 +720,7 @@ virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname, continue; } - cmd = virNetDevOpenvswitchCreateCmd(); + cmd = virNetDevOpenvswitchCreateCmd(NULL); virCommandAddArgList(cmd, "destroy", "queue", line, NULL); if (virCommandRun(cmd, NULL) < 0) { VIR_WARN("Unable to destroy queue on port %s", ifname); @@ -722,15 +736,17 @@ static int virNetDevOpenvswitchInterfaceClearRxQos(const char *ifname) { g_autoptr(virCommand) cmd = NULL; + g_autofree char *errbuf = NULL; - cmd = virNetDevOpenvswitchCreateCmd(); + cmd = virNetDevOpenvswitchCreateCmd(&errbuf); virCommandAddArgList(cmd, "set", "Interface", ifname, NULL); virCommandAddArgFormat(cmd, "ingress_policing_rate=%llu", 0llu); virCommandAddArgFormat(cmd, "ingress_policing_burst=%llu", 0llu); if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to reset ingress on port %1$s"), ifname); + _("Unable to reset ingress on port %1$s: %2$s"), + ifname, NULLSTR(errbuf)); return -1; } @@ -754,6 +770,7 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifname, { char vmuuidstr[VIR_UUID_STRING_BUFLEN]; g_autoptr(virCommand) cmd = NULL; + g_autofree char *errbuf = NULL; g_autofree char *vmid_ex_id = NULL; g_autofree char *ifname_ex_id = NULL; g_autofree char *average = NULL; @@ -777,7 +794,7 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifname, qos_uuid = virNetDevOpenvswitchFindUUID("qos", vmid_ex_id, ifname_ex_id); /* create qos and set */ - cmd = virNetDevOpenvswitchCreateCmd(); + cmd = virNetDevOpenvswitchCreateCmd(&errbuf); if (queue_uuid && *queue_uuid) { g_auto(GStrv) lines = g_strsplit(queue_uuid, "\n", 0); virCommandAddArgList(cmd, "set", "queue", lines[0], NULL); @@ -806,17 +823,20 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifname, if (virCommandRun(cmd, NULL) < 0) { if (queue_uuid && *queue_uuid) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to set queue configuration on port %1$s"), ifname); + _("Unable to set queue configuration on port %1$s: %2$s"), + ifname, NULLSTR(errbuf)); } else { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to create and set qos configuration on port %1$s"), ifname); + _("Unable to create and set qos configuration on port %1$s: %2$s"), + ifname, NULLSTR(errbuf)); } return -1; } if (qos_uuid && *qos_uuid) { g_auto(GStrv) lines = g_strsplit(qos_uuid, "\n", 0); - g_autoptr(virCommand) qoscmd = virNetDevOpenvswitchCreateCmd(); + g_autofree char *qoserrbuf = NULL; + g_autoptr(virCommand) qoscmd = virNetDevOpenvswitchCreateCmd(&qoserrbuf); virCommandAddArgList(qoscmd, "set", "qos", lines[0], NULL); virCommandAddArgFormat(qoscmd, "other_config:min-rate=%s", average); @@ -829,7 +849,8 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifname, virCommandAddArgList(qoscmd, vmid_ex_id, ifname_ex_id, NULL); if (virCommandRun(qoscmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to set qos configuration on port %1$s"), ifname); + _("Unable to set qos configuration on port %1$s: %2$s"), + ifname, NULLSTR(qoserrbuf)); return -1; } } @@ -842,8 +863,9 @@ virNetDevOpenvswitchInterfaceSetRxQos(const char *ifname, const virNetDevBandwidthRate *rx) { g_autoptr(virCommand) cmd = NULL; + g_autofree char *errbuf = NULL; - cmd = virNetDevOpenvswitchCreateCmd(); + cmd = virNetDevOpenvswitchCreateCmd(&errbuf); virCommandAddArgList(cmd, "set", "Interface", ifname, NULL); virCommandAddArgFormat(cmd, "ingress_policing_rate=%llu", rx->average * VIR_NETDEV_RX_TO_OVS); @@ -853,7 +875,8 @@ virNetDevOpenvswitchInterfaceSetRxQos(const char *ifname, if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to set vlan configuration on port %1$s"), ifname); + _("Unable to set vlan configuration on port %1$s: %2$s"), + ifname, NULLSTR(errbuf)); return -1; } -- 2.41.0

2 1

Error : virHostCPUGetKVMMaxVCPUs:1228 : KVM is not supported on this platform: Function not implemented.
by Mario Marietto 08 Sep '23

08 Sep '23

Hello. I'm running Debian bookworm on my ARM Chromebook,model "xe303c12" and I've recompiled the kernel (5.4) to enable KVM,so now my system sounds like this : $ lsb_release -a No LSB modules are available. Distributor ID: Debian Description: Debian GNU/Linux 12 (bookworm) Release: 12 Codename: bookworm $ uname -a Linux chromarietto 5.4.244-stb-cbe #8 SMP PREEMPT Sat Aug 19 22:19:32 UTC 2023 armv7l GNU/Linux $ uname -r 5.4.244-stb-cbe $ kvm-ok INFO: /dev/kvm exists KVM acceleration can be used $ qemu-system-arm --version QEMU emulator version 5.1.0 (v5.1.0-dirty) Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers $ python3 --version Python 3.11.2 I have installed libvirt 9.7.0,qemu 5.1 and virt-manager from source code with the final goal to be able to connect qemu,kvm and libvirt together to virtualize FreeBSD 13.2 for arm 32 bit. Some useful informations about my platform : root@chromarietto:/home/marietto/Desktop# virt-host-validate QEMU: Checking if device /dev/kvm exists : PASS QEMU: Checking if device /dev/kvm is accessible : PASS QEMU: Checking if device /dev/vhost-net exists : PASS QEMU: Checking if device /dev/net/tun exists : PASS QEMU: Checking for cgroup 'cpu' controller support : PASS QEMU: Checking for cgroup 'cpuacct' controller support : PASS QEMU: Checking for cgroup 'cpuset' controller support : PASS QEMU: Checking for cgroup 'memory' controller support : PASS QEMU: Checking for cgroup 'devices' controller support : PASS QEMU: Checking for cgroup 'blkio' controller support : PASS QEMU: Checking for device assignment IOMMU support : WARN (No ACPI IORT table found, IOMMU not supported by this hardware platform) QEMU: Checking for secure guest support : WARN (Unknown if this platform has Secure Guest support) LXC: Checking for Linux >= 2.6.26 : PASS LXC: Checking for namespace ipc : PASS LXC: Checking for namespace mnt : PASS LXC: Checking for namespace pid : PASS LXC: Checking for namespace uts : PASS LXC: Checking for namespace net : PASS LXC: Checking for namespace user : PASS LXC: Checking for cgroup 'cpu' controller support : PASS LXC: Checking for cgroup 'cpuacct' controller support : PASS LXC: Checking for cgroup 'cpuset' controller support : PASS LXC: Checking for cgroup 'memory' controller support : PASS LXC: Checking for cgroup 'devices' controller support : PASS LXC: Checking for cgroup 'freezer' controller support : FAIL (Enable 'freezer' in kernel Kconfig file or mount/enable cgroup controller in your system) LXC: Checking for cgroup 'blkio' controller support : PASS LXC: Checking if device /sys/fs/fuse/connections exists : PASS # lsmod | grep kvm no errors (I have embedded the options needed to enable KVM inside the kernel) # virsh --connect qemu:///system capabilities | grep baselabel <baselabel type='kvm'>+1002:+1002</baselabel> <baselabel type='qemu'>+1002:+1002</baselabel> The error that I'm not able to fix is the following one : root@chromarietto:~# virsh domcapabilities --machine virt --emulatorbin /usr/local/bin/qemu-system-arm 2023-08-29 10:17:59.110+0000: 1763: error : virHostCPUGetKVMMaxVCPUs:1228 : KVM is not supported on this platform: Function not implemented ; error: failed to get emulator capabilities error: KVM is not supported on this platform: Function not implemented and this is the log that I've got when I ran libvirtd with the debug option enabled root@chromarietto:~# libvirtd --debug [Tue, 29 Aug 2023 10:10:11 virt-manager 2141] DEBUG (createvm:494) UEFI found, setting it as default. [Tue, 29 Aug 2023 10:10:11 virt-manager 2141] DEBUG (createvm:728) Guest type set to os_type=hvm, arch=armv7l, dom_type=kvm [Tue, 29 Aug 2023 10:10:11 virt-manager 2141] DEBUG (guest:546) Prefer EFI => True 2023-08-29 10:10:12.972+0000: 1765: error : virHostCPUGetKVMMaxVCPUs:1228 : KVM is not supported on this platform: Function not implemented [Tue, 29 Aug 2023 10:10:12 virt-manager 2141] DEBUG (domcapabilities:250) Error fetching domcapabilities XML Traceback (most recent call last): File "/usr/local/share/virt-manager/virtinst/domcapabilities.py", line 245, in build_from_params xml = conn.getDomainCapabilities(emulator, arch, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3/dist-packages/libvirt.py", line 4612, in getDomainCapabilities raise libvirtError('virConnectGetDomainCapabilities() failed') libvirt.libvirtError: KVM is not supported on this platform: Function not implemented *2023-08-29 10:10:14.157+0000: 1762: error : virHostCPUGetKVMMaxVCPUs:1228 : KVM is not supported on this platform: Function not implemented* [Tue, 29 Aug 2023 10:10:14 virt-manager 2141] DEBUG (domcapabilities:250) Error fetching domcapabilities XML Traceback (most recent call last): File "/usr/local/share/virt-manager/virtinst/domcapabilities.py", line 245, in build_from_params xml = conn.getDomainCapabilities(emulator, arch, ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3/dist-packages/libvirt.py", line 4612, in getDomainCapabilities raise libvirtError('virConnectGetDomainCapabilities() failed') libvirt.libvirtError: KVM is not supported on this platform: Function not implemented [Tue, 29 Aug 2023 10:10:14 virt-manager 2141] DEBUG (createvm:497) Error checking for UEFI default Traceback (most recent call last): File "/usr/local/share/virt-manager/virtManager/createvm.py", line 491, in _set_caps_state guest.enable_uefi() File "/usr/local/share/virt-manager/virtinst/guest.py", line 589, in enable_uefi path = self._lookup_default_uefi_path() ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/share/virt-manager/virtinst/guest.py", line 848, in _lookup_default_uefi_path raise RuntimeError(_("Libvirt version does not support UEFI.")) RuntimeError: Libvirt version does not support UEFI Does anyone know how to fix that error ? -- Mario.

3 6

[libvirt PATCH v3] ch: Fix cloud-hypervisor version processing
by Praveen K Paladugu 08 Sep '23

08 Sep '23

Refactor the version processing logic in ch driver to support versions from non-release cloud-hypervisor binaries. This version also supports versions with branch prefixes in them. Signed-off-by: Praveen K Paladugu <prapal(a)linux.microsoft.com> --- src/ch/ch_conf.c | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/src/ch/ch_conf.c b/src/ch/ch_conf.c index a8565d9537..5573119b23 100644 --- a/src/ch/ch_conf.c +++ b/src/ch/ch_conf.c @@ -172,6 +172,37 @@ virCHDriverConfigDispose(void *obj) #define MIN_VERSION ((15 * 1000000) + (0 * 1000) + (0)) +/** + * chPreProcessVersionString: + * + * Returns: a pointer to numerical version without branch/commit info + */ +static char * +chPreProcessVersionString(char *version) +{ + char *tmp_version = version; + g_autofree char *ret_version = NULL; + char *sub_string; + + if ((sub_string = strrchr(version, '/')) != NULL) { + tmp_version = sub_string + 1; + } + + if (tmp_version[0] == 'v') { + tmp_version = tmp_version + 1; + } + + // Duplicate the string in both cases. This would allow the calling method + // free the returned string in a consistent manner. + if ((sub_string = strchr(tmp_version, '-')) != NULL) { + ret_version = g_strndup(tmp_version, sub_string - tmp_version); + } else{ + ret_version = g_strdup(tmp_version); + } + + return g_steal_pointer(&ret_version); + +} int chExtractVersion(virCHDriver *driver) { @@ -193,13 +224,20 @@ chExtractVersion(virCHDriver *driver) tmp = help; - /* expected format: cloud-hypervisor v<major>.<minor>.<micro> */ - if ((tmp = STRSKIP(tmp, "cloud-hypervisor v")) == NULL) { + /* Below are some example version formats and expected outputs: + * cloud-hypervisor v32.0.0 (expected: 32.0.0) + * cloud-hypervisor v33.0-104-ge0e3779e-dirty (expected: 33.0) + * cloud-hypervisor testing/v32.0.131-1-ga5d6db5c-dirty (expected: 32.0.131) + */ + if ((tmp = STRSKIP(tmp, "cloud-hypervisor ")) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unexpected output of cloud-hypervisor binary")); return -1; } + tmp = chPreProcessVersionString(tmp); + VIR_DEBUG("Cloud-Hypervisor version detected: %s", tmp); + if (virStringParseVersion(&version, tmp, true) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse cloud-hypervisor version: %1$s"), tmp); -- 2.41.0

3 2

Re: [libvirt PATCH v3] ch: Fix cloud-hypervisor version processing
by Praveen Paladugu 08 Sep '23

08 Sep '23

On Thu, Sep 07, 2023 at 09:02:44AM +0200, Jiri Denemark wrote: > On Wed, Sep 06, 2023 at 10:50:30 -0500, Praveen K Paladugu wrote: > > Refactor the version processing logic in ch driver to support versions > > from non-release cloud-hypervisor binaries. This version also supports > > versions with branch prefixes in them. > > > > Signed-off-by: Praveen K Paladugu <prapal(a)linux.microsoft.com> > > --- > > src/ch/ch_conf.c | 42 ++++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 40 insertions(+), 2 deletions(-) > > > > diff --git a/src/ch/ch_conf.c b/src/ch/ch_conf.c > > index a8565d9537..5573119b23 100644 > > --- a/src/ch/ch_conf.c > > +++ b/src/ch/ch_conf.c > > @@ -172,6 +172,37 @@ virCHDriverConfigDispose(void *obj) > > > > #define MIN_VERSION ((15 * 1000000) + (0 * 1000) + (0)) > > > > +/** > > + * chPreProcessVersionString: > > + * > > + * Returns: a pointer to numerical version without branch/commit info > > + */ > > +static char * > > +chPreProcessVersionString(char *version) > > +{ > > + char *tmp_version = version; > > + g_autofree char *ret_version = NULL; > > As Daniel pointed out in previous version this g_autofree and > g_steal_pointer below are useless as there is no path that actually free > the string here. > Thanks Jirka, I expected "autofree" attribute to be carried to "tmp" in the calling method, which would be freed when tmp goes out of scope. So, I implemented it this way. With some experimentation, I confirmed the correct behavior. I will fix this in my next version. > > + char *sub_string; > > + > > + if ((sub_string = strrchr(version, '/')) != NULL) { > > + tmp_version = sub_string + 1; > > + } > > + > > + if (tmp_version[0] == 'v') { > > + tmp_version = tmp_version + 1; > > + } > > + > > + // Duplicate the string in both cases. This would allow the calling method > > + // free the returned string in a consistent manner. > > Our coding style document says we prefer /* ... */ comments. > > > + if ((sub_string = strchr(tmp_version, '-')) != NULL) { > > + ret_version = g_strndup(tmp_version, sub_string - tmp_version); > > + } else{ > > + ret_version = g_strdup(tmp_version); > > + } > > + > > + return g_steal_pointer(&ret_version); > > + > > +} > > int > > chExtractVersion(virCHDriver *driver) > > { > > @@ -193,13 +224,20 @@ chExtractVersion(virCHDriver *driver) > > > > tmp = help; > > > > - /* expected format: cloud-hypervisor v<major>.<minor>.<micro> */ > > - if ((tmp = STRSKIP(tmp, "cloud-hypervisor v")) == NULL) { > > + /* Below are some example version formats and expected outputs: > > + * cloud-hypervisor v32.0.0 (expected: 32.0.0) > > + * cloud-hypervisor v33.0-104-ge0e3779e-dirty (expected: 33.0) > > + * cloud-hypervisor testing/v32.0.131-1-ga5d6db5c-dirty (expected: 32.0.131) > > + */ > > + if ((tmp = STRSKIP(tmp, "cloud-hypervisor ")) == NULL) { > > virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > > _("Unexpected output of cloud-hypervisor binary")); > > return -1; > > } > > > > + tmp = chPreProcessVersionString(tmp); > > So chPreProcessVersionString returns an allocated string that will never > be freed because you assign it to tmp that's only used to point inside > other buffers and thus is (correctly) not freed anywhere. You need to > make sure the pointer returned here is freed before chExtractVersion > exits. Understood. > > > + VIR_DEBUG("Cloud-Hypervisor version detected: %s", tmp); > > + > > if (virStringParseVersion(&version, tmp, true) < 0) { > > virReportError(VIR_ERR_INTERNAL_ERROR, > > _("Unable to parse cloud-hypervisor version: %1$s"), tmp); > > Jirka

1 0