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 I
suggest you rather accept it in the virCheckFlags inside the parser code
rather than masking it out.

Reply via email to