[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for 2.7 0/2] backup compression
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH for 2.7 0/2] backup compression |
Date: |
Wed, 27 Apr 2016 10:15:42 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden>
> > > > Signed-off-by: Denis V. Lunev <address@hidden>
> > > > CC: Jeff Cody <address@hidden>
> > > > CC: Markus Armbruster <address@hidden>
> > > > CC: Eric Blake <address@hidden>
> > > > CC: John Snow <address@hidden>
> > > >
> > > > 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