[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] overflow of int ret: use ssize_t for ret
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH] overflow of int ret: use ssize_t for ret |
Date: |
Fri, 23 Nov 2012 16:56:15 +0100 |
On Fri, Nov 23, 2012 at 3:15 PM, Peter Maydell <address@hidden> wrote:
> On 23 November 2012 14:11, Stefan Hajnoczi <address@hidden> wrote:
>> On Thu, Nov 22, 2012 at 10:07 AM, Stefan Priebe <address@hidden> wrote:
>>> diff --git a/block/rbd.c b/block/rbd.c
>>> index 5a0f79f..0384c6c 100644
>>> --- a/block/rbd.c
>>> +++ b/block/rbd.c
>>> @@ -69,7 +69,7 @@ typedef enum {
>>> typedef struct RBDAIOCB {
>>> BlockDriverAIOCB common;
>>> QEMUBH *bh;
>>> - int ret;
>>> + ssize_t ret;
>>> QEMUIOVector *qiov;
>>> char *bounce;
>>> RBDAIOCmd cmd;
>>> @@ -86,7 +86,7 @@ typedef struct RADOSCB {
>>> int done;
>>> int64_t size;
>>> char *buf;
>>> - int ret;
>>> + ssize_t ret;
>>> } RADOSCB;
>>>
>>> #define RBD_FD_READ 0
>>
>> I preferred your previous patch:
>>
>> ssize_t on 32-bit hosts has sizeof(ssize_t) == 4. In
>> qemu_rbd_complete_aio() we may assign acb->ret = rcb->size. Here the
>> size field is int64_t, so ssize_t ret would truncate the value.
>
> The rcb size field should be a size_t: it is used for calling
> rbd_aio_write and rbd_aio_read so if we've overflowed 32 bits
> then we've already got a problem there.
You are right that size_t is fine for read/write.
But size_t is not fine for discard:
1. librbd takes uint64_t for discard.
2. Completing a discard request uses size.
3. BlockDriverCompletionFunc only passes int ret value <--- broken for
large discard
Stefan Priebe has been discarding large regions (e.g. >4 GB must be
possible on a 32-bit host).
The previous int64_t patch didn't clean up types completely but it
made things better. AFAICT we need to be able to discard >4 GB so
ssize_t/size_t doesn't cut it on 32-bit hosts.
Stefan