Re: [PATCH v2 4/5] migration: Teach QEMUFile to be QIOChannel-aware
* Peter Xu (pet...@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
Yep, one day we'll sort out the block case, but until then
Reviewed-by: Dr. David Alan Gilbert
> ---
> 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, _input_ops);
> +return qemu_fopen_ops(ioc, _input_ops, true);
> }
>
> QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc)
> {
> object_ref(OBJECT(ioc));
> -return qemu_fopen_ops(ioc, _output_ops);
> +return qemu_fopen_ops(ioc, _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
> #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, _ops);
> +comp_param[i].file = qemu_fopen_ops(NULL, _ops, false);
> comp_param[i].done = true;
> comp_param[i].quit = false;
> qemu_mutex_init(_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
[PATCH v2 4/5] migration: Teach QEMUFile to be QIOChannel-aware
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
---
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, _input_ops);
+return qemu_fopen_ops(ioc, _input_ops, true);
}
QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc)
{
object_ref(OBJECT(ioc));
-return qemu_fopen_ops(ioc, _output_ops);
+return qemu_fopen_ops(ioc, _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
#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, _ops);
+comp_param[i].file = qemu_fopen_ops(NULL, _ops, false);
comp_param[i].done = true;
comp_param[i].quit = false;
qemu_mutex_init(_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, _write_ops);
+return qemu_fopen_ops(bs, _write_ops, false);
}
-return qemu_fopen_ops(bs, _read_ops);
+return qemu_fopen_ops(bs, _read_ops, false);
}
--
2.31.1
