From c2da4eac89d932ce1bf15440c4ae2095e123d8b8 Mon Sep 17 00:00:00 2001 From: Edison Su Date: Fri, 14 Jun 2013 17:24:31 -0700 Subject: [PATCH] fix NPE for cache ref cnt --- client/tomcatconf/applicationContext.xml.in | 1 - .../storage/image/store/TemplateObject.java | 50 +++++----- .../storage/snapshot/SnapshotObject.java | 20 ++-- .../storage/volume/VolumeObject.java | 16 ++- .../kvm/storage/KVMStorageProcessor.java | 2 - .../discoverer/LibvirtServerDiscoverer.java | 97 +++++++++++-------- 6 files changed, 97 insertions(+), 89 deletions(-) diff --git a/client/tomcatconf/applicationContext.xml.in b/client/tomcatconf/applicationContext.xml.in index 181219a611f..2a746a9cd05 100644 --- a/client/tomcatconf/applicationContext.xml.in +++ b/client/tomcatconf/applicationContext.xml.in @@ -317,7 +317,6 @@ - diff --git a/engine/storage/image/src/org/apache/cloudstack/storage/image/store/TemplateObject.java b/engine/storage/image/src/org/apache/cloudstack/storage/image/store/TemplateObject.java index 5ab52de8500..74a7d43e00b 100644 --- a/engine/storage/image/src/org/apache/cloudstack/storage/image/store/TemplateObject.java +++ b/engine/storage/image/src/org/apache/cloudstack/storage/image/store/TemplateObject.java @@ -44,8 +44,8 @@ import com.cloud.storage.DataStoreRole; import com.cloud.storage.Storage.ImageFormat; import com.cloud.storage.Storage.TemplateType; import com.cloud.storage.VMTemplateStoragePoolVO; -import com.cloud.storage.VMTemplateVO; import com.cloud.storage.VMTemplateStorageResourceAssoc.Status; +import com.cloud.storage.VMTemplateVO; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VMTemplatePoolDao; import com.cloud.utils.component.ComponentContext; @@ -122,12 +122,12 @@ public class TemplateObject implements TemplateInfo { } /* - * + * * // If the template that was passed into this allocator is not * installed in the storage pool, // add 3 * (template size on secondary * storage) to the running total VMTemplateHostVO templateHostVO = * _storageMgr.findVmTemplateHost(templateForVmCreation.getId(), null); - * + * * if (templateHostVO == null) { VMTemplateSwiftVO templateSwiftVO = * _swiftMgr.findByTmpltId(templateForVmCreation.getId()); if * (templateSwiftVO != null) { long templateSize = @@ -152,12 +152,14 @@ public class TemplateObject implements TemplateInfo { return this.imageVO.getFormat(); } -// public boolean stateTransit(TemplateEvent e) throws NoTransitionException { -// this.imageVO = imageDao.findById(this.imageVO.getId()); -// boolean result = imageMgr.getStateMachine().transitTo(this.imageVO, e, null, imageDao); -// this.imageVO = imageDao.findById(this.imageVO.getId()); -// return result; -// } + // public boolean stateTransit(TemplateEvent e) throws NoTransitionException + // { + // this.imageVO = imageDao.findById(this.imageVO.getId()); + // boolean result = imageMgr.getStateMachine().transitTo(this.imageVO, e, + // null, imageDao); + // this.imageVO = imageDao.findById(this.imageVO.getId()); + // return result; + // } @Override public void processEvent(Event event) { @@ -175,9 +177,10 @@ public class TemplateObject implements TemplateInfo { templEvent = TemplateEvent.OperationFailed; } -// if (templEvent != null && this.getDataStore().getRole() == DataStoreRole.Image) { -// this.stateTransit(templEvent); -// } + // if (templEvent != null && this.getDataStore().getRole() == + // DataStoreRole.Image) { + // this.stateTransit(templEvent); + // } } objectInStoreMgr.update(this, event); @@ -238,9 +241,10 @@ public class TemplateObject implements TemplateInfo { templEvent = TemplateEvent.OperationFailed; } -// if (templEvent != null && this.getDataStore().getRole() == DataStoreRole.Image) { -// this.stateTransit(templEvent); -// } + // if (templEvent != null && this.getDataStore().getRole() == + // DataStoreRole.Image) { + // this.stateTransit(templEvent); + // } } objectInStoreMgr.update(this, event); } catch (NoTransitionException e) { @@ -264,9 +268,8 @@ public class TemplateObject implements TemplateInfo { return; } - if (this.dataStore.getRole() == DataStoreRole.Image || - this.dataStore.getRole() == DataStoreRole.ImageCache) { - TemplateDataStoreVO store = templateStoreDao.findById(this.dataStore.getId()); + if (this.dataStore.getRole() == DataStoreRole.Image || this.dataStore.getRole() == DataStoreRole.ImageCache) { + TemplateDataStoreVO store = templateStoreDao.findByStoreTemplate(dataStore.getId(), this.getId()); store.incrRefCnt(); store.setLastUpdated(new Date()); templateStoreDao.update(store.getId(), store); @@ -278,9 +281,8 @@ public class TemplateObject implements TemplateInfo { if (this.dataStore == null) { return; } - if (this.dataStore.getRole() == DataStoreRole.Image || - this.dataStore.getRole() == DataStoreRole.ImageCache) { - TemplateDataStoreVO store = templateStoreDao.findById(this.dataStore.getId()); + if (this.dataStore.getRole() == DataStoreRole.Image || this.dataStore.getRole() == DataStoreRole.ImageCache) { + TemplateDataStoreVO store = templateStoreDao.findByStoreTemplate(dataStore.getId(), this.getId()); store.decrRefCnt(); store.setLastUpdated(new Date()); templateStoreDao.update(store.getId(), store); @@ -292,9 +294,8 @@ public class TemplateObject implements TemplateInfo { if (this.dataStore == null) { return null; } - if (this.dataStore.getRole() == DataStoreRole.Image || - this.dataStore.getRole() == DataStoreRole.ImageCache) { - TemplateDataStoreVO store = templateStoreDao.findById(this.dataStore.getId()); + if (this.dataStore.getRole() == DataStoreRole.Image || this.dataStore.getRole() == DataStoreRole.ImageCache) { + TemplateDataStoreVO store = templateStoreDao.findByStoreTemplate(dataStore.getId(), this.getId()); return store.getRefCnt(); } return null; @@ -442,5 +443,4 @@ public class TemplateObject implements TemplateInfo { return true; } - } diff --git a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java index 03104882706..1cba96efd40 100644 --- a/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java +++ b/engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java @@ -22,7 +22,6 @@ import java.util.Date; import javax.inject.Inject; -import com.cloud.storage.DataStoreRole; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; @@ -41,6 +40,7 @@ import com.cloud.agent.api.Answer; import com.cloud.agent.api.to.DataObjectType; import com.cloud.agent.api.to.DataTO; import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.storage.DataStoreRole; import com.cloud.storage.Snapshot; import com.cloud.storage.SnapshotVO; import com.cloud.storage.dao.SnapshotDao; @@ -279,9 +279,9 @@ public class SnapshotObject implements SnapshotInfo { return; } - if (this.store.getRole() == DataStoreRole.Image || - this.store.getRole() == DataStoreRole.ImageCache) { - SnapshotDataStoreVO store = snapshotStoreDao.findById(this.store.getId()); + if (this.store.getRole() == DataStoreRole.Image || this.store.getRole() == DataStoreRole.ImageCache) { + SnapshotDataStoreVO store = snapshotStoreDao.findByStoreSnapshot(this.store.getRole(), this.store.getId(), + this.getId()); store.incrRefCnt(); store.setLastUpdated(new Date()); snapshotStoreDao.update(store.getId(), store); @@ -293,9 +293,9 @@ public class SnapshotObject implements SnapshotInfo { if (this.store == null) { return; } - if (this.store.getRole() == DataStoreRole.Image || - this.store.getRole() == DataStoreRole.ImageCache) { - SnapshotDataStoreVO store = snapshotStoreDao.findById(this.store.getId()); + if (this.store.getRole() == DataStoreRole.Image || this.store.getRole() == DataStoreRole.ImageCache) { + SnapshotDataStoreVO store = snapshotStoreDao.findByStoreSnapshot(this.store.getRole(), this.store.getId(), + this.getId()); store.decrRefCnt(); store.setLastUpdated(new Date()); snapshotStoreDao.update(store.getId(), store); @@ -307,9 +307,9 @@ public class SnapshotObject implements SnapshotInfo { if (this.store == null) { return null; } - if (this.store.getRole() == DataStoreRole.Image || - this.store.getRole() == DataStoreRole.ImageCache) { - SnapshotDataStoreVO store = snapshotStoreDao.findById(this.store.getId()); + if (this.store.getRole() == DataStoreRole.Image || this.store.getRole() == DataStoreRole.ImageCache) { + SnapshotDataStoreVO store = snapshotStoreDao.findByStoreSnapshot(this.store.getRole(), this.store.getId(), + this.getId()); return store.getRefCnt(); } return null; diff --git a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java index e799098bbb4..d689195957c 100644 --- a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java +++ b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java @@ -252,7 +252,6 @@ public class VolumeObject implements VolumeInfo { } } - @Override public String getName() { return this.volumeVO.getName(); @@ -456,9 +455,8 @@ public class VolumeObject implements VolumeInfo { return; } - if (this.dataStore.getRole() == DataStoreRole.Image || - this.dataStore.getRole() == DataStoreRole.ImageCache) { - VolumeDataStoreVO store = volumeStoreDao.findById(this.dataStore.getId()); + if (this.dataStore.getRole() == DataStoreRole.Image || this.dataStore.getRole() == DataStoreRole.ImageCache) { + VolumeDataStoreVO store = volumeStoreDao.findByStoreVolume(this.dataStore.getId(), this.getId()); store.incrRefCnt(); store.setLastUpdated(new Date()); volumeStoreDao.update(store.getId(), store); @@ -470,9 +468,8 @@ public class VolumeObject implements VolumeInfo { if (this.dataStore == null) { return; } - if (this.dataStore.getRole() == DataStoreRole.Image || - this.dataStore.getRole() == DataStoreRole.ImageCache) { - VolumeDataStoreVO store = volumeStoreDao.findById(this.dataStore.getId()); + if (this.dataStore.getRole() == DataStoreRole.Image || this.dataStore.getRole() == DataStoreRole.ImageCache) { + VolumeDataStoreVO store = volumeStoreDao.findByStoreVolume(this.dataStore.getId(), this.getId()); store.decrRefCnt(); store.setLastUpdated(new Date()); volumeStoreDao.update(store.getId(), store); @@ -484,9 +481,8 @@ public class VolumeObject implements VolumeInfo { if (this.dataStore == null) { return null; } - if (this.dataStore.getRole() == DataStoreRole.Image || - this.dataStore.getRole() == DataStoreRole.ImageCache) { - VolumeDataStoreVO store = volumeStoreDao.findById(this.dataStore.getId()); + if (this.dataStore.getRole() == DataStoreRole.Image || this.dataStore.getRole() == DataStoreRole.ImageCache) { + VolumeDataStoreVO store = volumeStoreDao.findByStoreVolume(this.dataStore.getId(), this.getId()); return store.getRefCnt(); } return null; diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index a03ac697a77..f0f916a411c 100644 --- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -699,8 +699,6 @@ public class KVMStorageProcessor implements StorageProcessor { s_logger.debug("Failed to attach volume: " + vol.getPath() + ", due to " + e.toString()); return new AttachAnswer(e.toString()); } - - } @Override diff --git a/server/src/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java b/server/src/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java index c92ff500b22..f59bdf370aa 100644 --- a/server/src/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java +++ b/server/src/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java @@ -23,7 +23,6 @@ import java.util.List; import java.util.Map; import java.util.UUID; -import javax.ejb.Local; import javax.inject.Inject; import javax.naming.ConfigurationException; @@ -62,20 +61,26 @@ import com.cloud.resource.ServerResource; import com.cloud.resource.UnableDeleteHostException; import com.cloud.utils.ssh.SSHCmdHelper; -public abstract class LibvirtServerDiscoverer extends DiscovererBase implements Discoverer, -Listener, ResourceStateAdapter { +public abstract class LibvirtServerDiscoverer extends DiscovererBase implements Discoverer, Listener, + ResourceStateAdapter { private static final Logger s_logger = Logger.getLogger(LibvirtServerDiscoverer.class); private String _hostIp; - private final int _waitTime = 5; /*wait for 5 minutes*/ + private final int _waitTime = 5; /* wait for 5 minutes */ private String _kvmPrivateNic; private String _kvmPublicNic; private String _kvmGuestNic; - @Inject HostDao _hostDao = null; - @Inject ClusterDao _clusterDao; - @Inject ResourceManager _resourceMgr; - @Inject AgentManager _agentMgr; - @Inject ConfigurationDao _configDao; - @Inject NetworkModel _networkMgr; + @Inject + HostDao _hostDao = null; + @Inject + ClusterDao _clusterDao; + @Inject + ResourceManager _resourceMgr; + @Inject + AgentManager _agentMgr; + @Inject + ConfigurationDao _configDao; + @Inject + NetworkModel _networkMgr; public abstract Hypervisor.HypervisorType getHypervisorType(); @@ -92,8 +97,7 @@ Listener, ResourceStateAdapter { } @Override - public AgentControlAnswer processControlCommand(long agentId, - AgentControlCommand cmd) { + public AgentControlAnswer processControlCommand(long agentId, AgentControlCommand cmd) { // TODO Auto-generated method stub return null; } @@ -127,14 +131,13 @@ Listener, ResourceStateAdapter { } @Override - public Map> find(long dcId, - Long podId, Long clusterId, URI uri, String username, - String password, List hostTags) throws DiscoveryException { + public Map> find(long dcId, Long podId, Long clusterId, URI uri, + String username, String password, List hostTags) throws DiscoveryException { ClusterVO cluster = _clusterDao.findById(clusterId); - if(cluster == null || cluster.getHypervisorType() != getHypervisorType()) { - if(s_logger.isInfoEnabled()) - s_logger.info("invalid cluster id or cluster is not for " + getHypervisorType() + " hypervisors"); + if (cluster == null || cluster.getHypervisorType() != getHypervisorType()) { + if (s_logger.isInfoEnabled()) + s_logger.info("invalid cluster id or cluster is not for " + getHypervisorType() + " hypervisors"); return null; } @@ -153,11 +156,16 @@ Listener, ResourceStateAdapter { InetAddress ia = InetAddress.getByName(hostname); agentIp = ia.getHostAddress(); String guid = UUID.nameUUIDFromBytes(agentIp.getBytes()).toString(); - String guidWithTail = guid + "-LibvirtComputingResource";/*tail added by agent.java*/ + String guidWithTail = guid + "-LibvirtComputingResource";/* + * tail + * added by + * agent + * .java + */ if (_resourceMgr.findHostByGuid(guidWithTail) != null) { s_logger.debug("Skipping " + agentIp + " because " + guidWithTail + " is already in the database."); return null; - } + } sshConnection = new com.trilead.ssh2.Connection(agentIp, 22); @@ -172,7 +180,7 @@ Listener, ResourceStateAdapter { return null; } - List netInfos = _networkMgr.getPhysicalNetworkInfo(dcId, getHypervisorType()); + List netInfos = _networkMgr.getPhysicalNetworkInfo(dcId, getHypervisorType()); String kvmPrivateNic = null; String kvmPublicNic = null; String kvmGuestNic = null; @@ -193,7 +201,7 @@ Listener, ResourceStateAdapter { kvmPrivateNic = _kvmPrivateNic; kvmPublicNic = _kvmPublicNic; kvmGuestNic = _kvmGuestNic; - } + } if (kvmPublicNic == null) { kvmPublicNic = (kvmGuestNic != null) ? kvmGuestNic : kvmPrivateNic; @@ -207,7 +215,8 @@ Listener, ResourceStateAdapter { kvmGuestNic = (kvmPublicNic != null) ? kvmPublicNic : kvmPrivateNic; } - String parameters = " -m " + _hostIp + " -z " + dcId + " -p " + podId + " -c " + clusterId + " -g " + guid + " -a"; + String parameters = " -m " + _hostIp + " -z " + dcId + " -p " + podId + " -c " + clusterId + " -g " + guid + + " -a"; parameters += " --pubNic=" + kvmPublicNic; parameters += " --prvNic=" + kvmPrivateNic; @@ -220,8 +229,8 @@ Listener, ResourceStateAdapter { params.put("zone", Long.toString(dcId)); params.put("pod", Long.toString(podId)); - params.put("cluster", Long.toString(clusterId)); - params.put("guid", guid); + params.put("cluster", Long.toString(clusterId)); + params.put("guid", guid); params.put("agentIp", agentIp); kvmResource.configure("kvm agent", params); resources.put(kvmResource, details); @@ -238,16 +247,16 @@ Listener, ResourceStateAdapter { _clusterDao.update(clusterId, cluster); } - //save user name and password + // save user name and password _hostDao.loadDetails(connectedHost); Map hostDetails = connectedHost.getDetails(); hostDetails.put("password", password); hostDetails.put("username", username); _hostDao.saveDetails(connectedHost); return resources; - } catch (DiscoveredWithErrorException e){ + } catch (DiscoveredWithErrorException e) { throw e; - }catch (Exception e) { + } catch (Exception e) { String msg = " can't setup agent, due to " + e.toString() + " - " + e.getMessage(); s_logger.warn(msg); } finally { @@ -259,7 +268,7 @@ Listener, ResourceStateAdapter { } private HostVO waitForHostConnect(long dcId, long podId, long clusterId, String guid) { - for (int i = 0; i < _waitTime *2; i++) { + for (int i = 0; i < _waitTime * 2; i++) { List hosts = _resourceMgr.listAllUpAndEnabledHosts(Host.Type.Routing, clusterId, podId, dcId); for (HostVO host : hosts) { if (host.getGuid().equalsIgnoreCase(guid)) { @@ -283,7 +292,8 @@ Listener, ResourceStateAdapter { @Override public boolean configure(String name, Map params) throws ConfigurationException { -// _setupAgentPath = Script.findScript(getPatchPath(), "setup_agent.sh"); + // _setupAgentPath = Script.findScript(getPatchPath(), + // "setup_agent.sh"); _kvmPrivateNic = _configDao.getValue(Config.KvmPrivateNetwork.key()); if (_kvmPrivateNic == null) { _kvmPrivateNic = "cloudbr0"; @@ -312,15 +322,14 @@ Listener, ResourceStateAdapter { } @Override - public void postDiscovery(List hosts, long msId) - throws DiscoveryException { + public void postDiscovery(List hosts, long msId) throws DiscoveryException { // TODO Auto-generated method stub } @Override public boolean matchHypervisor(String hypervisor) { // for backwards compatibility, if not supplied, always let to try it - if(hypervisor == null) + if (hypervisor == null) return true; return getHypervisorType().toString().equalsIgnoreCase(hypervisor); @@ -340,6 +349,11 @@ Listener, ResourceStateAdapter { /* KVM requires host are the same in cluster */ ClusterVO clusterVO = _clusterDao.findById(host.getClusterId()); + if (clusterVO == null) { + s_logger.debug("cannot find cluster: " + host.getClusterId()); + throw new IllegalArgumentException("cannot add host, due to can't find cluster: " + host.getClusterId()); + } + List hostsInCluster = _resourceMgr.listAllHostsInCluster(clusterVO.getId()); if (!hostsInCluster.isEmpty()) { HostVO oneHost = hostsInCluster.get(0); @@ -347,8 +361,9 @@ Listener, ResourceStateAdapter { String hostOsInCluster = oneHost.getDetail("Host.OS"); String hostOs = ssCmd.getHostDetails().get("Host.OS"); if (!hostOsInCluster.equalsIgnoreCase(hostOs)) { - throw new IllegalArgumentException("Can't add host: " + firstCmd.getPrivateIpAddress() + " with hostOS: " + hostOs + " into a cluster," - + "in which there are " + hostOsInCluster + " hosts added"); + throw new IllegalArgumentException("Can't add host: " + firstCmd.getPrivateIpAddress() + + " with hostOS: " + hostOs + " into a cluster," + "in which there are " + hostOsInCluster + + " hosts added"); } } @@ -358,17 +373,17 @@ Listener, ResourceStateAdapter { } @Override - public HostVO createHostVOForDirectConnectAgent(HostVO host, StartupCommand[] startup, ServerResource resource, Map details, - List hostTags) { + public HostVO createHostVOForDirectConnectAgent(HostVO host, StartupCommand[] startup, ServerResource resource, + Map details, List hostTags) { // TODO Auto-generated method stub return null; } @Override - public DeleteHostAnswer deleteHost(HostVO host, boolean isForced, boolean isForceDeleteStorage) throws UnableDeleteHostException { - if (host.getType() != Host.Type.Routing || - (host.getHypervisorType() != HypervisorType.KVM && - host.getHypervisorType() != HypervisorType.LXC)) { + public DeleteHostAnswer deleteHost(HostVO host, boolean isForced, boolean isForceDeleteStorage) + throws UnableDeleteHostException { + if (host.getType() != Host.Type.Routing + || (host.getHypervisorType() != HypervisorType.KVM && host.getHypervisorType() != HypervisorType.LXC)) { return null; }