When a client of the IDL tries to commit a read-modify-write transaction
but the database has changed in the meantime, the IDL tells its client to
wait for the IDL to change and then try the transaction again by returning
TXN_TRY_AGAIN. The "wait for the IDL to change" part is important because
there's no point in retrying the transaction before the IDL has received
the database updates (the transaction would fail in the same way all over
again).
However, the logic was incomplete: the database update can be received
*before* the reply to the transaction RPC (I think that in the current
ovsdb-server implementation this will always happen, in fact). When this
happens, the right thing to do is to retry the transaction immediately;
if we wait, then we're waiting for an additional change to the database
that may never come, causing an indefinite hang.
This commit therefore breaks the "try again" IDL commit status code
into two, one that means "try again immediately" and another that means
"wait for a change then try again". When an update is processed after a
transaction is committed but before the reply is received, the "try again
now" tells the IDL client not to wait for another database change before
retrying its transaction.
Bug #5980.
Reported-by: Ram Jothikumar <rjothikumar@nicira.com>
Reproduced-by: Alex Yip <alex@nicira.com>
Until now, "emer" has effectively been "off" because no messages were ever
logged at "emer" level. Justin points out that it is useful to use "emer"
for messages that indicate a fatal error. This commit makes that change
and adds a new "off" level to really turn off all logging to a facility.
Once in a while someone reports a problem caused by running multiple
ovs-vswitchd processes at the same time. This fixes the problem by
requiring ovs-vswitchd to obtain a database lock before taking any actions.
The database prevents multiple ports or interfaces with a single name, but
duplicates can still occur if, for example, two bridges' "ports" columns
both point to a single Port record. The existing warning just says in this
case that the database contains a duplicate port name. This prompts users
to dump the Port table to look for the duplicate. Of course there isn't
one, so then they ask me to point out the problem.
This commit improves the log message to point out the actual problem.
The state machine didn't have a proper state for "not yet committed or
aborted", which meant that destroying an ovsdb_idl_txn without committing
or aborting it caused a segfault. This fixes the problem by adding a new
state TXN_UNCOMMITTED to the state machine.
This is related to commit 79554078d "ovsdb-idl: Fix bad logic in
ovsdb_idl_txn_commit() state transitions", which fixed a related bug.
Bug #2438.
In an effort to make CFM easier to understand and configure, this
patch removes the Maintenance_Point and Monitor tables from the
database. As a consequence, users will only be able to configure
one remote maintenance point. Furthermore, before this patch each
remote maintenance point maintained its own separate fault flag in
the database. This flag is no longer reported, users will need to
infer the fault status from the global CFM fault flag.
Previously, if --private-key or another option that requires SSL support
was used, but OVS was built without OpenSSL support, then OVS would fail
with an error message that the specified option was not supported. This
confused users because it made them think that the option had been removed:
http://openvswitch.org/pipermail/discuss/2011-April/005034.html
This commit improves the error message: OVS will now report that it was
built without SSL support. This should be make the problem clear to users.
Reported-by: Aaron Rosen <arosen@clemson.edu>
Feature #5325.
In each of the cases converted here, an shash was used simply to maintain
a set of strings, with the shash_nodes' 'data' values set to NULL. This
commit converts them to use sset instead.
Garbage collection introduced in
c5f341ab19 changed ovs-vsctl so that
it would allow the garbage collector to reclaim unused tables
instead of manually deleting them itself. Since garbage collection
runs at transaction completion, undeleted tables would hang around
and could conflict with future actions in a given transaction.
This commit backs out this change.
The following command is an example of something that would have
failed before this commit.
ovs-vsctl -- add-br b \
-- del-br b \
-- add-br b \
-- set Interface b other_config:test=test
ovs-vsctl will, in upcoming commits, want to more closely examine its
ovsdb_symbol_table structures. This could be done by providing a more
complete API, but it doesn't seem worth it to me. This commit instead goes
the other way, exposing the internals to clients. This commit also
eliminates the ovsdb_symbol_table_find_uncreated() function, which
ovs-vsctl can now implement itself.
The name 'created' better reflects the actual meaning of this member: in
both ovsdb and ovs-vsctl, it is true if a row has been created with the
symbol's UUID and false otherwise.
The check for uncreated symbols does not rely on anything that happens
during transaction commit, so there is no point in allowing the transaction
to be sent to the database server, only to fail there, before reporting
the problem locally.
Coverity pointed out some inconsistencies on tests for whether columnp and
keyp were nonnull. These tests were, at best, confusing, but in fact every
caller always passed nonnull for both parameters, so this commit drops all
of those tests.
Coverity #10715, 10710.
We had retained but deprecated the use of the older 'managers' column in the
'Open vSwitch' table for compatibility with applications that might still use
it, but that created more problems than it solved. This commit removes the
'managers' column from the schema, and removes all references to it from the
code, init scripts, documentation, and tests.
The default format is the same as before (which the testsuite verifies).
The most important use for the new formatting options is --bare, which
outputs a format that is easy to parse from a shell script. An upcoming
patch will start using that.
This allows listing records that match specified criteria, instead
of just records that have specific names.
This will be used in an upcoming patch, along with --columns, to
list all of the interfaces whose type is 'internal'.
This allows the user to list just selected columns from a table,
for example just the "name" column.
This will become more useful as additional formatting options
are added in upcoming commits.
An shash always has string keys and sometimes it hash string values as
well. Usually the general-purpose shash functions are fine for working
with string-to-string maps, but this commit introduces a few more
specialized functions that only work with string-to-string maps. It's not
clear yet to me whether this should actually be a new data structure, so
for now the new functions just work on shashes.
This commit also converts one user of shash_destroy() to use smap_destroy().
This is the only existing user of these functions that I spotted as a
trivial conversion candidate while grepping.
These new functions will see more use in the following commit.
Reviewed by Justin Pettit.
The controller commands use the "target" column of the Controller table,
but they don't supply it as a prerequisite, which makes those commands
hang. This commit fixes the problem.
This makes it possible to run tests that need access to installation
directories, such as the rundir, without having access to the actual
installation directories (/var/run is generally not world-writable), by
setting environment variables. This is not a good way to do things in
general--usually it would be better to choose the correct directories
at configure time--so for now this is undocumented.
This commit implements a subset of the 802.1ag specification for
Connectivity Fault Management (CFM) using Continuity Check Messages
(CCM). When CFM is configured on an interface CCMs are broadcast
at regular intervals to detect missing or unexpected connectivity.
The wrong prerequisite function was being called for set-br-external-id, so
it didn't set up to monitor the right columns, so an assertion failed.
This fixes the problem.
Also, change the test for external IDs so that it will find the regression.
(Running all of the ovs-vsctl commands together caused the column to be
monitored due to another command's intervention.)
Reported-by: Mikael Doverhag <mdoverhag@nicira.com>
Bug #4095.
Until now, ovs-vsctl has always fetched a complete replica of the Open
vSwitch database at startup time. This is a little expensive, but that is
not normally a problem because the database is small. However, it can
cause problems when Open vSwitch is partially upgraded: if ovs-vsctl is
upgraded before the database, and new columns or tables were added to the
database schema in the newer version of Open vSwitch, then ovs-vsctl will
report an error and fail to work, because it cannot monitor the new
columns. The same problem occurs if the database is upgraded before
ovs-vsctl and columns or tables were removed in the new version of Open
vSwitch.
This commit fixes the problem in the most common case, by making ovs-vsctl
only replicate the columns and tables in the database that it will actually
use during a given execution. This will still fail if the database has
changed in major ways, but only if the changes would actually cause
problems for what ovs-vsctl is trying to do anyhow.
Bug #3388.
Until now, by default the IDL replicated all tables and all columns in the
database, and a few functions made it possible to avoid replicating
selected columns. This commit adds a mode in which nothing is replicated
by default and the client code is responsible for specifying each column
and table that it is interested in. The following commit adds a user for
this mode.
I'm retaining the "managers" column in the Open_vSwitch table for now, but
I hope that applications transition to using "manager_options" eventually
so that we could drop it.
CC: Andrew Lambeth <wal@nicira.com>
CC: Jeremy Stribling <strib@nicira.com>
The bridge device is always an internal interface, so it seems best to
actually mark it as one. Jesse would like to eventually make the "type"
column mandatory for Interface records, so here's a step in the right
direction.
OVSDB is transactional but it does not implement any form of locking. This
means that read-modify-write operations must verify that the values that
they read are still in place before writing. This commit adds such
checking.
Bug #2387.
Bug #3856.
Bug #3906.
When ovs-vsctl is not actually going to modify the database, it is less
interesting in the log, so we might as well only log it at "debug" level.
Suggested-by: Neil McKee <neil.mckee@inmon.com>
