Maor Lipchuk has posted comments on this change.
Change subject: Fix log print values when vdsm version is not supported in
cluster
......................................................................
Patch Set 1:
(4 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HandleVdsVersionCommand.java
Line 1: package org.ovirt.engine.core.bll;
Line 2:
Line 3: import static
org.ovirt.engine.core.common.businessentities.NonOperationalReason.VERSION_INCOMPATIBLE_WITH_CLUSTER;
Line 4: import static
org.ovirt.engine.core.common.businessentities.NonOperationalReason.CLUSTER_VERSION_INCOMPATIBLE_WITH_CLUSTER;
Why use import static here, we can simply import NonOperationalReason
and just call NonOperationalReason.CLUSTER_VERSION_INC
OMPATIBLE_WITH_CLUSTER in the constructor of SetNonOperationalVdsParameters
(same for VERSION_INCOMPATIBL
E_WITH_CLUSTER)
Line 5:
Line 6: import java.util.HashMap;
Line 7: import java.util.HashSet;
Line 8: import java.util.Map;
Line 66:
Line 67: // move to non operational if vds-vdc version not supported OR
cluster
Line 68: // version is not supported
Line 69: if (!vdsmVersionSupported) {
Line 70: Map<String, String> customLogValues = new HashMap<String,
String>();
Please use diamond as so : new HashMap<>();
Line 71: customLogValues.put("CompatibilityVersion",
Line 72: Config.<HashSet<Version>>
GetValue(ConfigValues.SupportedVDSMVersions).toString());
Line 73: customLogValues.put("VdsSupportedVersions",
vdsmVersion.toString());
Line 74: SetNonOperationalVdsParameters tempVar = new
SetNonOperationalVdsParameters(getVdsId(),
Line 74: SetNonOperationalVdsParameters tempVar = new
SetNonOperationalVdsParameters(getVdsId(),
Line 75: VERSION_INCOMPATIBLE_WITH_CLUSTER,
Line 76: customLogValues);
Line 77: tempVar.setSaveToDb(true);
Line 78:
Backend.getInstance().runInternalAction(VdcActionType.SetNonOperationalVds,
tempVar, ExecutionHandler.createInternalJobContext());
Please consider to extract a method of those 2 lines above since they are
duplicated with lines 87-88 (I would also consider to use the auditLogEnum as
an argument in the new method, along with the customLogValues)
Line 79: }
Line 80: if
(!VersionSupport.checkClusterVersionSupported(cluster.getcompatibility_version(),
vds)) {
Line 81: Map<String, String> customLogValues = new HashMap<String,
String>();
Line 82: customLogValues.put("CompatibilityVersion",
cluster.getcompatibility_version().toString());
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java
Line 371: VM_IMPORT_FAILED(125),
Line 372: VM_NOT_RESPONDING(126),
Line 373: VDS_RUN_IN_NO_KVM_MODE(127),
Line 374: VDS_VERSION_NOT_SUPPORTED_FOR_CLUSTER(141),
Line 375: VDS_CLUSTER_VERSION_NOT_SUPPORTED(10201),
Those numbers doesn't really mean something, IINM it's only used in the DB.
The thing is that for now those codes are grouped by subjects, and every audit
log which has code which is bigger then 10,100 is ususally used for network Qos
or mom policies.
maybe you can use code 154, but again, this it is not that important.
Line 376: VM_CLEARED(129),
Line 377: VM_PAUSED_ENOSPC(138),
Line 378: VM_PAUSED_ERROR(139),
Line 379: VM_IMPORT_INFO(144),
--
To view, visit http://gerrit.ovirt.org/17719
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iacfb3875a0617f978b4453769a0b1c5704282fa5
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Martin Sivák <msi...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
