Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-22 Thread Ulrich Windl
On 21 Jan 2009 at 16:36, Konrad Rzeszutek wrote: > > On Wed, Jan 21, 2009 at 11:48:41AM +0100, Ulrich Windl wrote: > > > > On 20 Jan 2009 at 9:23, Konrad Rzeszutek wrote: > > > > > I would recommend that you provide as the first variable in all of the > > > structs > > > an unsigned int calle

Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-21 Thread Konrad Rzeszutek
On Wed, Jan 21, 2009 at 11:48:41AM +0100, Ulrich Windl wrote: > > On 20 Jan 2009 at 9:23, Konrad Rzeszutek wrote: > > > I would recommend that you provide as the first variable in all of the > > structs > > an unsigned int called 'version'. This way if the structs are extended they > > would be

Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-21 Thread Ulrich Windl
On 20 Jan 2009 at 9:23, Konrad Rzeszutek wrote: > I would recommend that you provide as the first variable in all of the structs > an unsigned int called 'version'. This way if the structs are extended they > would be backwards compatible and there is an easy way to identify which > version of st

Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-21 Thread Hans de Goede
Hans de Goede wrote: > On Tue, Jan 20, 2009 at 07:58:07PM +0100, Hans de Goede wrote: > > > > Hi, > > > > Konrad Rzeszutek wrote: > > > > Thanks for the review! > > > > > I presume you have run this program (and the test-code) through > > > valgrind with no memory leaks? >

Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-20 Thread Konrad Rzeszutek
On Tue, Jan 20, 2009 at 07:58:07PM +0100, Hans de Goede wrote: > > Hi, > > Konrad Rzeszutek wrote: > > Thanks for the review! > > > I presume you have run this program (and the test-code) through > > valgrind with no memory leaks? > > > > Erm, no, has iscsiadm been run through valgrind? If

Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-20 Thread Konrad Rzeszutek
On Tue, Jan 20, 2009 at 07:40:08PM +0100, Bart Van Assche wrote: > On Tue, Jan 20, 2009 at 7:20 PM, Konrad Rzeszutek > wrote: > > > >> > I would recommend that you provide as the first variable in all of the > >> > structs > >> > an unsigned int called 'version'. This way if the structs are exte

Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-20 Thread Hans de Goede
Hi, Konrad Rzeszutek wrote: Thanks for the review! > I presume you have run this program (and the test-code) through > valgrind with no memory leaks? > Erm, no, has iscsiadm been run through valgrind? If not I'm not going to be running libiscsi through it either (sorry) libiscsi builds on

Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-20 Thread Bart Van Assche
On Tue, Jan 20, 2009 at 7:20 PM, Konrad Rzeszutek wrote: > >> > I would recommend that you provide as the first variable in all of the >> > structs >> > an unsigned int called 'version'. This way if the structs are extended they >> > would be backwards compatible and there is an easy way to iden

Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-20 Thread Konrad Rzeszutek
> > I would recommend that you provide as the first variable in all of the > > structs > > an unsigned int called 'version'. This way if the structs are extended they > > would be backwards compatible and there is an easy way to identify which > > version of structs they are. > > > > Erm, given

Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-20 Thread Hans de Goede
Konrad Rzeszutek wrote: > On Mon, Jan 19, 2009 at 02:07:23PM +0100, Hans de Goede wrote: >> Hi All, >> >> Short intro: I'm a long time Linux an and developer. Since Sept 1st I work >> for >> RedHat on the installer team (anaconda the installer for Fedora and RHEL). >> >> We currently make all

Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-20 Thread Konrad Rzeszutek
NACK. I presume you have run this program (and the test-code) through valgrind with no memory leaks? Please see my comments below. > diff -urN open-iscsi-2.0-870.1.orig/libiscsi/libiscsi.c > open-iscsi-2.0-870.1/libiscsi/libiscsi.c > --- open-iscsi-2.0-870.1.orig/libiscsi/libiscsi.c 1970-0

Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-20 Thread Hans de Goede
Bart Van Assche wrote: > On Mon, Jan 19, 2009 at 4:35 PM, Hans de Goede wrote: >>> - libiscsi_discover_sendtargets - maybe (very maybe) the "int port" could >>> be dropped and >>> "const char *address" could be of the >>> form "address_or_host[:port]". >>>

Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-19 Thread Ulrich Windl
On 19 Jan 2009 at 17:16, Boaz Harrosh wrote: [...] > - libiscsi_discover_sendtargets - maybe (very maybe) the "int port" could be > dropped and > "const char *address" could be of the form > "address_or_host[:port]". > Regarding

Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-19 Thread Bart Van Assche
On Mon, Jan 19, 2009 at 4:35 PM, Hans de Goede wrote: >> - libiscsi_discover_sendtargets - maybe (very maybe) the "int port" could be >> dropped and >> "const char *address" could be of the form >> "address_or_host[:port]". >> Re

Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-19 Thread Hans de Goede
Boaz Harrosh wrote: > Hans de Goede wrote: >> Hi All, >> >> The API currently offers pretty minimal functionality (just what we need in >> anaconda) I'm fine with extending this (patches welcome). But currently I >> would >> like to focus on the set of functionality as the current API offers

Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-19 Thread Boaz Harrosh
Hans de Goede wrote: > Hi All, > > The API currently offers pretty minimal functionality (just what we need in > anaconda) I'm fine with extending this (patches welcome). But currently I > would > like to focus on the set of functionality as the current API offers and try > to > get that rig

Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-19 Thread Hans de Goede
Bart Van Assche wrote: > On Mon, Jan 19, 2009 at 2:07 PM, Hans de Goede wrote: >> Therefore we would like to export (some) of the functionality of iscsiadm as >> a >> C-library. > > Great ! > >> I've got documentation of the proposed API here: >> http://people.atrpms.net/~hdegoede/html/libis

Re: PATCH: make open-iscsi userspace tools functionality available as a library

2009-01-19 Thread Bart Van Assche
On Mon, Jan 19, 2009 at 2:07 PM, Hans de Goede wrote: > Therefore we would like to export (some) of the functionality of iscsiadm as a > C-library. Great ! > I've got documentation of the proposed API here: > http://people.atrpms.net/~hdegoede/html/libiscsi_8h.html Not so great: libiscsi_get_