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

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

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.

(we can then, after successfully negotiating, send NBD_OPT_ABORT and not
do anything...)

> Signed-off-by: Eric Blake <eblake@...696...>
> ---
>  tests/run/nbd-tester-client.c | 47 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/tests/run/nbd-tester-client.c b/tests/run/nbd-tester-client.c
> index 28db8ee..099bec1 100644
> --- a/tests/run/nbd-tester-client.c
> +++ b/tests/run/nbd-tester-client.c
> @@ -403,6 +403,53 @@ int setup_connection_common(int sock, char *name, CONNECTION_TYPE ctype,
>  	negotiationflags = htonl(negotiationflags);
>  	WRITE_ALL_ERRCHK(sock, &negotiationflags, sizeof(negotiationflags), err,
>  			 "Could not write reserved field: %s", strerror(errno));
> +	if (handshakeflags & NBD_FLAG_FIXED_NEWSTYLE) {
> +		/* Intentionally throw an unknown option at the server */
> +		tmp64 = htonll(opts_magic);
> +		WRITE_ALL_ERRCHK(sock, &tmp64, sizeof(tmp64), err,
> +				 "Could not write magic: %s", strerror(errno));
> +		tmp32 = htonl(0x7654321);
> +		WRITE_ALL_ERRCHK(sock, &tmp32, sizeof(tmp32), err,
> +				 "Could not write option: %s", strerror(errno));
> +		tmp32 = htonl((uint32_t) sizeof(tmp32));
> +		WRITE_ALL_ERRCHK(sock, &tmp32, sizeof(tmp32), err,
> +				 "Could not write option length: %s", strerror(errno));
> +		WRITE_ALL_ERRCHK(sock, &tmp32, sizeof(tmp32), err,
> +				 "Could not write option payload: %s", strerror(errno));
> +		/* Expect proper error from server */
> +		READ_ALL_ERRCHK(sock, &tmp64, sizeof(tmp64), err,
> +				"Could not read magic: %s", strerror(errno));
> +		tmp64 = ntohll(tmp64);
> +		if (tmp64 != 0x3e889045565a9LL) {
> +			strncpy(errstr, "magic does not match", errstr_len);
> +			goto err;
> +		}
> +		READ_ALL_ERRCHK(sock, &tmp32, sizeof(tmp32), err,
> +				"Could not read option: %s", strerror(errno));
> +		tmp32 = ntohl(tmp32);
> +		if (tmp32 != 0x7654321) {
> +			strncpy(errstr, "option does not match", errstr_len);
> +			goto err;
> +		}
> +		READ_ALL_ERRCHK(sock, &tmp32, sizeof(tmp32), err,
> +				"Could not read status: %s", strerror(errno));
> +		tmp32 = ntohl(tmp32);
> +		if (tmp32 != NBD_REP_ERR_UNSUP) {
> +			strncpy(errstr, "status does not match", errstr_len);
> +			goto err;
> +		}
> +		READ_ALL_ERRCHK(sock, &tmp32, sizeof(tmp32), err,
> +				"Could not read length: %s", strerror(errno));
> +		tmp32 = ntohl(tmp32);
> +		while (tmp32) {
> +			char buf[1024];
> +			size_t len = tmp32 < sizeof(buf) ? tmp32 : sizeof(buf);
> +			READ_ALL_ERRCHK(sock, buf, len, err,
> +					"Could not read payload: %s", strerror(errno));
> +			tmp32 -= len;
> +		}
> +	}
> +	/* Now for the real connection */
>  	/* magic */
>  	tmp64 = htonll(opts_magic);
>  	WRITE_ALL_ERRCHK(sock, &tmp64, sizeof(tmp64), err,
> -- 
> 2.7.4
> 
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most 
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> Nbd-general mailing list
> Nbd-general@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nbd-general
> 

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