Federico Simoncelli 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: (3 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStoragePoolWithStoragesCommand.java
Line 150: if (existingInDb) {
Line 151:
getCompensationContext().snapshotEntity(mapFromDB);
Line 152: }
Line 153: final storage_domain_static staticDomain =
storageDomain.getStorageStaticData();
Line 154:
getCompensationContext().snapshotEntity(staticDomain);
This could be problematic if it's an heavy task. It's done for all the domains
(even if it's not strictly required) because it's hard to know in advance if
it's needed or not (the two different conditions are below).
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.
Line 158: if
(getStoragePool().getcompatibility_version().compareTo(Version.v3_1) >= 0
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.
We start to have too many of these around. I might try to unify them somewhere.
Something like:
StorageFormatType getPreferredFormat(Version dcVers, StorageFormatType
suggested)
For dcVers < 3.1 we return "suggested", and for dcVers >= 3.1 we return V3. Not
sure how much it would be helpful though. I still need to investigate. If it's
not too ugly let's not block on this.
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);
I don't think that having these two updates (here and line 170) would be
problematic since these are conditions that are rarely satisfied (and even more
rarely satisfied both). Please consider that the masterStorageDomain block is
used only once per call.
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
