From: Pankaj Gupta <pagupta@redhat.com> To: pannengyuan@huawei.com Cc: liyiting@huawei.com, kuhn chenqun <kuhn.chenqun@huawei.com>, zhang zhanghailiang <zhang.zhanghailiang@huawei.com>, qemu-devel@nongnu.org, mst@redhat.com Subject: Re: [PATCH v2 1/3] virtio: add ability to delete vq through a pointer Date: Wed, 4 Dec 2019 03:33:07 -0500 (EST) Message-ID: <606337023.38890965.1575448387620.JavaMail.zimbra@redhat.com> (raw) In-Reply-To: <1575444716-17632-1-git-send-email-pannengyuan@huawei.com> > 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); 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? 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 ` Pankaj Gupta [this message] 2019-12-05 2:30 ` [PATCH v2 1/3] virtio: add ability to delete vq through a pointer Pan Nengyuan 2019-12-05 4:51 ` Pankaj Gupta 2019-12-09 15:58 ` Michael S. Tsirkin 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=606337023.38890965.1575448387620.JavaMail.zimbra@redhat.com \ --to=pagupta@redhat.com \ --cc=kuhn.chenqun@huawei.com \ --cc=liyiting@huawei.com \ --cc=mst@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