> 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?
Thanks,
Haoqian
