On 09/06/2012 10:29 AM, Kevin Wolf wrote: > Am 30.08.2012 20:47, schrieb Jeff Cody: >> The command for live block commit is added, which has the following >> arguments: >> >> device: the block device to perform the commit on (mandatory) >> base: the base image to commit into; optional (if not specified, >> it is the underlying original image) >> top: the top image of the commit - all data from inside top down >> to base will be committed into base. optional (if not specified, >> it is the active image) - see note below >> speed: maximum speed, in bytes/sec >> >> note: eventually this will support merging down the active layer, >> but that code is not yet complete. If the active layer is passed >> in currently as top, or top is left to the default, then the error >> QERR_TOP_NOT_FOUND will be returned. >> >> The is done as a block job, so upon completion a BLOCK_JOB_COMPLETED will >> be emitted. >> >> Signed-off-by: Jeff Cody <jc...@redhat.com> >> --- >> blockdev.c | 83 >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> qapi-schema.json | 30 ++++++++++++++++++++ >> qmp-commands.hx | 6 ++++ >> 3 files changed, 119 insertions(+) >> >> diff --git a/blockdev.c b/blockdev.c >> index 68d65fb..e0d6ca0 100644 >> --- a/blockdev.c >> +++ b/blockdev.c >> @@ -827,6 +827,89 @@ exit: >> return; >> } >> >> +void qmp_block_commit(const char *device, >> + bool has_base, const char *base, >> + bool has_top, const char *top, >> + bool has_speed, int64_t speed, >> + Error **errp) >> +{ >> + BlockDriverState *bs; >> + BlockDriverState *base_bs, *top_bs, *child_bs; >> + Error *local_err = NULL; >> + int orig_base_flags, orig_top_flags; >> + BlockReopenQueue *reopen_queue = NULL; >> + /* This will be part of the QMP command, if/when the >> + * BlockdevOnError change for blkmirror makes it in >> + */ >> + BlockErrorAction on_error = BLOCK_ERR_REPORT; >> + >> + /* drain all i/o before commits */ >> + bdrv_drain_all(); >> + >> + bs = bdrv_find(device); >> + if (!bs) { >> + error_set(errp, QERR_DEVICE_NOT_FOUND, device); >> + return; >> + } >> + if (base && has_base) { >> + base_bs = bdrv_find_backing_image(bs, base); >> + } else { >> + base_bs = bdrv_find_base(bs); >> + } >> + >> + if (base_bs == NULL) { >> + error_set(errp, QERR_BASE_NOT_FOUND, "NULL"); > > Shouldn't it be base rather than "NULL"? >
Yes >> + return; >> + } >> + >> + if (top && has_top) { >> + /* if we want to allow the active layer, >> + * use 'bdrv_find_image()' here */ >> + top_bs = bdrv_find_backing_image(bs, top); >> + if (top_bs == NULL) { >> + error_set(errp, QERR_TOP_NOT_FOUND, top); >> + return; >> + } >> + } else { >> + /* we will eventually default to the top layer,i.e. top_bs = bs */ >> + error_set(errp, QERR_TOP_NOT_FOUND, top); >> + return; >> + } >> + >> + child_bs = bdrv_find_child(bs, top_bs); >> + >> + orig_base_flags = bdrv_get_flags(base_bs); /* what we are writing into >> */ >> + orig_top_flags = bdrv_get_flags(child_bs); /* to change the backing >> file */ >> + >> + /* convert base_bs to r/w, if necessary */ >> + if (!(orig_base_flags & BDRV_O_RDWR)) { >> + reopen_queue = bdrv_reopen_queue(reopen_queue, base_bs, >> + orig_base_flags | BDRV_O_RDWR); >> + } >> + if (!(orig_top_flags & BDRV_O_RDWR)) { >> + reopen_queue = bdrv_reopen_queue(reopen_queue, base_bs, >> + orig_base_flags | BDRV_O_RDWR); > > I think you mean child_bs and orig_top_flags. (Why isn't it called > orig_child_flags?) > Yes, thanks - and (per your comments on the previous patch), I'll move the reopen bits into commit_start(). >> + } >> + if (reopen_queue) { >> + bdrv_reopen_multiple(reopen_queue, &local_err); >> + if (local_err != NULL) { >> + error_propagate(errp, local_err); >> + return; >> + } >> + } >> + >> + commit_start(bs, base_bs, top_bs, speed, on_error, >> + block_job_cb, bs, orig_base_flags, orig_top_flags, >> + &local_err); >> + if (local_err != NULL) { >> + error_propagate(errp, local_err); >> + return; >> + } >> + /* Grab a reference so hotplug does not delete the BlockDriverState from >> + * underneath us. >> + */ >> + drive_get_ref(drive_get_by_blockdev(bs)); >> +} >> >> static void eject_device(BlockDriverState *bs, int force, Error **errp) >> { >> diff --git a/qapi-schema.json b/qapi-schema.json >> index bd8ad74..45feda6 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -1401,6 +1401,36 @@ >> 'returns': 'str' } >> >> ## >> +# @block-commit >> +# >> +# Live commit of data from child image nodes into parent nodes - i.e., >> +# writes data between 'top' and 'base' into 'base'. >> +# >> +# @device: the name of the device >> +# >> +# @base: #optional The parent image of the device to write data into. >> +# If not specified, this is the original parent image. > > "The file name of the parent image", actually. > > Which I'm afraid means that this isn't an API for the long term, but > there's little we can do about it now... > >> +# >> +# @top: #optional The child image, above which data will not be committed >> +# down. If not specified, this is the active layer. > > Again, the file name. > > Kevin >