[Libvir] Re: Virtual CPU functions
I've tried to illustrate an exemple of virDomainGetVcpus API with a 2 dimensional cpumaps array. If you are agree with this, I can modify the patch of Michel Ponceau in this sense before the end of this week. I've proposed the virDomainGetCpus API to be in accordance with the virDomainPinVcpu which is per vpcu and not per domain as virDomainGetVcpus API. The virDomainPinVcpu API is'nt symmetrical with virDomainGetVcpus and must be called N times to perform the CPU mapping of a domain which isn't very satisfying. Another complaint against the virDomainPinVcpu versus virDomainGetVcpus API is that the cpumap parameter hasn't the same meaning in these two APIs. This point requires to duplicate macro for bit manipulations on these maps. Philippe Berthault ______________________________________________________________________ virConnectPtr pConn; virDomainPtr pDomain; virNodeInfo nodeInfo; virDomainInfo domInfo; virVcpuInfoPtr pVcpuInfos; int nbPhysCpus; unsigned char *cpuMaps[]; /* 1st dimension = per virtual cpu, 2nd dimension = per physical cpu */ int oneCpuMapLen; int vcpu, cpu; #define CPU_USABLE(maps,vcpu,cpu) (maps[vcpu][((cpu) / 8)] & (1 >> ((cpu) % 8))) ... virNodeGetInfo(pConn, &nodeInfo); nbPhysCpus = nodeInfo.cpus; /* ? or ? */ nbPhysCpus = nodeInfo.nodes * nodeInfo.sockets * nodeInfo.cores * nodeInfo.threads; virDomainGetInfo(pDomain, &domInfo); pVcpuInfos = malloc(sizeof(virVcpuInfo)*domInfo.nrVirtCpu); oneCpuMapLen = (nbPhysCpus + 7) / 8; cpuMaps = calloc(domInfo.nrVirtCpu, oneCpuMapLen); virDomainGetVcpus(pDomain, pVcpuInfos, domInfo.nrVirtCpu, cpuMaps, oneCpuMapLen); for (vcpu = 0; vcpu < domInfo.nrVirtCpu; vcpu++) { for (cpu = 0; cpu < nbPhysCpus; cpu++) { int byteCpu = cpu / 8; int bitCpu = cpu % 8; int mask = 1 >> bitCpu; /* lowest CPU number is least significant bit as M.Ponceau said */ int cpuUsable = cpuMaps[vcpu][byteCpu] & mask; ... /* or */ int cpuUsable = CPU_USABLE(cpuMaps, vcpu, cpu); ... if (cpuUsable) { printf("The physical cpu #%d is usable by virtual cpu #%d of domain #%s\n", cpu, vcpu, virDomainGetName(pDomain)); } } } ______________________________________________________________________
On Tue, Aug 01, 2006 at 05:06:59PM +0200, Philippe Berthault wrote:
I've tried to illustrate an exemple of virDomainGetVcpus API with a 2 dimensional cpumaps array.
Hum, right, the best is probably to go down to some examples at this point.
If you are agree with this, I can modify the patch of Michel Ponceau in this sense before the end of this week.
That would be good, even though I can't garantee the proposed API won't change, that would allow more people to work on the code.
I've proposed the virDomainGetCpus API to be in accordance with the virDomainPinVcpu which is per vpcu and not per domain as virDomainGetVcpus API.
I wonder how people are most likely to use those APIs. Building scenarios like: - physical CPU is to be locked to serve only VCPU N in domain D - domain A and domain B should be served by disjoint physical CPUs sets - monitoring are the most common uses I would guess but I may be wrong. First would require: - virDomainPinVcpu I guess Second would require: - virDomainGetCpus and a number of calls to limit to sets :-\ The last one is likely to require getting full maps, and since it is likely to be called frequently the cheapest the better If people who expressed interest on the list about VCPU function could express their principal use case it may help getting the APIs right.
The virDomainPinVcpu API is'nt symmetrical with virDomainGetVcpus and must be called N times to perform the CPU mapping of a domain which isn't very satisfying. Another complaint against the virDomainPinVcpu versus virDomainGetVcpus API is that the cpumap parameter hasn't the same meaning in these two APIs. This point requires to duplicate macro for bit manipulations on these maps.
Yes those are good points. There is an orthogonality decision on the API, access could be done by VCPU or by CPU, if you provide one at a time kind of accesses and of the wrong kind you may hit API problems really fast.
Philippe Berthault ______________________________________________________________________
virConnectPtr pConn; virDomainPtr pDomain; virNodeInfo nodeInfo; virDomainInfo domInfo; virVcpuInfoPtr pVcpuInfos; int nbPhysCpus; unsigned char *cpuMaps[]; /* 1st dimension = per virtual cpu, 2nd dimension = per physical cpu */ int oneCpuMapLen; int vcpu, cpu;
#define CPU_USABLE(maps,vcpu,cpu) (maps[vcpu][((cpu) / 8)] & (1 >> ((cpu) % 8)))
Hum, I don't think the array code is right here, it is basically a one dimention array, so maps[][] won't do what you expect.
... virNodeGetInfo(pConn, &nodeInfo);
nbPhysCpus = nodeInfo.cpus; /* ? or ? */ nbPhysCpus = nodeInfo.nodes * nodeInfo.sockets * nodeInfo.cores * nodeInfo.threads;
virDomainGetInfo(pDomain, &domInfo); pVcpuInfos = malloc(sizeof(virVcpuInfo)*domInfo.nrVirtCpu);
oneCpuMapLen = (nbPhysCpus + 7) / 8; cpuMaps = calloc(domInfo.nrVirtCpu, oneCpuMapLen);
virDomainGetVcpus(pDomain, pVcpuInfos, domInfo.nrVirtCpu, cpuMaps, oneCpuMapLen); for (vcpu = 0; vcpu < domInfo.nrVirtCpu; vcpu++) { for (cpu = 0; cpu < nbPhysCpus; cpu++) { int byteCpu = cpu / 8; int bitCpu = cpu % 8; int mask = 1 >> bitCpu; /* lowest CPU number is least significant bit as M.Ponceau said */
int cpuUsable = cpuMaps[vcpu][byteCpu] & mask; ... /* or */ int cpuUsable = CPU_USABLE(cpuMaps, vcpu, cpu); ...
both are wrong IMHO :-) one need to compute the index based on domInfo.nrVirtCpu
if (cpuUsable) { printf("The physical cpu #%d is usable by virtual cpu #%d of domain #%s\n", cpu, vcpu, virDomainGetName(pDomain)); } } }
I'm tempted to say: - let's collect use case - post the code you have - distinguish the problem of getting low level access in the library right and getting the API right those are separate thanks, Daniel -- Daniel Veillard | Red Hat http://redhat.com/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Daniel Veillard a écrit :
On Tue, Aug 01, 2006 at 05:06:59PM +0200, Philippe Berthault wrote:
I've tried to illustrate an exemple of virDomainGetVcpus API with a 2 dimensional cpumaps array.
Hum, right, the best is probably to go down to some examples at this point.
If you are agree with this, I can modify the patch of Michel Ponceau in this sense before the end of this week.
That would be good, even though I can't garantee the proposed API won't change, that would allow more people to work on the code.
I've proposed the virDomainGetCpus API to be in accordance with the virDomainPinVcpu which is per vpcu and not per domain as virDomainGetVcpus API.
I wonder how people are most likely to use those APIs. Building scenarios like: - physical CPU is to be locked to serve only VCPU N in domain D - domain A and domain B should be served by disjoint physical CPUs sets - monitoring are the most common uses I would guess but I may be wrong. First would require: - virDomainPinVcpu I guess Second would require: - virDomainGetCpus and a number of calls to limit to sets :-\ The last one is likely to require getting full maps, and since it is likely to be called frequently the cheapest the better
If people who expressed interest on the list about VCPU function could express their principal use case it may help getting the APIs right.
About third point (monitoring): I think the virDomainGetVcpus API isn't adequate for this purpose. It would be better to have an API (to be defined) which give the state of all physical CPUs because it's the hardware resources we need to monitor, not the virtual ones. The virDomainGetVcpus API permits to obtain the relation vcpu->physical_cpu(s) but for monitoring usage, it's not interesting. It would be better to have an API which give the reverse relation : physical_cpu->vpcu(s) independently of domains and give the physical CPU usage. With the virDomainGetVcpus API, it's impossible to determine if a physical cpu is underused or overused and it's this information we need to know for monitoring and for load-balancing.
The virDomainPinVcpu API is'nt symmetrical with virDomainGetVcpus and must be called N times to perform the CPU mapping of a domain which isn't very satisfying. Another complaint against the virDomainPinVcpu versus virDomainGetVcpus API is that the cpumap parameter hasn't the same meaning in these two APIs. This point requires to duplicate macro for bit manipulations on these maps.
Yes those are good points. There is an orthogonality decision on the API, access could be done by VCPU or by CPU, if you provide one at a time kind of accesses and of the wrong kind you may hit API problems really fast.
Philippe Berthault ______________________________________________________________________
virConnectPtr pConn; virDomainPtr pDomain; virNodeInfo nodeInfo; virDomainInfo domInfo; virVcpuInfoPtr pVcpuInfos; int nbPhysCpus; unsigned char *cpuMaps[]; /* 1st dimension = per virtual cpu, 2nd dimension = per physical cpu */ int oneCpuMapLen; int vcpu, cpu;
#define CPU_USABLE(maps,vcpu,cpu) (maps[vcpu][((cpu) / 8)] & (1 >> ((cpu) % 8)))
Hum, I don't think the array code is right here, it is basically a one dimention array, so maps[][] won't do what you expect.
... virNodeGetInfo(pConn, &nodeInfo);
nbPhysCpus = nodeInfo.cpus; /* ? or ? */ nbPhysCpus = nodeInfo.nodes * nodeInfo.sockets * nodeInfo.cores * nodeInfo.threads;
virDomainGetInfo(pDomain, &domInfo); pVcpuInfos = malloc(sizeof(virVcpuInfo)*domInfo.nrVirtCpu);
oneCpuMapLen = (nbPhysCpus + 7) / 8; cpuMaps = calloc(domInfo.nrVirtCpu, oneCpuMapLen);
virDomainGetVcpus(pDomain, pVcpuInfos, domInfo.nrVirtCpu, cpuMaps, oneCpuMapLen); for (vcpu = 0; vcpu < domInfo.nrVirtCpu; vcpu++) { for (cpu = 0; cpu < nbPhysCpus; cpu++) { int byteCpu = cpu / 8; int bitCpu = cpu % 8; int mask = 1 >> bitCpu; /* lowest CPU number is least significant bit as M.Ponceau said */
int cpuUsable = cpuMaps[vcpu][byteCpu] & mask; ... /* or */ int cpuUsable = CPU_USABLE(cpuMaps, vcpu, cpu); ...
both are wrong IMHO :-) one need to compute the index based on domInfo.nrVirtCpu
I don't understand why you think both are wrong. The first 'for' loop index is based on domInfo.nrVirtCpu.
if (cpuUsable) { printf("The physical cpu #%d is usable by virtual cpu #%d of domain #%s\n", cpu, vcpu, virDomainGetName(pDomain)); } } }
I'm tempted to say: - let's collect use case - post the code you have - distinguish the problem of getting low level access in the library right and getting the API right those are separate
thanks,
Daniel
On Tue, Aug 01, 2006 at 07:05:49PM +0200, Philippe Berthault wrote:
unsigned char *cpuMaps[]; /* 1st dimension = per virtual cpu, 2nd dimension = per physical cpu */ ...
virDomainGetVcpus(pDomain, pVcpuInfos, domInfo.nrVirtCpu, cpuMaps, oneCpuMapLen); for (vcpu = 0; vcpu < domInfo.nrVirtCpu; vcpu++) { for (cpu = 0; cpu < nbPhysCpus; cpu++) { int byteCpu = cpu / 8; int bitCpu = cpu % 8; int mask = 1 >> bitCpu; /* lowest CPU number is least significant bit as M.Ponceau said */
int cpuUsable = cpuMaps[vcpu][byteCpu] & mask; ... /* or */ int cpuUsable = CPU_USABLE(cpuMaps, vcpu, cpu); ...
both are wrong IMHO :-) one need to compute the index based on domInfo.nrVirtCpu
I don't understand why you think both are wrong. The first 'for' loop index is based on domInfo.nrVirtCpu.
Just a C implem nitpick, but that can lead to interesting debugging sessions (I think I got beaten by that a couple of time). Nowhere the compiler can guess that you're accessing a 2 dimentional array with a row of length domInfo.nrVirtCpu, it's a one dimension array so I think cpuMaps[vcpu][byteCpu] is equivalent to cpuMaps[vcpu + byteCpu] there and unless I misunderstood both case that won't work as expected. This is also why I suggested a macro this can of code is easy to get wrong. Daniel -- Daniel Veillard | Red Hat http://redhat.com/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Just a C implem nitpick, but that can lead to interesting debugging sessions (I think I got beaten by that a couple of time). Nowhere the compiler can guess that you're accessing a 2 dimentional array with a row of length domInfo.nrVirtCpu, it's a one dimension array so I think cpuMaps[vcpu][byteCpu] is equivalent to cpuMaps[vcpu + byteCpu] there and unless I misunderstood both case that won't work as expected. This is also why I suggested a macro this can of code is easy to get wrong.
Daniel OK, you are right. ;-) Thanks.
On Wed, Aug 02, 2006 at 09:48:35AM +0200, Philippe Berthault wrote:
Just a C implem nitpick, but that can lead to interesting debugging sessions (I think I got beaten by that a couple of time). Nowhere the compiler can guess that you're accessing a 2 dimentional array with a row of length domInfo.nrVirtCpu, it's a one dimension array so I think cpuMaps[vcpu][byteCpu] is equivalent to cpuMaps[vcpu + byteCpu] there and unless I misunderstood both case that won't work as expected. This is also why I suggested a macro this can of code is easy to get wrong.
Daniel OK, you are right. ;-)
That happens, sometimes, when it's clearly not the case don't hesistate to yell a bit because I can be stubborn :-) Daniel -- Daniel Veillard | Red Hat http://redhat.com/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Philippe Berthault wrote:
Daniel Veillard a écrit :
On Tue, Aug 01, 2006 at 05:06:59PM +0200, Philippe Berthault wrote:
[snip]
I wonder how people are most likely to use those APIs. Building scenarios like: - physical CPU is to be locked to serve only VCPU N in domain D - domain A and domain B should be served by disjoint physical CPUs sets - monitoring are the most common uses I would guess but I may be wrong. First would require: - virDomainPinVcpu I guess Second would require: - virDomainGetCpus and a number of calls to limit to sets :-\ The last one is likely to require getting full maps, and since it is likely to be called frequently the cheapest the better
If people who expressed interest on the list about VCPU function could express their principal use case it may help getting the APIs right.
About third point (monitoring): I think the virDomainGetVcpus API isn't adequate for this purpose. It would be better to have an API (to be defined) which give the state of all physical CPUs because it's the hardware resources we need to monitor, not the virtual ones. The virDomainGetVcpus API permits to obtain the relation vcpu->physical_cpu(s) but for monitoring usage, it's not interesting. It would be better to have an API which give the reverse relation : physical_cpu->vpcu(s) independently of domains and give the physical CPU usage. With the virDomainGetVcpus API, it's impossible to determine if a physical cpu is underused or overused and it's this information we need to know for monitoring and for load-balancing.
I agree and do not see a monitoring use case for these APIs, only setting/getting configuration. Perhaps we need more host-side entry points, for monitoring host resources? Regards, Jim
participants (3)
-
Daniel Veillard -
Jim Fehlig -
Philippe Berthault