Re: [Qemu-block] [PATCH 2/4] nbd: Limit nbdflags to 16 bits

[Eric Blake]

On 08/01/2016 03:17 AM, Paolo Bonzini wrote:
> 
> 
> On 21/07/2016 21:34, Eric Blake wrote:
>> Furthermore, upstream NBD has never passed the global flags to
>> the kernel via ioctl(NBD_SET_FLAGS) (the ioctl was first
>> introduced in NBD 2.9.22; then a latent bug in NBD 3.1 actually
>> tried to OR the global flags with the transmission flags, with
>> the disaster that the addition of NBD_FLAG_NO_ZEROES in 3.9
>> caused all earlier NBD 3.x clients to treat every export as
>> read-only; NBD 3.10 and later intentionally clip things to 16
>> bits to pass only transmission flags).  Qemu should follow suit,
>> since the current two global flags (NBD_FLAG_FIXED_NEWSTYLE
>> and NBD_FLAG_NO_ZEROES) have no impact on the kernel's behavior
>> during transmission.
> 
> Should squash this in too:
> 
> diff --git a/nbd/server.c b/nbd/server.c
> index 80fbb4d..6fa2f9c 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -575,7 +575,7 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData 
> *data)
>  
>      oldStyle = client->exp != NULL && !client->tlscreds;
>      if (oldStyle) {
> -        TRACE("advertising size %" PRIu64 " and flags %x",
> +        TRACE("advertising size %" PRIu64 " and flags %" PRIx16,
>                client->exp->size, client->exp->nbdflags | myflags);

No, we shouldn't. Last time I tried that, we tickled a clang bug where
%hx gripes when presented an 'int' argument, in spite of the int
argument being computed as 'short | short'. See commit 2cb34749, and
your discussion leading up to it:
https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg04663.html

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

signature.asc
Description: OpenPGP digital signature