> 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?
>>> + 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.
>>> - 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
