From: Kern Sibbald <kern@sibbald.com>
Date: Sat, 12 Apr 2008 20:59:25 +0000 (+0000)
Subject: kes  Rework class structures for VOLRES, DCR, and DEVICE to make
X-Git-Tag: Release-3.0.0~1563
X-Git-Url: https://git.sur5r.net/?a=commitdiff_plain;h=17350b711d777172e5ef8b1e865d979e92ff0a5c;p=bacula%2Fbacula

kes  Rework class structures for VOLRES, DCR, and DEVICE to make
     the method names a bit more logical, and for more logically
     handling the responsibilities.

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

diff --git a/bacula/src/stored/acquire.c b/bacula/src/stored/acquire.c
index 58ed74db62..67080def3d 100644
--- a/bacula/src/stored/acquire.c
+++ b/bacula/src/stored/acquire.c
@@ -286,11 +286,7 @@ default_path:
 
 get_out:
    dev->dlock();
-   if (dcr && dcr->reserved_device) {
-      dev->reserved_device--;
-      Dmsg2(50, "Dec reserve=%d dev=%s\n", dev->reserved_device, dev->print_name());
-      dcr->reserved_device = false;
-   }
+   dcr->clear_reserved();
    /* 
     * Normally we are blocked, but in at least one error case above 
     *   we are not blocked because we unsuccessfully tried changing
@@ -317,6 +313,7 @@ DCR *acquire_device_for_append(DCR *dcr)
 {
    DEVICE *dev = dcr->dev;
    JCR *jcr = dcr->jcr;
+   bool ok = false;
 
    init_device_wait_timers(dcr);
 
@@ -358,8 +355,8 @@ DCR *acquire_device_for_append(DCR *dcr)
             /* Reduce "noise" -- don't print if job canceled */
             Jmsg(jcr, M_FATAL, 0, _("Could not ready device %s for append.\n"),
                dev->print_name());
-            Dmsg2(200, "jid=%u Could not ready device %s for append.\n", 
-               (uint32_t)jcr->JobId, dev->print_name());
+            Dmsg1(200, "Could not ready device %s for append.\n", 
+               dev->print_name());
          }
          goto get_out;
       }
@@ -372,29 +369,13 @@ DCR *acquire_device_for_append(DCR *dcr)
    }
    dev->VolCatInfo.VolCatJobs++;              /* increment number of jobs on vol */
    dir_update_volume_info(dcr, false, false); /* send Volume info to Director */
-   dev->dlock();
-   if (dcr->reserved_device) {
-      dev->reserved_device--;
-      Dmsg3(100, "jid=%u Dec reserve=%d dev=%s\n", (uint32_t)jcr->JobId,
-            dev->reserved_device, dev->print_name());
-      dcr->reserved_device = false;
-   }
-   dev->dunblock(DEV_LOCKED);
-   return dcr;
+   ok = true;
 
-/*
- * Error return
- */
 get_out:
    dev->dlock();
-   if (dcr->reserved_device) {
-      dev->reserved_device--;
-      Dmsg3(100, "jid=%u Dec reserve=%d dev=%s\n", (uint32_t)jcr->JobId, 
-            dev->reserved_device, dev->print_name());
-      dcr->reserved_device = false;
-   }
+   dcr->clear_reserved();
    dev->dunblock(DEV_LOCKED);
-   return NULL;
+   return ok ? dcr : NULL;
 }
 
 /*
@@ -447,11 +428,7 @@ bool release_device(DCR *dcr)
    Dmsg2(100, "release_device device %s is %s\n", dev->print_name(), dev->is_tape()?"tape":"disk");
 
    /* if device is reserved, job never started, so release the reserve here */
-   if (dcr->reserved_device) {
-      dev->reserved_device--;
-      Dmsg2(100, "Dec reserve=%d dev=%s\n", dev->reserved_device, dev->print_name());
-      dcr->reserved_device = false;
-   }
+   dcr->clear_reserved();
 
    if (dev->can_read()) {
       dev->clear_read();              /* clear read bit */
@@ -664,7 +641,7 @@ void detach_dcr_from_dev(DCR *dcr)
    /* Detach this dcr only if attached */
    if (dcr->attached_to_dev && dev) {
       dev->dlock();
-      unreserve_device(dcr);
+      dcr->unreserve_device();
       dcr->dev->attached_dcrs->remove(dcr);  /* detach dcr from device */
       dcr->attached_to_dev = false;
 //    remove_dcr_from_dcrs(dcr);      /* remove dcr from jcr list */
diff --git a/bacula/src/stored/append.c b/bacula/src/stored/append.c
index a6ff753814..382dd2d556 100644
--- a/bacula/src/stored/append.c
+++ b/bacula/src/stored/append.c
@@ -70,7 +70,7 @@ bool do_append_data(JCR *jcr)
       return false;
    }                                              
 
-   Dmsg1(100, "Start append data. res=%d\n", dev->reserved_device);
+   Dmsg1(100, "Start append data. res=%d\n", dev->num_reserved());
 
    memset(&rec, 0, sizeof(rec));
 
diff --git a/bacula/src/stored/askdir.c b/bacula/src/stored/askdir.c
index 8d993457e0..dcaa00b8d8 100644
--- a/bacula/src/stored/askdir.c
+++ b/bacula/src/stored/askdir.c
@@ -242,7 +242,7 @@ bool dir_get_volume_info(DCR *dcr, enum get_vol_info_rw writing)
  * Returns: true  on success dcr->VolumeName is volume
  *                reserve_volume() called on Volume name
  *          false on failure dcr->VolumeName[0] == 0
- *                also sets dcr->volume_in_use if at least one 
+ *                also sets dcr->found_in_use if at least one 
  *                in use volume was found.
  *
  *          Volume information returned in dcr
@@ -255,7 +255,7 @@ bool dir_find_next_appendable_volume(DCR *dcr)
     bool rtn;
 
     Dmsg2(200, "dir_find_next_appendable_volume: reserved=%d Vol=%s\n", 
-       dcr->reserved_device, dcr->VolumeName);
+       dcr->is_reserved(), dcr->VolumeName);
 
     /*
      * Try the forty oldest or most available volumes.  Note,
@@ -264,7 +264,7 @@ bool dir_find_next_appendable_volume(DCR *dcr)
      */
     lock_volumes();
     P(vol_info_mutex);
