[ABRT PATCH 0/4] vmcore patches v2

[Richard Marko]

Mostly fixing error handling + adding dependency on kexec-tools.

Please note that it still produces
selinux AVC when trying to access /var/crash on fedora 19.
(rhbz#1002637).

I've left logging intact for now, will address it globally
in next patchset.

Only following patch was updated:
[ABRT PATCHv2 4/4] vmcore: fail gracefully if dump_dir is not
 accessible


Richard Marko (4):
  vmcore: don't fail if /etc/kdump.conf is not readable
  vmcore: use re.MULTILINE instead of numerical value
  spec: vmcore: require kexec-tools
  vmcore: fail gracefully if dump_dir is not accessible

 abrt.spec.in                        |  1 +
 src/hooks/abrt_harvest_vmcore.py.in | 46 +++++++++++++++++++++++++++++--------
 2 files changed, 37 insertions(+), 10 deletions(-)

-- 
1.8.3.1

[ABRT PATCH 1/4] vmcore: don't fail if /etc/kdump.conf is not readable

[Richard Marko]

Signed-off-by: Richard Marko <rmarko(a)redhat.com&gt;
---
 src/hooks/abrt_harvest_vmcore.py.in | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/hooks/abrt_harvest_vmcore.py.in b/src/hooks/abrt_harvest_vmcore.py.in
index 4f2e1d0..f2ac4eb 100644
--- a/src/hooks/abrt_harvest_vmcore.py.in
+++ b/src/hooks/abrt_harvest_vmcore.py.in
@@ -47,10 +47,17 @@ def parse_kdump():
     This function parses /etc/kdump.conf to get a path to kdump's
     dump directory.
     """
+    # default
+    dump_path = '/var/crash'
 
     # filesystem types that can be used by kdump for dumping
     fs_types = ['ext4', 'ext3', 'ext2', 'minix',
'btrfs', 'xfs']
 
+    if not os.access('/etc/kdump.conf', os.R_OK):
+        sys.stderr.write("/etc/kdump.conf not readable, using "
+                         "default path '%s'\n" % dump_path)
+        return dump_path
+
     with open('/etc/kdump.conf') as conf_file:
         kdump_conf = conf_file.read()
 
@@ -59,9 +66,6 @@ def parse_kdump():
     result = regexp.search(kdump_conf)
     if result:
         dump_path = result.group(1).strip()
-    else:
-        # default
-        dump_path = '/var/crash'
 
     # default
     partition = None
-- 
1.8.3.1

[ABRT PATCH 2/4] vmcore: use re.MULTILINE instead of numerical value

[Richard Marko]

Signed-off-by: Richard Marko <rmarko(a)redhat.com&gt;
---
 src/hooks/abrt_harvest_vmcore.py.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/hooks/abrt_harvest_vmcore.py.in b/src/hooks/abrt_harvest_vmcore.py.in
index f2ac4eb..3951307 100644
--- a/src/hooks/abrt_harvest_vmcore.py.in
+++ b/src/hooks/abrt_harvest_vmcore.py.in
@@ -62,7 +62,7 @@ def parse_kdump():
         kdump_conf = conf_file.read()
 
     # first uncommented path instruction
-    regexp = re.compile('^(?<!#)path (.*)$', flags=8)  # MULTILINE
+    regexp = re.compile('^(?<!#)path (.*)$', flags=re.MULTILINE)
     result = regexp.search(kdump_conf)
     if result:
         dump_path = result.group(1).strip()
@@ -71,7 +71,7 @@ def parse_kdump():
     partition = None
     # first uncommented fs_type partition instruction
     for fs_type in fs_types:
-        regexp = re.compile('^(?<!#)' + fs_type + ' (.*)$', flags=8)
+        regexp = re.compile('^(?<!#)' + fs_type + ' (.*)$',
flags=re.MULTILINE)
         result = regexp.search(kdump_conf)
         if result:
             partition = result.group(1)
-- 
1.8.3.1

[ABRT PATCH 3/4] spec: vmcore: require kexec-tools

[Richard Marko]

Signed-off-by: Richard Marko <rmarko(a)redhat.com&gt;
---
 abrt.spec.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/abrt.spec.in b/abrt.spec.in
index ef20425..007d4d1 100644
--- a/abrt.spec.in
+++ b/abrt.spec.in
@@ -201,6 +201,7 @@ Group: System Environment/Libraries
 Requires: %{name} = %{version}-%{release}
 Requires: abrt-addon-kerneloops
 Requires: crash
+Requires: kexec-tools
 
 %description addon-vmcore
 This package contains plugin for collecting kernel crash information from
-- 
1.8.3.1

[ABRT PATCHv2 4/4] vmcore: fail gracefully if dump_dir is not accessible

[Richard Marko]

Previously os.listdir on inaccessible dump_dir resulted
in exception. This replaces insufficient os.path.exists check
(which is already handled in abrt-vmcore.service file) with
os.access call which checks for dir readability.

Also adding proper error handling for shutil calls later on.

Signed-off-by: Richard Marko <rmarko(a)redhat.com&gt;
---
 src/hooks/abrt_harvest_vmcore.py.in | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/src/hooks/abrt_harvest_vmcore.py.in b/src/hooks/abrt_harvest_vmcore.py.in
index 3951307..894dd47 100644
--- a/src/hooks/abrt_harvest_vmcore.py.in
+++ b/src/hooks/abrt_harvest_vmcore.py.in
@@ -186,8 +186,10 @@ def harvest_vmcore():
 
     dump_dir = parse_kdump()
 
-    if not os.path.exists(dump_dir):
-        sys.exit(0)
+    if not os.access(dump_dir, os.R_OK):
+        sys.stderr.write("Dump directory '%s' not accessible. "
+                         "Exiting.\n" % dump_dir)
+        sys.exit(1)
 
     # Wait for abrtd to start. Give it at least 1 second to initialize.
     for i in xrange(10):
@@ -219,8 +221,15 @@ def harvest_vmcore():
     except ConfigParser.NoOptionError:
         abrtdumpdir = '@DEFAULT_DUMP_LOCATION@'
 
+    try:
+        filelist = os.listdir(dump_dir)
+    except OSError:
+        sys.stderr.write("Dump directory '%s' not accessible. "
+                         "Exiting.\n" % dump_dir)
+        sys.exit(1)
+
     # Go through all directories in core dump directory
-    for cfile in os.listdir(dump_dir):
+    for cfile in filelist:
         f_full = os.path.join(dump_dir, cfile)
         if not os.path.isdir(f_full):
             continue
@@ -239,9 +248,22 @@ def harvest_vmcore():
         # Copy/move vmcore directory to abrt spool dir.
         # We use .new suffix - we must make sure abrtd doesn't try
         # to process partially-copied directory.
-        shutil.copytree(f_full, destdirnew)
+
+        try:
+            shutil.copytree(f_full, destdirnew)
+        except (OSError, shutil.Error):
+            sys.stderr.write("Unable to copy '%s' to '%s'.
Skipping"
+                             % (f_full, destdirnew))
+
+            # delete .new dir so we don't create mess
+            shutil.rmtree(destdirnew)
+            continue
+
         if copyvmcore == 'no':
-            shutil.rmtree(f_full)
+            try:
+                shutil.rmtree(f_full)
+            except OSError:
+                sys.stderr.write("Unable to delete '%s'. Ignoring" %
f_full)
 
         # Let abrtd know what type of problem it is:
         create_abrtd_info(destdirnew)
-- 
1.8.3.1