[Libvir] [PATCH] Check availability of driver calls
This is a repost of a patch I sent before which adds the ability to check for driver features. This is completely internal to libvirt and does not add any public APIs. The original discussion is here: https://www.redhat.com/archives/libvir-list/2007-July/thread.html#00437 and continues here: https://www.redhat.com/archives/libvir-list/2007-August/thread.html#00000 Daniel Veillard said:
+typedef int + (*virDrvSupportsFeature) (virConnectPtr conn, virDrvFeature feature);
I would rather use , int features) where you OR the features, allows to know in one call if you get what you want or not.
My objection to that is here: https://www.redhat.com/archives/libvir-list/2007-August/msg00000.html Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Mon, Aug 20, 2007 at 03:13:34PM +0100, Richard W.M. Jones wrote:
This is a repost of a patch I sent before which adds the ability to check for driver features. This is completely internal to libvirt and does not add any public APIs.
The original discussion is here: https://www.redhat.com/archives/libvir-list/2007-July/thread.html#00437 and continues here: https://www.redhat.com/archives/libvir-list/2007-August/thread.html#00000
Daniel Veillard said:
+typedef int + (*virDrvSupportsFeature) (virConnectPtr conn, virDrvFeature feature);
I would rather use , int features) where you OR the features, allows to know in one call if you get what you want or not.
My objection to that is here: https://www.redhat.com/archives/libvir-list/2007-August/msg00000.html
The complexity comes if you think you need to return an array of integer. I guess if you just consider passing an unsigned long in and back, filled as an OR'ed bitfiled with the set of features you ask for you do 1 round trip you still pas and return an atomic value, you just need a little bit of decoding work at the C level to analyze the bits in the values, but that's won't be much more complex than a switch based on the enum, and IMHO more reliable, because I still (sorry) consider enums in APIs to be a big problem in C. Even if it's considered an internal API, I would avoid enums there because of the RPC. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
On Mon, Aug 20, 2007 at 10:26:04AM -0400, Daniel Veillard wrote:
The complexity comes if you think you need to return an array of integer. I guess if you just consider passing an unsigned long in and back, filled
It should be a fixed-size type, surely, and even then I doubt 64 bits is enough in the mid to long term. john
On Mon, Aug 20, 2007 at 03:31:19PM +0100, John Levon wrote:
On Mon, Aug 20, 2007 at 10:26:04AM -0400, Daniel Veillard wrote:
The complexity comes if you think you need to return an array of integer. I guess if you just consider passing an unsigned long in and back, filled
It should be a fixed-size type, surely, and even then I doubt 64 bits is enough in the mid to long term.
Since it's not public and just an RPC layer, if you need to check for more you can add a new separate call. I would not worry for this. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Daniel Veillard wrote:
On Mon, Aug 20, 2007 at 03:13:34PM +0100, Richard W.M. Jones wrote:
This is a repost of a patch I sent before which adds the ability to check for driver features. This is completely internal to libvirt and does not add any public APIs.
The original discussion is here: https://www.redhat.com/archives/libvir-list/2007-July/thread.html#00437 and continues here: https://www.redhat.com/archives/libvir-list/2007-August/thread.html#00000
Daniel Veillard said:
+typedef int + (*virDrvSupportsFeature) (virConnectPtr conn, virDrvFeature feature); I would rather use , int features) where you OR the features, allows to know in one call if you get what you want or not. My objection to that is here: https://www.redhat.com/archives/libvir-list/2007-August/msg00000.html
The complexity comes if you think you need to return an array of integer. I guess if you just consider passing an unsigned long in and back, filled as an OR'ed bitfiled with the set of features you ask for you do 1 round trip
This doesn't work though, because different parts of the system have to answer different parts of a request such as: VIR_DRV_FEATURE_MIGRATION_V1 | VIR_DRV_FEATURE_REMOTE. VIR_DRV_FEATURE_MIGRATION_V1 goes through to end driver. VIR_DRV_FEATURE_REMOTE is answered by the local end of remote (src/remote_internal.c). My real point is I don't understand the case where it is ever useful to query more than one feature.
you still pas and return an atomic value, you just need a little bit of decoding work at the C level to analyze the bits in the values, but that's won't be much more complex than a switch based on the enum, and IMHO more reliable, because I still (sorry) consider enums in APIs to be a big problem in C. Even if it's considered an internal API, I would avoid enums there because of the RPC.
I meant to get rid of the enums. Attached is a version which uses macros instead so the type should always be 'int' (ie. ABI stable). Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903
On Mon, Aug 20, 2007 at 04:43:27PM +0100, Richard W.M. Jones wrote:
Daniel Veillard wrote:
On Mon, Aug 20, 2007 at 03:13:34PM +0100, Richard W.M. Jones wrote:
This is a repost of a patch I sent before which adds the ability to check for driver features. This is completely internal to libvirt and does not add any public APIs.
The original discussion is here: https://www.redhat.com/archives/libvir-list/2007-July/thread.html#00437 and continues here: https://www.redhat.com/archives/libvir-list/2007-August/thread.html#00000
Daniel Veillard said:
+typedef int + (*virDrvSupportsFeature) (virConnectPtr conn, virDrvFeature feature); I would rather use , int features) where you OR the features, allows to know in one call if you get what you want or not. My objection to that is here: https://www.redhat.com/archives/libvir-list/2007-August/msg00000.html
The complexity comes if you think you need to return an array of integer. I guess if you just consider passing an unsigned long in and back, filled as an OR'ed bitfiled with the set of features you ask for you do 1 round trip
This doesn't work though, because different parts of the system have to answer different parts of a request such as:
VIR_DRV_FEATURE_MIGRATION_V1 | VIR_DRV_FEATURE_REMOTE.
VIR_DRV_FEATURE_MIGRATION_V1 goes through to end driver. VIR_DRV_FEATURE_REMOTE is answered by the local end of remote (src/remote_internal.c). My real point is I don't understand the case where it is ever useful to query more than one feature.
My first inclination was to also suggest a bitfield, but looking at the use case now I agree its overkill. If we did ever need to query more than one feature here, the roundtrip time would be dwarfed by the time to do the actual migration.
you still pas and return an atomic value, you just need a little bit of decoding work at the C level to analyze the bits in the values, but that's won't be much more complex than a switch based on the enum, and IMHO more reliable, because I still (sorry) consider enums in APIs to be a big problem in C. Even if it's considered an internal API, I would avoid enums there because of the RPC.
I meant to get rid of the enums. Attached is a version which uses macros instead so the type should always be 'int' (ie. ABI stable).
Ok, that's fine by me. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
On Tue, Aug 21, 2007 at 03:42:44AM +0100, Daniel P. Berrange wrote:
On Mon, Aug 20, 2007 at 04:43:27PM +0100, Richard W.M. Jones wrote:
This doesn't work though, because different parts of the system have to answer different parts of a request such as:
VIR_DRV_FEATURE_MIGRATION_V1 | VIR_DRV_FEATURE_REMOTE.
VIR_DRV_FEATURE_MIGRATION_V1 goes through to end driver. VIR_DRV_FEATURE_REMOTE is answered by the local end of remote (src/remote_internal.c). My real point is I don't understand the case where it is ever useful to query more than one feature.
My first inclination was to also suggest a bitfield, but looking at the use case now I agree its overkill. If we did ever need to query more than one feature here, the roundtrip time would be dwarfed by the time to do the actual migration.
Bah, if I'm in the minority that's fine, go ahead !
you still pas and return an atomic value, you just need a little bit of decoding work at the C level to analyze the bits in the values, but that's won't be much more complex than a switch based on the enum, and IMHO more reliable, because I still (sorry) consider enums in APIs to be a big problem in C. Even if it's considered an internal API, I would avoid enums there because of the RPC.
I meant to get rid of the enums. Attached is a version which uses macros instead so the type should always be 'int' (ie. ABI stable).
Ok, that's fine by me.
okay +1 Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
John Levon -
Richard W.M. Jones