[libvirt PATCH 00/10] Cleanup and test more firmware handling scenarios

Daniel P. Berrangé posted 10 patches 2 years, 2 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
src/conf/domain_conf.c                        | 121 +++++++++++-------
src/conf/domain_conf.h                        |   2 +-
src/qemu/qemu_domain.c                        |  18 +--
src/qemu/qemu_domain.h                        |   8 +-
src/qemu/qemu_firmware.c                      |  13 +-
src/qemu/qemu_process.c                       |   4 +-
tests/qemuxml2argvdata/bios-nvram-no-path.err |   1 +
tests/qemuxml2argvdata/bios-nvram-no-path.xml |  19 +++
...-nvram-rw-template-vars.x86_64-latest.args |  41 ++++++
.../bios-nvram-rw-template-vars.xml           |  36 ++++++
.../bios-nvram-rw-template.err                |   1 +
.../bios-nvram-rw-template.xml                |  36 ++++++
.../bios-nvram-rw-vars.x86_64-latest.args     |  41 ++++++
tests/qemuxml2argvdata/bios-nvram-rw-vars.xml |  36 ++++++
.../bios-nvram-template.x86_64-latest.args    |  37 ++++++
.../qemuxml2argvdata/bios-nvram-template.xml  |  21 +++
tests/qemuxml2argvdata/os-firmware-bios.xml   |   1 -
.../os-firmware-efi-bad-loader-path.err       |   1 +
.../os-firmware-efi-bad-loader-path.xml       |  67 ++++++++++
.../os-firmware-efi-bad-loader-type.err       |   1 +
.../os-firmware-efi-bad-loader-type.xml       |  67 ++++++++++
.../os-firmware-efi-bad-nvram-path.err        |   1 +
.../os-firmware-efi-bad-nvram-path.xml        |  68 ++++++++++
.../os-firmware-efi-bad-nvram-template.err    |   1 +
.../os-firmware-efi-bad-nvram-template.xml    |  68 ++++++++++
.../os-firmware-efi-secboot.xml               |   1 -
tests/qemuxml2argvdata/os-firmware-efi.xml    |   1 -
tests/qemuxml2argvtest.c                      |   9 ++
.../os-firmware-bios.x86_64-latest.xml        |   1 -
.../os-firmware-efi-secboot.x86_64-latest.xml |   1 -
.../os-firmware-efi.x86_64-latest.xml         |   1 -
31 files changed, 647 insertions(+), 77 deletions(-)
create mode 100644 tests/qemuxml2argvdata/bios-nvram-no-path.err
create mode 100644 tests/qemuxml2argvdata/bios-nvram-no-path.xml
create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template-vars.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template-vars.xml
create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template.err
create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template.xml
create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-vars.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-vars.xml
create mode 100644 tests/qemuxml2argvdata/bios-nvram-template.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/bios-nvram-template.xml
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-loader-path.err
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-loader-path.xml
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-loader-type.err
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-loader-type.xml
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-path.err
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-path.xml
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-template.err
create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-template.xml
[libvirt PATCH 00/10] Cleanup and test more firmware handling scenarios
Posted by Daniel P. Berrangé 2 years, 2 months ago
There are a mind bending number of possible ways to configure the
firmware with/without NVRAM. Only a small portion are tested and
many error scenarios are silently ignored.

This series attempts to get coverage of every possible XML config
scenario and report explicit errors in all invalid configs.

There is an open question on patch 4.  Essentially the use of NVRAM
combined with writable executable feels like an accidental feature
in libvirt that hasn't really been thought through. I'd like to
better define expectations here but there are several possible
strategies and I'm undecided which is best.

