Peter Xu <pet...@redhat.com> writes: > On Mon, Jun 17, 2024 at 03:57:31PM -0300, Fabiano Rosas wrote: >> Add a multifd test for mapped-ram with passing of fds into QEMU. This >> is how libvirt will consume the feature. >> >> There are a couple of details to the fdset mechanism: >> >> - multifd needs two distinct file descriptors (not duplicated with >> dup()) so it can enable O_DIRECT only on the channels that do >> aligned IO. The dup() system call creates file descriptors that >> share status flags, of which O_DIRECT is one. >> >> - the open() access mode flags used for the fds passed into QEMU need >> to match the flags QEMU uses to open the file. Currently O_WRONLY >> for src and O_RDONLY for dst. >> >> Note that fdset code goes under _WIN32 because fd passing is not >> supported on Windows. >> >> Signed-off-by: Fabiano Rosas <faro...@suse.de> >> --- >> - dropped Peter's r-b >> >> - stopped removing the fdset at the end of the tests. The migration >> code should always cleanup after itself. > > Ah, that looks also ok.
I made a mistake here. We still need to require that the management layer explicitly removes the fds they added by calling qmp_remove_fd(). The reason I thought it was ok to not remove the fds was that after your suggestion to use monitor_fdset_free_if_empty() in patch 7, I mistakenly put monitor_fdset_free() instead. So the qmp_remove_fd() was not finding any fdsets and I thought that was QEMU removing everything. Which is silly, because the whole purpose of the patch is to not do that. I think I'll just fix this in the migration tree. It's just a revert to v2 of this patch and the correction to patch 7.