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 4: I would prefer that you didn't submit this
(5 inline comments)
Who is verifying a patch?
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommand.java
Line 92: if (sdType == StorageDomainType.Master) {
Line 93: calcStoragePoolStatusByDomainsStatus();
Line 94: }
Line 95:
Line 96: // upgrade the domain format to the
storage pool format
So, if user choose other format, it will be automatic changed to format of
pool, at case that we passed canDoAction?
Line 97: if (sdType == StorageDomainType.Data ||
sdType == StorageDomainType.Master) {
Line 98: final StorageDomainStaticDAO sdStatDao
= getDbFacade().getStorageDomainStaticDAO();
Line 99: final storage_domain_static domain =
sdStatDao.get(getStorageDomain().getId());
Line 100: final StorageFormatType targetFormat
= getStoragePool().getStoragePoolFormatType();
Line 93: calcStoragePoolStatusByDomainsStatus();
Line 94: }
Line 95:
Line 96: // upgrade the domain format to the
storage pool format
Line 97: if (sdType == StorageDomainType.Data ||
sdType == StorageDomainType.Master) {
why so many final? Also u don't need that variable
Line 98: final StorageDomainStaticDAO sdStatDao
= getDbFacade().getStorageDomainStaticDAO();
Line 99: final storage_domain_static domain =
sdStatDao.get(getStorageDomain().getId());
Line 100: final StorageFormatType targetFormat
= getStoragePool().getStoragePoolFormatType();
Line 101:
Line 94: }
Line 95:
Line 96: // upgrade the domain format to the
storage pool format
Line 97: if (sdType == StorageDomainType.Data ||
sdType == StorageDomainType.Master) {
Line 98: final StorageDomainStaticDAO sdStatDao
= getDbFacade().getStorageDomainStaticDAO();
unneeded query
Line 99: final storage_domain_static domain =
sdStatDao.get(getStorageDomain().getId());
Line 100: final StorageFormatType targetFormat
= getStoragePool().getStoragePoolFormatType();
Line 101:
Line 102: if (domain.getStorageFormat() !=
targetFormat) {
Line 102: if (domain.getStorageFormat() !=
targetFormat) {
Line 103: log.infoFormat("Updating storage
domain {0} (type {1}) to format {2}",
Line 104: getStorageDomain().getId(),
sdType, targetFormat);
Line 105:
domain.setStorageFormat(targetFormat);
Line 106: sdStatDao.update(domain);
what about compensation? actually it is not needed here at all, so u need to
clean up all unneeded code
Line 107: }
Line 108: }
Line 109:
Line 110: getCompensationContext().stateChanged();
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageHandlingCommandBase.java
Line 320: return true;
Line 321: }
Line 322: Set<StorageFormatType> supportedFormatsSet =
Line 323:
getSupportedStorageFormatSet(storagePool.getcompatibility_version());
Line 324: if
(supportedFormatsSet.contains(storageDomain.getStorageFormat())) {
These will cause regression, if domain is a first
storagePool.getStoragePoolFormatType() can be null
Line 325: return true;
Line 326: }
Line 327:
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_FORMAT_ILLEGAL);
Line 328: getReturnValue().getCanDoActionMessages().add(
--
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: 4
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
