Re: [Nbd] Testing NBD server implementations for correctness

[Wouter Verhelst <w@...112...>]

Hi,

On Mon, Sep 26, 2016 at 03:22:52AM +0200, Carl-Daniel Hailfinger wrote:
> On 25.09.2016 13:24, Wouter Verhelst wrote:
> > On Sun, Sep 25, 2016 at 11:43:39AM +0100, Alex Bligh wrote:
> >>> On 25 Sep 2016, at 04:56, Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@...2724...> wrote:
> >>>
> >>> I am currently developing some experimental NBD protocol extensions to
> >>> change the characteristics of the backing store. Now I want to test
> >>> whether my implementation actually survives a stress test.
> >>> Which applications can test a NBD server thoroughly? So far, I have used
> >>> nbd-verify 0.4 and it did uncover one bug. However, there are probably
> >>> other testing programs as well. I'm especially looking for tests of
> >>> unaligned read/write/trim.
> >>>
> >>> Any hints would be appreciated.
> >> Have you tried nbd-tester-client (in nbd/tests/run)? In particular
> >> the integrity test stuff, which does a pretty thorough stress test.
> > You may have to modify the "simple_test" script in the "tests/run"
> > directory to run your nbd server rather than the reference
> > implementation, if you're working on a separate implementation.
> 
> Thanks. I'm extending nbdkit because prototyping in python helps quite a
> bit.

Sure.

> Running nbd-tester-client against nbdkit with oldstyle negotiation was fun.
> I managed to segfault nbdkit and noticed that nbd-tester-client speaks
> the oldstyle protocol incorrectly, ignoring flags sent by the server.
> Patch follows.

Whoops.

Honestly though, the oldstyle protocol shouldn't be used anymore, but
yeah, I know there are still implementations out in the wild that do
(and there isn't much I can do to fix that, other than discouraging). In
that light, I guess it does make sense to fix bugs in the test client,
even if we won't be using it ourselves.

> diff -r e691aad45ed2 tests/run/nbd-tester-client.c
> --- a/tests/run/nbd-tester-client.c	Thu Sep 15 13:25:47 2016 +0100
> +++ b/tests/run/nbd-tester-client.c	Mon Sep 26 03:18:26 2016 +0200
> @@ -384,7 +384,12 @@
>  		READ_ALL_ERRCHK(sock, &size, sizeof(size), err,
>  				"Could not read size: %s", strerror(errno));
>  		size = ntohll(size);
> -		READ_ALL_ERRCHK(sock, buf, 128, err, "Could not read data: %s",
> +		uint32_t flags;
> +		READ_ALL_ERRCHK(sock, &flags, sizeof(uint32_t), err,
> +				"Could not read flags: %s", strerror(errno));
> +		flags = ntohl(flags);
> +		*serverflags = flags;
> +		READ_ALL_ERRCHK(sock, buf, 124, err, "Could not read data: %s",
>  				strerror(errno));
>  		goto end;
>  	}
> 
> 
> Without this patch, oldstyle connections of nbd-tester-client to any
> server will cause the message "Server did not supply flush capability
> flags" if flush is requested by nbd-tester-client, even if it is
> supported by the server.

Thanks, applied.

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12