On Tue, Jul 08, 2014 at 03:50:01PM +0200, Pavel Reichl wrote:
On Mon, 2014-07-07 at 11:57 +0200, Jakub Hrozek wrote:
> On Mon, Jul 07, 2014 at 11:47:28AM +0200, Pavel Březina wrote:
> > >I will start a separate thread also with #2351 in mind.
> > >
> > >New patches are attached, this time I consider them ready for review, so
> > >I'll remove the RFC tag.
> >
> > LGTM
>
> btw Jan did some testing already on RHEL6:
>
https://bugzilla.redhat.com/show_bug.cgi?id=1110369
>
> > Can you just add a debug message that will say that ifp_sysbus_reconnect is
> > executed?
>
> Sure, do the attached patches look any better?
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Hi
patches LGTM, I just noted one little nitpick, but I really don't think
its important so ACK to patches as they are now.
> static struct data_provider_iface ifp_dp_methods = {
> @@ -257,6 +260,39 @@ fail:
> return ret;
> }
>
> +static int ifp_sysbus_reconnect(struct sbus_request *dbus_req, void *data)
> +{
> + struct resp_ctx *rctx = talloc_get_type(data, struct resp_ctx);
> + struct ifp_ctx *ifp_ctx = (struct ifp_ctx*) rctx->pvt_ctx;
> + errno_t ret;
> +
> + DEBUG(SSSDBG_TRACE_FUNC, "Attempting to reconnect to the system
bus\n");
> +
> + if (ifp_ctx->sysbus) {
> + DEBUG(SSSDBG_TRACE_LIBS, "Already connected to sysbus\n");
> + goto done;
> + }
> +
> + /* Connect to the D-BUS system bus and set up methods */
> + ret = sysbus_init(ifp_ctx, ifp_ctx->rctx->ev,
> + INFOPIPE_IFACE,
> + ifp_ifaces,
> + ifp_ctx, &ifp_ctx->sysbus);
> + if (ret == ERR_NO_SYSBUS) {
> + DEBUG(SSSDBG_MINOR_FAILURE,
> + "The system bus is not available..\n");
> + goto done;
> + } else if (ret != EOK) {
> + DEBUG(SSSDBG_CRIT_FAILURE,
> + "Failed to connect to the system message bus\n");
> + return ret;
> + }
> +
> + DEBUG(SSSDBG_TRACE_LIBS, "Reconnected to the system bus!\n");
I think it's a common practice to have a blank line before goto label,
but I don't think it's in our code guide.
It's not in the code guide, but I took the liberty to add the blank line
w/o re-sending the patches for another review, the code now looks
better.
> +done:
> + return sbus_request_return_and_finish(dbus_req, DBUS_TYPE_INVALID);
> +}
> +
Thank you for the review, I pushed the patches to master:
1a59af8245f183f22d87d067a90197d8e2ea958d
1746e8b8399da2a7a8da4aace186f66055ccfec1
149f40dc2d4ead57811c70b5028648ac83f6a1a7
b76419cf8830440b46c20a15585562343c7b1924
0c1d65998907930678da2d091789446f2c344d5d
1f2507e1fd089f2bf3458cfb4faeaa9669d72f98
4df1a6a977df74420867d9b1daddcca0eea4b2e1
The 1.11 version will be sent out separately.