rbd: inherit the parent image features when cloning an image #9334

yangdongsheng · 2016-05-26T02:19:25Z

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

yangdongsheng · 2016-05-26T02:19:39Z

trociny · 2016-05-26T10:27:50Z

@yangdongsheng Recently this fix to librbd::clone has been committed to master:

d305eaa

Could you please retest that it fixes this issue too? If it does not I believe we should fix this on API level, not CLI.

trociny · 2016-05-26T10:41:00Z

Actually, looking at it more closely, we still need modification in rbd, but I guess we could do it differently: instead of retrieving parent features we just could leave image options unset, relaying on API fuction (clone) to set it properly from parent. Does it make sense?

yangdongsheng · 2016-05-27T02:04:26Z

@trociny yea, good point. we should put this work into API function, otherwise, we have to do the same thing at every caller of this api. thanx, will update it soon.

trociny · 2016-05-27T05:11:43Z

@yangdongsheng Actually API functions should already work. I pointed you to the latest fix for clone. So I expect some work is only required to modify utils::get_image_options() to set only those options that are specified by the user.

yangdongsheng · 2016-06-01T07:47:36Z

Hi @trociny sorry for the late. yes, API side is almost ready, but there is something we should do for other options in clone. I have pushed some commits for them. Please help to check again. Thanx

trociny · 2016-06-01T09:10:06Z

I admit I have been only glancing the changes so far, but this additional logic for rbd utils::get_image_options looks alarming to me.

Ideally, I would like utils::get_image_options only set options specified by the user (so no fallback to rbd_default options from config here). And the logic how to set necessary missing options (either form config option or from parent) are performed internally. This way it will work the same both for users running CLI and API.

@yangdongsheng have you tried this? Do you see any reason why this wouldn't work?

@dillaman what do you think?

yangdongsheng · 2016-06-01T09:36:11Z

@trociny no I did not try it. But that sounds good to me. I will try it if @dillaman has no objection.

dillaman · 2016-06-01T12:16:16Z

I agree with @trociny

xinxinsh · 2016-06-03T03:43:44Z

should process --image-shared flag https://github.com/ceph/ceph/blob/master/src/tools/rbd/Utils.cc#L415

yangdongsheng · 2016-06-03T05:00:25Z

@xinxinsh Yes, good, thanx.

yangdongsheng · 2016-06-03T05:20:33Z

@trociny what about this version?

ErwanAliasr1 · 2016-06-03T09:12:36Z

The job was reported failed beause of my work on the CI.
I respawned the job and it's green : https://jenkins.ceph.com/job/ceph-pull-requests/6688/

dillaman · 2016-06-03T13:24:30Z

src/tools/rbd/Utils.cc