Daniel P. Berrangé (10):
  qemu: fix bad indentation for qemuDomainNVRAMPathFormat
  tests: add explicit test case for pflash loader lacking path
  tests: add test case for NVRAM with template
  conf: validate NVRAM template usage with R/W loader binary
  tests: don't permit NVRAM path when using firmware auto-select
  qemu: inline code for filling in per-VM NVRAM path
  conf: rename struct field for NVRAM template
  conf: switch nvram parsing to use XML node / property helpers
  conf: move nvram parsing into virDomainLoaderDefParseXML
  conf: stop ignoring <loader>/<nvram> with firmware auto-select

 src/conf/domain_conf.c                        | 121 +++++++++++-------
 src/conf/domain_conf.h                        |   2 +-
 src/qemu/qemu_domain.c                        |  18 +--
 src/qemu/qemu_domain.h                        |   8 +-
 src/qemu/qemu_firmware.c                      |  13 +-
 src/qemu/qemu_process.c                       |   4 +-
 tests/qemuxml2argvdata/bios-nvram-no-path.err |   1 +
 tests/qemuxml2argvdata/bios-nvram-no-path.xml |  19 +++
 ...-nvram-rw-template-vars.x86_64-latest.args |  41 ++++++
 .../bios-nvram-rw-template-vars.xml           |  36 ++++++
 .../bios-nvram-rw-template.err                |   1 +
 .../bios-nvram-rw-template.xml                |  36 ++++++
 .../bios-nvram-rw-vars.x86_64-latest.args     |  41 ++++++
 tests/qemuxml2argvdata/bios-nvram-rw-vars.xml |  36 ++++++
 .../bios-nvram-template.x86_64-latest.args    |  37 ++++++
 .../qemuxml2argvdata/bios-nvram-template.xml  |  21 +++
 tests/qemuxml2argvdata/os-firmware-bios.xml   |   1 -
 .../os-firmware-efi-bad-loader-path.err       |   1 +
 .../os-firmware-efi-bad-loader-path.xml       |  67 ++++++++++
 .../os-firmware-efi-bad-loader-type.err       |   1 +
 .../os-firmware-efi-bad-loader-type.xml       |  67 ++++++++++
 .../os-firmware-efi-bad-nvram-path.err        |   1 +
 .../os-firmware-efi-bad-nvram-path.xml        |  68 ++++++++++
 .../os-firmware-efi-bad-nvram-template.err    |   1 +
 .../os-firmware-efi-bad-nvram-template.xml    |  68 ++++++++++
 .../os-firmware-efi-secboot.xml               |   1 -
 tests/qemuxml2argvdata/os-firmware-efi.xml    |   1 -
 tests/qemuxml2argvtest.c                      |   9 ++
 .../os-firmware-bios.x86_64-latest.xml        |   1 -
 .../os-firmware-efi-secboot.x86_64-latest.xml |   1 -
 .../os-firmware-efi.x86_64-latest.xml         |   1 -
 31 files changed, 647 insertions(+), 77 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/bios-nvram-no-path.err
 create mode 100644 tests/qemuxml2argvdata/bios-nvram-no-path.xml
 create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template-vars.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template-vars.xml
 create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template.err
 create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template.xml
 create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-vars.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-vars.xml
 create mode 100644 tests/qemuxml2argvdata/bios-nvram-template.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/bios-nvram-template.xml
 create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-loader-path.err
 create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-loader-path.xml
 create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-loader-type.err
 create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-loader-type.xml
 create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-path.err
 create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-path.xml
 create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-template.err
 create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-template.xml

-- 
2.34.1


Re: [libvirt PATCH 00/10] Cleanup and test more firmware handling scenarios
Posted by Michal Prívozník 2 years, 2 months ago
On 2/15/22 19:54, Daniel P. Berrangé wrote:
> There are a mind bending number of possible ways to configure the
> firmware with/without NVRAM. Only a small portion are tested and
> many error scenarios are silently ignored.
> 
> This series attempts to get coverage of every possible XML config
> scenario and report explicit errors in all invalid configs.
> 
> There is an open question on patch 4.  Essentially the use of NVRAM
> combined with writable executable feels like an accidental feature
> in libvirt that hasn't really been thought through. I'd like to
> better define expectations here but there are several possible
> strategies and I'm undecided which is best.
> 
> Daniel P. Berrangé (10):
>   qemu: fix bad indentation for qemuDomainNVRAMPathFormat
>   tests: add explicit test case for pflash loader lacking path
>   tests: add test case for NVRAM with template
>   conf: validate NVRAM template usage with R/W loader binary
>   tests: don't permit NVRAM path when using firmware auto-select
>   qemu: inline code for filling in per-VM NVRAM path
>   conf: rename struct field for NVRAM template
>   conf: switch nvram parsing to use XML node / property helpers
>   conf: move nvram parsing into virDomainLoaderDefParseXML
>   conf: stop ignoring <loader>/<nvram> with firmware auto-select
> 
>  src/conf/domain_conf.c                        | 121 +++++++++++-------
>  src/conf/domain_conf.h                        |   2 +-
>  src/qemu/qemu_domain.c                        |  18 +--
>  src/qemu/qemu_domain.h                        |   8 +-
>  src/qemu/qemu_firmware.c                      |  13 +-
>  src/qemu/qemu_process.c                       |   4 +-
>  tests/qemuxml2argvdata/bios-nvram-no-path.err |   1 +
>  tests/qemuxml2argvdata/bios-nvram-no-path.xml |  19 +++
>  ...-nvram-rw-template-vars.x86_64-latest.args |  41 ++++++
>  .../bios-nvram-rw-template-vars.xml           |  36 ++++++
>  .../bios-nvram-rw-template.err                |   1 +
>  .../bios-nvram-rw-template.xml                |  36 ++++++
>  .../bios-nvram-rw-vars.x86_64-latest.args     |  41 ++++++
>  tests/qemuxml2argvdata/bios-nvram-rw-vars.xml |  36 ++++++
>  .../bios-nvram-template.x86_64-latest.args    |  37 ++++++
>  .../qemuxml2argvdata/bios-nvram-template.xml  |  21 +++
>  tests/qemuxml2argvdata/os-firmware-bios.xml   |   1 -
>  .../os-firmware-efi-bad-loader-path.err       |   1 +
>  .../os-firmware-efi-bad-loader-path.xml       |  67 ++++++++++
>  .../os-firmware-efi-bad-loader-type.err       |   1 +
>  .../os-firmware-efi-bad-loader-type.xml       |  67 ++++++++++
>  .../os-firmware-efi-bad-nvram-path.err        |   1 +
>  .../os-firmware-efi-bad-nvram-path.xml        |  68 ++++++++++
>  .../os-firmware-efi-bad-nvram-template.err    |   1 +
>  .../os-firmware-efi-bad-nvram-template.xml    |  68 ++++++++++
>  .../os-firmware-efi-secboot.xml               |   1 -
>  tests/qemuxml2argvdata/os-firmware-efi.xml    |   1 -
>  tests/qemuxml2argvtest.c                      |   9 ++
>  .../os-firmware-bios.x86_64-latest.xml        |   1 -
>  .../os-firmware-efi-secboot.x86_64-latest.xml |   1 -
>  .../os-firmware-efi.x86_64-latest.xml         |   1 -
>  31 files changed, 647 insertions(+), 77 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-no-path.err
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-no-path.xml
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template-vars.x86_64-latest.args
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template-vars.xml
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template.err
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-template.xml
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-vars.x86_64-latest.args
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-rw-vars.xml
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-template.x86_64-latest.args
>  create mode 100644 tests/qemuxml2argvdata/bios-nvram-template.xml
>  create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-loader-path.err
>  create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-loader-path.xml
>  create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-loader-type.err
>  create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-loader-type.xml
>  create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-path.err
>  create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-path.xml
>  create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-template.err
>  create mode 100644 tests/qemuxml2argvdata/os-firmware-efi-bad-nvram-template.xml
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

