On 08.09.2024 09:46, Alyssa Ross wrote: >Dom Rodriguez writes: > >> On 06.09.2024 13:22, Alyssa Ross wrote: >>>Dom Rodriguez writes: >>> >>>> crosvm was producing warnings when using `--vhost-user-gpu` and >>>> `--vhost-user-fs`. >>>> >>>> In this commit, I have adjusted the `crosvm` invocations to look >>>> something like `--vhost-user $DEVICE`, where `$DEVICE` is, in this case, >>>> `gpu` or `fs`. >>>> >>>> I have run unit tests, but would appreciate testing. >>> >>>How did you run them? release/checks/wayland is failing: >>> >>>vm-test-run-spectrum-wayland> machine # [ 14.028205] crosvm[968]: [2024-09-06T11:19:20.132395456+00:00 ERROR crosvm] arg parsing failed: Error parsing option '--vhost-user' with value 'gpu': missing field `socket` >>>vm-test-run-spectrum-wayland> machine # [ 14.029442] crosvm[968]: [src/main.rs:737] arg parsing failed: Error parsing option '--vhost-user' with value 'gpu': missing field `socket` >>> >>>Does it need to be (from memory) --vhost-user gpu,socket=path or something? >> >> I thought I had tested them, but as it turned out, the command I used >> was only for shellcheck. I've run unit tests again, and all seems to >> pass. I've queued up the v2 patch for review. > >Do you think we could change anything about the documentation to avoid >people making that mistake in future? It's hard to anticipate what will >confuse people in advance. From my newcomer pespective, I assumed the command on the documentation for testing ran all checks. I just blindly copied and pasted it. That was an error on my part, and I should have known better. I think this is more of a PEBKAC problem with me, rather than the documentation. >> Just a side note from my article about Flakes - we could use the >> `checks` output to run on the `pre-commit` Git hook. This could then >> check the commit message, run tests, etc. > >We could do that already, right? There's already a command you can run >to run all the checks. My concern would be that some of the checks are >slow (the ones that involve building images), so I think we might want a >release/checks/fast.nix or something so it's not too annoying — I find >slow pre-commit hooks to be a real drag if I want to keep rewording or >slightly modifying a commit. The flake eval cache would certainly help >with the other slow bit (Nix eval), though. I agree that the checks as a pre-commit hook would be slow. I would like to integrate a CI environment on the list for each patch. Perhaps we could use your suggestion of 'fast checks', and the CI could run all of the checks upon receiving a patch? I should have discussed this in the Flakes thread, so my suggestion is that we continue discussion there, and in Matrix/IRC if the need arises. Best wishes, -- Dom Rodriguez GPG Fingerprint: EB0D 45E6 D0DC 1BA1 A2B5 FC24 72DC F123 1E54 BD43