Allon Mureinik has posted comments on this change. Change subject: core: Configurable Default for Wipe After Delete per Storage Domain ......................................................................
Patch Set 2: (6 comments) http://gerrit.ovirt.org/#/c/35909/2/backend/manager/modules/dal/src/test/resources/fixtures.xml File backend/manager/modules/dal/src/test/resources/fixtures.xml: Line 4341: <value>2014-04-24 12:00:00</value> Line 4342: <value>2014-04-24 12:00:00</value> Line 4343: <value>7</value> Line 4344: <value>POSIX domain, part of 'rhel6.Mixed' storage pool</value> Line 4345: <value>true</value> Please mix a couple of falses in there somewhere Line 4346: </row> Line 4347: </table> Line 4348: Line 4349: <table name="storage_domain_dynamic"> http://gerrit.ovirt.org/#/c/35909/2/packaging/dbscripts/storages_sp.sql File packaging/dbscripts/storages_sp.sql: Line 418: v_storage_type INTEGER, Line 419: v_storage_domain_type INTEGER, Line 420: v_storage_domain_format_type VARCHAR(50), Line 421: v_last_time_used_as_master BIGINT, Line 422: v_wipe_after_delete boolean) please fix the indentation Line 423: RETURNS VOID Line 424: AS $procedure$ Line 425: BEGIN Line 426: INSERT INTO storage_domain_static(id, storage,storage_name, storage_description, storage_comment, storage_type, storage_domain_type, storage_domain_format_type, last_time_used_as_master, wipe_after_delete) Line 450: v_storage_type INTEGER, Line 451: v_storage_domain_type INTEGER, Line 452: v_storage_domain_format_type INTEGER, Line 453: v_last_time_used_as_master BIGINT, Line 454: v_wipe_after_delete boolean) here too Line 455: RETURNS VOID Line 456: Line 457: --The [storage_domain_static] table doesn't have a timestamp column. Optimistic concurrency logic cannot be generated Line 458: AS $procedure$ http://gerrit.ovirt.org/#/c/35909/2/packaging/dbscripts/upgrade/03_06_0590_add_wipe_after_delete_to_storage_domain_static.sql File packaging/dbscripts/upgrade/03_06_0590_add_wipe_after_delete_to_storage_domain_static.sql: Line 7: v_table = 'storage_domain_static'; Line 8: v_column = 'wipe_after_delete'; Line 9: if (not exists (SELECT 1 Line 10: FROM information_schema.columns Line 11: WHERE table_name ilike v_table AND check how the information schema stores this (iirc, it's in uppercase), and use "=" instead of "ilike". There's no regex here, just a straight-forward equality. Line 12: column_name ilike v_column)) then Line 13: -- The wipe_after_delete column does not exist. Line 14: PERFORM fn_db_add_column(v_table, v_column, 'boolean NOT NULL DEFAULT false'); Line 15: Line 10: FROM information_schema.columns Line 11: WHERE table_name ilike v_table AND Line 12: column_name ilike v_column)) then Line 13: -- The wipe_after_delete column does not exist. Line 14: PERFORM fn_db_add_column(v_table, v_column, 'boolean NOT NULL DEFAULT false'); please use spaces, not tabs. Also, worth considering using the alter statement directly. Line 15: Line 16: UPDATE storage_domain_static Line 17: SET wipe_after_delete = true Line 18: FROM vdc_options Line 19: WHERE vdc_options.option_name = 'SANWipeAfterDelete' AND Line 20: vdc_options.version = 'general' AND Line 21: vdc_options.option_value = 'true' AND Line 22: ( storage_domain_static.storage_type = 2 OR Line 23: storage_domain_static.storage_type = 3); -- 2 and 3 are the only storage types which are block domains. please use the IN operator Line 24: end if; Line 25: end Line 26: $$ LANGUAGE plpgsql; Line 27: -- To view, visit http://gerrit.ovirt.org/35909 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4e0d5a1c9158be1284ffe05d1bf1397c9113f6c0 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Ori Liel <ol...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@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
