From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35696) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDN7T-0004P7-2s for qemu-devel@nongnu.org; Wed, 06 Mar 2013 17:47:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UDN7Q-0002TW-CT for qemu-devel@nongnu.org; Wed, 06 Mar 2013 17:47:31 -0500 Received: from mx1.redhat.com ([209.132.183.28]:18802) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDN7Q-0002TS-46 for qemu-devel@nongnu.org; Wed, 06 Mar 2013 17:47:28 -0500 Message-ID: <5137C7AE.10309@redhat.com> Date: Wed, 06 Mar 2013 23:48:14 +0100 From: Laszlo Ersek MIME-Version: 1.0 References: <1362607171-24668-1-git-send-email-lersek@redhat.com> <1362607171-24668-2-git-send-email-lersek@redhat.com> <5137C3E0.3060600@redhat.com> In-Reply-To: <5137C3E0.3060600@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 1/3] qga: introduce guest-get-vcpus / guest-set-vcpus with stubs 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/06/13 23:32, Eric Blake wrote: > On 03/06/2013 02:59 PM, Laszlo Ersek wrote: >> +## >> +# @GuestLogicalProcessor: >> +# >> +# @logical-id: Arbitrary guest-specific unique identifier of the VCPU. >> +# >> +# @online: Whether the VCPU is enabled. >> +# >> +# @can-offline: Whether offlining the VCPU is possible. This member is always >> +# filled in by the guest agent when the structure is returned, >> +# and always ignored on input (hence it can be omitted then). > > Other places have used the notation '#optional' when documenting a > parameter that need not be present on input; although we don't have > anything that strictly requires/enforces that notation. I'll fix this in v3 if I'll have to respin, otherwise I'd prefer a followup patch. >> +# Returns: The length of the initial sublist that has been successfully >> +# processed. The guest agent maximizes this value. Possible cases: >> +# >> +# 0: if the @vcpus list was empty on input. Guest state >> +# has not been changed. Otherwise, >> +# >> +# Error: processing the first node of @vcpus failed for the >> +# reason returned. Guest state has not been changed. >> +# Otherwise, >> +# > >> + >> +int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) >> +{ >> + error_set(errp, QERR_UNSUPPORTED); >> + return -1; > > This returns an error even on an empty input @vcpus, while the docs said > that returning 0 takes priority. But it's so much of a corner case that > I don't care; always returning an error seems fine. I see what you mean. In my mind, "unsupported" beats everything else, as if there was a big banner on top of the schema file: "you'll get QERR_UNSUPPORTED from any interface that's not supported". I'd like to leave this as-is even if I have to respin; distinguishing between zero-length-list and "unsupported" seems awkward, plus I'd also like to accept an empty list without error (in the supported case). > Thus, although there are things you might change if you have to respin > the series for later review comments, I'm perfectly fine leaving this > as-is and you can use: > > Reviewed-by: Eric Blake Thanks much! Laszlo