Martin Betak 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
Done
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.",
Done
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());
While we already have an upgrade script which normalizes "" -> null, and will 
be adding validation to all commands setting the timezone, we still are missing 
handling of the case when user has older vm with invalid timezone that is 
non-empty ('blalbah', or just as simple typo such as 'Europe/Beriln') set via 
the REST API. Since we cannot fix such cases in db upgrade script, this is the 
last line of defense for us to prevent running a vm with invalid TZ. If the 
timezones had been validated from day 1, this of course would not be needed.
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,
Done
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."
Done
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: Michal Skrivanek <michal.skriva...@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

Reply via email to