Messages in this thread | | | Date | Mon, 23 Oct 2023 17:50:46 +0800 | Subject | Re: [PATCH] virtio_ring: add an error code check in virtqueue_resize | From | Su Hui <> |
| |
On 2023/10/23 13:46, Xuan Zhuo wrote: >>>>>>>> Well, what are the cases where it can happen practically? >>>>>>> Device error. Such as vp_active_vq() >>>>>>> >>>>>>> Thanks. >>>>>> Hmm interesting. OK. But do callers know to recover? >>>>> No. >>>>> >>>>> So I think WARN + broken is suitable. >>>>> >>>>> Thanks. >>>> Sorry for the late, is the following code okay? >>>> >>>> @@ -2739,7 +2739,7 @@ int virtqueue_resize(struct virtqueue *_vq, u32 num, >>>> void (*recycle)(struct virtqueue *vq, void *buf)) >>>> { >>>> struct vring_virtqueue *vq = to_vvq(_vq); >>>> - int err; >>>> + int err, err_reset; >>>> >>>> if (num > vq->vq.num_max) >>>> return -E2BIG; >>>> @@ -2759,7 +2759,15 @@ int virtqueue_resize(struct virtqueue *_vq, u32 num, >>>> else >>>> err = virtqueue_resize_split(_vq, num); >>>> >>>> - return virtqueue_enable_after_reset(_vq); >>>> + err_reset = virtqueue_enable_after_reset(_vq); >>>> + >>>> + if (err) { >>> No err. >>> >>> err is not important. >>> You can remove that. >> Emm, I'm a little confused that which code should I remove ? >> >> >> like this: >> if (vq->packed_ring) >> virtqueue_resize_packed(_vq, num); >> else >> virtqueue_resize_split(_vq, num); >> >> And we should set broken and warn inside virtqueue_enable_after_reset()?
In my opinion, we should return the error code of virtqueue_resize_packed() / virtqueue_resize_split(). But if this err is not important, this patch makes no sense. Maybe I misunderstand somewhere... If you think it's worth sending a patch, you can send it :).(I'm not familiar with this code).
Thanks, Su Hui
| |