Re: [Qemu-devel] [PATCH 3/3] iscsi_truncate: ensure there are no requests in flight

[Peter Lieven]

On 11.03.2013 12:44, Paolo Bonzini wrote:
> Il 11/03/2013 11:19, Peter Lieven ha scritto:
> > Am 11.03.2013 um 11:16 schrieb Paolo Bonzini <address@hidden>:
> >
> > > Il 11/03/2013 11:05, Peter Lieven ha scritto:
> > > > ensure that there are no pending I/Os before calling
> > > > the sync readcapacity commands. the block_resize monitor
> > > > command will also flush all I/O, but double check in
> > > > case iscsi_truncate() is called from elsewhere in the
> > > > future.
> > > >
> > > > Signed-off-by: Peter Lieven <address@hidden>
> > > > ---
> > > > block/iscsi.c |    4 ++++
> > > > 1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/block/iscsi.c b/block/iscsi.c
> > > > index 3d52921..de20d53 100644
> > > > --- a/block/iscsi.c
> > > > +++ b/block/iscsi.c
> > > > @@ -1167,6 +1167,10 @@ static int iscsi_truncate(BlockDriverState *bs,
> > > > int64_t offset)
> > > >          return -ENOTSUP;
> > > >      }
> > > >
> > > > +    /* ensure all async requests are completed before executing
> > > > +     * a sync readcapacity */
> > > > +    bdrv_drain_all();
> > > > +
> > > >      if ((ret = iscsi_readcapacity_sync(iscsilun)) != 0) {
> > > >          return ret;
> > > >      }
> > >
> > > NACK to this patch.  It would be a bug, let's fix it properly.
> >
> > ok, are you sure that never ever will some new feature call bdrv_truncate() on 
> > an iscsi
> > device?
>
> I'm sure some new feature _will_ call bdrv_truncate().  But as things
> stand now, it would be a bug to do so on an iSCSI device.
>
> For example, qcow1 (and VHDX) will already call it, but that's a bug and
> should be fixed otherwise.  Your patch will just cause an assertion failure.

In which case can qcow1 (and VHDX) be used in conjunction with an iSCSI Target?

Would it be ok for you to assert if iscsi_truncate() is called when there
are aio's in flight? How can this be checked?

I just want to make sure that nothing nasty happens in that case (such as
nested event loops).

Peter

> Paolo
>
> > otherwise the real fix would be to implement async read capacity commands like 
> > it
> > the first patch version for iscsi_truncate().
> >
> > Peter
> >
> >
> > > The other two are fine, however.
> > >
> > > Paolo