From: "Michael S. Tsirkin" <mst@redhat.com> To: Pankaj Gupta <pagupta@redhat.com> Cc: liyiting@huawei.com, kuhn chenqun <kuhn.chenqun@huawei.com>, pannengyuan@huawei.com, qemu-devel@nongnu.org, zhang zhanghailiang <zhang.zhanghailiang@huawei.com> Subject: Re: [PATCH v2 1/3] virtio: add ability to delete vq through a pointer Date: Mon, 9 Dec 2019 10:58:17 -0500 Message-ID: <20191209105101-mutt-send-email-mst@kernel.org> (raw) In-Reply-To: <606337023.38890965.1575448387620.JavaMail.zimbra@redhat.com> On Wed, Dec 04, 2019 at 03:33:07AM -0500, Pankaj Gupta wrote: > > > From: Pan Nengyuan <pannengyuan@huawei.com> > > > > Devices tend to maintain vq pointers, allow deleting them trough a vq > > pointer. > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> > > --- > > Changes v2 to v1: > > - add a new function virtio_delete_queue to cleanup vq through a vq pointer > > --- > > hw/virtio/virtio.c | 16 +++++++++++----- > > include/hw/virtio/virtio.h | 2 ++ > > 2 files changed, 13 insertions(+), 5 deletions(-) > > > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > > index 04716b5..6de3cfd 100644 > > --- a/hw/virtio/virtio.c > > +++ b/hw/virtio/virtio.c > > @@ -2330,17 +2330,23 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int > > queue_size, > > return &vdev->vq[i]; > > } > > > > +void virtio_delete_queue(VirtQueue *vq) > > +{ > > + vq->vring.num = 0; > > + vq->vring.num_default = 0; > > + vq->handle_output = NULL; > > + vq->handle_aio_output = NULL; > > + g_free(vq->used_elems); > > + vq->used_elems = NULL; > > +} > > + > > void virtio_del_queue(VirtIODevice *vdev, int n) > > { > > if (n < 0 || n >= VIRTIO_QUEUE_MAX) { > > abort(); > > } > > > > - vdev->vq[n].vring.num = 0; > > - vdev->vq[n].vring.num_default = 0; > > - vdev->vq[n].handle_output = NULL; > > - vdev->vq[n].handle_aio_output = NULL; > > - g_free(vdev->vq[n].used_elems); > > + virtio_delete_queue(&vdev->vq[n]); > > } > > > > static void virtio_set_isr(VirtIODevice *vdev, int value) > > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > > index c32a815..e18756d 100644 > > --- a/include/hw/virtio/virtio.h > > +++ b/include/hw/virtio/virtio.h > > @@ -183,6 +183,8 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int > > queue_size, > > > > void virtio_del_queue(VirtIODevice *vdev, int n); > > > > +void virtio_delete_queue(VirtQueue *vq); > > + > > void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem, > > unsigned int len); > > void virtqueue_flush(VirtQueue *vq, unsigned int count); > > -- > > 2.7.2.windows.1 > > > > > Overall it ooks good to me. > > Just one point: e.g in virtio_rng: "virtio_rng_device_unrealize" function > We are doing : virtio_del_queue(vdev, 0); Yea. Let's just bite the bullet and convert all callers. Not so many of them. > One can directly call "virtio_delete_queue". It can become confusing > to call multiple functions for same purpose. Instead, Can we make > "virtio_delete_queue" static inline? We can't really. > Other than that: > Reviewed-by: Pankaj Gupta <pagupta@redhat.com> > > > > >
next prev parent reply index Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-04 7:31 pannengyuan 2019-12-04 7:31 ` [PATCH v2 2/3] virtio-balloon: fix memory leak while attach virtio-balloon device pannengyuan 2019-12-04 10:16 ` Laurent Vivier 2019-12-04 7:31 ` [PATCH v2 3/3] virtio-serial-bus: fix memory leak while attach virtio-serial-bus pannengyuan 2019-12-04 10:17 ` Laurent Vivier 2019-12-04 14:41 ` Eric Blake 2019-12-09 16:51 ` Michael S. Tsirkin 2019-12-04 8:33 ` [PATCH v2 1/3] virtio: add ability to delete vq through a pointer Pankaj Gupta 2019-12-05 2:30 ` Pan Nengyuan 2019-12-05 4:51 ` Pankaj Gupta 2019-12-09 15:58 ` Michael S. Tsirkin [this message] 2019-12-04 9:40 ` Laurent Vivier 2019-12-04 14:40 ` Eric Blake 2019-12-05 2:35 ` Pan Nengyuan 2019-12-05 16:45 ` Amit Shah 2019-12-06 2:17 ` Pan Nengyuan 2019-12-06 8:56 ` Amit Shah 2019-12-06 9:00 ` Pan Nengyuan 2019-12-09 16:43 ` Michael S. Tsirkin 2019-12-09 16:58 ` Michael S. Tsirkin 2019-12-10 2:08 ` Pan Nengyuan
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20191209105101-mutt-send-email-mst@kernel.org \ --to=mst@redhat.com \ --cc=kuhn.chenqun@huawei.com \ --cc=liyiting@huawei.com \ --cc=pagupta@redhat.com \ --cc=pannengyuan@huawei.com \ --cc=qemu-devel@nongnu.org \ --cc=zhang.zhanghailiang@huawei.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
QEMU-Devel Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git git clone --mirror https://lore.kernel.org/qemu-devel/2 qemu-devel/git/2.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \ qemu-devel@nongnu.org public-inbox-index qemu-devel Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel AGPL code for this site: git clone https://public-inbox.org/public-inbox.git