Re: [libvirt] [PATCH v3 2/2] storage: Improve error handling on cdrom backend
On 07/11/2018 05:44 AM, Han Han wrote:
Implement virFileCheckCDROM in virStorageBackendVolOpen to check if
cdrom backend is ready. Skip the error of cdrom not ready.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1596096
Signed-off-by: Han Han <hhan(a)redhat.com>
---
src/storage/storage_util.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index da99043e0a..08b625d136 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1509,6 +1509,7 @@ virStorageBackendVolOpen(const char *path, struct stat *sb,
int fd, mode = 0;
char *base = last_component(path);
bool noerror = (flags & VIR_STORAGE_VOL_OPEN_NOERROR);
+ virFileCDRomStatus cd_status;
s/;/ = VIR_FILE_CDROM_UNKNOWN;/
if (lstat(path, sb) < 0) {
if (errno == ENOENT) {
@@ -1545,6 +1546,13 @@ virStorageBackendVolOpen(const char *path, struct stat *sb,
return -1;
}
+ if (virFileCheckCDROM(path, &cd_status)) {
+ if (cd_status != VIR_FILE_CDROM_DISC_OK) {
+ VIR_WARN("ignoring CDROM %s is not ready", path);
And we have no idea why it's not ready ever logged. IOW: You have a
cd_status there, but don't log what it is.
Also, what if this libvirt is compiled when __linux__ is not default and
thus uses the "#else" version of virFileCheckCDROM? The cd_status is not
filled in there, so we'd fail always.
+ return -2;
Return of -2 is supposed to be limited to :
" If VIR_STORAGE_VOL_OPEN_NOERROR is passed, we return -2 if file mode
is unexpected or the volume is a dangling symbolic link."
Note that other places returning -2 and using VIR_WARN will use
"noerror" which is set at the top of the method.
John
+ }
+ }
+
/* 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