Re: [PATCH v6 09/14] qmp: update virtio features map to support extended features

[Paolo Abeni <pabeni@redhat.com>]

On 9/22/25 3:15 PM, Michael S. Tsirkin wrote:
> On Mon, Sep 22, 2025 at 03:10:36PM +0200, Paolo Abeni wrote:
>> On 9/22/25 2:55 PM, Michael S. Tsirkin wrote:
>>> On Fri, Sep 12, 2025 at 03:07:00PM +0200, Paolo Abeni wrote:
>>>> Extend the VirtioDeviceFeatures struct with an additional u64
>>>> to track unknown features in the 64-127 bit range and decode
>>>> the full virtio features spaces for vhost and virtio devices.
>>>>
>>>> Also add entries for the soon-to-be-supported virtio net GSO over
>>>> UDP features.
>>>>
>>>> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>>>> Acked-by: Jason Wang <jasowang@redhat.com>
>>>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>>>> ---
>>>> v3 -> v4:
>>>>   - cleanup unknown features init
>>>>   - update QMP example and doc accordingly
>>>>   - use new virtio_features macro names
>>>>
>>>> v2 -> v3:
>>>>   - unknown-dev-features-dword2 -> unknown-dev-features2
>>>>   - _array -> _ex
>>>>   - fixed typos in entries description
>>>>
>>>> v1 -> v2:
>>>>   - uint128_t -> uint64_t[]
>>>> ---
>>>>  hw/virtio/virtio-hmp-cmds.c |  3 +-
>>>>  hw/virtio/virtio-qmp.c      | 91 +++++++++++++++++++++++++------------
>>>>  hw/virtio/virtio-qmp.h      |  3 +-
>>>>  qapi/virtio.json            |  9 +++-
>>>>  4 files changed, 74 insertions(+), 32 deletions(-)
>>>>
>>>> diff --git a/hw/virtio/virtio-hmp-cmds.c b/hw/virtio/virtio-hmp-cmds.c
>>>> index 7d8677bcf0..1daae482d3 100644
>>>> --- a/hw/virtio/virtio-hmp-cmds.c
>>>> +++ b/hw/virtio/virtio-hmp-cmds.c
>>>> @@ -74,7 +74,8 @@ static void hmp_virtio_dump_features(Monitor *mon,
>>>>      }
>>>>  
>>>>      if (features->has_unknown_dev_features) {
>>>> -        monitor_printf(mon, "  unknown-features(0x%016"PRIx64")\n",
>>>> +        monitor_printf(mon, "  unknown-features(0x%016"PRIx64"%016"PRIx64")\n",
>>>> +                       features->unknown_dev_features2,
>>>>                         features->unknown_dev_features);
>>>>      }
>>>>  }
>>>> diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
>>>> index 3b6377cf0d..502c9ae930 100644
>>>> --- a/hw/virtio/virtio-qmp.c
>>>> +++ b/hw/virtio/virtio-qmp.c
>>>> @@ -325,6 +325,20 @@ static const qmp_virtio_feature_map_t virtio_net_feature_map[] = {
>>>>      FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
>>>>              "VHOST_USER_F_PROTOCOL_FEATURES: Vhost-user protocol features "
>>>>              "negotiation supported"),
>>>> +    FEATURE_ENTRY(VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO, \
>>>> +            "VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO: Driver can receive GSO over "
>>>> +            "UDP tunnel packets"),
>>>> +    FEATURE_ENTRY(VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM, \
>>>> +            "VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO: Driver can receive GSO over "
>>>
>>> This really should be VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM.
>>>
>>> Given they all seem to start with repeating the feature name,
>>> why not just add it to the string automatically by the macro?
>>
>> UHmm... let me keep the things simple and just fix the string; macro
>> refactoring could be a follow-up, I hope?
>>
>> Thanks,
>>
>> Paolo
> 
> the fix can be a follow-up too, but I'd like it done now pls,
> there could be more I missed.

Np. I'll follow-up as soon as this series will be merged. I'll send v7
soon with the above typo fixed (plus the others pointed by Stefano).

Thanks,

Paolo