[libvirt] [PATCH 0/4] Add virDomainIOError API
We already provide ways to detect when a domain has been paused as a result of I/O error, but there was no way of getting the exact error or even the device that experienced it. This new API may be used for both. Jiri Denemark (4): virDomainIOError public API and remote protocol virsh: Implement domioerror command qemu: Refactor qemuMonitorGetBlockInfo qemu: Implement virDomainIOError include/libvirt/libvirt.h.in | 19 +++++++ src/driver.h | 6 ++ src/libvirt.c | 53 ++++++++++++++++++++ src/libvirt_public.syms | 5 ++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 82 +++++++++++++++++++++++++++++++ src/qemu/qemu_hotplug.c | 22 +++++--- src/qemu/qemu_monitor.c | 83 ++++++++++++++++++++++++++++--- src/qemu/qemu_monitor.h | 10 +++- src/qemu/qemu_monitor_json.c | 34 +++++++------ src/qemu/qemu_monitor_json.h | 3 +- src/qemu/qemu_monitor_text.c | 62 ++++++++++++++++-------- src/qemu/qemu_monitor_text.h | 3 +- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 +++++- src/remote_protocol-structs | 9 +++ tools/virsh.c | 111 ++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 11 ++++ 18 files changed, 468 insertions(+), 60 deletions(-) -- 1.7.8.4
We already provide ways to detect when a domain has been paused as a result of I/O error, but there was no way of getting the exact error or even the device that experienced it. This new API may be used for both. --- include/libvirt/libvirt.h.in | 19 +++++++++++++++ src/driver.h | 6 ++++ src/libvirt.c | 53 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 +++++++++- src/remote_protocol-structs | 9 +++++++ 7 files changed, 105 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 958e5a6..2c3423a 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3746,6 +3746,25 @@ int virConnectSetKeepAlive(virConnectPtr conn, int interval, unsigned int count); +/** + * virDomainIoError: + * + * Disk I/O error. + */ +typedef enum { + VIR_DOMAIN_IOERROR_NONE = 0, /* no error */ + VIR_DOMAIN_IOERROR_NO_SPACE = 1, /* no space left on the device */ + VIR_DOMAIN_IOERROR_UNSPEC = 2, /* unspecified I/O error */ + +#ifdef VIR_ENUM_SENTINELS + VIR_DOMAIN_IOERROR_LAST +#endif +} virDomainIoError; + +int virDomainGetIoError(virDomainPtr dom, + const char *dev, + unsigned int flags); + #ifdef __cplusplus } #endif diff --git a/src/driver.h b/src/driver.h index 24636a4..1dd3760 100644 --- a/src/driver.h +++ b/src/driver.h @@ -794,6 +794,11 @@ typedef int int *nparams, unsigned int flags); +typedef int + (*virDrvDomainGetIoError)(virDomainPtr dom, + const char *dev, + unsigned int flags); + /** * _virDriver: * @@ -962,6 +967,7 @@ struct _virDriver { virDrvNodeSuspendForDuration nodeSuspendForDuration; virDrvDomainSetBlockIoTune domainSetBlockIoTune; virDrvDomainGetBlockIoTune domainGetBlockIoTune; + virDrvDomainGetIoError domainGetIoError; }; typedef int diff --git a/src/libvirt.c b/src/libvirt.c index 7b8adf7..99edca3 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17882,3 +17882,56 @@ error: virDispatchError(dom->conn); return -1; } + +/** + * virDomainGetIoError: + * @dom: a domain object + * @dev: disk device to be inspected or NULL + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * The @disk parameter is either the device target (the dev attribute of + * target subelement), such as "vda", or NULL, in which case all disks will be + * inspected for errors. If only one disk experienced an I/O error, that error + * will be returned. However, if there are more disks with I/O errors, this + * function will fail and return -2, requiring the caller to check every + * device explicitly. + * + * Returns -2 if @dev is NULL and there are multiple disks with errors, -1 if + * the function fails to do its job, the I/O error (virDomainIoError) observed + * on the specified disk (or any disk if @dev is NULL). Namely, 0 is returned + * when there is no error on the disk. + */ +int +virDomainGetIoError(virDomainPtr dom, + const char *dev, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(dom, "dev=%s, flags=%x", + NULLSTR(dev), flags); + + virResetLastError(); + + if (!VIR_IS_DOMAIN(dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (dom->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (dom->conn->driver->domainGetIoError) { + int ret = dom->conn->driver->domainGetIoError(dom, dev, flags); + if (ret == -1) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom->conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 4ca7216..d778fdc 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -516,4 +516,9 @@ LIBVIRT_0.9.9 { virDomainSetNumaParameters; } LIBVIRT_0.9.8; +LIBVIRT_0.9.10 { + global: + virDomainGetIoError; +} LIBVIRT_0.9.9; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e28840b..d89f45b 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4750,6 +4750,7 @@ static virDriver remote_driver = { .domainGetBlockIoTune = remoteDomainGetBlockIoTune, /* 0.9.8 */ .domainSetNumaParameters = remoteDomainSetNumaParameters, /* 0.9.9 */ .domainGetNumaParameters = remoteDomainGetNumaParameters, /* 0.9.9 */ + .domainGetIoError = remoteDomainGetIoError, /* 0.9.10 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 514b0cc..b1635fb 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2349,6 +2349,16 @@ struct remote_node_suspend_for_duration_args { unsigned int flags; }; +struct remote_domain_get_io_error_args { + remote_nonnull_domain dom; + remote_string dev; + unsigned int flags; +}; + +struct remote_domain_get_io_error_ret { + int result; +}; + /*----- Protocol. -----*/ @@ -2654,7 +2664,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_NUMA_PARAMETERS = 254, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, /* autogen autogen */ - REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_IO_ERROR = 258 /* autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 2758315..1438485 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1832,6 +1832,14 @@ struct remote_node_suspend_for_duration_args { uint64_t duration; u_int flags; }; +struct remote_domain_get_io_error_args { + remote_nonnull_domain dom; + remote_string dev; + u_int flags; +}; +struct remote_domain_get_io_error_ret { + int result; +}; enum remote_procedure { REMOTE_PROC_OPEN = 1, REMOTE_PROC_CLOSE = 2, @@ -2090,4 +2098,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, + REMOTE_PROC_DOMAIN_GET_IO_ERROR = 258, }; -- 1.7.8.4
----- Original Message -----
From: "Jiri Denemark" <jdenemar@redhat.com> To: libvir-list@redhat.com Sent: Monday, January 23, 2012 2:30:54 PM Subject: [libvirt] [PATCH 1/4] virDomainIOError public API and remote protocol
We already provide ways to detect when a domain has been paused as a result of I/O error, but there was no way of getting the exact error or even the device that experienced it. This new API may be used for both. --- include/libvirt/libvirt.h.in | 19 +++++++++++++++ src/driver.h | 6 ++++ src/libvirt.c | 53 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 +++++++++- src/remote_protocol-structs | 9 +++++++ 7 files changed, 105 insertions(+), 1 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 958e5a6..2c3423a 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3746,6 +3746,25 @@ int virConnectSetKeepAlive(virConnectPtr conn, int interval, unsigned int count);
+/** + * virDomainIoError: + * + * Disk I/O error. + */ +typedef enum { + VIR_DOMAIN_IOERROR_NONE = 0, /* no error */ + VIR_DOMAIN_IOERROR_NO_SPACE = 1, /* no space left on the device */ + VIR_DOMAIN_IOERROR_UNSPEC = 2, /* unspecified I/O error */ + +#ifdef VIR_ENUM_SENTINELS + VIR_DOMAIN_IOERROR_LAST +#endif +} virDomainIoError; +
Hi Jiri, how do we detect an EIO error? We should need an additional error type here? -- Federico
On Tue, Jan 24, 2012 at 11:57:23 -0500, Federico Simoncelli wrote:
+/** + * virDomainIoError: + * + * Disk I/O error. + */ +typedef enum { + VIR_DOMAIN_IOERROR_NONE = 0, /* no error */ + VIR_DOMAIN_IOERROR_NO_SPACE = 1, /* no space left on the device */ + VIR_DOMAIN_IOERROR_UNSPEC = 2, /* unspecified I/O error */ + +#ifdef VIR_ENUM_SENTINELS + VIR_DOMAIN_IOERROR_LAST +#endif +} virDomainIoError; +
Hi Jiri, how do we detect an EIO error? We should need an additional error type here?
We could certainly add more error codes but doing so only makes sense when there is a hypervisor that can report them. Current qemu has three io-status values: ok, nospace, failed and these values are mapped to VIR_DOMAIN_IOERROR_NONE, VIR_DOMAIN_IOERROR_NO_SPACE, and VIR_DOMAIN_IOERROR_UNSPEC. I believe that EIO would be reported as "failed" by qemu and thus VIR_DOMAIN_IOERROR_UNSPEC by libvirt. It's possible that VIR_DOMAIN_IOERROR_UNSPEC is not the best name, though :-) Jirka
On 23.01.2012 14:30, Jiri Denemark wrote:
We already provide ways to detect when a domain has been paused as a result of I/O error, but there was no way of getting the exact error or even the device that experienced it. This new API may be used for both. --- include/libvirt/libvirt.h.in | 19 +++++++++++++++ src/driver.h | 6 ++++ src/libvirt.c | 53 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 +++++++++- src/remote_protocol-structs | 9 +++++++ 7 files changed, 105 insertions(+), 1 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 958e5a6..2c3423a 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3746,6 +3746,25 @@ int virConnectSetKeepAlive(virConnectPtr conn, int interval, unsigned int count);
+/** + * virDomainIoError: + * + * Disk I/O error. + */ +typedef enum { + VIR_DOMAIN_IOERROR_NONE = 0, /* no error */ + VIR_DOMAIN_IOERROR_NO_SPACE = 1, /* no space left on the device */ + VIR_DOMAIN_IOERROR_UNSPEC = 2, /* unspecified I/O error */
Just cosmetic, since this is expected to be extended one day, I'd like to see _UNSPEC either at the beginning or at the end, but not in the middle. However, the latter is not possible as it's API change. But I can live with this thought.
+ +#ifdef VIR_ENUM_SENTINELS + VIR_DOMAIN_IOERROR_LAST +#endif +} virDomainIoError; + +int virDomainGetIoError(virDomainPtr dom, + const char *dev, + unsigned int flags); + #ifdef __cplusplus } #endif diff --git a/src/driver.h b/src/driver.h index 24636a4..1dd3760 100644 --- a/src/driver.h +++ b/src/driver.h @@ -794,6 +794,11 @@ typedef int int *nparams, unsigned int flags);
+typedef int + (*virDrvDomainGetIoError)(virDomainPtr dom, + const char *dev, + unsigned int flags); + /** * _virDriver: * @@ -962,6 +967,7 @@ struct _virDriver { virDrvNodeSuspendForDuration nodeSuspendForDuration; virDrvDomainSetBlockIoTune domainSetBlockIoTune; virDrvDomainGetBlockIoTune domainGetBlockIoTune; + virDrvDomainGetIoError domainGetIoError; };
typedef int diff --git a/src/libvirt.c b/src/libvirt.c index 7b8adf7..99edca3 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17882,3 +17882,56 @@ error: virDispatchError(dom->conn); return -1; } + +/** + * virDomainGetIoError: + * @dom: a domain object + * @dev: disk device to be inspected or NULL + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * The @disk parameter is either the device target (the dev attribute of + * target subelement), such as "vda", or NULL, in which case all disks will be + * inspected for errors. If only one disk experienced an I/O error, that error + * will be returned. However, if there are more disks with I/O errors, this + * function will fail and return -2, requiring the caller to check every + * device explicitly. + * + * Returns -2 if @dev is NULL and there are multiple disks with errors, -1 if + * the function fails to do its job, the I/O error (virDomainIoError) observed + * on the specified disk (or any disk if @dev is NULL). Namely, 0 is returned + * when there is no error on the disk. + */ +int +virDomainGetIoError(virDomainPtr dom, + const char *dev, + unsigned int flags)
Do we want to narrow this only for disks? Maybe some day hypervisors will report IO errors on other devices as well (haven't tried to find out if they do now days, though). However, if we restrict this only for disks, I'd change the name, so it is obvious that it's disk-only, e.g. virDomainGetDiskIOError, so we can have this name reserved for future. But if you take the other way of letting this API to report IO errors for any device, I am perfectly comfortable with disk-only implementation for now.
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 4ca7216..d778fdc 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -516,4 +516,9 @@ LIBVIRT_0.9.9 { virDomainSetNumaParameters; } LIBVIRT_0.9.8;
+LIBVIRT_0.9.10 { + global: + virDomainGetIoError; +} LIBVIRT_0.9.9; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e28840b..d89f45b 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4750,6 +4750,7 @@ static virDriver remote_driver = { .domainGetBlockIoTune = remoteDomainGetBlockIoTune, /* 0.9.8 */ .domainSetNumaParameters = remoteDomainSetNumaParameters, /* 0.9.9 */ .domainGetNumaParameters = remoteDomainGetNumaParameters, /* 0.9.9 */ + .domainGetIoError = remoteDomainGetIoError, /* 0.9.10 */ };
static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 514b0cc..b1635fb 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -2349,6 +2349,16 @@ struct remote_node_suspend_for_duration_args { unsigned int flags; };
+struct remote_domain_get_io_error_args { + remote_nonnull_domain dom; + remote_string dev; + unsigned int flags; +}; + +struct remote_domain_get_io_error_ret { + int result; +}; +
/*----- Protocol. -----*/
@@ -2654,7 +2664,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_NUMA_PARAMETERS = 254, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, /* autogen autogen */ - REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_GET_IO_ERROR = 258 /* autogen autogen */
/* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 2758315..1438485 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1832,6 +1832,14 @@ struct remote_node_suspend_for_duration_args { uint64_t duration; u_int flags; }; +struct remote_domain_get_io_error_args { + remote_nonnull_domain dom; + remote_string dev; + u_int flags; +}; +struct remote_domain_get_io_error_ret { + int result; +}; enum remote_procedure { REMOTE_PROC_OPEN = 1, REMOTE_PROC_CLOSE = 2, @@ -2090,4 +2098,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_NUMA_PARAMETERS = 255, REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, + REMOTE_PROC_DOMAIN_GET_IO_ERROR = 258, };
This will need rebasing. ACK, no matter which way you take. Michal
On 25.01.2012 12:04, Michal Privoznik wrote:
On 23.01.2012 14:30, Jiri Denemark wrote:
We already provide ways to detect when a domain has been paused as a result of I/O error, but there was no way of getting the exact error or even the device that experienced it. This new API may be used for both. --- include/libvirt/libvirt.h.in | 19 +++++++++++++++ src/driver.h | 6 ++++ src/libvirt.c | 53 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 +++++++++- src/remote_protocol-structs | 9 +++++++ 7 files changed, 105 insertions(+), 1 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 958e5a6..2c3423a 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -3746,6 +3746,25 @@ int virConnectSetKeepAlive(virConnectPtr conn, int interval, unsigned int count);
+/** + * virDomainIoError: + * + * Disk I/O error. + */ +typedef enum { + VIR_DOMAIN_IOERROR_NONE = 0, /* no error */ + VIR_DOMAIN_IOERROR_NO_SPACE = 1, /* no space left on the device */ + VIR_DOMAIN_IOERROR_UNSPEC = 2, /* unspecified I/O error */
Just cosmetic, since this is expected to be extended one day, I'd like to see _UNSPEC either at the beginning or at the end, but not in the middle. However, the latter is not possible as it's API change. But I can live with this thought.
+ +#ifdef VIR_ENUM_SENTINELS + VIR_DOMAIN_IOERROR_LAST +#endif +} virDomainIoError; + +int virDomainGetIoError(virDomainPtr dom, + const char *dev, + unsigned int flags); + #ifdef __cplusplus } #endif diff --git a/src/driver.h b/src/driver.h index 24636a4..1dd3760 100644 --- a/src/driver.h +++ b/src/driver.h @@ -794,6 +794,11 @@ typedef int int *nparams, unsigned int flags);
+typedef int + (*virDrvDomainGetIoError)(virDomainPtr dom, + const char *dev, + unsigned int flags); + /** * _virDriver: * @@ -962,6 +967,7 @@ struct _virDriver { virDrvNodeSuspendForDuration nodeSuspendForDuration; virDrvDomainSetBlockIoTune domainSetBlockIoTune; virDrvDomainGetBlockIoTune domainGetBlockIoTune; + virDrvDomainGetIoError domainGetIoError; };
typedef int diff --git a/src/libvirt.c b/src/libvirt.c index 7b8adf7..99edca3 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -17882,3 +17882,56 @@ error: virDispatchError(dom->conn); return -1; } + +/** + * virDomainGetIoError: + * @dom: a domain object + * @dev: disk device to be inspected or NULL + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * The @disk parameter is either the device target (the dev attribute of + * target subelement), such as "vda", or NULL, in which case all disks will be + * inspected for errors. If only one disk experienced an I/O error, that error + * will be returned. However, if there are more disks with I/O errors, this + * function will fail and return -2, requiring the caller to check every + * device explicitly. + * + * Returns -2 if @dev is NULL and there are multiple disks with errors, -1 if + * the function fails to do its job, the I/O error (virDomainIoError) observed + * on the specified disk (or any disk if @dev is NULL). Namely, 0 is returned + * when there is no error on the disk. + */ +int +virDomainGetIoError(virDomainPtr dom, + const char *dev, + unsigned int flags)
Do we want to narrow this only for disks? Maybe some day hypervisors will report IO errors on other devices as well (haven't tried to find out if they do now days, though). However, if we restrict this only for disks, I'd change the name, so it is obvious that it's disk-only, e.g. virDomainGetDiskIOError, so we can have this name reserved for future. But if you take the other way of letting this API to report IO errors for any device, I am perfectly comfortable with disk-only implementation for now.
Oh, one more thing I thought is obvious but doesn't have to be for others. In case you'll take the more generic way, let @dev be the alias of the device which ought to be an unique string ID among the @dom. Michal
On Mon, Jan 23, 2012 at 14:30:54 +0100, Jiri Denemark wrote:
We already provide ways to detect when a domain has been paused as a result of I/O error, but there was no way of getting the exact error or even the device that experienced it. This new API may be used for both. ... + +/** + * virDomainGetIoError: + * @dom: a domain object + * @dev: disk device to be inspected or NULL + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * The @disk parameter is either the device target (the dev attribute of + * target subelement), such as "vda", or NULL, in which case all disks will be + * inspected for errors. If only one disk experienced an I/O error, that error + * will be returned. However, if there are more disks with I/O errors, this + * function will fail and return -2, requiring the caller to check every + * device explicitly. + * + * Returns -2 if @dev is NULL and there are multiple disks with errors, -1 if + * the function fails to do its job, the I/O error (virDomainIoError) observed + * on the specified disk (or any disk if @dev is NULL). Namely, 0 is returned + * when there is no error on the disk. + */ +int +virDomainGetIoError(virDomainPtr dom, + const char *dev, + unsigned int flags)
Actually, after talking a bit more about this with Federico on IRC, this API should rather provide a list of disks with error codes. I'll create a v2, in which I will also address Michal's comments. Jirka
--- tools/virsh.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 11 +++++ 2 files changed, 122 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index d635b56..92029fd 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -15809,6 +15809,116 @@ cleanup: } /* + * "domioerror" command + */ +static const char * +vshDomainIOErrorToString(int error) +{ + switch ((virDomainIoError) error) { + case VIR_DOMAIN_IOERROR_NONE: + return _("no error"); + case VIR_DOMAIN_IOERROR_NO_SPACE: + return _("no space"); + case VIR_DOMAIN_IOERROR_UNSPEC: + return _("unspecified error"); + case VIR_DOMAIN_IOERROR_LAST: + ; + } + + return _("unknown error"); +} + +static const vshCmdInfo info_domioerror[] = { + {"help", N_("Show I/O error on a disk device")}, + {"desc", N_("Show I/O error")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_domioerror[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id, or uuid")}, + {"disk", VSH_OT_DATA, 0, N_("disk device")}, + {"all", VSH_OT_BOOL, 0, N_("show error on all attached disks")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdDomIOError(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + const char *dev = NULL; + bool all = vshCommandOptBool(cmd, "all"); + char *xml; + xmlDocPtr xmldoc = NULL; + xmlXPathContextPtr ctxt = NULL; + xmlNodePtr *disks = NULL; + int ndisks; + int i; + int result; + bool ret = false; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vshCommandOptString(cmd, "disk", &dev) < 0) + goto cleanup; + if (dev && all) { + vshError(ctl, "%s", + _("--disk and --all options are mutually exclusive")); + goto cleanup; + } + + if (all) { + if (!(xml = virDomainGetXMLDesc(dom, 0))) + goto cleanup; + + xmldoc = virXMLParseStringCtxt(xml, _("(domain_definition)"), &ctxt); + VIR_FREE(xml); + if (!xmldoc) + goto cleanup; + + ndisks = virXPathNodeSet("./devices/disk/target", ctxt, &disks); + if (ndisks < 0) + goto cleanup; + + for (i = 0; i < ndisks; i++) { + char *disk; + + if (!(disk = virXMLPropString(disks[i], "dev"))) + continue; + + if ((result = virDomainGetIoError(dom, disk, 0)) < 0) { + VIR_FREE(disk); + goto cleanup; + } + + vshPrint(ctl, "%s: %s\n", disk, vshDomainIOErrorToString(result)); + VIR_FREE(disk); + } + } else { + if ((result = virDomainGetIoError(dom, dev, 0)) == -1) + goto cleanup; + if (result == -2) { + vshError(ctl, _("there are multiple devices with error")); + goto cleanup; + } + + vshPrint(ctl, "%s", vshDomainIOErrorToString(result)); + } + + ret = true; + +cleanup: + VIR_FREE(disks); + xmlXPathFreeContext(ctxt); + xmlFreeDoc(xmldoc); + virDomainFree(dom); + return ret; +} + +/* * "qemu-monitor-command" command */ static const vshCmdInfo info_qemu_monitor_command[] = { @@ -16014,6 +16124,7 @@ static const vshCmdDef domMonitoringCmds[] = { {"domiflist", cmdDomiflist, opts_domiflist, info_domiflist, 0}, {"domifstat", cmdDomIfstat, opts_domifstat, info_domifstat, 0}, {"dominfo", cmdDominfo, opts_dominfo, info_dominfo, 0}, + {"domioerror", cmdDomIOError, opts_domioerror, info_domioerror, 0}, {"dommemstat", cmdDomMemStat, opts_dommemstat, info_dommemstat, 0}, {"domstate", cmdDomstate, opts_domstate, info_domstate, 0}, {"list", cmdList, opts_list, info_list, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index 67f93a9..a8574a5 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -595,6 +595,17 @@ new size in kilobytes Returns basic information about the domain. +=item B<domioerror> I<domain-id> { [I<--all>] | [I<disk>] } + +Show domain I/O error. This command usually comes handy when B<domstate> +command says that a domain was paused due to I/O error. The B<domioerror> +command works in three modes. With I<--all> option, it prints all disk +devices configured for the domain and I/O errors associated with them. In +case I<disk> is specified, this command only prints the I/O error associated +with the specified disk. When neither I<--all> nor I<disk> is specified and +there is at most one disk device in I/O error state, the command prints it. +Otherwise the command fails. + =item B<domuuid> I<domain-name-or-id> Convert a domain name or id to domain UUID -- 1.7.8.4
On 23.01.2012 14:30, Jiri Denemark wrote:
--- tools/virsh.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 11 +++++ 2 files changed, 122 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index d635b56..92029fd 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -15809,6 +15809,116 @@ cleanup: }
/* + * "domioerror" command + */ +static const char * +vshDomainIOErrorToString(int error) +{ + switch ((virDomainIoError) error) {
I like this typecast as once we add something to virDomainIoError but forget to add it here, gcc will complain.
+ case VIR_DOMAIN_IOERROR_NONE: + return _("no error"); + case VIR_DOMAIN_IOERROR_NO_SPACE: + return _("no space"); + case VIR_DOMAIN_IOERROR_UNSPEC: + return _("unspecified error"); + case VIR_DOMAIN_IOERROR_LAST: + ; + } + + return _("unknown error"); +} +
Overall, looking good, but see my comment to 1/4. You may need to change this a bit then. ACK Michal
QEMU always sends details about all available block devices as an answer for "info block"/"query-block" command. On the other hand, our qemuMonitorGetBlockInfo was made for a single block devices queries only. Thus, when asking for multiple devices, we asked qemu multiple times to always get the same answer from which different parts were filtered. This patch makes qemuMonitorGetBlockInfo return a hash table of all block devices, which may later be used for getting details about specific devices. --- src/qemu/qemu_hotplug.c | 22 +++++++++++-------- src/qemu/qemu_monitor.c | 43 +++++++++++++++++++++++++++++++------- src/qemu/qemu_monitor.h | 9 +++++-- src/qemu/qemu_monitor_json.c | 28 ++++++++++-------------- src/qemu/qemu_monitor_json.h | 3 +- src/qemu/qemu_monitor_text.c | 47 ++++++++++++++++++++++++----------------- src/qemu/qemu_monitor_text.h | 3 +- 7 files changed, 95 insertions(+), 60 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4b60839..b4870be 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -155,34 +155,38 @@ qemuDomainCheckEjectableMedia(struct qemud_driver *driver, virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; + virHashTablePtr table; int ret = -1; int i; + qemuDomainObjEnterMonitor(driver, vm); + table = qemuMonitorGetBlockInfo(priv->mon); + qemuDomainObjExitMonitor(driver, vm); + + if (!table) + goto cleanup; + for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; - struct qemuDomainDiskInfo info; + struct qemuDomainDiskInfo *info; if (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK || disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { continue; } - memset(&info, 0, sizeof(info)); - - qemuDomainObjEnterMonitor(driver, vm); - if (qemuMonitorGetBlockInfo(priv->mon, disk->info.alias, &info) < 0) { - qemuDomainObjExitMonitor(driver, vm); + info = qemuMonitorBlockInfoLookup(table, disk->info.alias); + if (!info) goto cleanup; - } - qemuDomainObjExitMonitor(driver, vm); - if (info.tray_open && disk->src) + if (info->tray_open && disk->src) VIR_FREE(disk->src); } ret = 0; cleanup: + virHashFree(table); return ret; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index ad7e2a5..dda0521 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1227,24 +1227,51 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, return ret; } -int qemuMonitorGetBlockInfo(qemuMonitorPtr mon, - const char *devname, - struct qemuDomainDiskInfo *info) +virHashTablePtr +qemuMonitorGetBlockInfo(qemuMonitorPtr mon) { int ret; + virHashTablePtr table; + + VIR_DEBUG("mon=%p", mon); - VIR_DEBUG("mon=%p dev=%p info=%p", mon, devname, info); if (!mon) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", _("monitor must not be NULL")); - return -1; + return NULL; } + if (!(table = virHashCreate(32, (virHashDataFree) free))) + return NULL; + if (mon->json) - ret = qemuMonitorJSONGetBlockInfo(mon, devname, info); + ret = qemuMonitorJSONGetBlockInfo(mon, table); else - ret = qemuMonitorTextGetBlockInfo(mon, devname, info); - return ret; + ret = qemuMonitorTextGetBlockInfo(mon, table); + + if (ret < 0) { + virHashFree(table); + return NULL; + } + + return table; +} + +struct qemuDomainDiskInfo * +qemuMonitorBlockInfoLookup(virHashTablePtr blockInfo, + const char *devname) +{ + struct qemuDomainDiskInfo *info; + + VIR_DEBUG("blockInfo=%p dev=%s", blockInfo, NULLSTR(devname)); + + if (!(info = virHashLookup(blockInfo, devname))) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find info for device '%s'"), + NULLSTR(devname)); + } + + return info; } int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 15acf8b..4d52f06 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -235,9 +235,12 @@ int qemuMonitorGetBalloonInfo(qemuMonitorPtr mon, int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); -int qemuMonitorGetBlockInfo(qemuMonitorPtr mon, - const char *devname, - struct qemuDomainDiskInfo *info); + +virHashTablePtr qemuMonitorGetBlockInfo(qemuMonitorPtr mon); +struct qemuDomainDiskInfo * +qemuMonitorBlockInfoLookup(virHashTablePtr blockInfo, + const char *devname); + int qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon, const char *dev_name, long long *rd_req, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 4a76fc0..3afcff3 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1357,11 +1357,9 @@ cleanup: int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, - const char *devname, - struct qemuDomainDiskInfo *info) + virHashTablePtr table) { - int ret = 0; - bool found = false; + int ret; int i; virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-block", @@ -1389,6 +1387,7 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, for (i = 0; i < virJSONValueArraySize(devices); i++) { virJSONValuePtr dev = virJSONValueArrayGet(devices, i); + struct qemuDomainDiskInfo *info; const char *thisdev; if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) { @@ -1406,10 +1405,16 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, if (STRPREFIX(thisdev, QEMU_DRIVE_HOST_PREFIX)) thisdev += strlen(QEMU_DRIVE_HOST_PREFIX); - if (STRNEQ(thisdev, devname)) - continue; + if (VIR_ALLOC(info) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (virHashAddEntry(table, thisdev, info) < 0) { + VIR_FREE(info); + goto cleanup; + } - found = true; if (virJSONValueObjectGetBoolean(dev, "removable", &info->removable) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("cannot read %s value"), @@ -1429,15 +1434,6 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, */ ignore_value(virJSONValueObjectGetBoolean(dev, "tray-open", &info->tray_open)); - - break; - } - - if (!found) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find info for device '%s'"), - devname); - goto cleanup; } ret = 0; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 5991790..d221e59 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -63,8 +63,7 @@ int qemuMonitorJSONGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, - const char *devname, - struct qemuDomainDiskInfo *info); + virHashTablePtr table); int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon, const char *dev_name, long long *rd_req, diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 2c68be8..a33d192 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -772,14 +772,14 @@ int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon, int qemuMonitorTextGetBlockInfo(qemuMonitorPtr mon, - const char *devname, - struct qemuDomainDiskInfo *info) + virHashTablePtr table) { + struct qemuDomainDiskInfo *info; char *reply = NULL; int ret = -1; char *dummy; - const char *p, *eol; - int devnamelen = strlen(devname); + char *p, *eol; + char *dev; int tmp; if (qemuMonitorHMPCommand(mon, "info block", &reply) < 0) { @@ -805,16 +805,22 @@ int qemuMonitorTextGetBlockInfo(qemuMonitorPtr mon, if (STRPREFIX(p, QEMU_DRIVE_HOST_PREFIX)) p += strlen(QEMU_DRIVE_HOST_PREFIX); - if (STREQLEN(p, devname, devnamelen) && - p[devnamelen] == ':' && p[devnamelen+1] == ' ') { + eol = strchr(p, '\n'); + if (!eol) + eol = p + strlen(p) - 1; - eol = strchr(p, '\n'); - if (!eol) - eol = p + strlen(p); + dev = p; + p = strchr(p, ':'); + if (p && p < eol && *(p + 1) == ' ') { + if (VIR_ALLOC(info) < 0) { + virReportOOMError(); + goto cleanup; + } - p += devnamelen + 2; /* Skip to first label. */ + *p = '\0'; + p += 2; - while (*p) { + while (p < eol) { if (STRPREFIX(p, "removable=")) { p += strlen("removable="); if (virStrToLong_i(p, &dummy, 10, &tmp) == -1) @@ -839,24 +845,25 @@ int qemuMonitorTextGetBlockInfo(qemuMonitorPtr mon, /* skip to next label */ p = strchr(p, ' '); - if (!p || p >= eol) break; + if (!p) + break; p++; } - ret = 0; - goto cleanup; + if (virHashAddEntry(table, dev, info) < 0) + goto cleanup; + else + info = NULL; } - /* skip to next line */ - p = strchr(p, '\n'); - if (!p) break; - p++; + /* skip to the next line */ + p = eol + 1; } - qemuReportError(VIR_ERR_INVALID_ARG, - _("no info for device '%s'"), devname); + ret = 0; cleanup: + VIR_FREE(info); VIR_FREE(reply); return ret; } diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index dee2980..ba1b0d0 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -60,8 +60,7 @@ int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); int qemuMonitorTextGetBlockInfo(qemuMonitorPtr mon, - const char *devname, - struct qemuDomainDiskInfo *info); + virHashTablePtr table); int qemuMonitorTextGetBlockStatsInfo(qemuMonitorPtr mon, const char *dev_name, long long *rd_req, -- 1.7.8.4
On 23.01.2012 14:30, Jiri Denemark wrote:
QEMU always sends details about all available block devices as an answer for "info block"/"query-block" command. On the other hand, our qemuMonitorGetBlockInfo was made for a single block devices queries only. Thus, when asking for multiple devices, we asked qemu multiple times to always get the same answer from which different parts were filtered. This patch makes qemuMonitorGetBlockInfo return a hash table of all block devices, which may later be used for getting details about specific devices. --- src/qemu/qemu_hotplug.c | 22 +++++++++++-------- src/qemu/qemu_monitor.c | 43 +++++++++++++++++++++++++++++++------- src/qemu/qemu_monitor.h | 9 +++++-- src/qemu/qemu_monitor_json.c | 28 ++++++++++-------------- src/qemu/qemu_monitor_json.h | 3 +- src/qemu/qemu_monitor_text.c | 47 ++++++++++++++++++++++++----------------- src/qemu/qemu_monitor_text.h | 3 +- 7 files changed, 95 insertions(+), 60 deletions(-)
ACK Michal
On Wed, Jan 25, 2012 at 12:30:43 +0100, Michal Privoznik wrote:
On 23.01.2012 14:30, Jiri Denemark wrote:
QEMU always sends details about all available block devices as an answer for "info block"/"query-block" command. On the other hand, our qemuMonitorGetBlockInfo was made for a single block devices queries only. Thus, when asking for multiple devices, we asked qemu multiple times to always get the same answer from which different parts were filtered. This patch makes qemuMonitorGetBlockInfo return a hash table of all block devices, which may later be used for getting details about specific devices. --- src/qemu/qemu_hotplug.c | 22 +++++++++++-------- src/qemu/qemu_monitor.c | 43 +++++++++++++++++++++++++++++++------- src/qemu/qemu_monitor.h | 9 +++++-- src/qemu/qemu_monitor_json.c | 28 ++++++++++-------------- src/qemu/qemu_monitor_json.h | 3 +- src/qemu/qemu_monitor_text.c | 47 ++++++++++++++++++++++++----------------- src/qemu/qemu_monitor_text.h | 3 +- 7 files changed, 95 insertions(+), 60 deletions(-)
ACK
This patch was independent so I just pushed it. Jirka
--- src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 82 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 40 ++++++++++++++++++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 8 ++++ src/qemu/qemu_monitor_text.c | 15 ++++++++ 6 files changed, 147 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 7d79823..0b65d7d 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -175,6 +175,7 @@ struct qemuDomainDiskInfo { bool removable; bool locked; bool tray_open; + int io_status; }; #endif /* __QEMUD_CONF_H */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 608e82a..c696489 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11605,6 +11605,87 @@ cleanup: return ret; } +static int +qemuDomainGetIoError(virDomainPtr dom, + const char *dev, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + qemuDomainObjPrivatePtr priv; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virHashTablePtr table = NULL; + virDomainDiskDefPtr *disks; + int ndisks; + int ret = -1; + int i; + + virCheckFlags(0, -1); + + qemuDriverLock(driver); + virUUIDFormat(dom->uuid, uuidstr); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + priv = vm->privateData; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + + if (dev) { + if ((i = virDomainDiskIndexByName(vm->def, dev, false)) < 0) + goto endjob; + disks = vm->def->disks + i; + ndisks = 1; + } else { + disks = vm->def->disks; + ndisks = vm->def->ndisks; + } + + qemuDomainObjEnterMonitor(driver, vm); + table = qemuMonitorGetBlockInfo(priv->mon); + qemuDomainObjExitMonitor(driver, vm); + if (!table) + goto endjob; + + ret = 0; + for (i = 0; i < ndisks; i++) { + struct qemuDomainDiskInfo *info; + info = qemuMonitorBlockInfoLookup(table, disks[i]->info.alias); + if (!info) + goto endjob; + + if (info->io_status > 0) { + if (ret > 0) { + ret = -2; + goto endjob; + } + ret = info->io_status; + } + } + +endjob: + if (qemuDomainObjEndJob(driver, vm) == 0) + vm = NULL; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + virHashFree(table); + return ret; +} + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, .name = "QEMU", @@ -11757,6 +11838,7 @@ static virDriver qemuDriver = { .domainGetNumaParameters = qemuDomainGetNumaParameters, /* 0.9.9 */ .domainGetInterfaceParameters = qemuDomainGetInterfaceParameters, /* 0.9.9 */ .domainSetInterfaceParameters = qemuDomainSetInterfaceParameters, /* 0.9.9 */ + .domainGetIoError = qemuDomainGetIoError, /* 0.9.10 */ }; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index dda0521..94eb913 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -87,6 +87,20 @@ VIR_ENUM_IMPL(qemuMonitorVMStatus, "postmigrate", "prelaunch", "finish-migrate", "restore-vm", "running", "save-vm", "shutdown", "watchdog") +typedef enum { + QEMU_MONITOR_BLOCK_IO_STATUS_OK, + QEMU_MONITOR_BLOCK_IO_STATUS_FAILED, + QEMU_MONITOR_BLOCK_IO_STATUS_NOSPACE, + + QEMU_MONITOR_BLOCK_IO_STATUS_LAST +} qemuMonitorBlockIOStatus; + +VIR_ENUM_DECL(qemuMonitorBlockIOStatus) + +VIR_ENUM_IMPL(qemuMonitorBlockIOStatus, + QEMU_MONITOR_BLOCK_IO_STATUS_LAST, + "ok", "failed", "nospace") + char *qemuMonitorEscapeArg(const char *in) { int len = 0; @@ -1227,6 +1241,32 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, return ret; } +int +qemuMonitorBlockIOStatusToError(const char *status) +{ + int st = qemuMonitorBlockIOStatusTypeFromString(status); + + if (st < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown block IO status: %s"), status); + return -1; + } + + switch ((qemuMonitorBlockIOStatus) st) { + case QEMU_MONITOR_BLOCK_IO_STATUS_OK: + return VIR_DOMAIN_IOERROR_NONE; + case QEMU_MONITOR_BLOCK_IO_STATUS_FAILED: + return VIR_DOMAIN_IOERROR_UNSPEC; + case QEMU_MONITOR_BLOCK_IO_STATUS_NOSPACE: + return VIR_DOMAIN_IOERROR_NO_SPACE; + + /* unreachable */ + case QEMU_MONITOR_BLOCK_IO_STATUS_LAST: + break; + } + return -1; +} + virHashTablePtr qemuMonitorGetBlockInfo(qemuMonitorPtr mon) { diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 4d52f06..bb68a41 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -236,6 +236,7 @@ int qemuMonitorGetMemoryStats(qemuMonitorPtr mon, virDomainMemoryStatPtr stats, unsigned int nr_stats); +int qemuMonitorBlockIOStatusToError(const char *status); virHashTablePtr qemuMonitorGetBlockInfo(qemuMonitorPtr mon); struct qemuDomainDiskInfo * qemuMonitorBlockInfoLookup(virHashTablePtr blockInfo, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3afcff3..3be2705 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1389,6 +1389,7 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, virJSONValuePtr dev = virJSONValueArrayGet(devices, i); struct qemuDomainDiskInfo *info; const char *thisdev; + const char *status; if (!dev || dev->type != VIR_JSON_TYPE_OBJECT) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -1434,6 +1435,13 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, */ ignore_value(virJSONValueObjectGetBoolean(dev, "tray-open", &info->tray_open)); + + /* Missing io-status indicates no error */ + if ((status = virJSONValueObjectGetString(dev, "io-status"))) { + info->io_status = qemuMonitorBlockIOStatusToError(status); + if (info->io_status < 0) + goto cleanup; + } } ret = 0; diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index a33d192..2146a03 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -839,6 +839,21 @@ int qemuMonitorTextGetBlockInfo(qemuMonitorPtr mon, VIR_DEBUG("error reading tray_open: %s", p); else info->tray_open = (tmp != 0); + } else if (STRPREFIX(p, "io-status=")) { + char *end; + char c; + + p += strlen("io-status="); + end = strchr(p, ' '); + if (!end || end > eol) + end = eol; + + c = *end; + *end = '\0'; + info->io_status = qemuMonitorBlockIOStatusToError(p); + *end = c; + if (info->io_status < 0) + goto cleanup; } else { /* ignore because we don't parse all options */ } -- 1.7.8.4
On 23.01.2012 14:30, Jiri Denemark wrote:
--- src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 82 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 40 ++++++++++++++++++++ src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 8 ++++ src/qemu/qemu_monitor_text.c | 15 ++++++++ 6 files changed, 147 insertions(+), 0 deletions(-)
ACK, but see my comment to 1/4; Michal
participants (3)
-
Federico Simoncelli -
Jiri Denemark -
Michal Privoznik