Re: [Qemu-devel] [PATCH] RFC: vmcoreinfo device

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Apr 25, 2017 at 05:29:20PM -0300, Eduardo Habkost wrote:
> On Mon, Apr 24, 2017 at 05:03:55PM +0400, Marc-André Lureau wrote:
> [...]
> > diff --git a/include/hw/compat.h b/include/hw/compat.h
> > index 5d5be91daf..d0c9b71902 100644
> > --- a/include/hw/compat.h
> > +++ b/include/hw/compat.h
> > @@ -135,6 +135,10 @@
> >          .driver   = "vmgenid",\
> >          .property = "x-write-pointer-available",\
> >          .value    = "off",\
> > +    },{\
> > +        .driver   = "vmcoreinfo",\
> > +        .property = "x-write-pointer-available",\
> > +        .value    = "off",\
> >      },
> 
> My first reaction to this was "we don't need this compat property, because the
> device didn't even exist in QEMU 2.4".
> 
> But then I read commit f2a1ae45d8ec5ad494e66a9234499a2e0fbf4b40 and now I see
> why this is required: this is a compat property whose sole function is to
> prevent the device from being instantiated.
> 
> Instead of requiring an extra compat property, I suggest just checking if
> fw_cfg has DMA enabled. e.g.:
> 
>  static void vmgenid_realize(DeviceState *dev, Error **errp)
>  {
>      VmGenIdState *vms = VMGENID(dev);
> +    FWCfgState *fw_cfg = FW_CFG(object_resolve_path_type("", TYPE_FW_CFG, NULL));
> 
> -    if (!vms->write_pointer_available) {
> +    if (!fw_cfg || !fw_cfg_dma_enabled(fw_cfg)) {
>          error_setg(errp, "%s requires DMA write support in fw_cfg, "
>                     "which this machine type does not provide", VMGENID_DEVICE);
>          return;
> 
> 
> This has the additional benefit of handling other cases properly, like:
> 
>   $ qemu-system-x86_64 -device vmgenid -machine none
>   qemu-system-x86_64: -device vmgenid: vmgenid requires DMA write support in fw_cfg, which this machine type does not provide
>   $ qemu-system-x86_64 -device vmgenid -machine pc-i440fx-2.9 -global fw_cfg.dma_enabled=off
>   qemu-system-x86_64: -device vmgenid: vmgenid requires DMA write support in fw_cfg, which this machine type does not provide
>   $ qemu-system-x86_64 -device vmgenid -machine pc-i440fx-2.6 -global fw_cfg.dma_enabled=on
>   [boots normally]
>   $

It's quite ugly to make it poke at fw cfg internals though,
it shouldn't know how is write pointer implemented.
We need some kind of API that it can share with vm gen id.

> -- 
> Eduardo