-    dcr->volume_in_use = false;
+    dcr->clear_found_in_use();
     for (int vol_index=1;  vol_index < 40; vol_index++) {
        bash_spaces(dcr->media_type);
        bash_spaces(dcr->pool_name);
@@ -273,7 +273,7 @@ bool dir_find_next_appendable_volume(DCR *dcr)
        unbash_spaces(dcr->pool_name);
        Dmsg1(100, ">dird %s", dir->msg);
        if (do_get_volume_info(dcr)) {
-          if (!is_volume_in_use(dcr)) {
+          if (dcr->can_i_use_volume()) {
              Dmsg1(100, "Call reserve_volume. Vol=%s\n", dcr->VolumeName);
              if (reserve_volume(dcr, dcr->VolumeName) == 0) {
                 Dmsg2(100, "Could not reserve volume %s on %s\n", dcr->VolumeName,
@@ -286,7 +286,8 @@ bool dir_find_next_appendable_volume(DCR *dcr)
              goto get_out;
           } else {
              Dmsg1(100, "Volume %s is in use.\n", dcr->VolumeName);
-             dcr->volume_in_use = true;
+             /* If volume is not usable, it is in use by someone else */
+             dcr->set_found_in_use();
              continue;
           }
        }
diff --git a/bacula/src/stored/autochanger.c b/bacula/src/stored/autochanger.c
index a6f37b6716..9ed6c8f768 100644
--- a/bacula/src/stored/autochanger.c
+++ b/bacula/src/stored/autochanger.c
@@ -136,6 +136,7 @@ int autoload_device(DCR *dcr, int writing, BSOCK *dir)
       if (dir) {
          return 0;                    /* For user, bail out right now */
       }
+      /* ***FIXME*** this really should not be here */
       if (dir_find_next_appendable_volume(dcr)) {
          slot = dcr->VolCatInfo.InChanger ? dcr->VolCatInfo.Slot : 0;
       } else {
@@ -367,6 +368,7 @@ bool unload_autochanger(DCR *dcr, int loaded)
       } else {
          dev->Slot = 0;            /* nothing loaded */
       }
+      dev->clear_unload();
       free_volume(dev);            /* Free any volume associated with this drive */
       free_pool_memory(changer);
       unlock_changer(dcr);
@@ -421,7 +423,7 @@ static bool unload_other_drive(DCR *dcr, int slot)
            dcr->VolumeName, dev->print_name());
       Dmsg4(100, "Vol %s for dev=%s is busy dev=%s slot=%d\n",
            dcr->VolumeName, dcr->dev->print_name(), dev->print_name(), dev->Slot);
-      Dmsg2(100, "num_writ=%d reserv=%d\n", dev->num_writers, dev->reserved_device);
+      Dmsg2(100, "num_writ=%d reserv=%d\n", dev->num_writers, dev->num_reserved());
       return false;
    }
    return unload_dev(dcr, dev);
@@ -459,7 +461,7 @@ bool unload_dev(DCR *dcr, DEVICE *dev)
                 dcr->device->changer_command, "unload");
    dev->close();
    Dmsg2(200, "close dev=%s reserve=%d\n", dev->print_name(), 
-      dev->reserved_device);
+      dev->num_reserved());
    Dmsg1(100, "Run program=%s\n", changer_cmd);
    int stat = run_program_full_output(changer_cmd, timeout, results.addr());
    dcr->VolCatInfo.Slot = save_slot;
@@ -479,6 +481,7 @@ bool unload_dev(DCR *dcr, DEVICE *dev)
       Dmsg0(100, "Slot unloaded\n");
    }
    free_volume(dev);               /* Free any volume associated with this drive */
+   dev->clear_unload();
    unlock_changer(dcr);
    dev->dunlock();
    free_pool_memory(changer_cmd);
diff --git a/bacula/src/stored/dev.c b/bacula/src/stored/dev.c
index 7c1410b410..99eee1f09c 100644
--- a/bacula/src/stored/dev.c
+++ b/bacula/src/stored/dev.c
@@ -711,7 +711,7 @@ bool DEVICE::rewind(DCR *dcr)
    unsigned int i;
    bool first = true;
 
-   Dmsg3(400, "rewind res=%d fd=%d %s\n", reserved_device, m_fd, print_name());
+   Dmsg3(400, "rewind res=%d fd=%d %s\n", num_reserved(), m_fd, print_name());
    state &= ~(ST_EOT|ST_EOF|ST_WEOT);  /* remove EOF/EOT flags */
    block_num = file = 0;
    file_size = 0;
diff --git a/bacula/src/stored/dev.h b/bacula/src/stored/dev.h
index edb13c7e39..a8f7bdf84e 100644
--- a/bacula/src/stored/dev.h
+++ b/bacula/src/stored/dev.h
@@ -35,6 +35,37 @@
  *
  */
 
+/*
+ * Some details of how volume and device reservations work
+ *
+ * class VOLRES:
+ *   set_in_use()     volume being used on current drive
+ *   clear_in_use()   no longer being used.  Can be re-used or moved.
+ *   set_swapping()   set volume being moved to another drive
+ *   is_swapping()    volume is being moved to another drive
+ *   clear_swapping() volume normal
+ *
+ * class DEVICE:
+ *   set_load()       set to load volume
+ *   needs_load()     volume must be loaded (i.e. set_load done)
+ *   clear_load()     load done.
+ *   set_unload()     set to unload volume
+ *   needs_unload()    volume must be unloaded
+ *   clear_unload()   volume unloaded
+ *
+ *    reservations are temporary until the drive is acquired
+ *   inc_reserved()   increments num of reservations
+ *   dec_reserved()   decrements num of reservations
+ *   num_reserved()   number of reservations
+ *
+ * class DCR:
+ *   set_reserved()   sets local reserve flag and calls dev->inc_reserved()
+ *   clear_reserved() clears local reserve flag and calls dev->dec_reserved()
+ *   is_reserved()    returns local reserved flag
+ *   unreserve_device()  much more complete unreservation
+ *
+ */
+
 
 #ifndef __DEV_H
 #define __DEV_H 1
@@ -182,7 +213,11 @@ private:
    int m_blocked;                     /* set if we must wait (i.e. change tape) */
    int m_count;                       /* Mutex use count -- DEBUG only */
    pthread_t m_pid;                   /* Thread that locked -- DEBUG only */
+   bool m_unload;                     /* set when Volume must be unloaded */
+   bool m_load;                       /* set when Volume must be loaded */
+   int m_num_reserved;                /* counter of device reservations */
 public:
+   DEVICE * volatile swap_dev;        /* Swap vol from this device */
    dlist *attached_dcrs;              /* attached DCR list */
    pthread_mutex_t m_mutex;           /* access control */
    pthread_mutex_t spool_mutex;       /* mutex for updating spool_size */
@@ -192,7 +227,6 @@ public:
    int dev_prev_blocked;              /* previous blocked state */
    int num_waiting;                   /* number of threads waiting */
    int num_writers;                   /* number of writing threads */
-   int reserved_device;               /* number of device reservations */
    int capabilities;                  /* capabilities mask */
    int state;                         /* state mask */
    int dev_errno;                     /* Our own errno */
@@ -283,10 +317,11 @@ public:
    int is_labeled() const { return state & ST_LABEL; }
    int is_mounted() const { return state & ST_MOUNTED; }
    int is_unmountable() const { return (is_dvd() || (is_file() && is_removable())); }
+   int num_reserved() const { return m_num_reserved; };
    int is_part_spooled() const { return state & ST_PART_SPOOLED; }
    int have_media() const { return state & ST_MEDIA; }
    int is_short_block() const { return state & ST_SHORT; }
-   int is_busy() const { return (state & ST_READ) || num_writers || reserved_device; }
+   int is_busy() const { return (state & ST_READ) || num_writers || num_reserved(); }
    int at_eof() const { return state & ST_EOF; }
    int at_eot() const { return state & ST_EOT; }
    int at_weot() const { return state & ST_WEOT; }
@@ -308,6 +343,8 @@ public:
                     (m_blocked == BST_UNMOUNTED ||
                      m_blocked == BST_WAITING_FOR_SYSOP ||
                      m_blocked == BST_UNMOUNTED_WAITING_FOR_SYSOP); };
+   bool must_unload() const { return m_unload; };
+   bool must_load() const { return m_load; };
    const char *strerror() const;
    const char *archive_name() const;
    const char *name() const;
@@ -327,8 +364,12 @@ public:
    void set_part_spooled(int val) { if (val) state |= ST_PART_SPOOLED; \
           else state &= ~ST_PART_SPOOLED;
    };
+   void set_unload() { m_unload = true; };
+   void set_load() { m_load = true; };
+   void inc_reserved() { m_num_reserved++; }
    void set_mounted(int val) { if (val) state |= ST_MOUNTED; \
           else state &= ~ST_MOUNTED; };
+   void dec_reserved() { m_num_reserved--; ASSERT(m_num_reserved>=0); };
    void clear_append() { state &= ~ST_APPEND; };
    void clear_read() { state &= ~ST_READ; };
    void clear_labeled() { state &= ~ST_LABEL; };
@@ -340,6 +381,8 @@ public:
    void clear_media() { state &= ~ST_MEDIA; };
    void clear_short_block() { state &= ~ST_SHORT; };
    void clear_freespace_ok() { state &= ~ST_FREESPACE_OK; };
+   void clear_unload() { m_unload = false; };
+   void clear_load() { m_load = false; };
    char *bstrerror(void) { return errmsg; };
    char *print_errmsg() { return errmsg; };
 
@@ -421,12 +464,13 @@ inline const char *DEVICE::print_name() const { return prt_name; }
 class DCR {
 private:
    bool m_dev_locked;                 /* set if dev already locked */
+   bool m_reserved;                   /* set if reserved device */
+   bool m_found_in_use;               /* set if a volume found in use */
 
 public:
    dlink dev_link;                    /* link to attach to dev */
    JCR *jcr;                          /* pointer to JCR */
    DEVICE * volatile dev;             /* pointer to device */
-   DEVICE * volatile swap_dev;        /* Swap vol from this device */
    DEVRES *device;                    /* pointer to device resource */
    DEV_BLOCK *block;                  /* pointer to block */
    DEV_RECORD *rec;                   /* pointer to record */
@@ -439,13 +483,10 @@ public:
    bool NewVol;                       /* set if new Volume mounted */
    bool WroteVol;                     /* set if Volume written */
    bool NewFile;                      /* set when EOF written */
-   bool reserved_device;              /* set if reserve done */
    bool reserved_volume;              /* set if we reserved a volume */
    bool any_volume;                   /* Any OK for dir_find_next... */
    bool attached_to_dev;              /* set when attached to dev */
-   bool volume_in_use;                /* set in dir_find_next_appendable_volume() */
    bool keep_dcr;                     /* do not free dcr in release_dcr */
-   bool unload_device;                /* set if device must be unloaded */
    uint32_t VolFirstIndex;            /* First file index this Volume */
    uint32_t VolLastIndex;             /* Last file index this Volume */
    uint32_t FileIndex;                /* Current File Index */
@@ -466,11 +507,22 @@ public:
    VOLUME_CAT_INFO VolCatInfo;        /* Catalog info for desired volume */
 
    /* Methods */
+   bool found_in_use() const { return m_found_in_use; };
+   void set_found_in_use() { m_found_in_use = true; };
+   void clear_found_in_use() { m_found_in_use = false; };
+   bool is_reserved() const { return m_reserved; };
    bool is_dev_locked() { return m_dev_locked; }
    void dlock() { dev->dlock(); m_dev_locked = true; }
    void dunlock() { m_dev_locked = false; dev->dunlock(); }
    void dblock(int why) { dev->dblock(why); }
 
+
+   /* Methods in reserve.c */
+   void clear_reserved();
+   void set_reserved();
+   void unreserve_device();
+   bool can_i_use_volume();
+
    /* Methods in mount.c */
    bool mount_next_write_volume();
    bool mount_next_read_volume();
@@ -488,7 +540,7 @@ public:
  */
 class VOLRES { 
    bool m_swapping;                   /* set when swapping to another drive */
-   bool m_reserved;                   /* set when volume reserved */
+   bool m_in_use;                     /* set when volume reserved or in use */
 public:
    dlink link;
    char *vol_name;                    /* Volume name */
@@ -497,9 +549,9 @@ public:
    bool is_swapping() const { return m_swapping; };
    void set_swapping() { m_swapping = true; };
    void clear_swapping() { m_swapping = false; };
-   bool is_reserved() const { return m_reserved; };
-   void set_reserved() { m_reserved = true; };
-   void clear_reserved() { m_reserved = false; };
+   bool is_in_use() const { return m_in_use; };
+   void set_in_use() { m_in_use = true; };
+   void clear_in_use() { m_in_use = false; };
 };
 
 
diff --git a/bacula/src/stored/device.c b/bacula/src/stored/device.c
index 0006cdf6d1..b3c4c91fc4 100644
--- a/bacula/src/stored/device.c
+++ b/bacula/src/stored/device.c
@@ -122,8 +122,7 @@ bool fixup_device_block_write_error(DCR *dcr)
         edit_uint64_with_commas(dev->VolCatInfo.VolCatBlocks, b2),
         bstrftime(dt, sizeof(dt), time(NULL)));
 
