Re: [PATCH] Forbid merging with partially assembled IMSM array

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 4046 bytes --]

On Fri, 30 May 2014 13:11:43 +0000 "Baldysiak, Pawel"
<pawel.baldysiak@intel.com> wrote:

> Changes introduced in commit:
> 0431869cec4c673309d9aa30a2df4b778bc0bd24
> enabled adding devices to partially assembled array.
> It causes assemble process to override checking controller of device.
> 
> For example:
> If ones created IMSM array will be stopped, and some disks will be
> attached to different controller, mdadm will allow to fully assemble this
> array as one md device (due to marge one part to another).
> 
> This patch resolve this problem by forbidding merge operation
> on arrays with IMSM metadata.

Why do you think this is a good thing?
You seem to be saying "You cannot access that data  because I don't like
which controllers you have attached it to".

My thought is that you should provide access to data whenever possible.

Is there some important situation where the current behaviour will cause real
problems?

Maybe a warning might be appropriate, but failure doesn't seem sensible...

NeilBrown

> 
> Signed-off-by: Pawel Baldysiak <pawel.baldysiak@intel.com>
> Reviewed-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com>
> 
> ---
> Assemble.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
> 
> diff --git a/Assemble.c b/Assemble.c
> index a57d384..9fb65e5 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -1329,6 +1329,29 @@ try_again:
>                                                return 1;
>                                }
>                                for (dv = pre_exist->devs; dv; dv = dv->next) {
> +                                             if (strncmp(mp->metadata, "imsm", 4) == 0 && !c->force) {
> +                                                             int dfd;
> +                                                             char dn[20];
> +                                                             struct supertype *st2;
> +                                                             sprintf(dn, "%d:%d", dv->disk.major,
> +                                                                                             dv->disk.minor);
> +                                                             st2 = dup_super(st);
> +                                                             dfd = dev_open(dn, O_RDONLY);
> +                                                             if ((dfd > 0) && (st2->ss->load_super(st2, dfd, NULL) ||
> +                                                                  (st->ss->compare_super(st, st2) != 0))) {
> +                                                                             pr_err("IMSM metadata mismatch!\n");
> +                                                                             pr_err("Aborting...\n");
> +                                                                             close(dfd);
> +                                                             } else {
> +                                                                             pr_err("IMSM Array already partially assembled!\n");
> +                                                                             pr_err("Aborting...\n");
> +                                                             }
> +                                                             if (st2) {
> +                                                                             st2->ss->free_super(st2);
> +                                                                             free(st2);
> +                                                             }
> +                                                             return 1;
> +                                             }
>                                                /* We want to add this device to our list,
>                                                 * but it could already be there if "mdadm -I"
>                                                 * started *after* we checked for O_EXCL.
> --
> 1.9.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]