[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH nbdkit v2 3/3] tests: Add a simple test of nbdkit_export_name.



On 9/12/19 4:01 PM, Richard W.M. Jones wrote:
> ---
>  tests/Makefile.am         |  3 ++
>  tests/test-export-name.sh | 86 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 89 insertions(+)

> +# Create a sh plugin which echos the export name back in the device.

echoes? One of those weird words with dual acceptable spellings, where
it depends on who you ask for which variant is preferred (we already
fixed the code base to prefer zeroes over zeros, though).

> +start_nbdkit -P exportname.pid -U $sock \
> +             sh - <<'EOF'
> +case "$1" in
> +    open)
> +        h=`mktemp $tmpdir/disk-XXXXXX`
> +        echo -n "$3" > $h

I prefer printf over echo -n (we require bash which means we are more
portable than a random shell where POSIX says -n is undefined; but you
can still make bash misbehave with shopt -s xpg_echo)

printf %s "%3" > $h

> +        echo $h
> +        ;;

Hmm - instead of making the handle be the name of a temporary file that
contains the export name, you could just make the handle _be_ the export
name:

open)
  printf %s "$3"
  ;;

> +    get_size)
> +        stat -c '%s' "$2"
> +        ;;

at which point, you can simplify to:

get_size)
  echo ${#2}
  ;;

> +    pread)
> +        dd if="$2" skip=$4 count=$3 iflag=skip_bytes,count_bytes
> +        ;;

and this would be something like:

pread)
  echo "$2" | dd skip=$4 count=$3 iflag=skip_bytes,count_bytes
  ;;

or bypass dd by exploiting your knowledge of the client:
pread)
  if test $4.$3 = "0.${#2}"; then
    printf %s "$2"
  else
    echo "EINVAL unexpected alignment" >&2
  fi
  ;;

With those changes, we cut down on the number of fork()s and filesystem
modifications :)

> +    *) exit 2 ;;
> +esac
> +EOF
> +
> +# Try to read back various export names from the plugin.
> +for e in "" "test" "/" "//" " " \
> +         "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
> +do
> +    export e sock
> +    nbdsh -c '
> +import os
> +
> +e = os.environ["e"]
> +h.set_export_name (e)
> +h.connect_unix (os.environ["sock"])

Could be done with nbdsh --connect "nbd+unix://$e?socket=$sock".  But
what you did here works as well (--uri would work too once you could
rely on nbdsh 1.2, but for now sticking to --connect is more portable)

> +
> +size = h.get_size ()
> +print ("size=%r e=%r" % (size, e))

Cute test.  Up to you what you want to fold in from my suggestions, but
I think the series is ready.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]