- Devel - libvirt List Archives

[PATCH] NEWS: add note about virt-ssh-helper perf improvements
by Daniel P. Berrangé 27 Nov '20

27 Nov '20

Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- NEWS.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index aa8a217eb6..8245e34efa 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -44,6 +44,13 @@ v6.10.0 (unreleased) * **Bug fixes** + * remote: fixed performance regression in SSH tunnelling + + The virt-ssh-helper binary introduced in 6.9.0 had very + poor scalability which impacted libvirt tunnelled migration + and storage volume upload/download in particular. It has been + updated and now has performance on a par with netcat. + * **Removed features** * hyperv: removed support for the Hyper-V V1 WMI API -- 2.28.0

2 1

[libvirt][RFC PATCH] add a new 'default' option for attribute mode in numatune
by Luyao Zhong 27 Nov '20

27 Nov '20

Hi Libvirt experts, I would like enhence the numatune snippet configuration. Given a example snippet: <domain> ... <numatune> <memory mode="strict" nodeset="1-4,^3"/> <memnode cellid="0" mode="strict" nodeset="1"/> <memnode cellid="2" mode="preferred" nodeset="2"/> </numatune> ... </domain> Currently, attribute mode is either 'interleave', 'strict', or 'preferred', I propose to add a new 'default' option. I give the reason as following. Presume we are using cgroups v1, Libvirt sets cpuset.mems for all vcpu threads according to 'nodeset' in memory element. And translate the memnode element to qemu config options (--object memory-backend-ram) for per numa cell, which invoking mbind() system call at the end.[1] But what if we want using default memory policy and request each guest numa cell pinned to different host memory nodes? We can't use mbind via qemu config options, because (I quoto here) "For MPOL_DEFAULT, the nodemask and maxnode arguments must be specify the empty set of nodes." [2] So my solution is introducing a new 'default' option for attribute mode. e.g. <domain> ... <numatune> <memory mode="default" nodeset="1-2"/> <memnode cellid="0" mode="default" nodeset="1"/> <memnode cellid="1" mode="default" nodeset="2"/> </numatune> ... </domain> If the mode is 'default', libvirt should avoid generating qemu command line '--object memory-backend-ram', and invokes cgroups to set cpuset.mems for per guest numa combining with numa topology config. Presume the numa topology is : <cpu> ... <numa> <cell id='0' cpus='0-3' memory='512000' unit='KiB' /> <cell id='1' cpus='4-7' memory='512000' unit='KiB' /> </numa> ... </cpu> Then libvirt should set cpuset.mems to '1' for vcpus 0-3, and '2' for vcpus 4-7. Is this reasonable and feasible? Welcome any comments. Regards, Luyao [1]https://github.com/qemu/qemu/blob/f2a1cf9180f63e88bb38ff21c169da97c3f2bad… [2]https://man7.org/linux/man-pages/man2/mbind.2.html -- 2.25.1

4 25

[libvirt][PATCH v1 0/3] introduce 'restrictive' mode in numatune
by Luyao Zhong 27 Nov '20

27 Nov '20

Before this patch set, numatune only has three memory modes: static, interleave and prefered. These memory policies are ultimately set by mbind() system call. Memory policy could be 'hard coded' into the kernel, but none of above policies fit our requirment under this case. mbind() support default memory policy, but it requires a NULL nodemask. So obviously setting allowed memory nodes is cgroups' mission under this case. So we introduce a new option for mode in numatune named 'restrictive'. <numatune> <memory mode="restrictive" nodeset="1-4,^3"/> <memnode cellid="0" mode="restrictive" nodeset="1"/> <memnode cellid="2" mode="restrictive" nodeset="2"/> </numatune> The config above means we only use cgroups to restrict the allowed memory nodes and not setting any specific memory policies explicitly. RFC discussion: https://www.redhat.com/archives/libvir-list/2020-November/msg01256.html Regards, Luyao Luyao Zhong (3): docs: add docs for 'restrictive' option for mode in numatune schema: add 'restrictive' config option for mode in numatune qemu: add parser and formatter for 'restrictive' mode in numatune docs/formatdomain.rst | 7 +++- docs/schemas/domaincommon.rng | 2 + include/libvirt/libvirt-domain.h | 1 + src/conf/numa_conf.c | 9 +++++ src/qemu/qemu_command.c | 6 ++- src/qemu/qemu_process.c | 27 +++++++++++++ src/util/virnuma.c | 3 ++ .../numatune-memnode-invalid-mode.err | 1 + .../numatune-memnode-invalid-mode.xml | 33 +++++++++++++++ ...emnode-restrictive-mode.x86_64-latest.args | 40 +++++++++++++++++++ .../numatune-memnode-restrictive-mode.xml | 33 +++++++++++++++ tests/qemuxml2argvtest.c | 2 + ...memnode-restrictive-mode.x86_64-latest.xml | 40 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 14 files changed, 202 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/numatune-memnode-invalid-mode.err create mode 100644 tests/qemuxml2argvdata/numatune-memnode-invalid-mode.xml create mode 100644 tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/numatune-memnode-restrictive-mode.xml create mode 100644 tests/qemuxml2xmloutdata/numatune-memnode-restrictive-mode.x86_64-latest.xml -- 2.25.4

1 3

[PATCH v2] gitlab-ci: publish test report as an artifact
by Paolo Bonzini 26 Nov '20

26 Nov '20

Since version 0.55, "meson test" produces JUnit XML in the meson-logs directory. The XML can be parsed by GitLab and showed as part of the CI report. However, if the build and tests are performed by "meson dist", the tests are performed in "meson dist"'s own build directory and the logs are not accessible. So switch from "ninja dist" to "meson dist --no-tests" after a separate build step that is shared by the normal and the DIST=skip cases. Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com> --- v1->v2: only do it for new-enough distros For an example see https://gitlab.com/bonzini/libvirt/-/pipelines/221545357/test_report. Test durations however are not yet available in upstream Meson. --- .gitlab-ci.yml | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6792accf8f..c4b54201f8 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -40,6 +40,36 @@ stages: <<: *container_job_definition allow_failure: true +# For new enough distros that have Meson 0.55, include the JUnit XML +# report in the artifacts. In order to preserve it, we cannot use +# "meson dist" and need to do the compilation test manually. +# Fortunately, "meson dist --no-tests" was also added in Meson 0.55. +.native_meson055_build_job_template: &native_meson055_build_job_definition + stage: builds + image: $CI_REGISTRY_IMAGE/ci-$NAME:latest + cache: + paths: + - ccache/ + key: "$CI_JOB_NAME" + before_script: + - *script_variables + script: + - meson build --werror || (cat build/meson-logs/meson-log.txt && exit 1) + - ninja -C build + - ninja -C build test + - DESTDIR=$PWD/install/ ninja -C build install + - meson dist -C build --no-tests + - if test -x /usr/bin/rpmbuild && test "$RPM" != "skip"; + then + rpmbuild --nodeps -ta build/meson-dist/libvirt-*.tar.xz; + fi + artifacts: + when: always + paths: + - build/meson-logs/ + reports: + junit: build/meson-logs/testlog.junit.xml + .native_build_job_template: &native_build_job_definition stage: builds image: $CI_REGISTRY_IMAGE/ci-$NAME:latest @@ -292,7 +322,7 @@ x64-debian-10-clang: CC: clang x64-debian-sid: - <<: *native_build_job_definition + <<: *native_meson055_build_job_definition needs: - x64-debian-sid-container variables: @@ -343,21 +373,21 @@ x64-fedora-31: RPM: skip x64-fedora-32: - <<: *native_build_job_definition + <<: *native_meson055_build_job_definition needs: - x64-fedora-32-container variables: NAME: fedora-32 x64-fedora-rawhide: - <<: *native_build_job_definition + <<: *native_meson055_build_job_definition needs: - x64-fedora-rawhide-container variables: NAME: fedora-rawhide x64-fedora-rawhide-clang: - <<: *native_build_job_definition + <<: *native_meson055_build_job_definition needs: - x64-fedora-rawhide-container variables: -- 2.28.0

1 0

[PATCH] NEWS: Mention network disk support in 'virsh attach-disk'
by Peter Krempa 26 Nov '20

26 Nov '20

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- NEWS.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index aa8a217eb6..8c2b5def0f 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -42,6 +42,14 @@ v6.10.0 (unreleased) * **Improvements** + * virsh: Support network disks in ``virsh attach-disk`` + + The ``virsh attach-disk`` helper command which simplifies attaching of disks + without the need for the user to formulate the disk XML manually now + supports network-backed images. Users can specify the protocol and host + specification with new command line arguments. Please refer to the man + page of virsh for further information. + * **Bug fixes** * **Removed features** -- 2.28.0

2 1

[PATCH] gitlab-ci: publish test report as an artifact
by Paolo Bonzini 26 Nov '20

26 Nov '20

"meson test" produces JUnit XML in the meson-logs directory. The XML can be parsed by GitLab and showed as part of the CI report. However, if the build and tests are performed by "meson dist", the tests are performed in "meson dist"'s own build directory and the logs are not accessible. So switch from "ninja dist" to "meson dist --no-tests" after a separate build step that is shared by the normal and the DIST=skip cases. Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com> --- For an example see https://gitlab.com/bonzini/libvirt/-/pipelines/221545357/test_report. Test durations however are not yet available in upstream Meson. --- .gitlab-ci.yml | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6792accf8f..ce7b60dc6b 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -51,17 +51,23 @@ stages: - *script_variables script: - meson build --werror || (cat build/meson-logs/meson-log.txt && exit 1) + - ninja -C build + - ninja -C build test + - DESTDIR=$PWD/install/ ninja -C build install - if test "$DIST" != "skip"; then - ninja -C build dist; - else - ninja -C build; - ninja -C build test; + meson dist -C build --no-tests; fi - if test -x /usr/bin/rpmbuild && test "$RPM" != "skip"; then rpmbuild --nodeps -ta build/meson-dist/libvirt-*.tar.xz; fi + artifacts: + when: always + paths: + - build/meson-logs/ + reports: + junit: build/meson-logs/testlog.junit.xml # Jobs that we delegate to Cirrus CI because they require an operating # system other than Linux. These jobs will only run if the required -- 2.28.0

