Merge pull request #1375

stored: remove warning for maximum block size for tapes
bareos · Mar 2, 2023 · 0bf71f4 · 0bf71f4
2 parents 8e21475 + c4affb1
commit 0bf71f4
Show file tree

Hide file tree

Showing 14 changed files with 100 additions and 49 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,6 +23,7 @@ and since Bareos version 20 this project adheres to [Semantic Versioning](https:
 - FreeBSD: build cleanup [PR #1336]
 - improvements to pr-tool [PR #1389]
 - file checksums: add new signature algorithm xxh128 [PR #1359]
+- stored: remove warning for maximum block size for tapes [PR #1375]
 
 ### Removed
 - remove no longer used pkglists [PR #1335]
@@ -67,6 +68,7 @@ and since Bareos version 20 this project adheres to [Semantic Versioning](https:
 [PR #1367]: https://github.com/bareos/bareos/pull/1367
 [PR #1373]: https://github.com/bareos/bareos/pull/1373
 [PR #1374]: https://github.com/bareos/bareos/pull/1374
+[PR #1375]: https://github.com/bareos/bareos/pull/1375
 [PR #1377]: https://github.com/bareos/bareos/pull/1377
 [PR #1378]: https://github.com/bareos/bareos/pull/1378
 [PR #1386]: https://github.com/bareos/bareos/pull/1386

diff --git a/core/src/dird/dird_conf.cc b/core/src/dird/dird_conf.cc
@@ -4013,7 +4013,9 @@ static bool SaveResource(int type, ResourceItem* items, int pass)
       break;
     default:
       // Ensure that all required items are present
-      if (!ValidateResource(type, items, allocated_resource)) { return false; }
+      if (pass == 1 && !ValidateResource(type, items, allocated_resource)) {
+        return false;
+      }
       break;
   }
 
@@ -4023,7 +4025,20 @@ static bool SaveResource(int type, ResourceItem* items, int pass)
    * record. */
   if (pass == 2) {
     bool ret = UpdateResourcePointer(type, items);
-    return ret;
+    bool validation = true;
+    switch (type) {
+      case R_JOBDEFS:
+      case R_JOB:
+      case R_DIRECTOR:
+        break;
+      default: {
+        BareosResource* pass1_resource = my_config->GetResWithName(
+            type, allocated_resource->resource_name_);
+        validation = ValidateResource(type, items, pass1_resource);
+      } break;
+    }
+
+    return validation && ret;
   }
 
   if (!AddResourceCopyToEndOfChain(type)) { return false; }

diff --git a/core/src/lib/bnet.cc b/core/src/lib/bnet.cc
@@ -3,7 +3,7 @@
 
    Copyright (C) 2000-2011 Free Software Foundation Europe e.V.
    Copyright (C) 2011-2012 Planets Communications B.V.
-   Copyright (C) 2013-2022 Bareos GmbH & Co. KG
+   Copyright (C) 2013-2023 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
@@ -164,10 +164,8 @@ bool BnetTlsClient(BareosSocket* bsock,
   if (!bsock->tls_conn_init->TlsBsockConnect(bsock)) { goto err; }
 
   if (VerifyPeer) {
-    /*
-     * If there's an Allowed CN verify list, use that to validate the remote
-     * certificate's CN. Otherwise, we use standard host/CN matching.
-     */
+    /* If there's an Allowed CN verify list, use that to validate the remote
+     * certificate's CN. Otherwise, we use standard host/CN matching. */
     if (!verify_list.empty()) {
       if (!bsock->tls_conn_init->TlsPostconnectVerifyCn(jcr, verify_list)) {
         Qmsg1(bsock->jcr(), M_FATAL, 0,
@@ -274,27 +272,23 @@ const char* resolv_host(int family, const char* host, dlist<IPADDR>* addr_list)
       case AF_INET:
         addr = new IPADDR(rp->ai_addr->sa_family);
         addr->SetType(IPADDR::R_MULTIPLE);
-        /*
-         * Some serious casting to get the struct in_addr *
+        /* Some serious casting to get the struct in_addr *
          * rp->ai_addr == struct sockaddr
          * as this is AF_INET family we can cast that
          * to struct_sockaddr_in. Of that we need the
          * address of the sin_addr member which contains a
-         * struct in_addr
-         */
+         * struct in_addr */
         addr->SetAddr4(&(((struct sockaddr_in*)rp->ai_addr)->sin_addr));
         break;
       case AF_INET6:
         addr = new IPADDR(rp->ai_addr->sa_family);
         addr->SetType(IPADDR::R_MULTIPLE);
-        /*
-         * Some serious casting to get the struct in6_addr *
+        /* Some serious casting to get the struct in6_addr *
          * rp->ai_addr == struct sockaddr
          * as this is AF_INET6 family we can cast that
          * to struct_sockaddr_in6. Of that we need the
          * address of the sin6_addr member which contains a
-         * struct in6_addr
-         */
+         * struct in6_addr */
         addr->SetAddr6(&(((struct sockaddr_in6*)rp->ai_addr)->sin6_addr));
         break;
       default:
@@ -634,7 +628,7 @@ bool BareosSocket::FormatAndSendResponseMessage(
 
   StartTimer(30);  // 30 seconds
   if (send(m.c_str(), m.size()) <= 0) {
-    Dmsg1(100, "Could not send response message: %d\n", m.c_str());
+    Dmsg1(100, "Could not send response message: %s\n", m.c_str());
     StopTimer();
     return false;
   }

diff --git a/core/src/stored/device_resource.cc b/core/src/stored/device_resource.cc
@@ -3,7 +3,7 @@
 
    Copyright (C) 2000-2011 Free Software Foundation Europe e.V.
    Copyright (C) 2011-2012 Planets Communications B.V.
-   Copyright (C) 2013-2022 Bareos GmbH & Co. KG
+   Copyright (C) 2013-2023 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
@@ -273,32 +273,73 @@ void DeviceResource::CreateAndAssignSerialNumber(uint16_t number)
   resource_name_ = strdup(tmp_name.c_str());
 }
 
-bool DeviceResource::Validate()
+static void WarnOnNonZeroBlockSize(int max_block_size, std::string_view name)
 {
-  if (max_block_size > 0 && device_type != DeviceType::B_TAPE_DEV) {
+  if (max_block_size > 0) {
     my_config->AddWarning(fmt::format(
         FMT_STRING(
             "Device {:s}: Setting 'Maximum Block Size' is only supported on  "
             "tape devices"),
-        resource_name_));
+        name));
   }
+}
 
+static void WarnOnZeroMaxConcurrentJobs(int max_concurrent_jobs,
+                                        std::string_view name)
+{
   if (max_concurrent_jobs == 0) {
     my_config->AddWarning(fmt::format(
         FMT_STRING("Device {:s}: unlimited (0) 'Maximum Concurrent Jobs' (the "
                    "default) reduces the restore peformance."),
-        resource_name_));
+        name));
     my_config->AddWarning(fmt::format(
         FMT_STRING("Device {:s}: the default value for 'Maximum Concurrent "
                    "Jobs' will change from 0 (unlimited) to 1 in Bareos 23."),
-        resource_name_));
-  } else if (max_concurrent_jobs > 1 && device_type != DeviceType::B_TAPE_DEV) {
+        name));
+  }
+}
+
+static void WarnOnGtOneMaxConcurrentJobs(int max_concurrent_jobs,
+                                         std::string_view name)
+{
+  if (max_concurrent_jobs > 1) {
     my_config->AddWarning(fmt::format(
         FMT_STRING(
             "Device {:s}: setting 'Maximum Concurrent Jobs' on "
             "device that are not of type tape to a value higher than 1 is not "
             "recommended as it will reduce the restore performance."),
-        resource_name_));
+        name));
+  }
+}
+
+static bool ValidateTapeDevice(const DeviceResource& resource)
+{
+  ASSERT(resource.device_type == DeviceType::B_TAPE_DEV);
+
+  WarnOnZeroMaxConcurrentJobs(resource.max_concurrent_jobs,
+                              resource.resource_name_);
+
+  return true;
+}
+
+static bool ValidateGenericDevice(const DeviceResource& resource)
+{
+  WarnOnNonZeroBlockSize(resource.max_block_size, resource.resource_name_);
+  WarnOnZeroMaxConcurrentJobs(resource.max_concurrent_jobs,
+                              resource.resource_name_);
+  WarnOnGtOneMaxConcurrentJobs(resource.max_concurrent_jobs,
+                               resource.resource_name_);
+  return true;
+}
+
+
+bool DeviceResource::Validate()
+{
+  to_lower(device_type);
+  if (device_type == DeviceType::B_TAPE_DEV) {
+    return ValidateTapeDevice(*this);
+  } else {
+    return ValidateGenericDevice(*this);
   }
   return true;
 }

diff --git a/core/src/stored/stored_conf.cc b/core/src/stored/stored_conf.cc
@@ -735,9 +735,6 @@ static bool SaveResource(int type, ResourceItem* items, int pass)
   int i;
   int error = 0;
 
-  BareosResource* allocated_resource = *resources[type].allocated_resource_;
-  if (pass == 2 && !allocated_resource->Validate()) { return false; }
-
   // Ensure that all required items are present
   for (i = 0; items[i].name; i++) {
     if (items[i].flags & CFG_ITEM_REQUIRED) {
@@ -756,15 +753,18 @@ static bool SaveResource(int type, ResourceItem* items, int pass)
 
   // save previously discovered pointers into dynamic memory
   if (pass == 2) {
+    BareosResource* allocated_resource = my_config->GetResWithName(
+        type, (*items->allocated_resource)->resource_name_);
+    if (allocated_resource && !allocated_resource->Validate()) { return false; }
     switch (type) {
       case R_DEVICE:
       case R_MSGS:
       case R_NDMP:
         // Resources not containing a resource
         break;
       case R_DIRECTOR: {
-        DirectorResource* p = dynamic_cast<DirectorResource*>(
-            my_config->GetResWithName(R_DIRECTOR, res_dir->resource_name_));
+        DirectorResource* p
+            = dynamic_cast<DirectorResource*>(allocated_resource);
         if (!p) {
           Emsg1(M_ERROR_TERM, 0, _("Cannot find Director resource %s\n"),
                 res_dir->resource_name_);
@@ -775,8 +775,7 @@ static bool SaveResource(int type, ResourceItem* items, int pass)
         break;
       }
       case R_STORAGE: {
-        StorageResource* p = dynamic_cast<StorageResource*>(
-            my_config->GetResWithName(R_STORAGE, res_store->resource_name_));
+        StorageResource* p = dynamic_cast<StorageResource*>(allocated_resource);
         if (!p) {
           Emsg1(M_ERROR_TERM, 0, _("Cannot find Storage resource %s\n"),
                 res_store->resource_name_);
@@ -791,9 +790,7 @@ static bool SaveResource(int type, ResourceItem* items, int pass)
       }
       case R_AUTOCHANGER: {
         AutochangerResource* p
-            = dynamic_cast<AutochangerResource*>(my_config->GetResWithName(
-                R_AUTOCHANGER, res_changer->resource_name_));
-
+            = dynamic_cast<AutochangerResource*>(allocated_resource);
         if (!p) {
           Emsg1(M_ERROR_TERM, 0, _("Cannot find AutoChanger resource %s\n"),
                 res_changer->resource_name_);

diff --git a/core/src/tests/addresses_and_ports.cc b/core/src/tests/addresses_and_ports.cc
@@ -1,7 +1,7 @@
 /*
    BAREOS® - Backup Archiving REcovery Open Sourced
 
-   Copyright (C) 2018-2022 Bareos GmbH & Co. KG
+   Copyright (C) 2018-2023 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
@@ -119,7 +119,9 @@ static bool try_binding_director_port(std::string path_to_config,
 
   bool start_socket_server_ok
       = directordaemon::StartSocketServer(directordaemon::me->DIRaddrs);
-  EXPECT_TRUE(start_socket_server_ok) << "Could not start SocketServer";
+  EXPECT_TRUE(start_socket_server_ok)
+      << "Could not start SocketServer; "
+      << "Is a local director blocking the port ?";
   if (!start_socket_server_ok) { return false; }
 
   result = test_sockets(family, port);

diff --git a/core/src/tests/configs/sd_backend/bareos-sd.d/device/tape1.conf b/core/src/tests/configs/sd_backend/bareos-sd.d/device/tape1.conf
@@ -1,7 +1,7 @@
 Device {
   Name = tape1
   Media Type = File
-  Device Type = tape
+  Device Type = Tape
   Archive Device = /dev/null
   LabelMedia = yes
   Random Access = yes

diff --git a/core/src/tests/configs/sd_reservation/bareos-sd.d/device/auto1.conf b/core/src/tests/configs/sd_reservation/bareos-sd.d/device/auto1.conf
@@ -1,7 +1,7 @@
 Device {
   Name = auto1dev0
   Media Type = File
-  Device Type = tape
+  Device Type = Tape
   Archive Device = .
   LabelMedia = yes
   Random Access = yes
@@ -28,7 +28,7 @@ Device {
 Device {
   Name = auto1dev2
   Media Type = File
-  Device Type = tape
+  Device Type = Tape
   Archive Device = .
   LabelMedia = yes
   Random Access = yes
@@ -41,7 +41,7 @@ Device {
 Device {
   Name = auto1dev3
   Media Type = File
-  Device Type = tape
+  Device Type = Tape
   Archive Device = .
   LabelMedia = yes
   Random Access = yes
@@ -54,7 +54,7 @@ Device {
 Device {
   Name = auto1dev4
   Media Type = File
-  Device Type = tape
+  Device Type = Tape
   Archive Device = .
   LabelMedia = yes
   Random Access = yes

diff --git a/core/src/tests/configs/sd_reservation/bareos-sd.d/device/single2.conf b/core/src/tests/configs/sd_reservation/bareos-sd.d/device/single2.conf
@@ -2,5 +2,5 @@ Device {
   Name = single2
   Media Type = NotFile
   Archive Device = .
-  Device Type = tape
+  Device Type = Tape
 }
diff --git a/...nfigs/statistics_thread/sd_statistics_thread/default_config/bareos-sd.d/device/tape1.conf b/...nfigs/statistics_thread/sd_statistics_thread/default_config/bareos-sd.d/device/tape1.conf
@@ -1,7 +1,7 @@
 Device {
   Name = tape1
   Media Type = File
-  Device Type = tape
+  Device Type = Tape
   Archive Device = /dev/null
   LabelMedia = yes
   Random Access = yes

diff --git a/...igs/statistics_thread/sd_statistics_thread/only_collect_set/bareos-sd.d/device/tape1.conf b/...igs/statistics_thread/sd_statistics_thread/only_collect_set/bareos-sd.d/device/tape1.conf
@@ -1,7 +1,7 @@
 Device {
   Name = tape1
   Media Type = File
-  Device Type = tape
+  Device Type = Tape
   Archive Device = /dev/null
   LabelMedia = yes
   Random Access = yes

diff --git a/...gs/statistics_thread/sd_statistics_thread/only_interval_set/bareos-sd.d/device/tape1.conf b/...gs/statistics_thread/sd_statistics_thread/only_interval_set/bareos-sd.d/device/tape1.conf
@@ -1,7 +1,7 @@
 Device {
   Name = tape1
   Media Type = File
-  Device Type = tape
+  Device Type = Tape
   Archive Device = /dev/null
   LabelMedia = yes
   Random Access = yes

diff --git a/systemtests/tests/autochanger/create_autochanger_configs.sh.in b/systemtests/tests/autochanger/create_autochanger_configs.sh.in
@@ -1,7 +1,7 @@
 #!/bin/bash
 #   BAREOS® - Backup Archiving REcovery Open Sourced
 #
-#   Copyright (C) 2021-2021 Bareos GmbH & Co. KG
+#   Copyright (C) 2021-2023 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
@@ -86,7 +86,7 @@ EOF
     cat << EOF >> "${tape_device_conf_file}"
 Device {
     Name = "${bareos_tape_device_name}"
-    DeviceType = tape
+    DeviceType = Tape
 
     DriveIndex = ${drive_index}
 

diff --git a/systemtests/tests/block-size/create_autochanger_configs.sh.in b/systemtests/tests/block-size/create_autochanger_configs.sh.in
@@ -1,7 +1,7 @@
 #!/bin/bash
 #   BAREOS® - Backup Archiving REcovery Open Sourced
 #
-#   Copyright (C) 2021-2021 Bareos GmbH & Co. KG
+#   Copyright (C) 2021-2023 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
@@ -86,7 +86,7 @@ EOF
     cat << EOF >> "${tape_device_conf_file}"
 Device {
     Name = "${bareos_tape_device_name}"
-    DeviceType = tape
+    DeviceType = Tape
     DriveIndex = ${drive_index}
     ArchiveDevice = ${tape_device}
     MediaType = LTO