Add iommu device when VM configured with > 255 vcpus
Hi All, I vaguely recall a discussion about $subject, but can't find it now. Perhaps buried in another thread. The topic has been raised internally again, and I'd like to gauge the community's interest in automatically adding the necessary devices/config when user has specified vcpus > 255. The comparison for prior art is a bit of a stretch, but we e.g. add type='spice'/> when spice graphics is configured. I know libvirt has generally tried to avoid policy decisions, but it's not clear to me where we stand with cases such as this, where every x86 VM with > 255 vcpus needs a similarly configured iommu. Regards, Jim
Re: [PATCH 1/3] meson: Improve default firewall backend configuration
On 5/28/24 12:59 PM, Andrea Bolognani wrote:
On Tue, May 28, 2024 at 12:50:51PM GMT, Laine Stump wrote:
On 5/28/24 12:31 PM, Pavel Hrdina wrote:
On Tue, May 28, 2024 at 05:49:19PM +0200, Andrea Bolognani wrote:
+ if (not firewall_backend_priority.contains('nftables') or
+ not firewall_backend_priority.contains('iptables') or
+ firewall_backend_priority.length() != 2)
No need to have a check for specific values. Meson will already check if
they are from the list of choices defined in meson_options.txt .
But we don't just need to check that the values in the list are all valid
options; we also want to make sure that nobody has entered the same value
multiple times (which this ends up doing by making sure that there is at
least one entry for each valid value, and that the list is the same length
as the number of valid values).
Yes, that was exactly the idea.
Or do we not care if someone repeats the same value? Maybe somebody wants to
include iptables support in the build, but not look for it automatically
(instead only use it if it's explicitly requested in network.conf). One way
of doing that would be to sent firewall_backend_priority = nftables,nftables
(that does seem a bit obtuse; perhaps it would be better to allow limiting
the length of the option list to 1)
If that's something that we want to allow, then we should include
explicit support for it rather than make it possible through obscure
runes :)
Agreed. Just trying to come up with a valid advantage for not checking
that all values are present :-)
I'm not sure we really need to bother, but I don't feel strongly
either way so I could be persuaded to look into it. Perhaps as an
after-release follow up, though?
Seeing that we could all change our opinion tomorrow, my choice would be
to push what you currently have and then discuss tweaks later. The
important thing that needs to be gotten correct from the beginning is
the basic public "API", and I think the way you have it is solid, and
none of these details will affect that.
+option('firewall_backend_priority', type: 'array', choices: ['nftables',
'iptables'], description: 'firewall backends to try, preferred ones first')
What about "order of firewall backends to try"? The part "preferred ones
first" sounds strange to me.
Sure, that works too.
Re: [PATCH 1/3] meson: Improve default firewall backend configuration
On Tue, May 28, 2024 at 09:59:17AM -0700, Andrea Bolognani wrote:
> On Tue, May 28, 2024 at 12:50:51PM GMT, Laine Stump wrote:
> > On 5/28/24 12:31 PM, Pavel Hrdina wrote:
> > > On Tue, May 28, 2024 at 05:49:19PM +0200, Andrea Bolognani wrote:
> > > > + if (not firewall_backend_priority.contains('nftables') or
> > > > + not firewall_backend_priority.contains('iptables') or
> > > > + firewall_backend_priority.length() != 2)
> > >
> > > No need to have a check for specific values. Meson will already check if
> > > they are from the list of choices defined in meson_options.txt .
> >
> > But we don't just need to check that the values in the list are all valid
> > options; we also want to make sure that nobody has entered the same value
> > multiple times (which this ends up doing by making sure that there is at
> > least one entry for each valid value, and that the list is the same length
> > as the number of valid values).
>
> Yes, that was exactly the idea.
True, that is not checked so we still need to duplicate the list here
that I wanted to avoid.
> > Or do we not care if someone repeats the same value? Maybe somebody wants to
> > include iptables support in the build, but not look for it automatically
> > (instead only use it if it's explicitly requested in network.conf). One way
> > of doing that would be to sent firewall_backend_priority = nftables,nftables
> >
> > (that does seem a bit obtuse; perhaps it would be better to allow limiting
> > the length of the option list to 1)
>
> If that's something that we want to allow, then we should include
> explicit support for it rather than make it possible through obscure
> runes :)
>
> I'm not sure we really need to bother, but I don't feel strongly
> either way so I could be persuaded to look into it. Perhaps as an
> after-release follow up, though?
>
> > > > +option('firewall_backend_priority', type: 'array', choices:
> > > > ['nftables', 'iptables'], description: 'firewall backends to try,
> > > > preferred ones first')
> > >
> > > What about "order of firewall backends to try"? The part "preferred ones
> > > first" sounds strange to me.
>
> Sure, that works too.
Reviewed-by: Pavel Hrdina
signature.asc
Description: PGP signature
Re: [PATCH 1/3] meson: Improve default firewall backend configuration
On Tue, May 28, 2024 at 12:50:51PM GMT, Laine Stump wrote:
> On 5/28/24 12:31 PM, Pavel Hrdina wrote:
> > On Tue, May 28, 2024 at 05:49:19PM +0200, Andrea Bolognani wrote:
> > > + if (not firewall_backend_priority.contains('nftables') or
> > > + not firewall_backend_priority.contains('iptables') or
> > > + firewall_backend_priority.length() != 2)
> >
> > No need to have a check for specific values. Meson will already check if
> > they are from the list of choices defined in meson_options.txt .
>
> But we don't just need to check that the values in the list are all valid
> options; we also want to make sure that nobody has entered the same value
> multiple times (which this ends up doing by making sure that there is at
> least one entry for each valid value, and that the list is the same length
> as the number of valid values).
Yes, that was exactly the idea.
> Or do we not care if someone repeats the same value? Maybe somebody wants to
> include iptables support in the build, but not look for it automatically
> (instead only use it if it's explicitly requested in network.conf). One way
> of doing that would be to sent firewall_backend_priority = nftables,nftables
>
> (that does seem a bit obtuse; perhaps it would be better to allow limiting
> the length of the option list to 1)
If that's something that we want to allow, then we should include
explicit support for it rather than make it possible through obscure
runes :)
I'm not sure we really need to bother, but I don't feel strongly
either way so I could be persuaded to look into it. Perhaps as an
after-release follow up, though?
> > > +option('firewall_backend_priority', type: 'array', choices: ['nftables',
> > > 'iptables'], description: 'firewall backends to try, preferred ones
> > > first')
> >
> > What about "order of firewall backends to try"? The part "preferred ones
> > first" sounds strange to me.
Sure, that works too.
--
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 1/3] meson: Improve default firewall backend configuration
On 5/28/24 12:31 PM, Pavel Hrdina wrote:
On Tue, May 28, 2024 at 05:49:19PM +0200, Andrea Bolognani wrote:
The current implementation requires users to configure the
preference as such:
-Dfirewall_backend_default_1=iptables
-Dfirewall_backend_default_2=nftables
In addition to being more verbose than one would hope, there
are several things that could go wrong.
First of all, meson performs no validation on the provided
values, so mistakes will only be caught by the compiler.
Additionally, it's entirely possible to provide nonsensical
combinations, such as repeating the same value twice.
Change things so that the preference can now be configured
as such:
-Dfirewall_backend_priority=iptables,nftables
Checks have been added to prevent invalid values from being
accepted.
Signed-off-by: Andrea Bolognani
---
meson.build | 16 +---
meson_options.txt| 3 +--
src/network/bridge_driver_conf.c | 6 +-
src/network/meson.build | 6 --
src/network/network.conf.in | 13 +++--
5 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/meson.build b/meson.build
index f67c3d2724..ed0e9686f8 100644
--- a/meson.build
+++ b/meson.build
@@ -1635,15 +1635,17 @@ endif
if not get_option('driver_network').disabled() and conf.has('WITH_LIBVIRTD')
conf.set('WITH_NETWORK', 1)
- firewall_backend_default_1 = get_option('firewall_backend_default_1')
- firewall_backend_default_conf = firewall_backend_default_1
- firewall_backend_default_1 = 'VIR_FIREWALL_BACKEND_' +
firewall_backend_default_1.to_upper()
- conf.set('FIREWALL_BACKEND_DEFAULT_1', firewall_backend_default_1)
- firewall_backend_default_2 = get_option('firewall_backend_default_2')
- firewall_backend_default_2 = 'VIR_FIREWALL_BACKEND_' +
firewall_backend_default_2.to_upper()
- conf.set('FIREWALL_BACKEND_DEFAULT_2', firewall_backend_default_2)
+ firewall_backend_priority = get_option('firewall_backend_priority')
+ if (not firewall_backend_priority.contains('nftables') or
+ not firewall_backend_priority.contains('iptables') or
+ firewall_backend_priority.length() != 2)
No need to have a check for specific values. Meson will already check if
they are from the list of choices defined in meson_options.txt .
But we don't just need to check that the values in the list are all
valid options; we also want to make sure that nobody has entered the
same value multiple times (which this ends up doing by making sure that
there is at least one entry for each valid value, and that the list is
the same length as the number of valid values).
Or do we not care if someone repeats the same value? Maybe somebody
wants to include iptables support in the build, but not look for it
automatically (instead only use it if it's explicitly requested in
network.conf). One way of doing that would be to sent
firewall_backend_priority = nftables,nftables
(that does seem a bit obtuse; perhaps it would be better to allow
limiting the length of the option list to 1)
Anyway, I'm fine with it either way. Both are better than what I had before.
+error('invalid value for firewall_backend_priority option')
+ endif
+ conf.set('FIREWALL_BACKEND_PRIORITY_0', 'VIR_FIREWALL_BACKEND_' + firewall_backend_priority[0].to_upper())
+ conf.set('FIREWALL_BACKEND_PRIORITY_1', 'VIR_FIREWALL_BACKEND_' +
firewall_backend_priority[1].to_upper())
+ conf.set('FIREWALL_BACKEND_PRIORITY_NUM', firewall_backend_priority.length())
elif get_option('driver_network').enabled()
error('libvirtd must be enabled to build the network driver')
endif
diff --git a/meson_options.txt b/meson_options.txt
index ad354a8668..8723d13231 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -115,8 +115,7 @@ option('dtrace', type: 'feature', value: 'auto',
description: 'use dtrace for st
option('firewalld', type: 'feature', value: 'auto', description: 'firewalld
support')
# dep:firewalld
option('firewalld_zone', type: 'feature', value: 'auto', description:
'whether to install firewalld libvirt zone')
-option('firewall_backend_default_1', type: 'string', value: 'nftables',
description: 'first firewall backend to try when none is specified')
-option('firewall_backend_default_2', type: 'string', value: 'iptables',
description: 'second firewall backend to try when none is specified (and first
is unavailable)')
+option('firewall_backend_priority', type: 'array', choices: ['nftables',
'iptables'], description: 'firewall backends to try, preferred ones first')
What about "order of firewall backends to try"? The part "preferred ones
first" sounds strange to me.
Otherwise looks good.
Pavel
Re: [PATCH 1/3] meson: Improve default firewall backend configuration
On 5/28/24 11:49 AM, Andrea Bolognani wrote:
The current implementation requires users to configure the
preference as such:
-Dfirewall_backend_default_1=iptables
-Dfirewall_backend_default_2=nftables
In addition to being more verbose than one would hope, there
are several things that could go wrong.
First of all, meson performs no validation on the provided
values, so mistakes will only be caught by the compiler.
Additionally, it's entirely possible to provide nonsensical
combinations, such as repeating the same value twice.
Change things so that the preference can now be configured
as such:
-Dfirewall_backend_priority=iptables,nftables
Checks have been added to prevent invalid values from being
accepted.
Signed-off-by: Andrea Bolognani
---
meson.build | 16 +---
meson_options.txt| 3 +--
src/network/bridge_driver_conf.c | 6 +-
src/network/meson.build | 6 --
src/network/network.conf.in | 13 +++--
5 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/meson.build b/meson.build
index f67c3d2724..ed0e9686f8 100644
--- a/meson.build
+++ b/meson.build
@@ -1635,15 +1635,17 @@ endif
if not get_option('driver_network').disabled() and conf.has('WITH_LIBVIRTD')
conf.set('WITH_NETWORK', 1)
- firewall_backend_default_1 = get_option('firewall_backend_default_1')
- firewall_backend_default_conf = firewall_backend_default_1
- firewall_backend_default_1 = 'VIR_FIREWALL_BACKEND_' +
firewall_backend_default_1.to_upper()
- conf.set('FIREWALL_BACKEND_DEFAULT_1', firewall_backend_default_1)
- firewall_backend_default_2 = get_option('firewall_backend_default_2')
- firewall_backend_default_2 = 'VIR_FIREWALL_BACKEND_' +
firewall_backend_default_2.to_upper()
- conf.set('FIREWALL_BACKEND_DEFAULT_2', firewall_backend_default_2)
+ firewall_backend_priority = get_option('firewall_backend_priority')
+ if (not firewall_backend_priority.contains('nftables') or
+ not firewall_backend_priority.contains('iptables') or
+ firewall_backend_priority.length() != 2)
Okay, yeah. That does actually make sure that both possibilities are in
the list, and (indirectly) that none is duplicated.
+error('invalid value for firewall_backend_priority option')
+ endif
+ conf.set('FIREWALL_BACKEND_PRIORITY_0', 'VIR_FIREWALL_BACKEND_' + firewall_backend_priority[0].to_upper())
+ conf.set('FIREWALL_BACKEND_PRIORITY_1', 'VIR_FIREWALL_BACKEND_' +
firewall_backend_priority[1].to_upper())
+ conf.set('FIREWALL_BACKEND_PRIORITY_NUM', firewall_backend_priority.length())
elif get_option('driver_network').enabled()
error('libvirtd must be enabled to build the network driver')
endif
diff --git a/meson_options.txt b/meson_options.txt
index ad354a8668..8723d13231 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -115,8 +115,7 @@ option('dtrace', type: 'feature', value: 'auto',
description: 'use dtrace for st
option('firewalld', type: 'feature', value: 'auto', description: 'firewalld
support')
# dep:firewalld
option('firewalld_zone', type: 'feature', value: 'auto', description:
'whether to install firewalld libvirt zone')
-option('firewall_backend_default_1', type: 'string', value: 'nftables',
description: 'first firewall backend to try when none is specified')
-option('firewall_backend_default_2', type: 'string', value: 'iptables',
description: 'second firewall backend to try when none is specified (and first
is unavailable)')
+option('firewall_backend_priority', type: 'array', choices: ['nftables',
'iptables'], description: 'firewall backends to try, preferred ones first')
Yes! This is the way I initially wanted to do it, but didn't have enough
meson-foo to make it work.
option('host_validate', type: 'feature', value: 'auto', description: 'build
virt-host-validate')
option('init_script', type: 'combo', choices: ['systemd', 'openrc', 'check',
'none'], value: 'check', description: 'Style of init script to install')
option('loader_nvram', type: 'string', value: '', description: 'Pass list of pairs of
: paths. Both pairs and list items are separated by a colon.')
diff --git a/src/network/bridge_driver_conf.c b/src/network/bridge_driver_conf.c
index 8f4956dace..e2f3613a41 100644
--- a/src/network/bridge_driver_conf.c
+++ b/src/network/bridge_driver_conf.c
@@ -67,8 +67,12 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg
G_GNUC_UNUSED,
g_autofree char *fwBackendStr = NULL;
bool fwBackendSelected = false;
size_t i;
-int fwBackends[] = { FIREWALL_BACKEND_DEFAULT_1,
FIREWALL_BACKEND_DEFAULT_2 };
+int fwBackends[] = {
+FIREWALL_BACKEND_PRIORITY_0,
+FIREWALL_BACKEND_PRIORITY_1,
+};
G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) == VIR_FIREWALL_BACKEND_LAST);
+G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) == FIREWALL_BACKEND_PRIORITY_NUM);
I had to think about this for a minute, since at first glance those both
Re: [PATCH 3/3] rpm: Configure firewall backends explicitly
On Tue, May 28, 2024 at 05:49:21PM +0200, Andrea Bolognani wrote: > Signed-off-by: Andrea Bolognani > --- > libvirt.spec.in | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
Re: [PATCH 2/3] meson: Include firewall backend selection in summary
On Tue, May 28, 2024 at 05:49:20PM +0200, Andrea Bolognani wrote: > Signed-off-by: Andrea Bolognani > --- > meson.build | 5 + > 1 file changed, 5 insertions(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
Re: [PATCH 1/3] meson: Improve default firewall backend configuration
On Tue, May 28, 2024 at 05:49:19PM +0200, Andrea Bolognani wrote:
> The current implementation requires users to configure the
> preference as such:
>
> -Dfirewall_backend_default_1=iptables
> -Dfirewall_backend_default_2=nftables
>
> In addition to being more verbose than one would hope, there
> are several things that could go wrong.
>
> First of all, meson performs no validation on the provided
> values, so mistakes will only be caught by the compiler.
> Additionally, it's entirely possible to provide nonsensical
> combinations, such as repeating the same value twice.
>
> Change things so that the preference can now be configured
> as such:
>
> -Dfirewall_backend_priority=iptables,nftables
>
> Checks have been added to prevent invalid values from being
> accepted.
>
> Signed-off-by: Andrea Bolognani
> ---
> meson.build | 16 +---
> meson_options.txt| 3 +--
> src/network/bridge_driver_conf.c | 6 +-
> src/network/meson.build | 6 --
> src/network/network.conf.in | 13 +++--
> 5 files changed, 26 insertions(+), 18 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index f67c3d2724..ed0e9686f8 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1635,15 +1635,17 @@ endif
>
> if not get_option('driver_network').disabled() and conf.has('WITH_LIBVIRTD')
>conf.set('WITH_NETWORK', 1)
> - firewall_backend_default_1 = get_option('firewall_backend_default_1')
> - firewall_backend_default_conf = firewall_backend_default_1
> - firewall_backend_default_1 = 'VIR_FIREWALL_BACKEND_' +
> firewall_backend_default_1.to_upper()
> - conf.set('FIREWALL_BACKEND_DEFAULT_1', firewall_backend_default_1)
>
> - firewall_backend_default_2 = get_option('firewall_backend_default_2')
> - firewall_backend_default_2 = 'VIR_FIREWALL_BACKEND_' +
> firewall_backend_default_2.to_upper()
> - conf.set('FIREWALL_BACKEND_DEFAULT_2', firewall_backend_default_2)
> + firewall_backend_priority = get_option('firewall_backend_priority')
> + if (not firewall_backend_priority.contains('nftables') or
> + not firewall_backend_priority.contains('iptables') or
> + firewall_backend_priority.length() != 2)
No need to have a check for specific values. Meson will already check if
they are from the list of choices defined in meson_options.txt .
> +error('invalid value for firewall_backend_priority option')
> + endif
>
> + conf.set('FIREWALL_BACKEND_PRIORITY_0', 'VIR_FIREWALL_BACKEND_' +
> firewall_backend_priority[0].to_upper())
> + conf.set('FIREWALL_BACKEND_PRIORITY_1', 'VIR_FIREWALL_BACKEND_' +
> firewall_backend_priority[1].to_upper())
> + conf.set('FIREWALL_BACKEND_PRIORITY_NUM',
> firewall_backend_priority.length())
> elif get_option('driver_network').enabled()
>error('libvirtd must be enabled to build the network driver')
> endif
> diff --git a/meson_options.txt b/meson_options.txt
> index ad354a8668..8723d13231 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -115,8 +115,7 @@ option('dtrace', type: 'feature', value: 'auto',
> description: 'use dtrace for st
> option('firewalld', type: 'feature', value: 'auto', description: 'firewalld
> support')
> # dep:firewalld
> option('firewalld_zone', type: 'feature', value: 'auto', description:
> 'whether to install firewalld libvirt zone')
> -option('firewall_backend_default_1', type: 'string', value: 'nftables',
> description: 'first firewall backend to try when none is specified')
> -option('firewall_backend_default_2', type: 'string', value: 'iptables',
> description: 'second firewall backend to try when none is specified (and
> first is unavailable)')
> +option('firewall_backend_priority', type: 'array', choices: ['nftables',
> 'iptables'], description: 'firewall backends to try, preferred ones first')
What about "order of firewall backends to try"? The part "preferred ones
first" sounds strange to me.
Otherwise looks good.
Pavel
signature.asc
Description: PGP signature
Re: [PATCH 0/3] meson: Improve default firewall backend configuration
On 5/28/24 11:49 AM, Andrea Bolognani wrote: Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/1308917840 See 1/3 for details. Andrea Bolognani (3): meson: Improve default firewall backend configuration meson: Include firewall backend selection in summary rpm: Configure firewall backends explicitly Reviewed-by: Laine Stump Thanks for doing this - I had put "make the backend default selection better" on a *mental* list, but not a physical list, and then I forgot (easy to forget because doing things in a meson build file is for me an exercise in poking at it until it works correctly). I've tried it out and it works as advertised. I would prefer to categorize this as a bugfix and push it before the release (so that we don't have the build-time config different in this release than in later releases)
Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling
On Tue, May 28, 2024 at 09:06:04AM +, Gonglei (Arei) wrote: > Hi Peter, > > > -Original Message- > > From: Peter Xu [mailto:pet...@redhat.com] > > Sent: Wednesday, May 22, 2024 6:15 AM > > To: Yu Zhang > > Cc: Michael Galaxy ; Jinpu Wang > > ; Elmar Gerdes ; > > zhengchuan ; Gonglei (Arei) > > ; Daniel P. Berrangé ; > > Markus Armbruster ; Zhijian Li (Fujitsu) > > ; qemu-de...@nongnu.org; Yuval Shaia > > ; Kevin Wolf ; Prasanna > > Kumar Kalever ; Cornelia Huck > > ; Michael Roth ; Prasanna > > Kumar Kalever ; Paolo Bonzini > > ; qemu-bl...@nongnu.org; devel@lists.libvirt.org; > > Hanna Reitz ; Michael S. Tsirkin ; > > Thomas Huth ; Eric Blake ; Song > > Gao ; Marc-André Lureau > > ; Alex Bennée ; > > Wainer dos Santos Moschetta ; Beraldo Leal > > ; Pannengyuan ; > > Xiexiangyou ; Fabiano Rosas > > Subject: Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling > > > > On Fri, May 17, 2024 at 03:01:59PM +0200, Yu Zhang wrote: > > > Hello Michael and Peter, > > > > Hi, > > > > > > > > Exactly, not so compelling, as I did it first only on servers widely > > > used for production in our data center. The network adapters are > > > > > > Ethernet controller: Broadcom Inc. and subsidiaries NetXtreme BCM5720 > > > 2-port Gigabit Ethernet PCIe > > > > Hmm... I definitely thinks Jinpu's Mellanox ConnectX-6 looks more > > reasonable. > > > > https://lore.kernel.org/qemu-devel/CAMGffEn-DKpMZ4tA71MJYdyemg0Zda15 > > wvaqk81vxtkzx-l...@mail.gmail.com/ > > > > Appreciate a lot for everyone helping on the testings. > > > > > InfiniBand controller: Mellanox Technologies MT27800 Family > > > [ConnectX-5] > > > > > > which doesn't meet our purpose. I can choose RDMA or TCP for VM > > > migration. RDMA traffic is through InfiniBand and TCP through Ethernet > > > on these two hosts. One is standby while the other is active. > > > > > > Now I'll try on a server with more recent Ethernet and InfiniBand > > > network adapters. One of them has: > > > BCM57414 NetXtreme-E 10Gb/25Gb RDMA Ethernet Controller (rev 01) > > > > > > The comparison between RDMA and TCP on the same NIC could make more > > sense. > > > > It looks to me NICs are powerful now, but again as I mentioned I don't > > think it's > > a reason we need to deprecate rdma, especially if QEMU's rdma migration has > > the chance to be refactored using rsocket. > > > > Is there anyone who started looking into that direction? Would it make > > sense > > we start some PoC now? > > > > My team has finished the PoC refactoring which works well. > > Progress: > 1. Implement io/channel-rdma.c, > 2. Add unit test tests/unit/test-io-channel-rdma.c and verifying it is > successful, > 3. Remove the original code from migration/rdma.c, > 4. Rewrite the rdma_start_outgoing_migration and > rdma_start_incoming_migration logic, > 5. Remove all rdma_xxx functions from migration/ram.c. (to prevent RDMA live > migration from polluting the core logic of live migration), > 6. The soft-RoCE implemented by software is used to test the RDMA live > migration. It's successful. > > We will be submit the patchset later. That's great news, thank you! -- Peter Xu
[PATCH 3/3] rpm: Configure firewall backends explicitly
Signed-off-by: Andrea Bolognani
---
libvirt.spec.in | 1 +
1 file changed, 1 insertion(+)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 6fb223c74a..4381dbe30c 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1387,6 +1387,7 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y'
%{_specdir}/libvirt.spec)
%{?enable_werror} \
-Dexpensive_tests=enabled \
-Dinit_script=systemd \
+ -Dfirewall_backend_priority=nftables,iptables \
-Ddocs=enabled \
-Dtests=enabled \
-Drpath=disabled \
--
2.45.1
[PATCH 2/3] meson: Include firewall backend selection in summary
Signed-off-by: Andrea Bolognani
---
meson.build | 5 +
1 file changed, 5 insertions(+)
diff --git a/meson.build b/meson.build
index ed0e9686f8..e98ab0d5ac 100644
--- a/meson.build
+++ b/meson.build
@@ -2382,6 +2382,11 @@ misc_summary = {
'sysctl config': conf.has('WITH_SYSCTL'),
'userfaultfd sysctl': conf.has('WITH_USERFAULTFD_SYSCTL'),
}
+if conf.has('WITH_NETWORK')
+ misc_summary += {
+'firewall backends': firewall_backend_priority,
+ }
+endif
summary(misc_summary, section: 'Miscellaneous', bool_yn: true, list_sep: ' ')
devtools_summary = {
--
2.45.1
[PATCH 1/3] meson: Improve default firewall backend configuration
The current implementation requires users to configure the
preference as such:
-Dfirewall_backend_default_1=iptables
-Dfirewall_backend_default_2=nftables
In addition to being more verbose than one would hope, there
are several things that could go wrong.
First of all, meson performs no validation on the provided
values, so mistakes will only be caught by the compiler.
Additionally, it's entirely possible to provide nonsensical
combinations, such as repeating the same value twice.
Change things so that the preference can now be configured
as such:
-Dfirewall_backend_priority=iptables,nftables
Checks have been added to prevent invalid values from being
accepted.
Signed-off-by: Andrea Bolognani
---
meson.build | 16 +---
meson_options.txt| 3 +--
src/network/bridge_driver_conf.c | 6 +-
src/network/meson.build | 6 --
src/network/network.conf.in | 13 +++--
5 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/meson.build b/meson.build
index f67c3d2724..ed0e9686f8 100644
--- a/meson.build
+++ b/meson.build
@@ -1635,15 +1635,17 @@ endif
if not get_option('driver_network').disabled() and conf.has('WITH_LIBVIRTD')
conf.set('WITH_NETWORK', 1)
- firewall_backend_default_1 = get_option('firewall_backend_default_1')
- firewall_backend_default_conf = firewall_backend_default_1
- firewall_backend_default_1 = 'VIR_FIREWALL_BACKEND_' +
firewall_backend_default_1.to_upper()
- conf.set('FIREWALL_BACKEND_DEFAULT_1', firewall_backend_default_1)
- firewall_backend_default_2 = get_option('firewall_backend_default_2')
- firewall_backend_default_2 = 'VIR_FIREWALL_BACKEND_' +
firewall_backend_default_2.to_upper()
- conf.set('FIREWALL_BACKEND_DEFAULT_2', firewall_backend_default_2)
+ firewall_backend_priority = get_option('firewall_backend_priority')
+ if (not firewall_backend_priority.contains('nftables') or
+ not firewall_backend_priority.contains('iptables') or
+ firewall_backend_priority.length() != 2)
+error('invalid value for firewall_backend_priority option')
+ endif
+ conf.set('FIREWALL_BACKEND_PRIORITY_0', 'VIR_FIREWALL_BACKEND_' +
firewall_backend_priority[0].to_upper())
+ conf.set('FIREWALL_BACKEND_PRIORITY_1', 'VIR_FIREWALL_BACKEND_' +
firewall_backend_priority[1].to_upper())
+ conf.set('FIREWALL_BACKEND_PRIORITY_NUM', firewall_backend_priority.length())
elif get_option('driver_network').enabled()
error('libvirtd must be enabled to build the network driver')
endif
diff --git a/meson_options.txt b/meson_options.txt
index ad354a8668..8723d13231 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -115,8 +115,7 @@ option('dtrace', type: 'feature', value: 'auto',
description: 'use dtrace for st
option('firewalld', type: 'feature', value: 'auto', description: 'firewalld
support')
# dep:firewalld
option('firewalld_zone', type: 'feature', value: 'auto', description: 'whether
to install firewalld libvirt zone')
-option('firewall_backend_default_1', type: 'string', value: 'nftables',
description: 'first firewall backend to try when none is specified')
-option('firewall_backend_default_2', type: 'string', value: 'iptables',
description: 'second firewall backend to try when none is specified (and first
is unavailable)')
+option('firewall_backend_priority', type: 'array', choices: ['nftables',
'iptables'], description: 'firewall backends to try, preferred ones first')
option('host_validate', type: 'feature', value: 'auto', description: 'build
virt-host-validate')
option('init_script', type: 'combo', choices: ['systemd', 'openrc', 'check',
'none'], value: 'check', description: 'Style of init script to install')
option('loader_nvram', type: 'string', value: '', description: 'Pass list of
pairs of : paths. Both pairs and list items are separated by a
colon.')
diff --git a/src/network/bridge_driver_conf.c b/src/network/bridge_driver_conf.c
index 8f4956dace..e2f3613a41 100644
--- a/src/network/bridge_driver_conf.c
+++ b/src/network/bridge_driver_conf.c
@@ -67,8 +67,12 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg
G_GNUC_UNUSED,
g_autofree char *fwBackendStr = NULL;
bool fwBackendSelected = false;
size_t i;
-int fwBackends[] = { FIREWALL_BACKEND_DEFAULT_1,
FIREWALL_BACKEND_DEFAULT_2 };
+int fwBackends[] = {
+FIREWALL_BACKEND_PRIORITY_0,
+FIREWALL_BACKEND_PRIORITY_1,
+};
G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) == VIR_FIREWALL_BACKEND_LAST);
+G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) == FIREWALL_BACKEND_PRIORITY_NUM);
int nFwBackends = G_N_ELEMENTS(fwBackends);
if (access(filename, R_OK) == 0) {
diff --git a/src/network/meson.build b/src/network/meson.build
index bf2893accc..07cd5cda55 100644
--- a/src/network/meson.build
+++ b/src/network/meson.build
@@ -51,7 +51,8 @@ if conf.has('WITH_NETWORK')
}
network_options_conf = configuration_data({
-'FIREWALL_BACKEND':
[PATCH 0/3] meson: Improve default firewall backend configuration
Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/1308917840 See 1/3 for details. Andrea Bolognani (3): meson: Improve default firewall backend configuration meson: Include firewall backend selection in summary rpm: Configure firewall backends explicitly libvirt.spec.in | 1 + meson.build | 21 ++--- meson_options.txt| 3 +-- src/network/bridge_driver_conf.c | 6 +- src/network/meson.build | 6 -- src/network/network.conf.in | 13 +++-- 6 files changed, 32 insertions(+), 18 deletions(-) -- 2.45.1
Re: [PATCH] virsh: Provide completer for some pool-X-as commands
On Mon, May 27, 2024 at 18:34:38 +, Abhiram Tilak wrote:
> Provides completers for auth-type and source-format commands for
> virsh pool-create-as and pool-define-as commands. Use Empty completers
> for options where completions are not required. I left the ones where
> I was not sure if they need completers.
>
> Related Issue: https://gitlab.com/libvirt/libvirt/-/issues/9
> Signed-off-by: Abhiram Tilak
> ---
> src/libvirt_private.syms | 2 ++
> tools/virsh-completer-pool.c | 48 +++-
> tools/virsh-completer-pool.h | 10
> tools/virsh-pool.c | 8 ++
> 4 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 6b6bcc368a..5a413ca832 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1117,6 +1117,8 @@ virStorageAuthDefCopy;
> virStorageAuthDefFormat;
> virStorageAuthDefFree;
> virStorageAuthDefParse;
> +virStorageAuthTypeFromString;
> +virStorageAuthTypeToString;
> virStorageFileFeatureTypeFromString;
> virStorageFileFeatureTypeToString;
> virStorageFileFormatTypeFromString;
> diff --git a/tools/virsh-completer-pool.c b/tools/virsh-completer-pool.c
> index 3568bb985b..4a771d894e 100644
> --- a/tools/virsh-completer-pool.c
> +++ b/tools/virsh-completer-pool.c
> @@ -84,7 +84,6 @@ virshPoolEventNameCompleter(vshControl *ctl G_GNUC_UNUSED,
> return g_steal_pointer();
> }
>
> -
Spurious whitespace change.
> char **
> virshPoolTypeCompleter(vshControl *ctl,
> const vshCmd *cmd,
> @@ -106,3 +105,50 @@ virshPoolTypeCompleter(vshControl *ctl,
>
> return virshCommaStringListComplete(type_str, (const char **)tmp);
> }
> +
Two empty lines between functions in this file.
> +char **
> +virshPoolFormatCompleter(vshControl *ctl G_GNUC_UNUSED,
> + const vshCmd *cmd G_GNUC_UNUSED,
> + unsigned int flags)
> +{
> +size_t i = 0, j = 0;
One declaration per line please;
> +g_auto(GStrv) tmp = NULL;
> +size_t nformats = VIR_STORAGE_POOL_FS_LAST + VIR_STORAGE_POOL_NETFS_LAST
> +
> + VIR_STORAGE_POOL_DISK_LAST +
> VIR_STORAGE_POOL_LOGICAL_LAST;
> +
> +virCheckFlags(0, NULL);
> +
> +tmp = g_new0(char *, nformats + 1);
> +
> +/* Club all PoolFormats for completion */
> +for (i = 0; i < VIR_STORAGE_POOL_FS_LAST; i++)
> +tmp[j++] = g_strdup(virStoragePoolFormatFileSystemTypeToString(i));
> +
> +for (i = 0; i < VIR_STORAGE_POOL_NETFS_LAST; i++)
> +tmp[j++] =
> g_strdup(virStoragePoolFormatFileSystemNetTypeToString(i));
> +
> +for (i = 0; i < VIR_STORAGE_POOL_DISK_LAST; i++)
> +tmp[j++] = g_strdup(virStoragePoolFormatDiskTypeToString(i));
I don't think it makes sense to complete the "unknown" value here ...
> +
> +for (i = 0; i < VIR_STORAGE_POOL_LOGICAL_LAST; i++)
> +tmp[j++] = g_strdup(virStoragePoolFormatLogicalTypeToString(i));
... and here.
> +
> +return g_steal_pointer();
> +}
> +
> +char ** virshPoolAuthTypeCompleter(vshControl *ctl G_GNUC_UNUSED,
> + const vshCmd *cmd G_GNUC_UNUSED,
> + unsigned int flags)
Wrong header formatting style, and two lines between functions please.
> +{
> +size_t i = 0;
> +g_auto(GStrv) tmp = NULL;
> +
> +virCheckFlags(0, NULL);
> +
> +tmp = g_new0(char *, VIR_STORAGE_AUTH_TYPE_LAST + 1);
> +
> +for (i = 0; i < VIR_STORAGE_AUTH_TYPE_LAST; i++)
> +tmp[i] = g_strdup(virStorageAuthTypeToString(i));
> +
> +return g_steal_pointer();
> +}
> diff --git a/tools/virsh-completer-pool.h b/tools/virsh-completer-pool.h
> index bff3e5742b..4a53f99577 100644
> --- a/tools/virsh-completer-pool.h
> +++ b/tools/virsh-completer-pool.h
> @@ -40,3 +40,13 @@ char **
> virshPoolTypeCompleter(vshControl *ctl,
> const vshCmd *cmd,
> unsigned int flags);
> +
> +char **
> +virshPoolFormatCompleter(vshControl *ctl,
> + const vshCmd *cmd,
> + unsigned int flags);
> +
> +char **
> +virshPoolAuthTypeCompleter(vshControl *ctl,
> + const vshCmd *cmd,
> + unsigned int flags);
> diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
> index 67cc1b94cf..1a294bb0f6 100644
> --- a/tools/virsh-pool.c
> +++ b/tools/virsh-pool.c
> @@ -81,26 +81,32 @@
> }, \
> {.name = "source-path", \
> .type = VSH_OT_STRING, \
> + .completer = virshCompletePathLocalExisting, \
> .help = N_("source path for underlying storage") \
> }, \
> {.name = "source-dev", \
> .type = VSH_OT_STRING, \
> + .completer = virshCompleteEmpty, \
This is taking the path to a block device, so this should complete
existing filenames.
> .help = N_("source device for underlying storage") \
> }, \
> {.name = "source-name", \
> .type
Re: [PATCH] virsh: Provide completer for some pool-X-as commands
This patch was considerably old (like 10 days), it got stuck in the mailing
list and
had to resend this. I didn't rebase this to master before sending, hence
the conflicts.
On Tue, 28 May 2024 at 00:06, Abhiram Tilak wrote:
> Provides completers for auth-type and source-format commands for
> virsh pool-create-as and pool-define-as commands. Use Empty completers
> for options where completions are not required. I left the ones where
> I was not sure if they need completers.
>
> Related Issue: https://gitlab.com/libvirt/libvirt/-/issues/9
> Signed-off-by: Abhiram Tilak
> ---
> src/libvirt_private.syms | 2 ++
> tools/virsh-completer-pool.c | 48 +++-
> tools/virsh-completer-pool.h | 10
> tools/virsh-pool.c | 8 ++
> 4 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 6b6bcc368a..5a413ca832 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1117,6 +1117,8 @@ virStorageAuthDefCopy;
> virStorageAuthDefFormat;
> virStorageAuthDefFree;
> virStorageAuthDefParse;
> +virStorageAuthTypeFromString;
> +virStorageAuthTypeToString;
> virStorageFileFeatureTypeFromString;
> virStorageFileFeatureTypeToString;
> virStorageFileFormatTypeFromString;
> diff --git a/tools/virsh-completer-pool.c b/tools/virsh-completer-pool.c
> index 3568bb985b..4a771d894e 100644
> --- a/tools/virsh-completer-pool.c
> +++ b/tools/virsh-completer-pool.c
> @@ -84,7 +84,6 @@ virshPoolEventNameCompleter(vshControl *ctl
> G_GNUC_UNUSED,
> return g_steal_pointer();
> }
>
> -
> char **
> virshPoolTypeCompleter(vshControl *ctl,
> const vshCmd *cmd,
> @@ -106,3 +105,50 @@ virshPoolTypeCompleter(vshControl *ctl,
>
> return virshCommaStringListComplete(type_str, (const char **)tmp);
> }
> +
> +char **
> +virshPoolFormatCompleter(vshControl *ctl G_GNUC_UNUSED,
> + const vshCmd *cmd G_GNUC_UNUSED,
> + unsigned int flags)
> +{
> +size_t i = 0, j = 0;
> +g_auto(GStrv) tmp = NULL;
> +size_t nformats = VIR_STORAGE_POOL_FS_LAST +
> VIR_STORAGE_POOL_NETFS_LAST +
> + VIR_STORAGE_POOL_DISK_LAST +
> VIR_STORAGE_POOL_LOGICAL_LAST;
> +
> +virCheckFlags(0, NULL);
> +
> +tmp = g_new0(char *, nformats + 1);
> +
> +/* Club all PoolFormats for completion */
> +for (i = 0; i < VIR_STORAGE_POOL_FS_LAST; i++)
> +tmp[j++] =
> g_strdup(virStoragePoolFormatFileSystemTypeToString(i));
> +
> +for (i = 0; i < VIR_STORAGE_POOL_NETFS_LAST; i++)
> +tmp[j++] =
> g_strdup(virStoragePoolFormatFileSystemNetTypeToString(i));
> +
> +for (i = 0; i < VIR_STORAGE_POOL_DISK_LAST; i++)
> +tmp[j++] = g_strdup(virStoragePoolFormatDiskTypeToString(i));
> +
> +for (i = 0; i < VIR_STORAGE_POOL_LOGICAL_LAST; i++)
> +tmp[j++] = g_strdup(virStoragePoolFormatLogicalTypeToString(i));
> +
> +return g_steal_pointer();
> +}
> +
> +char ** virshPoolAuthTypeCompleter(vshControl *ctl G_GNUC_UNUSED,
> + const vshCmd *cmd G_GNUC_UNUSED,
> + unsigned int flags)
> +{
> +size_t i = 0;
> +g_auto(GStrv) tmp = NULL;
> +
> +virCheckFlags(0, NULL);
> +
> +tmp = g_new0(char *, VIR_STORAGE_AUTH_TYPE_LAST + 1);
> +
> +for (i = 0; i < VIR_STORAGE_AUTH_TYPE_LAST; i++)
> +tmp[i] = g_strdup(virStorageAuthTypeToString(i));
> +
> +return g_steal_pointer();
> +}
> diff --git a/tools/virsh-completer-pool.h b/tools/virsh-completer-pool.h
> index bff3e5742b..4a53f99577 100644
> --- a/tools/virsh-completer-pool.h
> +++ b/tools/virsh-completer-pool.h
> @@ -40,3 +40,13 @@ char **
> virshPoolTypeCompleter(vshControl *ctl,
> const vshCmd *cmd,
> unsigned int flags);
> +
> +char **
> +virshPoolFormatCompleter(vshControl *ctl,
> + const vshCmd *cmd,
> + unsigned int flags);
> +
> +char **
> +virshPoolAuthTypeCompleter(vshControl *ctl,
> + const vshCmd *cmd,
> + unsigned int flags);
> diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
> index 67cc1b94cf..1a294bb0f6 100644
> --- a/tools/virsh-pool.c
> +++ b/tools/virsh-pool.c
> @@ -81,26 +81,32 @@
> }, \
> {.name = "source-path", \
> .type = VSH_OT_STRING, \
> + .completer = virshCompletePathLocalExisting, \
> .help = N_("source path for underlying storage") \
> }, \
> {.name = "source-dev", \
> .type = VSH_OT_STRING, \
> + .completer = virshCompleteEmpty, \
> .help = N_("source device for underlying storage") \
> }, \
> {.name = "source-name", \
> .type = VSH_OT_STRING, \
> + .completer = virshCompleteEmpty, \
> .help = N_("source name for underlying storage") \
> }, \
> {.name = "target", \
> .type = VSH_OT_STRING, \
> +
Re: [PATCH 1/3] tests: Add TPM coverage to default-models tests
On Tue, May 28, 2024 at 08:59:46AM GMT, Peter Krempa wrote: > On Mon, May 27, 2024 at 19:31:34 +0200, Andrea Bolognani wrote: > > Note that TPM supports seems to be currently missing from s390x > > and loongarch64 QEMU builds. I'm not entirely sure whether > > there's a good reason for that or it's simply an oversight, but > > either way we have to skip them for now. > > I presume you mean that TPM support was not built into the QEMU builds > used to capture the capability dumps, right? That's just the thing: I don't know :) In both cases the default configuration disables it, but it's unclear to me whether that is something that could be addressed with a simple patch or there are factors at play that make TPM inherently incompatible with these architectures. > > +++ b/tests/qemuxmlconfdata/s390x-ccw-default-models.xml > > @@ -14,6 +14,7 @@ > > > > > > > > + For s390x, Thomas has confirmed that TPM is just not a thing and will never be a thing, so while the comment is technically accurate I agree that stronger language should be used to reflect the situation. Thomas, please correct me if I got this wrong :) > > +++ b/tests/qemuxmlconfdata/loongarch64-virt-default-models.xml > > @@ -14,6 +14,7 @@ > > > > > > > > + For loongarch64, maybe Xianglai Li can provide some insight. Is TPM something that could be flipped on at the QEMU level? -- Andrea Bolognani / Red Hat / Virtualization
[PATCH v2] run.in: Detect binaries in builddir properly
When attempting to run:
libvirt.git/_build # ./run --selinux ./src/libvirtd
the following error is thrown:
Refusing to change selinux context of file './src/libvirtd' outside build
directory
which is obviously wrong. The problem is 'being inside of build
directory' is detected by simple progpath.startswith(builddir).
While builddir is an absolute path, progpath isn't necessarily.
And while looking into the code, I've noticed chcon() function
accessing variable outside its scope when printing out the path
it's working on.
Signed-off-by: Michal Privoznik
---
v2 of:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/UZ6F7MWEJBUMUUBODXGAXQW4NY2UEEVF/
diff to v1:
- error out if binary to run can't be identified (i.e. 'which' returns
None).
run.in | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/run.in b/run.in
index 5b89b3dcd5..cada74dfcd 100644
--- a/run.in
+++ b/run.in
@@ -138,7 +138,7 @@ def change_unit(name, action):
def chcon(path, user, role, type):
-print("Setting file context of {} to u={}, r={}, t={}...".format(progpath,
+print("Setting file context of {} to u={}, r={}, t={}...".format(path,
user,
role,
type))
@@ -187,6 +187,10 @@ else:
try:
dorestorecon = False
progpath = shutil.which(prog)
+if not progpath:
+raise Exception("Can't find executable {}"
+.format(prog))
+progpath = os.path.abspath(progpath)
if len(try_stop_units):
print("Temporarily stopping systemd units...")
--
2.44.1
Entering freeze for libvirt-10.4.0
I have just tagged v10.4.0-rc1 in the repository and pushed signed tarballs to https://download.libvirt.org/ 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] qemucapabilitiestest: Add test data for qemu-9.1 dev cycle
On Mon, May 27, 2024 at 16:26:44 +0200, Peter Krempa wrote: > Add test data based on qemu commit v9.0.0-995-g60b54b67c6 on x86_64 > > Comparison to previous release: > > Feature additions: > - 9.1 machine type added > - 'SierraForest' cpu type added > - 'SapphireRapids-v3-x86_64-cpu' added > - 'VFIO_MIGRATION' event added (and corresponding 'migration-events' >bool for the device > - 'exit-on-error' argument for 'migrate-incoming' added > - 'sev-guest' gained 'legacy-vm-type' boolean > - cpu topology added 'module' fields > - 'query-machines' gained 'compat-props' boolean and it returns various The end of the sentence is missing here. > - 'deprecated-props' argument for 'query-cpu-model-expansion' added > > Deprecated removals: > - legacy non-shared-storage migration fully removed (config/stats) > - legacy migration compression fully removed > - RDMA support removed > - dropped 'nios2' field type from 'query-cpus-fast' return data >fields under 'compat-props' > > Note that this dump was done on a newer kernel version which resulted in > the 'pcommit' feature being removed from the few test cases which depend > on the real CPU flag dump. > > Signed-off-by: Peter Krempa > --- > .../domaincapsdata/qemu_9.1.0-q35.x86_64.xml | 290 + > .../domaincapsdata/qemu_9.1.0-tcg.x86_64.xml | 289 + > tests/domaincapsdata/qemu_9.1.0.x86_64.xml| 290 + > .../caps_9.1.0_x86_64.replies | 43370 > .../caps_9.1.0_x86_64.xml | 3946 ++ > ...host-model-fallback-kvm.x86_64-latest.args | 2 +- > ...host-model-fallback-tcg.x86_64-latest.args | 2 +- > ...cpu-host-model-features.x86_64-latest.args | 2 +- > .../cpu-host-model-kvm.x86_64-latest.args | 2 +- > ...st-model-nofallback-kvm.x86_64-latest.args | 2 +- > ...st-model-nofallback-tcg.x86_64-latest.args | 2 +- > .../cpu-host-model-tcg.x86_64-latest.args | 2 +- > 12 files changed, 48192 insertions(+), 7 deletions(-) > create mode 100644 tests/domaincapsdata/qemu_9.1.0-q35.x86_64.xml > create mode 100644 tests/domaincapsdata/qemu_9.1.0-tcg.x86_64.xml > create mode 100644 tests/domaincapsdata/qemu_9.1.0.x86_64.xml > create mode 100644 tests/qemucapabilitiesdata/caps_9.1.0_x86_64.replies > create mode 100644 tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml Reviewed-by: Jiri Denemark
Re: [PATCH 2/3] tests: Delete some redundant test cases
On Mon, May 27, 2024 at 19:31:35 +0200, Andrea Bolognani wrote: > The default-models tests provide coverage for these scenarios > now. > > Signed-off-by: Andrea Bolognani > --- > .../aarch64-tpm.aarch64-latest.args | 34 - > .../aarch64-tpm.aarch64-latest.xml| 29 > tests/qemuxmlconfdata/aarch64-tpm.xml | 15 > .../tpm-emulator-spapr.ppc64-latest.args | 45 > .../tpm-emulator-spapr.ppc64-latest.xml | 1 - > tests/qemuxmlconfdata/tpm-emulator-spapr.xml | 70 --- > tests/qemuxmlconftest.c | 2 - > 7 files changed, 196 deletions(-) > delete mode 100644 tests/qemuxmlconfdata/aarch64-tpm.aarch64-latest.args > delete mode 100644 tests/qemuxmlconfdata/aarch64-tpm.aarch64-latest.xml > delete mode 100644 tests/qemuxmlconfdata/aarch64-tpm.xml > delete mode 100644 tests/qemuxmlconfdata/tpm-emulator-spapr.ppc64-latest.args > delete mode 12 tests/qemuxmlconfdata/tpm-emulator-spapr.ppc64-latest.xml > delete mode 100644 tests/qemuxmlconfdata/tpm-emulator-spapr.xml Reviewed-by: Peter Krempa
Re: [PATCH 1/3] tests: Add TPM coverage to default-models tests
On Mon, May 27, 2024 at 19:31:34 +0200, Andrea Bolognani wrote: > We have a non-trivial amount of architecture-specific logic > dealing with TPM, so it's good to have coverage for it. > > Note that TPM supports seems to be currently missing from s390x > and loongarch64 QEMU builds. I'm not entirely sure whether I presume you mean that TPM support was not built into the QEMU builds used to capture the capability dumps, right? > there's a good reason for that or it's simply an oversight, but > either way we have to skip them for now. > > Signed-off-by: Andrea Bolognani > --- > ...aarch64-virt-default-models.aarch64-latest.abi-update.args | 3 +++ > .../aarch64-virt-default-models.aarch64-latest.abi-update.xml | 3 +++ > .../aarch64-virt-default-models.aarch64-latest.args | 3 +++ > .../aarch64-virt-default-models.aarch64-latest.xml| 3 +++ > tests/qemuxmlconfdata/aarch64-virt-default-models.xml | 3 +++ > tests/qemuxmlconfdata/loongarch64-virt-default-models.xml | 1 + > .../ppc64-pseries-default-models.ppc64-latest.abi-update.args | 3 +++ > .../ppc64-pseries-default-models.ppc64-latest.abi-update.xml | 4 > .../ppc64-pseries-default-models.ppc64-latest.args| 3 +++ > .../ppc64-pseries-default-models.ppc64-latest.xml | 4 > tests/qemuxmlconfdata/ppc64-pseries-default-models.xml| 3 +++ > ...riscv64-virt-default-models.riscv64-latest.abi-update.args | 3 +++ > .../riscv64-virt-default-models.riscv64-latest.abi-update.xml | 3 +++ > .../riscv64-virt-default-models.riscv64-latest.args | 3 +++ > .../riscv64-virt-default-models.riscv64-latest.xml| 3 +++ > tests/qemuxmlconfdata/riscv64-virt-default-models.xml | 3 +++ > tests/qemuxmlconfdata/s390x-ccw-default-models.xml| 1 + > .../x86_64-pc-default-models.x86_64-latest.abi-update.args| 3 +++ > .../x86_64-pc-default-models.x86_64-latest.abi-update.xml | 3 +++ > .../x86_64-pc-default-models.x86_64-latest.args | 3 +++ > .../x86_64-pc-default-models.x86_64-latest.xml| 3 +++ > tests/qemuxmlconfdata/x86_64-pc-default-models.xml| 3 +++ > .../x86_64-q35-default-models.x86_64-latest.abi-update.args | 3 +++ > .../x86_64-q35-default-models.x86_64-latest.abi-update.xml| 3 +++ > .../x86_64-q35-default-models.x86_64-latest.args | 3 +++ > .../x86_64-q35-default-models.x86_64-latest.xml | 3 +++ > tests/qemuxmlconfdata/x86_64-q35-default-models.xml | 3 +++ > 27 files changed, 79 insertions(+) [...] > diff --git a/tests/qemuxmlconfdata/loongarch64-virt-default-models.xml > b/tests/qemuxmlconfdata/loongarch64-virt-default-models.xml > index 109fb3b3ea..e59ebdeed4 100644 > --- a/tests/qemuxmlconfdata/loongarch64-virt-default-models.xml > +++ b/tests/qemuxmlconfdata/loongarch64-virt-default-models.xml > @@ -14,6 +14,7 @@ > > > > + > > [...] > diff --git a/tests/qemuxmlconfdata/s390x-ccw-default-models.xml > b/tests/qemuxmlconfdata/s390x-ccw-default-models.xml > index a196129628..dd97349e6f 100644 > --- a/tests/qemuxmlconfdata/s390x-ccw-default-models.xml > +++ b/tests/qemuxmlconfdata/s390x-ccw-default-models.xml > @@ -14,6 +14,7 @@ > > > > + Both the commit message and this comment isn't really clear whether TPM is not supported by the platform itself or just was omitted when building qemu which was used for the caps dump. I think it will be even harder to understand this for anyone who finds this comment later. I suggest: TPM was not compiled into the QEMU binary used for the capability dump, but platform supports it Or something similar more clear. Reviewed-by: Peter Krempa
