Skip to content

sd: refactor the SD's backend interface#1272

Merged
arogge merged 26 commits intobareos:masterfrom
arogge:dev/arogge/master/demux-backend
Nov 9, 2022
Merged

sd: refactor the SD's backend interface#1272
arogge merged 26 commits intobareos:masterfrom
arogge:dev/arogge/master/demux-backend

Conversation

@arogge
Copy link
Member

@arogge arogge commented Oct 6, 2022

This changes the SD Backend and Device interface in a way that is supposed to make it easier to add new backends.

  • file device is now also built and loaded dynamically
  • Device Type setting will accept any string
  • presence of a configured storage backend checked after loading the configuration
  • systemtests fail early if a daemon config doesn't pass a test with -t
  • Remove IsFile() and IsFifo() from Device (in favor of new checks)
  • Increase CMake required version to 3.17
  • static linking of chunked, droplet and gentape into the backends that use them

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General
  • PR name is meaningful
  • Purpose of the PR is understood
  • Commit descriptions are understandable and well formatted
  • Check backport line
  • Is the PR title usable as CHANGELOG entry?
  • Separate commit for CHANGELOG.md ("update CHANGELOG.md"). The PR number is correct.
  • Merge corresponding PR in git.bareos.com/ci-scripts
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR
  • bareos-check-sources --since-merge does not report any problems

@arogge arogge force-pushed the dev/arogge/master/demux-backend branch 5 times, most recently from b3168f6 to b47cec3 Compare October 11, 2022 12:39
@arogge arogge marked this pull request as ready for review October 12, 2022 05:13
@arogge arogge force-pushed the dev/arogge/master/demux-backend branch 2 times, most recently from a6502d9 to 17356b7 Compare October 19, 2022 10:23
@arogge arogge assigned pstorz and unassigned alaaeddineelamri Oct 20, 2022
@arogge arogge force-pushed the dev/arogge/master/demux-backend branch from 17356b7 to 3485021 Compare October 25, 2022 15:35
@pstorz pstorz self-requested a review October 26, 2022 08:03
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still want to ship libbareossd-chunked.so and libbareossd-gentape.so?
No, we want to link chunked and gentape statically into the backends (i.e. droplet and tape) instead of shipping them as shared libraries.

@arogge arogge requested a review from pstorz November 2, 2022 19:09
@arogge arogge force-pushed the dev/arogge/master/demux-backend branch from 4a17601 to 15ff3fe Compare November 3, 2022 08:18
@pstorz pstorz changed the title Refactor the SD's backend interface sd: refactor the SD's backend interface Nov 3, 2022
@arogge arogge force-pushed the dev/arogge/master/demux-backend branch 2 times, most recently from 2b49360 to f09340b Compare November 4, 2022 09:29
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just minimal changes, please check my comments.

@arogge arogge force-pushed the dev/arogge/master/demux-backend branch 2 times, most recently from d57f5a4 to d41e8bd Compare November 7, 2022 16:36
this patch amends the SD configuration in every systemtest to have the
correct BackendDirectory set, so we can load dynamic backends during
testing.
It also removed the BackendDirectory when built without dynamic
backends.
remove the unused FlushDevice() function from the backend interface and
all interfaces.
Instead of building unix_file_device/win32_file_device directory into
libbareossd, this patch builds them as a dynamic backend and removes
special handling for them.
Also move all backend build configuration into the backends directory.
This patch removes the remaining traces of these three removed backends
from the codebase.
arogge added 18 commits November 7, 2022 17:37
Previously the device-type for a device where the type was not specified
was determined when it was first used. Now we do the check for every
device right after the configuration was loaded and fail early if
needed.
This patch turns the SD's device_type into a std::string, so we can use
arbitrary backend names without registering them in a central list of
backends.
If you configure a device, the backend will be checked when the
configuration is loaded. Thus a lot of the existing test configurations
need to be adapted to either use existing storge backends or to have
BackendDirectory configured.
This patch introduces a plugin regsitry that contains all loaded
backends. Storage backends will self-register their factory either at
startup (in case the backend is statically linked into the SD) or when
the backend is loaded.

From the backend's perspective, there's no difference if it is
statically or dynamically loaded anymore.

Finally, the loading of the backends is done right after loading the
configuration, so for every device we check the presence of its backend
and fail if it is missing or cannot be loaded.
This patch removes now unused parts of the old storage backend
interface.
Instead of returning a factory that will then provide Device*, we can
return the Device* from the PluginRegistry's factory directly. This
removes the BackendInterface layer that wrapped the factory again.
Previously the droplet tests were integrated into the sd_backend tests.
With the new backend loading, we cannot configure a droplet backend if
we did not build it, thus we need to extract the droplet tests so we can
disable them completely.
This patch refactors all uses of IsFile() into individual checks that
should do what the original author intended.
This is now implemented via SeekType. The refactored code also catches
some cases when a backend does not override a required method.
Previously IsTape() was called for display purposes. This patch displays
the configured Device Type instead.
Previously, the baseclasses for generic tape support and chunked devices
were built as shared modules. As this code is never shared, we can just
link it all into the resulting backends with no drawbacks at all.
Historically, we linked against a 3rd-party package. Thus we were using
dynamic linking.
As we're shipping libdroplet ourselves and there is no other consumer
than the droplet storage backend, it is easier to link it directly into
the backend.
As we have so many kinds of plugins, we'll just call it
ImplementationFactory as it is a factory that returns implementations.
@arogge arogge force-pushed the dev/arogge/master/demux-backend branch 2 times, most recently from 7143bcb to 62674a3 Compare November 7, 2022 16:42
@arogge arogge requested a review from pstorz November 7, 2022 16:44
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work!

also set cmake_policy version to 3.17...3.19 enabling all policies
present in CMake up until 3.19.
CMake 3.19 contains the newest policy we require (CMP0109) which makes
find_program() only check for executability but not readability which
allows to find sudo.
@arogge arogge force-pushed the dev/arogge/master/demux-backend branch from 615be9b to bd4a80c Compare November 9, 2022 15:17
@arogge arogge merged commit 95f1be8 into bareos:master Nov 9, 2022
@arogge arogge deleted the dev/arogge/master/demux-backend branch November 9, 2022 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants