At 04/11/2011 01:56 PM, Taku Izumi Write:
Wen-san, Hu-san, Osier-san,
Thank you for reviewing. I'll update this.
to Wen-san,
+
+ if (flags & ~(VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG)) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virDispatchError(NULL); + return -1; + } Why we check the flags here? We can check it in hypervisor's implementation.
We can't use this API with flags except VIR_DOMAIN_VCPU_LIVE or VIR_DOMAIN_VCPU_CONFIG. I think validation should be done here.
The other API virDomain*Flags do not check the flags here. If we do not check it here, it will cause some problems?
+ int ret; + ret = conn->driver->domainPinVcpuFlags (domain, vcpu, cpumap, maplen, flags); There is no need to add space between 'domainPinVcpuFlags' and '(domain', it is old coding
+ if (conn->driver->domainPinVcpuFlags) { style.
I intentionally did because such a style is adopted in a lot of other places where drivers' function are invoked.
Best regards, Taku Izumi