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

[Thomas Huth]

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?

 Thomas