[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 02/10] block/json: Add JSON protocol driver
From: |
Benoît Canet |
Subject: |
Re: [Qemu-devel] [PATCH 02/10] block/json: Add JSON protocol driver |
Date: |
Wed, 5 Mar 2014 22:18:53 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
The Wednesday 05 Mar 2014 à 21:21:46 (+0100), Max Reitz wrote :
> On 05.03.2014 21:20, Benoît Canet wrote:
> >The Wednesday 05 Mar 2014 à 20:58:12 (+0100), Max Reitz wrote :
> >>On 05.03.2014 17:04, Benoît Canet wrote:
> >>>The Monday 03 Mar 2014 à 16:28:45 (+0100), Max Reitz wrote :
> >>>>Add a JSON protocol driver which allows supplying block driver options
> >>>>through the filename rather than separately. Other than that, it is a
> >>>>pure passthrough driver which identifies itself as a filter.
> >>>>
> >>>>This patch implements the functions bdrv_parse_filename(),
> >>>>bdrv_file_open(), bdrv_close(), bdrv_aio_readv(), bdrv_aio_writev(),
> >>>>bdrv_getlength(), bdrv_refresh_limits() and bdrv_get_info().
> >>>>
> >>>>Signed-off-by: Max Reitz <address@hidden>
> >>>>---
> >>>> block/Makefile.objs | 2 +-
> >>>> block/json.c | 136
> >>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>> 2 files changed, 137 insertions(+), 1 deletion(-)
> >>>> create mode 100644 block/json.c
> >>>>
> >>>>diff --git a/block/Makefile.objs b/block/Makefile.objs
> >>>>index fd88c03..d4b70f4 100644
> >>>>--- a/block/Makefile.objs
> >>>>+++ b/block/Makefile.objs
> >>>>@@ -5,7 +5,7 @@ block-obj-y += qed-check.o
> >>>> block-obj-$(CONFIG_VHDX) += vhdx.o vhdx-endian.o vhdx-log.o
> >>>> block-obj-$(CONFIG_QUORUM) += quorum.o
> >>>> block-obj-y += parallels.o blkdebug.o blkverify.o
> >>>>-block-obj-y += snapshot.o qapi.o
> >>>>+block-obj-y += snapshot.o qapi.o json.o
> >>>> block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
> >>>> block-obj-$(CONFIG_POSIX) += raw-posix.o
> >>>> block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
> >>>>diff --git a/block/json.c b/block/json.c
> >>>>new file mode 100644
> >>>>index 0000000..6d63cf6
> >>>>--- /dev/null
> >>>>+++ b/block/json.c
> >>>>@@ -0,0 +1,136 @@
> >>>>+/*
> >>>>+ * JSON filename wrapper protocol driver
> >>>>+ *
> >>>>+ * Copyright (c) 2014 Red Hat Inc.
> >>>>+ *
> >>>>+ * This program is free software; you can redistribute it and/or modify
> >>>>+ * it under the terms of the GNU General Public License as published by
> >>>>+ * the Free Software Foundation; either version 2 of the License, or
> >>>>+ * (at your option) any later version.
> >>>>+ *
> >>>>+ * This program is distributed in the hope that it will be useful,
> >>>>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>>>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >>>>+ * GNU General Public License for more details.
> >>>>+ *
> >>>>+ * You should have received a copy of the GNU General Public License
> >>>>+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
> >>>>+ */
> >>>>+
> >>>>+#include "qemu-common.h"
> >>>>+#include "block/block_int.h"
> >>>>+#include "qapi/qmp/qdict.h"
> >>>>+#include "qapi/qmp/qjson.h"
> >>>>+
> >>>>+static void json_parse_filename(const char *filename, QDict *options,
> >>>>+ Error **errp)
> >>>>+{
> >>>>+ QObject *file_options_obj;
> >>>>+ QDict *full_options;
> >>>>+
> >>>>+ if (!strstart(filename, "json:", &filename)) {
> >>>>+ error_setg(errp, "Unknown protocol prefix for JSON block
> >>>>driver");
> >>>>+ return;
> >>>>+ }
> >>>>+
> >>>>+ file_options_obj = qobject_from_json(filename);
> >>>>+ if (!file_options_obj) {
> >>>>+ error_setg(errp, "Could not parse the JSON options");
> >>>>+ return;
> >>>>+ }
> >>>>+
> >>>>+ if (qobject_type(file_options_obj) != QTYPE_QDICT) {
> >>>>+ qobject_decref(file_options_obj);
> >>>>+ error_setg(errp, "Invalid JSON object");
> >>>>+ return;
> >>>>+ }
> >>>>+
> >>>>+ full_options = qdict_new();
> >>>>+ qdict_put_obj(full_options, "x-options", file_options_obj);
> >>>>+ qdict_flatten(full_options);
> >>>>+
> >>>>+ qdict_join(options, full_options, true);
> >>>>+ assert(qdict_size(full_options) == 0);
> >>>>+ QDECREF(full_options);
> >>>>+}
> >>>>+
> >>>>+static int json_open(BlockDriverState *bs, QDict *options, int flags,
> >>>>+ Error **errp)
> >>>>+{
> >>>>+ int ret;
> >>>>+
> >>>>+ assert(bs->file == NULL);
> >>>>+ ret = bdrv_open_image(&bs->file, NULL, options, "x-options", flags,
> >>>>false,
> >>>>+ errp);
> >>>>+ if (ret < 0) {
> >>>>+ return ret;
> >>>>+ }
> >>>>+
> >>>>+ return 0;
> >>>Why not ?
> >>> return ret;
> >>>}
> >>>
> >>>Do you plan to enrich this code path later ?
> >>You're right, that'd be shorter. It's just that I'm so in the habit
> >>of writing "ret = foo(); if (ret < 0) { return ret; / goto fail; }"
> >>that I didn't notice. ;-)
> >>
> >>>>+}
> >>>>+
> >>>>+static void json_close(BlockDriverState *bs)
> >>>>+{
> >>>>+}
> >>>>+
> >>>>+static BlockDriverAIOCB *json_aio_readv(BlockDriverState *bs,
> >>>>+ int64_t sector_num, QEMUIOVector
> >>>>*qiov,
> >>>>+ int nb_sectors,
> >>>>+ BlockDriverCompletionFunc *cb,
> >>>>+ void *opaque)
> >>>>+{
> >>>>+ return bdrv_aio_readv(bs->file, sector_num, qiov, nb_sectors, cb,
> >>>>opaque);
> >>>>+}
> >>>>+
> >>>>+static BlockDriverAIOCB *json_aio_writev(BlockDriverState *bs,
> >>>>+ int64_t sector_num,
> >>>>QEMUIOVector *qiov,
> >>>>+ int nb_sectors,
> >>>>+ BlockDriverCompletionFunc *cb,
> >>>>+ void *opaque)
> >>>>+{
> >>>>+ return bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb,
> >>>>opaque);
> >>>>+}
> >>>>+
> >>>>+static int64_t json_getlength(BlockDriverState *bs)
> >>>>+{
> >>>>+ return bdrv_getlength(bs->file);
> >>>>+}
> >>>>+
> >>>>+static int json_refresh_limits(BlockDriverState *bs)
> >>>>+{
> >>>>+ bs->bl = bs->file->bl;
> >>>>+ return 0;
> >>>>+}
> >>>>+
> >>>>+static int json_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
> >>>>+{
> >>>>+ return bdrv_get_info(bs->file, bdi);
> >>>>+}
> >>>>+
> >>>>+static BlockDriver bdrv_json = {
> >>>>+ .format_name = "json",
> >>>>+ .protocol_name = "json",
> >>>>+ .instance_size = 0,
> >>>>+
> >>>>+ .bdrv_parse_filename = json_parse_filename,
> >>>>+ .bdrv_file_open = json_open,
> >>>>+ .bdrv_close = json_close,
> >>>>+
> >>>>+ .bdrv_aio_readv = json_aio_readv,
> >>>>+ .bdrv_aio_writev = json_aio_writev,
> >>>>+
> >>>>+ .has_variable_length = true,
> >>>>+ .bdrv_getlength = json_getlength,
> >>>>+
> >>>>+ .bdrv_refresh_limits = json_refresh_limits,
> >>>>+ .bdrv_get_info = json_get_info,
> >>>>+
> >>>>+ .authorizations = { true, true },
> >>>Paolo make rewrite the snapshot authorization code.
> >>>In the new version you need
> >>> .is_filter = true;
> >>> .bdrv_recurse_is_first_children = your_implementation;
> >>>
> >>>I don't know when these patchset will be reviewed and merged so maybe this
> >>>version is correct.
> >>I'll just rebase on top of Paolo's series then.
> >It's a sugestion made by Paolo. I wrote the actual code.
> >The series is :
> >"[PATCH] Rewrite block filter snapshot authorization mechanism"
>
> Okay, thank you again. :-)
>
> Max
>
> >>
> >>Thanks,
> >>
> >>Max
> >>
> >>>>+};
> >>>>+
> >>>>+static void bdrv_json_init(void)
> >>>>+{
> >>>>+ bdrv_register(&bdrv_json);
> >>>>+}
> >>>>+
> >>>>+block_init(bdrv_json_init);
> >>>>--
> >>>>1.9.0
> >>>>
> >>>>
> >>
>
>
I have a request: if you have some spare bandwith could you please review.
[QEMU 2.0 Fix] Fix bdrv_swap behavior regarding the node graph
and
[PATCH 0/2] Allow to repair broken quorum files
The first one is a fix that should go in 2.0.
The second would render Quorum usable in production so I would like to make it
merge before the 2.0 hard freeze if it's doable.
Best regards
Benoît
[Qemu-devel] [PATCH 02/10] block/json: Add JSON protocol driver, Max Reitz, 2014/03/03
[Qemu-devel] [PATCH 04/10] block/json: Add functions for writing zeroes etc., Max Reitz, 2014/03/03
[Qemu-devel] [PATCH 05/10] block/json: Add bdrv_co_get_block_status(), Max Reitz, 2014/03/03
- Re: [Qemu-devel] [PATCH 05/10] block/json: Add bdrv_co_get_block_status(), Benoît Canet, 2014/03/05
- Re: [Qemu-devel] [PATCH 05/10] block/json: Add bdrv_co_get_block_status(), Max Reitz, 2014/03/05
- Re: [Qemu-devel] [PATCH 05/10] block/json: Add bdrv_co_get_block_status(), Benoît Canet, 2014/03/05
- Re: [Qemu-devel] [PATCH 05/10] block/json: Add bdrv_co_get_block_status(), Max Reitz, 2014/03/05
- Re: [Qemu-devel] [PATCH 05/10] block/json: Add bdrv_co_get_block_status(), Benoît Canet, 2014/03/05
- Re: [Qemu-devel] [PATCH 05/10] block/json: Add bdrv_co_get_block_status(), Max Reitz, 2014/03/06