On 4/9/25 10:43, 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_VOL_XML_PARSE_VALIDATE flag before > virStorageVolDefParseXML() and the VIR_STORAGE_VOL_CREATE_VALIDATE before > 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> > --- > src/conf/storage_conf.c | 2 ++ > src/storage/storage_driver.c | 4 +++- > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c > index 68842004b7..3bbd727eb7 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_VOL_XML_PARSE_VALIDATE);
Nit pick, no need to wrap the flag in braces. > + > return virStorageVolDefParseXML(pool, ctxt, flags); > } > > diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c > index 86c03762d2..2f5a26bbef 100644 > --- a/src/storage/storage_driver.c > +++ b/src/storage/storage_driver.c > @@ -1877,6 +1877,7 @@ storageVolCreateXML(virStoragePoolPtr pool, > virStorageVolPtr vol = NULL, newvol = NULL; > g_autoptr(virStorageVolDef) voldef = NULL; > unsigned int parseFlags = VIR_VOL_XML_PARSE_OPT_CAPACITY; > + unsigned int buildFlags = flags; Here, I'd declare this variable ... > > virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA | > VIR_STORAGE_VOL_CREATE_VALIDATE, NULL); > @@ -1953,7 +1954,8 @@ storageVolCreateXML(virStoragePoolPtr pool, > voldef->building = true; > virObjectUnlock(obj); > ... at the beginning of this block (not visible in this limited context). The reason is - we try to keep the scope of variables as small as possible so that the overall logic of the code is easier to follow. For instance, in this specific case, declaring the variable at the beginning of the function makes me thing it's used throughout the whole function while in fact its use is limited to this small block. Another reason is - some clever compilers will reuse stack for these blocks. For instance, if I had two blocks, each with 8 bytes worth of variables, then the compiler might allocate 8 bytes for the first block and then just reuse it for the next block. Then there is clang which would allocate 8+8 bytes. https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/XT2HHWXNM64FLLVOHECB3DE72JMYQXKQ/ > - buildret = backend->buildVol(obj, buildvoldef, flags); > + buildFlags &= ~(VIR_STORAGE_VOL_CREATE_VALIDATE); > + buildret = backend->buildVol(obj, buildvoldef, buildFlags); > > VIR_FREE(buildvoldef); > Michal
