Re: install/53220: sysinst dumps core with extended partitioning.

[Robert Elz <kre%munnari.OZ.AU@localhost>]

The following reply was made to PR install/53220; it has been noted by GNATS.

From: Robert Elz <kre%munnari.OZ.AU@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: install/53220: sysinst dumps core with extended partitioning.
Date: Tue, 01 May 2018 11:26:05 +0700

 Some progress on this ...
 
 The sysinst code contains this (disks.c)...
 
 static bool
 is_gpt(const char *dev)
 {
         check_available_binaries();
                                 
         if (!have_gpt)
                 return false;           
                         
         return !run_program(RUN_SILENT | RUN_ERROR_OK,
                 "sh -c 'gpt show %s |grep -e Pri\\ GPT\\ table'", dev);
 }
 
 which looks kind of OK, except there is no "grep" in the MFS miniroot
 so that is always going to fail (return false).   "sed" is installed, and can
 generally used to do what grep can do, with the notable exception of
 exiting with a status indicating whether a string (or RE) exists in the file.
 
 I (in a local copy only, as I don't really think this is the right way) changed
 this to
 
         return !run_program(RUN_SILENT | RUN_ERROR_OK,
                 "sh -c 'test -n \"$(gpt show %s | while read line; do "
                 "case \"${line}\" in (*\"Pri GPT table\"*) echo FOUND;exit 0;;" 
                 "esac; done)\"'", dev);      
 
 which is somewhat convoluted, but does work to determine if there
 is a GPT label on the drive (variations that attempt to be simpler
 will probably fail.)   Provided gpt (which is tested for) and sh
 (kind of mandatory) exist (and given "test" is built into sh) this
 should be OK.
 
 A much better solution would be to have a new sub-command for
 gpt like
 
 	gpt [-q] query dev
 
 which would (if the already existing -q option is not given)
 print something like "GPT Primary Label @%d on %s\n"
 or "No GPT on %s" as appropriate, and then (regardless of -q)
 exit(0) if there is a GPT label, and exit(1) otherwise, then
 sysinst could just do:
 
 	return !run_program(RUN_SILENT | RUN_ERROR_OK,
 		"sh -c 'gpt -q query %s'", dev);
 
 (the name "query" for the command could be anything of course).
 (It could also just forget the "sh -c" and use (for the 2nd line):
 		"gpt -q qery %s", dev);
 
 
 But this one isn't the real problem, though it probably doesn't help.
 
 This is also in sysinst - in partman.c ...
 
 #define remove_vnd_options() (void)0
                 if (!have_vnd)
                         remove_vnd_options();
                 else if (!(vnds = calloc(MAX_VND, sizeof(*vnds))))
                         have_vnd = 0;
 
 Combine that with:
 
         have_vnd = binary_available("vnconfig");
 
 when "vnconfig" was renamed as "vndconfig" ages ago, and you
 have a recipe for potential disaster, when someone eventually
 decides that enough time has passed, and the old vnconfig
 link to vndconfig should simply be dropped (changing the 4
 occurrences of vnconfig into vndconfig should be easy!)
 
 Of course, the same recipe for disaster (not potential) exists when
 there is no insalled vnconfig (or vndconfig) program at all - which is
 the case on the MFS mini-roon that's embedded in the INSTALL
 kernel.
 
 Fixing this would mean actually writing the function, which
 given the simplicity of the other similar functions does't look
 like it should be too hard, eg, this example...
 
 void
 remove_cgd_options() 
 {
         /* 
          * No CGD available, remove the following menu entries:
          */
         remove_menu_option(MENU_pmdiskentry, MSG_encrypt);
         remove_menu_option(MENU_pmpartentry, MSG_encrypt);
 }
 
 (and similar for gpt, raid, lvm, cgd, color -- several of which also have
 no support on the MFS embedded mini-root)
 
 kre