Re: [Qemu-devel] [PATCH] add reopen to blockdev-transaction

[Kevin Wolf]

Am 02.03.2012 14:25, schrieb Paolo Bonzini:
> Il 02/03/2012 14:00, Kevin Wolf ha scritto:
>> Am 01.03.2012 17:52, schrieb Paolo Bonzini:
>>>>> But you can even keep from your first patch the drive-reopen command and
>>>>> not make it atomic, that shouldn't be a problem.
>>>>
>>>> I'm not sure whether it makes sense for a separate drive-reopen or
>>>> whether to just add this to blockdev-transaction (or even both); I can
>>>> make libvirt use whichever color bikeshed we pick.  There's definitely a
>>>> transaction aspect here
>>>
>>> It's not so much atomicity, it's just safety.  The drive-reopen command
>>> must be implemented in a similar way to bdrv_append; it must not do a
>>> close+reopen in the same way as the existing blockdev-snapshot-sync
>>> command, but that's just that blockdev-snapshot-sync was implemented
>>> poorly.
>>
>> For reopen this is a bit harder because you deal with already opened
>> images and you must never have the same image opened twice at the same time.
> 
> This is only for read-write images, and the backing files are read-only,
> so this shouldn't be a problem, no?

Opening an image read-write that is still open read-only may break the
read-only instance.

You can argue that opening an image read-only while a read-write
instance is open can be tolerated if you flushed the image and made sure
no new requests are coming in. This is what happens with live migration.
It's a case that has given us enough headaches that I would not want to
introduce similar behaviour in more cases.

So in short: Regardless of ro/rw, opening images twice is bad. Just
don't do it.

If anything, a possible solution could look like the bdrv_reopen
proposal which already includes prepare/commit/abort functions in the
block driver.

Kevin