Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement sparse image flashing (yet another) #6

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

blenk92
Copy link

@blenk92 blenk92 commented Oct 29, 2019

Hi,

unfortunately I didn't see #5 until I was almost done implementing sparse image flashing as well. However, I thought as I anyway already invested the time I might just open another PR so you can choose what ever solution you prefer.

As far as I can see #5 does not use the API provided in (include/sparse/sparse.h) which I did. So the only thing that needed to be done was handling input provided by the callback function sparse_file_callback().

Add sources of libsparse of the android open source project
Original source was gathered from:
https://android.googlesource.com/platform/system/core/+/master/libsparse/
(Commit Hash ff60db1bb1e70182c62f9af4cde57a4f30417415)

Signed-off-by: Maximilian Blenk <[email protected]>
@blenk92 blenk92 force-pushed the sparse branch 3 times, most recently from 1463792 to 5e0f104 Compare October 29, 2019 17:16
@abozhinov444
Copy link

abozhinov444 commented Nov 7, 2019

Hi,
are there any specific dependencies? Because i cannot build it.
` make

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o firehose.o firehose.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o qdl.o qdl.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o sahara.o sahara.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o util.o util.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o patch.o patch.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o program.o program.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o ufs.o ufs.c

make -C libsparse

make[1]: Entering directory '/home/c_abozhi/projects/qdl2/libsparse'

g++ -O2 -Wall -I include -c -o backed_block.o backed_block.cpp

g++ -O2 -Wall -I include -c -o output_file.o output_file.cpp

g++ -O2 -Wall -I include -c -o sparse.o sparse.cpp

g++ -O2 -Wall -I include -c -o sparse_crc32.o sparse_crc32.cpp

g++ -O2 -Wall -I include -c -o sparse_err.o sparse_err.cpp

g++ -O2 -Wall -I include -c -o sparse_read.o sparse_read.cpp

g++ -O2 -Wall -I include -c -o stringprintf.o stringprintf.cpp

ar rcs libsparse.a backed_block.o output_file.o sparse.o sparse_crc32.o sparse_err.o sparse_read.o stringprintf.o

make[1]: Leaving directory '/home/c_abozhi/projects/qdl2/libsparse'

g++ -o qdl firehose.o qdl.o sahara.o util.o patch.o program.o ufs.o libsparse/libsparse.a libsparse/libsparse.a xml2-config --libs -ludev

/usr/bin/ld: libsparse/libsparse.a(output_file.o): undefined reference to symbol 'gzerror'
//lib/x86_64-linux-gnu/libz.so.1: error adding symbols: DSO missing from command line

collect2: error: ld returned 1 exit status

Makefile:11: recipe for target 'qdl' failed

make: *** [qdl] Error 1
`

@blenk92
Copy link
Author

blenk92 commented Nov 7, 2019

Hi,
are there any specific dependencies? Because i cannot build it.
` make

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o firehose.o firehose.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o qdl.o qdl.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o sahara.o sahara.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o util.o util.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o patch.o patch.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o program.o program.c

cc -O2 -Wall -g xml2-config --cflags -I libsparse/include -c -o ufs.o ufs.c

make -C libsparse

make[1]: Entering directory '/home/c_abozhi/projects/qdl2/libsparse'

g++ -O2 -Wall -I include -c -o backed_block.o backed_block.cpp

g++ -O2 -Wall -I include -c -o output_file.o output_file.cpp

g++ -O2 -Wall -I include -c -o sparse.o sparse.cpp

g++ -O2 -Wall -I include -c -o sparse_crc32.o sparse_crc32.cpp

g++ -O2 -Wall -I include -c -o sparse_err.o sparse_err.cpp

g++ -O2 -Wall -I include -c -o sparse_read.o sparse_read.cpp

g++ -O2 -Wall -I include -c -o stringprintf.o stringprintf.cpp

ar rcs libsparse.a backed_block.o output_file.o sparse.o sparse_crc32.o sparse_err.o sparse_read.o stringprintf.o

make[1]: Leaving directory '/home/c_abozhi/projects/qdl2/libsparse'

g++ -o qdl firehose.o qdl.o sahara.o util.o patch.o program.o ufs.o libsparse/libsparse.a libsparse/libsparse.a xml2-config --libs -ludev

/usr/bin/ld: libsparse/libsparse.a(output_file.o): undefined reference to symbol 'gzerror'
//lib/x86_64-linux-gnu/libz.so.1: error adding symbols: DSO missing from command line

collect2: error: ld returned 1 exit status

Makefile:11: recipe for target 'qdl' failed

make: *** [qdl] Error 1
`

My bad.. seems like I didn't specfiy -lz. I updated that... For some reason works for me without...

Thanks for the hint ;-)

Could you try again?

@abozhinov444
Copy link

