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++) {