expose invariant tsc flag for kvm guests

Il 22/04/2014 15:10, Marcelo Tosatti ha scritto:

target-i386/cpu.c void cpu_x86_cpuid(CPUX86State *env, uin · line 2363

Quoted 10 lines

target-i386/cpu.c +10 -0

+    case 0x80000007:
+        *eax = 0;
+        *ebx = 0;
+        *ecx = 0;
+
+        if (kvm_enabled()) {
+            *edx = env->features[FEAT_8000_0007_EDX];
+        } else {
+            *edx = 0;
+        }

I think TCG is able to eliminate unsupported features too before they get in env->features[]. Or are those patches not in yet? Eduardo?

Invariant TSC documentation mentions that "invariant TSC will run at a constant rate in all ACPI P-, C-. and T-states".

This is not the case if migration to a host with different TSC frequency is allowed, or if savevm is performed. So block migration/savevm.

Also do not expose invariant tsc flag by default.

Signed-off-by Marcelo Tosatti <[email protected]>

Index: qemu-invariant-tsc/target-i386/kvm.c
===================================================================

target-i386/kvm.c +4 -0

@@ -33,6 +33,8 @@
 #include "exec/ioport.h"
 #include <asm/hyperv.h>
 #include "hw/pci/pci.h"
+#include "migration/migration.h"
+#include "qapi/qmp/qerror.h"
 
 //#define DEBUG_KVM
 

@@ -447,6 +449,8 @@ static bool hyperv_enabled(X86CPU *cpu)
             cpu->hyperv_relaxed_timing);
 }
 
+Error *invtsc_mig_blocker;
+
 #define KVM_MAX_CPUID_ENTRIES  100
 
 int kvm_arch_init_vcpu(CPUState *cs)

@@ -702,6 +706,16 @@ int kvm_arch_init_vcpu(CPUState *cs)
                                   !!(c->ecx & CPUID_EXT_SMX);
     }

Invariant TSC documentation mentions that "invariant TSC will run at a constant rate in all ACPI P-, C-. and T-states".

This is not the case if migration to a host with different TSC frequency is allowed, or if savevm is performed. So block migration/savevm.

Also do not expose invariant tsc flag by default.

Signed-off-by Marcelo Tosatti <[email protected]>

Index: qemu-invariant-tsc/target-i386/kvm.c
===================================================================

target-i386/kvm.c +14 -0

@@ -33,6 +33,8 @@
 #include "exec/ioport.h"
 #include <asm/hyperv.h>
 #include "hw/pci/pci.h"
+#include "migration/migration.h"
+#include "qapi/qmp/qerror.h"
 
 //#define DEBUG_KVM
 

@@ -447,6 +449,8 @@ static bool hyperv_enabled(X86CPU *cpu)
             cpu->hyperv_relaxed_timing);
 }
 
+Error *invtsc_mig_blocker;
+
 #define KVM_MAX_CPUID_ENTRIES  100
 
 int kvm_arch_init_vcpu(CPUState *cs)

@@ -702,6 +706,16 @@ int kvm_arch_init_vcpu(CPUState *cs)
                                   !!(c->ecx & CPUID_EXT_SMX);
     }
 
