[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] glusterfs: fix max_discard
From: |
Peter Lieven |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] glusterfs: fix max_discard |
Date: |
Tue, 03 Feb 2015 12:47:28 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 |
Am 03.02.2015 um 12:37 schrieb Kevin Wolf:
> Am 03.02.2015 um 12:30 hat Peter Lieven geschrieben:
>> Am 03.02.2015 um 08:31 schrieb Denis V. Lunev:
>>> On 02/02/15 23:46, Denis V. Lunev wrote:
>>>> On 02/02/15 23:40, Peter Lieven wrote:
>>>>> Am 02.02.2015 um 21:09 schrieb Denis V. Lunev:
>>>>>> qemu_gluster_co_discard calculates size to discard as follows
>>>>>> size_t size = nb_sectors * BDRV_SECTOR_SIZE;
>>>>>> ret = glfs_discard_async(s->fd, offset, size,
>>>>>> &gluster_finish_aiocb, acb);
>>>>>>
>>>>>> glfs_discard_async is declared as follows:
>>>>>> int glfs_discard_async (glfs_fd_t *fd, off_t length, size_t lent,
>>>>>> glfs_io_cbk fn, void *data) __THROW
>>>>>> This is problematic on i686 as sizeof(size_t) == 4.
>>>>>>
>>>>>> Set bl_max_discard to SIZE_MAX >> BDRV_SECTOR_BITS to avoid overflow
>>>>>> on i386.
>>>>>>
>>>>>> Signed-off-by: Denis V. Lunev <address@hidden>
>>>>>> CC: Kevin Wolf <address@hidden>
>>>>>> CC: Peter Lieven <address@hidden>
>>>>>> ---
>>>>>> block/gluster.c | 9 +++++++++
>>>>>> 1 file changed, 9 insertions(+)
>>>>>>
>>>>>> diff --git a/block/gluster.c b/block/gluster.c
>>>>>> index 1eb3a8c..8a8c153 100644
>>>>>> --- a/block/gluster.c
>>>>>> +++ b/block/gluster.c
>>>>>> @@ -622,6 +622,11 @@ out:
>>>>>> return ret;
>>>>>> }
>>>>>> +static void qemu_gluster_refresh_limits(BlockDriverState *bs, Error
>>>>>> **errp)
>>>>>> +{
>>>>>> + bs->bl.max_discard = MIN(SIZE_MAX >> BDRV_SECTOR_BITS, INT_MAX);
>>>>>> +}
>>>>>> +
>>>>> Looking at the gluster code bl.max_transfer_length should have the same
>>>>> limit, but thats a different patch.
>>>> ha, the same applies to nbd code too.
>>>>
>>>> I'll do this stuff tomorrow and also I think that some
>>>> audit in other drivers could reveal something interesting.
>>>>
>>>> Den
>>> ok. The situation is well rotten here on i686.
>>>
>>> The problem comes from the fact that QEMUIOVector
>>> and iovec uses size_t as length. All API calls use
>>> this abstraction. Thus all conversion operations
>>> from nr_sectors to size could bang at any moment.
>>>
>>> Putting dirty hands here is problematic from my point
>>> of view. Should we really care about this? 32bit
>>> applications are becoming old good history of IT...
>> The host has to be 32bit to be in trouble. And at least if we have KVM the
>> host
>> has to support long mode.
>>
>> I have on my todo to add generic code for honouring bl.max_transfer_length
>> in block.c. We could change default maximum from INT_MAX to SIZE_MAX >>
>> BDRV_SECTOR_BITS
>> for bl.max_transfer_length.
> So the conclusion is that we'll apply this series as it is and you'll
> take care of the rest later?
Yes, and actually we need a macro like
#define BDRV_MAX_REQUEST_SECTORS MIN(SIZE_MAX >> BDRV_SECTOR_BITS, INT_MAX)
as limit for everything. Because bdrv_check_byte_request already has a size_t
argument.
So we could already create an overflow in bdrv_check_request when we convert
nb_sectors to size_t.
I will create a patch to catch at least this overflow shortly.
Peter
- [Qemu-devel] [PATCH v3 0/2] fix max_discard for NBD/gluster block drivers, Denis V. Lunev, 2015/02/02
- [Qemu-devel] [PATCH 2/2] nbd: fix max_discard, Denis V. Lunev, 2015/02/02
- [Qemu-devel] [PATCH 1/2] glusterfs: fix max_discard, Denis V. Lunev, 2015/02/02
- Re: [Qemu-devel] [PATCH 1/2] glusterfs: fix max_discard, Peter Lieven, 2015/02/02
- Re: [Qemu-devel] [PATCH 1/2] glusterfs: fix max_discard, Denis V. Lunev, 2015/02/02
- Re: [Qemu-devel] [PATCH 1/2] glusterfs: fix max_discard, Denis V. Lunev, 2015/02/03
- Re: [Qemu-devel] [PATCH 1/2] glusterfs: fix max_discard, Peter Lieven, 2015/02/03
- Re: [Qemu-devel] [PATCH 1/2] glusterfs: fix max_discard, Kevin Wolf, 2015/02/03
- Re: [Qemu-devel] [PATCH 1/2] glusterfs: fix max_discard,
Peter Lieven <=
- Re: [Qemu-devel] [PATCH 1/2] glusterfs: fix max_discard, Denis V. Lunev, 2015/02/03
- Re: [Qemu-devel] [PATCH 1/2] glusterfs: fix max_discard, Peter Lieven, 2015/02/03