Re: [PATCH 3/3] virt-host-validate: Detect SMMU presence on ARMs by parsing IORT table

On 4/5/23 19:31, Andrea Bolognani wrote: Since you're bringing this up, I find the QEMU style horrible. It boils down to why we include 'Fixes' in the first place. IMO, the part of commit message above is for humans, and that's why I tend to put output of: 'git describe $hash' or 'git describe --contains $hash' there because that gives me (as a human) a hint which version the regression occurred in (v9.2.0 in this case). But for 'Fixes:' field - my understanding is that it's for maintainers so that they know what other commits to backport. And thus it should be as unique as possible. But these abbreviated hashes are horrible in that sense. TLDR: the length of abbreviated hashes changes over time Longer version: So, when you do 'git log --oneline' and see those abbreviated hashes, for both libvirt.git and qemu.git they're both shortened to 10 characters. And it makes sense because each repo has relatively small number of objects in the pack (~42K for libvirt and ~703K for qemu; obtained via 'git count-objects -v' and looking at 'in-pack' value) and 10 chars are sufficient to guarantee uniqueness. But if you take a look at a bigger repo, say kernel.git (~9M objects), 10 characters are no longer sufficient and git needs to use 12 characters. IOW as the repository grows, abbreviated hashes are less and less unique. BTw: the code that picks the length for abbreviated hashes can be found here: https://github.com/git/git/blob/master/object-name.c#L762 We can expect length to bump to 11 soon-ish for qemu. Not to mention, that these calculations set the starting position, lower limit, after which git still has to walk through all hashes to make sure there's no conflict using that lower limit. Therefore, even adding one new commit can cause conflict on say 10 characters. And if I look at this from a downstream maintainer POV and my (probably unique) workflow, then whenever I backport a patch, I also bring up git-log (in less) and do a quick search of the hash '/$hash'. This finds all 'Fixes: $hash' lines and thus brings my attention to fixes of the commit I'm about to backport. Using these abbreviated hashes doesn't work with my workflow (obviously). Now, sure - I can search for just an abbreviated hash - but how much abbreviated? Also, what good is the commit subject? I certainly do not remember commits by their subjects. And thus I need to look at the commit anyway. For instance, from your example I'd still need to: git show $abbrevHash to understand what the commit did (okay, not in this example, because I'm author of both commits and wrote them in span of a a couple of days; but if I were a downstream maintainer you can bet I'd need to do that). Therefore, using: Fixes: $abbrevHash ($commitSubject) is a horrible idea that looks good on paper but in fact hurts everybody involved. Yeah, there's still a case with device tree (i.e. no ACPI tables) so I was dubious whether to put it there. But you're right, it can't hurt. Thanks, Michal