linux-raid.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 21/33] Monitor: link containers and volumes in statelist
       [not found] <A9DE54D0CD747C4CB06DCE5B6FA2246FDA893C42@irsmsx504.ger.corp.intel.com>
@ 2010-07-05 10:48 ` Hawrylewicz Czarnowski, Przemyslaw
  2010-07-06  7:11 ` Neil Brown
  1 sibling, 0 replies; 2+ messages in thread
From: Hawrylewicz Czarnowski, Przemyslaw @ 2010-07-05 10:48 UTC (permalink / raw)
  To: linux-raid@vger.kernel.org

From: Czarnowska, Anna 
Sent: Monday, July 05, 2010 11:35 AM
To: Neil Brown
Cc: linux-raid@vger.kernel.org; Czarnowska, Anna; Hawrylewicz Czarnowski, Przemyslaw; Labun, Marcin; Neubauer, Wojciech; Williams, Dan J; Ciechanowski, Ed; dledford@redhat.com
Subject: [PATCH 21/33] Monitor: link containers and volumes in statelist

To avoid repeated retrieving of parent container or volumes list.
Simplifies finding unused disk in a container.
Function check_disk_is_free will be used for spare sharing.

Signed-off-by: Marcin Labun <marcin.labun@intel.com>
---
 Monitor.c |  123 ++++++++++++++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 102 insertions(+), 21 deletions(-)

