Re: [Qemu-devel] [PATCH 6/7] blockjobs: split interface into public/private, Part 1

2016-10-25 Thread Jeff Cody
On Thu, Oct 13, 2016 at 06:57:01PM -0400, John Snow wrote:
> 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.
> 
> There are remaining uses of private state by qemu-img, and several
> cases in blockdev.c and block/io.c where we grab job->blk for the
> purposes of acquiring an AIOContext.
> 
> These will be corrected in future patches.
> 
> Signed-off-by: John Snow 
> ---
>  block/backup.c   |   2 +-
>  block/commit.c   |   2 +-
>  block/mirror.c   |   2 +-
>  block/stream.c   |   2 +-
>  blockjob.c   |   2 +-
>  include/block/block.h|   3 +-
>  include/block/blockjob.h | 205 +-
>  include/block/blockjob_int.h | 232 
> +++
>  tests/test-blockjob-txn.c|   2 +-
>  tests/test-blockjob.c|   2 +-
>  10 files changed, 244 insertions(+), 210 deletions(-)
>  create mode 100644 include/block/blockjob_int.h
> 
> diff --git a/block/backup.c b/block/backup.c
> index 6a60ca8..6d12100 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -16,7 +16,7 @@
>  #include "trace.h"
>  #include "block/block.h"
>  #include "block/block_int.h"
> -#include "block/blockjob.h"
> +#include "block/blockjob_int.h"
>  #include "block/block_backup.h"
>  #include "qapi/error.h"
>  #include "qapi/qmp/qerror.h"
> diff --git a/block/commit.c b/block/commit.c
> index 475a375..d555600 100644
> --- a/block/commit.c
> +++ b/block/commit.c
> @@ -15,7 +15,7 @@
>  #include "qemu/osdep.h"
>  #include "trace.h"
>  #include "block/block_int.h"
> -#include "block/blockjob.h"
> +#include "block/blockjob_int.h"
>  #include "qapi/error.h"
>  #include "qapi/qmp/qerror.h"
>  #include "qemu/ratelimit.h"
> diff --git a/block/mirror.c b/block/mirror.c
> index 4374fb4..c81b5e0 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -13,7 +13,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "trace.h"
> -#include "block/blockjob.h"
> +#include "block/blockjob_int.h"
>  #include "block/block_int.h"
>  #include "sysemu/block-backend.h"
>  #include "qapi/error.h"
> diff --git a/block/stream.c b/block/stream.c
> index 7d6877d..906f7f3 100644
> --- a/block/stream.c
> +++ b/block/stream.c
> @@ -14,7 +14,7 @@
>  #include "qemu/osdep.h"
>  #include "trace.h"
>  #include "block/block_int.h"
> -#include "block/blockjob.h"
> +#include "block/blockjob_int.h"
>  #include "qapi/error.h"
>  #include "qapi/qmp/qerror.h"
>  #include "qemu/ratelimit.h"
> diff --git a/blockjob.c b/blockjob.c
> index d118a1f..e6f0d97 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -27,7 +27,7 @@
>  #include "qemu-common.h"
>  #include "trace.h"
>  #include "block/block.h"
> -#include "block/blockjob.h"
> +#include "block/blockjob_int.h"
>  #include "block/block_int.h"
>  #include "sysemu/block-backend.h"
>  #include "qapi/qmp/qerror.h"
> diff --git a/include/block/block.h b/include/block/block.h
> index 107c603..89b5feb 100644
> --- 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"
>  
>  /* 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 */
> diff --git a/include/block/blockjob.h b/include/block/blockjob.h
> index 5b61140..bfc8233 100644
> --- a/include/block/blockjob.h
> +++ b/include/block/blockjob.h
> @@ -28,78 +28,15 @@
>  
>  #include "block/block.h"
>  
> -/**
> - * BlockJobDriver:
> - *
> - * A class type for block job driver.
> - */
> -typedef struct BlockJobDriver {
> -/** Derived BlockJob struct size */
> -size_t instance_size;
> -
> -/** String describing the operation, part of query-block-jobs QMP API */
> -BlockJobType job_type;
> -
> -/** Optional callback for job types that support setting a speed limit */
> -void (*set_speed)(BlockJob *job, int64_t speed, Error **errp);
> -
> -/** Optional callback for job types that need to forward I/O status 
> reset */
> -void (*iostatus_reset)(BlockJob *job);
> -
> -/**
> - * Optional callback for job types whose completion must be triggered
> - * manually.
> - */
> -void (*complete)(BlockJob *job, Error **errp);
> -
> -/**
> - * If the callback is not NULL, it will be invoked when all the jobs
> - * belonging to the same transaction 

Re: [Qemu-devel] [PATCH 6/7] blockjobs: split interface into public/private, Part 1

2016-10-14 Thread Kevin Wolf
Am 14.10.2016 um 00:57 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.
> 
> There are remaining uses of private state by qemu-img, and several
> cases in blockdev.c and block/io.c where we grab job->blk for the
> purposes of acquiring an AIOContext.
> 
> These will be corrected in future patches.
> 
> Signed-off-by: John Snow 

Reviewed-by: Kevin Wolf 



[Qemu-devel] [PATCH 6/7] blockjobs: split interface into public/private, Part 1

2016-10-13 Thread John Snow
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.

There are remaining uses of private state by qemu-img, and several
cases in blockdev.c and block/io.c where we grab job->blk for the
purposes of acquiring an AIOContext.

These will be corrected in future patches.

Signed-off-by: John Snow 
---
 block/backup.c   |   2 +-
 block/commit.c   |   2 +-
 block/mirror.c   |   2 +-
 block/stream.c   |   2 +-
 blockjob.c   |   2 +-
 include/block/block.h|   3 +-
 include/block/blockjob.h | 205 +-
 include/block/blockjob_int.h | 232 +++
 tests/test-blockjob-txn.c|   2 +-
 tests/test-blockjob.c|   2 +-
 10 files changed, 244 insertions(+), 210 deletions(-)
 create mode 100644 include/block/blockjob_int.h

diff --git a/block/backup.c b/block/backup.c
index 6a60ca8..6d12100 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -16,7 +16,7 @@
 #include "trace.h"
 #include "block/block.h"
 #include "block/block_int.h"
-#include "block/blockjob.h"
+#include "block/blockjob_int.h"
 #include "block/block_backup.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
diff --git a/block/commit.c b/block/commit.c
index 475a375..d555600 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -15,7 +15,7 @@
 #include "qemu/osdep.h"
 #include "trace.h"
 #include "block/block_int.h"
-#include "block/blockjob.h"
+#include "block/blockjob_int.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
 #include "qemu/ratelimit.h"
diff --git a/block/mirror.c b/block/mirror.c
index 4374fb4..c81b5e0 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -13,7 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "trace.h"
-#include "block/blockjob.h"
+#include "block/blockjob_int.h"
 #include "block/block_int.h"
 #include "sysemu/block-backend.h"
 #include "qapi/error.h"
diff --git a/block/stream.c b/block/stream.c
index 7d6877d..906f7f3 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -14,7 +14,7 @@
 #include "qemu/osdep.h"
 #include "trace.h"
 #include "block/block_int.h"
-#include "block/blockjob.h"
+#include "block/blockjob_int.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qerror.h"
 #include "qemu/ratelimit.h"
diff --git a/blockjob.c b/blockjob.c
index d118a1f..e6f0d97 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -27,7 +27,7 @@
 #include "qemu-common.h"
 #include "trace.h"
 #include "block/block.h"
-#include "block/blockjob.h"
+#include "block/blockjob_int.h"
 #include "block/block_int.h"
 #include "sysemu/block-backend.h"
 #include "qapi/qmp/qerror.h"
diff --git a/include/block/block.h b/include/block/block.h
index 107c603..89b5feb 100644
--- 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"
 
 /* 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 */
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 5b61140..bfc8233 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -28,78 +28,15 @@
 
 #include "block/block.h"
 
-/**
- * BlockJobDriver:
- *
- * A class type for block job driver.
- */
-typedef struct BlockJobDriver {
-/** Derived BlockJob struct size */
-size_t instance_size;
-
-/** String describing the operation, part of query-block-jobs QMP API */
-BlockJobType job_type;
-
-/** Optional callback for job types that support setting a speed limit */
-void (*set_speed)(BlockJob *job, int64_t speed, Error **errp);
-
-/** Optional callback for job types that need to forward I/O status reset 
*/
-void (*iostatus_reset)(BlockJob *job);
-
-/**
- * Optional callback for job types whose completion must be triggered
- * manually.
- */
-void (*complete)(BlockJob *job, Error **errp);
-
-/**
- * If the callback is not NULL, it will be invoked when all the jobs
- * belonging to the same transaction complete; or upon this job's
- * completion if it is not in a transaction. Skipped if NULL.
- *
- * All jobs will complete with a call to either .commit() or .abort() but
- * never both.
- */
-void (*commit)(BlockJob *job);
-
-/**
- * If the callback is not NULL, it will be invoked when any job in the
- * same transaction fails;