Re: [libvirt] [PATCH] report invalid x86 cpu map error

On Mon, Jan 14, 2019 at 15:03:26 +0000, Daniel P. Berrangé wrote: > On Mon, Jan 14, 2019 at 03:36:38PM +0100, Jiri Denemark wrote: > > On Mon, Jan 14, 2019 at 14:21:42 +0000, Daniel P. Berrangé wrote: > > > On Mon, Jan 14, 2019 at 02:56:43PM +0100, Jiri Denemark wrote: > > > > On Mon, Jan 14, 2019 at 20:07:34 +0800, zhenwei pi wrote: > > > > > Let libvirtd handle invalid x86 cpu map error, and report the real reason. > > > > > > > > > > This issue can be reproduced : > > > > > 1, rm -rf /share/libvirt/cpu_map > > > > > 2, start libvirtd > > > > > 3, virsh create INSTANCE.xml > > > > > > > > > > Libvirtd reports error : > > > > > error: invalid argument: Failed to parse group 'tss' > > > > > > > > > > In face, libvirtd gets invalid cpu map. > > > > > With this patch, libvirtd reports error : > > > > > error: unsupported configuration: invalid x86 cpu map > > > > > > > > > > Signed-off-by: zhenwei pi <pizhenwei(a)bytedance.com&gt; > > > > > --- > > > > > src/cpu/cpu_x86.c | 5 ++++- > > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c > > > > > index d3a88da21d..91419d91d4 100644 > > > > > --- a/src/cpu/cpu_x86.c > > > > > +++ b/src/cpu/cpu_x86.c > > > > > @@ -2902,8 +2902,11 @@ virCPUx86ValidateFeatures(virCPUDefPtr cpu) > > > > > virCPUx86MapPtr map; > > > > > size_t i; > > > > > > > > > > - if (!(map = virCPUx86GetMap())) > > > > > + if (!(map = virCPUx86GetMap())) { > > > > > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > > > > > + _("invalid x86 cpu map")); > > > > > return -1; > > > > > + } > > > > > > > > This is definitely not the right place to fix the issue. virCPUx86GetMap > > > > is called from a lot of places and your patch would only fix one of > > > > them. Moreover the message itself is not very helpful as it doesn't say > > > > why the CPU map is invalid. The right solution would be to show the > > > > error reported from virCPUx86LoadMap. However, it is called inside > > > > virCPUx86DriverOnceInit, which is only called once per libvirtd life > > > > time. I think you'd need to store that error and reuse it everytime > > > > cpuMap is NULL inside virCPUx86GetMap. > > > > > > VIR_ONCE_GLOBAL_INIT already keeps a record of errors raised in the > > > initializer & re-raises them > > > > Eh, good to know :-) Anyway, it seems it doesn't really work and mostly > > some random unrelated error is reported. In my case I see > > > > virsh # cpu-models x86_64 > > error: failed to get CPU model names > > error: Storage volume not found: no storage vol with matching path > > '/vm/systemrescuecd-x86-2.5.1.iso' > > > > So there's definitely a bug to be fixed somewhere. > > Hmm, what steps did you use to get into that situation ? It works as > expected for me, re-reporting the original error. I just moved away the CPU map file. But it appears I have a domain defined pointing to a non-existent disk image. It seems the problem shows up when multiple errors are raised when libvirtd is starting up.

It looks like the author of this patch has an invalid group 'tss' configured somewhere, which triggered the "Failed to parse group 'tss'" error.