From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53758) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLIJD-0001hO-E0 for qemu-devel@nongnu.org; Wed, 05 Mar 2014 15:21:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WLIJ8-0002B2-KW for qemu-devel@nongnu.org; Wed, 05 Mar 2014 15:20:55 -0500 Received: from lnantes-156-75-100-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:55785 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLIJ8-0002Am-1U for qemu-devel@nongnu.org; Wed, 05 Mar 2014 15:20:50 -0500 Date: Wed, 5 Mar 2014 21:20:48 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140305202047.GA4514@irqsave.net> References: <1393860533-2063-1-git-send-email-mreitz@redhat.com> <1393860533-2063-3-git-send-email-mreitz@redhat.com> <20140305160455.GD1709@irqsave.net> <531781D4.40800@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <531781D4.40800@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 02/10] block/json: Add JSON protocol driver List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: =?iso-8859-1?Q?Beno=EEt?= Canet , Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi The Wednesday 05 Mar 2014 =E0 20:58:12 (+0100), Max Reitz wrote : > On 05.03.2014 17:04, Beno=EEt Canet wrote: > >The Monday 03 Mar 2014 =E0 16:28:45 (+0100), Max Reitz wrote : > >>Add a JSON protocol driver which allows supplying block driver option= s > >>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 > >>--- > >> 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 +=3D qed-check.o > >> block-obj-$(CONFIG_VHDX) +=3D vhdx.o vhdx-endian.o vhdx-log.o > >> block-obj-$(CONFIG_QUORUM) +=3D quorum.o > >> block-obj-y +=3D parallels.o blkdebug.o blkverify.o > >>-block-obj-y +=3D snapshot.o qapi.o > >>+block-obj-y +=3D snapshot.o qapi.o json.o > >> block-obj-$(CONFIG_WIN32) +=3D raw-win32.o win32-aio.o > >> block-obj-$(CONFIG_POSIX) +=3D raw-posix.o > >> block-obj-$(CONFIG_LINUX_AIO) +=3D 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 mod= ify > >>+ * 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 . > >>+ */ > >>+ > >>+#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 dri= ver"); > >>+ return; > >>+ } > >>+ > >>+ file_options_obj =3D qobject_from_json(filename); > >>+ if (!file_options_obj) { > >>+ error_setg(errp, "Could not parse the JSON options"); > >>+ return; > >>+ } > >>+ > >>+ if (qobject_type(file_options_obj) !=3D QTYPE_QDICT) { > >>+ qobject_decref(file_options_obj); > >>+ error_setg(errp, "Invalid JSON object"); > >>+ return; > >>+ } > >>+ > >>+ full_options =3D 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) =3D=3D 0); > >>+ QDECREF(full_options); > >>+} > >>+ > >>+static int json_open(BlockDriverState *bs, QDict *options, int flags= , > >>+ Error **errp) > >>+{ > >>+ int ret; > >>+ > >>+ assert(bs->file =3D=3D NULL); > >>+ ret =3D bdrv_open_image(&bs->file, NULL, options, "x-options", f= lags, false, > >>+ errp); > > > >>+ if (ret < 0) { > >>+ return ret; > >>+ } > >>+ > >>+ return 0; > >Why not ? > > return ret; > >} > > > >Do you plan to enrich this code path later ? >=20 > You're right, that'd be shorter. It's just that I'm so in the habit > of writing "ret =3D foo(); if (ret < 0) { return ret; / goto fail; }" > that I didn't notice. ;-) >=20 > >>+} > >>+ > >>+static void json_close(BlockDriverState *bs) > >>+{ > >>+} > >>+ > >>+static BlockDriverAIOCB *json_aio_readv(BlockDriverState *bs, > >>+ int64_t sector_num, QEMUIOVe= ctor *qiov, > >>+ int nb_sectors, > >>+ BlockDriverCompletionFunc *c= b, > >>+ 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, QEMUIOV= ector *qiov, > >>+ int nb_sectors, > >>+ BlockDriverCompletionFunc *= cb, > >>+ void *opaque) > >>+{ > >>+ return bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, c= b, opaque); > >>+} > >>+ > >>+static int64_t json_getlength(BlockDriverState *bs) > >>+{ > >>+ return bdrv_getlength(bs->file); > >>+} > >>+ > >>+static int json_refresh_limits(BlockDriverState *bs) > >>+{ > >>+ bs->bl =3D 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 =3D { > >>+ .format_name =3D "json", > >>+ .protocol_name =3D "json", > >>+ .instance_size =3D 0, > >>+ > >>+ .bdrv_parse_filename =3D json_parse_filename, > >>+ .bdrv_file_open =3D json_open, > >>+ .bdrv_close =3D json_close, > >>+ > >>+ .bdrv_aio_readv =3D json_aio_readv, > >>+ .bdrv_aio_writev =3D json_aio_writev, > >>+ > >>+ .has_variable_length =3D true, > >>+ .bdrv_getlength =3D json_getlength, > >>+ > >>+ .bdrv_refresh_limits =3D json_refresh_limits, > >>+ .bdrv_get_info =3D json_get_info, > >>+ > >>+ .authorizations =3D { true, true }, > >Paolo make rewrite the snapshot authorization code. > >In the new version you need > > .is_filter =3D true; > > .bdrv_recurse_is_first_children =3D your_implementation; > > > >I don't know when these patchset will be reviewed and merged so maybe = this > >version is correct. >=20 > 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" >=20 >=20 > Thanks, >=20 > Max >=20 > >>+}; > >>+ > >>+static void bdrv_json_init(void) > >>+{ > >>+ bdrv_register(&bdrv_json); > >>+} > >>+ > >>+block_init(bdrv_json_init); > >>--=20 > >>1.9.0 > >> > >> >=20 >=20