Re: [libvirt PATCH] tests: Only mock $INODE64 symbols on x86_64 macOS
On Fri, Feb 05, 2021 at 11:27:56AM +0100, Andrea Bolognani wrote: > The version of macOS running on Apple Silicon doesn't need to > concern itself with backwards compatibility with 32-bit > applications, and so it could jettison all the symbol aliasing > shenanigans involved. > > https://gitlab.com/libvirt/libvirt/-/issues/121 > > Signed-off-by: Andrea Bolognani > --- > tests/virfilewrapper.c | 2 +- > tests/virmockstathelpers.c | 4 ++-- > tests/virpcimock.c | 2 +- > 3 files changed, 4 insertions(+), 4 deletions(-) > Hi Andrea, Reviewed-by: Roman Bolshakov Tested-by: Roman Bolshakov Thanks, Roman
[PATCH] virStorageSourceClear: Unref @vhostuser
The @vhostuser member of virStorageSource structure is allocated during parsing in virDomainDiskSourceVHostUserParse() but never freed leading to a memleak. Since the member is an object it has to be unrefed instead of g_free()-d. Signed-off-by: Michal Privoznik --- src/conf/storage_source_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c index 7706bbd8da..67c3aedefb 100644 --- a/src/conf/storage_source_conf.c +++ b/src/conf/storage_source_conf.c @@ -1125,6 +1125,7 @@ virStorageSourceClear(virStorageSourcePtr def) virStorageEncryptionFree(def->encryption); virStoragePRDefFree(def->pr); virStorageSourceNVMeDefFree(def->nvme); +virObjectUnref(def->vhostuser); virStorageSourceSeclabelsClear(def); virStoragePermsFree(def->perms); VIR_FREE(def->timestamps); -- 2.26.2
Re: [PATCH] virStorageSourceClear: Unref @vhostuser
On Wed, Feb 10, 2021 at 10:52:43AM +0100, Michal Privoznik wrote: > The @vhostuser member of virStorageSource structure is allocated > during parsing in virDomainDiskSourceVHostUserParse() but never > freed leading to a memleak. Since the member is an object it has > to be unrefed instead of g_free()-d. > > Signed-off-by: Michal Privoznik > --- > src/conf/storage_source_conf.c | 1 + > 1 file changed, 1 insertion(+) Oops, thanks for fixing it. Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
Re: [libvirt PATCH 1/5] conf: define a new "maximum" CPU mode
On Tue, Feb 09, 2021 at 03:30:47PM +0100, Pavel Hrdina wrote:
> On Tue, Feb 09, 2021 at 01:58:57PM +, Daniel P. Berrangé wrote:
> > For hardware virtualization this is functionally identical to the
> > existing host-passthrough mode so the same caveats apply.
> >
> > For emulated guest this exposes the maximum featureset supported by
> > the emulator. Note that despite being emulated this is not guaranteed
> > to be migration safe, especially if different emulator software versions
> > are used on each host.
> >
> > Signed-off-by: Daniel P. Berrangé
> > ---
> > docs/formatdomain.rst| 24
> > src/conf/cpu_conf.c | 13 +
> > src/conf/cpu_conf.h | 1 +
> > src/cpu/cpu.c| 1 +
> > src/qemu/qemu_capabilities.c | 1 +
> > src/qemu/qemu_command.c | 1 +
> > src/qemu/qemu_domain.c | 1 +
> > src/qemu/qemu_validate.c | 1 +
> > 8 files changed, 39 insertions(+), 4 deletions(-)
>
> [...]
>
> > diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
> > index f98b0a0eb3..eb4bfbbcfa 100644
> > --- a/src/conf/cpu_conf.c
> > +++ b/src/conf/cpu_conf.c
> > @@ -402,10 +403,11 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
> > if ((migratable = virXMLPropString(ctxt->node, "migratable"))) {
> > int val;
> >
> > -if (def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
> > +if (def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH &&
> > +def->mode != VIR_CPU_MODE_MAXIMUM) {
> > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > _("Attribute migratable is only allowed for "
> > - "host-passthrough CPU"));
> > + "'host-passthrough' / 'maximum' CPU mode"));
>
> Since you are modifying the error message I would reformat it to single
> line.
That'd would make the line way longer than normal practice. I know
we don't need to strictly stick to 80 chars, but at the same time we
don't want things massively long.
> Reviewed-by: Pavel Hrdina
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 1/5] conf: define a new "maximum" CPU mode
On Wed, Feb 10, 2021 at 10:39:22AM +, Daniel P. Berrangé wrote:
> On Tue, Feb 09, 2021 at 03:30:47PM +0100, Pavel Hrdina wrote:
> > On Tue, Feb 09, 2021 at 01:58:57PM +, Daniel P. Berrangé wrote:
> > > For hardware virtualization this is functionally identical to the
> > > existing host-passthrough mode so the same caveats apply.
> > >
> > > For emulated guest this exposes the maximum featureset supported by
> > > the emulator. Note that despite being emulated this is not guaranteed
> > > to be migration safe, especially if different emulator software versions
> > > are used on each host.
> > >
> > > Signed-off-by: Daniel P. Berrangé
> > > ---
> > > docs/formatdomain.rst| 24
> > > src/conf/cpu_conf.c | 13 +
> > > src/conf/cpu_conf.h | 1 +
> > > src/cpu/cpu.c| 1 +
> > > src/qemu/qemu_capabilities.c | 1 +
> > > src/qemu/qemu_command.c | 1 +
> > > src/qemu/qemu_domain.c | 1 +
> > > src/qemu/qemu_validate.c | 1 +
> > > 8 files changed, 39 insertions(+), 4 deletions(-)
> >
> > [...]
> >
> > > diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
> > > index f98b0a0eb3..eb4bfbbcfa 100644
> > > --- a/src/conf/cpu_conf.c
> > > +++ b/src/conf/cpu_conf.c
> > > @@ -402,10 +403,11 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
> > > if ((migratable = virXMLPropString(ctxt->node, "migratable"))) {
> > > int val;
> > >
> > > -if (def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
> > > +if (def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH &&
> > > +def->mode != VIR_CPU_MODE_MAXIMUM) {
> > > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > > _("Attribute migratable is only allowed for "
> > > - "host-passthrough CPU"));
> > > + "'host-passthrough' / 'maximum' CPU mode"));
> >
> > Since you are modifying the error message I would reformat it to single
> > line.
>
> That'd would make the line way longer than normal practice. I know
> we don't need to strictly stick to 80 chars, but at the same time we
> don't want things massively long.
I'm aware of that. We did not change all errors and we probably don't
have a syntax-check for that but there was an agreement while ago that
for error messages we can ignore the 80 chars limit [1].
Feel free to push it like it is, I just wanted to point out that and
personally I try to change errors to single line if I have to modify
them
Pavel
[1]
> > Reviewed-by: Pavel Hrdina
>
>
>
> 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 :|
>
signature.asc
Description: PGP signature
Re: [PATCH 08/12] vsh: Deduplicate filtering in vshReadlineOptionsGenerator()
On 2/9/21 5:00 PM, Jonathon Jongsma wrote: On Thu, 4 Feb 2021 15:13:33 +0100 Michal Privoznik wrote: This is what we do for completer callbacks: we let them generate all possible outputs ignoring any partial input (e.g. prefix of a domain name) and then use vshCompleterFilter() to filter out those strings which don't fit the partial input (prefix). The same algorithm is implemented in vshReadlineOptionsGenerator() even though a bit differently. There is no need to have the same code twice. I think this might be clearer stated a bit differently. For example, if I'm understanding correctly, a suggested alternate commit message: Completer callbacks generate all possible outputs ignoring any partial input (e.g. prefix of a domain name) and then use vshCompleterFilter() to filter out those strings which don't fit the partial input (prefix). In contrast, vshReadlineOptionsGenerator() does some internal filtering and only generates completions that match a given prefix. Rather than treating these scenarios differently, simply generate all possible options and filter them all at the end. Yup, this sounds way better. Thanks! Michal
Re: [PATCH 00/12] vsh: Improve completer code
On 2/9/21 5:42 PM, Jonathon Jongsma wrote: On Thu, 4 Feb 2021 15:13:25 +0100 Michal Privoznik wrote: There is no functional change to completion. No user visible change, nor to a developer who writes a completer callback. Some code deduplication and code cleanup and adapting code to 2021. Michal Prívozník (12): lib: Substitute some STREQLEN with STRPREFIX vsh: Don't put VSH_OT_ALIAS onto list of completions vsh: Use g_auto(GStrv) to free string list returned by completer callback vsh: Accept NULL @list in vshCompleterFilter() vsh: Prefer g_strdup_printf() over g_snprintf() in vshReadlineOptionsGenerator() vsh: Use g_auto() for string lists returned in readline command/options generators vsh: Rewrite opt->type check in vshReadlineParse() vsh: Deduplicate filtering in vshReadlineOptionsGenerator() vsh: Deduplicate filtering in vshReadlineCommandGenerator() vsh: Simplify condition for calling completer callback vsh: Rework vshReadlineCommandGenerator() vsh: Drop unused @text arg from readline generators src/libxl/xen_xl.c | 2 +- src/util/vircgroupv2.c | 10 +-- tools/vsh.c| 152 - 3 files changed, 63 insertions(+), 101 deletions(-) Reviewed-by: Jonathon Jongsma Thanks, fixed those two commit messages and pushed. Michal
Re: [libvirt PATCH 1/5] conf: define a new "maximum" CPU mode
On Wed, Feb 10, 2021 at 11:53:53AM +0100, Pavel Hrdina wrote:
> On Wed, Feb 10, 2021 at 10:39:22AM +, Daniel P. Berrangé wrote:
> > On Tue, Feb 09, 2021 at 03:30:47PM +0100, Pavel Hrdina wrote:
> > > On Tue, Feb 09, 2021 at 01:58:57PM +, Daniel P. Berrangé wrote:
> > > > For hardware virtualization this is functionally identical to the
> > > > existing host-passthrough mode so the same caveats apply.
> > > >
> > > > For emulated guest this exposes the maximum featureset supported by
> > > > the emulator. Note that despite being emulated this is not guaranteed
> > > > to be migration safe, especially if different emulator software versions
> > > > are used on each host.
> > > >
> > > > Signed-off-by: Daniel P. Berrangé
> > > > ---
> > > > docs/formatdomain.rst| 24
> > > > src/conf/cpu_conf.c | 13 +
> > > > src/conf/cpu_conf.h | 1 +
> > > > src/cpu/cpu.c| 1 +
> > > > src/qemu/qemu_capabilities.c | 1 +
> > > > src/qemu/qemu_command.c | 1 +
> > > > src/qemu/qemu_domain.c | 1 +
> > > > src/qemu/qemu_validate.c | 1 +
> > > > 8 files changed, 39 insertions(+), 4 deletions(-)
> > >
> > > [...]
> > >
> > > > diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
> > > > index f98b0a0eb3..eb4bfbbcfa 100644
> > > > --- a/src/conf/cpu_conf.c
> > > > +++ b/src/conf/cpu_conf.c
> > > > @@ -402,10 +403,11 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
> > > > if ((migratable = virXMLPropString(ctxt->node, "migratable"))) {
> > > > int val;
> > > >
> > > > -if (def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
> > > > +if (def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH &&
> > > > +def->mode != VIR_CPU_MODE_MAXIMUM) {
> > > > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > > > _("Attribute migratable is only allowed for
> > > > "
> > > > - "host-passthrough CPU"));
> > > > + "'host-passthrough' / 'maximum' CPU
> > > > mode"));
> > >
> > > Since you are modifying the error message I would reformat it to single
> > > line.
> >
> > That'd would make the line way longer than normal practice. I know
> > we don't need to strictly stick to 80 chars, but at the same time we
> > don't want things massively long.
>
> I'm aware of that. We did not change all errors and we probably don't
> have a syntax-check for that but there was an agreement while ago that
> for error messages we can ignore the 80 chars limit [1].
Within reason I think that's ok, but I think this example gets a bit
too long at nearly 120 chars
>
> Feel free to push it like it is, I just wanted to point out that and
> personally I try to change errors to single line if I have to modify
> them
>
> Pavel
>
> [1]
The example here was already under 80 chars even when on one
line.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 1/5] conf: define a new "maximum" CPU mode
On Wed, Feb 10, 2021 at 11:01:24AM +, Daniel P. Berrangé wrote:
> On Wed, Feb 10, 2021 at 11:53:53AM +0100, Pavel Hrdina wrote:
> > On Wed, Feb 10, 2021 at 10:39:22AM +, Daniel P. Berrangé wrote:
> > > On Tue, Feb 09, 2021 at 03:30:47PM +0100, Pavel Hrdina wrote:
> > > > On Tue, Feb 09, 2021 at 01:58:57PM +, Daniel P. Berrangé wrote:
> > > > > For hardware virtualization this is functionally identical to the
> > > > > existing host-passthrough mode so the same caveats apply.
> > > > >
> > > > > For emulated guest this exposes the maximum featureset supported by
> > > > > the emulator. Note that despite being emulated this is not guaranteed
> > > > > to be migration safe, especially if different emulator software
> > > > > versions
> > > > > are used on each host.
> > > > >
> > > > > Signed-off-by: Daniel P. Berrangé
> > > > > ---
> > > > > docs/formatdomain.rst| 24
> > > > > src/conf/cpu_conf.c | 13 +
> > > > > src/conf/cpu_conf.h | 1 +
> > > > > src/cpu/cpu.c| 1 +
> > > > > src/qemu/qemu_capabilities.c | 1 +
> > > > > src/qemu/qemu_command.c | 1 +
> > > > > src/qemu/qemu_domain.c | 1 +
> > > > > src/qemu/qemu_validate.c | 1 +
> > > > > 8 files changed, 39 insertions(+), 4 deletions(-)
> > > >
> > > > [...]
> > > >
> > > > > diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
> > > > > index f98b0a0eb3..eb4bfbbcfa 100644
> > > > > --- a/src/conf/cpu_conf.c
> > > > > +++ b/src/conf/cpu_conf.c
> > > > > @@ -402,10 +403,11 @@ virCPUDefParseXML(xmlXPathContextPtr ctxt,
> > > > > if ((migratable = virXMLPropString(ctxt->node, "migratable"))) {
> > > > > int val;
> > > > >
> > > > > -if (def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
> > > > > +if (def->mode != VIR_CPU_MODE_HOST_PASSTHROUGH &&
> > > > > +def->mode != VIR_CPU_MODE_MAXIMUM) {
> > > > > virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > > > > _("Attribute migratable is only allowed
> > > > > for "
> > > > > - "host-passthrough CPU"));
> > > > > + "'host-passthrough' / 'maximum' CPU
> > > > > mode"));
> > > >
> > > > Since you are modifying the error message I would reformat it to single
> > > > line.
> > >
> > > That'd would make the line way longer than normal practice. I know
> > > we don't need to strictly stick to 80 chars, but at the same time we
> > > don't want things massively long.
> >
> > I'm aware of that. We did not change all errors and we probably don't
> > have a syntax-check for that but there was an agreement while ago that
> > for error messages we can ignore the 80 chars limit [1].
>
> Within reason I think that's ok, but I think this example gets a bit
> too long at nearly 120 chars
I figured that we don't care about the line being that long. The main
point about not splitting error messages is to help with git grep.
> >
> > Feel free to push it like it is, I just wanted to point out that and
> > personally I try to change errors to single line if I have to modify
> > them
> >
> > Pavel
> >
> > [1]
>
> The example here was already under 80 chars even when on one
> line.
We should definitely improve the example to make it more obvious.
Pavel
signature.asc
Description: PGP signature
Re: [PATCH] build: Remove unused 'conflicts' key from virt_daemon_unit
On 2/9/21 7:38 PM, Jim Fehlig wrote: The 'conflict' key in a virt_daemon_unit dictionary is not used when generating systemd service and socket files. The comment associated with the key claims the default is 'true', and a few build files needlessly set it to 'true' when defining their virt_daemon_unit. Remove the 'conflict' key and its use in the affect build files. Signed-off-by: Jim Fehlig --- src/interface/meson.build | 1 - src/libxl/meson.build | 1 - src/lxc/meson.build | 1 - src/meson.build | 1 - src/network/meson.build | 1 - src/node_device/meson.build | 1 - src/nwfilter/meson.build| 1 - src/qemu/meson.build| 1 - src/secret/meson.build | 1 - src/storage/meson.build | 1 - src/vbox/meson.build| 1 - src/vz/meson.build | 1 - 12 files changed, 12 deletions(-) Reviewed-by: Michal Privoznik Michal
[libvirt PATCH] schemas: Add support for maximum CPU mode
Signed-off-by: Jiri Denemark --- docs/schemas/cputypes.rng | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/schemas/cputypes.rng b/docs/schemas/cputypes.rng index f66bc62ba7..77c8fa783b 100644 --- a/docs/schemas/cputypes.rng +++ b/docs/schemas/cputypes.rng @@ -9,6 +9,7 @@ custom host-model host-passthrough +maximum -- 2.30.0
Re: [libvirt PATCH] schemas: Add support for maximum CPU mode
On Wed, Feb 10, 2021 at 02:09:33PM +0100, Jiri Denemark wrote: > Signed-off-by: Jiri Denemark > --- > docs/schemas/cputypes.rng | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature
Re: [libvirt PATCH] schemas: Add support for maximum CPU mode
On Wed, Feb 10, 2021 at 02:09:33PM +0100, Jiri Denemark wrote: > Signed-off-by: Jiri Denemark > --- > docs/schemas/cputypes.rng | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/docs/schemas/cputypes.rng b/docs/schemas/cputypes.rng > index f66bc62ba7..77c8fa783b 100644 > --- a/docs/schemas/cputypes.rng > +++ b/docs/schemas/cputypes.rng > @@ -9,6 +9,7 @@ > custom > host-model > host-passthrough > +maximum > > > Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH v3 0/2] qemu: Do not use canonical path for system memory
v3 of: https://listman.redhat.com/archives/libvir-list/2021-January/msg00684.html diff to v2: - Rebased as new capabilities and capabilities files were added since v2 - Replaced imaginary QEMU commit $HASH with actual commit hash (QEMU part was merged meanwhile) - Worked in Peter's review of v2 Michal Prívozník (2): qemu_capabilities: Introduce QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID qemu: Do not Use canonical path for system memory src/qemu/qemu_capabilities.c | 6 src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 35 --- src/qemu/qemu_command.h | 3 +- src/qemu/qemu_hotplug.c | 2 +- .../caps_4.0.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 + .../caps_4.0.0.riscv32.xml| 1 + .../caps_4.0.0.riscv64.xml| 1 + .../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 + .../caps_4.0.0.x86_64.xml | 1 + .../caps_4.1.0.x86_64.xml | 1 + .../caps_4.2.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + .../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 + .../caps_4.2.0.x86_64.xml | 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 + .../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 + .../caps_5.1.0.x86_64.xml | 1 + .../caps_5.2.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 + .../caps_5.2.0.riscv64.xml| 1 + .../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 + .../caps_5.2.0.x86_64.xml | 1 + .../caps_6.0.0.x86_64.xml | 1 + .../disk-vhostuser.x86_64-latest.args | 3 +- .../hugepages-memaccess3.x86_64-latest.args | 4 +-- 30 files changed, 68 insertions(+), 9 deletions(-) -- 2.26.2
[PATCH v3 1/2] qemu_capabilities: Introduce QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID
This capability tracks whether memory-backend-file has
"x-use-canonical-path-for-ramblock-id" attribute. Introduced into
QEMU by commit fa0cb34d2210cc749b9a70db99bb41c56ad20831. As of
QEMU commit 8db0b20415c129cf5e577a593a4a0372d90b7cc9 the property
is considered stable by qemu despite the 'x-' prefix to preserve
compatibility with released qemu versions.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_capabilities.c | 6 ++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 +
tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 +
tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 +
25 files changed, 30 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index ebd6607888..cd34988806 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -614,6 +614,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
/* 390 */
"vhost-user-blk",
+ "memory-backend-file.x-use-canonical-path-for-ramblock-id",
);
@@ -1682,6 +1683,11 @@ static struct virQEMUCapsStringFlags
virQEMUCapsObjectPropsMemoryBackendFile[] =
{ "discard-data", QEMU_CAPS_OBJECT_MEMORY_FILE_DISCARD },
{ "align", QEMU_CAPS_OBJECT_MEMORY_FILE_ALIGN },
{ "pmem", QEMU_CAPS_OBJECT_MEMORY_FILE_PMEM },
+/* As of QEMU commit 8db0b20415c129cf5e577a593a4a0372d90b7cc9 the
+ * "x-use-canonical-path-for-ramblock-id" property is considered stable and
+ * supported. The 'x-' prefix was kept for compatibility with already
+ * released qemu versions. */
+{ "x-use-canonical-path-for-ramblock-id",
QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID },
};
static struct virQEMUCapsStringFlags
virQEMUCapsObjectPropsMemoryBackendMemfd[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 8cb5673042..c422636d3a 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -594,6 +594,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
/* 390 */
QEMU_CAPS_DEVICE_VHOST_USER_BLK, /* -device vhost-user-blk */
+QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID, /* -object
memory-backend-file,x-use-canonical-path-for-ramblock-id= */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
index c28ada94fb..5e7761851f 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml
@@ -181,6 +181,7 @@
+
400
0
61700240
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
index a15edd87de..1d67935bd8 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml
@@ -189,6 +189,7 @@
+
400
0
42900240
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
index de2b578b82..d56a19e6f9 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml
@@ -182,6 +182,7 @@
+
400
0
0
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
index 754ad6db53..d35cb7e2ca 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml
@@ -182,6 +182,7 @@
+
400
0
0
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml
index 4a10deea01..ca9592ea11 100644
--- a/tests/qemucapabilitiesdata/ca
[PATCH v3 2/2] qemu: Do not Use canonical path for system memory
In commit 88957116c9d3cb4705380c3702c9d4315fb500bb I've adapted
libvirt to QEMU's deprecation of -mem-path and -mem-prealloc and
switched to memory-backend-* even for system memory. My claim was
that that's what QEMU does under the hood anyway. And indeed it
was: see QEMU commit 900c0ba373aada4c13d47d95330aa72ec4067ba5 and
look at function create_default_memdev().
However, then commit d96c4d5f193e0e45beec80a6277728b32875bddb was
merged into QEMU. While it was fixing a bug, it also changed the
create_default_memdev() function in which it started turning off
use of canonical path (by setting
"x-use-canonical-path-for-ramblock-id" attribute to false). This
wasn't documented until QEMU commit XXX. The path affects
migration - the same path has to be used on the source and on the
destination. Therefore, if there is old guest started with '-m X'
it has "pc.ram" block which doesn't use canonical path and thus
when migrating to newer QEMU which uses memory-backend-* we have
to turn off the canonical path explicitly. Otherwise,
"/objects/pc.ram" path would be expected by QEMU which doesn't
match the source.
Ideally, we would need to set it only for some machine types
(4.0 and older) because newer machine types already do what we
are doing. However, we treat machine types as opaque strings and
therefore we don't want to parse nor inspect their versions. But
then again, newer machine types already do what we are doing in
this commit, so when old machine types are deprecated and removed
we can remove our hack and forget it ever happened.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1912201
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_command.c | 35 ---
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_hotplug.c | 2 +-
.../disk-vhostuser.x86_64-latest.args | 3 +-
.../hugepages-memaccess3.x86_64-latest.args | 4 +--
5 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 92036d26c0..699d89de1d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2975,7 +2975,8 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
qemuDomainObjPrivatePtr priv,
const virDomainDef *def,
const virDomainMemoryDef *mem,
-bool force)
+bool force,
+bool systemMemory)
{
const char *backendType = "memory-backend-file";
virDomainNumatuneMemMode mode;
@@ -2992,6 +2993,10 @@ qemuBuildMemoryBackendProps(virJSONValuePtr
*backendProps,
bool needHugepage = !!pagesize;
bool useHugepage = !!pagesize;
int discard = mem->discard;
+bool disableCanonicalPath = false;
+
+/* Disabling canonical path is required for migration compatibility of
+ * system memory objects, see below */
/* The difference between @needHugepage and @useHugepage is that the latter
* is true whenever huge page is defined for the current memory cell.
@@ -3106,6 +3111,9 @@ qemuBuildMemoryBackendProps(virJSONValuePtr *backendProps,
if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0)
return -1;
+if (systemMemory)
+disableCanonicalPath = true;
+
} else if (useHugepage || mem->nvdimmPath || memAccess ||
def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) {
@@ -3148,10 +3156,29 @@ qemuBuildMemoryBackendProps(virJSONValuePtr
*backendProps,
if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0)
return -1;
+
+if (systemMemory)
+disableCanonicalPath = true;
+
} else {
backendType = "memory-backend-ram";
}
+/* This is a terrible hack, but unfortunately there is no better way.
+ * The replacement for '-m X' argument is not simple '-machine
+ * memory-backend' and '-object memory-backend-*,size=X' (which was the
+ * idea). This is because of create_default_memdev() in QEMU sets
+ * 'x-use-canonical-path-for-ramblock-id' attribute to false and is
+ * documented in QEMU in qemu-options.hx under 'memory-backend'. Note
+ * that QEMU consideres 'x-use-canonical-path-for-ramblock-id' stable
+ * and supported despite the 'x-' prefix.
+ * See QEMU commit 8db0b20415c129cf5e577a593a4a0372d90b7cc9.
+ */
+if (disableCanonicalPath &&
+virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID) &&
+virJSONValueObjectAdd(props, "b:x-use-canonical-path-for-ramblock-id",
false, NULL) < 0)
+return -1;
+
if (!priv->memPrealloc &&
virJSONValueObjectAdd(props, "B:prealloc", prealloc, NULL) < 0)
return -1;
@@ -3263,7 +3290,7 @@ qemuBuildMemoryCellBackendStr(virDomainDefPtr def,
mem.info.alias = alias;
if ((rc = qemuBuildMemoryBackendProps(&props, a
Re: [PATCH v2 2/2] rpc: introduce macro MILLISECONDS_PER_SECOND
On Tue, Feb 09, 2021 at 09:19:48 +, BiaoxiangYe wrote:
> From: BiaoXiang Ye
>
> Use macro instead of magic number, keep the style consistent
> with MICROSEC_PER_SEC.
I'm not sure where's the magic in converting seconds to milliseconds or
microseconds to seconds. However, I can see it's a bit easier to spot
we're converting seconds and milliseconds rather than milliseconds and
microseconds, for example.
>
> Signed-off-by: BiaoXiang Ye
> ---
> src/rpc/virkeepalive.c | 18 ++
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/src/rpc/virkeepalive.c b/src/rpc/virkeepalive.c
> index 786f9038ef..baea2e6478 100644
> --- a/src/rpc/virkeepalive.c
> +++ b/src/rpc/virkeepalive.c
> @@ -32,6 +32,7 @@
>
> #define VIR_FROM_THIS VIR_FROM_RPC
> #define MICROSEC_PER_SEC(1000 * 1000)
> +#define MILLISECONDS_PER_SECOND (1000)
> VIR_LOG_INIT("rpc.keepalive");
Please, keep the empty line above VIR_LOG_INIT.
MICROSEC_PER_SEC vs MILLISECONDS_PER_SECOND definitely don't use
consistent style. Ideally you could use G_USEC_PER_SEC as Jonathon
suggested, but I'm afraid there's no corresponding G_MSEC_PER_SEC to be
used in this patch. I guess G_USEC_PER_SEC and VIR_MSEC_PER_SEC could
be usable as a workaround... Or just use numbers instead of both macros,
they are quite clear anyway.
Jirka
Re: [PATCH v3 0/2] qemu: Do not use canonical path for system memory
On 2/10/21 2:38 PM, Michal Privoznik wrote: v3 of: https://listman.redhat.com/archives/libvir-list/2021-January/msg00684.html diff to v2: - Rebased as new capabilities and capabilities files were added since v2 Huh, except meanwhile another commits were pushed which create conflict. I've pushed updated commits into my gitlab: https://gitlab.com/MichalPrivoznik/libvirt/-/commits/qemu_machine_memory_backend_v3/ Michal
Re: [PATCH v3 1/2] qemu_capabilities: Introduce QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID
On Wed, Feb 10, 2021 at 14:38:13 +0100, Michal Privoznik wrote: > This capability tracks whether memory-backend-file has > "x-use-canonical-path-for-ramblock-id" attribute. Introduced into > QEMU by commit fa0cb34d2210cc749b9a70db99bb41c56ad20831. As of > QEMU commit 8db0b20415c129cf5e577a593a4a0372d90b7cc9 the property > is considered stable by qemu despite the 'x-' prefix to preserve > compatibility with released qemu versions. > > Signed-off-by: Michal Privoznik > --- Reviewed-by: Peter Krempa
Re: [PATCH v3 2/2] qemu: Do not Use canonical path for system memory
On Wed, Feb 10, 2021 at 14:38:14 +0100, Michal Privoznik wrote:
> In commit 88957116c9d3cb4705380c3702c9d4315fb500bb I've adapted
> libvirt to QEMU's deprecation of -mem-path and -mem-prealloc and
> switched to memory-backend-* even for system memory. My claim was
> that that's what QEMU does under the hood anyway. And indeed it
> was: see QEMU commit 900c0ba373aada4c13d47d95330aa72ec4067ba5 and
> look at function create_default_memdev().
>
> However, then commit d96c4d5f193e0e45beec80a6277728b32875bddb was
> merged into QEMU. While it was fixing a bug, it also changed the
> create_default_memdev() function in which it started turning off
> use of canonical path (by setting
> "x-use-canonical-path-for-ramblock-id" attribute to false). This
> wasn't documented until QEMU commit XXX. The path affects
s/XXX/actual commit id/
> migration - the same path has to be used on the source and on the
> destination. Therefore, if there is old guest started with '-m X'
> it has "pc.ram" block which doesn't use canonical path and thus
> when migrating to newer QEMU which uses memory-backend-* we have
> to turn off the canonical path explicitly. Otherwise,
> "/objects/pc.ram" path would be expected by QEMU which doesn't
> match the source.
>
> Ideally, we would need to set it only for some machine types
> (4.0 and older) because newer machine types already do what we
> are doing. However, we treat machine types as opaque strings and
> therefore we don't want to parse nor inspect their versions. But
> then again, newer machine types already do what we are doing in
> this commit, so when old machine types are deprecated and removed
> we can remove our hack and forget it ever happened.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1912201
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/qemu_command.c | 35 ---
> src/qemu/qemu_command.h | 3 +-
> src/qemu/qemu_hotplug.c | 2 +-
> .../disk-vhostuser.x86_64-latest.args | 3 +-
> .../hugepages-memaccess3.x86_64-latest.args | 4 +--
> 5 files changed, 38 insertions(+), 9 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 92036d26c0..699d89de1d 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
[...]
> @@ -3148,10 +3156,29 @@ qemuBuildMemoryBackendProps(virJSONValuePtr
> *backendProps,
>
> if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0)
> return -1;
> +
> +if (systemMemory)
> +disableCanonicalPath = true;
> +
> } else {
> backendType = "memory-backend-ram";
> }
>
> +/* This is a terrible hack, but unfortunately there is no better way.
> + * The replacement for '-m X' argument is not simple '-machine
> + * memory-backend' and '-object memory-backend-*,size=X' (which was the
> + * idea). This is because of create_default_memdev() in QEMU sets
> + * 'x-use-canonical-path-for-ramblock-id' attribute to false and is
> + * documented in QEMU in qemu-options.hx under 'memory-backend'. Note
> + * that QEMU consideres 'x-use-canonical-path-for-ramblock-id' stable
> + * and supported despite the 'x-' prefix.
> + * See QEMU commit 8db0b20415c129cf5e577a593a4a0372d90b7cc9.
> + */
> +if (disableCanonicalPath &&
> +virQEMUCapsGet(priv->qemuCaps,
> QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID) &&
> +virJSONValueObjectAdd(props,
> "b:x-use-canonical-path-for-ramblock-id", false, NULL) < 0)
> +return -1;
I don't remember whether you and Daniel reached some consensus about the
machine type version check, but if the consensus is to always output it
then okay.
>From my side:
Reviewed-by: Peter Krempa
[libvirt PATCH 4/4] ci: Drop the prepare.sh script
The purpose of this script was to prepare a customized environment in the container, but was actually never used and it required the usage of sudo to switch the environment from root's context to a regular user's one. The thing is that once someone needs a custom script they would very likely to debug something and would also benefit from root privileges in general, so the usage of 'sudo' in such case was a bit cumbersome. Signed-off-by: Erik Skultety --- ci/prepare.sh | 13 - 1 file changed, 13 deletions(-) delete mode 100644 ci/prepare.sh diff --git a/ci/prepare.sh b/ci/prepare.sh deleted file mode 100644 index da6fc9a1b5..00 --- a/ci/prepare.sh +++ /dev/null @@ -1,13 +0,0 @@ -# This script is used to prepare the environment that will be used -# to build libvirt inside the container. -# -# You can customize it to your liking, or alternatively use a -# completely different script by passing -# -# CI_PREPARE_SCRIPT=/path/to/your/prepare/script -# -# to make. -# -# Note that this script will have root privileges inside the -# container, so it can be used for things like installing additional -# packages. -- 2.29.2
[libvirt PATCH 3/4] ci: Expose CI_USER_LOGIN as a Makefile variable for users to use
More often than not I find myself debugging the containers which means that I need to have root inside, but without manually tweaking the Makefile each time the execution would simply fail thanks to the uid/gid mapping we do. What if we expose the CI_USER_LOGIN variable, so that when needed, the root can be simply passed with this variable and voila - you have a root shell inside the container with CWD=~root. Signed-off-by: Erik Skultety --- ci/Makefile | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/ci/Makefile b/ci/Makefile index 1a376a7f0c..84f2f77526 100644 --- a/ci/Makefile +++ b/ci/Makefile @@ -47,13 +47,13 @@ CI_REUSE = 0 # We need the container process to run with current host IDs # so that it can access the passed in build directory -CI_UID = $(shell id -u) -CI_GID = $(shell id -g) +CI_UID = $(shell id -u $(CI_USER_LOGIN)) +CI_GID = $(shell id -g $(CI_USER_LOGIN)) # We also need the user's login and home directory to prepare the # environment the way some programs expect it -CI_USER_LOGIN = $(shell echo "$$USER") -CI_USER_HOME = $(shell echo "$$HOME") +CI_USER_LOGIN = $(shell whoami) +CI_USER_HOME = $(shell eval echo "~$(CI_USER_LOGIN)") CI_ENGINE = auto # Container engine we are going to use, can be overridden per make @@ -132,6 +132,13 @@ ifeq ($(CI_ENGINE),podman) --gidmap $(CI_GID):0:1 \ --gidmap $(CI_GID_OTHER):$(CI_GID_OTHER):$(CI_GID_OTHER_RANGE) \ $(NULL) + + # In case we want to debug in the container, having root is actually + # preferable, so reset the CI_PODMAN_ARGS and don't actually perform + # any uid/gid mapping + ifeq ($(CI_UID), 0) + CI_PODMAN_ARGS= + endif endif # Args to use when cloning a git repo. @@ -238,6 +245,7 @@ ci-help: @echo @echo "CI_CLEAN=0 - do not delete '$(CI_SCRATCHDIR)' after completion" @echo "CI_REUSE=1 - re-use existing '$(CI_SCRATCHDIR)' content" + @echo "CI_USER_LOGIN= - which user should run in the container (default is $$USER)" @echo "CI_ENGINE=auto - container engine to use (podman, docker)" @echo "CI_MESON_ARGS= - extra arguments passed to meson" @echo "CI_NINJA_ARGS= - extra arguments passed to ninja" -- 2.29.2
[libvirt PATCH 1/4] ci: Specify the shebang sequence for build.sh
This is necessary for the follow up patch, because the default entrypoint for a Dockerfile is exec. Signed-off-by: Erik Skultety --- ci/build.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ci/build.sh b/ci/build.sh index 740b46a935..0f23df1fa3 100644 --- a/ci/build.sh +++ b/ci/build.sh @@ -1,3 +1,5 @@ +#!/bin/sh + # This script is used to build libvirt inside the container. # # You can customize it to your liking, or alternatively use a -- 2.29.2
[libvirt PATCH 0/4] ci: Adjustments to remove our dependency on sudo
Our Debian containers don't have sudo pre-installed and the only reason we actually needed it was to run a custom prepare script which as it turns out d= oes nothing really. Erik Skultety (4): ci: Specify the shebang sequence for build.sh ci: Run podman command directly without wrapping it with prepare.sh ci: Expose CI_USER_LOGIN as a Makefile variable for users to use ci: Drop the prepare.sh script ci/Makefile | 43 --- ci/build.sh | 2 ++ ci/prepare.sh | 13 - 3 files changed, 26 insertions(+), 32 deletions(-) delete mode 100644 ci/prepare.sh --=20 2.29.2
[libvirt PATCH 2/4] ci: Run podman command directly without wrapping it with prepare.sh
The prepare.sh script isn't currently used and forces us to make use
of sudo to switch the user inside the container from root to $USER
which created a problem on our Debian Slim-based containers which don't
have the 'sudo' package installed.
This patch removes the sudo invocation and instead runs the CMD
directly with podman.
Summary of the changes:
- move the corresponding env variables which we need to be set in the
environment from the sudo invocation to the podman invocation
- pass --workdir to podman to retain the original behaviour we had with
sudo spawning a login shell.
- MESON_ARGS env variable doesn't need to propagated to the execution
environment anymore (like we had to do with sudo), because it's
defined in the Dockerfile
Signed-off-by: Erik Skultety
---
ci/Makefile | 27 ---
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/ci/Makefile b/ci/Makefile
index 7938e14c15..1a376a7f0c 100644
--- a/ci/Makefile
+++ b/ci/Makefile
@@ -82,7 +82,6 @@ CI_HOME_MOUNTS = \
$(NULL)
CI_SCRIPT_MOUNTS = \
- --volume $(CI_SCRATCHDIR)/prepare:$(CI_USER_HOME)/prepare:z \
--volume $(CI_SCRATCHDIR)/build:$(CI_USER_HOME)/build:z \
$(NULL)
@@ -150,6 +149,8 @@ CI_GIT_ARGS = \
# --userwe execute as the same user & group account
# as dev so that file ownership matches host
# instead of root:root
+# --workdir we change to user's home dir in the container
+# before running the workload
# --volume to pass in the cloned git repo & config
# --ulimit lower files limit for performance reasons
# --interactive
@@ -158,6 +159,11 @@ CI_ENGINE_ARGS = \
--rm \
--interactive \
--tty \
+ --user $(CI_UID):$(CI_GID) \
+ --workdir $(CI_USER_HOME) \
+ --env CI_CONT_SRCDIR="$(CI_CONT_SRCDIR)" \
+ --env CI_MESON_ARGS="$(CI_MESON_ARGS)" \
+ --env CI_NINJA_ARGS="$(CI_NINJA_ARGS)" \
$(CI_PODMAN_ARGS) \
$(CI_PWDB_MOUNTS) \
$(CI_HOME_MOUNTS) \
@@ -178,9 +184,8 @@ ci-prepare-tree: ci-check-engine
cp /etc/passwd $(CI_SCRATCHDIR); \
cp /etc/group $(CI_SCRATCHDIR); \
mkdir -p $(CI_SCRATCHDIR)/home; \
- cp "$(CI_PREPARE_SCRIPT)" $(CI_SCRATCHDIR)/prepare; \
cp "$(CI_BUILD_SCRIPT)" $(CI_SCRATCHDIR)/build; \
- chmod +x "$(CI_SCRATCHDIR)/prepare" "$(CI_SCRATCHDIR)/build"; \
+ chmod +x "$(CI_SCRATCHDIR)/build"; \
echo "Cloning $(CI_GIT_ROOT) to $(CI_HOST_SRCDIR)"; \
git clone $(CI_GIT_ARGS) $(CI_GIT_ROOT) $(CI_HOST_SRCDIR) ||
exit 1; \
for mod in $$(git submodule | awk '{ print $$2 }' | sed -E
's,^../,,g') ; \
@@ -192,18 +197,10 @@ ci-prepare-tree: ci-check-engine
fi
ci-run-command@%: ci-prepare-tree
- $(CI_ENGINE) run $(CI_ENGINE_ARGS) $(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG) \
- /bin/bash -c ' \
- $(CI_USER_HOME)/prepare || exit 1; \
- sudo \
- --login \
- --user="#$(CI_UID)" \
- --group="#$(CI_GID)" \
- MESON_OPTS="$$MESON_OPTS" \
- CI_CONT_SRCDIR="$(CI_CONT_SRCDIR)" \
- CI_MESON_ARGS="$(CI_MESON_ARGS)" \
- CI_NINJA_ARGS="$(CI_NINJA_ARGS)" \
- $(CI_COMMAND) || exit 1'
+ $(CI_ENGINE) run \
+ $(CI_ENGINE_ARGS) \
+ $(CI_IMAGE_PREFIX)$*$(CI_IMAGE_TAG) \
+ $(CI_COMMAND)
@test "$(CI_CLEAN)" = "1" && rm -rf $(CI_SCRATCHDIR) || :
ci-shell@%:
--
2.29.2
[libvirt PATCH 0/4] RFE: ci: A couple of minor improvements
First of all, this builds on top of [1]. As I was testing some builds in local containers I noticed that when one does: $ make -C ci ci-list-images it will return images that we no longer support in lcitool nor in libvirt, but one can still pull them and run a build in those. So I created a Python script that compares the current registry with what we support in lcitool and if the= re images of distro we no longer support, it will flag those (see below). Now, I wonder where the proper place to run this would be, originally I creat= ed for our GitLab containers and ran it during the sanity-check phase [2]. But having it marked as a "soft" failure didn't seem correct. So I'm proposing to run this within the context of the "refresh" script. I also pondered whether I should go ahead and rewrite the refresh script to Python as well (like this series does for list-images.sh), but I actually like how straight forward the refresh script is, unlike list-images, it would not look better in Python for sure. The 3 resulting script hierarchy isn't perfec= t, but I'd like to hear some comments first. [1] https://listman.redhat.com/archives/libvir-list/2021-February/msg00636.ht= ml [2] https://gitlab.com/eskultety/libvirt/-/pipelines/253570058 EXAMPLE OUTPUT OF THE REGISTRY CHECKER: The following images can be purged from the registry: libvirt/libvirt/ci-debian-9 libvirt/libvirt/ci-debian-9-cross-aarch64 libvirt/libvirt/ci-debian-9-cross-mipsel libvirt/libvirt/ci-debian-9-cross-armv7l libvirt/libvirt/ci-debian-9-cross-armv6l libvirt/libvirt/ci-debian-9-cross-mips libvirt/libvirt/ci-debian-9-cross-ppc64le libvirt/libvirt/ci-debian-9-cross-s390x libvirt/libvirt/ci-debian-9-cross-mips64el libvirt/libvirt/ci-fedora-31 libvirt/libvirt/ci-opensuse-151 You can remove the above images over the API with the following code snippet: $ for image_id in 1154661 1154667 1154669 1154671 1154676 1154678 1154682 11= 54683 1154686 1154687 1154724 \ ;do \ curl --request DELETE --header "PRIVATE-TOKEN: " \ https://gitlab.com/api/v4/projects/192693/registry/repositories/$image_id \ ;done Erik Skultety (4): ci: Makefile: Specify a help target to replace ci-help ci: Rewrite list-images from Bash to Python ci: list-images: Split some generic logic to a util module ci: Introduce a new checker script 'check-registry.py' ci/Makefile | 12 ++--- ci/containers/check-registry.py | 96 + ci/containers/refresh | 5 ++ ci/containers/util.py | 25 + ci/list-images.py | 32 +++ 5 files changed, 161 insertions(+), 9 deletions(-) create mode 100644 ci/containers/check-registry.py create mode 100644 ci/containers/util.py create mode 100644 ci/list-images.py --=20 2.29.2
[libvirt PATCH 1/4] ci: Makefile: Specify a help target to replace ci-help
It's quite pointless to have a 'ci-help' target in the Makefile when one needs to actually open the Makefile to go through the list of targets to know what functionality the Makefile actually provides. It's much more intuitive to run "make help" in that case. Therefore, add a 'help' target and replace the old 'ci-help' target with it. Signed-off-by: Erik Skultety --- ci/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/Makefile b/ci/Makefile index 7938e14c15..1f71701047 100644 --- a/ci/Makefile +++ b/ci/Makefile @@ -226,7 +226,7 @@ ci-list-images: @sh list-images.sh "$(CI_IMAGE_PREFIX)" | grep cross @echo -ci-help: +help: @echo "Build libvirt inside containers used for CI" @echo @echo "Available targets:" @@ -235,7 +235,7 @@ ci-help: @echo "ci-check@\$$IMAGE - run a 'ninja test'" @echo "ci-shell@\$$IMAGE - run an interactive shell" @echo "ci-list-images - list available images" - @echo "ci-help - show this help message" + @echo "help- show this help message" @echo @echo "Available make variables:" @echo -- 2.29.2
[libvirt PATCH 3/4] ci: list-images: Split some generic logic to a util module
Parts of the code can be reused by another registry checker script
introduced in the next patch.
Signed-off-by: Erik Skultety
---
ci/containers/util.py | 25 +
ci/list-images.py | 43 +++
2 files changed, 48 insertions(+), 20 deletions(-)
create mode 100644 ci/containers/util.py
diff --git a/ci/containers/util.py b/ci/containers/util.py
new file mode 100644
index 00..1ce326bba2
--- /dev/null
+++ b/ci/containers/util.py
@@ -0,0 +1,25 @@
+import json
+import urllib.request as urllib
+
+
+PROJECT_ID = 192693
+
+
+def get_registry_uri(project_id,
+ base_uri="https://gitlab.com";,
+ api_version=4):
+uri =
f"{base_uri}/api/v{str(api_version)}/projects/{project_id}/registry/repositories"
+return uri
+
+
+def list_images(uri):
+"""
+Returns all container images as currently available for the given GitLab
+project.
+
+ret: list of container image names
+"""
+r = urllib.urlopen(uri)
+
+# read the HTTP response and load the JSON part of it
+return json.loads(r.read().decode())
diff --git a/ci/list-images.py b/ci/list-images.py
index 6862ac347f..1a1d87c10b 100644
--- a/ci/list-images.py
+++ b/ci/list-images.py
@@ -1,29 +1,32 @@
#!/usr/bin/env python3
-import json
-import urllib.request as urllib
+import containers.util as util
-PROJECT_ID = 192693
-DOMAIN = "https://gitlab.com";
-API = "api/v4"
-uri = f"{DOMAIN}/{API}/projects/{PROJECT_ID}/registry/repositories"
-r = urllib.urlopen(uri + "?per_page=100")
+def list_image_names(uri):
+images = util.list_images(uri)
-# read the HTTP response and load the JSON part of it
-data = json.loads(r.read().decode())
+# skip the "ci-" prefix each of our container images' name has
+names = [i["name"][3:] for i in images]
+names.sort()
-# skip the "ci-" prefix each of our container images' name has
-names = [i["name"][3:] for i in data]
-names.sort()
+return names
-names_native = [name for name in names if "cross" not in name]
-names_cross = [name for name in names if "cross" in name]
-print("Available x86 container images:\n")
-print("\t" + "\n\t".join(names_native))
+def main():
+names = list_image_names(util.get_registry_uri(util.PROJECT_ID) +
"?per_page=100")
-if names_cross:
-print()
-print("Available cross-compiler container images:\n")
-print("\t" + "\n\t".join(names_cross))
+names_native = [name for name in names if "cross" not in name]
+names_cross = [name for name in names if "cross" in name]
+
+print("Available x86 container images:\n")
+print("\t" + "\n\t".join(names_native))
+
+if names_cross:
+print()
+print("Available cross-compiler container images:\n")
+print("\t" + "\n\t".join(names_cross))
+
+
+if __name__ == "__main__":
+main()
--
2.29.2
[libvirt PATCH 2/4] ci: Rewrite list-images from Bash to Python
Convert the script to some human readable form. Speed-wise, both are
comparable.
Signed-off-by: Erik Skultety
---
ci/Makefile | 8 +---
ci/list-images.py | 29 +
2 files changed, 30 insertions(+), 7 deletions(-)
create mode 100644 ci/list-images.py
diff --git a/ci/Makefile b/ci/Makefile
index 1f71701047..92032e8ca0 100644
--- a/ci/Makefile
+++ b/ci/Makefile
@@ -217,13 +217,7 @@ ci-test@%:
ci-list-images:
@echo
- @echo "Available x86 container images:"
- @echo
- @sh list-images.sh "$(CI_IMAGE_PREFIX)" | grep -v cross
- @echo
- @echo "Available cross-compiler container images:"
- @echo
- @sh list-images.sh "$(CI_IMAGE_PREFIX)" | grep cross
+ @python3 list-images.py
@echo
help:
diff --git a/ci/list-images.py b/ci/list-images.py
new file mode 100644
index 00..6862ac347f
--- /dev/null
+++ b/ci/list-images.py
@@ -0,0 +1,29 @@
+#!/usr/bin/env python3
+
+import json
+import urllib.request as urllib
+
+PROJECT_ID = 192693
+DOMAIN = "https://gitlab.com";
+API = "api/v4"
+
+uri = f"{DOMAIN}/{API}/projects/{PROJECT_ID}/registry/repositories"
+r = urllib.urlopen(uri + "?per_page=100")
+
+# read the HTTP response and load the JSON part of it
+data = json.loads(r.read().decode())
+
+# skip the "ci-" prefix each of our container images' name has
+names = [i["name"][3:] for i in data]
+names.sort()
+
+names_native = [name for name in names if "cross" not in name]
+names_cross = [name for name in names if "cross" in name]
+
+print("Available x86 container images:\n")
+print("\t" + "\n\t".join(names_native))
+
+if names_cross:
+print()
+print("Available cross-compiler container images:\n")
+print("\t" + "\n\t".join(names_cross))
--
2.29.2
[libvirt PATCH 4/4] ci: Introduce a new checker script 'check-registry.py'
When removing the dependency on sudo in our CI Makefile, I realized
that the list of images that we pull contains some old images that we
no longer support in lcitool, but can be still pulled from the
registry. This is a simple checker which runs in context of the refresh
script which informs the developer/maintainer that there are some old
redundant images that should be purged from the registry, so that
people don't try to run local builds in those (which will most likely
fail anyway).
Signed-off-by: Erik Skultety
---
ci/containers/check-registry.py | 96 +
ci/containers/refresh | 5 ++
2 files changed, 101 insertions(+)
create mode 100644 ci/containers/check-registry.py
diff --git a/ci/containers/check-registry.py b/ci/containers/check-registry.py
new file mode 100644
index 00..f0159da54d
--- /dev/null
+++ b/ci/containers/check-registry.py
@@ -0,0 +1,96 @@
+#!/usr/bin/env python3
+
+import argparse
+import subprocess
+import shutil
+import sys
+
+import util
+
+
+def get_image_distro(image_name: str):
+name_prefix = "ci-"
+name_suffix = "-cross-"
+
+distro = image_name[len(name_prefix):]
+
+index = distro.find(name_suffix)
+if index > 0:
+distro = distro[:index]
+
+return distro
+
+
+def get_undesirables(registry_distros_d, hosts_l):
+""" Returns a dictionary of 'id':'name' pairs of images that can be
purged"""
+
+undesirables_d = {}
+for distro in registry_distros_d:
+if distro not in hosts_l:
+for image in registry_distros_d[distro]:
+undesirables_d[str(image["id"])] = image["path"]
+
+return undesirables_d if undesirables_d else None
+
+
+def get_lcitool_hosts(lcitool_path):
+""" Returns a list of supported hosts by lcitool
+@param lcitool_path: absolute path to local copy of lcitool, if
omitted it is
+assumed lcitool is installed in the current environment"""
+
+if not lcitool_path:
+lcitool_path = "lcitool"
+if not shutil.which(lcitool_path):
+sys.exit("error: 'lcitool' not installed")
+
+lcitool_out = subprocess.check_output([lcitool_path, "hosts"])
+return lcitool_out.decode("utf-8").splitlines()
+
+
+def main():
+parser = argparse.ArgumentParser()
+parser.add_argument("project_id",
+help="GitLab project ID")
+parser.add_argument("--lcitool_path",
+dest="lcitool",
+metavar="PATH",
+help="absolute path to lcitool, CWD is used if
omitted")
+
+args = parser.parse_args()
+
+uri = util.get_registry_uri(util.PROJECT_ID)
+images_json = util.list_images(uri + "?per_page=100")
+
+registry_distros_d = {}
+for image in images_json:
+distro_name = get_image_distro(image["name"])
+
+try:
+registry_distros_d[distro_name].append(image)
+except KeyError:
+registry_distros_d[distro_name] = [image]
+
+hosts_l = get_lcitool_hosts(args.lcitool)
+
+# print the list of images that we can safely purge from the registry
+undesirables_d = get_undesirables(registry_distros_d, hosts_l)
+if undesirables_d:
+undesirable_image_names = "\t" + "\n\t".join(undesirables_d.values())
+undesirable_image_ids = " ".join(undesirables_d.keys())
+
+sys.exit(f"""
+The following images can be purged from the registry:
+
+{undesirable_image_names}
+
+You can remove the above images over the API with the following code snippet:
+
+\t$ for image_id in {undesirable_image_ids} \\
+\t;do \\
+\t\tcurl --request DELETE --header "PRIVATE-TOKEN: " \\
+\t\t{util.get_registry_uri(util.PROJECT_ID)}/$image_id \\
+\t;done""")
+
+
+if __name__ == "__main__":
+main()
diff --git a/ci/containers/refresh b/ci/containers/refresh
index a39d0b0996..9dfaf75300 100755
--- a/ci/containers/refresh
+++ b/ci/containers/refresh
@@ -40,3 +40,8 @@ do
$LCITOOL dockerfile $host libvirt > ci-$host.Dockerfile
done
+
+# Check whether there are some old images in the container registry that could
+# be purged - mainly because the list-images script would list them as
+# available
+/usr/bin/env python3 check-registry.py --lcitool_path "$LCITOOL" 192693
--
2.29.2
[PATCH] qemu: match alias when looking for proper to detach.
Previously we only checked MAC address and PCI address (or CCW
address). This is not enough information in cases where PCI address
isn't provided and multiple interfaces have the same MAC address (for
example, a virtio + hostdev "teaming" pair - their MAC addresses are
always the same).
Resolves: https://bugzilla.redhat.com/1926190
Signed-off-by: Laine Stump
---
Arguably, it would be nice to overhaul the device matching used for
virDomainDeviceDetach for *all* the device types, as they could all be
matched by looking at alias (and PCI address, for that matter). On the
other hand, for all other device types there are already enough fields
being matched to assure a unique match even without looking at
alias/PCI address, and this patch is intended to fix a current problem
being experienced in the wild, meaning it's likely that it will need
to be backported to stable branches, and I'd rather not force
backporting a sweeping change to stable branches just to bring in a
fix for one corner case)
src/conf/domain_conf.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 07e6f39256..8f2207bdf6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16431,6 +16431,11 @@ virDomainNetFindIdx(virDomainDefPtr def,
virDomainNetDefPtr net)
&net->info.addr.ccw))
continue;
+if (net->info.alias &&
+STRNEQ_NULLABLE(def->nets[i]->info.alias, net->info.alias)) {
+continue;
+}
+
if (matchidx >= 0) {
/* there were multiple matches on mac address, and no
* qualifying guest-side PCI/CCW address was given, so we must
--
2.29.2
[PATCH 1/2] qemu: Fix swtpm device with aarch64
Starting a VM with swtpm device fails with qemu-system-aarch64.
E.g. with TPM device config
QEMU reports the following error
error: internal error: process exited while connecting to monitor:
2021-02-07T05:15:35.378927Z qemu-system-aarch64: -device
tpm-tis,tpmdev=tpm-tpm0,id=tpm0: 'tpm-tis' is not a valid device model name
Indeed the TPM device name is 'tpm-tis-device' [1][2] for aarch64,
versus the shorter 'tpm-tis' for x86. The devices are the same from
a functional POV, i.e. they both emulate a TPM device conforming to
the TIS specification. Account for the unfortunate name difference
when building the TPM device option in qemuBuildTPMDevStr(). Also
include a test case for 'tpm-tis-device'.
[1] https://qemu.readthedocs.io/en/latest/specs/tpm.html
[2] https://github.com/qemu/qemu/commit/c294ac327ca99342b90bd3a83d2cef9b447afaa7
Signed-off-by: Jim Fehlig
---
src/qemu/qemu_command.c | 3 ++
.../aarch64-tpm.aarch64-latest.args | 37 +++
tests/qemuxml2argvdata/aarch64-tpm.xml| 15
tests/qemuxml2argvtest.c | 1 +
4 files changed, 56 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f0333d4f1a..96b956d7ae 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9203,6 +9203,9 @@ qemuBuildTPMDevStr(const virDomainDef *def,
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
const char *model = virDomainTPMModelTypeToString(tpm->model);
+if (tpm->model == VIR_DOMAIN_TPM_MODEL_TIS && def->os.arch ==
VIR_ARCH_AARCH64)
+model = "tpm-tis-device";
+
virBufferAsprintf(&buf, "%s,tpmdev=tpm-%s,id=%s",
model, tpm->info.alias, tpm->info.alias);
diff --git a/tests/qemuxml2argvdata/aarch64-tpm.aarch64-latest.args
b/tests/qemuxml2argvdata/aarch64-tpm.aarch64-latest.args
new file mode 100644
index 00..94a083d816
--- /dev/null
+++ b/tests/qemuxml2argvdata/aarch64-tpm.aarch64-latest.args
@@ -0,0 +1,37 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-aarch64test \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-aarch64test/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-aarch64test/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-aarch64test/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-aarch64 \
+-name guest=aarch64test,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-aarch64test/master-key.aes \
+-machine virt,accel=tcg,usb=off,dump-guest-core=off,gic-version=2,\
+memory-backend=mach-virt.ram \
+-cpu cortex-a15 \
+-m 1024 \
+-object memory-backend-ram,id=mach-virt.ram,size=1073741824 \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid 496d7ea8-9739-544b-4ebd-ef08be936e8b \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot strict=on \
+-tpmdev emulator,id=tpm-tpm0,chardev=chrtpm \
+-chardev socket,id=chrtpm,path=/dev/test \
+-device tpm-tis-device,tpmdev=tpm-tpm0,id=tpm0 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
+resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/aarch64-tpm.xml
b/tests/qemuxml2argvdata/aarch64-tpm.xml
new file mode 100644
index 00..d338a20f17
--- /dev/null
+++ b/tests/qemuxml2argvdata/aarch64-tpm.xml
@@ -0,0 +1,15 @@
+
+ aarch64test
+ 496d7ea8-9739-544b-4ebd-ef08be936e8b
+ 1048576
+ 1
+
+hvm
+
+
+/usr/bin/qemu-system-aarch64
+
+
+
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index faa71a7a16..d09db77d8c 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2478,6 +2478,7 @@ mymain(void)
DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-enc");
DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-pstate");
DO_TEST_CAPS_LATEST_PPC64("tpm-emulator-spapr");
+DO_TEST_CAPS_ARCH_LATEST("aarch64-tpm", "aarch64");
DO_TEST_PARSE_ERROR("pci-domain-invalid", NONE);
DO_TEST_PARSE_ERROR("pci-bus-invalid", NONE);
--
2.29.2
[PATCH 2/2] qemu: Validate TPM TIS device
TPM devices with model='tpm-tis' are only valid with x86 and aarch64
virt machines. Add a check to qemuValidateDomainDeviceDefTPM() to
ensure VIR_DOMAIN_TPM_MODEL_TIS is only used with these architectures.
Signed-off-by: Jim Fehlig
---
The conditional is a bit distasteful, but so far I haven't come up with
anything better. I aslo worry about future architectures gaining support
for emulated TPM TIS devices.
src/qemu/qemu_validate.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index a70737327e..d6ff5e5eef 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -4299,6 +4299,13 @@ qemuValidateDomainDeviceDefTPM(virDomainTPMDef *tpm,
switch (tpm->model) {
case VIR_DOMAIN_TPM_MODEL_TIS:
+if (!ARCH_IS_X86(def->os.arch) && (def->os.arch != VIR_ARCH_AARCH64)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("TPM model %s is only available for "
+ "x86 and aarch64 guests"),
+ virDomainTPMModelTypeToString(tpm->model));
+return -1;
+}
flag = QEMU_CAPS_DEVICE_TPM_TIS;
break;
case VIR_DOMAIN_TPM_MODEL_CRB:
--
2.29.2
[PATCH 0/2] qemu: swtpm improvements
These patches are fallout from a discussion about TPM devices and aarch64 https://listman.redhat.com/archives/libvir-list/2021-February/msg00526.html Jim Fehlig (2): qemu: Fix swtpm device with aarch64 qemu: Validate TPM TIS device src/qemu/qemu_command.c | 3 ++ src/qemu/qemu_validate.c | 7 .../aarch64-tpm.aarch64-latest.args | 37 +++ tests/qemuxml2argvdata/aarch64-tpm.xml| 15 tests/qemuxml2argvtest.c | 1 + 5 files changed, 63 insertions(+) create mode 100644 tests/qemuxml2argvdata/aarch64-tpm.aarch64-latest.args create mode 100644 tests/qemuxml2argvdata/aarch64-tpm.xml -- 2.29.2
Re: [PATCH v3 2/2] qemu: Do not Use canonical path for system memory
On 2/10/21 5:10 PM, Peter Krempa wrote:
On Wed, Feb 10, 2021 at 14:38:14 +0100, Michal Privoznik wrote:
In commit 88957116c9d3cb4705380c3702c9d4315fb500bb I've adapted
libvirt to QEMU's deprecation of -mem-path and -mem-prealloc and
switched to memory-backend-* even for system memory. My claim was
that that's what QEMU does under the hood anyway. And indeed it
was: see QEMU commit 900c0ba373aada4c13d47d95330aa72ec4067ba5 and
look at function create_default_memdev().
However, then commit d96c4d5f193e0e45beec80a6277728b32875bddb was
merged into QEMU. While it was fixing a bug, it also changed the
create_default_memdev() function in which it started turning off
use of canonical path (by setting
"x-use-canonical-path-for-ramblock-id" attribute to false). This
wasn't documented until QEMU commit XXX. The path affects
s/XXX/actual commit id/
migration - the same path has to be used on the source and on the
destination. Therefore, if there is old guest started with '-m X'
it has "pc.ram" block which doesn't use canonical path and thus
when migrating to newer QEMU which uses memory-backend-* we have
to turn off the canonical path explicitly. Otherwise,
"/objects/pc.ram" path would be expected by QEMU which doesn't
match the source.
Ideally, we would need to set it only for some machine types
(4.0 and older) because newer machine types already do what we
are doing. However, we treat machine types as opaque strings and
therefore we don't want to parse nor inspect their versions. But
then again, newer machine types already do what we are doing in
this commit, so when old machine types are deprecated and removed
we can remove our hack and forget it ever happened.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1912201
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_command.c | 35 ---
src/qemu/qemu_command.h | 3 +-
src/qemu/qemu_hotplug.c | 2 +-
.../disk-vhostuser.x86_64-latest.args | 3 +-
.../hugepages-memaccess3.x86_64-latest.args | 4 +--
5 files changed, 38 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 92036d26c0..699d89de1d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
[...]
@@ -3148,10 +3156,29 @@ qemuBuildMemoryBackendProps(virJSONValuePtr
*backendProps,
if (qemuBuildMemoryBackendPropsShare(props, memAccess) < 0)
return -1;
+
+if (systemMemory)
+disableCanonicalPath = true;
+
} else {
backendType = "memory-backend-ram";
}
+/* This is a terrible hack, but unfortunately there is no better way.
+ * The replacement for '-m X' argument is not simple '-machine
+ * memory-backend' and '-object memory-backend-*,size=X' (which was the
+ * idea). This is because of create_default_memdev() in QEMU sets
+ * 'x-use-canonical-path-for-ramblock-id' attribute to false and is
+ * documented in QEMU in qemu-options.hx under 'memory-backend'. Note
+ * that QEMU consideres 'x-use-canonical-path-for-ramblock-id' stable
+ * and supported despite the 'x-' prefix.
+ * See QEMU commit 8db0b20415c129cf5e577a593a4a0372d90b7cc9.
+ */
+if (disableCanonicalPath &&
+virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_X_USE_CANONICAL_PATH_FOR_RAMBLOCK_ID)
&&
+virJSONValueObjectAdd(props, "b:x-use-canonical-path-for-ramblock-id",
false, NULL) < 0)
+return -1;
I don't remember whether you and Daniel reached some consensus about the
machine type version check, but if the consensus is to always output it
then okay.
Yeah, this is the consensus. The reason is that distros may introduce
their own machine types with version appearing at unexpected place (or
not appearing at all). Also, finding an ordering function is not trivial
and we also would need to know which machine types have the feature
turned on/off by default. Since QEMU claims it is safe to turn the
feature off at all times, let's not bother with parsing anything and
just turn it off always.
From my side:
Reviewed-by: Peter Krempa
Thanks, pushed.
Michal
[PATCH 2/7] conf: use virDomainNetTeamingInfoPtr instead of virDomainNetTeamingInfo
To make it easier to split out the parsing/formatting of the
element into separate functions (so we can more easily add the
element to , change its virDomainNetDef so that it
points to a virDomainNetTeamingInfo rather than containing one.
Signed-off-by: Laine Stump
---
src/conf/domain_conf.c | 31
src/conf/domain_conf.h | 2 +-
src/conf/domain_validate.c | 26 ---
src/qemu/qemu_command.c| 10 -
src/qemu/qemu_domain.c | 2 +-
src/qemu/qemu_migration.c | 4 ++--
src/qemu/qemu_validate.c | 42 --
7 files changed, 63 insertions(+), 54 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7d7acb940a..bbf54c90ba 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2542,7 +2542,7 @@ virDomainNetDefFree(virDomainNetDefPtr def)
g_free(def->backend.tap);
g_free(def->backend.vhost);
-g_free(def->teaming.persistent);
+virDomainNetTeamingInfoFree(def->teaming);
g_free(def->virtPortProfile);
g_free(def->script);
g_free(def->downscript);
@@ -11447,18 +11447,23 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
}
}
-if (teamingType) {
-int tmpTeaming;
+if (teamingType || teamingPersistent) {
+def->teaming = g_new0(virDomainNetTeamingInfo, 1);
-if ((tmpTeaming = virDomainNetTeamingTypeFromString(teamingType)) <=
0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown teaming type '%s'"),
- teamingType);
-goto error;
+if (teamingType) {
+int tmpTeaming;
+
+if ((tmpTeaming = virDomainNetTeamingTypeFromString(teamingType))
<= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown teaming type '%s'"),
+ teamingType);
+goto error;
+}
+def->teaming->type = tmpTeaming;
}
-def->teaming.type = tmpTeaming;
+
+def->teaming->persistent = g_steal_pointer(&teamingPersistent);
}
-def->teaming.persistent = g_steal_pointer(&teamingPersistent);
rv = virXPathULong("string(./tune/sndbuf)", ctxt, &def->tune.sndbuf);
if (rv >= 0) {
@@ -25825,10 +25830,10 @@ virDomainNetDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "\n");
}
-if (def->teaming.type != VIR_DOMAIN_NET_TEAMING_TYPE_NONE) {
+if (def->teaming && def->teaming->type !=
VIR_DOMAIN_NET_TEAMING_TYPE_NONE) {
virBufferAsprintf(buf, "teaming.type));
-virBufferEscapeString(buf, " persistent='%s'",
def->teaming.persistent);
+ virDomainNetTeamingTypeToString(def->teaming->type));
+virBufferEscapeString(buf, " persistent='%s'",
def->teaming->persistent);
virBufferAddLit(buf, "/>\n");
}
if (def->linkstate) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 92fe588b3f..fb695a212b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1038,7 +1038,7 @@ struct _virDomainNetDef {
char *tap;
char *vhost;
} backend;
-virDomainNetTeamingInfo teaming;
+virDomainNetTeamingInfoPtr teaming;
union {
virDomainChrSourceDefPtr vhostuser;
struct {
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index b6f53886cd..703946b3e5 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -1507,18 +1507,20 @@ virDomainNetDefValidate(const virDomainNetDef *net)
return -1;
}
-if (net->teaming.type == VIR_DOMAIN_NET_TEAMING_TYPE_TRANSIENT) {
-if (!net->teaming.persistent) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("teaming persistent attribute must be set if
teaming type is 'transient'"));
-return -1;
-}
-} else {
-if (net->teaming.persistent) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("teaming persistent attribute not allowed if
teaming type is '%s'"),
- virDomainNetTeamingTypeToString(net->teaming.type));
-return -1;
+if (net->teaming) {
+if (net->teaming->type == VIR_DOMAIN_NET_TEAMING_TYPE_TRANSIENT) {
+if (!net->teaming->persistent) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("teaming persistent attribute must be set if
teaming type is 'transient'"));
+return -1;
+}
+} else {
+if (net->teaming->persistent) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("teaming persistent attribute not allowed if
teaming type is '%s'"),
+
virDomainNetTeamingTypeToString(net->teaming->typ
[PATCH 5/7] conf: parse/format element in plain
The element in allows pairing two interfaces
together as a simple "failover bond" network device in a guest. One of
the devices will be the "transient" interface - it will be preferred
for all network traffic when it is present, but may be removed when
necessary, in particular during migration, when traffic will instead
go through the other interface of the pair - the "persistent"
interface. As it happens, in the QEMU implementation of this teaming
pair (called "virtio failover" in QEMU) the transient interface is
always a host network device assigned to the guest using VFIO (aka
"hostdev"); the persistent interface is always an emulated virtio NIC.
When support was initially added for , it was written to
require that the transient/hostdev device be defined using ; this was done because the virtio failover
implementation in QEMU and the virtio guest driver demands that the
two interfaces in the pair have matching MAC addresses, and the only
way libvirt can guarantee the MAC address of a hostdev network device
is to use , whose main purpose is to
configure the device's MAC address. (note that in turn requires that the network device be an SRIOV
VF (Virtual Function), as that is the only type of network device
whose MAC address we can set in a way that will survive the device's
driver init in the guest).
It has recently come up that some users are unable to use
because they are running in a container environment where libvirt
doesn't have the necessary privileges or resources to set the VF's MAC
address (because setting the VF MAC is done via the same device's PF
(Physical Function), and the PF is not exposed to libvirt's container.
At the same time, they *are* able to set the VF's MAC address in
advance of staring up libvirt in the container. So they could
theoretically use the feature if libvirt just skipped the
"setting the MAC address" part.
Fortunately, that is *exactly* the difference between (a "hostdev VF") and (a "plain hostdev" - it
could be *any PCI device; libvirt doesn't know what type of PCI device
it is, and doesn't care).
But what *is* still needed is for libvirt to provide a small bit of
information on the commandline argument for the hostdev, telling QEMU
that this device will be part of a team ("failover pair"), and the id
of the other device in the pair.
So, what we need to do is add support for the element to
plain , and that is what this patch does.
(actually, this patch adds parsing/formatting of the element
in . The next patch will actually wire that into the qemu
driver.)
Signed-off-by: Laine Stump
---
docs/formatdomain.rst | 51 +++
docs/schemas/domaincommon.rng | 3 +
src/conf/domain_conf.c| 5 ++
src/conf/domain_conf.h| 1 +
src/conf/domain_validate.c| 19 ++
.../net-virtio-teaming-hostdev.xml| 48 ++
.../net-virtio-teaming-hostdev.xml| 64 +++
tests/qemuxml2xmltest.c | 3 +
8 files changed, 194 insertions(+)
create mode 100644 tests/qemuxml2argvdata/net-virtio-teaming-hostdev.xml
create mode 100644 tests/qemuxml2xmloutdata/net-virtio-teaming-hostdev.xml
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 2493be595f..eafd6b3396 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -4837,6 +4837,22 @@ support in the hypervisor and the guest network driver).
...
+The second interface in this example is referencing a network that is
+a pool of SRIOV VFs (i.e. a "hostdev network"). You could instead
+directly reference an SRIOV VF device:
+
+::
+
+ ...
+
+
+
+
+
+
+
+ ...
+
The element required attribute ``type`` will be set to either
``"persistent"`` to indicate a device that should always be present in the
domain, or ``"transient"`` to indicate a device that may periodically be
@@ -4858,6 +4874,41 @@ once migration is completed; while migration is taking
place, network traffic
will use the virtio NIC. (Of course the emulated virtio NIC and the hostdev NIC
must be connected to the same subnet for bonding to work properly).
+:since:`Since 7.1.0` The element can also be added to a
+plain device.
+
+::
+
+ ...
+
+
+
+
+
+
+
+ ...
+
+This device must be a network device, but not necessarily an SRIOV
+VF. Using plain rather than or is useful if the
+device that will be assigned with VFIO is a standard NIC (not a VF) or
+if libvirt doesn't have the necessary resources and privileges to set
+the VF's MAC address (e.g. if libvirt is running unprivileged, or in a
+container). This of course means that the user (or another
+application) is responsible for setting the MAC address of the device
+in a way such that it will survive guest driver initialization. For
+standard NICs (i.e. not an SRIOV VF) thi
[PATCH 0/7] support (aka "QEMU virtio failover") with plain
This is explained in excruciating detail in Patch 5, but in short, allowing the element to be in a plain will permit someone who is running libvirt unprivileged, or in a container with no access to a PF device, to use the feature, which encapsulates a lot of functionality related to assigning an SRIOV network device to a guest as a part of a failover bond device (the other device in the pair is an emulated virtio NIC), which in turn permits the guest to be migrated by transparently unplugging the SRIOV NIC prior to migration, and plugging a new one in after migration is completed. (Previously we required for this feature, but that type of device needs to send netlink messages to the PF of the SRIOV VF that's being assigned, and that simply isn't possible sometimes.) Laine Stump (7): conf: make teaming info an official type conf: use virDomainNetTeamingInfoPtr instead of virDomainNetTeamingInfo conf: separate Parse/Format functions for virDomainNetTeamingInfo schema: separate teaming element definition from interface element conf: parse/format element in plain qemu: plug config from into qemu commandline news: document support for in NEWS.rst | 6 ++ docs/formatdomain.rst | 51 +++ docs/schemas/domaincommon.rng | 42 + src/conf/domain_conf.c| 87 +-- src/conf/domain_conf.h| 13 ++- src/conf/domain_validate.c| 45 +++--- src/conf/virconftypes.h | 3 + src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 17 ++-- src/qemu/qemu_domain.c| 2 +- src/qemu/qemu_migration.c | 4 +- src/qemu/qemu_validate.c | 42 - .../net-virtio-teaming-hostdev.args | 40 + .../net-virtio-teaming-hostdev.xml| 48 ++ tests/qemuxml2argvtest.c | 3 + .../net-virtio-teaming-hostdev.xml| 64 ++ tests/qemuxml2xmltest.c | 3 + 17 files changed, 384 insertions(+), 87 deletions(-) create mode 100644 tests/qemuxml2argvdata/net-virtio-teaming-hostdev.args create mode 100644 tests/qemuxml2argvdata/net-virtio-teaming-hostdev.xml create mode 100644 tests/qemuxml2xmloutdata/net-virtio-teaming-hostdev.xml -- 2.29.2
[PATCH 1/7] conf: make teaming info an official type
This struct was previously defined only within virDomainNetDef where
it was used, but I need to also use it in virDomainHostdevDef, so move
the internal struct out to its own "official" struct and give it the
standard typedef duo and *Free() function.
Signed-off-by: Laine Stump
---
src/conf/domain_conf.c | 11 +++
src/conf/domain_conf.h | 12
src/conf/virconftypes.h | 3 +++
src/libvirt_private.syms | 1 +
4 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8f2207bdf6..7d7acb940a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2476,6 +2476,17 @@ virDomainVsockDefFree(virDomainVsockDefPtr vsock)
}
+void
+virDomainNetTeamingInfoFree(virDomainNetTeamingInfoPtr teaming)
+{
+if (!teaming)
+return;
+
+g_free(teaming->persistent);
+g_free(teaming);
+}
+
+
void
virDomainNetDefFree(virDomainNetDefPtr def)
{
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 8b1c8643be..92fe588b3f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -956,6 +956,11 @@ typedef enum {
VIR_DOMAIN_NET_TEAMING_TYPE_LAST
} virDomainNetTeamingType;
+struct _virDomainNetTeamingInfo {
+virDomainNetTeamingType type;
+char *persistent; /* alias name of persistent device */
+};
+
/* link interface states */
typedef enum {
VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT = 0, /* Default link state
(up) */
@@ -1033,10 +1038,7 @@ struct _virDomainNetDef {
char *tap;
char *vhost;
} backend;
-struct {
-virDomainNetTeamingType type;
-char *persistent; /* alias name of persistent device */
-} teaming;
+virDomainNetTeamingInfo teaming;
union {
virDomainChrSourceDefPtr vhostuser;
struct {
@@ -3100,6 +3102,8 @@ void virDomainActualNetDefFree(virDomainActualNetDefPtr
def);
virDomainVsockDefPtr virDomainVsockDefNew(virDomainXMLOptionPtr xmlopt);
void virDomainVsockDefFree(virDomainVsockDefPtr vsock);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainVsockDef, virDomainVsockDefFree);
+void virDomainNetTeamingInfoFree(virDomainNetTeamingInfoPtr teaming);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainNetTeamingInfo,
virDomainNetTeamingInfoFree);
void virDomainNetDefFree(virDomainNetDefPtr def);
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainNetDef, virDomainNetDefFree);
void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def);
diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h
index 9042a2b34f..c51241cfa5 100644
--- a/src/conf/virconftypes.h
+++ b/src/conf/virconftypes.h
@@ -246,6 +246,9 @@ typedef virDomainNVRAMDef *virDomainNVRAMDefPtr;
typedef struct _virDomainNetDef virDomainNetDef;
typedef virDomainNetDef *virDomainNetDefPtr;
+typedef struct _virDomainNetTeamingInfo virDomainNetTeamingInfo;
+typedef virDomainNetTeamingInfo *virDomainNetTeamingInfoPtr;
+
typedef struct _virDomainOSDef virDomainOSDef;
typedef virDomainOSDef *virDomainOSDefPtr;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 837986845f..affa2df323 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -537,6 +537,7 @@ virDomainNetRemove;
virDomainNetRemoveHostdev;
virDomainNetResolveActualType;
virDomainNetSetModelString;
+virDomainNetTeamingInfoFree;
virDomainNetTypeFromString;
virDomainNetTypeSharesHostView;
virDomainNetTypeToString;
--
2.29.2
[PATCH 4/7] schema: separate teaming element definition from interface element
Signed-off-by: Laine Stump --- docs/schemas/domaincommon.rng | 39 --- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 7a2706a4fb..31960fb7cf 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3529,23 +3529,7 @@ - - - - -persistent - - - - -transient - - - - - - - + @@ -3581,6 +3565,27 @@ + + + + + + +persistent + + + + +transient + + + + + + + + + -- 2.29.2
[PATCH 7/7] news: document support for in
Signed-off-by: Laine Stump --- NEWS.rst | 6 ++ 1 file changed, 6 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index c6ae6a6c60..a9a1a61119 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -35,6 +35,12 @@ v7.1.0 (unreleased) ``virNetworkGetXMLDesc()``, and ``virDomainScreenshot()``, APIs have been implemented in the Hyper-V driver. + * Support element in plain devices + +This is useful when libvirt doesn't have the privileges necessary +to set the hostdev device's MAC address (which is a necessary +part of the alternate ). + * **Improvements** * **Bug fixes** -- 2.29.2
[PATCH 3/7] conf: separate Parse/Format functions for virDomainNetTeamingInfo
In preparation for using the same element in two places, split the
parsing/formating for that subelement out of the virDomainNetDef
functions into their own functions.
Signed-off-by: Laine Stump
---
src/conf/domain_conf.c | 74 +-
1 file changed, 45 insertions(+), 29 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index bbf54c90ba..3fe8517f39 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10629,6 +10629,34 @@
virDomainChrSourceReconnectDefParseXML(virDomainChrSourceReconnectDefPtr def,
}
+static int
+virDomainNetTeamingInfoParseXML(xmlXPathContextPtr ctxt,
+virDomainNetTeamingInfoPtr *teaming)
+{
+g_autofree char *typeStr = virXPathString("string(./teaming/@type)", ctxt);
+g_autofree char *persistentStr =
virXPathString("string(./teaming/@persistent)", ctxt);
+g_autoptr(virDomainNetTeamingInfo) tmpTeaming = NULL;
+int tmpType;
+
+if (!typeStr && !persistentStr)
+return 0;
+
+tmpTeaming = g_new0(virDomainNetTeamingInfo, 1);
+
+if ((tmpType = virDomainNetTeamingTypeFromString(typeStr)) <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown teaming type '%s'"),
+ typeStr);
+return -1;
+}
+
+tmpTeaming->type = tmpType;
+tmpTeaming->persistent = g_steal_pointer(&persistentStr);
+*teaming = g_steal_pointer(&tmpTeaming);
+return 0;
+}
+
+
static virDomainNetDefPtr
virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlNodePtr node,
@@ -10683,8 +10711,6 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
g_autofree char *vhostuser_type = NULL;
g_autofree char *trustGuestRxFilters = NULL;
g_autofree char *vhost_path = NULL;
-g_autofree char *teamingType = NULL;
-g_autofree char *teamingPersistent = NULL;
const char *prefix = xmlopt ? xmlopt->config.netPrefix : NULL;
if (!(def = virDomainNetDefNew(xmlopt)))
@@ -10895,10 +10921,6 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
if (!vhost_path && (tmp = virXMLPropString(cur, "vhost")))
vhost_path = virFileSanitizePath(tmp);
VIR_FREE(tmp);
-} else if (virXMLNodeNameEqual(cur, "teaming") &&
- !teamingType && !teamingPersistent) {
-teamingType = virXMLPropString(cur, "type");
-teamingPersistent = virXMLPropString(cur, "persistent");
}
}
cur = cur->next;
@@ -11447,23 +11469,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
}
}
-if (teamingType || teamingPersistent) {
-def->teaming = g_new0(virDomainNetTeamingInfo, 1);
-
-if (teamingType) {
-int tmpTeaming;
-
-if ((tmpTeaming = virDomainNetTeamingTypeFromString(teamingType))
<= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown teaming type '%s'"),
- teamingType);
-goto error;
-}
-def->teaming->type = tmpTeaming;
-}
-
-def->teaming->persistent = g_steal_pointer(&teamingPersistent);
-}
+if (virDomainNetTeamingInfoParseXML(ctxt, &def->teaming) < 0)
+goto error;
rv = virXPathULong("string(./tune/sndbuf)", ctxt, &def->tune.sndbuf);
if (rv >= 0) {
@@ -25513,6 +25520,19 @@ virDomainChrSourceReconnectDefFormat(virBufferPtr buf,
}
+static void
+virDomainNetTeamingInfoFormat(virDomainNetTeamingInfoPtr teaming,
+ virBufferPtr buf)
+{
+if (teaming && teaming->type != VIR_DOMAIN_NET_TEAMING_TYPE_NONE) {
+virBufferAsprintf(buf, "type));
+virBufferEscapeString(buf, " persistent='%s'", teaming->persistent);
+virBufferAddLit(buf, "/>\n");
+}
+}
+
+
int
virDomainNetDefFormat(virBufferPtr buf,
virDomainNetDefPtr def,
@@ -25830,12 +25850,8 @@ virDomainNetDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "\n");
}
-if (def->teaming && def->teaming->type !=
VIR_DOMAIN_NET_TEAMING_TYPE_NONE) {
-virBufferAsprintf(buf, "teaming->type));
-virBufferEscapeString(buf, " persistent='%s'",
def->teaming->persistent);
-virBufferAddLit(buf, "/>\n");
-}
+virDomainNetTeamingInfoFormat(def->teaming, buf);
+
if (def->linkstate) {
virBufferAsprintf(buf, "\n",
virDomainNetInterfaceLinkStateTypeToString(def->linkstate));
--
2.29.2
