[sheepdog] [PATCH v8 2/3] collie: let collie use sockfd cache

[Liu Yuan]

On Wed, Jul 31, 2013 at 04:11:47PM +0900, Hitoshi Mitake wrote:
> From: Hitoshi Mitake <mitake.hitoshi at gmail.com>
> 
> This patch lets collie use sockfd cache. Because the sockfd caching
> mechanism is now in libsheepdog, collie can enjoy it on some
> subcommands which issue many requests to sheep.
> 
> Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> ---
> v8:
>  - fix an unsuitable error message of collie
> 
> v5:
>  - trivial cleaning. remove an old prototype of function
> 
> v4:
>  - remove the redundant function, collie_exec_req_nid()
> 
>  collie/collie.c |    6 ++++++
>  collie/common.c |   24 ++++++++++++++----------
>  2 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/collie/collie.c b/collie/collie.c
> index 2f8e9d0..d6eef73 100644
> --- a/collie/collie.c
> +++ b/collie/collie.c
> @@ -17,6 +17,7 @@
>  #include "sheep.h"
>  #include "collie.h"
>  #include "util.h"
> +#include "sockfd_cache.h"
>  
>  #define EPOLL_SIZE 4096
>  
> @@ -418,6 +419,11 @@ int main(int argc, char **argv)
>  		exit(EXIT_SYSFAIL);
>  	}
>  
> +	if (sockfd_init()) {
> +		fprintf(stderr, "sockfd_init() failed\n");
> +		exit(EXIT_SYSFAIL);
> +	}
> +
>  	ret = command_fn(argc, argv);
>  	if (ret == EXIT_USAGE)
>  		subcommand_usage(argv[1], argv[2], EXIT_USAGE);
> diff --git a/collie/common.c b/collie/common.c
> index b2b9fb1..2aca7bd 100644
> --- a/collie/common.c
> +++ b/collie/common.c
> @@ -11,6 +11,7 @@
>  
>  #include "collie.h"
>  #include "sha1.h"
> +#include "sockfd_cache.h"
>  
>  char *size_to_str(uint64_t _size, char *str, int str_size)
>  {
> @@ -169,13 +170,18 @@ out:
>  	return ret;
>  }
>  
> -int collie_exec_req(const char *host, int port, struct sd_req *hdr, void *data)
> +int collie_exec_req(const char *host, int port, struct sd_req *hdr, void *buf)
>  {
> -	int fd, ret;
> -	struct sd_rsp *rsp = (struct sd_rsp *)hdr;
> +	struct node_id nid;
> +	struct sockfd *sfd;
> +	int ret;
> +
> +	memset(&nid, 0, sizeof(nid));
> +	str_to_addr(host, nid.addr);
> +	nid.port = port;
>  
> -	fd = connect_to(host, port);
> -	if (fd < 0)
> +	sfd = sockfd_cache_get(&nid);
> +	if (!sfd)
>  		return -1;
>  
>  	/*
> @@ -183,13 +189,11 @@ int collie_exec_req(const char *host, int port, struct sd_req *hdr, void *data)
>  	 * 1. We can't get the newest epoch
>  	 * 2. Some operations might take unexpected long time
>  	 */
> -	ret = exec_req(fd, hdr, data, NULL, 0, UINT32_MAX);
> -	close(fd);
> +	ret = exec_req(sfd->fd, hdr, buf, NULL, 0, UINT32_MAX);
>  
> -	if (ret)
> -		return -1;
> +	sockfd_cache_put(&nid, sfd);
>  
> -	return rsp->result;
> +	return ret;

Any reason you return 'ret' instead of old rsp->result? This kind of change is
quite hard to catch, so please write it in the commit log if you change the old
semantics of any function. I think we should return rsp->result to the caller.

Thanks
Yuan