| Message ID | 1362607171-24668-2-git-send-email-lersek@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 03/06/2013 02:59 PM, Laszlo Ersek wrote: > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > qga/qapi-schema.json | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++ > qga/commands-posix.c | 12 ++++++++ > qga/commands-win32.c | 12 ++++++++ > 3 files changed, 96 insertions(+), 0 deletions(-) > > diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json > index d91d903..cba881c 100644 > --- a/qga/qapi-schema.json > +++ b/qga/qapi-schema.json > @@ -515,3 +515,75 @@ > ## > { 'command': 'guest-network-get-interfaces', > 'returns': ['GuestNetworkInterface'] } > + > +## > +# @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. > +# > +# Since: 1.5 > +## > +{ 'type': 'GuestLogicalProcessor', > + 'data': {'logical-id': 'int', Should logical-id be 'str' instead of 'int', since we said it is arbitrary what the guest names its vcpus? Then again, integers can be made to work (even if the guest OS prefers to name cpus via strings, the agent can track a 1:1 lookup table between OS string and integer number handed over qga, perhaps even by returning an invariant pointer address of the OS string as the integer identifier), so I won't insist. > +# 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. 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 <eblake@redhat.com>
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 <eblake@redhat.com> Thanks much! Laszlo
On Wed, Mar 06, 2013 at 11:48:14PM +0100, Laszlo Ersek wrote: > 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). > That's the general understanding for the current interfaces: "unsupported" is a higher-level error than the errors that individual commands might document. So I think we should keep this as-is for consistency, and if it does need to be documented better then a patch adding the big banner at the top of the schema is probably the best approach actually. > > 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 <eblake@redhat.com> > > Thanks much! > Laszlo >
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index d91d903..cba881c 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -515,3 +515,75 @@ ## { 'command': 'guest-network-get-interfaces', 'returns': ['GuestNetworkInterface'] } + +## +# @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). +# +# Since: 1.5 +## +{ 'type': 'GuestLogicalProcessor', + 'data': {'logical-id': 'int', + 'online': 'bool', + '*can-offline': 'bool'} } + +## +# @guest-get-vcpus: +# +# Retrieve the list of the guest's logical processors. +# +# This is a read-only operation. +# +# Returns: The list of all VCPUs the guest knows about. Each VCPU is put on the +# list exactly once, but their order is unspecified. +# +# Since: 1.5 +## +{ 'command': 'guest-get-vcpus', + 'returns': ['GuestLogicalProcessor'] } + +## +# @guest-set-vcpus: +# +# Attempt to reconfigure (currently: enable/disable) logical processors inside +# the guest. +# +# The input list is processed node by node in order. In each node @logical-id +# is used to look up the guest VCPU, for which @online specifies the requested +# state. The set of distinct @logical-id's is only required to be a subset of +# the guest-supported identifiers. There's no restriction on list length or on +# repeating the same @logical-id (with possibly different @online field). +# Preferably the input list should describe a modified subset of +# @guest-get-vcpus' return value. +# +# 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, +# +# < length(@vcpus): more than zero initial nodes have been processed, +# but not the entire @vcpus list. Guest state has +# changed accordingly. To retrieve the error +# (assuming it persists), repeat the call with the +# successfully processed initial sublist removed. +# Otherwise, +# +# length(@vcpus): call successful. +# +# Since: 1.5 +## +{ 'command': 'guest-set-vcpus', + 'data': {'vcpus': ['GuestLogicalProcessor'] }, + 'returns': 'int' } diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 7a0202e..7257145 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -1083,6 +1083,18 @@ void qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **err) } #endif +GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp) +{ + error_set(errp, QERR_UNSUPPORTED); + return NULL; +} + +int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) +{ + error_set(errp, QERR_UNSUPPORTED); + return -1; +} + /* register init/cleanup routines for stateful command groups */ void ga_command_state_init(GAState *s, GACommandState *cs) { diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 7e8ecb3..7781205 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -278,6 +278,18 @@ GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **err) return NULL; } +GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp) +{ + error_set(errp, QERR_UNSUPPORTED); + return NULL; +} + +int64_t qmp_guest_set_vcpus(GuestLogicalProcessorList *vcpus, Error **errp) +{ + error_set(errp, QERR_UNSUPPORTED); + return -1; +} + /* register init/cleanup routines for stateful command groups */ void ga_command_state_init(GAState *s, GACommandState *cs) {
Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- qga/qapi-schema.json | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++ qga/commands-posix.c | 12 ++++++++ qga/commands-win32.c | 12 ++++++++ 3 files changed, 96 insertions(+), 0 deletions(-)