Andrea, any hint what might have changed?

(In reply to Jiri Kortus from comment #0)
> Description of problem:
> Running virt-install with parameters aiming to boot from CD-ROM in UEFI mode
> with custom loader in read-only mode fails with the following error message:
>
> ERROR loader attribute 'readonly' cannot be specified when firmware
> autoselection is enabled

Hi Jiri,

the behavior you're seeing is the intended one. It was changed on
purpose with

  commit de2a338dcebde096d4806e43702513284607db8f
  Author: Andrea Bolognani <abologna>
  Date:   Wed Jun 15 12:00:58 2022 +0200

    conf: Validate firmware configuration more thoroughly
    
    Generally speaking, when firmware autoselection is in use we
    don't want any information to be provided manually. There are
    two exceptions:
    
      * we still want the path to the NVRAM file to be customizable;
    
      * using <loader secure='yes'/> was how you would ask for a
        firmware that implements the Secure Boot feature in the
        original approach to firmware autoselection, so we want to
        keep that working.
    
    Anything else should result in a descriptive error.
    
    Resolves: https://gitlab.com/libvirt/libvirt/-/issues/327
    Signed-off-by: Andrea Bolognani <abologna>
    Reviewed-by: Michal Privoznik <mprivozn>

and surrounding commits, all of which are included in libvirt 8.6.0.  

Basically, libvirt supports two different ways of selecting firmware:
autoselection, where the firmware is automatically picked based on
requested features (if any), and manual mode, where all information
about the firmware are provided explicitly by the user.

In virt-install terms, these correspond to

  --boot uefi

and

  --boot loader=...,loader_ro=...,loader_type=...,nvram_template=...

respectively.

Up until the commit mentioned above, libvirt would allow mixing the
two, which would often result in confusing behaviors. Now, the two
are more clearly separated and mixing them results in an error.

> I guess this is a special case and in the end I found out I can probably get
> by just fine without specifying the loader -- with only '--boot=uefi' (the
> remaining --boot parameters were probably some relics and probably were
> needed for the test in the past).

Correct.

> However, as mentioned earlier, this
> combination worked fine in previous versions of virt-install and I guess
> there should be some way to boot from regular (cdrom, pxe, ...) source in
> UEFI mode with a custom firmware where the read-only mode might still make
> sense.

This is still possible: simply omit the 'uefi' token and you will
switch to manual firmware selection mode, where all the other
attributes are accepted and, indeed, expected.


Cole, do you think it would make sense to validate this upfront in
virt-manager and provide a nicer, more actionable error message?  I'm
thinking of something along the lines of

  Conflicting firmware selection request detected.

  Use either '--boot uefi' for firmware autoselection or '--boot
  loader=...,loader_ro=...,loader_type=...,nvram_template=... for
  manual firmware selection.

Hi Andrea,

thank you for the clarification, now it makes more sense to me. A nicer, more descriptive error message would definitely be beneficial. I guess there may be uses like mine (with --boot uefi,loader=...,... for historical reasons) where the users will face the same error 'out of the blue' after they update virt-manager and such a hint might be useful.

Thank you,
Jirka

I generally don't like adding those types of checks to virt-install. Maybe in the future libvirt re-allows overriding one of those values when firmware=efi is used, and now our virt-install check will block legitimate usage. Or another new property is added to libvirt XML, which is also incompatible with firmware=efi, and we forget to update the list in our validation check, so the error message is inaccurate.

The libvirt error message is descriptive enough IMO, so I don't think we need to change anything in virt-install. Unless we get a lot of reports about it, like if that `--boot` line is copied from man page or some common blog post etc.

(In reply to Cole Robinson from comment #4)
> I generally don't like adding those types of checks to virt-install. Maybe
> in the future libvirt re-allows overriding one of those values when
> firmware=efi is used, and now our virt-install check will block legitimate
> usage. Or another new property is added to libvirt XML, which is also
> incompatible with firmware=efi, and we forget to update the list in our
> validation check, so the error message is inaccurate.

Fair enough.

> The libvirt error message is descriptive enough IMO, so I don't think we
> need to change anything in virt-install. Unless we get a lot of reports
> about it, like if that `--boot` line is copied from man page or some common
> blog post etc.

This is what the relevant section of the manual page for virt-install
currently looks like:

  ``--boot uefi``
      Configure the VM to boot from UEFI. In order for virt-install to know the
      correct UEFI parameters, libvirt needs to be advertising known UEFI binaries
      via domcapabilities XML, so this will likely only work if using properly
      configured distro packages.

  ``--boot loader=/.../OVMF_CODE.fd,loader.readonly=yes,loader.type=pflash,nvram.template=/.../OVMF_VARS.fd,loader_secure=no``
      Specify that the virtual machine use the custom OVMF binary as boot firmware,
      mapped as a virtual flash chip. In addition, request that libvirt instantiate
      the VM-specific UEFI varstore from the custom "/.../OVMF_VARS.fd" varstore
      template. This is the recommended UEFI setup, and should be used if
      --boot uefi doesn't know about your UEFI binaries. If your UEFI firmware
      supports Secure boot feature you can enable it via loader_secure.

I think it could be reworked to be more in line with current best
practices. The fact that it claims that specifying the various bits
manually is the recommended configuration is especially inaccurate
these days: firmware autoselection is definitely the way to go for
the vast majority of users at this point.

Interestingly, I had suggested making changes to this documentation a
few months back[1] but never followed through O:-) Let me prepare a
patch real quick.


[1] https://bugzilla.redhat.com/show_bug.cgi?id=2112154#c11

(In reply to Andrea Bolognani from comment #5)
> Interestingly, I had suggested making changes to this documentation a
> few months back[1] but never followed through O:-) Let me prepare a
> patch real quick.

https://github.com/virt-manager/virt-manager/pull/467

Since the root issue is the same, closing this as a duplicate

*** This bug has been marked as a duplicate of bug 2112154 ***