qemuDomainInitializePflashStorageSource() always attaches a non-NULL
src->backingStore used as an empty virStorageSource chain terminator
(type VIR_STORAGE_TYPE_NONE). qemuValidateDomainDefNvram() incorrectly
interpreted every non-NULL backingStore as a genuine backing overlay and
reported VIR_ERR_CONFIG_UNSUPPORTED, so legitimate UEFI/NVRAM setups were
rejected.
Check virStorageSourceIsBacking(src->backingStore) instead of a plain
pointer test so only a real backing node is rejected.

Regression introduced in commit bca731d0f562f0842f56ec2206fdbd721a468f5b.

Signed-off-by: Fima Shevrin <efim.shevrin@virtuozzo.com>
---
 src/qemu/qemu_validate.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 642244b62e..d8347d1964 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -740,7 +740,14 @@ qemuValidateDomainDefNvram(const virDomainDef *def,
         return -1;
     }

-    if (src->backingStore) {
+    /* qemuDomainInitializePflashStorageSource() always sets
+     * src->backingStore to a fresh empty virStorageSource as a
+     * chain terminator, so a plain `if (src->backingStore)` check
+     * is always true and rejects every UEFI/NVRAM domain after
+     * upstream commit that introduced the terminator. Use
+     * virStorageSourceIsBacking() (type != VIR_STORAGE_TYPE_NONE)
+     * so we only reject genuine backing chains. */
+    if (virStorageSourceIsBacking(src->backingStore)) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                         _("backingStore is not supported with NVRAM"));
         return -1;
--
2.47.1

On Fri, May 22, 2026 at 15:49:27 +0000, Efim Shevrin via Devel wrote:
> qemuDomainInitializePflashStorageSource() always attaches a non-NULL
> src->backingStore used as an empty virStorageSource chain terminator
> (type VIR_STORAGE_TYPE_NONE). qemuValidateDomainDefNvram() incorrectly
> interpreted every non-NULL backingStore as a genuine backing overlay and
> reported VIR_ERR_CONFIG_UNSUPPORTED, so legitimate UEFI/NVRAM setups were
> rejected.
> Check virStorageSourceIsBacking(src->backingStore) instead of a plain
> pointer test so only a real backing node is rejected.
> 
> Regression introduced in commit bca731d0f562f0842f56ec2206fdbd721a468f5b.

Huh, this is a 4 year old commit, are you sure it's a regression at that
point?

Looking a bit further I've found my more recent commit d57630c282a which
adds the terminator.


> Signed-off-by: Fima Shevrin <efim.shevrin@virtuozzo.com>
> ---
>  src/qemu/qemu_validate.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 642244b62e..d8347d1964 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -740,7 +740,14 @@ qemuValidateDomainDefNvram(const virDomainDef *def,
>          return -1;
>      }
> 
> -    if (src->backingStore) {
> +    /* qemuDomainInitializePflashStorageSource() always sets
> +     * src->backingStore to a fresh empty virStorageSource as a
> +     * chain terminator, so a plain `if (src->backingStore)` check
> +     * is always true and rejects every UEFI/NVRAM domain after
> +     * upstream commit that introduced the terminator. Use
> +     * virStorageSourceIsBacking() (type != VIR_STORAGE_TYPE_NONE)
> +     * so we only reject genuine backing chains. */

IMO such an extensive commit is not needed. All code handling
virStorageSource needs to do the same.

I'll likely drop it before pushing.


> +    if (virStorageSourceIsBacking(src->backingStore)) {
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                          _("backingStore is not supported with NVRAM"));
>          return -1;

Reviewed-by: Peter Krempa <pkrempa@redhat.com>

on the code. I'll hold off for a bit to discuss the commit message.