Eduardo Habkost <ehabk...@redhat.com> writes: > On Wed, Apr 14, 2021 at 03:51:37PM +0200, Vitaly Kuznetsov wrote: >> Eduardo Habkost <ehabk...@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 <vkuzn...@redhat.com> >> >> Acked-by: Dr. David Alan Gilbert <dgilb...@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.
All make sense to me, let me try to draft v3 based on your proposal. Thanks! -- Vitaly