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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 3/22/23 06:42, Daniel Henrique Barboza wrote:


On 3/22/23 06:25, Daniel P. Berrangé wrote:
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@xxxxxxxxxxxxxxxx>
---
  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.

Makes sense. Any name suggestions for this new constant?

VIR_DOMAIN_LOADER_TYPE_SKIP is the first thing that comes to mind. If we want to
keep it consistent we should also change this new type to <loader type='skip'/>
as well.

VIR_DOMAIN_LOADER_TYPE_OTHER seems more appropriate to indicate that the firmware
will be loaded not by QEMU, but other means (e.g. kernel).

I'll do that for v2.


Daniel



Thanks,

Daniel



With regards,
Daniel




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux