Re: [libvirt] [PATCH 09/17] Introduce virDomainPinHypervisorFlags and virDomainGetHypervisorPinInfo functions.

On 08/03/2012 12:36 AM, Hu Tao wrote: > From: Tang Chen <tangchen(a)cn.fujitsu.com&gt; > > Introduce 2 APIs for client to use. > 1) virDomainPinHypervisorFlags: call remote driver api, such as remoteDomainPinHypervisorFlags. > 2) virDomainGetHypervisorPinInfo: call remote driver api, such as remoteDomainGetHypervisorPinInfo. > > Signed-off-by: Tang Chen <tangchen(a)cn.fujitsu.com&gt; > Signed-off-by: Hu Tao <hutao(a)cn.fujitsu.com&gt; > --- > include/libvirt/libvirt.h.in | 10 +++ > src/driver.h | 13 +++- > src/libvirt.c | 147 ++++++++++++++++++++++++++++++++++++++++++ > src/libvirt_public.syms | 2 + > 4 files changed, 171 insertions(+), 1 deletion(-) Here's where I start to have questions on whether this approach makes sense. We already have a function for doing pinning, so why not add a new flag and reuse the existing function instead of adding a new one? That is, it might be nicer to change virDomainPinVcpuFlags (side note, why did we name it with 'Flags' in the name? In retrospect, that was a dumb naming choice) to have the 'vcpu' argument become signed, with -1 now being a valid value for all hypervisor threads not tied to a vcpu thread. Since the function has extern "C" linkage, it would not be an ABI break (it _would_ be a minor API break, but the only code that would fail to recompile is code that uses a function pointer to virDomainPinVcpuFlags, since most code would just get C's implicit casting rules). That is, instead of adding a new function, why can't: virDomainPinVcpuFlags(dom, -1, map, len, flags) serve as a way to pin all non-vcpu threads? Or if changing from 'unsigned int vcpu' to 'int vcpu' in the pinning function is unpalatable, then we could use: virDomainPinVcpuFlags(dom, 0, map, len, flags | VIR_DOMAIN_PIN_VCPU_HYPERVISOR) And for querying the hypervisor pinning, how about: virDomainGetVcpuPinInfo(dom, 1, map, len, flags | VIR_DOMAIN_GET_VCPU_PIN_HYPERVISOR)