stored: remove Device::IsFile()

This patch refactors all uses of IsFile() into individual checks that
should do what the original author intended.

14 files changed, 125 insertions, 95 deletions

diff --git a/core/src/stored/backends/droplet_device.h b/core/src/stored/backends/droplet_device.h
index 3c63dfed2..1eaae6506 100644
--- a/core/src/stored/backends/droplet_device.h
+++ b/core/src/stored/backends/droplet_device.h
@@ -2,7 +2,7 @@
    BAREOS® - Backup Archiving REcovery Open Sourced
 
    Copyright (C) 2014-2017 Planets Communications B.V.
-   Copyright (C) 2014-2021 Bareos GmbH & Co. KG
+   Copyright (C) 2014-2022 Bareos GmbH & Co. KG
 
    This program is Free Software; you can redistribute it and/or
    modify it under the terms of version three of the GNU Affero General Public
@@ -80,6 +80,8 @@ class DropletDevice : public ChunkedDevice {
   ~DropletDevice();
 
   // Interface from Device
+  SeekMode GetSeekMode() const override { return SeekMode::BYTES; }
+  bool CanReadConcurrently() const override { return true; }
   int d_close(int fd) override;
   int d_open(const char* pathname, int flags, int mode) override;
   int d_ioctl(int fd, ioctl_req_t request, char* mt = NULL) override;
diff --git a/core/src/stored/backends/generic_tape_device.h b/core/src/stored/backends/generic_tape_device.h
index 65c2a5b80..c6924e25b 100644
--- a/core/src/stored/backends/generic_tape_device.h
+++ b/core/src/stored/backends/generic_tape_device.h
@@ -2,7 +2,7 @@
    BAREOS® - Backup Archiving REcovery Open Sourced
 
    Copyright (C) 2014-2014 Planets Communications B.V.
-   Copyright (C) 2014-2021 Bareos GmbH & Co. KG
+   Copyright (C) 2014-2022 Bareos GmbH & Co. KG
 
    This program is Free Software; you can redistribute it and/or
    modify it under the terms of version three of the GNU Affero General Public
@@ -38,6 +38,7 @@ class generic_tape_device : public Device {
   virtual ~generic_tape_device() { close(nullptr); }
 
   // Interface from Device
+  virtual SeekMode GetSeekMode() const override { return SeekMode::FILE_BLOCK; }
   virtual void OpenDevice(DeviceControlRecord* dcr, DeviceMode omode) override;
   virtual char* StatusDev() override;
   virtual bool eod(DeviceControlRecord* dcr) override;
diff --git a/core/src/stored/backends/gfapi_device.h b/core/src/stored/backends/gfapi_device.h
index 7164260c5..049cfaeb8 100644
--- a/core/src/stored/backends/gfapi_device.h
+++ b/core/src/stored/backends/gfapi_device.h
@@ -56,6 +56,8 @@ class gfapi_device : public Device {
   ~gfapi_device();
 
   // Interface from Device
+  SeekMode GetSeekMode() const override { return SeekMode::BYTES; }
+  bool CanReadConcurrently() const override { return true; }
   int d_close(int) override;
   int d_open(const char* pathname, int flags, int mode) override;
   int d_ioctl(int fd, ioctl_req_t request, char* mt = NULL) override;
diff --git a/core/src/stored/backends/unix_fifo_device.h b/core/src/stored/backends/unix_fifo_device.h
index 6e6d31bbf..078795db7 100644
--- a/core/src/stored/backends/unix_fifo_device.h
+++ b/core/src/stored/backends/unix_fifo_device.h
@@ -2,7 +2,7 @@
    BAREOS® - Backup Archiving REcovery Open Sourced
 
    Copyright (C) 2013-2013 Planets Communications B.V.
-   Copyright (C) 2013-2021 Bareos GmbH & Co. KG
+   Copyright (C) 2013-2022 Bareos GmbH & Co. KG
 
    This program is Free Software; you can redistribute it and/or
    modify it under the terms of version three of the GNU Affero General Public
@@ -40,6 +40,7 @@ class unix_fifo_device : public Device {
   ~unix_fifo_device() { close(nullptr); }
 
   // Interface from Device
+  SeekMode GetSeekMode() const override { return SeekMode::NOSEEK; }
   void OpenDevice(DeviceControlRecord* dcr, DeviceMode omode) override;
   bool eod(DeviceControlRecord* dcr) override;
   bool MountBackend(DeviceControlRecord* dcr, int timeout) override;
diff --git a/core/src/stored/backends/unix_file_device.cc b/core/src/stored/backends/unix_file_device.cc
index f866c203e..2b3150dd1 100644
--- a/core/src/stored/backends/unix_file_device.cc
+++ b/core/src/stored/backends/unix_file_device.cc
@@ -207,6 +207,11 @@ bool unix_file_device::UnmountBackend(DeviceControlRecord* dcr, int timeout)
   return retval;
 }
 
+bool unix_file_device::ScanForVolumeImpl(DeviceControlRecord* dcr)
+{
+  return ScanDirectoryForVolume(dcr);
+}
+
 int unix_file_device::d_open(const char* pathname, int flags, int mode)
 {
   return ::open(pathname, flags, mode);
diff --git a/core/src/stored/backends/unix_file_device.h b/core/src/stored/backends/unix_file_device.h
index 3c8ec0a73..9b4847c5f 100644
--- a/core/src/stored/backends/unix_file_device.h
+++ b/core/src/stored/backends/unix_file_device.h
@@ -38,8 +38,11 @@ class unix_file_device : public Device {
   ~unix_file_device() { close(nullptr); }
 
   // Interface from Device
+  SeekMode GetSeekMode() const override { return SeekMode::BYTES; }
+  bool CanReadConcurrently() const override { return true; }
   bool MountBackend(DeviceControlRecord* dcr, int timeout) override;
   bool UnmountBackend(DeviceControlRecord* dcr, int timeout) override;
+  bool ScanForVolumeImpl(DeviceControlRecord* dcr) override;
   int d_close(int) override;
   int d_open(const char* pathname, int flags, int mode) override;
   int d_ioctl(int fd, ioctl_req_t request, char* mt = NULL) override;
diff --git a/core/src/stored/backends/win32_fifo_device.h b/core/src/stored/backends/win32_fifo_device.h
index a1f7716ff..080e0489f 100644
--- a/core/src/stored/backends/win32_fifo_device.h
+++ b/core/src/stored/backends/win32_fifo_device.h
@@ -42,6 +42,7 @@ class win32_fifo_device : public Device {
   ~win32_fifo_device() { close(nullptr); }
 
   // Interface from Device
+  SeekMode GetSeekMode() const override { return SeekMode::NOSEEK; }
   void OpenDevice(DeviceControlRecord* dcr, DeviceMode omode) override;
   bool eod(DeviceControlRecord* dcr) override;
   bool MountBackend(DeviceControlRecord* dcr, int timeout) override;
diff --git a/core/src/stored/backends/win32_file_device.cc b/core/src/stored/backends/win32_file_device.cc
index 07510845f..e75fbf880 100644
--- a/core/src/stored/backends/win32_file_device.cc
+++ b/core/src/stored/backends/win32_file_device.cc
@@ -196,6 +196,11 @@ bool win32_file_device::UnmountBackend(DeviceControlRecord* dcr, int timeout)
   return retval;
 }
 
+bool win32_file_device::ScanForVolumeImpl(DeviceControlRecord* dcr)
+{
+  return ScanDirectoryForVolume(dcr);
+}
+
 int win32_file_device::d_open(const char* pathname, int flags, int mode)
 {
   return ::open(pathname, flags, mode);
diff --git a/core/src/stored/backends/win32_file_device.h b/core/src/stored/backends/win32_file_device.h
index 31a87fa98..ba2655438 100644
--- a/core/src/stored/backends/win32_file_device.h
+++ b/core/src/stored/backends/win32_file_device.h
@@ -37,8 +37,11 @@ class win32_file_device : public Device {
   ~win32_file_device() { close(nullptr); }
 
   // Interface from Device
+  SeekMode GetSeekMode() const override { return SeekMode::BYTES; }
+  bool CanReadConcurrently() const override { return true; }
   bool MountBackend(DeviceControlRecord* dcr, int timeout) override;
   bool UnmountBackend(DeviceControlRecord* dcr, int timeout) override;
+  bool ScanForVolumeImpl(DeviceControlRecord* dcr) override;
   int d_close(int) override;
   int d_open(const char* pathname, int flags, int mode) override;
   int d_ioctl(int fd, ioctl_req_t request, char* mt = NULL) override;
diff --git a/core/src/stored/dev.cc b/core/src/stored/dev.cc
index bfae1a23c..a545b5f94 100644
--- a/core/src/stored/dev.cc
+++ b/core/src/stored/dev.cc
@@ -191,7 +191,7 @@ static void InitiateDevice(JobControlRecord* jcr, Device* dev)
    * - Check that the mount point is available
    * - Check that (un)mount commands are defined
    */
-  if (dev->IsFile() && dev->RequiresMount()) {
+  if (dev->RequiresMount()) {
     struct stat statp;
     if (!dev->device_resource->mount_point
         || stat(dev->device_resource->mount_point, &statp) < 0) {
diff --git a/core/src/stored/dev.h b/core/src/stored/dev.h
index 769420258..4a47788e6 100644
--- a/core/src/stored/dev.h
+++ b/core/src/stored/dev.h
@@ -170,6 +170,13 @@ enum
 // Make sure you have enough bits to store all above bit fields.
 #define ST_BYTES NbytesForBits(ST_MAX + 1)
 
+enum class SeekMode
+{
+  NOSEEK,      // device cannot be seeked (e.g. a FIFO)
+  FILE_BLOCK,  // device works with file and blocknum (like a tape)
+  BYTES        // device uses byte offsets (like a plain file)
+};
+
 struct DeviceType {
   static constexpr std::string_view B_DROPLET_DEV = "droplet";
   static constexpr std::string_view B_FIFO_DEV = "fifo";
@@ -292,17 +299,12 @@ class Device {
   bool RequiresMount() const { return BitIsSet(CAP_REQMOUNT, capabilities); }
   bool IsRemovable() const { return BitIsSet(CAP_REM, capabilities); }
   bool IsTape() const { return (dev_type == DeviceType::B_TAPE_DEV); }
-  bool IsFile() const
-  {
-    return (dev_type == DeviceType::B_FILE_DEV || dev_type == DeviceType::B_GFAPI_DEV ||
-            dev_type == DeviceType::B_DROPLET_DEV);
-  }
   bool IsFifo() const { return dev_type == DeviceType::B_FIFO_DEV; }
   bool IsOpen() const { return fd >= 0; }
   bool IsOffline() const { return BitIsSet(ST_OFFLINE, state); }
   bool IsLabeled() const { return BitIsSet(ST_LABEL, state); }
   bool IsMounted() const { return BitIsSet(ST_MOUNTED, state); }
-  bool IsUnmountable() const { return ((IsFile() && IsRemovable())); }
+  bool IsUnmountable() const { return (IsRemovable() && RequiresMount()); }
   int NumReserved() const { return num_reserved_; }
   bool is_part_spooled() const { return BitIsSet(ST_PART_SPOOLED, state); }
   bool have_media() const { return BitIsSet(ST_MEDIA, state); }
@@ -403,7 +405,11 @@ class Device {
   bool unmount(DeviceControlRecord* dcr, int timeout);
   void EditMountCodes(PoolMem& omsg, const char* imsg);
   bool OfflineOrRewind();
-  bool ScanDirForVolume(DeviceControlRecord* dcr);
+protected:
+  bool ScanDirectoryForVolume(DeviceControlRecord* dcr);
+  virtual bool ScanForVolumeImpl(DeviceControlRecord* dcr);
+public:
+  bool ScanForVolume(DeviceControlRecord* dcr);
   void SetSlotNumber(slot_number_t slot);
   void InvalidateSlotNumber();
 
@@ -447,6 +453,8 @@ class Device {
     return true;
   }
   virtual bool DeviceStatus(DeviceStatusInformation*) { return false; }
+  virtual SeekMode GetSeekMode() const = 0;
+  virtual bool CanReadConcurrently() const { return false; }
 
   // Low level operations
   virtual int d_ioctl(int fd, ioctl_req_t request, char* mt_com = NULL) = 0;
@@ -493,14 +501,13 @@ class SpoolDevice :public Device
  public:
   SpoolDevice() = default;
   ~SpoolDevice() {   close(nullptr); }
+  SeekMode GetSeekMode() const override { return SeekMode::NOSEEK; }
   int d_ioctl(int, ioctl_req_t, char*) override {return -1;}
   int d_open(const char*, int, int) override {return -1;}
   int d_close(int) override {return -1;}
   ssize_t d_read(int, void*, size_t) override { return 0;}
   ssize_t d_write(int, const void*, size_t) override { return 0;}
-  boffset_t d_lseek(DeviceControlRecord*,
-                    boffset_t,
-                    int) override { return 0;}
+  boffset_t d_lseek(DeviceControlRecord*, boffset_t, int) override { return 0;}
   bool d_truncate(DeviceControlRecord*) override {return false;}
 };
 /* clang-format on */
diff --git a/core/src/stored/mount.cc b/core/src/stored/mount.cc
index 33ab5e107..5f2891d06 100644
--- a/core/src/stored/mount.cc
+++ b/core/src/stored/mount.cc
@@ -244,11 +244,10 @@ mount_next_vol:
 
   while (!dev->open(dcr, mode)) {
     Dmsg1(150, "OpenDevice failed: ERR=%s\n", dev->bstrerror());
-    if (dev->IsFile() && dev->IsRemovable()) {
-      bool ok = true;
-      Dmsg0(150, "call scan_dir_for_vol\n");
-      if (ok && dev->ScanDirForVolume(dcr)) {
-        if (dev->open(dcr, mode)) { break; /* got a valid volume */ }
+    if (dev->IsRemovable()) {
+      Dmsg0(150, "call ScanForVolume\n");
+      if (dev->ScanForVolume(dcr) && dev->open(dcr, mode)) {
+        break; /* got a valid volume */
       }
     }
     if (TryAutolabel(false) == try_read_vol) {
@@ -654,85 +653,78 @@ void DeviceControlRecord::DoSwapping(bool)
  */
 bool DeviceControlRecord::is_eod_valid()
 {
-  DeviceControlRecord* dcr = this;
-
-  if (dev->IsTape()) {
-    /*
-     * Check if we are positioned on the tape at the same place
-     * that the database says we should be.
-     */
-    if (dev->VolCatInfo.VolCatFiles == dev->GetFile()) {
-      Jmsg(jcr, M_INFO, 0,
-           _("Ready to append to end of Volume \"%s\" at file=%d.\n"),
-           VolumeName, dev->GetFile());
-    } else if (dev->GetFile() > dev->VolCatInfo.VolCatFiles) {
-      Jmsg(jcr, M_WARNING, 0,
-           _("For Volume \"%s\":\n"
-             "The number of files mismatch! Volume=%u Catalog=%u\n"
-             "Correcting Catalog\n"),
-           VolumeName, dev->GetFile(), dev->VolCatInfo.VolCatFiles);
-      dev->VolCatInfo.VolCatFiles = dev->GetFile();
-      dev->VolCatInfo.VolCatBlocks = dev->GetBlockNum();
-      if (!dcr->DirUpdateVolumeInfo(false, true)) {
-        Jmsg(jcr, M_WARNING, 0, _("Error updating Catalog\n"));
+  switch (dev->GetSeekMode()) {
+    case SeekMode::FILE_BLOCK: {
+      /*
+       * Check if we are positioned on the tape at the same place
+       * that the database says we should be.
+       */
+      if (dev->VolCatInfo.VolCatFiles == dev->GetFile()) {
+        Jmsg(jcr, M_INFO, 0,
+             _("Ready to append to end of Volume \"%s\" at file=%d.\n"),
+             VolumeName, dev->GetFile());
+      } else if (dev->GetFile() > dev->VolCatInfo.VolCatFiles) {
+        Jmsg(jcr, M_WARNING, 0,
+             _("For Volume \"%s\":\n"
+               "The number of files mismatch! Volume=%u Catalog=%u\n"
+               "Correcting Catalog\n"),
+             VolumeName, dev->GetFile(), dev->VolCatInfo.VolCatFiles);
+        dev->VolCatInfo.VolCatFiles = dev->GetFile();
+        dev->VolCatInfo.VolCatBlocks = dev->GetBlockNum();
+        if (!DirUpdateVolumeInfo(false, true)) {
+          Jmsg(jcr, M_WARNING, 0, _("Error updating Catalog\n"));
+          MarkVolumeInError();
+          return false;
+        }
+      } else {
+        Jmsg(jcr, M_ERROR, 0,
+             _("Bareos cannot write on tape Volume \"%s\" because:\n"
+               "The number of files mismatch! Volume=%u Catalog=%u\n"),
+             VolumeName, dev->GetFile(), dev->VolCatInfo.VolCatFiles);
         MarkVolumeInError();
         return false;
       }
-    } else {
-      Jmsg(jcr, M_ERROR, 0,
-           _("Bareos cannot write on tape Volume \"%s\" because:\n"
-             "The number of files mismatch! Volume=%u Catalog=%u\n"),
-           VolumeName, dev->GetFile(), dev->VolCatInfo.VolCatFiles);
-      MarkVolumeInError();
-      return false;
-    }
-  } else if (dev->IsFile()) {
-    char ed1[50], ed2[50];
-
-    boffset_t pos;
-    pos = dev->d_lseek(dcr, (boffset_t)0, SEEK_CUR);
-    if (dev->VolCatInfo.VolCatBytes == (uint64_t)pos) {
-      Jmsg(jcr, M_INFO, 0,
-           _("Ready to append to end of Volume \"%s\""
-             " size=%s\n"),
-           VolumeName, edit_uint64(dev->VolCatInfo.VolCatBytes, ed1));
-    } else if ((uint64_t)pos > dev->VolCatInfo.VolCatBytes) {
-      Jmsg(jcr, M_WARNING, 0,
-           _("For Volume \"%s\":\n"
-             "The sizes do not match! Volume=%s Catalog=%s\n"
-             "Correcting Catalog\n"),
-           VolumeName, edit_uint64(pos, ed1),
-           edit_uint64(dev->VolCatInfo.VolCatBytes, ed2));
-      dev->VolCatInfo.VolCatBytes = (uint64_t)pos;
-      dev->VolCatInfo.VolCatFiles = (uint32_t)(pos >> 32);
-      if (!dcr->DirUpdateVolumeInfo(false, true)) {
-        Jmsg(jcr, M_WARNING, 0, _("Error updating Catalog\n"));
+    } break;
+    case SeekMode::BYTES: {
+      char ed1[50], ed2[50];
+
+      boffset_t pos;
+      pos = dev->d_lseek(this, (boffset_t)0, SEEK_CUR);
+      if (dev->VolCatInfo.VolCatBytes == (uint64_t)pos) {
+        Jmsg(jcr, M_INFO, 0,
+             _("Ready to append to end of Volume \"%s\""
+               " size=%s\n"),
+             VolumeName, edit_uint64(dev->VolCatInfo.VolCatBytes, ed1));
+      } else if ((uint64_t)pos > dev->VolCatInfo.VolCatBytes) {
+        Jmsg(jcr, M_WARNING, 0,
+             _("For Volume \"%s\":\n"
+               "The sizes do not match! Volume=%s Catalog=%s\n"
+               "Correcting Catalog\n"),
+             VolumeName, edit_uint64(pos, ed1),
+             edit_uint64(dev->VolCatInfo.VolCatBytes, ed2));
+        dev->VolCatInfo.VolCatBytes = (uint64_t)pos;
+        dev->VolCatInfo.VolCatFiles = (uint32_t)(pos >> 32);
+        if (!DirUpdateVolumeInfo(false, true)) {
+          Jmsg(jcr, M_WARNING, 0, _("Error updating Catalog\n"));
+          MarkVolumeInError();
+          return false;
+        }
+      } else {
+        Mmsg(jcr->errmsg,
+             _("Bareos cannot write on disk Volume \"%s\" because: "
+               "The sizes do not match! Volume=%s Catalog=%s\n"),
+             VolumeName, edit_uint64(pos, ed1),
+             edit_uint64(dev->VolCatInfo.VolCatBytes, ed2));
+        Jmsg(jcr, M_ERROR, 0, jcr->errmsg);
+        Dmsg0(050, jcr->errmsg);
         MarkVolumeInError();
         return false;
       }
-    } else {
-      Mmsg(jcr->errmsg,
-           _("Bareos cannot write on disk Volume \"%s\" because: "
-             "The sizes do not match! Volume=%s Catalog=%s\n"),
-           VolumeName, edit_uint64(pos, ed1),
-           edit_uint64(dev->VolCatInfo.VolCatBytes, ed2));
-      Jmsg(jcr, M_ERROR, 0, jcr->errmsg);
-      Dmsg0(050, jcr->errmsg);
-      MarkVolumeInError();
-      return false;
-    }
-  } else if (dev->IsFifo()) {
-    return true;
-  } else {
-    Mmsg1(
-        jcr->errmsg,
-        _("Don't know how to check if EOD is valid for a device of type %s\n"),
-        dev->dev_type.c_str());
-    Jmsg(jcr, M_ERROR, 0, jcr->errmsg);
-    Dmsg0(050, jcr->errmsg);
-    return false;
+    } break;
+    case SeekMode::NOSEEK: {
+      return true;
+    } break;
   }
-
   return true;
 }
 
diff --git a/core/src/stored/scan.cc b/core/src/stored/scan.cc
index 4d6d271ce..857a877f2 100644
--- a/core/src/stored/scan.cc
+++ b/core/src/stored/scan.cc
@@ -2,7 +2,7 @@
    BAREOS® - Backup Archiving REcovery Open Sourced
 
    Copyright (C) 2006-2011 Free Software Foundation Europe e.V.
-   Copyright (C) 2016-2021 Bareos GmbH & Co. KG
+   Copyright (C) 2016-2022 Bareos GmbH & Co. KG
 
    This program is Free Software; you can redistribute it and/or
    modify it under the terms of version three of the GNU Affero General Public
@@ -35,8 +35,15 @@ namespace storagedaemon {
 /* Forward referenced functions */
 static bool IsVolumeNameLegal(char* name);
 
+bool Device::ScanForVolume(DeviceControlRecord* dcr)
+{
+  return ScanForVolumeImpl(dcr);  // call virtual implementation
+}
+
+/* default implementation: don't scan at all */
+bool Device::ScanForVolumeImpl(DeviceControlRecord*) { return false; }
 
-bool Device::ScanDirForVolume(DeviceControlRecord* dcr)
+bool Device::ScanDirectoryForVolume(DeviceControlRecord* dcr)
 {
   DIR* dp;
   struct dirent* result;
diff --git a/core/src/stored/vol_mgr.cc b/core/src/stored/vol_mgr.cc
index b4eff74cc..0d4b69516 100644
--- a/core/src/stored/vol_mgr.cc
+++ b/core/src/stored/vol_mgr.cc
@@ -420,7 +420,7 @@ VolumeReservationItem* reserve_volume(DeviceControlRecord* dcr,
    * accesses by multiple readers at once without disturbing each other.
    */
   if (me->filedevice_concurrent_read && !dcr->IsWriting()
-      && dcr->dev->IsFile()) {
+      && dcr->dev->CanReadConcurrently()) {
     nvol->SetJobid(dcr->jcr->JobId);
     nvol->SetReading();
     vol = nvol;
@@ -761,7 +761,8 @@ bool FreeVolume(Device* dev)
      *  - Config option filedevice_concurrent_read is not on.
      *  - The device is not of type File.
      */
-    if (vol->IsWriting() || !me->filedevice_concurrent_read || !dev->IsFile()) {
+    if (vol->IsWriting() || !me->filedevice_concurrent_read
+        || !dev->CanReadConcurrently()) {
       vol_list->remove(vol);
     }
     Dmsg2(debuglevel, "=== remove volume %s dev=%s\n", vol->vol_name,