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

Change how BlockDevMgr::last_update_time is initialized #1548

Conversation

GuillaumeGomez
Copy link
Collaborator

Fixes #1509.

@mulkieran: Is it how you had it in mind?

@stratis-bot
Copy link

Test with Jenkins?

1 similar comment
@stratis-bot
Copy link

Test with Jenkins?

@mulkieran
Copy link
Member

ok to test

@mulkieran mulkieran added this to In progress (long term) in 2019June via automation Jun 2, 2019
@mulkieran mulkieran moved this from In progress (long term) to In progress in 2019June Jun 10, 2019
@mulkieran
Copy link
Member

@GuillaumeGomez Thanks for the PR! You'll note that in BlockDevMgr::initialize last_update_time is set to None.

This is more-or-less necessarily correct, because when the block devices belonging to the blockdevmgr are first claimed by stratisd, the actual value of the variable length metadata is not yet known. This metadata contains information about all sorts of higher-level devices that haven't yet been constructed.
So, the last_update_time must remain an Option.

However, at some later time, stratisd may come up again, and see devices that have already been initialized, and construct a blockdevmgr to manage them. Currently, it, incorrectly, constructs the blockdevmgr with a last_update_time value of None. It should, instead, find out the last update time from the existing metadata on the devices which it has been given.

I think that the area where the crucial mistake is made is really where StratPool::setup calls Backstore::setup with a last_update_time value of None. That can't always be right.

I would encourage you to look at the method get_metadata. This method is looking for the most recent variable length metadata written to all the devices. Unfortunately, having found the most recent value, it only return the metadata, and not the update time. If it can be expanded to return both values, then the value is available to StratPool::setup.

I'm afraid that the solution is less localized than I thought, but I think that that is the right place to start.

Copy link
Member

@mulkieran mulkieran left a comment

Choose a reason for hiding this comment

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

Please see comment.

@GuillaumeGomez
Copy link
Collaborator Author

I'll take a look.

@mulkieran mulkieran removed this from In progress in 2019June Jun 27, 2019
@mulkieran mulkieran added this to In progress (long term) in 2019July via automation Jun 27, 2019
@mulkieran mulkieran moved this from In progress (long term) to In progress in 2019July Jul 2, 2019
@stratis-bot
Copy link

Test with Jenkins?

@mulkieran mulkieran moved this from In progress to In progress (long term) in 2019July Jul 9, 2019
@mulkieran mulkieran assigned jbaublitz and unassigned GuillaumeGomez Jul 9, 2019
@mulkieran
Copy link
Member

Superceded by #1590.

@mulkieran mulkieran closed this Jul 23, 2019
@mulkieran mulkieran removed this from In progress (long term) in 2019July Jul 23, 2019
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.

Stratis pool becomes broken after being activated on machine with newer stratisd and incorrect date/time.
4 participants