[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PULL 2/5] block/nvme: support larger that 512 bytes se
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PULL 2/5] block/nvme: support larger that 512 bytes sector devices |
Date: |
Mon, 29 Jul 2019 15:25:15 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 |
On 29.07.19 15:16, Peter Maydell wrote:
> On Mon, 22 Jul 2019 at 18:26, Max Reitz <address@hidden> wrote:
>>
>> From: Maxim Levitsky <address@hidden>
>>
>> Currently the driver hardcodes the sector size to 512,
>> and doesn't check the underlying device. Fix that.
>>
>> Also fail if underlying nvme device is formatted with metadata
>> as this needs special support.
>>
>> Signed-off-by: Maxim Levitsky <address@hidden>
>> Message-id: address@hidden
>> Signed-off-by: Max Reitz <address@hidden>
>
>> +static int64_t nvme_get_blocksize(BlockDriverState *bs)
>> +{
>> + BDRVNVMeState *s = bs->opaque;
>> + assert(s->blkshift >= BDRV_SECTOR_BITS);
>> + return 1 << s->blkshift;
>> +}
>
> Hi -- Coverity points out here that we calculate the
> "1 << s->blkshift" as a 32-bit shift, but then return an
> int64_t type (CID 1403771).
>
> Can the blkshift ever really be 31 or more ?
>
> The types here seem weird anyway -- we return an int64_t,
> but the only user of this is nvme_probe_blocksizes(),
> which uses the value only to set BlockSizes::phys and ::log,
> both of which are of type "uint32_t". That leads me to think
> that the right return type for the function is uint32_t.
>
> PS: this is the only Coverity issue currently outstanding so
> if it's a trivial fix it might be nice to put it into rc3.
Maxim, what do you think?
How about we let nvme_identify() limit blkshift to something sane and
then return a uint32_t here?
In theory it would be limited by page_size, and that has a maximum value
of 2^27. In practice, though, that limit is checked by another 32-bit
shift...
Max
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PULL 0/5] Block patches for 4.1.0-rc2, Max Reitz, 2019/07/22
- [Qemu-block] [PULL 3/5] block/nvme: don't touch the completion entries, Max Reitz, 2019/07/22
- [Qemu-block] [PULL 4/5] block: Dec. drained_end_counter before bdrv_wakeup, Max Reitz, 2019/07/22
- [Qemu-block] [PULL 5/5] block: Only the main loop can change AioContexts, Max Reitz, 2019/07/22
- Re: [Qemu-block] [PULL 0/5] Block patches for 4.1.0-rc2, Peter Maydell, 2019/07/23