qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Cc: qemu-block@nongnu.org,  qemu-devel@nongnu.org,  jsnow@redhat.com,
	kwolf@redhat.com,  hreitz@redhat.com,  devel@lists.libvirt.org,
	eblake@redhat.com,  michael.roth@amd.com,  pbonzini@redhat.com,
	pkrempa@redhat.com,  f.ebner@proxmox.com
Subject: Re: [RFC 01/15] scripts/qapi: support type-based unions
Date: Thu, 28 Mar 2024 10:15:55 +0100	[thread overview]
Message-ID: <87frwaemyc.fsf@pond.sub.org> (raw)
In-Reply-To: <20240313150907.623462-2-vsementsov@yandex-team.ru> (Vladimir Sementsov-Ogievskiy's message of "Wed, 13 Mar 2024 18:08:53 +0300")

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> Look at block-job-change command: we have to specify both 'id' to chose
> the job to operate on and 'type' for QAPI union be parsed. But for user
> this looks redundant: when we specify 'id', QEMU should be able to get
> corresponding job's type.
>
> This commit brings such a possibility: just specify some Enum type as
> 'discriminator', and define a function somewhere with prototype
>
> bool YourUnionType_mapper(YourUnionType *, EnumType *out, Error **errp)
>
> Further commits will use this functionality to upgrade block-job-change
> interface and introduce some new interfaces.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  scripts/qapi/introspect.py |  5 +++-
>  scripts/qapi/schema.py     | 50 +++++++++++++++++++++++---------------
>  scripts/qapi/types.py      |  3 ++-
>  scripts/qapi/visit.py      | 43 ++++++++++++++++++++++++++------
>  4 files changed, 73 insertions(+), 28 deletions(-)

I believe you need to update docs/devel/qapi-code-gen.rst.

Current text:

    Union types
    -----------

    Syntax::

        UNION = { 'union': STRING,
                  'base': ( MEMBERS | STRING ),
                  'discriminator': STRING,
                  'data': BRANCHES,
                  '*if': COND,
                  '*features': FEATURES }
        BRANCHES = { BRANCH, ... }
        BRANCH = STRING : TYPE-REF
               | STRING : { 'type': TYPE-REF, '*if': COND }

    Member 'union' names the union type.

    The 'base' member defines the common members.  If it is a MEMBERS_
    object, it defines common members just like a struct type's 'data'
    member defines struct type members.  If it is a STRING, it names a
    struct type whose members are the common members.

    Member 'discriminator' must name a non-optional enum-typed member of
    the base struct.  That member's value selects a branch by its name.
    If no such branch exists, an empty branch is assumed.

If I understand your commit message correctly, this paragraph is no
longer true.

    Each BRANCH of the 'data' object defines a branch of the union.  A
    union must have at least one branch.

    The BRANCH's STRING name is the branch name.  It must be a value of
    the discriminator enum type.

    The BRANCH's value defines the branch's properties, in particular its
    type.  The type must a struct type.  The form TYPE-REF_ is shorthand
    for :code:`{ 'type': TYPE-REF }`.

    In the Client JSON Protocol, a union is represented by an object with
    the common members (from the base type) and the selected branch's
    members.  The two sets of member names must be disjoint.

    Example::

     { 'enum': 'BlockdevDriver', 'data': [ 'file', 'qcow2' ] }
     { 'union': 'BlockdevOptions',
       'base': { 'driver': 'BlockdevDriver', '*read-only': 'bool' },
       'discriminator': 'driver',
       'data': { 'file': 'BlockdevOptionsFile',
                 'qcow2': 'BlockdevOptionsQcow2' } }

    Resulting in these JSON objects::

     { "driver": "file", "read-only": true,
       "filename": "/some/place/my-image" }
     { "driver": "qcow2", "read-only": false,
       "backing": "/some/place/my-image", "lazy-refcounts": true }

    The order of branches need not match the order of the enum values.
    The branches need not cover all possible enum values.  In the
    resulting generated C data types, a union is represented as a struct
    with the base members in QAPI schema order, and then a union of
    structures for each branch of the struct.

    The optional 'if' member specifies a conditional.  See `Configuring
    the schema`_ below for more on this.

    The optional 'features' member specifies features.  See Features_
    below for more on this.



  reply	other threads:[~2024-03-28  9:17 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-13 15:08 [RFC 00/15] block job API Vladimir Sementsov-Ogievskiy
2024-03-13 15:08 ` [RFC 01/15] scripts/qapi: support type-based unions Vladimir Sementsov-Ogievskiy
2024-03-28  9:15   ` Markus Armbruster [this message]
2024-03-28 10:44     ` Vladimir Sementsov-Ogievskiy
2024-03-28  9:40   ` Markus Armbruster
2024-03-28 10:18     ` Vladimir Sementsov-Ogievskiy
2024-03-13 15:08 ` [RFC 02/15] qapi: rename BlockJobChangeOptions to JobChangeOptions Vladimir Sementsov-Ogievskiy
2024-03-13 15:08 ` [RFC 03/15] blockjob: block_job_change_locked(): check job type Vladimir Sementsov-Ogievskiy
2024-03-13 15:08 ` [RFC 04/15] qapi: block-job-change: make copy-mode parameter optional Vladimir Sementsov-Ogievskiy
2024-03-28  9:24   ` Markus Armbruster
2024-03-28 10:56     ` Vladimir Sementsov-Ogievskiy
2024-03-13 15:08 ` [RFC 05/15] qapi: JobChangeOptions: make type member optional and deprecated Vladimir Sementsov-Ogievskiy
2024-03-13 15:08 ` [RFC 06/15] blockjob: move change action implementation to job from block-job Vladimir Sementsov-Ogievskiy
2024-03-13 15:08 ` [RFC 07/15] qapi: add job-change Vladimir Sementsov-Ogievskiy
2024-03-13 15:09 ` [RFC 08/15] qapi: job-change: support speed parameter Vladimir Sementsov-Ogievskiy
2024-03-13 15:09 ` [RFC 09/15] qapi: job-complete: introduce no-block-replace option for mirror Vladimir Sementsov-Ogievskiy
2024-03-13 15:09 ` [RFC 10/15] qapi: query-jobs: add information specific for job type Vladimir Sementsov-Ogievskiy
2024-03-13 15:09 ` [RFC 11/15] job-qmp: job_query_single_locked: add assertion on job ret Vladimir Sementsov-Ogievskiy
2024-03-13 15:09 ` [RFC 12/15] qapi: rename BlockDeviceIoStatus to IoStatus Vladimir Sementsov-Ogievskiy
2024-03-13 15:09 ` [RFC 13/15] qapi: move IoStatus to common.json Vladimir Sementsov-Ogievskiy
2024-03-13 15:09 ` [RFC 14/15] qapi: query-job: add block-job specific information Vladimir Sementsov-Ogievskiy
2024-03-13 15:09 ` [RFC 15/15] qapi/block-core: derpecate block-job- APIs 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=87frwaemyc.fsf@pond.sub.org \
    --to=armbru@redhat.com \
    --cc=devel@lists.libvirt.org \
    --cc=eblake@redhat.com \
    --cc=f.ebner@proxmox.com \
    --cc=hreitz@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=michael.roth@amd.com \
    --cc=pbonzini@redhat.com \
    --cc=pkrempa@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@yandex-team.ru \
    /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).