On Mon, Sep 12, 2011 at 10:14:08AM +0100, Stefan Hajnoczi wrote:
> > +static void
> > +iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
> > +{
> > + IscsiAIOCB *acb = (IscsiAIOCB *)blockacb;
> > + IscsiLun *iscsilun = acb->iscsilun;
> > +
> > + acb->status = -ECANCELED;
> > + acb->common.cb(acb->common.opaque, acb->status);
> > + acb->canceled = 1;
> > +
> > + iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
> > + iscsi_abort_task_cb, NULL);
> > +}
>
> The asynchronous abort task call looks odd. If a caller allocates a
> buffer and issues a read request, then we need to make sure that the
> request is really aborted by the time .bdrv_aio_cancel() returns.
Shouldn't new drivers use coroutines instead of aio instead?
(just poking around, I don't fully understand the new scheme myself yet
either)
> BDRV_O_NOCACHE is just for local files and sets the O_DIRECT hint. It
> doesn't affect the cache semantics that the guest sees.
Now that I fixed it it doesn't. But that's been a fairly recent change,
which isn't always easy to pick up people having external patches.
>
> > +/*
> > + * We support iscsi url's on the form
> > + * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
> > + */
Is having username + password on the command line really a that good idea?
Also what about the more complicated iSCSI authentification schemes?
> What will happen if BDRV_SECTOR_SIZE is not a multiple of 512?
hell will break lose for all of qemu.