-   /* Called with have_vol=false, release=true */
-   dcr->unload_device = true;
+   dev->set_unload();
    if (!dcr->mount_next_write_volume()) {
       free_block(label_blk);
       dcr->block = block;
diff --git a/bacula/src/stored/label.c b/bacula/src/stored/label.c
index 9f47dbe88f..d1bcec1103 100644
--- a/bacula/src/stored/label.c
+++ b/bacula/src/stored/label.c
@@ -77,7 +77,7 @@ int read_dev_volume_label(DCR *dcr)
    bool have_ansi_label = false;
 
    Dmsg4(100, "Enter read_volume_label res=%d device=%s vol=%s dev_Vol=%s\n",
-      dev->reserved_device, dev->print_name(), VolName, 
+      dev->num_reserved(), dev->print_name(), VolName, 
       dev->VolHdr.VolumeName[0]?dev->VolHdr.VolumeName:"*NULL*");
 
    if (!dev->is_open()) {
diff --git a/bacula/src/stored/mount.c b/bacula/src/stored/mount.c
index 6055ddb242..17a34d5d78 100644
--- a/bacula/src/stored/mount.c
+++ b/bacula/src/stored/mount.c
@@ -72,7 +72,7 @@ bool DCR::mount_next_write_volume()
    int mode;
    DCR *dcr = this;
 
-   Dmsg2(150, "Enter mount_next_volume(release=%d) dev=%s\n", unload_device,
+   Dmsg2(150, "Enter mount_next_volume(release=%d) dev=%s\n", dev->must_unload(),
       dev->print_name());
 
    init_device_wait_timers(dcr);
@@ -100,27 +100,29 @@ mount_next_vol:
       goto bail_out;
    }
    recycle = false;
-   if (unload_device) {
+   if (dev->must_unload()) {
       Dmsg0(150, "mount_next_volume release=1\n");
       unload_autochanger(dcr, -1);
       release_volume();
-      unload_device = false;
+      dev->clear_unload();
       ask = true;                     /* ask operator to mount tape */
    }
-
    /*
     * See if we are asked to swap the Volume from another device
     *  if so, unload the other device here, and attach the
     *  volume to our drive.
     */
-   if (swap_dev) {
-      Dmsg1(150, "Swap vol=%d\n", swap_dev->vol->vol_name);
-      dev->vol = swap_dev->vol;      /* take its volume */
-      swap_dev->vol = NULL;
-      unload_dev(dcr, swap_dev);
-      swap_dev = NULL;
-      dev->vol->clear_swapping();
-      dev->VolHdr.VolumeName[0] = 0;  /* don't yet have right Volume */
+   if (dev->swap_dev) {
+      Dmsg1(100, "Swap unloading %s\n", dev->swap_dev->print_name());
+      if (dev->swap_dev->must_unload()) {
+         unload_dev(dcr, dev->swap_dev);
+      }
+      if (dev->vol) {
+         dev->vol->clear_swapping();
+         dev->vol->set_in_use();
+         dev->VolHdr.VolumeName[0] = 0;  /* don't yet have right Volume */
+      }
+      dev->swap_dev = NULL;
    }
    if (!is_suitable_volume_mounted()) {
       bool have_vol = false;
@@ -176,7 +178,7 @@ mount_next_vol:
     *   and read the label. If there is no tape in the drive,
     *   we will fail, recurse and ask the operator the next time.
     */
-   if (!unload_device && dev->is_tape() && dev->has_cap(CAP_AUTOMOUNT)) {
+   if (!dev->must_unload() && dev->is_tape() && dev->has_cap(CAP_AUTOMOUNT)) {
       Dmsg0(150, "(1)Ask=0\n");
       ask = false;                 /* don't ask SYSOP this time */
    }
@@ -186,7 +188,7 @@ mount_next_vol:
       ask = false;
    }
    Dmsg2(150, "Ask=%d autochanger=%d\n", ask, autochanger);
-   unload_device = true;     /* release next time if we "recurse" */
+   dev->must_unload();       /* release next time if we "recurse" */
 
    if (ask && !dir_ask_sysop_to_mount_volume(dcr, ST_APPEND)) {
       Dmsg0(150, "Error return ask_sysop ...\n");
@@ -470,7 +472,7 @@ bool DCR::is_suitable_volume_mounted()
 {
 
    /* Volume mounted? */
-   if (dev->VolHdr.VolumeName[0] == 0 || swap_dev || unload_device) {
+   if (dev->VolHdr.VolumeName[0] == 0 || dev->swap_dev || dev->must_unload()) {
       return false;                      /* no */
    }
    bstrncpy(VolumeName, dev->VolHdr.VolumeName, sizeof(VolumeName));
diff --git a/bacula/src/stored/read_record.c b/bacula/src/stored/read_record.c
index 4190d11848..8cd7b4f0b5 100644
--- a/bacula/src/stored/read_record.c
+++ b/bacula/src/stored/read_record.c
@@ -228,11 +228,13 @@ bool read_records(DCR *dcr,
              *  he wants to know if they matched the bsr, then he must
              *  check the match_stat in the record */
             ok = record_cb(dcr, rec);
+#ifdef xxx
             /*
              * If this is the end of the Session (EOS) for this record
              *  we can remove the record.  Note, there is a separate
              *  record to read each session. If a new session is seen
              *  a new record will be created at approx line 157 above.
+             * However, it seg faults in the for line at lineno 196.
              */
             if (rec->FileIndex == EOS_LABEL) {
                Dmsg2(dbglvl, "Remove EOS rec. SI=%d ST=%d\n", rec->VolSessionId,
@@ -240,6 +242,7 @@ bool read_records(DCR *dcr,
                recs->remove(rec);
                free_record(rec);
             }
+#endif
             continue;
          } /* end if label record */
 
diff --git a/bacula/src/stored/reserve.c b/bacula/src/stored/reserve.c
index f2c81f4f31..5a3d2cf47a 100644
--- a/bacula/src/stored/reserve.c
+++ b/bacula/src/stored/reserve.c
@@ -53,7 +53,7 @@ static bool reserve_device_for_append(DCR *dcr, RCTX &rctx);
 static bool use_storage_cmd(JCR *jcr);
 static void queue_reserve_message(JCR *jcr);
 static void pop_reserve_messages(JCR *jcr);
-void switch_device(DCR *dcr, DEVICE *dev);
+//void switch_device(DCR *dcr, DEVICE *dev);
 
 /* Requests from the Director daemon */
 static char use_storage[]  = "use storage=%127s media_type=%127s "
@@ -178,11 +178,11 @@ static void debug_list_volumes(const char *imsg)
    lock_volumes();
    foreach_dlist(vol, vol_list) {
       if (vol->dev) {
-         Mmsg(msg, "List %s: %s reserved=%d on device %s\n", imsg, 
-              vol->vol_name, vol->is_reserved(), vol->dev->print_name());
+         Mmsg(msg, "List %s: %s in_use=%d on device %s\n", imsg, 
+              vol->vol_name, vol->is_in_use(), vol->dev->print_name());
       } else {
-         Mmsg(msg, "List %s: %s reserved=%d no dev\n", imsg, vol->vol_name, 
-              vol->is_reserved());
+         Mmsg(msg, "List %s: %s in_use=%d no dev\n", imsg, vol->vol_name, 
+              vol->is_in_use());
       }
       Dmsg1(dbglvl, "%s", msg.c_str());
    }
@@ -217,13 +217,13 @@ void list_volumes(void sendit(const char *msg, int len, void *sarg), void *arg)
       if (dev) {
          len = Mmsg(msg, "%s on device %s\n", vol->vol_name, dev->print_name());
          sendit(msg.c_str(), len, arg);
-         len = Mmsg(msg, "    Reader=%d writers=%d devres=%d volres=%d\n", 
-            dev->can_read()?1:0, dev->num_writers, dev->reserved_device, 
-            vol->is_reserved());
+         len = Mmsg(msg, "    Reader=%d writers=%d devres=%d volinuse=%d\n", 
+            dev->can_read()?1:0, dev->num_writers, dev->num_reserved(),   
+            vol->is_in_use());
          sendit(msg.c_str(), len, arg);
       } else {
-         len = Mmsg(msg, "%s no device. volres= %d\n", vol->vol_name, 
-            vol->is_reserved());
+         len = Mmsg(msg, "%s no device. volinuse= %d\n", vol->vol_name, 
+            vol->is_in_use());
          sendit(msg.c_str(), len, arg);
       }
    }
@@ -260,7 +260,6 @@ static void free_vol_item(VOLRES *vol)
    }
 }
 
-
 /*
  * Put a new Volume entry in the Volume list. This
  *  effectively reserves the volume so that it will
@@ -277,7 +276,7 @@ static void free_vol_item(VOLRES *vol)
  *
  *  1. The Volume list entry must be attached to the drive (rather than 
  *       attached to a job as it currently is. I.e. the drive that "owns" 
- *       the volume (reserved, in use, mounted)
+ *       the volume (in use, mounted)
  *       must point to the volume (still to be maintained in a list).
  *
  *  2. The Volume is entered in the list when a drive is reserved.  
@@ -337,8 +336,8 @@ VOLRES *reserve_volume(DCR *dcr, const char *VolumeName)
     */
    if (dev->vol) {
       vol = dev->vol;
-      Dmsg4(dbglvl, "Vol attached=%s, newvol=%s volres=%d on %s\n",
-         vol->vol_name, VolumeName, vol->is_reserved(), dev->print_name());
+      Dmsg4(dbglvl, "Vol attached=%s, newvol=%s volinuse=%d on %s\n",
+         vol->vol_name, VolumeName, vol->is_in_use(), dev->print_name());
       /*
        * Make sure we don't remove the current volume we are inserting
        *  because it was probably inserted by another job, or it
@@ -347,19 +346,19 @@ VOLRES *reserve_volume(DCR *dcr, const char *VolumeName)
       if (strcmp(vol->vol_name, VolumeName) == 0) {
          Dmsg2(dbglvl, "=== set reserved vol=%s dev=%s\n", VolumeName,
                vol->dev->print_name());
-         vol->set_reserved();           /* retake vol if released previously */
+         vol->set_in_use();             /* retake vol if released previously */
          dcr->reserved_volume = true;   /* reserved volume */
          goto get_out;                  /* Volume already on this device */
       } else {
          /* Don't release a volume if it was reserved by someone other than us */
-         if (vol->is_reserved() && !dcr->reserved_volume) { 
+         if (vol->is_in_use() && !dcr->reserved_volume) { 
             Dmsg1(dbglvl, "Cannot free vol=%s. It is reserved.\n", vol->vol_name);
             vol = NULL;                  /* vol in use */
             goto get_out;
          }
          Dmsg2(dbglvl, "reserve_vol free vol=%s at %p\n", vol->vol_name, vol->vol_name);
          free_volume(dev);
-         dcr->unload_device = true;     /* have to unload current volume */
+         dev->set_unload();             /* have to unload current volume */
          debug_list_volumes("reserve_vol free");
       }
    }
@@ -391,14 +390,21 @@ VOLRES *reserve_volume(DCR *dcr, const char *VolumeName)
       if (dev != vol->dev) {
          /* Caller wants to switch Volume to another device */
          if (!vol->dev->is_busy() && !vol->is_swapping()) {
+            Dmsg3(dbglvl, "==== Swap vol=%s from dev=%s to %s\n", 
+               VolumeName, vol->dev->print_name(), dev->print_name());
+            free_volume(dev);            /* free any volume attached to our drive */
             vol->set_swapping();
-            dcr->swap_dev = vol->dev;   /* remember to get this vol */
+            vol->dev->set_unload();
+            dev->swap_dev = vol->dev;    /* remember to get this vol */
+            vol->dev->vol = NULL;        /* take volume */
+            vol->dev = dev;
+            dev->vol = vol;
             Dmsg3(dbglvl, "==== Swap vol=%s from dev=%s to %s\n", 
                VolumeName, vol->dev->print_name(), dev->print_name());
          } else {
             Dmsg3(dbglvl, "==== Swap not possible Vol busy vol=%s from dev=%s to %s\n", 
                VolumeName, vol->dev->print_name(), dev->print_name());
-            vol = NULL;                 /* device busy */
+            vol = NULL;                  /* device busy */
             goto get_out;
          }
       } else {
@@ -410,9 +416,9 @@ VOLRES *reserve_volume(DCR *dcr, const char *VolumeName)
 
 get_out:
    if (vol) {
-      Dmsg2(dbglvl, "=== set reserved. vol=%s dev=%s\n", vol->vol_name,
+      Dmsg2(dbglvl, "=== set in_use. vol=%s dev=%s\n", vol->vol_name,
             vol->dev->print_name());
-      vol->set_reserved();
+      vol->set_in_use();
       dcr->reserved_volume = true;
    }
    debug_list_volumes("end new volume");
@@ -424,6 +430,7 @@ get_out:
  * Switch from current device to given device  
  *   (not yet used) 
  */
+#ifdef xxx
 void switch_device(DCR *dcr, DEVICE *dev)
 {
    DCR save_dcr;
@@ -443,11 +450,11 @@ void switch_device(DCR *dcr, DEVICE *dev)
    bstrncpy(dcr->pool_type, save_dcr.pool_type, sizeof(dcr->pool_type));
    bstrncpy(dcr->dev_name, dev->dev_name, sizeof(dcr->dev_name));
 
-   dev->reserved_device++;
-   dcr->reserved_device = true;
+// dcr->set_reserved();
 
    dev->dunlock();
 }
+#endif
 
 /*
  * Search for a Volume name in the Volume list.
@@ -455,43 +462,56 @@ void switch_device(DCR *dcr, DEVICE *dev)
  *  Returns: VOLRES entry on success
  *           NULL if the Volume is not in the list
  */
-VOLRES *find_volume(DCR *dcr)
+VOLRES *find_volume(const char *VolumeName) 
 {
    VOLRES vol, *fvol;
    /* Do not lock reservations here */
    lock_volumes();
-   vol.vol_name = bstrdup(dcr->VolumeName);
+   vol.vol_name = bstrdup(VolumeName);
    fvol = (VOLRES *)vol_list->binary_search(&vol, my_compare);
    free(vol.vol_name);
-   Dmsg2(dbglvl, "find_vol=%s found=%d\n", dcr->VolumeName, fvol!=NULL);
+   Dmsg2(dbglvl, "find_vol=%s found=%d\n", VolumeName, fvol!=NULL);
    debug_list_volumes("find_volume");
    unlock_volumes();
    return fvol;
 }
 
+void DCR::set_reserved()
+{
+   m_reserved = true;
+   Dmsg2(dbglvl, "Inc reserve=%d dev=%s\n", dev->num_reserved(), dev->print_name());
+   dev->inc_reserved();
+}
+
+void DCR::clear_reserved()
+{
+   if (m_reserved) {
+      m_reserved = false;
+      dev->dec_reserved();
+      Dmsg2(dbglvl, "Dec reserve=%d dev=%s\n", dev->num_reserved(), dev->print_name());
+   }
+}
+
 /* 
  * Remove any reservation from a drive and tell the system
  *  that the volume is unused at least by us.
  */
-void unreserve_device(DCR *dcr)
+void DCR::unreserve_device()
 {
-   DEVICE *dev = dcr->dev;
    lock_volumes();
-   if (dcr->reserved_device) {
-      dcr->reserved_device = false;
-      dcr->reserved_volume = false;
-      dev->reserved_device--;
-      Dmsg2(dbglvl, "Dec reserve=%d dev=%s\n", dev->reserved_device, dev->print_name());
+   if (is_reserved()) {
+      clear_reserved();
+      reserved_volume = false;
       /* If we set read mode in reserving, remove it */
       if (dev->can_read()) {
          dev->clear_read();
       }
       if (dev->num_writers < 0) {
-         Jmsg1(dcr->jcr, M_ERROR, 0, _("Hey! num_writers=%d!!!!\n"), dev->num_writers);
+         Jmsg1(jcr, M_ERROR, 0, _("Hey! num_writers=%d!!!!\n"), dev->num_writers);
          dev->num_writers = 0;
       }
-      if (dev->reserved_device == 0 && dev->num_writers == 0) {
-         volume_unused(dcr);
+      if (dev->num_reserved() == 0 && dev->num_writers == 0) {
+         volume_unused(this);
       }
    }
    unlock_volumes();
@@ -530,7 +550,7 @@ bool volume_unused(DCR *dcr)
    }
 #endif
 #ifdef xxx
-   if (dev->num_writers > 0 || dev->reserved_device > 0) {
+   if (dev->num_writers > 0 || dev->num_reserved() > 0) {
       ASSERT(0);
    }
 #endif
@@ -542,8 +562,8 @@ bool volume_unused(DCR *dcr)
     *  where the tapes are or last were.
     */
    Dmsg3(dbglvl, "=== mark not reserved vol=%s num_writers=%d dev_reserved=%d\n",
-      dev->vol->vol_name, dev->num_writers, dev->reserved_device);
-   dev->vol->clear_reserved();
+      dev->vol->vol_name, dev->num_writers, dev->num_reserved());
+   dev->vol->clear_in_use();
    Dmsg2(dbglvl, "=== set not reserved. Vol=%s dev=%s\n", dev->vol->vol_name,
          dev->print_name());
    if (dev->is_tape() || dev->is_autochanger()) {
@@ -611,34 +631,35 @@ void free_volume_list()
    unlock_volumes();
 }
 
-bool is_volume_in_use(DCR *dcr)
+bool DCR::can_i_use_volume()
 {
-   bool rtn = false;
+   bool rtn = true;
    VOLRES *vol;
 
    lock_volumes();
-   vol = find_volume(dcr);
+   vol = find_volume(VolumeName);
    if (!vol) {
-      Dmsg1(dbglvl, "Vol=%s not in use.\n", dcr->VolumeName);
+      Dmsg1(dbglvl, "Vol=%s not in use.\n", VolumeName);
       goto get_out;                   /* vol not in list */
    }
    ASSERT(vol->dev != NULL);
 
-   if (dcr->dev == vol->dev) {        /* same device OK */
-      Dmsg1(dbglvl, "Vol=%s on same dev.\n", dcr->VolumeName);
+   if (dev == vol->dev) {        /* same device OK */
+      Dmsg1(dbglvl, "Vol=%s on same dev.\n", VolumeName);
       goto get_out;
    } else {
-      Dmsg3(dbglvl, "Vol=%s on %s we have %s\n", dcr->VolumeName,
-            vol->dev->print_name(), dcr->dev->print_name());
+      Dmsg3(dbglvl, "Vol=%s on %s we have %s\n", VolumeName,
+            vol->dev->print_name(), dev->print_name());
    }
+   /* ***FIXME*** check this ... */
    if (!vol->dev->is_busy()) {
-      Dmsg2(dbglvl, "Vol=%s dev=%s not busy.\n", dcr->VolumeName, vol->dev->print_name());
+      Dmsg2(dbglvl, "Vol=%s dev=%s not busy.\n", VolumeName, vol->dev->print_name());
       goto get_out;
    } else {
-      Dmsg2(dbglvl, "Vol=%s dev=%s busy.\n", dcr->VolumeName, vol->dev->print_name());
+      Dmsg2(dbglvl, "Vol=%s dev=%s busy.\n", VolumeName, vol->dev->print_name());
    }
-   Dmsg2(dbglvl, "Vol=%s in use by %s.\n", dcr->VolumeName, vol->dev->print_name());
-   rtn = true;
+   Dmsg2(dbglvl, "Vol=%s in use by %s.\n", VolumeName, vol->dev->print_name());
+   rtn = false;
 
 get_out:
    unlock_volumes();
@@ -1090,10 +1111,10 @@ int search_res_for_device(RCTX &rctx)
             /* Debug code */
             if (rctx.store->append == SD_APPEND) {
                Dmsg2(dbglvl, "Device %s reserved=%d for append.\n", 
-                  rctx.device->hdr.name, rctx.jcr->dcr->dev->reserved_device);
+                  rctx.device->hdr.name, rctx.jcr->dcr->dev->num_reserved());
             } else {
                Dmsg2(dbglvl, "Device %s reserved=%d for read.\n", 
-                  rctx.device->hdr.name, rctx.jcr->read_dcr->dev->reserved_device);
+                  rctx.device->hdr.name, rctx.jcr->read_dcr->dev->num_reserved());
             }
             return stat;
          }
@@ -1113,10 +1134,10 @@ int search_res_for_device(RCTX &rctx)
             /* Debug code */
             if (rctx.store->append == SD_APPEND) {
                Dmsg2(dbglvl, "Device %s reserved=%d for append.\n", 
-                  rctx.device->hdr.name, rctx.jcr->dcr->dev->reserved_device);
+                  rctx.device->hdr.name, rctx.jcr->dcr->dev->num_reserved());
             } else {
                Dmsg2(dbglvl, "Device %s reserved=%d for read.\n", 
-                  rctx.device->hdr.name, rctx.jcr->read_dcr->dev->reserved_device);
+                  rctx.device->hdr.name, rctx.jcr->read_dcr->dev->num_reserved());
             }
             return stat;
          }
@@ -1184,7 +1205,7 @@ static int reserve_device(RCTX &rctx)
 
       rctx.jcr->dcr = dcr;
       Dmsg5(dbglvl, "Reserved=%d dev_name=%s mediatype=%s pool=%s ok=%d\n",
-               dcr->dev->reserved_device,
+               dcr->dev->num_reserved(),
                dcr->dev_name, dcr->media_type, dcr->pool_name, ok);
       Dmsg3(dbglvl, "Vol=%s num_writers=%d, have_vol=%d\n", 
          rctx.VolumeName, dcr->dev->num_writers, rctx.have_volume);
@@ -1214,10 +1235,10 @@ static int reserve_device(RCTX &rctx)
              *   non-used drive and our one and only volume is mounted
              *   elsewhere, so we bail out and retry using that drive.
              */
-            if (dcr->volume_in_use && !rctx.PreferMountedVols) {
+            if (dcr->found_in_use() && !rctx.PreferMountedVols) {
                rctx.PreferMountedVols = true;
                if (dcr->VolumeName[0]) {
-                  unreserve_device(dcr);
+                  dcr->unreserve_device();
                }
                goto bail_out;
             }
@@ -1234,7 +1255,7 @@ static int reserve_device(RCTX &rctx)
              */
             if (dcr->dev->num_writers != 0) {
                if (dcr->VolumeName[0]) {
-                  unreserve_device(dcr);
+                  dcr->unreserve_device();
                }
                goto bail_out;
             }
@@ -1245,7 +1266,7 @@ static int reserve_device(RCTX &rctx)
       if (ok) {
          rctx.jcr->read_dcr = dcr;
          Dmsg5(dbglvl, "Read reserved=%d dev_name=%s mediatype=%s pool=%s ok=%d\n",
-               dcr->dev->reserved_device,
+               dcr->dev->num_reserved(),
                dcr->dev_name, dcr->media_type, dcr->pool_name, ok);
       }
    }
@@ -1299,7 +1320,7 @@ static bool reserve_device_for_read(DCR *dcr)
    if (dev->is_busy()) {
       Dmsg4(dbglvl, "Device %s is busy ST_READ=%d num_writers=%d reserved=%d.\n", 
          dev->print_name(),
-         dev->state & ST_READ?1:0, dev->num_writers, dev->reserved_device);
+         dev->state & ST_READ?1:0, dev->num_writers, dev->num_reserved());
       Mmsg(jcr->errmsg, _("3602 JobId=%u device %s is busy (already reading/writing).\n"),
             jcr->JobId, dev->print_name());
       queue_reserve_message(jcr);
@@ -1309,9 +1330,7 @@ static bool reserve_device_for_read(DCR *dcr)
    dev->clear_append();
    dev->set_read();
    ok = true;
-   dev->reserved_device++;
-   Dmsg3(dbglvl, "Inc reserve=%d dev=%s %p\n", dev->reserved_device, dev->print_name(), dev);
-   dcr->reserved_device = true;
+   dcr->set_reserved();
 
 bail_out:
    dev->dunlock();
@@ -1370,10 +1389,7 @@ static bool reserve_device_for_append(DCR *dcr, RCTX &rctx)
       goto bail_out;
    }
 
-   dev->reserved_device++;
-   Dmsg3(dbglvl, "Inc reserve=%d dev=%s %p\n", dev->reserved_device, 
-      dev->print_name(), dev);
-   dcr->reserved_device = true;
+   dcr->set_reserved();
    ok = true;
 
 bail_out:
@@ -1397,7 +1413,7 @@ static int is_pool_ok(DCR *dcr)
       Mmsg(jcr->errmsg, _(
 "3608 JobId=%u wants Pool=\"%s\" but have Pool=\"%s\" nreserve=%d on drive %s.\n"), 
             (uint32_t)jcr->JobId, dcr->pool_name, dev->pool_name,
-            dev->reserved_device, dev->print_name());
+            dev->num_reserved(), dev->print_name());
       queue_reserve_message(jcr);
       Dmsg2(dbglvl, "failed: busy num_writers=0, reserved, pool=%s wanted=%s\n",
          dev->pool_name, dcr->pool_name);
@@ -1412,10 +1428,10 @@ static bool is_max_jobs_ok(DCR *dcr)
 
    Dmsg4(dbglvl, "MaxJobs=%d Jobs=%d reserves=%d Vol=%s\n",
          dcr->VolCatInfo.VolCatMaxJobs,
-         dcr->VolCatInfo.VolCatJobs, dev->reserved_device,
+         dcr->VolCatInfo.VolCatJobs, dev->num_reserved(),
          dcr->VolumeName);
    if (dcr->VolCatInfo.VolCatMaxJobs > 0 && dcr->VolCatInfo.VolCatMaxJobs <=
-        (dcr->VolCatInfo.VolCatJobs + dev->reserved_device)) {
+        (dcr->VolCatInfo.VolCatJobs + dev->num_reserved())) {
       /* Max Job Vols depassed or already reserved */
       Mmsg(jcr->errmsg, _("3610 JobId=%u Volume max jobs exceeded on drive %s.\n"), 
             (uint32_t)jcr->JobId, dev->print_name());
@@ -1461,13 +1477,13 @@ static int can_reserve_drive(DCR *dcr, RCTX &rctx)
       /* If he wants a free drive, but this one is busy, no go */
       if (!rctx.PreferMountedVols && dev->is_busy()) {
          /* Save least used drive */
-         if ((dev->num_writers + dev->reserved_device) < rctx.num_writers) {
-            rctx.num_writers = dev->num_writers + dev->reserved_device;
+         if ((dev->num_writers + dev->num_reserved()) < rctx.num_writers) {
+            rctx.num_writers = dev->num_writers + dev->num_reserved();
             rctx.low_use_drive = dev;
             Dmsg2(dbglvl, "set low use drive=%s num_writers=%d\n", 
                dev->print_name(), rctx.num_writers);
          } else {
-            Dmsg1(dbglvl, "not low use num_writers=%d\n", dev->num_writers+dev->reserved_device);
+            Dmsg1(dbglvl, "not low use num_writers=%d\n", dev->num_writers+dev->num_reserved());
          }
          Dmsg0(dbglvl, "failed: !prefMnt && busy.\n");
          Mmsg(jcr->errmsg, _("3605 JobId=%u wants free drive but device %s is busy.\n"), 
@@ -1508,7 +1524,7 @@ static int can_reserve_drive(DCR *dcr, RCTX &rctx)
                   dev->VolHdr.VolumeName, dev->vol?dev->vol->vol_name:"*none*", rctx.VolumeName);
             return 0;
          }
-         if (is_volume_in_use(dcr)) {
+         if (!dcr->can_i_use_volume()) {
             return 0;              /* fail if volume on another drive */
          }
       }
@@ -1529,7 +1545,7 @@ static int can_reserve_drive(DCR *dcr, RCTX &rctx)
     */
    if (dev->num_writers == 0) {
       /* Now check if there are any reservations on the drive */
-      if (dev->reserved_device) {           
+      if (dev->num_reserved()) {           
          return is_pool_ok(dcr);
       } else if (dev->can_append()) {
          if (is_pool_ok(dcr)) {
@@ -1537,6 +1553,7 @@ static int can_reserve_drive(DCR *dcr, RCTX &rctx)
          } else {
             /* Changing pool, unload old tape if any in drive */
             Dmsg0(dbglvl, "OK dev: num_writers=0, not reserved, pool change, unload changer\n");
+            /* ***FIXME*** use set_unload() */
             unload_autochanger(dcr, 0);
          }
       }
diff --git a/bacula/src/stored/status.c b/bacula/src/stored/status.c
index deea17e914..f91444e4e9 100644
--- a/bacula/src/stored/status.c
+++ b/bacula/src/stored/status.c
@@ -367,7 +367,8 @@ static void send_device_status(DEVICE *dev, STATUS_PKT *sp)
       dev->state & ST_MOUNTED ? "" : "!");
    sendit(msg, len, sp);
 
-   len = Mmsg(msg, _("num_writers=%d block=%d\n\n"), dev->num_writers, dev->blocked());
+   len = Mmsg(msg, _("num_writers=%d reserved=%d block=%d\n\n"), dev->num_writers, 
+              dev->num_reserved(), dev->blocked());
    sendit(msg, len, sp);
 
    len = Mmsg(msg, _("Device parameters:\n"));
diff --git a/bacula/technotes-2.3 b/bacula/technotes-2.3
index e19ebe7860..cfa908b0d6 100644
--- a/bacula/technotes-2.3
+++ b/bacula/technotes-2.3
@@ -24,6 +24,10 @@ Add long term statistics job table
 
 
 General:
+12Apr08
+kes  Rework class structures for VOLRES, DCR, and DEVICE to make
+     the method names a bit more logical, and for more logically
+     handling the responsibilities.
 11Apr08
 kes  Remove redundant code in terminating the scheduler that just
      causes a seg fault in many cases.