On Tue, Oct 10, 2017 at 4:02 AM, Kevin Wolf <[email protected]> wrote: > Am 10.10.2017 um 08:58 hat Markus Armbruster geschrieben: >> Doug Gale <[email protected]> writes: >> >> > I used exclamations as a concise way of indicating that the driver did >> > something nonsensical, or horribly invalid, like something likely to >> > cause a memory corruption, trying to start the controller with a >> > nonsense configuration, providing invalid PRDs or writing to >> > unrecognized MMIO offsets that might hang or do something extremely >> > bad on a poorly implemented controller. Exclaiming is not shouting, I >> > thought shouting was when it was all uppercase. >> > >> > If a driver might trash memory or hang a controller, maybe it should >> > shout at the driver author or person investigating an unstable VM. >> > None of those messages with exclamations should ever happen. If any of >> > them are possible when the driver is correct, then I have made a >> > mistake. >> >> Please do not top-quote on this mailing list. >> >> Existing tracepoints do not use exclamation marks to signal severity. >> >> Consider using assertions for "if this happens, either the program's >> state is shot (and continuing is unsafe), or the author's mental state >> was shot (and continuing is probably unsafe, too)" conditions. >> Tracepoints are for tracing. > > Assertions are for checking that assumptions in qemu code hold true. > Here it's about bad guest code, and you can't let qemu abort for that. > > Tracing is the right tool to detect bad guest code, and I think it makes > sense to mark conditions that shouldn't happen with a correctly > operating guest driver. I'm not sure if an exclamation mark is the best > syntax for this, because I wouldn't have intuitively understood what > it's supposed to tell me. > > If we do use the exclamation mark, we should document it somewhere > (docs/devel/tracing.txt?) and make it a qemu-wide pattern. If not, we > should choose something else and still document it and use it > consistently. > > Kevin >
Should I give up on indicating severity for now (removing the exclamations) and just group together the severe ones with a comment heading in the trace-events file? I added "" at the end of some traces to help possibly-incorrect parsing code find the end of the string reliably. Was that a good idea or is it okay and expected to end them with something like PRIx64 and drop the extra ""? I found a few cases of missing 0x that I will fix in the next version of this patch. Policy is to have 0x before every hex value, right? Also, so I'm clear, when I submit the patch I should put the patch first and put my message after the -- at the end? And use "nvme: Add tracing" for the commit message, And put nvme: Add tracing v3 in my subject line in a completely new email thread?
