On 03/07/13 00:20, Eric Blake wrote:
> On 03/06/2013 02:59 PM, Laszlo Ersek wrote:
>> Signed-off-by: Laszlo Ersek <ler...@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 <ebl...@redhat.com>
> if I didn't find a reason for a respin.
Yay!
Thanks
Laszlo
