[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 06/46] ivshmem: remove unnecessary dup()
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH v3 06/46] ivshmem: remove unnecessary dup() |
Date: |
Tue, 22 Sep 2015 11:29:01 -0400 (EDT) |
Hi
----- Original Message -----
> On 15.09.2015 18:07, address@hidden wrote:
> > From: Marc-André Lureau <address@hidden>
> >
> > qemu_chr_fe_get_msgfd() transfers ownership, there is no need to dup the
> > fd.
> >
>
> Are you sure? (tested?)
> There is a specific comment that the dup is done because of the get_msgfds
> implementation,
> I checked tcp_get_msgfds and it closes unused fds, is this ok?
>
> static int tcp_get_msgfds(...)
> {
> ...
> /* Close unused fds */
> for (i = to_copy, i < s->read_msgfds_num; i++) {
> close(s->read_msgfds[i]);
> }
> ...
Yes, it closes the remaining fds. But for the others, the one it got, there is
no need for an extra dup() since the ownership is transfered.
>
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> > hw/misc/ivshmem.c | 21 ++++++---------------
> > 1 file changed, 6 insertions(+), 15 deletions(-)
> >
> > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> > index dd15f0e..fbeb731 100644
> > --- a/hw/misc/ivshmem.c
> > +++ b/hw/misc/ivshmem.c
> > @@ -480,7 +480,7 @@ static bool fifo_update_and_get(IVShmemState *s, const
> > uint8_t *buf, int size,
> > static void ivshmem_read(void *opaque, const uint8_t *buf, int size)
> > {
> > IVShmemState *s = opaque;
> > - int incoming_fd, tmp_fd;
> > + int incoming_fd;
> > int guest_max_eventfd;
> > long incoming_posn;
> >
> > @@ -495,21 +495,21 @@ static void ivshmem_read(void *opaque, const uint8_t
> > *buf, int size)
> > }
> >
> > /* pick off s->server_chr->msgfd and store it, posn should accompany
> > msg */
> > - tmp_fd = qemu_chr_fe_get_msgfd(s->server_chr);
> > - IVSHMEM_DPRINTF("posn is %ld, fd is %d\n", incoming_posn, tmp_fd);
> > + incoming_fd = qemu_chr_fe_get_msgfd(s->server_chr);
> > + IVSHMEM_DPRINTF("posn is %ld, fd is %d\n", incoming_posn,
> > incoming_fd);
> >
> > /* make sure we have enough space for this guest */
> > if (incoming_posn >= s->nb_peers) {
> > if (increase_dynamic_storage(s, incoming_posn) < 0) {
> > error_report("increase_dynamic_storage() failed");
> > - if (tmp_fd != -1) {
> > - close(tmp_fd);
> > + if (incoming_fd != -1) {
> > + close(incoming_fd);
> > }
> > return;
> > }
> > }
> >
> > - if (tmp_fd == -1) {
> > + if (incoming_fd == -1) {
> > /* if posn is positive and unseen before then this is our posn*/
> > if ((incoming_posn >= 0) &&
> > (s->peers[incoming_posn].eventfds == NULL)) {
> > @@ -524,15 +524,6 @@ static void ivshmem_read(void *opaque, const uint8_t
> > *buf, int size)
> > }
> > }
> >
> > - /* because of the implementation of get_msgfd, we need a dup */
> > - incoming_fd = dup(tmp_fd);
> > -
> > - if (incoming_fd == -1) {
> > - error_report("could not allocate file descriptor %s",
> > strerror(errno));
> > - close(tmp_fd);
> > - return;
> > - }
> > -
> > /* if the position is -1, then it's shared memory region fd */
> > if (incoming_posn == -1) {
> >
> >
>
>
- [Qemu-devel] [PATCH v3 04/46] ivshmem: fix number of bytes to push to fifo, (continued)
- [Qemu-devel] [PATCH v3 04/46] ivshmem: fix number of bytes to push to fifo, marcandre . lureau, 2015/09/15
- [Qemu-devel] [PATCH v3 05/46] ivshmem: factor out the incoming fifo handling, marcandre . lureau, 2015/09/15
- [Qemu-devel] [PATCH v3 07/46] ivshmem: remove superflous ivshmem_attr field, marcandre . lureau, 2015/09/15
- [Qemu-devel] [PATCH v3 06/46] ivshmem: remove unnecessary dup(), marcandre . lureau, 2015/09/15
- [Qemu-devel] [PATCH v3 08/46] ivshmem: remove useless doorbell field, marcandre . lureau, 2015/09/15
- [Qemu-devel] [PATCH v3 09/46] ivshmem: more qdev conversion, marcandre . lureau, 2015/09/15
- [Qemu-devel] [PATCH v3 10/46] ivshmem: remove last exit(1), marcandre . lureau, 2015/09/15
- [Qemu-devel] [PATCH v3 11/46] ivshmem: limit maximum number of peers to G_MAXUINT16, marcandre . lureau, 2015/09/15