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

Move static header functionality into a separate module #1568

Closed
wants to merge 12 commits into from

Conversation

mulkieran
Copy link
Member

@mulkieran mulkieran commented Jun 19, 2019

This accomplishes the following:

  • Better encapsulation:

    • Since all static header related functionality is now in a separate module, the dependencies of the bda module on static header functionality is made more explicit and obvious.
    • Some constants that were previously shared among all parts of the BDA are now confined to the static header module. The two constants are the stratis magic number and the metadata version.
    • Since it is now obvious that StaticHeader need not be exported, the metadata module has better encapsulation.
    • A number of tests that solely tested static header behavior are now incorporated into the static header tests module and simplified.
  • Better documentation: It is now explicitly stated that all methods that read or write the static header seek to the 0 location before beginning. This was not at all obvious before.

  • StaticHeader::write is made an instance method, insuring, at the level of the StaticHeader impl, that only a valid signature block can be written to the device by our code. Previously it was necessary to rely on all code that invoked the write method, since the method could take an arbitrary buffer.

@mulkieran mulkieran self-assigned this Jun 19, 2019
@mulkieran mulkieran changed the title Master static header Move static header functionality into a separate module Jun 19, 2019
@mulkieran
Copy link
Member Author

The final goal is to use the same tricks used in mda.rs with types to ensure that our sizes are correct.

@mulkieran
Copy link
Member Author

Blocked by #1571.

Also, fix the header documentation.

Signed-off-by: mulhern <[email protected]>
Also, fix the header documentation.

Signed-off-by: mulhern <[email protected]>
This is a rough split, the next step is to induce better encapsulation.

Signed-off-by: mulhern <[email protected]>
Set the value in the StaticHeader constructor. This is a value that
the StaticHeader stores but the BDA decides.

Signed-off-by: mulhern <[email protected]>
This way, it becomes unnecessary to export StaticHeader at all.

Signed-off-by: mulhern <[email protected]>
It wipes the static header and nothing else. It improves encapsulation.

Signed-off-by: mulhern <[email protected]>
This is really where the property belongs.

Note that this means that the test module must import _BDA_STATIC_HDR_SIZE.
This is a strong hint that some of the tests now in the bda test module
really belong in a static header module.

Signed-off-by: mulhern <[email protected]>
For more precision and better encapsulation.
Rename or improve the test documentation as appropriate.

Signed-off-by: mulhern <[email protected]>
For better encapsulation and more sharing.

Signed-off-by: mulhern <[email protected]>
These functions are slightly better at the module level, now that a module
exists.

The read function just reads some bytes according to the static header
geometry.

The wipe function just wipes some bytes according to the static header
geometry.

The setup function does is not an instance method and may or may not
actually yield a StaticHeader.
This ensures that it is not possible to use StaticHeader::write to write
arbitrary data to the static header location, i.e., our code ensures that
the data written represents a valid static header.

Signed-off-by: mulhern <[email protected]>
@mulkieran
Copy link
Member Author

This will be obsoleted by #1572 and #1574 , perhaps others. It might be useful as a reference for #1573 .

@mulkieran mulkieran closed this Jun 24, 2019
@mulkieran mulkieran deleted the master-static-header branch June 24, 2019 14:32
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