Re: [Qemu-devel] [RFC PATCH v3 0/2] Point-in-time snapshot exporting over NBD

[Ian Main]

On Thu, Oct 17, 2013 at 01:36:41PM +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).

In general this seems to work great.  I did a bunch of testing and was
able to mount filesystems over NBD, do many writes (on backed up
image)/reads (on target), intense IO etc and all held up fine.

There are definitely some issues with some of the commands allowing you
to blow things up.  I'm interested to hear opinions on whether this is a
showstopper at this time or not.

> 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

I had to create a custom python script to make these commands work as
they require nested dicts.  Is that normal or did I miss something
entirely?
 
>     (where ide0-hd0 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-complete device=ide0-hd0

To be clear it is block-job-cancel that is used.  There is no 'complete'
handler for the backup block job so this just returns an error.

>  2. (HMP) drive_del target0

In order for this to work with libvirt I think we are going to need a QMP
interface for this.
 
>  3. (SHELL) rm BACKUP.qcow2
> 
> v3: Base on blockdev-add.
>     The syntax blockdev-add backing=<id> is new: This will make referenced BDS
>     in the middle of a backing chain, which has significant effects over all
>     existing block operations in QMP. It needs to be reviewed and tested 
> carefully.
> 
>     I'm listing the commands here that can take a device id as parameter (but
>     may not be complete).
> 
>     These commands do not mutate the backing chain (not directly, at least) 
> and
>     should be safe:
>         block_passwd
>         block_set_io_throttle
>         block-job-set-speed
>         block-job-cancel
>         block-job-pause
>         block-job-resume
>         block-job-complete
>         drive-backup
>         blockdev-snapshot-sync
>         blockdev-snapshot-internal-sync
>         blockdev-snapshot-delete-internal-sync: These should be safe.
>         nbd-server-add
> 
>     These can mutates the chain (removing, closing or swapping BDS), need more
>     work to convert them to safe operations with a BDS in the middle.
>         device_del

I couldn't figure out how to make this operate on block devices or how
to make it break things.. :)

>         eject: it does bdrv_close the device.
>         change: internally calls eject.

Calling eject or change on the target causes a 'failed command: WRITE DMA'
backtrace in the VM which causes the root fs to be remounted read only.
I'm guessing from the CoW failing..

>         block-commit: it deletes intermediate BDSes, which may include other 
> named BDS.
>         block-stream: 
>         drive-mirror: it swaps active with target when complete.

Doing a mirror of the target causes a segfault when completed.. :)  I
gather you are not allowed to perform multiple block jobs on 1 BDS at a
time as I get 'in use' errors when trying to do the same BDS as the one
you are backing up.  Doing a drive-mirror on the bottom-most image of
3 seemed to work ok.
 
>     Resizing a middle BDS need to be reviewed too:
>         block_resize: TBD.

I tried resizing a middle BDS and got a 'middle0 is in use' error.  The
topmost BDS got a 'feature/command not supported' error.  I was able to
resize the bottom BDS and then got I/O errors, eg:

{u'timestamp': {u'seconds': 1384913492, u'microseconds': 993397},
u'data': {u'device': u'ide0-hd0', u'action': u'report', u'operation':
u'read'}, u'event': u'BLOCK_IO_ERROR'}

However the NBD export *seemed* to be ok and everything kept running..
 
>     Adding and backing HD referencing will put other BDS in middle, but should
>     not immediately break things:
>         blockdev-add

Not quite sure what you mean here.  I can add other BDS's but could I
put one in the middle while in a backup?  That would require being able
to change the 'backing' on the fly which I don't think we can do?  (or
should be able to do.. heh).
 
> Fam Zheng (2):
>   block: parse "backing" option to reference existing BDS
>   qmp: add command 'blockdev-backup'
> 
>  block.c          | 27 +++++++++++++++++++-----
>  blockdev.c       | 63 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qapi-schema.json | 49 +++++++++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx  | 46 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 180 insertions(+), 5 deletions(-)
> 
> -- 
> 1.8.3.1
>