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
Task Claimed by Васил Божурски November 27 2012 19:31 UTC

I would like to work on this task.

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

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

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

The work on this task is ready to be reviewed.

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

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

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

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.

Move MENU_distmedium by gnrp November 28 2012 22:22 UTC

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.

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

The work on this task is ready to be reviewed.

You have to wait a bit by gnrp November 30 2012 00:55 UTC

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. 

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

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

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

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.

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

The work on this task is ready to be reviewed.

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

Congratulations, this task has been completed successfully.

Perfect by Julian Coleman December 2 2012 14:46 UTC

This is perfect.


Thank you!


J