Re: tgtadm/tgtd management issue: buffer size truncated sometimes

On 27 Jan 2014, at 19:42, FUJITA Tomonori <[email protected]> wrote:

> Hi,
> 
> On Mon, 27 Jan 2014 18:44:42 +0400
> Artyom Pervukhin <[email protected]> wrote:
> 
>> I've done some additional stracing on the tgtd's side this time, please see below:
>> 
>> Testing 300 targets:
>> 
>> command:
>> 
>> root@sandbox:~ # tgtadm -C 0 --op show --mode target
>> 
>> (command shows full output)
>> 
>> tgtd writes:
>> 
>> write(10, "\0\0\0\0\305\325\4\0", 8)    = 8
>> write(10, "Target 1: iqn.2014-01:d1\n    Sys"..., 316861) = 224000
>> write(10, "       State: ready\n    I_T nexu"..., 92861) = 92861
>> 
>> command:
>> 
>> root@sandbox:~ # tgtadm -C 0 --op show --mode target | wc -c                                                                                                                
>> 316860
>> 
>> tgtd writes stays the same as above.
>> 
>> 
>> 
>> Testing 900 targets:
>> 
>> root@sandbox:~ # tgtadm -C 0 --op show --mode target
>> 
>> (command shows incomplete output)
>> 
>> tgtd writes:
>> 
>> write(10, "\0\0\0\0u\205\16\0", 8)      = 8
>> write(10, "Target 1: iqn.2014-01:d1\n    Sys"..., 951661) = 256000
>> write(10, "a: No\n            Prevent remova"..., 695661) = 176000
>> write(10, " Removable media: No\n           "..., 519661) = 176000
>> write(10, "T nexus information:\n    LUN inf"..., 343661) = 176000
>> write(10, "  Backing store path: None\n     "..., 167661) = 167661
>> 
>> command:
>> 
>> root@sandbox:~ # tgtadm -C 0 --op show --mode target | wc -c                                                                                                                
>> 929621
>> 
>> tgtd writes:
>> 
>> write(10, "\0\0\0\0u\205\16\0", 8)      = 8
>> write(10, "Target 1: iqn.2014-01:d1\n    Sys"..., 951661) = 944000
>> write(10, "t removal: No\n            Readon"..., 7661) = -1 EPIPE (Broken pipe)
>> --- SIGPIPE (Broken pipe) @ 0 (0) ---
>> 
>> 
>> 
>> Seeing this, I assumed the bug is on tgtadm's side.
> 
> Thanks a lot for the investigation. The following fixes the problem?
> 
> Note that tgtd uses a stream to store what tgtd sends to tgtadm. The
> buffer of the stream is managed glibc (internally allocated via
> malloc). That's a limit about how many targets you can see via tgtadm.
> 
> 
> diff --git a/usr/tgtadm.c b/usr/tgtadm.c
> index cc63fbf..8e0d5dd 100644
> --- a/usr/tgtadm.c
> +++ b/usr/tgtadm.c
> @@ -234,7 +234,8 @@ static int ipc_mgmt_connect(int *fd)
> static int ipc_mgmt_rsp(int fd, struct tgtadm_req *req)
> {
> 	struct tgtadm_rsp rsp;
> -	int err, rest, len;
> +	int err, len, done;
> +	char *buf;
> 
> retry:
> 	err = recv(fd, &rsp, sizeof(rsp), MSG_WAITALL);
> @@ -278,23 +279,28 @@ retry:
> 		}
> 	}
> 
> -	rest = rsp.len - sizeof(rsp);
> -	if (!rest)
> +	len = rsp.len - sizeof(rsp);
> +	if (!len)
> 		return 0;
> 
> -	while (rest) {
> -		char buf[BUFSIZE];
> -		memset(buf, 0, sizeof(buf));
> -		len = min_t(int, sizeof(buf) - 1, rest);
> -		err = read(fd, buf, len);
> -		if (err <= 0) {
> -			eprintf("\ncan't get the full response, %m\n");
> -			return errno;
> +	buf = malloc(len);
> +	done = 0;
> +	while (len > done) {
> +		int ret;
> +		ret = read(fd, buf + done, len - done);
> +		if (ret < 0) {
> +			if (errno == EAGAIN)
> +				continue;
> +			fprintf(stderr, "failed to read from tgtd, %d", errno);
> +			break;
> 		}
> -		fputs(buf, stdout);
> -		rest -= len;
> +		done += ret;
> 	}
> 
> +	if (done == len)
> +		fputs(buf, stdout);
> +	free(buf);
> +
> 	return 0;
> }
> 

I’ve just applied this patch and the problem is gone: all tests I’ve used before are passing.

Thanks a lot!