Re: [Qemu-devel] [PATCHv3] block: optimize zero writes with bdrv_write_zeroes

[Kevin Wolf]

Am 07.05.2014 um 18:02 hat Peter Lieven geschrieben:
> On 07.05.2014 17:45, Peter Lieven wrote:
> >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 <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.
> >>>>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.
> >I am in blockdev_init and want to set dinfo->bdrv->detect_zeroes to
> >the correct ID. Do you have a fast solution? Everything else in this
> >function is not using QAPI calls.
> 
> Actually, there is already a manual parsing for the werror and rerror in this 
> function.
> 
> I think for the future there might be need for a generic function that maps a 
> string
> to the ID of an enum object.
> 
> If you don't have objections I would leave the parsing function as is for the 
> moment.

Ah, so this is not for actual QAPI code, but the command line. Yeah, I
guess that's okay then, at least for the moment.

Kevin