Re: [PATCH V1 2/4] migration: per-mode blockers

[Daniel P . Berrangé]

On Mon, Oct 23, 2023 at 10:37:59AM -0400, Steven Sistare wrote:
> On 10/23/2023 8:46 AM, Daniel P. Berrangé wrote:
> > On Thu, Oct 19, 2023 at 01:47:44PM -0700, Steve Sistare wrote:
> >> Extend the blocker interface so that a blocker can be registered for
> >> one or more migration modes.  The existing interfaces register a
> >> blocker for all modes, and the new interfaces take a varargs list
> >> of modes.
> >>
> >> Internally, maintain a separate blocker list per mode.  The same Error
> >> object may be added to multiple lists.  When a block is deleted, it is
> >> removed from every list, and the Error is freed.
> > 
> > I'm not sure that assocating blockers with migration modes is
> > the optimal way to model this.
> > 
> > IIUC, some of the migration blockers exist because the feature
> > relies on state that only exists on the current host.
> > 
> > This isn't a problem with CPR since the migration is within
> > the same host.  At the time though, these blockers should
> > likely be redundant for a normal migration that uses "localhost".
> > 
> > We can't express the distinction between localhost-migrate
> > and cross-host-migrate historically, but we should have done.
> > This new patch largely enables that I think which is good.
> > 
> > What I think this means is that we shouldn't tie blockers
> > to modes, but rather have different types of blockers as
> > a bit set
> > 
> >   enum MigrationBlockerType {
> >      MIGRATION_BLOCKER_LOCAL_HOST = (1 << 0),
> >      MIGRATION_BLOCKER_CROSS_HOST = (1 << 1),
> >   };
> > 
> >   #define MIGRATION_BLOCKER_ALL 0xff
> > 
> > 
> > Cpr would check for blockers with MIGRATION_BLOCKER_LOCAL_HOST
> > set only.
> > 
> > Normal migration within localhost only would similarly only
> > check MIGRATION_BLOCKER_LOCAL_HOST
> > 
> > Normal migration between arbitrary host would check for
> > MIGRATION_BLOCKER_LOCAL_HOST and MIGRATION_BLOCKER_CROSS_HOST
> 
> Or, we could define MIG_MODE_LOCAL to relax the blockers for local 
> migrations. 
> The user would add mode explicitly to the migrate command, or we could 
> implicitly switch from normal mode to local mode if we infer that the src
> and target are the same node. MIG_MODE_LOCAL and MIG_MODE_CPR_REBOOT would 
> relax the same blockers for now, but conceivably that could change.
> 
> When I add cpr-exec mode, it will have its own mode-specific blockers.  
> But, in your scheme, it could map to a new MigrationBlockerType.

Yes, there could be further types of blocker.

Do you have an example of something that would be a CPR blocker
only ?


I was thinking that migration blockers have a functional classification
which motivates their existance.

The different migration modes are describing particular usage
scenarios, and a given usage scenario will imply blockers for
one or more functional reasons.

> I do prefer mode as the way of specifying the type of migration.

Sure, I didn't mean to suggest "mode" as an input to 'migrate'
is bad. Just that I see migration blockers classification as
being distinct from the 'mode'. So a user could specify 'mode'
with 'migrate'  and that ends up mapping to certain types of
blocker.

> The question is whether we map mode directly to blockers, or map mode 
> plus other criteria such as locality to MigrationBlockerType(s) which 
> map to blockers.  
> 
> One consideration is, how will the user specify the equivalent of 
> only-migratable 
> on the command line?  I was thinking of adding -only-migratable 
> <mode1,mode2,...> 
> in a future patch, but if additional criteria maps to blockers, then we need 
> additional options or syntax.

I guess I could see wanting to use --only-migratable to express that I
want a guest that can do a localhost-migration, and CPR, but don't
care about cross-host-migration, which would point towards blocker
types being exposed.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|