On Tue, 2008-08-05 at 08:15 -0400, bkearney@xxxxxxxxxx wrote:
> # HG changeset patch
> # User bkearney@xxxxxxxxxxxxxxxxxxxxx
> # Date 1217876425 14400
> # Node ID 3507f83147d566fb690dc9b87c0dbb35cd9e6f4c
> # Parent 6a207373b908ab521d33cd675c7c8d3854bdc1f1
> multiple nic support for virt-image. Added support to allow multiple
> interface elements in the virt-image.xml. The command line can specify
> any number of -w or -b elements and the tool will add default networks
> up to the number of nics specified. It is assumbed that eth0 is the first
> item specified.
Looks good. Two small nits:
> diff -r 6a207373b908 -r 3507f83147d5 virt-image
> --- a/virt-image Tue Jul 29 11:21:07 2008 -0400
> +++ b/virt-image Mon Aug 04 15:00:25 2008 -0400
> @@ -59,17 +59,14 @@
> cli.get_vcpus(vcpus, check_cpu, guest, conn)
>
> def get_networks(domain, macs, bridges, networks, guest):
> - (macs, networks) = cli.digest_networks(macs, bridges, networks)
> -
> - nnics = 0
> - if domain.interface:
> - nnics = 1
> + nnics = domain.interface
> + (macs, networks) = cli.digest_networks(macs, bridges, networks, nnics)
>
> if nnics == 0 and len(networks) > 0:
> print >> sys.stderr, _("Warning: image does not support networking, ignoring network related options")
> return
This check should be 'if len(networks) > nnics' and then warn that the
last len(networks) - nnics network specs will be ignored.
> diff -r 6a207373b908 -r 3507f83147d5 virtinst/cli.py
> --- a/virtinst/cli.py Tue Jul 29 11:21:07 2008 -0400
> +++ b/virtinst/cli.py Mon Aug 04 15:00:25 2008 -0400
> @@ -262,41 +262,41 @@
> fail(_("Unknown network type ") + network)
> guest.nics.append(n)
>
> -def digest_networks(macs, bridges, networks):
> +def digest_networks(macs, bridges, networks, nics = 1):
> if type(bridges) != list and bridges != None:
> bridges = [ bridges ]
>
> if type(macs) != list and macs != None:
> macs = [ macs ]
> + elif macs is None:
> + macs = []
>
> if type(networks) != list and networks != None:
> networks = [ networks ]
> + elif networks is None:
> + networks = []
It would be clearer to write these two as
if macs is None:
macs = []
elif type(macs) != list
macs = [ macs ]
same for networks
David
_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/et-mgmt-tools
