Re: [PATCH RFC 13/15] hw/virtio/vhost-user: add shadow virtqueues and eventfd intercepts

Connor Kite <[email protected]>
Newsgroups dev.linux.lists.virtio-fs,org.nongnu.qemu-devel
Message-ID <CA+spn3oU7dtEPO_5-hT6x1-n+4HYxO9a8_aes7Jy3afWDDY_Ag@mail.gmail.com>
On Tue, Jul 28, 2026 at 12:42 PM Stefan Hajnoczi <[email protected]> wrote:
>
> > +
> > +        if (svq->hdev_kick.initialized == false) {
> > +            int r = event_notifier_init(&svq->hdev_kick, 0);
>
> Where is event_notifier_cleanup() called?
>

I have added in to the error code paths when calling
vhost_user_set_vring_kick.

I left most cleanup to the end of the patch series, and it was underdeveloped at
the time this went out for RFC, with the intent of getting feedback on
the overall
approach.  In hindsight, it is probably better practice to implement
cleanup at the
same time as the init/start code, per Hanna's comment.

I will pull the svq cleanup code forward to this patch.

> > +            if (r) {
> > +                error_report("Failed to create kick event notifier");
> > +                return r;
> > +            }
> > +        }
> > +
> > +        file->fd = event_notifier_get_fd(&svq->hdev_kick);
>
> Modifying the function argument is probably not expected but
> vhost_virtqueue_start() doesn't use it after this call, so there is no
> immediate problem. It would be safer to have a local struct
> vhost_vring_file that can be modified without affecting the caller's
> copy.
>

That makes sense.  Swapped over to a local struct.

...

> > +
> > +        if (svq->hdev_call.initialized == false) {
> > +            int r = event_notifier_init(&svq->hdev_call, 0);
>
> Where is event_notifier_cleanup() called?
>

Same as when setting the kick.

> > +            if (r) {
> > +                error_report("Failed to create call event notifier");
> > +                return r;
> > +            }
> > +        }
> > +
> > +        file->fd = event_notifier_get_fd(&svq->hdev_call);
>
> Same as above.
>

Swapped over to a local struct as above.

Thanks!
Connor
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.