Bug 643302 - some enhancements to tgt's configuration files
Summary: some enhancements to tgt's configuration files
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: scsi-target-utils
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Pádraig Brady
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 861249
TreeView+ depends on / blocked
 
Reported: 2010-10-15 08:33 UTC by Satoru SATOH
Modified: 2012-12-18 16:26 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 861249 (view as bug list)
Environment:
Last Closed: 2012-11-16 07:36:37 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
example of rpm spec changes (1.39 KB, patch)
2010-10-15 08:35 UTC, Satoru SATOH
no flags Details | Diff
modified version of targets.conf: all comments of target examples are removed. (488 bytes, text/plain)
2010-10-15 08:38 UTC, Satoru SATOH
no flags Details
/etc/tgt/conf.d/sample.conf: sample target configurations (6.10 KB, text/plain)
2010-10-15 08:39 UTC, Satoru SATOH
no flags Details
allow conf.d files to include directories (1.08 KB, patch)
2012-09-26 23:26 UTC, Pádraig Brady
no flags Details | Diff
possible fix to comment 20 (341 bytes, patch)
2012-11-06 23:34 UTC, Pádraig Brady
no flags Details | Diff
fix for comment 20 (1.15 KB, patch)
2012-11-07 12:46 UTC, Pádraig Brady
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
CPAN 79869 0 None None None Never

Description Satoru SATOH 2010-10-15 08:33:51 UTC
Description of problem:

It is possible to use the "include" directive in /etc/tgt/targets.conf to load custom configs other than /etc/tgt/targets.conf and there is a line "... include ..." in it.

Unfortunately, the line is written as just a comment and users have to write the line anyhow to add their configuration files to customize.


Instead, how about changing it like followings?

0. add the line 'include /etc/tgt/conf.d/*.conf' in targets.conf
1. create /etc/tgt/conf.d by default.

and optionally

2. add /etc/tgt/conf.d/sample.conf (its content came from targets.conf in RPM)


And, users only have to add their custom config files to add iscsi targets and there is no need to modify files provided by the scsi-target-utils' RPM at all.

Also it would be possible that users make their custom rpm only contains custom configuration files under /etc/tgt/conf.d and all they have to do is to install the RPM to complete the iscsi target setup.

I will post some example files to accomplish this.



Version-Release number of selected component (if applicable):
1.0.1-3.fc13.x86_64 (but I guess it should be applicable to others in F14/RHEL 6+).

Comment 1 Satoru SATOH 2010-10-15 08:35:33 UTC
Created attachment 453673 [details]
example of rpm spec changes

Comment 2 Satoru SATOH 2010-10-15 08:38:48 UTC
Created attachment 453675 [details]
modified version of targets.conf: all comments of target examples are removed.

Comment 3 Satoru SATOH 2010-10-15 08:39:57 UTC
Created attachment 453677 [details]
/etc/tgt/conf.d/sample.conf: sample target configurations

Comment 4 Fedora Admin XMLRPC Client 2011-06-30 22:03:45 UTC
This package has changed ownership in the Fedora Package Database.  Reassigning to the new owner of this component.

Comment 5 Andy Grover 2011-12-20 21:06:15 UTC
lets get this in F17.

Comment 6 Andy Grover 2012-08-07 23:53:54 UTC
Thanks Satoh-san, rpm updated and building for rawhide (F18) now.

Comment 7 Pádraig Brady 2012-09-20 12:29:04 UTC
Any chance we could backport this to F17 and RHEL6 ?
It would simplify OpenStack Folsom installation for one thing.

Comment 8 Andy Grover 2012-09-20 21:11:17 UTC
F17 sure. Do you by any chance want to handle that one?

RHEL6 more difficult, we may want to clone this bug and handle separately if this is something that OpenStack would really like.

Comment 9 Pádraig Brady 2012-09-20 22:47:06 UTC
OK I'll have a look at updating F17.

