Re: [Qemu-devel] [PATCH] xen: make xen-platform a default device

[Paul Durrant]

> -----Original Message-----
> From: Stefano Stabellini [mailto:address@hidden
> Sent: 23 May 2014 12:37
> To: Paul Durrant
> Cc: Stefano Stabellini; Gerd Hoffmann; Paolo Bonzini; qemu-
> address@hidden; Anthony Liguori; address@hidden; Anthony Perard
> Subject: RE: [PATCH] xen: make xen-platform a default device
> 
> On Fri, 23 May 2014, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Stefano Stabellini [mailto:address@hidden
> > > Sent: 23 May 2014 11:11
> > > To: Paul Durrant
> > > Cc: Stefano Stabellini; Gerd Hoffmann; Paolo Bonzini; qemu-
> > > address@hidden; Anthony Liguori; address@hidden
> > > Subject: RE: [PATCH] xen: make xen-platform a default device
> > >
> > > On Fri, 23 May 2014, Paul Durrant wrote:
> > > > > -----Original Message-----
> > > > > From: Stefano Stabellini [mailto:address@hidden
> > > > > Sent: 22 May 2014 14:57
> > > > > To: Gerd Hoffmann
> > > > > Cc: Paolo Bonzini; Stefano Stabellini; Paul Durrant; qemu-
> > > address@hidden;
> > > > > Anthony Liguori; address@hidden
> > > > > Subject: Re: [PATCH] xen: make xen-platform a default device
> > > > >
> > > > > On Thu, 22 May 2014, Gerd Hoffmann wrote:
> > > > > > > > Given that libxl always passes -nodefaults to QEMU, this patch 
> > > > > > > > is
> > > going
> > > > > > > > to effectively disable xen_platform_pci for all Xen users. It 
> > > > > > > > is not
> a
> > > > > > > > good idea. With the patch applied a Xen user would have no way
> to
> > > > > enable
> > > > > > > > xen_platform_pci except for passing some magic command line
> > > runes
> > > > > via
> > > > > > > > device_model_args_hvm.
> > > > > > >
> > > > > > > In fact this code only runs for "-M xenfv".  If you use "-M pc", 
> > > > > > > the
> > > > > > > xen-platform device has to be added manually.  Perhaps it would
> be
> > > > > > > worthwhile to do the opposite, i.e. add the xen-platform device
> to "-
> > > M
> > > > > > > pc" if not using -nodefaults.
> > > > > >
> > > > > > /me looks at the code.  Yes, all the differences between xenfv and
> pc
> > > > > > machine types are guarded by if (xen_enabled()) these days,
> except
> > > for
> > > > > > adding the platform device.
> > > > > >
> > > > > > So using the pc machine type should just work on xen, and give you
> a
> > > > > > machine without the platform device.  So it can be added via -
> device, at
> > > > > > any slot, if needed.  No need to patch qemu at all.  Adding or not
> > > > > > adding xen-platform can easily handled by libxl then, depending on
> the
> > > > > > xen_platform_pci switch in the config file.
> > > > >
> > > > > I agree. Changing libxl to always use -M pc and using -device to add
> > > > > xen-platform when needed sounds like the best option.
> > > >
> > > > Really? You opposed this before:
> > > >
> > > > http://lists.xen.org/archives/html/xen-devel/2013-06/msg01946.html
> > > >
> > > > stating that you wanted a mechanism to query the running version of
> > > QEMU before choosing the machine type.
> > >
> > > Many QEMU releases have been made since then. I feel more
> comfortable
> > > with using -M pc, especially given the alternatives that have been
> > > proposed. Of course having a mechanism to query the running version of
> > > QEMU would still be the safest solution.
> >
> > But I thought the problem was that, should someone change what 'pc'
> means, then libxl would invoke the 'wrong' machine type on a domain
> restore. I know that xenfv is essentially 'pc' at the moment but would could
> fix on a particular version if compatibility becomes an issue. So, rather than
> choosing 'pc' now, should we not choose 'pc-i440fx-1.6' which is the current
> default?
> 
> One issue is that -M pc didn't always work with Xen. Now it does and we
> are already relying on it in libxl since
> 2bc047635b51abd41c917aa2b813211ee0de2c38. It is safe because all QEMU
> releases from 1.6 onward work well with Xen and -M pc. Older QEMU
> releases are considered ancient and unmaintained. This is what I was
> referring to in my last reply. I really meant "we should move away from
> xenfv and use a pc.* machine that does not create xen-platform by
> default".
> 
> As you say the other issue is the version of the pc machine, especially
> in relation to save/restore. However since:
> 
> commit 2bc047635b51abd41c917aa2b813211ee0de2c38
> Author: Anthony PERARD <address@hidden>
> Date:   Wed Nov 27 18:21:34 2013 +0000
> 
>     libxl: Handle xen_platform_pci=0 case with qemu-xen.
> 
> we are simply calling QEMU with -M pc if xen_platform_pci=0. I think we
> should change that too and backport the patch to 4.4. pc-i440fx-1.6
> seems like a good choice to me.

Yep - that sounds good.

  Paul