From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33812) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZeP2M-00038e-CI for qemu-devel@nongnu.org; Tue, 22 Sep 2015 10:59:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZeP2G-0004Qt-C9 for qemu-devel@nongnu.org; Tue, 22 Sep 2015 10:59:18 -0400 Received: from mx6-phx2.redhat.com ([209.132.183.39]:35732) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZeP2G-0004Qd-57 for qemu-devel@nongnu.org; Tue, 22 Sep 2015 10:59:12 -0400 Date: Tue, 22 Sep 2015 10:59:08 -0400 (EDT) From: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Message-ID: <999019625.15207582.1442933948058.JavaMail.zimbra@redhat.com> In-Reply-To: <56016774.4040703@huawei.com> References: <1442333283-13119-1-git-send-email-marcandre.lureau@redhat.com> <1442333283-13119-42-git-send-email-marcandre.lureau@redhat.com> <56016774.4040703@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 41/46] ivshmem: do not keep shm_fd open List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Claudio Fontana Cc: marcandre lureau , drjones@redhat.com, cam@cs.ualberta.ca, qemu-devel@nongnu.org, stefanha@redhat.com Hi ----- Original Message ----- > On 15.09.2015 18:07, marcandre.lureau@redhat.com wrote: > > From: Marc-Andr=C3=A9 Lureau > >=20 > > Remove shm_fd from device state, closing it as early as possible to avo= id > > leaks. > >=20 > > Signed-off-by: Marc-Andr=C3=A9 Lureau > > --- > > hw/misc/ivshmem.c | 14 +++++--------- > > 1 file changed, 5 insertions(+), 9 deletions(-) > >=20 > > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c > > index 4adcac5..f9ac955 100644 > > --- a/hw/misc/ivshmem.c > > +++ b/hw/misc/ivshmem.c > > @@ -88,7 +88,6 @@ typedef struct IVShmemState { > > MemoryRegion ivshmem; > > uint64_t ivshmem_size; /* size of shared memory region */ > > uint32_t ivshmem_64bit; > > - int shm_fd; /* shared memory file descriptor */ >=20 > is it in no way useful during debugging to have access to this field? > Or is it easily available elsewhere? How would it be useful during debugging? Once the memory is mapped there is= n't much you can do with it, it's just keeping a fd open, isn't it? >=20 > Ciao C. >=20 > > =20 > > Peer *peers; > > int nb_peers; /* how many peers we have space for */ > > @@ -235,7 +234,7 @@ static uint64_t ivshmem_io_read(void *opaque, hwadd= r > > addr, > > =20 > > case IVPOSITION: > > /* return my VM ID if the memory is mapped */ > > - if (s->shm_fd >=3D 0) { > > + if (memory_region_is_mapped(&s->ivshmem)) { > > ret =3D s->vm_id; > > } else { > > ret =3D -1; > > @@ -356,8 +355,6 @@ static int create_shared_memory_BAR(IVShmemState *s= , > > int fd, uint8_t attr, > > return -1; > > } > > =20 > > - s->shm_fd =3D fd; > > - > > memory_region_init_ram_ptr(&s->ivshmem, OBJECT(s), "ivshmem.bar2", > > s->ivshmem_size, ptr); > > vmstate_register_ram(&s->ivshmem, DEVICE(s)); > > @@ -535,7 +532,7 @@ static void ivshmem_read(void *opaque, const uint8_= t > > *buf, int size) > > if (incoming_posn =3D=3D -1) { > > void * map_ptr; > > =20 > > - if (s->shm_fd >=3D 0) { > > + if (memory_region_is_mapped(&s->ivshmem)) { > > error_report("shm already initialized"); > > close(incoming_fd); > > return; > > @@ -564,9 +561,7 @@ static void ivshmem_read(void *opaque, const uint8_= t > > *buf, int size) > > =20 > > memory_region_add_subregion(&s->bar, 0, &s->ivshmem); > > =20 > > - /* only store the fd if it is successfully mapped */ > > - s->shm_fd =3D incoming_fd; > > - > > + close(incoming_fd); > > return; > > } > > =20 > > @@ -827,6 +822,7 @@ static void pci_ivshmem_realize(PCIDevice *dev, Err= or > > **errp) > > } > > =20 > > create_shared_memory_BAR(s, fd, attr, errp); > > + close(fd); > > } > > } > > =20 > > @@ -842,7 +838,7 @@ static void pci_ivshmem_exit(PCIDevice *dev) > > error_free(s->migration_blocker); > > } > > =20 > > - if (s->shm_fd >=3D 0) { > > + if (memory_region_is_mapped(&s->ivshmem)) { > > void *addr =3D memory_region_get_ram_ptr(&s->ivshmem); > > =20 > > vmstate_unregister_ram(&s->ivshmem, DEVICE(dev)); > >=20 >=20 >=20 >=20 >=20