On Sun, Feb 26, 2012 at 1:19 PM, Florian Haas <[email protected]> wrote: > On Sat, Feb 25, 2012 at 10:02 AM, Rasto Levrinc <[email protected]> > wrote: >> Hi, >> >> There are couple of issues in iSCSITarget meta-data. >> >> 1. there is a extra space in name attribute in status action and it >> causes all sort of problems: >> >> <action name="status " <-- >> >> what it does, is that crm shell doesn't understand "status " action only >> "status", but cib doesn't accept "status". So I can't simply fix it in >> the GUI. > > Fixed in the metadata: > > https://github.com/ClusterLabs/resource-agents/commit/ebb5e5d103066cb19b46427ef0f28937f4943dbe >
Thanks. > However, iirc RAs expose "status" operation only for age-old > compatibility reasons, and Pacemaker only ever uses "monitor". Which > is why, I guess, no-one has run into this problem before. Any reason > for LCMC to use "status" at all? It doesn't. The LCMC does skip the "status" action exactly that reason by default, but it doesn't skip the "status " (with space) action. Reason for that, is that anytime new actions show up in the future, I don't have to change the code at all. > >> And some not very critical... >> >> 2. short descriptions are not really short, I think it's not necessary >> prepend every one of them with "iSCSI target" >> >> The worst offender >> <shortdesc lang="en">Manages an iSCSI target export</shortdesc> >> >> could be changed to "Implementation", or "Daemon Implementation" to be >> short and descriptive. > > Yeah, that one was just a copy & paste error. Fixed too, thanks. > > About the others, I can only surmise you're bikeshedding -- those look > fine to me. However, I'll be happy to take a patch if you have better > suggestions. I think, I'm never bikeshedding. :) It is a mirror issue but the "iSCSI target" in every short description is redundant, since they all belong to the iSCSI target RA, and because the long descriptions are cut in the display, then they all look like "iSCSI target..." and you must mouse over to actually see what they are. I am exaggerating a bit here. So I propose as general style rule, don't include RA name to the short descriptions. > >> 3. defaults are computed in this way that they may be different in >> different cluster nodes and may change after the cluster is configured, >> which is not very useful in my opinion. > > That was my way of trying to provide a "reasonable" default across > distros. The alternative would be that every distro packager would > have to patch the RA to provide the proper default for their platform > -- which would be tgt for RHEL/CentOS, iet for SLES 11 and then lio > for SLES 11 SP2+ (I think), undefined for Debian. You get the picture. > I think the existing way of figuring out the defaults is saner, if not > perfect. Feel free to convince me otherwise, though. You are right about that. The problem I am having is, that they are two types of defaults, that you can't distinguish just by looking in the meta-data. The first is that are used by RA, so you don't have to define 20 parameters, only if you want use something other than the default. This is the most common use. The second type is a suggested value, that is advertised as a default, but unless it is stored like normal value in the cib, it is not used by the RA. The third type is a combination from the two above, like iSCSITarget. I am solving it by keeping track of the RAs, what kind of defaults they are using for now, but I'd preferred that there was some consistency in it. Rasto -- Dipl.-Ing. Rastislav Levrinc [email protected] Linux Cluster Management Console http://lcmc.sf.net/ _______________________________________________________ Linux-HA-Dev: [email protected] http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev Home Page: http://linux-ha.org/
