[sheepdog] [PATCH] sheep: don't update vdi_iocb in vdi_* functions

MORITA Kazutaka morita.kazutaka at gmail.com
Fri Aug 30 10:07:06 CEST 2013


From: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>

We are using vdi_iocb to get a snapid from a vdi object, but it looks
like abusing of vdi_iocb.  This patch makes how to use vdi_iocb and
vdi_info clear like as follows.

 - vdi_iocb is used to pass parameters from clients.
 - vdi_info is used to get info from sheepdog.

To prevent from abusing them again, this patch makes some vdi_iocb
variables constant where possible.

Signed-off-by: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
---
 sheep/sheep_priv.h |    9 ++++++---
 sheep/vdi.c        |   32 ++++++++++++++++----------------
 2 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/sheep/sheep_priv.h b/sheep/sheep_priv.h
index a3206be..52b6b05 100644
--- a/sheep/sheep_priv.h
+++ b/sheep/sheep_priv.h
@@ -164,6 +164,7 @@ struct siocb {
 	uint64_t offset;
 };
 
+/* This structure is used to pass parameters to vdi_* functions. */
 struct vdi_iocb {
 	const char *name;
 	const char *tag;
@@ -175,8 +176,10 @@ struct vdi_iocb {
 	int nr_copies;
 };
 
+/* This structure is used to get information from sheepdog. */
 struct vdi_info {
 	uint32_t vid;
+	uint32_t snapid;
 	uint32_t free_bit;
 	uint64_t create_time;
 };
@@ -278,9 +281,9 @@ int get_max_copy_number(void);
 int get_req_copy_number(struct request *req);
 int add_vdi_state(uint32_t vid, int nr_copies, bool snapshot);
 int vdi_exist(uint32_t vid);
-int vdi_create(struct vdi_iocb *iocb, uint32_t *new_vid);
-int vdi_delete(struct vdi_iocb *iocb, struct request *req);
-int vdi_lookup(struct vdi_iocb *iocb, struct vdi_info *info);
+int vdi_create(const struct vdi_iocb *iocb, uint32_t *new_vid);
+int vdi_delete(const struct vdi_iocb *iocb, struct request *req);
+int vdi_lookup(const struct vdi_iocb *iocb, struct vdi_info *info);
 void clean_vdi_state(void);
 
 int read_vdis(char *data, int len, unsigned int *rsp_len);
diff --git a/sheep/vdi.c b/sheep/vdi.c
index d5df668..532db93 100644
--- a/sheep/vdi.c
+++ b/sheep/vdi.c
@@ -188,8 +188,8 @@ out:
 }
 
 /* TODO: should be performed atomically */
