Hi, On 26.09.2016 09:53, Wouter Verhelst wrote: > On Mon, Sep 26, 2016 at 03:22:52AM +0200, Carl-Daniel Hailfinger wrote: >> Running nbd-tester-client against nbdkit with oldstyle negotiation was fun. >> I managed to segfault nbdkit
Side note: I'm going to try and get a backtrace from the nbdkit segfault and submit a bug report there. >> 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). Well, nbdkit supports both, and the protocol style has to be specified on the command line. I just happened to leave out the switch telling nbdkit to use newstyle protocol. Admittedly nbdkit also has some implementation bugs in oldstyle negotiation, but they cancel each other out internally. Lots of documentation out there does not mention that the oldstyle protocol should not be used, and I fear most of that documentation hasn't been touched in years, so changing it would be impractical. Maybe it would make sense to have nbdkit print some diagnostics that it's using the (default) oldstyle protocol? > 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. Thanks. >> 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. Wow, that was quick! Thank you. I stumbled upon another problem: Apparently nbd-tester-client and nbdkit disagree on what constitutes a valid flush request. nbd-tester-client complains: ./flush 15901: Requests: 3536 ** (process:15901): WARNING **: Could not run test: Received error from server: 22 (0x16). Handle is -9223372036854764544 (0x8000000000002C00). nbdkit complains: nbdkit: python[7]: error: invalid flush request: expecting offset and length == 0 Not sure where that bug is, nbdkit or nbd-tester-client. - Is the request correct and should have been accepted? - Is the reject correct, but the response is screwed up? Regards, Carl-Daniel ------------------------------------------------------------------------------ _______________________________________________ Nbd-general mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/nbd-general
