Hi Jan, On Fri, Dec 12, 2014 at 9:44 AM, Jan Kaluža <jkal...@redhat.com> wrote: > On 12/11/2014 03:05 PM, Plüm, Rüdiger, Vodafone Group wrote: >> >> Looks fine in general. Details: > > I hope I've finally fixed everything now :), see the attached patch please.
Aren't the following parameters missing for the merge: unsigned int max_attempts_set:1; unsigned int vhosted:1; unsigned int inactive:1; vhosted does not seem to be used, but seems a good candidate to differentiate base vs vhost balancer. Detail, sticky can't be NULL here: + if ((!b2->s->sticky || *b2->s->sticky == 0) + && b1->s->sticky && *b1->s->sticky) { + PROXY_STRNCPY(b2->s->sticky_path, b1->s->sticky_path); + PROXY_STRNCPY(b2->s->sticky, b1->s->sticky); + } + I am (still) confused about the shallow copy: >> >> + *b2 = *b1; IIUC, Rüdiger's point is that base balancers' parameters shouldn't be modified by vhosts specifics, because some vhosts (or RewriteRules) may use the default ones. But then why would they share (at runtime) the same lbmethod, members list (dynamic with balancer-manager), backend connections (reslist, same timeouts, reusability, states, any worker parameter). IMHO there should be a deep copy here, that's another balancer, with its own mutexes, own members having their own parameters, own sockets... Regards, Yann.