[libvirt] [PATCH V2 4/4] qemu: Ensure reported VCPU state is current in driver API

[Viktor Mihajlovski]

On 29.09.2016 16:35, John Ferlan wrote:
[...]
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -1478,13 +1478,17 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm,
>>          virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, i);
>>          pid_t vcpupid = qemuDomainGetVcpuPid(vm, i);
>>          virVcpuInfoPtr vcpuinfo = info + ncpuinfo;
>> +        bool vcpuhalted = qemuDomainGetVcpuHalted(vm, i);
>>  
>>          if (!vcpu->online)
>>              continue;
>>  
>>          if (info) {
>>              vcpuinfo->number = i;
>> -            vcpuinfo->state = VIR_VCPU_RUNNING;
>> +            if (vcpuhalted)
>> +                vcpuinfo->state = VIR_VCPU_HALTED;
> 
> And this causes the client to see "halted" even though the vcpu may be
> running, but just not busy.
> 
> Also if (vm->def->virtType == VIR_DOMAIN_VIRT_QEMU), then we'll always
> be halted since qemuDomainRefreshVcpuHalted will avoid the refetch of data.
> 
> 
agree: as discussed in 3/4, wrong default for vcpuhalted
>> +            else
>> +                vcpuinfo->state = VIR_VCPU_RUNNING;
>>  
>>              if (qemuGetProcessInfo(&vcpuinfo->cpuTime,
>>                                     &vcpuinfo->cpu, NULL,
>> @@ -5370,6 +5374,7 @@ qemuDomainGetVcpus(virDomainPtr dom,
>>                     unsigned char *cpumaps,
>>                     int maplen)
>>  {
> 
> The opposite end of virDomainGetVcpus a/k/a 'virsh vcpuinfo'
> 
>> +    virQEMUDriverPtr driver = dom->conn->privateData;
>>      virDomainObjPtr vm;
>>      int ret = -1;
>>  
>> @@ -5385,6 +5390,13 @@ qemuDomainGetVcpus(virDomainPtr dom,
>>          goto cleanup;
>>      }
>>  
>> +    if (qemuDomainRefreshVcpuHalted(driver, vm, QEMU_ASYNC_JOB_NONE) < 0) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                       "%s",
>> +                       _("could not refresh CPU states"));
> 
> This overwrites what message qemuDomainRefreshVcpuHalted should have
> generated.  Besides the "%s", could be on the previous line...
> 
yeah, rebase damage (similar to rc = 2)
[...]
now, the comments are rather easy to incorporate in a V3 of this series,
but the main question for me is how to address your concerns about
exposing the idleness of x86 vcpus?

-- 

Mit freundlichen Grüßen/Kind Regards
   Viktor Mihajlovski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294