Skip to content

add HTTP streaming support#755

Merged
ejoerns merged 11 commits into
rauc:masterfrom
jluebbe:http-streaming
Feb 24, 2022
Merged

add HTTP streaming support#755
ejoerns merged 11 commits into
rauc:masterfrom
jluebbe:http-streaming

Conversation

@jluebbe
Copy link
Copy Markdown
Member

@jluebbe jluebbe commented Jul 22, 2021

This adds support for rauc install https://update-server.example/update.rauc for verity format bundles without needing temporary space. Internally, it is based on mounting the bundle using NBD and a unprivileged helper process.

  • better configure.ac error handling
  • documentation
  • improved tests
  • release notes

@jluebbe jluebbe added the enhancement Adds new functionality or enhanced handling to RAUC label Jul 22, 2021
@jluebbe jluebbe requested a review from ejoerns July 22, 2021 16:24
@jluebbe jluebbe self-assigned this Jul 22, 2021
@lgtm-com

This comment was marked as outdated.

@jluebbe jluebbe force-pushed the http-streaming branch 3 times, most recently from a78b390 to 89ad338 Compare July 26, 2021 15:54
@lgtm-com

This comment has been minimized.

Copy link
Copy Markdown
Contributor

@zyga zyga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick first pass. I'll do a separate review of the nbd code.

Comment thread src/bundle.c
Comment thread src/config_file.c
Comment thread test/rauc.t
Comment thread src/nbd.c Outdated
Comment thread src/nbd.c
Comment thread src/nbd.c
Comment thread src/nbd.c Outdated
@lgtm-com

This comment has been minimized.

@lgtm-com

This comment was marked as outdated.

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 17, 2021

Codecov Report

Merging #755 (3a0ce3e) into master (15f3f9b) will decrease coverage by 2.87%.
The diff coverage is 41.30%.

❗ Current head 3a0ce3e differs from pull request most recent head e573033. Consider uploading reports for the commit e573033 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #755      +/-   ##
==========================================
- Coverage   72.45%   69.58%   -2.88%     
==========================================
  Files          28       30       +2     
  Lines        9109    10070     +961     
==========================================
+ Hits         6600     7007     +407     
- Misses       2509     3063     +554     
Impacted Files Coverage Δ
include/bundle.h 100.00% <ø> (ø)
include/config_file.h 100.00% <ø> (ø)
include/utils.h 100.00% <ø> (+50.00%) ⬆️
src/stats.c 0.00% <0.00%> (ø)
src/nbd.c 35.78% <35.78%> (ø)
src/main.c 74.98% <52.63%> (-0.41%) ⬇️
src/service.c 74.17% <54.54%> (-0.83%) ⬇️
src/utils.c 72.68% <61.76%> (-2.18%) ⬇️
src/bundle.c 66.21% <72.22%> (+1.67%) ⬆️
src/config_file.c 84.02% <76.92%> (-0.14%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15f3f9b...e573033. Read the comment docs.

@mschwan-phytec
Copy link
Copy Markdown
Contributor

Many thanks for adding this feature! I gave this a quick try using a simple webserver built with nginx to serve the bundle and installation of the bundle succeeded just fine. As I am building RAUC with Yocto, the corresponding changes in meta-rauc to support this feature were easily added. @jluebbe If you like, I could already open a PR to add these changes. Apart from trying this out and adding Yocto build support, is there anything in particular that I can help with to get this PR merged a little faster?

@jluebbe
Copy link
Copy Markdown
Member Author

jluebbe commented Sep 14, 2021

Many thanks for adding this feature! I gave this a quick try using a simple webserver built with nginx to serve the bundle and installation of the bundle succeeded just fine.

Thanks for the feedback!

As I am building RAUC with Yocto, the corresponding changes in meta-rauc to support this feature were easily added. @jluebbe If you like, I could already open a PR to add these changes.

You could just open a WIP PR there as well. Due to the new dependency, we should probably have a PACKAGECONFIG option.

Apart from trying this out and adding Yocto build support, is there anything in particular that I can help with to get this PR merged a little faster?

More testing feedback is definitely appreciated. If you don't want to go through the code, feedback on the documentation is also very useful. Are there any aspects which are not yet documented enough?

@ejoerns ejoerns added this to the Release v1.6 milestone Sep 22, 2021
Copy link
Copy Markdown
Member

@ejoerns ejoerns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my first review round and includes everything I've found so far. Most things found are just minor issues, refactoring requests or error handling quirks.

All in all my tests on actual hardware looked very promising and did not reveal any major show stopper yet. After verity support, this is another well done and important milestone for RAUC 👍

I have a bunch of method documentation and in-code comments that I used for making the code more parsable for my poor human eyes. Maybe I can drop some of them in the second round as they might also help others or at least give a more comprehensive and cleaner internal API documentation.

Comment thread docs/advanced.rst Outdated
Comment thread include/nbd.h Outdated
Comment thread include/nbd.h Outdated
Comment thread src/bundle.c
Comment thread src/main.c Outdated
Comment thread src/nbd.c Outdated
Comment thread src/nbd.c Outdated
Comment thread src/nbd.c Outdated
Comment thread src/nbd.c
Comment thread src/bundle.c Outdated
@lgtm-com

This comment was marked as outdated.

This is provided by glibc, but needs to be defined locally for musl.

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
With sockets, it's can happen that we are only able to partially read/write the
buffer. In that case we need to restart for the remaining size.

This simplifies the higher-level code for local sockets.

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Link against libnl3 to support confuguring NBD via netlink.

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
@jluebbe jluebbe marked this pull request as ready for review February 18, 2022 15:54
@jluebbe jluebbe requested a review from ejoerns February 18, 2022 15:55
@jluebbe
Copy link
Copy Markdown
Member Author

jluebbe commented Feb 18, 2022

@ejoerns You should be able to use git range-diff 98f14cdfa9ea..32cec8f21879 15f3f9b0d86c..4918dd51d71e to review the changes. I intend to split off the proxy part of nbd.c into a new nbd-proxy.c, but that should be a separate PR to avoid making my fixes more difficult to follow during review. I've fixed the typo in the docs locally.

@jluebbe
Copy link
Copy Markdown
Member Author

jluebbe commented Feb 24, 2022

@ejoerns I've updated the PR as discussed.

ejoerns
ejoerns previously approved these changes Feb 24, 2022
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
@jluebbe
Copy link
Copy Markdown
Member Author

jluebbe commented Feb 24, 2022

@ejoerns I've fixed the uncrustify failure.

@ejoerns ejoerns merged commit 1f8f90b into rauc:master Feb 24, 2022
@jluebbe jluebbe deleted the http-streaming branch February 24, 2022 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Adds new functionality or enhanced handling to RAUC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants