On 11/19/2010 07:55 AM, Eduardo Otubo wrote:
> Now adding some basic operation network functions and its UUID
> "helpers".
> ---
> src/phyp/phyp_driver.c | 202 ++++++++++++++++++++++++++++++++++++++++++++++-
> src/phyp/phyp_uuid.c | 177 ++++++++++++++++++++++++++++++++++++++++++
> src/phyp/phyp_uuid.h | 8 ++
> 3 files changed, 382 insertions(+), 5 deletions(-)
>
> +int
> +phypListNetworkMAC(virConnectPtr conn, long long *macs, int nnets)
This should probably be static.
> +{
> + ConnectionData *connection_data = conn->networkPrivateData;
> + phyp_driverPtr phyp_driver = conn->privateData;
> + LIBSSH2_SESSION *session = connection_data->session;
> + int system_type = phyp_driver->system_type;
> + char *managed_system = phyp_driver->managed_system;
> + int vios_id = phyp_driver->vios_id;
> + int exit_status = 0;
> + int got = -1;
> + char *cmd = NULL;
> + char *ret = NULL;
> + char *line, *next_line;
> + virBuffer buf = VIR_BUFFER_INITIALIZER;
> +
> + virBufferAddLit(&buf, "lshwres");
> + if (system_type == HMC)
> + virBufferVSprintf(&buf, " -m %s", managed_system);
> +
> + virBufferVSprintf(&buf, " -r virtualio --rsubtype eth --level lpar"
> + "|grep -v lpar_id=%d| sed -e 's/^.*mac_addr=//g'",
That two-process grep | sed is more efficient as one, and ^ can only
match once, so the g is pointless:
| sed '/lpar_id=%d/d; s/^.*mac_addr=//'
> +int
> +phypListNetworks(virConnectPtr conn, char **const names, int nnames)
Likewise a candidate for static (in fact, it's sometimes a good idea to
mark everything new as static, see what doesn't compile, and fix the few
fallouts - it's always better to shoot for the smallest possible scope).
> + virBufferAddLit(&buf, "lshwres");
> + if (system_type == HMC)
> + virBufferVSprintf(&buf, " -m %s", managed_system);
> + virBufferVSprintf(&buf, " -r virtualio --rsubtype slot --level slot"
> + "|grep eth|grep -v lpar_id=%d|"
> + "sed -e 's/^.*drc_name=//g'", vios_id);
Here, you can replace grep | grep | sed with:
sed -n '/lpar_id=%d/d; /eth/ s/^.*drc_name=//p'
Mostly looks okay, but there's still the issue of using long long
instead of a 6-byte array for MACs that might be worth changing.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list