[PATCH] wic: allow creation of partitions not in table

[PATCH] wic: allow creation of partitions not in table

[Alexandre Belloni]

For some architectures it is necessary to reserve space on disk without
it being present in the partition table.

For example, u-boot on i.mx is placed at an offset of 1kB on the sdcard.
While it would be possible to create a partition at that offset and
place u-boot there, it would then be necessary to update the default
u-boot environment to use partition 2 on the mmc instead of partition 1.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 scripts/lib/image/help.py                            |  6 ++++++
 scripts/lib/wic/imager/direct.py                     |  1 +
 .../lib/wic/kickstart/custom_commands/partition.py   |  6 ++++++
 scripts/lib/wic/utils/partitionedfs.py               | 20 +++++++++++++++-----
 4 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/scripts/lib/image/help.py b/scripts/lib/image/help.py
index 0d8a6adfaa71..aab0b609afd5 100644
--- a/scripts/lib/image/help.py
+++ b/scripts/lib/image/help.py
@@ -737,6 +737,12 @@ DESCRIPTION
                               to start a partition on an x KBytes
                               boundary.
 
+         --no-table: This option is specific to wic. Space will be
+                     reserved for the partition and it will be
+                     populated but it will not be added to the
+                     partition table. It may be useful for
+                     bootloaders.
+
     * bootloader
 
       This command allows the user to specify various bootloader
diff --git a/scripts/lib/wic/imager/direct.py b/scripts/lib/wic/imager/direct.py
index b1dc3e96f456..c39405feb74a 100644
--- a/scripts/lib/wic/imager/direct.py
+++ b/scripts/lib/wic/imager/direct.py
@@ -262,6 +262,7 @@ class DirectImageCreator(BaseImageCreator):
                                        fsopts = p.fsopts,
                                        boot = p.active,
                                        align = p.align,
+                                       no_table = p.no_table,
                                        part_type = p.part_type)
 
         self.__image.layout_partitions(self._ptable_format)
diff --git a/scripts/lib/wic/kickstart/custom_commands/partition.py b/scripts/lib/wic/kickstart/custom_commands/partition.py
index 7a307065f289..9be6b0457b48 100644
--- a/scripts/lib/wic/kickstart/custom_commands/partition.py
+++ b/scripts/lib/wic/kickstart/custom_commands/partition.py
@@ -49,6 +49,7 @@ class Wic_PartData(Mic_PartData):
         self.source = kwargs.get("source", None)
         self.sourceparams = kwargs.get("sourceparams", None)
         self.rootfs = kwargs.get("rootfs-dir", None)
+        self.no_table = kwargs.get("no-table", False)
         self.source_file = ""
         self.size = 0
 
@@ -61,6 +62,8 @@ class Wic_PartData(Mic_PartData):
                 retval += " --sourceparams=%s" % self.sourceparams
             if self.rootfs:
                 retval += " --rootfs-dir=%s" % self.rootfs
+        if self.no_table:
+            retval += " --no-table"
 
         return retval
 
@@ -521,4 +524,7 @@ class Wic_Partition(Mic_Partition):
         # use specified rootfs path to fill the partition
         op.add_option("--rootfs-dir", type="string", action="store",
                       dest="rootfs", default=None)
+        # wether to add the partition in the partition table
+        op.add_option("--no-table", dest="no_table", action="store_true",
+                      default=False)
         return op
diff --git a/scripts/lib/wic/utils/partitionedfs.py b/scripts/lib/wic/utils/partitionedfs.py
index f109e2c227c5..745558b090cb 100644
--- a/scripts/lib/wic/utils/partitionedfs.py
+++ b/scripts/lib/wic/utils/partitionedfs.py
@@ -61,6 +61,7 @@ class Image:
         self.disks[disk_name] = \
                 { 'disk': None,     # Disk object
                   'numpart': 0,     # Number of allocate partitions
+                  'realpart': 0,    # Number of partitions in the partition table
                   'partitions': [], # Indexes to self.partitions
                   'offset': 0,      # Offset of next partition (in sectors)
                   # Minimum required disk size to fit all partitions (in bytes)
@@ -85,7 +86,7 @@ class Image:
         self.__add_disk(part['disk_name'])
 
     def add_partition(self, size, disk_name, mountpoint, source_file = None, fstype = None,
-                      label=None, fsopts = None, boot = False, align = None,
+                      label=None, fsopts = None, boot = False, align = None, no_table=False,
                       part_type = None):
         """ Add the next partition. Prtitions have to be added in the
         first-to-last order. """
@@ -109,6 +110,7 @@ class Image:
                      'num': None, # Partition number
                      'boot': boot, # Bootable flag
                      'align': align, # Partition alignment
+                     'no_table' : no_table, # Partition does not appear in partition table
                      'part_type' : part_type } # Partition type
 
             self.__add_partition(part)
@@ -147,6 +149,8 @@ class Image:
             # Get the disk where the partition is located
             d = self.disks[p['disk_name']]
             d['numpart'] += 1
+            if not p['no_table']:
+                d['realpart'] += 1
             d['ptable_format'] = ptable_format
 
             if d['numpart'] == 1:
@@ -182,10 +186,13 @@ class Image:
             d['offset'] += p['size']
 
             p['type'] = 'primary'
-            p['num'] = d['numpart']
+            if not p['no_table']:
+                p['num'] = d['realpart']
+            else:
+                p['num'] = 0
 
             if d['ptable_format'] == "msdos":
-                if d['numpart'] > 2:
+                if d['realpart'] > 2:
                     # Every logical partition requires an additional sector for
                     # the EBR, so steal the last sector from the end of each
                     # partition starting from the 3rd one for the EBR. This
@@ -193,9 +200,9 @@ class Image:
                     # correctly.
                     p['size'] -= 1
 
-                if d['numpart'] > 3:
+                if d['realpart'] > 3:
                     p['type'] = 'logical'
-                    p['num'] = d['numpart'] + 1
+                    p['num'] = d['realpart'] + 1
 
             d['partitions'].append(n)
             msger.debug("Assigned %s to %s%d, sectors range %d-%d size %d "
@@ -257,6 +264,9 @@ class Image:
         msger.debug("Creating partitions")
 
         for p in self.partitions:
+            if p['num'] == 0:
+                continue
+
             d = self.disks[p['disk_name']]
             if d['ptable_format'] == "msdos" and p['num'] == 5:
                 # The last sector of the 3rd partition was reserved for the EBR
-- 
2.1.0

Re: [PATCH] wic: allow creation of partitions not in table

[Maciej Borzecki]

On 02/05 00:03, Alexandre Belloni wrote:
> For some architectures it is necessary to reserve space on disk without
> it being present in the partition table.
>
> For example, u-boot on i.mx is placed at an offset of 1kB on the sdcard.
> While it would be possible to create a partition at that offset and
> place u-boot there, it would then be necessary to update the default
> u-boot environment to use partition 2 on the mmc instead of partition
> 1.

I'm wondering if reusing parition here is the right choice. How about
making this a separate command, since the operation of putting some
binary data at a particular location within the image sounds like
something very specific to wic.

Could name it 'raw-copy', with options --source and --offset. The
command would be applied after image assembly is complete. Offset could
be expressed in bytes by default (plus k/M/G suffix). Source could
either be a 'bootloader', that implies a default bootloader for this
target, or a name of a file from $DEPLOY_DIR_IMAGE.

Assuming IMAGE_BOOTLOADER='u-boot.img' is set somewhere, the kickstart
could look like:

part / --source rootfs --ondisk mmcblk --fstype=ext4 --label root --align 4 --start 1M
raw-copy --source bootloader --offset 1kB


If at some point we extend the bootloader command to include the name of
bootloader file:

part / --source rootfs --ondisk mmcblk --fstype=ext4 --label root --align 4 --start 1M
bootloader --name u-boot.img
raw-copy --source bootloader --offset 1kB

What do you think?

--
Maciej Borzęcki
Senior Software Developer at Open-RnD Sp. z o.o., Poland
www.open-rnd.pl
mobile: +48 889 117 365, fax: +48 42 657 9079


Niniejsza wiadomość wraz z załącznikami może
zawierać chronione prawem lub poufne informacje i została
wysłana wyłącznie do wiadomości i użytku osób, do których
została zaadresowana. Jeśli wiadomość została otrzymana
przypadkowo zabrania się jej kopiowania lub rozsyłania do osób
trzecich. W takim przypadku uprasza się o natychmiastowe
zniszczenie wiadomości oraz poinformowanie nadawcy o
zaistniałej sytuacji za pomocą wiadomości zwrotnej.
Dziękujemy.

This message, including any attachments hereto,
may contain privileged or confidential information and is sent
solely for the attention and use of the intended addressee(s).
If you are not an intended addressee, you may neither use this
message nor copy or deliver it to anyone. In such case, you
should immediately destroy this message and kindly notify the
sender by reply email. Thank you.

Re: [PATCH] wic: allow creation of partitions not in table

[Alexandre Belloni]

Hi,

On 05/02/2015 at 10:02:26 +0100, Maciej Borzecki wrote :
> On 02/05 00:03, Alexandre Belloni wrote:
> > For some architectures it is necessary to reserve space on disk without
> > it being present in the partition table.
> >
> > For example, u-boot on i.mx is placed at an offset of 1kB on the sdcard.
> > While it would be possible to create a partition at that offset and
> > place u-boot there, it would then be necessary to update the default
> > u-boot environment to use partition 2 on the mmc instead of partition
> > 1.
> 
> I'm wondering if reusing parition here is the right choice. How about
> making this a separate command, since the operation of putting some
> binary data at a particular location within the image sounds like
> something very specific to wic.
> 
> Could name it 'raw-copy', with options --source and --offset. The
> command would be applied after image assembly is complete. Offset could
> be expressed in bytes by default (plus k/M/G suffix). Source could
> either be a 'bootloader', that implies a default bootloader for this
> target, or a name of a file from $DEPLOY_DIR_IMAGE.
> 
> Assuming IMAGE_BOOTLOADER='u-boot.img' is set somewhere, the kickstart
> could look like:
> 
> part / --source rootfs --ondisk mmcblk --fstype=ext4 --label root --align 4 --start 1M
> raw-copy --source bootloader --offset 1kB
> 
> 
> If at some point we extend the bootloader command to include the name of
> bootloader file:
> 
> part / --source rootfs --ondisk mmcblk --fstype=ext4 --label root --align 4 --start 1M
> bootloader --name u-boot.img
> raw-copy --source bootloader --offset 1kB
> 
> What do you think?
> 

I though about that, I actually created a rawcopy plugin to dump file
into partitions.

The main issue with what you suggest is that you lose the whole
automatic offset calculations. What if the bootloader is larger than 1M
in your example? It will overwrite / You also lose the alignment
calculations unless you add them back in the new class.

Another issue is that you may want to copy multiple files at different
offsets. For example for am335x:
SPL at 128k, backup SPL at 256k, u-boot at 384k in a partition, u-boot
environment in a partition, rootfs.

This would give something like:
part SPL --source rawcopy --sourceparams="file=MLO" --ondisk mmcblk --no-table --align 128
part SPL.backup --source rawcopy --sourceparams="file=MLO" --ondisk mmcblk --no-table --align 128
part uboot --source rawcopy --sourceparams="file=u-boot.img" --ondisk mmcblk --align 128
part uboot.env --source rawcopy --sourceparams="file=u-boot.env" --ondisk mmcblk --align 128
part / --source rootfs --ondisk mmcblk --fstype=ext4 --label root --align 4

So, u-boot and its environment are in separate partitions and can be
updated easily from Linux.

Maybe instead of using part --no-table, we can name it nopart?

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH] wic: allow creation of partitions not in table

