[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv3] block: optimize zero writes with bdrv_write_z
From: |
Peter Lieven |
Subject: |
Re: [Qemu-devel] [PATCHv3] block: optimize zero writes with bdrv_write_zeroes |
Date: |
Wed, 07 May 2014 16:26:56 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 |
On 07.05.2014 04:52, Eric Blake wrote:
On 05/06/2014 06:23 PM, Peter Lieven wrote:
this patch tries to optimize zero write requests
by automatically using bdrv_write_zeroes if it is
supported by the format.
This significantly speeds up file system initialization and
should speed zero write test used to test backend storage
performance.
Signed-off-by: Peter Lieven <address@hidden>
---
v2->v3: - moved parameter parsing to blockdev_init
- added per device detect_zeroes status to
hmp (info block -v) and qmp (query-block) [Eric]
- added support to enable detect-zeroes also
for hot added devices [Eric]
- added missing entry to qemu_common_drive_opts
- fixed description of qemu_iovec_is_zero [Fam]
+static BdrvDetectZeroes parse_detect_zeroes(const char *buf, Error **errp)
+{
+ if (!buf || !strcmp(buf, "off")) {
+ return BDRV_DETECT_ZEROES_OFF;
+ } else if (!strcmp(buf, "on")) {
+ return BDRV_DETECT_ZEROES_ON;
+ } else if (!strcmp(buf, "unmap")) {
+ return BDRV_DETECT_ZEROES_UNMAP;
+ } else {
+ error_setg(errp, "invalid value for detect-zeroes: %s",
+ buf);
+ }
+ return BDRV_DETECT_ZEROES_OFF;
+}
Isn't there QAPI generated code that you can use instead of open-coding
this conversion between string and enum values?
Actually I have no idea. As you pointed out in the qapi patch I sent
it was quite hard for me to crawl through the whole stuff as one who is not
familiar with it. Can somebody advise here? Anyhow, I wonder
how this would work since qapi doesn't know the C Macros.
+++ b/qapi-schema.json
@@ -937,6 +937,8 @@
# @encryption_key_missing: true if the backing device is encrypted but an
# valid encryption key is missing
#
+# @detect_zeroes: detect and optimize zero writes (Since 2.1)
+#
# @bps: total throughput limit in bytes per second is specified
#
# @bps_rd: read throughput limit in bytes per second is specified
@@ -972,6 +974,7 @@
'data': { 'file': 'str', '*node-name': 'str', 'ro': 'bool', 'drv': 'str',
'*backing_file': 'str', 'backing_file_depth': 'int',
'encrypted': 'bool', 'encryption_key_missing': 'bool',
+ 'detect_zeroes': 'str',
For new additions, we try to prefer dash over underscore. Eww - we've
already been inconsistent in this struct, with backing_file vs. node-name.
Maybe s/detect_zeroes/detect-zeroes/. I obviously can't argue too
strongly in light of the rest of the struct in isolation. However, you
DID use detect-zeroes on the input side later in the patch, so being
consistent between the two sites would be nice (given that node-name was
named consistently).
I tried to be consistent here. All "setters" use a dash while all "getters"
have an underscore. Look e.g. at all the I/O thortteling parameters.
One reason might be that a dash is not allowed as a member name in a struct.
From this perspective the only inconsistent one is node-name.
On the other hand, I _can_ argue strongly that open-coding this as 'str'
is wrong. You already added an enum type, so use it:
'detect_zeroes': 'BlockdevDetectZeroesOptions',
(hmm, in this context, it's not really an option, so maybe there is some
other name we can bikeshed about; but beyond 'BlockdevDetectZeroes', I
don't have any other good ideas)
I tried to, however it did not compile for me when I tried this.
zero is one of those odd words with two different pluralized spellings
both in common use. Code base currently has a 1:2 ratio between 'zeros'
vs. 'zeroes', so I guess you're okay; on the other hand, 'man qemu-img'
documents that 'convert -S' detects "zeros".
The reason I choosed zeroEs is that the function in the background
is named bdrv_write_zeroEs.
'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
'image': 'ImageInfo',
@@ -4250,6 +4253,20 @@
'data': [ 'ignore', 'unmap' ] }
##
+# @BlockdevDetectZeroesOptions
+#
+# Selects the operation mode for zero write detection.
s/Selects/Describes/ since you are also going to use this enum on the
output field.
Ok
+#
+# @off: Disabled
+# @on: Enabled
Maybe more details? Seeing this doesn't tell me the tradeoffs involved
in tweaking the parameter (one is faster, while the other uses less
storage resources - so maybe mention those benefits). I see the
documentation later on for the command line, so maybe repeating some of
that here would help someone going by just the QMP interface.
Will do.
Peter
+# @unmap: Enabled and even try to unmap blocks if possible
+#
+# Since: 2.1
+##
+{ 'enum': 'BlockdevDetectZeroesOptions',
+ 'data': [ 'off', 'on', 'unmap' ] }
+
+##
# @BlockdevAioOptions
#
# Selects the AIO backend to handle I/O requests
@@ -4327,7 +4346,8 @@
'*aio': 'BlockdevAioOptions',
'*rerror': 'BlockdevOnError',
'*werror': 'BlockdevOnError',
- '*read-only': 'bool' } }
+ '*read-only': 'bool',
+ '*detect-zeroes': 'BlockdevDetectZeroesOptions' } }
This part is fine.
@var{copy-on-read} is "on" or "off" and enables whether to copy read backing
file sectors into the image file.
address@hidden address@hidden
address@hidden is "off", "on" or "unmap" and enables the automatic
+conversion of plain zero writes by the OS to driver specific optimized
+zero write commands. If "unmap" is chosen and @var{discard} is "on"
+a zero write may even be converted to an UNMAP operation.
This looks good.
- [Qemu-devel] [PATCHv3] block: optimize zero writes with bdrv_write_zeroes, Peter Lieven, 2014/05/06
- [Qemu-devel] [PATCHv3] block: optimize zero writes with bdrv_write_zeroes, Peter Lieven, 2014/05/06
- Re: [Qemu-devel] [PATCHv3] block: optimize zero writes with bdrv_write_zeroes, Eric Blake, 2014/05/06
- Re: [Qemu-devel] [PATCHv3] block: optimize zero writes with bdrv_write_zeroes,
Peter Lieven <=
- Re: [Qemu-devel] [PATCHv3] block: optimize zero writes with bdrv_write_zeroes, Kevin Wolf, 2014/05/07
- Re: [Qemu-devel] [PATCHv3] block: optimize zero writes with bdrv_write_zeroes, Peter Lieven, 2014/05/07
- Re: [Qemu-devel] [PATCHv3] block: optimize zero writes with bdrv_write_zeroes, Kevin Wolf, 2014/05/07
- Re: [Qemu-devel] [PATCHv3] block: optimize zero writes with bdrv_write_zeroes, Peter Lieven, 2014/05/07
- Re: [Qemu-devel] [PATCHv3] block: optimize zero writes with bdrv_write_zeroes, Peter Lieven, 2014/05/07
- Re: [Qemu-devel] [PATCHv3] block: optimize zero writes with bdrv_write_zeroes, Kevin Wolf, 2014/05/08
- Re: [Qemu-devel] [PATCHv3] block: optimize zero writes with bdrv_write_zeroes, Peter Lieven, 2014/05/07