[libvirt] [PATCH] Improve some error messages about unsupported APIs/URIs
If there is no driver for a URI we report "no hypervisor driver available" This is bad because not all virt drivers are hypervisors (ie container based virt). If there is no driver support for an API we report "this function is not supported by the hypervisor" This is bad for the same reason, and additionally because it is also used for the network, interface & storage drivers. * src/util/virterror.c: Improve error messages --- src/util/virterror.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/util/virterror.c b/src/util/virterror.c index 96dd1e7..9f632ec 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -760,15 +760,15 @@ virErrorMsg(virErrorNumber error, const char *info) break; case VIR_ERR_NO_SUPPORT: if (info == NULL) - errmsg = _("this function is not supported by the hypervisor"); + errmsg = _("this function is not supported by the connection driver"); else - errmsg = _("this function is not supported by the hypervisor: %s"); + errmsg = _("this function is not supported by the connection driver: %s"); break; case VIR_ERR_NO_CONNECT: if (info == NULL) - errmsg = _("no hypervisor driver available"); + errmsg = _("no connection driver available"); else - errmsg = _("no hypervisor driver available for %s"); + errmsg = _("no connection driver available for %s"); break; case VIR_ERR_INVALID_CONN: if (info == NULL) -- 1.6.6.1
On 06/22/2010 11:40 AM, Daniel P. Berrange wrote:
If there is no driver for a URI we report
"no hypervisor driver available"
This is bad because not all virt drivers are hypervisors (ie container based virt).
If there is no driver support for an API we report
"this function is not supported by the hypervisor"
This is bad for the same reason, and additionally because it is also used for the network, interface & storage drivers.
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2010/6/22 Daniel P. Berrange <berrange@redhat.com>:
If there is no driver for a URI we report
"no hypervisor driver available"
This is bad because not all virt drivers are hypervisors (ie container based virt).
If there is no driver support for an API we report
"this function is not supported by the hypervisor"
This is bad for the same reason, and additionally because it is also used for the network, interface & storage drivers.
* src/util/virterror.c: Improve error messages --- src/util/virterror.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/util/virterror.c b/src/util/virterror.c index 96dd1e7..9f632ec 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -760,15 +760,15 @@ virErrorMsg(virErrorNumber error, const char *info) break; case VIR_ERR_NO_SUPPORT: if (info == NULL) - errmsg = _("this function is not supported by the hypervisor"); + errmsg = _("this function is not supported by the connection driver"); else - errmsg = _("this function is not supported by the hypervisor: %s"); + errmsg = _("this function is not supported by the connection driver: %s");
As you said this message can also be triggered by network, interface, storage, etc. drivers not supporting a function. Therefore, I think we should not include the word 'connection' in the message. Matthias
On Tue, Jun 22, 2010 at 08:01:09PM +0200, Matthias Bolte wrote:
2010/6/22 Daniel P. Berrange <berrange@redhat.com>:
If there is no driver for a URI we report
"no hypervisor driver available"
This is bad because not all virt drivers are hypervisors (ie container based virt).
If there is no driver support for an API we report
"this function is not supported by the hypervisor"
This is bad for the same reason, and additionally because it is also used for the network, interface & storage drivers.
* src/util/virterror.c: Improve error messages --- src/util/virterror.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/util/virterror.c b/src/util/virterror.c index 96dd1e7..9f632ec 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -760,15 +760,15 @@ virErrorMsg(virErrorNumber error, const char *info) break; case VIR_ERR_NO_SUPPORT: if (info == NULL) - errmsg = _("this function is not supported by the hypervisor"); + errmsg = _("this function is not supported by the connection driver"); else - errmsg = _("this function is not supported by the hypervisor: %s"); + errmsg = _("this function is not supported by the connection driver: %s");
As you said this message can also be triggered by network, interface, storage, etc. drivers not supporting a function. Therefore, I think we should not include the word 'connection' in the message.
The virConnectPtr is the connection in this context and the driver activated there determines what's used for the hypervisor, storage, network, interface APIs. Hypervisor was too narrow, but connection is relevant here IMHO. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2010/6/22 Daniel P. Berrange <berrange@redhat.com>:
On Tue, Jun 22, 2010 at 08:01:09PM +0200, Matthias Bolte wrote:
2010/6/22 Daniel P. Berrange <berrange@redhat.com>:
If there is no driver for a URI we report
"no hypervisor driver available"
This is bad because not all virt drivers are hypervisors (ie container based virt).
If there is no driver support for an API we report
"this function is not supported by the hypervisor"
This is bad for the same reason, and additionally because it is also used for the network, interface & storage drivers.
* src/util/virterror.c: Improve error messages --- src/util/virterror.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/util/virterror.c b/src/util/virterror.c index 96dd1e7..9f632ec 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -760,15 +760,15 @@ virErrorMsg(virErrorNumber error, const char *info) break; case VIR_ERR_NO_SUPPORT: if (info == NULL) - errmsg = _("this function is not supported by the hypervisor"); + errmsg = _("this function is not supported by the connection driver"); else - errmsg = _("this function is not supported by the hypervisor: %s"); + errmsg = _("this function is not supported by the connection driver: %s");
As you said this message can also be triggered by network, interface, storage, etc. drivers not supporting a function. Therefore, I think we should not include the word 'connection' in the message.
The virConnectPtr is the connection in this context and the driver activated there determines what's used for the hypervisor, storage, network, interface APIs. Hypervisor was too narrow, but connection is relevant here IMHO.
Okay, I can interpret 'connection driver' as a driver attached to (or used by) a connection. ACK. Matthias
On 06/23/2010 04:16 AM, Matthias Bolte wrote:
case VIR_ERR_NO_SUPPORT:
if (info == NULL)
Would it be practical to expand this with category messages like? case VIR_ERR_NO_SUPPORT_STORAGE: if (info == NULL) errmsg = _("this function is not supported by the storage driver"); ... case VIR_ERR_NO_SUPPORT_NETWORK: if (info == NULL) errmsg = _("this function is not supported by the network driver"); ... Or maybe figure out a "%s" replaceable keyword for network/storage/hypervisor/etc above? Regards and best wishes, Justin Clift -- Salasaga - Open Source eLearning IDE http://www.salasaga.org
On 06/22/2010 03:48 PM, Justin Clift wrote:
On 06/23/2010 04:16 AM, Matthias Bolte wrote:
case VIR_ERR_NO_SUPPORT:
if (info == NULL)
Would it be practical to expand this with category messages like?
case VIR_ERR_NO_SUPPORT_STORAGE: if (info == NULL) errmsg = _("this function is not supported by the storage driver"); ...
case VIR_ERR_NO_SUPPORT_NETWORK: if (info == NULL) errmsg = _("this function is not supported by the network driver"); ...
Or maybe figure out a "%s" replaceable keyword for network/storage/hypervisor/etc above?
The whole error reporting scheme needs some loving care, and I've got a partial patch series posted to the list earlier that started that process. But it was deemed less important than getting 0.8.2 finalized (as well as helping with RHEL-6 backports), so it's been on the back burner. At this point, a minimally-invasive patch, like Daniel's proposal, is better than making things better when the code is all going to be refactored down the road anyways. But thanks for the thought; it may affect my refactoring efforts! -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
how many drivers aren't hypervisors ? On Tue, Jun 22, 2010 at 6:09 PM, Eric Blake <eblake@redhat.com> wrote:
On 06/22/2010 03:48 PM, Justin Clift wrote:
On 06/23/2010 04:16 AM, Matthias Bolte wrote:
case VIR_ERR_NO_SUPPORT:
if (info == NULL)
Would it be practical to expand this with category messages like?
case VIR_ERR_NO_SUPPORT_STORAGE: if (info == NULL) errmsg = _("this function is not supported by the storage driver"); ...
case VIR_ERR_NO_SUPPORT_NETWORK: if (info == NULL) errmsg = _("this function is not supported by the network driver"); ...
Or maybe figure out a "%s" replaceable keyword for network/storage/hypervisor/etc above?
The whole error reporting scheme needs some loving care, and I've got a partial patch series posted to the list earlier that started that process. But it was deemed less important than getting 0.8.2 finalized (as well as helping with RHEL-6 backports), so it's been on the back burner. At this point, a minimally-invasive patch, like Daniel's proposal, is better than making things better when the code is all going to be refactored down the road anyways. But thanks for the thought; it may affect my refactoring efforts!
-- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Why not be consistent with http://libvirt.org/goals.html. I'm really agreed with use the concept hypervisor. On Tue, Jun 22, 2010 at 6:09 PM, Eric Blake <eblake@redhat.com> wrote:
On 06/22/2010 03:48 PM, Justin Clift wrote:
On 06/23/2010 04:16 AM, Matthias Bolte wrote:
case VIR_ERR_NO_SUPPORT:
if (info == NULL)
Would it be practical to expand this with category messages like?
case VIR_ERR_NO_SUPPORT_STORAGE: if (info == NULL) errmsg = _("this function is not supported by the storage driver"); ...
case VIR_ERR_NO_SUPPORT_NETWORK: if (info == NULL) errmsg = _("this function is not supported by the network driver"); ...
Or maybe figure out a "%s" replaceable keyword for network/storage/hypervisor/etc above?
The whole error reporting scheme needs some loving care, and I've got a partial patch series posted to the list earlier that started that process. But it was deemed less important than getting 0.8.2 finalized (as well as helping with RHEL-6 backports), so it's been on the back burner. At this point, a minimally-invasive patch, like Daniel's proposal, is better than making things better when the code is all going to be refactored down the road anyways. But thanks for the thought; it may affect my refactoring efforts!
-- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (5)
-
Camilo Aguilar -
Daniel P. Berrange -
Eric Blake -
Justin Clift -
Matthias Bolte