Itamar Heim has posted comments on this change.
Change subject: tools: engine-config need to remove all ver 2.2 values
......................................................................
Patch Set 6: I would prefer that you didn't submit this
(25 inline comments)
multiple keys here are deleted which i don't think are correct.
in general, this patch is a bit big to review. a patch per key would have been
much nicer allowing a discussion per key, then a cleanup patch for the
remaining items.
or at least the commit message to describe the list of changes done by this
patch:
1. obsolete keys/code
2. keys moved from per cluster level to one general value.
....................................................
File backend/manager/dbscripts/upgrade/pre_upgrade/0000_config.sql
Line 89
Line 90
Line 91
Line 92
Line 93
per cluster level (probably)
Line 284
Line 285
Line 286
Line 287
Line 288
per cluster level
Line 287
Line 288
Line 289
Line 290
Line 291
per cluster level
Line 291
Line 292
Line 293
Line 294
Line 295
per cluster level
Line 380
Line 381
Line 382
Line 383
Line 384
per cluster level
Line 521
Line 522
Line 523
Line 524
Line 525
per cluster level
Line 536
Line 537
Line 538
Line 539
Line 540
per cluster level
Line 616
Line 617
Line 618
Line 619
Line 620
per cluster level
Line 661: select fn_db_delete_config_value('VdsFenceType','3.2');
Line 662: select fn_db_delete_config_value('VM64BitMaxMemorySizeInMB','2.2');
Line 663: select fn_db_delete_config_value('VM64BitMaxMemorySizeInMB','3.0');
Line 664: select fn_db_delete_config_value('VM64BitMaxMemorySizeInMB','3.1');
Line 665: select fn_db_delete_config_value('VM64BitMaxMemorySizeInMB','3.2');
why not just delete all where cluster level ='2.2' as well?
Line 666:
------------------------------------------------------------------------------------
Line 667: -- Split config section
Line 668: -- The purpose of this section is to treat config option that was once
Line 669: -- general, and should now be version-specific.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
Line 242
Line 243
Line 244
Line 245
Line 246
per cluster level
Line 247
Line 248
Line 249
Line 250
Line 251
per cluster level
Line 252
Line 253
Line 254
Line 255
Line 256
per cluster level
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmCustomPropertiesQuery.java
Line 22
Line 23
Line 24
Line 25
Line 26
per cluster level
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsCommand.java
Line 168
Line 169
Line 170
Line 171
Line 172
why is this not by cluster level?
btw, there is a spelling mistake worth fixing here (s/clsuster/cluster)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
Line 328
Line 329
Line 330
Line 331
Line 332
per cluster level
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
Line 66
Line 67
Line 68
Line 69
Line 70
per cluster level
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.java
Line 65
Line 66
Line 67
Line 68
Line 69
per cluster level
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
Line 755: @DefaultValueAttribute("true")
Line 756: SendSMPOnRunVm(215),
Line 757: @Reloadable
Line 758: @TypeConverterAttribute(String.class)
Line 759: @DefaultValueAttribute("pc-0.14")
why?
Line 760: EmulatedMachine(216),
Line 761: @TypeConverterAttribute(String.class)
Line 762: @DefaultValueAttribute(" WHERE RowNum BETWEEN %1$s AND %2$s")
Line 763: @OptionBehaviourAttribute(behaviour =
OptionBehaviour.ValueDependent, dependentOn = ConfigValues.DBEngine,
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/ConfigurationValues.java
Line 58
Line 59
Line 60
Line 61
Line 62
this should remain
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/VmValidationUtils.java
Line 16
Line 17
Line 18
Line 19
Line 20
why is this not per cluster level?
Line 35
Line 36
Line 37
Line 38
Line 39
same, should be per cluster level
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendCapabilitiesResource.java
Line 385
Line 386
Line 387
Line 388
Line 389
why is this not per cluster level?
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/vmproperties/VmPropertiesUtils.java
Line 144
Line 145
Line 146
Line 147
Line 148
why is this not per cluster level?
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilderBase.java
Line 51: }
Line 52: final String compatibilityVersion =
vm.getvds_group_compatibility_version().toString();
Line 53: addCpuPinning(compatibilityVersion);
Line 54: createInfo.add(VdsProperties.emulatedMachine, Config.<String>
GetValue(
Line 55: ConfigValues.EmulatedMachine));
EmulatedMachine is per compatibility version
Line 56: // send cipher suite and spice secure channels parameters only
if ssl
Line 57: // enabled.
Line 58: if (Config.<Boolean> GetValue(ConfigValues.SSLEnabled)) {
Line 59: createInfo.add(VdsProperties.spiceSslCipherSuite,
Line 208:
Line 209: if (vm.getvm_type() == VmType.Desktop) {
Line 210:
Line 211: String soundDeviceTypeConfig = Config.<String> GetValue(
Line 212: ConfigValues.DesktopAudioDeviceType);
how does removing per compatibility version will deal with some OSs only
relevant for newer cluster levels?
Line 213: String vmOS = vm.getos().name();
Line 214:
Line 215: Pattern regexPattern = Pattern.compile(String
Line 216: .format(OS_REGEX, vmOS));
--
To view, visit http://gerrit.ovirt.org/9131
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I08cccaa4724ae84785fa0f3d5ea4141626c0f575
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Itamar Heim <ih...@redhat.com>
Gerrit-Reviewer: Livnat Peer <lp...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
