Frank Kobzik has posted comments on this change. Change subject: backend: RNG device sources reporting ......................................................................
Patch Set 26: (5 comments) http://gerrit.ovirt.org/#/c/22258/26/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractRngDeviceCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractRngDeviceCommand.java: Line 69: Line 70: return true; Line 71: } Line 72: Line 73: boolean isRngSupportedByCluster() { > why did you remove the method modifier? because of testing. i needed to mock this method. to be honest i don't like it, because i'm mocking logic of the class itself. i'll rewrite it so that i'll mock only FeatureSupported, which is actually the intention. thanks for noticing. Line 74: VDSGroup cluster = getVdsGroup(); Line 75: VmRngDevice.Source source = getParameters().getRngDevice().getSource(); Line 76: return cluster != null && FeatureSupported.virtIoRngSupported(cluster.getcompatibility_version()) Line 77: && cluster.getRequiredRngSources().contains(source); http://gerrit.ovirt.org/#/c/22258/26/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java: Line 1730: @TypeConverterAttribute(String.class) Line 1731: @DefaultValueAttribute("RANDOM") Line 1732: ClusterRequiredRngSourcesDefault, Line 1733: Line 1734: Invalid; > why did you add this ';' i dont think its needed, and for sure not related my fault. i added it while rebasing (thought it was correct). http://gerrit.ovirt.org/#/c/22258/26/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 410: Line 411: vds.setSupportedEmulatedMachines(AssignStringValueFromArray(xmlRpcStruct, VdsProperties.emulatedMachines)); Line 412: Line 413: vds.getSupportedRngSources().clear(); Line 414: vds.getSupportedRngSources().addAll(VmRngDevice.csvToSourcesSet(AssignStringValueFromArray(xmlRpcStruct, VdsProperties.rngSources).toUpperCase())); > did you verify this works ok on hosts that doesn't report this value? thanks for this catch. of course it'll crash - when there is no rngSources key in xmlRpcStruct, then ASVFA will return null. Line 415: Line 416: String hooksStr = ""; // default value if hooks is not in the xml rpc struct Line 417: if (xmlRpcStruct.containsKey(VdsProperties.hooks)) { Line 418: hooksStr = xmlRpcStruct.get(VdsProperties.hooks).toString(); http://gerrit.ovirt.org/#/c/22258/26/packaging/dbscripts/create_views.sql File packaging/dbscripts/create_views.sql: Line 754: vds_spm_id_map ON vds_static.vds_id = vds_spm_id_map.vds_id; Line 755: Line 756: Line 757: Line 758: CREATE OR REPLACE VIEW vds_with_tags > please add here as well (not sure if needed) i'm not sure either. i didn't include in it last patchset so far. there are differences between these views and i'm not sure what should i do. i'll ask around. Line 759: as Line 760: SELECT vds_groups.vds_group_id, vds_groups.name AS vds_group_name, vds_groups.description AS vds_group_description, vds_groups.architecture as architecture, Line 761: vds_static.vds_id, vds_static.vds_name, vds_static.ip, vds_static.vds_unique_id, Line 762: vds_static.host_name, vds_static.free_text_comment, vds_static.port, vds_static.vds_strength, vds_static.server_SSL_enabled, vds_static.vds_type, http://gerrit.ovirt.org/#/c/22258/26/packaging/dbscripts/upgrade/03_05_0470_add_rng_device_columns.sql File packaging/dbscripts/upgrade/03_05_0470_add_rng_device_columns.sql: Line 1: -- rng sources required by cluster Line 2: select fn_db_add_column('vds_groups', 'required_rng_sources', 'varchar(255)'); > should we put 'RANDOM' for 3.5 clusters? we can i guess. all vdsms compatible with >= 3.5 should support rng. Line 3: -- rng sources supported by host -- 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: 26 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: 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
