Re: [PATCH] Use module reloading in driver disc operations (#586905)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

Hi,

A few small comments inline, but in general ACK.

Regards,

Hans


On 05/06/2010 03:35 PM, Martin Sivak wrote:

We have to load all drivers to get access to devices containing driver discs. But when we copy the DD content into RAM, we have to reinitialize those drivers to pick up updated versions.
---
  loader/driverdisk.c   |   68 +++++++++---------------------
  loader/driverdisk.h   |    9 +---
  loader/driverselect.c |    4 +-
  loader/hdinstall.c    |    4 +-
  loader/loader.c       |   51 +++++++++++++----------
  loader/modules.c      |  110 ++++++++++++++++++++++++++++++++++++++++++++++++-
  loader/modules.h      |    4 ++
  scripts/upd-instroot  |    2 +-
  8 files changed, 171 insertions(+), 81 deletions(-)

diff --git a/loader/driverdisk.c b/loader/driverdisk.c
index 192beb0..bfebc06 100644
--- a/loader/driverdisk.c
+++ b/loader/driverdisk.c
@@ -63,40 +63,6 @@
  /* boot flags */
  extern uint64_t flags;

-/* modprobe DD mode */
-int modprobeDDmode(void)
-{
-    FILE *f = fopen("/etc/depmod.d/ddmode.conf", "w");
-    if (f) {
-        struct utsname unamedata;
-
-        if (uname(&unamedata))
-            fprintf(f, " pblacklist /lib/modules\n");
-        else
-            fprintf(f, " pblacklist /lib/modules/%s\n", unamedata.release);
-        fclose(f);
-    }
-
-    return f==NULL;
-}
-
-int modprobeNormalmode(void)
-{
-    /* remove depmod overrides */
-    if (unlink("/etc/depmod.d/ddmode.conf")) {
-        logMessage(ERROR, "removing ddmode.conf failed");
-        return -1;
-    }
-
-    /* run depmod to refresh modules db */
-    if (system("depmod -a")) {
-        logMessage(ERROR, "depmod -a failed");
-        return -1;
-    }
-
-    return 0;
-}
-
  /*
   * check if the RPM in question provides
   * Provides: userptr
@@ -362,6 +328,7 @@ int getRemovableDevices(char *** devNames) {
      devs = getDevices(DEVICE_DISK | DEVICE_CDROM);

      if(devs) for (i = 0; devs[i] ; i++) {
+            logMessage(DEBUGLVL, "Considering device %s (isremovable: %d)", devs[i]->device, devs[i]->priv.removable);
          if (devs[i]->priv.removable) {
              *devNames = realloc(*devNames, (numDevices + 2) * sizeof(char *));
              (*devNames)[numDevices] = strdup(devs[i]->device);
@@ -383,7 +350,7 @@ int getRemovableDevices(char *** devNames) {
   * usecancel: if 1, use cancel instead of back
   */
  int loadDriverFromMedia(int class, struct loaderData_s *loaderData,
-                        int usecancel, int noprobe) {
+                        int usecancel, int noprobe, GTree *moduleState) {
      char * device = NULL, * part = NULL, * ddfile = NULL;
      char ** devNames = NULL;
      enum { DEV_DEVICE, DEV_PART, DEV_CHOOSEFILE, DEV_LOADFILE,
@@ -609,6 +576,9 @@ int loadDriverFromMedia(int class, struct loaderData_s *loaderData,
                  break;
              }

+            /* Unload all devices and load them again to use the updated modules */
+            logMessage(INFO, "Trying to refresh loaded drivers");
+            mlRestoreModuleState(moduleState);
              busProbe(0);

              devices = getDevices(class);
@@ -659,7 +629,7 @@ int loadDriverFromMedia(int class, struct loaderData_s *loaderData,


  /* looping way to load driver disks */
-int loadDriverDisks(int class, struct loaderData_s *loaderData) {
+int loadDriverDisks(int class, struct loaderData_s *loaderData, GTree *moduleState) {
      int rc;

      rc = newtWinChoice(_("Driver disk"), _("Yes"), _("No"),
@@ -667,7 +637,7 @@ int loadDriverDisks(int class, struct loaderData_s *loaderData) {
      if (rc != 1)
          return LOADER_OK;

-    rc = loadDriverFromMedia(DEVICE_ANY, loaderData, 1, 0);
+    rc = loadDriverFromMedia(DEVICE_ANY, loaderData, 1, 0, moduleState);
      if (rc == LOADER_BACK)
          return LOADER_OK;

@@ -676,23 +646,27 @@ int loadDriverDisks(int class, struct loaderData_s *loaderData) {
                             _("Do you wish to load any more driver disks?"));
          if (rc != 1)
              break;
-        loadDriverFromMedia(DEVICE_ANY, loaderData, 0, 0);
+        loadDriverFromMedia(DEVICE_ANY, loaderData, 0, 0, moduleState);
      } while (1);

      return LOADER_OK;
  }

-static void loadFromLocation(struct loaderData_s * loaderData, char * dir) {
+static void loadFromLocation(struct loaderData_s * loaderData, char * dir, GTree *moduleState) {
      if (verifyDriverDisk(dir) == LOADER_BACK) {
          logMessage(ERROR, "not a valid driver disk");
          return;
      }

      loadDriverDisk(loaderData, dir);
+
+    /* Unload all devices and load them again to use the updated modules */
+    logMessage(INFO, "Trying to refresh loaded drivers");
+    mlRestoreModuleState(moduleState);
      busProbe(0);
  }

-void getDDFromSource(struct loaderData_s * loaderData, char * src) {
+void getDDFromSource(struct loaderData_s * loaderData, char * src, GTree *moduleState) {
      char *path = "/tmp/dd.img";
      int unlinkf = 0;

@@ -712,7 +686,7 @@ void getDDFromSource(struct loaderData_s * loaderData, char * src) {
       * scsi cdrom drives */
  #if !defined(__s390__)&&  !defined(__s390x__)
      } else if (!strncmp(src, "cdrom", 5)) {
-        loadDriverDisks(DEVICE_ANY, loaderData);
+        loadDriverDisks(DEVICE_ANY, loaderData, moduleState);
          return;
  #endif
      } else if (!strncmp(src, "path:", 5)) {
@@ -724,7 +698,7 @@ void getDDFromSource(struct loaderData_s * loaderData, char * src) {
      }

      if (!mountLoopback(path, "/tmp/drivers", "/dev/loop6")) {
-        loadFromLocation(loaderData, "/tmp/drivers");
+        loadFromLocation(loaderData, "/tmp/drivers", moduleState);
          umountLoopback("/tmp/drivers", "/dev/loop6");
          unlink("/tmp/drivers");
          if (unlinkf) unlink(path);
@@ -732,7 +706,7 @@ void getDDFromSource(struct loaderData_s * loaderData, char * src) {

  }

-static void getDDFromDev(struct loaderData_s * loaderData, char * dev);
+static void getDDFromDev(struct loaderData_s * loaderData, char * dev, GTree *moduleState);

  void useKickstartDD(struct loaderData_s * loaderData,
                      int argc, char ** argv) {
@@ -796,22 +770,22 @@ void useKickstartDD(struct loaderData_s * loaderData,
      }

      if (dev) {
-        getDDFromDev(loaderData, dev);
+        getDDFromDev(loaderData, dev, NULL);
      } else {
-        getDDFromSource(loaderData, src);
+        getDDFromSource(loaderData, src, NULL);
      }

      g_strfreev(remaining);
      return;
  }

-static void getDDFromDev(struct loaderData_s * loaderData, char * dev) {
+static void getDDFromDev(struct loaderData_s * loaderData, char * dev, GTree* moduleState) {
      if (doPwMount(dev, "/tmp/drivers", "auto", "ro", NULL)) {
          logMessage(ERROR, "unable to mount driver disk %s", dev);
          return;
      }

-    loadFromLocation(loaderData, "/tmp/drivers");
+    loadFromLocation(loaderData, "/tmp/drivers", moduleState);
      umount("/tmp/drivers");
      unlink("/tmp/drivers");
  }
diff --git a/loader/driverdisk.h b/loader/driverdisk.h
index 98bfd4a..4dc8685 100644
--- a/loader/driverdisk.h
+++ b/loader/driverdisk.h
@@ -32,9 +32,9 @@
  extern char *ddFsTypes[];

  int loadDriverFromMedia(int class, struct loaderData_s *loaderData,
-                        int usecancel, int noprobe);
+                        int usecancel, int noprobe, GTree *moduleState);

-int loadDriverDisks(int class, struct loaderData_s *loaderData);
+int loadDriverDisks(int class, struct loaderData_s *loaderData, GTree *moduleState);

  int getRemovableDevices(char *** devNames);

@@ -42,13 +42,10 @@ int chooseManualDriver(int class, struct loaderData_s *loaderData);
  void useKickstartDD(struct loaderData_s * loaderData, int argc,
                      char ** argv);

-void getDDFromSource(struct loaderData_s * loaderData, char * src);
+void getDDFromSource(struct loaderData_s * loaderData, char * src, GTree *moduleState);

  int loadDriverDiskFromPartition(struct loaderData_s *loaderData, char* device);

  GSList* findDriverDiskByLabel(void);

-int modprobeNormalmode();
-int modprobeDDmode();
-
  #endif
diff --git a/loader/driverselect.c b/loader/driverselect.c
index f9379f0..14d05e7 100644
--- a/loader/driverselect.c
+++ b/loader/driverselect.c
@@ -155,7 +155,7 @@ int chooseManualDriver(int class, struct loaderData_s *loaderData) {
              if (i != 1)
                  return LOADER_BACK;

-            loadDriverFromMedia(class, loaderData, 1, 1);
+            loadDriverFromMedia(class, loaderData, 1, 1, NULL);
              continue;
          } else {
              break;
@@ -235,7 +235,7 @@ int chooseManualDriver(int class, struct loaderData_s *loaderData) {
      if (done == -1)
          return LOADER_BACK;
      if (done == -2) {
-        loadDriverFromMedia(class, loaderData, 1, 1);
+        loadDriverFromMedia(class, loaderData, 1, 1, NULL);
          return chooseManualDriver(class, loaderData);
      }

diff --git a/loader/hdinstall.c b/loader/hdinstall.c
index e05be0c..54a5fd9 100644
--- a/loader/hdinstall.c
+++ b/loader/hdinstall.c
@@ -218,7 +218,7 @@ char * mountHardDrive(struct installMethod * method,
                  return NULL;
              }

-            rc = loadDriverFromMedia(DEVICE_DISK, loaderData, 0, 0);
+            rc = loadDriverFromMedia(DEVICE_DISK, loaderData, 0, 0, NULL);
              continue;
          }

@@ -303,7 +303,7 @@ char * mountHardDrive(struct installMethod * method,
              loaderData->stage2Data = NULL;
              return NULL;
          } else if (es.reason == NEWT_EXIT_HOTKEY&&  es.u.key == NEWT_KEY_F2) {
-            rc = loadDriverFromMedia(DEVICE_DISK, loaderData, 0, 0);
+            rc = loadDriverFromMedia(DEVICE_DISK, loaderData, 0, 0, NULL);
              continue;
          }

diff --git a/loader/loader.c b/loader/loader.c
index c828b96..91abdab 100644
--- a/loader/loader.c
+++ b/loader/loader.c
@@ -1379,7 +1379,7 @@ static char *doLoaderMain(struct loaderData_s *loaderData,
                      break;
                  }

-                rc = loadDriverFromMedia(class, loaderData, 0, 0);
+                rc = loadDriverFromMedia(class, loaderData, 0, 0, NULL);
                  if (rc == LOADER_BACK) {
                      step = STEP_DRIVER;
                      dir = -1;
@@ -1799,6 +1799,7 @@ int main(int argc, char ** argv) {

      char *path, *fmt;
      GSList *dd, *dditer;
+    GTree *moduleState;

      gchar *cmdLine = NULL, *ksFile = NULL, *virtpcon = NULL;
      gboolean mediacheck = FALSE;
@@ -1963,9 +1964,6 @@ int main(int argc, char ** argv) {
      /* FIXME: this is a bit of a hack */
      loaderData.modInfo = modInfo;

-    /* Setup depmod&  modprobe so we can load multiple DDs */
-    modprobeDDmode();
-
      /* If there is /.rundepmod file present, rerun depmod */
      if (!access("/.rundepmod", R_OK)){
          if (system("depmod -a")) {
@@ -1974,6 +1972,25 @@ int main(int argc, char ** argv) {
          }
      }

+    /* this allows us to do an early load of modules specified on the
+     * command line to allow automating the load order of modules so that
+     * eg, certain scsi controllers are definitely first.
+     * FIXME: this syntax is likely to change in a future release
+     *        but is done as a quick hack for the present.
+     */
+    if (!mlInitModuleConfig()) {
+        logMessage(ERROR, "unable to initialize kernel module loading");
+        abort();
+    }
+
+    earlyModuleLoad(0);
+
+    /* Save list of preloaded modules so we can restore the state */
+    moduleState = mlSaveModuleState();
+
+    /* Load all known devices */
+    busProbe(FL_NOPROBE(flags));
+
      if (FL_AUTOMODDISK(flags)) {
          /* Load all autodetected DDs */
          logMessage(INFO, "Trying to detect vendor driver discs");
@@ -1986,6 +2003,10 @@ int main(int argc, char ** argv) {
              }
              else {
                  logMessage(INFO, "Automatic driver disk loader succeeded for %s.", (char*)(dditer->data));
+
+                /* Unload all devices and load them again to use the updated modules */
+                mlRestoreModuleState(moduleState);
+                busProbe(0);
              }

              /* clean the device record */
@@ -2000,30 +2021,16 @@ int main(int argc, char ** argv) {

      if (FL_MODDISK(flags)) {
          startNewt();
-        loadDriverDisks(DEVICE_ANY,&loaderData);
+        loadDriverDisks(DEVICE_ANY,&loaderData, moduleState);
      }

      if (!access("/dd.img", R_OK)) {
          logMessage(INFO, "found /dd.img, loading drivers");
-        getDDFromSource(&loaderData, "path:/dd.img");
+        getDDFromSource(&loaderData, "path:/dd.img", moduleState);
      }

      /* Reset depmod&  modprobe to normal mode and get the rest of drivers*/
-    modprobeNormalmode();
-
-    /* this allows us to do an early load of modules specified on the
-     * command line to allow automating the load order of modules so that
-     * eg, certain scsi controllers are definitely first.
-     * FIXME: this syntax is likely to change in a future release
-     *        but is done as a quick hack for the present.
-     */
-    if (!mlInitModuleConfig()) {
-        logMessage(ERROR, "unable to initialize kernel module loading");
-        abort();
-    }
-
-    earlyModuleLoad(0);
-
+    mlFreeModuleState(moduleState);
      busProbe(FL_NOPROBE(flags));

      /* HAL daemon */
@@ -2055,7 +2062,7 @@ int main(int argc, char ** argv) {
       * we won't have network devices available (and that's the only thing
       * we support with this right now */
      if (loaderData.ddsrc != NULL) {
-        getDDFromSource(&loaderData, loaderData.ddsrc);
+        getDDFromSource(&loaderData, loaderData.ddsrc, NULL);
      }

      /* JKFIXME: loaderData->ksFile is set to the arg from the command line,
diff --git a/loader/modules.c b/loader/modules.c
index 07f781a..b7608bf 100644
--- a/loader/modules.c
+++ b/loader/modules.c
@@ -24,7 +24,6 @@
   *            Jeremy Katz<katzj@xxxxxxxxxx>
   *            David Cantrell<dcantrell@xxxxxxxxxx>
   */
-
  #include<ctype.h>
  #include<errno.h>
  #include<fcntl.h>
@@ -39,6 +38,8 @@
  #include<sys/wait.h>
  #include<unistd.h>
  #include<glib.h>
+#include<glib/gprintf.h>
+#include<assert.h>

  #include "loader.h"
  #include "log.h"
@@ -408,3 +409,110 @@ void loadKickstartModule(struct loaderData_s * loaderData,
      g_strfreev(args);
      return;
  }
+
+inline gint gcmp(gconstpointer a, gconstpointer b, gpointer userptr)
+{
+    return g_strcmp0(a, b);
+}
+
+int processModuleLines(GTree *data, int (*f)(gchar**, GTree*)){
+    char *line = NULL;
+    size_t linesize = 0;
+    gchar** lineparts = NULL;
+    int count = 0;
+
+    FILE *file = fopen("/proc/modules", "r");
+    if (file == NULL) return -1;
+
+    while (1) {
+        if (getline(&line,&linesize, file)<  0) break;
+        assert(*line);
+


Note that getline will include a new line in the buffer it reads /
creates, so


+        lineparts = g_strsplit_set(line, " ", 4);


You may want to add '\n' to the delimiter set here.


+	assert(lineparts);
+


Ugh, assert is not meant for function return value checking. Compilation
with -DNDEBUG will turn it unto a no-op and you will still want to check
the return value then ...


+        free(line);
+        line = NULL;
+        int ret = f(lineparts, data);
+        g_strfreev(lineparts);
+
+        if (ret<  0) break;
+        count+=ret;
+    }
+
+    fclose(file);
+    return count;
+}
+
+inline int cb_savestate(gchar** parts, GTree *data)
+{
+    logMessage(DEBUGLVL, "Saving module %s", parts[0]);
+    g_tree_insert(data, g_strdup(parts[0]), (gchar*)1);
+    return 1;
+}
+
+GTree* mlSaveModuleState()
+{
+    GTree *state = NULL;
+
+    state = g_tree_new_full(gcmp, NULL, g_free, NULL);
+    if(!state) return NULL;


Why does what comes down to an out of memory error, result
in an error return here, while you are using assert (and thus abort)
above ? Note I'm not claiming the oom handling in the rest
of loader deserves any awards.


+
+    processModuleLines(state, cb_savestate);
+
+    return state;
+}
+
+inline int cb_restorestate(gchar** parts, GTree *data)
+{
+    pid_t pid;
+    int status;
+
+    /* this module has to stay loaded */
+    if (g_tree_lookup(data, parts[0])){
+        return 0;
+    }
+
+    /* this module is still required */
+    if (parts[2][0]!='0'){
+	return 0;
+    }	
+
+    /* rmmod parts[0] */
+    pid = fork();
+    if (pid == 0) {
+        execl("/sbin/rmmod", "-f", parts[0], NULL);
+        _exit(1);
+    }
+    else if (pid<  0) {
+        logMessage(ERROR, "Module %s removal FAILED", parts[0]);
+        return 0;
+    }
+
+    waitpid(pid,&status, 0);
+    if (WEXITSTATUS(status)) {
+      logMessage(DEBUGLVL, "Module %s was NOT removed", parts[0]);
+      return 0;
+    }
+    else{
+      logMessage(DEBUGLVL, "Module %s was removed", parts[0]);
+      return 1;
+    }
+
+    return 0;
+}
+
+void mlRestoreModuleState(GTree *state)
+{
+    if(!state) return;
+
+    logMessage(INFO, "Restoring module state...");
+    while(processModuleLines(state, cb_restorestate)>0);
+}
+
+void mlFreeModuleState(GTree *state)
+{
+    if(!state) return;
+
+    g_tree_destroy(state);
+}
diff --git a/loader/modules.h b/loader/modules.h
index 88fa25f..605d01e 100644
--- a/loader/modules.h
+++ b/loader/modules.h
@@ -40,4 +40,8 @@ gboolean mlAddBlacklist(gchar *);
  gboolean mlRemoveBlacklist(gchar *);
  void loadKickstartModule(struct loaderData_s *, int, char **);

+GTree* mlSaveModuleState();
+void mlRestoreModuleState(GTree *state);
+void mlFreeModuleState(GTree *state);
+
  #endif
diff --git a/scripts/upd-instroot b/scripts/upd-instroot
index c97326a..23cac11 100755
--- a/scripts/upd-instroot
+++ b/scripts/upd-instroot
@@ -477,6 +477,7 @@ sbin/reiserfsck
  sbin/reiserfstune
  sbin/resize_reiserfs
  sbin/resize2fs
+sbin/rmmod
  sbin/route
  sbin/setfiles
  sbin/sfdisk
@@ -826,7 +827,6 @@ sbin/depmod
  sbin/netstat
  sbin/restore
  sbin/rrestore
-sbin/rmmod
  sbin/route
  sbin/mount.cifs
  sbin/umount.cifs


_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux