[PATCH v3 0/3] various improvements of bs

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v3 0/3] various improvements of bs_sheepdog.c
@ 2013-10-17  7:38 Hitoshi Mitake
  2013-10-17  7:38 ` [PATCH v3 1/3] bs_sheepdog.c: mock the behavior of QEMU's sheepdog driver in a correct manner Hitoshi Mitake
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Hitoshi Mitake @ 2013-10-17  7:38 UTC (permalink / raw)
  To: stgt; +Cc: sheepdog, Hitoshi Mitake

Hi tgt list and sheepdog list,

This patchset improves bs_sheepdog.c of tgtd.
1. correcting a treatment of snapshot VDIs
2. removing segfault which can happen during logical unit deletion
3. validating address of sheep process before logical unit creation

Please consider applying them.

Thanks,
Hitoshi

v3: return error when --backing-store is invalid form (the 3rd patch)

Hitoshi Mitake (3):
  bs_sheepdog.c: mock the behavior of QEMU's sheepdog driver in a
    correct manner
  bs_sheepdog.c: fix segmentation fault caused by deleting logical unit
  bs_sheepdog.c: test connection when new logical unit is created

 usr/bs_sheepdog.c |  139 +++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 109 insertions(+), 30 deletions(-)

-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/3] bs_sheepdog.c: mock the behavior of QEMU's sheepdog driver in a correct manner
  2013-10-17  7:38 [PATCH v3 0/3] various improvements of bs_sheepdog.c Hitoshi Mitake
@ 2013-10-17  7:38 ` Hitoshi Mitake
  2013-10-17  7:38 ` [PATCH v3 2/3] bs_sheepdog.c: fix segmentation fault caused by deleting logical unit Hitoshi Mitake
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Hitoshi Mitake @ 2013-10-17  7:38 UTC (permalink / raw)
  To: stgt; +Cc: sheepdog, Hitoshi Mitake

Current bs_sheepdog.c doesn't mock the behavior of QEMU's sheepdog
driver in a correct manner when it opens snapshot VDIs. It treats the
snapshot VDIs as read-only disks, but this behavior is different from
the QEMU's driver. This patch lets bs_sheepdog.c mock the original
behavior (delete an original VDI of the snapshot -> create a new non
snapshot VDI based on the snapshot and open it). This patch also
removes confusing notations of --backing-store option of
bs_sheepdog.c.

Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>
---
 usr/bs_sheepdog.c |  109 +++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 76 insertions(+), 33 deletions(-)

diff --git a/usr/bs_sheepdog.c b/usr/bs_sheepdog.c
index 4f7cde0..11ddce6 100644
--- a/usr/bs_sheepdog.c
+++ b/usr/bs_sheepdog.c
@@ -622,8 +622,11 @@ static int reload_inode(struct sheepdog_access_info *ai)
 	if (ret)
 		return -1;
 
-	read_object(ai, (char *)&ai->inode, vid_to_vdi_oid(vid),
-		    ai->inode.nr_copies, SD_INODE_SIZE, 0);
+	ret = read_object(ai, (char *)&ai->inode, vid_to_vdi_oid(vid),
+			  ai->inode.nr_copies, SD_INODE_SIZE, 0);
+	if (ret)
+		return -1;
+
 	return 0;
 }
 
@@ -917,9 +920,9 @@ static int sd_open(struct sheepdog_access_info *ai, char *filename, int flags)
 	enum {
 		EXPECT_PROTO,
 		EXPECT_PATH,
-		EXPECT_VDI,
-		EXPECT_HOST_OR_PORT,
+		EXPECT_HOST,
 		EXPECT_PORT,
+		EXPECT_VDI,
 		EXPECT_TAG_OR_SNAP,
 		EXPECT_NOTHING,
 	} parse_state = EXPECT_PROTO;
@@ -942,9 +945,6 @@ static int sd_open(struct sheepdog_access_info *ai, char *filename, int flags)
 	 * tcp:<host>:<port>:<vdi>
 	 * tcp:<host>:<port>:<vdi>:<tag>
 	 * tcp:<host>:<port>:<vdi>:<snapid>
-	 * tcp:<port>:<vdi>
-	 * tcp:<port>:<vdi>:<tag>
-	 * tcp:<port>:<vdi>:<snapid>
 	 */
 
 	result = strtok_r(filename, ":", &saveptr);
@@ -957,7 +957,7 @@ static int sd_open(struct sheepdog_access_info *ai, char *filename, int flags)
 				parse_state = EXPECT_PATH;
 			} else if (!strcmp("tcp", result)) {
 				ai->is_unix = 0;
-				parse_state = EXPECT_HOST_OR_PORT;
+				parse_state = EXPECT_HOST;
 			} else {
 				eprintf("unknown protocol of sheepdog vdi:"\
 					" %s\n", result);
@@ -968,26 +968,14 @@ static int sd_open(struct sheepdog_access_info *ai, char *filename, int flags)
 			strncpy(ai->uds_path, result, UNIX_PATH_MAX);
 			parse_state = EXPECT_VDI;
 			break;
-		case EXPECT_HOST_OR_PORT:
-			len = strlen(result);
-			for (i = 0; i < len; i++) {
-				if (!isdigit(result[i])) {
-					/* result is a hostname */
-					strncpy(ai->hostname, result,
-						HOST_NAME_MAX);
-					parse_state = EXPECT_PORT;
-					goto next_token;
-				}
-			}
-
-			/* result is a port, use localhost as hostname */
-			strncpy(ai->hostname, "localhost", strlen("localhost"));
-set_port:
+		case EXPECT_HOST:
+			strncpy(ai->hostname, result, HOST_NAME_MAX);
+			parse_state = EXPECT_PORT;
+			break;
+		case EXPECT_PORT:
 			ai->port = atoi(result);
 			parse_state = EXPECT_VDI;
 			break;
-		case EXPECT_PORT:
-			goto set_port;
 		case EXPECT_VDI:
 			strncpy(vdi_name, result, SD_MAX_VDI_LEN);
 			parse_state = EXPECT_TAG_OR_SNAP;
@@ -1017,8 +1005,6 @@ trans_to_expect_nothing:
 				parse_state);
 			exit(1);
 		}
-
-next_token:;
 	} while ((result = strtok_r(NULL, ":", &saveptr)) != NULL);
 
 	if (parse_state != EXPECT_NOTHING &&
@@ -1039,7 +1025,8 @@ next_token:;
 	else
 		dprintf("snapid: %d\n", snapid);
 
-	ai->is_snapshot = !(snapid == -1) && strlen(tag);
+	dprintf("VDI name: %s\n", vdi_name);
+	ai->is_snapshot = !(snapid == -1) || strlen(tag);
 	ret = find_vdi_name(ai, vdi_name, snapid == -1 ? 0 : snapid, tag, &vid,
 			    ai->is_snapshot);
 	if (ret)
@@ -1087,6 +1074,53 @@ static void set_medium_error(int *result, uint8_t *key, uint16_t *asc)
 	*asc = ASC_READ_ERROR;
 }
 
+static int create_branch(struct sheepdog_access_info *ai)
+{
+	struct sheepdog_vdi_req hdr;
+	struct sheepdog_vdi_rsp *rsp = (struct sheepdog_vdi_rsp *)&hdr;
+	unsigned int wlen = 0, rlen;
+	int ret;
+
+	hdr.opcode = SD_OP_DEL_VDI;
+	hdr.vdi_id = ai->inode.vdi_id;
+	hdr.flags = SD_FLAG_CMD_WRITE;
+	wlen = SD_MAX_VDI_LEN;
+	hdr.data_length = wlen;
+
+	ret = do_req(ai, (struct sheepdog_req *)&hdr, ai->inode.name,
+		     &wlen, &rlen);
+	if (ret) {
+		eprintf("deleting snapshot VDI for creating branch failed\n");
+		return -1;
+	}
+
+	memset(&hdr, 0, sizeof(hdr));
+	hdr.opcode = SD_OP_NEW_VDI;
+	hdr.vdi_id = ai->inode.vdi_id;
+
+	hdr.flags = SD_FLAG_CMD_WRITE;
+	wlen = SD_MAX_VDI_LEN;
+	hdr.data_length = wlen;
+	hdr.vdi_size = ai->inode.vdi_size;
+	ret = do_req(ai, (struct sheepdog_req *)&hdr, ai->inode.name,
+		     &wlen, &rlen);
+	if (ret) {
+		eprintf("creating new VDI for creating branch failed\n");
+		return -1;
+	}
+
+	ret = read_object(ai, (char *)&ai->inode, vid_to_vdi_oid(rsp->vdi_id),
+			  ai->inode.nr_copies, SD_INODE_SIZE, 0);
+	if (ret) {
+		eprintf("reloading new inode object failed");
+		return -1;
+	}
+
+	dprintf("creating branch from snapshot, new VDI ID: %x\n", rsp->vdi_id);
+
+	return 0;
+}
+
 static void bs_sheepdog_request(struct scsi_cmd *cmd)
 {
 	int ret = 0;
@@ -1110,11 +1144,20 @@ static void bs_sheepdog_request(struct scsi_cmd *cmd)
 	case WRITE_12:
 	case WRITE_16:
 		if (ai->is_snapshot) {
-			length = scsi_get_out_length(cmd);
-			ret = sd_io(ai, 1, scsi_get_out_buffer(cmd),
-				    length, cmd->offset);
-		} else
-			ret = -1;
+			ret = create_branch(ai);
+			if (ret) {
+				eprintf("creating writable VDI from"\
+					" snapshot failed\n");
+				set_medium_error(&result, &key, &asc);
+
+				return;
+			}
+			ai->is_snapshot = 0;
+		}
+
+		length = scsi_get_out_length(cmd);
+		ret = sd_io(ai, 1, scsi_get_out_buffer(cmd),
+			    length, cmd->offset);
 
 		if (ret)
 			set_medium_error(&result, &key, &asc);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/3] bs_sheepdog.c: fix segmentation fault caused by deleting logical unit
  2013-10-17  7:38 [PATCH v3 0/3] various improvements of bs_sheepdog.c Hitoshi Mitake
  2013-10-17  7:38 ` [PATCH v3 1/3] bs_sheepdog.c: mock the behavior of QEMU's sheepdog driver in a correct manner Hitoshi Mitake
