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 <[email protected]> > >> --- > >> 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
