Re: [libvirt] [PATCH 1/2] conf: add crash to hyperv features

On Wed, Nov 11, 2015 at 08:34:02 +0100, Peter Krempa wrote: > On Thu, Nov 05, 2015 at 16:54:02 +0300, Dmitry Andreev wrote: >> >> On 05.11.2015 14:06, Jiri Denemark wrote: >>> On Mon, Oct 26, 2015 at 13:23:32 +0300, Dmitry Andreev wrote: >>>> Add crash CPU feature for Hyper-V. Hyper-V crash MSR's can be used >>>> by Hyper-V based guests to notify about occurred guest crash. >>>> >>>> XML: >>>> <features> >>>> <hyperv> >>>> <crash state='on'/> >>>> </hyperv> >>>> </features> >>> Sounds like this is related to an existing panic device we already >>> support. So what does enabling hv_crash do in QEMU? Is it an additional >>> channel to a panic device or is the panic device still needed even if >>> hv_crash is enabled? In any case, I think we should map this somehow to >>> the panic device instead of copying 1:1 the way QEMU enables hv_crash. >> pvpanic and Hyper-V crash are independent ways for guest to notify about >> OS crash. Both ways rise the 'qemu guest panicked' event. Domain can >> have both hv_crash and pvpanic enabled at the same time. >> >> pvpanic is in <devices> section in domain configuration because it is an >> ISA device. Hyper-V crash is a hypervisor's feature, which enables a set >> of model-specific registers. Guest can use this registers to send >> notification and store additional information about a crash. This is a >> part of Microsoft hypervisor interface. > Device or not, I don't really like having two distinct places to > configure similar functionality. > > <device> > <panic model='hyperv'/> > > will do just fine IMO. Yeah, this what I was thinking about. After all, we already do something similar on Power. The guest panic notification is an integral part of the platform itself so there's no device we need to add. To reflect this in our domain XMLs, we just always add <device> <panic/> </device> Thus using <panic> element for hv_crash seems like the best approach to me. We'd have just one place for configuring all kinds of guest crash notifications. The question is what the XML should look like. The form suggested by Peter looks good, but then we should probably add the model attribute to all panic devices to make it consistent. So a theoretical XML using all currently supported panic "devices" would be: <device> <panic model='hyperv'/> <!-- hv_crash --> <panic model='isa'/> <!-- pvpanic --> <panic model='pseries'/> <!-- Power --> </device> 'pseries' model would only be allowed on Power, while the others would only be allowed on x86. We'd need to automatically add model='...' to existing device, but it should be pretty easy. Any older libvirt would just ignore the attribute completely and a new libvirt would add model='isa' on x86 and model='pseries' on Power. Jirka -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list