Mailing List Archive: Linux-HA: Dev

[PATCH 0 of 3] iSCSITarget, iSCSILogicalUnit: several improvements

Index | Next | Previous | View Threaded

florian.haas at linbit

Jun 23, 2009, 6:01 AM

Post #1 of 6 (611 views)
Permalink

[PATCH 0 of 3] iSCSITarget, iSCSILogicalUnit: several improvements

A set of incremental improvements to the iSCSITarget and
iSCSILogicalUnit RAs:

- iSCSITarget: replace the ugly "while read" loop on connection
shutdown with something prettier.

- iSCSITarget: add ACL support to allow access by initiator IP
address. On IET, this manages entries in the
/etc/initiators.{allow,deny} files. On tgt, it allows initiators
using the "tgtadm --op bind" commant.

- iSCSILogicalUnit: add support for per-LU parameters.

Questions: should I rename the instance attribute "params" to
"parameters", so as to avoid confusion with the CRM shell's "params"
command? And, should the "initiators" instance attribute be renamed to
"allowed-initiators" to remove ambiguity?

As always, feedback and comments are much appreciated.

Thanks!

Cheers,
Florian
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev[at]lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

dejanmm at fastmail

Jun 29, 2009, 2:31 AM

Post #2 of 6 (545 views)
Permalink

Re: [PATCH 0 of 3] iSCSITarget, iSCSILogicalUnit: several improvements [In reply to]

Hi,

On Tue, Jun 23, 2009 at 03:01:56PM +0200, Florian Haas wrote:
> A set of incremental improvements to the iSCSITarget and
> iSCSILogicalUnit RAs:
>
> - iSCSITarget: replace the ugly "while read" loop on connection
> shutdown with something prettier.
>
> - iSCSITarget: add ACL support to allow access by initiator IP
> address. On IET, this manages entries in the
> /etc/initiators.{allow,deny} files. On tgt, it allows initiators
> using the "tgtadm --op bind" commant.
>
> - iSCSILogicalUnit: add support for per-LU parameters.

Applied.

> Questions: should I rename the instance attribute "params" to
> "parameters", so as to avoid confusion with the CRM shell's "params"
> command?

Yes, probably. The parser may even become nervous ;-)

> And, should the "initiators" instance attribute be renamed to
> "allowed-initiators" to remove ambiguity?

Yes. BTW, it won't work if the configuration's changed back from
some to none, so perhaps you should remove those files in that
case.

Cheers,

Dejan

> As always, feedback and comments are much appreciated.
>
> Thanks!
>
> Cheers,
> Florian
> _______________________________________________________
> Linux-HA-Dev: Linux-HA-Dev[at]lists.linux-ha.org
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev[at]lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

florian.haas at linbit

Jun 29, 2009, 2:41 AM

Post #3 of 6 (543 views)
Permalink

Re: [PATCH 0 of 3] iSCSITarget, iSCSILogicalUnit: several improvements [In reply to]

Dejan,

On 2009-06-29 11:31, Dejan Muhamedagic wrote:
> Hi,
>
> On Tue, Jun 23, 2009 at 03:01:56PM +0200, Florian Haas wrote:
>> A set of incremental improvements to the iSCSITarget and
>> iSCSILogicalUnit RAs:
>>
>> - iSCSITarget: replace the ugly "while read" loop on connection
>> shutdown with something prettier.
>>
>> - iSCSITarget: add ACL support to allow access by initiator IP
>> address. On IET, this manages entries in the
>> /etc/initiators.{allow,deny} files. On tgt, it allows initiators
>> using the "tgtadm --op bind" commant.
>>
>> - iSCSILogicalUnit: add support for per-LU parameters.
>
> Applied.

Thanks.

>> Questions: should I rename the instance attribute "params" to
>> "parameters", so as to avoid confusion with the CRM shell's "params"
>> command?
>
> Yes, probably. The parser may even become nervous ;-)

Will do.

>> And, should the "initiators" instance attribute be renamed to
>> "allowed-initiators" to remove ambiguity?
>
> Yes. BTW, it won't work if the configuration's changed back from
> some to none, so perhaps you should remove those files in that
> case.

You lost me there. What files are you referring to? And what is it
exactly that won't work?

Cheers,
Florian

Attachments:

signature.asc (0.25 KB)

dejanmm at fastmail

Jun 29, 2009, 2:57 AM

Post #4 of 6 (544 views)
Permalink

Re: [PATCH 0 of 3] iSCSITarget, iSCSILogicalUnit: several improvements [In reply to]

Hi,

On Mon, Jun 29, 2009 at 11:41:52AM +0200, Florian Haas wrote:
> Dejan,
>
> On 2009-06-29 11:31, Dejan Muhamedagic wrote:
> > Hi,
> >
> > On Tue, Jun 23, 2009 at 03:01:56PM +0200, Florian Haas wrote:
> >> A set of incremental improvements to the iSCSITarget and
> >> iSCSILogicalUnit RAs:
> >>
> >> - iSCSITarget: replace the ugly "while read" loop on connection
> >> shutdown with something prettier.
> >>
> >> - iSCSITarget: add ACL support to allow access by initiator IP
> >> address. On IET, this manages entries in the
> >> /etc/initiators.{allow,deny} files. On tgt, it allows initiators
> >> using the "tgtadm --op bind" commant.
> >>
> >> - iSCSILogicalUnit: add support for per-LU parameters.
> >
> > Applied.
>
> Thanks.
>
> >> Questions: should I rename the instance attribute "params" to
> >> "parameters", so as to avoid confusion with the CRM shell's "params"
> >> command?
> >
> > Yes, probably. The parser may even become nervous ;-)
>
> Will do.
>
> >> And, should the "initiators" instance attribute be renamed to
> >> "allowed-initiators" to remove ambiguity?
> >
> > Yes. BTW, it won't work if the configuration's changed back from
> > some to none, so perhaps you should remove those files in that
> > case.
>
> You lost me there. What files are you referring to? And what is it
> exactly that won't work?

Sorry. The /etc/initiators.deny and such. If the user first
specifies some initiators and runs the resource, then later
changes it to empty, the files won't change, i.e. the "none" case
is not enforced. Which also gets me thinking about multiple
iSCSITarget instances. Perhaps you need a more elaborate scheme
here. Or perhaps just give up on it and tell users to set it
themselves? :)

Cheers,

Dejan

> Cheers,
> Florian
>

> _______________________________________________________
> Linux-HA-Dev: Linux-HA-Dev[at]lists.linux-ha.org
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/

_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev[at]lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

florian.haas at linbit

Jun 29, 2009, 3:05 AM

Post #5 of 6 (544 views)
Permalink

Re: [PATCH 0 of 3] iSCSITarget, iSCSILogicalUnit: several improvements [In reply to]

On 2009-06-29 11:57, Dejan Muhamedagic wrote:
>>> Yes. BTW, it won't work if the configuration's changed back from
>>> some to none, so perhaps you should remove those files in that
>>> case.
>> You lost me there. What files are you referring to? And what is it
>> exactly that won't work?
>
> Sorry. The /etc/initiators.deny and such. If the user first
> specifies some initiators and runs the resource, then later
> changes it to empty, the files won't change, i.e. the "none" case
> is not enforced.

The resource agent does not implement reload, so on reconfiguration the
resource will get stopped with the old config (which will remove entries
in /etc/initiators,{allow,deny}) and then restarted with the new
configuration (where, by default, no access restrictions are put in place).

At least that's the way it works for me, with Pacemaker 1.0.4. Am I
missing something? Or does this work only accidentally?

Florian

Attachments:

signature.asc (0.25 KB)

dejanmm at fastmail

Jun 29, 2009, 7:20 AM

Post #6 of 6 (541 views)
Permalink

Re: [PATCH 0 of 3] iSCSITarget, iSCSILogicalUnit: several improvements [In reply to]

On Mon, Jun 29, 2009 at 12:05:50PM +0200, Florian Haas wrote:
> On 2009-06-29 11:57, Dejan Muhamedagic wrote:
> >>> Yes. BTW, it won't work if the configuration's changed back from
> >>> some to none, so perhaps you should remove those files in that
> >>> case.
> >> You lost me there. What files are you referring to? And what is it
> >> exactly that won't work?
> >
> > Sorry. The /etc/initiators.deny and such. If the user first
> > specifies some initiators and runs the resource, then later
> > changes it to empty, the files won't change, i.e. the "none" case
> > is not enforced.
>
> The resource agent does not implement reload, so on reconfiguration the
> resource will get stopped with the old config (which will remove entries
> in /etc/initiators,{allow,deny}) and then restarted with the new
> configuration (where, by default, no access restrictions are put in place).
>
> At least that's the way it works for me, with Pacemaker 1.0.4. Am I
> missing something? Or does this work only accidentally?

Didn't test it myself, it just looked so in the code and
according to the documentation. But I probably misunderstood.

Cheers,

Dejan

> Florian
>

> _______________________________________________________
> Linux-HA-Dev: Linux-HA-Dev[at]lists.linux-ha.org
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/

_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev[at]lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact lists@gossamer-threads.com