From: "Alex Bennée" <alex.bennee@linaro.org>
To: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-devel@nongnu.org, mst@redhat.com,
Coiby Xu <Coiby.Xu@gmail.com>, Hanna Reitz <hreitz@redhat.com>,
"open list:Block layer core" <qemu-block@nongnu.org>
Subject: Re: [PATCH v1 1/5] block/vhost-user-blk-server: don't expose VHOST_USER_F_PROTOCOL_FEATURES
Date: Thu, 28 Jul 2022 11:27:09 +0100 [thread overview]
Message-ID: <87edy5h6qv.fsf@linaro.org> (raw)
In-Reply-To: <YuJM5XY9u8lV3pA6@redhat.com>
Kevin Wolf <kwolf@redhat.com> writes:
> Am 27.07.2022 um 17:56 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>
>> Message-Id: <20220726192150.2435175-6-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 ;
>
> I didn't see this series yet when I replied to the other series this is
> split off from, but of course, my comments are still relevant for this
> one.
>
> I asked for a changed commit message (the "experiment" part should
> probably go away if we're merging it; instead, it should explain that
> in vu_get_features_exec(), libvhost-user adds the vhost-user protocol
> level VHOST_USER_F_PROTOCOL_FEATURES flag anyway and the device is the
> wrong layer to add it, but the behaviour doesn't change with this patch)
> and noted the extra space before the semicolon.
OK - mst do you want me to respin or can you tweak the commit?
>
> Kevin
--
Alex Bennée
next prev parent reply other threads:[~2022-07-28 10:32 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-27 15:56 [PATCH for 7.1 v1 0/5] virtio fixes (split from virtio-gpio series) Alex Bennée
2022-07-27 15:56 ` [PATCH v1 1/5] block/vhost-user-blk-server: don't expose VHOST_USER_F_PROTOCOL_FEATURES Alex Bennée
2022-07-28 8:46 ` Kevin Wolf
2022-07-28 10:27 ` Alex Bennée [this message]
2022-07-27 15:56 ` [PATCH v1 2/5] hw/virtio: incorporate backend features in features Alex Bennée
2022-07-27 15:56 ` [PATCH v1 3/5] hw/virtio: gracefully handle unset vhost_dev vdev Alex Bennée
2022-07-27 15:56 ` [PATCH v1 4/5] hw/virtio: handle un-configured shutdown in virtio-pci Alex Bennée
2022-07-27 15:56 ` [PATCH v1 5/5] hw/virtio: fix vhost_user_read tracepoint Alex Bennée
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87edy5h6qv.fsf@linaro.org \
--to=alex.bennee@linaro.org \
--cc=Coiby.Xu@gmail.com \
--cc=hreitz@redhat.com \
--cc=kwolf@redhat.com \
--cc=mst@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).