Fix "DiskImage" copy constructor #301
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/ost |
barpavel
force-pushed
the
fix_DiskImage_copy_constructor
branch
from
c78eb31
to
75d9401
Compare
|
/ost |
bennyz
approved these changes
Apr 25, 2022
ahadas
reviewed
Apr 27, 2022
...es/common/src/main/java/org/ovirt/engine/core/common/businessentities/storage/DiskImage.java
Show resolved
Hide resolved
|
/ost |
Some "Date" data members are copied using reference copying, while in some 3 places (without any visible reason) a new object is created. Changing for consistency to a simple reference copy: if we want to protect from changing an internal "Date" object via one of the containing "DiskImage" instances, we probably need to do it in many more places in this class (and other classes as well). Signed-off-by: Pavel Bar <pbar@redhat.com>
barpavel
force-pushed
the
fix_DiskImage_copy_constructor
branch
from
6173ddb
to
667b04b
Compare
…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>
barpavel
force-pushed
the
fix_DiskImage_copy_constructor
branch
from
667b04b
to
33f796d
Compare
|
/ost |
ahadas
approved these changes
Jun 2, 2022
dangel101
added a commit
to dangel101/ovirt-engine
that referenced
this pull request
Sep 28, 2022
…if nothing needs doing Since the tag 'skip_ansible_lint oVirt#301' doesn't work and '# noqa no-changed-when' cannot be set on code blocks, adding it to skip list
dangel101
added a commit
to dangel101/ovirt-engine
that referenced
this pull request
Sep 29, 2022
…if nothing needs doing Since the tag 'skip_ansible_lint oVirt#301' doesn't work and '# noqa no-changed-when' cannot be set on code blocks, adding it to skip list
dangel101
added a commit
to dangel101/ovirt-engine
that referenced
this pull request
Oct 3, 2022
'Commands should not change things if nothing needs doing' Since the tag 'skip_ansible_lint oVirt#301' doesn't work and '# noqa no-changed-when' cannot be set on code blocks, adding it to skip list
dangel101
added a commit
to dangel101/ovirt-engine
that referenced
this pull request
Oct 6, 2022
'Commands should not change things if nothing needs doing' Since the tag 'skip_ansible_lint oVirt#301' doesn't work and '# noqa no-changed-when' cannot be set on code blocks, adding it to skip list
mwperina
pushed a commit
to dangel101/ovirt-engine
that referenced
this pull request
Oct 7, 2022
'Commands should not change things if nothing needs doing' Since the tag 'skip_ansible_lint oVirt#301' doesn't work and '# noqa no-changed-when' cannot be set on code blocks, adding it to skip list
mwperina
pushed a commit
to dangel101/ovirt-engine
that referenced
this pull request
Oct 7, 2022
'Commands should not change things if nothing needs doing' Since the tag 'skip_ansible_lint oVirt#301' doesn't work and '# noqa no-changed-when' cannot be set on code blocks, adding it to skip list
mwperina
pushed a commit
to dangel101/ovirt-engine
that referenced
this pull request
Oct 10, 2022
'Commands should not change things if nothing needs doing' Since the tag 'skip_ansible_lint oVirt#301' doesn't work and '# noqa no-changed-when' cannot be set on code blocks, adding it to skip list
mwperina
pushed a commit
to dangel101/ovirt-engine
that referenced
this pull request
Oct 10, 2022
'Commands should not change things if nothing needs doing' Since the tag 'skip_ansible_lint oVirt#301' doesn't work and '# noqa no-changed-when' cannot be set on code blocks, adding it to skip list
mwperina
pushed a commit
to dangel101/ovirt-engine
that referenced
this pull request
Oct 10, 2022
'Commands should not change things if nothing needs doing' Since the tag 'skip_ansible_lint oVirt#301' doesn't work and '# noqa no-changed-when' cannot be set on code blocks, adding it to skip list
mwperina
pushed a commit
to dangel101/ovirt-engine
that referenced
this pull request
Oct 10, 2022
'Commands should not change things if nothing needs doing' Since the tag 'skip_ansible_lint oVirt#301' doesn't work and '# noqa no-changed-when' cannot be set on code blocks, adding it to skip list
dangel101
added a commit
to dangel101/ovirt-engine
that referenced
this pull request
Oct 18, 2022
'Commands should not change things if nothing needs doing' Since the tag 'skip_ansible_lint oVirt#301' doesn't work and '# noqa no-changed-when' cannot be set on code blocks, adding it to skip list
dangel101
added a commit
to dangel101/ovirt-engine
that referenced
this pull request
Oct 19, 2022
'Commands should not change things if nothing needs doing' Since the tag 'skip_ansible_lint oVirt#301' doesn't work and '# noqa no-changed-when' cannot be set on code blocks, adding it to skip list
dangel101
added a commit
to dangel101/ovirt-engine
that referenced
this pull request
Oct 25, 2022
'Commands should not change things if nothing needs doing' Since the tag 'skip_ansible_lint oVirt#301' doesn't work and '# noqa no-changed-when' cannot be set on code blocks, adding it to skip list
dangel101
added a commit
to dangel101/ovirt-engine
that referenced
this pull request
Oct 26, 2022
'Commands should not change things if nothing needs doing' Since the tag 'skip_ansible_lint oVirt#301' doesn't work and '# noqa no-changed-when' cannot be set on code blocks, adding it to skip list
mwperina
pushed a commit
that referenced
this pull request
Oct 27, 2022
'Commands should not change things if nothing needs doing' Since the tag 'skip_ansible_lint #301' doesn't work and '# noqa no-changed-when' cannot be set on code blocks, adding it to skip list
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A few "DiskImage" data members were overridden with wrong values at the end of Copy Constructor.
The data members are:
CreationDate&LastModified&ImageStatusSeems like the issue started at:
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