[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v1 02/18] block/pcache: empty pcache driver filt
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [PATCH v1 02/18] block/pcache: empty pcache driver filter |
Date: |
Wed, 23 Nov 2016 16:15:42 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 15.11.2016 um 07:36 hat Pavel Butsykin geschrieben:
> The basic version of pcache driver for easy preparation of a patch set.
>
> Signed-off-by: Pavel Butsykin <address@hidden>
> ---
> block/Makefile.objs | 1 +
> block/pcache.c | 144
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 145 insertions(+)
> create mode 100644 block/pcache.c
>
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index 7d4031d..c60f680 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -4,6 +4,7 @@ block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o
> qed-cluster.o
> block-obj-y += qed-check.o
> block-obj-y += vhdx.o vhdx-endian.o vhdx-log.o
> block-obj-y += quorum.o
> +block-obj-y += pcache.o
> block-obj-y += parallels.o blkdebug.o blkverify.o blkreplay.o
> block-obj-y += block-backend.o snapshot.o qapi.o
> block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
> diff --git a/block/pcache.c b/block/pcache.c
> new file mode 100644
> index 0000000..59461df
> --- /dev/null
> +++ b/block/pcache.c
> @@ -0,0 +1,144 @@
> +/*
> + * Prefetch cache driver filter
> + *
> + * Copyright (C) 2015-2016 Parallels IP Holdings GmbH. All rights reserved.
"All rights reserved" doesn't really agree with licensing under the GPL.
It would be preferable to remove this note to avoid any ambiguity.
> + *
> + * Author: Pavel Butsykin <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later. See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "block/block_int.h"
> +#include "qapi/error.h"
> +#include "qapi/qmp/qstring.h"
> +
> +
> +static QemuOptsList runtime_opts = {
> + .name = "pcache",
> + .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
> + .desc = {
> + {
> + .name = "x-image",
> + .type = QEMU_OPT_STRING,
> + .help = "[internal use only, will be removed]",
> + },
blkdebug/blkverify have this because they have to support legacy syntax
like -drive file=blkdebug:blkdebug.conf:test.img, i.e. it has to deal
with filenames.
Here we don't have to support a legacy syntax, so I would completely
avoid this from the beginning. You already support the "image" option,
which should be good enough.
> + { /* end of list */ }
> + },
> +};
> +
> +typedef struct PCacheAIOCB {
> + Coroutine *co;
> + int ret;
> +} PCacheAIOCB;
> +
> +static void pcache_aio_cb(void *opaque, int ret)
> +{
> + PCacheAIOCB *acb = opaque;
> +
> + acb->ret = ret;
> + qemu_coroutine_enter(acb->co);
> +}
> +
> +static coroutine_fn int pcache_co_preadv(BlockDriverState *bs, uint64_t
> offset,
> + uint64_t bytes, QEMUIOVector *qiov,
> + int flags)
> +{
> + PCacheAIOCB acb = {
> + .co = qemu_coroutine_self(),
> + };
> +
> + bdrv_aio_preadv(bs->file, offset, qiov, bytes, pcache_aio_cb, &acb);
> +
> + qemu_coroutine_yield();
> +
> + return acb.ret;
> +}
As I commented on patch 1, all of this can be replaced by a single line:
return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
> +static coroutine_fn int pcache_co_pwritev(BlockDriverState *bs, uint64_t
> offset,
> + uint64_t bytes, QEMUIOVector *qiov,
> + int flags)
> +{
> + PCacheAIOCB acb = {
> + .co = qemu_coroutine_self(),
> + };
> +
> + bdrv_aio_pwritev(bs->file, offset, qiov, bytes, pcache_aio_cb, &acb);
> +
> + qemu_coroutine_yield();
> +
> + return acb.ret;
> +}
Same here.
> +static int pcache_file_open(BlockDriverState *bs, QDict *options, int flags,
> + Error **errp)
> +{
> + QemuOpts *opts;
> + Error *local_err = NULL;
> + int ret = 0;
> +
> + opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
> + qemu_opts_absorb_qdict(opts, options, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + ret = -EINVAL;
> + goto fail;
> + }
> +
> + assert(bs->file == NULL);
> + bs->file = bdrv_open_child(qemu_opt_get(opts, "x-image"), options,
> + "image", bs, &child_format, false,
> &local_err);
When removing x-image, the first parameter becomes NULL here.
> + if (local_err) {
> + ret = -EINVAL;
> + error_propagate(errp, local_err);
> + }
> +fail:
> + qemu_opts_del(opts);
> + return ret;
> +}
> +
> +static void pcache_close(BlockDriverState *bs)
> +{
> +}
> +
> +static void pcache_parse_filename(const char *filename, QDict *options,
> + Error **errp)
> +{
> + qdict_put(options, "x-image", qstring_from_str(filename));
> +}
This one goes away.
> +static int64_t pcache_getlength(BlockDriverState *bs)
> +{
> + return bdrv_getlength(bs->file->bs);
> +}
> +
> +static bool pcache_recurse_is_first_non_filter(BlockDriverState *bs,
> + BlockDriverState *candidate)
> +{
> + return bdrv_recurse_is_first_non_filter(bs->file->bs, candidate);
> +}
> +
> +static BlockDriver bdrv_pcache = {
> + .format_name = "pcache",
> + .protocol_name = "pcache",
Remove this one, it would only be used for something like
"pcache:<filename>" syntax.
> + .instance_size = 0,
> +
> + .bdrv_parse_filename = pcache_parse_filename,
> + .bdrv_file_open = pcache_file_open,
> + .bdrv_close = pcache_close,
> + .bdrv_getlength = pcache_getlength,
> +
> + .bdrv_co_preadv = pcache_co_preadv,
> + .bdrv_co_pwritev = pcache_co_pwritev,
> +
> + .is_filter = true,
> + .bdrv_recurse_is_first_non_filter = pcache_recurse_is_first_non_filter,
> +};
> +
> +static void bdrv_cache_init(void)
> +{
> + bdrv_register(&bdrv_pcache);
> +}
> +
> +block_init(bdrv_cache_init);
You should also add pcache to the QAPI schema, so that blockdev-add can
create instances of it.
Kevin
- [Qemu-block] [PATCH v1 13/18] block/pcache: inflight readahead request waiting for aio read, (continued)
- [Qemu-block] [PATCH v1 13/18] block/pcache: inflight readahead request waiting for aio read, Pavel Butsykin, 2016/11/15
- [Qemu-block] [PATCH v1 10/18] block/pcache: skip readahead for unallocated clusters, Pavel Butsykin, 2016/11/15
- [Qemu-block] [PATCH v1 05/18] tests/test-rbcache: add test cases, Pavel Butsykin, 2016/11/15
- [Qemu-block] [PATCH v1 14/18] backup/pcache: pick up parts of the cache, Pavel Butsykin, 2016/11/15
- [Qemu-block] [PATCH v1 08/18] block/pcache: updating statistics for overlapping requests, Pavel Butsykin, 2016/11/15
- [Qemu-block] [PATCH v1 12/18] block/pcache: add reading data from the cache, Pavel Butsykin, 2016/11/15
- [Qemu-block] [PATCH v1 03/18] util/rbtree: add rbtree from linux kernel, Pavel Butsykin, 2016/11/15
- [Qemu-block] [PATCH v1 06/18] block/pcache: statistics collection read requests, Pavel Butsykin, 2016/11/15
- [Qemu-block] [PATCH v1 02/18] block/pcache: empty pcache driver filter, Pavel Butsykin, 2016/11/15
- Re: [Qemu-block] [PATCH v1 02/18] block/pcache: empty pcache driver filter,
Kevin Wolf <=
- [Qemu-block] [PATCH v1 11/18] block/pcache: cache invalidation on AIO write requests, Pavel Butsykin, 2016/11/15
- [Qemu-block] [PATCH v1 15/18] block/pcache: drop used pcache nodes, Pavel Butsykin, 2016/11/15
- [Qemu-block] [PATCH v1 17/18] block/pcache: add tracepoints, Pavel Butsykin, 2016/11/15
- [Qemu-block] [PATCH v1 18/18] block/pcache: debug build, Pavel Butsykin, 2016/11/15
- [Qemu-block] [PATCH v1 01/18] block/io: add bdrv_aio_{preadv, pwritev}, Pavel Butsykin, 2016/11/15