> 2023年7月31日 06:13，Raphael Norwitz <raphael.norw...@nutanix.com> 写道：
>
>>
>> On Jul 28, 2023, at 3:49 AM, Li Feng <fen...@smartx.com> wrote:
>>
>>
>>
>>> 2023年7月28日 下午2:04，Michael S. Tsirkin <m...@redhat.com> 写道：
>>>
>>> On Tue, Jul 25, 2023 at 06:42:45PM +0800, Li Feng wrote:
>>>> Get_inflight_fd is sent only once. When reconnecting to the backend,
>>>> qemu sent set_inflight_fd to the backend.
>>>
>>> I don't understand what you are trying to say here.
>>> Should be:
>>> Currently ABCD. This is wrong/unnecessary because EFG. This patch HIJ.
>>
>> Thanks, I will reorganize the commit message in v3.
>>>
>>>> Signed-off-by: Li Feng <fen...@smartx.com>
>>>> ---
>>>> hw/scsi/vhost-scsi-common.c | 37 ++++++++++++++++++-------------------
>>>> 1 file changed, 18 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/hw/scsi/vhost-scsi-common.c b/hw/scsi/vhost-scsi-common.c
>>>> index a06f01af26..664adb15b4 100644
>>>> --- a/hw/scsi/vhost-scsi-common.c
>>>> +++ b/hw/scsi/vhost-scsi-common.c
>>>> @@ -52,20 +52,28 @@ int vhost_scsi_common_start(VHostSCSICommon *vsc)
>>>>
>>>> vsc->dev.acked_features = vdev->guest_features;
>>>>
>>>> - assert(vsc->inflight == NULL);
>>>> - vsc->inflight = g_new0(struct vhost_inflight, 1);
>>>> - ret = vhost_dev_get_inflight(&vsc->dev,
>>>> - vs->conf.virtqueue_size,
>>>> - vsc->inflight);
>>>> + ret = vhost_dev_prepare_inflight(&vsc->dev, vdev);
>>>> if (ret < 0) {
>>>> - error_report("Error get inflight: %d", -ret);
>>>> + error_report("Error setting inflight format: %d", -ret);
>>>> goto err_guest_notifiers;
>>>> }
>>>>
>>>> - ret = vhost_dev_set_inflight(&vsc->dev, vsc->inflight);
>>>> - if (ret < 0) {
>>>> - error_report("Error set inflight: %d", -ret);
>>>> - goto err_guest_notifiers;
>>>> + if (vsc->inflight) {
>>>> + if (!vsc->inflight->addr) {
>>>> + ret = vhost_dev_get_inflight(&vsc->dev,
>>>> + vs->conf.virtqueue_size,
>>>> + vsc->inflight);
>>>> + if (ret < 0) {
>>>> + error_report("Error get inflight: %d", -ret);
>>>
>>> As long as you are fixing this - should be "getting inflight”.
>> I will fix it in v3.
>>>
>>>> + goto err_guest_notifiers;
>>>> + }
>>>> + }
>>>> +
>
> Looks like you reworked this a bit so to avoid a potential crash if
> vsc->inflight is NULL
>
> Should we fix it for vhost-user-blk too?
>
This check is mainly for the vhost-scsi code, that doesn’t need allocate the
inflight memory.
The vhost-user-blk doesn’t need this check, because there isn't a vhost-blk
device that reuse the code.
>>>> + ret = vhost_dev_set_inflight(&vsc->dev, vsc->inflight);
>>>> + if (ret < 0) {
>>>> + error_report("Error set inflight: %d", -ret);
>>>> + goto err_guest_notifiers;
>>>> + }
>>>> }
>>>>
>>>> ret = vhost_dev_start(&vsc->dev, vdev, true);
>>>> @@ -85,9 +93,6 @@ int vhost_scsi_common_start(VHostSCSICommon *vsc)
>>>> return ret;
>>>>
>>>> err_guest_notifiers:
>>>> - g_free(vsc->inflight);
>>>> - vsc->inflight = NULL;
>>>> -
>>>> k->set_guest_notifiers(qbus->parent, vsc->dev.nvqs, false);
>>>> err_host_notifiers:
>>>> vhost_dev_disable_notifiers(&vsc->dev, vdev);
>>>> @@ -111,12 +116,6 @@ void vhost_scsi_common_stop(VHostSCSICommon *vsc)
>>>> }
>>>> assert(ret >= 0);
>>>>
>
> As I said before, I think this introduces a leak.
I have answered in the previous mail.
>
>>>> - if (vsc->inflight) {
>>>> - vhost_dev_free_inflight(vsc->inflight);
>>>> - g_free(vsc->inflight);
>>>> - vsc->inflight = NULL;
>>>> - }
>>>> -
>>>> vhost_dev_disable_notifiers(&vsc->dev, vdev);
>>>> }
>>>>
>>>> --
>>>> 2.41.0
