This interesting thread is temporarilu suspended because Michel has left
for some (long) vacation until August 15th.
I think the last proposal is a good one and deserves to be implemented and
tested.
I am confident that Michel will be willing to update its patch accordingly
when he comes back.
Is it OK for you ?.
Side question: is there a defined "sign-off" procedure to follow when
submitting patches ?
Cordialement.
Jean-Paul
Daniel Veillard <veillard(a)redhat.com>
Envoyé par : libvir-list-bounces(a)redhat.com
17/07/2006 15:08
Veuillez répondre à veillard
Pour : "Daniel P. Berrange" <berrange(a)redhat.com>
cc : libvir-list(a)redhat.com, michel.ponceau(a)bull.net
Objet : Re: [Libvir] Re: Proposal : add 3 functions to Libvirt API, for
virtual CPUs
On Mon, Jul 17, 2006 at 01:50:25PM +0100, Daniel P. Berrange wrote:
On Tue, Jul 11, 2006 at 10:50:53AM -0400, Daniel Veillard wrote:
> We still have a relatively simple API for the common case, and for
special
> cases we have an extension capability with relatively clear
definitions. it's
> a bitstrange but I think that should cover most case as best as
possible
I dont particularly like this as an API because I think it will be error
prone for application developers. Most app developers will only ever
have
a handful of CPUs in their test machines, so they'll never the
alternate
codepath for > 256 cpu case. Likewise I don't like the idea of a
virVcpuInfo
struct which has a variable size because it will totally confuse
people
who
haven't read the API docs very carefully, again leading to
obscure bugs.
The root problem is that we have two conflicting goals here
1. Want to have virVcpuInfo be a fixed size struct
2. We want a cpumap of arbitrary size
Right this is confusing, I wanted to avoid another allocation in the
general
case but this made things even more confusing.
The obvious solution to this problem is to *remove* the cpumap data
from
the virVcpuInfo structure completely, and always pass in a separately
malloc'd array of the correct size. So I'd suggest:
typedef struct _virVcpuInfo virVcpuInfo;
struct _virVcpuInfo {
unsigned int number; /* virtual CPU number */
int state; /* value from virVcpuState */
unsigned long long cpuTime; /* CPU time used, in nanoseconds */
int cpu; /* real CPU number, or -1 if offline */
}
virDomainGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int
maxinfo,
char *cpumap, int maplen);
Agreed, this is the most logical approach
The client applications calling this API already have to malloc() the
memory
region for the 'info' parameter of a correct size, so having
to also
malloc
the cpumap parameter is no extra trouble.
virDomainInfo info;
virDomainVpuInfoPtr cpuInfo;
int cpuMapLen;
char *cpuMap;
virDomainGetInfo(domain, &info);
cpuInfo = malloc(sizeof(virDomainVcpuInfo)*info.nrVirtCpu);
cpuMapLen = (info.nrVirtCpu + 7) / 8 ;
cpuMap = malloc(cpuMapLen);
virDomainGetVCpus(domain, cpuInfo, info.nrVirtCpu, cpuMap, cpuMapLen);
... do stuff with the data ...
free(cpuInfo);
free(cpuMap);
So you can see there is minimal extra work to always pass in cpuMap as
a separate parameter. If an application didn't care about the cpuMap
data they could simply pass in NULL.
Agree. We should keep an example of use (like above completed) in the
source
tree to help developpers.
Daniel
--
Daniel Veillard | Red Hat
http://redhat.com/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit
http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine
http://rpmfind.net/
--
Libvir-list mailing list
Libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list