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