Skip to content

cmake: fix the build on AArch64#10311

Closed
tone-zhang wants to merge 185 commits intoceph:masterfrom
tone-zhang:tone_cmake
Closed

cmake: fix the build on AArch64#10311
tone-zhang wants to merge 185 commits intoceph:masterfrom
tone-zhang:tone_cmake

Conversation

@tone-zhang
Copy link
Contributor

When compile the Ceph, because some compiling options are
missed, the compiler stops at src/erasure-code.
Update the related CMakelists.txt files, and make the
Ceph pass the compiling.

When compile the Ceph UT, because the -msse and -msse2
are hardcoded in the test/CMakeLists.txt, the Ceph UT build
fail in AArm64. Update the source code to fix the issue.

In Ceph erasure UT, because the usful libary is missed, the
link is broken. Update test/erasure-code/CMakeLists.txt to
fix the link error.

Signed-off-by: tone.zhang tone.zhang@linaro.org

perf_local.cc
perf_helper.cc)

# create a tmp file with an empty main()
Copy link
Contributor

@tchaikov tchaikov Jul 15, 2016

Choose a reason for hiding this comment

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

maybe we should extract this piece of code into a cmake module in cmake/modules, probably named "CheckCompileTargets", maybe a better name? and check the defined variables here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool! It is good idea. Will do. Thanks!

@tone-zhang
Copy link
Contributor Author

@tchaikov I updated the code, could you please have a review? Thanks!
@ALL Any comments are welcome.

if (INTEL_SSE4_2)
set(SSE4_FLAGS "${SSE4_FLAGS} -msse4.2")
endif (INTEL_SSE4_2)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${SSE4_FLAGS}")
Copy link
Contributor

Choose a reason for hiding this comment

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

i am not sure if we want enable SSE3 and SSE4 globally. ec_jerasure_sse{3,4} are compiled as plugins, so user can opt out of them, but if we enable it, we force the user to use a host machine with better instruction set support than the build machine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tchaikov Right. Should leave the choice to the user. I will roll back the change in src/CMakeLists.txt, and update erasure-code/jerasure/CMakeLists.txt

@tchaikov
Copy link
Contributor

tchaikov commented Jul 18, 2016

could you remove the Reviewed-by: Kefu Chai <kchai@redhat.com> and probably other Reviewed-by lines in the commit message? in general, we add the "Reviewed-by" line in the merge commit. see https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#preparing-and-sending-patches

@tone-zhang
Copy link
Contributor Author

@tchaikov Thanks a lot for your remind. I will remove the "reviewed-by" in next update.

@tone-zhang tone-zhang force-pushed the tone_cmake branch 3 times, most recently from d5924db to aa7833e Compare July 19, 2016 08:37
@tone-zhang
Copy link
Contributor Author

@tchaikov I updated the code according to your comments, could you please have a review? Thanks!

@tchaikov
Copy link
Contributor

@tone-zhang will take a look at your PR tomorrow.

@tchaikov tchaikov self-assigned this Jul 21, 2016
@tone-zhang
Copy link
Contributor Author

@tchaikov Thanks a lot!

@@ -0,0 +1,70 @@
# CMake module to get the compile options to match the target
Copy link
Contributor

Choose a reason for hiding this comment

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

@tone-zhang i'd suggest you have two commits for this PR

  1. a refactor only commit: extract CheckCompileTargets.cmake out of src/CMakeLists.txt and use this module in src/jerssure/CMakeLists.txt instead of repeating it in the top level CMakeLists.txt. i think it's better off include the module at the place where we are using it.
  2. a bug fix for AArch64 build: to include(CheckCompileTargets) in src/test/CMakeLists.txt. and other fixes for building failures on ARM.

also

  1. could you fix the typo in your commit message: s/AArm64/AArch64/ ?
  2. could you prefix the title of your commit message with the subcomponent your are changing ? see https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#preparing-and-sending-patches. in this case, the prefix would be "cmake: ".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tchaikov Kefu, thanks a lot! Tow commits can make the PR clearer. I will do it now.
Also I will correct the commit message. Thanks!

