[libvirt] [PATCH] conf: Fix backwards migration of pSeries guests

[Peter Krempa]

On Wed, Jul 19, 2017 at 16:41:54 +0200, Andrea Bolognani wrote:
> On Wed, 2017-07-19 at 15:33 +0200, Peter Krempa wrote:
> > > Recent commits made it so that pci-root controllers for
> > 
> > Did we release this?
> 
> Nope, it will be in the next release.
> 
> > > +        /* Don't format the model name for PHBs when migrating so that
> > > +         * guests that only use the default one can be migrated to older
> > > +         * libvirt version which don't know about PHBs at all */
> > > +        if (virDomainControllerIsPCIHostBridge(def) &&
> > 
> > This function has a confusing name, since it's explicitly checking for a
> > pSeries specific thing but the name does not really imply that.
> 
> Point taken about pSeries not being mentioned in the
> name, but PHBs are a pSeries-specific concept so it didn't
> really occur to me at the time.

So I thin that function should be renamed, because the "PCI Host Bridge"
name sounds really generic.

> 
> > > +            flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE) {
> > > +            formatModelName = false;
> > > +        }
> > 
> > Won't this suppress the formatting of the type even if the user
> > specified it explicitly? In that case we should not suppress it IMO.
> > It will cause problems once there's a different model available.
> 
> How would the code know whether the model was set manually
> by the user or automatically by libvirt?

You can record it in a boolean.

> In any case, that made me realize that not sending the model,
> even if automatically filled in, could cause issues in the
> future if a new model is added and becomes the default, as
> the guest ABI would not be preserved during migration then.

That is the main reason to fill all the values in right away. Since
there apparently was a period, where a default would be used, but not
recorded, it needs some trickery unfortunately.

In such case you basically standardize, that the now-filled-in default
model (which can't ever change) if it was not provided by the user is to
be dropped from the migratable XML.

Once you start assign a new default model, that then needs to be
explicitly sent over, so that the migration will not be successufl
unless the destination is able to use the new model.

This means basically, that a missing model name becomes assigned to a
particular value.

> Any help on how to deal with this? I'm clearly not very good
> at the whole migration thing :(
> 
> > If the term 'PCI host bridge' is a p-series specific thing, we should
> > make it more obvious in the code since it's too generic sounding without
> > knowledge of the context.
> 
> Maybe I can change it to virDomainControllerIsPSeriesPHB()?
> That would require passing in the virDomainDef though.

That looks way better. It's clear at first glance that it's not a
general thing. (At least for the lazy not reading the documentation for
the function)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170720/42dc4db9/attachment-0001.sig>