On Mon, Jul 11, 2016 at 10:29:56AM +0200, Sumit Bose wrote:
> On Wed, Jun 22, 2016 at 03:30:28PM +0200, Petr Cech wrote:
> > On 06/22/2016 11:14 AM, Lukas Slebodnik wrote:
> > > On (20/06/16 14:44), Petr Cech wrote:
> > > > >Hi,
> > > > >
> > > > >there is patch for #3023 attached.
> > > > >
> > > > >Reason for this patch is explained in commit message.
> > > > >
> > > > >I am open to discussion. In my opinion we don't need to
connect sysdb pointer
> > > > >with domain structure because it is not used in other upgrade
functions too.
> > > > >
> > > > >Regards
> > > > >
> > > > >--
> > > > >Petr^4 Čech
> > > > From d0cee759147272f33427388af76ea66307e34881 Mon Sep 17 00:00:00
2001
> > > > >From: Petr Cech <pcech(a)redhat.com>
> > > > >Date: Mon, 20 Jun 2016 09:19:03 -0300
> > > > >Subject: [PATCH] SYSDB: Fixing DB update
> > > > >
> > > > >Functions sysdb_user_base_dn() and sysdb_group_base_dn() expect
> > > > >that struct sss_domain_info contains pointer to struct sysdb_ctx.
> > > > >This is not true in case of sysdb_upgrade functions.
> > > > >This patch fixes the situation and revert code to the state
before
> > > > >12a000c8c7c07259e438fb1e992134bdd07d9a30 commit.
> > > > >
> > > I didn't test but it make sense.
> > > It would be good to add comment to the code. So it will not be changed
> > > in future (ldb_dn_new_fmt -> sysdb_user_*_dn)
> > >
> > > Other alternative might be to "temporary initialize"
> > > dom->sysdb and then set it back to NULL.
> > > Or permanently initialize it sooner if sysdb is not destroyed
> > > after upgrade.
> > >
> > > Others might have different ideas/preferences as well.
> > >
> > > LS
> >
> > Hi Lukas,
> >
> > thanks for comment. I agree that we should protect this part of code from
> > the changes in future.
> >
> > I vote for comment in code. So, there is new patch attached.
>
> Thank you, I think comments are ok for the update code.
>
> I tested the version 10 update by just setting the version in the cache
> file to 0.10 and it fails without the patch and works as expected with
> the patch (please note that later on another update fails becasue we add
> an index item unconditionally without checking if it already exists and
> I started with a 0.17 cache).
>
> There is just a minor typo (see below) which might also be fixed while
> pushing the patch.
>
> Just waiting for CI ...
... passed
http://sssd-ci.duckdns.org/logs/job/49/45/summary.html
ACK
master:
* 311836214245600566f881ff6253594e0999008e
sssd-1-13:
* 0a7784d3584a5a6db1e0251e1bcc9dd1790f1f38
LS