Re: [RFC 1/4] Add qapi for block error injection

[Philippe Mathieu-Daudé]

Hi Tony,

On 9/19/19 9:48 PM, Tony Asleson wrote:
> Proof of concept code to dynamically create/delete media errors
> for block devices using the qapi interface.  This is useful for testing
> the OS and the block and FS layers for error handling.  Utilizing this
> in a VM allows it to be OS agnostic and test the OS at it's lowest levels
> of hardware interaction.
> 
> Signed-off-by: Tony Asleson <address@hidden>
> ---
>  block/Makefile.objs  |   2 +-
>  block/error_inject.c | 179 +++++++++++++++++++++++++++++++++++++++++++
>  block/error_inject.h |  43 +++++++++++
>  block/qapi.c         |  18 +++++
>  qapi/block.json      |  36 +++++++++
>  5 files changed, 277 insertions(+), 1 deletion(-)
>  create mode 100644 block/error_inject.c
>  create mode 100644 block/error_inject.h
> 
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index 35f3bca4d9..c8fcbc276b 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -1,4 +1,4 @@
> -block-obj-y += raw-format.o vmdk.o vpc.o
> +block-obj-y += raw-format.o vmdk.o vpc.o error_inject.o
>  block-obj-$(CONFIG_QCOW1) += qcow.o
>  block-obj-$(CONFIG_VDI) += vdi.o
>  block-obj-$(CONFIG_CLOOP) += cloop.o
> diff --git a/block/error_inject.c b/block/error_inject.c
> new file mode 100644
> index 0000000000..26a47dfb8c
> --- /dev/null
> +++ b/block/error_inject.c
> @@ -0,0 +1,179 @@
> +/*
> + * Error injection code for block devices
> + *
> + * Copyright (c) 2019 Red Hat, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "error_inject.h"
> +
> +#include <gmodule.h>
> +
> +#include "qemu/thread.h"
> +
> +static QemuMutex error_inject_lock;
> +static GHashTable *error_inject_data;
> +
> +
> +static void delete_lba_entries(void *entry)
> +{
> +    GSequence *e = (GSequence *) entry;
> +    g_sequence_free(e);
> +}
> +
> +struct value {
> +    uint64_t error_lba;
> +
> +    /*
> +     * TODO Actually do something with behavior
> +     */
> +    MediaErrorBehavior behavior;
> +    /*
> +     * TODO Add data for generating bitrot, when we do change free function
> +     */
> +};
> +
> +static int key_compare(gconstpointer a, gconstpointer b, gpointer data)
> +{
> +    uint64_t left = ((struct value *)a)->error_lba;
> +    uint64_t right = ((struct value *)b)->error_lba;
> +
> +    if (left < right) {
> +        return -1;
> +    } else if (left > right) {
> +        return 1;
> +    } else {
> +        return 0;
> +    }
> +}
> +
> +static uint64_t error_lba_get(GSequenceIter *iter)
> +{
> +    gpointer tmp = g_sequence_get(iter);
> +    return ((struct value *)tmp)->error_lba;
> +}
> +
> +void media_error_create(const char *device_id, uint64_t lba,
> +                        MediaErrorBehavior behavior)
> +{
> +    qemu_mutex_lock(&error_inject_lock);
> +
> +    GSequence *block_device = g_hash_table_lookup(error_inject_data, 
> device_id);
> +    if (!block_device) {
> +        block_device = g_sequence_new(g_free);
> +        char *key = strdup(device_id);
> +        g_hash_table_insert(error_inject_data, key, block_device);
> +    }
> +
> +    struct value lookup = {lba, MEDIA_ERROR_BEHAVIOR__MAX};
> +    if (!g_sequence_lookup(block_device, &lookup, key_compare, NULL)) {
> +        struct value *val = g_new(struct value, 1);
> +        val->error_lba = lba;
> +        val->behavior = behavior;
> +
> +        g_sequence_insert_sorted(block_device, val, key_compare, NULL);
> +    }
> +
> +    qemu_mutex_unlock(&error_inject_lock);
> +}
> +
> +void media_error_delete(const char *device_id, uint64_t lba)

