5:10 a.m.
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