On Wed, Jun 21, 2017 at 02:07:15PM +0800, Eli Qiao wrote:
On Tuesday, 20 June 2017 at 8:39 PM, Martin Kletzander wrote:On Mon, Jun 12, 2017 at 05:48:41PM +0800, Eli Qiao wrote: > This patch adds 3 major private interface. > > virResctrlGetFreeCache: return free cache, default cache substract cache > allocated. > virResctrlSetCachetunes: set cache banks which defined in a domain. > virResctrlRemoveCachetunes: remove cache allocation group from the > host. > > There's some existed issue when do syntax-check as I reference the cache > tune and cachebank definition (from conf/domain_conf.h) in > util/virresctrl.h. >Yes, util/ cannot depend on conf/ in libvirt due to various reasons. All the data you want to use in util/ need to be defined there. If that corresponds to some XML, the parsers and formatters must be in conf/. In rare cases, there might be need for two data structures, one in util/ and one in conf/. I don't think that's needed in this case.I can move the virDomainCacheBank definition to util/virresctrl.h but what about the virCapsHostPtr, we need the host cache information and resctrl information it’s defined in src/conf/capabilities.h” Do we need to define another copy in virresctrl.h ?
You don't need to pass the whole structure of all the data. Can't the
qemu function just do something like:
virResctrlLock()
foreach cachetune:
region = virResctrlGetFreeRegion(size, type)
foreach cachetune.vcpu:
virResctrlSetRegion(vcpu.pid, region)
Or like with some other tuning, you can have a function that determines
the region when given vcpu and just call it for all vcpus. You can
save the regions in the status XML, so that not only users can see it,
but you can also reference them from that aforementioned function. Or
you could have saved pairs of id: region, but I think that's not needed.
That reminds me, unless referred to from somewhere, the cachetune
doesn't even need the id.
But basically, you don't need to pass the whole cachetune or any other
structure. The code is very messy, check your pointers and don't
compare references to NULLs. Read the diffs after yourself. I know it
works for you, but the code needs to be readable as well.
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