-      return -EINVAL;
-    }
-    features &= ~RBD_FEATURE_STRIPINGV2;
+    stripe_specified = true;
  }

  if (vm.count(at::IMAGE_SHARED) && vm[at::IMAGE_SHARED].as<bool>()) {
    features &= ~RBD_FEATURES_SINGLE_CLIENT;


The features haven't potentially been initialized to the default feature set at this time if !features_specified

yangdongsheng · 2016-07-15T09:10:10Z

@dillaman Updated. thanx

dillaman · 2016-07-18T20:41:45Z

Hopefully the last of the test failures:
http://qa-proxy.ceph.com/teuthology/jdillaman-2016-07-18_07:58:43-rbd-wip-jd-testing---basic-smithi/320578/teuthology.log

yangdongsheng · 2016-07-20T01:52:34Z

@dillaman Oh, sorry about it. It seems I have to setup a local integration testing environment for a fully testing now. Thanx for your help.

yangdongsheng · 2016-07-22T03:09:06Z

@dillaman I found I can't reproduce the fail by run CEPH_CLI_TEST_DUP_COMMAND=1 CEPH_REF=001c65727312bac894ce6f7dc6159c962aaaa2a3 TESTDIR="/tmp/cephtest" CEPH_ARGS="--cluster ceph" CEPH_ID="0" ceph-coverage /tmp/coverage timeout 3h ../qa/workunits/rbd/journal.sh in my local ceph environment. Is that possible this failure is not related with this PR?

@yuriw could you help to take this PR into your testing? thanx.

trociny · 2016-07-22T04:40:37Z

@yangdongsheng You don't need to set CEPH_* env variables, just run:

../qa/workunits/rbd/journal.sh

But make sure your PATH is set correctly and the script runs your binary and lib:

which rbd
ldd `which rbd` |grep rbd

Also you can try to reproduce manually, basically it failed on this:

# create an image without journaling feature  
rbd create --size 256 ${src} 
# copy the image setting journaling features
rbd copy --image-feature exclusive-lock --image-feature journaling \
    --journal-pool rbd \
    --journal-object-size 20M \
    --journal-splay-width 6 \
    ${src} ${image}
# check that journaling settings are applied
rbd journal info --image ${image}

From teuthology.log it failed reporting that journaling feature was not enabled for the image. Does it work for you?

Another alarming message in teuthology.log: "rbd: image format 1 is deprecated" after rbd create ... commands. It is not expected.

yangdongsheng · 2016-07-22T05:42:01Z

[root@workstation build]# rbd create --size 256 test
2016-07-22 01:27:13.659840 7fc76127a5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
2016-07-22 01:27:13.660008 7fc76127a5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
2016-07-22 01:27:13.661730 7fc76127a5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
[root@workstation build]# rbd copy --image-feature exclusive-lock --image-feature journaling     --journal-pool rbd     --journal-object-size 20M     --journal-splay-width 6     test copy
2016-07-22 01:27:22.501427 7f713d69b5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
2016-07-22 01:27:22.501591 7f713d69b5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
2016-07-22 01:27:22.503236 7f713d69b5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
Image copy: 100% complete...done.
[root@workstation build]# rbd journal info --image copy
2016-07-22 01:27:28.490416 7f3518cad5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
2016-07-22 01:27:28.490573 7f3518cad5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
2016-07-22 01:27:28.491536 7f3518cad5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
rbd journal '10212ae8944a':
        header_oid: journal.10212ae8944a
        object_oid_prefix: journal_data.0.10212ae8944a.
        order: 25 (32768 kB objects)
        splay_width: 6
        object_pool: rbd
[root@workstation build]# rbd info copy
2016-07-22 01:27:46.543855 7f0b1759d5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
2016-07-22 01:27:46.544664 7f0b1759d5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
2016-07-22 01:27:46.545525 7f0b1759d5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
rbd image 'copy':
        size 256 MB in 64 objects
        order 22 (4096 kB objects)
        block_name_prefix: rbd_data.10212ae8944a
        format: 2
        features: exclusive-lock, journaling
        flags: 
        journal: 10212ae8944a
        mirroring state: disabled
[root@workstation build]# which rbd
/data/ceph/build/bin/rbd
[root@workstation build]# ldd `which rbd` |grep rbd
        librbd.so.1 => /data/ceph/build/lib/librbd.so.1 (0x00007fb94f5c0000)
[root@workstation build]# rbd --version
ceph version v10.2.0-2599-g74a8078 (74a8078a0f72f619642bf6698a8c9f709e940107)

The 74a8078 is the commit ID of this branch.

Thanx @trociny I manually tested my patch as above. is that same to you?

In addition, I tried the xml format.

[root@workstation build]# rbd --format xml journal info --image copy
2016-07-22 01:39:51.267635 7f18f21a55c0 -1 WARNING: the following dangerous and experimental features are enabled: *
2016-07-22 01:39:51.267792 7f18f21a55c0 -1 WARNING: the following dangerous and experimental features are enabled: *
2016-07-22 01:39:51.268863 7f18f21a55c0 -1 WARNING: the following dangerous and experimental features are enabled: *
<journal><journal_id>10212ae8944a</journal_id><header_oid>journal.10212ae8944a</header_oid><object_oid_prefix>journal_data.0.10212ae8944a.</object_oid_prefix><order>25</order><splay_width>6</splay_width><object_pool>rbd</object_pool></journal>[root@workstation build]#

yangdongsheng · 2016-07-22T05:53:47Z

Oh, so we should create test in format 1 what might be the default in this case. I see the problem now.

I guess the rbd_default_format is 1 in the test case

trociny · 2016-07-22T06:52:18Z

@yangdongsheng Actually, I don't see rbd_default_format is explicitly set in that test, and there many other tests from that teuthology run where this warning was observed. And this is the main question: why it uses old format here as default? Don't you observe this in your local environment (don't you have rbd_default_format = 2 explicitly set in your config)?

I have not built your branch yet, so can't recheck this in my env.

yangdongsheng · 2016-07-22T07:21:26Z

@trociny I tried my local env again. I did not observe the deprecating warning in creating.

[root@workstation build]# ldd `which rbd`|grep rbd
        librbd.so.1 => /data/ceph/build/lib/librbd.so.1 (0x00007fcf29338000)
[root@workstation build]# which rbd
/data/ceph/build/bin/rbd
[root@workstation build]# rbd --version
ceph version v10.2.0-2599-g74a8078 (74a8078a0f72f619642bf6698a8c9f709e940107)
[root@workstation build]# rbd create --size 1G test1
2016-07-22 03:17:29.375010 7f660fb9c5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
2016-07-22 03:17:29.375183 7f660fb9c5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
2016-07-22 03:17:29.376296 7f660fb9c5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
[root@workstation build]# rbd info test1
2016-07-22 03:17:33.516528 7ff98ac0f5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
2016-07-22 03:17:33.516821 7ff98ac0f5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
2016-07-22 03:17:33.517604 7ff98ac0f5c0 -1 WARNING: the following dangerous and experimental features are enabled: *
rbd image 'test1':
        size 1024 MB in 256 objects
        order 22 (4096 kB objects)
        block_name_prefix: rbd_data.101f2ae8944a
        format: 2
        features: layering, exclusive-lock, object-map, fast-diff, deep-flatten
        flags: 
[root@workstation build]# ceph daemon mon.a config show|grep rbd_default
    "rbd_default_format": "2",
    "rbd_default_order": "22",
    "rbd_default_stripe_count": "0",
    "rbd_default_stripe_unit": "0",
    "rbd_default_features": "61",
    "rbd_default_map_options": "",

…s(). Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

… options specified Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

…options specified Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

…overwriting. Currently, if we want to use the default options for rbd, we need to omit the RBD_IMAGE_OPTION_FEATURES, but if we want --image-shared. we need to overwrite something bese on the default value of image options. This patch introduce two flags in image_options, RBD_IMAGE_OPTION_FEATURES_SET means, we want to set something after you get the features from default, parent or user. And RBD_IMAGE_OPTION_FEATURES_CLEAR will do a bit clear. Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

…ecified. Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

… creating Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

… cloning Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

… copying Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

yangdongsheng · 2016-07-22T11:38:30Z

Okey, I found the problem now, there is an uninitialized variable of format. when getting format in creating.

yangdongsheng · 2016-07-26T11:02:36Z

@trociny or @dillaman I hope I have fixed the problem above, could you help to queue this PR for integration testing again? thanx a lot.

dillaman added feature rbd labels May 26, 2016

dillaman changed the title ~~tools/rbd: Inherit the Parent Image properties while Cloning a rbd Image~~ rbd: inherit the parent image features when cloning an image May 27, 2016

yangdongsheng force-pushed the rbd_clone_hiberit branch from d570a04 to 368fa3b Compare June 1, 2016 07:44

yangdongsheng force-pushed the rbd_clone_hiberit branch 7 times, most recently from 4e3b6f2 to 184c9fb Compare June 2, 2016 23:58

zhouyuan mentioned this pull request Jun 3, 2016

librbd: Inherit the Parent Image properties while Cloning a rbd Image #9473

Closed

yangdongsheng force-pushed the rbd_clone_hiberit branch from 184c9fb to 330d392 Compare June 3, 2016 05:01

dillaman reviewed Jun 3, 2016
View reviewed changes

dillaman added the wip-jason-testing label Jul 12, 2016

yangdongsheng force-pushed the rbd_clone_hiberit branch from cff5acc to 74a8078 Compare July 15, 2016 09:01

dillaman removed the wip-jason-testing label Jul 18, 2016

yangdongsheng added 13 commits July 22, 2016 07:04

tools/rbd: only set the options specified by user in get_image_option…

a9db199

…s(). Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

librbd: set the default options when they are not specified.

6f5571f

Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

librbd/clone: set RBD_FEATURE_STRIPINGV2 in feature if stripe related…

d97fe01

… options specified Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

librbd/copy: set RBD_FEATURE_STRIPINGV2 in feature if stripe related …

dd299e0

…options specified Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

tools/rbd: set RBD_IMAGE_OPTION_FEATURES_CLEAR when --image-shared sp…

3508f40

…ecified. Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

pybind: don't set the default values for the options not specified in…

c37df5e

… creating Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

pybind: don't set the default values for the options not specified in…

4942076

… cloning Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

pybind: don't set the default values for the options not specified in…

98251ea

… copying Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

librbd: set RBD_FEATURE_STRIPINGV2 in create if needed.

df81ba1

Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

librbd: check the RBD_FEATURE_LAYERING for the features of child image.

cfae775

Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

tools/rbd: set order to default value if it is not specified in options.

a187b34

Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

tools/rbd: use create4() for both of format 1 and format 2.

6264acb

Signed-off-by: Dongsheng Yang <dongsheng.yang@easystack.cn>

yangdongsheng force-pushed the rbd_clone_hiberit branch from 74a8078 to 6264acb Compare July 22, 2016 11:05

dillaman added the wip-jason-testing label Jul 26, 2016

dillaman merged commit bdeef71 into ceph:master Jul 27, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rbd: inherit the parent image features when cloning an image #9334

rbd: inherit the parent image features when cloning an image #9334

yangdongsheng commented May 26, 2016

yangdongsheng commented May 26, 2016

trociny commented May 26, 2016 •

edited

trociny commented May 26, 2016

yangdongsheng commented May 27, 2016

trociny commented May 27, 2016

yangdongsheng commented Jun 1, 2016

trociny commented Jun 1, 2016

yangdongsheng commented Jun 1, 2016

dillaman commented Jun 1, 2016

xinxinsh commented Jun 3, 2016

yangdongsheng commented Jun 3, 2016

yangdongsheng commented Jun 3, 2016

ErwanAliasr1 commented Jun 3, 2016

dillaman Jun 3, 2016

yangdongsheng commented Jul 15, 2016

dillaman commented Jul 18, 2016 •

edited

yangdongsheng commented Jul 20, 2016

yangdongsheng commented Jul 22, 2016

trociny commented Jul 22, 2016

yangdongsheng commented Jul 22, 2016 •

edited

yangdongsheng commented Jul 22, 2016 •

edited

trociny commented Jul 22, 2016

yangdongsheng commented Jul 22, 2016

yangdongsheng commented Jul 22, 2016

yangdongsheng commented Jul 26, 2016

rbd: inherit the parent image features when cloning an image #9334

rbd: inherit the parent image features when cloning an image #9334

Conversation

yangdongsheng commented May 26, 2016

yangdongsheng commented May 26, 2016

trociny commented May 26, 2016 • edited

trociny commented May 26, 2016

yangdongsheng commented May 27, 2016

trociny commented May 27, 2016

yangdongsheng commented Jun 1, 2016

trociny commented Jun 1, 2016

yangdongsheng commented Jun 1, 2016

dillaman commented Jun 1, 2016

xinxinsh commented Jun 3, 2016

yangdongsheng commented Jun 3, 2016

yangdongsheng commented Jun 3, 2016

ErwanAliasr1 commented Jun 3, 2016

dillaman Jun 3, 2016

Choose a reason for hiding this comment

yangdongsheng commented Jul 15, 2016

dillaman commented Jul 18, 2016 • edited

yangdongsheng commented Jul 20, 2016

yangdongsheng commented Jul 22, 2016

trociny commented Jul 22, 2016

yangdongsheng commented Jul 22, 2016 • edited

yangdongsheng commented Jul 22, 2016 • edited

trociny commented Jul 22, 2016

yangdongsheng commented Jul 22, 2016

yangdongsheng commented Jul 22, 2016

yangdongsheng commented Jul 26, 2016

trociny commented May 26, 2016 •

edited

dillaman commented Jul 18, 2016 •

edited

yangdongsheng commented Jul 22, 2016 •

edited

yangdongsheng commented Jul 22, 2016 •

edited