@tchaikov tchaikov changed the title Fix the Ceph and Ceph UT compiling error in AArm64 cmake: fix the build on AArch64 Jul 22, 2016
tone.zhang added 2 commits July 22, 2016 13:50
Extract the CheckCompileTargets.cmake module out of
src/CMakeLists.txt. It is a used to generate some the compiler options
according to the CPU type.
Update the related CMakeLists.txt files to invoke the common module

Signed-off-by: tone.zhang <tone.zhang@linaro.org>
Fix one bug in src/test/CMakeLists.txt, the compiler options
"-msse -msse2" should be set according to the CPU type.
Also correct the dependent library in erasure-code UT
for AArch64

Signed-off-by: tone.zhang <tone.zhang@linaro.org>
@tone-zhang
Copy link
Contributor Author

@tchaikov Kefu, I split the PR to two commits. Could you please have a review? Thanks a lot!

@tchaikov
Copy link
Contributor

@tone-zhang needs rebase.

stiopaa1 and others added 9 commits July 25, 2016 07:15
Signed-off-by: Michal Jarzabek <stiopa@gmail.com>
Signed-off-by: Michal Jarzabek <stiopa@gmail.com>
Signed-off-by: SirishaGuduru sirishaguduru99@gmail.com
Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>
Signed-off-by: Alyona Kiseleva <akiselyova@mirantis.com>
For RocksDB, iterator will bypass row cache.
We don't want this to happen for get(), after row cache is enabled.

Signed-off-by: Jianjian Huo <jianjian.huo@ssi.samsung.com>
Signed-off-by: Mark Nelson <mnelson@redhat.com>
Signed-off-by: Haomai Wang <haomai@xsky.com>
Signed-off-by: Haomai Wang <haomai@xsky.com>
chhabaramesh and others added 22 commits July 25, 2016 07:25
Signed-off-by: Ramesh Chander <Ramesh.Chander@sandisk.com>
Signed-off-by: John Spray <john.spray@redhat.com>
This file documents how to configure RGW to use Apache/FastCGI, so rename
the file and modify the title and intro to make that clear.

Also, add a note that CGI can pose a security risk - e.g. http://httpoxy.org

Signed-off-by: Nathan Cutler <ncutler@suse.com>
Revert the RGW torrent commit for now, it was causing issues.

