September 2021 - Devel - libvirt List Archives

[PATCH 0/4] Various cleanups
by Peter Krempa 20 Sep '21

20 Sep '21

Few patches from old branches. Peter Krempa (4): util: virdevmapper: Sanitize use of macros for buffer size virDevMapperGetTargets: Use a linked list as return type util: virstring: Remove unused 'virStringListMerge' qemuMonitorJSONGetStatus: Refactor cleanup src/libvirt_private.syms | 1 - src/qemu/qemu_cgroup.c | 11 ++++---- src/qemu/qemu_monitor_json.c | 18 +++++-------- src/qemu/qemu_namespace.c | 9 +++---- src/util/virdevmapper.c | 49 +++++++++++++++++------------------- src/util/virdevmapper.h | 2 +- src/util/virstring.c | 35 -------------------------- src/util/virstring.h | 3 --- tests/qemuhotplugmock.c | 10 +++----- 9 files changed, 43 insertions(+), 95 deletions(-) -- 2.31.1

2 5

[PATCH 00/14] virsh: Completion improvements and checking tool
by Peter Krempa 20 Sep '21

20 Sep '21

When inspecting whether https://gitlab.com/libvirt/libvirt/-/issues/9 is still valid I wrote a tool which outputs command options missing completers. Now that I had a bit of time with lot of interruptions which is ideal for going through such a thing I decided to clean up the tool and post it along with a few fixes and additions to completers. After this patchset the following completers are still missing: $ virsh self-test --completers-missing attach-disk: source, targetbus, driver, subdriver, cache, io, type, mode, sourcetype, source-protocol, source-host-transport, source-host-socket attach-interface: type, source, script, model blockcopy: dest change-media: source detach-interface: type domdisplay: type domxml-from-native: format domxml-to-native: format dump: file get-user-sshkeys: user metadata: uri, key numatune: mode, nodeset qemu-monitor-event: event restore: file save: file save-image-define: file save-image-dumpxml: file save-image-edit: file screenshot: file set-user-sshkeys: user set-user-password: user cpu-models: arch domcapabilities: virttype, emulatorbin, arch, machine hypervisor-cpu-baseline: virttype, emulator, arch, machine hypervisor-cpu-compare: virttype, emulator, arch, machine maxvcpus: type iface-bridge: bridge iface-unbridge: bridge net-update: command, section nodedev-detach: driver snapshot-create-as: memspec find-storage-pool-sources-as: type find-storage-pool-sources: type pool-create-as: source-path, source-dev, source-name, target, source-format, auth-type, secret-usage, secret-uuid, adapter-name, adapter-wwnn, adapter-wwpn, adapter-parent, adapter-parent-wwnn, adapter-parent-wwpn, adapter-parent-fabric-wwn, source-protocol-ver pool-define-as: source-path, source-dev, source-name, target, source-format, auth-type, secret-usage, secret-uuid, adapter-name, adapter-wwnn, adapter-wwpn, adapter-parent, adapter-parent-wwnn, adapter-parent-wwpn, adapter-parent-fabric-wwn, source-protocol-ver vol-create-as: format, backing-vol, backing-vol-format vol-download: file vol-wipe: algorithm cd: dir The list is not so long any more. Mostly missing completers are for remote file paths (passed to libvirt or the VM itself), few enums, guest-agent-based user list and a few which need to be assesed. Peter Krempa (14): virshCheckpointNameCompleter: Sanitize forward declaration use virsh-completer*.h: Use modern header style virsh: Remove hack using 'VSH_CMD_FLAG_ALIAS' to hide virsh commands vshCmddefCheckInternals: Sanitize command alias validation vsh: Introduce '--completers-missing' for 'self-test' command virsh-snapshot: Use 'virshSnapshotNameCompleter' for '--from' of 'snapshot-list' virsh: Use 'virshStoragePoolNameCompleter' for two options vsh: Add completer for '--command' of 'help' command virsh: Provide completers for options taking comma separated list of disk targets virsh: Expand VIRSH_COMMON_OPT_FILE for cases when it's not a local file used by virsh virsh: completer: Introduce dummy completer for local files virsh: Use 'virshCompletePathLocalExisting' for options reading local files virsh: Introduce virshCompleteEmpty and use it for places where we can't suggest anything virsh-completer: Provide completer for '--top' and '--base' for blockjobs tools/virsh-backup.c | 2 + tools/virsh-checkpoint.c | 3 + tools/virsh-completer-checkpoint.h | 7 +- tools/virsh-completer-domain.c | 136 +++++++++++++++++++++++ tools/virsh-completer-domain.h | 170 ++++++++++++++++++----------- tools/virsh-completer-host.h | 28 +++-- tools/virsh-completer-interface.h | 14 ++- tools/virsh-completer-network.h | 35 +++--- tools/virsh-completer-nodedev.h | 21 ++-- tools/virsh-completer-nwfilter.h | 14 ++- tools/virsh-completer-pool.h | 21 ++-- tools/virsh-completer-secret.h | 14 ++- tools/virsh-completer-snapshot.h | 7 +- tools/virsh-completer-volume.h | 14 ++- tools/virsh-completer.c | 35 ++++++ tools/virsh-completer.h | 18 ++- tools/virsh-domain.c | 84 +++++++++++++- tools/virsh-network.c | 1 + tools/virsh-pool.c | 9 ++ tools/virsh-secret.c | 2 + tools/virsh-snapshot.c | 4 + tools/virsh-volume.c | 12 +- tools/virsh.c | 4 +- tools/virsh.h | 2 + tools/virt-admin.c | 3 +- tools/vsh.c | 66 +++++++++-- tools/vsh.h | 10 +- 27 files changed, 568 insertions(+), 168 deletions(-) -- 2.31.1

4 24

[PATCH 0/2] virsh: Fix fallback code path for vcpuinfo
by Peter Krempa 20 Sep '21

20 Sep '21

Peter Krempa (2): virshDomainGetVcpuBitmap: Return bitmap when taking the fallback path virshDomainGetVcpuBitmap: Refactor cleanup tools/virsh-domain.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) -- 2.31.1

5 6

[libvirt PATCH] gitlab: remove obsolete job rules for TEMPORARILY_DISABLED variable
by Daniel P. Berrangé 20 Sep '21

20 Sep '21

We previously had a 'rules:' entry that caused a job to be skipped if the variable "TEMPORARILY_DISABLED" was set. This is no longer needed since we can set a similar flag in ci/manifest.yml and re-generate to temporarily skip a job. Unfortunately the 'rules:' entry had an unexpected side-effect on the pipelines that was never previously noticed. Instead of only running pipelines on push, the mere existance of the 'rules:' entry caused triggering of pipelines on merge requests too. The newly auto-generated ci/gitlab.yml file does not have a 'rules:' for the container job template, and thus only runs on git push. The result is that build jobs try to run on merge requests and the container jobs they depend on don't exist. This breaks the entire pipeline with a message that the config is invalid due to broken job dependencies. This fixes a regression introduced in commit ccc7a44adbea003d6a0dc2f156adb2856c28bd4c Author: Daniel P. Berrangé <berrange(a)redhat.com> Date: Thu Sep 9 14:49:01 2021 +0100 ci: re-generate containers/gitlab config from manifest Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- .gitlab-ci.yml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index b396a1511d..d486faca58 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -18,10 +18,6 @@ include: '/ci/gitlab.yml' .native_build_job: extends: .gitlab_native_build_job - rules: - - if: "$TEMPORARILY_DISABLED" - allow_failure: true - - when: on_success cache: paths: - ccache/ @@ -45,10 +41,6 @@ include: '/ci/gitlab.yml' paths: - ccache/ key: "$CI_JOB_NAME" - rules: - - if: "$TEMPORARILY_DISABLED" - allow_failure: true - - when: on_success before_script: - *script_variables script: -- 2.31.1

