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 called 'version'.

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? Erm, no,

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

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 backwards

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 hdego...@redhat.com 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-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

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 the amount

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 kon...@virtualiron.com 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

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 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 kon...@virtualiron.com 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

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 not I'm not

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 hdego...@redhat.com 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

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 hdego...@redhat.com 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:

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 and try to

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 hdego...@redhat.com 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 defaults