[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: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated() |
Date: |
Fri, 21 Jun 2013 15:14:15 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 21.06.2013 um 13:42 hat Peter Lieven geschrieben:
> Am 21.06.2013 13:07, schrieb Kevin Wolf:
> > Am 21.06.2013 um 11:45 hat Peter Lieven geschrieben:
> >> Am 21.06.2013 11:18, schrieb Kevin Wolf:
> >>> 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?
> >> Is there a document how to use it? Or can you help here?
> > The idea would be to take a read lock while any request is in flight
> > (i.e. qemu_co_rwlock_rdlock() before it's started and
> > qemu_co_rwlock_unlock() when it completes), and to take a write lock
> > (qemu_co_rwlock_wrlock) for the part of iscsi_co_is_allocated() that
> > requires that no other request runs in parallel.
> Wouldn't this require that all the other operations in iscsi.c would
> also take these lock? Currently there are only aio requests
> implemented as it seems.
Hm, okay, that makes it a bit harder because AIO callbacks aren't called
in coroutine context. So taking the lock would be easy, but releasing
them could turn out somewhat tricky.
> Would it help here to add sth like this?
>
> while (iscsi_process_flush(iscsilun)) {
> if (qemu_in_coroutine()) {
> qemu_coroutine_yield();
> } else {
> qemu_aio_wait();
> }
> }
You're always in a coroutine here.
The problem is that if you yield, you need to reenter the coroutine at
some point or the is_allocated request would never complete. That's
basically what the coroutine locks do for you: They yield and only
reenter when the lock can be taken.
Kevin
- [Qemu-devel] [PATCH 0/2] iscsi: support for is_allocated and inproved has_zero_init, Peter Lieven, 2013/06/20
- [Qemu-devel] [PATCH 1/2] iscsi: add support for bdrv_co_is_allocated(), Peter Lieven, 2013/06/20
- 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, 2013/06/21
- 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