2 1

[PATCH] tools: virsh-snapshot: refactor small functions
by Kristina Hanicova 20 Sep '21

20 Sep '21

This patch includes: * removal of dead code * simplifying nested if conditions * removal of unnecessary variables * usage of "direct" boolean return Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com> --- tools/virsh-snapshot.c | 43 +++++++++++++++--------------------------- 1 file changed, 15 insertions(+), 28 deletions(-) diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 2659b4340d..3bdad03df0 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -771,7 +771,6 @@ virshSnapshotFilter(vshControl *ctl, virDomainSnapshotPtr snapshot, g_autofree char *xml = NULL; g_autoptr(xmlDoc) xmldoc = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; - int ret = -1; g_autofree char *state = NULL; if (!snapshot) @@ -796,20 +795,17 @@ virshSnapshotFilter(vshControl *ctl, virDomainSnapshotPtr snapshot, return -1; } if (STREQ(state, "disk-snapshot")) { - ret = ((flags & (VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY | - VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)) == - (VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY | - VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)); - } else { - if (!(flags & VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL)) - ret = 0; - else if (STREQ(state, "shutoff")) - ret = !!(flags & VIR_DOMAIN_SNAPSHOT_LIST_INACTIVE); - else - ret = !!(flags & VIR_DOMAIN_SNAPSHOT_LIST_ACTIVE); + return ((flags & (VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY | + VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)) == + (VIR_DOMAIN_SNAPSHOT_LIST_DISK_ONLY | + VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL)); } - return ret; + if (!(flags & VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL)) + return 0; + if (STREQ(state, "shutoff")) + return !!(flags & VIR_DOMAIN_SNAPSHOT_LIST_INACTIVE); + return !!(flags & VIR_DOMAIN_SNAPSHOT_LIST_ACTIVE); } /* @@ -869,14 +865,8 @@ cmdSnapshotInfo(vshControl *ctl, const vshCmd *cmd) * attempt a fallback. */ current = virDomainSnapshotIsCurrent(snapshot, 0); if (current < 0) { - g_autoptr(virshDomainSnapshot) other = NULL; - vshResetLibvirtError(); current = 0; - if (other) { - if (STREQ(name, virDomainSnapshotGetName(other))) - current = 1; - } } vshPrint(ctl, "%-15s %s\n", _("Current:"), current > 0 ? _("yes") : _("no")); @@ -1776,10 +1766,8 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd) vshResetLibvirtError(); result = virDomainRevertToSnapshot(snapshot, flags); } - if (result < 0) - return false; - return true; + return result >= 0; } /* @@ -1844,16 +1832,15 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd) /* XXX If we wanted, we could emulate DELETE_CHILDREN_ONLY even on * older servers that reject the flag, by manually computing the * list of descendants. But that's a lot of code to maintain. */ - if (virDomainSnapshotDelete(snapshot, flags) == 0) { - if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) - vshPrintExtra(ctl, _("Domain snapshot %s children deleted\n"), name); - else - vshPrintExtra(ctl, _("Domain snapshot %s deleted\n"), name); - } else { + if (virDomainSnapshotDelete(snapshot, flags) < 0) { vshError(ctl, _("Failed to delete snapshot %s"), name); return false; } + if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) + vshPrintExtra(ctl, _("Domain snapshot %s children deleted\n"), name); + else + vshPrintExtra(ctl, _("Domain snapshot %s deleted\n"), name); return true; } -- 2.31.1

3 2

[libvirt PATCH 0/2] conf: log error when attempting to live update device acpi index
by Laine Stump 17 Sep '21

17 Sep '21

This isn't possible (acpi index is set when devices are probed at guest startup) and libvirt wasn't even trying to communicate the change to the guest in any way, but instead of logging an error, we were just pretending the request had succeeded. Laine Stump (2): conf: reformat virDomainDefCompatibleDevice for upcoming additional check conf: log error on attempts to modify ACPI index of active device src/conf/domain_conf.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) -- 2.31.1

