> 2025年4月4日 15:30，Stefano Garzarella <sgarz...@redhat.com> 写道：
>
> 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
>
Sorry for the late reply. Both patch v1 and v2 are not comprehensive enough.
We should not only consider the block I/O interfaces vhost-user-blk/scsi.
Thanks,
Haoqian
