[libvirt] [PATCH 2/2] storage: Improve error handling on cdrom backend

[Peter Krempa]

On Tue, Jul 03, 2018 at 10:29:39 +0800, Han Han wrote:
> Implement virFileCdromStatus() in virStorageBackendVolOpen to show
> detailed errors or warnings of cdrom instead of general Input/output error.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1596096
> Signed-off-by: Han Han <hhan at redhat.com>
> ---
>  src/storage/storage_util.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
> index a701a75702..5a7ed4c76f 100644
> --- a/src/storage/storage_util.c
> +++ b/src/storage/storage_util.c
> @@ -1538,6 +1538,44 @@ virStorageBackendVolOpen(const char *path, struct stat *sb,
>          return -1;
>      }
>  
> +    if (virFileIsCDROM(path) == 1) {
> +        switch (virFileCdromStatus(path)) {
> +            case VIR_FILE_CDROM_NO_INFO	:

I think this is such a corner case that it does not make sense to
provide as detailed error messages as you can see below.

Treating this as any other storage volume which is lacking the backing
file should be sufficient. That means that it might be required to skip
the error in case of the CDROM but reporting this status is IMO overkill
and definitely not worth the code complexity.

> +                if (noerror) {
> +                    VIR_WARN("ignoring unavailable information of %s", path);
> +                    return -2;
> +                }
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("Cdrom %s information is unavailable"), path);
> +                return -1;
> +            case VIR_FILE_CDROM_NO_DISC :
> +                if (noerror) {
> +                    VIR_WARN("ignoring no disc %s", path);
> +                    return -2;
> +                }
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("Cdrom %s has no disc"), path);
> +                return -1;
> +            case VIR_FILE_CDROM_TREY_OPEN :
> +                if (noerror) {
> +                    VIR_WARN("ignoring trey open of %s", path);
> +                    return -2;
> +                }
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("Cdrom %s is on trey-open status"), path);
> +                return -1;
> +            case VIR_FILE_CDROM_DRIVE_NOT_READY :
> +                if (noerror) {
> +                    VIR_WARN("ignoring %s not ready", path);
> +                    return -2;
> +                }
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("Cdrom %s is on not ready"), path);
> +                return -1;
> +            case VIR_FILE_CDROM_DISC_OK :
> +                VIR_INFO("Cdrom %s status is ok", path);
> +        }
> +    }
>      /* O_NONBLOCK should only matter during open() for fifos and
>       * sockets, which we already filtered; but using it prevents a
>       * TOCTTOU race.  However, later on we will want to read() the
> -- 
> 2.17.1
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180710/4e67ccc6/attachment-0001.sig>