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 <js...@redhat.com> > --- > 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; or upon this job's failure (due to error or > - * cancellation) 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 (*abort)(BlockJob *job); > - > - /** > - * If the callback is not NULL, it will be invoked when the job > transitions > - * into the paused state. Paused jobs must not perform any asynchronous > - * I/O or event loop activity. This callback is used to quiesce jobs. > - */ > - void coroutine_fn (*pause)(BlockJob *job); > - > - /** > - * If the callback is not NULL, it will be invoked when the job > transitions > - * out of the paused state. Any asynchronous I/O or event loop activity > - * should be restarted from this callback. > - */ > - void coroutine_fn (*resume)(BlockJob *job); > - > - /* > - * If the callback is not NULL, it will be invoked before the job is > - * resumed in a new AioContext. This is the place to move any resources > - * besides job->blk to the new AioContext. > - */ > - void (*attached_aio_context)(BlockJob *job, AioContext *new_context); > -} BlockJobDriver; > +typedef struct BlockJobDriver BlockJobDriver; > +typedef struct BlockJobTxn BlockJobTxn; > > /** > * BlockJob: > * > * Long-running operation on a BlockDriverState. > */ > -struct BlockJob { > +typedef struct BlockJob { > /** The job type, including the job vtable. */ > const BlockJobDriver *driver; > > @@ -198,7 +135,7 @@ struct BlockJob { > /** Non-NULL if this job is part of a transaction */ > BlockJobTxn *txn; > QLIST_ENTRY(BlockJob) txn_list; > -}; > +} BlockJob; > > typedef enum BlockJobCreateFlags { > BLOCK_JOB_DEFAULT = 0x00, > @@ -227,76 +164,6 @@ BlockJob *block_job_next(BlockJob *job); > BlockJob *block_job_get(const char *id); > > /** > - * block_job_create: > - * @job_id: The id of the newly-created job, or %NULL to have one > - * generated automatically. > - * @job_type: The class object for the newly-created job. > - * @bs: The block > - * @speed: The maximum speed, in bytes per second, or 0 for unlimited. > - * @cb: Completion function for the job. > - * @opaque: Opaque pointer value passed to @cb. > - * @errp: Error object. > - * > - * Create a new long-running block device job and return it. The job > - * will call @cb asynchronously when the job completes. Note that > - * @bs may have been closed at the time the @cb it is called. If > - * this is the case, the job may be reported as either cancelled or > - * completed. > - * > - * This function is not part of the public job interface; it should be > - * called from a wrapper that is specific to the job type. > - */ > -void *block_job_create(const char *job_id, const BlockJobDriver *driver, > - BlockDriverState *bs, int64_t speed, int flags, > - BlockCompletionFunc *cb, void *opaque, Error **errp); > - > -/** > - * block_job_sleep_ns: > - * @job: The job that calls the function. > - * @clock: The clock to sleep on. > - * @ns: How many nanoseconds to stop for. > - * > - * Put the job to sleep (assuming that it wasn't canceled) for @ns > - * nanoseconds. Canceling the job will interrupt the wait immediately. > - */ > -void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns); > - > -/** > - * block_job_yield: > - * @job: The job that calls the function. > - * > - * Yield the block job coroutine. > - */ > -void block_job_yield(BlockJob *job); > - > -/** > - * block_job_ref: > - * @bs: The block device. > - * > - * Grab a reference to the block job. Should be paired with block_job_unref. > - */ > -void block_job_ref(BlockJob *job); > - > -/** > - * block_job_unref: > - * @bs: The block device. > - * > - * Release reference to the block job and release resources if it is the last > - * reference. > - */ > -void block_job_unref(BlockJob *job); > - > -/** > - * block_job_completed: > - * @job: The job being completed. > - * @ret: The status code. > - * > - * Call the completion function that was registered at creation time, and > - * free @job. > - */ > -void block_job_completed(BlockJob *job, int ret); > - > -/** > * block_job_set_speed: > * @job: The job to set the speed for. > * @speed: The new value > @@ -325,14 +192,6 @@ void block_job_cancel(BlockJob *job); > void block_job_complete(BlockJob *job, Error **errp); > > /** > - * block_job_is_cancelled: > - * @job: The job being queried. > - * > - * Returns whether the job is scheduled for cancellation. > - */ > -bool block_job_is_cancelled(BlockJob *job); > - > -/** > * block_job_query: > * @job: The job to get information about. > * > @@ -341,15 +200,6 @@ bool block_job_is_cancelled(BlockJob *job); > BlockJobInfo *block_job_query(BlockJob *job, Error **errp); > > /** > - * block_job_pause_point: > - * @job: The job that is ready to pause. > - * > - * Pause now if block_job_pause() has been called. Block jobs that perform > - * lots of I/O must call this between requests so that the job can be paused. > - */ > -void coroutine_fn block_job_pause_point(BlockJob *job); > - > -/** > * block_job_pause: > * @job: The job to be paused. > * > @@ -392,22 +242,6 @@ void block_job_resume(BlockJob *job); > void block_job_user_resume(BlockJob *job); > > /** > - * block_job_enter: > - * @job: The job to enter. > - * > - * Continue the specified job by entering the coroutine. > - */ > -void block_job_enter(BlockJob *job); > - > -/** > - * block_job_ready: > - * @job: The job which is now ready to complete. > - * > - * Send a BLOCK_JOB_READY event for the specified job. > - */ > -void block_job_event_ready(BlockJob *job); > - > -/** > * block_job_cancel_sync: > * @job: The job to be canceled. > * > @@ -453,37 +287,6 @@ int block_job_complete_sync(BlockJob *job, Error **errp); > void block_job_iostatus_reset(BlockJob *job); > > /** > - * block_job_error_action: > - * @job: The job to signal an error for. > - * @on_err: The error action setting. > - * @is_read: Whether the operation was a read. > - * @error: The error that was reported. > - * > - * Report an I/O error for a block job and possibly stop the VM. Return the > - * action that was selected based on @on_err and @error. > - */ > -BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError > on_err, > - int is_read, int error); > - > -typedef void BlockJobDeferToMainLoopFn(BlockJob *job, void *opaque); > - > -/** > - * block_job_defer_to_main_loop: > - * @job: The job > - * @fn: The function to run in the main loop > - * @opaque: The opaque value that is passed to @fn > - * > - * Execute a given function in the main loop with the BlockDriverState > - * AioContext acquired. Block jobs must call bdrv_unref(), bdrv_close(), and > - * anything that uses bdrv_drain_all() in the main loop. > - * > - * The @job AioContext is held while @fn executes. > - */ > -void block_job_defer_to_main_loop(BlockJob *job, > - BlockJobDeferToMainLoopFn *fn, > - void *opaque); > - > -/** > * block_job_txn_new: > * > * Allocate and return a new block job transaction. Jobs can be added to the > diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h > new file mode 100644 > index 0000000..8eced19 > --- /dev/null > +++ b/include/block/blockjob_int.h > @@ -0,0 +1,232 @@ > +/* > + * Declarations for long-running block device operations > + * > + * Copyright (c) 2011 IBM Corp. > + * Copyright (c) 2012 Red Hat, Inc. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > copy > + * of this software and associated documentation files (the "Software"), to > deal > + * in the Software without restriction, including without limitation the > rights > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > + * copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > FROM, > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > + * THE SOFTWARE. > + */ > + > +#ifndef BLOCKJOB_INT_H > +#define BLOCKJOB_INT_H > + > +#include "block/blockjob.h" > +#include "block/block.h" > + > +/** > + * BlockJobDriver: > + * > + * A class type for block job driver. > + */ > +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; or upon this job's failure (due to error or > + * cancellation) 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 (*abort)(BlockJob *job); > + > + /** > + * If the callback is not NULL, it will be invoked when the job > transitions > + * into the paused state. Paused jobs must not perform any asynchronous > + * I/O or event loop activity. This callback is used to quiesce jobs. > + */ > + void coroutine_fn (*pause)(BlockJob *job); > + > + /** > + * If the callback is not NULL, it will be invoked when the job > transitions > + * out of the paused state. Any asynchronous I/O or event loop activity > + * should be restarted from this callback. > + */ > + void coroutine_fn (*resume)(BlockJob *job); > + > + /* > + * If the callback is not NULL, it will be invoked before the job is > + * resumed in a new AioContext. This is the place to move any resources > + * besides job->blk to the new AioContext. > + */ > + void (*attached_aio_context)(BlockJob *job, AioContext *new_context); > +}; > + > +/** > + * block_job_create: > + * @job_id: The id of the newly-created job, or %NULL to have one > + * generated automatically. > + * @job_type: The class object for the newly-created job. > + * @bs: The block > + * @speed: The maximum speed, in bytes per second, or 0 for unlimited. > + * @cb: Completion function for the job. > + * @opaque: Opaque pointer value passed to @cb. > + * @errp: Error object. > + * > + * Create a new long-running block device job and return it. The job > + * will call @cb asynchronously when the job completes. Note that > + * @bs may have been closed at the time the @cb it is called. If > + * this is the case, the job may be reported as either cancelled or > + * completed. > + * > + * This function is not part of the public job interface; it should be > + * called from a wrapper that is specific to the job type. > + */ > +void *block_job_create(const char *job_id, const BlockJobDriver *driver, > + BlockDriverState *bs, int64_t speed, int flags, > + BlockCompletionFunc *cb, void *opaque, Error **errp); > + > +/** > + * block_job_sleep_ns: > + * @job: The job that calls the function. > + * @clock: The clock to sleep on. > + * @ns: How many nanoseconds to stop for. > + * > + * Put the job to sleep (assuming that it wasn't canceled) for @ns > + * nanoseconds. Canceling the job will interrupt the wait immediately. > + */ > +void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns); > + > +/** > + * block_job_yield: > + * @job: The job that calls the function. > + * > + * Yield the block job coroutine. > + */ > +void block_job_yield(BlockJob *job); > + > +/** > + * block_job_ref: > + * @bs: The block device. > + * > + * Grab a reference to the block job. Should be paired with block_job_unref. > + */ > +void block_job_ref(BlockJob *job); > + > +/** > + * block_job_unref: > + * @bs: The block device. > + * > + * Release reference to the block job and release resources if it is the last > + * reference. > + */ > +void block_job_unref(BlockJob *job); > + > +/** > + * block_job_completed: > + * @job: The job being completed. > + * @ret: The status code. > + * > + * Call the completion function that was registered at creation time, and > + * free @job. > + */ > +void block_job_completed(BlockJob *job, int ret); > + > +/** > + * block_job_is_cancelled: > + * @job: The job being queried. > + * > + * Returns whether the job is scheduled for cancellation. > + */ > +bool block_job_is_cancelled(BlockJob *job); > + > +/** > + * block_job_pause_point: > + * @job: The job that is ready to pause. > + * > + * Pause now if block_job_pause() has been called. Block jobs that perform > + * lots of I/O must call this between requests so that the job can be paused. > + */ > +void coroutine_fn block_job_pause_point(BlockJob *job); > + > +/** > + * block_job_enter: > + * @job: The job to enter. > + * > + * Continue the specified job by entering the coroutine. > + */ > +void block_job_enter(BlockJob *job); > + > +/** > + * block_job_ready: > + * @job: The job which is now ready to complete. > + * > + * Send a BLOCK_JOB_READY event for the specified job. > + */ > +void block_job_event_ready(BlockJob *job); > + > +/** > + * block_job_error_action: > + * @job: The job to signal an error for. > + * @on_err: The error action setting. > + * @is_read: Whether the operation was a read. > + * @error: The error that was reported. > + * > + * Report an I/O error for a block job and possibly stop the VM. Return the > + * action that was selected based on @on_err and @error. > + */ > +BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError > on_err, > + int is_read, int error); > + > +typedef void BlockJobDeferToMainLoopFn(BlockJob *job, void *opaque); > + > +/** > + * block_job_defer_to_main_loop: > + * @job: The job > + * @fn: The function to run in the main loop > + * @opaque: The opaque value that is passed to @fn > + * > + * Execute a given function in the main loop with the BlockDriverState > + * AioContext acquired. Block jobs must call bdrv_unref(), bdrv_close(), and > + * anything that uses bdrv_drain_all() in the main loop. > + * > + * The @job AioContext is held while @fn executes. > + */ > +void block_job_defer_to_main_loop(BlockJob *job, > + BlockJobDeferToMainLoopFn *fn, > + void *opaque); > + > +#endif > diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c > index b79e0c6..f9afc3b 100644 > --- a/tests/test-blockjob-txn.c > +++ b/tests/test-blockjob-txn.c > @@ -13,7 +13,7 @@ > #include "qemu/osdep.h" > #include "qapi/error.h" > #include "qemu/main-loop.h" > -#include "block/blockjob.h" > +#include "block/blockjob_int.h" > #include "sysemu/block-backend.h" > > typedef struct { > diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c > index 18bf850..60b78a3 100644 > --- a/tests/test-blockjob.c > +++ b/tests/test-blockjob.c > @@ -13,7 +13,7 @@ > #include "qemu/osdep.h" > #include "qapi/error.h" > #include "qemu/main-loop.h" > -#include "block/blockjob.h" > +#include "block/blockjob_int.h" > #include "sysemu/block-backend.h" > > static const BlockJobDriver test_block_job_driver = { > -- > 2.7.4 >
Reviewed-by: Jeff Cody <jc...@redhat.com>