Re: [PATCH v2 00/25] migration: Postcopy Preemption

[Dr. David Alan Gilbert]

* Peter Xu (peterx@redhat.com) wrote:
> On Tue, Mar 01, 2022 at 10:27:10AM +0000, Daniel P. Berrangé wrote:
> > > I also didn't know whether there's other limitations of it.  For example,
> > > will a new socket pair be a problem for any VM environment (either a
> > > limitation from the management app, container, and so on)?  I think it's
> > > the same to multifd in that aspect, but I never explored.
> > 
> > If it needs extra sockets that is something apps will need to be aware
> > of unfortunately and explicitly opt-in to :-( Migration is often
> > tunnelled/proxied over other channels, so whatever does that needs to
> > be aware of possibility of seeing extra sockets.
> 
> Ah, then probably it can never be the default.  But for sure it could be
> nice that higher level can opt-in and make it a default at some point as
> long as it knows the network topology is safe to do so.
> 
> > 
> > > > > TODO List
> > > > > =========
> > > > > 
> > > > > TLS support
> > > > > -----------
> > > > > 
> > > > > I only noticed its missing very recently.  Since soft freeze is 
> > > > > coming, and
> > > > > obviously I'm still growing this series, so I tend to have the 
> > > > > existing
> > > > > material discussed. Let's see if it can still catch the train for 
> > > > > QEMU 7.0
> > > > > release (soft freeze on 2022-03-08)..
> > > > 
> > > > I don't like the idea of shipping something that is only half finished.
> > > > It means that when apps probe for the feature, they'll see preempt
> > > > capability present, but have no idea whether they're using a QEMU that
> > > > is broken when combined with TLS or not. We shouldn't merge something
> > > > just to meet the soft freeze deadline if we know key features are 
> > > > broken.
> > > 
> > > IMHO merging and declaring support are two problems.
> > > 
> > > To me, it's always fine to merge the code that implemented the fundation 
> > > of a
> > > feature.  The feature can be worked upon in the future.
> > > 
> > > Requiring a feature to be "complete" sometimes can cause burden to not 
> > > only
> > > the author of the series but also reviewers.  It's IMHO not necessary to
> > > bind these two ideas.
> > > 
> > > It's sometimes also hard to define "complete": take the TLS as example, no
> > > one probably even noticed that it won't work with TLS and I just noticed 
> > > it
> > > merely these two days..  We obviously can't merge partial patchset, but if
> > > the patchset is well isolated, then it's not a blocker for merging, imho.
> > > 
> > > Per my understanding, what you worried is when we declare it supported but
> > > later we never know when TLS will be ready for it.  One solution is I can
> > > rename the capability as x-, then after the TLS side ready I drop the x-
> > > prefix.  Then Libvirt or any mgmt software doesn't need to support this
> > > until we drop the x-, so there's no risk of compatibility.
> > > 
> > > Would that sound okay to you?
> > 
> > If it has an x- prefix then we can basically ignore it from a mgmt app
> > POV until it is actually finished.
> > 
> > > I can always step back and work on TLS first before it's merged, but again
> > > I don't think it's required.
> > 
> > Apps increasingly consider use of TLS to be a mandatory feature for
> > migration, so until that works, this preempt has to be considered
> > unsupported & unfinished IMHO. So either TLS should be ready when
> > it merges, or it should be clearly marked unsupported at the QAPI
> > level.
> 
> Yes, I fully agree with it, and for huge vm migrations I think TLS is in
> many cases mandatory.
> 
> I do plan to work on it right afterwards if this series land, but as the
> series grows I just noticed maybe we should start landing some codes that's
> already solid.  Landing the code as another benefit that I want to make
> sure the code merged at least won't affect the existing features.
> 
> So what I'm curious is why TLS is getting quite some attentions in the past
> few years but I didn't even see any selftests included in migration-test on
> tls.  That's something I wanted to look into, maybe even before adding the
> preempt+tls support. But maybe I just missed something, as I didn't use tls
> a lot in the past.

Hmm, I think it's worth getting TLS working before putting the full
series in, because it might impact the way you wire the channels up -
it's going to take some care; but lets see which parts we can/should
take.

Dave
> Thanks,
> 
> -- 
> Peter Xu
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK