I'll modify the patch to set cxgb4i to NOT_REQ for cxgb4i, will change it back 
to OPT when we add support.
-Anish

> -----Original Message-----
> From: Karen Xie
> Sent: Friday, July 25, 2014 12:19 PM
> To: Mike Christie; Anish Bhatt
> Cc: [email protected]
> Subject: RE: [PATCH] iscsiadm : make iface.ipaddress optional in iface configs
> for transports that don't have a hard requirement on it.
> Importance: High
> 
> Hi, Mike,
> 
> cxgb3i supports setting the iscsi ip address specified in the iface config. 
> cxgb4i
> does not support it, for setting it prints out a message but does not return 
> an
> error. Yes, we will fix it.
> 
> Thanks,
> Karen
> ________________________________________
> From: Mike Christie [[email protected]]
> Sent: Friday, July 25, 2014 11:14 AM
> To: Anish Bhatt
> Cc: [email protected]; Karen Xie
> Subject: Re: [PATCH] iscsiadm : make iface.ipaddress optional in iface configs
> for transports that don't have a hard requirement on it.
> 
> On 07/24/2014 09:47 PM, Anish Bhatt wrote:
> > If an ipaddress is setup for a chelsio interface, then
> > /sys/class/iscsi_host/ip_address returns the ip address from the
> > netdev correctly, otherwise it is all zeros. It will always return the
> > value from netdev, since we don't actually support setting
> > iface.ipaddress in the transport
> 
> I am confused about the last statement. If you do not support
> iface.ipaddress why is it optional in your patch? Also, is that statement that
> for both drivers or only cxgb3i. It looks like cxgb4i only.
> 
> For cxgb3i, it looks like cxgbi_set_host_param will set the
> chba->ipv4addr to what is passed in, and it looks like
> cxgbi_get_host_param will return that value. It then looks like
> update_address will look at ipv4addr and use it if set. So it looks like it 
> could
> use the iface.ipaddress and so SET_HOST_IP_OPT is ok to use.
> However, should cxgbi_get_host_param be returning the netdev ip address
> if cxgbi_set_host_param/ISCSI_HOST_PARAM_IPADDRESS has not yet been
> called?
> 
> For cxgb4i, it looks like cxgbi_set_host_param will set the
> chba->ipv4addr to what is passed in, and it looks like
> cxgbi_get_host_param will return that value. However, I am not seeing that
> driver ever touch ipv4addr. Is that right? So will it always use the netdev
> ipaddress, but actually return in /sys/class/iscsi_host/ip_address whatever
> was set by the iscsi tools in iface.ipaddress? If so, should cxgb4i not be
> SET_HOST_IP_NOT_REQ and should the cxgb4i
> ISCSI_HOST_PARAM_IPADDRESS code be fixed so it returns the value we are
> actually using?
> 
> 
> > -Anish
> > ________________________________________
> > From: Mike Christie [[email protected]]
> > Sent: Thursday, July 24, 2014 9:21 AM
> > To: [email protected]
> > Cc: Anish Bhatt; Karen Xie; Anish Bhatt
> > Subject: Re: [PATCH] iscsiadm : make iface.ipaddress optional in iface
> configs for transports that don't have a hard requirement on it.
> >
> > On 07/24/2014 01:14 AM, Anish Bhatt wrote:
> >> From: Anish Bhatt <[email protected]>
> >>
> >> Signed-off-by: Anish Bhatt <[email protected]>
> >> ---
> >>  usr/initiator_common.c | 15 ++++++++++++---
> >>  usr/transport.c        |  8 ++++----
> >>  usr/transport.h        |  6 ++++++
> >>  3 files changed, 22 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/usr/initiator_common.c b/usr/initiator_common.c index
> >> 50f8d41..8ff993d 100644
> >> --- a/usr/initiator_common.c
> >> +++ b/usr/initiator_common.c
> >> @@ -685,9 +685,18 @@ int iscsi_host_set_net_params(struct iface_rec
> >> *iface,
> >>
> >>       /* if we need to set the ip addr then set all the iface net settings 
> >> */
> >>       if (!iface_is_bound_by_ipaddr(iface)) {
> >> -             log_warning("Please set the iface.ipaddress for iface %s, "
> >> -                         "then retry the login command.\n", iface->name);
> >> -             return EINVAL;
> >> +             if (t->template->set_host_ip == SET_HOST_IP_REQ) {
> >> +                     log_warning("Please set the iface.ipaddress for 
> >> iface "
> >> +                                 "%s, then retry the login command.\n",
> >> +                                 iface->name);
> >> +                     return EINVAL;
> >> +             } else if (t->template->set_host_ip == SET_HOST_IP_OPT) {
> >> +                     log_info("Optional iface.ipaddress for iface %s "
> >> +                              "not set.\n", iface->name);
> >> +                     return 0;
> >> +             } else {
> >
> > Is there a way to tell if it was not set in the iface config file but
> > is actually already set in the driver/kernel already?
> >
> > If I read the /sys/class/iscsi_host/ip_address file will that return
> > the value from the netdev that will be used or the already setup value?
> >
> >

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

Reply via email to