Re: [Qemu-devel] [Qemu-block] [PATCH v5 2/3] file-posix: Drop s->lock_fd

[Max Reitz]

On 14.11.18 14:54, Alberto Garcia wrote:
> On Thu 11 Oct 2018 09:21:34 AM CEST, Fam Zheng wrote:
>> The lock_fd field is not strictly necessary because transferring locked
>> bytes from old fd to the new one shouldn't fail anyway. This spares the
>> user one fd per image.
>>
>> Signed-off-by: Fam Zheng <address@hidden>
>> Reviewed-by: Max Reitz <address@hidden>
> 
> One of my tests (not published yet) starts to fail after this
> patch. Here's how you can reproduce the error:
> 
> $ qemu-img create -f qcow2 hd.qcow2 4G
> $ qemu-system-x86_64 -qmp stdio
> 
> { "execute": "qmp_capabilities" }
> { "execute": "blockdev-add", "arguments": {"driver": "qcow2", "node-name": 
> "hd0", "file": {"driver": "file", "filename": "hd.qcow2", "locking": "on" }}}
> { "execute": "human-monitor-command", "arguments": {"command-line": "qemu-io 
> hd0 \"reopen -o file.locking=on\""}}
> { "execute": "human-monitor-command", "arguments": {"command-line": "qemu-io 
> hd0 \"reopen -o file.locking=off\""}}
> { "execute": "blockdev-del", "arguments": {"node-name": "hd0"}}
> { "execute": "blockdev-add", "arguments": {"driver": "qcow2", "node-name": 
> "hd0", "file": {"driver": "file", "filename": "hd.qcow2"}}}
> 
> {"error": {"class": "GenericError", "desc": "Failed to get \"consistent 
> read\" lock"}}

To me that looks like a problem in the general reopen code.
raw_reopen_prepare() is called and succeeds.  Then bdrv_reopen_prepare()
notices the option wasn't handled and therefore fails.
bdrv_reopen_multiple() thus doesn't set bs_entry->prepared to true,
which means raw_reopen_abort() won't be called.

We should always call either BlockDriver.bdrv_reopen_commit() or
BlockDriver.bdrv_reopen_abort() when BlockDriver.bdrv_reopen_prepare()
succeeded.

Max

signature.asc
Description: OpenPGP digital signature