This new API is a superset of virDomainBlockPull and
virDomainBlockRebase which supports extendable list of
virTypedParameterPtr params.
If "base" == NULL and flags == 0 it is equivalent to virDomainBlockPull.
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com>
---
include/libvirt/libvirt-domain.h | 20 +++++++++
src/driver-hypervisor.h | 8 ++++
src/libvirt-domain.c | 69 ++++++++++++++++++++++++++++++++
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, 129 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 71bb49fe6c..cd33001fb4 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -4459,6 +4459,26 @@ 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 virDomainBlockRebase2 parameter 'base'
+ *
+ * Since: 11.6.0
+ */
+#define VIR_DOMAIN_BLOCK_REBASE_PARAM_BASE "base"
+
+/**
+ * VIR_DOMAIN_BLOCK_REBASE_PARAM_BANDWIDTH:
+ * Macro for the virDomainBlockRebase2 parameter 'bandwidth'
+ *
+ * Since: 11.6.0
+ */
+#define VIR_DOMAIN_BLOCK_REBASE_PARAM_BANDWIDTH "bandwidth"
+
+int virDomainBlockRebase2(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..e64e00674c 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
+(*virDrvDomainBlockRebase2)(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;
+ virDrvDomainBlockRebase2 domainBlockRebase2;
virDrvDomainBlockCopy domainBlockCopy;
virDrvDomainBlockCommit domainBlockCommit;
virDrvConnectSetKeepAlive connectSetKeepAlive;
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index ca110bdf85..7924a0d6cc 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11197,6 +11197,75 @@ virDomainBlockRebase(virDomainPtr dom, const char *disk,
}
+/**
+ * virDomainBlockRebase2:
+ * @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 virDomainBlockRebaseFlags
+ *
+ * Generalized version of virDomainBlockRebase() which can act either as
+ * virDomainBlockRebase() or virDomainBlockPull() depending on @params and @flags.
+ *
+ * Returns 0 if the operation has started, -1 on failure.
+ *
+ * Since: 11.6.0
+ */
+int
+virDomainBlockRebase2(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);
+ VIR_TYPED_PARAMS_DEBUG(params, nparams);
+
+ virResetLastError();
+
+ virCheckDomainReturn(dom, -1);
+ conn = dom->conn;
+
+ virCheckReadOnlyGoto(conn->flags, error);
+ virCheckNonNullArgGoto(disk, error);
+
+ if (flags & VIR_DOMAIN_BLOCK_REBASE_COPY) {
+ const char *base = NULL;
+ if (virTypedParamsGetString(params, nparams,
+ VIR_DOMAIN_BLOCK_REBASE_PARAM_BASE,
+ &base) <= 0 ||
+ base == NULL)
+ goto error;
+ } else if (flags & (VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
+ VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
+ VIR_DOMAIN_BLOCK_REBASE_COPY_RAW |
+ VIR_DOMAIN_BLOCK_REBASE_COPY_DEV)) {
+ virReportInvalidArg(flags, "%s",
+ _("use of flags requires a copy job"));
+ goto error;
+ }
+
+ if (conn->driver->domainBlockRebase2) {
+ int ret;
+ ret = conn->driver->domainBlockRebase2(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..a1cb290c22 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.6.0 {
+ global:
+ virDomainBlockRebase2;
+} 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..5902b1f04b 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 */
+ .domainBlockRebase2 = remoteDomainBlockRebase2, /* 11.6.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..41a85a4f71 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 rebase2 tunable parameters. */
+const REMOTE_DOMAIN_BLOCK_REBASE2_PARAMETERS_MAX = 2;
+
/* 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_rebase2_args {
+ remote_nonnull_domain dom;
+ remote_nonnull_string path;
+ remote_typed_param params<REMOTE_DOMAIN_BLOCK_REBASE2_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_REBASE2 = 454
};
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index 0f87d13a5a..9cfb476fd9 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_rebase2_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_REBASE2 = 454,
};
--
2.47.3[replying both to cover letter and first patch, I've concatenated the
info from the cover letter here]
On Thu, Aug 21, 2025 at 14:51:36 +0300, Nikolai Barybin via Devel wrote:
> Current implementation of virDomainBlockPull is actually a subset of
> virDomainBlockRebase with NULL base.
>
> I suggest to add new generalised API call virDomainBlockRebase2 which
> will serve as a superset of both and support params list.
>
> Having params list is more flexible and extendible for future possible
> improvements of qemu block-stream or block-copy.
Could you please outline your motivations to do this besides the last
paragraph?
I'm asking because I personally think that virDomainBlockRebase trying
to "magically" do either block-stream or block-copy was a mistake.
The mistake was partially rectified by having virDomainBlockCopy as the
proper endpoint for block-copy/blockdev-mirror.
We do lack a fully-featured interface for 'block-stream' (to do
intermediate layer pull) but I don't think the fix for it is to have
once again a API that does both.
> For now only 'base' and 'bandwidth' params are proposed, but we can
> extend them, for example, to some block copy params
> (VIR_DOMAIN_BLOCK_REBASE_COPY).
On Thu, Aug 21, 2025 at 14:51:36 +0300, Nikolai Barybin via Devel wrote:
>
> This new API is a superset of virDomainBlockPull and
> virDomainBlockRebase which supports extendable list of
> virTypedParameterPtr params.
>
> If "base" == NULL and flags == 0 it is equivalent to virDomainBlockPull.
>
> Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com>
> ---
> include/libvirt/libvirt-domain.h | 20 +++++++++
> src/driver-hypervisor.h | 8 ++++
> src/libvirt-domain.c | 69 ++++++++++++++++++++++++++++++++
> 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, 129 insertions(+), 1 deletion(-)
I presume this series is more of a RFC, right? Otherwise we're missing
virsh support.
>
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 71bb49fe6c..cd33001fb4 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -4459,6 +4459,26 @@ 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 virDomainBlockRebase2 parameter 'base'
> + *
> + * Since: 11.6.0
11.6 was already released. 11.7 goes into freeze today.
> + */
> +#define VIR_DOMAIN_BLOCK_REBASE_PARAM_BASE "base"
> +
> +/**
> + * VIR_DOMAIN_BLOCK_REBASE_PARAM_BANDWIDTH:
> + * Macro for the virDomainBlockRebase2 parameter 'bandwidth'
> + *
> + * Since: 11.6.0
> + */
> +#define VIR_DOMAIN_BLOCK_REBASE_PARAM_BANDWIDTH "bandwidth"
The documentation for both of these is really poor. Bandwidth lacks what
the unit is, both lack which parameter type is expected. Base lacks what
it actually accepts and controls.
> +
> +int virDomainBlockRebase2(virDomainPtr dom, const char *disk,
> + virTypedParameterPtr params, int nparams,
> + unsigned int flags);
> +
[...]
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index ca110bdf85..7924a0d6cc 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -11197,6 +11197,75 @@ virDomainBlockRebase(virDomainPtr dom, const char *disk,
> }
>
>
> +/**
> + * virDomainBlockRebase2:
> + * @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 virDomainBlockRebaseFlags
> + *
> + * Generalized version of virDomainBlockRebase() which can act either as
> + * virDomainBlockRebase() or virDomainBlockPull() depending on @params and @flags.
This documentation is poor as well. You need to document the individual
parameters and how they impact the behaviour of the API. The specific
modes which are identical to existing APIs can be mentioned, but there
isn't really value saying that it (currently) does the same thing as the
two apis.
> + *
> + * Returns 0 if the operation has started, -1 on failure.
> + *
> + * Since: 11.6.0
> + */
> +int
> +virDomainBlockRebase2(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);
> + VIR_TYPED_PARAMS_DEBUG(params, nparams);
> +
> + virResetLastError();
> +
> + virCheckDomainReturn(dom, -1);
> + conn = dom->conn;
> +
> + virCheckReadOnlyGoto(conn->flags, error);
> + virCheckNonNullArgGoto(disk, error);
> +
> + if (flags & VIR_DOMAIN_BLOCK_REBASE_COPY) {
> + const char *base = NULL;
> + if (virTypedParamsGetString(params, nparams,
> + VIR_DOMAIN_BLOCK_REBASE_PARAM_BASE,
> + &base) <= 0 ||
> + base == NULL)
> + goto error;
> + } else if (flags & (VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
> + VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
> + VIR_DOMAIN_BLOCK_REBASE_COPY_RAW |
> + VIR_DOMAIN_BLOCK_REBASE_COPY_DEV)) {
> + virReportInvalidArg(flags, "%s",
> + _("use of flags requires a copy job"));
> + goto error;
> + }
> +
> + if (conn->driver->domainBlockRebase2) {
> + int ret;
> + ret = conn->driver->domainBlockRebase2(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..a1cb290c22 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.6.0 {
> + global:
> + virDomainBlockRebase2;
> +} 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..5902b1f04b 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 */
> + .domainBlockRebase2 = remoteDomainBlockRebase2, /* 11.6.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..41a85a4f71 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 rebase2 tunable parameters. */
> +const REMOTE_DOMAIN_BLOCK_REBASE2_PARAMETERS_MAX = 2;
'2' makes it un-extensible without modifying this. There's no point in
setting it so low as the code still needs to validate every parameter
sent by the client.
> +
> /* Upper limit on block copy tunable parameters. */
> const REMOTE_DOMAIN_BLOCK_COPY_PARAMETERS_MAX = 16;
>
>> Current implementation of virDomainBlockPull is actually a subset of >> virDomainBlockRebase with NULL base. >> >> I suggest to add new generalised API call virDomainBlockRebase2 which >> will serve as a superset of both and support params list. >> >> Having params list is more flexible and extendible for future possible >> improvements of qemu block-stream or block-copy. > Could you please outline your motivations to do this besides the last > paragraph? > > I'm asking because I personally think that virDomainBlockRebase trying > to "magically" do either block-stream or block-copy was a mistake. > > The mistake was partially rectified by having virDomainBlockCopy as the > proper endpoint for block-copy/blockdev-mirror. > > We do lack a fully-featured interface for 'block-stream' (to do > intermediate layer pull) but I don't think the fix for it is to have > once again a API that does both. > Peter, thank you for your review. Actually, you're totally right - that was my initial motivation: to provide better interface for 'block-stream'. I agree we don't have to mix stream and copy within single API call. I'll prepare v2 patchset with 'block-stream'-only API and keep in mind all your code related comments. Best regards, Nikolai
© 2016 - 2025 Red Hat, Inc.