[Maciej Borzecki]

On 02/05 11:27, Alexandre Belloni wrote:
> Hi,
>
> On 05/02/2015 at 10:02:26 +0100, Maciej Borzecki wrote :
> > On 02/05 00:03, Alexandre Belloni wrote:
> > > For some architectures it is necessary to reserve space on disk without
> > > it being present in the partition table.
> > >
> > > For example, u-boot on i.mx is placed at an offset of 1kB on the sdcard.
> > > While it would be possible to create a partition at that offset and
> > > place u-boot there, it would then be necessary to update the default
> > > u-boot environment to use partition 2 on the mmc instead of partition
> > > 1.
> >
> > I'm wondering if reusing parition here is the right choice. How about
> > making this a separate command, since the operation of putting some
> > binary data at a particular location within the image sounds like
> > something very specific to wic.
> >
> > Could name it 'raw-copy', with options --source and --offset. The
> > command would be applied after image assembly is complete. Offset could
> > be expressed in bytes by default (plus k/M/G suffix). Source could
> > either be a 'bootloader', that implies a default bootloader for this
> > target, or a name of a file from $DEPLOY_DIR_IMAGE.
> >
> > Assuming IMAGE_BOOTLOADER='u-boot.img' is set somewhere, the kickstart
> > could look like:
> >
> > part / --source rootfs --ondisk mmcblk --fstype=ext4 --label root --align 4 --start 1M
> > raw-copy --source bootloader --offset 1kB
> >
> >
> > If at some point we extend the bootloader command to include the name of
> > bootloader file:
> >
> > part / --source rootfs --ondisk mmcblk --fstype=ext4 --label root --align 4 --start 1M
> > bootloader --name u-boot.img
> > raw-copy --source bootloader --offset 1kB
> >
> > What do you think?
> >
>
> I though about that, I actually created a rawcopy plugin to dump file
> into partitions.
>
> The main issue with what you suggest is that you lose the whole
> automatic offset calculations. What if the bootloader is larger than 1M
> in your example? It will overwrite / You also lose the alignment
> calculations unless you add them back in the new class.
>
> Another issue is that you may want to copy multiple files at different
> offsets. For example for am335x:
> SPL at 128k, backup SPL at 256k, u-boot at 384k in a partition, u-boot
> environment in a partition, rootfs.

