Re: [Xen-devel] [PATCH] libxl: refactor toolstack save restore code

[George Dunlap <dunlapg@xxxxxxxxx>]

On Fri, Jun 5, 2015 at 7:15 PM, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
> On Fri, Jun 05, 2015 at 06:46:35PM +0100, Andrew Cooper wrote:
>> On 05/06/15 18:01, Wei Liu wrote:
>> > This patch does following things:
>> > 1. Document v1 format.
>> > 2. Factor out function to handle QEMU restore data and function to
>> >    handle v1 blob for restore path.
>> > 3. Refactor save function to generate different blobs in the order
>> >    specified in format specification.
>> > 4. Change functions to use "goto out" idiom.
>> >
>> > No functional changes introduced.
>> >
>> > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
>> > ---
>> > I wrote this patch when exploring the idea of have toolstack save / 
>> > restore v2
>> > to record max pages information. Though that idea has been abandon it 
>> > wouldn't
>> > hurt to have clearer code structure and documentation.
>> > ---
>> >  tools/libxl/libxl_dom.c | 247 
>> > +++++++++++++++++++++++++++++-------------------
>> >  1 file changed, 150 insertions(+), 97 deletions(-)
>> >
>> > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
>> > index 867172a..23c4691 100644
>> > --- a/tools/libxl/libxl_dom.c
>> > +++ b/tools/libxl/libxl_dom.c
>> > @@ -1032,6 +1032,15 @@ struct libxl__physmap_info {
>> >      char name[];
>> >  };
>> >
>> > +/* Bump version every time when toolstack saved data changes.
>> > + * Different types of data are arranged in the specified order.
>> > + *
>> > + * Version 1:
>> > + *   uint32_t version
>> > + *   QEMU physmap data:
>> > + *     uint32_t count
>> > + *     libxl__physmap_info * count
>> > + */
>>
>> Ah fantastic - this was some information which IanJ asked me to detail
>> in the libxl v2 stream spec, and I had not gotten around to working out
>> yet.  (Current handling was just to pass it verbatim back to libxl.)
>>
>> It is probably the kind of thing which needs splitting into appropriate
>> v2 records, rather than having a structured binary blob inside a
>> structered stream, both defined for the same library.
>>
>> OOI, why does libxl need to send this information for Qemu? We don't
>> have any equivalent in XenServer.
>>
>
> It is for upstream QEMU to save / restore physical map information.  I
> think XenServer has not yet switched to upstream QEMU so you don't have
> this.

I'm not super familiar with the qemu stuff, but it certainly seems to
be the case that qemu-trad does *not* expect to know the physical
layout of memory, whereas qemu-upstream *does*.  This is the source of
the MMIO hole resizing issues as well -- on qemu-trad, moving the
memory around didn't cause any problems; on qemu-upstream, if you add
memory to a guest's physmap where it wasn't before, if qemu doesn't
know about it, it will crash when trying to access it.  I assume
that's what this section is for.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel