From: Max Reitz <mreitz@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
"qemu-block@nongnu.org" <qemu-block@nongnu.org>
Cc: Kevin Wolf <kwolf@redhat.com>, John Snow <jsnow@redhat.com>,
"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
Markus Armbruster <armbru@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 2/4] blkdebug: Allow taking/unsharing permissions
Date: Thu, 19 Sep 2019 18:49:17 +0200 [thread overview]
Message-ID: <84586806-87cf-0024-f583-d4ef9fde2212@redhat.com> (raw)
In-Reply-To: <79ab5656-5845-2c4c-6802-2be22baf56d9@virtuozzo.com>
[-- Attachment #1.1: Type: text/plain, Size: 4947 bytes --]
On 18.09.19 18:01, Vladimir Sementsov-Ogievskiy wrote:
> 12.09.2019 16:56, Max Reitz wrote:
>> Sometimes it is useful to be able to add a node to the block graph that
>> takes or unshare a certain set of permissions for debugging purposes.
>> This patch adds this capability to blkdebug.
>>
>> (Note that you cannot make blkdebug release or share permissions that it
>> needs to take or cannot share, because this might result in assertion
>> failures in the block layer. But if the blkdebug node has no parents,
>> it will not take any permissions and share everything by default, so you
>> can then freely choose what permissions to take and share.)
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> qapi/block-core.json | 29 +++++++++++-
>> block/blkdebug.c | 106 ++++++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 133 insertions(+), 2 deletions(-)
>>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index e6edd641f1..336043e02c 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -3347,6 +3347,21 @@
>> '*state': 'int',
>> 'new_state': 'int' } }
>>
>> +##
>> +# @BlockdevPermission:
>> +#
>> +# Permissions that an edge in the block graph can take or share.
>> +#
>> +# Since: 4.2
>> +##
>> +{ 'enum': 'BlockdevPermission',
>> + 'data': [
>> + 'consistent-read',
>> + 'write',
>> + 'write-unchanged',
>> + 'resize',
>> + 'graph-mod' ] }
>
> :)
>
> BlockPermission is already here since 4.0 and has exactly same content. (And better documented)
Oops. Very good, then, thanks. :-)
[...]
>> @@ -344,6 +349,84 @@ static void blkdebug_parse_filename(const char *filename, QDict *options,
>> qdict_put_str(options, "x-image", filename);
>> }
>>
>> +static int blkdebug_parse_perm_list(uint64_t *dest, QDict *options,
>> + const char *prefix, Error **errp)
>> +{
>> + int ret = 0;
>> + QDict *subqdict = NULL;
>> + QObject *crumpled_subqdict = NULL;
>> + QList *perm_list;
>> + const QListEntry *perm;
>> +
>> + qdict_extract_subqdict(options, &subqdict, prefix);
>> + if (!qdict_size(subqdict)) {
>> + goto out;
>> + }
>> +
>> + crumpled_subqdict = qdict_crumple(subqdict, errp);
>> + if (!crumpled_subqdict) {
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + perm_list = qobject_to(QList, crumpled_subqdict);
>> + if (!perm_list) {
>> + /* Omit the trailing . from the prefix */
>> + error_setg(errp, "%.*s expects a list",
>> + (int)(strlen(prefix) - 1), prefix);
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + for (perm = qlist_first(perm_list); perm; perm = qlist_next(perm)) {
>> + const char *perm_name;
>> + BlockdevPermission perm_bit;
>> +
>> + perm_name = qstring_get_try_str(qobject_to(QString, perm->value));
>> + if (!perm_name) {
>> + /* Omit the trailing . from the prefix */
>> + error_setg(errp, "%.*s expects a list of enum strings",
>> + (int)(strlen(prefix) - 1), prefix);
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + perm_bit = qapi_enum_parse(&BlockdevPermission_lookup, perm_name,
>> + BLOCKDEV_PERMISSION__MAX, errp);
>> + if (perm_bit == BLOCKDEV_PERMISSION__MAX) {
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + *dest |= UINT64_C(1) << perm_bit;
>> + }
>> +
>> +out:
>> + qobject_unref(subqdict);
>> + qobject_unref(crumpled_subqdict);
>> + return ret;
>> +}
>> +
>> +static int blkdebug_parse_perms(BDRVBlkdebugState *s, QDict *options,
>> + Error **errp)
>> +{
>> + int ret;
>> +
>> + ret = blkdebug_parse_perm_list(&s->take_child_perms, options,
>> + "take-child-perms.", errp);
>> + if (ret < 0) {
>> + return ret;
>> + }
>> +
>> + ret = blkdebug_parse_perm_list(&s->unshare_child_perms, options,
>> + "unshare-child-perms.", errp);
>> + if (ret < 0) {
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>
>
> It's a pity that being described in json, these new parameters still not parsed automatically..
That would require changing the whole bdrv_open() infrastructure to use
QAPI types.
>> +
>> static QemuOptsList runtime_opts = {
>
> and that we have to keep these runtime_opts everywhere, which duplicates json definitions..
Well, it duplicates some json definitions. I don’t add the new
parameters added in this patch to the list, because there is no point in
supporting them outside of blockdev-add.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2019-09-19 16:50 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-12 13:56 [Qemu-devel] [PATCH 0/4] mirror: Do not dereference invalid pointers Max Reitz
2019-09-12 13:56 ` [Qemu-devel] [PATCH 1/4] " Max Reitz
2019-09-13 22:43 ` John Snow
2019-09-18 15:16 ` Vladimir Sementsov-Ogievskiy
2019-09-12 13:56 ` [Qemu-devel] [PATCH 2/4] blkdebug: Allow taking/unsharing permissions Max Reitz
2019-09-18 16:01 ` Vladimir Sementsov-Ogievskiy
2019-09-19 16:49 ` Max Reitz [this message]
2019-09-12 13:56 ` [Qemu-devel] [PATCH 3/4] iotests: Add @error to wait_until_completed Max Reitz
2019-09-13 22:53 ` John Snow
2019-09-16 7:56 ` Max Reitz
2019-09-18 16:09 ` Vladimir Sementsov-Ogievskiy
2019-09-12 13:56 ` [Qemu-devel] [PATCH 4/4] iotests: Add test for failing mirror complete Max Reitz
2019-09-18 16:30 ` Vladimir Sementsov-Ogievskiy
2019-09-19 16:51 ` Max Reitz
2019-09-18 18:46 ` John Snow
2019-09-19 16:58 ` Max Reitz
2019-09-19 17:02 ` John Snow
2019-09-19 17:06 ` Max Reitz
2019-09-18 15:38 ` [Qemu-devel] [PATCH 0/4] mirror: Do not dereference invalid pointers Vladimir Sementsov-Ogievskiy
2019-09-19 16:45 ` Max Reitz
2019-09-19 16:50 ` Vladimir Sementsov-Ogievskiy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=84586806-87cf-0024-f583-d4ef9fde2212@redhat.com \
--to=mreitz@redhat.com \
--cc=armbru@redhat.com \
--cc=jsnow@redhat.com \
--cc=kwolf@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=vsementsov@virtuozzo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).