TDBStore and NVStore should create an error if co exist. #9136

yossi2le · 2018-12-18T12:41:41Z

Description

TDBStore and NVStore usually use the same flash address for storage and therefore should not be instantiated simultaneously in the same process.
This PR adds a checkup for NVStore and TDBStore to create a runtime error in case that both of them co-exist in the same application.

This PR is dependent on #9135

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

offirko

LGTM

dannybenor · 2018-12-19T10:26:31Z

features/storage/system_storage/SystemStorage.cpp

+static SingletonPtr<PlatformMutex> system_storage_mutex;
+
+MBED_WEAK int set_internal_storage_ownership(internal_mem_resident_type_e in_mem_res, internal_mem_resident_type_e *out_mem_res)
+{


Lets create a simpler function without parameters, with the mutex inside, named "avoid_conflict_nvstore_tdbstore"

I have changed the name of the function and removed the output parameter

cmonr · 2018-12-19T17:49:27Z

Pinging @AnotherButler to look over the .md files.

cmonr

LGTM, pending docs review.

cmonr · 2018-12-19T17:52:18Z

Whoops, nevermind. @AnotherButler no need to review this. The docs are already a part of #9135.

…flash

NirSonnenschein · 2018-12-24T08:58:21Z

Technically could be ready for CI but as new commits were pushed @cmonr could you please take look to check it is still ok?

NirSonnenschein · 2018-12-26T08:49:25Z

CI started pending review of new changes

mbed-ci · 2018-12-26T13:35:09Z

Test run: FAILED

Summary: 3 of 7 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-IAR

yossi2le · 2018-12-26T15:22:25Z

The CI fail because it fails to build mbed-os-example-bootloader. I have issued a PR ARMmbed/mbed-os-example-bootloader#95 to fix the example and solve the issue.

0xc0170 · 2019-01-02T12:05:32Z

I removed CI as this still is waiting for preceeding PR

yogpan01 · 2019-01-07T09:05:39Z

@yossi2le Please merge #9135 and get this one also merged.
@0xc0170 please priortize these PRs as we need them to start testing on client side.

0xc0170 · 2019-01-07T12:27:52Z

@yossi2le Please merge #9135 and get this one also merged.

Merged, restarting CI here

mbed-ci · 2019-01-07T17:03:42Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 2
Build artifacts

dannybenor · 2019-01-08T11:49:50Z

@0xc0170 Can this be merged?

0xc0170 · 2019-01-09T13:56:26Z

Release label updated based on the agreement

adbridge · 2019-01-14T13:14:58Z

@yossi2le @dannybenor
We may need to look at how you storage guys are developing and pushing your PRs as I am having immense problems trying to patch this over to 5.11 branch.
So the problem came down to the fact that both #9135 and this PR both contain the following commits:
1d71fb1
b8b7292
9590441
b17d13e

Now Github itself seems to be able to resolve the fact that those 4 commits already exist. But neither
git am or git cherry-pick can , so this causes major issues in our automation!

davidsaada · 2019-01-14T14:40:16Z

@adbridge
Just to make sure we understand: the description of this PR says that it depends on #9135, so these commits will naturally be in both. So was the problem the missing rebase in this PR after #9135 was merged? As far as I manage to see, they were both merged in at the same time, so this was bound to happen. What are we missing?

adbridge · 2019-01-14T15:08:47Z

@davidsaada yes I suspect this needed to be rebased after #9135 went in.

yossi2le force-pushed the yossi_tdbstore_nvstore_co_exist branch from 87dd9e3 to 852b69d Compare December 18, 2018 13:35

yossi2le mentioned this pull request Dec 18, 2018

Get type for BlockDevice #9135

Merged

yossi2le force-pushed the yossi_tdbstore_nvstore_co_exist branch from 852b69d to e511c48 Compare December 18, 2018 15:28

cmonr requested a review from a team December 18, 2018 16:24

cmonr added the needs: review label Dec 18, 2018

offirko approved these changes Dec 19, 2018

View reviewed changes

dannybenor suggested changes Dec 19, 2018

View reviewed changes

dannybenor approved these changes Dec 19, 2018

View reviewed changes

yossi2le force-pushed the yossi_tdbstore_nvstore_co_exist branch 3 times, most recently from 9882e84 to 1c8ef90 Compare December 19, 2018 14:03

cmonr requested a review from AnotherButler December 19, 2018 17:48

cmonr approved these changes Dec 19, 2018

View reviewed changes

cmonr added release-version: 5.12.0-rc1 needs: preceding PR labels Dec 19, 2018

cmonr removed the request for review from AnotherButler December 19, 2018 17:51

Yossi Levy added 6 commits December 23, 2018 11:03

Add get_type method to block devices.

1d71fb1

astyle fix

b8b7292

astyle fix

9590441

Documnent changes and adding const to get_type method

b17d13e

Create runtime error if TDBStore and NVStore are created in internal …

fcd24aa

…flash

removing output parameter from avoid_conflict_nvstore_tdbstore function.

677dbd1

yossi2le force-pushed the yossi_tdbstore_nvstore_co_exist branch from 1c8ef90 to 677dbd1 Compare December 23, 2018 12:28

NirSonnenschein added the needs: CI label Dec 31, 2018

NirSonnenschein removed the needs: review label Dec 31, 2018

0xc0170 removed the needs: CI label Jan 2, 2019

0xc0170 removed the needs: preceding PR label Jan 7, 2019

0xc0170 added the ready for merge label Jan 8, 2019

0xc0170 merged commit cc9562e into ARMmbed:master Jan 8, 2019

0xc0170 added release-version: 5.11.2 and removed ready for merge release-version: 5.12.0-rc1 labels Jan 8, 2019

adbridge added Manually Patch release-version: 5.11.2 and removed release-version: 5.11.2 Manually Patch labels Jan 14, 2019

adbridge added release-version: 5.11.2 and removed Manually Patch labels Jan 25, 2019

TDBStore and NVStore should create an error if co exist. #9136

TDBStore and NVStore should create an error if co exist. #9136

Uh oh!

Conversation

yossi2le commented Dec 18, 2018

Description

Pull request type

Uh oh!

offirko left a comment

Choose a reason for hiding this comment

Uh oh!

dannybenor Dec 19, 2018

Choose a reason for hiding this comment

Uh oh!

yossi2le Dec 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmonr commented Dec 19, 2018

Uh oh!

cmonr left a comment

Choose a reason for hiding this comment

Uh oh!

cmonr commented Dec 19, 2018

Uh oh!

NirSonnenschein commented Dec 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NirSonnenschein commented Dec 26, 2018

Uh oh!

mbed-ci commented Dec 26, 2018

Test run: FAILED

Uh oh!

yossi2le commented Dec 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Jan 2, 2019

Uh oh!

yogpan01 commented Jan 7, 2019

Uh oh!

0xc0170 commented Jan 7, 2019

Uh oh!

mbed-ci commented Jan 7, 2019

Test run: SUCCESS

Uh oh!

dannybenor commented Jan 8, 2019

Uh oh!

0xc0170 commented Jan 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adbridge commented Jan 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidsaada commented Jan 14, 2019

Uh oh!

adbridge commented Jan 14, 2019

Uh oh!

Uh oh!

yossi2le Dec 19, 2018 •

edited

Loading

NirSonnenschein commented Dec 24, 2018 •

edited

Loading

yossi2le commented Dec 26, 2018 •

edited

Loading

0xc0170 commented Jan 9, 2019 •

edited

Loading

adbridge commented Jan 14, 2019 •

edited

Loading