diff --git a/tests/test_cli.py b/tests/test_cli.py index 90b5ba35f..ad0f9e409 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1105,11 +1105,12 @@ c.add_valid("--network bridge:mybr0,model=e1000") # --network bridge: c.add_valid("--network network:default --mac RANDOM") # VirtualNetwork with a random macaddr c.add_valid("--vnc --keymap=local") # --keymap local c.add_valid("--panic 0x505") # ISA panic with iobase specified +c.add_valid("--mac 22:11:11:11:11:11 --check mac_in_use=off") # colliding mac, but check is skipped +c.add_invalid("--mac 22:11:11:11:11:11") # Colliding macaddr will error c.add_invalid("--graphics vnc --vnclisten 1.2.3.4") # mixing old and new c.add_invalid("--network=FOO") # Nonexistent network c.add_invalid("--mac 1234") # Invalid mac c.add_invalid("--network user --bridge foo0") # Mixing bridge and network -c.add_invalid("--connect %(URI-TEST-FULL)s --mac 22:22:33:12:34:AB") # Colliding macaddr c = vinst.add_category("storage-back-compat", "--pxe --noautoconsole") c.add_valid("--file %(EXISTIMG1)s --nonsparse --file-size 4") # Existing file, other opts @@ -1321,6 +1322,8 @@ c.add_valid("--original-xml " + _CLONE_UNMANAGED + " --auto-clone") # Auto flag c.add_valid("--connect %(URI-TEST-FULL)s -o test-clone --auto-clone --clone-running --nonsparse") # Auto flag, actual VM, skip state check c.add_valid("--connect %(URI-TEST-FULL)s -o test-clone-simple -n newvm --preserve-data --file %(EXISTIMG1)s") # Preserve data shouldn't complain about existing volume c.add_valid("-n clonetest --original-xml " + _CLONE_UNMANAGED + " --file %(EXISTIMG3)s --file %(EXISTIMG4)s --check path_exists=off") # Skip existing file check +c.add_valid("-n clonetest --original-xml " + _CLONE_UNMANAGED + " --auto-clone --mac 22:11:11:11:11:11 --check all=off") # Colliding mac but we skip the check +c.add_invalid("-n clonetest --original-xml " + _CLONE_UNMANAGED + " --auto-clone --mac 22:11:11:11:11:11", grep="--check mac_in_use=off") # Colliding mac should fail c.add_invalid("--auto-clone") # Just the auto flag c.add_invalid("-o test --clone-running --file foo") # Didn't specify new name c.add_invalid("-o test --clone-running --auto-clone -n test") # new name raises error diff --git a/tests/test_xmlconfig.py b/tests/test_xmlconfig.py index 3596739b5..73d91bfbc 100644 --- a/tests/test_xmlconfig.py +++ b/tests/test_xmlconfig.py @@ -268,6 +268,9 @@ class TestXMLMisc(unittest.TestCase): self.assertTrue(qemumac != testmac) self.assertTrue(len(randommac) == len(testmac)) + # Ensure is_conflict_net doesn't error on None + virtinst.DeviceInterface.is_conflict_net(self.conn, None) + def test_support_misc(self): try: self.conn.lookupByName("foobar-idontexist") diff --git a/virtManager/device/netlist.py b/virtManager/device/netlist.py index 0e70b6d47..4e4c08e67 100644 --- a/virtManager/device/netlist.py +++ b/virtManager/device/netlist.py @@ -298,6 +298,7 @@ class vmmNetworkList(vmmGObjectUI): def validate_device(self, net): self._check_network_is_running(net) + virtinst.DeviceInterface.is_conflict_net(net.conn, net.macaddr) net.validate() def reset_state(self): diff --git a/virtinst/cli.py b/virtinst/cli.py index c6a39121e..a43d87172 100644 --- a/virtinst/cli.py +++ b/virtinst/cli.py @@ -311,18 +311,35 @@ def check_path_search(conn, path): path, searchdata.user, searchdata.fixlist) # pragma: no cover +def _optional_fail(msg, checkname, warn_on_skip=True): + """ + Handle printing a message with an associated --check option + """ + do_check = get_global_state().get_validation_check(checkname) + if do_check: + fail(msg + (_(" (Use --check %s=off or " + "--check all=off to override)") % checkname)) + + log.debug("Skipping --check %s error condition '%s'", + checkname, msg) + if warn_on_skip: + log.warning(msg) + + +def validate_mac(conn, macaddr): + """ + There's legitimate use cases for creating/cloning VMs with duplicate + MACs, so we do the collision check here but allow it to be skipped + with --check + """ + try: + DeviceInterface.is_conflict_net(conn, macaddr) + return + except Exception as e: + _optional_fail(str(e), "mac_in_use") + + def validate_disk(dev, warn_overwrite=False): - def _optional_fail(msg, checkname, warn_on_skip=True): - do_check = get_global_state().get_validation_check(checkname) - if do_check: - fail(msg + (_(" (Use --check %s=off or " - "--check all=off to override)") % checkname)) - - log.debug("Skipping --check %s error condition '%s'", - checkname, msg) - if warn_on_skip: - log.warning(msg) - def check_path_exists(dev): """ Prompt if disk file already exists and preserve mode is not used @@ -1586,6 +1603,8 @@ class ParserCheck(VirtCLIParser): cb=cls.set_cb, lookup_cb=None) cls.add_arg("path_exists", None, is_onoff=True, cb=cls.set_cb, lookup_cb=None) + cls.add_arg("mac_in_use", None, is_onoff=True, + cb=cls.set_cb, lookup_cb=None) cls.add_arg("all", "all_checks", is_onoff=True) def set_cb(self, inst, val, virtarg): diff --git a/virtinst/cloner.py b/virtinst/cloner.py index d830df3aa..ec943bf00 100644 --- a/virtinst/cloner.py +++ b/virtinst/cloner.py @@ -171,10 +171,7 @@ class Cloner(object): # MAC address for the new guest clone def set_clone_macs(self, mac): - maclist = xmlutil.listify(mac) - for m in maclist: - DeviceInterface.is_conflict_net(self.conn, m) - self._clone_macs = maclist + self._clone_macs = xmlutil.listify(mac) def get_clone_macs(self): return self._clone_macs clone_macs = property(get_clone_macs, set_clone_macs) diff --git a/virtinst/devices/interface.py b/virtinst/devices/interface.py index 02f756fc2..229f9a089 100644 --- a/virtinst/devices/interface.py +++ b/virtinst/devices/interface.py @@ -159,6 +159,9 @@ class DeviceInterface(Device): """ Raise RuntimeError if the passed mac conflicts with a defined VM """ + if not searchmac: + return + vms = conn.fetch_all_domains() for vm in vms: for nic in vm.devices.interface: @@ -252,12 +255,6 @@ class DeviceInterface(Device): # Build API # ############# - def validate(self): - if not self.macaddr: - return - - self.is_conflict_net(self.conn, self.macaddr) - def set_default_source(self): if self.conn.is_qemu_unprivileged() or self.conn.is_test(): self.type = self.TYPE_USER diff --git a/virtinst/virtclone.py b/virtinst/virtclone.py index 5f49d14f5..0dbd4d77d 100644 --- a/virtinst/virtclone.py +++ b/virtinst/virtclone.py @@ -51,6 +51,8 @@ def get_clone_macaddr(new_mac, design): return design.clone_macs = new_mac + for mac in design.clone_macs: + cli.validate_mac(design.conn, mac) def get_clone_diskfile(new_diskfiles, design, preserve, auto_clone): diff --git a/virtinst/virtinstall.py b/virtinst/virtinstall.py index fa9366fd1..08c091268 100644 --- a/virtinst/virtinstall.py +++ b/virtinst/virtinstall.py @@ -588,6 +588,8 @@ def build_guest_instance(conn, options): # cli specific disk validation for disk in guest.devices.disk: cli.validate_disk(disk) + for net in guest.devices.interface: + cli.validate_mac(net.conn, net.macaddr) validate_required_options(options, guest, installer) show_guest_warnings(options, guest, osdata)