From: Kern Sibbald <kern@sibbald.com>
Date: Wed, 4 Apr 2007 18:18:12 +0000 (+0000)
Subject: kes  Implement new code for freeing in use volumes that should
X-Git-Tag: Release-7.0.0~6620
X-Git-Url: https://git.sur5r.net/?a=commitdiff_plain;h=abef8271979074d70e8c67a3591d0ce69e8425f2;p=bacula%2Fbacula

kes  Implement new code for freeing in use volumes that should
     resolve if not all, some of the problems of multiple drive
     tape conflicts described in bug 801.

git-svn-id: https://bacula.svn.sourceforge.net/svnroot/bacula/trunk@4507 91ce42f0-d328-0410-95d8-f526ca767f89
---

diff --git a/bacula/src/stored/acquire.c b/bacula/src/stored/acquire.c
index 21ae93a3d3..436c39d235 100644
--- a/bacula/src/stored/acquire.c
+++ b/bacula/src/stored/acquire.c
@@ -558,7 +558,7 @@ bool release_device(DCR *dcr)
       free_pool_memory(alert);
    }
    dcr->dev_locked = false;              /* set no longer locked */
-   unlock_device(dev);
+   dev->unlock();
    if (jcr->read_dcr == dcr) {
       jcr->read_dcr = NULL;
    }
@@ -645,7 +645,7 @@ void detach_dcr_from_dev(DCR *dcr)
          Jmsg1(dcr->jcr, M_ERROR, 0, _("Hey! num_writers=%d!!!!\n"), dev->num_writers);
          dev->num_writers = 0;
       }
-      unlock_device(dev);
+      dev->unlock();
    }
 
    /* Detach this dcr only if attached */
diff --git a/bacula/src/stored/bcopy.c b/bacula/src/stored/bcopy.c
index d5893ede9a..b8a6bd5599 100644
--- a/bacula/src/stored/bcopy.c
+++ b/bacula/src/stored/bcopy.c
@@ -189,10 +189,10 @@ int main (int argc, char *argv[])
    lock_device(out_dev);
    if (out_dev->open(out_jcr->dcr, OPEN_READ_WRITE) < 0) {
       Emsg1(M_FATAL, 0, _("dev open failed: %s\n"), out_dev->errmsg);
-      unlock_device(out_dev);
+      out_dev->unlock();
       exit(1);
    }
-   unlock_device(out_dev);
+   out_dev->unlock();
    if (!acquire_device_for_append(out_jcr->dcr)) {
       free_jcr(in_jcr);
       exit(1);
diff --git a/bacula/src/stored/block.c b/bacula/src/stored/block.c
index a05b9c46cd..faa51c94d5 100644
--- a/bacula/src/stored/block.c
+++ b/bacula/src/stored/block.c
@@ -388,7 +388,7 @@ bool write_block_to_device(DCR *dcr)
 
 bail_out:
    if (!dcr->dev_locked) {            /* did we lock dev above? */
-      unlock_device(dev);             /* unlock it now */
+      dev->unlock();                  /* unlock it now */
    }
    return stat;
 }
diff --git a/bacula/src/stored/btape.c b/bacula/src/stored/btape.c
index d2056e2a02..76e73315b7 100644
--- a/bacula/src/stored/btape.c
+++ b/bacula/src/stored/btape.c
@@ -1,22 +1,7 @@
-/*
- *
- *   Bacula Tape manipulation program
- *
- *    Has various tape manipulation commands -- mostly for
- *    use in determining how tapes really work.
- *
- *     Kern Sibbald, April MM
- *
- *   Note, this program reads stored.conf, and will only
- *     talk to devices that are configured.
- *
- *   Version $Id$
- *
- */
 /*
    Bacula® - The Network Backup Solution
 
-   Copyright (C) 2000-2006 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.
@@ -40,6 +25,21 @@
    (FSFE), Fiduciary Program, Sumatrastrasse 25, 8006 Zürich,
    Switzerland, email:ftf@fsfeurope.org.
 */
+/*
+ *
+ *   Bacula Tape manipulation program
+ *
+ *    Has various tape manipulation commands -- mostly for
+ *    use in determining how tapes really work.
+ *
+ *     Kern Sibbald, April MM
+ *
+ *   Note, this program reads stored.conf, and will only
+ *     talk to devices that are configured.
+ *
+ *   Version $Id$
+ *
+ */
 
 #include "bacula.h"
 #include "stored.h"
@@ -355,21 +355,23 @@ static void terminate_btape(int stat)
 static bool open_the_device()
 {
    DEV_BLOCK *block;
+   bool ok = true;
 
    block = new_block(dev);
    lock_device(dev);
    Dmsg1(200, "Opening device %s\n", dcr->VolumeName);
    if (dev->open(dcr, OPEN_READ_WRITE) < 0) {
       Emsg1(M_FATAL, 0, _("dev open failed: %s\n"), dev->errmsg);
-      unlock_device(dev);
-      free_block(block);
-      return false;
+      ok = false;
+      goto bail_out;
    }
    Pmsg1(000, _("open device %s: OK\n"), dev->print_name());
    dev->set_append();                 /* put volume in append mode */
-   unlock_device(dev);
+
+bail_out:
+   dev->unlock();
    free_block(block);
-   return true;
+   return ok;
 }
 
 
@@ -2383,12 +2385,12 @@ static int flush_block(DEV_BLOCK *block, int dump)
          if (!fixup_device_block_write_error(jcr->dcr)) {
             Pmsg1(000, _("Cannot fixup device error. %s\n"), dev->bstrerror());
             ok = false;
-            unlock_device(dev);
+            dev->unlock();
             return 0;
          }
          BlockNumber = 0;             /* start counting for second tape */
       }
-      unlock_device(dev);
+      dev->unlock();
       return 1;                       /* end of tape reached */
    }
 
@@ -2407,7 +2409,7 @@ static int flush_block(DEV_BLOCK *block, int dump)
    last_file = this_file;
    last_block_num = this_block_num;
 
-   unlock_device(dev);
+   dev->unlock();
    return 1;
 }
 
diff --git a/bacula/src/stored/dev.c b/bacula/src/stored/dev.c
index 689fa9589c..f63f70f79a 100644
--- a/bacula/src/stored/dev.c
+++ b/bacula/src/stored/dev.c
@@ -5,7 +5,7 @@
  *              Kern Sibbald, MM
  *
  *     NOTE!!!! None of these routines are reentrant. You must
- *        use lock_device() and unlock_device() at a higher level,
+ *        use lock_device() and dev->unlock() at a higher level,
  *        or use the xxx_device() equivalents.  By moving the
  *        thread synchronization to a higher level, we permit
  *        the higher level routines to "seize" the device and
@@ -1843,6 +1843,17 @@ void DEVICE::clrerror(int func)
 #endif
 }
 
+
+/*
+ * Clear volume header
+ */
+void DEVICE::clear_volhdr()
+{
+   free_volume(this);
+   memset(&VolHdr, 0, sizeof(VolHdr));
+}
+
+
 /*
  * Close the device
  */
@@ -1878,9 +1889,8 @@ void DEVICE::close()
    EndFile = EndBlock = 0;
    openmode = 0;
    Slot = -1;             /* unknown slot */
-   free_volume(this);
+   clear_volhdr();
    memset(&VolCatInfo, 0, sizeof(VolCatInfo));
-   memset(&VolHdr, 0, sizeof(VolHdr));
    if (tid) {
       stop_thread_timer(tid);
       tid = 0;
@@ -2439,8 +2449,7 @@ void set_os_device_parameters(DCR *dcr)
          dev->clrerror(MTSETBSIZ);
       }
    }
-/* Turn this on later when fully tested */
-#if defined(xxxMTIOCSETEOTMODEL) 
+#if defined(MTIOCSETEOTMODEL) 
    uint32_t neof;
    if (dev->has_cap(CAP_TWOEOF)) {
       neof = 2;
diff --git a/bacula/src/stored/dev.h b/bacula/src/stored/dev.h
index 134618c7b0..b37c48d195 100644
--- a/bacula/src/stored/dev.h
+++ b/bacula/src/stored/dev.h
@@ -41,12 +41,6 @@
 
 #undef DCR                            /* used by Bacula */
 
-/* #define NEW_LOCK 1 */
-
-#define new_lock_device(dev)             _new_lock_device(__FILE__, __LINE__, (dev))
-#define new_lock_device_state(dev,state) _new_lock_device(__FILE__, __LINE__, (dev), (state))
-#define new_unlock_device(dev)           _new_unlock_device(__FILE__, __LINE__, (dev))
-
 #define lock_device(d) _lock_device(__FILE__, __LINE__, (d))
 #define unlock_device(d) _unlock_device(__FILE__, __LINE__, (d))
 #define block_device(d, s) _block_device(__FILE__, __LINE__, (d), s)
@@ -369,6 +363,7 @@ public:
    void lock() { P(m_mutex); } 
    void unlock() { V(m_mutex); } 
 
+   void clear_volhdr();          /* in dev.c */
    void block(int why);          /* in dev.c */
    void unblock();               /* in dev.c */
    void close();                 /* in dev.c */
diff --git a/bacula/src/stored/device.c b/bacula/src/stored/device.c
index 810b3da170..2696ecc0b5 100644
--- a/bacula/src/stored/device.c
+++ b/bacula/src/stored/device.c
@@ -99,7 +99,7 @@ bool fixup_device_block_write_error(DCR *dcr)
 
    block_device(dev, BST_DOING_ACQUIRE);
    /* Unlock, but leave BLOCKED */
-   unlock_device(dev);
+   dev->unlock();
 
    bstrncpy(PrevVolName, dev->VolCatInfo.VolCatName, sizeof(PrevVolName));
    bstrncpy(dev->VolHdr.PrevVolumeName, PrevVolName, sizeof(dev->VolHdr.PrevVolumeName));
@@ -284,7 +284,7 @@ bool first_open_device(DCR *dcr)
    Dmsg1(129, "open dev %s OK\n", dev->print_name());
 
 bail_out:
-   unlock_device(dev);
+   dev->unlock();
    return ok;
 }
 
@@ -317,25 +317,6 @@ bool open_device(DCR *dcr)
 }
 
 
-
-#ifdef xxx
-void dev_lock(DEVICE *dev)
-{
-   int errstat;
-   if ((errstat=rwl_writelock(&dev->lock))) {
-      Emsg1(M_ABORT, 0, _("Device write lock failure. ERR=%s\n"), strerror(errstat));
-   }
-}
-
-void dev_unlock(DEVICE *dev)
-{
-   int errstat;
-   if ((errstat=rwl_writeunlock(&dev->lock))) {
-      Emsg1(M_ABORT, 0, _("Device write unlock failure. ERR=%s\n"), strerror(errstat));
-   }
-}
-#endif 
-
 /*
  * When dev_blocked is set, all threads EXCEPT thread with id no_wait_id
  * must wait. The no_wait_id thread is out obtaining a new volume
diff --git a/bacula/src/stored/dircmd.c b/bacula/src/stored/dircmd.c
index d78f723143..ba5a2140c6 100644
--- a/bacula/src/stored/dircmd.c
+++ b/bacula/src/stored/dircmd.c
@@ -1,24 +1,3 @@
-/*
- *  This file handles accepting Director Commands
- *
- *    Most Director commands are handled here, with the
- *    exception of the Job command command and subsequent
- *    subcommands that are handled
- *    in job.c.
- *
- *    N.B. in this file, in general we must use P(dev->mutex) rather
- *      than lock_device(dev) so that we can examine the blocked
- *      state rather than blocking ourselves because a Job
- *      thread has the device blocked. In some "safe" cases,
- *      we can do things to a blocked device. CAREFUL!!!!
- *
- *    File daemon commands are handled in fdcmd.c
- *
- *     Kern Sibbald, May MMI
- *
- *   Version $Id$
- *
- */
 /*
    Bacula® - The Network Backup Solution
 
@@ -506,7 +485,7 @@ static void label_volume_if_ok(DCR *dcr, char *oldname,
 
 bail_out:
    if (!dev->is_open()) {
-      free_volume(dev);
+      dev->clear_volhdr();
    }
    give_back_device_lock(dev, &hold);
    return;
diff --git a/bacula/src/stored/label.c b/bacula/src/stored/label.c
index da6f9908ad..063375c548 100644
--- a/bacula/src/stored/label.c
+++ b/bacula/src/stored/label.c
@@ -332,8 +332,7 @@ bool write_new_volume_label_to_dev(DCR *dcr, const char *VolName,
    }
    Dmsg1(150, "Label type=%d\n", dev->label_type);
    if (!dev->rewind(dcr)) {
-      free_volume(dev);
-      memset(&dev->VolHdr, 0, sizeof(dev->VolHdr));
+      dev->clear_volhdr();
       Dmsg2(30, "Bad status on %s from rewind: ERR=%s\n", dev->print_name(), dev->print_errmsg());
       if (!forge_on) {
          goto bail_out;
@@ -399,8 +398,7 @@ bool write_new_volume_label_to_dev(DCR *dcr, const char *VolName,
    return true;
 
 bail_out:
-   free_volume(dev);
-   memset(&dev->VolHdr, 0, sizeof(dev->VolHdr));
+   dev->clear_volhdr();
    dev->clear_append();               /* remove append since this is PRE_LABEL */
    return false;
 }