-static int create_vdi_obj(struct vdi_iocb *iocb, uint32_t new_vid,
-			  uint32_t cur_vid)
+static int create_vdi_obj(const struct vdi_iocb *iocb, uint32_t new_snapid,
+			  uint32_t new_vid, uint32_t cur_vid)
 {
 	/* we are not called concurrently */
 	struct sd_inode *new = NULL, *base = NULL, *cur = NULL;
@@ -241,7 +241,7 @@ static int create_vdi_obj(struct vdi_iocb *iocb, uint32_t new_vid,
 	new->copy_policy = 0;
 	new->nr_copies = iocb->nr_copies;
 	new->block_size_shift = find_next_bit(&block_size, BITS_PER_LONG, 0);
-	new->snap_id = iocb->snapid;
+	new->snap_id = new_snapid;
 
 	if (iocb->base_vid) {
 		int i;
@@ -322,15 +322,15 @@ static int get_vdi_bitmap_range(const char *name, unsigned long *left,
 	return SD_RES_SUCCESS;
 }
 
-static inline bool vdi_has_tag(struct vdi_iocb *iocb)
+static inline bool vdi_has_tag(const struct vdi_iocb *iocb)
 {
 	if ((iocb->tag && iocb->tag[0]) || iocb->snapid)
 		return true;
 	return false;
 }
 
-static inline bool vdi_tag_match(struct vdi_iocb *iocb,
-			     struct sd_inode *inode)
+static inline bool vdi_tag_match(const struct vdi_iocb *iocb,
+				 const struct sd_inode *inode)
 {
 	const char *tag = iocb->tag;
 
@@ -342,7 +342,8 @@ static inline bool vdi_tag_match(struct vdi_iocb *iocb,
 }
 
 static int fill_vdi_info_range(uint32_t left, uint32_t right,
-			      struct vdi_iocb *iocb, struct vdi_info *info)
+			       const struct vdi_iocb *iocb,
+			       struct vdi_info *info)
 {
 	struct sd_inode *inode;
 	bool vdi_found = false;
@@ -385,7 +386,7 @@ static int fill_vdi_info_range(uint32_t left, uint32_t right,
 				 * Rollback & snap create, read, delete on
 				 * current working VDI
 				 */
-				iocb->snapid = inode->snap_id + 1;
+				info->snapid = inode->snap_id + 1;
 				if (vdi_is_snapshot(inode))
 					/* Current workding VDI is deleted */
 					break;
@@ -403,7 +404,7 @@ out:
 
 /* Fill the VDI information from right to left in the bitmap */
 static int fill_vdi_info(unsigned long left, unsigned long right,
-			 struct vdi_iocb *iocb, struct vdi_info *info)
+			 const struct vdi_iocb *iocb, struct vdi_info *info)
 {
 	int ret;
 
@@ -423,12 +424,11 @@ static int fill_vdi_info(unsigned long left, unsigned long right,
 }
 
 /* Return SUCCESS if we find targeted VDI specified by iocb and fill info */
-int vdi_lookup(struct vdi_iocb *iocb, struct vdi_info *info)
+int vdi_lookup(const struct vdi_iocb *iocb, struct vdi_info *info)
 {
 	unsigned long left, right;
 	int ret;
 
-
 	ret = get_vdi_bitmap_range(iocb->name, &left, &right);
 	info->free_bit = right;
 	sd_debug("%s left %lx right %lx, %x", iocb->name, left, right, ret);
@@ -484,7 +484,7 @@ static void vdi_flush(uint32_t vid)
  * Fresh create started with id = 1. Both rollback & snap create started with
  * current working VDI's snap_id + 1. Working VDI always has the highest snapid.
  */
-int vdi_create(struct vdi_iocb *iocb, uint32_t *new_vid)
+int vdi_create(const struct vdi_iocb *iocb, uint32_t *new_vid)
 {
 	const char *name = iocb->name;
 	struct vdi_info info = {};
@@ -506,8 +506,8 @@ int vdi_create(struct vdi_iocb *iocb, uint32_t *new_vid)
 		sd_err("%s", sd_strerror(ret));
 		return ret;
 	}
-	if (!iocb->snapid)
-		iocb->snapid = 1;
+	if (info.snapid == 0)
+		info.snapid = 1;
 	*new_vid = info.free_bit;
 	ret = notify_vdi_add(*new_vid, iocb->nr_copies, info.vid);
 	if (ret != SD_RES_SUCCESS)
@@ -519,12 +519,12 @@ int vdi_create(struct vdi_iocb *iocb, uint32_t *new_vid)
 		 *new_vid, iocb->base_vid, info.vid, iocb->nr_copies,
 		 iocb->snapid);
 
-	return create_vdi_obj(iocb, *new_vid, info.vid);
+	return create_vdi_obj(iocb, info.snapid, *new_vid, info.vid);
 }
 
 static int start_deletion(struct request *req, uint32_t vid);
 
-int vdi_delete(struct vdi_iocb *iocb, struct request *req)
+int vdi_delete(const struct vdi_iocb *iocb, struct request *req)
 {
 	struct vdi_info info;
 	int ret;
-- 
1.7.9.5




More information about the sheepdog mailing list