Re: [ovs-dev] [PATCH v3 5/8] ovsdb-client: Add new "backup" command.

[Ben Pfaff]

On Thu, Dec 14, 2017 at 05:55:36PM -0800, Justin Pettit wrote:
> 
> 
> > On Dec 13, 2017, at 3:10 PM, Ben Pfaff  wrote:
> > 
> > diff --git a/ovsdb/ovsdb-client.1.in b/ovsdb/ovsdb-client.1.in
> > index 694a7abed46d..2e2df5e5aa7f 100644
> > --- a/ovsdb/ovsdb-client.1.in
> > +++ b/ovsdb/ovsdb-client.1.in
> > @@ -30,6 +30,9 @@ ovsdb\-client \- command-line interface to 
> > \fBovsdb-server\fR(1)
> > \fBovsdb\-client \fR[\fIoptions\fR] \fBdump\fI \fR[\fIserver\fR] 
> > \fR[\fIdatabase\fR]\fR [\fItable\fR
> > [\fIcolumn\fR...]]
> > .br
> > +\fBovsdb\-client \fR[\fIoptions\fR]
> > +\fBbackup\fI \fR[\fIserver\fR] \fR[\fIdatabase\fR] > \fIsnapshot\fR
> 
> Is the first "\fI" doing anything?  Same question about the third "\fR".  I 
> think some of this is a copy/paste from the other prototypes.  I don't think 
> they make a visual difference, but those lines are pretty complicated, so it 
> might be nice to simplify them if possible.
> 
> > @@ -138,6 +141,21 @@ and prints it on stdout as a series of tables. If 
> > \fItable\fR is
> > specified, only that table is retrieved.  If at least one \fIcolumn\fR
> > is specified, only those columns are retrieved.
> > .
> > +.IP "\fBbackup\fI \fR[\fIserver\fR] \fR[\fIdatabase\fR] > \fIsnapshot\fR"
> 
> Same question about simplifying the markup.
> 
> > +Connects to \fIserver\fR, retrieves a snapshot of the schema and data
> > +in \fIdatabase\fR, and prints it on stdout in the format used for
> > +OVSDB standalone and active-backup database.  This is an appropriate
> 
> That sentence reads a little funny to me.  Maybe make "database" plural?
> 
> Acked-by: Justin Pettit 

Thanks, I fixed all of that and applied this patch to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 5/8] ovsdb-client: Add new "backup" command.

[Justin Pettit]

> On Dec 13, 2017, at 3:10 PM, Ben Pfaff  wrote:
> 
> diff --git a/ovsdb/ovsdb-client.1.in b/ovsdb/ovsdb-client.1.in
> index 694a7abed46d..2e2df5e5aa7f 100644
> --- a/ovsdb/ovsdb-client.1.in
> +++ b/ovsdb/ovsdb-client.1.in
> @@ -30,6 +30,9 @@ ovsdb\-client \- command-line interface to 
> \fBovsdb-server\fR(1)
> \fBovsdb\-client \fR[\fIoptions\fR] \fBdump\fI \fR[\fIserver\fR] 
> \fR[\fIdatabase\fR]\fR [\fItable\fR
> [\fIcolumn\fR...]]
> .br
> +\fBovsdb\-client \fR[\fIoptions\fR]
> +\fBbackup\fI \fR[\fIserver\fR] \fR[\fIdatabase\fR] > \fIsnapshot\fR

Is the first "\fI" doing anything?  Same question about the third "\fR".  I 
think some of this is a copy/paste from the other prototypes.  I don't think 
they make a visual difference, but those lines are pretty complicated, so it 
might be nice to simplify them if possible.

> @@ -138,6 +141,21 @@ and prints it on stdout as a series of tables. If 
> \fItable\fR is
> specified, only that table is retrieved.  If at least one \fIcolumn\fR
> is specified, only those columns are retrieved.
> .
> +.IP "\fBbackup\fI \fR[\fIserver\fR] \fR[\fIdatabase\fR] > \fIsnapshot\fR"

Same question about simplifying the markup.

> +Connects to \fIserver\fR, retrieves a snapshot of the schema and data
> +in \fIdatabase\fR, and prints it on stdout in the format used for
> +OVSDB standalone and active-backup database.  This is an appropriate

That sentence reads a little funny to me.  Maybe make "database" plural?

Acked-by: Justin Pettit 

--Justin


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 5/8] ovsdb-client: Add new "backup" command.

[Ben Pfaff]

Signed-off-by: Ben Pfaff 
---
 Documentation/ref/ovsdb.7.rst |   4 ++
 NEWS  |   1 +
 ovsdb/file.c  |  22 ---
 ovsdb/file.h  |   2 +
 ovsdb/ovsdb-client.1.in   |  18 ++
 ovsdb/ovsdb-client.c  | 134 ++
 tests/ovsdb-client.at |  51 
 7 files changed, 224 insertions(+), 8 deletions(-)

diff --git a/Documentation/ref/ovsdb.7.rst b/Documentation/ref/ovsdb.7.rst
index 1392ce6f72d7..ddc573a47c94 100644
--- a/Documentation/ref/ovsdb.7.rst
+++ b/Documentation/ref/ovsdb.7.rst
@@ -313,6 +313,10 @@ database file, e.g. using ``cp``, effectively makes a 
snapshot, and because
 OVSDB database files are append-only, it works even if the database is being
 modified when the snapshot takes place.
 
