Michael Kublin has posted comments on this change.
Change subject: core: Allow to attach V1 and V2 data domains to a DC 3.1
......................................................................
Patch Set 6: I would prefer that you didn't submit this
(2 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStoragePoolWithStoragesCommand.java
Line 153: final storage_domain_static staticDomain =
storageDomain.getStorageStaticData();
Line 154:
getCompensationContext().snapshotEntity(staticDomain);
Line 155: // for data centers >= 3.1 we enforce the
domain version to V3
Line 156: // this strictly needs to be before selecting
the masterStorageDomain because the pool master
Line 157: // version (setmaster_domain_version) depends
on the domain upgrade.
By the way that flow is really strange, here we are changing formats of storage
domains, after that we are changing format of pool according to format of
storage, also a change is done only at engine side and reflected only at engine
DB, so it is means that format of storage domain is not important to vdsm? If
yes, why we need it?
Line 158: if
(getStoragePool().getcompatibility_version().compareTo(Version.v3_1) >= 0
Line 159: && staticDomain.getStorageFormat() !=
StorageFormatType.V3) {
Line 160:
staticDomain.setStorageFormat(StorageFormatType.V3);
Line 161:
getStorageDomainStaticDAO().update(staticDomain);
Line 157: // version (setmaster_domain_version) depends
on the domain upgrade.
Line 158: if
(getStoragePool().getcompatibility_version().compareTo(Version.v3_1) >= 0
Line 159: && staticDomain.getStorageFormat() !=
StorageFormatType.V3) {
Line 160:
staticDomain.setStorageFormat(StorageFormatType.V3);
Line 161:
getStorageDomainStaticDAO().update(staticDomain);
These two updates is useless, the flow runs in transaction, the changes are not
seen until transaction is committed, what is a reason to perform unneeded
operation?
These makes code inefficient and unreadable.
Line 162: }
Line 163:
storageDomain.setstorage_pool_id(getStoragePool().getId());
Line 164: if (masterStorageDomain == null
Line 165: &&
storageDomain.getstorage_domain_type() == StorageDomainType.Data) {
--
To view, visit http://gerrit.ovirt.org/7445
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2424b80ba914f3d83ec99f442970bc103232563a
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
