[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/4] blkdebug: Allow taking/unsharing permission
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH 2/4] blkdebug: Allow taking/unsharing permissions |
Date: |
Thu, 19 Sep 2019 18:49:17 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
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 <address@hidden>
>> ---
>> 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
signature.asc
Description: OpenPGP digital signature