Quoting Nina Schoetterl-Glausch (2022-12-14 19:27:15)
> On Wed, 2022-12-14 at 13:38 +0100, Nico Boehr wrote:
> > Right now, we have a test which sets storage keys, then migrates the VM
> > and - after migration finished - verifies the skeys are still there.
> >
> > Add a new version of the test which changes storage keys while the
> > migration is in progress. This is achieved by adding a command line
> > argument to the existing migration-skey test.
> >
> > Signed-off-by: Nico Boehr <nrb@xxxxxxxxxxxxx>
>
> Reviewed-by: Nina Schoetterl-Glausch <nsg@xxxxxxxxxxxxx>
>
> Indentation should be fixed IMO, feel free to ignore the rest.
Thanks!
[...]
> > diff --git a/s390x/migration-skey.c b/s390x/migration-skey.c
> > index a91eb6b5a63e..0f862cc9d821 100644
> > --- a/s390x/migration-skey.c
> > +++ b/s390x/migration-skey.c
> >
[...]
> > +static struct verify_result verify_test_pattern(unsigned char seed)
> > +{
[...]
> > - /* don't log anything when key matches to avoid spamming the log */
> > if (actual_key.val != expected_key.val) {
> > - key_mismatches++;
> > - report_fail("page %d expected_key=0x%x actual_key=0x%x", i, expected_key.val, actual_key.val);
>
> I feel like setting verify_failed here also would be nicer.
I had this before, Claudio requested to remove it...
> Could also do
> return (struct verify_result) {
> ...
> }
> Just a suggestion.
No strong opinion. I'll do whatever you prefer.
[...]
> > +static void report_verify_result(struct verify_result * const result)
>
> Why const? Why not also pointer to const?
Yes right, I'll make this a const struct.
> > +{
> > + if (result->verify_failed)
> > + report_fail("page skey mismatch: first page idx = %lu, addr = 0x%lx, "
> > + "expected_key = 0x%x, actual_key = 0x%x",
>
> Indent is off here.
Yes done.
> I have a slight preference for %02x for the keys. Just a suggestion.
Yes, changed.
> [...]
>
> > -int main(void)
> > +int main(int argc, char **argv)
> > {
> > report_prefix_push("migration-skey");
> >
> > - if (test_facility(169))
> > + if (test_facility(169)) {
> > report_skip("storage key removal facility is active");
> > - else
> > - test_migration();
> > + goto error;
> > + }
> >
> > - migrate_once();
> > + parse_args(argc, argv);
> > +
> > + switch (arg_test_to_run) {
>
> break statements should be indented.
Fixed.
