From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55544) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZHj3g-00078W-2t for qemu-devel@nongnu.org; Tue, 21 Jul 2015 21:42:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZHj3f-0003y6-6q for qemu-devel@nongnu.org; Tue, 21 Jul 2015 21:42:56 -0400 Date: Wed, 22 Jul 2015 09:42:46 +0800 From: Fam Zheng Message-ID: <20150722014246.GB18149@ad.nay.redhat.com> References: <1435202570-12360-1-git-send-email-famz@redhat.com> <1435202570-12360-6-git-send-email-famz@redhat.com> <55AD14DA.7040004@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55AD14DA.7040004@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 5/6] qmp: Add blockdev-mirror command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: Kevin Wolf , qemu-block@nongnu.org, Jeff Cody , qemu-devel@nongnu.org, Markus Armbruster , Stefan Hajnoczi , pbonzini@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 > >--- > > 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