Michael Kublin has posted comments on this change.
Change subject: core: initVdsOnUp - remove reconstruct and host status.
......................................................................
Patch Set 6: (4 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java
Line 183: new
ConnectStoragePoolVDSCommandParameters(vds.getId(), storagePoolId,
Line 184: vds.getVdsSpmId(), masterDomain.getId(),
Line 185:
storagePool.getmaster_domain_version())).getSucceeded();
Line 186: } catch (VDSNetworkException | VdcBLLException e) {
Line 187: if (isMasterDomainInactiveOrUnknown) {
First of all in case of network exception we should move to NonOp.
Second only VdcBLLException can be thrown from runVdsCommand() , so in order to
understand network exception special code should be written
Line 188: result.setSuccess(true);
Line 189: } else {
Line 190: log.errorFormat("Could not connect host {0} to pool
{1}", vds.getName(), storagePool
Line 191: .getname());
Line 194: } catch (RuntimeException exp) {
Line 195: log.errorFormat("Could not connect host {0} to pool {1}",
vds.getName(), storagePool
Line 196: .getname());
Line 197: result.setSuccess(false);
Line 198: }
I don't like such style, I prefer that in this line we will have a simple if
condition:
if(result.isSuccess()) all calculation can be done before.
Line 199: if (result.isSuccess() && !isMasterDomainInactiveOrUnknown) {
Line 200: result.setSuccess(proceedVdsStats());
Line 201: if (!result.isSuccess()) {
Line 202: AuditLogDirector.log(new AuditLogableBase(getVdsId()),
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageDomainDAODbFacadeImpl.java
Line 22: @SuppressWarnings("synthetic-access")
Line 23: public class StorageDomainDAODbFacadeImpl extends BaseDAODbFacade
implements StorageDomainDAO {
Line 24:
Line 25: @Override
Line 26: public Guid getMasterStorageDomainIdForPool(Guid pool) {
Replace this by new query
Line 27: return getStorageDomainIdForPoolByType(pool,
StorageDomainType.Master);
Line 28: }
Line 29:
Line 30: @Override
Line 26: public Guid getMasterStorageDomainIdForPool(Guid pool) {
Line 27: return getStorageDomainIdForPoolByType(pool,
StorageDomainType.Master);
Line 28: }
Line 29:
Line 30: @Override
Please make query more generic
Line 31: public StorageDomain getMasterStorageDomainForPool(Guid pool) {
Line 32: return
getCallsHandler().executeRead("Getmaster_storage_domain_By_storagePoolId",
Line 33: StorageDomainRowMapper.instance,
Line 34: getCustomMapSqlParameterSource()
--
To view, visit http://gerrit.ovirt.org/13709
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6418b9d2826146e1e4ceff4341c6d7cd3a0024af
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
