Re: [PATCH 1/2] virtio-net: make VirtIONet.vlans an array instead of a pointer

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 3/8/25 09:38, Akihiko Odaki wrote:
> On 2025/08/02 23:45, Michael Tokarev wrote:
>> On 02.08.2025 17:21, Michael Tokarev wrote:
>>> This field is a fixed-size buffer (number of elements is MAX_VLAN,
>>> known at build time).  There's no need to allocate it dynamically,
>>> it can be made an integral part of VirtIONet structure.
>>>
>>> This field is the only user of VMSTATE_BUFFER_POINTER_UNSAFE() macro.
>>>
>>> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
>>> ---
>>>   hw/net/virtio-net.c            | 8 +++-----
>>>   include/hw/virtio/virtio-net.h | 2 +-
>>>   2 files changed, 4 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>>> @@ -3524,7 +3524,7 @@ static const VMStateDescription 
>>> vmstate_virtio_net_device = {
>>>            * buffer; hold onto your endiannesses; it's actually used 
>>> as a bitmap
>>>            * but based on the uint.
>>>            */
>>> -        VMSTATE_BUFFER_POINTER_UNSAFE(vlans, VirtIONet, 0, MAX_VLAN 
>>> >> 3),
>>> +        VMSTATE_BUFFER(vlans, VirtIONet),
>>
>> This doesn't compile.  And I can't figure out, so far, what's needed
>> here :)
> 
> The type check is failing because VMSTATE_BUFFER() expects an array of 
> uint8_t but it is an array of uint32_t.
> 
> Now I get why it used an "UNSAFE" macro. We should usually use 
> VMSTATE_UINT32_ARRAY() instead, but we need "BUFFER" for compatibility, 
> so it needed the "UNSAFE" variant to disable the type check.
> 
> Fortunately there is a variant that is "BUFFER" and "UNSAFE" but not 
> "POINTER". So this should be:
> VMSTATE_BUFFER_UNSAFE(vlans, VirtIONet, 0,
>                        sizeof(typeof_field(VirtIONet, vlans)))

Nice.