Re: [Qemu-devel] [PATCH v16 00/14] Drop in_use from BlockDriverState and enable point-in-time snapshot exporting over NBD

[Benoît Canet]

The Monday 10 Mar 2014 à 14:38:12 (+0800), Fam Zheng wrote :
> On Mon, 03/10 06:13, Benoît Canet wrote:
> > The Friday 07 Mar 2014 à 16:20:42 (+0800), Fam Zheng wrote :
> > > This series adds for point-in-time snapshot NBD exporting based on
> > > blockdev-backup (variant of drive-backup with existing device as target).
> > > 
> > > We get a thin point-in-time snapshot by COW mechanism of drive-backup, and
> > > export it through built in NBD server. The steps are as below:
> > > 
> > >  1. (SHELL) qemu-img create -f qcow2 BACKUP.qcow2 <source size here>
> > > 
> > >     (Alternatively we can use -o backing_file=RUNNING-VM.img to omit 
> > > explicitly
> > >     providing the size by ourselves, but it's risky because 
> > > RUNNING-VM.qcow2 is
> > >     used r/w by guest. Whether or not setting backing file in the image 
> > > file
> > >     doesn't matter, as we are going to override the backing hd in the next
> > >     step)
> > > 
> > >  2. (QMP) blockdev-add backing=source-drive file.driver=file 
> > > file.filename=BACKUP.qcow2 id=target0 if=none driver=qcow2
> > > 
> > >     (where source-drive is the running BlockDriverState name for
> > >     RUNNING-VM.img. This patch implements "backing=" option to override
> > >     backing_hd for added drive)
> > > 
> > >  3. (QMP) blockdev-backup device=source-drive sync=none target=target0
> > > 
> > >     (this is the QMP command introduced by this series, which use a named
> > >     device as target of drive-backup)
> > > 
> > >  4. (QMP) nbd-server-add device=target0
> > > 
> > > When image fleecing done:
> > > 
> > >  1. (QMP) block-job-cancel device=source-drive
> > > 
> > >  2. (HMP) drive_del target0
> > > 
> > >  3. (SHELL) rm BACKUP.qcow2
> > > 
> > > v16: Address review comments from Jeff and Markus (thanks for reviewing!)
> > >      (A side-by-side diff of v15 -> v16: http://goo.gl/1goc6S)
> > > 
> > >     [03/14] block: Replace in_use with operation blocker
> > >             Improve error message. (Markus)
> > > 
> > >     [05/14] block: Add bdrv_set_backing_hd()
> > >             in bdrv_set_backing_hd:
> > >             * Don't call bdrv_ref and bdrv_unref. (Jeff)
> > >             * Add "bs->open_flags &= ~BDRV_O_NO_BACKING". (Jeff)
> > >             in bdrv_open_backing_file:
> > >             * Remove unnecessary "bs->backing_hd = NULL;". (Jeff)
> > >             * Don't get wrong refcnt on backing_hd. (Jeff)
> > >             in bdrv_append:
> > >             * Remove unnessary backing_file and backing_format copy, it's 
> > > taken
> > >               care of in bdrv_set_backing_hd(). (Jeff)
> > > 
> > >     [06/14] block: Add backing_blocker in BlockDriverState
> > >             Add call to bdrv_ref() since bdrv_set_backing_hd() doesn't do 
> > > it
> > >             now.
> > > 
> > >     [08/14] block: Support dropping active in bdrv_drop_intermediate
> > >             Verify active, top, base are in the same backing chain. (Jeff)
> > >             Remember to change backing file when base != NULL. (Jeff)
> > >             
> > >     [09/14] stream: Use bdrv_drop_intermediate and drop 
> > > close_unused_images
> > >             Drop more unnecessary code. (Jeff)
> > > 
> > > Thanks,
> > > Fam
> > > 
> > > 
> > > Fam Zheng (14):
> > >   block: Add BlockOpType enum
> > >   block: Introduce op_blockers to BlockDriverState
> > >   block: Replace in_use with operation blocker
> > >   block: Move op_blocker check from block_job_create to its caller
> > >   block: Add bdrv_set_backing_hd()
> > >   block: Add backing_blocker in BlockDriverState
> > >   block: Parse "backing" option to reference existing BDS
> > >   block: Support dropping active in bdrv_drop_intermediate
> > >   stream: Use bdrv_drop_intermediate and drop close_unused_images
> > >   qmp: Add command 'blockdev-backup'
> > >   block: Allow backup on referenced named BlockDriverState
> > >   block: Add blockdev-backup to transaction
> > >   qemu-iotests: Test blockdev-backup in 055
> > >   qemu-iotests: Image fleecing test case 083
> > > 
> > >  block-migration.c               |   7 +-
> > >  block.c                         | 314 
> > > +++++++++++++++++++++++++++-------------
> > >  block/backup.c                  |  26 ++++
> > >  block/commit.c                  |   2 +-
> > >  block/mirror.c                  |   1 +
> > >  block/stream.c                  |  42 +-----
> > >  blockdev.c                      | 122 ++++++++++++++--
> > >  blockjob.c                      |  14 +-
> > >  hw/block/dataplane/virtio-blk.c |  19 ++-
> > >  include/block/block.h           |  29 +++-
> > >  include/block/block_int.h       |   9 +-
> > >  include/block/blockjob.h        |   3 +
> > >  qapi-schema.json                |  50 +++++++
> > >  qmp-commands.hx                 |  44 ++++++
> > >  tests/qemu-iotests/055          | 275 +++++++++++++++++++++++++++++------
> > >  tests/qemu-iotests/055.out      |   4 +-
> > >  tests/qemu-iotests/083          |  99 +++++++++++++
> > >  tests/qemu-iotests/083.out      |   5 +
> > >  tests/qemu-iotests/group        |   1 +
> > >  19 files changed, 854 insertions(+), 212 deletions(-)
> > >  create mode 100755 tests/qemu-iotests/083
> > >  create mode 100644 tests/qemu-iotests/083.out
> > > 
> > > -- 
> > > 1.9.0
> > > 
> > 
> > Hi Fam,
> > 
> > I want to rebase my patches on top of yours but I have trouble applying your
> > series to kevin/block-next. Is it based on it ?
> > 
> 
> No, I used qemu.git master. I see some conflicts, I will rebase and send to
> list.  But I think you meant kevin/block, since kevin/block-next looks very
> old, right?
> 
> Fam
> 

I tried both without success.
Thanks

Best regards

Benoît