fs: Avoid unnecessary cell DNS lookups

Currently, many routines in 'fs' cause afsconf_GetCellInfo to be
called for the given cell, which causes an AFSDB/SRV lookup for the
given cell if the cell has no dbservers specified in the local
CellServDB. Sites often define such CellServDB records to indicate
that the given cell exists, but the dbserver IPs should only be looked
up via DNS.

However, 'fs' is only calling afsconf_GetCellInfo in order to
canonicalize the cell name (we're allowed to give an abbreviated name
for a cell on the command line if the abbreviation is unambiguous, but
we want to give the full name to the kernel). We don't care about the
other cell info at all, so triggering a DNS lookup is completely
unnecessary.

If our DNS server is not responding, e.g. because we've lost network
access entirely, this causes the relevant 'fs' commands to fail,
possibly after a long delay. Some fs commands such as 'fs setcell' are
often run during client startup, and can still otherwise run fine
without network access, and so such failures are unnecessary.

To fix this, we introduce a new function, called afsconf_GetCellName,
which only returns the full name for a cell, and does not require a
DNS lookup for such "empty" cells in the local CellServDB. For all
code paths in 'fs' that just need the cell name (which is all of them
besides 'fs mkmount'), use this new function.

Change-Id: I45eb1bdc6ba3e3b4653be2306dcbf79c1015724c
Reviewed-on: https://gerrit.openafs.org/13540
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

diff --git a/src/auth/cellconfig.c b/src/auth/cellconfig.c
index d41d4d2..381ed84 100644 (file)
--- a/src/auth/cellconfig.c
+++ b/src/auth/cellconfig.c
@@ -1420,9 +1420,14 @@ afsconf_GetAfsdbInfo(char *acellName, char *aservice,
 }
 #endif /* windows */
 
