Re: install/56890: sysinst should ignore default label eveninDISKLABEL_NO_ONDISK_VERIFY case

[Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>]

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

From: Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: martin%duskware.de@localhost, tsutsui%ceres.dti.ne.jp@localhost
Subject: Re: install/56890: sysinst should ignore default label eveninDISKLABEL_NO_ONDISK_VERIFY
	 case
Date: Fri, 24 Jun 2022 00:26:24 +0900

 >  I like the patch, but would actually do the MD hooks change you proposed.
 >  How about documenting that if DISKLABEL_NO_ONDISK_VERIFY is defined, the
 >  MD code has to provide something like:
 >  
 >  	bool md_disklabel_is_default(const struct disklabel*);
 >  
 >  and move the two tests you implemented (for the two architectures currently
 >  defining DISKLABEL_NO_ONDISK_VERIFY) into their arch/md.c?
 
 Here is a dump one:
 ---
 
 Index: defs.h
 ===================================================================
 RCS file: /cvsroot/src/usr.sbin/sysinst/defs.h,v
 retrieving revision 1.84
 diff -u -p -d -r1.84 defs.h
 --- defs.h	19 Jun 2022 12:08:31 -0000	1.84
 +++ defs.h	23 Jun 2022 01:46:36 -0000
 @@ -667,6 +667,7 @@ bool	md_gpt_post_write(struct disk_parti
   */
  bool	md_pre_disklabel(struct install_partition_desc*, struct disk_partitions*);
  bool	md_post_disklabel(struct install_partition_desc*, struct disk_partitions*);
 +bool	md_disklabel_is_default(const struct disklabel *);
  int	md_pre_mount(struct install_partition_desc*, size_t);
  int	md_post_newfs(struct install_partition_desc*);
  int	md_post_extract(struct install_partition_desc*, bool upgrade);
 Index: disklabel.c
 ===================================================================
 RCS file: /cvsroot/src/usr.sbin/sysinst/disklabel.c,v
 retrieving revision 1.48
 diff -u -p -d -r1.48 disklabel.c
 --- disklabel.c	21 Jun 2022 16:08:25 -0000	1.48
 +++ disklabel.c	23 Jun 2022 01:46:36 -0000
 @@ -198,16 +198,7 @@ disklabel_parts_read(const char *disk, d
  	int fd;
  	char diskpath[MAXPATHLEN];
  	uint flags;
 -#ifndef DISKLABEL_NO_ONDISK_VERIFY
 -	bool have_raw_label = false;
 -
 -	/*
 -	 * Verify we really have a disklabel.
 -	 */
 -	if (run_program(RUN_SILENT | RUN_ERROR_OK,
 -	    "disklabel -r %s", disk) == 0)
 -		have_raw_label = true;
 -#endif
 +	bool have_own_label = false;
  
  	/* read partitions */
  
 @@ -304,8 +295,30 @@ disklabel_parts_read(const char *disk, d
  	}
  	close(fd);
  
 -#ifndef DISKLABEL_NO_ONDISK_VERIFY
 -	if (!have_raw_label) {
 +	/*
 +	 * Verify we really have a disklabel on the target disk.
 +	 */
 +	if (run_program(RUN_SILENT | RUN_ERROR_OK,
 +	    "disklabel -r %s", disk) == 0) {
 +		have_own_label = true;
 +	}
 +#ifdef DISKLABEL_NO_ONDISK_VERIFY
 +	else {
 +		/*
 +		 * disklabel(8) with -r checks a native disklabel at
 +		 * LABELOFFSET sector, but several ports don't have
 +		 * a native label and use emulated one translated from
 +		 * port specific MD disk partition information.
 +		 * Unfortunately, there is no MI way to check whether
 +		 * the disk has a native BSD disklabel by readdisklabel(9)
 +		 * via DIOCGDINFO.  So check if returned label looks
 +		 * defaults set by readdisklabel(9) per MD way.
 +		 */
 +		have_own_label = !md_disklabel_is_default(&parts->l);
 +	}
 +#endif
 +
 +	if (!have_own_label) {
  		bool found_real_part = false;
  
  		if (parts->l.d_npartitions <= RAW_PART ||
 @@ -338,7 +351,6 @@ no_valid_label:
  			return NULL;
  		}
  	}
 -#endif
  
  	return &parts->dp;
  }
 Index: arch/amiga/md.c
 ===================================================================
 RCS file: /cvsroot/src/usr.sbin/sysinst/arch/amiga/md.c,v
 retrieving revision 1.7
 diff -u -p -d -r1.7 md.c
 --- arch/amiga/md.c	29 Jan 2022 16:01:16 -0000	1.7
 +++ arch/amiga/md.c	23 Jun 2022 01:46:36 -0000
 @@ -100,6 +100,25 @@ md_post_disklabel(struct install_partiti
  	return true;
  }
  
 +#ifdef DISKLABEL_NO_ONDISK_VERIFY
 +/*
 + * hook to check if disklabel returned by readdisklabel(9) via DIOCGDINFO
 + * seems the default one, on ports that have no BSD disklabel on disks.
 + */
 +bool
 +md_disklabel_is_default(const struct disklabel *lp)
 +{
 +	bool maybe_default =
 +	    lp->d_npartitions == RAW_PART + 1 &&
 +	    lp->d_partitions[RAW_PART].p_size == 0x1fffffff &&
 +	    lp->d_partitions[0].p_size == lp->d_partitions[RAW_PART].p_size &&
 +	    lp->d_partitions[0].p_offset == 0;
 +	    lp->d_partitions[0].p_fstype == FS_BSDFFS;
 +
 +	return maybe_default;
 +}
 +#endif
 +
  /*
   * hook called after upgrade() or install() has finished setting
   * up the target disk but immediately before the user is given the
 Index: arch/x68k/md.c
 ===================================================================
 RCS file: /cvsroot/src/usr.sbin/sysinst/arch/x68k/md.c,v
 retrieving revision 1.12
 diff -u -p -d -r1.12 md.c
 --- arch/x68k/md.c	29 Jan 2022 16:01:21 -0000	1.12
 +++ arch/x68k/md.c	23 Jun 2022 01:46:36 -0000
 @@ -264,6 +264,25 @@ md_post_disklabel(struct install_partiti
  	return true;
  }
  
 +#ifdef DISKLABEL_NO_ONDISK_VERIFY
 +/*
 + * hook to check if disklabel returned by readdisklabel(9) via DIOCGDINFO
 + * seems the default one, on ports that have no BSD disklabel on disks.
 + */
 +bool
 +md_disklabel_is_default(const struct disklabel *lp)
 +{
 +	bool maybe_default =
 +	    lp->d_npartitions == RAW_PART + 1 &&
 +	    lp->d_partitions[0].p_size == lp->d_partitions[RAW_PART].p_size &&
 +	    lp->d_partitions[0].p_fstype == FS_UNUSED &&
 +	    lp->d_bbsize == 0 &&
 +	    lp->d_sbsize == 0;
 +
 +	return maybe_default;
 +}
 +#endif
 +
  /*
   * hook called after upgrade() or install() has finished setting
   * up the target disk but immediately before the user is given the
 
 ---
 
 Note several ports (including x68k) support both a BSD disklabel
 and port specific native OS derived label, so a check by disklabel -r
 might still be worth to try even in DISKLABEL_NO_ONDISK_VERIFY case.
 
 ---
 Izumi Tsutsui