[stgt] [PATCH] iscsi: target redirect support

[FUJITA Tomonori]

On Mon, 14 Jun 2010 18:13:53 -0700
Chandra Seetharaman <sekharan at us.ibm.com> wrote:

> Hi Tomo,
> 
> I tested with two tgtds one the redirector tgtd and the other, serving
> tgtd. Hope this is the model you intended. (Let me know if otherwise).
> 
> I tried two different things, and both of them worked as expected.
> 
> 1. Ran the redirector tgtd and the serving tgtd in a same node listening
> to different portal.

Sorry, I'm not sure what you did. I suspect that you run two tgtd
daemons on the single node?

Can you tell me the exact commands that you run?


> 2. Ran the redirector tgtd on one node and the service tgtd on another.

Sorry, again. I'm not sure what 'Ran the redirector tgtd on one node'
means. I guess that you run tgtd daemons on the two nodes.


> In case one, I have to first start the serving tgtd first in order for
> it to grab the luns the target is supposed to serve (or modify the
> targets.conf file to not have luns in them). If I used the same
> targets.conf file and changed the order, the serving tgtd doesn't get
> the access to the backing store, which is not correct.

Why tgtd doesn't get the access? That's your targets.conf problem,
isn't it? targets.conf for the first tgtd daemon can have a target
without lun (except for lun0).

I also think that tgtd doesn't lock backing files so two daemons can
open the same backing file.


> In effect, (1) I think we need to be able to mark the redirector tgtd as
> such (with an option in the command line or such),

I'm not sure why we need this since sounds like you can avoid the
issue with a proper targets.conf.

btw, I think that we need to modify the redirect configuration on the
fly so I avoided the command line.


> or (2) as and when
> the tgtd is instructed to be a redirector for a target then the tgtd has
> to close all the open backing store for the target. IMO, option (1)
> sounds better, what do you think ? (or am I way off :)...
> 
> >From the code point of view, it all looks good except a missing
> free()....
> 
> On Mon, 2010-06-14 at 15:15 +0900, FUJITA Tomonori wrote:
> 
> <snip>
> > +	p = strdup(target->redirect_info.addr);
> > +	if (!p)
> > +		return 0;
> > +	str = p;
> > +
> > +	if (*p == '[') {
> > +		p++;
> > +		if (!(q = strchr(p, ']')))
> 
> str needs to be freed before this return.

Oops, thanks! I'll fix it and merge the updated patch.


> > +			return 0;
> > +		*(q++) = '\0';
> > +	}
> > +
> > +	memset(&hints, 0, sizeof(hints));
> > +	hints.ai_socktype = SOCK_STREAM;
> > +	hints.ai_flags = AI_NUMERICHOST;
> > +
> > +	ret = getaddrinfo(p, NULL, &hints, &res);
> > +	if (ret < 0) {
> > +		free(str);
> > +		return 0;
> > +	}
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe stgt" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html