[{"id":2148,"web_url":"https://patchwork.libcamera.org/comment/2148/","msgid":"<4550bb7a-50a3-e477-92a4-751ac6ea0a25@ideasonboard.com>","date":"2019-07-04T07:45:28","subject":"Re: [libcamera-devel] [RFC] test: Move resource locking to test\n\tcases","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Niklas,\n\nOn 04/07/2019 02:30, Niklas Söderlund wrote:\n> Instead of blocking any test which require access to a kernel resource\n> (vimc or vivid) from running in parallel move the resource locking into\n> test library and allow meson to execute them in parallel.\n> \n> Resource locking is achieve using a lockfile for each resource in /tmp\n> and lockf(). The call to lockf() blocks until it can acquire the\n> resource and it keeps to lock until the test executable terminates\n> closing the file descriptor to the lock file  and freeing the lock. This\n> design ensures the process never deadlocks. The reason a separate lock\n> file is needed instead of locking the media device associated with the\n> resource is that such a locking scheme is already used by libcamera\n> itself.\n\nCan't we instead request the lock from the libcamera locking then ?\n\n\n> With this change I cut down the runtime of tests by 50%. The largest\n> increase is from allowing self-contained tests in parallel with tests\n> which require a kernel resource. This is because most of the tests who\n> needs a kernel resource needs vimc so there is heavy contention for that\n> single resource and little gain from meson to execute them in parallel.\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  test/camera/camera_test.h                     |  2 +-\n>  test/camera/meson.build                       |  2 +-\n>  test/controls/control_list.cpp                |  4 ++\n>  test/controls/meson.build                     |  2 +-\n>  test/libtest/test.cpp                         | 37 ++++++++++++++++++-\n>  test/libtest/test.h                           | 15 +++++++-\n>  test/media_device/media_device_test.h         |  2 +-\n>  test/media_device/meson.build                 |  2 +-\n>  test/v4l2_subdevice/meson.build               |  2 +-\n>  test/v4l2_subdevice/v4l2_subdevice_test.h     |  2 +-\n>  test/v4l2_videodevice/buffer_sharing.cpp      |  2 +-\n>  test/v4l2_videodevice/capture_async.cpp       |  2 +-\n>  test/v4l2_videodevice/double_open.cpp         |  2 +-\n>  test/v4l2_videodevice/formats.cpp             |  2 +-\n>  test/v4l2_videodevice/meson.build             |  2 +-\n>  test/v4l2_videodevice/request_buffers.cpp     |  2 +-\n>  test/v4l2_videodevice/stream_on_off.cpp       |  2 +-\n>  .../v4l2_videodevice_test.cpp                 | 16 +++++++-\n>  test/v4l2_videodevice/v4l2_videodevice_test.h |  4 +-\n>  19 files changed, 85 insertions(+), 19 deletions(-)\n> \n> diff --git a/test/camera/camera_test.h b/test/camera/camera_test.h\n> index ffc8a485bfaff836..0f3c15cc3dd34f3c 100644\n> --- a/test/camera/camera_test.h\n> +++ b/test/camera/camera_test.h\n> @@ -17,7 +17,7 @@ class CameraTest : public Test\n>  {\n>  public:\n>  \tCameraTest()\n> -\t\t: cm_(nullptr) {}\n> +\t\t: Test(VIMC), cm_(nullptr) {}\n>  \n>  protected:\n>  \tint init();\n> diff --git a/test/camera/meson.build b/test/camera/meson.build\n> index 35e97ce5de1a72ca..b10b1e76aaec0233 100644\n> --- a/test/camera/meson.build\n> +++ b/test/camera/meson.build\n> @@ -12,5 +12,5 @@ foreach t : camera_tests\n>                       dependencies : libcamera_dep,\n>                       link_with : test_libraries,\n>                       include_directories : test_includes_internal)\n> -    test(t[0], exe, suite : 'camera', is_parallel : false)\n> +    test(t[0], exe, suite : 'camera')\n>  endforeach\n> diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp\n> index c834edc352f5d7d4..0fc3ded2545df2e1 100644\n> --- a/test/controls/control_list.cpp\n> +++ b/test/controls/control_list.cpp\n> @@ -18,6 +18,10 @@ using namespace libcamera;\n>  \n>  class ControlListTest : public Test\n>  {\n> +public:\n> +\tControlListTest()\n> +\t\t: Test(VIMC) {}\n> +\n>  protected:\n>  \tint init()\n>  \t{\n> diff --git a/test/controls/meson.build b/test/controls/meson.build\n> index f4fc7b947dd9e2a6..724871e3136f369b 100644\n> --- a/test/controls/meson.build\n> +++ b/test/controls/meson.build\n> @@ -9,5 +9,5 @@ foreach t : control_tests\n>                       dependencies : libcamera_dep,\n>                       link_with : test_libraries,\n>                       include_directories : test_includes_internal)\n> -    test(t[0], exe, suite : 'controls', is_parallel : false)\n> +    test(t[0], exe, suite : 'controls')\n>  endforeach\n> diff --git a/test/libtest/test.cpp b/test/libtest/test.cpp\n> index b119cf19273900d2..cf13fa42dd9fdc5a 100644\n> --- a/test/libtest/test.cpp\n> +++ b/test/libtest/test.cpp\n> @@ -5,11 +5,15 @@\n>   * test.cpp - libcamera test base class\n>   */\n>  \n> +#include <fcntl.h>\n> +#include <iostream>\n>  #include <stdlib.h>\n> +#include <sys/stat.h>\n>  \n>  #include \"test.h\"\n>  \n> -Test::Test()\n> +Test::Test(TestResource resource)\n> +\t: resource_(resource)\n>  {\n>  }\n>  \n> @@ -25,6 +29,9 @@ int Test::execute()\n>  \tif (ret)\n>  \t\treturn errno;\n>  \n> +\tif (!resourceLock())\n> +\t\treturn TestFail;\n> +\n>  \tret = init();\n>  \tif (ret)\n>  \t\treturn ret;\n> @@ -35,3 +42,31 @@ int Test::execute()\n>  \n>  \treturn ret;\n>  }\n> +\n> +bool Test::resourceLock()\n> +{\n> +\tconst char *path;\n> +\n> +\tswitch (resource_) {\n> +\tcase None:\n> +\t\treturn true;\n> +\tcase VIMC:\n> +\t\tpath = \"/tmp/libcamera-test-vimc\";\n> +\t\tbreak;\n> +\tcase VIVID:\n> +\t\tpath = \"/tmp/libcamera-test-vivid\";\n> +\t\tbreak;\n> +\t}\n> +\n> +\tint fd = ::open(path, O_CREAT | O_RDWR, 0666);\n> +\tif (fd < 0) {\n> +\t\tstd::cerr << \"Failed to open resource lockfile\" << std::endl;\n> +\t\tperror(\"FOO\");\n> +\t\treturn false;\n> +\t}\n> +\n> +\tif (lockf(fd, F_LOCK, 0))\n> +\t\treturn false;\n\n\nIs there a way that we can do this at the library level, perhaps within\nour media controller layer?\n\n\n\n> +\n> +\treturn true;\n> +}\n> diff --git a/test/libtest/test.h b/test/libtest/test.h\n> index ec08bf97c03d563e..259fb589eb621489 100644\n> --- a/test/libtest/test.h\n> +++ b/test/libtest/test.h\n> @@ -15,10 +15,16 @@ enum TestStatus {\n>  \tTestSkip = 77,\n>  };\n>  \n> +enum TestResource {\n> +\tNone,\n> +\tVIMC,\n> +\tVIVID,\n> +};\n> +\n>  class Test\n>  {\n>  public:\n> -\tTest();\n> +\tTest(TestResource resource = None);\n>  \tvirtual ~Test();\n>  \n>  \tint execute();\n> @@ -27,6 +33,13 @@ protected:\n>  \tvirtual int init() { return 0; }\n>  \tvirtual int run() = 0;\n>  \tvirtual void cleanup() { }\n> +\n> +\tTestResource resource() { return resource_; }\n> +\n> +private:\n> +\tbool resourceLock();\n> +\n> +\tTestResource resource_;\n>  };\n>  \n>  #define TEST_REGISTER(klass)\t\t\t\t\t\t\\\n> diff --git a/test/media_device/media_device_test.h b/test/media_device/media_device_test.h\n> index cdbd14841d5ccca2..9e34ba86c397a99c 100644\n> --- a/test/media_device/media_device_test.h\n> +++ b/test/media_device/media_device_test.h\n> @@ -20,7 +20,7 @@ class MediaDeviceTest : public Test\n>  {\n>  public:\n>  \tMediaDeviceTest()\n> -\t\t: media_(nullptr), enumerator_(nullptr) {}\n> +\t\t: Test(VIMC), media_(nullptr), enumerator_(nullptr) {}\n>  \n>  protected:\n>  \tint init();\n> diff --git a/test/media_device/meson.build b/test/media_device/meson.build\n> index 6a0e468434b58efe..6a5b24d27aa0f618 100644\n> --- a/test/media_device/meson.build\n> +++ b/test/media_device/meson.build\n> @@ -18,5 +18,5 @@ foreach t : media_device_tests\n>                       link_with : [test_libraries, lib_mdev_test],\n>                       include_directories : test_includes_internal)\n>  \n> -    test(t[0], exe, suite : 'media_device', is_parallel : false)\n> +    test(t[0], exe, suite : 'media_device')\n>  endforeach\n> diff --git a/test/v4l2_subdevice/meson.build b/test/v4l2_subdevice/meson.build\n> index 0521984b2a787ab9..47676adbe5a33f7e 100644\n> --- a/test/v4l2_subdevice/meson.build\n> +++ b/test/v4l2_subdevice/meson.build\n> @@ -8,5 +8,5 @@ foreach t : v4l2_subdevice_tests\n>          dependencies : libcamera_dep,\n>          link_with : test_libraries,\n>          include_directories : test_includes_internal)\n> -    test(t[0], exe, suite : 'v4l2_subdevice', is_parallel : false)\n> +    test(t[0], exe, suite : 'v4l2_subdevice')\n>  endforeach\n> diff --git a/test/v4l2_subdevice/v4l2_subdevice_test.h b/test/v4l2_subdevice/v4l2_subdevice_test.h\n> index 96646a155536aadf..a9002834bf8959b2 100644\n> --- a/test/v4l2_subdevice/v4l2_subdevice_test.h\n> +++ b/test/v4l2_subdevice/v4l2_subdevice_test.h\n> @@ -21,7 +21,7 @@ class V4L2SubdeviceTest : public Test\n>  {\n>  public:\n>  \tV4L2SubdeviceTest()\n> -\t\t: scaler_(nullptr){};\n> +\t\t: Test(VIMC), scaler_(nullptr){};\n>  \n>  protected:\n>  \tint init() override;\n> diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp\n> index 1bc478fe8f8e1163..d619713257055fd5 100644\n> --- a/test/v4l2_videodevice/buffer_sharing.cpp\n> +++ b/test/v4l2_videodevice/buffer_sharing.cpp\n> @@ -23,7 +23,7 @@ class BufferSharingTest : public V4L2VideoDeviceTest\n>  {\n>  public:\n>  \tBufferSharingTest()\n> -\t\t: V4L2VideoDeviceTest(\"vivid\", \"vivid-000-vid-cap\"),\n> +\t\t: V4L2VideoDeviceTest(VIVID, \"vivid-000-vid-cap\"),\n>  \t\t  output_(nullptr), framesCaptured_(0), framesOutput_(0) {}\n>  \n>  protected:\n> diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp\n> index cea4fffbf7a5603d..9ae8043b31a68f0e 100644\n> --- a/test/v4l2_videodevice/capture_async.cpp\n> +++ b/test/v4l2_videodevice/capture_async.cpp\n> @@ -18,7 +18,7 @@ class CaptureAsyncTest : public V4L2VideoDeviceTest\n>  {\n>  public:\n>  \tCaptureAsyncTest()\n> -\t\t: V4L2VideoDeviceTest(\"vimc\", \"Raw Capture 0\"), frames(0) {}\n> +\t\t: V4L2VideoDeviceTest(VIMC, \"Raw Capture 0\"), frames(0) {}\n>  \n>  \tvoid receiveBuffer(Buffer *buffer)\n>  \t{\n> diff --git a/test/v4l2_videodevice/double_open.cpp b/test/v4l2_videodevice/double_open.cpp\n> index 5768d4043d0ba03f..ff1c7b136e803929 100644\n> --- a/test/v4l2_videodevice/double_open.cpp\n> +++ b/test/v4l2_videodevice/double_open.cpp\n> @@ -15,7 +15,7 @@ class DoubleOpen : public V4L2VideoDeviceTest\n>  {\n>  public:\n>  \tDoubleOpen()\n> -\t\t: V4L2VideoDeviceTest(\"vimc\", \"Raw Capture 0\") {}\n> +\t\t: V4L2VideoDeviceTest(VIMC, \"Raw Capture 0\") {}\n>  protected:\n>  \tint run()\n>  \t{\n> diff --git a/test/v4l2_videodevice/formats.cpp b/test/v4l2_videodevice/formats.cpp\n> index ee7d357de0752cae..25cd147f804ba0d5 100644\n> --- a/test/v4l2_videodevice/formats.cpp\n> +++ b/test/v4l2_videodevice/formats.cpp\n> @@ -19,7 +19,7 @@ class Format : public V4L2VideoDeviceTest\n>  {\n>  public:\n>  \tFormat()\n> -\t\t: V4L2VideoDeviceTest(\"vimc\", \"Raw Capture 0\") {}\n> +\t\t: V4L2VideoDeviceTest(VIMC, \"Raw Capture 0\") {}\n>  protected:\n>  \tint run()\n>  \t{\n> diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build\n> index 76be5e142bb63aa6..dc8204872efdc038 100644\n> --- a/test/v4l2_videodevice/meson.build\n> +++ b/test/v4l2_videodevice/meson.build\n> @@ -14,5 +14,5 @@ foreach t : v4l2_videodevice_tests\n>                       dependencies : libcamera_dep,\n>                       link_with : test_libraries,\n>                       include_directories : test_includes_internal)\n> -    test(t[0], exe, suite : 'v4l2_videodevice', is_parallel : false)\n> +    test(t[0], exe, suite : 'v4l2_videodevice')\n>  endforeach\n> diff --git a/test/v4l2_videodevice/request_buffers.cpp b/test/v4l2_videodevice/request_buffers.cpp\n> index c4aedf7b3cd61e80..bbc4073c363c128a 100644\n> --- a/test/v4l2_videodevice/request_buffers.cpp\n> +++ b/test/v4l2_videodevice/request_buffers.cpp\n> @@ -11,7 +11,7 @@ class RequestBuffersTest : public V4L2VideoDeviceTest\n>  {\n>  public:\n>  \tRequestBuffersTest()\n> -\t\t: V4L2VideoDeviceTest(\"vimc\", \"Raw Capture 0\") {}\n> +\t\t: V4L2VideoDeviceTest(VIMC, \"Raw Capture 0\") {}\n>  \n>  protected:\n>  \tint run()\n> diff --git a/test/v4l2_videodevice/stream_on_off.cpp b/test/v4l2_videodevice/stream_on_off.cpp\n> index 7664adc4c1f07046..fe021667c44559fa 100644\n> --- a/test/v4l2_videodevice/stream_on_off.cpp\n> +++ b/test/v4l2_videodevice/stream_on_off.cpp\n> @@ -11,7 +11,7 @@ class StreamOnStreamOffTest : public V4L2VideoDeviceTest\n>  {\n>  public:\n>  \tStreamOnStreamOffTest()\n> -\t\t: V4L2VideoDeviceTest(\"vimc\", \"Raw Capture 0\") {}\n> +\t\t: V4L2VideoDeviceTest(VIMC, \"Raw Capture 0\") {}\n>  protected:\n>  \tint run()\n>  \t{\n> diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp\n> index b26d06ad45197c8f..aa39e455360c868c 100644\n> --- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp\n> +++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp\n> @@ -28,6 +28,20 @@ bool exists(const std::string &path)\n>  \n>  int V4L2VideoDeviceTest::init()\n>  {\n> +\tconst char *driver;\n> +\n> +\tswitch (resource()) {\n> +\tcase VIMC:\n> +\t\tdriver = \"vimc\";\n> +\t\tbreak;\n> +\tcase VIVID:\n> +\t\tdriver = \"vivid\";\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tstd::cerr << \"Unkown driver for resource\" << std::endl;\n\ns/Unkown/Unknown/\n\n\n> +\t\treturn TestFail;\n> +\t}\n> +\n>  \tenumerator_ = DeviceEnumerator::create();\n>  \tif (!enumerator_) {\n>  \t\tcerr << \"Failed to create device enumerator\" << endl;\n> @@ -39,7 +53,7 @@ int V4L2VideoDeviceTest::init()\n>  \t\treturn TestFail;\n>  \t}\n>  \n> -\tDeviceMatch dm(driver_);\n> +\tDeviceMatch dm(driver);\n>  \tdm.add(entity_);\n>  \n>  \tmedia_ = enumerator_->search(dm);\n> diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.h b/test/v4l2_videodevice/v4l2_videodevice_test.h\n> index 3321b5a4f98fdb1d..b90a5fca6ce48fdd 100644\n> --- a/test/v4l2_videodevice/v4l2_videodevice_test.h\n> +++ b/test/v4l2_videodevice/v4l2_videodevice_test.h\n> @@ -22,8 +22,8 @@ using namespace libcamera;\n>  class V4L2VideoDeviceTest : public Test\n>  {\n>  public:\n> -\tV4L2VideoDeviceTest(const char *driver, const char *entity)\n> -\t\t: driver_(driver), entity_(entity), capture_(nullptr)\n> +\tV4L2VideoDeviceTest(TestResource resource, const char *entity)\n> +\t\t: Test(resource), entity_(entity), capture_(nullptr)\n>  \t{\n>  \t}\n>  \n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D9E3B60C2F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  4 Jul 2019 09:45:31 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4594B24B;\n\tThu,  4 Jul 2019 09:45:31 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1562226331;\n\tbh=Nf1jwE4v3nJ+9Y3cK+CaJ7htn7FbJAtbHmMwqZn1kn0=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=ujhN8ddTmqZs2J44xCYHVGRqYUGr/dtV94zWYKjDSvsFsrR7YZAjyCPfvJr8CR2Ai\n\todazFo2N7fI4IDNO5fCgEcc6redDrZScAwEgWirlP0Bz5gKMTq8/Umv70KOZgWRovV\n\tbczWd0xBKVRKoushWLpG64Ugb6E84IlaGil4+0Q8=","Reply-To":"kieran.bingham@ideasonboard.com","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20190704013004.14600-1-niklas.soderlund@ragnatech.se>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<4550bb7a-50a3-e477-92a4-751ac6ea0a25@ideasonboard.com>","Date":"Thu, 4 Jul 2019 08:45:28 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.1","MIME-Version":"1.0","In-Reply-To":"<20190704013004.14600-1-niklas.soderlund@ragnatech.se>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [RFC] test: Move resource locking to test\n\tcases","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 04 Jul 2019 07:45:32 -0000"}},{"id":2153,"web_url":"https://patchwork.libcamera.org/comment/2153/","msgid":"<20190704124953.GI6569@pendragon.ideasonboard.com>","date":"2019-07-04T12:49:53","subject":"Re: [libcamera-devel] [RFC] test: Move resource locking to test\n\tcases","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\n> Hi Niklas,\n> \n> On 04/07/2019 02:30, Niklas Söderlund wrote:\n> > Instead of blocking any test which require access to a kernel resource\n> > (vimc or vivid) from running in parallel move the resource locking into\n> > test library and allow meson to execute them in parallel.\n> > \n> > Resource locking is achieve using a lockfile for each resource in /tmp\n\ns/achieve/achieved/\n\n> > and lockf(). The call to lockf() blocks until it can acquire the\n> > resource and it keeps to lock until the test executable terminates\n\ns/to lock/the lock/\ns/terminates/terminates,/\n\n> > closing the file descriptor to the lock file  and freeing the lock. This\n\ns/  / /\n\n> > design ensures the process never deadlocks. The reason a separate lock\n\ns/process never deadlocks/test processes never deadlock/\n\nThis is however not entirely true, tests that need to lock multiple\ndevices (for instance to test buffer sharing) may deadlock if they don't\ntake the locks in the same order.\n\nEdit: I've now read the rest of the series and I see you're not locking\nindividual devices, but you lock at the vimc or vivid level. There\nshould thus be no risk of deadlock indeed.\n\n> > file is needed instead of locking the media device associated with the\n> > resource is that such a locking scheme is already used by libcamera\n> > itself.\n> \n> Can't we instead request the lock from the libcamera locking then ?\n> \n> > With this change I cut down the runtime of tests by 50%. The largest\n> > increase is from allowing self-contained tests in parallel with tests\n> > which require a kernel resource. This is because most of the tests who\n> > needs a kernel resource needs vimc so there is heavy contention for that\n> > single resource and little gain from meson to execute them in parallel.\n> > \n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  test/camera/camera_test.h                     |  2 +-\n> >  test/camera/meson.build                       |  2 +-\n> >  test/controls/control_list.cpp                |  4 ++\n> >  test/controls/meson.build                     |  2 +-\n> >  test/libtest/test.cpp                         | 37 ++++++++++++++++++-\n> >  test/libtest/test.h                           | 15 +++++++-\n> >  test/media_device/media_device_test.h         |  2 +-\n> >  test/media_device/meson.build                 |  2 +-\n> >  test/v4l2_subdevice/meson.build               |  2 +-\n> >  test/v4l2_subdevice/v4l2_subdevice_test.h     |  2 +-\n> >  test/v4l2_videodevice/buffer_sharing.cpp      |  2 +-\n> >  test/v4l2_videodevice/capture_async.cpp       |  2 +-\n> >  test/v4l2_videodevice/double_open.cpp         |  2 +-\n> >  test/v4l2_videodevice/formats.cpp             |  2 +-\n> >  test/v4l2_videodevice/meson.build             |  2 +-\n> >  test/v4l2_videodevice/request_buffers.cpp     |  2 +-\n> >  test/v4l2_videodevice/stream_on_off.cpp       |  2 +-\n> >  .../v4l2_videodevice_test.cpp                 | 16 +++++++-\n> >  test/v4l2_videodevice/v4l2_videodevice_test.h |  4 +-\n> >  19 files changed, 85 insertions(+), 19 deletions(-)\n> > \n> > diff --git a/test/camera/camera_test.h b/test/camera/camera_test.h\n> > index ffc8a485bfaff836..0f3c15cc3dd34f3c 100644\n> > --- a/test/camera/camera_test.h\n> > +++ b/test/camera/camera_test.h\n> > @@ -17,7 +17,7 @@ class CameraTest : public Test\n> >  {\n> >  public:\n> >  \tCameraTest()\n> > -\t\t: cm_(nullptr) {}\n> > +\t\t: Test(VIMC), cm_(nullptr) {}\n> >  \n> >  protected:\n> >  \tint init();\n> > diff --git a/test/camera/meson.build b/test/camera/meson.build\n> > index 35e97ce5de1a72ca..b10b1e76aaec0233 100644\n> > --- a/test/camera/meson.build\n> > +++ b/test/camera/meson.build\n> > @@ -12,5 +12,5 @@ foreach t : camera_tests\n> >                       dependencies : libcamera_dep,\n> >                       link_with : test_libraries,\n> >                       include_directories : test_includes_internal)\n> > -    test(t[0], exe, suite : 'camera', is_parallel : false)\n> > +    test(t[0], exe, suite : 'camera')\n> >  endforeach\n> > diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp\n> > index c834edc352f5d7d4..0fc3ded2545df2e1 100644\n> > --- a/test/controls/control_list.cpp\n> > +++ b/test/controls/control_list.cpp\n> > @@ -18,6 +18,10 @@ using namespace libcamera;\n> >  \n> >  class ControlListTest : public Test\n> >  {\n> > +public:\n> > +\tControlListTest()\n> > +\t\t: Test(VIMC) {}\n> > +\n> >  protected:\n> >  \tint init()\n> >  \t{\n> > diff --git a/test/controls/meson.build b/test/controls/meson.build\n> > index f4fc7b947dd9e2a6..724871e3136f369b 100644\n> > --- a/test/controls/meson.build\n> > +++ b/test/controls/meson.build\n> > @@ -9,5 +9,5 @@ foreach t : control_tests\n> >                       dependencies : libcamera_dep,\n> >                       link_with : test_libraries,\n> >                       include_directories : test_includes_internal)\n> > -    test(t[0], exe, suite : 'controls', is_parallel : false)\n> > +    test(t[0], exe, suite : 'controls')\n> >  endforeach\n> > diff --git a/test/libtest/test.cpp b/test/libtest/test.cpp\n> > index b119cf19273900d2..cf13fa42dd9fdc5a 100644\n> > --- a/test/libtest/test.cpp\n> > +++ b/test/libtest/test.cpp\n> > @@ -5,11 +5,15 @@\n> >   * test.cpp - libcamera test base class\n> >   */\n> >  \n> > +#include <fcntl.h>\n> > +#include <iostream>\n> >  #include <stdlib.h>\n> > +#include <sys/stat.h>\n> >  \n> >  #include \"test.h\"\n> >  \n> > -Test::Test()\n> > +Test::Test(TestResource resource)\n> > +\t: resource_(resource)\n> >  {\n> >  }\n> >  \n> > @@ -25,6 +29,9 @@ int Test::execute()\n> >  \tif (ret)\n> >  \t\treturn errno;\n> >  \n> > +\tif (!resourceLock())\n> > +\t\treturn TestFail;\n> > +\n> >  \tret = init();\n> >  \tif (ret)\n> >  \t\treturn ret;\n> > @@ -35,3 +42,31 @@ int Test::execute()\n> >  \n> >  \treturn ret;\n> >  }\n> > +\n> > +bool Test::resourceLock()\n> > +{\n> > +\tconst char *path;\n> > +\n> > +\tswitch (resource_) {\n> > +\tcase None:\n> > +\t\treturn true;\n> > +\tcase VIMC:\n> > +\t\tpath = \"/tmp/libcamera-test-vimc\";\n> > +\t\tbreak;\n> > +\tcase VIVID:\n> > +\t\tpath = \"/tmp/libcamera-test-vivid\";\n> > +\t\tbreak;\n> > +\t}\n> > +\n> > +\tint fd = ::open(path, O_CREAT | O_RDWR, 0666);\n> > +\tif (fd < 0) {\n> > +\t\tstd::cerr << \"Failed to open resource lockfile\" << std::endl;\n> > +\t\tperror(\"FOO\");\n> > +\t\treturn false;\n> > +\t}\n> > +\n> > +\tif (lockf(fd, F_LOCK, 0))\n> > +\t\treturn false;\n> \n> Is there a way that we can do this at the library level, perhaps within\n> our media controller layer?\n\nI wonder if we should rework this by making resources more explicitly\nmanaged. I especially foresee the need to lock multiple resources, so\nhandling this at the media device level could indeed be a good idea.\nWould it make sense to add a lock function that takes a list of media\ndevices and tries to lock them all ? We could avoid AB-BA deadlocks by\nsorting the resources in the lock function. I think I would prefer for\nthe lock function to be called explicitly in the init() or run()\nhandler, instead of modifying the base Test constructor. We could add a\nMediaResourceLocker (the name can be discussed) class that could serve\nas a base of all tests that require media devices and add the required\nfeatures there.\n\n> > +\n> > +\treturn true;\n> > +}\n> > diff --git a/test/libtest/test.h b/test/libtest/test.h\n> > index ec08bf97c03d563e..259fb589eb621489 100644\n> > --- a/test/libtest/test.h\n> > +++ b/test/libtest/test.h\n> > @@ -15,10 +15,16 @@ enum TestStatus {\n> >  \tTestSkip = 77,\n> >  };\n> >  \n> > +enum TestResource {\n> > +\tNone,\n> > +\tVIMC,\n> > +\tVIVID,\n> > +};\n> > +\n> >  class Test\n> >  {\n> >  public:\n> > -\tTest();\n> > +\tTest(TestResource resource = None);\n> >  \tvirtual ~Test();\n> >  \n> >  \tint execute();\n> > @@ -27,6 +33,13 @@ protected:\n> >  \tvirtual int init() { return 0; }\n> >  \tvirtual int run() = 0;\n> >  \tvirtual void cleanup() { }\n> > +\n> > +\tTestResource resource() { return resource_; }\n> > +\n> > +private:\n> > +\tbool resourceLock();\n> > +\n> > +\tTestResource resource_;\n> >  };\n> >  \n> >  #define TEST_REGISTER(klass)\t\t\t\t\t\t\\\n> > diff --git a/test/media_device/media_device_test.h b/test/media_device/media_device_test.h\n> > index cdbd14841d5ccca2..9e34ba86c397a99c 100644\n> > --- a/test/media_device/media_device_test.h\n> > +++ b/test/media_device/media_device_test.h\n> > @@ -20,7 +20,7 @@ class MediaDeviceTest : public Test\n> >  {\n> >  public:\n> >  \tMediaDeviceTest()\n> > -\t\t: media_(nullptr), enumerator_(nullptr) {}\n> > +\t\t: Test(VIMC), media_(nullptr), enumerator_(nullptr) {}\n> >  \n> >  protected:\n> >  \tint init();\n> > diff --git a/test/media_device/meson.build b/test/media_device/meson.build\n> > index 6a0e468434b58efe..6a5b24d27aa0f618 100644\n> > --- a/test/media_device/meson.build\n> > +++ b/test/media_device/meson.build\n> > @@ -18,5 +18,5 @@ foreach t : media_device_tests\n> >                       link_with : [test_libraries, lib_mdev_test],\n> >                       include_directories : test_includes_internal)\n> >  \n> > -    test(t[0], exe, suite : 'media_device', is_parallel : false)\n> > +    test(t[0], exe, suite : 'media_device')\n> >  endforeach\n> > diff --git a/test/v4l2_subdevice/meson.build b/test/v4l2_subdevice/meson.build\n> > index 0521984b2a787ab9..47676adbe5a33f7e 100644\n> > --- a/test/v4l2_subdevice/meson.build\n> > +++ b/test/v4l2_subdevice/meson.build\n> > @@ -8,5 +8,5 @@ foreach t : v4l2_subdevice_tests\n> >          dependencies : libcamera_dep,\n> >          link_with : test_libraries,\n> >          include_directories : test_includes_internal)\n> > -    test(t[0], exe, suite : 'v4l2_subdevice', is_parallel : false)\n> > +    test(t[0], exe, suite : 'v4l2_subdevice')\n> >  endforeach\n> > diff --git a/test/v4l2_subdevice/v4l2_subdevice_test.h b/test/v4l2_subdevice/v4l2_subdevice_test.h\n> > index 96646a155536aadf..a9002834bf8959b2 100644\n> > --- a/test/v4l2_subdevice/v4l2_subdevice_test.h\n> > +++ b/test/v4l2_subdevice/v4l2_subdevice_test.h\n> > @@ -21,7 +21,7 @@ class V4L2SubdeviceTest : public Test\n> >  {\n> >  public:\n> >  \tV4L2SubdeviceTest()\n> > -\t\t: scaler_(nullptr){};\n> > +\t\t: Test(VIMC), scaler_(nullptr){};\n> >  \n> >  protected:\n> >  \tint init() override;\n> > diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp\n> > index 1bc478fe8f8e1163..d619713257055fd5 100644\n> > --- a/test/v4l2_videodevice/buffer_sharing.cpp\n> > +++ b/test/v4l2_videodevice/buffer_sharing.cpp\n> > @@ -23,7 +23,7 @@ class BufferSharingTest : public V4L2VideoDeviceTest\n> >  {\n> >  public:\n> >  \tBufferSharingTest()\n> > -\t\t: V4L2VideoDeviceTest(\"vivid\", \"vivid-000-vid-cap\"),\n> > +\t\t: V4L2VideoDeviceTest(VIVID, \"vivid-000-vid-cap\"),\n> >  \t\t  output_(nullptr), framesCaptured_(0), framesOutput_(0) {}\n> >  \n> >  protected:\n> > diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp\n> > index cea4fffbf7a5603d..9ae8043b31a68f0e 100644\n> > --- a/test/v4l2_videodevice/capture_async.cpp\n> > +++ b/test/v4l2_videodevice/capture_async.cpp\n> > @@ -18,7 +18,7 @@ class CaptureAsyncTest : public V4L2VideoDeviceTest\n> >  {\n> >  public:\n> >  \tCaptureAsyncTest()\n> > -\t\t: V4L2VideoDeviceTest(\"vimc\", \"Raw Capture 0\"), frames(0) {}\n> > +\t\t: V4L2VideoDeviceTest(VIMC, \"Raw Capture 0\"), frames(0) {}\n> >  \n> >  \tvoid receiveBuffer(Buffer *buffer)\n> >  \t{\n> > diff --git a/test/v4l2_videodevice/double_open.cpp b/test/v4l2_videodevice/double_open.cpp\n> > index 5768d4043d0ba03f..ff1c7b136e803929 100644\n> > --- a/test/v4l2_videodevice/double_open.cpp\n> > +++ b/test/v4l2_videodevice/double_open.cpp\n> > @@ -15,7 +15,7 @@ class DoubleOpen : public V4L2VideoDeviceTest\n> >  {\n> >  public:\n> >  \tDoubleOpen()\n> > -\t\t: V4L2VideoDeviceTest(\"vimc\", \"Raw Capture 0\") {}\n> > +\t\t: V4L2VideoDeviceTest(VIMC, \"Raw Capture 0\") {}\n> >  protected:\n> >  \tint run()\n> >  \t{\n> > diff --git a/test/v4l2_videodevice/formats.cpp b/test/v4l2_videodevice/formats.cpp\n> > index ee7d357de0752cae..25cd147f804ba0d5 100644\n> > --- a/test/v4l2_videodevice/formats.cpp\n> > +++ b/test/v4l2_videodevice/formats.cpp\n> > @@ -19,7 +19,7 @@ class Format : public V4L2VideoDeviceTest\n> >  {\n> >  public:\n> >  \tFormat()\n> > -\t\t: V4L2VideoDeviceTest(\"vimc\", \"Raw Capture 0\") {}\n> > +\t\t: V4L2VideoDeviceTest(VIMC, \"Raw Capture 0\") {}\n> >  protected:\n> >  \tint run()\n> >  \t{\n> > diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build\n> > index 76be5e142bb63aa6..dc8204872efdc038 100644\n> > --- a/test/v4l2_videodevice/meson.build\n> > +++ b/test/v4l2_videodevice/meson.build\n> > @@ -14,5 +14,5 @@ foreach t : v4l2_videodevice_tests\n> >                       dependencies : libcamera_dep,\n> >                       link_with : test_libraries,\n> >                       include_directories : test_includes_internal)\n> > -    test(t[0], exe, suite : 'v4l2_videodevice', is_parallel : false)\n> > +    test(t[0], exe, suite : 'v4l2_videodevice')\n> >  endforeach\n> > diff --git a/test/v4l2_videodevice/request_buffers.cpp b/test/v4l2_videodevice/request_buffers.cpp\n> > index c4aedf7b3cd61e80..bbc4073c363c128a 100644\n> > --- a/test/v4l2_videodevice/request_buffers.cpp\n> > +++ b/test/v4l2_videodevice/request_buffers.cpp\n> > @@ -11,7 +11,7 @@ class RequestBuffersTest : public V4L2VideoDeviceTest\n> >  {\n> >  public:\n> >  \tRequestBuffersTest()\n> > -\t\t: V4L2VideoDeviceTest(\"vimc\", \"Raw Capture 0\") {}\n> > +\t\t: V4L2VideoDeviceTest(VIMC, \"Raw Capture 0\") {}\n> >  \n> >  protected:\n> >  \tint run()\n> > diff --git a/test/v4l2_videodevice/stream_on_off.cpp b/test/v4l2_videodevice/stream_on_off.cpp\n> > index 7664adc4c1f07046..fe021667c44559fa 100644\n> > --- a/test/v4l2_videodevice/stream_on_off.cpp\n> > +++ b/test/v4l2_videodevice/stream_on_off.cpp\n> > @@ -11,7 +11,7 @@ class StreamOnStreamOffTest : public V4L2VideoDeviceTest\n> >  {\n> >  public:\n> >  \tStreamOnStreamOffTest()\n> > -\t\t: V4L2VideoDeviceTest(\"vimc\", \"Raw Capture 0\") {}\n> > +\t\t: V4L2VideoDeviceTest(VIMC, \"Raw Capture 0\") {}\n> >  protected:\n> >  \tint run()\n> >  \t{\n> > diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp\n> > index b26d06ad45197c8f..aa39e455360c868c 100644\n> > --- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp\n> > +++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp\n> > @@ -28,6 +28,20 @@ bool exists(const std::string &path)\n> >  \n> >  int V4L2VideoDeviceTest::init()\n> >  {\n> > +\tconst char *driver;\n> > +\n> > +\tswitch (resource()) {\n> > +\tcase VIMC:\n> > +\t\tdriver = \"vimc\";\n> > +\t\tbreak;\n> > +\tcase VIVID:\n> > +\t\tdriver = \"vivid\";\n> > +\t\tbreak;\n> > +\tdefault:\n> > +\t\tstd::cerr << \"Unkown driver for resource\" << std::endl;\n> \n> s/Unkown/Unknown/\n> \n> > +\t\treturn TestFail;\n> > +\t}\n> > +\n> >  \tenumerator_ = DeviceEnumerator::create();\n> >  \tif (!enumerator_) {\n> >  \t\tcerr << \"Failed to create device enumerator\" << endl;\n> > @@ -39,7 +53,7 @@ int V4L2VideoDeviceTest::init()\n> >  \t\treturn TestFail;\n> >  \t}\n> >  \n> > -\tDeviceMatch dm(driver_);\n> > +\tDeviceMatch dm(driver);\n> >  \tdm.add(entity_);\n> >  \n> >  \tmedia_ = enumerator_->search(dm);\n> > diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.h b/test/v4l2_videodevice/v4l2_videodevice_test.h\n> > index 3321b5a4f98fdb1d..b90a5fca6ce48fdd 100644\n> > --- a/test/v4l2_videodevice/v4l2_videodevice_test.h\n> > +++ b/test/v4l2_videodevice/v4l2_videodevice_test.h\n> > @@ -22,8 +22,8 @@ using namespace libcamera;\n> >  class V4L2VideoDeviceTest : public Test\n> >  {\n> >  public:\n> > -\tV4L2VideoDeviceTest(const char *driver, const char *entity)\n> > -\t\t: driver_(driver), entity_(entity), capture_(nullptr)\n> > +\tV4L2VideoDeviceTest(TestResource resource, const char *entity)\n> > +\t\t: Test(resource), entity_(entity), capture_(nullptr)\n> >  \t{\n> >  \t}\n> >","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B08C661569\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  4 Jul 2019 14:50:22 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1C56B24B;\n\tThu,  4 Jul 2019 14:50:22 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1562244622;\n\tbh=Cq8D8epyJF5uhjkt/pVcxzXiSm5tS4v6tFpsA5fpNJs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iUUQVarZ6Aloll28js/KT09JafyAQ2DbLClUMe2MgPmFn/5oesGwBVS6LNpaxggn5\n\txQcoCOsMOqfkjPrCDNCndNM8VXS30A2Fug6Ag/idvos9foTjDxPQQEpjGUxAS1TkQh\n\tCgB8pSj18blL2pqZgmuVHpaaTsFJDAzApNX3J8jE=","Date":"Thu, 4 Jul 2019 15:49:53 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20190704124953.GI6569@pendragon.ideasonboard.com>","References":"<20190704013004.14600-1-niklas.soderlund@ragnatech.se>\n\t<4550bb7a-50a3-e477-92a4-751ac6ea0a25@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<4550bb7a-50a3-e477-92a4-751ac6ea0a25@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [RFC] test: Move resource locking to test\n\tcases","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 04 Jul 2019 12:50:22 -0000"}},{"id":2163,"web_url":"https://patchwork.libcamera.org/comment/2163/","msgid":"<20190704144014.GC17685@bigcity.dyn.berto.se>","date":"2019-07-04T14:40:14","subject":"Re: [libcamera-devel] [RFC] test: Move resource locking to test\n\tcases","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent, Kieran,\n\nThanks for your feedback.\n\nOn 2019-07-04 15:49:53 +0300, Laurent Pinchart wrote:\n> Hello,\n> \n> > Hi Niklas,\n> > \n> > On 04/07/2019 02:30, Niklas Söderlund wrote:\n> > > Instead of blocking any test which require access to a kernel resource\n> > > (vimc or vivid) from running in parallel move the resource locking into\n> > > test library and allow meson to execute them in parallel.\n> > > \n> > > Resource locking is achieve using a lockfile for each resource in /tmp\n> \n> s/achieve/achieved/\n> \n> > > and lockf(). The call to lockf() blocks until it can acquire the\n> > > resource and it keeps to lock until the test executable terminates\n> \n> s/to lock/the lock/\n> s/terminates/terminates,/\n> \n> > > closing the file descriptor to the lock file  and freeing the lock. This\n> \n> s/  / /\n> \n> > > design ensures the process never deadlocks. The reason a separate lock\n> \n> s/process never deadlocks/test processes never deadlock/\n> \n> This is however not entirely true, tests that need to lock multiple\n> devices (for instance to test buffer sharing) may deadlock if they don't\n> take the locks in the same order.\n> \n> Edit: I've now read the rest of the series and I see you're not locking\n> individual devices, but you lock at the vimc or vivid level. There\n> should thus be no risk of deadlock indeed.\n> \n> > > file is needed instead of locking the media device associated with the\n> > > resource is that such a locking scheme is already used by libcamera\n> > > itself.\n> > \n> > Can't we instead request the lock from the libcamera locking then ?\n> > \n> > > With this change I cut down the runtime of tests by 50%. The largest\n> > > increase is from allowing self-contained tests in parallel with tests\n> > > which require a kernel resource. This is because most of the tests who\n> > > needs a kernel resource needs vimc so there is heavy contention for that\n> > > single resource and little gain from meson to execute them in parallel.\n> > > \n> > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > > ---\n> > >  test/camera/camera_test.h                     |  2 +-\n> > >  test/camera/meson.build                       |  2 +-\n> > >  test/controls/control_list.cpp                |  4 ++\n> > >  test/controls/meson.build                     |  2 +-\n> > >  test/libtest/test.cpp                         | 37 ++++++++++++++++++-\n> > >  test/libtest/test.h                           | 15 +++++++-\n> > >  test/media_device/media_device_test.h         |  2 +-\n> > >  test/media_device/meson.build                 |  2 +-\n> > >  test/v4l2_subdevice/meson.build               |  2 +-\n> > >  test/v4l2_subdevice/v4l2_subdevice_test.h     |  2 +-\n> > >  test/v4l2_videodevice/buffer_sharing.cpp      |  2 +-\n> > >  test/v4l2_videodevice/capture_async.cpp       |  2 +-\n> > >  test/v4l2_videodevice/double_open.cpp         |  2 +-\n> > >  test/v4l2_videodevice/formats.cpp             |  2 +-\n> > >  test/v4l2_videodevice/meson.build             |  2 +-\n> > >  test/v4l2_videodevice/request_buffers.cpp     |  2 +-\n> > >  test/v4l2_videodevice/stream_on_off.cpp       |  2 +-\n> > >  .../v4l2_videodevice_test.cpp                 | 16 +++++++-\n> > >  test/v4l2_videodevice/v4l2_videodevice_test.h |  4 +-\n> > >  19 files changed, 85 insertions(+), 19 deletions(-)\n> > > \n> > > diff --git a/test/camera/camera_test.h b/test/camera/camera_test.h\n> > > index ffc8a485bfaff836..0f3c15cc3dd34f3c 100644\n> > > --- a/test/camera/camera_test.h\n> > > +++ b/test/camera/camera_test.h\n> > > @@ -17,7 +17,7 @@ class CameraTest : public Test\n> > >  {\n> > >  public:\n> > >  \tCameraTest()\n> > > -\t\t: cm_(nullptr) {}\n> > > +\t\t: Test(VIMC), cm_(nullptr) {}\n> > >  \n> > >  protected:\n> > >  \tint init();\n> > > diff --git a/test/camera/meson.build b/test/camera/meson.build\n> > > index 35e97ce5de1a72ca..b10b1e76aaec0233 100644\n> > > --- a/test/camera/meson.build\n> > > +++ b/test/camera/meson.build\n> > > @@ -12,5 +12,5 @@ foreach t : camera_tests\n> > >                       dependencies : libcamera_dep,\n> > >                       link_with : test_libraries,\n> > >                       include_directories : test_includes_internal)\n> > > -    test(t[0], exe, suite : 'camera', is_parallel : false)\n> > > +    test(t[0], exe, suite : 'camera')\n> > >  endforeach\n> > > diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp\n> > > index c834edc352f5d7d4..0fc3ded2545df2e1 100644\n> > > --- a/test/controls/control_list.cpp\n> > > +++ b/test/controls/control_list.cpp\n> > > @@ -18,6 +18,10 @@ using namespace libcamera;\n> > >  \n> > >  class ControlListTest : public Test\n> > >  {\n> > > +public:\n> > > +\tControlListTest()\n> > > +\t\t: Test(VIMC) {}\n> > > +\n> > >  protected:\n> > >  \tint init()\n> > >  \t{\n> > > diff --git a/test/controls/meson.build b/test/controls/meson.build\n> > > index f4fc7b947dd9e2a6..724871e3136f369b 100644\n> > > --- a/test/controls/meson.build\n> > > +++ b/test/controls/meson.build\n> > > @@ -9,5 +9,5 @@ foreach t : control_tests\n> > >                       dependencies : libcamera_dep,\n> > >                       link_with : test_libraries,\n> > >                       include_directories : test_includes_internal)\n> > > -    test(t[0], exe, suite : 'controls', is_parallel : false)\n> > > +    test(t[0], exe, suite : 'controls')\n> > >  endforeach\n> > > diff --git a/test/libtest/test.cpp b/test/libtest/test.cpp\n> > > index b119cf19273900d2..cf13fa42dd9fdc5a 100644\n> > > --- a/test/libtest/test.cpp\n> > > +++ b/test/libtest/test.cpp\n> > > @@ -5,11 +5,15 @@\n> > >   * test.cpp - libcamera test base class\n> > >   */\n> > >  \n> > > +#include <fcntl.h>\n> > > +#include <iostream>\n> > >  #include <stdlib.h>\n> > > +#include <sys/stat.h>\n> > >  \n> > >  #include \"test.h\"\n> > >  \n> > > -Test::Test()\n> > > +Test::Test(TestResource resource)\n> > > +\t: resource_(resource)\n> > >  {\n> > >  }\n> > >  \n> > > @@ -25,6 +29,9 @@ int Test::execute()\n> > >  \tif (ret)\n> > >  \t\treturn errno;\n> > >  \n> > > +\tif (!resourceLock())\n> > > +\t\treturn TestFail;\n> > > +\n> > >  \tret = init();\n> > >  \tif (ret)\n> > >  \t\treturn ret;\n> > > @@ -35,3 +42,31 @@ int Test::execute()\n> > >  \n> > >  \treturn ret;\n> > >  }\n> > > +\n> > > +bool Test::resourceLock()\n> > > +{\n> > > +\tconst char *path;\n> > > +\n> > > +\tswitch (resource_) {\n> > > +\tcase None:\n> > > +\t\treturn true;\n> > > +\tcase VIMC:\n> > > +\t\tpath = \"/tmp/libcamera-test-vimc\";\n> > > +\t\tbreak;\n> > > +\tcase VIVID:\n> > > +\t\tpath = \"/tmp/libcamera-test-vivid\";\n> > > +\t\tbreak;\n> > > +\t}\n> > > +\n> > > +\tint fd = ::open(path, O_CREAT | O_RDWR, 0666);\n> > > +\tif (fd < 0) {\n> > > +\t\tstd::cerr << \"Failed to open resource lockfile\" << std::endl;\n> > > +\t\tperror(\"FOO\");\n> > > +\t\treturn false;\n> > > +\t}\n> > > +\n> > > +\tif (lockf(fd, F_LOCK, 0))\n> > > +\t\treturn false;\n> > \n> > Is there a way that we can do this at the library level, perhaps within\n> > our media controller layer?\n\nI don't feel it's a good idea to add this functionality in libcameras \nmedia controller layer as it's only use is for our testcases. We also \nhave tests which do not start the camera manager and access the v4l2 \ndevices directly to unit test individual libcamera objects.\n\n> \n> I wonder if we should rework this by making resources more explicitly\n> managed. I especially foresee the need to lock multiple resources, so\n> handling this at the media device level could indeed be a good idea.\n\nWe can not place a lockf() call on the media devices as this is already \nused inside libcamera to manage concurrent libcamera instances.\n\n> Would it make sense to add a lock function that takes a list of media\n> devices and tries to lock them all ? We could avoid AB-BA deadlocks by\n> sorting the resources in the lock function. I think I would prefer for\n> the lock function to be called explicitly in the init() or run()\n> handler, instead of modifying the base Test constructor. We could add a\n> MediaResourceLocker (the name can be discussed) class that could serve\n> as a base of all tests that require media devices and add the required\n> features there.\n\nThis is indeed an idea which could be explored further. I will see what \nI can dream up for a v1.\n\n> \n> > > +\n> > > +\treturn true;\n> > > +}\n> > > diff --git a/test/libtest/test.h b/test/libtest/test.h\n> > > index ec08bf97c03d563e..259fb589eb621489 100644\n> > > --- a/test/libtest/test.h\n> > > +++ b/test/libtest/test.h\n> > > @@ -15,10 +15,16 @@ enum TestStatus {\n> > >  \tTestSkip = 77,\n> > >  };\n> > >  \n> > > +enum TestResource {\n> > > +\tNone,\n> > > +\tVIMC,\n> > > +\tVIVID,\n> > > +};\n> > > +\n> > >  class Test\n> > >  {\n> > >  public:\n> > > -\tTest();\n> > > +\tTest(TestResource resource = None);\n> > >  \tvirtual ~Test();\n> > >  \n> > >  \tint execute();\n> > > @@ -27,6 +33,13 @@ protected:\n> > >  \tvirtual int init() { return 0; }\n> > >  \tvirtual int run() = 0;\n> > >  \tvirtual void cleanup() { }\n> > > +\n> > > +\tTestResource resource() { return resource_; }\n> > > +\n> > > +private:\n> > > +\tbool resourceLock();\n> > > +\n> > > +\tTestResource resource_;\n> > >  };\n> > >  \n> > >  #define TEST_REGISTER(klass)\t\t\t\t\t\t\\\n> > > diff --git a/test/media_device/media_device_test.h b/test/media_device/media_device_test.h\n> > > index cdbd14841d5ccca2..9e34ba86c397a99c 100644\n> > > --- a/test/media_device/media_device_test.h\n> > > +++ b/test/media_device/media_device_test.h\n> > > @@ -20,7 +20,7 @@ class MediaDeviceTest : public Test\n> > >  {\n> > >  public:\n> > >  \tMediaDeviceTest()\n> > > -\t\t: media_(nullptr), enumerator_(nullptr) {}\n> > > +\t\t: Test(VIMC), media_(nullptr), enumerator_(nullptr) {}\n> > >  \n> > >  protected:\n> > >  \tint init();\n> > > diff --git a/test/media_device/meson.build b/test/media_device/meson.build\n> > > index 6a0e468434b58efe..6a5b24d27aa0f618 100644\n> > > --- a/test/media_device/meson.build\n> > > +++ b/test/media_device/meson.build\n> > > @@ -18,5 +18,5 @@ foreach t : media_device_tests\n> > >                       link_with : [test_libraries, lib_mdev_test],\n> > >                       include_directories : test_includes_internal)\n> > >  \n> > > -    test(t[0], exe, suite : 'media_device', is_parallel : false)\n> > > +    test(t[0], exe, suite : 'media_device')\n> > >  endforeach\n> > > diff --git a/test/v4l2_subdevice/meson.build b/test/v4l2_subdevice/meson.build\n> > > index 0521984b2a787ab9..47676adbe5a33f7e 100644\n> > > --- a/test/v4l2_subdevice/meson.build\n> > > +++ b/test/v4l2_subdevice/meson.build\n> > > @@ -8,5 +8,5 @@ foreach t : v4l2_subdevice_tests\n> > >          dependencies : libcamera_dep,\n> > >          link_with : test_libraries,\n> > >          include_directories : test_includes_internal)\n> > > -    test(t[0], exe, suite : 'v4l2_subdevice', is_parallel : false)\n> > > +    test(t[0], exe, suite : 'v4l2_subdevice')\n> > >  endforeach\n> > > diff --git a/test/v4l2_subdevice/v4l2_subdevice_test.h b/test/v4l2_subdevice/v4l2_subdevice_test.h\n> > > index 96646a155536aadf..a9002834bf8959b2 100644\n> > > --- a/test/v4l2_subdevice/v4l2_subdevice_test.h\n> > > +++ b/test/v4l2_subdevice/v4l2_subdevice_test.h\n> > > @@ -21,7 +21,7 @@ class V4L2SubdeviceTest : public Test\n> > >  {\n> > >  public:\n> > >  \tV4L2SubdeviceTest()\n> > > -\t\t: scaler_(nullptr){};\n> > > +\t\t: Test(VIMC), scaler_(nullptr){};\n> > >  \n> > >  protected:\n> > >  \tint init() override;\n> > > diff --git a/test/v4l2_videodevice/buffer_sharing.cpp b/test/v4l2_videodevice/buffer_sharing.cpp\n> > > index 1bc478fe8f8e1163..d619713257055fd5 100644\n> > > --- a/test/v4l2_videodevice/buffer_sharing.cpp\n> > > +++ b/test/v4l2_videodevice/buffer_sharing.cpp\n> > > @@ -23,7 +23,7 @@ class BufferSharingTest : public V4L2VideoDeviceTest\n> > >  {\n> > >  public:\n> > >  \tBufferSharingTest()\n> > > -\t\t: V4L2VideoDeviceTest(\"vivid\", \"vivid-000-vid-cap\"),\n> > > +\t\t: V4L2VideoDeviceTest(VIVID, \"vivid-000-vid-cap\"),\n> > >  \t\t  output_(nullptr), framesCaptured_(0), framesOutput_(0) {}\n> > >  \n> > >  protected:\n> > > diff --git a/test/v4l2_videodevice/capture_async.cpp b/test/v4l2_videodevice/capture_async.cpp\n> > > index cea4fffbf7a5603d..9ae8043b31a68f0e 100644\n> > > --- a/test/v4l2_videodevice/capture_async.cpp\n> > > +++ b/test/v4l2_videodevice/capture_async.cpp\n> > > @@ -18,7 +18,7 @@ class CaptureAsyncTest : public V4L2VideoDeviceTest\n> > >  {\n> > >  public:\n> > >  \tCaptureAsyncTest()\n> > > -\t\t: V4L2VideoDeviceTest(\"vimc\", \"Raw Capture 0\"), frames(0) {}\n> > > +\t\t: V4L2VideoDeviceTest(VIMC, \"Raw Capture 0\"), frames(0) {}\n> > >  \n> > >  \tvoid receiveBuffer(Buffer *buffer)\n> > >  \t{\n> > > diff --git a/test/v4l2_videodevice/double_open.cpp b/test/v4l2_videodevice/double_open.cpp\n> > > index 5768d4043d0ba03f..ff1c7b136e803929 100644\n> > > --- a/test/v4l2_videodevice/double_open.cpp\n> > > +++ b/test/v4l2_videodevice/double_open.cpp\n> > > @@ -15,7 +15,7 @@ class DoubleOpen : public V4L2VideoDeviceTest\n> > >  {\n> > >  public:\n> > >  \tDoubleOpen()\n> > > -\t\t: V4L2VideoDeviceTest(\"vimc\", \"Raw Capture 0\") {}\n> > > +\t\t: V4L2VideoDeviceTest(VIMC, \"Raw Capture 0\") {}\n> > >  protected:\n> > >  \tint run()\n> > >  \t{\n> > > diff --git a/test/v4l2_videodevice/formats.cpp b/test/v4l2_videodevice/formats.cpp\n> > > index ee7d357de0752cae..25cd147f804ba0d5 100644\n> > > --- a/test/v4l2_videodevice/formats.cpp\n> > > +++ b/test/v4l2_videodevice/formats.cpp\n> > > @@ -19,7 +19,7 @@ class Format : public V4L2VideoDeviceTest\n> > >  {\n> > >  public:\n> > >  \tFormat()\n> > > -\t\t: V4L2VideoDeviceTest(\"vimc\", \"Raw Capture 0\") {}\n> > > +\t\t: V4L2VideoDeviceTest(VIMC, \"Raw Capture 0\") {}\n> > >  protected:\n> > >  \tint run()\n> > >  \t{\n> > > diff --git a/test/v4l2_videodevice/meson.build b/test/v4l2_videodevice/meson.build\n> > > index 76be5e142bb63aa6..dc8204872efdc038 100644\n> > > --- a/test/v4l2_videodevice/meson.build\n> > > +++ b/test/v4l2_videodevice/meson.build\n> > > @@ -14,5 +14,5 @@ foreach t : v4l2_videodevice_tests\n> > >                       dependencies : libcamera_dep,\n> > >                       link_with : test_libraries,\n> > >                       include_directories : test_includes_internal)\n> > > -    test(t[0], exe, suite : 'v4l2_videodevice', is_parallel : false)\n> > > +    test(t[0], exe, suite : 'v4l2_videodevice')\n> > >  endforeach\n> > > diff --git a/test/v4l2_videodevice/request_buffers.cpp b/test/v4l2_videodevice/request_buffers.cpp\n> > > index c4aedf7b3cd61e80..bbc4073c363c128a 100644\n> > > --- a/test/v4l2_videodevice/request_buffers.cpp\n> > > +++ b/test/v4l2_videodevice/request_buffers.cpp\n> > > @@ -11,7 +11,7 @@ class RequestBuffersTest : public V4L2VideoDeviceTest\n> > >  {\n> > >  public:\n> > >  \tRequestBuffersTest()\n> > > -\t\t: V4L2VideoDeviceTest(\"vimc\", \"Raw Capture 0\") {}\n> > > +\t\t: V4L2VideoDeviceTest(VIMC, \"Raw Capture 0\") {}\n> > >  \n> > >  protected:\n> > >  \tint run()\n> > > diff --git a/test/v4l2_videodevice/stream_on_off.cpp b/test/v4l2_videodevice/stream_on_off.cpp\n> > > index 7664adc4c1f07046..fe021667c44559fa 100644\n> > > --- a/test/v4l2_videodevice/stream_on_off.cpp\n> > > +++ b/test/v4l2_videodevice/stream_on_off.cpp\n> > > @@ -11,7 +11,7 @@ class StreamOnStreamOffTest : public V4L2VideoDeviceTest\n> > >  {\n> > >  public:\n> > >  \tStreamOnStreamOffTest()\n> > > -\t\t: V4L2VideoDeviceTest(\"vimc\", \"Raw Capture 0\") {}\n> > > +\t\t: V4L2VideoDeviceTest(VIMC, \"Raw Capture 0\") {}\n> > >  protected:\n> > >  \tint run()\n> > >  \t{\n> > > diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.cpp b/test/v4l2_videodevice/v4l2_videodevice_test.cpp\n> > > index b26d06ad45197c8f..aa39e455360c868c 100644\n> > > --- a/test/v4l2_videodevice/v4l2_videodevice_test.cpp\n> > > +++ b/test/v4l2_videodevice/v4l2_videodevice_test.cpp\n> > > @@ -28,6 +28,20 @@ bool exists(const std::string &path)\n> > >  \n> > >  int V4L2VideoDeviceTest::init()\n> > >  {\n> > > +\tconst char *driver;\n> > > +\n> > > +\tswitch (resource()) {\n> > > +\tcase VIMC:\n> > > +\t\tdriver = \"vimc\";\n> > > +\t\tbreak;\n> > > +\tcase VIVID:\n> > > +\t\tdriver = \"vivid\";\n> > > +\t\tbreak;\n> > > +\tdefault:\n> > > +\t\tstd::cerr << \"Unkown driver for resource\" << std::endl;\n> > \n> > s/Unkown/Unknown/\n> > \n> > > +\t\treturn TestFail;\n> > > +\t}\n> > > +\n> > >  \tenumerator_ = DeviceEnumerator::create();\n> > >  \tif (!enumerator_) {\n> > >  \t\tcerr << \"Failed to create device enumerator\" << endl;\n> > > @@ -39,7 +53,7 @@ int V4L2VideoDeviceTest::init()\n> > >  \t\treturn TestFail;\n> > >  \t}\n> > >  \n> > > -\tDeviceMatch dm(driver_);\n> > > +\tDeviceMatch dm(driver);\n> > >  \tdm.add(entity_);\n> > >  \n> > >  \tmedia_ = enumerator_->search(dm);\n> > > diff --git a/test/v4l2_videodevice/v4l2_videodevice_test.h b/test/v4l2_videodevice/v4l2_videodevice_test.h\n> > > index 3321b5a4f98fdb1d..b90a5fca6ce48fdd 100644\n> > > --- a/test/v4l2_videodevice/v4l2_videodevice_test.h\n> > > +++ b/test/v4l2_videodevice/v4l2_videodevice_test.h\n> > > @@ -22,8 +22,8 @@ using namespace libcamera;\n> > >  class V4L2VideoDeviceTest : public Test\n> > >  {\n> > >  public:\n> > > -\tV4L2VideoDeviceTest(const char *driver, const char *entity)\n> > > -\t\t: driver_(driver), entity_(entity), capture_(nullptr)\n> > > +\tV4L2VideoDeviceTest(TestResource resource, const char *entity)\n> > > +\t\t: Test(resource), entity_(entity), capture_(nullptr)\n> > >  \t{\n> > >  \t}\n> > >  \n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 177356156B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  4 Jul 2019 16:40:17 +0200 (CEST)","by mail-lj1-x243.google.com with SMTP id 131so6409399ljf.4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 04 Jul 2019 07:40:17 -0700 (PDT)","from localhost (customer-145-14-112-32.stosn.net. [145.14.112.32])\n\tby smtp.gmail.com with ESMTPSA id\n\tu9sm924640lfb.38.2019.07.04.07.40.15\n\t(version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256);\n\tThu, 04 Jul 2019 07:40:15 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=k7THSvVWPoYcHL5EtMwsSIENDUA+BJ1Ewa/TBmT7wFo=;\n\tb=V8GgBcroGhoOS+elgk+HRxB4Xc/exwynna4orZMd8sVXUP3urXkozIdKHbs5LUFPzr\n\t3amC/Mwye4mSe45Paqthdrl3T2QFP4ciS0hEZIlN2ZHgQeASLvCP5oro1qLumoNgWeWs\n\todP/bQ8psX/VsLTTRyYnHYDzmI62N/ovf5vbEwOyapB02h6+TrHphVcelG2m5yEDDrjK\n\tPFWrslBYEv5Ez2EfZoEHm5qzBN1DoduEbmYMH1vMr/xpnT3iFccF5Cl7obU4z7sao7/d\n\tq66IdifD8+rd/78Xu8CJSr++VeYMRYgVGahFpqMwSpIfRqJUdOnQNLPHxqOin3fKTQr+\n\tO9Hg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=k7THSvVWPoYcHL5EtMwsSIENDUA+BJ1Ewa/TBmT7wFo=;\n\tb=elr/hjpNRGGL2sjI5Kp4RO38fuOyIk3JG33liuvmzGdPUcuxkWbXAlo9tpEI6Bq+lA\n\t5nAe86oJhM3Rw6+Chrrw46RwhI3Mi7zE2or2UeJbK9zhoTi013jgwnXxC6zpE7UDKWKt\n\tWK2onPwra9dl90nRN2hxoMjLyBvLUWxmSl764iNaTO2G1poQzo/pXBVi9siOwBpZf/qH\n\tQKyataTCMWKn5JtLSc5zVL3B6AyKoeVZ4BLAyS7/E5zPcRKRaBlSwLHzIGmhpJ9AhBUf\n\tQWF9JuhC68N19LiQwV1I7bEamBl3ZlEgLPGTvBN0g7iBQKiXgaS/qfL/9/eVGYtlxREe\n\tq+EA==","X-Gm-Message-State":"APjAAAUUrGLjamPa8hmVwW9rjC+ZNeDGyEDB1wUKTsJx1M0hLiOiwkL8\n\tdbXQfiG5nLxvOtTiYdGvDjSefg==","X-Google-Smtp-Source":"APXvYqySjdZUx+jnwTkge7lvMMycLCqhAF3/HdrOOgNdeVZmhlRgbBYsPVC65T1zi/rmR0YunRAGiQ==","X-Received":"by 2002:a2e:9a49:: with SMTP id k9mr25016154ljj.78.1562251216235;\n\tThu, 04 Jul 2019 07:40:16 -0700 (PDT)","Date":"Thu, 4 Jul 2019 16:40:14 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20190704144014.GC17685@bigcity.dyn.berto.se>","References":"<20190704013004.14600-1-niklas.soderlund@ragnatech.se>\n\t<4550bb7a-50a3-e477-92a4-751ac6ea0a25@ideasonboard.com>\n\t<20190704124953.GI6569@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190704124953.GI6569@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [RFC] test: Move resource locking to test\n\tcases","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Thu, 04 Jul 2019 14:40:17 -0000"}}]