Merge ~aciba/ubuntu/+source/hibagent:lp1896638-path-to-swapfile-by-uuid-kinetic into ubuntu/+source/hibagent:ubuntu/kinetic-devel

Proposed by Alberto Contreras
Status: Merged
Merge reported by: Robie Basak
Merged at revision: 9bbca7aec6433edc5db1740ec56d6f792d5d51cc
Proposed branch: ~aciba/ubuntu/+source/hibagent:lp1896638-path-to-swapfile-by-uuid-kinetic
Merge into: ubuntu/+source/hibagent:ubuntu/kinetic-devel
Diff against target: 149 lines (+121/-0)
4 files modified
debian/changelog (+11/-0)
debian/patches/lp1896638-set-resume-device-by-partition-uuid.patch (+33/-0)
debian/patches/lp2013336-do-not-modify-GRUB-config-on-GRUB2-systems.patch (+75/-0)
debian/patches/series (+2/-0)
Reviewer Review Type Date Requested Status
Simon Chopin (community) Approve
Review via email: mp+439728@code.launchpad.net

Description of the change

Cherry-picked upstream fixes 43636fcf and 0f995e80 in pkg/ubuntu/kinetic-devel:

  * d/p/lp1896638-set-resume-device-by-partition-uuid: Set resume device
    by PARTUUID instead of by name. Thanks to Tony Nie <email address hidden>.
    (LP: #1896638)
  * d/p/lp2013336-do-not-modify-GRUB-config-on-GRUB2-systems: Do not attempt to
    modify non existent GRUB config file. Thanks to Robert Schweikert
    <email address hidden>. (LP: #2013336)

Please tag & sponsor.

PPA: ppa:aciba/hibagent-sru-lp1896638-swapfile

Steps to test:

* Launch an EC2 instance
# sudo apt update && sudo apt install -y hibagent
# sudo /usr/bin/enable-ec2-spot-hibernation
# journalctl -xeu hibagent.service | grep Errno
Failed to initialize swap, reason: [Errno 2] No such file or directory: '/boot/grub/menu.lst'

# sudo apt-add-repository -ys ppa:aciba/hibagent-sru-lp1896638-swapfile
# sudo apt install -y hibagent
# sudo /usr/bin/enable-ec2-spot-hibernation

* Make sure the agent is active and running:
# systemctl status hibagent

* Make sure the swap is correctly configured:
$ swapon --show
NAME TYPE SIZE USED PRIO
/swap file 3.9G 512K -2

* Make sure the resume partition is defined by uuid:
$ grep no_console /boot/grub/grub.cfg
... no_console_suspend=1 resume_offset=780288 resume=PARTUUID=14aaf7d8-41e2-43b2-b149-a402305a5136 ...

To post a comment you must log in.
Revision history for this message
Simon Chopin (schopin) wrote :

I have a small nitpick on the patches metadata and a small changelog issue that needs fixing, otherwise LGTM.

As I don't have access to an EC2 instance, can you confirm to me that you actually tested your changes using the steps outlined in your protocol?

review: Needs Fixing
f0937cd... by Alberto Contreras

  * d/p/lp1896638-set-resume-device-by-partition-uuid: Set resume device
    by PARTUUID instead of by name. Thanks to Tony Nie <email address hidden>.
    (LP: #1896638)

c412ac1... by Alberto Contreras

  * d/p/lp2013336-do-not-modify-GRUB-config-on-GRUB2-systems: Do not attempt to
    modify non existent GRUB config file. Thanks to Robert Schweikert
    <email address hidden>. (LP: #2013336)

9bbca7a... by Alberto Contreras

changelog

Revision history for this message
Alberto Contreras (aciba) wrote :

Thanks for pointing out the issues and the review. I have fixed them.

I confirm I have actually tested the changes using the steps outlined in my protocol for B, F, J and K MRs.

Revision history for this message
Simon Chopin (schopin) wrote :

Uploaded, with an adjustment to the version number.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index ff27dd5..4c28f30 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,14 @@
1hibagent (1.0.1-0ubuntu3.22.10.1) kinetic; urgency=medium
2
3 * d/p/lp1896638-set-resume-device-by-partition-uuid: Set resume device
4 by PARTUUID instead of by name. Thanks to Tony Nie <zirann@amazon.com>.
5 (LP: #1896638)
6 * d/p/lp2013336-do-not-modify-GRUB-config-on-GRUB2-systems: Do not attempt to
7 modify non existent GRUB config file. Thanks to Robert Schweikert
8 <rjschwei@suse.com>. (LP: #2013336)
9
10 -- Alberto Contreras <alberto.contreras@canonical.com> Tue, 04 Apr 2023 18:03:13 +0200
11
1hibagent (1.0.1-0ubuntu2) hirsute; urgency=medium12hibagent (1.0.1-0ubuntu2) hirsute; urgency=medium
213
3 * debian/copyright: Set Source to public upstream GitHub repository14 * debian/copyright: Set Source to public upstream GitHub repository
diff --git a/debian/patches/lp1896638-set-resume-device-by-partition-uuid.patch b/debian/patches/lp1896638-set-resume-device-by-partition-uuid.patch
4new file mode 10064415new file mode 100644
index 0000000..39ef202
--- /dev/null
+++ b/debian/patches/lp1896638-set-resume-device-by-partition-uuid.patch
@@ -0,0 +1,33 @@
1From: Tony Nie <zirann@amazon.com>
2Subject: Set resume device by PARTUUID instead of by device name
3Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/ec2-hibinit-agent/+bug/1896638
4Origin: upstream, https://github.com/aws/ec2-hibernate-linux-agent/commit/43636fcf
5---
6 agent/hibagent | 7 +++++++
7 1 file changed, 7 insertions(+)
8
9Index: b/agent/hibagent
10===================================================================
11--- a/agent/hibagent 2023-03-24 13:12:50.192981414 +0100
12+++ b/agent/hibagent 2023-03-24 13:17:13.151025129 +0100
13@@ -99,6 +99,11 @@ def get_swap_space():
14 return int(lines[0].split()[2]) * 1024
15
16
17+def get_partuuid(device):
18+ return check_output(
19+ ['lsblk', '-dno', 'PARTUUID', device]).decode('ascii').strip()
20+
21+
22 def patch_grub_config(swap_device, offset, grub_file, grub2_dir):
23 log("Updating GRUB to use the device %s with offset %d for resume" % (swap_device, offset))
24 lines = []
25@@ -128,6 +133,8 @@ def patch_grub_config(swap_device, offse
26 # Do GRUB2 update as well
27 if grub2_dir and os.path.exists(grub2_dir):
28 offset_file = os.path.join(grub2_dir, '99-set-swap.cfg')
29+ if swap_device.startswith("/dev"):
30+ swap_device = "PARTUUID=%s" % get_partuuid(swap_device)
31 if not os.path.exists(offset_file):
32 with open(offset_file, 'w') as fl:
33 fl.write('GRUB_CMDLINE_LINUX_DEFAULT="$GRUB_CMDLINE_LINUX_DEFAULT no_console_suspend=1 '
diff --git a/debian/patches/lp2013336-do-not-modify-GRUB-config-on-GRUB2-systems.patch b/debian/patches/lp2013336-do-not-modify-GRUB-config-on-GRUB2-systems.patch
0new file mode 10064434new file mode 100644
index 0000000..737b401
--- /dev/null
+++ b/debian/patches/lp2013336-do-not-modify-GRUB-config-on-GRUB2-systems.patch
@@ -0,0 +1,75 @@
1From: Robert Schweikert <rjschwei@suse.com>
2Subject: Do not attempt to modify non existent GRUB config file
3
4Distributions either use GRUB or GRUB2, we need to guard against attempts
5to modify menu.lst on GRUB2 distributions
6
7Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/hibagent/+bug/2013336
8Origin: upstream, https://github.com/aws/ec2-hibernate-linux-agent/commit/0f995e80
9---
10 agent/hibagent | 48 +++++++++++++++++++++++++-----------------------
11 1 file changed, 25 insertions(+), 23 deletions(-)
12
13diff --git a/agent/hibagent b/agent/hibagent
14index 02e2b03..f076220 100755
15--- a/agent/hibagent
16+++ b/agent/hibagent
17@@ -100,30 +100,32 @@ def get_swap_space():
18
19
20 def patch_grub_config(swap_device, offset, grub_file, grub2_dir):
21- log("Updating GRUB to use the device %s with offset %d for resume" % (swap_device, offset))
22- lines = []
23- with open(grub_file) as fl:
24- for ln in fl.readlines():
25- params = ln.split()
26- if not params or params[0] != 'kernel':
27- lines.append(ln)
28- continue
29-
30- new_params = []
31- for param in params:
32- if "resume_offset=" in param or "resume=" in param:
33+ log("Updating GRUB to use the device %s with offset %d for resume"
34+ % (swap_device, offset))
35+ if grub_file and os.path.exists(grub_file):
36+ lines = []
37+ with open(grub_file) as fl:
38+ for ln in fl.readlines():
39+ params = ln.split()
40+ if not params or params[0] != 'kernel':
41+ lines.append(ln)
42 continue
43- new_params.append(param)
44-
45- if "no_console_suspend=" not in ln:
46- new_params.append("no_console_suspend=1")
47- new_params.append("resume_offset=%d" % offset)
48- new_params.append("resume=%s" % swap_device)
49-
50- lines.append(" ".join(new_params)+"\n")
51-
52- with open(grub_file, "w") as fl:
53- fl.write("".join(lines))
54+
55+ new_params = []
56+ for param in params:
57+ if "resume_offset=" in param or "resume=" in param:
58+ continue
59+ new_params.append(param)
60+
61+ if "no_console_suspend=" not in ln:
62+ new_params.append("no_console_suspend=1")
63+ new_params.append("resume_offset=%d" % offset)
64+ new_params.append("resume=%s" % swap_device)
65+
66+ lines.append(" ".join(new_params)+"\n")
67+
68+ with open(grub_file, "w") as fl:
69+ fl.write("".join(lines))
70
71 # Do GRUB2 update as well
72 if grub2_dir and os.path.exists(grub2_dir):
73--
742.37.2
75
diff --git a/debian/patches/series b/debian/patches/series
index 7dc0cd8..3174b35 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -7,3 +7,5 @@ use-sh-for-init-scripts.patch
7add-lsb-init-info.patch7add-lsb-init-info.patch
8fix-enable-ec2-spot-hibernation.patch8fix-enable-ec2-spot-hibernation.patch
90009-Use-Thread.is_alive-for-Python-3.9-compatibility.patch90009-Use-Thread.is_alive-for-Python-3.9-compatibility.patch
10lp1896638-set-resume-device-by-partition-uuid.patch
11lp2013336-do-not-modify-GRUB-config-on-GRUB2-systems.patch

Subscribers

People subscribed via source and target branches