| Message ID | 20240617185731.9725-3-farosas@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration/mapped-ram: Add direct-io support | expand |
17.06.2024 21:57, Fabiano Rosas wrote: > When the "file:" migration support was added we missed the special > case in the qemu_open_old implementation that allows for a particular > file name format to be used to refer to a set of file descriptors that > have been previously provided to QEMU via the add-fd QMP command. > > When using this fdset feature, we should not truncate the migration > file because being given an fd means that the management layer is in > control of the file and will likely already have some data written to > it. This is further indicated by the presence of the 'offset' > argument, which indicates the start of the region where QEMU is > allowed to write. > > Fix the issue by replacing the O_TRUNC flag on open by an ftruncate > call, which will take the offset into consideration. > > Fixes: 385f510df5 ("migration: file URI offset") > Suggested-by: Daniel P. Berrangé <berrange@redhat.com> > Reviewed-by: Prasad Pandit <pjp@fedoraproject.org> > Reviewed-by: Peter Xu <peterx@redhat.com> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > migration/file.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) Is it a stable material? Thanks, /mjt > diff --git a/migration/file.c b/migration/file.c > index 2bb8c64092..a903710f06 100644 > --- a/migration/file.c > +++ b/migration/file.c > @@ -84,12 +84,19 @@ void file_start_outgoing_migration(MigrationState *s, > > trace_migration_file_outgoing(filename); > > - fioc = qio_channel_file_new_path(filename, O_CREAT | O_WRONLY | O_TRUNC, > - 0600, errp); > + fioc = qio_channel_file_new_path(filename, O_CREAT | O_WRONLY, 0600, errp); > if (!fioc) { > return; > } > > + if (ftruncate(fioc->fd, offset)) { > + error_setg_errno(errp, errno, > + "failed to truncate migration file to offset %" PRIx64, > + offset); > + object_unref(OBJECT(fioc)); > + return; > + } > + > outgoing_args.fname = g_strdup(filename); > > ioc = QIO_CHANNEL(fioc);
On Sat, Jun 22, 2024 at 07:21:52AM +0300, Michael Tokarev wrote: > 17.06.2024 21:57, Fabiano Rosas wrote: > > When the "file:" migration support was added we missed the special > > case in the qemu_open_old implementation that allows for a particular > > file name format to be used to refer to a set of file descriptors that > > have been previously provided to QEMU via the add-fd QMP command. > > > > When using this fdset feature, we should not truncate the migration > > file because being given an fd means that the management layer is in > > control of the file and will likely already have some data written to > > it. This is further indicated by the presence of the 'offset' > > argument, which indicates the start of the region where QEMU is > > allowed to write. > > > > Fix the issue by replacing the O_TRUNC flag on open by an ftruncate > > call, which will take the offset into consideration. > > > > Fixes: 385f510df5 ("migration: file URI offset") > > Suggested-by: Daniel P. Berrangé <berrange@redhat.com> > > Reviewed-by: Prasad Pandit <pjp@fedoraproject.org> > > Reviewed-by: Peter Xu <peterx@redhat.com> > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > Signed-off-by: Fabiano Rosas <farosas@suse.de> > > --- > > migration/file.c | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > Is it a stable material? I suppose yes. Thanks.
diff --git a/migration/file.c b/migration/file.c index 2bb8c64092..a903710f06 100644 --- a/migration/file.c +++ b/migration/file.c @@ -84,12 +84,19 @@ void file_start_outgoing_migration(MigrationState *s, trace_migration_file_outgoing(filename); - fioc = qio_channel_file_new_path(filename, O_CREAT | O_WRONLY | O_TRUNC, - 0600, errp); + fioc = qio_channel_file_new_path(filename, O_CREAT | O_WRONLY, 0600, errp); if (!fioc) { return; } + if (ftruncate(fioc->fd, offset)) { + error_setg_errno(errp, errno, + "failed to truncate migration file to offset %" PRIx64, + offset); + object_unref(OBJECT(fioc)); + return; + } + outgoing_args.fname = g_strdup(filename); ioc = QIO_CHANNEL(fioc);
When the "file:" migration support was added we missed the special case in the qemu_open_old implementation that allows for a particular file name format to be used to refer to a set of file descriptors that have been previously provided to QEMU via the add-fd QMP command. When using this fdset feature, we should not truncate the migration file because being given an fd means that the management layer is in control of the file and will likely already have some data written to it. This is further indicated by the presence of the 'offset' argument, which indicates the start of the region where QEMU is allowed to write. Fix the issue by replacing the O_TRUNC flag on open by an ftruncate call, which will take the offset into consideration. Fixes: 385f510df5 ("migration: file URI offset") Suggested-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Prasad Pandit <pjp@fedoraproject.org> Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Fabiano Rosas <farosas@suse.de> --- migration/file.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)