Re: [PATCH v7 08/21] block: introduce preallocate filter

[Vladimir Sementsov-Ogievskiy]

13.11.2020 17:32, Max Reitz wrote:
> On 21.10.20 16:58, Vladimir Sementsov-Ogievskiy wrote:
> > It's intended to be inserted between format and protocol nodes to
> > preallocate additional space (expanding protocol file) on writes
> > crossing EOF. It improves performance for file-systems with slow
> > allocation.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> > ---
> >   docs/system/qemu-block-drivers.rst.inc |  26 ++
> >   qapi/block-core.json                   |  20 +-
> >   block/preallocate.c                    | 559 +++++++++++++++++++++++++
> >   block/meson.build                      |   1 +
> >   4 files changed, 605 insertions(+), 1 deletion(-)
> >   create mode 100644 block/preallocate.c
>
> [...]
>
> > +    if (end <= s->file_end) {
> > +        /* No preallocation needed. */
> > +        return want_merge_zero && offset >= s->zero_start;
> > +    }
> > +
> > +    /* Now we want new preallocation, as request writes beyond s->data_end. */
>
> True, but isn’t s->file_end more important?

Yes, file_end should be here.

> > +
> > +    prealloc_start = want_merge_zero ? MIN(offset, s->file_end) : s->file_end;
> > +    prealloc_end = QEMU_ALIGN_UP(end + s->opts.prealloc_size,
> > +                                 s->opts.prealloc_align);
>
> [...]
>
> > +        if (prealloc == PREALLOC_MODE_FALLOC) {
> > +            /*
> > +             * If offset <= s->file_end, the task is already done, just
> > +             * update s->file_end, to move part of "filter preallocation"
>
> s/file_end/data_end/

yes

> With those fixed, and with %s/5\.2/6.0/:
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>

Thanks!


--
Best regards,
Vladimir