On Wed, Sep 6, 2023 at 6:01 PM Albert Esteve <aest...@redhat.com> wrote:
> > > On Wed, Sep 6, 2023 at 4:43 PM Philippe Mathieu-Daudé <phi...@linaro.org> > wrote: > >> On 6/9/23 16:33, Albert Esteve wrote: >> >> > I note you ignored my comment regarding adding a 'Error **' >> argument in >> > the previous version: >> > >> https://lore.kernel.org/qemu-devel/911eef0c-d04f-2fcf-e78b-2475cd7af...@linaro.org/ >> < >> https://lore.kernel.org/qemu-devel/911eef0c-d04f-2fcf-e78b-2475cd7af...@linaro.org/ >> > >> > >> > Sorry I missed those comments somehow :/ >> >> Ah, I see. >> >> > I'll check them and resend. >> >> You can also object to them, explaining why this isn't really >> useful, if you think so. But first read the big comment in >> include/qapi/error.h. >> >> > Sure, I understand. So far I tend to trust the judgement of the more > experienced > Qemu developers over my own, but if I wouldn't agree with what is > suggested I would object :) > So: > - Regarding the two functions with the same, seems to be solved with the > squash before, > and it was probably causing the compile error to begin with, so one less > thing to worry about! > - Regarding splitting the commit, sure, no problem. I'll ensure they do > compile separately. > - Regarding the error, I read the long comment in the error file and > checked surrounding code. I think > you are right and will be better propagating the error. > But I think I will omit the Error propagation for `vhost_user_backend_handle_shared_object_lookup`. In this function returning an error code does not necessarily mean that we should log an error. So if we pass the local_err from the backend_read function to the handler, we cannot be sure when we should actually print the log. `vhost_backend_handle_iotlb_msg` has the same issue and does not propagate the error either, relies solely on `error_report` calls. Therefore, I will propagate it for `vhost_user_send_resp` and `vhost_user_backend_send_dmabuf_fd` only. > > And I think I would address all your comments with that! Thanks for the > feedback! > > >> Thanks, >> >> Phil. >> >>
