Logged-In As
ACCOUNT
Not Logged In
Fix bug install/17792: Install process order - network verses disk The NetBSD Project
Status: Closed Time to complete: 168 hrs Mentors: Julian Coleman, Julian Fagir Tags: C, system

The problem in this bug report is clear: The function get_and_unpack_sets() in util.c calls the menu MENU_distmedium as long as the status is set to retry.


The solution is to run this menu once before actually getting into this loop, before actually writing the changes to disk (see the file install.c or upgrade.c for an example).
Though the user who is used to fetch its sources from more than one source will be confused about the changed order of the question, 99% of the people won't even notice it. 

http://gnats.netbsd.org/cgi-bin/query-pr-single.pl?number=17792

Uploaded Work
File name/URL File size Date submitted
FixInstall.rar 12.2 KB November 28 2012 14:12 UTC
distmedium_fix2.zip 16.3 KB November 29 2012 16:11 UTC
distmedium_fix3.zip 16.3 KB November 29 2012 16:17 UTC
distmedium_fix4.zip 21.9 KB December 02 2012 14:39 UTC
Comments
Васил Божурски on November 27 2012 19:31 UTC Task Claimed

I would like to work on this task.

Aleksej Saushev on November 28 2012 08:20 UTC Task Assigned

This task has been assigned to Васил Божурски. You have 168 hours to complete this task, good luck!

Васил Божурски on November 28 2012 14:12 UTC Ready for review

The work on this task is ready to be reviewed.

Julian Coleman on November 28 2012 21:38 UTC The bug is an ordering problem

Hi,


Looking at the bug report, the problem is that the sysinst program erased the old disk contents before it ran the network setup.  This caused a problem because the network setup failed.


If you look at the main menu when you run sysinst, the first choice (a:) is to run the installation, but there is also a utility menu (e:) where there is a choice to configure the network (c:).  So, a possible solution to this problem is to offer the chance to configure the network before the installation starts.


Thanks,


J

Julian Coleman on November 28 2012 21:38 UTC Task Needs More Work

One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.

Julian Fagir on November 28 2012 22:22 UTC Move MENU_distmedium

Hm, I would do it the other way round: Move up the call of


process_menu(MENU_distmedium, &status);


out of get_and_unpack_sets somewhere else, so the user has to define the location of his set files somewhere before the write_disklabel() call in install.c and upgrade.c, and set the status variable in get_and_unpack_sets somewhere from the outside.

Васил Божурски on November 29 2012 16:18 UTC Ready for review

The work on this task is ready to be reviewed.

Julian Fagir on November 30 2012 00:55 UTC You have to wait a bit

Please wait a bit for a review. Testing this requires some time (you have to update sources, rebuild an image, fire up a virtual machine, recreate the test case, test some other problems). But from a first glance, it looks ok.


Just some general coding tips:



  • use a continuous indention style. In get_medium, you once didn't indent (for do... while), once you indented the closing brackets (the last if...).

  • define variables only in the beginning of a function. In your addition to install.c, you introduce the variable `int status` in the middle of the code. 

Julian Coleman on December 2 2012 13:43 UTC Looks good ...

Hi,


This looks very good.  With a quick test, I was asked for where to get the installation sets from before the disk was written to.  A couple of comments though.  I think that there is a missing change to defs.h, something like:


--- /usr/src/distrib/utils/sysinst/defs.h       2012-08-03 17:22:28.000000000 +0100
+++ ./defs.h    2012-12-02 13:29:26.000000000 +0000
@@ -446,4 +446,5 @@
 void   run_makedev(void);
 int    boot_media_still_needed(void);
+int    get_medium(int *);
 int    get_via_floppy(void);
 int    get_via_cdrom(void);
@@ -455,5 +456,5 @@
 unsigned int    get_kernel_set(void);
 unsigned int    set_X11_selected(void);
-int    get_and_unpack_sets(int, msg, msg, msg);
+int    get_and_unpack_sets(int, msg, msg, msg, int *);
 int    sanity_check(void);
 int    set_timezone(void);


Also, using (int) instead of (int *) for the arguments for get_and_unpack_sets() and for get_medium() would be more consistent with the rest of the code.


Thanks!


J

Julian Coleman on December 2 2012 13:43 UTC Task Needs More Work

One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.

Васил Божурски on December 2 2012 14:40 UTC Ready for review

The work on this task is ready to be reviewed.

Julian Coleman on December 2 2012 14:44 UTC Task Closed

Congratulations, this task has been completed successfully.

Julian Coleman on December 2 2012 14:46 UTC Perfect

This is perfect.


Thank you!


J