On Sun, Jan 5, 2020 at 1:39 PM Michael S. Tsirkin <m...@redhat.com> wrote:
> On Thu, Jan 02, 2020 at 09:09:04AM +0200, Yuri Benditovich wrote: > > > > > > On Thu, Jan 2, 2020 at 1:50 AM Michael S. Tsirkin <m...@redhat.com> > wrote: > > > > On Thu, Dec 26, 2019 at 11:29:50AM +0200, Yuri Benditovich wrote: > > > On Thu, Dec 26, 2019 at 10:58 AM Jason Wang <jasow...@redhat.com> > wrote: > > > > > > > > > > > > On 2019/12/26 下午12:36, Yuri Benditovich wrote: > > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1708480 > > > > > Fix leak of region reference that prevents complete > > > > > device deletion on hot unplug. > > > > > > > > > > > > More information is needed here, the bug said only q35 can meet > this > > > > issue. What makes q35 different here? > > > > > > > > > > I do not have any ready answer, I did not dig into it too much. > > > Probably Michael Tsirkin or Paolo Bonzini can answer without > digging. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Yuri Benditovich <yuri.benditov...@daynix.com> > > > > > --- > > > > > hw/virtio/virtio.c | 5 +++++ > > > > > 1 file changed, 5 insertions(+) > > > > > > > > > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > > > > > index 04716b5f6c..baadec8abc 100644 > > > > > --- a/hw/virtio/virtio.c > > > > > +++ b/hw/virtio/virtio.c > > > > > @@ -2340,6 +2340,11 @@ void virtio_del_queue(VirtIODevice > *vdev, int > > n) > > > > > vdev->vq[n].vring.num_default = 0; > > > > > vdev->vq[n].handle_output = NULL; > > > > > vdev->vq[n].handle_aio_output = NULL; > > > > > + /* > > > > > + * with vring.num = 0 the queue will be ignored > > > > > + * in later loops of region cache reset > > > > > + */ > > > > > > > > > > > > I can't get the meaning of this comment. > > > > > > > > Thanks > > > > > > > > > > > > > + virtio_virtqueue_reset_region_cache(&vdev->vq[n]); > > > > > > Do we need to drop this from virtio_device_free_virtqueues then? > > > > > > > > Not mandatory. Repetitive virtio_virtqueue_reset_region_cache does not > do > > anything bad. > > Some of virtio devices do not do 'virtio_del_queue' at all. Currently > > virtio_device_free_virtqueues resets region cache for them. > > IMO, not calling 'virtio_del_queue' is a bug, but not in the scope of > current > > series, I'll take care of that later. > > Maybe we should just del all queues in virtio_device_unrealize? > Will allow us to drop some logic tracking which vqs were created. > > Yes, this is also possible with some rework of virtio_device_free_virtqueues. virtio-net has some additional operations around queue deletion, it deletes queues when switches from single queue to multiple. > > > > > > > > g_free(vdev->vq[n].used_elems); > > > > > } > > > > > > > > > > > > > > >
