Re: [PATCH 1/4] conf: add loader type 'none'

On Wed, Mar 22, 2023 at 06:10:18AM -0300, Daniel Henrique Barboza wrote: > Today, trying to boot a RISC-V Fedora Rawhide image in a RISC-V QEMU domain > results in the following error: > > ==== > error: Failed to start domain 'riscv-fedora' > error: internal error: process exited while connecting to monitor: > 2023-03-20T17:31:02.650862Z qemu-system-riscv64: Some ROM regions are overlapping > These ROM regions might have been loaded by direct user request or by default. > They could be BIOS/firmware images, a guest kernel, initrd or some other file loaded > into guest memory. > Check whether you intended to load all this guest code, and whether it has been built > to load to the correct addresses. > ==== > > This happens because the default RISC-V QEMU firmware, OpenSBI, is > always loaded unless "-bios none" is passed in the command line, and the > Fedora Rawhide guest kernel has its own ROM. Other machines such as > PPC64 'pseries' shows the same behavior: the default firmware is always > loaded unless specified otherwise with the '-bios' option. > > At this moment we don't have XML support for '-bios none'. Using > "<qemu:commandline>" works but it will leave the domain in a tainted > state. It'll also have unpredictable consequences with the autoselect > firmware feature libvirt has. > > Add a new loader type 'none' that, if no path is specified and we're not > use firmware autoselection, will tell QEMU that no default firmware > should be used: > > <os> > <loader type='none'/> > (...) > </os> > > Signed-off-by: Daniel Henrique Barboza <dbarboza(a)ventanamicro.com&gt; > --- > src/conf/domain_conf.c | 5 +++-- > src/conf/domain_validate.c | 2 +- > src/conf/schemas/domaincommon.rng | 1 + > 3 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 9ef50c818b..d7a5cd094b 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -16837,7 +16837,7 @@ virDomainLoaderDefParseXMLLoader(virDomainLoaderDef *loader, > return -1; > > if (virXMLPropEnum(loaderNode, "type", virDomainLoaderTypeFromString, > - VIR_XML_PROP_NONZERO, &loader->type) < 0) > + VIR_XML_PROP_NONE, &loader->type) < 0) > return -1; > > if (!(loader->path = virXMLNodeContentString(loaderNode))) > @@ -26259,7 +26259,8 @@ virDomainLoaderDefFormat(virBuffer *buf, > virBufferAsprintf(&loaderAttrBuf, " secure='%s'", > virTristateBoolTypeToString(loader->secure)); > > - if (loader->type != VIR_DOMAIN_LOADER_TYPE_NONE) > + if (loader->type != VIR_DOMAIN_LOADER_TYPE_NONE || > + (loader->type == VIR_DOMAIN_LOADER_TYPE_NONE && !loader->path)) > virBufferAsprintf(&loaderAttrBuf, " type='%s'", > virDomainLoaderTypeToString(loader->type)); > VIR_DOMAIN_LOADER_TYPE_NONE is a constant we're already using internally to track when the user has not given any <loader> element at all. It is not a good idea to overload for this for the user explicitly requesting a config.

With regards, Daniel