Re: [Xen-devel] [PATCH v18 12/13] x86/hvm: Remove redundant save functions

[Isaila Alexandru <aisaila@xxxxxxxxxxxxxxx>]

On Fri, 2018-09-07 at 03:43 -0600, Jan Beulich wrote:
> > > > On 03.09.18 at 15:14, <aisaila@xxxxxxxxxxxxxxx> wrote:
> > 
> > This patch removes the redundant save functions and renames the
> > save_one* to save. It then changes the domain param to vcpu in the
> > save funcs and adapts print messages in order to match the format
> > of the
> > other save related messages.
> > 
> > Signed-off-by: Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>
> > 
> > ---
> > Changes since V17:
> >     - Refit HVM_REGISTER_SAVE_RESTORE(CPU)
> >     - Add const to the added struct domain *d
> 
> Excuse me, but how many more times do I need to point out that this
> is wrong? Once again for a single-vCPU guest saving the second PIC
> instance will become impossible with this. Just to re-iterate: The
> check
> above has to be limited to just HVMSR_PER_VCPU kind records.

Sorry for the misunderstanding of this matter but in v16 I had

 +    if ( hvm_sr_handlers[typecode].kind == HVMSR_PER_VCPU &&
 +        instance >= d->max_vcpus )
 +        return -ENOENT;
 +    if ( hvm_sr_handlers[typecode].kind == HVMSR_PER_DOM )
 +        instance = 0;

and this solved the save call for per_dom having instance = 0.
Later, in v17 I dropped the PER_DOM if and in v18 I dropped the &&
PER_VCPU. 

Now the question is how exactly should I go with this?
> 
> >      ctxt.size = hvm_sr_handlers[typecode].size;
> > -    if ( hvm_sr_handlers[typecode].kind == HVMSR_PER_VCPU )
> > -        ctxt.size *= d->max_vcpus;
> >      ctxt.data = xmalloc_bytes(ctxt.size);
> >      if ( !ctxt.data )
> >          return -ENOMEM;
> >  
> > -    if ( (rv = hvm_sr_handlers[typecode].save(d, &ctxt)) != 0 )
> > +    if ( (rv = hvm_sr_handlers[typecode].save(d->vcpu[instance],
> > &ctxt)) != 0 )
> 
> And you need to use d->vcpu[0] for all others here.
> 
I can add a switch (kind) here and for the PER_VCPU case to call
save(d->vcpu[instance]) then for PER_DOM case call save(d->vcpu[0]). 

Is this a good way to go?

Thanks, 
Alex

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel