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

[Stefano Stabellini]

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.