You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Chip Childers <ch...@sungard.com> on 2013/10/04 22:26:30 UTC
Re: [1/2] Refactor Storage Related Resource Code These changes are a
joint effort between Edison and I to refactor some of the code around
snapshotting VM volumes and creating templates/volumes from VM volume
snapshots. In general, we were working towards al
I some outstanding questions I just posted to the review.
Any thoughts on those questions?
https://reviews.apache.org/r/14477/
On Fri, Oct 4, 2013 at 4:07 PM, <ed...@apache.org> wrote:
> Updated Branches:
> refs/heads/master c9f41d404 -> 180cfa19e
>
>
>
> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/180cfa19/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServerStorageProcessor.java
> ----------------------------------------------------------------------
> diff --git
> a/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServerStorageProcessor.java
> b/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServerStorageProcessor.java
> index 739b974..5da0571 100644
> ---
> a/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServerStorageProcessor.java
> +++
> b/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServerStorageProcessor.java
> @@ -18,6 +18,36 @@
> */
> package com.cloud.hypervisor.xen.resource;
>
> +import java.io.File;
> +import java.net.URI;
> +import java.util.Arrays;
> +import java.util.HashMap;
> +import java.util.List;
> +import java.util.Map;
> +import java.util.Set;
> +import java.util.UUID;
> +
> +import org.apache.cloudstack.storage.command.AttachAnswer;
> +import org.apache.cloudstack.storage.command.AttachCommand;
> +import org.apache.cloudstack.storage.command.AttachPrimaryDataStoreAnswer;
> +import org.apache.cloudstack.storage.command.AttachPrimaryDataStoreCmd;
> +import org.apache.cloudstack.storage.command.CopyCmdAnswer;
> +import org.apache.cloudstack.storage.command.CopyCommand;
> +import org.apache.cloudstack.storage.command.CreateObjectAnswer;
> +import org.apache.cloudstack.storage.command.CreateObjectCommand;
> +import org.apache.cloudstack.storage.command.DeleteCommand;
> +import org.apache.cloudstack.storage.command.DettachAnswer;
> +import org.apache.cloudstack.storage.command.DettachCommand;
> +import org.apache.cloudstack.storage.command.ForgetObjectCmd;
> +import org.apache.cloudstack.storage.command.IntroduceObjectAnswer;
> +import org.apache.cloudstack.storage.command.IntroduceObjectCmd;
> +import org.apache.cloudstack.storage.datastore.protocol.DataStoreProtocol;
> +import org.apache.cloudstack.storage.to.SnapshotObjectTO;
> +import org.apache.cloudstack.storage.to.TemplateObjectTO;
> +import org.apache.cloudstack.storage.to.VolumeObjectTO;
> +import org.apache.log4j.Logger;
> +import org.apache.xmlrpc.XmlRpcException;
> +
> import com.cloud.agent.api.Answer;
> import com.cloud.agent.api.CreateStoragePoolCommand;
> import com.cloud.agent.api.to.DataObjectType;
> @@ -51,33 +81,6 @@ import com.xensource.xenapi.VBD;
> import com.xensource.xenapi.VDI;
> import com.xensource.xenapi.VM;
> import com.xensource.xenapi.VMGuestMetrics;
> -import org.apache.cloudstack.storage.command.AttachAnswer;
> -import org.apache.cloudstack.storage.command.AttachCommand;
> -import org.apache.cloudstack.storage.command.AttachPrimaryDataStoreAnswer;
> -import org.apache.cloudstack.storage.command.AttachPrimaryDataStoreCmd;
> -import org.apache.cloudstack.storage.command.CopyCmdAnswer;
> -import org.apache.cloudstack.storage.command.CopyCommand;
> -import org.apache.cloudstack.storage.command.CreateObjectAnswer;
> -import org.apache.cloudstack.storage.command.CreateObjectCommand;
> -import org.apache.cloudstack.storage.command.DeleteCommand;
> -import org.apache.cloudstack.storage.command.DettachAnswer;
> -import org.apache.cloudstack.storage.command.DettachCommand;
> -import org.apache.cloudstack.storage.datastore.protocol.DataStoreProtocol;
> -import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
> -import org.apache.cloudstack.storage.to.SnapshotObjectTO;
> -import org.apache.cloudstack.storage.to.TemplateObjectTO;
> -import org.apache.cloudstack.storage.to.VolumeObjectTO;
> -import org.apache.log4j.Logger;
> -import org.apache.xmlrpc.XmlRpcException;
> -
> -import java.io.File;
> -import java.net.URI;
> -import java.util.Arrays;
> -import java.util.HashMap;
> -import java.util.List;
> -import java.util.Map;
> -import java.util.Set;
> -import java.util.UUID;
>
> import static com.cloud.utils.ReflectUtil.flattenProperties;
> import static com.google.common.collect.Lists.newArrayList;
> @@ -841,8 +844,7 @@ public class XenServerStorageProcessor implements
> StorageProcessor {
>
> URI uri = new URI(storeUrl);
> String tmplpath = uri.getHost() + ":" + uri.getPath() +
> "/" + srcData.getPath();
> - PrimaryDataStoreTO destStore =
> (PrimaryDataStoreTO)destData.getDataStore();
> - String poolName = destStore.getUuid();
> + String poolName = destData.getDataStore().getUuid();
> Connection conn = hypervisorResource.getConnection();
>
> SR poolsr = null;
> @@ -892,8 +894,7 @@ public class XenServerStorageProcessor implements
> StorageProcessor {
>
> try {
> Connection conn = hypervisorResource.getConnection();
> - PrimaryDataStoreTO primaryStore =
> (PrimaryDataStoreTO)data.getDataStore();
> - SR poolSr = hypervisorResource.getStorageRepository(conn,
> primaryStore.getUuid());
> + SR poolSr = hypervisorResource.getStorageRepository(conn,
> data.getDataStore().getUuid());
> VDI.Record vdir = new VDI.Record();
> vdir.nameLabel = volume.getName();
> vdir.SR = poolSr;
> @@ -921,7 +922,6 @@ public class XenServerStorageProcessor implements
> StorageProcessor {
> Connection conn = hypervisorResource.getConnection();
> DataTO srcData = cmd.getSrcTO();
> DataTO destData = cmd.getDestTO();
> - PrimaryDataStoreTO pool =
> (PrimaryDataStoreTO)destData.getDataStore();
> VolumeObjectTO volume = (VolumeObjectTO)destData;
> VDI vdi = null;
> try {
> @@ -943,7 +943,7 @@ public class XenServerStorageProcessor implements
> StorageProcessor {
>
> return new CopyCmdAnswer(newVol);
> } catch (Exception e) {
> - s_logger.warn("Unable to create volume; Pool=" + pool + ";
> Disk: ", e);
> + s_logger.warn("Unable to create volume; Pool=" + destData +
> "; Disk: ", e);
> return new CopyCmdAnswer(e.toString());
> }
> }
> @@ -956,13 +956,12 @@ public class XenServerStorageProcessor implements
> StorageProcessor {
> int wait = cmd.getWait();
> VolumeObjectTO srcVolume = (VolumeObjectTO)srcData;
> VolumeObjectTO destVolume = (VolumeObjectTO)destData;
> - PrimaryDataStoreTO primaryStore =
> (PrimaryDataStoreTO)destVolume.getDataStore();
> DataStoreTO srcStore = srcVolume.getDataStore();
>
> if (srcStore instanceof NfsTO) {
> NfsTO nfsStore = (NfsTO)srcStore;
> try {
> - SR primaryStoragePool =
> hypervisorResource.getStorageRepository(conn, primaryStore.getUuid());
> + SR primaryStoragePool =
> hypervisorResource.getStorageRepository(conn,
> destVolume.getDataStore().getUuid());
> String srUuid = primaryStoragePool.getUuid(conn);
> URI uri = new URI(nfsStore.getUrl());
> String volumePath = uri.getHost() + ":" + uri.getPath() +
> File.separator + srcVolume.getPath();
> @@ -1179,8 +1178,7 @@ public class XenServerStorageProcessor implements
> StorageProcessor {
> DataTO cacheData = cmd.getCacheTO();
> DataTO destData = cmd.getDestTO();
> int wait = cmd.getWait();
> - PrimaryDataStoreTO primaryStore =
> (PrimaryDataStoreTO)srcData.getDataStore();
> - String primaryStorageNameLabel = primaryStore.getUuid();
> + String primaryStorageNameLabel = srcData.getDataStore().getUuid();
> String secondaryStorageUrl = null;
> NfsTO cacheStore = null;
> String destPath = null;
> @@ -1415,7 +1413,6 @@ public class XenServerStorageProcessor implements
> StorageProcessor {
> DataTO srcData = cmd.getSrcTO();
> SnapshotObjectTO snapshot = (SnapshotObjectTO)srcData;
> DataTO destData = cmd.getDestTO();
> - PrimaryDataStoreTO pool =
> (PrimaryDataStoreTO)destData.getDataStore();
> DataStoreTO imageStore = srcData.getDataStore();
>
> if (!(imageStore instanceof NfsTO)) {
> @@ -1423,7 +1420,7 @@ public class XenServerStorageProcessor implements
> StorageProcessor {
> }
>
> NfsTO nfsImageStore = (NfsTO)imageStore;
> - String primaryStorageNameLabel = pool.getUuid();
> + String primaryStorageNameLabel =
> destData.getDataStore().getUuid();
> String secondaryStorageUrl = nfsImageStore.getUrl();
> int wait = cmd.getWait();
> boolean result = false;
> @@ -1503,4 +1500,32 @@ public class XenServerStorageProcessor implements
> StorageProcessor {
> }
> return new Answer(cmd, false, "unsupported storage type");
> }
> +
> + @Override
> + public Answer introduceObject(IntroduceObjectCmd cmd) {
> + try {
> + Connection conn = hypervisorResource.getConnection();
> + DataStoreTO store = cmd.getDataTO().getDataStore();
> + SR poolSr = hypervisorResource.getStorageRepository(conn,
> store.getUuid());
> + poolSr.scan(conn);
> + return new IntroduceObjectAnswer(cmd.getDataTO());
> + } catch (Exception e) {
> + s_logger.debug("Failed to introduce object", e);
> + return new Answer(cmd, false, e.toString());
> + }
> + }
> +
> + @Override
> + public Answer forgetObject(ForgetObjectCmd cmd) {
> + try {
> + Connection conn = hypervisorResource.getConnection();
> + DataTO data = cmd.getDataTO();
> + VDI vdi = VDI.getByUuid(conn, data.getPath());
> + vdi.forget(conn);
> + return new IntroduceObjectAnswer(cmd.getDataTO());
> + } catch (Exception e) {
> + s_logger.debug("Failed to introduce object", e);
> + return new Answer(cmd, false, e.toString());
> + }
> + }
> }
>
>
> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/180cfa19/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
> ----------------------------------------------------------------------
> diff --git
> a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
> b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
> index 2297e6a..0b53cfd 100755
> --- a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
> +++ b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java
> @@ -48,7 +48,6 @@ import
> org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
> import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
> import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
> import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
> -
> import org.apache.log4j.Logger;
> import org.springframework.stereotype.Component;
>
> @@ -69,13 +68,11 @@ import com.cloud.event.ActionEventUtils;
> import com.cloud.event.EventTypes;
> import com.cloud.event.EventVO;
> import com.cloud.event.UsageEventUtils;
> -import com.cloud.event.dao.EventDao;
> import com.cloud.exception.InvalidParameterValueException;
> import com.cloud.exception.PermissionDeniedException;
> import com.cloud.exception.ResourceAllocationException;
> import com.cloud.exception.StorageUnavailableException;
> import com.cloud.host.HostVO;
> -import com.cloud.host.dao.HostDao;
> import com.cloud.hypervisor.Hypervisor.HypervisorType;
> import com.cloud.projects.Project.ListProjectResourcesCriteria;
> import com.cloud.resource.ResourceManager;
> @@ -193,10 +190,10 @@ public class SnapshotManagerImpl extends ManagerBase
> implements SnapshotManager,
> @Inject VolumeDataFactory volFactory;
> @Inject SnapshotDataFactory snapshotFactory;
> @Inject EndPointSelector _epSelector;
> - @Inject
> - private ResourceManager _resourceMgr;
> - @Inject
> - protected List<SnapshotStrategy> snapshotStrategies;
> + @Inject
> + private ResourceManager _resourceMgr;
> + @Inject
> + protected List<SnapshotStrategy> snapshotStrategies;
>
>
> private int _totalRetries;
> @@ -261,16 +258,38 @@ public class SnapshotManagerImpl extends ManagerBase
> implements SnapshotManager,
> }
>
> @Override
> + public boolean revertSnapshot(Long snapshotId) {
> + Snapshot snapshot = _snapshotDao.findById(snapshotId);
> + if (snapshot == null) {
> + throw new InvalidParameterValueException("No such snapshot");
> + }
> +
> + SnapshotStrategy snapshotStrategy = null;
> + for (SnapshotStrategy strategy : snapshotStrategies) {
> + if (strategy.canHandle(snapshot)) {
> + snapshotStrategy = strategy;
> + break;
> + }
> + }
> +
> + if (snapshotStrategy == null) {
> + return false;
> + }
> +
> + return snapshotStrategy.revertSnapshot(snapshotId);
> + }
> +
> + @Override
> @DB
> @ActionEvent(eventType = EventTypes.EVENT_SNAPSHOT_CREATE,
> eventDescription = "creating snapshot", async = true)
> public Snapshot createSnapshot(Long volumeId, Long policyId, Long
> snapshotId, Account snapshotOwner) {
> VolumeInfo volume = volFactory.getVolume(volumeId);
> if (volume == null) {
> - throw new InvalidParameterValueException("No such volume
> exist");
> + throw new InvalidParameterValueException("No such volume
> exist");
> }
>
> if (volume.getState() != Volume.State.Ready) {
> - throw new InvalidParameterValueException("Volume is not in
> ready state");
> + throw new InvalidParameterValueException("Volume is not in
> ready state");
> }
>
>
> @@ -281,16 +300,16 @@ public class SnapshotManagerImpl extends ManagerBase
> implements SnapshotManager,
> SnapshotInfo snapshot = snapshotFactory.getSnapshot(snapshotId,
> DataStoreRole.Primary);
>
> try {
> - postCreateSnapshot(volumeId, snapshot.getId(), policyId);
> - //Check if the snapshot was removed while backingUp. If
> yes, do not log snapshot create usage event
> - SnapshotVO freshSnapshot =
> _snapshotDao.findById(snapshot.getId());
> - if ((freshSnapshot != null) && backedUp) {
> -
> UsageEventUtils.publishUsageEvent(EventTypes.EVENT_SNAPSHOT_CREATE,
> snapshot.getAccountId(),
> - snapshot.getDataCenterId(),
> snapshotId, snapshot.getName(), null, null,
> - volume.getSize(),
> snapshot.getClass().getName(), snapshot.getUuid());
> - }
> + postCreateSnapshot(volumeId, snapshot.getId(), policyId);
> + //Check if the snapshot was removed while backingUp. If yes,
> do not log snapshot create usage event
> + SnapshotVO freshSnapshot =
> _snapshotDao.findById(snapshot.getId());
> + if ((freshSnapshot != null) && backedUp) {
> +
> UsageEventUtils.publishUsageEvent(EventTypes.EVENT_SNAPSHOT_CREATE,
> snapshot.getAccountId(),
> + snapshot.getDataCenterId(), snapshotId,
> snapshot.getName(), null, null,
> + volume.getSize(), snapshot.getClass().getName(),
> snapshot.getUuid());
> + }
>
> -
> _resourceLimitMgr.incrementResourceCount(snapshotOwner.getId(),
> ResourceType.snapshot);
> +
> _resourceLimitMgr.incrementResourceCount(snapshotOwner.getId(),
> ResourceType.snapshot);
>
> } catch(Exception e) {
> s_logger.debug("Failed to create snapshot", e);
> @@ -311,12 +330,12 @@ public class SnapshotManagerImpl extends ManagerBase
> implements SnapshotManager,
>
> @Override
> public Snapshot backupSnapshot(Long snapshotId) {
> - SnapshotInfo snapshot = snapshotFactory.getSnapshot(snapshotId,
> DataStoreRole.Image);
> - if (snapshot != null) {
> - throw new CloudRuntimeException("Already in the backup
> snapshot:" + snapshotId);
> - }
> + SnapshotInfo snapshot = snapshotFactory.getSnapshot(snapshotId,
> DataStoreRole.Image);
> + if (snapshot != null) {
> + throw new CloudRuntimeException("Already in the backup
> snapshot:" + snapshotId);
> + }
>
> - return snapshotSrv.backupSnapshot(snapshot);
> + return snapshotSrv.backupSnapshot(snapshot);
> }
>
> /*
> @@ -412,14 +431,14 @@ public class SnapshotManagerImpl extends ManagerBase
> implements SnapshotManager,
>
> @Override
> public SnapshotVO getParentSnapshot(VolumeInfo volume) {
> - long preId = _snapshotDao.getLastSnapshot(volume.getId(),
> DataStoreRole.Primary);
> + long preId = _snapshotDao.getLastSnapshot(volume.getId(),
> DataStoreRole.Primary);
>
> - SnapshotVO preSnapshotVO = null;
> - if (preId != 0 && !(volume.getLastPoolId() != null &&
> !volume.getLastPoolId().equals(volume.getPoolId()))) {
> - preSnapshotVO = _snapshotDao.findByIdIncludingRemoved(preId);
> - }
> + SnapshotVO preSnapshotVO = null;
> + if (preId != 0 && !(volume.getLastPoolId() != null &&
> !volume.getLastPoolId().equals(volume.getPoolId()))) {
> + preSnapshotVO = _snapshotDao.findByIdIncludingRemoved(preId);
> + }
>
> - return preSnapshotVO;
> + return preSnapshotVO;
> }
>
> private Long getSnapshotUserId() {
> @@ -463,7 +482,7 @@ public class SnapshotManagerImpl extends ManagerBase
> implements SnapshotManager,
> s_logger.debug("Max snaps: " + policy.getMaxSnaps() + "
> exceeded for snapshot policy with Id: " + policyId + ". Deleting oldest
> snapshot: " + oldSnapId);
> }
> if(deleteSnapshot(oldSnapId)){
> - //log Snapshot delete event
> + //log Snapshot delete event
> ActionEventUtils.onCompletedActionEvent(User.UID_SYSTEM,
> oldestSnapshot.getAccountId(), EventVO.LEVEL_INFO,
> EventTypes.EVENT_SNAPSHOT_DELETE, "Successfully deleted oldest snapshot: "
> + oldSnapId, 0);
> }
> snaps.remove(oldestSnapshot);
> @@ -485,27 +504,27 @@ public class SnapshotManagerImpl extends ManagerBase
> implements SnapshotManager,
> _accountMgr.checkAccess(caller, null, true, snapshotCheck);
> SnapshotStrategy snapshotStrategy = null;
> for (SnapshotStrategy strategy : snapshotStrategies) {
> - if (strategy.canHandle(snapshotCheck)) {
> - snapshotStrategy = strategy;
> - break;
> - }
> + if (strategy.canHandle(snapshotCheck)) {
> + snapshotStrategy = strategy;
> + break;
> + }
> }
> try {
> - boolean result =
> snapshotStrategy.deleteSnapshot(snapshotId);
> - if (result) {
> + boolean result = snapshotStrategy.deleteSnapshot(snapshotId);
> + if (result) {
> if (snapshotCheck.getState() == Snapshot.State.BackedUp) {
> -
> UsageEventUtils.publishUsageEvent(EventTypes.EVENT_SNAPSHOT_DELETE,
> snapshotCheck.getAccountId(),
> -
> snapshotCheck.getDataCenterId(), snapshotId, snapshotCheck.getName(), null,
> null, 0L,
> -
> snapshotCheck.getClass().getName(), snapshotCheck.getUuid());
> - }
> +
> UsageEventUtils.publishUsageEvent(EventTypes.EVENT_SNAPSHOT_DELETE,
> snapshotCheck.getAccountId(),
> + snapshotCheck.getDataCenterId(), snapshotId,
> snapshotCheck.getName(), null, null, 0L,
> + snapshotCheck.getClass().getName(),
> snapshotCheck.getUuid());
> + }
>
> _resourceLimitMgr.decrementResourceCount(snapshotCheck.getAccountId(),
> ResourceType.snapshot);
>
> _resourceLimitMgr.decrementResourceCount(snapshotCheck.getAccountId(),
> ResourceType.secondary_storage,
> new Long(snapshotCheck.getSize()));
> - }
> - return result;
> + }
> + return result;
> } catch (Exception e) {
> - s_logger.debug("Failed to delete snapshot: " +
> snapshotCheck.getId() + ":" + e.toString());
> - throw new CloudRuntimeException("Failed to delete
> snapshot:" + e.toString());
> + s_logger.debug("Failed to delete snapshot: " +
> snapshotCheck.getId() + ":" + e.toString());
> + throw new CloudRuntimeException("Failed to delete snapshot:"
> + e.toString());
> }
> }
>
> @@ -544,10 +563,10 @@ public class SnapshotManagerImpl extends ManagerBase
> implements SnapshotManager,
> }
>
> Ternary<Long, Boolean, ListProjectResourcesCriteria>
> domainIdRecursiveListProject = new Ternary<Long, Boolean,
> ListProjectResourcesCriteria>(cmd.getDomainId(), cmd.isRecursive(), null);
> - _accountMgr.buildACLSearchParameters(caller, id,
> cmd.getAccountName(), cmd.getProjectId(), permittedAccounts,
> domainIdRecursiveListProject, cmd.listAll(), false);
> - Long domainId = domainIdRecursiveListProject.first();
> - Boolean isRecursive = domainIdRecursiveListProject.second();
> - ListProjectResourcesCriteria listProjectResourcesCriteria =
> domainIdRecursiveListProject.third();
> + _accountMgr.buildACLSearchParameters(caller, id,
> cmd.getAccountName(), cmd.getProjectId(), permittedAccounts,
> domainIdRecursiveListProject, cmd.listAll(), false);
> + Long domainId = domainIdRecursiveListProject.first();
> + Boolean isRecursive = domainIdRecursiveListProject.second();
> + ListProjectResourcesCriteria listProjectResourcesCriteria =
> domainIdRecursiveListProject.third();
>
> Filter searchFilter = new Filter(SnapshotVO.class, "created",
> false, cmd.getStartIndex(), cmd.getPageSizeVal());
> SearchBuilder<SnapshotVO> sb = _snapshotDao.createSearchBuilder();
> @@ -560,7 +579,7 @@ public class SnapshotManagerImpl extends ManagerBase
> implements SnapshotManager,
> sb.and("snapshotTypeEQ", sb.entity().getsnapshotType(),
> SearchCriteria.Op.IN);
> sb.and("snapshotTypeNEQ", sb.entity().getsnapshotType(),
> SearchCriteria.Op.NEQ);
> sb.and("dataCenterId", sb.entity().getDataCenterId(),
> SearchCriteria.Op.EQ);
> -
> +
> if (tags != null && !tags.isEmpty()) {
> SearchBuilder<ResourceTagVO> tagSearch =
> _resourceTagDao.createSearchBuilder();
> for (int count=0; count < tags.size(); count++) {
> @@ -595,7 +614,7 @@ public class SnapshotManagerImpl extends ManagerBase
> implements SnapshotManager,
> if (zoneId != null) {
> sc.setParameters("dataCenterId", zoneId);
> }
> -
> +
> if (name != null) {
> sc.setParameters("name", "%" + name + "%");
> }
> @@ -763,10 +782,10 @@ public class SnapshotManagerImpl extends ManagerBase
> implements SnapshotManager,
> long domainLimit =
> _resourceLimitMgr.findCorrectResourceLimitForDomain(_domainMgr.getDomain(owner.getDomainId()),
> ResourceType.snapshot);
> int max = cmd.getMaxSnaps().intValue();
> if (owner.getType() != Account.ACCOUNT_TYPE_ADMIN &&
> ((accountLimit != -1 && max > accountLimit) || (domainLimit != -1 && max >
> domainLimit))) {
> - String message = "domain/account";
> - if (owner.getType() == Account.ACCOUNT_TYPE_PROJECT) {
> - message = "domain/project";
> - }
> + String message = "domain/account";
> + if (owner.getType() == Account.ACCOUNT_TYPE_PROJECT) {
> + message = "domain/project";
> + }
>
> throw new InvalidParameterValueException("Max number of
> snapshots shouldn't exceed the " + message + " level snapshot limit");
> }
> @@ -905,37 +924,37 @@ public class SnapshotManagerImpl extends ManagerBase
> implements SnapshotManager,
>
>
> private boolean hostSupportSnapsthotForVolume(HostVO host, VolumeInfo
> volume) {
> - if (host.getHypervisorType() != HypervisorType.KVM) {
> - return true;
> - }
> + if (host.getHypervisorType() != HypervisorType.KVM) {
> + return true;
> + }
>
> //Turn off snapshot by default for KVM if the volume attached to
> vm that is not in the Stopped/Destroyed state,
> - //unless it is set in the global flag
> - Long vmId = volume.getInstanceId();
> - if (vmId != null) {
> - VMInstanceVO vm = _vmDao.findById(vmId);
> - if (vm.getState() != VirtualMachine.State.Stopped &&
> vm.getState() != VirtualMachine.State.Destroyed) {
> - boolean snapshotEnabled =
> Boolean.parseBoolean(_configDao.getValue("kvm.snapshot.enabled"));
> - if (!snapshotEnabled) {
> - s_logger.debug("Snapshot is not supported on host
> " + host + " for the volume " + volume + " attached to the vm " + vm);
> - return false;
> - }
> - }
> - }
> -
> - // Determine host capabilities
> - String caps = host.getCapabilities();
> -
> - if (caps != null) {
> - String[] tokens = caps.split(",");
> - for (String token : tokens) {
> - if (token.contains("snapshot")) {
> - return true;
> - }
> - }
> - }
> - return false;
> - }
> + //unless it is set in the global flag
> + Long vmId = volume.getInstanceId();
> + if (vmId != null) {
> + VMInstanceVO vm = _vmDao.findById(vmId);
> + if (vm.getState() != VirtualMachine.State.Stopped &&
> vm.getState() != VirtualMachine.State.Destroyed) {
> + boolean snapshotEnabled =
> Boolean.parseBoolean(_configDao.getValue("kvm.snapshot.enabled"));
> + if (!snapshotEnabled) {
> + s_logger.debug("Snapshot is not supported on host " +
> host + " for the volume " + volume + " attached to the vm " + vm);
> + return false;
> + }
> + }
> + }
> +
> + // Determine host capabilities
> + String caps = host.getCapabilities();
> +
> + if (caps != null) {
> + String[] tokens = caps.split(",");
> + for (String token : tokens) {
> + if (token.contains("snapshot")) {
> + return true;
> + }
> + }
> + }
> + return false;
> + }
>
> private boolean supportedByHypervisor(VolumeInfo volume) {
> HypervisorType hypervisorType;
> @@ -967,10 +986,10 @@ public class SnapshotManagerImpl extends ManagerBase
> implements SnapshotManager,
> }
> }
>
> - // if volume is attached to a vm in destroyed or expunging
> state; disallow
> - if (volume.getInstanceId() != null) {
> - UserVmVO userVm =
> _vmDao.findById(volume.getInstanceId());
> - if (userVm != null) {
> + // if volume is attached to a vm in destroyed or expunging state;
> disallow
> + if (volume.getInstanceId() != null) {
> + UserVmVO userVm = _vmDao.findById(volume.getInstanceId());
> + if (userVm != null) {
> if (userVm.getState().equals(State.Destroyed) ||
> userVm.getState().equals(State.Expunging)) {
> throw new CloudRuntimeException("Creating snapshot
> failed due to volume:" + volume.getId()
> + " is associated with vm:" +
> userVm.getInstanceName() + " is in "
> @@ -993,11 +1012,11 @@ public class SnapshotManagerImpl extends
> ManagerBase implements SnapshotManager,
> throw new CloudRuntimeException(
> "There is other active vm snapshot tasks on
> the instance to which the volume is attached, please try again later");
> }
> - }
> - }
> + }
> + }
>
> - return true;
> - }
> + return true;
> + }
> @Override
> @DB
> public SnapshotInfo takeSnapshot(VolumeInfo volume) throws
> ResourceAllocationException {
> @@ -1124,10 +1143,10 @@ public class SnapshotManagerImpl extends
> ManagerBase implements SnapshotManager,
> public boolean canOperateOnVolume(Volume volume) {
> List<SnapshotVO> snapshots =
> _snapshotDao.listByStatus(volume.getId(), Snapshot.State.Creating,
> Snapshot.State.CreatedOnPrimary,
> Snapshot.State.BackingUp);
> - if (snapshots.size() > 0) {
> - return false;
> - }
> - return true;
> + if (snapshots.size() > 0) {
> + return false;
> + }
> + return true;
> }
>
> @Override
>
>
> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/180cfa19/services/secondary-storage/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
> ----------------------------------------------------------------------
> diff --git
> a/services/secondary-storage/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
> b/services/secondary-storage/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
> index 3ef950b..e26f02d 100755
> ---
> a/services/secondary-storage/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
> +++
> b/services/secondary-storage/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java
> @@ -16,14 +16,15 @@
> // under the License.
> package org.apache.cloudstack.storage.resource;
>
> -import static com.cloud.utils.S3Utils.putFile;
> -import static com.cloud.utils.StringUtils.join;
> -import static com.cloud.utils.db.GlobalLock.executeWithNoWaitLock;
> -import static java.lang.String.format;
> -import static java.util.Arrays.asList;
> -import static org.apache.commons.lang.StringUtils.substringAfterLast;
> -
> -import java.io.*;
> +import java.io.BufferedReader;
> +import java.io.BufferedWriter;
> +import java.io.File;
> +import java.io.FileInputStream;
> +import java.io.FileOutputStream;
> +import java.io.FileReader;
> +import java.io.FileWriter;
> +import java.io.IOException;
> +import java.io.InputStream;
> import java.math.BigInteger;
> import java.net.InetAddress;
> import java.net.URI;
> @@ -39,10 +40,6 @@ import java.util.concurrent.Callable;
>
> import javax.naming.ConfigurationException;
>
> -import com.cloud.agent.api.storage.*;
> -import com.cloud.storage.VMTemplateStorageResourceAssoc;
> -import com.cloud.storage.template.*;
> -import com.cloud.utils.SwiftUtil;
> import org.apache.cloudstack.storage.command.CopyCmdAnswer;
> import org.apache.cloudstack.storage.command.CopyCommand;
> import org.apache.cloudstack.storage.command.DeleteCommand;
> @@ -53,6 +50,8 @@ import
> org.apache.cloudstack.storage.template.DownloadManagerImpl;
> import
> org.apache.cloudstack.storage.template.DownloadManagerImpl.ZfsPathParser;
> import org.apache.cloudstack.storage.template.UploadManager;
> import org.apache.cloudstack.storage.template.UploadManagerImpl;
> +import org.apache.cloudstack.storage.to.ImageStoreTO;
> +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
> import org.apache.cloudstack.storage.to.SnapshotObjectTO;
> import org.apache.cloudstack.storage.to.TemplateObjectTO;
> import org.apache.cloudstack.storage.to.VolumeObjectTO;
> @@ -88,6 +87,14 @@ import
> com.cloud.agent.api.SecStorageSetupCommand.Certificates;
> import com.cloud.agent.api.SecStorageVMSetupCommand;
> import com.cloud.agent.api.StartupCommand;
> import com.cloud.agent.api.StartupSecondaryStorageCommand;
> +import com.cloud.agent.api.storage.CreateEntityDownloadURLCommand;
> +import com.cloud.agent.api.storage.DeleteEntityDownloadURLCommand;
> +import com.cloud.agent.api.storage.DownloadAnswer;
> +import com.cloud.agent.api.storage.ListTemplateAnswer;
> +import com.cloud.agent.api.storage.ListTemplateCommand;
> +import com.cloud.agent.api.storage.ListVolumeAnswer;
> +import com.cloud.agent.api.storage.ListVolumeCommand;
> +import com.cloud.agent.api.storage.UploadCommand;
> import com.cloud.agent.api.to.DataObjectType;
> import com.cloud.agent.api.to.DataStoreTO;
> import com.cloud.agent.api.to.DataTO;
> @@ -101,16 +108,34 @@ import
> com.cloud.hypervisor.Hypervisor.HypervisorType;
> import com.cloud.resource.ServerResourceBase;
> import com.cloud.storage.DataStoreRole;
> import com.cloud.storage.Storage.ImageFormat;
> +import com.cloud.storage.Storage.StoragePoolType;
> import com.cloud.storage.StorageLayer;
> +import com.cloud.storage.VMTemplateStorageResourceAssoc;
> +import com.cloud.storage.template.Processor;
> import com.cloud.storage.template.Processor.FormatInfo;
> +import com.cloud.storage.template.QCOW2Processor;
> +import com.cloud.storage.template.RawImageProcessor;
> +import com.cloud.storage.template.TemplateLocation;
> +import com.cloud.storage.template.TemplateProp;
> +import com.cloud.storage.template.VhdProcessor;
> +import com.cloud.storage.template.VmdkProcessor;
> import com.cloud.utils.NumbersUtil;
> import com.cloud.utils.S3Utils;
> import com.cloud.utils.S3Utils.FileNamingStrategy;
> +import com.cloud.utils.SwiftUtil;
> import com.cloud.utils.exception.CloudRuntimeException;
> import com.cloud.utils.net.NetUtils;
> import com.cloud.utils.script.OutputInterpreter;
> import com.cloud.utils.script.Script;
> import com.cloud.vm.SecondaryStorageVm;
> +import com.google.common.io.Files;
> +
> +import static com.cloud.utils.S3Utils.putFile;
> +import static com.cloud.utils.StringUtils.join;
> +import static com.cloud.utils.db.GlobalLock.executeWithNoWaitLock;
> +import static java.lang.String.format;
> +import static java.util.Arrays.asList;
> +import static org.apache.commons.lang.StringUtils.substringAfterLast;
>
> public class NfsSecondaryStorageResource extends ServerResourceBase
> implements SecondaryStorageResource {
>
> @@ -357,6 +382,7 @@ public class NfsSecondaryStorageResource extends
> ServerResourceBase implements S
> snapshotName = snapshotName + ".vhd";
> }
> snapshotPath = snapshotPath.substring(0, index);
> +
> snapshotPath = srcMountPoint + File.separator + snapshotPath;
> String destMountPoint = this.getRootDir(destDataStore.getUrl());
> String destPath = destMountPoint + File.separator +
> destData.getPath();
> @@ -424,7 +450,7 @@ public class NfsSecondaryStorageResource extends
> ServerResourceBase implements S
>
> // get snapshot file name
> String templateName = srcFile.getName();
> - // add kvm file extension for copied template name
> + // add kvm file extension for copied template name
> String fileName = templateName + "." +
> srcFormat.getFileExtension();
> String destFileFullPath = destFile.getAbsolutePath() +
> File.separator + fileName;
> s_logger.debug("copy snapshot " + srcFile.getAbsolutePath() +
> " to template " + destFileFullPath);
> @@ -509,15 +535,16 @@ public class NfsSecondaryStorageResource extends
> ServerResourceBase implements S
> DataTO destData = cmd.getDestTO();
> DataStoreTO srcDataStore = srcData.getDataStore();
> DataStoreTO destDataStore = destData.getDataStore();
> - if (srcDataStore.getRole() == DataStoreRole.Image ||
> srcDataStore.getRole() == DataStoreRole.ImageCache) {
> + if (srcDataStore.getRole() == DataStoreRole.Image ||
> srcDataStore.getRole() == DataStoreRole.ImageCache ||
> + srcDataStore.getRole() == DataStoreRole.Primary) {
> if (!(srcDataStore instanceof NfsTO)) {
> s_logger.debug("only support nfs storage as src, when
> create template from snapshot");
> return Answer.createUnsupportedCommandAnswer(cmd);
> }
>
> if (destDataStore instanceof NfsTO) {
> - return copySnapshotToTemplateFromNfsToNfs(cmd,
> (SnapshotObjectTO) srcData, (NfsTO) srcDataStore,
> - (TemplateObjectTO) destData, (NfsTO)
> destDataStore);
> + return copySnapshotToTemplateFromNfsToNfs(cmd,
> (SnapshotObjectTO) srcData, (NfsTO)srcDataStore,
> + (TemplateObjectTO) destData,
> (NfsTO)destDataStore);
> } else if (destDataStore instanceof SwiftTO) {
> //create template on the same data store
> CopyCmdAnswer answer =
> (CopyCmdAnswer)copySnapshotToTemplateFromNfsToNfs(cmd, (SnapshotObjectTO)
> srcData, (NfsTO) srcDataStore,
> @@ -543,8 +570,8 @@ public class NfsSecondaryStorageResource extends
> ServerResourceBase implements S
> execute(deleteCommand);
> } catch (Exception e) {
> s_logger.debug("Failed to clean up staging area:", e);
> - }
> -
> + }
> +
> TemplateObjectTO template = new TemplateObjectTO();
> template.setPath(swiftPath);
> template.setSize(templateFile.length());
> @@ -569,7 +596,7 @@ public class NfsSecondaryStorageResource extends
> ServerResourceBase implements S
> execute(deleteCommand);
> } catch (Exception e) {
> s_logger.debug("Failed to clean up staging area:", e);
> - }
> + }
> return result;
> }
> }
> @@ -792,7 +819,7 @@ public class NfsSecondaryStorageResource extends
> ServerResourceBase implements S
> processor.configure("template processor", new HashMap<String,
> Object>());
> return processor.getVirtualSize(file);
> } catch (Exception e) {
> - s_logger.debug("Failed to get virtual size:" ,e);
> + s_logger.debug("Failed to get virtual size:" ,e);
> }
> return file.length();
> }
> @@ -2226,8 +2253,8 @@ public class NfsSecondaryStorageResource extends
> ServerResourceBase implements S
> *
> * CIFS parameters are documented with mount.cifs at
> * http://linux.die.net/man/8/mount.cifs
> - * For simplicity, when a URI is used to specify a CIFS share,
> - * options such as domain,user,password are passed as query
> parameters.
> + * For simplicity, when a URI is used to specify a CIFS share,
> + * options such as domain,user,password are passed as query
> parameters.
> *
> * @param uri
> * crresponding to the remote device. Will throw for
> unsupported
> @@ -2262,7 +2289,7 @@ public class NfsSecondaryStorageResource extends
> ServerResourceBase implements S
> return dir;
> }
>
> -
> +
> protected void umount(String localRootPath, URI uri) {
> ensureLocalRootPathExists(localRootPath, uri);
>
> @@ -2286,7 +2313,7 @@ public class NfsSecondaryStorageResource extends
> ServerResourceBase implements S
> }
> s_logger.debug("Successfully umounted " + localRootPath);
> }
> -
> +
> protected void mount(String localRootPath, String remoteDevice, URI
> uri) {
> s_logger.debug("mount " + uri.toString() + " on " +
> localRootPath);
> ensureLocalRootPathExists(localRootPath, uri);
>
>
>