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@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@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@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

---

sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org