Thanks!
Please let me know if you think I can help implementing something or be
useful in some other way.
I'll also try to propose something.
2017-01-05 17:48 GMT+01:00 Ben Pfaff <b...@ovn.org>:
> On Thu, Dec 29, 2016 at 03:55:46PM -0700, Lukasz Rzasik wrote:
> > Adding / removing a range of integers to a column accepting a set of
> > integers requires enumarating all of the integers. This patch simplifies
> > it by introducing 'range' concept to the database commands. Two integers
> > separated by a hyphen represent an inclusive range.
> >
> > The patch adds positive and negative tests for the new syntax.
> > The patch was tested by 'make check'. Covarage was tested by
> > 'make check-lcov'.
> >
> > Signed-off-by: Lukasz Rzasik <lukasz.rza...@gmail.com>
> > Suggested-by: <my_ovs_disc...@yahoo.com>
> > Suggested-by: Ben Pfaff <b...@ovn.org>
>
> Thanks. I applied this to master.
>
> I folded in the following incremental, which is mostly stylistic.
>
> --8<--------------------------cut here-------------------------->8--
>
> diff --git a/NEWS b/NEWS
> index beee6cc..1b0aa5d 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -59,6 +59,9 @@ Post-v2.6.0
> * Ports now have a "protected" flag. Protected ports can not forward
> frames to other protected ports. Unprotected ports can receive and
> forward frames to protected and other unprotected ports.
> + - ovs-vsctl, ovn-nbctl, ovn-sbctl, vtep-ctl:
> + * Database commands now accept integer ranges, e.g. "set port
> + eth0 trunks=1-10" to enable trunking VLANs 1 to 10.
>
> v2.6.0 - 27 Sep 2016
> ---------------------
> diff --git a/lib/db-ctl-base.man b/lib/db-ctl-base.man
> index 26828d6..88529b5 100644
> --- a/lib/db-ctl-base.man
> +++ b/lib/db-ctl-base.man
> @@ -31,7 +31,7 @@ columns can have an empty set of values, represented as
> \fB[]\fR, and
> square brackets may optionally enclose other non-empty sets or single
> values as well. For a column accepting a set of integers, database
> commands
> accept a range. A range is represented by two integers separated by
> -\fB-\fR. A range is inclusive. A range have a maximum size of 4096
> +\fB-\fR. A range is inclusive. A range has a maximum size of 4096
> elements. If more elements are needed, they can be specified in seperate
> ranges.
> .PP
> diff --git a/lib/ovsdb-data.c b/lib/ovsdb-data.c
> index a7a6eef..840e18f 100644
> --- a/lib/ovsdb-data.c
> +++ b/lib/ovsdb-data.c
> @@ -501,9 +501,9 @@ ovsdb_atom_from_string__(union ovsdb_atom *atom,
> long long int integer, end;
> if (range_end_atom
> && str_to_llong_range(s, 10, &integer, &end)) {
> - if (!ovsdb_atom_range_check(integer, end)) {
> + if (end < integer) {
> return xasprintf("\"%s\" is not a valid range. "
> - "Range start has to be less than end.", s);
> + "Range end cannot be before start.", s);
> }
> *range_end_atom = alloc_default_atoms(type, 1);
> if (!(*range_end_atom)) {
> @@ -638,24 +638,22 @@ ovsdb_atom_from_string(union ovsdb_atom *atom,
>
> if (!error && range_end_atom && *range_end_atom) {
> /* Check range constraints */
> + int64_t start = atom->integer;
> + int64_t end = (*range_end_atom)->integer;
> if (base->enum_) {
> - /* If enum, every atom in range has to be checked */
> - union ovsdb_atom ai = *atom;
> - ++ai.integer;
> - while (!error) {
> + for (int64_t i = start + 1; i <= end; i++) {
> + union ovsdb_atom ai = { .integer = i };
> error = ovsdb_atom_check_constraints(&ai, base);
> - if (ai.integer == (*range_end_atom)->integer) {
> + if (error) {
> break;
> }
> - ++ai.integer;
> }
> } else {
> error = ovsdb_atom_check_constraints(*range_end_atom, base);
> }
>
> if (!error) {
> - error = ovsdb_atom_range_check_size(atom->integer,
> -
> (*range_end_atom)->integer);
> + error = ovsdb_atom_range_check_size(start, end);
> }
> }
>
> @@ -1857,11 +1855,9 @@ ovsdb_datum_add_unsafe(struct ovsdb_datum *datum,
> datum->n += range_end_atom ?
> (range_end_atom->integer - key->integer + 1) : 1;
> datum->keys = xrealloc(datum->keys, datum->n * sizeof *datum->keys);
> - if (range_end_atom
> - && ovsdb_atom_range_check(key->integer,
> range_end_atom->integer)) {
> - union ovsdb_atom ai;
> - for (ai = *key; ai.integer <= range_end_atom->integer;
> ++ai.integer) {
> - ovsdb_atom_clone(&datum->keys[idx++], &ai, type->key.type);
> + if (range_end_atom && key->integer <= range_end_atom->integer) {
> + for (int64_t i = key->integer; i <= range_end_atom->integer; i++)
> {
> + datum->keys[idx++].integer = i;
> }
> } else {
> ovsdb_atom_clone(&datum->keys[idx], key, type->key.type);
> @@ -2146,7 +2142,8 @@ ovsdb_token_is_delim(unsigned char c)
> struct ovsdb_error *
> ovsdb_atom_range_check_size(int64_t range_start, int64_t range_end)
> {
> - if (range_end - range_start >= MAX_OVSDB_ATOM_RANGE_SIZE) {
> + if ((uint64_t) range_end - (uint64_t) range_start
> + >= MAX_OVSDB_ATOM_RANGE_SIZE) {
> return ovsdb_error("constraint violation",
> "Range \"%"PRId64"-%"PRId64"\" is too big. "
> "Maximum allowed size is %d.",
> diff --git a/lib/ovsdb-data.h b/lib/ovsdb-data.h
> index 46ff75b..1bf90d5 100644
> --- a/lib/ovsdb-data.h
> +++ b/lib/ovsdb-data.h
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2009, 2010, 2011, 2012, 2015, 2016 Nicira, Inc.
> +/* Copyright (c) 2009, 2010, 2011, 2012, 2015, 2016, 2017 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -279,14 +279,7 @@ struct ovsdb_symbol *ovsdb_symbol_table_insert(struct
> ovsdb_symbol_table *,
> char *ovsdb_token_parse(const char **, char **outp)
> OVS_WARN_UNUSED_RESULT;
> bool ovsdb_token_is_delim(unsigned char);
>
> -static inline bool
> -ovsdb_atom_range_check(long long begin, long long end)
> -{
> - return begin < end ? true : false;
> -}
> -
> -struct ovsdb_error *ovsdb_atom_range_check_size(
> - int64_t range_start,
> +struct ovsdb_error *ovsdb_atom_range_check_size(int64_t range_start,
> int64_t range_end);
>
> #endif /* ovsdb-data.h */
> diff --git a/lib/util.c b/lib/util.c
> index 4d10d70..1c06ce0 100644
> --- a/lib/util.c
> +++ b/lib/util.c
> @@ -679,21 +679,19 @@ str_to_uint(const char *s, int base, unsigned int *u)
> }
> }
>
> -bool str_to_llong_range(const char *s, int base, long long *begin,
> - long long *end)
> +bool
> +str_to_llong_range(const char *s, int base, long long *begin,
> + long long *end)
> {
> char *tail;
> - if (!str_to_llong_with_tail(s, &tail, base, begin) || *tail != '-') {
> - *begin = 0;
> - *end = 0;
> - return false;
> - }
> - if (!str_to_llong(++tail, base, end)) {
> - *begin = 0;
> - *end = 0;
> - return false;
> + if (str_to_llong_with_tail(s, &tail, base, begin)
> + && *tail == '-'
> + && str_to_llong(tail + 1, base, end)) {
> + return true;
> }
> - return true;
> + *begin = 0;
> + *end = 0;
> + return false;
> }
>
> /* Converts floating-point string 's' into a double. If successful,
> stores
> diff --git a/tests/ovsdb-data.at b/tests/ovsdb-data.at
> index 414935b..8c40bcf 100644
> --- a/tests/ovsdb-data.at
> +++ b/tests/ovsdb-data.at
> @@ -274,15 +274,15 @@ OVSDB_CHECK_NEGATIVE([real not acceptable integer
> string atom],
>
> OVSDB_CHECK_NEGATIVE([inverted range is not acceptable integer string
> atom positive and negative],
> [[parse-atom-strings -- '["integer"]' '10--10' ]],
> - ["10--10" is not a valid range. Range start has to be less than end.])
> + ["10--10" is not a valid range. Range end cannot be before start.])
>
> OVSDB_CHECK_NEGATIVE([inverted range is not acceptable integer string
> atom negative],
> [[parse-atom-strings -- '["integer"]' '-10--100' ]],
> - ["-10--100" is not a valid range. Range start has to be less than end.])
> + ["-10--100" is not a valid range. Range end cannot be before start.])
>
> OVSDB_CHECK_NEGATIVE([inverted range is not acceptable integer string
> atom positive],
> [[parse-atom-strings -- '["integer"]' '100-10' ]],
> - ["100-10" is not a valid range. Range start has to be less than end.])
> + ["100-10" is not a valid range. Range end cannot be before start.])
>
> OVSDB_CHECK_NEGATIVE([too big range is not acceptable integer string atom
> positive and negative],
> [[parse-atom-strings -- '["integer"]' '-2000-2096' ]],
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
