Hi Maxime,
Thanks for the review.
>
> I think this include isn't needed looking at the rest of the patch.
I agree. I will remove this line here.
> > #define VHOST_USER_VERSION 0x1
> >
> > +typedef int (*msg_handler)(struct virtio_net *dev, struct VhostUserMsg
> *msg,
> > + int fd);
> > +
> > +struct vhost_user_dev_priv {
> > + msg_handler vhost_user_msg_handler;
> > + char data[0];
> > +};
> >
> > /* vhost_user.c */
> > int vhost_user_msg_handler(int vid, int fd);
> >
>
> I think the wording is a bit misleading, I'm fine with having a private_data
> pointer, but it should only be used by the external backend.
>
> Maybe what you need here is a new API to be to register a callback for the
> external backend to handle specific requests.
That's exactly what I need.
Shall I rework the code like this?
/* vhost.h */
struct virtio_net {
....
void *extern_data; /*<< private data for external backend */
}
/* vhost_user.h */
typedef int (*msg_handler)(struct virtio_net *dev, struct VhostUserMsg *msg,
int fd);
struct vhost_user_dev_extern {
msg_handler post_vhost_user_msg_handler;
char data[0];
};
int
vhost_user_register_call_back(struct virtio_net *dev, msg_handler
post_msg_handler);
>
> Also, it might be interesting for the external backend to register callbacks
> for
> existing requests. For example .pre_vhost_user_msg_handler
> and .post_vhost_user_msg_handler. Doing so, the external backend could
> for example catch beforehand any change that could affect resources being
> used. Tomasz, Pawel, do you think that could help for the issue you reported?
>
I think it is definitely a good idea. However there will be a problem. As
vhost_crypto does not require pre_vhost_user_msg_handler I think it may not
appropriate to add pre_vhost_user_msg_handler in this patchset.
Thanks a million Maxime.
> Thanks,
> Maxime