3 11

[libvirt PATCH 0/3] Reduce stack frame size of virNWFilterRuleDefFixup
by Tim Wiederhake 17 Sep '21

17 Sep '21

When libvirt is build with sanitizers enabled, in debug mode, on clang, virNWFilterRuleDefFixup exceeds the maximum stack frame size of 8192 bytes, as specified in meson.build: ../src/conf/nwfilter_conf.c:2190:1: error: stack frame size of 10616 bytes in function 'virNWFilterRuleDefFixup' [-Werror,-Wframe-larger-than=] virNWFilterRuleDefFixup(virNWFilterRuleDef *rule) ^ 1 error generated. This series reworks the function a bit to bring the frame size below 8192. Regards, Tim Tim Wiederhake (3): virNWFilterRuleDefFixup: Factor out ethHdr as variable virNWFilterRuleDefFixup: Factor out ipHdr as variable virNWFilterRuleDefFixup: Factor out portData as variable src/conf/nwfilter_conf.c | 179 ++++++++++++++++----------------------- 1 file changed, 73 insertions(+), 106 deletions(-) -- 2.31.1

2 10

[libvirt PATCH v3 0/1] Add a PCI/PCIe device VPD Capability
by Dmitrii Shcherbakov 17 Sep '21

17 Sep '21

Add support for deserializing the binary PCI/PCIe VPD format and exposing VPD resources as XML elements in a new nested capability of PCI/PCIe devices called 'vpd'. The VPD format is specified in "I.3. VPD Definitions" in PCI specs (2.2+) and "6.28.1 VPD Format" PCIe 4.0. As section 6.28 in PCIe 4.0 notes, the PCI Local Bus and PCIe VPD formats are binary compatible and PCIe 4.0 merely started incorporating what was already present in PCI specs. Linux kernel exposes a binary blob in the VPD format via sysfs since v2.6.26 (commit 94e6108803469a37ee1e3c92dafdd1d59298602f) which requires a parser to interpret. There are usage scenarios where information such as the board serial number needs to be retrieved from PCI(e) VPD. Projects like Nova can utilize this information for cases which involve virtual interface plugging on SmartNIC DPUs but there may be other scenarios and types of information useful to retrieve from VPD. The fact that the format is binary requires proper parsing instead of substring searching hence the full parser is proposed. Likewise, checksum validation requires proper parsing as well. The patch follows a prior discussion on the mailing list which has additional context about the use-case but a narrower proposal: https://listman.redhat.com/archives/libvir-list/2021-May/msg00873.html https://www.mail-archive.com/libvir-list@redhat.com/msg218165.html The new functionality is mostly contained in virpcivpd with a couple of new functions added to virpci. Additionally, the necessary XML serialization/deserialization and glue code is added to expose the VPD capability to external clients as XML. A new capability flag is added along with a new capability in order to allow for filtering of PCI devices with the VPD capability using virsh: virsh nodedev-list --cap vpd sudo virsh nodedev-dumpxml --device pci_dddd_bb_ss_f In this example having the root uid is required in order to access the vpd sysfs entry, therefore, the nodedev XML output will only contain the VPD capability if virsh is run as root. The capability is treated as dynamic due to the presence of read-write sections in the VPD format per PCI/PCIe specs (the idea being that read-write resource fields may potentially be altered by the DPU OS over time independently from the host OS). Unit tests cover the parser functionality (including many possible invalid cases), in-memory representation as well as XML serialization and deserialization. Manual functional testing was performed with 2 DPUs and several other NIC models which expose PCI(e) VPD. Testing have also been performed for devices that do not have VPD or those that expose a VPD capability but exhibit invalid behavior (I/O errors while reading a sysfs entry). Per the existing guidelines, the implementation relies heavily on glib for various purposes. https://libvirt.org/glib-adoption.html v3 of the patch is a rebase of v2 on top of the latest changes to the master branch. Dmitrii Shcherbakov (1): Add a PCI/PCIe device VPD Capability build-aux/syntax-check.mk | 4 +- docs/drvnodedev.html.in | 46 ++ docs/formatnode.html.in | 24 +- docs/schemas/nodedev.rng | 40 + include/libvirt/libvirt-nodedev.h | 1 + po/POTFILES.in | 1 + src/conf/node_device_conf.c | 258 ++++++ src/conf/node_device_conf.h | 6 +- src/conf/virnodedeviceobj.c | 7 +- src/libvirt_private.syms | 17 + src/node_device/node_device_driver.c | 2 + src/node_device/node_device_udev.c | 2 + src/util/meson.build | 1 + src/util/virpci.c | 60 ++ src/util/virpci.h | 3 + src/util/virpcivpd.c | 771 +++++++++++++++++ src/util/virpcivpd.h | 106 +++ src/util/virpcivpdpriv.h | 42 + tests/meson.build | 1 + .../pci_0000_42_00_0_vpd.xml | 33 + .../pci_0000_42_00_0_vpd.xml | 1 + tests/nodedevxml2xmltest.c | 1 + tests/testutils.c | 51 ++ tests/testutils.h | 6 + tests/virpcitest.c | 3 + tests/virpcivpdtest.c | 777 ++++++++++++++++++ tools/virsh-nodedev.c | 3 + 27 files changed, 2262 insertions(+), 5 deletions(-) create mode 100644 src/util/virpcivpd.c create mode 100644 src/util/virpcivpd.h create mode 100644 src/util/virpcivpdpriv.h create mode 100644 tests/nodedevschemadata/pci_0000_42_00_0_vpd.xml create mode 120000 tests/nodedevxml2xmlout/pci_0000_42_00_0_vpd.xml create mode 100644 tests/virpcivpdtest.c -- 2.30.2

