Re: [PATCH v2 12/31] qapi/qom: Add ObjectOptions for can-*
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.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 12/31] qapi/qom: Add ObjectOptions for can-*
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
Re: [PATCH v2 12/31] qapi/qom: Add ObjectOptions for can-*
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
> ---
> 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
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[PATCH v2 12/31] qapi/qom: Add ObjectOptions for can-*
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
---
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' } }
+
##
# @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',
--
2.29.2
