| Message ID | 20181116155325.22428-4-berrange@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Misc fixes to NBD | expand |
On 11/16/18 9:53 AM, Daniel P. Berrangé wrote: > The helpers for starting/stopping qemu-nbd in 058 will be useful in > other test cases, so move them into a common.nbd file. In fact, I already have a proposed patch 228 that I'm trying to clean up for potential inclusion in 3.1 that could use this! [I'm still hammering on those patches, so I don't know if they will make 3.1 or be delayed to 4.0 - this patch is not needed in 3.1 unless either your later patches are 3.1 material or my patch ends up ready in time] https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg00308.html > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > tests/qemu-iotests/058 | 47 +++++------------------------ > tests/qemu-iotests/common.nbd | 56 +++++++++++++++++++++++++++++++++++ > 2 files changed, 64 insertions(+), 39 deletions(-) > create mode 100644 tests/qemu-iotests/common.nbd > Reviewed-by: Eric Blake <eblake@redhat.com>
On 11/16/18 9:53 AM, Daniel P. Berrangé wrote: > The helpers for starting/stopping qemu-nbd in 058 will be useful in > other test cases, so move them into a common.nbd file. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > tests/qemu-iotests/058 | 47 +++++------------------------ > tests/qemu-iotests/common.nbd | 56 +++++++++++++++++++++++++++++++++++ > 2 files changed, 64 insertions(+), 39 deletions(-) > create mode 100644 tests/qemu-iotests/common.nbd > > - > -_cleanup_nbd() > -{ > -_wait_for_nbd() > -{ > +++ b/tests/qemu-iotests/common.nbd > @@ -0,0 +1,56 @@ > +#!/bin/bash I know we're using bash, > + > +function nbd_server_stop() > +{ > +function nbd_server_wait_for_unix_socket() and bash supports 'function', but it is an obsolete syntactic sugar thing that I don't recommend using. (In ksh, it actually makes a difference in behavior whether you use 'function' or not, and using it in 'bash' makes it harder to port code over to 'ksh' - and hence in bash it is obsolete because here it does NOT cause the change in behavior that ksh users expect)
On 11/16/18 3:41 PM, Eric Blake wrote: >> +#!/bin/bash > > I know we're using bash, > >> + >> +function nbd_server_stop() >> +{ > >> +function nbd_server_wait_for_unix_socket() > > and bash supports 'function', but it is an obsolete syntactic sugar > thing that I don't recommend using. (In ksh, it actually makes a > difference in behavior whether you use 'function' or not, and using it > in 'bash' makes it harder to port code over to 'ksh' - and hence in bash > it is obsolete because here it does NOT cause the change in behavior > that ksh users expect) > Of course, I hit send too soon, before getting to my punchline: Since we already have so many existing iotests that use 'function', it's better to clean that up as a separate patch.
On 11/16/18 9:53 AM, Daniel P. Berrangé wrote: > The helpers for starting/stopping qemu-nbd in 058 will be useful in > other test cases, so move them into a common.nbd file. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > +function nbd_server_start_unix_socket() > +{ > + nbd_server_stop > + $QEMU_NBD -v -t -k "$nbd_unix_socket" $@ & Needs to be "$@" to properly preserve whitespace and/or empty arguments (the latter if someone passes -x '' for a default-named export).
On Fri, Nov 16, 2018 at 03:43:16PM -0600, Eric Blake wrote: > On 11/16/18 3:41 PM, Eric Blake wrote: > > > > +#!/bin/bash > > > > I know we're using bash, > > > > > + > > > +function nbd_server_stop() > > > +{ > > > > > +function nbd_server_wait_for_unix_socket() > > > > and bash supports 'function', but it is an obsolete syntactic sugar > > thing that I don't recommend using. (In ksh, it actually makes a > > difference in behavior whether you use 'function' or not, and using it > > in 'bash' makes it harder to port code over to 'ksh' - and hence in bash > > it is obsolete because here it does NOT cause the change in behavior > > that ksh users expect) > > > > Of course, I hit send too soon, before getting to my punchline: > > Since we already have so many existing iotests that use 'function', it's > better to clean that up as a separate patch. Yeah, I actually thought 'function' was the preferred syntax and omitting it was bad since so many iotests used it :-) Regards, Daniel
On Sat, Nov 17, 2018 at 09:01:57PM -0600, Eric Blake wrote: > On 11/16/18 9:53 AM, Daniel P. Berrangé wrote: > > The helpers for starting/stopping qemu-nbd in 058 will be useful in > > other test cases, so move them into a common.nbd file. > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > --- > > > +function nbd_server_start_unix_socket() > > +{ > > + nbd_server_stop > > + $QEMU_NBD -v -t -k "$nbd_unix_socket" $@ & > > Needs to be "$@" to properly preserve whitespace and/or empty arguments (the > latter if someone passes -x '' for a default-named export). ok Regards, Daniel
diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058 index 5eb8784669..cd73250c48 100755 --- a/tests/qemu-iotests/058 +++ b/tests/qemu-iotests/058 @@ -29,55 +29,19 @@ echo "QA output created by $seq" here=`pwd` status=1 # failure is the default! -nbd_unix_socket=$TEST_DIR/test_qemu_nbd_socket -nbd_snapshot_img="nbd:unix:$nbd_unix_socket" -rm -f "${TEST_DIR}/qemu-nbd.pid" - -_cleanup_nbd() -{ - local NBD_SNAPSHOT_PID - if [ -f "${TEST_DIR}/qemu-nbd.pid" ]; then - read NBD_SNAPSHOT_PID < "${TEST_DIR}/qemu-nbd.pid" - rm -f "${TEST_DIR}/qemu-nbd.pid" - if [ -n "$NBD_SNAPSHOT_PID" ]; then - kill "$NBD_SNAPSHOT_PID" - fi - fi - rm -f "$nbd_unix_socket" -} - -_wait_for_nbd() -{ - for ((i = 0; i < 300; i++)) - do - if [ -r "$nbd_unix_socket" ]; then - return - fi - sleep 0.1 - done - echo "Failed in check of unix socket created by qemu-nbd" - exit 1 -} - -converted_image=$TEST_IMG.converted - _export_nbd_snapshot() { - _cleanup_nbd - $QEMU_NBD -v -t -k "$nbd_unix_socket" "$TEST_IMG" -l $1 & - _wait_for_nbd + nbd_server_start_unix_socket "$TEST_IMG" -l $1 } _export_nbd_snapshot1() { - _cleanup_nbd - $QEMU_NBD -v -t -k "$nbd_unix_socket" "$TEST_IMG" -l snapshot.name=$1 & - _wait_for_nbd + nbd_server_start_unix_socket "$TEST_IMG" -l snapshot.name=$1 } _cleanup() { - _cleanup_nbd + nbd_server_stop _cleanup_test_img rm -f "$converted_image" } @@ -87,6 +51,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 . ./common.rc . ./common.filter . ./common.pattern +. ./common.nbd _supported_fmt qcow2 _supported_proto file @@ -95,6 +60,10 @@ _require_command QEMU_NBD # Internal snapshots are (currently) impossible with refcount_bits=1 _unsupported_imgopts 'refcount_bits=1[^0-9]' +nbd_snapshot_img="nbd:unix:$nbd_unix_socket" + +converted_image=$TEST_IMG.converted + # Use -f raw instead of -f $IMGFMT for the NBD connection QEMU_IO_NBD="$QEMU_IO -f raw --cache=$CACHEMODE" diff --git a/tests/qemu-iotests/common.nbd b/tests/qemu-iotests/common.nbd new file mode 100644 index 0000000000..f920a578f1 --- /dev/null +++ b/tests/qemu-iotests/common.nbd @@ -0,0 +1,56 @@ +#!/bin/bash +# -*- shell-script-mode -*- +# +# Helpers for NBD server related config +# +# Copyright (C) 2018 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + +nbd_unix_socket="${TEST_DIR}/qemu-nbd.sock" +nbd_pid_file="${TEST_DIR}/qemu-nbd.pid" + +function nbd_server_stop() +{ + local NBD_PID + if [ -f "$nbd_pid_file" ]; then + read NBD_PID < "$nbd_pid_file" + rm -f "$nbd_pid_file" + if [ -n "$NBD_PID" ]; then + kill "$NBD_PID" + fi + fi + rm -f "$nbd_unix_socket" +} + +function nbd_server_wait_for_unix_socket() +{ + for ((i = 0; i < 300; i++)) + do + if [ -r "$nbd_unix_socket" ]; then + return + fi + sleep 0.1 + done + echo "Failed in check of unix socket created by qemu-nbd" + exit 1 +} + +function nbd_server_start_unix_socket() +{ + nbd_server_stop + $QEMU_NBD -v -t -k "$nbd_unix_socket" $@ & + nbd_server_wait_for_unix_socket +}
The helpers for starting/stopping qemu-nbd in 058 will be useful in other test cases, so move them into a common.nbd file. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/qemu-iotests/058 | 47 +++++------------------------ tests/qemu-iotests/common.nbd | 56 +++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 39 deletions(-) create mode 100644 tests/qemu-iotests/common.nbd