Re: [PATCH v2 28/29] migration: Add direct-io parameter

[Fabiano Rosas]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Oct 31, 2023 at 10:05:56AM -0300, Fabiano Rosas wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>> 
>> > On Mon, Oct 30, 2023 at 07:51:34PM -0300, Fabiano Rosas wrote:
>> >> I could use some advice on how to solve this situation. The fdset code
>> >> at monitor/fds.c and the add-fd command don't seem to be usable outside
>> >> the original use-case of passing fds with different open flags.
>> >> 
>> >> There are several problems, the biggest one being that there's no way to
>> >> manipulate the set of file descriptors aside from asking for duplication
>> >> of an fd that matches a particular set of flags.
>> >> 
>> >> That doesn't work for us because the two fds we need (one for main
>> >> channel, other for secondary channels) will have the same open flags. So
>> >> the fdset code will always return the first one it finds in the set.
>> >
>> > QEMU may want multiple FDs *internally*, but IMHO that fact should
>> > not be exposed to mgmt applications. It would be valid for a QEMU
>> > impl to share the same FD across multiple threads, or have a different
>> > FD for each thread. All threads are using pread/pwrite, so it is safe
>> > for them to use the same FD if they desire. It is a private impl choice
>> > for QEMU at any given point in time and could change over time.
>> >
>> 
>> Sure, I don't disagree. However up until last week we had a seemingly
>> usable "add-fd" command that allows the user to provide a *set of file
>> descriptors* to QEMU. It's just now that we're learning that interface
>> serves only a special use-case.
>
> AFAICT though we don't need add-fd to support passing many files
> for our needs. Saving only requires a single FD. All others can
> be opened by dup(), so the limitation of add-fd is irrelevant
> surely ?

Only once we decide to use one FD. If we had a generic add-fd backend,
then that's already a user-facing API, so the "implementation detail"
argument becomes weaker.

With a single FD we'll need to be very careful about what code is
allowed to run while the multifd channels are doing IO. Since O_DIRECT
is not widely supported, now we have to also be careful about someone
using that QEMUFile handle to do unaligned writes and not even noticing
that it breaks direct IO. None of this in unworkable, of course, I just
find the design way clearer with just the file name + offset.

>> > Thus from the POV of the mgmt app, QEMU is writing to a single file, no
>> > matter how many threads are involved & thus it should only need to supply
>> > a single FD for thta file. QEMU can then call 'dup()' on that FD as many
>> > times as it desires, and use fcntl() to toggle O_DIRECT if and when
>> > it needs to.
>> 
>> Ok, so I think the way to go here is for QEMU to receive a file + offset
>> instead of an FD. That way QEMU can have adequate control of the
>> resources for the migration. I don't remember why we went on the FD
>> tangent. Is it not acceptable for libvirt to provide the file name +
>> offset?
>
> FD passing means QEMU does not need privileges to open the file
> which could be useful.

Ok, let me give this a try. Let's see how bad it is to juggle the flag
around the main channel.