On Mon, Aug 06, 2012 at 05:31:23PM -0600, Eric Blake wrote: > On 08/03/2012 12:36 AM, Hu Tao wrote: > > From: Tang Chen <tangchen@xxxxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxxx> > > Signed-off-by: Hu Tao <hutao@xxxxxxxxxxxxxx> > > --- > > 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) While what you describe could be made to work, I tend to prefer the idea of adding in new APIs specifically for this, and dealing with code duplication inside the drivers instead of at the public API level. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list