clearlinux / swupd-server

Software update server (deprecated)
Other
13 stars 17 forks source link

use libarchive directly #48

Closed pohly closed 6 years ago

pohly commented 7 years ago

This may or may not work without changes for Clear: it might work that GNU tar can unpack fullfiles created this way, but long filenames might be problematic. xattrs are known to be problematic (not used outside of Ostro?).

If this change turns out to be problematic, then it could be merged by switching to bsdtar in client and server also in Clear and introducing a format change. Once that's done, support for GNU tar and the necessary ifdefs can be removed, followed by converting more code to calling libarchive directly.

For example:

aaronovz1 commented 7 years ago

@pohly are you able to rebase this pull request? I'd like to test against latest 3.4.0 release. Cheers

pohly commented 7 years ago

@aaronovz1 I've stopped working on swupd. Someone else (you?) will have to take over and rebase these patches.

aaronovz1 commented 7 years ago

@pohly that is a shame! Is there a reason? What is your recommendation for OTA mechanisms under Yocto for production platforms?

pohly commented 7 years ago

@aaronovz1 https://wiki.yoctoproject.org/wiki/System_Update is an overview of several solutions that all work in Yocto.

aaronovz1 commented 7 years ago

@pohly I am aware of that page thanks. I was just curious if there was a reason you're not maintaining anymore and whether anyone else will be.

pohly commented 7 years ago

@aaronovz1 just to clarify, I've never been a swupd maintainer, just a user and part-time contributor. Nothing has changed regarding maintenance, it's still done by the same team as before.

aaronovz1 commented 7 years ago

@pohly Oh right my bad. I saw you listed on the meta-swupd maintainer list. I don't see a way to rebase someone else's pull request. I assume I would need to fork and redo the patches myself and make a new pull request?

pohly commented 7 years ago

@aaronovz1 Yes, clone my branch and submit your own, rebased one.

matthewrsj commented 7 years ago

@pohly we like this idea, but we need to do some testing for client compatibility before merging. I'll work with @tmarcu to set up a test stream for a few releases.

matthewrsj commented 7 years ago

swupd-client as-is can't untar these archives. I'll have to test using bsdtar on swupd-client or switching to libarchive there as well.

pohly commented 7 years ago

swupd-client already has a configure option to select bsdtar. That's how we've been using this. Making swupd-client call libarchive directly would also open the door for further improvements, like directly streaming data to the staging directory.

Alternatively one could enhance the code so that it calls bsdtar, but produces the tar flavor understood by GNU tar.

matthewrsj commented 7 years ago

I just did a quick check with bsdtar and although we see less errors, it still fails to verify --install. Strangely, I am able to subsequently verify --fix on the target directory and that works.

pohly commented 7 years ago

Do you have a branch somewhere with this PR and PR #48 merged? I could test that revision together with meta-swupd.

matthewrsj commented 7 years ago

This is #48, did you mean #47?

No we just checked out your branch (well @tmarcu did) and built content with it. We then tried to use swupd-client with and without bsdtar to install an OS and then fix it.

phmccarty commented 7 years ago

@matthewrsj @tmarcu Was this PR rebased before testing?

pohly commented 7 years ago

