Fix #6472 Create resource "globals" when loading a new resource, not at first use

- always create "globals" and avoid to have to test & create a global
  before to use it.
- replace the setNumConcurrentXXX() by incNumConcurrentXXX()
  as the "setNumConcurrentXXX()" was always used to increment
- now the "count" is always accurate, even if it can be > max
- add some comments

diff --git a/bacula/src/dird/dird.c b/bacula/src/dird/dird.c
index 85abf35eb5b7a66d9a7dd50e98424dc6c45b24a4..dbc2b0cbe6c5902e8ad497a24b3370ae452d5d66 100644 (file)
--- a/bacula/src/dird/dird.c
+++ b/bacula/src/dird/dird.c
@@ -220,6 +220,7 @@ static void dir_debug_print(JCR *jcr, FILE *fp)
 /* DELETE ME when bugs in MA1512, MA1632 MA1639 are fixed */
 extern void (*MA1512_reload_job_end_cb)(JCR *,void *);
 static void reload_job_end_cb(JCR *jcr, void *ctx);
+static void connect_and_init_globals();
 
 /* We use a dedicated thread to find the next jobs to run
  * in order to avoid issues with a HUP signal that might
@@ -381,6 +382,7 @@ int main (int argc, char *argv[])
 
    config = New(CONFIG());
    parse_dir_config(config, configfile, M_ERROR_TERM);
+   connect_and_init_globals();
 
    /* If the director variable is not set, check_resources() will stop the process */
    if (init_crypto() != 0) {
@@ -579,6 +581,80 @@ static int find_free_reload_table_entry()
 
 static pthread_mutex_t reload_mutex = PTHREAD_MUTEX_INITIALIZER;
 
+
+/*
+ * Walk through "globals" tables and plug them into the matching resource
+ * After a reload_config() we need to connect the existing "globals" to
+ * the new version of the resources, or create new globals for brand
+ * new resources.
+ * "globals" are first created when loading the resources for the first time
+ */
+static void connect_and_init_globals()
+{
+   CLIENT_GLOBALS *cg;
+   CLIENT *client;
+   foreach_dlist(cg, &client_globals) {
+      client = GetClientResWithName(cg->name);
+      if (!client) {
+         Dmsg1(50, _("Client=%s not found. Assuming it was removed!!!\n"), cg->name);
+      } else {
+         client->globals = cg;      /* Set globals pointer */
+      }
+   }
+   foreach_res(client, R_CLIENT) {
+      if (client->globals == NULL) {
+         client->create_client_globals(); /* if no globals exist, then create it */
+      }
+   }
+   STORE_GLOBALS *sg;
+   STORE *store;
+   foreach_dlist(sg, &store_globals) {
+      store = GetStoreResWithName(sg->name);
+      if (!store) {
+         Dmsg1(50, _("Storage=%s not found. Assuming it was removed!!!\n"), sg->name);
+      } else {
+         store->globals = sg;       /* set globals pointer */
+         Dmsg2(200, "Reload found numConcurrent=%ld for Store %s\n",
+            sg->NumConcurrentJobs, sg->name);
+      }
+   }
+   foreach_res(store, R_STORAGE) {
+      if (store->globals == NULL) {
+         store->create_store_globals(); /* if no globals exist, then create it */
+      }
+   }
+   JOB_GLOBALS *jg;
+   JOB *job;
+   foreach_dlist(jg, &job_globals) {
+      job = GetJobResWithName(jg->name);
+      if (!job) {
+         Dmsg1(50, _("Job=%s not found. Assuming it was removed!!!\n"), jg->name);
+      } else {
+         job->globals = jg;         /* Set globals pointer */
+      }
+   }
+   foreach_res(job, R_JOB) {
+      if (job->globals == NULL) {
+         job->create_job_globals(); /* if no globals exist, then create it */
+      }
+   }
+   SCHED_GLOBALS *schg;
+   SCHED *sched;
+   foreach_dlist(schg, &sched_globals) {
+      sched = GetSchedResWithName(schg->name);
+      if (!sched) {
+         Dmsg1(50, _("Schedule=%s not found. Assuming it was removed!!!\n"), schg->name);
+      } else {
+         sched->globals = schg;     /* Set globals pointer */
+      }
+   }
+   foreach_res(sched, R_SCHEDULE) {
+      if (sched->globals == NULL) {
+         sched->create_sched_globals(); /* if no globals exist, then create it */
+      }
+   }
+}
+
 /*
  * If we get here, we have received a SIGHUP, which means to
  *    reread our configuration file.
@@ -696,52 +772,7 @@ void reload_config(int sig)
          }
       }
       endeach_jcr(jcr);
-      /*
-       * Now walk through globals tables and plug them into the
-       * new resources.
-       */
-      CLIENT_GLOBALS *cg;
-      foreach_dlist(cg, &client_globals) {
-         CLIENT *client;
-         client = GetClientResWithName(cg->name);
-         if (!client) {
-            Qmsg(NULL, M_INFO, 0, _("Client=%s not found. Assuming it was removed!!!\n"), cg->name);
-         } else {
-            client->globals = cg;      /* Set globals pointer */
-         }
-      }
-      STORE_GLOBALS *sg;
-      foreach_dlist(sg, &store_globals) {
-         STORE *store;
-         store = GetStoreResWithName(sg->name);
-         if (!store) {
-            Qmsg(NULL, M_INFO, 0, _("Storage=%s not found. Assuming it was removed!!!\n"), sg->name);
-         } else {
-            store->globals = sg;       /* set globals pointer */
-            Dmsg2(200, "Reload found numConcurrent=%ld for Store %s\n",
-               sg->NumConcurrentJobs, sg->name);
-         }
-      }
-      JOB_GLOBALS *jg;
-      foreach_dlist(jg, &job_globals) {
-         JOB *job;
-         job = GetJobResWithName(jg->name);
-         if (!job) {
-            Qmsg(NULL, M_INFO, 0, _("Job=%s not found. Assuming it was removed!!!\n"), jg->name);
-         } else {
-            job->globals = jg;         /* Set globals pointer */
-         }
-      }
-      SCHED_GLOBALS *schg;
-      foreach_dlist(schg, &sched_globals) {
-         SCHED *sched;
-         sched = GetSchedResWithName(schg->name);
-         if (!sched) {
-            Qmsg(NULL, M_INFO, 0, _("Schedule=%s not found. Assuming it was removed!!!\n"), schg->name);
-         } else {
-            sched->globals = schg;     /* Set globals pointer */
-         }
-      };
+      connect_and_init_globals();
    }
 
    /* Reset other globals */

