Frank Kobzik has posted comments on this change. Change subject: core: OsInfo-related changes ......................................................................
Patch Set 30: (3 comments) http://gerrit.ovirt.org/#/c/30838/30/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 322: protected boolean canDoAddVmCommand() { Line 323: boolean returnValue = false; Line 324: returnValue = areParametersLegal(getReturnValue().getCanDoActionMessages()); Line 325: // Check if number of monitors passed is legal Line 326: returnValue = returnValue && checkNumberOfMonitors(getParameters().graphicsTypesToBeSet()) > why do you need to pass it as parameter? its accessible from the method as I changed it. There is no need for the argument now. Line 327: && checkSingleQxlDisplay(); Line 328: Line 329: returnValue = Line 330: returnValue http://gerrit.ovirt.org/#/c/30838/30/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java: Line 156: } Line 157: } Line 158: Line 159: // check graphics devices inherited from template Line 160: if (!VmHandler.isGraphicsAndDisplaySupported(getParameters().getVmStaticData().getOsId(), > why here and not in addVmCommand? this command is the same but responsible I actually removed it from this class. It's checked using super in AddVmCommand. Line 161: getEntityGraphicsTypes(getVmTemplateId()), Line 162: getParameters().getVm().getDefaultDisplayType(), Line 163: getReturnValue().getCanDoActionMessages(), Line 164: getVdsGroup().getcompatibility_version())) { http://gerrit.ovirt.org/#/c/30838/30/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 459: Line 460: Set<GraphicsType> getGraphicsTypesForVm(VM vm) { Line 461: Set<GraphicsType> graphicsTypes = new HashSet<>(); Line 462: Line 463: for (VmDevice vmDevice : DbFacade.getInstance().getVmDeviceDao().getVmDeviceByVmIdAndType(vm.getId(), VmDeviceGeneralType.GRAPHICS)) { > how this works? on import vm the vm is not in the db, getting devices from You're right. Line 464: graphicsTypes.add(GraphicsType.fromVmDeviceType(VmDeviceType.valueOf(vmDevice.getDevice()))); Line 465: } Line 466: Line 467: return graphicsTypes; -- 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: 30 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org 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
