Daniel P. Berrangé <berra...@redhat.com> writes:
> On Mon, Oct 23, 2023 at 05:35:58PM -0300, Fabiano Rosas wrote:
>> Allow multifd to open file-backed channels. This will be used when
>> enabling the fixed-ram migration stream format which expects a
>> seekable transport.
>>
>> The QIOChannel read and write methods will use the preadv/pwritev
>> versions which don't update the file offset at each call so we can
>> reuse the fd without re-opening for every channel.
>>
>> Note that this is just setup code and multifd cannot yet make use of
>> the file channels.
>>
>> Signed-off-by: Fabiano Rosas <faro...@suse.de>
>> ---
>> migration/file.c | 64 +++++++++++++++++++++++++++++++++++++++++--
>> migration/file.h | 10 +++++--
>> migration/migration.c | 2 +-
>> migration/multifd.c | 14 ++++++++--
>> migration/options.c | 7 +++++
>> migration/options.h | 1 +
>> 6 files changed, 90 insertions(+), 8 deletions(-)
>>
>> diff --git a/migration/file.c b/migration/file.c
>> index cf5b1bf365..93b9b7bf5d 100644
>> --- a/migration/file.c
>> +++ b/migration/file.c
>> @@ -17,6 +17,12 @@
>
>> +void file_send_channel_create(QIOTaskFunc f, void *data)
>> +{
>> + QIOChannelFile *ioc;
>> + QIOTask *task;
>> + Error *errp = NULL;
>> +
>> + ioc = qio_channel_file_new_path(outgoing_args.fname,
>> + outgoing_args.flags,
>> + outgoing_args.mode, &errp);
>> + if (!ioc) {
>> + file_migration_cancel(errp);
>> + return;
>> + }
>> +
>> + task = qio_task_new(OBJECT(ioc), f, (gpointer)data, NULL);
>> + qio_task_run_in_thread(task, qio_channel_file_connect_worker,
>> + (gpointer)data, NULL, NULL);
>> +}
>> +
>> void file_start_outgoing_migration(MigrationState *s, const char *filespec,
>> Error **errp)
>> {
>> - g_autofree char *filename = g_strdup(filespec);
>> g_autoptr(QIOChannelFile) fioc = NULL;
>> + g_autofree char *filename = g_strdup(filespec);
>> uint64_t offset = 0;
>> QIOChannel *ioc;
>> + int flags = O_CREAT | O_TRUNC | O_WRONLY;
>> + mode_t mode = 0660;
>>
>> trace_migration_file_outgoing(filename);
>>
>> @@ -50,12 +105,15 @@ void file_start_outgoing_migration(MigrationState *s,
>> const char *filespec,
>> return;
>> }
>>
>> - fioc = qio_channel_file_new_path(filename, O_CREAT | O_WRONLY | O_TRUNC,
>> - 0600, errp);
By the way, we're experimenting with add-fd to flesh out the interface
with libvirt and I see that the flags here can conflict with the flags
set on the fd passed through `virsh --pass-fd ...` due to this at
monitor_fdset_dup_fd_add():
if ((flags & O_ACCMODE) == (mon_fd_flags & O_ACCMODE)) {
fd = mon_fdset_fd->fd;
break;
}
We're requiring the O_RDONLY, O_WRONLY, O_RDWR flags defined here to
match the fdset passed into QEMU. Should we just sync the code of the
two projects to use the same flags? That feels a little clumsy to me.
>> + fioc = qio_channel_file_new_path(filename, flags, mode, errp);
>
> So this initially opens the file with O_CREAT|O_TRUNC which
> makes sense.
>
>> if (!fioc) {
>> return;
>> }
>>
>> + outgoing_args.fname = g_strdup(filename);
>> + outgoing_args.flags = flags;
>> + outgoing_args.mode = mode;
>
> We're passing on O_CREAT|O_TRUNC to all the multifd threads too. This
> doesn't make sense to me - the file should already exist and be truncated
> by the time the threads open it. I would think they should only be using
> O_WRONLY and no mode at all.
>
Ok.
