Re: [Qemu-devel] [PATCH v2 3/3] qga: implement qmp_guest_set_vcpus() for Linux with sysfs

[Laszlo Ersek <lersek@redhat.com>]

On 03/07/13 00:20, Eric Blake wrote:
> On 03/06/2013 02:59 PM, Laszlo Ersek wrote:
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>  qga/commands-posix.c |   38 ++++++++++++++++++++++++++++++++------
>>  1 files changed, 32 insertions(+), 6 deletions(-)
>>
>>  
>> +int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp)
>> +{
>> +    int64_t processed;
>> +    Error *local_err = NULL;
>> +
>> +    processed = 0;
>> +    while (vcpus != NULL) {
>> +        transfer_vcpu(vcpus->value, false, &local_err);
>> +        if (local_err != NULL) {
>> +            break;
>> +        }
>> +        ++processed;
>> +        vcpus = vcpus->next;
>> +    }
>> +
>> +    if (local_err != NULL) {
>> +        if (processed == 0) {
>> +            error_propagate(errp, local_err);
> 
> Do we need to set processed to -1 here, to flag to the caller that we
> propagated an error?  I'm not sure enough of the mechanics of the call
> chain, so maybe this already works even if you leave things as returning 0.

In general,

error set  output value
on output  would appear valid  what to do
---------  ------------------  --------------------------
yes        yes                 error short-circuits value
yes        no                  error short-circuits value
no         yes                 use value
no         no                  should not happen normally, exceptional
                                 cases exist (when it communicates
                                 something different from error=set),
                                 they need documentation

In particular qemu-ga follows suit; from the generated
qmp_marshal_input_guest_set_vcpus() function
[qga/qapi-generated/qga-qmp-marshal.c]:

    retval = qmp_guest_set_vcpus(vcpus, errp);
    if (!error_is_set(errp)) {
        qmp_marshal_output_guest_set_vcpus(retval, ret, errp);
    }

Also, I tested all branches that are reachable without hacking the
kernel or poking into the process with gdb. I fed qga JSON from a
terminal (using
<http://wiki.libvirt.org/page/Qemu_guest_agent#Configure_guest_agent_without_libvirt_interference>
and <http://wiki.qemu.org/Features/QAPI/GuestAgent>), and errors and
retvals are mutually exclusive.

> Depending on that answer, you can add:
> Reviewed-by: Eric Blake <eblake@redhat.com>
> if I didn't find a reason for a respin.

Yay!

Thanks
Laszlo