@@ -589,8 +587,7 @@ void create_volume_label(DEVICE *dev, const char *VolName,
 
    ASSERT(dev != NULL);
 
-   free_volume(dev);         /* release any old volume */
-   memset(&dev->VolHdr, 0, sizeof(dev->VolHdr));
+   dev->clear_volhdr();          /* release any old volume */
 
    bstrncpy(dev->VolHdr.Id, BaculaId, sizeof(dev->VolHdr.Id));
    dev->VolHdr.VerNum = BaculaTapeVersion;
diff --git a/bacula/src/stored/mount.c b/bacula/src/stored/mount.c
index 6ad18646b5..9d542b98f4 100644
--- a/bacula/src/stored/mount.c
+++ b/bacula/src/stored/mount.c
@@ -590,8 +590,7 @@ void release_volume(DCR *dcr)
    dev->EndBlock = dev->EndFile = 0;
    memset(&dev->VolCatInfo, 0, sizeof(dev->VolCatInfo));
    memset(&dcr->VolCatInfo, 0, sizeof(dcr->VolCatInfo));
-   free_volume(dev);
-   memset(&dev->VolHdr, 0, sizeof(dev->VolHdr));
+   dev->clear_volhdr();
    /* Force re-read of label */
    dev->clear_labeled();
    dev->clear_read();
diff --git a/bacula/src/stored/reserve.c b/bacula/src/stored/reserve.c
index c1d4dc4c91..4b459993f9 100644
--- a/bacula/src/stored/reserve.c
+++ b/bacula/src/stored/reserve.c
@@ -138,12 +138,18 @@ VOLRES *new_volume(DCR *dcr, const char *VolumeName)
     */
    lock_reservations();
    P(vol_list_lock);
+   /*
+    * First, if this dcr already is attached to any device,
+    *  remove any old volumes attached to this device as they
+    *  are no longer used (there should at max be one).
+    */
    if (dcr->dev) {
 again:
       foreach_dlist(vol, vol_list) {
          if (vol && vol->dev == dcr->dev) {
             vol_list->remove(vol);
             if (vol->vol_name) {
+               Dmsg1(100, "new_vol free vol=%s\n", vol->vol_name);
                free(vol->vol_name);
             }
             free(vol);
@@ -157,20 +163,29 @@ again:
    vol->dev = dcr->dev;
    vol->dcr = dcr;
    Dmsg2(100, "New Vol=%s dev=%s\n", VolumeName, dcr->dev->print_name());
+   /*
+    * Now try to insert the new Volume
+    */
    nvol = (VOLRES *)vol_list->binary_insert(vol, my_compare);
    if (nvol != vol) {
+      Dmsg2(100, "Found vol=%s same dcr=%d\n", nvol->vol_name, dcr==nvol->dcr);
+      /*
+       * At this point, a Volume with this name already is in the
+       *  list, free our temp structure
+       */
       free(vol->vol_name);
       free(vol);
       vol = NULL;
       if (dcr->dev) {
          DEVICE *dev = nvol->dev;
+         /* ***FIXME*** don't we need a mutex here? */
          if (!dev->is_busy()) {
             Dmsg3(100, "Swap vol=%s from dev=%s to %s\n", VolumeName,
                dev->print_name(), dcr->dev->print_name());
             nvol->dev = dcr->dev;
             dev->VolHdr.VolumeName[0] = 0;
          } else {
-            Dmsg3(100, "!!!! could not swap vol=%s from dev=%s to %s\n", VolumeName,
+            Dmsg3(100, "Logic ERROR!!!! could not swap vol=%s from dev=%s to %s\n", VolumeName,
                dev->print_name(), dcr->dev->print_name());
          }
       }
@@ -195,6 +210,7 @@ VOLRES *find_volume(const char *VolumeName)
    fvol = (VOLRES *)vol_list->binary_search(&vol, my_compare);
    free(vol.vol_name);
    V(vol_list_lock);
+   Dmsg2(100, "find_vol=%s found=%d\n", VolumeName, fvol!=NULL);
    return fvol;
 }
 
@@ -208,8 +224,12 @@ bool free_volume(DEVICE *dev)
 {
    VOLRES vol, *fvol;
 
-   P(vol_list_lock);
    if (dev->VolHdr.VolumeName[0] == 0) {
+      return false;
+   }
+
+   P(vol_list_lock);
+#ifdef xxx
       Dmsg1(100, "free_volume: no vol on dev %s\n", dev->print_name());
       /*
        * Our device has no VolumeName listed, but
@@ -229,6 +249,7 @@ bool free_volume(DEVICE *dev)
       }
       goto bail_out;
    }
+#endif
    Dmsg1(400, "free_volume %s\n", dev->VolHdr.VolumeName);
    vol.vol_name = bstrdup(dev->VolHdr.VolumeName);
    fvol = (VOLRES *)vol_list->binary_search(&vol, my_compare);
@@ -239,8 +260,8 @@ bool free_volume(DEVICE *dev)
       free(fvol);
    }
    free(vol.vol_name);
-   dev->VolHdr.VolumeName[0] = 0;
-bail_out:
+//   dev->VolHdr.VolumeName[0] = 0;
+//bail_out:
    V(vol_list_lock);
    return fvol != NULL;
 }
@@ -252,6 +273,12 @@ void free_unused_volume(DCR *dcr)
 
    P(vol_list_lock);
    for (vol=(VOLRES *)vol_list->first(); vol; vol=(VOLRES *)vol_list->next(vol)) {
+      /*
+       * Releease this volume, but only if we inserted it (same dcr) and
+       *  it is not attached to a device or the Volume in the device is
+       *  different.
+       * I wonder if this is right, kes ...
+       */
       if (vol->dcr == dcr && (vol->dev == NULL || 
           strcmp(vol->vol_name, vol->dev->VolHdr.VolumeName) != 0)) {
          vol_list->remove(vol);
diff --git a/bacula/src/stored/spool.c b/bacula/src/stored/spool.c
index abcdb0a7b9..aaaed4d087 100644
--- a/bacula/src/stored/spool.c
+++ b/bacula/src/stored/spool.c
@@ -204,6 +204,10 @@ static bool close_data_spool_file(DCR *dcr)
 
 static const char *spool_name = "*spool*";
 
+/*
+ * NB! This routine locks the device, but if committing will
+ *     not unlock it. If not committing, it will be unlocked.
+ */
 static bool despool_data(DCR *dcr, bool commit)
 {
    DEVICE *rdev;
@@ -215,7 +219,8 @@ static bool despool_data(DCR *dcr, bool commit)
    char ec1[50];
 
    Dmsg0(100, "Despooling data\n");
-   /* Commit means that the job is done, so we commit, otherwise, we
+   /*
+    * Commit means that the job is done, so we commit, otherwise, we
     *  are despooling because of user spool size max or some error  
     *  (e.g. filesystem full).
     */
@@ -325,7 +330,7 @@ static bool despool_data(DCR *dcr, bool commit)
    /* If doing a commit, leave the device locked -- unlocked in release_device() */
    if (!commit) {
       dcr->dev_locked = false;
-      unlock_device(dcr->dev);
+      dcr->dev->unlock();
    }
    return ok;
 }
diff --git a/bacula/src/stored/stored.c b/bacula/src/stored/stored.c
index 290dca1b5b..efe7c5a416 100644
--- a/bacula/src/stored/stored.c
+++ b/bacula/src/stored/stored.c
@@ -579,7 +579,7 @@ void terminate_stored(int sig)
    foreach_res(device, R_DEVICE) {
       Dmsg1(10, "Term device %s\n", device->device_name);
       if (device->dev) {
-         free_volume(device->dev);
+         device->dev->clear_volhdr();
          device->dev->term();
          device->dev = NULL;
       } else {
diff --git a/bacula/src/version.h b/bacula/src/version.h
index 8a22001560..d94769d542 100644
--- a/bacula/src/version.h
+++ b/bacula/src/version.h
@@ -3,9 +3,9 @@
  */
 
 #undef  VERSION
-#define VERSION "2.1.6"
-#define BDATE   "02 April 2007"
-#define LSMDATE "02Apr07"
+#define VERSION "2.1.7"
+#define BDATE   "04 April 2007"
+#define LSMDATE "04Apr07"
 
 #define PROG_COPYRIGHT "Copyright (C) %d-2007 Free Software Foundation Europe e.V.\n"
 #define BYEAR "2007"       /* year for copyright messages in progs */