Re: [Qemu-devel] [patch 2/2] target-i386: block migration and savevm if invariant tsc is exposed

[Eduardo Habkost]

On Fri, Apr 25, 2014 at 11:08:00PM +0200, Paolo Bonzini wrote:
> Il 25/04/2014 01:18, Eduardo Habkost ha scritto:
> >On Fri, Apr 25, 2014 at 12:57:48AM +0200, Paolo Bonzini wrote:
> >>Il 24/04/2014 22:57, Eduardo Habkost ha scritto:
> >>>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?
> >
> >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).
> 
> >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.

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".


> 
> >>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.
> 
> >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?

Management can use "-cpu host" and the "feature-words" QOM property to
check and report which features are really supported by the host, before
trying to use them. That means management will think "invtsc" is
completely unavailable if it is not present on "-cpu host" by default.

But there's also another point: management may _want_ invtsc to be
unset, because it may expect a feature to be available only if it can be
safely migrated.

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.

-- 
Eduardo