* [PATCH v2 01/11] migration: support file: uri for source migration
2022-10-10 13:33 [PATCH v2 00/11] Add support for fixed ram offsets during migration Nikolay Borisov
@ 2022-10-10 13:33 ` Nikolay Borisov
2022-10-18 8:56 ` Daniel P. Berrangé
2022-10-18 9:10 ` Daniel P. Berrangé
2022-10-10 13:33 ` [PATCH v2 02/11] migration: Add support for 'file:' uri for incoming migration Nikolay Borisov
` (9 subsequent siblings)
10 siblings, 2 replies; 23+ messages in thread
From: Nikolay Borisov @ 2022-10-10 13:33 UTC (permalink / raw)
To: dgilbert, berrange
Cc: qemu-devel, jfehlig, Claudio.Fontana, dfaggioli, Nikolay Borisov
Implement support for a "file:" uri so that a migration can be initiated
directly to a file from QEMU.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
migration/file.c | 23 +++++++++++++++++++++++
migration/file.h | 9 +++++++++
migration/meson.build | 1 +
migration/migration.c | 3 +++
4 files changed, 36 insertions(+)
create mode 100644 migration/file.c
create mode 100644 migration/file.h
diff --git a/migration/file.c b/migration/file.c
new file mode 100644
index 000000000000..02896a7cab99
--- /dev/null
+++ b/migration/file.c
@@ -0,0 +1,23 @@
+#include "qemu/osdep.h"
+#include "channel.h"
+#include "io/channel-file.h"
+#include "file.h"
+#include "qemu/error-report.h"
+
+
+void file_start_outgoing_migration(MigrationState *s, const char *fname, Error **errp)
+{
+ QIOChannelFile *ioc;
+
+ ioc = qio_channel_file_new_path(fname, O_CREAT|O_TRUNC|O_WRONLY, 0660, errp);
+ if (!ioc) {
+ error_report("Error creating a channel");
+ return;
+ }
+
+ qio_channel_set_name(QIO_CHANNEL(ioc), "migration-file-outgoing");
+ migration_channel_connect(s, QIO_CHANNEL(ioc), NULL, NULL);
+ object_unref(OBJECT(ioc));
+}
+
+
diff --git a/migration/file.h b/migration/file.h
new file mode 100644
index 000000000000..d476eb1157f9
--- /dev/null
+++ b/migration/file.h
@@ -0,0 +1,9 @@
+#ifndef QEMU_MIGRATION_FILE_H
+#define QEMU_MIGRATION_FILE_H
+
+void file_start_outgoing_migration(MigrationState *s,
+ const char *filename,
+ Error **errp);
+
+#endif
+
diff --git a/migration/meson.build b/migration/meson.build
index 690487cf1a81..30a8392701c3 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -17,6 +17,7 @@ softmmu_ss.add(files(
'colo.c',
'exec.c',
'fd.c',
+ 'file.c',
'global_state.c',
'migration.c',
'multifd.c',
diff --git a/migration/migration.c b/migration/migration.c
index bb8bbddfe467..8813b78b9a6b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -20,6 +20,7 @@
#include "migration/blocker.h"
#include "exec.h"
#include "fd.h"
+#include "file.h"
#include "socket.h"
#include "sysemu/runstate.h"
#include "sysemu/sysemu.h"
@@ -2414,6 +2415,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
exec_start_outgoing_migration(s, p, &local_err);
} else if (strstart(uri, "fd:", &p)) {
fd_start_outgoing_migration(s, p, &local_err);
+ } else if (strstart(uri, "file:", &p)) {
+ file_start_outgoing_migration(s, p, &local_err);
} else {
if (!(has_resume && resume)) {
yank_unregister_instance(MIGRATION_YANK_INSTANCE);
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 01/11] migration: support file: uri for source migration
2022-10-10 13:33 ` [PATCH v2 01/11] migration: support file: uri for source migration Nikolay Borisov
@ 2022-10-18 8:56 ` Daniel P. Berrangé
2022-10-18 9:10 ` Daniel P. Berrangé
1 sibling, 0 replies; 23+ messages in thread
From: Daniel P. Berrangé @ 2022-10-18 8:56 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: dgilbert, qemu-devel, jfehlig, Claudio.Fontana, dfaggioli
On Mon, Oct 10, 2022 at 04:33:58PM +0300, Nikolay Borisov wrote:
> Implement support for a "file:" uri so that a migration can be initiated
> directly to a file from QEMU.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> migration/file.c | 23 +++++++++++++++++++++++
> migration/file.h | 9 +++++++++
> migration/meson.build | 1 +
> migration/migration.c | 3 +++
> 4 files changed, 36 insertions(+)
> create mode 100644 migration/file.c
> create mode 100644 migration/file.h
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 01/11] migration: support file: uri for source migration
2022-10-10 13:33 ` [PATCH v2 01/11] migration: support file: uri for source migration Nikolay Borisov
2022-10-18 8:56 ` Daniel P. Berrangé
@ 2022-10-18 9:10 ` Daniel P. Berrangé
2022-10-18 9:49 ` Nikolay Borisov
1 sibling, 1 reply; 23+ messages in thread
From: Daniel P. Berrangé @ 2022-10-18 9:10 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: dgilbert, qemu-devel, jfehlig, Claudio.Fontana, dfaggioli
On Mon, Oct 10, 2022 at 04:33:58PM +0300, Nikolay Borisov wrote:
> Implement support for a "file:" uri so that a migration can be initiated
> directly to a file from QEMU.
Can we add a reminder here
Unlike other migration protocol backends, the 'file' protocol cannot
honour non-blocking mode. POSIX file/block storage will always report
ready to read/write, regardless of how slow the underlying storage
will be at servicing the request.
For outgoing migration this limitation is not a serious problem as
the migration data transfer always happens in a dedicated thread.
It may, however, result in delays in honouring a request to cancel
the migration operation.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> migration/file.c | 23 +++++++++++++++++++++++
> migration/file.h | 9 +++++++++
> migration/meson.build | 1 +
> migration/migration.c | 3 +++
> 4 files changed, 36 insertions(+)
> create mode 100644 migration/file.c
> create mode 100644 migration/file.h
>
> diff --git a/migration/file.c b/migration/file.c
> new file mode 100644
> index 000000000000..02896a7cab99
> --- /dev/null
> +++ b/migration/file.c
> @@ -0,0 +1,23 @@
> +#include "qemu/osdep.h"
> +#include "channel.h"
> +#include "io/channel-file.h"
> +#include "file.h"
> +#include "qemu/error-report.h"
> +
> +
> +void file_start_outgoing_migration(MigrationState *s, const char *fname, Error **errp)
> +{
> + QIOChannelFile *ioc;
> +
> + ioc = qio_channel_file_new_path(fname, O_CREAT|O_TRUNC|O_WRONLY, 0660, errp);
> + if (!ioc) {
> + error_report("Error creating a channel");
> + return;
> + }
> +
> + qio_channel_set_name(QIO_CHANNEL(ioc), "migration-file-outgoing");
> + migration_channel_connect(s, QIO_CHANNEL(ioc), NULL, NULL);
> + object_unref(OBJECT(ioc));
> +}
> +
> +
> diff --git a/migration/file.h b/migration/file.h
> new file mode 100644
> index 000000000000..d476eb1157f9
> --- /dev/null
> +++ b/migration/file.h
> @@ -0,0 +1,9 @@
> +#ifndef QEMU_MIGRATION_FILE_H
> +#define QEMU_MIGRATION_FILE_H
> +
> +void file_start_outgoing_migration(MigrationState *s,
> + const char *filename,
> + Error **errp);
> +
> +#endif
> +
> diff --git a/migration/meson.build b/migration/meson.build
> index 690487cf1a81..30a8392701c3 100644
> --- a/migration/meson.build
> +++ b/migration/meson.build
> @@ -17,6 +17,7 @@ softmmu_ss.add(files(
> 'colo.c',
> 'exec.c',
> 'fd.c',
> + 'file.c',
> 'global_state.c',
> 'migration.c',
> 'multifd.c',
> diff --git a/migration/migration.c b/migration/migration.c
> index bb8bbddfe467..8813b78b9a6b 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -20,6 +20,7 @@
> #include "migration/blocker.h"
> #include "exec.h"
> #include "fd.h"
> +#include "file.h"
> #include "socket.h"
> #include "sysemu/runstate.h"
> #include "sysemu/sysemu.h"
> @@ -2414,6 +2415,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> exec_start_outgoing_migration(s, p, &local_err);
> } else if (strstart(uri, "fd:", &p)) {
> fd_start_outgoing_migration(s, p, &local_err);
> + } else if (strstart(uri, "file:", &p)) {
> + file_start_outgoing_migration(s, p, &local_err);
> } else {
> if (!(has_resume && resume)) {
> yank_unregister_instance(MIGRATION_YANK_INSTANCE);
> --
> 2.34.1
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 01/11] migration: support file: uri for source migration
2022-10-18 9:10 ` Daniel P. Berrangé
@ 2022-10-18 9:49 ` Nikolay Borisov
0 siblings, 0 replies; 23+ messages in thread
From: Nikolay Borisov @ 2022-10-18 9:49 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: dgilbert, qemu-devel, jfehlig, Claudio.Fontana, dfaggioli
On 18.10.22 г. 12:10 ч., Daniel P. Berrangé wrote:
> On Mon, Oct 10, 2022 at 04:33:58PM +0300, Nikolay Borisov wrote:
>> Implement support for a "file:" uri so that a migration can be initiated
>> directly to a file from QEMU.
>
> Can we add a reminder here
>
> Unlike other migration protocol backends, the 'file' protocol cannot
> honour non-blocking mode. POSIX file/block storage will always report
> ready to read/write, regardless of how slow the underlying storage
> will be at servicing the request.
>
> For outgoing migration this limitation is not a serious problem as
> the migration data transfer always happens in a dedicated thread.
> It may, however, result in delays in honouring a request to cancel
> the migration operation.
Sure, I assume the same text should be added to the incoming migration
patch as well and emphasize that it could be more problematic there?
In any case I'd wait to gather more feedback before sending v3
>
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>> migration/file.c | 23 +++++++++++++++++++++++
>> migration/file.h | 9 +++++++++
>> migration/meson.build | 1 +
>> migration/migration.c | 3 +++
>> 4 files changed, 36 insertions(+)
>> create mode 100644 migration/file.c
>> create mode 100644 migration/file.h
>>
>> diff --git a/migration/file.c b/migration/file.c
>> new file mode 100644
>> index 000000000000..02896a7cab99
>> --- /dev/null
>> +++ b/migration/file.c
>> @@ -0,0 +1,23 @@
>> +#include "qemu/osdep.h"
>> +#include "channel.h"
>> +#include "io/channel-file.h"
>> +#include "file.h"
>> +#include "qemu/error-report.h"
>> +
>> +
>> +void file_start_outgoing_migration(MigrationState *s, const char *fname, Error **errp)
>> +{
>> + QIOChannelFile *ioc;
>> +
>> + ioc = qio_channel_file_new_path(fname, O_CREAT|O_TRUNC|O_WRONLY, 0660, errp);
>> + if (!ioc) {
>> + error_report("Error creating a channel");
>> + return;
>> + }
>> +
>> + qio_channel_set_name(QIO_CHANNEL(ioc), "migration-file-outgoing");
>> + migration_channel_connect(s, QIO_CHANNEL(ioc), NULL, NULL);
>> + object_unref(OBJECT(ioc));
>> +}
>> +
>> +
>> diff --git a/migration/file.h b/migration/file.h
>> new file mode 100644
>> index 000000000000..d476eb1157f9
>> --- /dev/null
>> +++ b/migration/file.h
>> @@ -0,0 +1,9 @@
>> +#ifndef QEMU_MIGRATION_FILE_H
>> +#define QEMU_MIGRATION_FILE_H
>> +
>> +void file_start_outgoing_migration(MigrationState *s,
>> + const char *filename,
>> + Error **errp);
>> +
>> +#endif
>> +
>> diff --git a/migration/meson.build b/migration/meson.build
>> index 690487cf1a81..30a8392701c3 100644
>> --- a/migration/meson.build
>> +++ b/migration/meson.build
>> @@ -17,6 +17,7 @@ softmmu_ss.add(files(
>> 'colo.c',
>> 'exec.c',
>> 'fd.c',
>> + 'file.c',
>> 'global_state.c',
>> 'migration.c',
>> 'multifd.c',
>> diff --git a/migration/migration.c b/migration/migration.c
>> index bb8bbddfe467..8813b78b9a6b 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -20,6 +20,7 @@
>> #include "migration/blocker.h"
>> #include "exec.h"
>> #include "fd.h"
>> +#include "file.h"
>> #include "socket.h"
>> #include "sysemu/runstate.h"
>> #include "sysemu/sysemu.h"
>> @@ -2414,6 +2415,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>> exec_start_outgoing_migration(s, p, &local_err);
>> } else if (strstart(uri, "fd:", &p)) {
>> fd_start_outgoing_migration(s, p, &local_err);
>> + } else if (strstart(uri, "file:", &p)) {
>> + file_start_outgoing_migration(s, p, &local_err);
>> } else {
>> if (!(has_resume && resume)) {
>> yank_unregister_instance(MIGRATION_YANK_INSTANCE);
>> --
>> 2.34.1
>>
>
> With regards,
> Daniel
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 02/11] migration: Add support for 'file:' uri for incoming migration
2022-10-10 13:33 [PATCH v2 00/11] Add support for fixed ram offsets during migration Nikolay Borisov
2022-10-10 13:33 ` [PATCH v2 01/11] migration: support file: uri for source migration Nikolay Borisov
@ 2022-10-10 13:33 ` Nikolay Borisov
2022-10-18 10:01 ` Daniel P. Berrangé
2022-10-10 13:34 ` [PATCH v2 03/11] migration: Make migration json writer part of MigrationState struct Nikolay Borisov
` (8 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Nikolay Borisov @ 2022-10-10 13:33 UTC (permalink / raw)
To: dgilbert, berrange
Cc: qemu-devel, jfehlig, Claudio.Fontana, dfaggioli, Nikolay Borisov
This is a counterpart to the 'file:' uri support for source migration,
now a file can also serve as the source of an incoming migration.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
migration/file.c | 15 +++++++++++++++
migration/file.h | 1 +
migration/migration.c | 2 ++
3 files changed, 18 insertions(+)
diff --git a/migration/file.c b/migration/file.c
index 02896a7cab99..93eb718aa0f4 100644
--- a/migration/file.c
+++ b/migration/file.c
@@ -21,3 +21,18 @@ void file_start_outgoing_migration(MigrationState *s, const char *fname, Error *
}
+void file_start_incoming_migration(const char *fname, Error **errp)
+{
+ QIOChannelFile *ioc;
+
+ ioc = qio_channel_file_new_path(fname, O_RDONLY, 0, errp);
+ if (!ioc) {
+ error_report("Error creating a channel");
+ return;
+ }
+
+ qio_channel_set_name(QIO_CHANNEL(ioc), "migration-file-incoming");
+ migration_channel_process_incoming(QIO_CHANNEL(ioc));
+ object_unref(OBJECT(ioc));
+}
+
diff --git a/migration/file.h b/migration/file.h
index d476eb1157f9..cdbd291322d4 100644
--- a/migration/file.h
+++ b/migration/file.h
@@ -5,5 +5,6 @@ void file_start_outgoing_migration(MigrationState *s,
const char *filename,
Error **errp);
+void file_start_incoming_migration(const char *fname, Error **errp);
#endif
diff --git a/migration/migration.c b/migration/migration.c
index 8813b78b9a6b..140b0f1a54bd 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -506,6 +506,8 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp)
exec_start_incoming_migration(p, errp);
} else if (strstart(uri, "fd:", &p)) {
fd_start_incoming_migration(p, errp);
+ } else if (strstart(uri, "file:", &p)) {
+ file_start_incoming_migration(p, errp);
} else {
error_setg(errp, "unknown migration protocol: %s", uri);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 02/11] migration: Add support for 'file:' uri for incoming migration
2022-10-10 13:33 ` [PATCH v2 02/11] migration: Add support for 'file:' uri for incoming migration Nikolay Borisov
@ 2022-10-18 10:01 ` Daniel P. Berrangé
0 siblings, 0 replies; 23+ messages in thread
From: Daniel P. Berrangé @ 2022-10-18 10:01 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: dgilbert, qemu-devel, jfehlig, Claudio.Fontana, dfaggioli
On Mon, Oct 10, 2022 at 04:33:59PM +0300, Nikolay Borisov wrote:
> This is a counterpart to the 'file:' uri support for source migration,
> now a file can also serve as the source of an incoming migration.
As with the prvious patch, can we add a reminder:
Unlike other migration protocol backends, the 'file' protocol cannot
honour non-blocking mode. POSIX file/block storage will always report
ready to read/write, regardless of how slow the underlying storage
will be at servicing the request.
For incoming migration this limitation may result in the main event
loop not being fully responsive while loading the VM state. This
won't impact the VM since it is not running at this phase, however,
it may impact management applications.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> migration/file.c | 15 +++++++++++++++
> migration/file.h | 1 +
> migration/migration.c | 2 ++
> 3 files changed, 18 insertions(+)
> diff --git a/migration/migration.c b/migration/migration.c
> index 8813b78b9a6b..140b0f1a54bd 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -506,6 +506,8 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp)
> exec_start_incoming_migration(p, errp);
> } else if (strstart(uri, "fd:", &p)) {
> fd_start_incoming_migration(p, errp);
> + } else if (strstart(uri, "file:", &p)) {
> + file_start_incoming_migration(p, errp);
A <tab> crept in there by mistake.
> } else {
> error_setg(errp, "unknown migration protocol: %s", uri);
> }
With the above fixed
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 03/11] migration: Make migration json writer part of MigrationState struct
2022-10-10 13:33 [PATCH v2 00/11] Add support for fixed ram offsets during migration Nikolay Borisov
2022-10-10 13:33 ` [PATCH v2 01/11] migration: support file: uri for source migration Nikolay Borisov
2022-10-10 13:33 ` [PATCH v2 02/11] migration: Add support for 'file:' uri for incoming migration Nikolay Borisov
@ 2022-10-10 13:34 ` Nikolay Borisov
2022-10-18 10:06 ` Daniel P. Berrangé
2022-10-10 13:34 ` [PATCH v2 04/11] io: add pwritev support to QIOChannelFile Nikolay Borisov
` (7 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Nikolay Borisov @ 2022-10-10 13:34 UTC (permalink / raw)
To: dgilbert, berrange
Cc: qemu-devel, jfehlig, Claudio.Fontana, dfaggioli, Nikolay Borisov
This is required so that migration stream configuration is written
to the migration stream. This would allow analyze-migration to
parse enabled capabilities for the migration and adjust its behavior
accordingly. This is in preparation for analyze-migration.py to support
'fixed-ram' capability format changes.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
migration/migration.c | 5 +++++
migration/migration.h | 3 +++
migration/savevm.c | 38 ++++++++++++++++++++++----------------
3 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 140b0f1a54bd..d0779bbaf862 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1896,6 +1896,8 @@ static void migrate_fd_cleanup(MigrationState *s)
g_free(s->hostname);
s->hostname = NULL;
+ json_writer_free(s->vmdesc);
+
qemu_savevm_state_cleanup();
if (s->to_dst_file) {
@@ -2154,6 +2156,7 @@ void migrate_init(MigrationState *s)
error_free(s->error);
s->error = NULL;
s->hostname = NULL;
+ s->vmdesc = NULL;
migrate_set_state(&s->state, MIGRATION_STATUS_NONE, MIGRATION_STATUS_SETUP);
@@ -4269,6 +4272,8 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
return;
}
+ s->vmdesc = json_writer_new(false);
+
if (multifd_save_setup(&local_err) != 0) {
error_report_err(local_err);
migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
diff --git a/migration/migration.h b/migration/migration.h
index cdad8aceaaab..96f27aba2210 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -17,6 +17,7 @@
#include "exec/cpu-common.h"
#include "hw/qdev-core.h"
#include "qapi/qapi-types-migration.h"
+#include "qapi/qmp/json-writer.h"
#include "qemu/thread.h"
#include "qemu/coroutine_int.h"
#include "io/channel.h"
@@ -261,6 +262,8 @@ struct MigrationState {
int state;
+ JSONWriter *vmdesc;
+
/* State related to return path */
struct {
/* Protected by qemu_file_lock */
diff --git a/migration/savevm.c b/migration/savevm.c
index 48e85c052c2c..174cdbefc29d 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1137,13 +1137,18 @@ void qemu_savevm_non_migratable_list(strList **reasons)
void qemu_savevm_state_header(QEMUFile *f)
{
+ MigrationState *s = migrate_get_current();
trace_savevm_state_header();
qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
qemu_put_be32(f, QEMU_VM_FILE_VERSION);
- if (migrate_get_current()->send_configuration) {
+ if (s->send_configuration) {
qemu_put_byte(f, QEMU_VM_CONFIGURATION);
- vmstate_save_state(f, &vmstate_configuration, &savevm_state, 0);
+ json_writer_start_object(s->vmdesc, NULL);
+ json_writer_start_object(s->vmdesc, "configuration");
+ vmstate_save_state(f, &vmstate_configuration, &savevm_state, s->vmdesc);
+ json_writer_end_object(s->vmdesc);
+
}
}
@@ -1364,15 +1369,16 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
bool in_postcopy,
bool inactivate_disks)
{
- g_autoptr(JSONWriter) vmdesc = NULL;
+ MigrationState *s = migrate_get_current();
int vmdesc_len;
SaveStateEntry *se;
int ret;
- vmdesc = json_writer_new(false);
- json_writer_start_object(vmdesc, NULL);
- json_writer_int64(vmdesc, "page_size", qemu_target_page_size());
- json_writer_start_array(vmdesc, "devices");
+ if (!s->send_configuration) {
+ json_writer_start_object(s->vmdesc, NULL);
+ }
+ json_writer_int64(s->vmdesc, "page_size", qemu_target_page_size());
+ json_writer_start_array(s->vmdesc, "devices");
QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
@@ -1385,12 +1391,12 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
trace_savevm_section_start(se->idstr, se->section_id);
- json_writer_start_object(vmdesc, NULL);
- json_writer_str(vmdesc, "name", se->idstr);
- json_writer_int64(vmdesc, "instance_id", se->instance_id);
+ json_writer_start_object(s->vmdesc, NULL);
+ json_writer_str(s->vmdesc, "name", se->idstr);
+ json_writer_int64(s->vmdesc, "instance_id", se->instance_id);
save_section_header(f, se, QEMU_VM_SECTION_FULL);
- ret = vmstate_save(f, se, vmdesc);
+ ret = vmstate_save(f, se, s->vmdesc);
if (ret) {
qemu_file_set_error(f, ret);
return ret;
@@ -1398,7 +1404,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
trace_savevm_section_end(se->idstr, se->section_id, 0);
save_section_footer(f, se);
- json_writer_end_object(vmdesc);
+ json_writer_end_object(s->vmdesc);
}
if (inactivate_disks) {
@@ -1417,14 +1423,14 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
qemu_put_byte(f, QEMU_VM_EOF);
}
- json_writer_end_array(vmdesc);
- json_writer_end_object(vmdesc);
- vmdesc_len = strlen(json_writer_get(vmdesc));
+ json_writer_end_array(s->vmdesc);
+ json_writer_end_object(s->vmdesc);
+ vmdesc_len = strlen(json_writer_get(s->vmdesc));
if (should_send_vmdesc()) {
qemu_put_byte(f, QEMU_VM_VMDESCRIPTION);
qemu_put_be32(f, vmdesc_len);
- qemu_put_buffer(f, (uint8_t *)json_writer_get(vmdesc), vmdesc_len);
+ qemu_put_buffer(f, (uint8_t *)json_writer_get(s->vmdesc), vmdesc_len);
}
return 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 03/11] migration: Make migration json writer part of MigrationState struct
2022-10-10 13:34 ` [PATCH v2 03/11] migration: Make migration json writer part of MigrationState struct Nikolay Borisov
@ 2022-10-18 10:06 ` Daniel P. Berrangé
2022-10-19 15:43 ` Nikolay Borisov
0 siblings, 1 reply; 23+ messages in thread
From: Daniel P. Berrangé @ 2022-10-18 10:06 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: dgilbert, qemu-devel, jfehlig, Claudio.Fontana, dfaggioli
On Mon, Oct 10, 2022 at 04:34:00PM +0300, Nikolay Borisov wrote:
> This is required so that migration stream configuration is written
> to the migration stream. This would allow analyze-migration to
> parse enabled capabilities for the migration and adjust its behavior
> accordingly. This is in preparation for analyze-migration.py to support
> 'fixed-ram' capability format changes.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> migration/migration.c | 5 +++++
> migration/migration.h | 3 +++
> migration/savevm.c | 38 ++++++++++++++++++++++----------------
> 3 files changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 140b0f1a54bd..d0779bbaf862 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1896,6 +1896,8 @@ static void migrate_fd_cleanup(MigrationState *s)
> g_free(s->hostname);
> s->hostname = NULL;
>
> + json_writer_free(s->vmdesc);
> +
> qemu_savevm_state_cleanup();
>
> if (s->to_dst_file) {
> @@ -2154,6 +2156,7 @@ void migrate_init(MigrationState *s)
> error_free(s->error);
> s->error = NULL;
> s->hostname = NULL;
> + s->vmdesc = NULL;
>
> migrate_set_state(&s->state, MIGRATION_STATUS_NONE, MIGRATION_STATUS_SETUP);
>
> @@ -4269,6 +4272,8 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
> return;
> }
>
> + s->vmdesc = json_writer_new(false);
> +
> if (multifd_save_setup(&local_err) != 0) {
> error_report_err(local_err);
> migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
> diff --git a/migration/migration.h b/migration/migration.h
> index cdad8aceaaab..96f27aba2210 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -17,6 +17,7 @@
> #include "exec/cpu-common.h"
> #include "hw/qdev-core.h"
> #include "qapi/qapi-types-migration.h"
> +#include "qapi/qmp/json-writer.h"
> #include "qemu/thread.h"
> #include "qemu/coroutine_int.h"
> #include "io/channel.h"
> @@ -261,6 +262,8 @@ struct MigrationState {
>
> int state;
>
> + JSONWriter *vmdesc;
> +
> /* State related to return path */
> struct {
> /* Protected by qemu_file_lock */
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 48e85c052c2c..174cdbefc29d 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1137,13 +1137,18 @@ void qemu_savevm_non_migratable_list(strList **reasons)
>
> void qemu_savevm_state_header(QEMUFile *f)
> {
> + MigrationState *s = migrate_get_current();
> trace_savevm_state_header();
> qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
> qemu_put_be32(f, QEMU_VM_FILE_VERSION);
>
> - if (migrate_get_current()->send_configuration) {
> + if (s->send_configuration) {
> qemu_put_byte(f, QEMU_VM_CONFIGURATION);
> - vmstate_save_state(f, &vmstate_configuration, &savevm_state, 0);
> + json_writer_start_object(s->vmdesc, NULL);
> + json_writer_start_object(s->vmdesc, "configuration");
> + vmstate_save_state(f, &vmstate_configuration, &savevm_state, s->vmdesc);
> + json_writer_end_object(s->vmdesc);
> +
IIUC, this is changing the info that is written in the VM
configuration section, by adding an extra level of nesting
to the object.
Isn't this going to cause backwards compatibility problems ?
Nothing in the patch seems to take account of the exctra
'configuiration' object that has been started
Also, there's two json_writer_start_object calls, but only
one json_writer_end_object.
BTW, some <tab> crept into this patch.
> }
> }
>
> @@ -1364,15 +1369,16 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
> bool in_postcopy,
> bool inactivate_disks)
> {
> - g_autoptr(JSONWriter) vmdesc = NULL;
> + MigrationState *s = migrate_get_current();
> int vmdesc_len;
> SaveStateEntry *se;
> int ret;
>
> - vmdesc = json_writer_new(false);
> - json_writer_start_object(vmdesc, NULL);
> - json_writer_int64(vmdesc, "page_size", qemu_target_page_size());
> - json_writer_start_array(vmdesc, "devices");
> + if (!s->send_configuration) {
> + json_writer_start_object(s->vmdesc, NULL);
> + }
> + json_writer_int64(s->vmdesc, "page_size", qemu_target_page_size());
> + json_writer_start_array(s->vmdesc, "devices");
> QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>
> if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
> @@ -1385,12 +1391,12 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>
> trace_savevm_section_start(se->idstr, se->section_id);
>
> - json_writer_start_object(vmdesc, NULL);
> - json_writer_str(vmdesc, "name", se->idstr);
> - json_writer_int64(vmdesc, "instance_id", se->instance_id);
> + json_writer_start_object(s->vmdesc, NULL);
> + json_writer_str(s->vmdesc, "name", se->idstr);
> + json_writer_int64(s->vmdesc, "instance_id", se->instance_id);
>
> save_section_header(f, se, QEMU_VM_SECTION_FULL);
> - ret = vmstate_save(f, se, vmdesc);
> + ret = vmstate_save(f, se, s->vmdesc);
> if (ret) {
> qemu_file_set_error(f, ret);
> return ret;
> @@ -1398,7 +1404,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
> trace_savevm_section_end(se->idstr, se->section_id, 0);
> save_section_footer(f, se);
>
> - json_writer_end_object(vmdesc);
> + json_writer_end_object(s->vmdesc);
> }
>
> if (inactivate_disks) {
> @@ -1417,14 +1423,14 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
> qemu_put_byte(f, QEMU_VM_EOF);
> }
>
> - json_writer_end_array(vmdesc);
> - json_writer_end_object(vmdesc);
> - vmdesc_len = strlen(json_writer_get(vmdesc));
> + json_writer_end_array(s->vmdesc);
> + json_writer_end_object(s->vmdesc);
> + vmdesc_len = strlen(json_writer_get(s->vmdesc));
>
> if (should_send_vmdesc()) {
> qemu_put_byte(f, QEMU_VM_VMDESCRIPTION);
> qemu_put_be32(f, vmdesc_len);
> - qemu_put_buffer(f, (uint8_t *)json_writer_get(vmdesc), vmdesc_len);
> + qemu_put_buffer(f, (uint8_t *)json_writer_get(s->vmdesc), vmdesc_len);
> }
>
> return 0;
> --
> 2.34.1
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 03/11] migration: Make migration json writer part of MigrationState struct
2022-10-18 10:06 ` Daniel P. Berrangé
@ 2022-10-19 15:43 ` Nikolay Borisov
2022-10-19 16:02 ` Daniel P. Berrangé
0 siblings, 1 reply; 23+ messages in thread
From: Nikolay Borisov @ 2022-10-19 15:43 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: dgilbert, qemu-devel, jfehlig, Claudio.Fontana, dfaggioli
On 18.10.22 г. 13:06 ч., Daniel P. Berrangé wrote:
> On Mon, Oct 10, 2022 at 04:34:00PM +0300, Nikolay Borisov wrote:
>> This is required so that migration stream configuration is written
>> to the migration stream. This would allow analyze-migration to
>> parse enabled capabilities for the migration and adjust its behavior
>> accordingly. This is in preparation for analyze-migration.py to support
>> 'fixed-ram' capability format changes.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>> migration/migration.c | 5 +++++
>> migration/migration.h | 3 +++
>> migration/savevm.c | 38 ++++++++++++++++++++++----------------
>> 3 files changed, 30 insertions(+), 16 deletions(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 140b0f1a54bd..d0779bbaf862 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -1896,6 +1896,8 @@ static void migrate_fd_cleanup(MigrationState *s)
>> g_free(s->hostname);
>> s->hostname = NULL;
>>
>> + json_writer_free(s->vmdesc);
>> +
>> qemu_savevm_state_cleanup();
>>
>> if (s->to_dst_file) {
>> @@ -2154,6 +2156,7 @@ void migrate_init(MigrationState *s)
>> error_free(s->error);
>> s->error = NULL;
>> s->hostname = NULL;
>> + s->vmdesc = NULL;
>>
>> migrate_set_state(&s->state, MIGRATION_STATUS_NONE, MIGRATION_STATUS_SETUP);
>>
>> @@ -4269,6 +4272,8 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
>> return;
>> }
>>
>> + s->vmdesc = json_writer_new(false);
>> +
>> if (multifd_save_setup(&local_err) != 0) {
>> error_report_err(local_err);
>> migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
>> diff --git a/migration/migration.h b/migration/migration.h
>> index cdad8aceaaab..96f27aba2210 100644
>> --- a/migration/migration.h
>> +++ b/migration/migration.h
>> @@ -17,6 +17,7 @@
>> #include "exec/cpu-common.h"
>> #include "hw/qdev-core.h"
>> #include "qapi/qapi-types-migration.h"
>> +#include "qapi/qmp/json-writer.h"
>> #include "qemu/thread.h"
>> #include "qemu/coroutine_int.h"
>> #include "io/channel.h"
>> @@ -261,6 +262,8 @@ struct MigrationState {
>>
>> int state;
>>
>> + JSONWriter *vmdesc;
>> +
>> /* State related to return path */
>> struct {
>> /* Protected by qemu_file_lock */
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index 48e85c052c2c..174cdbefc29d 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -1137,13 +1137,18 @@ void qemu_savevm_non_migratable_list(strList **reasons)
>>
>> void qemu_savevm_state_header(QEMUFile *f)
>> {
>> + MigrationState *s = migrate_get_current();
>> trace_savevm_state_header();
>> qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
>> qemu_put_be32(f, QEMU_VM_FILE_VERSION);
>>
>> - if (migrate_get_current()->send_configuration) {
>> + if (s->send_configuration) {
>> qemu_put_byte(f, QEMU_VM_CONFIGURATION);
>> - vmstate_save_state(f, &vmstate_configuration, &savevm_state, 0);
>> + json_writer_start_object(s->vmdesc, NULL);
>> + json_writer_start_object(s->vmdesc, "configuration");
>> + vmstate_save_state(f, &vmstate_configuration, &savevm_state, s->vmdesc);
>> + json_writer_end_object(s->vmdesc);
>> +
>
> IIUC, this is changing the info that is written in the VM
> configuration section, by adding an extra level of nesting
> to the object.
>
> Isn't this going to cause backwards compatibility problems ?
>
> Nothing in the patch seems to take account of the exctra
> 'configuiration' object that has been started
The resulting json looks like:
{
"configuration": {
"vmsd_name": "configuration",
"version": 1,
"fields": [
{
"name": "len",
"type": "uint32",
"size": 4
},
{
"name": "name",
"type": "buffer",
"size": 13
}
],
"subsections": [
{
"vmsd_name": "configuration/capabilities",
"version": 1,
"fields": [
{
"name": "caps_count",
"type": "uint32",
"size": 4
},
{
"name": "capabilities",
"type": "capability",
"size": 10
}
]
}
]
},
"page_size": 4096,
"devices": [
{
"name": "timer",
"instance_id": 0,
//ommitted
So the "configuration" object is indeed added, but older versions of
qemu can ignore it without any problem.
>
> Also, there's two json_writer_start_object calls, but only
> one json_writer_end_object.
That's intentional, the first one begins the top-level object and it is
actually paired with the final call to
json_writer_end_object(s->vmdesc); in
qemu_savevm_state_complete_precopy_non_iterable .
>
> BTW, some <tab> crept into this patch.
Will fix this.
PS. I usually work on the linux kernel so vim used my linuxsty.vim
settings. However, I eventually instsalled .editorconfig support so I
guess those are leftovers.
>
>
>> }
>> }
>>
>> @@ -1364,15 +1369,16 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>> bool in_postcopy,
>> bool inactivate_disks)
>> {
>> - g_autoptr(JSONWriter) vmdesc = NULL;
>> + MigrationState *s = migrate_get_current();
>> int vmdesc_len;
>> SaveStateEntry *se;
>> int ret;
>>
>> - vmdesc = json_writer_new(false);
>> - json_writer_start_object(vmdesc, NULL);
>> - json_writer_int64(vmdesc, "page_size", qemu_target_page_size());
>> - json_writer_start_array(vmdesc, "devices");
>> + if (!s->send_configuration) {
>> + json_writer_start_object(s->vmdesc, NULL);
>> + }
>> + json_writer_int64(s->vmdesc, "page_size", qemu_target_page_size());
>> + json_writer_start_array(s->vmdesc, "devices");
>> QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>>
>> if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
>> @@ -1385,12 +1391,12 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>>
>> trace_savevm_section_start(se->idstr, se->section_id);
>>
>> - json_writer_start_object(vmdesc, NULL);
>> - json_writer_str(vmdesc, "name", se->idstr);
>> - json_writer_int64(vmdesc, "instance_id", se->instance_id);
>> + json_writer_start_object(s->vmdesc, NULL);
>> + json_writer_str(s->vmdesc, "name", se->idstr);
>> + json_writer_int64(s->vmdesc, "instance_id", se->instance_id);
>>
>> save_section_header(f, se, QEMU_VM_SECTION_FULL);
>> - ret = vmstate_save(f, se, vmdesc);
>> + ret = vmstate_save(f, se, s->vmdesc);
>> if (ret) {
>> qemu_file_set_error(f, ret);
>> return ret;
>> @@ -1398,7 +1404,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>> trace_savevm_section_end(se->idstr, se->section_id, 0);
>> save_section_footer(f, se);
>>
>> - json_writer_end_object(vmdesc);
>> + json_writer_end_object(s->vmdesc);
>> }
>>
>> if (inactivate_disks) {
>> @@ -1417,14 +1423,14 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>> qemu_put_byte(f, QEMU_VM_EOF);
>> }
>>
>> - json_writer_end_array(vmdesc);
>> - json_writer_end_object(vmdesc);
>> - vmdesc_len = strlen(json_writer_get(vmdesc));
>> + json_writer_end_array(s->vmdesc);
>> + json_writer_end_object(s->vmdesc);
>> + vmdesc_len = strlen(json_writer_get(s->vmdesc));
>>
>> if (should_send_vmdesc()) {
>> qemu_put_byte(f, QEMU_VM_VMDESCRIPTION);
>> qemu_put_be32(f, vmdesc_len);
>> - qemu_put_buffer(f, (uint8_t *)json_writer_get(vmdesc), vmdesc_len);
>> + qemu_put_buffer(f, (uint8_t *)json_writer_get(s->vmdesc), vmdesc_len);
>> }
>>
>> return 0;
>> --
>> 2.34.1
>>
>
> With regards,
> Daniel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 03/11] migration: Make migration json writer part of MigrationState struct
2022-10-19 15:43 ` Nikolay Borisov
@ 2022-10-19 16:02 ` Daniel P. Berrangé
0 siblings, 0 replies; 23+ messages in thread
From: Daniel P. Berrangé @ 2022-10-19 16:02 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: dgilbert, qemu-devel, jfehlig, Claudio.Fontana, dfaggioli
On Wed, Oct 19, 2022 at 06:43:46PM +0300, Nikolay Borisov wrote:
>
>
> On 18.10.22 г. 13:06 ч., Daniel P. Berrangé wrote:
> > On Mon, Oct 10, 2022 at 04:34:00PM +0300, Nikolay Borisov wrote:
> > > This is required so that migration stream configuration is written
> > > to the migration stream. This would allow analyze-migration to
> > > parse enabled capabilities for the migration and adjust its behavior
> > > accordingly. This is in preparation for analyze-migration.py to support
> > > 'fixed-ram' capability format changes.
> > >
> > > Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> > > ---
> > > migration/migration.c | 5 +++++
> > > migration/migration.h | 3 +++
> > > migration/savevm.c | 38 ++++++++++++++++++++++----------------
> > > 3 files changed, 30 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/migration/migration.c b/migration/migration.c
> > > index 140b0f1a54bd..d0779bbaf862 100644
> > > --- a/migration/migration.c
> > > +++ b/migration/migration.c
> > > @@ -1896,6 +1896,8 @@ static void migrate_fd_cleanup(MigrationState *s)
> > > g_free(s->hostname);
> > > s->hostname = NULL;
> > > + json_writer_free(s->vmdesc);
> > > +
> > > qemu_savevm_state_cleanup();
> > > if (s->to_dst_file) {
> > > @@ -2154,6 +2156,7 @@ void migrate_init(MigrationState *s)
> > > error_free(s->error);
> > > s->error = NULL;
> > > s->hostname = NULL;
> > > + s->vmdesc = NULL;
> > > migrate_set_state(&s->state, MIGRATION_STATUS_NONE, MIGRATION_STATUS_SETUP);
> > > @@ -4269,6 +4272,8 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
> > > return;
> > > }
> > > + s->vmdesc = json_writer_new(false);
> > > +
> > > if (multifd_save_setup(&local_err) != 0) {
> > > error_report_err(local_err);
> > > migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
> > > diff --git a/migration/migration.h b/migration/migration.h
> > > index cdad8aceaaab..96f27aba2210 100644
> > > --- a/migration/migration.h
> > > +++ b/migration/migration.h
> > > @@ -17,6 +17,7 @@
> > > #include "exec/cpu-common.h"
> > > #include "hw/qdev-core.h"
> > > #include "qapi/qapi-types-migration.h"
> > > +#include "qapi/qmp/json-writer.h"
> > > #include "qemu/thread.h"
> > > #include "qemu/coroutine_int.h"
> > > #include "io/channel.h"
> > > @@ -261,6 +262,8 @@ struct MigrationState {
> > > int state;
> > > + JSONWriter *vmdesc;
> > > +
> > > /* State related to return path */
> > > struct {
> > > /* Protected by qemu_file_lock */
> > > diff --git a/migration/savevm.c b/migration/savevm.c
> > > index 48e85c052c2c..174cdbefc29d 100644
> > > --- a/migration/savevm.c
> > > +++ b/migration/savevm.c
> > > @@ -1137,13 +1137,18 @@ void qemu_savevm_non_migratable_list(strList **reasons)
> > > void qemu_savevm_state_header(QEMUFile *f)
> > > {
> > > + MigrationState *s = migrate_get_current();
> > > trace_savevm_state_header();
> > > qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
> > > qemu_put_be32(f, QEMU_VM_FILE_VERSION);
> > > - if (migrate_get_current()->send_configuration) {
> > > + if (s->send_configuration) {
> > > qemu_put_byte(f, QEMU_VM_CONFIGURATION);
> > > - vmstate_save_state(f, &vmstate_configuration, &savevm_state, 0);
> > > + json_writer_start_object(s->vmdesc, NULL);
> > > + json_writer_start_object(s->vmdesc, "configuration");
> > > + vmstate_save_state(f, &vmstate_configuration, &savevm_state, s->vmdesc);
> > > + json_writer_end_object(s->vmdesc);
> > > +
> >
> > IIUC, this is changing the info that is written in the VM
> > configuration section, by adding an extra level of nesting
> > to the object.
> >
> > Isn't this going to cause backwards compatibility problems ?
> >
> > Nothing in the patch seems to take account of the exctra
> > 'configuiration' object that has been started
>
> The resulting json looks like:
>
> {
> "configuration": {
> "vmsd_name": "configuration",
> "version": 1,
> "fields": [
> {
> "name": "len",
> "type": "uint32",
> "size": 4
> },
> {
> "name": "name",
> "type": "buffer",
> "size": 13
> }
> ],
> "subsections": [
> {
> "vmsd_name": "configuration/capabilities",
> "version": 1,
> "fields": [
> {
> "name": "caps_count",
> "type": "uint32",
> "size": 4
> },
> {
> "name": "capabilities",
> "type": "capability",
> "size": 10
> }
> ]
> }
> ]
> },
> "page_size": 4096,
> "devices": [
> {
> "name": "timer",
> "instance_id": 0,
> //ommitted
>
> So the "configuration" object is indeed added, but older versions of qemu
> can ignore it without any problem.
IIUC, after looking further, this JSON is only used by the
analyze-migration.py script ? If that's right, then we should
have the change to analyze-migration.py that copes with the
"configuration" option in the same patch.
> > Also, there's two json_writer_start_object calls, but only
> > one json_writer_end_object.
>
> That's intentional, the first one begins the top-level object and it is
> actually paired with the final call to json_writer_end_object(s->vmdesc); in
> qemu_savevm_state_complete_precopy_non_iterable .
Oh right, can you add a comment to both locations, mentioning
where their respective pair lives.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 04/11] io: add pwritev support to QIOChannelFile
2022-10-10 13:33 [PATCH v2 00/11] Add support for fixed ram offsets during migration Nikolay Borisov
` (2 preceding siblings ...)
2022-10-10 13:34 ` [PATCH v2 03/11] migration: Make migration json writer part of MigrationState struct Nikolay Borisov
@ 2022-10-10 13:34 ` Nikolay Borisov
2022-10-18 10:11 ` Daniel P. Berrangé
2022-10-10 13:34 ` [PATCH v2 05/11] io: Add support for seekable channels Nikolay Borisov
` (6 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Nikolay Borisov @ 2022-10-10 13:34 UTC (permalink / raw)
To: dgilbert, berrange
Cc: qemu-devel, jfehlig, Claudio.Fontana, dfaggioli, Nikolay Borisov
The upcoming 'fixed-ram' feature would require qemu to write data at
specific offsets of the file. This is currently missing so this patch
adds it. I've chosen to implement it as a type-specific function rather
than plumb it through the generic channel interface as it relies on
being able to do seeks.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
include/io/channel-file.h | 5 +++++
io/channel-file.c | 24 ++++++++++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/include/io/channel-file.h b/include/io/channel-file.h
index 50e8eb113868..0a5d54f5e58e 100644
--- a/include/io/channel-file.h
+++ b/include/io/channel-file.h
@@ -89,4 +89,9 @@ qio_channel_file_new_path(const char *path,
mode_t mode,
Error **errp);
+ssize_t qio_channel_file_pwritev(QIOChannel *ioc,
+ const struct iovec *iov,
+ size_t niov,
+ off_t offset,
+ Error **errp);
#endif /* QIO_CHANNEL_FILE_H */
diff --git a/io/channel-file.c b/io/channel-file.c
index b67687c2aa64..da17d0a11ba7 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -136,6 +136,30 @@ static ssize_t qio_channel_file_writev(QIOChannel *ioc,
return ret;
}
+ssize_t qio_channel_file_pwritev(QIOChannel *ioc,
+ const struct iovec *iov,
+ size_t niov,
+ off_t offset,
+ Error **errp)
+{
+ QIOChannelFile *fioc = QIO_CHANNEL_FILE(ioc);
+ ssize_t ret;
+
+ retry:
+ ret = pwritev(fioc->fd, iov, niov, offset);
+ if (ret <= 0) {
+ if (errno == EAGAIN) {
+ return QIO_CHANNEL_ERR_BLOCK;
+ }
+ if (errno == EINTR) {
+ goto retry;
+ }
+ error_setg_errno(errp, errno, "Unable to write to file");
+ return -1;
+ }
+ return ret;
+}
+
static int qio_channel_file_set_blocking(QIOChannel *ioc,
bool enabled,
Error **errp)
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 04/11] io: add pwritev support to QIOChannelFile
2022-10-10 13:34 ` [PATCH v2 04/11] io: add pwritev support to QIOChannelFile Nikolay Borisov
@ 2022-10-18 10:11 ` Daniel P. Berrangé
0 siblings, 0 replies; 23+ messages in thread
From: Daniel P. Berrangé @ 2022-10-18 10:11 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: dgilbert, qemu-devel, jfehlig, Claudio.Fontana, dfaggioli
On Mon, Oct 10, 2022 at 04:34:01PM +0300, Nikolay Borisov wrote:
> The upcoming 'fixed-ram' feature would require qemu to write data at
> specific offsets of the file. This is currently missing so this patch
> adds it. I've chosen to implement it as a type-specific function rather
> than plumb it through the generic channel interface as it relies on
> being able to do seeks.
Well the base QIOChannel does have an 'io_seek' callback, so it
already has some support for seeking.
In addition to this QIOChannelFile class, the QIOChannelBuffer
and QIOChannelNull classes could also allow seekable IO ops.
So I think it is OK to add this to the QIOChannel base class,
especially since your next patch adds a "SEEKABLE" feature
flag.
We have io_writev/io_readv callbacks. If we add
io_pwritev/io_preadv callbacks too, we should validate that
they exist when the SEEKABLE feature is marked present.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> include/io/channel-file.h | 5 +++++
> io/channel-file.c | 24 ++++++++++++++++++++++++
> 2 files changed, 29 insertions(+)
>
> diff --git a/include/io/channel-file.h b/include/io/channel-file.h
> index 50e8eb113868..0a5d54f5e58e 100644
> --- a/include/io/channel-file.h
> +++ b/include/io/channel-file.h
> @@ -89,4 +89,9 @@ qio_channel_file_new_path(const char *path,
> mode_t mode,
> Error **errp);
>
> +ssize_t qio_channel_file_pwritev(QIOChannel *ioc,
> + const struct iovec *iov,
> + size_t niov,
> + off_t offset,
> + Error **errp);
> #endif /* QIO_CHANNEL_FILE_H */
> diff --git a/io/channel-file.c b/io/channel-file.c
> index b67687c2aa64..da17d0a11ba7 100644
> --- a/io/channel-file.c
> +++ b/io/channel-file.c
> @@ -136,6 +136,30 @@ static ssize_t qio_channel_file_writev(QIOChannel *ioc,
> return ret;
> }
>
> +ssize_t qio_channel_file_pwritev(QIOChannel *ioc,
> + const struct iovec *iov,
> + size_t niov,
> + off_t offset,
> + Error **errp)
> +{
> + QIOChannelFile *fioc = QIO_CHANNEL_FILE(ioc);
> + ssize_t ret;
> +
> + retry:
> + ret = pwritev(fioc->fd, iov, niov, offset);
> + if (ret <= 0) {
> + if (errno == EAGAIN) {
> + return QIO_CHANNEL_ERR_BLOCK;
> + }
> + if (errno == EINTR) {
> + goto retry;
> + }
> + error_setg_errno(errp, errno, "Unable to write to file");
> + return -1;
> + }
> + return ret;
> +}
> +
> static int qio_channel_file_set_blocking(QIOChannel *ioc,
> bool enabled,
> Error **errp)
> --
> 2.34.1
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 05/11] io: Add support for seekable channels
2022-10-10 13:33 [PATCH v2 00/11] Add support for fixed ram offsets during migration Nikolay Borisov
` (3 preceding siblings ...)
2022-10-10 13:34 ` [PATCH v2 04/11] io: add pwritev support to QIOChannelFile Nikolay Borisov
@ 2022-10-10 13:34 ` Nikolay Borisov
2022-10-18 10:14 ` Daniel P. Berrangé
2022-10-10 13:34 ` [PATCH v2 06/11] io: Add preadv support to QIOChannelFile Nikolay Borisov
` (5 subsequent siblings)
10 siblings, 1 reply; 23+ messages in thread
From: Nikolay Borisov @ 2022-10-10 13:34 UTC (permalink / raw)
To: dgilbert, berrange
Cc: qemu-devel, jfehlig, Claudio.Fontana, dfaggioli, Nikolay Borisov
Add a bunch of auxiliarry methods and a feature flag to work with
SEEKABLE channels. Currently the only channel considered seekable is
QIOChannelFile. Also add a bunch of helper functions to QEMUFile that
can make use of this channel feature. All of this is in prepration for
supporting 'fixed-ram' migration stream feature.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
include/io/channel.h | 1 +
include/migration/qemu-file-types.h | 2 +
io/channel-file.c | 5 +++
migration/qemu-file.c | 59 +++++++++++++++++++++++++++++
migration/qemu-file.h | 3 ++
5 files changed, 70 insertions(+)
diff --git a/include/io/channel.h b/include/io/channel.h
index c680ee748021..4fc37c78e68c 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -41,6 +41,7 @@ enum QIOChannelFeature {
QIO_CHANNEL_FEATURE_SHUTDOWN,
QIO_CHANNEL_FEATURE_LISTEN,
QIO_CHANNEL_FEATURE_WRITE_ZERO_COPY,
+ QIO_CHANNEL_FEATURE_SEEKABLE,
};
diff --git a/include/migration/qemu-file-types.h b/include/migration/qemu-file-types.h
index 2867e3da84ab..eb0325ee8687 100644
--- a/include/migration/qemu-file-types.h
+++ b/include/migration/qemu-file-types.h
@@ -50,6 +50,8 @@ unsigned int qemu_get_be16(QEMUFile *f);
unsigned int qemu_get_be32(QEMUFile *f);
uint64_t qemu_get_be64(QEMUFile *f);
+bool qemu_file_is_seekable(QEMUFile *f);
+
static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv)
{
qemu_put_be64(f, *pv);
diff --git a/io/channel-file.c b/io/channel-file.c
index da17d0a11ba7..d84a6737f2f7 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -35,6 +35,7 @@ qio_channel_file_new_fd(int fd)
ioc->fd = fd;
+ qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_SEEKABLE);
trace_qio_channel_file_new_fd(ioc, fd);
return ioc;
@@ -59,6 +60,10 @@ qio_channel_file_new_path(const char *path,
return NULL;
}
+ if (lseek(ioc->fd, 0, SEEK_CUR) != (off_t)-1) {
+ qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_SEEKABLE);
+ }
+
trace_qio_channel_file_new_path(ioc, path, flags, mode, ioc->fd);
return ioc;
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 4f400c2e5265..07ba1125e83f 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -30,6 +30,7 @@
#include "qemu-file.h"
#include "trace.h"
#include "qapi/error.h"
+#include "io/channel-file.h"
#define IO_BUF_SIZE 32768
#define MAX_IOV_SIZE MIN_CONST(IOV_MAX, 64)
@@ -260,6 +261,10 @@ static void qemu_iovec_release_ram(QEMUFile *f)
memset(f->may_free, 0, sizeof(f->may_free));
}
+bool qemu_file_is_seekable(QEMUFile *f)
+{
+ return qio_channel_has_feature(f->ioc, QIO_CHANNEL_FEATURE_SEEKABLE);
+}
/**
* Flushes QEMUFile buffer
@@ -538,6 +543,60 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, size_t size)
}
}
+void qemu_put_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen, off_t pos)
+{
+ Error *err = NULL;
+ struct iovec iov = { .iov_base = (char *)buf, .iov_len = buflen };
+
+ if (f->last_error) {
+ return;
+ }
+
+ qemu_fflush(f);
+
+ if (qio_channel_file_pwritev(f->ioc, &iov, 1, pos, &err) == (off_t)-1)
+ goto error;
+
+ return;
+
+ error:
+ qemu_file_set_error_obj(f, -EIO, err);
+ return;
+}
+
+void qemu_set_offset(QEMUFile *f, off_t off, int whence)
+{
+ Error *err = NULL;
+ off_t ret;
+
+ qemu_fflush(f);
+
+ if (!qemu_file_is_writable(f)) {
+ f->buf_index = 0;
+ f->buf_size = 0;
+ }
+
+ ret = qio_channel_io_seek(f->ioc, off, whence, &err);
+ if (ret == (off_t)-1) {
+ qemu_file_set_error_obj(f, -EIO, err);
+ }
+}
+
+off_t qemu_get_offset(QEMUFile *f)
+{
+ Error *err = NULL;
+ off_t ret;
+
+ qemu_fflush(f);
+
+ ret = qio_channel_io_seek(f->ioc, 0, SEEK_CUR, &err);
+ if (ret == (off_t)-1) {
+ qemu_file_set_error_obj(f, -EIO, err);
+ }
+ return ret;
+}
+
+
void qemu_put_byte(QEMUFile *f, int v)
{
if (f->last_error) {
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index fa13d04d787c..33cfc07b81d1 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -148,6 +148,9 @@ int qemu_file_shutdown(QEMUFile *f);
QEMUFile *qemu_file_get_return_path(QEMUFile *f);
void qemu_fflush(QEMUFile *f);
void qemu_file_set_blocking(QEMUFile *f, bool block);
+void qemu_set_offset(QEMUFile *f, off_t off, int whence);
+off_t qemu_get_offset(QEMUFile *f);
+void qemu_put_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen, off_t pos);
void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 05/11] io: Add support for seekable channels
2022-10-10 13:34 ` [PATCH v2 05/11] io: Add support for seekable channels Nikolay Borisov
@ 2022-10-18 10:14 ` Daniel P. Berrangé
2022-10-18 10:46 ` Nikolay Borisov
0 siblings, 1 reply; 23+ messages in thread
From: Daniel P. Berrangé @ 2022-10-18 10:14 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: dgilbert, qemu-devel, jfehlig, Claudio.Fontana, dfaggioli
On Mon, Oct 10, 2022 at 04:34:02PM +0300, Nikolay Borisov wrote:
> Add a bunch of auxiliarry methods and a feature flag to work with
> SEEKABLE channels. Currently the only channel considered seekable is
> QIOChannelFile. Also add a bunch of helper functions to QEMUFile that
> can make use of this channel feature. All of this is in prepration for
> supporting 'fixed-ram' migration stream feature.
QIOChannelBuffer/Null are also seekable.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> include/io/channel.h | 1 +
> include/migration/qemu-file-types.h | 2 +
> io/channel-file.c | 5 +++
> migration/qemu-file.c | 59 +++++++++++++++++++++++++++++
> migration/qemu-file.h | 3 ++
> 5 files changed, 70 insertions(+)
Can you separate the migration/ tree bits into a second patch
that follows the io/ bits.
>
> diff --git a/include/io/channel.h b/include/io/channel.h
> index c680ee748021..4fc37c78e68c 100644
> --- a/include/io/channel.h
> +++ b/include/io/channel.h
> @@ -41,6 +41,7 @@ enum QIOChannelFeature {
> QIO_CHANNEL_FEATURE_SHUTDOWN,
> QIO_CHANNEL_FEATURE_LISTEN,
> QIO_CHANNEL_FEATURE_WRITE_ZERO_COPY,
> + QIO_CHANNEL_FEATURE_SEEKABLE,
> };
>
>
> diff --git a/include/migration/qemu-file-types.h b/include/migration/qemu-file-types.h
> index 2867e3da84ab..eb0325ee8687 100644
> --- a/include/migration/qemu-file-types.h
> +++ b/include/migration/qemu-file-types.h
> @@ -50,6 +50,8 @@ unsigned int qemu_get_be16(QEMUFile *f);
> unsigned int qemu_get_be32(QEMUFile *f);
> uint64_t qemu_get_be64(QEMUFile *f);
>
> +bool qemu_file_is_seekable(QEMUFile *f);
> +
> static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv)
> {
> qemu_put_be64(f, *pv);
> diff --git a/io/channel-file.c b/io/channel-file.c
> index da17d0a11ba7..d84a6737f2f7 100644
> --- a/io/channel-file.c
> +++ b/io/channel-file.c
> @@ -35,6 +35,7 @@ qio_channel_file_new_fd(int fd)
>
> ioc->fd = fd;
>
> + qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_SEEKABLE);
> trace_qio_channel_file_new_fd(ioc, fd);
>
> return ioc;
> @@ -59,6 +60,10 @@ qio_channel_file_new_path(const char *path,
> return NULL;
> }
>
> + if (lseek(ioc->fd, 0, SEEK_CUR) != (off_t)-1) {
> + qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_SEEKABLE);
> + }
> +
> trace_qio_channel_file_new_path(ioc, path, flags, mode, ioc->fd);
Wondering why you do the lseek() sanitytest for only one of the
two constructors ? Shouldn't we do it for both ?
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 05/11] io: Add support for seekable channels
2022-10-18 10:14 ` Daniel P. Berrangé
@ 2022-10-18 10:46 ` Nikolay Borisov
2022-10-18 10:53 ` Daniel P. Berrangé
0 siblings, 1 reply; 23+ messages in thread
From: Nikolay Borisov @ 2022-10-18 10:46 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: dgilbert, qemu-devel, jfehlig, Claudio.Fontana, dfaggioli
On 18.10.22 г. 13:14 ч., Daniel P. Berrangé wrote:
> On Mon, Oct 10, 2022 at 04:34:02PM +0300, Nikolay Borisov wrote:
>> Add a bunch of auxiliarry methods and a feature flag to work with
>> SEEKABLE channels. Currently the only channel considered seekable is
>> QIOChannelFile. Also add a bunch of helper functions to QEMUFile that
>> can make use of this channel feature. All of this is in prepration for
>> supporting 'fixed-ram' migration stream feature.
>
> QIOChannelBuffer/Null are also seekable.
Right, however I think for seek we also want to rely on the feature of
filesystem that when you seek beyond EOF you won't actually allocate/use
up the space from (eof, CUR_SEEK), with ChannelBuffer we'd have to
actually allocate the space or add this support on top.
>
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>> include/io/channel.h | 1 +
>> include/migration/qemu-file-types.h | 2 +
>> io/channel-file.c | 5 +++
>> migration/qemu-file.c | 59 +++++++++++++++++++++++++++++
>> migration/qemu-file.h | 3 ++
>> 5 files changed, 70 insertions(+)
>
> Can you separate the migration/ tree bits into a second patch
> that follows the io/ bits.
>
>>
>> diff --git a/include/io/channel.h b/include/io/channel.h
>> index c680ee748021..4fc37c78e68c 100644
>> --- a/include/io/channel.h
>> +++ b/include/io/channel.h
>> @@ -41,6 +41,7 @@ enum QIOChannelFeature {
>> QIO_CHANNEL_FEATURE_SHUTDOWN,
>> QIO_CHANNEL_FEATURE_LISTEN,
>> QIO_CHANNEL_FEATURE_WRITE_ZERO_COPY,
>> + QIO_CHANNEL_FEATURE_SEEKABLE,
>> };
>>
>>
>> diff --git a/include/migration/qemu-file-types.h b/include/migration/qemu-file-types.h
>> index 2867e3da84ab..eb0325ee8687 100644
>> --- a/include/migration/qemu-file-types.h
>> +++ b/include/migration/qemu-file-types.h
>> @@ -50,6 +50,8 @@ unsigned int qemu_get_be16(QEMUFile *f);
>> unsigned int qemu_get_be32(QEMUFile *f);
>> uint64_t qemu_get_be64(QEMUFile *f);
>>
>> +bool qemu_file_is_seekable(QEMUFile *f);
>> +
>> static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv)
>> {
>> qemu_put_be64(f, *pv);
>> diff --git a/io/channel-file.c b/io/channel-file.c
>> index da17d0a11ba7..d84a6737f2f7 100644
>> --- a/io/channel-file.c
>> +++ b/io/channel-file.c
>> @@ -35,6 +35,7 @@ qio_channel_file_new_fd(int fd)
>>
>> ioc->fd = fd;
>>
>> + qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_SEEKABLE);
>> trace_qio_channel_file_new_fd(ioc, fd);
>>
>> return ioc;
>> @@ -59,6 +60,10 @@ qio_channel_file_new_path(const char *path,
>> return NULL;
>> }
>>
>> + if (lseek(ioc->fd, 0, SEEK_CUR) != (off_t)-1) {
>> + qio_channel_set_feature(QIO_CHANNEL(ioc), QIO_CHANNEL_FEATURE_SEEKABLE);
>> + }
>> +
>> trace_qio_channel_file_new_path(ioc, path, flags, mode, ioc->fd);
>
> Wondering why you do the lseek() sanitytest for only one of the
> two constructors ? Shouldn't we do it for both ?
Good point, AFAIR my train of thought was something along the lines of
"what if the path being passed actually lies on a remote fs, we
definitely need to test for this support", however now that I think
about it a bit more - nothing prevents for the passed in fd to also be
lying on a remote fs, so good point. Will fix this in next iteration.
>
>
>
>
> With regards,
> Daniel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 05/11] io: Add support for seekable channels
2022-10-18 10:46 ` Nikolay Borisov
@ 2022-10-18 10:53 ` Daniel P. Berrangé
0 siblings, 0 replies; 23+ messages in thread
From: Daniel P. Berrangé @ 2022-10-18 10:53 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: dgilbert, qemu-devel, jfehlig, Claudio.Fontana, dfaggioli
On Tue, Oct 18, 2022 at 01:46:45PM +0300, Nikolay Borisov wrote:
>
>
> On 18.10.22 г. 13:14 ч., Daniel P. Berrangé wrote:
> > On Mon, Oct 10, 2022 at 04:34:02PM +0300, Nikolay Borisov wrote:
> > > Add a bunch of auxiliarry methods and a feature flag to work with
> > > SEEKABLE channels. Currently the only channel considered seekable is
> > > QIOChannelFile. Also add a bunch of helper functions to QEMUFile that
> > > can make use of this channel feature. All of this is in prepration for
> > > supporting 'fixed-ram' migration stream feature.
> >
> > QIOChannelBuffer/Null are also seekable.
>
> Right, however I think for seek we also want to rely on the feature of
> filesystem that when you seek beyond EOF you won't actually allocate/use up
> the space from (eof, CUR_SEEK), with ChannelBuffer we'd have to actually
> allocate the space or add this support on top.
I'm fine with not implementing this for ChannelBuffer. Mostly making
the point that the APIs should be in QIOChannel base class, so that
someone could implement it in any subclass in future where it makes
sense.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 06/11] io: Add preadv support to QIOChannelFile
2022-10-10 13:33 [PATCH v2 00/11] Add support for fixed ram offsets during migration Nikolay Borisov
` (4 preceding siblings ...)
2022-10-10 13:34 ` [PATCH v2 05/11] io: Add support for seekable channels Nikolay Borisov
@ 2022-10-10 13:34 ` Nikolay Borisov
2022-10-10 13:34 ` [PATCH v2 07/11] migration: add qemu_get_buffer_at Nikolay Borisov
` (4 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Nikolay Borisov @ 2022-10-10 13:34 UTC (permalink / raw)
To: dgilbert, berrange
Cc: qemu-devel, jfehlig, Claudio.Fontana, dfaggioli, Nikolay Borisov
preadv is going to be needed when 'fixed-ram'-enabled stream are to be
restored. Simply add a wrapper around preadv that's specific to
QIOChannelFile.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
include/io/channel-file.h | 5 +++++
io/channel-file.c | 26 ++++++++++++++++++++++++++
2 files changed, 31 insertions(+)
diff --git a/include/io/channel-file.h b/include/io/channel-file.h
index 0a5d54f5e58e..2187f5affd23 100644
--- a/include/io/channel-file.h
+++ b/include/io/channel-file.h
@@ -89,6 +89,11 @@ qio_channel_file_new_path(const char *path,
mode_t mode,
Error **errp);
+ssize_t qio_channel_file_preadv(QIOChannel *ioc,
+ const struct iovec *iov,
+ size_t niov,
+ off_t offset,
+ Error **errp);
ssize_t qio_channel_file_pwritev(QIOChannel *ioc,
const struct iovec *iov,
size_t niov,
diff --git a/io/channel-file.c b/io/channel-file.c
index d84a6737f2f7..edca64ad63a7 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -141,6 +141,32 @@ static ssize_t qio_channel_file_writev(QIOChannel *ioc,
return ret;
}
+ssize_t qio_channel_file_preadv(QIOChannel *ioc,
+ const struct iovec *iov,
+ size_t niov,
+ off_t offset,
+ Error **errp)
+{
+ QIOChannelFile *fioc = QIO_CHANNEL_FILE(ioc);
+ ssize_t ret;
+
+ retry:
+ ret = preadv(fioc->fd, iov, niov, offset);
+ if (ret < 0) {
+ if (errno == EAGAIN) {
+ return QIO_CHANNEL_ERR_BLOCK;
+ }
+ if (errno == EINTR) {
+ goto retry;
+ }
+
+ error_setg_errno(errp, errno, "Unable to read from file");
+ return -1;
+ }
+
+ return ret;
+}
+
ssize_t qio_channel_file_pwritev(QIOChannel *ioc,
const struct iovec *iov,
size_t niov,
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 07/11] migration: add qemu_get_buffer_at
2022-10-10 13:33 [PATCH v2 00/11] Add support for fixed ram offsets during migration Nikolay Borisov
` (5 preceding siblings ...)
2022-10-10 13:34 ` [PATCH v2 06/11] io: Add preadv support to QIOChannelFile Nikolay Borisov
@ 2022-10-10 13:34 ` Nikolay Borisov
2022-10-10 13:34 ` [PATCH v2 08/11] migration/ram: Introduce 'fixed-ram' migration stream capability Nikolay Borisov
` (3 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Nikolay Borisov @ 2022-10-10 13:34 UTC (permalink / raw)
To: dgilbert, berrange
Cc: qemu-devel, jfehlig, Claudio.Fontana, dfaggioli, Nikolay Borisov
Restoring a 'fixed-ram' enabled migration stream would require reading
from specific offsets in the file so add a helper to QEMUFile that uses
the newly introduced qio_channel_file_preadv.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
migration/qemu-file.c | 23 +++++++++++++++++++++++
migration/qemu-file.h | 1 +
2 files changed, 24 insertions(+)
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 07ba1125e83f..73fffbdb9b7e 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -564,6 +564,29 @@ void qemu_put_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen, off_t po
return;
}
+
+size_t qemu_get_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen, off_t pos)
+{
+ Error *err = NULL;
+ struct iovec iov = { .iov_base = (char *)buf, .iov_len = buflen };
+ ssize_t ret;
+
+ if (f->last_error) {
+ return 0;
+ }
+
+ ret = qio_channel_file_preadv(f->ioc, &iov, 1, pos, &err);
+ if (ret == -1) {
+ goto error;
+ }
+
+ return (size_t)ret;
+
+ error:
+ qemu_file_set_error_obj(f, -EIO, err);
+ return 0;
+}
+
void qemu_set_offset(QEMUFile *f, off_t off, int whence)
{
Error *err = NULL;
diff --git a/migration/qemu-file.h b/migration/qemu-file.h
index 33cfc07b81d1..ab10c3ad7e42 100644
--- a/migration/qemu-file.h
+++ b/migration/qemu-file.h
@@ -151,6 +151,7 @@ void qemu_file_set_blocking(QEMUFile *f, bool block);
void qemu_set_offset(QEMUFile *f, off_t off, int whence);
off_t qemu_get_offset(QEMUFile *f);
void qemu_put_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen, off_t pos);
+size_t qemu_get_buffer_at(QEMUFile *f, const uint8_t *buf, size_t buflen, off_t pos);
void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 08/11] migration/ram: Introduce 'fixed-ram' migration stream capability
2022-10-10 13:33 [PATCH v2 00/11] Add support for fixed ram offsets during migration Nikolay Borisov
` (6 preceding siblings ...)
2022-10-10 13:34 ` [PATCH v2 07/11] migration: add qemu_get_buffer_at Nikolay Borisov
@ 2022-10-10 13:34 ` Nikolay Borisov
2022-10-10 13:34 ` [PATCH v2 09/11] migration: Refactor precopy ram loading code Nikolay Borisov
` (2 subsequent siblings)
10 siblings, 0 replies; 23+ messages in thread
From: Nikolay Borisov @ 2022-10-10 13:34 UTC (permalink / raw)
To: dgilbert, berrange
Cc: qemu-devel, jfehlig, Claudio.Fontana, dfaggioli, Nikolay Borisov
Implement 'fixed-ram' feature. The core of the feature is to ensure that
each ram page of the migration stream has a specific offset in the
resulting migration stream. The reason why we'd want such behavior are
two fold:
- When doing a 'fixed-ram' migration the resulting file will have a
bounded size, since pages which are dirtied multiple times will
always go to a fixed location in the file, rather than constantly
being added to a sequential stream. This eliminates cases where a vm
with, say, 1g of ram can result in a migration file that's 10s of
Gbs, provided that the workload constantly redirties memory.
- It paves the way to implement DIO-enabled save/restore of the
migration stream as the pages are ensured to be written at aligned
offsets.
The features requires changing the format. First, a bitmap is introduced
which tracks which pages have been written (i.e are dirtied) during
migration and subsequently it's being written in the resultin file,
again at a fixed location for every ramblock. Zero pages are ignored as
they'd be zero in the destination migration as well. With the changed
format data would look like the following:
|name len|name|used_len|pc*|bitmap_size|pages_offset|bitmap|pages|
* pc - refers to the page_size/mr->addr members, so newly added members
begin from "bitmap_size".
This layout is initialized during ram_save_setup so instead of having a
sequential stream of pages that follow the ramblock headers the dirty
pages for a ramblock follow its header. Since all pages have a fixed
location RAM_SAVE_FLAG_EOS is no longer generated on every migration
iteration but there is effectively a single RAM_SAVE_FLAG_EOS right at
the end.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
include/exec/ramblock.h | 7 +++
migration/migration.c | 51 +++++++++++++++++++++-
migration/migration.h | 1 +
migration/ram.c | 97 +++++++++++++++++++++++++++++++++--------
migration/savevm.c | 1 +
qapi/migration.json | 2 +-
6 files changed, 138 insertions(+), 21 deletions(-)
diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h
index 6cbedf9e0c9a..30216c1a41d3 100644
--- a/include/exec/ramblock.h
+++ b/include/exec/ramblock.h
@@ -43,6 +43,13 @@ struct RAMBlock {
size_t page_size;
/* dirty bitmap used during migration */
unsigned long *bmap;
+ /* shadow dirty bitmap used when migrating to a file */
+ unsigned long *shadow_bmap;
+ /* offset in the file pages belonging to this ramblock are saved, used
+ * only during migration to a file
+ */
+ off_t bitmap_offset;
+ uint64_t pages_offset;
/* bitmap of already received pages in postcopy */
unsigned long *receivedmap;
diff --git a/migration/migration.c b/migration/migration.c
index d0779bbaf862..1d08664da5db 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -165,7 +165,8 @@ INITIALIZE_MIGRATE_CAPS_SET(check_caps_background_snapshot,
MIGRATION_CAPABILITY_XBZRLE,
MIGRATION_CAPABILITY_X_COLO,
MIGRATION_CAPABILITY_VALIDATE_UUID,
- MIGRATION_CAPABILITY_ZERO_COPY_SEND);
+ MIGRATION_CAPABILITY_ZERO_COPY_SEND,
+ MIGRATION_CAPABILITY_FIXED_RAM);
/* When we add fault tolerance, we could have several
migrations at once. For now we don't need to add
@@ -1325,6 +1326,27 @@ static bool migrate_caps_check(bool *cap_list,
}
#endif
+ if (cap_list[MIGRATION_CAPABILITY_FIXED_RAM]) {
+ if (cap_list[MIGRATION_CAPABILITY_MULTIFD]) {
+ error_setg(errp, "Directly mapped memory incompatible with multifd");
+ return false;
+ }
+
+ if (cap_list[MIGRATION_CAPABILITY_XBZRLE]) {
+ error_setg(errp, "Directly mapped memory incompatible with xbzrle");
+ return false;
+ }
+
+ if (cap_list[MIGRATION_CAPABILITY_COMPRESS]) {
+ error_setg(errp, "Directly mapped memory incompatible with compression");
+ return false;
+ }
+
+ if (cap_list[MIGRATION_CAPABILITY_POSTCOPY_RAM]) {
+ error_setg(errp, "Directly mapped memory incompatible with postcopy ram");
+ return false;
+ }
+ }
/* incoming side only */
if (runstate_check(RUN_STATE_INMIGRATE) &&
@@ -2629,6 +2651,11 @@ MultiFDCompression migrate_multifd_compression(void)
return s->parameters.multifd_compression;
}
+int migrate_fixed_ram(void)
+{
+ return migrate_get_current()->enabled_capabilities[MIGRATION_CAPABILITY_FIXED_RAM];
+}
+
int migrate_multifd_zlib_level(void)
{
MigrationState *s;
@@ -4189,6 +4216,21 @@ static void *bg_migration_thread(void *opaque)
return NULL;
}
+static int
+migrate_check_fixed_ram(MigrationState *s, Error **errp)
+{
+ if (!s->enabled_capabilities[MIGRATION_CAPABILITY_FIXED_RAM])
+ return 0;
+
+ if (!qemu_file_is_seekable(s->to_dst_file)) {
+ error_setg(errp, "Directly mapped memory requires a seekable transport");
+ return -1;
+ }
+
+ return 0;
+}
+
+
void migrate_fd_connect(MigrationState *s, Error *error_in)
{
Error *local_err = NULL;
@@ -4264,6 +4306,12 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
return;
}
+ if (migrate_check_fixed_ram(s, &local_err) < 0) {
+ migrate_fd_cleanup(s);
+ migrate_fd_error(s, local_err);
+ return;
+ }
+
if (resume) {
/* Wakeup the main migration thread to do the recovery */
migrate_set_state(&s->state, MIGRATION_STATUS_POSTCOPY_PAUSED,
@@ -4397,6 +4445,7 @@ static Property migration_properties[] = {
DEFINE_PROP_STRING("tls-authz", MigrationState, parameters.tls_authz),
/* Migration capabilities */
+ DEFINE_PROP_MIG_CAP("x-fixed-ram", MIGRATION_CAPABILITY_FIXED_RAM),
DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
DEFINE_PROP_MIG_CAP("x-rdma-pin-all", MIGRATION_CAPABILITY_RDMA_PIN_ALL),
DEFINE_PROP_MIG_CAP("x-auto-converge", MIGRATION_CAPABILITY_AUTO_CONVERGE),
diff --git a/migration/migration.h b/migration/migration.h
index 96f27aba2210..9aab1b16f407 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -410,6 +410,7 @@ bool migrate_zero_blocks(void);
bool migrate_dirty_bitmaps(void);
bool migrate_ignore_shared(void);
bool migrate_validate_uuid(void);
+int migrate_fixed_ram(void);
bool migrate_auto_converge(void);
bool migrate_use_multifd(void);
diff --git a/migration/ram.c b/migration/ram.c
index dc1de9ddbc68..4f5ddaff356b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1261,9 +1261,14 @@ static int save_zero_page_to_file(RAMState *rs, QEMUFile *file,
int len = 0;
if (buffer_is_zero(p, TARGET_PAGE_SIZE)) {
- len += save_page_header(rs, file, block, offset | RAM_SAVE_FLAG_ZERO);
- qemu_put_byte(file, 0);
- len += 1;
+ if (migrate_fixed_ram()) {
+ /* for zero pages we don't need to do anything */
+ len = 1;
+ } else {
+ len += save_page_header(rs, file, block, offset | RAM_SAVE_FLAG_ZERO);
+ qemu_put_byte(file, 0);
+ len += 1;
+ }
ram_release_page(block->idstr, offset);
}
return len;
@@ -1342,15 +1347,22 @@ static bool control_save_page(RAMState *rs, RAMBlock *block, ram_addr_t offset,
static int save_normal_page(RAMState *rs, RAMBlock *block, ram_addr_t offset,
uint8_t *buf, bool async)
{
- ram_transferred_add(save_page_header(rs, rs->f, block,
- offset | RAM_SAVE_FLAG_PAGE));
- if (async) {
- qemu_put_buffer_async(rs->f, buf, TARGET_PAGE_SIZE,
- migrate_release_ram() &&
- migration_in_postcopy());
- } else {
- qemu_put_buffer(rs->f, buf, TARGET_PAGE_SIZE);
- }
+
+ if (migrate_fixed_ram()) {
+ qemu_put_buffer_at(rs->f, buf, TARGET_PAGE_SIZE,
+ block->pages_offset + offset);
+ set_bit(offset >> TARGET_PAGE_BITS, block->shadow_bmap);
+ } else {
+ ram_transferred_add(save_page_header(rs, rs->f, block,
+ offset | RAM_SAVE_FLAG_PAGE));
+ if (async) {
+ qemu_put_buffer_async(rs->f, buf, TARGET_PAGE_SIZE,
+ migrate_release_ram() &&
+ migration_in_postcopy());
+ } else {
+ qemu_put_buffer(rs->f, buf, TARGET_PAGE_SIZE);
+ }
+ }
ram_transferred_add(TARGET_PAGE_SIZE);
ram_counters.normal++;
return 1;
@@ -2683,6 +2695,8 @@ static void ram_save_cleanup(void *opaque)
block->clear_bmap = NULL;
g_free(block->bmap);
block->bmap = NULL;
+ g_free(block->shadow_bmap);
+ block->shadow_bmap = NULL;
}
xbzrle_cleanup();
@@ -3044,6 +3058,7 @@ static void ram_list_init_bitmaps(void)
*/
block->bmap = bitmap_new(pages);
bitmap_set(block->bmap, 0, pages);
+ block->shadow_bmap = bitmap_new(block->used_length >> TARGET_PAGE_BITS);
block->clear_bmap_shift = shift;
block->clear_bmap = bitmap_new(clear_bmap_size(pages, shift));
}
@@ -3226,12 +3241,34 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
qemu_put_buffer(f, (uint8_t *)block->idstr, strlen(block->idstr));
qemu_put_be64(f, block->used_length);
if (migrate_postcopy_ram() && block->page_size !=
- qemu_host_page_size) {
+ qemu_host_page_size) {
qemu_put_be64(f, block->page_size);
}
if (migrate_ignore_shared()) {
qemu_put_be64(f, block->mr->addr);
}
+
+ if (migrate_fixed_ram()) {
+ long num_pages = block->used_length >> TARGET_PAGE_BITS;
+ long bitmap_size = BITS_TO_LONGS(num_pages) * sizeof(unsigned long);
+
+
+ /* Needed for external programs (think analyze-migration.py) */
+ qemu_put_be32(f, bitmap_size);
+
+ /*
+ * Make pages offset aligned to TARGET_PAGE_SIZE to enable
+ * DIO in the future. Also add 8 to account for the page offset
+ * itself
+ */
+ block->bitmap_offset = qemu_get_offset(f) + 8;
+ block->pages_offset = ROUND_UP(block->bitmap_offset +
+ bitmap_size, TARGET_PAGE_SIZE);
+ qemu_put_be64(f, block->pages_offset);
+
+ /* Now prepare offset for next ramblock */
+ qemu_set_offset(f, block->pages_offset + block->used_length, SEEK_SET);
+ }
}
}
@@ -3249,6 +3286,17 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
return 0;
}
+static void ram_save_shadow_bmap(QEMUFile *f)
+{
+ RAMBlock *block;
+
+ RAMBLOCK_FOREACH_MIGRATABLE(block) {
+ long num_pages = block->used_length >> TARGET_PAGE_BITS;
+ long bitmap_size = BITS_TO_LONGS(num_pages) * sizeof(unsigned long);
+ qemu_put_buffer_at(f, (uint8_t *)block->shadow_bmap, bitmap_size, block->bitmap_offset);
+ }
+}
+
/**
* ram_save_iterate: iterative stage for migration
*
@@ -3358,9 +3406,15 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
return ret;
}
- qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
- qemu_fflush(f);
- ram_transferred_add(8);
+ /*
+ * For fixed ram we don't want to pollute the migration stream with
+ * EOS flags.
+ */
+ if (!migrate_fixed_ram()) {
+ qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
+ qemu_fflush(f);
+ ram_transferred_add(8);
+ }
ret = qemu_file_get_error(f);
}
@@ -3405,7 +3459,10 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
pages = ram_find_and_save_block(rs);
/* no more blocks to sent */
if (pages == 0) {
- break;
+ if (migrate_fixed_ram()) {
+ ram_save_shadow_bmap(f);
+ }
+ break;
}
if (pages < 0) {
ret = pages;
@@ -3428,8 +3485,10 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
return ret;
}
- qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
- qemu_fflush(f);
+ if (!migrate_fixed_ram()) {
+ qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
+ qemu_fflush(f);
+ }
return 0;
}
diff --git a/migration/savevm.c b/migration/savevm.c
index 174cdbefc29d..654f64dd08e2 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -240,6 +240,7 @@ static bool should_validate_capability(int capability)
/* Validate only new capabilities to keep compatibility. */
switch (capability) {
case MIGRATION_CAPABILITY_X_IGNORE_SHARED:
+ case MIGRATION_CAPABILITY_FIXED_RAM:
return true;
default:
return false;
diff --git a/qapi/migration.json b/qapi/migration.json
index 88ecf86ac876..6196617171e8 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -485,7 +485,7 @@
##
{ 'enum': 'MigrationCapability',
'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
- 'compress', 'events', 'postcopy-ram',
+ 'compress', 'events', 'postcopy-ram', 'fixed-ram',
{ 'name': 'x-colo', 'features': [ 'unstable' ] },
'release-ram',
'block', 'return-path', 'pause-before-switchover', 'multifd',
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 09/11] migration: Refactor precopy ram loading code
2022-10-10 13:33 [PATCH v2 00/11] Add support for fixed ram offsets during migration Nikolay Borisov
` (7 preceding siblings ...)
2022-10-10 13:34 ` [PATCH v2 08/11] migration/ram: Introduce 'fixed-ram' migration stream capability Nikolay Borisov
@ 2022-10-10 13:34 ` Nikolay Borisov
2022-10-10 13:34 ` [PATCH v2 10/11] migration: Add support for 'fixed-ram' migration restore Nikolay Borisov
2022-10-10 13:34 ` [PATCH v2 11/11] analyze-migration.py: add initial support for fixed ram streams Nikolay Borisov
10 siblings, 0 replies; 23+ messages in thread
From: Nikolay Borisov @ 2022-10-10 13:34 UTC (permalink / raw)
To: dgilbert, berrange
Cc: qemu-devel, jfehlig, Claudio.Fontana, dfaggioli, Nikolay Borisov
To facilitate easier implementaiton of the 'fixed-ram' migration restore
factor out the code responsible for parsing the ramblocks headers. This
also makes ram_load_precopy easier to comprehend.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
migration/ram.c | 142 +++++++++++++++++++++++++++---------------------
1 file changed, 80 insertions(+), 62 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 4f5ddaff356b..1dd68c221667 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4253,6 +4253,83 @@ void colo_flush_ram_cache(void)
trace_colo_flush_ram_cache_end();
}
+static int parse_ramblock(QEMUFile *f, RAMBlock *block, ram_addr_t length)
+{
+ int ret = 0;
+ /* ADVISE is earlier, it shows the source has the postcopy capability on */
+ bool postcopy_advised = postcopy_is_advised();
+
+ assert(block);
+
+ if (!qemu_ram_is_migratable(block)) {
+ error_report("block %s should not be migrated !", block->idstr);
+ ret = -EINVAL;
+ }
+
+ if (length != block->used_length) {
+ Error *local_err = NULL;
+
+ ret = qemu_ram_resize(block, length, &local_err);
+ if (local_err) {
+ error_report_err(local_err);
+ }
+ }
+ /* For postcopy we need to check hugepage sizes match */
+ if (postcopy_advised && migrate_postcopy_ram() &&
+ block->page_size != qemu_host_page_size) {
+ uint64_t remote_page_size = qemu_get_be64(f);
+ if (remote_page_size != block->page_size) {
+ error_report("Mismatched RAM page size %s "
+ "(local) %zd != %" PRId64, block->idstr,
+ block->page_size, remote_page_size);
+ ret = -EINVAL;
+ }
+ }
+ if (migrate_ignore_shared()) {
+ hwaddr addr = qemu_get_be64(f);
+ if (ramblock_is_ignored(block) &&
+ block->mr->addr != addr) {
+ error_report("Mismatched GPAs for block %s "
+ "%" PRId64 "!= %" PRId64, block->idstr,
+ (uint64_t)addr,
+ (uint64_t)block->mr->addr);
+ ret = -EINVAL;
+ }
+ }
+ ram_control_load_hook(f, RAM_CONTROL_BLOCK_REG, block->idstr);
+
+ return ret;
+}
+
+static int parse_ramblocks(QEMUFile *f, ram_addr_t total_ram_bytes)
+{
+ int ret = 0;
+
+ /* Synchronize RAM block list */
+ while (!ret && total_ram_bytes) {
+ char id[256];
+ RAMBlock *block;
+ ram_addr_t length;
+ int len = qemu_get_byte(f);
+
+ qemu_get_buffer(f, (uint8_t *)id, len);
+ id[len] = 0;
+ length = qemu_get_be64(f);
+
+ block = qemu_ram_block_by_name(id);
+ if (block) {
+ ret = parse_ramblock(f, block, length);
+ } else {
+ error_report("Unknown ramblock \"%s\", cannot accept "
+ "migration", id);
+ ret = -EINVAL;
+ }
+ total_ram_bytes -= length;
+ }
+
+ return ret;
+}
+
/**
* ram_load_precopy: load pages in precopy case
*
@@ -4267,14 +4344,13 @@ static int ram_load_precopy(QEMUFile *f)
{
MigrationIncomingState *mis = migration_incoming_get_current();
int flags = 0, ret = 0, invalid_flags = 0, len = 0, i = 0;
- /* ADVISE is earlier, it shows the source has the postcopy capability on */
- bool postcopy_advised = postcopy_is_advised();
+
if (!migrate_use_compression()) {
invalid_flags |= RAM_SAVE_FLAG_COMPRESS_PAGE;
}
while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
- ram_addr_t addr, total_ram_bytes;
+ ram_addr_t addr;
void *host = NULL, *host_bak = NULL;
uint8_t ch;
@@ -4345,65 +4421,7 @@ static int ram_load_precopy(QEMUFile *f)
switch (flags & ~RAM_SAVE_FLAG_CONTINUE) {
case RAM_SAVE_FLAG_MEM_SIZE:
- /* Synchronize RAM block list */
- total_ram_bytes = addr;
- while (!ret && total_ram_bytes) {
- RAMBlock *block;
- char id[256];
- ram_addr_t length;
-
- len = qemu_get_byte(f);
- qemu_get_buffer(f, (uint8_t *)id, len);
- id[len] = 0;
- length = qemu_get_be64(f);
-
- block = qemu_ram_block_by_name(id);
- if (block && !qemu_ram_is_migratable(block)) {
- error_report("block %s should not be migrated !", id);
- ret = -EINVAL;
- } else if (block) {
- if (length != block->used_length) {
- Error *local_err = NULL;
-
- ret = qemu_ram_resize(block, length,
- &local_err);
- if (local_err) {
- error_report_err(local_err);
- }
- }
- /* For postcopy we need to check hugepage sizes match */
- if (postcopy_advised && migrate_postcopy_ram() &&
- block->page_size != qemu_host_page_size) {
- uint64_t remote_page_size = qemu_get_be64(f);
- if (remote_page_size != block->page_size) {
- error_report("Mismatched RAM page size %s "
- "(local) %zd != %" PRId64,
- id, block->page_size,
- remote_page_size);
- ret = -EINVAL;
- }
- }
- if (migrate_ignore_shared()) {
- hwaddr addr = qemu_get_be64(f);
- if (ramblock_is_ignored(block) &&
- block->mr->addr != addr) {
- error_report("Mismatched GPAs for block %s "
- "%" PRId64 "!= %" PRId64,
- id, (uint64_t)addr,
- (uint64_t)block->mr->addr);
- ret = -EINVAL;
- }
- }
- ram_control_load_hook(f, RAM_CONTROL_BLOCK_REG,
- block->idstr);
- } else {
- error_report("Unknown ramblock \"%s\", cannot "
- "accept migration", id);
- ret = -EINVAL;
- }
-
- total_ram_bytes -= length;
- }
+ ret = parse_ramblocks(f, addr);
break;
case RAM_SAVE_FLAG_ZERO:
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 10/11] migration: Add support for 'fixed-ram' migration restore
2022-10-10 13:33 [PATCH v2 00/11] Add support for fixed ram offsets during migration Nikolay Borisov
` (8 preceding siblings ...)
2022-10-10 13:34 ` [PATCH v2 09/11] migration: Refactor precopy ram loading code Nikolay Borisov
@ 2022-10-10 13:34 ` Nikolay Borisov
2022-10-10 13:34 ` [PATCH v2 11/11] analyze-migration.py: add initial support for fixed ram streams Nikolay Borisov
10 siblings, 0 replies; 23+ messages in thread
From: Nikolay Borisov @ 2022-10-10 13:34 UTC (permalink / raw)
To: dgilbert, berrange
Cc: qemu-devel, jfehlig, Claudio.Fontana, dfaggioli, Nikolay Borisov
Add the necessary code to parse the format changes for 'fixed-ram'
capability. One of the more notable changes in behavior is that in the
'fixed-ram' case ram pages are restored in one go rather than constantly
looping through the migration stream. Also due to idiosyncrasies of the
format I have added the 'ram_migrated' since it was easier to simply
return directly from ->load_state rather than introducing more
conditionals around the code to prevent ->load_state being called
multiple times (from
qemu_loadvm_section_start_full/qemu_loadvm_section_part_end i.e. from
multiple QEMU_VM_SECTION_(PART|END) flags).
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
migration/migration.h | 2 +
migration/ram.c | 95 ++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 95 insertions(+), 2 deletions(-)
diff --git a/migration/migration.h b/migration/migration.h
index 9aab1b16f407..7a832d072415 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -96,6 +96,8 @@ struct MigrationIncomingState {
bool have_listen_thread;
QemuThread listen_thread;
+ bool ram_migrated;
+
/* For the kernel to send us notifications */
int userfault_fd;
/* To notify the fault_thread to wake, e.g., when need to quit */
diff --git a/migration/ram.c b/migration/ram.c
index 1dd68c221667..15441ed9f745 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4330,6 +4330,90 @@ static int parse_ramblocks(QEMUFile *f, ram_addr_t total_ram_bytes)
return ret;
}
+
+static int parse_ramblocks_fixed_ram(QEMUFile *f)
+{
+ int ret = 0;
+
+ while (!ret) {
+ char id[256];
+ RAMBlock *block;
+ ram_addr_t length;
+ unsigned long clear_bit_idx;
+ long num_pages, bitmap_size;
+ int len = qemu_get_byte(f);
+ g_autofree unsigned long *dirty_bitmap = NULL;
+
+ qemu_get_buffer(f, (uint8_t *)id, len);
+ id[len] = 0;
+ length = qemu_get_be64(f);
+
+ block = qemu_ram_block_by_name(id);
+ if (block) {
+ ret = parse_ramblock(f, block, length);
+ if (ret < 0) {
+ return ret;
+ }
+ } else {
+ error_report("Unknown ramblock \"%s\", cannot accept "
+ "migration", id);
+ ret = -EINVAL;
+ continue;
+ }
+
+ /* 1. read the bitmap size */
+ num_pages = length >> TARGET_PAGE_BITS;
+ bitmap_size = qemu_get_be32(f);
+
+ assert(bitmap_size == BITS_TO_LONGS(num_pages)*sizeof(unsigned long));
+
+ block->pages_offset = qemu_get_be64(f);
+
+ /* 2. read the actual bitmap */
+ dirty_bitmap = g_malloc0(bitmap_size);
+ if (qemu_get_buffer(f, (uint8_t *)dirty_bitmap, bitmap_size) != bitmap_size) {
+ error_report("Error parsing dirty bitmap");
+ return -EINVAL;
+ }
+
+#define BUFSIZE (4*1024*1024)
+ for (unsigned long set_bit_idx = find_first_bit(dirty_bitmap, num_pages);
+ set_bit_idx < num_pages;
+ set_bit_idx = find_next_bit(dirty_bitmap, num_pages, clear_bit_idx + 1)) {
+
+ clear_bit_idx = find_next_zero_bit(dirty_bitmap, num_pages, set_bit_idx + 1);
+ unsigned long len = TARGET_PAGE_SIZE * (clear_bit_idx - set_bit_idx);
+ ram_addr_t offset = set_bit_idx << TARGET_PAGE_BITS;
+
+ for (size_t written = 0, completed = 0; completed < len; offset += written) {
+ void *host = host_from_ram_block_offset(block, offset);
+ size_t read_len = MIN(len, BUFSIZE);
+
+ written = qemu_get_buffer_at(f, host, read_len,
+ block->pages_offset + offset);
+ completed += written;
+ }
+ }
+
+ /* Skip pages array */
+ qemu_set_offset(f, block->pages_offset + length, SEEK_SET);
+
+ /* Check if this is the last ramblock */
+ if (qemu_get_be64(f) == RAM_SAVE_FLAG_EOS) {
+ ret = 1;
+ } else {
+ /*
+ * If not, adjust the internal file index to account for the
+ * previous 64 bit read
+ */
+ qemu_file_skip(f, -8);
+ ret = 0;
+ }
+ }
+
+ return ret;
+}
+
/**
* ram_load_precopy: load pages in precopy case
*
@@ -4349,7 +4433,7 @@ static int ram_load_precopy(QEMUFile *f)
invalid_flags |= RAM_SAVE_FLAG_COMPRESS_PAGE;
}
- while (!ret && !(flags & RAM_SAVE_FLAG_EOS)) {
+ while (!ret && !(flags & RAM_SAVE_FLAG_EOS) && !mis->ram_migrated) {
ram_addr_t addr;
void *host = NULL, *host_bak = NULL;
uint8_t ch;
@@ -4421,7 +4505,14 @@ static int ram_load_precopy(QEMUFile *f)
switch (flags & ~RAM_SAVE_FLAG_CONTINUE) {
case RAM_SAVE_FLAG_MEM_SIZE:
- ret = parse_ramblocks(f, addr);
+ if (migrate_fixed_ram()) {
+ ret = parse_ramblocks_fixed_ram(f);
+ if (ret == 1) {
+ mis->ram_migrated = true;
+ }
+ } else {
+ ret = parse_ramblocks(f, addr);
+ }
break;
case RAM_SAVE_FLAG_ZERO:
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 11/11] analyze-migration.py: add initial support for fixed ram streams
2022-10-10 13:33 [PATCH v2 00/11] Add support for fixed ram offsets during migration Nikolay Borisov
` (9 preceding siblings ...)
2022-10-10 13:34 ` [PATCH v2 10/11] migration: Add support for 'fixed-ram' migration restore Nikolay Borisov
@ 2022-10-10 13:34 ` Nikolay Borisov
10 siblings, 0 replies; 23+ messages in thread
From: Nikolay Borisov @ 2022-10-10 13:34 UTC (permalink / raw)
To: dgilbert, berrange
Cc: qemu-devel, jfehlig, Claudio.Fontana, dfaggioli, Nikolay Borisov
This commit introduces the minimum code necessary to support parsing
migration strems with 'fixed-ram' capability set. The only thing really
missing is the implementation of write_or_dump_fixed_ram() which deals
with '-x'/'-m' options.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
scripts/analyze-migration.py | 49 +++++++++++++++++++++++++++++++++---
1 file changed, 46 insertions(+), 3 deletions(-)
diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
index b82a1b0c58c4..9785a640fbf8 100755
--- a/scripts/analyze-migration.py
+++ b/scripts/analyze-migration.py
@@ -23,6 +23,7 @@
import collections
import struct
import sys
+import math
def mkdir_p(path):
@@ -119,11 +120,16 @@ def __init__(self, file, version_id, ramargs, section_key):
self.file = file
self.section_key = section_key
self.TARGET_PAGE_SIZE = ramargs['page_size']
+ self.TARGET_PAGE_BITS = math.log2(self.TARGET_PAGE_SIZE)
self.dump_memory = ramargs['dump_memory']
self.write_memory = ramargs['write_memory']
+ self.fixed_ram = ramargs['fixed-ram']
self.sizeinfo = collections.OrderedDict()
+ self.bitmap_offset = collections.OrderedDict()
+ self.pages_offset = collections.OrderedDict()
self.data = collections.OrderedDict()
self.data['section sizes'] = self.sizeinfo
+ self.ram_read = False
self.name = ''
if self.write_memory:
self.files = { }
@@ -140,7 +146,13 @@ def __str__(self):
def getDict(self):
return self.data
+ def write_or_dump_fixed_ram(self):
+ pass
+
def read(self):
+ if self.fixed_ram and self.ram_read:
+ return
+
# Read all RAM sections
while True:
addr = self.file.read64()
@@ -167,7 +179,25 @@ def read(self):
f.truncate(0)
f.truncate(len)
self.files[self.name] = f
+
+ if self.fixed_ram:
+ bitmap_len = self.file.read32()
+ # skip the pages_offset which we don't need
+ offset = self.file.tell() + 8
+ self.bitmap_offset[self.name] = offset
+ offset = ((offset + bitmap_len + self.TARGET_PAGE_SIZE - 1) // self.TARGET_PAGE_SIZE) * self.TARGET_PAGE_SIZE
+ self.pages_offset[self.name] = offset
+ self.file.file.seek(offset + len)
+
flags &= ~self.RAM_SAVE_FLAG_MEM_SIZE
+ if self.fixed_ram:
+ self.ram_read = True
+ # now we should rewind to the ram page offset of the first
+ # ram section
+ if self.fixed_ram:
+ if self.write_memory or self.dump_memory:
+ self.write_or_dump_fixed_ram()
+ return
if flags & self.RAM_SAVE_FLAG_COMPRESS:
if flags & self.RAM_SAVE_FLAG_CONTINUE:
@@ -208,7 +238,7 @@ def read(self):
# End of RAM section
if flags & self.RAM_SAVE_FLAG_EOS:
- break
+ return
if flags != 0:
raise Exception("Unknown RAM flags: %x" % flags)
@@ -521,6 +551,7 @@ def read(self, desc_only = False, dump_memory = False, write_memory = False):
ramargs['page_size'] = self.vmsd_desc['page_size']
ramargs['dump_memory'] = dump_memory
ramargs['write_memory'] = write_memory
+ ramargs['fixed-ram'] = False
self.section_classes[('ram',0)][1] = ramargs
while True:
@@ -528,8 +559,20 @@ def read(self, desc_only = False, dump_memory = False, write_memory = False):
if section_type == self.QEMU_VM_EOF:
break
elif section_type == self.QEMU_VM_CONFIGURATION:
- section = ConfigurationSection(file)
- section.read()
+ config_desc = self.vmsd_desc.get('configuration')
+ if config_desc is not None:
+ config = VMSDSection(file, 1, config_desc, 'configuration')
+ config.read()
+ caps = config.data.get("configuration/capabilities")
+ if caps is not None:
+ caps = caps.data["capabilities"]
+ if type(caps) != list:
+ caps = [caps]
+ for i in caps:
+ # chomp out string length
+ cap = i.data[1:].decode("utf8")
+ if cap == "fixed-ram":
+ ramargs['fixed-ram'] = True
elif section_type == self.QEMU_VM_SECTION_START or section_type == self.QEMU_VM_SECTION_FULL:
section_id = file.read32()
name = file.readstr()
--
2.34.1
^ permalink raw reply related [flat|nested] 23+ messages in thread