Redundant partitions for sparc image loop mounts

[sidt4]

On debian unstable / udisks-2.10.0-3:

$ udisksctl loop-setup -f  ./ubuntu.com/ubuntu-7.10-server-sparc.iso 

$ df -h
/dev/loop0p1    473M  473M     0 100% /media/sidt/Ubuntu-Server 7.10 sparc
/dev/loop0p3    473M  473M     0 100% /media/sidt/Ubuntu-Server 7.10 sparc1
/dev/loop0p5    473M  473M     0 100% /media/sidt/Ubuntu-Server 7.10 sparc2
/dev/loop0p2    473M  473M     0 100% /media/sidt/Ubuntu-Server 7.10 sparc3
/dev/loop0p4    473M  473M     0 100% /media/sidt/Ubuntu-Server 7.10 sparc4
/dev/loop0p6    473M  473M     0 100% /media/sidt/Ubuntu-Server 7.10 sparc5
/dev/loop0p7    473M  473M     0 100% /media/sidt/Ubuntu-Server 7.10 sparc6
/dev/loop0p8    473M  473M     0 100% /media/sidt/Ubuntu-Server 7.10 sparc7

# mount -o loop ./ubuntu.com/ubuntu-7.10-server-sparc.iso /tmp/sparc

$ df -h
/dev/loop1      473M  473M     0 100% /tmp/sparc

Reference files:

• https://old-releases.ubuntu.com/releases/dapper/ubuntu-6.06-server-sparc.iso
• https://old-releases.ubuntu.com/releases/edgy/ubuntu-6.10-server-sparc.iso
• https://old-releases.ubuntu.com/releases/feisty/ubuntu-7.04-server-sparc.iso
• https://old-releases.ubuntu.com/releases/gutsy/ubuntu-7.10-server-sparc.iso

[vojtechtrefny]

udisksctl loop-setup by default enables partition scan, this is equivalent of running losetup with -P/--partscan. So it's kernel, not udisks who sees the partitions (fdisk sees the partitions as well, partition tables on ISO images are often very complicated, so this might be intentional, but it's possible this is a bug in the Sun partition table support). We have an option to disable parscan in the API, but it's not propagated to udisksctl so that might be one possibility for improvement in udisks.

[sidt4]

Below patch works as expected.

diff --git a/tools/udisksctl.c b/tools/udisksctl.c
index fad85e94..7d207f98 100644
--- a/tools/udisksctl.c
+++ b/tools/udisksctl.c
@@ -1402,6 +1402,7 @@ static gboolean opt_loop_no_user_interaction = FALSE;
 static gboolean opt_loop_read_only = FALSE;
 static gint64   opt_loop_offset = 0;
 static gint64   opt_loop_size = 0;
+static gboolean opt_loop_no_partition_scan = FALSE;
 
 static const GOptionEntry command_loop_setup_entries[] =
 {
@@ -1450,6 +1451,15 @@ static const GOptionEntry command_loop_setup_entries[] =
     "Do not authenticate the user if needed",
     NULL
   },
+  {
+    "no-partition-scan",
+    0, /* no short option */
+    0,
+    G_OPTION_ARG_NONE,
+    &opt_loop_no_partition_scan,
+    "Do not scan the loop device for partitions",
+    NULL
+  },
   {
     NULL
   }
@@ -1645,6 +1655,10 @@ handle_command_loop (gint        *argc,
         g_variant_builder_add (&builder,
                                "{sv}",
                                "size", g_variant_new_uint64 (opt_loop_size));
+      if (opt_loop_no_partition_scan)
+        g_variant_builder_add (&builder,
+                               "{sv}",
+                               "no-part-scan", g_variant_new_boolean (TRUE));
     }
   options = g_variant_builder_end (&builder);
   g_variant_ref_sink (options);

[tbzatek]

Below patch works as expected.

Yeah, we can do that, good idea. Can you open a pull request please?

[sidt4]

Can you open a pull request please?

Okay. Will do.

But, it's not clear to me what should be the default option for loop-setup from udisksctl client point of view ?

no-partition-scan = TRUE

-or-

no-partition-scan = FALSE

The backend has no-partition-scan = FALSE, and since udisksctl doesn't set the option currently, partitions are scanned in loop devices. This is where i faced an issue in my script ( which tried mounting ISO images as loop devices, but had issues during unmount as there were lot of partitions which the unmount code in my script wasn't expecting )

I was thinking of -partition-scan option to udisksctl, in line with losetup which has the following option.

 -P, --partscan                create a partitioned loop device

Yes. this solution will break the existing scripts which rely on the current behavior. But, we need to think which option makes a meaningful default for most users.

I'll submit a PR once this is sorted.

[tbzatek]

That's a good question. When no-part-scan is not specified, udisks actually forces the ioctl LO_FLAGS_PARTSCAN flag. If I remember correctly the default behaviour of partscan has changed a while back, not sure whether the change was in the kernel or losetup only.

However given that we already released udisks-2.10.0, I would suggest not to change the behaviour and only provide a no-partition-scan option to match the no-part-scan flag of the D-Bus API.

Let's ask @vojtechtrefny for his opinion too.

[vojtechtrefny]

I also don't think we should change the behaviour. I agree that having the same default as losetup would make more sense, but changing it now could break more things than fix.

[sidt4]

PR #1166

[tbzatek]

Fixed by #1166, thanks!