Re: [libvirt PATCH 004/351] meson: drop loader_nvram build option

On Thu, Jul 16, 2020 at 04:23:31PM +0200, Pavel Hrdina wrote: > On Thu, Jul 16, 2020 at 04:12:31PM +0200, Ján Tomko wrote: > > On a Thursday in 2020, Pavel Hrdina wrote: > > > With the switch to meson it should be safe to drop this build option. > > > We don't have any deprecation policy so let's take the opportunity when > > > everything will break current users. > > > > > > > The commit message does not mention that this option is deprecated > > or why it's safe to drop it. > > > > Also, this is another patch that can be pushed separately. > > > > > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > > > --- > > > configure.ac | 3 --- > > > m4/virt-loader-nvram.m4 | 49 ----------------------------------------- > > > 2 files changed, 52 deletions(-) > > > delete mode 100644 m4/virt-loader-nvram.m4 > > > > > > diff --git a/configure.ac b/configure.ac > > > index 97dbfe9ec2b..23074d3badd 100644 > > > --- a/configure.ac > > > +++ b/configure.ac > > > @@ -515,7 +515,6 @@ LIBVIRT_ARG_NUMAD > > > LIBVIRT_ARG_INIT_SCRIPT > > > LIBVIRT_ARG_CHRDEV_LOCK_FILES > > > LIBVIRT_ARG_DEFAULT_EDITOR > > > -LIBVIRT_ARG_LOADER_NVRAM > > > LIBVIRT_ARG_LOGIN_SHELL > > > LIBVIRT_ARG_HOST_VALIDATE > > > LIBVIRT_ARG_TLS_PRIORITY > > > @@ -528,7 +527,6 @@ LIBVIRT_CHECK_NUMAD > > > LIBVIRT_CHECK_INIT_SCRIPT > > > LIBVIRT_CHECK_CHRDEV_LOCK_FILES > > > LIBVIRT_CHECK_DEFAULT_EDITOR > > > -LIBVIRT_CHECK_LOADER_NVRAM > > > LIBVIRT_CHECK_LOGIN_SHELL > > > LIBVIRT_CHECK_HOST_VALIDATE > > > LIBVIRT_CHECK_TLS_PRIORITY > > > @@ -1047,7 +1045,6 @@ LIBVIRT_RESULT_NUMAD > > > LIBVIRT_RESULT_INIT_SCRIPT > > > LIBVIRT_RESULT_CHRDEV_LOCK_FILES > > > LIBVIRT_RESULT_DEFAULT_EDITOR > > > -LIBVIRT_RESULT_LOADER_NVRAM > > > LIBVIRT_RESULT_LOGIN_SHELL > > > LIBVIRT_RESULT_HOST_VALIDATE > > > LIBVIRT_RESULT_TLS_PRIORITY > > > diff --git a/m4/virt-loader-nvram.m4 b/m4/virt-loader-nvram.m4 > > > deleted file mode 100644 > > > index ed2ae0cf27b..00000000000 > > > --- a/m4/virt-loader-nvram.m4 > > > +++ /dev/null > > > @@ -1,49 +0,0 @@ > > > -dnl The loader:nvram list check > > > -dnl > > > -dnl Copyright (C) 2016 Red Hat, Inc. > > > -dnl > > > -dnl This library is free software; you can redistribute it and/or > > > -dnl modify it under the terms of the GNU Lesser General Public > > > -dnl License as published by the Free Software Foundation; either > > > -dnl version 2.1 of the License, or (at your option) any later version. > > > -dnl > > > -dnl This library is distributed in the hope that it will be useful, > > > -dnl but WITHOUT ANY WARRANTY; without even the implied warranty of > > > -dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > > -dnl Lesser General Public License for more details. > > > -dnl > > > -dnl You should have received a copy of the GNU Lesser General Public > > > -dnl License along with this library. If not, see > > > -dnl <http://www.gnu.org/licenses/>;. > > > -dnl > > > - > > > -AC_DEFUN([LIBVIRT_ARG_LOADER_NVRAM], [ > > > - LIBVIRT_ARG_WITH([LOADER_NVRAM], > > > - [Pass list of pairs of <loader>:<nvram> paths. > > > - Both pairs and list items are separated by a colon.], > > > - ['']) > > > -]) > > > - > > > -AC_DEFUN([LIBVIRT_CHECK_LOADER_NVRAM], [ > > > - if test "x$with_loader_nvram" != "xno" && \ > > > - test "x$with_loader_nvram" != "x" ; then > > > - l=$(echo $with_loader_nvram | tr ':' '\n' | wc -l) > > > - if test $(expr $l % 2) -ne 0 ; then > > > - AC_MSG_ERROR([Malformed --with-loader-nvram argument]) > > > - elif test $l -gt 0 ; then > > > - AC_MSG_WARN([Note that --with-loader-nvram is obsolete and will be removed soon]) > > > - fi > > > - AC_DEFINE_UNQUOTED([DEFAULT_LOADER_NVRAM], ["$with_loader_nvram"], > > > - [List of loader:nvram pairs]) > > > > This makes any #ifdef DEFAULT_LOADER_NVRAM dead code. > > > > Is libxl okay with this change? There does not seem to be any reading of > > firmware descriptor files in libxl code. > > Good point, I'm OK with adding this option into meson as well, it's > fairly simple. Michal, what do you think about it? I think libxl needs to be updated to use the firmware descriptors for the same reason we use them in QEMU. Until that happens though, I think we have to keep the args, as we can't arbitrarily break libxl just because QEMU has been converted.