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;
}
Doesn't that result in a bare
-blockdev zero-co://
would have 0 byte in size?
I'm copying what null-co:// does today, and it's convenient for tests. Why is a default size bad?
Fam
> + 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 :)