Re: [PATCH v3 16/16] tests/qtest/migration: Add a test for mapped-ram with passing of fds

[Fabiano Rosas <farosas@suse.de>]

Peter Xu <peterx@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 <farosas@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.