1 0

[libvirt PATCH 0/2] fix regression in SSH tunnelling performance
by Daniel P. Berrangé 26 Nov '20

26 Nov '20

In testing the "vol-download" command in virsh, downloading a 1G file takes a ridiculous amount of time (minutes) with the new SSH helper. After the first patch is applied the time gets down to a much more reasonable 5.5 seconds on my test machine. By comparison netcat achieved 4 seconds. After applying the second patch, the time is reduced to 3.5 seconds, so we actually end up beating netcat, as long as we have glib >= 2.64.0 available. Daniel P. Berrangé (2): remote: make ssh-helper massively faster util: avoid glib event loop workaround where possible src/remote/remote_ssh_helper.c | 113 ++++++++++++++++++++------------- src/util/vireventglib.c | 29 ++++++--- 2 files changed, 89 insertions(+), 53 deletions(-) -- 2.25.4

4 8

Migration with "--p2p --tunnelled" hanging in v6.9.0
by Christian Ehrhardt 26 Nov '20

26 Nov '20

Hi, I'm wondering about the best next steps to debug a migration issue. What I found is that with libvirt v6.9.0 a migration hangs if used like: $ virsh migrate --unsafe --live --p2p --tunnelled h-migr-test \ qemu+ssh://testkvm-hirsute-to/system Just "--live --p2p" works fine. Also a bunch of other migration option combinations work with the same build/setup, just p2p+tunnelled fails. Also if either source or target are not on 6.9 the migration works (former version used is v6.6 for me). I looked at alternating qemu versions (5.0 / 5.1), but it had no impact. It only depends on libvirt to be on the new version on source&target to trigger the issue. Unfortunately there is no crash or error to debug into, it just gets stuck with the "virsh migrate" hanging on the source and never leaving the "paused" state on the target. I have compared setups with the least amount of "change": good: Qemu 5.1 + Libvirt 6.9 -> Qemu 5.1 / Libvirt 6.6 bad: Qemu 5.1 + Libvirt 6.9 -> Qemu 5.1 / Libvirt 6.9 [1] has the debug logs of those, beginning with a libvirtd restart that one can likely skip and then into the migration that hangs in the bad case. But I failed to see an obvious reason in the log. In git/news I only found these changes which sounded to be relevant: f51cbe92c0 qemu: Allow migration over UNIX socket c69915ccaf peer2peer migration: allow connecting to local sockets But I'm not using unix: and in the logs the only unix: mentions are for the qemu monitor and qemu-guest-agent. I wanted to ask: - if something related was recently changed that comes to mind? - if someone else sees anything in the linked logs that I missed? - if someone else has seen/reproduced the same? - for best practise to debug a hanging migration Thanks in advance! [1]: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1904584/+attachment/543… -- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd

3 12

[libvirt PATCH 0/4] cgroup cpu period and quota fixes
by Pavel Hrdina 26 Nov '20

26 Nov '20

Pavel Hrdina (4): qemu: move cgroup cpu period and quota defines to vircgroup.h vircgroupv1: use defines for cpu period and quota limits vircgroupv2: use defines for cpu period and quota limits vircgroup: fix cpu quota maximum limit src/qemu/qemu_driver.c | 21 ++++++++------------- src/util/vircgroup.h | 7 +++++++ src/util/vircgroupv1.c | 23 ++++++++++++----------- src/util/vircgroupv2.c | 25 +++++++++++++------------ 4 files changed, 40 insertions(+), 36 deletions(-) -- 2.26.2

2 5

[PATCH] qemu: Tweak debug message for qemuMigrationSrcPerformPeer2Peer3
by Martin Kletzander 25 Nov '20

25 Nov '20

