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

Hitoshi Mitake mitake.hitoshi at lab.ntt.co.jp
Tue Jul 22 11:46:02 CEST 2014


At Tue, 22 Jul 2014 16:42:30 +0800,
Ruoyu wrote:
> 
> 
> On 2014年07月22日 15:32, 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
> >
> > 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},
> The option for the sub command alter-copy should be "capht", 'c' means 
> '--copies'

Thanks, I'll modify it before pushing if the idea is accepted.

> 
> In fact, this patch is something like the first submit I worked several 
> weeks ago.

Yes, I prefer the original one.

> Yuan raised dog alter command at that time. I think it could be used to 
> modify other settings in the future.

I cannot imagine the other settings. Do you have some ideas?

Thanks,
Hitoshi

> >   	{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,},
> >   };
> >   
> 
> 



More information about the sheepdog mailing list