Skip to content

Commit

Permalink
core: fix the issue with fields being initialized twice during Copy C…
Browse files Browse the repository at this point in the history
…onstructor

1. For some reason during Copy Constructor of the "DiskImage" class,
the "CreationDate" & "LastModified" fields are overridden from having the
correct date of the original "DiskImage", to the current date.

2. Additionally there are 2 "ImageStatus" modifications: keeping the last one
("ImageStatus.LOCKED" status), since it's actually the one that matters.
There is an open question whether all disks created with this Copy Constructor
should always end up in "ImageStatus.LOCKED" status, but since the existing
code might rely on this strange behavior and the investigation would take a
lot of time, the decision was to keep the current behavior.

This code was refactored lately in:
https://gerrit.ovirt.org/c/ovirt-engine/+/117500/17/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/storage/DiskImage.java#77

But it looks like this implementation started long before - at 2012:
https://gerrit.ovirt.org/c/ovirt-engine/+/3835/4/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DiskImage.java#350

Signed-off-by: Pavel Bar <pbar@redhat.com>
  • Loading branch information
barpavel authored and ahadas committed Jun 2, 2022
1 parent f90d9ec commit 54307d9
Showing 1 changed file with 1 addition and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,7 @@ protected DiskImage(DiskImage diskImage) {
appList = diskImage.appList;
setImageTemplateId(diskImage.getImageTemplateId());
setParentId(diskImage.getParentId());
setImageStatus(diskImage.getImageStatus());
if (diskImage.getLastModified() != null) {
setLastModified(diskImage.getLastModified());
}
setLastModified(diskImage.getLastModified() != null ? diskImage.getLastModified() : new Date());
storageIds = new ArrayList<>(diskImage.storageIds);
setVmSnapshotId(diskImage.getVmSnapshotId());
setId(diskImage.getId());
Expand All @@ -111,8 +108,6 @@ protected DiskImage(DiskImage diskImage) {
// TODO: is it ok to use shallow copy here?!
snapshots = new ArrayList<>(diskImage.snapshots);
actualDiskWithSnapthotsSize = diskImage.actualDiskWithSnapthotsSize;
setCreationDate(new Date());
setLastModified(new Date());
setImageStatus(ImageStatus.LOCKED);
setDiskProfileId(diskImage.getDiskProfileId());
getImage().setSequenceNumber(diskImage.getImage().getSequenceNumber());
Expand Down

0 comments on commit 54307d9

Please sign in to comment.