Add disk2 status and update volume retrieval method#23
Conversation
Add free space for a secondary storage. The volume displays the Sys-Audio volume if it is not managed by dom0.
marmarek
left a comment
There was a problem hiding this comment.
This script already uses qubesadmin module, don't use subprocess for calling qvm-* tools - it's quite expensive, especially for things called every few seconds.
See status_net(). You may want to use run_with_args() method instead of run - it's a bit faster.
And see also comment next to the status_net() call - which you conveniently removed - don't... But you do validate the output, so it's probably okay here. But check if sys-audio is running first, to not start it unintentionally.
|
ok, I'll put the comment back as it was originally for |
Yes. Or even better, take global
Sounds ok.
Yes, at least qubes-i3status should not start it on its own. |
I modify "status_volume()" and "status_volume_dom0()". For the Disks, instead of displaying "disk" en "disk2", it displays the name of pools
|
As you wanted, I modified the script for the volume: Now,
I restore comment next to the status_net() . For the disks, instead of displaying "disk" and "disk2". The names of the pools are displayed directly, I found it more readable because now, it displays the same names as in the Qubes Disk Space Monitor. |
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2026060617-devel&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2026050504-devel&flavor=update
Failed tests22 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/176874#dependencies 22 fixed
Unstable testsDetails
Performance TestsPerformance degradation:20 performance degradations
Remaining performance tests:91 tests
|
ben-grande
left a comment
There was a problem hiding this comment.
I like what has been done and I don't want to discourage you with the amount of comments I made.
| usage = int(pool.usage) | ||
| free = size - usage | ||
|
|
||
| # Détermination of free size |
| return json_output("volume", f"{vm} : {volume_pct} %", color) | ||
|
|
||
|
|
||
| def status_volume_dom0(): |
There was a problem hiding this comment.
No dom0, might be a guivm that is also an audiovm.
There was a problem hiding this comment.
I would like to known what is the problem with guivm in it.
I just configure the audiovm and do nothing with the guivm in the script :/
|
|
||
| if all_off: | ||
| return json_output("volume", "Volume: mute") | ||
| return json_output("volume", "dom0: mute") |
There was a problem hiding this comment.
|
|
||
| def status_volume(): | ||
|
|
||
| # find default audio-vm |
There was a problem hiding this comment.
We don't refer to AudioVM as audio-vm.
| def status_volume(): | ||
|
|
||
| # find default audio-vm | ||
| vm = getattr(app, "default_audiovm", None) |
There was a problem hiding this comment.
I think this information should either be cached in qubes.api.internal.SystemInfoCache or you should query it once and handle events that change this data. See other repos for dispatcher.add_handler.
| else: | ||
| disk2_free = f"{free} Bytes" | ||
|
|
||
| return json_output("disk2", f"{pool_name}: {disk2_free}") |
There was a problem hiding this comment.
I don't think the entry can remain disk2 or return the output right know. It is in a loop, meaning it will only get the result of the first pool. If all are disk2, it will be overwitten? I don't think two entries can have the same identifier. Add diskN, where N increases per iteration.
|
|
||
|
|
||
| def status_disk2(): | ||
| default_pool_name = app.property_get_default("default_pool_private") |
There was a problem hiding this comment.
|
|
||
| def status_disk2(): | ||
| default_pool_name = app.property_get_default("default_pool_private") | ||
| pools = app.pools |
There was a problem hiding this comment.
| continue | ||
|
|
||
| # Filter only pools with a driver of 'lvm_thin' | ||
| if getattr(pool, "driver", None) != "lvm_thin": |
There was a problem hiding this comment.
Why? What about people using other file systems?
There was a problem hiding this comment.
you're right, i just think for myself because i just use lvm lol
| return entry | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Remove this extra line please.
|
Thank you for checking my PR, and for your recommendations. |
Add free space for a secondary storage.
The volume displays the Sys-Audio volume if it is not managed by dom0.
I test it on 4.2 and 4.3rc4