Tal Nisan has posted comments on this change. Change subject: engine: Host name validation cleanup ......................................................................
Patch Set 1: (3 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java Line 287: addCanDoActionMessage(VdcBllMessages.VDS_CLUSTER_IS_NOT_VALID); Line 288: returnValue = false; Line 289: } else { Line 290: VDS vds = getParameters().getvds(); Line 291: String hostName = vds.gethost_name(); Agree, better to have it in the same context as the name validation, makes more sense Line 292: if(!validateVdsName(vds)) { Line 293: returnValue = false; Line 294: } else if (!ValidationUtils.validHostname(hostName)) { Line 295: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_INVALID_VDS_HOSTNAME); Line 290: VDS vds = getParameters().getvds(); Line 291: String hostName = vds.gethost_name(); Line 292: if(!validateVdsName(vds)) { Line 293: returnValue = false; Line 294: } else if (!ValidationUtils.validHostname(hostName)) { Should the validation of the host name be a part of the validateVdsName method? Not sure why it has to be separated Line 295: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_INVALID_VDS_HOSTNAME); Line 296: returnValue = false; Line 297: } else if (getDbFacade().getVdsDao().getByHostname(vds.gethost_name()) != null) { Line 298: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_VDS_WITH_SAME_HOST_EXIST); .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsCommand.java Line 201: final int maxVdsNameLength = Config.<Integer> GetValue(ConfigValues.MaxVdsNameLength); Line 202: final String vdsName = vds.getName(); Line 203: if (vdsName == null || vdsName.isEmpty()) { Line 204: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NAME_MAY_NOT_BE_EMPTY); Line 205: } else if (vdsName.length() > maxVdsNameLength) { I'd use vdsName.trim().length(), we don't want spaces in the start or end to be counted in the length Line 206: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NAME_LENGTH_IS_TOO_LONG); Line 207: } else if(getDbFacade().getVdsDao().getByName(vdsName) != null) { Line 208: return failCanDoAction(VdcBllMessages.VDS_TRY_CREATE_WITH_EXISTING_PARAMS); Line 209: } -- To view, visit http://gerrit.ovirt.org/11189 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5288bfd8020c8339d9d8870c6551b63351be79e4 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Laszlo Hornyak <lhorn...@redhat.com> Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches