Re: [Nbd] [PATCH v2 5/6] nbd: Test recent bug fixes

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

On Wed, Oct 19, 2016 at 03:59:21PM -0500, Eric Blake wrote:
> On 10/18/2016 06:07 AM, Wouter Verhelst wrote:
> > On Mon, Oct 17, 2016 at 03:23:39PM -0500, Eric Blake wrote:
> >> Add a test of intentionally provoking a server error during option
> >> negotiation to prove that a client can still fall back to known
> >> options; thus covering two recent bug fixes for a server sending
> >> the wrong length in an error reply, and for a server not reading
> >> enough data when replying to an unknown command.
> > 
> > While this is a good idea, it might be better to do this conditionally,
> > and add another test to TESTS which triggers it. That way, if we break
> > it again in the future, the reason why will be more obvious.
> 
> Trickier said than done.  There is no 'testflags' parameter passed all
> the way into setup_connection_common(), so it would require quite a bit
> of additional framework to decide whether to do normal connection
> (return a socket ready for further reads) or a short connection (return
> a socket still in the middle of negotiation so we can test further
> options).  But I'll play with the idea.

Change the CONECTION_TYPE enum, adding a CONNECTION_TYPE_WRONGOPT (or some
such) to it? That enum was meant as that "framework" ;-)

> > (we can then, after successfully negotiating, send NBD_OPT_ABORT and not
> > do anything...)
> 
> That part makes sense - sticking the handshake negotiation testing in a
> dedicated test, where there is no subsequent I/O, is indeed a nice
> separation of tasks when trying to debug any test failures.

Exactly :-)

-- 
< 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