On Wed, Jul 08, 2020 at 07:17:52PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 08.07.2020 15:07, Stefan Hajnoczi wrote:
> > On Sat, Jun 20, 2020 at 05:36:47PM +0300, Vladimir Sementsov-Ogievskiy 
> > wrote:
> > > It may be used for file-systems with slow allocation.
> > > 
> > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> > > ---
> > >   qapi/block-core.json |   3 +-
> > >   block/preallocate.c  | 255 +++++++++++++++++++++++++++++++++++++++++++
> > >   block/Makefile.objs  |   1 +
> > >   3 files changed, 258 insertions(+), 1 deletion(-)
> > >   create mode 100644 block/preallocate.c
> > 
> > Please add documentation to docs/system/qemu-block-drivers.rst.inc
> > describing the purpose of this block driver and how to use it.
> 
> This implies adding new section "Filters", yes?

Yes, please.

> > 
> > Since this filter grows the file I guess it's intended to be below an
> > image format?
> 
> Yes, between format and protocol nodes.

Thanks for confirming. Please include this in the documentation. While
reading the code I was thinking about how this can work if the guest is
directly exposed to the filter. Users might wonder about the same thing.

> > > +    /* Length should not have changed */
> > > +    assert(len == bdrv_getlength(bs->file->bs));
> > > +
> > > +    start = write_zero ? MIN(offset, len) : len;
> > > +    end = QEMU_ALIGN_UP(offset + bytes + s->prealloc_size, 
> > > s->prealloc_align);
> > > +
> > > +    ret = bdrv_co_pwrite_zeroes_locked(bs->file, start, end - start,
> > > +                                       BDRV_REQ_NO_FALLBACK, lock);
> > > +
> > > +    bdrv_co_range_unlock(lock);
> > 
> > Hmm...if this piece of code is the only user of bdrv_co_range_try_lock()
> > then a BDRV_REQ_NO_WAIT flag might be a simpler API.
> > 
> > I thought the lock request would be used to perform multiple operations,
> > but if it's just for a single operation then I think it's less code and
> > easier to understand without the lock request.
> 
> Hmm, again, I don't remember exact reasons. Firstly, I was afraid of length
> change during try_lock and have a double check for bdrv_getlength(). Then
> I decided that it's impossible and change the check to an assertion.
> Probably, the only reason to leave locked range was "I already have the code,
> it will help with copy-on-read, why not to use it".. OK, I'll try rewrite it
> with help of new flag.

Thanks!

Stefan

Attachment: signature.asc
Description: PGP signature

Reply via email to