Rewrite ionice command filter using ChainingRegExpFilter

Currently, the ionice command prepended to a dd command is allowed by three
rootwrap RegExpFilter's that cover 3 arguments patterns. However, this
doesn't support if either 'iflag=direct' or 'oflag=direct' is omitted.
Because of this problem, deletion of volumes may fail if volume_clear_ionice
is set, as 'iflag=direct' is omitted.

This commit fixes this problem by replacing the filters with
ChainingRegExpFilter's, which allow to execute ionice to be combined with the
other allowed commands, including 'dd'.

Originally '-c[0-3]( -n[0-7])?' was allowed as an ionice option, but it is
invalid to specify -n[0-7] in a single option (for example, when '-c2 -n7'
is specified, ionice causes an error "invalid class argument: '2 -n7'").
In this patch, 2 filters are provided to cover the case only with -c option
and the case with both -c and -n options.

Change-Id: Ia074bf3244b7f010bd9e3b5e46c3152c1848f3d3
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
Closes-Bug: 1334422

cinder/tests/test_rootwrap_filter.py

@@ -0,0 +1,45 @@
+# Copyright (c) 2014 Hitachi Data Systems, Inc.
+# All Rights Reserved.
+#
+#   Licensed under the Apache License, Version 2.0 (the "License"); you may
+#   not use this file except in compliance with the License. You may obtain
+#   a copy of the License at
+#
+#       http://www.apache.org/licenses/LICENSE-2.0
+#
+#   Unless required by applicable law or agreed to in writing, software
+#   distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#   WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#   License for the specific language governing permissions and limitations
+#   under the License.
+
+import os
+
+import mock
+from oslo.rootwrap import wrapper
+
+from cinder import test
+
+
+class RootwrapFilterTest(test.TestCase):
+    """Test cases for etc/cinder/rootwrap.d/volume.filters
+    """
+
+    def setUp(self):
+        super(RootwrapFilterTest, self).setUp()
+        self._filters = wrapper.load_filters(['etc/cinder/rootwrap.d/'])
+
+    @mock.patch.object(os, 'access', return_value=True)
+    def _test_match(self, cmd, mock_access):
+        filtermatch = wrapper.match_filter(self._filters, cmd,
+                                           exec_dirs=['/usr/bin'])
+        self.assertIsNotNone(filtermatch)
+
+    def test_ionice_filter(self):
+        self._test_match(['ionice', '-c3', 'dd', 'if=/aaa', 'of=/bbb'])
+        self._test_match(['ionice', '-c2', '-n7', 'dd', 'if=/aaa', 'of=/bbb',
+                          'bs=1M', 'count=1024', 'oflag=direct'])
+        self._test_match(['ionice', '-c1', 'dd', 'if=/aaa', 'of=/bbb',
+                          'iflag=direct'])
+        self._test_match(['ionice', '-c0', 'dd', 'if=/aaa', 'of=/bbb',
+                          'convert=datasync'])

etc/cinder/rootwrap.d/volume.filters

@@ -41,11 +41,12 @@ iscsiadm: CommandFilter, iscsiadm, root
 # cinder/volume/drivers/lvm.py: 'shred', '-n0', '-z', '-s%dMiB'
 shred: CommandFilter, shred, root
 
-#cinder/volume/.py: utils.temporary_chown(path, 0), ...
+# cinder/volume/utils.py: utils.temporary_chown(path, 0)
 chown: CommandFilter, chown, root
-ionice_1: RegExpFilter, ionice, root, ionice, -c[0-3]( -n[0-7])?, dd, if=\S+, of=\S+, count=\d+, bs=\S+
+
-ionice_2: RegExpFilter, ionice, root, ionice, -c[0-3]( -n[0-7])?, dd, if=\S+, of=\S+, count=\d+, bs=\S+, iflag=direct, oflag=direct
+# cinder/volume/utils.py: copy_volume(..., ionice='...')
-ionice_3: RegExpFilter, ionice, root, ionice, -c[0-3]( -n[0-7])?, dd, if=\S+, of=\S+, count=\d+, bs=\S+, conv=fdatasync
+ionice_1: ChainingRegExpFilter, ionice, root, ionice, -c[0-3], -n[0-7]
+ionice_2: ChainingRegExpFilter, ionice, root, ionice, -c[0-3]
 
 # cinder/volume/utils.py: setup_blkio_cgroup()
 cgcreate: CommandFilter, cgcreate, root