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

Place static header specific functionality in a separate module, static_header in metadata module #1573

Closed
5 tasks done
mulkieran opened this issue Jun 24, 2019 · 9 comments
Closed
5 tasks done
Assignees
Labels
omnibus aggregator for other issues

Comments

@mulkieran
Copy link
Member

mulkieran commented Jun 24, 2019

Goals:

  • Move all static header specific functions into a static header module. Some of these methods are currently part of the BDA implementation, these will need to be identified and moved. BDA::write and BDA::save are some of these.
  • Move all static header specific tests into a test submodule in the static header module.
  • Currently, all methods that read and write static headers implicitly or explicitly seek to 0 before beginning. Make this explicit, by adding a parameter to these methods for the offset of the static header from the start of the device. All BDA methods will invoke these methods with a value of 0.
  • Probably, as a last step, hoist metadata module up a level so that it is on the same level as backstore module.

Procedure:

  1. Many static header specific tests currently in the bda tests module make use of unnecessarily general BDA methods to set up their test condition. These should be changed so they make use of static header specific methods. An obvious example is that BDA::initialize which initializes MDA headers is used, when all that is necessary is BDA::write which only initializes the static headers.
    NOTE: BDA::write is static header specific functionality which should eventually be moved into the proposed static_header module, but for this initial step, the best course is to leave it where it is.
    It should be possible to make this a separate PR.
    Outdated.
  2. Additional PRs.

You'll know when you're done when:

  • The static_header module is the only module which explicitly imports sizes::static_header_sizes.
  • ...

Component issues:

Reference:
https://stratis-storage.github.io/StratisSoftwareDesign.pdf, p.13 has a useful diagram of the metadata, helping to distinguish between the BDA, the MDA, the static header and the signature blocks.

@mulkieran
Copy link
Member Author

@GuillaumeGomez I think that all possible preparations that can be made have been made. I think it should now be possible to split out the relevant parts of the bda module, which is the StaticHeader definition and implementation and all StaticHeader-specific tests and any StaticHeader specific constants, into a static_header module. Moving onto the main October board. Happy refactoring!

@GuillaumeGomez
Copy link
Contributor

I think it'll have to wait mi-november at best. We're in the middle of the conferences rush! :)

@mulkieran
Copy link
Member Author

Ok. Happy conferencing!

@mulkieran mulkieran added the promote? promote to external bug tracker label Jan 10, 2020
@mulkieran
Copy link
Member Author

@GuillaumeGomez Would you like to come back to this? If yes, great! If not, please let us know, and thanks for all the work that you have done!

@GuillaumeGomez
Copy link
Contributor

Completely slipped out of my mind... So yes, definitely! Don't hesitate to ping me if you don't see any updates, kinda busy trying to do multiple things at once. :3

@mulkieran
Copy link
Member Author

Ok!, let me check and see what's next.

@mulkieran
Copy link
Member Author

Right, now I remember! All the prep work was done, and I thought now it should be possible to make a new file static_header.rs in the metadata module, and split out all the StaticHeader functionality into that module including all the static header-specific tests without unnecessary difficulty. Note that we're now doing development against the develop and not the master branch any more, so please grab that branch before you begin.

And thanks!

@GuillaumeGomez
Copy link
Contributor

Let's go for it then!

@mulkieran
Copy link
Member Author

Fixed by all the remaining attached issues.

@mulkieran mulkieran removed the promote? promote to external bug tracker label Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
omnibus aggregator for other issues
Projects
None yet
Development

No branches or pull requests

3 participants