You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2018/12/06 20:29:02 UTC

[GitHub] rhtyd closed pull request #3000: CLOUDSTACK-2998: Added two more device name patterns to valid bridge slaves (lo* and dummy*)

rhtyd closed pull request #3000: CLOUDSTACK-2998: Added two more device name patterns to valid bridge slaves (lo* and dummy*)
URL: https://github.com/apache/cloudstack/pull/3000
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

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 @@ private String matchPifFileInDirectory(final String bridgeName) {
         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 @@ private String matchPifFileInDirectory(final String bridgeName) {
         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 @@ private void deleteExistingLinkLocalRouteTable(String linkLocalBr) {
         }
         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 26fcd01f63f..3b61337164a 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.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 @@
     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 @@ private String matchPifFileInDirectory(final String bridgeName) {
         return "";
     }
 
-    String [] _ifNamePatterns = {
+    static String [] ifNamePatterns = {
             "^eth",
             "^bond",
             "^vlan",
@@ -1319,15 +1320,18 @@ private String matchPifFileInDirectory(final String bridgeName) {
             "^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 synchronized boolean findOrCreateTunnelNetwork(final String nwName) {
     }
 
     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 synchronized boolean configureTunnelNetwork(final long networkId,
                 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 @@ So if getMinSpeed() returns null we fall back to getSpeed().
 
         /* 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 @@ So if getMinSpeed() returns null we fall back to getSpeed().
         }
 
         /* 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 @@ protected KVMStoragePoolManager getPoolManager() {
     }
 
     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 synchronized String attachOrDetachISO(final Connect conn, final String vm
     }
 
     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<DiskDef> disks = null;
         Domain dm = null;
         DiskDef diskdef = null;
@@ -2895,9 +2899,9 @@ private String getIqn() {
             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 VmStatsEntry getVmStat(final Connect conn, final String vmName) throws Li
     }
 
     /**
-    * 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 boolean configureDefaultNetworkRulesForSystemVm(final Connect conn, final
     }
 
     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 String getHostDistro() {
     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 @@
     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 void testCreateVMFromSpecLegacy() {
         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 void testCreateVMFromSpecLegacy() {
     }
 
     /**
-        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 void testCreateVMFromSpecWithTopology6() {
         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 void testCreateVMFromSpecWithTopology6() {
     }
 
     /**
-        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 void testCreateVMFromSpecWithTopology4() {
         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 void testCreateVMFromSpecWithTopology4() {
     }
 
     /**
-        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 void testCreateVMFromSpec() {
         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 @@ The configure() method of LibvirtComputingResource has not been called, so the d
            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 @@ static void assertNodeExists(final Document doc, final String xPathExpr) {
     }
 
     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 void testIsInterface () {
         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";


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services