[PATCH] xen: Create EFI_VENDOR directory

[PATCH] xen: Create EFI_VENDOR directory

[Jason Andryuk]

make install-xen fails when EFI_VENDOR is set (=fedora) with:
install: cannot create regular file '/home/user/xen/dist/install/boot/efi/efi/fedora/xen-4.15.0-rc.efi': No such file or directory

Create the EFI_VENDOR directory so xen.efi can be installed within.

This removes the need for Fedora and Qubes xen.spec files to manually
create the directory in advance.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
 xen/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/Makefile b/xen/Makefile
index 0b97e459e2..f372b44de3 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -289,6 +289,7 @@ _install: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
 		ln -sf $(T)-$(XEN_FULLVERSION).efi $(D)$(EFI_DIR)/$(T)-$(XEN_VERSION).efi; \
 		ln -sf $(T)-$(XEN_FULLVERSION).efi $(D)$(EFI_DIR)/$(T).efi; \
 		if [ -n '$(EFI_MOUNTPOINT)' -a -n '$(EFI_VENDOR)' ]; then \
+			$(INSTALL_DIR) $(D)$(EFI_MOUNTPOINT)/efi/$(EFI_VENDOR); \
 			$(INSTALL_DATA) $(TARGET).efi $(D)$(EFI_MOUNTPOINT)/efi/$(EFI_VENDOR)/$(T)-$(XEN_FULLVERSION).efi; \
 		elif [ "$(D)" = "$(patsubst $(shell cd $(XEN_ROOT) && pwd)/%,%,$(D))" ]; then \
 			echo 'EFI installation only partially done (EFI_VENDOR not set)' >&2; \
-- 
2.30.2

Re: for-4.15 [PATCH] xen: Create EFI_VENDOR directory

[Andrew Cooper]

On 22/03/2021 13:33, Jason Andryuk wrote:
> make install-xen fails when EFI_VENDOR is set (=fedora) with:
> install: cannot create regular file '/home/user/xen/dist/install/boot/efi/efi/fedora/xen-4.15.0-rc.efi': No such file or directory
>
> Create the EFI_VENDOR directory so xen.efi can be installed within.
>
> This removes the need for Fedora and Qubes xen.spec files to manually
> create the directory in advance.
>
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

This wants backporting, and should be considered for 4.15 at this point.

Re: [PATCH] xen: Create EFI_VENDOR directory

[Ian Jackson]

Jason Andryuk writes ("[PATCH] xen: Create EFI_VENDOR directory"):
> make install-xen fails when EFI_VENDOR is set (=fedora) with:
> install: cannot create regular file '/home/user/xen/dist/install/boot/efi/efi/fedora/xen-4.15.0-rc.efi': No such file or directory
> 
> Create the EFI_VENDOR directory so xen.efi can be installed within.
> 
> This removes the need for Fedora and Qubes xen.spec files to manually
> create the directory in advance.

I reviewed this.

Release-Acked-by: Ian Jackson <iwj@xenproject.org>

Presuming someone else also reviews it.  Andy, was your Acked-by a
Reviewed-by ?

Ian.

Re: [PATCH] xen: Create EFI_VENDOR directory

[Jan Beulich]

On 22.03.2021 14:33, Jason Andryuk wrote:
> make install-xen fails when EFI_VENDOR is set (=fedora) with:
> install: cannot create regular file '/home/user/xen/dist/install/boot/efi/efi/fedora/xen-4.15.0-rc.efi': No such file or directory
> 
> Create the EFI_VENDOR directory so xen.efi can be installed within.
> 
> This removes the need for Fedora and Qubes xen.spec files to manually
> create the directory in advance.

While I'm not strictly against, I'd like to point out that it was
deliberate to not create this directory here. I also didn't expect
anyone's xen.spec to do so. Instead I'd expect the distro to create
it during OS installation. If this was a bad assumption, I'd prefer
if the commit message here could point out why such an expectation
won't hold in general.

Jan

Re: [PATCH] xen: Create EFI_VENDOR directory

[Andrew Cooper]

On 22/03/2021 14:52, Jan Beulich wrote:
> On 22.03.2021 14:33, Jason Andryuk wrote:
>> make install-xen fails when EFI_VENDOR is set (=fedora) with:
>> install: cannot create regular file '/home/user/xen/dist/install/boot/efi/efi/fedora/xen-4.15.0-rc.efi': No such file or directory
>>
>> Create the EFI_VENDOR directory so xen.efi can be installed within.
>>
>> This removes the need for Fedora and Qubes xen.spec files to manually
>> create the directory in advance.
> While I'm not strictly against, I'd like to point out that it was
> deliberate to not create this directory here. I also didn't expect
> anyone's xen.spec to do so. Instead I'd expect the distro to create
> it during OS installation. If this was a bad assumption, I'd prefer
> if the commit message here could point out why such an expectation
> won't hold in general.

This reasoning is broken for anything other `make install DESTDIR=/` on
a live system.

It is incompatible with how RPM, deb, etc packages work.

~Andrew

Re: [PATCH] xen: Create EFI_VENDOR directory

[Jan Beulich]

On 22.03.2021 15:59, Andrew Cooper wrote:
> On 22/03/2021 14:52, Jan Beulich wrote:
>> On 22.03.2021 14:33, Jason Andryuk wrote:
>>> make install-xen fails when EFI_VENDOR is set (=fedora) with:
>>> install: cannot create regular file '/home/user/xen/dist/install/boot/efi/efi/fedora/xen-4.15.0-rc.efi': No such file or directory
>>>
>>> Create the EFI_VENDOR directory so xen.efi can be installed within.
>>>
>>> This removes the need for Fedora and Qubes xen.spec files to manually
>>> create the directory in advance.
>> While I'm not strictly against, I'd like to point out that it was
>> deliberate to not create this directory here. I also didn't expect
>> anyone's xen.spec to do so. Instead I'd expect the distro to create
>> it during OS installation. If this was a bad assumption, I'd prefer
>> if the commit message here could point out why such an expectation
>> won't hold in general.
> 
> This reasoning is broken for anything other `make install DESTDIR=/` on
> a live system.
> 
> It is incompatible with how RPM, deb, etc packages work.

I'm afraid I don't understand, for both of your statements. If distro
installation put in place the designated directory, there wouldn't be
any live system lacking it, and there wouldn't be any concern in the
packaging of any software.

To take a perhaps too extreme example - packages typically expect e.g.
/usr to exist as well, don't they?

Jan

Re: [PATCH] xen: Create EFI_VENDOR directory

[Jason Andryuk]

On Mon, Mar 22, 2021 at 11:15 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 22.03.2021 15:59, Andrew Cooper wrote:
> > On 22/03/2021 14:52, Jan Beulich wrote:
> >> On 22.03.2021 14:33, Jason Andryuk wrote:
> >>> make install-xen fails when EFI_VENDOR is set (=fedora) with:
> >>> install: cannot create regular file '/home/user/xen/dist/install/boot/efi/efi/fedora/xen-4.15.0-rc.efi': No such file or directory
> >>>
> >>> Create the EFI_VENDOR directory so xen.efi can be installed within.
> >>>
> >>> This removes the need for Fedora and Qubes xen.spec files to manually
> >>> create the directory in advance.
> >> While I'm not strictly against, I'd like to point out that it was
> >> deliberate to not create this directory here. I also didn't expect
> >> anyone's xen.spec to do so. Instead I'd expect the distro to create
> >> it during OS installation. If this was a bad assumption, I'd prefer
> >> if the commit message here could point out why such an expectation
> >> won't hold in general.

I didn't write this for an rpm xen.spec - I just cross referenced out
of curiosity.

> > This reasoning is broken for anything other `make install DESTDIR=/` on
> > a live system.
> >
> > It is incompatible with how RPM, deb, etc packages work.
>
> I'm afraid I don't understand, for both of your statements. If distro
> installation put in place the designated directory, there wouldn't be
> any live system lacking it, and there wouldn't be any concern in the
> packaging of any software.
>
> To take a perhaps too extreme example - packages typically expect e.g.
> /usr to exist as well, don't they?

But you have to install -d ${D}/usr before install ${D}/usr/file, right?

It's a surprising sequence to:
1) see 'EFI installation only partially done (EFI_VENDOR not set)'
2) set EFI_VENDOR
3) see xen.efi installation fail

