[libvirt] [resend v2 1/7] Resctrl: Add some utils functions

[Daniel P. Berrange]

On Tue, Feb 07, 2017 at 02:43:17PM +0800, Eli Qiao wrote:
> On Tuesday, 7 February 2017 at 12:11 AM, Daniel P. Berrange wrote:
> 
> > On Mon, Feb 06, 2017 at 10:23:36AM +0800, Eli Qiao wrote:
> > > This patch adds some utils struct and functions to expose resctrl
> > > information.
> > > 
> > > virResCtrlAvailable: If resctrl interface exist on host
> > > virResCtrlGet: get specify type resource contral information
> > > virResCtrlInit: initialize resctrl struct from the host's sys fs.
> > > ResCtrlAll[]: an array to maintain resource control information.
> > > 
> > > Signed-off-by: Eli Qiao <liyong.qiao at intel.com (mailto:liyong.qiao at intel.com)>
> > > ---
> > > +};
> > > +
> > > +
> > > +typedef struct _virResCacheBank virResCacheBank;
> > > +typedef virResCacheBank *virResCacheBankPtr;
> > > +struct _virResCacheBank {
> > > + unsigned int host_id;
> > > + unsigned long long cache_size;
> > > + unsigned long long cache_left;
> > > + unsigned long long cache_min;
> > > + virBitmapPtr cpu_mask;
> > > +};
> > > 
> > 
> > 
> > > +typedef struct _virResCtrl virResCtrl;
> > > +typedef virResCtrl *virResCtrlPtr;
> > > +struct _virResCtrl {
> > > + bool enabled;
> > > + const char *name;
> > > + int num_closid;
> > > + int cbm_len;
> > > + int min_cbm_bits;
> > > + const char* cache_level;
> > > + int num_banks;
> > > + virResCacheBankPtr cache_banks;
> > > +};
> > > 
> > 
> > 
> > Can any of these fields change at runtime, or are they all
> > immutable once populated.
> 
> Only cache_banks will be change at runtime, such as cache_left
> on each socket will be updated if libvirt allocates cache to domains.

Ok, so the API is returning a direct point to the virResCtrlPtr
struct and there's no locking here. So if cache_banks struct
contents changes while some caller is reading the virResCtrlPtr
struct we have race problems.

I'd suggest that best option is probably to take the mutable
cache_left field *out* of the struct. That lets the callers
treat the entire struct as immutable.

Then add a separate API to explicitly query the cache_left
value - only that api needs to do locking to return a consistent
point-in-time view of the cache_left value.


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