Skip to content

fix: disable SavePage bit in other vendors - #664

Open
madjesc wants to merge 1 commit into
LinearTapeFileSystem:release/v2.4.9.1from
madjesc:fix/savepage-other-vendors
Open

madjesc wants to merge 1 commit into
LinearTapeFileSystem:release/v2.4.9.1from
madjesc:fix/savepage-other-vendors

Conversation

@madjesc

@madjesc madjesc commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary of changes

This pull request includes the following changes or fixes.

  • Fix of issue 2.4.9.1 mkltfs failed #656: mkltfs and ltfsck fail on release/2.4.9.1 with non-IBM drives (for example HP).
  • Set the Mode Select "save pages" (SP) bit only for IBM drives, and only when the Device Configuration Extension Mode Page (10h[01h]) is changed.
  • Add a bool save parameter to the modeselect backend operation (struct tape_ops) and to every tape driver implementation.
  • In tape_set_pews() and tape_enable_append_only_mode(), set save to true only when the drive vendor is IBM_VENDOR_ID.
  • All other modeselect callers pass save = false.
  • In the sg driver, build the CDB byte as 0x10 | (save ? 1 : 0) instead of the hardcoded 0x11.
  • Remove trailing whitespace in a few places and add #include <stdbool.h> to sg_tape.c.

Description

Motivation and context

Since #652, sg_modeselect() always sets PF=1 and SP=1 (cdb[1] = 0x11). This breaks non-IBM drives, because HP drive configuration pages are not savable. The reporter's HP Ultrium 6 returns Invalid Field in CDB (05/2400, -20501) for the Mode Select. That breaks several operations:

Tool Failing message
mkltfs LTFS17103E Cannot set PEWS, then LTFS12053E Cannot format medium: backend call failed (-20500), then LTFS15023I Formatting failed
ltfsck LTFS12031E Cannot set compression, then LTFS16080E Cannot check volume (8)
both LTFS17155E Cannot set WRITE MODE

The reporter found that main and commits up to 8518dac5 work, and that #652 introduced the regression.

What changes

The SP bit is now set only when both conditions hold:

  1. The caller asks for it through the new save argument. Only tape_set_pews() and tape_enable_append_only_mode() do this.
  2. The drive vendor reported by tape_get_info() matches IBM_VENDOR_ID.

Note

If tape_get_info() fails, savable stays false. Mode Select is then sent without SP, so the code fails safe.

Implementation notes

  • The new signature is:
    int (*modeselect)(void *device, unsigned char *buf, const size_t size, bool save);
  • Only the Linux sg driver uses save. The other drivers accept the parameter to match the interface and ignore it:
    • cam (FreeBSD)
    • file debug
    • ITDT image
    • lin_tape
    • scsipi (NetBSD)
    • IOKit (macOS)
  • src/libltfs/tape.c now includes tape_drivers/ibm_tape.h for IBM_VENDOR_ID.

Verification

The reporter tested this patch on an HP Ultrium 6 drive on Fedora 45 (see #656):

  • ltfsck -m /dev/sg5 completes with LTFS16022I Volume is consistent.
  • Mounting the volume with ltfs succeeds and the file system is usable.
Remaining non-fatal errors on the HP drive
  • Mode Select returns 05/2600 (Invalid Field in Parameter List, EDEV_ILLEGAL_REQUEST, 20500) when setting append-only mode. LTFS handles this and logs LTFS17266I Skip setting the append only mode because the drive doesn't seem to support it.
  • Mode Sense and Log Sense return 05/2400 because the HP drive doesn't support those pages.

I have not yet tested on IBM drives.

Dependencies

None.

Fixes #656

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have confirmed my fix is effective or that my feature works

Make the code only change the Device Configuration Extension Mode Page
(10h[01h]) to savable.
And ignore other vendors
@madjesc
madjesc changed the base branch from main to release/v2.4.9.1 November 5, 2026 18:05
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.

1 participant