Re: [libvirt] Re: [PATCH 2/6] host (node) device enumeration - internal API additions
On Tue, Oct 21, 2008 at 01:54:13PM -0400, David Lively wrote: This patch contains the internal API additions. Looks fine to me, just a couple of questions: +// TODO: Implement PCI bus devs +// TODO: Implement USB bus devs Any reason it's not in ? I would expect HAL to report them, no ? + +// TODO: virNodeDeviceDefParseString/File/Node for virNodeDeviceCreate No sure, could you explain a bit ? thanks, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: [PATCH 2/6] host (node) device enumeration - internal API additions
On Tue, Oct 28, 2008 at 11:32:03AM -0400, David Lively wrote: On Tue, 2008-10-28 at 14:13 +0100, Daniel Veillard wrote: On Tue, Oct 21, 2008 at 01:54:13PM -0400, David Lively wrote: This patch contains the internal API additions. Looks fine to me, just a couple of questions: +// TODO: Implement PCI bus devs +// TODO: Implement USB bus devs Any reason it's not in ? I would expect HAL to report them, no ? I posted a new patch set yesterday that (among other things) removed the PCI_BUS and USB_BUS devices. Currently HAL doesn't report them as anything special, e.g.: udi = '/org/freedesktop/Hal/devices/pci_8086_27cc' info.linux.driver = 'ehci_hcd' (string) info.parent = '/org/freedesktop/Hal/devices/computer' (string) info.product = '82801G (ICH7 Family) USB2 EHCI Controller' (string) info.subsystem = 'pci' (string) info.udi = '/org/freedesktop/Hal/devices/pci_8086_27cc' (string) info.vendor = 'Intel Corporation' (string) linux.hotplug_type = 2 (0x2) (int) linux.subsystem = 'pci' (string) linux.sysfs_path = '/sys/devices/pci:00/:00:1d.7' (string) pci.device_class = 12 (0xc) (int) pci.device_protocol = 32 (0x20) (int) pci.device_subclass = 3 (0x3) (int) pci.linux.sysfs_path = '/sys/devices/pci:00/:00:1d.7' (string) pci.product = '82801G (ICH7 Family) USB2 EHCI Controller' (string) pci.product_id = 10188 (0x27cc) (int) pci.subsys_product_id = 22594 (0x5842) (int) pci.subsys_vendor = 'Intel Corporation' (string) pci.subsys_vendor_id = 32902 (0x8086) (int) pci.vendor = 'Intel Corporation' (string) pci.vendor_id = 32902 (0x8086) (int) udi = '/org/freedesktop/Hal/devices/pci_8086_244e' info.parent = '/org/freedesktop/Hal/devices/computer' (string) info.product = '82801 PCI Bridge' (string) info.subsystem = 'pci' (string) info.udi = '/org/freedesktop/Hal/devices/pci_8086_244e' (string) info.vendor = 'Intel Corporation' (string) linux.hotplug_type = 2 (0x2) (int) linux.subsystem = 'pci' (string) linux.sysfs_path = '/sys/devices/pci:00/:00:1e.0' (string) pci.device_class = 6 (0x6) (int) pci.device_protocol = 1 (0x1) (int) pci.device_subclass = 4 (0x4) (int) pci.linux.sysfs_path = '/sys/devices/pci:00/:00:1e.0' (string) pci.product = '82801 PCI Bridge' (string) pci.product_id = 9294 (0x244e) (int) pci.subsys_product_id = 0 (0x0) (int) pci.subsys_vendor_id = 0 (0x0) (int) pci.vendor = 'Intel Corporation' (string) pci.vendor_id = 32902 (0x8086) (int) So at this point, I just report them as PCI devices, though of course the PCI bridge will be the parent of PCI devices on that bus, and the USB controller will be the parent of USB devices controlled by it. Any client code that needs to recognize them specifically as PCI or USB bus controllers can use the PCI device attributes for that. Hmmm ... currently I expose the product vendor info for PCI devices, along with their bus addressing info. But perhaps I should also expose their device_class, protocol, and subclass?? + +// TODO: virNodeDeviceDefParseString/File/Node for virNodeDeviceCreate Currently I have no need to parse a NodeDevice definition. But virNodeDeviceCreate will need to do that, once it's implemented. (I don't have any NPIV NICs or other such toys to play with right now. Daniel B says he'll implement Create/Destroy since he does.) I also thought it would be better to postpone the parser work until needed since the node definition seems likely to change as we work out the details here. Okay, thanks for the clarification, and sorry for the delay, as you can guess I didn't yet reached the end of the folder grin/ ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [EMAIL PROTECTED] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [PATCH 2/6] host (node) device enumeration - internal API additions
This patch contains the internal API additions.
diff --git a/src/hash.c b/src/hash.c
index 0a5bdcd..424c4a7 100644
--- a/src/hash.c
+++ b/src/hash.c
@@ -687,6 +687,9 @@ virGetConnect(void) {
ret-storageVols = virHashCreate(20);
if (ret-storageVols == NULL)
goto failed;
+ret-nodeDevices = virHashCreate(256);
+if (ret-nodeDevices == NULL)
+goto failed;
pthread_mutex_init(ret-lock, NULL);
@@ -703,6 +706,8 @@ failed:
virHashFree(ret-storagePools, (virHashDeallocator) virStoragePoolFreeName);
if (ret-storageVols != NULL)
virHashFree(ret-storageVols, (virHashDeallocator) virStorageVolFreeName);
+if (ret-nodeDevices != NULL)
+virHashFree(ret-nodeDevices, (virHashDeallocator) virNodeDeviceFree);
pthread_mutex_destroy(ret-lock);
VIR_FREE(ret);
@@ -730,6 +735,8 @@ virReleaseConnect(virConnectPtr conn) {
virHashFree(conn-storagePools, (virHashDeallocator) virStoragePoolFreeName);
if (conn-storageVols != NULL)
virHashFree(conn-storageVols, (virHashDeallocator) virStorageVolFreeName);
+if (conn-nodeDevices != NULL)
+virHashFree(conn-nodeDevices, (virHashDeallocator) virNodeDeviceFree);
virResetError(conn-err);
if (__lastErr.conn == conn)
@@ -1318,3 +1325,126 @@ virUnrefStorageVol(virStorageVolPtr vol) {
pthread_mutex_unlock(vol-conn-lock);
return (refs);
}
+
+
+/**
+ * virGetNodeDevice:
+ * @conn: the hypervisor connection
+ * @name: device name (unique on node)
+ *
+ * Lookup if the device is already registered for that connection,
+ * if yes return a new pointer to it, if no allocate a new structure,
+ * and register it in the table. In any case a corresponding call to
+ * virFreeNodeDevice() is needed to not leak data.
+ *
+ * Returns a pointer to the node device, or NULL in case of failure
+ */
+virNodeDevicePtr
+__virGetNodeDevice(virConnectPtr conn, const char *name)
+{
+virNodeDevicePtr ret = NULL;
+
+if ((!VIR_IS_CONNECT(conn)) || (name == NULL)) {
+virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
+return(NULL);
+}
+pthread_mutex_lock(conn-lock);
+
+ret = (virNodeDevicePtr) virHashLookup(conn-nodeDevices, name);
+if (ret == NULL) {
+ if (VIR_ALLOC(ret) 0) {
+virHashError(conn, VIR_ERR_NO_MEMORY, _(allocating node dev));
+goto error;
+}
+ret-magic = VIR_NODE_DEVICE_MAGIC;
+ret-conn = conn;
+ret-name = strdup(name);
+if (ret-name == NULL) {
+virHashError(conn, VIR_ERR_NO_MEMORY, _(copying node dev name));
+goto error;
+}
+
+if (virHashAddEntry(conn-nodeDevices, name, ret) 0) {
+virHashError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(failed to add node dev to conn hash table));
+goto error;
+}
+conn-refs++;
+}
+ret-refs++;
+pthread_mutex_unlock(conn-lock);
+return(ret);
+
+error:
+pthread_mutex_unlock(conn-lock);
+if (ret != NULL) {
+VIR_FREE(ret-name);
+VIR_FREE(ret);
+}
+return(NULL);
+}
+
+
+/**
+ * virReleaseNodeDevice:
+ * @dev: the dev to release
+ *
+ * Unconditionally release all memory associated with a dev.
+ * The conn.lock mutex must be held prior to calling this, and will
+ * be released prior to this returning. The dev obj must not
+ * be used once this method returns.
+ *
+ * It will also unreference the associated connection object,
+ * which may also be released if its ref count hits zero.
+ */
+static void
+virReleaseNodeDevice(virNodeDevicePtr dev) {
+virConnectPtr conn = dev-conn;
+DEBUG(release dev %p %s, dev, dev-name);
+
+if (virHashRemoveEntry(conn-nodeDevices, dev-name, NULL) 0)
+virHashError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(dev missing from connection hash table));
+
+dev-magic = -1;
+VIR_FREE(dev-name);
+VIR_FREE(dev);
+
+DEBUG(unref connection %p %s %d, conn, conn-name, conn-refs);
+conn-refs--;
+if (conn-refs == 0) {
+virReleaseConnect(conn);
+/* Already unlocked mutex */
+return;
+}
+
+pthread_mutex_unlock(conn-lock);
+}
+
+
+/**
+ * virUnrefNodeDevice:
+ * @dev: the dev to unreference
+ *
+ * Unreference the dev. If the use count drops to zero, the structure is
+ * actually freed.
+ *
+ * Returns the reference count or -1 in case of failure.
+ */
+int
+virUnrefNodeDevice(virNodeDevicePtr dev) {
+int refs;
+
+pthread_mutex_lock(dev-conn-lock);
+DEBUG(unref dev %p %s %d, dev, dev-name, dev-refs);
+dev-refs--;
+refs = dev-refs;
+if (refs == 0) {
+virReleaseNodeDevice(dev);
+/* Already unlocked mutex */
+return (0);
+}
+
+pthread_mutex_unlock(dev-conn-lock);
+return (refs);
+}
diff --git a/src/internal.h b/src/internal.h
index 2ae764d..76b57ad 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -37,6
