On Sun, 18 Sep 2016 17:29:16 +0200, Christian Seiler wrote :
Hi again,
this is my review of the targetcli-fb packaging.
[snip]
I agree with all your changes.
That all said, there's one major issue that isn't resolved
here:
There's no startup logic! I see neither init script nor systemd
service file. There was something upstream (that was minimally
patched in Debian) in the old version, but that's gone now, as
far as I can tell. In that case, the correct course of action for
Debian packaging would be to add this in the debian/ directory and
install it from there. I don't have time to fix that today, and I
think this is something that can wait until after the packaging
is imported into Alioth. But just to keep in mind that this needs
to be done before the package can enter Debian.
As you observe, there is no startup logic in targetcli-fb. The startup
logic is provided by rtslib-fb which contains a utility named
"targetctl".
I would describe targetctl as a trimmed down version of targetcli.
It accepts only three commands to restore, save or clear the
configuration. Moreover, it is not interactive so it does not depend on
configshell-fb.
https://github.com/open-iscsi/rtslib-fb/blob/master/scripts/targetctl
In Debian, the python-rtslib-fb package maintained by Thomas Goirand as
part of Open Stack contains the targetctl utility. The package also
contains an init script but no systemd service file.
http://git.openstack.org/cgit/openstack/deb-python-rtslib-fb/tree/debian/...
Anyway, if I hear positive feedback from both you (Christophe) and
Ritesh about getting this onto Alioth ASAP, I'll just rebase your
commits on the actual upstream version (and not previous git master)
when adding your commits to the packaging repository.
My feedback is definitely positive.
With best regards,
--
Christophe Vu-Brugier