qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 19/34] qcow2: Make qcow2_update_options() suitab


From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH 19/34] qcow2: Make qcow2_update_options() suitable for transactions
Date: Wed, 13 May 2015 14:06:15 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0

On 08.05.2015 19:21, Kevin Wolf wrote:
Before we can allow updating options at runtime with bdrv_reopen(), we
need to split the function into prepare/commit/abort parts.

Signed-off-by: Kevin Wolf <address@hidden>
---
  block/qcow2.c | 101 ++++++++++++++++++++++++++++++++++++++--------------------
  1 file changed, 67 insertions(+), 34 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 84d6e0f..fc9375e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -538,17 +538,24 @@ static void read_cache_sizes(QemuOpts *opts, uint64_t 
*l2_cache_size,
      }
  }
-static int qcow2_update_options(BlockDriverState *bs, QDict *options,
-                                int flags, Error **errp)
+typedef struct Qcow2ReopenState {
+    Qcow2Cache* l2_table_cache;
+    Qcow2Cache* refcount_block_cache;

*cough*

+    bool use_lazy_refcounts;
+    int overlap_check;
+    bool discard_passthrough[QCOW2_DISCARD_MAX];
+} Qcow2ReopenState;
+
+static int qcow2_update_options_prepare(BlockDriverState *bs,
+                                        Qcow2ReopenState *r,
+                                        QDict *options, int flags,
+                                        Error **errp)
  {
      BDRVQcowState *s = bs->opaque;
      QemuOpts *opts = NULL;
      const char *opt_overlap_check, *opt_overlap_check_template;
      int overlap_check_template = 0;
      uint64_t l2_cache_size, refcount_cache_size;
-    Qcow2Cache* l2_table_cache = NULL;
-    Qcow2Cache* refcount_block_cache = NULL;
-    bool use_lazy_refcounts;
      int i;
      Error *local_err = NULL;
      int ret;
@@ -590,18 +597,18 @@ static int qcow2_update_options(BlockDriverState *bs, 
QDict *options,
      }
/* alloc L2 table/refcount block cache */
-    l2_table_cache = qcow2_cache_create(bs, l2_cache_size);
-    refcount_block_cache = qcow2_cache_create(bs, refcount_cache_size);
-    if (l2_table_cache == NULL || refcount_block_cache == NULL) {
+    r->l2_table_cache = qcow2_cache_create(bs, l2_cache_size);
+    r->refcount_block_cache = qcow2_cache_create(bs, refcount_cache_size);
+    if (r->l2_table_cache == NULL || r->refcount_block_cache == NULL) {
          error_setg(errp, "Could not allocate metadata caches");
          ret = -ENOMEM;
          goto fail;
      }
/* Enable lazy_refcounts according to image and command line options */
-    use_lazy_refcounts = qemu_opt_get_bool(opts, QCOW2_OPT_LAZY_REFCOUNTS,
+    r->use_lazy_refcounts = qemu_opt_get_bool(opts, QCOW2_OPT_LAZY_REFCOUNTS,
          (s->compatible_features & QCOW2_COMPAT_LAZY_REFCOUNTS));
-    if (use_lazy_refcounts && s->qcow_version < 3) {
+    if (r->use_lazy_refcounts && s->qcow_version < 3) {
          error_setg(errp, "Lazy refcounts require a qcow2 image with at least "
                     "qemu 1.1 compatibility level");
          ret = -EINVAL;
@@ -640,46 +647,72 @@ static int qcow2_update_options(BlockDriverState *bs, 
QDict *options,
          goto fail;
      }
- /*
-     * Start updating fields in BDRVQcowState.
-     * After this point no failure is allowed any more.
-     */
-    s->overlap_check = 0;
+    r->overlap_check = 0;
      for (i = 0; i < QCOW2_OL_MAX_BITNR; i++) {
          /* overlap-check defines a template bitmask, but every flag may be
           * overwritten through the associated boolean option */
-        s->overlap_check |=
+        r->overlap_check |=
              qemu_opt_get_bool(opts, overlap_bool_option_names[i],
                                overlap_check_template & (1 << i)) << i;
      }
- s->l2_table_cache = l2_table_cache;
-    s->refcount_block_cache = refcount_block_cache;
-
-    s->use_lazy_refcounts = use_lazy_refcounts;
-
-    s->discard_passthrough[QCOW2_DISCARD_NEVER] = false;
-    s->discard_passthrough[QCOW2_DISCARD_ALWAYS] = true;
-    s->discard_passthrough[QCOW2_DISCARD_REQUEST] =
+    r->discard_passthrough[QCOW2_DISCARD_NEVER] = false;
+    r->discard_passthrough[QCOW2_DISCARD_ALWAYS] = true;
+    r->discard_passthrough[QCOW2_DISCARD_REQUEST] =
          qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_REQUEST,
                            flags & BDRV_O_UNMAP);
-    s->discard_passthrough[QCOW2_DISCARD_SNAPSHOT] =
+    r->discard_passthrough[QCOW2_DISCARD_SNAPSHOT] =
          qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_SNAPSHOT, true);
-    s->discard_passthrough[QCOW2_DISCARD_OTHER] =
+    r->discard_passthrough[QCOW2_DISCARD_OTHER] =
          qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_OTHER, false);
ret = 0;
  fail:
-    if (ret < 0) {
-        if (l2_table_cache) {
-            qcow2_cache_destroy(bs, l2_table_cache);
-        }
-        if (refcount_block_cache) {
-            qcow2_cache_destroy(bs, refcount_block_cache);
-        }
-    }
      qemu_opts_del(opts);
      opts = NULL;
+    return ret;
+}
+
+static void qcow2_update_options_commit(BlockDriverState *bs,
+                                        Qcow2ReopenState *r)
+{
+    BDRVQcowState *s = bs->opaque;
+    int i;
+
+    s->l2_table_cache = r->l2_table_cache;
+    s->refcount_block_cache = r->refcount_block_cache;
+
+    s->overlap_check = r->overlap_check;
+    s->use_lazy_refcounts = r->use_lazy_refcounts;
+
+    for (i = 0; i < QCOW2_DISCARD_MAX; i++) {
+        s->discard_passthrough[i] = r->discard_passthrough[i];
+    }
+}
+
+static void qcow2_update_options_abort(BlockDriverState *bs,
+                                       Qcow2ReopenState *r)
+{
+    if (r->l2_table_cache) {
+        qcow2_cache_destroy(bs, r->l2_table_cache);
+    }
+    if (r->refcount_block_cache) {
+        qcow2_cache_destroy(bs, r->refcount_block_cache);
+    }
+}

Okay, now it makes sense not to set {l2_table,refcount_block}_cache to NULL in patch 18 after they've been moved to *s.

Reviewed-by: Max Reitz <address@hidden>

+
+static int qcow2_update_options(BlockDriverState *bs, QDict *options,
+                                int flags, Error **errp)
+{
+    Qcow2ReopenState r = {};
+    int ret;
+
+    ret = qcow2_update_options_prepare(bs, &r, options, flags, errp);
+    if (ret >= 0) {
+        qcow2_update_options_commit(bs, &r);
+    } else {
+        qcow2_update_options_abort(bs, &r);
+    }
return ret;
  }




reply via email to

[Prev in Thread] Current Thread [Next in Thread]