On Sat Aug 13, 2022 at 12:39 AM +04, Stefan Sperling wrote:
> Use raid1c-specific meta-data while looking for a key disk that
> belongs to a RAID 1C volume.
>
> By dumb luck this is only a cosmetic issue, because struct layout
> happens to put the field in the same place.

Good catch.  OK kn
One suggestion inline.

>  
> diff a87e94ce1617958c99b63ed0a056e46650a27375 
> ba39970f6365aeeb1b486101f6573c60b7f88573
> commit - a87e94ce1617958c99b63ed0a056e46650a27375
> commit + ba39970f6365aeeb1b486101f6573c60b7f88573
> blob - 762f6ee57d5e7902c097d71830fea6e63080e7b5
> blob + 737fafbaad517c55293c69f0a6ddcb8fbb720c83
> --- sys/dev/softraid.c
> +++ sys/dev/softraid.c
> @@ -2593,10 +2593,13 @@ sr_ioctl_vol(struct sr_softc *sc, struct bioc_vol *bv)
>               bv->bv_nodisk = sd->sd_meta->ssdi.ssd_chunk_no;
>  
>  #ifdef CRYPTO
> -             if ((sd->sd_meta->ssdi.ssd_level == 'C' ||
> -                 sd->sd_meta->ssdi.ssd_level == 0x1C) &&
> +             if (sd->sd_meta->ssdi.ssd_level == 'C' &&
>                   sd->mds.mdd_crypto.key_disk != NULL)
>                       bv->bv_nodisk++;
> +
> +             if (sd->sd_meta->ssdi.ssd_level == 0x1C &&

I'd use either `else if' here or merge them into
                if ((C && mdd_crypto.key_disk) ||
                    (1C && sr1c_crypto.key_disk))

> +                 sd->mds.mdd_raid1c.sr1c_crypto.key_disk != NULL)
> +                     bv->bv_nodisk++;
>  #endif
>               if (bv->bv_status == BIOC_SVREBUILD)
>                       bv->bv_percent = sr_rebuild_percent(sd);
> @@ -2650,10 +2653,13 @@ sr_ioctl_disk(struct sr_softc *sc, struct bioc_disk *b
>                       src = sd->sd_vol.sv_chunks[bd->bd_diskid];
>  #ifdef CRYPTO
>               else if (bd->bd_diskid == sd->sd_meta->ssdi.ssd_chunk_no &&
> -                 (sd->sd_meta->ssdi.ssd_level == 'C' ||
> -                 sd->sd_meta->ssdi.ssd_level == 0x1C) &&
> +                 sd->sd_meta->ssdi.ssd_level == 'C' &&
>                   sd->mds.mdd_crypto.key_disk != NULL)
>                       src = sd->mds.mdd_crypto.key_disk;
> +             else if (bd->bd_diskid == sd->sd_meta->ssdi.ssd_chunk_no &&
> +                 sd->sd_meta->ssdi.ssd_level == 0x1C &&
> +                 sd->mds.mdd_raid1c.sr1c_crypto.key_disk != NULL)
> +                     src = sd->mds.mdd_crypto.key_disk;

Since this is the same plus a third condition, `else if' in both places
seems cleanest here.

>  #endif
>               else
>                       break;

Reply via email to