From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41592) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V1nMt-00037v-6F for qemu-devel@nongnu.org; Tue, 23 Jul 2013 20:55:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V1nMq-00013H-C9 for qemu-devel@nongnu.org; Tue, 23 Jul 2013 20:55:51 -0400 Received: from mx1.redhat.com ([209.132.183.28]:29471) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V1nMq-00013B-3y for qemu-devel@nongnu.org; Tue, 23 Jul 2013 20:55:48 -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 r6O0tlvA026670 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 23 Jul 2013 20:55:47 -0400 Date: Wed, 24 Jul 2013 08:55:44 +0800 From: Fam Zheng Message-ID: <20130724005544.GA4818@T430s.redhat.com> References: <1374530960-22031-1-git-send-email-imain@redhat.com> <20130723115351.GH20256@stefanha-thinkpad.redhat.com> <20130723195505.GA2744@gate.mains.priv> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130723195505.GA2744@gate.mains.priv> Subject: Re: [Qemu-devel] [PATCH V6 0/3] Implement sync modes for drive-backup. Reply-To: famz@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ian Main Cc: kwolf@redhat.com, qemu-devel@nongnu.org, rjones@redhat.com, Stefan Hajnoczi , pbonzini@redhat.com On Tue, 07/23 12:55, Ian Main wrote: > On Tue, Jul 23, 2013 at 01:53:51PM +0200, Stefan Hajnoczi wrote: > > On Mon, Jul 22, 2013 at 03:09:17PM -0700, Ian Main wrote: > > > This patch adds sync modes on top of the work that Stefan Hajnoczi has done. > > > > > > These patches apply on kevin/block. > > > > > > Hopefully all is in order as this is my first QEMU patch. Many thanks to > > > Stephan and Fam Zheng for their help. > > > > > > V2: > > > > > > - No longer poll, instead use qemu_coroutine_yield(). > > > - Use bdrv_co_is_allocated(). > > > - Much better SYNC_MODE_NONE test. > > > > > > V3: > > > > > > - A few style fixes. > > > - Better commit message explaining how TOP and NONE operate. > > > - Verified using checkpatch.pl. > > > > > > V4: > > > > > > - Add patch to use the source as a backing hd during backup. > > > - Add patch to default sync mode none to qcow2. > > > > > > V5: > > > > > > - Fix qcow2 patch. Forgot to git add final version. > > > > > > V6: > > > > > > - Default to requiring 'format' when mode is absolute-paths. > > > - Removed one bad hunk that was misapplying. > > > - Fixed docs, examples and tests to match changes. > > > - Added tests for format bad/missing. > > > - Added bdrv_set_in_use() to target. > > > - Default to qcow2 patch not required. > > > > > > Ian Main (3): > > > Implement sync modes for drive-backup. > > > Add tests for sync modes 'TOP' and 'NONE' > > > Add backing drive while performing backup. > > > > > > block/backup.c | 107 +++++++++++++++++++++++++++++------------ > > > blockdev.c | 36 +++++++++----- > > > include/block/block_int.h | 4 +- > > > qapi-schema.json | 4 +- > > > qmp-commands.hx | 2 + > > > tests/qemu-iotests/055 | 108 +++++++++++++++++++++++++++++++++++++----- > > > tests/qemu-iotests/055.out | 4 +- > > > tests/qemu-iotests/group | 2 +- > > > tests/qemu-iotests/iotests.py | 5 ++ > > > 9 files changed, 211 insertions(+), 61 deletions(-) > > > > This patch mostly takes care of image fleecing except it does not give > > the target a device name which can be used by nbd-server-add. > > It's not clear to me how to do that. I looked through Fams series and > was hoping to find a clue there but I may have missed it. In short, my patches added QMP, the device id version of drive-backup: blockdev-backup device=source-id target=target-id where it needs the target already there: (HMP) drive_add ...,backing=source-id" "backing=source-id" is added there too, it's just the same purpose to override backing_hd, as you added in block/backup.c Then we have the target-id to use with NBD. > > > Fam's series tackles the target device name and some of the overlapping > > problems with your series. > > Yes which is great. > > > The core feature in your series is sync=top|none and that needs to be > > merged. > > > > Now we need to figure out which patches to take and what must be > > changed. Please see the sub-threads on Fam's series. Perhaps we can > > reach a consensus there. > > Yes I'm happy to offer whatever help this is. It does seem like > applying this is the next logical move however. Fam could then base his > patch on top of this one. At some point soon we need to reconcile > everything and to me that seems the logical way. There actually doesn't > seem to be too much overlap in effort from what I can see. The overlapping I can see is overriding backing_hd, but I'm not sure which way will end merged, or both. > > Fam, what do you think? I'm happy to make changes to make it easier for > your patchset too. > I think it's fine till now, the hard and unresolved parts are adding bits to control interface, which is independent on your patches. Thanks. -- Fam