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
bye,
Sumit
>
> bye,
> Sumit
>
> >
> > Regards
> >
> > --
> > Petr^4 Čech
>
> > From 729e8bb6b5c03d14b423c5dcccd46ae1cb4ebcdc 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.
> >
> > Resolves:
> >
https://fedorahosted.org/sssd/ticket/3023
> > ---
> > src/db/sysdb_upgrade.c | 22 +++++++++++++++++++---
> > 1 file changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/db/sysdb_upgrade.c b/src/db/sysdb_upgrade.c
> > index
113f24644e3e3de1d4c46d375492c2fe1e6b2f83..f68800eca6b362b62dbd30eecfa607d4b577df22 100644
> > --- a/src/db/sysdb_upgrade.c
> > +++ b/src/db/sysdb_upgrade.c
> > @@ -443,12 +443,23 @@ int sysdb_check_upgrade_02(struct sss_domain_info
*domains,
> > goto done;
> > }
> >
> > - users_dn = sysdb_user_base_dn(tmp_ctx, dom);
> > + /*
> > + * dom->sysdb->ldb is not initialized,
> > + * so ldb_dn_new_fmt() shouldn't be changed to sysdb_*_base_dn()
> > + */
> > + users_dn = ldb_dn_new_fmt(tmp_ctx, sysdb->ldb,
> > + SYSDB_TMPL_USER_BASE, dom->name);
>
> ^ missing space
>
> > if (!users_dn) {
> > ret = ENOMEM;
> > goto done;
> > }
> > - groups_dn = sysdb_group_base_dn(tmp_ctx, dom);
> > +
> > + /*
> > + * dom->sysdb->ldb is not initialized,
> > + * so ldb_dn_new_fmt() shouldn't be changed to sysdb_*_base_dn()
> > + */
> > + groups_dn = ldb_dn_new_fmt(tmp_ctx, sysdb->ldb,
> > + SYSDB_TMPL_GROUP_BASE, dom->name);
> > if (!groups_dn) {
> > ret = ENOMEM;
> > goto done;
> > @@ -1045,7 +1056,12 @@ int sysdb_upgrade_10(struct sysdb_ctx *sysdb, struct
sss_domain_info *domain,
> > return ret;
> > }
> >
> > - basedn = sysdb_user_base_dn(tmp_ctx, domain);
> > + /*
> > + * dom->sysdb->ldb is not initialized,
> > + * so ldb_dn_new_fmt() shouldn't be changed to sysdb_*_base_dn()
> > + */
> > + basedn = ldb_dn_new_fmt(tmp_ctx, sysdb->ldb,
> > + SYSDB_TMPL_USER_BASE, domain->name);
> > if (basedn == NULL) {
> > ret = EIO;
> > goto done;
> > --
> > 2.5.5
> >
>
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel(a)lists.fedorahosted.org
> >
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org