On 4/7/25 15:11, Peter Krempa via Devel wrote: > 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 <kshch...@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
Yeah, that was the initial plan. > I > suggest you rather accept it in the virCheckFlags inside the parser code > rather than masking it out. > Well, I was the one who suggested to Kirill to mask the flag out. Accepting the flag inside a function and then never using it just felt wrong. Michal
