On Wed, Mar 13, 2024 at 09:58:47 +0100, Peter Krempa wrote:
> On Tue, Mar 12, 2024 at 18:06:44 +0100, Jiri Denemark wrote:
> > Features marked with added='yes' in CPU model definitions have to be
> > removed before migration, otherwise older libvirt would complain about
> > unknown CPU features. We only do this for features that were enabled for
> > a given CPU model even with older libvirt, which just ignored the
> > features. And only for features we added ourselves when updating CPU
> > definition during domain startup, that is we do not remove features
> > which were explicitly mentioned by a user.
> >
> > That said, this is not the safest thing we could do, but it's
> > effectively the same thing we did before the affected features were
> > added: we ignored them completely on both sides of migration.
>
> IIUC if the feature is part of the model it'll never be expanded into
> the definition and thus will never be checked during migration, right?
All features known to libvirt are checked during migration, even those
that are part of a model in our CPU map. If a feature is only part of a
model in QEMU and not libvirt, it will be explicitly added to the
definition once domain starts so that it can be checked during
migration. But, if such feature is unknown to libvirt, it will just be
ignored completely.
> Thus effectively the full checking could be done only for migration
> between new versions if the destination somehow signals that it's new
> enough for the source to not drop the features.
Theoretically yes, but I could not come up with a reasonable way to do
this. This algorithm to ignore features is quite generic and can be used
in the future for other features (but hopefully we won't have to do it).
I was thinking about sending a list of all supported CPU features in a
migration cookie, but that sounds like a little bit too much as we
currently support 332 features on x86.
> > @@ -6664,6 +6691,25 @@ qemuDomainMakeCPUMigratable(virArch arch,
> > return -1;
> > }
> >
> > + if (virCPUx86GetAddedFeatures(cpu->model, &data.added) < 0)
> > + return -1;
> > +
> > + /* Drop features marked as added in a cpu model, but only
> > + * when they are not mentioned in origCPU, i.e., when they were not
> > + * explicitly mentioned by the user.
> > + */
> > + if (data.added) {
> > + g_auto(GStrv) keep = NULL;
> > +
> > + if (origCPU) {
> > + keep = virCPUDefListFeatures(origCPU);
> > + data.keep = keep;
> > + }
> > +
> > + if (virCPUDefFilterFeatures(cpu, qemuDomainDropAddedCPUFeatures, &data) < 0)
> > + return -1;
>
> Mmm, O(n^2) ...
Well, there are at most 77 features marked as added, so it's at most
n*77, which makes it O(n) :-) And even n is limited to 332...
Jirka
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
