Show CPU/memory usage in Qube manager again#424
Show CPU/memory usage in Qube manager again#424alimirjamali wants to merge 1 commit intoQubesOS:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #424 +/- ##
==========================================
+ Coverage 69.05% 69.19% +0.13%
==========================================
Files 17 17
Lines 3855 3895 +40
==========================================
+ Hits 2662 2695 +33
- Misses 1193 1200 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
d330493 to
4301eb1
Compare
marmarta
left a comment
There was a problem hiding this comment.
I played around with this PR for a bit, and while I generally love it, I have comments:
- sorting: I think that sorting on usage is kinda problematic right now; it keeps dom0 as top item, which is a bit counterintuitive but consistent with the rest of the code, but also on ascending sorting puts all the non-running VMs on top and the running ones on bottom, which is definitely unintuitive, and on descending sorting puts dom0 at the very end, even though it is definitely running. I think any sorting on usage should put VMs that are consuming resources on top, disregarding special dom0 status too.
- I get different numbers on CPU usage in qube manager and in domains widget, which is not great; there seems to be different rounding, because the error is off-by-one sometimes, but I think they need to be consistent
The suggested changes will be applied. I changed the PR status to draft while I work on it and until I finish it. |
|
Hi @alimirjamali @marmarta After thoroughly going through the files and code, the sorting for CPU and MEM columns currently returns int(vm.CPU_usage) directly. The problem is: The fix I am thinking of — 1) for CPU and MEM sort values, return -1 for non-running VMs so they always sort below running ones, and keep dom0 pinned at the top the same way it is handled in the existing UserRole + 1 sort logic. Would it be okay if I open a PR with this fix building on your work? I will be testing it using MockQubesComplete. |
fixes: QubesOS/qubes-issues#7817