Multiple nic patch with DLutterkort's comments applied

08-05-2008, 08:39 PM

# HG changeset patch
# User bkearney@localhost.localdomain
# Date 1217876425 14400
# Node ID d9910a83bbf6a7aa85ebcc14f949071c9e9c5eb6
# 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.

diff -r 6a207373b908 -r d9910a83bbf6 doc/image.rng
--- a/doc/image.rng Tue Jul 29 11:21:07 2008 -0400
+++ b/doc/image.rng Mon Aug 04 15:00:25 2008 -0400
@@ -47,8 +47,10 @@
<element name="vcpu"><ref name="countCPU"/></element>

<element name="memory"><ref name="memoryKB"/></element>
- 
- <element name="interface"><empty/></element>
+ 
+ <zeroOrMore>
+ <element name="interface"><empty/></element>
+ </zeroOrMore>

<element name="graphics"><empty/></element>
</element>
diff -r 6a207373b908 -r d9910a83bbf6 tests/image.py
--- a/tests/image.py Tue Jul 29 11:21:07 2008 -0400
+++ b/tests/image.py Mon Aug 04 15:00:25 2008 -0400
@@ -31,7 +31,16 @@
self.assertTrue(img.domain)
self.assertEqual(5, len(img.storage))
self.assertEqual(2, len(img.domain.boots))
+ self.assertEqual(1, img.domain.interface)
boot = img.domain.boots[0]
self.assertEqual("xvdb", boot.drives[1].target)
+
+ def testMultipleNics(self):
+ file = open(os.path.join("tests", "image2nics.xml"), "r")
+ xml = file.read()
+ file.close()
+
+ img = virtinst.ImageParser.parse(xml, ".")
+ self.assertEqual(2, img.domain.interface)
if __name__ == "__main__":
unittest.main()
diff -r 6a207373b908 -r d9910a83bbf6 tests/image.xml
--- a/tests/image.xml Tue Jul 29 11:21:07 2008 -0400
+++ b/tests/image.xml Mon Aug 04 15:00:25 2008 -0400
@@ -28,10 +28,12 @@
<drive disk="data.raw" target="hdb"/>
<drive disk="scratch.raw" target="hdd"/>
</boot>
- <vcpu>7</vcpu>
- <memory>262144</memory>
- <interface/>
- <graphics/>
+ <devices>
+ <vcpu>7</vcpu>
+ <memory>262144</memory>
+ <interface/>
+ <graphics/>
+ </devices>
</domain>
<storage>
<disk file="disk.img"/>
diff -r 6a207373b908 -r d9910a83bbf6 tests/image2nics.xml
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/image2nics.xml Mon Aug 04 15:00:25 2008 -0400
@@ -0,0 +1,49 @@
+<image>
+ <name>test-image</name>
+ <label>A simple test image</label>
+ <domain>
+ <boot type='xen'>
+ <guest>
+ <os_type>xen</os_type>
+ <arch>i386</arch>
+ <features><pae/></features>
+ </guest>
+ <os>
+ <loader>pygrub</loader>
+ </os>
+ <drive disk="root.raw" target="xvda"/>
+ <drive disk="data.raw" target="xvdb"/>
+ <drive disk="scratch.raw" target="xvdc"/>
+ </boot>
+ <boot type="hvm">
+ <guest>
+ <arch>i686</arch>
+ <features><pae/></features>
+ </guest>
+ <os>
+ <type>hvm</type>
+ <loader dev="hd"/>
+ </os>
+ <drive disk="root.raw" target="hda"/>
+ <drive disk="data.raw" target="hdb"/>
+ <drive disk="scratch.raw" target="hdd"/>
+ </boot>
+ <devices>
+ <vcpu>7</vcpu>
+ <memory>262144</memory>
+ <interface/>
+ <interface/>
+ <graphics/>
+ </devices>
+ </domain>
+ <storage>
+ <disk file="disk.img"/>
+ <disk size="4096" use="system">
+ <partition file="boot.img"/>
+ <partition file="root.img"/>
+ </disk>
+ <disk file="root.raw" format="raw" size="4096" use="system"/>
+ <disk file="data.raw" format="raw" size='2048' use="data"/>
+ <disk file="scratch.raw" format="raw" size='100' use='scratch'/>
+ </storage>
+</image>
diff -r 6a207373b908 -r d9910a83bbf6 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,15 @@
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 = domain.interface
+ (macs, networks) = cli.digest_networks(macs, bridges, networks, nnics)

- nnics = 0
- if domain.interface:
- nnics = 1
-
- if nnics == 0 and len(networks) > 0:
- print >> sys.stderr, _("Warning: image does not support networking, ignoring network related options")
- return
- elif nnics == 1 and len(networks) == 0:
- fail(_("The image needs one network interface"))
+ if len(networks) > nnics:
+ print >> sys.stderr, (_("Warning: more networks were provided [%i] then nics required [%i]. All extras are ignored") % (len(networks), nnics))
+ networks = networks[0:nnics]
+ macs = macs [0:nnics]
+ elif nnics > len(networks):
+ fail(_("The image requires %i network interface") % nnics)

map(lambda m, n: cli.get_network(m, n, guest), macs, networks)

diff -r 6a207373b908 -r d9910a83bbf6 virtinst/ImageParser.py
--- a/virtinst/ImageParser.py Tue Jul 29 11:21:07 2008 -0400
+++ b/virtinst/ImageParser.py Mon Aug 04 15:00:25 2008 -0400
@@ -90,7 +90,7 @@
self.boots = []
self.vcpu = None
self.memory = None
- self.interface = None
+ self.interface = 0
self.graphics = None
if not node is None:
self.parseXML(node)
@@ -99,7 +99,7 @@
self.boots = [ Boot(b) for b in node.xpathEval("boot") ]
self.vcpu = xpathString(node, "devices/vcpu", 1)
self.memory = xpathString(node, "devices/memory")
- self.interface = node.xpathEval("count(devices/interface)") > 0
+ self.interface = int(node.xpathEval("count(devices/interface)"))
self.graphics = node.xpathEval("count(devices/graphics)") > 0

# FIXME: There must be a better way to check this
diff -r 6a207373b908 -r d9910a83bbf6 virtinst/UnWare.py
--- a/virtinst/UnWare.py Tue Jul 29 11:21:07 2008 -0400
+++ b/virtinst/UnWare.py Mon Aug 04 15:00:25 2008 -0400
@@ -143,7 +143,9 @@
self.label = image.label
self.vcpu = domain.vcpu
self.memory = domain.memory
- self.interface = domain.interface
+ # Make this a boolean based on the existence of one or more
+ # interfaces in the domain
+ self.interface = domain.interface > 0

self.disks = []
for d in boot.drives:
diff -r 6a207373b908 -r d9910a83bbf6 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:
+ if macs is None:
+ macs = []
+ elif type(macs) != list:
macs = [ macs ]
-
- if type(networks) != list and networks != None:
- networks = [ networks ]
+
+ if networks is None:
+ networks = []
+ elif type(networks) != list:
+ networks = [ macs ]

if bridges is not None and networks != None:
fail(_("Cannot mix both --bridge and --network arguments"))

- # ensure we have equal length lists
+
if bridges != None:
networks = map(lambda b: "bridge:" + b, bridges)
-
- if networks != None:
- if macs != None:
- if len(macs) != len(networks):
- fail(_("Need to pass equal numbers of networks & mac addresses"))
- else:
- macs = [ None ] * len(networks)
- else:
- if os.getuid() == 0:
- net = util.default_network()
- networks = [net[0] + ":" + net[1]]
- else:
- networks = ["user"]
- if macs != None:
- if len(macs) > 1:
- fail(_("Need to pass equal numbers of networks & mac addresses"))
- else:
- macs = [ None ]
-
+
+ # ensure we have equal length lists
+ if len(macs) != len(networks):
+ fail(_("Need to pass equal numbers of networks & mac addresses"))
+
+ # Create extra networks up to the number of nics requested
+ if len(macs) < nics:
+ for cnt in range(len(macs),nics):
+ if os.getuid() == 0:
+ net = util.default_network()
+ networks.append(net[0] + ":" + net[1])
+ else:
+ networks.append("user")
+ macs.append(None)
+
return (macs, networks)

def get_graphics(vnc, vncport, nographics, sdl, keymap, guest):

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@redhat.com
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

08-05-2008, 09:28 PM

