> We sort of test the same in test-callbacks.sh. Just using different > means. I think I would not mind having this as well.
Ok. The original idea Joe suggested was to check the function output of module targets directly. From what I can tell, test-callbacks.sh covers the callbacks and test-ftrace.sh checks that ftrace can still trace a function after it's been livepatched, neither actually checks that a replacement function in a module target runs. I can be wrong though, still getting familiar with the livepatch tests. > I was *just* in the middle of replying to the patch when yours came in, > so I'll just move over here. I had noticed the same thing re: > test-callbacks.sh despite originally suggested writing this test to > Pablo (and forgot about the callbacks test module). With that, I agree > that it's a nice basic sanity check that's obvious about what it's > testing. Fair point. Altough biased I think it is nice to have this explict sanity check. > A nit but is 'noinline' keyword needed here? proc_create_single() below > takes a function pointer so hopefully test_klp_mod_target_show() stays > even without it? > No strong preference either way. I won't fault a livepatch developer > for being paranoid w/respect to the compiler :D Yeah I think you're right, not strictly needed, just wanted to be sure. After some experience rebasing kpatch integration tests I've been bitten enough times to be paranoid about that :) But since I will work on a follow-up for the other suggestions, I think I'll drop it there. Anyways, thanks for the reviews! Pablo
signature.asc
Description: PGP signature
