Patchwork [V2,05/12] snapshot: add paired functions for internal snapshot id and name

login
register
mail settings
Submitter Wayne Xia
Date June 14, 2013, 11:39 a.m.
Message ID <1371209999-15579-6-git-send-email-xiawenc@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/251391/
State New
Headers show

Comments

Wayne Xia - June 14, 2013, 11:39 a.m.
Internal snapshot's ID and name concept are both visible in general
block level, they are observed by user in "info snapshots", so it is
possible to mess them up. Although we can separate the two concept in
programming, but if they can be distinguished in string itself, things
will be simple and clear, so introduce two functions to do it.

The implemention, qcow2 snapshot calls snapshot_id_string_generate() to
make sure it follows the rule in driver. If caller or user give a check
with snapshot_name_wellformed() before create snapshot, then the ID
and name will never mess up. The check can be also taken in
qcow2_snapshot_create(), but require it to return error reason.

For rbd, it have no ID, so have no impact.
For sheepdog, ID can't be determined in qemu, so still can't guarantee
that no more mess up for ID and name.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block/qcow2-snapshot.c   |    2 +-
 block/snapshot.c         |   21 +++++++++++++++++++++
 include/block/snapshot.h |    3 +++
 3 files changed, 25 insertions(+), 1 deletions(-)

Patch

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 992a5c8..7108d46 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -284,7 +284,7 @@  static void find_new_snapshot_id(BlockDriverState *bs,
         if (id > id_max)
             id_max = id;
     }
-    snprintf(id_str, id_str_size, "%d", id_max + 1);
+    snapshot_id_string_generate(id_max + 1, id_str, id_str_size);
 }
 
 static int find_snapshot_by_id(BlockDriverState *bs, const char *id_str)
diff --git a/block/snapshot.c b/block/snapshot.c
index 481a3ee..40e0cc7 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -228,3 +228,24 @@  int bdrv_snapshot_load_tmp(BlockDriverState *bs,
     }
     return -ENOTSUP;
 }
+
+/*
+ * Return true if the given internal snapshot name is valid, false
+ * otherwise.
+ *
+ * To prevent clashes with internal snapshot IDs, names consisting only
+ * of digits are rejected.  Empty strings are also rejected.
+ */
+bool snapshot_name_wellformed(const char *name)
+{
+    return strspn(name, "0123456789") != strlen(name);
+}
+
+/*
+ * Following function generate id string, used by block driver, such as qcow2.
+ * Since no better place to go, place the funtion here for now.
+ */
+void snapshot_id_string_generate(int id, char *id_str, int id_str_size)
+{
+    snprintf(id_str, id_str_size, "%d", id);
+}
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 9d06dc1..3d93719 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -56,4 +56,7 @@  int bdrv_snapshot_list(BlockDriverState *bs,
                        QEMUSnapshotInfo **psn_info);
 int bdrv_snapshot_load_tmp(BlockDriverState *bs,
                            const char *snapshot_name);
+
+bool snapshot_name_wellformed(const char *name);
+void snapshot_id_string_generate(int id, char *id_str, int id_str_size);
 #endif