[PATCH 1/2] wic/utils/partitionedfs.py: assemble .wic images as sparse files

[PATCH 1/2] wic/utils/partitionedfs.py: assemble .wic images as sparse files

[Joshua Lock]

From: Joshua Lock <joshua.lock@collabora.co.uk>

The individual partitions created by wic are sparse but without
this change the assembled image is written as one (potentially
very) large file.

Preserve sparseness in the assembled image by passing the sparse
conversion symbol.

[YOCTO #9099]

Signed-off-by: Joshua Lock <joshua.lock@collabora.co.uk>
---
 scripts/lib/wic/utils/partitionedfs.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/lib/wic/utils/partitionedfs.py b/scripts/lib/wic/utils/partitionedfs.py
index 5a103bb..3e2b420 100644
--- a/scripts/lib/wic/utils/partitionedfs.py
+++ b/scripts/lib/wic/utils/partitionedfs.py
@@ -340,7 +340,7 @@ class Image(object):
             source = part['source_file']
             if source:
                 # install source_file contents into a partition
-                cmd = "dd if=%s of=%s bs=%d seek=%d count=%d conv=notrunc" % \
+                cmd = "dd if=%s of=%s bs=%d seek=%d count=%d conv=notrunc,sparse" % \
                       (source, image_file, self.sector_size,
                        part['start'], part['size'])
                 exec_cmd(cmd)
-- 
2.5.5

[PATCH 2/2] oeqa/selftest/wic: add test case for sparse images

[Joshua Lock]

From: Joshua Lock <joshua.lock@collabora.co.uk>

Add a testcase to build a directdisk image and check that the
used disk size is less than the apparent size, as wic now
assembles images as sparse files.

Signed-off-by: Joshua Lock <joshua.lock@collabora.co.uk>
Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 meta/lib/oeqa/selftest/wic.py | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/meta/lib/oeqa/selftest/wic.py b/meta/lib/oeqa/selftest/wic.py
index a569fbf..dd32075 100644
--- a/meta/lib/oeqa/selftest/wic.py
+++ b/meta/lib/oeqa/selftest/wic.py
@@ -276,3 +276,16 @@ class Wic(oeSelfTest):
             status, output = qemu.run_serial(command)
             self.assertEqual(1, status, 'Failed to run command "%s": %s' % (command, output))
             self.assertEqual(output, '/dev/root /\r\n/dev/vda3 /mnt')
+
+    def test_sparseness(self):
+        """Test that assembled images are sparse; apparent size > disk usage"""
+        self.assertEqual(0, runCmd("wic create directdisk "
+                                   "--image-name core-image-minimal").status)
+        images = glob(self.resultdir + "directdisk-*.direct")
+        self.assertEqual(1, len(images))
+
+        imagestat = os.stat(images.pop())
+        # st_blocks is the "number of 512-byte blocks allocated for file"
+        used = imagestat.st_blocks*512
+        apparent = imagestat.st_size
+        self.assertLess(used, apparent)
-- 
2.5.5

Re: [PATCH 1/2] wic/utils/partitionedfs.py: assemble .wic images as sparse files

[Alexander Kanevskiy]

[-- Attachment #1: Type: text/plain, Size: 3875 bytes --]

On Fri, Apr 8, 2016 at 12:14 PM, Joshua Lock <joshua.g.lock@intel.com>
wrote:

> From: Joshua Lock <joshua.lock@collabora.co.uk>
>
> The individual partitions created by wic are sparse but without
> this change the assembled image is written as one (potentially
> very) large file.
>
> Preserve sparseness in the assembled image by passing the sparse
> conversion symbol.
>
> [YOCTO #9099]
>
> Signed-off-by: Joshua Lock <joshua.lock@collabora.co.uk>
> ---
>  scripts/lib/wic/utils/partitionedfs.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/lib/wic/utils/partitionedfs.py
> b/scripts/lib/wic/utils/partitionedfs.py
> index 5a103bb..3e2b420 100644
> --- a/scripts/lib/wic/utils/partitionedfs.py
> +++ b/scripts/lib/wic/utils/partitionedfs.py
> @@ -340,7 +340,7 @@ class Image(object):
>              source = part['source_file']
>              if source:
>                  # install source_file contents into a partition
> -                cmd = "dd if=%s of=%s bs=%d seek=%d count=%d
> conv=notrunc" % \
> +                cmd = "dd if=%s of=%s bs=%d seek=%d count=%d
> conv=notrunc,sparse" % \
>                        (source, image_file, self.sector_size,
>                         part['start'], part['size'])
>                  exec_cmd(cmd)
>



This might be dangerous. Even in recent version of DD (coreutils 8.25), it
might not detect in source file mapped/unmapped blocks properly,
(just use is_nul() function to check if block contains only zeros, instead
of e.g. fiemap).
Result might be so, that in output file block which must be zeros is
actually become unmapped. And as consequence, if it might not be written to
destination drive.

Let me show example based on Ostro Project sparse images:

kad@orava:/tmp> ls -ls 2222
308416 -rw-r--r-- 1 kad users 5280628736 Apr  6 00:53 2222
kad@orava:/tmp> grep /MappedBlocksCount 2222.bmap
    <MappedBlocksCount> 77104   </MappedBlocksCount>

Now, let's copy it with dd:
kad@orava:/tmp> ~/cu/coreutils-8.25/src/dd if=2222 of=4444
conv=notrunc,sparse bs=4096
1289216+0 records in
1289216+0 records out
5280628736 bytes (5.3 GB, 4.9 GiB) copied, 1.31239 s, 4.0 GB/s
kad@orava:/tmp> bmaptool create 4444 -o 4444.bmap
kad@orava:/tmp> ls -ls 4444
308348 -rw-r--r-- 1 kad users 5280628736 Apr  9 13:22 4444
kad@orava:/tmp> grep /MappedBlocksCount 4444.bmap
    <MappedBlocksCount> 77087   </MappedBlocksCount>
kad@orava:/tmp>

As you can see, in result of dd copied file we have less blocks mapped than
in original.

Diffing block ranges:  (- original file, + dd copied)
     <BlockMap>
-        <Range> 0-4 </Range>
-        <Range> 768-3321 </Range>
-        <Range> 4608-7161 </Range>
+        <Range> 0 </Range>
+        <Range> 768-770 </Range>
+        <Range> 772-774 </Range>
+        <Range> 776 </Range>
+        <Range> 780-3321 </Range>
+        <Range> 4608-4610 </Range>
+        <Range> 4612-4614 </Range>
+        <Range> 4616 </Range>
+        <Range> 4620-7161 </Range>
-        <Range> 1289211-1289215 </Range>
+        <Range> 1289211 </Range>
+        <Range> 1289215 </Range>
     </BlockMap>

As you can see, some blocks that must be cleaned (e.g. 1-3 around MBR or in
the end, 1289211-1289215 where GPT bits located) are not marked as mapped,
thus in case of sparse aware writing tool used to write to destination
drive it might result as unwritten zero blocks, thus some old data would
left on device.

I would suggest not to accept this patch right now, and do copy function
that would be aware about real mapped/unmapped blocks.
Something similar that I've did for Ostro Project here:
https://github.com/ostroproject/meta-ostro/commit/d883169c3266c3c44f22db5f75ae94c4cf0f2924

but adopted to be generic for use inside wic.


-- 
br, Alexander Kanevskiy

[-- Attachment #2: Type: text/html, Size: 5580 bytes --]

Re: [PATCH 1/2] wic/utils/partitionedfs.py: assemble .wic images as sparse files

[Philip Balister]

On 04/09/2016 03:36 AM, Alexander Kanevskiy wrote:
> On Fri, Apr 8, 2016 at 12:14 PM, Joshua Lock <joshua.g.lock@intel.com>
> wrote:
> 
>> From: Joshua Lock <joshua.lock@collabora.co.uk>
>>
>> The individual partitions created by wic are sparse but without
>> this change the assembled image is written as one (potentially
>> very) large file.
>>
>> Preserve sparseness in the assembled image by passing the sparse
>> conversion symbol.
>>
>> [YOCTO #9099]
>>
>> Signed-off-by: Joshua Lock <joshua.lock@collabora.co.uk>
>> ---
>>  scripts/lib/wic/utils/partitionedfs.py | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/lib/wic/utils/partitionedfs.py
>> b/scripts/lib/wic/utils/partitionedfs.py
>> index 5a103bb..3e2b420 100644
>> --- a/scripts/lib/wic/utils/partitionedfs.py
>> +++ b/scripts/lib/wic/utils/partitionedfs.py
>> @@ -340,7 +340,7 @@ class Image(object):
>>              source = part['source_file']
>>              if source:
>>                  # install source_file contents into a partition
>> -                cmd = "dd if=%s of=%s bs=%d seek=%d count=%d
>> conv=notrunc" % \
>> +                cmd = "dd if=%s of=%s bs=%d seek=%d count=%d
>> conv=notrunc,sparse" % \
>>                        (source, image_file, self.sector_size,
>>                         part['start'], part['size'])
>>                  exec_cmd(cmd)
>>
> 
> 
> 
> This might be dangerous. Even in recent version of DD (coreutils 8.25), it
> might not detect in source file mapped/unmapped blocks properly,
> (just use is_nul() function to check if block contains only zeros, instead
> of e.g. fiemap).
> Result might be so, that in output file block which must be zeros is
> actually become unmapped. And as consequence, if it might not be written to
> destination drive.

Does it work with bmaptool?

Philip

> 
> Let me show example based on Ostro Project sparse images:
> 
> kad@orava:/tmp> ls -ls 2222
> 308416 -rw-r--r-- 1 kad users 5280628736 Apr  6 00:53 2222
> kad@orava:/tmp> grep /MappedBlocksCount 2222.bmap
>     <MappedBlocksCount> 77104   </MappedBlocksCount>
> 
> Now, let's copy it with dd:
> kad@orava:/tmp> ~/cu/coreutils-8.25/src/dd if=2222 of=4444
> conv=notrunc,sparse bs=4096
> 1289216+0 records in
> 1289216+0 records out
> 5280628736 bytes (5.3 GB, 4.9 GiB) copied, 1.31239 s, 4.0 GB/s
> kad@orava:/tmp> bmaptool create 4444 -o 4444.bmap
> kad@orava:/tmp> ls -ls 4444
> 308348 -rw-r--r-- 1 kad users 5280628736 Apr  9 13:22 4444
> kad@orava:/tmp> grep /MappedBlocksCount 4444.bmap
>     <MappedBlocksCount> 77087   </MappedBlocksCount>
> kad@orava:/tmp>
> 
> As you can see, in result of dd copied file we have less blocks mapped than
> in original.
> 
> Diffing block ranges:  (- original file, + dd copied)
>      <BlockMap>
> -        <Range> 0-4 </Range>
> -        <Range> 768-3321 </Range>
> -        <Range> 4608-7161 </Range>
> +        <Range> 0 </Range>
> +        <Range> 768-770 </Range>
> +        <Range> 772-774 </Range>
> +        <Range> 776 </Range>
> +        <Range> 780-3321 </Range>
> +        <Range> 4608-4610 </Range>
> +        <Range> 4612-4614 </Range>
> +        <Range> 4616 </Range>
> +        <Range> 4620-7161 </Range>
> -        <Range> 1289211-1289215 </Range>
> +        <Range> 1289211 </Range>
> +        <Range> 1289215 </Range>
>      </BlockMap>
> 
> As you can see, some blocks that must be cleaned (e.g. 1-3 around MBR or in
> the end, 1289211-1289215 where GPT bits located) are not marked as mapped,
> thus in case of sparse aware writing tool used to write to destination
> drive it might result as unwritten zero blocks, thus some old data would
> left on device.
> 
> I would suggest not to accept this patch right now, and do copy function
> that would be aware about real mapped/unmapped blocks.
> Something similar that I've did for Ostro Project here:
> https://github.com/ostroproject/meta-ostro/commit/d883169c3266c3c44f22db5f75ae94c4cf0f2924
> 
> but adopted to be generic for use inside wic.
> 
> 
> 
> 

Re: [PATCH 1/2] wic/utils/partitionedfs.py: assemble .wic images as sparse files

[Robert Yang]

On 04/08/2016 05:14 PM, Joshua Lock wrote:
> From: Joshua Lock <joshua.lock@collabora.co.uk>
>
> The individual partitions created by wic are sparse but without
> this change the assembled image is written as one (potentially
> very) large file.
>
> Preserve sparseness in the assembled image by passing the sparse
> conversion symbol.
>
> [YOCTO #9099]
>
> Signed-off-by: Joshua Lock <joshua.lock@collabora.co.uk>
> ---
>   scripts/lib/wic/utils/partitionedfs.py | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/lib/wic/utils/partitionedfs.py b/scripts/lib/wic/utils/partitionedfs.py
> index 5a103bb..3e2b420 100644
> --- a/scripts/lib/wic/utils/partitionedfs.py
> +++ b/scripts/lib/wic/utils/partitionedfs.py
> @@ -340,7 +340,7 @@ class Image(object):
>               source = part['source_file']
>               if source:
>                   # install source_file contents into a partition
> -                cmd = "dd if=%s of=%s bs=%d seek=%d count=%d conv=notrunc" % \
> +                cmd = "dd if=%s of=%s bs=%d seek=%d count=%d conv=notrunc,sparse" % \

Hi Joshua,

I got errors with this patch since my dd doesn't support conversion sparse:
dd: invalid conversion: `sparse'
Try `dd --help' for more information.

So that it would fail.

$ dd --version
dd (coreutils) 8.13
Copyright (C) 2011 Free Software Foundation, Inc.

The host is Ubuntu 12.04, it works well except this error.

// Robert

>                         (source, image_file, self.sector_size,
>                          part['start'], part['size'])
>                   exec_cmd(cmd)
>

Re: [PATCH 1/2] wic/utils/partitionedfs.py: assemble .wic images as sparse files

[Alexander Kanevskiy]

[-- Attachment #1: Type: text/plain, Size: 1297 bytes --]

On Sat, Apr 9, 2016 at 7:41 PM, Philip Balister <philip@balister.org> wrote:

> > This might be dangerous. Even in recent version of DD (coreutils 8.25),
> it
> > might not detect in source file mapped/unmapped blocks properly,
> > (just use is_nul() function to check if block contains only zeros,
> instead
> > of e.g. fiemap).
> > Result might be so, that in output file block which must be zeros is
> > actually become unmapped. And as consequence, if it might not be written
> to
> > destination drive.
>
> Does it work with bmaptool?
>

bmaptool can work with practically any sparse file (even if bmap file are
not provided) to flash it efficiently on raw device.
However, with such cases like "dd conv=sparse" it should be used with extra
caution, as some of needed zeroed blocks might not be written to
destination device.

Yocto bug #9099 now has two patches attached that adding initial bmap-tools
support to OE-core.
Those patches allows to create bmap files for simpler cases, like .hddimg
as well as shipping standalone version of bmaptool in deploy/tools/*
It probably would be content of next release, as I don't think anyone would
be merging them now.

For assembling images with wic, it would require a bit more work.

-- 
br, Alexander Kanevskiy

[-- Attachment #2: Type: text/html, Size: 1878 bytes --]

Re: [PATCH 1/2] wic/utils/partitionedfs.py: assemble .wic images as sparse files

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 532 bytes --]

On 9 April 2016 at 11:36, Alexander Kanevskiy <kad@kad.name> wrote:

> This might be dangerous. Even in recent version of DD (coreutils 8.25), it
> might not detect in source file mapped/unmapped blocks properly,
> (just use is_nul() function to check if block contains only zeros, instead
> of e.g. fiemap).
> Result might be so, that in output file block which must be zeros is
> actually become unmapped. And as consequence, if it might not be written to
> destination drive.
>

Agreed, I've a revert queued.

Ross

[-- Attachment #2: Type: text/html, Size: 891 bytes --]