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

[Fam Zheng]

On 2013年12月12日 16:14, Markus Armbruster wrote:
> Fam Zheng <address@hidden> writes:
>
> > On 2013年11月28日 16:39, 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 ide0-hd0 is the running BlockDriverState name for
> > >       RUNNING-VM.img. This patch implements "backing=" option to override
> > >       backing_hd for added drive)
>
> Are source-drive and ide0-hd0 the same thing?

Yes, typo. Sorry for that.

> > >    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-complete device=ide0-hd0
> > >
> > >    2. (HMP) drive_del target0
> > >
> > >    3. (SHELL) rm BACKUP.qcow2
> > >
> > > v6: Address Paolo's comments, (except for bitmask):
> > >       - Add blocker for all backing_hd references, a relatively big change, some
> > >         patches are reordered.
> > >       - Introduce a few other necessary patches.
> > >       - Move two snapshot checks into bdrv_snapshot_*.
> > >
> > >       The interface is unchanged.
> >
> > Hi,
> >
> > Based on the size of change, this series needs some more review before
> > merging. And I'd like to know if there is any concern or objection
> > with op_blocker introduced here. I would like to base my next series
> > (incremental backup with dirty bitmap) on it.
> >
> > Any more reviews/comments?
>
> I started looking over it.  First observation: needs a rebase :-}

I'll do it now.

Fam