On 23.02.2012 15:20, Luiz Capitulino wrote:
> On Sun, 19 Feb 2012 12:15:43 +0100
> Michal Privoznik <mpriv...@redhat.com> wrote:
>
>> This command returns an array of:
>>
>> [ifname, ipaddr, ipaddr_family, prefix, hwaddr]
>>
>> for each interface in the system that has an IP address.
>> Currently, only IPv4 and IPv6 are supported.
>>
>> Signed-off-by: Michal Privoznik <mpriv...@redhat.com>
>> ---
>> diff to v3:
>> -use ctpop32() instead of separate count_one_bits()
>>
>> diff to v2:
>> -Properly set IP addr family for IPv6
>>
>> diff to v1:
>> -move from guest-getip to guest-network-info
>> -replace black boxed algorithm for population count
>> -several coding styles improvements
>> qapi-schema-guest.json | 29 ++++++++
>> qga/guest-agent-commands.c | 157
>> ++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 186 insertions(+), 0 deletions(-)
>>
>> diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json
>> index 5f8a18d..ca4fdc5 100644
>> --- a/qapi-schema-guest.json
>> +++ b/qapi-schema-guest.json
>> @@ -219,3 +219,32 @@
>> ##
>> { 'command': 'guest-fsfreeze-thaw',
>> 'returns': 'int' }
>> +
>> +##
>> +# @guest-network-info:
>> +#
>> +# Get list of guest IP addresses, MAC addresses
>> +# and netmasks.
>> +#
>> +# @name: The name of interface for which info are being delivered
>> +#
>> +# @ipaddr: IP address assigned to @name
>> +#
>> +# @ipaddrtype: Type of @ipaddr (e.g. ipv4, ipv6)
>> +#
>> +# @prefix: Network prefix length
>> +#
>> +# @hwaddr: Hardware address of @name
>> +#
>> +# Returns: List of GuestNetworkInfo on success.
>> +#
>> +# Since: 1.1
>> +##
>> +{ 'enum': 'GuestIpAddrType',
>> + 'data': [ 'ipv4', 'ipv6' ] }
>> +{ 'type': 'GuestNetworkInfo',
>> + 'data': {'iface': {'name': 'str', 'ipaddr': 'str',
>> + 'ipaddrtype': 'GuestIpAddrType',
>> + 'prefix': 'int', 'hwaddr': 'str'} } }
>> +{ 'command': 'guest-network-info',
>> + 'returns': ['GuestNetworkInfo'] }
>
> No need for short names like 'ipaddr', longer names like 'ip-address' are
> better.
>
> Also, please, document the enum, the type and the command separately. You can
> look
> for examples in the qapi-schema.json file (yes, we're not doing it in this
> file, but
> we should).
>
> Do we really need the 'iface' dict, btw?
I think yes as it creates an envelope over one quintuplet so it is
obvious what value belongs to what interface. Moreover, if we would ever
expand this command to report a network but not (directly) interface
related info, say routing, whatever, we can simply add another
dictionary here. However, thinking about whole format now, what about
switching to more flexible schema:
{ 'enum': 'GuestIpAddressType',
'data': [ 'ipv4', 'ipv6' ] }
{ 'type': 'GuestIpAddress',
'data': {'ip-address': 'str',
'ip-address-type': 'GuestIpAddressType',
'prefix': 'int'} }
{ 'type': 'GuestNetworkInfo',
'data': {'interface': {'name': 'str',
'*hardware-address': 'str',
'*address': ['GuestIpAddress'] } } }
{ 'command': 'guest-network-info',
'returns': ['GuestNetworkInfo'] }
The advantage is, one get pair <interface; [list of addresses]> instead
of list<interface; address>; In other words, create an associative array
with network interface name as the key.
>> +/*
>> + * Get the list of interfaces among with their
>> + * IP/MAC addresses, prefixes and IP versions
>> + */
>> +GuestNetworkInfoList *qmp_guest_network_info(Error **err)
>> +{
>> + GuestNetworkInfoList *head = NULL, *cur_item = NULL;
>> + struct ifaddrs *ifap = NULL, *ifa = NULL;
>> + char err_msg[512];
>> +
>> + g_debug("guest-network-info called");
>> +
>> + if (getifaddrs(&ifap) < 0) {
>> + snprintf(err_msg, sizeof(err_msg),
>> + "getifaddrs failed : %s", strerror(errno));
>> + error_set(err, QERR_QGA_COMMAND_FAILED, err_msg);
>> + goto error;
>> + }
>
> Generic errors are bad for the user. The best thing to do would be to have a
> QERR_
> for the possible errors getifaddrs() can return. However, getifaddrs() can
> return
> errors from several functions, so I don't know what's the best thing to do
> here.
>
> Ideas, Michael?
Once we settle down on this I can send another version for review.
Personally, if guest agent would report description (see my other e-mail
[1]) I don't see big advantage in introducing dozens of error codes here.
1: http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg03171.html
Regards,
Michal