Now it works. I have tried to flash some images, but after first sparse image it exits. How is it verified?
[PROGRAM] flashed "abootbak" successfully
LOG: start 790528, num 16384
LOG: Finished sector address 806912
LOG: start 806912, num 28216
LOG: Finished sector address 835128
LOG: start 835128, num 0
LOG: Finished sector address 835128
qdl: failed to read: Connection timed out
[PROGRAM] failed
[PROGRAM] flashed "boot" successfully at 417kB/s

I am trying to flash boot, system, systemrw and user images. They work with my solution. Now i am debugging. Will report back, again.

@blenk92
Copy link
Author

blenk92 commented Nov 7, 2019

Now it works. I have tried to flash some images, but after first sparse image it exits. How is it verified?
[PROGRAM] flashed "abootbak" successfully
LOG: start 790528, num 16384
LOG: Finished sector address 806912
LOG: start 806912, num 28216
LOG: Finished sector address 835128
LOG: start 835128, num 0
LOG: Finished sector address 835128
qdl: failed to read: Connection timed out
[PROGRAM] failed
[PROGRAM] flashed "boot" successfully at 417kB/s

I am trying to flash boot, system, systemrw and user images. They work with my solution. Now i am debugging. Will report back, again.

hmm do you have any kind of additional info you can give me? e.g. xmls, or at best also the images? (I flashed some devices and it never failed)

But as I said, I'm fine with your solution as well. Seems to also work for me... I'm not sure if it is then worth spending too much time to debug this solution...

@abozhinov444
Copy link

Sorry, but i could not share xml and images, because of "Company Policies" .

Why you call firehose_program_sparse_callback at line 544, again? It brokes things on my device.

@blenk92
Copy link
Author

blenk92 commented Nov 7, 2019

Sorry, but i could not share xml and images, because of "Company Policies" .

Why you call firehose_program_sparse_callback at line 544, again? It brokes things on my device.

Well if there is still data in the buffer this also needs to be flashed. Flashing is always performed if a don't care block is reached or the buffer is full. Maybe the buffer is already empty ? That should be checked for sure... Could you retry?

@abozhinov444
Copy link

I will retry tomorrow. But what i understand when i read the libsparse code is that sparse_file_callback() process all the file at one pass with don't care chunks. Did you remember where was bug in my code before? This function should write "skip" chunks.

@blenk92
Copy link
Author

blenk92 commented Nov 7, 2019

I will retry tomorrow. But what i understand when i read the libsparse code is that sparse_file_callback() process all the file at one pass with don't care chunks. Did you remember where was bug in my code before? This function should write "skip" chunks.

Not sure, if this is what you mean.. But I'd argue that it depends on the use-case if "don't care chunks" have to be written.. Actually, I would even argue if chunks are "don't care", then the image really shouldn't care what kind of data there is (which means for me there I don't have to overwrite existing data). Am I missing something here?

@abozhinov444
Copy link

I mean that all "don't care chunk" are written during sparse_file_callback() and last call of firehose_program_sparse_callback() is not needed. Function that writes it is named callback_file_skip(). Also all data should be overwritten, i didn't mean anything else.

Extend XML with the optional "sparse" tag. If sparse is true the file
is interpreted as sparse file and only non "Don't Care" Sections are
flashed.

Signed-off-by: Maximilian Blenk <[email protected]>
@blenk92
Copy link
Author

blenk92 commented Nov 20, 2019

I mean that all "don't care chunk" are written during sparse_file_callback() and last call of firehose_program_sparse_callback() is not needed. Function that writes it is named callback_file_skip(). Also all data should be overwritten, i didn't mean anything else.

I just checked your implementation, and from what i saw you are doing the exact same thing, not write the don't care chunks (as they obviously do not contain data). From what I see the don't care chunks are not written during the flash (i expect the data agrument of firehose_program_sparse_callback() to be NULL in that case).

Also I figured out why it's failing, increasing the timeout fixed it for me. Maybe you also want to retry... The last call is indeed needed because the buffer may still contain data that hasn't been flashed yet. That, however, depends on the sparse image itself.

@abozhinov444
Copy link

No, i am writing, i just modified source to pass zero filled buffer. So in my callback i do not process NULL case.

For the second paragraph, i think it is better just to send data on 1 megabyte chunk like in my code. Because sparse callback returns a lot 4096kb buffers and when they are send rapidly fast as Host PC is times faster device programmer fails. This with timeouts is workaround for me, because if image is bigger then usual, writing will fail again.

@blenk92
Copy link
Author

blenk92 commented May 7, 2020

we have got this patch now in production for more than 6 months. Images have increased in size quite some and we haven't seen any issues (with quite some flashes of different images per day), so I don't think that the timeout thing is an actual issue. From my point this is certainly well-tested.

@jwinarske
Copy link

@andersson what are your thoughts on merging this? Happy to help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants