[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] i386: Make 'hv-reenlightenment' require explicit 'tsc-fre
From: |
Eduardo Habkost |
Subject: |
Re: [PATCH v2] i386: Make 'hv-reenlightenment' require explicit 'tsc-frequency' setting |
Date: |
Wed, 14 Apr 2021 10:39:03 -0400 |
On Wed, Apr 14, 2021 at 03:51:37PM +0200, Vitaly Kuznetsov wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
>
> > My apologies, this was lost under the noise in my mail inbox.
> > (I promise I'm trying to improve)
> >
> > On Wed, Mar 31, 2021 at 01:39:48PM +0200, Vitaly Kuznetsov wrote:
> >> Commit 561dbb41b1d7 "i386: Make migration fail when Hyper-V reenlightenment
> >> was enabled but 'user_tsc_khz' is unset" forbade migrations with when guest
> >> has opted for reenlightenment notifications but 'tsc-frequency' wasn't set
> >> explicitly on the command line. This works but the migration fails late and
> >> this may come as an unpleasant surprise. To make things more explicit,
> >> require 'tsc-frequency=' on the command line when 'hv-reenlightenment' was
> >> enabled. Make the change affect 6.0+ machine types only to preserve
> >> previously-valid configurations.
> >>
> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> >> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >
> > Even if the 6.0 release gets delayed, I wouldn't be comfortable
> > including this in a -rc4.
> >
> > What if the user does not plan to live migrate the machine at
> > all? Why is this case different from the ~25
> > migrate_add_blocker() calls in QEMU, where we block migration but
> > still let the VM run?
>
> The question is when do we want to let the user know about the
> problem. By refusing to start with 'hv-reenlightenment' and without
> 'tsc-frequency' we make it sure he knows early.
>
> We can, indeed, replace this with migrate_add_blocker() call but the
> fact that the VM is not migratable may come as a (late) surprise (we
> can certainly print a warning but these may be hidden by upper layers).
>
> Also, v1 of this patch was implementing a slightly different approach
> failing the migration late if we can't set tsc frequency on the
> destination host. Explicit 'tsc-frequency=' was not required.
>
> Personally, I'm comfortable with any approach, please advise.
I agree with what you are trying to do, I just wonder why we
wouldn't do exactly the same for all other migrate_add_blocker()
calls too (whatever the solution we choose here).
I'd suggest the following:
- For people who use "-only-migratable", using
migrate_add_blocker() here already solves the problem.
- For people who don't use "-only-migratable", we could change
migrate_add_blocker() to print a warning by default (only on
machine types where live migration is supported).
- For people who don't need live migration and don't want to see
those warnings, we can introduce a "-no-migration" option.
--
Eduardo