Moti Asayag has posted comments on this change.
Change subject: engine: Allow Import/Snapshot Duplicate MAC Address
......................................................................
Patch Set 3: (6 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/VmInterfaceManager.java
Line 45: * Used to snapshot the saved entities.
Line 46: * @return <code>true</code> if the MAC wasn't used,
<code>false</code> if it was.
Line 47: */
Line 48: public void add(final VmNetworkInterface iface,
CompensationContext compensationContext, boolean allocateMac,
Line 49: Version vdsGroupCompatibilityVersion) {
s/vdsGroupCompatibilityVersion/clusterCompatibilityVersion
Please add vdsGroupCompatibilityVersion to the javadoc's param as well.
Line 50:
Line 51: if (allocateMac) {
Line 52: iface.setMacAddress(getMacPoolManager().allocateNewMac());
Line 53: } else if (Config.<Boolean>
GetValue(ConfigValues.HotPlugEnabled, vdsGroupCompatibilityVersion.getValue()))
{
Line 49: Version vdsGroupCompatibilityVersion) {
Line 50:
Line 51: if (allocateMac) {
Line 52: iface.setMacAddress(getMacPoolManager().allocateNewMac());
Line 53: } else if (Config.<Boolean>
GetValue(ConfigValues.HotPlugEnabled, vdsGroupCompatibilityVersion.getValue()))
{
Please extract the check of HotPlugEnabled to FeatureSupported class.
Line 54: getMacPoolManager().forceAddMac(iface.getMacAddress());
Line 55: } else if (!getMacPoolManager().addMac(iface.getMacAddress()))
{
Line 56: auditLogMacInUse(iface);
Line 57: log.errorFormat("VmInterfaceManager::Mac {0} is in use.",
iface.getMacAddress());
Line 70: public Void runInTransaction() {
Line 71: AuditLogableBase logable = new AuditLogableBase();
Line 72: logable.addCustomValue("MACAddr",
iface.getMacAddress());
Line 73: log(logable, AuditLogType.MAC_ADDRESS_IS_IN_USE);
Line 74: log.warnFormat("VmInterfaceManager::Network Interface
{0} has MAC address {1} which is in use, " +
The prefix of VmInterfaceManager:: is redundant.
In addition, the two message parts can be squashed into one instead of
concatenating them.
Line 75: "therefore the action failed.",
iface.getName(), iface.getMacAddress());
Line 76: return null;
Line 77: }
Line 78: });
Line 85: AuditLogableBase logable = new AuditLogableBase();
Line 86: logable.addCustomValue("MACAddr",
iface.getMacAddress());
Line 87: logable.addCustomValue("IfaceName", iface.getName());
Line 88: log(logable,
AuditLogType.MAC_ADDRESS_IS_IN_USE_UNPLUG);
Line 89: log.warnFormat("VmInterfaceManager::Network Interface
{0} has MAC address {1} which is in use, " +
same comments as above.
In addition, please add the VM id to the log.
Line 90: "therefore it is being unplugged from VM.",
iface.getName(), iface.getMacAddress());
Line 91: return null;
Line 92: }
Line 93: });
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java
Line 330: * In case a MAC address is already in use, the user will be
issued a warning in the audit log.
Line 331: *
Line 332: * @param nics
Line 333: * The nics from snapshot.
Line 334: * @param version
please add: The compatibility version of the VM's cluster
Line 335: */
Line 336: protected void synchronizeNics(Guid vmId,
List<VmNetworkInterface> nics, CompensationContext compensationContext, Version
version) {
Line 337: VmInterfaceManager vmInterfaceManager = new
VmInterfaceManager();
Line 338:
....................................................
File
backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties
Line 321: USER_UPDATE_VM_POOL_WITH_VMS_FAILED=Failed to update VM Pool
${VmPoolName}(User: ${UserName}).
Line 322: USER_VM_POOL_MAX_SUBSEQUENT_FAILURES_REACHED=Not all VMs where
successfully created in VM Pool ${VmPoolName}.
Line 323: MAC_POOL_EMPTY=No MAC addresses left in the MAC Address Pool.
Line 324: MAC_ADDRESS_IS_IN_USE=Mac Address ${MACAddr} is in use.
Line 325: MAC_ADDRESS_IS_IN_USE_UNPLUG=Mac Address ${MACAddr} is in use,
therefore network interface ${IfaceName} was unplugged.
Adding the VM Name will aid to identify VM.
Line 326: MAC_ADDRESSES_POOL_NOT_INITIALIZED=Mac Address Pool is not
initialized. ${Message}
Line 327: USER_PASSWORD_CHANGED=Password changed successfully for ${UserName}
Line 328: USER_PASSWORD_CHANGE_FAILED=Failed to change password. (User:
${UserName})
Line 329: USER_CLEAR_UNKNOWN_VMS=All VMs' status on Non-Responsive Host
${VdsName} were changed to 'Down' by ${UserName}
--
To view, visit http://gerrit.ovirt.org/12241
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I269157b37426942331002c33a22fa354ba0cf39e
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Muli Salem <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Muli Salem <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches