[libvirt] [PATCH v3 1/9] Implement public API for virDomainGetIOThreadsInfo

[Daniel P. Berrange]

On Thu, Mar 05, 2015 at 05:21:41PM +0100, Ján Tomko wrote:
> On Thu, Mar 05, 2015 at 07:45:51AM -0500, John Ferlan wrote:
> > >> +    unsigned char *cpumap;             /* CPU map for thread */
> > >> +    int cpumaplen;                     /* cpumap size */
> > > 
> > >> +    size_t nresources;                 /* count of resources using IOThread */
> > >> +    char **resources;                  /* array of resources using IOThread */
> > > 
> > > "resources" is too vague.
> > 
> > Suggestion?  Is "devices" better?
> > 
> > Today it's the disk source path, but I remember reading something where
> > an IOThread could be potentially used for something else (perhaps
> > network, but I cannot find the reference quickly).
> > 
> 
> I just worry that putting the path here could be a problem sometime in
> the future if the attribute gets extended (I think some of the Block*
> APIs had that problem).
> 
> Perhaps Peter or Eric will voice their opinion.

The XML config already provides information about what device is
associated with what I/O thread. As such I don't think we should
include the 'resources' field in the struct here at all. It is just
duplicating information from the XML, in a format that is not at all
extensible, which is just asking for trouble as you point out.

These proposed APIs are all about reporting & updating the CPU pinning
of the I/O threads, and info about the list of resource names is not
relevant for that task, so again this just seems like extra pain for
no gain.

IOW, just drop the 'resources' and 'nresources' fields from the struct

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|