On Tue, 2008-08-05 at 16:39 -0400, bkearney@redhat.com wrote:
> diff -r 6a207373b908 -r d9910a83bbf6 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:
> + if macs is None:
> + macs = []
> + elif type(macs) != list:
> macs = [ macs ]
> -
> - if type(networks) != list and networks != None:
> - networks = [ networks ]
> +
> + if networks is None:
> + networks = []
> + elif type(networks) != list:
> + networks = [ macs ]
>
> if bridges is not None and networks != None:
> fail(_("Cannot mix both --bridge and --network arguments"))
>
> - # ensure we have equal length lists
> +
> if bridges != None:
> networks = map(lambda b: "bridge:" + b, bridges)
> -
> - if networks != None:
> - if macs != None:
> - if len(macs) != len(networks):
> - fail(_("Need to pass equal numbers of networks & mac addresses"))
> - else:
> - macs = [ None ] * len(networks)
> - else:
> - if os.getuid() == 0:
> - net = util.default_network()
> - networks = [net[0] + ":" + net[1]]
> - else:
> - networks = ["user"]
> - if macs != None:
> - if len(macs) > 1:
> - fail(_("Need to pass equal numbers of networks & mac addresses"))
> - else:
> - macs = [ None ]
> -
> +
> + # ensure we have equal length lists
> + if len(macs) != len(networks):
> + fail(_("Need to pass equal numbers of networks & mac addresses"))
> +
> + # Create extra networks up to the number of nics requested
> + if len(macs) < nics:
> + for cnt in range(len(macs),nics):
> + if os.getuid() == 0:
> + net = util.default_network()
> + networks.append(net[0] + ":" + net[1])
> + else:
> + networks.append("user")
> + macs.append(None)
> +
> return (macs, networks)

I didn't notice this the first time through: it used to be that you
could just specify the network/bridge to connect to, without giving an
explicit MAC (so that you use a random MAC)

We definitely want to keep that ability. I think the above should be
changed so that the behavior is (1) you need to specify nnics networks
and/or bridges (2) you may specify up to nnics explicit MACS, with the
meaning that successive -w and -b options talk about successive
interfaces, and explicit MACS are assigned to them in sequence. For
interfaces that don't have an explicit MAC assigned, we'll generate a
random MAC.

For example,

virt-image -b br0 -w default -m 00:16:..

would connect the first interface to the bridge 'br0' on the host, and
the second interface to the 'default' network. The first interface would
get the explicit MAC assigned, whereas the second one would get some
random MAC.

David

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@redhat.com
https://www.redhat.com/mailman/listinfo/et-mgmt-tools

08-06-2008, 12:08 PM

David Lutterkort wrote:

On Tue, 2008-08-05 at 16:39 -0400, bkearney@redhat.com wrote:

diff -r 6a207373b908 -r d9910a83bbf6 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:

+ if macs is None:
+ macs = []
+ elif type(macs) != list:
macs = [ macs ]
-
- if type(networks) != list and networks != None:
- networks = [ networks ]
+
+ if networks is None:
+ networks = []
+ elif type(networks) != list:
+ networks = [ macs ]

if bridges is not None and networks != None:

fail(_("Cannot mix both --bridge and --network arguments"))

- # ensure we have equal length lists

+
if bridges != None:
networks = map(lambda b: "bridge:" + b, bridges)
-
- if networks != None:
- if macs != None:
- if len(macs) != len(networks):
- fail(_("Need to pass equal numbers of networks & mac addresses"))
- else:
- macs = [ None ] * len(networks)
- else:
- if os.getuid() == 0:
- net = util.default_network()
- networks = [net[0] + ":" + net[1]]
- else:
- networks = ["user"]
- if macs != None:
- if len(macs) > 1:
- fail(_("Need to pass equal numbers of networks & mac addresses"))
- else:
- macs = [ None ]
-
+
+ # ensure we have equal length lists
+ if len(macs) != len(networks):

+ fail(_("Need to pass equal numbers of networks & mac addresses"))
+
+ # Create extra networks up to the number of nics requested
+ if len(macs) < nics:

+ for cnt in range(len(macs),nics):
+ if os.getuid() == 0:
+ net = util.default_network()
+ networks.append(net[0] + ":" + net[1])
+ else:
+ networks.append("user")
+ macs.append(None)
+
return (macs, networks)

I didn't notice this the first time through: it used to be that you
could just specify the network/bridge to connect to, without giving an
explicit MAC (so that you use a random MAC)

We definitely want to keep that ability. I think the above should be
changed so that the behavior is (1) you need to specify nnics networks
and/or bridges (2) you may specify up to nnics explicit MACS, with the
meaning that successive -w and -b options talk about successive
interfaces, and explicit MACS are assigned to them in sequence. For
interfaces that don't have an explicit MAC assigned, we'll generate a
random MAC.

For example,

virt-image -b br0 -w default -m 00:16:..

would connect the first interface to the bridge 'br0' on the host, and

the second interface to the 'default' network. The first interface would
get the explicit MAC assigned, whereas the second one would get some
random MAC.

I will rework and submit. I will error out if you pass more MAC
addresses then networks or bridges.

-- bk

_______________________________________________
et-mgmt-tools mailing list
et-mgmt-tools@redhat.com
https://www.redhat.com/mailman/listinfo/et-mgmt-tools