Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sap_storage_setup: Add exact size disk check on top of approximate check #839

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

marcelmamula
Copy link
Contributor

Problem:

  • Current disk assignment works on approximate validation with (disk_size_gb-8) <= fs.disk_size <= (disk_size_gb+8) which invalidates this role when filesystems with less than 8 GB differences are used.

  • Current loop will pick first found, invalidating correct entries, causing cascading failure during LV creation because of device: ""

Example:

Disks with sizes 15, 20 and 25 are used.

/hana/shared will never be loaded, because /hana/log will find 25GB disk, use it and /hana/shared would be left without entry in following list of disks.volume_map

    sap_storage_setup_definition: [
      {
        "disk_size": 15,
        "filesystem_type": "xfs",
        "mountpoint": "/hana/data",
        "name": "hana_data"
      },
      {
        "disk_size": 20,
        "filesystem_type": "xfs",
        "mountpoint": "/hana/log",
        "name": "hana_log"
      },
      {
        "disk_size": 25,
        "filesystem_type": "xfs",
        "mountpoint": "/hana/shared",
        "name": "hana_shared"
      }
    ]

Solution:

Add initial check for exact sizes, followed up by original approximate check which will run empty if all disk sizes were correctly found during first pass.

  • First pass: disk_size_gb == fs.disk_size
  • Second pass: (disk_size_gb-8) <= fs.disk_size <= (disk_size_gb+8)

@marcelmamula marcelmamula added the bug Something isn't working label Aug 21, 2024
@marcelmamula marcelmamula self-assigned this Aug 21, 2024
Copy link
Contributor

@ja9fuchs ja9fuchs left a comment

Choose a reason for hiding this comment

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

lgtm, nice catch for tiny disk sizing
👍

@marcelmamula marcelmamula removed the request for review from sean-freeman August 22, 2024 07:50
@marcelmamula marcelmamula merged commit ca4ea52 into sap-linuxlab:dev Aug 22, 2024
3 of 4 checks passed
@marcelmamula marcelmamula deleted the storage branch September 9, 2024 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants