Am 28.06.2016 um 14:32 hat Pavel Butsykin geschrieben:
> On 28.06.2016 14:47, Kevin Wolf wrote:
> >Am 31.05.2016 um 11:15 hat Denis V. Lunev geschrieben:
> >>From: Pavel Butsykin <pbutsy...@virtuozzo.com>
> >>
> >>For bdrv_pwrite_compressed() it looks like most of the code creating
> >>coroutine
> >>is duplicated in blk_prw(). So we can just add a
> >>flag(BDRV_REQ_WRITE_COMPRESSED)
> >>and use the blk_prw() as a generic one.
> >>
> >>Signed-off-by: Pavel Butsykin <pbutsy...@virtuozzo.com>
> >>Signed-off-by: Denis V. Lunev <d...@openvz.org>
> >>CC: Jeff Cody <jc...@redhat.com>
> >>CC: Markus Armbruster <arm...@redhat.com>
> >>CC: Eric Blake <ebl...@redhat.com>
> >>CC: John Snow <js...@redhat.com>
> >>CC: Stefan Hajnoczi <stefa...@redhat.com>
> >>CC: Kevin Wolf <kw...@redhat.com>
> >
> >Oh, so you already do use a flag. Nice. :-)
> >
> >>diff --git a/block/block-backend.c b/block/block-backend.c
> >>index 3c1fc50..9e1c793 100644
> >>--- a/block/block-backend.c
> >>+++ b/block/block-backend.c
> >>@@ -785,7 +785,11 @@ int coroutine_fn blk_co_pwritev(BlockBackend *blk,
> >>int64_t offset,
> >> flags |= BDRV_REQ_FUA;
> >> }
> >>
> >>- return bdrv_co_pwritev(blk_bs(blk), offset, bytes, qiov, flags);
> >>+ if (flags & BDRV_REQ_WRITE_COMPRESSED) {
> >>+ return bdrv_co_pwritev_compressed(blk_bs(blk), offset, bytes,
> >>qiov);
> >>+ } else {
> >>+ return bdrv_co_pwritev(blk_bs(blk), offset, bytes, qiov, flags);
> >>+ }
> >> }
> >
> >If you move the processing of the flag inside bdrv_co_pwritev(), where I
> >think it belongs anyway, you could use the flag from the start (by going
> >through bdrv_prwv_co()) instead of temporarily introducing your own
> >coroutine wrapper. I think that would make the initial conversion
> >patches quite a bit simpler.
>
> You propose to add bdrv_driver_compressed and call it from
> bdrv_aligned_pwritev ?
I'm not sure if we can easily move it that much down the caller chain.
If we can, great. But here I was just thinking of making the switch at
the start of bdrv_co_pwritev() so that you can reuse the existing
wrappers like bdrv_prwv_co().
The long-term advantage of putting it into bdrv_aligned_pwritev() could
be that we would ge the common alignment handling. But I think qcow2
still errors out if you rewrite an already allocated cluster with
qcow2_pwritev_compressed(), so currently doing sub-cluster (or even
sub-sector) writes doesn't make much sense anyway. So I doubt it's worth
the hassle now.
Kevin
