[libvirt] [PATCH 0/2] Document firmware autoselection and fix one trivial bug

*** BLURB HERE *** Michal Prívozník (2): qemuFirmwareFetchConfigs: Fix check for @privileged news: Document firmware autoselection for QEMU driver docs/news.xml | 16 ++++++++++++++++ src/qemu/qemu_firmware.c | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) -- 2.19.2

The qemuFirmwareFetchConfigs() function is supposed to fetch all firmware descriptions from paths defined by firmware.json specification. This includes user's $HOME directory. However, it was agreed that if libvirtd is running as privileged user then his $HOME is ignored (thus $HOME is included in the search only for regular users). Well, I got the condition wrong - it should have been reversed. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_firmware.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index b423ab10e4..4ce010caaa 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -974,7 +974,7 @@ qemuFirmwareFetchConfigs(char ***firmwares, *firmwares = NULL; - if (privileged) { + if (!privileged) { /* This is a slight divergence from the specification. * Since the system daemon runs as root, it doesn't make * much sense to parse files in root's home directory. It -- 2.19.2

On Wed, 2019-03-13 at 11:08 +0100, Michal Privoznik wrote:
The qemuFirmwareFetchConfigs() function is supposed to fetch all firmware descriptions from paths defined by firmware.json specification. This includes user's $HOME directory. However, it was agreed that if libvirtd is running as privileged user then his $HOME is ignored (thus $HOME is included in the search only for regular users). Well, I got the condition wrong - it should have been reversed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_firmware.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index b423ab10e4..4ce010caaa 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -974,7 +974,7 @@ qemuFirmwareFetchConfigs(char ***firmwares,
*firmwares = NULL;
- if (privileged) { + if (!privileged) { /* This is a slight divergence from the specification. * Since the system daemon runs as root, it doesn't make * much sense to parse files in root's home directory. It
How about moving the comment block right before the condition? That would make it IMHO more obvious what part of the code it refers to. Either way, Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization

Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/news.xml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 81466c3d55..b3bfdcbfa1 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -66,6 +66,22 @@ virtio device version is presented to the guest. </description> </change> + <change> + <summary> + qemu: Enable firmware autoselection + </summary> + <description> + Libvirt allows users to provide loader path for some time now. + However, this puts some burden on users because they need to + know what firmware meeets their requirements. Now that qemu + ships firmware description files this burden can be moved onto + libvirt. It is as easy as setting <code>firmware</code> + attribute to <code>os</code> element (accepted values are + <code>bios</code> and <code>efi</code>). Moreover, libvirt + automatically enables domain features needed for firmware it + chooses. + </description> + </change> </section> <section title="Improvements"> </section> -- 2.19.2

On Wed, Mar 13, 2019 at 11:08:23AM +0100, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/news.xml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index 81466c3d55..b3bfdcbfa1 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -66,6 +66,22 @@ virtio device version is presented to the guest. </description> </change> + <change> + <summary> + qemu: Enable firmware autoselection + </summary> + <description> + Libvirt allows users to provide loader path for some time now. + However, this puts some burden on users because they need to + know what firmware meeets their requirements. Now that qemu
s/meeets/meets
+ ships firmware description files this burden can be moved onto + libvirt. It is as easy as setting <code>firmware</code>
s/setting/setting the
+ attribute to <code>os</code> element (accepted values are
s/to/in the
+ <code>bios</code> and <code>efi</code>). Moreover, libvirt + automatically enables domain features needed for firmware it + chooses. + </description> + </change> </section> <section title="Improvements"> </section> --
Reviewed-by: Erik Skultety <eskultet@redhat.com>

On Wed, 2019-03-13 at 11:08 +0100, Michal Privoznik wrote: [...]
+ <change> + <summary> + qemu: Enable firmware autoselection + </summary> + <description> + Libvirt allows users to provide loader path for some time now. + However, this puts some burden on users because they need to + know what firmware meeets their requirements. Now that qemu
s/eee/ee/ s/qemu/QEMU/
+ ships firmware description files this burden can be moved onto + libvirt. It is as easy as setting <code>firmware</code> + attribute to <code>os</code> element (accepted values are
[...] onto libvirt: for users, it is as easy as setting the <code>firmware</code> attribute of the <code>os</code> element [...] With the above addressed, Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
participants (3)
-
Andrea Bolognani
-
Erik Skultety
-
Michal Privoznik