Re: [PATCH v2 2/2] nbd: Add testsuite coverage

[Jose E. Marchesi]

    > Hi Eric.
    >
    >      +Using NBD connections in tests
    >      +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    >      +
    >      +If your test requires an NBD server (only useful when poke is 
compiled
    >      +with libnbd), the dg-nbd directive is what you need.  It has one 
form::
    >      +
    >      +  /* { dg-nbd foo.data /tmp/pokenbd.[pid]/sock } */
    >      +
    >      +This serves a previously-created temporary file (use dg-data with 
the
    >      +named file form) over the named Unix socket.  The server at this
    >      +socket is provided by nbdkit, and will be shut down gracefully when
    >      +the testsuite completes.
    >      +
    >      +To use the server as an IO space, your test will then follow up 
with::
    >      +
    >      +  /* { dg-command "open 
(\"nbd+unix:///?socket=/tmp/pokenbd.[pid]/sock\")" } */
    >
    > Does NBDKIT == "yes" imply an unix environment?  If not, we may need to
    > add extra logic to skip these tests in non-unix systems (such as mingw
    > or cygwin.)
    
    To my knowledge, no one has tried to port libnbd or nbdkit to Cygwin
    yet (although I suspect that porting to Cygwin will be easy, while
    porting to mingw will require some heavy effort since native Windows
    sockets tend to need wrappers).  Cygwin supports Unix sockets, but
    mingw does not.  So you are correct that this may need tweaks down the
    road (either to support whatever mingw actually can do, or to skip on
    mingw where Unix sockets are not available); but given that libnbd and
    nbdkit are not yet build for either platform (at least, not in my
    quick web search), it shouldn't be a problem for now.

Yeah I agree.
    
    >           +# Create a temporary NBD server for a file previously
    > create with the
    >
    > s/create/created/
    >
    >           # Uncomment the following couple of lines to run the testsuite 
with
    >      -    # valgring.
    >      +    # valgrind.
    >
    > Spurious change?
    
    Typo fix. But I can separate it out and do a general typo fixup patch
    instead if desired.

Argh I must be blind :D
    
    >
    >      diff --git a/testsuite/poke.cmd/nbd.pk b/testsuite/poke.cmd/nbd.pk
    >      new file mode 100644
    >      index 00000000..f7296da9
    >      --- /dev/null
    >      +++ b/testsuite/poke.cmd/nbd.pk
    >      @@ -0,0 +1,8 @@
    >      +/* { dg-do run } */
    >      +/* { dg-data {c*} {0x10 0x20 0x30 0x40 0x50 0x60 0x70 0x80} nbddata 
} */
    >      +/* { dg-nbd nbddata /tmp/pokenbd.[pid]/sock } */
    >      +
    >      +/* { dg-command ".nbd nbd+unix:///?socket=/tmp/pokenbd.[pid]/sock" 
} */
    >      +/* { dg-command {.set obase 16} }  */
    >      +/* { dg-command { byte[8] @ 0#B } } */
    >      +/* { dg-output {[0x10,0x20,0x30,0x40,0x50,0x60,0x70,0x80]} } */
    >
    > Please call the test nbd-1.pk.  This will avoid renames when we add more
    > tests for the .nbd command.
    
    Good idea.  Another reason for me to respin this patch is to follow up
    with my idea of using nbdkit's data server, rather than requiring a
    temporary file.  Also, I noticed that typing '/tmp/pokenbd.[pid]/' is
    long and would only get longer to actually honor $TMPDIR, I may
    implement a helper function [dg-tmpdir] that factors that out.
    
Good idea.

    >
    >      diff --git a/testsuite/poke.pkl/open-3.pk 
b/testsuite/poke.pkl/open-3.pk
    >      new file mode 100644
    >      index 00000000..57b45f2e
    >      --- /dev/null
    >      +++ b/testsuite/poke.pkl/open-3.pk
    >      @@ -0,0 +1,8 @@
    >      +/* { dg-do run } */
    >      +/* { dg-data {c*} {0x10} nbddata } */
    >      +/* { dg-nbd nbddata /tmp/pokenbd.[pid]/open-3 } */
    >      +
    >      +/* { dg-command { .set obase 10 } } */
    >      +/* { dg-command "defvar foo = open
    > (\"nbd+unix:///?socket=/tmp/pokenbd.[pid]/open-3\")" } */
    >      +/* { dg-command { get_ios == foo } } */
    >      +/* { dg-output "1" } */
    >
    > Very nice :)
    > We are gonna also need a few tests mapping stuff (both read and write)
    > in a NBD ios.
    
    I'll add some.  Glad to see the ideas for how to test this have worked
    out so far.  And even though patch 1 is ready, I'll feel a bit more
    comfortable posting the next revision of patch 2 before committing to
    the series (I also want to post patches overhauling the ios-dev.h
    .get_c/.put_c into .pread/.pwrite, and if those land first, it's less
    churn on the ios-dev-nbd.c patch).

Thanks!