On Fri, Jun 05, 2026 at 10:14:49 +0000, Efim Shevrin wrote:
>
> > 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.
>
> You're right, sorry, bca731d0f5 only added the validation. The actual change
> that
> makes src->backingStore non-NULL for pflash/nvram is your later commit
> d57630c282a ("qemu: Install backing store terminators for 'pflash' blockdevs",
> 2024-11-04). I'll fix the attribution in v2.
>
> > IMO such an extensive commit is not needed. All code handling
> > virStorageSource
> > needs to do the same. I'll likely drop it before pushing.
>
> Agreed, the comment is more general guidance than something specific to this
> hunk.
> Dropped the inline comment in v2.
>
> From 1327b8ce4844fe91a6a62cc6702551c52e6813e7 Mon Sep 17 00:00:00 2001
> From: Fima Shevrin <[email protected]>
> Date: Sun, 3 May 2026 22:22:44 +0300
> Subject: [PATCH v2] qemu: validate: treat only real backing chains as NVRAM
> backingStore
>
> 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.
>
> Fixes: d57630c282ae7220d8998f74b7e4fec21841797f
>
> Signed-off-by: Fima Shevrin <[email protected]>
> ---
> v2:
> - dropped the inline comment per review
> - fixed attribution: Fixes: d57630c282a instead of bca731d0f5
>
> src/qemu/qemu_validate.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 642244b62e..2b5a2535c5 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -740,7 +740,7 @@ qemuValidateDomainDefNvram(const virDomainDef *def,
> return -1;
> }
>
> - if (src->backingStore) {
> + if (virStorageSourceIsBacking(src->backingStore)) {
I have 'if (virStorageSourceHasBacking(src)) {' in my private branch
where I've fixed v1 of this patch ...
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> _("backingStore is not supported with NVRAM"));
> return -1;
> --
> 2.47.1
Looks good, but the patch is un-applicable because you've pasted it into
your mail client. Please use 'git publish' or 'git send-email' per our
docs to send patches so that they are not chewed up the next time.
I'll apply it manually this time, based on the v1 I already have and I
already fixed so that I don't have to do it again.
Reviewed-by: Peter Krempa <[email protected]>
