qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Benoît Canet" <benoit.canet@irqsave.net>
To: Jeff Cody <jcody@redhat.com>
Cc: kwolf@redhat.com, benoit.canet@irqsave.net, pkrempa@redhat.com,
	famz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com
Subject: Re: [Qemu-devel] [PATCH v6 for 2.1 10/10] block: add QAPI command to allow live backing file change
Date: Wed, 18 Jun 2014 15:15:07 +0200	[thread overview]
Message-ID: <20140618131507.GD4107@irqsave.net> (raw)
In-Reply-To: <cb9eba113f25a79d6b1090804fa0c8df9cb4483a.1403041699.git.jcody@redhat.com>

The Tuesday 17 Jun 2014 à 17:53:58 (-0400), Jeff Cody wrote :
> This allows a user to make a live change to the backing file recorded in
> an open image.
> 
> The image file to modify can be specified 2 ways:
> 
> 1) image filename
> 2) image node-name
> 
> Note: this does not cause the backing file itself to be reopened; it
> merely changes the backing filename in the image file structure, and
> in internal BDS structures.
> 
> It is the responsibility of the user to pass a filename string that
> can be resolved when the image chain is reopened, and the filename
> string is not validated.
> 
> A good analogy for this command is that it is a live version of
> 'qemu-img rebase -u', with respect to changing the backing file string.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  blockdev.c           | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  qapi/block-core.json |  60 ++++++++++++++++++++++++++++++
>  qmp-commands.hx      |  74 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 236 insertions(+)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 42c7b3f..273e52a 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2414,6 +2414,108 @@ void qmp_block_job_complete(const char *device, Error **errp)
>      block_job_complete(job, errp);
>  }
>  
> +void qmp_change_backing_file(const char *device,
> +                             bool has_image, const char *image,
> +                             bool has_image_node_name,
> +                             const char *image_node_name,
> +                             const char *backing_file,
> +                             Error **errp)
> +{
> +    BlockDriverState *bs = NULL;
> +    BlockDriverState *image_bs = NULL;
> +    Error *local_err = NULL;
> +    bool ro;
> +    int open_flags;
> +    int ret;
> +
> +    /* validate argument combinations */
> +    if (has_image && has_image_node_name) {
> +        error_setg(errp, "'image' and 'image-node-name' "
> +                         "are mutually exclusive");
> +        return;
> +    }
> +
> +    /* find the top layer BDS of the chain */
> +    bs = bdrv_find(device);
> +    if (!bs) {
> +        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> +        return;
> +    }
> +
> +    if (has_image_node_name) {
> +        image_bs = bdrv_lookup_bs(NULL, image_node_name, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            return;
> +        }
> +    }
> +
> +    if (has_image) {
> +        if (!strcmp(bs->filename, image)) {
> +            image_bs = bs;
> +        } else {
> +            image_bs = bdrv_find_backing_image(bs, image);
> +        }
> +    }
> +
> +    if (!has_image && !has_image_node_name) {
> +        image_bs = bs;
> +    }
> +
> +    if (!image_bs) {
> +        error_setg(errp, "image file not found");
> +        return;
> +    }
> +
> +    if (bdrv_find_base(image_bs) == image_bs) {
> +        error_setg(errp, "not allowing backing file change on an image "
> +                         "without a backing file");
> +        return;
> +    }
> +
> +    /* even though we are not necessarily operating on bs, we need it to
> +     * determine if block ops are currently prohibited on the chain */
> +    if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_CHANGE, errp)) {
> +        return;
> +    }
> +
> +    /* final sanity check */
> +    if (!bdrv_chain_contains(bs, image_bs)) {
> +        error_setg(errp, "'%s' and image file are not in the same chain",
> +                   device);
> +        return;
> +    }
> +
> +    /* if not r/w, reopen to make r/w */
> +    open_flags = image_bs->open_flags;
> +    ro = bdrv_is_read_only(image_bs);
> +
> +    if (ro) {
> +        bdrv_reopen(image_bs, open_flags | BDRV_O_RDWR, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            return;
> +        }
> +    }
> +
> +    ret = bdrv_change_backing_file(image_bs, backing_file,
> +                               image_bs->drv ? image_bs->drv->format_name : "");
> +
> +    if (ret < 0) {
> +        error_setg_errno(errp, -ret, "Could not change backing file to '%s'",
> +                         backing_file);
> +        /* don't exit here, so we can try to restore open flags if
> +         * appropriate */
> +    }
> +
> +    if (ro) {
> +        bdrv_reopen(image_bs, open_flags, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err); /* will preserve prior errp */
> +        }
> +    }
> +}
> +
>  void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
>  {
>      QmpOutputVisitor *ov = qmp_output_visitor_new();
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index c76d45d..0939143 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -680,6 +680,66 @@
>    'data': 'BlockdevSnapshot' }
>  
>  ##
> +# @change-backing-file
> +#
> +# Change the backing file in the image file metadata.  This does not cause QEMU
> +# to reopen the image file to reparse the backing filename (it may, however,
> +# perform a reopen to change permissions from r/o -> r/w -> r/o, if needed).
> +# The new backing file string is written into the image file metadata, and the
> +# QEMU internal strings are updated.
> +#
> +# The image file to perform the operation on can be specified by two different
> +# methods:
> +#
> +#  Method 1: Supply the device name (e.g. 'virtio0'), and optionally the image
> +#            filename.  This would use arguments @device and @image.
> +#
> +#  Method 2: Supply the device name, and the node-name of the image to modify,
> +#            via @image-node-name.
> +#
> +# Arguments @image and @image-node-name are mutually exclusive.
> +#
> +# Method 1 interface
> +#---------------------
> +# @image:          #optional The file name of the image to modify.  If omitted,
> +#                            and @image-node-name is not supplied, then the
> +#                            default is the active layer of the chain described
> +#                            by @device.
> +#
> +# Method 2 interface
> +#---------------------
> +# @image-node-name #optional The name of the block driver state node of the
> +#                            image to modify.  The @device argument is used to
> +#                            verify @image-node-name is in the chain described
> +#                            by @device.
> +#
> +# Common arguments
> +#---------------------
> +# @device:          The name of the device.
> +#
> +# @backing-file:    The string to write as the backing file.  This string is
> +#                   not validated, so care should be taken when specifying
> +#                   the string or the image chain may not be able to be
> +#                   reopened again.
> +#
> +#                   If a pathname string is such that it cannot be
> +#                   resolved by QEMU, that means that subsequent QMP or
> +#                   HMP commands must use node-names for the image in
> +#                   question, as filename lookup methods will fail.
> +#
> +#
> +# Returns: Nothing on success
> +#          If @device does not exist or cannot be determined, DeviceNotFound
> +#          If @image is specified, but not @device, GenericError
> +#          If both @image and @image-node-name are specified, GenericError
> +#
> +# Since: 2.1
> +##
> +{ 'command': 'change-backing-file',
> +  'data': { 'device': 'str', '*image': 'str', '*image-node-name': 'str',
> +            'backing-file': 'str' } }
> +
> +##
>  # @block-commit
>  #
>  # Live commit of data from overlay image nodes into backing nodes - i.e.,
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 314cbba..c1cbfd2 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -1438,6 +1438,80 @@ Example:
>  EQMP
>  
>      {
> +        .name       = "change-backing-file",
> +        .args_type  = "device:s,image:s?,image-node-name:s?,backing-file:s",
> +        .mhandler.cmd_new = qmp_marshal_input_change_backing_file,
> +    },
> +
> +SQMP
> +change-backing-file
> +-------------------
> +Since: 2.1
> +
> +Change the backing file in the image file metadata.  This does not cause QEMU
> +to reopen the image file to reparse the backing filename (it may, however,
> +perform a reopen to change permissions from r/o -> r/w -> r/o, if needed).
> +The new backing file string is written into the image file metadata, and the
> +QEMU internal strings are updated.
> +
> +The image file to perform the operation on can be specified by two different
> +methods:
> +
> + Method 1: Supply the device name (e.g. 'virtio0'), and optionally the image
> +           filename.  This would use arguments "device" and "image".
> +
> + Method 2: Supply the device name, and the node-name of the image to modify,
> +           via "image-node-name".
> +
> +Arguments:
> +
> +Arguments "image" or "image-node-name" are mutually exclusive.
> +
> +
> +Method 1 interface
> +--------------------
> +- "image":              The file name of the image to modify.  If omitted,
> +                        and "image-node-name" is not supplied, then the
> +                        default is the active layer of the chain described
> +                        by device.
> +                        (json-string, optional)
> +
> +
> +Method 2 interface
> +--------------------
> +- "image-node-name":    The name of the block driver state node of the
> +                        image to modify.  The "device" is argument is used to
> +                        verify "image-node-name" is in the chain described by
> +                        "device".
> +                        (json-string, optional)
> +
> +
> +Common arguments
> +--------------------
> +- "device":             The name of the device.
> +                        (json-string)
> +
> +- "backing-file":       The string to write as the backing file.  This string is
> +                        not validated, so care should be taken when specifying
> +                        the string or the image chain may not be able to be
> +                        reopened again.
> +                        (json-string)
> +
> +                        If a pathname string is such that it cannot be
> +                        resolved by QEMU, that means that subsequent QMP or
> +                        HMP commands must use node-names for the image in
> +                        question, as filename lookup methods will fail.
> +
> +
> +Returns: Nothing on success
> +         If "device" does not exist or cannot be determined, DeviceNotFound
> +         If "image" is specified, but not "device, GenericError
> +         If both "image" and "image-node-name" are specified, GenericError
> +
> +
> +EQMP
> +
> +    {
>          .name       = "balloon",
>          .args_type  = "value:M",
>          .mhandler.cmd_new = qmp_marshal_input_balloon,
> -- 
> 1.9.3
> 

Could you provide an example script of how these backing file manipulation
are suposed to be done ?

Best regards

Benoît

  reply	other threads:[~2014-06-18 13:15 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-17 21:53 [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-names Jeff Cody
2014-06-17 21:53 ` [Qemu-devel] [PATCH v6 for 2.1 01/10] block: Auto-generate node_names for each BDS entry Jeff Cody
2014-06-18 12:53   ` Benoît Canet
2014-06-18 13:13     ` Jeff Cody
2014-06-18 13:31       ` Benoît Canet
2014-06-19  8:55   ` Stefan Hajnoczi
2014-06-19 12:30     ` Jeff Cody
2014-06-19 17:03       ` Eric Blake
2014-06-20  4:24       ` Stefan Hajnoczi
2014-06-23 12:41       ` Stefan Hajnoczi
2014-06-17 21:53 ` [Qemu-devel] [PATCH v6 for 2.1 02/10] block: add helper function to determine if a BDS is in a chain Jeff Cody
2014-06-19  6:27   ` Stefan Hajnoczi
2014-06-23 10:24   ` Benoît Canet
2014-06-17 21:53 ` [Qemu-devel] [PATCH v6 for 2.1 03/10] block: simplify bdrv_find_base() and bdrv_find_overlay() Jeff Cody
2014-06-19  6:31   ` Stefan Hajnoczi
2014-06-17 21:53 ` [Qemu-devel] [PATCH v6 for 2.1 04/10] block: make 'top' argument to block-commit optional Jeff Cody
2014-06-17 22:25   ` Eric Blake
2014-06-19 16:56     ` Eric Blake
2014-06-19  6:40   ` Stefan Hajnoczi
2014-06-17 21:53 ` [Qemu-devel] [PATCH v6 for 2.1 05/10] block: Accept node-name arguments for block-commit Jeff Cody
2014-06-18 12:58   ` Benoît Canet
2014-06-17 21:53 ` [Qemu-devel] [PATCH v6 for 2.1 06/10] block: extend block-commit to accept a string for the backing file Jeff Cody
2014-06-19  7:49   ` Stefan Hajnoczi
2014-06-17 21:53 ` [Qemu-devel] [PATCH v6 for 2.1 07/10] block: add ability for block-stream to use node-name Jeff Cody
2014-06-18 13:06   ` Benoît Canet
2014-06-19  8:01   ` Stefan Hajnoczi
2014-06-17 21:53 ` [Qemu-devel] [PATCH v6 for 2.1 08/10] block: add backing-file option to block-stream Jeff Cody
2014-06-19  8:04   ` Stefan Hajnoczi
2014-06-17 21:53 ` [Qemu-devel] [PATCH v6 for 2.1 09/10] block: Add QMP documentation for block-stream Jeff Cody
2014-06-19  8:06   ` Stefan Hajnoczi
2014-06-17 21:53 ` [Qemu-devel] [PATCH v6 for 2.1 10/10] block: add QAPI command to allow live backing file change Jeff Cody
2014-06-18 13:15   ` Benoît Canet [this message]
2014-06-19  8:37     ` Stefan Hajnoczi
2014-06-19 19:08       ` Jeff Cody
2014-06-19  8:37   ` Stefan Hajnoczi
2014-06-19  9:17 ` [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-names Stefan Hajnoczi
2014-06-19 16:26   ` Jeff Cody
2014-06-19 16:49     ` Eric Blake
2014-06-19 16:54       ` Eric Blake
2014-06-19 18:22       ` [Qemu-devel] Op Blockers on child nodes (was Re: [PATCH v6 for 2.1 00/10] Modify block jobs to use) node-names Jeff Cody
2014-06-24 12:55       ` [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-names Kevin Wolf
2014-06-23 13:08     ` Stefan Hajnoczi
2014-06-23 14:17       ` Benoît Canet
2014-06-24  2:48       ` Fam Zheng
2014-06-24 13:32         ` Jeff Cody
2014-06-24 14:08           ` Kevin Wolf
2014-06-24 15:30             ` Benoît Canet
2014-06-19 17:49   ` Benoît Canet
2014-06-24 17:08   ` Jeff Cody

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140618131507.GD4107@irqsave.net \
    --to=benoit.canet@irqsave.net \
    --cc=famz@redhat.com \
    --cc=jcody@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pkrempa@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).