Skip to content

Commit 2df6802

Browse files
sureshanapartisureshanaparti
authored
Allocate new ROOT volume (on restore virtual machine operation) only when resource count increment succeeds (#8555)
* Allocate new volume on restore virtual machine operation when resource count increment succeeds - keep them in transaction, and fail operation if resource count increment fails * Added some (negative) unit tests for restore vm
1 parent f702f7f commit 2df6802

File tree

2 files changed

+234
-40
lines changed

2 files changed

+234
-40
lines changed

‎server/src/main/java/com/cloud/vm/UserVmManagerImpl.java‎

Lines changed: 60 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,7 @@
354354
import com.cloud.utils.db.SearchCriteria;
355355
import com.cloud.utils.db.Transaction;
356356
import com.cloud.utils.db.TransactionCallbackNoReturn;
357+
import com.cloud.utils.db.TransactionCallbackWithException;
357358
import com.cloud.utils.db.TransactionCallbackWithExceptionNoReturn;
358359
import com.cloud.utils.db.TransactionStatus;
359360
import com.cloud.utils.db.UUIDManager;
@@ -7765,49 +7766,68 @@ public UserVm restoreVirtualMachine(final Account caller, final long vmId, final
77657766

77667767
List<Volume> newVols = new ArrayList<>();
77677768
for (VolumeVO root : rootVols) {
7768-
if ( !Volume.State.Allocated.equals(root.getState()) || newTemplateId != null ){
7769-
Long templateId = root.getTemplateId();
7770-
boolean isISO = false;
7771-
if (templateId == null) {
7772-
// Assuming that for a vm deployed using ISO, template ID is set to NULL
7773-
isISO = true;
7774-
templateId = vm.getIsoId();
7775-
}
7776-
7777-
/* If new template/ISO is provided allocate a new volume from new template/ISO otherwise allocate new volume from original template/ISO */
7778-
Volume newVol = null;
7779-
if (newTemplateId != null) {
7780-
if (isISO) {
7781-
newVol = volumeMgr.allocateDuplicateVolume(root, null);
7782-
vm.setIsoId(newTemplateId);
7783-
vm.setGuestOSId(template.getGuestOSId());
7784-
vm.setTemplateId(newTemplateId);
7785-
} else {
7786-
newVol = volumeMgr.allocateDuplicateVolume(root, newTemplateId);
7787-
vm.setGuestOSId(template.getGuestOSId());
7788-
vm.setTemplateId(newTemplateId);
7789-
}
7790-
// check and update VM if it can be dynamically scalable with the new template
7791-
updateVMDynamicallyScalabilityUsingTemplate(vm, newTemplateId);
7792-
} else {
7793-
newVol = volumeMgr.allocateDuplicateVolume(root, null);
7794-
}
7795-
newVols.add(newVol);
7769+
if ( !Volume.State.Allocated.equals(root.getState()) || newTemplateId != null ) {
7770+
final UserVmVO userVm = vm;
7771+
Pair<UserVmVO, Volume> vmAndNewVol = Transaction.execute(new TransactionCallbackWithException<Pair<UserVmVO, Volume>, CloudRuntimeException>() {
7772+
@Override
7773+
public Pair<UserVmVO, Volume> doInTransaction(final TransactionStatus status) throws CloudRuntimeException {
7774+
Long templateId = root.getTemplateId();
7775+
boolean isISO = false;
7776+
if (templateId == null) {
7777+
// Assuming that for a vm deployed using ISO, template ID is set to NULL
7778+
isISO = true;
7779+
templateId = userVm.getIsoId();
7780+
}
7781+
7782+
/* If new template/ISO is provided allocate a new volume from new template/ISO otherwise allocate new volume from original template/ISO */
7783+
Volume newVol = null;
7784+
if (newTemplateId != null) {
7785+
if (isISO) {
7786+
newVol = volumeMgr.allocateDuplicateVolume(root, null);
7787+
userVm.setIsoId(newTemplateId);
7788+
userVm.setGuestOSId(template.getGuestOSId());
7789+
userVm.setTemplateId(newTemplateId);
7790+
} else {
7791+
newVol = volumeMgr.allocateDuplicateVolume(root, newTemplateId);
7792+
userVm.setGuestOSId(template.getGuestOSId());
7793+
userVm.setTemplateId(newTemplateId);
7794+
}
7795+
// check and update VM if it can be dynamically scalable with the new template
7796+
updateVMDynamicallyScalabilityUsingTemplate(userVm, newTemplateId);
7797+
} else {
7798+
newVol = volumeMgr.allocateDuplicateVolume(root, null);
7799+
}
7800+
newVols.add(newVol);
7801+
7802+
if (userVmDetailsDao.findDetail(userVm.getId(), VmDetailConstants.ROOT_DISK_SIZE) == null && !newVol.getSize().equals(template.getSize())) {
7803+
VolumeVO resizedVolume = (VolumeVO) newVol;
7804+
if (template.getSize() != null) {
7805+
resizedVolume.setSize(template.getSize());
7806+
_volsDao.update(resizedVolume.getId(), resizedVolume);
7807+
}
7808+
}
7809+
7810+
// 1. Save usage event and update resource count for user vm volumes
7811+
try {
7812+
_resourceLimitMgr.incrementResourceCount(newVol.getAccountId(), ResourceType.volume, newVol.isDisplay());
7813+
_resourceLimitMgr.incrementResourceCount(newVol.getAccountId(), ResourceType.primary_storage, newVol.isDisplay(), new Long(newVol.getSize()));
7814+
} catch (final CloudRuntimeException e) {
7815+
throw e;
7816+
} catch (final Exception e) {
7817+
s_logger.error("Unable to restore VM " + userVm.getUuid(), e);
7818+
throw new CloudRuntimeException(e);
7819+
}
7820+
7821+
// 2. Create Usage event for the newly created volume
7822+
UsageEventVO usageEvent = new UsageEventVO(EventTypes.EVENT_VOLUME_CREATE, newVol.getAccountId(), newVol.getDataCenterId(), newVol.getId(), newVol.getName(), newVol.getDiskOfferingId(), template.getId(), newVol.getSize());
7823+
_usageEventDao.persist(usageEvent);
77967824

7797-
if (userVmDetailsDao.findDetail(vm.getId(), VmDetailConstants.ROOT_DISK_SIZE) == null && !newVol.getSize().equals(template.getSize())) {
7798-
VolumeVO resizedVolume = (VolumeVO) newVol;
7799-
if (template.getSize() != null) {
7800-
resizedVolume.setSize(template.getSize());
7801-
_volsDao.update(resizedVolume.getId(), resizedVolume);
7825+
return new Pair<>(userVm, newVol);
78027826
}
7803-
}
7827+
});
78047828

7805-
// 1. Save usage event and update resource count for user vm volumes
7806-
_resourceLimitMgr.incrementResourceCount(newVol.getAccountId(), ResourceType.volume, newVol.isDisplay());
7807-
_resourceLimitMgr.incrementResourceCount(newVol.getAccountId(), ResourceType.primary_storage, newVol.isDisplay(), new Long(newVol.getSize()));
7808-
// 2. Create Usage event for the newly created volume
7809-
UsageEventVO usageEvent = new UsageEventVO(EventTypes.EVENT_VOLUME_CREATE, newVol.getAccountId(), newVol.getDataCenterId(), newVol.getId(), newVol.getName(), newVol.getDiskOfferingId(), template.getId(), newVol.getSize());
7810-
_usageEventDao.persist(usageEvent);
7829+
vm = vmAndNewVol.first();
7830+
Volume newVol = vmAndNewVol.second();
78117831

78127832
handleManagedStorage(vm, root);
78137833

‎server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import com.cloud.exception.InsufficientCapacityException;
3131
import com.cloud.exception.InsufficientServerCapacityException;
3232
import com.cloud.exception.InvalidParameterValueException;
33+
import com.cloud.exception.PermissionDeniedException;
3334
import com.cloud.exception.ResourceAllocationException;
3435
import com.cloud.exception.ResourceUnavailableException;
3536
import com.cloud.host.Host;
@@ -47,13 +48,16 @@
4748
import com.cloud.storage.DiskOfferingVO;
4849
import com.cloud.storage.GuestOSVO;
4950
import com.cloud.storage.ScopeType;
51+
import com.cloud.storage.Snapshot;
52+
import com.cloud.storage.SnapshotVO;
5053
import com.cloud.storage.Storage;
5154
import com.cloud.storage.VMTemplateVO;
5255
import com.cloud.storage.Volume;
5356
import com.cloud.storage.VolumeApiService;
5457
import com.cloud.storage.VolumeVO;
5558
import com.cloud.storage.dao.DiskOfferingDao;
5659
import com.cloud.storage.dao.GuestOSDao;
60+
import com.cloud.storage.dao.SnapshotDao;
5761
import com.cloud.storage.dao.VMTemplateDao;
5862
import com.cloud.storage.dao.VolumeDao;
5963
import com.cloud.template.VirtualMachineTemplate;
@@ -66,6 +70,7 @@
6670
import com.cloud.user.UserDataVO;
6771
import com.cloud.user.UserVO;
6872
import com.cloud.user.dao.AccountDao;
73+
import com.cloud.user.dao.UserDao;
6974
import com.cloud.user.dao.UserDataDao;
7075
import com.cloud.uservm.UserVm;
7176
import com.cloud.utils.Pair;
@@ -74,10 +79,14 @@
7479
import com.cloud.vm.dao.NicDao;
7580
import com.cloud.vm.dao.UserVmDao;
7681
import com.cloud.vm.dao.UserVmDetailsDao;
82+
import com.cloud.vm.snapshot.VMSnapshotVO;
83+
import com.cloud.vm.snapshot.dao.VMSnapshotDao;
84+
7785
import org.apache.cloudstack.api.BaseCmd.HTTPMethod;
7886
import org.apache.cloudstack.api.command.user.vm.DeployVMCmd;
7987
import org.apache.cloudstack.api.command.user.vm.DeployVnfApplianceCmd;
8088
import org.apache.cloudstack.api.command.user.vm.ResetVMUserDataCmd;
89+
import org.apache.cloudstack.api.command.user.vm.RestoreVMCmd;
8190
import org.apache.cloudstack.api.command.user.vm.UpdateVMCmd;
8291
import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd;
8392
import org.apache.cloudstack.context.CallContext;
@@ -191,6 +200,9 @@ public class UserVmManagerImplTest {
191200
@Mock
192201
private AccountDao accountDao;
193202

203+
@Mock
204+
private UserDao userDao;
205+
194206
@Mock
195207
ResourceLimitService resourceLimitMgr;
196208

@@ -218,6 +230,12 @@ public class UserVmManagerImplTest {
218230
@Mock
219231
private VolumeDao volumeDaoMock;
220232

233+
@Mock
234+
private SnapshotDao snapshotDaoMock;
235+
236+
@Mock
237+
private VMSnapshotDao vmSnapshotDaoMock;
238+
221239
@Mock
222240
AccountVO account;
223241

@@ -1221,4 +1239,160 @@ public void testSetVmRequiredFieldsForImportFromLastHost() {
12211239
Mockito.verify(userVmVoMock).setLastHostId(2L);
12221240
Mockito.verify(userVmVoMock).setState(VirtualMachine.State.Running);
12231241
}
1242+
1243+
@Test(expected = InvalidParameterValueException.class)
1244+
public void testRestoreVMNoVM() throws ResourceUnavailableException, InsufficientCapacityException {
1245+
CallContext callContextMock = Mockito.mock(CallContext.class);
1246+
Mockito.lenient().doReturn(accountMock).when(callContextMock).getCallingAccount();
1247+
1248+
RestoreVMCmd cmd = Mockito.mock(RestoreVMCmd.class);
1249+
when(cmd.getVmId()).thenReturn(vmId);
1250+
when(cmd.getTemplateId()).thenReturn(2L);
1251+
when(userVmDao.findById(vmId)).thenReturn(null);
1252+
1253+
userVmManagerImpl.restoreVM(cmd);
1254+
}
1255+
1256+
@Test(expected = CloudRuntimeException.class)
1257+
public void testRestoreVMWithVolumeSnapshots() throws ResourceUnavailableException, InsufficientCapacityException {
1258+
CallContext callContextMock = Mockito.mock(CallContext.class);
1259+
Mockito.lenient().doReturn(accountMock).when(callContextMock).getCallingAccount();
1260+
Mockito.lenient().doNothing().when(accountManager).checkAccess(accountMock, null, true, userVmVoMock);
1261+
1262+
RestoreVMCmd cmd = Mockito.mock(RestoreVMCmd.class);
1263+
when(cmd.getVmId()).thenReturn(vmId);
1264+
when(cmd.getTemplateId()).thenReturn(2L);
1265+
when(userVmDao.findById(vmId)).thenReturn(userVmVoMock);
1266+
1267+
List<VolumeVO> volumes = new ArrayList<>();
1268+
long rootVolumeId = 1l;
1269+
VolumeVO rootVolumeOfVm = Mockito.mock(VolumeVO.class);
1270+
Mockito.when(rootVolumeOfVm.getId()).thenReturn(rootVolumeId);
1271+
volumes.add(rootVolumeOfVm);
1272+
when(volumeDaoMock.findByInstanceAndType(vmId, Volume.Type.ROOT)).thenReturn(volumes);
1273+
1274+
List<SnapshotVO> snapshots = new ArrayList<>();
1275+
SnapshotVO snapshot = Mockito.mock(SnapshotVO.class);
1276+
snapshots.add(snapshot);
1277+
when(snapshotDaoMock.listByStatus(rootVolumeId, Snapshot.State.Creating, Snapshot.State.CreatedOnPrimary, Snapshot.State.BackingUp)).thenReturn(snapshots);
1278+
1279+
userVmManagerImpl.restoreVM(cmd);
1280+
}
1281+
1282+
@Test(expected = InvalidParameterValueException.class)
1283+
public void testRestoreVirtualMachineNoOwner() throws ResourceUnavailableException, InsufficientCapacityException {
1284+
long userId = 1l;
1285+
long accountId = 2l;
1286+
long newTemplateId = 2l;
1287+
when(accountMock.getId()).thenReturn(userId);
1288+
when(userVmDao.findById(vmId)).thenReturn(userVmVoMock);
1289+
when(userVmVoMock.getAccountId()).thenReturn(accountId);
1290+
when(accountDao.findById(accountId)).thenReturn(null);
1291+
1292+
userVmManagerImpl.restoreVirtualMachine(accountMock, vmId, newTemplateId);
1293+
}
1294+
1295+
@Test(expected = PermissionDeniedException.class)
1296+
public void testRestoreVirtualMachineOwnerDisabled() throws ResourceUnavailableException, InsufficientCapacityException {
1297+
long userId = 1l;
1298+
long accountId = 2l;
1299+
long newTemplateId = 2l;
1300+
when(accountMock.getId()).thenReturn(userId);
1301+
when(userVmDao.findById(vmId)).thenReturn(userVmVoMock);
1302+
when(userVmVoMock.getAccountId()).thenReturn(accountId);
1303+
when(accountDao.findById(accountId)).thenReturn(callerAccount);
1304+
when(callerAccount.getState()).thenReturn(Account.State.DISABLED);
1305+
1306+
userVmManagerImpl.restoreVirtualMachine(accountMock, vmId, newTemplateId);
1307+
}
1308+
1309+
@Test(expected = CloudRuntimeException.class)
1310+
public void testRestoreVirtualMachineNotInRightState() throws ResourceUnavailableException, InsufficientCapacityException {
1311+
long userId = 1l;
1312+
long accountId = 2l;
1313+
long newTemplateId = 2l;
1314+
when(accountMock.getId()).thenReturn(userId);
1315+
when(userVmDao.findById(vmId)).thenReturn(userVmVoMock);
1316+
when(userVmVoMock.getAccountId()).thenReturn(accountId);
1317+
when(userVmVoMock.getUuid()).thenReturn("a967643d-7633-4ab4-ac26-9c0b63f50cc1");
1318+
when(accountDao.findById(accountId)).thenReturn(callerAccount);
1319+
when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Starting);
1320+
1321+
userVmManagerImpl.restoreVirtualMachine(accountMock, vmId, newTemplateId);
1322+
}
1323+
1324+
@Test(expected = InvalidParameterValueException.class)
1325+
public void testRestoreVirtualMachineNoRootVolume() throws ResourceUnavailableException, InsufficientCapacityException {
1326+
long userId = 1l;
1327+
long accountId = 2l;
1328+
long currentTemplateId = 1l;
1329+
long newTemplateId = 2l;
1330+
when(accountMock.getId()).thenReturn(userId);
1331+
when(userVmDao.findById(vmId)).thenReturn(userVmVoMock);
1332+
when(userVmVoMock.getAccountId()).thenReturn(accountId);
1333+
when(userVmVoMock.getUuid()).thenReturn("a967643d-7633-4ab4-ac26-9c0b63f50cc1");
1334+
when(accountDao.findById(accountId)).thenReturn(callerAccount);
1335+
when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Running);
1336+
when(userVmVoMock.getTemplateId()).thenReturn(currentTemplateId);
1337+
1338+
VMTemplateVO currentTemplate = Mockito.mock(VMTemplateVO.class);
1339+
when(templateDao.findById(currentTemplateId)).thenReturn(currentTemplate);
1340+
when(volumeDaoMock.findByInstanceAndType(vmId, Volume.Type.ROOT)).thenReturn(new ArrayList<VolumeVO>());
1341+
1342+
userVmManagerImpl.restoreVirtualMachine(accountMock, vmId, newTemplateId);
1343+
}
1344+
1345+
@Test(expected = InvalidParameterValueException.class)
1346+
public void testRestoreVirtualMachineMoreThanOneRootVolume() throws ResourceUnavailableException, InsufficientCapacityException {
1347+
long userId = 1l;
1348+
long accountId = 2l;
1349+
long currentTemplateId = 1l;
1350+
long newTemplateId = 2l;
1351+
when(accountMock.getId()).thenReturn(userId);
1352+
when(userVmDao.findById(vmId)).thenReturn(userVmVoMock);
1353+
when(userVmVoMock.getAccountId()).thenReturn(accountId);
1354+
when(userVmVoMock.getUuid()).thenReturn("a967643d-7633-4ab4-ac26-9c0b63f50cc1");
1355+
when(accountDao.findById(accountId)).thenReturn(callerAccount);
1356+
when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Running);
1357+
when(userVmVoMock.getTemplateId()).thenReturn(currentTemplateId);
1358+
1359+
VMTemplateVO currentTemplate = Mockito.mock(VMTemplateVO.class);
1360+
when(currentTemplate.isDeployAsIs()).thenReturn(false);
1361+
when(templateDao.findById(currentTemplateId)).thenReturn(currentTemplate);
1362+
List<VolumeVO> volumes = new ArrayList<>();
1363+
VolumeVO rootVolume1 = Mockito.mock(VolumeVO.class);
1364+
volumes.add(rootVolume1);
1365+
VolumeVO rootVolume2 = Mockito.mock(VolumeVO.class);
1366+
volumes.add(rootVolume2);
1367+
when(volumeDaoMock.findByInstanceAndType(vmId, Volume.Type.ROOT)).thenReturn(volumes);
1368+
1369+
userVmManagerImpl.restoreVirtualMachine(accountMock, vmId, newTemplateId);
1370+
}
1371+
1372+
@Test(expected = InvalidParameterValueException.class)
1373+
public void testRestoreVirtualMachineWithVMSnapshots() throws ResourceUnavailableException, InsufficientCapacityException {
1374+
long userId = 1l;
1375+
long accountId = 2l;
1376+
long currentTemplateId = 1l;
1377+
long newTemplateId = 2l;
1378+
when(accountMock.getId()).thenReturn(userId);
1379+
when(userVmDao.findById(vmId)).thenReturn(userVmVoMock);
1380+
when(userVmVoMock.getAccountId()).thenReturn(accountId);
1381+
when(accountDao.findById(accountId)).thenReturn(callerAccount);
1382+
when(userVmVoMock.getState()).thenReturn(VirtualMachine.State.Running);
1383+
when(userVmVoMock.getTemplateId()).thenReturn(currentTemplateId);
1384+
1385+
VMTemplateVO currentTemplate = Mockito.mock(VMTemplateVO.class);
1386+
when(templateDao.findById(currentTemplateId)).thenReturn(currentTemplate);
1387+
List<VolumeVO> volumes = new ArrayList<>();
1388+
VolumeVO rootVolumeOfVm = Mockito.mock(VolumeVO.class);
1389+
volumes.add(rootVolumeOfVm);
1390+
when(volumeDaoMock.findByInstanceAndType(vmId, Volume.Type.ROOT)).thenReturn(volumes);
1391+
List<VMSnapshotVO> vmSnapshots = new ArrayList<>();
1392+
VMSnapshotVO vmSnapshot = Mockito.mock(VMSnapshotVO.class);
1393+
vmSnapshots.add(vmSnapshot);
1394+
when(vmSnapshotDaoMock.findByVm(vmId)).thenReturn(vmSnapshots);
1395+
1396+
userVmManagerImpl.restoreVirtualMachine(accountMock, vmId, newTemplateId);
1397+
}
12241398
}

0 commit comments

Comments
 (0)