@ 2013-10-17  7:38 ` Hitoshi Mitake
  2013-10-17  7:38 ` [PATCH v3 3/3] bs_sheepdog.c: test connection when new logical unit is created Hitoshi Mitake
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Hitoshi Mitake @ 2013-10-17  7:38 UTC (permalink / raw)
  To: stgt; +Cc: sheepdog, Hitoshi Mitake

This patch removes a bug in bs_exit() of bs_sheepdog.c which caused
segmentation fault.

Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>
---
 usr/bs_sheepdog.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/usr/bs_sheepdog.c b/usr/bs_sheepdog.c
index 11ddce6..de66d59 100644
--- a/usr/bs_sheepdog.c
+++ b/usr/bs_sheepdog.c
@@ -1233,15 +1233,20 @@ static void bs_sheepdog_exit(struct scsi_lu *lu)
 	struct sheepdog_access_info *ai =
 		(struct sheepdog_access_info *)(info + 1);
 
-	struct sheepdog_fd_list *p;
+	struct sheepdog_fd_list *p, *next;
 
 	bs_thread_close(info);
 
-	list_for_each_entry(p, &ai->fd_list_head, list) {
+	list_for_each_entry_safe(p, next, &ai->fd_list_head, list) {
 		close(p->fd);
 		list_del(&p->list);
 		free(p);
 	}
+
+	pthread_rwlock_destroy(&ai->fd_list_lock);
+	pthread_rwlock_destroy(&ai->inode_lock);
+
+	dprintf("cleaned logical unit %p safely\n", lu);
 }
 
 static struct backingstore_template sheepdog_bst = {
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 3/3] bs_sheepdog.c: test connection when new logical unit is created
  2013-10-17  7:38 [PATCH v3 0/3] various improvements of bs_sheepdog.c Hitoshi Mitake
  2013-10-17  7:38 ` [PATCH v3 1/3] bs_sheepdog.c: mock the behavior of QEMU's sheepdog driver in a correct manner Hitoshi Mitake
  2013-10-17  7:38 ` [PATCH v3 2/3] bs_sheepdog.c: fix segmentation fault caused by deleting logical unit Hitoshi Mitake
