AUTOFS: Store entry objects below map objects

[Jakub Hrozek]

[PATCH 1/5] AUTOFS: Do not fail if search base is not provided
We need to fall back to rootDSE provided search base instead of failing

[PATCH 2/5] AUTOFS: Add sysdb tests
I knew I would be changing the sysdb interface so I coded up tests to
avoid regressions

[PATCH 3/5] AUTOFS: Add entry objects below map objects
https://fedorahosted.org/sssd/ticket/1506

Changes how the new autofs entry objects are handled. Instead of
creating the entry on the cn=autofs,cn=custom level, the entry is
created below the map it belongs to.

[PATCH 4/5] AUTOFS: Use both key and value in entry RDN
This patch switches from using just key in the RDN to using both key and
value. That is neccessary to allow multiple direct mounts in a single
map.

[PATCH 5/5] AUTOFS: convert the existing autofs entries during a sysdb upgrade
The sysdb is upgraded to a new version to convert the existing entries
to the new format.

[Pavel Březina]

On 09/23/2012 08:47 PM, Jakub Hrozek wrote:
> [PATCH 1/5] AUTOFS: Do not fail if search base is not provided
> We need to fall back to rootDSE provided search base instead of failing

Ack.
> [PATCH 2/5] AUTOFS: Add sysdb tests
> I knew I would be changing the sysdb interface so I coded up tests to
> avoid regressions

Nack.
You don't free test_ctx in the three last tests.
> [PATCH 3/5] AUTOFS: Add entry objects below map objects
> https://fedorahosted.org/sssd/ticket/1506

Nack.
/home/pavka/git/sssd/.git/rebase-apply/patch:273: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

SSSD cannot be compiled with this patch as HEAD:
src/db/sysdb_autofs.c:266:56: error: 'name' undeclared (first use in 
this function)
> Changes how the new autofs entry objects are handled. Instead of
> creating the entry on the cn=autofs,cn=custom level, the entry is
> created below the map it belongs to.
>
> [PATCH 4/5] AUTOFS: Use both key and value in entry RDN
> This patch switches from using just key in the RDN to using both key and
> value. That is neccessary to allow multiple direct mounts in a single
> map.

Ack,
> [PATCH 5/5] AUTOFS: convert the existing autofs entries during a sysdb upgrade
> The sysdb is upgraded to a new version to convert the existing entries
> to the new format.

Ack.

[Jakub Hrozek]

On Mon, Sep 24, 2012 at 01:12:51PM +0200, Pavel Březina wrote:
> On 09/23/2012 08:47 PM, Jakub Hrozek wrote:
> > [PATCH 1/5] AUTOFS: Do not fail if search base is not provided
> > We need to fall back to rootDSE provided search base instead of failing
>
> Ack.
> > [PATCH 2/5] AUTOFS: Add sysdb tests
> > I knew I would be changing the sysdb interface so I coded up tests to
> > avoid regressions
>
> Nack.
> You don't free test_ctx in the three last tests.

Fixed.
> > [PATCH 3/5] AUTOFS: Add entry objects below map objects
> > https://fedorahosted.org/sssd/ticket/1506
>
> Nack.
> /home/pavka/git/sssd/.git/rebase-apply/patch:273: new blank line at EOF.
>
> warning: 1 line adds whitespace errors.

Fixed. Sorry, I squashed that hunk into a wrong patch.
> SSSD cannot be compiled with this patch as HEAD:
> src/db/sysdb_autofs.c:266:56: error: 'name' undeclared (first use in
> this function)
> > Changes how the new autofs entry objects are handled. Instead of
> > creating the entry on the cn=autofs,cn=custom level, the entry is
> > created below the map it belongs to.
> >
> > [PATCH 4/5] AUTOFS: Use both key and value in entry RDN
> > This patch switches from using just key in the RDN to using both key and
> > value. That is neccessary to allow multiple direct mounts in a single
> > map.
>
> Ack,
> > [PATCH 5/5] AUTOFS: convert the existing autofs entries during a sysdb upgrade
> > The sysdb is upgraded to a new version to convert the existing entries
> > to the new format.
>
> Ack.

New patches are attached. Thank you for the review.

[Pavel Březina]

On 09/24/2012 03:44 PM, Jakub Hrozek wrote:
> On Mon, Sep 24, 2012 at 01:12:51PM +0200, Pavel Březina wrote:
> > On 09/23/2012 08:47 PM, Jakub Hrozek wrote:
> > > [PATCH 1/5] AUTOFS: Do not fail if search base is not provided
> > > We need to fall back to rootDSE provided search base instead of failing
> >
> > Ack.
> > > [PATCH 2/5] AUTOFS: Add sysdb tests
> > > I knew I would be changing the sysdb interface so I coded up tests to
> > > avoid regressions
> >
> > Nack.
> > You don't free test_ctx in the three last tests.
>
> Fixed.
> > > [PATCH 3/5] AUTOFS: Add entry objects below map objects
> > > https://fedorahosted.org/sssd/ticket/1506
> >
> > Nack.
> > /home/pavka/git/sssd/.git/rebase-apply/patch:273: new blank line at EOF.
> >
> > warning: 1 line adds whitespace errors.
>
> Fixed. Sorry, I squashed that hunk into a wrong patch.
> > SSSD cannot be compiled with this patch as HEAD:
> > src/db/sysdb_autofs.c:266:56: error: 'name' undeclared (first use in
> > this function)
> > > Changes how the new autofs entry objects are handled. Instead of
> > > creating the entry on the cn=autofs,cn=custom level, the entry is
> > > created below the map it belongs to.
> > >
> > > [PATCH 4/5] AUTOFS: Use both key and value in entry RDN
> > > This patch switches from using just key in the RDN to using both key and
> > > value. That is neccessary to allow multiple direct mounts in a single
> > > map.
> >
> > Ack,
> > > [PATCH 5/5] AUTOFS: convert the existing autofs entries during a sysdb upgrade
> > > The sysdb is upgraded to a new version to convert the existing entries
> > > to the new format.
> >
> > Ack.
>
> New patches are attached. Thank you for the review.

Nack for patch 3.
mod_autofs_entry() don't use default branch for enum in the switch 
statement. If the default branch is present, compiler cannot warn you 
that you have forgotten some value.

[Jakub Hrozek]

On Mon, Sep 24, 2012 at 04:41:51PM +0200, Pavel Březina wrote:
> On 09/24/2012 03:44 PM, Jakub Hrozek wrote:
> > On Mon, Sep 24, 2012 at 01:12:51PM +0200, Pavel Březina wrote:
> > > On 09/23/2012 08:47 PM, Jakub Hrozek wrote:
> > > > [PATCH 1/5] AUTOFS: Do not fail if search base is not provided
> > > > We need to fall back to rootDSE provided search base instead of failing
> > >
> > > Ack.
> > > > [PATCH 2/5] AUTOFS: Add sysdb tests
> > > > I knew I would be changing the sysdb interface so I coded up tests to
> > > > avoid regressions
> > >
> > > Nack.
> > > You don't free test_ctx in the three last tests.
> >
> > Fixed.
> > > > [PATCH 3/5] AUTOFS: Add entry objects below map objects
> > > > https://fedorahosted.org/sssd/ticket/1506
> > >
> > > Nack.
> > > /home/pavka/git/sssd/.git/rebase-apply/patch:273: new blank line at EOF.
> > >
> > > warning: 1 line adds whitespace errors.
> >
> > Fixed. Sorry, I squashed that hunk into a wrong patch.
> > > SSSD cannot be compiled with this patch as HEAD:
> > > src/db/sysdb_autofs.c:266:56: error: 'name' undeclared (first use in
> > > this function)
> > > > Changes how the new autofs entry objects are handled. Instead of
> > > > creating the entry on the cn=autofs,cn=custom level, the entry is
> > > > created below the map it belongs to.
> > > >
> > > > [PATCH 4/5] AUTOFS: Use both key and value in entry RDN
> > > > This patch switches from using just key in the RDN to using both key and
> > > > value. That is neccessary to allow multiple direct mounts in a single
> > > > map.
> > >
> > > Ack,
> > > > [PATCH 5/5] AUTOFS: convert the existing autofs entries during a sysdb upgrade
> > > > The sysdb is upgraded to a new version to convert the existing entries
> > > > to the new format.
> > >
> > > Ack.
> >
> > New patches are attached. Thank you for the review.
>
> Nack for patch 3.
> mod_autofs_entry() don't use default branch for enum in the switch
> statement. If the default branch is present, compiler cannot warn
> you that you have forgotten some value.

New patches attached again.

[Pavel Březina]

On 09/24/2012 04:52 PM, Jakub Hrozek wrote:
> On Mon, Sep 24, 2012 at 04:41:51PM +0200, Pavel Březina wrote:
> > On 09/24/2012 03:44 PM, Jakub Hrozek wrote:
> > > On Mon, Sep 24, 2012 at 01:12:51PM +0200, Pavel Březina wrote:
> > > > On 09/23/2012 08:47 PM, Jakub Hrozek wrote:
> > > > > [PATCH 1/5] AUTOFS: Do not fail if search base is not provided
> > > > > We need to fall back to rootDSE provided search base instead of failing
> > > >
> > > > Ack.
> > > > > [PATCH 2/5] AUTOFS: Add sysdb tests
> > > > > I knew I would be changing the sysdb interface so I coded up tests to
> > > > > avoid regressions
> > > >
> > > > Nack.
> > > > You don't free test_ctx in the three last tests.
> > >
> > > Fixed.
> > > > > [PATCH 3/5] AUTOFS: Add entry objects below map objects
> > > > > https://fedorahosted.org/sssd/ticket/1506
> > > >
> > > > Nack.
> > > > /home/pavka/git/sssd/.git/rebase-apply/patch:273: new blank line at EOF.
> > > >
> > > > warning: 1 line adds whitespace errors.
> > >
> > > Fixed. Sorry, I squashed that hunk into a wrong patch.
> > > > SSSD cannot be compiled with this patch as HEAD:
> > > > src/db/sysdb_autofs.c:266:56: error: 'name' undeclared (first use in
> > > > this function)
> > > > > Changes how the new autofs entry objects are handled. Instead of
> > > > > creating the entry on the cn=autofs,cn=custom level, the entry is
> > > > > created below the map it belongs to.
> > > > >
> > > > > [PATCH 4/5] AUTOFS: Use both key and value in entry RDN
> > > > > This patch switches from using just key in the RDN to using both key and
> > > > > value. That is neccessary to allow multiple direct mounts in a single
> > > > > map.
> > > >
> > > > Ack,
> > > > > [PATCH 5/5] AUTOFS: convert the existing autofs entries during a sysdb upgrade
> > > > > The sysdb is upgraded to a new version to convert the existing entries
> > > > > to the new format.
> > > >
> > > > Ack.
> > >
> > > New patches are attached. Thank you for the review.
> >
> > Nack for patch 3.
> > mod_autofs_entry() don't use default branch for enum in the switch
> > statement. If the default branch is present, compiler cannot warn
> > you that you have forgotten some value.
>
> New patches attached again.

Ack. Good job.

[Jakub Hrozek]

On Mon, Sep 24, 2012 at 04:53:46PM +0200, Pavel Březina wrote:
> > New patches attached again.
>
> Ack. Good job.

Pushed to master.