| Message ID | 20210224135255.253837-13-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qapi/qom: QAPIfy --object and object-add | expand |
On 2/24/21 7:52 AM, Kevin Wolf wrote: > This adds a QAPI schema for the properties of the can-* objects. > > can-bus doesn't have any properties, so it only needs to be added to the > ObjectType enum without adding a new branch to ObjectOptions. I somewhat prefer 'can-bus': {}, to make it explicit that we thought about it, but since we allow defaulted union branches, your approach works too. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > qapi/qom.json | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/qapi/qom.json b/qapi/qom.json > index f22b7aa99b..4b1cd4b8dc 100644 > --- a/qapi/qom.json > +++ b/qapi/qom.json > @@ -207,6 +207,21 @@ > 'returns': [ 'ObjectPropertyInfo' ], > 'allow-preconfig': true } > > +## > +# @CanHostSocketcanProperties: > +# > +# Properties for can-host-socketcan objects. > +# > +# @if: interface name of the host system CAN bus to connect to > +# > +# @canbus: object ID of the can-bus object to connect to the host interface > +# > +# Since: 2.12 > +## > +{ 'struct': 'CanHostSocketcanProperties', > + 'data': { 'if': 'str', > + 'canbus': 'str' } } > + Okay, matches net/can/can_socketcan.c:can_host_socketcan_class_init() (after chasing down the parent class in net/can/can_host.c:can_host_class_init() to find "canbus"). > ## > # @CryptodevBackendProperties: > # > @@ -439,6 +454,8 @@ > 'authz-listfile', > 'authz-pam', > 'authz-simple', > + 'can-bus', > + 'can-host-socketcan', > 'cryptodev-backend', > 'cryptodev-backend-builtin', > 'cryptodev-vhost-user', > @@ -479,6 +496,7 @@ > 'authz-listfile': 'AuthZListFileProperties', > 'authz-pam': 'AuthZPAMProperties', > 'authz-simple': 'AuthZSimpleProperties', > + 'can-host-socketcan': 'CanHostSocketcanProperties', > 'cryptodev-backend': 'CryptodevBackendProperties', > 'cryptodev-backend-builtin': 'CryptodevBackendProperties', > 'cryptodev-vhost-user': 'CryptodevVhostUserProperties', > Reviewed-by: Eric Blake <eblake@redhat.com>
Am 26.02.2021 um 20:42 hat Eric Blake geschrieben: > On 2/24/21 7:52 AM, Kevin Wolf wrote: > > This adds a QAPI schema for the properties of the can-* objects. > > > > can-bus doesn't have any properties, so it only needs to be added to the > > ObjectType enum without adding a new branch to ObjectOptions. > > I somewhat prefer > > 'can-bus': {}, > > to make it explicit that we thought about it, but since we allow > defaulted union branches, your approach works too. The QAPI generator disagrees: ../qapi/qom.json: In union 'ObjectOptions': ../qapi/qom.json:492: 'data' member 'can-bus' misses key 'type' It seems we can't use inline definitions of struct types because we already use that for the extended description of branch types. And adding a whole named struct without content is probably a bit too much? Kevin
On 3/2/21 12:32 PM, Kevin Wolf wrote: > Am 26.02.2021 um 20:42 hat Eric Blake geschrieben: >> On 2/24/21 7:52 AM, Kevin Wolf wrote: >>> This adds a QAPI schema for the properties of the can-* objects. >>> >>> can-bus doesn't have any properties, so it only needs to be added to the >>> ObjectType enum without adding a new branch to ObjectOptions. >> >> I somewhat prefer >> >> 'can-bus': {}, >> >> to make it explicit that we thought about it, but since we allow >> defaulted union branches, your approach works too. > > The QAPI generator disagrees: > > ../qapi/qom.json: In union 'ObjectOptions': > ../qapi/qom.json:492: 'data' member 'can-bus' misses key 'type' > > It seems we can't use inline definitions of struct types because we > already use that for the extended description of branch types. And > adding a whole named struct without content is probably a bit too much? Oh, maybe I'm remembering an experiment I did with a patch to add that once, but it never went anywhere, since in the meantime we added the 'any enum not listed is acceptable as adding no additional members'. So my preference stems from (faulty?) memory on my part, and your patch is fine as is.
diff --git a/qapi/qom.json b/qapi/qom.json index f22b7aa99b..4b1cd4b8dc 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -207,6 +207,21 @@ 'returns': [ 'ObjectPropertyInfo' ], 'allow-preconfig': true } +## +# @CanHostSocketcanProperties: +# +# Properties for can-host-socketcan objects. +# +# @if: interface name of the host system CAN bus to connect to +# +# @canbus: object ID of the can-bus object to connect to the host interface +# +# Since: 2.12 +## +{ 'struct': 'CanHostSocketcanProperties', + 'data': { 'if': 'str', + 'canbus': 'str' } } + ## # @CryptodevBackendProperties: # @@ -439,6 +454,8 @@ 'authz-listfile', 'authz-pam', 'authz-simple', + 'can-bus', + 'can-host-socketcan', 'cryptodev-backend', 'cryptodev-backend-builtin', 'cryptodev-vhost-user', @@ -479,6 +496,7 @@ 'authz-listfile': 'AuthZListFileProperties', 'authz-pam': 'AuthZPAMProperties', 'authz-simple': 'AuthZSimpleProperties', + 'can-host-socketcan': 'CanHostSocketcanProperties', 'cryptodev-backend': 'CryptodevBackendProperties', 'cryptodev-backend-builtin': 'CryptodevBackendProperties', 'cryptodev-vhost-user': 'CryptodevVhostUserProperties',
This adds a QAPI schema for the properties of the can-* objects. can-bus doesn't have any properties, so it only needs to be added to the ObjectType enum without adding a new branch to ObjectOptions. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- qapi/qom.json | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)