Re: [Xen-devel] [PATCH 2/4] sysctl/libxl: Add interface for returning IO topology data

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 12/04/2014 06:55 AM, Dario Faggioli wrote:
> On Tue, 2014-12-02 at 16:34 -0500, Boris Ostrovsky wrote:
> > Make XEN_SYSCTL_topologyinfo more generic so that it can return both
> > CPU and IO topology (support for returning the latter is added in the
> > subsequent patch)
> Is it always the case that we need the full information (i.e., both CPU
> and IO topology), at all levels above Xen?
>
> I appreciate the performance implications (one hcall instead of two) in
> cases where we do, but I still think, as Andrew suggested, that having a
> new XEN_SYSCTL_iotopology (or something like that) and leaving
> *_topologyinfo alone would make a better low-level interface.
>
> All IMHO, of course.
I don't feel strongly either way so I can go that route (and it will 
make patch 4 completely unnecessary).
(I am not sure though I understood Andrew's reasoning for splitting into 
two sysctls.
> > To do so move (and rename) previously used cpu_to_core/cpu_to_socket/
> > cpu_to_node into struct xen_sysctl_cputopo and move it, together with
> > newly added struct xen_sysctl_iotopo, to xen_sysctl_topologyinfo.
> >
> > Add libxl_get_topology() to handle new interface and modify
> > libxl_get_cpu_topology() to be a wrapper around it.
> This is, I think, useful, and I'd go for it, even if we adding a new
> hypercall at the Xen and xc level.
>
> Of course, it would work the other way round: the new libxl function
> would wrap the existing libxl_get_cpu_topology() and a new libxl call
> (something like libxl_get_io_topology() ?).
>
> > Adjust xenpm and python's low-level C libraries for new interface.
> All in one patch? Wouldn't splitting it at least in two (hypervisor and
> tools parts) be better? If it were me, I'd probably split even more...
I could not split it because this patch changes sysctl interface (more 
specifically, xen_sysctl_topologyinfo_t/xc_topologyinfo_t) so anyone who 
uses this struct needed to be updated at the same time.
Of course, if I were to leave current interface intact and add another 
sysctl for IO topology then this will not be necessary
> > This change requires bumping XEN_SYSCTL_INTERFACE_VERSION
> Which would not be the case if we take the approach of adding a new,
> iotopology specific, hcall, would it?
I would think that any changes to a public interface, including adding a 
new function, require new version.
(And if we get a new version, even if we split topology into CPU and IO 
sysctls, I'd still like to put cpu_to_[core|socket||node] into a single 
structure).
Thanks.
-boris

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel