|
From: | Wolfgang |
Subject: | Re: [Qemu-devel] [PATCH] qmp: implement a qmp command get_link_status |
Date: | Wed, 17 Dec 2014 11:18:07 +0100 |
> + 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?
> +
> + 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.
> +#
> +# 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)?
Doesn't your code above support any network client, not just NICs?
> +#
> +# 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?"
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"?
Stefan, what's the QMP equivalent to "info network"?
> +#
> +# Notes: this is an Proxmox VE extension and not offical part of Qemu.
Really?
> +##
> +{ '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,
[Prev in Thread] | Current Thread | [Next in Thread] |