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

Re: [PATCH 2/3] Prevent NetworkManager from destroying configured interfaces.



On 02/15/2012 07:50 PM, David Cantrell wrote:
Any network interface configured by dracut on s390x is destroyed when
loader starts up with a graphical interface override flag passed on
the boot command line (e.g., 'vnc').  We call kickstartNetworkUp(),
which writes out ifcfg files to down everything and bring up the
interface the user wants.


I am trying to understand what is happening here:

So dracut configures and brings up (which is new thing?) a network device.
Loader then tries to start NetworkManager. From your patchset
this thing doesn't seem as a problem, I wonder if it takes
control over the devices (even whether NM_CONTROLLED is yes),
but I suppose so.

The problem seems to be that the condition in kickstartNetworkUp()

    if ((is_nm_connected() == TRUE) &&
        (loaderData->netDev != NULL) && (loaderData->netDev_set == 1))
        return 0;

doesn't prevent the device activated in dracut from reactivating.
Shouldn't we better fix this condition for s390x rather than remove
the stuff all over for the arch?

Now why the condition would fail? - I don't think that it is because
of is_nm_connected() - if the active device is controlled by NM it should be
detected (if it is not controlled I imagine it would hit us later).

So is it because of the flags? We used to set them for s390 case
in readNetInfo() you removed in related 471be3d8abebb8565965e0d320c71ead74a7399.
Shouldn't we keep it in? Well, the configuration is written by dracut
instead of linuxrc.s390, but this does not matter, we still need
to suck pre-configuration (be it from linuxrc.s390 or dracut) in loader somehow.

To sum up: to me it seems that readNetInfo shouldn't have been removed,
then the loaderData->netDev and loaderData->netDev_set flags would
be set properly and the condition in kickstartNetworkUp would prevent
device reactivation.


(I am not sure if activating the device in dracut itself would not cause
problems when NM is starting and if it takes it over neatly, but from your
patches it seems that this should work ok and the problems come later.)

Also I have one comment below:


In dracut-land, all of that has already happened, so suppress these
steps on s390x.

NOTE:  This is all taken care on master since loader has been removed.

Related: rhbz#783227
---
  loader/loader.c     |   39 ++++++++++++++++++---------------------
  loader/net.c        |    2 ++
  loader/nfsinstall.c |    2 ++
  loader/urlinstall.c |    2 ++
  4 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/loader/loader.c b/loader/loader.c
