I am in doubts whether the fix is worth the risk of breaking something
else. The code for network setup in loader is fragile using all the
*_set and other flags, and it is easy to break some unpredicted path
of installation. Also we are going to get rid of all this code in loader
in Fedora.
It seems to me that the real problem here is that the fail of
network activation ([Retry]) should be handled in readNetConfig,
but this would probably need some significant code consolidation /
refactoring
of both uses of the readNetConfig (STEP_NETWORK and
kickstartNetworUp -> activateDevice). My opinion on fixing this is:
http://fedoraproject.org/wiki/Anaconda/Network#Back_and_Retry_in_loader_in_poor_state
Your patch assumes that rc == LOADER_ERROR for
rc = installMethods[validMethods[loaderData->method]].prompt(loaderData);
means fail of network configuration (for all installMethods,
although looking at the code perhaps we can get
this rc actually (incidentally?) only for nfs mehtod), but I am not
sure if resetting the flags (e.g. in case when network was
brought up successfully, but the url or nfs server address was
actually wrong and then fixed by user in reiteration of STEP_METHOD)
won't change behaviour or cause problems in following
STEP_NETWORK.
Sorry for my sceptic reply but this is just my experience, I've seen
(and made myself) similar patches fixing something in one place
and breaking something else. These changes are very difficult to
review.
On 12/22/2011 06:22 PM, Mark Hamzy wrote:
For https://bugzilla.redhat.com/show_bug.cgi?id=758450 , the following is the proposed patch.
Comments?
---
loader/loader.c | 15 +++++++++++++++
loader/net.c | 3 +++
loader/nfsinstall.c | 3 ++-
3 files changed, 20 insertions(+), 1 deletions(-)
diff --git a/loader/loader.c b/loader/loader.c
index c60795f..ba85734 100644
--- a/loader/loader.c
+++ b/loader/loader.c
@@ -1441,6 +1441,21 @@ static void doLoaderMain(struct loaderData_s *loaderData,
if (rc == LOADER_BACK)
break;
+ if (rc == LOADER_ERROR) {
+ needsNetwork = 1;
+
+ /* Don't retry the same failed settings. Clear ipinfo-set
+ * so the user can enter new data */
+ loaderData->ipinfo_set = 0;
+#ifdef ENABLE_IPV6
+ loaderData->ipv6info_set = 0;
+#endif
+ loaderData->netDev_set = 0;
+ logMessage(DEBUGLVL, "in STEP_METHOD, retry (LOADER_ERROR)");
+
+ break;
+ }
+
class = installMethods[validMethods[loaderData->method]].type;
step = STEP_DRIVER;
dir = 1;
diff --git a/loader/net.c b/loader/net.c
index d556554..31d8205 100644
--- a/loader/net.c
+++ b/loader/net.c
@@ -655,6 +655,8 @@ int configureTCPIP(char * device, iface_t * iface,
#endif
#ifdef ENABLE_IPV6
+ logMessage(DEBUGLVL, "in configureTCPIP() iface->ipv4method = %d, FL_NOIPV4(flags) = %ld", iface->ipv4method, FL_NOIPV4(flags));
+ logMessage(DEBUGLVL, "in configureTCPIP() iface->ipv6method = %d, FL_NOIPV6(flags) = %ld", iface->ipv6method, FL_NOIPV6(flags));
/* If the user provided any of the following boot paramters, skip
* prompting for network configuration information:
* ip=<val> ipv6=<val>
@@ -671,6 +673,7 @@ int configureTCPIP(char * device, iface_t * iface,
logMessage(DEBUGLVL, "in configureTCPIP(), detected network boot args, skipping form");
}
#else
+ logMessage(DEBUGLVL, "in configureTCPIP() iface->ipv4method = %d, FL_NOIPV4(flags) = %ld", iface->ipv4method, FL_NOIPV4(flags));
if (iface->ipv4method> IPV4_UNUSED_METHOD || FL_NOIPV4(flags)) {
skipForm = 1;
newtPopWindow();
diff --git a/loader/nfsinstall.c b/loader/nfsinstall.c
index f46e79e..f02ab16 100644
--- a/loader/nfsinstall.c
+++ b/loader/nfsinstall.c
@@ -257,7 +257,8 @@ int promptForNfs(struct loaderData_s *loaderData) {
newtWinMessage(_("Error"), _("OK"),
_("The URL provided does not contain an installable tree."));
free(url);
- continue;
+ loaderData->instRepo = NULL;
+ return LOADER_ERROR;
}
free(url);
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
