From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43253) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avKdS-0000bn-Hx for qemu-devel@nongnu.org; Wed, 27 Apr 2016 04:15:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1avKdN-00074o-Sv for qemu-devel@nongnu.org; Wed, 27 Apr 2016 04:15:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46141) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avKdN-00074f-Lx for qemu-devel@nongnu.org; Wed, 27 Apr 2016 04:15:45 -0400 Date: Wed, 27 Apr 2016 10:15:42 +0200 From: Kevin Wolf Message-ID: <20160427081542.GB4756@noname.str.redhat.com> References: <1461400913-7321-1-git-send-email-den@openvz.org> <20160426094654.GE10650@stefanha-x1.localdomain> <57205CE2.4070009@openvz.org> <20160427075910.GA1421@ad.usersys.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160427075910.GA1421@ad.usersys.redhat.com> 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: Fam Zheng Cc: "Denis V. Lunev" , Stefan Hajnoczi , Pavel Butsykin , Jeff Cody , Markus Armbruster , qemu-devel@nongnu.org, John Snow Am 27.04.2016 um 09:59 hat Fam Zheng geschrieben: > 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(). Yes, I agree. Actually, I have patched write_compressed in VMDK already as I converted all of the block drivers from the legacy .bdrv_read/write interface to a new .bdrv_preadv/pwritev one (will send out the series today, hopefully), but it would be much nicer to do this properly in block/io.c. Kevin