[libvirt] Re: [PATCH 0/6] host (node) device enumeration, take two

2008-10-23 Thread Daniel P. Berrange
On Tue, Oct 21, 2008 at 01:45:47PM -0400, David Lively wrote:
 Ok, here's my substantially-reworked node device enumeration patch, this
 time done with a proper understanding of the public-obj/Obj/Def
 model :-) as last discussed here:
 https://www.redhat.com/archives/libvir-list/2008-September/msg00398.html
 
 I've broken it into the following pieces:
   1-public-apiadditions to the public API
   2-internal-api  additions to the internal API
   3-local-node-driversthe HAL  DeviceKit implementations
   4-remote-node-driverthe remote driver
   5-virsh-support virsh support
   6-python-bindings   python bindings
 
 Big differences from the last submission:
   * public-obj/Obj/Def object model finally understood :-)
   * capabilities structure now struct-ified, handled like
 other Def bits

I like the way this bit turned out - makes it very clear what properties
we are exporting in the XML as part of our API/ABI gaurentee.

   * using newfangled array-based lists for NodeDeviceObj's

The individual capabilities within a device are still using a linked list,
though that's not a critical problem from the thread safety point of view.

   * added flags args to various public APIs (as suggested by Dan V)
   * bus folded into capabilities (as discussed w/Dan B)

Yes, this looks much nicer too.

   * device key no longer exists - name is unique on node
 
 Some pieces are still incomplete, but I thought it would be better to
 post what I have since it's useful as is.  Here's what I know of
 that's left to do:
   * finish Python bindings (will get to Real Soon Now)
   * submit Java bindings (I have them, and have been using them)
   * implement virNodeDeviceCreate/Destroy (I have no plans for these)

No need to worry about this - I'd like to use them to implement the
creation/deletion of NPIV virtual HBAs eventually.

   * subscribe to HAL events  update dev state from them (next for me?)
   * implement pci_bus/usb_bus capabilities (for PCI/USB controllers)

While on the subject of USB, I've just noticed that HAL seems to
expose 2 devices for every USB device - 'usb' and 'usb_device',
which we're mapping into just a 'usb' capability. Unless there's
compelling reason to have both we might consider just filtering
out one of the two.

   * DeviceKit implementation is barely a proof of concept

Noticed one problem - on my build it refuses to enumerate devices
if you pass in a NULL for subsystem name list. I made a really
quick  dirty hack to just try it out

@@ -300,13 +301,18 @@ static int devkitNodeDriverStartup(void)
 }
 
 /* Populate with known devices */
-devs = devkit_client_enumerate_by_subsystem(devkit_client, NULL, err);
-if (err) {
-DEBUG0(devkit_client_enumerate_by_subsystem failed);
-devs = NULL;
-goto failure;
+for (i = 0 ; i  ARRAY_CARDINALITY(caps_tbl) ; i++) {
+const char *caps[] = { caps_tbl[i].cap_name, NULL };
+devs = devkit_client_enumerate_by_subsystem(devkit_client,
+caps,
+err);
+if (err) {
+DEBUG0(devkit_client_enumerate_by_subsystem failed);
+devs = NULL;
+goto failure;
+}
+g_list_foreach(devs, dev_create, devkit_client);
 }
-g_list_foreach(devs, dev_create, devkit_client);
 
 driverState-privateData = devkit_client;
 


 * need to resolve naming  other issues (see
 https://www.redhat.com/archives/libvir-list/2008-September/msg00430.html
 * ... and then fill in impl (no hurry; devkit immature now)

I'm still wondering if it is worth us santizing the device names ourselves
somehow, though it might end up being rather a large job.  Will have to
think some more about it.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] Re: [PATCH 0/6] host (node) device enumeration, take two

2008-10-23 Thread David Lively
On Thu, 2008-10-23 at 13:53 +0100, Daniel P. Berrange wrote:
 On Tue, Oct 21, 2008 at 01:45:47PM -0400, David Lively wrote:
* using newfangled array-based lists for NodeDeviceObj's
 
 The individual capabilities within a device are still using a linked list,
 though that's not a critical problem from the thread safety point of view.

Right.  This was intentional, since there are no per-capability
operations (and hence no need for per-capability locking granularity).

* subscribe to HAL events  update dev state from them (next for me?)

BTW, I'm working on this now.  Should have something to submit in
another day or two.

* implement pci_bus/usb_bus capabilities (for PCI/USB controllers)
 
 While on the subject of USB, I've just noticed that HAL seems to
 expose 2 devices for every USB device - 'usb' and 'usb_device',
 which we're mapping into just a 'usb' capability. Unless there's
 compelling reason to have both we might consider just filtering
 out one of the two.

Yeah, I meant to bring this up.  HAL uses usb_device for the USB
devices, and usb for the USB interface(s) provided by the device.  The
usb_device is the parent of the usb interface(s).  The HAL usb
namespace mirrors the attributes of the parent usb_device, and adds a
few of its own, specific to the interface.  Right now I'm reporting both
mostly so the device hierarchy (as defined by the parent relation) is
consistent.

I see two choices:
(a) Just report the interface (usb) objects and fixup their parent to
be the parent of their owning USB device.  Each interface object (as in
HAL) will reflect the details of the parent USB device, as well as the
interface-specific bits.
(b) Split libvirt's usb capability into usb_device and
usb_interface.  A usb_interface will always have a usb_device as it's
parent (and I would *not* replicate the usb_device details in the
usb_interface - that's the whole reason for keeping it as a separate
device).

I'm leaning fairly strongly towards (b).  What do you all think?

 
* DeviceKit implementation is barely a proof of concept
 
 Noticed one problem - on my build it refuses to enumerate devices
 if you pass in a NULL for subsystem name list. I made a really
 quick  dirty hack to just try it out
 
 @@ -300,13 +301,18 @@ static int devkitNodeDriverStartup(void)
  }
  
  /* Populate with known devices */
 -devs = devkit_client_enumerate_by_subsystem(devkit_client, NULL, err);
 -if (err) {
 -DEBUG0(devkit_client_enumerate_by_subsystem failed);
 -devs = NULL;
 -goto failure;
 +for (i = 0 ; i  ARRAY_CARDINALITY(caps_tbl) ; i++) {
 +const char *caps[] = { caps_tbl[i].cap_name, NULL };
 +devs = devkit_client_enumerate_by_subsystem(devkit_client,
 +caps,
 +err);
 +if (err) {
 +DEBUG0(devkit_client_enumerate_by_subsystem failed);
 +devs = NULL;
 +goto failure;
 +}
 +g_list_foreach(devs, dev_create, devkit_client);

Oh yeah.  I forgot I fixed devkit_client_enumerate_by_subsystem to work
as advertised with a NULL subsystem.  I submitted the fix a couple
months ago to [EMAIL PROTECTED], but never got any
acknowledgment, and haven't seen any activity on the list whatsoever, so
I'm not surprised it hasn't made it in.  Any idea where I should submit
devkit fixes?

In any case, I'll put your workaround in for now.  It won't pick up
devices in subsystems not listed in caps_tbl, but those are fairly
useless devices anyway (since we won't recognize any device caps if we
don't explicitly handle that subsystem in caps_tbl).

  }
 -g_list_foreach(devs, dev_create, devkit_client);
  
  driverState-privateData = devkit_client;
  
 
 
  * need to resolve naming  other issues (see
  https://www.redhat.com/archives/libvir-list/2008-September/msg00430.html
  * ... and then fill in impl (no hurry; devkit immature now)
 
 I'm still wondering if it is worth us santizing the device names ourselves
 somehow, though it might end up being rather a large job.  Will have to
 think some more about it.

Yeah, it's a nasty issue.  In a lot of ways, the HAL names are pretty
nice.  For example, I really like NICs named by MAC address (so knowing
that a particular NIC is eth0 is *not* encoded in the device name, but
rather in a capability).  This is also much nicer than naming by (e.g.)
sysfs path, which encodes too much info about where the card is plugged
in (that's what parent info is for).

But I don't see any sign that Devkit is exposing HALish names, though
I'd imagine (if only to make HAL-Devkit transition easier) that they
will need to expose a device property like HAL_UDI.  With no activity
on the devkit-devel list, I'm not sure where they're going.

Thanks for the comments.  I'll incorporate your devkit workaround, and
wait on further comment before submitting the next take.

Dave

--