31.05.2019 18:49, Kevin Wolf wrote: > Am 31.05.2019 um 16:10 hat Vladimir Sementsov-Ogievskiy geschrieben: >> 31.05.2019 13:51, Stefan Hajnoczi wrote: >>> On Tue, May 28, 2019 at 11:45:44AM +0300, Vladimir Sementsov-Ogievskiy >>> wrote: >>>> We have similar padding code in bdrv_co_pwritev, >>>> bdrv_co_do_pwrite_zeroes and bdrv_co_preadv. Let's combine and unify >>>> it. >>>> >>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >>>> --- >>>> block/io.c | 344 ++++++++++++++++++++++++++++------------------------- >>> >>> Hmmm...this adds more lines than it removes. O_o >> >> It's near to be the same size, and keep in mind big comment. > > If you take the whole series into account, it looks even less > favourable, despite some comments: > > 3 files changed, 273 insertions(+), 165 deletions(-) > >>> >>> Merging a change like this can still be useful but there's a risk of >>> making the code harder to understand due to the additional layers of >>> abstraction. >> >> It's a preparation for adding qiov_offset parameter to read/write path. Seems >> correct to unify similar things, which I'm going to change. And I really want >> to make code more understandable than it was.. But my view is not general >> of course. > > Depending on the changes you're going to make (e.g. adding more users of > the same functionality, or making the duplicated code much larger), this > can be a good justification even if the code size increases. > > I'd suggest to add the explanation (like 'This is in preparation for > ...') to the commit message then. > > Kevin >
I think, I'll resend the whole series with this preparation next week. -- Best regards, Vladimir
