WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] tools: add closure to xc_domain_save switch_qemu_logdirty callback
From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Date: Wed, 13 Oct 2010 14:10:56 +0100
Cc: Brendan, Cully <brendan@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
Delivery-date: Wed, 13 Oct 2010 06:11:55 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1286974006.2003.3286.camel@xxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <92be1317280b14e63b38.1286899510@xxxxxxxxxxxxxxxxxxxxx> <alpine.DEB.2.00.1010131212160.2423@kaball-desktop> <1286974006.2003.3286.camel@xxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Alpine 2.00 (DEB 1167 2008-08-23)
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