Re: [Qemu-devel] [PATCH v2 5/6] qmp: Add blockdev-mirror command

[Fam Zheng <famz@redhat.com>]

On Mon, 07/20 17:33, Max Reitz wrote:
> On 25.06.2015 05:22, Fam Zheng wrote:
> >This will start a mirror job from a named device to another named
> >device, its relation with drive-mirror is similar with blockdev-backup
> >to drive-backup.
> >
> >In blockdev-mirror, the target node should be prepared by blockdev-add,
> >which will be responsible for assigning a name to the new node, so
> >'node-name' in drive-mirror is dropped.
> >
> >Signed-off-by: Fam Zheng <famz@redhat.com>
> >---
> >  blockdev.c           | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  qapi/block-core.json | 47 ++++++++++++++++++++++++++++++++++++++++
> >  qmp-commands.hx      | 48 +++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 156 insertions(+)
> >
> >diff --git a/blockdev.c b/blockdev.c
> >index de6383f..e0dba07 100644
> >--- a/blockdev.c
> >+++ b/blockdev.c
> >@@ -2932,6 +2932,10 @@ static void blockdev_mirror_common(BlockDriverState *bs,
> >      if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_MIRROR_TARGET, errp)) {
> >          return;
> >      }
> >+    if (target->blk) {
> >+        error_setg(errp, "Cannot mirror to an attached block device");
> >+        return;
> >+    }
> 
> 1) Why?

To match the limitation of drive-mirror. We don't yet have a block job that
writes to attached device yet (except for stream, but that's only copy on
read). I've no idea what that implies, and I don't know if there is even a use
case.

> 
> 2) I think this should be noted in the QMP interface documentation. "the
> name of the device to mirror to" sounds like it is actually meant to be an
> attached block device.

I'll update the documentation.

Fam