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

Honor sector size change via ModeSelect6 in scsicd #1406

Merged
merged 7 commits into from
Jan 9, 2024

Conversation

kkaempf
Copy link
Contributor

@kkaempf kkaempf commented Dec 26, 2023

DEC's VMS can't handle 2k sector sizes.

Fixes #1397

@rdmark
Copy link
Member

rdmark commented Jan 5, 2024

@kkaempf The logic looks reasonable to me!

Since you've gotten the hang of the googletest unit tests now (to some extent at least ;) ), have you thought about how SCSICD::ModeSelect can be tested?

@kkaempf
Copy link
Contributor Author

kkaempf commented Jan 5, 2024

Since you've gotten the hang of the googletest unit tests now (to some extent at least ;) ),

🤣

have you thought about how SCSICD::ModeSelect can be tested?

Working on it ...
I'd need to test ModeSense (check if scsicd sector size is 2048), ModeSelect (set to 512), and ModeSense again (verify sector size is 512).

@kkaempf
Copy link
Contributor Author

kkaempf commented Jan 5, 2024

Now with test 🤡

Requires #1405 to be merged first !

@rdmark rdmark changed the base branch from main to develop January 7, 2024 06:10
@rdmark
Copy link
Member

rdmark commented Jan 7, 2024

@kkaempf The other PR has been merged now so please rebase this one!

Also, note that I changed the base branch from main to develop. All new development happens in the latter. You may need to do some resetting of HEAD and force pushing to get back to a clean changeset.

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>
and honor sector size setting.

DEC's VMS can't handle 2k sector sizes and uses ModeSelect6 to set the
sector size to 512 bytes.

Fixes PiSCSI#1397

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>
Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>
Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>
The track calculation is based on sector size and must be reset after
change of sector size.

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>
The disk cache is based on sector size and must be resized when the
sector size changes.

Disk::ResizeCache needs a `raw` parameter, make this value accessible
from the current cache.

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>
@kkaempf
Copy link
Contributor Author

kkaempf commented Jan 7, 2024

Well, that escalated a bit. 🤯
Changing the sector size also affect CD track and Disk cache sizes.
Re-create both when changing the sector size.

cpp/devices/disk.h Outdated Show resolved Hide resolved
Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>
@kkaempf kkaempf requested a review from rdmark January 8, 2024 18:11
Copy link

sonarcloud bot commented Jan 8, 2024

Copy link
Member

@rdmark rdmark left a comment

Choose a reason for hiding this comment

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

Thanks for your contributions!

@rdmark rdmark merged commit b7f65d3 into PiSCSI:develop Jan 9, 2024
8 checks passed
@kkaempf kkaempf deleted the issue-1397 branch January 13, 2024 11:14
kkaempf added a commit to kkaempf/piscsi that referenced this pull request Apr 2, 2024
* Make ModeSelect() non-const

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* Implement ModeSelect for scsicd

and honor sector size setting.

DEC's VMS can't handle 2k sector sizes and uses ModeSelect6 to set the
sector size to 512 bytes.

Fixes PiSCSI#1397

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* Test sector size setting via ModeSelect in SCSICD

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* Re-calculate total blocks when sector size changes

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* Reset CD data tracks after sector size change

The track calculation is based on sector size and must be reset after
change of sector size.

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* resize cache after change of sector size

The disk cache is based on sector size and must be resized when the
sector size changes.

Disk::ResizeCache needs a `raw` parameter, make this value accessible
from the current cache.

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* Make GetRawMode const

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

---------

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>
rdmark added a commit that referenced this pull request Apr 9, 2024
rdmark added a commit that referenced this pull request Apr 13, 2024
…ging (#1451)

* Revert "Don't ResizeCache on sector change if no filename is defined (#1438)"

This reverts commit dd9a329.

* Revert "Add ModeSense page 0x25 (DEC special function control page) (#1412)"

This reverts commit 1121b8d.

* Revert "DiskCache needs a size"

This reverts commit 7cc8df2.

* Revert "Honor sector size change via ModeSelect6 in scsicd (#1406)"

This reverts commit b7f65d3.

* Revert "Multiple fixes for ModeSelect (#1405)"

This reverts commit ad5eae9.
rdmark pushed a commit that referenced this pull request May 1, 2024
* Make ModeSelect() non-const

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* Implement ModeSelect for scsicd

and honor sector size setting.

DEC's VMS can't handle 2k sector sizes and uses ModeSelect6 to set the
sector size to 512 bytes.

Fixes #1397

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* Test sector size setting via ModeSelect in SCSICD

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* Re-calculate total blocks when sector size changes

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* Reset CD data tracks after sector size change

The track calculation is based on sector size and must be reset after
change of sector size.

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* resize cache after change of sector size

The disk cache is based on sector size and must be resized when the
sector size changes.

Disk::ResizeCache needs a `raw` parameter, make this value accessible
from the current cache.

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* Make GetRawMode const

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

---------

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>
rdmark added a commit that referenced this pull request May 1, 2024
…ging (#1451)

* Revert "Don't ResizeCache on sector change if no filename is defined (#1438)"

This reverts commit dd9a329.

* Revert "Add ModeSense page 0x25 (DEC special function control page) (#1412)"

This reverts commit 1121b8d.

* Revert "DiskCache needs a size"

This reverts commit 7cc8df2.

* Revert "Honor sector size change via ModeSelect6 in scsicd (#1406)"

This reverts commit b7f65d3.

* Revert "Multiple fixes for ModeSelect (#1405)"

This reverts commit ad5eae9.
kkaempf added a commit to kkaempf/piscsi that referenced this pull request Aug 9, 2024
* Make ModeSelect() non-const

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* Implement ModeSelect for scsicd

and honor sector size setting.

DEC's VMS can't handle 2k sector sizes and uses ModeSelect6 to set the
sector size to 512 bytes.

Fixes PiSCSI#1397

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* Test sector size setting via ModeSelect in SCSICD

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* Re-calculate total blocks when sector size changes

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* Reset CD data tracks after sector size change

The track calculation is based on sector size and must be reset after
change of sector size.

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* resize cache after change of sector size

The disk cache is based on sector size and must be resized when the
sector size changes.

Disk::ResizeCache needs a `raw` parameter, make this value accessible
from the current cache.

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

* Make GetRawMode const

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>

---------

Signed-off-by: Klaus Kämpf <kkaempf@gmail.com>
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.

ModePageDevice::ModeSelect() is unimplemented but called by ScsiController::XferOutBlockOriented()
2 participants