From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40103) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avKNL-0001FR-2G for qemu-devel@nongnu.org; Wed, 27 Apr 2016 03:59:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1avKNH-0002xX-0w for qemu-devel@nongnu.org; Wed, 27 Apr 2016 03:59:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56147) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avKNG-0002xJ-QK for qemu-devel@nongnu.org; Wed, 27 Apr 2016 03:59:06 -0400 Date: Wed, 27 Apr 2016 15:59:10 +0800 From: Fam Zheng Message-ID: <20160427075910.GA1421@ad.usersys.redhat.com> References: <1461400913-7321-1-git-send-email-den@openvz.org> <20160426094654.GE10650@stefanha-x1.localdomain> <57205CE2.4070009@openvz.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <57205CE2.4070009@openvz.org> Subject: Re: [Qemu-devel] [PATCH for 2.7 0/2] backup compression List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Denis V. Lunev" Cc: Stefan Hajnoczi , Kevin Wolf , Pavel Butsykin , Jeff Cody , Markus Armbruster , qemu-devel@nongnu.org, John Snow On Wed, 04/27 09:32, Denis V. Lunev wrote: > On 04/26/2016 12:46 PM, Stefan Hajnoczi wrote: > > On Sat, Apr 23, 2016 at 11:41:51AM +0300, Denis V. Lunev wrote: > > > The idea is simple - backup is "written-once" data. It is written block > > > by block and it is large enough. It would be nice to save storage > > > space and compress it. > > > > > > These patches add the ability to compress data during backup. This > > > functionality is implemented by means of adding options to the qmp/hmp > > > commands(drive-backup, blockdev-backup). The implementation is quite > > > simple, because the responsibility for data compression imposed on the > > > format driver. > > > > > > Signed-off-by: Pavel Butsykin > > > Signed-off-by: Denis V. Lunev > > > CC: Jeff Cody > > > CC: Markus Armbruster > > > CC: Eric Blake > > > CC: John Snow > > > > > > Pavel Butsykin (2): > > > drive-backup: added support for data compression > > > blockdev-backup: added support for data compression > > > > > > block/backup.c | 13 +++++++++++++ > > > blockdev.c | 20 ++++++++++++++++++-- > > > hmp-commands.hx | 8 +++++--- > > > hmp.c | 3 ++- > > > include/block/block_int.h | 1 + > > > qapi/block-core.json | 3 ++- > > > qmp-commands.hx | 7 +++++-- > > > 7 files changed, 46 insertions(+), 9 deletions(-) > > bdrv_write_compressed() hasn't been called from a running VM before, it > > was purely a synchronous qemu-img operation. The > > qcow2.c:qcow2_write_compressed() code doesn't seem to be written with > > coroutine context in mind but I think the lock in > > block/backup.c:backup_do_cow() will protect against race conditions. > > > > Please include a test case (using qcow2?). > I have looked into the code. The only problematic thing there > is that the compression is performed directly in the write, > which could take a bit of time. > > Thus responsiveness of the guest system could be reduced. > Though I do not expect here troubles. I think that it is worth > to give this tech a chance. Compression could be moved to > the outer thread in the next step if required. As far as I can tell, in order to be used in a coroutine, we need a "qemu_co_mutex_lock(&s->lock);" pair in vmdk_write_compressed. Something similar probably apply to qcow2 as well. Maybe what we should do is adding a bdrv_co_write_compressed(). Fam