On Thu, Sep 11, 2025 at 18:27:42 +0300, Nikolai Barybin via Devel wrote:
> This new API is a combination of virDomainBlockPull and
> virDomainBlockRebase (without copy mode) which supports extendable
> list of virTypedParameterPtr params.
>
> It is designed to allow more configurable use of underlying
> 'block-stream' QMP command.
>
> If "base" == NULL and flags == 0 it is equivalent to virDomainBlockPull.
>
> Signed-off-by: Nikolai Barybin <nikolai.bary...@virtuozzo.com>
> ---
> include/libvirt/libvirt-domain.h | 43 +++++++++++++++++++++
> src/driver-hypervisor.h | 8 ++++
> src/libvirt-domain.c | 65 ++++++++++++++++++++++++++++++++
> src/libvirt_public.syms | 5 +++
> src/remote/remote_driver.c | 1 +
> src/remote/remote_protocol.x | 17 ++++++++-
> src/remote_protocol-structs | 10 +++++
> 7 files changed, 148 insertions(+), 1 deletion(-)
With series that add APIs we usually require the corresponding virsh
implementation as reference. It can be a separate patch.
> diff --git a/include/libvirt/libvirt-domain.h
> b/include/libvirt/libvirt-domain.h
> index 71bb49fe6c..f90f9b5c9b 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -4459,6 +4459,49 @@ int virDomainBlockRebase(virDomainPtr dom, const char
> *disk,
> const char *base, unsigned long bandwidth,
> unsigned int flags);
>
> +/**
> + * VIR_DOMAIN_BLOCK_REBASE_PARAM_BASE:
> + * Macro for the virDomainBlockRebaseParams parameter 'base'.
> + * Expects string specifying new base block device.
> + *
> + * Since: 11.8.0
> + */
> +#define VIR_DOMAIN_BLOCK_REBASE_PARAM_BASE "base"
> +
> +/**
> + * VIR_DOMAIN_BLOCK_REBASE_PARAM_BANDWIDTH_MIB:
> + * Macro for the virDomainBlockRebaseParams parameter 'bandwidth-mib'.
> + * Expects desired bandwidth in MiB/s.
> + *
> + * Since: 11.8.0
> + */
> +#define VIR_DOMAIN_BLOCK_REBASE_PARAM_BANDWIDTH_MIB "bandwidth-mib"
> +
> +/**
> + * VIR_DOMAIN_BLOCK_REBASE_PARAM_BANDWIDTH_BYTES:
> + * Macro for the virDomainBlockRebaseParams parameter 'bandwidth-bytes'.
> + * Expects desired bandwidth in Bytes/s.
> + *
> + * Since: 11.8.0
> + */
> +#define VIR_DOMAIN_BLOCK_REBASE_PARAM_BANDWIDTH_BYTES "bandwidth-bytes"
This API doesn't have to implement any legacy combinations of
parameters. Please stick with 'bandwidth' in bytes as unsigned long
long.
Also you ought to document the expected type of the typed paramter.
> +
> +/**
> + * virDomainBlockRebaseParamsFlags:
> + *
> + * Flags available for virDomainBlockRebaseParams().
> + *
> + * Since: 11.8.0
> + */
> +typedef enum {
> + /* Keep backing chain referenced using relative names (Since: 11.8.0) */
> + VIR_DOMAIN_BLOCK_REBASE_PARAMS_RELATIVE = 1 << 4,
> +} virDomainBlockRebaseParamsFlags;
> +
> +int virDomainBlockRebaseParams(virDomainPtr dom, const char *disk,
> + virTypedParameterPtr params, int nparams,
> + unsigned int flags);
> +
> /**
> * virDomainBlockCopyFlags:
> *
> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
> index 6a43688b0c..83cf5c6205 100644
> --- a/src/driver-hypervisor.h
> +++ b/src/driver-hypervisor.h
> @@ -1089,6 +1089,13 @@ typedef int
> unsigned long bandwidth,
> unsigned int flags);
>
> +typedef int
> +(*virDrvDomainBlockRebaseParams)(virDomainPtr dom,
> + const char *path,
> + virTypedParameterPtr params,
> + int nparams,
> + unsigned int flags);
> +
> typedef int
> (*virDrvDomainBlockCopy)(virDomainPtr dom,
> const char *path,
> @@ -1681,6 +1688,7 @@ struct _virHypervisorDriver {
> virDrvDomainBlockJobSetSpeed domainBlockJobSetSpeed;
> virDrvDomainBlockPull domainBlockPull;
> virDrvDomainBlockRebase domainBlockRebase;
> + virDrvDomainBlockRebaseParams domainBlockRebaseParams;
> virDrvDomainBlockCopy domainBlockCopy;
> virDrvDomainBlockCommit domainBlockCommit;
> virDrvConnectSetKeepAlive connectSetKeepAlive;
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index ca110bdf85..c06bd8b6ce 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -11197,6 +11197,71 @@ virDomainBlockRebase(virDomainPtr dom, const char
> *disk,
> }
>
>
> +/**
> + * virDomainBlockRebaseParams:
> + * @dom: pointer to domain object
> + * @disk: path to the block device, or device shorthand
> + * @params: pointer to block rebase parameter
> + * @nparams: number of block rebase parameters
> + * @flags: bitwise-OR of virDomainBlockRebaseParamsFlags
> + *
> + * Generelized version of virDomainBlockPull with more precise params setting
> + * for underlying QMP 'block-stream'.
Don't mention hypervisor specific implementation details in the API
docs. This is meant to be a generic interface for any hypervisor which
supports a similar operation.
> + *
> + * VIR_DOMAIN_BLOCK_REBASE_PARAM_BASE in @params specifies intermidiate
> block device
> + * in a disk backing chain which will result in a new base for a given disk.
> If ommitted
> + * or NULL it is equavivalent to normal block pull, when whole backing chain
> is pulled
> + * into top image.
> + *
> + * VIR_DOMAIN_BLOCK_REBASE_PARAM_BANDWIDTH_MIB and
> VIR_DOMAIN_BLOCK_REBASE_PARAM_BANDWIDTH_BYTES
> + * in @params specify bandwidth in MiB/s or Bytes/s respectively.
As noted before, please no adaptation of old insane parameters.
> + *
> + * If @flags contains VIR_DOMAIN_BLOCK_REBASE_PARAMS_RELATIVE, the name
> recorded
> + * into the active disk as the location for @base will be kept relative.
> + * The operation will fail if libvirt can't infer the name.
> + *
> + * Returns 0 if the operation has started, -1 on failure.
> + *
> + * Since: 11.8.0
> + */
> +int
> +virDomainBlockRebaseParams(virDomainPtr dom,
> + const char *disk,
> + virTypedParameterPtr params,
> + int nparams,
> + unsigned int flags)
> +{
> + virConnectPtr conn;
> +
> + VIR_DOMAIN_DEBUG(dom, "disk=%s, params=%p, nparams=%u, flags=0x%x",
> + disk, params, nparams, flags);
loging the 'params' pointer is pointless
> + VIR_TYPED_PARAMS_DEBUG(params, nparams);
... especially with this.
> +
> + virResetLastError();
> +
> + virCheckDomainReturn(dom, -1);
> + conn = dom->conn;
> +
> + virCheckReadOnlyGoto(conn->flags, error);
> + virCheckNonNullArgGoto(disk, error);
> +
> + if (conn->driver->domainBlockRebaseParams) {
> + int ret;
> + ret = conn->driver->domainBlockRebaseParams(dom, disk, params,
> nparams,
> + flags);
> + if (ret < 0)
> + goto error;
> + return ret;
> + }
> +
> + virReportUnsupportedError();
> +
> + error:
> + virDispatchError(dom->conn);
> + return -1;
> +}
> +
> +
> /**
> * virDomainBlockCopy:
> * @dom: pointer to domain object
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index c506acd2ed..0b8365cd2c 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -956,4 +956,9 @@ LIBVIRT_11.2.0 {
> virDomainDelThrottleGroup;
> } LIBVIRT_10.2.0;
>
> +LIBVIRT_11.8.0 {
> + global:
> + virDomainBlockRebaseParams;
> +} LIBVIRT_11.2.0;
> +
> # .... define new API here using predicted next version number ....
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index ec71eaed87..d9ca026aef 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -7865,6 +7865,7 @@ static virHypervisorDriver hypervisor_driver = {
> .domainBlockJobSetSpeed = remoteDomainBlockJobSetSpeed, /* 0.9.4 */
> .domainBlockPull = remoteDomainBlockPull, /* 0.9.4 */
> .domainBlockRebase = remoteDomainBlockRebase, /* 0.9.10 */
> + .domainBlockRebaseParams = remoteDomainBlockRebaseParams, /* 11.8.0 */
> .domainBlockCopy = remoteDomainBlockCopy, /* 1.2.9 */
> .domainBlockCommit = remoteDomainBlockCommit, /* 0.10.2 */
> .connectSetKeepAlive = remoteConnectSetKeepAlive, /* 0.9.8 */
> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
> index 3c93203210..762614b1fa 100644
> --- a/src/remote/remote_protocol.x
> +++ b/src/remote/remote_protocol.x
> @@ -119,6 +119,9 @@ const REMOTE_DOMAIN_NUMA_PARAMETERS_MAX = 16;
> /* Upper limit on list of perf events. */
> const REMOTE_DOMAIN_PERF_EVENTS_MAX = 64;
>
> +/* Upper limit on block_rebase_params tunable parameters. */
> +const REMOTE_DOMAIN_BLOCK_REBASE_PARAMS_PARAMETERS_MAX = 16;
> +
> /* Upper limit on block copy tunable parameters. */
> const REMOTE_DOMAIN_BLOCK_COPY_PARAMETERS_MAX = 16;
>
> @@ -1440,6 +1443,12 @@ struct remote_domain_block_rebase_args {
> unsigned hyper bandwidth;
> unsigned int flags;
> };
> +struct remote_domain_block_rebase_params_args {
> + remote_nonnull_domain dom;
> + remote_nonnull_string path;
> + remote_typed_param
> params<REMOTE_DOMAIN_BLOCK_REBASE_PARAMS_PARAMETERS_MAX>;
> + unsigned int flags;
> +};
> struct remote_domain_block_copy_args {
> remote_nonnull_domain dom;
> remote_nonnull_string path;
> @@ -7119,5 +7128,11 @@ enum remote_procedure {
> * @generate: both
> * @acl: none
> */
> - REMOTE_PROC_DOMAIN_EVENT_NIC_MAC_CHANGE = 453
> + REMOTE_PROC_DOMAIN_EVENT_NIC_MAC_CHANGE = 453,
> +
> + /**
> + * @generate: both
> + * @acl: domain:block_write
> + */
> + REMOTE_PROC_DOMAIN_BLOCK_REBASE_PARAMS = 454
> };
> diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
> index 0f87d13a5a..ec0963550a 100644
> --- a/src/remote_protocol-structs
> +++ b/src/remote_protocol-structs
> @@ -1010,6 +1010,15 @@ struct remote_domain_block_rebase_args {
> uint64_t bandwidth;
> u_int flags;
> };
> +struct remote_domain_block_rebase_params_args {
> + remote_nonnull_domain dom;
> + remote_nonnull_string path;
> + struct {
> + u_int params_len;
> + remote_typed_param * params_val;
> + } params;
> + u_int flags;
> +};
> struct remote_domain_block_copy_args {
> remote_nonnull_domain dom;
> remote_nonnull_string path;
> @@ -3791,4 +3800,5 @@ enum remote_procedure {
> REMOTE_PROC_DOMAIN_SET_THROTTLE_GROUP = 451,
> REMOTE_PROC_DOMAIN_DEL_THROTTLE_GROUP = 452,
> REMOTE_PROC_DOMAIN_EVENT_NIC_MAC_CHANGE = 453,
> + REMOTE_PROC_DOMAIN_BLOCK_REBASE_PARAMS = 454,
> };
> --
> 2.43.5
>
