From patchwork Fri Dec 8 00:12:28 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 845926 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ytCVp3mwKz9s7v for ; Fri, 8 Dec 2017 11:13:22 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id BB7B5C13; Fri, 8 Dec 2017 00:12:50 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 9DDFABC1 for ; Fri, 8 Dec 2017 00:12:48 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 753A6189 for ; Fri, 8 Dec 2017 00:12:47 +0000 (UTC) X-Originating-IP: 208.91.3.26 Received: from sigabrt.benpfaff.org (unknown [208.91.3.26]) (Authenticated sender: blp@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id C347EC5A44; Fri, 8 Dec 2017 01:12:44 +0100 (CET) From: Ben Pfaff To: dev@openvswitch.org Date: Thu, 7 Dec 2017 16:12:28 -0800 Message-Id: <20171208001240.25829-2-blp@ovn.org> X-Mailer: git-send-email 2.10.2 In-Reply-To: <20171208001240.25829-1-blp@ovn.org> References: <20171208001240.25829-1-blp@ovn.org> X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Ben Pfaff Subject: [ovs-dev] [PATCH 01/13] log: Allow client to specify magic. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org Until now, the logging code in ovsdb has only supported a single file format, for OVSDB standalone database files. Upcoming commits will add support for another, incompatible format, which uses a different magic string for identification. This commit allows the logging code to support both formats. Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --- ovsdb/file.c | 5 +++-- ovsdb/log.c | 54 ++++++++++++++++++++++++++++++++++++++++-------------- ovsdb/log.h | 5 ++++- ovsdb/ovsdb-tool.c | 10 +++++----- tests/ovsdb-log.at | 27 +++++++++++++++++++++++++++ tests/test-ovsdb.c | 17 ++++++++++++++--- 6 files changed, 93 insertions(+), 25 deletions(-) diff --git a/ovsdb/file.c b/ovsdb/file.c index 6a406da2a838..16461a75bfe5 100644 --- a/ovsdb/file.c +++ b/ovsdb/file.c @@ -129,7 +129,7 @@ ovsdb_file_open_log(const char *file_name, enum ovsdb_log_open_mode open_mode, ovs_assert(logp || schemap); - error = ovsdb_log_open(file_name, open_mode, -1, &log); + error = ovsdb_log_open(file_name, OVSDB_MAGIC, open_mode, -1, &log); if (error) { goto error; } @@ -440,7 +440,8 @@ ovsdb_file_save_copy__(const char *file_name, int locking, struct ovsdb_log *log; struct json *json; - error = ovsdb_log_open(file_name, OVSDB_LOG_CREATE, locking, &log); + error = ovsdb_log_open(file_name, OVSDB_MAGIC, + OVSDB_LOG_CREATE, locking, &log); if (error) { return error; } diff --git a/ovsdb/log.c b/ovsdb/log.c index 380f5e93d464..e6f66e668fe5 100644 --- a/ovsdb/log.c +++ b/ovsdb/log.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc. +/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 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. @@ -42,6 +42,7 @@ struct ovsdb_log { off_t prev_offset; off_t offset; char *name; + char *magic; struct lockfile *lockfile; FILE *stream; struct ovsdb_error *read_error; @@ -53,12 +54,17 @@ struct ovsdb_log { * the new log into '*filep' and returns NULL; otherwise returns NULL and * stores NULL into '*filep'. * + * 'magic' is a short text string put at the beginning of every record and used + * to distinguish one kind of log file from another. For a conventional OVSDB + * log file, use OVSDB_MAGIC. + * * 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, enum ovsdb_log_open_mode open_mode, +ovsdb_log_open(const char *name, const char *magic, + enum ovsdb_log_open_mode open_mode, int locking, struct ovsdb_log **filep) { struct lockfile *lockfile; @@ -118,10 +124,30 @@ ovsdb_log_open(const char *name, enum ovsdb_log_open_mode open_mode, goto error_unlock; } - if (!fstat(fd, &s) && s.st_size == 0) { - /* It's (probably) a new file so fsync() its parent directory to ensure - * that its directory entry is committed to disk. */ - fsync_parent_dir(name); + if (!fstat(fd, &s)) { + if (s.st_size == 0) { + /* It's (probably) a new file so fsync() its parent directory to + * ensure that its directory entry is committed to disk. */ + fsync_parent_dir(name); + } else if (s.st_size >= strlen(magic) && S_ISREG(s.st_mode)) { + /* Try to read the magic from the first log record. If it's not + * the magic we expect, this is the wrong kind of file, so reject + * it immediately. */ + size_t magic_len = strlen(magic); + char *buf = xzalloc(magic_len + 1); + bool err = (read(fd, buf, magic_len) == magic_len + && strcmp(buf, magic)); + free(buf); + if (err) { + error = ovsdb_error(NULL, "%s: bad magic (unexpected " + "kind of file)", name); + goto error_close; + } + if (lseek(fd, 0, SEEK_SET)) { + error = ovsdb_io_error(errno, "%s: seek failed", name); + goto error_close; + } + } } stream = fdopen(fd, open_mode == OVSDB_LOG_READ_ONLY ? "rb" : "w+b"); @@ -132,6 +158,7 @@ ovsdb_log_open(const char *name, enum ovsdb_log_open_mode open_mode, file = xmalloc(sizeof *file); file->name = xstrdup(name); + file->magic = xstrdup(magic); file->lockfile = lockfile; file->stream = stream; file->prev_offset = 0; @@ -162,21 +189,20 @@ ovsdb_log_close(struct ovsdb_log *file) } } -static const char magic[] = "OVSDB JSON "; - static bool -parse_header(char *header, unsigned long int *length, +parse_header(const char *magic, char *header, unsigned long int *length, uint8_t sha1[SHA1_DIGEST_SIZE]) { char *p; /* 'header' must consist of a magic string... */ - if (strncmp(header, magic, strlen(magic))) { + size_t magic_len = strlen(magic); + if (strncmp(header, magic, magic_len) || header[magic_len] != ' ') { return false; } /* ...followed by a length in bytes... */ - *length = strtoul(header + strlen(magic), &p, 10); + *length = strtoul(header + magic_len + 1, &p, 10); if (!*length || *length == ULONG_MAX || *p != ' ') { return false; } @@ -256,7 +282,7 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp) goto error; } - if (!parse_header(header, &data_length, expected_sha1)) { + if (!parse_header(file->magic, header, &data_length, expected_sha1)) { error = ovsdb_syntax_error(NULL, NULL, "%s: parse error at offset " "%lld in header line \"%.*s\"", file->name, (long long int) file->offset, @@ -356,8 +382,8 @@ ovsdb_log_write(struct ovsdb_log *file, struct json *json) /* Compose header. */ sha1_bytes(json_string, length, sha1); - snprintf(header, sizeof header, "%s%"PRIuSIZE" "SHA1_FMT"\n", - magic, length, SHA1_ARGS(sha1)); + snprintf(header, sizeof header, "%s %"PRIuSIZE" "SHA1_FMT"\n", + file->magic, length, SHA1_ARGS(sha1)); /* Write. */ if (fwrite(header, strlen(header), 1, file->stream) != 1 diff --git a/ovsdb/log.h b/ovsdb/log.h index 5fe636b4c387..fbcea1f2b991 100644 --- a/ovsdb/log.h +++ b/ovsdb/log.h @@ -29,7 +29,10 @@ enum ovsdb_log_open_mode { OVSDB_LOG_CREATE /* Create new file, read/write. */ }; -struct ovsdb_error *ovsdb_log_open(const char *name, enum ovsdb_log_open_mode, +#define OVSDB_MAGIC "OVSDB JSON" + +struct ovsdb_error *ovsdb_log_open(const char *name, const char *magic, + enum ovsdb_log_open_mode, int locking, struct ovsdb_log **) OVS_WARN_UNUSED_RESULT; void ovsdb_log_close(struct ovsdb_log *); diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c index 8908bae3628a..45b3f7348c3d 100644 --- a/ovsdb/ovsdb-tool.c +++ b/ovsdb/ovsdb-tool.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc. + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 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. @@ -217,8 +217,8 @@ do_create(struct ovs_cmdl_context *ctx) ovsdb_schema_destroy(schema); /* Create database file. */ - check_ovsdb_error(ovsdb_log_open(db_file_name, OVSDB_LOG_CREATE, - -1, &log)); + check_ovsdb_error(ovsdb_log_open(db_file_name, OVSDB_MAGIC, + OVSDB_LOG_CREATE, -1, &log)); check_ovsdb_error(ovsdb_log_write(log, json)); check_ovsdb_error(ovsdb_log_commit(log)); ovsdb_log_close(log); @@ -519,8 +519,8 @@ do_show_log(struct ovs_cmdl_context *ctx) struct ovsdb_schema *schema; unsigned int i; - check_ovsdb_error(ovsdb_log_open(db_file_name, OVSDB_LOG_READ_ONLY, - -1, &log)); + check_ovsdb_error(ovsdb_log_open(db_file_name, OVSDB_MAGIC, + OVSDB_LOG_READ_ONLY, -1, &log)); shash_init(&names); schema = NULL; for (i = 0; ; i++) { diff --git a/tests/ovsdb-log.at b/tests/ovsdb-log.at index 3e7cdf828466..c8efaaec1a50 100644 --- a/tests/ovsdb-log.at +++ b/tests/ovsdb-log.at @@ -73,6 +73,33 @@ file: read: end of log AT_CHECK([test -f .file.~lock~]) AT_CLEANUP +AT_SETUP([write one, reread - alternative magic]) +AT_KEYWORDS([ovsdb log]) +AT_CAPTURE_FILE([file]) +# Sometimes you just need more magic: +# http://www.catb.org/jargon/html/magic-story.html +AT_CHECK( + [[test-ovsdb --magic="MORE MAGIC" 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 --magic="MORE MAGIC" log-io file read-only read read read read], [0], + [[file: open successful +file: read: [0] +file: read: [1] +file: read: [2] +file: read: end of log +]], [ignore]) +AT_CHECK( + [test-ovsdb log-io file read-only], [1], [], + [test-ovsdb: ovsdb error: file: bad magic (unexpected kind of file) +]) +AT_CHECK([test -f .file.~lock~]) +AT_CLEANUP + AT_SETUP([write one, reread, append]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index 553a4f6a5bf1..4d6a894e3e83 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -55,6 +55,9 @@ struct test_ovsdb_pvt_context { bool track; }; +/* Magic to pass to ovsdb_log_open(). */ +static const char *magic = OVSDB_MAGIC; + OVS_NO_RETURN static void usage(void); static void parse_options(int argc, char *argv[], struct test_ovsdb_pvt_context *pvt); @@ -76,10 +79,14 @@ main(int argc, char *argv[]) static void parse_options(int argc, char *argv[], struct test_ovsdb_pvt_context *pvt) { + enum { + OPT_MAGIC = CHAR_MAX + 1, + }; static const struct option long_options[] = { {"timeout", required_argument, NULL, 't'}, {"verbose", optional_argument, NULL, 'v'}, {"change-track", optional_argument, NULL, 'c'}, + {"magic", required_argument, NULL, OPT_MAGIC}, {"help", no_argument, NULL, 'h'}, {NULL, 0, NULL, 0}, }; @@ -116,6 +123,10 @@ parse_options(int argc, char *argv[], struct test_ovsdb_pvt_context *pvt) pvt->track = true; break; + case OPT_MAGIC: + magic = optarg; + break; + case '?': exit(EXIT_FAILURE); @@ -131,8 +142,8 @@ usage(void) { printf("%s: Open vSwitch database test utility\n" "usage: %s [OPTIONS] COMMAND [ARG...]\n\n" - " log-io FILE FLAGS COMMAND...\n" - " open FILE with FLAGS, run COMMANDs\n" + " [--magic=MAGIC] log-io FILE FLAGS COMMAND...\n" + " open FILE with FLAGS (and MAGIC), run COMMANDs\n" " default-atoms\n" " test ovsdb_atom_default()\n" " default-data\n" @@ -316,7 +327,7 @@ do_log_io(struct ovs_cmdl_context *ctx) ovs_fatal(0, "unknown log-io open mode \"%s\"", mode_string); } - check_ovsdb_error(ovsdb_log_open(name, mode, -1, &log)); + check_ovsdb_error(ovsdb_log_open(name, magic, mode, -1, &log)); printf("%s: open successful\n", name); for (i = 3; i < ctx->argc; i++) { From patchwork Fri Dec 8 00:12:29 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 845927 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ytCWh1P4Gz9s7v for ; Fri, 8 Dec 2017 11:14:08 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id EB08FC14; Fri, 8 Dec 2017 00:12:52 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 65AE3BC1 for ; Fri, 8 Dec 2017 00:12:50 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 23731189 for ; Fri, 8 Dec 2017 00:12:49 +0000 (UTC) X-Originating-IP: 208.91.3.26 Received: from sigabrt.benpfaff.org (unknown [208.91.3.26]) (Authenticated sender: blp@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id DEA2DC5A46; Fri, 8 Dec 2017 01:12:46 +0100 (CET) From: Ben Pfaff To: dev@openvswitch.org Date: Thu, 7 Dec 2017 16:12:29 -0800 Message-Id: <20171208001240.25829-3-blp@ovn.org> X-Mailer: git-send-email 2.10.2 In-Reply-To: <20171208001240.25829-1-blp@ovn.org> References: <20171208001240.25829-1-blp@ovn.org> X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Ben Pfaff Subject: [ovs-dev] [PATCH 02/13] log: Require log entries to be JSON objects. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org The current and upcoming users of the OVSDB logging module only use JSON objects as records. This commit simplifies the users slightly by allowing them to always assume that the records are JSON objects. Unfortunately this resulted in a large number of updates to tests, which didn't always use JSON objects. Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --- ovsdb/log.c | 17 +++++ tests/ovsdb-log.at | 212 ++++++++++++++++++++++++++--------------------------- 2 files changed, 123 insertions(+), 106 deletions(-) diff --git a/ovsdb/log.c b/ovsdb/log.c index e6f66e668fe5..d9fedd9ded6c 100644 --- a/ovsdb/log.c +++ b/ovsdb/log.c @@ -254,6 +254,16 @@ parse_body(struct ovsdb_log *file, off_t offset, unsigned long int length, return NULL; } +/* Attempts to read a log record from 'file'. + * + * If successful, returns NULL and stores in '*jsonp' the JSON object that the + * record contains. The caller owns the data and must eventually free it (with + * json_destroy()). + * + * If a read error occurs, returns the error and stores NULL in '*jsonp'. + * + * If the read reaches end of file, returns NULL and stores NULL in + * '*jsonp'. */ struct ovsdb_error * ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp) { @@ -315,6 +325,13 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp) json->u.string); goto error; } + if (json->type != JSON_OBJECT) { + error = ovsdb_syntax_error(NULL, NULL, "%s: %lu bytes starting at " + "offset %lld are not a JSON object", + file->name, data_length, + (long long int) data_offset); + goto error; + } file->prev_offset = file->offset; file->offset = data_offset + data_length; diff --git a/tests/ovsdb-log.at b/tests/ovsdb-log.at index c8efaaec1a50..29c0c5913c15 100644 --- a/tests/ovsdb-log.at +++ b/tests/ovsdb-log.at @@ -19,14 +19,14 @@ AT_SETUP([write one, reread]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file create 'write:[0]']], [0], + [[test-ovsdb log-io file create 'write:{"x":0}']], [0], [[file: open successful -file: write:[0] successful +file: write:{"x":0} successful ]], [ignore]) AT_CHECK( [test-ovsdb log-io file read-only read read], [0], [[file: open successful -file: read: [0] +file: read: {"x":0} file: read: end of log ]], [ignore]) AT_CHECK([test -f .file.~lock~]) @@ -36,14 +36,14 @@ AT_SETUP([check that create fails if file exists]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file create 'write:[1]']], [0], + [[test-ovsdb log-io file create 'write:{"x":1}']], [0], [[file: open successful -file: write:[1] successful +file: write:{"x":1} successful ]], [ignore]) AT_CHECK( [test-ovsdb log-io file read-only read], [0], [[file: open successful -file: read: [1] +file: read: {"x":1} ]], [ignore]) AT_CHECK( [test-ovsdb log-io file create read], [1], @@ -56,18 +56,18 @@ AT_SETUP([write one, reread]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], + [[test-ovsdb log-io file create 'write:{"x":0}' 'write:{"x":1}' 'write:{"x":2}']], [0], [[file: open successful -file: write:[0] successful -file: write:[1] successful -file: write:[2] successful +file: write:{"x":0} successful +file: write:{"x":1} successful +file: write:{"x":2} successful ]], [ignore]) AT_CHECK( [test-ovsdb log-io file read-only read read read read], [0], [[file: open successful -file: read: [0] -file: read: [1] -file: read: [2] +file: read: {"x":0} +file: read: {"x":1} +file: read: {"x":2} file: read: end of log ]], [ignore]) AT_CHECK([test -f .file.~lock~]) @@ -79,18 +79,18 @@ AT_CAPTURE_FILE([file]) # Sometimes you just need more magic: # http://www.catb.org/jargon/html/magic-story.html AT_CHECK( - [[test-ovsdb --magic="MORE MAGIC" log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], + [[test-ovsdb --magic="MORE_MAGIC" log-io file create 'write:{"x":0}' 'write:{"x":1}' 'write:{"x":2}']], [0], [[file: open successful -file: write:[0] successful -file: write:[1] successful -file: write:[2] successful +file: write:{"x":0} successful +file: write:{"x":1} successful +file: write:{"x":2} successful ]], [ignore]) AT_CHECK( - [test-ovsdb --magic="MORE MAGIC" log-io file read-only read read read read], [0], + [test-ovsdb --magic="MORE_MAGIC" log-io file read-only read read read read], [0], [[file: open successful -file: read: [0] -file: read: [1] -file: read: [2] +file: read: {"x":0} +file: read: {"x":1} +file: read: {"x":2} file: read: end of log ]], [ignore]) AT_CHECK( @@ -104,27 +104,27 @@ AT_SETUP([write one, reread, append]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], + [[test-ovsdb log-io file create 'write:{"x":0}' 'write:{"x":1}' 'write:{"x":2}']], [0], [[file: open successful -file: write:[0] successful -file: write:[1] successful -file: write:[2] successful +file: write:{"x":0} successful +file: write:{"x":1} successful +file: write:{"x":2} successful ]], [ignore]) AT_CHECK( - [[test-ovsdb log-io file read/write read read read 'write:["append"]']], [0], + [[test-ovsdb log-io file read/write read read read 'write:{"append":0}']], [0], [[file: open successful -file: read: [0] -file: read: [1] -file: read: [2] -file: write:["append"] successful +file: read: {"x":0} +file: read: {"x":1} +file: read: {"x":2} +file: write:{"append":0} successful ]], [ignore]) AT_CHECK( [test-ovsdb log-io file read-only read read read read read], [0], [[file: open successful -file: read: [0] -file: read: [1] -file: read: [2] -file: read: ["append"] +file: read: {"x":0} +file: read: {"x":1} +file: read: {"x":2} +file: read: {"append":0} file: read: end of log ]], [ignore]) AT_CHECK([test -f .file.~lock~]) @@ -134,23 +134,23 @@ AT_SETUP([write, reread one, overwrite]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], + [[test-ovsdb log-io file create 'write:{"x":0}' 'write:{"x":1}' 'write:{"x":2}']], [0], [[file: open successful -file: write:[0] successful -file: write:[1] successful -file: write:[2] successful +file: write:{"x":0} successful +file: write:{"x":1} successful +file: write:{"x":2} successful ]], [ignore]) AT_CHECK( - [[test-ovsdb log-io file read/write read 'write:["more data"]']], [0], + [[test-ovsdb log-io file read/write read 'write:{"more data":0}']], [0], [[file: open successful -file: read: [0] -file: write:["more data"] successful +file: read: {"x":0} +file: write:{"more data":0} successful ]], [ignore]) AT_CHECK( [test-ovsdb log-io file read-only read read read], [0], [[file: open successful -file: read: [0] -file: read: ["more data"] +file: read: {"x":0} +file: read: {"more data":0} file: read: end of log ]], [ignore]) AT_CHECK([test -f .file.~lock~]) @@ -160,20 +160,20 @@ AT_SETUP([write, add corrupted data, read]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], + [[test-ovsdb log-io file create 'write:{"x":0}' 'write:{"x":1}' 'write:{"x":2}']], [0], [[file: open successful -file: write:[0] successful -file: write:[1] successful -file: write:[2] successful +file: write:{"x":0} successful +file: write:{"x":1} successful +file: write:{"x":2} successful ]], [ignore]) AT_CHECK([echo 'xxx' >> file]) AT_CHECK( [test-ovsdb log-io file read-only read read read read], [0], [[file: open successful -file: read: [0] -file: read: [1] -file: read: [2] -file: read failed: syntax error: file: parse error at offset 174 in header line "xxx" +file: read: {"x":0} +file: read: {"x":1} +file: read: {"x":2} +file: read failed: syntax error: file: parse error at offset 186 in header line "xxx" ]], [ignore]) AT_CHECK([test -f .file.~lock~]) AT_CLEANUP @@ -182,29 +182,29 @@ AT_SETUP([write, add corrupted data, read, overwrite]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], + [[test-ovsdb log-io file create 'write:{"x":0}' 'write:{"x":1}' 'write:{"x":2}']], [0], [[file: open successful -file: write:[0] successful -file: write:[1] successful -file: write:[2] successful +file: write:{"x":0} successful +file: write:{"x":1} successful +file: write:{"x":2} successful ]], [ignore]) AT_CHECK([echo 'xxx' >> file]) AT_CHECK( - [[test-ovsdb log-io file read/write read read read read 'write:[3]']], [0], + [[test-ovsdb log-io file read/write read read read read 'write:{"x":3}']], [0], [[file: open successful -file: read: [0] -file: read: [1] -file: read: [2] -file: read failed: syntax error: file: parse error at offset 174 in header line "xxx" -file: write:[3] successful +file: read: {"x":0} +file: read: {"x":1} +file: read: {"x":2} +file: read failed: syntax error: file: parse error at offset 186 in header line "xxx" +file: write:{"x":3} successful ]], [ignore]) AT_CHECK( [test-ovsdb log-io file read-only read read read read read], [0], [[file: open successful -file: read: [0] -file: read: [1] -file: read: [2] -file: read: [3] +file: read: {"x":0} +file: read: {"x":1} +file: read: {"x":2} +file: read: {"x":3} file: read: end of log ]], [ignore]) AT_CHECK([test -f .file.~lock~]) @@ -214,30 +214,30 @@ AT_SETUP([write, corrupt some data, read, overwrite]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], + [[test-ovsdb log-io file create 'write:{"x":0}' 'write:{"x":1}' 'write:{"x":2}']], [0], [[file: open successful -file: write:[0] successful -file: write:[1] successful -file: write:[2] successful +file: write:{"x":0} successful +file: write:{"x":1} successful +file: write:{"x":2} successful ]], [ignore]) -AT_CHECK([[sed 's/\[2]/[3]/' < file > file.tmp]]) +AT_CHECK([[sed 's/{"x":2}/{"x":3}/' < file > file.tmp]]) AT_CHECK([mv file.tmp file]) -AT_CHECK([[grep -c '\[3]' file]], [0], [1 +AT_CHECK([[grep -c '{"x":3}' file]], [0], [1 ]) AT_CHECK( - [[test-ovsdb log-io file read/write read read read 'write:["longer data"]']], [0], + [[test-ovsdb log-io file read/write read read read 'write:{"longer data":0}']], [0], [[file: open successful -file: read: [0] -file: read: [1] -file: read failed: syntax error: file: 4 bytes starting at offset 170 have SHA-1 hash 5c031e5c0d3a9338cc127ebe40bb2748b6a67e78 but should have hash 98f55556e7ffd432381b56a19bd485b3e6446442 -file: write:["longer data"] successful +file: read: {"x":0} +file: read: {"x":1} +file: read failed: syntax error: file: 8 bytes starting at offset 178 have SHA-1 hash 2683fd63b5b9fd49df4f2aa25bf7db5cbbebbe6f but should have hash 3d8ed30f471ad1b7b4b571cb0c7d5ed3e81350aa +file: write:{"longer data":0} successful ]], [ignore]) AT_CHECK( [test-ovsdb log-io file read-only read read read read], [0], [[file: open successful -file: read: [0] -file: read: [1] -file: read: ["longer data"] +file: read: {"x":0} +file: read: {"x":1} +file: read: {"longer data":0} file: read: end of log ]], [ignore]) AT_CHECK([test -f .file.~lock~]) @@ -247,30 +247,30 @@ AT_SETUP([write, truncate file, read, overwrite]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], + [[test-ovsdb log-io file create 'write:{"x":0}' 'write:{"x":1}' 'write:{"x":2}']], [0], [[file: open successful -file: write:[0] successful -file: write:[1] successful -file: write:[2] successful +file: write:{"x":0} successful +file: write:{"x":1} successful +file: write:{"x":2} successful ]], [ignore]) -AT_CHECK([[sed 's/\[2]/2/' < file > file.tmp]]) +AT_CHECK([[sed 's/{"x":2}/2/' < file > file.tmp]]) AT_CHECK([mv file.tmp file]) AT_CHECK([[grep -c '^2$' file]], [0], [1 ]) AT_CHECK( - [[test-ovsdb log-io file read/write read read read 'write:["longer data"]']], [0], + [[test-ovsdb log-io file read/write read read read 'write:{"longer data":0}']], [0], [[file: open successful -file: read: [0] -file: read: [1] -file: read failed: I/O error: file: error reading 4 bytes starting at offset 170 (End of file) -file: write:["longer data"] successful +file: read: {"x":0} +file: read: {"x":1} +file: read failed: I/O error: file: error reading 8 bytes starting at offset 178 (End of file) +file: write:{"longer data":0} successful ]], [ignore]) AT_CHECK( [test-ovsdb log-io file read-only read read read read], [0], [[file: open successful -file: read: [0] -file: read: [1] -file: read: ["longer data"] +file: read: {"x":0} +file: read: {"x":1} +file: read: {"longer data":0} file: read: end of log ]], [ignore]) AT_CHECK([test -f .file.~lock~]) @@ -280,29 +280,29 @@ AT_SETUP([write bad JSON, read, overwrite]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) AT_CHECK( - [[test-ovsdb log-io file create 'write:[0]' 'write:[1]' 'write:[2]']], [0], + [[test-ovsdb log-io file create 'write:{"x":0}' 'write:{"x":1}' 'write:{"x":2}']], [0], [[file: open successful -file: write:[0] successful -file: write:[1] successful -file: write:[2] successful +file: write:{"x":0} successful +file: write:{"x":1} successful +file: write:{"x":2} successful ]], [ignore]) AT_CHECK([[printf '%s\n%s\n' 'OVSDB JSON 5 d910b02871075d3156ec8675dfc95b7d5d640aa6' 'null' >> file]]) AT_CHECK( - [[test-ovsdb log-io file read/write read read read read 'write:["replacement data"]']], [0], + [[test-ovsdb log-io file read/write read read read read 'write:{"replacement data":0}']], [0], [[file: open successful -file: read: [0] -file: read: [1] -file: read: [2] -file: read failed: syntax error: file: 5 bytes starting at offset 228 are not valid JSON (line 0, column 4, byte 4: syntax error at beginning of input) -file: write:["replacement data"] successful +file: read: {"x":0} +file: read: {"x":1} +file: read: {"x":2} +file: read failed: syntax error: file: 5 bytes starting at offset 240 are not valid JSON (line 0, column 4, byte 4: syntax error at beginning of input) +file: write:{"replacement data":0} successful ]], [ignore]) AT_CHECK( [test-ovsdb log-io file read-only read read read read read], [0], [[file: open successful -file: read: [0] -file: read: [1] -file: read: [2] -file: read: ["replacement data"] +file: read: {"x":0} +file: read: {"x":1} +file: read: {"x":2} +file: read: {"replacement data":0} file: read: end of log ]], [ignore]) AT_CHECK([test -f .file.~lock~]) From patchwork Fri Dec 8 00:12:30 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 845928 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ytCXS50Qgz9s82 for ; Fri, 8 Dec 2017 11:14:48 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id DBFC7CAA; Fri, 8 Dec 2017 00:12:53 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id D0534C18 for ; Fri, 8 Dec 2017 00:12:50 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 7E2F6FE for ; Fri, 8 Dec 2017 00:12:50 +0000 (UTC) X-Originating-IP: 208.91.3.26 Received: from sigabrt.benpfaff.org (unknown [208.91.3.26]) (Authenticated sender: blp@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 8FD27C5A51; Fri, 8 Dec 2017 01:12:48 +0100 (CET) From: Ben Pfaff To: dev@openvswitch.org Date: Thu, 7 Dec 2017 16:12:30 -0800 Message-Id: <20171208001240.25829-4-blp@ovn.org> X-Mailer: git-send-email 2.10.2 In-Reply-To: <20171208001240.25829-1-blp@ovn.org> References: <20171208001240.25829-1-blp@ovn.org> X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Ben Pfaff Subject: [ovs-dev] [PATCH 03/13] log: Make json parameter to ovsdb_log_write() const. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --- ovsdb/log.c | 4 +++- ovsdb/log.h | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/ovsdb/log.c b/ovsdb/log.c index d9fedd9ded6c..1a372d7b73ee 100644 --- a/ovsdb/log.c +++ b/ovsdb/log.c @@ -360,8 +360,10 @@ ovsdb_log_unread(struct ovsdb_log *file) file->offset = file->prev_offset; } +/* Writes log record 'json' to 'file'. Returns NULL if successful or an error + * (which the caller must eventually destroy) on failure. */ struct ovsdb_error * -ovsdb_log_write(struct ovsdb_log *file, struct json *json) +ovsdb_log_write(struct ovsdb_log *file, const struct json *json) { uint8_t sha1[SHA1_DIGEST_SIZE]; struct ovsdb_error *error; diff --git a/ovsdb/log.h b/ovsdb/log.h index fbcea1f2b991..fd495e518dd0 100644 --- a/ovsdb/log.h +++ b/ovsdb/log.h @@ -41,7 +41,7 @@ struct ovsdb_error *ovsdb_log_read(struct ovsdb_log *, struct json **) OVS_WARN_UNUSED_RESULT; void ovsdb_log_unread(struct ovsdb_log *); -struct ovsdb_error *ovsdb_log_write(struct ovsdb_log *, struct json *) +struct ovsdb_error *ovsdb_log_write(struct ovsdb_log *, const struct json *) OVS_WARN_UNUSED_RESULT; struct ovsdb_error *ovsdb_log_commit(struct ovsdb_log *) OVS_WARN_UNUSED_RESULT; From patchwork Fri Dec 8 00:12:31 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 845929 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ytCY65RgQz9s82 for ; Fri, 8 Dec 2017 11:15:22 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id D300ECB3; Fri, 8 Dec 2017 00:12:54 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 7BB6BBE7 for ; Fri, 8 Dec 2017 00:12:52 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 20CCFFE for ; Fri, 8 Dec 2017 00:12:52 +0000 (UTC) X-Originating-IP: 208.91.3.26 Received: from sigabrt.benpfaff.org (unknown [208.91.3.26]) (Authenticated sender: blp@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id DDC92C5A49; Fri, 8 Dec 2017 01:12:49 +0100 (CET) From: Ben Pfaff To: dev@openvswitch.org Date: Thu, 7 Dec 2017 16:12:31 -0800 Message-Id: <20171208001240.25829-5-blp@ovn.org> X-Mailer: git-send-email 2.10.2 In-Reply-To: <20171208001240.25829-1-blp@ovn.org> References: <20171208001240.25829-1-blp@ovn.org> X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Ben Pfaff Subject: [ovs-dev] [PATCH 04/13] log: Log write errors. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org This saves all the callers from logging them separately. Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --- ovsdb/log.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ovsdb/log.c b/ovsdb/log.c index 1a372d7b73ee..f3c6e22ae212 100644 --- a/ovsdb/log.c +++ b/ovsdb/log.c @@ -25,6 +25,7 @@ #include #include "openvswitch/json.h" +#include "openvswitch/vlog.h" #include "lockfile.h" #include "ovsdb.h" #include "ovsdb-error.h" @@ -33,6 +34,8 @@ #include "transaction.h" #include "util.h" +VLOG_DEFINE_THIS_MODULE(ovsdb_log); + enum ovsdb_log_mode { OVSDB_LOG_READ, OVSDB_LOG_WRITE @@ -411,6 +414,10 @@ ovsdb_log_write(struct ovsdb_log *file, const struct json *json) { error = ovsdb_io_error(errno, "%s: write failed", file->name); + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); + VLOG_WARN_RL(&rl, "%s: write failed (%s)", + file->name, ovs_strerror(errno)); + /* Remove any partially written data, ignoring errors since there is * nothing further we can do. */ ignore(ftruncate(fileno(file->stream), file->offset)); From patchwork Fri Dec 8 00:12:32 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 845931 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ytCZK6wgMz9s7v for ; Fri, 8 Dec 2017 11:16:25 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id A4B13CBB; Fri, 8 Dec 2017 00:12:59 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 9EBFDCBA for ; Fri, 8 Dec 2017 00:12:57 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 5DE1F189 for ; Fri, 8 Dec 2017 00:12:53 +0000 (UTC) X-Originating-IP: 208.91.3.26 Received: from sigabrt.benpfaff.org (unknown [208.91.3.26]) (Authenticated sender: blp@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 75974C5A46; Fri, 8 Dec 2017 01:12:51 +0100 (CET) From: Ben Pfaff To: dev@openvswitch.org Date: Thu, 7 Dec 2017 16:12:32 -0800 Message-Id: <20171208001240.25829-6-blp@ovn.org> X-Mailer: git-send-email 2.10.2 In-Reply-To: <20171208001240.25829-1-blp@ovn.org> References: <20171208001240.25829-1-blp@ovn.org> X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Ben Pfaff Subject: [ovs-dev] [PATCH 05/13] log: Add new open mode OVSDB_LOG_CREATE_EXCL. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org Until now, OVSDB_LOG_CREATE implied EXCL, but this commit breaks them apart. Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --- ovsdb/file.c | 2 +- ovsdb/log.c | 23 ++++++++++++++++++----- ovsdb/log.h | 3 ++- ovsdb/ovsdb-tool.c | 2 +- tests/ovsdb-log.at | 7 ++++++- tests/test-ovsdb.c | 2 ++ 6 files changed, 30 insertions(+), 9 deletions(-) diff --git a/ovsdb/file.c b/ovsdb/file.c index 16461a75bfe5..fca07c3d722d 100644 --- a/ovsdb/file.c +++ b/ovsdb/file.c @@ -441,7 +441,7 @@ ovsdb_file_save_copy__(const char *file_name, int locking, struct json *json; error = ovsdb_log_open(file_name, OVSDB_MAGIC, - OVSDB_LOG_CREATE, locking, &log); + OVSDB_LOG_CREATE_EXCL, locking, &log); if (error) { return error; } diff --git a/ovsdb/log.c b/ovsdb/log.c index f3c6e22ae212..f95075034b3d 100644 --- a/ovsdb/log.c +++ b/ovsdb/log.c @@ -95,11 +95,16 @@ ovsdb_log_open(const char *name, const char *magic, lockfile = NULL; } - if (open_mode == OVSDB_LOG_READ_ONLY) { + switch (open_mode) { + case OVSDB_LOG_READ_ONLY: flags = O_RDONLY; - } else if (open_mode == OVSDB_LOG_READ_WRITE) { + break; + + case OVSDB_LOG_READ_WRITE: flags = O_RDWR; - } else if (open_mode == OVSDB_LOG_CREATE) { + break; + + case OVSDB_LOG_CREATE_EXCL: #ifndef _WIN32 if (stat(name, &s) == -1 && errno == ENOENT && lstat(name, &s) == 0 && S_ISLNK(s.st_mode)) { @@ -114,7 +119,13 @@ ovsdb_log_open(const char *name, const char *magic, #else flags = O_RDWR | O_CREAT | O_EXCL; #endif - } else { + break; + + case OVSDB_LOG_CREATE: + flags = O_RDWR | O_CREAT; + break; + + default: OVS_NOT_REACHED(); } #ifdef _WIN32 @@ -122,7 +133,9 @@ ovsdb_log_open(const char *name, const char *magic, #endif fd = open(name, flags, 0666); if (fd < 0) { - const char *op = open_mode == OVSDB_LOG_CREATE ? "create" : "open"; + const char *op = (open_mode == OVSDB_LOG_CREATE_EXCL ? "create" + : open_mode == OVSDB_LOG_CREATE ? "create or open" + : "open"); error = ovsdb_io_error(errno, "%s: %s failed", name, op); goto error_unlock; } diff --git a/ovsdb/log.h b/ovsdb/log.h index fd495e518dd0..08c6a7783094 100644 --- a/ovsdb/log.h +++ b/ovsdb/log.h @@ -26,7 +26,8 @@ struct 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. */ + OVSDB_LOG_CREATE_EXCL, /* Create new file, read/write. */ + OVSDB_LOG_CREATE /* Create or open file, read/write. */ }; #define OVSDB_MAGIC "OVSDB JSON" diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c index 45b3f7348c3d..da6b7142a4fa 100644 --- a/ovsdb/ovsdb-tool.c +++ b/ovsdb/ovsdb-tool.c @@ -218,7 +218,7 @@ do_create(struct ovs_cmdl_context *ctx) /* Create database file. */ check_ovsdb_error(ovsdb_log_open(db_file_name, OVSDB_MAGIC, - OVSDB_LOG_CREATE, -1, &log)); + OVSDB_LOG_CREATE_EXCL, -1, &log)); check_ovsdb_error(ovsdb_log_write(log, json)); check_ovsdb_error(ovsdb_log_commit(log)); ovsdb_log_close(log); diff --git a/tests/ovsdb-log.at b/tests/ovsdb-log.at index 29c0c5913c15..826e334efddd 100644 --- a/tests/ovsdb-log.at +++ b/tests/ovsdb-log.at @@ -46,9 +46,14 @@ AT_CHECK( file: read: {"x":1} ]], [ignore]) AT_CHECK( - [test-ovsdb log-io file create read], [1], + [test-ovsdb log-io file create-excl read], [1], [], [test-ovsdb: I/O error: file: create failed (File exists) ]) +AT_CHECK( + [test-ovsdb log-io file create read], [0], + [file: open successful +file: read: {"x":1} +]) AT_CHECK([test -f .file.~lock~]) AT_CLEANUP diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index 4d6a894e3e83..e06ecad4f368 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -323,6 +323,8 @@ do_log_io(struct ovs_cmdl_context *ctx) mode = OVSDB_LOG_READ_WRITE; } else if (!strcmp(mode_string, "create")) { mode = OVSDB_LOG_CREATE; + } else if (!strcmp(mode_string, "create-excl")) { + mode = OVSDB_LOG_CREATE_EXCL; } else { ovs_fatal(0, "unknown log-io open mode \"%s\"", mode_string); } From patchwork Fri Dec 8 00:12:33 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 845930 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ytCYk1VYCz9s7v for ; Fri, 8 Dec 2017 11:15:53 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id B9100CBC; Fri, 8 Dec 2017 00:12:56 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id DA4F5CB6 for ; Fri, 8 Dec 2017 00:12:54 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 876E4FE for ; Fri, 8 Dec 2017 00:12:54 +0000 (UTC) X-Originating-IP: 208.91.3.26 Received: from sigabrt.benpfaff.org (unknown [208.91.3.26]) (Authenticated sender: blp@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id A95ADC5A4F; Fri, 8 Dec 2017 01:12:52 +0100 (CET) From: Ben Pfaff To: dev@openvswitch.org Date: Thu, 7 Dec 2017 16:12:33 -0800 Message-Id: <20171208001240.25829-7-blp@ovn.org> X-Mailer: git-send-email 2.10.2 In-Reply-To: <20171208001240.25829-1-blp@ovn.org> References: <20171208001240.25829-1-blp@ovn.org> X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Ben Pfaff Subject: [ovs-dev] [PATCH 06/13] log: Make reading in writing mode less of an error. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org Clients are intended to use the OVSDB log code by reading zero or more records and then writing zero or more records, but not reading after any write has occurred. Until now, ovsdb_log_read() has signaled the latter behavior as an error. Upcoming changes to OVSDB are going to make it an expected behavior in some cases, so this commit changes it so that it just becomes an empty read. Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --- ovsdb/log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ovsdb/log.c b/ovsdb/log.c index f95075034b3d..6a1a4484ad34 100644 --- a/ovsdb/log.c +++ b/ovsdb/log.c @@ -296,7 +296,7 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp) if (file->read_error) { return ovsdb_error_clone(file->read_error); } else if (file->mode == OVSDB_LOG_WRITE) { - return OVSDB_BUG("reading file in write mode"); + return NULL; } if (!fgets(header, sizeof header, file->stream)) { From patchwork Fri Dec 8 00:12:34 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 845933 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ytCbb4pGCz9s7v for ; Fri, 8 Dec 2017 11:17:31 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 8CA29CE5; Fri, 8 Dec 2017 00:13:02 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id CEA9ACBB for ; Fri, 8 Dec 2017 00:12:58 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 140F73FB for ; Fri, 8 Dec 2017 00:12:56 +0000 (UTC) X-Originating-IP: 208.91.3.26 Received: from sigabrt.benpfaff.org (unknown [208.91.3.26]) (Authenticated sender: blp@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 04B30C5A44; Fri, 8 Dec 2017 01:12:53 +0100 (CET) From: Ben Pfaff To: dev@openvswitch.org Date: Thu, 7 Dec 2017 16:12:34 -0800 Message-Id: <20171208001240.25829-8-blp@ovn.org> X-Mailer: git-send-email 2.10.2 In-Reply-To: <20171208001240.25829-1-blp@ovn.org> References: <20171208001240.25829-1-blp@ovn.org> X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Ben Pfaff Subject: [ovs-dev] [PATCH 07/13] log: New functions for replacing a log's contents. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org These functions will acquire users in future commits. This new functionality made the internal state machine of the log hard enough to understand that I thought that it was best to make it explicit with a 'state' variable, so this commit introduces one. This commit duplicates code and logic from ovsdb_rename() and ovsdb_compact() in ovsdb/file.c. A future commit removes this duplication. Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --- ovsdb/log.c | 294 ++++++++++++++++++++++++++++++++++++++++++++++++----- ovsdb/log.h | 14 +++ tests/ovsdb-log.at | 74 ++++++++++++++ tests/test-ovsdb.c | 58 +++++++++-- 4 files changed, 404 insertions(+), 36 deletions(-) diff --git a/ovsdb/log.c b/ovsdb/log.c index 6a1a4484ad34..22cd9fe89599 100644 --- a/ovsdb/log.c +++ b/ovsdb/log.c @@ -36,23 +36,57 @@ VLOG_DEFINE_THIS_MODULE(ovsdb_log); -enum ovsdb_log_mode { - OVSDB_LOG_READ, - OVSDB_LOG_WRITE +/* State in a log's state machine. + * + * OVSDB_LOG_READ is the initial state for a newly opened log. Log records may + * be read in this state only. Reaching end of file does not cause a state + * transition. A read error transitions to OVSDB_LOG_READ_ERROR. + * + * OVSDB_LOG_READ_ERROR prevents further reads from succeeding; they will + * report the same error as before. A log write transitions away to + * OVSDB_LOG_WRITE or OVSDB_LOG_WRITE_ERROR. + * + * OVSDB_LOG_WRITE is the state following a call to ovsdb_log_write(), when all + * goes well. Any state other than OVSDB_LOG_BROKEN may transition to this + * state. A write error transitions to OVSDB_LOG_WRITE_ERROR. + * + * OVSDB_LOG_WRITE_ERROR is the state following a write error. Further writes + * retry and might transition back to OVSDB_LOG_WRITE. + * + * OVSDB_LOG_BROKEN is the state following a call to ovsdb_log_replace() or + * ovsdb_log_replace_commit(), if it fails in a spectacular enough way that no + * further reads or writes can succeed. This is a terminal state. + */ +enum ovsdb_log_state { + OVSDB_LOG_READ, /* Ready to read. */ + OVSDB_LOG_READ_ERROR, /* Read failed, see 'error' for details. */ + OVSDB_LOG_WRITE, /* Ready to write. */ + OVSDB_LOG_WRITE_ERROR, /* Write failed, see 'error' for details. */ + OVSDB_LOG_BROKEN, /* Disk on fire, see 'error' for details. */ }; struct ovsdb_log { + enum ovsdb_log_state state; + struct ovsdb_error *error; + off_t prev_offset; off_t offset; char *name; char *magic; struct lockfile *lockfile; FILE *stream; - struct ovsdb_error *read_error; - bool write_error; - enum ovsdb_log_mode mode; }; +/* Whether the OS supports renaming open files. + * + * (Making this a variable makes it easier to test both strategies on Unix-like + * systems.) */ +#ifdef _WIN32 +static bool rename_open_files = false; +#else +static bool rename_open_files = true; +#endif + /* 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'. @@ -173,15 +207,14 @@ ovsdb_log_open(const char *name, const char *magic, } file = xmalloc(sizeof *file); + file->state = OVSDB_LOG_READ; + file->error = NULL; file->name = xstrdup(name); file->magic = xstrdup(magic); file->lockfile = lockfile; file->stream = stream; file->prev_offset = 0; file->offset = 0; - file->read_error = NULL; - file->write_error = false; - file->mode = OVSDB_LOG_READ; *filep = file; return NULL; @@ -197,10 +230,12 @@ void ovsdb_log_close(struct ovsdb_log *file) { if (file) { + ovsdb_error_destroy(file->error); free(file->name); - fclose(file->stream); + if (file->stream) { + fclose(file->stream); + } lockfile_unlock(file->lockfile); - ovsdb_error_destroy(file->read_error); free(file); } } @@ -283,6 +318,20 @@ parse_body(struct ovsdb_log *file, off_t offset, unsigned long int length, struct ovsdb_error * ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp) { + *jsonp = NULL; + switch (file->state) { + case OVSDB_LOG_READ: + break; + + case OVSDB_LOG_READ_ERROR: + case OVSDB_LOG_WRITE_ERROR: + case OVSDB_LOG_BROKEN: + return ovsdb_error_clone(file->error); + + case OVSDB_LOG_WRITE: + return NULL; + } + uint8_t expected_sha1[SHA1_DIGEST_SIZE]; uint8_t actual_sha1[SHA1_DIGEST_SIZE]; struct ovsdb_error *error; @@ -291,20 +340,13 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp) struct json *json; char header[128]; - *jsonp = json = NULL; - - if (file->read_error) { - return ovsdb_error_clone(file->read_error); - } else if (file->mode == OVSDB_LOG_WRITE) { - return NULL; - } + json = NULL; if (!fgets(header, sizeof header, file->stream)) { if (feof(file->stream)) { - error = NULL; - } else { - error = ovsdb_io_error(errno, "%s: read failed", file->name); + return NULL; } + error = ovsdb_io_error(errno, "%s: read failed", file->name); goto error; } @@ -355,7 +397,8 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp) return NULL; error: - file->read_error = ovsdb_error_clone(error); + file->state = OVSDB_LOG_READ_ERROR; + file->error = ovsdb_error_clone(error); json_destroy(json); return error; } @@ -372,7 +415,7 @@ error: void ovsdb_log_unread(struct ovsdb_log *file) { - ovs_assert(file->mode == OVSDB_LOG_READ); + ovs_assert(file->state == OVSDB_LOG_READ); file->offset = file->prev_offset; } @@ -389,9 +432,16 @@ ovsdb_log_write(struct ovsdb_log *file, const struct json *json) json_string = NULL; - if (file->mode == OVSDB_LOG_READ || file->write_error) { - file->mode = OVSDB_LOG_WRITE; - file->write_error = false; + switch (file->state) { + case OVSDB_LOG_WRITE: + break; + + case OVSDB_LOG_READ: + case OVSDB_LOG_READ_ERROR: + case OVSDB_LOG_WRITE_ERROR: + file->state = OVSDB_LOG_WRITE; + ovsdb_error_destroy(file->error); + file->error = NULL; if (fseeko(file->stream, file->offset, SEEK_SET)) { error = ovsdb_io_error(errno, "%s: cannot seek to offset %lld", file->name, (long long int) file->offset); @@ -402,6 +452,10 @@ ovsdb_log_write(struct ovsdb_log *file, const struct json *json) file->name, (long long int) file->offset); goto error; } + break; + + case OVSDB_LOG_BROKEN: + return ovsdb_error_clone(file->error); } if (json->type != JSON_OBJECT && json->type != JSON_ARRAY) { @@ -443,7 +497,8 @@ ovsdb_log_write(struct ovsdb_log *file, const struct json *json) return NULL; error: - file->write_error = true; + file->state = OVSDB_LOG_WRITE_ERROR; + file->error = ovsdb_error_clone(error); free(json_string); return error; } @@ -451,7 +506,7 @@ error: struct ovsdb_error * ovsdb_log_commit(struct ovsdb_log *file) { - if (fsync(fileno(file->stream))) { + if (file->stream && fsync(fileno(file->stream))) { return ovsdb_io_error(errno, "%s: fsync failed", file->name); } return NULL; @@ -465,3 +520,186 @@ ovsdb_log_get_offset(const struct ovsdb_log *log) { return log->offset; } + +/* Attempts to atomically replace the contents of 'log', on disk, by the 'n' + * entries in 'entries'. If successful, returns NULL, otherwise returns an + * error (which the caller must eventually free). + * + * If successful, 'log' will be in write mode at the end of the log. */ +struct ovsdb_error * OVS_WARN_UNUSED_RESULT +ovsdb_log_replace(struct ovsdb_log *log, struct json **entries, size_t n) +{ + struct ovsdb_error *error; + struct ovsdb_log *new; + + error = ovsdb_log_replace_start(log, &new); + if (error) { + return error; + } + + for (size_t i = 0; i < n; i++) { + error = ovsdb_log_write(new, entries[i]); + if (error) { + ovsdb_log_replace_abort(new); + return error; + } + } + + return ovsdb_log_replace_commit(log, new); +} + +struct ovsdb_error * OVS_WARN_UNUSED_RESULT +ovsdb_log_replace_start(struct ovsdb_log *old, + struct ovsdb_log **newp) +{ + /* If old->name is a symlink, then we want the new file to be in the same + * directory as the symlink's referent. */ + char *deref_name = follow_symlinks(old->name); + char *tmp_name = xasprintf("%s.tmp", deref_name); + free(deref_name); + + struct ovsdb_error *error; + + ovs_assert(old->lockfile); + + /* Remove temporary file. (It might not exist.) */ + if (unlink(tmp_name) < 0 && errno != ENOENT) { + error = ovsdb_io_error(errno, "failed to remove %s", tmp_name); + free(tmp_name); + *newp = NULL; + return error; + } + + /* Create temporary file. */ + error = ovsdb_log_open(tmp_name, old->magic, OVSDB_LOG_CREATE_EXCL, + false, newp); + free(tmp_name); + return error; +} + +/* Rename 'old' to 'new', replacing 'new' if it exists. Returns NULL if + * successful, otherwise an ovsdb_error that the caller must destroy. */ +static struct ovsdb_error * OVS_WARN_UNUSED_RESULT +ovsdb_rename(const char *old, const char *new) +{ +#ifdef _WIN32 + /* Avoid rename() because it fails if the destination exists. */ + int error = (MoveFileEx(old, new, MOVEFILE_REPLACE_EXISTING + | MOVEFILE_WRITE_THROUGH | MOVEFILE_COPY_ALLOWED) + ? 0 : EACCES); +#else + int error = rename(old, new) ? errno : 0; +#endif + + return (error + ? ovsdb_io_error(error, "failed to rename \"%s\" to \"%s\"", + old, new) + : NULL); +} + +struct ovsdb_error * OVS_WARN_UNUSED_RESULT +ovsdb_log_replace_commit(struct ovsdb_log *old, struct ovsdb_log *new) +{ + struct ovsdb_error *error = ovsdb_log_commit(new); + if (error) { + ovsdb_log_replace_abort(new); + return error; + } + + /* Replace original file by the temporary file. + * + * We support two strategies: + * + * - The preferred strategy is to rename the temporary file over the + * original one in-place, then close the original one. This works on + * Unix-like systems. It does not work on Windows, which does not + * allow open files to be renamed. The approach has the advantage + * that, at any point, we can drop back to something that already + * works. + * + * - Alternatively, we can close both files, rename, then open the new + * file (which now has the original name). This works on all + * systems, but if reopening the file fails then 'old' is broken. + * + * We make the strategy a variable instead of an #ifdef to make it easier + * to test both strategies on Unix-like systems, and to make the code + * easier to read. */ + if (!rename_open_files) { + fclose(old->stream); + old->stream = NULL; + + fclose(new->stream); + new->stream = NULL; + } + + /* Rename 'old' to 'new'. We dereference the old name because, if it is a + * symlink, we want to replace the referent of the symlink instead of the + * symlink itself. */ + char *deref_name = follow_symlinks(old->name); + error = ovsdb_rename(new->name, deref_name); + free(deref_name); + + if (error) { + ovsdb_log_replace_abort(new); + return error; + } + if (rename_open_files) { + fsync_parent_dir(old->name); + fclose(old->stream); + old->stream = new->stream; + new->stream = NULL; + } else { + old->stream = fopen(old->name, "r+b"); + if (!old->stream) { + old->error = ovsdb_io_error(errno, "%s: could not reopen log", + old->name); + old->state = OVSDB_LOG_BROKEN; + return ovsdb_error_clone(old->error); + } + + if (fseek(old->stream, new->offset, SEEK_SET)) { + old->error = ovsdb_io_error(errno, "%s: seek failed", old->name); + old->state = OVSDB_LOG_BROKEN; + return ovsdb_error_clone(old->error); + } + } + + /* Replace 'old' by 'new' in memory. + * + * 'old' transitions to OVSDB_LOG_WRITE (it was probably in that mode + * anyway). */ + old->state = OVSDB_LOG_WRITE; + ovsdb_error_destroy(old->error); + old->error = NULL; + /* prev_offset only matters for OVSDB_LOG_READ. */ + old->offset = new->offset; + /* Keep old->name. */ + free(old->magic); + old->magic = new->magic; + new->magic = NULL; + /* Keep old->lockfile. */ + /* stream was already replaced above. */ + /* Free 'new'. */ + ovsdb_log_close(new); + + return NULL; +} + +void +ovsdb_log_replace_abort(struct ovsdb_log *new) +{ + if (new) { + /* Unlink the new file, but only after we close it (because Windows + * does not allow removing an open file). */ + char *name = xstrdup(new->name); + ovsdb_log_close(new); + unlink(name); + free(name); + } +} + +void +ovsdb_log_disable_renaming_open_files(void) +{ + rename_open_files = false; +} diff --git a/ovsdb/log.h b/ovsdb/log.h index 08c6a7783094..41734e73b351 100644 --- a/ovsdb/log.h +++ b/ovsdb/log.h @@ -49,4 +49,18 @@ struct ovsdb_error *ovsdb_log_commit(struct ovsdb_log *) off_t ovsdb_log_get_offset(const struct ovsdb_log *); +struct ovsdb_error *ovsdb_log_replace(struct ovsdb_log *, + struct json **entries, size_t n) + OVS_WARN_UNUSED_RESULT; +struct ovsdb_error *ovsdb_log_replace_start(struct ovsdb_log *old, + struct ovsdb_log **newp) + OVS_WARN_UNUSED_RESULT; +struct ovsdb_error *ovsdb_log_replace_commit(struct ovsdb_log *old, + struct ovsdb_log *new) + OVS_WARN_UNUSED_RESULT; +void ovsdb_log_replace_abort(struct ovsdb_log *new); + +/* For testing. */ +void ovsdb_log_disable_renaming_open_files(void); + #endif /* ovsdb/log.h */ diff --git a/tests/ovsdb-log.at b/tests/ovsdb-log.at index 826e334efddd..125ddd187551 100644 --- a/tests/ovsdb-log.at +++ b/tests/ovsdb-log.at @@ -78,6 +78,80 @@ file: read: end of log AT_CHECK([test -f .file.~lock~]) AT_CLEANUP +AT_SETUP([write one, replace, commit]) +AT_KEYWORDS([ovsdb log]) +AT_CAPTURE_FILE([file]) +for option in '' --no-rename-open-files; do + rm -f file + AT_CHECK( + [[test-ovsdb $option log-io file create \ + 'write:{"x":0}' \ + 'replace_start' \ + 'new-write:{"x":1}' \ + 'new-write:{"x":2}' \ + 'old-write:{"x":4}' \ + 'replace_commit' \ + 'read' \ + 'write:{"x":3}']], [0], + [[file: open successful +file: write:{"x":0} successful +file: replace_start successful +(temp): write:{"x":1} successful +(temp): write:{"x":2} successful +file: write:{"x":4} successful +file: replace_commit successful +file: read: end of log +file: write:{"x":3} successful +]]) + AT_CHECK( + [test-ovsdb log-io file read-only read read read read], [0], + [[file: open successful +file: read: {"x":1} +file: read: {"x":2} +file: read: {"x":3} +file: read: end of log +]], [ignore]) +done +AT_CHECK([test -f .file.~lock~]) +AT_CLEANUP + +AT_SETUP([write one, replace, abort]) +AT_KEYWORDS([ovsdb log]) +AT_CAPTURE_FILE([file]) +for option in '' --no-rename-open-files; do + rm -f file + AT_CHECK( + [[test-ovsdb $option log-io file create \ + 'write:{"x":0}' \ + 'replace_start' \ + 'new-write:{"x":1}' \ + 'new-write:{"x":2}' \ + 'old-write:{"x":4}' \ + 'replace_abort' \ + 'read' \ + 'write:{"x":3}']], [0], + [[file: open successful +file: write:{"x":0} successful +file: replace_start successful +(temp): write:{"x":1} successful +(temp): write:{"x":2} successful +file: write:{"x":4} successful +file: replace_abort successful +file: read: end of log +file: write:{"x":3} successful +]]) + AT_CHECK( + [test-ovsdb log-io file read-only read read read read], [0], + [[file: open successful +file: read: {"x":0} +file: read: {"x":4} +file: read: {"x":3} +file: read: end of log +]], [ignore]) +done +AT_CHECK([test -f .file.~lock~]) +AT_CLEANUP + AT_SETUP([write one, reread - alternative magic]) AT_KEYWORDS([ovsdb log]) AT_CAPTURE_FILE([file]) diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index e06ecad4f368..6d2799c1a131 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -81,12 +81,14 @@ parse_options(int argc, char *argv[], struct test_ovsdb_pvt_context *pvt) { enum { OPT_MAGIC = CHAR_MAX + 1, + OPT_NO_RENAME_OPEN_FILES }; static const struct option long_options[] = { {"timeout", required_argument, NULL, 't'}, {"verbose", optional_argument, NULL, 'v'}, {"change-track", optional_argument, NULL, 'c'}, {"magic", required_argument, NULL, OPT_MAGIC}, + {"no-rename-open-files", no_argument, NULL, OPT_NO_RENAME_OPEN_FILES}, {"help", no_argument, NULL, 'h'}, {NULL, 0, NULL, 0}, }; @@ -127,6 +129,10 @@ parse_options(int argc, char *argv[], struct test_ovsdb_pvt_context *pvt) magic = optarg; break; + case OPT_NO_RENAME_OPEN_FILES: + ovsdb_log_disable_renaming_open_files(); + break; + case '?': exit(EXIT_FAILURE); @@ -142,7 +148,8 @@ usage(void) { printf("%s: Open vSwitch database test utility\n" "usage: %s [OPTIONS] COMMAND [ARG...]\n\n" - " [--magic=MAGIC] log-io FILE FLAGS COMMAND...\n" + " [--magic=MAGIC] [--no-rename-open-files] " + " log-io FILE FLAGS COMMAND...\n" " open FILE with FLAGS (and MAGIC), run COMMANDs\n" " default-atoms\n" " test ovsdb_atom_default()\n" @@ -314,7 +321,6 @@ do_log_io(struct ovs_cmdl_context *ctx) struct ovsdb_error *error; enum ovsdb_log_open_mode mode; - struct ovsdb_log *log; int i; if (!strcmp(mode_string, "read-only")) { @@ -329,17 +335,41 @@ do_log_io(struct ovs_cmdl_context *ctx) ovs_fatal(0, "unknown log-io open mode \"%s\"", mode_string); } + struct ovsdb_log *log; check_ovsdb_error(ovsdb_log_open(name, magic, mode, -1, &log)); printf("%s: open successful\n", name); + struct ovsdb_log *replacement = NULL; + for (i = 3; i < ctx->argc; i++) { const char *command = ctx->argv[i]; + + struct ovsdb_log *target; + const char *target_name; + if (!strncmp(command, "old-", 4)) { + command += 4; + target = log; + target_name = name; + } else if (!strncmp(command, "new-", 4)) { + if (!replacement) { + ovs_fatal(0, "%s: can't execute command without " + "replacement log", command); + } + + command += 4; + target = replacement; + target_name = "(temp)"; + } else { + target = log; + target_name = name; + } + if (!strcmp(command, "read")) { struct json *json; - error = ovsdb_log_read(log, &json); + error = ovsdb_log_read(target, &json); if (!error) { - printf("%s: read: ", name); + printf("%s: read: ", target_name); if (json) { print_and_free_json(json); } else { @@ -349,20 +379,32 @@ do_log_io(struct ovs_cmdl_context *ctx) } } else if (!strncmp(command, "write:", 6)) { struct json *json = parse_json(command + 6); - error = ovsdb_log_write(log, json); + error = ovsdb_log_write(target, json); json_destroy(json); } else if (!strcmp(command, "commit")) { - error = ovsdb_log_commit(log); + error = ovsdb_log_commit(target); + } else if (!strcmp(command, "replace_start")) { + ovs_assert(!replacement); + error = ovsdb_log_replace_start(log, &replacement); + } else if (!strcmp(command, "replace_commit")) { + ovs_assert(replacement); + error = ovsdb_log_replace_commit(log, replacement); + replacement = NULL; + } else if (!strcmp(command, "replace_abort")) { + ovs_assert(replacement); + ovsdb_log_replace_abort(replacement); + replacement = NULL; + error = NULL; } else { ovs_fatal(0, "unknown log-io command \"%s\"", command); } if (error) { char *s = ovsdb_error_to_string(error); - printf("%s: %s failed: %s\n", name, command, s); + printf("%s: %s failed: %s\n", target_name, command, s); free(s); ovsdb_error_destroy(error); } else { - printf("%s: %s successful\n", name, command); + printf("%s: %s successful\n", target_name, command); } } From patchwork Fri Dec 8 00:12:35 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 845932 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ytCZw3QnVz9s7v for ; Fri, 8 Dec 2017 11:16:56 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 9961ECCC; Fri, 8 Dec 2017 00:13:00 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 373AACBB for ; Fri, 8 Dec 2017 00:12:58 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id B1ECF189 for ; Fri, 8 Dec 2017 00:12:57 +0000 (UTC) X-Originating-IP: 208.91.3.26 Received: from sigabrt.benpfaff.org (unknown [208.91.3.26]) (Authenticated sender: blp@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 739EEC5A49; Fri, 8 Dec 2017 01:12:55 +0100 (CET) From: Ben Pfaff To: dev@openvswitch.org Date: Thu, 7 Dec 2017 16:12:35 -0800 Message-Id: <20171208001240.25829-9-blp@ovn.org> X-Mailer: git-send-email 2.10.2 In-Reply-To: <20171208001240.25829-1-blp@ovn.org> References: <20171208001240.25829-1-blp@ovn.org> X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Ben Pfaff Subject: [ovs-dev] [PATCH 08/13] log: New function ovsdb_log_compose_record(). X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org This will acquire a new user later on. Signed-off-by: Ben Pfaff --- ovsdb/log.c | 55 ++++++++++++++++++++++++++++++++----------------------- ovsdb/log.h | 4 ++++ 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/ovsdb/log.c b/ovsdb/log.c index 22cd9fe89599..1769e12266d1 100644 --- a/ovsdb/log.c +++ b/ovsdb/log.c @@ -24,6 +24,7 @@ #include #include +#include "openvswitch/dynamic-string.h" #include "openvswitch/json.h" #include "openvswitch/vlog.h" #include "lockfile.h" @@ -419,18 +420,32 @@ ovsdb_log_unread(struct ovsdb_log *file) file->offset = file->prev_offset; } +void +ovsdb_log_compose_record(const struct json *json, + const char *magic, struct ds *header, struct ds *data) +{ + ovs_assert(json->type == JSON_OBJECT || json->type == JSON_ARRAY); + ovs_assert(!header->length); + ovs_assert(!data->length); + + /* Compose content. Add a new-line (replacing the null terminator) to make + * the file easier to read, even though it has no semantic value. */ + json_to_ds(json, 0, data); + ds_put_char(data, '\n'); + + /* Compose header. */ + uint8_t sha1[SHA1_DIGEST_SIZE]; + sha1_bytes(data->string, data->length, sha1); + ds_put_format(header, "%s %"PRIuSIZE" "SHA1_FMT"\n", + magic, data->length, SHA1_ARGS(sha1)); +} + /* Writes log record 'json' to 'file'. Returns NULL if successful or an error * (which the caller must eventually destroy) on failure. */ struct ovsdb_error * ovsdb_log_write(struct ovsdb_log *file, const struct json *json) { - uint8_t sha1[SHA1_DIGEST_SIZE]; struct ovsdb_error *error; - char *json_string; - char header[128]; - size_t length; - - json_string = NULL; switch (file->state) { case OVSDB_LOG_WRITE: @@ -463,22 +478,18 @@ ovsdb_log_write(struct ovsdb_log *file, const struct json *json) goto error; } - /* Compose content. Add a new-line (replacing the null terminator) to make - * the file easier to read, even though it has no semantic value. */ - json_string = json_to_string(json, 0); - length = strlen(json_string) + 1; - json_string[length - 1] = '\n'; - - /* Compose header. */ - sha1_bytes(json_string, length, sha1); - snprintf(header, sizeof header, "%s %"PRIuSIZE" "SHA1_FMT"\n", - file->magic, length, SHA1_ARGS(sha1)); + struct ds header = DS_EMPTY_INITIALIZER; + struct ds data = DS_EMPTY_INITIALIZER; + ovsdb_log_compose_record(json, file->magic, &header, &data); + size_t total_length = header.length + data.length; /* Write. */ - if (fwrite(header, strlen(header), 1, file->stream) != 1 - || fwrite(json_string, length, 1, file->stream) != 1 - || fflush(file->stream)) - { + bool ok = (fwrite(header.string, header.length, 1, file->stream) == 1 + && fwrite(data.string, data.length, 1, file->stream) == 1 + && fflush(file->stream) == 0); + ds_destroy(&header); + ds_destroy(&data); + if (!ok) { error = ovsdb_io_error(errno, "%s: write failed", file->name); static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); @@ -492,14 +503,12 @@ ovsdb_log_write(struct ovsdb_log *file, const struct json *json) goto error; } - file->offset += strlen(header) + length; - free(json_string); + file->offset += total_length; return NULL; error: file->state = OVSDB_LOG_WRITE_ERROR; file->error = ovsdb_error_clone(error); - free(json_string); return error; } diff --git a/ovsdb/log.h b/ovsdb/log.h index 41734e73b351..cc8469c01e57 100644 --- a/ovsdb/log.h +++ b/ovsdb/log.h @@ -19,6 +19,7 @@ #include #include "compiler.h" +struct ds; struct json; struct ovsdb_log; @@ -42,6 +43,9 @@ struct ovsdb_error *ovsdb_log_read(struct ovsdb_log *, struct json **) OVS_WARN_UNUSED_RESULT; void ovsdb_log_unread(struct ovsdb_log *); +void ovsdb_log_compose_record(const struct json *, const char *magic, + struct ds *header, struct ds *data); + struct ovsdb_error *ovsdb_log_write(struct ovsdb_log *, const struct json *) OVS_WARN_UNUSED_RESULT; struct ovsdb_error *ovsdb_log_commit(struct ovsdb_log *) From patchwork Fri Dec 8 00:12:36 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 845934 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ytCcF0YmNz9s7v for ; Fri, 8 Dec 2017 11:18:05 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 9E07FCEA; Fri, 8 Dec 2017 00:13:03 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id AEAD6CBF for ; Fri, 8 Dec 2017 00:12:59 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 46D75FE for ; Fri, 8 Dec 2017 00:12:59 +0000 (UTC) X-Originating-IP: 208.91.3.26 Received: from sigabrt.benpfaff.org (unknown [208.91.3.26]) (Authenticated sender: blp@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 22866C5A51; Fri, 8 Dec 2017 01:12:56 +0100 (CET) From: Ben Pfaff To: dev@openvswitch.org Date: Thu, 7 Dec 2017 16:12:36 -0800 Message-Id: <20171208001240.25829-10-blp@ovn.org> X-Mailer: git-send-email 2.10.2 In-Reply-To: <20171208001240.25829-1-blp@ovn.org> References: <20171208001240.25829-1-blp@ovn.org> X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Ben Pfaff Subject: [ovs-dev] [PATCH 09/13] log: Support using /dev/stdin for opening logs read-only. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org On Unix-like systems, usually /dev/stdin opens a duplicate of fd 0, and this will be convenient in a few places later on. This commit makes this support universal. Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --- ovsdb/log.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ovsdb/log.c b/ovsdb/log.c index 1769e12266d1..d25f766474dc 100644 --- a/ovsdb/log.c +++ b/ovsdb/log.c @@ -166,7 +166,13 @@ ovsdb_log_open(const char *name, const char *magic, #ifdef _WIN32 flags = flags | O_BINARY; #endif - fd = open(name, flags, 0666); + /* Special case for /dev/stdin to make it work even if the operating system + * doesn't support it under that name. */ + if (!strcmp(name, "/dev/stdin") && open_mode == OVSDB_LOG_READ_ONLY) { + fd = dup(STDIN_FILENO); + } else { + fd = open(name, flags, 0666); + } if (fd < 0) { const char *op = (open_mode == OVSDB_LOG_CREATE_EXCL ? "create" : open_mode == OVSDB_LOG_CREATE ? "create or open" From patchwork Fri Dec 8 00:12:37 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 845935 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ytCdG1gQQz9s83 for ; Fri, 8 Dec 2017 11:18:58 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 524F9CF8; Fri, 8 Dec 2017 00:13:05 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 76002CD6 for ; Fri, 8 Dec 2017 00:13:02 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id CD004456 for ; Fri, 8 Dec 2017 00:13:00 +0000 (UTC) X-Originating-IP: 208.91.3.26 Received: from sigabrt.benpfaff.org (unknown [208.91.3.26]) (Authenticated sender: blp@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id B6AC0C5A49; Fri, 8 Dec 2017 01:12:58 +0100 (CET) From: Ben Pfaff To: dev@openvswitch.org Date: Thu, 7 Dec 2017 16:12:37 -0800 Message-Id: <20171208001240.25829-11-blp@ovn.org> X-Mailer: git-send-email 2.10.2 In-Reply-To: <20171208001240.25829-1-blp@ovn.org> References: <20171208001240.25829-1-blp@ovn.org> X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Ben Pfaff Subject: [ovs-dev] [PATCH 10/13] log: Support multiple magic. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org Some OVSDB tools will want to open files that might be standalone or clustered databases, and so it's better if ovsdb_log_open() can accept more than one valid "magic". This commit makes that possible. Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --- ovsdb/log.c | 147 +++++++++++++++++++++++++++++++++++++---------------- ovsdb/log.h | 6 ++- tests/ovsdb-log.at | 2 +- 3 files changed, 108 insertions(+), 47 deletions(-) diff --git a/ovsdb/log.c b/ovsdb/log.c index d25f766474dc..dd641884a2ee 100644 --- a/ovsdb/log.c +++ b/ovsdb/log.c @@ -88,13 +88,19 @@ static bool rename_open_files = false; static bool rename_open_files = true; #endif +static bool parse_header(char *header, const char **magicp, + unsigned long int *length, + uint8_t sha1[SHA1_DIGEST_SIZE]); +static bool is_magic_ok(const char *needle, const char *haystack); + /* 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'. * * 'magic' is a short text string put at the beginning of every record and used * to distinguish one kind of log file from another. For a conventional OVSDB - * log file, use OVSDB_MAGIC. + * log file, use OVSDB_MAGIC. To accept more than one magic string, separate + * them with "|", e.g. "MAGIC 1|MAGIC 2". * * 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 @@ -107,12 +113,16 @@ ovsdb_log_open(const char *name, const char *magic, { struct lockfile *lockfile; struct ovsdb_error *error; - struct ovsdb_log *file; struct stat s; FILE *stream; int flags; int fd; + /* If we can create a new file, we need to know what kind of magic to + * use, so there must be only one kind. */ + if (open_mode == OVSDB_LOG_CREATE_EXCL || open_mode == OVSDB_LOG_CREATE) { + ovs_assert(!strchr(magic, '|')); + } *filep = NULL; ovs_assert(locking == -1 || locking == false || locking == true); @@ -181,43 +191,54 @@ ovsdb_log_open(const char *name, const char *magic, goto error_unlock; } - if (!fstat(fd, &s)) { - if (s.st_size == 0) { - /* It's (probably) a new file so fsync() its parent directory to - * ensure that its directory entry is committed to disk. */ - fsync_parent_dir(name); - } else if (s.st_size >= strlen(magic) && S_ISREG(s.st_mode)) { - /* Try to read the magic from the first log record. If it's not - * the magic we expect, this is the wrong kind of file, so reject - * it immediately. */ - size_t magic_len = strlen(magic); - char *buf = xzalloc(magic_len + 1); - bool err = (read(fd, buf, magic_len) == magic_len - && strcmp(buf, magic)); - free(buf); - if (err) { - error = ovsdb_error(NULL, "%s: bad magic (unexpected " - "kind of file)", name); - goto error_close; - } - if (lseek(fd, 0, SEEK_SET)) { - error = ovsdb_io_error(errno, "%s: seek failed", name); - goto error_close; - } - } - } - 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; + close(fd); + goto error_unlock; + } + + /* Read the magic from the first log record. */ + char header[128]; + const char *actual_magic; + if (!fgets(header, sizeof header, stream)) { + if (ferror(stream)) { + error = ovsdb_io_error(errno, "%s: read error", name); + goto error_fclose; + } + + /* We need to be able to report what kind of file this is but we can't + * if it's empty and we accept more than one. */ + if (strchr(magic, '|')) { + error = ovsdb_error(NULL, "%s: unexpected end of file", name); + goto error_fclose; + } + actual_magic = magic; + + /* It's an empty file and therefore probably a new file, so fsync() + * its parent directory to ensure that its directory entry is + * committed to disk. */ + fsync_parent_dir(name); + } else { + unsigned long int length; + uint8_t sha1[SHA1_DIGEST_SIZE]; + if (!parse_header(header, &actual_magic, &length, sha1) + || !is_magic_ok(actual_magic, magic)) { + error = ovsdb_error(NULL, "%s: unexpected file format", name); + goto error_fclose; + } + } + + if (fseek(stream, 0, SEEK_SET)) { + error = ovsdb_io_error(errno, "%s: seek failed", name); + goto error_fclose; } - file = xmalloc(sizeof *file); + struct ovsdb_log *file = xmalloc(sizeof *file); file->state = OVSDB_LOG_READ; file->error = NULL; file->name = xstrdup(name); - file->magic = xstrdup(magic); + file->magic = xstrdup(actual_magic); file->lockfile = lockfile; file->stream = stream; file->prev_offset = 0; @@ -225,20 +246,43 @@ ovsdb_log_open(const char *name, const char *magic, *filep = file; return NULL; -error_close: - close(fd); +error_fclose: + fclose(stream); error_unlock: lockfile_unlock(lockfile); error: return error; } +/* Returns true if 'needle' is one of the |-delimited words in 'haystack'. */ +static bool +is_magic_ok(const char *needle, const char *haystack) +{ + /* 'needle' can't be multiple words. */ + if (strchr(needle, '|')) { + return false; + } + + size_t n = strlen(needle); + for (;;) { + if (!strncmp(needle, haystack, n) && strchr("|", haystack[n])) { + return true; + } + haystack = strchr(haystack, '|'); + if (!haystack) { + return false; + } + haystack++; + } +} + void ovsdb_log_close(struct ovsdb_log *file) { if (file) { ovsdb_error_destroy(file->error); free(file->name); + free(file->magic); if (file->stream) { fclose(file->stream); } @@ -247,20 +291,34 @@ ovsdb_log_close(struct ovsdb_log *file) } } +const char * +ovsdb_log_get_magic(const struct ovsdb_log *log) +{ + return log->magic; +} + static bool -parse_header(const char *magic, char *header, unsigned long int *length, - uint8_t sha1[SHA1_DIGEST_SIZE]) +parse_header(char *header, const char **magicp, + unsigned long int *length, uint8_t sha1[SHA1_DIGEST_SIZE]) { - char *p; + /* 'header' must consist of "OVSDB "... */ + const char lead[] = "OVSDB "; + if (strncmp(lead, header, strlen(lead))) { + return false; + } - /* 'header' must consist of a magic string... */ - size_t magic_len = strlen(magic); - if (strncmp(header, magic, magic_len) || header[magic_len] != ' ') { + /* ...followed by a magic string... */ + char *magic = header + strlen(lead); + size_t magic_len = strcspn(magic, " "); + if (magic[magic_len] != ' ') { return false; } + magic[magic_len] = '\0'; + *magicp = magic; /* ...followed by a length in bytes... */ - *length = strtoul(header + magic_len + 1, &p, 10); + char *p; + *length = strtoul(magic + magic_len + 1, &p, 10); if (!*length || *length == ULONG_MAX || *p != ' ') { return false; } @@ -342,7 +400,6 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp) uint8_t expected_sha1[SHA1_DIGEST_SIZE]; uint8_t actual_sha1[SHA1_DIGEST_SIZE]; struct ovsdb_error *error; - off_t data_offset; unsigned long data_length; struct json *json; char header[128]; @@ -356,8 +413,11 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp) error = ovsdb_io_error(errno, "%s: read failed", file->name); goto error; } + off_t data_offset = file->offset + strlen(header); - if (!parse_header(file->magic, header, &data_length, expected_sha1)) { + const char *magic; + if (!parse_header(header, &magic, &data_length, expected_sha1) + || strcmp(magic, file->magic)) { error = ovsdb_syntax_error(NULL, NULL, "%s: parse error at offset " "%lld in header line \"%.*s\"", file->name, (long long int) file->offset, @@ -365,7 +425,6 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp) goto error; } - data_offset = file->offset + strlen(header); error = parse_body(file, data_offset, data_length, actual_sha1, &json); if (error) { goto error; @@ -442,7 +501,7 @@ ovsdb_log_compose_record(const struct json *json, /* Compose header. */ uint8_t sha1[SHA1_DIGEST_SIZE]; sha1_bytes(data->string, data->length, sha1); - ds_put_format(header, "%s %"PRIuSIZE" "SHA1_FMT"\n", + ds_put_format(header, "OVSDB %s %"PRIuSIZE" "SHA1_FMT"\n", magic, data->length, SHA1_ARGS(sha1)); } diff --git a/ovsdb/log.h b/ovsdb/log.h index cc8469c01e57..4b74ca11ce6a 100644 --- a/ovsdb/log.h +++ b/ovsdb/log.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2009, 2010, 2011 Nicira, Inc. +/* Copyright (c) 2009, 2010, 2011, 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. @@ -31,7 +31,7 @@ enum ovsdb_log_open_mode { OVSDB_LOG_CREATE /* Create or open file, read/write. */ }; -#define OVSDB_MAGIC "OVSDB JSON" +#define OVSDB_MAGIC "JSON" struct ovsdb_error *ovsdb_log_open(const char *name, const char *magic, enum ovsdb_log_open_mode, @@ -39,6 +39,8 @@ struct ovsdb_error *ovsdb_log_open(const char *name, const char *magic, OVS_WARN_UNUSED_RESULT; void ovsdb_log_close(struct ovsdb_log *); +const char *ovsdb_log_get_magic(const struct ovsdb_log *); + struct ovsdb_error *ovsdb_log_read(struct ovsdb_log *, struct json **) OVS_WARN_UNUSED_RESULT; void ovsdb_log_unread(struct ovsdb_log *); diff --git a/tests/ovsdb-log.at b/tests/ovsdb-log.at index 125ddd187551..9aa1e870039c 100644 --- a/tests/ovsdb-log.at +++ b/tests/ovsdb-log.at @@ -174,7 +174,7 @@ file: read: end of log ]], [ignore]) AT_CHECK( [test-ovsdb log-io file read-only], [1], [], - [test-ovsdb: ovsdb error: file: bad magic (unexpected kind of file) + [test-ovsdb: ovsdb error: file: unexpected file format ]) AT_CHECK([test -f .file.~lock~]) AT_CLEANUP From patchwork Fri Dec 8 00:12:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 845937 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ytCf23zvRz9s83 for ; Fri, 8 Dec 2017 11:19:38 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 4E6EFCEB; Fri, 8 Dec 2017 00:13:06 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id AE0EECCB for ; Fri, 8 Dec 2017 00:13:02 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id E3CB4418 for ; Fri, 8 Dec 2017 00:13:01 +0000 (UTC) X-Originating-IP: 208.91.3.26 Received: from sigabrt.benpfaff.org (unknown [208.91.3.26]) (Authenticated sender: blp@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 0BA1BC5A51; Fri, 8 Dec 2017 01:12:59 +0100 (CET) From: Ben Pfaff To: dev@openvswitch.org Date: Thu, 7 Dec 2017 16:12:38 -0800 Message-Id: <20171208001240.25829-12-blp@ovn.org> X-Mailer: git-send-email 2.10.2 In-Reply-To: <20171208001240.25829-1-blp@ovn.org> References: <20171208001240.25829-1-blp@ovn.org> X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Ben Pfaff Subject: [ovs-dev] [PATCH 11/13] log: Replace ovsdb_log_get_offset() by a more abstract mechanism. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org Upcoming support for clustered databases will need to provide a more abstract way to determine when a given file should be compacted, so this changes the standalone database support to use this mechanism in advance. Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --- ovsdb/file.c | 24 ++++++++---------------- ovsdb/log.c | 26 +++++++++++++++++++------- ovsdb/log.h | 22 +++++++++++++++++++++- 3 files changed, 48 insertions(+), 24 deletions(-) diff --git a/ovsdb/file.c b/ovsdb/file.c index fca07c3d722d..094fa0ce207a 100644 --- a/ovsdb/file.c +++ b/ovsdb/file.c @@ -73,7 +73,6 @@ static struct ovsdb_error *ovsdb_file_create(struct ovsdb *, struct ovsdb_log *, const char *file_name, unsigned int n_transactions, - off_t snapshot_size, struct ovsdb_file **filep); /* Opens database 'file_name' and stores a pointer to the new database in @@ -209,7 +208,6 @@ ovsdb_file_open__(const char *file_name, * * The schema precedes the snapshot in the log; we could compensate for its * size, but it's just not that important. */ - off_t snapshot_size = 0; unsigned int n_transactions = 0; while ((error = ovsdb_log_read(log, &json)) == NULL && json) { struct ovsdb_txn *txn; @@ -230,7 +228,7 @@ ovsdb_file_open__(const char *file_name, } if (n_transactions == 1) { - snapshot_size = ovsdb_log_get_offset(log); + ovsdb_log_mark_base(log); } } if (error) { @@ -247,8 +245,7 @@ ovsdb_file_open__(const char *file_name, if (!read_only) { struct ovsdb_file *file; - error = ovsdb_file_create(db, log, file_name, n_transactions, - snapshot_size, &file); + error = ovsdb_file_create(db, log, file_name, n_transactions, &file); if (error) { goto error; } @@ -515,7 +512,6 @@ struct ovsdb_file { long long int last_compact; long long int next_compact; unsigned int n_transactions; - off_t snapshot_size; }; static const struct ovsdb_replica_class ovsdb_file_class; @@ -523,8 +519,7 @@ static const struct ovsdb_replica_class ovsdb_file_class; static struct ovsdb_error * ovsdb_file_create(struct ovsdb *db, struct ovsdb_log *log, const char *file_name, - unsigned int n_transactions, off_t snapshot_size, - struct ovsdb_file **filep) + unsigned int n_transactions, struct ovsdb_file **filep) { struct ovsdb_file *file; char *deref_name; @@ -548,7 +543,6 @@ ovsdb_file_create(struct ovsdb *db, struct ovsdb_log *log, file->file_name = abs_name; file->last_compact = time_msec(); file->next_compact = file->last_compact + COMPACT_MIN_MSEC; - file->snapshot_size = snapshot_size; file->n_transactions = n_transactions; ovsdb_add_replica(db, &file->replica); @@ -601,12 +595,9 @@ ovsdb_file_commit(struct ovsdb_replica *replica, * tried), and if there are at least 100 transactions in the database, and * if the database is at least 10 MB, and the database is at least 4x the * size of the previous snapshot, then compact the database. */ - off_t log_size = ovsdb_log_get_offset(file->log); if (time_msec() >= file->next_compact && file->n_transactions >= 100 - && log_size >= 10 * 1024 * 1024 - && log_size / 4 >= file->snapshot_size) - { + && ovsdb_log_has_grown(file->log)) { error = ovsdb_file_compact(file); if (error) { char *s = ovsdb_error_to_string(error); @@ -653,10 +644,9 @@ ovsdb_file_compact(struct ovsdb_file *file) int retval; comment = xasprintf("compacting database online " - "(%.3f seconds old, %u transactions, %llu bytes)", + "(%.3f seconds old, %u transactions)", (time_wall_msec() - file->last_compact) / 1000.0, - file->n_transactions, - (unsigned long long) ovsdb_log_get_offset(file->log)); + file->n_transactions); VLOG_INFO("%s: %s", file->file_name, comment); /* Commit the old version, so that we can be assured that we'll eventually @@ -686,6 +676,7 @@ ovsdb_file_compact(struct ovsdb_file *file) if (error) { goto exit; } + ovsdb_log_mark_base(new_log); /* Replace original file by the temporary file. * @@ -740,6 +731,7 @@ ovsdb_file_compact(struct ovsdb_file *file) ovs_fatal(0, "error reading database"); } json_destroy(json); + ovsdb_log_mark_base(file->log); } /* Success! */ diff --git a/ovsdb/log.c b/ovsdb/log.c index dd641884a2ee..27f4b328d451 100644 --- a/ovsdb/log.c +++ b/ovsdb/log.c @@ -76,6 +76,7 @@ struct ovsdb_log { char *magic; struct lockfile *lockfile; FILE *stream; + off_t base; }; /* Whether the OS supports renaming open files. @@ -243,6 +244,7 @@ ovsdb_log_open(const char *name, const char *magic, file->stream = stream; file->prev_offset = 0; file->offset = 0; + file->base = 0; *filep = file; return NULL; @@ -586,13 +588,21 @@ ovsdb_log_commit(struct ovsdb_log *file) return NULL; } -/* Returns the current offset into the file backing 'log', in bytes. This - * reflects the number of bytes that have been read or written in the file. If - * the whole file has been read, this is the file size. */ -off_t -ovsdb_log_get_offset(const struct ovsdb_log *log) +/* Sets the current position in 'log' as the "base", that is, the initial size + * of the log that ovsdb_log_has_grown() uses to determine whether the log has + * grown enough to make compacting worthwhile. */ +void +ovsdb_log_mark_base(struct ovsdb_log *log) { - return log->offset; + log->base = log->offset; +} + +/* Returns true if 'log' has grown enough above the base that it's worthwhile + * to compact it, false otherwise. */ +bool +ovsdb_log_has_grown(const struct ovsdb_log *log) +{ + return log->offset > 10 * 1024 * 1024 && log->offset / 4 > log->base; } /* Attempts to atomically replace the contents of 'log', on disk, by the 'n' @@ -618,6 +628,7 @@ ovsdb_log_replace(struct ovsdb_log *log, struct json **entries, size_t n) return error; } } + ovsdb_log_mark_base(new); return ovsdb_log_replace_commit(log, new); } @@ -752,7 +763,8 @@ ovsdb_log_replace_commit(struct ovsdb_log *old, struct ovsdb_log *new) old->magic = new->magic; new->magic = NULL; /* Keep old->lockfile. */ - /* stream was already replaced above. */ + old->base = new->base; + /* Free 'new'. */ ovsdb_log_close(new); diff --git a/ovsdb/log.h b/ovsdb/log.h index 4b74ca11ce6a..e68e922f871a 100644 --- a/ovsdb/log.h +++ b/ovsdb/log.h @@ -16,6 +16,25 @@ #ifndef OVSDB_LOG_H #define OVSDB_LOG_H 1 +/* OVSDB log. + * + * A log consists of a series of records. After opening or creating a log with + * ovsdb_log_open(), the client may use ovsdb_log_read() to read any existing + * records, one by one. The client may also use ovsdb_log_write() to write new + * records (if some records have not yet been read at this point, then the + * first write truncates them). + * + * Log writes are atomic. A client may use ovsdb_log_commit() to ensure that + * they are durable. + * + * Logs provide a mechansim to allow the client to tell when they have grown + * enough that compacting may be warranted. After reading existing log + * contents, the client uses ovsdb_log_mark_base() to mark the "base" to be + * considered as the initial size of the log. Thereafter, a client may call + * ovsdb_log_has_grown() to get an indication whether the log has grown enough + * that compacting is advised. + */ + #include #include "compiler.h" @@ -53,7 +72,8 @@ struct ovsdb_error *ovsdb_log_write(struct ovsdb_log *, const struct json *) struct ovsdb_error *ovsdb_log_commit(struct ovsdb_log *) OVS_WARN_UNUSED_RESULT; -off_t ovsdb_log_get_offset(const struct ovsdb_log *); +void ovsdb_log_mark_base(struct ovsdb_log *); +bool ovsdb_log_has_grown(const struct ovsdb_log *); struct ovsdb_error *ovsdb_log_replace(struct ovsdb_log *, struct json **entries, size_t n) From patchwork Fri Dec 8 00:12:39 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 845938 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ytCff0SQXz9s84 for ; Fri, 8 Dec 2017 11:20:09 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 34B00CFF; Fri, 8 Dec 2017 00:13:08 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp2.linuxfoundation.org (smtp2.linux-foundation.org [172.17.192.36]) by mail.linuxfoundation.org (Postfix) with ESMTPS id B13E2CEC for ; Fri, 8 Dec 2017 00:13:04 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp2.linuxfoundation.org (Postfix) with ESMTPS id C17291DAA8 for ; Fri, 8 Dec 2017 00:13:03 +0000 (UTC) X-Originating-IP: 208.91.3.26 Received: from sigabrt.benpfaff.org (unknown [208.91.3.26]) (Authenticated sender: blp@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 5D22EC5A46; Fri, 8 Dec 2017 01:13:01 +0100 (CET) From: Ben Pfaff To: dev@openvswitch.org Date: Thu, 7 Dec 2017 16:12:39 -0800 Message-Id: <20171208001240.25829-13-blp@ovn.org> X-Mailer: git-send-email 2.10.2 In-Reply-To: <20171208001240.25829-1-blp@ovn.org> References: <20171208001240.25829-1-blp@ovn.org> X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp2.linux-foundation.org Cc: Ben Pfaff Subject: [ovs-dev] [PATCH 12/13] log: Use absolute name of log file. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org In ovsdb-server, the OVSDB log code is used to open the databases specified on the command line before ovsdb-server daemonizes itself. Afterward, it is occasionally necessary for ovsdb-server to reference those files by name again. When that happens, if daemonization changed the current directory to the root, any relative names are no longer valid and those references will fail. Until now, this was handled at a higher level in ovsdb-server, but in the future it will be convenient to handle it in the log code itself. This commit prepares for that by making the log code take the absolute name of log files itself. Signed-off-by: Ben Pfaff Acked-by: Justin Pettit --- ovsdb/log.c | 93 +++++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 60 insertions(+), 33 deletions(-) diff --git a/ovsdb/log.c b/ovsdb/log.c index 27f4b328d451..e364667ac673 100644 --- a/ovsdb/log.c +++ b/ovsdb/log.c @@ -72,7 +72,8 @@ struct ovsdb_log { off_t prev_offset; off_t offset; - char *name; + char *name; /* Absolute name of file. */ + char *display_name; /* For use in log messages, etc. */ char *magic; struct lockfile *lockfile; FILE *stream; @@ -124,8 +125,22 @@ ovsdb_log_open(const char *name, const char *magic, if (open_mode == OVSDB_LOG_CREATE_EXCL || open_mode == OVSDB_LOG_CREATE) { ovs_assert(!strchr(magic, '|')); } + *filep = NULL; + /* Get the absolute name of the file because we might need to access it by + * name again later after the process has changed directory (e.g. because + * daemonize() chdirs to "/"). + * + * We save the user-provided name of the file for use in log messages, to + * reduce user confusion. */ + char *abs_name = abs_file_name(NULL, name); + if (!abs_name) { + error = ovsdb_io_error(0, "could not determine current " + "working directory"); + goto error; + } + ovs_assert(locking == -1 || locking == false || locking == true); if (locking < 0) { locking = open_mode != OVSDB_LOG_READ_ONLY; @@ -238,7 +253,8 @@ ovsdb_log_open(const char *name, const char *magic, struct ovsdb_log *file = xmalloc(sizeof *file); file->state = OVSDB_LOG_READ; file->error = NULL; - file->name = xstrdup(name); + file->name = abs_name; + file->display_name = xstrdup(name); file->magic = xstrdup(actual_magic); file->lockfile = lockfile; file->stream = stream; @@ -253,6 +269,7 @@ error_fclose: error_unlock: lockfile_unlock(lockfile); error: + free(abs_name); return error; } @@ -284,6 +301,7 @@ ovsdb_log_close(struct ovsdb_log *file) if (file) { ovsdb_error_destroy(file->error); free(file->name); + free(file->display_name); free(file->magic); if (file->stream) { fclose(file->stream); @@ -359,8 +377,9 @@ parse_body(struct ovsdb_log *file, off_t offset, unsigned long int length, json_parser_abort(parser); return ovsdb_io_error(ferror(file->stream) ? errno : EOF, "%s: error reading %lu bytes " - "starting at offset %lld", file->name, - length, (long long int) offset); + "starting at offset %lld", + file->display_name, length, + (long long int) offset); } sha1_update(&ctx, input, chunk); json_parser_feed(parser, input, chunk); @@ -412,7 +431,7 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp) if (feof(file->stream)) { return NULL; } - error = ovsdb_io_error(errno, "%s: read failed", file->name); + error = ovsdb_io_error(errno, "%s: read failed", file->display_name); goto error; } off_t data_offset = file->offset + strlen(header); @@ -422,7 +441,8 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp) || strcmp(magic, file->magic)) { error = ovsdb_syntax_error(NULL, NULL, "%s: parse error at offset " "%lld in header line \"%.*s\"", - file->name, (long long int) file->offset, + file->display_name, + (long long int) file->offset, (int) strcspn(header, "\n"), header); goto error; } @@ -436,7 +456,7 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp) error = ovsdb_syntax_error(NULL, NULL, "%s: %lu bytes starting at " "offset %lld have SHA-1 hash "SHA1_FMT" " "but should have hash "SHA1_FMT, - file->name, data_length, + file->display_name, data_length, (long long int) data_offset, SHA1_ARGS(actual_sha1), SHA1_ARGS(expected_sha1)); @@ -446,7 +466,7 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp) if (json->type == JSON_STRING) { error = ovsdb_syntax_error(NULL, NULL, "%s: %lu bytes starting at " "offset %lld are not valid JSON (%s)", - file->name, data_length, + file->display_name, data_length, (long long int) data_offset, json->u.string); goto error; @@ -454,7 +474,7 @@ ovsdb_log_read(struct ovsdb_log *file, struct json **jsonp) if (json->type != JSON_OBJECT) { error = ovsdb_syntax_error(NULL, NULL, "%s: %lu bytes starting at " "offset %lld are not a JSON object", - file->name, data_length, + file->display_name, data_length, (long long int) data_offset); goto error; } @@ -487,6 +507,24 @@ ovsdb_log_unread(struct ovsdb_log *file) file->offset = file->prev_offset; } +static struct ovsdb_error * +ovsdb_log_truncate(struct ovsdb_log *file) +{ + file->state = OVSDB_LOG_WRITE; + + struct ovsdb_error *error = NULL; + if (fseeko(file->stream, file->offset, SEEK_SET)) { + error = ovsdb_io_error(errno, "%s: cannot seek to offset %lld", + file->display_name, + (long long int) file->offset); + } else if (ftruncate(fileno(file->stream), file->offset)) { + error = ovsdb_io_error(errno, "%s: cannot truncate to length %lld", + file->display_name, + (long long int) file->offset); + } + return error; +} + void ovsdb_log_compose_record(const struct json *json, const char *magic, struct ds *header, struct ds *data) @@ -512,8 +550,6 @@ ovsdb_log_compose_record(const struct json *json, struct ovsdb_error * ovsdb_log_write(struct ovsdb_log *file, const struct json *json) { - struct ovsdb_error *error; - switch (file->state) { case OVSDB_LOG_WRITE: break; @@ -521,19 +557,13 @@ ovsdb_log_write(struct ovsdb_log *file, const struct json *json) case OVSDB_LOG_READ: case OVSDB_LOG_READ_ERROR: case OVSDB_LOG_WRITE_ERROR: - file->state = OVSDB_LOG_WRITE; ovsdb_error_destroy(file->error); - file->error = NULL; - if (fseeko(file->stream, file->offset, SEEK_SET)) { - error = ovsdb_io_error(errno, "%s: cannot seek to offset %lld", - file->name, (long long int) file->offset); - goto error; - } - if (ftruncate(fileno(file->stream), file->offset)) { - error = ovsdb_io_error(errno, "%s: cannot truncate to length %lld", - file->name, (long long int) file->offset); - goto error; + file->error = ovsdb_log_truncate(file); + if (file->error) { + file->state = OVSDB_LOG_WRITE_ERROR; + return ovsdb_error_clone(file->error); } + file->state = OVSDB_LOG_WRITE; break; case OVSDB_LOG_BROKEN: @@ -541,8 +571,7 @@ ovsdb_log_write(struct ovsdb_log *file, const struct json *json) } if (json->type != JSON_OBJECT && json->type != JSON_ARRAY) { - error = OVSDB_BUG("bad JSON type"); - goto error; + return OVSDB_BUG("bad JSON type"); } struct ds header = DS_EMPTY_INITIALIZER; @@ -557,33 +586,31 @@ ovsdb_log_write(struct ovsdb_log *file, const struct json *json) ds_destroy(&header); ds_destroy(&data); if (!ok) { - error = ovsdb_io_error(errno, "%s: write failed", file->name); + int error = errno; static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); VLOG_WARN_RL(&rl, "%s: write failed (%s)", - file->name, ovs_strerror(errno)); + file->name, ovs_strerror(error)); /* Remove any partially written data, ignoring errors since there is * nothing further we can do. */ ignore(ftruncate(fileno(file->stream), file->offset)); - goto error; + file->error = ovsdb_io_error(error, "%s: write failed", + file->display_name); + file->state = OVSDB_LOG_WRITE_ERROR; + return ovsdb_error_clone(file->error); } file->offset += total_length; return NULL; - -error: - file->state = OVSDB_LOG_WRITE_ERROR; - file->error = ovsdb_error_clone(error); - return error; } struct ovsdb_error * ovsdb_log_commit(struct ovsdb_log *file) { if (file->stream && fsync(fileno(file->stream))) { - return ovsdb_io_error(errno, "%s: fsync failed", file->name); + return ovsdb_io_error(errno, "%s: fsync failed", file->display_name); } return NULL; } From patchwork Fri Dec 8 00:12:40 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 845939 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3ytCgH2GBhz9s84 for ; Fri, 8 Dec 2017 11:20:43 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 1D734D1A; Fri, 8 Dec 2017 00:13:09 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp2.linuxfoundation.org (smtp2.linux-foundation.org [172.17.192.36]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 070F3CFC for ; Fri, 8 Dec 2017 00:13:06 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp2.linuxfoundation.org (Postfix) with ESMTPS id 327B11DAA8 for ; Fri, 8 Dec 2017 00:13:05 +0000 (UTC) X-Originating-IP: 208.91.3.26 Received: from sigabrt.benpfaff.org (unknown [208.91.3.26]) (Authenticated sender: blp@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id E1D46C5A4F; Fri, 8 Dec 2017 01:13:02 +0100 (CET) From: Ben Pfaff To: dev@openvswitch.org Date: Thu, 7 Dec 2017 16:12:40 -0800 Message-Id: <20171208001240.25829-14-blp@ovn.org> X-Mailer: git-send-email 2.10.2 In-Reply-To: <20171208001240.25829-1-blp@ovn.org> References: <20171208001240.25829-1-blp@ovn.org> X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp2.linux-foundation.org Cc: Ben Pfaff Subject: [ovs-dev] [PATCH 13/13] log: Add async commit support. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org The OVSDB log code has always had the ability to commit the log to disk and wait for the commit to finish. This patch introduces a new feature that allows the client to start a commit in the background and then to determine asynchronously that the commit has completed. This will be especially useful later for the distributed database feature. Signed-off-by: Ben Pfaff --- ovsdb/file.c | 4 +- ovsdb/log.c | 152 +++++++++++++++++++++++++++++++++++++++++++++++++++-- ovsdb/log.h | 7 ++- ovsdb/ovsdb-tool.c | 2 +- tests/test-ovsdb.c | 2 +- 5 files changed, 158 insertions(+), 9 deletions(-) diff --git a/ovsdb/file.c b/ovsdb/file.c index 094fa0ce207a..40fecdf59af5 100644 --- a/ovsdb/file.c +++ b/ovsdb/file.c @@ -651,7 +651,7 @@ ovsdb_file_compact(struct ovsdb_file *file) /* Commit the old version, so that we can be assured that we'll eventually * have either the old or the new version. */ - error = ovsdb_log_commit(file->log); + error = ovsdb_log_commit_block(file->log); if (error) { goto exit; } @@ -854,7 +854,7 @@ ovsdb_file_txn_commit(struct json *json, const char *comment, } if (durable) { - error = ovsdb_log_commit(log); + error = ovsdb_log_commit_block(log); if (error) { return ovsdb_wrap_error(error, "committing transaction failed"); } diff --git a/ovsdb/log.c b/ovsdb/log.c index e364667ac673..e2c58d1cc8b1 100644 --- a/ovsdb/log.c +++ b/ovsdb/log.c @@ -24,12 +24,17 @@ #include #include +#include "lockfile.h" #include "openvswitch/dynamic-string.h" #include "openvswitch/json.h" #include "openvswitch/vlog.h" -#include "lockfile.h" -#include "ovsdb.h" +#include "ovs-atomic.h" +#include "ovs-rcu.h" +#include "ovs-thread.h" #include "ovsdb-error.h" +#include "ovsdb.h" +#include "openvswitch/poll-loop.h" +#include "seq.h" #include "sha1.h" #include "socket-util.h" #include "transaction.h" @@ -78,6 +83,7 @@ struct ovsdb_log { struct lockfile *lockfile; FILE *stream; off_t base; + struct afsync *afsync; }; /* Whether the OS supports renaming open files. @@ -95,6 +101,9 @@ static bool parse_header(char *header, const char **magicp, uint8_t sha1[SHA1_DIGEST_SIZE]); static bool is_magic_ok(const char *needle, const char *haystack); +static struct afsync *afsync_create(int fd, uint64_t initial_ticket); +static uint64_t afsync_destroy(struct afsync *); + /* 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'. @@ -261,6 +270,7 @@ ovsdb_log_open(const char *name, const char *magic, file->prev_offset = 0; file->offset = 0; file->base = 0; + file->afsync = NULL; *filep = file; return NULL; @@ -300,6 +310,7 @@ ovsdb_log_close(struct ovsdb_log *file) { if (file) { ovsdb_error_destroy(file->error); + afsync_destroy(file->afsync); free(file->name); free(file->display_name); free(file->magic); @@ -606,8 +617,10 @@ ovsdb_log_write(struct ovsdb_log *file, const struct json *json) return NULL; } +/* Attempts to commit 'file' to disk. Waits for the commit to succeed or fail. + * Returns NULL if successful, otherwise the error that occurred. */ struct ovsdb_error * -ovsdb_log_commit(struct ovsdb_log *file) +ovsdb_log_commit_block(struct ovsdb_log *file) { if (file->stream && fsync(fileno(file->stream))) { return ovsdb_io_error(errno, "%s: fsync failed", file->display_name); @@ -712,7 +725,7 @@ ovsdb_rename(const char *old, const char *new) struct ovsdb_error * OVS_WARN_UNUSED_RESULT ovsdb_log_replace_commit(struct ovsdb_log *old, struct ovsdb_log *new) { - struct ovsdb_error *error = ovsdb_log_commit(new); + struct ovsdb_error *error = ovsdb_log_commit_block(new); if (error) { ovsdb_log_replace_abort(new); return error; @@ -784,6 +797,10 @@ ovsdb_log_replace_commit(struct ovsdb_log *old, struct ovsdb_log *new) ovsdb_error_destroy(old->error); old->error = NULL; /* prev_offset only matters for OVSDB_LOG_READ. */ + if (old->afsync) { + uint64_t ticket = afsync_destroy(old->afsync); + old->afsync = afsync_create(fileno(old->stream), ticket + 1); + } old->offset = new->offset; /* Keep old->name. */ free(old->magic); @@ -816,3 +833,130 @@ ovsdb_log_disable_renaming_open_files(void) { rename_open_files = false; } + +struct afsync { + pthread_t thread; + atomic_uint64_t cur, next; + struct seq *request, *complete; + int fd; +}; + +static void * +afsync_thread(void *afsync_) +{ + struct afsync *afsync = afsync_; + uint64_t cur = 0; + for (;;) { + ovsrcu_quiesce_start(); + + uint64_t request_seq = seq_read(afsync->request); + + uint64_t next; + atomic_read_explicit(&afsync->next, &next, memory_order_acquire); + if (next == UINT64_MAX) { + break; + } + + if (cur != next && afsync->fd != -1) { + int error = fsync(afsync->fd) ? errno : 0; + if (!error) { + cur = next; + atomic_store_explicit(&afsync->cur, cur, memory_order_release); + seq_change(afsync->complete); + } else { + VLOG_WARN("fsync failed (%s)", ovs_strerror(error)); + } + } + + seq_wait(afsync->request, request_seq); + poll_block(); + } + return NULL; +} + +static struct afsync * +afsync_create(int fd, uint64_t initial_ticket) +{ + struct afsync *afsync = xzalloc(sizeof *afsync); + atomic_init(&afsync->cur, initial_ticket); + atomic_init(&afsync->next, initial_ticket); + afsync->request = seq_create(); + afsync->complete = seq_create(); + afsync->thread = ovs_thread_create("log_fsync", afsync_thread, afsync); + afsync->fd = fd; + return afsync; +} + +static uint64_t +afsync_destroy(struct afsync *afsync) +{ + if (!afsync) { + return 0; + } + + uint64_t next; + atomic_read(&afsync->next, &next); + atomic_store(&afsync->next, UINT64_MAX); + seq_change(afsync->request); + xpthread_join(afsync->thread, NULL); + + seq_destroy(afsync->request); + seq_destroy(afsync->complete); + + free(afsync); + + return next; +} + +static struct afsync * +ovsdb_log_get_afsync(struct ovsdb_log *log) +{ + if (!log->afsync) { + log->afsync = afsync_create(log->stream ? fileno(log->stream) : -1, 0); + } + return log->afsync; +} + +/* Starts committing 'log' to disk. Returns a ticket that can be passed to + * ovsdb_log_commit_wait() or compared against the return value of + * ovsdb_log_commit_progress() later. */ +uint64_t +ovsdb_log_commit_start(struct ovsdb_log *log) +{ + struct afsync *afsync = ovsdb_log_get_afsync(log); + + uint64_t orig; + atomic_add_explicit(&afsync->next, 1, &orig, memory_order_acq_rel); + + seq_change(afsync->request); + + return orig + 1; +} + +/* Returns a ticket value that represents the current progress of commits to + * 'log'. Suppose that some call to ovsdb_log_commit_start() returns X and any + * call ovsdb_log_commit_progress() returns Y, for the same 'log'. Then commit + * X is complete if and only if X <= Y. */ +uint64_t +ovsdb_log_commit_progress(struct ovsdb_log *log) +{ + struct afsync *afsync = ovsdb_log_get_afsync(log); + uint64_t cur; + atomic_read_explicit(&afsync->cur, &cur, memory_order_acquire); + return cur; +} + +/* Causes poll_block() to wake up if and when ovsdb_log_commit_progress(log) + * would return at least 'goal'. */ +void +ovsdb_log_commit_wait(struct ovsdb_log *log, uint64_t goal) +{ + struct afsync *afsync = ovsdb_log_get_afsync(log); + uint64_t complete = seq_read(afsync->complete); + uint64_t cur = ovsdb_log_commit_progress(log); + if (cur < goal) { + seq_wait(afsync->complete, complete); + } else { + poll_immediate_wake(); + } +} diff --git a/ovsdb/log.h b/ovsdb/log.h index e68e922f871a..57770a989ae2 100644 --- a/ovsdb/log.h +++ b/ovsdb/log.h @@ -35,6 +35,7 @@ * that compacting is advised. */ +#include #include #include "compiler.h" @@ -69,7 +70,11 @@ void ovsdb_log_compose_record(const struct json *, const char *magic, struct ovsdb_error *ovsdb_log_write(struct ovsdb_log *, const struct json *) OVS_WARN_UNUSED_RESULT; -struct ovsdb_error *ovsdb_log_commit(struct ovsdb_log *) + +uint64_t ovsdb_log_commit_start(struct ovsdb_log *); +uint64_t ovsdb_log_commit_progress(struct ovsdb_log *); +void ovsdb_log_commit_wait(struct ovsdb_log *, uint64_t); +struct ovsdb_error *ovsdb_log_commit_block(struct ovsdb_log *) OVS_WARN_UNUSED_RESULT; void ovsdb_log_mark_base(struct ovsdb_log *); diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c index da6b7142a4fa..7dc19758353b 100644 --- a/ovsdb/ovsdb-tool.c +++ b/ovsdb/ovsdb-tool.c @@ -220,7 +220,7 @@ do_create(struct ovs_cmdl_context *ctx) check_ovsdb_error(ovsdb_log_open(db_file_name, OVSDB_MAGIC, OVSDB_LOG_CREATE_EXCL, -1, &log)); check_ovsdb_error(ovsdb_log_write(log, json)); - check_ovsdb_error(ovsdb_log_commit(log)); + check_ovsdb_error(ovsdb_log_commit_block(log)); ovsdb_log_close(log); json_destroy(json); diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index 6d2799c1a131..3ca0958e3163 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -382,7 +382,7 @@ do_log_io(struct ovs_cmdl_context *ctx) error = ovsdb_log_write(target, json); json_destroy(json); } else if (!strcmp(command, "commit")) { - error = ovsdb_log_commit(target); + error = ovsdb_log_commit_block(target); } else if (!strcmp(command, "replace_start")) { ovs_assert(!replacement); error = ovsdb_log_replace_start(log, &replacement);