[libvirt] [PATCH] xen: Fix bogus error when attaching a device
The xm internal xen driver only supports disk and network devices to be added to a guest. On an attempt to attach any other device the xm driver used VIR_ERR_XML_ERROR which resulted in a completely bogus error message: error: Failed to attach device from pci.xml error: XML description for unknown device is not well formed or invalid --- src/xen/xm_internal.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index fcc9378..00f0df8 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -2980,8 +2980,8 @@ xenXMDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, } default: - xenXMError(VIR_ERR_XML_ERROR, - "%s", _("unknown device")); + xenXMError(VIR_ERR_OPERATION_INVALID, "%s", + _("Xm driver only supports adding disk or network devices")); goto cleanup; } -- 1.7.3.1
The xm internal xen driver only supports disk and network devices to be added to a guest. On an attempt to attach any other device the xm driver used VIR_ERR_XML_ERROR which resulted in a completely bogus error message:
error: Failed to attach device from pci.xml error: XML description for unknown device is not well formed or invalid --- src/xen/xm_internal.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index fcc9378..00f0df8 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -2980,8 +2980,8 @@ xenXMDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, }
default: - xenXMError(VIR_ERR_XML_ERROR, - "%s", _("unknown device")); + xenXMError(VIR_ERR_OPERATION_INVALID, "%s", + _("Xm driver only supports adding disk or network devices")); goto cleanup; }
It looks like many other similar errors in other drivers use VIR_ERR_CONFIG_UNSUPPORTED. Would that maybe be a better choice than VIR_ERR_OPERATION_INVALID, which seems to be used when something normally valid is attempted while in an inavalid state (eg, trying to stop a domain when it's already stopped, or perform some operation that can be done only when a domain is stopped, but it's currently running.)
default: - xenXMError(VIR_ERR_XML_ERROR, - "%s", _("unknown device")); + xenXMError(VIR_ERR_OPERATION_INVALID, "%s", + _("Xm driver only supports adding disk or network devices")); goto cleanup; }
It looks like many other similar errors in other drivers use VIR_ERR_CONFIG_UNSUPPORTED. Would that maybe be a better choice than VIR_ERR_OPERATION_INVALID
Yeah, right, I guess it's not worth a v2, is it? Jirka
default: - xenXMError(VIR_ERR_XML_ERROR, - "%s", _("unknown device")); + xenXMError(VIR_ERR_OPERATION_INVALID, "%s", + _("Xm driver only supports adding disk or network devices")); goto cleanup; }
It looks like many other similar errors in other drivers use VIR_ERR_CONFIG_UNSUPPORTED. Would that maybe be a better choice than VIR_ERR_OPERATION_INVALID Yeah, right, I guess it's not worth a v2, is it?
No. The new message is in any case better than the old, and there's not much that could be screwed up ;-). So ACK with the different error code.
On Tue, Oct 05, 2010 at 09:48:24AM -0400, Laine Stump wrote:
The xm internal xen driver only supports disk and network devices to be added to a guest. On an attempt to attach any other device the xm driver used VIR_ERR_XML_ERROR which resulted in a completely bogus error message:
error: Failed to attach device from pci.xml error: XML description for unknown device is not well formed or invalid --- src/xen/xm_internal.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index fcc9378..00f0df8 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -2980,8 +2980,8 @@ xenXMDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, }
default: - xenXMError(VIR_ERR_XML_ERROR, - "%s", _("unknown device")); + xenXMError(VIR_ERR_OPERATION_INVALID, "%s", + _("Xm driver only supports adding disk or network devices")); goto cleanup; }
It looks like many other similar errors in other drivers use VIR_ERR_CONFIG_UNSUPPORTED. Would that maybe be a better choice than VIR_ERR_OPERATION_INVALID, which seems to be used when something normally valid is attempted while in an inavalid state (eg, trying to stop a domain when it's already stopped, or perform some operation that can be done only when a domain is stopped, but it's currently running.)
That is correct, OPERATION_INVALID is for reporting an operation that could succeed, if the guest were in the correct lifecycle state. If 'Xen' doesn't support this type of device at all, then CONFIG_UNSUPPORTED is the better choice. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (3)
-
Daniel P. Berrange -
Jiri Denemark -
Laine Stump