Re: [Xen-devel] [PATCH] tools: add closure to xc_domain_save switch_qemu_logdirty callback

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 13 Oct 2010, Ian Campbell wrote:
> On Wed, 2010-10-13 at 12:14 +0100, Stefano Stabellini wrote:
> > On Tue, 12 Oct 2010, Ian Campbell wrote:
> > > @@ -1876,7 +1876,7 @@ int xc_domain_save(xc_interface *xch, in
> > >                                 NULL, 0, NULL, 0, NULL) < 0 )
> > >              DPRINTF("Warning - couldn't disable shadow mode");
> > >          if ( hvm )
> > > -            switch_qemu_logdirty(dom, 0);
> > > +            callbacks->switch_qemu_logdirty(dom, 0, callbacks->data);
> > >      }
> > >  
> > >      if ( live_shinfo )
> > 
> > I think that at the beginning of xc_domain_save we should check if
> > callbacks->switch_qemu_logdirty is NULL and print an error an return in
> > that case.
> 
> We didn't do so for the original function pointer parameter.
> 
> Not passing in this callback is a pretty fundamental error in the
> caller, there's not really anything they can do with the error code.
> This change will break compilation for any caller which has not been
> updated so I don't think there is too much danger of toolstacks missing
> the need for the change.
> 
> Propagating an EINVAL doesn't really help unless all callers reliably
> test the return code and do something sensible with it.
> 
> On the other hand given that at least one caller has a valid reason not
> to use the callback (unless this changes means it now could, as I
> wondered in the original changelog but forgot to CC Brendan about) then
> I think this would be more reasonable than EINVAL
> 
> Subject: libxc allow omission of hvm switch_qemu_logdirty on save
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>


yeah, this patch is OK too


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel