Re: [Qemu-block] [PATCH v5 24/27] iotests: Disable image locking in 085

[Fam Zheng]

On Wed, 05/25 15:52, Max Reitz wrote:
> On 17.05.2016 09:35, Fam Zheng wrote:
> > The cases is about live snapshot features. Disable image locking because
> > otherwise a few tests are going to fail because we reuse the same images
> > at blockdev-add.
> > 
> > Signed-off-by: Fam Zheng <address@hidden>
> > ---
> >  tests/qemu-iotests/085 | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085
> > index aa77eca..48f6684 100755
> > --- a/tests/qemu-iotests/085
> > +++ b/tests/qemu-iotests/085
> > @@ -102,6 +102,7 @@ function add_snapshot_image()
> >      cmd="{ 'execute': 'blockdev-add', 'arguments':
> >             { 'options':
> >               { 'driver': 'qcow2', 'node-name': 'snap_${1}', ${extra_params}
> > +               'lock-mode': 'off',
> >                 'file':
> >                 { 'driver': 'file', 'filename': '${snapshot_file}',
> >                   'node-name': 'file_${1}' } } } }"
> > @@ -130,7 +131,7 @@ echo === Running QEMU ===
> >  echo
> >  
> >  qemu_comm_method="qmp"
> > -_launch_qemu -drive file="${TEST_IMG}.1",if=virtio -drive 
> > file="${TEST_IMG}.2",if=virtio
> > +_launch_qemu -drive file="${TEST_IMG}.1",if=virtio,lock-mode=off -drive 
> > file="${TEST_IMG}.2",if=virtio,lock-mode=off
> >  h=$QEMU_HANDLE
> >  
> >  echo
> > 
> 
> So as far as I understand it, add_snapshot_image() is supposed to add
> images from the backing chain to the running VM. The top image is never
> used by add_snapshot_image(), thus the lock-mode=off in the QEMU command
> line seems superfluous.

But down the backing chain is 10-snapshot-v0.qcow2, created in
create_single_snapshot (or create_group_snapshot?). Without lock-mode=off in
the command line, the shared lock cannot work.

> 
> Since the backing chain is opened read-only by the VM, it is locked in
> shared mode, basically. Therefore, we can simply use explicitly shared
> lock mode in add_snapshot_image(); or, alternatively, it is completely
> sufficient to specify "'read-only': true" there instead of forcing a
> non-exclusive locking mode.
> 
> And indeed, for me the test passes if I undo the changes done by this
> patch and just insert said "'read-only': true" in the blockdev-add
> invocation done by add_snapshot_image().

For above reason, this doesn't work for me. Am I missing anything?

Fam