[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 2/4] nbd: Limit nbdflags to 16 bits
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-block] [PATCH 2/4] nbd: Limit nbdflags to 16 bits |
Date: |
Mon, 1 Aug 2016 13:50:07 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 |
On 01/08/2016 13:43, Eric Blake wrote:
> 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
Uff, you're right. :( I remembered the discussion, but not the outcome.
Paolo