Re: [Qemu-devel] [PATCH v2 05/11] blockjobs: split interface into public/private

[Kevin Wolf <kwolf@redhat.com>]

Am 01.10.2016 um 00:00 hat John Snow geschrieben:
> To make it a little more obvious which functions are intended to be
> public interface and which are intended to be for use only by jobs
> themselves, split the interface into "public" and "private" files.
> 
> Convert blockjobs (e.g. block/backup) to using the private interface.
> Leave blockdev and others on the public interface.
> 
> Give up and let qemu-img use the internal interface, though it doesn't
> strictly need to be using it.
> 
> As a side-effect, hide the BlockJob and BlockJobDriver implementation
> from most of the QEMU codebase.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

> --- a/block/replication.c
> +++ b/block/replication.c
> @@ -15,7 +15,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu-common.h"
>  #include "block/nbd.h"
> -#include "block/blockjob.h"
> +#include "block/blockjob_int.h"
>  #include "block/block_int.h"
>  #include "block/block_backup.h"
>  #include "sysemu/block-backend.h"

This one is wrong. replication.c is a block job user, not part of the
implementation. After removing this hunk, the result still compiles.

> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -7,16 +7,15 @@
>  #include "qemu/coroutine.h"
>  #include "block/accounting.h"
>  #include "block/dirty-bitmap.h"
> +#include "block/blockjob.h"
>  #include "qapi/qmp/qobject.h"
>  #include "qapi-types.h"
>  #include "qemu/hbitmap.h"

Hm... This header file doesn't need any of the definitions from
blockjob.h, so I guess .c files should include it explicitly rather than
getting it from here. That would be a separate patch, though.

>  /* block.c */
>  typedef struct BlockDriver BlockDriver;
> -typedef struct BlockJob BlockJob;
>  typedef struct BdrvChild BdrvChild;
>  typedef struct BdrvChildRole BdrvChildRole;
> -typedef struct BlockJobTxn BlockJobTxn;
>  
>  typedef struct BlockDriverInfo {
>      /* in bytes, 0 if irrelevant */

> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -37,7 +37,7 @@
>  #include "sysemu/sysemu.h"
>  #include "sysemu/block-backend.h"
>  #include "block/block_int.h"
> -#include "block/blockjob.h"
> +#include "block/blockjob_int.h"
>  #include "block/qapi.h"
>  #include "crypto/init.h"
>  #include "trace/control.h"

qemu-img doesn't compile without including blockjob_int.h, but that's
just a sign that the split isn't completely right yet. Maybe it needs to
use the pulic function block_job_query() to get the information it takes
directly from the BlockJob struct.

Kevin