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

On 04/28/2015 09:00 AM, Kevin Wolf wrote:
> From: Stefan Hajnoczi <stefanha@redhat.com>
> 
> 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 <stefanha@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  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.

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 <eblake@redhat.com>
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)

$ diff -u <(sed -n 's/^-//p' patch) <(sed -n 's/^+//p' patch)
+#include "trace.h"
 #include "sysemu/qtest.h"
+#include "block/blockjob.h"
+#include "block/block_int.h"
+
+#define NOT_DONE 0x7fffffff /* used while emulated sync operation in
progress */
+
 static BlockAIOCB *bdrv_aio_readv_em(BlockDriverState *bs,
         int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
         BlockCompletionFunc *cb, void *opaque);
@@ -465,7 +497,6 @@
     return waited;
 }

-
 static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
                                    size_t size)
 {
@@ -2511,8 +2542,3 @@
         bdrv_flush_io_queue(bs->file);
     }
 }
-
--- a/block/Makefile.objs
-block-obj-y += null.o mirror.o
--- /dev/null

Am 28.04.2015 um 19:15 hat Eric Blake geschrieben:
> On 04/28/2015 09:00 AM, Kevin Wolf wrote:
> > From: Stefan Hajnoczi <stefanha@redhat.com>
> > 
> > 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 <stefanha@redhat.com>
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  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 <eblake@redhat.com>
> 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