[PATCH 6/7] qemu: plug config from into qemu commandline
Signed-off-by: Laine Stump --- src/qemu/qemu_command.c | 15 --- .../net-virtio-teaming-hostdev.args | 40 +++ tests/qemuxml2argvtest.c | 3 ++ 3 files changed, 53 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/net-virtio-teaming-hostdev.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 09516d407f..30663c933e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4427,6 +4427,7 @@ qemuBuildPCIHostdevDevStr(const virDomainDef *def, g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci; int backend = pcisrc->backend; +virDomainNetTeamingInfoPtr teaming; /* caller has to assign proper passthrough backend type */ switch ((virDomainHostdevSubsysPCIBackendType)backend) { @@ -4461,11 +4462,15 @@ qemuBuildPCIHostdevDevStr(const virDomainDef *def, if (qemuBuildRomStr(&buf, dev->info) < 0) return NULL; -if (dev->parentnet && dev->parentnet->teaming && -dev->parentnet->teaming->type == VIR_DOMAIN_NET_TEAMING_TYPE_TRANSIENT && -dev->parentnet->teaming->persistent) { -virBufferAsprintf(&buf, ",failover_pair_id=%s", - dev->parentnet->teaming->persistent); +if (dev->parentnet) +teaming = dev->parentnet->teaming; +else +teaming = dev->teaming; + +if (teaming && +teaming->type == VIR_DOMAIN_NET_TEAMING_TYPE_TRANSIENT && +teaming->persistent) { +virBufferAsprintf(&buf, ",failover_pair_id=%s", teaming->persistent); } return virBufferContentAndReset(&buf); diff --git a/tests/qemuxml2argvdata/net-virtio-teaming-hostdev.args b/tests/qemuxml2argvdata/net-virtio-teaming-hostdev.args new file mode 100644 index 00..19e7260843 --- /dev/null +++ b/tests/qemuxml2argvdata/net-virtio-teaming-hostdev.args @@ -0,0 +1,40 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i386 \ +-name QEMUGuest1 \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-realtime mlock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ +-netdev user,id=hostua-backup0 \ +-device virtio-net-pci,failover=on,netdev=hostua-backup0,id=ua-backup0,\ +mac=00:11:22:33:44:55,bus=pci.0,addr=0x3 \ +-netdev user,id=hostua-backup1 \ +-device virtio-net-pci,failover=on,netdev=hostua-backup1,id=ua-backup1,\ +mac=66:44:33:22:11:00,bus=pci.0,addr=0x4 \ +-device vfio-pci,host=:03:07.1,id=hostdev0,bus=pci.0,addr=0x5,\ +failover_pair_id=ua-backup0 \ +-device vfio-pci,host=:03:07.2,id=hostdev1,bus=pci.0,addr=0x6,\ +failover_pair_id=ua-backup1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x7 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index faa71a7a16..93d6a608bf 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1503,6 +1503,9 @@ mymain(void) QEMU_CAPS_VIRTIO_NET_FAILOVER, QEMU_CAPS_DEVICE_VFIO_PCI); DO_TEST_PARSE_ERROR("net-virtio-teaming", NONE); +DO_TEST("net-virtio-teaming-hostdev", +QEMU_CAPS_VIRTIO_NET_FAILOVER, +QEMU_CAPS_DEVICE_VFIO_PCI); DO_TEST("net-eth", NONE); DO_TEST("net-eth-ifname", NONE); DO_TEST("net-eth-names", NONE); -- 2.29.2
Re: [PATCH] qemu: match alias when looking for proper to detach.
On Wed, Feb 10, 2021 at 04:33:21PM -0500, Laine Stump wrote: Previously we only checked MAC address and PCI address (or CCW address). This is not enough information in cases where PCI address isn't provided and multiple interfaces have the same MAC address (for example, a virtio + hostdev "teaming" pair - their MAC addresses are always the same). Resolves: https://bugzilla.redhat.com/1926190 Signed-off-by: Laine Stump --- Arguably, it would be nice to overhaul the device matching used for virDomainDeviceDetach for *all* the device types, as they could all be matched by looking at alias (and PCI address, for that matter). On the other hand, for all other device types there are already enough fields being matched to assure a unique match even without looking at alias/PCI address, and this patch is intended to fix a current problem being experienced in the wild, meaning it's likely that it will need to be backported to stable branches, and I'd rather not force backporting a sweeping change to stable branches just to bring in a fix for one corner case) At first I thought this could be taken into account if and only if there were multiple mac addresses, but it looks like all the other cases that could fail are already taken care of, so it makes sense the way it is written. Reviewed-by: Martin Kletzander src/conf/domain_conf.c | 5 + 1 file changed, 5 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 07e6f39256..8f2207bdf6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16431,6 +16431,11 @@ virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net) &net->info.addr.ccw)) continue; +if (net->info.alias && +STRNEQ_NULLABLE(def->nets[i]->info.alias, net->info.alias)) { +continue; +} + if (matchidx >= 0) { /* there were multiple matches on mac address, and no * qualifying guest-side PCI/CCW address was given, so we must -- 2.29.2 signature.asc Description: PGP signature
Re: [PATCH 2/2] qemu: Validate TPM TIS device
On Wed, 2021-02-10 at 10:34 -0700, Jim Fehlig wrote: > I aslo worry about future architectures gaining support > for emulated TPM TIS devices. It's okay to be conservative - we can always relax the check later. > +if (!ARCH_IS_X86(def->os.arch) && (def->os.arch != > VIR_ARCH_AARCH64)) { > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("TPM model %s is only available for " > + "x86 and aarch64 guests"), Please don't split the error message into two separate lines, and sprinkle some quotes around '%s' while you're at it. https://libvirt.org/coding-style.html#error-message-format -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 0/2] qemu: swtpm improvements
On Wed, 2021-02-10 at 10:34 -0700, Jim Fehlig wrote: > These patches are fallout from a discussion about TPM devices and aarch64 > > https://listman.redhat.com/archives/libvir-list/2021-February/msg00526.html > > Jim Fehlig (2): > qemu: Fix swtpm device with aarch64 > qemu: Validate TPM TIS device With the nit I pointed out in 2/2 addressed, Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH v2 0/6] APIs for reporting tainting and deprecation messages
On 2/9/21 5:01 PM, Daniel P. Berrangé wrote: This is a follow up to https://listman.redhat.com/archives/libvir-list/2021-January/msg00988.html I pushed the first non-API parts of that series already. This posting takes a different approach to the APIs. Instead of separte APIs for tainting and deprecations, there is now one API for reporting general informational messages. This is explicitly only targetted at humans. v2: - Change formatting for better translation - Fix leak of strings Daniel P. Berrangé (6): conf: record deprecation messages against the domain qemu: record deprecation messages against the domain src: define virDomainGetMessages API remote: add RPC support for the virDomainGetMessages API qemu: implement virDomainGetMessages API tools: report messages for 'dominfo' command include/libvirt/libvirt-domain.h| 9 + src/conf/domain_conf.c | 45 -- src/conf/domain_conf.h | 5 +++ src/driver-hypervisor.h | 6 +++ src/libvirt-domain.c| 54 +++ src/libvirt_private.syms| 3 ++ src/libvirt_public.syms | 5 +++ src/qemu/qemu_domain.c | 5 +++ src/qemu/qemu_domain.h | 3 ++ src/qemu/qemu_driver.c | 58 + src/qemu/qemu_process.c | 5 +++ src/remote/remote_daemon_dispatch.c | 45 ++ src/remote/remote_driver.c | 44 ++ src/remote/remote_protocol.x| 21 ++- src/remote_protocol-structs | 11 ++ tools/virsh-domain-monitor.c| 10 + 16 files changed, 325 insertions(+), 4 deletions(-) Reviewed-by: Michal Privoznik Michal
Re: [PATCH 0/3 V3] Linstor storage backend
any progress on this, or interest? Cheers, Rene On Thu, Feb 4, 2021 at 9:01 AM Rene Peinthor wrote: > I didn't get any feedback on V2, nor any updates if you still reviewing, > but I found myself a few issues. > > First in V2 the element wasn't optional > (which it should be) and I improved a few error messages, > because sometimes it wasn't clear from where the error > originated. > > Best regards, > Rene > > Rene Peinthor (3): > storage: Linstor configuration > storage: Linstor support > storage: Add tests for the Linstor storage backend > > docs/schemas/storagepool.rng | 27 + > docs/storage.html.in | 39 + > include/libvirt/libvirt-storage.h | 1 + > meson.build | 6 + > meson_options.txt | 1 + > po/POTFILES.in| 1 + > src/conf/domain_conf.c| 1 + > src/conf/storage_conf.c | 13 +- > src/conf/storage_conf.h | 1 + > src/conf/virstorageobj.c | 4 +- > src/storage/meson.build | 25 + > src/storage/storage_backend.c | 6 + > src/storage/storage_backend_linstor.c | 783 ++ > src/storage/storage_backend_linstor.h | 24 + > src/storage/storage_backend_linstor_priv.h| 53 ++ > src/storage/storage_driver.c | 1 + > src/test/test_driver.c| 1 + > tests/linstorjsondata/broken.json | 1 + > tests/linstorjsondata/resource-group.json | 1 + > .../linstorjsondata/resource-list-test2.json | 332 > .../storage-pools-ssdpool.json| 72 ++ > tests/linstorjsondata/storage-pools.json | 192 + > tests/linstorjsondata/volume-def-list.json| 158 > .../volume-definition-test2.json | 1 + > tests/meson.build | 6 + > tests/storagebackendlinstortest.c | 372 + > .../storagepoolcapsschemadata/poolcaps-fs.xml | 7 + > .../poolcaps-full.xml | 7 + > tests/storagepoolxml2argvtest.c | 1 + > tests/storagepoolxml2xmlin/pool-linstor.xml | 8 + > tests/storagevolxml2xmlin/vol-linstor.xml | 10 + > tools/virsh-pool.c| 3 + > 32 files changed, 2156 insertions(+), 2 deletions(-) > create mode 100644 src/storage/storage_backend_linstor.c > create mode 100644 src/storage/storage_backend_linstor.h > create mode 100644 src/storage/storage_backend_linstor_priv.h > create mode 100644 tests/linstorjsondata/broken.json > create mode 100644 tests/linstorjsondata/resource-group.json > create mode 100644 tests/linstorjsondata/resource-list-test2.json > create mode 100644 tests/linstorjsondata/storage-pools-ssdpool.json > create mode 100644 tests/linstorjsondata/storage-pools.json > create mode 100644 tests/linstorjsondata/volume-def-list.json > create mode 100644 tests/linstorjsondata/volume-definition-test2.json > create mode 100644 tests/storagebackendlinstortest.c > create mode 100644 tests/storagepoolxml2xmlin/pool-linstor.xml > create mode 100644 tests/storagevolxml2xmlin/vol-linstor.xml > > -- > 2.30.0 > >
[libvirt PATCH 3/6] ci: Move ppc64le build from Debian sid to Debian 10
Debian sid is currently broken on ppc64le, so move the build to Debian 10; do the opposite for the aarch64 and mips64el builds to try and restore the 10/sid balance. Signed-off-by: Andrea Bolognani --- .gitlab-ci.yml | 30 +++--- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index cd7282a385..bb1d4d46b5 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -191,7 +191,7 @@ x64-ubuntu-2004-container: # Cross-build containers build jobs aarch64-debian-10-container: - extends: .container_job + extends: .container_optional_job variables: NAME: debian-10-cross-aarch64 @@ -216,7 +216,7 @@ mips-debian-10-container: NAME: debian-10-cross-mips mips64el-debian-10-container: - extends: .container_job + extends: .container_optional_job variables: NAME: debian-10-cross-mips64el @@ -226,7 +226,7 @@ mipsel-debian-10-container: NAME: debian-10-cross-mipsel ppc64le-debian-10-container: - extends: .container_optional_job + extends: .container_job variables: NAME: debian-10-cross-ppc64le @@ -236,7 +236,7 @@ s390x-debian-10-container: NAME: debian-10-cross-s390x aarch64-debian-sid-container: - extends: .container_optional_job + extends: .container_job variables: NAME: debian-sid-cross-aarch64 @@ -256,7 +256,7 @@ i686-debian-sid-container: NAME: debian-sid-cross-i686 mips64el-debian-sid-container: - extends: .container_optional_job + extends: .container_job variables: NAME: debian-sid-cross-mips64el @@ -266,7 +266,7 @@ mipsel-debian-sid-container: NAME: debian-sid-cross-mipsel ppc64le-debian-sid-container: - extends: .container_job + extends: .container_optional_job variables: NAME: debian-sid-cross-ppc64le @@ -435,12 +435,12 @@ x64-macos-1015-build: # Cross compiled build jobs -aarch64-debian-10: +aarch64-debian-sid: extends: .cross_build_job needs: -- aarch64-debian-10-container +- aarch64-debian-sid-container variables: -NAME: debian-10 +NAME: debian-sid CROSS: aarch64 armv6l-debian-10: @@ -475,12 +475,12 @@ mips-debian-10: NAME: debian-10 CROSS: mips -mips64el-debian-10: +mips64el-debian-sid: extends: .cross_build_job needs: -- mips64el-debian-10-container +- mips64el-debian-sid-container variables: -NAME: debian-10 +NAME: debian-sid CROSS: mips64el mipsel-debian-10: @@ -491,12 +491,12 @@ mipsel-debian-10: NAME: debian-10 CROSS: mipsel -ppc64le-debian-sid: +ppc64le-debian-10: extends: .cross_build_job needs: -- ppc64le-debian-sid-container +- ppc64le-debian-10-container variables: -NAME: debian-sid +NAME: debian-10 CROSS: ppc64le s390x-debian-sid: -- 2.26.2
[libvirt PATCH 0/6] ci: Paint the pipeline green
Various workarounds that are necessary due to breakages in external services and distribution archives, plus fixes for a couple of issues that were discovered in the process. Andrea Bolognani (6): ci: Shuffle cross-building jobs around ci: Mark container build jobs as required/optional correctly ci: Move ppc64le build from Debian sid to Debian 10 ci: Refresh Dockerfiles ci: Add temporary workaround for Fedora Rawhide ci: Build on FreeBSD 12.2 .gitlab-ci.yml| 66 +-- ci/containers/ci-centos-7.Dockerfile | 3 +- ci/containers/ci-centos-8.Dockerfile | 3 +- ci/containers/ci-centos-stream.Dockerfile | 3 +- .../ci-debian-10-cross-aarch64.Dockerfile | 3 +- .../ci-debian-10-cross-armv6l.Dockerfile | 3 +- .../ci-debian-10-cross-armv7l.Dockerfile | 3 +- .../ci-debian-10-cross-i686.Dockerfile| 3 +- .../ci-debian-10-cross-mips.Dockerfile| 3 +- .../ci-debian-10-cross-mips64el.Dockerfile| 3 +- .../ci-debian-10-cross-mipsel.Dockerfile | 3 +- .../ci-debian-10-cross-ppc64le.Dockerfile | 3 +- .../ci-debian-10-cross-s390x.Dockerfile | 3 +- ci/containers/ci-debian-10.Dockerfile | 3 +- .../ci-debian-sid-cross-aarch64.Dockerfile| 3 +- .../ci-debian-sid-cross-armv6l.Dockerfile | 3 +- .../ci-debian-sid-cross-armv7l.Dockerfile | 3 +- .../ci-debian-sid-cross-i686.Dockerfile | 3 +- .../ci-debian-sid-cross-mips64el.Dockerfile | 3 +- .../ci-debian-sid-cross-mipsel.Dockerfile | 3 +- .../ci-debian-sid-cross-ppc64le.Dockerfile| 3 +- .../ci-debian-sid-cross-s390x.Dockerfile | 3 +- ci/containers/ci-debian-sid.Dockerfile| 3 +- ci/containers/ci-fedora-32.Dockerfile | 3 +- ci/containers/ci-fedora-33.Dockerfile | 3 +- ...ci-fedora-rawhide-cross-mingw32.Dockerfile | 6 +- ...ci-fedora-rawhide-cross-mingw64.Dockerfile | 6 +- ci/containers/ci-fedora-rawhide.Dockerfile| 6 +- ci/containers/ci-opensuse-152.Dockerfile | 3 +- ci/containers/ci-ubuntu-1804.Dockerfile | 3 +- ci/containers/ci-ubuntu-2004.Dockerfile | 3 +- 31 files changed, 99 insertions(+), 66 deletions(-) -- 2.26.2
[libvirt PATCH 2/6] ci: Mark container build jobs as required/optional correctly
Whether a container build job is considered required depends on whether the corresponding cross-build job exists, and in a few cases the two got out of sync over time. Signed-off-by: Andrea Bolognani --- .gitlab-ci.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 4f6cd60d51..cd7282a385 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -201,7 +201,7 @@ armv6l-debian-10-container: NAME: debian-10-cross-armv6l armv7l-debian-10-container: - extends: .container_optional_job + extends: .container_job variables: NAME: debian-10-cross-armv7l @@ -246,7 +246,7 @@ armv6l-debian-sid-container: NAME: debian-sid-cross-armv6l armv7l-debian-sid-container: - extends: .container_job + extends: .container_optional_job variables: NAME: debian-sid-cross-armv7l @@ -271,7 +271,7 @@ ppc64le-debian-sid-container: NAME: debian-sid-cross-ppc64le s390x-debian-sid-container: - extends: .container_optional_job + extends: .container_job variables: NAME: debian-sid-cross-s390x -- 2.26.2
[libvirt PATCH 4/6] ci: Refresh Dockerfiles
Signed-off-by: Andrea Bolognani --- ci/containers/ci-centos-7.Dockerfile | 3 ++- ci/containers/ci-centos-8.Dockerfile | 3 ++- ci/containers/ci-centos-stream.Dockerfile| 3 ++- ci/containers/ci-debian-10-cross-aarch64.Dockerfile | 3 ++- ci/containers/ci-debian-10-cross-armv6l.Dockerfile | 3 ++- ci/containers/ci-debian-10-cross-armv7l.Dockerfile | 3 ++- ci/containers/ci-debian-10-cross-i686.Dockerfile | 3 ++- ci/containers/ci-debian-10-cross-mips.Dockerfile | 3 ++- ci/containers/ci-debian-10-cross-mips64el.Dockerfile | 3 ++- ci/containers/ci-debian-10-cross-mipsel.Dockerfile | 3 ++- ci/containers/ci-debian-10-cross-ppc64le.Dockerfile | 3 ++- ci/containers/ci-debian-10-cross-s390x.Dockerfile| 3 ++- ci/containers/ci-debian-10.Dockerfile| 3 ++- ci/containers/ci-debian-sid-cross-aarch64.Dockerfile | 3 ++- ci/containers/ci-debian-sid-cross-armv6l.Dockerfile | 3 ++- ci/containers/ci-debian-sid-cross-armv7l.Dockerfile | 3 ++- ci/containers/ci-debian-sid-cross-i686.Dockerfile| 3 ++- ci/containers/ci-debian-sid-cross-mips64el.Dockerfile| 3 ++- ci/containers/ci-debian-sid-cross-mipsel.Dockerfile | 3 ++- ci/containers/ci-debian-sid-cross-ppc64le.Dockerfile | 3 ++- ci/containers/ci-debian-sid-cross-s390x.Dockerfile | 3 ++- ci/containers/ci-debian-sid.Dockerfile | 3 ++- ci/containers/ci-fedora-32.Dockerfile| 3 ++- ci/containers/ci-fedora-33.Dockerfile| 3 ++- ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile | 3 ++- ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile | 3 ++- ci/containers/ci-fedora-rawhide.Dockerfile | 3 ++- ci/containers/ci-opensuse-152.Dockerfile | 3 ++- ci/containers/ci-ubuntu-1804.Dockerfile | 3 ++- ci/containers/ci-ubuntu-2004.Dockerfile | 3 ++- 30 files changed, 60 insertions(+), 30 deletions(-) diff --git a/ci/containers/ci-centos-7.Dockerfile b/ci/containers/ci-centos-7.Dockerfile index a847e9135b..233b99c38b 100644 --- a/ci/containers/ci-centos-7.Dockerfile +++ b/ci/containers/ci-centos-7.Dockerfile @@ -2,7 +2,7 @@ # # $ lcitool dockerfile centos-7 libvirt # -# https://gitlab.com/libvirt/libvirt-ci/-/commit/740f5254f607de914a92d664196d045149edb45a +# https://gitlab.com/libvirt/libvirt-ci/-/commit/824b894aa3ca40e9d2807b693765e213cb8aa832 FROM docker.io/library/centos:7 RUN yum update -y && \ @@ -65,6 +65,7 @@ WEiJKtQrZDJloqtyi/mmRa1VsV7RYR0VPJjhK/R8EQ7Ysshy\n\ glusterfs-api-devel \ gnutls-devel \ iproute \ +iptables \ iscsi-initiator-utils \ kmod \ libacl-devel \ diff --git a/ci/containers/ci-centos-8.Dockerfile b/ci/containers/ci-centos-8.Dockerfile index 5b81fb8f27..ee7941d87c 100644 --- a/ci/containers/ci-centos-8.Dockerfile +++ b/ci/containers/ci-centos-8.Dockerfile @@ -2,7 +2,7 @@ # # $ lcitool dockerfile centos-8 libvirt # -# https://gitlab.com/libvirt/libvirt-ci/-/commit/740f5254f607de914a92d664196d045149edb45a +# https://gitlab.com/libvirt/libvirt-ci/-/commit/824b894aa3ca40e9d2807b693765e213cb8aa832 FROM docker.io/library/centos:8 RUN dnf update -y && \ @@ -37,6 +37,7 @@ RUN dnf update -y && \ gnutls-devel \ iproute \ iproute-tc \ +iptables \ iscsi-initiator-utils \ kmod \ libacl-devel \ diff --git a/ci/containers/ci-centos-stream.Dockerfile b/ci/containers/ci-centos-stream.Dockerfile index e286857e00..c090a2ca94 100644 --- a/ci/containers/ci-centos-stream.Dockerfile +++ b/ci/containers/ci-centos-stream.Dockerfile @@ -2,7 +2,7 @@ # # $ lcitool dockerfile centos-stream libvirt # -# https://gitlab.com/libvirt/libvirt-ci/-/commit/740f5254f607de914a92d664196d045149edb45a +# https://gitlab.com/libvirt/libvirt-ci/-/commit/824b894aa3ca40e9d2807b693765e213cb8aa832 FROM docker.io/library/centos:8 RUN dnf install -y centos-release-stream && \ @@ -39,6 +39,7 @@ RUN dnf install -y centos-release-stream && \ gnutls-devel \ iproute \ iproute-tc \ +iptables \ iscsi-initiator-utils \ kmod \ libacl-devel \ diff --git a/ci/containers/ci-debian-10-cross-aarch64.Dockerfile b/ci/containers/ci-debian-10-cross-aarch64.Dockerfile index 53a3e42951..cc92e04c87 100644 --- a/ci/containers/ci-debian-10-cross-aarch64.Dockerfile +++ b/ci/containers/ci-debian-10-cross-aarch64.Dockerfile @@ -2,7 +2,7 @@ # # $ lcitool dockerfile --cross aarch64 debian-10 libvirt # -# https://gitlab.com/libvirt/libvirt-ci/-/commit/740f5254f607de914a92d664196d045149edb45a +# https://gitlab.com/libvirt/libvirt-ci/-/commit/824b894aa3ca40e9d2807b693765e213cb8aa832 FROM docker.io/library/debian:10-slim RUN export DEBIAN_FRONTEND=noninteractive && \ @@ -25,6 +25,7 @@ RUN export DEBIAN_FRO
[libvirt PATCH 5/6] ci: Add temporary workaround for Fedora Rawhide
The .repo files for Fedora Rawhide are already pointing to the Fedora 35 key, but all RPMs are still signed with the Fedora 34 key, resulting in GPG key at file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-35-x86_64 (0x9867C58F) is already installed The GPG keys listed for the "Fedora - Rawhide - Developmental packages for the next Fedora release" repository are already installed but they are not correct for this package. Check that the correct key URLs are configured for this repository.. Failing package is: nosync-1.1-10.fc34.x86_64 GPG Keys are configured as: file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-35-x86_64 The downloaded packages were saved in cache until the next successful transaction. You can remove cached packages by executing 'dnf clean packages'. Error: GPG check FAILED Temporarily tweak the .repo files so that the Fedora 34 key is used for validation. We should be able to revert this in a few days. Signed-off-by: Andrea Bolognani --- ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile | 3 ++- ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile | 3 ++- ci/containers/ci-fedora-rawhide.Dockerfile | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile b/ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile index 553dfd47f0..da0a6cc02c 100644 --- a/ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile +++ b/ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile @@ -5,7 +5,8 @@ # https://gitlab.com/libvirt/libvirt-ci/-/commit/824b894aa3ca40e9d2807b693765e213cb8aa832 FROM registry.fedoraproject.org/fedora:rawhide -RUN dnf install -y nosync && \ +RUN sed -Ei 's|^gpgkey=.*$|gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-34-x86_64|g' /etc/yum.repos.d/*.repo && \ +dnf install -y nosync && \ echo -e '#!/bin/sh\n\ if test -d /usr/lib64\n\ then\n\ diff --git a/ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile b/ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile index 0183b15b28..a1e50a34b2 100644 --- a/ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile +++ b/ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile @@ -5,7 +5,8 @@ # https://gitlab.com/libvirt/libvirt-ci/-/commit/824b894aa3ca40e9d2807b693765e213cb8aa832 FROM registry.fedoraproject.org/fedora:rawhide -RUN dnf install -y nosync && \ +RUN sed -Ei 's|^gpgkey=.*$|gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-34-x86_64|g' /etc/yum.repos.d/*.repo && \ +dnf install -y nosync && \ echo -e '#!/bin/sh\n\ if test -d /usr/lib64\n\ then\n\ diff --git a/ci/containers/ci-fedora-rawhide.Dockerfile b/ci/containers/ci-fedora-rawhide.Dockerfile index f331c8d74c..e3968b2199 100644 --- a/ci/containers/ci-fedora-rawhide.Dockerfile +++ b/ci/containers/ci-fedora-rawhide.Dockerfile @@ -5,7 +5,8 @@ # https://gitlab.com/libvirt/libvirt-ci/-/commit/824b894aa3ca40e9d2807b693765e213cb8aa832 FROM registry.fedoraproject.org/fedora:rawhide -RUN dnf install -y nosync && \ +RUN sed -Ei 's|^gpgkey=.*$|gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-34-x86_64|g' /etc/yum.repos.d/*.repo && \ +dnf install -y nosync && \ echo -e '#!/bin/sh\n\ if test -d /usr/lib64\n\ then\n\ -- 2.26.2
[libvirt PATCH 6/6] ci: Build on FreeBSD 12.2
The FreeBSD 12.1 image on Cirrus CI is currently broken, but that's okay because a FreeBSD 12.2 image is also available and we'd rather build on the more up-to-date target anyway. Signed-off-by: Andrea Bolognani --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index bb1d4d46b5..4563bccdf1 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -418,7 +418,7 @@ x64-freebsd-12-build: NAME: freebsd-12 CIRRUS_VM_INSTANCE_TYPE: freebsd_instance CIRRUS_VM_IMAGE_SELECTOR: image_family -CIRRUS_VM_IMAGE_NAME: freebsd-12-1 +CIRRUS_VM_IMAGE_NAME: freebsd-12-2 INSTALL_COMMAND: pkg install -y x64-macos-1015-build: -- 2.26.2
[libvirt PATCH 1/6] ci: Shuffle cross-building jobs around
Keep them ordered by architecture, the same way the corresponding container jobs are, to make it easier to jump between the two sections and compare them. Signed-off-by: Andrea Bolognani --- .gitlab-ci.yml | 42 +- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 5779b1b8b2..4f6cd60d51 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -435,6 +435,14 @@ x64-macos-1015-build: # Cross compiled build jobs +aarch64-debian-10: + extends: .cross_build_job + needs: +- aarch64-debian-10-container + variables: +NAME: debian-10 +CROSS: aarch64 + armv6l-debian-10: extends: .cross_build_job needs: @@ -451,13 +459,13 @@ armv7l-debian-10: NAME: debian-10 CROSS: armv7l -mips64el-debian-10: +i686-debian-sid: extends: .cross_build_job needs: -- mips64el-debian-10-container +- i686-debian-sid-container variables: -NAME: debian-10 -CROSS: mips64el +NAME: debian-sid +CROSS: i686 mips-debian-10: extends: .cross_build_job @@ -467,13 +475,13 @@ mips-debian-10: NAME: debian-10 CROSS: mips -aarch64-debian-10: +mips64el-debian-10: extends: .cross_build_job needs: -- aarch64-debian-10-container +- mips64el-debian-10-container variables: NAME: debian-10 -CROSS: aarch64 +CROSS: mips64el mipsel-debian-10: extends: .cross_build_job @@ -483,29 +491,21 @@ mipsel-debian-10: NAME: debian-10 CROSS: mipsel -s390x-debian-sid: - extends: .cross_build_job - needs: -- s390x-debian-sid-container - variables: -NAME: debian-sid -CROSS: s390x - -i686-debian-sid: +ppc64le-debian-sid: extends: .cross_build_job needs: -- i686-debian-sid-container +- ppc64le-debian-sid-container variables: NAME: debian-sid -CROSS: i686 +CROSS: ppc64le -ppc64le-debian-sid: +s390x-debian-sid: extends: .cross_build_job needs: -- ppc64le-debian-sid-container +- s390x-debian-sid-container variables: NAME: debian-sid -CROSS: ppc64le +CROSS: s390x mingw32-fedora-rawhide: extends: .cross_build_job -- 2.26.2
Re: [PATCH 0/7] support (aka "QEMU virtio failover") with plain
On 2/11/21 8:57 AM, Laine Stump wrote: This is explained in excruciating detail in Patch 5, but in short, allowing the element to be in a plain will permit someone who is running libvirt unprivileged, or in a container with no access to a PF device, to use the feature, which encapsulates a lot of functionality related to assigning an SRIOV network device to a guest as a part of a failover bond device (the other device in the pair is an emulated virtio NIC), which in turn permits the guest to be migrated by transparently unplugging the SRIOV NIC prior to migration, and plugging a new one in after migration is completed. (Previously we required for this feature, but that type of device needs to send netlink messages to the PF of the SRIOV VF that's being assigned, and that simply isn't possible sometimes.) Laine Stump (7): conf: make teaming info an official type conf: use virDomainNetTeamingInfoPtr instead of virDomainNetTeamingInfo conf: separate Parse/Format functions for virDomainNetTeamingInfo schema: separate teaming element definition from interface element conf: parse/format element in plain qemu: plug config from into qemu commandline news: document support for in NEWS.rst | 6 ++ docs/formatdomain.rst | 51 +++ docs/schemas/domaincommon.rng | 42 + src/conf/domain_conf.c| 87 +-- src/conf/domain_conf.h| 13 ++- src/conf/domain_validate.c| 45 +++--- src/conf/virconftypes.h | 3 + src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 17 ++-- src/qemu/qemu_domain.c| 2 +- src/qemu/qemu_migration.c | 4 +- src/qemu/qemu_validate.c | 42 - .../net-virtio-teaming-hostdev.args | 40 + .../net-virtio-teaming-hostdev.xml| 48 ++ tests/qemuxml2argvtest.c | 3 + .../net-virtio-teaming-hostdev.xml| 64 ++ tests/qemuxml2xmltest.c | 3 + 17 files changed, 384 insertions(+), 87 deletions(-) create mode 100644 tests/qemuxml2argvdata/net-virtio-teaming-hostdev.args create mode 100644 tests/qemuxml2argvdata/net-virtio-teaming-hostdev.xml create mode 100644 tests/qemuxml2xmloutdata/net-virtio-teaming-hostdev.xml Reviewed-by: Michal Privoznik Michal
Re: [PATCH 5/7] conf: parse/format element in plain
On 2/11/21 8:57 AM, Laine Stump wrote: The element in allows pairing two interfaces together as a simple "failover bond" network device in a guest. One of the devices will be the "transient" interface - it will be preferred for all network traffic when it is present, but may be removed when necessary, in particular during migration, when traffic will instead go through the other interface of the pair - the "persistent" interface. As it happens, in the QEMU implementation of this teaming pair (called "virtio failover" in QEMU) the transient interface is always a host network device assigned to the guest using VFIO (aka "hostdev"); the persistent interface is always an emulated virtio NIC. When support was initially added for , it was written to require that the transient/hostdev device be defined using ; this was done because the virtio failover implementation in QEMU and the virtio guest driver demands that the two interfaces in the pair have matching MAC addresses, and the only way libvirt can guarantee the MAC address of a hostdev network device is to use , whose main purpose is to configure the device's MAC address. (note that in turn requires that the network device be an SRIOV VF (Virtual Function), as that is the only type of network device whose MAC address we can set in a way that will survive the device's driver init in the guest). It has recently come up that some users are unable to use because they are running in a container environment where libvirt doesn't have the necessary privileges or resources to set the VF's MAC address (because setting the VF MAC is done via the same device's PF (Physical Function), and the PF is not exposed to libvirt's container. At the same time, they *are* able to set the VF's MAC address in advance of staring up libvirt in the container. So they could theoretically use the feature if libvirt just skipped the "setting the MAC address" part. Fortunately, that is *exactly* the difference between (a "hostdev VF") and (a "plain hostdev" - it could be *any PCI device; libvirt doesn't know what type of PCI device it is, and doesn't care). But what *is* still needed is for libvirt to provide a small bit of information on the commandline argument for the hostdev, telling QEMU that this device will be part of a team ("failover pair"), and the id of the other device in the pair. So, what we need to do is add support for the element to plain , and that is what this patch does. (actually, this patch adds parsing/formatting of the element in . The next patch will actually wire that into the qemu driver.) Signed-off-by: Laine Stump --- docs/formatdomain.rst | 51 +++ docs/schemas/domaincommon.rng | 3 + src/conf/domain_conf.c| 5 ++ src/conf/domain_conf.h| 1 + src/conf/domain_validate.c| 19 ++ .../net-virtio-teaming-hostdev.xml| 48 ++ .../net-virtio-teaming-hostdev.xml| 64 +++ tests/qemuxml2xmltest.c | 3 + 8 files changed, 194 insertions(+) create mode 100644 tests/qemuxml2argvdata/net-virtio-teaming-hostdev.xml create mode 100644 tests/qemuxml2xmloutdata/net-virtio-teaming-hostdev.xml There're only very few of differences between these two files (mostly PCI addresses in the out xml) and neither of them is related to this feature. Perhaps make one symlink of the other and add those diffs into the input xml? diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 2493be595f..eafd6b3396 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -4837,6 +4837,22 @@ support in the hypervisor and the guest network driver). ... +The second interface in this example is referencing a network that is +a pool of SRIOV VFs (i.e. a "hostdev network"). You could instead +directly reference an SRIOV VF device: + +:: + + ... + + + + + + + + ... + The element required attribute ``type`` will be set to either ``"persistent"`` to indicate a device that should always be present in the domain, or ``"transient"`` to indicate a device that may periodically be @@ -4858,6 +4874,41 @@ once migration is completed; while migration is taking place, network traffic will use the virtio NIC. (Of course the emulated virtio NIC and the hostdev NIC must be connected to the same subnet for bonding to work properly). +:since:`Since 7.1.0` The element can also be added to a +plain device. + +:: + + ... + + + + + + + + ... + +This device must be a network device, but not necessarily an SRIOV +VF. Using plain rather than or is useful if the +device that will be assigned with VFIO is a standard NIC (not a VF) or +if libvirt doesn't have the necessary resources and privileges to set +the VF's MAC a
Re: [libvirt PATCH 1/6] ci: Shuffle cross-building jobs around
On Thu, Feb 11, 2021 at 02:06:41PM +0100, Andrea Bolognani wrote: > Keep them ordered by architecture, the same way the corresponding > container jobs are, to make it easier to jump between the two > sections and compare them. > > Signed-off-by: Andrea Bolognani > --- > .gitlab-ci.yml | 42 +- > 1 file changed, 21 insertions(+), 21 deletions(-) Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 2/6] ci: Mark container build jobs as required/optional correctly
On Thu, Feb 11, 2021 at 02:06:42PM +0100, Andrea Bolognani wrote: > Whether a container build job is considered required depends on > whether the corresponding cross-build job exists, and in a few > cases the two got out of sync over time. > > Signed-off-by: Andrea Bolognani > --- > .gitlab-ci.yml | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 3/6] ci: Move ppc64le build from Debian sid to Debian 10
On Thu, Feb 11, 2021 at 02:06:43PM +0100, Andrea Bolognani wrote: > Debian sid is currently broken on ppc64le, so move the build to > Debian 10; do the opposite for the aarch64 and mips64el builds to > try and restore the 10/sid balance. > > Signed-off-by: Andrea Bolognani > --- > .gitlab-ci.yml | 30 +++--- > 1 file changed, 15 insertions(+), 15 deletions(-) Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 4/6] ci: Refresh Dockerfiles
On Thu, Feb 11, 2021 at 02:06:44PM +0100, Andrea Bolognani wrote: > Signed-off-by: Andrea Bolognani > --- > ci/containers/ci-centos-7.Dockerfile | 3 ++- > ci/containers/ci-centos-8.Dockerfile | 3 ++- > ci/containers/ci-centos-stream.Dockerfile| 3 ++- > ci/containers/ci-debian-10-cross-aarch64.Dockerfile | 3 ++- > ci/containers/ci-debian-10-cross-armv6l.Dockerfile | 3 ++- > ci/containers/ci-debian-10-cross-armv7l.Dockerfile | 3 ++- > ci/containers/ci-debian-10-cross-i686.Dockerfile | 3 ++- > ci/containers/ci-debian-10-cross-mips.Dockerfile | 3 ++- > ci/containers/ci-debian-10-cross-mips64el.Dockerfile | 3 ++- > ci/containers/ci-debian-10-cross-mipsel.Dockerfile | 3 ++- > ci/containers/ci-debian-10-cross-ppc64le.Dockerfile | 3 ++- > ci/containers/ci-debian-10-cross-s390x.Dockerfile| 3 ++- > ci/containers/ci-debian-10.Dockerfile| 3 ++- > ci/containers/ci-debian-sid-cross-aarch64.Dockerfile | 3 ++- > ci/containers/ci-debian-sid-cross-armv6l.Dockerfile | 3 ++- > ci/containers/ci-debian-sid-cross-armv7l.Dockerfile | 3 ++- > ci/containers/ci-debian-sid-cross-i686.Dockerfile| 3 ++- > ci/containers/ci-debian-sid-cross-mips64el.Dockerfile| 3 ++- > ci/containers/ci-debian-sid-cross-mipsel.Dockerfile | 3 ++- > ci/containers/ci-debian-sid-cross-ppc64le.Dockerfile | 3 ++- > ci/containers/ci-debian-sid-cross-s390x.Dockerfile | 3 ++- > ci/containers/ci-debian-sid.Dockerfile | 3 ++- > ci/containers/ci-fedora-32.Dockerfile| 3 ++- > ci/containers/ci-fedora-33.Dockerfile| 3 ++- > ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile | 3 ++- > ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile | 3 ++- > ci/containers/ci-fedora-rawhide.Dockerfile | 3 ++- > ci/containers/ci-opensuse-152.Dockerfile | 3 ++- > ci/containers/ci-ubuntu-1804.Dockerfile | 3 ++- > ci/containers/ci-ubuntu-2004.Dockerfile | 3 ++- > 30 files changed, 60 insertions(+), 30 deletions(-) Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 5/6] ci: Add temporary workaround for Fedora Rawhide
On Thu, Feb 11, 2021 at 02:06:45PM +0100, Andrea Bolognani wrote: > The .repo files for Fedora Rawhide are already pointing to the > Fedora 35 key, but all RPMs are still signed with the Fedora 34 > key, resulting in > > GPG key at file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-35-x86_64 > (0x9867C58F) is already installed > The GPG keys listed for the "Fedora - Rawhide - Developmental packages for > the next Fedora release" repository > are already installed but they are not correct for this package. > Check that the correct key URLs are configured for this repository.. > Failing package is: nosync-1.1-10.fc34.x86_64 >GPG Keys are configured as: > file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-35-x86_64 > The downloaded packages were saved in cache until the next successful > transaction. > You can remove cached packages by executing 'dnf clean packages'. > Error: GPG check FAILED > > Temporarily tweak the .repo files so that the Fedora 34 key is > used for validation. We should be able to revert this in a few > days. Hmm, isn't the real bug the ordering: dnf install -y nosync && \ nosync dnf update -y --nogpgcheck fedora-gpg-keys && \ nosync dnf update -y && \ We installed nosync too early. We need to update fedora-gpg-keys as the *first* thing we do in rawhide images, and only then attempt to install nosync. > > Signed-off-by: Andrea Bolognani > --- > ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile | 3 ++- > ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile | 3 ++- > ci/containers/ci-fedora-rawhide.Dockerfile | 3 ++- > 3 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile > b/ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile > index 553dfd47f0..da0a6cc02c 100644 > --- a/ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile > +++ b/ci/containers/ci-fedora-rawhide-cross-mingw32.Dockerfile > @@ -5,7 +5,8 @@ > # > https://gitlab.com/libvirt/libvirt-ci/-/commit/824b894aa3ca40e9d2807b693765e213cb8aa832 > FROM registry.fedoraproject.org/fedora:rawhide > > -RUN dnf install -y nosync && \ > +RUN sed -Ei > 's|^gpgkey=.*$|gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-34-x86_64|g' > /etc/yum.repos.d/*.repo && \ > +dnf install -y nosync && \ > echo -e '#!/bin/sh\n\ > if test -d /usr/lib64\n\ > then\n\ > diff --git a/ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile > b/ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile > index 0183b15b28..a1e50a34b2 100644 > --- a/ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile > +++ b/ci/containers/ci-fedora-rawhide-cross-mingw64.Dockerfile > @@ -5,7 +5,8 @@ > # > https://gitlab.com/libvirt/libvirt-ci/-/commit/824b894aa3ca40e9d2807b693765e213cb8aa832 > FROM registry.fedoraproject.org/fedora:rawhide > > -RUN dnf install -y nosync && \ > +RUN sed -Ei > 's|^gpgkey=.*$|gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-34-x86_64|g' > /etc/yum.repos.d/*.repo && \ > +dnf install -y nosync && \ > echo -e '#!/bin/sh\n\ > if test -d /usr/lib64\n\ > then\n\ > diff --git a/ci/containers/ci-fedora-rawhide.Dockerfile > b/ci/containers/ci-fedora-rawhide.Dockerfile > index f331c8d74c..e3968b2199 100644 > --- a/ci/containers/ci-fedora-rawhide.Dockerfile > +++ b/ci/containers/ci-fedora-rawhide.Dockerfile > @@ -5,7 +5,8 @@ > # > https://gitlab.com/libvirt/libvirt-ci/-/commit/824b894aa3ca40e9d2807b693765e213cb8aa832 > FROM registry.fedoraproject.org/fedora:rawhide > > -RUN dnf install -y nosync && \ > +RUN sed -Ei > 's|^gpgkey=.*$|gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-34-x86_64|g' > /etc/yum.repos.d/*.repo && \ > +dnf install -y nosync && \ > echo -e '#!/bin/sh\n\ > if test -d /usr/lib64\n\ > then\n\ > -- > 2.26.2 > Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 5/6] ci: Add temporary workaround for Fedora Rawhide
On Thu, 2021-02-11 at 14:48 +, Daniel P. Berrangé wrote: > On Thu, Feb 11, 2021 at 02:06:45PM +0100, Andrea Bolognani wrote: > > The .repo files for Fedora Rawhide are already pointing to the > > Fedora 35 key, but all RPMs are still signed with the Fedora 34 > > key, resulting in > > > > GPG key at file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-35-x86_64 > > (0x9867C58F) is already installed > > The GPG keys listed for the "Fedora - Rawhide - Developmental packages > > for the next Fedora release" repository > > are already installed but they are not correct for this package. > > Check that the correct key URLs are configured for this repository.. > > Failing package is: nosync-1.1-10.fc34.x86_64 > >GPG Keys are configured as: > > file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-35-x86_64 > > The downloaded packages were saved in cache until the next successful > > transaction. > > You can remove cached packages by executing 'dnf clean packages'. > > Error: GPG check FAILED > > > > Temporarily tweak the .repo files so that the Fedora 34 key is > > used for validation. We should be able to revert this in a few > > days. > > Hmm, isn't the real bug the ordering: > > dnf install -y nosync && \ > nosync dnf update -y --nogpgcheck fedora-gpg-keys && \ > nosync dnf update -y && \ > > We installed nosync too early. We need to update fedora-gpg-keys > as the *first* thing we do in rawhide images, and only then > attempt to install nosync. Yeah, I thought of that as well at first, and it's definitely something that we need to fix in lcitool. I'll post a patch later. That said, in this case the problem can't be solved by simply inverting the commands: from the output above, you'll see that the signing key for Fedora 35 is already present on the system; however, since the RPMs that are currently in the Rawhide repos are still signed with the Fedora 34 key, we need to convince dnf to use the latter for validation. That's what the hack in this patch does. -- Andrea Bolognani / Red Hat / Virtualization
[PATCH 00/19] qemu: migrate block bitmaps when migrating storage
When we are copying storage we should also preserve the block dirty bitmaps. This series implements their migration. For standard migration with shared storage we let qemu flush the bitmaps to disk and reload them on destination. There is possibility to migrate them using the migration stream too but it's not implemented in this series. Note that the first patch depends on the following qemu patches: https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg03659.html We can now also formulate the condition for enabling incremental backup feature, but it still depends on stabilization of 'blockdev-reopen'. Peter Krempa (19): qemucapabilitiesdata: Update test data for qemu-6.0 on x86_64 qemu: capabilities: Introduce QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING qemu: Probe whether an image is 'qcow2 v2' from query-named-block-nodes virDomainMigrateVersion3Full: Don't set 'cancelled' to the same value qemuMigrationSrcPerformPeer2Peer3: Don't leak 'dom_xml' on cleanup testutils: virTestRewrapFile: Rewrap also '.argv' files storagevolxml2argvdata: Rewrap all output files storage: Format qcow2v3 volumes by default qemu: monitor: Introduce qemuMonitorBitmapRemove qemu: blockjob: Use qemuMonitorBitmapRemove for single bitmap removal qemu: migration_params: Add infrastructure for 'dirty-bitmaps' migration feature qemu: migration_cookie: Add XML handling for setting up bitmap migration qemu: migration_cookie: Add helpers for transforming the cookie into migration params qemu: domain: Store list of temporary bitmaps for migration in status XML tests: qemustatusxml2xml: Add status XML from migration with bitmaps tests: qemumigrationcookie: Add testing for block dirty bitmap migration qemu: migration: Clean up temporary bitmaps when cancelling a migration qemu: migration: Migrate block dirty bitmaps corresponding to checkpoints qemu: capabilities: Enable QEMU_CAPS_INCREMENTAL_BACKUP src/libvirt-domain.c | 2 - src/qemu/qemu_blockjob.c | 24 +- src/qemu/qemu_capabilities.c | 8 +- src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_domain.c| 90 ++- src/qemu/qemu_domain.h| 15 + src/qemu/qemu_migration.c | 366 ++- src/qemu/qemu_migration_cookie.c | 249 src/qemu/qemu_migration_cookie.h | 41 ++ src/qemu/qemu_migration_params.c | 24 + src/qemu/qemu_migration_params.h | 5 + src/qemu/qemu_monitor.c | 13 + src/qemu/qemu_monitor.h | 8 + src/qemu/qemu_monitor_json.c | 35 ++ src/qemu/qemu_monitor_json.h | 6 + src/storage/storage_util.c| 2 +- tests/meson.build | 2 +- tests/qemublocktest.c | 2 + tests/qemublocktestdata/bitmap/synthetic.json | 2 +- tests/qemublocktestdata/bitmap/synthetic.out | 1 + .../caps_6.0.0.x86_64.replies | 510 +--- .../caps_6.0.0.x86_64.xml | 16 +- .../nbd-bitmaps-xml2xml-in.xml| 52 ++ .../nbd-bitmaps-xml2xml-migparams.json| 25 + .../nbd-bitmaps-xml2xml-out.xml | 51 ++ tests/qemumigrationcookiexmltest.c| 166 - tests/qemumonitorjsontest.c | 2 + .../migration-out-nbd-bitmaps-in.xml | 574 ++ .../migration-out-nbd-bitmaps-out.xml | 1 + tests/qemustatusxml2xmltest.c | 1 + tests/storagevolxml2argvdata/iso-input.argv | 6 +- tests/storagevolxml2argvdata/iso.argv | 4 +- .../logical-from-qcow2.argv | 6 +- tests/storagevolxml2argvdata/luks-cipher.argv | 8 +- .../luks-convert-encrypt.argv | 23 +- .../luks-convert-encrypt2fileqcow2.argv | 21 +- .../luks-convert-encrypt2fileraw.argv | 20 +- .../luks-convert-qcow2.argv | 21 +- .../storagevolxml2argvdata/luks-convert.argv | 20 +- tests/storagevolxml2argvdata/luks.argv| 8 +- tests/storagevolxml2argvdata/qcow2-1.1.argv | 8 +- .../storagevolxml2argvdata/qcow2-compat.argv | 8 +- .../qcow2-from-logical-compat.argv| 8 +- tests/storagevolxml2argvdata/qcow2-lazy.argv | 9 +- .../qcow2-luks-convert-encrypt.argv | 2 +- .../qcow2-luks-convert-encrypt2fileqcow2.argv | 2 +- tests/storagevolxml2argvdata/qcow2-luks.argv | 2 +- ...ow2-nobacking-convert-prealloc-compat.argv | 10 +- .../qcow2-nobacking-prealloc-compat.argv | 8 +- .../qcow2-nocapacity-convert-prealloc.argv| 9 +- .../qcow2-nocapacity.argv | 6 +- .../qcow2-nocow-compat.argv | 9 +- tests/storagevolxml2argvdata/qcow2-nocow.argv | 9 +- .../qcow2-ze
[PATCH 03/19] qemu: Probe whether an image is 'qcow2 v2' from query-named-block-nodes
Such images don't support stuff like dirty bitmaps. Note that the synthetic test for detecting bitmaps is used as an example to prevent adding additional test cases. Signed-off-by: Peter Krempa --- src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 11 +++ tests/qemublocktest.c | 2 ++ tests/qemublocktestdata/bitmap/synthetic.json | 2 +- tests/qemublocktestdata/bitmap/synthetic.out | 1 + 5 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index b0068f2a82..7a039641ba 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -732,6 +732,9 @@ struct _qemuBlockNamedNodeData { /* the cluster size of the image is valid only when > 0 */ unsigned long long clusterSize; + +/* image version */ +bool qcow2v2; }; GHashTable * diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d3b2a2c7a5..65c30e3bb4 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2979,6 +2979,7 @@ qemuMonitorJSONBlockGetNamedNodeDataWorker(size_t pos G_GNUC_UNUSED, GHashTable *nodes = opaque; virJSONValuePtr img; virJSONValuePtr bitmaps; +virJSONValuePtr format_specific; const char *nodename; g_autoptr(qemuBlockNamedNodeData) ent = NULL; @@ -3001,6 +3002,16 @@ qemuMonitorJSONBlockGetNamedNodeDataWorker(size_t pos G_GNUC_UNUSED, if ((bitmaps = virJSONValueObjectGetArray(val, "dirty-bitmaps"))) qemuMonitorJSONBlockGetNamedNodeDataBitmaps(bitmaps, ent); +/* query qcow2 format specific props */ +if ((format_specific = virJSONValueObjectGetObject(img, "format-specific")) && +STREQ_NULLABLE(virJSONValueObjectGetString(format_specific, "type"), "qcow2")) { +virJSONValuePtr qcow2props = virJSONValueObjectGetObject(format_specific, "data"); + +if (qcow2props && +STREQ_NULLABLE(virJSONValueObjectGetString(qcow2props, "compat"), "0.10")) +ent->qcow2v2 = true; +} + if (virHashAddEntry(nodes, nodename, ent) < 0) return -1; diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index ddaf73359d..bbfcfee92d 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -599,6 +599,8 @@ testQemuDetectBitmapsWorker(GHashTable *nodedata, return; virBufferAsprintf(buf, "%s:\n", nodename); +if (data->qcow2v2) +virBufferAddLit(buf, " qcow2 v2\n"); virBufferAdjustIndent(buf, 1); for (i = 0; i < data->nbitmaps; i++) { diff --git a/tests/qemublocktestdata/bitmap/synthetic.json b/tests/qemublocktestdata/bitmap/synthetic.json index 3712c8e5fc..cd468a42a2 100644 --- a/tests/qemublocktestdata/bitmap/synthetic.json +++ b/tests/qemublocktestdata/bitmap/synthetic.json @@ -12,7 +12,7 @@ "format-specific": { "type": "qcow2", "data": { - "compat": "1.1", + "compat": "0.10", "compression-type": "zlib", "lazy-refcounts": false, "bitmaps": [ diff --git a/tests/qemublocktestdata/bitmap/synthetic.out b/tests/qemublocktestdata/bitmap/synthetic.out index cde7228e01..2d9545fc9b 100644 --- a/tests/qemublocktestdata/bitmap/synthetic.out +++ b/tests/qemublocktestdata/bitmap/synthetic.out @@ -1,4 +1,5 @@ libvirt-1-format: + qcow2 v2 current: record:1 busy:0 persist:1 inconsist:1 gran:65536 dirty:0 top-ok: record:1 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 top-inactive: record:0 busy:0 persist:1 inconsist:0 gran:65536 dirty:0 -- 2.29.2
[PATCH 01/19] qemucapabilitiesdata: Update test data for qemu-6.0 on x86_64
Include the 'transform' member of 'block-bitmap-mapping'. Note that this is based on uncommited patches and will be updated once they are merged. --- .../caps_6.0.0.x86_64.replies | 510 ++ .../caps_6.0.0.x86_64.xml | 16 +- 2 files changed, 306 insertions(+), 220 deletions(-) diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.replies b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.replies index a1e3850b59..1af61272af 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.replies @@ -21,7 +21,7 @@ "minor": 2, "major": 5 }, -"package": "v5.2.0-1810-g2436651b26" +"package": "v5.2.0-1934-ge7bdfa1463" }, "id": "libvirt-2" } @@ -667,6 +667,10 @@ "name": "vhost-user-vsock-device", "parent": "vhost-vsock-common" }, +{ + "name": "virtio-blk-pci-transitional", + "parent": "virtio-blk-pci-base" +}, { "name": "pcie-pci-bridge", "parent": "base-pci-bridge" @@ -708,8 +712,8 @@ "parent": "pci-device" }, { - "name": "sev-guest", - "parent": "confidential-guest-support" + "name": "chardev-stdio", + "parent": "chardev-fd" }, { "name": "usb-redir", @@ -732,8 +736,8 @@ "parent": "pci-vga" }, { - "name": "virtio-blk-pci-transitional", - "parent": "virtio-blk-pci-base" + "name": "kvm-pit", + "parent": "pit-common" }, { "name": "Haswell-v1-x86_64-cpu", @@ -756,8 +760,8 @@ "parent": "generic-pc-machine" }, { - "name": "kvm-pit", - "parent": "pit-common" + "name": "sev-guest", + "parent": "confidential-guest-support" }, { "name": "ich9-usb-uhci5", @@ -811,6 +815,10 @@ "name": "usb-hub", "parent": "usb-device" }, +{ + "name": "chardev-serial", + "parent": "chardev-fd" +}, { "name": "virtio-blk-device", "parent": "virtio-device" @@ -852,8 +860,8 @@ "parent": "accel" }, { - "name": "chardev-serial", - "parent": "chardev-fd" + "name": "Cooperlake-x86_64-cpu", + "parent": "x86_64-cpu" }, { "name": "vhost-user-vsock-pci", @@ -891,14 +899,14 @@ "name": "memory-backend-ram", "parent": "memory-backend" }, -{ - "name": "PCIE", - "parent": "PCI" -}, { "name": "e1000e", "parent": "pci-device" }, +{ + "name": "PCIE", + "parent": "PCI" +}, { "name": "n270-x86_64-cpu", "parent": "x86_64-cpu" @@ -907,10 +915,6 @@ "name": "pxb-host", "parent": "pci-host-bridge" }, -{ - "name": "Cooperlake-x86_64-cpu", - "parent": "x86_64-cpu" -}, { "name": "scsi-disk", "parent": "scsi-disk-base" @@ -979,14 +983,14 @@ "name": "pci-ipmi-kcs", "parent": "pci-device" }, -{ - "name": "intel-iommu-iommu-memory-region", - "parent": "qemu:iommu-memory-region" -}, { "name": "xio3130-downstream", "parent": "pcie-slot" }, +{ + "name": "intel-iommu-iommu-memory-region", + "parent": "qemu:iommu-memory-region" +}, { "name": "vhost-user-vsock-pci-non-transitional", "parent": "vhost-user-vsock-pci-base" @@ -995,14 +999,14 @@ "name": "pc-i440fx-2.3-machine", "parent": "generic-pc-machine" }, -{ - "name": "PCI", - "parent": "bus" -}, { "name": "microvm-machine", "parent": "x86-machine" }, +{ + "name": "PCI", + "parent": "bus" +}, { "name": "sdhci-bus", "parent": "sd-bus" @@ -1104,8 +1108,8 @@ "parent": "pci-device" }, { - "name": "virtio-input-host-pci", - "parent": "virtio-input-host-pci-base-type" + "name": "virtio-9p-pci-transitional", + "parent": "virtio-9p-pci-base" }, { "name": "nvdimm", @@ -1116,8 +1120,8 @@ "parent": "generic-pc-machine" }, { - "name": "virtio-9p-pci-transitional", - "parent": "virtio-9p-pci-base" + "name": "virtio-input-host-pci", + "parent": "virtio-input-host-pci-base-type" }, { "name": "Opteron_G1-x86_64-cpu", @@ -1143,6 +1147,10 @@ "name": "i82557c", "parent": "pci-device" }, +{ + "name": "i82557b", + "parent": "pci-device" +}, { "name": "virtio-scsi-device", "parent": "virtio-scsi-common" @@ -1151,10 +1159,6 @@ "name": "pxb-pcie", "parent": "pci-device" }, -{ - "name": "i82557b", - "parent": "pci-device" -}, { "name": "Haswell-IBRS-x86_64-cpu", "parent": "x86_64-cpu" @@ -1172,12 +1176,8 @@ "parent": "sys-bus-device" }, { - "name": "chardev-memory", - "parent": "chardev-ringbuf" -}, -
[PATCH 02/19] qemu: capabilities: Introduce QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING
The capability represents qemu's ability to setup mappings for migrating block dirty bitmaps and is based on presence of the 'transform' property of the 'block-bitmap-mapping' property of 'migrate-set-parameters' QMP command. Signed-off-by: Peter Krempa --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + 2 files changed, 3 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ccf810ff96..38555dde98 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -616,6 +616,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "vhost-user-blk", "cpu-max", "memory-backend-file.x-use-canonical-path-for-ramblock-id", + "migration-param.block-bitmap-mapping", ); @@ -1549,6 +1550,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "migrate-set-parameters/arg-type/xbzrle-cache-size", QEMU_CAPS_MIGRATION_PARAM_XBZRLE_CACHE_SIZE }, { "set-numa-node/arg-type/+hmat-lb", QEMU_CAPS_NUMA_HMAT }, { "netdev_add/arg-type/+vhost-vdpa", QEMU_CAPS_NETDEV_VHOST_VDPA }, +{ "migrate-set-parameters/arg-type/block-bitmap-mapping/transform", QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING }, }; typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 6fd7922926..03d6ba60cf 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -596,6 +596,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_DEVICE_VHOST_USER_BLK, /* -device vhost-user-blk */ QEMU_CAPS_CPU_MAX, /* -cpu max */ QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID, /* -object memory-backend-file,x-use-canonical-path-for-ramblock-id= */ +QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING, /* block-bitmap-mapping in migrate-set-parameters */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; -- 2.29.2
[PATCH 04/19] virDomainMigrateVersion3Full: Don't set 'cancelled' to the same value
It's already initialized to '1'. Signed-off-by: Peter Krempa --- src/libvirt-domain.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index dba89a7d3a..e9688a15b4 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -3089,7 +3089,6 @@ virDomainMigrateVersion3Full(virDomainPtr domain, virTypedParamsReplaceString(¶ms, &nparams, VIR_MIGRATE_PARAM_URI, uri_out) < 0) { -cancelled = 1; virErrorPreserveLast(&orig_err); goto finish; } @@ -3098,7 +3097,6 @@ virDomainMigrateVersion3Full(virDomainPtr domain, VIR_MIGRATE_PARAM_URI, &uri) <= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domainMigratePrepare3 did not set uri")); -cancelled = 1; virErrorPreserveLast(&orig_err); goto finish; } -- 2.29.2
[PATCH 08/19] storage: Format qcow2v3 volumes by default
Format the new volumes with 'compat=1.1' since the minimum supported qemu version is now 1.5 rather the pre-historic compat=0.10. Signed-off-by: Peter Krempa --- src/storage/storage_util.c | 2 +- .../storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv | 2 +- tests/storagevolxml2argvdata/qcow2-compat.argv | 2 +- tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv | 2 +- tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.argv| 2 +- .../qcow2-luks-convert-encrypt2fileqcow2.argv | 2 +- tests/storagevolxml2argvdata/qcow2-luks.argv| 2 +- .../qcow2-nobacking-convert-prealloc-compat.argv| 2 +- .../storagevolxml2argvdata/qcow2-nobacking-prealloc-compat.argv | 2 +- .../qcow2-nocapacity-convert-prealloc.argv | 2 +- tests/storagevolxml2argvdata/qcow2-nocapacity.argv | 2 +- tests/storagevolxml2argvdata/qcow2-nocow-compat.argv| 2 +- tests/storagevolxml2argvdata/qcow2-zerocapacity.argv| 2 +- 13 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index b5adb05826..6d8dd3cc16 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -777,7 +777,7 @@ storageBackendCreateQemuImgOpts(virStorageEncryptionInfoDefPtr encinfo, if (info->compat) virBufferAsprintf(&buf, "compat=%s,", info->compat); else if (info->format == VIR_STORAGE_FILE_QCOW2) -virBufferAddLit(&buf, "compat=0.10,"); +virBufferAddLit(&buf, "compat=1.1,"); if (info->features && info->format == VIR_STORAGE_FILE_QCOW2) { if (virBitmapIsBitSet(info->features, diff --git a/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv b/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv index 1320e2ee2f..c313a04fb3 100644 --- a/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv +++ b/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv @@ -1,7 +1,7 @@ qemu-img \ create \ -f qcow2 \ --o compat=0.10 /var/lib/libvirt/images/sparse-qcow2.img 1073741824K +-o compat=1.1 /var/lib/libvirt/images/sparse-qcow2.img 1073741824K qemu-img \ convert \ --image-opts \ diff --git a/tests/storagevolxml2argvdata/qcow2-compat.argv b/tests/storagevolxml2argvdata/qcow2-compat.argv index 3071d7a790..5ee974afe9 100644 --- a/tests/storagevolxml2argvdata/qcow2-compat.argv +++ b/tests/storagevolxml2argvdata/qcow2-compat.argv @@ -2,4 +2,4 @@ qemu-img \ create \ -f qcow2 \ -b /dev/null \ --o backing_fmt=raw,compat=0.10 /var/lib/libvirt/images/OtherDemo.img 5242880K +-o backing_fmt=raw,compat=1.1 /var/lib/libvirt/images/OtherDemo.img 5242880K diff --git a/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv b/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv index 1971da200d..dcafffc3a4 100644 --- a/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv +++ b/tests/storagevolxml2argvdata/qcow2-from-logical-compat.argv @@ -2,4 +2,4 @@ qemu-img \ convert \ -f raw \ -O qcow2 \ --o compat=0.10 /dev/HostVG/Swap /var/lib/libvirt/images/OtherDemo.img +-o compat=1.1 /dev/HostVG/Swap /var/lib/libvirt/images/OtherDemo.img diff --git a/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.argv b/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.argv index de8aef4233..891746d921 100644 --- a/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.argv +++ b/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt.argv @@ -3,7 +3,7 @@ create \ -f qcow2 \ --object secret,id=OtherDemoLuks.img_encrypt0,file=/path/to/secretFile \ -o encrypt.format=luks,encrypt.key-secret=OtherDemoLuks.img_encrypt0,\ -compat=0.10 /var/lib/libvirt/images/OtherDemoLuks.img 5242880K +compat=1.1 /var/lib/libvirt/images/OtherDemoLuks.img 5242880K qemu-img \ convert \ --image-opts \ diff --git a/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt2fileqcow2.argv b/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt2fileqcow2.argv index 517156ca83..fc9c4ab825 100644 --- a/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt2fileqcow2.argv +++ b/tests/storagevolxml2argvdata/qcow2-luks-convert-encrypt2fileqcow2.argv @@ -1,7 +1,7 @@ qemu-img \ create \ -f qcow2 \ --o compat=0.10 /var/lib/libvirt/images/sparse-qcow2.img 1073741824K +-o compat=1.1 /var/lib/libvirt/images/sparse-qcow2.img 1073741824K qemu-img \ convert \ --image-opts \ diff --git a/tests/storagevolxml2argvdata/qcow2-luks.argv b/tests/storagevolxml2argvdata/qcow2-luks.argv index 4b51b374ca..c0568e10e3 100644 --- a/tests/storagevolxml2argvdata/qcow2-luks.argv +++ b/tests/storagevolxml2argvdata/qcow2-luks.argv @@ -5,4 +5,4 @@ create \ --object secret,id=OtherDemoLuks.img_encrypt0,file=/path/to/secretFile \ -o backing_fmt=raw,encrypt.format=luks,\ encrypt.key-secret=OtherDemoLuks.img_encrypt0,\ -compat=0.10 /var/lib/libvirt/image
[PATCH 12/19] qemu: migration_cookie: Add XML handling for setting up bitmap migration
In cases where we are copying the storage we need to ensure that also bitmaps are copied properly. This patch adds migration cookie XML infrastructure which will allow the migration sides reach consensus on which bitmaps to migrate. Signed-off-by: Peter Krempa --- src/qemu/qemu_migration_cookie.c | 134 +++ src/qemu/qemu_migration_cookie.h | 34 2 files changed, 168 insertions(+) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 6f2b1b2f57..94ba9c83d0 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -51,6 +51,7 @@ VIR_ENUM_IMPL(qemuMigrationCookieFlag, "cpu", "allowReboot", "capabilities", + "block-dirty-bitmaps", ); @@ -116,6 +117,39 @@ qemuMigrationCookieCapsFree(qemuMigrationCookieCapsPtr caps) G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMigrationCookieCaps, qemuMigrationCookieCapsFree); +static void +qemuMigrationBlockDirtyBitmapsDiskBitmapFree(qemuMigrationBlockDirtyBitmapsDiskBitmapPtr bmp) +{ +if (!bmp) +return; + +g_free(bmp->bitmapname); +g_free(bmp->alias); +g_free(bmp->sourcebitmap); +g_free(bmp); +} + +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMigrationBlockDirtyBitmapsDiskBitmap, + qemuMigrationBlockDirtyBitmapsDiskBitmapFree); + + +static void +qemuMigrationBlockDirtyBitmapsDiskFree(qemuMigrationBlockDirtyBitmapsDiskPtr dsk) +{ +if (!dsk) +return; + +g_free(dsk->target); +if (dsk->bitmaps) +g_slist_free_full(dsk->bitmaps, + (GDestroyNotify) qemuMigrationBlockDirtyBitmapsDiskBitmapFree); +g_free(dsk); +} + +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuMigrationBlockDirtyBitmapsDisk, + qemuMigrationBlockDirtyBitmapsDiskFree); + + void qemuMigrationCookieFree(qemuMigrationCookiePtr mig) { @@ -135,6 +169,9 @@ qemuMigrationCookieFree(qemuMigrationCookiePtr mig) g_clear_pointer(&mig->jobInfo, qemuDomainJobInfoFree); virCPUDefFree(mig->cpu); qemuMigrationCookieCapsFree(mig->caps); +if (mig->blockDirtyBitmaps) +g_slist_free_full(mig->blockDirtyBitmaps, + (GDestroyNotify) qemuMigrationBlockDirtyBitmapsDiskFree); g_free(mig); } @@ -758,6 +795,48 @@ qemuMigrationCookieNBDXMLFormat(qemuMigrationCookieNBDPtr nbd, } +static void +qemuMigrationCookieBlockDirtyBitmapsFormat(virBufferPtr buf, + GSList *bitmaps) +{ +g_auto(virBuffer) disksBuf = VIR_BUFFER_INIT_CHILD(buf); +GSList *nextdisk; + +for (nextdisk = bitmaps; nextdisk; nextdisk = nextdisk->next) { +qemuMigrationBlockDirtyBitmapsDiskPtr disk = nextdisk->data; +g_auto(virBuffer) diskAttrBuf = VIR_BUFFER_INITIALIZER; +g_auto(virBuffer) diskChildBuf = VIR_BUFFER_INIT_CHILD(&disksBuf); +bool hasBitmaps = false; +GSList *nextbitmap; + +if (disk->skip || !disk->bitmaps) +continue; + +for (nextbitmap = disk->bitmaps; nextbitmap; nextbitmap = nextbitmap->next) { +qemuMigrationBlockDirtyBitmapsDiskBitmapPtr bitmap = nextbitmap->data; + +if (bitmap->skip) +continue; + +virBufferAsprintf(&diskChildBuf, + "\n", + bitmap->bitmapname, bitmap->alias); + +hasBitmaps = true; +} + +if (!hasBitmaps) +continue; + +virBufferAsprintf(&diskAttrBuf, " target='%s'", disk->target); +virXMLFormatElement(&disksBuf, "disk", &diskAttrBuf, &diskChildBuf); +} + + +virXMLFormatElement(buf, "blockDirtyBitmaps", NULL, &disksBuf); +} + + int qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver, virQEMUCapsPtr qemuCaps, @@ -829,6 +908,9 @@ qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver, if (mig->flags & QEMU_MIGRATION_COOKIE_CAPS) qemuMigrationCookieCapsXMLFormat(buf, mig->caps); +if (mig->flags & QEMU_MIGRATION_COOKIE_BLOCK_DIRTY_BITMAPS) +qemuMigrationCookieBlockDirtyBitmapsFormat(buf, mig->blockDirtyBitmaps); + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "\n"); return 0; @@ -1132,6 +1214,53 @@ qemuMigrationCookieXMLParseMandatoryFeatures(xmlXPathContextPtr ctxt, } +static int +qemuMigrationCookieBlockDirtyBitmapsParse(xmlXPathContextPtr ctxt, + qemuMigrationCookiePtr mig) +{ +g_autoslist(qemuMigrationBlockDirtyBitmapsDisk) disks = NULL; +g_autofree xmlNodePtr *disknodes = NULL; +int ndisknodes; +size_t i; +VIR_XPATH_NODE_AUTORESTORE(ctxt) + +if ((ndisknodes = virXPathNodeSet("./blockDirtyBitmaps/disk", ctxt, &disknodes)) < 0) +return -1; + +for (i = 0; i < ndisknodes; i++) { +GSList *bitmaps = NULL; +qemuMigrationBlockD
[PATCH 06/19] testutils: virTestRewrapFile: Rewrap also '.argv' files
The suffix is used for output files of 'storagevolxml2argvtest. Signed-off-by: Peter Krempa --- tests/testutils.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/testutils.c b/tests/testutils.c index 7ecf7923b8..8734790457 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -332,6 +332,7 @@ virTestRewrapFile(const char *filename) g_autoptr(virCommand) cmd = NULL; if (!(virStringHasSuffix(filename, ".args") || + virStringHasSuffix(filename, ".argv") || virStringHasSuffix(filename, ".ldargs"))) return 0; -- 2.29.2
[PATCH 09/19] qemu: monitor: Introduce qemuMonitorBitmapRemove
The non-transaction wrapper is useful for code paths which want to delete individual bitmaps or for cleanup after a failed job where we want to attempt to delete every bitmap individually to prevent a failure from cleaning up the rest. Signed-off-by: Peter Krempa --- src/qemu/qemu_monitor.c | 13 + src/qemu/qemu_monitor.h | 5 + src/qemu/qemu_monitor_json.c | 24 src/qemu/qemu_monitor_json.h | 6 ++ tests/qemumonitorjsontest.c | 2 ++ 5 files changed, 50 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 14966d4096..2fb7d6569b 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4645,6 +4645,19 @@ qemuMonitorTransactionBitmapRemove(virJSONValuePtr actions, } +int +qemuMonitorBitmapRemove(qemuMonitorPtr mon, +const char *node, +const char *name) +{ +VIR_DEBUG("node='%s', name='%s'", node, name); + +QEMU_CHECK_MONITOR(mon); + +return qemuMonitorJSONBitmapRemove(mon, node, name); +} + + int qemuMonitorTransactionBitmapEnable(virJSONValuePtr actions, const char *node, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 7a039641ba..544611b443 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1478,6 +1478,11 @@ int qemuMonitorTransactionBitmapRemove(virJSONValuePtr actions, const char *node, const char *name); + +int +qemuMonitorBitmapRemove(qemuMonitorPtr mon, +const char *node, +const char *name); int qemuMonitorTransactionBitmapEnable(virJSONValuePtr actions, const char *node, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 65c30e3bb4..f19bb589b2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -9253,6 +9253,30 @@ qemuMonitorJSONTransactionBitmapRemove(virJSONValuePtr actions, } +int +qemuMonitorJSONBitmapRemove(qemuMonitorPtr mon, +const char *node, +const char *name) +{ +g_autoptr(virJSONValue) cmd = NULL; +g_autoptr(virJSONValue) reply = NULL; + +if (!(cmd = qemuMonitorJSONMakeCommand("block-dirty-bitmap-remove", + "s:node", node, + "s:name", name, + NULL))) +return -1; + +if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) +return -1; + +if (qemuMonitorJSONCheckError(cmd, reply) < 0) +return -1; + +return 0; +} + + int qemuMonitorJSONTransactionBitmapEnable(virJSONValuePtr actions, const char *node, diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index ba1531fee8..8e2ab24a59 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -659,6 +659,12 @@ int qemuMonitorJSONTransactionBitmapRemove(virJSONValuePtr actions, const char *node, const char *name); + +int +qemuMonitorJSONBitmapRemove(qemuMonitorPtr mon, +const char *node, +const char *name); + int qemuMonitorJSONTransactionBitmapEnable(virJSONValuePtr actions, const char *node, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 956423f10f..af02c0b0e6 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1215,6 +1215,7 @@ GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayOpen, "foodev", true) GEN_TEST_FUNC(qemuMonitorJSONBlockdevTrayClose, "foodev") GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumRemove, "foodev") GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumInsert, "foodev", "newnode") +GEN_TEST_FUNC(qemuMonitorJSONBitmapRemove, "foodev", "newnode") GEN_TEST_FUNC(qemuMonitorJSONJobDismiss, "jobname") GEN_TEST_FUNC(qemuMonitorJSONJobCancel, "jobname", false) GEN_TEST_FUNC(qemuMonitorJSONJobComplete, "jobname") @@ -3132,6 +3133,7 @@ mymain(void) DO_TEST_GEN(qemuMonitorJSONBlockdevTrayClose); DO_TEST_GEN(qemuMonitorJSONBlockdevMediumRemove); DO_TEST_GEN(qemuMonitorJSONBlockdevMediumInsert); +DO_TEST_GEN(qemuMonitorJSONBitmapRemove); DO_TEST_GEN(qemuMonitorJSONJobDismiss); DO_TEST_GEN(qemuMonitorJSONJobCancel); DO_TEST_GEN(qemuMonitorJSONJobComplete); -- 2.29.2
[PATCH 14/19] qemu: domain: Store list of temporary bitmaps for migration in status XML
Add status XML infrastructure for storing a list of block dirty bitmaps which are temporarily used when migrating a VM with VIR_MIGRATE_NON_SHARED_DISK for cleanup after a libvirtd restart during migration. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c | 90 -- src/qemu/qemu_domain.h | 15 +++ 2 files changed, 102 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 53b4fb5f4f..ed37917670 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -86,6 +86,18 @@ qemuJobAllocPrivate(void) } +void +qemuDomainJobPrivateMigrateTempBitmapFree(qemuDomainJobPrivateMigrateTempBitmapPtr bmp) +{ +if (!bmp) +return; + +g_free(bmp->nodename); +g_free(bmp->bitmapname); +g_free(bmp); +} + + static void qemuJobFreePrivate(void *opaque) { @@ -95,6 +107,9 @@ qemuJobFreePrivate(void *opaque) return; qemuMigrationParamsFree(priv->migParams); +if (priv->migTempBitmaps) +g_slist_free_full(priv->migTempBitmaps, + (GDestroyNotify) qemuDomainJobPrivateMigrateTempBitmapFree); VIR_FREE(priv); } @@ -165,6 +180,28 @@ qemuDomainObjPrivateXMLFormatNBDMigration(virBufferPtr buf, return 0; } + +static void +qemuDomainObjPrivateXMLFormatMigrationBlockDirtyBitmapsTemp(virBufferPtr buf, +GSList *bitmaps) +{ +g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); +GSList *next; + +for (next = bitmaps; next; next = next->next) { +qemuDomainJobPrivateMigrateTempBitmapPtr t = next->data; +g_auto(virBuffer) bitmapBuf = VIR_BUFFER_INITIALIZER; + +virBufferAsprintf(&bitmapBuf, " name='%s'", t->bitmapname); +virBufferAsprintf(&bitmapBuf, " nodename='%s'", t->nodename); + +virXMLFormatElement(&childBuf, "bitmap", &bitmapBuf, NULL); +} + +virXMLFormatElement(buf, "tempBlockDirtyBitmaps", NULL, &childBuf); +} + + static int qemuDomainFormatJobPrivate(virBufferPtr buf, qemuDomainJobObjPtr job, @@ -172,9 +209,14 @@ qemuDomainFormatJobPrivate(virBufferPtr buf, { qemuDomainJobPrivatePtr priv = job->privateData; -if (job->asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT && -qemuDomainObjPrivateXMLFormatNBDMigration(buf, vm) < 0) -return -1; +if (job->asyncJob == QEMU_ASYNC_JOB_MIGRATION_OUT) { +if (qemuDomainObjPrivateXMLFormatNBDMigration(buf, vm) < 0) +return -1; + +if (priv->migTempBitmaps) +qemuDomainObjPrivateXMLFormatMigrationBlockDirtyBitmapsTemp(buf, + priv->migTempBitmaps); +} if (priv->migParams) qemuMigrationParamsFormat(buf, priv->migParams); @@ -267,6 +309,45 @@ qemuDomainObjPrivateXMLParseJobNBD(virDomainObjPtr vm, return 0; } + +static int +qemuDomainParseJobPrivateXMLMigrationBlockDirtyBitmapsTemp(qemuDomainJobPrivatePtr jobPriv, + xmlXPathContextPtr ctxt) +{ +g_autoslist(qemuDomainJobPrivateMigrateTempBitmap) bitmaps = NULL; +g_autofree xmlNodePtr *nodes = NULL; +size_t i; +int n; + +if ((n = virXPathNodeSet("./tempBlockDirtyBitmaps/bitmap", ctxt, &nodes)) < 0) +return -1; + +if (n == 0) +return 0; + +for (i = 0; i < n; i++) { +g_autofree char *bitmapname = virXMLPropString(nodes[i], "name"); +g_autofree char *nodename = virXMLPropString(nodes[i], "nodename"); +qemuDomainJobPrivateMigrateTempBitmapPtr bmp; + +if (!bitmapname || !nodename) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed ")); +return -1; +} + +bmp = g_new0(qemuDomainJobPrivateMigrateTempBitmap, 1); +bmp->nodename = g_steal_pointer(&nodename); +bmp->bitmapname = g_steal_pointer(&bitmapname); + +bitmaps = g_slist_prepend(bitmaps, bmp); +} + +jobPriv->migTempBitmaps = g_slist_reverse(g_steal_pointer(&bitmaps)); +return 0; +} + + static int qemuDomainParseJobPrivate(xmlXPathContextPtr ctxt, qemuDomainJobObjPtr job, @@ -277,6 +358,9 @@ qemuDomainParseJobPrivate(xmlXPathContextPtr ctxt, if (qemuDomainObjPrivateXMLParseJobNBD(vm, ctxt) < 0) return -1; +if (qemuDomainParseJobPrivateXMLMigrationBlockDirtyBitmapsTemp(priv, ctxt) < 0) +return -1; + if (qemuMigrationParamsParse(ctxt, &priv->migParams) < 0) return -1; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 7453881a31..d8ed3aebe4 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -487,6 +487,20 @@ struct _qemuDomainXmlNsDef { char **capsdel; }; + +typedef struct _qemuDomainJobPrivateMigrateTempBitmap qemuDomainJobPrivateMigrateTempBitmap; +typedef qemuDoma
[PATCH 11/19] qemu: migration_params: Add infrastructure for 'dirty-bitmaps' migration feature
Add the migration capability flag and the propagation of the corresponding mapping configuration. The mapping will be produced from the bitmaps on disk depending on both sides of the migration and the necessity to perform merges. Signed-off-by: Peter Krempa --- src/qemu/qemu_migration_params.c | 24 src/qemu/qemu_migration_params.h | 5 + 2 files changed, 29 insertions(+) diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 510dad783a..8f491e0ed2 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -63,6 +63,7 @@ struct _qemuMigrationParams { unsigned long long compMethods; /* bit-wise OR of qemuMigrationCompressMethod */ virBitmapPtr caps; qemuMigrationParamValue params[QEMU_MIGRATION_PARAM_LAST]; +virJSONValuePtr blockDirtyBitmapMapping; }; typedef enum { @@ -89,6 +90,7 @@ VIR_ENUM_IMPL(qemuMigrationCapability, "pause-before-switchover", "late-block-activate", "multifd", + "dirty-bitmaps", ); @@ -265,6 +267,7 @@ qemuMigrationParamsFree(qemuMigrationParamsPtr migParams) } virBitmapFree(migParams->caps); +virJSONValueFree(migParams->blockDirtyBitmapMapping); g_free(migParams); } @@ -524,6 +527,20 @@ qemuMigrationParamsSetCompression(virTypedParameterPtr params, } +void +qemuMigrationParamsSetBlockDirtyBitmapMapping(qemuMigrationParamsPtr migParams, + virJSONValuePtr *params) +{ +virJSONValueFree(migParams->blockDirtyBitmapMapping); +migParams->blockDirtyBitmapMapping = g_steal_pointer(params); + +if (migParams->blockDirtyBitmapMapping) +ignore_value(virBitmapSetBit(migParams->caps, QEMU_MIGRATION_CAP_BLOCK_DIRTY_BITMAPS)); +else +ignore_value(virBitmapClearBit(migParams->caps, QEMU_MIGRATION_CAP_BLOCK_DIRTY_BITMAPS)); +} + + qemuMigrationParamsPtr qemuMigrationParamsFromFlags(virTypedParameterPtr params, int nparams, @@ -747,6 +764,12 @@ qemuMigrationParamsToJSON(qemuMigrationParamsPtr migParams) return NULL; } +if (migParams->blockDirtyBitmapMapping && +virJSONValueObjectAppend(params, "block-bitmap-mapping", + migParams->blockDirtyBitmapMapping) < 0) +return NULL; +migParams->blockDirtyBitmapMapping = NULL; + return g_steal_pointer(¶ms); } @@ -1202,6 +1225,7 @@ qemuMigrationParamsReset(virQEMUDriverPtr driver, goto cleanup; qemuMigrationParamsResetTLS(driver, vm, asyncJob, origParams, apiFlags); +/* We don't reset 'block-bitmap-mapping' as it can't be unset */ cleanup: virErrorRestore(&err); diff --git a/src/qemu/qemu_migration_params.h b/src/qemu/qemu_migration_params.h index 9876101bfc..f1db42ce94 100644 --- a/src/qemu/qemu_migration_params.h +++ b/src/qemu/qemu_migration_params.h @@ -39,6 +39,7 @@ typedef enum { QEMU_MIGRATION_CAP_PAUSE_BEFORE_SWITCHOVER, QEMU_MIGRATION_CAP_LATE_BLOCK_ACTIVATE, QEMU_MIGRATION_CAP_MULTIFD, +QEMU_MIGRATION_CAP_BLOCK_DIRTY_BITMAPS, QEMU_MIGRATION_CAP_LAST } qemuMigrationCapability; @@ -132,6 +133,10 @@ qemuMigrationParamsGetULL(qemuMigrationParamsPtr migParams, qemuMigrationParam param, unsigned long long *value); +void +qemuMigrationParamsSetBlockDirtyBitmapMapping(qemuMigrationParamsPtr migParams, + virJSONValuePtr *params); + int qemuMigrationParamsCheck(virQEMUDriverPtr driver, virDomainObjPtr vm, -- 2.29.2
[PATCH 16/19] tests: qemumigrationcookie: Add testing for block dirty bitmap migration
Test the XML infrastructure for migration cookie element as well as the conversion to migration parameters for QMP schema validation. Signed-off-by: Peter Krempa --- tests/meson.build | 2 +- .../nbd-bitmaps-xml2xml-in.xml| 52 ++ .../nbd-bitmaps-xml2xml-migparams.json| 25 +++ .../nbd-bitmaps-xml2xml-out.xml | 51 ++ tests/qemumigrationcookiexmltest.c| 166 +++--- 5 files changed, 269 insertions(+), 27 deletions(-) create mode 100644 tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-in.xml create mode 100644 tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-migparams.json create mode 100644 tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-out.xml diff --git a/tests/meson.build b/tests/meson.build index 0de0783839..b9b2255666 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -452,7 +452,7 @@ if conf.has('WITH_QEMU') { 'name': 'qemuhotplugtest', 'link_with': [ test_qemu_driver_lib, test_utils_qemu_monitor_lib ], 'link_whole': [ test_utils_qemu_lib ] }, { 'name': 'qemumemlocktest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib ] }, { 'name': 'qemumigparamstest', 'link_with': [ test_qemu_driver_lib, test_utils_qemu_monitor_lib ], 'link_whole': [ test_utils_qemu_lib ] }, -{ 'name': 'qemumigrationcookiexmltest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] }, +{ 'name': 'qemumigrationcookiexmltest', 'link_with': [ test_qemu_driver_lib, test_utils_qemu_monitor_lib ], 'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] }, { 'name': 'qemumonitorjsontest', 'link_with': [ test_qemu_driver_lib, test_utils_qemu_monitor_lib ], 'link_whole': [ test_utils_qemu_lib ] }, { 'name': 'qemusecuritytest', 'sources': [ 'qemusecuritytest.c', 'qemusecuritymock.c' ], 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib ] }, { 'name': 'qemustatusxml2xmltest', 'link_with': [ test_qemu_driver_lib ], 'link_whole': [ test_utils_qemu_lib, test_file_wrapper_lib ] }, diff --git a/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-in.xml b/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-in.xml new file mode 100644 index 00..b219c25f27 --- /dev/null +++ b/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-in.xml @@ -0,0 +1,52 @@ + + migr + 10b01607-0323-486b-afe2-3014a8a5b98b + sourcehost + 1f5e0da0-fecf-413f-9bf1-1aa9c21e71e4 + + + + + + +EPYC-Rome +AMD + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-migparams.json b/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-migparams.json new file mode 100644 index 00..100da7c270 --- /dev/null +++ b/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-migparams.json @@ -0,0 +1,25 @@ +{ + "block-bitmap-mapping": [ +{ + "node-name": "libvirt-1-format", + "alias": "vda", + "bitmaps": [ +{ + "name": "a", + "alias": "libvirt-vda-a", + "transform": { +"persistent": true + } +}, +{ + "name": "b", + "alias": "libvirt-vda-b" +}, +{ + "name": "c", + "alias": "libvirt-vda-c" +} + ] +} + ] +} diff --git a/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-out.xml b/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-out.xml new file mode 100644 index 00..09b6fa291c --- /dev/null +++ b/tests/qemumigrationcookiexmldata/nbd-bitmaps-xml2xml-out.xml @@ -0,0 +1,51 @@ + + migr + 10b01607-0323-486b-afe2-3014a8a5b98b + hostname + 4a802f00-4cba-5df6-9679-a08c4c5b577f + + + + + +EPYC-Rome +AMD + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/qemumigrationcookiexmltest.c b/tests/qemumigrationcookiexmltest.c index 5fe0ba8a8a..7f2437a7fe 100644 --- a/tests/qemumigrationcookiexmltest.c +++ b/tests/qemumigrationcookiexmltest.c @@ -25,9 +25,13 @@ #include "internal.h" #include "testutilsqemu.h" +#include "testutilsqemuschema.h" #include "configmake.h" +#define LIBVIRT_QEMU_MIGRATION_PARAMSPRIV_H_ALLOW + #include "qemu/qemu_migration_cookie.h" +#include "qemu/qemu_migration_paramspriv.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -61,13 +65,33 @@ struct testQemuMigrationCookieData { qemuMigrationParty cookiePopulateParty; +qemuMigrationCookiePtr cookie; + char *xmlstr; int xmlstrlen; char *infile; char *outfile; +char
[PATCH 17/19] qemu: migration: Clean up temporary bitmaps when cancelling a migration
In case when the block migration job required temporary bitmaps for merging the appropriate checkpoints we need to clean them up when cancelling the job. On success we don't need to do that though as the bitmaps are just temporary thus are not written to disk. Signed-off-by: Peter Krempa --- src/qemu/qemu_migration.c | 31 +++ 1 file changed, 31 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 37f0d43d24..36424f8493 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -834,6 +834,32 @@ qemuMigrationSrcNBDCopyCancel(virQEMUDriverPtr driver, } +static int +qemuMigrationSrcCancelRemoveTempBitmaps(virDomainObjPtr vm, +qemuDomainAsyncJob asyncJob) +{ +qemuDomainObjPrivatePtr priv = vm->privateData; +virQEMUDriverPtr driver = priv->driver; +qemuDomainJobPrivatePtr jobPriv = priv->job.privateData; +GSList *next; + +if (!jobPriv->migTempBitmaps) +return 0; + +for (next = jobPriv->migTempBitmaps; next; next = next->next) { +qemuDomainJobPrivateMigrateTempBitmapPtr t = next->data; + +if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) +return -1; +qemuMonitorBitmapRemove(priv->mon, t->nodename, t->bitmapname); +if (qemuDomainObjExitMonitor(driver, vm) < 0) +return -1; +} + +return 0; +} + + static virStorageSourcePtr qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource(virDomainDiskDefPtr disk, const char *host, @@ -3999,6 +4025,8 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_MIGRATION_OUT, dconn); +qemuMigrationSrcCancelRemoveTempBitmaps(vm, QEMU_ASYNC_JOB_MIGRATION_OUT); + if (priv->job.current->status != QEMU_DOMAIN_JOB_STATUS_CANCELED) priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_FAILED; } @@ -5701,6 +5729,9 @@ qemuMigrationSrcCancel(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_NONE, NULL) < 0) return -1; +if (qemuMigrationSrcCancelRemoveTempBitmaps(vm, QEMU_ASYNC_JOB_NONE) < 0) +return -1; + return 0; } -- 2.29.2
[PATCH 19/19] qemu: capabilities: Enable QEMU_CAPS_INCREMENTAL_BACKUP
For incremental backup we need QEMU_CAPS_BLOCKDEV, QEMU_CAPS_BLOCKDEV_REOPEN, QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING. Signed-off-by: Peter Krempa --- src/qemu/qemu_capabilities.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 38555dde98..7cc2532954 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -5164,8 +5164,10 @@ virQEMUCapsInitQMPVersionCaps(virQEMUCapsPtr qemuCaps) void virQEMUCapsInitProcessCapsInterlock(virQEMUCapsPtr qemuCaps) { -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) -virQEMUCapsClear(qemuCaps, QEMU_CAPS_INCREMENTAL_BACKUP); +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV) && +virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV_REOPEN) && +virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING)) +virQEMUCapsSet(qemuCaps, QEMU_CAPS_INCREMENTAL_BACKUP); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_USB_STORAGE) && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_STORAGE_WERROR)) { -- 2.29.2
[PATCH 15/19] tests: qemustatusxml2xml: Add status XML from migration with bitmaps
The XML sample shows the status XML when migrating with bitmaps including the element added in previous commit. It will also be used for the migration cookie test. Signed-off-by: Peter Krempa --- .../migration-out-nbd-bitmaps-in.xml | 574 ++ .../migration-out-nbd-bitmaps-out.xml | 1 + tests/qemustatusxml2xmltest.c | 1 + 3 files changed, 576 insertions(+) create mode 100644 tests/qemustatusxml2xmldata/migration-out-nbd-bitmaps-in.xml create mode 12 tests/qemustatusxml2xmldata/migration-out-nbd-bitmaps-out.xml diff --git a/tests/qemustatusxml2xmldata/migration-out-nbd-bitmaps-in.xml b/tests/qemustatusxml2xmldata/migration-out-nbd-bitmaps-in.xml new file mode 100644 index 00..b3d24eda98 --- /dev/null +++ b/tests/qemustatusxml2xmldata/migration-out-nbd-bitmaps-in.xml @@ -0,0 +1,574 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +EPYC-Rome +AMD + + + + + + + + + + + + + + + + + + + + + + + + + + -2 + +migr +10b01607-0323-486b-afe2-3014a8a5b98b +1024000 +1024000 +1 + + /machine + + + hvm + + + + + + + + + EPYC-Rome + AMD + + + + + + + + + + + + + + + + + + + + + + + + +destroy +restart +restart + + + + + + /home/pipo/git/qemu.git/build/x86_64-softmmu/qemu-system-x86_64 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
[PATCH 07/19] storagevolxml2argvdata: Rewrap all output files
Use scripts/test-wrap-argv.py to rewrap the output files so that any further changes don't introduce churn since we are rewrapping the output automatically now. Signed-off-by: Peter Krempa --- tests/storagevolxml2argvdata/iso-input.argv | 6 +++-- tests/storagevolxml2argvdata/iso.argv | 4 +++- .../logical-from-qcow2.argv | 6 +++-- tests/storagevolxml2argvdata/luks-cipher.argv | 8 --- .../luks-convert-encrypt.argv | 23 --- .../luks-convert-encrypt2fileqcow2.argv | 19 ++- .../luks-convert-encrypt2fileraw.argv | 20 ++-- .../luks-convert-qcow2.argv | 21 +++-- .../storagevolxml2argvdata/luks-convert.argv | 20 ++-- tests/storagevolxml2argvdata/luks.argv| 8 --- tests/storagevolxml2argvdata/qcow2-1.1.argv | 8 --- .../storagevolxml2argvdata/qcow2-compat.argv | 8 --- .../qcow2-from-logical-compat.argv| 8 --- tests/storagevolxml2argvdata/qcow2-lazy.argv | 9 +--- ...ow2-nobacking-convert-prealloc-compat.argv | 10 +--- .../qcow2-nobacking-prealloc-compat.argv | 8 --- .../qcow2-nocapacity-convert-prealloc.argv| 9 +--- .../qcow2-nocapacity.argv | 6 ++--- .../qcow2-nocow-compat.argv | 9 +--- tests/storagevolxml2argvdata/qcow2-nocow.argv | 9 +--- .../qcow2-zerocapacity.argv | 5 +++- 21 files changed, 147 insertions(+), 77 deletions(-) diff --git a/tests/storagevolxml2argvdata/iso-input.argv b/tests/storagevolxml2argvdata/iso-input.argv index 1db0265945..203f27ca22 100644 --- a/tests/storagevolxml2argvdata/iso-input.argv +++ b/tests/storagevolxml2argvdata/iso-input.argv @@ -1,2 +1,4 @@ -qemu-img convert -f raw -O raw /var/lib/libvirt/images/test.iso \ -/var/lib/libvirt/images/sparse.img +qemu-img \ +convert \ +-f raw \ +-O raw /var/lib/libvirt/images/test.iso /var/lib/libvirt/images/sparse.img diff --git a/tests/storagevolxml2argvdata/iso.argv b/tests/storagevolxml2argvdata/iso.argv index 172a255eb8..f74505ecec 100644 --- a/tests/storagevolxml2argvdata/iso.argv +++ b/tests/storagevolxml2argvdata/iso.argv @@ -1 +1,3 @@ -qemu-img create -f raw /var/lib/libvirt/images/test.iso 1024K +qemu-img \ +create \ +-f raw /var/lib/libvirt/images/test.iso 1024K diff --git a/tests/storagevolxml2argvdata/logical-from-qcow2.argv b/tests/storagevolxml2argvdata/logical-from-qcow2.argv index 7beded8478..c1b3cef4b9 100644 --- a/tests/storagevolxml2argvdata/logical-from-qcow2.argv +++ b/tests/storagevolxml2argvdata/logical-from-qcow2.argv @@ -1,2 +1,4 @@ -qemu-img convert -f qcow2 -O raw /var/lib/libvirt/images/OtherDemo.img \ -/dev/HostVG/Swap +qemu-img \ +convert \ +-f qcow2 \ +-O raw /var/lib/libvirt/images/OtherDemo.img /dev/HostVG/Swap diff --git a/tests/storagevolxml2argvdata/luks-cipher.argv b/tests/storagevolxml2argvdata/luks-cipher.argv index a8a19f03ff..bb4438949c 100644 --- a/tests/storagevolxml2argvdata/luks-cipher.argv +++ b/tests/storagevolxml2argvdata/luks-cipher.argv @@ -1,5 +1,7 @@ -qemu-img create -f luks \ +qemu-img \ +create \ +-f luks \ --object secret,id=LuksDemo.img_encrypt0,file=/path/to/secretFile \ -o key-secret=LuksDemo.img_encrypt0,cipher-alg=serpent-256,cipher-mode=cbc,\ -hash-alg=sha256,ivgen-alg=plain64,ivgen-hash-alg=sha256 \ -/var/lib/libvirt/images/LuksDemo.img 5242880K +hash-alg=sha256,ivgen-alg=plain64,\ +ivgen-hash-alg=sha256 /var/lib/libvirt/images/LuksDemo.img 5242880K diff --git a/tests/storagevolxml2argvdata/luks-convert-encrypt.argv b/tests/storagevolxml2argvdata/luks-convert-encrypt.argv index 78bce96aaa..456ae621d1 100644 --- a/tests/storagevolxml2argvdata/luks-convert-encrypt.argv +++ b/tests/storagevolxml2argvdata/luks-convert-encrypt.argv @@ -1,11 +1,18 @@ -qemu-img create -f luks \ +qemu-img \ +create \ +-f luks \ --object secret,id=LuksDemo.img_encrypt0,file=/path/to/secretFile \ --o key-secret=LuksDemo.img_encrypt0 \ -/var/lib/libvirt/images/LuksDemo.img 5242880K -qemu-img convert --image-opts -n --target-image-opts \ +-o key-secret=LuksDemo.img_encrypt0 /var/lib/libvirt/images/LuksDemo.img \ +5242880K +qemu-img \ +convert \ +--image-opts \ +-n \ +--target-image-opts \ --object secret,id=LuksDemo.img_encrypt0,file=/path/to/secretFile \ ---object secret,id=OtherDemo.img_encrypt0,file=/path/to/inputSecretFile \ -driver=luks,file.filename=/var/lib/libvirt/images/OtherDemo.img,\ -key-secret=OtherDemo.img_encrypt0 \ -driver=luks,file.filename=/var/lib/libvirt/images/LuksDemo.img,\ +--object secret,id=OtherDemo.img_encrypt0,\ +file=/path/to/inputSecretFile driver=luks,\ +file.filename=/var/lib/libvirt/images/OtherDemo.img,\ +key-secret=OtherDemo.img_encrypt0 driver=luks,\ +file.filename=/var/lib/libvirt/images/LuksDemo.img,\ key-secret=LuksDemo.img_encrypt0 diff --git a/tests/storagevolxml2argvdata/luks-convert-encrypt2fileqcow2.argv b/tests/storagevolxml2argvdata/luks-convert-encrypt
[PATCH 13/19] qemu: migration_cookie: Add helpers for transforming the cookie into migration params
'qemuMigrationCookieBlockDirtyBitmapsMatchDisks' maps the bitmaps from the migration cookie to actual disk objects definition pointers. 'qemuMigrationCookieBlockDirtyBitmapsToParams' converts the bitmap definitions from the migration cookie into parameters for the 'block-bitmap-mapping' migration parameter. Signed-off-by: Peter Krempa --- src/qemu/qemu_migration_cookie.c | 115 +++ src/qemu/qemu_migration_cookie.h | 7 ++ 2 files changed, 122 insertions(+) diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 94ba9c83d0..ffb7533b6a 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -1568,3 +1568,118 @@ qemuMigrationCookieParse(virQEMUDriverPtr driver, return g_steal_pointer(&mig); } + + +/** + * qemuMigrationCookieBlockDirtyBitmapsMatchDisks: + * @def: domain definition + * @disks: list of qemuMigrationBlockDirtyBitmapsDiskPtr + * + * Matches all of the @disks to the actual domain disk definition objects + * by looking up the target. + */ +int +qemuMigrationCookieBlockDirtyBitmapsMatchDisks(virDomainDefPtr def, + GSList *disks) +{ +GSList *next; + +for (next = disks; next; next = next->next) { +qemuMigrationBlockDirtyBitmapsDiskPtr disk = next->data; + +if (!(disk->disk = virDomainDiskByTarget(def, disk->target))) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Can't find disk '%s' in domain definition"), + disk->target); +return -1; +} + +disk->nodename = disk->disk->src->nodeformat; +} + +return 0; +} + + +/** + * qemuMigrationCookieBlockDirtyBitmapsToParams: + * @disks: list of qemuMigrationBlockDirtyBitmapsDisk + * @mapping: filled with resulting mapping + * + * Converts @disks into the arguments for 'block-bitmap-mapping' migration + * parameter. + */ +int +qemuMigrationCookieBlockDirtyBitmapsToParams(GSList *disks, + virJSONValuePtr *mapping) +{ +g_autoptr(virJSONValue) map = virJSONValueNewArray(); +bool hasDisks = false; +GSList *nextdisk; + +for (nextdisk = disks; nextdisk; nextdisk = nextdisk->next) { +qemuMigrationBlockDirtyBitmapsDiskPtr disk = nextdisk->data; +g_autoptr(virJSONValue) jsondisk = NULL; +g_autoptr(virJSONValue) jsonbitmaps = virJSONValueNewArray(); +bool hasBitmaps = false; +GSList *nextbitmap; + +if (disk->skip || !disk->bitmaps) +continue; + +for (nextbitmap = disk->bitmaps; nextbitmap; nextbitmap = nextbitmap->next) { +qemuMigrationBlockDirtyBitmapsDiskBitmapPtr bitmap = nextbitmap->data; +g_autoptr(virJSONValue) jsonbitmap = NULL; +g_autoptr(virJSONValue) transform = NULL; +const char *bitmapname = bitmap->sourcebitmap; + +if (bitmap->skip) +continue; + +/* if there isn't an override, use the real name */ +if (!bitmapname) +bitmapname = bitmap->bitmapname; + +if (bitmap->persistent == VIR_TRISTATE_BOOL_YES) { +if (virJSONValueObjectCreate(&transform, + "b:persistent", true, NULL) < 0) +return -1; +} + +if (virJSONValueObjectCreate(&jsonbitmap, + "s:name", bitmapname, + "s:alias", bitmap->alias, + "A:transform", &transform, + NULL) < 0) +return -1; + +if (virJSONValueArrayAppend(jsonbitmaps, jsonbitmap) < 0) +return -1; + +jsonbitmap = NULL; +hasBitmaps = true; +} + +if (!hasBitmaps) +continue; + +if (virJSONValueObjectCreate(&jsondisk, + "s:node-name", disk->nodename, + "s:alias", disk->target, + "a:bitmaps", &jsonbitmaps, + NULL) < 0) +return -1; + +if (virJSONValueArrayAppend(map, jsondisk) < 0) +return -1; + +jsondisk = NULL; +hasDisks = true; +} + +if (!hasDisks) +return 0; + +*mapping = g_steal_pointer(&map); +return 0; +} diff --git a/src/qemu/qemu_migration_cookie.h b/src/qemu/qemu_migration_cookie.h index 8636f955da..f8393e0ce0 100644 --- a/src/qemu/qemu_migration_cookie.h +++ b/src/qemu/qemu_migration_cookie.h @@ -226,3 +226,10 @@ qemuMigrationCookieXMLFormat(virQEMUDriverPtr driver, virQEMUCapsPtr qemuCaps, virBufferPtr buf, qemuMigrationCookiePtr mig); + +int qemuMigrationCoo
[PATCH 10/19] qemu: blockjob: Use qemuMonitorBitmapRemove for single bitmap removal
There's no need in the cleanup steps to invoke a transaction to delete a single bitmap. Signed-off-by: Peter Krempa --- src/qemu/qemu_blockjob.c | 24 +++- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 582fe45c66..aa065b1319 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -1420,7 +1420,6 @@ qemuBlockJobProcessEventFailedActiveCommit(virQEMUDriverPtr driver, qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivatePtr priv = vm->privateData; -g_autoptr(virJSONValue) actions = virJSONValueNewArray(); virDomainDiskDefPtr disk = job->disk; VIR_DEBUG("active commit job '%s' on VM '%s' failed", job->name, vm->def->name); @@ -1428,13 +1427,12 @@ qemuBlockJobProcessEventFailedActiveCommit(virQEMUDriverPtr driver, if (!disk) return; -ignore_value(qemuMonitorTransactionBitmapRemove(actions, disk->mirror->nodeformat, - "libvirt-tmp-activewrite")); - if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0) return; -qemuMonitorTransaction(priv->mon, &actions); +qemuMonitorBitmapRemove(priv->mon, +disk->mirror->nodeformat, +"libvirt-tmp-activewrite"); if (qemuDomainObjExitMonitor(priv->driver, vm) < 0) return; @@ -1502,7 +1500,6 @@ qemuBlockJobProcessEventConcludedBackup(virQEMUDriverPtr driver, unsigned long long progressTotal) { g_autoptr(qemuBlockStorageSourceAttachData) backend = NULL; -g_autoptr(virJSONValue) actions = NULL; qemuBackupNotifyBlockjobEnd(vm, job->disk, newstate, job->errmsg, progressCurrent, progressTotal, asyncJob); @@ -1511,23 +1508,16 @@ qemuBlockJobProcessEventConcludedBackup(virQEMUDriverPtr driver, !(backend = qemuBlockStorageSourceDetachPrepare(job->data.backup.store, NULL))) return; -if (job->data.backup.bitmap) { -actions = virJSONValueNewArray(); - -if (qemuMonitorTransactionBitmapRemove(actions, - job->disk->src->nodeformat, - job->data.backup.bitmap) < 0) -return; -} - if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return; if (backend) qemuBlockStorageSourceAttachRollback(qemuDomainGetMonitor(vm), backend); -if (actions) -qemuMonitorTransaction(qemuDomainGetMonitor(vm), &actions); +if (job->data.backup.bitmap) +qemuMonitorBitmapRemove(qemuDomainGetMonitor(vm), +job->disk->src->nodeformat, +job->data.backup.bitmap); if (qemuDomainObjExitMonitor(driver, vm) < 0) return; -- 2.29.2
[PATCH 18/19] qemu: migration: Migrate block dirty bitmaps corresponding to checkpoints
Preserve block dirty bitmaps after migration with QEMU_MONITOR_MIGRATE_NON_SHARED_(DISK|INC). This patch implements functions which offer the bitmaps to the destination, check for eligibility on destination and then configure source for the migration. Signed-off-by: Peter Krempa --- src/qemu/qemu_migration.c | 333 +- 1 file changed, 331 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 36424f8493..16bfad0390 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2203,6 +2203,91 @@ qemuMigrationSrcCleanup(virDomainObjPtr vm, } +/** + * qemuMigrationSrcBeginPhaseBlockDirtyBitmaps: + * @mig: migration cookie struct + * @vm: domain object + * @migrate_disks: disks which are being migrated + * @nmigrage_disks: number of @migrate_disks + * + * Enumerates block dirty bitmaps on disks which will undergo storage migration + * and fills them into @mig to be offered to the destination. + */ +static int +qemuMigrationSrcBeginPhaseBlockDirtyBitmaps(qemuMigrationCookiePtr mig, +virDomainObjPtr vm, +const char **migrate_disks, +size_t nmigrate_disks) + +{ +GSList *disks = NULL; +qemuDomainObjPrivatePtr priv = vm->privateData; +size_t i; + +g_autoptr(GHashTable) blockNamedNodeData = NULL; + +if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, priv->job.asyncJob))) +return -1; + +for (i = 0; i < vm->def->ndisks; i++) { +qemuMigrationBlockDirtyBitmapsDiskPtr disk; +GSList *bitmaps = NULL; +virDomainDiskDefPtr diskdef = vm->def->disks[i]; +qemuBlockNamedNodeDataPtr nodedata = virHashLookup(blockNamedNodeData, diskdef->src->nodeformat); +size_t j; + +if (!nodedata) +continue; + +if (migrate_disks) { +bool migrating = false; + +for (j = 0; j < nmigrate_disks; j++) { +if (STREQ(migrate_disks[j], diskdef->dst)) { +migrating = true; +break; +} +} + +if (!migrating) +continue; +} + +for (j = 0; j < nodedata->nbitmaps; j++) { +qemuMigrationBlockDirtyBitmapsDiskBitmapPtr bitmap; + +if (!qemuBlockBitmapChainIsValid(diskdef->src, + nodedata->bitmaps[j]->name, + blockNamedNodeData)) +continue; + +bitmap = g_new0(qemuMigrationBlockDirtyBitmapsDiskBitmap, 1); +bitmap->bitmapname = g_strdup(nodedata->bitmaps[j]->name); +bitmap->alias = g_strdup_printf("libvirt-%s-%s", +diskdef->dst, +nodedata->bitmaps[j]->name); +bitmaps = g_slist_prepend(bitmaps, bitmap); +} + +if (!bitmaps) +continue; + +disk = g_new0(qemuMigrationBlockDirtyBitmapsDisk, 1); +disk->target = g_strdup(diskdef->dst); +disk->bitmaps = bitmaps; +disks = g_slist_prepend(disks, disk); +} + +if (!disks) +return 0; + +mig->blockDirtyBitmaps = disks; +mig->flags |= QEMU_MIGRATION_COOKIE_BLOCK_DIRTY_BITMAPS; + +return 0; +} + + /* The caller is supposed to lock the vm and start a migration job. */ static char * qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver, @@ -2315,6 +2400,12 @@ qemuMigrationSrcBeginPhase(virQEMUDriverPtr driver, if (!(mig = qemuMigrationCookieNew(vm->def, priv->origname))) return NULL; +if (cookieFlags & QEMU_MIGRATION_COOKIE_NBD && +virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_INCREMENTAL_BACKUP) && +qemuMigrationSrcBeginPhaseBlockDirtyBitmaps(mig, vm, migrate_disks, +nmigrate_disks) < 0) +return NULL; + if (qemuMigrationCookieFormat(mig, driver, vm, QEMU_MIGRATION_SOURCE, cookieout, cookieoutlen, @@ -2528,6 +2619,92 @@ qemuMigrationDstPrepare(virDomainObjPtr vm, migrateFrom, fd, NULL); } + +/** + * qemuMigrationDstPrepareAnyBlockDirtyBitmaps: + * @vm: domain object + * @mig: migration cookie + * @migParams: migration parameters + * @flags: migration flags + * + * Checks whether block dirty bitmaps offered by the migration source are + * to be migrated (e.g. they don't exist, the destination is compatible etc) + * and sets up destination qemu for migrating the bitmaps as well as updates the + * list of eligible bitmaps in the migration cookie to be sent back to the + * source. + */ +static int +qemuMigrationDstPrepareAnyBlockDirtyBitmaps(virDomainObjPtr vm, +qemuM
[PATCH 05/19] qemuMigrationSrcPerformPeer2Peer3: Don't leak 'dom_xml' on cleanup
Use g_autofree for 'dom_xml' to free it on some of the (unlikely) code paths jumping to cleanup prior to the deallocation which is done right after it's not needed any more since it's a big string. Noticed when running under valgrind: ==2204780== 8,192 bytes in 1 blocks are definitely lost in loss record 2,539 of 2,551 ==2204780==at 0x483BCE8: realloc (vg_replace_malloc.c:834) ==2204780==by 0x4D890DF: g_realloc (in /usr/lib64/libglib-2.0.so.0.6600.4) ==2204780==by 0x4DA3AF0: g_string_append_vprintf (in /usr/lib64/libglib-2.0.so.0.6600.4) ==2204780==by 0x4917293: virBufferAsprintf (virbuffer.c:307) ==2204780==by 0x49B0B75: virDomainChrDefFormat (domain_conf.c:26109) ==2204780==by 0x49E25EF: virDomainDefFormatInternalSetRootName (domain_conf.c:28956) ==2204780==by 0x15F81D24: qemuDomainDefFormatBufInternal (qemu_domain.c:6204) ==2204780==by 0x15F8270D: qemuDomainDefFormatXMLInternal (qemu_domain.c:6229) ==2204780==by 0x15F8270D: qemuDomainDefFormatLive (qemu_domain.c:6279) ==2204780==by 0x15FD8100: qemuMigrationSrcBeginPhase (qemu_migration.c:2395) ==2204780==by 0x15FE0F0D: qemuMigrationSrcPerformPeer2Peer3 (qemu_migration.c:4640) ==2204780==by 0x15FE0F0D: qemuMigrationSrcPerformPeer2Peer (qemu_migration.c:5093) ==2204780==by 0x15FE0F0D: qemuMigrationSrcPerformJob (qemu_migration.c:5168) ==2204780==by 0x15FE280E: qemuMigrationSrcPerform (qemu_migration.c:5372) ==2204780==by 0x15F9BA3D: qemuDomainMigratePerform3Params (qemu_driver.c:11841) Signed-off-by: Peter Krempa --- src/qemu/qemu_migration.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f44d31c971..37f0d43d24 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4347,7 +4347,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver, char *uri_out = NULL; char *cookiein = NULL; char *cookieout = NULL; -char *dom_xml = NULL; +g_autofree char *dom_xml = NULL; int cookieinlen = 0; int cookieoutlen = 0; int ret = -1; -- 2.29.2
Re: [PATCH 2/2] qemu: Validate TPM TIS device
On Thu, 2021-02-11 at 08:44 -0700, Jim Fehlig wrote: > On 2/11/21 3:37 AM, Andrea Bolognani wrote: > > On Wed, 2021-02-10 at 10:34 -0700, Jim Fehlig wrote: > > > +if (!ARCH_IS_X86(def->os.arch) && (def->os.arch != > > > VIR_ARCH_AARCH64)) { > > > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > > > + _("TPM model %s is only available for " > > > + "x86 and aarch64 guests"), > > > > Please don't split the error message into two separate lines > > I'm surprised that slipped through my copy and paste since I've always > preferred > errors on one line for easy searching. We have a lot of split error messages > throughout the code. This file is particularly bad. Is there any desire to > fix > existing cases, or just avoid new ones? I don't think we're particularly interested in going back and altering existing messages, and most importantly in the churn doing so would generate :) -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 2/2] qemu: Validate TPM TIS device
On 2/11/21 3:37 AM, Andrea Bolognani wrote: On Wed, 2021-02-10 at 10:34 -0700, Jim Fehlig wrote: I aslo worry about future architectures gaining support for emulated TPM TIS devices. It's okay to be conservative - we can always relax the check later. +if (!ARCH_IS_X86(def->os.arch) && (def->os.arch != VIR_ARCH_AARCH64)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("TPM model %s is only available for " + "x86 and aarch64 guests"), Please don't split the error message into two separate lines I'm surprised that slipped through my copy and paste since I've always preferred errors on one line for easy searching. We have a lot of split error messages throughout the code. This file is particularly bad. Is there any desire to fix existing cases, or just avoid new ones? and sprinkle some quotes around '%s' while you're at it. Will do. https://libvirt.org/coding-style.html#error-message-format The entire page is worth a re-read on occasion :-) Regards, Jim
[PATCH v2 3/4] utils: Deprecate hex-with-suffix sizes
Supporting '0x20M' looks odd, particularly since we have a 'B' suffix that is ambiguous for bytes, as well as a less-frequently-used 'E' suffix for extremely large exibytes. In practice, people using hex inputs are specifying values in bytes (and would have written 0x200, or possibly relied on default_suffix in the case of qemu_strtosz_MiB), and the use of scaling suffixes makes the most sense for inputs in decimal (where the user would write 32M). But rather than outright dropping support for hex-with-suffix, let's follow our deprecation policy. Sadly, since qemu_strtosz() does not have an Err** parameter, and plumbing that in would be a much larger task, we instead go with just directly emitting the deprecation warning to stderr. Signed-off-by: Eric Blake --- docs/system/deprecated.rst | 8 util/cutils.c | 10 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 2fcac7861e07..113c2e933f1b 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -146,6 +146,14 @@ library enabled as a cryptography provider. Neither the ``nettle`` library, or the built-in cryptography provider are supported on FIPS enabled hosts. +hexadecimal sizes with scaling multipliers (since 6.0) +'' + +Input parameters that take a size value should only use a size suffix +(such as 'k' or 'M') when the base is written in decimal, and not when +the value is hexadecimal. That is, '0x20M' is deprecated, and should +be written either as '32M' or as '0x200'. + QEMU Machine Protocol (QMP) commands diff --git a/util/cutils.c b/util/cutils.c index 22d27b0fd21b..6a8a175e0d71 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -250,6 +250,9 @@ static int64_t suffix_mul(char suffix, int64_t unit) * fractional portion is truncated to byte * - 0x7fEE - hexadecimal, unit determined by @default_suffix * + * The following cause a deprecation warning, and may be removed in the future + * - 0xabc{kKmMgGtTpP} - hex with scaling suffix + * * The following are intentionally not supported * - octal, such as 08 * - fractional hex, such as 0x1.8 @@ -272,7 +275,7 @@ static int do_strtosz(const char *nptr, const char **end, int retval; const char *endptr, *f; unsigned char c; -bool mul_required = false; +bool mul_required = false, hex = false; uint64_t val; int64_t mul; double fraction = 0.0; @@ -298,6 +301,7 @@ static int do_strtosz(const char *nptr, const char **end, retval = -EINVAL; goto out; } +hex = true; } else if (*endptr == '.') { /* * Input looks like a fraction. Make sure even 1.k works @@ -320,6 +324,10 @@ static int do_strtosz(const char *nptr, const char **end, c = *endptr; mul = suffix_mul(c, unit); if (mul > 0) { +if (hex) { +warn_report("Using a multiplier suffix on hex numbers " +"is deprecated: %s", nptr); +} endptr++; } else { mul = suffix_mul(default_suffix, unit); -- 2.30.1
[PATCH v2 4/4] utils: Deprecate inexact fractional suffix sizes
The value '1.1k' is inexact; 1126.4 bytes is not possible, so we happen to truncate it to 1126. Our use of fractional sizes is intended for convenience, but when a user specifies a fraction that is not a clean translation to binary, truncating/rounding behind their backs can cause confusion. Better is to deprecate inexact values, which still leaves '1.5k' as valid, but alerts the user to spell out their values as a precise byte number in cases where they are currently being rounded. Note that values like '0.1G' in the testsuite need adjustment as a result. Since qemu_strtosz() does not have an Err** parameter, and plumbing that in would be a much larger task, we instead go with just directly emitting the deprecation warning to stderr. Signed-off-by: Eric Blake --- I'm not a fan of this patch, but am proposing it for discussion purposes. --- docs/system/deprecated.rst | 9 + tests/test-cutils.c| 6 +++--- tests/test-keyval.c| 4 ++-- tests/test-qemu-opts.c | 4 ++-- util/cutils.c | 9 +++-- 5 files changed, 23 insertions(+), 9 deletions(-) diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 113c2e933f1b..2c9cb849eec5 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -154,6 +154,15 @@ Input parameters that take a size value should only use a size suffix the value is hexadecimal. That is, '0x20M' is deprecated, and should be written either as '32M' or as '0x200'. +inexact sizes via scaled fractions (since 6.0) +'' + +Input parameters that take a size value should only use a fractional +size (such as '1.5M') that will result in an exact byte value. The +use of inexact values (such as '1.1M') that require truncation or +rounding is deprecated, and you should instead consider writing your +unusual size in bytes (here, '1153433' or '1153434' as desired). + QEMU Machine Protocol (QMP) commands diff --git a/tests/test-cutils.c b/tests/test-cutils.c index bad3a6099389..c6c33866277b 100644 --- a/tests/test-cutils.c +++ b/tests/test-cutils.c @@ -2124,11 +2124,11 @@ static void test_qemu_strtosz_float(void) g_assert_cmpint(res, ==, 1024); g_assert(endptr == str + 3); -/* For convenience, we permit values that are not byte-exact */ -str = "12.345M"; +/* Fractional values should still be byte-exact */ +str = "12.125M"; err = qemu_strtosz(str, &endptr, &res); g_assert_cmpint(err, ==, 0); -g_assert_cmpint(res, ==, (uint64_t) (12.345 * MiB)); +g_assert_cmpint(res, ==, (uint64_t) (12.125 * MiB)); g_assert(endptr == str + 7); } diff --git a/tests/test-keyval.c b/tests/test-keyval.c index e20c07cf3ea5..7a45c22942e6 100644 --- a/tests/test-keyval.c +++ b/tests/test-keyval.c @@ -525,7 +525,7 @@ static void test_keyval_visit_size(void) visit_free(v); /* Suffixes */ -qdict = keyval_parse("sz1=8b,sz2=1.5k,sz3=2M,sz4=0.1G,sz5=16777215T", +qdict = keyval_parse("sz1=8b,sz2=1.5k,sz3=2M,sz4=0.125G,sz5=16777215T", NULL, NULL, &error_abort); v = qobject_input_visitor_new_keyval(QOBJECT(qdict)); qobject_unref(qdict); @@ -537,7 +537,7 @@ static void test_keyval_visit_size(void) visit_type_size(v, "sz3", &sz, &error_abort); g_assert_cmphex(sz, ==, 2 * MiB); visit_type_size(v, "sz4", &sz, &error_abort); -g_assert_cmphex(sz, ==, GiB / 10); +g_assert_cmphex(sz, ==, GiB / 8); visit_type_size(v, "sz5", &sz, &error_abort); g_assert_cmphex(sz, ==, 16777215ULL * TiB); visit_check_struct(v, &error_abort); diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index 6568e31a7229..549e994938fe 100644 --- a/tests/test-qemu-opts.c +++ b/tests/test-qemu-opts.c @@ -720,10 +720,10 @@ static void test_opts_parse_size(void) g_assert_cmphex(qemu_opt_get_size(opts, "size1", 0), ==, 8); g_assert_cmphex(qemu_opt_get_size(opts, "size2", 0), ==, 1536); g_assert_cmphex(qemu_opt_get_size(opts, "size3", 0), ==, 2 * MiB); -opts = qemu_opts_parse(&opts_list_02, "size1=0.1G,size2=16777215T", +opts = qemu_opts_parse(&opts_list_02, "size1=0.125G,size2=16777215T", false, &error_abort); g_assert_cmpuint(opts_count(opts), ==, 2); -g_assert_cmphex(qemu_opt_get_size(opts, "size1", 0), ==, GiB / 10); +g_assert_cmphex(qemu_opt_get_size(opts, "size1", 0), ==, GiB / 8); g_assert_cmphex(qemu_opt_get_size(opts, "size2", 0), ==, 16777215ULL * TiB); /* Beyond limit with suffix */ diff --git a/util/cutils.c b/util/cutils.c index 6a8a175e0d71..1154b9de131a 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -246,12 +246,13 @@ static int64_t suffix_mul(char suffix, int64_t unit) * The size parsing supports the following syntaxes * - 12345 - decimal, scale determined by @default_suffix and @unit * - 12345{bBkKmMgGtTpPeE} - decimal, scale determined by suffix and @unit
Re: [PATCH v2 3/4] utils: Deprecate hex-with-suffix sizes
On 2/11/21 9:44 PM, Eric Blake wrote: > Supporting '0x20M' looks odd, particularly since we have a 'B' suffix > that is ambiguous for bytes, as well as a less-frequently-used 'E' > suffix for extremely large exibytes. In practice, people using hex > inputs are specifying values in bytes (and would have written > 0x200, or possibly relied on default_suffix in the case of > qemu_strtosz_MiB), and the use of scaling suffixes makes the most > sense for inputs in decimal (where the user would write 32M). But > rather than outright dropping support for hex-with-suffix, let's > follow our deprecation policy. Sadly, since qemu_strtosz() does not > have an Err** parameter, and plumbing that in would be a much larger > task, we instead go with just directly emitting the deprecation > warning to stderr. > > Signed-off-by: Eric Blake > --- > docs/system/deprecated.rst | 8 > util/cutils.c | 10 +- > 2 files changed, 17 insertions(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
[libvirt PATCH 0/8] More VIR_FREE removals
Only 90 this time. These are all functions that behave similar to the *Free() functions, but their names don't end in "Free" so I missed them last time. Laine Stump (8): esx: replace VIR_FREE with g_free in any ESX_VI__TEMPLATE__FREE conf: convert VIR_FREE to g_free in other functions that free their arg locking: convert VIR_FREE to g_free in other functions that free their arg openvz: convert VIR_FREE to g_free in other functions that free their arg remote: convert VIR_FREE to g_free in other functions that free their arg qemu: convert VIR_FREE to g_free in other functions that free their arg util: convert VIR_FREE to g_free in other functions that free their arg vmware: convert VIR_FREE to g_free in other functions that free their arg src/conf/capabilities.c | 38 +++ src/esx/esx_vi.c | 18 +++ src/esx/esx_vi_types.c| 24 +-- src/locking/lock_manager.c| 4 ++-- src/openvz/openvz_conf.c | 2 +- src/qemu/qemu_domain.c| 2 +- src/qemu/qemu_driver.c| 8 +++ src/remote/remote_daemon_stream.c | 2 +- src/util/virconf.c| 4 ++-- src/util/virerror.c | 4 ++-- src/util/virobject.c | 2 +- src/util/virstring.c | 4 ++-- src/vmware/vmware_conf.c | 4 ++-- src/vmware/vmware_driver.c| 4 ++-- 14 files changed, 60 insertions(+), 60 deletions(-) -- 2.29.2
[libvirt PATCH 3/8] locking: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump --- src/locking/lock_manager.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c index db724eb30f..90ec5cdfbe 100644 --- a/src/locking/lock_manager.c +++ b/src/locking/lock_manager.c @@ -245,8 +245,8 @@ void virLockManagerPluginUnref(virLockManagerPluginPtr plugin) return; } -VIR_FREE(plugin->name); -VIR_FREE(plugin); +g_free(plugin->name); +g_free(plugin); } #else /* !WITH_DLFCN_H */ void virLockManagerPluginUnref(virLockManagerPluginPtr plugin G_GNUC_UNUSED) -- 2.29.2
[libvirt PATCH 4/8] openvz: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump --- src/openvz/openvz_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 1783dce233..041a031a3a 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -478,7 +478,7 @@ openvzFreeDriver(struct openvz_driver *driver) virObjectUnref(driver->xmlopt); virObjectUnref(driver->domains); virObjectUnref(driver->caps); -VIR_FREE(driver); +g_free(driver); } -- 2.29.2
[libvirt PATCH 1/8] esx: replace VIR_FREE with g_free in any ESX_VI__TEMPLATE__FREE
Invocations of the macro ESX_VI__TEMPLATE__FREE() will free the main object (referenced as "item") that's pointing to all the things being VIR_FREEd in the body, so it is safe for all the pointers in item to just be g_freed rather that VIR_FREEd. Signed-off-by: Laine Stump --- src/esx/esx_vi.c | 18 +- src/esx/esx_vi_types.c | 24 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 7e05318fdc..2eb8048858 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -805,19 +805,19 @@ ESX_VI__TEMPLATE__FREE(Context, virMutexDestroy(item->sessionLock); esxVI_CURL_Free(&item->curl); -VIR_FREE(item->url); -VIR_FREE(item->ipAddress); -VIR_FREE(item->username); -VIR_FREE(item->password); +g_free(item->url); +g_free(item->ipAddress); +g_free(item->username); +g_free(item->password); esxVI_ServiceContent_Free(&item->service); esxVI_UserSession_Free(&item->session); -VIR_FREE(item->sessionLock); +g_free(item->sessionLock); esxVI_Datacenter_Free(&item->datacenter); -VIR_FREE(item->datacenterPath); +g_free(item->datacenterPath); esxVI_ComputeResource_Free(&item->computeResource); -VIR_FREE(item->computeResourcePath); +g_free(item->computeResourcePath); esxVI_HostSystem_Free(&item->hostSystem); -VIR_FREE(item->hostSystemName); +g_free(item->hostSystemName); esxVI_SelectionSpec_Free(&item->selectSet_folderToChildEntity); esxVI_SelectionSpec_Free(&item->selectSet_hostSystemToParent); esxVI_SelectionSpec_Free(&item->selectSet_hostSystemToVm); @@ -1419,7 +1419,7 @@ ESX_VI__TEMPLATE__ALLOC(Response) /* esxVI_Response_Free */ ESX_VI__TEMPLATE__FREE(Response, { -VIR_FREE(item->content); +g_free(item->content); xmlFreeDoc(item->document); }) diff --git a/src/esx/esx_vi_types.c b/src/esx/esx_vi_types.c index 6821587e44..4d3617e0a8 100644 --- a/src/esx/esx_vi_types.c +++ b/src/esx/esx_vi_types.c @@ -900,8 +900,8 @@ ESX_VI__TEMPLATE__ALLOC(AnyType) ESX_VI__TEMPLATE__FREE(AnyType, { xmlFreeNode(item->node); -VIR_FREE(item->other); -VIR_FREE(item->value); +g_free(item->other); +g_free(item->value); }) const char * @@ -1117,7 +1117,7 @@ ESX_VI__TEMPLATE__FREE(String, { esxVI_String_Free(&item->_next); -VIR_FREE(item->value); +g_free(item->value); }) /* esxVI_String_Validate */ @@ -1421,7 +1421,7 @@ ESX_VI__TEMPLATE__ALLOC(DateTime) /* esxVI_DateTime_Free */ ESX_VI__TEMPLATE__FREE(DateTime, { -VIR_FREE(item->value); +g_free(item->value); }) /* esxVI_DateTime_Validate */ @@ -1564,8 +1564,8 @@ ESX_VI__TEMPLATE__ALLOC(Fault); /* esxVI_Fault_Free */ ESX_VI__TEMPLATE__FREE(Fault, { -VIR_FREE(item->faultcode); -VIR_FREE(item->faultstring); +g_free(item->faultcode); +g_free(item->faultstring); }) /* esxVI_Fault_Validate */ @@ -1595,7 +1595,7 @@ ESX_VI__TEMPLATE__ALLOC(MethodFault); /* esxVI_MethodFault_Free */ ESX_VI__TEMPLATE__FREE(MethodFault, { -VIR_FREE(item->_actualType); +g_free(item->_actualType); }) int @@ -1638,8 +1638,8 @@ ESX_VI__TEMPLATE__FREE(ManagedObjectReference, { esxVI_ManagedObjectReference_Free(&item->_next); -VIR_FREE(item->type); -VIR_FREE(item->value); +g_free(item->type); +g_free(item->value); }) /* esxVI_ManagedObjectReference_DeepCopy */ @@ -1732,17 +1732,17 @@ ESX_VI__TEMPLATE__ALLOC(Event) ESX_VI__TEMPLATE__FREE(Event, { esxVI_Event_Free(&item->_next); -VIR_FREE(item->_actualType); +g_free(item->_actualType); esxVI_Int_Free(&item->key); esxVI_Int_Free(&item->chainId); esxVI_DateTime_Free(&item->createdTime); -VIR_FREE(item->userName); +g_free(item->userName); /* FIXME: datacenter is currently ignored */ /* FIXME: computeResource is currently ignored */ /* FIXME: host is currently ignored */ esxVI_VmEventArgument_Free(&item->vm); -VIR_FREE(item->fullFormattedMessage); +g_free(item->fullFormattedMessage); }) /* esxVI_Event_Validate */ -- 2.29.2
[libvirt PATCH 7/8] util: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump --- src/util/virconf.c | 4 ++-- src/util/virerror.c | 4 ++-- src/util/virobject.c | 2 +- src/util/virstring.c | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/util/virconf.c b/src/util/virconf.c index d85fc32b64..16107bce96 100644 --- a/src/util/virconf.c +++ b/src/util/virconf.c @@ -151,11 +151,11 @@ virConfFreeValue(virConfValuePtr val) return; if (val->type == VIR_CONF_STRING && val->str != NULL) -VIR_FREE(val->str); +g_free(val->str); if (val->type == VIR_CONF_LIST && val->list != NULL) virConfFreeList(val->list); -VIR_FREE(val); +g_free(val); } virConfPtr diff --git a/src/util/virerror.c b/src/util/virerror.c index 14054add41..708081414a 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -158,7 +158,7 @@ virLastErrFreeData(void *data) if (!err) return; virResetError(err); -VIR_FREE(err); +g_free(err); } @@ -477,7 +477,7 @@ void virFreeError(virErrorPtr err) { virResetError(err); -VIR_FREE(err); +g_free(err); } /** diff --git a/src/util/virobject.c b/src/util/virobject.c index ce40ffae22..370a64bd6d 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -655,5 +655,5 @@ virObjectListFreeCount(void *list, for (i = 0; i < count; i++) virObjectUnref(((void **)list)[i]); -VIR_FREE(list); +g_free(list); } diff --git a/src/util/virstring.c b/src/util/virstring.c index c98435388a..c3e64007fe 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -108,9 +108,9 @@ virStringListFreeCount(char **strings, return; for (i = 0; i < count; i++) -VIR_FREE(strings[i]); +g_free(strings[i]); -VIR_FREE(strings); +g_free(strings); } -- 2.29.2
[libvirt PATCH 5/8] remote: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump --- src/remote/remote_daemon_stream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_stream.c index b4a3cb6741..57816eb10e 100644 --- a/src/remote/remote_daemon_stream.c +++ b/src/remote/remote_daemon_stream.c @@ -419,7 +419,7 @@ int daemonFreeClientStream(virNetServerClientPtr client, } virObjectUnref(stream->st); -VIR_FREE(stream); +g_free(stream); return ret; } -- 2.29.2
[libvirt PATCH 2/8] conf: convert VIR_FREE to g_free in other functions that free their arg
Previous patches have converted VIR_FREE to g_free in functions with names ending in Free() and Dispose(), but there are a few similar functions with names that don't fit that pattern, but server the same purpose (and thus can survive the same conversion). in particular *Free*(), and *Unref(). Signed-off-by: Laine Stump --- src/conf/capabilities.c | 38 +++--- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index dd3321db9a..69d9bb0e38 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -118,10 +118,10 @@ virCapabilitiesFreeHostNUMACell(virCapsHostNUMACellPtr cell) virCapabilitiesClearHostNUMACellCPUTopology(cell->cpus, cell->ncpus); -VIR_FREE(cell->cpus); -VIR_FREE(cell->siblings); -VIR_FREE(cell->pageinfo); -VIR_FREE(cell); +g_free(cell->cpus); +g_free(cell->siblings); +g_free(cell->pageinfo); +g_free(cell); } static void @@ -129,9 +129,9 @@ virCapabilitiesFreeGuestMachine(virCapsGuestMachinePtr machine) { if (machine == NULL) return; -VIR_FREE(machine->name); -VIR_FREE(machine->canonical); -VIR_FREE(machine); +g_free(machine->name); +g_free(machine->canonical); +g_free(machine); } static void @@ -141,13 +141,13 @@ virCapabilitiesFreeGuestDomain(virCapsGuestDomainPtr dom) if (dom == NULL) return; -VIR_FREE(dom->info.emulator); -VIR_FREE(dom->info.loader); +g_free(dom->info.emulator); +g_free(dom->info.loader); for (i = 0; i < dom->info.nmachines; i++) virCapabilitiesFreeGuestMachine(dom->info.machines[i]); -VIR_FREE(dom->info.machines); +g_free(dom->info.machines); -VIR_FREE(dom); +g_free(dom); } void @@ -157,17 +157,17 @@ virCapabilitiesFreeGuest(virCapsGuestPtr guest) if (guest == NULL) return; -VIR_FREE(guest->arch.defaultInfo.emulator); -VIR_FREE(guest->arch.defaultInfo.loader); +g_free(guest->arch.defaultInfo.emulator); +g_free(guest->arch.defaultInfo.loader); for (i = 0; i < guest->arch.defaultInfo.nmachines; i++) virCapabilitiesFreeGuestMachine(guest->arch.defaultInfo.machines[i]); -VIR_FREE(guest->arch.defaultInfo.machines); +g_free(guest->arch.defaultInfo.machines); for (i = 0; i < guest->arch.ndomains; i++) virCapabilitiesFreeGuestDomain(guest->arch.domains[i]); -VIR_FREE(guest->arch.domains); +g_free(guest->arch.domains); -VIR_FREE(guest); +g_free(guest); } @@ -177,7 +177,7 @@ virCapabilitiesFreeStoragePool(virCapsStoragePoolPtr pool) if (!pool) return; -VIR_FREE(pool); +g_free(pool); } @@ -190,7 +190,7 @@ virCapabilitiesHostNUMAUnref(virCapsHostNUMAPtr caps) if (g_atomic_int_dec_and_test(&caps->refs)) { g_ptr_array_unref(caps->cells); -VIR_FREE(caps); +g_free(caps); } } @@ -409,7 +409,7 @@ virCapabilitiesFreeMachines(virCapsGuestMachinePtr *machines, virCapabilitiesFreeGuestMachine(machines[i]); machines[i] = NULL; } -VIR_FREE(machines); +g_free(machines); } /** -- 2.29.2
[libvirt PATCH 6/8] qemu: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 8 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 14d05cfe3b..13efdcbad1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -95,7 +95,7 @@ qemuJobFreePrivate(void *opaque) return; qemuMigrationParamsFree(priv->migParams); -VIR_FREE(priv); +g_free(priv); } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 53ac3b0c17..58236f1ae2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17677,12 +17677,12 @@ qemuDomainFreeResctrlMonData(virQEMUResctrlMonDataPtr resdata) { size_t i = 0; -VIR_FREE(resdata->name); -VIR_FREE(resdata->vcpus); +g_free(resdata->name); +g_free(resdata->vcpus); for (i = 0; i < resdata->nstats; i++) virResctrlMonitorStatsFree(resdata->stats[i]); -VIR_FREE(resdata->stats); -VIR_FREE(resdata); +g_free(resdata->stats); +g_free(resdata); } -- 2.29.2
[libvirt PATCH 8/8] vmware: convert VIR_FREE to g_free in other functions that free their arg
Signed-off-by: Laine Stump --- src/vmware/vmware_conf.c | 4 ++-- src/vmware/vmware_driver.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index 55cd1d6f2d..f905962b85 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -56,8 +56,8 @@ vmwareFreeDriver(struct vmware_driver *driver) virObjectUnref(driver->domains); virObjectUnref(driver->caps); virObjectUnref(driver->xmlopt); -VIR_FREE(driver->vmrun); -VIR_FREE(driver); +g_free(driver->vmrun); +g_free(driver); } diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 481c59ef3c..5f1edeb7f5 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -110,8 +110,8 @@ vmwareDataFreeFunc(void *data) { vmwareDomainPtr dom = data; -VIR_FREE(dom->vmxPath); -VIR_FREE(dom); +g_free(dom->vmxPath); +g_free(dom); } static int -- 2.29.2
[libvirt RFC PATCH 1/5] conf: rename and narrow scope of virDomainHostdevDefClear()
This function had the name "Clear", but some of its fields were being cleared, and others were just being freed, but the pointers not cleared. In fact, all the uses of virDomainHostdevDefClear() ended up freeing the memory containing the HosdevDef immediately after clearing it. So it is acceptable to change the VIR_FREEs in this function to g_frees, but that would be confusing to future me, who will have forgotten that a "cleared" hostdevdef is never re-used. In order to eliminate the confusion, we can rename the function to virDomainHostdevDefFreeContents(), which is more accurate (doesn't make false claims). In the meantime, this function was only ever called from other functions within the same file, so there is no need for it to be declared in domain_conf.h, much less be listed among the exported functions in libvirt_private.syms, so lets just make this renamed function static for efficiency's sake. Signed-off-by: Laine Stump --- src/conf/domain_conf.c | 19 +++ src/conf/domain_conf.h | 1 - src/libvirt_private.syms | 1 - 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 17e0d1ed31..3b9d0da232 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1339,6 +1339,7 @@ static virClassPtr virDomainXMLOptionClass; static void virDomainObjDispose(void *obj); static void virDomainXMLOptionDispose(void *obj); +static void virDomainHostdevDefFreeContents(virDomainHostdevDefPtr def); static void virDomainChrSourceDefFormat(virBufferPtr buf, @@ -2430,7 +2431,7 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def) g_free(def->data.direct.linkdev); break; case VIR_DOMAIN_NET_TYPE_HOSTDEV: -virDomainHostdevDefClear(&def->data.hostdev.def); +virDomainHostdevDefFree(&def->data.hostdev.def); break; default: break; @@ -2531,7 +2532,7 @@ virDomainNetDefFree(virDomainNetDefPtr def) break; case VIR_DOMAIN_NET_TYPE_HOSTDEV: -virDomainHostdevDefClear(&def->data.hostdev.def); +virDomainHostdevDefFree(&def->data.hostdev.def); break; case VIR_DOMAIN_NET_TYPE_ETHERNET: @@ -3009,7 +3010,8 @@ virDomainHostdevSubsysSCSIClear(virDomainHostdevSubsysSCSIPtr scsisrc) } -void virDomainHostdevDefClear(virDomainHostdevDefPtr def) +static void +virDomainHostdevDefFreeContents(virDomainHostdevDefPtr def) { if (!def) return; @@ -3030,13 +3032,13 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) case VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES: switch ((virDomainHostdevCapsType) def->source.caps.type) { case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_STORAGE: -VIR_FREE(def->source.caps.u.storage.block); +g_free(def->source.caps.u.storage.block); break; case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_MISC: -VIR_FREE(def->source.caps.u.misc.chardev); +g_free(def->source.caps.u.misc.chardev); break; case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_NET: -VIR_FREE(def->source.caps.u.net.ifname); +g_free(def->source.caps.u.net.ifname); virNetDevIPInfoClear(&def->source.caps.u.net.ip); break; case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST: @@ -3049,7 +3051,7 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) virDomainHostdevSubsysSCSIClear(&def->source.subsys.u.scsi); break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST: -VIR_FREE(def->source.subsys.u.scsi_host.wwpn); +g_free(def->source.subsys.u.scsi_host.wwpn); break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: @@ -3061,6 +3063,7 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) } } + void virDomainTPMDefFree(virDomainTPMDefPtr def) { if (!def) @@ -3089,7 +3092,7 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def) return; /* free all subordinate objects */ -virDomainHostdevDefClear(def); +virDomainHostdevDefFreeContents(def); /* If there is a parentnet device object, it will handle freeing * the memory. diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e263e6098b..415826134d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3126,7 +3126,6 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainVideoDef, virDomainVideoDefFree); void virDomainVideoDefClear(virDomainVideoDefPtr def); virDomainHostdevDefPtr virDomainHostdevDefNew(void); -void virDomainHostdevDefClear(virDomainHostdevDefPtr def); void virDomainHostdevDefFree(virDomainHostdevDefPtr def); void virDomainHubDefFree(virDomainHubDefPtr def); void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 49e665a5f0.
[libvirt RFC PATCH 0/5] eliminating VIR_FREE in the *Clear() functions
I've looked at a few of these, and one thing I've found is that very often we have a function called somethingSomethingClear(), and: 1) The only places it is ever called will immediately free the memory of the object as soon as they clear it. and very possibly 2) It doesn't actually *clear* everything. Some items are cleared via VIR_FREE(), but then some of the other pointers call bobLoblawFree(def->bobloblaw) and then don't actually set def->bobloblaw to NULL - so the functions aren't actually "Clearing", they're "Freeing and then clearing a few things, but not everything". So I'm wondering if it is worthwhile to A) audit all the *Clear() functions and rename the functions that don't actually need to clear the contents to be, e.g. bobLobLawFreeContents(), while also replacing VIR_FREE with g_free(). (this is what I've done in these 5 patches) or if we should B) just do the wholesale approach and (as danpb suggested last week) change all VIR_FREE in *Clear() functions to g_free(), and put a "memset(obj, 0, sizeof(*obj))" at the end of each function, ignoring whether or not we actually need that. (B) would obviously be much faster to get done, and simpler for everyone to keep track of what's happening, but of course it is less efficient. Very likely this efficiency is completely meaningless in the large scheme (even in the medium or small scheme). (I actually like the idea of 0'ing out *everything*[*] when we're done with it, extra cycles be damned! I think of the two choices above, after going through this exercise, I'd say (B) is a more reasonable choice, but since I tried this, I thought I'd send it and see if anyone else has an opinion (or different idea) [*](including all those places I've replaced VIR_FREE with g_free in the name of "progress?") Laine Stump (5): conf: rename and narrow scope of virDomainHostdevDefClear() conf: rename virDomainHostdevSubsysSCSIClear conf: replace pointless VIR_FREEs with g_free in virDomainVideoDefClear() conf: don't call virDomainDeviceInfoClear from virDomainDeviceInfoParseXML conf: replace virDomainDeviceInfoClear with virDomainDeviceInfoFreeContents src/conf/device_conf.c | 15 +++- src/conf/device_conf.h | 2 +- src/conf/domain_conf.c | 81 src/conf/domain_conf.h | 1 - src/libvirt_private.syms | 1 - 5 files changed, 47 insertions(+), 53 deletions(-) -- 2.29.2
[libvirt RFC PATCH 2/5] conf: rename virDomainHostdevSubsysSCSIClear
This function is only called from one place, and the object being "cleared" is never again referenced before being freed, so it doesn't need to be "cleared", just change its name to *FreeContents() so that we can remove its VIR_FREEs with a "clear" conscience (pun intended). Signed-off-by: Laine Stump --- src/conf/domain_conf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3b9d0da232..3505d29dbe 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2997,13 +2997,13 @@ virDomainHostdevDefNew(void) static void -virDomainHostdevSubsysSCSIClear(virDomainHostdevSubsysSCSIPtr scsisrc) +virDomainHostdevSubsysSCSIFreeContents(virDomainHostdevSubsysSCSIPtr scsisrc) { if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { virObjectUnref(scsisrc->u.iscsi.src); scsisrc->u.iscsi.src = NULL; } else { -VIR_FREE(scsisrc->u.host.adapter); +g_free(scsisrc->u.host.adapter); virObjectUnref(scsisrc->u.host.src); scsisrc->u.host.src = NULL; } @@ -3048,7 +3048,7 @@ virDomainHostdevDefFreeContents(virDomainHostdevDefPtr def) case VIR_DOMAIN_HOSTDEV_MODE_SUBSYS: switch ((virDomainHostdevSubsysType) def->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: -virDomainHostdevSubsysSCSIClear(&def->source.subsys.u.scsi); +virDomainHostdevSubsysSCSIFreeContents(&def->source.subsys.u.scsi); break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST: g_free(def->source.subsys.u.scsi_host.wwpn); -- 2.29.2
[libvirt RFC PATCH 3/5] conf: replace pointless VIR_FREEs with g_free in virDomainVideoDefClear()
This function does a memset(0) at the end anyway, so there's no point in VIR_FREE vs g_free. Signed-off-by: Laine Stump --- src/conf/domain_conf.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3505d29dbe..d8b8ef38f5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2960,13 +2960,13 @@ virDomainVideoDefClear(virDomainVideoDefPtr def) virDomainDeviceInfoClear(&def->info); if (def->accel) -VIR_FREE(def->accel->rendernode); -VIR_FREE(def->accel); -VIR_FREE(def->res); -VIR_FREE(def->virtio); +g_free(def->accel->rendernode); +g_free(def->accel); +g_free(def->res); +g_free(def->virtio); if (def->driver) -VIR_FREE(def->driver->vhost_user_binary); -VIR_FREE(def->driver); +g_free(def->driver->vhost_user_binary); +g_free(def->driver); virObjectUnref(def->privateData); memset(def, 0, sizeof(*def)); -- 2.29.2
[libvirt RFC PATCH 4/5] conf: don't call virDomainDeviceInfoClear from virDomainDeviceInfoParseXML
The DeviceInfo object sent to virDomainDeviceInfoParseXML() was allocated with g_new0() just prior to the call (although in each case there is some other code in between, none of it touches the DeviceInfo); pretty clearly, the call to virDomainDeviceInfoClear() at the top of virDomainDeviceInfoParseXML() was just overly paranoid "safety code", and is unnecessary. Likewise, the call to clear the device info on return when there is a parse failure is also unnecessary, since in every case the caller immediately frees the object after the failure. So let's remove these. Signed-off-by: Laine Stump --- src/conf/domain_conf.c | 4 1 file changed, 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d8b8ef38f5..f2b00129e1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6533,8 +6533,6 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt, g_autofree char *rombar = NULL; g_autofree char *aliasStr = NULL; -virDomainDeviceInfoClear(info); - cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { @@ -6611,8 +6609,6 @@ virDomainDeviceInfoParseXML(virDomainXMLOptionPtr xmlopt, ret = 0; cleanup: -if (ret < 0) -virDomainDeviceInfoClear(info); return ret; } -- 2.29.2
[libvirt RFC PATCH 5/5] conf: replace virDomainDeviceInfoClear with virDomainDeviceInfoFreeContents
In every case where virDomainDeviceInfoClear() is still being called, the object containing the DeviceInfo is freed immediately thereafter (yes, I checked them all!). This means we don't need to clear any of the data in the DeviceInfo, just free memory that it points to. Signed-off-by: Laine Stump --- src/conf/device_conf.c | 15 ++- src/conf/device_conf.h | 2 +- src/conf/domain_conf.c | 40 3 files changed, 27 insertions(+), 30 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index 714ac50762..743d7f7468 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -83,22 +83,19 @@ virZPCIDeviceAddressParseXML(xmlNodePtr node, } void -virDomainDeviceInfoClear(virDomainDeviceInfoPtr info) +virDomainDeviceInfoFreeContents(virDomainDeviceInfoPtr info) { -VIR_FREE(info->alias); -memset(&info->addr, 0, sizeof(info->addr)); -info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE; -VIR_FREE(info->romfile); -VIR_FREE(info->loadparm); -info->isolationGroup = 0; -info->isolationGroupLocked = false; +g_free(info->alias); +g_free(info->romfile); +g_free(info->loadparm); } + void virDomainDeviceInfoFree(virDomainDeviceInfoPtr info) { if (info) { -virDomainDeviceInfoClear(info); +virDomainDeviceInfoFreeContents(info); g_free(info); } } diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h index a51bdf10ee..444d1b16c6 100644 --- a/src/conf/device_conf.h +++ b/src/conf/device_conf.h @@ -181,7 +181,7 @@ struct _virDomainDeviceInfo { bool isolationGroupLocked; }; -void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info); +void virDomainDeviceInfoFreeContents(virDomainDeviceInfoPtr info); void virDomainDeviceInfoFree(virDomainDeviceInfoPtr info); bool virDomainDeviceInfoAddressIsEqual(const virDomainDeviceInfo *a, diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f2b00129e1..b8db94974b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1880,7 +1880,7 @@ void virDomainInputDefFree(virDomainInputDefPtr def) if (!def) return; -virDomainDeviceInfoClear(&def->info); +virDomainDeviceInfoFreeContents(&def->info); g_free(def->source.evdev); g_free(def->virtio); g_free(def); @@ -2210,7 +2210,7 @@ virDomainDiskDefFree(virDomainDiskDefPtr def) g_free(def->domain_name); g_free(def->blkdeviotune.group_name); g_free(def->virtio); -virDomainDeviceInfoClear(&def->info); +virDomainDeviceInfoFreeContents(&def->info); virObjectUnref(def->privateData); g_free(def); @@ -2342,7 +2342,7 @@ void virDomainControllerDefFree(virDomainControllerDefPtr def) if (!def) return; -virDomainDeviceInfoClear(&def->info); +virDomainDeviceInfoFreeContents(&def->info); g_free(def->virtio); g_free(def); @@ -2408,7 +2408,7 @@ void virDomainFSDefFree(virDomainFSDefPtr def) virObjectUnref(def->src); g_free(def->dst); -virDomainDeviceInfoClear(&def->info); +virDomainDeviceInfoFreeContents(&def->info); g_free(def->virtio); virObjectUnref(def->privateData); g_free(def->binary); @@ -2471,7 +2471,7 @@ virDomainVsockDefFree(virDomainVsockDefPtr vsock) return; virObjectUnref(vsock->privateData); -virDomainDeviceInfoClear(&vsock->info); +virDomainDeviceInfoFreeContents(&vsock->info); g_free(vsock->virtio); g_free(vsock); } @@ -2556,7 +2556,7 @@ virDomainNetDefFree(virDomainNetDefPtr def) virNetDevIPInfoClear(&def->guestIP); virNetDevIPInfoClear(&def->hostIP); -virDomainDeviceInfoClear(&def->info); +virDomainDeviceInfoFreeContents(&def->info); g_free(def->filter); virHashFree(def->filterparams); @@ -2806,7 +2806,7 @@ void virDomainChrDefFree(virDomainChrDefPtr def) } virObjectUnref(def->source); -virDomainDeviceInfoClear(&def->info); +virDomainDeviceInfoFreeContents(&def->info); g_free(def); } @@ -2835,7 +2835,7 @@ void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def) break; } -virDomainDeviceInfoClear(&def->info); +virDomainDeviceInfoFreeContents(&def->info); g_free(def); } @@ -2855,7 +2855,7 @@ void virDomainSoundDefFree(virDomainSoundDefPtr def) if (!def) return; -virDomainDeviceInfoClear(&def->info); +virDomainDeviceInfoFreeContents(&def->info); for (i = 0; i < def->ncodecs; i++) virDomainSoundCodecDefFree(def->codecs[i]); @@ -2895,7 +2895,7 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def) if (!def) return; -virDomainDeviceInfoClear(&def->info); +virDomainDeviceInfoFreeContents(&def->info); g_free(def->virtio); g_free(def); @@ -2906,7 +2906,7 @@ void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def) if (!def) return; -virDomainDeviceInfoClear(&def->info); +vir
Re: [PATCH 02/19] qemu: capabilities: Introduce QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING
On Thu, Feb 11, 2021 at 16:37:41 +0100, Peter Krempa wrote: > The capability represents qemu's ability to setup mappings for migrating > block dirty bitmaps and is based on presence of the 'transform' property > of the 'block-bitmap-mapping' property of 'migrate-set-parameters' QMP > command. > > Signed-off-by: Peter Krempa > --- > src/qemu/qemu_capabilities.c | 2 ++ > src/qemu/qemu_capabilities.h | 1 + > 2 files changed, 3 insertions(+) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index ccf810ff96..38555dde98 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -616,6 +616,7 @@ VIR_ENUM_IMPL(virQEMUCaps, >"vhost-user-blk", >"cpu-max", >"memory-backend-file.x-use-canonical-path-for-ramblock-id", > + "migration-param.block-bitmap-mapping", > ); > > > @@ -1549,6 +1550,7 @@ static struct virQEMUCapsStringFlags > virQEMUCapsQMPSchemaQueries[] = { > { "migrate-set-parameters/arg-type/xbzrle-cache-size", > QEMU_CAPS_MIGRATION_PARAM_XBZRLE_CACHE_SIZE }, > { "set-numa-node/arg-type/+hmat-lb", QEMU_CAPS_NUMA_HMAT }, > { "netdev_add/arg-type/+vhost-vdpa", QEMU_CAPS_NETDEV_VHOST_VDPA }, > +{ "migrate-set-parameters/arg-type/block-bitmap-mapping/transform", > QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING }, > }; > So how is it possible this change is not reflected in tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml after the previous patch updated QEMU replies? Interestingly enough, tests pass after this patch so either the capability detection is not working or QEMU replies do not actually contain what you're looking for here. Jirka