Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

verify /dev/mem is not present in QM partition #495

Merged
merged 2 commits into from
Aug 8, 2024

Conversation

pengshanyu
Copy link
Collaborator

@pengshanyu pengshanyu commented Jul 25, 2024

I want ensure that /dev/mem is not accesible inside QM partition.

Test Design (improved based on PR review):

  1. Using test expression in QM partition:

    Execute the following command:

    podman exec qm test -e /dev/mem

  2. Expected Result:

    /dev/mem does not exist, the command returns a non-zero value

  3. Verify whether the test passed or not:

    PASS: if the command returns a non-zero value, it means /dev/mem is not present
    FAIL: if the command returns 0, it means /dev/mem is present

Copy link
Collaborator

@Yarboa Yarboa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Yarboa
Copy link
Collaborator

Yarboa commented Jul 28, 2024

/packit test --identifier e2e-ffi

@pengshanyu
Copy link
Collaborator Author

will improve it based on the review of the design

Copy link
Collaborator

@Yarboa Yarboa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Yarboa
Copy link
Collaborator

Yarboa commented Jul 28, 2024

/packit test --identifier e2e-multi-bluechi-agents

Copy link
Collaborator

@Yarboa Yarboa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dougsland
Copy link
Collaborator

please squash commits for easy review.

@pengshanyu
Copy link
Collaborator Author

@dougsland @Yarboa thank you so much for the review.
This PR may be closed due to changes in test scope.

@Yarboa
Copy link
Collaborator

Yarboa commented Aug 1, 2024

#495 (comment)
Closing

@Yarboa Yarboa closed this Aug 1, 2024
@pengshanyu pengshanyu reopened this Aug 1, 2024
@pengshanyu pengshanyu changed the title verify /dev/mem is not accessible from QM partition verify /dev/mem is not present in QM partition Aug 1, 2024
@pengshanyu
Copy link
Collaborator Author

pengshanyu commented Aug 1, 2024

Sorry for the confusion caused by the comment above.
According to the latest decision, I need to continue this work.

@pengshanyu pengshanyu force-pushed the dev-mem-not-accessible branch 2 times, most recently from 86216e9 to 7afe651 Compare August 6, 2024 04:18
@dennisbrendel
Copy link
Contributor

Just a general comment: this is not a special property of qm, but of all containers created by podman. So it could also be considered a smoke test in a different place

Signed-off-by: pengshanyu <[email protected]>

add a single newline

Signed-off-by: pengshanyu <[email protected]>

remove -i and -a option

Signed-off-by: pengshanyu <[email protected]>

change test name

Signed-off-by: pengshanyu <[email protected]>

use test -e to check if file exists

Signed-off-by: pengshanyu <[email protected]>

remove output redirection

Signed-off-by: pengshanyu <[email protected]>

replace hyphen with low-line

Signed-off-by: pengshanyu <[email protected]>
Signed-off-by: pengshanyu <[email protected]>
@@ -0,0 +1,10 @@
Verifies that /dev/mem is not present in QM partition.
Copy link
Collaborator

@dougsland dougsland Aug 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please send another patch later using this format below, I will unblock you for now.
@pengshanyu. Thanks! Also, your commit message container several duplicate signed-off-by (SOB) next time please remove the dup lines.

Title:
    Foobar 123 123

Description:
    Foobar 123 123

Input:
    Foobar 123 123

Expected Result:
    Foobar 123123

Jira:
    Foobar 123123

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @dougsland, sorry, I didn't notice them. Sure, I'll pay attention next time. Thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not your fault at all, we are starting to create it ;-)

@dougsland dougsland merged commit 8252785 into containers:main Aug 8, 2024
8 checks passed
@pengshanyu pengshanyu deleted the dev-mem-not-accessible branch August 8, 2024 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants