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

[Daniel P. Berrange]

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 at cn.fujitsu.com>
> > 
> > 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 at cn.fujitsu.com>
> > Signed-off-by: Hu Tao <hutao at cn.fujitsu.com>
> > ---
> >  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 :|