Re: [libvirt] anyone implementing host device enumeration?
Hi Daniel -
I'm implementing virNodeDeviceDef, as suggested, but I just noticed
something that really bothers me. In a nutshell, it's that the current
implementation of this scheme unnecessarily increases the (time)
complexity of O(1) operations to O(n).
For example, take virDomainGetXMLDesc(). One would think dumping the
XML descriptor for a domain object we have in hand would be O(1), but in
fact, it's O(n) (where n is the number of domains with the same driver),
because qemudDomainDumpXML must find the DomainObjPtr (from which it can
get the def). I suppose this lookup could be made O(log n) if the
domains were sorted by UUID on the list, but the fact remains that
lookup could be eliminated entirely.
Perhaps it's just a matter of allowing the public objects
(virDomain, virNetwork, etc.) to hold a pointer to their optional (never
used by remote driver, for example) private state objects. So I guess
I'm suggesting adding a void * to each of the public objects, which
drivers can use for this purpose. I'll go ahead and do this for
NodeDevices in the next incarnation of this patch to show you what I
mean. (It won't be hard go to the conventional lookup impl if this
turns out to be a bad idea.)
Dave
On Fri, 2008-10-03 at 16:23 +0100, Daniel P. Berrange wrote:
On Tue, Sep 30, 2008 at 12:17:27AM -0400, David Lively wrote:
diff --git a/src/internal.h b/src/internal.h
index d96504d..9a94f6d 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -292,6 +307,25 @@ struct _virStorageVol {
};
+/**
+ * _virNodeDevice:
+ *
+ * Internal structure associated with a node device
+ */
+struct _virNodeDevice {
+unsigned int magic; /* specific value to check */
+int refs; /* reference count */
+virConnectPtr conn; /* pointer back to the connection
*/
+char *key; /* device key */
+char *name; /* device name */
+char *parent_key; /* parent device */
+char *bus_name; /* (optional) bus name */
+xmlNodePtr bus; /* (optional) bus descriptor */
+char **cap_names; /* capability names (null-term) */
+xmlNodePtr *caps; /* capability descs (null-term) */
+};
This is not the right way to maintain the internal representation of
devices.
The model we follow is to have 2, sometimes 3 structs to represent
an object. I'll summarize in terms of domain objects
- virDomainPtr - the public opaque struct representing a domain
the internal struct is defined in internal.h
and merely contains reference counting, connection
pointer, and any unique keys (ID, name, UUID).
- virDomainObjPtr - the internal struct representing the state of
a domain object. Not all drivers use us - only
stateful drivers do. It is used to track state
such as guest PID, logfile, running state,
and a pointer to the live and inactive configs
This is done in domain_conf.h/.c
- virDomainDefPtr - the internal struct representing the canonical
configuration of a domain. This is a direct
mapping of the XML into a series of structs.
This is done in domain_conf.h/.c
So, in the context for host devices we need a few changes. In the
internal.h file, you should only store the key/name attributes.
There should then be a separate file handling configuration parsing
and formatting, node_device_conf.h/.c. There is probably no need
for a state object, so skip virNodeDeviceObjPtr, and just create a
virNodeDeviceDefPtr representing the XML format as a series of
structs/unions, etc. See virDomainDefPtr for a good example. This
should not store any xmlNodePtr instances - it should be all done
as explicit struct/enum fields
The node_device_conf.c file should at mimium have 2 methods, one
for converting an XML document into a virNodeDeviceDefPtr object,
and one for the converting a virNodeDeviceObjPtr back into an XML
document. Follow the existing API naming / method signatures that
are seen in domain_conf.h / network_conf.h for current 'best practice'
Daniel
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] anyone implementing host device enumeration?
On Thu, Oct 09, 2008 at 12:25:18PM -0400, David Lively wrote:
Hi Daniel -
I'm implementing virNodeDeviceDef, as suggested, but I just noticed
something that really bothers me. In a nutshell, it's that the current
implementation of this scheme unnecessarily increases the (time)
complexity of O(1) operations to O(n).
For example, take virDomainGetXMLDesc(). One would think dumping the
XML descriptor for a domain object we have in hand would be O(1), but in
fact, it's O(n) (where n is the number of domains with the same driver),
because qemudDomainDumpXML must find the DomainObjPtr (from which it can
get the def). I suppose this lookup could be made O(log n) if the
domains were sorted by UUID on the list, but the fact remains that
lookup could be eliminated entirely.
Sure the lookup algorithm is O(n), but have you actually got real
world benchmarks showing this is the serious bottleneck? In the XML
dump example, the time to lookup the object is likely dwarfed by the
time to generate the XML doc, or to talk to the QEMU monitor.
IMHO, optimizing this is overkill. Worst case we can just hash on
the UUID if it ever provdes a problem.
Perhaps it's just a matter of allowing the public objects
(virDomain, virNetwork, etc.) to hold a pointer to their optional (never
used by remote driver, for example) private state objects. So I guess
I'm suggesting adding a void * to each of the public objects, which
drivers can use for this purpose. I'll go ahead and do this for
NodeDevices in the next incarnation of this patch to show you what I
mean. (It won't be hard go to the conventional lookup impl if this
turns out to be a bad idea.)
No please don't do that. We really need to keep the hard separation
between the public vir{Domain,Network,Storage,Node}Ptr objects, and
the internal objects completely separated. The only association
should be via the various unique keys - ID, Name UUID as appropriate.
The public objects do not neccessarily have the same lifecycle as
the internal object - eg, an app can hold onto the public object
even after the internal representation has been killed off.
Keeping a reference from the public to private objects, means we also
need to have a reverse references from priate to public objects, so we
can kill off the references when objects go away. This also means we
need to have complex locking from the internal drivers to the public
objects which for thread safety which I really do not want.
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
Re: [libvirt] anyone implementing host device enumeration?
On Thu, 2008-10-09 at 18:01 +0100, Daniel P. Berrange wrote: On Thu, Oct 09, 2008 at 12:25:18PM -0400, David Lively wrote: Hi Daniel - I'm implementing virNodeDeviceDef, as suggested, but I just noticed something that really bothers me. In a nutshell, it's that the current implementation of this scheme unnecessarily increases the (time) complexity of O(1) operations to O(n). For example, take virDomainGetXMLDesc(). One would think dumping the XML descriptor for a domain object we have in hand would be O(1), but in fact, it's O(n) (where n is the number of domains with the same driver), because qemudDomainDumpXML must find the DomainObjPtr (from which it can get the def). I suppose this lookup could be made O(log n) if the domains were sorted by UUID on the list, but the fact remains that lookup could be eliminated entirely. Sure the lookup algorithm is O(n), but have you actually got real world benchmarks showing this is the serious bottleneck? In the XML Well ... uhh ... no. :-o dump example, the time to lookup the object is likely dwarfed by the time to generate the XML doc, or to talk to the QEMU monitor. IMHO, optimizing this is overkill. Worst case we can just hash on the UUID if it ever provdes a problem. Okay, I'll buy that. And (now) I see what you mean about the differing object lifetime. ... In fact (perhaps I shouldn't be admitting this publicly), your whole public-obj/Obj scheme now finally makes sense to me! The public objs are really just a kind of handle to the private Objs, and they exist independently of one another. public objs belong to the apps, private Objs to the appropriate driver. And (now that I look at the Xen driver), I finally understand that there are stateless drivers. In fact, both the devkit and HAL node-dev drivers need to be stateful (since the device name isn't a reasonable handle for either devkit or HAL devices), so I'm creating a virNodeDeviceObj as well as a virNodeDeviceDef. Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] anyone implementing host device enumeration?
On Thu, Oct 09, 2008 at 02:29:18PM -0400, David Lively wrote: On Thu, 2008-10-09 at 18:01 +0100, Daniel P. Berrange wrote: dump example, the time to lookup the object is likely dwarfed by the time to generate the XML doc, or to talk to the QEMU monitor. IMHO, optimizing this is overkill. Worst case we can just hash on the UUID if it ever provdes a problem. Okay, I'll buy that. And (now) I see what you mean about the differing object lifetime. ... In fact (perhaps I shouldn't be admitting this publicly), your whole public-obj/Obj scheme now finally makes sense to me! The public objs are really just a kind of handle to the private Objs, and they exist independently of one another. public objs belong to the apps, private Objs to the appropriate driver. And (now that I look at the Xen driver), I finally understand that there are stateless drivers. I think its clear that the major thing lacking here is good documentation on the structure / internals of libvirt its object/driver modelling. Until very recently it was pretty much every driver for itself. With the introduction of unified internal objects, we do now have some formal data structure, beyond just the driver.h API contracts. Until we document this its not surprising that people find it all rather opaque to understand Time for me to add a section to the website docs covering internal architecture in more detail... In fact, both the devkit and HAL node-dev drivers need to be stateful (since the device name isn't a reasonable handle for either devkit or HAL devices), so I'm creating a virNodeDeviceObj as well as a virNodeDeviceDef. Great, that meshes with my mental model of things 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
Re: [libvirt] anyone implementing host device enumeration?
On Tue, 2008-09-30 at 19:31 +0200, Daniel Veillard wrote: Good to see that you seems to have the python bindings ready too ! Many python bindings are not really ready (in this patch). But I should have them in the next patch (Monday/Tuesday next week). I have complete java bindings for the current API, and will submit them once we agree upon it. I wonder how many of them should be future-proofed by adding them a final flags argument too ... For example it may be useful to only lookup devices which are local to the machine, or the opposite only remote devices. We don't have to specify now flags values, but having the APIs ready is sufficient. The virNodeNum/virNodeListDevices devices can probably all share the same flags definitions when needed. I agree. I'll add a flags arg to virNodeNum*/virNodeList*. The LookupByName/LookupByKey may be able to use the same set. I wonder a bit about the key argument though, I assume it's something actually defined by the lower APIs (hal/devkit). As long as the lookup keys are unique, I don't see why we'd want a flags arg for these functions. Currently the key is implementation- (HAL- or Devkit-) dependent, but I have questions about that (and for that matter, the name arg -- if it's unique, why have a separate key??). Dan B brings up some similar points, so I'll address this in my response to his post rather than here. For virNodeDeviceCreate maybe a flags may be needed too, though the flexibility of the API is provided by the XML. I think the XML should provide sufficient flexibility here. But let me know if you really want me to add a flag arg. +int virNodeDeviceDestroy(virNodeDevicePtr dev); + +int virNodeDeviceFree (virNodeDevicePtr dev); Maybe Destroy need flags too, for example to force (or not) destruction of devices which may be in use. Ok, I'll add a flags arg to Destroy as well. FWIW, I'm not wild about the names virNodeDeviceCreate/Destroy. Don't Attach and Detach make more sense? If there are no objections, I'll change those in my next iteration (though I'll still leave them unimplemented). @@ -332,6 +340,12 @@ skip_function = ( 'virCopyLastError', # Python API is called virGetLastError instead 'virConnectOpenAuth', # Python C code is manually written 'virDefaultErrorFunc', # Python virErrorFuncHandler impl calls this from C +'virNodeDeviceGetKey', +'virNodeDeviceGetName', +'virNodeDeviceGetParentKey', +'virNodeDeviceGetBus', +'virNodeDeviceNumOfCaps', +'virNodeDeviceListCaps', ) Strange how are the accessors supposed to work from python then ? They don't. They're just here so you don't get errors building the Python bindings. I'll implement real bindings for these soon. Hum, the libvirt_lxc really need to link to the device discovery libs ? I was making host device enumeration work for ALL hypervisors (though of course the remote driver has a remote impl), so I think this is necessary. But that said, I'm still digesting Dan B's point about licensing issues, and I really have no clue about how lxc and openvz work with libvirt (do they have separate control daemons, like Xen, or are they more like QEMU, or ???). virLibConnError(virConnectPtr conn, virErrorNumber error, const char *info) virLibConnWarning(virConnectPtr conn, virErrorNumber error, const char *info) you really need to export them ? Oh, uhhh, apparently not :-) (I did at one point, but must have removed that code.) I'll un-export them ... Hum ... we need the comment on the accessors, so they get extracted as part of the API when doing 'make rebuild' in docs/ added to the resulting XML, which then will provide the python accessors automatically I think. Will do. Thanks for the response. I'll implement the things discussed here (plus in Dan B's comments - another response coming) and submit a new patch on Monday or early Tuesday next week. Dave -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] anyone implementing host device enumeration?
On Tue, 2008-09-30 at 19:02 +0100, Daniel P. Berrange wrote:
On Tue, Sep 30, 2008 at 12:17:27AM -0400, David Lively wrote:
- Split stateful drivers out of libvirt.so, so they're only used
by the daemon, and not apps linking to libvirt.so.
This solves the licensing problem that HAL/DeviceKit introduce
libvirt.so needs to be LGPL to allow closed-source apps to link
to it. HAL/DeviceKit are both GPL or AFL licensed, by virtue
of using DBus. Since LGPL is not AFL compatible, if we link to
HAL/DeviceKit/DBus we do so under the GPL, and thus would
prevent closed source apps using libvirt.so We don't want this,
so we ned to make sure only the libvirtd daemon links to the
GPL bits.
Ok. I see. Will do.
* figure out how devkit and HAL correlate, so we report device info
consistently
This is looking like it'll be much harder than I had anticipated.
Yeah, I struggled for a while trying to reconcile them. But devkit's
not even close to complete yet, so I gave up. I'm assuming that, since
there are a lot of HAL - DevKit conversions to be done, some kind of
(probably domain-specific) method(s) of correlating the two will
eventually be described.
So I'm thinking perhaps we need to simplify our modelling a little
so its not so closely replicating HAL, getting rid of the distinct
elements for 'class' and 'bus' and having a device simply have a
'subsystem'. And instead of having a complete tree hierarchy, have
a specialized hierarchy. eg if we can identify a 'usb' or 'pci'
device parent for a device, then include its name, but don't
claim to provide a full hierarchy.
Yes! I find the distinction between bus and capability to be pretty
arbitrary anyway (being on a bus is a type of capability, IMO).
So I'll drop the bus stuff, and combine bus capabilities with the
other capabilities.
The other interesting question, is what policy we should define
for compatability. Do we absolutely need to have compatible
keys names for devices, whether using HAL vs DeviceKit, or
can be just say that the format of a name is opaque and liable
to change ? This has upgrade implications, for example, if we
ship libvirt with a HAL backend in Fedora 10, and then switched
it to the DeviceKit backend in Fedora 11, do we need to ensure
consistent naming across the upgrade path. I don't know...
Right. I was hoping to find a consistent set of keys/names to use, but
gave up. (sysfs-paths seemed the most common denominator, but not all
HAL devices have sysfs-paths, and I think the HAL names look more
appropriate anyway.) I'm hoping that devkit will eventually add a
property that either specifies or is convertible to a HAL name.
Also, if names are really unique (as specified), then why have separate
keys? I'd prefer dropping the Key functions and using Name as the key
(as we do for storage pools, etc.).
+struct _stringv {
+int len;
+char **const base;
+const int maxlen;
+};
+
+typedef struct _stringv stringv;
Perhaps I'm mis-undersatnding what this does, but isn't this
similar to the virStringList class in internal.h ?
Well, it's a vector instead of a singly-linked list. I use it when I
know the max length ahead of time (while the list is used in pool source
discovery code that knows of no limit on the number of sources it will
find). I could use a list for the former case, but that would require
an extra copy of each name (plus unnecessary alloc/dealloc code for the
list).
+static char *nodeDeviceDumpXML(virNodeDevicePtr dev,
+ unsigned int flags ATTRIBUTE_UNUSED)
+{
+xmlBufferPtr buf = xmlBufferCreate();
...
+xmlBufferCCat(buf, /key\n);
This should use the libvirt buffer routines from buf.h which
ensure you don't ignore return values indicating OOM like
you have here :-) See buf.h, or HACKING file for some examples.
Note I call xmlNodeDump(buf, ...). I suppose the right thing to do is
to add virBufNodeDump(...) to use libxml to dump into a virBuf ???
+/* TODO: Can we avoid this copy? */
+xml = strdup((const char *)xmlBufferContent(buf));
Yes, you can with the libvirt buf.h APIs :-)
But if I have to have to call xmlNodeDump, I can't avoid it ...
Thanks for the comments!
Dave
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] anyone implementing host device enumeration?
On Fri, Oct 03, 2008 at 11:02:55AM -0400, David Lively wrote:
On Tue, 2008-09-30 at 19:02 +0100, Daniel P. Berrange wrote:
On Tue, Sep 30, 2008 at 12:17:27AM -0400, David Lively wrote:
- Split stateful drivers out of libvirt.so, so they're only used
by the daemon, and not apps linking to libvirt.so.
This solves the licensing problem that HAL/DeviceKit introduce
libvirt.so needs to be LGPL to allow closed-source apps to link
to it. HAL/DeviceKit are both GPL or AFL licensed, by virtue
of using DBus. Since LGPL is not AFL compatible, if we link to
HAL/DeviceKit/DBus we do so under the GPL, and thus would
prevent closed source apps using libvirt.so We don't want this,
so we ned to make sure only the libvirtd daemon links to the
GPL bits.
Ok. I see. Will do.
No, no don't do that ! I've already got this work underway, since it
impacts much more than just the host device drivers. Just carry on
with your existing way, and when its ready to merge it'll be easy to
tweak the makefile to have it build in a suitable way.
So I'm thinking perhaps we need to simplify our modelling a little
so its not so closely replicating HAL, getting rid of the distinct
elements for 'class' and 'bus' and having a device simply have a
'subsystem'. And instead of having a complete tree hierarchy, have
a specialized hierarchy. eg if we can identify a 'usb' or 'pci'
device parent for a device, then include its name, but don't
claim to provide a full hierarchy.
Yes! I find the distinction between bus and capability to be pretty
arbitrary anyway (being on a bus is a type of capability, IMO).
So I'll drop the bus stuff, and combine bus capabilities with the
other capabilities.
The other interesting question, is what policy we should define
for compatability. Do we absolutely need to have compatible
keys names for devices, whether using HAL vs DeviceKit, or
can be just say that the format of a name is opaque and liable
to change ? This has upgrade implications, for example, if we
ship libvirt with a HAL backend in Fedora 10, and then switched
it to the DeviceKit backend in Fedora 11, do we need to ensure
consistent naming across the upgrade path. I don't know...
Right. I was hoping to find a consistent set of keys/names to use, but
gave up. (sysfs-paths seemed the most common denominator, but not all
HAL devices have sysfs-paths, and I think the HAL names look more
appropriate anyway.) I'm hoping that devkit will eventually add a
property that either specifies or is convertible to a HAL name.
Also, if names are really unique (as specified), then why have separate
keys? I'd prefer dropping the Key functions and using Name as the key
(as we do for storage pools, etc.).
The ultimate goal with the storage was that 'key' was unique consistent
for the same storage on every host the storage was attached to, while
'name' could vary across machines.
This distinction doesn't entirely apply to host devices, so perhaps
droping key is OK - we could always re-add it later if we found a
compelling need.
+static char *nodeDeviceDumpXML(virNodeDevicePtr dev,
+ unsigned int flags ATTRIBUTE_UNUSED)
+{
+xmlBufferPtr buf = xmlBufferCreate();
...
+xmlBufferCCat(buf, /key\n);
This should use the libvirt buffer routines from buf.h which
ensure you don't ignore return values indicating OOM like
you have here :-) See buf.h, or HACKING file for some examples.
Note I call xmlNodeDump(buf, ...). I suppose the right thing to do is
to add virBufNodeDump(...) to use libxml to dump into a virBuf ???
Ok, this actually makes me notice another problem - I'll reply
to the original patch with details. In summary though, the internal
data structure should not be using an xmlNodePtr as its canonical
form, and thus there should not be a need for xmlNodeDump.
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
Re: [libvirt] anyone implementing host device enumeration?
On Tue, Sep 30, 2008 at 12:17:27AM -0400, David Lively wrote:
diff --git a/src/internal.h b/src/internal.h
index d96504d..9a94f6d 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -292,6 +307,25 @@ struct _virStorageVol {
};
+/**
+ * _virNodeDevice:
+ *
+ * Internal structure associated with a node device
+ */
+struct _virNodeDevice {
+unsigned int magic; /* specific value to check */
+int refs; /* reference count */
+virConnectPtr conn; /* pointer back to the connection */
+char *key; /* device key */
+char *name; /* device name */
+char *parent_key; /* parent device */
+char *bus_name; /* (optional) bus name */
+xmlNodePtr bus; /* (optional) bus descriptor */
+char **cap_names; /* capability names (null-term) */
+xmlNodePtr *caps; /* capability descs (null-term) */
+};
This is not the right way to maintain the internal representation of
devices.
The model we follow is to have 2, sometimes 3 structs to represent
an object. I'll summarize in terms of domain objects
- virDomainPtr - the public opaque struct representing a domain
the internal struct is defined in internal.h
and merely contains reference counting, connection
pointer, and any unique keys (ID, name, UUID).
- virDomainObjPtr - the internal struct representing the state of
a domain object. Not all drivers use us - only
stateful drivers do. It is used to track state
such as guest PID, logfile, running state,
and a pointer to the live and inactive configs
This is done in domain_conf.h/.c
- virDomainDefPtr - the internal struct representing the canonical
configuration of a domain. This is a direct
mapping of the XML into a series of structs.
This is done in domain_conf.h/.c
So, in the context for host devices we need a few changes. In the
internal.h file, you should only store the key/name attributes.
There should then be a separate file handling configuration parsing
and formatting, node_device_conf.h/.c. There is probably no need
for a state object, so skip virNodeDeviceObjPtr, and just create a
virNodeDeviceDefPtr representing the XML format as a series of
structs/unions, etc. See virDomainDefPtr for a good example. This
should not store any xmlNodePtr instances - it should be all done
as explicit struct/enum fields
The node_device_conf.c file should at mimium have 2 methods, one
for converting an XML document into a virNodeDeviceDefPtr object,
and one for the converting a virNodeDeviceObjPtr back into an XML
document. Follow the existing API naming / method signatures that
are seen in domain_conf.h / network_conf.h for current 'best practice'
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
Re: [libvirt] anyone implementing host device enumeration?
On Fri, Oct 03, 2008 at 01:20:35PM -0400, David Lively wrote: Okay, I see what you mean. I'll create a virNodeDeviceDefPtr and follow the established pattern. I'm really trying to avoid creating a struct/union that must be extended every time we support a new capability. I struggled quite a bit with the right representation for capabilities and bus details.. At one point, I had my own (general-purpose; i.e., not specific to any type of capability) virNodeDeviceCapabilities type, but it looked so much like a DOM tree that just using a xmlNode seemed like the best choice. Are you suggesting creating a struct for each kind of currently-supported capability, or are you suggesting creating a single struct that's general enough to represent all capabilities? I'm suggesting a something that is specific for each capability. Now if we were to support all metadata that HAL exposes this would be a huge job. But I don't believe we should be exposing all the metadata that HAL has, because in the future this XML format has to work with DeviceKit which is basically exposing UDev metadata, and VMWare's APIs which expose hardware info in their own way. Thus, IMHO, we should expose specific capabilities we know we care about, for specific sub-substems, and not try to handle the entire of HAL. A good starting point would be - PCI, domain, bus, slot, function, and vendor/product - USB, bus, device and vendor/product Basically same info required for attaching the device to a domain, thus matching the struct virDomainHostdevDefPtr in domain_conf.h - NIC, name mac address - Block, name and host adapter - Host adapter, name For these also have a way to link to the parent device associated with them (ie the PCI/USB device providing them). That would basically be enough for use of the existing domain/storage and network APIs which is what we need this data for, and this minimal info should be satisifiable with VMWare's APIs, and DeviceKit. Regards, 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
Re: [libvirt] anyone implementing host device enumeration?
I definitely wasn't planning on covering all of HAL :-) I assume you weren't planning on exposing these capability-specific representations in the public API. Right? (If not, ignore the rest of this ...) So I guess I don't see the value of having these cap-specific internal representations. I keep a string array of the cap names for ListDevicesByCap, but other than that, the capability data is used only by virNodeDeviceGetXMLDesc(). So it seems natural to represent it in a form that can easily be converted to XML, and that can represent all the XML we'll need to output (like xmlNode). Otherwise, we are forced to write more capability-specific code, with no particular advantage (no stronger typing guarantees if we don't expose specific cap types). Dave P.S. I do think it would be useful to have access to capability details other than GetXMLDesc. Perhaps: const char *virNodeDeviceCapabilityProperty(virNodeDevicePtr dev, const char *cap, const char *prop) but note this exposes them only in a general (property / value) way, and is quite easily implemented with a xmlNode representation. On Fri, 2008-10-03 at 18:41 +0100, Daniel P. Berrange wrote: On Fri, Oct 03, 2008 at 01:20:35PM -0400, David Lively wrote: Okay, I see what you mean. I'll create a virNodeDeviceDefPtr and follow the established pattern. I'm really trying to avoid creating a struct/union that must be extended every time we support a new capability. I struggled quite a bit with the right representation for capabilities and bus details.. At one point, I had my own (general-purpose; i.e., not specific to any type of capability) virNodeDeviceCapabilities type, but it looked so much like a DOM tree that just using a xmlNode seemed like the best choice. Are you suggesting creating a struct for each kind of currently-supported capability, or are you suggesting creating a single struct that's general enough to represent all capabilities? I'm suggesting a something that is specific for each capability. Now if we were to support all metadata that HAL exposes this would be a huge job. But I don't believe we should be exposing all the metadata that HAL has, because in the future this XML format has to work with DeviceKit which is basically exposing UDev metadata, and VMWare's APIs which expose hardware info in their own way. Thus, IMHO, we should expose specific capabilities we know we care about, for specific sub-substems, and not try to handle the entire of HAL. A good starting point would be - PCI, domain, bus, slot, function, and vendor/product - USB, bus, device and vendor/product Basically same info required for attaching the device to a domain, thus matching the struct virDomainHostdevDefPtr in domain_conf.h - NIC, name mac address - Block, name and host adapter - Host adapter, name For these also have a way to link to the parent device associated with them (ie the PCI/USB device providing them). That would basically be enough for use of the existing domain/storage and network APIs which is what we need this data for, and this minimal info should be satisifiable with VMWare's APIs, and DeviceKit. Regards, Daniel -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] anyone implementing host device enumeration?
On Fri, Oct 03, 2008 at 02:35:06PM -0400, David Lively wrote: I definitely wasn't planning on covering all of HAL :-) I assume you weren't planning on exposing these capability-specific representations in the public API. Right? (If not, ignore the rest of this ...) Not at this time. For the forseeable future I expect the XML format to be the only publically exposed representation of configuration. A long time down the road when we're confident the representation is long term sustainable correct, we might consider exposing the structs, but that is a long way off. So I guess I don't see the value of having these cap-specific internal representations. I keep a string array of the cap names for ListDevicesByCap, but other than that, the capability data is used only by virNodeDeviceGetXMLDesc(). So it seems natural to represent it in a form that can easily be converted to XML, and that can represent all the XML we'll need to output (like xmlNode). Otherwise, we are forced to write more capability-specific code, with no particular advantage (no stronger typing guarantees if we don't expose specific cap types). The idea is that by having formal internal representations it makes it easier for internal driver code to work with it in a gaurenteed consistent fashion. While the structs may only be used by your specific driver for the intiial commit, experiance has shown our needs evolve over time and we'll probably end up with other internal code wanting it. We previously had each hypervisor driver using an adhoc representation internally for domain configs, and it just wasn't sustainable our internal usage evolved. P.S. I do think it would be useful to have access to capability details other than GetXMLDesc. Perhaps: const char *virNodeDeviceCapabilityProperty(virNodeDevicePtr dev, const char *cap, const char *prop) but note this exposes them only in a general (property / value) way, and is quite easily implemented with a xmlNode representation. I'm wary of exposing sub-sets of the XML docs in simple key/value pairs because our XML formats have tended to expand over time, so things that started off key/value pairs gained extra attributes/child elements, all of which are desired. It is simple enough for applications to wrap in a property 'getter' using XPath on the XML doc if desired - virt-manager does this internally for many things. 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
Re: [libvirt] anyone implementing host device enumeration?
On Tue, Sep 30, 2008 at 12:17:27AM -0400, David Lively wrote: Hi Daniel - I'm not really ready to submit this yet (hence no [PATCH] in the subject), but it is functional enough to play with, mostly with the HAL-based driver, though the devkit-based one works too. In particular, I'm not yet happy with the code to gather capabilities and bus info (though gather_capabilities in node_device_hal.c is close to what I'm going for). Also, many of the details (which properties do we care about for which buses and capabilities?) that make this useful are missing, though I've provided a few guesses to get started. And finally, it's had very little testing at this point. Okay, I think giving a bit of feedback at least on the API entry points makes sense even if not complete. Configure --with-hal or --with-devkit to support one or both (if both are configured, HAL is preferred, provided we can talk to hald on dbus). There are quite a few TODO's in the patch (mostly little things). The set of dependancies for libvirt is becoming incredibly large but that's fine :-) * figure out how devkit and HAL correlate, so we report device info consistently that has the potential of being nasty like seeing devices listed twice * register for devkit events to keep device info up-to-date okay it looks like there is still some work, but thanks a lot for sharing that first version, very appreciated ! Good to see that you seems to have the python bindings ready too ! +/* + * Host device enumeration + */ + +/** + * virNodeDevice: + * + * A virNodeDevice contains a node (host) device details. + */ + +typedef struct _virNodeDevice virNodeDevice; + +/** + * virNodeDevicePtr: + * + * A virNodeDevicePtr is a pointer to a virNodeDevice structure. Get + * one via virNodeDeviceLookupByKey, virNodeDeviceLookupByName, or + * virNodeDeviceCreate. Be sure to Call virNodeDeviceFree when done + * using a virNodeDevicePtr obtained from any of the above functions to + * avoid leaking memory. + */ [...] +const char *virNodeDeviceGetKey (virNodeDevicePtr dev); + +const char *virNodeDeviceGetName (virNodeDevicePtr dev); + +const char *virNodeDeviceGetParentKey(virNodeDevicePtr dev); + +const char *virNodeDeviceGetBus (virNodeDevicePtr dev); + +int virNodeDeviceNumOfCaps (virNodeDevicePtr dev); + +int virNodeDeviceListCaps(virNodeDevicePtr dev, + char **const names, + int maxnames); + +char * virNodeDeviceGetXMLDesc (virNodeDevicePtr dev, + unsigned int flags); + Opaque type with accessors, looks fine to me ! +typedef virNodeDevice *virNodeDevicePtr; + + +int virNodeNumOfDevices (virConnectPtr conn); + +int virNodeListDevices (virConnectPtr conn, + char **const names, + int maxnames); + +int virNodeNumOfDevicesByCap (virConnectPtr conn, + const char *cap); + +int virNodeListDevicesByCap (virConnectPtr conn, + const char *cap, + char **const names, + int maxnames); + +int virNodeNumOfDevicesByBus (virConnectPtr conn, + const char *bus); + +int virNodeListDevicesByBus (virConnectPtr conn, + const char *bus, + char **const names, + int maxnames); + +virNodeDevicePtrvirNodeDeviceLookupByName (virConnectPtr conn, + const char *name); + +virNodeDevicePtrvirNodeDeviceLookupByKey (virConnectPtr conn, + const char *key); + +virNodeDevicePtrvirNodeDeviceCreate (virConnectPtr conn, + const char *xml); I wonder how many of them should be future-proofed by adding them a final flags argument too ... For example it may be useful to only lookup devices which are local to the machine, or the opposite only remote devices. We don't have to specify now flags values, but having the APIs ready is sufficient. The virNodeNum/virNodeListDevices devices can probably all share the same flags definitions when needed. The LookupByName/LookupByKey may be able to use the same set. I wonder a bit about the key argument
Re: [libvirt] anyone implementing host device enumeration?
On Thu, Aug 21, 2008 at 03:18:57PM -0400, David Lively wrote: Hi - I'm about to start working on host device enumeration, along the (HAL-ish) lines of what was discussed back in April: https://www.redhat.com/archives/libvir-list/2008-April/msg5.html I know the xml details haven't been fully fleshed out, but there seems to be agreement that it will be a fairly direct mapping from (a subset of the) HAL info to the details that we need in the xml. Doubtless it will take a while to figure out exactly what subset suffices (and, for that matter, if everything needed is available via HAL ...), but I think the work is well-defined for some of the obvious details (discussed in the above thread) on which there's broad agreement. Is anyone working on such an implementation? Did you ever start any work on this project ? The oVirt guys really want this done asap, so if you've not started on it, or have a partial start to work from, I plan to make time to look at it next week Regards, 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
Re: [libvirt] anyone implementing host device enumeration?
Hi Daniel -
I have an implementation underway right now, with both HAL- and
Devkit-based drivers. We have some folks who need this *next week*,
so I'm trying to get the most functionality finished up today and this
weekend.
The generic (HAL/Devkit-agnostic) framework is essentially done and
tested (except for a few odds and ends like some missing virsh fns).
The HAL-based driver is ~80% done. The Devkit-based driver isn't very
capable yet, mostly because of the limited Devkit functionality
available (also, there are some serious naming issues to discuss).
I haven't yet implemented virNodeDevice{Create,Destroy}, nor am I
registering for HAL/Devkit events. We don't need this functionality
right away, so I may wait 'til next week to implement them. The
underlying virNodeDevice infrastructure is ref-counted and accessed via
a conn-lock-protected virHashTable (like connections, domains, etc.),
so concurrent access shouldn't be a problem.
I'll post a patch (with some TODOs ...) on Sunday or Monday morning.
Dave
On Fri, 2008-09-26 at 14:15 +0100, Daniel P. Berrange wrote:
On Thu, Aug 21, 2008 at 03:18:57PM -0400, David Lively wrote:
Hi -
I'm about to start working on host device enumeration, along the
(HAL-ish) lines of what was discussed back in April:
https://www.redhat.com/archives/libvir-list/2008-April/msg5.html
I know the xml details haven't been fully fleshed out, but there seems
to be agreement that it will be a fairly direct mapping from (a subset
of the) HAL info to the details that we need in the xml. Doubtless it
will take a while to figure out exactly what subset suffices (and, for
that matter, if everything needed is available via HAL ...), but I think
the work is well-defined for some of the obvious details (discussed in
the above thread) on which there's broad agreement.
Is anyone working on such an implementation?
Did you ever start any work on this project ? The oVirt guys really want
this done asap, so if you've not started on it, or have a partial start
to work from, I plan to make time to look at it next week
Regards,
Daniel
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] anyone implementing host device enumeration?
On Thu, Aug 21, 2008 at 03:18:57PM -0400, David Lively wrote: Hi - I'm about to start working on host device enumeration, along the (HAL-ish) lines of what was discussed back in April: https://www.redhat.com/archives/libvir-list/2008-April/msg5.html I know the xml details haven't been fully fleshed out, but there seems to be agreement that it will be a fairly direct mapping from (a subset of the) HAL info to the details that we need in the xml. Doubtless it will take a while to figure out exactly what subset suffices (and, for that matter, if everything needed is available via HAL ...), but I think the work is well-defined for some of the obvious details (discussed in the above thread) on which there's broad agreement. Is anyone working on such an implementation? No one that I'm aware of - so go for it. FYI, one complication has come up since my initial proposal. HAL is going away, to be replaced by DeviceKit :-( That said they will co-exist and HAL will be around for a while and they will both expose very similar properties for devices so it shouldn't too much trouble. I think we just need to be conservative when deciding how many of the HAL properties we need to add in libvirt. I'd go for a pretty minimal set at first. 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
Re: [libvirt] anyone implementing host device enumeration?
On Fri, Aug 22, 2008 at 09:44:47AM +0100, Daniel P. Berrange wrote: On Thu, Aug 21, 2008 at 03:18:57PM -0400, David Lively wrote: Hi - I'm about to start working on host device enumeration, along the (HAL-ish) lines of what was discussed back in April: https://www.redhat.com/archives/libvir-list/2008-April/msg5.html I know the xml details haven't been fully fleshed out, but there seems to be agreement that it will be a fairly direct mapping from (a subset of the) HAL info to the details that we need in the xml. Doubtless it will take a while to figure out exactly what subset suffices (and, for that matter, if everything needed is available via HAL ...), but I think the work is well-defined for some of the obvious details (discussed in the above thread) on which there's broad agreement. Is anyone working on such an implementation? No one that I'm aware of - so go for it. FYI, one complication has come up since my initial proposal. HAL is going away, to be replaced by DeviceKit :-( That said they will co-exist and HAL will be around for a while and they will both expose very similar properties for devices so it shouldn't too much trouble. I think we just need to be conservative when deciding how many of the HAL properties we need to add in libvirt. I'd go for a pretty minimal set at first. And HAL based detection might work on more platforms at least initially like OpenSolaris, 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
