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

ceph-disk: implement prepare --no-locking #14728

Merged
merged 2 commits into from Apr 26, 2017

Conversation

Projects
None yet
4 participants
@ghost

ghost commented Apr 22, 2017

No description provided.

@ghost ghost added core feature labels Apr 22, 2017

@ghost ghost requested review from tchaikov and liewegas Apr 22, 2017

ldachary added some commits Apr 22, 2017

ceph-disk: Prepare keeps track of args
Signed-off-by: Dan van der Ster <daniel.vanderster@cern.ch>
Signed-off-by: Loic Dachary <loic@dachary.org>
ceph-disk: implement prepare --no-locking
Signed-off-by: Dan van der Ster <daniel.vanderster@cern.ch>
Signed-off-by: Loic Dachary <loic@dachary.org>

@tchaikov tchaikov added the needs-qa label Apr 22, 2017

@ghost

This comment has been minimized.

ghost commented Apr 22, 2017

teuthology-suite -k distro --verbose --suite ceph-disk --ceph wip-ceph-disk-lock  --machine-type vps

@ghost ghost removed the needs-qa label Apr 22, 2017

@ghost

This comment has been minimized.

ghost commented Apr 22, 2017

@tchaikov thanks for the quick review :-) Removing the needs-qa because I still have doubts about not locking at all. IIRC there are race conditions even when preparing a single device. The global lock is clearly overkill but safe. However it may be necessary to have a per-device prepare lock so that activate does not happen until the prepare lock is lifted for a given partition and its associated auxiliary devices (journal etc.).

@ghost

This comment has been minimized.

ghost commented Apr 22, 2017

Hum, I remembered incorrectly. The activate lock is the one that is tricky, the prepare lock should not cause problems. Let see how the ceph-disk suite behaves.

@liewegas liewegas added the needs-qa label Apr 25, 2017

@tchaikov tchaikov self-assigned this Apr 25, 2017

@ghost

This comment has been minimized.

ghost commented Apr 25, 2017

The one that failed was because of infrastructure noise.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Apr 26, 2017

all three tests in ceph-disk suite passed.

@tchaikov tchaikov removed their assignment Apr 26, 2017

@tchaikov tchaikov merged commit f0f55a6 into ceph:master Apr 26, 2017

3 of 4 checks passed

arm build Build triggered. sha1 is merged.
Details
Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment