Re: [libvirt] [PATCH 2/3] conf: add support for choosing firmware type

Currently if you want to enable UEFI firmware for a guest you need to know about the hypervisor platform specific firmware path. This does not even work for all platforms, as hypervisors like VMWare don't expose UEFI as a path, they just have a direct config option for turning it on or off. This adds ability to use the much simpler: <loader firmware="uefi|bios"/> to choose the different firmware types. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- docs/formatdomain.html.in | 9 +++++- docs/schemas/domaincommon.rng | 12 +++++++- src/conf/domain_conf.c | 70 ++++++++++++++++++++++++++++++++++++++----- src/conf/domain_conf.h | 11 +++++++ 4 files changed, 93 insertions(+), 9 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 7008005..b8e9315 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -143,7 +143,14 @@ <code>pflash</code>. Moreover, some firmwares may implement the Secure boot feature. Attribute <code>secure</code> can be used then to control it. - <span class="since">Since 2.1.0</span></dd> + <span class="since">Since 2.1.0</span>. The <code>firmware</code> + attribute can be used to request a specific type of firmware image + based on its common name, accepting the values <code>uefi</code> + and <code>bios</code>. <span class="since">Since 2.4.0</span>. When + <code>firmware</code> is set, it is not neccessary to provide any

+ path for the loader, nor set the <code>type</code> attribute or + <code>nvram</code> elements, as they will be automatically set + to the hypervisor specific default values.</dd>

<dt><code>nvram</code></dt> <dd>Some UEFI firmwares may want to use a non-volatile memory to store some variables. In the host, this is represented as a file and the diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 6eeb4e9..197b542 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -276,7 +276,17 @@ </choice> </attribute> </optional> - <ref name="absFilePath"/> + <optional> + <attribute name="firmware"> + <choice> + <value>bios</value> + <value>uefi</value> + </choice> + </attribute> + </optional> + <optional> + <ref name="absFilePath"/> + </optional> </element> </optional> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7972a4e..054be94 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -835,6 +835,12 @@ VIR_ENUM_IMPL(virDomainLoader, "rom", "pflash") +VIR_ENUM_IMPL(virDomainLoaderFirmware, + VIR_DOMAIN_LOADER_FIRMWARE_LAST, + "default", + "bios", + "uefi") + /* Internal mapping: subset of block job types that can be present in * <mirror> XML (remaining types are not two-phase). */ VIR_ENUM_DECL(virDomainBlockJob) @@ -15539,17 +15545,36 @@ virDomainLoaderDefParseXML(xmlNodePtr node, char *readonly_str = NULL; char *secure_str = NULL; char *type_str = NULL; + char *firmware_str = NULL; readonly_str = virXMLPropString(node, "readonly"); secure_str = virXMLPropString(node, "secure"); type_str = virXMLPropString(node, "type"); + firmware_str = virXMLPropString(node, "firmware"); loader->path = (char *) xmlNodeGetContent(node); - if (readonly_str && - (loader->readonly = virTristateBoolTypeFromString(readonly_str)) <= 0) { - virReportError(VIR_ERR_XML_DETAIL, - _("unknown readonly value: %s"), readonly_str); - goto cleanup; + if (loader->path && STREQ(loader->path, "")) + VIR_FREE(loader->path);

+ + if (firmware_str) { + int firmware; + if ((firmware = virDomainLoaderFirmwareTypeFromString(firmware_str)) < 0) { + virReportError(VIR_ERR_XML_DETAIL, + _("unknown firmware value: %s"), firmware_str); + goto cleanup; + } + loader->firmware = firmware; + } + + if (readonly_str) { + if ((loader->readonly = virTristateBoolTypeFromString(readonly_str)) <= 0) { + virReportError(VIR_ERR_XML_DETAIL, + _("unknown readonly value: %s"), readonly_str); + goto cleanup; + } + } else { + if (loader->firmware == VIR_DOMAIN_LOADER_FIRMWARE_UEFI) + loader->readonly = VIR_TRISTATE_SWITCH_ON; } if (secure_str && @@ -15567,6 +15592,29 @@ virDomainLoaderDefParseXML(xmlNodePtr node, goto cleanup; } loader->type = type; + + switch (loader->firmware) { + case VIR_DOMAIN_LOADER_FIRMWARE_UEFI: + if (loader->type != VIR_DOMAIN_LOADER_TYPE_PFLASH) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("UEFI firmware must use pflash type")); + goto cleanup; + } + break; + case VIR_DOMAIN_LOADER_FIRMWARE_BIOS: + if (loader->type != VIR_DOMAIN_LOADER_TYPE_ROM) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("BIOS firmware must use ROM type")); + goto cleanup; + } + break; + case VIR_DOMAIN_LOADER_FIRMWARE_DEFAULT: + case VIR_DOMAIN_LOADER_FIRMWARE_LAST: + break; + } + } else { + if (loader->firmware == VIR_DOMAIN_LOADER_FIRMWARE_UEFI) + loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH; } ret = 0; @@ -15574,6 +15622,7 @@ virDomainLoaderDefParseXML(xmlNodePtr node, VIR_FREE(readonly_str); VIR_FREE(secure_str); VIR_FREE(type_str); + VIR_FREE(firmware_str); return ret; } @@ -22872,18 +22921,25 @@ virDomainLoaderDefFormat(virBufferPtr buf, const char *readonly = virTristateBoolTypeToString(loader->readonly); const char *secure = virTristateBoolTypeToString(loader->secure); const char *type = virDomainLoaderTypeToString(loader->type); + const char *firmware = virDomainLoaderFirmwareTypeToString(loader->firmware); virBufferAddLit(buf, "<loader"); + if (loader->firmware) + virBufferAsprintf(buf, " firmware='%s'", firmware); + if (loader->readonly) virBufferAsprintf(buf, " readonly='%s'", readonly); if (loader->secure) virBufferAsprintf(buf, " secure='%s'", secure); - virBufferAsprintf(buf, " type='%s'>", type); + virBufferAsprintf(buf, " type='%s'", type); - virBufferEscapeString(buf, "%s</loader>\n", loader->path); + if (loader->path) + virBufferEscapeString(buf, ">%s</loader>\n", loader->path); + else + virBufferAddLit(buf, "/>\n"); if (loader->nvram || loader->templt) { virBufferAddLit(buf, "<nvram"); virBufferEscapeString(buf, " template='%s'", loader->templt); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 561a179..204c330 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1724,6 +1724,16 @@ struct _virDomainBIOSDef { }; typedef enum { + VIR_DOMAIN_LOADER_FIRMWARE_DEFAULT = 0, + VIR_DOMAIN_LOADER_FIRMWARE_BIOS, + VIR_DOMAIN_LOADER_FIRMWARE_UEFI, + + VIR_DOMAIN_LOADER_FIRMWARE_LAST +} virDomainLoaderFirmware; + +VIR_ENUM_DECL(virDomainLoaderFirmware) + +typedef enum { VIR_DOMAIN_LOADER_TYPE_ROM = 0, VIR_DOMAIN_LOADER_TYPE_PFLASH, @@ -1735,6 +1745,7 @@ VIR_ENUM_DECL(virDomainLoader) typedef struct _virDomainLoaderDef virDomainLoaderDef; typedef virDomainLoaderDef *virDomainLoaderDefPtr; struct _virDomainLoaderDef { + virDomainLoaderFirmware firmware; char *path; int readonly; /* enum virTristateBool */ virDomainLoader type;