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(-) > > > > >