In the domain capabilities XML there are FW image paths printed.
There are two sources for the image paths (in order of
preference):

  1) firmware descriptor files - as returned by
  qemuFirmwareGetSupported()

  2) a compile time list of FW:NRAM pairs which can be overridden
  in qemu.conf

If either of those contains a duplicate FW image path (which is
a valid use case) it is printed twice in the capabilities XML.
While it's technically not a bug, it doesn't look good.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_capabilities.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 9b19930964..489a6872c4 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -5136,12 +5136,22 @@ virQEMUCapsFillDomainLoaderCaps(virDomainCapsLoaderPtr capsLoader,
 
     for (i = 0; i < nfirmwares; i++) {
         const char *filename = firmwares[i]->name;
+        size_t j;
 
         if (!virFileExists(filename)) {
             VIR_DEBUG("loader filename=%s does not exist", filename);
             continue;
         }
 
+        /* Put only unique FW images onto the list */
+        for (j = 0; j < capsLoader->values.nvalues; j++) {
+            if (STREQ(filename, capsLoader->values.values[j]))
+                break;
+        }
+
+        if (j != capsLoader->values.nvalues)
+            continue;
+
         if (VIR_STRDUP(capsLoader->values.values[capsLoader->values.nvalues],
                        filename) < 0)
             return -1;
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, 2019-09-12 at 15:31 +0200, Michal Privoznik wrote:
> In the domain capabilities XML there are FW image paths printed.
> There are two sources for the image paths (in order of
> preference):
> 
>   1) firmware descriptor files - as returned by
>   qemuFirmwareGetSupported()
> 
>   2) a compile time list of FW:NRAM pairs which can be overridden
>   in qemu.conf
> 
> If either of those contains a duplicate FW image path (which is
> a valid use case) it is printed twice in the capabilities XML.
> While it's technically not a bug, it doesn't look good.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_capabilities.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, Sep 12, 2019 at 03:31:24PM +0200, Michal Privoznik wrote:
> In the domain capabilities XML there are FW image paths printed.
> There are two sources for the image paths (in order of
> preference):
> 
>   1) firmware descriptor files - as returned by
>   qemuFirmwareGetSupported()
> 
>   2) a compile time list of FW:NRAM pairs which can be overridden
>   in qemu.conf
> 
> If either of those contains a duplicate FW image path (which is
> a valid use case) it is printed twice in the capabilities XML.
> While it's technically not a bug, it doesn't look good.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Yes, this fixes what we discussed here (and thanks for the elaboration
there):

    https://www.redhat.com/archives/libvir-list/2019-September/msg00485.html

So: Tested-by: Kashyap Chamarthy <kchamart@redhat.com>

Tested with:

    $> git describe
    v5.7.0-108-g84442d0c27

(And same EDK2 RPM as I noted here:
https://www.redhat.com/archives/libvir-list/2019-September/msg00519.html)

Result (only unique binaries are listed):

    $> sudo ./tools/virsh domcapabilities --machine q35 --arch x86_64 
    [...]
    <os supported='yes'>
      <enum name='firmware'>
        <value>bios</value>
        <value>efi</value>
      </enum>
      <loader supported='yes'>
        <value>/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd</value>
        <value>/usr/share/edk2/ovmf/OVMF_CODE.fd</value>
        <value>/usr/share/edk2.git/ovmf-x64/OVMF_CODE-pure-efi.fd</value>
        <value>/usr/share/edk2.git/ovmf-x64/OVMF_CODE-with-csm.fd</value>
        <enum name='type'>
          <value>rom</value>
          <value>pflash</value>
        </enum>
        <enum name='readonly'>
          <value>yes</value>
          <value>no</value>
        </enum>
        <enum name='secure'>
          <value>yes</value>
          <value>no</value>
        </enum>
      </loader>
    </os>

  [...]

> ---
>  src/qemu/qemu_capabilities.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 9b19930964..489a6872c4 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -5136,12 +5136,22 @@ virQEMUCapsFillDomainLoaderCaps(virDomainCapsLoaderPtr capsLoader,
>  
>      for (i = 0; i < nfirmwares; i++) {
>          const char *filename = firmwares[i]->name;
> +        size_t j;
>  
>          if (!virFileExists(filename)) {
>              VIR_DEBUG("loader filename=%s does not exist", filename);
>              continue;
>          }
>  
> +        /* Put only unique FW images onto the list */
> +        for (j = 0; j < capsLoader->values.nvalues; j++) {
> +            if (STREQ(filename, capsLoader->values.values[j]))
> +                break;
> +        }
> +
> +        if (j != capsLoader->values.nvalues)
> +            continue;
> +
>          if (VIR_STRDUP(capsLoader->values.values[capsLoader->values.nvalues],
>                         filename) < 0)
>              return -1;
> -- 
> 2.21.0
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

-- 
/kashyap

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list