qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v3 18/22] qcow2: Use intermediate helper CB for


From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH v3 18/22] qcow2: Use intermediate helper CB for amend
Date: Tue, 02 Dec 2014 10:59:23 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0

On 2014-11-28 at 15:13, Stefan Hajnoczi wrote:
On Thu, Nov 20, 2014 at 06:06:34PM +0100, Max Reitz wrote:
@@ -2674,6 +2743,7 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
      bool encrypt;
      int ret;
      QemuOptDesc *desc = opts->list->desc;
+    Qcow2AmendHelperCBInfo helper_cb_info;
while (desc && desc->name) {
          if (!qemu_opt_find(opts, desc->name)) {
@@ -2731,6 +2801,12 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
          desc++;
      }
+ helper_cb_info = (Qcow2AmendHelperCBInfo){
+        .original_status_cb = status_cb,
+        .original_cb_opaque = cb_opaque,
+        .total_operations = (new_version < old_version)
If you respin, another way of writing this is without total_operations
here (so it initializes to 0)...

+    };
+
      /* Upgrade first (some features may require compat=1.1) */
      if (new_version > old_version) {
          s->qcow_version = new_version;
@@ -2789,7 +2865,9 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
/* Downgrade last (so unsupported features can be removed before) */
      if (new_version < old_version) {
-        ret = qcow2_downgrade(bs, new_version, status_cb, cb_opaque);
+        helper_cb_info.current_operation = QCOW2_DOWNGRADING;
...and then helper_cb_info.total_operations++ here.

That way the new_version < old_version check is not duplicated into the
helper_cb_info initializer.

The code is clearer because we assign current_operation and
total_operations at the same time.

Just a style suggestion, feel free to ignore if you don't like it.

I think helper_cb_info.total_operations should be set right when creating the object. We can only do .total_operations++ once, and that is for the very first operation because after that is executed, we may no longer change .total_operations; and I don't think we should treat the first operation differently than the rest.

Max



reply via email to

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