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.
Thanks for posting this - its been very useful to see it in action
even in its current state.
> 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).
Makes sense, since from my little poking around DeviceKit itself is
very incomplete at this stage in its life.
> There are quite a few TODO's in the patch (mostly little things).
One other item for the list
- 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.
>
> The larger TODO's are (in (my) priority order):
> * generalize gather_capabilities (node_device_hal.c) to work for HAL
> bus info as well
> * share the nodeDevices hash table among (non-remote) connections
> (perhaps store it in the virNodeDriver?)
> * register for HAL events to keep device info up-to-date
> * finish virsh, python support (for now there's enough to get devs/xml)
> * support more capability & bus properties (in HAL)
> * generalize gather_capabilities to work for gathering bus & capability
> info for both HAL & devkit drivers
> * 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.
DeviceKit as it stands is basically a front-end to udev providing
a DBus API for accessing info udev has about devices. This is
inherantly Linux-specific. The OS-agnostic APis are apparently
ending up in the sub-system specific daemons like DeviceKit-Disks,
but only the disk one exists at this time.
So HAL is clearly the more portable option for a little while to
come, but for Linux at least DeviceKit will (eventually) be the
preferred way to access this kind of info.
There are some interesting differences in the way info is provided.
HAL has devices organized as a tree rooted at a 'computer' device.
DeviceKit basically presents a flat list of devices, and provides
no info correlating them. If you need to find out which PCI device
is associated with a network device 'ethX', then we'd need to go
outside the device kit API / dataset.
This is rather painful :-( I don't think we want to have to manually
create an entire hierarchy of all devices ourselves when using
DeviceKit, but if we wanted to replicate the data from our HAL backend
that's what we'd end up having todo.
DeviceKit also seems to have removed the distinction between 'bus'
and 'class'. Everything is now just a 'subsystem'. Some sub-systems
are physical and can be mapped to what HAL called a 'bus' (pci,
usb, etc), while othre sub-systems seem to be virtual and map
to what HAL calls a 'class' (net, block, video, etc).
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.
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...
> * register for devkit events to keep device info up-to-date
>
> Dave
>
> P.S. I'm afraid the patch is rather large, but remember to skip the
> generated files when looking it over:
>
> configure.in | 97 ++++++
> include/libvirt/libvirt.h | 78 +++++
> include/libvirt/libvirt.h.in | 78 +++++
> include/libvirt/virterror.h | 4
> python/generator.py | 15 +
> python/libvir.c | 127 ++++++++
> python/libvirt-python-api.xml | 17 +
> python/libvirt_wrap.h | 10
> python/types.c | 15 +
> qemud/remote.c | 282 +++++++++++++++++++
> qemud/remote_dispatch_localvars.h | 20 +
> qemud/remote_dispatch_proc_switch.h | 93 ++++++
> qemud/remote_dispatch_prototypes.h | 11
> qemud/remote_protocol.c | 220 +++++++++++++++
> qemud/remote_protocol.h | 184 ++++++++++++
> qemud/remote_protocol.x | 117 +++++++
> src/Makefile.am | 19 +
> src/driver.h | 67 ++++
> src/hash.c | 181 ++++++++++++
> src/internal.h | 52 +++
> src/libvirt.c | 528
> +++++++++++++++++++++++++++++++++++\
> -
> src/libvirt_sym.version | 14
> src/node_device.c | 262 +++++++++++++++++
> src/node_device.h | 38 ++
> src/node_device_devkit.c | 299 ++++++++++++++++++++
> src/node_device_hal.c | 475
> ++++++++++++++++++++++++++++++++
> src/remote_internal.c | 364 ++++++++++++++++++++++++
> src/virsh.c | 80 +++++
> src/virterror.c | 21 +
> 29 files changed, 3757 insertions(+), 11 deletions(-)
I've not had time for a full code review, so i'll just point out
the things I noticed in a short glance through.
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + *
> + * Author: David F. Lively <dlively@xxxxxxxxxxxxxxx>
> + */
> +
> +#include <config.h>
> +
> +#include <unistd.h>
> +#include <errno.h>
> +
> +#include "internal.h"
> +#include "memory.h"
> +
> +
> +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 ?
> +static char *nodeDeviceDumpXML(virNodeDevicePtr dev,
> + unsigned int flags ATTRIBUTE_UNUSED)
> +{
> + xmlBufferPtr buf = xmlBufferCreate();
> + char *xml;
> + int i;
> +
> + if (buf == NULL)
> + return NULL;
> +
> + xmlBufferCCat(buf, "<device>\n <name>");
> + xmlBufferCCat(buf, dev->name);
> + xmlBufferCCat(buf, "</name>\n <key>");
> + xmlBufferCCat(buf, dev->key);
> + 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.
> + if (dev->parent_key) {
> + xmlBufferCCat(buf, " <parent>");
> + xmlBufferCCat(buf, dev->parent_key);
> + xmlBufferCCat(buf, "</parent>\n");
> + }
> + if (dev->bus) {
> + xmlBufferCCat(buf, " ");
> + xmlNodeDump(buf, NULL, dev->bus, 1, 1);
> + xmlBufferCCat(buf, "\n");
> + }
> + if (dev->caps) {
> + for (i = 0; dev->caps[i]; i++) {
> + if (dev->caps[i]->parent == NULL) {
> + xmlBufferCCat(buf, " ");
> + xmlNodeDump(buf, NULL, dev->caps[i], 1, 1);
> + xmlBufferCCat(buf, "\n");
> + }
> + }
> + }
> + xmlBufferCCat(buf, "</device>\n");
> +
> + /* TODO: Can we avoid this copy? */
> + xml = strdup((const char *)xmlBufferContent(buf));
Yes, you can with the libvirt buf.h APIs :-)
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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list