[libvirt] [PATCH 8/8] xen: Cleanup improper VIR_ERR_NO_SUPPORT use
Daniel P. Berrange
berrange at redhat.com
Wed Aug 24 21:41:40 UTC 2011
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 :|
More information about the libvir-list
mailing list