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

librbd: Include WorkQueue.h since we use it #13322

Merged
merged 1 commit into from Feb 10, 2017

Conversation

Projects
None yet
5 participants
@b-ranto
Contributor

b-ranto commented Feb 8, 2017

We use m_work_queue of type ContextWQ in handle_update function but we
do not include common/WorkQueue.h that defines ContextWQ. This results
in dereference of an incomplete type and causes build error in latest
Fedora rawhide (future 26).

Signed-off-by: Boris Ranto branto@redhat.com

@ktdreyer ktdreyer requested review from dillaman and trociny Feb 8, 2017

@badone

badone approved these changes Feb 9, 2017

LGTM

@@ -11,6 +11,7 @@
#include "common/Cond.h"
#include "common/Mutex.h"
#include "common/Cond.h"
#include "common/WorkQueue.h"

This comment has been minimized.

@trociny

trociny Feb 9, 2017

Contributor

Please, remove then class ContextWQ forward declaration below.

Another approach would be to define Journal<I>::MetadataListener::handle_update() in Journal.cc.

This comment has been minimized.

@badone

badone Feb 9, 2017

Contributor

There's already a forward declaration just after the include lines?

This comment has been minimized.

@trociny

trociny Feb 9, 2017

Contributor

Yes, and it has became redundant after WorkQueue.h was included.

This comment has been minimized.

@b-ranto

b-ranto Feb 9, 2017

Contributor

@badone Yes there is one but it is incomplete and we call its method in Journal<I>::MetadataListener::handle_update(). The recent compilers seem to be more restrictive towards this and do not allow incomplete declarations for this purpose.

This comment has been minimized.

@badone

badone Feb 9, 2017

Contributor

ahhh... I misread @trociny 's update and thought he was asking you to add a forward declaration for some reason (re-reading it I have no idea why). Sorry for the noise.

@trociny

This comment has been minimized.

Contributor

trociny commented Feb 9, 2017

I expect kraken and jewel are affected too? Then it would be nice if you create a tracker issue, so we wouldn't forget to backport this.

@b-ranto

This comment has been minimized.

Contributor

b-ranto commented Feb 9, 2017

@trociny Yep, kraken and jewel are affected as well. http://tracker.ceph.com/issues/18862

EDIT: I have also updated the patch to remove the (now) redundant line and commit message now includes the tracker issue.

librbd: Include WorkQueue.h since we use it
We use m_work_queue of type ContextWQ in handle_update function but we
do not include common/WorkQueue.h that defines ContextWQ. This results
in dereference of an incomplete type and causes build error in latest
Fedora rawhide (future 26).

Fixes: http://tracker.ceph.com/issues/18862

Signed-off-by: Boris Ranto <branto@redhat.com>
@trociny

trociny approved these changes Feb 9, 2017

👍

@dillaman dillaman merged commit 9a3c4d0 into master Feb 10, 2017

2 of 3 checks passed

default Build finished.
Details
Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details

@dillaman dillaman deleted the wip-include-workqueue branch Feb 10, 2017

smithfarm pushed a commit to SUSE/ceph that referenced this pull request Feb 22, 2017

divergent_priors2: give divergent time to come up
Fixes: ceph#13322
Signed-off-by: Samuel Just <sjust@redhat.com>
(cherry picked from commit c0b0ec2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment