On 12/16/18 10:50 PM, Yuval Shaia wrote: > Hi Adit, > I have noticed that this field is checked only in modify_qp command. > > For all other command we have: > ret = pvrdma_cmd_post() > if (ret < 0) > error; > else > ok; > > While for modify_qp the flow is a bit different: > ret = pvrdma_cmd_post() > if (ret < 0) > error; > else > else if (rsp.hdr.err > 0) > error; > else > ok > > Any reason behind this? Sorry for the late reply. I got side-tracked by other stuff after the holiday break and this was just pushed to the back of the queue :(. Anyway, the reason we have this field is that we can report QP modify errors when connecting virtual QPs. There is an async process that happens in the hypervisor for this command and we expose a specific error to the guest. Other commands are pretty synchronous in completion so there isnt a need to set the error for them and reporting success/failure is sufficient. Hope that clarifies things. > > If not, can we get rid of this field? I understand that this is command > interface with the device and cannot just be deleted but can we at least be > consist with checking return value from command execution? and if this > field is never used then at least add some note in struct definition saying > something like "deprecated field"? > > Thanks, > Yuval >