It was in SaveState but now moved to MigrationState altogether, reverted
its meaning, then renamed to "send_configuration". Again, using
HW_COMPAT_2_3 for old PC/SPAPR machines, and accel_register_prop() for
xen_init().
Removing savevm_skip_configuration().
Signed-off-by: Peter Xu <peterx@redhat.com>
---
hw/i386/pc_piix.c | 1 -
hw/ppc/spapr.c | 1 -
hw/xen/xen-common.c | 2 +-
include/hw/compat.h | 4 ++++
include/migration/misc.h | 1 -
migration/migration.c | 2 ++
migration/migration.h | 3 +++
migration/savevm.c | 15 ++++-----------
8 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index b662618..537de7c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -326,7 +326,6 @@ static void pc_compat_2_3(MachineState *machine)
if (kvm_enabled()) {
pcms->smm = ON_OFF_AUTO_OFF;
}
- savevm_skip_configuration();
}
static void pc_compat_2_2(MachineState *machine)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index edbdbfd..29fac1b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3580,7 +3580,6 @@ static void spapr_machine_2_3_instance_options(MachineState *machine)
{
spapr_machine_2_4_instance_options(machine);
savevm_skip_section_footers();
- savevm_skip_configuration();
}
static void spapr_machine_2_3_class_options(MachineClass *mc)
diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
index 186808b..ac6a2b9 100644
--- a/hw/xen/xen-common.c
+++ b/hw/xen/xen-common.c
@@ -143,7 +143,7 @@ static int xen_init(MachineState *ms)
qemu_add_vm_change_state_handler(xen_change_state_handler, NULL);
accel_register_prop(accel, "migration", "store-global-state", "off");
- savevm_skip_configuration();
+ accel_register_prop(accel, "migration", "send-configuration", "off");
savevm_skip_section_footers();
return 0;
diff --git a/include/hw/compat.h b/include/hw/compat.h
index a506a74..1a3fd94 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -183,6 +183,10 @@
.value = "off",\
},{\
.driver = "migration",\
+ .property = "send-configuration",\
+ .value = "off",\
+ },{\
+ .driver = "migration",\
.property = "store-global-state",\
.value = "off",\
},
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 23e533f..35b41bc 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -42,7 +42,6 @@ int64_t self_announce_delay(int round)
void dump_vmstate_json_to_file(FILE *out_fp);
void savevm_skip_section_footers(void);
-void savevm_skip_configuration(void);
/* migration/migration.c */
void qemu_start_incoming_migration(const char *uri, Error **errp);
diff --git a/migration/migration.c b/migration/migration.c
index 9af5c76..f69fe28 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1985,6 +1985,8 @@ static Property migration_properties[] = {
DEFINE_PROP_BOOL("store-global-state", MigrationState,
store_global_state, true),
DEFINE_PROP_BOOL("only-migratable", MigrationState, only_migratable, false),
+ DEFINE_PROP_BOOL("send-configuration", MigrationState,
+ send_configuration, true),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/migration/migration.h b/migration/migration.h
index 34377dd..4d4ea0d 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -142,6 +142,9 @@ struct MigrationState
/* Whether the VM is only allowing for migratable devices */
bool only_migratable;
+
+ /* Whether we send QEMU_VM_CONFIGURATION during migration */
+ bool send_configuration;
};
void migrate_set_state(int *state, int old_state, int new_state);
diff --git a/migration/savevm.c b/migration/savevm.c
index d8ea522..f678a8a 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -287,7 +287,6 @@ typedef struct SaveStateEntry {
typedef struct SaveState {
QTAILQ_HEAD(, SaveStateEntry) handlers;
int global_section_id;
- bool skip_configuration;
uint32_t len;
const char *name;
uint32_t target_page_bits;
@@ -296,15 +295,8 @@ typedef struct SaveState {
static SaveState savevm_state = {
.handlers = QTAILQ_HEAD_INITIALIZER(savevm_state.handlers),
.global_section_id = 0,
- .skip_configuration = false,
};
-void savevm_skip_configuration(void)
-{
- savevm_state.skip_configuration = true;
-}
-
-
static void configuration_pre_save(void *opaque)
{
SaveState *state = opaque;
@@ -970,11 +962,11 @@ void qemu_savevm_state_header(QEMUFile *f)
qemu_put_be32(f, QEMU_VM_FILE_MAGIC);
qemu_put_be32(f, QEMU_VM_FILE_VERSION);
- if (!savevm_state.skip_configuration || enforce_config_section()) {
+ if (migrate_get_current()->send_configuration ||
+ enforce_config_section()) {
qemu_put_byte(f, QEMU_VM_CONFIGURATION);
vmstate_save_state(f, &vmstate_configuration, &savevm_state, 0);
}
-
}
void qemu_savevm_state_begin(QEMUFile *f)
@@ -1984,7 +1976,8 @@ int qemu_loadvm_state(QEMUFile *f)
return -ENOTSUP;
}
- if (!savevm_state.skip_configuration || enforce_config_section()) {
+ if (migrate_get_current()->send_configuration ||
+ enforce_config_section()) {
if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) {
error_report("Configuration section missing");
return -EINVAL;
--
2.7.4
Peter Xu <peterx@redhat.com> wrote:
> It was in SaveState but now moved to MigrationState altogether, reverted
> its meaning, then renamed to "send_configuration". Again, using
> HW_COMPAT_2_3 for old PC/SPAPR machines, and accel_register_prop() for
> xen_init().
>
> Removing savevm_skip_configuration().
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
> - if (!savevm_state.skip_configuration || enforce_config_section()) {
> + if (migrate_get_current()->send_configuration ||
> + enforce_config_section()) {
> qemu_put_byte(f, QEMU_VM_CONFIGURATION);
> vmstate_save_state(f, &vmstate_configuration, &savevm_state, 0);
> }
It is a different problem, but this patch makes enforce_config_section
optional, no? We can get the same behaviour with this new option,
right?
Looking at the code, it is not clear to me if it is easier to put
enforce_config_option on top of this patch, or let things as they are.
Opinions?
Thanks, Juan.
On Mon, Jun 19, 2017 at 05:56:29PM +0200, Juan Quintela wrote:
> Peter Xu <peterx@redhat.com> wrote:
> > It was in SaveState but now moved to MigrationState altogether, reverted
> > its meaning, then renamed to "send_configuration". Again, using
> > HW_COMPAT_2_3 for old PC/SPAPR machines, and accel_register_prop() for
> > xen_init().
> >
> > Removing savevm_skip_configuration().
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
Thanks!
>
>
> > - if (!savevm_state.skip_configuration || enforce_config_section()) {
> > + if (migrate_get_current()->send_configuration ||
> > + enforce_config_section()) {
> > qemu_put_byte(f, QEMU_VM_CONFIGURATION);
> > vmstate_save_state(f, &vmstate_configuration, &savevm_state, 0);
> > }
>
> It is a different problem, but this patch makes enforce_config_section
> optional, no? We can get the same behaviour with this new option,
> right?
>
> Looking at the code, it is not clear to me if it is easier to put
> enforce_config_option on top of this patch, or let things as they are.
>
> Opinions?
There can be a very tiny difference. Consider this cmdline:
qemu -M enforce-config-section=true -global migration.send-configuration=false
For current patch, enforce-config-section=true will make sure this
cmdline will send the config section no matter what we provided in
-global section.
If we remove the enforce_config_section() above and also use
MigrationState.send_configuration, the enforce-config-section=true
will be replaced by the latter send-configuration=false, then config
section will not be sent.
However, I'll vote for your suggestion since after all this cmdline is
not really making much sense (considering it's providing two conflict
confugrations). And anyone who starts to use "-global migration.*"
then he/she should know the possible side effect.
--
Peter Xu
Peter Xu <peterx@redhat.com> wrote: > On Mon, Jun 19, 2017 at 05:56:29PM +0200, Juan Quintela wrote: >> Peter Xu <peterx@redhat.com> wrote: >> > It was in SaveState but now moved to MigrationState altogether, reverted >> > its meaning, then renamed to "send_configuration". Again, using >> > HW_COMPAT_2_3 for old PC/SPAPR machines, and accel_register_prop() for >> > xen_init(). >> > >> > Removing savevm_skip_configuration(). >> > >> > Signed-off-by: Peter Xu <peterx@redhat.com> >> It is a different problem, but this patch makes enforce_config_section >> optional, no? We can get the same behaviour with this new option, >> right? >> >> Looking at the code, it is not clear to me if it is easier to put >> enforce_config_option on top of this patch, or let things as they are. >> >> Opinions? > > There can be a very tiny difference. Consider this cmdline: > > qemu -M enforce-config-section=true -global migration.send-configuration=false > > For current patch, enforce-config-section=true will make sure this > cmdline will send the config section no matter what we provided in > -global section. > > If we remove the enforce_config_section() above and also use > MigrationState.send_configuration, the enforce-config-section=true > will be replaced by the latter send-configuration=false, then config > section will not be sent. > > However, I'll vote for your suggestion since after all this cmdline is > not really making much sense (considering it's providing two conflict > confugrations). And anyone who starts to use "-global migration.*" > then he/she should know the possible side effect. Agreed. Thanks for taking this, Juan.
© 2016 - 2026 Red Hat, Inc.