[Sheepdog] [PATCH v5 02/17] store: add dynamic mechanism to chain the available backend stores.

MORITA Kazutaka morita.kazutaka at lab.ntt.co.jp
Wed Jan 4 21:27:43 CET 2012


This patch has style problems.  Use tabs instead of spaces where
possible.

>  - change global store structure to a pointer
>  - use a list to maintain the stores.
>  - use /obj/.store to remember backend store persistently.

How about writing the backend driver name into 'config'?  Sheepdog
uses the file to store global info.

>  - now we can specify the backend store in the command
> 	collie cluster format -b farm #use farm
>    if no store specified, currently sheep will use 'simple' store.
>    if specified store not available, collie will return a list of
>    available stores.
> 
> Signed-off-by: Liu Yuan <tailai.ly at taobao.com>
> ---
>  collie/cluster.c         |   59 +++++++++++++++++++++++-
>  collie/collie.c          |    1 +
>  include/sheep.h          |    9 ++-
>  include/sheepdog_proto.h |    1 +
>  sheep/ops.c              |   30 ++++++++++++
>  sheep/sheep_priv.h       |   34 +++++++++++++-
>  sheep/simple_store.c     |   10 +---
>  sheep/store.c            |  115 +++++++++++++++++++++++++++++++++++----------
>  8 files changed, 219 insertions(+), 40 deletions(-)
> 
> diff --git a/collie/cluster.c b/collie/cluster.c
> index 6fbda6b..a97ef79 100644
> --- a/collie/cluster.c
> +++ b/collie/cluster.c
> @@ -20,6 +20,7 @@ struct cluster_cmd_data {
>  	int copies;
>  	int nohalt;
>  	int force;
> +	char argv[10];
>  } cluster_cmd_data;
>  
>  static void set_nohalt(uint16_t *p)
> @@ -28,6 +29,53 @@ static void set_nohalt(uint16_t *p)
>  		*p |= SD_FLAG_NOHALT;
>  }
>  
> +static int get_store_index(char *name)
> +{
> +	int ret = -1;
> +	if (!strlen(name) || strcmp(name, "simple") == 0)
> +		ret = 0;
> +	return ret;
> +}
> +
> +static int list_store(void)
> +{
> +	int fd, ret;
> +	struct sd_req hdr;
> +	struct sd_rsp *rsp = (struct sd_rsp *)&hdr;
> +	unsigned rlen, wlen;
> +	char buf[512] = { 0 };
> +
> +	fd = connect_to(sdhost, sdport);
> +	if (fd < 0)
> +		return EXIT_SYSFAIL;
> +
> +	memset(&hdr, 0, sizeof(hdr));
> +
> +        wlen = 0;
> +        rlen = 512;
> +        hdr.opcode = SD_OP_GET_STORE_LIST;
> +        hdr.data_length = rlen;
> +
> +        ret = exec_req(fd, &hdr, buf, &wlen, &rlen);
> +        close(fd);
> +
> +        if (ret) {
> +                fprintf(stderr, "Failed to connect\n");
> +                return EXIT_FAILURE;

Should return EXIT_SYSFAIL.

> +        }
> +
> +        if (rsp->result != SD_RES_SUCCESS) {
> +                fprintf(stderr, "Restore failed: %s\n",
> +                                sd_strerror(rsp->result));
> +                return EXIT_FAILURE;
> +        }
> +
> +	printf("Available stores:\n");
> +	printf("---------------------------------------\n");
> +	printf("%s\n", buf);
> +	return EXIT_SUCCESS;

Should return EXIT_FAILURE because this function is called only when
an error occurs.


> +}
> +
>  static int cluster_format(int argc, char **argv)
>  {
>  	int fd, ret;
> @@ -35,6 +83,9 @@ static int cluster_format(int argc, char **argv)
>  	struct sd_so_rsp *rsp = (struct sd_so_rsp *)&hdr;
>  	unsigned rlen, wlen;
>  	struct timeval tv;
> +	uint8_t idx;
> +
> +	idx = get_store_index(cluster_cmd_data.argv);

How about sending the driver name instead of its index?  Then, we
don't need to maintain consistency between index_to_name() and
get_store_index().

Thanks,

Kazutaka



More information about the sheepdog mailing list