This is an automated email from the ASF dual-hosted git repository.

dahn pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git

commit 085bd3bda5fdc3926e78f029e93debe77dc9dfcc
Merge: 0b5a5e8043d 0d5047b8b73
Author: Daan Hoogland <d...@onecht.net>
AuthorDate: Sat Feb 1 17:51:50 2025 +0100

    Merge branch '4.19' into 4.20

 .../user/firewall/CreateFirewallRuleCmd.java       |   7 +-
 .../user/firewall/CreateFirewallRuleCmdTest.java   |  91 ++++++++
 .../service/NetworkOrchestrationService.java       |   3 +
 .../engine/orchestration/NetworkOrchestrator.java  |   2 +-
 .../main/java/com/cloud/host/dao/HostDaoImpl.java  | 237 ++++++++++-----------
 .../java/com/cloud/network/NetworkServiceImpl.java |   4 +
 .../java/com/cloud/network/vpc/VpcManagerImpl.java |   4 +-
 .../com/cloud/server/ManagementServerImpl.java     |   4 +-
 .../java/com/cloud/vm/FirstFitPlannerTest.java     |   2 -
 ui/src/views/network/FirewallRules.vue             |   3 +
 10 files changed, 227 insertions(+), 130 deletions(-)

diff --cc 
api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java
index 56c818f832b,0809c935c4b..efccb5c09b0
--- 
a/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java
+++ 
b/api/src/main/java/org/apache/cloudstack/api/command/user/firewall/CreateFirewallRuleCmd.java
@@@ -19,6 -19,9 +19,7 @@@ package org.apache.cloudstack.api.comma
  import java.util.ArrayList;
  import java.util.List;
  
+ import org.apache.commons.collections.CollectionUtils;
 -import org.apache.commons.lang3.StringUtils;
 -import org.apache.log4j.Logger;
  
  import org.apache.cloudstack.acl.RoleType;
  import org.apache.cloudstack.api.APICommand;
@@@ -40,6 -43,6 +41,7 @@@ import com.cloud.exception.ResourceUnav
  import com.cloud.network.IpAddress;
  import com.cloud.network.rules.FirewallRule;
  import com.cloud.user.Account;
++import com.cloud.utils.StringUtils;
  import com.cloud.utils.net.NetUtils;
  
  @APICommand(name = "createFirewallRule", description = "Creates a firewall 
rule for a given IP address", responseObject = FirewallResponse.class, 
entityType = {FirewallRule.class},
@@@ -103,32 -103,12 +105,32 @@@ public class CreateFirewallRuleCmd exte
          return protocol.trim();
      }
  
 +    public void setProtocol(String protocol) {
 +        this.protocol = protocol;
 +    }
 +
 +    public Integer getPublicStartPort() {
 +        return publicStartPort;
 +    }
 +
 +    public void setPublicStartPort(Integer publicStartPort) {
 +        this.publicStartPort = publicStartPort;
 +    }
 +
 +    public Integer getPublicEndPort() {
 +        return publicEndPort;
 +    }
 +
 +    public void setPublicEndPort(Integer publicEndPort) {
 +        this.publicEndPort = publicEndPort;
 +    }
 +
      @Override
      public List<String> getSourceCidrList() {
-         if (cidrlist != null) {
+         if (CollectionUtils.isNotEmpty(cidrlist) && !(cidrlist.size() == 1 && 
StringUtils.isBlank(cidrlist.get(0)))) {
              return cidrlist;
          } else {
-             List<String> oneCidrList = new ArrayList<String>();
+             List<String> oneCidrList = new ArrayList<>();
              oneCidrList.add(NetUtils.ALL_IP4_CIDRS);
              return oneCidrList;
          }
diff --cc 
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
index 7efc29b02a6,26b63d2d728..5106fd824c7
--- 
a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
+++ 
b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
@@@ -4872,9 -4792,9 +4872,9 @@@ public class NetworkOrchestrator extend
  
      @Override
      public ConfigKey<?>[] getConfigKeys() {
-         return new ConfigKey<?>[]{NetworkGcWait, NetworkGcInterval, 
NetworkLockTimeout,
+         return new ConfigKey<?>[]{NetworkGcWait, NetworkGcInterval, 
NetworkLockTimeout, DeniedRoutes,
                  GuestDomainSuffix, NetworkThrottlingRate, MinVRVersion,
                  PromiscuousMode, MacAddressChanges, ForgedTransmits, 
MacLearning, RollingRestartEnabled,
 -                TUNGSTEN_ENABLED };
 +                TUNGSTEN_ENABLED, NSX_ENABLED };
      }
  }
diff --cc engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java
index e5584239a32,4f0ae3cce9c..94a16497e87
--- a/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java
+++ b/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java
@@@ -60,7 -60,7 +60,8 @@@ import com.cloud.org.Grouping
  import com.cloud.org.Managed;
  import com.cloud.resource.ResourceState;
  import com.cloud.utils.DateUtil;
 +import com.cloud.utils.Pair;
+ import com.cloud.utils.StringUtils;
  import com.cloud.utils.db.Attribute;
  import com.cloud.utils.db.DB;
  import com.cloud.utils.db.Filter;
@@@ -79,10 -80,13 +80,10 @@@ import com.cloud.utils.exception.CloudR
  @DB
  @TableGenerator(name = "host_req_sq", table = "op_host", pkColumnName = "id", 
valueColumnName = "sequence", allocationSize = 1)
  public class HostDaoImpl extends GenericDaoBase<HostVO, Long> implements 
HostDao { //FIXME: , ExternalIdDao {
 -    private static final Logger s_logger = 
Logger.getLogger(HostDaoImpl.class);
 -    private static final Logger status_logger = 
Logger.getLogger(Status.class);
 -    private static final Logger state_logger = 
Logger.getLogger(ResourceState.class);
  
-     private static final String LIST_HOST_IDS_BY_COMPUTETAGS = "SELECT 
filtered.host_id, COUNT(filtered.tag) AS tag_count "
-                                                              + "FROM (SELECT 
host_id, tag, is_tag_a_rule FROM host_tags GROUP BY host_id,tag) AS filtered "
-                                                              + "WHERE tag 
IN(%s) AND is_tag_a_rule = 0 "
+     private static final String LIST_HOST_IDS_BY_HOST_TAGS = "SELECT 
filtered.host_id, COUNT(filtered.tag) AS tag_count "
+                                                              + "FROM (SELECT 
host_id, tag, is_tag_a_rule FROM host_tags GROUP BY host_id,tag,is_tag_a_rule) 
AS filtered "
+                                                              + "WHERE tag IN 
(%s) AND (is_tag_a_rule = 0 OR is_tag_a_rule IS NULL) "
                                                               + "GROUP BY 
host_id "
                                                               + "HAVING 
tag_count = %s ";
      private static final String SEPARATOR = ",";
@@@ -625,9 -595,9 +626,7 @@@
              sb.append(" ");
          }
  
