Re: [PATCH] block: Introduce zero-co:// and zero-aio://

[Philippe Mathieu-Daudé]

On 3/10/21 3:17 PM, fam@euphon.net wrote:
> From: Fam Zheng <famzheng@amazon.com>
> 
> null-co:// has a read-zeroes=off default, when used to in security
> analysis, this can cause false positives because the driver doesn't
> write to the read buffer.
> 
> null-co:// has the highest possible performance as a block driver, so
> let's keep it that way. This patch introduces zero-co:// and
> zero-aio://, largely similar with null-*://, but have read-zeroes=on by
> default, so it's more suitable in cases than null-co://.

Thanks!

> 
> Signed-off-by: Fam Zheng <fam@euphon.net>
> ---
>  block/null.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 91 insertions(+)

> +static int zero_file_open(BlockDriverState *bs, QDict *options, int flags,
> +                          Error **errp)
> +{
> +    QemuOpts *opts;
> +    BDRVNullState *s = bs->opaque;
> +    int ret = 0;
> +
> +    opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
> +    qemu_opts_absorb_qdict(opts, options, &error_abort);
> +    s->length =
> +        qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 1 << 30);

Please do not use this magic default value, let's always explicit the
size.

    s->length = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0);
    if (s->length < 0) {
        error_setg(errp, "%s is invalid", BLOCK_OPT_SIZE);
        ret = -EINVAL;
    }

> +    s->latency_ns =
> +        qemu_opt_get_number(opts, NULL_OPT_LATENCY, 0);
> +    if (s->latency_ns < 0) {
> +        error_setg(errp, "latency-ns is invalid");
> +        ret = -EINVAL;
> +    }
> +    s->read_zeroes = true;
> +    qemu_opts_del(opts);
> +    bs->supported_write_flags = BDRV_REQ_FUA;
> +    return ret;
> +}

Otherwise LGTM :)