2 3

[libvirt PATCH] Fix typos
by Tim Wiederhake 17 Sep '21

17 Sep '21

Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com> --- src/ch/ch_monitor.c | 2 +- tests/virstoragetest.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 4ed7cbfee7..d277466262 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -789,7 +789,7 @@ virCHMonitorResumeVM(virCHMonitor *mon) * @mon: Pointer to the monitor * @info: Get VM info * - * Retrive the VM info and store in @info + * Retrieve the VM info and store in @info * * Returns 0 on success. */ diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index ec185d8660..ccfc9f7001 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -100,7 +100,7 @@ testPrepImages(void) return NULL; } - /* create the folowing real backing chain with qcow2 images with absolute + /* create the following real backing chain with qcow2 images with absolute * backing and different qcow2 versions: * datadir/raw <- datadir/qcow2 (qcow2v2) <- datadir/wrap (qcow2v3) */ cmdraw = virCommandNewArgList(qemuimg, "create", -- 2.31.1

2 1

[PATCH] vsh: Move 'vshReadlineOptionsGenerator' out of '#if WITH_READLINE'
by Peter Krempa 17 Sep '21

17 Sep '21

It's now also used in vshCompleteHelpCommand which is outside of the conditionally compiled code. Fixes: 80f70c74a7 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- Pushed under the build-fix rule. tools/vsh.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index cf24586b25..9057310077 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2543,17 +2543,12 @@ vshTreePrint(vshControl *ctl, vshTreeLookup lookup, void *opaque, return ret; } -#if WITH_READLINE - -/* ----------------- - * Readline stuff - * ----------------- - */ /** * vshReadlineCommandGenerator: * - * Generator function for command completion. + * Generator function for command completion. Used also for completing the + * '--command' option of the 'help' command. * * Returns a string list of all commands, or NULL on failure. */ @@ -2590,6 +2585,15 @@ vshReadlineCommandGenerator(void) return g_steal_pointer(&ret); } + +#if WITH_READLINE + +/* ----------------- + * Readline stuff + * ----------------- + */ + + static char ** vshReadlineOptionsGenerator(const vshCmdDef *cmd, vshCmd *last) -- 2.31.1

1 0