I was working on a fedora system, and I was using `make && sh
install.sh` to install (but be sure to `rm -r dist/install/var/run`
since otherwise that'll break booting).  I wanted xen.efi to end up in
/boot/efi/EFI/fedora after running `sh install.sh`, and EFI_VENDOR
appeared to be the way to do it.  Again, it was surprising to enable
an option and then have it break.

Regards,
Jason

Re: [PATCH] xen: Create EFI_VENDOR directory

[Jan Beulich]

On 22.03.2021 16:36, Jason Andryuk wrote:
> On Mon, Mar 22, 2021 at 11:15 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 22.03.2021 15:59, Andrew Cooper wrote:
>>> On 22/03/2021 14:52, Jan Beulich wrote:
>>>> On 22.03.2021 14:33, Jason Andryuk wrote:
>>>>> make install-xen fails when EFI_VENDOR is set (=fedora) with:
>>>>> install: cannot create regular file '/home/user/xen/dist/install/boot/efi/efi/fedora/xen-4.15.0-rc.efi': No such file or directory
>>>>>
>>>>> Create the EFI_VENDOR directory so xen.efi can be installed within.
>>>>>
>>>>> This removes the need for Fedora and Qubes xen.spec files to manually
>>>>> create the directory in advance.
>>>> While I'm not strictly against, I'd like to point out that it was
>>>> deliberate to not create this directory here. I also didn't expect
>>>> anyone's xen.spec to do so. Instead I'd expect the distro to create
>>>> it during OS installation. If this was a bad assumption, I'd prefer
>>>> if the commit message here could point out why such an expectation
>>>> won't hold in general.
> 
> I didn't write this for an rpm xen.spec - I just cross referenced out
> of curiosity.
> 
>>> This reasoning is broken for anything other `make install DESTDIR=/` on
>>> a live system.
>>>
>>> It is incompatible with how RPM, deb, etc packages work.
>>
>> I'm afraid I don't understand, for both of your statements. If distro
>> installation put in place the designated directory, there wouldn't be
>> any live system lacking it, and there wouldn't be any concern in the
>> packaging of any software.
>>
>> To take a perhaps too extreme example - packages typically expect e.g.
>> /usr to exist as well, don't they?
> 
> But you have to install -d ${D}/usr before install ${D}/usr/file, right?

Sure, but I take it that about every package can rely on it to be
there, and not have to take care of creating it. There ought to
be an "owning" package for that directory, and that's the package
responsible for creating it. The same would then go for wherever
you want xen.efi to go.

> It's a surprising sequence to:
> 1) see 'EFI installation only partially done (EFI_VENDOR not set)'
> 2) set EFI_VENDOR
> 3) see xen.efi installation fail
> 
> I was working on a fedora system, and I was using `make && sh
> install.sh` to install (but be sure to `rm -r dist/install/var/run`
> since otherwise that'll break booting).  I wanted xen.efi to end up in
> /boot/efi/EFI/fedora after running `sh install.sh`, and EFI_VENDOR
> appeared to be the way to do it.  Again, it was surprising to enable
> an option and then have it break.

Well - the thing is that according to my understanding you don't simply
pick a directory name of your liking, but you use the one your distro
uses. Otherwise I consider it quite likely that upon next re-building
of the EFI partition your binary won't be put back.

Jan

Re: [PATCH] xen: Create EFI_VENDOR directory

[Andrew Cooper]

On 22/03/2021 15:15, Jan Beulich wrote:
> On 22.03.2021 15:59, Andrew Cooper wrote:
>> On 22/03/2021 14:52, Jan Beulich wrote:
>>> On 22.03.2021 14:33, Jason Andryuk wrote:
>>>> make install-xen fails when EFI_VENDOR is set (=fedora) with:
>>>> install: cannot create regular file '/home/user/xen/dist/install/boot/efi/efi/fedora/xen-4.15.0-rc.efi': No such file or directory
>>>>
>>>> Create the EFI_VENDOR directory so xen.efi can be installed within.
>>>>
>>>> This removes the need for Fedora and Qubes xen.spec files to manually
>>>> create the directory in advance.
>>> While I'm not strictly against, I'd like to point out that it was
>>> deliberate to not create this directory here. I also didn't expect
>>> anyone's xen.spec to do so. Instead I'd expect the distro to create
>>> it during OS installation. If this was a bad assumption, I'd prefer
>>> if the commit message here could point out why such an expectation
>>> won't hold in general.
>> This reasoning is broken for anything other `make install DESTDIR=/` on
>> a live system.
>>
>> It is incompatible with how RPM, deb, etc packages work.
> I'm afraid I don't understand, for both of your statements. If distro
> installation put in place the designated directory, there wouldn't be
> any live system lacking it, and there wouldn't be any concern in the
> packaging of any software.
>
> To take a perhaps too extreme example - packages typically expect e.g.
> /usr to exist as well, don't they?

No.  A buildroot starts out fully empty, by design.

The packaging environment (usually a chroot) invokes `make install
DESTDIR=/path/to/staging/root` so you don't interfere with any of the
tools inside the environment, and the resulting tar/cpio has the
buildroot stripped out of paths.

The failure being discussed here is the build within the packaging
environment, not the metadata which forms the final package.  Installing
a deb/rpm/etc will make directories as applicable.

~Andrew

Re: [PATCH] xen: Create EFI_VENDOR directory

[Jan Beulich]

On 22.03.2021 18:08, Andrew Cooper wrote:
> On 22/03/2021 15:15, Jan Beulich wrote:
>> On 22.03.2021 15:59, Andrew Cooper wrote:
>>> On 22/03/2021 14:52, Jan Beulich wrote:
>>>> On 22.03.2021 14:33, Jason Andryuk wrote:
>>>>> make install-xen fails when EFI_VENDOR is set (=fedora) with:
>>>>> install: cannot create regular file '/home/user/xen/dist/install/boot/efi/efi/fedora/xen-4.15.0-rc.efi': No such file or directory
>>>>>
>>>>> Create the EFI_VENDOR directory so xen.efi can be installed within.
>>>>>
>>>>> This removes the need for Fedora and Qubes xen.spec files to manually
>>>>> create the directory in advance.
>>>> While I'm not strictly against, I'd like to point out that it was
>>>> deliberate to not create this directory here. I also didn't expect
>>>> anyone's xen.spec to do so. Instead I'd expect the distro to create
>>>> it during OS installation. If this was a bad assumption, I'd prefer
>>>> if the commit message here could point out why such an expectation
>>>> won't hold in general.
>>> This reasoning is broken for anything other `make install DESTDIR=/` on
>>> a live system.
>>>
>>> It is incompatible with how RPM, deb, etc packages work.
>> I'm afraid I don't understand, for both of your statements. If distro
>> installation put in place the designated directory, there wouldn't be
>> any live system lacking it, and there wouldn't be any concern in the
>> packaging of any software.
>>
>> To take a perhaps too extreme example - packages typically expect e.g.
>> /usr to exist as well, don't they?
> 
> No.  A buildroot starts out fully empty, by design.
> 
> The packaging environment (usually a chroot) invokes `make install
> DESTDIR=/path/to/staging/root` so you don't interfere with any of the
> tools inside the environment, and the resulting tar/cpio has the
> buildroot stripped out of paths.
> 
> The failure being discussed here is the build within the packaging
> environment, not the metadata which forms the final package.  Installing
> a deb/rpm/etc will make directories as applicable.

Ah, I see. But then this _still_ isn't the right thing to do. In fact,
the package build and installation shouldn't put xen.efi in the EFI
partition _at all_. The build system doing so is for developers only,
so they don't need to invoke boot loader configuration every time they
rebuild and re-install. Hence the packaging build shouldn't set
EFI_VENDOR in the first place. There it instead should be a subsequent
boot loader re-configuration which picks up xen.efi from its install
location (under $(EFI_DIR)) and places it on the EFI partition.

With the above I take back my "I'm not strictly against" - I'm pretty
firmly against. If the warning emitted is deemed a problem, I'm sure
we can find a way to silence it.

Jan

Re: [PATCH] xen: Create EFI_VENDOR directory

[Jason Andryuk]

On Tue, Mar 23, 2021 at 3:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 22.03.2021 18:08, Andrew Cooper wrote:
> > On 22/03/2021 15:15, Jan Beulich wrote:
> >> On 22.03.2021 15:59, Andrew Cooper wrote:
> >>> On 22/03/2021 14:52, Jan Beulich wrote:
> >>>> On 22.03.2021 14:33, Jason Andryuk wrote:
> >>>>> make install-xen fails when EFI_VENDOR is set (=fedora) with:
> >>>>> install: cannot create regular file '/home/user/xen/dist/install/boot/efi/efi/fedora/xen-4.15.0-rc.efi': No such file or directory
> >>>>>
> >>>>> Create the EFI_VENDOR directory so xen.efi can be installed within.
> >>>>>
> >>>>> This removes the need for Fedora and Qubes xen.spec files to manually
> >>>>> create the directory in advance.
> >>>> While I'm not strictly against, I'd like to point out that it was
> >>>> deliberate to not create this directory here. I also didn't expect
> >>>> anyone's xen.spec to do so. Instead I'd expect the distro to create
> >>>> it during OS installation. If this was a bad assumption, I'd prefer
> >>>> if the commit message here could point out why such an expectation
> >>>> won't hold in general.
> >>> This reasoning is broken for anything other `make install DESTDIR=/` on
> >>> a live system.
> >>>
> >>> It is incompatible with how RPM, deb, etc packages work.
> >> I'm afraid I don't understand, for both of your statements. If distro
> >> installation put in place the designated directory, there wouldn't be
> >> any live system lacking it, and there wouldn't be any concern in the
> >> packaging of any software.
> >>
> >> To take a perhaps too extreme example - packages typically expect e.g.
> >> /usr to exist as well, don't they?
> >
> > No.  A buildroot starts out fully empty, by design.
> >
> > The packaging environment (usually a chroot) invokes `make install
> > DESTDIR=/path/to/staging/root` so you don't interfere with any of the
> > tools inside the environment, and the resulting tar/cpio has the
> > buildroot stripped out of paths.
> >
> > The failure being discussed here is the build within the packaging
> > environment, not the metadata which forms the final package.  Installing
> > a deb/rpm/etc will make directories as applicable.
>
> Ah, I see. But then this _still_ isn't the right thing to do. In fact,
> the package build and installation shouldn't put xen.efi in the EFI
> partition _at all_. The build system doing so is for developers only,
> so they don't need to invoke boot loader configuration every time they
> rebuild and re-install. Hence the packaging build shouldn't set
> EFI_VENDOR in the first place. There it instead should be a subsequent
> boot loader re-configuration which picks up xen.efi from its install
> location (under $(EFI_DIR)) and places it on the EFI partition.

On Fedora, RPMs drop EFI binaries directly into /boot/efi/EFI/fedora/.
grub, shim, fwupdate and xen are all packaged that way.  It seems
reasonable to have those important binaries tracked by the package
manager.

Does SuSE populate EFI_VENDOR from EFI_DIR when some boot loader
script is called?

I think Xen's population of EFI_VENDOR is sensible and fine.  Plus
it's *used*.  I don't see why it needs to be removed.  I was just
trying to fix a bug.

Regards,
Jason

Re: [PATCH] xen: Create EFI_VENDOR directory

[Jan Beulich]

On 23.03.2021 13:34, Jason Andryuk wrote:
> On Tue, Mar 23, 2021 at 3:23 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 22.03.2021 18:08, Andrew Cooper wrote:
>>> On 22/03/2021 15:15, Jan Beulich wrote:
>>>> On 22.03.2021 15:59, Andrew Cooper wrote:
>>>>> On 22/03/2021 14:52, Jan Beulich wrote:
>>>>>> On 22.03.2021 14:33, Jason Andryuk wrote:
>>>>>>> make install-xen fails when EFI_VENDOR is set (=fedora) with:
>>>>>>> install: cannot create regular file '/home/user/xen/dist/install/boot/efi/efi/fedora/xen-4.15.0-rc.efi': No such file or directory
>>>>>>>
>>>>>>> Create the EFI_VENDOR directory so xen.efi can be installed within.
>>>>>>>
>>>>>>> This removes the need for Fedora and Qubes xen.spec files to manually
>>>>>>> create the directory in advance.
>>>>>> While I'm not strictly against, I'd like to point out that it was
>>>>>> deliberate to not create this directory here. I also didn't expect
>>>>>> anyone's xen.spec to do so. Instead I'd expect the distro to create
>>>>>> it during OS installation. If this was a bad assumption, I'd prefer
>>>>>> if the commit message here could point out why such an expectation
>>>>>> won't hold in general.
>>>>> This reasoning is broken for anything other `make install DESTDIR=/` on
>>>>> a live system.
>>>>>
>>>>> It is incompatible with how RPM, deb, etc packages work.
>>>> I'm afraid I don't understand, for both of your statements. If distro
>>>> installation put in place the designated directory, there wouldn't be
>>>> any live system lacking it, and there wouldn't be any concern in the
>>>> packaging of any software.
>>>>
>>>> To take a perhaps too extreme example - packages typically expect e.g.
>>>> /usr to exist as well, don't they?
>>>
>>> No.  A buildroot starts out fully empty, by design.
>>>
>>> The packaging environment (usually a chroot) invokes `make install
>>> DESTDIR=/path/to/staging/root` so you don't interfere with any of the
>>> tools inside the environment, and the resulting tar/cpio has the
>>> buildroot stripped out of paths.
>>>
>>> The failure being discussed here is the build within the packaging
>>> environment, not the metadata which forms the final package.  Installing
>>> a deb/rpm/etc will make directories as applicable.
>>
>> Ah, I see. But then this _still_ isn't the right thing to do. In fact,
>> the package build and installation shouldn't put xen.efi in the EFI
>> partition _at all_. The build system doing so is for developers only,
>> so they don't need to invoke boot loader configuration every time they
>> rebuild and re-install. Hence the packaging build shouldn't set
>> EFI_VENDOR in the first place. There it instead should be a subsequent
>> boot loader re-configuration which picks up xen.efi from its install
>> location (under $(EFI_DIR)) and places it on the EFI partition.
> 
> On Fedora, RPMs drop EFI binaries directly into /boot/efi/EFI/fedora/.
> grub, shim, fwupdate and xen are all packaged that way.  It seems
> reasonable to have those important binaries tracked by the package
> manager.
> 
> Does SuSE populate EFI_VENDOR from EFI_DIR when some boot loader
> script is called?

Yes. And back at the time, when I consulted our EFI person, I was left
with the impression that this is the only reasonable approach. The
primary reason, as said, was that the EFI partition as a whole may get
rebuilt perhaps even from scratch at any point. Hence it's not
reasonable to expect package-managed files to live there. (This is
also expressed by us installing xen.efi into two places, which ought
to be a clear indication by itself that one of them is only to ease
things, not for packaging.)

As indicated - if the error is confusing, let's try to address it by,
perhaps, covering the case of the directory not existing also via the
"partially done" message. And the message, should that also be deemed
confusing, could maybe be dropped - I wouldn't be happy about it, but
if e.g. it was replaced by one reporting the secondary install step
was actually done, this would look acceptable.

Jan

Re: [PATCH] xen: Create EFI_VENDOR directory

[Ian Jackson]

Jan Beulich writes ("Re: [PATCH] xen: Create EFI_VENDOR directory"):
> On 23.03.2021 13:34, Jason Andryuk wrote:
...
> > On Fedora, RPMs drop EFI binaries directly into /boot/efi/EFI/fedora/.
> > grub, shim, fwupdate and xen are all packaged that way.  It seems
> > reasonable to have those important binaries tracked by the package
> > manager.
> > 
> > Does SuSE populate EFI_VENDOR from EFI_DIR when some boot loader
> > script is called?
> 
> Yes. And back at the time, when I consulted our EFI person, I was left
> with the impression that this is the only reasonable approach. The
> primary reason, as said, was that the EFI partition as a whole may get
> rebuilt perhaps even from scratch at any point. Hence it's not
> reasonable to expect package-managed files to live there.

I agree with this analysis but it is for people like Fedora to decide
how they want to build their packages.

There is also the case of ad-hoc packages (eg our "make debball")
which the user might reasonably choose to have dump things in the EFI
system partition.

Conversely, I see no downside to the mkdir.  Jan, is there some actual
harm in it ?  If not, we should be accomodating to people's build and
packaging strategies even if we don't entirely approve of them.

Thanks,
Ian.

Re: [PATCH] xen: Create EFI_VENDOR directory

[Jan Beulich]

On 23.03.2021 14:41, Ian Jackson wrote:
> Jan Beulich writes ("Re: [PATCH] xen: Create EFI_VENDOR directory"):
>> On 23.03.2021 13:34, Jason Andryuk wrote:
> ...
>>> On Fedora, RPMs drop EFI binaries directly into /boot/efi/EFI/fedora/.
>>> grub, shim, fwupdate and xen are all packaged that way.  It seems
>>> reasonable to have those important binaries tracked by the package
>>> manager.
>>>
>>> Does SuSE populate EFI_VENDOR from EFI_DIR when some boot loader
>>> script is called?
>>
>> Yes. And back at the time, when I consulted our EFI person, I was left
>> with the impression that this is the only reasonable approach. The
>> primary reason, as said, was that the EFI partition as a whole may get
>> rebuilt perhaps even from scratch at any point. Hence it's not
>> reasonable to expect package-managed files to live there.
> 
> I agree with this analysis but it is for people like Fedora to decide
> how they want to build their packages.
> 
> There is also the case of ad-hoc packages (eg our "make debball")
> which the user might reasonably choose to have dump things in the EFI
> system partition.

Well, it that's deemed reasonable, then perhaps yes. Albeit such
ad-hoc packaging could then also involve the creation of that dir.

> Conversely, I see no downside to the mkdir.  Jan, is there some actual
> harm in it ?  If not, we should be accomodating to people's build and
> packaging strategies even if we don't entirely approve of them.

"Actual harm" is relative: Nothing's going to break afaict. There'll
be a leftover dir from an install immediately followed by an
uninstall. I consider such okay for the purpose of the install step
that I did outline; I wouldn't consider it okay for a package
install/uninstall. But nothing worse, I guess. So bottom line - my
objection is not to be taken as a NAK. If everyone else wants the
change, then so be it.

Jan

Re: [PATCH] xen: Create EFI_VENDOR directory

[Michael Young]

On Tue, 23 Mar 2021, Ian Jackson wrote:

> Jan Beulich writes ("Re: [PATCH] xen: Create EFI_VENDOR directory"):
>> On 23.03.2021 13:34, Jason Andryuk wrote:
> ...
>>> On Fedora, RPMs drop EFI binaries directly into /boot/efi/EFI/fedora/.
>>> grub, shim, fwupdate and xen are all packaged that way.  It seems
>>> reasonable to have those important binaries tracked by the package
>>> manager.
>>>
>>> Does SuSE populate EFI_VENDOR from EFI_DIR when some boot loader
>>> script is called?
>>
>> Yes. And back at the time, when I consulted our EFI person, I was left
>> with the impression that this is the only reasonable approach. The
>> primary reason, as said, was that the EFI partition as a whole may get
>> rebuilt perhaps even from scratch at any point. Hence it's not
>> reasonable to expect package-managed files to live there.
>
> I agree with this analysis but it is for people like Fedora to decide
> how they want to build their packages.
>
> There is also the case of ad-hoc packages (eg our "make debball")
> which the user might reasonably choose to have dump things in the EFI
> system partition.
>
> Conversely, I see no downside to the mkdir.  Jan, is there some actual
> harm in it ?  If not, we should be accomodating to people's build and
> packaging strategies even if we don't entirely approve of them.

There is a request in https://bugzilla.redhat.com/show_bug.cgi?id=1750733 
for xen on Fedora to install the efi file elsewhere and then copy it to 
/boot/efi post install. I could change the current Fedora set up for 
Fedora 35 (which should have xen-4.15) if there is a good reason to do so. 
I am not sure how useful the xen.efi file in /boot/efi is anyway for the 
the Fedora set up as it will generally use a xen*.gz file in /boot via 
grub.

 	Michael Young

Re: [PATCH] xen: Create EFI_VENDOR directory

[Roman Shaposhnik]

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

On Tue, Mar 23, 2021 at 6:36 AM Jan Beulich <jbeulich@suse.com> wrote:

> On 23.03.2021 13:34, Jason Andryuk wrote:
> > On Tue, Mar 23, 2021 at 3:23 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 22.03.2021 18:08, Andrew Cooper wrote:
> >>> On 22/03/2021 15:15, Jan Beulich wrote:
> >>>> On 22.03.2021 15:59, Andrew Cooper wrote:
> >>>>> On 22/03/2021 14:52, Jan Beulich wrote:
> >>>>>> On 22.03.2021 14:33, Jason Andryuk wrote:
> >>>>>>> make install-xen fails when EFI_VENDOR is set (=fedora) with:
> >>>>>>> install: cannot create regular file
> '/home/user/xen/dist/install/boot/efi/efi/fedora/xen-4.15.0-rc.efi': No
> such file or directory
> >>>>>>>
> >>>>>>> Create the EFI_VENDOR directory so xen.efi can be installed within.
> >>>>>>>
> >>>>>>> This removes the need for Fedora and Qubes xen.spec files to
> manually
> >>>>>>> create the directory in advance.
> >>>>>> While I'm not strictly against, I'd like to point out that it was
> >>>>>> deliberate to not create this directory here. I also didn't expect
> >>>>>> anyone's xen.spec to do so. Instead I'd expect the distro to create
> >>>>>> it during OS installation. If this was a bad assumption, I'd prefer
> >>>>>> if the commit message here could point out why such an expectation
> >>>>>> won't hold in general.
> >>>>> This reasoning is broken for anything other `make install DESTDIR=/`
> on
> >>>>> a live system.
> >>>>>
> >>>>> It is incompatible with how RPM, deb, etc packages work.
> >>>> I'm afraid I don't understand, for both of your statements. If distro
> >>>> installation put in place the designated directory, there wouldn't be
> >>>> any live system lacking it, and there wouldn't be any concern in the
> >>>> packaging of any software.
> >>>>
> >>>> To take a perhaps too extreme example - packages typically expect e.g.
> >>>> /usr to exist as well, don't they?
> >>>
> >>> No.  A buildroot starts out fully empty, by design.
> >>>
> >>> The packaging environment (usually a chroot) invokes `make install
> >>> DESTDIR=/path/to/staging/root` so you don't interfere with any of the
> >>> tools inside the environment, and the resulting tar/cpio has the
> >>> buildroot stripped out of paths.
> >>>
> >>> The failure being discussed here is the build within the packaging
> >>> environment, not the metadata which forms the final package.
> Installing
> >>> a deb/rpm/etc will make directories as applicable.
> >>
> >> Ah, I see. But then this _still_ isn't the right thing to do. In fact,
> >> the package build and installation shouldn't put xen.efi in the EFI
> >> partition _at all_. The build system doing so is for developers only,
> >> so they don't need to invoke boot loader configuration every time they
> >> rebuild and re-install. Hence the packaging build shouldn't set
> >> EFI_VENDOR in the first place. There it instead should be a subsequent
> >> boot loader re-configuration which picks up xen.efi from its install
> >> location (under $(EFI_DIR)) and places it on the EFI partition.
> >
> > On Fedora, RPMs drop EFI binaries directly into /boot/efi/EFI/fedora/.
> > grub, shim, fwupdate and xen are all packaged that way.  It seems
> > reasonable to have those important binaries tracked by the package
> > manager.
> >
> > Does SuSE populate EFI_VENDOR from EFI_DIR when some boot loader
> > script is called?
>
> Yes. And back at the time, when I consulted our EFI person, I was left
> with the impression that this is the only reasonable approach. The
> primary reason, as said, was that the EFI partition as a whole may get
> rebuilt perhaps even from scratch at any point. Hence it's not
> reasonable to expect package-managed files to live there. (This is
> also expressed by us installing xen.efi into two places, which ought
> to be a clear indication by itself that one of them is only to ease
> things, not for packaging.)
>

Big +1 to the above -- in running our own distro we've come to appreciate
that very point -- EFI partition is basically a cache and the source of
truth
is always elsewhere.

Thanks,
Roman.

[-- Attachment #2: Type: text/html, Size: 5406 bytes --]

Re: [PATCH] xen: Create EFI_VENDOR directory

[Roman Shaposhnik]

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

On Tue, Mar 23, 2021 at 8:59 AM Michael Young <m.a.young@durham.ac.uk>
wrote:

>
>
> On Tue, 23 Mar 2021, Ian Jackson wrote:
>
> > Jan Beulich writes ("Re: [PATCH] xen: Create EFI_VENDOR directory"):
> >> On 23.03.2021 13:34, Jason Andryuk wrote:
> > ...
> >>> On Fedora, RPMs drop EFI binaries directly into /boot/efi/EFI/fedora/.
> >>> grub, shim, fwupdate and xen are all packaged that way.  It seems
> >>> reasonable to have those important binaries tracked by the package
> >>> manager.
> >>>
> >>> Does SuSE populate EFI_VENDOR from EFI_DIR when some boot loader
> >>> script is called?
> >>
> >> Yes. And back at the time, when I consulted our EFI person, I was left
> >> with the impression that this is the only reasonable approach. The
> >> primary reason, as said, was that the EFI partition as a whole may get
> >> rebuilt perhaps even from scratch at any point. Hence it's not
> >> reasonable to expect package-managed files to live there.
> >
> > I agree with this analysis but it is for people like Fedora to decide
> > how they want to build their packages.
> >
> > There is also the case of ad-hoc packages (eg our "make debball")
> > which the user might reasonably choose to have dump things in the EFI
> > system partition.
> >
> > Conversely, I see no downside to the mkdir.  Jan, is there some actual
> > harm in it ?  If not, we should be accomodating to people's build and
> > packaging strategies even if we don't entirely approve of them.
>
> There is a request in https://bugzilla.redhat.com/show_bug.cgi?id=1750733
> for xen on Fedora to install the efi file elsewhere and then copy it to
> /boot/efi post install. I could change the current Fedora set up for
> Fedora 35 (which should have xen-4.15) if there is a good reason to do so.
> I am not sure how useful the xen.efi file in /boot/efi is anyway for the
> the Fedora set up as it will generally use a xen*.gz file in /boot via
> grub.
>

FWIW: /boot as the source of truth and EFI partition as a cache for that is
also what we've settled on in EVE distro.

As a somewhat unrelated side-note: on ARM at least, with u-boot playing the
role of UEFI more and more -- we can actually pick EFI payloads straight
from source of truth and completely by-pass EFI partition altogether (which
is fine -- it is, after all, only a cache).

Thanks,
Roman.

[-- Attachment #2: Type: text/html, Size: 3120 bytes --]