Hi Keir, I see the patch you've committed for this issue
(http://xenbits2.xensource.com/staging/xen-unstable.hg?rev/ae203b55e7c8), but
my concern goes a littler further.
What will this data be used for? Will higher-level tools expect to find
"stepping" and break if they don't? Will XenSource's management tools
(which I assume will make use of this data) function properly in this
case? I agree that hardware-specific data needs to be visible somehow to
higher-level tools, but it makes me nervous...
(I don't have any particular objection to your fix; just voicing my
concerns.)
On Fri, 2007-03-02 at 13:39 -0700, Alex Williamson wrote:
> Hi Ewan,
>
> There are a couple problems with this patch for non-x86 (and maybe
> even on x86), see below:
>
> On Tue, 2007-02-27 at 01:56 +0000, Xen staging patchbot-unstable wrote:
> > diff -r e7b2a282c9e7 -r 50e0616fd012 tools/python/xen/xend/XendNode.py
> > --- a/tools/python/xen/xend/XendNode.py Mon Feb 26 17:20:36 2007 +0000
> > +++ b/tools/python/xen/xend/XendNode.py Tue Feb 27 00:37:27 2007 +0000
> > @@ -81,7 +81,7 @@ class XendNode:
> > for cpu_uuid, cpu in saved_cpus.items():
> > self.cpus[cpu_uuid] = cpu
> >
> > - # verify we have enough cpus here
> > + cpuinfo = parse_proc_cpuinfo()
> > physinfo = self.physinfo_dict()
> > cpu_count = physinfo['nr_cpus']
> > cpu_features = physinfo['hw_caps']
> > @@ -91,12 +91,23 @@ class XendNode:
> > if cpu_count != len(self.cpus):
> > self.cpus = {}
> > for i in range(cpu_count):
> > - cpu_uuid = uuid.createString()
> > - cpu_info = {'uuid': cpu_uuid,
> > - 'host': self.uuid,
> > - 'number': i,
> > - 'features': cpu_features}
> > - self.cpus[cpu_uuid] = cpu_info
> > + u = uuid.createString()
> > + self.cpus[u] = {'uuid': u, 'number': i }
> > +
> > + for u in self.cpus.keys():
> > + log.error(self.cpus[u])
> > + number = self.cpus[u]['number']
> > + log.error(number)
> > + log.error(cpuinfo)
> > + self.cpus[u].update(
> > + { 'host' : self.uuid,
> > + 'features' : cpu_features,
> > + 'speed' : int(float(cpuinfo[number]['cpu MHz'])),
> > + 'vendor' : cpuinfo[number]['vendor_id'],
> > + 'modelname': cpuinfo[number]['model name'],
> > + 'stepping' : cpuinfo[number]['stepping'],
> > + 'flags' : cpuinfo[number]['flags'],
> > + })
>
> On ia64, dom0 doesn't automatically get vcpus for each physical cpu,
> so the first problem is that we're not going to have a /proc/cpuinfo
> entry for every cpu in self.cpus.keys. I think it's likely x86 could
> run into this problem too if a cpu was hotplugged or booted with the
> dom0_max_vcpus options.
>
> The second problem is that /proc/cpuinfo fields are very architecture
> specific. I'd suggest importing arch and having separate cases for x86,
> ia64, and powerpc. For ia64, think the most appropriate mapping would
> be:
>
> self.cpus[u].update(
> { 'host' : self.uuid,
> 'features' : cpu_features,
> 'speed' : int(float(cpuinfo[0]['cpu MHz'])),
> 'vendor' : cpuinfo[0]['vendor'],
> 'modelname': cpuinfo[0]['family'],
> 'stepping' : cpuinfo[0]['model'],
> 'flags' : cpuinfo[0]['features'],
> })
>
> Hollis or Jimi might be able to chime in with identifiers that would
> work on powerpc. Thanks,
>
> Alex
--
Hollis Blanchard
IBM Linux Technology Center
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home