Thanks, Kyle. I added your Acked-by and applied this to master.
On Thu, Aug 30, 2012 at 01:21:36AM +0000, Kyle Mestery (kmestery) wrote: > Looks good to me. > > Acked-by: Kyle Mestery <[email protected]> > > On Aug 29, 2012, at 7:09 PM, Ben Pfaff wrote: > > > OVSDB has always had the ability to mark a column as "immutable", so that > > its value cannot be changed in a given row after that row is initially > > inserted. However, we discovered recently that ovsdb-server has never > > enforced this constraint. This commit implements enforcement. > > > > Reported-by: Paul Ingram <[email protected]> > > Signed-off-by: Ben Pfaff <[email protected]> > > --- > > NEWS | 2 ++ > > lib/ovsdb-idl-provider.h | 3 ++- > > ovsdb/execution.c | 18 +++++++++++++++++- > > ovsdb/mutation.c | 8 +++++++- > > ovsdb/ovsdb-idlc.in | 5 +++++ > > tests/ovs-vsctl.at | 12 ++++++++++++ > > tests/ovsdb-execution.at | 43 +++++++++++++++++++++++++++++++++++++++++++ > > tests/ovsdb-mutation.at | 12 ++++++++++++ > > utilities/ovs-vsctl.c | 22 ++++++++++++++++++++-- > > 9 files changed, 120 insertions(+), 5 deletions(-) > > > > diff --git a/NEWS b/NEWS > > index 872f8d0d..cbc5c58 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -18,6 +18,8 @@ post-v1.8.0 > > are true, but because we do not know of any users for this > > feature it seems better on balance to remove it. (The ovs-pki-cgi > > program was not included in distribution packaging.) > > + - ovsdb-server now enforces the immutability of immutable columns. > > This > > + was not enforced in earlier versions due to an oversight. > > - Stable bond mode is deprecated and will be removed no earlier than > > February 2013. Please email [email protected] with concerns. > > - The autopath action is deprecated and will be removed no earlier than > > diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h > > index 546acbb..7ea5ce6 100644 > > --- a/lib/ovsdb-idl-provider.h > > +++ b/lib/ovsdb-idl-provider.h > > @@ -1,4 +1,4 @@ > > -/* Copyright (c) 2009, 2010, 2011 Nicira, Inc. > > +/* Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc. > > * > > * Licensed under the Apache License, Version 2.0 (the "License"); > > * you may not use this file except in compliance with the License. > > @@ -41,6 +41,7 @@ struct ovsdb_idl_row { > > struct ovsdb_idl_column { > > char *name; > > struct ovsdb_type type; > > + bool mutable; > > void (*parse)(struct ovsdb_idl_row *, const struct ovsdb_datum *); > > void (*unparse)(struct ovsdb_idl_row *); > > }; > > diff --git a/ovsdb/execution.c b/ovsdb/execution.c > > index 1aff0c5..300c247 100644 > > --- a/ovsdb/execution.c > > +++ b/ovsdb/execution.c > > @@ -1,4 +1,4 @@ > > -/* Copyright (c) 2009, 2010, 2011 Nicira, Inc. > > +/* Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc. > > * > > * Licensed under the Apache License, Version 2.0 (the "License"); > > * you may not use this file except in compliance with the License. > > @@ -435,6 +435,22 @@ ovsdb_execute_update(struct ovsdb_execution *x, struct > > ovsdb_parser *parser, > > error = parse_row(row_json, table, x->symtab, &row, &columns); > > } > > if (!error) { > > + size_t i; > > + > > + for (i = 0; i < columns.n_columns; i++) { > > + const struct ovsdb_column *column = columns.columns[i]; > > + > > + if (!column->mutable) { > > + error = ovsdb_syntax_error(parser->json, > > + "constraint violation", > > + "Cannot update immutable column > > %s " > > + "in table %s.", > > + column->name, > > table->schema->name); > > + break; > > + } > > + } > > + } > > + if (!error) { > > error = ovsdb_condition_from_json(table->schema, where, x->symtab, > > &condition); > > } > > diff --git a/ovsdb/mutation.c b/ovsdb/mutation.c > > index 0dcd16f..5fd983a 100644 > > --- a/ovsdb/mutation.c > > +++ b/ovsdb/mutation.c > > @@ -1,4 +1,4 @@ > > -/* Copyright (c) 2009, 2010, 2011 Nicira, Inc. > > +/* Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc. > > * > > * Licensed under the Apache License, Version 2.0 (the "License"); > > * you may not use this file except in compliance with the License. > > @@ -95,6 +95,12 @@ ovsdb_mutation_from_json(const struct ovsdb_table_schema > > *ts, > > "No column %s in table %s.", > > column_name, ts->name); > > } > > + if (!m->column->mutable) { > > + return ovsdb_syntax_error(json, "constraint violation", > > + "Cannot mutate immutable column %s in " > > + "table %s.", column_name, ts->name); > > + } > > + > > ovsdb_type_clone(&m->type, &m->column->type); > > > > mutator_name = json_string(array->elems[1]); > > diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in > > index 0b1933b..478109a 100755 > > --- a/ovsdb/ovsdb-idlc.in > > +++ b/ovsdb/ovsdb-idlc.in > > @@ -577,11 +577,16 @@ static void\n%s_columns_init(void) > > for columnName, column in sorted(table.columns.iteritems()): > > cs = "%s_col_%s" % (structName, columnName) > > d = {'cs': cs, 'c': columnName, 's': structName} > > + if column.mutable: > > + mutable = "true" > > + else: > > + mutable = "false" > > print > > print " /* Initialize %(cs)s. */" % d > > print " c = &%(cs)s;" % d > > print " c->name = \"%(c)s\";" % d > > print column.type.cInitType(" ", "c->type") > > + print " c->mutable = %s;" % mutable > > print " c->parse = %(s)s_parse_%(c)s;" % d > > print " c->unparse = %(s)s_unparse_%(c)s;" % d > > print "}" > > diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at > > index 84cc272..e9036191 100644 > > --- a/tests/ovs-vsctl.at > > +++ b/tests/ovs-vsctl.at > > @@ -736,6 +736,18 @@ AT_CHECK([RUN_OVS_VSCTL([clear netflow `cat > > netflow-uuid` targets])], > > AT_CHECK([RUN_OVS_VSCTL([destroy b br2])], > > [1], [], [ovs-vsctl: no row "br2" in table Bridge > > ], [OVS_VSCTL_CLEANUP]) > > +AT_CHECK([RUN_OVS_VSCTL([add i br1 name x])], > > + [1], [], [ovs-vsctl: cannot modify read-only column name in table > > Interface > > +], [OVS_VSCTL_CLEANUP]) > > +AT_CHECK([RUN_OVS_VSCTL([set port br1 name br2])], > > + [1], [], [ovs-vsctl: cannot modify read-only column name in table Port > > +], [OVS_VSCTL_CLEANUP]) > > +AT_CHECK([RUN_OVS_VSCTL([remove b br1 name br1])], > > + [1], [], [ovs-vsctl: cannot modify read-only column name in table Bridge > > +], [OVS_VSCTL_CLEANUP]) > > +AT_CHECK([RUN_OVS_VSCTL([clear b br1 name])], > > + [1], [], [ovs-vsctl: cannot modify read-only column name in table Bridge > > +], [OVS_VSCTL_CLEANUP]) > > OVS_VSCTL_CLEANUP > > AT_CLEANUP > > > > diff --git a/tests/ovsdb-execution.at b/tests/ovsdb-execution.at > > index 480f610..6a3b5d1 100644 > > --- a/tests/ovsdb-execution.at > > +++ b/tests/ovsdb-execution.at > > @@ -111,6 +111,15 @@ gc_schema () { > > "isRoot": false}}} > > EOF > > } > > + > > +immutable_schema () { > > + cat <<'EOF' > > +{"name": "immutable", > > + "tables": { > > + "a": { > > + "columns": {"i": {"type": "integer", "mutable": false}}}}} > > +EOF > > +} > > ] > > m4_divert_pop([PREPARE_TESTS]) > > > > @@ -908,6 +917,40 @@ OVSDB_CHECK_EXECUTION([weak references], > > [{"rows":[{"_uuid":["uuid","<3>"],"b":2,"b2a":["set",[]]},{"_uuid":["uuid","<4>"],"b":3,"b2a":["set",[]]}]}] > > ]]) > > > > +OVSDB_CHECK_EXECUTION([immutable columns], > > + [immutable_schema], > > + [[[["immutable", > > + {"op": "insert", > > + "table": "a", > > + "row": {"i": 5}, > > + "uuid-name": "row1"}]]], > > + [[["immutable", > > + {"op": "update", > > + "table": "a", > > + "row": {"i": 10}, > > + "where": []}]]], > > + [[["immutable", > > + {"op": "update", > > + "table": "a", > > + "row": {"i": 5}, > > + "where": []}]]], > > + [[["immutable", > > + {"op": "mutate", > > + "table": "a", > > + "where": [], > > + "mutations": [["i", "-=", 5]]}]]], > > + [[["immutable", > > + {"op": "mutate", > > + "table": "a", > > + "where": [], > > + "mutations": [["i", "*=", 1]]}]]]], > > + [[[{"uuid":["uuid","<0>"]}] > > +[{"details":"Cannot update immutable column i in table > > a.","error":"constraint > > violation","syntax":"{\"op\":\"update\",\"row\":{\"i\":10},\"table\":\"a\",\"where\":[]}"}] > > +[{"details":"Cannot update immutable column i in table > > a.","error":"constraint > > violation","syntax":"{\"op\":\"update\",\"row\":{\"i\":5},\"table\":\"a\",\"where\":[]}"}] > > +[{"details":"Cannot mutate immutable column i in table > > a.","error":"constraint violation","syntax":"[\"i\",\"-=\",5]"}] > > +[{"details":"Cannot mutate immutable column i in table > > a.","error":"constraint violation","syntax":"[\"i\",\"*=\",1]"}] > > +]]) > > + > > OVSDB_CHECK_EXECUTION([garbage collection], > > [gc_schema], > > [dnl Check that inserting a row without any references is a no-op. > > diff --git a/tests/ovsdb-mutation.at b/tests/ovsdb-mutation.at > > index 3d75317..fc898b5 100644 > > --- a/tests/ovsdb-mutation.at > > +++ b/tests/ovsdb-mutation.at > > @@ -99,6 +99,18 @@ test-ovsdb: syntax > > "["u","delete",["uuid","9179ca6d-6d65-400a-b455-3ad92783a099" > > ]]) > > AT_CLEANUP > > > > +AT_SETUP([disallowed mutations on immutable columns]) > > +AT_KEYWORDS([ovsdb negative mutation]) > > +AT_CHECK([[test-ovsdb parse-mutations \ > > + '{"columns": > > + {"i": {"type": "integer", "mutable": false}}}' \ > > + '[["i", "+=", 1]]' > > +]], > > + [1], [], > > + [[test-ovsdb: syntax "["i","+=",1]": constraint violation: Cannot mutate > > immutable column i in table mytable. > > +]]) > > +AT_CLEANUP > > + > > OVSDB_CHECK_POSITIVE([mutations on sets], > > [[parse-mutations \ > > '{"columns": > > diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c > > index 0a7c2c6..e6bd63b 100644 > > --- a/utilities/ovs-vsctl.c > > +++ b/utilities/ovs-vsctl.c > > @@ -2763,7 +2763,7 @@ error: > > return error; > > } > > > > -static void > > +static const struct ovsdb_idl_column * > > pre_parse_column_key_value(struct vsctl_context *ctx, > > const char *arg, > > const struct vsctl_table_class *table) > > @@ -2780,6 +2780,18 @@ pre_parse_column_key_value(struct vsctl_context *ctx, > > > > pre_get_column(ctx, table, column_name, &column); > > free(column_name); > > + > > + return column; > > +} > > + > > +static void > > +check_mutable(const struct vsctl_table_class *table, > > + const struct ovsdb_idl_column *column) > > +{ > > + if (!column->mutable) { > > + vsctl_fatal("cannot modify read-only column %s in table %s", > > + column->name, table->class->name); > > + } > > } > > > > static void > > @@ -3112,7 +3124,10 @@ pre_cmd_set(struct vsctl_context *ctx) > > > > table = pre_get_table(ctx, table_name); > > for (i = 3; i < ctx->argc; i++) { > > - pre_parse_column_key_value(ctx, ctx->argv[i], table); > > + const struct ovsdb_idl_column *column; > > + > > + column = pre_parse_column_key_value(ctx, ctx->argv[i], table); > > + check_mutable(table, column); > > } > > } > > > > @@ -3195,6 +3210,7 @@ pre_cmd_add(struct vsctl_context *ctx) > > > > table = pre_get_table(ctx, table_name); > > pre_get_column(ctx, table, column_name, &column); > > + check_mutable(table, column); > > } > > > > static void > > @@ -3251,6 +3267,7 @@ pre_cmd_remove(struct vsctl_context *ctx) > > > > table = pre_get_table(ctx, table_name); > > pre_get_column(ctx, table, column_name, &column); > > + check_mutable(table, column); > > } > > > > static void > > @@ -3316,6 +3333,7 @@ pre_cmd_clear(struct vsctl_context *ctx) > > const struct ovsdb_idl_column *column; > > > > pre_get_column(ctx, table, ctx->argv[i], &column); > > + check_mutable(table, column); > > } > > } > > > > -- > > 1.7.2.5 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
