Omer Frenkel has posted comments on this change. Change subject: backend: RNG device sources reporting ......................................................................
Patch Set 6: (4 comments) http://gerrit.ovirt.org/#/c/22258/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateRngDeviceCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateRngDeviceCommand.java: Line 26: VmRngDevice device = getParameters().getRngDevice(); Line 27: log.infoFormat("RNG Device id: %s of VM id: %s is no more compatible with cluster. Removing the device.", Line 28: device.getId(), device.getVmId()); Line 29: Line 30: Backend.getInstance().runAction(VdcActionType.RemoveRngDevice, getParameters()); it doesnt make sense to call an action inside canDoAction phase. you can move this code to execute and add some informative audit log Line 31: Line 32: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_RNG_SOURCE_NOT_SUPPORTED); Line 33: } Line 34: http://gerrit.ovirt.org/#/c/22258/6/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateRngDeviceTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateRngDeviceTest.java: Line 24: Assert.assertEquals(true, command.canDoAction()); Line 25: } Line 26: Line 27: private UpdateRngDeviceCommand mockCommand(final Version mockCompatibilityVersion) { Line 28: final Guid vmId = new Guid("a09f57b1-5739-4352-bf88-a6f834ed46db"); why not use IDs from FixturesTool? Line 29: final Guid clusterId = new Guid("e862dae0-5c41-416a-922c-5395e7245c9b"); Line 30: final Guid deviceId = new Guid("b24ae590-f42b-49b6-b8f4-cbbc720b230d"); Line 31: VmRngDevice dev = getDevice(deviceId, vmId); Line 32: http://gerrit.ovirt.org/#/c/22258/6/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsGroupDAODbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsGroupDAODbFacadeImpl.java: Line 228: entity.setMigrateOnError(MigrateOnErrorOptions.forValue(rs.getInt("migrate_on_error"))); Line 229: entity.setVirtService(rs.getBoolean("virt_service")); Line 230: entity.setGlusterService(rs.getBoolean("gluster_service")); Line 231: entity.setTunnelMigration(rs.getBoolean("tunnel_migration")); Line 232: entity.getRequiredRngSources().clear(); why do you need to call clear if this is a new entity? Line 233: entity.getRequiredRngSources().addAll(VmRngDevice.csvToSourcesSet(rs.getString("required_rng_sources"))); Line 234: entity.setEmulatedMachine(rs.getString("emulated_machine")); Line 235: entity.setDetectEmulatedMachine(rs.getBoolean("detect_emulated_machine")); Line 236: entity.setTrustedService(rs.getBoolean("trusted_service")); http://gerrit.ovirt.org/#/c/22258/6/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql File packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql: Line 228: select fn_db_add_config_value('ClusterRequiredRngSourcesDefault','','3.1'); Line 229: select fn_db_add_config_value('ClusterRequiredRngSourcesDefault','','3.2'); Line 230: select fn_db_add_config_value('ClusterRequiredRngSourcesDefault','','3.3'); Line 231: select fn_db_add_config_value('ClusterRequiredRngSourcesDefault','','3.4'); Line 232: select fn_db_add_config_value('ClusterRequiredRngSourcesDefault','random','3.5'); all hosts in 3.5 reports the random rng source? need to be sure we dont make hosts move to non-op. with no reason Line 233: Line 234: -- by default use no proxy Line 235: select fn_db_add_config_value('SpiceProxyDefault','','general'); Line 236: -- To view, visit http://gerrit.ovirt.org/22258 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id60a39118aef439146e6f0f308eec84e1ccaab9a Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Itamar Heim <ih...@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