Commit 49186372dbe8 forgot to add the new parameter. Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- Pushed as 'trivial'. src/qemu/qemu_migration.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 4be8f3c64c94..fcb33d0364a1 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4356,12 +4356,12 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver, VIR_DEBUG("driver=%p, sconn=%p, dconn=%p, dconnuri=%s, vm=%p, xmlin=%s, " "dname=%s, uri=%s, graphicsuri=%s, listenAddress=%s, " - "nmigrate_disks=%zu, migrate_disks=%p, nbdPort=%d, " + "nmigrate_disks=%zu, migrate_disks=%p, nbdPort=%d, nbdURI=%s, " "bandwidth=%llu, useParams=%d, flags=0x%lx", driver, sconn, dconn, NULLSTR(dconnuri), vm, NULLSTR(xmlin), NULLSTR(dname), NULLSTR(uri), NULLSTR(graphicsuri), NULLSTR(listenAddress), nmigrate_disks, migrate_disks, nbdPort, - bandwidth, useParams, flags); + NULLSTR(nbdURI), bandwidth, useParams, flags); /* Unlike the virDomainMigrateVersion3 counterpart, we don't need * to worry about auto-setting the VIR_MIGRATE_CHANGE_PROTECTION -- 2.29.2

1 0

[libvirt PATCH 0/2] Add more documentation for migrations over UNIX sockets
by Martin Kletzander 25 Nov '20

25 Nov '20

Few words about SELinux that might not be very clear to some. Martin Kletzander (2): qemu: Disable NBD TLS migration over UNIX socket docs: Document SELinux caveats when migrating over UNIX sockets docs/manpages/virsh.rst | 9 ++++++++- docs/migration.html.in | 9 +++++++++ src/qemu/qemu_migration.c | 10 ++++++++-- 3 files changed, 25 insertions(+), 3 deletions(-) -- 2.29.2

2 4

[PATCH v2 0/5] Hypervisor CPU Baseline Cleanups and Fixes
by Collin Walling 25 Nov '20

25 Nov '20

The following patches provide some TLC to the hypervisor CPU baseline handler within the qemu_driver code. #1 checks for the cpu-model-expansion capability before executing the baseline handler since it is used for feature expansion. #2 fixes a styling where a < 0 condition was missing from one of the if (function()) lines for consistency's sake. #3 will check if the cpu definition(s) are valid and contain a model name. #4 checks the cpu definition(s) model names against the model names known by the hypervisor. This patch must come before #5. #5 will allow the baseline command to be ran with a single cpu definition, whereas before the command would simply fail with an unhelpful error message. A CPU model expansion will be performed in this case, which will produce the same result as if the model were actually baselined. Note: without patch #4, #5 can result in a segfault in the case where a single CPU model is provided and the model is not recognized by the hypervisor. This is because cpu model expansion will return 0 and the result will be NULL. Since the QMP response returns "GenericError" in the case of a missing CPU model, or if the command is not supported (and perhaps for other reasons I am unsure of -- the response does not explicitly detail that the CPU model provided was erroneous), we cannot rely on this response always meaning there was a missing CPU model. So, to be safe-and-sure, the CPU model is checked against the list of CPU models known to the hypervisor prior to baselining / expanding (which were retrieved at some point previously during libvirt init). Collin Walling (5): qemu: check for model-expansion cap before baselining qemu: fix one instance of rc check styling in baseline qemu: report error if missing model name when baselining qemu: check if cpu model is supported before baselining qemu: fix error message when baselining with a single cpu src/qemu/qemu_driver.c | 45 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 8 deletions(-) -- 2.26.2

2 10

[PATCH 0/8] run iptables directly rather than via firewalld
by Laine Stump 25 Nov '20

25 Nov '20

The reasoning for this is explained in Patch 8/8 Laine Stump (8): util: fix typo in VIR_MOCK_WRAP_RET_ARGS() util/tests: enable locking on iptables/ebtables commandlines in unit tests util/tests: enable locking on iptables/ebtables commandlines by default tests: fix iptables test case commandline options in virfirewalltest.c network: be more verbose about the reason for a firewall reload util: always check for ebtables/iptables binaries, even when using firewalld util: synchronize with firewalld before we start calling iptables directly util: call iptables directly rather than via firewalld src/libvirt_private.syms | 2 +- src/network/bridge_driver.c | 10 +- src/util/virfirewall.c | 155 +++--- src/util/virfirewall.h | 2 +- src/util/viriptables.c | 7 + tests/networkxml2firewalldata/base.args | 34 ++ .../nat-default-linux.args | 19 + .../nat-ipv6-linux.args | 30 ++ .../nat-ipv6-masquerade-linux.args | 34 ++ .../nat-many-ips-linux.args | 33 ++ .../nat-no-dhcp-linux.args | 29 ++ .../nat-tftp-linux.args | 21 + .../route-default-linux.args | 14 + tests/networkxml2firewalltest.c | 2 - tests/nwfilterebiptablestest.c | 466 +++++++++--------- .../ah-ipv6-linux.args | 9 + tests/nwfilterxml2firewalldata/ah-linux.args | 9 + .../all-ipv6-linux.args | 9 + tests/nwfilterxml2firewalldata/all-linux.args | 9 + tests/nwfilterxml2firewalldata/arp-linux.args | 5 + .../comment-linux.args | 19 + .../conntrack-linux.args | 7 + .../esp-ipv6-linux.args | 9 + tests/nwfilterxml2firewalldata/esp-linux.args | 9 + .../example-1-linux.args | 12 + .../example-2-linux.args | 10 + .../hex-data-linux.args | 10 + .../icmp-direction-linux.args | 6 + .../icmp-direction2-linux.args | 6 + .../icmp-direction3-linux.args | 6 + .../nwfilterxml2firewalldata/icmp-linux.args | 3 + .../icmpv6-linux.args | 4 + .../nwfilterxml2firewalldata/igmp-linux.args | 9 + tests/nwfilterxml2firewalldata/ip-linux.args | 3 + .../nwfilterxml2firewalldata/ipset-linux.args | 18 + .../ipt-no-macspoof-linux.args | 2 + .../nwfilterxml2firewalldata/ipv6-linux.args | 15 + .../nwfilterxml2firewalldata/iter1-linux.args | 9 + .../nwfilterxml2firewalldata/iter2-linux.args | 171 +++++++ .../nwfilterxml2firewalldata/iter3-linux.args | 15 + tests/nwfilterxml2firewalldata/mac-linux.args | 4 + .../nwfilterxml2firewalldata/rarp-linux.args | 6 + .../sctp-ipv6-linux.args | 9 + .../nwfilterxml2firewalldata/sctp-linux.args | 9 + tests/nwfilterxml2firewalldata/stp-linux.args | 11 + .../target-linux.args | 33 ++ .../target2-linux.args | 12 + .../tcp-ipv6-linux.args | 9 + tests/nwfilterxml2firewalldata/tcp-linux.args | 13 + .../udp-ipv6-linux.args | 9 + tests/nwfilterxml2firewalldata/udp-linux.args | 9 + .../udplite-ipv6-linux.args | 9 + .../udplite-linux.args | 9 + .../nwfilterxml2firewalldata/vlan-linux.args | 7 + tests/nwfilterxml2firewalltest.c | 146 +++--- tests/virfirewalltest.c | 236 ++++----- 56 files changed, 1259 insertions(+), 514 deletions(-) -- 2.28.0

2 12

[PATCH v3 0/2] hyperv: storage volume XML changes
by Matt Coleman 25 Nov '20

25 Nov '20

Changes since v2: * raise an error if the parent device cannot be located * correct the disk count and use constants instead of magic numbers Here's a GitLab MR if you'd prefer to review it there: https://gitlab.com/iammattcoleman/libvirt/-/merge_requests/12/commits This link shows a diff between v2 and v3: https://gitlab.com/iammattcoleman/libvirt/-/merge_requests/12/diffs?diff_id… Matt Coleman (2): hyperv: XML parsing of storage volumes schema: add support for Windows file paths and device names docs/schemas/basictypes.rng | 2 +- docs/schemas/domaincommon.rng | 5 +- src/hyperv/hyperv_driver.c | 425 +++++++++++++++++- src/hyperv/hyperv_driver.h | 5 + src/hyperv/hyperv_private.h | 2 + src/hyperv/hyperv_wmi.c | 45 ++ src/hyperv/hyperv_wmi.h | 8 + src/hyperv/hyperv_wmi_classes.h | 19 + src/hyperv/hyperv_wmi_generator.input | 134 ++++++ .../disk-hyperv-physical.xml | 17 + .../disk-hyperv-virtual.xml | 17 + .../disk-hyperv-physical.xml | 23 + .../disk-hyperv-virtual.xml | 23 + tests/genericxml2xmltest.c | 2 + 14 files changed, 724 insertions(+), 3 deletions(-) create mode 100644 tests/genericxml2xmlindata/disk-hyperv-physical.xml create mode 100644 tests/genericxml2xmlindata/disk-hyperv-virtual.xml create mode 100644 tests/genericxml2xmloutdata/disk-hyperv-physical.xml create mode 100644 tests/genericxml2xmloutdata/disk-hyperv-virtual.xml -- 2.27.0

2 3

[PATCH 0/7] qemu: migration: TLS enforcement and non-shared-storage error improvement
by Peter Krempa 25 Nov '20

25 Nov '20

A collection of fixes for migration. Peter Krempa (7): docs: migration: Fix example for unix socket migration qemu: migration: Remove TODO about implementing NBD for TUNNELLED migration qemu: migration: Aggregate logic depending on tunnelled migration qemu: migration: Forbid tunnelled non-shared storage migration with -blockdev docs: migration: Mention that features may not work with tunnelled migration qemu: conf: Introduce "migrate_tls_force" qemu.conf option docs: migration: Add a mention of VIR_MIGRATE_TLS and it's enforcement for qemu docs/migration.html.in | 21 ++++++++++- src/qemu/libvirtd_qemu.aug | 1 + src/qemu/qemu.conf | 8 ++++ src/qemu/qemu_conf.c | 2 + src/qemu/qemu_conf.h | 1 + src/qemu/qemu_migration.c | 60 ++++++++++++++++++++++-------- src/qemu/test_libvirtd_qemu.aug.in | 1 + 7 files changed, 78 insertions(+), 16 deletions(-) -- 2.28.0

2 10

[PATCH 0/2] qemu: Fix setting of 'current' checkpoint/snapshot
by Peter Krempa 25 Nov '20

25 Nov '20

See patch 1 Peter Krempa (2): qemu: snapshot: Write metadata of previously-'current' snapshot on update qemu: checkpoint: Write metadata of previously-'current' checkpoint on update src/qemu/qemu_checkpoint.c | 35 +++++++++++++++++++++++++++++++++-- src/qemu/qemu_snapshot.c | 38 +++++++++++++++++++++++++++++++++++--- 2 files changed, 68 insertions(+), 5 deletions(-) -- 2.28.0

2 3

[PATCH 0/5] qemu: Don't abort() when creating internal snapshot with <disk type='volume'
by Peter Krempa 25 Nov '20

25 Nov '20

See patch 5/5 Peter Krempa (5): virCommandAddArg: Don't abort on invalid input virDomainDiskTranslateSourcePool: Don't break error message in half qemuDomainSnapshotForEachQcow2Raw: Avoid a level of indentation qemuDomainSnapshotForEachQcow2Raw: Lock out operation on unsupported storage qemuDomainSnapshotForEachQcow2: Pass in 'def' rather than selecting it internally src/conf/domain_conf.c | 3 +- src/qemu/qemu_domain.c | 102 +++++++++++++++++++++------------------ src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_snapshot.c | 16 ++++-- src/util/vircommand.c | 2 +- 5 files changed, 70 insertions(+), 55 deletions(-) -- 2.28.0

2 6

[libvirt PATCH 0/3] Introudce virCommandPassFDIndex
by Ján Tomko 24 Nov '20

24 Nov '20

Simplify passing file descriptors via add-fd by adding a new parameter to virCommandPassFD that returns the index of the passed FD right away, removing the need to look it up later and pretend to handle errors which cannot happen - we've just put the file descriptor there, why would it not be there? This should also silence Coverity - in that regard it's an alternative to Pavel's patch: https://www.redhat.com/archives/libvir-list/2020-November/msg00885.html Ján Tomko (3): util: introduce virCommandPassFDIndex qemu: introduce qemuBuildFDSet qemu: use qemuVirCommandGetDevSet less src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 38 +++++++++++++++++++++++++------------- src/util/vircommand.c | 28 ++++++++++++++++++++++++++-- src/util/vircommand.h | 5 +++++ 4 files changed, 57 insertions(+), 15 deletions(-) -- 2.26.2

2 4

[PATCH v2 0/2] virnetdaemon: Wait for "daemon-stop" thread to finish before quitting
by Michal Privoznik 24 Nov '20

24 Nov '20

This is a v2 of: https://www.redhat.com/archives/libvir-list/2020-November/msg00639.html but it implements a different approach per Nikolay's suggestion. Michal Prívozník (2): DO NOT MERGE virnetdaemon: Wait for "daemon-stop" thread to finish before quitting src/libvirt_remote.syms | 1 + src/remote/remote_daemon.c | 73 +++++++++++++++++++++++--------------- src/rpc/virnetdaemon.c | 17 +++++++++ src/rpc/virnetdaemon.h | 3 ++ 4 files changed, 66 insertions(+), 28 deletions(-) -- 2.26.2

2 3

[PATCH 0/2] hyperv: storage volume XML changes
by Matt Coleman 24 Nov '20

24 Nov '20

Here's a GitLab MR if you'd prefer to review it there: https://gitlab.com/iammattcoleman/libvirt/-/merge_requests/12/commits Matt Coleman (2): hyperv: XML parsing of storage volumes schema: add support for Windows file paths and device names docs/schemas/basictypes.rng | 2 +- docs/schemas/domaincommon.rng | 5 +- src/hyperv/hyperv_driver.c | 408 +++++++++++++++++- src/hyperv/hyperv_driver.h | 3 + src/hyperv/hyperv_wmi.c | 45 ++ src/hyperv/hyperv_wmi.h | 8 + src/hyperv/hyperv_wmi_classes.h | 19 + src/hyperv/hyperv_wmi_generator.input | 134 ++++++ .../disk-hyperv-physical.xml | 17 + .../disk-hyperv-virtual.xml | 17 + .../disk-hyperv-physical.xml | 23 + .../disk-hyperv-virtual.xml | 23 + tests/genericxml2xmltest.c | 2 + 13 files changed, 703 insertions(+), 3 deletions(-) create mode 100644 tests/genericxml2xmlindata/disk-hyperv-physical.xml create mode 100644 tests/genericxml2xmlindata/disk-hyperv-virtual.xml create mode 100644 tests/genericxml2xmloutdata/disk-hyperv-physical.xml create mode 100644 tests/genericxml2xmloutdata/disk-hyperv-virtual.xml -- 2.27.0

3 10

[libvirt PATCH v3] ci: Switch to meson build system
by Erik Skultety 24 Nov '20

24 Nov '20

Add meson required bits to the ci logic in the repo to be able to run a meson build in a container. This patch also drops a few environment variables we don't need with meson anymore. Signed-off-by: Erik Skultety <eskultet(a)redhat.com> --- v2: https://www.redhat.com/archives/libvir-list/2020-November/msg00380.html Since v2: - squashed both original patches into 1 - dropped some unnecessary code which meson handles implicitly ci/Makefile | 32 ++++++++------------------------ ci/build.sh | 29 +++-------------------------- 2 files changed, 11 insertions(+), 50 deletions(-) diff --git a/ci/Makefile b/ci/Makefile index c7c8eb9a45..6c253f0b1e 100644 --- a/ci/Makefile +++ b/ci/Makefile @@ -20,26 +20,8 @@ CI_HOST_SRCDIR = $(CI_SCRATCHDIR)/src # the $(CI_HOST_SRCDIR) directory from the host CI_CONT_SRCDIR = $(CI_USER_HOME)/libvirt -# Relative directory to perform the build in. This -# defaults to using a separate build dir, but can be -# set to empty string for an in-source tree build. -CI_VPATH = build - -# The directory holding the build output inside the -# container. -CI_CONT_BUILDDIR = $(CI_CONT_SRCDIR)/$(CI_VPATH) - -# Can be overridden with mingw{32,64}-configure if desired -CI_CONFIGURE = $(CI_CONT_SRCDIR)/configure - -# Default to using all possible CPUs -CI_SMP = $(shell getconf _NPROCESSORS_ONLN) - -# Any extra arguments to pass to make -CI_MAKE_ARGS = - -# Any extra arguments to pass to configure -CI_CONFIGURE_ARGS = +# Any extra arguments to pass to ninja +CI_NINJA_ARGS = # Script containing environment preparation steps CI_PREPARE_SCRIPT = $(CI_ROOTDIR)/prepare.sh @@ -221,12 +203,15 @@ ci-run-command@%: ci-prepare-tree --user="#$(CI_UID)" \ --group="#$(CI_GID)" \ CONFIGURE_OPTS="$$CONFIGURE_OPTS" \ + MESON_OPTS="$$MESON_OPTS" \ CI_CONT_SRCDIR="$(CI_CONT_SRCDIR)" \ CI_CONT_BUILDDIR="$(CI_CONT_BUILDDIR)" \ CI_SMP="$(CI_SMP)" \ CI_CONFIGURE="$(CI_CONFIGURE)" \ CI_CONFIGURE_ARGS="$(CI_CONFIGURE_ARGS)" \ CI_MAKE_ARGS="$(CI_MAKE_ARGS)" \ + CI_CONT_SRCDIR="$(CI_CONT_SRCDIR)" \ + CI_NINJA_ARGS="$(CI_NINJA_ARGS)" \ $(CI_COMMAND) || exit 1' @test "$(CI_CLEAN)" = "1" && rm -rf $(CI_SCRATCHDIR) || : @@ -236,8 +221,8 @@ ci-shell@%: ci-build@%: $(MAKE) -C $(CI_ROOTDIR) ci-run-command@$* CI_COMMAND="$(CI_USER_HOME)/build" -ci-check@%: - $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_MAKE_ARGS="check" +ci-test@%: + $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_NINJA_ARGS=test ci-list-images: @echo @@ -266,6 +251,5 @@ ci-help: @echo " CI_CLEAN=0 - do not delete '$(CI_SCRATCHDIR)' after completion" @echo " CI_REUSE=1 - re-use existing '$(CI_SCRATCHDIR)' content" @echo " CI_ENGINE=auto - container engine to use (podman, docker)" - @echo " CI_CONFIGURE_ARGS= - extra arguments passed to configure" - @echo " CI_MAKE_ARGS= - extra arguments passed to make, e.g. space delimited list of targets" + @echo " CI_NINJA_ARGS= - extra arguments passed to ninja" @echo diff --git a/ci/build.sh b/ci/build.sh index 2da84c080a..4530283cd8 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -7,32 +7,9 @@ # # to make. -mkdir -p "$CI_CONT_BUILDDIR" || exit 1 -cd "$CI_CONT_BUILDDIR" +cd "$CI_CONT_SRCDIR" export VIR_TEST_DEBUG=1 -NOCONFIGURE=1 "$CI_CONT_SRCDIR/autogen.sh" || exit 1 -# $CONFIGURE_OPTS is a env that can optionally be set in the container, -# populated at build time from the Dockerfile. A typical use case would -# be to pass --host/--target args to trigger cross-compilation -# -# This can be augmented by make local args in $CI_CONFIGURE_ARGS -"$CI_CONFIGURE" $CONFIGURE_OPTS $CI_CONFIGURE_ARGS -if test $? != 0; then - test -f config.log && cat config.log - exit 1 -fi -find -name test-suite.log -delete - -make -j"$CI_SMP" $CI_MAKE_ARGS - -if test $? != 0; then \ - LOGS=$(find -name test-suite.log) - if test "$LOGS"; then - echo "=== LOG FILE(S) START ===" - cat $LOGS - echo "=== LOG FILE(S) END ===" - fi - exit 1 -fi +meson build --werror $MESON_OPTS || (cat build/meson-logs/meson-log.txt && exit 1) +ninja -C build $CI_NINJA_ARGS -- 2.26.2

2 1

[PATCH 01/30] util: convert pointers to use g_autofree
by Ryan Gahagan 24 Nov '20

24 Nov '20

From: Barrett Schonefeld <bschoney(a)utexas.edu> - src/util/virxml.c Signed-off-by: Barrett Schonefeld <bschoney(a)utexas.edu> --- src/util/virxml.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index a3b819d85c..7df50e4b4d 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -696,8 +696,8 @@ catchXMLError(void *ctx, const char *msg G_GNUC_UNUSED, ...) unsigned int n, col; /* GCC warns if signed, because compared with sizeof() */ int domcode = VIR_FROM_XML; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - char *contextstr = NULL; - char *pointerstr = NULL; + g_autofree char *contextstr = NULL; + g_autofree char *pointerstr = NULL; /* conditions for error printing */ @@ -763,9 +763,6 @@ catchXMLError(void *ctx, const char *msg G_GNUC_UNUSED, ...) contextstr, pointerstr); } - - VIR_FREE(contextstr); - VIR_FREE(pointerstr); } /** -- 2.29.0

2 56

[PATCH 0/4] Final changes to fix test suite on macOS
by Roman Bolshakov 24 Nov '20

24 Nov '20

The set of changes fixes qemuxml2argvtest and closes issue #58. Note, qemucapsprobe hasn't been fixed yet but it has no impact on the current test suite. Pipeline results: https://gitlab.com/roolebo/libvirt/-/pipelines/213377159 Roman Bolshakov (4): tests: Fix opendir mocks on macOS tests: Fix mock chaining on macOS qemuxml2argvtest: Increase timeout on macOS ci: Run test suite on macOS ci/cirrus/build.yml | 3 +-- tests/meson.build | 8 +++++++- tests/virfilewrapper.c | 4 ++++ tests/virmock.h | 37 +++++++++++++++++++++++++++++++++++-- tests/virpcimock.c | 5 +++++ 5 files changed, 52 insertions(+), 5 deletions(-) -- 2.29.2

4 18

[PATCH] util: convert char pointers to use g_autofree
by Ryan Gahagan 24 Nov '20

24 Nov '20

From: Barrett Schonefeld <bschoney(a)utexas.edu> additional conversions to the GLib API in src/util per issue #11. Related issue: https://gitlab.com/libvirt/libvirt/-/issues/11 Signed-off-by: Barrett Schonefeld <bschoney(a)utexas.edu> --- src/util/vircgroupv1.c | 3 +- src/util/virdnsmasq.c | 43 ++++++++------------- src/util/virfile.c | 9 ++--- src/util/virhostcpu.c | 4 +- src/util/virlockspace.c | 6 +-- src/util/virlog.c | 12 ++---- src/util/virmacmap.c | 3 +- src/util/virnetdevbandwidth.c | 48 ++++++++--------------- src/util/virresctrl.c | 25 ++++-------- src/util/virrotatingfile.c | 11 ++---- src/util/virscsihost.c | 25 +++++------- src/util/virsecret.c | 14 +++---- src/util/virstorageencryption.c | 19 +++------ src/util/virstoragefilebackend.c | 4 +- src/util/virsysinfo.c | 18 +++------ src/util/viruri.c | 7 +--- src/util/virutil.c | 66 +++++++++++--------------------- src/util/virvhba.c | 57 ++++++++++----------------- src/util/virxml.c | 7 +--- 19 files changed, 130 insertions(+), 251 deletions(-) diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 731e9d61d4..984cd50409 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1549,7 +1549,7 @@ virCgroupV1GetMemoryStat(virCgroupPtr group, unsigned long long *unevictable) { int ret = -1; - char *stat = NULL; + g_autofree char *stat = NULL; char *line = NULL; unsigned long long cacheVal = 0; unsigned long long activeAnonVal = 0; @@ -1614,7 +1614,6 @@ virCgroupV1GetMemoryStat(virCgroupPtr group, ret = 0; cleanup: - VIR_FREE(stat); return ret; } diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index 9030f9218a..93bc4a129f 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -164,7 +164,7 @@ addnhostsWrite(const char *path, dnsmasqAddnHost *hosts, unsigned int nhosts) { - char *tmp; + g_autofree char *tmp = NULL; FILE *f; bool istmp = true; size_t i, j; @@ -180,7 +180,7 @@ addnhostsWrite(const char *path, istmp = false; if (!(f = fopen(path, "w"))) { rc = -errno; - goto cleanup; + return rc; } } @@ -192,7 +192,7 @@ addnhostsWrite(const char *path, if (istmp) unlink(tmp); - goto cleanup; + return rc; } for (j = 0; j < hosts[i].nhostnames; j++) { @@ -203,7 +203,7 @@ addnhostsWrite(const char *path, if (istmp) unlink(tmp); - goto cleanup; + return rc; } } @@ -214,24 +214,21 @@ addnhostsWrite(const char *path, if (istmp) unlink(tmp); - goto cleanup; + return rc; } } if (VIR_FCLOSE(f) == EOF) { rc = -errno; - goto cleanup; + return rc; } if (istmp && rename(tmp, path) < 0) { rc = -errno; unlink(tmp); - goto cleanup; + return rc; } - cleanup: - VIR_FREE(tmp); - return rc; } @@ -364,7 +361,7 @@ hostsfileWrite(const char *path, dnsmasqDhcpHost *hosts, unsigned int nhosts) { - char *tmp; + g_autofree char *tmp = NULL; FILE *f; bool istmp = true; size_t i; @@ -380,7 +377,7 @@ hostsfileWrite(const char *path, istmp = false; if (!(f = fopen(path, "w"))) { rc = -errno; - goto cleanup; + return rc; } } @@ -392,24 +389,21 @@ hostsfileWrite(const char *path, if (istmp) unlink(tmp); - goto cleanup; + return rc; } } if (VIR_FCLOSE(f) == EOF) { rc = -errno; - goto cleanup; + return rc; } if (istmp && rename(tmp, path) < 0) { rc = -errno; unlink(tmp); - goto cleanup; + return rc; } - cleanup: - VIR_FREE(tmp); - return rc; } @@ -686,15 +680,13 @@ static int dnsmasqCapsSetFromFile(dnsmasqCapsPtr caps, const char *path) { int ret = -1; - char *buf = NULL; + g_autofree char *buf = NULL; if (virFileReadAll(path, 1024 * 1024, &buf) < 0) - goto cleanup; + return ret; ret = dnsmasqCapsSetFromBuffer(caps, buf); - cleanup: - VIR_FREE(buf); return ret; } @@ -704,7 +696,9 @@ dnsmasqCapsRefreshInternal(dnsmasqCapsPtr caps, bool force) int ret = -1; struct stat sb; virCommandPtr cmd = NULL; - char *help = NULL, *version = NULL, *complete = NULL; + g_autofree char *help = NULL; + g_autofree char *version = NULL; + g_autofree char *complete = NULL; if (!caps || caps->noRefresh) return 0; @@ -749,9 +743,6 @@ dnsmasqCapsRefreshInternal(dnsmasqCapsPtr caps, bool force) cleanup: virCommandFree(cmd); - VIR_FREE(help); - VIR_FREE(version); - VIR_FREE(complete); return ret; } diff --git a/src/util/virfile.c b/src/util/virfile.c index f57272ca2f..38207f1948 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -3328,9 +3328,9 @@ virFileIsSharedFixFUSE(const char *path, FILE *f = NULL; struct mntent mb; char mntbuf[1024]; - char *mntDir = NULL; - char *mntType = NULL; - char *canonPath = NULL; + g_autofree char *mntDir = NULL; + g_autofree char *mntType = NULL; + g_autofree char *canonPath = NULL; size_t maxMatching = 0; int ret = -1; @@ -3381,9 +3381,6 @@ virFileIsSharedFixFUSE(const char *path, ret = 0; cleanup: - VIR_FREE(canonPath); - VIR_FREE(mntType); - VIR_FREE(mntDir); endmntent(f); return ret; } diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index c531d65f86..4f6c3390ce 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -87,7 +87,7 @@ virHostCPUGetStatsFreeBSD(int cpuNum, int *nparams) { const char *sysctl_name; - long *cpu_times; + g_autofree long *cpu_times = NULL; struct clockinfo clkinfo; size_t i, j, cpu_times_size, clkinfo_size; int cpu_times_num, offset, hz, stathz, ret = -1; @@ -172,8 +172,6 @@ virHostCPUGetStatsFreeBSD(int cpuNum, ret = 0; cleanup: - VIR_FREE(cpu_times); - return ret; } diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index b90e13f506..71d5dfb83e 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -515,7 +515,7 @@ int virLockSpaceCreateResource(virLockSpacePtr lockspace, const char *resname) { int ret = -1; - char *respath = NULL; + g_autofree char *respath = NULL; VIR_DEBUG("lockspace=%p resname=%s", lockspace, resname); @@ -538,7 +538,6 @@ int virLockSpaceCreateResource(virLockSpacePtr lockspace, cleanup: virMutexUnlock(&lockspace->lock); - VIR_FREE(respath); return ret; } @@ -547,7 +546,7 @@ int virLockSpaceDeleteResource(virLockSpacePtr lockspace, const char *resname) { int ret = -1; - char *respath = NULL; + g_autofree char *respath = NULL; VIR_DEBUG("lockspace=%p resname=%s", lockspace, resname); @@ -575,7 +574,6 @@ int virLockSpaceDeleteResource(virLockSpacePtr lockspace, cleanup: virMutexUnlock(&lockspace->lock); - VIR_FREE(respath); return ret; } diff --git a/src/util/virlog.c b/src/util/virlog.c index 6b7a4512e9..2bf606b8c5 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -505,8 +505,8 @@ virLogVMessage(virLogSourcePtr source, va_list vargs) { static bool logInitMessageStderr = true; - char *str = NULL; - char *msg = NULL; + g_autofree char *str = NULL; + g_autofree char *msg = NULL; char timestamp[VIR_TIME_STRING_BUFLEN]; size_t i; int saved_errno = errno; @@ -528,7 +528,8 @@ virLogVMessage(virLogSourcePtr source, if (source->serial < virLogFiltersSerial) virLogSourceUpdate(source); if (priority < source->priority) - goto cleanup; + errno = saved_errno; + return; /* * serialize the error message, add level and timestamp @@ -601,11 +602,6 @@ virLogVMessage(virLogSourcePtr source, str, msg, (void *) STDERR_FILENO); } virLogUnlock(); - - cleanup: - VIR_FREE(str); - VIR_FREE(msg); - errno = saved_errno; } diff --git a/src/util/virmacmap.c b/src/util/virmacmap.c index f9047d0fb1..e68742de10 100644 --- a/src/util/virmacmap.c +++ b/src/util/virmacmap.c @@ -129,7 +129,7 @@ static int virMacMapLoadFile(virMacMapPtr mgr, const char *file) { - char *map_str = NULL; + g_autofree char *map_str = NULL; virJSONValuePtr map = NULL; int map_str_len = 0; size_t i; @@ -189,7 +189,6 @@ virMacMapLoadFile(virMacMapPtr mgr, ret = 0; cleanup: - VIR_FREE(map_str); virJSONValueFree(map); return ret; } diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c index c8eb5cfc79..2ae03e8edc 100644 --- a/src/util/virnetdevbandwidth.c +++ b/src/util/virnetdevbandwidth.c @@ -102,7 +102,7 @@ virNetDevBandwidthManipulateFilter(const char *ifname, bool create_new) { int ret = -1; - char *filter_id = NULL; + g_autofree char *filter_id = NULL; virCommandPtr cmd = NULL; unsigned char ifmac[VIR_MAC_BUFLEN]; char *mac[2] = {NULL, NULL}; @@ -157,7 +157,6 @@ virNetDevBandwidthManipulateFilter(const char *ifname, cleanup: VIR_FREE(mac[1]); VIR_FREE(mac[0]); - VIR_FREE(filter_id); virCommandFree(cmd); return ret; } @@ -195,9 +194,9 @@ virNetDevBandwidthSet(const char *ifname, int ret = -1; virNetDevBandwidthRatePtr rx = NULL, tx = NULL; /* From domain POV */ virCommandPtr cmd = NULL; - char *average = NULL; - char *peak = NULL; - char *burst = NULL; + g_autofree char *average = NULL; + g_autofree char *peak = NULL; + g_autofree char *burst = NULL; if (!bandwidth) { /* nothing to be enabled */ @@ -385,9 +384,6 @@ virNetDevBandwidthSet(const char *ifname, cleanup: virCommandFree(cmd); - VIR_FREE(average); - VIR_FREE(peak); - VIR_FREE(burst); return ret; } @@ -533,10 +529,10 @@ virNetDevBandwidthPlug(const char *brname, { int ret = -1; virCommandPtr cmd = NULL; - char *class_id = NULL; - char *qdisc_id = NULL; - char *floor = NULL; - char *ceil = NULL; + g_autofree char *class_id = NULL; + g_autofree char *qdisc_id = NULL; + g_autofree char *floor = NULL; + g_autofree char *ceil = NULL; char ifmacStr[VIR_MAC_STRING_BUFLEN]; if (id <= 2) { @@ -586,10 +582,6 @@ virNetDevBandwidthPlug(const char *brname, ret = 0; cleanup: - VIR_FREE(ceil); - VIR_FREE(floor); - VIR_FREE(qdisc_id); - VIR_FREE(class_id); virCommandFree(cmd); return ret; } @@ -610,8 +602,8 @@ virNetDevBandwidthUnplug(const char *brname, int ret = -1; int cmd_ret = 0; virCommandPtr cmd = NULL; - char *class_id = NULL; - char *qdisc_id = NULL; + g_autofree char *class_id = NULL; + g_autofree char *qdisc_id = NULL; if (id <= 2) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid class ID %d"), id); @@ -645,8 +637,6 @@ virNetDevBandwidthUnplug(const char *brname, ret = 0; cleanup: - VIR_FREE(qdisc_id); - VIR_FREE(class_id); virCommandFree(cmd); return ret; } @@ -673,9 +663,9 @@ virNetDevBandwidthUpdateRate(const char *ifname, { int ret = -1; virCommandPtr cmd = NULL; - char *class_id = NULL; - char *rate = NULL; - char *ceil = NULL; + g_autofree char *class_id = NULL; + g_autofree char *rate = NULL; + g_autofree char *ceil = NULL; class_id = g_strdup_printf("1:%x", id); rate = g_strdup_printf("%llukbps", new_rate); @@ -696,9 +686,6 @@ virNetDevBandwidthUpdateRate(const char *ifname, cleanup: virCommandFree(cmd); - VIR_FREE(class_id); - VIR_FREE(rate); - VIR_FREE(ceil); return ret; } @@ -725,16 +712,13 @@ virNetDevBandwidthUpdateFilter(const char *ifname, unsigned int id) { int ret = -1; - char *class_id = NULL; + g_autofree char *class_id = NULL; class_id = g_strdup_printf("1:%x", id); if (virNetDevBandwidthManipulateFilter(ifname, ifmac_ptr, id, class_id, true, true) < 0) - goto cleanup; + return ret; - ret = 0; - cleanup: - VIR_FREE(class_id); - return ret; + return 0; } diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index d3087b98c1..1c2d175295 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -709,7 +709,7 @@ virResctrlGetMonitorInfo(virResctrlInfoPtr resctrl) { int ret = -1; int rv = -1; - char *featurestr = NULL; + g_autofree char *featurestr = NULL; char **features = NULL; size_t nfeatures = 0; virResctrlInfoMongrpPtr info_monitor = NULL; @@ -771,7 +771,6 @@ virResctrlGetMonitorInfo(virResctrlInfoPtr resctrl) ret = 0; cleanup: - VIR_FREE(featurestr); g_strfreev(features); VIR_FREE(info_monitor); return ret; @@ -1736,7 +1735,7 @@ virResctrlAllocGetGroup(virResctrlInfoPtr resctrl, const char *groupname, virResctrlAllocPtr *alloc) { - char *schemata = NULL; + g_autofree char *schemata = NULL; int rv = virFileReadValueString(&schemata, SYSFS_RESCTRL_PATH "/%s/schemata", groupname); @@ -1753,11 +1752,9 @@ virResctrlAllocGetGroup(virResctrlInfoPtr resctrl, if (virResctrlAllocParse(resctrl, *alloc, schemata) < 0) goto error; - VIR_FREE(schemata); return 0; error: - VIR_FREE(schemata); virObjectUnref(*alloc); *alloc = NULL; return -1; @@ -2354,8 +2351,8 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl, virResctrlAllocPtr alloc, const char *machinename) { - char *schemata_path = NULL; - char *alloc_str = NULL; + g_autofree char *schemata_path = NULL; + g_autofree char *alloc_str = NULL; int ret = -1; int lockfd = -1; @@ -2403,8 +2400,6 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl, ret = 0; cleanup: virResctrlUnlock(lockfd); - VIR_FREE(alloc_str); - VIR_FREE(schemata_path); return ret; } @@ -2413,8 +2408,8 @@ static int virResctrlAddPID(const char *path, pid_t pid) { - char *tasks = NULL; - char *pidstr = NULL; + g_autofree char *tasks = NULL; + g_autofree char *pidstr = NULL; int ret = 0; if (!path) { @@ -2436,8 +2431,6 @@ virResctrlAddPID(const char *path, ret = 0; cleanup: - VIR_FREE(tasks); - VIR_FREE(pidstr); return ret; } @@ -2657,8 +2650,8 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor, size_t i = 0; unsigned long long val = 0; g_autoptr(DIR) dirp = NULL; - char *datapath = NULL; - char *filepath = NULL; + g_autofree char *datapath = NULL; + g_autofree char *filepath = NULL; struct dirent *ent = NULL; virResctrlMonitorStatsPtr stat = NULL; @@ -2737,8 +2730,6 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor, ret = 0; cleanup: - VIR_FREE(datapath); - VIR_FREE(filepath); virResctrlMonitorStatsFree(stat); return ret; } diff --git a/src/util/virrotatingfile.c b/src/util/virrotatingfile.c index 9f1ef17c3e..6fe5fa3154 100644 --- a/src/util/virrotatingfile.c +++ b/src/util/virrotatingfile.c @@ -362,8 +362,8 @@ static int virRotatingFileWriterRollover(virRotatingFileWriterPtr file) { size_t i; - char *nextpath = NULL; - char *thispath = NULL; + g_autofree char *nextpath = NULL; + g_autofree char *thispath = NULL; int ret = -1; VIR_DEBUG("Rollover %s", file->basepath); @@ -373,7 +373,7 @@ virRotatingFileWriterRollover(virRotatingFileWriterPtr file) virReportSystemError(errno, _("Unable to remove %s"), file->basepath); - goto cleanup; + return ret; } } else { nextpath = g_strdup_printf("%s.%zu", file->basepath, file->maxbackup - 1); @@ -391,7 +391,7 @@ virRotatingFileWriterRollover(virRotatingFileWriterPtr file) virReportSystemError(errno, _("Unable to rename %s to %s"), thispath, nextpath); - goto cleanup; + return ret; } VIR_FREE(nextpath); @@ -402,9 +402,6 @@ virRotatingFileWriterRollover(virRotatingFileWriterPtr file) VIR_DEBUG("Rollover done %s", file->basepath); ret = 0; - cleanup: - VIR_FREE(nextpath); - VIR_FREE(thispath); return ret; } diff --git a/src/util/virscsihost.c b/src/util/virscsihost.c index 969cdd9f79..cad48b4059 100644 --- a/src/util/virscsihost.c +++ b/src/util/virscsihost.c @@ -95,12 +95,12 @@ virSCSIHostFindByPCI(const char *sysfs_prefix, const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_HOST_PATH; struct dirent *entry = NULL; g_autoptr(DIR) dir = NULL; - char *host_link = NULL; - char *host_path = NULL; + g_autofree char *host_link = NULL; + g_autofree char *host_path = NULL; char *p = NULL; char *ret = NULL; - char *buf = NULL; - char *unique_path = NULL; + g_autofree char *buf = NULL; + g_autofree char *unique_path = NULL; unsigned int read_unique_id; if (virDirOpen(&dir, prefix) < 0) @@ -113,7 +113,7 @@ virSCSIHostFindByPCI(const char *sysfs_prefix, host_link = g_strdup_printf("%s/%s", prefix, entry->d_name); if (virFileResolveLink(host_link, &host_path) < 0) - goto cleanup; + return ret; if (!strstr(host_path, parentaddr)) { VIR_FREE(host_link); @@ -131,13 +131,13 @@ virSCSIHostFindByPCI(const char *sysfs_prefix, } if (virFileReadAll(unique_path, 1024, &buf) < 0) - goto cleanup; + return ret; if ((p = strchr(buf, '\n'))) *p = '\0'; if (virStrToLong_ui(buf, NULL, 10, &read_unique_id) < 0) - goto cleanup; + return ret; VIR_FREE(buf); @@ -150,11 +150,6 @@ virSCSIHostFindByPCI(const char *sysfs_prefix, break; } - cleanup: - VIR_FREE(unique_path); - VIR_FREE(host_link); - VIR_FREE(host_path); - VIR_FREE(buf); return ret; } @@ -226,7 +221,7 @@ virSCSIHostGetNameByParentaddr(unsigned int domain, unsigned int unique_id) { char *name = NULL; - char *parentaddr = NULL; + g_autofree char *parentaddr = NULL; parentaddr = g_strdup_printf("%04x:%02x:%02x.%01x", domain, bus, slot, function); @@ -235,11 +230,9 @@ virSCSIHostGetNameByParentaddr(unsigned int domain, _("Failed to find scsi_host using PCI '%s' " "and unique_id='%u'"), parentaddr, unique_id); - goto cleanup; + return name; } - cleanup: - VIR_FREE(parentaddr); return name; } diff --git a/src/util/virsecret.c b/src/util/virsecret.c index 54d6bbcb7c..67c9b68215 100644 --- a/src/util/virsecret.c +++ b/src/util/virsecret.c @@ -65,8 +65,8 @@ int virSecretLookupParseSecret(xmlNodePtr secretnode, virSecretLookupTypeDefPtr def) { - char *uuid; - char *usage; + g_autofree char *uuid = NULL; + g_autofree char *usage = NULL; int ret = -1; uuid = virXMLPropString(secretnode, "uuid"); @@ -74,20 +74,20 @@ virSecretLookupParseSecret(xmlNodePtr secretnode, if (uuid == NULL && usage == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing secret uuid or usage attribute")); - goto cleanup; + return ret; } if (uuid && usage) { virReportError(VIR_ERR_XML_ERROR, "%s", _("either secret uuid or usage expected")); - goto cleanup; + return ret; } if (uuid) { if (virUUIDParse(uuid, def->u.uuid) < 0) { virReportError(VIR_ERR_XML_ERROR, _("invalid secret uuid '%s'"), uuid); - goto cleanup; + return ret; } def->type = VIR_SECRET_LOOKUP_TYPE_UUID; } else { @@ -96,10 +96,6 @@ virSecretLookupParseSecret(xmlNodePtr secretnode, def->type = VIR_SECRET_LOOKUP_TYPE_USAGE; } ret = 0; - - cleanup: - VIR_FREE(uuid); - VIR_FREE(usage); return ret; } diff --git a/src/util/virstorageencryption.c b/src/util/virstorageencryption.c index 399c6926bd..db886c1e76 100644 --- a/src/util/virstorageencryption.c +++ b/src/util/virstorageencryption.c @@ -142,7 +142,7 @@ virStorageEncryptionSecretParse(xmlXPathContextPtr ctxt, { VIR_XPATH_NODE_AUTORESTORE(ctxt) virStorageEncryptionSecretPtr ret; - char *type_str = NULL; + g_autofree char *type_str = NULL; ret = g_new0(virStorageEncryptionSecret, 1); @@ -164,12 +164,9 @@ virStorageEncryptionSecretParse(xmlXPathContextPtr ctxt, if (virSecretLookupParseSecret(node, &ret->seclookupdef) < 0) goto cleanup; - VIR_FREE(type_str); - return ret; cleanup: - VIR_FREE(type_str); virStorageEncryptionSecretFree(ret); return NULL; } @@ -180,12 +177,12 @@ virStorageEncryptionInfoParseCipher(xmlNodePtr info_node, virStorageEncryptionInfoDefPtr info) { int ret = -1; - char *size_str = NULL; + g_autofree char *size_str = NULL; if (!(info->cipher_name = virXMLPropString(info_node, "name"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("cipher info missing 'name' attribute")); - goto cleanup; + return ret; } if ((size_str = virXMLPropString(info_node, "size")) && @@ -193,22 +190,19 @@ virStorageEncryptionInfoParseCipher(xmlNodePtr info_node, virReportError(VIR_ERR_XML_ERROR, _("cannot parse cipher size: '%s'"), size_str); - goto cleanup; + return ret; } if (!size_str) { virReportError(VIR_ERR_XML_ERROR, "%s", _("cipher info missing 'size' attribute")); - goto cleanup; + return ret; } info->cipher_mode = virXMLPropString(info_node, "mode"); info->cipher_hash = virXMLPropString(info_node, "hash"); ret = 0; - - cleanup: - VIR_FREE(size_str); return ret; } @@ -237,7 +231,7 @@ virStorageEncryptionParseNode(xmlNodePtr node, xmlNodePtr *nodes = NULL; virStorageEncryptionPtr encdef = NULL; virStorageEncryptionPtr ret = NULL; - char *format_str = NULL; + g_autofree char *format_str = NULL; int n; size_t i; @@ -297,7 +291,6 @@ virStorageEncryptionParseNode(xmlNodePtr node, ret = g_steal_pointer(&encdef); cleanup: - VIR_FREE(format_str); VIR_FREE(nodes); virStorageEncryptionFree(encdef); diff --git a/src/util/virstoragefilebackend.c b/src/util/virstoragefilebackend.c index 2779b5c307..55c62b0212 100644 --- a/src/util/virstoragefilebackend.c +++ b/src/util/virstoragefilebackend.c @@ -51,7 +51,7 @@ virStorageFileLoadBackendModule(const char *name, const char *regfunc, bool forceload) { - char *modfile = NULL; + g_autofree char *modfile = NULL; int ret; if (!(modfile = virFileFindResourceFull(name, @@ -64,8 +64,6 @@ virStorageFileLoadBackendModule(const char *name, ret = virModuleLoad(modfile, regfunc, forceload); - VIR_FREE(modfile); - return ret; } #endif /* WITH_STORAGE_DIR || WITH_STORAGE_FS || WITH_STORAGE_GLUSTER */ diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 217f842a37..45a950c85a 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -393,7 +393,7 @@ virSysinfoParseARMProcessor(const char *base, virSysinfoDefPtr ret) const char *cur; char *eol, *tmp_base; virSysinfoProcessorDefPtr processor; - char *processor_type = NULL; + g_autofree char *processor_type = NULL; if (!(tmp_base = strstr(base, "model name")) && !(tmp_base = strstr(base, "Processor"))) @@ -411,7 +411,7 @@ virSysinfoParseARMProcessor(const char *base, virSysinfoDefPtr ret) cur = strchr(base, ':') + 1; if (VIR_EXPAND_N(ret->processor, ret->nprocessor, 1) < 0) - goto error; + return -1; processor = &ret->processor[ret->nprocessor - 1]; virSkipSpaces(&cur); @@ -424,12 +424,7 @@ virSysinfoParseARMProcessor(const char *base, virSysinfoDefPtr ret) base = cur; } - VIR_FREE(processor_type); return 0; - - error: - VIR_FREE(processor_type); - return -1; } /* virSysinfoRead for ARMv7 @@ -532,9 +527,9 @@ static int virSysinfoParseS390Processor(const char *base, virSysinfoDefPtr ret) { const char *tmp_base; - char *manufacturer = NULL; - char *procline = NULL; - char *ncpu = NULL; + g_autofree char *manufacturer = NULL; + g_autofree char *procline = NULL; + g_autofree char *ncpu = NULL; int result = -1; virSysinfoProcessorDefPtr processor; @@ -593,9 +588,6 @@ virSysinfoParseS390Processor(const char *base, virSysinfoDefPtr ret) result = 0; error: - VIR_FREE(manufacturer); - VIR_FREE(procline); - VIR_FREE(ncpu); return result; } diff --git a/src/util/viruri.c b/src/util/viruri.c index 11753a0aef..d49821451e 100644 --- a/src/util/viruri.c +++ b/src/util/viruri.c @@ -209,7 +209,7 @@ char * virURIFormat(virURIPtr uri) { xmlURI xmluri; - char *tmpserver = NULL; + g_autofree char *tmpserver = NULL; char *ret; memset(&xmluri, 0, sizeof(xmluri)); @@ -241,12 +241,9 @@ virURIFormat(virURIPtr uri) ret = (char *)xmlSaveUri(&xmluri); if (!ret) { virReportOOMError(); - goto cleanup; + return ret; } - cleanup: - VIR_FREE(tmpserver); - return ret; } diff --git a/src/util/virutil.c b/src/util/virutil.c index a0cd0f1bcd..a7c163ab94 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -598,7 +598,7 @@ char *virGetUserRuntimeDirectory(void) static int virGetUserEnt(uid_t uid, char **name, gid_t *group, char **dir, char **shell, bool quiet) { - char *strbuf; + g_autofree char *strbuf = NULL; struct passwd pwbuf; struct passwd *pw = NULL; long val = sysconf(_SC_GETPW_R_SIZE_MAX); @@ -668,13 +668,12 @@ virGetUserEnt(uid_t uid, char **name, gid_t *group, char **dir, char **shell, bo if (shell) VIR_FREE(*shell); } - VIR_FREE(strbuf); return ret; } static char *virGetGroupEnt(gid_t gid) { - char *strbuf; + g_autofree char *strbuf = NULL; char *ret; struct group grbuf; struct group *gr = NULL; @@ -717,7 +716,6 @@ static char *virGetGroupEnt(gid_t gid) } ret = g_strdup(gr->gr_name); - VIR_FREE(strbuf); return ret; } @@ -759,7 +757,7 @@ char *virGetGroupName(gid_t gid) static int virGetUserIDByName(const char *name, uid_t *uid, bool missing_ok) { - char *strbuf = NULL; + g_autofree char *strbuf = NULL; struct passwd pwbuf; struct passwd *pw = NULL; long val = sysconf(_SC_GETPW_R_SIZE_MAX); @@ -775,7 +773,7 @@ virGetUserIDByName(const char *name, uid_t *uid, bool missing_ok) while ((rc = getpwnam_r(name, &pwbuf, strbuf, strbuflen, &pw)) == ERANGE) { if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) - goto cleanup; + return ret; } if (!pw) { @@ -788,16 +786,13 @@ virGetUserIDByName(const char *name, uid_t *uid, bool missing_ok) } ret = 1; - goto cleanup; + return ret; } if (uid) *uid = pw->pw_uid; ret = 0; - cleanup: - VIR_FREE(strbuf); - return ret; } @@ -840,7 +835,7 @@ virGetUserID(const char *user, uid_t *uid) static int virGetGroupIDByName(const char *name, gid_t *gid, bool missing_ok) { - char *strbuf = NULL; + g_autofree char *strbuf = NULL; struct group grbuf; struct group *gr = NULL; long val = sysconf(_SC_GETGR_R_SIZE_MAX); @@ -856,7 +851,7 @@ virGetGroupIDByName(const char *name, gid_t *gid, bool missing_ok) while ((rc = getgrnam_r(name, &grbuf, strbuf, strbuflen, &gr)) == ERANGE) { if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) - goto cleanup; + return ret; } if (!gr) { @@ -869,16 +864,13 @@ virGetGroupIDByName(const char *name, gid_t *gid, bool missing_ok) } ret = 1; - goto cleanup; + return ret; } if (gid) *gid = gr->gr_gid; ret = 0; - cleanup: - VIR_FREE(strbuf); - return ret; } @@ -949,7 +941,7 @@ int virGetGroupList(uid_t uid, gid_t gid, gid_t **list) { int ret = 0; - char *user = NULL; + g_autofree char *user = NULL; gid_t primary; *list = NULL; @@ -987,19 +979,17 @@ virGetGroupList(uid_t uid, gid_t gid, gid_t **list) for (i = 0; i < ret; i++) { if ((*list)[i] == gid) - goto cleanup; + return ret; } if (VIR_APPEND_ELEMENT(*list, i, gid) < 0) { ret = -1; VIR_FREE(*list); - goto cleanup; + return ret; } else { ret = i; } } - cleanup: - VIR_FREE(user); return ret; } @@ -1405,8 +1395,8 @@ virSetDeviceUnprivSGIO(const char *path, const char *sysfs_dir, int unpriv_sgio) { - char *sysfs_path = NULL; - char *val = NULL; + g_autofree char *sysfs_path = NULL; + g_autofree char *val = NULL; int ret = -1; int rc; @@ -1416,20 +1406,17 @@ virSetDeviceUnprivSGIO(const char *path, if (!virFileExists(sysfs_path)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unpriv_sgio is not supported by this kernel")); - goto cleanup; + return ret; } val = g_strdup_printf("%d", unpriv_sgio); if ((rc = virFileWriteStr(sysfs_path, val, 0)) < 0) { virReportSystemError(-rc, _("failed to set %s"), sysfs_path); - goto cleanup; + return ret; } ret = 0; - cleanup: - VIR_FREE(sysfs_path); - VIR_FREE(val); return ret; } @@ -1438,8 +1425,8 @@ virGetDeviceUnprivSGIO(const char *path, const char *sysfs_dir, int *unpriv_sgio) { - char *sysfs_path = NULL; - char *buf = NULL; + g_autofree char *sysfs_path = NULL; + g_autofree char *buf = NULL; char *tmp = NULL; int ret = -1; @@ -1449,11 +1436,11 @@ virGetDeviceUnprivSGIO(const char *path, if (!virFileExists(sysfs_path)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unpriv_sgio is not supported by this kernel")); - goto cleanup; + return ret; } if (virFileReadAll(sysfs_path, 1024, &buf) < 0) - goto cleanup; + return ret; if ((tmp = strchr(buf, '\n'))) *tmp = '\0'; @@ -1461,13 +1448,10 @@ virGetDeviceUnprivSGIO(const char *path, if (virStrToLong_i(buf, NULL, 10, unpriv_sgio) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to parse value of %s"), sysfs_path); - goto cleanup; + return ret; } ret = 0; - cleanup: - VIR_FREE(sysfs_path); - VIR_FREE(buf); return ret; } @@ -1488,7 +1472,7 @@ virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) int rc = -1; uid_t theuid; gid_t thegid; - char *tmp_label = NULL; + g_autofree char *tmp_label = NULL; char *sep = NULL; char *owner = NULL; char *group = NULL; @@ -1501,7 +1485,7 @@ virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) virReportError(VIR_ERR_INVALID_ARG, _("Failed to parse uid and gid from '%s'"), label); - goto cleanup; + return rc; } *sep = '\0'; owner = tmp_label; @@ -1512,7 +1496,7 @@ virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) */ if (virGetUserID(owner, &theuid) < 0 || virGetGroupID(group, &thegid) < 0) - goto cleanup; + return rc; if (uidPtr) *uidPtr = theuid; @@ -1520,10 +1504,6 @@ virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) *gidPtr = thegid; rc = 0; - - cleanup: - VIR_FREE(tmp_label); - return rc; } diff --git a/src/util/virvhba.c b/src/util/virvhba.c index a4e88024d1..d8642ea041 100644 --- a/src/util/virvhba.c +++ b/src/util/virvhba.c @@ -49,7 +49,7 @@ bool virVHBAPathExists(const char *sysfs_prefix, int host) { - char *sysfs_path = NULL; + g_autofree char *sysfs_path = NULL; bool ret = false; sysfs_path = g_strdup_printf("%s/host%d", @@ -58,7 +58,6 @@ virVHBAPathExists(const char *sysfs_prefix, if (virFileExists(sysfs_path)) ret = true; - VIR_FREE(sysfs_path); return ret; } @@ -79,8 +78,8 @@ bool virVHBAIsVportCapable(const char *sysfs_prefix, int host) { - char *scsi_host_path = NULL; - char *fc_host_path = NULL; + g_autofree char *scsi_host_path = NULL; + g_autofree char *fc_host_path = NULL; bool ret = false; fc_host_path = g_strdup_printf("%s/host%d/%s", @@ -94,8 +93,6 @@ virVHBAIsVportCapable(const char *sysfs_prefix, if (virFileExists(fc_host_path) || virFileExists(scsi_host_path)) ret = true; - VIR_FREE(fc_host_path); - VIR_FREE(scsi_host_path); return ret; } @@ -115,19 +112,19 @@ virVHBAGetConfig(const char *sysfs_prefix, int host, const char *entry) { - char *sysfs_path = NULL; + g_autofree char *sysfs_path = NULL; char *p = NULL; - char *buf = NULL; + g_autofree char *buf = NULL; char *result = NULL; sysfs_path = g_strdup_printf("%s/host%d/%s", sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH, host, entry); if (!virFileExists(sysfs_path)) - goto cleanup; + return result; if (virFileReadAll(sysfs_path, 1024, &buf) < 0) - goto cleanup; + return result; if ((p = strchr(buf, '\n'))) *p = '\0'; @@ -139,9 +136,6 @@ virVHBAGetConfig(const char *sysfs_prefix, result = g_strdup(p); - cleanup: - VIR_FREE(sysfs_path); - VIR_FREE(buf); return result; } @@ -160,8 +154,8 @@ virVHBAFindVportHost(const char *sysfs_prefix) const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH; g_autoptr(DIR) dir = NULL; struct dirent *entry = NULL; - char *max_vports = NULL; - char *vports = NULL; + g_autofree char *max_vports = NULL; + g_autofree char *vports = NULL; char *state = NULL; char *ret = NULL; @@ -212,16 +206,13 @@ virVHBAFindVportHost(const char *sysfs_prefix) ((strlen(max_vports) == strlen(vports)) && strcmp(max_vports, vports) > 0)) { ret = g_strdup(entry->d_name); - goto cleanup; + return ret; } VIR_FREE(max_vports); VIR_FREE(vports); } - cleanup: - VIR_FREE(max_vports); - VIR_FREE(vports); return ret; } @@ -241,7 +232,8 @@ virVHBAManageVport(const int parent_host, int operation) { int ret = -1; - char *operation_path = NULL, *vport_name = NULL; + g_autofree char *operation_path = NULL; + g_autofree char *vport_name = NULL; const char *operation_file = NULL; switch (operation) { @@ -254,7 +246,7 @@ virVHBAManageVport(const int parent_host, default: virReportError(VIR_ERR_OPERATION_INVALID, _("Invalid vport operation (%d)"), operation); - goto cleanup; + return ret; } operation_path = g_strdup_printf("%s/host%d/%s", SYSFS_FC_HOST_PATH, @@ -270,7 +262,7 @@ virVHBAManageVport(const int parent_host, _("vport operation '%s' is not supported " "for host%d"), operation_file, parent_host); - goto cleanup; + return ret; } } @@ -290,9 +282,6 @@ virVHBAManageVport(const int parent_host, "vport create/delete failed"), vport_name, operation_path); - cleanup: - VIR_FREE(vport_name); - VIR_FREE(operation_path); return ret; } @@ -315,8 +304,8 @@ vhbaReadCompareWWN(const char *prefix, const char *f_name, const char *wwn) { - char *path; - char *buf = NULL; + g_autofree char *path = NULL; + g_autofree char *buf = NULL; char *p; int ret = -1; @@ -324,11 +313,11 @@ vhbaReadCompareWWN(const char *prefix, if (!virFileExists(path)) { ret = 0; - goto cleanup; + return ret; } if (virFileReadAll(path, 1024, &buf) < 0) - goto cleanup; + return ret; if ((p = strchr(buf, '\n'))) *p = '\0'; @@ -342,10 +331,6 @@ vhbaReadCompareWWN(const char *prefix, else ret = 1; - cleanup: - VIR_FREE(path); - VIR_FREE(buf); - return ret; } @@ -407,7 +392,7 @@ virVHBAGetHostByFabricWWN(const char *sysfs_prefix, const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH; struct dirent *entry = NULL; g_autoptr(DIR) dir = NULL; - char *vport_create_path = NULL; + g_autofree char *vport_create_path = NULL; char *ret = NULL; if (virDirOpen(&dir, prefix) < 0) @@ -428,7 +413,7 @@ virVHBAGetHostByFabricWWN(const char *sysfs_prefix, if ((rc = vhbaReadCompareWWN(prefix, entry->d_name, "fabric_name", fabric_wwn)) < 0) - goto cleanup; + return ret; if (rc == 0) continue; @@ -437,8 +422,6 @@ virVHBAGetHostByFabricWWN(const char *sysfs_prefix, break; } - cleanup: - VIR_FREE(vport_create_path); return ret; } diff --git a/src/util/virxml.c b/src/util/virxml.c index a3b819d85c..7df50e4b4d 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -696,8 +696,8 @@ catchXMLError(void *ctx, const char *msg G_GNUC_UNUSED, ...) unsigned int n, col; /* GCC warns if signed, because compared with sizeof() */ int domcode = VIR_FROM_XML; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - char *contextstr = NULL; - char *pointerstr = NULL; + g_autofree char *contextstr = NULL; + g_autofree char *pointerstr = NULL; /* conditions for error printing */ @@ -763,9 +763,6 @@ catchXMLError(void *ctx, const char *msg G_GNUC_UNUSED, ...) contextstr, pointerstr); } - - VIR_FREE(contextstr); - VIR_FREE(pointerstr); } /** -- 2.29.0

4 5

Libvirt NVME support
by Suraj Kasi 24 Nov '20

24 Nov '20

Hi, We wanted to check if it’s possible to specify a disk’s target as nvme (so that the disk shows up as a nvme disk to the guest VM). Per libvirt documentation it looks like (since Libvirt 6.0.0) we can specify the disk type as nvme and disks source as a nvme. But the documentation does not say anything about being specify the disk’s target as nvme. Is it possible to present the disk to the guest as a nvme disk, if so how? Example from Libvirt documentation (https://libvirt.org/formatdomain.html) ----- <disk type='nvme' device='disk'> <driver name='qemu' type='raw'/> <source type='pci' managed='yes' namespace='1'> <address domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> </source> <target dev='vde' bus='virtio'/> </disk> ----- But we want to do something similar to the following: two vNVMe controllers where the first one has one namespace and the second one has two namespaces ----- # first NVMe controller, one namespace <disk type='network' device='disk'> <driver name='qemu' type='raw'/> <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/123'> <host name='example.com' port='3260'/> <auth username='myuser'> <secret type='iscsi' usage='libvirtiscsi'/> </auth> </source> <target dev='nvme0' bus='nvme'/> </disk> # second NVMe controller, first namespace <disk type='network' device='disk'> <driver name='qemu' type='raw'/> <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/456'> <host name='example.com' port='3260'/> <auth username='myuser'> <secret type='iscsi' usage='libvirtiscsi'/> </auth> </source> <target dev='nvme1' namespace='1' bus='nvme'/> </disk> # second NVMe controller, first namespace <disk type='network' device='disk'> <driver name='qemu' type='raw'/> <source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/789'> <host name='example.com' port='3260'/> <auth username='myuser'> <secret type='iscsi' usage='libvirtiscsi'/> </auth> </source> <target dev='nvme1' namespace='2' bus='nvme'/> </disk> ----- If in case this is not yet supported, would it be merged if we were to implement it? Thanks, Suraj

6 26