diff --git a/bacula/src/dird/dird_conf.c b/bacula/src/dird/dird_conf.c
index 59c12b5ee1e245ec11ec5390629edaca3f4a2cb2..e3306b60a9495df6178b67521e986f3b1b5c0baa 100644 (file)
--- a/bacula/src/dird/dird_conf.c
+++ b/bacula/src/dird/dird_conf.c
@@ -105,31 +105,24 @@ void CLIENT::create_client_globals()
 
 int32_t CLIENT::getNumConcurrentJobs()
 {
-   if (!globals) {
-      return 0;
-   }
+   LOCK_GUARD(globals_mutex);
    return globals->NumConcurrentJobs;
 }
 
-void CLIENT::setNumConcurrentJobs(int32_t num)
+int32_t CLIENT::incNumConcurrentJobs(int32_t inc)
 {
-   P(globals_mutex);
-   if (!globals) {
-      create_client_globals();
-   }
-   globals->NumConcurrentJobs = num;
+   P(globals_mutex); // be sure globals is initialized
+   int32_t num = globals->NumConcurrentJobs += inc;
    V(globals_mutex);
    ASSERT(num >= 0);
    Dmsg2(200, "Set NumConcurrentJobs=%ld for Client %s\n",
-      num, globals->name);
+         num, globals->name);
+   return num;
 }
 
 BSOCK *CLIENT::getBSOCK(int timeout)
 {
    P(globals_mutex);
-   if (!globals) {
-      create_client_globals();
-   }
    if (!globals->socket) {
       globals->socket = New(BsockMeeting());
    }
@@ -140,9 +133,6 @@ BSOCK *CLIENT::getBSOCK(int timeout)
 bool CLIENT::getBSOCK_state(POOLMEM *&buf)
 {
    P(globals_mutex);
-   if (!globals) {
-      create_client_globals();
-   }
    if (!globals->socket) {
       globals->socket = New(BsockMeeting());
    }
@@ -153,9 +143,6 @@ bool CLIENT::getBSOCK_state(POOLMEM *&buf)
 void CLIENT::setBSOCK(BSOCK *sock)
 {
    P(globals_mutex);
-   if (!globals) {
-      create_client_globals();
-   }
    if (!globals->socket) {
       globals->socket = New(BsockMeeting());
    }
@@ -179,9 +166,6 @@ char *CLIENT::address(POOLMEM *&buf)
 void CLIENT::setAddress(char *addr)
 {
    P(globals_mutex);
-   if (!globals) {
-      create_client_globals();
-   }
    if (globals->SetIPaddress) {
       free(globals->SetIPaddress);
    }
@@ -191,7 +175,8 @@ void CLIENT::setAddress(char *addr)
 
 bool CLIENT::is_enabled()
 {
-   if (!globals || globals->enabled < 0) {
+   LOCK_GUARD(globals_mutex);
+   if (globals->enabled < 0) { /* not yet modified, use default from resource */
       return Enabled;
    }
    return globals->enabled;
@@ -200,9 +185,6 @@ bool CLIENT::is_enabled()
 void CLIENT::setEnabled(bool val)
 {
    P(globals_mutex);
-   if (!globals) {
-      create_client_globals();
-   }
    /* TODO: We probably need to set -1 (not set) when we are back to the default value */
    globals->enabled = val? 1 : 0;
    V(globals_mutex);
@@ -221,28 +203,25 @@ void JOB::create_job_globals()
 
 int32_t JOB::getNumConcurrentJobs()
 {
-   if (!globals) {
-      return 0;
-   }
+   LOCK_GUARD(globals_mutex);
    return globals->NumConcurrentJobs;
 }
 
-void JOB::setNumConcurrentJobs(int32_t num)
+int32_t JOB::incNumConcurrentJobs(int32_t inc)
 {
    P(globals_mutex);
-   if (!globals) {
-      create_job_globals();
-   }
-   globals->NumConcurrentJobs = num;
+   int32_t num = globals->NumConcurrentJobs += inc;
    V(globals_mutex);
    ASSERT(num >= 0);
    Dmsg2(200, "Set NumConcurrentJobs=%ld for Job %s\n",
       num, globals->name);
+   return num;
 }
 
 bool JOB::is_enabled()
 {
-   if (!globals || globals->enabled < 0) {
+   LOCK_GUARD(globals_mutex);
+   if (globals->enabled < 0) { /* not yet modified, use default from resource */
       return Enabled;
    }
    return globals->enabled;
@@ -251,9 +230,6 @@ bool JOB::is_enabled()
 void JOB::setEnabled(bool val)
 {
    P(globals_mutex);
-   if (!globals) {
-      create_job_globals();
-   }
    globals->enabled = val ? 1 : 0;
    V(globals_mutex);
    Dmsg2(200, "Set Enabled=%d for Job %s\n",
@@ -271,49 +247,42 @@ void STORE::create_store_globals()
 
 int32_t STORE::getNumConcurrentReadJobs()
 {
-   if (!globals) {
-      return 0;
-   }
+   LOCK_GUARD(globals_mutex);
    return globals->NumConcurrentReadJobs;
 }
 
-void STORE::setNumConcurrentReadJobs(int32_t num)
+int32_t STORE::incNumConcurrentReadJobs(int32_t inc)
 {
    P(globals_mutex);
-   if (!globals) {
-      create_store_globals();
-   }
-   globals->NumConcurrentReadJobs = num;
+   int32_t num = globals->NumConcurrentReadJobs += inc;
    V(globals_mutex);
    Dmsg2(200, "Set NumConcurrentReadJobs=%ld for Store %s\n",
       num, globals->name);
    ASSERT(num >= 0);
+   return num;
 }
 
 int32_t STORE::getNumConcurrentJobs()
 {
-   if (!globals) {
-      return 0;
-   }
+   LOCK_GUARD(globals_mutex);
    return globals->NumConcurrentJobs;
 }
 
-void STORE::setNumConcurrentJobs(int32_t num)
+int32_t STORE::incNumConcurrentJobs(int32_t inc)
 {
    P(globals_mutex);
-   if (!globals) {
-      create_store_globals();
-   }
-   globals->NumConcurrentJobs = num;
+   int32_t num = globals->NumConcurrentJobs += inc;
    V(globals_mutex);
    Dmsg2(200, "Set numconcurrentJobs=%ld for Store %s\n",
       num, globals->name);
    ASSERT(num >= 0);
+   return num;
 }
 
 bool STORE::is_enabled()
 {
-   if (!globals || globals->enabled < 0) {
+   LOCK_GUARD(globals_mutex);
+   if (globals->enabled < 0) { /* not yet modified, use default from resource */
       return Enabled;
    }
    return globals->enabled;
@@ -322,9 +291,6 @@ bool STORE::is_enabled()
 void STORE::setEnabled(bool val)
 {
    P(globals_mutex);
-   if (!globals) {
-      create_store_globals();
-   }
    globals->enabled = val ? 1 : 0;
    V(globals_mutex);
    Dmsg2(200, "Set Enabled=%d for Storage %s\n",
@@ -342,7 +308,8 @@ void SCHED::create_sched_globals()
 
 bool SCHED::is_enabled()
 {
-   if (!globals || globals->enabled < 0) {
+   LOCK_GUARD(globals_mutex);
+   if (globals->enabled < 0) { /* not yet modified, use default from resource */
       return Enabled;
    }
    return globals->enabled;
@@ -351,9 +318,6 @@ bool SCHED::is_enabled()
 void SCHED::setEnabled(bool val)
 {
    P(globals_mutex);
-   if (!globals) {
-      create_sched_globals();
-   }
    globals->enabled = val ? 1 : 0;
    V(globals_mutex);
    Dmsg2(200, "Set Enabled=%d for Schedule %s\n",

diff --git a/bacula/src/dird/dird_conf.h b/bacula/src/dird/dird_conf.h
index a3f26a46156d166aea4576cb058b1909c2f33ac9..fde5a0d3e439ec9abea221dcb319dc0c19b32ab5 100644 (file)
--- a/bacula/src/dird/dird_conf.h
+++ b/bacula/src/dird/dird_conf.h
@@ -269,7 +269,7 @@ public:
 class CLIENT {
 public:
    RES hdr;
-   CLIENT_GLOBALS *globals;           /* global variables */
+   CLIENT_GLOBALS *globals;           /* data shared by all version of the res */
    uint32_t FDport;                   /* Where File daemon listens */
    utime_t FileRetention;             /* file retention period in seconds */
    utime_t JobRetention;              /* job retention period in seconds */
@@ -302,7 +302,7 @@ public:
    char *name() const;
    void create_client_globals();
    int32_t getNumConcurrentJobs();
-   void setNumConcurrentJobs(int32_t num);
+   int32_t incNumConcurrentJobs(int32_t inc);
    char *address(POOLMEM *&buf);
    void setAddress(char *addr);
    bool is_enabled();
@@ -332,7 +332,7 @@ public:
 class STORE {
 public:
    RES   hdr;
-   STORE_GLOBALS *globals;            /* global variables */
+   STORE_GLOBALS *globals;            /* data shared by all version of the res */
    uint32_t SDport;                   /* port where Directors connect */
    uint32_t SDDport;                  /* data port for File daemon */
    char *address;
@@ -368,8 +368,8 @@ public:
    void create_store_globals();
    int32_t getNumConcurrentJobs();
    int32_t getNumConcurrentReadJobs();
-   void setNumConcurrentJobs(int32_t num);
-   void setNumConcurrentReadJobs(int32_t num);
+   int32_t incNumConcurrentJobs(int32_t inc);
+   int32_t incNumConcurrentReadJobs(int32_t inc);
    bool is_enabled();
    void setEnabled(bool val);
 };
@@ -431,7 +431,7 @@ public:
 class JOB {
 public:
    RES   hdr;
-   JOB_GLOBALS *globals;              /* global variables */
+   JOB_GLOBALS *globals;              /* data shared by all version of the res */
    uint32_t JobType;                  /* job type (backup, verify, restore */
    uint32_t JobLevel;                 /* default backup/verify level */
    uint32_t RestoreJobId;             /* What -- JobId to restore */
@@ -521,7 +521,7 @@ public:
    char *name() const;
    void create_job_globals();
    int32_t getNumConcurrentJobs();
-   void setNumConcurrentJobs(int32_t num);
+   int32_t incNumConcurrentJobs(int32_t inc);
    bool is_enabled();
    void setEnabled(bool val);
 };

diff --git a/bacula/src/dird/jobq.c b/bacula/src/dird/jobq.c
index dcf3b2c200fc7e256d5a4f3c17ba890c8499f80c..18dc1321ca7ef2085c769d5a0f54bfdc20bbc7fb 100644 (file)
--- a/bacula/src/dird/jobq.c
+++ b/bacula/src/dird/jobq.c
@@ -194,7 +194,6 @@ void *sched_wait(void *arg)
 /* Procedure to update the client->NumConcurrentJobs */
 static void update_client_numconcurrentjobs(JCR *jcr, int val)
 {
-   int num;
    if (!jcr->client) {
       return;
    }
@@ -211,8 +210,7 @@ static void update_client_numconcurrentjobs(JCR *jcr, int val)
       if (jcr->no_client_used() || jcr->wasVirtualFull) {
          break;
       }
-      num = jcr->client->getNumConcurrentJobs();
-      jcr->client->setNumConcurrentJobs(num + val);
+      jcr->client->incNumConcurrentJobs(val);
       break;
    }
 }
@@ -484,12 +482,10 @@ void *jobq_server(void *arg)
           *  put into the ready queue.
           */
          if (jcr->acquired_resource_locks) {
-            int num;
             dec_read_store(jcr);
             dec_write_store(jcr);
             update_client_numconcurrentjobs(jcr, -1);
-            num = jcr->job->getNumConcurrentJobs() - 1;
-            jcr->job->setNumConcurrentJobs(num);
+            jcr->job->incNumConcurrentJobs(-1);
             jcr->acquired_resource_locks = false;
          }
 
@@ -810,8 +806,8 @@ static bool acquire_resources(JCR *jcr)
       Dmsg1(200, "Wstore=%s\n", jcr->wstore->name());
       int num = jcr->wstore->getNumConcurrentJobs();
       if (num < jcr->wstore->MaxConcurrentJobs) {
-         Dmsg1(200, "Inc wncj=%d\n", num + 1);
-         jcr->wstore->setNumConcurrentJobs(num + 1);
+         num = jcr->wstore->incNumConcurrentJobs(1);
+         Dmsg1(200, "Inc wncj=%d\n", num);
       } else if (jcr->rstore) {
          dec_read_store(jcr);
          skip_this_jcr = true;
@@ -837,9 +833,7 @@ static bool acquire_resources(JCR *jcr)
       }
    }
    if (jcr->job->getNumConcurrentJobs() < jcr->job->MaxConcurrentJobs) {
-      int num;
-      num = jcr->job->getNumConcurrentJobs() + 1;
-      jcr->job->setNumConcurrentJobs(num);
+      jcr->job->incNumConcurrentJobs(1);
    } else {
       /* Back out previous locks */
       dec_write_store(jcr);
@@ -871,10 +865,8 @@ bool inc_read_store(JCR *jcr)
         maxread == 0     ||     /* No limit set */
         numread < maxread))     /* Below the limit */
    {
-      num++;
-      numread++;
-      jcr->rstore->setNumConcurrentReadJobs(numread);
-      jcr->rstore->setNumConcurrentJobs(num);
+      numread = jcr->rstore->incNumConcurrentReadJobs(1);
+      num = jcr->rstore->incNumConcurrentJobs(1);
       Dmsg1(200, "Inc rncj=%d\n", num);
       V(rstore_mutex);
       return true;
@@ -887,10 +879,8 @@ void dec_read_store(JCR *jcr)
 {
    if (jcr->rstore) {
       P(rstore_mutex);
-      int numread = jcr->rstore->getNumConcurrentReadJobs() - 1;
-      int num = jcr->rstore->getNumConcurrentJobs() - 1;
-      jcr->rstore->setNumConcurrentReadJobs(numread);
-      jcr->rstore->setNumConcurrentJobs(num);
+      jcr->rstore->incNumConcurrentReadJobs(-1);
+      int num = jcr->rstore->incNumConcurrentJobs(-1);
       Dmsg1(200, "Dec rncj=%d\n", num);
       V(rstore_mutex);
    }
@@ -899,8 +889,7 @@ void dec_read_store(JCR *jcr)
 static void dec_write_store(JCR *jcr)
 {
    if (jcr->wstore) {
-      int num = jcr->wstore->getNumConcurrentJobs() - 1;
+      int num = jcr->wstore->incNumConcurrentJobs(-1);
       Dmsg1(200, "Dec wncj=%d\n", num);
-      jcr->wstore->setNumConcurrentJobs(num);
    }
 }

diff --git a/bacula/src/lib/res.c b/bacula/src/lib/res.c
index c3bdb684d77044a3a4e990d46a3cafdecc5d42f5..6710359ef65f0d1e9dd7f1674a530045ae068cef 100644 (file)
--- a/bacula/src/lib/res.c
+++ b/bacula/src/lib/res.c
@@ -56,7 +56,7 @@ void b_LockRes(const char *file, int line)
        Emsg3(M_ABORT, 0, _("rwl_writelock failure at %s:%d:  ERR=%s\n"),
              file, line, strerror(errstat));
     }
-    res_locked++;
+    res_locked++; /* for debug only */
 }
 
 void b_UnlockRes(const char *file, int line)
@@ -66,7 +66,7 @@ void b_UnlockRes(const char *file, int line)
       Emsg3(M_ABORT, 0, _("rwl_writeunlock failure at %s:%d:. ERR=%s\n"),
            file, line, strerror(errstat));
    }
-   res_locked--;
+   res_locked--;  /* for debug only */
 #ifdef TRACE_RES
    Pmsg4(000, "UnLockRes locked=%d wactive=%d at %s:%d\n",
          res_locked, res_lock.w_active, file, line);