Re: [Qemu-devel] [PATCH 2/4] block: add a helper to change writeback mode on the fly

[Kevin Wolf]

Am 17.03.2011 15:44, schrieb Daniel P. Berrange:
> On Tue, Mar 15, 2011 at 03:11:32PM +0100, Christoph Hellwig wrote:
>> Add a new bdrv_change_cache that can set/clear the writeback flag
>> at runtime by stopping all I/O and closing/reopening the image file.
>>
>> All code is based on a patch from Prerna Saxena <address@hidden>
>> with minimal refactoring.
>>
>> Signed-off-by: Christoph Hellwig <address@hidden>
>>
>>  
>> +static int bdrv_reopen(BlockDriverState *bs, int bdrv_flags)
>> +{
>> +    BlockDriver *drv = bs->drv;
>> +    int ret;
>> +
>> +    if (bdrv_flags == bs->open_flags) {
>> +        return 0;
>> +    }
>> +
>> +    /* Quiesce IO for the given block device */
>> +    qemu_aio_flush();
>> +    bdrv_flush(bs);
>> +
>> +    bdrv_close(bs);
>> +    ret = bdrv_open(bs, bs->filename, bdrv_flags, drv);
>> +
>> +    /*
>> +     * A failed attempt to reopen the image file must lead to 'abort()'
>> +     */
>> +    if (ret != 0) {
>> +        abort();
>> +    }
>> +
>> +    return ret;
>> +}
> 
> 
> 
>> +
>> +int bdrv_change_cache(BlockDriverState *bs, bool enable)
>> +{
>> +    int bdrv_flags = 0;
>> +
>> +    bdrv_flags = bs->open_flags & ~BDRV_O_CACHE_WB;
>> +    if (enable) {
>> +        bdrv_flags |= BDRV_O_CACHE_WB;
>> +    }
>> +
>> +    return bdrv_reopen(bs, bdrv_flags);
>> +}
> 
> Is there any way we can manage todo this *without* closing and
> re-opening the file descriptor ?
> 
> One of the things we're considering for the future is to enable
> QEMU to be passed open FD(s) for its drives, from the management
> system, instead of having QEMU open the files itself. 

What about cache mode, read-only flags etc. that are set with the right
flags during the open? Must qemu just assume that the management has
done the right thing?

And what about things like backing files or other information that
depends on the content of the image file?

> If QEMU expects to close+reopen any of its disks at any time the
> guest requests, this will complicate life somewhat

It does expect this. For example for making backing files temporarily
read-write during a 'commit' monitor command, we already reopen the
image. (Let's hope nobody uses -snapshot, live snapshots and commit, he
would lose his disk...)

Kevin