[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated() |
Date: |
Fri, 21 Jun 2013 22:01:08 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6 |
Il 21/06/2013 18:06, ronnie sahlberg ha scritto:
> Should we really mix co-routines and AIO in the same backend?
>
> Would it not be better to instead add a new bdrb_aio_is_allocaed and
> use non-blocking async calls to libiscsi ?
Certainly, but is_allocated's code is not the tidiest.
I'm going to replace the is_allocated callback with a more generic one
(Peter knows, and Kevin might have suspected it too), and I will do the
cleanups at the time I do that.
Paolo
>
> On Fri, Jun 21, 2013 at 2:18 AM, Kevin Wolf <address@hidden> wrote:
>> Am 20.06.2013 um 20:20 hat Peter Lieven geschrieben:
>>> Signed-off-by: Peter Lieven <address@hidden>
>>> ---
>>> block/iscsi.c | 57
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 57 insertions(+)
>>>
>>> diff --git a/block/iscsi.c b/block/iscsi.c
>>> index 0bbf0b1..e6b966d 100644
>>> --- a/block/iscsi.c
>>> +++ b/block/iscsi.c
>>> @@ -49,6 +49,7 @@ typedef struct IscsiLun {
>>> uint64_t num_blocks;
>>> int events;
>>> QEMUTimer *nop_timer;
>>> + uint8_t lbpme;
>>> } IscsiLun;
>>>
>>> typedef struct IscsiAIOCB {
>>> @@ -800,6 +801,60 @@ iscsi_getlength(BlockDriverState *bs)
>>> return len;
>>> }
>>>
>>> +static int coroutine_fn iscsi_co_is_allocated(BlockDriverState *bs,
>>> + int64_t sector_num,
>>> + int nb_sectors, int *pnum)
>>> +{
>>> + IscsiLun *iscsilun = bs->opaque;
>>> + struct scsi_task *task = NULL;
>>> + struct scsi_get_lba_status *lbas = NULL;
>>> + struct scsi_lba_status_descriptor *lbasd = NULL;
>>> + int ret;
>>> +
>>> + *pnum = nb_sectors;
>>> +
>>> + if (iscsilun->lbpme == 0) {
>>> + return 1;
>>> + }
>>> +
>>> + /* in-flight requests could invalidate the lba status result */
>>> + while (iscsi_process_flush(iscsilun)) {
>>> + qemu_aio_wait();
>>> + }
>>
>> Note that you're blocking here. The preferred way would be something
>> involving a yield from the coroutine and a reenter as soon as all
>> requests are done. Maybe a CoRwLock does what you need?
>>
>>> +
>>> + task = iscsi_get_lba_status_sync(iscsilun->iscsi, iscsilun->lun,
>>> + sector_qemu2lun(sector_num, iscsilun),
>>> + 8+16);
>>
>> Spacing around operators (8 + 16)
>>
>>> +
>>> + if (task == NULL || task->status != SCSI_STATUS_GOOD) {
>>> + scsi_free_scsi_task(task);
>>> + return 1;
>>
>> Error cases should set *pnum = 0 and return 0.
>>
>> Kevin
>
>
- Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated(), (continued)
- Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated(), Peter Lieven, 2013/06/21
- Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated(), Kevin Wolf, 2013/06/21
- Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated(), Peter Lieven, 2013/06/21
- Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated(), Kevin Wolf, 2013/06/21
- Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated(), Peter Lieven, 2013/06/21
- Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated(), Paolo Bonzini, 2013/06/21
- Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated(), Peter Lieven, 2013/06/21
- Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated(), ronnie sahlberg, 2013/06/21
- Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated(), Peter Lieven, 2013/06/21
- Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated(), ronnie sahlberg, 2013/06/21
- Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated(),
Paolo Bonzini <=
- Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated(), Stefan Hajnoczi, 2013/06/24
- Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated(), Paolo Bonzini, 2013/06/24
- Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated(), Peter Lieven, 2013/06/24
[Qemu-devel] [PATCH 2/2] iscsi: add intelligent has_zero_init check, Peter Lieven, 2013/06/20
[Qemu-devel] [PATCH 0/2] iscsi: support for is_allocated and inproved has_zero_init, Peter Lieven, 2013/06/20