On Fri, Jul 16, 2021 at 12:44:49PM +0200, Cornelia Huck wrote: > On Fri, Jul 16 2021, Daniel P. Berrangé <berra...@redhat.com> wrote: > > > On Fri, Jul 16, 2021 at 11:10:04AM +0200, Cornelia Huck wrote: > >> On Thu, Jul 15 2021, Markus Armbruster <arm...@redhat.com> wrote: > >> > >> > Pierre Morel <pmo...@linux.ibm.com> writes: > >> > > >> >> On 7/15/21 8:16 AM, Markus Armbruster wrote: > >> >>> Pierre Morel <pmo...@linux.ibm.com> writes: > >> >>> > >> >>>> Drawers and Books are levels 4 and 3 of the S390 CPU > >> >>>> topology. > >> >>>> We allow the user to define these levels and we will > >> >>>> store the values inside the S390CcwMachineState. > >> >>> > >> >>> Double-checking: are these members specific to S390? > >> >> > >> >> Yes AFAIK > >> > > >> > Makes me wonder whether they should be conditional on TARGET_S390X. > >> > > >> > What happens when you specify them for another target? Silently > >> > ignored, or error? > >> > >> I'm wondering whether we should include them in the base machine state > >> and treat them as we treat 'dies' (i.e. the standard parser errors out > >> if they are set, and only the s390x parser supports them.) > > > > To repeat what i just wrote in my reply to patch 1, I think we ought to > > think about a different approach to handling the usage constraints, > > which doesn't require full re-implementation of the smp_parse method > > each time. There should be a way for each target to report topology > > constraints, such the the single smp_parse method can do the right > > thing, especially wrt error reporting for unsupported values. > > That would mean that all possible fields would need to go into common > code, right?
Yes, that is an implication of what i'm suggesting. > I'm wondering whether there are more architecture/cpu specific values > lurking in the corner, it would get unwieldy if we need to go beyond the > existing fields and drawers/books. Is the book/drawer thing architecture specific, or is it machine type / CPU specific. ie do /all/ the s390x machine types / CPUS QEMU support the book/drawer concept, or only a subset. If only a subset, then restricting it per target on QAPI doesn't fully solve the root problem, and we instead are better focusing on accurate runtime error reporting. 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 :|
