From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51021) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDO9I-0001pL-00 for qemu-devel@nongnu.org; Wed, 06 Mar 2013 18:53:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UDO9E-0005yF-7m for qemu-devel@nongnu.org; Wed, 06 Mar 2013 18:53:27 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50444) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDO9D-0005x8-V1 for qemu-devel@nongnu.org; Wed, 06 Mar 2013 18:53:24 -0500 Message-ID: <5137D76D.8030603@redhat.com> Date: Thu, 07 Mar 2013 00:55:25 +0100 From: Laszlo Ersek MIME-Version: 1.0 References: <1362607171-24668-1-git-send-email-lersek@redhat.com> <1362607171-24668-4-git-send-email-lersek@redhat.com> <5137CF38.50301@redhat.com> In-Reply-To: <5137CF38.50301@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 3/3] qga: implement qmp_guest_set_vcpus() for Linux with sysfs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: mdroth@linux.vnet.ibm.com, qemu-devel@nongnu.org On 03/07/13 00:20, Eric Blake wrote: > On 03/06/2013 02:59 PM, Laszlo Ersek wrote: >> Signed-off-by: Laszlo Ersek >> --- >> 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 and ), and errors and retvals are mutually exclusive. > Depending on that answer, you can add: > Reviewed-by: Eric Blake > if I didn't find a reason for a respin. Yay! Thanks Laszlo