Re: [OSSTEST PATCH] preseed_base: Use "keep" NIC NamePolicy when "force-mac-address"

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Tue, Jun 18, 2024 at 12:04:05PM +0100, Andrew Cooper wrote:
> On 18/06/2024 10:44 am, Anthony PERARD wrote:
> > On Mon, Jun 17, 2024 at 04:34:09PM +0100, Andrew Cooper wrote:
> >> On 17/06/2024 3:40 pm, Anthony PERARD wrote:
> >>> diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> >>> index 3545f3fd..d974fea5 100644
> >>> --- a/Osstest/Debian.pm
> >>> +++ b/Osstest/Debian.pm
> >>> @@ -972,7 +972,19 @@ END
> >>>          # is going to be added to dom0's initrd, which is used by some 
> >>> guests
> >>>          # (created with ts-debian-install).
> >>>          preseed_hook_installscript($ho, $sfx,
> >>> -            '/usr/lib/base-installer.d/', '05ifnamepolicy', <<'END');
> >>> +            '/usr/lib/base-installer.d/', '05ifnamepolicy',
> >>> +            $ho->{Flags}{'force-mac-address'} ? <<'END' : <<'END');
> >> The conditional looks suspicious if both options are <<'END'.
> > That works fine, this pattern is already used in few places in osstest,
> > like here:
> > https://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=ts-host-install;h=0b6aaeeae228551064618abfa624321992a2eb2d;hb=HEAD#l240
> >     >  $ho->{Flags}{'force-mac-address'} ? <<END : <<END);
> >
> > Or even here:
> > https://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=ts-xen-build;h=c294a51eafc26e53b5417529b943224902870acf;hb=HEAD#l173
> >     > buildcmd_stamped_logged(600, 'xen', 'configure', <<END,<<END,<<END);
> >
> >> Doesn't this just write 70-eth-keep-policy.link unconditionally?
> > I've check that, on a different host, and the "mac" name policy is used
> > as expected, so the file "70-eth-keep-policy.link" isn't created on that
> > host.
>
> This is horrifying.  Given a construct which specifically lets you
> choose a semantically meaningful name, using END for all options is rude.
>
> Despite the pre-existing antipatterns, it would be better to turn this
> one into:
>
> $ho->{Flags}{'force-mac-address'} ? <<'END_KEEP' : <<'END_MAC');

I've push the patch with this change.

Cheers,

--

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech