Upgraded jobs use wrong pool

With the introduction of a fix for "#1679 Job Overrides are Not listed
In Manual Run Confirmation" we introduced a bug in an interactive run
as we do an pool override early and ignore any changes later on by
setting a suppress boolean value. This doesn't work when a job gets
upgraded later on to lets say Full because no earlier Full was found
but it started out as an incremental Job.

Moved get_level_since_time() from fd_cmds.c to job.c where it
makes much more sense. Fixed comments and let it return a boolean
which is true when it updated the pool due to a missing
full/differential etc. Now when get_level_since_time() returns true
we force apply_pool_overrides() to ignore any overrides and always
reevaluate the pool it needs to save too (fullpool/diffpool/incpool)

Also moved the call to get_level_since_time() and apply_pool_overrides()
into setup_job() so we don't have to code it twice in
do_native_backup_init() and do_ndmp_backup_init().

get_level_since_time() now always encodes the since value into the
jcr->since variable so we only need one argument to the function
being the jcr and no longer a char pointer and size which for most
calls were pointing to something in the jcr anyway.

Fixes #88: Upgraded jobs use wrong pool

src/dird/backup.c

@@ -56,17 +56,6 @@ bool do_native_backup_init(JCR *jcr)
 {
    free_rstorage(jcr);                   /* we don't read so release */
 
-   if (!get_or_create_fileset_record(jcr)) {
-      return false;
-   }
-
-   /*
-    * Get definitive Job level and since time
-    */
-   get_level_since_time(jcr, jcr->since, sizeof(jcr->since));
-
-   apply_pool_overrides(jcr);
-
    if (!allow_duplicate_job(jcr)) {
       return false;
    }

src/dird/fd_cmds.c

@@ -154,125 +154,6 @@ int connect_to_file_daemon(JCR *jcr, int retry_interval, int max_retry_time, int
    return 1;
 }
 
-/*
- * This subroutine edits the last job start time into a
- *   "since=date/time" buffer that is returned in the
- *   variable since.  This is used for display purposes in
- *   the job report.  The time in jcr->stime is later
- *   passed to tell the File daemon what to do.
- */
-void get_level_since_time(JCR *jcr, char *since, int since_len)
-{
-   int JobLevel;
-   bool have_full;
-   bool do_full = false;
-   bool do_diff = false;
-   utime_t now;
-   utime_t last_full_time = 0;
-   utime_t last_diff_time;
-   char prev_job[MAX_NAME_LENGTH];
-
-   since[0] = 0;
-   /* If job cloned and a since time already given, use it */
-   if (jcr->cloned && jcr->stime && jcr->stime[0]) {
-      bstrncpy(since, _(", since="), since_len);
-      bstrncat(since, jcr->stime, since_len);
-      return;
-   }
-   /* Make sure stime buffer is allocated */
-   if (!jcr->stime) {
-      jcr->stime = get_pool_memory(PM_MESSAGE);
-   }
-   jcr->PrevJob[0] = jcr->stime[0] = 0;
-   /*
-    * Lookup the last FULL backup job to get the time/date for a
-    * differential or incremental save.
-    */
-   JobLevel = jcr->getJobLevel();
-   switch (JobLevel) {
-   case L_DIFFERENTIAL:
-   case L_INCREMENTAL:
-      POOLMEM *stime = get_pool_memory(PM_MESSAGE);
-      /* Look up start time of last Full job */
-      now = (utime_t)time(NULL);
-      jcr->jr.JobId = 0;     /* flag to return since time */
-      /*
-       * This is probably redundant, but some of the code below
-       * uses jcr->stime, so don't remove unless you are sure.
-       */
-      if (!db_find_job_start_time(jcr,jcr->db, &jcr->jr, &jcr->stime, jcr->PrevJob)) {
-         do_full = true;
-      }
-      have_full = db_find_last_job_start_time(jcr, jcr->db, &jcr->jr,
-                                              &stime, prev_job, L_FULL);
-      if (have_full) {
-         last_full_time = str_to_utime(stime);
-      } else {
-         do_full = true;               /* No full, upgrade to one */
-      }
-      Dmsg4(50, "have_full=%d do_full=%d now=%lld full_time=%lld\n", have_full,
-            do_full, now, last_full_time);
-      /* Make sure the last diff is recent enough */
-      if (have_full && JobLevel == L_INCREMENTAL && jcr->res.job->MaxDiffInterval > 0) {
-         /* Lookup last diff job */
-         if (db_find_last_job_start_time(jcr, jcr->db, &jcr->jr,
-                                         &stime, prev_job, L_DIFFERENTIAL)) {
-            last_diff_time = str_to_utime(stime);
-            /* If no Diff since Full, use Full time */
-            if (last_diff_time < last_full_time) {
-               last_diff_time = last_full_time;
-            }
-            Dmsg2(50, "last_diff_time=%lld last_full_time=%lld\n", last_diff_time,
-                  last_full_time);
-         } else {
-            /* No last differential, so use last full time */
-            last_diff_time = last_full_time;
-            Dmsg1(50, "No last_diff_time setting to full_time=%lld\n", last_full_time);
-         }
-         do_diff = ((now - last_diff_time) >= jcr->res.job->MaxDiffInterval);
-         Dmsg2(50, "do_diff=%d diffInter=%lld\n", do_diff, jcr->res.job->MaxDiffInterval);
-      }
-      /* Note, do_full takes precedence over do_diff */
-      if (have_full && jcr->res.job->MaxFullInterval > 0) {
-         do_full = ((now - last_full_time) >= jcr->res.job->MaxFullInterval);
-      }
-      free_pool_memory(stime);
-
-      if (do_full) {
-         /* No recent Full job found, so upgrade this one to Full */
-         Jmsg(jcr, M_INFO, 0, "%s", db_strerror(jcr->db));
-         Jmsg(jcr, M_INFO, 0, _("No prior or suitable Full backup found in catalog. Doing FULL backup.\n"));
-         bsnprintf(since, since_len, _(" (upgraded from %s)"),
-            level_to_str(JobLevel));
-         jcr->setJobLevel(jcr->jr.JobLevel = L_FULL);
-       } else if (do_diff) {
-         /* No recent diff job found, so upgrade this one to Diff */
-         Jmsg(jcr, M_INFO, 0, _("No prior or suitable Differential backup found in catalog. Doing Differential backup.\n"));
-         bsnprintf(since, since_len, _(" (upgraded from %s)"), level_to_str(JobLevel));
-         jcr->setJobLevel(jcr->jr.JobLevel = L_DIFFERENTIAL);
-      } else {
-         if (jcr->res.job->rerun_failed_levels) {
-            if (db_find_failed_job_since(jcr, jcr->db, &jcr->jr,
-                                         jcr->stime, JobLevel)) {
-               Jmsg(jcr, M_INFO, 0, _("Prior failed job found in catalog. Upgrading to %s.\n"),
-                  level_to_str(JobLevel));
-               bsnprintf(since, since_len, _(" (upgraded from %s)"),
-                  level_to_str(JobLevel));
-               jcr->setJobLevel(jcr->jr.JobLevel = JobLevel);
-               jcr->jr.JobId = jcr->JobId;
-               break;
-            }
-         }
-         bstrncpy(since, _(", since="), since_len);
-         bstrncat(since, jcr->stime, since_len);
-      }
-      jcr->jr.JobId = jcr->JobId;
-      break;
-   }
-   Dmsg3(100, "Level=%c last start time=%s job=%s\n",
-         JobLevel, jcr->stime, jcr->PrevJob);
-}
-
 static void send_since_time(JCR *jcr)
 {
    BSOCK   *fd = jcr->file_bsock;

src/dird/job.c

@@ -206,6 +206,24 @@ bool setup_job(JCR *jcr)
     */
    switch (jcr->getJobType()) {
    case JT_BACKUP:
+      if (!jcr->is_JobLevel(L_VIRTUAL_FULL)) {
+         if (get_or_create_fileset_record(jcr)) {
+            /*
+             * See if we need to upgrade the level. If get_level_since_time returns true
+             * it has updated the level of the backup and we run apply_pool_overrides
+             * with the force flag so the correct pool (full, diff, incr) is selected.
+             * For all others we respect any set ignore flags.
+             */
+            if (get_level_since_time(jcr)) {
+               apply_pool_overrides(jcr, true);
+            } else {
+               apply_pool_overrides(jcr, false);
+            }
+         } else {
+            goto bail_out;
+         }
+      }
+
       switch (jcr->getJobProtocol()) {
       case PT_NDMP:
          if (!do_ndmp_backup_init(jcr)) {
@@ -835,14 +853,166 @@ bool allow_duplicate_job(JCR *jcr)
    return true;
 }
 
-void apply_pool_overrides(JCR *jcr)
+/*
+ * This subroutine edits the last job start time into a
+ * "since=date/time" buffer that is returned in the
+ * variable since.  This is used for display purposes in
+ * the job report.  The time in jcr->stime is later
+ * passed to tell the File daemon what to do.
+ */
+bool get_level_since_time(JCR *jcr)
+{
+   int JobLevel;
+   bool have_full;
+   bool do_full = false;
+   bool do_diff = false;
+   bool pool_updated = false;
+   utime_t now;
+   utime_t last_full_time = 0;
+   utime_t last_diff_time;
+   char prev_job[MAX_NAME_LENGTH];
+
+   jcr->since[0] = 0;
+
+   /*
+    * If job cloned and a since time already given, use it
+    */
+   if (jcr->cloned && jcr->stime && jcr->stime[0]) {
+      bstrncpy(jcr->since, _(", since="), sizeof(jcr->since));
+      bstrncat(jcr->since, jcr->stime, sizeof(jcr->since));
+      return pool_updated;
+   }
+
+   /*
+    * Make sure stime buffer is allocated
+    */
+   if (!jcr->stime) {
+      jcr->stime = get_pool_memory(PM_MESSAGE);
+   }
+   jcr->PrevJob[0] = jcr->stime[0] = 0;
+
+   /*
+    * Lookup the last FULL backup job to get the time/date for a
+    * differential or incremental save.
+    */
+   JobLevel = jcr->getJobLevel();
+   switch (JobLevel) {
+   case L_DIFFERENTIAL:
+   case L_INCREMENTAL:
+      POOLMEM *stime = get_pool_memory(PM_MESSAGE);
+
+      /*
+       * Look up start time of last Full job
+       */
+      now = (utime_t)time(NULL);
+      jcr->jr.JobId = 0;     /* flag to return since time */
+
+      /*
+       * This is probably redundant, but some of the code below
+       * uses jcr->stime, so don't remove unless you are sure.
+       */
+      if (!db_find_job_start_time(jcr,jcr->db, &jcr->jr, &jcr->stime, jcr->PrevJob)) {
+         do_full = true;
+      }
+
+      have_full = db_find_last_job_start_time(jcr, jcr->db, &jcr->jr,
+                                              &stime, prev_job, L_FULL);
+      if (have_full) {
+         last_full_time = str_to_utime(stime);
+      } else {
+         do_full = true;               /* No full, upgrade to one */
+      }
+
+      Dmsg4(50, "have_full=%d do_full=%d now=%lld full_time=%lld\n", have_full,
+            do_full, now, last_full_time);
+
+      /*
+       * Make sure the last diff is recent enough
+       */
+      if (have_full && JobLevel == L_INCREMENTAL && jcr->res.job->MaxDiffInterval > 0) {
+         /*
+          * Lookup last diff job
+          */
+         if (db_find_last_job_start_time(jcr, jcr->db, &jcr->jr,
+                                         &stime, prev_job, L_DIFFERENTIAL)) {
+            last_diff_time = str_to_utime(stime);
+            /*
+             * If no Diff since Full, use Full time
+             */
+            if (last_diff_time < last_full_time) {
+               last_diff_time = last_full_time;
+            }
+            Dmsg2(50, "last_diff_time=%lld last_full_time=%lld\n", last_diff_time,
+                  last_full_time);
+         } else {
+            /*
+             * No last differential, so use last full time
+             */
+            last_diff_time = last_full_time;
+            Dmsg1(50, "No last_diff_time setting to full_time=%lld\n", last_full_time);
+         }
+         do_diff = ((now - last_diff_time) >= jcr->res.job->MaxDiffInterval);
+         Dmsg2(50, "do_diff=%d diffInter=%lld\n", do_diff, jcr->res.job->MaxDiffInterval);
+      }
+
+      /*
+       * Note, do_full takes precedence over do_diff
+       */
+      if (have_full && jcr->res.job->MaxFullInterval > 0) {
+         do_full = ((now - last_full_time) >= jcr->res.job->MaxFullInterval);
+      }
+      free_pool_memory(stime);
+
+      if (do_full) {
+         /*
+          * No recent Full job found, so upgrade this one to Full
+          */
+         Jmsg(jcr, M_INFO, 0, "%s", db_strerror(jcr->db));
+         Jmsg(jcr, M_INFO, 0, _("No prior or suitable Full backup found in catalog. Doing FULL backup.\n"));
+         bsnprintf(jcr->since, sizeof(jcr->since), _(" (upgraded from %s)"), level_to_str(JobLevel));
+         jcr->setJobLevel(jcr->jr.JobLevel = L_FULL);
+         pool_updated = true;
+       } else if (do_diff) {
+         /*
+          * No recent diff job found, so upgrade this one to Diff
+          */
+         Jmsg(jcr, M_INFO, 0, _("No prior or suitable Differential backup found in catalog. Doing Differential backup.\n"));
+         bsnprintf(jcr->since, sizeof(jcr->since), _(" (upgraded from %s)"), level_to_str(JobLevel));
+         jcr->setJobLevel(jcr->jr.JobLevel = L_DIFFERENTIAL);
+         pool_updated = true;
+      } else {
+         if (jcr->res.job->rerun_failed_levels) {
+            if (db_find_failed_job_since(jcr, jcr->db, &jcr->jr, jcr->stime, JobLevel)) {
+               Jmsg(jcr, M_INFO, 0, _("Prior failed job found in catalog. Upgrading to %s.\n"), level_to_str(JobLevel));
+               bsnprintf(jcr->since, sizeof(jcr->since), _(" (upgraded from %s)"), level_to_str(JobLevel));
+               jcr->setJobLevel(jcr->jr.JobLevel = JobLevel);
+               jcr->jr.JobId = jcr->JobId;
+               pool_updated = true;
+               break;
+            }
+         }
+         bstrncpy(jcr->since, _(", since="), sizeof(jcr->since));
+         bstrncat(jcr->since, jcr->stime, sizeof(jcr->since));
+      }
+      jcr->jr.JobId = jcr->JobId;
+      break;
+   }
+
+   Dmsg3(100, "Level=%c last start time=%s job=%s\n",
+         JobLevel, jcr->stime, jcr->PrevJob);
+
+   return pool_updated;
+}
+
+void apply_pool_overrides(JCR *jcr, bool force)
 {
    bool pool_override = false;
 
    /*
     * If a cmdline pool override is given ignore any level pool overrides.
+    * Unless a force is given then we always apply any overrides.
     */
-   if (jcr->IgnoreLevelPoolOverides) {
+   if (!force && jcr->IgnoreLevelPoolOverides) {
       return;
    }
 
@@ -898,7 +1068,6 @@ void apply_pool_overrides(JCR *jcr)
    }
 }
 
-
 /*
  * Get or create a Client record for this Job
  */

src/dird/ndmp_dma.c

@@ -1112,17 +1112,6 @@ bool do_ndmp_backup_init(JCR *jcr)
 {
    free_rstorage(jcr);                   /* we don't read so release */
 
-   if (!get_or_create_fileset_record(jcr)) {
-      return false;
-   }
-
-   /*
-    * Get definitive Job level and since time
-    */
-   get_level_since_time(jcr, jcr->since, sizeof(jcr->since));
-
-   apply_pool_overrides(jcr);
-
    if (!allow_duplicate_job(jcr)) {
       return false;
    }

src/dird/protos.h

@@ -99,7 +99,6 @@ int get_attributes_and_put_in_catalog(JCR *jcr);
 void get_attributes_and_compare_to_catalog(JCR *jcr, JobId_t JobId);
 int put_file_into_catalog(JCR *jcr, long file_index, char *fname,
                           char *link, char *attr, int stream);
-void get_level_since_time(JCR *jcr, char *since, int since_len);
 int send_runscripts_commands(JCR *jcr);
 bool send_restore_objects(JCR *jcr);
 bool cancel_file_daemon_job(UAContext *ua, JCR *jcr);
@@ -116,7 +115,8 @@ void update_job_end_record(JCR *jcr);
 bool get_or_create_client_record(JCR *jcr);
 bool get_or_create_fileset_record(JCR *jcr);
 DBId_t get_or_create_pool_record(JCR *jcr, char *pool_name);
-void apply_pool_overrides(JCR *jcr);
+bool get_level_since_time(JCR *jcr);
+void apply_pool_overrides(JCR *jcr, bool force = false);
 JobId_t run_job(JCR *jcr);
 bool cancel_job(UAContext *ua, JCR *jcr);
 void get_job_storage(USTORERES *store, JOBRES *job, RUNRES *run);