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

["Michael S. Tsirkin" <mst@redhat.com>]

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.

-- 
MST