10.03.2021 17:17, 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://.
>
> Signed-off-by: Fam Zheng <fam@euphon.net>
> ---
> block/null.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 91 insertions(+)
>
> diff --git a/block/null.c b/block/null.c
> index cc9b1d4ea7..5de97e8fda 100644
> --- a/block/null.c
> +++ b/block/null.c
> @@ -76,6 +76,30 @@ static void null_aio_parse_filename(const char *filename, QDict *options,
> }
> }
>
> +static void zero_co_parse_filename(const char *filename, QDict *options,
> + Error **errp)
> +{
> + /* This functions only exists so that a zero-co:// filename is accepted
> + * with the zero-co driver. */
> + if (strcmp(filename, "zero-co://")) {
> + error_setg(errp, "The only allowed filename for this driver is "
> + "'zero-co://'");
> + return;
> + }
> +}
> +
> +static void zero_aio_parse_filename(const char *filename, QDict *options,
> + Error **errp)
> +{
> + /* This functions only exists so that a zero-aio:// filename is accepted
> + * with the zero-aio driver. */
> + if (strcmp(filename, "zero-aio://")) {
> + error_setg(errp, "The only allowed filename for this driver is "
> + "'zero-aio://'");
> + return;
> + }
> +}
> +
> static int null_file_open(BlockDriverState *bs, QDict *options, int flags,
> Error **errp)
> {
> @@ -99,6 +123,29 @@ static int null_file_open(BlockDriverState *bs, QDict *options, int flags,
> return ret;
> }
>
> +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);
> + 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;
> +}
> +
> static int64_t null_getlength(BlockDriverState *bs)
> {
> BDRVNullState *s = bs->opaque;
> @@ -316,10 +363,54 @@ static BlockDriver bdrv_null_aio = {
> .strong_runtime_opts = null_strong_runtime_opts,
> };
>
> +static BlockDriver bdrv_zero_co = {
> + .format_name = "zero-co",
> + .protocol_name = "zero-co",
> + .instance_size = sizeof(BDRVNullState),
> +
> + .bdrv_file_open = zero_file_open,
> + .bdrv_parse_filename = zero_co_parse_filename,
> + .bdrv_getlength = null_getlength,
> + .bdrv_get_allocated_file_size = null_allocated_file_size,
> +
> + .bdrv_co_preadv = null_co_preadv,
> + .bdrv_co_pwritev = null_co_pwritev,
> + .bdrv_co_flush_to_disk = null_co_flush,
> + .bdrv_reopen_prepare = null_reopen_prepare,
> +
> + .bdrv_co_block_status = null_co_block_status,
> +
> + .bdrv_refresh_filename = null_refresh_filename,
> + .strong_runtime_opts = null_strong_runtime_opts,
> +};
> +
> +static BlockDriver bdrv_zero_aio = {
> + .format_name = "zero-aio",
> + .protocol_name = "zero-aio",
> + .instance_size = sizeof(BDRVNullState),
> +
> + .bdrv_file_open = zero_file_open,
> + .bdrv_parse_filename = zero_aio_parse_filename,
> + .bdrv_getlength = null_getlength,
> + .bdrv_get_allocated_file_size = null_allocated_file_size,
> +
> + .bdrv_aio_preadv = null_aio_preadv,
> + .bdrv_aio_pwritev = null_aio_pwritev,
> + .bdrv_aio_flush = null_aio_flush,
> + .bdrv_reopen_prepare = null_reopen_prepare,
> +
> + .bdrv_co_block_status = null_co_block_status,
> +
> + .bdrv_refresh_filename = null_refresh_filename,
> + .strong_runtime_opts = null_strong_runtime_opts,
> +};
I don't think we need separate zero-aio driver. What is the actual difference?
Probably zero-co is enough, and we can call it just zero. And then, maybe add null driver (same as null-co, but without read_zeroes option) and deprecate null-co and null-aio drivers. Thus we'll get two clean well defined things: null and zero drivers..
Sounds good to me, I'll just call it zero:// for this patch and leave the null convergence and deprecation business for another patch.
Fam
> +
> static void bdrv_null_init(void)
> {
> bdrv_register(&bdrv_null_co);
> bdrv_register(&bdrv_null_aio);
> + bdrv_register(&bdrv_zero_co);
> + bdrv_register(&bdrv_zero_aio);
> }
>
> block_init(bdrv_null_init);
>
--
Best regards,
Vladimir