[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [v2 Patch 0/9]block: Image file reopen and dynamic host
From: |
Jeff Cody |
Subject: |
Re: [Qemu-devel] [v2 Patch 0/9]block: Image file reopen and dynamic host pagecache change |
Date: |
Thu, 09 Aug 2012 00:26:05 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120717 Thunderbird/14.0 |
On 07/30/2012 05:34 PM, Supriya Kannery wrote:
> For changing host pagecache setting of a running VM, it is
> important to have a safe way of reopening its image file.
>
> V1 introduced:
> * a generic way to reopen image files safely.
> In this approach, before reopening an image, for each
> block driver, its state will be stashed. Incase preparation,
> (bdrv_reopen_prepare) for reopening returns success, the stashed
> state will be cleared (bdrv_reopen_commit) and reopened state will
> be used further. Incase preparation of reopening returns failure,
> the state of the driver will be rolled back (bdrv_reopen_abort)
> to the stashed state. This approach is implemented for raw-posix,
> raw-win32, vmdk, qcow, qcow2 and qed block drivers.
>
> * qmp and hmp command 'block_set_hostcache' using which host
> pagecache setting for a block device can be changed
> when the VM is running.
>
> * BDRVReopenState, a generic structure which can be
> extended by each of the block drivers to reopen
> respective image files.
>
> V2:
> * Changed ordering of patches such that code changes related to
> generic framework for safely reopening images gets applied first.
>
> * For block drivers not having bdrv_reopen_xx functions
> implemented, return "feature not supported" error.
>
> Testing:
> =======
> [Thanks! to Yoganananth Subramanian for helping out with testing]
>
> Steps:
> 1) boot up guest image of different formats qed, raw, qcow2, vmdk
> 2) run iozone in these guests
> command: iozone -a
> 3) view cache setting of image file through qemu monitor
> command: info block
> "hostcache =" 0/1 should be displayed
> 4) Toggle hostcache value using block_set_hostcache
> command: block_set_hostcache virtio0 on
> 5) Disable and enable hostcache at randon intervals while iozone is
> running inside guest.
> command: block_set_hostcache virtio0 on/off
> 6) Info block should reflect toggled hostcache value
> and iozone should complete without any issue
>
> Results:
> Verified above steps for raw-posix, qcow2, qed and vmdk images.
> raw-posix, qed and vmdk images (split files) worked fine. With
> qcow2 image getting an error of double free after iozone running
> for a while.
I believe the double free you are seeing above is due to closing the
refcount at the end of qcow2_reopen_commit() (see comments on patch
5/9). Whenever that image is closed with bdrv_close(),
qcow2_refcount_close() is called again.
I had to make just a few small changes to the patches to get them
working for reopen() calls that had file access permission changes
(r/o->r/w), so I will try and document each of these changes in replies
to your other patches.
I applied the changes in my other replies your patches to my github branch
so they would work for going from r/o->r/w->r/o, for testing block commit:
https://github.com/codyprime/qemu-kvm-jtc/commits/jtc-live-commit-v3
I only made patches for qcow2 and raw-posix, which is what I have been
testing with block commit (although I imagine the changes for the other
formats are similar).
Another thought, that is more design oriented: as it currently is,
reopen() is not transactional for multiple BlockDriverStates. It would
be nice if we could queue up multiple BDSs to reopen into a transaction,
and only perform the commit() if the prepare() was successful for all of
the BDS entries. Perhaps using something like QSIMPLEQ could be useful
to accomplish this.
I think this is especially important for formats like qcow2, where we
need to reopen both the bs and bs->file.
>
> To Do:
> ======
> * Debug the issue with qcow2 image and resolve asap.
> * Enhance code around dup3 in raw-posix to fall back to
> dup2/dup when dup3 is not supported by OS.
> * Do some more extensive testing, especially with qcow2 and
> qed drivers.
>
> New block command added:
> "block_set_hostcache"
> -- Sets hostcache parameter for block device while guest is running.
>
> Usage:
> block_set_hostcache <device> <option>
> <device> = block device
> <option> = on/off
>
> qemu/block.c | 79 ++++++++++++++++++++++
> qemu/block.h | 5 +
> qemu/block/qcow.c | 108 ++++++++++++++++++++++++++++++
> qemu/block/qcow2.c | 175
> +++++++++++++++++++++++++++++++++++++++++++++++++
> qemu/block/qed.c | 103 ++++++++++++++++++++++++++++
> qemu/block/raw-posix.c | 121 +++++++++++++++++++++++++++++++++
> qemu/block/raw-win32.c | 96 ++++++++++++++++++++++++++
> qemu/block/raw.c | 20 +++++
> qemu/block/vmdk.c | 103 ++++++++++++++++++++++++++++
> qemu/block_int.h | 12 +++
> qemu/blockdev.c | 19 +++++
> qemu/hmp-commands.hx | 15 ++++
> qemu/hmp.c | 11 ++
> qemu/hmp.h | 1
> qemu/qapi-schema.json | 21 ++++-
> qemu/qemu-common.h | 1
> qemu/qmp-commands.hx | 24 ++++++
> 17 files changed, 912 insertions(+), 2 deletions(-)
>
>
>
>
>