From: dinglimin <dinglimin@cmss.chinamobile.com>

The BlockCopy command is designed to copy a chain of disk backup images to dest.
For RBD external destination, before the modification, only the '--XML' parameter is supported by 'blockdev-mirror'.
After the modification, the '--dest' parameter(--dest 'rbd:xxx/xxx:auth_supported=none:mon_host=xxx.xxx.xxx.xxx')can be used.

Signed-off-by: dinglimin <dinglimin@cmss.chinamobile.com>
---
 src/qemu/qemu_driver.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 760d30a..db15898 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14963,12 +14963,27 @@ qemuDomainBlockCopyCommon(virDomainObj *vm,
     virStorageSource *mirrorBacking = NULL;
     g_autoptr(GHashTable) blockNamedNodeData = NULL;
     int rc = 0;
+    const char *p = NULL;
 
     /* Preliminaries: find the disk we are editing, sanity checks */
     virCheckFlags(VIR_DOMAIN_BLOCK_COPY_SHALLOW |
                   VIR_DOMAIN_BLOCK_COPY_REUSE_EXT |
                   VIR_DOMAIN_BLOCK_COPY_TRANSIENT_JOB, -1);
 
+    /* if mirror->path contains 'rbd:' prefix, set rbd attributes */
+    if (STRPREFIX(mirror->path, "rbd:")) {
+        mirror->format = VIR_STORAGE_FILE_RAW;
+        mirror->protocol = VIR_STORAGE_NET_PROTOCOL_RBD;
+        mirror->type = VIR_STORAGE_TYPE_NETWORK;
+
+        p = g_strdup(mirror->path);
+        if (virStorageSourceParseRBDColonString(p, mirror) < 0) {
+          virReportError(VIR_ERR_INVALID_ARG, "%s",
+                         _("RBD path conversion failed"));
+          return -1;
+        }
+    }
+  
     if (virStorageSourceIsRelative(mirror)) {
         virReportError(VIR_ERR_INVALID_ARG, "%s",
                        _("absolute path must be used as block copy target"));
-- 
1.8.3.1

On Sat, Oct 09, 2021 at 15:55:54 +0800, dinglimin@cmss.chinamobile.com wrote:
> From: dinglimin <dinglimin@cmss.chinamobile.com>
> 
> The BlockCopy command is designed to copy a chain of disk backup images to dest.
> For RBD external destination, before the modification, only the '--XML' parameter is supported by 'blockdev-mirror'.
> After the modification, the '--dest' parameter(--dest 'rbd:xxx/xxx:auth_supported=none:mon_host=xxx.xxx.xxx.xxx')can be used.
> 
> Signed-off-by: dinglimin <dinglimin@cmss.chinamobile.com>
> ---
>  src/qemu/qemu_driver.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 760d30a..db15898 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -14963,12 +14963,27 @@ qemuDomainBlockCopyCommon(virDomainObj *vm,
>      virStorageSource *mirrorBacking = NULL;
>      g_autoptr(GHashTable) blockNamedNodeData = NULL;
>      int rc = 0;
> +    const char *p = NULL;
>  
>      /* Preliminaries: find the disk we are editing, sanity checks */
>      virCheckFlags(VIR_DOMAIN_BLOCK_COPY_SHALLOW |
>                    VIR_DOMAIN_BLOCK_COPY_REUSE_EXT |
>                    VIR_DOMAIN_BLOCK_COPY_TRANSIENT_JOB, -1);
>  
> +    /* if mirror->path contains 'rbd:' prefix, set rbd attributes */
> +    if (STRPREFIX(mirror->path, "rbd:")) {
> +        mirror->format = VIR_STORAGE_FILE_RAW;
> +        mirror->protocol = VIR_STORAGE_NET_PROTOCOL_RBD;
> +        mirror->type = VIR_STORAGE_TYPE_NETWORK;
> +
> +        p = g_strdup(mirror->path);
> +        if (virStorageSourceParseRBDColonString(p, mirror) < 0) {
> +          virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                         _("RBD path conversion failed"));
> +          return -1;
> +        }
> +    }

NACK,

the correct approach is as you mention in the commit message is to use
the "--xml" parameter of 'virsh blockcopy' to pass a XML describing the
RBD destionation.

We don't want to add a hack to do this via the --dest parameter. Use the
XML.

On Sun, Oct 10, 2021 at 21:41:57 +0200, Peter Krempa wrote:
> On Sat, Oct 09, 2021 at 15:55:54 +0800, dinglimin@cmss.chinamobile.com wrote:
> > From: dinglimin <dinglimin@cmss.chinamobile.com>
> > 
> > The BlockCopy command is designed to copy a chain of disk backup images to dest.
> > For RBD external destination, before the modification, only the '--XML' parameter is supported by 'blockdev-mirror'.
> > After the modification, the '--dest' parameter(--dest 'rbd:xxx/xxx:auth_supported=none:mon_host=xxx.xxx.xxx.xxx')can be used.
> > 
> > Signed-off-by: dinglimin <dinglimin@cmss.chinamobile.com>
> > ---
> >  src/qemu/qemu_driver.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)

[...]

> 
> NACK,
> 
> the correct approach is as you mention in the commit message is to use
> the "--xml" parameter of 'virsh blockcopy' to pass a XML describing the
> RBD destionation.
> 
> We don't want to add a hack to do this via the --dest parameter. Use the
> XML.
> 

As a side note. If you want to implement syntax suggar to create the
--xml argument for you via command line parameters you can add virsh
command called 'virsh block-copy-as'. Take inspiration from 'virsh
attach-disk-as' on how network disks are specified.

Modifying the daemon code to accept this hack is not acceptable
nevertheless.