Re: [libvirt PATCH 00/10] Cleanup and test more firmware handling scenarios
Posted by Cole Robinson 2 years ago
On 2/16/22 8:17 AM, Michal Prívozník wrote:
> On 2/15/22 19:54, Daniel P. Berrangé wrote:
>> There are a mind bending number of possible ways to configure the
>> firmware with/without NVRAM. Only a small portion are tested and
>> many error scenarios are silently ignored.
>>
>> This series attempts to get coverage of every possible XML config
>> scenario and report explicit errors in all invalid configs.
>>
>> There is an open question on patch 4.  Essentially the use of NVRAM
>> combined with writable executable feels like an accidental feature
>> in libvirt that hasn't really been thought through. I'd like to
>> better define expectations here but there are several possible
>> strategies and I'm undecided which is best.
>>
>> Daniel P. Berrangé (10):
>>   qemu: fix bad indentation for qemuDomainNVRAMPathFormat
>>   tests: add explicit test case for pflash loader lacking path
>>   tests: add test case for NVRAM with template
>>   conf: validate NVRAM template usage with R/W loader binary
>>   tests: don't permit NVRAM path when using firmware auto-select
>>   qemu: inline code for filling in per-VM NVRAM path
>>   conf: rename struct field for NVRAM template
>>   conf: switch nvram parsing to use XML node / property helpers
>>   conf: move nvram parsing into virDomainLoaderDefParseXML
>>   conf: stop ignoring <loader>/<nvram> with firmware auto-select
>>

>>
> 
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> 
> Michal
> 

I don't see the last 3 patches in git. Daniel was that intentional?

Thanks,
Cole

Re: [libvirt PATCH 00/10] Cleanup and test more firmware handling scenarios
Posted by Daniel P. Berrangé 2 years ago
On Mon, Apr 04, 2022 at 02:45:41PM -0400, Cole Robinson wrote:
> On 2/16/22 8:17 AM, Michal Prívozník wrote:
> > On 2/15/22 19:54, Daniel P. Berrangé wrote:
> >> There are a mind bending number of possible ways to configure the
> >> firmware with/without NVRAM. Only a small portion are tested and
> >> many error scenarios are silently ignored.
> >>
> >> This series attempts to get coverage of every possible XML config
> >> scenario and report explicit errors in all invalid configs.
> >>
> >> There is an open question on patch 4.  Essentially the use of NVRAM
> >> combined with writable executable feels like an accidental feature
> >> in libvirt that hasn't really been thought through. I'd like to
> >> better define expectations here but there are several possible
> >> strategies and I'm undecided which is best.
> >>
> >> Daniel P. Berrangé (10):
> >>   qemu: fix bad indentation for qemuDomainNVRAMPathFormat
> >>   tests: add explicit test case for pflash loader lacking path
> >>   tests: add test case for NVRAM with template
> >>   conf: validate NVRAM template usage with R/W loader binary
> >>   tests: don't permit NVRAM path when using firmware auto-select
> >>   qemu: inline code for filling in per-VM NVRAM path
> >>   conf: rename struct field for NVRAM template
> >>   conf: switch nvram parsing to use XML node / property helpers
> >>   conf: move nvram parsing into virDomainLoaderDefParseXML
> >>   conf: stop ignoring <loader>/<nvram> with firmware auto-select
> >>
> 
> >>
> > 
> > Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> > 
> > Michal
> > 
> 
> I don't see the last 3 patches in git. Daniel was that intentional?

I didn't merge the ones with comments from Michal on, and that also
invalidated the last few. I need to revisit the remaining ones again.


With 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 :|