On Thu, Mar 17, 2011 at 3:11 PM, Kevin Wolf <kw...@redhat.com> wrote:
> 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 <pre...@linux.vnet.ibm.com>
>>> with minimal refactoring.
>>>
>>> Signed-off-by: Christoph Hellwig <h...@lst.de>
>>>
>>>
>>> +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...)
I think we need to use the most robust solution possible. We don't
want to get into a situation that can be avoided. Especially in cases
where human errors can (==will) be made.
I suggested using /proc/$pid/fd to reopen an existing file descriptor.
That interface is only available on Linux and possibly some of
{Solaris, *BSD, OSX}. So there needs to be a fallback, but in this
situation it feels right to take advantage of whatever means are
provided by the host OS to make safe transitions between image files.
Stefan