FWIW, I rebased the meta-swupd patches (i.e. PR #47, PR #48 and some inappropriate patches) onto 3.6.1. The extra patches shouldn't affect your usage of swupd-server. You can get the result from https://github.com/pohly/swupd-server/commits/meta-swupd-patches

Have there been format changes between 3.3.0 and 3.6.1? Are those documented? I remember vaguely that a Wiki was mentioned for this kind of information in the past, but I am not sure anymore whether that was just an idea or actually gets documented.

tmarcu commented 7 years ago

I rebased this onto our latest master before testing it. We cannot create images with the updates made using this patch, @matthewrsj any new updates on this or should I re-test?

matthewrsj commented 7 years ago

@tmarcu according to our testing it looks like swupd-client needs to use libarchive as well. I think this patch needs to wait until that work has been done on the client side. The client side is where we would see most of the benefit from libarchive anyways.

pohly commented 7 years ago

On Fri, 2017-09-22 at 10:54 -0700, Matthew Johnson wrote:

@tmarcu according to our testing it looks like swupd-client needs to use libarchive as well.

There's a configure option that makes it call bsdtar, so that works already. No need to wait.

The client side is where we would see most of the benefit from libarchive anyways.

Hmm? Using libarchive in swupd-server also had huge benefits when run under pseudo.

matthewrsj commented 7 years ago

There's a configure option that makes it call bsdtar, so that works already. No need to wait.

As mentioned above we did testing with bsdtar.

Hmm? Using libarchive in swupd-server also had huge benefits when run under pseudo.

I mean that performance is more important for swupd-client than swupd-server, since swupd-client is user-facing.

matthewrsj commented 7 years ago

@pohly specifically we see several errors that swupd-client was unable to unpack the archive for directories, even when running with bsdtar. Interestingly, when I then ran swupd verify --fix on the target directory these were cleaned up. That's nice, but obviously these need to be successfully installed the first time.

If you could share your testing method where you are seeing this working maybe we can identify the difference.

pohly commented 7 years ago

On Fri, 2017-09-22 at 19:14 +0000, Matthew Johnson wrote:

@pohly specifically we see several errors that swupd-client was unable to unpack the archive for directories, even when running with bsdtar.

Please provide more information. What error? Can it be reproduced when invoking bsdtar manually? Can you upload one of the problematic tar archives?

If you could share your testing method where you are seeing this working maybe we can identify the difference.

I'm not doing anything special, but both the OS content and thus the update will be very different. Let's investigate the error, that should lead to an explanation.

pohly commented 7 years ago

On Fri, 2017-09-22 at 11:21 -0700, Matthew Johnson wrote:

Hmm? Using libarchive in swupd-server also had huge benefits when run under pseudo. I mean that performance is more important for swupd-client than swupd-server, since swupd-client is user-facing.

I'm not sure how much faster libarchive will be on the client.

For us, it made a huge difference on the server side. That is relevant when every single pull request test ends up being blocked for hours (I no longer have the exact time, but definitely more than one hour) while preparing the swupd update.

matthewrsj commented 7 years ago

Please provide more information. What error? Can it be reproduced when invoking bsdtar manually? Can you upload one of the problematic tar archives?

I can redo the test and get more information.

I'm not sure how much faster libarchive will be on the client.

Performance on the client is important to us, even for small improvements. That's why I bring it up. That may not be the case for you since you are considering A/B partition updates followed by a reboot.

For us, it made a huge difference on the server side. That is relevant when every single pull request test ends up being blocked for hours (I no longer have the exact time, but definitely more than one hour) while preparing the swupd update.

I understand that swupd-server would see significant improvements, much more significant than swupd-client due to the number of files being processed for creating an update. I'm saying for Clear Linux purposes update creation time is less of an issue. Users don't see how long this takes and Clear is still able to get out ~8 releases per week, with (I believe) QA being the bottleneck not swupd-server.

pohly commented 7 years ago

I might have found the issue with bsdtar on the client, see https://github.com/clearlinux/swupd-client/issues/280

Are you testing with a UTF-8 locale on the client?

matthewrsj commented 6 years ago

@pohly as I responded in clearlinux/swupd-client#280 we are testing with a UTF-8 locale on the client.

pohly commented 6 years ago

On Sat, 2017-09-23 at 13:15 +0200, Patrick Ohly wrote:

On Fri, 2017-09-22 at 19:14 +0000, Matthew Johnson wrote:

@pohly specifically we see several errors that swupd-client was unable to unpack the archive for directories, even when running with bsdtar.

Please provide more information. What error? Can it be reproduced when invoking bsdtar manually? Can you upload one of the problematic tar archives?

Have you had time to look into these questions and analyze the problem further?

As discussed elsewhere, the UTF-8 issue that I ran into shouldn't affect you, so it must be something else.

matthewrsj commented 6 years ago

No I have not.

I am working on potentially introducing libarchive into the client, so that may be the solution for this PR.

matthewrsj commented 6 years ago

Closing this PR as this project is no longer being actively developed. If this functionality is still required please open a PR or issue against https://github.com/clearlinux/mixer-tools (the new project)