Booting from eMMC?

>
> This would give something like:
> part SPL --source rawcopy --sourceparams="file=MLO" --ondisk mmcblk --no-table --align 128
> part SPL.backup --source rawcopy --sourceparams="file=MLO" --ondisk mmcblk --no-table --align 128
> part uboot --source rawcopy --sourceparams="file=u-boot.img" --ondisk mmcblk --align 128
> part uboot.env --source rawcopy --sourceparams="file=u-boot.env" --ondisk mmcblk --align 128
> part / --source rootfs --ondisk mmcblk --fstype=ext4 --label root --align 4
>

It makes sense now, rawcopy plugin is what I have been missing. I'd be
careful with --align though. It would be much more readable to have an
explicit --start or --offset option, but that has been sitting in my
TODO list for quite some time now.

Other than that the patch looks good to me.

--
Maciej Borzęcki
Senior Software Developer at Open-RnD Sp. z o.o., Poland
www.open-rnd.pl
mobile: +48 889 117 365, fax: +48 42 657 9079


Niniejsza wiadomość wraz z załącznikami może
zawierać chronione prawem lub poufne informacje i została
wysłana wyłącznie do wiadomości i użytku osób, do których
została zaadresowana. Jeśli wiadomość została otrzymana
przypadkowo zabrania się jej kopiowania lub rozsyłania do osób
trzecich. W takim przypadku uprasza się o natychmiastowe
zniszczenie wiadomości oraz poinformowanie nadawcy o
zaistniałej sytuacji za pomocą wiadomości zwrotnej.
Dziękujemy.

This message, including any attachments hereto,
may contain privileged or confidential information and is sent
solely for the attention and use of the intended addressee(s).
If you are not an intended addressee, you may neither use this
message nor copy or deliver it to anyone. In such case, you
should immediately destroy this message and kindly notify the
sender by reply email. Thank you.

Re: [PATCH] wic: allow creation of partitions not in table

[Alexandre Belloni]

Hi,

I found one remaining bug in that patch, I'll send v2 soon.

On 05/02/2015 at 00:03:16 +0100, Alexandre Belloni wrote :
> For some architectures it is necessary to reserve space on disk without
> it being present in the partition table.
> 
> For example, u-boot on i.mx is placed at an offset of 1kB on the sdcard.
> While it would be possible to create a partition at that offset and
> place u-boot there, it would then be necessary to update the default
> u-boot environment to use partition 2 on the mmc instead of partition 1.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
>  scripts/lib/image/help.py                            |  6 ++++++
>  scripts/lib/wic/imager/direct.py                     |  1 +
>  .../lib/wic/kickstart/custom_commands/partition.py   |  6 ++++++
>  scripts/lib/wic/utils/partitionedfs.py               | 20 +++++++++++++++-----
>  4 files changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/lib/image/help.py b/scripts/lib/image/help.py
> index 0d8a6adfaa71..aab0b609afd5 100644
> --- a/scripts/lib/image/help.py
> +++ b/scripts/lib/image/help.py
> @@ -737,6 +737,12 @@ DESCRIPTION
>                                to start a partition on an x KBytes
>                                boundary.
>  
> +         --no-table: This option is specific to wic. Space will be
> +                     reserved for the partition and it will be
> +                     populated but it will not be added to the
> +                     partition table. It may be useful for
> +                     bootloaders.
> +
>      * bootloader
>  
>        This command allows the user to specify various bootloader
> diff --git a/scripts/lib/wic/imager/direct.py b/scripts/lib/wic/imager/direct.py
> index b1dc3e96f456..c39405feb74a 100644
> --- a/scripts/lib/wic/imager/direct.py
> +++ b/scripts/lib/wic/imager/direct.py
> @@ -262,6 +262,7 @@ class DirectImageCreator(BaseImageCreator):
>                                         fsopts = p.fsopts,
>                                         boot = p.active,
>                                         align = p.align,
> +                                       no_table = p.no_table,
>                                         part_type = p.part_type)
>  
>          self.__image.layout_partitions(self._ptable_format)
> diff --git a/scripts/lib/wic/kickstart/custom_commands/partition.py b/scripts/lib/wic/kickstart/custom_commands/partition.py
> index 7a307065f289..9be6b0457b48 100644
> --- a/scripts/lib/wic/kickstart/custom_commands/partition.py
> +++ b/scripts/lib/wic/kickstart/custom_commands/partition.py
> @@ -49,6 +49,7 @@ class Wic_PartData(Mic_PartData):
>          self.source = kwargs.get("source", None)
>          self.sourceparams = kwargs.get("sourceparams", None)
>          self.rootfs = kwargs.get("rootfs-dir", None)
> +        self.no_table = kwargs.get("no-table", False)
>          self.source_file = ""
>          self.size = 0
>  
> @@ -61,6 +62,8 @@ class Wic_PartData(Mic_PartData):
>                  retval += " --sourceparams=%s" % self.sourceparams
>              if self.rootfs:
>                  retval += " --rootfs-dir=%s" % self.rootfs
> +        if self.no_table:
> +            retval += " --no-table"
>  
>          return retval
>  
> @@ -521,4 +524,7 @@ class Wic_Partition(Mic_Partition):
>          # use specified rootfs path to fill the partition
>          op.add_option("--rootfs-dir", type="string", action="store",
>                        dest="rootfs", default=None)
> +        # wether to add the partition in the partition table
> +        op.add_option("--no-table", dest="no_table", action="store_true",
> +                      default=False)
>          return op
> diff --git a/scripts/lib/wic/utils/partitionedfs.py b/scripts/lib/wic/utils/partitionedfs.py
> index f109e2c227c5..745558b090cb 100644
> --- a/scripts/lib/wic/utils/partitionedfs.py
> +++ b/scripts/lib/wic/utils/partitionedfs.py
> @@ -61,6 +61,7 @@ class Image:
>          self.disks[disk_name] = \
>                  { 'disk': None,     # Disk object
>                    'numpart': 0,     # Number of allocate partitions
> +                  'realpart': 0,    # Number of partitions in the partition table
>                    'partitions': [], # Indexes to self.partitions
>                    'offset': 0,      # Offset of next partition (in sectors)
>                    # Minimum required disk size to fit all partitions (in bytes)
> @@ -85,7 +86,7 @@ class Image:
>          self.__add_disk(part['disk_name'])
>  
>      def add_partition(self, size, disk_name, mountpoint, source_file = None, fstype = None,
> -                      label=None, fsopts = None, boot = False, align = None,
> +                      label=None, fsopts = None, boot = False, align = None, no_table=False,
>                        part_type = None):
>          """ Add the next partition. Prtitions have to be added in the
>          first-to-last order. """
> @@ -109,6 +110,7 @@ class Image:
>                       'num': None, # Partition number
>                       'boot': boot, # Bootable flag
>                       'align': align, # Partition alignment
> +                     'no_table' : no_table, # Partition does not appear in partition table
>                       'part_type' : part_type } # Partition type
>  
>              self.__add_partition(part)
> @@ -147,6 +149,8 @@ class Image:
>              # Get the disk where the partition is located
>              d = self.disks[p['disk_name']]
>              d['numpart'] += 1
> +            if not p['no_table']:
> +                d['realpart'] += 1
>              d['ptable_format'] = ptable_format
>  
>              if d['numpart'] == 1:
> @@ -182,10 +186,13 @@ class Image:
>              d['offset'] += p['size']
>  
>              p['type'] = 'primary'
> -            p['num'] = d['numpart']
> +            if not p['no_table']:
> +                p['num'] = d['realpart']
> +            else:
> +                p['num'] = 0
>  
>              if d['ptable_format'] == "msdos":
> -                if d['numpart'] > 2:
> +                if d['realpart'] > 2:
>                      # Every logical partition requires an additional sector for
>                      # the EBR, so steal the last sector from the end of each
>                      # partition starting from the 3rd one for the EBR. This
> @@ -193,9 +200,9 @@ class Image:
>                      # correctly.
>                      p['size'] -= 1
>  
> -                if d['numpart'] > 3:
> +                if d['realpart'] > 3:
>                      p['type'] = 'logical'
> -                    p['num'] = d['numpart'] + 1
> +                    p['num'] = d['realpart'] + 1
>  
>              d['partitions'].append(n)
>              msger.debug("Assigned %s to %s%d, sectors range %d-%d size %d "
> @@ -257,6 +264,9 @@ class Image:
>          msger.debug("Creating partitions")
>  
>          for p in self.partitions:
> +            if p['num'] == 0:
> +                continue
> +
>              d = self.disks[p['disk_name']]
>              if d['ptable_format'] == "msdos" and p['num'] == 5:
>                  # The last sector of the 3rd partition was reserved for the EBR
> -- 
> 2.1.0
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com