[PATCH v6 04/11] nbd: Update qapi to support exporting multiple bitmaps
Since 'block-export-add' is new to 5.2, we can still tweak the
interface; there, allowing 'bitmaps':['str'] is nicer than
'bitmap':'str'. This wires up the qapi and qemu-nbd changes to permit
passing multiple bitmaps as distinct metadata contexts that the NBD
client may request, but the actual support for more than one will
require a further patch to the server.
Note that there are no changes made to the existing deprecated
'nbd-server-add' command; this required splitting the QAPI type
BlockExportOptionsNbd, which fortunately does not affect QMP
introspection.
Signed-off-by: Eric Blake
---
docs/system/deprecated.rst | 3 ++-
qapi/block-export.json | 41 +++---
blockdev-nbd.c | 6 +-
nbd/server.c | 19 --
qemu-nbd.c | 18 -
5 files changed, 58 insertions(+), 29 deletions(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 0ebce37a1919..32a0e620dbb9 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -257,7 +257,8 @@ the 'wait' field, which is only applicable to sockets in
server mode
Use the more generic commands ``block-export-add`` and ``block-export-del``
-instead.
+instead. As part of this deprecation, where ``nbd-server-add`` used a
+single ``bitmap``, the new ``block-export-add`` uses a list of ``bitmaps``.
Human Monitor Protocol (HMP) commands
-
diff --git a/qapi/block-export.json b/qapi/block-export.json
index 480c497690b0..c4125f4d2104 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -63,10 +63,10 @@
'*max-connections': 'uint32' } }
##
-# @BlockExportOptionsNbd:
+# @BlockExportOptionsNbdBase:
#
-# An NBD block export (options shared between nbd-server-add and the NBD branch
-# of block-export-add).
+# An NBD block export (common options shared between nbd-server-add and
+# the NBD branch of block-export-add).
#
# @name: Export name. If unspecified, the @device parameter is used as the
#export name. (Since 2.12)
@@ -74,15 +74,27 @@
# @description: Free-form description of the export, up to 4096 bytes.
# (Since 5.0)
#
-# @bitmap: Also export the dirty bitmap reachable from @device, so the
-# NBD client can use NBD_OPT_SET_META_CONTEXT with
-# "qemu:dirty-bitmap:NAME" to inspect the bitmap. (since 4.0)
-#
# Since: 5.0
##
+{ 'struct': 'BlockExportOptionsNbdBase',
+ 'data': { '*name': 'str', '*description': 'str' } }
+
+##
+# @BlockExportOptionsNbd:
+#
+# An NBD block export (distinct options used in the NBD branch of
+# block-export-add).
+#
+# @bitmaps: Also export each of the named dirty bitmaps reachable from
+# @device, so the NBD client can use NBD_OPT_SET_META_CONTEXT with
+# the metadata context name "qemu:dirty-bitmap:BITMAP" to inspect
+# each bitmap.
+#
+# Since: 5.2
+##
{ 'struct': 'BlockExportOptionsNbd',
- 'data': { '*name': 'str', '*description': 'str',
-'*bitmap': 'str' } }
+ 'base': 'BlockExportOptionsNbdBase',
+ 'data': { '*bitmaps': ['str'] } }
##
# @BlockExportOptionsVhostUserBlk:
@@ -106,19 +118,24 @@
##
# @NbdServerAddOptions:
#
-# An NBD block export.
+# An NBD block export, per legacy nbd-server-add command.
#
# @device: The device name or node name of the node to be exported
#
# @writable: Whether clients should be able to write to the device via the
#NBD connection (default false).
#
+# @bitmap: Also export a single dirty bitmap reachable from @device, so the
+# NBD client can use NBD_OPT_SET_META_CONTEXT with the metadata
+# context name "qemu:dirty-bitmap:BITMAP" to inspect the bitmap
+# (since 4.0).
+#
# Since: 5.0
##
{ 'struct': 'NbdServerAddOptions',
- 'base': 'BlockExportOptionsNbd',
+ 'base': 'BlockExportOptionsNbdBase',
'data': { 'device': 'str',
-'*writable': 'bool' } }
+'*writable': 'bool', '*bitmap': 'str' } }
##
# @nbd-server-add:
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index cee9134b12eb..d1d41f635564 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -209,8 +209,12 @@ void qmp_nbd_server_add(NbdServerAddOptions *arg, Error
**errp)
.has_writable = arg->has_writable,
.writable = arg->writable,
};
-QAPI_CLONE_MEMBERS(BlockExportOptionsNbd, _opts->u.nbd,
+QAPI_CLONE_MEMBERS(BlockExportOptionsNbdBase, _opts->u.nbd,
qapi_NbdServerAddOptions_base(arg));
+if (arg->has_bitmap) {
+export_opts->u.nbd.has_bitmaps = true;
+QAPI_LIST_ADD(export_opts->u.nbd.bitmaps, g_strdup(arg->bitmap));
+}
/*
* nbd-server-add doesn't complain when a read-only device should be
diff --git a/nbd/server.c b/nbd/server.c
index 08b621f70a3a..8d01662b4511 100644
--- a/nbd/server.c
+++
Re: [PATCH v6 0/4] qemu: Support rbd namespace in rbd source name
ping. Any reviews? On Sat, Oct 10, 2020 at 3:07 PM Han Han wrote: > Diffence from v5: > - conf: implement virStringSplit() to parse the src->path > - tests: Add the cases of path 'pool//image' to > disk-network-rbd-namespace test > - temporarily removed the patch of news > - tweak of version in docs > - rebase to the latest main branch > > v5: > https://www.redhat.com/archives/libvir-list/2020-September/msg00081.html > git repo: https://gitlab.com/hhan2/libvirt/-/commits/rbd-namespace-v6 > > Han Han (4): > qemu_capabilities: Add QEMU_CAPS_RBD_NAMESPACE > util: Add member ns to the storage source struct > conf: Support to parse rbd namespace from source name > qemu: Implement rbd namespace to the source name attribute > > docs/formatdomain.rst | 16 ++ > src/conf/domain_conf.c| 47 -- > src/qemu/qemu_block.c | 1 + > src/qemu/qemu_capabilities.c | 2 + > src/qemu/qemu_capabilities.h | 1 + > src/qemu/qemu_domain.c| 8 +++ > src/util/virstoragefile.c | 2 + > src/util/virstoragefile.h | 1 + > .../caps_5.0.0.aarch64.xml| 1 + > .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + > .../caps_5.0.0.riscv64.xml| 1 + > .../caps_5.0.0.x86_64.xml | 1 + > .../caps_5.1.0.x86_64.xml | 1 + > .../caps_5.2.0.x86_64.xml | 1 + > ...k-network-rbd-namespace.x86_64-latest.args | 48 ++ > .../disk-network-rbd-namespace.xml| 40 +++ > tests/qemuxml2argvtest.c | 1 + > ...sk-network-rbd-namespace.x86_64-latest.xml | 49 +++ > tests/qemuxml2xmltest.c | 1 + > 19 files changed, 219 insertions(+), 4 deletions(-) > create mode 100644 > tests/qemuxml2argvdata/disk-network-rbd-namespace.x86_64-latest.args > create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-namespace.xml > create mode 100644 > tests/qemuxml2xmloutdata/disk-network-rbd-namespace.x86_64-latest.xml > > -- > 2.26.2 > >
Re: [libvirt PATCH 1/2] rpc: Fix virt-ssh-helper detection
On Mon, Oct 26, 2020 at 7:30 PM Andrea Bolognani wrote:
>
> When trying to figure out whether virt-ssh-helper is available
> on the remote host, we mistakenly look for the helper by the
> name it had while the feature was being worked on instead of
> the one that was ultimately picked, and thus end up using the
> netcat fallback every single time.
>
> Fixes: f8ec7c842df9e40c6607eae9b0223766cb226336
> Signed-off-by: Andrea Bolognani
> ---
> src/rpc/virnetclient.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
> index c6591ecdfc..2eabacd6e8 100644
> --- a/src/rpc/virnetclient.c
> +++ b/src/rpc/virnetclient.c
> @@ -453,7 +453,7 @@ virNetClientSSHHelperCommand(virNetClientProxy proxy,
>
> switch (proxy) {
> case VIR_NET_CLIENT_PROXY_AUTO:
> -return g_strdup_printf("sh -c 'which virt-nc 1>/dev/null 2>&1; "
> +return g_strdup_printf("sh -c 'which virt-ssh-helper 1>/dev/null
> 2>&1; "
> "if test $? = 0; then "
> "%s; "
> "else"
> --
> 2.26.2
>
Reviewed-by: Neal Gompa
--
真実はいつも一つ！/ Always, there's only one truth!
Re: [libvirt PATCH 2/2] news: Mention virt-ssh-helper detection fix
On Mon, Oct 26, 2020 at 7:31 PM Andrea Bolognani wrote: > > Signed-off-by: Andrea Bolognani > --- > NEWS.rst | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/NEWS.rst b/NEWS.rst > index 3587bc2c13..5bd8ed1c91 100644 > --- a/NEWS.rst > +++ b/NEWS.rst > @@ -46,6 +46,12 @@ v6.9.0 (unreleased) > Relying on the "Description" field caused queries to fail on non-"en-US" > systems. The queries have been updated to avoid using localized strings. > > + * rpc: Fix ``virt-ssh-helper`` detection > + > +libvirt 6.8.0 failed to correctly detect the availability of the new > +``virt-ssh-helper`` command on the remote host, and thus always used the > +fallback instead; this has now been fixed. > + > > v6.8.0 (2020-10-01) > === > -- > 2.26.2 > Reviewed-by: Neal Gompa -- 真実はいつも一つ！/ Always, there's only one truth!
[libvirt PATCH 0/2] rpc: Fix virt-ssh-helper detection
See patch 1 for details. Andrea Bolognani (2): rpc: Fix virt-ssh-helper detection news: Mention virt-ssh-helper detection fix NEWS.rst | 6 ++ src/rpc/virnetclient.c | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) -- 2.26.2
Re: [libvirt PATCH v2 10/10] rpc: use new virt-ssh-helper binary for remote tunnelling
On Fri, 2020-07-24 at 18:22 +0200, Andrea Bolognani wrote:
> On Fri, 2020-07-24 at 16:14 +0100, Daniel P. Berrangé wrote:
> > char *
> > +virNetClientSSHHelperCommand(virNetClientProxy proxy,
> > + const char *netcatPath,
> > + const char *socketPath,
> > + const char *driverURI,
> > + bool readonly)
> > +{
> [...]
> > +switch (proxy) {
> > +case VIR_NET_CLIENT_PROXY_AUTO:
> > +return g_strdup_printf("sh -c 'which virt-nc 1>/dev/null 2>&1; "
> > + "if test $? = 0; then "
> > + "%s; "
> > + "else"
> > + "%s; "
> > + "fi'", helpercmd, nccmd);
>
> s/virt-nc/virt-ssh-helper/
It appears that you forgot to fix this before pushing, and so 6.8.0
was released with broken detection which causes it to only ever use
virt-ssh-helper when forced through URI parameters.
I have prepared the obvious fix[1], can you please ACK it so that we
can release 6.9.0 with the feature working as intended? Thanks!
[1] https://www.redhat.com/archives/libvir-list/2020-October/msg01410.html
--
Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH 1/2] rpc: Fix virt-ssh-helper detection
When trying to figure out whether virt-ssh-helper is available
on the remote host, we mistakenly look for the helper by the
name it had while the feature was being worked on instead of
the one that was ultimately picked, and thus end up using the
netcat fallback every single time.
Fixes: f8ec7c842df9e40c6607eae9b0223766cb226336
Signed-off-by: Andrea Bolognani
---
src/rpc/virnetclient.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index c6591ecdfc..2eabacd6e8 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -453,7 +453,7 @@ virNetClientSSHHelperCommand(virNetClientProxy proxy,
switch (proxy) {
case VIR_NET_CLIENT_PROXY_AUTO:
-return g_strdup_printf("sh -c 'which virt-nc 1>/dev/null 2>&1; "
+return g_strdup_printf("sh -c 'which virt-ssh-helper 1>/dev/null 2>&1;
"
"if test $? = 0; then "
"%s; "
"else"
--
2.26.2
[libvirt PATCH 2/2] news: Mention virt-ssh-helper detection fix
Signed-off-by: Andrea Bolognani --- NEWS.rst | 6 ++ 1 file changed, 6 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 3587bc2c13..5bd8ed1c91 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -46,6 +46,12 @@ v6.9.0 (unreleased) Relying on the "Description" field caused queries to fail on non-"en-US" systems. The queries have been updated to avoid using localized strings. + * rpc: Fix ``virt-ssh-helper`` detection + +libvirt 6.8.0 failed to correctly detect the availability of the new +``virt-ssh-helper`` command on the remote host, and thus always used the +fallback instead; this has now been fixed. + v6.8.0 (2020-10-01) === -- 2.26.2
Re: Entering freeze for libvirt-6.9.0
> On Oct 26, 2020, at 2:02 PM, Jiri Denemark wrote: > > I have just tagged v6.9.0-rc1 in the repository and pushed signed > tarballs and source RPMs to https://libvirt.org/sources/ > > Please give the release candidate some testing and in case you find a > serious issue which should have a fix in the upcoming release, feel > free to reply to this thread to make sure the issue is more visible. I'm not sure if it's a "serious" issue, since the code does work, but I'd like to see my "Hyper-V code cleanup" patchset make it into 6.9.0. It removes some duplicate functions that I accidentally committed due to the WQL formatting being so different. It also includes some general code cleanup. Thanks! -- Matt
[PATCH] rpm: Fix handling of SOURCE_DATE_EPOCH
Contemporary versions of Fedora automatically set SOURCE_DATE_EPOCH
based on the changelog entry date stamp. In scenarios where it already
is defined, we do not want to redefine it.
Additionally, when building the libvirt package in an Open Build Service
instance, the spec file is not present in %_specdir, but instead in %_sourcedir.
Signed-off-by: Neal Gompa
---
libvirt.spec.in | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 2a4324b974..eb0fba4b5a 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1128,7 +1128,16 @@ exit 1
# place macros above and build commands below this comment
-export SOURCE_DATE_EPOCH=$(stat --printf='%Y' %{_specdir}/%{name}.spec)
+if [ -z "${SOURCE_DATE_EPOCH}" ]; then
+# If SOURCE_DATE_EPOCH is not defined, set it based on spec file
+if [ ! -f "%{_specdir}/%{name}.spec" ]; then
+# OBS does not install the spec file into the specdir
+SPECFILE_PATH="%{_sourcedir}/%{name}.spec"
+else
+SPECFILE_PATH="%{_specdir}/%{name}.spec"
+fi
+export SOURCE_DATE_EPOCH=$(stat --printf='%Y' ${SPECFILE_PATH})
+fi
%meson \
-Drunstatedir=%{_rundir} \
--
2.28.0
Re: Entering freeze for libvirt-6.9.0
On Mon, Oct 26, 2020 at 2:03 PM Jiri Denemark wrote: > > I have just tagged v6.9.0-rc1 in the repository and pushed signed > tarballs and source RPMs to https://libvirt.org/sources/ > > Please give the release candidate some testing and in case you find a > serious issue which should have a fix in the upcoming release, feel > free to reply to this thread to make sure the issue is more visible. > > If you have not done so yet, please update NEWS.rst to document any > significant change you made since the last release. > I encountered two basic issues when testing package builds in my pipeline, and have sent patches to the list: * https://www.redhat.com/archives/libvir-list/2020-October/msg01407.html * https://www.redhat.com/archives/libvir-list/2020-October/msg01408.html Those two were the most urgent I discovered with a quick run for my pipeline. -- 真実はいつも一つ！/ Always, there's only one truth!
[PATCH] Revert "spec: Simplify setting features off by default"
As it turns out, the rather complicated structure that is
currently used for enabling or disabling features in the libvirt
build does not cleanly map well to RPM's bcond feature.
Consequently, we need these back in order to support trivially
activating these features through extra macros as build inputs.
This reverts commit 31d687a3218c9072d7050dd608e013e063ca180f.
Signed-off-by: Neal Gompa
---
libvirt.spec.in | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 2a4324b974..84515cc7de 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -98,14 +98,14 @@
%define with_numactl 0%{!?_without_numactl:1}
# A few optional bits off by default, we enable later
-%define with_fuse 0
-%define with_sanlock 0
-%define with_numad0
-%define with_firewalld_zone 0
-%define with_libssh2 0
-%define with_wireshark0
-%define with_libssh 0
-%define with_dmidecode0
+%define with_fuse 0%{!?_without_fuse:0}
+%define with_sanlock 0%{!?_without_sanlock:0}
+%define with_numad0%{!?_without_numad:0}
+%define with_firewalld_zone 0%{!?_without_firewalld:0}
+%define with_libssh2 0%{!?_without_libssh2:0}
+%define with_wireshark0%{!?_without_wireshark:0}
+%define with_libssh 0%{!?_without_libssh:0}
+%define with_dmidecode0%{!?_without_dmidecode:0}
# Finally set the OS / architecture specific special cases
--
2.28.0
Re: [PATCH 09/13] util: hash: Don't use 'const' with virHashTablePtr
On 10/26/20 10:45 AM, Peter Krempa wrote: > We didn't use it rigorously and some helpers even dasted it away. Remove s/dasted/cast/ (both the typo, and the odd English word whose present and past tense forms are identical) > const from all hash utility functions. > > Signed-off-by: Peter Krempa > --- -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 0/6] qemu: Fix cdrom as SCSI hostdev via -blockdev
Peter, > Sent: Saturday, October 24, 2020 at 8:28 AM > From: "daggs" > To: "Peter Krempa" > Cc: libvir-list@redhat.com > Subject: Re: [PATCH 0/6] qemu: Fix cdrom as SCSI hostdev via -blockdev > > Greetings Peter, > > looks like I was mistaken, I see the same prints in 6.8.0 + your patchset. > this means it is not related, so question is, is this a livbirt misconfig or > qemu bug? > > any insights? > can you comment on the below please? I think it is an qemu bug Dagg.
Entering freeze for libvirt-6.9.0
I have just tagged v6.9.0-rc1 in the repository and pushed signed tarballs and source RPMs to https://libvirt.org/sources/ Please give the release candidate some testing and in case you find a serious issue which should have a fix in the upcoming release, feel free to reply to this thread to make sure the issue is more visible. If you have not done so yet, please update NEWS.rst to document any significant change you made since the last release. Thanks, Jirka
Re: [PATCH v3] qemuMonitorJSONCheckReply: Use g_autofree
On a Wednesday in 2020, Yi Li wrote: Eliminate cleanup code by using g_autofree. Signed-off-by: Yi Li --- src/qemu/qemu_monitor_json.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) Reviewed-by: Ján Tomko And pushed. Jano signature.asc Description: PGP signature
Re: [PATCH] docs: page.xsl: Improve generation of paragraph anchor links
On 10/26/20 1:51 PM, Peter Krempa wrote: Use the 'parent' axis to check whether the parent is a div with class='section' rather than looking for 'toc-backref' anchor to see whether to generate one of the headerlink alternatives. Both hare docutils-specific thus apply to docs generated from RST documents. This adds the links for pages generated from RST documents which don't have a table of contents (and thus lack the 'toc-backref' anchors) and thus fixes pages such as hacking.html and news.html to have reasonable links which can be shared. Signed-off-by: Peter Krempa --- Note that whether or not to add table of contents to hacking.rst is out of scope of this patch. docs/page.xsl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Michal Privoznik Michal
Re: [PATCH] docs/system: Deprecate raspi2/raspi3 machine aliases
Ping? (patch Acked).
On 10/19/20 10:21 AM, Philippe Mathieu-Daudé wrote:
Since commit aa35ec2213b ("hw/arm/raspi: Use more
specific machine names") the raspi2/raspi3 machines
have been renamed as raspi2b/raspi3b.
As more Raspberry Pi 2/3 models are emulated, in order
to avoid confusion deprecate the raspi2/raspi3 machine
aliases.
Signed-off-by: Philippe Mathieu-Daudé
---
docs/system/deprecated.rst | 5 +
1 file changed, 5 insertions(+)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 905628f3a0c..f0c7aaeb2ff 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -355,6 +355,11 @@ This machine has been renamed ``fuloong2e``.
These machine types are very old and likely can not be used for live migration
from old QEMU versions anymore. A newer machine type should be used instead.
+Raspberry Pi ``raspi2`` and ``raspi3`` machines (since 5.2)
+'
+
+These machines have been respectively renamed ``raspi2b`` and ``raspi3b``.
+
Device options
--
Re: [PATCH 0/9] several small cleanups in virpci.c et. al.
On a Tuesday in 2020, Laine Stump wrote: These started with a couple small cleanups to functions I was going to change the API for, but then I decided not to change the API, and just didn't want to waste the effort spent cleaning them up. Thank you. Laine Stump (9): util: simplify virHostdevPCISysfsPath() util: simplify virPCIFile() and its callers util: simplify virPCIDriverDir() and its callers util: simplify virPCIProbeStubDriver() util: avoid manual VIR_FREE of a g_autofree pointer in virPCIGetName() util: use more g_autofree in virpci.c util: remove unneeded cleanup:/ret in virpci.c util: don't use virPCIGetSysfsFile() util: remove unused function virPCIGetSysfsFile() src/hypervisor/virhostdev.c | 10 +-- src/libvirt_private.syms| 1 - src/util/virnetdev.c| 6 +- src/util/virpci.c | 146 src/util/virpci.h | 4 - 5 files changed, 53 insertions(+), 114 deletions(-) Beautiful. Jano -- 2.26.2 signature.asc Description: PGP signature
Re: [PATCH 1/1] virt-aa-helper: allow hard links for mounts
On Montag, 26. Oktober 2020 09:12:38 CET Michal Privoznik wrote: > On 10/23/20 4:19 PM, Christian Schoenebeck wrote: > > On Donnerstag, 22. Oktober 2020 19:07:33 CEST Michal Privoznik wrote: > >> [Please don't CC random people on patches until asked to, we are all > >> subscribed to the list] > > > > Got it, I'll refrain from CCing on libvirt in future. > > > > Not as erratic as it looks like though: I CCed people who touched this > > specific AppArmor permission before, plus the virtiofs maintainers. > > Yeah, I understand that. BTW: it's okay to CC people when replying :-) > > >> On 10/22/20 4:58 PM, Christian Schoenebeck wrote: > >>> Guests should be allowed to create hard links on mounted pathes, since > >>> many applications rely on this functionality and would error on guest > >>> with current "rw" AppArmor permission with 9pfs. > >>> > >>> Signed-off-by: Christian Schoenebeck > >>> --- > >>> > >>>src/security/virt-aa-helper.c | 2 +- > >>>1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/src/security/virt-aa-helper.c > >>> b/src/security/virt-aa-helper.c > >>> index 12429278fb..5a6f4a5f7d 100644 > >>> --- a/src/security/virt-aa-helper.c > >>> +++ b/src/security/virt-aa-helper.c > >>> @@ -1142,7 +1142,7 @@ get_files(vahControl * ctl) > >>> > >>>/* We don't need to add deny rw rules for readonly > >>>mounts, > >>> > >>> * this can only lead to troubles when mounting / > >>> readonly. > >>> */ > >>> > >>> -if (vah_add_path(, fs->src->path, fs->readonly ? "R" : > >>> "rw", true) != 0) +if (vah_add_path(, fs->src->path, > >>> fs->readonly ? "R" : "rwl", true) != 0)> > >>> > >>>goto cleanup; > >>> > >>>} > >>> > >>>} > >> > >> Reviewed-by: Michal Privoznik > >> > >> but I will give a day or two for other developers to chime in. > >> > >> Michal > > > > Yes, please wait couple days to see whether there are reactions. > > Okay, so nobody objected and we can expect the freeze of upstream today, > so I am pushing this. Yes, makes sense. Thanks Michal! Best regards, Christian Schoenebeck
Re: [PATCH 03/13] util: virhash: Standardize on 'opaque' for opaque data
On a Monday in 2020, Peter Krempa wrote: Rename 'data' argument which is used for opaque data. Signed-off-by: Peter Krempa --- src/util/virhash.c | 24 src/util/virhash.h | 14 +++--- 2 files changed, 19 insertions(+), 19 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH v2 0/5] Hypervisor CPU Baseline Cleanups and Fixes
Polite ping. Have there been relevant updates elsewhere that I might've missed? Thanks! On 9/24/20 8:22 PM, Collin Walling wrote: > The following patches provide some TLC to the hypervisor CPU baseline > handler within the qemu_driver code. > > #1 checks for the cpu-model-expansion capability before > executing the baseline handler since it is used for feature expansion. > > #2 fixes a styling where a < 0 condition was missing from one of the > if (function()) lines for consistency's sake. > > #3 will check if the cpu definition(s) are valid and contain a model > name. > > #4 checks the cpu definition(s) model names against the model names > known by the hypervisor. This patch must come before #5. > > #5 will allow the baseline command to be ran with a single cpu > definition, whereas before the command would simply fail with an > unhelpful error message. A CPU model expansion will be performed in > this case, which will produce the same result as if the model were > actually baselined. > > Note: without patch #4, #5 can result in a segfault in the case where > a single CPU model is provided and the model is not recognized by the > hypervisor. This is because cpu model expansion will return 0 and the > result will be NULL. > > Since the QMP response returns "GenericError" in the case of a missing > CPU model, or if the command is not supported (and perhaps for other > reasons I am unsure of -- the response does not explicitly detail that > the CPU model provided was erroneous), we cannot rely on this > response always meaning there was a missing CPU model. > > So, to be safe-and-sure, the CPU model is checked against the list of > CPU models known to the hypervisor prior to baselining / expanding > (which were retrieved at some point previously during libvirt init). > > Collin Walling (5): > qemu: check for model-expansion cap before baselining > qemu: fix one instance of rc check styling in baseline > qemu: report error if missing model name when baselining > qemu: check if cpu model is supported before baselining > qemu: fix error message when baselining with a single cpu > > src/qemu/qemu_driver.c | 45 ++ > 1 file changed, 37 insertions(+), 8 deletions(-) > -- Regards, Collin Stay safe and stay healthy
Re: [PATCH] qemu: Add test cases for 'host_cdrom' blockdev backend via
On a Monday in 2020, Peter Krempa wrote: Simulate that the device is a cdrom when the path equals to /dev/cdrom to provide testing for the 'host_cdrom' backend. Signed-off-by: Peter Krempa --- I've already posted this before but I mistakenly identified is as wrong and self-NACKed it, but we don't really have a test for a backed by a host device, so this patch makes sense. tests/qemuxml2argvdata/disk-cdrom.args | 4 ++-- tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args | 4 ++-- tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args | 6 +++--- tests/qemuxml2argvdata/disk-cdrom.xml| 5 +++-- tests/qemuxml2argvtest.c | 11 +++ tests/qemuxml2xmloutdata/disk-cdrom.xml | 5 +++-- 6 files changed, 24 insertions(+), 11 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH] meson: remove non-pkg-config fallback for devmapper
On a Tuesday in 2020, Michal Prívozník wrote:
On 10/20/20 12:22 PM, Daniel P. Berrangé wrote:
The fallback for distros which lack pkg-config support for devmapper
references an undefined variable "tmp". It appears non of our supported
build platforms are triggering this bug and so the fallback code can be
removed entirely rather than fixed.
Signed-off-by: Daniel P. Berrangé
---
meson.build | 7 ---
1 file changed, 7 deletions(-)
diff --git a/meson.build b/meson.build
index a17d759702..c3ba34bbe0 100644
--- a/meson.build
+++ b/meson.build
@@ -1020,13 +1020,6 @@ endif
devmapper_version = '1.0.0'
devmapper_dep = dependency('devmapper', version: '>=' + devmapper_version,
required: false)
-if not devmapper_dep.found()
- # devmapper is missing pkg-config files in ubuntu, suse, etc
- devmapper_dep = cc.find_library('devmapper', required: false)
- if devmapper_dep.found() and not cc.has_function('dm_task_run',
dependencies: tmp)
-devmapper_dep = dependency('', required: false)
- endif
-endif
if devmapper_dep.found()
conf.set('WITH_DEVMAPPER', 1)
endif
Reviewed-by: Michal Privoznik
And I still owe us the complete drop of libdevmapper (used in
src/storage/storage_backend_mpath.c and parthelper).
You can file an issue for it, just in case some other brave soul wants
to volunteer for this task.
Jano
Michal
signature.asc
Description: PGP signature
Re: [PATCH 10/13] util: hash: Reimplement virHashTable using GHashTable
On Mon, Oct 26, 2020 at 04:45:50PM +0100, Peter Krempa wrote: > Glib's hash table provides basically the same functionality as our hash > table. > > In most cases the only thing that remains in the virHash* wrappers is > NULL-checks of '@table' argument as glib's hash functions don't tolerate > NULL. > > In case of iterators, we adapt the existing API of iterators to glibs to > prevent having rewrite all callers at this point. > > Signed-off-by: Peter Krempa > --- > src/libvirt_private.syms | 4 - > src/util/meson.build | 1 - > src/util/virhash.c | 416 ++- > src/util/virhash.h | 4 +- > src/util/virhashcode.c | 125 > src/util/virhashcode.h | 33 Our hash code impl uses Murmurhash which makes some efforts to be robust against malicious inputs triggering collisons, notably using a random seed. The new code uses g_str_hash which is much weaker, and the API docs explicitly recommend against using it if the input can be from an untrusted user. IOW, I don't think we should be removing our virhashcode impl. If anything we should upgrade our hash code impl to use SipHash which is used by perl, python, ruby, rust and more. 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: [PATCH 0/1] virt-aa-helper: allow hard links for mounts
On Thu, Oct 22, 2020 at 05:10:10PM +0200, Christian Schoenebeck wrote: > I'm suggesting to add the AppArmor permission "l" on libvirt export pathes > to allow guests creating hard links, which is currently a problem for 9pfs > mounts. > > Since the suggested patch would affect virtiofs as well, I would ask David > and Stefan for feedback. If necessary I would change the patch to exclude > virtiofs from this change. virtiofsd supports the link(2) operation and enabling it seems reasonable to me. It is enabled in non-AppArmor configuration. Stefan signature.asc Description: PGP signature
[PATCH 13/13] tests: Remove 'virhashtest'
There's no much sense to test the remnants of the functions which just
NULL-check prior to handing off to g_hash_table* functions.
Signed-off-by: Peter Krempa
---
tests/meson.build | 1 -
tests/virhashdata.h | 284 ---
tests/virhashtest.c | 539
3 files changed, 824 deletions(-)
delete mode 100644 tests/virhashdata.h
delete mode 100644 tests/virhashtest.c
diff --git a/tests/meson.build b/tests/meson.build
index 6984780066..8decdfa20c 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -310,7 +310,6 @@ tests += [
{ 'name': 'virfilecachetest' },
{ 'name': 'virfiletest' },
{ 'name': 'virfirewalltest' },
- { 'name': 'virhashtest' },
{ 'name': 'virhostcputest', 'link_whole': [ test_file_wrapper_lib ] },
{ 'name': 'virhostdevtest' },
{ 'name': 'viriscsitest' },
diff --git a/tests/virhashdata.h b/tests/virhashdata.h
deleted file mode 100644
index 1b457f2857..00
--- a/tests/virhashdata.h
+++ /dev/null
@@ -1,284 +0,0 @@
-/*
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library. If not, see
- *
[PATCH 12/13] util: hash: Add deprecation notices for functions which have g_hash_table replacements
For functions which have reasonable replacement, let's encourage usage of g_hash_table_ alternatives. Signed-off-by: Peter Krempa --- src/util/virhash.c | 31 ++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/util/virhash.c b/src/util/virhash.c index d943ca3b99..49e6e93f77 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -52,7 +52,7 @@ VIR_ONCE_GLOBAL_INIT(virHashAtomic); * virHashNew: * @dataFree: callback to free data * - * Create a new GHashTable *. + * Create a new GHashTable * for use with string-based keys. * * Returns the newly created object. */ @@ -97,6 +97,8 @@ virHashAtomicDispose(void *obj) * * Free the hash @table and its contents. The userdata is * deallocated with function provided at creation time. + * + * Deprecated: consider using g_hash_table_unref instead */ void virHashFree(GHashTable *table) @@ -117,6 +119,10 @@ virHashFree(GHashTable *table) * Add the @userdata to the hash @table. This can later be retrieved * by using @name. Duplicate entries generate errors. * + * Deprecated: Consider using g_hash_table_insert insert. Note that + * g_hash_table_instead doesn't fail if entry exists. Also note that + * g_hash_table_insert doesn't copy the key. + * * Returns 0 the addition succeeded and -1 in case of error. */ int @@ -146,6 +152,9 @@ virHashAddEntry(GHashTable *table, const char *name, void *userdata) * by using @name. Existing entry for this tuple * will be removed and freed with @f if found. * + * Deprecated: consider using g_hash_table_insert insert. Note that + * g_hash_table_insert doesn't copy the key. + * * Returns 0 the addition succeeded and -1 in case of error. */ int @@ -182,6 +191,8 @@ virHashAtomicUpdate(virHashAtomicPtr table, * * Find the userdata specified by @name * + * Deprecated: consider using g_hash_table_lookup instead + * * Returns a pointer to the userdata */ void * @@ -202,6 +213,8 @@ virHashLookup(GHashTable *table, * * Find whether entry specified by @name exists. * + * Deprecated: consider using g_hash_table_contains instead + * * Returns true if the entry exists and false otherwise */ bool @@ -223,6 +236,9 @@ virHashHasEntry(GHashTable *table, * Find the userdata specified by @name * and remove it from the hash without freeing it. * + * Deprecated: consider using g_hash_table_steal_extended once we upgrade to + * glib 2.58 + * * Returns a pointer to the userdata */ void *virHashSteal(GHashTable *table, const char *name) @@ -262,6 +278,8 @@ virHashAtomicSteal(virHashAtomicPtr table, * * Query the number of elements installed in the hash @table. * + * Deprecated: consider using g_hash_table_size instead + * * Returns the number of elements in the hash table or * -1 in case of error */ @@ -284,6 +302,8 @@ virHashSize(GHashTable *table) * it from the hash @table. Existing userdata for this tuple will be removed * and freed with @f. * + * Deprecated: consider using g_hash_table_remove + * * Returns 0 if the removal succeeded and -1 in case of error or not found. */ int @@ -325,6 +345,9 @@ virHashRemoveEntry(GHashTable *table, * If @iter fails and returns a negative value, the evaluation is stopped and -1 * is returned. * + * Deprecated: Consider using g_hash_table_foreach as replacement for + * virHashForEach, rewrite your code if it would require virHashForEachSafe. + * * Returns 0 on success or -1 on failure. */ int @@ -417,6 +440,8 @@ virHashSearcherWrapFunc(gpointer key, * will be removed from the hash table & its payload passed to the * data freer callback registered at creation. * + * Deprecated: consider using g_hash_table_foreach_remove instead + * * Returns number of items removed on success, -1 on failure */ ssize_t @@ -438,6 +463,8 @@ virHashRemoveSet(GHashTable *table, * * Free the hash @table's contents. The userdata is * deallocated with the function provided at creation time. + * + * Deprecated: consider using g_hash_table_remove_all instead */ void virHashRemoveAll(GHashTable *table) @@ -460,6 +487,8 @@ virHashRemoveAll(GHashTable *table) * returns non-zero will be returned by this function. * The elements are processed in a undefined order. Caller is * responsible for freeing the @name. + * + * Deprecated: consider using g_hash_table_find instead */ void *virHashSearch(GHashTable *table, virHashSearcher iter, -- 2.26.2
[PATCH 10/13] util: hash: Reimplement virHashTable using GHashTable
Glib's hash table provides basically the same functionality as our hash
table.
In most cases the only thing that remains in the virHash* wrappers is
NULL-checks of '@table' argument as glib's hash functions don't tolerate
NULL.
In case of iterators, we adapt the existing API of iterators to glibs to
prevent having rewrite all callers at this point.
Signed-off-by: Peter Krempa
---
src/libvirt_private.syms | 4 -
src/util/meson.build | 1 -
src/util/virhash.c | 416 ++-
src/util/virhash.h | 4 +-
src/util/virhashcode.c | 125
src/util/virhashcode.h | 33
6 files changed, 107 insertions(+), 476 deletions(-)
delete mode 100644 src/util/virhashcode.c
delete mode 100644 src/util/virhashcode.h
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f7b0d11ca2..9a5269fd0b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2219,10 +2219,6 @@ virHashSteal;
virHashUpdateEntry;
-# util/virhashcode.h
-virHashCodeGen;
-
-
# util/virhook.h
virHookCall;
virHookInitialize;
diff --git a/src/util/meson.build b/src/util/meson.build
index 3b711b1dc2..99f72cb04a 100644
--- a/src/util/meson.build
+++ b/src/util/meson.build
@@ -36,7 +36,6 @@ util_sources = [
'virgettext.c',
'virgic.c',
'virhash.c',
- 'virhashcode.c',
'virhook.c',
'virhostcpu.c',
'virhostmem.c',
diff --git a/src/util/virhash.c b/src/util/virhash.c
index a5ab48dbf5..ed658714e8 100644
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -21,42 +21,13 @@
#include "virerror.h"
#include "virhash.h"
-#include "viralloc.h"
#include "virlog.h"
-#include "virhashcode.h"
-#include "virrandom.h"
-#include "virstring.h"
#include "virobject.h"
#define VIR_FROM_THIS VIR_FROM_NONE
VIR_LOG_INIT("util.hash");
-#define MAX_HASH_LEN 8
-
-/* #define DEBUG_GROW */
-
-/*
- * A single entry in the hash table
- */
-typedef struct _virHashEntry virHashEntry;
-typedef virHashEntry *virHashEntryPtr;
-struct _virHashEntry {
-struct _virHashEntry *next;
-char *name;
-void *payload;
-};
-
-/*
- * The entire hash table
- */
-struct _virHashTable {
-virHashEntryPtr *table;
-uint32_t seed;
-size_t size;
-size_t nbElems;
-virHashDataFree dataFree;
-};
struct _virHashAtomic {
virObjectLockable parent;
@@ -76,13 +47,6 @@ static int virHashAtomicOnceInit(void)
VIR_ONCE_GLOBAL_INIT(virHashAtomic);
-static size_t
-virHashComputeKey(const virHashTable *table, const char *name)
-{
-uint32_t value = virHashCodeGen(name, strlen(name), table->seed);
-return value % table->size;
-}
-
/**
* virHashNew:
@@ -95,18 +59,7 @@ virHashComputeKey(const virHashTable *table, const char
*name)
virHashTablePtr
virHashNew(virHashDataFree dataFree)
{
-virHashTablePtr table = NULL;
-
-table = g_new0(virHashTable, 1);
-
-table->seed = virRandomBits(32);
-table->size = 32;
-table->nbElems = 0;
-table->dataFree = dataFree;
-
-table->table = g_new0(virHashEntryPtr, table->size);
-
-return table;
+return g_hash_table_new_full(g_str_hash, g_str_equal, g_free, dataFree);
}
@@ -138,66 +91,6 @@ virHashAtomicDispose(void *obj)
}
-/**
- * virHashGrow:
- * @table: the hash table
- * @size: the new size of the hash table
- *
- * resize the hash table
- *
- * Returns 0 in case of success, -1 in case of failure
- */
-static int
-virHashGrow(virHashTablePtr table, size_t size)
-{
-size_t oldsize, i;
-virHashEntryPtr *oldtable;
-
-#ifdef DEBUG_GROW
-size_t nbElem = 0;
-#endif
-
-if (table == NULL)
-return -1;
-if (size < 8)
-return -1;
-if (size > 8 * 2048)
-return -1;
-
-oldsize = table->size;
-oldtable = table->table;
-if (oldtable == NULL)
-return -1;
-
-table->table = g_new0(virHashEntryPtr, size);
-table->size = size;
-
-for (i = 0; i < oldsize; i++) {
-virHashEntryPtr iter = oldtable[i];
-while (iter) {
-virHashEntryPtr next = iter->next;
-size_t key = virHashComputeKey(table, iter->name);
-
-iter->next = table->table[key];
-table->table[key] = iter;
-
-#ifdef DEBUG_GROW
-nbElem++;
-#endif
-iter = next;
-}
-}
-
-VIR_FREE(oldtable);
-
-#ifdef DEBUG_GROW
-VIR_DEBUG("virHashGrow : from %d to %d, %ld elems", oldsize,
- size, nbElem);
-#endif
-
-return 0;
-}
-
/**
* virHashFree:
* @table: the hash table
@@ -208,76 +101,12 @@ virHashGrow(virHashTablePtr table, size_t size)
void
virHashFree(virHashTablePtr table)
{
-size_t i;
-
if (table == NULL)
return;
-for (i = 0; i < table->size; i++) {
-virHashEntryPtr iter = table->table[i];
-while (iter) {
-virHashEntryPtr next = iter->next;
-
-if (table->dataFree)
-table->dataFree(iter->payload);
-g_free(iter->name);
-VIR_FREE(iter);
[PATCH 02/13] util: hash: Rewrite sorting of elements in virHashGetItems
All but one of the callers either use the list in arbitrary order or
sorted by key. Rewrite the function so that it supports sorting by key
natively and make it return the element count. This in turn allows to
rewrite the only caller to sort by value internally.
This allows to remove multiple sorting functions which were sorting by
key and the function will be also later reused for some hash operations
internally.
Signed-off-by: Peter Krempa
---
src/conf/nwfilter_params.c| 10 +---
src/hyperv/hyperv_wmi.c | 2 +-
src/locking/lock_daemon.c | 2 +-
src/nwfilter/nwfilter_ebiptables_driver.c | 13 +++--
src/qemu/qemu_interop_config.c| 9 +---
src/rpc/virnetdaemon.c| 11 +---
src/util/virhash.c| 66 ++-
src/util/virhash.h| 5 +-
src/util/virlockspace.c | 2 +-
tests/virhashtest.c | 11 +---
10 files changed, 59 insertions(+), 72 deletions(-)
diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c
index 73160a38a4..dd2b92c97b 100644
--- a/src/conf/nwfilter_params.c
+++ b/src/conf/nwfilter_params.c
@@ -755,13 +755,6 @@ virNWFilterParseParamAttributes(xmlNodePtr cur)
}
-static int
-virNWFilterFormatParameterNameSorter(const virHashKeyValuePair *a,
- const virHashKeyValuePair *b)
-{
-return strcmp(a->key, b->key);
-}
-
int
virNWFilterFormatParamAttributes(virBufferPtr buf,
virHashTablePtr table,
@@ -779,8 +772,7 @@ virNWFilterFormatParamAttributes(virBufferPtr buf,
return -1;
}
-items = virHashGetItems(table,
-virNWFilterFormatParameterNameSorter);
+items = virHashGetItems(table, NULL, true);
if (!items)
return -1;
diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
index 421cbfa31b..3de7f0ed90 100644
--- a/src/hyperv/hyperv_wmi.c
+++ b/src/hyperv/hyperv_wmi.c
@@ -683,7 +683,7 @@ hypervSerializeEmbeddedParam(hypervParamPtr p, const char
*resourceUri,
/* retrieve parameters out of hash table */
numKeys = virHashSize(p->embedded.table);
-items = virHashGetItems(p->embedded.table, NULL);
+items = virHashGetItems(p->embedded.table, NULL, false);
if (!items) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Could not read embedded param hash table"));
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
index 8f16dfd064..021792cc69 100644
--- a/src/locking/lock_daemon.c
+++ b/src/locking/lock_daemon.c
@@ -730,7 +730,7 @@ virLockDaemonPreExecRestart(const char *state_file,
}
-tmp = pairs = virHashGetItems(lockDaemon->lockspaces, NULL);
+tmp = pairs = virHashGetItems(lockDaemon->lockspaces, NULL, false);
while (tmp && tmp->key) {
virLockSpacePtr lockspace = (virLockSpacePtr)tmp->value;
diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c
b/src/nwfilter/nwfilter_ebiptables_driver.c
index da6f88f135..8dfc870ab7 100644
--- a/src/nwfilter/nwfilter_ebiptables_driver.c
+++ b/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -3126,9 +3126,12 @@ virNWFilterRuleInstSortPtr(const void *a, const void *b)
static int
-ebiptablesFilterOrderSort(const virHashKeyValuePair *a,
- const virHashKeyValuePair *b)
+ebiptablesFilterOrderSort(const void *va,
+ const void *vb)
{
+const virHashKeyValuePair *a = va;
+const virHashKeyValuePair *b = vb;
+
/* elements' values has been limited to range [-1000, 1000] */
return *(virNWFilterChainPriority *)a->value -
*(virNWFilterChainPriority *)b->value;
@@ -3288,13 +3291,15 @@ ebtablesGetSubChainInsts(virHashTablePtr chains,
size_t *ninsts)
{
g_autofree virHashKeyValuePairPtr filter_names = NULL;
+size_t nfilter_names;
size_t i;
-filter_names = virHashGetItems(chains,
- ebiptablesFilterOrderSort);
+filter_names = virHashGetItems(chains, _names, false);
if (filter_names == NULL)
return -1;
+qsort(filter_names, nfilter_names, sizeof(*filter_names),
ebiptablesFilterOrderSort);
+
for (i = 0; filter_names[i].key; i++) {
g_autofree ebtablesSubChainInstPtr inst = NULL;
enum l3_proto_idx idx = ebtablesGetProtoIdxByFiltername(
diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c
index 53b251f056..e806d1e41f 100644
--- a/src/qemu/qemu_interop_config.c
+++ b/src/qemu/qemu_interop_config.c
@@ -83,13 +83,6 @@ qemuBuildFileList(virHashTablePtr files, const char *dir)
return ret;
}
-static int
-qemuConfigFilesSorter(const virHashKeyValuePair *a,
- const virHashKeyValuePair *b)
-{
-return strcmp(a->key, b->key);
-}
-
#define QEMU_SYSTEM_LOCATION PREFIX "/share/qemu"
#define QEMU_ETC_LOCATION
[PATCH 00/13] Replace virHashTable by GHashTable (for 6.10)
Our hash table API was surprisingly close to glibs. Peter Krempa (13): virhashtest: testHashGetItems: Remove test case for sorting by value util: hash: Rewrite sorting of elements in virHashGetItems util: virhash: Standardize on 'opaque' for opaque data util: hash: Introduce virHashForEachSorted Use virHashForEachSorted in tested code tests: remove virdeterministichashmock.so util: hash: Add delete-safe hash iterator util: hash: Use virHashForEachSafe in places which might delete the element util: hash: Don't use 'const' with virHashTablePtr util: hash: Reimplement virHashTable using GHashTable util: hash: Retire 'virHashTable' in favor of 'GHashTable' util: hash: Add deprecation notices for functions which have g_hash_table replacements tests: Remove 'virhashtest' src/conf/backup_conf.c| 2 +- src/conf/domain_addr.h| 2 +- src/conf/domain_conf.c| 12 +- src/conf/domain_conf.h| 2 +- src/conf/nwfilter_conf.h | 2 +- src/conf/nwfilter_ipaddrmap.c | 2 +- src/conf/nwfilter_params.c| 34 +- src/conf/nwfilter_params.h| 18 +- src/conf/snapshot_conf.c | 2 +- src/conf/virchrdev.c | 4 +- src/conf/virdomainmomentobjlist.c | 4 +- src/conf/virdomainobjlist.c | 6 +- src/conf/virinterfaceobj.c| 2 +- src/conf/virnetworkobj.c | 8 +- src/conf/virnodedeviceobj.c | 2 +- src/conf/virnwfilterbindingdef.h | 2 +- src/conf/virnwfilterbindingobjlist.c | 4 +- src/conf/virsecretobj.c | 2 +- src/conf/virstorageobj.c | 14 +- src/hyperv/hyperv_driver.c| 4 +- src/hyperv/hyperv_wmi.c | 18 +- src/hyperv/hyperv_wmi.h | 12 +- src/hypervisor/virclosecallbacks.c| 2 +- src/libvirt_private.syms | 6 +- src/libxl/libxl_logger.c | 2 +- src/locking/lock_daemon.c | 4 +- src/nwfilter/nwfilter_dhcpsnoop.c | 6 +- src/nwfilter/nwfilter_ebiptables_driver.c | 19 +- src/nwfilter/nwfilter_gentech_driver.c| 36 +- src/nwfilter/nwfilter_gentech_driver.h| 2 +- src/nwfilter/nwfilter_learnipaddr.c | 4 +- src/nwfilter/nwfilter_tech_driver.h | 2 +- src/qemu/qemu_agent.c | 4 +- src/qemu/qemu_backup.c| 14 +- src/qemu/qemu_backup.h| 2 +- src/qemu/qemu_block.c | 42 +- src/qemu/qemu_block.h | 18 +- src/qemu/qemu_blockjob.c | 6 +- src/qemu/qemu_capabilities.c | 8 +- src/qemu/qemu_checkpoint.c| 6 +- src/qemu/qemu_checkpoint.h| 2 +- src/qemu/qemu_conf.c | 4 +- src/qemu/qemu_conf.h | 2 +- src/qemu/qemu_domain.c| 8 +- src/qemu/qemu_domain.h| 2 +- src/qemu/qemu_driver.c| 24 +- src/qemu/qemu_interop_config.c| 13 +- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_migration_cookie.c | 2 +- src/qemu/qemu_monitor.c | 28 +- src/qemu/qemu_monitor.h | 20 +- src/qemu/qemu_monitor_json.c | 50 +- src/qemu/qemu_monitor_json.h | 22 +- src/qemu/qemu_process.c | 18 +- src/qemu/qemu_qapi.c | 14 +- src/qemu/qemu_qapi.h | 6 +- src/qemu/qemu_snapshot.c | 16 +- src/rpc/virnetdaemon.c| 13 +- src/security/security_selinux.c | 2 +- src/util/meson.build | 1 - src/util/virfilecache.c | 2 +- src/util/virhash.c| 569 +++--- src/util/virhash.h| 57 +- src/util/virhashcode.c| 125 src/util/virhashcode.h| 33 - src/util/viriptables.c| 4 +- src/util/virlockspace.c | 4 +- src/util/virmacmap.c | 4 +- src/util/virstoragefile.c | 8 +- src/util/virsystemd.c | 2 +- tests/meson.build | 2 - tests/nwfilterxml2firewalltest.c | 24 +- tests/qemublocktest.c | 26 +- tests/qemuhotplugtest.c | 6 +- tests/qemumigparamstest.c
[PATCH 07/13] util: hash: Add delete-safe hash iterator
'virHashForEach' historically allowed deletion of the current element as
'virHashRemoveSet' didn't exits. To prevent us from having to deepy
analyse all iterators add virHashForEachSafe which first gets a list of
elements and iterates them outside of the hash table.
This will allow replace the internals of the hash table with other
implementation which don't allow such operation.
Signed-off-by: Peter Krempa
---
src/libvirt_private.syms | 1 +
src/util/virhash.c | 26 +++---
src/util/virhash.h | 1 +
3 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1793b81ad9..f7b0d11ca2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2203,6 +2203,7 @@ virHashAtomicSteal;
virHashAtomicUpdate;
virHashEqual;
virHashForEach;
+virHashForEachSafe;
virHashForEachSorted;
virHashFree;
virHashGetItems;
diff --git a/src/util/virhash.c b/src/util/virhash.c
index f205291de9..e54052985f 100644
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -481,7 +481,7 @@ virHashRemoveEntry(virHashTablePtr table, const char *name)
/**
- * virHashForEach, virHashForEachSorted
+ * virHashForEach, virHashForEachSorted, virHashForEachSafe
* @table: the hash table to process
* @iter: callback to process each element
* @opaque: opaque data to pass to the iterator
@@ -490,14 +490,14 @@ virHashRemoveEntry(virHashTablePtr table, const char
*name)
*
* The elements are iterated in arbitrary order.
*
- * virHashForEach allows the callback to remove the current
+ * virHashForEach, virHashForEachSafe allow the callback to remove the current
* element using virHashRemoveEntry but calling other virHash* functions is
* prohibited. Note that removing the entry invalidates @key and @payload in
* the callback.
*
* virHashForEachSorted iterates the elements in order by sorted key.
*
- * virHashForEachSorted is more computationally
+ * virHashForEachSorted and virHashForEachSafe are more computationally
* expensive than virHashForEach.
*
* If @iter fails and returns a negative value, the evaluation is stopped and
-1
@@ -531,6 +531,26 @@ virHashForEach(virHashTablePtr table, virHashIterator
iter, void *opaque)
}
+int
+virHashForEachSafe(virHashTablePtr table,
+ virHashIterator iter,
+ void *opaque)
+{
+g_autofree virHashKeyValuePairPtr items = virHashGetItems(table, NULL,
false);
+size_t i;
+
+if (!items)
+return -1;
+
+for (i = 0; items[i].key; i++) {
+if (iter((void *)items[i].value, items[i].key, opaque) < 0)
+return -1;
+}
+
+return 0;
+}
+
+
int
virHashForEachSorted(virHashTablePtr table,
virHashIterator iter,
diff --git a/src/util/virhash.h b/src/util/virhash.h
index 1a59e9799d..b00ab0447e 100644
--- a/src/util/virhash.h
+++ b/src/util/virhash.h
@@ -136,6 +136,7 @@ bool virHashEqual(const virHashTable *table1,
* Iterators
*/
int virHashForEach(virHashTablePtr table, virHashIterator iter, void *opaque);
+int virHashForEachSafe(virHashTablePtr table, virHashIterator iter, void
*opaque);
int virHashForEachSorted(virHashTablePtr table, virHashIterator iter, void
*opaque);
ssize_t virHashRemoveSet(virHashTablePtr table, virHashSearcher iter, const
void *opaque);
void *virHashSearch(const virHashTable *table, virHashSearcher iter,
--
2.26.2
[PATCH 01/13] virhashtest: testHashGetItems: Remove test case for sorting by value
Upcoming patch will rewrite virHashGetItems to remove the sorting
function since the prevalent mode is to order by keys.
Remove the test for it.
Signed-off-by: Peter Krempa
---
tests/virhashtest.c | 19 ---
1 file changed, 19 deletions(-)
diff --git a/tests/virhashtest.c b/tests/virhashtest.c
index ad50aae003..8cc3109929 100644
--- a/tests/virhashtest.c
+++ b/tests/virhashtest.c
@@ -368,13 +368,6 @@ testHashGetItemsCompKey(const virHashKeyValuePair *a,
return strcmp(a->key, b->key);
}
-static int
-testHashGetItemsCompValue(const virHashKeyValuePair *a,
- const virHashKeyValuePair *b)
-{
-return strcmp(a->value, b->value);
-}
-
static int
testHashGetItems(const void *data G_GNUC_UNUSED)
{
@@ -416,18 +409,6 @@ testHashGetItems(const void *data G_GNUC_UNUSED)
}
VIR_FREE(array);
-if (!(array = virHashGetItems(hash, testHashGetItemsCompValue)) ||
-STRNEQ(array[0].key, "c") ||
-STRNEQ(array[0].value, "1") ||
-STRNEQ(array[1].key, "b") ||
-STRNEQ(array[1].value, "2") ||
-STRNEQ(array[2].key, "a") ||
-STRNEQ(array[2].value, "3") ||
-array[3].key || array[3].value) {
-VIR_TEST_VERBOSE("\nfailed to get items with value sort");
-goto cleanup;
-}
-
ret = 0;
cleanup:
--
2.26.2
[PATCH 09/13] util: hash: Don't use 'const' with virHashTablePtr
We didn't use it rigorously and some helpers even dasted it away. Remove
const from all hash utility functions.
Signed-off-by: Peter Krempa
---
src/conf/nwfilter_params.c | 2 +-
src/conf/nwfilter_params.h | 2 +-
src/util/virhash.c | 17 -
src/util/virhash.h | 12 ++--
4 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c
index dd2b92c97b..33bd7437f1 100644
--- a/src/conf/nwfilter_params.c
+++ b/src/conf/nwfilter_params.c
@@ -983,7 +983,7 @@ virNWFilterVarAccessGetIntIterId(const virNWFilterVarAccess
*vap)
bool
virNWFilterVarAccessIsAvailable(const virNWFilterVarAccess *varAccess,
-const virHashTable *hash)
+virHashTablePtr hash)
{
const char *varName = virNWFilterVarAccessGetVarName(varAccess);
const char *res;
diff --git a/src/conf/nwfilter_params.h b/src/conf/nwfilter_params.h
index 4962a86864..b91f830c31 100644
--- a/src/conf/nwfilter_params.h
+++ b/src/conf/nwfilter_params.h
@@ -119,7 +119,7 @@ virNWFilterVarAccessType virNWFilterVarAccessGetType(
unsigned int virNWFilterVarAccessGetIterId(const virNWFilterVarAccess *vap);
unsigned int virNWFilterVarAccessGetIndex(const virNWFilterVarAccess *vap);
bool virNWFilterVarAccessIsAvailable(const virNWFilterVarAccess *vap,
- const virHashTable *hash);
+ virHashTablePtr hash);
typedef struct _virNWFilterVarCombIterEntry virNWFilterVarCombIterEntry;
typedef virNWFilterVarCombIterEntry *virNWFilterVarCombIterEntryPtr;
diff --git a/src/util/virhash.c b/src/util/virhash.c
index 4f8df8fae3..a5ab48dbf5 100644
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -360,7 +360,8 @@ virHashGetEntry(const virHashTable *table,
* Returns a pointer to the userdata
*/
void *
-virHashLookup(const virHashTable *table, const char *name)
+virHashLookup(virHashTablePtr table,
+ const char *name)
{
virHashEntryPtr entry = virHashGetEntry(table, name);
@@ -381,7 +382,7 @@ virHashLookup(const virHashTable *table, const char *name)
* Returns true if the entry exists and false otherwise
*/
bool
-virHashHasEntry(const virHashTable *table,
+virHashHasEntry(virHashTablePtr table,
const char *name)
{
return !!virHashGetEntry(table, name);
@@ -434,7 +435,7 @@ virHashAtomicSteal(virHashAtomicPtr table,
* -1 in case of error
*/
ssize_t
-virHashSize(const virHashTable *table)
+virHashSize(virHashTablePtr table)
{
if (table == NULL)
return -1;
@@ -652,15 +653,13 @@ virHashRemoveAll(virHashTablePtr table)
* The elements are processed in a undefined order. Caller is
* responsible for freeing the @name.
*/
-void *virHashSearch(const virHashTable *ctable,
+void *virHashSearch(virHashTablePtr table,
virHashSearcher iter,
const void *opaque,
char **name)
{
size_t i;
-/* Cast away const for internal detection of misuse. */
-virHashTablePtr table = (virHashTablePtr)ctable;
if (table == NULL || iter == NULL)
return NULL;
@@ -739,7 +738,7 @@ virHashGetItems(virHashTablePtr table,
struct virHashEqualData
{
bool equal;
-const virHashTable *table2;
+virHashTablePtr table2;
virHashValueComparator compar;
};
@@ -760,8 +759,8 @@ static int virHashEqualSearcher(const void *payload, const
char *name,
return 0;
}
-bool virHashEqual(const virHashTable *table1,
- const virHashTable *table2,
+bool virHashEqual(virHashTablePtr table1,
+ virHashTablePtr table2,
virHashValueComparator compar)
{
struct virHashEqualData data = {
diff --git a/src/util/virhash.h b/src/util/virhash.h
index b00ab0447e..3af4786e9b 100644
--- a/src/util/virhash.h
+++ b/src/util/virhash.h
@@ -60,7 +60,7 @@ typedef int (*virHashSearcher) (const void *payload, const
char *name,
virHashTablePtr virHashNew(virHashDataFree dataFree);
virHashAtomicPtr virHashAtomicNew(virHashDataFree dataFree);
void virHashFree(virHashTablePtr table);
-ssize_t virHashSize(const virHashTable *table);
+ssize_t virHashSize(virHashTablePtr table);
/*
* Add a new entry to the hash table.
@@ -88,8 +88,8 @@ void virHashRemoveAll(virHashTablePtr table);
/*
* Retrieve the userdata.
*/
-void *virHashLookup(const virHashTable *table, const char *name);
-bool virHashHasEntry(const virHashTable *table, const char *name);
+void *virHashLookup(virHashTablePtr table, const char *name);
+bool virHashHasEntry(virHashTablePtr table, const char *name);
/*
* Retrieve & remove the userdata.
@@ -127,8 +127,8 @@ virHashKeyValuePairPtr virHashGetItems(virHashTablePtr
table,
* of two keys.
*/
typedef int (*virHashValueComparator)(const void *value1, const void *value2);
-bool virHashEqual(const virHashTable *table1,
- const
[PATCH 11/13] util: hash: Retire 'virHashTable' in favor of 'GHashTable'
Don't hide our use of GHashTable behind our typedef. This will also
promote the use of glibs hash function directly.
Signed-off-by: Peter Krempa
---
src/conf/backup_conf.c| 2 +-
src/conf/domain_addr.h| 2 +-
src/conf/domain_conf.c| 12 +++---
src/conf/domain_conf.h| 2 +-
src/conf/nwfilter_conf.h | 2 +-
src/conf/nwfilter_ipaddrmap.c | 2 +-
src/conf/nwfilter_params.c| 24 +--
src/conf/nwfilter_params.h| 18
src/conf/snapshot_conf.c | 2 +-
src/conf/virchrdev.c | 2 +-
src/conf/virdomainmomentobjlist.c | 2 +-
src/conf/virdomainobjlist.c | 4 +-
src/conf/virinterfaceobj.c| 2 +-
src/conf/virnetworkobj.c | 4 +-
src/conf/virnodedeviceobj.c | 2 +-
src/conf/virnwfilterbindingdef.h | 2 +-
src/conf/virnwfilterbindingobjlist.c | 2 +-
src/conf/virsecretobj.c | 2 +-
src/conf/virstorageobj.c | 10 ++---
src/hyperv/hyperv_driver.c| 4 +-
src/hyperv/hyperv_wmi.c | 16
src/hyperv/hyperv_wmi.h | 12 +++---
src/hypervisor/virclosecallbacks.c| 2 +-
src/libxl/libxl_logger.c | 2 +-
src/locking/lock_daemon.c | 2 +-
src/nwfilter/nwfilter_dhcpsnoop.c | 6 +--
src/nwfilter/nwfilter_ebiptables_driver.c | 6 +--
src/nwfilter/nwfilter_gentech_driver.c| 36
src/nwfilter/nwfilter_gentech_driver.h| 2 +-
src/nwfilter/nwfilter_learnipaddr.c | 4 +-
src/nwfilter/nwfilter_tech_driver.h | 2 +-
src/qemu/qemu_agent.c | 4 +-
src/qemu/qemu_backup.c| 14 +++
src/qemu/qemu_backup.h| 2 +-
src/qemu/qemu_block.c | 42 +--
src/qemu/qemu_block.h | 18
src/qemu/qemu_blockjob.c | 6 +--
src/qemu/qemu_capabilities.c | 8 ++--
src/qemu/qemu_checkpoint.c| 6 +--
src/qemu/qemu_checkpoint.h| 2 +-
src/qemu/qemu_conf.c | 4 +-
src/qemu/qemu_conf.h | 2 +-
src/qemu/qemu_domain.c| 2 +-
src/qemu/qemu_domain.h| 2 +-
src/qemu/qemu_driver.c| 24 +--
src/qemu/qemu_interop_config.c| 4 +-
src/qemu/qemu_migration.c | 2 +-
src/qemu/qemu_migration_cookie.c | 2 +-
src/qemu/qemu_monitor.c | 28 ++---
src/qemu/qemu_monitor.h | 20 -
src/qemu/qemu_monitor_json.c | 50 +++
src/qemu/qemu_monitor_json.h | 22 +-
src/qemu/qemu_process.c | 18
src/qemu/qemu_qapi.c | 14 +++
src/qemu/qemu_qapi.h | 6 +--
src/qemu/qemu_snapshot.c | 16
src/rpc/virnetdaemon.c| 2 +-
src/security/security_selinux.c | 2 +-
src/util/virfilecache.c | 2 +-
src/util/virhash.c| 42 +--
src/util/virhash.h| 44
src/util/viriptables.c| 4 +-
src/util/virlockspace.c | 2 +-
src/util/virmacmap.c | 2 +-
src/util/virstoragefile.c | 8 ++--
src/util/virsystemd.c | 2 +-
tests/nwfilterxml2firewalltest.c | 24 +--
tests/qemublocktest.c | 24 +--
tests/qemuhotplugtest.c | 6 +--
tests/qemumigparamstest.c | 4 +-
tests/qemumonitorjsontest.c | 26 ++--
tests/qemumonitortestutils.c | 6 +--
tests/qemumonitortestutils.h | 4 +-
tests/qemusecuritymock.c | 4 +-
tests/qemuxml2argvtest.c | 4 +-
tests/qemuxml2xmltest.c | 2 +-
tests/testutilsqemu.c | 6 +--
tests/testutilsqemu.h | 4 +-
tests/testutilsqemuschema.c | 10 ++---
tests/testutilsqemuschema.h | 8 ++--
tests/virhashtest.c | 31 +++---
81 files changed, 389 insertions(+), 394 deletions(-)
diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c
index 5c475239ad..ea812cc432 100644
--- a/src/conf/backup_conf.c
+++ b/src/conf/backup_conf.c
@@ -519,7 +519,7 @@ virDomainBackupAlignDisks(virDomainBackupDefPtr def,
virDomainDefPtr dom,
const char *suffix)
{
-g_autoptr(virHashTable) disks = virHashNew(NULL);
+g_autoptr(GHashTable) disks =
[PATCH 08/13] util: hash: Use virHashForEachSafe in places which might delete the element
Convert all calls to virHashForEach where it's not obvious that the
callback is _not_ deleting the current element from the hash to
virHashForEachSafe which will be deemed safe to do such operation.
Now that no iterator used with virHashForEach deletes current element we
can document that virHashForEach must not touch the hash table in any
way.
Signed-off-by: Peter Krempa
---
src/conf/virchrdev.c | 2 +-
src/conf/virdomainmomentobjlist.c| 2 +-
src/conf/virdomainobjlist.c | 2 +-
src/conf/virnetworkobj.c | 4 ++--
src/conf/virnwfilterbindingobjlist.c | 2 +-
src/conf/virstorageobj.c | 4 ++--
src/util/virhash.c | 4 +++-
tests/virhashtest.c | 2 +-
8 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c
index 5e5c03d03b..f35ae6a93e 100644
--- a/src/conf/virchrdev.c
+++ b/src/conf/virchrdev.c
@@ -299,7 +299,7 @@ void virChrdevFree(virChrdevsPtr devs)
return;
virMutexLock(>lock);
-virHashForEach(devs->hash, virChrdevFreeClearCallbacks, NULL);
+virHashForEachSafe(devs->hash, virChrdevFreeClearCallbacks, NULL);
virHashFree(devs->hash);
virMutexUnlock(>lock);
virMutexDestroy(>lock);
diff --git a/src/conf/virdomainmomentobjlist.c
b/src/conf/virdomainmomentobjlist.c
index 511bf1d415..5665819874 100644
--- a/src/conf/virdomainmomentobjlist.c
+++ b/src/conf/virdomainmomentobjlist.c
@@ -475,7 +475,7 @@ virDomainMomentForEach(virDomainMomentObjListPtr moments,
virHashIterator iter,
void *data)
{
-return virHashForEach(moments->objs, iter, data);
+return virHashForEachSafe(moments->objs, iter, data);
}
diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index e9a4b271df..6d8b1eebba 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -838,7 +838,7 @@ virDomainObjListForEach(virDomainObjListPtr doms,
virObjectRWLockWrite(doms);
else
virObjectRWLockRead(doms);
-virHashForEach(doms->objs, virDomainObjListHelper, );
+virHashForEachSafe(doms->objs, virDomainObjListHelper, );
virObjectRWUnlock(doms);
return data.ret;
}
diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index 46205b163c..6d2f4b6ec9 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -1471,7 +1471,7 @@ virNetworkObjListForEach(virNetworkObjListPtr nets,
struct virNetworkObjListForEachHelperData data = {
.callback = callback, .opaque = opaque, .ret = 0};
virObjectRWLockRead(nets);
-virHashForEach(nets->objs, virNetworkObjListForEachHelper, );
+virHashForEachSafe(nets->objs, virNetworkObjListForEachHelper, );
virObjectRWUnlock(nets);
return data.ret;
}
@@ -1851,7 +1851,7 @@ virNetworkObjPortForEach(virNetworkObjPtr obj,
void *opaque)
{
virNetworkObjPortListForEachData data = { iter, opaque, false };
-virHashForEach(obj->ports, virNetworkObjPortForEachCallback, );
+virHashForEachSafe(obj->ports, virNetworkObjPortForEachCallback, );
if (data.err)
return -1;
return 0;
diff --git a/src/conf/virnwfilterbindingobjlist.c
b/src/conf/virnwfilterbindingobjlist.c
index 4cbb62abfa..ffcf8faf3a 100644
--- a/src/conf/virnwfilterbindingobjlist.c
+++ b/src/conf/virnwfilterbindingobjlist.c
@@ -365,7 +365,7 @@
virNWFilterBindingObjListForEach(virNWFilterBindingObjListPtr bindings,
callback, opaque, 0,
};
virObjectRWLockRead(bindings);
-virHashForEach(bindings->objs, virNWFilterBindingObjListHelper, );
+virHashForEachSafe(bindings->objs, virNWFilterBindingObjListHelper, );
virObjectRWUnlock(bindings);
return data.ret;
}
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 219594582c..ada53f2e8c 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -465,7 +465,7 @@ virStoragePoolObjListForEach(virStoragePoolObjListPtr pools,
struct _virStoragePoolObjListForEachData data = { .iter = iter,
.opaque = opaque };
-virHashForEach(pools->objs, virStoragePoolObjListForEachCb, );
+virHashForEachSafe(pools->objs, virStoragePoolObjListForEachCb, );
}
@@ -753,7 +753,7 @@ virStoragePoolObjForEachVolume(virStoragePoolObjPtr obj,
.iter = iter, .opaque = opaque };
virObjectRWLockRead(obj->volumes);
-virHashForEach(obj->volumes->objsKey, virStoragePoolObjForEachVolumeCb,
+virHashForEachSafe(obj->volumes->objsKey, virStoragePoolObjForEachVolumeCb,
);
virObjectRWUnlock(obj->volumes);
return 0;
diff --git a/src/util/virhash.c b/src/util/virhash.c
index e54052985f..4f8df8fae3 100644
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -490,7 +490,9 @@ virHashRemoveEntry(virHashTablePtr table, const char *name)
*
* The elements are iterated in
[PATCH 05/13] Use virHashForEachSorted in tested code
The simplest way to write tests is to check the output against expected output, but we must ensure that the output is stable. We can use virHashForEachSorted as a hash iterator to ensure stable ordering. This patch fixes 3 instances of hash iteration which is tested in various parts, including test output changes in appropriate places. Signed-off-by: Peter Krempa --- src/qemu/qemu_domain.c| 6 +- src/util/virmacmap.c | 2 +- tests/qemumonitorjsontest.c | 2 +- .../blockjob-blockdev-in.xml | 116 +- 4 files changed, 63 insertions(+), 63 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d7dbca487a..e770940aca 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2223,9 +2223,9 @@ qemuDomainObjPrivateXMLFormatBlockjobs(virBufferPtr buf, virTristateBoolTypeToString(virTristateBoolFromBool(bj))); if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV) && -virHashForEach(priv->blockjobs, - qemuDomainObjPrivateXMLFormatBlockjobIterator, - ) < 0) +virHashForEachSorted(priv->blockjobs, + qemuDomainObjPrivateXMLFormatBlockjobIterator, + ) < 0) return -1; virXMLFormatElement(buf, "blockjobs", , ); diff --git a/src/util/virmacmap.c b/src/util/virmacmap.c index 2d203e72af..fe71b06dd7 100644 --- a/src/util/virmacmap.c +++ b/src/util/virmacmap.c @@ -244,7 +244,7 @@ virMacMapDumpStrLocked(virMacMapPtr mgr, arr = virJSONValueNewArray(); -if (virHashForEach(mgr->macs, virMACMapHashDumper, arr) < 0) +if (virHashForEachSorted(mgr->macs, virMACMapHashDumper, arr) < 0) goto cleanup; if (!(*str = virJSONValueToString(arr, true))) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 61935134af..d7c9af7e88 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2792,7 +2792,7 @@ testBlockNodeNameDetect(const void *opaque) blockstatsJson))) goto cleanup; -virHashForEach(nodedata, testBlockNodeNameDetectFormat, ); +virHashForEachSorted(nodedata, testBlockNodeNameDetectFormat, ); virBufferTrim(, "\n"); diff --git a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml index 8ffc91bdcb..c70742418b 100644 --- a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml +++ b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml @@ -234,9 +234,12 @@ - - - + + + + + + @@ -244,9 +247,61 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -284,61 +339,6 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -2 -- 2.26.2
[PATCH 06/13] tests: remove virdeterministichashmock.so
Code which is sensitive to ordering now uses deterministic iterator
functions, so we can remove the mock override.
Signed-off-by: Peter Krempa
---
tests/meson.build| 1 -
tests/qemublocktest.c| 2 +-
tests/qemumonitorjsontest.c | 2 +-
tests/qemuxml2xmltest.c | 3 +--
tests/virdeterministichashmock.c | 36
tests/virmacmaptest.c| 2 +-
6 files changed, 4 insertions(+), 42 deletions(-)
delete mode 100644 tests/virdeterministichashmock.c
diff --git a/tests/meson.build b/tests/meson.build
index a59002c083..6984780066 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -56,7 +56,6 @@ mock_libs = [
{ 'name': 'domaincapsmock' },
{ 'name': 'shunload', 'sources': [ 'shunloadhelper.c' ] },
{ 'name': 'vircgroupmock' },
- { 'name': 'virdeterministichashmock' },
{ 'name': 'virfilecachemock' },
{ 'name': 'virgdbusmock' },
{ 'name': 'virhostcpumock' },
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index c39f96716f..70dce54d0e 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -1385,4 +1385,4 @@ mymain(void)
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
-VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virdeterministichash"))
+VIR_TEST_MAIN(mymain)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index d7c9af7e88..ec4199de68 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -3479,4 +3479,4 @@ mymain(void)
return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
}
-VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virdeterministichash"))
+VIR_TEST_MAIN(mymain)
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 44ac9fbce7..3e56d0fc50 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1528,8 +1528,7 @@ mymain(void)
VIR_TEST_MAIN_PRELOAD(mymain,
VIR_TEST_MOCK("virpci"),
VIR_TEST_MOCK("virrandom"),
- VIR_TEST_MOCK("domaincaps"),
- VIR_TEST_MOCK("virdeterministichash"))
+ VIR_TEST_MOCK("domaincaps"))
#else
diff --git a/tests/virdeterministichashmock.c b/tests/virdeterministichashmock.c
deleted file mode 100644
index 4d0c88f600..00
--- a/tests/virdeterministichashmock.c
+++ /dev/null
@@ -1,36 +0,0 @@
-/*
- * Copyright (C) 2016 Red Hat, Inc.
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library. If not, see
- *
[PATCH 03/13] util: virhash: Standardize on 'opaque' for opaque data
Rename 'data' argument which is used for opaque data.
Signed-off-by: Peter Krempa
---
src/util/virhash.c | 24
src/util/virhash.h | 14 +++---
2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/src/util/virhash.c b/src/util/virhash.c
index 2a27ce8de6..f644990712 100644
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -484,7 +484,7 @@ virHashRemoveEntry(virHashTablePtr table, const char *name)
* virHashForEach
* @table: the hash table to process
* @iter: callback to process each element
- * @data: opaque data to pass to the iterator
+ * @opaque: opaque data to pass to the iterator
*
* Iterates over every element in the hash table, invoking the
* 'iter' callback. The callback is allowed to remove the current element
@@ -495,7 +495,7 @@ virHashRemoveEntry(virHashTablePtr table, const char *name)
* Returns 0 on success or -1 on failure.
*/
int
-virHashForEach(virHashTablePtr table, virHashIterator iter, void *data)
+virHashForEach(virHashTablePtr table, virHashIterator iter, void *opaque)
{
size_t i;
int ret = -1;
@@ -507,7 +507,7 @@ virHashForEach(virHashTablePtr table, virHashIterator iter,
void *data)
virHashEntryPtr entry = table->table[i];
while (entry) {
virHashEntryPtr next = entry->next;
-ret = iter(entry->payload, entry->name, data);
+ret = iter(entry->payload, entry->name, opaque);
if (ret < 0)
return ret;
@@ -524,7 +524,7 @@ virHashForEach(virHashTablePtr table, virHashIterator iter,
void *data)
* virHashRemoveSet
* @table: the hash table to process
* @iter: callback to identify elements for removal
- * @data: opaque data to pass to the iterator
+ * @opaque: opaque data to pass to the iterator
*
* Iterates over all elements in the hash table, invoking the 'iter'
* callback. If the callback returns a non-zero value, the element
@@ -536,7 +536,7 @@ virHashForEach(virHashTablePtr table, virHashIterator iter,
void *data)
ssize_t
virHashRemoveSet(virHashTablePtr table,
virHashSearcher iter,
- const void *data)
+ const void *opaque)
{
size_t i, count = 0;
@@ -548,7 +548,7 @@ virHashRemoveSet(virHashTablePtr table,
while (*nextptr) {
virHashEntryPtr entry = *nextptr;
-if (!iter(entry->payload, entry->name, data)) {
+if (!iter(entry->payload, entry->name, opaque)) {
nextptr = >next;
} else {
count++;
@@ -568,7 +568,7 @@ virHashRemoveSet(virHashTablePtr table,
static int
_virHashRemoveAllIter(const void *payload G_GNUC_UNUSED,
const char *name G_GNUC_UNUSED,
- const void *data G_GNUC_UNUSED)
+ const void *opaque G_GNUC_UNUSED)
{
return 1;
}
@@ -590,7 +590,7 @@ virHashRemoveAll(virHashTablePtr table)
* virHashSearch:
* @table: the hash table to search
* @iter: an iterator to identify the desired element
- * @data: extra opaque information passed to the iter
+ * @opaque: extra opaque information passed to the iter
* @name: the name of found user data, pass NULL to ignore
*
* Iterates over the hash table calling the 'iter' callback
@@ -601,7 +601,7 @@ virHashRemoveAll(virHashTablePtr table)
*/
void *virHashSearch(const virHashTable *ctable,
virHashSearcher iter,
-const void *data,
+const void *opaque,
char **name)
{
size_t i;
@@ -615,7 +615,7 @@ void *virHashSearch(const virHashTable *ctable,
for (i = 0; i < table->size; i++) {
virHashEntryPtr entry;
for (entry = table->table[i]; entry; entry = entry->next) {
-if (iter(entry->payload, entry->name, data)) {
+if (iter(entry->payload, entry->name, opaque)) {
if (name)
*name = g_strdup(entry->name);
return entry->payload;
@@ -691,9 +691,9 @@ struct virHashEqualData
};
static int virHashEqualSearcher(const void *payload, const char *name,
-const void *data)
+const void *opaque)
{
-struct virHashEqualData *vhed = (void *)data;
+struct virHashEqualData *vhed = (void *)opaque;
const void *value;
value = virHashLookup(vhed->table2, name);
diff --git a/src/util/virhash.h b/src/util/virhash.h
index d8b31e43a1..688e1adf4d 100644
--- a/src/util/virhash.h
+++ b/src/util/virhash.h
@@ -34,25 +34,25 @@ typedef void (*virHashDataFree) (void *payload);
* virHashIterator:
* @payload: the data in the hash
* @name: the hash key
- * @data: user supplied data blob
+ * @opaque: user supplied data blob
*
* Callback to process a hash entry during iteration
*
* Returns -1 to stop the iteration, e.g. in case of an error
*/
-typedef int (*virHashIterator) (void *payload, const char
[PATCH 04/13] util: hash: Introduce virHashForEachSorted
Iterate the hash elements sorted by key. This is useful to provide a
stable ordering such as in cases when the output is checked in tests.
Signed-off-by: Peter Krempa
---
src/libvirt_private.syms | 1 +
src/util/virhash.c | 39 +++
src/util/virhash.h | 1 +
3 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 95e50835ad..1793b81ad9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2203,6 +2203,7 @@ virHashAtomicSteal;
virHashAtomicUpdate;
virHashEqual;
virHashForEach;
+virHashForEachSorted;
virHashFree;
virHashGetItems;
virHashHasEntry;
diff --git a/src/util/virhash.c b/src/util/virhash.c
index f644990712..f205291de9 100644
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -481,14 +481,25 @@ virHashRemoveEntry(virHashTablePtr table, const char
*name)
/**
- * virHashForEach
+ * virHashForEach, virHashForEachSorted
* @table: the hash table to process
* @iter: callback to process each element
* @opaque: opaque data to pass to the iterator
*
- * Iterates over every element in the hash table, invoking the
- * 'iter' callback. The callback is allowed to remove the current element
- * using virHashRemoveEntry but calling other virHash* functions is prohibited.
+ * Iterates over every element in the hash table, invoking the 'iter' callback.
+ *
+ * The elements are iterated in arbitrary order.
+ *
+ * virHashForEach allows the callback to remove the current
+ * element using virHashRemoveEntry but calling other virHash* functions is
+ * prohibited. Note that removing the entry invalidates @key and @payload in
+ * the callback.
+ *
+ * virHashForEachSorted iterates the elements in order by sorted key.
+ *
+ * virHashForEachSorted is more computationally
+ * expensive than virHashForEach.
+ *
* If @iter fails and returns a negative value, the evaluation is stopped and
-1
* is returned.
*
@@ -520,6 +531,26 @@ virHashForEach(virHashTablePtr table, virHashIterator
iter, void *opaque)
}
+int
+virHashForEachSorted(virHashTablePtr table,
+ virHashIterator iter,
+ void *opaque)
+{
+g_autofree virHashKeyValuePairPtr items = virHashGetItems(table, NULL,
true);
+size_t i;
+
+if (!items)
+return -1;
+
+for (i = 0; items[i].key; i++) {
+if (iter((void *)items[i].value, items[i].key, opaque) < 0)
+return -1;
+}
+
+return 0;
+}
+
+
/**
* virHashRemoveSet
* @table: the hash table to process
diff --git a/src/util/virhash.h b/src/util/virhash.h
index 688e1adf4d..1a59e9799d 100644
--- a/src/util/virhash.h
+++ b/src/util/virhash.h
@@ -136,6 +136,7 @@ bool virHashEqual(const virHashTable *table1,
* Iterators
*/
int virHashForEach(virHashTablePtr table, virHashIterator iter, void *opaque);
+int virHashForEachSorted(virHashTablePtr table, virHashIterator iter, void
*opaque);
ssize_t virHashRemoveSet(virHashTablePtr table, virHashSearcher iter, const
void *opaque);
void *virHashSearch(const virHashTable *table, virHashSearcher iter,
const void *opaque, char **name);
--
2.26.2
Re: [PATCH 2/3] qemu: Update to new design of 'block-export-add'
On 10/26/20 8:19 AM, Peter Krempa wrote:
> qemu decided to modify the arguments of 'block-export-add' to include an
> array of bitmaps rather than a single bitmap.
>
> Since we've added the code prior to qemu setting the interface in stone
> and thus it will be changed incompatibly and we already have tests for
> the new interface we need to update the code and qemu capabilities data
> at the same time.
>
> Use a array of bitmaps as the 'bitmaps' argument instead of 'bitmap' and
> bump qemu capabilities for the upcoming 5.2.0 release to commit TBD
The TBD proves that qemu is still in a state of flux; I'll be posting v6
of those patches today, but if they make it into soft freeze, we have
something to use. Whether that timing then works for you to get it into
this month's libvirt release, or whether you defer this patch to next
month, I think we'll be okay (the key part is that we avoid a libvirt
release targetting an unreleased unstable qemu interface, which patch
1/3 accomplished).
> +++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.replies
> @@ -21,7 +21,7 @@
>"minor": 1,
>"major": 5
> },
> -"package": "v5.1.0-2207-g96292515c0"
> +"package": "v5.1.0-2477-gbb564b7029-dirty"
This part is fishy; but I presume it will be cleaned up when you resolve
the TBD in the commit message.
> +++ b/tests/qemumonitorjsontest.c
> @@ -3052,11 +3052,12 @@ testQemuMonitorJSONBlockExportAdd(const void *opaque)
> const testGenericData *data = opaque;
> g_autoptr(qemuMonitorTest) test = NULL;
> g_autoptr(virJSONValue) nbddata = NULL;
> +const char *bitmaps[] = { "bitmap1", "bitmap2", NULL };
>
> if (!(test = qemuMonitorTestNewSchema(data->xmlopt, data->schema)))
> return -1;
>
> -if (!(nbddata = qemuBlockExportGetNBDProps("nodename", "exportname",
> true, "bitmapname")))
> +if (!(nbddata = qemuBlockExportGetNBDProps("nodename", "exportname",
> true, bitmaps)))
> return -1;
>
> if (qemuMonitorTestAddItem(test, "block-export-add", "{\"return\":{}}")
> < 0)
>
Still waiting on qemu, but once that is in, the code changes plus your
respin to fix the commit message and testsuite are good enough to add:
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[libvirt PATCH] NEWS: Fix vertical spacing between sections
Looking at the entire repository reveals we're not too consistent about this, but at least in this specific document we mostly have two blank lines between sections, so let's stick with that. Signed-off-by: Andrea Bolognani --- Pushed as trivial. NEWS.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 6b7735b9a0..3587bc2c13 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -46,6 +46,7 @@ v6.9.0 (unreleased) Relying on the "Description" field caused queries to fail on non-"en-US" systems. The queries have been updated to avoid using localized strings. + v6.8.0 (2020-10-01) === @@ -143,6 +144,7 @@ v6.8.0 (2020-10-01) in libvirt. udev backend is used on Linux OSes and devd can be eventually implemented as replacement for FreeBSD. + v6.7.0 (2020-09-01) === -- 2.26.2
[PATCH] qemu: Add test cases for 'host_cdrom' blockdev backend via
Simulate that the device is a cdrom when the path equals to /dev/cdrom
to provide testing for the 'host_cdrom' backend.
Signed-off-by: Peter Krempa
---
I've already posted this before but I mistakenly identified is as wrong
and self-NACKed it, but we don't really have a test for a backed by a host device, so this patch makes sense.
tests/qemuxml2argvdata/disk-cdrom.args | 4 ++--
tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args | 4 ++--
tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args | 6 +++---
tests/qemuxml2argvdata/disk-cdrom.xml| 5 +++--
tests/qemuxml2argvtest.c | 11 +++
tests/qemuxml2xmloutdata/disk-cdrom.xml | 5 +++--
6 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/tests/qemuxml2argvdata/disk-cdrom.args
b/tests/qemuxml2argvdata/disk-cdrom.args
index cbac368129..e506a4befe 100644
--- a/tests/qemuxml2argvdata/disk-cdrom.args
+++ b/tests/qemuxml2argvdata/disk-cdrom.args
@@ -25,8 +25,8 @@ server,nowait \
-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 \
+-drive file=/dev/cdrom,format=raw,if=none,id=drive-ide0-0-0,readonly=on \
+-device ide-cd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
-drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,readonly=on \
-device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \
-drive if=none,id=drive-ide0-1-0,readonly=on \
diff --git a/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args
b/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args
index 2440acc78a..0621746a3b 100644
--- a/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args
+++ b/tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args
@@ -27,8 +27,8 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
-no-acpi \
-boot strict=on \
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
--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 \
+-drive file=/dev/cdrom,format=raw,if=none,id=drive-ide0-0-0,readonly=on \
+-device ide-cd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
-drive file=/root/boot.iso,format=raw,if=none,id=drive-ide0-0-1,readonly=on \
-device ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \
-drive if=none,id=drive-ide0-1-0,readonly=on \
diff --git a/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args
b/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args
index 0d375ffd1c..beac75ec1d 100644
--- a/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-cdrom.x86_64-latest.args
@@ -29,11 +29,11 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
-no-acpi \
-boot strict=on \
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
--blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1",\
+-blockdev '{"driver":"host_cdrom","filename":"/dev/cdrom",\
"node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}' \
--blockdev '{"node-name":"libvirt-4-format","read-only":false,"driver":"raw",\
+-blockdev '{"node-name":"libvirt-4-format","read-only":true,"driver":"raw",\
"file":"libvirt-4-storage"}' \
--device ide-hd,bus=ide.0,unit=0,drive=libvirt-4-format,id=ide0-0-0,bootindex=1
\
+-device ide-cd,bus=ide.0,unit=0,drive=libvirt-4-format,id=ide0-0-0 \
-blockdev '{"driver":"file","filename":"/root/boot.iso",\
"node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}' \
-blockdev '{"node-name":"libvirt-3-format","read-only":true,"driver":"raw",\
diff --git a/tests/qemuxml2argvdata/disk-cdrom.xml
b/tests/qemuxml2argvdata/disk-cdrom.xml
index b051b16642..54d2927ac3 100644
--- a/tests/qemuxml2argvdata/disk-cdrom.xml
+++ b/tests/qemuxml2argvdata/disk-cdrom.xml
@@ -14,10 +14,11 @@
destroy
/usr/bin/qemu-system-i386
-
+
-
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c5a0095e0d..ef8a871a19 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -407,6 +407,17 @@ testCompareXMLToArgvCreateArgs(virQEMUDriverPtr drv,
VIR_QEMU_PROCESS_START_COLD) < 0)
return NULL;
+for (i = 0; i < vm->def->ndisks; i++) {
+virDomainDiskDefPtr disk = vm->def->disks[i];
+
+/* host cdrom requires special treatment in qemu, mock it */
+if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM &&
+disk->src->format == VIR_STORAGE_FILE_RAW &&
+virStorageSourceIsBlockLocal(disk->src) &&
+STREQ(disk->src->path, "/dev/cdrom"))
+disk->src->hostcdrom = true;
+}
+
for (i = 0; i < vm->def->nhostdevs; i++) {
virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i];
diff --git
Re: [PATCH 3/3] qemu: capabilities: Re-enable detection of QEMU_CAPS_BLOCK_EXPORT_ADD
On 10/26/20 8:19 AM, Peter Krempa wrote: > Now that qemu stabilized it's interface and we've switched to the new > design we can re-enable use of 'block-export-add' > > This reverts commit TBD!! Cute - but it works! Obviously, as with 2/3, you'll add S-o-b when the commit message is fixed. Code-wise: Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 1/3] qemu: capabilities: Disable detection of QEMU_CAPS_BLOCK_EXPORT_ADD
On 10/26/20 8:19 AM, Peter Krempa wrote: > We use the capability to switch to using 'block-export-add' in the > upcoming qemu release instead of the at the same time deprecated > 'nbd-server-add'. > > Unfortunately qemu wants to change the interface of 'block-export-add' > before the release. Since we've tried to stay up to date and added the > code before it was written in stone, we need to disable the use of the > new interface for the upcoming libvirt release so that we don't have a > version of libvirt which would not work with the upcoming qemu version. > > Remove the detection of 'block-export-add' until we are more sure how > the qemu interface will look. > > This patch partially reverts commit adb9f7123adb94645 > > Signed-off-by: Peter Krempa > --- > src/qemu/qemu_capabilities.c | 1 - > tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 - > 2 files changed, 2 deletions(-) Reviewed-by: Eric Blake Safe for libvirt now no matter what happens in qemu's soft freeze this week. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
[PATCH] spec: keep existing nwfilters uuid on update
Now on every nwfilter config package update we overwrite existing filters
entirely. It is desired to bring new version of filters on update but we'd
better keep their uuids I guess.
Actually patch primarily address noise in logs on update. If both libvirtd and
firewalld are running and libvirt is using firewalld backend then on firewalld
restart we reload all nwfilters. So if node is updated and we have update for
both firewalld and libvirt then in the process of update first new nwfilters of
libvirt package are copied to /etc/libvirt/nwfilters then firewalld is
restarted and then libvirtd is restarted. In this process firewalld restart
cause log messages like [1]. The issue is libvirt brings nwfilters without
in definition and on handling firewalld restart libvirt generates
missing uuid and then fail to update filter definition because it is already
present in filters list with different uuid.
[1] virNWFilterObjListAssignDef:337 : operation failed: filter 'no-ip-spoofing'
already exists with uuid c302edf9-8a48-40d8-a652-f70b2c563ad1
Signed-off-by: Nikolay Shirokovskiy
---
libvirt.spec.in | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 2a4324b..6a31440 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1438,7 +1438,18 @@ fi
rm -rf %{_localstatedir}/lib/rpm-state/libvirt || :
%post daemon-config-nwfilter
-cp %{_datadir}/libvirt/nwfilter/*.xml %{_sysconfdir}/libvirt/nwfilter/
+# keep existing filters uuid on update
+for dfile in %{_datadir}/libvirt/nwfilter/*.xml; do
+sfile=%{_sysconfdir}/libvirt/nwfilter/`basename $dfile`
+if [ -f "$sfile" ]; then
+ uuidstr=`sed -n '/.*<\/uuid>/p' "$sfile"`
+ if [ ! -z "$uuidstr" ]; then
+sed -e "s,,&\n$uuidstr," "$dfile" > "$sfile"
+continue
+ fi
+fi
+cp "$dfile" "$sfile"
+done
# libvirt saves these files with mode 600
chmod 600 %{_sysconfdir}/libvirt/nwfilter/*.xml
# Make sure libvirt picks up the new nwfilter defininitons
--
1.8.3.1
Re: [PATCH 2/3] qemu: Update to new design of 'block-export-add'
On Mon, Oct 26, 2020 at 14:19:29 +0100, Peter Krempa wrote: > qemu decided to modify the arguments of 'block-export-add' to include an > array of bitmaps rather than a single bitmap. > > Since we've added the code prior to qemu setting the interface in stone > and thus it will be changed incompatibly and we already have tests for > the new interface we need to update the code and qemu capabilities data > at the same time. > > Use a array of bitmaps as the 'bitmaps' argument instead of 'bitmap' and > bump qemu capabilities for the upcoming 5.2.0 release to commit TBD > --- Note that sign-off will be added after I capture the capabilities after the patches will be pushed to upstream qemu.
[PATCH 2/3] qemu: Update to new design of 'block-export-add'
qemu decided to modify the arguments of 'block-export-add' to include an
array of bitmaps rather than a single bitmap.
Since we've added the code prior to qemu setting the interface in stone
and thus it will be changed incompatibly and we already have tests for
the new interface we need to update the code and qemu capabilities data
at the same time.
Use a array of bitmaps as the 'bitmaps' argument instead of 'bitmap' and
bump qemu capabilities for the upcoming 5.2.0 release to commit TBD
---
src/qemu/qemu_block.c | 17 +-
src/qemu/qemu_block.h |2 +-
.../caps_5.2.0.x86_64.replies | 2377 +
.../caps_5.2.0.x86_64.xml |2 +-
tests/qemumonitorjsontest.c |3 +-
5 files changed, 1339 insertions(+), 1062 deletions(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 0c2710a7c6..eb5f625b98 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -3369,20 +3369,30 @@ virJSONValuePtr
qemuBlockExportGetNBDProps(const char *nodename,
const char *exportname,
bool writable,
- const char *bitmap)
+ const char **bitmaps)
{
g_autofree char *exportid = NULL;
+g_autoptr(virJSONValue) bitmapsarr = NULL;
virJSONValuePtr ret = NULL;
exportid = g_strdup_printf("libvirt-nbd-%s", nodename);
+if (bitmaps && *bitmaps) {
+bitmapsarr = virJSONValueNewArray();
+
+while (*bitmaps) {
+if (virJSONValueArrayAppendString(bitmapsarr, *(bitmaps++)) < 0)
+return NULL;
+}
+}
+
if (virJSONValueObjectCreate(,
"s:type", "nbd",
"s:id", exportid,
"s:node-name", nodename,
"b:writable", writable,
"s:name", exportname,
- "S:bitmap", bitmap,
+ "A:bitmaps", ,
NULL) < 0)
return NULL;
@@ -3418,11 +3428,12 @@ qemuBlockExportAddNBD(virDomainObjPtr vm,
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) {
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCK_EXPORT_ADD)) {
g_autoptr(virJSONValue) nbdprops = NULL;
+const char *bitmaps[2] = { bitmap, NULL };
if (!(nbdprops = qemuBlockExportGetNBDProps(src->nodeformat,
exportname,
writable,
-bitmap)))
+bitmaps)))
return -1;
return qemuMonitorBlockExportAdd(priv->mon, );
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index 64a95951f7..c725654155 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -281,7 +281,7 @@ virJSONValuePtr
qemuBlockExportGetNBDProps(const char *nodename,
const char *exportname,
bool writable,
- const char *bitmap);
+ const char **bitmaps);
int
diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.replies
b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.replies
index f5501c9bf9..3faef248d2 100644
--- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.replies
+++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.replies
@@ -21,7 +21,7 @@
"minor": 1,
"major": 5
},
-"package": "v5.1.0-2207-g96292515c0"
+"package": "v5.1.0-2477-gbb564b7029-dirty"
},
"id": "libvirt-2"
}
@@ -72,9 +72,6 @@
{
"name": "rtc-reset-reinjection"
},
-{
- "name": "xen-load-devices-state"
-},
{
"name": "query-command-line-options"
},
@@ -93,48 +90,24 @@
{
"name": "getfd"
},
-{
- "name": "xen-set-global-dirty-log"
-},
{
"name": "change"
},
{
"name": "human-monitor-command"
},
-{
- "name": "inject-nmi"
-},
-{
- "name": "system_wakeup"
-},
{
"name": "x-exit-preconfig"
},
{
"name": "cont"
},
-{
- "name": "pmemsave"
-},
-{
- "name": "memsave"
-},
-{
- "name": "system_powerdown"
-},
-{
- "name": "system_reset"
-},
{
"name": "stop"
},
{
"name": "query-iothreads"
},
-{
- "name": "query-kvm"
-},
{
"name": "query-name"
},
@@ -180,6 +153,27 @@
{
"name": "query-memdev"
},
+{
+ "name": "pmemsave"
+},
+{
+ "name": "memsave"
+},
+{
+ "name": "query-kvm"
+},
+{
+ "name": "inject-nmi"
+},
+{
+
[PATCH 1/3] qemu: capabilities: Disable detection of QEMU_CAPS_BLOCK_EXPORT_ADD
We use the capability to switch to using 'block-export-add' in the
upcoming qemu release instead of the at the same time deprecated
'nbd-server-add'.
Unfortunately qemu wants to change the interface of 'block-export-add'
before the release. Since we've tried to stay up to date and added the
code before it was written in stone, we need to disable the use of the
new interface for the upcoming libvirt release so that we don't have a
version of libvirt which would not work with the upcoming qemu version.
Remove the detection of 'block-export-add' until we are more sure how
the qemu interface will look.
This patch partially reverts commit adb9f7123adb94645
Signed-off-by: Peter Krempa
---
src/qemu/qemu_capabilities.c | 1 -
tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 -
2 files changed, 2 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a67fb785b5..78be372a0c 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1172,7 +1172,6 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
{ "block-dirty-bitmap-merge", QEMU_CAPS_BITMAP_MERGE },
{ "query-cpu-model-baseline", QEMU_CAPS_QUERY_CPU_MODEL_BASELINE },
{ "query-cpu-model-comparison", QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON },
-{ "block-export-add", QEMU_CAPS_BLOCK_EXPORT_ADD },
};
struct virQEMUCapsStringFlags virQEMUCapsMigration[] = {
diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
index 7e07ac486d..63e947c1b3 100644
--- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
@@ -246,7 +246,6 @@
-
5001050
0
--
2.26.2
[PATCH 3/3] qemu: capabilities: Re-enable detection of QEMU_CAPS_BLOCK_EXPORT_ADD
Now that qemu stabilized it's interface and we've switched to the new
design we can re-enable use of 'block-export-add'
This reverts commit TBD!!
---
src/qemu/qemu_capabilities.c | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 +
2 files changed, 2 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 78be372a0c..a67fb785b5 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1172,6 +1172,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
{ "block-dirty-bitmap-merge", QEMU_CAPS_BITMAP_MERGE },
{ "query-cpu-model-baseline", QEMU_CAPS_QUERY_CPU_MODEL_BASELINE },
{ "query-cpu-model-comparison", QEMU_CAPS_QUERY_CPU_MODEL_COMPARISON },
+{ "block-export-add", QEMU_CAPS_BLOCK_EXPORT_ADD },
};
struct virQEMUCapsStringFlags virQEMUCapsMigration[] = {
diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
index 27d5d27175..d7eae9d535 100644
--- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml
@@ -246,6 +246,7 @@
+
5001050
0
--
2.26.2
[PATCH 0/3] qemu: Prevent breakage of libvirt-6.9.0 with qemu-5.2.0
Justification from 1/3: Unfortunately qemu wants to change the interface of 'block-export-add' before the release. Since we've tried to stay up to date and added the code before it was written in stone, we need to disable the use of the new interface for the upcoming libvirt release so that we don't have a version of libvirt which would not work with the upcoming qemu version. Patch 2/3 is the fix itself which uses the new interface. Patch 1/3 and 3/3 are a plan-B option if qemu doesn't include the fix until libvirt-6.9.0 will be about to be released. In such case I'll push patch 1/3 and patches 2-3/3 will be left for a subsequent release. Peter Krempa (3): qemu: capabilities: Disable detection of QEMU_CAPS_BLOCK_EXPORT_ADD qemu: Update to new design of 'block-export-add' qemu: capabilities: Re-enable detection of QEMU_CAPS_BLOCK_EXPORT_ADD src/qemu/qemu_block.c | 17 +- src/qemu/qemu_block.h |2 +- .../caps_5.2.0.x86_64.replies | 2377 + .../caps_5.2.0.x86_64.xml |2 +- tests/qemumonitorjsontest.c |3 +- 5 files changed, 1339 insertions(+), 1062 deletions(-) -- 2.26.2
Re: [PATCH v5 06/12] nbd: Update qapi to support exporting multiple bitmaps
On 10/26/20 5:50 AM, Peter Krempa wrote:
> On Fri, Oct 23, 2020 at 13:36:46 -0500, Eric Blake wrote:
>> Since 'nbd-server-add' is deprecated, and 'block-export-add' is new to
>> 5.2, we can still tweak the interface. Allowing 'bitmaps':['str'] is
>> nicer than 'bitmap':'str'. This wires up the qapi and qemu-nbd
>> changes to permit passing multiple bitmaps as distinct metadata
>> contexts that the NBD client may request, but the actual support for
>> more than one will require a further patch to the server.
>>
>> Signed-off-by: Eric Blake
>> Reviewed-by: Vladimir Sementsov-Ogievskiy
>> ---
>> +++ b/qapi/block-export.json
>> @@ -74,10 +74,10 @@
>> # @description: Free-form description of the export, up to 4096 bytes.
>> # (Since 5.0)
>> #
>> -# @bitmap: Also export the dirty bitmap reachable from @device, so the
>> -# NBD client can use NBD_OPT_SET_META_CONTEXT with the
>> -# metadata context name "qemu:dirty-bitmap:NAME" to inspect the
>> -# bitmap. (since 4.0)
>> +# @bitmaps: Also export each of the named dirty bitmaps reachable from
>> +# @device, so the NBD client can use NBD_OPT_SET_META_CONTEXT with
>> +# the metadata context name "qemu:dirty-bitmap:BITMAP" to inspect
>> +# each bitmap. (since 5.2)
>
> Given unsynchronised release cycles between qemu and management apps
> it's not cool to deprecate an interface without having at least one
> release where the replacement interface is already stable.
>
That's why I'm trying as hard as possible to get the block-export-add
interface perfect in its 5.2 release; if we agree that allowing qemu to
expose more than one bitmap is beneficial (and I argue that it is), then
the new interface MUST support that from the get-go, and not something
where we release it with 5.2 having 'bitmap' and 6.0 adding 'bitmaps'.
> This means that any project wanting to stay up to date will either have
> to use deprecated interfaces for at least one release and develop the
> replacement only when there's a stable interface or hope that they don't
> have to change the interfaces too often.
>
> This specifically impacts libvirt as we have validators which notify us
> that a deprecated interface is used and we want to stay in sync, so that
> there's one less group of users to worry about at the point qemu will
> want to delete the interface.
The deprecated interface is nbd-server-add; for _that_ interface, the
'bitmap' parameter will continue to work until nbd-server-add is removed
in 6.1 (so you have all of 5.2 and 6.0 to switch from nbd-server-add to
block-export-add).
What this patch does, then, is alter the deprecation from merely
changing the command from nbd-server-add to block-export-add with all
parameter names remaining the same, to instead changing both the command
name and the 'bitmap'=>'bitmaps' parameter. But I agree that libvirt
wants to do an all-or-none conversion: there is no reason for libvirt to
use block-export-add until 5.2 is actually released, at which point we
have locked in the new interface; and this patch is a demonstration that
we are still debating about a tweak to that interface before it becomes
locked in.
>
>> # @allocation-depth: Also export the allocation depth map for @device, so
>> #the NBD client can use NBD_OPT_SET_META_CONTEXT with
>> @@ -88,7 +88,7 @@
>> ##
>> { 'struct': 'BlockExportOptionsNbd',
>>'data': { '*name': 'str', '*description': 'str',
>> -'*bitmap': 'str', '*allocation-depth': 'bool' } }
>> +'*bitmaps': ['str'], '*allocation-depth': 'bool' } }
>
> This adds 'bitmaps' also to nbd-server-add, which should not happen. You
> probably want to stop using 'base' for 'NbdServerAddOptions' and just
> duplicate everything else.
I can respin this to NOT add 'bitmaps' to the legacy 'nbd-server-add',
if you think that would be better. It is more complex in the QAPI code,
but not too much more difficulty in the glue code; and the glue code all
goes away in 6.1 when the deprecation cycle ends.
>
>> # @NbdServerAddOptions:
>> @@ -100,12 +100,18 @@
>> # @writable: Whether clients should be able to write to the device via the
>> #NBD connection (default false).
>> #
>> +# @bitmap: Also export a single dirty bitmap reachable from @device, so the
>> +# NBD client can use NBD_OPT_SET_META_CONTEXT with the metadata
>> +# context name "qemu:dirty-bitmap:BITMAP" to inspect the bitmap
>> +# (since 4.0). Mutually exclusive with @bitmaps, and newer
>> +# clients should use that instead.
>
> This doesn't make sense, nbd-server-add never had @bitmaps. Also adding
> features to a deprecated interface doesn't IMO make sense if you want to
> motivate users switch to thne new one.
Fair enough. v6 coming up later today.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[PATCH] docs: page.xsl: Improve generation of paragraph anchor links
Use the 'parent' axis to check whether the parent is a div with class='section' rather than looking for 'toc-backref' anchor to see whether to generate one of the headerlink alternatives. Both hare docutils-specific thus apply to docs generated from RST documents. This adds the links for pages generated from RST documents which don't have a table of contents (and thus lack the 'toc-backref' anchors) and thus fixes pages such as hacking.html and news.html to have reasonable links which can be shared. Signed-off-by: Peter Krempa --- Note that whether or not to add table of contents to hacking.rst is out of scope of this patch. docs/page.xsl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/page.xsl b/docs/page.xsl index 36168305fa..07bfc52a4c 100644 --- a/docs/page.xsl +++ b/docs/page.xsl @@ -196,7 +196,7 @@ - + -- 2.26.2
Re: [libvirt PATCH v5 6/6] Include vdpa devices in node device list
On 10/26/20 6:53 AM, John Ferlan wrote:
On 10/14/20 1:08 PM, Jonathon Jongsma wrote:
The current udev node device driver ignores all events related to vdpa
devices. Since libvirt now supports vDPA network devices, include these
devices in the device list.
Example output:
virsh # nodedev-list
[...ommitted long list of nodedevs...]
vdpa_vdpa0
virsh # nodedev-dumpxml vdpa_vdpa0
vdpa_vdpa0
/sys/devices/vdpa0
computer
vhost_vdpa
/dev/vhost-vdpa-0
NOTE: normally the 'parent' would be a PCI device instead of 'computer',
but this example output is from the vdpa_sim kernel module, so it
doesn't have a normal parent device.
Signed-off-by: Jonathon Jongsma
---
docs/formatnode.html.in| 9 +
docs/schemas/nodedev.rng | 10 ++
include/libvirt/libvirt-nodedev.h | 1 +
src/conf/node_device_conf.c| 14
src/conf/node_device_conf.h| 11 ++-
src/conf/virnodedeviceobj.c| 4 ++-
src/node_device/node_device_udev.c | 53 ++
tools/virsh-nodedev.c | 3 ++
8 files changed, 103 insertions(+), 2 deletions(-)
Coverity notes a RESOURCE_LEAK
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index 29a7eaa07c..b1b8427c05 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1142,6 +1142,55 @@ udevProcessCSS(struct udev_device *device,
return 0;
}
+
+static int
+udevGetVDPACharDev(const char *sysfs_path,
+ virNodeDevCapDataPtr data)
+{
+struct dirent *entry;
+DIR *dir = NULL;
+int direrr;
+
+if (virDirOpenIfExists(, sysfs_path) <= 0)
+return -1;
Any return after this leaks @dir - need a VIR_CLOSE_DIR(dir)
Sigh. I have a nice patch series that converts all DIR*'s to
g_autoptr(DIR). You'd think I would have seen this one in review :-/
+
+while ((direrr = virDirRead(dir, , NULL)) > 0) {
+if (g_str_has_prefix(entry->d_name, "vhost-vdpa")) {
+g_autofree char *chardev = g_strdup_printf("/dev/%s",
entry->d_name);
+
+if (!virFileExists(chardev)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("vDPA chardev path '%s' does not exist"),
+ chardev);
+return -1;
+}
+VIR_DEBUG("vDPA chardev is at '%s'", chardev);
+
+data->vdpa.chardev = g_steal_pointer();
+break;
+}
+}
+
+if (direrr < 0)
+return -1;
+
+return 0;
+}
+
John
[...]
Re: [PATCH 1/2] qemu: Don't try to start NBD server twice
On Mon, Oct 26, 2020 at 11:41:35 +0100, Michal Privoznik wrote: > In one of recent patches the way that we start NBD server for > incoming migration was reworked (v6.8.0-rc1~298). A new boolean > was introduced that tracks whether the NBD server was started so > that we don't start it twice nor record in the port in the port > allocator twice. Well, this idea is good, but in the > implementation the boolean is never set, so we are reserving the > port twice and would be starting the NBD server twice too if it > wasn't for port reservation fail. > > Fixes: e74d627bb3bc2684cbe3edc1e2f7cc745b4e1ff3 > Reported-by: Vjaceslavs Klimovs > Signed-off-by: Michal Privoznik > --- > src/qemu/qemu_migration.c | 8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) Reviewed-by: Peter Krempa
Re: [PATCH 2/2] qemu_migration: Don't mangle NBD part of migration cookie
On Mon, Oct 26, 2020 at 11:41:36 +0100, Michal Privoznik wrote: > In recent commit v6.8.0-135-g518be41aaa the formatting of NBD > into migration cookie was moved into a separate function and with > it it was switched from direct printing into the output buffer to > virXMLFormatElement(). But there was a typo. The > virXMLFormatElement() accepts two buffers on input, one for > element attributes and another for child elements. Well, the line > that was supposed to add NBD port into the attributes buffer > printed the attribute directly into the output buffer which > produced this mangled XML: > > >port='49153' > > > > > > Changing the incriminated line to print into the attributes > buffer fixes the problem. > > Fixes: 518be41aaa3ebaac5f2307f268d24dc1b40b6b5c > Signed-off-by: Michal Privoznik > --- > src/qemu/qemu_migration_cookie.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Oops! I'm planning on adding tests for the qemu-migration cookie XML since we don't have any and it would prevent this problem. Reviewed-by: Peter Krempa
Re: [PATCH v5 06/12] nbd: Update qapi to support exporting multiple bitmaps
On Fri, Oct 23, 2020 at 13:36:46 -0500, Eric Blake wrote:
> Since 'nbd-server-add' is deprecated, and 'block-export-add' is new to
> 5.2, we can still tweak the interface. Allowing 'bitmaps':['str'] is
> nicer than 'bitmap':'str'. This wires up the qapi and qemu-nbd
> changes to permit passing multiple bitmaps as distinct metadata
> contexts that the NBD client may request, but the actual support for
> more than one will require a further patch to the server.
>
> Signed-off-by: Eric Blake
> Reviewed-by: Vladimir Sementsov-Ogievskiy
> ---
> docs/system/deprecated.rst | 4 +++-
> qapi/block-export.json | 18 --
> blockdev-nbd.c | 13 +
> nbd/server.c | 19 +--
> qemu-nbd.c | 10 +-
> 5 files changed, 46 insertions(+), 18 deletions(-)
>
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 905628f3a0cb..d6cd027ac740 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -268,7 +268,9 @@ the 'wait' field, which is only applicable to sockets in
> server mode
>
>
> Use the more generic commands ``block-export-add`` and ``block-export-del``
> -instead.
> +instead. As part of this deprecation, it is now preferred to export a
> +list of dirty bitmaps via ``bitmaps``, rather than a single bitmap via
> +``bitmap``.
>
> Human Monitor Protocol (HMP) commands
> -
> diff --git a/qapi/block-export.json b/qapi/block-export.json
> index 893d5cde5dfe..c7c749d61097 100644
> --- a/qapi/block-export.json
> +++ b/qapi/block-export.json
> @@ -74,10 +74,10 @@
> # @description: Free-form description of the export, up to 4096 bytes.
> # (Since 5.0)
> #
> -# @bitmap: Also export the dirty bitmap reachable from @device, so the
> -# NBD client can use NBD_OPT_SET_META_CONTEXT with the
> -# metadata context name "qemu:dirty-bitmap:NAME" to inspect the
> -# bitmap. (since 4.0)
> +# @bitmaps: Also export each of the named dirty bitmaps reachable from
> +# @device, so the NBD client can use NBD_OPT_SET_META_CONTEXT with
> +# the metadata context name "qemu:dirty-bitmap:BITMAP" to inspect
> +# each bitmap. (since 5.2)
Given unsynchronised release cycles between qemu and management apps
it's not cool to deprecate an interface without having at least one
release where the replacement interface is already stable.
This means that any project wanting to stay up to date will either have
to use deprecated interfaces for at least one release and develop the
replacement only when there's a stable interface or hope that they don't
have to change the interfaces too often.
This specifically impacts libvirt as we have validators which notify us
that a deprecated interface is used and we want to stay in sync, so that
there's one less group of users to worry about at the point qemu will
want to delete the interface.
> # @allocation-depth: Also export the allocation depth map for @device, so
> #the NBD client can use NBD_OPT_SET_META_CONTEXT with
> @@ -88,7 +88,7 @@
> ##
> { 'struct': 'BlockExportOptionsNbd',
>'data': { '*name': 'str', '*description': 'str',
> -'*bitmap': 'str', '*allocation-depth': 'bool' } }
> +'*bitmaps': ['str'], '*allocation-depth': 'bool' } }
This adds 'bitmaps' also to nbd-server-add, which should not happen. You
probably want to stop using 'base' for 'NbdServerAddOptions' and just
duplicate everything else.
> # @NbdServerAddOptions:
> @@ -100,12 +100,18 @@
> # @writable: Whether clients should be able to write to the device via the
> #NBD connection (default false).
> #
> +# @bitmap: Also export a single dirty bitmap reachable from @device, so the
> +# NBD client can use NBD_OPT_SET_META_CONTEXT with the metadata
> +# context name "qemu:dirty-bitmap:BITMAP" to inspect the bitmap
> +# (since 4.0). Mutually exclusive with @bitmaps, and newer
> +# clients should use that instead.
This doesn't make sense, nbd-server-add never had @bitmaps. Also adding
features to a deprecated interface doesn't IMO make sense if you want to
motivate users switch to thne new one.
> +#
> # Since: 5.0
> ##
> { 'struct': 'NbdServerAddOptions',
>'base': 'BlockExportOptionsNbd',
>'data': { 'device': 'str',
> -'*writable': 'bool' } }
> +'*writable': 'bool', '*bitmap': 'str' } }
>
> ##
> # @nbd-server-add:
Re: [libvirt PATCH v5 6/6] Include vdpa devices in node device list
On 10/14/20 1:08 PM, Jonathon Jongsma wrote:
> The current udev node device driver ignores all events related to vdpa
> devices. Since libvirt now supports vDPA network devices, include these
> devices in the device list.
>
> Example output:
>
> virsh # nodedev-list
> [...ommitted long list of nodedevs...]
> vdpa_vdpa0
>
> virsh # nodedev-dumpxml vdpa_vdpa0
>
> vdpa_vdpa0
> /sys/devices/vdpa0
> computer
>
> vhost_vdpa
>
>
> /dev/vhost-vdpa-0
>
>
>
> NOTE: normally the 'parent' would be a PCI device instead of 'computer',
> but this example output is from the vdpa_sim kernel module, so it
> doesn't have a normal parent device.
>
> Signed-off-by: Jonathon Jongsma
> ---
> docs/formatnode.html.in| 9 +
> docs/schemas/nodedev.rng | 10 ++
> include/libvirt/libvirt-nodedev.h | 1 +
> src/conf/node_device_conf.c| 14
> src/conf/node_device_conf.h| 11 ++-
> src/conf/virnodedeviceobj.c| 4 ++-
> src/node_device/node_device_udev.c | 53 ++
> tools/virsh-nodedev.c | 3 ++
> 8 files changed, 103 insertions(+), 2 deletions(-)
>
Coverity notes a RESOURCE_LEAK
> diff --git a/src/node_device/node_device_udev.c
> b/src/node_device/node_device_udev.c
> index 29a7eaa07c..b1b8427c05 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c
> @@ -1142,6 +1142,55 @@ udevProcessCSS(struct udev_device *device,
> return 0;
> }
>
> +
> +static int
> +udevGetVDPACharDev(const char *sysfs_path,
> + virNodeDevCapDataPtr data)
> +{
> +struct dirent *entry;
> +DIR *dir = NULL;
> +int direrr;
> +
> +if (virDirOpenIfExists(, sysfs_path) <= 0)
> +return -1;
Any return after this leaks @dir - need a VIR_CLOSE_DIR(dir)
> +
> +while ((direrr = virDirRead(dir, , NULL)) > 0) {
> +if (g_str_has_prefix(entry->d_name, "vhost-vdpa")) {
> +g_autofree char *chardev = g_strdup_printf("/dev/%s",
> entry->d_name);
> +
> +if (!virFileExists(chardev)) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("vDPA chardev path '%s' does not exist"),
> + chardev);> +return -1;
> +}
> +VIR_DEBUG("vDPA chardev is at '%s'", chardev);
> +
> +data->vdpa.chardev = g_steal_pointer();
> +break;
> +}
> +}
> +
> +if (direrr < 0)
> +return -1;
> +
> +return 0;
> +}
> +
John
[...]
[PATCH 1/2] qemu: Don't try to start NBD server twice
In one of recent patches the way that we start NBD server for
incoming migration was reworked (v6.8.0-rc1~298). A new boolean
was introduced that tracks whether the NBD server was started so
that we don't start it twice nor record in the port in the port
allocator twice. Well, this idea is good, but in the
implementation the boolean is never set, so we are reserving the
port twice and would be starting the NBD server twice too if it
wasn't for port reservation fail.
Fixes: e74d627bb3bc2684cbe3edc1e2f7cc745b4e1ff3
Reported-by: Vjaceslavs Klimovs
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_migration.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 2f5d61f8e7..6f764b0c73 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -479,9 +479,11 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver,
QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
goto cleanup;
-if (!server_started &&
-qemuMonitorNBDServerStart(priv->mon, , tls_alias) < 0)
-goto exit_monitor;
+if (!server_started) {
+if (qemuMonitorNBDServerStart(priv->mon, , tls_alias) < 0)
+goto exit_monitor;
+server_started = true;
+}
if (qemuBlockExportAddNBD(vm, diskAlias, disk->src, diskAlias, true,
NULL) < 0)
goto exit_monitor;
--
2.26.2
[PATCH 2/2] qemu_migration: Don't mangle NBD part of migration cookie
In recent commit v6.8.0-135-g518be41aaa the formatting of NBD
into migration cookie was moved into a separate function and with
it it was switched from direct printing into the output buffer to
virXMLFormatElement(). But there was a typo. The
virXMLFormatElement() accepts two buffers on input, one for
element attributes and another for child elements. Well, the line
that was supposed to add NBD port into the attributes buffer
printed the attribute directly into the output buffer which
produced this mangled XML:
port='49153'
Changing the incriminated line to print into the attributes
buffer fixes the problem.
Fixes: 518be41aaa3ebaac5f2307f268d24dc1b40b6b5c
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_migration_cookie.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c
index 708c2cced7..39445ef8de 100644
--- a/src/qemu/qemu_migration_cookie.c
+++ b/src/qemu/qemu_migration_cookie.c
@@ -747,7 +747,7 @@ qemuMigrationCookieNBDXMLFormat(qemuMigrationCookieNBDPtr
nbd,
size_t i;
if (nbd->port)
-virBufferAsprintf(buf, " port='%d'", nbd->port);
+virBufferAsprintf(, " port='%d'", nbd->port);
for (i = 0; i < nbd->ndisks; i++) {
virBufferEscapeString(, "disks[i].target);
--
2.26.2
[PATCH 0/2] qemu: Two almost trivial migration fixes
The report on libvirt-users [1] got me into testing and I've found another problem fixed in the other patch. 1: https://www.redhat.com/archives/libvirt-users/2020-October/msg00035.html Michal Prívozník (2): qemu: Don't try to start NBD server twice qemu_migration: Don't mangle NBD part of migration cookie src/qemu/qemu_migration.c| 8 +--- src/qemu/qemu_migration_cookie.c | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) -- 2.26.2
Re: [PATCH] pci: Refuse to hotplug PCI Devices when the Guest OS is not ready
On Fri, 23 Oct 2020 19:27:55 +0200 Igor Mammedov wrote: > On Fri, 23 Oct 2020 11:54:40 -0400 > "Michael S. Tsirkin" wrote: > > [...] > [...] > [...] > [...] > > > > Rather than adding_device_allowed, something like "query slot" > > might be helpful for debugging. That would help user figure out > > e.g. why isn't device visible without any races. > > Would be new command useful tough? What we end up is broken guest > (if I read commit message right) and a user who has no idea if > device_add was successful or not. > So what user should do in this case > - wait till it explodes? > - can user remove it or it would be stuck there forever? > - poll slot before hotplug, manually? > > (if this is the case then failing device_add cleanly doesn't sound bad, > it looks similar to another error we have "/* Check if hot-plug is disabled > on the slot */" > in pcie_cap_slot_pre_plug_cb) > > CCing libvirt, as it concerns not only QEMU. > > [...] > [...] > > > > I think we want QEMU management interface to be reasonably > > abstract and agnostic if possible. Pushing knowledge of hardware > > detail to management will just lead to pain IMHO. > > We supported device_add which practically never fails for years, > > For CPUs and RAM, device_add can fail so maybe management is also > prepared to handle errors on PCI hotplug path. There can be unarguable reasons for PCI hotplug to fail as well (attempting to plug to a bus that can't support it for one). The difference here is that it's a failure that we expect to be transitory. -- David Gibson Principal Software Engineer, Virtualization, Red Hat pgpnAv0PoDCpt.pgp Description: OpenPGP digital signature
Re: [PATCH] pci: Refuse to hotplug PCI Devices when the Guest OS is not ready
On Fri, Oct 23, 2020 at 19:27:55 +0200, Igor Mammedov wrote: > On Fri, 23 Oct 2020 11:54:40 -0400 > "Michael S. Tsirkin" wrote: > > > On Fri, Oct 23, 2020 at 09:47:14AM +0300, Marcel Apfelbaum wrote: > > > Hi David, > > > > > > On Fri, Oct 23, 2020 at 6:49 AM David Gibson wrote: > > > > > > On Thu, 22 Oct 2020 11:01:04 -0400 > > > "Michael S. Tsirkin" wrote: > > > > > > > On Thu, Oct 22, 2020 at 05:50:51PM +0300, Marcel Apfelbaum wrote: > > > > [...] > > > > > > > > Right. After detecting just failing unconditionally it a bit too > > > > simplistic IMHO. > > > > > > There's also another factor here, which I thought I'd mentioned > > > already, but looks like I didn't: I think we're still missing some > > > details in what's going on. > > > > > > The premise for this patch is that plugging while the indicator is in > > > transition state is allowed to fail in any way on the guest side. > > > I > > > don't think that's a reasonable interpretation, because it's > > > unworkable > > > for physical hotplug. If the indicator starts blinking while > > > you're in > > > the middle of shoving a card in, you'd be in trouble. > > > > > > So, what I'm assuming here is that while "don't plug while blinking" > > > is > > > the instruction for the operator to obey as best they can, on the > > > guest > > > side the rule has to be "start blinking, wait a while and by the time > > > you leave blinking state again, you can be confident any plugs or > > > unplugs have completed". Obviously still racy in the strict > > > computer > > > science sense, but about the best you can do with slow humans in the > > > mix. > > > > > > So, qemu should of course endeavour to follow that rule as though it > > > was a human operator on a physical machine and not plug when the > > > indicator is blinking. *But* the qemu plug will in practice be > > > fast > > > enough that if we're hitting real problems here, it suggests the guest > > > is still doing something wrong. > > > > > > > > > I personally think there is a little bit of over-engineering here. > > > Let's start with the spec: > > > > > >   Power Indicator Blinking > > >   A blinking Power Indicator indicates that the slot is powering > > > up or > > > powering down and that > > >   insertion or removal of the adapter is not permitted. > > > > > > What exactly is an interpretation here? > > > As you stated, the races are theoretical, the whole point of the indicator > > > is to let the operator know he can't plug the device just yet. > > > > > > I understand it would be more user friendly if the QEMU would wait > > > internally > > > for the > > > blinking to end, but the whole point of the indicator is to let the > > > operator > > > (human or machine) > > > know they can't plug the device at a specific time. > > > Should QEMU take the responsibility of the operator? Is it even > > > correct? > > > > > > Even if we would want such a feature, how is it related to this patch? > > > The patch simply refuses to start a hotplug operation when it knows it > > > will not > > > succeed. > > >  > > > Another way that would make sense to me would be is a new QEMU > > > interface other > > > than > > > "add_device", let's say "adding_device_allowed", that would return true > > > if the > > > hotplug is allowed > > > at this point of time. (I am aware of the theoretical races) > > > > Rather than adding_device_allowed, something like "query slot" > > might be helpful for debugging. That would help user figure out > > e.g. why isn't device visible without any races. > > Would be new command useful tough? What we end up is broken guest > (if I read commit message right) and a user who has no idea if > device_add was successful or not. > So what user should do in this case > - wait till it explodes? > - can user remove it or it would be stuck there forever? > - poll slot before hotplug, manually? > > (if this is the case then failing device_add cleanly doesn't sound bad, > it looks similar to another error we have "/* Check if hot-plug is disabled > on the slot */" > in pcie_cap_slot_pre_plug_cb) > > CCing libvirt, as it concerns not only QEMU. The only reason a separate command might make sense is if libvirt would want to provide a specific error to the user/upper management layer that the operation failed due to a transient failure (so that it can be retried later). We don't really want to go to a policy decision of how long to wait in such case, so unless qemu waits libvirt will plainly want to report an error. That said IMO 'device_add' should still fail if it's certain that the device won't be plugged in. This will fix any client which is currently in use. Adding a separate command is worth only for pre-checking for saner error handling. > > > The above will
Re: [PATCH] qemu: virtiofs: Add pool element to limit thread pool
On Tue, Oct 20, 2020 at 13:54:33 -0400, Masayoshi Mizuma wrote: > From: Masayoshi Mizuma > > virtiofsd has --thread-pool-size=NUM option to limit the > thread pool size. It will be useful to tune the performance. Usually it's great to document or at least link to a document which outlines how changing the parameter influences performance. In many cases theres just one good option and that should become default. In other cases it depends on deployment and in such case it's very good to prevent users from having to guess what it actually does. In the end it serves as a justification that adding a tuning knob is worth it. > Add pool element to use the option like as: > > > > I'd prefer if the attribute is named 'threads' as that seems to me to be more clear what it does without having to refer back to the documentation. > > > > > Signed-off-by: Masayoshi Mizuma > ---
Re: [PATCH v2] news: introduce memory failure event
On 10/23/20 3:53 PM, zhenwei pi wrote: Signed-off-by: zhenwei pi --- NEWS.rst | 10 ++ 1 file changed, 10 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index d0454b7840..428928e80b 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -13,6 +13,16 @@ v6.9.0 (unreleased) * **New features** + * Introduce memory failure event + +Libvirt could handle domain's memory failure event. Drivers need to +implement their own method. In my opinion this is unnecessary. The target audience of this file is users, not developers so much. Therefore, the second sentence does not add much value, because: a) users don't know how to implement that (unless an user is also a developer), b) by this time, users should be used to our driver model and the fact that some drivers implement a feature that other drivers don't (yet). + + * qemu: Implement memory failure event + +New event is implemented that is emitted whenever a guest encounters a +memory failure. + ACK to this hunk and pushed. Thanks for handling this. Michal
Re: [PATCH 1/1] virt-aa-helper: allow hard links for mounts
On 10/23/20 4:19 PM, Christian Schoenebeck wrote: On Donnerstag, 22. Oktober 2020 19:07:33 CEST Michal Privoznik wrote: [Please don't CC random people on patches until asked to, we are all subscribed to the list] Got it, I'll refrain from CCing on libvirt in future. Not as erratic as it looks like though: I CCed people who touched this specific AppArmor permission before, plus the virtiofs maintainers. Yeah, I understand that. BTW: it's okay to CC people when replying :-) On 10/22/20 4:58 PM, Christian Schoenebeck wrote: Guests should be allowed to create hard links on mounted pathes, since many applications rely on this functionality and would error on guest with current "rw" AppArmor permission with 9pfs. Signed-off-by: Christian Schoenebeck --- src/security/virt-aa-helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 12429278fb..5a6f4a5f7d 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1142,7 +1142,7 @@ get_files(vahControl * ctl) /* We don't need to add deny rw rules for readonly mounts, * this can only lead to troubles when mounting / readonly. */ -if (vah_add_path(, fs->src->path, fs->readonly ? "R" : "rw", true) != 0) +if (vah_add_path(, fs->src->path, fs->readonly ? "R" : "rwl", true) != 0)> goto cleanup; } } Reviewed-by: Michal Privoznik but I will give a day or two for other developers to chime in. Michal Yes, please wait couple days to see whether there are reactions. Okay, so nobody objected and we can expect the freeze of upstream today, so I am pushing this.
Re: [PATCH 00/10] Make mdev_types generic and support it on CSS devices
On Fri, Oct 23, 2020 at 07:31:42PM +0200, Boris Fiuczynski wrote: > Since channel subsystem devices can have the capability to create an > mdev device they should expose this by listing the mdev_types capability. > > Many patches of this series is concerned with refactoring existing PCI code. Hi Boris, I will start looking at the patches next week. Regards, Erik
Re: [PATCH v2 2/7] migration/dirtyrate: Implement qemuMonitorJSONExtractDirtyRateInfo
Thanks for the reminding. I would fix that in my V3 patches. 在 2020/10/26 14:24, Peter Krempa 写道: > On Sun, Oct 25, 2020 at 10:35:03 +0800, Hao Wang wrote: >> Hi Daniel, >> >> For the warning in your compiling -- "error: >> ‘qemuMonitorJSONExtractDirtyRateInfo’ defined but not used >> [-Werror=unused-function]": >> >> " qemuMonitorJSONExtractDirtyRateInfo" would be called by function >> "qemuMonitorJSONQueryDirtyRate" in [PATCH 3/7]. >> So please apply all the serial of patches before your compiling. Thanks! > > Note that according to our contributor guidelines [1] a patchset must > compile cleanly and pass all testsuite after every single patch. > > Please reorganize your series such that this can be achieved. Also note > that squashing all patches into 1 is not acceptable. Patchet must be > logically split into separable chunks. > > [1] https://libvirt.org/hacking.html#preparing-patches > > . >
Re: [libvirt PATCH v5 6/6] Include vdpa devices in node device list
On 10/14/20 1:08 PM, Jonathon Jongsma wrote: The current udev node device driver ignores all events related to vdpa devices. Since libvirt now supports vDPA network devices, include these devices in the device list. Example output: virsh # nodedev-list [...ommitted long list of nodedevs...] vdpa_vdpa0 virsh # nodedev-dumpxml vdpa_vdpa0 vdpa_vdpa0 /sys/devices/vdpa0 computer vhost_vdpa /dev/vhost-vdpa-0 NOTE: normally the 'parent' would be a PCI device instead of 'computer', but this example output is from the vdpa_sim kernel module, so it doesn't have a normal parent device. Signed-off-by: Jonathon Jongsma Reviewed-by: Laine Stump (I had left this patch in limbo in case anyone had issues with the particular element names, and then forgot about it for the last week. Seeing that nobody had an issue, I'm pushing it so it gets into the same release as the rest of the series)
Re: [PATCH v2 2/7] migration/dirtyrate: Implement qemuMonitorJSONExtractDirtyRateInfo
On Sun, Oct 25, 2020 at 10:35:03 +0800, Hao Wang wrote: > Hi Daniel, > > For the warning in your compiling -- "error: > ‘qemuMonitorJSONExtractDirtyRateInfo’ defined but not used > [-Werror=unused-function]": > > " qemuMonitorJSONExtractDirtyRateInfo" would be called by function > "qemuMonitorJSONQueryDirtyRate" in [PATCH 3/7]. > So please apply all the serial of patches before your compiling. Thanks! Note that according to our contributor guidelines [1] a patchset must compile cleanly and pass all testsuite after every single patch. Please reorganize your series such that this can be achieved. Also note that squashing all patches into 1 is not acceptable. Patchet must be logically split into separable chunks. [1] https://libvirt.org/hacking.html#preparing-patches
