[libvirt] [PATCH v4 2/4] vcpupin: implement the code to address the new API in the qemu driver

[Daniel Veillard]

On Fri, Jun 10, 2011 at 02:55:44PM +0900, Taku Izumi wrote:
> 
> This patch implements the code to address the new API (virDomainPinVcpuFlags)
> in the qemu driver.
> 
> Signed-off-by: Taku Izumi <izumi.taku at jp.fujitsu.com>
> ---
>  src/qemu/qemu_driver.c |   99 +++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 76 insertions(+), 23 deletions(-)
> 
> Index: libvirt/src/qemu/qemu_driver.c
> ===================================================================
> --- libvirt.orig/src/qemu/qemu_driver.c
> +++ libvirt/src/qemu/qemu_driver.c
> @@ -2880,17 +2880,24 @@ qemudDomainSetVcpus(virDomainPtr dom, un
>  
>  
>  static int
> -qemudDomainPinVcpu(virDomainPtr dom,
> -                   unsigned int vcpu,
> -                   unsigned char *cpumap,
> -                   int maplen) {
> +qemudDomainPinVcpuFlags(virDomainPtr dom,
> +                        unsigned int vcpu,
> +                        unsigned char *cpumap,
> +                        int maplen,
> +                        unsigned int flags) {
> +
>      struct qemud_driver *driver = dom->conn->privateData;
>      virDomainObjPtr vm;
> +    virDomainDefPtr persistentDef = NULL;
>      int maxcpu, hostcpus;
>      virNodeInfo nodeinfo;
>      int ret = -1;
> +    bool isActive;
>      qemuDomainObjPrivatePtr priv;
>  
> +    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> +                  VIR_DOMAIN_AFFECT_CONFIG, -1);
> +
>      qemuDriverLock(driver);
>      vm = virDomainFindByUUID(&driver->domains, dom->uuid);
>      qemuDriverUnlock(driver);
> @@ -2903,9 +2910,18 @@ qemudDomainPinVcpu(virDomainPtr dom,
>          goto cleanup;
>      }
>  
> -    if (!virDomainObjIsActive(vm)) {
> -        qemuReportError(VIR_ERR_OPERATION_INVALID,
> -                        "%s",_("cannot pin vcpus on an inactive domain"));
> +    isActive = virDomainObjIsActive(vm);
> +    if (flags == VIR_DOMAIN_AFFECT_CURRENT) {
> +        if (isActive)
> +            flags = VIR_DOMAIN_AFFECT_LIVE;
> +        else
> +            flags = VIR_DOMAIN_AFFECT_CONFIG;
> +    }
> +
> +    if (!isActive && (flags & VIR_DOMAIN_AFFECT_LIVE)) {
> +        qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                        _("a domain is inactive; can change only "
> +                          "persistent config"));
>          goto cleanup;
>      }
>  
> @@ -2918,27 +2934,54 @@ qemudDomainPinVcpu(virDomainPtr dom,
>          goto cleanup;
>      }
>  
> -    if (nodeGetInfo(dom->conn, &nodeinfo) < 0)
> -        goto cleanup;
> +    if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> +        if (!vm->persistent) {
> +            qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                   _("cannot change persistent config of a transient domain"));
> +            goto cleanup;
> +        }
> +        if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm)))
> +            goto cleanup;
> +    }
>  
> -    hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo);
> -    maxcpu = maplen * 8;
> -    if (maxcpu > hostcpus)
> -        maxcpu = hostcpus;
> +    if (flags & VIR_DOMAIN_AFFECT_LIVE) {
>  
> -    if (priv->vcpupids != NULL) {
> -        if (virProcessInfoSetAffinity(priv->vcpupids[vcpu],
> -                                      cpumap, maplen, maxcpu) < 0)
> +        if (nodeGetInfo(dom->conn, &nodeinfo) < 0)
>              goto cleanup;
> -    } else {
> -        qemuReportError(VIR_ERR_NO_SUPPORT,
> -                        "%s", _("cpu affinity is not supported"));
> -        goto cleanup;
> +
> +        hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo);
> +        maxcpu = maplen * 8;
> +        if (maxcpu > hostcpus)
> +            maxcpu = hostcpus;

  I know it's the old code behaviour but one could argue that when
modifying the XML config file, maybe what the user requested should be
saved to the permanent definition, and adjusted when the guest is
(re)booted accoding to current running host. But it's the same debate
as "cpu pinning informations is tuning" and whether tuning should or
should not be saved in the persistent long term config.

> +        if (priv->vcpupids != NULL) {
> +            if (virProcessInfoSetAffinity(priv->vcpupids[vcpu],
> +                                          cpumap, maplen, maxcpu) < 0)
> +                goto cleanup;
> +        } else {
> +            qemuReportError(VIR_ERR_NO_SUPPORT,
> +                            "%s", _("cpu affinity is not supported"));
> +            goto cleanup;

  But snce it's the existing behaviour, ACK :-)

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/