On Wed, Jan 18, 2017 at 08:18:40PM +0100, David Hildenbrand wrote: > Am 18.01.2017 um 18:34 schrieb Eduardo Habkost: > > On Wed, Jan 18, 2017 at 12:09:28PM -0500, Jason J. Herne wrote: > >> On 01/18/2017 12:00 PM, Eduardo Habkost wrote: > >>> On Tue, Jan 17, 2017 at 10:22:10AM -0500, Jason J. Herne wrote: > >>>> On 01/16/2017 08:01 PM, Eduardo Habkost wrote: > >>>>> This is a follow-up to the series that implements > >>>>> query-cpu-model-expansion. Before including the test script, the > >>>>> series has some fixes to allow the results of > >>>>> query-cpu-model-expansion to be used in the QEMU command-line. > >>>>> > >>>>> The script probably will work on s390x too, but I couldn't test > >>>>> it yet. > >>>>> > >>>> > >>>> Eduardo, > >>>> > >>>> This test seems to mostly work on s390. The only issue I ran into is > >>>> querying host model using tcg only. s390 requires kvm to query the host > >>>> model. Perhaps we could just skip the tcg host test case on s390? > >>> > >>> We could still try to test "host", but add it to a greylist where > >>> errors returned by query-cpu-model-expansion can be non-fatal. > >>> query-cpu-model-expansion model="host" can also fail with KVM if > >>> the host doesn't support CPU models. > >>> > >> > >> David had the idea to just support -cpu host for tcg. We could do that. > >> In the meantime, I'm ok with your greylist idea too. This would allow the > >> script to work properly on s390 right from the start, and we can remove the > >> greylist when s390 supports specifying -cpu host for tcg. > > > > I believe we will still need to ignore query-cpu-model-expansion > > errors on some cases, otherwise the test script will fail on > > hosts where KVM doesn't support CPU models in KVM. > > That is indeed true. For "host" + KVM there would have to be an extra > check. non-fatal error sound right for this case (e.g. a warning) > > > > > But we probably don't need a hardcoded greylist, anyway: we could > > just make the error non-fatal in case the CPU model is not > > reported as migration-safe in query-cpu-definitions. > > > > But I was wondering: > > > > 1) Isn't "-cpu host" the default CPU model on s390x on KVM, > > even if the host doesn't support CPU models? > > Yes, it has an inbuilt compatibility mode when specified. If KVM support > for cpu models is missing, using "-cpu host" will work (as it worked on > QEMU versions without cpu model support), doing something like "-cpu > host,vx=on" will not work, as modifying features is not possible (as the > interface for query/config is missing). > > Using "host" for all query-cpu-model is forbiden, as we can't tell what > this model looks like. > > > > > 2) Is it really correct to return an error on > > "query-cpu-model-expansion model=host type=full" if the host > > doesn't support CPU models? > > > > Yes it is, because there is no way to tell which features there are. > Returning { name: "host", props: {} } would be misleading, as it > would mean that there are no features. Which is wrong. We just can't > tell. It is up to the caller to handle this. E.g. ignoring and > continuing, doing compatibility stuff or simply reporting an error. Well, if there's a risk { props: {} } will be interpreted as "all features disabled" instead of "we don't know the real value for any feature", then I agree that returning an error is better and safer. > > Also think about "query-cpu-model-expansion model=host type=static", > which will primarily be used by libvirt on s390x. There is no way to > expand this into a static cpu model. Faking anything will just hide errors. Yes, static expansion of host model must always return an error if it's not possible to expand. > > If "host" can't be expanded, QEMU has to be treated like there is no CPU > model support (as for older QEMU versions). OK. I will propose a patch updating the query-cpu-model-expansion documentation to be more explicit about it. > > > What if it just returned { name: "host", props: {} } > > on those cases, meaning that the CPU model is valid and > > usable, but QEMU is unable to provide extra information about > > it. > > > > > -- > > David -- Eduardo -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list