From 9dce8a5dea508a69d31dd4159c566fdf119c223e Mon Sep 17 00:00:00 2001 From: Bitworks LLC Date: Thu, 6 Dec 2018 15:29:00 -0500 Subject: [PATCH] kvm: Added two more device name patterns to valid bridge slaves (lo* and dummy*) (#3000) Added dummy and lo devices to be treated as a normal bridge slave devs. Fixes #2998 Added two more device names (lo* and dummy*). Implemented tests. Code was refactored. Improved paths concatenation code from "+" to Paths.get. --- .../kvm/resource/BridgeVifDriver.java | 31 +---------- .../resource/LibvirtComputingResource.java | 52 ++++++++++--------- .../LibvirtComputingResourceTest.java | 40 +++++++++----- 3 files changed, 57 insertions(+), 66 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java index 39b92c3a352..ebaf23f1ca7 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java @@ -170,7 +170,7 @@ public class BridgeVifDriver extends VifDriverBase { for (File anInterface : interfaces) { final String fname = anInterface.getName(); s_logger.debug("matchPifFileInDirectory: file name '" + fname + "'"); - if (isInterface(fname)) { + if (LibvirtComputingResource.isInterface(fname)) { return fname; } } @@ -179,33 +179,6 @@ public class BridgeVifDriver extends VifDriverBase { return ""; } - private static final String [] IF_NAME_PATTERNS = { - "^eth", - "^bond", - "^vlan", - "^vx", - "^em", - "^ens", - "^eno", - "^enp", - "^team", - "^enx", - "^p\\d+p\\d+" - }; - - /** - * @param fname - * @return - */ - private static boolean isInterface(final String fname) { - StringBuilder commonPattern = new StringBuilder(); - for (final String ifNamePattern : IF_NAME_PATTERNS) { - commonPattern.append("|(").append(ifNamePattern).append(".*)"); - } - - return fname.matches(commonPattern.toString()); - } - protected boolean isBroadcastTypeVlanOrVxlan(final NicTO nic) { return nic != null && (nic.getBroadcastType() == Networks.BroadcastDomainType.Vlan || nic.getBroadcastType() == Networks.BroadcastDomainType.Vxlan); @@ -432,7 +405,7 @@ public class BridgeVifDriver extends VifDriverBase { } if (!foundLinkLocalBr) { Script.runSimpleBashScript("ip address add 169.254.0.1/16 dev " + linkLocalBr + ";" + "ip route add " + NetUtils.getLinkLocalCIDR() + " dev " + linkLocalBr + " src " + - NetUtils.getLinkLocalGateway()); + NetUtils.getLinkLocalGateway()); } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index cf6f610ef6f..ac92f37edab 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -26,6 +26,7 @@ import java.io.StringReader; import java.net.InetAddress; import java.net.URI; import java.net.URISyntaxException; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; @@ -278,7 +279,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv protected String _rngPath = "/dev/random"; protected int _rngRatePeriod = 1000; protected int _rngRateBytes = 2048; - private File _qemuSocketsPath; + protected File _qemuSocketsPath; private final String _qemuGuestAgentSocketName = "org.qemu.guest_agent.0"; protected WatchDogAction _watchDogAction = WatchDogAction.NONE; protected WatchDogModel _watchDogModel = WatchDogModel.I6300ESB; @@ -1308,7 +1309,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv return ""; } - String [] _ifNamePatterns = { + static String [] ifNamePatterns = { "^eth", "^bond", "^vlan", @@ -1319,15 +1320,18 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv "^enp", "^team", "^enx", + "^dummy", + "^lo", "^p\\d+p\\d+" }; + /** * @param fname * @return */ - boolean isInterface(final String fname) { + protected static boolean isInterface(final String fname) { StringBuffer commonPattern = new StringBuffer(); - for (final String ifNamePattern : _ifNamePatterns) { + for (final String ifNamePattern : ifNamePatterns) { commonPattern.append("|(").append(ifNamePattern).append(".*)"); } if(fname.matches(commonPattern.toString())) { @@ -1502,7 +1506,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } public synchronized boolean configureTunnelNetwork(final long networkId, - final long hostId, final String nwName) { + final long hostId, final String nwName) { try { final boolean findResult = findOrCreateTunnelNetwork(nwName); if (!findResult) { @@ -1532,7 +1536,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv if (result != null) { throw new CloudRuntimeException( "Unable to pre-configure OVS bridge " + nwName - + " for network ID:" + networkId); + + " for network ID:" + networkId); } } } catch (final Exception e) { @@ -2172,8 +2176,8 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv /* Add a VirtIO channel for SystemVMs for communication and provisioning */ if (vmTO.getType() != VirtualMachine.Type.User) { - devices.addDevice(new ChannelDef(vmTO.getName() + ".vport", ChannelDef.ChannelType.UNIX, - new File(_qemuSocketsPath + "/" + vmTO.getName() + ".agent"))); + File virtIoChannel = Paths.get(_qemuSocketsPath.getPath(), vmTO.getName() + ".agent").toFile(); + devices.addDevice(new ChannelDef(vmTO.getName() + ".vport", ChannelDef.ChannelType.UNIX, virtIoChannel)); } if (_rngEnable) { @@ -2182,8 +2186,8 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } /* Add a VirtIO channel for the Qemu Guest Agent tools */ - devices.addDevice(new ChannelDef(_qemuGuestAgentSocketName, ChannelDef.ChannelType.UNIX, - new File(_qemuSocketsPath + "/" + vmTO.getName() + "." + _qemuGuestAgentSocketName))); + File virtIoChannel = Paths.get(_qemuSocketsPath.getPath(), vmTO.getName() + "." + _qemuGuestAgentSocketName).toFile(); + devices.addDevice(new ChannelDef(_qemuGuestAgentSocketName, ChannelDef.ChannelType.UNIX, virtIoChannel)); devices.addDevice(new WatchDogDef(_watchDogAction, _watchDogModel)); @@ -2488,7 +2492,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } public synchronized String attachOrDetachISO(final Connect conn, final String vmName, String isoPath, final boolean isAttach, final Integer diskSeq) throws LibvirtException, URISyntaxException, - InternalErrorException { + InternalErrorException { final DiskDef iso = new DiskDef(); if (isoPath != null && isAttach) { final int index = isoPath.lastIndexOf("/"); @@ -2518,8 +2522,8 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } public synchronized String attachOrDetachDisk(final Connect conn, - final boolean attach, final String vmName, final KVMPhysicalDisk attachingDisk, - final int devId, final Long bytesReadRate, final Long bytesWriteRate, final Long iopsReadRate, final Long iopsWriteRate, final String cacheMode) throws LibvirtException, InternalErrorException { + final boolean attach, final String vmName, final KVMPhysicalDisk attachingDisk, + final int devId, final Long bytesReadRate, final Long bytesWriteRate, final Long iopsReadRate, final Long iopsWriteRate, final String cacheMode) throws LibvirtException, InternalErrorException { List disks = null; Domain dm = null; DiskDef diskdef = null; @@ -2895,9 +2899,9 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv speed = getCpuSpeed(hosts); /* - * Some CPUs report a single socket and multiple NUMA cells. - * We need to multiply them to get the correct socket count. - */ + * Some CPUs report a single socket and multiple NUMA cells. + * We need to multiply them to get the correct socket count. + */ cpuSockets = hosts.sockets; if (hosts.nodes > 0) { cpuSockets = hosts.sockets * hosts.nodes; @@ -3470,12 +3474,12 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } /** - * This method retrieves the memory statistics from the domain given as parameters. - * If no memory statistic is found, it will return {@link NumberUtils#LONG_ZERO} as the value of free memory in the domain. - * If it can retrieve the domain memory statistics, it will return the free memory statistic; that means, it returns the value at the first position of the array returned by {@link Domain#memoryStats(int)}. - * - * @return the amount of free memory in KBs - */ + * This method retrieves the memory statistics from the domain given as parameters. + * If no memory statistic is found, it will return {@link NumberUtils#LONG_ZERO} as the value of free memory in the domain. + * If it can retrieve the domain memory statistics, it will return the free memory statistic; that means, it returns the value at the first position of the array returned by {@link Domain#memoryStats(int)}. + * + * @return the amount of free memory in KBs + */ protected long getMemoryFreeInKBs(Domain dm) throws LibvirtException { MemoryStatistic[] mems = dm.memoryStats(NUMMEMSTATS); if (ArrayUtils.isEmpty(mems)) { @@ -3605,7 +3609,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } public boolean addNetworkRules(final String vmName, final String vmId, final String guestIP, final String guestIP6, final String sig, final String seq, final String mac, final String rules, final String vif, final String brname, - final String secIps) { + final String secIps) { if (!_canBridgeFirewall) { return false; } @@ -3853,7 +3857,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv public boolean isHostSecured() { // Test for host certificates final File confFile = PropertiesUtil.findConfigFile(KeyStoreUtils.AGENT_PROPSFILE); - if (confFile == null || !confFile.exists() || !new File(confFile.getParent() + "/" + KeyStoreUtils.CERT_FILENAME).exists()) { + if (confFile == null || !confFile.exists() || !Paths.get(confFile.getParent(), KeyStoreUtils.CERT_FILENAME).toFile().exists()) { return false; } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java index 97d14cfaefc..c2396e81c17 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java @@ -200,11 +200,11 @@ public class LibvirtComputingResourceTest { Random random = new Random(); /** - This test tests if the Agent can handle a vmSpec coming - from a <=4.1 management server. + This test tests if the Agent can handle a vmSpec coming + from a <=4.1 management server. - The overcommit feature has not been merged in there and thus - only 'speed' is set. + The overcommit feature has not been merged in there and thus + only 'speed' is set. */ @Test public void testCreateVMFromSpecLegacy() { @@ -222,6 +222,8 @@ public class LibvirtComputingResourceTest { final String vncPassword = "mySuperSecretPassword"; final LibvirtComputingResource lcr = new LibvirtComputingResource(); + lcr._qemuSocketsPath = new File("/var/run/qemu"); + final VirtualMachineTO to = new VirtualMachineTO(id, name, VirtualMachine.Type.User, cpus, speed, minRam, maxRam, BootloaderType.HVM, os, false, false, vncPassword); to.setVncAddr(vncAddr); to.setUuid("b0f0a72d-7efb-3cad-a8ff-70ebf30b3af9"); @@ -233,7 +235,7 @@ public class LibvirtComputingResourceTest { } /** - This test verifies that CPU topology is properly set for hex-core + This test verifies that CPU topology is properly set for hex-core */ @Test public void testCreateVMFromSpecWithTopology6() { @@ -252,6 +254,8 @@ public class LibvirtComputingResourceTest { final String vncPassword = "mySuperSecretPassword"; final LibvirtComputingResource lcr = new LibvirtComputingResource(); + lcr._qemuSocketsPath = new File("/var/run/qemu"); + final VirtualMachineTO to = new VirtualMachineTO(id, name, VirtualMachine.Type.User, cpus, minSpeed, maxSpeed, minRam, maxRam, BootloaderType.HVM, os, false, false, vncPassword); to.setVncAddr(vncAddr); to.setUuid("b0f0a72d-7efb-3cad-a8ff-70ebf30b3af9"); @@ -263,7 +267,7 @@ public class LibvirtComputingResourceTest { } /** - This test verifies that CPU topology is properly set for quad-core + This test verifies that CPU topology is properly set for quad-core */ @Test public void testCreateVMFromSpecWithTopology4() { @@ -282,6 +286,8 @@ public class LibvirtComputingResourceTest { final String vncPassword = "mySuperSecretPassword"; final LibvirtComputingResource lcr = new LibvirtComputingResource(); + lcr._qemuSocketsPath = new File("/var/run/qemu"); + final VirtualMachineTO to = new VirtualMachineTO(id, name, VirtualMachine.Type.User, cpus, minSpeed, maxSpeed, minRam, maxRam, BootloaderType.HVM, os, false, false, vncPassword); to.setVncAddr(vncAddr); to.setUuid("b0f0a72d-7efb-3cad-a8ff-70ebf30b3af9"); @@ -293,11 +299,11 @@ public class LibvirtComputingResourceTest { } /** - This test tests if the Agent can handle a vmSpec coming - from a >4.1 management server. + This test tests if the Agent can handle a vmSpec coming + from a >4.1 management server. - It tests if the Agent can handle a vmSpec with overcommit - data like minSpeed and maxSpeed in there + It tests if the Agent can handle a vmSpec with overcommit + data like minSpeed and maxSpeed in there */ @Test public void testCreateVMFromSpec() { @@ -316,6 +322,8 @@ public class LibvirtComputingResourceTest { final String vncPassword = "mySuperSecretPassword"; final LibvirtComputingResource lcr = new LibvirtComputingResource(); + lcr._qemuSocketsPath = new File("/var/run/qemu"); + final VirtualMachineTO to = new VirtualMachineTO(id, name, VirtualMachine.Type.User, cpus, minSpeed, maxSpeed, minRam, maxRam, BootloaderType.HVM, os, false, false, vncPassword); to.setVncAddr(vncAddr); @@ -359,7 +367,7 @@ public class LibvirtComputingResourceTest { Calling configure is also not possible since that looks for certain files on the system which are not present during testing */ - assertXpath(domainDoc, "/domain/devices/channel/source/@path", "null/" + to.getName() + ".org.qemu.guest_agent.0"); + assertXpath(domainDoc, "/domain/devices/channel/source/@path", "/var/run/qemu/" + to.getName() + ".org.qemu.guest_agent.0"); assertXpath(domainDoc, "/domain/devices/channel/target/@name", "org.qemu.guest_agent.0"); assertXpath(domainDoc, "/domain/memory/text()", String.valueOf( to.getMaxRam() / 1024 )); @@ -402,7 +410,7 @@ public class LibvirtComputingResourceTest { } static void assertXpath(final Document doc, final String xPathExpr, - final String expected) { + final String expected) { try { Assert.assertEquals(expected, XPathFactory.newInstance().newXPath() .evaluate(xPathExpr, doc)); @@ -5118,7 +5126,13 @@ public class LibvirtComputingResourceTest { final LibvirtComputingResource lvcr = new LibvirtComputingResource(); assertFalse(lvcr.isInterface("bla")); assertTrue(lvcr.isInterface("p99p00")); - for (final String ifNamePattern : lvcr._ifNamePatterns) { + assertTrue(lvcr.isInterface("lo1")); + assertTrue(lvcr.isInterface("lo_11")); + assertTrue(lvcr.isInterface("lo_public_1")); + assertTrue(lvcr.isInterface("dummy0")); + assertTrue(lvcr.isInterface("dummy_0")); + assertTrue(lvcr.isInterface("dummy_private_0")); + for (final String ifNamePattern : lvcr.ifNamePatterns) { // excluding regexps as "\\\\d+" won't replace with String.replaceAll(String,String); if (!ifNamePattern.contains("\\")) { final String ifName = ifNamePattern.replaceFirst("\\^", "") + "0";