Differential D45535

virstor: basic functional test
ClosedPublic
Actions

Authored by rlibby on Fri, Jun 7, 8:25 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Wed, Jun 26, 10:30 AM

	Unknown Object (File)
	Tue, Jun 25, 3:19 PM

	Unknown Object (File)
	Wed, Jun 12, 3:09 AM

	Unknown Object (File)
	Tue, Jun 11, 3:30 PM

	Unknown Object (File)
	Tue, Jun 11, 4:48 AM

	Unknown Object (File)
	Mon, Jun 10, 5:16 PM

Subscribers

Details

Reviewers

markj
pjd
asomers

Commits

rG7affbeeab1c9: virstor: basic functional test

Summary

Sponsored by: Dell EMC Isilon

Test Plan

% sudo kyua debug -k /usr/tests/sys/Kyuafile geom/class/virstor/virstor_test:basic
Executing command [ gvirstor label -v -s 64M -m 512 virstor.bjyhzA /dev/md0 ]
Executing command [ gvirstor destroy virstor.bjyhzA ]

Destroying test virstor device: virstor.bjyhzA
Removing test memory disk: md0

geom/class/virstor/virstor_test:basic -> passed

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

rlibby created this revision.Fri, Jun 7, 8:25 PM

Herald added subscribers: asomers, imp. · View Herald TranscriptFri, Jun 7, 8:25 PM

rlibby requested review of this revision.Fri, Jun 7, 8:25 PM

Harbormaster completed remote builds in B58106: Diff 139634.Fri, Jun 7, 8:26 PM

rlibby added a child revision: D45517: virstor: write large maps in chunks during label.Fri, Jun 7, 8:26 PM

tap is highly discouraged for new code. Can you use ATF instead?

asomers: tap was a bad example, use atf

Harbormaster completed remote builds in B58107: Diff 139641.Fri, Jun 7, 10:30 PM

realized we may need to wait after the re-taste

Harbormaster completed remote builds in B58110: Diff 139645.Sat, Jun 8, 1:09 AM

asomers requested changes to this revision.Sun, Jun 9, 8:41 PM

asomers added inline comments.

tests/sys/geom/class/virstor/conf.sh
6	This is going to create a temporary file every time somebody lists tests cases, which I don't think is what you want. And the temporary file won't even be guaranteed to be unique. I think you should move this into some kind of setup function , and remove the "-u". Worse, I think that the cleanup routine is executed in a different process, which means that it won't even have the correct value for $name.
tests/sys/geom/class/virstor/virstor_test.sh
22	Unless you _really_ need the device to stay resident in RAM. Also, I find "md0" to be a confusing variable name, because "md0" is also a valid value. I suggest just plain "md" or something else that isn't a valid value.

This revision now requires changes to proceed.Sun, Jun 9, 8:41 PM

rlibby added inline comments.Sun, Jun 9, 9:03 PM

tests/sys/geom/class/virstor/conf.sh
6	mktemp -u doesn't create files. It's just choosing a probably-unique name for the virstor device. But I agree it doesn't do it very well. Thanks for pointing this out. I'll look into the cleanup issue, I think that's new with the conversion to atf. There must be some way that atf tests usually communicate variables to cleanup functions?
tests/sys/geom/class/virstor/virstor_test.sh
22	Will change.

asomers added inline comments.Sun, Jun 9, 9:12 PM

tests/sys/geom/class/virstor/conf.sh
6	Yeah, communicating values to cleanup functions is usually done through files in the test's temporary directory. See for example TEST_MDS_FILE in geom_subr.sh.

rlibby edited the test plan for this revision. (Show Details)Sun, Jun 9, 10:51 PM

asomers feedback:

cleanup was broken since it runs in a different context
prefer swap to malloc type md
rename $md0

Harbormaster completed remote builds in B58115: Diff 139665.Sun, Jun 9, 11:01 PM

LGTM now

This revision is now accepted and ready to land.Sun, Jun 9, 11:34 PM

markj accepted this revision.Mon, Jun 10, 1:57 PM

rlibby mentioned this in D45517: virstor: write large maps in chunks during label.Mon, Jun 10, 4:23 PM

rlibby mentioned this in D45518: virstor: remove relation between chunk size and MAXPHYS.

Closed by commit rG7affbeeab1c9: virstor: basic functional test (authored by rlibby). · Explain WhyMon, Jun 10, 5:35 PM

This revision was automatically updated to reflect the committed changes.

rlibby added a commit: rG7affbeeab1c9: virstor: basic functional test.

Revision Contents
Changeset List

Path

Size

tests/

sys/

geom/

class/

1 line

virstor/

9 lines

31 lines

virstor_test.sh

73 lines

Diff 139695

tests/sys/geom/class/Makefile

Loading...

tests/sys/geom/class/virstor/Makefile

Loading...

tests/sys/geom/class/virstor/conf.sh

Loading...

tests/sys/geom/class/virstor/virstor_test.sh

Loading...