On 10/20/2017 02:48 PM, Eduardo Habkost wrote: > On Sun, Oct 15, 2017 at 04:56:28AM +0300, Michael S. Tsirkin wrote: >> On Tue, Oct 10, 2017 at 03:01:10PM -0300, Eduardo Habkost wrote: >>> On Tue, Oct 10, 2017 at 04:06:28PM +0100, Daniel P. Berrange wrote: >>>> On Tue, Oct 10, 2017 at 05:00:18PM +0200, Marc-André Lureau wrote: >>>>> Hi >>>>> >>>>> On Tue, Oct 10, 2017 at 10:31 AM, Daniel P. Berrange >>>>> <berra...@redhat.com> wrote: >>>>>> On Tue, Oct 10, 2017 at 12:44:26AM +0300, Michael S. Tsirkin wrote: >>>>>>> On Mon, Oct 09, 2017 at 02:02:18PM +0100, Daniel P. Berrange wrote: >>>>>>>> On Mon, Oct 09, 2017 at 02:43:44PM +0200, Igor Mammedov wrote: >>>>>>>>> On Mon, 9 Oct 2017 12:03:36 +0100 >>>>>>>>> "Daniel P. Berrange" <berra...@redhat.com> wrote: >>>>>>>>> >>>>>>>>>> On Mon, Sep 11, 2017 at 06:59:24PM +0200, Marc-André Lureau wrote: >>>>>>>>>>> See docs/specs/vmcoreinfo.txt for details. >>>>>>>>>>> >>>>>>>>>>> "etc/vmcoreinfo" fw_cfg entry is added when using "-device >>>>>>>>>>> vmcoreinfo". >>>>>>>>>> >>>>>>>>>> I'm wondering if you considered just adding the entry to fw_cfg by >>>>>>>>>> default, without requiring any -device arg ? Unless I'm >>>>>>>>>> misunderstanding, >>>>>>>>>> this doesn't feel like a device to me - its just a well known bucket >>>>>>>>>> in fw_cfg IIUC ? Obviously its existance would need to be tied to >>>>>>>>>> the latest machine type for ABI reasons though. The benefit of this >>>>>>>>>> is that it would "just work" without us having to plumb it through to >>>>>>>>>> all the downstream applications that use QEMU for mgmt guest >>>>>>>>>> (OpenStack, >>>>>>>>>> oVirt, GNOME Boxes, virt-manager, and countless other mgmt apps). >>>>>>>>> it follows model set by pvpanic device, it's easier to manage from >>>>>>>>> migration >>>>>>>>> POV, one could use it even for old machine types with new qemu (just >>>>>>>>> by adding >>>>>>>>> device, it makes instance not backwards migratable to old qemu but >>>>>>>>> should work >>>>>>>>> for forward migration) and if user doesn't need it, device could be >>>>>>>>> just omitted >>>>>>>>> from CLI. >>>>>>>> >>>>>>>> Sure but it means that in effect no one will have this functionality >>>>>>>> enabled >>>>>>>> for several years. pvpanic has been around a long time and I rarely >>>>>>>> see it >>>>>>>> present in configured guests :-( >>>>>>>> >>>>>>>> >>>>>>>> Regards, >>>>>>>> Daniel >>>>>>> >>>>>>> libvirt runs with -nodefaults, right? I'd argue pretty strongly >>>>>>> -nodefaults >>>>>>> shouldn't add optional devices anyway. >>>>>> >>>>>> This isn't really adding a device though is it - it is just a well known >>>>>> location in fw_cfg to receive data. >>>>> >>>>> Enabling the device on some configurations by default can be done as a >>>>> follow-up patch. Can we get this series reviewed & merged? >>>> >>>> The problem with the -device approach + turning it on by default is that >>>> there >>>> is no way to turn it off again if you don't want it. eg there's way to undo >>>> an implicit '-device foo' except via -nodefaults, but since libvirt uses >>>> that >>>> already it would negate the effect of enabling it by default >>>> unconditionally. >>> >>> It's still possible to add a -machine option that can >>> enable/disable automatic creation of the device. >>> >>> But I also don't see why it needs to be implemented using -device >>> if it's not really a device. A boolean machine or fw_cfg >>> property is good enough for that. >> >> It certainly feels like a device. It has state >> (that needs to be migrated), it has a host/guest interface. > > (Sorry for the late reply) > > That's convincing enough to me. :) > > >>>> >>>> Your previous approach of "-global fw_cfg.vmcoreinfo=on" is nicer in this >>>> respect, as you can trivially turn it on/off, overriding the default state >>>> in both directions. >>> >>> Both "-global fw_cfg.vmcoreinfo=on|off" and >>> "-machine vmcoreinfo=on|off" sound good enough to me. >> >> >> Certainly not a fw cfg flag. Can be a machine flag I guess >> but then we'd have to open-code each such device. >> And don't forget auto - this is what Daniel asks for. > > I'm not sure Daniel is really asking for "auto": he is just > asking for a way to disable the new default. If "vmcoreinfo=off" > and "vmcoreinfo=off" works, there's no need for a user-visible > "auto" value. > > (Actually, "auto" values makes compatibility code even messier, > because we would need one additional compat property/field to > tell QEMU what "auto" means on each machine) >
Reviving this... did any follow up changes happen? Marc-André patched virt-manager a few months back to enable -device vmcoreinfo for new VMs: https://www.redhat.com/archives/virt-tools-list/2018-February/msg00020.html And I see there's at least a bug tracking adding this to openstack for new VMs: https://bugzilla.redhat.com/show_bug.cgi?id=1555276 If this feature doesn't really have any downsides, it would be nice to get this tied to new machine types. Saves a lot of churn for higher levels of the stack Thanks, Cole