Signed-off-by: Daniel Gryniewicz <dang@redhat.com>
Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>
As same as amazon S3 interface,"PUT Bucket lifecycle" and
"DELETE Bucket lifecycle" have been implemented,
"GET Bucket lifecycle" not realized yet as S3cmd has not
realize it also.
The feature`s main point is to remove expire file per day.
Files transfer from hot layer to cold layer is not supported.
ToDo:Maybe to transfer from replicate pool to EC pool or
from ssd to sata pool will be valuable.

Now put all buckets which should do lifecycle into shard
objects in .rgw.lc pool.

lifecycle config file format:
<LifecycleConfiguration>
    <Rule>
        <ID>sample-rule</ID>
        <Prefix></Prefix>
        <Status>enable</Status>
        <Expiration>
           <Days>1</Days>
        </Expiration>
    </Rule>
</LifecycleConfiguration>

Signed-off-by: Ji Chen <insomnia@139.com>
Signed-off-by: Daniel Gryniewicz <dang@redhat.com>
some packages do not package python modules or scripts. so override
dh_python2 to exclude them.

this change silences warnings like:
```
warning: dpkg-gencontrol: package ceph-mon: unused substitution
variable ${python:Provides}
```

Signed-off-by: Kefu Chai <kchai@redhat.com>
* debian/control:
  as we have listed the linked libraries in Depends section, for example,
  python-rados depends on librados. and we don't need `dpkg-shlibdeps` to
  help figure out shared library substvar dependencies for us. by removing
  them, we can silence the warnings of
```
warning: dpkg-shlibdeps: package could avoid a useless dependency if
debian/python-rados/usr/lib/python2.7/dist-packages/rados.x86_64-linux-gnu.so
was not linked against libpthread.so.0 (it uses none of the library's
symbols)
```
  -lpthread is introduced by `python-config --ldflags` but it turns out we
  are not using any symbols from pthread in the extension directly. and
  pthread is included in glibc. so this does not added any extra
  dependency to python-* pacakges. but it's desirable to have less
  warnings.
* debian/rules: exclude python-* packages from dh_shlibdeps, as we will
  not use it to prepare the shlib deps substvars for these packages any
  more.

Signed-off-by: Kefu Chai <kchai@redhat.com>
* debian/control: Build-Depends: s/python-dev/python-all-dev/, per
    https://wiki.debian.org/Python/FAQ#Build_dependencies

Signed-off-by: Kefu Chai <kchai@redhat.com>
* ceph-base: use ${python:Depends} instead of listing the python
  dependencies manually, dh_python2 will scan the requirements
  of ceph-detect-init. and fill the subst var for us.
* ceph-common: add ${python:Depends}, as it packages ceph,
  and ceph-brag client.
* ceph-osd: it packages ceph-disk, so should add ${python:Depends}
  as its dependencies.

dh_python2 will figure them out.

Signed-off-by: Kefu Chai <kchai@redhat.com>
we raise UnicodeDecodeError at seeing non-ascii args if we fail to match
it with any command signatures. instead, we should use a unicode string
for representing the error in that case. please note, the exception is
not printed at all in real-world. =)

Fixes: http://tracker.ceph.com/issues/12287
Signed-off-by: Kefu Chai <kchai@redhat.com>
as async compressor is not used anywhere. and haomai agrees to disable
this test.

Signed-off-by: Kefu Chai <kchai@redhat.com>
In current multisite scenarios,if a bucket is created in master, we end
up storing multipart metadata in `$source-zone.rgw.buckets.non-ec` pool
instead of the zone's own non-ec pool, so we end up additionally
creating this pool and storing multipart metadata entries in it. Also if
a bucket is created in a secondary zone, and we initiate a multipart
upload, before mdlog sync with master, we end up getting errors during
complete multipart requests as omap entries are partly stored in the
`$zone.rgw.buckets.non-ec` as well as `$source-zone.rgw.buckets.non-ec`
pools which leads to total number of parts mismatch.

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

Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>
Currently when there is no bucket in the cluster and user execute
 "$radosgw-admin user  stats --uid=testid" command then rgw admin
not throwing meaningful error message.

With this fix it will show proper meaningful error message.

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

Reported-by: Abhishek Lekshmanan <abhishek@suse.com>
Signed-off-by: Gaurav Kumar Garg <garg.gaurav52@gmail.com>
it was broken by 1e8388c

Signed-off-by: Kefu Chai <kchai@redhat.com>
The previous form, libatomic-ops-devel, has been deprecated since May 23, 2012.

Signed-off-by: Nathan Cutler <ncutler@suse.com>
Signed-off-by: Nathan Cutler <ncutler@suse.com>
Signed-off-by: Nathan Cutler <ncutler@suse.com>
Signed-off-by: Samuel Just <sjust@redhat.com>
Signed-off-by: Samuel Just <sjust@redhat.com>
Signed-off-by: Nathan Cutler <ncutler@suse.com>
@tchaikov tchaikov removed their assignment Jul 25, 2016
@tchaikov
Copy link
Contributor

@tone-zhang could you remove irrelevant commits from your PR?

@tone-zhang
Copy link
Contributor Author

@tchaikov Kefu, I'm really sorry for the mess. Because my working branch is much out of date from the master branch, after rebased from ceph/master, all the different commits are required to push to my working branch in order to fetch the delta.
Now I'm working on my new branch, and try to commit the build update only. So may I create another PR to replace this one? I will figure out the new one comes from this one.
Really sorry for the inconvenience.

@tone-zhang
Copy link
Contributor Author

@tchaikov Kefu, I have created another PR "#10427" in order to make the PR looks clean. Could you please have a review the new one?
I'm really sorry for the inconvenience.

@tchaikov
Copy link
Contributor

next time, probably you could "git push -f" to update this commit.

@tchaikov tchaikov closed this Jul 25, 2016
@tone-zhang
Copy link
Contributor Author

@tchaikov Kefu, I will pay more attention next time. Thanks a lot for your patience. :)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.