Re: [Libvir] [PATCH] Block device and network stats (version 2)

[Daniel Veillard <veillard redhat com>]

On Tue, Aug 21, 2007 at 01:26:14PM +0100, John Levon wrote:
> On Tue, Aug 21, 2007 at 08:10:33AM -0400, Daniel Veillard wrote:
> 
> > > > The Xen impl as coded only works for disks named xvdN, because the code for
> > > > calculating device ID assumes xvdN device numbering scheme:
> > > > 
> > > >     device = 202 * 256 + minor;
> > > 
> > > What's with device numbering at all? Must we really repeat this crazy
> > > Linuxism from xend in libvirt :(
> > 
> >   Do you have an alternative ? Because this is commited in CVS and I 
> > would like to get 0.3.2 rolled out ASAP ! So if you have a practical
> > suggestion or a patch, please post it, I may delay the issue but
> > just stating 'crazy Linuxism' without specifying why and how this can
> > be fixed won't help.
> 
> I just checked it out, and it looks like it can be safely encapsulated
> in this case. I was worried it would end up like xend but it seems we
> can entirely wrap the knowledge of this to be Linux-only.

  Yeah we will need to #ifdef/#else on a per OS basis, which is not perfect
but this is still workable, as long as the API is clean I don't have too
much concerns.

> BTW I am a little concerned that we're re-implementing all this code
> when libxenstat can do it. I already wrote this code once :)

  Ah :-) Well that's LGPL so we can probably reuse pieces of it. But
I don't feel too confident relying on an installed Xen library at this
point.

> apologies,

  No need, express yourself :-) What I want to make sure is that 0.3.2
is not broken on a serious basis. If you think you can provide a solaris
patch for that new code I can wait for a day before actually rolling out the
release. Just tell me if you think it's possible,

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/