[libvirt] [PATCH] [PATCH] vcpupin: add clear feature

[Peter Krempa]

On Thu, Jan 04, 2018 at 05:46:32 -0500, Yi Wang wrote:
> We can't clear vcpupin settings of XML once we did vcpupin
> command, this is not convenient under some condition such
> as migration.
> 
> This patch introduces clear feature, which can clear vcpuin
> setting of XML.
> 
> Signed-off-by: Yi Wang <wang.yi59 at zte.com.cn>
> Signed-off-by: Xi Xu <xu.xi8 at zte.com.cn>
> ---
>  include/libvirt/libvirt-domain.h |  1 +
>  src/qemu/qemu_driver.c           | 24 +++++++++++++++++++-----
>  tools/virsh-domain.c             |  5 ++++-
>  tools/virsh.pod                  |  1 +
>  4 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 4048acf..7b171df 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -1837,6 +1837,7 @@ typedef enum {
>      VIR_DOMAIN_VCPU_MAXIMUM = (1 << 2), /* Max rather than current count */
>      VIR_DOMAIN_VCPU_GUEST   = (1 << 3), /* Modify state of the cpu in the guest */
>      VIR_DOMAIN_VCPU_HOTPLUGGABLE = (1 << 4), /* Make vcpus added hot(un)pluggable */
> +    VIR_DOMAIN_VCPU_CLEAR = (1 << 5), /* Clear vcpus pin info */

These are flags for a wrong API, you'll need to introduce new set of
flags for the pinning API.

>  } virDomainVcpuFlags;
>  
>  int                     virDomainSetVcpus       (virDomainPtr domain,
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 97b194b..9e8759f 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5001,7 +5001,7 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm,
>                        int vcpu,
>                        virQEMUDriverPtr driver,
>                        virQEMUDriverConfigPtr cfg,
> -                      virBitmapPtr cpumap)
> +                      virBitmapPtr cpumap, bool clear)

Please adhere to the coding standards.

>  {
>      virBitmapPtr tmpmap = NULL;
>      virDomainVcpuDefPtr vcpuinfo;
> @@ -5049,7 +5049,12 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm,
>      }
>  
>      virBitmapFree(vcpuinfo->cpumask);
> -    vcpuinfo->cpumask = tmpmap;
> +    if (clear) {
> +        virBitmapFree(tmpmap);
> +        vcpuinfo->cpumask = NULL;
> +    } else {
> +        vcpuinfo->cpumask = tmpmap;
> +    }

This will not work as expected. The API still takes the pinning map into
account when setting the pinning, so setting 'clear' will only remove
the information from the XML but will still apply 'cpumap' as the
desired pinning.

The API should not rely on the fact that the user passes in correct
cpumap in this case. You'll need to add logic which will set the pinning
to the value as if it was omitted in the XML.

>      tmpmap = NULL;
>  
>      if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0)
> @@ -5093,9 +5098,11 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
>      virBitmapPtr pcpumap = NULL;
>      virDomainVcpuDefPtr vcpuinfo = NULL;
>      virQEMUDriverConfigPtr cfg = NULL;
> +    bool clear = false;
>  
>      virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> -                  VIR_DOMAIN_AFFECT_CONFIG, -1);
> +                  VIR_DOMAIN_AFFECT_CONFIG |
> +                  VIR_DOMAIN_VCPU_CLEAR, -1);
>  
>      cfg = virQEMUDriverGetConfig(driver);
>  

[...]

> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 93cb020..4bad9e7 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -6860,7 +6860,7 @@ virshParseCPUList(vshControl *ctl, int *cpumaplen,
>      unsigned char *cpumap = NULL;
>      virBitmapPtr map = NULL;
>  
> -    if (cpulist[0] == 'r') {
> +    if (cpulist[0] == 'r' || STREQ("clear", cpulist)) {
>          if (!(map = virBitmapNew(maxcpu)))
>              return NULL;
>          virBitmapSetAll(map);
> @@ -6938,6 +6938,9 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
>          goto cleanup;
>      }
>  
> +    if (STREQ(cpulist, "clear"))
> +        flags |= VIR_DOMAIN_VCPU_CLEAR;
> +
>      /* Pin mode: pinning specified vcpu to specified physical cpus*/
>      if (!(cpumap = virshParseCPUList(ctl, &cpumaplen, cpulist, maxcpu)))
>          goto cleanup;
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 69cc423..caaa230 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -2857,6 +2857,7 @@ I<cpulist> is a list of physical CPU numbers. Its syntax is a comma
>  separated list and a special markup using '-' and '^' (ex. '0-4', '0-3,^2') can
>  also be allowed. The '-' denotes the range and the '^' denotes exclusive.
>  For pinning the I<vcpu> to all physical cpus specify 'r' as a I<cpulist>.
> +For clearing pinning info, specify 'clear' as a I<cpulist>.

The other special value is 'r' so I think this should also be a single
letter option. Or better a switch by itself.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180104/23bd5dc5/attachment-0001.sig>