Merge ~sylvain-pineau/checkbox-support:usb_scripts_low_mem into checkbox-support:master

Proposed by Sylvain Pineau
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)
Reviewer Review Type Date Requested Status
Maciej Kisielewski (community) Abstain
Review via email: mp+341304@code.launchpad.net

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.

To post a comment you must log in.
Revision history for this message
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.

review: Abstain
Revision history for this message
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 :)

Revision history for this message
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.

Revision history for this message
Maciej Kisielewski (kissiel) wrote :

Ack.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/checkbox_support/scripts/run_watcher.py b/checkbox_support/scripts/run_watcher.py
index 50f2db1..9283db6 100644
--- a/checkbox_support/scripts/run_watcher.py
+++ b/checkbox_support/scripts/run_watcher.py
@@ -228,4 +228,12 @@ def main():
228 watcher = LogWatcher("/var/log", callback, logfile="syslog")228 watcher = LogWatcher("/var/log", callback, logfile="syslog")
229 signal.signal(signal.SIGALRM, no_usb_timeout)229 signal.signal(signal.SIGALRM, no_usb_timeout)
230 signal.alarm(USB_INSERT_TIMEOUT)230 signal.alarm(USB_INSERT_TIMEOUT)
231 if ARGS.testcase == "insertion":
232 print("\n\nINSERT NOW\n\n", flush=True)
233 elif ARGS.testcase == "removal":
234 print("\n\nREMOVE NOW\n\n", flush=True)
231 watcher.loop()235 watcher.loop()
236
237
238if __name__ == "__main__":
239 main()
diff --git a/checkbox_support/scripts/usb_read_write.py b/checkbox_support/scripts/usb_read_write.py
index c589def..9dc63ff 100644
--- a/checkbox_support/scripts/usb_read_write.py
+++ b/checkbox_support/scripts/usb_read_write.py
@@ -36,12 +36,20 @@ PLAINBOX_SESSION_SHARE = os.environ.get('PLAINBOX_SESSION_SHARE', '')
36FOLDER_TO_MOUNT = tempfile.mkdtemp()36FOLDER_TO_MOUNT = tempfile.mkdtemp()
37REPETITION_NUM = 5 # number to repeat the read/write test units.37REPETITION_NUM = 5 # number to repeat the read/write test units.
38# Prepare a random file which size is RANDOM_FILE_SIZE.38# Prepare a random file which size is RANDOM_FILE_SIZE.
39# Default 1048576 = 1MB to perform writing test39RANDOM_FILE_SIZE = 104857600 # 100 MiB
40RANDOM_FILE_SIZE = 10485760040mem_bytes = os.sysconf('SC_PAGE_SIZE') * os.sysconf('SC_PHYS_PAGES')
41mem_mib = mem_bytes/(1024.**2)
42# On systems with less than 1 GiB of RAM, only generate a 20 MiB file
43if mem_mib < 1200:
44 RANDOM_FILE_SIZE = 20971520
41USB_INSERT_INFO = "usb_insert_info"45USB_INSERT_INFO = "usb_insert_info"
4246
43log_path = os.path.join(PLAINBOX_SESSION_SHARE, 'usb-rw.log')47log_path = os.path.join(PLAINBOX_SESSION_SHARE, 'usb-rw.log')
44logging.basicConfig(level=logging.DEBUG, filename=log_path)48logging.basicConfig(level=logging.DEBUG, filename=log_path)
49ch = logging.StreamHandler(sys.stdout)
50ch.setFormatter(logging.Formatter("%(levelname)s:%(message)s"))
51log = logging.getLogger('')
52log.addHandler(ch)
4553
4654
47class RandomData():55class RandomData():
@@ -88,6 +96,7 @@ class RandomData():
88 data = self._generate_test_data()96 data = self._generate_test_data()
89 while os.path.getsize(self.tfile.name) < size:97 while os.path.getsize(self.tfile.name) < size:
90 self.tfile.write(next(data).encode('UTF-8'))98 self.tfile.write(next(data).encode('UTF-8'))
99 self.tfile.close()
91 return self100 return self
92101
93102
@@ -120,7 +129,6 @@ def get_partition_info():
120 else:129 else:
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")
122 sys.exit(1)131 sys.exit(1)
123
124 return partition132 return partition
125133
126134
@@ -160,7 +168,6 @@ def mount_usb_storage(partition):
160 # use pipe so I could hide message like168 # use pipe so I could hide message like
161 # "umount: /tmp/tmpjzwb6lys: not mounted"169 # "umount: /tmp/tmpjzwb6lys: not mounted"
162 subprocess.call(['umount', FOLDER_TO_MOUNT], stderr=subprocess.PIPE)170 subprocess.call(['umount', FOLDER_TO_MOUNT], stderr=subprocess.PIPE)
163
164 # mount the target device/partition171 # mount the target device/partition
165 # if the return code of the shell command is non-zero,172 # if the return code of the shell command is non-zero,
166 # means something wrong.173 # means something wrong.
@@ -173,7 +180,6 @@ def mount_usb_storage(partition):
173 logging.debug("mount %s on %s successfully."180 logging.debug("mount %s on %s successfully."
174 % (device_to_mount, FOLDER_TO_MOUNT))181 % (device_to_mount, FOLDER_TO_MOUNT))
175 yield182 yield
176
177 finally:183 finally:
178 logging.info("context manager exit: unmount USB storage")184 logging.info("context manager exit: unmount USB storage")
179 if subprocess.call(['umount', FOLDER_TO_MOUNT]):185 if subprocess.call(['umount', FOLDER_TO_MOUNT]):
@@ -187,7 +193,6 @@ def read_test(random_file):
187 logging.debug("===================")193 logging.debug("===================")
188 logging.debug("reading test begins")194 logging.debug("reading test begins")
189 logging.debug("===================")195 logging.debug("===================")
190
191 read_test_list = []196 read_test_list = []
192 for idx in range(REPETITION_NUM):197 for idx in range(REPETITION_NUM):
193 read_test_list.append(read_test_unit(random_file, str(idx)))198 read_test_list.append(read_test_unit(random_file, str(idx)))
@@ -213,14 +218,11 @@ def read_test_unit(random_source_file, idx=""):
213 process = subprocess.Popen(['md5sum', random_source_file.tfile.name],218 process = subprocess.Popen(['md5sum', random_source_file.tfile.name],
214 stdout=subprocess.PIPE)219 stdout=subprocess.PIPE)
215 source_md5sum = process.communicate()[0].decode().split(" ")[0]220 source_md5sum = process.communicate()[0].decode().split(" ")[0]
216
217 logging.debug("%s %s (verified)" % (tfile_md5sum, path_random_file))221 logging.debug("%s %s (verified)" % (tfile_md5sum, path_random_file))
218 logging.debug("%s %s (source)"222 logging.debug("%s %s (source)"
219 % (source_md5sum, random_source_file.tfile.name))223 % (source_md5sum, random_source_file.tfile.name))
220
221 # Clean the target file224 # Clean the target file
222 os.remove(path_random_file)225 os.remove(path_random_file)
223
224 # verify the md5sum226 # verify the md5sum
225 if tfile_md5sum == source_md5sum:227 if tfile_md5sum == source_md5sum:
226 print("PASS: READING TEST: %s passes md5sum comparison."228 print("PASS: READING TEST: %s passes md5sum comparison."
@@ -238,7 +240,6 @@ def write_test(random_file):
238 logging.debug("===================")240 logging.debug("===================")
239 logging.debug("writing test begins")241 logging.debug("writing test begins")
240 logging.debug("===================")242 logging.debug("===================")
241
242 write_speed_list = []243 write_speed_list = []
243 for idx in range(REPETITION_NUM):244 for idx in range(REPETITION_NUM):
244 write_speed_list.append(write_test_unit(random_file, str(idx)))245 write_speed_list.append(write_test_unit(random_file, str(idx)))
@@ -253,13 +254,14 @@ def write_test_unit(random_file, idx=""):
253 """254 """
254 perform the writing test.255 perform the writing test.
255256
256 :param random_file: a RndomData object created to be written257 :param random_file: a RandomData object created to be written
257 :return: a float in MB/s to denote writing speed258 :return: a float in MB/s to denote writing speed
258 """259 """
259 target_file = os.path.join(260 target_file = os.path.join(
260 FOLDER_TO_MOUNT, os.path.basename(random_file.tfile.name)) + idx261 FOLDER_TO_MOUNT, os.path.basename(random_file.tfile.name)) + idx
261 process = subprocess.Popen([262 process = subprocess.Popen([
262 'dd', 'if=' + random_file.tfile.name, 'of=' + target_file],263 'dd', 'if=' + random_file.tfile.name, 'of=' + target_file, 'bs=1M',
264 'oflag=sync'],
263 stderr=subprocess.STDOUT, stdout=subprocess.PIPE)265 stderr=subprocess.STDOUT, stdout=subprocess.PIPE)
264 logging.debug("Apply command: %s" % process.args)266 logging.debug("Apply command: %s" % process.args)
265 # will get something like267 # will get something like
@@ -267,8 +269,6 @@ def write_test_unit(random_file, idx=""):
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', '']
268 list_dd_message = process.communicate()[0].decode().split("\n")270 list_dd_message = process.communicate()[0].decode().split("\n")
269 logging.debug(list_dd_message)271 logging.debug(list_dd_message)
270 logging.debug(get_md5sum(target_file))
271
272 try:272 try:
273 dd_speed = float(list_dd_message[2].split(" ")[-2])273 dd_speed = float(list_dd_message[2].split(" ")[-2])
274 print("PASS: WRITING TEST: %s" % target_file)274 print("PASS: WRITING TEST: %s" % target_file)
@@ -279,7 +279,6 @@ def write_test_unit(random_file, idx=""):
279 # (20 MB) copied, 99.647 s, 200 kB/s', '']279 # (20 MB) copied, 99.647 s, 200 kB/s', '']
280 print("ERROR: {}".format(list_dd_message))280 print("ERROR: {}".format(list_dd_message))
281 sys.exit(1)281 sys.exit(1)
282
283 return dd_speed282 return dd_speed
284283
285284
@@ -291,18 +290,9 @@ def gen_random_file():
291 :return: a RandomData object290 :return: a RandomData object
292 """291 """
293 logging.debug("generating a random file")292 logging.debug("generating a random file")
294
295 try:293 try:
296 # 1048576 = 1024 * 1024
297 # we are going to generate a 1M file
298 random_file = RandomData(RANDOM_FILE_SIZE)294 random_file = RandomData(RANDOM_FILE_SIZE)
299 # flush the remaining data in the memory buffer
300 # otherwise the md5sum will be different if you
301 # check it manually from your shell command md5sum
302 random_file.tfile.file.flush()
303
304 yield random_file295 yield random_file
305
306 finally:296 finally:
307 logging.info("Remove temporary folders and files.")297 logging.info("Remove temporary folders and files.")
308 # delete the mount folder298 # delete the mount folder
@@ -330,16 +320,14 @@ def get_md5sum(file_to_check):
330 # (b'07bc8f96b7c7dba2c1f3eb2f7dd50541 /tmp/tmp9jnuv329\n', None)320 # (b'07bc8f96b7c7dba2c1f3eb2f7dd50541 /tmp/tmp9jnuv329\n', None)
331 # will be returned by communicate() in this case321 # will be returned by communicate() in this case
332 md5sum = process.communicate()[0].decode().split(" ")[0]322 md5sum = process.communicate()[0].decode().split(" ")[0]
333
334 if md5sum:323 if md5sum:
335 logging.debug("MD5SUM: of %s \t\t\t\t\t%s"324 logging.debug("MD5SUM of %s: %s"
336 % (file_to_check, md5sum))325 % (file_to_check, md5sum))
337 return md5sum326 return md5sum
338 else:327 else:
339 logging.error("Could not found file to check its MD5SUM. \328 logging.error("Could not found file to check its MD5SUM. \
340 Check the folder permission?")329 Check the folder permission?")
341 sys.exit(1)330 sys.exit(1)
342
343 except OSError as e:331 except OSError as e:
344 if e.errno == errno.ENOENT:332 if e.errno == errno.ENOENT:
345 logging.error("%s info file was not found. \333 logging.error("%s info file was not found. \

Subscribers

People subscribed via source and target branches