Omer Frenkel has posted comments on this change.

Change subject: backend: Implementation support for GuestOsInfo and Timezone 
reporting
......................................................................


Patch Set 18:

(2 comments)

https://gerrit.ovirt.org/#/c/29322/18/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java:

Line 435:     }
Line 436: 
Line 437:     private static void updateGuestOsInfo(VmDynamic vm, Map<String, 
Object> xmlRpcStruct) {
Line 438:         Map<String, Object> guestOsInfoStruct = (Map<String, Object>) 
xmlRpcStruct.get(VdsProperties.GUEST_OS_INFO);
Line 439:         vm.setGuestOsArch(AssignStringValue(guestOsInfoStruct, 
VdsProperties.GUEST_OS_INFO_ARCH));
please add try..catch for the case vdsm send unknown arch type
Line 440:         vm.setGuestOsCodename(AssignStringValue(guestOsInfoStruct, 
VdsProperties.GUEST_OS_INFO_CODENAME));
Line 441:         
vm.setGuestOsDistribution(AssignStringValue(guestOsInfoStruct, 
VdsProperties.GUEST_OS_INFO_DISTRIBUTION));
Line 442:         
vm.setGuestOsKernelVersion(AssignStringValue(guestOsInfoStruct, 
VdsProperties.GUEST_OS_INFO_KERNEL));
Line 443:         vm.setGuestOsType(AssignStringValue(guestOsInfoStruct, 
VdsProperties.GUEST_OS_INFO_TYPE));


Line 439:         vm.setGuestOsArch(AssignStringValue(guestOsInfoStruct, 
VdsProperties.GUEST_OS_INFO_ARCH));
Line 440:         vm.setGuestOsCodename(AssignStringValue(guestOsInfoStruct, 
VdsProperties.GUEST_OS_INFO_CODENAME));
Line 441:         
vm.setGuestOsDistribution(AssignStringValue(guestOsInfoStruct, 
VdsProperties.GUEST_OS_INFO_DISTRIBUTION));
Line 442:         
vm.setGuestOsKernelVersion(AssignStringValue(guestOsInfoStruct, 
VdsProperties.GUEST_OS_INFO_KERNEL));
Line 443:         vm.setGuestOsType(AssignStringValue(guestOsInfoStruct, 
VdsProperties.GUEST_OS_INFO_TYPE));
in case of bad os name, this implementation inside would just set "other" i 
think its better to check here if the os is ok and if not log a warning and 
then set "other"
Line 444:         vm.setGuestOsVersion(AssignStringValue(guestOsInfoStruct, 
VdsProperties.GUEST_OS_INFO_VERSION));
Line 445:     }
Line 446: 
Line 447:     /**


-- 
To view, visit https://gerrit.ovirt.org/29322
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idc1e54d39706601c0faf9d76785f1257ee20755b
Gerrit-PatchSet: 18
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Martin Betak <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Shahar Havivi <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to