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 5: I would prefer that you didn't submit this

(4 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStoragePoolWithStoragesCommand.java
Line 152:                         }
Line 153:                         // for data centers >= 3.1 we enforce the 
domain version to V3
Line 154:                         if 
(getStoragePool().getcompatibility_version().compareTo(Version.v3_1) >= 0) {
Line 155:                             final storage_domain_static staticDomain 
= storageDomain.getStorageStaticData();
Line 156:                             
staticDomain.setStorageFormat(StorageFormatType.V3);
update is done in wrong place, look down domain is updated at line 167
Line 157:                             
getDbFacade().getStorageDomainStaticDAO().update(staticDomain);
Line 158:                         }
Line 159:                         
storageDomain.setstorage_pool_id(getStoragePool().getId());
Line 160:                         if (masterStorageDomain == null


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java
Line 82:                     executeInNewTransaction(new 
TransactionMethod<Object>() {
Line 83:                         @Override
Line 84:                         public Object runInTransaction() {
Line 85:                             final StorageDomainType sdType = 
getStorageDomain().getstorage_domain_type();
Line 86: 
In order to make that code logical u should remove all code which is related to 
compensation: lines 87,88, 108. Otherwise it is look like that for some reason 
u perform snapshot of status of storage domain, but storageType is not
Line 87:                             
getCompensationContext().snapshotEntityStatus(map, map.getstatus());
Line 88:                             
map.setstatus(StorageDomainStatus.Maintenance);
Line 89:                             
getStoragePoolIsoMapDAO().updateStatus(map.getId(), map.getstatus());
Line 90: 


Line 96:                             if (sdType == StorageDomainType.Data || 
sdType == StorageDomainType.Master) {
Line 97:                                 final storage_domain_static domain = 
getStorageDomain().getStorageStaticData();
Line 98:                                 final StorageFormatType targetFormat = 
getStoragePool().getStoragePoolFormatType();
Line 99: 
Line 100:                                 if (domain.getStorageFormat() != 
targetFormat) {
Again question, when storage domain is created it format passed to VDSM, now u 
created a split at engine db one status, at VDSM is another. And split is 
created at the end of command it is mean not any call to VDSM will be done.
Line 101:                                     log.infoFormat("Updating storage 
domain {0} (type {1}) to format {2}",
Line 102:                                         getStorageDomain().getId(), 
sdType, targetFormat);
Line 103:                                     
domain.setStorageFormat(targetFormat);
Line 104:                                     
getDbFacade().getStorageDomainStaticDAO().update(domain);


Line 99: 
Line 100:                                 if (domain.getStorageFormat() != 
targetFormat) {
Line 101:                                     log.infoFormat("Updating storage 
domain {0} (type {1}) to format {2}",
Line 102:                                         getStorageDomain().getId(), 
sdType, targetFormat);
Line 103:                                     
domain.setStorageFormat(targetFormat);
there are no method getStorageDomainStaticDAO() ?
Line 104:                                     
getDbFacade().getStorageDomainStaticDAO().update(domain);
Line 105:                                 }
Line 106:                             }
Line 107: 


--
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: 5
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

Reply via email to