[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 3/9] libxl: New event generation API



Ian Campbell writes ("Re: [Xen-devel] [PATCH 3/9] libxl: New event generation 
API"):
> On Tue, 2012-01-24 at 16:23 +0000, Ian Jackson wrote:
> > Ian Campbell writes ("Re: [Xen-devel] [PATCH 3/9] libxl: New event 
> > generation API"):
> > > On Fri, 2012-01-13 at 19:25 +0000, Ian Jackson wrote:
> 
> > > > +    ret = libxl_evenable_domain_death(ctx, domid, 0, &deathw);
...
> > > > +        diskws = xmalloc(sizeof(*diskws) * d_config.num_disks);
> > > 
> > > I didn't see this getting freed on the exit path.
> > 
> > This is deliberate.  Why bother freeing things when the process is
> > about to exit.
> 
> Usually I agree.
> 
> However xl is intended as a libxl testbed as well as a toolstack in its
> own right and it is useful to be able to run tools such as valgrind on
> it to detect leaks in the library, but this requires not having too many
> "false positives" in xl.

Hmm, true.  OK, I will clean up these evgens.

> > > Incidentally we have libxl_device_disk.removable which might be an
> > > opportunity to optimise? Assuming it is meaningful in that way. I think
> > > in reality only emulated CD-ROM devices ever generate this event but
> > > perhaps exposing that in the API overcomplicates things.
> > 
> > Optimise to save on pointless xenstore watches you mean ?
> 
> That and event overhead generally of having the events registered and
> being tracked etc. In the common case we probably have 1 disk and 1
> cdrom so we've effectively doubled the amount of stuff we're dealing
> with -- whether this becomes significant e.g. on a system with 100+ VMs
> I'm not sure.

OK, it's an easy enough test to add.  I'll do so.

Ian.

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.