qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 1/4] block-commit: Expose granularity


From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH 1/4] block-commit: Expose granularity
Date: Mon, 07 Apr 2014 21:10:49 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0

On 07.04.2014 21:06, Eric Blake wrote:
On 04/07/2014 11:29 AM, Max Reitz wrote:
Allow QMP users to manipulate the granularity used in the block-commit
command.

Signed-off-by: Max Reitz <address@hidden>
---
@@ -214,6 +215,13 @@ void commit_start(BlockDriverState *bs, BlockDriverState 
*base,
      orig_base_flags    = bdrv_get_flags(base);
      orig_overlay_flags = bdrv_get_flags(overlay_bs);
+ if (!granularity) {
+        granularity = COMMIT_BUFFER_SIZE;
+    }
Default granularity of 0 becomes buffer size...

@@ -1876,6 +1876,15 @@ void qmp_block_commit(const char *device,
       */
      BlockdevOnError on_error = BLOCKDEV_ON_ERROR_REPORT;
+ granularity = has_granularity ? granularity : 0;
+
+    if (has_granularity &&
+        (granularity < BDRV_SECTOR_SIZE || (granularity & (granularity - 1))))
+    {
+        error_set(errp, QERR_INVALID_PARAMETER, "granularity");
...but this code rejects attempts for me to explicitly set granularity
to the default.  Should it be 'if (granularity &&' instead of your
current wording?

I intentionally rejected a granularity of 0, as I thought the user were always capable of dropping this parameter for obtaining the default. But since you are bringing this up, I guess there may be cases where a user is forced to give the parameter but wants to keep the default value nonetheless; I will change it to accept 0 in v2.

Also, is it worth using qemu-common.h's is_power_of_2 instead of
inlining it yourself?  (I don't care, as I recognize the bit
manipulations, but other readers might prefer the named version for its
legibility)

You are right, I will use the function from qemu-common.h.

Max

+++ b/qapi-schema.json
@@ -2112,6 +2112,9 @@
  #
  # @speed:  #optional the maximum speed, in bytes per second
  #
+# @granularity: #optional the granularity to be used for the operation, in
+#               bytes; has to be a power of two and at least 512 (since 2.1)
+#
At least you documented here that an explicit '0' is rejected, even
though it might be nicer to allow it for the sake of requesting the
default even if the default later changes in size.

Overall, though, I'm liking this series.





reply via email to

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