[PKG-Openstack-devel] Changes I have in mind for the python-rtslib-fb package in Debian

[Christian Seiler]

(Adding the openstack alioth list to CC.)

On 09/13/2016 11:36 AM, Christophe Vu-Brugier wrote:
> I would like to discuss with you some changes for the python-rtslib-fb
> package you maintain in Debian.
> 
> As you may know, I am trying to help with packaging targetcli-fb in
> Debian. Since targetcli-fb depends on rtslib-fb, I looked at your
> python-rtslib-fb package and already submitted some very simple
> patches. I have larger changes in mind and I would like to discuss
> them with you before implementing them eventually.

As I'll be co-maintaining the configshell-fb and targetcli-fb
packages with Christophe and Ritesh, I'd also like to add a
few comments from my perspective:

> 1. Python library / Python program split
> 
> rtslib-fb is a Python library and a Python program, targetctl, that
> uses the library to save on disk or restore from disk the
> configuration of the "target" driver in Linux.
> 
> Today, the python-rtslib-fb and python3-rtslib-fb packages contain the
> library and targetctl built for Python2 or Python3. Additionally, the
> python-rtslib-fb package contain an init script.
> 
> As a consequence, python3-rtslib-fb depends on python-rtslib-fb for
> the init script. Having a python3-foo package depend on a python-foo
> package is unusual: aside from python3-rtslib-fb, I found only two
> packages doing that (python3-cxx-dev and python3-regex).
> 
> Moreover, since targetctl is either targetctl-python2 or
> targetctl-python3, the packages use `update-alternatives` to make
> targetctl point to one of them.
> 
> Instead of that, I suggest splitting the source package as follows:
>  * python-rtslib-fb would contain only the Python 2 library
>  * python3-rtslib-fb would contain only the Python 3 library
>  * a new package (named "targetctl" or "target-service", ...) would
>    contain the targetctl Python program, the init script and some
>    additional directories (see section 2).
> 
> The "targetctl" package would be built for a single version of
> Python. I think building for Python 3 would be more future-proof.

I completely agree with this.

> 2. Missing directories expected by the Linux kernel
> 
> The "target" driver in the Linux kernel stores some information on
> disk so that it persists across reboots.
> 
> Information about "Persistent Reservations" is stored in /var/target/pr
> and information about "Asymmetric Logical Unit Access" is stored
> in /var/target/alua.
> 
> If these directories are missing then the persistent reservations or
> ALUA features fail. The link below describes one such failure.
> 
>   http://target-devel.vger.kernel.narkive.com/Ujpp6wSs/pr-registration-on-lio-iscsi-returns-sense-key-not-ready
> 
> I suggest that the package providing `targetctl` also create the
> /var/target/pr and /var/target/alua when installed.

I don't like the fact that something like /var/target is hardcoded in
the kernel, but as it is, I would also agree here that these paths
should be created by the corresponding package for now.

One could of course have a discussion with upstream on how to solve
this in a better way in the future, but for now I think we should be
able to provide this functionality within Debian.

> 3. Debian patch to store saveconfig.json to /etc/rtslib-fb-target/
> 
> Debian changes the default location where the saveconfig.json file is
> retrieved/stored from /etc/target/ to /etc/rtslib-fb-target/. The
> `targetcli` command also manipulates this file, so either we patch
> targetcli-fb to retrieve/store saveconfig.json in
> /etc/rtslib-fb-target as well or we drop the patch from the
> python-rtslib-fb package.
> 
> I understand your concern that /etc/target is too generic, but a lot
> of things related to the LIO iSCSI target are named after "target":
> the "driver/target/" directory in the Linux kernel source code, the
> "target-devel" mailing list, the "targetctl" and "targetcli"
> utilities...
> 
> So I would favor dropping this patch to behave the same as
> targetctl/targetcli upstream.
> 
> That means being able to move /etc/rtslib-fb-target/saveconfig.json to
> /etc/target/saveconfig.json during an upgrade.

I also think that changing the name to /etc/rtslib-fb-target is not
ideal at the moment, especially since this diverges from upstream.
OTOH, I understand that /etc/target might seem a bit too generic. So
maybe we could talk to upstream and have the standard path be renamed
to /etc/lio-target-fb? (With a proper fallback logic if that doesn't
exist but the previous path does.) I personally think that that would
be far more descriptive as compared to rtslib-fb-target.

I'd leave it as-is for the moment (changing it multiple times is
going to be too much of a mess), but have discussion with upstream to
perhaps change that.

> 4. Integration with systemd
> 
> I think it would be nice to provide a systemd unit file in addition to
> the sysv init script.

I think that's something I'd like to have in the long term, but not
very urgent, as it's not an rcS init script, and thus supported by
systemd just fine at the moment.

I'm more worried about the 'sleep 10' in the init script as that will
delay boot and shutdown, and I don't grok why that's in there. Any
reason?

> You are probably busy tackling other tasks and I don't want to consume
> too much of your time. However, the changes I mention may imply
> implementing transitions during a package upgrade (e.g. moving a
> configuration file, changing packages that depend on python-rtslib-fb
> to add a dependency to a potential "targetctl" package, etc.). Since I
> am not familiar with this, I may ask for some guidance.
> 
> Also, I know nothing about OpenStack and debian-openstack release
> schedule: it may not be the right time to implement the above changes.
> 
> As a result, hearing me pushing for these changes may sound scary :)
> But be sure that I don't want to rush this work nor consume too much
> of your time.

I'll let this stand here because I also can't comment on it.

OTOH, I think both Christophe and I could contribute to this
package (because our packages depend on it), and most changes
we had in mind (apart from the package split mentioned in
this email) won't have any real impact on OpenStack. So maybe
we could join the Alioth openstack team and start to co-
maintain the rtslib-fb package with you? It appears that
rtslib-fb has only one rdep in openstack (cinder), so I think
the only change that needs some coordination (the split of
the package to have separate packages for the Python module
one the one hand, and the CLI utility and the init script on
the other hand) is not going to be too difficult to manage.

What do you think?

Regards,
Christian