| Message ID | 20210721193409.910462-5-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migrations: Fix potential rare race of migration-test after yank | expand |
* Peter Xu (peterx@redhat.com) wrote: > migration uses QIOChannel typed qemufiles. In follow up patches, we'll need > the capability to identify this fact, so that we can get the backing QIOChannel > from a QEMUFile. > > We can also define types for QEMUFile but so far since we only need to be able > to identify QIOChannel, introduce a boolean which is simpler. > > Introduce another helper qemu_file_get_ioc() to return the ioc backend of a > qemufile if has_ioc is set. > > No functional change. > > Signed-off-by: Peter Xu <peterx@redhat.com> Yep, one day we'll sort out the block case, but until then Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > migration/qemu-file-channel.c | 4 ++-- > migration/qemu-file.c | 17 ++++++++++++++++- > migration/qemu-file.h | 4 +++- > migration/ram.c | 2 +- > migration/savevm.c | 4 ++-- > 5 files changed, 24 insertions(+), 7 deletions(-) > > diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c > index 867a5ed0c3..2f8b1fcd46 100644 > --- a/migration/qemu-file-channel.c > +++ b/migration/qemu-file-channel.c > @@ -187,11 +187,11 @@ static const QEMUFileOps channel_output_ops = { > QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc) > { > object_ref(OBJECT(ioc)); > - return qemu_fopen_ops(ioc, &channel_input_ops); > + return qemu_fopen_ops(ioc, &channel_input_ops, true); > } > > QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc) > { > object_ref(OBJECT(ioc)); > - return qemu_fopen_ops(ioc, &channel_output_ops); > + return qemu_fopen_ops(ioc, &channel_output_ops, true); > } > diff --git a/migration/qemu-file.c b/migration/qemu-file.c > index 1eacf9e831..6338d8e2ff 100644 > --- a/migration/qemu-file.c > +++ b/migration/qemu-file.c > @@ -55,6 +55,8 @@ struct QEMUFile { > Error *last_error_obj; > /* has the file has been shutdown */ > bool shutdown; > + /* Whether opaque points to a QIOChannel */ > + bool has_ioc; > }; > > /* > @@ -101,7 +103,7 @@ bool qemu_file_mode_is_not_valid(const char *mode) > return false; > } > > -QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops) > +QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops, bool has_ioc) > { > QEMUFile *f; > > @@ -109,6 +111,7 @@ QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops) > > f->opaque = opaque; > f->ops = ops; > + f->has_ioc = has_ioc; > return f; > } > > @@ -851,3 +854,15 @@ void qemu_file_set_blocking(QEMUFile *f, bool block) > f->ops->set_blocking(f->opaque, block, NULL); > } > } > + > +/* > + * Return the ioc object if it's a migration channel. Note: it can return NULL > + * for callers passing in a non-migration qemufile. E.g. see qemu_fopen_bdrv() > + * and its usage in e.g. load_snapshot(). So we need to check against NULL > + * before using it. If without the check, migration_incoming_state_destroy() > + * could fail for load_snapshot(). > + */ > +QIOChannel *qemu_file_get_ioc(QEMUFile *file) > +{ > + return file->has_ioc ? QIO_CHANNEL(file->opaque) : NULL; > +} > diff --git a/migration/qemu-file.h b/migration/qemu-file.h > index a9b6d6ccb7..3f36d4dc8c 100644 > --- a/migration/qemu-file.h > +++ b/migration/qemu-file.h > @@ -27,6 +27,7 @@ > > #include <zlib.h> > #include "exec/cpu-common.h" > +#include "io/channel.h" > > /* Read a chunk of data from a file at the given position. The pos argument > * can be ignored if the file is only be used for streaming. The number of > @@ -119,7 +120,7 @@ typedef struct QEMUFileHooks { > QEMURamSaveFunc *save_page; > } QEMUFileHooks; > > -QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops); > +QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops, bool has_ioc); > void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks); > int qemu_get_fd(QEMUFile *f); > int qemu_fclose(QEMUFile *f); > @@ -179,5 +180,6 @@ void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data); > size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset, > ram_addr_t offset, size_t size, > uint64_t *bytes_sent); > +QIOChannel *qemu_file_get_ioc(QEMUFile *file); > > #endif > diff --git a/migration/ram.c b/migration/ram.c > index f728f5072f..08b3cb7a4a 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -550,7 +550,7 @@ static int compress_threads_save_setup(void) > /* comp_param[i].file is just used as a dummy buffer to save data, > * set its ops to empty. > */ > - comp_param[i].file = qemu_fopen_ops(NULL, &empty_ops); > + comp_param[i].file = qemu_fopen_ops(NULL, &empty_ops, false); > comp_param[i].done = true; > comp_param[i].quit = false; > qemu_mutex_init(&comp_param[i].mutex); > diff --git a/migration/savevm.c b/migration/savevm.c > index 72848b946c..96b5e5d639 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -168,9 +168,9 @@ static const QEMUFileOps bdrv_write_ops = { > static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable) > { > if (is_writable) { > - return qemu_fopen_ops(bs, &bdrv_write_ops); > + return qemu_fopen_ops(bs, &bdrv_write_ops, false); > } > - return qemu_fopen_ops(bs, &bdrv_read_ops); > + return qemu_fopen_ops(bs, &bdrv_read_ops, false); > } > > > -- > 2.31.1 >
diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c index 867a5ed0c3..2f8b1fcd46 100644 --- a/migration/qemu-file-channel.c +++ b/migration/qemu-file-channel.c @@ -187,11 +187,11 @@ static const QEMUFileOps channel_output_ops = { QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc) { object_ref(OBJECT(ioc)); - return qemu_fopen_ops(ioc, &channel_input_ops); + return qemu_fopen_ops(ioc, &channel_input_ops, true); } QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc) { object_ref(OBJECT(ioc)); - return qemu_fopen_ops(ioc, &channel_output_ops); + return qemu_fopen_ops(ioc, &channel_output_ops, true); } diff --git a/migration/qemu-file.c b/migration/qemu-file.c index 1eacf9e831..6338d8e2ff 100644 --- a/migration/qemu-file.c +++ b/migration/qemu-file.c @@ -55,6 +55,8 @@ struct QEMUFile { Error *last_error_obj; /* has the file has been shutdown */ bool shutdown; + /* Whether opaque points to a QIOChannel */ + bool has_ioc; }; /* @@ -101,7 +103,7 @@ bool qemu_file_mode_is_not_valid(const char *mode) return false; } -QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops) +QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops, bool has_ioc) { QEMUFile *f; @@ -109,6 +111,7 @@ QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops) f->opaque = opaque; f->ops = ops; + f->has_ioc = has_ioc; return f; } @@ -851,3 +854,15 @@ void qemu_file_set_blocking(QEMUFile *f, bool block) f->ops->set_blocking(f->opaque, block, NULL); } } + +/* + * Return the ioc object if it's a migration channel. Note: it can return NULL + * for callers passing in a non-migration qemufile. E.g. see qemu_fopen_bdrv() + * and its usage in e.g. load_snapshot(). So we need to check against NULL + * before using it. If without the check, migration_incoming_state_destroy() + * could fail for load_snapshot(). + */ +QIOChannel *qemu_file_get_ioc(QEMUFile *file) +{ + return file->has_ioc ? QIO_CHANNEL(file->opaque) : NULL; +} diff --git a/migration/qemu-file.h b/migration/qemu-file.h index a9b6d6ccb7..3f36d4dc8c 100644 --- a/migration/qemu-file.h +++ b/migration/qemu-file.h @@ -27,6 +27,7 @@ #include <zlib.h> #include "exec/cpu-common.h" +#include "io/channel.h" /* Read a chunk of data from a file at the given position. The pos argument * can be ignored if the file is only be used for streaming. The number of @@ -119,7 +120,7 @@ typedef struct QEMUFileHooks { QEMURamSaveFunc *save_page; } QEMUFileHooks; -QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops); +QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops, bool has_ioc); void qemu_file_set_hooks(QEMUFile *f, const QEMUFileHooks *hooks); int qemu_get_fd(QEMUFile *f); int qemu_fclose(QEMUFile *f); @@ -179,5 +180,6 @@ void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data); size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset, ram_addr_t offset, size_t size, uint64_t *bytes_sent); +QIOChannel *qemu_file_get_ioc(QEMUFile *file); #endif diff --git a/migration/ram.c b/migration/ram.c index f728f5072f..08b3cb7a4a 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -550,7 +550,7 @@ static int compress_threads_save_setup(void) /* comp_param[i].file is just used as a dummy buffer to save data, * set its ops to empty. */ - comp_param[i].file = qemu_fopen_ops(NULL, &empty_ops); + comp_param[i].file = qemu_fopen_ops(NULL, &empty_ops, false); comp_param[i].done = true; comp_param[i].quit = false; qemu_mutex_init(&comp_param[i].mutex); diff --git a/migration/savevm.c b/migration/savevm.c index 72848b946c..96b5e5d639 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -168,9 +168,9 @@ static const QEMUFileOps bdrv_write_ops = { static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable) { if (is_writable) { - return qemu_fopen_ops(bs, &bdrv_write_ops); + return qemu_fopen_ops(bs, &bdrv_write_ops, false); } - return qemu_fopen_ops(bs, &bdrv_read_ops); + return qemu_fopen_ops(bs, &bdrv_read_ops, false); }
migration uses QIOChannel typed qemufiles. In follow up patches, we'll need the capability to identify this fact, so that we can get the backing QIOChannel from a QEMUFile. We can also define types for QEMUFile but so far since we only need to be able to identify QIOChannel, introduce a boolean which is simpler. Introduce another helper qemu_file_get_ioc() to return the ioc backend of a qemufile if has_ioc is set. No functional change. Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/qemu-file-channel.c | 4 ++-- migration/qemu-file.c | 17 ++++++++++++++++- migration/qemu-file.h | 4 +++- migration/ram.c | 2 +- migration/savevm.c | 4 ++-- 5 files changed, 24 insertions(+), 7 deletions(-)