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

[Philip Balister <philip@balister.org>]

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.
> 
> 
> 
>