Re: [RFC PATCH] docs/interop: define STANDALONE protocol feature for vhost-user

[Alex Bennée]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Fri, Jul 07, 2023 at 08:58:00AM +0100, Alex Bennée wrote:
>> 
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Tue, Jul 04, 2023 at 01:36:00PM +0100, Alex Bennée wrote:
>> >> Currently QEMU has to know some details about the back-end to be able
>> >> to setup the guest. While various parts of the setup can be delegated
>> >> to the backend (for example config handling) this is a very piecemeal
>> >> approach.
>> >
>> >> This patch suggests a new feature flag (VHOST_USER_PROTOCOL_F_STANDALONE)
>> >> which the back-end can advertise which allows a probe message to be
>> >> sent to get all the details QEMU needs to know in one message.
>> >
>> > The reason we do piecemeal is that these existing pieces can be reused
>> > as others evolve or fall by wayside.
>> 
>> Sure I have no objection in principle but we then turn code like:
>> 
>>         if (dev->protocol_features & (1ULL << 
>> VHOST_USER_PROTOCOL_F_STANDALONE)) {
>>             err = vhost_user_get_backend_specs(dev, errp);
>>             if (err < 0) {
>>                 error_setg_errno(errp, EPROTO, "vhost_get_backend_specs 
>> failed");
>>                 return -EPROTO;
>>             }
>>         }
>> 
>> to
>> 
>>         if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_ID) &&
>>             dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_CFGSZ) &&
>>             dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MINVQ) &&
>>             dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MAXVQ)
>>         ) {
>>             err = vhost_user_get_virtio_id(dev, errp);
>>             if (err < 0) {
>>                 error_setg_errno(errp, EPROTO, "vhost_get_backend_id 
>> failed");
>>                 return -EPROTO;
>>             }
>>             err = vhost_user_get_virtio_cfgsz(dev, errp);
>>             if (err < 0) {
>>                 error_setg_errno(errp, EPROTO, "vhost_get_backend_cfgsz 
>> failed");
>>                 return -EPROTO;
>>             }
>>             err = vhost_user_get_virtio_minvq(dev, errp);
>>             if (err < 0) {
>>                 error_setg_errno(errp, EPROTO, "vhost_get_backend_minvq 
>> failed");
>>                 return -EPROTO;
>>             }
>>             err = vhost_user_get_virtio_maxvq(dev, errp);
>>             if (err < 0) {
>>                 error_setg_errno(errp, EPROTO, "vhost_get_backend_maxvq 
>> failed");
>>                 return -EPROTO;
>>             }
>>             dev->specs.valid = true;
>>         }
>> 
>> for little gain IMHO.
>>
>> > For example, I can think of instances where you want to connect
>> > specifically to e.g. networking backend, and specify it
>> > on command line. Reasons could be many, e.g. for debugging,
>> > or to prevent connecting to wrong device on wrong channel
>> > (kind of like type safety).
>> 
>> I don't quite follow what you are trying to say here.
>
> That some or all of these might be better on qemu command line
> not come from backend. Then we'll want to *send* it to backend.
> All this at our discretion without protocol changes.

That doesn't solve the standalone problem though (not all VMM's are QEMU
after all). I'm currently putting together a PoC with the
vhost-user-device and I was intending:

 - no CLI args, probe and if nothing fail
 - CLI args, probe with no response, continue with CLI args
 - CLI args, probe with response, check args match (or in bounds for
   vqs) and fail if not

Stefan wasn't super keen on the vhost-user-device in v2 being user
creatable because things could go weird quite quickly in hard to debug
ways:

  Message-Id: <20230418162140.373219-1-alex.bennee@linaro.org>
  Date: Tue, 18 Apr 2023 17:21:27 +0100
  Subject: [PATCH v2 00/13] virtio: add vhost-user-generic and reduce copy and 
paste
  From: =?UTF-8?q?Alex=20Benn=C3=A9e?= <alex.bennee@linaro.org>

However it certainly is useful from a development point of view being
able to plug in new VirtIO backends without having to copy and paste
another slightly different stub into QEMU. I was pondering a middle
ground of maybe making the CLI options all x- variants to emphasise the
"here be dragons please know what you are doing" aspect of them.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro