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 <table width=100%> <tr valign=top> <td> <td>Daniel Veillard <veillard@redhat.com> Envoyé par : libvir-list-bounces@redhat.com 17/07/2006 15:08 Veuillez répondre à veillard <td> Pour : "Daniel P. Berrange" <berrange@redhat.com> cc : libvir-list@redhat.com, michel.ponceau@bull.net Objet : Re: [Libvir] Re: Proposal : add 3 functions to Libvirt API, for virtual CPUs</table> 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@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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list