| Message ID | 1525791496-125188-2-git-send-email-anton.nefedov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | qcow2: cluster space preallocation | expand |
On 05/08/2018 09:58 AM, Anton Nefedov wrote: > Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com> > Reviewed-by: Alberto Garcia <berto@igalia.com> > --- > block/mirror.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) Reviewed-by: Eric Blake <eblake@redhat.com> That said, > > diff --git a/block/mirror.c b/block/mirror.c > index 820f512..a22ddef 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -1098,6 +1098,15 @@ static BlockDriver bdrv_mirror_top = { > .bdrv_child_perm = bdrv_mirror_top_child_perm, > }; > > +static void mirror_top_set_supported_flags(BlockDriverState *bs) > +{ > + bs->supported_write_flags = BDRV_REQ_FUA & > + bs->backing->bs->supported_write_flags; > + bs->supported_zero_flags = > + (BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) & > + bs->backing->bs->supported_zero_flags; > +} > + This is a pretty short static function... > static void mirror_start_job(const char *job_id, BlockDriverState *bs, > int creation_flags, BlockDriverState *target, > const char *replaces, int64_t speed, > @@ -1163,6 +1172,8 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs, > return; > } > > + mirror_top_set_supported_flags(mirror_top_bs); ...with exactly one caller. Wouldn't it be easier to just inline it? > + > /* Make sure that the source is not resized while the job is running */ > s = block_job_create(job_id, driver, NULL, mirror_top_bs, > BLK_PERM_CONSISTENT_READ, >
On 15/5/2018 5:35 PM, Eric Blake wrote: > On 05/08/2018 09:58 AM, Anton Nefedov wrote: >> diff --git a/block/mirror.c b/block/mirror.c >> index 820f512..a22ddef 100644 >> --- a/block/mirror.c >> +++ b/block/mirror.c >> @@ -1098,6 +1098,15 @@ static BlockDriver bdrv_mirror_top = { >> .bdrv_child_perm = bdrv_mirror_top_child_perm, >> }; >> +static void mirror_top_set_supported_flags(BlockDriverState *bs) >> +{ >> + bs->supported_write_flags = BDRV_REQ_FUA & >> + bs->backing->bs->supported_write_flags; >> + bs->supported_zero_flags = >> + (BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) & >> + bs->backing->bs->supported_zero_flags; >> +} >> + > > This is a pretty short static function... > >> static void mirror_start_job(const char *job_id, BlockDriverState *bs, >> int creation_flags, BlockDriverState >> *target, >> const char *replaces, int64_t speed, >> @@ -1163,6 +1172,8 @@ static void mirror_start_job(const char *job_id, >> BlockDriverState *bs, >> return; >> } >> + mirror_top_set_supported_flags(mirror_top_bs); > > ...with exactly one caller. Wouldn't it be easier to just inline it? > idk, I felt mirror_start_job() was quite massive already, even considering there are just a few new lines.
diff --git a/block/mirror.c b/block/mirror.c index 820f512..a22ddef 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1098,6 +1098,15 @@ static BlockDriver bdrv_mirror_top = { .bdrv_child_perm = bdrv_mirror_top_child_perm, }; +static void mirror_top_set_supported_flags(BlockDriverState *bs) +{ + bs->supported_write_flags = BDRV_REQ_FUA & + bs->backing->bs->supported_write_flags; + bs->supported_zero_flags = + (BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) & + bs->backing->bs->supported_zero_flags; +} + static void mirror_start_job(const char *job_id, BlockDriverState *bs, int creation_flags, BlockDriverState *target, const char *replaces, int64_t speed, @@ -1163,6 +1172,8 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs, return; } + mirror_top_set_supported_flags(mirror_top_bs); + /* Make sure that the source is not resized while the job is running */ s = block_job_create(job_id, driver, NULL, mirror_top_bs, BLK_PERM_CONSISTENT_READ,