From: "Benoît Canet" <benoit.canet@irqsave.net>
To: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-devel@nongnu.org, stefanha@redhat.com, quintela@redhat.com
Subject: Re: [Qemu-devel] [PATCH v2] block: Add error handling to bdrv_invalidate_cache()
Date: Tue, 18 Mar 2014 21:09:55 +0100 [thread overview]
Message-ID: <20140318200954.GA3060@irqsave.net> (raw)
In-Reply-To: <1395153402-14807-1-git-send-email-kwolf@redhat.com>
The Tuesday 18 Mar 2014 à 15:36:42 (+0100), Kevin Wolf wrote :
> If it returns an error, the migrated VM will not be started, but qemu
> exits with an error message.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>
> v2:
> - Update quorum as well (not built by default, so it escaped my
> attention in v1)
>
> block.c | 28 ++++++++++++++++++++++------
> block/qcow2.c | 22 +++++++++++++++++++---
> block/qed.c | 21 ++++++++++++++++++---
> block/quorum.c | 9 +++++++--
> include/block/block.h | 4 ++--
> include/block/block_int.h | 2 +-
> migration.c | 8 +++++++-
> 7 files changed, 76 insertions(+), 18 deletions(-)
>
> diff --git a/block.c b/block.c
> index 53f5b44..acb70fd 100644
> --- a/block.c
> +++ b/block.c
> @@ -4781,27 +4781,43 @@ flush_parent:
> return bdrv_co_flush(bs->file);
> }
>
> -void bdrv_invalidate_cache(BlockDriverState *bs)
> +void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)
> {
> + Error *local_err = NULL;
> + int ret;
> +
> if (!bs->drv) {
> return;
> }
>
> if (bs->drv->bdrv_invalidate_cache) {
> - bs->drv->bdrv_invalidate_cache(bs);
> + bs->drv->bdrv_invalidate_cache(bs, &local_err);
> } else if (bs->file) {
> - bdrv_invalidate_cache(bs->file);
> + bdrv_invalidate_cache(bs->file, &local_err);
> + }
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> }
>
> - refresh_total_sectors(bs, bs->total_sectors);
> + ret = refresh_total_sectors(bs, bs->total_sectors);
> + if (ret < 0) {
> + error_setg_errno(errp, -ret, "Could not refresh total sector count");
> + return;
> + }
> }
>
> -void bdrv_invalidate_cache_all(void)
> +void bdrv_invalidate_cache_all(Error **errp)
> {
> BlockDriverState *bs;
> + Error *local_err = NULL;
>
> QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
> - bdrv_invalidate_cache(bs);
> + bdrv_invalidate_cache(bs, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> }
> }
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 945c9d6..b9dc960 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1156,7 +1156,7 @@ static void qcow2_close(BlockDriverState *bs)
> qcow2_free_snapshots(bs);
> }
>
> -static void qcow2_invalidate_cache(BlockDriverState *bs)
> +static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
> {
> BDRVQcowState *s = bs->opaque;
> int flags = s->flags;
> @@ -1164,6 +1164,8 @@ static void qcow2_invalidate_cache(BlockDriverState *bs)
> AES_KEY aes_decrypt_key;
> uint32_t crypt_method = 0;
> QDict *options;
> + Error *local_err = NULL;
> + int ret;
>
> /*
> * Backing files are read-only which makes all of their metadata immutable,
> @@ -1178,11 +1180,25 @@ static void qcow2_invalidate_cache(BlockDriverState *bs)
>
> qcow2_close(bs);
>
> - bdrv_invalidate_cache(bs->file);
> + bdrv_invalidate_cache(bs->file, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
>
> memset(s, 0, sizeof(BDRVQcowState));
> options = qdict_clone_shallow(bs->options);
> - qcow2_open(bs, options, flags, NULL);
> +
> + ret = qcow2_open(bs, options, flags, &local_err);
> + if (local_err) {
> + error_setg(errp, "Could not reopen qcow2 layer: %s",
> + error_get_pretty(local_err));
> + error_free(local_err);
> + return;
> + } else if (ret < 0) {
> + error_setg_errno(errp, -ret, "Could not reopen qcow2 layer");
> + return;
> + }
>
> QDECREF(options);
>
> diff --git a/block/qed.c b/block/qed.c
> index 837accd..3bd9db9 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -1558,16 +1558,31 @@ static int bdrv_qed_change_backing_file(BlockDriverState *bs,
> return ret;
> }
>
> -static void bdrv_qed_invalidate_cache(BlockDriverState *bs)
> +static void bdrv_qed_invalidate_cache(BlockDriverState *bs, Error **errp)
> {
> BDRVQEDState *s = bs->opaque;
> + Error *local_err = NULL;
> + int ret;
>
> bdrv_qed_close(bs);
>
> - bdrv_invalidate_cache(bs->file);
> + bdrv_invalidate_cache(bs->file, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
>
> memset(s, 0, sizeof(BDRVQEDState));
> - bdrv_qed_open(bs, NULL, bs->open_flags, NULL);
> + ret = bdrv_qed_open(bs, NULL, bs->open_flags, &local_err);
> + if (local_err) {
> + error_setg(errp, "Could not reopen qed layer: %s",
> + error_get_pretty(local_err));
> + error_free(local_err);
> + return;
> + } else if (ret < 0) {
> + error_setg_errno(errp, -ret, "Could not reopen qed layer");
> + return;
> + }
> }
>
> static int bdrv_qed_check(BlockDriverState *bs, BdrvCheckResult *result,
> diff --git a/block/quorum.c b/block/quorum.c
> index 33bf2ae..7f580a8 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -625,13 +625,18 @@ static int64_t quorum_getlength(BlockDriverState *bs)
> return result;
> }
>
> -static void quorum_invalidate_cache(BlockDriverState *bs)
> +static void quorum_invalidate_cache(BlockDriverState *bs, Error **errp)
> {
> BDRVQuorumState *s = bs->opaque;
> + Error *local_err = NULL;
> int i;
>
> for (i = 0; i < s->num_children; i++) {
> - bdrv_invalidate_cache(s->bs[i]);
> + bdrv_invalidate_cache(s->bs[i], &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> }
> }
>
> diff --git a/include/block/block.h b/include/block/block.h
> index bd34d14..1ed55d8 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -329,8 +329,8 @@ BlockDriverAIOCB *bdrv_aio_ioctl(BlockDriverState *bs,
> BlockDriverCompletionFunc *cb, void *opaque);
>
> /* Invalidate any cached metadata used by image formats */
> -void bdrv_invalidate_cache(BlockDriverState *bs);
> -void bdrv_invalidate_cache_all(void);
> +void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp);
> +void bdrv_invalidate_cache_all(Error **errp);
>
> void bdrv_clear_incoming_migration_all(void);
>
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 4fc5ea8..cd5bc73 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -153,7 +153,7 @@ struct BlockDriver {
> /*
> * Invalidate any cached meta-data.
> */
> - void (*bdrv_invalidate_cache)(BlockDriverState *bs);
> + void (*bdrv_invalidate_cache)(BlockDriverState *bs, Error **errp);
>
> /*
> * Flushes all data that was already written to the OS all the way down to
> diff --git a/migration.c b/migration.c
> index 00f465e..e0e24d4 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -101,6 +101,7 @@ void qemu_start_incoming_migration(const char *uri, Error **errp)
> static void process_incoming_migration_co(void *opaque)
> {
> QEMUFile *f = opaque;
> + Error *local_err = NULL;
> int ret;
>
> ret = qemu_loadvm_state(f);
> @@ -115,7 +116,12 @@ static void process_incoming_migration_co(void *opaque)
>
> bdrv_clear_incoming_migration_all();
> /* Make sure all file formats flush their mutable metadata */
> - bdrv_invalidate_cache_all();
> + bdrv_invalidate_cache_all(&local_err);
> + if (local_err) {
> + qerror_report_err(local_err);
> + error_free(local_err);
> + exit(EXIT_FAILURE);
> + }
>
> if (autostart) {
> vm_start();
> --
> 1.8.3.1
>
Look good
Reviewed-by: Benoit Canet <benoit@irqsave.net>
prev parent reply other threads:[~2014-03-18 20:10 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-18 14:36 [Qemu-devel] [PATCH v2] block: Add error handling to bdrv_invalidate_cache() Kevin Wolf
2014-03-18 20:09 ` Benoît Canet [this message]
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=20140318200954.GA3060@irqsave.net \
--to=benoit.canet@irqsave.net \
--cc=kwolf@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.