[ovs-dev] [PATCH 2/2] ovsdb-server: Improve message for "add-db" of database already open.

[Ben Pfaff]

Thanks for the reviews.  I applied both of these to master.

On Mon, Apr 07, 2014 at 09:53:32AM -0700, Alex Wang wrote:
> Acked-by: Alex Wang <alexw at nicira.com>
> 
> 
> On Tue, Mar 25, 2014 at 3:51 PM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > Otherwise you get a fairly obscure message about a lockfile already being
> > locked.
> >
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> >  ovsdb/ovsdb-server.c  |   40 +++++++++++++++++++++++++++++++++++++++-
> >  tests/ovsdb-server.at |    5 ++---
> >  2 files changed, 41 insertions(+), 4 deletions(-)
> >
> > diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> > index 6de77e4..a85a672 100644
> > --- a/ovsdb/ovsdb-server.c
> > +++ b/ovsdb/ovsdb-server.c
> > @@ -1,4 +1,4 @@
> > -/* Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> > +/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> >   *
> >   * Licensed under the Apache License, Version 2.0 (the "License");
> >   * you may not use this file except in compliance with the License.
> > @@ -19,6 +19,7 @@
> >  #include <getopt.h>
> >  #include <inttypes.h>
> >  #include <signal.h>
> > +#include <sys/stat.h>
> >  #include <unistd.h>
> >
> >  #include "column.h"
> > @@ -328,6 +329,36 @@ main(int argc, char *argv[])
> >      return 0;
> >  }
> >
> > +/* Returns true if 'filename' is known to be already open as a database,
> > + * false if not.
> > + *
> > + * "False negatives" are possible. */
> > +static bool
> > +is_already_open(struct server_config *config OVS_UNUSED,
> > +                const char *filename OVS_UNUSED)
> > +{
> > +#ifndef _WIN32
> > +    struct stat s;
> > +
> > +    if (!stat(filename, &s)) {
> > +        struct shash_node *node;
> > +
> > +        SHASH_FOR_EACH (node, config->all_dbs) {
> > +            struct db *db = node->data;
> > +            struct stat s2;
> > +
> > +            if (!stat(db->filename, &s2)
> > +                && s.st_dev == s2.st_dev
> > +                && s.st_ino == s2.st_ino) {
> > +                return true;
> > +            }
> > +        }
> > +    }
> > +#endif  /* !_WIN32 */
> > +
> > +    return false;
> > +}
> > +
> >  static char *
> >  open_db(struct server_config *config, const char *filename)
> >  {
> > @@ -335,6 +366,13 @@ open_db(struct server_config *config, const char
> > *filename)
> >      struct db *db;
> >      char *error;
> >
> > +    /* If we know that the file is already open, return a good error
> > message.
> > +     * Otherwise, if the file is open, we'll fail later on with a harder
> > to
> > +     * interpret file locking error. */
> > +    if (is_already_open(config, filename)) {
> > +        return xasprintf("%s: already open", filename);
> > +    }
> > +
> >      db = xzalloc(sizeof *db);
> >      db->filename = xstrdup(filename);
> >
> > diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
> > index 0cc4375..6a55e60 100644
> > --- a/tests/ovsdb-server.at
> > +++ b/tests/ovsdb-server.at
> > @@ -194,9 +194,8 @@ AT_CHECK([ovsdb-client list-tables unix:socket
> > constraints], [0], [ignore], [ign
> >  AT_CHECK([ovsdb-client list-tables unix:socket ordinals], [0], [ignore],
> > [ignore])
> >
> >  # Add an already added database.
> > -AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/add-db db2], 2, [],
> > [stderr])
> > -AT_CHECK([sed 's/(.*)/(...)/' stderr], [0],
> > -  [I/O error: db2: failed to lock lockfile (...)
> > +AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/add-db db2], 2, [],
> > +  [db2: already open
> >  ovs-appctl: ovsdb-server: server returned an error
> >  ])
> >
> > --
> > 1.7.10.4
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> >