From nobody Mon Apr 15 12:21:59 2024 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4VJ5s43DKSz5GDwY; Mon, 15 Apr 2024 12:22:16 +0000 (UTC) (envelope-from fernando.apesteguia@gmail.com) Received: from mail-lf1-f52.google.com (mail-lf1-f52.google.com [209.85.167.52]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4VJ5s41DQrz4Gnq; Mon, 15 Apr 2024 12:22:16 +0000 (UTC) (envelope-from fernando.apesteguia@gmail.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-lf1-f52.google.com with SMTP id 2adb3069b0e04-518e5a74702so543823e87.1; Mon, 15 Apr 2024 05:22:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713183733; x=1713788533; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=1+8hOKqyizpx42VDwEBRlK3dr3KFqkxARx+8CyJeBpw=; b=W5L21ak9eZ7YS83OhcSnMjxp4q+wJ8xs5HD1kTYcqa52cbqWQj1LCkv3/i1K2m27ut Cec+hV+p7b0DLtgSJb1BiLddRuM2JodUvwEPUwZcS0eoxhWOwH6T20StIce/i7xZM0J8 aFJ/MJwjyrM6FWTtp5/UyPc+ISubqrl1qbZH5cXJHq090fnpm6qoMitZEWumDtnieARN xPJ4FPw2A4C8CNdkrdpnNmUNemSDG+YMRlXDpr4kkRF90LSgcCc9c7VvYhikzdOjUbwy e2Z7O5YzFRcSAA2OEesMhD6iY8Z72nFHC/rS0GZ8aUhlj7LU02vtw5DQ8gkQUdLG/DyT zQBQ== X-Forwarded-Encrypted: i=1; AJvYcCXmdrY84xq/TD0F/cNIRHgP3zVnetK8N2Tom6KYYV3P7kW3nteVTFwRYaQgWU9jmfoT7WH43DOOYocAnuve/gDiDLIcQ9z3pbIk3jfvYCG+wlDMN9JlIgZrN2kIy+uh/1lkwFq5E6iwKIT5BRa7RvAY1g== X-Gm-Message-State: AOJu0Yzwt3W6jhe1zuZSEX4Y+5bRXeAcv+BQ2QiEncWE321ujkHAMv7z Te4P9EkFXDbJlBErBs0dmHV1phMAYtbFIixZ6m2vBzt7TRJPiwjxMi+7yRdL X-Google-Smtp-Source: AGHT+IHUqoNbB6LuhdJ1l0adwfh6tviIOEFzBlSEKBDe+STy2t5SW9MGEP4eay9npXgSwLNMODu+WQ== X-Received: by 2002:a05:6512:927:b0:516:d922:a06f with SMTP id f7-20020a056512092700b00516d922a06fmr2551884lft.23.1713183733100; Mon, 15 Apr 2024 05:22:13 -0700 (PDT) Received: from mail-lf1-f47.google.com (mail-lf1-f47.google.com. [209.85.167.47]) by smtp.gmail.com with ESMTPSA id u24-20020ac25198000000b0051593cfb556sm1249952lfi.239.2024.04.15.05.22.12 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 15 Apr 2024 05:22:12 -0700 (PDT) Received: by mail-lf1-f47.google.com with SMTP id 2adb3069b0e04-516d68d7a8bso2628517e87.1; Mon, 15 Apr 2024 05:22:12 -0700 (PDT) X-Forwarded-Encrypted: i=1; AJvYcCUAsrFZltay0447wtoLjycFcW8IbSdSYFGJwX733S7et6Dp/Lm9Ihd4tCgdQzeiMjboSSvzUApHzWGuqgr1CF/Ea4tQgfiMMZz5VrIJb7fIFQ0cxfmGdIzm9eQ6+vxYPva3T3soQTV6E4mn+e/TLQ5m3A== X-Received: by 2002:a05:6512:1091:b0:518:c87b:6901 with SMTP id j17-20020a056512109100b00518c87b6901mr1220352lfg.34.1713183732093; Mon, 15 Apr 2024 05:22:12 -0700 (PDT) List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 References: <202404141746.43EHkobl002927@gitrepo.freebsd.org> <6114cce6-dd6e-478f-97af-1a2813f29332@FreeBSD.org> In-Reply-To: <6114cce6-dd6e-478f-97af-1a2813f29332@FreeBSD.org> From: =?UTF-8?Q?Fernando_Apestegu=C3=ADa?= Date: Mon, 15 Apr 2024 14:21:59 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: git: bc0c6c9cf3a9 - main - freebsd-update: Add check for kernel modules To: Kyle Evans Cc: src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Content-Type: multipart/alternative; boundary="000000000000c96a17061621aba3" X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; TAGGED_FROM(0.00)[]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US] X-Rspamd-Queue-Id: 4VJ5s41DQrz4Gnq --000000000000c96a17061621aba3 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, Apr 15, 2024 at 5:44=E2=80=AFAM Kyle Evans wro= te: > On 4/14/24 12:46, Fernando Apestegu=C3=ADa wrote: > > The branch main has been updated by fernape: > > > > URL: > https://cgit.FreeBSD.org/src/commit/?id=3Dbc0c6c9cf3a9f9a54dbdd92dd8f1f65= ff8092d17 > > > > commit bc0c6c9cf3a9f9a54dbdd92dd8f1f65ff8092d17 > > Author: Fernando Apestegu=C3=ADa > > AuthorDate: 2023-04-19 16:08:47 +0000 > > Commit: Fernando Apestegu=C3=ADa > > CommitDate: 2024-04-14 17:46:23 +0000 > > > > freebsd-update: Add check for kernel modules > > > > People get confused when some software (VirtualBox, etc) does not > work as > > expected (or at all) after a major upgrade. > > > > We have a nice way to deal with this when using sources, namely > including > > PORTS_MODULES in /etc/make.conf, but we lack something similar for > binary > > updates. > > > > This patch retrieves a list of kernel modules installed from > packages and > > advises the user to recompile from ports to avoid problems. > > > > Approved by: zlei@ > > Differential Revision: https://reviews.freebsd.org/D39695 > > --- > > usr.sbin/freebsd-update/freebsd-update.sh | 58 > +++++++++++++++++++++++++++++++ > > 1 file changed, 58 insertions(+) > > > > diff --git a/usr.sbin/freebsd-update/freebsd-update.sh > b/usr.sbin/freebsd-update/freebsd-update.sh > > index 4a6a8d78330b..d1cd46963a6c 100644 > > --- a/usr.sbin/freebsd-update/freebsd-update.sh > > +++ b/usr.sbin/freebsd-update/freebsd-update.sh > > @@ -655,6 +655,63 @@ fetch_setup_verboselevel () { > > esac > > } > > > > +# Check if there are any kernel modules installed from ports. > > +# In that case warn the user that a rebuild from ports (i.e. not from > > +# packages) might need necessary for the modules to work in the new > release. > > +upgrade_check_kmod_ports() { > > + local mod_name > > + local modules > > + local pattern > > + local pkg_name > > + local port_name > > + local report > > + local w > > + > > + if ! command -v pkg >/dev/null; then > > + echo "Skipping kernel modules check. pkg(8) not present." > > + return > > + fi > > + > > + # Most modules are in /boot/modules but we should actually look > > + # in every path configured in module_path > > + search_files=3D"/boot/defaults/loader.conf /boot/loader.conf" > > Woof... this is inherently quite fragile, and it completely ignores how > loader.conf are actually processed. The final module_path will always > get passed to the kernel via kenv(1) and exposed as kern.module_path, > please strongly consider just grabbing it from one of them instea . > Thanks for the feedback. kern.module_path seems like the way to do this indeed. Here is a review: https://reviews.freebsd.org/D44797 Cheers. > > Thanks, > > Kyle Evans > --000000000000c96a17061621aba3 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Mon, Apr 15, 2024 at 5:44=E2=80=AF= AM Kyle Evans <kevans@freebsd.org<= /a>> wrote:
O= n 4/14/24 12:46, Fernando Apestegu=C3=ADa wrote:
> The branch main has been updated by fernape:
>
> URL:
https://= cgit.FreeBSD.org/src/commit/?id=3Dbc0c6c9cf3a9f9a54dbdd92dd8f1f65ff8092d17<= /a>
>
> commit bc0c6c9cf3a9f9a54dbdd92dd8f1f65ff8092d17
> Author:=C2=A0 =C2=A0 =C2=A0Fernando Apestegu=C3=ADa <fernape@FreeBS= D.org>
> AuthorDate: 2023-04-19 16:08:47 +0000
> Commit:=C2=A0 =C2=A0 =C2=A0Fernando Apestegu=C3=ADa <fernape@FreeBS= D.org>
> CommitDate: 2024-04-14 17:46:23 +0000
>
>=C2=A0 =C2=A0 =C2=A0 freebsd-update: Add check for kernel modules
>=C2=A0 =C2=A0 =C2=A0
>=C2=A0 =C2=A0 =C2=A0 People get confused when some software (VirtualBox= , etc) does not work as
>=C2=A0 =C2=A0 =C2=A0 expected (or at all) after a major upgrade.
>=C2=A0 =C2=A0 =C2=A0
>=C2=A0 =C2=A0 =C2=A0 We have a nice way to deal with this when using so= urces, namely including
>=C2=A0 =C2=A0 =C2=A0 PORTS_MODULES in /etc/make.conf, but we lack somet= hing similar for binary
>=C2=A0 =C2=A0 =C2=A0 updates.
>=C2=A0 =C2=A0 =C2=A0
>=C2=A0 =C2=A0 =C2=A0 This patch retrieves a list of kernel modules inst= alled from packages and
>=C2=A0 =C2=A0 =C2=A0 advises the user to recompile from ports to avoid = problems.
>=C2=A0 =C2=A0 =C2=A0
>=C2=A0 =C2=A0 =C2=A0 Approved by:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 zlei@
>=C2=A0 =C2=A0 =C2=A0 Differential Revision:=C2=A0
https://revi= ews.freebsd.org/D39695
> ---
>=C2=A0 =C2=A0usr.sbin/freebsd-update/freebsd-update.sh | 58 +++++++++++= ++++++++++++++++++++
>=C2=A0 =C2=A01 file changed, 58 insertions(+)
>
> diff --git a/usr.sbin/freebsd-update/freebsd-update.sh b/usr.sbin/free= bsd-update/freebsd-update.sh
> index 4a6a8d78330b..d1cd46963a6c 100644
> --- a/usr.sbin/freebsd-update/freebsd-update.sh
> +++ b/usr.sbin/freebsd-update/freebsd-update.sh
> @@ -655,6 +655,63 @@ fetch_setup_verboselevel () {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0esac
>=C2=A0 =C2=A0}
>=C2=A0 =C2=A0
> +# Check if there are any kernel modules installed from ports.
> +# In that case warn the user that a rebuild from ports (i.e. not from=
> +# packages) might need necessary for the modules to work in the new r= elease.
> +upgrade_check_kmod_ports() {
> +=C2=A0 =C2=A0 =C2=A0local mod_name
> +=C2=A0 =C2=A0 =C2=A0local modules
> +=C2=A0 =C2=A0 =C2=A0local pattern
> +=C2=A0 =C2=A0 =C2=A0local pkg_name
> +=C2=A0 =C2=A0 =C2=A0local port_name
> +=C2=A0 =C2=A0 =C2=A0local report
> +=C2=A0 =C2=A0 =C2=A0local w
> +
> +=C2=A0 =C2=A0 =C2=A0if ! command -v pkg >/dev/null; then
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0echo "Skipping k= ernel modules check. pkg(8) not present."
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return
> +=C2=A0 =C2=A0 =C2=A0fi
> +
> +=C2=A0 =C2=A0 =C2=A0# Most modules are in /boot/modules but we should= actually look
> +=C2=A0 =C2=A0 =C2=A0# in every path configured in module_path
> +=C2=A0 =C2=A0 =C2=A0search_files=3D"/boot/defaults/loader.conf /= boot/loader.conf"

Woof... this is inherently quite fragile, and it completely ignores how loader.conf are actually processed.=C2=A0 The final module_path will always=
get passed to the kernel via kenv(1) and exposed as kern.module_path,
please strongly consider just grabbing it from one of them instea .

Thanks for the feedback. kern.module_path seem= s like the way to do this indeed.

Here is a re= view:
=C2=A0
Cheers.
=

Thanks,

Kyle Evans
--000000000000c96a17061621aba3--