6:27 a.m.
Starting with 5.2.0, libvirt has a way to select the firmware by
specifying its type, provided configuration files for the firmware are
shipped. Currently we start the appliance as UEFI if any of the firmware
are found, so instead we can try to just set the firmware type iff:
- the libvirt autoselection works
- the 'efi' firmware is available
The only behaviour change is that the default firmware may be a
non-debug version, so we get no debug messages even when running in
verbose mode. This most probably will need an addition in libvirt to
select this feature among the available firmwares.
Pino Toscano (4):
launch: libvirt: parse firmware autoselection
lib: uefi: reset out parameters earlier
lib: allow to use libvirt firmware autoselection
lib: uefi: use the efi libvirt firmware if available
lib/appliance-uefi.c | 32 ++++++++++++++++++++++++++------
lib/guestfs-internal.h | 2 +-
lib/launch-direct.c | 3 ++-
lib/launch-libvirt.c | 40 +++++++++++++++++++++++++++++++++++++++-
4 files changed, 68 insertions(+), 9 deletions(-)
--
2.24.1
6:27 a.m.
New subject: [PATCH 1/4] launch: libvirt: parse firmware autoselection
Parse from the domain capabilities whether libvirt supports the
autoselection of firmware, and which values are supported.
---
lib/launch-libvirt.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c
index 95aed8054..fa7b6987c 100644
--- a/lib/launch-libvirt.c
+++ b/lib/launch-libvirt.c
@@ -135,6 +135,11 @@ struct backend_libvirt_data {
char *uefi_code; /* UEFI (firmware) code and variables. */
char *uefi_vars;
char *default_qemu; /* default qemu (from domcapabilities) */
+ char **firmware_autoselect; /* supported firmwares (from domcapabilities);
+ * NULL means "not supported", otherwise it
+ * contains a list with supported values for
+ * <os firmware='...'>
+ */
char guestfsd_path[UNIX_PATH_MAX]; /* paths to sockets */
char console_path[UNIX_PATH_MAX];
};
@@ -843,6 +848,8 @@ parse_domcapabilities (guestfs_h *g, const char *domcapabilities_xml,
CLEANUP_XMLFREEDOC xmlDocPtr doc = NULL;
CLEANUP_XMLXPATHFREECONTEXT xmlXPathContextPtr xpathCtx = NULL;
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpathObj = NULL;
+ xmlNodeSetPtr nodes;
+ size_t i;
doc = xmlReadMemory (domcapabilities_xml, strlen (domcapabilities_xml),
NULL, NULL, XML_PARSE_NONET);
@@ -869,6 +876,26 @@ parse_domcapabilities (guestfs_h *g, const char
*domcapabilities_xml,
assert (xpathObj->type == XPATH_STRING);
data->default_qemu = safe_strdup (g, (char *) xpathObj->stringval);
+ /*
+ * This gives us whether the firmware autoselection is supported,
+ * and which values are allowed.
+ */
+#define XPATH_EXPR
"/domainCapabilities/os/enum[@name='firmware']/value"
+ xpathObj = xmlXPathEvalExpression (BAD_CAST XPATH_EXPR, xpathCtx);
+ if (xpathObj == NULL) {
+ error (g, _("unable to evaluate XPath expression: %s"), XPATH_EXPR);
+ return -1;
+ }
+#undef XPATH_EXPR
+
+ assert (xpathObj->type == XPATH_NODESET);
+ nodes = xpathObj->nodesetval;
+ if (nodes != NULL) {
+ data->firmware_autoselect = safe_calloc (g, nodes->nodeNr + 1, sizeof (char
*));
+ for (i = 0; i < (size_t) nodes->nodeNr; ++i)
+ data->firmware_autoselect[i] = (char *)
xmlNodeGetContent(nodes->nodeTab[i]);
+ }
+
return 0;
}
@@ -2072,6 +2099,9 @@ shutdown_libvirt (guestfs_h *g, void *datav, int check_for_errors)
free (data->default_qemu);
data->default_qemu = NULL;
+ guestfs_int_free_string_list (data->firmware_autoselect);
+ data->firmware_autoselect = NULL;
+
return ret;
}
--
2.24.1
6:27 a.m.
New subject: [PATCH 2/4] lib: uefi: reset out parameters earlier
Make sure they are always reset, no matter the code branches later on.
This is mostly code motion.
---
lib/appliance-uefi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/appliance-uefi.c b/lib/appliance-uefi.c
index 5fca8cd37..138a8d2f9 100644
--- a/lib/appliance-uefi.c
+++ b/lib/appliance-uefi.c
@@ -56,6 +56,9 @@
int
guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars, int *flags)
{
+ *code = *vars = NULL;
+ *flags = 0;
+
#ifdef __aarch64__
size_t i;
@@ -102,7 +105,5 @@ guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars, int
*flags)
#endif
/* Not found. */
- *code = *vars = NULL;
- *flags = 0;
return 0;
}
--
2.24.1
6:27 a.m.
New subject: [PATCH 3/4] lib: allow to use libvirt firmware autoselection
Enhance the UEFi firmware lookup function with the information on the
libvirt firmware autoselection, allowing it to return a value to use for
the appliance.
At the moment no firmware is selected this way, so there is no behaviour
change.
---
lib/appliance-uefi.c | 18 ++++++++++++++----
lib/guestfs-internal.h | 2 +-
lib/launch-direct.c | 3 ++-
lib/launch-libvirt.c | 10 +++++++++-
4 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/lib/appliance-uefi.c b/lib/appliance-uefi.c
index 138a8d2f9..d2908867c 100644
--- a/lib/appliance-uefi.c
+++ b/lib/appliance-uefi.c
@@ -38,14 +38,20 @@
* is aarch64 only currently, since that's the only architecture where
* UEFI is mandatory (and that only for RHEL).
*
+ * C<firmwares> is an optional list of allowed values for the firmware
+ * autoselection of libvirt. It is C<NULL> to indicate it is not
+ * supported. C<*firmware> is set to one of the strings in
+ * C<firmwares> in case one can be used.
+ *
* C<*code> is initialized with the path to the read-only UEFI code
* file. C<*vars> is initialized with the path to a copy of the UEFI
* vars file (which is cleaned up automatically on exit).
*
- * If C<*code> == C<*vars> == C<NULL> then no UEFI firmware is
- * available.
+ * In case a UEFI firmare is available, either C<*firmware> is set to
+ * a non-C<NULL> value, or C<*code> and C<*vars> are.
*
- * C<*code> and C<*vars> should be freed by the caller.
+ * C<*code> and C<*vars> should be freed by the caller, and
+ * C<*firmware> B<must> not.
*
* If the function returns C<-1> then there was a real error which
* should cause appliance building to fail (no UEFI firmware is not an
@@ -54,10 +60,14 @@
* See also F<virt-v2v.git/v2v/utils.ml>:find_uefi_firmware
*/
int
-guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars, int *flags)
+guestfs_int_get_uefi (guestfs_h *g, char *const *firmwares,
+ const char **firmware, char **code, char **vars,
+ int *flags)
{
*code = *vars = NULL;
*flags = 0;
+ if (firmware)
+ *firmware = NULL;
#ifdef __aarch64__
size_t i;
diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h
index 75b8a5c8e..6799c265d 100644
--- a/lib/guestfs-internal.h
+++ b/lib/guestfs-internal.h
@@ -691,7 +691,7 @@ extern char *guestfs_int_appliance_command_line (guestfs_h *g, const
char *appli
#define APPLIANCE_COMMAND_LINE_IS_TCG 1
/* appliance-uefi.c */
-extern int guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars, int *flags);
+extern int guestfs_int_get_uefi (guestfs_h *g, char *const *firmwares, const char
**firmware, char **code, char **vars, int *flags);
/* launch.c */
extern int64_t guestfs_int_timeval_diff (const struct timeval *x, const struct timeval
*y);
diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index ee2dcb884..ae6ca093b 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -533,7 +533,8 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
#endif
/* UEFI (firmware) if required. */
- if (guestfs_int_get_uefi (g, &uefi_code, &uefi_vars, &uefi_flags) == -1)
+ if (guestfs_int_get_uefi (g, NULL, NULL, &uefi_code, &uefi_vars,
+ &uefi_flags) == -1)
goto cleanup0;
if (uefi_flags & UEFI_FLAG_SECURE_BOOT_REQUIRED) {
/* Implementing this requires changes to the qemu command line.
diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c
index fa7b6987c..864eae314 100644
--- a/lib/launch-libvirt.c
+++ b/lib/launch-libvirt.c
@@ -140,6 +140,10 @@ struct backend_libvirt_data {
* contains a list with supported values for
* <os firmware='...'>
*/
+ const char *firmware; /* firmware to set in autoselection mode;
+ * points to one of the elements in
+ * "firmware_autoselect"
+ */
char guestfsd_path[UNIX_PATH_MAX]; /* paths to sockets */
char console_path[UNIX_PATH_MAX];
};
@@ -442,7 +446,8 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri)
goto cleanup;
/* UEFI code and variables, on architectures where that is required. */
- if (guestfs_int_get_uefi (g, &data->uefi_code, &data->uefi_vars,
+ if (guestfs_int_get_uefi (g, data->firmware_autoselect, &data->firmware,
+ &data->uefi_code, &data->uefi_vars,
&uefi_flags) == -1)
goto cleanup;
if (uefi_flags & UEFI_FLAG_SECURE_BOOT_REQUIRED) {
@@ -1207,6 +1212,9 @@ construct_libvirt_xml_boot (guestfs_h *g,
cmdline = guestfs_int_appliance_command_line (g, params->appliance_dev, flags);
start_element ("os") {
+ if (params->data->firmware)
+ attribute ("firmware", params->data->firmware);
+
start_element ("type") {
#ifdef MACHINE_TYPE
attribute ("machine", MACHINE_TYPE);
--
2.24.1
6:27 a.m.
New subject: [PATCH 4/4] lib: uefi: use the efi libvirt firmware if available
In case libvirt supports the firmware autoselection and there is an EFI
firmware available, use it directly instead of handling the firmware
manually.
---
lib/appliance-uefi.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/lib/appliance-uefi.c b/lib/appliance-uefi.c
index d2908867c..0f07ed5a9 100644
--- a/lib/appliance-uefi.c
+++ b/lib/appliance-uefi.c
@@ -72,6 +72,15 @@ guestfs_int_get_uefi (guestfs_h *g, char *const *firmwares,
#ifdef __aarch64__
size_t i;
+ if (firmwares && firmware) {
+ for (i = 0; firmwares[i] != NULL; ++i) {
+ if (STREQ(firmwares[i], "efi")) {
+ *firmware = firmwares[i];
+ return 0;
+ }
+ }
+ }
+
for (i = 0; guestfs_int_uefi_aarch64_firmware[i].code != NULL; ++i) {
const char *codefile = guestfs_int_uefi_aarch64_firmware[i].code;
const char *code_debug_file =
--
2.24.1
6:49 a.m.
On 1/16/20 1:27 PM, Pino Toscano wrote:
Starting with 5.2.0, libvirt has a way to select the firmware by
specifying its type, provided configuration files for the firmware are
shipped. Currently we start the appliance as UEFI if any of the firmware
are found, so instead we can try to just set the firmware type iff:
- the libvirt autoselection works
- the 'efi' firmware is available
The only behaviour change is that the default firmware may be a
non-debug version, so we get no debug messages even when running in
verbose mode. This most probably will need an addition in libvirt to
select this feature among the available firmwares.
Firmware autoselection parses JSON files at well defined locations
giving each file precedence over others based on the path it's found
under and it's name.
The file name precedence is based on alphabetical ordering. That is why
file names usually consists of two leading numbers, e.g. 40-bios.json,
50-ovmf-sb.json, 60-ovmf.json and so on. Now, the default, distro wide
path where these files live is /usr/share/qemu/firmware/ and has the
least precedence. If a sysadmin wants to alter config of some FW blob,
instead of fighting with package manager and overwriting files under
/usr/.. they can put changed versions under /etc/qemu/firmware which
takes precedence over the former path. For instance, if 40-bios.json is
found under both paths, only the one from /etc is considered and the
other is not even parsed. If you would apply this logic one more time,
you will get $HOME/.config/qemu/firmware which allows non-root users to
override their sysadmin's config and have their private FW blobs with
the highest priority.
So if libguestfs wants libvirt to select certain FW image it can install
its own JSON descriptor somewhere into a suitable path and if the file
has the correct content it will direct libvirt into using FW blob with
debug messages enabled.
Pino Toscano (4):
launch: libvirt: parse firmware autoselection
lib: uefi: reset out parameters earlier
lib: allow to use libvirt firmware autoselection
lib: uefi: use the efi libvirt firmware if available
lib/appliance-uefi.c | 32 ++++++++++++++++++++++++++------
lib/guestfs-internal.h | 2 +-
lib/launch-direct.c | 3 ++-
lib/launch-libvirt.c | 40 +++++++++++++++++++++++++++++++++++++++-
4 files changed, 68 insertions(+), 9 deletions(-)
I'm no libguestfs devel, but this looks sane to me.
Michal
6:53 a.m.
On Thu, Jan 16, 2020 at 01:49:09PM +0100, Michal Privoznik wrote:
On 1/16/20 1:27 PM, Pino Toscano wrote:
> Starting with 5.2.0, libvirt has a way to select the firmware by
> specifying its type, provided configuration files for the firmware are
> shipped. Currently we start the appliance as UEFI if any of the firmware
> are found, so instead we can try to just set the firmware type iff:
> - the libvirt autoselection works
> - the 'efi' firmware is available
>
> The only behaviour change is that the default firmware may be a
> non-debug version, so we get no debug messages even when running in
> verbose mode. This most probably will need an addition in libvirt to
> select this feature among the available firmwares.
Firmware autoselection parses JSON files at well defined locations giving
each file precedence over others based on the path it's found under and it's
name.
The file name precedence is based on alphabetical ordering. That is why file
names usually consists of two leading numbers, e.g. 40-bios.json,
50-ovmf-sb.json, 60-ovmf.json and so on. Now, the default, distro wide path
where these files live is /usr/share/qemu/firmware/ and has the least
precedence. If a sysadmin wants to alter config of some FW blob, instead of
fighting with package manager and overwriting files under /usr/.. they can
put changed versions under /etc/qemu/firmware which takes precedence over
the former path. For instance, if 40-bios.json is found under both paths,
only the one from /etc is considered and the other is not even parsed. If
you would apply this logic one more time, you will get
$HOME/.config/qemu/firmware which allows non-root users to override their
sysadmin's config and have their private FW blobs with the highest priority.
So if libguestfs wants libvirt to select certain FW image it can install its
own JSON descriptor somewhere into a suitable path and if the file has the
correct content it will direct libvirt into using FW blob with debug
messages enabled.
I not sure we should encourage applications / libraries to do this.
Installing a JSON descriptor into any of the directories mentioned above
will affect *all* applications on the host using libvirt, which is not
appropriate IMHO.
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 :|
9:12 a.m.
The patch series looks fine to me.
TBH I'm not really bothered about verbose using a non-debug firmware.
It's rare that we would need to actually debug a UEFI issue, plus the
debug output of UEFI is kind of crazy.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
1771
days inactive
1771
days old
7 comments
4 participants
participants (4)
-
Daniel P. Berrangé -
Michal Privoznik -
Pino Toscano -
Richard W.M. Jones