[SSSD] [PATCH] Use refcount to keep track of server structures returned from failover

Lukas Slebodnik lslebodn at redhat.com
Mon Oct 12 07:29:21 UTC 2015


On (12/10/15 09:26), Jakub Hrozek wrote:
>On Mon, Oct 12, 2015 at 08:44:27AM +0200, Lukas Slebodnik wrote:
>> On (12/10/15 07:42), Lukas Slebodnik wrote:
>> >On (11/10/15 22:00), Jakub Hrozek wrote:
>> >>Hi,
>> >>
>> >>the attached patches are my proposal to fix
>> >>https://fedorahosted.org/sssd/ticket/2829
>> >>
>> >>I haven't tested them past make check yet, because I'm not sure I like
>> >>them myself :) but at the same time I can't see a better way to keep
>> >>track of the servers and let callers set state of servers.
>> >>
>> >>The most ugly thing so far IMO is the fo_internal_owner member. I would
>> >>prefer to instead have a fo_server_wrap structure that would be used for
>> >>the server_list, but I didn't want to do a large change before we agree
>> >>the refcount is a good idea at all.
>> >>
>> >>The other ugly side-effect is that we need to be sure that nobody calls
>> >>talloc_free on the fo_server structure. Instead, only the parent context
>> >>can be freed (that's also what the first patch is about).
>> >
>> >>From 2f71ee096701e869ed3e4646cbf8a4bf7a380f00 Mon Sep 17 00:00:00 2001
>> >>From: Jakub Hrozek <jhrozek at redhat.com>
>> >>Date: Sun, 11 Oct 2015 18:08:46 +0200
>> >>Subject: [PATCH 1/4] FO: Don't free rc-allocated structure
>> >>
>> >>---
>> >> src/providers/fail_over.c | 1 -
>> >> 1 file changed, 1 deletion(-)
>> >>
>> >>diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c
>> >>index b076687ac6e571f7e27402fd11ac60183ea46951..b309f1c68d0f4219d4b97eb0c01416e53ea856d0 100644
>> >>--- a/src/providers/fail_over.c
>> >>+++ b/src/providers/fail_over.c
>> >>@@ -507,7 +507,6 @@ create_server_common(TALLOC_CTX *mem_ctx, struct fo_ctx *ctx, const char *name)
>> >> 
>> >>     common->name = talloc_strdup(common, name);
>> >>     if (common->name == NULL) {
>> >>-        talloc_free(common);
>> >>         return NULL;
>> >>     }
>> >> 
>> >>-- 
>> >>2.4.3
>> >>
>> >
>> >>From 3dd31148b43d0b466c055cf637c00ea8391b694b Mon Sep 17 00:00:00 2001
>> >>From: Jakub Hrozek <jhrozek at redhat.com>
>> >>Date: Sun, 11 Oct 2015 15:31:44 +0200
>> >>Subject: [PATCH 2/4] tests: Reduce failover code duplication
>> >>
>> >Slightly related request.
>> >Could you also merge check bashed failover test?
>> >src/tests/fail_over-tests.c
>> Or we can just remove src/tests/fail_over-tests.c
>> if the same functionality is coverd in cmocka failover test.
>> 
>> LS
>
>Not all of it (at least not explicitly), but I'm fine with merging the
>two. I just don't think it should block review of this set.
sure,
I didn't want to review patches :-)

LS


More information about the sssd-devel mailing list