Hi Neil,
NeilBrown wrote:
On Mon, 6 Jul 2015 16:52:12 +0800 Guoqing Jiang <gqjiang@xxxxxxxx>
wrote:
Modifying an exiting device's superblock or creating a new superblock
on an existing device needs to be checked because the device could be
in use by another node in another array. So, we check this by taking
all superblock locks in userspace so that we don't step onto an active
device used by another node and safeguard against accidental edits.
After the edit is complete, we release all locks and the lockspace so
that it can be used by the kernel space.
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx>
Signed-off-by: Guoqing Jiang <gqjiang@xxxxxxxx>
Hi,
thanks for these.
I've applied 1 and 2.
Could you re-do this on to follow the kernel style of #ifdefs,
so that the #ifdefs only appear in header files.
i.e. when NO_DLM is defined, is_clustered() always evaluates to zero
and clustered_get_dlmlock() always succeeds etc.
Thanks, I guess you mean the following incremental modification. Please
let me know if I misunderstood.
diff --git a/mdadm.h b/mdadm.h
index 59f851e..4d9e8c8 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -1429,9 +1429,26 @@ extern char *fd2devnm(int fd);
extern int in_initrd(void);
extern int get_cluster_name(char **name);
+#ifndef NO_DLM
extern int is_clustered(struct supertype *st);
extern int cluster_get_dlmlock(struct supertype *st, int *lockid);
extern int cluster_release_dlmlock(struct supertype *st, int lockid);
+#else
+static inline int is_clustered(struct supertype *st)
+{
+ return 0;
+}
+
+static inline int cluster_get_dlmlock(struct supertype *st, int *lockid)
+{
+ return 0;
+}
+
+static inline int cluster_release_dlmlock(struct supertype *st, int lockid)
+{
+ return 0;
+}
+#endif
#define _ROUND_UP(val, base) (((val) + (base) - 1) & ~(base - 1))
#define ROUND_UP(val, base) _ROUND_UP(val, (typeof(val))(base))
diff --git a/super1.c b/super1.c
index c49a3b3..bd88c36 100644
--- a/super1.c
+++ b/super1.c
@@ -1072,10 +1072,9 @@ static int update_super1(struct supertype *st,
struct mdinfo *info,
* ignored.
*/
int rv = 0;
+ int lockid;
struct mdp_superblock_1 *sb = st->sb;
-#ifndef NO_DLM
- int lockid;
if (is_clustered(st)) {
rv = cluster_get_dlmlock(st, &lockid);
if (rv) {
@@ -1084,7 +1083,6 @@ static int update_super1(struct supertype *st,
struct mdinfo *info,
return rv;
}
}
-#endif
if (strcmp(update, "homehost") == 0 &&
homehost) {
@@ -1342,10 +1340,9 @@ static int update_super1(struct supertype *st,
struct mdinfo *info,
rv = -1;
sb->sb_csum = calc_sb_1_csum(sb);
-#ifndef NO_DLM
if (is_clustered(st))
cluster_release_dlmlock(st, lockid);
-#endif
+
return rv;
}
@@ -1449,9 +1446,8 @@ static int add_to_super1(struct supertype *st,
mdu_disk_info_t *dk,
struct mdp_superblock_1 *sb = st->sb;
__u16 *rp = sb->dev_roles + dk->number;
struct devinfo *di, **dip;
-
-#ifndef NO_DLM
int rv, lockid;
+
if (is_clustered(st)) {
rv = cluster_get_dlmlock(st, &lockid);
if (rv) {
@@ -1460,7 +1456,7 @@ static int add_to_super1(struct supertype *st,
mdu_disk_info_t *dk,
return rv;
}
}
-#endif
+
if ((dk->state & 6) == 6) /* active, sync */
*rp = __cpu_to_le16(dk->raid_disk);
else if ((dk->state & ~2) == 0) /* active or idle -> spare */
@@ -1487,10 +1483,9 @@ static int add_to_super1(struct supertype *st,
mdu_disk_info_t *dk,
di->next = NULL;
*dip = di;
-#ifndef NO_DLM
if (is_clustered(st))
cluster_release_dlmlock(st, lockid);
-#endif
+
return 0;
}
#endif
@@ -1504,9 +1499,8 @@ static int store_super1(struct supertype *st, int fd)
struct align_fd afd;
int sbsize;
unsigned long long dsize;
-
-#ifndef NO_DLM
int rv, lockid;
+
if (is_clustered(st)) {
rv = cluster_get_dlmlock(st, &lockid);
if (rv) {
@@ -1515,7 +1509,6 @@ static int store_super1(struct supertype *st, int fd)
return rv;
}
}
-#endif
if (!get_dev_size(fd, NULL, &dsize))
return 1;
@@ -1576,10 +1569,9 @@ static int store_super1(struct supertype *st, int fd)
}
}
fsync(fd);
-#ifndef NO_DLM
if (is_clustered(st))
cluster_release_dlmlock(st, lockid);
-#endif
+
return 0;
}
@@ -2329,7 +2321,6 @@ static int write_bitmap1(struct supertype *st, int
fd, enum bitmap_update update
static void free_super1(struct supertype *st)
{
-#ifndef NO_DLM
int rv, lockid;
if (is_clustered(st)) {
rv = cluster_get_dlmlock(st, &lockid);
@@ -2339,7 +2330,7 @@ static void free_super1(struct supertype *st)
return;
}
}
-#endif
+
if (st->sb)
free(st->sb);
while (st->info) {
@@ -2350,10 +2341,8 @@ static void free_super1(struct supertype *st)
free(di);
}
st->sb = NULL;
-#ifndef NO_DLM
if (is_clustered(st))
cluster_release_dlmlock(st, lockid);
-#endif
}
#ifndef MDASSEMBLE
Thanks,
Guoqing
