You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2018/01/04 10:38:08 UTC
[GitHub] rhtyd closed pull request #2139: CLOUDSTACK-9921: NPE when storage garbage collector is running.
rhtyd closed pull request #2139: CLOUDSTACK-9921: NPE when storage garbage collector is running.
URL: https://github.com/apache/cloudstack/pull/2139
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/engine/schema/src/com/cloud/storage/dao/SnapshotDao.java b/engine/schema/src/com/cloud/storage/dao/SnapshotDao.java
index fe635586370..1c11f9b6180 100755
--- a/engine/schema/src/com/cloud/storage/dao/SnapshotDao.java
+++ b/engine/schema/src/com/cloud/storage/dao/SnapshotDao.java
@@ -61,4 +61,6 @@
void updateVolumeIds(long oldVolId, long newVolId);
+ List<SnapshotVO> listByStatusNotIn(long volumeId, Snapshot.State... status);
+
}
diff --git a/engine/schema/src/com/cloud/storage/dao/SnapshotDaoImpl.java b/engine/schema/src/com/cloud/storage/dao/SnapshotDaoImpl.java
index a6941cf5165..560edc93816 100755
--- a/engine/schema/src/com/cloud/storage/dao/SnapshotDaoImpl.java
+++ b/engine/schema/src/com/cloud/storage/dao/SnapshotDaoImpl.java
@@ -69,6 +69,7 @@
private SearchBuilder<SnapshotVO> AccountIdSearch;
private SearchBuilder<SnapshotVO> InstanceIdSearch;
private SearchBuilder<SnapshotVO> StatusSearch;
+ private SearchBuilder<SnapshotVO> notInStatusSearch;
private GenericSearchBuilder<SnapshotVO, Long> CountSnapshotsByAccount;
@Inject
ResourceTagDao _tagsDao;
@@ -187,6 +188,11 @@ protected void init() {
StatusSearch.and("status", StatusSearch.entity().getState(), SearchCriteria.Op.IN);
StatusSearch.done();
+ notInStatusSearch = createSearchBuilder();
+ notInStatusSearch.and("volumeId", notInStatusSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
+ notInStatusSearch.and("status", notInStatusSearch.entity().getState(), SearchCriteria.Op.NOTIN);
+ notInStatusSearch.done();
+
CountSnapshotsByAccount = createSearchBuilder(Long.class);
CountSnapshotsByAccount.select(null, Func.COUNT, null);
CountSnapshotsByAccount.and("account", CountSnapshotsByAccount.entity().getAccountId(), SearchCriteria.Op.EQ);
@@ -352,4 +358,12 @@ public void updateVolumeIds(long oldVolId, long newVolId) {
UpdateBuilder ub = getUpdateBuilder(snapshot);
update(ub, sc, null);
}
+
+ @Override
+ public List<SnapshotVO> listByStatusNotIn(long volumeId, Snapshot.State... status) {
+ SearchCriteria<SnapshotVO> sc = this.notInStatusSearch.create();
+ sc.setParameters("volumeId", volumeId);
+ sc.setParameters("status", (Object[]) status);
+ return listBy(sc, null);
+ }
}
diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java
index c74507cf85a..85bed63c79b 100755
--- a/server/src/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/com/cloud/vm/UserVmManagerImpl.java
@@ -225,6 +225,7 @@
import com.cloud.storage.GuestOSVO;
import com.cloud.storage.SnapshotVO;
import com.cloud.storage.Storage;
+import com.cloud.storage.Snapshot;
import com.cloud.storage.Storage.ImageFormat;
import com.cloud.storage.Storage.TemplateType;
import com.cloud.storage.StorageManager;
@@ -5327,11 +5328,20 @@ public UserVm moveVMToUser(final AssignVMCmd cmd) throws ResourceAllocationExcep
}
}
+ final List<VolumeVO> volumes = _volsDao.findByInstance(cmd.getVmId());
+
+ for (VolumeVO volume : volumes) {
+ List<SnapshotVO> snapshots = _snapshotDao.listByStatusNotIn(volume.getId(), Snapshot.State.Destroyed,Snapshot.State.Error);
+ if (snapshots != null && snapshots.size() > 0) {
+ throw new InvalidParameterValueException(
+ "Snapshots exists for volume: "+ volume.getName()+ ", Detach volume or remove snapshots for volume before assigning VM to another user.");
+ }
+ }
+
DataCenterVO zone = _dcDao.findById(vm.getDataCenterId());
// Get serviceOffering and Volumes for Virtual Machine
final ServiceOfferingVO offering = _serviceOfferingDao.findByIdIncludingRemoved(vm.getId(), vm.getServiceOfferingId());
- final List<VolumeVO> volumes = _volsDao.findByInstance(cmd.getVmId());
//Remove vm from instance group
removeInstanceFromInstanceGroup(cmd.getVmId());
@@ -5386,16 +5396,6 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
_resourceLimitMgr.incrementResourceCount(newAccount.getAccountId(), ResourceType.primary_storage, new Long(volume.getSize()));
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_CREATE, volume.getAccountId(), volume.getDataCenterId(), volume.getId(), volume.getName(),
volume.getDiskOfferingId(), volume.getTemplateId(), volume.getSize(), Volume.class.getName(), volume.getUuid(), volume.isDisplayVolume());
- //snapshots: mark these removed in db
- List<SnapshotVO> snapshots = _snapshotDao.listByVolumeIdIncludingRemoved(volume.getId());
- for (SnapshotVO snapshot : snapshots) {
- boolean result = _snapshotService.deleteSnapshot(snapshot.getId());
- if (result) {
- s_logger.info("Snapshot id: " + snapshot.getId() + " delete successfully ");
- } else {
- s_logger.error("Unable to delete Snapshot id: " + snapshot.getId());
- }
- }
}
//update resource count of new account
diff --git a/test/integration/component/test_assign_vm.py b/test/integration/component/test_assign_vm.py
index 466fadfc735..73088585958 100644
--- a/test/integration/component/test_assign_vm.py
+++ b/test/integration/component/test_assign_vm.py
@@ -112,6 +112,7 @@ def setUpClass(cls):
cls.services["ostype"])
cls.services["virtual_machine"]["zoneid"] = cls.zone.id
cls.services["virtual_machine"]["template"] = cls.template.id
+ cls.hypervisor = cls.testClient.getHypervisorInfo()
def create_domain_account_user(parentDomain=None):
domain = Domain.create(cls.api_client,
@@ -415,17 +416,13 @@ def test_13_move_across_subdomain_vm_snapshot(self):
"""
# Validate the following:
# 1. deploy VM in sub subdomain1 with snapshot.
- # 3. assignVirtualMachine to subdomain2
+ # 3. assign VM will fail with Exception as VM with snapshots cannot be assigned to other users.
if self.hypervisor.lower() in ['hyperv', 'lxc']:
self.skipTest("Snapshots feature is not supported on %s" % self.hypervisor)
self.create_vm(self.sdomain_account_user1['account'], self.sdomain_account_user1['domain'], snapshot=True)
- self.virtual_machine.assign_virtual_machine(self.apiclient, self.sdomain_account_user2['account'].name ,self.sdomain_account_user2['domain'].id)
- snapshots = list_snapshots(self.apiclient,
- id=self.snapshot.id)
- self.assertEqual(snapshots,
- None,
- "Snapshots stil present for a vm in domain")
-
+ with self.assertRaises(Exception):
+ self.virtual_machine.assign_virtual_machine(self.apiclient, self.sdomain_account_user2['account'].name ,self.sdomain_account_user2['domain'].id)
+
@attr(tags = ["advanced"])
@log_test_exceptions
def test_14_move_across_subdomain_vm_project(self):
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services