- Devel - libvirt List Archives

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

07 Sep '23

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

3 2

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

07 Sep '23

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

1 0

[PATCH] manpage: virsh: Note that 'virsh undefine --remove-all-storage' doesn't remove backing images
by Peter Krempa 07 Sep '23

07 Sep '23

Removing a backing image could break other image chains as it's theoretically possible to share backing chains. As --storage/--remove-all-storage is fully implemented in virsh as a helper option, which enumerates and deletes VM's volumes. We do not plan to make it any more complicated. Document that backing chains are not removed. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- docs/manpages/virsh.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 91e1d5de37..4ae3bb4d93 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -4614,12 +4614,13 @@ undefined; if not all of the requested volumes could be deleted, the error message indicates what still remains behind. If a volume path is not found in the domain definition, it's treated as if the volume was successfully deleted. Only volumes managed by libvirt in storage pools can be removed this -way. +way. Note that this also removes only the top level image of a backing chain, +any backing stores of the image are kept as they may be shared. (See ``domblklist`` for list of target names associated to a domain). Example: --storage vda,/path/to/storage.img The *--remove-all-storage* flag specifies that all of the domain's storage -volumes should be deleted. +volumes should be deleted as if they were specified via *--storage*. The *--delete-storage-volume-snapshots* (previously *--delete-snapshots*) flag specifies that any snapshots associated with -- 2.41.0

2 1

[PATCH] virsh-snapshot: Avoid passing NULL to qsort() in virshSnapshotListCollect()
by Michal Privoznik 07 Sep '23

07 Sep '23

If a domain has no snapshots and 'virsh snapshot-list' is called, this gets all the way down to virshSnapshotListCollect() which then collects all snapshots (none), and passes them to qsort() which doesn't like being called with NULL: extern void qsort (void *__base, size_t __nmemb, size_t __size, __compar_fn_t __compar) __nonnull ((1, 4)); Resolves: https://gitlab.com/libvirt/libvirt/-/issues/533 Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- tools/virsh-snapshot.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index d7889a38e4..ecb935b2b4 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -1310,9 +1310,11 @@ virshSnapshotListCollect(vshControl *ctl, virDomainPtr dom, } } } - if (!(orig_flags & VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL)) + if (!(orig_flags & VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL) && + snaplist->snaps) { qsort(snaplist->snaps, snaplist->nsnaps, sizeof(*snaplist->snaps), virshSnapSorter); + } snaplist->nsnaps -= deleted; ret = g_steal_pointer(&snaplist); -- 2.41.0

2 1

[PATCH] lxc: fix lxcContainerMountAllFS() DEREF_BEFORE_CHECK
by Dmitry Frolov 07 Sep '23

07 Sep '23