@ 2013-10-17  7:38 ` Hitoshi Mitake
  2013-10-20 12:54 ` [sheepdog] [PATCH v3 0/3] various improvements of bs_sheepdog.c Hitoshi Mitake
  2013-10-21  1:27 ` FUJITA Tomonori
  4 siblings, 0 replies; 6+ messages in thread
From: Hitoshi Mitake @ 2013-10-17  7:38 UTC (permalink / raw)
  To: stgt; +Cc: sheepdog, Hitoshi Mitake

This patch adds creation of a test connection in the process of making
logical unit. This is required for validating --backing-store option
and avoid forever trying of reconnection.

Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>
---

v3: return error when --backing-store is invalid form

 usr/bs_sheepdog.c |   33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/usr/bs_sheepdog.c b/usr/bs_sheepdog.c
index de66d59..1eb09c0 100644
--- a/usr/bs_sheepdog.c
+++ b/usr/bs_sheepdog.c
@@ -909,7 +909,7 @@ out:
 
 static int sd_open(struct sheepdog_access_info *ai, char *filename, int flags)
 {
-	int ret = 0, i, len;
+	int ret = 0, i, len, fd;
 	uint32_t vid = 0;
 	char *orig_filename;
 
@@ -973,6 +973,15 @@ static int sd_open(struct sheepdog_access_info *ai, char *filename, int flags)
 			parse_state = EXPECT_PORT;
 			break;
 		case EXPECT_PORT:
+			len = strlen(result);
+			for (i = 0; i < len; i++) {
+				if (!isdigit(result[i])) {
+					eprintf("invalid tcp port number:"\
+						" %s\n", result);
+					return -1;
+				}
+			}
+
 			ai->port = atoi(result);
 			parse_state = EXPECT_VDI;
 			break;
@@ -999,6 +1008,7 @@ trans_to_expect_nothing:
 			eprintf("invalid VDI path of sheepdog, unexpected"\
 				" token: %s (entire: %s)\n",
 				result, orig_filename);
+			ret = -1;
 			goto out;
 		default:
 			eprintf("BUG: invalid state of parser: %d\n",
@@ -1011,6 +1021,7 @@ trans_to_expect_nothing:
 	    parse_state != EXPECT_TAG_OR_SNAP) {
 		eprintf("invalid VDI path of sheepdog: %s (state: %d)\n",
 			orig_filename, parse_state);
+		ret = -1;
 		goto out;
 	}
 
@@ -1020,6 +1031,26 @@ trans_to_expect_nothing:
 	else
 		dprintf("hostname: %s, port: %d\n", ai->hostname, ai->port);
 
+	/*
+	 * test connection for validating command line option
+	 *
+	 * if this step is skipped, the main thread of tgtd will try to
+	 * reconnect to sheep process forever
+	 */
+	fd = ai->is_unix ?
+		connect_to_sdog_unix(ai->uds_path) :
+		connect_to_sdog_tcp(ai->hostname, ai->port);
+
+	if (fd < 0) {
+		eprintf("connecting to sheep process failed, "\
+			"please verify the --backing-store option: %s",
+			orig_filename);
+		ret = -1;
+		goto out;
+	}
+
+	close(fd);		/* we don't need this connection */
+
 	if (snapid == -1)
 		dprintf("tag: %s\n", tag);
 	else
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [sheepdog] [PATCH v3 0/3] various improvements of bs_sheepdog.c
  2013-10-17  7:38 [PATCH v3 0/3] various improvements of bs_sheepdog.c Hitoshi Mitake
                   ` (2 preceding siblings ...)
  2013-10-17  7:38 ` [PATCH v3 3/3] bs_sheepdog.c: test connection when new logical unit is created Hitoshi Mitake
@ 2013-10-20 12:54 ` Hitoshi Mitake
  2013-10-21  1:27 ` FUJITA Tomonori
  4 siblings, 0 replies; 6+ messages in thread
From: Hitoshi Mitake @ 2013-10-20 12:54 UTC (permalink / raw)
  To: Hitoshi Mitake; +Cc: stgt, sheepdog

At Thu, 17 Oct 2013 16:38:31 +0900,
Hitoshi Mitake wrote:
> 
> Hi tgt list and sheepdog list,
> 
> This patchset improves bs_sheepdog.c of tgtd.
> 1. correcting a treatment of snapshot VDIs
> 2. removing segfault which can happen during logical unit deletion
> 3. validating address of sheep process before logical unit creation
> 
> Please consider applying them.
> 
> Thanks,
> Hitoshi
> 
> v3: return error when --backing-store is invalid form (the 3rd patch)
> 
> Hitoshi Mitake (3):
>   bs_sheepdog.c: mock the behavior of QEMU's sheepdog driver in a
>     correct manner
>   bs_sheepdog.c: fix segmentation fault caused by deleting logical unit
>   bs_sheepdog.c: test connection when new logical unit is created
> 
>  usr/bs_sheepdog.c |  139 +++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 109 insertions(+), 30 deletions(-)
> 

ping?

Thanks,
Hitoshi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 0/3] various improvements of bs_sheepdog.c
  2013-10-17  7:38 [PATCH v3 0/3] various improvements of bs_sheepdog.c Hitoshi Mitake
                   ` (3 preceding siblings ...)
  2013-10-20 12:54 ` [sheepdog] [PATCH v3 0/3] various improvements of bs_sheepdog.c Hitoshi Mitake
@ 2013-10-21  1:27 ` FUJITA Tomonori
  4 siblings, 0 replies; 6+ messages in thread
