[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [Nbd] [PATCH] Fix flags processing to avoid bogus read-only export



On 05/04/2016 04:45 AM, Alex Bligh wrote:
> As described here:
>   https://www.mail-archive.com/nbd-general@lists.sourceforge.net/msg03878.html
> 
> NBD clients prior to 3.10 incorrectly attempt to merge the server flags
> with the transmission flags. This means that any such client cannot connect
> to a server (e.g. NBD server post 3.10, gonbdserver) which supports
> NBD_FLAG_NO_ZEROES without making the export read-only.
> 
> This one line fix avoids the issue by simply passing the kernel the
> transmission flags without an attempt to merge them. This is the
> behaviour nbd-client (post 3.10) does, and also qemu's current behaviour.

Not completely qemu's current behavior (qemu-nbd 2.6 calls into the
kernel with the server's advertised bits shifted by 16, rather than
eliminated), but slated to be added to qemu 2.7 several months down the
road.  But not worth rewriting the commit message for that.

> 
> Signed-off-by: Alex Bligh <alex@...872...>
> ---
>  nbd-client.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@...696...>

> 
> diff --git a/nbd-client.c b/nbd-client.c
> index cc9a06e..ddc849c 100644
> --- a/nbd-client.c
> +++ b/nbd-client.c
> @@ -318,7 +318,7 @@ void negotiate(int sock, u64 *rsize64, u32 *flags, char* name, uint32_t needed_f
>  	} else {
>  		if(read(sock, &tmp, sizeof(tmp)) < 0)
>  			err("Failed/4: %m\n");
> -		*flags |= (uint32_t)ntohs(tmp);
> +		*flags = (uint32_t)ntohs(tmp);

This is C, and ntohs() is required to give unsigned output, so the cast
to uint32_t is technically unnecessary.  You could drop it while
touching this line, but then the patch is no longer quite as minimal,
and since it is intended for backports only, probably  not worth the
extra churn.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: