[SSSD] [PATCH] sss_override: support import and export

Jakub Hrozek jhrozek at redhat.com
Tue Aug 18 11:23:22 UTC 2015


On Mon, Aug 17, 2015 at 12:17:29PM +0200, Lukas Slebodnik wrote:
> On (16/08/15 17:59), Pavel Březina wrote:
> >https://fedorahosted.org/sssd/ticket/2737
> >
> >Hi, it should work... :-) However, I wanted to make import as transaction so
> >no changes are made if some error occurs, but I had some troubles with it so
> >I gave up eventually.
> >
> >Of course, it would be quite difficult to make it safe across domains thus my
> >intention was only to ensure that either all changes are done in a domain or
> >none. But still leaving the possibility that changes are commited in one
> >domain but cancelled in another.
> >
> >I tried to start sysdb transaction on all used domains and then commit/cancel
> >it, writing some neat mechanism for it. However, I occasionally run into a
> >problem when data provider hangs when trying to safe a user. It looked like
> >some sort of race condition.
> >
> >Unfortunately I managed to delete the code so I can't show it to you, I think
> >it would be a nice feature so if anyone familiar with ldb want to step in,
> >he's welcome.
> 
> I briefly look at the patches and here are few comments
> 
> >From 24655f677acf9f64201a611ababdcfeff4317037 Mon Sep 17 00:00:00 2001
> >From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
> >Date: Sat, 15 Aug 2015 16:42:27 +0200
> >Subject: [PATCH 2/2] sss_override: add import and export
> >
> >Resolves:
> >https://fedorahosted.org/sssd/ticket/2737
> >---
> > Makefile.am                    |   2 +
> > src/tools/common/sss_colondb.c | 298 ++++++++++++++
> > src/tools/common/sss_colondb.h |  71 ++++
> > src/tools/sss_override.c       | 866 +++++++++++++++++++++++++++++++++++------
> > 4 files changed, 1128 insertions(+), 109 deletions(-)
> > create mode 100644 src/tools/common/sss_colondb.c
> > create mode 100644 src/tools/common/sss_colondb.h
> >
> >diff --git a/Makefile.am b/Makefile.am
> >index ed107fd5dc76b768176a3d7236b0bf1c75f212bf..f8c7c29df08c2ba9e74508c0f84ac57f6e6bac26 100644
> >--- a/Makefile.am
> >+++ b/Makefile.am
> >@@ -651,6 +651,7 @@ dist_noinst_HEADERS = \
> >     src/lib/sifp/sss_sifp_private.h \
> >     src/tests/cmocka/test_utils.h \
> >     src/tools/common/sss_tools.h \
> >+    src/tools/common/sss_colondb.h \
> >     $(NULL)
> > 
> > 
> >@@ -1331,6 +1332,7 @@ sss_signal_LDADD = \
> > 
> > sss_override_SOURCES = \
> >     src/tools/sss_override.c \
> >+    src/tools/common/sss_colondb.c \
> >     $(SSSD_TOOLS_OBJ) \
> >     $(NULL)
> > sss_override_LDADD = \
> >diff --git a/src/tools/common/sss_colondb.c b/src/tools/common/sss_colondb.c
> >new file mode 100644
> >index 0000000000000000000000000000000000000000..be9396832e93fa3b646255dae214a34fe4d6c29c
> >--- /dev/null
> >+++ b/src/tools/common/sss_colondb.c
> >@@ -0,0 +1,298 @@
> >+/*
> >+    Authors:
> >+        Pavel Březina <pbrezina at redhat.com>
> >+
> >+    Copyright (C) 2015 Red Hat
> >+
> >+    This program is free software; you can redistribute it and/or modify
> >+    it under the terms of the GNU General Public License as published by
> >+    the Free Software Foundation; either version 3 of the License, or
> >+    (at your option) any later version.
> >+
> >+    This program is distributed in the hope that it will be useful,
> >+    but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >+    GNU General Public License for more details.
> >+
> >+    You should have received a copy of the GNU General Public License
> >+    along with this program.  If not, see <http://www.gnu.org/licenses/>.
> >+*/
> >+
> >+#include <stdlib.h>
> >+
> >+#include "util/util.h"
> >+#include "util/strtonum.h"
> >+#include "tools/common/sss_colondb.h"
> >+
> >+#define IS_STD_FILE(db) ((db)->file == stdin || (db)->file == stdout)
> >+#define IS_LINE_END(str) ((str) == NULL || *(str) == '\0' || *(str) == '\n')
>           ^^^^^^^^^^^
>           This macro is unused.

I wonder if we could use the macro later:
    30 static char *read_field_as_string(char *line,
    31                                   const char **_value)
    32 {
    33     char *rest;
    34     char *value;
    35
    36     if (line == NULL || *line == '\0' || *line == '\n') {
---> HERE
    37         /* There is nothing else to read. */

>           Side note: if there is expectation occurrence '\n' should be higher
>           that occurrence of '\0' then you should change order in condition.

Yes, this wouldn't hurt.

> 
> //snip
> 
> >+errno_t sss_colondb_writeline(struct sss_colondb *db,
> >+                              struct sss_colondb_write_field *table)
> >+{
> >+    TALLOC_CTX *tmp_ctx;
> >+    char *line = NULL;
> >+    errno_t ret;
> >+    int i;
> >+
> >+    if (db->mode != SSS_COLONDB_WRITE) {
> >+        return ERR_INTERNAL;
> >+    }
> >+
> >+    tmp_ctx = talloc_new(NULL);
> >+    if (tmp_ctx == NULL) {
> >+        DEBUG(SSSDBG_CRIT_FAILURE, "talloc_new() failed.\n");
> >+        return ENOMEM;
> >+    }
> >+
> >+    for (i = 0; table[i].type != SSS_COLONDB_SENTINEL; i++) {
> >+        switch (table[i].type) {
> >+        case SSS_COLONDB_UINT32:
> >+            if (table[i].data.uint32 == 0) {
> >+                line = talloc_asprintf_append(line, ":");
> >+            } else {
> >+                line = talloc_asprintf_append(line, ":%u", table[i].data.uint32);
>                                        uint32_t has different fotmat specifier
>                                        @see man inttypes.h

+1

> >+            break;
> >+        case SSS_COLONDB_STRING:
> >+            if (table[i].data.str == NULL) {
> >+                line = talloc_asprintf_append(line, ":");
> >+            } else {
> >+                line = talloc_asprintf_append(line, ":%s", table[i].data.str);
>                  Maybe I'm wrong but the first entry in /etc/passwd is string
>                  and does not start with ":". I'm not sure about Dell VAS
>                  format.
>                  I cannot see any special case for index 0

Later in the code we have:

> >+    /* Remove starting : */
> >+    line++;



More information about the sssd-devel mailing list