-         if (logger.isTraceEnabled()) {
-             logger.trace("Following hosts got reset: " + sb.toString());
 -        if (s_logger.isTraceEnabled()) {
 -            s_logger.trace("Following hosts got reset: " + sb.toString());
--        }
++        logger.trace("Following hosts got reset: {}", sb);
      }
  
      /*
@@@ -637,8 -607,8 +636,7 @@@
          SearchCriteria<Long> sc = ClustersOwnedByMSSearch.create();
          sc.setParameters("server", managementServerId);
  
--        List<Long> clusters = customSearch(sc, null);
--        return clusters;
++        return customSearch(sc, null);
      }
  
      /*
@@@ -648,13 -618,13 +646,11 @@@
          SearchCriteria<Long> sc = 
ClustersForHostsNotOwnedByAnyMSSearch.create();
          sc.setJoinParameters("ClusterManagedSearch", "managed", 
Managed.ManagedState.Managed);
  
--        List<Long> clusters = customSearch(sc, null);
--        return clusters;
++        return customSearch(sc, null);
      }
  
      /**
       * This determines if hosts belonging to cluster(@clusterId) are up for 
grabs
--     *
       * This is used for handling following cases:
       * 1. First host added in cluster
       * 2. During MS restart all hosts in a cluster are without any MS
@@@ -664,9 -634,9 +660,7 @@@
          sc.setParameters("cluster", clusterId);
  
          List<HostVO> hosts = search(sc, null);
--        boolean ownCluster = (hosts == null || hosts.size() == 0);
--
--        return ownCluster;
++        return (hosts == null || hosts.isEmpty());
      }
  
      @Override
@@@ -679,18 -649,18 +673,18 @@@
          }
          // reset hosts that are suitable candidates for reconnect
          resetHosts(managementServerId, lastPingSecondsAfter);
 -        if (s_logger.isDebugEnabled()) {
 -            s_logger.debug("Completed resetting hosts suitable for 
reconnect");
 +        if (logger.isDebugEnabled()) {
 +            logger.debug("Completed resetting hosts suitable for reconnect");
          }
  
--        List<HostVO> assignedHosts = new ArrayList<HostVO>();
++        List<HostVO> assignedHosts = new ArrayList<>();
  
 -        if (s_logger.isDebugEnabled()) {
 -            s_logger.debug("Acquiring hosts for clusters already owned by 
this management server");
 +        if (logger.isDebugEnabled()) {
 +            logger.debug("Acquiring hosts for clusters already owned by this 
management server");
          }
          List<Long> clusters = 
findClustersOwnedByManagementServer(managementServerId);
          txn.start();
--        if (clusters.size() > 0) {
++        if (!clusters.isEmpty()) {
              // handle clusters already owned by @managementServerId
              SearchCriteria<HostVO> sc = UnmanagedDirectConnectSearch.create();
              sc.setParameters("lastPinged", lastPingSecondsAfter);
@@@ -705,17 -675,17 +699,13 @@@
                  sb.append(host.getId());
                  sb.append(" ");
              }
-             if (logger.isTraceEnabled()) {
-                 logger.trace("Following hosts got acquired for clusters 
already owned: " + sb.toString());
 -            if (s_logger.isTraceEnabled()) {
 -                s_logger.trace("Following hosts got acquired for clusters 
already owned: " + sb.toString());
--            }
--        }
-         if (logger.isDebugEnabled()) {
-             logger.debug("Completed acquiring hosts for clusters already 
owned by this management server");
 -        if (s_logger.isDebugEnabled()) {
 -            s_logger.debug("Completed acquiring hosts for clusters already 
owned by this management server");
++            logger.trace("Following hosts got acquired for clusters already 
owned: {}", sb);
          }
++        logger.debug("Completed acquiring hosts for clusters already owned by 
this management server");
  
          if (assignedHosts.size() < limit) {
 -            if (s_logger.isDebugEnabled()) {
 -                s_logger.debug("Acquiring hosts for clusters not owned by any 
management server");
 +            if (logger.isDebugEnabled()) {
 +                logger.debug("Acquiring hosts for clusters not owned by any 
management server");
              }
              // for remaining hosts not owned by any MS check if they can be 
owned (by owning full cluster)
              clusters = findClustersForHostsNotOwnedByAnyManagementServer();
@@@ -723,7 -693,7 +713,7 @@@
              if (clusters.size() > limit) {
                  updatedClusters = clusters.subList(0, limit.intValue());
              }
--            if (updatedClusters.size() > 0) {
++            if (!updatedClusters.isEmpty()) {
                  SearchCriteria<HostVO> sc = 
UnmanagedDirectConnectSearch.create();
                  sc.setParameters("lastPinged", lastPingSecondsAfter);
                  sc.setJoinParameters("ClusterManagedSearch", "managed", 
Managed.ManagedState.Managed);
@@@ -731,10 -701,10 +721,10 @@@
                  List<HostVO> unmanagedHosts = lockRows(sc, null, true);
  
                  // group hosts based on cluster
--                Map<Long, List<HostVO>> hostMap = new HashMap<Long, 
List<HostVO>>();
++                Map<Long, List<HostVO>> hostMap = new HashMap<>();
                  for (HostVO host : unmanagedHosts) {
                      if (hostMap.get(host.getClusterId()) == null) {
--                        hostMap.put(host.getClusterId(), new 
ArrayList<HostVO>());
++                        hostMap.put(host.getClusterId(), new ArrayList<>());
                      }
                      hostMap.get(host.getClusterId()).add(host);
                  }
@@@ -755,13 -725,13 +745,9 @@@
                          break;
                      }
                  }
-                 if (logger.isTraceEnabled()) {
-                     logger.trace("Following hosts got acquired from newly 
owned clusters: " + sb.toString());
 -                if (s_logger.isTraceEnabled()) {
 -                    s_logger.trace("Following hosts got acquired from newly 
owned clusters: " + sb.toString());
--                }
--            }
-             if (logger.isDebugEnabled()) {
-                 logger.debug("Completed acquiring hosts for clusters not 
owned by any management server");
 -            if (s_logger.isDebugEnabled()) {
 -                s_logger.debug("Completed acquiring hosts for clusters not 
owned by any management server");
++                logger.trace("Following hosts got acquired from newly owned 
clusters: {}", sb);
              }
++            logger.debug("Completed acquiring hosts for clusters not owned by 
any management server");
          }
          txn.commit();
  
@@@ -815,7 -785,16 +801,16 @@@
      }
  
      @Override
 -    public List<HostVO> listByHostTag(Host.Type type, Long clusterId, Long 
podId, long dcId, String hostTag) {
 +    public List<HostVO> listByHostTag(Host.Type type, Long clusterId, Long 
podId, Long dcId, String hostTag) {
+         return listHostsWithOrWithoutHostTags(type, clusterId, podId, dcId, 
hostTag, true);
+     }
+ 
 -    private List<HostVO> listHostsWithOrWithoutHostTags(Host.Type type, Long 
clusterId, Long podId, long dcId, String hostTags, boolean withHostTags) {
++    private List<HostVO> listHostsWithOrWithoutHostTags(Host.Type type, Long 
clusterId, Long podId, Long dcId, String hostTags, boolean withHostTags) {
+         if (StringUtils.isEmpty(hostTags)) {
 -            s_logger.debug("Host tags not specified, to list hosts");
 -            return new ArrayList();
++            logger.debug("Host tags not specified, to list hosts");
++            return new ArrayList<>();
+         }
+ 
          SearchBuilder<HostVO> hostSearch = createSearchBuilder();
          HostVO entity = hostSearch.entity();
          hostSearch.and("type", entity.getType(), SearchCriteria.Op.EQ);
@@@ -839,13 -818,25 +836,25 @@@
          sc.setParameters("status", Status.Up.toString());
          sc.setParameters("resourceState", ResourceState.Enabled.toString());
  
-         List<HostVO> tmpHosts = listBy(sc);
-         List<HostVO> correctHostsByHostTags = new ArrayList();
-         List<Long> hostIdsByComputeOffTags = 
findHostByComputeOfferings(hostTag);
+         List<HostVO> upAndEnabledHosts = listBy(sc);
+         if (CollectionUtils.isEmpty(upAndEnabledHosts)) {
 -            return new ArrayList();
++            return new ArrayList<>();
+         }
  
-         tmpHosts.forEach((host) -> { 
if(hostIdsByComputeOffTags.contains(host.getId())) 
correctHostsByHostTags.add(host);});
+         List<Long> hostIdsByHostTags = findHostIdsByHostTags(hostTags);
+         if (CollectionUtils.isEmpty(hostIdsByHostTags)) {
 -            return withHostTags ? new ArrayList() : upAndEnabledHosts;
++            return withHostTags ? new ArrayList<>() : upAndEnabledHosts;
+         }
  
-         return correctHostsByHostTags;
+         if (withHostTags) {
 -            List<HostVO> upAndEnabledHostsWithHostTags = new ArrayList();
++            List<HostVO> upAndEnabledHostsWithHostTags = new ArrayList<>();
+             upAndEnabledHosts.forEach((host) -> { if 
(hostIdsByHostTags.contains(host.getId())) 
upAndEnabledHostsWithHostTags.add(host);});
+             return upAndEnabledHostsWithHostTags;
+         } else {
 -            List<HostVO> upAndEnabledHostsWithoutHostTags = new ArrayList();
++            List<HostVO> upAndEnabledHostsWithoutHostTags = new ArrayList<>();
+             upAndEnabledHosts.forEach((host) -> { if 
(!hostIdsByHostTags.contains(host.getId())) 
upAndEnabledHostsWithoutHostTags.add(host);});
+             return upAndEnabledHostsWithoutHostTags;
+         }
      }
  
      @Override
@@@ -921,12 -905,12 +923,12 @@@
      @DB
      @Override
      public List<HostVO> findLostHosts(long timeout) {
--        List<HostVO> result = new ArrayList<HostVO>();
++        List<HostVO> result = new ArrayList<>();
          String sql = "select h.id from host h left join  cluster c on 
h.cluster_id=c.id where h.mgmt_server_id is not null and h.last_ping < ? and 
h.status in ('Up', 'Updating', 'Disconnected', 'Connecting') and h.type not in 
('ExternalFirewall', 'ExternalLoadBalancer', 'TrafficMonitor', 
'SecondaryStorage', 'LocalSecondaryStorage', 'L2Networking') and (h.cluster_id 
is null or c.managed_state = 'Managed') ;";
          try (TransactionLegacy txn = TransactionLegacy.currentTxn();
--                PreparedStatement pstmt = txn.prepareStatement(sql);) {
++                PreparedStatement pstmt = txn.prepareStatement(sql)) {
              pstmt.setLong(1, timeout);
--            try (ResultSet rs = pstmt.executeQuery();) {
++            try (ResultSet rs = pstmt.executeQuery()) {
                  while (rs.next()) {
                      long id = rs.getLong(1); //ID column
                      result.add(findById(id));
@@@ -959,7 -943,7 +961,7 @@@
          HashMap<String, HashMap<String, VgpuTypesInfo>> groupDetails = 
host.getGpuGroupDetails();
          if (groupDetails != null) {
              // Create/Update GPU group entries
--            _hostGpuGroupsDao.persist(host.getId(), new 
ArrayList<String>(groupDetails.keySet()));
++            _hostGpuGroupsDao.persist(host.getId(), new 
ArrayList<>(groupDetails.keySet()));
              // Create/Update VGPU types entries
              _vgpuTypesDao.persist(host.getId(), groupDetails);
          }
@@@ -1002,7 -986,7 +1004,7 @@@
  
          boolean persisted = super.update(hostId, host);
          if (!persisted) {
--            return persisted;
++            return false;
          }
  
          saveDetails(host);
@@@ -1011,7 -995,7 +1013,7 @@@
  
          txn.commit();
  
--        return persisted;
++        return true;
      }
  
      @Override
@@@ -1022,11 -1006,11 +1024,10 @@@
                  + "select h.data_center_id, h.type, count(*) as count from 
host as h INNER JOIN mshost as m ON h.mgmt_server_id=m.msid "
                  + "where h.status='Up' and h.type='Routing' and m.last_update 
> ? " + "group by h.data_center_id, h.type) as t " + "ORDER by 
t.data_center_id, t.type";
  
--        ArrayList<RunningHostCountInfo> l = new 
ArrayList<RunningHostCountInfo>();
++        ArrayList<RunningHostCountInfo> l = new ArrayList<>();
  
          TransactionLegacy txn = TransactionLegacy.currentTxn();
--        ;
--        PreparedStatement pstmt = null;
++        PreparedStatement pstmt;
          try {
              pstmt = txn.prepareAutoCloseStatement(sql);
              String gmtCutTime = 
DateUtil.getDateDisplayString(TimeZone.getTimeZone("GMT"), cutTime);
@@@ -1050,9 -1034,9 +1051,7 @@@
  
      @Override
      public long getNextSequence(long hostId) {
-         if (logger.isTraceEnabled()) {
-             logger.trace("getNextSequence(), hostId: " + hostId);
 -        if (s_logger.isTraceEnabled()) {
 -            s_logger.trace("getNextSequence(), hostId: " + hostId);
--        }
++        logger.trace("getNextSequence(), hostId: {}", hostId);
  
          TableGenerator tg = _tgs.get("host_req_sq");
          assert tg != null : "how can this be wrong!";
@@@ -1121,31 -1105,31 +1120,30 @@@
              HostVO ho = findById(host.getId());
              assert ho != null : "How how how? : " + host.getId();
  
 -            if (status_logger.isDebugEnabled()) {
 -
 -                StringBuilder str = new StringBuilder("Unable to update host 
for event:").append(event.toString());
 -                str.append(". Name=").append(host.getName());
 -                str.append("; 
New=[status=").append(newStatus.toString()).append(":msid=").append(newStatus.lostConnection()
 ? "null" : host.getManagementServerId())
 -                
.append(":lastpinged=").append(host.getLastPinged()).append("]");
 -                str.append("; 
Old=[status=").append(oldStatus.toString()).append(":msid=").append(host.getManagementServerId()).append(":lastpinged=").append(oldPingTime)
 -                .append("]");
 -                str.append("; 
DB=[status=").append(vo.getStatus().toString()).append(":msid=").append(vo.getManagementServerId()).append(":lastpinged=").append(vo.getLastPinged())
 -                .append(":old update 
count=").append(oldUpdateCount).append("]");
 -                status_logger.debug(str.toString());
++            // TODO handle this if(debug){}else{log.debug} it makes no sense
 +            if (logger.isDebugEnabled()) {
- 
-                 StringBuilder str = new StringBuilder("Unable to update host 
for event:").append(event.toString());
-                 str.append(". Name=").append(host.getName());
-                 str.append("; 
New=[status=").append(newStatus.toString()).append(":msid=").append(newStatus.lostConnection()
 ? "null" : host.getManagementServerId())
-                 
.append(":lastpinged=").append(host.getLastPinged()).append("]");
-                 str.append("; 
Old=[status=").append(oldStatus.toString()).append(":msid=").append(host.getManagementServerId()).append(":lastpinged=").append(oldPingTime)
-                 .append("]");
-                 str.append("; 
DB=[status=").append(vo.getStatus().toString()).append(":msid=").append(vo.getManagementServerId()).append(":lastpinged=").append(vo.getLastPinged())
-                 .append(":old update 
count=").append(oldUpdateCount).append("]");
-                 logger.debug(str.toString());
++                String str = "Unable to update host for event:" + event +
++                        ". Name=" + host.getName() +
++                        "; New=[status=" + newStatus + ":msid=" + 
(newStatus.lostConnection() ? "null" : host.getManagementServerId()) +
++                        ":lastpinged=" + host.getLastPinged() + "]" +
++                        "; Old=[status=" + oldStatus.toString() + ":msid=" + 
host.getManagementServerId() + ":lastpinged=" + oldPingTime +
++                        "]" +
++                        "; DB=[status=" + vo.getStatus().toString() + 
":msid=" + vo.getManagementServerId() + ":lastpinged=" + vo.getLastPinged() +
++                        ":old update count=" + oldUpdateCount + "]";
++                logger.debug(str);
              } else {
--                StringBuilder msg = new StringBuilder("Agent status update: 
[");
--                msg.append("id = " + host.getId());
--                msg.append("; name = " + host.getName());
--                msg.append("; old status = " + oldStatus);
--                msg.append("; event = " + event);
--                msg.append("; new status = " + newStatus);
--                msg.append("; old update count = " + oldUpdateCount);
--                msg.append("; new update count = " + newUpdateCount + "]");
-                 logger.debug(msg.toString());
 -                status_logger.debug(msg.toString());
++                String msg = "Agent status update: [" + "id = " + 
host.getId() +
++                        "; name = " + host.getName() +
++                        "; old status = " + oldStatus +
++                        "; event = " + event +
++                        "; new status = " + newStatus +
++                        "; old update count = " + oldUpdateCount +
++                        "; new update count = " + newUpdateCount + "]";
++                logger.debug(msg);
              }
  
              if (ho.getState() == newStatus) {
-                 logger.debug("Host " + ho.getName() + " state has already 
been updated to " + newStatus);
 -                status_logger.debug("Host " + ho.getName() + " state has 
already been updated to " + newStatus);
++                logger.debug("Host {} state has already been updated to {}", 
ho.getName(), newStatus);
                  return true;
              }
          }
@@@ -1171,25 -1155,25 +1169,24 @@@
          int result = update(ub, sc, null);
          assert result <= 1 : "How can this update " + result + " rows? ";
  
 -        if (state_logger.isDebugEnabled() && result == 0) {
++        // TODO handle this if(debug){}else{log.debug} it makes no sense
 +        if (logger.isDebugEnabled() && result == 0) {
              HostVO ho = findById(host.getId());
              assert ho != null : "How how how? : " + host.getId();
  
--            StringBuilder str = new StringBuilder("Unable to update resource 
state: [");
--            str.append("m = " + host.getId());
--            str.append("; name = " + host.getName());
--            str.append("; old state = " + oldState);
--            str.append("; event = " + event);
--            str.append("; new state = " + newState + "]");
-             logger.debug(str.toString());
 -            state_logger.debug(str.toString());
++            String str = "Unable to update resource state: [" + "m = " + 
host.getId() +
++                    "; name = " + host.getName() +
++                    "; old state = " + oldState +
++                    "; event = " + event +
++                    "; new state = " + newState + "]";
++            logger.debug(str);
          } else {
--            StringBuilder msg = new StringBuilder("Resource state update: [");
--            msg.append("id = " + host.getId());
--            msg.append("; name = " + host.getName());
--            msg.append("; old state = " + oldState);
--            msg.append("; event = " + event);
--            msg.append("; new state = " + newState + "]");
-             logger.debug(msg.toString());
 -            state_logger.debug(msg.toString());
++            String msg = "Resource state update: [" + "id = " + host.getId() +
++                    "; name = " + host.getName() +
++                    "; old state = " + oldState +
++                    "; event = " + event +
++                    "; new state = " + newState + "]";
++            logger.debug(msg);
          }
  
          return result > 0;
@@@ -1364,7 -1272,7 +1361,7 @@@
          Filter orderByFilter = new Filter(HostVO.class, "created", true, 
null, null);
  
          List<HostVO> hosts = search(sc, orderByFilter, null, false);
--        if (hosts != null && hosts.size() > 0) {
++        if (hosts != null && !hosts.isEmpty()) {
              return hosts.get(0);
          }
  
@@@ -1407,19 -1317,19 +1404,19 @@@
      }
  
      @Override
-     public List<Long> listClustersByHostTag(String computeOfferingTags) {
+     public List<Long> listClustersByHostTag(String hostTags) {
          TransactionLegacy txn = TransactionLegacy.currentTxn();
-         String sql = this.LIST_CLUSTERID_FOR_HOST_TAG;
 -        String selectStmtToListClusterIdsByHostTags = 
this.LIST_CLUSTER_IDS_FOR_HOST_TAGS;
--        PreparedStatement pstmt = null;
--        List<Long> result = new ArrayList();
-         List<String> tags = 
Arrays.asList(computeOfferingTags.split(this.SEPARATOR));
-         String subselect = getHostIdsByComputeTags(tags);
-         sql = String.format(sql, subselect);
 -        List<String> tags = Arrays.asList(hostTags.split(this.SEPARATOR));
++        String selectStmtToListClusterIdsByHostTags = 
LIST_CLUSTER_IDS_FOR_HOST_TAGS;
++        PreparedStatement pstmt;
++        List<Long> result = new ArrayList<>();
++        List<String> tags = Arrays.asList(hostTags.split(SEPARATOR));
+         String selectStmtToListHostIdsByHostTags = 
getSelectStmtToListHostIdsByHostTags(tags);
+         selectStmtToListClusterIdsByHostTags = 
String.format(selectStmtToListClusterIdsByHostTags, 
selectStmtToListHostIdsByHostTags);
  
          try {
-             pstmt = txn.prepareStatement(sql);
+             pstmt = 
txn.prepareStatement(selectStmtToListClusterIdsByHostTags);
  
-             for(int i = 0; i < tags.size(); i++){
+             for (int i = 0; i < tags.size(); i++){
                  pstmt.setString(i+1, tags.get(i));
              }
  
@@@ -1434,16 -1344,16 +1431,16 @@@
          }
      }
  
-     private List<Long> findHostByComputeOfferings(String computeOfferingTags){
+     private List<Long> findHostIdsByHostTags(String hostTags){
          TransactionLegacy txn = TransactionLegacy.currentTxn();
--        PreparedStatement pstmt = null;
--        List<Long> result = new ArrayList();
-         List<String> tags = 
Arrays.asList(computeOfferingTags.split(this.SEPARATOR));
-         String select = getHostIdsByComputeTags(tags);
 -        List<String> tags = Arrays.asList(hostTags.split(this.SEPARATOR));
++        PreparedStatement pstmt;
++        List<Long> result = new ArrayList<>();
++        List<String> tags = Arrays.asList(hostTags.split(SEPARATOR));
+         String selectStmtToListHostIdsByHostTags = 
getSelectStmtToListHostIdsByHostTags(tags);
          try {
-             pstmt = txn.prepareStatement(select);
+             pstmt = txn.prepareStatement(selectStmtToListHostIdsByHostTags);
  
-             for(int i = 0; i < tags.size(); i++){
+             for (int i = 0; i < tags.size(); i++){
                  pstmt.setString(i+1, tags.get(i));
              }
  
@@@ -1479,41 -1389,16 +1476,41 @@@
          return new ArrayList<>(result);
      }
  
 +    @Override
 +    public List<Long> listSsvmHostsWithPendingMigrateJobsOrderedByJobCount() {
 +        String query = "SELECT cel.host_id, COUNT(*) " +
 +                "FROM cmd_exec_log cel " +
 +                "JOIN host h ON cel.host_id = h.id " +
 +                "WHERE h.removed IS NULL " +
 +                "GROUP BY cel.host_id " +
 +                "ORDER BY 2";
 +
 +        TransactionLegacy txn = TransactionLegacy.currentTxn();
 +        List<Long> result = new ArrayList<>();
 +
 +        PreparedStatement pstmt;
 +        try {
 +            pstmt = txn.prepareAutoCloseStatement(query);
 +            ResultSet rs = pstmt.executeQuery();
 +            while (rs.next()) {
 +                result.add((long) rs.getInt(1));
 +            }
 +        } catch (SQLException e) {
 +            logger.warn("SQLException caught while listing SSVMs with least 
migrate jobs.", e);
 +        }
 +        return result;
 +    }
 +
-     private String getHostIdsByComputeTags(List<String> offeringTags){
-         List<String> questionMarks = new ArrayList();
-         offeringTags.forEach((tag) -> { questionMarks.add("?"); });
-         return String.format(this.LIST_HOST_IDS_BY_COMPUTETAGS, 
String.join(",", questionMarks),questionMarks.size());
+     private String getSelectStmtToListHostIdsByHostTags(List<String> 
hostTags){
 -        List<String> questionMarks = new ArrayList();
 -        hostTags.forEach((tag) -> { questionMarks.add("?"); });
 -        return String.format(this.LIST_HOST_IDS_BY_HOST_TAGS, 
String.join(",", questionMarks), questionMarks.size());
++        List<String> questionMarks = new ArrayList<>();
++        hostTags.forEach((tag) -> questionMarks.add("?"));
++        return String.format(LIST_HOST_IDS_BY_HOST_TAGS, 
String.join(SEPARATOR, questionMarks), questionMarks.size());
      }
  
      @Override
      public List<HostVO> listHostsWithActiveVMs(long offeringId) {
          TransactionLegacy txn = TransactionLegacy.currentTxn();
--        PreparedStatement pstmt = null;
++        PreparedStatement pstmt;
          List<HostVO> result = new ArrayList<>();
          StringBuilder sql = new StringBuilder(GET_HOSTS_OF_ACTIVE_VMS);
          try {
@@@ -1540,7 -1425,7 +1537,7 @@@
  
      @Override
      public List<String> listOrderedHostsHypervisorVersionsInDatacenter(long 
datacenterId, HypervisorType hypervisorType) {
--        PreparedStatement pstmt = null;
++        PreparedStatement pstmt;
          List<String> result = new ArrayList<>();
          try {
              TransactionLegacy txn = TransactionLegacy.currentTxn();

Reply via email to