[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v5 1/9] nbd: Create struct for tracking export i
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-block] [PATCH v5 1/9] nbd: Create struct for tracking export info |
Date: |
Thu, 13 Jul 2017 16:56:54 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 |
On 13/07/2017 16:35, Eric Blake wrote:
> On 07/13/2017 07:31 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 13.07.2017 15:09, Vladimir Sementsov-Ogievskiy wrote:
>>> 07.07.2017 23:30, Eric Blake wrote:
>>>> The NBD Protocol is introducing some additional information
>>>> about exports, such as minimum request size and alignment, as
>>>> well as an advertised maximum request size. It will be easier
>>>> to feed this information back to the block layer if we gather
>>>> all the information into a struct, rather than adding yet more
>>>> pointer parameters during negotiation.
>>>
>>> // it was me who do so)
>>>
>
>>>> +int nbd_init(int fd, QIOChannelSocket *sioc, NBDExportInfo *info,
>>>> Error **errp)
>>>> {
>>>> - unsigned long sectors = size / BDRV_SECTOR_SIZE;
>>>> - if (size / BDRV_SECTOR_SIZE != sectors) {
>>>> - error_setg(errp, "Export size %lld too large for 32-bit
>>>> kernel",
>>>> - (long long) size);
>>>> + unsigned long sectors = info->size / BDRV_SECTOR_SIZE;
>>>> + if (info->size / BDRV_SECTOR_SIZE != sectors) {
>>>> + error_setg(errp, "Export size %" PRId64 " too large for
>>>> 32-bit kernel",
>>>
>>> should be PRIu64
>
> Even with an unsigned size, we can't support more than the maximum off_t
> (2^63-1) rather than the full uint64_t range (2^64-1). Any positive
> size prints the same, and if any caller is passing in a negative size,
> things are already weird. But you are correct that matching unsigned to
> PRIu64 is nicer, even if we already make blatant assumptions that all
> platforms that qemu runs on can interchange signedness in printf without
> repercussion.
>
>>>
>>>> + info->size);
>>>> return -E2BIG;
>>>> }
>>>>
>>> [...]
>>>
>>
>> with fixed:
>>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>>
>> (if it is not too late ;)
>
> Depends on whether Paolo wants to send a v2 pull request omitting the
> NBD stuff (in which case I'll submit a separate NBD pull request), or
> whether we just let the pull request happen (in which case I'll submit
> followup patches to address your reviews). It's slightly cleaner to get
> it right from the get-go, but followups are better than nothing, so I'm
> not too fussed either way.
I've fixed the PRIu64 already when applying (and also the tracepoint in
patch 2).
Paolo
signature.asc
Description: OpenPGP digital signature
[Qemu-block] [PATCH v5 2/9] nbd: Don't bother tracing an NBD_OPT_ABORT response failure, Eric Blake, 2017/07/07
[Qemu-block] [PATCH v5 4/9] nbd: Simplify trace of client flags in negotiation, Eric Blake, 2017/07/07
[Qemu-block] [PATCH v5 3/9] nbd: Expose and debug more NBD constants, Eric Blake, 2017/07/07
[Qemu-block] [PATCH v5 5/9] nbd: Refactor reply to NBD_OPT_EXPORT_NAME, Eric Blake, 2017/07/07