Re: [PATCH] wic: Introduce the --use-label partition parameter

[Kevin Hao <kexin.hao@windriver.com>]

[-- Attachment #1: Type: text/plain, Size: 2467 bytes --]

On Thu, Sep 06, 2018 at 09:27:39AM -0400, Tom Rini wrote:
> On Thu, Sep 06, 2018 at 09:11:20PM +0800, Kevin Hao wrote:
> 
> > We can use this parameter to make the wic use the label to name a
> > partition in /etc/fstab.
> > 
> > Signed-off-by: Kevin Hao <kexin.hao@windriver.com>
> 
> Reviewed-by: Tom Rini <trini@konsulko.com>
> 
> A few minor things below:
> 
> [snip]
> > @@ -945,6 +945,14 @@ DESCRIPTION
> >                          label is already in use by another filesystem,
> >                          a new label is created for the partition.
> >  
> > +         --use-label: This option is specific to wic. It makes wic to use the
> > +                      label in /etc/fstab to specify a partition. If the
> > +                      --use-label and --use-uuid are used at the same time,
> > +                      we prefer the uuid because it is less likely to cause
> > +                      name confliction. We don't support to use this parameter
> > +                      on a root partition since the kernel can't understand the
> > +                      LABEL syntax.
> 
> I think we should have the last sentence read more like:
> We don't support using this parameter on the root partition since it
> requires an initramfs to parse this value and we do not currently
> support that.
> 
> Only because while what you said is correct, this makes it clear what
> would be needed to support it.

Absolutely. I will rephrase it as what you suggested.

> 
> > @@ -197,10 +198,14 @@ class KickStart():
> >                                               (confpath, lineno, err))
> >                      if line.startswith('part'):
> >                          # SquashFS does not support UUID
> > -                        if parsed.fstype == 'squashfs' and parsed.use_uuid:
> > +                        if parsed.fstype == 'squashfs' and (parsed.use_uuid or parsed.use_label):
> >                              err = "%s:%d: SquashFS does not support UUID" \
> >                                    % (confpath, lineno)
> >                              raise KickStartError(err)
> 
> This hunk is going to conflict with Emmanuel's patch.  Further I think
> it should be a separate check on fstype == squashfs and (label &&
> use_label) with a label-specific error message.  Thanks!

Thanks for the notice. I will rebase on the Emmanuel's new version patch.

Thanks,
Kevin
> 
> -- 
> Tom



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 499 bytes --]