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 <vsement...@virtuozzo.com>
>>
>> (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
