Omer Frenkel has posted comments on this change.
Change subject: engine: Incorrect VM TimeZone handling
......................................................................
Patch Set 5:
(5 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
Line 634: protected void handleMemoryAdjustments() {
Line 635: // nothing to do in RunVmCommand class
Line 636: }
Line 637:
Line 638: // set the timezone to use the engine default if trying to run VM
with incorrect TZ set
please use javadoc for method description
Line 639: private void handleTimeZone() {
Line 640:
Line 641: if (!isVmTimeZoneValid(getVm())) {
Line 642: log.warnFormat("ResourceManager::{0}::Attempting to run
VM {1} with incorrect timezone '{2}'. Resetting timezone to engine default.",
Line 638: // set the timezone to use the engine default if trying to run VM
with incorrect TZ set
Line 639: private void handleTimeZone() {
Line 640:
Line 641: if (!isVmTimeZoneValid(getVm())) {
Line 642: log.warnFormat("ResourceManager::{0}::Attempting to run
VM {1} with incorrect timezone '{2}'. Resetting timezone to engine default.",
"ResourceManage" is not relevant, class name already logged by log4j and vm
name is better than vm id (or both)
please use this log message:
log.warnFormat("Attempting to run VM {0} - {1} with
incorrect timezone '{2}'. Resetting timezone to engine default.",
getVmId(), getVm().getName(), getVm().getTimeZone());
Line 643: getClass().getName(),
Line 644: getVmId().toString(),
Line 645: getVm().getTimeZone());
Line 646: getVm().getStaticData().setTimeZone(null);
Line 645: getVm().getTimeZone());
Line 646: getVm().getStaticData().setTimeZone(null);
Line 647: // only persist corrected timezone if this wasn't a
Run-Once
Line 648: if (!getVm().isRunOnce()) {
Line 649: getVmStaticDAO().update(getVm().getStaticData());
it doesn't make much sense that run vm command update the vm timezone,
invalid timezones should be blocked in update vm (as done in this patch) and in
add vm, and provide an upgrade script for any existing vms with invalid TZ.
Line 650: }
Line 651: }
Line 652: }
Line 653:
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java
Line 214: ACTION_TYPE_FAILED_HOST_NOT_EXIST(ErrorType.BAD_PARAMETERS),
Line 215: ACTION_TYPE_FAILED_VM_SNAPSHOT_NOT_IN_PREVIEW(ErrorType.CONFLICT),
Line 216:
ACTION_TYPE_FAILED_SHAREABLE_DISK_NOT_SUPPORTED(ErrorType.CONFLICT),
Line 217: FAILED_TO_RUN_LDAP_QUERY(ErrorType.INTERNAL_ERROR),
Line 218: ACTION_TYPE_FAILED_INVALID_TIMEZONE,
please add error type, i believe bad_params is suitable here
Line 219:
Line 220: VDS_CANNOT_REMOVE_DEFAULT_VDS_GROUP(ErrorType.CONFLICT),
Line 221: VDS_CANNOT_REMOVE_VDS_DETECTED_RUNNING_VM(ErrorType.CONFLICT),
Line 222: ACTION_TYPE_FAILED_DETECTED_PINNED_VMS(ErrorType.CONFLICT),
....................................................
File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
Line 739: ROLE_TYPE_CANNOT_BE_EMPTY=Invalid Role Type.
Line 740: CANNOT_ADD_ACTION_GROUPS_TO_ROLE_TYPE=Cannot add administrator's
action group to a User Role.
Line 741: STORAGE_DOMAIN_NOT_ATTACHED_TO_STORAGE_POOL=Storage Domain is already
detached from the Data Center.
Line 742: VDS_APPROVE_VDS_NOT_FOUND=Cannot approve Host - Host does not exists.
Line 743: ACTION_TYPE_FAILED_INVALID_TIMEZONE="Invalid time zone for given VM
OS type."
no need for quotes signs, also if its a general "action type" message (for
example to be used in addVm and in updateVm, change message to be:
Cannot ${action} ${type}. Invalid time zone for given VM OS type.
Line 744:
Line 745: # Quota messages.
Line 746: ACTION_TYPE_FAILED_QUOTA_NOT_EXIST=Cannot ${action} ${type}. Quota
doesn't exist.
Line 747: ACTION_TYPE_FAILED_QUOTA_IS_NOT_VALID=Cannot ${action} ${type}. Quota
is not valid.
--
To view, visit http://gerrit.ovirt.org/17524
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd36656bd4f0288e86b1cbd46d702fa68051e28b
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Betak <mbe...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Martin Betak <mbe...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@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
