[libcamera-devel] [PATCH v2] libcamera: v4l2_device: openat(2) with O_CLOEXEC to cleanup after exec(3)

[Laurent Pinchart]

Hi Elias,

One more comment, I'd highly recommend stopping the cameras and camera
manager before calling exec(), as there may be more side effects due to
objects not being destroyed.

On Sun, Mar 26, 2023 at 12:03:25PM +0300, Laurent Pinchart via libcamera-devel wrote:
> On Thu, Mar 23, 2023 at 08:05:00AM -0600, Elias Naur wrote:
> > On Mon, 20 Mar 2023 at 17:56, Laurent Pinchart wrote:
> > > On Sun, Mar 12, 2023 at 03:22:05PM -0600, Elias Naur via libcamera-devel wrote:
> > > > It's generally a good idea to openat(2) with O_CLOEXEC, but this patch
> > > > also fixes a real (corner-)case: I have an excutable that (1) uses
> > > > v4l2-compat to drive a RPi camera, (2) self-updates through exec(3).
> > > > Without O_CLOEXEC of the kernel devices, an update while the
> > > > camera is opened will result in -EBUSY errors when the update tries to
> > > > open the camera.
> > >
> > > I agree about the change, but I wonder what returns -EBUSY, as V4L2
> > > allows opening device nodes multiple times. Do you get a -EBUSY error
> > > from V4L2Device::open() after exec(), or from a different location ? If
> > > the error comes from V4L2Device::open(), what device does it come from ?
> > 
> > Efter exec'ing an update while the camera device is open, the next
> > open results in
> > 
> > [91]  INFO RPI raspberrypi.cpp:1487 Registered camera
> > /base/soc/i2c0mux/i2c at 1/ov5647 at 36 to Unicam device /dev/media3 and ISP
> > device /dev/media1
> > [89]  INFO Camera camera.cpp:1028 configuring streams: (0) 960x960-YUV420
> > [91] ERROR V4L2 v4l2_videodevice.cpp:1047 /dev/video0[149:cap]: Unable
> > to set format: Resource busy
> 
> Ah ok, the error comes from setFormat(), not open(). That makes sense.
> Thanks, that's the information I was looking for. I'll update the commit
> message to record this information, as follows:
> 
> When an executable using libcamera calls exec(3) while a camera is in
> use, file descriptors corresponding to the V4L2 video devices are kept
> open has they have been created without O_CLOEXEC. This results in the
> video devices staying busy, preventing the new executable from using
> them:
> 
> [91] ERROR V4L2 v4l2_videodevice.cpp:1047 /dev/video0[149:cap]: Unableto set format: Resource busy
> 
> Fix this by opening video devices with O_CLOEXEC, which is generally a
> good idea in libraries.
> 
> > > > It's generally a good idea to openat(2) with O_CLOEXEC, but this patch
> > > > also fixes a real (corner-)case: I have an excutable that (1) uses
> > > > v4l2-compat to drive a RPi camera, (2) self-updates through exec(3).
> > > > Without O_CLOEXEC of the kernel devices, an update while the
> > > > camera is opened will result in -EBUSY errors when the update tries to
> > > > open the camera.
> 
> > Let me know if you need more.

-- 
Regards,

Laurent Pinchart