[SSSD] [PATCH] DATA_PROVIDER: BE_REQ as string in log message

Jakub Hrozek jhrozek at redhat.com
Wed Aug 5 09:23:28 UTC 2015


On Wed, Aug 05, 2015 at 10:03:30AM +0200, Pavel Reichl wrote:
> 
> 
> On 07/15/2015 08:53 AM, Petr Cech wrote:
> >Hi,
> >
> >I investigated the situation around the log message, which mentioned
> >Lukas. I prepared this patch. The result is that the original message
> >> [sssd[be[cygnus.dev]]] [be_get_account_info] (0x0200): Got request for
> >[0x1001][1][name=celestian]
> >changed to
> >> [sssd[be[cygnus.dev]]] [be_get_account_info] (0x0200): Got request for
> >[0x1001][FAST BE_REQ_USER][1][name=celestian]
> >
> >
> >A)
> >I would like to ask if mark 'FAST' is useful, or if I should remove it.
> I think it can't hurt and might be useful, so IMO leave it.

Yes, IIRC FAST flag is always set for identity lookups and never set for
PAM responder lookups. I think it tells the responder whether to attempt
to reconnect if offline.

This might be useful data for the Internal document.

> >
> >
> >B)
> >While writing a patch Lukas noticed another similar logging messages
> >> [sssd[pam]] [sss_dp_get_account_msg] (0x0400): Creating request for
> >[LDAP][3][1][name=mof_user6]
> >
> >I investigated it. This is the same thing -- BE_REQ_*, but it is no longer
> >in the provider, but in the responder. Can you please advise me where I
> >could the function 'be_req2str' write?
> I think you should move it to separate file, as you don't want to share more
> code then necessary.
> There are 2 possibilities where to have this file
> a) in src/providers  - responders already link with some modules from this
> folder, so I suppose it's viable

Yes, this is a bit of hierarchy violation, but in the end we need to
share the constants somehow. I think it's fine to keep the definite
version in the providers/dp_* hierarchy, because that's where the
interface is defined, the responder is a consumer.

> 
> SSSD_RESPONDER_OBJ = \
>     ...
>     src/providers/data_provider_iface_generated.c \
>     src/providers/data_provider_iface_generated.h
> 
> b) move it to src/util which is a natural place for shared code
> 
> In my opinion a) is better because code is closely coupled with back end and
> is not general enough to be part of utils.

I agree. In future, especially if we document the interface and promote
3rd parties to write responders and providers, we might want to put the
interface into its own directory, but for now it's fine.

> 
> Sorry for not giving definitive answers. :-)
> 
> >+
> >+const char* be_req2str(dbus_uint32_t req_type)
> >+{
> // I was thinking whether you should use some
> static char output[??];
> 
> //and fill it with "FAST" in a single condition and thus simplifying
> subsequent case
> 
> if (req_type & BE_REQ_FAST0)
> 	sprintf(output, "FAST");

I think this would be fine if we formatted the output, but we only
return const strings..

> 
> 
> >+    switch (req_type & BE_REQ_TYPE_MASK) {
> >+    case BE_REQ_USER:
> >+        return req_type & BE_REQ_FAST ? "FAST BE_REQ_USER" : "BE_REQ_USER";
> >+    case BE_REQ_GROUP:
> >+        return req_type & BE_REQ_FAST ? "FAST BE_REQ_GROUP" : "BE_REQ_GROUP";
> >+    case BE_REQ_INITGROUPS:
> >+        return req_type & BE_REQ_FAST ? "FAST BE_REQ_INITGROUPS" : "BE_REQ_INITGROUPS";
> >+    case BE_REQ_NETGROUP:
> >+        return req_type & BE_REQ_FAST ? "FAST BE_REQ_NETGROUP" : "BE_REQ_NETGROUP";
> >+    case BE_REQ_SERVICES:
> >+        return req_type & BE_REQ_FAST ? "FAST BE_REQ_SERVICES" : "BE_REQ_SERVICES";
> >+    case BE_REQ_SUDO_FULL:
> >+        return req_type & BE_REQ_FAST ? "FAST BE_REQ_SUDO_FULL" : "BE_REQ_SUDO_FULL";
> >+    case BE_REQ_SUDO_RULES:
> >+        return req_type & BE_REQ_FAST ? "FAST BE_REQ_SUDO_RULES" : "BE_REQ_SUDO_RULES";
> >+    case BE_REQ_AUTOFS:
> >+        return req_type & BE_REQ_FAST ? "FAST BE_REQ_AUTOFS" : "BE_REQ_AUTOFS";
> >+    case BE_REQ_HOST:
> >+        return req_type & BE_REQ_FAST ? "FAST BE_REQ_HOST" : "BE_REQ_HOST";
> >+    case BE_REQ_BY_SECID:
> >+        return req_type & BE_REQ_FAST ? "FAST BE_REQ_BY_SECID" : "BE_REQ_BY_SECID";
> >+    case BE_REQ_USER_AND_GROUP:
> >+        return req_type & BE_REQ_FAST ? "FAST BE_REQ_USER_AND_GROUP" : "BE_REQ_USER_AND_GROUP";
> >+    case BE_REQ_BY_UUID:
> >+        return req_type & BE_REQ_FAST ? "FAST BE_REQ_BY_UUID" : "BE_REQ_BY_UUID";
> >+    case BE_REQ_BY_CERT:
> >+        return req_type & BE_REQ_FAST ? "FAST BE_REQ_BY_CERT" : "BE_REQ_BY_CERT";
> >+    }
> >+    return "UNKNOWN_REQ";
> >+}
> 
> 
> >
> >The first message is coming from
> >> src/providers/data_provider_be.c --> be_get_account_info,
> >the second is from
> >> src/responder/common/responder_dp --> sss_dp_get_account_msg
> >
> >
> >Thanks.
> >
> >Petr
> >
> >
> >_______________________________________________
> >sssd-devel mailing list
> >sssd-devel at lists.fedorahosted.org
> >https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> 



More information about the sssd-devel mailing list