On Wed, May 15, 2024 at 1:47 AM Li Feng wrote:
>
>
>
> > 2024年5月14日 21:58，Raphael Norwitz 写道：
> >
> > The code for these two patches looks fine. Just some questions on the
> > failure case you're trying to fix.
> >
> >
> > On Tue, May 14
The case your describing makes sense but now I have some concerns on
the vhost_dev_cleanup bit.
On Wed, May 15, 2024 at 1:47 AM Li Feng wrote:
>
>
>
> > 2024年5月14日 21:58，Raphael Norwitz 写道：
> >
> > Code looks good. Just a question on the error case you're tryin
OK - I'm happy with this approach then.
On Wed, May 15, 2024 at 10:48 PM Li Feng wrote:
>
>
>
> 2024年5月15日 23:47，Raphael Norwitz 写道：
>
> The case your describing makes sense but now I have some concerns on
> the vhost_dev_cleanup bit.
>
> On Wed, May 15
from the guest os,
> s->connected has no chance to be set to false, resulting in
> subsequent reconnection not being executed.
>
> The next patch will completely fix this issue with a better approach.
>
Reviewed-by: Raphael Norwitz
> Signed-off-by: Li Feng
>
ensure that even if vhost_dev_init initialization fails, the event
> handler still needs to be reinstalled when s->connected is false.
>
> All vhost-user devices have this issue, including vhost-user-blk/scsi.
>
> Fixes: 71e076a07d ("hw/virtio: generalise CHR_EVENT_CLOSED handlin
I have no issues with these APIs, but I'm not a QMP expert so others
should review those bits.
For the vhost-user-blk code:
Acked-by: Raphael Norwitz
On Tue, Jun 25, 2024 at 8:19 AM Vladimir Sementsov-Ogievskiy
wrote:
>
> v5:
> 03: drop extra check on is is runstate running
g at the
vhost level. I do agree we shouldn't re-use vdev->started.
Maybe we should add another 'active' variable in vhost_dev? I'm happy
to send a patch for that.
Until we agree on a better solution I'm happy with the FIXME.
Reviewed-by: Raphael Norwitz
>
>
> ping
Apologies for the late review - busy week. First pass looks good but will review
comprehensively tomorrow or over the weekend.
I feel like it would be easier to review if the first 4 patches were
squashed together, but that’s not a big deal.
For this one:
Reviewed-by: Raphael Norwitz
On Fri, Aug 26, 2022 at 05:32:41PM +0300, Daniil Tatianin wrote:
> This is the first step towards moving all device config s
On Fri, Aug 26, 2022 at 05:32:42PM +0300, Daniil Tatianin wrote:
> Use the common helper instead of duplicating the same logic.
>
> Signed-off-by: Daniil Tatianin
Reviewed-by: Raphael Norwitz
> ---
> hw/block/virtio-blk.c | 16 +++-
> 1 file changed, 7 insertio
The vhost-user-blk bits in meson.build and Maintainers should probably
be in patch 8?
Otherwise LGTM.
On Fri, Aug 26, 2022 at 05:32:45PM +0300, Daniil Tatianin wrote:
> This way we can reuse it for other virtio-blk devices, e.g
> vhost-user-blk, which currently does not control its config space s
On Fri, Aug 26, 2022 at 05:32:46PM +0300, Daniil Tatianin wrote:
> It is useful to have the ability to disable these features for
> compatibility with older VMs that don't have these implemented.
>
> Signed-off-by: Daniil Tatianin
Reviewed-by: Raphael Norwitz
> ---
&g
On Fri, Aug 26, 2022 at 05:32:47PM +0300, Daniil Tatianin wrote:
> No reason to have this be a separate field. This also makes it more akin
> to what the virtio-blk device does.
>
> Signed-off-by: Daniil Tatianin
Reviewed-by: Raphael Norwitz
> ---
> hw/block/vhost-user-
On Fri, Aug 26, 2022 at 05:32:43PM +0300, Daniil Tatianin wrote:
> Use the new common helper. As an added bonus this also makes use of
> config size sanity checking via the 'max_size' field.
>
> Signed-off-by: Daniil Tatianin
> ---
> hw/net/virtio-net.c | 8 ++--
> 1 file changed, 6 insertio
On Fri, Aug 26, 2022 at 05:32:44PM +0300, Daniil Tatianin wrote:
> This has no more users and is superseded by virtio_get_config_size.
>
> Signed-off-by: Daniil Tatianin
Reviewed-by: Raphael Norwitz
> ---
> hw/virtio/virtio.c | 15 ---
> include/hw/vi
of migration failed: Invalid argument
>
> This is caused by the newer (destination) VM requiring a bigger BAR0
> alignment because it has to cover a bigger configuration space, which
> isn't actually needed since those additional config fields are not
> active (write-zeroes/discard).
> I can squash the first four if that would be better.
I think so. Just may be easier for other reviewers :)
Thanks for the changes. For the whole series:
Reviewed-by: Raphael Norwitz
On Tue, Sep 06, 2022 at 10:31:06AM +0300, Daniil Tatianin wrote:
> This patch set attempts to align vhost-user-blk with virtio-blk in
> terms of backward compatibility and flexibility. It also improves
> the vi
> Thanks for reviewing! Could you send a Pull request? Or do we need an
> ack from someone else?
mst typically includes the vhost-user-blk patches in his PRs. Usually a few
other people review but I'm not sure it's required.
A lot of folks have been busy prepping for KVM Forum the last few weeks
Feng fen...@smartx.com<mailto:fen...@smartx.com>
Reviewed-by: Raphael Norwitz
>---
> hw/block/vhost-user-blk.c | 4
> 1 file changed, 4 insertions(+)
>
>diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
>index 9117222456..db30bb754f 100644
&g
I have some concerns from the vhost-user-blk side.
>On Tue, Sep 13, 2022 at 5:13 PM Hao Chen wrote:
>>
>> When use dpdk-vdpa tests vdpa device. You need to specify the mac address to
>> start the virtual machine through libvirt or qemu, but now, the libvirt or
>> qemu can call dpdk vdpa ven
If I read your response on the other thread correctly, this change is intended
to prioritize the MAC address exposed by DPDK over the one provided by the
QEMU command line? Sounds reasonable in principle, but I would get confirmation
from vDPA/vhost-net maintainers.
That said the way you’re h
for ring 0
qemu-system-x86_64: Verify ring failure on region 0
This has already been fixed for vhost scsi devices and was
recently vhost-user scsi devices. This commit fixes it for
vhost-user-blk devices.
Suggested-by: Phillippe Mathieu-Daude
Signed-off-by: Raphael Norwitz
---
hw/block/vhost
On Sun, Mar 29, 2020 at 09:30:24AM -0400, Michael S. Tsirkin wrote:
>
> On Mon, Mar 23, 2020 at 01:29:24PM +0800, Li Feng wrote:
> > the G_IO_HUP is watched in tcp_chr_connect, and the callback
> > vhost_user_blk_watch is not needed, because tcp_chr_hup is registered as
> > callback. And it will c
On Mon, Mar 23, 2020 at 01:29:24PM +0800, Li Feng wrote:
>
> the G_IO_HUP is watched in tcp_chr_connect, and the callback
> vhost_user_blk_watch is not needed, because tcp_chr_hup is registered as
> callback. And it will close the tcp link.
>
> Signed-off-by: Li Feng
Re
I’m happy from the vhost, vhost-user-blk and vhost-user-scsi side. For
other device types it looks pretty straightforward, but their maintainers
should probably confirm.
Since you plan to change the behavior of these helpers in subsequent
patches, maybe consider sending the other device types sepa
Apologies for mixing up patches last time. This looks good from a
vhost-user-blk perspective, but I worry that some of these changes
could impact other vhost device types.
I agree with adding notifiers_set to struct vhost_dev, and setting it in
vhost_dev_enable/disable notifiers, but is there any
to SIGSEGV. Add checks for the vring address value before calling unmap.
> Also add assert() in the vhost_memory_unmap() routine.
>
> Signed-off-by: Dima Stepanov
Reviewed-by: Raphael Norwitz
> ---
> hw/virtio/vhost.c | 27 +--
> 1 file changed, 21 inser
As you correctly point out, this code needs to be looked at more
carefully so that
if the device does disconnect in the background we can handle the migration path
gracefully. In particular, we need to decide whether a migration
should be allowed
to continue if a device disconnects durning the migr
On Thu, May 7, 2020 at 11:35 AM Dima Stepanov wrote:
>
> What do you think?
>
Apologies - I tripped over the if (dev->started && r < 0) check.
Never-mind my point with race conditions and failing migrations.
Rather than modifying vhost_dev_set_log(), it may be clearer to put a
check after vhost_
I'm mostly happy with this. A couple comments.
On Wed, May 20, 2020 at 11:54 AM Dima Stepanov wrote:
>
> A socket write during vhost-user communication may trigger a disconnect
> event, calling vhost_user_blk_disconnect() and clearing all the
> vhost_dev structures holding data that vhost-user fu
; support them if we want to use them.
>
> Signed-off-by: Stefan Hajnoczi
Reviewed-by: Raphael Norwitz
> ---
> hw/scsi/vhost-scsi.c | 2 ++
> hw/scsi/vhost-user-scsi.c | 2 ++
> 2 files changed, 4 insertions(+)
>
> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-
device backend can declare whether or not it supports the packed
> ring layout.
>
> Signed-off-by: Stefan Hajnoczi
Reviewed-by: Raphael Norwitz
> ---
> hw/block/vhost-user-blk.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/block/vhost-user-blk.c b/hw/bl
On Mon, May 25, 2020 at 4:58 AM Dima Stepanov wrote:
>
> On Wed, May 20, 2020 at 06:53:13PM +0300, Dima Stepanov wrote:
> > A socket write during vhost-user communication may trigger a disconnect
> > event, calling vhost_user_blk_disconnect() and clearing all the
> > vhost_dev structures holding d
t; accidentally be compiled out.
>
> Suggested-by: Markus Armbruster
> Signed-off-by: Stefan Hajnoczi
Reviewed-by: Raphael Norwitz
> ---
> contrib/vhost-user-blk/vhost-user-blk.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/vhost-user-bl
BH changes are based on the similar changes for the vhost-user-net
> device:
> commit e7c83a885f865128ae3cf1946f8cb538b63cbfba
> "vhost-user: delay vhost_user_stop"
>
> Signed-off-by: Dima Stepanov
Reviewed-by: Raphael Norwitz
Li Feng - would you also li
I'm happy with the code but as David pointed out with virtio-scsi, we
should probably add a comment about virtio_pci_optimal_num_queues()
capping the number of VQs here too.
On Wed, May 27, 2020 at 6:34 AM Stefan Hajnoczi wrote:
>
> Automatically size the number of request virtqueues to match th
BUS(&vpci_dev->bus));
> diff --git a/hw/virtio/virtio-scsi-pci.c b/hw/virtio/virtio-scsi-pci.c
> index e82e7e5680..c52d68053a 100644
> --- a/hw/virtio/virtio-scsi-pci.c
> +++ b/hw/virtio/virtio-scsi-pci.c
> @@ -51,7 +51,8 @@ static void virtio_scsi_pci_realize(VirtIOPCIProxy
> *vpci_dev, Error **errp)
> char *bus_name;
>
> if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
> -vpci_dev->nvectors = vs->conf.num_queues + 3;
> +vpci_dev->nvectors = vs->conf.num_queues +
> + VIRTIO_SCSI_VQ_NUM_FIXED + 1;
> }
>
> /*
> --
> 2.25.4
>
Reviewed-by: Raphael Norwitz
On Mon, Jul 6, 2020 at 7:00 AM Stefan Hajnoczi wrote:
>
> Automatically size the number of request virtqueues to match the number
> of vCPUs. This ensures that completion interrupts are handled on the
> same vCPU that submitted the request. No IPI is necessary to complete
> an I/O request and pe
On Fri, Jul 10, 2020 at 5:53 AM Stefan Hajnoczi wrote:
>
> On Thu, Jul 09, 2020 at 11:02:24AM -0700, Raphael Norwitz wrote:
> > On Mon, Jul 6, 2020 at 7:00 AM Stefan Hajnoczi wrote:
> > > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > > index
by: Stefan Hajnoczi
Reviewed-by: Raphael Norwitz
> ---
> include/hw/virtio/virtio-scsi.h | 2 ++
> hw/core/machine.c | 3 +++
> hw/scsi/vhost-scsi.c| 3 ++-
> hw/scsi/vhost-user-scsi.c | 3 ++-
> hw/scsi/virtio-scsi.c | 6 +-
>
Generally I’m happy with this. Some comments:
On Mon, Aug 24, 2020 at 4:40 AM Dima Stepanov wrote:
>
> vhost-user devices can get a disconnect in the middle of the VHOST-USER
> handshake on the migration start. If disconnect event happened right
> before sending next VHOST-USER command, then the
On Mon, Aug 24, 2020 at 4:41 AM Dima Stepanov wrote:
>
> If the vhost-user-blk daemon provides only one virtqueue, but device was
> added with several queues, then QEMU will send more VHOST-USER command
> than expected by daemon side. The vhost_virtqueue_start() routine
> handles such case by chec
On Thu, Aug 27, 2020 at 8:17 AM Michael S. Tsirkin wrote:
>
> On Tue, Aug 04, 2020 at 01:36:45PM +0300, Dima Stepanov wrote:
> > Reference e-mail threads:
> > - https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg01509.html
> > - https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg05
On Mon, Aug 31, 2020 at 4:37 AM Dima Stepanov wrote:
>
> On Thu, Aug 27, 2020 at 09:46:03PM -0400, Raphael Norwitz wrote:
> > On Mon, Aug 24, 2020 at 4:41 AM Dima Stepanov
> > wrote:
> > >
> > > If the vhost-user-blk daemon provides only one virtqueue, but
igration issue was slightly discussed earlier:
> - https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg01509.html
> - https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg05241.html
>
> Signed-off-by: Dima Stepanov
Reviewed-by: Raphael Norwitz
> ---
> hw/block/vhost-use
es such case by checking the return value from the
> virtio_queue_get_desc_addr() function call. Add the same check to the
> vhost_dev_set_log() routine.
>
> Signed-off-by: Dima Stepanov
Reviewed-by: Raphael Norwitz
> ---
> hw/virtio/vhost.c | 12
> 1 file change
s provide a new vhost_user_ops structure with the methods to
> initialize device, its command line or make a proper vhost-user
> responses.
>
> Signed-off-by: Dima Stepanov
Reviewed-by: Raphael Norwitz
> ---
> tests/qtest/vhost-user-test.c | 105
>
On Fri, Sep 4, 2020 at 5:34 AM Dima Stepanov wrote:
>
> Add support for the vhost-user-blk-pci device. This node can be used by
> the vhost-user-blk tests. Tests for the vhost-user-blk device are added
> in the following patches.
>
> Signed-off-by: Dima Stepanov
> ---
> tests/qtest/libqos/virtio
On Fri, Sep 4, 2020 at 5:35 AM Dima Stepanov wrote:
>
> Add vhost_user_ops structure for the vhost-user-blk device class. Add
> the test_reconnect and test_migrate tests for this device.
>
> Signed-off-by: Dima Stepanov
Reviewed-by: Raphael Norwitz
Just one small suggestion.
On Fri, Sep 4, 2020 at 5:36 AM Dima Stepanov wrote:
>
> Add new migrate_reconnect test for the vhost-user-blk device. Perform a
> disconnect after sending response for the VHOST_USER_SET_LOG_BASE
> command.
>
> Signed-off-by: Dima Stepanov
Reviewed-by: Raphael Norwitz
&g
This works for me, and looks good, but I figure those who added the
check should confirm that these tests are reliable now.
Marc-Andre - thoughts?
On Fri, Sep 4, 2020 at 5:36 AM Dima Stepanov wrote:
>
> For now a QTEST_VHOST_USER_FIXME environment variable is used to
> separate reconnect tests f
On Fri, Sep 11, 2020 at 4:43 AM Dima Stepanov wrote:
>
> Add support for the vhost-user-blk-pci device. This node can be used by
> the vhost-user-blk tests. Tests for the vhost-user-blk device are added
> in the following patches.
>
> Signed-off-by: Dima Stepanov
Reviewed-b
MST already sent a PR with all the patches :)
On Wed, Sep 16, 2020 at 6:13 PM Dima Stepanov wrote:
>
> On Mon, Sep 14, 2020 at 09:23:42PM -0400, Raphael Norwitz wrote:
> > On Fri, Sep 11, 2020 at 4:43 AM Dima Stepanov
> > wrote:
> > >
> > > Add support for
rdware spec.
Grammar here. Should be something like "...DMA request sizes which are
to large for the hardware spec".
>
> Signed-off-by: Andy Pei
Acked-by: Raphael Norwitz
> ---
> hw/block/vhost-user-blk.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/bl
> >
> > I see. I hadn't looked at the rest of the series yet because I ran out
> > of time, but now that I'm skimming them, I see quite a few places that
> > use non-EPROTO, but I wonder which of them actually should be
> > reconnected. So far all I saw were presumably persistent errors where a
>
ts is limited it seems
> harmless to try reconnecting on any error.
>
> So relax the condition of whether to retry connecting to check for any
> error.
>
> This patch amends a527e312b5 "vhost-user-blk: Implement reconnection
> during realize".
>
> Signed-off-by: Ro
Roman Kagan wrote:
> Fix the only callsite that doesn't propagate the error code from the
> generic vhost code.
>
> Signed-off-by: Roman Kagan
> ---
Reviewed-by: Raphael Norwitz
> hw/block/vhost-user-blk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
ce VIRTIO_F_RING_RESET is negotiated
> by the guest (Linux >= v6.0), but not supported by the device.
>
> Fixes: 69e1c14aa2 ("virtio: core: vq reset feature negotation support")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1318
> Signed-off-by: Stefano Garzarella
Looks good. For vhost-user-blk and vhost-user-scsi:
Acked-by: Raphael Norwitz
>
> [1] https://gitlab.com/virtio-fs/virtiofsd
> [2]
> https://github.com/rust-vmm/vhost/blob/240fc2966/crates/vhost-user-backend/src/event_loop.rs#L217
> Fixes: 02b61f38d3 ("hw/virtio: incorporate backend features in features")
> Reported-by: German Maglione
&
> On Nov 24, 2022, at 2:54 AM, Stefano Garzarella wrote:
>
> On Thu, Nov 24, 2022 at 01:50:19AM -0500, Michael S. Tsirkin wrote:
>> On Thu, Nov 24, 2022 at 12:19:25AM +, Raphael Norwitz wrote:
>>>
>>> > On Nov 23, 2022, at 8:16 AM, Stefano Garzarel
> On Nov 28, 2022, at 11:41 AM, Alex Bennée wrote:
>
> ..and use for both virtio-user-blk and virtio-user-gpio. This avoids
> the circular close by deferring shutdown due to disconnection until a
> later point. virtio-user-blk already had this mechanism in place so
The mechanism was originally c
> On Nov 29, 2022, at 12:30 AM, Michael S. Tsirkin wrote:
>
> On Tue, Nov 29, 2022 at 05:18:58AM +, Raphael Norwitz wrote:
>>> On Nov 28, 2022, at 11:41 AM, Alex Bennée wrote:
>>>
>>> ..and use for both virtio-user-blk and virtio-user-gpio. This avoids
onnection error occurs and we reconnect, the error
> message is printed using error_report_err(), but otherwise ignored.
>
> Signed-off-by: Kevin Wolf
Reviewed-by: Raphael Norwitz
> ---
> hw/block/vhost-user-blk.c | 14 +-
> 1 file changed, 13 insertions(+), 1 deletion(
> Set it to 1 if the backend doesn't have multiqueue support.
>
> Fixes: c90bd505a3e8210c23d69fecab9ee6f56ec4a161
> Signed-off-by: Kevin Wolf
Reviewed-by: Raphael Norwitz
> ---
> hw/virtio/vhost-user.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --g
I'm mostly happy with this. My biggest overall comment is that I think
this should be split into two, as your refactor using different event
handlers for init is a standalone improvement over and above the bugfix.
I would have the first commit split out vhost_user_blk_event_init() and
vhost_user_b
Couple commit message NITs but otherwise I'm happy with this.
Reviewed-by: Raphael Norwitz
On Wed, Mar 24, 2021 at 12:38:28PM +0300, Denis Plotnikov wrote:
> It is useful to use different connect/disconnect event handlers
> on device initialization and operation as seen from
Looks good, just clean up the commit message to reflect the way you've
now split the patches.
Reviewed-by: Raphael Norwitz
On Wed, Mar 24, 2021 at 12:38:29PM +0300, Denis Plotnikov wrote:
> Commit 4bcad76f4c39 ("vhost-user-blk: delay vhost_user_blk_disconnect")
> introduced
oo \
> -device vhost-user-blk-pci,queue-size=4096,chardev=foo
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1935014
> Signed-off-by: Kevin Wolf
> ---
> hw/block/vhost-user-blk.c | 5 +
> 1 file changed, 5 insertions(+)
Reviewed-by: Raphael Norwitz
>
>
Given what you've shown with the use-after-free, I agree the changes
need to be reverted. I'm a little uneasy about removing the reconnect
logic from the device realization completely though.
On Thu, Apr 22, 2021 at 07:02:17PM +0200, Kevin Wolf wrote:
> This is a partial revert of commits 77542d43
Code looks ok - question about the commit message.
Acked-by: Raphael Norwitz
On Thu, Apr 22, 2021 at 07:02:18PM +0200, Kevin Wolf wrote:
> Now that vhost_user_blk_connect() is not called from an event handler
> any more, but directly from vhost_user_blk_device_realize(), we don'
On Wed, Apr 28, 2021 at 07:31:13PM +0200, Kevin Wolf wrote:
> Am 28.04.2021 um 18:52 hat Raphael Norwitz geschrieben:
> > Given what you've shown with the use-after-free, I agree the changes
> > need to be reverted. I'm a little uneasy about removing the reconnect
Acked-by: Raphael Norwitz
On Thu, Apr 22, 2021 at 07:02:19PM +0200, Kevin Wolf wrote:
> VIRTIO_F_RING_PACKED and VIRTIO_F_IOMMU_PLATFORM need to be supported by
> the vhost device, otherwise advertising it to the guest doesn't result
> in a working configuration. They are currently
On Thu, Apr 22, 2021 at 07:02:20PM +0200, Kevin Wolf wrote:
> Commit 2943b53f6 (' virtio: force VIRTIO_F_IOMMU_PLATFORM') made sure
> that vhost can't just reject VIRTIO_F_IOMMU_PLATFORM when it was
> requested. However, just adding it back to the negotiated flags isn't
> right either because it pr
Reviewed-by: Raphael Norwitz
On Thu, Apr 22, 2021 at 07:02:21PM +0200, Kevin Wolf wrote:
> Creating a device with a number of queues that isn't supported by the
> backend is pointless, the device won't work properly and the error
> messages are rather confusing.
>
>
Got it - thanks for the clarification.
Reviewed-by: Raphael Norwitz
On Thu, Apr 29, 2021 at 11:34:11AM +0200, Kevin Wolf wrote:
> Am 28.04.2021 um 21:24 hat Raphael Norwitz geschrieben:
> > On Thu, Apr 22, 2021 at 07:02:20PM +0200, Kevin Wolf wrote:
> > > Commit 2943b53f
Makes sense - I see why it makes reporting better at realize time.
Thanks for the clarification.
On Thu, Apr 29, 2021 at 11:26:29AM +0200, Kevin Wolf wrote:
> Am 28.04.2021 um 20:08 hat Raphael Norwitz geschrieben:
> > Code looks ok - question about the commit message.
> >
> &
On Thu, Apr 29, 2021 at 02:48:52PM +0200, Kevin Wolf wrote:
> So maybe patch 2 should come first and also fix the preexisting bug, and
> of course this patch needs a v2 that doesn't introduce the new instances
> of the bug.
Sounds good to me.
>
> Kevin
>
So we're not going with the suggestion to retry once or a fixed number
of times? Any reason why not?
On Thu, Apr 29, 2021 at 07:13:12PM +0200, Kevin Wolf wrote:
> This is a partial revert of commits 77542d43149 and bc79c87bcde.
>
> Usually, an error during initialisation means that the configurat
Acked-by: Raphael Norwitz
On Thu, Apr 29, 2021 at 07:13:11PM +0200, Kevin Wolf wrote:
> We have to set errp before jumping to virtio_err, otherwise the caller
> (virtio_device_realize()) will take this as success and crash when it
> later tries to access things that we've alread
. The others just keep printing the error with
> error_report_err().
>
> Signed-off-by: Kevin Wolf
Reviewed-by: Raphael Norwitz
> ---
> include/hw/virtio/vhost.h| 2 +-
> backends/cryptodev-vhost.c | 5 -
> backends/vhost-user.c| 4 ++--
> hw/block
nnection goes away, without ending up
> in an endless loop if it's a permanent error in the configuration.
>
> Signed-off-by: Kevin Wolf
Reviewed-by: Raphael Norwitz
> ---
> include/hw/virtio/vhost-backend.h | 3 ++-
> hw/virtio/vhost-backend.c
re callbacks in VhostOps to return
> 0/-errno: .vhost_set_owner(), .vhost_get_features() and
> .vhost_virtqueue_set_busyloop_timeout(). The implementations of these
> functions are trivial as they generally just send a message to the
> backend.
>
> Signed-off-by: Kevin Wo
On Wed, Jun 09, 2021 at 05:46:55PM +0200, Kevin Wolf wrote:
> Instead of letting the caller make up a meaningless error message, add
> an Error parameter to allow reporting the real error.
>
> Signed-off-by: Kevin Wolf
Reviewed-by: Raphael Norwitz
> ---
> hw/block/vho
inguished in the caller.
>
> Signed-off-by: Kevin Wolf
Just one commmit message suggestion.
Reviewed-by: Raphael Norwitz
> ---
> include/hw/virtio/vhost-backend.h | 2 +-
> include/hw/virtio/vhost.h | 4 ++--
> hw/block/vhost-user-blk.c | 9 +
sconnecting the
> chardev, add this while touching the code.
>
> Signed-off-by: Kevin Wolf
Reviewed-by: Raphael Norwitz
> ---
> hw/block/vhost-user-blk.c | 48 ++-
> 1 file changed, 32 insertions(+), 16 deletions(-)
>
> diff --git a/hw/
High level looks good but I have some questions.
Rather than a new boolean I'd rather we re-used started_vu by changing
it to an enum and having different values for starting and started.
On Tue, Mar 29, 2022 at 12:15:46AM +0800, Jie Wang wrote:
> During Virtio1.0 dev(start_on_kick) in vhost_user
tcopy part of the message handler for these
> two commands.
>
> Signed-off-by: Kevin Wolf
Reviewed-by: Raphael Norwitz
> ---
> subprojects/libvhost-user/libvhost-user.c | 9 +++--
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/subprojects/libvhost
implementation that doesn't send a file descriptor. Keep
> accepting, but ignoring a file descriptor for compatibility with older
> QEMU versions.
>
> Signed-off-by: Kevin Wolf
Reviewed-by: Raphael Norwitz
> ---
> hw/virtio/vhost-user.c| 2 +-
>
it to describe the
> existing behaviour of libvhost-user (guest address, user address and
> size must match).
>
> Signed-off-by: Kevin Wolf
Reviewed-by: Raphael Norwitz
> ---
> docs/interop/vhost-user.rst | 17 +
> 1 file changed, 17 insertions(+)
&g
Hey Andrey - apologies for the late reply here.
It sounds like you are dealing with a buggy guest, rather than a QEMU issue.
> On Apr 10, 2023, at 11:39 AM, Andrey Ryabinin wrote:
>
>
>
> On 4/10/23 10:35, Andrey Ryabinin wrote:
>> Some guests hang on boot when using the vhost-user-blk-pci de
ss cases.
>
> Signed-off-by: Stefan Hajnoczi
Reviewed-by: Raphael Norwitz
> ---
> hw/block/vhost-user-blk.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 2dd3d93ca0..d9d9dc8a89
> On Nov 23, 2023, at 12:54 AM, Li Feng wrote:
>
> Keep it the same to vhost-user-blk.
> At the same time, fix the vhost_reset_device.
>
> Signed-off-by: Li Feng
Reviewed-by: Raphael Norwitz
> ---
> hw/scsi/vhost-user-scsi.c | 16
> hw/virtio/
> On Nov 23, 2023, at 12:54 AM, Li Feng wrote:
>
> If the error occurs in vhost_dev_init, the value of s->connected is set to
> true
> in advance, and there is no chance to enter this function execution again
> in the future.
>
> Signed-off-by: Li Feng
R
The code for these two patches looks fine. Just some questions on the
failure case you're trying to fix.
On Tue, May 14, 2024 at 2:12 AM Li Feng wrote:
>
> This reverts commit f02a4b8e6431598612466f76aac64ab492849abf.
>
> Since the current patch cannot completely fix the lost reconnect
> problem
Code looks good. Just a question on the error case you're trying to fix.
On Tue, May 14, 2024 at 2:12 AM Li Feng wrote:
>
> When the vhost-user is reconnecting to the backend, and if the vhost-user
> fails
> at the get_features in vhost_dev_init(), then the reconnect will fail
> and it will not
Very excited to see this. High level looks good modulo a few small things.
My major concern is around existing vhost-user-scsi backends which don’t
support VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD. IMO we should hide the reconnect
behavior behind a VHOST_USER_PROTOCOL_F_INFLIGHT_SHMFD check. We may
> On Jul 25, 2023, at 6:19 AM, Li Feng wrote:
>
> Thanks for your comments.
>
>> 2023年7月25日 上午1:21，Raphael Norwitz 写道：
>>
>> Very excited to see this. High level looks good modulo a few small things.
>>
>> My major concern is around existing vhost-
gned-off-by: Li Feng
Reviewed-by: Raphael Norwitz
> ---
> hw/virtio/vhost.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index abf0d03c8d..e2f6ffb446 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@
> On Jul 28, 2023, at 3:48 AM, Li Feng wrote:
>
> Thanks for your reply.
>
>> 2023年7月28日 上午5:21，Raphael Norwitz 写道：
>>
>>
>>
>>> On Jul 25, 2023, at 6:19 AM, Li Feng wrote:
>>>
>>> Thanks for your comments.
>>>
&g
1 - 100 of 121 matches
Mail list logo
