Re: [PATCH] storage: virStorageVolDefParse and storageVolCreateXML flags fix

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&gt; > > --- > > 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.