On Mon, Apr 07, 2025 at 14:49:14 +0200, Peter Krempa via Devel wrote:
On Mon, Apr 07, 2025 at 14:25:43 +0200, Kirill Shchetiniuk via Devel
wrote:
> When the new storage was created using virsh with --validate option
> following errors occurred:
>
> # virsh vol-create default --file vol-def.xml --validate
> error: Failed to create vol from vol-def.xml
> error: unsupported flags (0x4) in function virStorageVolDefParseXML
>
> and after virStorageVolDefParse fix:
>
> # virsh vol-create default --file vol-def.xml --validate
> error: Failed to create vol from vol-def.xml
> error: unsupported flags (0x4) in function storageBackendCreateQemuImg
>
> Clear the VIR_STORAGE_VOL_CREATE_VALIDATE flag before
> virStorageVolDefParseXML and backend->buildVol (traces down to
> storageBackendCreateQemuImg) calls, as the XML schema validation is
> already complete within previous steps and there is no validation later.
>
> Signed-off-by: Kirill Shchetiniuk <kshcheti(a)redhat.com>
> ---
> NEWS.rst | 5 +++++
> src/conf/storage_conf.c | 2 ++
> src/storage/storage_driver.c | 4 +++-
> 3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/NEWS.rst b/NEWS.rst
> index e2dc4e508b..dd345bad7b 100644
> --- a/NEWS.rst
> +++ b/NEWS.rst
> @@ -28,6 +28,11 @@ v11.3.0 (unreleased)
>
> * **Bug fixes**
>
> + * storage: Fix new volume creation
> +
> + No more errors occur when new storage volume is being created
> + using ``vol-create`` with ``--validate`` option.
Changes to NEWS must be always in a separate patch. Mainly you
definitely do not want to have conflicts when backporting patches, where
NEWS definitely do not match.
> v11.2.0 (2025-04-01)
> ====================
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index 68842004b7..f6d804bb39 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -1409,6 +1409,8 @@ virStorageVolDefParse(virStoragePoolDef *pool,
> "volume", &ctxt,
"storagevol.rng", validate)))
> return NULL;
>
> + flags &= ~(VIR_STORAGE_VOL_CREATE_VALIDATE);
This function gets VIR_VOL_XML_PARSE_VALIDATE. In storageVolCreateXML
VIR_STORAGE_VOL_CREATE_VALIDATE is converted to VIR_VOL_XML_PARSE_VALIDATE.
Where did VIR_STORAGE_VOL_CREATE_VALIDATE leak from?
Either way this hunk is incorrect as this flag should not get here and
the caller needs to be fixed.
If in fact you meant to mask out VIR_VOL_XML_PARSE_VALIDATE here I
suggest you rather accept it in the virCheckFlags inside the parser code
rather than masking it out.