mdraid: Add 'bitmap' option to MDRaidCreate method #1124
Conversation
|
Can one of the admins verify this patch? |
| The @bitmap option can have the same values as are accepted by | ||
| SetBitmapLocation. |
There was a problem hiding this comment.
Could you please explicitly state the values accepted, for the sake of completeness? I.e. The value for the bitmap, currently only the values 'none' and 'internal' are supported.
There was a problem hiding this comment.
Also, looking at the mdadm manpage, in case bd_md_create(bitmap=False), libblockdev doesn't add any --bitmap= option, leaving the logic on automatic mdadm rule:
When creating an array on devices which are 100G or larger, mdadm automatically adds an internal bitmap as it will usually be beneficial. This can be suppressed with --bitmap=none or by selecting a different consistency policy with --consistency-policy.
So either the values in UDisks should read 'auto' and 'internal', or we should change bd_md_create() to accept values of 'none', 'auto' and 'internal'. Intentionally omitting 'clustered' and filename as those we don't want to support.
There was a problem hiding this comment.
When creating an array on devices which are 100G or larger, mdadm automatically adds an internal bitmap as it will usually be beneficial. This can be suppressed with --bitmap=none or by selecting a different consistency policy with --consistency-policy.
Oh, interesting, let's bring in the experts.
@mauelsha, from our discussion in Brno I had the impression that mdadm will never use a bitmap but that it is always a good idea to use one. Now the man page seems to say that a bitmap is not always a good idea, and that mdadm does in fact decides to use a bitmap exactly when it is a good idea.
Can you clarify?
Right now I am thinking that we don't need to change anything, after all. New mdraid devices will have a bitmap or not as decided by the mdraid maintainers, and Cockpit allows it to be adjusted in case the maintainers are wrong in certain situations.
|
Jenkins, ok to test. |
Please don't hold the release because of this. |
| @@ -765,7 +766,16 @@ handle_mdraid_create (UDisksManager *_object, | |||
| } | |||
| disks[disks_top] = NULL; | |||
|
|
|||
| if (!bd_md_create (array_name, arg_level, disks, 0, NULL, FALSE, arg_chunk, NULL, &error)) | |||
| g_variant_lookup (arg_options, "bitmap", "^&ay", &option_bitmap); | |||
| if (!(g_strcmp0 (option_bitmap, "none") == 0 || g_strcmp0 (option_bitmap, "internal") == 0)) | |||
There was a problem hiding this comment.
One more thing, revealed by the failed tests - in case of a NULL value this check should be skipped.
|
I've addressed this in #1127 to speed things up. |
No description provided.