Arik Hadas has posted comments on this change.
Change subject: core: save memory state on live snapshot with memory
......................................................................
Patch Set 9: (16 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java
Line 130:
Line 131: @Override
Line 132: public NGuid getStorageDomainId() {
Line 133: if (cachedStorageDomainId.equals(Guid.Empty) && getVm() !=
null) {
Line 134: long sizeNeeded = getVm().getTotalMemorySizeInBytes() /
BYTES_IN_GB;
well, you're right. I've also noticed that - this calculation was copied from
the hibernate command. after fixing it, I discussed it with Omer and it is a
known thing which is OK - the "missing" space will be less than 1GB. you assume
that there's some threshold which is more than a 1GB less than the storage
capacity, so in the worst case we'll pass this threshold and then the user will
be warned about it. I've made a change exactly as you suggest, but I didn't
commit it - as it seems there's no point to change this.
Line 135: StorageDomain storageDomain =
VmHandler.findStorageDomainForMemory(getVm().getStoragePoolId(), sizeNeeded);
Line 136: if (storageDomain != null) {
Line 137: cachedStorageDomainId = storageDomain.getId();
Line 138: }
Line 132: public NGuid getStorageDomainId() {
Line 133: if (cachedStorageDomainId.equals(Guid.Empty) && getVm() !=
null) {
Line 134: long sizeNeeded = getVm().getTotalMemorySizeInBytes() /
BYTES_IN_GB;
Line 135: StorageDomain storageDomain =
VmHandler.findStorageDomainForMemory(getVm().getStoragePoolId(), sizeNeeded);
Line 136: if (storageDomain != null) {
yes.. this code was taken from hibernate command without taking the relevant
can-do-action check.. I'll add the missing check
Line 137: cachedStorageDomainId = storageDomain.getId();
Line 138: }
Line 139: }
Line 140: return cachedStorageDomainId;
Line 217: String[] strings =
getVm().getHibernationVolHandle().split(",");
Line 218: Guid[] guids = new Guid[strings.length];
Line 219: for (int i=0; i<strings.length; ++i) {
Line 220: guids[i] = new Guid(strings[i]);
Line 221: }
Done in a later patch
Line 222:
Line 223: VDSReturnValue vdsRetValue1 = runVdsCommand(
Line 224: VDSCommandType.DeleteImageGroup,
Line 225: new
DeleteImageGroupVDSCommandParameters(guids[1], guids[0], guids[2],
Line 224: VDSCommandType.DeleteImageGroup,
Line 225: new
DeleteImageGroupVDSCommandParameters(guids[1], guids[0], guids[2],
Line 226: false, false,
getVm().getVdsGroupCompatibilityVersion().toString()));
Line 227:
Line 228: if (!vdsRetValue1.getSucceeded()) {
1.
2. already done in a later patch. I don't
Line 229: // log..
Line 230: }
Line 231: }
Line 232: } else {
Line 225: new
DeleteImageGroupVDSCommandParameters(guids[1], guids[0], guids[2],
Line 226: false, false,
getVm().getVdsGroupCompatibilityVersion().toString()));
Line 227:
Line 228: if (!vdsRetValue1.getSucceeded()) {
Line 229: // log..
right, see: http://gerrit.ovirt.org/#/c/15119/
Line 230: }
Line 231: }
Line 232: } else {
Line 233: revertToActiveSnapshot(createdSnapshot.getId());
Line 230: }
Line 231: }
Line 232: } else {
Line 233: revertToActiveSnapshot(createdSnapshot.getId());
Line 234: // TODO: remove the memory image if exists
see http://gerrit.ovirt.org/#/c/15119/6
Line 235: }
Line 236:
Line 237: incrementVmGeneration();
Line 238:
Line 315: }
Line 316: else {
Line 317: return new
SnapshotVDSCommandParameters(getVm().getRunOnVds().getValue(),
Line 318: getVm().getId(),
Line 319: ImagesHandler.filterImageDisks(pluggedDisks,
false, true));
Done
Line 320: }
Line 321: }
Line 322:
Line 323: private void handleVdsLiveSnapshotFailure(VdcBLLException e) {
Line 477:
Line 478: return list;
Line 479: }
Line 480:
Line 481: private interface MemoryImageBuilder {
Done (will be extracted to separate files, when will be used as part of
hibernate command)
Line 482: void build();
Line 483: String getVolumeStringRepresentation();
Line 484: }
Line 485:
Line 477:
Line 478: return list;
Line 479: }
Line 480:
Line 481: private interface MemoryImageBuilder {
Liron - please see later latches where new builder is added, that way we can
add functionality without changing existing code (just extending), which is
much better than the many if-else blocks that we currently have in our commands
(the classic is RunVmCommand)..
Line 482: void build();
Line 483: String getVolumeStringRepresentation();
Line 484: }
Line 485:
Line 482: void build();
Line 483: String getVolumeStringRepresentation();
Line 484: }
Line 485:
Line 486: private class DefaultMemoryImageBuilder implements
MemoryImageBuilder {
agree, not now..
Line 487: private Guid storageDomainId;
Line 488: private Guid storagePoolId;
Line 489: private Guid memoryDumpImageGroupId;
Line 490: private Guid memoryDumpVolumeId;
Line 501: }
Line 502:
Line 503: @Override
Line 504: public void build() {
Line 505: createImageForMemoryDump();
Done
Line 506: createImageForVmMetaData();
Line 507: }
Line 508:
Line 509: private void createImageForVmMetaData() {
Line 545: storagePoolId,
Line 546: storageDomainId,
Line 547: memoryDumpImageGroupId,
Line 548:
getVm().getTotalMemorySizeInBytes(),
Line 549: VolumeType.Preallocated,
Done at http://gerrit.ovirt.org/#/c/15120
Line 550: VolumeFormat.RAW,
Line 551: memoryDumpVolumeId,
Line 552: "",
Line 553:
getStoragePool().getcompatibility_version().toString()));
Line 576: vmConfVolumeId);
Line 577: }
Line 578: }
Line 579:
Line 580: private class NullableImageBuilder implements MemoryImageBuilder {
Done
Line 581: @Override
Line 582: public void build() {
Line 583: //no op
Line 584: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HibernateVmCommand.java
Line 374: * Returns the meta data that should be allocated when saving
state of image.
Line 375: *
Line 376: * @return - Meta data size for allocation in bytes.
Line 377: */
Line 378: static long getMetaDataSizeInBytes() {
Done
Line 379: return (long) 10 * 1024;
Line 380: }
Line 381:
Line 382: private static Log log =
LogFactory.getLog(HibernateVmCommand.class);
....................................................
File
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/SnapshotDaoTest.java
Line 115: }
Line 116:
Line 117: @Test
Line 118: public void getSnaphsotByTypeAndStatusForExistingEntity() throws
Exception {
Line 119: assertNotNull(dao.get(EXISTING_VM_ID, SnapshotType.REGULAR,
SnapshotStatus.OK));
Done
Line 120: }
Line 121:
Line 122: @Test
Line 123: public void getSnaphsotByTypeAndStatusForNonExistingEntity()
throws Exception {
Line 120: }
Line 121:
Line 122: @Test
Line 123: public void getSnaphsotByTypeAndStatusForNonExistingEntity()
throws Exception {
Line 124: assertNull(dao.get(EXISTING_VM_ID, SnapshotType.REGULAR,
SnapshotStatus.LOCKED));
but it should be null..
Line 125: }
Line 126:
Line 127: @Test
Line 128: public void getIdByTypeReturnsIdForExistingByTypeAndStatus()
throws Exception {
--
To view, visit http://gerrit.ovirt.org/14291
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbb28efda1b63e506233a88399488a256e6ab1c8
Gerrit-PatchSet: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
