Hi!
On Tue, May 9, 2023 at 12:12 PM Marc-André Lureau <
marcandre.lur...@gmail.com> wrote:
> Hi
>
> On Wed, May 3, 2023 at 12:21 PM Albert Esteve <aest...@redhat.com> wrote:
>
>> Add vu_write_msg_cb type as a member of the VuDev
>> struct.
>>
>> In order to interact with the virtio-dmabuf
>> API, vhost-user backends have available a special
>> message type that can be sent to the frontend
>> in Qemu, in order to add, lookup, or remove
>> entries.
>>
>> To send these messages and avoid code replication,
>> backends will need the write_msg method to be exposed
>> to them, similarly to how the read_msg is for
>> receiving messages.
>>
>
> I think read_msg was introduced to blend libvhost-user IO to qemu mainloop
> & coroutine. Is that what you have in mind for write_msg?
>
Uhm, after grep'ing, it seems that read_msg is only used within
libvhost-user source, so I guess it is mainly used to
allow backends to provide custom methods? Maybe I am misunderstanding.
But my idea for adding `write_msg` is exposing the write method (i.e.,
vu_message_write) to the backends,
without having the function signature in the header. This way, vhost-user
backends that want to write a message,
can just use `dev->write_msg(args...)`. Which would be equivalent to
`vu_message_write(args...)` if this
was visible for others.
Another option could be to have a specific public method sending the
requests to the frontend, that
internally, would use `vu_message_write`. But since we introduce three new
message types that
backends can send, I thought adding different methods would be a bit too
verbose.
>
>> Signed-off-by: Albert Esteve <aest...@redhat.com>
>> ---
>> subprojects/libvhost-user/libvhost-user.c | 1 +
>> subprojects/libvhost-user/libvhost-user.h | 16 ++++++++++++++++
>> 2 files changed, 17 insertions(+)
>>
>> diff --git a/subprojects/libvhost-user/libvhost-user.c
>> b/subprojects/libvhost-user/libvhost-user.c
>> index 6b4b721225..c50b353915 100644
>> --- a/subprojects/libvhost-user/libvhost-user.c
>> +++ b/subprojects/libvhost-user/libvhost-user.c
>> @@ -2115,6 +2115,7 @@ vu_init(VuDev *dev,
>> dev->sock = socket;
>> dev->panic = panic;
>> dev->read_msg = read_msg ? read_msg : vu_message_read_default;
>> + dev->write_msg = vu_message_write;
>>
>
> You are not making it customizable? And the callback is not used.
>
Making it customizable would require changing the signature of `vu_init`,
and I did not see
the need for this usecase. I just want to expose the static method to the
backends.
The callback is not used because there is still no virtio device to use it.
But this whole
infrastructure will be nice to have for the next device that would require
it (e.g., virtio-video).
In that regard, this commit could be skipped from the PATCH and just change
it once there
is a virtio device that needs to send a `VHOST_USER_BACKEND_SHARED_OBJECT`
message. Basically, I needed it for testing (just had a dummy vhost-user
backend that I used for
sending messages), and then decided to keep it. But maybe having a simpler
patch is better.
>
>
>> dev->set_watch = set_watch;
>> dev->remove_watch = remove_watch;
>> dev->iface = iface;
>> diff --git a/subprojects/libvhost-user/libvhost-user.h
>> b/subprojects/libvhost-user/libvhost-user.h
>> index 784db65f7c..f5d7162886 100644
>> --- a/subprojects/libvhost-user/libvhost-user.h
>> +++ b/subprojects/libvhost-user/libvhost-user.h
>> @@ -242,6 +242,7 @@ typedef void (*vu_set_features_cb) (VuDev *dev,
>> uint64_t features);
>> typedef int (*vu_process_msg_cb) (VuDev *dev, VhostUserMsg *vmsg,
>> int *do_reply);
>> typedef bool (*vu_read_msg_cb) (VuDev *dev, int sock, VhostUserMsg
>> *vmsg);
>> +typedef bool (*vu_write_msg_cb) (VuDev *dev, int sock, VhostUserMsg
>> *vmsg);
>> typedef void (*vu_queue_set_started_cb) (VuDev *dev, int qidx, bool
>> started);
>> typedef bool (*vu_queue_is_processed_in_order_cb) (VuDev *dev, int qidx);
>> typedef int (*vu_get_config_cb) (VuDev *dev, uint8_t *config, uint32_t
>> len);
>> @@ -429,6 +430,21 @@ struct VuDev {
>> */
>> vu_read_msg_cb read_msg;
>>
>> + /*
>> + * @write_msg: custom method to write vhost-user message
>> + *
>> + * Write data to vhost_user socket fd from the passed
>> + * VhostUserMsg *vmsg struct.
>> + *
>> + * For the details, please refer to vu_message_write in
>> libvhost-user.c
>> + * which will be used by default when calling vu_unit.
>> + * No custom method is allowed.
>>
>
> "No custom method is allowed"?
>
I meant that I am not making it customizable (from your previous point),
as opposed to the `read_msg` method.
>
>
>> + *
>> + * Returns: true if vhost-user message successfully sent, false
>> otherwise.
>> + *
>> + */
>> + vu_write_msg_cb write_msg;
>> +
>>
>
>
> --
> Marc-André Lureau
>
