Re: [Xen-devel] [PATCH] xen: reset creation_finished flag on soft reset

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@xxxxxxxxxx]
> Sent: 01 September 2017 11:43
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; xen-
> devel@xxxxxxxxxxxxx; George Dunlap <George.Dunlap@xxxxxxxxxx>; Ian
> Jackson <Ian.Jackson@xxxxxxxxxx>; Jan Beulich <jbeulich@xxxxxxxx>;
> Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>; Wei Liu
> <wei.liu2@xxxxxxxxxx>
> Subject: Re: [PATCH] xen: reset creation_finished flag on soft reset
> 
> Paul Durrant <Paul.Durrant@xxxxxxxxxx> writes:
> 
> >> -----Original Message-----
> >> From: Vitaly Kuznetsov [mailto:vkuznets@xxxxxxxxxx]
> >> Sent: 01 September 2017 10:27
> >> To: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Paul Durrant
> >> <Paul.Durrant@xxxxxxxxxx>
> >> Cc: xen-devel@xxxxxxxxxxxxx; George Dunlap
> <George.Dunlap@xxxxxxxxxx>;
> >> Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Jan Beulich <jbeulich@xxxxxxxx>;
> >> Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>; Stefano Stabellini
> >> <sstabellini@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>; Wei Liu
> >> <wei.liu2@xxxxxxxxxx>
> >> Subject: Re: [PATCH] xen: reset creation_finished flag on soft reset
> >>
> >> Andrew Cooper <andrew.cooper3@xxxxxxxxxx> writes:
> >>
> >> > On 01/09/2017 10:11, Vitaly Kuznetsov wrote:
> >> >> C/s e7dabe5 ("x86/hvm: don't unconditionally create a default ioreq
> >> >> server") broke soft reset when QEMU traditional is being used. During
> >> >> soft reset QEMU is relaunched and default ioreq server needs to be
> >> >> re-created upon first HVM_PARAM_*IOREQ_* request. The flag will
> be
> >> >> set back to 'true' when toolstack unpauses the domain, just like after
> >> >> normal creation.
> >> >>
> >> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
> >> >
> >> > Sorry, but nack.  d->creation_finished is used for a number of things,
> >> > one being TLB safety before the vcpus have started executing.
> >> >
> >> > We either need to split the variable, or rework e7dabe5 to not use this.
> >> >
> >>
> >> I think that adding another flag is a bad idea, even 'creation_finished'
> >> flag looks a bit hackish to me. Adjusting e7dabe5 is probably
> >> better. However, while reading its blurb I don't fully understand the
> >> change: on migration we create new domain and thus reset
> >> creation_finished. During QEMU launch we still need to create ioreq
> >> server. Paul, could you please elaborate a bit (e.g. what are we
> >> guarding against, when creating ioreq server is redundant) so we can
> >> suggest a fix for soft reset?
> >
> > My memory is hazy as to the exact problem, but I think it was an issue
> > with the COLO project. IIRC they repeatedly 'migrate' a VM but then
> > resume the original. Without e7dabe5 the sending VM ends up with a
> > default ioreq server after the first migration because the save code
> > reads the HVM params that trigger its creation.
> >
> > I wonder whether the easiest thing to do would be to modify qemu trad
> > to do explicit ioreq server creation? It's really not that much
> > code-change... 20-30 lines or so.
> 
> I was thinking about this too, I'll try. It will hopefuly allow to get
> rid of the 'side effect' which creates default ioreq server on HVM
> parameters read.

Yes indeed. At that point I'd actually propose getting rid of those params 
altogether since nothing will use them anymore.

Cheers,

  Paul

> 
> Thanks!
> 
> --
>   Vitaly
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel