Thanks for your reply, and I will send a new version of this
patch.
>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@xxxxxxxxxx>
>> Signed-off-by: Xi Xu <xu.xi8@xxxxxxxxxx>
>> ---
>> 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.
---
Best wishes
Yi Wang--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
