On Thu, Oct 13, 2016 at 9:02 PM, Takashi Menjo
<menjo.takashi@xxxxxxxxxxxxx> wrote:
> This commit lets the sheepdog driver reject forms of filename below.
> This is to avoid unintended deletion of working VDI when a snapshot is
> given as a LUN.
>
> * unix:<path_of_unix_domain_socket>:<vdi>:<tag>
> * unix:<path_of_unix_domain_socket>:<vdi>:<snapid>
> * tcp:<host>:<port>:<vdi>:<tag>
> * tcp:<host>:<port>:<vdi>:<snapid>
>
> In the case that a snapshot is given as a LUN by any of them above, the
> sheepdog driver attempts to delete it then create a new writable VDI when
> some data is being written to it. The deletion request contains name and
> VDI ID of the snapshot. However, sheep daemon uses its name but ignores
> VDI ID and, what is worse, the request contains neither snapshot ID nor
> tag. So sheep daemon receiving the request interprets it as deletion of
> working VDI, not snapshot. If there is working VDI with the same name,
> it will be deleted unintentionally.
>
> I decided not to fix the sheepdog driver to send snapshot ID or tag but
> to let it reject snapshot as a LUN because, even if I fix it, creating
> a new writable VDI will be failed when working VDI exists. If you want
> workaround, use "dog vdi clone" command to create a new writable VDI from
> snapshot then give the new VDI as a LUN.
>
> Cc: Teruaki Ishizaki <ishizaki.teruaki@xxxxxxxxxxxxx>
> Cc: Hitoshi Mitake <mitake.hitoshi@xxxxxxxxxxxxx>
> Signed-off-by: Takashi Menjo <menjo.takashi@xxxxxxxxxxxxx>
> ---
> usr/bs_sheepdog.c | 112 +-----------------------------------------------------
> 1 file changed, 2 insertions(+), 110 deletions(-)
Acked-by: Hitoshi Mitake <mitake.hitoshi@xxxxxxxxxxxxx>
Thanks,
Hitoshi
>
> diff --git a/usr/bs_sheepdog.c b/usr/bs_sheepdog.c
> index fc91ad1..f702c12 100644
> --- a/usr/bs_sheepdog.c
> +++ b/usr/bs_sheepdog.c
> @@ -273,9 +273,6 @@ struct sheepdog_access_info {
> /* unix domain socket */
> char uds_path[UNIX_PATH_MAX];
>
> - /* if the opened VDI is a snapshot, write commands cannot be issued */
> - int is_snapshot;
> -
> /*
> * maximum length of fd_list_head: nr_iothreads + 1
> * (+ 1 is for main thread)
> @@ -1129,8 +1126,6 @@ static int sd_open(struct sheepdog_access_info *ai, char *filename, int flags)
> uint32_t vid = 0;
> char *orig_filename;
>
> - uint32_t snapid = -1;
> - char tag[SD_MAX_VDI_TAG_LEN + 1];
> char vdi_name[SD_MAX_VDI_LEN + 1];
> char *saveptr = NULL, *result;
> enum {
> @@ -1139,11 +1134,9 @@ static int sd_open(struct sheepdog_access_info *ai, char *filename, int flags)
> EXPECT_HOST,
> EXPECT_PORT,
> EXPECT_VDI,
> - EXPECT_TAG_OR_SNAP,
> EXPECT_NOTHING,
> } parse_state = EXPECT_PROTO;
>
> - memset(tag, 0, sizeof(tag));
> memset(vdi_name, 0, sizeof(vdi_name));
>
> orig_filename = strdup(filename);
> @@ -1156,11 +1149,7 @@ static int sd_open(struct sheepdog_access_info *ai, char *filename, int flags)
> * expected form of filename:
> *
> * unix:<path_of_unix_domain_socket>:<vdi>
> - * unix:<path_of_unix_domain_socket>:<vdi>:<tag>
> - * unix:<path_of_unix_domain_socket>:<vdi>:<snapid>
> * tcp:<host>:<port>:<vdi>
> - * tcp:<host>:<port>:<vdi>:<tag>
> - * tcp:<host>:<port>:<vdi>:<snapid>
> */
>
> result = strtok_r(filename, ":", &saveptr);
> @@ -1205,21 +1194,6 @@ static int sd_open(struct sheepdog_access_info *ai, char *filename, int flags)
> break;
> case EXPECT_VDI:
> strncpy(vdi_name, result, SD_MAX_VDI_LEN);
> - parse_state = EXPECT_TAG_OR_SNAP;
> - break;
> - case EXPECT_TAG_OR_SNAP:
> - len = strlen(result);
> - for (i = 0; i < len; i++) {
> - if (!isdigit(result[i])) {
> - /* result is a tag */
> - strncpy(tag, result,
> - SD_MAX_VDI_TAG_LEN);
> - goto trans_to_expect_nothing;
> - }
> - }
> -
> - snapid = atoi(result);
> -trans_to_expect_nothing:
> parse_state = EXPECT_NOTHING;
> break;
> case EXPECT_NOTHING:
> @@ -1235,8 +1209,7 @@ trans_to_expect_nothing:
> }
> } while ((result = strtok_r(NULL, ":", &saveptr)) != NULL);
>
> - if (parse_state != EXPECT_NOTHING &&
> - parse_state != EXPECT_TAG_OR_SNAP) {
> + if (parse_state != EXPECT_NOTHING) {
> eprintf("invalid VDI path of sheepdog: %s (state: %d)\n",
> orig_filename, parse_state);
> ret = -1;
> @@ -1269,15 +1242,8 @@ trans_to_expect_nothing:
>
> close(fd); /* we don't need this connection */
>
> - if (snapid == -1)
> - dprintf("tag: %s\n", tag);
> - else
> - dprintf("snapid: %d\n", snapid);
> -
> 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);
> + ret = find_vdi_name(ai, vdi_name, 0, "", &vid, 0);
> if (ret)
> goto out;
>
> @@ -1326,69 +1292,6 @@ 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, need_reload = 0;
> -
> - ret = pthread_rwlock_wrlock(&ai->inode_lock);
> - if (ret) {
> - eprintf("failed to get inode lock %s\n", strerror(ret));
> - return -1;
> - }
> -
> - if (!ai->is_snapshot)
> - /* check again the snapshot flag to avoid race condition */
> - goto out;
> -
> - memset(&hdr, 0, sizeof(hdr));
> - hdr.opcode = SD_OP_DEL_VDI;
> - hdr.vdi_id = ai->inode.vdi_id;
> - hdr.flags = SD_FLAG_CMD_WRITE;
> - wlen = SD_MAX_VDI_LEN;
> - rlen = 0;
> - 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");
> - goto out;
> - }
> -
> - 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;
> - rlen = 0;
> - 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");
> - goto out;
> - }
> -
> - ret = read_object(ai, (char *)&ai->inode, vid_to_vdi_oid(rsp->vdi_id),
> - ai->inode.nr_copies, SD_INODE_SIZE, 0, &need_reload);
> - if (ret) {
> - eprintf("reloading new inode object failed");
> - goto out;
> - }
> -
> - ai->is_snapshot = 0;
> - dprintf("creating branch from snapshot, new VDI ID: %x\n", rsp->vdi_id);
> -out:
> - pthread_rwlock_unlock(&ai->inode_lock);
> -
> - return ret;
> -}
> -
> struct inflight_thread {
> unsigned long min_idx, max_idx;
> struct list_head list;
> @@ -1453,17 +1356,6 @@ static void bs_sheepdog_request(struct scsi_cmd *cmd)
> case WRITE_10:
> case WRITE_12:
> case WRITE_16:
> - if (ai->is_snapshot) {
> - ret = create_branch(ai);
> - if (ret) {
> - eprintf("creating writable VDI from"\
> - " snapshot failed\n");
> - set_medium_error(&result, &key, &asc);
> -
> - break;
> - }
> - }
> -
> length = scsi_get_out_length(cmd);
>
> myself.min_idx = cmd->offset / object_size;
> --
> 2.7.4
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe stgt" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
