On Mon, 19 Jan 2026 21:52:12 +0300
Vladimir Sementsov-Ogievskiy <[email protected]> wrote:

> virtio-pci, virtio-mmio and virtio-ccw handle config notifier equally
> but with different code (mmio adds a separate function, when pci use
> common function). Let's chose the more compact way (pci) and reuse it
> for mmio.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>

Ultimately is up to Michael to decide, but I'm not sure patch makes
things cleaner. I would even say, its current form, it does not.

I do like the idea of getting rid of duplicated logic though.


> ---
>  hw/s390x/virtio-ccw.c          | 35 ++++++++++++-------------
>  hw/virtio/virtio-mmio.c        | 41 +++++------------------------
>  hw/virtio/virtio-pci.c         | 34 +++---------------------
>  hw/virtio/virtio.c             | 48 +++++++++++++++++++++++++++++++---
>  include/hw/virtio/virtio-pci.h |  3 ---
>  include/hw/virtio/virtio.h     | 16 +++++++++---
>  6 files changed, 84 insertions(+), 93 deletions(-)
[..]
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 3dc9423eae..ea43ea620c 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
[..]
> +static void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev,

Function name says 'pci' but it actually is not PCI specific.

> +                                                     VirtQueue *vq,
> +                                                     int n, bool assign,
> +                                                     bool with_irqfd)
> +{
> +    if (n == VIRTIO_CONFIG_IRQ_IDX) {
> +        virtio_config_set_guest_notifier_fd_handler(vdev, assign, 
> with_irqfd);
> +    } else {
> +        virtio_queue_set_guest_notifier_fd_handler(vq, assign, with_irqfd);
> +    }
> +}
> +
[..]
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 27cd98d2fe..25ef7c9dff 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -420,8 +420,6 @@ void virtio_queue_update_used_idx(VirtIODevice *vdev, int 
> n);
>  VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n);
>  uint16_t virtio_get_queue_index(VirtQueue *vq);
>  EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq);
> -void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
> -                                                bool with_irqfd);
>  int virtio_device_start_ioeventfd(VirtIODevice *vdev);
>  int virtio_device_grab_ioeventfd(VirtIODevice *vdev);
>  void virtio_device_release_ioeventfd(VirtIODevice *vdev);
> @@ -435,8 +433,18 @@ void virtio_queue_aio_detach_host_notifier(VirtQueue 
> *vq, AioContext *ctx);
>  VirtQueue *virtio_vector_first_queue(VirtIODevice *vdev, uint16_t vector);
>  VirtQueue *virtio_vector_next_queue(VirtQueue *vq);
>  EventNotifier *virtio_config_get_guest_notifier(VirtIODevice *vdev);
> -void virtio_config_set_guest_notifier_fd_handler(VirtIODevice *vdev,
> -                                                 bool assign, bool 
> with_irqfd);
> +
> +/**
> + * virtio_queue_set_guest_notifier - set/unset queue or config guest
> + *     notifier
> + *
> + * @vdev: the VirtIO device
> + * @n: queue number, or VIRTIO_CONFIG_IRQ_IDX to set config notifer

Semantically are mixing queue numbers and interrupt indexes here.

> + * @assign: true to set notifier, false to unset
> + * @with_irqfd: irqfd enabled
> + */
> +int virtio_queue_set_guest_notifier(VirtIODevice *vdev, int n, bool assign,
> +                                    bool with_irqfd);

The name says 'queue' but it is not always queue any more.

Regards,
Halil

Reply via email to