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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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 get that right. Most important here is to have a clean, clear and usable API which is also future proof, as I want to freeze the ABI of the available functions asap. WRT this I would especially like feedback on the futrue proofness of the libiscsi_node struct.


First of all Thank you very much for doing this. I'm sure it will prove
very useful in the future, the issue has been raised in the passed, multiple
times.

I can see that you have not attempted to refactor iscsiadm so to use the library.
Which means you will be shipping the same exact code twice.

If you look at the code you will see that there actually is not all that much duplication.

Is your long term
plan to do that once libiscsi is mature enough, or you would rather keep these
two separate and duplicated?


To keep them separate, see above.


Regarding the API:
- The overall state and division of labor looks very nice. :-)

- 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 and all that.


Nah, that is ok for a cmdline interface, but cumbersome for an API, we could do something where port == 0 would choose the default port though.

- libiscsi_discover_isns - Looks like it is missing the actual input parameters usually
                           a "char *iqn" I think?


I've checked the (AFAIK currently incomplete) iscsiadm code and that does not have any parameters. Given that this is pretty much a pie in the sky, we could just remove this function completely for now.

- libiscsi_discover_firmware - What is that for? Also missing an input parameter.


This reads iscsi target info (portal and authentication info) from the BIOS of machines who support booting from iscsi, there is no input parameter, as machines tend to have only one BIOS (to query).

- I can't help noticing the missing of query functions, Both query of DB of discovered
  Nodes, as well as info of logged-in nodes.

Ack, those are not needed by anaconda, but should probably be added to make this more generally usefull, patches welcome :)

- libiscsi_node_set_parameter - Question: (Sorry have not read the code) Is that persistent,
                                gets saved in DB. Or is just for current session/login?


Actually it only changes the database, so it will not influence parameters of the current session. I guess this needs better documentation making this clear.

Regards,

Hans

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux