Re: [Qemu-devel] [PATCH 0/2] Remove hardcoded xen-platform device initialization (v4)

[Paul Durrant]

> -----Original Message-----
> From: Laszlo Ersek [mailto:address@hidden
> Sent: 18 June 2013 14:14
> To: Michael S. Tsirkin
> Cc: Paul Durrant; address@hidden
> Subject: Re: [Qemu-devel] [PATCH 0/2] Remove hardcoded xen-platform
> device initialization (v4)
> 
> On 06/18/13 15:01, Michael S. Tsirkin wrote:
> > On Tue, Jun 18, 2013 at 12:57:54PM +0000, Paul Durrant wrote:
> >>> -----Original Message-----
> >>> From: Michael S. Tsirkin [mailto:address@hidden
> >>> Sent: 18 June 2013 13:52
> >>> To: Laszlo Ersek
> >>> Cc: Paul Durrant; address@hidden
> >>> Subject: Re: [Qemu-devel] [PATCH 0/2] Remove hardcoded xen-
> platform
> >>> device initialization (v4)
> >>>
> >>> On Tue, Jun 18, 2013 at 02:37:58PM +0200, Laszlo Ersek wrote:
> >>>> Hi Paul,
> >>>>
> >>>> (xen-devel snipped)
> >>>>
> >>>> On 06/18/13 13:16, Paul Durrant wrote:
> >>>>> Because of concerns over backwards compatibility and a suggestion
> that
> >>>>> xenfv should be retired in favour of using the pc machine type I have
> re-
> >>>>> worked my original patch into 2 patches:
> >>>>>
> >>>>> [PATCH 1/2] Allow use of pc machine type (accel=xen) for Xen HVM
> >>>>> [PATCH 2/2] Move hardcoded initialization of xen-platform device.
> >>>>>
> >>>>> Application of both these patches allows alternative pc machine types
> to
> >>> be
> >>>>> used with the accel=xen option, but preserves the hardcoded
> creation of
> >>>>> the xen-platform device only for machine type xenfv.
> >>>>>
> >>>>> v3:
> >>>>> - Add test for xen_enabled() that went missing in v2
> >>>>>
> >>>>> v4:
> >>>>> - Remove erroneous whitespace hunk
> >>>>> - Replace hw_error() with fprintf()+exit(1)
> >>>>> - Add braces to single-line if
> >>>>
> >>>> can you please offer an opinion in the
> >>>>
> >>>>   [PATCH 1/2] pvpanic: initialization cleanup
> >>>>   http://thread.gmane.org/gmane.comp.emulators.qemu/216940
> >>>>
> >>>> thread?
> >>>>
> >>>> >From where I stand (which is "quite afar" :)) this series of yours seems
> >>>> somewhat related to my doubt there.
> >>>>
> >>>> Thanks!
> >>>> Laszlo
> >>>
> >>> OK will make it skip fwcfg as we did earlier.
> >>> Thanks for the review.
> >>>
> >>
> >> Yes, I think the assert(fw_cfg) would be problematic in the xen case
> where, up until my patch, machine types was necessarily xenfv.
> >>
> >>   Paul
> >
> > Do you guys actually need the pvpanic device?
> > How do you know which port to use without fwcfg?
> 
> Xen domains don't know the port and don't use the pvpanic device, but
> qemu starts at least. In other words, the pvpanic device is created, but
> unreachable. Maybe the has_pvpanic logic should depend on (or extended
> with) !xen_enabled().
> 

That seems entirely reasonable to me.

  Paul