Skip to content

Conversation

taigrr
Copy link
Contributor

@taigrr taigrr commented Mar 19, 2022

Please see #306
The HEAD of main already fails existing test cases, and this patch causes those test cases to pass.

tai@juggernaut:~/code/foss/ghw/pkg/snapshot[jaypipes ?]$ go test
--- FAIL: TestCloneSystemTree (0.07s)
    clonetree_linux_test.go:85: Expected nil err, but got open /sys/devices/system/node/node0/hugepages/hugepages-1048576kB/demote: permission denied
FAIL
exit status 1
FAIL    github.com/jaypipes/ghw/pkg/snapshot    0.076s
tai@juggernaut:~/code/foss/ghw/pkg/snapshot[jaypipes ?]$ git co hugepages-demote
Switched to branch 'hugepages-demote'
Your branch is up to date with 'origin/hugepages-demote'.
tai@juggernaut:~/code/foss/ghw/pkg/snapshot[hugepages-demote ?]$ go test
PASS
ok      github.com/jaypipes/ghw/pkg/snapshot    0.100s
tai@juggernaut:~/code/foss/ghw/pkg/snapshot[hugepages-demote ?]$

@taigrr
Copy link
Contributor Author

taigrr commented Mar 19, 2022

Looks like the project is using go <= 1.15. I'll update the code to be more backwards compatible.

taigrr added 2 commits March 19, 2022 13:06
Signed-off-by: Tai Groot <tai@taigrr.com>
Signed-off-by: Tai Groot <tai@taigrr.com>
@taigrr taigrr force-pushed the hugepages-demote branch from 525c485 to f031448 Compare March 19, 2022 20:06
@ffromani
Copy link
Collaborator

the proper fix would be to NOT copy the write-only files, we had some examples of this already. But we can improve later, this seems like a good enough fix. Thanks!

@ffromani ffromani merged commit c82d7b1 into jaypipes:main Mar 21, 2022
@taigrr
Copy link
Contributor Author

taigrr commented Mar 21, 2022

Aha, I referenced the code here:

buf, err := ioutil.ReadFile(fp)
if err != nil {
if errors.Is(err, os.ErrPermission) {
// example: /sys/devices/virtual/block/zram0/compact is 0400
trace("permission denied reading %q - skipped\n", fp)
continue
}
return err
}

@ffromani
Copy link
Collaborator

Aha, I referenced the code here:

buf, err := ioutil.ReadFile(fp)
if err != nil {
if errors.Is(err, os.ErrPermission) {
// example: /sys/devices/virtual/block/zram0/compact is 0400
trace("permission denied reading %q - skipped\n", fp)

sure, again this fix is good! we can perhaps improve even more, but this can wait a bit. Better to have a working ghw-snapshot sooner than later.

If I get some more ghw time this week, I'll perhaps post a further refinement.

@jaypipes
Copy link
Owner

@taigrr hey, sorry for the late response here, just wanted to say thank you for the bug report and the fix! :)

@taigrr
Copy link
Contributor Author

taigrr commented Mar 24, 2022

@jaypipes no worries, we got there :)

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.

3 participants