Liron Aravot has posted comments on this change. Change subject: core: Calculate storage format when adding an SD ......................................................................
Patch Set 3: (2 comments) http://gerrit.ovirt.org/#/c/30960/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStorageDomainCommand.java: Line 164: if (sd.getStorageFormat() == null) { Line 165: if (sd.getStorageDomainType().isDataDomain()) { Line 166: sd.setStorageFormat(VersionStorageFormatUtil.getPreferredForVersion( Line 167: getTargetStoragePool().getcompatibility_version(), sd.getStorageType()) Line 168: ); > It can't be null. - host doesn't have to have a data center, cluster can also be "floating" afaik. - right now the behavior is that we can add a domain through any host in the system as unattached domain and later attach it to any dc, seems that after this change the user will have to "remember" to select appropriate host to add the domain , otherwise the version might be too high to attach later on to a needed data center. i assume that the needed behavior is that when you *DO* have a dc, you can use the needed version, oterhwise use V1. - this command is being extended by multiple commands, are you sure that having this is correct for all of it? Line 169: } else { Line 170: sd.setStorageFormat(StorageFormatType.V1); Line 171: } Line 172: } http://gerrit.ovirt.org/#/c/30960/3/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StorageDomainStatic.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/StorageDomainStatic.java: Line 52 Line 53 Line 54 Line 55 Line 56 I'd leave the default at least for now to narrow to possible side effects (for example - was this change tested with the AddExisting storage domain flows?) -- To view, visit http://gerrit.ovirt.org/30960 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If469bb655e9a65e2d0afcee164655fa0bdfa5d99 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Allon Mureinik <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
