Re: [RFC PATCH v2] dm ioctl: fix erroneous EINVAL when signaled

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dne 23. 07. 24 v 14:51 Mikulas Patocka napsal(a):


On Wed, 17 Jul 2024, Khazhismel Kumykov wrote:

do_resume when loading a new map first calls dm_suspend, which could
silently fail. When we proceeded to dm_swap_table, we would bail out
with EINVAL. Instead, attempt to restore new_map and return ERESTARTSYS
when signaled.

Signed-off-by: Khazhismel Kumykov <khazhy@xxxxxxxxxx>
---
  drivers/md/dm-ioctl.c | 23 +++++++++++++++++++++--
  1 file changed, 21 insertions(+), 2 deletions(-)

v2: don't leak new_map if we can't assign it back to hc.

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index c2c07bfa6471..0591455ad63c 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1181,8 +1181,27 @@ static int do_resume(struct dm_ioctl *param)
  			suspend_flags &= ~DM_SUSPEND_LOCKFS_FLAG;
  		if (param->flags & DM_NOFLUSH_FLAG)
  			suspend_flags |= DM_SUSPEND_NOFLUSH_FLAG;
-		if (!dm_suspended_md(md))
-			dm_suspend(md, suspend_flags);
+		if (!dm_suspended_md(md)) {
+			r = dm_suspend(md, suspend_flags);
+			if (r == -EINTR)
+				r = -ERESTARTSYS;

I'd like to ask why the "EINTR -> ERESTARTSYS" conversion is here and why
it isn't in dm_suspend?

What do libdevmapper+lvm maintainers think about it? Does lvm hadle EINTR
by restarting the ioctl syscall? Should we return ERESTARTSYS when suspend
is interrupted?

In general - with suspend failures - we are just stopping whole operation - and restoring previous state - so user can run operation again.

There is no special check for exact reason of ioctl failure.

Regards

Zdenek





[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux