| Message ID | 20230830134055.106812-6-lersek@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | vhost-user: call VHOST_USER_SET_VRING_ENABLE synchronously | expand |
On 8/30/23 15:40, Laszlo Ersek wrote: > In order to avoid a forward-declaration for "vhost_user_write_sync" in a > subsequent patch, hoist "vhost_user_write_sync" -> > "vhost_user_get_features" -> "vhost_user_get_u64" just above > "vhost_set_vring". > > This is purely code movement -- no observable change. > > Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:vhost) > Cc: Eugenio Perez Martin <eperezma@redhat.com> > Cc: German Maglione <gmaglione@redhat.com> > Cc: Liu Jiang <gerry@linux.alibaba.com> > Cc: Sergio Lopez Pascual <slp@redhat.com> > Cc: Stefano Garzarella <sgarzare@redhat.com> > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > --- > > Notes: > v2: > > - pick up R-b from Stefano > > - rename "vhost_user_write_msg" to "vhost_user_write_sync" (in code and > commit message) [Stefano] > > hw/virtio/vhost-user.c | 170 ++++++++++---------- > 1 file changed, 85 insertions(+), 85 deletions(-) Phil reviewed v1: http://mid.mail-archive.com/98150923-39ef-7581-6144-8d0ad8d4dd52@linaro.org and I would've kept his R-b (similar to Stefano's) across the vhost_user_write_msg->vhost_user_write_sync rename in v2; so I'm copying it here: Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Hope that's OK. Laszlo > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index 4129ba72e408..c79b6f77cdca 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -1083,6 +1083,91 @@ static int vhost_user_set_vring_endian(struct vhost_dev *dev, > return vhost_user_write(dev, &msg, NULL, 0); > } > > +static int vhost_user_get_u64(struct vhost_dev *dev, int request, uint64_t *u64) > +{ > + int ret; > + VhostUserMsg msg = { > + .hdr.request = request, > + .hdr.flags = VHOST_USER_VERSION, > + }; > + > + if (vhost_user_per_device_request(request) && dev->vq_index != 0) { > + return 0; > + } > + > + ret = vhost_user_write(dev, &msg, NULL, 0); > + if (ret < 0) { > + return ret; > + } > + > + ret = vhost_user_read(dev, &msg); > + if (ret < 0) { > + return ret; > + } > + > + if (msg.hdr.request != request) { > + error_report("Received unexpected msg type. Expected %d received %d", > + request, msg.hdr.request); > + return -EPROTO; > + } > + > + if (msg.hdr.size != sizeof(msg.payload.u64)) { > + error_report("Received bad msg size."); > + return -EPROTO; > + } > + > + *u64 = msg.payload.u64; > + > + return 0; > +} > + > +static int vhost_user_get_features(struct vhost_dev *dev, uint64_t *features) > +{ > + if (vhost_user_get_u64(dev, VHOST_USER_GET_FEATURES, features) < 0) { > + return -EPROTO; > + } > + > + return 0; > +} > + > +/* Note: "msg->hdr.flags" may be modified. */ > +static int vhost_user_write_sync(struct vhost_dev *dev, VhostUserMsg *msg, > + bool wait_for_reply) > +{ > + int ret; > + > + if (wait_for_reply) { > + bool reply_supported = virtio_has_feature(dev->protocol_features, > + VHOST_USER_PROTOCOL_F_REPLY_ACK); > + if (reply_supported) { > + msg->hdr.flags |= VHOST_USER_NEED_REPLY_MASK; > + } > + } > + > + ret = vhost_user_write(dev, msg, NULL, 0); > + if (ret < 0) { > + return ret; > + } > + > + if (wait_for_reply) { > + uint64_t dummy; > + > + if (msg->hdr.flags & VHOST_USER_NEED_REPLY_MASK) { > + return process_message_reply(dev, msg); > + } > + > + /* > + * We need to wait for a reply but the backend does not > + * support replies for the command we just sent. > + * Send VHOST_USER_GET_FEATURES which makes all backends > + * send a reply. > + */ > + return vhost_user_get_features(dev, &dummy); > + } > + > + return 0; > +} > + > static int vhost_set_vring(struct vhost_dev *dev, > unsigned long int request, > struct vhost_vring_state *ring) > @@ -1255,91 +1340,6 @@ static int vhost_user_set_vring_err(struct vhost_dev *dev, > return vhost_set_vring_file(dev, VHOST_USER_SET_VRING_ERR, file); > } > > -static int vhost_user_get_u64(struct vhost_dev *dev, int request, uint64_t *u64) > -{ > - int ret; > - VhostUserMsg msg = { > - .hdr.request = request, > - .hdr.flags = VHOST_USER_VERSION, > - }; > - > - if (vhost_user_per_device_request(request) && dev->vq_index != 0) { > - return 0; > - } > - > - ret = vhost_user_write(dev, &msg, NULL, 0); > - if (ret < 0) { > - return ret; > - } > - > - ret = vhost_user_read(dev, &msg); > - if (ret < 0) { > - return ret; > - } > - > - if (msg.hdr.request != request) { > - error_report("Received unexpected msg type. Expected %d received %d", > - request, msg.hdr.request); > - return -EPROTO; > - } > - > - if (msg.hdr.size != sizeof(msg.payload.u64)) { > - error_report("Received bad msg size."); > - return -EPROTO; > - } > - > - *u64 = msg.payload.u64; > - > - return 0; > -} > - > -static int vhost_user_get_features(struct vhost_dev *dev, uint64_t *features) > -{ > - if (vhost_user_get_u64(dev, VHOST_USER_GET_FEATURES, features) < 0) { > - return -EPROTO; > - } > - > - return 0; > -} > - > -/* Note: "msg->hdr.flags" may be modified. */ > -static int vhost_user_write_sync(struct vhost_dev *dev, VhostUserMsg *msg, > - bool wait_for_reply) > -{ > - int ret; > - > - if (wait_for_reply) { > - bool reply_supported = virtio_has_feature(dev->protocol_features, > - VHOST_USER_PROTOCOL_F_REPLY_ACK); > - if (reply_supported) { > - msg->hdr.flags |= VHOST_USER_NEED_REPLY_MASK; > - } > - } > - > - ret = vhost_user_write(dev, msg, NULL, 0); > - if (ret < 0) { > - return ret; > - } > - > - if (wait_for_reply) { > - uint64_t dummy; > - > - if (msg->hdr.flags & VHOST_USER_NEED_REPLY_MASK) { > - return process_message_reply(dev, msg); > - } > - > - /* > - * We need to wait for a reply but the backend does not > - * support replies for the command we just sent. > - * Send VHOST_USER_GET_FEATURES which makes all backends > - * send a reply. > - */ > - return vhost_user_get_features(dev, &dummy); > - } > - > - return 0; > -} > - > static int vhost_user_set_vring_addr(struct vhost_dev *dev, > struct vhost_vring_addr *addr) > { >
On 31/8/23 09:20, Laszlo Ersek wrote: > On 8/30/23 15:40, Laszlo Ersek wrote: >> In order to avoid a forward-declaration for "vhost_user_write_sync" in a >> subsequent patch, hoist "vhost_user_write_sync" -> >> "vhost_user_get_features" -> "vhost_user_get_u64" just above >> "vhost_set_vring". >> >> This is purely code movement -- no observable change. >> >> Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:vhost) >> Cc: Eugenio Perez Martin <eperezma@redhat.com> >> Cc: German Maglione <gmaglione@redhat.com> >> Cc: Liu Jiang <gerry@linux.alibaba.com> >> Cc: Sergio Lopez Pascual <slp@redhat.com> >> Cc: Stefano Garzarella <sgarzare@redhat.com> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> >> --- >> >> Notes: >> v2: >> >> - pick up R-b from Stefano >> >> - rename "vhost_user_write_msg" to "vhost_user_write_sync" (in code and >> commit message) [Stefano] >> >> hw/virtio/vhost-user.c | 170 ++++++++++---------- >> 1 file changed, 85 insertions(+), 85 deletions(-) > > Phil reviewed v1: > > http://mid.mail-archive.com/98150923-39ef-7581-6144-8d0ad8d4dd52@linaro.org > > and I would've kept his R-b (similar to Stefano's) across the > vhost_user_write_msg->vhost_user_write_sync rename in v2; so I'm copying > it here: > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > Hope that's OK. Sure! (same for patch 2/7) Thanks :) Phil.
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 4129ba72e408..c79b6f77cdca 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -1083,6 +1083,91 @@ static int vhost_user_set_vring_endian(struct vhost_dev *dev, return vhost_user_write(dev, &msg, NULL, 0); } +static int vhost_user_get_u64(struct vhost_dev *dev, int request, uint64_t *u64) +{ + int ret; + VhostUserMsg msg = { + .hdr.request = request, + .hdr.flags = VHOST_USER_VERSION, + }; + + if (vhost_user_per_device_request(request) && dev->vq_index != 0) { + return 0; + } + + ret = vhost_user_write(dev, &msg, NULL, 0); + if (ret < 0) { + return ret; + } + + ret = vhost_user_read(dev, &msg); + if (ret < 0) { + return ret; + } + + if (msg.hdr.request != request) { + error_report("Received unexpected msg type. Expected %d received %d", + request, msg.hdr.request); + return -EPROTO; + } + + if (msg.hdr.size != sizeof(msg.payload.u64)) { + error_report("Received bad msg size."); + return -EPROTO; + } + + *u64 = msg.payload.u64; + + return 0; +} + +static int vhost_user_get_features(struct vhost_dev *dev, uint64_t *features) +{ + if (vhost_user_get_u64(dev, VHOST_USER_GET_FEATURES, features) < 0) { + return -EPROTO; + } + + return 0; +} + +/* Note: "msg->hdr.flags" may be modified. */ +static int vhost_user_write_sync(struct vhost_dev *dev, VhostUserMsg *msg, + bool wait_for_reply) +{ + int ret; + + if (wait_for_reply) { + bool reply_supported = virtio_has_feature(dev->protocol_features, + VHOST_USER_PROTOCOL_F_REPLY_ACK); + if (reply_supported) { + msg->hdr.flags |= VHOST_USER_NEED_REPLY_MASK; + } + } + + ret = vhost_user_write(dev, msg, NULL, 0); + if (ret < 0) { + return ret; + } + + if (wait_for_reply) { + uint64_t dummy; + + if (msg->hdr.flags & VHOST_USER_NEED_REPLY_MASK) { + return process_message_reply(dev, msg); + } + + /* + * We need to wait for a reply but the backend does not + * support replies for the command we just sent. + * Send VHOST_USER_GET_FEATURES which makes all backends + * send a reply. + */ + return vhost_user_get_features(dev, &dummy); + } + + return 0; +} + static int vhost_set_vring(struct vhost_dev *dev, unsigned long int request, struct vhost_vring_state *ring) @@ -1255,91 +1340,6 @@ static int vhost_user_set_vring_err(struct vhost_dev *dev, return vhost_set_vring_file(dev, VHOST_USER_SET_VRING_ERR, file); } -static int vhost_user_get_u64(struct vhost_dev *dev, int request, uint64_t *u64) -{ - int ret; - VhostUserMsg msg = { - .hdr.request = request, - .hdr.flags = VHOST_USER_VERSION, - }; - - if (vhost_user_per_device_request(request) && dev->vq_index != 0) { - return 0; - } - - ret = vhost_user_write(dev, &msg, NULL, 0); - if (ret < 0) { - return ret; - } - - ret = vhost_user_read(dev, &msg); - if (ret < 0) { - return ret; - } - - if (msg.hdr.request != request) { - error_report("Received unexpected msg type. Expected %d received %d", - request, msg.hdr.request); - return -EPROTO; - } - - if (msg.hdr.size != sizeof(msg.payload.u64)) { - error_report("Received bad msg size."); - return -EPROTO; - } - - *u64 = msg.payload.u64; - - return 0; -} - -static int vhost_user_get_features(struct vhost_dev *dev, uint64_t *features) -{ - if (vhost_user_get_u64(dev, VHOST_USER_GET_FEATURES, features) < 0) { - return -EPROTO; - } - - return 0; -} - -/* Note: "msg->hdr.flags" may be modified. */ -static int vhost_user_write_sync(struct vhost_dev *dev, VhostUserMsg *msg, - bool wait_for_reply) -{ - int ret; - - if (wait_for_reply) { - bool reply_supported = virtio_has_feature(dev->protocol_features, - VHOST_USER_PROTOCOL_F_REPLY_ACK); - if (reply_supported) { - msg->hdr.flags |= VHOST_USER_NEED_REPLY_MASK; - } - } - - ret = vhost_user_write(dev, msg, NULL, 0); - if (ret < 0) { - return ret; - } - - if (wait_for_reply) { - uint64_t dummy; - - if (msg->hdr.flags & VHOST_USER_NEED_REPLY_MASK) { - return process_message_reply(dev, msg); - } - - /* - * We need to wait for a reply but the backend does not - * support replies for the command we just sent. - * Send VHOST_USER_GET_FEATURES which makes all backends - * send a reply. - */ - return vhost_user_get_features(dev, &dummy); - } - - return 0; -} - static int vhost_user_set_vring_addr(struct vhost_dev *dev, struct vhost_vring_addr *addr) {