On Thu, Mar 27, 2025 at 02:53:24PM +0800, Haoqian He wrote:
2025年3月25日 17:51，Stefano Garzarella <sgarz...@redhat.com> 写道：
On Tue, Mar 25, 2025 at 04:39:46PM +0800, Haoqian He wrote:
2025年3月24日 22:31，Stefano Garzarella <sgarz...@redhat.com> 写道：
On Thu, Mar 20, 2025 at 08:21:30PM +0800, Haoqian He wrote:
2025年3月19日 23:20，Stefano Garzarella <sgarz...@redhat.com> 写道：
On Fri, Mar 14, 2025 at 06:15:34AM -0400, Haoqian He wrote:
[...]
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 6386910280..c99d56f519 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -187,6 +187,7 @@ struct VirtioDeviceClass {
void (*set_config)(VirtIODevice *vdev, const uint8_t *config);
void (*reset)(VirtIODevice *vdev);
void (*set_status)(VirtIODevice *vdev, uint8_t val);
+ int (*set_status_ext)(VirtIODevice *vdev, uint8_t val);
Why we need a new callback instead having `set_status` returning int ?
Because there are other devices such as virtio-net, virtio-ballon, etc.,
we only focus on vhost-user-blk/scsi when live migration.
Why only them?
What I mean, is why in devices where it's not important, don't we just return 0?
It seems more complicated to maintain and confusing for new devices to have 2
callbacks for the same thing.
Stefano
The series of these patches only want to fix that the inflight IO can't be
completed due to the disconnection between and the vhost-user backend for
vhost-user-blk / scsi devices during live migration. For other virito devices
the issue does not exist, and `vm_state_notify` cannot distinguish specific
devices, it's better not to return error.
Why for example for vhost-user-fs it doesn't exist?
I try to list the virtio sub-devices as follows:
hw/virtio/virtio-iommu.c: vdc->set_status = virtio_iommu_set_status;
hw/virtio/virtio-balloon.c: vdc->set_status = virtio_balloon_set_status;
hw/virtio/virtio-rng.c: vdc->set_status = virtio_rng_set_status;
hw/virtio/virtio-crypto.c: vdc->set_status = virtio_crypto_set_status;
hw/virtio/vhost-vsock.c: vdc->set_status = vhost_vsock_set_status;
hw/virtio/vhost-user-vsock.c: vdc->set_status = vuv_set_status;
hw/virtio/vhost-user-scmi.c: vdc->set_status = vu_scmi_set_status;
hw/virtio/vhost-user-fs.c: vdc->set_status = vuf_set_status;
hw/virtio/vhost-user-base.c: vdc->set_status = vub_set_status;
hw/virtio/vdpa-dev.c: vdc->set_status = vhost_vdpa_device_set_status;
tests/qtest/libqos/virtio-pci.c: .set_status = qvirtio_pci_set_status,
tests/qtest/libqos/virtio-pci-modern.c: .set_status = set_status,
tests/qtest/libqos/virtio-mmio.c: .set_status = qvirtio_mmio_set_status,
hw/scsi/vhost-user-scsi.c: vdc->set_status = vhost_user_scsi_set_status;
hw/scsi/vhost-scsi.c: vdc->set_status = vhost_scsi_set_status;
hw/net/virtio-net.c: vdc->set_status = virtio_net_set_status;
hw/char/virtio-serial-bus.c: vdc->set_status = set_status;
hw/block/vhost-user-blk.c: vdc->set_status = vhost_user_blk_set_status;
hw/block/virtio-blk.c: vdc->set_status = virtio_blk_set_status;
If the new function pointer type is not added, the number of functions affected
will be very huge. Although it may seem a bit complicated to use two callbacks,
it's much safer.
I can understand that it requires more change, but I don't understand why it's
safer, can you elaborate?
Anyway let's see what Michael says, if it's okay for him to have 2 callbacks
for the same thing but differing only by the return value, no objection for me.
Thanks,
Stefano
Hi Stefano, I removed set_status_ext in patch v3, and only changed the return
type of set_status to int. The new changes were applied to all vhost-user
devices, and virtio returned 0 for other devices.
Could you please review patch v3 is reasonable?
There are still questions like those a few lines above that I haven't
received answers to, please don't send new versions if we haven't
cleared up doubts about the current one first.
I still don't understand why we are only considering vhost-user-blk and
vhost-user-scsi, can you elaborate?
Thanks,
Stefano
