Re: [Qemu-block] [PULL 76/76] block: move I/O request processing to block/io.c

[Kevin Wolf]

Am 28.04.2015 um 19:15 hat Eric Blake geschrieben:
> On 04/28/2015 09:00 AM, Kevin Wolf wrote:
> > From: Stefan Hajnoczi <address@hidden>
> > 
> > The block.c file has grown to over 6000 lines.  It is time to split this
> > file so there are fewer conflicts and the code is easier to maintain.
> > 
> > Extract I/O request processing code:
> >  * Read
> >  * Write
> >  * Zero writes and making the image empty
> >  * Flush
> >  * Discard
> >  * ioctl
> >  * Tracked requests and queuing
> >  * Throttling and copy-on-read
> >  * Block status and allocated functions
> >  * Refreshing block limits
> >  * Reading/writing vmstate
> >  * qemu_blockalign() and friends
> > 
> > The patch simply moves code from block.c into block/io.c.
> > 
> > Signed-off-by: Stefan Hajnoczi <address@hidden>
> > Signed-off-by: Kevin Wolf <address@hidden>
> > ---
> >  block.c             | 3394 
> > +++++++--------------------------------------------
> >  block/Makefile.objs |    2 +-
> >  block/io.c          | 2540 ++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 2982 insertions(+), 2954 deletions(-)
> >  create mode 100644 block/io.c
> 
> Kevin, compare your diff to Stefan's:
> 
> > ---
> >  block.c             | 2512 
> > --------------------------------------------------
> >  block/Makefile.objs |    2 +-
> >  block/io.c          | 2540 
> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 2541 insertions(+), 2513 deletions(-)
> >  create mode 100644 block/io.c
> 
> The difference is that Stefan has done 'git config diff.algorithm
> patience', which makes for MUCH more legible diffs on a code motion
> patch.  http://wiki.qemu.org/Contribute/SubmitAPatch mentions this as a
> hint for a nicer setup.

Indeed, it does. I used --patience for reviewing the patch but neglected
to add it when sending the pull request. That said, I don't think I
could have enabled it for a single patch, and I'm not sure if it's
universally better (if so, why wouldn't it be the default?)

> At any rate, you've already sent the pull request, so I'm probably too
> late; but for the record, here's why I would add:
> Reviewed-by: Eric Blake <address@hidden>
> when reviewing Stefan's version of the patch (the two versions give the
> same end result, but my review technique falls flat on Kevin's replay of
> the patch)

You could apply both series and diff the result if you wanted.

But this one was easy enough to review on my own (...famous last words?)

Kevin

pgpXMtWZuXaR5.pgp
Description: PGP signature