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
|