index 75ffe80..f020366 100644
--- a/loader/loader.c
+++ b/loader/loader.c
@@ -194,10 +194,12 @@ void doGdbserver(struct loaderData_s *loaderData) {
          pid_t loaderPid = getpid();
          iface_init_iface_t(&iface);

+#if !defined(__s390__)&&  !defined(__s390x__)
          if (kickstartNetworkUp(loaderData,&iface)) {
              logMessage(ERROR, "can't run gdbserver due to no network");
              return;
          }
+#endif

          checked_asprintf(&pid, "%d", loaderPid);

@@ -750,9 +752,11 @@ static void parseCmdLineFlags(struct loaderData_s * loaderData) {
              }
          } else if (!strcasecmp(k, "noeject")) {
              flags |= LOADER_FLAGS_NOEJECT;
+#if !defined(__s390__)&&  !defined(__s390x__)
          } else if (!strcasecmp(k, "sshd")) {
              logMessage(INFO, "early networking required for sshd");
              flags |= LOADER_FLAGS_EARLY_NETWORKING;
+#endif
          } else if (!strcasecmp(k, "noverifyssl")) {
              flags |= LOADER_FLAGS_NOVERIFYSSL;
          } else if (v != NULL) {
@@ -916,6 +920,7 @@ static void parseCmdLineFlags(struct loaderData_s * loaderData) {
                   * by loader, so an active connection is ready once we get
                   * to anaconda
                   */
+#if !defined(__s390__)&&  !defined(__s390x__)
                  if (!strcasecmp(k, "syslog") || !strcasecmp(k, "vnc")) {
                      logMessage(INFO, "early networking required for %s", k);
                      flags |= LOADER_FLAGS_EARLY_NETWORKING;
@@ -925,6 +930,7 @@ static void parseCmdLineFlags(struct loaderData_s * loaderData) {
                      logMessage(INFO, "early networking required for remote kickstart configuration");
                      flags |= LOADER_FLAGS_EARLY_NETWORKING;
                  }
+#endif

                  if (v != NULL) {
                      checked_asprintf(&extraArgs[numExtraArgs], "--%s=%s", k, v)
@@ -1356,12 +1362,17 @@ static void doLoaderMain(struct loaderData_s *loaderData,
                  if (loaderData->ksFile)
                      flags |= LOADER_FLAGS_IS_KICKSTART;

-                if (FL_HAVE_CMSCONF(flags)) {
+#if defined(__s390__) || defined(__s390x__)
+                {
                      loaderData->ipinfo_set = 1;
  #ifdef ENABLE_IPV6
                      loaderData->ipv6info_set = 1;
  #endif
+                    step = STEP_EXTRAS;
+                    dir = 1;
+                    break;
                  }
+#endif

                  rc = chooseNetworkInterface(loaderData);
                  if (rc == LOADER_BACK || rc == LOADER_ERROR || (dir == -1&&  rc == LOADER_NOOP)) {
@@ -1392,15 +1403,6 @@ static void doLoaderMain(struct loaderData_s *loaderData,
                      doExit(EXIT_FAILURE);
                  }

-
-                /* s390 provides all config info by way of linuxrc.s390 */
-                if (FL_HAVE_CMSCONF(flags)) {
-                    loaderData->ipinfo_set = 1;
-#ifdef ENABLE_IPV6
-                    loaderData->ipv6info_set = 1;
-#endif
-                }
-
                  /* populate netDev based on any kickstart data */
                  logMessage(DEBUGLVL, "in doLoaderMain, calling setupIfaceStruct()");
                  setupIfaceStruct(&iface, loaderData);
@@ -1745,7 +1747,9 @@ int main(int argc, char ** argv) {
      FILE *f;

      moduleInfoSet modInfo;
+#if !defined(__s390__)&&  !defined(__s390x__)
      iface_t iface;
+#endif

      char ** argptr, ** tmparg;
      char * anacondaArgs[50];
@@ -1844,8 +1848,10 @@ int main(int argc, char ** argv) {
          logMessage(INFO, "text mode forced due to serial/virtpconsole");
          flags |= LOADER_FLAGS_TEXT;
      }
+#if !defined(__s390__)&&  !defined(__s390x__)
      if (hasGraphicalOverride())
          flags |= LOADER_FLAGS_EARLY_NETWORKING;
+#endif
      set_fw_search_path(&loaderData, "/firmware:/lib/firmware");
      start_fw_loader(&loaderData);

@@ -1994,17 +2000,6 @@ int main(int argc, char ** argv) {
      mlFreeModuleState(moduleState);
      busProbe(FL_NOPROBE(flags));

-    /* Disable all network interfaces in NetworkManager by default */
-#if !defined(__s390__)&&  !defined(__s390x__)
-    {
-        int i;
-
-        if ((i = writeDisabledNetInfo()) != 0) {
-            logMessage(ERROR, "writeDisabledNetInfo failure: %d", i);
-        }
-    }
-#endif
-

Why to remove this, especially when it is guarded to not happen on s390?
We need this in other archs.

  #if defined(__s390__) || defined(__s390x__)
      /* Start NetworkManager until we have systemd init on s390 too */
      if (iface_start_NetworkManager())
@@ -2040,9 +2035,11 @@ int main(int argc, char ** argv) {
              outputKSFile = runKickstart(&loaderData, loaderData.ksFile);
      }

+#if !defined(__s390__)&&  !defined(__s390x__)
      if (FL_EARLY_NETWORKING(flags)) {
          kickstartNetworkUp(&loaderData,&iface);
      }
+#endif

      doLoaderMain(&loaderData, modInfo);
  #ifdef USESELINUX
diff --git a/loader/net.c b/loader/net.c
index f8843c5..9ed8ec9 100644
--- a/loader/net.c
+++ b/loader/net.c
@@ -1755,6 +1755,7 @@ int chooseNetworkInterface(struct loaderData_s * loaderData) {
      return LOADER_OK;
  }

+#if !defined(__s390__)&&  !defined(__s390x__)
  /* JKFIXME: bad name.  this function brings up networking early on a
   * kickstart install so that we can do things like grab the ks.cfg from
   * the network */
@@ -1797,6 +1798,7 @@ int kickstartNetworkUp(struct loaderData_s * loaderData, iface_t * iface) {

      return activateDevice(loaderData, iface);
  }
+#endif

  int disconnectDevice(char *device) {
      int rc;
diff --git a/loader/nfsinstall.c b/loader/nfsinstall.c
index eb9579f..a52234e 100644
--- a/loader/nfsinstall.c
+++ b/loader/nfsinstall.c
@@ -325,10 +325,12 @@ int getFileFromNfs(char * url, char * dest, struct loaderData_s * loaderData) {
      NMState state;
      const GPtrArray *devices;

+#if !defined(__s390__)&&  !defined(__s390x__)
      if (kickstartNetworkUp(loaderData,&iface)) {
          logMessage(ERROR, "unable to bring up network");
          return 1;
      }
+#endif

      /* if they just did 'linux ks', they want us to figure it out from
       * the dhcp/bootp information
diff --git a/loader/urlinstall.c b/loader/urlinstall.c
index f3713d0..7594895 100644
--- a/loader/urlinstall.c
+++ b/loader/urlinstall.c
@@ -231,10 +231,12 @@ int getFileFromUrl(char * url, char * dest,

      iface_init_iface_t(&iface);

+#if !defined(__s390__)&&  !defined(__s390x__)
      if (kickstartNetworkUp(loaderData,&iface)) {
          logMessage(ERROR, "unable to bring up network");
          return 1;
      }
+#endif

      logMessage(INFO, "file location: %s", url);



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