| Message ID | 20220726192150.2435175-6-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | virtio-gpio and various virtio cleanups | expand |
Am 26.07.2022 um 21:21 hat Alex Bennée geschrieben: > This bit is unused in actual VirtIO feature negotiation and should > only appear in the vhost-user messages between master and slave. > > [AJB: experiment, this doesn't break the tests but I'm not super > confident of the range of tests] > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> I guess the rationale is that this doesn't make a difference because vu_get_features_exec(), which is the only caller of .get_features(), adds the VHOST_USER_F_PROTOCOL_FEATURES bit back before sending a response. Can you please add this to the commit message? > block/export/vhost-user-blk-server.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c > index 3409d9e02e..d31436006d 100644 > --- a/block/export/vhost-user-blk-server.c > +++ b/block/export/vhost-user-blk-server.c > @@ -125,8 +125,7 @@ static uint64_t vu_blk_get_features(VuDev *dev) > 1ull << VIRTIO_BLK_F_MQ | > 1ull << VIRTIO_F_VERSION_1 | > 1ull << VIRTIO_RING_F_INDIRECT_DESC | > - 1ull << VIRTIO_RING_F_EVENT_IDX | > - 1ull << VHOST_USER_F_PROTOCOL_FEATURES; > + 1ull << VIRTIO_RING_F_EVENT_IDX ; This has an extra space before the semicolon. Kevin
在 2022/7/27 03:21, Alex Bennée 写道: > This bit is unused in actual VirtIO feature negotiation and should > only appear in the vhost-user messages between master and slave. I might be wrong, but this is actually used between master and slave not the device and driver? Thanks > > [AJB: experiment, this doesn't break the tests but I'm not super > confident of the range of tests] > > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > block/export/vhost-user-blk-server.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c > index 3409d9e02e..d31436006d 100644 > --- a/block/export/vhost-user-blk-server.c > +++ b/block/export/vhost-user-blk-server.c > @@ -125,8 +125,7 @@ static uint64_t vu_blk_get_features(VuDev *dev) > 1ull << VIRTIO_BLK_F_MQ | > 1ull << VIRTIO_F_VERSION_1 | > 1ull << VIRTIO_RING_F_INDIRECT_DESC | > - 1ull << VIRTIO_RING_F_EVENT_IDX | > - 1ull << VHOST_USER_F_PROTOCOL_FEATURES; > + 1ull << VIRTIO_RING_F_EVENT_IDX ; > > if (!vexp->handler.writable) { > features |= 1ull << VIRTIO_BLK_F_RO;
Jason Wang <jasowang@redhat.com> writes: > 在 2022/7/27 03:21, Alex Bennée 写道: >> This bit is unused in actual VirtIO feature negotiation and should >> only appear in the vhost-user messages between master and slave. > > > I might be wrong, but this is actually used between master and slave > not the device and driver? Argh you may be right. However got confused with grepping: static const VuDevIface vu_blk_iface = { .get_features = vu_blk_get_features, .queue_set_started = vu_blk_queue_set_started, .get_protocol_features = vu_blk_get_protocol_features, .get_config = vu_blk_get_config, .set_config = vu_blk_set_config, .process_msg = vu_blk_process_msg, }; and this is all part of libvhostuser but vhost-user-blk-server is in the main tree. I guess it isn't moved into tools/ because it also re-uses bits of the block layer? Anyway I shall drop this patch. > > Thanks > > >> >> [AJB: experiment, this doesn't break the tests but I'm not super >> confident of the range of tests] >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> block/export/vhost-user-blk-server.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c >> index 3409d9e02e..d31436006d 100644 >> --- a/block/export/vhost-user-blk-server.c >> +++ b/block/export/vhost-user-blk-server.c >> @@ -125,8 +125,7 @@ static uint64_t vu_blk_get_features(VuDev *dev) >> 1ull << VIRTIO_BLK_F_MQ | >> 1ull << VIRTIO_F_VERSION_1 | >> 1ull << VIRTIO_RING_F_INDIRECT_DESC | >> - 1ull << VIRTIO_RING_F_EVENT_IDX | >> - 1ull << VHOST_USER_F_PROTOCOL_FEATURES; >> + 1ull << VIRTIO_RING_F_EVENT_IDX ; >> if (!vexp->handler.writable) { >> features |= 1ull << VIRTIO_BLK_F_RO;
diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c index 3409d9e02e..d31436006d 100644 --- a/block/export/vhost-user-blk-server.c +++ b/block/export/vhost-user-blk-server.c @@ -125,8 +125,7 @@ static uint64_t vu_blk_get_features(VuDev *dev) 1ull << VIRTIO_BLK_F_MQ | 1ull << VIRTIO_F_VERSION_1 | 1ull << VIRTIO_RING_F_INDIRECT_DESC | - 1ull << VIRTIO_RING_F_EVENT_IDX | - 1ull << VHOST_USER_F_PROTOCOL_FEATURES; + 1ull << VIRTIO_RING_F_EVENT_IDX ; if (!vexp->handler.writable) { features |= 1ull << VIRTIO_BLK_F_RO;
This bit is unused in actual VirtIO feature negotiation and should only appear in the vhost-user messages between master and slave. [AJB: experiment, this doesn't break the tests but I'm not super confident of the range of tests] Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- block/export/vhost-user-blk-server.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)