On 09/25/2012 10:29 AM, Jeff Cody wrote: > This adds the live commit coroutine. This iteration focuses on the > commit only below the active layer, and not the active layer itself. > > The behaviour is similar to block streaming; the sectors are walked > through, and anything that exists above 'base' is committed back down > into base. At the end, intermediate images are deleted, and the > chain stitched together. Images are restored to their original open > flags upon completion. > > Signed-off-by: Jeff Cody <jc...@redhat.com>
> +void commit_start(BlockDriverState *bs, BlockDriverState *base,
> + BlockDriverState *top, int64_t speed,
> + BlockErrorAction on_error, BlockDriverCompletionFunc *cb,
> + void *opaque, Error **errp)
> +{
> + CommitBlockJob *s;
> + BlockReopenQueue *reopen_queue = NULL;
> + int orig_overlay_flags;
> + int orig_base_flags;
> + BlockDriverState *overlay_bs;
> + Error *local_err = NULL;
> +
> + if ((on_error == BLOCK_ERR_STOP_ANY ||
> + on_error == BLOCK_ERR_STOP_ENOSPC) &&
> + !bdrv_iostatus_is_enabled(bs)) {
> + error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
> + return;
> + }
> +
> + overlay_bs = bdrv_find_overlay(bs, top);
> +
> + if (overlay_bs == NULL) {
> + error_setg(errp, "Could not find overlay image for %s:",
> top->filename);
> + return;
> + }
> +
> + orig_base_flags = bdrv_get_flags(base);
> + orig_overlay_flags = bdrv_get_flags(overlay_bs);
I think you are missing a check here that base is on the backing chain
of top. See also my comments to 5/7.
> +
> + /* convert base_bs & overlay_bs to r/w, if necessary */
> + if (!(orig_base_flags & BDRV_O_RDWR)) {
> + reopen_queue = bdrv_reopen_queue(reopen_queue, base,
> + orig_base_flags | BDRV_O_RDWR);
> + }
> + if (!(orig_overlay_flags & BDRV_O_RDWR)) {
> + reopen_queue = bdrv_reopen_queue(reopen_queue, overlay_bs,
> + orig_overlay_flags | BDRV_O_RDWR);
> + }
> + if (reopen_queue) {
> + bdrv_reopen_multiple(reopen_queue, &local_err);
Is it valid to make a no-op call, such as:
{ "execute":"block-commit", "arguments":{
"device":"drive0", "top":"base", "base":"base" }}
If so, should we do an early exit here, rather than temporarily changing
base to R/W just to change it back to R/O?
If not, should we be rejecting it up front (again, back to the question
of failing if 'base' is not a backing file of 'top', even if both 'top'
and 'base' are backing files of 'device').
--
Eric Blake ebl...@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
