[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Bacula-devel] Patch: Migration jobmedia table insert incomplete


Hello,

Attached, you will find a patch that I am proposing to release.  Feedback 
would be appreciated.  

Best regards,

Kern

On Sunday 10 February 2008 04.52:26 Peter Much wrote:
> Abstract:
> ---------
> Migration jobs do not insert the jobmedia.startfile attribute
> in the catalog.
>
>
> Impact:
> -------
> Very slow restore.
>
>
> Example:
> --------
> bacula=> select jobid,name,type,jobfiles,poolid,priorjobid from job
>          where jobid=1728 or jobid=1729 or jobid=1748 or jobid=1749
>          order by jobid;
>  jobid |        name        | type | jobfiles | poolid | priorjobid
> -------+--------------------+------+----------+--------+------------
>   1728 | HomeDirsDisp       | B    |     4398 |      5 |          0
>   1729 | HomeDirsDisp       | M    |     4398 |      2 |          0
>   1748 | MoveToTapeInternal | g    |        0 |      5 |          0
>   1749 | HomeDirsDisp       | B    |     4398 |      5 |       1729
> (4 rows)
>
>  -> job 1728 saves directly to pool 5. Job 1729 saves the same data
>     to pool 2, and migrating job 1748/1749 moves it on to pool 5.
>
> bacula=> select jobid,firstindex,lastindex,startfile,endfile,
>                 startblock,endblock from jobmedia
>          where jobid=1728 or jobid=1729 or jobid=1748 or jobid=1749
>          order by jobid;
>  jobid | firstindex | lastindex | startfile | endfile | startblock |
> endblock
> -------+------------+-----------+-----------+---------+------------+-------
>---- 1728 |          1 |      4398 |        55 |      55 |          0 |     
> 3466 1729 |          1 |      4398 |         0 |       0 |        217 |
> 223662044 1749 |          1 |      4398 |         0 |      56 |          0
> |      3466 (3 rows)
>
>  -> Job 1728 sets startfile attribute, migrate job 1749 sets just 0.
>
>
> Comment:
> --------
> 1. I have tried to fix the StartFile issue, so that my restores
>    run better. I have not further checked how the other data are
>    derived on Migration and if there may be more problems.
>    Usually I do such, but I was tired at that point. ;)
>
> 2. The existence of jcr->dcr is checked twice in that function,
>    once right after entry, and once at label "ok_out:". Could it
>    be something "stealing" this object inflight?
>    If so, then my earlier using of "dev" as a shorthand for
>    jcr->dcr->dev is illegitim, and things need to be written
>    more cumbersomely.
>    (I have obviousely not gone into the layout of the whole
>    software conception already; therefore these patches should
>    be considered more as suggestions to where the problem seems
>    to arise, and not so much as readymade fixes!)
>
> Patch:
> ------
> --- src/stored/mac.c.orig       Wed Oct  3 13:36:47 2007
> +++ src/stored/mac.c    Sun Feb 10 04:28:54 2008
> @@ -89,7 +89,7 @@
>     }
>
>     Dmsg3(200, "Found %d volumes names for %s. First=%s\n",
> jcr->NumReadVolumes, -      jcr->VolList->VolumeName, Type);
> +      Type, jcr->VolList->VolumeName);
>
>     /* Ready devices for reading and writing */
>     if (!acquire_device_for_read(jcr->read_dcr) ||
> @@ -98,8 +98,26 @@
>        goto bail_out;
>     }
>
> -   Dmsg2(200, "===== After acquire pos %u:%u\n", jcr->dcr->dev->file,
> jcr->dcr->dev->block_num); +   dev = jcr->dcr->dev;
> +   Dmsg2(200, "===== After acquire pos %u:%u\n", dev->file,
> dev->block_num);
>
> +   /* On MIGRATE we must setup some dcr jobmedia data. On BACKUP
> +    * this is done in write_session_label().
> +    */
> +   switch(jcr->JobType) {
> +   case JT_MIGRATE:
> +      if (dev->is_tape()) {
> +         jcr->dcr->StartBlock = dev->block_num;
> +         jcr->dcr->StartFile  = dev->file;
> +      } else {
> +         jcr->dcr->StartBlock = (uint32_t)dev->file_addr;
> +         jcr->dcr->StartFile = (uint32_t)(dev->file_addr >> 32);
> +      }
> +      break;
> +   default:
> +      break;
> +   }
> +
>     set_jcr_job_status(jcr, JS_Running);
>     dir_send_job_status(jcr);
>
> @@ -117,7 +135,6 @@
>
>  ok_out:
>     if (jcr->dcr) {
> -      dev = jcr->dcr->dev;
>        if (ok || dev->can_write()) {
>           /* Flush out final partial block of this session */
>           if (!write_block_to_device(jcr->dcr)) {
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2008.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> _______________________________________________
> Bacula-devel mailing list
> Bacula-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.sourceforge.net/lists/listinfo/bacula-devel


 This patch fixes a migration bug that always has a zero index entry
 (JobMedia record) as the first entry. This causes Bacula to search
 for the first record during a restore rather than seek directly to
 it.

 Apply this patch to Bacula 2.2.8 (and possibly any prior 2.2.x version) with:

 cd <bacula-source>
 patch -p0 <2.2.8-jobmedia.patch
 ./configure <your-options>
 make
 ...
 make install


Index: src/stored/device.c
===================================================================
--- src/stored/device.c	(revision 6391)
+++ src/stored/device.c	(working copy)
@@ -1,7 +1,7 @@
 /*
    Bacula® - The Network Backup Solution
 
-   Copyright (C) 2000-2008 Free Software Foundation Europe e.V.
+   Copyright (C) 2000-2007 Free Software Foundation Europe e.V.
 
    The main author of Bacula is Kern Sibbald, with contributions from
    many others, a complete list can be found in the file AUTHORS.
@@ -200,6 +200,19 @@
    return ok;                               /* device locked */
 }
 
+void set_start_vol_position(DCR *dcr)
+{
+   DEVICE *dev = dcr->dev;
+   /* Set new start position */
+   if (dev->is_tape()) {
+      dcr->StartBlock = dev->block_num;
+      dcr->StartFile = dev->file;
+   } else {
+      dcr->StartBlock = (uint32_t)dev->file_addr;
+      dcr->StartFile  = (uint32_t)(dev->file_addr >> 32);
+   }
+}
+
 /*
  * We have a new Volume mounted, so reset the Volume parameters
  *  concerning this job.  The global changes were made earlier
@@ -208,24 +221,11 @@
 void set_new_volume_parameters(DCR *dcr)
 {
    JCR *jcr = dcr->jcr;
-   DEVICE *dev = dcr->dev;
    if (dcr->NewVol && !dir_get_volume_info(dcr, GET_VOL_INFO_FOR_WRITE)) {
       Jmsg1(jcr, M_ERROR, 0, "%s", jcr->errmsg);
    }
-   /* Set new start/end positions */
-   if (dev->is_tape()) {
-      dcr->StartBlock = dev->block_num;
-      dcr->StartFile = dev->file;
-   } else {
-      dcr->StartBlock = (uint32_t)dev->file_addr;
-      dcr->StartFile  = (uint32_t)(dev->file_addr >> 32);
-   }
-   /* Reset indicies */
-   dcr->VolFirstIndex = 0;
-   dcr->VolLastIndex = 0;
+   set_new_file_parameters(dcr);
    jcr->NumWriteVolumes++;
-   dcr->NewVol = false;
-   dcr->WroteVol = false;
 }
 
 /*
@@ -235,16 +235,8 @@
  */
 void set_new_file_parameters(DCR *dcr)
 {
-   DEVICE *dev = dcr->dev;
+   set_start_vol_position(dcr);
 
-   /* Set new start/end positions */
-   if (dev->is_tape()) {
-      dcr->StartBlock = dev->block_num;
-      dcr->StartFile = dev->file;
-   } else {
-      dcr->StartBlock = (uint32_t)dev->file_addr;
-      dcr->StartFile  = (uint32_t)(dev->file_addr >> 32);
-   }
    /* Reset indicies */
    dcr->VolFirstIndex = 0;
    dcr->VolLastIndex = 0;
Index: src/stored/mac.c
===================================================================
--- src/stored/mac.c	(revision 6391)
+++ src/stored/mac.c	(working copy)
@@ -1,15 +1,7 @@
 /*
- * SD -- mac.c --  responsible for doing
- *     migration, archive, and copy jobs.
- *
- *     Kern Sibbald, January MMVI
- *
- *   Version $Id$
- */
-/*
    Bacula® - The Network Backup Solution
 
-   Copyright (C) 2006-2006 Free Software Foundation Europe e.V.
+   Copyright (C) 2006-2008 Free Software Foundation Europe e.V.
 
    The main author of Bacula is Kern Sibbald, with contributions from
    many others, a complete list can be found in the file AUTHORS.
@@ -33,6 +25,14 @@
    (FSFE), Fiduciary Program, Sumatrastrasse 25, 8006 Zürich,
    Switzerland, email:ftf@xxxxxxxxxxxxxx
 */
+/*
+ * SD -- mac.c --  responsible for doing
+ *     migration, archive, and copy jobs.
+ *
+ *     Kern Sibbald, January MMVI
+ *
+ *   Version $Id$
+ */
 
 #include "bacula.h"
 #include "stored.h"
@@ -108,6 +108,7 @@
 
    jcr->dcr->VolFirstIndex = jcr->dcr->VolLastIndex = 0;
    jcr->run_time = time(NULL);
+   set_start_vol_position(jcr->dcr);
 
    ok = read_records(jcr->read_dcr, record_cb, mount_next_read_volume);
    goto ok_out;
Index: src/stored/protos.h
===================================================================
--- src/stored/protos.h	(revision 6391)
+++ src/stored/protos.h	(working copy)
@@ -126,6 +126,7 @@
 bool     open_device(DCR *dcr);
 bool     first_open_device(DCR *dcr);
 bool     fixup_device_block_write_error(DCR *dcr);
+void     set_start_vol_position(DCR *dcr);
 void     set_new_volume_parameters(DCR *dcr);
 void     set_new_file_parameters(DCR *dcr);
 bool     is_device_unmounted(DEVICE *dev);
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Bacula-devel mailing list
Bacula-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/bacula-devel


This mailing list archive is a service of Copilotco.