Can you explain the goal of this function? I don't understand it from
the hardware PoV. Or is it a simple code cleanup function?

Once hw starts to break, it is unlikely it recovers magically to a
previous sane state...

In real world, block hardware starts to fail, controller notice
inconsistency and set some bits, the controller driver react.
Eventually the hw will still be used, with the failing block avoided.

What do you think? How is your current use case setup?

Thanks,

Phil.

> +{
> +    qemu_mutex_lock(&error_inject_lock);
> +
> +    GSequence *block_device = g_hash_table_lookup(error_inject_data, 
> device_id);
> +    if (block_device) {
> +        struct value find = { lba, MEDIA_ERROR_BEHAVIOR__MAX};
> +        GSequenceIter *found = g_sequence_lookup(block_device, &find,
> +                                                 key_compare, NULL);
> +        if (found) {
> +            g_sequence_remove(found);
> +        }
> +    }
> +
> +    qemu_mutex_unlock(&error_inject_lock);
> +}
> +
> +int error_in_read(const char *device_id, uint64_t lba, uint64_t len,
> +                  uint64_t *error_lba)
> +{
> +    uint64_t error_sector = 0;
> +    const uint64_t transfer_end = lba + len;
> +    int ec = 0;
> +    *error_lba = 0xFFFFFFFFFFFFFFFF;
> +
> +    qemu_mutex_lock(&error_inject_lock);
> +
> +    GSequence *block_device = g_hash_table_lookup(error_inject_data, 
> device_id);
> +    if (block_device && g_sequence_get_length(block_device) != 0) {
> +        struct value find = {lba, MEDIA_ERROR_BEHAVIOR__MAX};
> +        GSequenceIter *iter = g_sequence_search(block_device, &find,
> +                                                key_compare, NULL);
> +
> +        /*
> +         * g_sequence_seach returns where the item would be inserted.
> +         * In the case of a direct match, it's spot is inserted after the
> +         * existing, thus we need to check the one immediately before
> +         * the insertion point to see if it is the one we are looking for.
> +         */
> +        GSequenceIter *prev = g_sequence_iter_prev(iter);
> +        error_sector = error_lba_get(prev);
> +
> +        if (error_sector >= lba && error_sector < transfer_end) {
> +            *error_lba = error_sector;
> +            ec = 1;
> +        } else {
> +            /*
> +             * Lets look at next until we find one in our transfer or bail
> +             * if the error(s) logical block address are greater than the
> +             * end of our transfer.
> +             */
> +            while (!g_sequence_iter_is_end(iter)) {
> +                error_sector = error_lba_get(iter);
> +
> +                if (error_sector >= transfer_end) {
> +                    break;
> +                }
> +                if (error_sector >= lba && error_sector < transfer_end) {
> +                    *error_lba = error_sector;
> +                    ec = 1;
> +                    break;
> +                } else {
> +                    iter = g_sequence_iter_next(iter);
> +                }
> +            }
> +        }
> +    }
> +
> +    qemu_mutex_unlock(&error_inject_lock);
> +
> +    return ec;
> +}
> +
> +
> +static void __attribute__((__constructor__)) error_inject_init(void)
> +{
> +    qemu_mutex_init(&error_inject_lock);
> +
> +    error_inject_data = g_hash_table_new_full(g_str_hash,
> +                                              g_str_equal,
> +                                              g_free,
> +                                              delete_lba_entries);
> +}
> diff --git a/block/error_inject.h b/block/error_inject.h
> new file mode 100644
> index 0000000000..5f2d143c90
> --- /dev/null
> +++ b/block/error_inject.h
> @@ -0,0 +1,43 @@
> +/*
> + * Error injection code for block devices
> + *
> + * Copyright (c) 2019 Red Hat, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#ifndef ERROR_INJECT
> +#define ERROR_INJECT
> +#include "qemu/osdep.h"
> +
> +#include <stdint.h>
> +
> +#include "qapi/qapi-commands-block.h"
> +#include "qapi/qapi-types-block.h"
> +
> +
> +void media_error_create(const char *device_id, uint64_t lba,
> +                        MediaErrorBehavior behavior);
> +void media_error_delete(const char *device_id, uint64_t lba);
> +
> +
> +int error_in_read(const char *device_id, uint64_t lba, uint64_t len,
> +                  uint64_t *error_lba);
> +
> +#endif
> diff --git a/block/qapi.c b/block/qapi.c
> index 15f1030264..d66201a831 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -26,11 +26,14 @@
>  #include "qemu-common.h"
>  #include "block/qapi.h"
>  #include "block/block_int.h"
> +#include "block/error_inject.h"
>  #include "block/throttle-groups.h"
>  #include "block/write-threshold.h"
>  #include "qapi/error.h"
> +#include "qapi/qapi-commands-block.h"
>  #include "qapi/qapi-commands-block-core.h"
>  #include "qapi/qobject-output-visitor.h"
> +#include "qapi/qapi-types-block.h"
>  #include "qapi/qapi-visit-block-core.h"
>  #include "qapi/qmp/qbool.h"
>  #include "qapi/qmp/qdict.h"
> @@ -841,3 +844,18 @@ void bdrv_image_info_dump(ImageInfo *info)
>          bdrv_image_info_specific_dump(info->format_specific);
>      }
>  }
> +
> +void qmp_media_error_create(const char *device_id, uint64_t lba,
> +        MediaErrorBehavior behavior, Error **errp)
> +{
> +    /*
> +     * We could validate the device_id and lba for existence and range, but 
> we
> +     * want to be able to add entries before a device is hot plugged too.
> +     */
> +    media_error_create(device_id, lba, behavior);
> +}
> +
> +void qmp_media_error_delete(const char *device_id, uint64_t lba, Error 
> **errp)
> +{
> +    media_error_delete(device_id, lba);
> +}
> diff --git a/qapi/block.json b/qapi/block.json
> index 145c268bb6..2b11954b44 100644
> --- a/qapi/block.json
> +++ b/qapi/block.json
> @@ -453,3 +453,39 @@
>  { 'event': 'QUORUM_REPORT_BAD',
>    'data': { 'type': 'QuorumOpType', '*error': 'str', 'node-name': 'str',
>              'sector-num': 'int', 'sectors-count': 'int' } }
> +
> +##
> +# @MediaErrorBehavior:
> +#
> +# Enumerated type for classifying type of read error behavior.
> +#
> +##
> +{ 'enum': 'MediaErrorBehavior', 'data': ['hard', 'fixed_on_write', 'flakey'] 
> }
> +
> +##
> +# @media-error-create:
> +#
> +# Example:
> +# -> {"execute": "media-error-create",
> +#     "arguments": {"device_id": "12345678, "lba" : 10000, "behavior" : 
> "hard"}}
> +# <- { "return": {} }
> +#
> +# Create a synthetic media error for the specified logical block address when
> +# it is read by the guest OS.
> +#
> +##
> +{ 'command':'media-error-create',
> +  'data': {'device_id': 'str', 'lba': 'uint64', 'behavior': 
> 'MediaErrorBehavior'}}
> +
> +##
> +# @media-error-delete:
> +#
> +# Delete a synthetic media error for the specified logical block address.
> +#
> +# # Example:
> +# -> {"execute": "media-error-delete",
> +#     "arguments": {"device_id": "01020304", "lba" : 10000}
> +# <- { "return": {} }
> +##
> +{ 'command':'media-error-delete',
> +  'data': {'device_id': 'str', 'lba': 'uint64'}}
>