+    c = cpuid_find_entry(&cpuid_data.cpuid, 0x80000007, 0);
+    if (c && (c->edx & 1<<8)) {
+        /* for migration */
+        invtsc_mig_blocker = NULL;
+        error_set(&invtsc_mig_blocker, QERR_MIGRATION_NOT_SUPPORTED, "cpu");
+        migrate_add_blocker(invtsc_mig_blocker);
+        /* for savevm */
+        vmstate_x86_cpu.unmigratable = 1;
+    }
+
     cpuid_data.cpuid.padding = 0;
     r = kvm_vcpu_ioctl(cs, KVM_SET_CPUID2, &cpuid_data);
     if (r) {

Index: qemu-invariant-tsc/target-i386/cpu-qom.h
===================================================================

target-i386/cpu-qom.h +1 -1

@@ -116,7 +116,7 @@ static inline X86CPU *x86_env_get_cpu(CP
 #define ENV_OFFSET offsetof(X86CPU, env)
 
 #ifndef CONFIG_USER_ONLY
-extern const struct VMStateDescription vmstate_x86_cpu;
+extern struct VMStateDescription vmstate_x86_cpu;
 #endif
 
 /**

Index: qemu-invariant-tsc/target-i386/machine.c
===================================================================

target-i386/machine.c +1 -1

@@ -613,7 +613,7 @@ static const VMStateDescription vmstate_
     }
 };
 
-const VMStateDescription vmstate_x86_cpu = {
+VMStateDescription vmstate_x86_cpu = {
     .name = "cpu",
     .version_id = 12,
     .minimum_version_id = 3,

Index: qemu-invariant-tsc/target-i386/cpu.c
===================================================================

target-i386/cpu.c +0 -0

         env->features[w] =
             kvm_arch_get_supported_cpuid(s, wi->cpuid_eax, wi->cpuid_ecx,
                                          wi->cpuid_reg);

Quoted 3 lines · Marcelo Tosatti

Invariant TSC documentation mentions that "invariant TSC will run at a constant rate in all ACPI P-, C-. and T-states".

Quoted 4 lines

This is not the case if migration to a host with different TSC frequency is allowed, or if savevm is performed. So block migration/savevm.

Also do not expose invariant tsc flag by default.

What do you mean "do not expose invtsc by default", exactly? It is already not exposed by default because the default CPU model is qemu64 and qemu64 doesn't have it enabled.

Quoted 4 lines · Eduardo Habkost

Quoted 3 lines · Marcelo Tosatti

Invariant TSC documentation mentions that "invariant TSC will run at a constant rate in all ACPI P-, C-. and T-states".

Quoted 8 lines

Quoted 4 lines

This is not the case if migration to a host with different TSC frequency is allowed, or if savevm is performed. So block migration/savevm.

Also do not expose invariant tsc flag by default.

What do you mean "do not expose invtsc by default", exactly? It is already not exposed by default because the default CPU model is qemu64 and qemu64 doesn't have it enabled.

Should be "do not expose invtsc by default with -cpu host".

Since it blocks migration, i considered it a special flag that should be set when user is aware that migration is going to be blocked.

Makes sense?

Not for "-cpu host". If somebody needs migration to work, they shouldn't be using "-cpu host" anyway (I don't know if you have seen the other comments in my message?).

Il 22/04/2014 21:14, Eduardo Habkost ha scritto:

Quoted 3 lines

Not for "-cpu host". If somebody needs migration to work, they shouldn't be using "-cpu host" anyway (I don't know if you have seen the other comments in my message?).

I'm not entirely sure. If you have hosts with exactly identical chipsets, "-cpu host" migration will in all likelihood work. Marcelo's approach is safer.

Quoted 8 lines

Il 22/04/2014 21:14, Eduardo Habkost ha scritto:

Quoted 3 lines

Not for "-cpu host". If somebody needs migration to work, they shouldn't be using "-cpu host" anyway (I don't know if you have seen the other comments in my message?).

I'm not entirely sure. If you have hosts with exactly identical chipsets, "-cpu host" migration will in all likelihood work. Marcelo's approach is safer.

If that didn't break other use cases, I would agree.

But "-cpu host" today covers two use cases: 1) enabling everything that can be enabled, even if it breaks migration; 2) enabling all stuff that can be safely enabled without breaking migration.

Now we can't do both at the same time[1].

(1) is important for management software;
(2) works only if you are lucky.

Why would it make sense to break (1) to try make (2) work?

[1] I would even argue that we never did both at the same time."-cpu host" depends on host hardware capabilities, host kernel capabilities, and host QEMU version (we never took care of keeping guest ABI with "-cpu host"). If migration did work, it was never supposed to.

Il 24/04/2014 22:57, Eduardo Habkost ha scritto:

Quoted 10 lines · Paolo Bonzini

Quoted 8 lines

Il 22/04/2014 21:14, Eduardo Habkost ha scritto:

Quoted 3 lines

Not for "-cpu host". If somebody needs migration to work, they shouldn't be using "-cpu host" anyway (I don't know if you have seen the other comments in my message?).

I'm not entirely sure. If you have hosts with exactly identical chipsets, "-cpu host" migration will in all likelihood work. Marcelo's approach is safer.

Quoted 5 lines

If that didn't break other use cases, I would agree.

But "-cpu host" today covers two use cases: 1) enabling everything that can be enabled, even if it breaks migration; 2) enabling all stuff that can be safely enabled without breaking migration.

What does it enable *now* that breaks migration?

Quoted 5 lines

Now we can't do both at the same time[1].

(1) is important for management software;
(2) works only if you are lucky.

Or if you plan ahead. With additional logic even invariant TSC in principle can be made to work across migration if the host clocks are synchronized well enough (PTP accuracy is in the 100-1000 TSC ticks range).

Quoted 7 lines

Why would it make sense to break (1) to try make (2) work?

[1] I would even argue that we never did both at the same time."-cpu host" depends on host hardware capabilities, host kernel capabilities, and host QEMU version (we never took care of keeping guest ABI with "-cpu host"). If migration did work, it was never supposed to.

I think this is where I disagree. Migration of the PMU is one thing that obviously was done with "-cpu host" in mind.

Quoted 7 lines

Il 24/04/2014 22:57, Eduardo Habkost ha scritto:

Quoted 5 lines

If that didn't break other use cases, I would agree.

But "-cpu host" today covers two use cases: 1) enabling everything that can be enabled, even if it breaks migration; 2) enabling all stuff that can be safely enabled without breaking migration.

Quoted 1 line

What does it enable *now* that breaks migration?

Every single feature it enables can break it. It breaks if you upgrade to a QEMU version with new feature words. It breaks if you upgrade to a kernel which supports new features.

A feature that doesn't let you upgrade the kernel isn't a feature I expect users to be relying upon.

Quoted 4 lines

Or if you plan ahead. With additional logic even invariant TSC in principle can be made to work across migration if the host clocks are synchronized well enough (PTP accuracy is in the 100-1000 TSC ticks range).

Yes, it is possible in the future. But we never planned for it, so "-cpu host" never supported migration.

We may try to make a reliable implementation of use case (2) some day, yes. But the choice I see right now is between trying not break a feature that was never declared to exist, or breaking an existing interface that is required to solve existing bugs between libvirt and QEMU.

Il 25/04/2014 01:18, Eduardo Habkost ha scritto:

Quoted 8 lines · Paolo Bonzini

Quoted 7 lines

Il 24/04/2014 22:57, Eduardo Habkost ha scritto:

Quoted 5 lines

If that didn't break other use cases, I would agree.

But "-cpu host" today covers two use cases: 1) enabling everything that can be enabled, even if it breaks migration; 2) enabling all stuff that can be safely enabled without breaking migration.

Quoted 5 lines

Quoted 1 line

What does it enable *now* that breaks migration?

Every single feature it enables can break it. It breaks if you upgrade to a QEMU version with new feature words. It breaks if you upgrade to a kernel which supports new features.

Yes, but it doesn't break it if you have the same processor, QEMU and kernel versions; this is not the case for invtsc (at least without improvements that aren't there yet).

Quoted 2 lines

A feature that doesn't let you upgrade the kernel isn't a feature I expect users to be relying upon.

It lets you upgrade the kernel as long as you do it for all machines. Whether this is acceptable depends on what you're using virt for.

It can be fine for "cattle" VMs that you can throw away at any time. Though people who buy into the cattle vs. pet distinction will tell you that you don't migrate cattle and this would make this discussion moot.

It can be fine for personal VMs (e.g. Windows VMs) that people use for development and will likely throw away at the end of each working day. Though for these VMs you also won't bother with migration, doing maintenance at night is easier (and if you need "-cpu host" you're probably doing other things such as GPU assignment that prevents migration).

So I couldn't indeed think of uses of "-cpu host" together with migration. But if you're sure there is none, block it in QEMU. There's no reason why this should be specific to libvirt.

Quoted 8 lines

Quoted 5 lines

Or if you plan ahead. With additional logic even invariant TSC in principle can be made to work across migration if the host clocks are synchronized well enough (PTP accuracy is in the 100-1000 TSC ticks range).

Yes, it is possible in the future. But we never planned for it, so "-cpu host" never supported migration.

If it wasn't blocked, it was supported. You can change this, but this implies documenting it in release notes, and perhaps warning for a couple of releases to give non-libvirt users a chance to tell us what they're doing.

Quoted 5 lines

We may try to make a reliable implementation of use case (2) some day, yes. But the choice I see right now is between trying not break a feature that was never declared to exist, or breaking an existing interface that is required to solve existing bugs between libvirt and QEMU.

I'm not sure I follow, what existing interface would be broken and how?

Quoted 3 lines

So I couldn't indeed think of uses of "-cpu host" together with migration. But if you're sure there is none, block it in QEMU. There's no reason why this should be specific to libvirt.

It's not that there are no useful use cases, it is that we simply can't guarantee it will work because (by design) "-cpu host" enables features QEMU doesn't know about (so it doesn't know if migration is safe or not). And that's the main use case for "-cpu host".

So, what about doing the following on QEMU 2.1:

* "-cpu host,migratable=yes":
* No invtsc
* Migration not blocked
* No feature flag unknown to QEMU will be enabled
* "-cpu host,migratable=no":
* invtsc enabled
* Unknown-to-QEMU features enabled
* Migration blocked
* "-cpu host":
* Same behavior as "-cpu host,migratable=yes"
* Release notes indicating that "-cpu host,migratable=no" is required if the user doesn't care about migration and wants new (unknown to QEMU) features to be available

I was going to propose making "migratable=no" the default after a few releases, as I expect the majority of existing users of "-cpu host" to not care about migration. But I am not sure, because there's less harm in _not_ getting new bleeding edge features enabled, and those users (and management software) can start using "migratable=no" if they really want the new unmigratable features.

Il 28/04/2014 17:46, Eduardo Habkost ha scritto:

Quoted 8 lines

Quoted 3 lines

So I couldn't indeed think of uses of "-cpu host" together with migration. But if you're sure there is none, block it in QEMU. There's no reason why this should be specific to libvirt.

It's not that there are no useful use cases, it is that we simply can't guarantee it will work because (by design) "-cpu host" enables features QEMU doesn't know about (so it doesn't know if migration is safe or not). And that's the main use case for "-cpu host".

True, but in practice QEMU and KVM support is added in parallel, and we already have full support for Broadwell (IIRC) and are starting to add Skylake features.

Quoted 16 lines

So, what about doing the following on QEMU 2.1:

* "-cpu host,migratable=yes":
* No invtsc
* Migration not blocked
* No feature flag unknown to QEMU will be enabled
* "-cpu host,migratable=no":
* invtsc enabled
* Unknown-to-QEMU features enabled
* Migration blocked
* "-cpu host":
* Same behavior as "-cpu host,migratable=yes"
* Release notes indicating that "-cpu host,migratable=no" is required if the user doesn't care about migration and wants new (unknown to QEMU) features to be available

Quoted 6 lines

I was going to propose making "migratable=no" the default after a few releases, as I expect the majority of existing users of "-cpu host" to not care about migration. But I am not sure, because there's less harm in _not_ getting new bleeding edge features enabled, and those users (and management software) can start using "migratable=no" if they really want the new unmigratable features.

Makes sense. Basically "-cpu host,migratable=yes" is close to libvirt's host-model and Alex Graf's proposed "-cpu best". Should we call it "-cpu best" and drop migratability of "-cpu host"?

Quoted 3 lines

Makes sense. Basically "-cpu host,migratable=yes" is close to libvirt's host-model and Alex Graf's proposed "-cpu best". Should we call it "-cpu best" and drop migratability of "-cpu host"?

"-cpu best" is different from the modes above. It means "use the best existing CPU model (from the pre-defined table) that can run on this host".

And it would have the same ambiguities we found on "-cpu host": if a CPU model in the table have invtsc enabled, should it be considered a candidate for "-cpu best", or not?

Il 28/04/2014 21:23, Eduardo Habkost ha scritto:

Quoted 7 lines

Quoted 3 lines

Makes sense. Basically "-cpu host,migratable=yes" is close to libvirt's host-model and Alex Graf's proposed "-cpu best". Should we call it "-cpu best" and drop migratability of "-cpu host"?

"-cpu best" is different from the modes above. It means "use the best existing CPU model (from the pre-defined table) that can run on this host".

Yes, it's not exactly the same. In practice the behavior should be close.

Quoted 3 lines

And it would have the same ambiguities we found on "-cpu host": if a CPU model in the table have invtsc enabled, should it be considered a candidate for "-cpu best", or not?

No CPU model in the table should have invtsc enabled. :)

Expose "Invariant TSC" flag, if KVM is enabled. From Intel documentation:

17.13.1 Invariant TSC The time stamp counter in newer processors may support an enhancement, referred to as invariant TSC. Processor’s support for invariant TSC is indicated by CPUID.80000007H:EDX[8]. The invariant TSC will run at a constant rate in all ACPI P-, C-. and T-states. This is the architectural behavior moving forward. On processors with invariant TSC support, the OS may use the TSC for wall clock timer services (instead of ACPI or HPET timers). TSC reads are much more efficient and do not incur the overhead associated with a ring transition or access to a platform resource.

Signed-off-by Marcelo Tosatti <[email protected]>

Invariant TSC documentation mentions that "invariant TSC will run at a constant rate in all ACPI P-, C-. and T-states".

This is not the case if migration to a host with different TSC frequency is allowed, or if savevm is performed. So block migration/savevm.

Also do not expose invariant tsc flag by default.

Signed-off-by Marcelo Tosatti <[email protected]>

Index: qemu-invariant-tsc/target-i386/kvm.c
===================================================================

target-i386/kvm.c +4 -0

@@ -33,6 +33,8 @@
 #include "exec/ioport.h"
 #include <asm/hyperv.h>
 #include "hw/pci/pci.h"
+#include "migration/migration.h"
+#include "qapi/qmp/qerror.h"
 
 //#define DEBUG_KVM
 

@@ -447,6 +449,8 @@ static bool hyperv_enabled(X86CPU *cpu)
             cpu->hyperv_relaxed_timing);
 }
 
+Error *invtsc_mig_blocker;
+
 #define KVM_MAX_CPUID_ENTRIES  100
 
 int kvm_arch_init_vcpu(CPUState *cs)

target-i386/kvm.c +5 -0

+        migrate_add_blocker(invtsc_mig_blocker);
+        /* for savevm */
+        vmstate_x86_cpu.unmigratable = 1;
+    }
+
     cpuid_data.cpuid.padding = 0;
     r = kvm_vcpu_ioctl(cs, KVM_SET_CPUID2, &cpuid_data);
     if (r) {

Index: qemu-invariant-tsc/target-i386/cpu-qom.h
===================================================================

target-i386/cpu-qom.h +1 -1

@@ -116,7 +116,7 @@ static inline X86CPU *x86_env_get_cpu(CP
 #define ENV_OFFSET offsetof(X86CPU, env)
 
 #ifndef CONFIG_USER_ONLY
-extern const struct VMStateDescription vmstate_x86_cpu;
+extern struct VMStateDescription vmstate_x86_cpu;
 #endif
 
 /**

Index: qemu-invariant-tsc/target-i386/machine.c
===================================================================

target-i386/machine.c +1 -1

@@ -613,7 +613,7 @@ static const VMStateDescription vmstate_
     }
 };
 
-const VMStateDescription vmstate_x86_cpu = {
+VMStateDescription vmstate_x86_cpu = {
     .name = "cpu",
     .version_id = 12,
     .minimum_version_id = 3,

Invariant TSC documentation mentions that "invariant TSC will run at a constant rate in all ACPI P-, C-. and T-states".

This is not the case if migration to a host with different TSC frequency is allowed, or if savevm is performed. So block migration/savevm.

Signed-off-by Marcelo Tosatti <[email protected]>

Quoted 9 lines · Marcelo Tosatti

Invariant TSC documentation mentions that "invariant TSC will run at a constant rate in all ACPI P-, C-. and T-states".

This is not the case if migration to a host with different TSC frequency is allowed, or if savevm is performed. So block migration/savevm.

Signed-off-by Marcelo Tosatti <[email protected]>

[...]

Quoted 4 lines

target-i386/kvm.c +0 -0

@@ -702,6 +706,16 @@ int kvm_arch_init_vcpu(CPUState *cs)
                                   !!(c->ecx & CPUID_EXT_SMX);
     }
 

target-i386/kvm.c int kvm_arch_init_vcpu(CPUState *cs) · line 716

Quoted 8 lines

target-i386/kvm.c +8 -0

+    c = cpuid_find_entry(&cpuid_data.cpuid, 0x80000007, 0);
+    if (c && (c->edx & 1<<8) && invtsc_mig_blocker == NULL) {
+        /* for migration */
+        error_set(&invtsc_mig_blocker,
+                  QERR_DEVICE_FEATURE_BLOCKS_MIGRATION, "invtsc", "cpu");
+        migrate_add_blocker(invtsc_mig_blocker);
+        /* for savevm */
+        vmstate_x86_cpu.unmigratable = 1;

Did you ensure this will always happen before vmstate_register() is called for vmstate_x86_cpu? I believe kvm_arch_init_vcpu() is called a long long time after device_set_realized() (which is where vmstate_register() is called for DeviceState objects).

Quoted 15 lines · Eduardo Habkost

Quoted 9 lines · Marcelo Tosatti

Invariant TSC documentation mentions that "invariant TSC will run at a constant rate in all ACPI P-, C-. and T-states".

This is not the case if migration to a host with different TSC frequency is allowed, or if savevm is performed. So block migration/savevm.

Signed-off-by Marcelo Tosatti <[email protected]>

[...]

Quoted 4 lines

target-i386/kvm.c +0 -0

@@ -702,6 +706,16 @@ int kvm_arch_init_vcpu(CPUState *cs)
                                   !!(c->ecx & CPUID_EXT_SMX);
     }
 

Quoted 13 lines

target-i386/kvm.c int kvm_arch_init_vcpu(CPUState *cs) · line 716

Quoted 8 lines

target-i386/kvm.c +8 -0

+    c = cpuid_find_entry(&cpuid_data.cpuid, 0x80000007, 0);
+    if (c && (c->edx & 1<<8) && invtsc_mig_blocker == NULL) {
+        /* for migration */
+        error_set(&invtsc_mig_blocker,
+                  QERR_DEVICE_FEATURE_BLOCKS_MIGRATION, "invtsc", "cpu");
+        migrate_add_blocker(invtsc_mig_blocker);
+        /* for savevm */
+        vmstate_x86_cpu.unmigratable = 1;

Did you ensure this will always happen before vmstate_register() is called for vmstate_x86_cpu? I believe kvm_arch_init_vcpu() is called a long long time after device_set_realized() (which is where vmstate_register() is called for DeviceState objects).

x86_cpu_realizefn

-> qemu_init_vcpu -> qemu_kvm_start_vcpu ->
qemu_kvm_cpu_thread_fn

-> kvm_init_vcpu -> kvm_arch_init_vcpu

(quoted diff) +2 -0

@@ -2573,6 +2598,8 @@ static void x86_cpu_realizefn(DeviceState *dev,
 Error **errp)
     CPUX86State *env = &cpu->env;
     Error *local_err = NULL;
 
+    printf("%s: dev->realized=%d\n", __func__, dev->realized);
+
     if (env->features[FEAT_7_0_EBX] && env->cpuid_level < 7) {
         env->cpuid_level = 7;

     }

QEMU 1.7.93 monitor - type 'help' for more information (qemu) x86_cpu_realizefn: dev->realized=0
x86_cpu_realizefn: dev->realized=0
audio: Could not init `oss' audio driver

Have you tested if your patch actually blocks savevm?

Check vmsd for unmigratable field, allowing migratibility status to be modified after vmstate_register.

Signed-off-by Marcelo Tosatti <[email protected]>

savevm.c +0 -0

index 22123be..61a25c0 100644

Check vmsd for unmigratable field, allowing migratibility status to be modified after vmstate_register.

Signed-off-by Marcelo Tosatti <[email protected]>

savevm.c +1 -4

index da8aa24..c578e42 100644
@@ -232,7 +232,6 @@ typedef struct SaveStateEntry {
     const VMStateDescription *vmsd;
     void *opaque;
     CompatEntry *compat;
-    int no_migrate;
     int is_ram;
 } SaveStateEntry;
 

@@ -292,7 +291,6 @@ int register_savevm_live(DeviceState *dev,
     se->ops = ops;
     se->opaque = opaque;
     se->vmsd = NULL;
-    se->no_migrate = 0;
     /* if this is a live_savem then set is_ram */
     if (ops->save_live_setup != NULL) {
         se->is_ram = 1;

@@ -383,7 +381,6 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
     se->opaque = opaque;
     se->vmsd = vmsd;
     se->alias_id = alias_id;
-    se->no_migrate = vmsd->unmigratable;
 
     if (dev) {
         char *id = qdev_get_dev_path(dev);

@@ -452,7 +449,7 @@ bool qemu_savevm_state_blocked(Error **errp)
     SaveStateEntry *se;
 
     QTAILQ_FOREACH(se, &savevm_handlers, entry) {
-        if (se->no_migrate) {
+        if (se->vmsd && se->vmsd->unmigratable) {
             error_setg(errp, "State blocked by non-migratable device '%s'",
                        se->idstr);
             return true;