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

os/bluestore/NVMEDevice: fix the compilation issue for collect_metadata #14455

Merged
merged 1 commit into from Apr 11, 2017

Conversation

Projects
None yet
5 participants
@optimistyzy
Contributor

optimistyzy commented Apr 11, 2017

Signed-off-by: optimistyzy optimistyzy@gmail.com

bluestore/NVMEDevice: fix the compilation issue for collect_metadata
Signed-off-by: optimistyzy <optimistyzy@gmail.com>
@optimistyzy

This comment has been minimized.

Show comment
Hide comment
@optimistyzy

optimistyzy Apr 11, 2017

Contributor

@liupan1111 @yuyuyu101 tested with workload, it solves the issue.

Contributor

optimistyzy commented Apr 11, 2017

@liupan1111 @yuyuyu101 tested with workload, it solves the issue.

@liupan1111

This comment has been minimized.

Show comment
Hide comment
@liupan1111

liupan1111 Apr 11, 2017

Contributor

LGTM!

@liewegas This is a polish for your commit c431663

Could you help double check?

Contributor

liupan1111 commented Apr 11, 2017

LGTM!

@liewegas This is a polish for your commit c431663

Could you help double check?

@liewegas liewegas changed the title from bluestore/NVMEDevice: fix the compilation issue for collect_metadata to os/bluestore/NVMEDevice: fix the compilation issue for collect_metadata Apr 11, 2017

@liewegas

This comment has been minimized.

Show comment
Hide comment
@liewegas

liewegas Apr 11, 2017

Member

Thanks!

Speaking of which, I think @ifed01 was having some trouble getting SPDK to compile the other day. Since DPDK and SPDK are now both submodules, perhaps we can enable the build by default? (We can always disable it in the package builds if we don't want to ship it.)

Member

liewegas commented Apr 11, 2017

Thanks!

Speaking of which, I think @ifed01 was having some trouble getting SPDK to compile the other day. Since DPDK and SPDK are now both submodules, perhaps we can enable the build by default? (We can always disable it in the package builds if we don't want to ship it.)

@liewegas liewegas merged commit b8dc378 into ceph:master Apr 11, 2017

3 checks passed

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

This comment has been minimized.

Show comment
Hide comment
@liupan1111

liupan1111 Apr 11, 2017

Contributor

@liewegas,

In current code, we could find spdk is controlled by a macro "HAVE_SPDK":

#if defined(HAVE_SPDK)
  if (type == "ust-nvme") {
    return new NVMEDevice(cct, cb, cbpriv);
  }
#endif

And HAVE_SPDK will be defined only when "WITH_SPDK" is on when run cmake.

In my opinion, we could set "WITH_SPDK" to on as default(maybe off when ship), and add an new option in config_opt.h(maybe osd_device_type?), when set "userspace", NVMEDevice(spdk) will be used, otherwise, "kernel", KernelDevice will be used.

@yuyuyu101, do you agree?

Contributor

liupan1111 commented Apr 11, 2017

@liewegas,

In current code, we could find spdk is controlled by a macro "HAVE_SPDK":

#if defined(HAVE_SPDK)
  if (type == "ust-nvme") {
    return new NVMEDevice(cct, cb, cbpriv);
  }
#endif

And HAVE_SPDK will be defined only when "WITH_SPDK" is on when run cmake.

In my opinion, we could set "WITH_SPDK" to on as default(maybe off when ship), and add an new option in config_opt.h(maybe osd_device_type?), when set "userspace", NVMEDevice(spdk) will be used, otherwise, "kernel", KernelDevice will be used.

@yuyuyu101, do you agree?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment