From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52483) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TNoXq-0006jb-N7 for qemu-devel@nongnu.org; Mon, 15 Oct 2012 13:33:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TNoXm-00038Y-Gc for qemu-devel@nongnu.org; Mon, 15 Oct 2012 13:33:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46731) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TNoXm-00038L-8J for qemu-devel@nongnu.org; Mon, 15 Oct 2012 13:33:34 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q9FHXXBv020783 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 15 Oct 2012 13:33:33 -0400 Message-ID: <507C48E9.4040901@redhat.com> Date: Mon, 15 Oct 2012 19:33:29 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1348675011-8794-1-git-send-email-pbonzini@redhat.com> <1348675011-8794-28-git-send-email-pbonzini@redhat.com> In-Reply-To: <1348675011-8794-28-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 27/45] qmp: add drive-mirror command List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: jcody@redhat.com, qemu-devel@nongnu.org Am 26.09.2012 17:56, schrieb Paolo Bonzini: > This adds the monitor commands that start the mirroring job. > > Signed-off-by: Paolo Bonzini > --- > blockdev.c | 125 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- > hmp-commands.hx | 21 ++++++++++ > hmp.c | 28 +++++++++++++ > hmp.h | 1 + > qapi-schema.json | 33 +++++++++++++++ > qmp-commands.hx | 42 +++++++++++++++++++ > 6 file modificati, 249 inserzioni(+). 1 rimozione(-) > > diff --git a/blockdev.c b/blockdev.c > index 9069ca1..722aab5 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -1118,6 +1117,130 @@ void qmp_block_stream(const char *device, bool has_base, > trace_qmp_block_stream(bs, bs->job); > } > > +void qmp_drive_mirror(const char *device, const char *target, > + bool has_format, const char *format, > + enum MirrorSyncMode sync, > + bool has_mode, enum NewImageMode mode, > + bool has_speed, int64_t speed, Error **errp) > +{ > + BlockDriverInfo bdi; > + BlockDriverState *bs; > + BlockDriverState *source, *target_bs; > + BlockDriver *proto_drv; > + BlockDriver *drv = NULL; > + Error *local_err = NULL; > + int flags; > + uint64_t size; > + int ret; > + > + if (!has_speed) { > + speed = 0; > + } > + if (!has_mode) { > + mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS; > + } > + > + bs = bdrv_find(device); > + if (!bs) { > + error_set(errp, QERR_DEVICE_NOT_FOUND, device); > + return; > + } > + > + if (!has_format) { > + format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name; bs->drv can be NULL for removable media. (Worth a test case?) > + } > + if (format) { > + drv = bdrv_find_format(format); > + if (!drv) { > + error_set(errp, QERR_INVALID_BLOCK_FORMAT, format); > + return; > + } > + } > + > + if (!bdrv_is_inserted(bs)) { > + error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device); > + return; > + } > + > + if (bdrv_in_use(bs)) { > + error_set(errp, QERR_DEVICE_IN_USE, device); > + return; > + } > + > + flags = bs->open_flags | BDRV_O_RDWR; The two questions from last time are still open: Jeff's patches are in now, so we can do a bdrv_reopen() to remove BDRV_O_RDWR again when completing the mirror job. The other thing was the throttling. It's not entirely clear to me what our conclusion was, but you suggested to remove it entirely from the mirror because I/O throttling on the target is equivalent. Of course, you can only throttle the target as soon as it has a name. What was your plan with that? > + source = bs->backing_hd; > + if (!source && sync == MIRROR_SYNC_MODE_TOP) { > + sync = MIRROR_SYNC_MODE_FULL; > + } > + > + proto_drv = bdrv_find_protocol(target); > + if (!proto_drv) { > + error_set(errp, QERR_INVALID_BLOCK_FORMAT, format); Not a great error message for this case. Kevin