Re: [PATCH 09/21] hw/virtio/virtio-pci: Remove VIRTIO_PCI_FLAG_MIGRATE_EXTRA definition

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

On 17/1/25 10:06, Thomas Huth wrote:
> On 16/01/2025 00.22, Philippe Mathieu-Daudé wrote:
>> VIRTIO_PCI_FLAG_MIGRATE_EXTRA was only used by the
>> hw_compat_2_4[] array, via the 'migrate-extra=true'
>> property. We removed all machines using that array,
>> lets remove all the code around VIRTIO_PCI_FLAG_MIGRATE_EXTRA.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/hw/virtio/virtio-pci.h |  4 ----
>>   hw/virtio/virtio-pci.c         | 10 ----------
>>   2 files changed, 14 deletions(-)
>>
>> diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/ 
>> virtio-pci.h
>> index 971c5fabd44..dd6eb9a4fc7 100644
>> --- a/include/hw/virtio/virtio-pci.h
>> +++ b/include/hw/virtio/virtio-pci.h
>> @@ -32,7 +32,6 @@ DECLARE_OBJ_CHECKERS(VirtioPCIBusState, 
>> VirtioPCIBusClass,
>>   enum {
>>       VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT,
>>       VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT,
>> -    VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT,
>>       VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT,
>>       VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT,
>>       VIRTIO_PCI_FLAG_PAGE_PER_VQ_BIT,
>> @@ -57,9 +56,6 @@ enum {
>>   /* virtio version flags */
>>   #define VIRTIO_PCI_FLAG_DISABLE_PCIE (1 << 
>> VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT)
>> -/* migrate extra state */
>> -#define VIRTIO_PCI_FLAG_MIGRATE_EXTRA (1 << 
>> VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT)
>> -
>>   /* have pio notification for modern device ? */
>>   #define VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY \
>>       (1 << VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT)
>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>> index c773a9130c7..8dca3ae73e6 100644
>> --- a/hw/virtio/virtio-pci.c
>> +++ b/hw/virtio/virtio-pci.c
>> @@ -144,13 +144,6 @@ static const VMStateDescription 
>> vmstate_virtio_pci = {
>>       }
>>   };
>> -static bool virtio_pci_has_extra_state(DeviceState *d)
>> -{
>> -    VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
>> -
>> -    return proxy->flags & VIRTIO_PCI_FLAG_MIGRATE_EXTRA;
>> -}
>> -
>>   static void virtio_pci_save_extra_state(DeviceState *d, QEMUFile *f)
>>   {
>>       VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
>> @@ -2352,8 +2345,6 @@ static void virtio_pci_bus_reset_hold(Object 
>> *obj, ResetType type)
>>   static const Property virtio_pci_properties[] = {
>>       DEFINE_PROP_BIT("virtio-pci-bus-master-bug-migration", 
>> VirtIOPCIProxy, flags,
>>                       VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT, 
>> false),
>> -    DEFINE_PROP_BIT("migrate-extra", VirtIOPCIProxy, flags,
>> -                    VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT, true),
>>       DEFINE_PROP_BIT("modern-pio-notify", VirtIOPCIProxy, flags,
>>                       VIRTIO_PCI_FLAG_MODERN_PIO_NOTIFY_BIT, false),
>>       DEFINE_PROP_BIT("x-disable-pcie", VirtIOPCIProxy, flags,
>> @@ -2601,7 +2592,6 @@ static void 
>> virtio_pci_bus_class_init(ObjectClass *klass, void *data)
>>       k->load_queue = virtio_pci_load_queue;
>>       k->save_extra_state = virtio_pci_save_extra_state;
>>       k->load_extra_state = virtio_pci_load_extra_state;
>> -    k->has_extra_state = virtio_pci_has_extra_state;
> 
> Did you test your patches? Looking at the calling site:
> 
> static bool virtio_extra_state_needed(void *opaque)
> {
>      VirtIODevice *vdev = opaque;
>      BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> 
>      return k->has_extra_state &&
>          k->has_extra_state(qbus->parent);
> }
> 
> ... this will return now false instead of true, so I think your patch 
> here is wrong?

Oops, good catch...