Re: [Xen-devel] [XEN][RFC PATCH V2 11/17] xc: modify save/restore to support multiple device models

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 08/23/2012 08:52 PM, Ian Campbell wrote:
> On Thu, 2012-08-23 at 20:13 +0100, Julien Grall wrote:
>    
> > On 08/23/2012 02:27 PM, Ian Campbell wrote:
> >      
> > >        
> > > > @@ -103,6 +103,9 @@ static ssize_t rdexact(xc_interface *xch, struct 
> > > > restore_ctx *ctx,
> > > >    #else
> > > >    #define RDEXACT read_exact
> > > >    #endif
> > > > +
> > > > +#define QEMUSIG_SIZE 21
> > > > +
> > > >    /*
> > > >    ** In the state file (or during transfer), all page-table pages are
> > > >    ** converted into a 'canonical' form where references to actual mfns
> > > > @@ -467,7 +522,7 @@ static int buffer_tail_hvm(xc_interface *xch, struct 
> > > > restore_ctx *ctx,
> > > >                               int vcpuextstate, uint32_t vcpuextstate_size)
> > > >    {
> > > >        uint8_t *tmp;
> > > > -    unsigned char qemusig[21];
> > > > +    unsigned char qemusig[QEMUSIG_SIZE + 1];
> > > >
> > > >          
> > > An extra + 1 here?
> > >
> > >        
> > QEMUSIG_SIZE doesn't take into account the '\0'. So we need to add 1.
> > If an error occurred, without +1, the output log lost the last character.
> >      
> So this is just a bug fix for a pre-existing issue?
>    
Yes.

> > > [...]
> > >
> > >        
> > > > -    qemusig[20] = '\0';
> > > > +    qemusig[QEMUSIG_SIZE] = '\0';
> > > >
> > > >          
> > > This is one bigger than it used to be now.
> > >
> > > Perhaps this is an unrelated bug fix (I haven't check the real length of
> > > the sig), in which case please can you split it out and submit
> > > separately?
> > >
> > >        
> > #define QEMU_SIGNATURE "DeviceModelRecord0002"
> > Just checked, the length seems to be 21. I will send a patch with
> > this change.
> >      
> Perhaps use either sizeof(QEMU_SIGNATURE) or strlen(QEMU_SIGNATURE)
> (depending on which semantics you want)?
>    
Here, QEMU_SIZE needs to be define as strlen (QEMU_SIGNATURE),
but QEMU_SIGNATURE is not defined in libxc. It's defined
in libxl/libxl_internal.h.
By the way, I'm wondering why QEMU save (libxl__domain_save_device_model)
is made in libxl and restore (dump_qemu) in libxc ?


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