libvirt-6.5.0 breaks host passthrough migration

[Jiri Denemark]

On Sun, Jul 05, 2020 at 12:45:55 -0400, Mark Mielke wrote:
> Hi all:
> 
> With 6.4.0, live migration was working fine with Qemu 5.0. After trying out
> 6.5.0, migration broke with the following error:
> 
> libvirt.libvirtError: internal error: unable to execute QEMU command
> 'migrate': State blocked by non-migratable CPU device (invtsc flag)

Could you please describe the reproducer steps? For example, was the
domain you're trying to migrate already running when you upgrade libvirt
or is it freshly started by the new libvirt?

And it would be helpful to see the <cpu> element as shown by virsh
dumpxml before you try to start the domain as well as the QEMU command
line libvirt used to start the domain (in
/var/log/libvirt/qemu/$VM.log).

> I believe I traced the error back to this commit:
> 
> commit 201bd5db639c063862b0c1b1abfab9a9a7c92591
> Author: Jiri Denemark <jdenemar at redhat.com>
> Date:   Tue Jun 2 15:34:07 2020 +0200
> 
>     qemu: Fill default value in //cpu/@migratable attribute
> 
>     Before QEMU introduced migratable CPU property, "-cpu host" included all
>     features that could be enabled on the host, even those which would block
>     migration. In other words, the default was equivalent to migratable=off.
>     When the migratable property was introduced, the default changed to
>     migratable=on. Let's record the default in domain XML.
> 
>     Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
>     Reviewed-by: Michal Privoznik <mprivozn at redhat.com>
> 
> 
> Before this change, qemu was still being launched with "-cpu host", which
> for any somewhat modern version of qemu, defaults to migratable=on. The
> above comment acknowledges this, however, the implementation chooses the
> pessimistic and ancient (and no longer applicable!) value of migratable=off:
> 
> +    if (qemuCaps &&
> +        def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH &&
> +        !def->cpu->migratable) {
> +        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_MIGRATABLE))
> +            def->cpu->migratable = VIR_TRISTATE_SWITCH_ON;
> 
> *+        else if (ARCH_IS_X86(def->os.arch))+
>  def->cpu->migratable = VIR_TRISTATE_SWITCH_OFF;*
> +    }

The implementation seems to be doing exactly what the commit message
says. The migratable=off default should be used only when QEMU does not
support -cpu host,migratable=on|off, that is only when QEMU is very old.
Every non-ancient version of libvirt should have the
QEMU_CAPS_CPU_MIGRATABLE set and thus this code should choose
migrateble=on default.

> I think it is not a requirement for "migratable=XXX" to be explicit in
> libvirt. However, if there is some reason I am unaware of, and it is
> important for libvirt to know, then I think it is important for libvirt to
> find out the authoritative state rather than guessing.

Explicit defaults are always better for two reasons: they are visible to
users and they don't silently change.

Jirka