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@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/XT2HH...

- buildret = backend->buildVol(obj, buildvoldef, flags); + buildFlags &= ~(VIR_STORAGE_VOL_CREATE_VALIDATE); + buildret = backend->buildVol(obj, buildvoldef, buildFlags);

VIR_FREE(buildvoldef);

Michal

On 4/9/25 10:43, Kirill Shchetiniuk via Devel wrote:

Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com> --- NEWS.rst | 5 +++++ 1 file changed, 5 insertions(+)

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.

Actually, it's the API that's being fixed. virsh is just a wrapper around APIs.

+

v11.2.0 (2025-04-01) ====================

Michal