RE: Antw: [PATCH v2 1/2] iscsid: Changes to support ping through iscsiuio

2015-08-25 Thread Adheer Chandravanshi


> -Original Message-
> From: open-iscsi@googlegroups.com [mailto:open-iscsi@googlegroups.com]
> On Behalf Of Mike Christie
> Sent: Tuesday, August 25, 2015 8:09 AM
> To: open-iscsi@googlegroups.com
> Subject: Re: Antw: [PATCH v2 1/2] iscsid: Changes to support ping through
> iscsiuio
> 
> On 7/12/15, 11:34 PM, Adheer Chandravanshi wrote:
> >
> >
> >> -Original Message-
> >> From: open-iscsi@googlegroups.com
> >> [mailto:open-iscsi@googlegroups.com]
> >> On Behalf Of Ulrich Windl
> >> Sent: Friday, July 10, 2015 11:32 AM
> >> Cc: open-iscsi
> >> Subject: Antw: [PATCH v2 1/2] iscsid: Changes to support ping through
> >> iscsiuio
> >>
> >>>>>  schrieb am 09.07.2015 um 13:38
> >>>>> in Nachricht
> >> <1436441896-7161-2-git-send-email-adheer.chandravan...@qlogic.com>:
> >>> From: Adheer Chandravanshi 
> >>>
> >>> These changes are done in order to support ping operation for
> >>> drivers like bnx2i that use iscsiuio.
> >>>
> >>> Signed-off-by: Adheer Chandravanshi
> >> 
> >>> ---
> >>>   usr/iscsiadm.c |   38 +++---
> >>>   usr/iscsid_req.c   |   38 +-
> >>>   usr/iscsid_req.h   |2 +-
> >>>   usr/transport.c|1 +
> >>>   usr/transport.h|3 +++
> >>>   usr/uip_mgmt_ipc.c |   38
> >> +-
> >>>   usr/uip_mgmt_ipc.h |   13 +
> >>>   7 files changed, 119 insertions(+), 14 deletions(-)
> >>>
> >>> diff --git a/usr/iscsiadm.c b/usr/iscsiadm.c index aa7cf07..495af3a
> >>> 100644
> >>> --- a/usr/iscsiadm.c
> >>> +++ b/usr/iscsiadm.c
> >>> @@ -3099,10 +3099,10 @@ static char *iscsi_ping_stat_strs[] = {
> >>>   "No ARP response received",
> >>>   };
> >>>
> >>> -static char *iscsi_ping_stat_to_str(uint32_t status)
> >>> +static char *iscsi_ping_stat_to_str(int status)
> >>>   {
> >>>   if (status < 0 || status > ISCSI_PING_NO_ARP_RECEIVED) {
> >>> - log_error("Invalid ping status %u", status);
> >>> + log_error("Ping error: %s\n", strerror(status));
> >>
> >> "status" does not look very much like "errno", but isn't strerror()
> >> defined for errno values?
> >>
> >>
> >
> > "status" is used to get the status of ping from iscsiuio.
> > It will either contain the defined ping status or errno if ping fails due to
> some other problem.
> > So, strerror() is used only when status holds some errno .
> > And iscsi_ping_stat_strs[status] is used to return corresponding string for
> ping status code.
> >
> 
> That is such a funky API. In a separate patch could you fix this, so status is
> always a ISCSI_PING_* return code?
> 

Ulrich, Lee, Mike,
Thanks for the review and feedback.

I will fix this and the other comment regarding send_ping() in next patch set.

Thanks,
Adheer



> --
> 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 open-iscsi+unsubscr...@googlegroups.com.
> To post to this group, send email to open-iscsi@googlegroups.com.
> Visit this group at http://groups.google.com/group/open-iscsi.
> For more options, visit https://groups.google.com/d/optout.

-- 
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 open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.


Re: Antw: [PATCH v2 1/2] iscsid: Changes to support ping through iscsiuio

2015-08-24 Thread Mike Christie

On 7/12/15, 11:34 PM, Adheer Chandravanshi wrote:




-Original Message-
From: open-iscsi@googlegroups.com [mailto:open-iscsi@googlegroups.com]
On Behalf Of Ulrich Windl
Sent: Friday, July 10, 2015 11:32 AM
Cc: open-iscsi
Subject: Antw: [PATCH v2 1/2] iscsid: Changes to support ping through
iscsiuio


 schrieb am 09.07.2015 um 13:38 in
Nachricht

<1436441896-7161-2-git-send-email-adheer.chandravan...@qlogic.com>:

From: Adheer Chandravanshi 

These changes are done in order to support ping operation for drivers
like bnx2i that use iscsiuio.

Signed-off-by: Adheer Chandravanshi



---
  usr/iscsiadm.c |   38 +++---
  usr/iscsid_req.c   |   38 +-
  usr/iscsid_req.h   |2 +-
  usr/transport.c|1 +
  usr/transport.h|3 +++
  usr/uip_mgmt_ipc.c |   38

+-

  usr/uip_mgmt_ipc.h |   13 +
  7 files changed, 119 insertions(+), 14 deletions(-)

diff --git a/usr/iscsiadm.c b/usr/iscsiadm.c index aa7cf07..495af3a
100644
--- a/usr/iscsiadm.c
+++ b/usr/iscsiadm.c
@@ -3099,10 +3099,10 @@ static char *iscsi_ping_stat_strs[] = {
"No ARP response received",
  };

-static char *iscsi_ping_stat_to_str(uint32_t status)
+static char *iscsi_ping_stat_to_str(int status)
  {
if (status < 0 || status > ISCSI_PING_NO_ARP_RECEIVED) {
-   log_error("Invalid ping status %u", status);
+   log_error("Ping error: %s\n", strerror(status));


"status" does not look very much like "errno", but isn't strerror() defined for
errno values?




"status" is used to get the status of ping from iscsiuio.
It will either contain the defined ping status or errno if ping fails due to 
some other problem.
So, strerror() is used only when status holds some errno .
And iscsi_ping_stat_strs[status] is used to return corresponding string for 
ping status code.



That is such a funky API. In a separate patch could you fix this, so 
status is always a ISCSI_PING_* return code?


--
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 open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.


Re: Antw: [PATCH v2 1/2] iscsid: Changes to support ping through iscsiuio

2015-07-13 Thread The Lee-Man
On Sunday, July 12, 2015 at 9:35:07 PM UTC-7, Adheer Chandravanshi wrote:
>
>
>
> > -Original Message- 
> > From: open-...@googlegroups.com  [mailto:
> open-...@googlegroups.com ] 
> > On Behalf Of Ulrich Windl 
> > Sent: Friday, July 10, 2015 11:32 AM 
> > Cc: open-iscsi 
> > Subject: Antw: [PATCH v2 1/2] iscsid: Changes to support ping through 
> > iscsiuio 
> > 
> > >>> > schrieb am 09.07.2015 um 
> 13:38 in 
> > >>> Nachricht 
> > <1436441896-7161-2-git-send-email-adheer.chandravan...@qlogic.com 
> >: 
> > > From: Adheer Chandravanshi > 
> > > 
> > > These changes are done in order to support ping operation for drivers 
> > > like bnx2i that use iscsiuio. 
> > > 
> > > Signed-off-by: Adheer Chandravanshi 
> > > 
> > > --- 
> > >  usr/iscsiadm.c |   38 +++--- 
> > >  usr/iscsid_req.c   |   38 +- 
> > >  usr/iscsid_req.h   |2 +- 
> > >  usr/transport.c|1 + 
> > >  usr/transport.h|3 +++ 
> > >  usr/uip_mgmt_ipc.c |   38 
> > +- 
> > >  usr/uip_mgmt_ipc.h |   13 + 
> > >  7 files changed, 119 insertions(+), 14 deletions(-) 
> > > 
> > > diff --git a/usr/iscsiadm.c b/usr/iscsiadm.c index aa7cf07..495af3a 
> > > 100644 
> > > --- a/usr/iscsiadm.c 
> > > +++ b/usr/iscsiadm.c 
> > > @@ -3099,10 +3099,10 @@ static char *iscsi_ping_stat_strs[] = { 
> > >  "No ARP response received", 
> > >  }; 
> > > 
> > > -static char *iscsi_ping_stat_to_str(uint32_t status) 
> > > +static char *iscsi_ping_stat_to_str(int status) 
> > >  { 
> > >  if (status < 0 || status > ISCSI_PING_NO_ARP_RECEIVED) { 
> > > -log_error("Invalid ping status %u", status); 
> > > +log_error("Ping error: %s\n", strerror(status)); 
> > 
> > "status" does not look very much like "errno", but isn't strerror() 
> defined for 
> > errno values? 
> > 
> > 
>
> "status" is used to get the status of ping from iscsiuio. 
> It will either contain the defined ping status or errno if ping fails due 
> to some other problem. 
> So, strerror() is used only when status holds some errno . 
> And iscsi_ping_stat_strs[status] is used to return corresponding string 
> for ping status code. 
>
>
I would suggest adding a comment to that effect, then, since many might 
have the same question.
 

> > >  return NULL; 
> > >  } 
> > > 
> > [...] 
> > 
> > Regards, 
> > Ulrich 
> > 
> > 
>

-- 
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 open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.


RE: Antw: [PATCH v2 1/2] iscsid: Changes to support ping through iscsiuio

2015-07-12 Thread Adheer Chandravanshi


> -Original Message-
> From: open-iscsi@googlegroups.com [mailto:open-iscsi@googlegroups.com]
> On Behalf Of Ulrich Windl
> Sent: Friday, July 10, 2015 11:32 AM
> Cc: open-iscsi
> Subject: Antw: [PATCH v2 1/2] iscsid: Changes to support ping through
> iscsiuio
> 
> >>>  schrieb am 09.07.2015 um 13:38 in
> >>> Nachricht
> <1436441896-7161-2-git-send-email-adheer.chandravan...@qlogic.com>:
> > From: Adheer Chandravanshi 
> >
> > These changes are done in order to support ping operation for drivers
> > like bnx2i that use iscsiuio.
> >
> > Signed-off-by: Adheer Chandravanshi
> 
> > ---
> >  usr/iscsiadm.c |   38 +++---
> >  usr/iscsid_req.c   |   38 +-
> >  usr/iscsid_req.h   |2 +-
> >  usr/transport.c|1 +
> >  usr/transport.h|3 +++
> >  usr/uip_mgmt_ipc.c |   38
> +-
> >  usr/uip_mgmt_ipc.h |   13 +
> >  7 files changed, 119 insertions(+), 14 deletions(-)
> >
> > diff --git a/usr/iscsiadm.c b/usr/iscsiadm.c index aa7cf07..495af3a
> > 100644
> > --- a/usr/iscsiadm.c
> > +++ b/usr/iscsiadm.c
> > @@ -3099,10 +3099,10 @@ static char *iscsi_ping_stat_strs[] = {
> > "No ARP response received",
> >  };
> >
> > -static char *iscsi_ping_stat_to_str(uint32_t status)
> > +static char *iscsi_ping_stat_to_str(int status)
> >  {
> > if (status < 0 || status > ISCSI_PING_NO_ARP_RECEIVED) {
> > -   log_error("Invalid ping status %u", status);
> > +   log_error("Ping error: %s\n", strerror(status));
> 
> "status" does not look very much like "errno", but isn't strerror() defined 
> for
> errno values?
> 
> 

"status" is used to get the status of ping from iscsiuio.
It will either contain the defined ping status or errno if ping fails due to 
some other problem.
So, strerror() is used only when status holds some errno .
And iscsi_ping_stat_strs[status] is used to return corresponding string for 
ping status code.

> > return NULL;
> > }
> >
> [...]
> 
> Regards,
> Ulrich
> 
> 
> --
> 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 open-iscsi+unsubscr...@googlegroups.com.
> To post to this group, send email to open-iscsi@googlegroups.com.
> Visit this group at http://groups.google.com/group/open-iscsi.
> For more options, visit https://groups.google.com/d/optout.

-- 
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 open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at http://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.