Re: [PATCH v2] crosvm: Rename `--vhost-user-{fs,gpu}` args

[Dom Rodriguez <shymega@shymega.org.uk>]

[-- Attachment #1: Type: text/plain, Size: 4101 bytes --]

Hey,

Sorry for the super late reply. I was flying back home on the 28th, and
then I had to prepare for a new job - room organising, laptop
configuring, onboarding, etc :-/

On 28.09.2024 16:27, Alyssa Ross wrote:
>Dom Rodriguez <shymega@shymega.org.uk> writes:
>
>> On 07.09.2024 18:40, Alyssa Ross wrote:
>>>Dom Rodriguez <shymega@shymega.org.uk> 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,socket=$PATH`, where `$DEVICE` is, in this case,
>>>> `gpu` or `fs`, and `$PATH` is the path to the Unix socket.
>>>>
>>>> Signed-off-by: Dom Rodriguez <shymega@shymega.org.uk>
>>>> ---
>>>>  img/app/Makefile                   | 4 ++--
>>>>  release/checks/wayland/default.nix | 2 +-
>>>>  2 files changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/img/app/Makefile b/img/app/Makefile
>>>> index 3adf8c4..11ef6e1 100644
>>>> --- a/img/app/Makefile
>>>> +++ b/img/app/Makefile
>>>> @@ -152,8 +152,8 @@ run-crosvm: $(imgdir)/appvm/blk/root.img start-vhost-user-gpu start-virtiofsd
>>>>  	    --disk $(RUN_IMG) \
>>>>  	    -p "console=ttyS0 root=PARTLABEL=root" \
>>>>  	    --net tap-name=tap0 \
>>>> -	    --vhost-user-fs build/virtiofsd.sock:virtiofs0 \
>>>> -	    --vhost-user-gpu build/vhost-user-gpu.sock \
>>>> +	    --vhost-user fs,socket=build/virtiofsd.sock:virtiofs0 \
>>>> +	    --vhost-user gpu,socket=build/vhost-user-gpu.sock \
>>>>  	    --vsock cid=3 \
>>>>  	    --serial type=file,hardware=serial,path=build/serial.log \
>>>>  	    --serial type=stdout,hardware=virtio-console,stdin=true \
>>>
>>>This is still not right, I'm afraid.
>>>
>>>When I run nix-shell --run 'make clean && make run' VMM=crosvm in
>>>img/app, I get this error:
>>>
>>>[2024-09-07T16:36:32.592274891+00:00 ERROR crosvm] exiting with error 1: failed to connect to vhost-user socket path build/virtiofsd.sock:virtiofs0
>>>
>>>Probably the tag should also be a comma-separated key=value option?
>>>
>>>(This doesn't have any automated test, because it's just part of the
>>>development environment, and setting up a test environment to resemble a
>>>development machine isn't trivial.  Should be possible though.)
>>
>> It's bizarre. I've done some more testing, and it seems that
>> `--vhost-user` isn't *quite* there yet on feature parity with
>> `--vhost-user-fs`. Maybe I'm going wrong here, but it doesn't recognise
>> the tag as a k/v option.
>>
>> I have managed to get the tests running with `--vhost-user-fs`, but it
>> does look mismatched now.
>>
>> Would you prefer I revert the changes to `--vhost-user fs` =>
>> `--vhost-user-fs`?
>
>You're right!  I've just spent the last little while looking around
>trying to figure out what's up here, and I think I've figured it out.
>
>In this commit[1] they made the vhost-user-fs tag optional, because it
>can now be set with virtiosfd instead of the VMM.  This is probably
>good, because the more generic vhost-user becomes, the more hope
>Spectrum has of one day not needing Cloud Hypervisor patches. :)  So we
>could try using --vhost-user fs for crosvm, and then passing --tag to
>virtiofsd, but it'd need to be tested with VMM=qemu and
>VMM=cloud-hypervisor as well, because I'm not sure whether they support
>a backend-provided tag.

Oh, that explains *a lot*! I agree, a generic approach would be
preferable.

>If that's too much work, I'd also be happy to apply just the GPU part of
>this patch.  Just let me know. :)

I've started a new job, and so I've only really got weekends free - and
even then, I have Repair Cafes to volunteer at. 

If I send over the GPU patch only, can you apply that, and then we can
discuss on Matrix/IRC the best approach for `vhost-user-fs`? Feel free
to tag me - I'm mostly using IRC rightn ow.

Thanks.

>[1]: https://chromium.googlesource.com/crosvm/crosvm/+/b3f46c8fcbb0ce1047c4a5ca2c26c66e29e8eb44


Best wishes,
--
Dom Rodriguez

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]