Re: [Qemu-devel] [PATCH v15 11/21] iotests: 085: Avoid image locking conflict

[Fam Zheng]

On Wed, 04/26 14:30, Kevin Wolf wrote:
> Am 26.04.2017 um 05:34 hat Fam Zheng geschrieben:
> > In the case where we test the expected error when a blockdev-snapshot
> > target already has a backing image, the backing chain is opened multiple
> > times. This will be a problem when we use image locking, so use a
> > different backing file that is not already open.
> > 
> > Signed-off-by: Fam Zheng <address@hidden>
> > ---
> >  tests/qemu-iotests/085     | 34 ++++++++++++++++++++--------------
> >  tests/qemu-iotests/085.out |  3 ++-
> >  2 files changed, 22 insertions(+), 15 deletions(-)
> > 
> > diff --git a/tests/qemu-iotests/085 b/tests/qemu-iotests/085
> > index c53e97f..cc6efd8 100755
> > --- a/tests/qemu-iotests/085
> > +++ b/tests/qemu-iotests/085
> > @@ -45,7 +45,7 @@ _cleanup()
> >          rm -f "${TEST_DIR}/${i}-${snapshot_virt0}"
> >          rm -f "${TEST_DIR}/${i}-${snapshot_virt1}"
> >      done
> > -    rm -f "${TEST_IMG}.1" "${TEST_IMG}.2"
> > +    rm -f "${TEST_IMG}" "${TEST_IMG}.1" "${TEST_IMG}.2" "${TEST_IMG}.base"
> >  
> >  }
> >  trap "_cleanup; exit \$status" 0 1 2 3 15
> > @@ -87,24 +87,26 @@ function create_group_snapshot()
> >  }
> >  
> >  # ${1}: unique identifier for the snapshot filename
> > -# ${2}: true: open backing images; false: don't open them (default)
> > +# ${2}: extra_params to the blockdev-add command
> > +# ${3}: filename
> > +function do_blockdev_add()
> > +{
> > +    cmd="{ 'execute': 'blockdev-add', 'arguments':
> > +           { 'driver': 'qcow2', 'node-name': 'snap_${1}', ${2}
> > +             'file':
> > +             { 'driver': 'file', 'filename': '${3}',
> > +               'node-name': 'file_${1}' } } }"
> > +    _send_qemu_cmd $h "${cmd}" "return"
> > +}
> > +
> > +# ${1}: unique identifier for the snapshot filename
> >  function add_snapshot_image()
> >  {
> > -    if [ "${2}" = "true" ]; then
> > -        extra_params=""
> > -    else
> > -        extra_params="'backing': '', "
> > -    fi
> >      base_image="${TEST_DIR}/$((${1}-1))-${snapshot_virt0}"
> >      snapshot_file="${TEST_DIR}/${1}-${snapshot_virt0}"
> >      _make_test_img -b "${base_image}" "$size"
> >      mv "${TEST_IMG}" "${snapshot_file}"
> > -    cmd="{ 'execute': 'blockdev-add', 'arguments':
> > -           { 'driver': 'qcow2', 'node-name': 'snap_${1}', ${extra_params}
> > -             'file':
> > -             { 'driver': 'file', 'filename': '${snapshot_file}',
> > -               'node-name': 'file_${1}' } } }"
> > -    _send_qemu_cmd $h "${cmd}" "return"
> > +    do_blockdev_add "$1" "'backing': '', " "${snapshot_file}"
> >  }
> >  
> >  # ${1}: unique identifier for the snapshot filename
> > @@ -222,7 +224,11 @@ echo === Invalid command - snapshot node has a backing 
> > image ===
> >  echo
> >  
> >  SNAPSHOTS=$((${SNAPSHOTS}+1))
> > -add_snapshot_image ${SNAPSHOTS} true
> > +
> > +_make_test_img "$size"
> > +mv "${TEST_IMG}" "${TEST_IMG}.base"
> 
> The common idiom is:
> 
>     TEST_IMG="$TEST_IMG.base" _make_test_img "$size"
> 
> This avoids using mv, which is helpful if we ever want to extend the
> testcase for non-file protocols.

Yes. Cleaning up "mv" usages in all iotests should be done, once again.

I can change this one in this patch, though.

Fam