Re: [PATCH v2 1/2] storage: virStorageVolDefParse and storageVolCreateXML flags fix
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 <kshcheti(a)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/XT...
- buildret = backend->buildVol(obj, buildvoldef, flags);
+ buildFlags &= ~(VIR_STORAGE_VOL_CREATE_VALIDATE);
+ buildret = backend->buildVol(obj, buildvoldef, buildFlags);
VIR_FREE(buildvoldef);
Michal