Changes to speed up qvm-ls#36
Conversation
qubesuser
force-pushed
the
fast_qvm_ls
branch
5 times, most recently
from
949275c to
d8246fb
Compare
This is much more efficient.
This makes it get all the data with a single qubesd call.
If qubesd returns a label name, we can just assume it's valid. This makes qvm-ls take only one qubesd call.
It's slow and unnecessary
Tools should be written to be fast enough that spinners don't become necessary.
qubesuser
force-pushed
the
fast_qvm_ls
branch
from
d8246fb to
671150d
Compare
| vm_state = vm_list_info[0].strip().partition('state=')[2].split(' ')[0] | ||
| return vm_state | ||
|
|
||
| if not self._state or force: |
There was a problem hiding this comment.
Ouch, this will break all the long-running processes using vm.is_running() or such. One example is qvm-shutdown --wait. IMO caching of VM state should be opt-in, not opt-out.
marmarek
left a comment
marmarek
left a comment
There was a problem hiding this comment.
I haven't reviewed all of this properly. Will do when we agree on the final backend side shape. But at least separate GetAll from GetAllData support - currently parts of the latter are in commit related to the former.
| except qubesadmin.exc.QubesDaemonNoResponseError: | ||
| raise qubesadmin.exc.QubesPropertyAccessError(key) | ||
|
|
||
| self._set_item(key, value, False) |
There was a problem hiding this comment.
This is fragile. Especially for long-living processes - anyone else can modify value in the meantime. But also any extension is free to modify value in event handler, including event caused by this very property set.
So, if do this at all, at least make sure the application also receive events. Or at least have it opt-in (and explicitly enable, only in short-lived processes, like some qvm-* tools).
| hideseq = '\n' | ||
|
|
||
| self.stream.write(hideseq) | ||
| self.stream.flush() |
|
@marmarek Are we still interested in adding Are there other parts of this old PR that we are still interested in ? (if so I can try to take over) |
3 participants
This is the client part of a bunch of changes that get qvm-ls to around 200ms run time.
Warning: I haven't tested them much, and they change fundamental code, so they probably need fixes.
In particular, all properties are now cached by default, so anything that expects to see changes is now broken and needs to be fixed.
This makes use of the new APIs introduced in QubesOS/qubes-core-admin#165, and also contains a bunch of optimizations of the Python code.