Reviewing the sources, I found, that in function lxcContainerMountAllFS() pointers vmDef->fss[i]->src and vmDef->fss[i]->src->path are checked for NULL after dereferencing in VIR_DEBUG() macro. Fixes: 57487085dc ("lxc: don't try to reference NULL when mounting filesystems") --- src/lxc/lxc_container.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 21220661f7..58a6695458 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1467,13 +1467,15 @@ static int lxcContainerMountAllFS(virDomainDef *vmDef, if (STREQ(vmDef->fss[i]->dst, "/")) continue; + if (!(vmDef->fss[i]->src && vmDef->fss[i]->src->path)) + return -1; + VIR_DEBUG("Mounting '%s' -> '%s'", vmDef->fss[i]->src->path, vmDef->fss[i]->dst); if (lxcContainerResolveSymlinks(vmDef->fss[i], false) < 0) return -1; - if (!(vmDef->fss[i]->src && vmDef->fss[i]->src->path && - STRPREFIX(vmDef->fss[i]->src->path, vmDef->fss[i]->dst)) && + if (!STRPREFIX(vmDef->fss[i]->src->path, vmDef->fss[i]->dst) && lxcContainerUnmountSubtree(vmDef->fss[i]->dst, false) < 0) return -1; -- 2.34.1

4 4

[PATCH Libvirt v3 00/10] Support dirty page rate upper limit
by ~hyman 07 Sep '23

07 Sep '23

Hi, This is the latest version for the series, please review. Thanks, Yong v3: - rebase on master and fix conflicts - refactor the comments and update the version tag v2: The patch set introduce a new API virDomainSetVcpuDirtyLimit to allow upper Apps to set upper limits of dirty page rate for virtual CPUs, the corresponding virsh API as follows: limit-dirty-page-rate <domain> <rate> [--vcpu <number>] \ [--config] [--live] [--current] We put the dirty limit persistent info with the "vcpus" element in domain XML and extend dirtylimit statistics for domGetStats: <domain> ... <vcpu current='2'>3</vcpu> <vcpus> <vcpu id='0' hotpluggable='no' dirty_limit='10' order='1'.../> <vcpu id='1' hotpluggable='yes' dirty_limit='10' order='2'.../> </vcpus> ... </domain> If --vcpu option is not passed in the virsh command, set all virtual CPUs; if rate is set to zero, cancel the upper limit. Examples: To set the dirty page rate upper limit 10 MB/s for all virtual CPUs in c81_node1, use: # virsh limit-dirty-page-rate c81_node1 --rate 10 --live Set dirty page rate limit 10(MB/s) for all virtual CPUs successfully # virsh dumpxml c81_node1 | grep dirty_limit <vcpu id='0' enabled='yes' hotpluggable='no' order='1' dirty_limit='10'/> <vcpu id='1' enabled='yes' hotpluggable='no' order='2' dirty_limit='10'/> <vcpu id='2' enabled='yes' hotpluggable='no' order='3' dirty_limit='10'/> <vcpu id='3' enabled='no' hotpluggable='yes' dirty_limit='10'/> <vcpu id='4' enabled='no' hotpluggable='yes' dirty_limit='10'/> ...... Query the dirty limit info dynamically: # virsh domstats c81_node1 --dirtylimit Domain: 'c81_node1' dirtylimit.vcpu.0.limit=10 dirtylimit.vcpu.0.current=0 dirtylimit.vcpu.1.limit=10 dirtylimit.vcpu.1.current=0 dirtylimit.vcpu.2.limit=10 dirtylimit.vcpu.2.current=0 dirtylimit.vcpu.3.limit=10 dirtylimit.vcpu.3.current=0 dirtylimit.vcpu.4.limit=10 dirtylimit.vcpu.4.current=0 ...... To cancel the upper limit, use: # virsh limit-dirty-page-rate c81_node1 --rate 0 --live Cancel dirty page rate limit for all virtual CPUs successfully # virsh dumpxml c81_node1 | grep dirty_limit # virsh domstats c81_node1 --dirtylimit Domain: 'c81_node1' The dirty limit uses the QEMU dirty-limit feature introduced since 7.1.0, this feature allows CPU to be throttled as needed to keep their dirty page rate within the limit. It could, in some scenes, be used to provide quality-of-service in the aspect of the memory workload for virtual CPUs and QEMU itself use the feature to implement the dirty-limit throttle algorithm and apply it on the live migration, which improve responsiveness of large guests during live migration and can result in more stable read performance. The other application scenarios remain unexplored, before that, Libvirt could provide the basic API. Hyman Huang(黄勇) (10): qemu_capabilities: Introduce QEMU_CAPS_VCPU_DIRTY_LIMIT capability conf: Introduce XML for dirty limit configuration libvirt: Add virDomainSetVcpuDirtyLimit API qemu_driver: Implement qemuDomainSetVcpuDirtyLimit domain_validate: Export virDomainDefHasDirtyLimitStartupVcpus symbol qemu_process: Setup dirty limit after launching VM virsh: Introduce limit-dirty-page-rate api qemu_monitor: Implement qemuMonitorQueryVcpuDirtyLimit qemu_driver: Extend dirtlimit statistics for domGetStats virsh: Introduce command 'virsh domstats --dirtylimit' docs/formatdomain.rst | 7 +- docs/manpages/virsh.rst | 38 +++- include/libvirt/libvirt-domain.h | 5 + src/conf/domain_conf.c | 26 +++ src/conf/domain_conf.h | 8 + src/conf/domain_validate.c | 33 ++++ src/conf/domain_validate.h | 2 + src/conf/schemas/domaincommon.rng | 5 + src/driver-hypervisor.h | 7 + src/libvirt-domain.c | 68 +++++++ src/libvirt_private.syms | 1 + src/libvirt_public.syms | 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 181 ++++++++++++++++++ src/qemu/qemu_monitor.c | 25 +++ src/qemu/qemu_monitor.h | 22 +++ src/qemu/qemu_monitor_json.c | 107 +++++++++++ src/qemu/qemu_monitor_json.h | 9 + src/qemu/qemu_process.c | 44 +++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 18 +- src/remote_protocol-structs | 7 + .../qemucapabilitiesdata/caps_7.1.0_ppc64.xml | 1 + .../caps_7.1.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml | 1 + .../caps_7.2.0_x86_64+hvf.xml | 1 + .../caps_7.2.0_x86_64.xml | 1 + .../caps_8.0.0_riscv64.xml | 1 + .../caps_8.0.0_x86_64.xml | 1 + .../qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 + .../caps_8.1.0_x86_64.xml | 1 + tools/virsh-domain-monitor.c | 7 + tools/virsh-domain.c | 109 +++++++++++ 34 files changed, 739 insertions(+), 4 deletions(-) -- 2.38.5

4 17

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

06 Sep '23

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

3 2

[libvirt PATCH 0/2] Add cpu model for EPYC Genoa
by Tim Wiederhake 06 Sep '23

06 Sep '23

Plus one drive-by improvement to sync_qemu_features_i386.py. Tim Wiederhake (2): cpu_map: Ignore all vmx-* features in sync_qemu_features_i386.py cpu_map: Add cpu model EPYC Genoa src/cpu_map/index.xml | 1 + src/cpu_map/meson.build | 1 + src/cpu_map/sync_qemu_features_i386.py | 90 +------------- src/cpu_map/x86_EPYC-Genoa.xml | 115 ++++++++++++++++++ .../domaincapsdata/qemu_8.1.0-q35.x86_64.xml | 1 + .../domaincapsdata/qemu_8.1.0-tcg.x86_64.xml | 1 + tests/domaincapsdata/qemu_8.1.0.x86_64.xml | 1 + 7 files changed, 121 insertions(+), 89 deletions(-) create mode 100644 src/cpu_map/x86_EPYC-Genoa.xml -- 2.39.2

2 3

[libvirt PATCH] rpm: Drop obsolete Obsoletes
by Andrea Bolognani 06 Sep '23

06 Sep '23

All platforms we target come with at least libvirt 6.0.0, so none of the Obsoletes referring to earlier versions are useful at this point. Signed-off-by: Andrea Bolognani <abologna(a)redhat.com> --- libvirt.spec.in | 7 ------- 1 file changed, 7 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index b471afebb1..3e68961fb9 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -250,9 +250,6 @@ Requires: libvirt-daemon-driver-lxc = %{version}-%{release} %if %{with_qemu} Requires: libvirt-daemon-driver-qemu = %{version}-%{release} %endif -# We had UML driver, but we've removed it. -Obsoletes: libvirt-daemon-driver-uml <= 5.0.0 -Obsoletes: libvirt-daemon-uml <= 5.0.0 %if %{with_vbox} Requires: libvirt-daemon-driver-vbox = %{version}-%{release} %endif @@ -608,9 +605,6 @@ Requires: util-linux # From QEMU RPMs Requires: /usr/bin/qemu-img %endif -%if !%{with_storage_rbd} -Obsoletes: libvirt-daemon-driver-storage-rbd < 5.2.0 -%endif Obsoletes: libvirt-daemon-driver-storage-sheepdog < 8.8.0 %description daemon-driver-storage-core @@ -812,7 +806,6 @@ VirtualBox Summary: Libxl driver plugin for the libvirtd daemon Requires: libvirt-daemon-common = %{version}-%{release} Requires: libvirt-libs = %{version}-%{release} -Obsoletes: libvirt-daemon-driver-xen < 4.3.0 %description daemon-driver-libxl The Libxl driver plugin for the libvirtd daemon, providing -- 2.41.0

2 6

virDomainGetJobStats not populating VIR_DOMAIN_JOB_DISK_BPS
by João Vilaça 06 Sep '23

06 Sep '23

Hello, I'm using https://gitlab.com/libvirt/libvirt-go-module and executing 'GetJobStats' to get information about the progress of a VM with a disk migration in KubeVirt. Everything works correctly and the migration is successful, with the disk correctly copied. Across multiple executions of 'GetJobStats' while copying the disk, I get the information about data: *DataTotal:151592960 DataProcessedSet:true DataProcessed:163080901 DataRemainingSet:true DataRemaining:266240* about disk total values: *DiskTotal:21037056 DiskProcessedSet:true DiskProcessed:13631488 DiskRemainingSet:true DiskRemaining:7405568* but '*DiskBpsSet:false *' and '*DiskBps:0 *' are always false and 0. Even when I run 'virsh domjobinfo', I get Data and File absolute values, but not the throughput. Is QEMU not reporting this data? Can this be a bug? -- kind regards, João Vilaça

2 1