-int
-afsconf_GetCellInfo(struct afsconf_dir *adir, char *acellName, char *aservice,
-                   struct afsconf_cell *acellInfo)
+/* flags for _GetCellInfo */
+#define AFSCONF_GETCELL_EMPTYOK (0x1) /** it's okay to return 'empty' cells
+                                       * (that is, cells without any
+                                       * dbservers) */
+
+static int
+_GetCellInfo(struct afsconf_dir *adir, char *acellName, char *aservice,
+            struct afsconf_cell *acellInfo, afs_uint32 flags)
 {
     struct afsconf_entry *tce;
     struct afsconf_aliasentry *tcae;
@@ -1433,6 +1438,11 @@ afsconf_GetCellInfo(struct afsconf_dir *adir, char *acellName, char *aservice,
     int cnLen;
     int ambig;
     char tbuffer[64];
+    int emptyok = 0;
+
+    if ((flags & AFSCONF_GETCELL_EMPTYOK)) {
+       emptyok = 1;
+    }
 
     LOCK_GLOBAL_MUTEX;
     if (adir)
@@ -1482,7 +1492,7 @@ afsconf_GetCellInfo(struct afsconf_dir *adir, char *acellName, char *aservice,
            bestce = tce;
        }
     }
-    if (!ambig && bestce && bestce->cellInfo.numServers) {
+    if (!ambig && bestce && (bestce->cellInfo.numServers || emptyok)) {
        *acellInfo = bestce->cellInfo;  /* structure assignment */
        if (aservice) {
            tservice = afsconf_FindService(aservice);
@@ -1559,7 +1569,9 @@ afsconf_GetCellInfo(struct afsconf_dir *adir, char *acellName, char *aservice,
                 acellInfo->clone[i] = clone[i];
             }
             acellInfo->numServers = numServers;
-            acellInfo->flags |= AFSCONF_CELL_FLAG_DNS_QUERIED;
+           if (numServers) {
+               acellInfo->flags |= AFSCONF_CELL_FLAG_DNS_QUERIED;
+           }
         }
        UNLOCK_GLOBAL_MUTEX;
        return 0;
@@ -1570,6 +1582,63 @@ afsconf_GetCellInfo(struct afsconf_dir *adir, char *acellName, char *aservice,
 }
 
 /**
+ * Get info about a cell.
+ *
+ * @param[in] adir     afsconf object.
+ * @param[in] acellName        name of the cell to get. a cell name abbreviation can
+ *                     be given if it's unambiguous (e.g. "cell" can be given
+ *                     for "cell.example.com" if no other cells begin with
+ *                     "cell").
+ * @param[in] aservice name of the service in the cell, as accepted by
+ *                     afsconf_FindService. if NULL is given: for local
+ *                     lookups, no port information will be returned; for DNS
+ *                     lookups, we'll default to "afs3-vlserver".
+ * @param[out] acellInfo    info for the requested cell and service
+ *
+ * @return afsconf error codes
+ */
+int
+afsconf_GetCellInfo(struct afsconf_dir *adir, char *acellName, char *aservice,
+                   struct afsconf_cell *acellInfo)
+{
+    return _GetCellInfo(adir, acellName, aservice, acellInfo, 0);
+}
+
+/**
+ * Get a cell's name.
+ *
+ * This is similar to afsconf_GetCellInfo, but doesn't actually retrieve the
+ * info of the specified cell (beyond it's name). This can be useful to verify
+ * that a cell name is valid, or to canonicalize a possibly-abbreviated cell
+ * name. Unlike afsconf_GetCellInfo, this call can avoid DNS lookups if the
+ * cell name is specified in the local config, but the cell's servers are not.
+ *
+ * @param[in] adir     afsconf object.
+ * @param[in] acellName        name of the cell to get. a cell name abbreviation can
+ *                     be given if it's unambiguous (see afsconf_GetCellInfo).
+ * @param[out] buf     on success, the cell's name is written to this buffer.
+ * @param[in] buf_size size of 'buf'.
+ *
+ * @return afsconf error codes
+ * @retval AFSCONF_FAILURE  buf_size is too small to fit the cell's name.
+ */
+int
+afsconf_GetCellName(struct afsconf_dir *adir, char *acellName, char *buf,
+                   size_t buf_size)
+{
+    int code;
+    struct afsconf_cell info;
+    code = _GetCellInfo(adir, acellName, NULL, &info, AFSCONF_GETCELL_EMPTYOK);
+    if (code) {
+       return code;
+    }
+    if (strlcpy(buf, info.name, buf_size) >= buf_size) {
+       return AFSCONF_FAILURE;
+    }
+    return 0;
+}
+
+/**
  * Get the current localcell name.
  *
  * Internal function to get a pointer to the local cell name.

diff --git a/src/auth/cellconfig.p.h b/src/auth/cellconfig.p.h
index 2212f39..2713de7 100644 (file)
--- a/src/auth/cellconfig.p.h
+++ b/src/auth/cellconfig.p.h
@@ -130,6 +130,8 @@ extern int afsconf_GetAfsdbInfo(char *acellName, char *aservice,
 extern int afsconf_GetCellInfo(struct afsconf_dir *adir, char *acellName,
                               char *aservice,
                               struct afsconf_cell *acellInfo);
+extern int afsconf_GetCellName(struct afsconf_dir *adir, char *acellName,
+                              char *buf, size_t buf_size);
 extern int afsconf_GetLocalCell(struct afsconf_dir *adir,
                                char *aname, afs_int32 alen);
 extern int afsconf_Close(struct afsconf_dir *adir);

diff --git a/src/auth/liboafs_auth.la.sym b/src/auth/liboafs_auth.la.sym
index 5c9bf9b..4d34f21 100644 (file)
--- a/src/auth/liboafs_auth.la.sym
+++ b/src/auth/liboafs_auth.la.sym
@@ -17,6 +17,7 @@ afsconf_DeleteKey
 afsconf_GetAfsdbInfo
 afsconf_GetAllKeys
 afsconf_GetCellInfo
+afsconf_GetCellName
 afsconf_GetExtendedCellInfo
 afsconf_GetKey
 afsconf_GetLatestKey

diff --git a/src/venus/fs.c b/src/venus/fs.c
index 630f44c..103f309 100644 (file)
--- a/src/venus/fs.c
+++ b/src/venus/fs.c
@@ -66,7 +66,7 @@ static void ZapList(struct AclEntry *);
 static int PruneList(struct AclEntry **, int);
 static int CleanAcl(struct Acl *, char *);
 static int SetVolCmd(struct cmd_syndesc *as, void *arock);
-static int GetCellName(char *, struct afsconf_cell *);
+static int GetCellName(char *, char *, size_t);
 static void Die(int, char *);
 
 /*
@@ -2063,7 +2063,6 @@ CheckServersCmd(struct cmd_syndesc *as, void *arock)
     afs_int32 j;
     afs_int32 temp;
     char *tp;
-    struct afsconf_cell info;
     struct chservinfo checkserv;
 
     memset(&checkserv, 0, sizeof(struct chservinfo));
@@ -2086,12 +2085,12 @@ CheckServersCmd(struct cmd_syndesc *as, void *arock)
 
     /* now copy in optional cell name, if specified */
     if (as->parms[0].items) {
-       code = GetCellName(as->parms[0].items->data, &info);
+       code = GetCellName(as->parms[0].items->data, &checkserv.tbuffer[0],
+                          sizeof(checkserv.tbuffer));
        if (code) {
            return 1;
        }
-       strcpy(checkserv.tbuffer, info.name);
-       checkserv.tsize = strlen(info.name) + 1;
+       checkserv.tsize = strlen(checkserv.tbuffer) + 1;
     } else {
        strcpy(checkserv.tbuffer, "\0");
        checkserv.tsize = 0;
@@ -2957,7 +2956,7 @@ GetCellCmd(struct cmd_syndesc *as, void *arock)
 {
     afs_int32 code;
     struct ViceIoctl blob;
-    struct afsconf_cell info;
+    char cellName[MAXCELLCHARS];
     struct cmd_item *ti;
     struct a {
        afs_int32 stat;
@@ -2970,24 +2969,24 @@ GetCellCmd(struct cmd_syndesc *as, void *arock)
        /* once per cell */
        blob.out_size = sizeof(args);
        blob.out = (caddr_t) & args;
-       code = GetCellName(ti->data, &info);
+       code = GetCellName(ti->data, &cellName[0], sizeof(cellName));
        if (code) {
            error = 1;
            continue;
        }
-       blob.in_size = 1 + strlen(info.name);
-       blob.in = info.name;
+       blob.in_size = 1 + strlen(cellName);
+       blob.in = cellName;
        code = pioctl(0, VIOC_GETCELLSTATUS, &blob, 1);
        if (code) {
            if (errno == ENOENT)
                fprintf(stderr, "%s: the cell named '%s' does not exist\n",
-                       pn, info.name);
+                       pn, cellName);
            else
-               Die(errno, info.name);
+               Die(errno, cellName);
            error = 1;
            continue;
        }
-       printf("Cell %s status: ", info.name);
+       printf("Cell %s status: ", cellName);
        if (args.stat & 2)
            printf("no setuid allowed");
        else
@@ -3004,7 +3003,6 @@ SetCellCmd(struct cmd_syndesc *as, void *arock)
 {
     afs_int32 code;
     struct ViceIoctl blob;
-    struct afsconf_cell info;
     struct cmd_item *ti;
     struct a {
        afs_int32 stat;
@@ -3029,19 +3027,18 @@ SetCellCmd(struct cmd_syndesc *as, void *arock)
     /* set stat for all listed cells */
     for (ti = as->parms[0].items; ti; ti = ti->next) {
        /* once per cell */
-       code = GetCellName(ti->data, &info);
+       code = GetCellName(ti->data, &args.cname[0], sizeof(args.cname));
        if (code) {
            error = 1;
            continue;
        }
-       strcpy(args.cname, info.name);
        blob.in_size = sizeof(args);
        blob.in = (caddr_t) & args;
        blob.out_size = 0;
        blob.out = (caddr_t) 0;
        code = pioctl(0, VIOC_SETCELLSTATUS, &blob, 1);
        if (code) {
-           Die(errno, info.name);      /* XXX added cell name to Die() call */
+           Die(errno, args.cname);     /* XXX added cell name to Die() call */
            error = 1;
        }
     }
@@ -3049,7 +3046,7 @@ SetCellCmd(struct cmd_syndesc *as, void *arock)
 }
 
 static int
-GetCellName(char *cellName, struct afsconf_cell *info)
+GetCellName(char *cellName, char *buf, size_t buf_size)
 {
     struct afsconf_dir *tdir;
     int code;
@@ -3062,7 +3059,7 @@ GetCellName(char *cellName, struct afsconf_cell *info)
        return -1;
     }
 
-    code = afsconf_GetCellInfo(tdir, cellName, AFSCONF_VLDBSERVICE, info);
+    code = afsconf_GetCellName(tdir, cellName, buf, buf_size);
     if (code) {
        fprintf(stderr, "%s: cell %s not in %s\n", pn, cellName,
                AFSDIR_CLIENT_CELLSERVDB_FILEPATH);