crushtool: set type 0 name "device" for --build option #6824
Conversation
|
Simple test steps: |
|
@liewegas Could you take a look at this minor crushtool fix, thank you! |
| @@ -617,6 +617,7 @@ int main(int argc, const char **argv) | |||
| crush.set_item_name(i, "osd." + stringify(i)); | |||
| } | |||
|
|
|||
| crush.set_type_name(0, "device"); | |||
There was a problem hiding this comment.
How about "osd"? That's what the default OSDMap simple generator uses, and this function is also assuming "osd" naming above.
SandyXSD
force-pushed
the
wip-crushtool-build
branch
from
ee1038c
to
5afec1e
Compare
|
@liewegas Sure, that's definitely a better name :D. I have also changed the default type 0 name in the decompile process, but I'm not sure if there needs any other cleanup work. |
The crushmap built by crushtool cannot be used directly as the name of type 0 is not specified. We may decompile the crushmap(it will automatically add type 0) and then recompile it to make it usable, but that's extra effort. Signed-off-by: Sangdi Xu <xu.sangdi@h3c.com>
Change the type 0 name from "device" to "osd", just to make it consistent Signed-off-by: Sangdi Xu <xu.sangdi@h3c.com>
SandyXSD
force-pushed
the
wip-crushtool-build
branch
from
5afec1e
to
4269263
Compare
|
Actually the second patch I'm not sure about. Right now the "osd" naming is only used in the build method, so this keeps us consistent there, but leaves everything else with a generic "device". For example the second patch breaks the cli unit tests that expect the old naming. Let's just do the first patch? |
|
@liewegas Hmm, sorry I made it a bit confusing, but in fact it's the first patch that fails the unit test. Currently the behavior is that type 0 name is not specified when built, and then set as "device" in the decompile stage. However, if it's already named as "osd", decompile won't change it. So now we probably have two options:
|
|
@liewegas I will discard the second patch, but let's decide on the solution to --build opition first so that the bot may have an easier life. Any suggestion about the fix? Thank you! |
|
Sorry for the slow reply. On second thought I think what you have here looks okay. Thanks! |
crushtool: set type 0 name "device" for --build option Reviewed-by: Sage Weil <sage@redhat.com>
The crushmap built by crushtool cannot be used directly as the name of type 0 is not specified.
We may decompile the crushmap(it will automatically add type 0) and then recompile it to make it usable, but that's extra effort.
Signed-off-by: Sangdi Xu xu.sangdi@h3c.com