On Fri, May 23, 2025 at 10:23:58AM +0200, Magnus Kulke wrote:
> On Tue, May 20, 2025 at 07:07:06PM +0000, Wei Liu wrote:
> > On Tue, May 20, 2025 at 01:30:01PM +0200, Magnus Kulke wrote:
> > > Create the MSHV virtual machine by opening a partition and issuing
> > > the necessary ioctl to initialize it. This sets up the basic VM
> > > structure and initial configuration used by MSHV to manage guest state.
> > > 
> > > Signed-off-by: Magnus Kulke <magnusku...@linux.microsoft.com>
> > > ---
> > [...]
> > 
> > mshv_fd is neither stashed into a state structure nor freed after this
> > point.  Is it leaked?
> > 
> > Thanks,
> > Wei.
> > 
> 
> AFAIK the accelerator should not be initialized multiple times at runtime,
> so under normal circumstances the fd wouldn't leak. But in certain debug
> scenarios that would be the case. So, yes, we should make this more solid
> and exit early if MSHV_STATE has been previously initialized.
> 

I'm not talking about initialization specifically. I don't think QEMU
calls the initialization function of an accelerator multiple times.

What I mean is that after this point, the fd is neither closed nor
tracked. There is no way to cleanly handle it other than waiting for the
process to exist. One fd may not seem a lot, but it takes up precise
space in the file descriptor table in the kernel and is counted against
the fd limit.

My suggestion would be if this fd is no longer needed, it can be closed
in this same function.

If it is needed throughout the life cycle of the VM, we put it in a
either a global variable or (better) the accelerator state structure. If
we do the latter, we should also close it when we deinitialize the
accelerator if we have such a phase.

Thanks,
Wei.

> > >      s->nr_as = 1;
> > >      s->as = g_new0(MshvAddressSpace, s->nr_as);
> > >  
> > >      mshv_state = s;
> > >  
> > > +    qemu_register_reset(mshv_reset, NULL);
> > > +
> > >      register_mshv_memory_listener(s, &s->memory_listener, 
> > > &address_space_memory,
> > >                                    0, "mshv-memory");
> > >      memory_listener_register(&mshv_io_listener, &address_space_io);

Reply via email to