OpenStack would really like to:

  include /var/lib/cinder/volumes/*
  include /var/lib/nova/volumes/*

However adding that to /etc/tgt/targets.conf on package install/upgrade is very awkward. Do you perhaps have tricks to easily enable that without supporting /etc/tgt/conf.d/ ? Otherwise it would be really nice to have on RHEL.

thanks.

Comment 10 Fedora Update System 2012-09-21 00:23:20 UTC
scsi-target-utils-1.0.24-4.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/scsi-target-utils-1.0.24-4.fc17

Comment 11 Satoru SATOH 2012-09-21 02:05:39 UTC
This may be a off topic, but:

(In reply to comment #9)
> OK I'll have a look at updating F17.
> 
> OpenStack would really like to:
> 
>   include /var/lib/cinder/volumes/*
>   include /var/lib/nova/volumes/*
> 
> However adding that to /etc/tgt/targets.conf on package install/upgrade is
> very awkward. Do you perhaps have tricks to easily enable that without
> supporting /etc/tgt/conf.d/ ? Otherwise it would be really nice to have on
> RHEL.
> 
> thanks.

How about nesting includes ? That is, for example,

/etc/tgt/targets.conf:

include /etc/tgt/conf.d/*.conf


/etc/tgt/conf.d/cinder.conf:

include /etc/cinder/volumes.d/*.conf


/etc/tgt/conf.d/nova.conf:

include /etc/nova/volumes.d/*.conf


I'm not sure if tgt-admin (perl's Config::General module) supports
nesting includes but it looks so:
  http://search.cpan.org/dist/Config-General/General.pm

Comment 12 Pádraig Brady 2012-09-21 09:11:56 UTC
Yep that's what I intend to do on F17 and F18.

My question was directed to installations that didn't support /etc/tgt/conf.d
I.E. it looks very awkward, so it would be great to support this config option
automatically on RHEL 6 too.

thanks.

Comment 13 Fedora Update System 2012-09-23 23:50:18 UTC
scsi-target-utils-1.0.24-4.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 14 Pádraig Brady 2012-09-26 23:26:48 UTC
Created attachment 617838 [details]
allow conf.d files to include directories

When testing this it was noticed that perl's Config::General does not recursively expand globs, only doing so on the first level. I've added a query about that at:
https://rt.cpan.org/Public/Bug/Display.html?id=79869

In the meantime though we could still support the setup detailed in comment 11
by enabling the IncludeDirectories option as done in the attached patch.

Would it be OK if I updated Fedora 17 and Fedora 18 with that tweak?

Comment 15 Andy Grover 2012-09-27 01:06:36 UTC
That change looks good, but there's another issue I'd also like to fix, to put this all firmly to rest.

I'm concerned that if the system admin has modified targets.conf then the new targets.conf with the new include directive will be installed as .rpmnew, and thus stuff in tgt/conf.d/* won't get picked up. Perhaps we need to have tgt-admin actually start with a stub .conf that just includes tgt/targets.conf and tgt/conf.d/* to ensure this doesn't happen?

Comment 16 Pádraig Brady 2012-09-27 08:26:25 UTC
comment 15 sounds good to me.
Supports upgrade and traditional scripts that munge targets.conf

Comment 17 Fedora Update System 2012-09-27 17:18:58 UTC
scsi-target-utils-1.0.30-4.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/scsi-target-utils-1.0.30-4.fc18

Comment 18 Fedora Update System 2012-09-27 17:20:07 UTC
scsi-target-utils-1.0.24-5.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/scsi-target-utils-1.0.24-5.fc17

Comment 19 Fedora Update System 2012-09-28 08:23:46 UTC
Package scsi-target-utils-1.0.24-5.fc17:
* should fix your issue,
* was pushed to the Fedora 17 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing scsi-target-utils-1.0.24-5.fc17'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-14975/scsi-target-utils-1.0.24-5.fc17
then log in and leave karma (feedback).

Comment 20 Attila Fazekas 2012-11-06 21:09:12 UTC
Now, I have scsi-target-utils-1.0.32-1.fc18.x86_64.

Looks like the tgt-admin --execute (or --update) starts the parsing (as expected by most documentation) with the tgt/targets.conf and in this way it will never read the /etc/tgt/conf.d/*.

When restart the tgtd it starts with the tgt/tgtd.conf and read everything, but it is too late for cinder..

As a workaround now I have just one line in the tgtd.conf which includes the targets.conf which includes the /etc/tgt/conf.d/*.conf ... pretty long chain :)

I think if we change the tgt-admin behavior to start with the tgtd.conf causes more confusion, since /etc/tgt/targets.conf expected by most users.

Comment 21 Pádraig Brady 2012-11-06 23:34:04 UTC
Created attachment 639693 [details]
possible fix to comment 20

Thanks for looking into that Attila.
So is systemd using stale settings on update?
Perhaps this is addressed with this patch?

Comment 22 Attila Fazekas 2012-11-07 12:06:34 UTC
The systemd using the correct IMHO.

When you strace -Ff -e open tgt-admin --execute you can see does it read the cinders config files (2>&1 | grep conf[.]d) or not.
I expect it to read it. This is important.

The cinder uses the --update , which implies config re-parse like with --execute.
cinder does not restart the tgtd. I just saw a _manual_ restart caused only to read the cinder configs, but I expect it reread without touching the systemd.

The tgt-admin --update <..> MUST reread the cinder configs. Cinder expects it.

If you need more detailed examples, let me know.

Comment 23 Pádraig Brady 2012-11-07 12:46:26 UTC
Created attachment 640036 [details]
fix for comment 20

Oh, tgt-admin itself hardcodes /etc/tgt/targets.conf rather than reading the /etc/sysconfig value. That was unexpected. This patch should get it working.

Comment 24 Fedora Update System 2012-11-07 14:37:36 UTC
scsi-target-utils-1.0.32-2.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/scsi-target-utils-1.0.32-2.fc18

Comment 25 Fedora Update System 2012-11-07 14:38:15 UTC
scsi-target-utils-1.0.24-6.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/scsi-target-utils-1.0.24-6.fc17

Comment 26 Fedora Update System 2012-11-12 02:54:23 UTC
scsi-target-utils-1.0.32-2.fc18 has been pushed to the Fedora 18 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 27 Fedora Update System 2012-11-16 07:36:40 UTC
scsi-target-utils-1.0.24-6.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.