Merge ~jefferyto/ubuntu/+source/grub2:lp1475620 into ubuntu/+source/grub2:ubuntu/devel

Proposed by Jeffery To
Status: Needs review
Proposed branch: ~jefferyto/ubuntu/+source/grub2:lp1475620
Merge into: ubuntu/+source/grub2:ubuntu/devel
Diff against target: 32 lines (+6/-2)
2 files modified
debian/changelog (+4/-0)
debian/grub-common.service (+2/-2)
Reviewer Review Type Date Requested Status
Paride Legovini (community) Needs Information
git-ubuntu import Pending
Review via email: mp+444580@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Paride Legovini (paride) wrote :

Hello,

If I understand it right this change already landed upstream (Debian) as part of this MR:

https://salsa.debian.org/grub-team/grub/-/merge_requests/33/commits

A package containing your fix is currently in experimental. My expectation is that the grub2 will be synced during the Mantic cycle (or merged leaving a minimal delta). This will bring your fix to Ubuntu. Therefore I am not sure a separate upload to Ubuntu is really worth it, also considering that the Importance of LP: #1475620 is Low. What do you think?

Speaking about your MP: you modified an already released d/changelog entry, while you should have added a new one (2.06-2ubuntu19). However, again, I am not convinced an upload is justified.

review: Needs Information
Revision history for this message
Jeffery To (jefferyto) wrote :

> Hello,
>
> If I understand it right this change already landed upstream (Debian) as part
> of this MR:
>
> https://salsa.debian.org/grub-team/grub/-/merge_requests/33/commits
>
> A package containing your fix is currently in experimental. My expectation is
> that the grub2 will be synced during the Mantic cycle (or merged leaving a
> minimal delta). This will bring your fix to Ubuntu. Therefore I am not sure a
> separate upload to Ubuntu is really worth it, also considering that the
> Importance of LP: #1475620 is Low. What do you think?
>
> Speaking about your MP: you modified an already released d/changelog entry,
> while you should have added a new one (2.06-2ubuntu19). However, again, I am
> not convinced an upload is justified.

Thanks for the review - I wasn't expecting my change to be merged in that MR, that was why I opened this proposal "upstream" (relative to that MR). I think this proposal can be closed/rejected.

Unmerged commits

35fa29f... by Jeffery To

grub-common.service: Fix service run before actual system sleep

The recordfail functionality is intended to detect if resuming after
hibernation failed:

* On boot, grub sets recordfail
* Kernel attempts to resume
* If resuming was successful, grub-common.service clears recordfail
* If resuming was unsuccessful, on next boot grub finds that recordfail
  is already set and adjusts the timeout value (to
  GRUB_RECORDFAIL_TIMEOUT or 30 seconds if GRUB_RECORDFAIL_TIMEOUT is
  not set)

However, grub-common.service is configured to run after sleep.target.
sleep.target is reached before actual system sleep occurs, and so
grub-common.service will very likely be run before the system shuts
down, i.e. before grub sets recordfail for the attempted resume.

This replaces sleep.target with hibernate.target, hybrid-sleep.target,
and suspend-then-hibernate.target; these targets are reached after the
system has resumed. (suspend.target is omitted as the system is not
fully shut down during suspend and so grub is not involved.)

Fixes LP: #1475620

c2d9ca0... by Julian Andres Klode

2.06-2ubuntu18 (patches unapplied)

Imported using git-ubuntu import.

3d8470c... by Julian Andres Klode

2.06-2ubuntu17 (patches unapplied)

Imported using git-ubuntu import.

4b727f7... by Julian Andres Klode

2.06-2ubuntu16 (patches unapplied)

Imported using git-ubuntu import.

12d118c... by Julian Andres Klode

2.06-2ubuntu15 (patches unapplied)

Imported using git-ubuntu import.

271b551... by Julian Andres Klode

2.06-2ubuntu12 (patches unapplied)

Imported using git-ubuntu import.

fa6ee71... by dann frazier

2.06-2ubuntu11 (patches unapplied)

Imported using git-ubuntu import.

4481c7e... by Chris Coulson

2.06-2ubuntu10 (patches unapplied)

Imported using git-ubuntu import.

4b653d7... by dann frazier

2.06-2ubuntu7 (patches unapplied)

Imported using git-ubuntu import.

059e913... by Julian Andres Klode

2.06-2ubuntu6 (patches unapplied)

Imported using git-ubuntu import.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 9e90c5a..52f094d 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,5 +1,9 @@
6 grub2 (2.06-2ubuntu18) mantic; urgency=medium
7
8+ [ Jeffery To ]
9+ * grub-common.service: Fix service run before actual system sleep (LP:
10+ #1475620)
11+
12 * Cherry-pick "RISC-V: Handle R_RISCV_CALL_PLT reloc" (LP: #2022379)
13 * Drop i386 from grub-efi-amd64* (LP: #2020907)
14 * Turn depends on grub-efi-amd64/arm64 unversioned
15diff --git a/debian/grub-common.service b/debian/grub-common.service
16index fcf5474..9172f1a 100644
17--- a/debian/grub-common.service
18+++ b/debian/grub-common.service
19@@ -1,6 +1,6 @@
20 [Unit]
21 Description=Record successful boot for GRUB
22-After=sleep.target
23+After=hibernate.target hybrid-sleep.target suspend-then-hibernate.target
24 ConditionPathExists=/boot/grub/grub.cfg
25
26 [Service]
27@@ -12,4 +12,4 @@ ExecStartPost=/bin/sh -c 'if grub-editenv /boot/grub/grubenv list | grep -q init
28 StandardOutput=kmsg
29
30 [Install]
31-WantedBy=multi-user.target sleep.target
32+WantedBy=multi-user.target hibernate.target hybrid-sleep.target suspend-then-hibernate.target

Subscribers

People subscribed via source and target branches