diff --git a/Monitor.c b/Monitor.c
index 8e82797..e16af2d 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -38,6 +38,24 @@ static void alert(char *event, char *dev, char *disc, char *mailaddr, char *mail
  * At least it isn't MD_SB_DISKS.
  */
 #define MaxDisks 384
+struct state {
+     char *devname;
+     int devnum; /* to sync with mdstat info */
+     long utime;
+     int err;
+     char *spare_group;
+     int active, working, failed, spare, raid, total;
+     int expected_spares;
+     int devstate[MaxDisks];
+     int devid[MaxDisks];
+     int percent;
+     char *metadata_version;
+     struct state *next;
+     struct state *volumes;
+     struct state *parent;
+     struct state *missing;
+};
+
 int Monitor(mddev_dev_t devlist,
          char *mailaddr, char *alert_cmd,
          int period, int daemonise, int scan, int oneshot, @@ -85,22 +103,11 @@ int Monitor(mddev_dev_t devlist,
       * that appears in /proc/mdstat
       */
 
-     struct state {
-           char *devname;
-           int devnum; /* to sync with mdstat info */
-           long utime;
-           int err;
-           char *spare_group;
-           int active, working, failed, spare, raid;
-           int expected_spares;
-           int devstate[MaxDisks];
-           int devid[MaxDisks];
-           int percent;
-           struct state *next;
-     } *statelist = NULL;
      int finished = 0;
      struct mdstat_ent *mdstat = NULL;
      char *mailfrom = NULL;
+     struct state *statelist = NULL;
+     struct state *missing_parent_list = NULL;
 
      if (!mailaddr) {
            mailaddr = conf_get_mailaddr();
@@ -173,6 +180,11 @@ int Monitor(mddev_dev_t devlist,
                  st->devnum = INT_MAX;
                  st->percent = -2;
                  st->expected_spares = mdlist->spare_disks;
+                 st->metadata_version = NULL;
+                 st->missing = NULL;
+                 st->parent = NULL;
+                 st->volumes = NULL;
+                 st->total = 0;
                  if (mdlist->spare_group)
                        st->spare_group = strdup(mdlist->spare_group);
                  else
@@ -194,6 +206,11 @@ int Monitor(mddev_dev_t devlist,
                  st->percent = -2;
                  st->expected_spares = -1;
                  st->spare_group = NULL;
+                 st->metadata_version = NULL;
+                 st->missing = NULL;
+                 st->parent = NULL;
+                 st->volumes = NULL;
+                 st->total = 0;
                  if (mdlist) {
                        st->expected_spares = mdlist->spare_disks;
                        if (mdlist->spare_group)
@@ -281,11 +298,9 @@ int Monitor(mddev_dev_t devlist,
                      st->spare == array.spare_disks &&
                      (mse == NULL  || (
                            mse->percent == st->percent
-                           ))) {
-                       close(fd);
+                           )))
                        st->err = 0;
-                       continue;
-                 }
+
                  if (st->utime == 0 && /* new array */
                      mse &&  /* is in /proc/mdstat */
                      mse->pattern && strchr(mse->pattern, '_') /* degraded */ @@ -407,17 +422,53 @@ int Monitor(mddev_dev_t devlist,
                  st->failed = array.failed_disks;
                  st->utime = array.utime;
                  st->raid = array.raid_disks;
+                 st->total = array.raid_disks + array.nr_disks;
                  st->err = 0;
+                 if (mse && (mse->metadata_version))  {
+                       st->metadata_version = strdup(mse->metadata_version);
+                       st->parent = NULL;
+                       st->volumes = NULL;
+                       if (is_external(st->metadata_version) &&
+                       is_subarray(st->metadata_version+9)) {
+                             if (missing_parent_list)
+                                   st->missing = missing_parent_list;
+                             else
+                                   st->missing = NULL;
+                             missing_parent_list = st;
+                       }
+                 }
+           }
+           /* create parent list */
+           while (missing_parent_list) {
+                 struct state *last = NULL, *st2 = NULL;
+                 struct state *st;
+                 st = missing_parent_list;
+                 missing_parent_list = missing_parent_list->missing;
+                 st->missing = NULL;
+
+                 for (st2 = statelist; st2; st2 = st2->next) {
+                       if ((st2->parent == NULL) &&
+                       (st2->metadata_version) &&
+                       (st2->devnum != INT_MAX) &&
+                       (devname2devnum(st->metadata_version+10)
+                                   == st2->devnum)) {
+                             st->parent = st2;
+                             last = st2;
+                             while ((st2 = st2->volumes) != NULL)
+                                   last = st2;
+                             last->volumes = st;
+                             break;
+                       }
+                 }
            }
            /* now check if there are any new devices found in mdstat */
            if (scan) {
                  struct mdstat_ent *mse;
                  for (mse=mdstat; mse; mse=mse->next)
                        if (mse->devnum != INT_MAX &&
-                           mse->level &&
-                           (strcmp(mse->level, "raid0")!=0 &&
-                            strcmp(mse->level, "linear")!=0)
-                             ) {
+                       (!mse->level  || /* retrieve containers */
+                       (strcmp(mse->level, "raid0") != 0 &&
+                       strcmp(mse->level, "linear") != 0))) {
                              struct state *st = malloc(sizeof *st);
                              mdu_array_info_t array;
                              int fd;
@@ -430,6 +481,8 @@ int Monitor(mddev_dev_t devlist,
                                    if (fd >=0) close(fd);
                                    put_md_name(st->devname);
                                    free(st->devname);
+                                   if (st->metadata_version)
+                                         free(st->metadata_version);
                                    free(st);
                                    continue;
                              }
@@ -440,6 +493,11 @@ int Monitor(mddev_dev_t devlist,
                              st->devnum = mse->devnum;
                              st->percent = -2;
                              st->spare_group = NULL;
+                             st->metadata_version = NULL;
+                             st->missing = NULL;
+                             st->parent = NULL;
+                             st->volumes = NULL;
+                             st->total = 0;
                              st->expected_spares = -1;
                              statelist = st;
                              if (test)
@@ -519,6 +577,29 @@ int Monitor(mddev_dev_t devlist,
      return 0;
 }
 
+/* check if disk is used in donor array (native) or any volume in donor 
+container (external)*/ int check_disk_is_free(struct state *donor, int 
+disk_idx, int ext) {
+     struct state *vol = NULL;
+     int vol_disk;
+
+     if (!donor || disk_idx < 0 || donor->devid[disk_idx] == 0)
+           return INT_MAX;
+
+     /* native */
+     if (!ext)
+           return (donor->devid[disk_idx] > 0 && donor->devstate[disk_idx] == 0) 
+? disk_idx : INT_MAX;
+
+     /* external */
+     if (!donor->volumes)
+           return disk_idx;
+     for (vol = donor->volumes; vol; vol = vol->volumes)
+           for (vol_disk = 0; vol_disk < vol->total; vol_disk++)
+                 if (donor->devid[disk_idx] == vol->devid[vol_disk])
+                       return (vol->devstate[vol_disk]  == 0) ? disk_idx : INT_MAX;
+
+     return disk_idx;
+}
 
 static void alert(char *event, char *dev, char *disc, char *mailaddr, char *mailfrom, char *cmd,
              int dosyslog)
--
1.6.4.2


--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH 21/33] Monitor: link containers and volumes in statelist
       [not found] <A9DE54D0CD747C4CB06DCE5B6FA2246FDA893C42@irsmsx504.ger.corp.intel.com>
  2010-07-05 10:48 ` [PATCH 21/33] Monitor: link containers and volumes in statelist Hawrylewicz Czarnowski, Przemyslaw
@ 2010-07-06  7:11 ` Neil Brown
  1 sibling, 0 replies; 2+ messages in thread
From: Neil Brown @ 2010-07-06  7:11 UTC (permalink / raw)
  To: Czarnowska, Anna
  Cc: linux-raid@vger.kernel.org, Hawrylewicz Czarnowski, Przemyslaw,
	Labun, Marcin, Neubauer, Wojciech, Williams, Dan J,
	Ciechanowski, Ed, dledford@redhat.com

On Mon, 5 Jul 2010 10:35:26 +0100
"Czarnowska, Anna" <anna.czarnowska@intel.com> wrote:

> To avoid repeated retrieving of parent container or volumes list.
> 
> Simplifies finding unused disk in a container.
> 
> Function check_disk_is_free will be used for spare sharing.
> 
> 
> 
> Signed-off-by: Marcin Labun <marcin.labun@intel.com<mailto:marcin.labun@intel.com>>
> 
> ---
> 
>  Monitor.c |  123 ++++++++++++++++++++++++++++++++++++++++++++++++++----------
> 
>  1 files changed, 102 insertions(+), 21 deletions(-)
> 
> 
> 
> diff --git a/Monitor.c b/Monitor.c
> 
> index 8e82797..e16af2d 100644
> 
> --- a/Monitor.c
> 
> +++ b/Monitor.c
> 
> @@ -38,6 +38,24 @@ static void alert(char *event, char *dev, char *disc, char *mailaddr, char *mail
> 
>   * At least it isn't MD_SB_DISKS.
> 
>   */
> 
>  #define MaxDisks 384
> 
> +struct state {
> 
> +     char *devname;
> 
> +     int devnum; /* to sync with mdstat info */
> 
> +     long utime;
> 
> +     int err;
> 
> +     char *spare_group;
> 
> +     int active, working, failed, spare, raid, total;
> 
> +     int expected_spares;
> 
> +     int devstate[MaxDisks];
> 
> +     int devid[MaxDisks];
> 
> +     int percent;
> 
> +     char *metadata_version;
> 
> +     struct state *next;
> 
> +     struct state *volumes;
> 
> +     struct state *parent;
> 
> +     struct state *missing;

I'm having trouble figuring out what next/volumes/parent/missing are meant to
mean exactly, and there is a lack of any comments explaining it.
I suspect that is probably makes sense, but I need some help understanding
exactly what you are doing here....


> > @@ -281,11 +298,9 @@ int Monitor(mddev_dev_t devlist,
> 
>                       st->spare == array.spare_disks &&
> 
>                       (mse == NULL  || (
> 
>                             mse->percent == st->percent
> 
> -                           ))) {
> 
> -                       close(fd);
> 
> +                           )))
> 
>                         st->err = 0;
> 
> -                       continue;
> 
> -                 }
> 


That if statement no just sets st->err=0, which happens again a few lines
later, so it seems the if statement is now pointless....
But it was there for a reason - I think to short-circuit a collections of
tests that won't be needed if the 'if' succeeds.  Maybe you could explain
why you want to remove the 'if' now.



> 
> +                             if (missing_parent_list)
> 
> +                                   st->missing = missing_parent_list;
> 
> +                             else
> 
> +                                   st->missing = NULL;

This 'if' is exactly equivalent to
      st->missing = missing_parent_list;
!


>                   struct mdstat_ent *mse;
> 
>                   for (mse=mdstat; mse; mse=mse->next)
> 
>                         if (mse->devnum != INT_MAX &&
> 
> -                           mse->level &&
> 
> -                           (strcmp(mse->level, "raid0")!=0 &&
> 
> -                            strcmp(mse->level, "linear")!=0)
> 
> -                             ) {
> 
> +                       (!mse->level  || /* retrieve containers */
> 
> +                       (strcmp(mse->level, "raid0") != 0 &&
> 
> +                       strcmp(mse->level, "linear") != 0))) {

You are messing up the indenting rather badly.  Please preserve the
formatting style of the original code.

> 
> +                                   if (st->metadata_version)
> 
> +                                         free(st->metadata_version);


You don't need the test, as free(NULL) is defined to do noting.


> 
> 
> 
> +/* check if disk is used in donor array (native) or any volume in donor
> 
> +container (external)*/ int check_disk_is_free(struct state *donor, int
> 
> +disk_idx, int ext) {

Some how the comment and the function declaration got wrapped together.  Not
nice.


Thanks,
NeilBrown


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2010-07-06  7:11 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <A9DE54D0CD747C4CB06DCE5B6FA2246FDA893C42@irsmsx504.ger.corp.intel.com>
2010-07-05 10:48 ` [PATCH 21/33] Monitor: link containers and volumes in statelist Hawrylewicz Czarnowski, Przemyslaw
2010-07-06  7:11 ` Neil Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).