Frank Kobzik has posted comments on this change.

Change subject: core: OsInfo-related changes
......................................................................


Patch Set 39:

(2 comments)

http://gerrit.ovirt.org/#/c/30838/39/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java:

Line 586:     /**
Line 587:      * Returns true if given graphics type was reset in params (that 
means params contain given graphics device which
Line 588:      * is set to null).
Line 589:      */
Line 590:     private boolean graphicsResetInParams(GraphicsType type) {
> i think there is already a method like this in the base class?
This code is dead and it seems I forgot about it.
Line 591:         Map<GraphicsType, GraphicsDevice> graphicsDevices = 
getParameters().getGraphicsDevices();
Line 592:         return graphicsDevices.containsKey(type) && 
graphicsDevices.get(type) == null;
Line 593:     }
Line 594: 


http://gerrit.ovirt.org/#/c/30838/39/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java:

Line 455: 
Line 456:         return true;
Line 457:     }
Line 458: 
Line 459:     Set<GraphicsType> getGraphicsTypesForVm() {
> its the same method in every class? why not put it vmDeviceUtils?
Good point! I did it, but only for VmTemplateCommand , VmManagementCommandBase 
and ChangeVMCClusterCommand classes. 

The behavior in this case is different and specific for ImportVmCommand 
(instead of reading devices from the db, devices are read from 
managedVmDeviceMap).
Line 460:         Set<GraphicsType> graphicsTypes = new HashSet<>();
Line 461: 
Line 462:         for (VmDevice vmDevice : 
getVm().getManagedVmDeviceMap().values()) {
Line 463:             if (VmDeviceGeneralType.GRAPHICS == vmDevice.getType()) {


-- 
To view, visit http://gerrit.ovirt.org/30838
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If0c7126308ab4e5e5418d804a8550a50e9994894
Gerrit-PatchSet: 39
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Roy Golan <[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