From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55183) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wi4TL-0008DI-Pw for qemu-devel@nongnu.org; Wed, 07 May 2014 12:13:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wi4TG-0000AE-34 for qemu-devel@nongnu.org; Wed, 07 May 2014 12:13:31 -0400 Received: from mx-v6.kamp.de ([2a02:248:0:51::16]:34869 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wi4TF-00009y-Ns for qemu-devel@nongnu.org; Wed, 07 May 2014 12:13:25 -0400 Message-ID: <536A5B96.3090402@kamp.de> Date: Wed, 07 May 2014 18:13:10 +0200 From: Peter Lieven MIME-Version: 1.0 References: <1399422203-5861-1-git-send-email-pl@kamp.de> <53699FE4.5030004@redhat.com> <536A42B0.1070307@kamp.de> <20140507151938.GG4045@noname.str.redhat.com> <536A50A2.1020008@kamp.de> <20140507153853.GH4045@noname.str.redhat.com> In-Reply-To: <20140507153853.GH4045@noname.str.redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv3] block: optimize zero writes with bdrv_write_zeroes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: famz@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com On 07.05.2014 17:38, Kevin Wolf wrote: > Am 07.05.2014 um 17:26 hat Peter Lieven geschrieben: >> On 07.05.2014 17:19, Kevin Wolf wrote: >>> Am 07.05.2014 um 16:26 hat Peter Lieven geschrieben: >>>> 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 >>>>>> --- >>>>>> 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. >>> QAPI does generate C enums, so you should take whatever identifier it >>> uses instead of defining your own macros. You may need to include >>> qapi-types.h for this. It also creates a *_lookup array that maps enum >>> IDs to strings. >> Ah, cool stuff, thank you. I found the enum and the lookup array, >> but is there also a function that maps a string to an enum ID? > I don't think so, but if you need it, you're probably doing something > wrong because QAPI already calls you with an enum parameter and not a > char* one. > > Kevin This works for me: diff --git a/blockdev.c b/blockdev.c index 7810e9f..955bd49 100644 --- a/blockdev.c +++ b/blockdev.c @@ -288,6 +288,23 @@ static int parse_block_error_action(const char *buf, bool is_read, Error **errp) } } +static int parse_enum_option(const char *lookup[], const char *buf, int max, + int def, Error **errp) +{ + int i; + if (!buf) { + return def; + } + for (i = 0; i < max; i++) { + if (!strcmp(buf, lookup[i])) { + return i; + } + } + error_setg(errp, "invalid parameter value: %s", + buf); + return def; +} + static bool check_throttle_config(ThrottleConfig *cfg, Error **errp) { if (throttle_conflicting(cfg)) { @@ -324,6 +341,7 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts, QemuOpts *opts; const char *id; bool has_driver_specific_opts; + BlockdevDetectZeroesOptions detect_zeroes; BlockDriver *drv = NULL; /* Check common options by copying from bs_opts to opts, all other options @@ -452,6 +470,17 @@ static DriveInfo *blockdev_init(const char *file, QDict *bs_opts, } } + detect_zeroes = + parse_enum_option(BlockdevDetectZeroesOptions_lookup, + qemu_opt_get(opts, "detect-zeroes"), + BLOCKDEV_DETECT_ZEROES_OPTIONS_MAX, + BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF, + &error); + if (error) { + error_propagate(errp, error); + goto early_err; + } + /* init */ dinfo = g_malloc0(sizeof(*dinfo)); dinfo->id = g_strdup(qemu_opts_id(opts));