From: FUJITA Tomonori @ 2013-10-21  1:27 UTC (permalink / raw)
  To: mitake.hitoshi; +Cc: stgt, sheepdog

On Thu, 17 Oct 2013 16:38:31 +0900
Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp> wrote:

> Hi tgt list and sheepdog list,
> 
> This patchset improves bs_sheepdog.c of tgtd.
> 1. correcting a treatment of snapshot VDIs
> 2. removing segfault which can happen during logical unit deletion
> 3. validating address of sheep process before logical unit creation
> 
> Please consider applying them.
> 
> Thanks,
> Hitoshi
> 
> v3: return error when --backing-store is invalid form (the 3rd patch)
> 
> Hitoshi Mitake (3):
>   bs_sheepdog.c: mock the behavior of QEMU's sheepdog driver in a
>     correct manner
>   bs_sheepdog.c: fix segmentation fault caused by deleting logical unit
>   bs_sheepdog.c: test connection when new logical unit is created
> 
>  usr/bs_sheepdog.c |  139 +++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 109 insertions(+), 30 deletions(-)

Applied.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-10-21  1:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-17  7:38 [PATCH v3 0/3] various improvements of bs_sheepdog.c Hitoshi Mitake
2013-10-17  7:38 ` [PATCH v3 1/3] bs_sheepdog.c: mock the behavior of QEMU's sheepdog driver in a correct manner Hitoshi Mitake
2013-10-17  7:38 ` [PATCH v3 2/3] bs_sheepdog.c: fix segmentation fault caused by deleting logical unit Hitoshi Mitake
2013-10-17  7:38 ` [PATCH v3 3/3] bs_sheepdog.c: test connection when new logical unit is created Hitoshi Mitake
2013-10-20 12:54 ` [sheepdog] [PATCH v3 0/3] various improvements of bs_sheepdog.c Hitoshi Mitake
2013-10-21  1:27 ` FUJITA Tomonori

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.