You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by mike-tutkowski <gi...@git.apache.org> on 2016/09/02 21:58:16 UTC

[GitHub] cloudstack pull request #1671: Adding support for cross-cluster storage migr...

GitHub user mike-tutkowski opened a pull request:

    https://github.com/apache/cloudstack/pull/1671

    Adding support for cross-cluster storage migration for managed storage when using XenServer

    This PR adds support for cross-cluster storage migration of VMs that make use of managed storage with XenServer.
    
    Managed storage is when you have a 1:1 mapping between a virtual disk and a volume on a SAN (in the case of XenServer, an SR is placed on this SAN volume and a single virtual disk placed in the SR).
    
    Managed storage allows features such as storage QoS and SAN-side snapshots to work (sort of analogous to VMware VVols).
    
    This PR focuses on enabling VMs that are using managed storage to be migrated across XenServer clusters.
    
    I have successfully run the following tests on this branch:
    
    TestVolumes.py
    TestSnapshots.py
    TestVMSnapshots.py
    TestAddRemoveHosts.py
    TestVMMigrationWithStorage.py (which is a new test that is being added with this PR)


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mike-tutkowski/cloudstack copy-vol-migration

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/1671.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1671
    
----

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1671: Adding support for cross-cluster storage migr...

Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1671#discussion_r77635744
  
    --- Diff: plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixDeleteStoragePoolCommandWrapper.java ---
    @@ -32,22 +34,40 @@
     
     @ResourceWrapper(handles =  DeleteStoragePoolCommand.class)
     public final class CitrixDeleteStoragePoolCommandWrapper extends CommandWrapper<DeleteStoragePoolCommand, Answer, CitrixResourceBase> {
    -
         private static final Logger s_logger = Logger.getLogger(CitrixDeleteStoragePoolCommandWrapper.class);
     
         @Override
         public Answer execute(final DeleteStoragePoolCommand command, final CitrixResourceBase citrixResourceBase) {
             final Connection conn = citrixResourceBase.getConnection();
             final StorageFilerTO poolTO = command.getPool();
    +
             try {
    -            final SR sr = citrixResourceBase.getStorageRepository(conn, poolTO.getUuid());
    +            final SR sr;
    +
    +            // getRemoveDatastore being true indicates we are using managed storage and need to pull the SR name out of a Map
    +            // instead of pulling it out using getUuid of the StorageFilerTO instance.
    +            if (command.getRemoveDatastore()) {
    +                Map<String, String> details = command.getDetails();
    +
    +                String srNameLabel = details.get(DeleteStoragePoolCommand.DATASTORE_NAME);
    +
    +                sr = citrixResourceBase.getStorageRepository(conn, srNameLabel);
    +            }
    +            else {
    +                sr = citrixResourceBase.getStorageRepository(conn, poolTO.getUuid());
    +            }
    +
                 citrixResourceBase.removeSR(conn, sr);
    +
                 final Answer answer = new Answer(command, true, "success");
    +
                 return answer;
             } catch (final Exception e) {
    -            final String msg = "DeleteStoragePoolCommand XenAPIException:" + e.getMessage() + " host:" + citrixResourceBase.getHost().getUuid() + " pool: " + poolTO.getHost()
    -                    + poolTO.getPath();
    +            final String msg = "DeleteStoragePoolCommand XenAPIException:" + e.getMessage() + " host:" + citrixResourceBase.getHost().getUuid() +
    +                    " pool: " + poolTO.getHost() + poolTO.getPath();
    +
                 s_logger.warn(msg, e);
    --- End diff --
    
    Sure, sounds good.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1671: Adding support for cross-cluster storage migration f...

Posted by syed <gi...@git.apache.org>.
Github user syed commented on the issue:

    https://github.com/apache/cloudstack/pull/1671
  
    @mike-tutkowski I've reviewed the code. it LGTM, my only concern is of migrating between two different managed storage when they don't have same tags (which would ideally be the default)
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1671: Adding support for cross-cluster storage migr...

Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1671#discussion_r77631682
  
    --- Diff: plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixDeleteStoragePoolCommandWrapper.java ---
    @@ -32,22 +34,40 @@
     
     @ResourceWrapper(handles =  DeleteStoragePoolCommand.class)
     public final class CitrixDeleteStoragePoolCommandWrapper extends CommandWrapper<DeleteStoragePoolCommand, Answer, CitrixResourceBase> {
    -
         private static final Logger s_logger = Logger.getLogger(CitrixDeleteStoragePoolCommandWrapper.class);
     
         @Override
         public Answer execute(final DeleteStoragePoolCommand command, final CitrixResourceBase citrixResourceBase) {
             final Connection conn = citrixResourceBase.getConnection();
             final StorageFilerTO poolTO = command.getPool();
    +
             try {
    -            final SR sr = citrixResourceBase.getStorageRepository(conn, poolTO.getUuid());
    +            final SR sr;
    +
    +            // getRemoveDatastore being true indicates we are using managed storage and need to pull the SR name out of a Map
    +            // instead of pulling it out using getUuid of the StorageFilerTO instance.
    +            if (command.getRemoveDatastore()) {
    +                Map<String, String> details = command.getDetails();
    +
    +                String srNameLabel = details.get(DeleteStoragePoolCommand.DATASTORE_NAME);
    +
    +                sr = citrixResourceBase.getStorageRepository(conn, srNameLabel);
    +            }
    +            else {
    +                sr = citrixResourceBase.getStorageRepository(conn, poolTO.getUuid());
    +            }
    +
                 citrixResourceBase.removeSR(conn, sr);
    +
                 final Answer answer = new Answer(command, true, "success");
    +
                 return answer;
             } catch (final Exception e) {
    -            final String msg = "DeleteStoragePoolCommand XenAPIException:" + e.getMessage() + " host:" + citrixResourceBase.getHost().getUuid() + " pool: " + poolTO.getHost()
    -                    + poolTO.getPath();
    +            final String msg = "DeleteStoragePoolCommand XenAPIException:" + e.getMessage() + " host:" + citrixResourceBase.getHost().getUuid() +
    +                    " pool: " + poolTO.getHost() + poolTO.getPath();
    +
                 s_logger.warn(msg, e);
    --- End diff --
    
    Can we change this `warn` to an `error`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1671: Adding support for cross-cluster storage migr...

Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1671#discussion_r77634349
  
    --- Diff: engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java ---
    @@ -25,6 +25,13 @@
     import com.cloud.storage.StoragePool;
     
     public interface PrimaryDataStoreDriver extends DataStoreDriver {
    +    String BASIC_CREATE = "basicCreate";
    +    String BASIC_DELETE = "basicDelete";
    +    String BASIC_DELETE_FAILURE = "basicDeleteFailure";
    +    String BASIC_GRANT_ACCESS = "basicGrantAccess";
    +    String BASIC_REVOKE_ACCESS = "basicRevokeAccess";
    +    String BASIC_IQN = "basicIqn";
    --- End diff --
    
    These are like what we did with snapshots and cloning in a previous PR: Ways of passing info into the storage driver to have it perform actions that aren't covered by the current interface.
    
    For example, to make this PR work, we need to have two backend volumes exist for the same CloudStack volume simultaneously (during the copy of the data).
    
    "basicCreate" in volume_details for a given CloudStack volume ID tells the storage driver to simply create a volume on the backend with the same characteristics as the existing volume on the backend (for this CloudStack volume).
    
    We then communicate info back to the storage framework (for XenMotion/vMotion purposes) by adding "basicIqn" to the volume details.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1671: Adding support for cross-cluster storage migr...

Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1671#discussion_r77634892
  
    --- Diff: engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java ---
    @@ -2045,62 +2045,74 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy
     
         private Map<Volume, StoragePool> getPoolListForVolumesForMigration(final VirtualMachineProfile profile, final Host host, final Map<Long, Long> volumeToPool) {
             final List<VolumeVO> allVolumes = _volsDao.findUsableVolumesForInstance(profile.getId());
    -        final Map<Volume, StoragePool> volumeToPoolObjectMap = new HashMap<Volume, StoragePool> ();
    +        final Map<Volume, StoragePool> volumeToPoolObjectMap = new HashMap<>();
    +
             for (final VolumeVO volume : allVolumes) {
                 final Long poolId = volumeToPool.get(Long.valueOf(volume.getId()));
    -            final StoragePoolVO pool = _storagePoolDao.findById(poolId);
    +            final StoragePoolVO destPool = _storagePoolDao.findById(poolId);
                 final StoragePoolVO currentPool = _storagePoolDao.findById(volume.getPoolId());
                 final DiskOfferingVO diskOffering = _diskOfferingDao.findById(volume.getDiskOfferingId());
    -            if (pool != null) {
    +
    +            if (destPool != null) {
                     // Check if pool is accessible from the destination host and disk offering with which the volume was
                     // created is compliant with the pool type.
    -                if (_poolHostDao.findByPoolHost(pool.getId(), host.getId()) == null || pool.isLocal() != diskOffering.getUseLocalStorage()) {
    +                if (_poolHostDao.findByPoolHost(destPool.getId(), host.getId()) == null || destPool.isLocal() != diskOffering.getUseLocalStorage()) {
                         // Cannot find a pool for the volume. Throw an exception.
    -                    throw new CloudRuntimeException("Cannot migrate volume " + volume + " to storage pool " + pool + " while migrating vm to host " + host +
    +                    throw new CloudRuntimeException("Cannot migrate volume " + volume + " to storage pool " + destPool + " while migrating vm to host " + host +
                                 ". Either the pool is not accessible from the host or because of the offering with which the volume is created it cannot be placed on " +
                                 "the given pool.");
    -                } else if (pool.getId() == currentPool.getId()) {
    -                    // If the pool to migrate too is the same as current pool, the volume doesn't need to be migrated.
    +                } else if (destPool.getId() == currentPool.getId()) {
    +                    // If the pool to migrate to is the same as current pool, the volume doesn't need to be migrated.
                     } else {
    -                    volumeToPoolObjectMap.put(volume, pool);
    +                    volumeToPoolObjectMap.put(volume, destPool);
                     }
                 } else {
    -                // Find a suitable pool for the volume. Call the storage pool allocator to find the list of pools.
    -                final DiskProfile diskProfile = new DiskProfile(volume, diskOffering, profile.getHypervisorType());
    -                final DataCenterDeployment plan = new DataCenterDeployment(host.getDataCenterId(), host.getPodId(), host.getClusterId(), host.getId(), null, null);
    -                final ExcludeList avoid = new ExcludeList();
    -                boolean currentPoolAvailable = false;
    -
    -                final List<StoragePool> poolList = new ArrayList<StoragePool>();
    -                for (final StoragePoolAllocator allocator : _storagePoolAllocators) {
    -                    final List<StoragePool> poolListFromAllocator = allocator.allocateToPool(diskProfile, profile, plan, avoid, StoragePoolAllocator.RETURN_UPTO_ALL);
    -                    if (poolListFromAllocator != null && !poolListFromAllocator.isEmpty()) {
    -                        poolList.addAll(poolListFromAllocator);
    -                    }
    -                }
    +                if (currentPool.isManaged()) {
    +                    volumeToPoolObjectMap.put(volume, currentPool);
    +                } else {
    +                    // Find a suitable pool for the volume. Call the storage pool allocator to find the list of pools.
     
    -                if (poolList != null && !poolList.isEmpty()) {
    -                    // Volume needs to be migrated. Pick the first pool from the list. Add a mapping to migrate the
    -                    // volume to a pool only if it is required; that is the current pool on which the volume resides
    -                    // is not available on the destination host.
    -                    final Iterator<StoragePool> iter = poolList.iterator();
    -                    while (iter.hasNext()) {
    -                        if (currentPool.getId() == iter.next().getId()) {
    -                            currentPoolAvailable = true;
    -                            break;
    +                    final DiskProfile diskProfile = new DiskProfile(volume, diskOffering, profile.getHypervisorType());
    +                    final DataCenterDeployment plan = new DataCenterDeployment(host.getDataCenterId(), host.getPodId(), host.getClusterId(), host.getId(), null, null);
    --- End diff --
    
    For this PR, the limitation is not just storage tagging, but also the src and dest have to be the same zone-wide primary storage.
    
    I know you and I have another task on our shared to-do list to enable cross-primary storage migration (where managed storage is one or both of the primary storages). This, however, does not implement that functionality.
    
    This PR just enables Storage XenMotion/vMotion from one cluster to another where managed storage is involved (and the data effectively stays on the same primary storage).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1671: Adding support for cross-cluster storage migration f...

Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the issue:

    https://github.com/apache/cloudstack/pull/1671
  
    the failed test(test_extendPhysicalNetworkVlan) is not due to this PR. Its a test case cleanup issue. This PR looks good.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1671: Adding support for cross-cluster storage migr...

Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1671#discussion_r77626328
  
    --- Diff: engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java ---
    @@ -25,6 +25,13 @@
     import com.cloud.storage.StoragePool;
     
     public interface PrimaryDataStoreDriver extends DataStoreDriver {
    +    String BASIC_CREATE = "basicCreate";
    +    String BASIC_DELETE = "basicDelete";
    +    String BASIC_DELETE_FAILURE = "basicDeleteFailure";
    +    String BASIC_GRANT_ACCESS = "basicGrantAccess";
    +    String BASIC_REVOKE_ACCESS = "basicRevokeAccess";
    +    String BASIC_IQN = "basicIqn";
    --- End diff --
    
    @mike-tutkowski What are basic operations? are they different from normal operations? 
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1671: Adding support for cross-cluster storage migr...

Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1671#discussion_r77628932
  
    --- Diff: engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java ---
    @@ -2045,62 +2045,74 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy
     
         private Map<Volume, StoragePool> getPoolListForVolumesForMigration(final VirtualMachineProfile profile, final Host host, final Map<Long, Long> volumeToPool) {
             final List<VolumeVO> allVolumes = _volsDao.findUsableVolumesForInstance(profile.getId());
    -        final Map<Volume, StoragePool> volumeToPoolObjectMap = new HashMap<Volume, StoragePool> ();
    +        final Map<Volume, StoragePool> volumeToPoolObjectMap = new HashMap<>();
    +
             for (final VolumeVO volume : allVolumes) {
                 final Long poolId = volumeToPool.get(Long.valueOf(volume.getId()));
    -            final StoragePoolVO pool = _storagePoolDao.findById(poolId);
    +            final StoragePoolVO destPool = _storagePoolDao.findById(poolId);
                 final StoragePoolVO currentPool = _storagePoolDao.findById(volume.getPoolId());
                 final DiskOfferingVO diskOffering = _diskOfferingDao.findById(volume.getDiskOfferingId());
    -            if (pool != null) {
    +
    +            if (destPool != null) {
                     // Check if pool is accessible from the destination host and disk offering with which the volume was
                     // created is compliant with the pool type.
    -                if (_poolHostDao.findByPoolHost(pool.getId(), host.getId()) == null || pool.isLocal() != diskOffering.getUseLocalStorage()) {
    +                if (_poolHostDao.findByPoolHost(destPool.getId(), host.getId()) == null || destPool.isLocal() != diskOffering.getUseLocalStorage()) {
                         // Cannot find a pool for the volume. Throw an exception.
    -                    throw new CloudRuntimeException("Cannot migrate volume " + volume + " to storage pool " + pool + " while migrating vm to host " + host +
    +                    throw new CloudRuntimeException("Cannot migrate volume " + volume + " to storage pool " + destPool + " while migrating vm to host " + host +
                                 ". Either the pool is not accessible from the host or because of the offering with which the volume is created it cannot be placed on " +
                                 "the given pool.");
    -                } else if (pool.getId() == currentPool.getId()) {
    -                    // If the pool to migrate too is the same as current pool, the volume doesn't need to be migrated.
    +                } else if (destPool.getId() == currentPool.getId()) {
    +                    // If the pool to migrate to is the same as current pool, the volume doesn't need to be migrated.
                     } else {
    -                    volumeToPoolObjectMap.put(volume, pool);
    +                    volumeToPoolObjectMap.put(volume, destPool);
                     }
                 } else {
    -                // Find a suitable pool for the volume. Call the storage pool allocator to find the list of pools.
    -                final DiskProfile diskProfile = new DiskProfile(volume, diskOffering, profile.getHypervisorType());
    -                final DataCenterDeployment plan = new DataCenterDeployment(host.getDataCenterId(), host.getPodId(), host.getClusterId(), host.getId(), null, null);
    -                final ExcludeList avoid = new ExcludeList();
    -                boolean currentPoolAvailable = false;
    -
    -                final List<StoragePool> poolList = new ArrayList<StoragePool>();
    -                for (final StoragePoolAllocator allocator : _storagePoolAllocators) {
    -                    final List<StoragePool> poolListFromAllocator = allocator.allocateToPool(diskProfile, profile, plan, avoid, StoragePoolAllocator.RETURN_UPTO_ALL);
    -                    if (poolListFromAllocator != null && !poolListFromAllocator.isEmpty()) {
    -                        poolList.addAll(poolListFromAllocator);
    -                    }
    -                }
    +                if (currentPool.isManaged()) {
    +                    volumeToPoolObjectMap.put(volume, currentPool);
    +                } else {
    +                    // Find a suitable pool for the volume. Call the storage pool allocator to find the list of pools.
     
    -                if (poolList != null && !poolList.isEmpty()) {
    -                    // Volume needs to be migrated. Pick the first pool from the list. Add a mapping to migrate the
    -                    // volume to a pool only if it is required; that is the current pool on which the volume resides
    -                    // is not available on the destination host.
    -                    final Iterator<StoragePool> iter = poolList.iterator();
    -                    while (iter.hasNext()) {
    -                        if (currentPool.getId() == iter.next().getId()) {
    -                            currentPoolAvailable = true;
    -                            break;
    +                    final DiskProfile diskProfile = new DiskProfile(volume, diskOffering, profile.getHypervisorType());
    +                    final DataCenterDeployment plan = new DataCenterDeployment(host.getDataCenterId(), host.getPodId(), host.getClusterId(), host.getId(), null, null);
    --- End diff --
    
    @mike-tutkowski is the assumption here that both `src` and `dest` volume pools are the same/use the same tags? Would migration between two different primary managed storage using different tags be possible? 
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1671: Adding support for cross-cluster storage migration f...

Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:

    https://github.com/apache/cloudstack/pull/1671
  
    Also, here's a (roughly) 30-minute video I made that demonstrates this PR in action: https://www.youtube.com/watch?v=_pbnvQxTAqw&list=PLqOXKM0Bt13DFnQnwUx8ZtJzoyDV0Uuye&index=17


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1671: Adding support for cross-cluster storage migration f...

Posted by karuturi <gi...@git.apache.org>.
Github user karuturi commented on the issue:

    https://github.com/apache/cloudstack/pull/1671
  
    @mike-tutkowski I started our internal CI run on this to run the smoke test suite. It will post the results here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1671: Adding support for cross-cluster storage migration f...

Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on the issue:

    https://github.com/apache/cloudstack/pull/1671
  
    Thank you, Rajani!
    
    On Sep 12, 2016, at 11:41 PM, Rajani Karuturi <no...@github.com>> wrote:
    
    
    @mike-tutkowski<https://github.com/mike-tutkowski> I started our internal CI run on this to run the smoke test suite. It will post the results here.
    
    -
    You are receiving this because you were mentioned.
    Reply to this email directly, view it on GitHub<https://github.com/apache/cloudstack/pull/1671#issuecomment-246580106>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AC4SH8b7J7y-wbzJsCkaSvWHEw65PL0jks5qpjd6gaJpZM4J0HsP>.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1671: Adding support for cross-cluster storage migr...

Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1671#discussion_r77635639
  
    --- Diff: plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixCreateStoragePoolCommandWrapper.java ---
    @@ -39,20 +41,35 @@
         public Answer execute(final CreateStoragePoolCommand command, final CitrixResourceBase citrixResourceBase) {
             final Connection conn = citrixResourceBase.getConnection();
             final StorageFilerTO pool = command.getPool();
    +
             try {
    -            if (pool.getType() == StoragePoolType.NetworkFilesystem) {
    -                citrixResourceBase.getNfsSR(conn, Long.toString(pool.getId()), pool.getUuid(), pool.getHost(), pool.getPath(), pool.toString());
    -            } else if (pool.getType() == StoragePoolType.IscsiLUN) {
    -                citrixResourceBase.getIscsiSR(conn, pool.getUuid(), pool.getHost(), pool.getPath(), null, null, false);
    -            } else if (pool.getType() == StoragePoolType.PreSetup) {
    -            } else {
    -                return new Answer(command, false, "The pool type: " + pool.getType().name() + " is not supported.");
    +            if (command.getCreateDatastore()) {
    --- End diff --
    
    Yes, CloudStack does allow an admin to specify an IQN (and other storage info) when adding primary storage that leads to the creation of an SR for you.
    
    To my knowledge, this is not the case on, say, ESXi/vSphere, though. In that case, you have to create your datastore before adding it to CloudStack as primary storage.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1671: Adding support for cross-cluster storage migr...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cloudstack/pull/1671


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1671: Adding support for cross-cluster storage migr...

Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1671#discussion_r77629961
  
    --- Diff: plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xen610/XenServer610MigrateWithStorageReceiveCommandWrapper.java ---
    @@ -56,7 +55,7 @@
         public Answer execute(final MigrateWithStorageReceiveCommand command, final XenServer610Resource xenServer610Resource) {
             final Connection connection = xenServer610Resource.getConnection();
             final VirtualMachineTO vmSpec = command.getVirtualMachine();
    -        final List<Pair<VolumeTO, StorageFilerTO>> volumeToFiler = command.getVolumeToFiler();
    +        final List<Pair<VolumeTO, String>> volumeToStorageUuid = command.getVolumeToStorageUuid();
    --- End diff --
    
    @mike-tutkowski whould changing the map from `StorageFilerTO` to `String` change how other hypervisors like `KVM` would implement this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1671: Adding support for cross-cluster storage migr...

Posted by mike-tutkowski <gi...@git.apache.org>.
Github user mike-tutkowski commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1671#discussion_r77635261
  
    --- Diff: plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xen610/XenServer610MigrateWithStorageReceiveCommandWrapper.java ---
    @@ -56,7 +55,7 @@
         public Answer execute(final MigrateWithStorageReceiveCommand command, final XenServer610Resource xenServer610Resource) {
             final Connection connection = xenServer610Resource.getConnection();
             final VirtualMachineTO vmSpec = command.getVirtualMachine();
    -        final List<Pair<VolumeTO, StorageFilerTO>> volumeToFiler = command.getVolumeToFiler();
    +        final List<Pair<VolumeTO, String>> volumeToStorageUuid = command.getVolumeToStorageUuid();
    --- End diff --
    
    I checked where this was being accessed from throughout the codebase and all hypervisors should be OK with this change.
    
    They all seemed to only care about a string UUID in the StorageFilerTO object.
    
    This PR is only for XenMotion and vMotion. This PR does not enabled cross-cluster storage migration (for managed storage) on any other hypervisor(s) in CloudStack.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack pull request #1671: Adding support for cross-cluster storage migr...

Posted by syed <gi...@git.apache.org>.
Github user syed commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/1671#discussion_r77631355
  
    --- Diff: plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixCreateStoragePoolCommandWrapper.java ---
    @@ -39,20 +41,35 @@
         public Answer execute(final CreateStoragePoolCommand command, final CitrixResourceBase citrixResourceBase) {
             final Connection conn = citrixResourceBase.getConnection();
             final StorageFilerTO pool = command.getPool();
    +
             try {
    -            if (pool.getType() == StoragePoolType.NetworkFilesystem) {
    -                citrixResourceBase.getNfsSR(conn, Long.toString(pool.getId()), pool.getUuid(), pool.getHost(), pool.getPath(), pool.toString());
    -            } else if (pool.getType() == StoragePoolType.IscsiLUN) {
    -                citrixResourceBase.getIscsiSR(conn, pool.getUuid(), pool.getHost(), pool.getPath(), null, null, false);
    -            } else if (pool.getType() == StoragePoolType.PreSetup) {
    -            } else {
    -                return new Answer(command, false, "The pool type: " + pool.getType().name() + " is not supported.");
    +            if (command.getCreateDatastore()) {
    --- End diff --
    
    can we come here for non-managed storage?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cloudstack issue #1671: Adding support for cross-cluster storage migration f...

Posted by cloudmonger <gi...@git.apache.org>.
Github user cloudmonger commented on the issue:

    https://github.com/apache/cloudstack/pull/1671
  
    ### ACS CI BVT Run
     **Sumarry:**
     Build Number 92
     Hypervisor xenserver
     NetworkType Advanced
     Passed=102
     Failed=1
     Skipped=4
    
    _Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
    
    
    **Failed tests:**
    * test_non_contigiousvlan.py
    
     * test_extendPhysicalNetworkVlan Failing since 10 runs
    
    
    **Skipped tests:**
    test_vm_nic_adapter_vmxnet3
    test_static_role_account_acls
    test_3d_gpu_support
    test_deploy_vgpu_enabled_vm
    
    **Passed test suits:**
    test_deploy_vm_with_userdata.py
    test_affinity_groups_projects.py
    test_portable_publicip.py
    test_over_provisioning.py
    test_global_settings.py
    test_scale_vm.py
    test_service_offerings.py
    test_routers_iptables_default_policy.py
    test_loadbalance.py
    test_routers.py
    test_reset_vm_on_reboot.py
    test_snapshots.py
    test_deploy_vms_with_varied_deploymentplanners.py
    test_network.py
    test_router_dns.py
    test_login.py
    test_deploy_vm_iso.py
    test_list_ids_parameter.py
    test_public_ip_range.py
    test_multipleips_per_nic.py
    test_regions.py
    test_affinity_groups.py
    test_network_acl.py
    test_pvlan.py
    test_volumes.py
    test_ssvm.py
    test_nic.py
    test_deploy_vm_root_resize.py
    test_resource_detail.py
    test_secondary_storage.py
    test_vm_life_cycle.py
    test_disk_offerings.py


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---