On Mon, Jun 26, 2006 at 04:14:59PM +0200, michel.ponceau(a)bull.net wrote:
1) Currently the NULL virDomainPtr (meaning Domain0) is only
specified in
the 3 virDomain*Memory functions (GetMax, SetMax, Set). The other
virDomain* functions need the actual virDomainPtr for Domain0. What is the reason for
this Memory-specific
option?
I think it's problem. We should always require a virDomainPtr, I actually
think the shortcut is not implemented.
I wonder if it would be better to just remove it that would be cleaner,
however when think about other kind of virtualization, where the main
os would not be represented as a domain, but still with memory management,
in that case that should be kept.
Is it applicable to virtual CPUs?
Hum, similary, what if the main OS CPU usage could be set up even if
not mapped as a real domain, maybe this should be kept for the vCPU APIs
too, yes, a bit messy but more generic.
2) For "xm vcpu-list" equivalent function (suggested
virDomainGetVcpus) I propose to return in a new structure all the information provided by
Xen Daemon. I am trying the following addition in libvirt.h :
Remember to change libvirt.h.in as libvirt.h is generated by configure.
typedef struct _virVcpuInfo virVcpuInfo;
struct _virVcpuInfo {
int number; /* virtual CPU number */
unsigned int rather
unsigned char state; /* 'r'=running,
'b'=blocked, 'p'=offline
*/
Not too fond of using a letter there. I would prefer a number and a clean enum
unsigned long long cpuTime; /* CPU time used, in nanoseconds */
int cpu; /* last real CPU number allocated */
That's a bit fishy to me, how do you expect to use that information ?
char cpumap[128]; /* affinity map of real CPUs which
can be
allocated */
so we know we will never have libvirt used for a virtualization platform
which has no more than 256 physical CPU to offer ? Remember I don't want that
API to be limited to Xen and to me if we put this in the API this must be
future proof. I would at least grow the CPU map to 256 entries at the API
level. Oh and use unsigned char to go 0-255.
};
one last comment about packing the structure: it may be intersting to
reorganize fields in that arry to help,
int, int, long long, char, char[] looks better to me than the current
organization.
Another note is about access, do you really want to get those just from
xend or also from the hypervisor ? The hypervisor might be way cheaper to
reach and more accurate. Hopefully that structure still make sense with
the associated hypervisor informations could you look up the associated
hypervisor call and double check ?
typedef virVcpuInfo *virVcpuInfoPtr;
For cpumap, the xm command displays a string "x,y-z..." more compact than
the list of all possible CPUs "x y y+1 y+2...z..." provided by xend. But
the routines for this reformatting are not easy, so I propose to keep the
xend format in virCpuInfo. What is your opinion?
I never played with mapping (well I don't have anymore SMP box to play
locally), could you explain, I'm not sure I follow :-) . If what xm does
make sense, I don't see why introducing a new way is an improvement, but
I can hardly juge myself.
3) For "xm vcpu-pin" equivalent function (suggested
virDomainPinVcpu) I propose a CPU list argument in same format as virCpuInfo cpumap. Do
you agree?
I have the same worries about the hard constraints put on the number of
CPUs at the API level.
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/