+Another way to make a backup is to use ``ovsdb-client backup``, which
+connects to a running database server and outputs an atomic snapshot of its
+schema and content, in the same format used for on-disk databases.
+
 To restore from a backup, stop the database server or servers, overwrite
 the database file with the backup (e.g. with ``cp``), and then
 restart the servers.
diff --git a/NEWS b/NEWS
index 49d2fa597b97..752e98f073e1 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,7 @@ Post-v2.8.0
  * New file format documentation for developers in ovsdb(5).
  * Protocol documentation moved from ovsdb-server(1) to ovsdb-server(7).
  * ovsdb-client: New "get-schema-cksum" command.
+ * ovsdb-client: New "backup" command.
- OVN:
  * The "requested-chassis" option for a logical switch port now accepts a
chassis "hostname" in addition to a chassis "name".
diff --git a/ovsdb/file.c b/ovsdb/file.c
index 2f07bba3d30c..2986ce08062d 100644
--- a/ovsdb/file.c
+++ b/ovsdb/file.c
@@ -571,6 +571,19 @@ ovsdb_file_change_cb(const struct ovsdb_row *old,
 return true;
 }
 
+struct json *
+ovsdb_file_txn_annotate(struct json *json, const char *comment)
+{
+if (!json) {
+json = json_object_create();
+}
+if (comment) {
+json_object_put_string(json, "_comment", comment);
+}
+json_object_put(json, "_date", json_integer_create(time_wall_msec()));
+return json;
+}
+
 static struct ovsdb_error *
 ovsdb_file_commit(struct ovsdb_replica *replica,
   const struct ovsdb_txn *txn, bool durable)
@@ -843,14 +856,7 @@ ovsdb_file_txn_commit(struct json *json, const char 
*comment,
 {
 struct ovsdb_error *error;
 
-if (!json) {
-json = json_object_create();
-}
-if (comment) {
-json_object_put_string(json, "_comment", comment);
-}
-json_object_put(json, "_date", json_integer_create(time_wall_msec()));
-
+json = ovsdb_file_txn_annotate(json, comment);
 error = ovsdb_log_write(log, json);
 json_destroy(json);
 if (error) {
diff --git a/ovsdb/file.h b/ovsdb/file.h
index ee67b127dd25..c6ca92621044 100644
--- a/ovsdb/file.h
+++ b/ovsdb/file.h
@@ -44,4 +44,6 @@ struct ovsdb_error *ovsdb_file_read_schema(const char 
*file_name,
struct ovsdb_schema **)
 OVS_WARN_UNUSED_RESULT;
 
+struct json *ovsdb_file_txn_annotate(struct json *, const char *comment);
+
 #endif /* ovsdb/file.h */
diff --git a/ovsdb/ovsdb-client.1.in b/ovsdb/ovsdb-client.1.in
index 694a7abed46d..2e2df5e5aa7f 100644
--- a/ovsdb/ovsdb-client.1.in
+++ b/ovsdb/ovsdb-client.1.in
@@ -30,6 +30,9 @@ ovsdb\-client \- command-line interface to 
\fBovsdb-server\fR(1)
 \fBovsdb\-client \fR[\fIoptions\fR] \fBdump\fI \fR[\fIserver\fR] 
\fR[\fIdatabase\fR]\fR [\fItable\fR
 [\fIcolumn\fR...]]
 .br
+\fBovsdb\-client \fR[\fIoptions\fR]
+\fBbackup\fI \fR[\fIserver\fR] \fR[\fIdatabase\fR] > \fIsnapshot\fR
+.br
 \fBovsdb\-client \fR[\fIoptions\fR] \fBmonitor\fI \fR[\fIserver\fR] 
\fR[\fIdatabase\fR] \fItable\fR
 [\fIcolumn\fR[\fB,\fIcolumn\fR]...]...
 .br
@@ -138,6 +141,21 @@ and prints it on stdout as a series of tables. If 
\fItable\fR is
 specified, only that table is retrieved.  If at least one \fIcolumn\fR
 is specified, only those columns are retrieved.
 .
+.IP "\fBbackup\fI \fR[\fIserver\fR] \fR[\fIdatabase\fR] > \fIsnapshot\fR"
+Connects to \fIserver\fR, retrieves a snapshot of the schema and data
+in \fIdatabase\fR, and prints it on stdout in the format used for
+OVSDB standalone and active-backup database.  This is an appropriate
+way to back up a remote database.  The database snapshot that it
+outputs is suitable to be served up directly by \fBovsdb\-server\fR or
+used as the input to \fBovsdb\-client restore\fR.
+.IP
+Another way to back up a standalone or active-backup database is to
+copy its database file, e.g. with \fBcp\fR.  This is safe even if the
+database is in use.
+.IP
+The output does not include ephemeral columns, which by design do not
+survive across restarts of \fBovsdb\-server\fR.
+.
 .IP "\fBmonitor\fI \fR[\fIserver\fR] \fR[\fIdatabase\fR] \fItable\fR