[sheepdog] [PATCH] dog: remove alter subcommand and migrate sub-sub commands

Hitoshi Mitake mitake.hitoshi at lab.ntt.co.jp
Tue Jul 22 09:49:05 CEST 2014


At Tue, 22 Jul 2014 16:32:06 +0900,
Hitoshi Mitake wrote:
> 
> The name of subcommand "dog alter" is strange and confusing. Because
> many other commands of dog alter state of sheep cluster. This patch
> removes the subcommand and move sub-sub commands as below:
> - dog alter vdi-copy -> dog vdi alter-copy
> - dog alter cluster-copy -> dog cluster alter-copy
> 
> Cc: Ruoyu <liangry at ucweb.com>
> Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> ---
>  dog/Makefile.am |   3 +-
>  dog/alter.c     | 253 --------------------------------------------------------
>  dog/cluster.c   |  83 +++++++++++++++++++
>  dog/dog.c       |   1 -
>  dog/vdi.c       | 116 ++++++++++++++++++++++++++
>  5 files changed, 200 insertions(+), 256 deletions(-)
>  delete mode 100644 dog/alter.c

Ruoyu, could you review this patch? If it looks good to you, I'll push
it to master.

Thanks,
Hitoshi

> 
> diff --git a/dog/Makefile.am b/dog/Makefile.am
> index bd86452..a7ead61 100644
> --- a/dog/Makefile.am
> +++ b/dog/Makefile.am
> @@ -25,8 +25,7 @@ sbin_PROGRAMS		= dog
>  
>  dog_SOURCES		= farm/object_tree.c farm/sha1_file.c farm/snap.c \
>  			  farm/trunk.c farm/farm.c farm/slice.c \
> -			  dog.c common.c treeview.c vdi.c node.c cluster.c \
> -			  alter.c
> +			  dog.c common.c treeview.c vdi.c node.c cluster.c
>  
>  if BUILD_TRACE
>  dog_SOURCES		+= trace.c
> diff --git a/dog/alter.c b/dog/alter.c
> deleted file mode 100644
> index 432f01b..0000000
> --- a/dog/alter.c
> +++ /dev/null
> @@ -1,253 +0,0 @@
> -/*
> - * Copyright (C) 2011 Nippon Telegraph and Telephone Corporation.
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License version
> - * 2 as published by the Free Software Foundation.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program. If not, see <http://www.gnu.org/licenses/>.
> - */
> -
> -#include <time.h>
> -#include <string.h>
> -#include <ctype.h>
> -#include <sys/time.h>
> -
> -#include "dog.h"
> -#include "treeview.h"
> -
> -static struct sd_option alter_options[] = {
> -	{'c', "copies", true, "specify the data redundancy level"},
> -	{ 0, NULL, false, NULL },
> -};
> -
> -static struct alter_cmd_data {
> -	uint8_t copies;
> -	uint8_t copy_policy;
> -} alter_cmd_data;
> -
> -#define ALTER_CLUSTER_COPY_PRINT				\
> -	"    __\n"				\
> -	"   ()'`;\n"				\
> -	"   /\\|`  Caution! Changing cluster's redundancy level will affect\n" \
> -	"  /  |   all the VDIs to be created later.\n" \
> -	"(/_)_|_  Are you sure you want to continue? [yes/no]: "
> -
> -static int alter_cluster_copy(int argc, char **argv)
> -{
> -	int ret, log_length;
> -	struct sd_req hdr;
> -	struct sd_rsp *rsp = (struct sd_rsp *)&hdr;
> -	struct epoch_log *logs;
> -
> -	if (alter_cmd_data.copy_policy != 0) {
> -		sd_err("Changing redundancy level of erasure coded vdi "
> -			   "is not supported yet.");
> -		return EXIT_USAGE;
> -	}
> -	if (!alter_cmd_data.copies) {
> -		alter_cmd_data.copies = SD_DEFAULT_COPIES;
> -		printf("The cluster's redundancy level is not specified, "
> -			   "use %d as default.\n", SD_DEFAULT_COPIES);
> -	}
> -
> -	if (alter_cmd_data.copies > sd_nodes_nr) {
> -		char info[1024];
> -		snprintf(info, sizeof(info), "Number of copies (%d) is larger "
> -			 "than number of nodes (%d).\n"
> -			 "Are you sure you want to continue? [yes/no]: ",
> -			 alter_cmd_data.copies, sd_nodes_nr);
> -		confirm(info);
> -	}
> -
> -	log_length = sd_epoch * sizeof(struct epoch_log);
> -	logs = xmalloc(log_length);
> -	sd_init_req(&hdr, SD_OP_STAT_CLUSTER);
> -	hdr.data_length = log_length;
> -	ret = dog_exec_req(&sd_nid, &hdr, logs);
> -	if (ret < 0)
> -		goto failure;
> -
> -	if (rsp->result != SD_RES_SUCCESS) {
> -		sd_err("Response's result: %s", sd_strerror(rsp->result));
> -		goto failure;
> -	}
> -	if (logs->copy_policy) {
> -		sd_err("The cluster's copy policy is erasure code, "
> -			   "changing it is not supported yet.");
> -		goto failure;
> -	}
> -	if (logs->nr_copies == alter_cmd_data.copies) {
> -		sd_err("The cluster's redundancy level is already set to %d, "
> -			   "nothing changed.", alter_cmd_data.copies);
> -		goto failure;
> -	}
> -
> -	confirm(ALTER_CLUSTER_COPY_PRINT);
> -
> -	sd_init_req(&hdr, SD_OP_ALTER_CLUSTER_COPY);
> -	hdr.cluster.copies = alter_cmd_data.copies;
> -	hdr.cluster.copy_policy = alter_cmd_data.copy_policy;
> -	ret = send_light_req(&sd_nid, &hdr);
> -	if (ret == 0) {
> -		sd_info("The cluster's redundancy level is set to %d, "
> -				"the old one was %d.",
> -				alter_cmd_data.copies, logs->nr_copies);
> -		goto success;
> -	} else {
> -		sd_err("Changing the cluster's redundancy level failure.");
> -		goto failure;
> -	}
> -
> -success:
> -	free(logs);
> -	return EXIT_SUCCESS;
> -failure:
> -	free(logs);
> -	return EXIT_FAILURE;
> -}
> -
> -static void construct_vdi_tree(uint32_t vid, const char *name, const char *tag,
> -			       uint32_t snapid, uint32_t flags,
> -			       const struct sd_inode *i, void *data)
> -{
> -	add_vdi_tree(name, tag, vid, i->parent_vdi_id, false);
> -}
> -
> -static bool is_vdi_standalone(uint32_t vid, const char *name)
> -{
> -	struct vdi_tree *vdi;
> -
> -	init_tree();
> -	if (parse_vdi(construct_vdi_tree, SD_INODE_HEADER_SIZE, NULL) < 0)
> -		return EXIT_SYSFAIL;
> -
> -	vdi = find_vdi_from_root(vid, name);
> -	if (!vdi) {
> -		sd_err("failed to construct vdi tree");
> -		return false;
> -	}
> -
> -	return !vdi->pvid && list_empty(&vdi->children);
> -}
> -
> -#define ALTER_VDI_COPY_PRINT				\
> -	"    __\n"				\
> -	"   ()'`;\n"				\
> -	"   /\\|`  Caution! Changing VDI's redundancy level will affect\n" \
> -	"  /  |   the VDI itself only and trigger recovery.\n" \
> -	"(/_)_|_  Are you sure you want to continue? [yes/no]: "
> -
> -static int alter_vdi_copy(int argc, char **argv)
> -{
> -	int ret, old_nr_copies;
> -	uint32_t vid;
> -	const char *vdiname = argv[optind++];
> -	char buf[SD_INODE_HEADER_SIZE];
> -	struct sd_inode *inode = (struct sd_inode *)buf;
> -	struct sd_req hdr;
> -
> -	if (alter_cmd_data.copy_policy != 0) {
> -		sd_err("Changing redundancy level of erasure coded vdi "
> -			   "is not supported yet.");
> -		return EXIT_USAGE;
> -	}
> -	if (!alter_cmd_data.copies) {
> -		alter_cmd_data.copies = SD_DEFAULT_COPIES;
> -		printf("The vdi's redundancy level is not specified, "
> -			   "use %d as default.\n", SD_DEFAULT_COPIES);
> -	}
> -
> -	if (alter_cmd_data.copies > sd_nodes_nr) {
> -		char info[1024];
> -		snprintf(info, sizeof(info), "Number of copies (%d) is larger "
> -			 "than number of nodes (%d).\n"
> -			 "Are you sure you want to continue? [yes/no]: ",
> -			 alter_cmd_data.copies, sd_nodes_nr);
> -		confirm(info);
> -	}
> -
> -	ret = read_vdi_obj(vdiname, 0, "", &vid, inode, SD_INODE_HEADER_SIZE);
> -	if (ret != EXIT_SUCCESS) {
> -		sd_err("Reading %s's vdi object failure.", vdiname);
> -		return EXIT_FAILURE;
> -	}
> -
> -	if (inode->copy_policy) {
> -		sd_err("%s's copy policy is erasure code, "
> -			   "changing it is not supported yet.", vdiname);
> -		return EXIT_FAILURE;
> -	}
> -
> -	old_nr_copies = inode->nr_copies;
> -	if (old_nr_copies == alter_cmd_data.copies) {
> -		sd_err("%s's redundancy level is already set to %d, "
> -			   "nothing changed.", vdiname, old_nr_copies);
> -		return EXIT_FAILURE;
> -	}
> -
> -	if (!is_vdi_standalone(vid, inode->name)) {
> -		sd_err("Only standalone vdi supports "
> -			   "changing redundancy level.");
> -		sd_err("Please clone %s with -n (--no-share) "
> -			   "option first.", vdiname);
> -		return EXIT_FAILURE;
> -	}
> -
> -	confirm(ALTER_VDI_COPY_PRINT);
> -
> -	inode->nr_copies = alter_cmd_data.copies;
> -	ret = dog_write_object(vid_to_vdi_oid(vid), 0, inode,
> -			SD_INODE_HEADER_SIZE, 0, 0, old_nr_copies,
> -			inode->copy_policy, false, true);
> -	if (ret != SD_RES_SUCCESS) {
> -		sd_err("Overwrite the vdi object's header of %s failure "
> -			   "while setting its redundancy level.", vdiname);
> -		return EXIT_FAILURE;
> -	}
> -
> -	sd_init_req(&hdr, SD_OP_ALTER_VDI_COPY);
> -	hdr.vdi_state.new_vid = vid;
> -	hdr.vdi_state.copies = alter_cmd_data.copies;
> -	hdr.vdi_state.copy_policy = alter_cmd_data.copy_policy;
> -
> -	ret = send_light_req(&sd_nid, &hdr);
> -	if (ret == 0) {
> -		sd_info("%s's redundancy level is set to %d, the old one was %d.",
> -				vdiname, alter_cmd_data.copies, old_nr_copies);
> -		return EXIT_SUCCESS;
> -	}
> -	sd_err("Changing %s's redundancy level failure.", vdiname);
> -	return EXIT_FAILURE;
> -}
> -
> -static struct subcommand alter_cmd[] = {
> -	{"cluster-copy", NULL, "capht", "set the cluster's redundancy level",
> -	 NULL, CMD_NEED_NODELIST, alter_cluster_copy, alter_options},
> -	{"vdi-copy", "<vdiname>", "capht", "set the vdi's redundancy level",
> -	 NULL, CMD_NEED_ARG|CMD_NEED_NODELIST, alter_vdi_copy, alter_options},
> -	{NULL,},
> -};
> -
> -static int alter_parser(int ch, const char *opt)
> -{
> -	switch (ch) {
> -	case 'c':
> -		alter_cmd_data.copies =
> -			parse_copy(opt, &alter_cmd_data.copy_policy);
> -		if (!alter_cmd_data.copies) {
> -			sd_err("Invalid redundancy level %s.", opt);
> -			exit(EXIT_FAILURE);
> -		}
> -		break;
> -	}
> -
> -	return 0;
> -}
> -
> -struct command alter_command = {
> -	"alter",
> -	alter_cmd,
> -	alter_parser
> -};
> diff --git a/dog/cluster.c b/dog/cluster.c
> index fa62366..965dc5f 100644
> --- a/dog/cluster.c
> +++ b/dog/cluster.c
> @@ -571,6 +571,87 @@ static int cluster_check(int argc, char **argv)
>  	return EXIT_SUCCESS;
>  }
>  
> +#define ALTER_CLUSTER_COPY_PRINT				\
> +	"    __\n"				\
> +	"   ()'`;\n"				\
> +	"   /\\|`  Caution! Changing cluster's redundancy level will affect\n" \
> +	"  /  |   all the VDIs to be created later.\n" \
> +	"(/_)_|_  Are you sure you want to continue? [yes/no]: "
> +
> +static int cluster_alter_copy(int argc, char **argv)
> +{
> +	int ret, log_length;
> +	struct sd_req hdr;
> +	struct sd_rsp *rsp = (struct sd_rsp *)&hdr;
> +	struct epoch_log *logs;
> +
> +	if (cluster_cmd_data.copy_policy != 0) {
> +		sd_err("Changing redundancy level of erasure coded vdi "
> +			   "is not supported yet.");
> +		return EXIT_USAGE;
> +	}
> +	if (!cluster_cmd_data.copies) {
> +		cluster_cmd_data.copies = SD_DEFAULT_COPIES;
> +		printf("The cluster's redundancy level is not specified, "
> +			   "use %d as default.\n", SD_DEFAULT_COPIES);
> +	}
> +
> +	if (cluster_cmd_data.copies > sd_nodes_nr) {
> +		char info[1024];
> +		snprintf(info, sizeof(info), "Number of copies (%d) is larger "
> +			 "than number of nodes (%d).\n"
> +			 "Are you sure you want to continue? [yes/no]: ",
> +			 cluster_cmd_data.copies, sd_nodes_nr);
> +		confirm(info);
> +	}
> +
> +	log_length = sd_epoch * sizeof(struct epoch_log);
> +	logs = xmalloc(log_length);
> +	sd_init_req(&hdr, SD_OP_STAT_CLUSTER);
> +	hdr.data_length = log_length;
> +	ret = dog_exec_req(&sd_nid, &hdr, logs);
> +	if (ret < 0)
> +		goto failure;
> +
> +	if (rsp->result != SD_RES_SUCCESS) {
> +		sd_err("Response's result: %s", sd_strerror(rsp->result));
> +		goto failure;
> +	}
> +	if (logs->copy_policy) {
> +		sd_err("The cluster's copy policy is erasure code, "
> +			   "changing it is not supported yet.");
> +		goto failure;
> +	}
> +	if (logs->nr_copies == cluster_cmd_data.copies) {
> +		sd_err("The cluster's redundancy level is already set to %d, "
> +			   "nothing changed.", cluster_cmd_data.copies);
> +		goto failure;
> +	}
> +
> +	confirm(ALTER_CLUSTER_COPY_PRINT);
> +
> +	sd_init_req(&hdr, SD_OP_ALTER_CLUSTER_COPY);
> +	hdr.cluster.copies = cluster_cmd_data.copies;
> +	hdr.cluster.copy_policy = cluster_cmd_data.copy_policy;
> +	ret = send_light_req(&sd_nid, &hdr);
> +	if (ret == 0) {
> +		sd_info("The cluster's redundancy level is set to %d, "
> +				"the old one was %d.",
> +				cluster_cmd_data.copies, logs->nr_copies);
> +		goto success;
> +	} else {
> +		sd_err("Changing the cluster's redundancy level failure.");
> +		goto failure;
> +	}
> +
> +success:
> +	free(logs);
> +	return EXIT_SUCCESS;
> +failure:
> +	free(logs);
> +	return EXIT_FAILURE;
> +}
> +
>  static struct subcommand cluster_cmd[] = {
>  	{"info", NULL, "aprhvt", "show cluster information",
>  	 NULL, CMD_NEED_NODELIST, cluster_info, cluster_options},
> @@ -589,6 +670,8 @@ static struct subcommand cluster_cmd[] = {
>  	 cluster_reweight, cluster_options},
>  	{"check", NULL, "apht", "check and repair cluster", NULL,
>  	 CMD_NEED_NODELIST, cluster_check, cluster_options},
> +	{"alter-copy", NULL, "apht", "set the cluster's redundancy level",
> +	 NULL, CMD_NEED_NODELIST, cluster_alter_copy, cluster_options},
>  	{NULL,},
>  };
>  
> diff --git a/dog/dog.c b/dog/dog.c
> index 4eb4ad8..fba44cf 100644
> --- a/dog/dog.c
> +++ b/dog/dog.c
> @@ -175,7 +175,6 @@ static void init_commands(const struct command **commands)
>  		vdi_command,
>  		node_command,
>  		cluster_command,
> -		alter_command,
>  #ifdef HAVE_TRACE
>  		trace_command,
>  #endif
> diff --git a/dog/vdi.c b/dog/vdi.c
> index 9fc1677..42102bd 100644
> --- a/dog/vdi.c
> +++ b/dog/vdi.c
> @@ -2438,6 +2438,120 @@ static int vdi_cache(int argc, char **argv)
>  	return do_generic_subcommand(vdi_cache_cmd, argc, argv);
>  }
>  
> +static void construct_vdi_tree(uint32_t vid, const char *name, const char *tag,
> +			       uint32_t snapid, uint32_t flags,
> +			       const struct sd_inode *i, void *data)
> +{
> +	add_vdi_tree(name, tag, vid, i->parent_vdi_id, false);
> +}
> +
> +static bool is_vdi_standalone(uint32_t vid, const char *name)
> +{
> +	struct vdi_tree *vdi;
> +
> +	init_tree();
> +	if (parse_vdi(construct_vdi_tree, SD_INODE_HEADER_SIZE, NULL) < 0)
> +		return EXIT_SYSFAIL;
> +
> +	vdi = find_vdi_from_root(vid, name);
> +	if (!vdi) {
> +		sd_err("failed to construct vdi tree");
> +		return false;
> +	}
> +
> +	return !vdi->pvid && list_empty(&vdi->children);
> +}
> +
> +#define ALTER_VDI_COPY_PRINT				\
> +	"    __\n"				\
> +	"   ()'`;\n"				\
> +	"   /\\|`  Caution! Changing VDI's redundancy level will affect\n" \
> +	"  /  |   the VDI itself only and trigger recovery.\n" \
> +	"(/_)_|_  Are you sure you want to continue? [yes/no]: "
> +
> +static int vdi_alter_copy(int argc, char **argv)
> +{
> +	int ret, old_nr_copies;
> +	uint32_t vid;
> +	const char *vdiname = argv[optind++];
> +	char buf[SD_INODE_HEADER_SIZE];
> +	struct sd_inode *inode = (struct sd_inode *)buf;
> +	struct sd_req hdr;
> +
> +	if (vdi_cmd_data.copy_policy != 0) {
> +		sd_err("Changing redundancy level of erasure coded vdi "
> +			   "is not supported yet.");
> +		return EXIT_USAGE;
> +	}
> +	if (!vdi_cmd_data.nr_copies) {
> +		vdi_cmd_data.nr_copies = SD_DEFAULT_COPIES;
> +		printf("The vdi's redundancy level is not specified, "
> +			   "use %d as default.\n", SD_DEFAULT_COPIES);
> +	}
> +
> +	if (vdi_cmd_data.nr_copies > sd_nodes_nr) {
> +		char info[1024];
> +		snprintf(info, sizeof(info), "Number of copies (%d) is larger "
> +			 "than number of nodes (%d).\n"
> +			 "Are you sure you want to continue? [yes/no]: ",
> +			 vdi_cmd_data.nr_copies, sd_nodes_nr);
> +		confirm(info);
> +	}
> +
> +	ret = read_vdi_obj(vdiname, 0, "", &vid, inode, SD_INODE_HEADER_SIZE);
> +	if (ret != EXIT_SUCCESS) {
> +		sd_err("Reading %s's vdi object failure.", vdiname);
> +		return EXIT_FAILURE;
> +	}
> +
> +	if (inode->copy_policy) {
> +		sd_err("%s's copy policy is erasure code, "
> +			   "changing it is not supported yet.", vdiname);
> +		return EXIT_FAILURE;
> +	}
> +
> +	old_nr_copies = inode->nr_copies;
> +	if (old_nr_copies == vdi_cmd_data.nr_copies) {
> +		sd_err("%s's redundancy level is already set to %d, "
> +			   "nothing changed.", vdiname, old_nr_copies);
> +		return EXIT_FAILURE;
> +	}
> +
> +	if (!is_vdi_standalone(vid, inode->name)) {
> +		sd_err("Only standalone vdi supports "
> +			   "changing redundancy level.");
> +		sd_err("Please clone %s with -n (--no-share) "
> +			   "option first.", vdiname);
> +		return EXIT_FAILURE;
> +	}
> +
> +	confirm(ALTER_VDI_COPY_PRINT);
> +
> +	inode->nr_copies = vdi_cmd_data.nr_copies;
> +	ret = dog_write_object(vid_to_vdi_oid(vid), 0, inode,
> +			SD_INODE_HEADER_SIZE, 0, 0, old_nr_copies,
> +			inode->copy_policy, false, true);
> +	if (ret != SD_RES_SUCCESS) {
> +		sd_err("Overwrite the vdi object's header of %s failure "
> +			   "while setting its redundancy level.", vdiname);
> +		return EXIT_FAILURE;
> +	}
> +
> +	sd_init_req(&hdr, SD_OP_ALTER_VDI_COPY);
> +	hdr.vdi_state.new_vid = vid;
> +	hdr.vdi_state.copies = vdi_cmd_data.nr_copies;
> +	hdr.vdi_state.copy_policy = vdi_cmd_data.copy_policy;
> +
> +	ret = send_light_req(&sd_nid, &hdr);
> +	if (ret == 0) {
> +		sd_info("%s's redundancy level is set to %d, the old one was %d.",
> +				vdiname, vdi_cmd_data.nr_copies, old_nr_copies);
> +		return EXIT_SUCCESS;
> +	}
> +	sd_err("Changing %s's redundancy level failure.", vdiname);
> +	return EXIT_FAILURE;
> +}
> +
>  static struct subcommand vdi_cmd[] = {
>  	{"check", "<vdiname>", "sapht", "check and repair image's consistency",
>  	 NULL, CMD_NEED_NODELIST|CMD_NEED_ARG,
> @@ -2500,6 +2614,8 @@ static struct subcommand vdi_cmd[] = {
>  	 "Run 'dog vdi cache' for more information",
>  	 vdi_cache_cmd, CMD_NEED_ARG,
>  	 vdi_cache, vdi_options},
> +	{"alter-copy", "<vdiname>", "capht", "set the vdi's redundancy level",
> +	 NULL, CMD_NEED_ARG|CMD_NEED_NODELIST, vdi_alter_copy, vdi_options},
>  	{NULL,},
>  };
>  
> -- 
> 1.8.3.2
> 



More information about the sheepdog mailing list