From 7446f1480bb27ccb63feab066d901cc940d52462 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 15 Feb 2010 11:31:32 -0800 Subject: [PATCH] ovsdb: Allow ovsdb_log_open()'s caller to choose whether to lock. The current callers of ovsdb_log_open() always want to lock the file if they are accessing it for read/write access. An upcoming commit will add a new caller that does not fit this model (it wants to lock the file across a wider region) and so the caller should be able to choose whether to do locking. This commit adds that ability. Also, get rid of the use of flags to choose the open mode, which has always seemed somewhat crude and which this change would make even cruder. --- ovsdb/file.c | 4 +++- ovsdb/log.c | 35 ++++++++++++++++++++------- ovsdb/log.h | 14 ++++++++--- ovsdb/ovsdb-tool.c | 7 +++--- tests/ovsdb-log.at | 60 +++++++++++++++++++++++----------------------- tests/test-ovsdb.c | 32 +++++++++---------------- 6 files changed, 86 insertions(+), 66 deletions(-) diff --git a/ovsdb/file.c b/ovsdb/file.c index 31086af84..3e07b8449 100644 --- a/ovsdb/file.c +++ b/ovsdb/file.c @@ -43,13 +43,15 @@ static void ovsdb_file_replica_create(struct ovsdb *, struct ovsdb_log *); struct ovsdb_error * ovsdb_file_open(const char *file_name, bool read_only, struct ovsdb **dbp) { + enum ovsdb_log_open_mode open_mode; struct ovsdb_schema *schema; struct ovsdb_error *error; struct ovsdb_log *log; struct json *json; struct ovsdb *db; - error = ovsdb_log_open(file_name, read_only ? O_RDONLY : O_RDWR, &log); + open_mode = read_only ? OVSDB_LOG_READ_ONLY : OVSDB_LOG_READ_WRITE; + error = ovsdb_log_open(file_name, open_mode, -1, &log); if (error) { return error; } diff --git a/ovsdb/log.c b/ovsdb/log.c index 837f21554..6d07aca01 100644 --- a/ovsdb/log.c +++ b/ovsdb/log.c @@ -50,21 +50,33 @@ struct ovsdb_log { enum ovsdb_log_mode mode; }; +/* Attempts to open 'name' with the specified 'open_mode'. On success, stores + * the new log into '*filep' and returns NULL; otherwise returns NULL and + * stores NULL into '*filep'. + * + * Whether the file will be locked using lockfile_lock() depends on 'locking': + * use true to lock it, false not to lock it, or -1 to lock it only if + * 'open_mode' is a mode that allows writing. + */ struct ovsdb_error * -ovsdb_log_open(const char *name, int flags, struct ovsdb_log **filep) +ovsdb_log_open(const char *name, enum ovsdb_log_open_mode open_mode, + int locking, struct ovsdb_log **filep) { struct lockfile *lockfile; struct ovsdb_error *error; struct ovsdb_log *file; struct stat s; FILE *stream; - int accmode; + int flags; int fd; *filep = NULL; - accmode = flags & O_ACCMODE; - if (accmode == O_RDWR || accmode == O_WRONLY) { + assert(locking == -1 || locking == false || locking == true); + if (locking < 0) { + locking = open_mode != OVSDB_LOG_READ_ONLY; + } + if (locking) { int retval = lockfile_lock(name, 0, &lockfile); if (retval) { error = ovsdb_io_error(retval, "%s: failed to lock lockfile", @@ -75,9 +87,18 @@ ovsdb_log_open(const char *name, int flags, struct ovsdb_log **filep) lockfile = NULL; } + if (open_mode == OVSDB_LOG_READ_ONLY) { + flags = O_RDONLY; + } else if (open_mode == OVSDB_LOG_READ_WRITE) { + flags = O_RDWR; + } else if (open_mode == OVSDB_LOG_CREATE) { + flags = O_RDWR | O_CREAT | O_EXCL; + } else { + NOT_REACHED(); + } fd = open(name, flags, 0666); if (fd < 0) { - const char *op = flags & O_CREAT && flags & O_EXCL ? "create" : "open"; + const char *op = open_mode == OVSDB_LOG_CREATE ? "create" : "open"; error = ovsdb_io_error(errno, "%s: %s failed", op, name); goto error_unlock; } @@ -98,9 +119,7 @@ ovsdb_log_open(const char *name, int flags, struct ovsdb_log **filep) free(dir); } - stream = fdopen(fd, (accmode == O_RDONLY ? "rb" - : accmode == O_WRONLY ? "wb" - : "w+b")); + stream = fdopen(fd, open_mode == OVSDB_LOG_READ_ONLY ? "rb" : "w+b"); if (!stream) { error = ovsdb_io_error(errno, "%s: fdopen failed", name); goto error_close; diff --git a/ovsdb/log.h b/ovsdb/log.h index 2daa635ba..045740b93 100644 --- a/ovsdb/log.h +++ b/ovsdb/log.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2009 Nicira Networks +/* Copyright (c) 2009, 2010 Nicira Networks * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,8 +22,16 @@ struct json; struct ovsdb_log; -struct ovsdb_error *ovsdb_log_open(const char *name, int flags, - struct ovsdb_log **) WARN_UNUSED_RESULT; +/* Access mode for opening an OVSDB log. */ +enum ovsdb_log_open_mode { + OVSDB_LOG_READ_ONLY, /* Open existing file, read-only. */ + OVSDB_LOG_READ_WRITE, /* Open existing file, read/write. */ + OVSDB_LOG_CREATE /* Create new file, read/write. */ +}; + +struct ovsdb_error *ovsdb_log_open(const char *name, enum ovsdb_log_open_mode, + int locking, struct ovsdb_log **) + WARN_UNUSED_RESULT; void ovsdb_log_close(struct ovsdb_log *); struct ovsdb_error *ovsdb_log_read(struct ovsdb_log *, struct json **) diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c index 34c767618..f419fb852 100644 --- a/ovsdb/ovsdb-tool.c +++ b/ovsdb/ovsdb-tool.c @@ -164,8 +164,8 @@ do_create(int argc OVS_UNUSED, char *argv[]) ovsdb_schema_destroy(schema); /* Create database file. */ - check_ovsdb_error(ovsdb_log_open(db_file_name, O_RDWR | O_CREAT | O_EXCL, - &log)); + check_ovsdb_error(ovsdb_log_open(db_file_name, OVSDB_LOG_CREATE, + -1, &log)); check_ovsdb_error(ovsdb_log_write(log, json)); check_ovsdb_error(ovsdb_log_commit(log)); ovsdb_log_close(log); @@ -288,7 +288,8 @@ do_show_log(int argc OVS_UNUSED, char *argv[]) struct ovsdb_log *log; unsigned int i; - check_ovsdb_error(ovsdb_log_open(db_file_name, O_RDONLY, &log)); + check_ovsdb_error(ovsdb_log_open(db_file_name, OVSDB_LOG_READ_ONLY, + -1, &log)); shash_init(&names); for (i = 0; ; i++) { struct json *json; diff --git a/tests/ovsdb-log.at b/tests/ovsdb-log.at index 8aeb33b56..507ef8ea9 100644 --- a/tests/ovsdb-log.at +++ b/tests/ovsdb-log.at @@ -4,11 +4,11 @@ AT_SETUP([create empty, reread]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([log]) AT_CHECK( - [test-ovsdb log-io file 'O_CREAT|O_RDWR'], [0], + [test-ovsdb log-io file create], [0], [file: open successful ], [ignore]) AT_CHECK( - [test-ovsdb log-io file 'O_RDONLY' read], [0], + [test-ovsdb log-io file read-only read], [0], [file: open successful file: read: end of log ], [ignore]) @@ -19,12 +19,12 @@ AT_SETUP([write one, reread]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]']], [0], + [[test-ovsdb log-io file create 'write:[0]']], [0], [[file: open successful file: write:[0] successful ]], [ignore]) AT_CHECK( - [test-ovsdb log-io file 'O_RDONLY' read read], [0], + [test-ovsdb log-io file read-only read read], [0], [[file: open successful file: read: [0] file: read: end of log @@ -32,21 +32,21 @@ file: read: end of log AT_CHECK([test -f .file.~lock~]) AT_CLEANUP -AT_SETUP([check that O_EXCL works]) +AT_SETUP([check that create fails if file exists]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[1]']], [0], + [[test-ovsdb log-io file create 'write:[1]']], [0], [[file: open successful file: write:[1] successful ]], [ignore]) AT_CHECK( - [test-ovsdb log-io file 'O_RDONLY' read], [0], + [test-ovsdb log-io file read-only read], [0], [[file: open successful file: read: [1] ]], [ignore]) AT_CHECK( - [test-ovsdb -vlockfile:console:emer log-io file 'O_CREAT|O_RDWR|O_EXCL' read], [1], + [test-ovsdb -vlockfile:console:emer log-io file create read], [1], [], [test-ovsdb: I/O error: create: file failed (File exists) ]) AT_CHECK([test -f .file.~lock~]) @@ -56,14 +56,14 @@ AT_SETUP([write one, reread]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]' 'write:[1]' 'write:[2]']], [0], + [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], [[file: open successful file: write:[0] successful file: write:[1] successful file: write:[2] successful ]], [ignore]) AT_CHECK( - [test-ovsdb log-io file 'O_RDONLY' read read read read], [0], + [test-ovsdb log-io file read-only read read read read], [0], [[file: open successful file: read: [0] file: read: [1] @@ -77,14 +77,14 @@ AT_SETUP([write one, reread, append]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]' 'write:[1]' 'write:[2]']], [0], + [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], [[file: open successful file: write:[0] successful file: write:[1] successful file: write:[2] successful ]], [ignore]) AT_CHECK( - [[test-ovsdb log-io file 'O_RDWR' read read read 'write:["append"]']], [0], + [[test-ovsdb log-io file read/write read read read 'write:["append"]']], [0], [[file: open successful file: read: [0] file: read: [1] @@ -92,7 +92,7 @@ file: read: [2] file: write:["append"] successful ]], [ignore]) AT_CHECK( - [test-ovsdb log-io file 'O_RDONLY' read read read read read], [0], + [test-ovsdb log-io file read-only read read read read read], [0], [[file: open successful file: read: [0] file: read: [1] @@ -107,20 +107,20 @@ AT_SETUP([write, reread one, overwrite]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]' 'write:[1]' 'write:[2]']], [0], + [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], [[file: open successful file: write:[0] successful file: write:[1] successful file: write:[2] successful ]], [ignore]) AT_CHECK( - [[test-ovsdb log-io file 'O_RDWR' read 'write:["more data"]']], [0], + [[test-ovsdb log-io file read/write read 'write:["more data"]']], [0], [[file: open successful file: read: [0] file: write:["more data"] successful ]], [ignore]) AT_CHECK( - [test-ovsdb log-io file 'O_RDONLY' read read read], [0], + [test-ovsdb log-io file read-only read read read], [0], [[file: open successful file: read: [0] file: read: ["more data"] @@ -133,7 +133,7 @@ AT_SETUP([write, add corrupted data, read]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]' 'write:[1]' 'write:[2]']], [0], + [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], [[file: open successful file: write:[0] successful file: write:[1] successful @@ -141,7 +141,7 @@ file: write:[2] successful ]], [ignore]) AT_CHECK([echo 'xxx' >> file]) AT_CHECK( - [test-ovsdb log-io file 'O_RDONLY' read read read read], [0], + [test-ovsdb log-io file read-only read read read read], [0], [[file: open successful file: read: [0] file: read: [1] @@ -155,7 +155,7 @@ AT_SETUP([write, add corrupted data, read, overwrite]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]' 'write:[1]' 'write:[2]']], [0], + [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], [[file: open successful file: write:[0] successful file: write:[1] successful @@ -163,7 +163,7 @@ file: write:[2] successful ]], [ignore]) AT_CHECK([echo 'xxx' >> file]) AT_CHECK( - [[test-ovsdb log-io file 'O_RDWR' read read read read 'write:[3]']], [0], + [[test-ovsdb log-io file read/write read read read read 'write:[3]']], [0], [[file: open successful file: read: [0] file: read: [1] @@ -172,7 +172,7 @@ file: read failed: syntax error: file: parse error at offset 174 in header line file: write:[3] successful ]], [ignore]) AT_CHECK( - [test-ovsdb log-io file 'O_RDONLY' read read read read read], [0], + [test-ovsdb log-io file read-only read read read read read], [0], [[file: open successful file: read: [0] file: read: [1] @@ -187,7 +187,7 @@ AT_SETUP([write, corrupt some data, read, overwrite]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]' 'write:[1]' 'write:[2]']], [0], + [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], [[file: open successful file: write:[0] successful file: write:[1] successful @@ -198,7 +198,7 @@ AT_CHECK([mv file.tmp file]) AT_CHECK([[grep -c '\[3]' file]], [0], [1 ]) AT_CHECK( - [[test-ovsdb log-io file 'O_RDWR' read read read 'write:["longer data"]']], [0], + [[test-ovsdb log-io file read/write read read read 'write:["longer data"]']], [0], [[file: open successful file: read: [0] file: read: [1] @@ -206,7 +206,7 @@ file: read failed: syntax error: file: 4 bytes starting at offset 170 have SHA-1 file: write:["longer data"] successful ]], [ignore]) AT_CHECK( - [test-ovsdb log-io file 'O_RDONLY' read read read read], [0], + [test-ovsdb log-io file read-only read read read read], [0], [[file: open successful file: read: [0] file: read: [1] @@ -220,7 +220,7 @@ AT_SETUP([write, truncate file, read, overwrite]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]' 'write:[1]' 'write:[2]']], [0], + [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], [[file: open successful file: write:[0] successful file: write:[1] successful @@ -231,7 +231,7 @@ AT_CHECK([mv file.tmp file]) AT_CHECK([[grep -c '^2$' file]], [0], [1 ]) AT_CHECK( - [[test-ovsdb log-io file 'O_RDWR' read read read 'write:["longer data"]']], [0], + [[test-ovsdb log-io file read/write read read read 'write:["longer data"]']], [0], [[file: open successful file: read: [0] file: read: [1] @@ -239,7 +239,7 @@ file: read failed: I/O error: file: error reading 4 bytes starting at offset 170 file: write:["longer data"] successful ]], [ignore]) AT_CHECK( - [test-ovsdb log-io file 'O_RDONLY' read read read read], [0], + [test-ovsdb log-io file read-only read read read read], [0], [[file: open successful file: read: [0] file: read: [1] @@ -253,7 +253,7 @@ AT_SETUP([write bad JSON, read, overwrite]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file 'O_CREAT|O_RDWR' 'write:[0]' 'write:[1]' 'write:[2]']], [0], + [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], [[file: open successful file: write:[0] successful file: write:[1] successful @@ -261,7 +261,7 @@ file: write:[2] successful ]], [ignore]) AT_CHECK([[printf '%s\n%s\n' 'OVSDB JSON 5 d910b02871075d3156ec8675dfc95b7d5d640aa6' 'null' >> file]]) AT_CHECK( - [[test-ovsdb log-io file 'O_RDWR' read read read read 'write:["replacement data"]']], [0], + [[test-ovsdb log-io file read/write read read read read 'write:["replacement data"]']], [0], [[file: open successful file: read: [0] file: read: [1] @@ -270,7 +270,7 @@ file: read failed: syntax error: file: 5 bytes starting at offset 228 are not va file: write:["replacement data"] successful ]], [ignore]) AT_CHECK( - [test-ovsdb log-io file 'O_RDONLY' read read read read read], [0], + [test-ovsdb log-io file read-only read read read read read], [0], [[file: open successful file: read: [0] file: read: [1] diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index 2e12d495b..48a5007f3 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -257,34 +257,24 @@ static void do_log_io(int argc, char *argv[]) { const char *name = argv[1]; - char *mode = argv[2]; + char *mode_string = argv[2]; struct ovsdb_error *error; + enum ovsdb_log_open_mode mode; struct ovsdb_log *log; - char *save_ptr = NULL; - const char *token; - int flags; int i; - for (flags = 0, token = strtok_r(mode, " |", &save_ptr); token != NULL; - token = strtok_r(NULL, " |", &save_ptr)) - { - if (!strcmp(token, "O_RDONLY")) { - flags |= O_RDONLY; - } else if (!strcmp(token, "O_RDWR")) { - flags |= O_RDWR; - } else if (!strcmp(token, "O_TRUNC")) { - flags |= O_TRUNC; - } else if (!strcmp(token, "O_CREAT")) { - flags |= O_CREAT; - } else if (!strcmp(token, "O_EXCL")) { - flags |= O_EXCL; - } else if (!strcmp(token, "O_TRUNC")) { - flags |= O_TRUNC; - } + if (!strcmp(mode_string, "read-only")) { + mode = OVSDB_LOG_READ_ONLY; + } else if (!strcmp(mode_string, "read/write")) { + mode = OVSDB_LOG_READ_WRITE; + } else if (!strcmp(mode_string, "create")) { + mode = OVSDB_LOG_CREATE; + } else { + ovs_fatal(0, "unknown log-io open mode \"%s\"", mode_string); } - check_ovsdb_error(ovsdb_log_open(name, flags, &log)); + check_ovsdb_error(ovsdb_log_open(name, mode, -1, &log)); printf("%s: open successful\n", name); for (i = 3; i < argc; i++) { -- 2.43.0