4:39 a.m.
Error code VIR_ERR_NO_SUPPORT will be translated to "this function
is not supported by the connection driver", however, it's used
across the whole projects, this patch is trying to cleanup all
the improper use in the source tree.
The modification can be grouped to 3 following groups:
1) The error intends to tell user it's invalid operation.
s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID.
2) The error intends to tell the configuration of domain
is not supported.
s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/
3) The error intends to tell the function is not implemented
on some platform.
* s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
* and add error strings
e.g.
static int
lxcDomainInterfaceStats(virDomainPtr dom,
const char *path ATTRIBUTE_UNUSED,
struct _virDomainInterfaceStats *stats ATTRIBUTE_UNUSED)
-lxcError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+lxcError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("interface stats not implemented on this platform"));
return -1;
}
[PATCH 1/8] conf: Cleanup improper use of VIR_ERR_NO_SUPPORT in
[PATCH 2/8] lxc: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 3/8] nodeinfo: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 4/8] qemu: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 5/8] remote: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 6/8] storage: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 7/8] test: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 8/8] xen: Cleanup improper VIR_ERR_NO_SUPPORT use
Regards
Osier
4:39 a.m.
New subject: [libvirt] [PATCH 1/8] conf: Cleanup improper use of VIR_ERR_NO_SUPPORT in node_device_conf
---
src/conf/node_device_conf.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 548bbff..4803e2a 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1277,7 +1277,7 @@ virNodeDeviceGetWWNs(virNodeDeviceDefPtr def,
}
if (cap == NULL) {
- virNodeDeviceReportError(VIR_ERR_NO_SUPPORT,
+ virNodeDeviceReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("Device is not a fibre channel
HBA"));
ret = -1;
} else if (*wwnn == NULL || *wwpn == NULL) {
--
1.7.6
4:50 p.m.
New subject: [libvirt] [PATCH 1/8] conf: Cleanup improper use of VIR_ERR_NO_SUPPORT in node_device_conf
On Tue, Aug 23, 2011 at 05:39:38PM +0800, Osier Yang wrote:
---
src/conf/node_device_conf.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 548bbff..4803e2a 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1277,7 +1277,7 @@ virNodeDeviceGetWWNs(virNodeDeviceDefPtr def,
}
if (cap == NULL) {
- virNodeDeviceReportError(VIR_ERR_NO_SUPPORT,
+ virNodeDeviceReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("Device is not a fibre channel
HBA"));
ret = -1;
} else if (*wwnn == NULL || *wwpn == NULL) {
I think either CONFIG_SUPPORTED or just INTERNAL_ERROR is best
here.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
1:15 a.m.
New subject: [libvirt] [PATCH] conf: Substitute OPERATION_INVALID with INTERNAL_ERROR
---
src/conf/node_device_conf.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 4803e2a..6b0ef50 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1277,7 +1277,7 @@ virNodeDeviceGetWWNs(virNodeDeviceDefPtr def,
}
if (cap == NULL) {
- virNodeDeviceReportError(VIR_ERR_OPERATION_INVALID,
+ virNodeDeviceReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("Device is not a fibre channel
HBA"));
ret = -1;
} else if (*wwnn == NULL || *wwpn == NULL) {
--
1.7.6
4:06 a.m.
New subject: [libvirt] [PATCH] conf: Substitute OPERATION_INVALID with INTERNAL_ERROR
On Thu, Sep 01, 2011 at 02:15:54PM +0800, Osier Yang wrote:
---
src/conf/node_device_conf.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 4803e2a..6b0ef50 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1277,7 +1277,7 @@ virNodeDeviceGetWWNs(virNodeDeviceDefPtr def,
}
if (cap == NULL) {
- virNodeDeviceReportError(VIR_ERR_OPERATION_INVALID,
+ virNodeDeviceReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("Device is not a fibre channel
HBA"));
ret = -1;
} else if (*wwnn == NULL || *wwpn == NULL) {
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:39 a.m.
New subject: [libvirt] [PATCH 2/8] lxc: Cleanup improper VIR_ERR_NO_SUPPORT use
s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
Special case is changes on lxcDomainInterfaceStats, if it's not
implemented on the platform, prints error like:
lxcError(VIR_ERR_OPERATION_INVALID, "%s",
_("interface stats not implemented on this platform"));
As the function is supported by driver actually, error like
VIR_ERR_NO_SUPPORT is confused.
---
src/lxc/lxc_driver.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index a596945..5587b06 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -421,7 +421,7 @@ static virDomainPtr lxcDomainDefine(virConnectPtr conn, const char
*xml)
goto cleanup;
if ((def->nets != NULL) && !(driver->have_netns)) {
- lxcError(VIR_ERR_NO_SUPPORT,
+ lxcError(VIR_ERR_OPERATION_INVALID,
"%s", _("System lacks NETNS support"));
goto cleanup;
}
@@ -728,7 +728,7 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem)
{
}
if (driver->cgroup == NULL) {
- lxcError(VIR_ERR_NO_SUPPORT,
+ lxcError(VIR_ERR_OPERATION_INVALID,
"%s", _("cgroups must be configured on the host"));
goto cleanup;
}
@@ -1710,7 +1710,7 @@ static int lxcDomainStartWithFlags(virDomainPtr dom, unsigned int
flags)
}
if ((vm->def->nets != NULL) && !(driver->have_netns)) {
- lxcError(VIR_ERR_NO_SUPPORT,
+ lxcError(VIR_ERR_OPERATION_INVALID,
"%s", _("System lacks NETNS support"));
goto cleanup;
}
@@ -1786,7 +1786,7 @@ lxcDomainCreateAndStart(virConnectPtr conn,
goto cleanup;
if ((def->nets != NULL) && !(driver->have_netns)) {
- lxcError(VIR_ERR_NO_SUPPORT,
+ lxcError(VIR_ERR_OPERATION_INVALID,
"%s", _("System lacks NETNS support"));
goto cleanup;
}
@@ -2519,7 +2519,8 @@ static int
lxcDomainInterfaceStats(virDomainPtr dom,
const char *path ATTRIBUTE_UNUSED,
struct _virDomainInterfaceStats *stats ATTRIBUTE_UNUSED)
- lxcError(VIR_ERR_NO_SUPPORT, "%s", __FUNCTION__);
+ lxcError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("interface stats not implemented on this platform"));
return -1;
}
#endif
--
1.7.6
4:48 p.m.
New subject: [libvirt] [PATCH 2/8] lxc: Cleanup improper VIR_ERR_NO_SUPPORT use
On Tue, Aug 23, 2011 at 05:39:39PM +0800, Osier Yang wrote:
s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
Special case is changes on lxcDomainInterfaceStats, if it's not
implemented on the platform, prints error like:
lxcError(VIR_ERR_OPERATION_INVALID, "%s",
_("interface stats not implemented on this platform"));
As the function is supported by driver actually, error like
VIR_ERR_NO_SUPPORT is confused.
---
src/lxc/lxc_driver.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index a596945..5587b06 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -421,7 +421,7 @@ static virDomainPtr lxcDomainDefine(virConnectPtr conn, const char
*xml)
goto cleanup;
if ((def->nets != NULL) && !(driver->have_netns)) {
- lxcError(VIR_ERR_NO_SUPPORT,
+ lxcError(VIR_ERR_OPERATION_INVALID,
"%s", _("System lacks NETNS support"));
goto cleanup;
}
@@ -728,7 +728,7 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned long newmem)
{
}
if (driver->cgroup == NULL) {
- lxcError(VIR_ERR_NO_SUPPORT,
+ lxcError(VIR_ERR_OPERATION_INVALID,
"%s", _("cgroups must be configured on the
host"));
goto cleanup;
}
@@ -1710,7 +1710,7 @@ static int lxcDomainStartWithFlags(virDomainPtr dom, unsigned int
flags)
}
if ((vm->def->nets != NULL) && !(driver->have_netns)) {
- lxcError(VIR_ERR_NO_SUPPORT,
+ lxcError(VIR_ERR_OPERATION_INVALID,
"%s", _("System lacks NETNS support"));
goto cleanup;
}
@@ -1786,7 +1786,7 @@ lxcDomainCreateAndStart(virConnectPtr conn,
goto cleanup;
if ((def->nets != NULL) && !(driver->have_netns)) {
- lxcError(VIR_ERR_NO_SUPPORT,
+ lxcError(VIR_ERR_OPERATION_INVALID,
"%s", _("System lacks NETNS support"));
goto cleanup;
}
These should be CONFIG_UNSUPPORTED
@@ -2519,7 +2519,8 @@ static int
lxcDomainInterfaceStats(virDomainPtr dom,
const char *path ATTRIBUTE_UNUSED,
struct _virDomainInterfaceStats *stats ATTRIBUTE_UNUSED)
- lxcError(VIR_ERR_NO_SUPPORT, "%s", __FUNCTION__);
+ lxcError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("interface stats not implemented on this platform"));
return -1;
}
#endif
NO_SUPPORT was correct here.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
1:28 a.m.
New subject: [libvirt] [PATCH] lxc: Fix incorrect changes on error codes.
Fix incorrect changes introduced by commit 6ac47762bb9.
---
src/lxc/lxc_driver.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 5587b06..6b716c1 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1786,7 +1786,7 @@ lxcDomainCreateAndStart(virConnectPtr conn,
goto cleanup;
if ((def->nets != NULL) && !(driver->have_netns)) {
- lxcError(VIR_ERR_OPERATION_INVALID,
+ lxcError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("System lacks NETNS support"));
goto cleanup;
}
@@ -2519,8 +2519,7 @@ static int
lxcDomainInterfaceStats(virDomainPtr dom,
const char *path ATTRIBUTE_UNUSED,
struct _virDomainInterfaceStats *stats ATTRIBUTE_UNUSED)
- lxcError(VIR_ERR_OPERATION_INVALID, "%s",
- _("interface stats not implemented on this platform"));
+ lxcError(VIR_ERR_NO_SUPPORT, "%s", __FUNCTION__);
return -1;
}
#endif
--
1.7.6
4:06 a.m.
New subject: [libvirt] [PATCH] lxc: Fix incorrect changes on error codes.
On Thu, Sep 01, 2011 at 02:28:12PM +0800, Osier Yang wrote:
Fix incorrect changes introduced by commit 6ac47762bb9.
---
src/lxc/lxc_driver.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 5587b06..6b716c1 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1786,7 +1786,7 @@ lxcDomainCreateAndStart(virConnectPtr conn,
goto cleanup;
if ((def->nets != NULL) && !(driver->have_netns)) {
- lxcError(VIR_ERR_OPERATION_INVALID,
+ lxcError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("System lacks NETNS support"));
goto cleanup;
}
@@ -2519,8 +2519,7 @@ static int
lxcDomainInterfaceStats(virDomainPtr dom,
const char *path ATTRIBUTE_UNUSED,
struct _virDomainInterfaceStats *stats ATTRIBUTE_UNUSED)
- lxcError(VIR_ERR_OPERATION_INVALID, "%s",
- _("interface stats not implemented on this platform"));
+ lxcError(VIR_ERR_NO_SUPPORT, "%s", __FUNCTION__);
return -1;
}
#endif
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:39 a.m.
New subject: [libvirt] [PATCH 3/8] nodeinfo: Cleanup improper VIR_ERR_NO_SUPPORT use
---
src/nodeinfo.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index cae2564..b21e555 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -629,7 +629,7 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr
nodeinfo) {
}
#else
/* XXX Solaris will need an impl later if they port QEMU driver */
- nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
+ nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("node info not implemented on this platform"));
return -1;
#endif
@@ -658,7 +658,7 @@ int nodeGetCPUStats(virConnectPtr conn ATTRIBUTE_UNUSED,
return ret;
}
#else
- nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
+ nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("node CPU stats not implemented on this platform"));
return -1;
#endif
@@ -688,7 +688,7 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED,
# if HAVE_NUMACTL
if (numa_available() < 0) {
# endif
- nodeReportError(VIR_ERR_NO_SUPPORT,
+ nodeReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("NUMA not supported on this
host"));
return -1;
# if HAVE_NUMACTL
@@ -724,7 +724,7 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED,
return ret;
}
#else
- nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
+ nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("node memory stats not implemented on this platform"));
return -1;
#endif
@@ -818,7 +818,7 @@ nodeGetCellsFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED,
int maxCell;
if (numa_available() < 0) {
- nodeReportError(VIR_ERR_NO_SUPPORT,
+ nodeReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("NUMA not supported on this host"));
goto cleanup;
}
@@ -856,7 +856,7 @@ nodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED)
int n;
if (numa_available() < 0) {
- nodeReportError(VIR_ERR_NO_SUPPORT,
+ nodeReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("NUMA not supported on this host"));
goto cleanup;
}
@@ -885,14 +885,14 @@ int nodeGetCellsFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED,
int startCell ATTRIBUTE_UNUSED,
int maxCells ATTRIBUTE_UNUSED)
{
- nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
+ nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("NUMA memory information not available on this
platform"));
return -1;
}
unsigned long long nodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED)
{
- nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
+ nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("NUMA memory information not available on this
platform"));
return 0;
}
--
1.7.6
4:48 p.m.
New subject: [libvirt] [PATCH 3/8] nodeinfo: Cleanup improper VIR_ERR_NO_SUPPORT use
On Tue, Aug 23, 2011 at 05:39:40PM +0800, Osier Yang wrote:
---
src/nodeinfo.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index cae2564..b21e555 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -629,7 +629,7 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr
nodeinfo) {
}
#else
/* XXX Solaris will need an impl later if they port QEMU driver */
- nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
+ nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("node info not implemented on this platform"));
return -1;
#endif
@@ -658,7 +658,7 @@ int nodeGetCPUStats(virConnectPtr conn ATTRIBUTE_UNUSED,
return ret;
}
#else
- nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
+ nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("node CPU stats not implemented on this platform"));
return -1;
#endif
@@ -688,7 +688,7 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED,
# if HAVE_NUMACTL
if (numa_available() < 0) {
# endif
- nodeReportError(VIR_ERR_NO_SUPPORT,
+ nodeReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("NUMA not supported on this
host"));
return -1;
# if HAVE_NUMACTL
@@ -724,7 +724,7 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED,
return ret;
}
#else
- nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
+ nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("node memory stats not implemented on this platform"));
return -1;
#endif
@@ -818,7 +818,7 @@ nodeGetCellsFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED,
int maxCell;
if (numa_available() < 0) {
- nodeReportError(VIR_ERR_NO_SUPPORT,
+ nodeReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("NUMA not supported on this
host"));
goto cleanup;
}
@@ -856,7 +856,7 @@ nodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED)
int n;
if (numa_available() < 0) {
- nodeReportError(VIR_ERR_NO_SUPPORT,
+ nodeReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("NUMA not supported on this
host"));
goto cleanup;
}
@@ -885,14 +885,14 @@ int nodeGetCellsFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED,
int startCell ATTRIBUTE_UNUSED,
int maxCells ATTRIBUTE_UNUSED)
{
- nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
+ nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("NUMA memory information not available on this
platform"));
return -1;
}
unsigned long long nodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED)
{
- nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
+ nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("NUMA memory information not available on this
platform"));
return 0;
}
All these changes are wrong. NO_SUPPORT is correct for cases where the
driver method is simply stubbed out on an architecture.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
1:42 a.m.
New subject: [libvirt] [PATCH 3/8] nodeinfo: Cleanup improper VIR_ERR_NO_SUPPORT use
于 2011年08月25日 05:48, Daniel P. Berrange 写道:
On Tue, Aug 23, 2011 at 05:39:40PM +0800, Osier Yang wrote:
> ---
> src/nodeinfo.c | 16 ++++++++--------
> 1 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/src/nodeinfo.c b/src/nodeinfo.c
> index cae2564..b21e555 100644
> --- a/src/nodeinfo.c
> +++ b/src/nodeinfo.c
> @@ -629,7 +629,7 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED,
virNodeInfoPtr nodeinfo) {
> }
> #else
> /* XXX Solaris will need an impl later if they port QEMU driver */
> - nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
> + nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
> _("node info not implemented on this platform"));
> return -1;
> #endif
> @@ -658,7 +658,7 @@ int nodeGetCPUStats(virConnectPtr conn ATTRIBUTE_UNUSED,
> return ret;
> }
> #else
> - nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
> + nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
> _("node CPU stats not implemented on this
platform"));
> return -1;
> #endif
> @@ -688,7 +688,7 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED,
> # if HAVE_NUMACTL
> if (numa_available()< 0) {
> # endif
> - nodeReportError(VIR_ERR_NO_SUPPORT,
> + nodeReportError(VIR_ERR_OPERATION_INVALID,
Hi, Daniel
I think VIR_ERR_NO_SUPPORT is not correct, as the function is neither a
no-op stub
and not implemented. But per your explaination on [patch 0/8],
OPERATION_INVALID
is not proper here too. Change it into INTERNAL_ERROR.
> "%s", _("NUMA
not supported on this host"));
> return -1;
> # if HAVE_NUMACTL
> @@ -724,7 +724,7 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED,
> return ret;
> }
> #else
> - nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
> + nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
> _("node memory stats not implemented on this
platform"));
> return -1;
> #endif
> @@ -818,7 +818,7 @@ nodeGetCellsFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED,
> int maxCell;
>
> if (numa_available()< 0) {
> - nodeReportError(VIR_ERR_NO_SUPPORT,
> + nodeReportError(VIR_ERR_OPERATION_INVALID,
likewise
> "%s", _("NUMA not
supported on this host"));
> goto cleanup;
> }
> @@ -856,7 +856,7 @@ nodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED)
> int n;
>
> if (numa_available()< 0) {
> - nodeReportError(VIR_ERR_NO_SUPPORT,
likewise
> + nodeReportError(VIR_ERR_OPERATION_INVALID,
> "%s", _("NUMA not supported on this
host"));
> goto cleanup;
> }
> @@ -885,14 +885,14 @@ int nodeGetCellsFreeMemory(virConnectPtr conn
ATTRIBUTE_UNUSED,
> int startCell ATTRIBUTE_UNUSED,
> int maxCells ATTRIBUTE_UNUSED)
> {
> - nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
> + nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
> _("NUMA memory information not available on this
platform"));
> return -1;
> }
>
> unsigned long long nodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED)
> {
> - nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
> + nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
> _("NUMA memory information not available on this
platform"));
> return 0;
> }
All these changes are wrong. NO_SUPPORT is correct for cases where the
driver method is simply stubbed out on an architecture.
All other VIR_ERR_NO_SUPPORT are changed back. Patch following.
Osier
4:13 a.m.
New subject: [libvirt] [PATCH 3/8] nodeinfo: Cleanup improper VIR_ERR_NO_SUPPORT use
On Thu, Sep 01, 2011 at 02:42:51PM +0800, Osier Yang wrote:
于 2011年08月25日 05:48, Daniel P. Berrange 写道:
>On Tue, Aug 23, 2011 at 05:39:40PM +0800, Osier Yang wrote:
>>---
>> src/nodeinfo.c | 16 ++++++++--------
>> 1 files changed, 8 insertions(+), 8 deletions(-)
>>
>>diff --git a/src/nodeinfo.c b/src/nodeinfo.c
>>index cae2564..b21e555 100644
>>--- a/src/nodeinfo.c
>>+++ b/src/nodeinfo.c
>>@@ -629,7 +629,7 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED,
virNodeInfoPtr nodeinfo) {
>> }
>> #else
>> /* XXX Solaris will need an impl later if they port QEMU driver */
>>- nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
>>+ nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> _("node info not implemented on this
platform"));
>> return -1;
>> #endif
>>@@ -658,7 +658,7 @@ int nodeGetCPUStats(virConnectPtr conn ATTRIBUTE_UNUSED,
>> return ret;
>> }
>> #else
>>- nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
>>+ nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> _("node CPU stats not implemented on this
platform"));
>> return -1;
>> #endif
>>@@ -688,7 +688,7 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED,
>> # if HAVE_NUMACTL
>> if (numa_available()< 0) {
>> # endif
>>- nodeReportError(VIR_ERR_NO_SUPPORT,
>>+ nodeReportError(VIR_ERR_OPERATION_INVALID,
Hi, Daniel
I think VIR_ERR_NO_SUPPORT is not correct, as the function is
neither a no-op stub
and not implemented. But per your explaination on [patch 0/8],
OPERATION_INVALID
is not proper here too. Change it into INTERNAL_ERROR.
Yeah, ok.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
1:45 a.m.
New subject: [libvirt] [PATCH] nodeinfo: Fix incorrect error codes
Introduced by 5e495c8b, except the ones for checking if numa
is supported by host, all the NO_SUPPORT are changed back. For
the ones about numa checking, change them into INTERNAL_ERROR.
---
src/nodeinfo.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index b21e555..6448b79 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -629,7 +629,7 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr
nodeinfo) {
}
#else
/* XXX Solaris will need an impl later if they port QEMU driver */
- nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
_("node info not implemented on this platform"));
return -1;
#endif
@@ -658,7 +658,7 @@ int nodeGetCPUStats(virConnectPtr conn ATTRIBUTE_UNUSED,
return ret;
}
#else
- nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
_("node CPU stats not implemented on this platform"));
return -1;
#endif
@@ -688,7 +688,7 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED,
# if HAVE_NUMACTL
if (numa_available() < 0) {
# endif
- nodeReportError(VIR_ERR_OPERATION_INVALID,
+ nodeReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("NUMA not supported on this
host"));
return -1;
# if HAVE_NUMACTL
@@ -724,7 +724,7 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED,
return ret;
}
#else
- nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
_("node memory stats not implemented on this platform"));
return -1;
#endif
@@ -818,7 +818,7 @@ nodeGetCellsFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED,
int maxCell;
if (numa_available() < 0) {
- nodeReportError(VIR_ERR_OPERATION_INVALID,
+ nodeReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("NUMA not supported on this host"));
goto cleanup;
}
@@ -856,7 +856,7 @@ nodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED)
int n;
if (numa_available() < 0) {
- nodeReportError(VIR_ERR_OPERATION_INVALID,
+ nodeReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("NUMA not supported on this host"));
goto cleanup;
}
@@ -885,14 +885,14 @@ int nodeGetCellsFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED,
int startCell ATTRIBUTE_UNUSED,
int maxCells ATTRIBUTE_UNUSED)
{
- nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
_("NUMA memory information not available on this
platform"));
return -1;
}
unsigned long long nodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED)
{
- nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
_("NUMA memory information not available on this
platform"));
return 0;
}
--
1.7.6
4:12 a.m.
New subject: [libvirt] [PATCH] nodeinfo: Fix incorrect error codes
On Thu, Sep 01, 2011 at 02:45:31PM +0800, Osier Yang wrote:
Introduced by 5e495c8b, except the ones for checking if numa
is supported by host, all the NO_SUPPORT are changed back. For
the ones about numa checking, change them into INTERNAL_ERROR.
---
src/nodeinfo.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index b21e555..6448b79 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -629,7 +629,7 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr
nodeinfo) {
}
#else
/* XXX Solaris will need an impl later if they port QEMU driver */
- nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
_("node info not implemented on this platform"));
return -1;
#endif
@@ -658,7 +658,7 @@ int nodeGetCPUStats(virConnectPtr conn ATTRIBUTE_UNUSED,
return ret;
}
#else
- nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
_("node CPU stats not implemented on this platform"));
return -1;
#endif
@@ -688,7 +688,7 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED,
# if HAVE_NUMACTL
if (numa_available() < 0) {
# endif
- nodeReportError(VIR_ERR_OPERATION_INVALID,
+ nodeReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("NUMA not supported on this
host"));
return -1;
# if HAVE_NUMACTL
@@ -724,7 +724,7 @@ int nodeGetMemoryStats(virConnectPtr conn ATTRIBUTE_UNUSED,
return ret;
}
#else
- nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
_("node memory stats not implemented on this platform"));
return -1;
#endif
@@ -818,7 +818,7 @@ nodeGetCellsFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED,
int maxCell;
if (numa_available() < 0) {
- nodeReportError(VIR_ERR_OPERATION_INVALID,
+ nodeReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("NUMA not supported on this
host"));
goto cleanup;
}
@@ -856,7 +856,7 @@ nodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED)
int n;
if (numa_available() < 0) {
- nodeReportError(VIR_ERR_OPERATION_INVALID,
+ nodeReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("NUMA not supported on this
host"));
goto cleanup;
}
@@ -885,14 +885,14 @@ int nodeGetCellsFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED,
int startCell ATTRIBUTE_UNUSED,
int maxCells ATTRIBUTE_UNUSED)
{
- nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
_("NUMA memory information not available on this
platform"));
return -1;
}
unsigned long long nodeGetFreeMemory(virConnectPtr conn ATTRIBUTE_UNUSED)
{
- nodeReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ nodeReportError(VIR_ERR_NO_SUPPORT, "%s",
_("NUMA memory information not available on this
platform"));
return 0;
}
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:39 a.m.
New subject: [libvirt] [PATCH 4/8] qemu: Cleanup improper VIR_ERR_NO_SUPPORT use
* src/qemu/qemu_command.c: s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/
* src/qemu/qemu_driver.c: s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
* src/qemu/qemu_process.c: s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
---
src/qemu/qemu_command.c | 4 ++--
src/qemu/qemu_driver.c | 16 ++++++++--------
src/qemu/qemu_process.c | 4 ++--
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index dbfc7d9..287ad8d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4084,7 +4084,7 @@ qemuBuildCommandLine(virConnectPtr conn,
switch(console->targetType) {
case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO:
if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
- qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("virtio channel requires QEMU to support -device"));
goto error;
}
@@ -4109,7 +4109,7 @@ qemuBuildCommandLine(virConnectPtr conn,
break;
default:
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported console target type %s"),
NULLSTR(virDomainChrConsoleTargetTypeToString(console->targetType)));
goto error;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c8dda73..fc2538a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1552,7 +1552,7 @@ static int qemuDomainReboot(virDomainPtr dom, unsigned int flags) {
vm = NULL;
} else {
#endif
- qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+ qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("Reboot is not supported without the JSON
monitor"));
#if HAVE_YAJL
}
@@ -3309,7 +3309,7 @@ qemudDomainPinVcpuFlags(virDomainPtr dom,
cpumap, maplen, maxcpu) < 0)
goto cleanup;
} else {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("cpu affinity is not
supported"));
goto cleanup;
}
@@ -3563,7 +3563,7 @@ qemudDomainGetVcpus(virDomainPtr dom,
goto cleanup;
}
} else {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("cpu affinity is not
available"));
goto cleanup;
}
@@ -5637,7 +5637,7 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom,
}
if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
- qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup isn't
mounted"));
+ qemuReportError(VIR_ERR_OPERATION_INVALID, _("blkio cgroup isn't
mounted"));
goto cleanup;
}
@@ -5790,7 +5790,7 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
}
if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
- qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup isn't
mounted"));
+ qemuReportError(VIR_ERR_OPERATION_INVALID, _("blkio cgroup isn't
mounted"));
goto cleanup;
}
@@ -6887,8 +6887,8 @@ qemudDomainInterfaceStats (virDomainPtr dom,
const char *path ATTRIBUTE_UNUSED,
struct _virDomainInterfaceStats *stats ATTRIBUTE_UNUSED)
{
- qemuReportError(VIR_ERR_NO_SUPPORT,
- "%s", __FUNCTION__);
+ qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("interface stats not implemented on this platform"));
return -1;
}
#endif
@@ -8004,7 +8004,7 @@ qemuCPUCompare(virConnectPtr conn,
qemuDriverLock(driver);
if (!driver->caps || !driver->caps->host.cpu) {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("cannot get host CPU
capabilities"));
} else {
ret = cpuCompareXML(driver->caps->host.cpu, xmlDesc);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6f54b30..616c8e2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -261,7 +261,7 @@ qemuProcessGetVolumeQcowPassphrase(virConnectPtr conn,
if (conn->secretDriver == NULL ||
conn->secretDriver->lookupByUUID == NULL ||
conn->secretDriver->getValue == NULL) {
- qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+ qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("secret storage not supported"));
goto cleanup;
}
@@ -1464,7 +1464,7 @@ qemuProcessSetVcpuAffinites(virConnectPtr conn,
return 0;
if (priv->vcpupids == NULL) {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("cpu affinity is not supported"));
return -1;
}
--
1.7.6
4:47 p.m.
New subject: [libvirt] [PATCH 4/8] qemu: Cleanup improper VIR_ERR_NO_SUPPORT use
On Tue, Aug 23, 2011 at 05:39:41PM +0800, Osier Yang wrote:
* src/qemu/qemu_command.c:
s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/
* src/qemu/qemu_driver.c: s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
* src/qemu/qemu_process.c: s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
---
src/qemu/qemu_command.c | 4 ++--
src/qemu/qemu_driver.c | 16 ++++++++--------
src/qemu/qemu_process.c | 4 ++--
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index dbfc7d9..287ad8d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4084,7 +4084,7 @@ qemuBuildCommandLine(virConnectPtr conn,
switch(console->targetType) {
case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO:
if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
- qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("virtio channel requires QEMU to support -device"));
goto error;
}
@@ -4109,7 +4109,7 @@ qemuBuildCommandLine(virConnectPtr conn,
break;
default:
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported console target type %s"),
NULLSTR(virDomainChrConsoleTargetTypeToString(console->targetType)));
goto error;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c8dda73..fc2538a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1552,7 +1552,7 @@ static int qemuDomainReboot(virDomainPtr dom, unsigned int flags)
{
vm = NULL;
} else {
#endif
- qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+ qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("Reboot is not supported without the JSON
monitor"));
#if HAVE_YAJL
}
@@ -3309,7 +3309,7 @@ qemudDomainPinVcpuFlags(virDomainPtr dom,
cpumap, maplen, maxcpu) < 0)
goto cleanup;
} else {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("cpu affinity is not
supported"));
goto cleanup;
}
@@ -3563,7 +3563,7 @@ qemudDomainGetVcpus(virDomainPtr dom,
goto cleanup;
}
} else {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("cpu affinity is not
available"));
goto cleanup;
}
@@ -5637,7 +5637,7 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom,
}
if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
- qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup isn't
mounted"));
+ qemuReportError(VIR_ERR_OPERATION_INVALID, _("blkio cgroup isn't
mounted"));
goto cleanup;
}
@@ -5790,7 +5790,7 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
}
if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
- qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup isn't
mounted"));
+ qemuReportError(VIR_ERR_OPERATION_INVALID, _("blkio cgroup isn't
mounted"));
goto cleanup;
}
THe use of VIR_ERR_OPERATION_INVALID is not correct here, but I'm not
certain what other error is best. Perhaps ARGUMENT_UNSUPPORTED
@@ -6887,8 +6887,8 @@ qemudDomainInterfaceStats (virDomainPtr dom,
const char *path ATTRIBUTE_UNUSED,
struct _virDomainInterfaceStats *stats ATTRIBUTE_UNUSED)
{
- qemuReportError(VIR_ERR_NO_SUPPORT,
- "%s", __FUNCTION__);
+ qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("interface stats not implemented on this platform"));
return -1;
}
#endif
The original code was correct here.
@@ -8004,7 +8004,7 @@ qemuCPUCompare(virConnectPtr conn,
qemuDriverLock(driver);
if (!driver->caps || !driver->caps->host.cpu) {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("cannot get host CPU
capabilities"));
} else {
ret = cpuCompareXML(driver->caps->host.cpu, xmlDesc);
The use of OPERATION_INVALID is not correct here. Perhaps
ARGUMENT_UNSUPPORTED
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6f54b30..616c8e2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -261,7 +261,7 @@ qemuProcessGetVolumeQcowPassphrase(virConnectPtr conn,
if (conn->secretDriver == NULL ||
conn->secretDriver->lookupByUUID == NULL ||
conn->secretDriver->getValue == NULL) {
- qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
+ qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("secret storage not supported"));
goto cleanup;
}
NO_SUPPORT was the right value here.
@@ -1464,7 +1464,7 @@ qemuProcessSetVcpuAffinites(virConnectPtr
conn,
return 0;
if (priv->vcpupids == NULL) {
- qemuReportError(VIR_ERR_NO_SUPPORT,
+ qemuReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("cpu affinity is not supported"));
return -1;
}
Perhaps ARGUMENT_UNSUPPORTED
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:02 a.m.
New subject: [libvirt] [PATCH 4/8] qemu: Cleanup improper VIR_ERR_NO_SUPPORT use
于 2011年08月25日 05:47, Daniel P. Berrange 写道:
On Tue, Aug 23, 2011 at 05:39:41PM +0800, Osier Yang wrote:
> * src/qemu/qemu_command.c: s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/
>
> * src/qemu/qemu_driver.c: s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
>
> * src/qemu/qemu_process.c: s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
> ---
> src/qemu/qemu_command.c | 4 ++--
> src/qemu/qemu_driver.c | 16 ++++++++--------
> src/qemu/qemu_process.c | 4 ++--
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index dbfc7d9..287ad8d 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4084,7 +4084,7 @@ qemuBuildCommandLine(virConnectPtr conn,
> switch(console->targetType) {
> case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO:
> if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
> - qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
> + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> _("virtio channel requires QEMU to support
-device"));
> goto error;
> }
> @@ -4109,7 +4109,7 @@ qemuBuildCommandLine(virConnectPtr conn,
> break;
>
> default:
> - qemuReportError(VIR_ERR_NO_SUPPORT,
> + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("unsupported console target type %s"),
>
NULLSTR(virDomainChrConsoleTargetTypeToString(console->targetType)));
> goto error;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index c8dda73..fc2538a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1552,7 +1552,7 @@ static int qemuDomainReboot(virDomainPtr dom, unsigned int
flags) {
> vm = NULL;
> } else {
> #endif
> - qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
> + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> _("Reboot is not supported without the JSON
monitor"));
> #if HAVE_YAJL
> }
> @@ -3309,7 +3309,7 @@ qemudDomainPinVcpuFlags(virDomainPtr dom,
> cpumap, maplen, maxcpu)< 0)
> goto cleanup;
> } else {
> - qemuReportError(VIR_ERR_NO_SUPPORT,
> + qemuReportError(VIR_ERR_OPERATION_INVALID,
> "%s", _("cpu affinity is not
supported"));
> goto cleanup;
> }
> @@ -3563,7 +3563,7 @@ qemudDomainGetVcpus(virDomainPtr dom,
> goto cleanup;
> }
> } else {
> - qemuReportError(VIR_ERR_NO_SUPPORT,
> + qemuReportError(VIR_ERR_OPERATION_INVALID,
> "%s", _("cpu affinity is not
available"));
> goto cleanup;
> }
> @@ -5637,7 +5637,7 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom,
> }
>
> if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
> - qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup isn't
mounted"));
> + qemuReportError(VIR_ERR_OPERATION_INVALID, _("blkio cgroup
isn't mounted"));
> goto cleanup;
> }
>
> @@ -5790,7 +5790,7 @@ static int qemuDomainGetBlkioParameters(virDomainPtr dom,
> }
>
> if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_BLKIO)) {
> - qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup isn't
mounted"));
> + qemuReportError(VIR_ERR_OPERATION_INVALID, _("blkio cgroup
isn't mounted"));
> goto cleanup;
> }
>
THe use of VIR_ERR_OPERATION_INVALID is not correct here, but I'm not
certain what other error is best. Perhaps ARGUMENT_UNSUPPORTED
For VIR_ERR_OPERATION_INVALID:
errmsg = _("Requested operation is not valid");
For VIR_ERR_ARGUMENT_UNSUPPORTED:
errmsg = _("argument unsupported");
From the user's point of view, I think OPERATION_INVALID is more clear
and sensiable. E.g.
"Requested operation is not valid: blkio cgroup isn't mounted"
"argument unsupported: blkio cgroup isn't mounted"
Could we just extend the meaning for OPERATION_INVALID
so that it's not just used when the object operated on is not in
correct state, and used for things like above?
Otherwise, I'd prefer INTERNAL_ERROR here.
> @@ -6887,8 +6887,8 @@ qemudDomainInterfaceStats (virDomainPtr dom,
> const char *path ATTRIBUTE_UNUSED,
> struct _virDomainInterfaceStats *stats
ATTRIBUTE_UNUSED)
> {
> - qemuReportError(VIR_ERR_NO_SUPPORT,
> - "%s", __FUNCTION__);
> + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("interface stats not implemented on this
platform"));
> return -1;
> }
> #endif
The original code was correct here.
> @@ -8004,7 +8004,7 @@ qemuCPUCompare(virConnectPtr conn,
> qemuDriverLock(driver);
>
> if (!driver->caps || !driver->caps->host.cpu) {
> - qemuReportError(VIR_ERR_NO_SUPPORT,
> + qemuReportError(VIR_ERR_OPERATION_INVALID,
> "%s", _("cannot get host CPU
capabilities"));
> } else {
> ret = cpuCompareXML(driver->caps->host.cpu, xmlDesc);
The use of OPERATION_INVALID is not correct here. Perhaps
ARGUMENT_UNSUPPORTED
perhaps INTERNAL_ERROR is better here, as translated error string
for ARGUMENT_UNSUPPORTED is quite confused.
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 6f54b30..616c8e2 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -261,7 +261,7 @@ qemuProcessGetVolumeQcowPassphrase(virConnectPtr conn,
> if (conn->secretDriver == NULL ||
> conn->secretDriver->lookupByUUID == NULL ||
> conn->secretDriver->getValue == NULL) {
> - qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
> + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> _("secret storage not supported"));
> goto cleanup;
> }
NO_SUPPORT was the right value here.
> @@ -1464,7 +1464,7 @@ qemuProcessSetVcpuAffinites(virConnectPtr conn,
> return 0;
>
> if (priv->vcpupids == NULL) {
> - qemuReportError(VIR_ERR_NO_SUPPORT,
> + qemuReportError(VIR_ERR_OPERATION_INVALID,
> "%s", _("cpu affinity is not
supported"));
> return -1;
> }
Perhaps ARGUMENT_UNSUPPORTED
Perhaps INTERNAL_ERROR?
Daniel
3:04 a.m.
New subject: [libvirt] [PATCH 4/8] qemu: Cleanup improper VIR_ERR_NO_SUPPORT use
2011/9/1 Osier Yang <jyang(a)redhat.com>:
于 2011年08月25日 05:47, Daniel P. Berrange 写道:
>
> On Tue, Aug 23, 2011 at 05:39:41PM +0800, Osier Yang wrote:
>>
>> * src/qemu/qemu_command.c:
>> s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/
>>
>> * src/qemu/qemu_driver.c: s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
>>
>> * src/qemu/qemu_process.c:
>> s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
>> ---
>> src/qemu/qemu_command.c | 4 ++--
>> src/qemu/qemu_driver.c | 16 ++++++++--------
>> src/qemu/qemu_process.c | 4 ++--
>> 3 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index dbfc7d9..287ad8d 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -4084,7 +4084,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>> switch(console->targetType) {
>> case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO:
>> if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
>> - qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
>> + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> _("virtio channel requires QEMU to support
>> -device"));
>> goto error;
>> }
>> @@ -4109,7 +4109,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>> break;
>>
>> default:
>> - qemuReportError(VIR_ERR_NO_SUPPORT,
>> + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> _("unsupported console target type %s"),
>>
>> NULLSTR(virDomainChrConsoleTargetTypeToString(console->targetType)));
>> goto error;
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index c8dda73..fc2538a 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -1552,7 +1552,7 @@ static int qemuDomainReboot(virDomainPtr dom,
>> unsigned int flags) {
>> vm = NULL;
>> } else {
>> #endif
>> - qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
>> + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> _("Reboot is not supported without the JSON
>> monitor"));
>> #if HAVE_YAJL
>> }
>> @@ -3309,7 +3309,7 @@ qemudDomainPinVcpuFlags(virDomainPtr dom,
>> cpumap, maplen, maxcpu)< 0)
>> goto cleanup;
>> } else {
>> - qemuReportError(VIR_ERR_NO_SUPPORT,
>> + qemuReportError(VIR_ERR_OPERATION_INVALID,
>> "%s", _("cpu affinity is not
supported"));
>> goto cleanup;
>> }
>> @@ -3563,7 +3563,7 @@ qemudDomainGetVcpus(virDomainPtr dom,
>> goto cleanup;
>> }
>> } else {
>> - qemuReportError(VIR_ERR_NO_SUPPORT,
>> + qemuReportError(VIR_ERR_OPERATION_INVALID,
>> "%s", _("cpu affinity is not
>> available"));
>> goto cleanup;
>> }
>> @@ -5637,7 +5637,7 @@ static int
>> qemuDomainSetBlkioParameters(virDomainPtr dom,
>> }
>>
>> if (!qemuCgroupControllerActive(driver,
>> VIR_CGROUP_CONTROLLER_BLKIO)) {
>> - qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup isn't
>> mounted"));
>> + qemuReportError(VIR_ERR_OPERATION_INVALID, _("blkio cgroup
>> isn't mounted"));
>> goto cleanup;
>> }
>>
>> @@ -5790,7 +5790,7 @@ static int
>> qemuDomainGetBlkioParameters(virDomainPtr dom,
>> }
>>
>> if (!qemuCgroupControllerActive(driver,
>> VIR_CGROUP_CONTROLLER_BLKIO)) {
>> - qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup isn't
>> mounted"));
>> + qemuReportError(VIR_ERR_OPERATION_INVALID, _("blkio cgroup
>> isn't mounted"));
>> goto cleanup;
>> }
>>
> THe use of VIR_ERR_OPERATION_INVALID is not correct here, but I'm not
> certain what other error is best. Perhaps ARGUMENT_UNSUPPORTED
For VIR_ERR_OPERATION_INVALID:
errmsg = _("Requested operation is not valid");
For VIR_ERR_ARGUMENT_UNSUPPORTED:
errmsg = _("argument unsupported");
From the user's point of view, I think OPERATION_INVALID is more clear
and sensiable. E.g.
"Requested operation is not valid: blkio cgroup isn't mounted"
"argument unsupported: blkio cgroup isn't mounted"
Could we just extend the meaning for OPERATION_INVALID
so that it's not just used when the object operated on is not in
correct state, and used for things like above?
Otherwise, I'd prefer INTERNAL_ERROR here.
Do we have documented in detail somewhere when this error codes in
question here are to be used?
For example what's the difference between VIR_ERR_OPERATION_DENIED and
VIR_ERR_OPERATION_INVALID. VIR_ERR_OPERATION_DENIED is mostly used in
libvirt.c in case of a read-only connection, but it's also used in the
xen, secret and openvz drivers. In this drivers probably
VIR_ERR_OPERATION_INVALID was meant.
Anyway, I think we need documentation about in which situations what
error codes are applicable and what their intended meaning is. We
might also need to adjust the assigned error messages to improve error
reporting.
--
Matthias Bolte
http://photron.blogspot.com
3:25 a.m.
New subject: [libvirt] [PATCH 4/8] qemu: Cleanup improper VIR_ERR_NO_SUPPORT use
于 2011年09月01日 16:04, Matthias Bolte 写道:
2011/9/1 Osier Yang<jyang(a)redhat.com>:
> 于 2011年08月25日 05:47, Daniel P. Berrange 写道:
>> On Tue, Aug 23, 2011 at 05:39:41PM +0800, Osier Yang wrote:
>>> * src/qemu/qemu_command.c:
>>> s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/
>>>
>>> * src/qemu/qemu_driver.c: s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
>>>
>>> * src/qemu/qemu_process.c:
>>> s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
>>> ---
>>> src/qemu/qemu_command.c | 4 ++--
>>> src/qemu/qemu_driver.c | 16 ++++++++--------
>>> src/qemu/qemu_process.c | 4 ++--
>>> 3 files changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>>> index dbfc7d9..287ad8d 100644
>>> --- a/src/qemu/qemu_command.c
>>> +++ b/src/qemu/qemu_command.c
>>> @@ -4084,7 +4084,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>>> switch(console->targetType) {
>>> case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO:
>>> if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
>>> - qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
>>> + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>> _("virtio channel requires QEMU to support
>>> -device"));
>>> goto error;
>>> }
>>> @@ -4109,7 +4109,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>>> break;
>>>
>>> default:
>>> - qemuReportError(VIR_ERR_NO_SUPPORT,
>>> + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>> _("unsupported console target type
%s"),
>>>
>>> NULLSTR(virDomainChrConsoleTargetTypeToString(console->targetType)));
>>> goto error;
>>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>>> index c8dda73..fc2538a 100644
>>> --- a/src/qemu/qemu_driver.c
>>> +++ b/src/qemu/qemu_driver.c
>>> @@ -1552,7 +1552,7 @@ static int qemuDomainReboot(virDomainPtr dom,
>>> unsigned int flags) {
>>> vm = NULL;
>>> } else {
>>> #endif
>>> - qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
>>> + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
>>> _("Reboot is not supported without the JSON
>>> monitor"));
>>> #if HAVE_YAJL
>>> }
>>> @@ -3309,7 +3309,7 @@ qemudDomainPinVcpuFlags(virDomainPtr dom,
>>> cpumap, maplen, maxcpu)<
0)
>>> goto cleanup;
>>> } else {
>>> - qemuReportError(VIR_ERR_NO_SUPPORT,
>>> + qemuReportError(VIR_ERR_OPERATION_INVALID,
>>> "%s", _("cpu affinity is not
supported"));
>>> goto cleanup;
>>> }
>>> @@ -3563,7 +3563,7 @@ qemudDomainGetVcpus(virDomainPtr dom,
>>> goto cleanup;
>>> }
>>> } else {
>>> - qemuReportError(VIR_ERR_NO_SUPPORT,
>>> + qemuReportError(VIR_ERR_OPERATION_INVALID,
>>> "%s", _("cpu affinity is
not
>>> available"));
>>> goto cleanup;
>>> }
>>> @@ -5637,7 +5637,7 @@ static int
>>> qemuDomainSetBlkioParameters(virDomainPtr dom,
>>> }
>>>
>>> if (!qemuCgroupControllerActive(driver,
>>> VIR_CGROUP_CONTROLLER_BLKIO)) {
>>> - qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup
isn't
>>> mounted"));
>>> + qemuReportError(VIR_ERR_OPERATION_INVALID, _("blkio cgroup
>>> isn't mounted"));
>>> goto cleanup;
>>> }
>>>
>>> @@ -5790,7 +5790,7 @@ static int
>>> qemuDomainGetBlkioParameters(virDomainPtr dom,
>>> }
>>>
>>> if (!qemuCgroupControllerActive(driver,
>>> VIR_CGROUP_CONTROLLER_BLKIO)) {
>>> - qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup
isn't
>>> mounted"));
>>> + qemuReportError(VIR_ERR_OPERATION_INVALID, _("blkio cgroup
>>> isn't mounted"));
>>> goto cleanup;
>>> }
>>>
>> THe use of VIR_ERR_OPERATION_INVALID is not correct here, but I'm not
>> certain what other error is best. Perhaps ARGUMENT_UNSUPPORTED
> For VIR_ERR_OPERATION_INVALID:
>
> errmsg = _("Requested operation is not valid");
>
> For VIR_ERR_ARGUMENT_UNSUPPORTED:
>
> errmsg = _("argument unsupported");
>
> From the user's point of view, I think OPERATION_INVALID is more clear
> and sensiable. E.g.
> "Requested operation is not valid: blkio cgroup isn't mounted"
>
> "argument unsupported: blkio cgroup isn't mounted"
>
> Could we just extend the meaning for OPERATION_INVALID
> so that it's not just used when the object operated on is not in
> correct state, and used for things like above?
>
> Otherwise, I'd prefer INTERNAL_ERROR here.
Do we have documented in detail somewhere when this error codes in
question here are to be used?
For example what's the difference between VIR_ERR_OPERATION_DENIED and
VIR_ERR_OPERATION_INVALID. VIR_ERR_OPERATION_DENIED is mostly used in
libvirt.c in case of a read-only connection, but it's also used in the
xen, secret and openvz drivers. In this drivers probably
VIR_ERR_OPERATION_INVALID was meant.
Anyway, I think we need documentation about in which situations what
error codes are applicable and what their intended meaning is. We
might also need to adjust the assigned error messages to improve error
reporting.
Vote for having some documentation, determining to use which of the error
codes only from meanings of the translated strings or comments is no good,
actually there are many places in the codes are not consistent.
4:11 a.m.
New subject: [libvirt] [PATCH 4/8] qemu: Cleanup improper VIR_ERR_NO_SUPPORT use
On Thu, Sep 01, 2011 at 10:04:49AM +0200, Matthias Bolte wrote:
2011/9/1 Osier Yang <jyang(a)redhat.com>:
> 于 2011年08月25日 05:47, Daniel P. Berrange 写道:
>>
>> On Tue, Aug 23, 2011 at 05:39:41PM +0800, Osier Yang wrote:
>>>
>>> * src/qemu/qemu_command.c:
>>> s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/
>>>
>>> * src/qemu/qemu_driver.c: s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
>>>
>>> * src/qemu/qemu_process.c:
>>> s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
>>> ---
>>> src/qemu/qemu_command.c | 4 ++--
>>> src/qemu/qemu_driver.c | 16 ++++++++--------
>>> src/qemu/qemu_process.c | 4 ++--
>>> 3 files changed, 12 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>>> index dbfc7d9..287ad8d 100644
>>> --- a/src/qemu/qemu_command.c
>>> +++ b/src/qemu/qemu_command.c
>>> @@ -4084,7 +4084,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>>> switch(console->targetType) {
>>> case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO:
>>> if (!qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
>>> - qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
>>> + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s",
>>> _("virtio channel requires QEMU to support
>>> -device"));
>>> goto error;
>>> }
>>> @@ -4109,7 +4109,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>>> break;
>>>
>>> default:
>>> - qemuReportError(VIR_ERR_NO_SUPPORT,
>>> + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>> _("unsupported console target type
%s"),
>>>
>>> NULLSTR(virDomainChrConsoleTargetTypeToString(console->targetType)));
>>> goto error;
>>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>>> index c8dda73..fc2538a 100644
>>> --- a/src/qemu/qemu_driver.c
>>> +++ b/src/qemu/qemu_driver.c
>>> @@ -1552,7 +1552,7 @@ static int qemuDomainReboot(virDomainPtr dom,
>>> unsigned int flags) {
>>> vm = NULL;
>>> } else {
>>> #endif
>>> - qemuReportError(VIR_ERR_NO_SUPPORT, "%s",
>>> + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
>>> _("Reboot is not supported without the JSON
>>> monitor"));
>>> #if HAVE_YAJL
>>> }
>>> @@ -3309,7 +3309,7 @@ qemudDomainPinVcpuFlags(virDomainPtr dom,
>>> cpumap, maplen, maxcpu)< 0)
>>> goto cleanup;
>>> } else {
>>> - qemuReportError(VIR_ERR_NO_SUPPORT,
>>> + qemuReportError(VIR_ERR_OPERATION_INVALID,
>>> "%s", _("cpu affinity is not
supported"));
>>> goto cleanup;
>>> }
>>> @@ -3563,7 +3563,7 @@ qemudDomainGetVcpus(virDomainPtr dom,
>>> goto cleanup;
>>> }
>>> } else {
>>> - qemuReportError(VIR_ERR_NO_SUPPORT,
>>> + qemuReportError(VIR_ERR_OPERATION_INVALID,
>>> "%s", _("cpu affinity is
not
>>> available"));
>>> goto cleanup;
>>> }
>>> @@ -5637,7 +5637,7 @@ static int
>>> qemuDomainSetBlkioParameters(virDomainPtr dom,
>>> }
>>>
>>> if (!qemuCgroupControllerActive(driver,
>>> VIR_CGROUP_CONTROLLER_BLKIO)) {
>>> - qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup
isn't
>>> mounted"));
>>> + qemuReportError(VIR_ERR_OPERATION_INVALID, _("blkio
cgroup
>>> isn't mounted"));
>>> goto cleanup;
>>> }
>>>
>>> @@ -5790,7 +5790,7 @@ static int
>>> qemuDomainGetBlkioParameters(virDomainPtr dom,
>>> }
>>>
>>> if (!qemuCgroupControllerActive(driver,
>>> VIR_CGROUP_CONTROLLER_BLKIO)) {
>>> - qemuReportError(VIR_ERR_NO_SUPPORT, _("blkio cgroup
isn't
>>> mounted"));
>>> + qemuReportError(VIR_ERR_OPERATION_INVALID, _("blkio
cgroup
>>> isn't mounted"));
>>> goto cleanup;
>>> }
>>>
>> THe use of VIR_ERR_OPERATION_INVALID is not correct here, but I'm not
>> certain what other error is best. Perhaps ARGUMENT_UNSUPPORTED
>
> For VIR_ERR_OPERATION_INVALID:
>
> errmsg = _("Requested operation is not valid");
>
> For VIR_ERR_ARGUMENT_UNSUPPORTED:
>
> errmsg = _("argument unsupported");
>
> From the user's point of view, I think OPERATION_INVALID is more clear
> and sensiable. E.g.
> "Requested operation is not valid: blkio cgroup isn't mounted"
>
> "argument unsupported: blkio cgroup isn't mounted"
>
> Could we just extend the meaning for OPERATION_INVALID
> so that it's not just used when the object operated on is not in
> correct state, and used for things like above?
>
> Otherwise, I'd prefer INTERNAL_ERROR here.
Do we have documented in detail somewhere when this error codes in
question here are to be used?
For example what's the difference between VIR_ERR_OPERATION_DENIED and
VIR_ERR_OPERATION_INVALID. VIR_ERR_OPERATION_DENIED is mostly used in
libvirt.c in case of a read-only connection, but it's also used in the
xen, secret and openvz drivers. In this drivers probably
VIR_ERR_OPERATION_INVALID was meant.
I think of OPERATIOIN_DENIED as suitable for any kind of access control
violation.
The OpenVZ driver should have been using OPERATION_INVALID.
The secret driver is correct.
The Xen XM driver should just have that chunk of code deleted, since
it merely duplicates the check already present in libvirt.c
Anyway, I think we need documentation about in which situations what
error codes are applicable and what their intended meaning is. We
might also need to adjust the assigned error messages to improve error
reporting.
Yeah, you're right, we do really need some documentation.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:39 a.m.
New subject: [libvirt] [PATCH 5/8] remote: Cleanup improper VIR_ERR_NO_SUPPORT use
---
src/remote/remote_driver.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index e5bfa4b..28f333b 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2928,7 +2928,7 @@ static int remoteDomainEventRegister(virConnectPtr conn,
remoteDriverLock(priv);
if (priv->domainEventState->timer < 0) {
- remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event
support"));
+ remoteError(VIR_ERR_OPERATION_INVALID, "%s", _("no event
support"));
goto done;
}
@@ -3277,7 +3277,7 @@ remoteSecretGetValue (virSecretPtr secret, size_t *value_size,
/* internalFlags intentionally do not go over the wire */
if (internalFlags) {
- remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no internalFlags
support"));
+ remoteError(VIR_ERR_OPERATION_INVALID, "%s", _("no internalFlags
support"));
goto done;
}
@@ -3541,7 +3541,7 @@ static int remoteDomainEventRegisterAny(virConnectPtr conn,
remoteDriverLock(priv);
if (priv->domainEventState->timer < 0) {
- remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event
support"));
+ remoteError(VIR_ERR_OPERATION_INVALID, "%s", _("no event
support"));
goto done;
}
--
1.7.6
4:44 p.m.
New subject: [libvirt] [PATCH 5/8] remote: Cleanup improper VIR_ERR_NO_SUPPORT use
On Tue, Aug 23, 2011 at 05:39:42PM +0800, Osier Yang wrote:
---
src/remote/remote_driver.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index e5bfa4b..28f333b 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2928,7 +2928,7 @@ static int remoteDomainEventRegister(virConnectPtr conn,
remoteDriverLock(priv);
if (priv->domainEventState->timer < 0) {
- remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event
support"));
+ remoteError(VIR_ERR_OPERATION_INVALID, "%s", _("no event
support"));
goto done;
}
IMHO the original error code was correct here. While the driver method
exists, we're not able to use it since not event loop is available.
@@ -3277,7 +3277,7 @@ remoteSecretGetValue (virSecretPtr secret,
size_t *value_size,
/* internalFlags intentionally do not go over the wire */
if (internalFlags) {
- remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no internalFlags
support"));
+ remoteError(VIR_ERR_OPERATION_INVALID, "%s", _("no internalFlags
support"));
goto done;
}
This feels like it should be an INTERNAL_ERROR, since internalFlags are
not something the app can specify.
@@ -3541,7 +3541,7 @@ static int
remoteDomainEventRegisterAny(virConnectPtr conn,
remoteDriverLock(priv);
if (priv->domainEventState->timer < 0) {
- remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event
support"));
+ remoteError(VIR_ERR_OPERATION_INVALID, "%s", _("no event
support"));
goto done;
}
Again I think the original code was correct here
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:17 a.m.
New subject: [libvirt] [PATCH] remote: Fix incorrect error codes
Introduced by d4b53ef6c. For "no internalFlags support", the
error code is changed into INTERNAL_ERROR.
---
src/remote/remote_driver.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 603d589..79fcac0 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2936,7 +2936,7 @@ static int remoteDomainEventRegister(virConnectPtr conn,
remoteDriverLock(priv);
if (priv->domainEventState->timer < 0) {
- remoteError(VIR_ERR_OPERATION_INVALID, "%s", _("no event
support"));
+ remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event
support"));
goto done;
}
@@ -3285,7 +3285,7 @@ remoteSecretGetValue (virSecretPtr secret, size_t *value_size,
/* internalFlags intentionally do not go over the wire */
if (internalFlags) {
- remoteError(VIR_ERR_OPERATION_INVALID, "%s", _("no internalFlags
support"));
+ remoteError(VIR_ERR_INTERNAL_ERROR, "%s", _("no internalFlags
support"));
goto done;
}
@@ -3549,7 +3549,7 @@ static int remoteDomainEventRegisterAny(virConnectPtr conn,
remoteDriverLock(priv);
if (priv->domainEventState->timer < 0) {
- remoteError(VIR_ERR_OPERATION_INVALID, "%s", _("no event
support"));
+ remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event
support"));
goto done;
}
--
1.7.6
4:13 a.m.
New subject: [libvirt] [PATCH] remote: Fix incorrect error codes
On Thu, Sep 01, 2011 at 03:17:08PM +0800, Osier Yang wrote:
Introduced by d4b53ef6c. For "no internalFlags support",
the
error code is changed into INTERNAL_ERROR.
---
src/remote/remote_driver.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 603d589..79fcac0 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -2936,7 +2936,7 @@ static int remoteDomainEventRegister(virConnectPtr conn,
remoteDriverLock(priv);
if (priv->domainEventState->timer < 0) {
- remoteError(VIR_ERR_OPERATION_INVALID, "%s", _("no event
support"));
+ remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event
support"));
goto done;
}
@@ -3285,7 +3285,7 @@ remoteSecretGetValue (virSecretPtr secret, size_t *value_size,
/* internalFlags intentionally do not go over the wire */
if (internalFlags) {
- remoteError(VIR_ERR_OPERATION_INVALID, "%s", _("no internalFlags
support"));
+ remoteError(VIR_ERR_INTERNAL_ERROR, "%s", _("no internalFlags
support"));
goto done;
}
@@ -3549,7 +3549,7 @@ static int remoteDomainEventRegisterAny(virConnectPtr conn,
remoteDriverLock(priv);
if (priv->domainEventState->timer < 0) {
- remoteError(VIR_ERR_OPERATION_INVALID, "%s", _("no event
support"));
+ remoteError(VIR_ERR_NO_SUPPORT, "%s", _("no event
support"));
goto done;
}
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:39 a.m.
New subject: [libvirt] [PATCH 6/8] storage: Cleanup improper VIR_ERR_NO_SUPPORT use
---
src/storage/storage_backend.c | 12 ++++++------
src/storage/storage_backend_disk.c | 2 +-
src/storage/storage_backend_fs.c | 2 +-
src/storage/storage_backend_logical.c | 2 +-
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 889f530..72b37a1 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -387,7 +387,7 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
virCheckFlags(0, -1);
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_NO_SUPPORT,
+ virStorageReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("storage pool does not support
encrypted "
"volumes"));
goto cleanup;
@@ -461,7 +461,7 @@ virStorageGenerateQcowEncryption(virConnectPtr conn,
conn->secretDriver->lookupByUUID == NULL ||
conn->secretDriver->defineXML == NULL ||
conn->secretDriver->setValue == NULL) {
- virStorageReportError(VIR_ERR_NO_SUPPORT, "%s",
+ virStorageReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("secret storage not supported"));
goto cleanup;
}
@@ -740,7 +740,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
if (vol->target.format != VIR_STORAGE_FILE_QCOW &&
vol->target.format != VIR_STORAGE_FILE_QCOW2) {
- virStorageReportError(VIR_ERR_NO_SUPPORT,
+ virStorageReportError(VIR_ERR_OPERATION_INVALID,
_("qcow volume encryption unsupported with "
"volume format %s"), type);
return -1;
@@ -748,7 +748,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
enc = vol->target.encryption;
if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW &&
enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT) {
- virStorageReportError(VIR_ERR_NO_SUPPORT,
+ virStorageReportError(VIR_ERR_OPERATION_INVALID,
_("unsupported volume encryption format
%d"),
vol->target.encryption->format);
return -1;
@@ -880,13 +880,13 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn
ATTRIBUTE_UNUSED,
return -1;
}
if (vol->backingStore.path != NULL) {
- virStorageReportError(VIR_ERR_NO_SUPPORT, "%s",
+ virStorageReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("copy-on-write image not supported with "
"qcow-create"));
return -1;
}
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_NO_SUPPORT,
+ virStorageReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("encrypted volumes not supported
with "
"qcow-create"));
return -1;
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index 82b41ef..0eb34b9 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -574,7 +574,7 @@ virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED,
};
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_NO_SUPPORT,
+ virStorageReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("storage pool does not support
encrypted "
"volumes"));
return -1;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index ff5afaa..4f53d3f 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -866,7 +866,7 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn,
if (inputvol) {
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_NO_SUPPORT,
+ virStorageReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("storage pool does not support
"
"building encrypted volumes from "
"other volumes"));
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index ca4166d..a35b360 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -584,7 +584,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
const char **cmdargv = cmdargvnew;
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_NO_SUPPORT,
+ virStorageReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("storage pool does not support
encrypted "
"volumes"));
return -1;
--
1.7.6
4:42 p.m.
New subject: [libvirt] [PATCH 6/8] storage: Cleanup improper VIR_ERR_NO_SUPPORT use
On Tue, Aug 23, 2011 at 05:39:43PM +0800, Osier Yang wrote:
---
src/storage/storage_backend.c | 12 ++++++------
src/storage/storage_backend_disk.c | 2 +-
src/storage/storage_backend_fs.c | 2 +-
src/storage/storage_backend_logical.c | 2 +-
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 889f530..72b37a1 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -387,7 +387,7 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
virCheckFlags(0, -1);
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_NO_SUPPORT,
+ virStorageReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("storage pool does not support
encrypted "
"volumes"));
goto cleanup;
@@ -461,7 +461,7 @@ virStorageGenerateQcowEncryption(virConnectPtr conn,
conn->secretDriver->lookupByUUID == NULL ||
conn->secretDriver->defineXML == NULL ||
conn->secretDriver->setValue == NULL) {
- virStorageReportError(VIR_ERR_NO_SUPPORT, "%s",
+ virStorageReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("secret storage not supported"));
goto cleanup;
}
@@ -740,7 +740,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
if (vol->target.format != VIR_STORAGE_FILE_QCOW &&
vol->target.format != VIR_STORAGE_FILE_QCOW2) {
- virStorageReportError(VIR_ERR_NO_SUPPORT,
+ virStorageReportError(VIR_ERR_OPERATION_INVALID,
_("qcow volume encryption unsupported with
"
"volume format %s"), type);
return -1;
@@ -748,7 +748,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
enc = vol->target.encryption;
if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW &&
enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT) {
- virStorageReportError(VIR_ERR_NO_SUPPORT,
+ virStorageReportError(VIR_ERR_OPERATION_INVALID,
_("unsupported volume encryption format
%d"),
vol->target.encryption->format);
return -1;
@@ -880,13 +880,13 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn
ATTRIBUTE_UNUSED,
return -1;
}
if (vol->backingStore.path != NULL) {
- virStorageReportError(VIR_ERR_NO_SUPPORT, "%s",
+ virStorageReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("copy-on-write image not supported with "
"qcow-create"));
return -1;
}
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_NO_SUPPORT,
+ virStorageReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("encrypted volumes not supported
with "
"qcow-create"));
return -1;
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index 82b41ef..0eb34b9 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -574,7 +574,7 @@ virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED,
};
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_NO_SUPPORT,
+ virStorageReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("storage pool does not support
encrypted "
"volumes"));
return -1;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index ff5afaa..4f53d3f 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -866,7 +866,7 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn,
if (inputvol) {
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_NO_SUPPORT,
+ virStorageReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("storage pool does not support
"
"building encrypted volumes from "
"other volumes"));
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index ca4166d..a35b360 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -584,7 +584,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
const char **cmdargv = cmdargvnew;
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_NO_SUPPORT,
+ virStorageReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("storage pool does not support
encrypted "
"volumes"));
return -1;
All these are incorrect. They should be VIR_ERR_CONFIG_UNSUPPORTED.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:20 a.m.
New subject: [libvirt] [PATCH 6/8] storage: Cleanup improper VIR_ERR_NO_SUPPORT use
于 2011年08月25日 05:42, Daniel P. Berrange 写道:
On Tue, Aug 23, 2011 at 05:39:43PM +0800, Osier Yang wrote:
> ---
> src/storage/storage_backend.c | 12 ++++++------
> src/storage/storage_backend_disk.c | 2 +-
> src/storage/storage_backend_fs.c | 2 +-
> src/storage/storage_backend_logical.c | 2 +-
> 4 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 889f530..72b37a1 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -387,7 +387,7 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
> virCheckFlags(0, -1);
>
> if (vol->target.encryption != NULL) {
> - virStorageReportError(VIR_ERR_NO_SUPPORT,
> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
> "%s", _("storage pool does not support
encrypted "
> "volumes"));
> goto cleanup;
> @@ -461,7 +461,7 @@ virStorageGenerateQcowEncryption(virConnectPtr conn,
> conn->secretDriver->lookupByUUID == NULL ||
> conn->secretDriver->defineXML == NULL ||
> conn->secretDriver->setValue == NULL) {
> - virStorageReportError(VIR_ERR_NO_SUPPORT, "%s",
Per your previous explanation, this is changed back to NO_SUPPORT.
> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
"%s",
> _("secret storage not supported"));
> goto cleanup;
> }
> @@ -740,7 +740,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
>
> if (vol->target.format != VIR_STORAGE_FILE_QCOW&&
> vol->target.format != VIR_STORAGE_FILE_QCOW2) {
> - virStorageReportError(VIR_ERR_NO_SUPPORT,
> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
> _("qcow volume encryption unsupported with
"
> "volume format %s"), type);
> return -1;
> @@ -748,7 +748,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
> enc = vol->target.encryption;
> if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW&&
> enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT) {
> - virStorageReportError(VIR_ERR_NO_SUPPORT,
> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
> _("unsupported volume encryption format
%d"),
> vol->target.encryption->format);
> return -1;
> @@ -880,13 +880,13 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn
ATTRIBUTE_UNUSED,
> return -1;
> }
> if (vol->backingStore.path != NULL) {
> - virStorageReportError(VIR_ERR_NO_SUPPORT, "%s",
> + virStorageReportError(VIR_ERR_OPERATION_INVALID, "%s",
> _("copy-on-write image not supported with
"
> "qcow-create"));
> return -1;
> }
> if (vol->target.encryption != NULL) {
> - virStorageReportError(VIR_ERR_NO_SUPPORT,
> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
> "%s", _("encrypted volumes not
supported with "
> "qcow-create"));
> return -1;
> diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
> index 82b41ef..0eb34b9 100644
> --- a/src/storage/storage_backend_disk.c
> +++ b/src/storage/storage_backend_disk.c
> @@ -574,7 +574,7 @@ virStorageBackendDiskCreateVol(virConnectPtr conn
ATTRIBUTE_UNUSED,
> };
>
> if (vol->target.encryption != NULL) {
> - virStorageReportError(VIR_ERR_NO_SUPPORT,
> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
> "%s", _("storage pool does not support
encrypted "
> "volumes"));
> return -1;
> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
> index ff5afaa..4f53d3f 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -866,7 +866,7 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn,
>
> if (inputvol) {
> if (vol->target.encryption != NULL) {
> - virStorageReportError(VIR_ERR_NO_SUPPORT,
> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
> "%s", _("storage pool does not
support "
> "building encrypted volumes from
"
> "other volumes"));
> diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
> index ca4166d..a35b360 100644
> --- a/src/storage/storage_backend_logical.c
> +++ b/src/storage/storage_backend_logical.c
> @@ -584,7 +584,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
> const char **cmdargv = cmdargvnew;
>
> if (vol->target.encryption != NULL) {
> - virStorageReportError(VIR_ERR_NO_SUPPORT,
> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
> "%s", _("storage pool does not support
encrypted "
> "volumes"));
> return -1;
All these are incorrect. They should be VIR_ERR_CONFIG_UNSUPPORTED.
Except the one looks for secret API, agree with others should be
CONFIG_UNSUPPORTED.
Daniel
2:28 a.m.
New subject: [libvirt] [PATCH 6/8] storage: Cleanup improper VIR_ERR_NO_SUPPORT use
于 2011年09月01日 15:20, Osier Yang 写道:
于 2011年08月25日 05:42, Daniel P. Berrange 写道:
> On Tue, Aug 23, 2011 at 05:39:43PM +0800, Osier Yang wrote:
>> ---
>> src/storage/storage_backend.c | 12 ++++++------
>> src/storage/storage_backend_disk.c | 2 +-
>> src/storage/storage_backend_fs.c | 2 +-
>> src/storage/storage_backend_logical.c | 2 +-
>> 4 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/storage/storage_backend.c
>> b/src/storage/storage_backend.c
>> index 889f530..72b37a1 100644
>> --- a/src/storage/storage_backend.c
>> +++ b/src/storage/storage_backend.c
>> @@ -387,7 +387,7 @@ virStorageBackendCreateRaw(virConnectPtr conn
>> ATTRIBUTE_UNUSED,
>> virCheckFlags(0, -1);
>>
>> if (vol->target.encryption != NULL) {
>> - virStorageReportError(VIR_ERR_NO_SUPPORT,
>> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
>> "%s", _("storage pool does not
>> support encrypted "
>> "volumes"));
>> goto cleanup;
>> @@ -461,7 +461,7 @@ virStorageGenerateQcowEncryption(virConnectPtr
>> conn,
>> conn->secretDriver->lookupByUUID == NULL ||
>> conn->secretDriver->defineXML == NULL ||
>> conn->secretDriver->setValue == NULL) {
>> - virStorageReportError(VIR_ERR_NO_SUPPORT, "%s",
Per your previous explanation, this is changed back to NO_SUPPORT.
Please ignore this, after thinking a while, I think it's proper to use
CONFIG_UNSUPPORTED here.
>> + virStorageReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> _("secret storage not supported"));
>> goto cleanup;
>> }
>> @@ -740,7 +740,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
>>
>> if (vol->target.format != VIR_STORAGE_FILE_QCOW&&
>> vol->target.format != VIR_STORAGE_FILE_QCOW2) {
>> - virStorageReportError(VIR_ERR_NO_SUPPORT,
>> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
>> _("qcow volume encryption
>> unsupported with "
>> "volume format %s"), type);
>> return -1;
>> @@ -748,7 +748,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
>> enc = vol->target.encryption;
>> if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW&&
>> enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT) {
>> - virStorageReportError(VIR_ERR_NO_SUPPORT,
>> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
>> _("unsupported volume encryption
>> format %d"),
>> vol->target.encryption->format);
>> return -1;
>> @@ -880,13 +880,13 @@
>> virStorageBackendCreateQcowCreate(virConnectPtr conn ATTRIBUTE_UNUSED,
>> return -1;
>> }
>> if (vol->backingStore.path != NULL) {
>> - virStorageReportError(VIR_ERR_NO_SUPPORT, "%s",
>> + virStorageReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> _("copy-on-write image not supported
>> with "
>> "qcow-create"));
>> return -1;
>> }
>> if (vol->target.encryption != NULL) {
>> - virStorageReportError(VIR_ERR_NO_SUPPORT,
>> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
>> "%s", _("encrypted volumes not
>> supported with "
>> "qcow-create"));
>> return -1;
>> diff --git a/src/storage/storage_backend_disk.c
>> b/src/storage/storage_backend_disk.c
>> index 82b41ef..0eb34b9 100644
>> --- a/src/storage/storage_backend_disk.c
>> +++ b/src/storage/storage_backend_disk.c
>> @@ -574,7 +574,7 @@ virStorageBackendDiskCreateVol(virConnectPtr
>> conn ATTRIBUTE_UNUSED,
>> };
>>
>> if (vol->target.encryption != NULL) {
>> - virStorageReportError(VIR_ERR_NO_SUPPORT,
>> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
>> "%s", _("storage pool does not
>> support encrypted "
>> "volumes"));
>> return -1;
>> diff --git a/src/storage/storage_backend_fs.c
>> b/src/storage/storage_backend_fs.c
>> index ff5afaa..4f53d3f 100644
>> --- a/src/storage/storage_backend_fs.c
>> +++ b/src/storage/storage_backend_fs.c
>> @@ -866,7 +866,7 @@
>> _virStorageBackendFileSystemVolBuild(virConnectPtr conn,
>>
>> if (inputvol) {
>> if (vol->target.encryption != NULL) {
>> - virStorageReportError(VIR_ERR_NO_SUPPORT,
>> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
>> "%s", _("storage pool does not
>> support "
>> "building encrypted
>> volumes from "
>> "other volumes"));
>> diff --git a/src/storage/storage_backend_logical.c
>> b/src/storage/storage_backend_logical.c
>> index ca4166d..a35b360 100644
>> --- a/src/storage/storage_backend_logical.c
>> +++ b/src/storage/storage_backend_logical.c
>> @@ -584,7 +584,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr
>> conn,
>> const char **cmdargv = cmdargvnew;
>>
>> if (vol->target.encryption != NULL) {
>> - virStorageReportError(VIR_ERR_NO_SUPPORT,
>> + virStorageReportError(VIR_ERR_OPERATION_INVALID,
>> "%s", _("storage pool does not
>> support encrypted "
>> "volumes"));
>> return -1;
> All these are incorrect. They should be VIR_ERR_CONFIG_UNSUPPORTED.
Except the one looks for secret API, agree with others should be
CONFIG_UNSUPPORTED.
>
>
> Daniel
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
2:27 a.m.
New subject: [libvirt] [PATCH] storage: Fix incorrect error codes
Commit 0376f4a69b intended to fix incorrect use of VIR_ERR_NO_SUPPORT,
but replacing it with VIR_ERR_OPERATION_INVALID is not proper either.
---
src/storage/storage_backend.c | 12 ++++++------
src/storage/storage_backend_disk.c | 2 +-
src/storage/storage_backend_fs.c | 2 +-
src/storage/storage_backend_logical.c | 2 +-
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 72b37a1..d125504 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -387,7 +387,7 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
virCheckFlags(0, -1);
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("storage pool does not support
encrypted "
"volumes"));
goto cleanup;
@@ -461,7 +461,7 @@ virStorageGenerateQcowEncryption(virConnectPtr conn,
conn->secretDriver->lookupByUUID == NULL ||
conn->secretDriver->defineXML == NULL ||
conn->secretDriver->setValue == NULL) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("secret storage not supported"));
goto cleanup;
}
@@ -740,7 +740,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
if (vol->target.format != VIR_STORAGE_FILE_QCOW &&
vol->target.format != VIR_STORAGE_FILE_QCOW2) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("qcow volume encryption unsupported with "
"volume format %s"), type);
return -1;
@@ -748,7 +748,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
enc = vol->target.encryption;
if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW &&
enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported volume encryption format
%d"),
vol->target.encryption->format);
return -1;
@@ -880,13 +880,13 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn
ATTRIBUTE_UNUSED,
return -1;
}
if (vol->backingStore.path != NULL) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("copy-on-write image not supported with "
"qcow-create"));
return -1;
}
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("encrypted volumes not supported
with "
"qcow-create"));
return -1;
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index 0eb34b9..c08a10b 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -574,7 +574,7 @@ virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED,
};
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("storage pool does not support
encrypted "
"volumes"));
return -1;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 4f53d3f..61d86d2 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -866,7 +866,7 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn,
if (inputvol) {
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("storage pool does not support
"
"building encrypted volumes from "
"other volumes"));
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index a35b360..4f42047 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -584,7 +584,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
const char **cmdargv = cmdargvnew;
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("storage pool does not support
encrypted "
"volumes"));
return -1;
--
1.7.6
4:08 a.m.
New subject: [libvirt] [PATCH] storage: Fix incorrect error codes
On Thu, Sep 01, 2011 at 03:27:33PM +0800, Osier Yang wrote:
Commit 0376f4a69b intended to fix incorrect use of
VIR_ERR_NO_SUPPORT,
but replacing it with VIR_ERR_OPERATION_INVALID is not proper either.
---
src/storage/storage_backend.c | 12 ++++++------
src/storage/storage_backend_disk.c | 2 +-
src/storage/storage_backend_fs.c | 2 +-
src/storage/storage_backend_logical.c | 2 +-
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 72b37a1..d125504 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -387,7 +387,7 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
virCheckFlags(0, -1);
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("storage pool does not support
encrypted "
"volumes"));
goto cleanup;
@@ -461,7 +461,7 @@ virStorageGenerateQcowEncryption(virConnectPtr conn,
conn->secretDriver->lookupByUUID == NULL ||
conn->secretDriver->defineXML == NULL ||
conn->secretDriver->setValue == NULL) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("secret storage not supported"));
goto cleanup;
}
@@ -740,7 +740,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
if (vol->target.format != VIR_STORAGE_FILE_QCOW &&
vol->target.format != VIR_STORAGE_FILE_QCOW2) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("qcow volume encryption unsupported with
"
"volume format %s"), type);
return -1;
@@ -748,7 +748,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
enc = vol->target.encryption;
if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW &&
enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported volume encryption format
%d"),
vol->target.encryption->format);
return -1;
@@ -880,13 +880,13 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn
ATTRIBUTE_UNUSED,
return -1;
}
if (vol->backingStore.path != NULL) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("copy-on-write image not supported with "
"qcow-create"));
return -1;
}
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("encrypted volumes not supported
with "
"qcow-create"));
return -1;
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index 0eb34b9..c08a10b 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -574,7 +574,7 @@ virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED,
};
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("storage pool does not support
encrypted "
"volumes"));
return -1;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 4f53d3f..61d86d2 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -866,7 +866,7 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn,
if (inputvol) {
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("storage pool does not support
"
"building encrypted volumes from "
"other volumes"));
diff --git a/src/storage/storage_backend_logical.c
b/src/storage/storage_backend_logical.c
index a35b360..4f42047 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -584,7 +584,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
const char **cmdargv = cmdargvnew;
if (vol->target.encryption != NULL) {
- virStorageReportError(VIR_ERR_OPERATION_INVALID,
+ virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("storage pool does not support
encrypted "
"volumes"));
return -1;
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:39 a.m.
New subject: [libvirt] [PATCH 7/8] test: Cleanup improper VIR_ERR_NO_SUPPORT use
---
src/test/test_driver.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index b3e24b4..3dfe305 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4082,7 +4082,7 @@ testStorageFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED,
break;
default:
- testError(VIR_ERR_NO_SUPPORT,
+ testError(VIR_ERR_OPERATION_INVALID,
_("pool type '%s' does not support source
discovery"), type);
}
--
1.7.6
4:36 p.m.
New subject: [libvirt] [PATCH 7/8] test: Cleanup improper VIR_ERR_NO_SUPPORT use
On Tue, Aug 23, 2011 at 05:39:44PM +0800, Osier Yang wrote:
---
src/test/test_driver.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index b3e24b4..3dfe305 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4082,7 +4082,7 @@ testStorageFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED,
break;
default:
- testError(VIR_ERR_NO_SUPPORT,
+ testError(VIR_ERR_OPERATION_INVALID,
_("pool type '%s' does not support source
discovery"), type);
}
This patch has caused a virt-install to fail its tests
http://builder.virt-tools.org/module-virtinst.html
======================================================================
ERROR: testEnumerateiSCSI (tests.storage.TestStorage)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/var/lib/builder/source-root/virtinst/tests/storage.py", line 225, in
testEnumerateiSCSI
host=host)
File "/var/lib/builder/source-root/virtinst/virtinst/Storage.py", line 328, in
pool_list_from_sources
xml = conn.findStoragePoolSources(pool_type, source_xml, 0)
File "/var/lib/builder/install-root/lib64/python2.7/site-packages/libvirt.py",
line 2030, in findStoragePoolSources
if ret is None: raise libvirtError ('virConnectFindStoragePoolSources()
failed', conn=self)
libvirtError: Requested operation is not valid: pool type 'iscsi' does not support
source discovery
The storage drivers have lots of different pool types, so you
may have the driver method implemented, but different pool types
may still return VIR_ERR_NO_SUPPORT, and virt-install is expecting
that here.
I think this change should be reverted
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:36 a.m.
New subject: [libvirt] [PATCH] Revert "test: Cleanup improper VIR_ERR_NO_SUPPORT use"
This reverts commit 172214bd304ff958160307be2efd6614e9868946.
---
src/test/test_driver.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 3dfe305..b3e24b4 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4082,7 +4082,7 @@ testStorageFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED,
break;
default:
- testError(VIR_ERR_OPERATION_INVALID,
+ testError(VIR_ERR_NO_SUPPORT,
_("pool type '%s' does not support source
discovery"), type);
}
--
1.7.6
4:13 a.m.
New subject: [libvirt] [PATCH] Revert "test: Cleanup improper VIR_ERR_NO_SUPPORT use"
On Thu, Sep 01, 2011 at 03:36:59PM +0800, Osier Yang wrote:
This reverts commit 172214bd304ff958160307be2efd6614e9868946.
---
src/test/test_driver.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 3dfe305..b3e24b4 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4082,7 +4082,7 @@ testStorageFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED,
break;
default:
- testError(VIR_ERR_OPERATION_INVALID,
+ testError(VIR_ERR_NO_SUPPORT,
_("pool type '%s' does not support source
discovery"), type);
}
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:39 a.m.
New subject: [libvirt] [PATCH 8/8] xen: Cleanup improper VIR_ERR_NO_SUPPORT use
---
src/xen/xen_hypervisor.c | 4 ++--
src/xen/xend_internal.c | 26 +++++++++++++-------------
src/xen/xm_internal.c | 3 ++-
src/xenxs/xen_sxpr.c | 4 ++--
4 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index cb22b89..77085c9 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -1457,7 +1457,7 @@ xenHypervisorDomainBlockStats (virDomainPtr dom,
xenUnifiedUnlock(priv);
return ret;
#else
- virXenErrorFunc(VIR_ERR_NO_SUPPORT, __FUNCTION__,
+ virXenErrorFunc(VIR_ERR_OPERATION_INVALID, __FUNCTION__,
"block statistics not supported on this platform",
dom->id);
return -1;
@@ -1495,7 +1495,7 @@ xenHypervisorDomainInterfaceStats (virDomainPtr dom,
return linuxDomainInterfaceStats(path, stats);
#else
- virXenErrorFunc(VIR_ERR_NO_SUPPORT, __FUNCTION__,
+ virXenErrorFunc(VIR_ERR_OPERATION_INVALID, __FUNCTION__,
"/proc/net/dev: Interface not found", 0);
return -1;
#endif
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 6d5a854..f44d674 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -2793,14 +2793,14 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml,
goto cleanup;
}
} else {
- virXendError(VIR_ERR_NO_SUPPORT, "%s",
+ virXendError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("unsupported device type"));
goto cleanup;
}
break;
default:
- virXendError(VIR_ERR_NO_SUPPORT, "%s",
+ virXendError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("unsupported device type"));
goto cleanup;
}
@@ -2921,7 +2921,7 @@ xenDaemonUpdateDeviceFlags(virDomainPtr domain, const char *xml,
break;
default:
- virXendError(VIR_ERR_NO_SUPPORT, "%s",
+ virXendError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("unsupported device type"));
goto cleanup;
}
@@ -3032,7 +3032,7 @@ xenDaemonDetachDeviceFlags(virDomainPtr domain, const char *xml,
if (xenFormatSxprOnePCI(dev->data.hostdev, &buf, 1) < 0)
goto cleanup;
} else {
- virXendError(VIR_ERR_NO_SUPPORT, "%s",
+ virXendError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("unsupported device type"));
goto cleanup;
}
@@ -3222,7 +3222,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
/* Xen doesn't support renaming domains during migration. */
if (dname) {
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("xenDaemonDomainMigrate: Xen does not
support"
" renaming domains during migration"));
return -1;
@@ -3232,7 +3232,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
* ignores it.
*/
if (bandwidth) {
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("xenDaemonDomainMigrate: Xen does not
support"
" bandwidth limits during migration"));
return -1;
@@ -3260,7 +3260,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
* a nice error message.
*/
if (flags & VIR_MIGRATE_PAUSED) {
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("xenDaemonDomainMigrate: xend cannot migrate
paused domains"));
return -1;
}
@@ -3268,7 +3268,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
/* XXX we could easily do tunnelled & peer2peer migration too
if we want to. support these... */
if (flags != 0) {
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("xenDaemonDomainMigrate: unsupported
flag"));
return -1;
}
@@ -3569,7 +3569,7 @@ xenDaemonGetSchedulerType(virDomainPtr domain, int *nparams)
/* Support only xendConfigVersion >=4 */
priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
if (priv->xendConfigVersion < 4) {
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("unsupported in xendConfigVersion <
4"));
return NULL;
}
@@ -3645,7 +3645,7 @@ xenDaemonGetSchedulerParameters(virDomainPtr domain,
/* Support only xendConfigVersion >=4 */
priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
if (priv->xendConfigVersion < 4) {
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("unsupported in xendConfigVersion <
4"));
return (-1);
}
@@ -3752,7 +3752,7 @@ xenDaemonSetSchedulerParameters(virDomainPtr domain,
/* Support only xendConfigVersion >=4 and active domains */
priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
if (priv->xendConfigVersion < 4) {
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("unsupported in xendConfigVersion <
4"));
return (-1);
}
@@ -3871,7 +3871,7 @@ xenDaemonDomainBlockPeek (virDomainPtr domain, const char *path,
domain->name);
else {
/* This call always fails for dom0. */
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("domainBlockPeek is not supported for
dom0"));
return -1;
}
@@ -4060,7 +4060,7 @@ virDomainXMLDevID(virDomainPtr domain,
if (tmp == NULL)
return -1;
} else {
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("hotplug of device type not
supported"));
return -1;
}
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index 95387c9..24311a7 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -1571,7 +1571,8 @@ xenXMDomainBlockPeek (virDomainPtr dom ATTRIBUTE_UNUSED,
size_t size ATTRIBUTE_UNUSED,
void *buffer ATTRIBUTE_UNUSED)
{
- xenXMError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+ xenXMError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("block peeking not implemented"));
return -1;
}
diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index 1f5be5f..6d83040 100644
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -1865,7 +1865,7 @@ xenFormatSxprOnePCI(virDomainHostdevDefPtr def,
int detach)
{
if (def->managed) {
- XENXS_ERROR(VIR_ERR_NO_SUPPORT, "%s",
+ XENXS_ERROR(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("managed PCI devices not supported with XenD"));
return -1;
}
@@ -1915,7 +1915,7 @@ xenFormatSxprAllPCI(virDomainDefPtr def,
if (def->hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
def->hostdevs[i]->source.subsys.type ==
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
if (def->hostdevs[i]->managed) {
- XENXS_ERROR(VIR_ERR_NO_SUPPORT, "%s",
+ XENXS_ERROR(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("managed PCI devices not supported with
XenD"));
return -1;
}
--
1.7.6
4:41 p.m.
New subject: [libvirt] [PATCH 8/8] xen: Cleanup improper VIR_ERR_NO_SUPPORT use
On Tue, Aug 23, 2011 at 05:39:45PM +0800, Osier Yang wrote:
---
src/xen/xen_hypervisor.c | 4 ++--
src/xen/xend_internal.c | 26 +++++++++++++-------------
src/xen/xm_internal.c | 3 ++-
src/xenxs/xen_sxpr.c | 4 ++--
4 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index cb22b89..77085c9 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -1457,7 +1457,7 @@ xenHypervisorDomainBlockStats (virDomainPtr dom,
xenUnifiedUnlock(priv);
return ret;
#else
- virXenErrorFunc(VIR_ERR_NO_SUPPORT, __FUNCTION__,
+ virXenErrorFunc(VIR_ERR_OPERATION_INVALID, __FUNCTION__,
"block statistics not supported on this platform",
dom->id);
return -1;
@@ -1495,7 +1495,7 @@ xenHypervisorDomainInterfaceStats (virDomainPtr dom,
return linuxDomainInterfaceStats(path, stats);
#else
- virXenErrorFunc(VIR_ERR_NO_SUPPORT, __FUNCTION__,
+ virXenErrorFunc(VIR_ERR_OPERATION_INVALID, __FUNCTION__,
"/proc/net/dev: Interface not found", 0);
return -1;
#endif
These two changes are incorrect. Although we've registered a
driver impl for the blockstats/interfacestats APIs here, the
#if...#else.. conditional means these impls are no-op for
any non-Linux host. As such returning VIR_ERR_NO_SUPPORT
is correct.
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 6d5a854..f44d674 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -2793,14 +2793,14 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml,
goto cleanup;
}
} else {
- virXendError(VIR_ERR_NO_SUPPORT, "%s",
+ virXendError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("unsupported device type"));
goto cleanup;
}
break;
default:
- virXendError(VIR_ERR_NO_SUPPORT, "%s",
+ virXendError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("unsupported device type"));
goto cleanup;
}
@@ -2921,7 +2921,7 @@ xenDaemonUpdateDeviceFlags(virDomainPtr domain, const char *xml,
break;
default:
- virXendError(VIR_ERR_NO_SUPPORT, "%s",
+ virXendError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("unsupported device type"));
goto cleanup;
}
@@ -3032,7 +3032,7 @@ xenDaemonDetachDeviceFlags(virDomainPtr domain, const char *xml,
if (xenFormatSxprOnePCI(dev->data.hostdev, &buf, 1) < 0)
goto cleanup;
} else {
- virXendError(VIR_ERR_NO_SUPPORT, "%s",
+ virXendError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("unsupported device type"));
goto cleanup;
}
@@ -3222,7 +3222,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
/* Xen doesn't support renaming domains during migration. */
if (dname) {
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("xenDaemonDomainMigrate: Xen does not
support"
" renaming domains during migration"));
return -1;
@@ -3232,7 +3232,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
* ignores it.
*/
if (bandwidth) {
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("xenDaemonDomainMigrate: Xen does not
support"
" bandwidth limits during migration"));
return -1;
@@ -3260,7 +3260,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
* a nice error message.
*/
if (flags & VIR_MIGRATE_PAUSED) {
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("xenDaemonDomainMigrate: xend cannot
migrate paused domains"));
return -1;
}
@@ -3268,7 +3268,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
/* XXX we could easily do tunnelled & peer2peer migration too
if we want to. support these... */
if (flags != 0) {
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("xenDaemonDomainMigrate: unsupported
flag"));
return -1;
}
@@ -3569,7 +3569,7 @@ xenDaemonGetSchedulerType(virDomainPtr domain, int *nparams)
/* Support only xendConfigVersion >=4 */
priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
if (priv->xendConfigVersion < 4) {
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("unsupported in xendConfigVersion <
4"));
return NULL;
}
@@ -3645,7 +3645,7 @@ xenDaemonGetSchedulerParameters(virDomainPtr domain,
/* Support only xendConfigVersion >=4 */
priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
if (priv->xendConfigVersion < 4) {
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("unsupported in xendConfigVersion <
4"));
return (-1);
}
@@ -3752,7 +3752,7 @@ xenDaemonSetSchedulerParameters(virDomainPtr domain,
/* Support only xendConfigVersion >=4 and active domains */
priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
if (priv->xendConfigVersion < 4) {
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("unsupported in xendConfigVersion <
4"));
return (-1);
}
@@ -3871,7 +3871,7 @@ xenDaemonDomainBlockPeek (virDomainPtr domain, const char *path,
domain->name);
else {
/* This call always fails for dom0. */
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("domainBlockPeek is not supported for
dom0"));
return -1;
}
@@ -4060,7 +4060,7 @@ virDomainXMLDevID(virDomainPtr domain,
if (tmp == NULL)
return -1;
} else {
- virXendError(VIR_ERR_NO_SUPPORT,
+ virXendError(VIR_ERR_OPERATION_INVALID,
"%s", _("hotplug of device type not
supported"));
return -1;
}
All these are incorrect. VIR_ERR_OPERATION_INVALID is for cases where
an API call is not appropriate for the state of the guest. ie attempting
to pause a guest that is shutoff. ie usage of the API is invalid.
In these cases usage of the API *is* valid, but the features are not
suported. So they should be VIR_ERR_ARGUMENT_UNSUPPORTED
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index 95387c9..24311a7 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -1571,7 +1571,8 @@ xenXMDomainBlockPeek (virDomainPtr dom ATTRIBUTE_UNUSED,
size_t size ATTRIBUTE_UNUSED,
void *buffer ATTRIBUTE_UNUSED)
{
- xenXMError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+ xenXMError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("block peeking not implemented"));
return -1;
}
The original error is correct here.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:57 a.m.
New subject: [libvirt] [PATCH] xen: Fix incorrect use of error codes
Commit d07aa6a96 intended to fix incorrect use of VIR_ERR_NO_SUPPORT,
but some of the changes are not proper, this patch fixes the problems.
---
src/xen/xen_hypervisor.c | 4 ++--
src/xen/xend_internal.c | 12 ++++++------
src/xen/xm_internal.c | 3 +--
3 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 77085c9..cb22b89 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -1457,7 +1457,7 @@ xenHypervisorDomainBlockStats (virDomainPtr dom,
xenUnifiedUnlock(priv);
return ret;
#else
- virXenErrorFunc(VIR_ERR_OPERATION_INVALID, __FUNCTION__,
+ virXenErrorFunc(VIR_ERR_NO_SUPPORT, __FUNCTION__,
"block statistics not supported on this platform",
dom->id);
return -1;
@@ -1495,7 +1495,7 @@ xenHypervisorDomainInterfaceStats (virDomainPtr dom,
return linuxDomainInterfaceStats(path, stats);
#else
- virXenErrorFunc(VIR_ERR_OPERATION_INVALID, __FUNCTION__,
+ virXenErrorFunc(VIR_ERR_NO_SUPPORT, __FUNCTION__,
"/proc/net/dev: Interface not found", 0);
return -1;
#endif
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index f44d674..eb04f49 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -3222,7 +3222,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
/* Xen doesn't support renaming domains during migration. */
if (dname) {
- virXendError(VIR_ERR_OPERATION_INVALID,
+ virXendError(VIR_ERR_ARGUMENT_UNSUPPORTED,
"%s", _("xenDaemonDomainMigrate: Xen does not
support"
" renaming domains during migration"));
return -1;
@@ -3232,7 +3232,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
* ignores it.
*/
if (bandwidth) {
- virXendError(VIR_ERR_OPERATION_INVALID,
+ virXendError(VIR_ERR_ARGUMENT_UNSUPPORTED,
"%s", _("xenDaemonDomainMigrate: Xen does not
support"
" bandwidth limits during migration"));
return -1;
@@ -3260,7 +3260,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
* a nice error message.
*/
if (flags & VIR_MIGRATE_PAUSED) {
- virXendError(VIR_ERR_OPERATION_INVALID,
+ virXendError(VIR_ERR_ARGUMENT_UNSUPPORTED,
"%s", _("xenDaemonDomainMigrate: xend cannot migrate
paused domains"));
return -1;
}
@@ -3268,7 +3268,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
/* XXX we could easily do tunnelled & peer2peer migration too
if we want to. support these... */
if (flags != 0) {
- virXendError(VIR_ERR_OPERATION_INVALID,
+ virXendError(VIR_ERR_ARGUMENT_UNSUPPORTED,
"%s", _("xenDaemonDomainMigrate: unsupported
flag"));
return -1;
}
@@ -3871,7 +3871,7 @@ xenDaemonDomainBlockPeek (virDomainPtr domain, const char *path,
domain->name);
else {
/* This call always fails for dom0. */
- virXendError(VIR_ERR_OPERATION_INVALID,
+ virXendError(VIR_ERR_ARGUMENT_UNSUPPORTED,
"%s", _("domainBlockPeek is not supported for
dom0"));
return -1;
}
@@ -4060,7 +4060,7 @@ virDomainXMLDevID(virDomainPtr domain,
if (tmp == NULL)
return -1;
} else {
- virXendError(VIR_ERR_OPERATION_INVALID,
+ virXendError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("hotplug of device type not
supported"));
return -1;
}
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index 24311a7..95387c9 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -1571,8 +1571,7 @@ xenXMDomainBlockPeek (virDomainPtr dom ATTRIBUTE_UNUSED,
size_t size ATTRIBUTE_UNUSED,
void *buffer ATTRIBUTE_UNUSED)
{
- xenXMError(VIR_ERR_OPERATION_INVALID, "%s",
- _("block peeking not implemented"));
+ xenXMError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
}
--
1.7.6
4:14 a.m.
New subject: [libvirt] [PATCH] xen: Fix incorrect use of error codes
On Thu, Sep 01, 2011 at 03:57:13PM +0800, Osier Yang wrote:
Commit d07aa6a96 intended to fix incorrect use of
VIR_ERR_NO_SUPPORT,
but some of the changes are not proper, this patch fixes the problems.
---
src/xen/xen_hypervisor.c | 4 ++--
src/xen/xend_internal.c | 12 ++++++------
src/xen/xm_internal.c | 3 +--
3 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 77085c9..cb22b89 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -1457,7 +1457,7 @@ xenHypervisorDomainBlockStats (virDomainPtr dom,
xenUnifiedUnlock(priv);
return ret;
#else
- virXenErrorFunc(VIR_ERR_OPERATION_INVALID, __FUNCTION__,
+ virXenErrorFunc(VIR_ERR_NO_SUPPORT, __FUNCTION__,
"block statistics not supported on this platform",
dom->id);
return -1;
@@ -1495,7 +1495,7 @@ xenHypervisorDomainInterfaceStats (virDomainPtr dom,
return linuxDomainInterfaceStats(path, stats);
#else
- virXenErrorFunc(VIR_ERR_OPERATION_INVALID, __FUNCTION__,
+ virXenErrorFunc(VIR_ERR_NO_SUPPORT, __FUNCTION__,
"/proc/net/dev: Interface not found", 0);
return -1;
#endif
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index f44d674..eb04f49 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -3222,7 +3222,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
/* Xen doesn't support renaming domains during migration. */
if (dname) {
- virXendError(VIR_ERR_OPERATION_INVALID,
+ virXendError(VIR_ERR_ARGUMENT_UNSUPPORTED,
"%s", _("xenDaemonDomainMigrate: Xen does not
support"
" renaming domains during migration"));
return -1;
@@ -3232,7 +3232,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
* ignores it.
*/
if (bandwidth) {
- virXendError(VIR_ERR_OPERATION_INVALID,
+ virXendError(VIR_ERR_ARGUMENT_UNSUPPORTED,
"%s", _("xenDaemonDomainMigrate: Xen does not
support"
" bandwidth limits during migration"));
return -1;
@@ -3260,7 +3260,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
* a nice error message.
*/
if (flags & VIR_MIGRATE_PAUSED) {
- virXendError(VIR_ERR_OPERATION_INVALID,
+ virXendError(VIR_ERR_ARGUMENT_UNSUPPORTED,
"%s", _("xenDaemonDomainMigrate: xend cannot
migrate paused domains"));
return -1;
}
@@ -3268,7 +3268,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
/* XXX we could easily do tunnelled & peer2peer migration too
if we want to. support these... */
if (flags != 0) {
- virXendError(VIR_ERR_OPERATION_INVALID,
+ virXendError(VIR_ERR_ARGUMENT_UNSUPPORTED,
"%s", _("xenDaemonDomainMigrate: unsupported
flag"));
return -1;
}
@@ -3871,7 +3871,7 @@ xenDaemonDomainBlockPeek (virDomainPtr domain, const char *path,
domain->name);
else {
/* This call always fails for dom0. */
- virXendError(VIR_ERR_OPERATION_INVALID,
+ virXendError(VIR_ERR_ARGUMENT_UNSUPPORTED,
"%s", _("domainBlockPeek is not supported for
dom0"));
return -1;
}
@@ -4060,7 +4060,7 @@ virDomainXMLDevID(virDomainPtr domain,
if (tmp == NULL)
return -1;
} else {
- virXendError(VIR_ERR_OPERATION_INVALID,
+ virXendError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("hotplug of device type not
supported"));
return -1;
}
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index 24311a7..95387c9 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -1571,8 +1571,7 @@ xenXMDomainBlockPeek (virDomainPtr dom ATTRIBUTE_UNUSED,
size_t size ATTRIBUTE_UNUSED,
void *buffer ATTRIBUTE_UNUSED)
{
- xenXMError(VIR_ERR_OPERATION_INVALID, "%s",
- _("block peeking not implemented"));
+ xenXMError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
return -1;
}
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4:56 a.m.
New subject: [libvirt] [PATCH] xen: Fix incorrect use of error codes
于 2011年09月01日 17:14, Daniel P. Berrange 写道:
On Thu, Sep 01, 2011 at 03:57:13PM +0800, Osier Yang wrote:
> Commit d07aa6a96 intended to fix incorrect use of VIR_ERR_NO_SUPPORT,
> but some of the changes are not proper, this patch fixes the problems.
> ---
> src/xen/xen_hypervisor.c | 4 ++--
> src/xen/xend_internal.c | 12 ++++++------
> src/xen/xm_internal.c | 3 +--
> 3 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
> index 77085c9..cb22b89 100644
> --- a/src/xen/xen_hypervisor.c
> +++ b/src/xen/xen_hypervisor.c
> @@ -1457,7 +1457,7 @@ xenHypervisorDomainBlockStats (virDomainPtr dom,
> xenUnifiedUnlock(priv);
> return ret;
> #else
> - virXenErrorFunc(VIR_ERR_OPERATION_INVALID, __FUNCTION__,
> + virXenErrorFunc(VIR_ERR_NO_SUPPORT, __FUNCTION__,
> "block statistics not supported on this platform",
> dom->id);
> return -1;
> @@ -1495,7 +1495,7 @@ xenHypervisorDomainInterfaceStats (virDomainPtr dom,
>
> return linuxDomainInterfaceStats(path, stats);
> #else
> - virXenErrorFunc(VIR_ERR_OPERATION_INVALID, __FUNCTION__,
> + virXenErrorFunc(VIR_ERR_NO_SUPPORT, __FUNCTION__,
> "/proc/net/dev: Interface not found", 0);
> return -1;
> #endif
> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
> index f44d674..eb04f49 100644
> --- a/src/xen/xend_internal.c
> +++ b/src/xen/xend_internal.c
> @@ -3222,7 +3222,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
>
> /* Xen doesn't support renaming domains during migration. */
> if (dname) {
> - virXendError(VIR_ERR_OPERATION_INVALID,
> + virXendError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> "%s", _("xenDaemonDomainMigrate: Xen does not
support"
> " renaming domains during migration"));
> return -1;
> @@ -3232,7 +3232,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
> * ignores it.
> */
> if (bandwidth) {
> - virXendError(VIR_ERR_OPERATION_INVALID,
> + virXendError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> "%s", _("xenDaemonDomainMigrate: Xen does not
support"
> " bandwidth limits during migration"));
> return -1;
> @@ -3260,7 +3260,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
> * a nice error message.
> */
> if (flags& VIR_MIGRATE_PAUSED) {
> - virXendError(VIR_ERR_OPERATION_INVALID,
> + virXendError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> "%s", _("xenDaemonDomainMigrate: xend cannot
migrate paused domains"));
> return -1;
> }
> @@ -3268,7 +3268,7 @@ xenDaemonDomainMigratePerform (virDomainPtr domain,
> /* XXX we could easily do tunnelled& peer2peer migration too
> if we want to. support these... */
> if (flags != 0) {
> - virXendError(VIR_ERR_OPERATION_INVALID,
> + virXendError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> "%s", _("xenDaemonDomainMigrate: unsupported
flag"));
> return -1;
> }
> @@ -3871,7 +3871,7 @@ xenDaemonDomainBlockPeek (virDomainPtr domain, const char
*path,
> domain->name);
> else {
> /* This call always fails for dom0. */
> - virXendError(VIR_ERR_OPERATION_INVALID,
> + virXendError(VIR_ERR_ARGUMENT_UNSUPPORTED,
> "%s", _("domainBlockPeek is not supported for
dom0"));
> return -1;
> }
> @@ -4060,7 +4060,7 @@ virDomainXMLDevID(virDomainPtr domain,
> if (tmp == NULL)
> return -1;
> } else {
> - virXendError(VIR_ERR_OPERATION_INVALID,
> + virXendError(VIR_ERR_CONFIG_UNSUPPORTED,
> "%s", _("hotplug of device type not
supported"));
> return -1;
> }
> diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
> index 24311a7..95387c9 100644
> --- a/src/xen/xm_internal.c
> +++ b/src/xen/xm_internal.c
> @@ -1571,8 +1571,7 @@ xenXMDomainBlockPeek (virDomainPtr dom ATTRIBUTE_UNUSED,
> size_t size ATTRIBUTE_UNUSED,
> void *buffer ATTRIBUTE_UNUSED)
> {
> - xenXMError(VIR_ERR_OPERATION_INVALID, "%s",
> - _("block peeking not implemented"));
> + xenXMError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
> return -1;
> }
ACK
Daniel
Pushed series. Thanks
Osier
8:45 a.m.
于 2011年08月23日 17:39, Osier Yang 写道:
Error code VIR_ERR_NO_SUPPORT will be translated to "this
function
is not supported by the connection driver", however, it's used
across the whole projects, this patch is trying to cleanup all
the improper use in the source tree.
The modification can be grouped to 3 following groups:
1) The error intends to tell user it's invalid operation.
s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID.
2) The error intends to tell the configuration of domain
is not supported.
s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/
3) The error intends to tell the function is not implemented
on some platform.
* s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
* and add error strings
e.g.
static int
lxcDomainInterfaceStats(virDomainPtr dom,
const char *path ATTRIBUTE_UNUSED,
struct _virDomainInterfaceStats *stats ATTRIBUTE_UNUSED)
-lxcError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
+lxcError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("interface stats not implemented on this platform"));
return -1;
}
[PATCH 1/8] conf: Cleanup improper use of VIR_ERR_NO_SUPPORT in
[PATCH 2/8] lxc: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 3/8] nodeinfo: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 4/8] qemu: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 5/8] remote: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 6/8] storage: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 7/8] test: Cleanup improper VIR_ERR_NO_SUPPORT use
[PATCH 8/8] xen: Cleanup improper VIR_ERR_NO_SUPPORT use
Regards
Osier
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Argh, pushed these series
carelessly. Will revert it if there is
something wrong.
Osier
8:52 a.m.
On 08/23/2011 07:45 AM, Osier Yang wrote:
于 2011年08月23日 17:39, Osier Yang 写道:
> Error code VIR_ERR_NO_SUPPORT will be translated to "this function
> is not supported by the connection driver", however, it's used
> across the whole projects, this patch is trying to cleanup all
> the improper use in the source tree.
>
> The modification can be grouped to 3 following groups:
>
> 1) The error intends to tell user it's invalid operation.
>
> s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID.
>
> 2) The error intends to tell the configuration of domain
> is not supported.
>
> s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/
>
> 3) The error intends to tell the function is not implemented
> on some platform.
>
> * s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
> * and add error strings
ACK series - all relatively simple cleanups, and all make sense.
Argh, pushed these series carelessly. Will revert it if there is
something wrong.
No fears - nothing to revert. :)
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
8:52 a.m.
于 2011年08月23日 21:52, Eric Blake 写道:
On 08/23/2011 07:45 AM, Osier Yang wrote:
> 于 2011年08月23日 17:39, Osier Yang 写道:
>> Error code VIR_ERR_NO_SUPPORT will be translated to "this function
>> is not supported by the connection driver", however, it's used
>> across the whole projects, this patch is trying to cleanup all
>> the improper use in the source tree.
>>
>> The modification can be grouped to 3 following groups:
>>
>> 1) The error intends to tell user it's invalid operation.
>>
>> s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID.
>>
>> 2) The error intends to tell the configuration of domain
>> is not supported.
>>
>> s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/
>>
>> 3) The error intends to tell the function is not implemented
>> on some platform.
>>
>> * s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
>> * and add error strings
ACK series - all relatively simple cleanups, and all make sense.
> Argh, pushed these series carelessly. Will revert it if there is
> something wrong.
No fears - nothing to revert. :)
Thanks, :-)
Osier
4:55 p.m.
On Tue, Aug 23, 2011 at 05:39:37PM +0800, Osier Yang wrote:
Error code VIR_ERR_NO_SUPPORT will be translated to "this
function
is not supported by the connection driver", however, it's used
across the whole projects, this patch is trying to cleanup all
the improper use in the source tree.
The modification can be grouped to 3 following groups:
1) The error intends to tell user it's invalid operation.
s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID.
Most of these changes are wrong. Operation invalid is
defined as:
VIR_ERR_OPERATION_INVALID = 55, /* operation is not applicable at this
time */
In other words, the object is in the wrong state for the
operation be requested. For example, attempting to invoke
'virDomainSuspend' on a guest that is not currently running
would imply OPERATION_INVALID.
For arguments that aren't implemented, or are incorrect
or not possible on this platform, then one of the other
codes like ARGUMENT_UNSUPPORTED or CONFIG_UNSUPPORTED
or even just INTERNAL_ERROR
2) The error intends to tell the configuration of domain
is not supported.
s/VIR_ERR_NO_SUPPORT/VIR_ERR_CONFIG_UNSUPPORTED/
3) The error intends to tell the function is not implemented
on some platform.
* s/VIR_ERR_NO_SUPPORT/VIR_ERR_OPERATION_INVALID/
* and add error strings
This is not correct. On platforms where a driver method is
registered, but stubbed out to a no-op, then we should still
be reporting NO_SUPPORT. Ideally we would have not actually
registered the driver method at all on that platform, but it
is simpler to register it and make it a no-op stub and then
raise NO_SUPPORT.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4830
days inactive
4839
days old
43 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Matthias Bolte -
Osier Yang