Tanks for your tips.
2014-12-17 10:02 GMT+01:00 Markus Armbruster <arm...@redhat.com>:
> > + if (queues == 0) {
> > + error_set(errp, QERR_DEVICE_NOT_FOUND, name);
> > + return (int64_t) -1;
> > + }
> > +
> > + nc = ncs[0];
> > + ret = ncs[0]->link_down;
> > +
> > + if (nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
> > + ret = ncs[0]->peer->link_down;
> > + }
>
> Can you explain why examining only the first queue suffices?
>
>
Normally there should be only one nic with the name of @name.
I could check the number of queues tho verify this.
> +
> > + return (int64_t) ret ? 0 : 1;
>
> Superfluous cast of ret.
>
> > +}
> > +
> > void qmp_set_link(const char *name, bool up, Error **errp)
> > {
> > NetClientState *ncs[MAX_QUEUE_NUM];
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 9ffdcf8..c5321e6 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -1200,6 +1200,21 @@
> > { 'command': 'inject-nmi' }
> >
> > ##
> > +# @get_link_status
>
> We have two informational commands named like "get":
> trace-event-set-state and qom-get. We have many named query-FOO. But
> they usually don't take an argument. Hmm.
>
I thought get is more precise the only link_status;
>
> > +#
> > +# Get the current link state of the nics or nic.
> > +#
> > +# @name: name of the nic you get the state of
>
> "nics or nic" (one or more) or "the nic" (just one)?
>
>
It should only nic (just one).
First concept was to get both and the I forgot to change it.
> Doesn't your code above support any network client, not just NICs?
>
I didn't test it with network clients!
>
> > +#
> > +# Return: If link is up 1
> > +# If link is down 0
> > +# If an error occure an empty string.
>
> A QMP command makes the server send either a success response, of the
> type declared in the schema (here: 'returns': 'int'), or an error
> response.
>
> If we have nothing special to say about the error response in the
> schema's command documentation, we say nothing at all.
>
> Bool is a more natural type then int for the answer to "is the link up?"
>
Yes.
>
> We usually make return value objects to facilitate future extensions.
> Perhaps not an issue for a command that answers the "is the link up?"
> question. Begs the question whether we should we have a more general
> command to query NIC properties.
>
> Should link status be visible in HMP's "info network"?
>
I thought in fact of completeness.
>
> Stefan, what's the QMP equivalent to "info network"?
>
> > +#
> > +# Notes: this is an Proxmox VE extension and not offical part of Qemu.
>
> Really?
>
I pushed it first to this Open Source Project and forgot to erase it.
>
> > +##
> > +{ 'command': 'get_link_status', 'data': {'name': 'str'}, 'returns':
> 'int'}
> > +
> > +##
> > # @set_link:
> > #
> > # Sets the link status of a virtual network adapter.
> > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > index 718dd92..ecc501a 100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -1431,6 +1431,28 @@ Example:
> > EQMP
> >
> > {
> > + .name = "get_link_status",
> > + .args_type = "name:s",
> > + .mhandler.cmd_new = qmp_marshal_input_get_link_status,
> > + },
> > +
> > +SQMP
> > +get_link_status
> > +--------
> > +
> > +Get the link status of a network adapter.
> > +
> > +Arguments:
> > +
> > +- "name": network device name (json-string)
> > +
> > +Example:
> > +
> > +-> { "execute": "set_link", "arguments": { "name": "e1000.0" } }
> > +<- { "return": {1} }
> > +EQMP
> > +
> > + {
> > .name = "set_link",
> > .args_type = "name:s,up:b",
> > .mhandler.cmd_new = qmp_marshal_input_set_link,
>
