From: Sean Dague <sean@dague.net>
Date: Thu, 8 Dec 2016 15:09:06 +0000 (-0500)
Subject: Bump prlimit cpu time for qemu from 2 to 8
X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=b78b1f8ce3aa407307a6adc5c60de1e960547897;p=osstest%2Fopenstack-nova.git

Bump prlimit cpu time for qemu from 2 to 8

We've got user reported bugs that when opperating with slow NFS
backends with large (30+ GB) disk files, the prlimit of cpu_time 2 is
guessed to be the issue at hand because if folks hot patch a qemu-img
that runs before the prlimitted one, the prlimitted one succeeds.

This increases the allowed cpu timeout, as well as tweaking the error
message so that we return something more prescriptive when the
qemu-img command fails with prlimit abort.

The original bug (#1449062) the main mitigation concern here was a
carefully crafted image that gets qemu-img to generate > 1G of json,
and hence could be a node attack vector. cpu_time was never mentioned,
and I think was added originally as a belt and suspenders addition. As
such, bumping it to 8 seconds shouldn't impact our protection in any
real way.

Change-Id: I1f4549b787fd3b458e2c48a90bf80025987f08c4
Closes-Bug: #1646181
---

diff --git a/nova/tests/unit/virt/test_images.py b/nova/tests/unit/virt/test_images.py
index 3e43224a50..c6864f295d 100644
--- a/nova/tests/unit/virt/test_images.py
+++ b/nova/tests/unit/virt/test_images.py
@@ -16,6 +16,7 @@ import os
 
 import mock
 from oslo_concurrency import processutils
+import six
 
 from nova import exception
 from nova import test
@@ -54,6 +55,16 @@ class QemuTestCase(test.NoDBTestCase):
                           'qcow2',
                           'raw')
 
+    @mock.patch.object(utils, 'execute')
+    @mock.patch.object(os.path, 'exists', return_value=True)
+    def test_convert_image_with_prlimit_fail(self, path, mocked_execute):
+        mocked_execute.side_effect = \
+            processutils.ProcessExecutionError(exit_code=-9)
+        exc = self.assertRaises(exception.InvalidDiskInfo,
+                                images.qemu_img_info,
+                                '/fake/path')
+        self.assertIn('qemu-img aborted by prlimits', six.text_type(exc))
+
     @mock.patch.object(images, 'convert_image',
                        side_effect=exception.ImageUnacceptable)
     @mock.patch.object(images, 'qemu_img_info')
diff --git a/nova/virt/images.py b/nova/virt/images.py
index 33b2470059..fc4db6275e 100644
--- a/nova/virt/images.py
+++ b/nova/virt/images.py
@@ -39,7 +39,7 @@ CONF = nova.conf.CONF
 IMAGE_API = image.API()
 
 QEMU_IMG_LIMITS = processutils.ProcessLimits(
-    cpu_time=2,
+    cpu_time=8,
     address_space=1 * units.Gi)
 
 
@@ -62,8 +62,13 @@ def qemu_img_info(path, format=None):
             cmd = cmd + ('-f', format)
         out, err = utils.execute(*cmd, prlimit=QEMU_IMG_LIMITS)
     except processutils.ProcessExecutionError as exp:
-        msg = (_("qemu-img failed to execute on %(path)s : %(exp)s") %
-                {'path': path, 'exp': exp})
+        # this means we hit prlimits, make the exception more specific
+        if exp.exit_code == -9:
+            msg = (_("qemu-img aborted by prlimits when inspecting "
+                    "%(path)s : %(exp)s") % {'path': path, 'exp': exp})
+        else:
+            msg = (_("qemu-img failed to execute on %(path)s : %(exp)s") %
+                   {'path': path, 'exp': exp})
         raise exception.InvalidDiskInfo(reason=msg)
 
     if not out: