On Wed, Apr 01, 2026 at 15:17:29 +0100, Daniel P. Berrangé wrote:
> On Wed, Apr 01, 2026 at 03:27:15PM +0200, Peter Krempa via Devel wrote:
> > From: Peter Krempa <[email protected]>
> >
> > In certain cases it's useful for the caller to be able to determine if
> > given API supports some flags.
> >
> > To do this with 'virDomainBlockResize', if
> > 'VIR_DOMAIN_BLOCK_RESIZE_PROBE_FLAGS' is specified, the API will return
> > success right after flag verification, regardless of other input
> > arguments and without modifying anything.
> >
> > Signed-off-by: Peter Krempa <[email protected]>
> > ---
> > include/libvirt/libvirt-domain.h | 9 +++++++++
> > src/libvirt-domain.c | 12 ++++++++++++
> > src/qemu/qemu_driver.c | 7 ++++++-
> > src/vz/vz_driver.c | 7 ++++++-
> > 4 files changed, 33 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/libvirt/libvirt-domain.h
> > b/include/libvirt/libvirt-domain.h
> > index 4a8e3114b3..113c7eefe6 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -2354,6 +2354,15 @@ typedef enum {
> >
> > /* Disallow shrinking (Since: 12.3.0) */
> > VIR_DOMAIN_BLOCK_RESIZE_EXTEND = 1 << 2,
> > +
> > + /* Ensures that the 'virDomainBlockResize' API returns success after
> > + * checking support of 'flags' regardless of other arguments without
> > + * actually modifying any aspect of the domain.
> > + *
> > + * Use of the flag thus allows probing for support of other flags of
> > this
> > + * API. (Since: 12.3.0) */
> > + VIR_DOMAIN_BLOCK_RESIZE_PROBE_FLAGS = 1 << 3,
> > +
> > } virDomainBlockResizeFlags;
> >
> > int virDomainBlockResize (virDomainPtr dom,
> > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> > index db9eea5774..1b49d2f7e5 100644
> > --- a/src/libvirt-domain.c
> > +++ b/src/libvirt-domain.c
> > @@ -6469,6 +6469,18 @@ virDomainBlockPeek(virDomainPtr dom,
> > * can be found by calling virDomainGetXMLDesc() and inspecting
> > * elements within //domain/devices/disk.
> > *
> > + * If @flags contains VIR_DOMAIN_BLOCK_RESIZE_PROBE_FLAGS (since 12.3.0)
> > the
> > + * API will return success right after checking flags for support without
> > + * modifying the disk in any way, thus allowing probing of flags with the
> > + * following algorithm
> > + *
> > + * supp = virDomainBlockResize(dom, "", 0,
> > VIR_DOMAIN_BLOCK_RESIZE_PROBE_FLAGS | VIR_DOMAIN_BLOCK_RESIZE_EXTEND);
> > + *
> > + * if (supp == 0)
> > + * //flag supported
> > + * else
> > + * virResetLastError();
>
> This suggests "flag" (singular), but if you pass many flags at once,
> then the error only tells you that at least 1 flag is invalid, but
> not which flag is invalid. So we need O(n) calls with PROBE_FLAGS
> if we want to check n different flags as distinct things.
I thought about this but I don't expect that this would be too widely
used, so the inability to query flags in bulk shouldn't be a problem.
I don't think there's a reasonable possibility to do this within the API
itself, and would thus require a new API.
> Also the "else" branch can also still be an error for non-probe
> related issues. eg there are many errors that the remote driver
> can raise from a transport POV.
I didn't think about this but if used in close proximity with the real
call I don't think there are many options where the probing call would
error out but then the further real call would succeed.
> How much better is this than simply trying the RESIZE_EXTEND usage
> unconditionally, and then falling back to a different code path if
> seeing INVALID_ARG error ?
The unfortunate thing and the reason I've picked this approach is that
the code that follows here liberally uses VIR_ERR_INVALID_ARG. Even on
code paths where it makes sense to report error:
if (!(disk = virDomainDiskByName(vm->def, path, false))) {
virReportError(VIR_ERR_INVALID_ARG,
_("disk '%1$s' was not found in the domain config"),
path);
goto endjob;
}
The caller (visible later in series) wouldn't be able to determine if
this is a case where the user messed up the disk and needs to see error
or is a case where the failure is from the real think we want to prevent
(accidental shrinking of the disk).
It would be possible to add a new error code for the 'unallowed shrink'
though to sidestep that problem although it would really be specific to
this scenario.
