Merge ~sylvain-pineau/checkbox-support:usb_scripts_low_mem into checkbox-support:master
- Git
- lp:~sylvain-pineau/checkbox-support
- usb_scripts_low_mem
- Merge into master
Status: | Merged |
---|---|
Approved by: | Sylvain Pineau |
Approved revision: | 19b59f5e6f3fcf9cc593f2d0a2b3049fb6c72789 |
Merged at revision: | 4f429e965c29335ff869de199a01b4a441cc18d6 |
Proposed branch: | ~sylvain-pineau/checkbox-support:usb_scripts_low_mem |
Merge into: | checkbox-support:master |
Diff against target: |
178 lines (+23/-27) 2 files modified
checkbox_support/scripts/run_watcher.py (+8/-0) checkbox_support/scripts/usb_read_write.py (+15/-27) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Maciej Kisielewski (community) | Abstain | ||
Review via email: mp+341304@code.launchpad.net |
Commit message
Description of the change
This MR allows the non udisks2 usb test to run on low memory devices, tested on tillamook where we only have 100M to run checkbox and the storage test.
Maciej Kisielewski (kissiel) wrote : | # |
> Looks good, +1.
>
> Two subjective, style-related comments in line.
>
> One general thought I have for the whole usb_read_write.py is that I feel like
> instead of optimizing size of the "test file" we should generate the data on-
> the-fly and write it to two pipes, one to compute the hash for reference, and
> one that will be written to the drive.
>
> Or at least generate the random file in a less memory-hungry way.
> The reference md5 computation doesn't have to be done on a written file. First
> thought is to use hashlib from python, block by block. The real solution,
> however, is to use precomputed hashes. For any given size the hash can be
> known statically (there is a hardcoded seed).
>
> To sum up: content of RandomData could be written to the target file on the
> fly without a need of any huge buffer. If the size is known upfront
> precomputed hash can be used.
>
> I feel bad about circumventing the real problem in the script instead of
> fixing it.
Ha, the first line with +1 got changed to +0 when I kept reading the usb_read_write.py :)
Sylvain Pineau (sylvain-pineau) wrote : | # |
Freeing memory turned out to be quite challenging. I'd prefer to go with this working (but not perfect) version) til we can revisit the storage tests.
Maciej Kisielewski (kissiel) wrote : | # |
Ack.
Preview Diff
1 | diff --git a/checkbox_support/scripts/run_watcher.py b/checkbox_support/scripts/run_watcher.py | |||
2 | index 50f2db1..9283db6 100644 | |||
3 | --- a/checkbox_support/scripts/run_watcher.py | |||
4 | +++ b/checkbox_support/scripts/run_watcher.py | |||
5 | @@ -228,4 +228,12 @@ def main(): | |||
6 | 228 | watcher = LogWatcher("/var/log", callback, logfile="syslog") | 228 | watcher = LogWatcher("/var/log", callback, logfile="syslog") |
7 | 229 | signal.signal(signal.SIGALRM, no_usb_timeout) | 229 | signal.signal(signal.SIGALRM, no_usb_timeout) |
8 | 230 | signal.alarm(USB_INSERT_TIMEOUT) | 230 | signal.alarm(USB_INSERT_TIMEOUT) |
9 | 231 | if ARGS.testcase == "insertion": | ||
10 | 232 | print("\n\nINSERT NOW\n\n", flush=True) | ||
11 | 233 | elif ARGS.testcase == "removal": | ||
12 | 234 | print("\n\nREMOVE NOW\n\n", flush=True) | ||
13 | 231 | watcher.loop() | 235 | watcher.loop() |
14 | 236 | |||
15 | 237 | |||
16 | 238 | if __name__ == "__main__": | ||
17 | 239 | main() | ||
18 | diff --git a/checkbox_support/scripts/usb_read_write.py b/checkbox_support/scripts/usb_read_write.py | |||
19 | index c589def..9dc63ff 100644 | |||
20 | --- a/checkbox_support/scripts/usb_read_write.py | |||
21 | +++ b/checkbox_support/scripts/usb_read_write.py | |||
22 | @@ -36,12 +36,20 @@ PLAINBOX_SESSION_SHARE = os.environ.get('PLAINBOX_SESSION_SHARE', '') | |||
23 | 36 | FOLDER_TO_MOUNT = tempfile.mkdtemp() | 36 | FOLDER_TO_MOUNT = tempfile.mkdtemp() |
24 | 37 | REPETITION_NUM = 5 # number to repeat the read/write test units. | 37 | REPETITION_NUM = 5 # number to repeat the read/write test units. |
25 | 38 | # Prepare a random file which size is RANDOM_FILE_SIZE. | 38 | # Prepare a random file which size is RANDOM_FILE_SIZE. |
28 | 39 | # Default 1048576 = 1MB to perform writing test | 39 | RANDOM_FILE_SIZE = 104857600 # 100 MiB |
29 | 40 | RANDOM_FILE_SIZE = 104857600 | 40 | mem_bytes = os.sysconf('SC_PAGE_SIZE') * os.sysconf('SC_PHYS_PAGES') |
30 | 41 | mem_mib = mem_bytes/(1024.**2) | ||
31 | 42 | # On systems with less than 1 GiB of RAM, only generate a 20 MiB file | ||
32 | 43 | if mem_mib < 1200: | ||
33 | 44 | RANDOM_FILE_SIZE = 20971520 | ||
34 | 41 | USB_INSERT_INFO = "usb_insert_info" | 45 | USB_INSERT_INFO = "usb_insert_info" |
35 | 42 | 46 | ||
36 | 43 | log_path = os.path.join(PLAINBOX_SESSION_SHARE, 'usb-rw.log') | 47 | log_path = os.path.join(PLAINBOX_SESSION_SHARE, 'usb-rw.log') |
37 | 44 | logging.basicConfig(level=logging.DEBUG, filename=log_path) | 48 | logging.basicConfig(level=logging.DEBUG, filename=log_path) |
38 | 49 | ch = logging.StreamHandler(sys.stdout) | ||
39 | 50 | ch.setFormatter(logging.Formatter("%(levelname)s:%(message)s")) | ||
40 | 51 | log = logging.getLogger('') | ||
41 | 52 | log.addHandler(ch) | ||
42 | 45 | 53 | ||
43 | 46 | 54 | ||
44 | 47 | class RandomData(): | 55 | class RandomData(): |
45 | @@ -88,6 +96,7 @@ class RandomData(): | |||
46 | 88 | data = self._generate_test_data() | 96 | data = self._generate_test_data() |
47 | 89 | while os.path.getsize(self.tfile.name) < size: | 97 | while os.path.getsize(self.tfile.name) < size: |
48 | 90 | self.tfile.write(next(data).encode('UTF-8')) | 98 | self.tfile.write(next(data).encode('UTF-8')) |
49 | 99 | self.tfile.close() | ||
50 | 91 | return self | 100 | return self |
51 | 92 | 101 | ||
52 | 93 | 102 | ||
53 | @@ -120,7 +129,6 @@ def get_partition_info(): | |||
54 | 120 | else: | 129 | else: |
55 | 121 | logging.error("has no idea which partition to mount or not found") | 130 | logging.error("has no idea which partition to mount or not found") |
56 | 122 | sys.exit(1) | 131 | sys.exit(1) |
57 | 123 | |||
58 | 124 | return partition | 132 | return partition |
59 | 125 | 133 | ||
60 | 126 | 134 | ||
61 | @@ -160,7 +168,6 @@ def mount_usb_storage(partition): | |||
62 | 160 | # use pipe so I could hide message like | 168 | # use pipe so I could hide message like |
63 | 161 | # "umount: /tmp/tmpjzwb6lys: not mounted" | 169 | # "umount: /tmp/tmpjzwb6lys: not mounted" |
64 | 162 | subprocess.call(['umount', FOLDER_TO_MOUNT], stderr=subprocess.PIPE) | 170 | subprocess.call(['umount', FOLDER_TO_MOUNT], stderr=subprocess.PIPE) |
65 | 163 | |||
66 | 164 | # mount the target device/partition | 171 | # mount the target device/partition |
67 | 165 | # if the return code of the shell command is non-zero, | 172 | # if the return code of the shell command is non-zero, |
68 | 166 | # means something wrong. | 173 | # means something wrong. |
69 | @@ -173,7 +180,6 @@ def mount_usb_storage(partition): | |||
70 | 173 | logging.debug("mount %s on %s successfully." | 180 | logging.debug("mount %s on %s successfully." |
71 | 174 | % (device_to_mount, FOLDER_TO_MOUNT)) | 181 | % (device_to_mount, FOLDER_TO_MOUNT)) |
72 | 175 | yield | 182 | yield |
73 | 176 | |||
74 | 177 | finally: | 183 | finally: |
75 | 178 | logging.info("context manager exit: unmount USB storage") | 184 | logging.info("context manager exit: unmount USB storage") |
76 | 179 | if subprocess.call(['umount', FOLDER_TO_MOUNT]): | 185 | if subprocess.call(['umount', FOLDER_TO_MOUNT]): |
77 | @@ -187,7 +193,6 @@ def read_test(random_file): | |||
78 | 187 | logging.debug("===================") | 193 | logging.debug("===================") |
79 | 188 | logging.debug("reading test begins") | 194 | logging.debug("reading test begins") |
80 | 189 | logging.debug("===================") | 195 | logging.debug("===================") |
81 | 190 | |||
82 | 191 | read_test_list = [] | 196 | read_test_list = [] |
83 | 192 | for idx in range(REPETITION_NUM): | 197 | for idx in range(REPETITION_NUM): |
84 | 193 | read_test_list.append(read_test_unit(random_file, str(idx))) | 198 | read_test_list.append(read_test_unit(random_file, str(idx))) |
85 | @@ -213,14 +218,11 @@ def read_test_unit(random_source_file, idx=""): | |||
86 | 213 | process = subprocess.Popen(['md5sum', random_source_file.tfile.name], | 218 | process = subprocess.Popen(['md5sum', random_source_file.tfile.name], |
87 | 214 | stdout=subprocess.PIPE) | 219 | stdout=subprocess.PIPE) |
88 | 215 | source_md5sum = process.communicate()[0].decode().split(" ")[0] | 220 | source_md5sum = process.communicate()[0].decode().split(" ")[0] |
89 | 216 | |||
90 | 217 | logging.debug("%s %s (verified)" % (tfile_md5sum, path_random_file)) | 221 | logging.debug("%s %s (verified)" % (tfile_md5sum, path_random_file)) |
91 | 218 | logging.debug("%s %s (source)" | 222 | logging.debug("%s %s (source)" |
92 | 219 | % (source_md5sum, random_source_file.tfile.name)) | 223 | % (source_md5sum, random_source_file.tfile.name)) |
93 | 220 | |||
94 | 221 | # Clean the target file | 224 | # Clean the target file |
95 | 222 | os.remove(path_random_file) | 225 | os.remove(path_random_file) |
96 | 223 | |||
97 | 224 | # verify the md5sum | 226 | # verify the md5sum |
98 | 225 | if tfile_md5sum == source_md5sum: | 227 | if tfile_md5sum == source_md5sum: |
99 | 226 | print("PASS: READING TEST: %s passes md5sum comparison." | 228 | print("PASS: READING TEST: %s passes md5sum comparison." |
100 | @@ -238,7 +240,6 @@ def write_test(random_file): | |||
101 | 238 | logging.debug("===================") | 240 | logging.debug("===================") |
102 | 239 | logging.debug("writing test begins") | 241 | logging.debug("writing test begins") |
103 | 240 | logging.debug("===================") | 242 | logging.debug("===================") |
104 | 241 | |||
105 | 242 | write_speed_list = [] | 243 | write_speed_list = [] |
106 | 243 | for idx in range(REPETITION_NUM): | 244 | for idx in range(REPETITION_NUM): |
107 | 244 | write_speed_list.append(write_test_unit(random_file, str(idx))) | 245 | write_speed_list.append(write_test_unit(random_file, str(idx))) |
108 | @@ -253,13 +254,14 @@ def write_test_unit(random_file, idx=""): | |||
109 | 253 | """ | 254 | """ |
110 | 254 | perform the writing test. | 255 | perform the writing test. |
111 | 255 | 256 | ||
113 | 256 | :param random_file: a RndomData object created to be written | 257 | :param random_file: a RandomData object created to be written |
114 | 257 | :return: a float in MB/s to denote writing speed | 258 | :return: a float in MB/s to denote writing speed |
115 | 258 | """ | 259 | """ |
116 | 259 | target_file = os.path.join( | 260 | target_file = os.path.join( |
117 | 260 | FOLDER_TO_MOUNT, os.path.basename(random_file.tfile.name)) + idx | 261 | FOLDER_TO_MOUNT, os.path.basename(random_file.tfile.name)) + idx |
118 | 261 | process = subprocess.Popen([ | 262 | process = subprocess.Popen([ |
120 | 262 | 'dd', 'if=' + random_file.tfile.name, 'of=' + target_file], | 263 | 'dd', 'if=' + random_file.tfile.name, 'of=' + target_file, 'bs=1M', |
121 | 264 | 'oflag=sync'], | ||
122 | 263 | stderr=subprocess.STDOUT, stdout=subprocess.PIPE) | 265 | stderr=subprocess.STDOUT, stdout=subprocess.PIPE) |
123 | 264 | logging.debug("Apply command: %s" % process.args) | 266 | logging.debug("Apply command: %s" % process.args) |
124 | 265 | # will get something like | 267 | # will get something like |
125 | @@ -267,8 +269,6 @@ def write_test_unit(random_file, idx=""): | |||
126 | 267 | # '1049076 bytes (1.0 MB) copied, 0.00473357 s, 222 MB/s', ''] | 269 | # '1049076 bytes (1.0 MB) copied, 0.00473357 s, 222 MB/s', ''] |
127 | 268 | list_dd_message = process.communicate()[0].decode().split("\n") | 270 | list_dd_message = process.communicate()[0].decode().split("\n") |
128 | 269 | logging.debug(list_dd_message) | 271 | logging.debug(list_dd_message) |
129 | 270 | logging.debug(get_md5sum(target_file)) | ||
130 | 271 | |||
131 | 272 | try: | 272 | try: |
132 | 273 | dd_speed = float(list_dd_message[2].split(" ")[-2]) | 273 | dd_speed = float(list_dd_message[2].split(" ")[-2]) |
133 | 274 | print("PASS: WRITING TEST: %s" % target_file) | 274 | print("PASS: WRITING TEST: %s" % target_file) |
134 | @@ -279,7 +279,6 @@ def write_test_unit(random_file, idx=""): | |||
135 | 279 | # (20 MB) copied, 99.647 s, 200 kB/s', ''] | 279 | # (20 MB) copied, 99.647 s, 200 kB/s', ''] |
136 | 280 | print("ERROR: {}".format(list_dd_message)) | 280 | print("ERROR: {}".format(list_dd_message)) |
137 | 281 | sys.exit(1) | 281 | sys.exit(1) |
138 | 282 | |||
139 | 283 | return dd_speed | 282 | return dd_speed |
140 | 284 | 283 | ||
141 | 285 | 284 | ||
142 | @@ -291,18 +290,9 @@ def gen_random_file(): | |||
143 | 291 | :return: a RandomData object | 290 | :return: a RandomData object |
144 | 292 | """ | 291 | """ |
145 | 293 | logging.debug("generating a random file") | 292 | logging.debug("generating a random file") |
146 | 294 | |||
147 | 295 | try: | 293 | try: |
148 | 296 | # 1048576 = 1024 * 1024 | ||
149 | 297 | # we are going to generate a 1M file | ||
150 | 298 | random_file = RandomData(RANDOM_FILE_SIZE) | 294 | random_file = RandomData(RANDOM_FILE_SIZE) |
151 | 299 | # flush the remaining data in the memory buffer | ||
152 | 300 | # otherwise the md5sum will be different if you | ||
153 | 301 | # check it manually from your shell command md5sum | ||
154 | 302 | random_file.tfile.file.flush() | ||
155 | 303 | |||
156 | 304 | yield random_file | 295 | yield random_file |
157 | 305 | |||
158 | 306 | finally: | 296 | finally: |
159 | 307 | logging.info("Remove temporary folders and files.") | 297 | logging.info("Remove temporary folders and files.") |
160 | 308 | # delete the mount folder | 298 | # delete the mount folder |
161 | @@ -330,16 +320,14 @@ def get_md5sum(file_to_check): | |||
162 | 330 | # (b'07bc8f96b7c7dba2c1f3eb2f7dd50541 /tmp/tmp9jnuv329\n', None) | 320 | # (b'07bc8f96b7c7dba2c1f3eb2f7dd50541 /tmp/tmp9jnuv329\n', None) |
163 | 331 | # will be returned by communicate() in this case | 321 | # will be returned by communicate() in this case |
164 | 332 | md5sum = process.communicate()[0].decode().split(" ")[0] | 322 | md5sum = process.communicate()[0].decode().split(" ")[0] |
165 | 333 | |||
166 | 334 | if md5sum: | 323 | if md5sum: |
168 | 335 | logging.debug("MD5SUM: of %s \t\t\t\t\t%s" | 324 | logging.debug("MD5SUM of %s: %s" |
169 | 336 | % (file_to_check, md5sum)) | 325 | % (file_to_check, md5sum)) |
170 | 337 | return md5sum | 326 | return md5sum |
171 | 338 | else: | 327 | else: |
172 | 339 | logging.error("Could not found file to check its MD5SUM. \ | 328 | logging.error("Could not found file to check its MD5SUM. \ |
173 | 340 | Check the folder permission?") | 329 | Check the folder permission?") |
174 | 341 | sys.exit(1) | 330 | sys.exit(1) |
175 | 342 | |||
176 | 343 | except OSError as e: | 331 | except OSError as e: |
177 | 344 | if e.errno == errno.ENOENT: | 332 | if e.errno == errno.ENOENT: |
178 | 345 | logging.error("%s info file was not found. \ | 333 | logging.error("%s info file was not found. \ |
Looks good, +1.
Two subjective, style-related comments in line.
One general thought I have for the whole usb_read_write.py is that I feel like instead of optimizing size of the "test file" we should generate the data on-the-fly and write it to two pipes, one to compute the hash for reference, and one that will be written to the drive.
Or at least generate the random file in a less memory-hungry way.
The reference md5 computation doesn't have to be done on a written file. First thought is to use hashlib from python, block by block. The real solution, however, is to use precomputed hashes. For any given size the hash can be known statically (there is a hardcoded seed).
To sum up: content of RandomData could be written to the target file on the fly without a need of any huge buffer. If the size is known upfront precomputed hash can be used.
I feel bad about circumventing the real problem in the script instead of fixing it.