diff mbox

[02/10] block/json: Add JSON protocol driver

Message ID 1393860533-2063-3-git-send-email-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz March 3, 2014, 3:28 p.m. UTC
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 <mreitz@redhat.com>
---
 block/Makefile.objs |   2 +-
 block/json.c        | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 137 insertions(+), 1 deletion(-)
 create mode 100644 block/json.c

Comments

Benoît Canet March 5, 2014, 4:04 p.m. UTC | #1
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 <mreitz@redhat.com>
> ---
>  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 ?


> +}
> +
> +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.

> +};
> +
> +static void bdrv_json_init(void)
> +{
> +    bdrv_register(&bdrv_json);
> +}
> +
> +block_init(bdrv_json_init);
> -- 
> 1.9.0
> 
>
Max Reitz March 5, 2014, 7:58 p.m. UTC | #2
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 <mreitz@redhat.com>
>> ---
>>   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.


Thanks,

Max

>> +};
>> +
>> +static void bdrv_json_init(void)
>> +{
>> +    bdrv_register(&bdrv_json);
>> +}
>> +
>> +block_init(bdrv_json_init);
>> -- 
>> 1.9.0
>>
>>
Benoît Canet March 5, 2014, 8:20 p.m. UTC | #3
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 <mreitz@redhat.com>
> >>---
> >>  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"

> 
> 
> Thanks,
> 
> Max
> 
> >>+};
> >>+
> >>+static void bdrv_json_init(void)
> >>+{
> >>+    bdrv_register(&bdrv_json);
> >>+}
> >>+
> >>+block_init(bdrv_json_init);
> >>-- 
> >>1.9.0
> >>
> >>
> 
>
Max Reitz March 5, 2014, 8:21 p.m. UTC | #4
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 <mreitz@redhat.com>
>>>> ---
>>>>   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
>>>>
>>>>
>>
Benoît Canet March 5, 2014, 9:18 p.m. UTC | #5
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 <mreitz@redhat.com>
> >>>>---
> >>>>  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
diff mbox

Patch

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;
+}
+
+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 },
+};
+
+static void bdrv_json_init(void)
+{
+    bdrv_register(&bdrv_json);
+}
+
+block_init(bdrv_json_init);