>>> On 23.08.17 at 11:27, <jgr...@suse.com> wrote: > On 22/08/17 12:04, Jan Beulich wrote: >>>>> On 16.08.17 at 14:52, <jgr...@suse.com> wrote: >>> @@ -89,44 +89,50 @@ static void __init parse_iommu_param(char *s) >>> s += 3; >>> >>> ss = strchr(s, ','); >>> - if ( ss ) >>> - *ss = '\0'; >>> - >>> - if ( !parse_bool(s) ) >>> - iommu_enable = 0; >>> - else if ( !strcmp(s, "force") || !strcmp(s, "required") ) >>> + if ( !ss ) >>> + ss = strchr(s, '\0'); >>> + >>> + b = parse_bool(s); >> >> I don't think this will work as intended for "iommu=yes,...". Did I >> perhaps overlook the same issue in some of the earlier patches? > > I'm just of the opposite opinion: the former implementation didn't work > as intended in that case: > > Let a hypervisor be built with a pre-defined command line "iommu=no". > A user supplied command line "iommu=yes" wouldn't change iommu_enable. > > My patch changes that: setting iommu_enable to 0 or 1 in case > parse_bool() succeeded is the correct thing to do. > > Maybe I should mention this in the commit message, though.
That's an orthogonal aspect, which I agree with. My comment was about parse_bool() expecting a nul-terminated "yes" (or alike), though. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel