Re: [PATCH] qemu: fix regression with pinning

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/27/2012 10:18 AM, Peter Krempa wrote:
> On 08/27/12 08:09, Martin Kletzander wrote:
>> Commit 4b03d59167f4a4c6ec57def315a61d977466e75b changed the pinning
>> behavior in a way that makes some machine non-startable.
> 
> s/machine/machines/
> 

OK, fixed.

>> The comment mentioning that we cannot control each vcpu when there is
>> no VCPU<->PID mapping available is true, however, this isn't
>> necessarily an error, because this can be caused by old QEMU without
>> support for "query-cpus" command as well as a software emulated
>> machines that don't create more than one process.
>> ---
>>   src/qemu/qemu_cgroup.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
>> index 2237d11..b45bb49 100644
>> --- a/src/qemu/qemu_cgroup.c
>> +++ b/src/qemu/qemu_cgroup.c
>> @@ -567,9 +567,9 @@ int qemuSetupCgroupForVcpu(struct qemud_driver
>> *driver, virDomainObjPtr vm)
>>           /* If we don't know VCPU<->PID mapping or all vcpu runs in
>> the same
>>            * thread, we cannot control each vcpu.
>>            */
>> -        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> -                        _("Unable to get vcpus' pids."));
>> -        goto cleanup;
> 
> Hm, the cleanup label should be named "error" in this function.
> 

Yes it should, maybe some other patch... =)

>> +        VIR_DEBUG("%s", _("Unable to get vcpus' pids."));
> 
> We don't translate debug messages.
> 
> Wouldn't a VIR_WARN be better in this case? Users usualy don't have
> debug logs turned on.
> 

OK, I've changed it to:

VIR_WARN("Unable to get vcpus' pids.");

But it's hard to say if this is warning or not. Anyway, definitely
better than error.

>> +        virCgroupFree(&cgroup);
>> +        return 0;
>>       }
>>
>>       for (i = 0; i < priv->nvcpupids; i++) {
> 
> ACK when you fix the message.
> 

Thanks, fixed and pushed.

Martin

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]