search

checkpoint-restore / criu

Checkpoint/Restore tool
criu.org
Other
2.93k stars 585 forks source link

page-xfer: Pull tcp_cork,nodelay(). #2233

Closed osctobe closed 1 year ago

osctobe commented 1 year ago

Move tcp_cork() and tcp_nodelay() to the only user: page-xfer.c. While at it, fix error messages (as they do not refer to restoring the sockopt values) and demote them as they are not fatal to the page transfer.

avagin commented 1 year ago

criu/page-xfer.c:163:28: error: 'SOL_TCP' undeclared (first use in this function); did you mean 'SOL_TIPC'? 163 | if (setsockopt(sk, SOL_TCP, TCP_CORK, &val, sizeof(val))) | ^~~ | SOL_TIPC criu/page-xfer.c:163:28: note: each undeclared identifier is reported only once for each function it appears in criu/page-xfer.c:163:37: error: 'TCP_CORK' undeclared (first use in this function) 163 | if (setsockopt(sk, SOL_TCP, TCP_CORK, &val, sizeof(val))) | ^~~~ criu/page-xfer.c: In function 'tcp_nodelay': criu/page-xfer.c:170:28: error: 'SOL_TCP' undeclared (first use in this function); did you mean 'SOL_TIPC'? 170 | if (setsockopt(sk, SOL_TCP, TCP_NODELAY, &val, sizeof(val))) | ^~~ | SOL_TIPC criu/page-xfer.c:170:37: error: 'TCP_NODELAY' undeclared (first use in this function); did you mean 'O_NDELAY'? 170 | if (setsockopt(sk, SOL_TCP, TCP_NODELAY, &val, sizeof(val))) | ^~~ | O_NDELAY

avagin commented 1 year ago

While at it, fix and demote error logs as they are not fatal and do not restore the sockopt values.

I don't see this part. Do we really need this pr?

osctobe commented 1 year ago

While at it, fix and demote error logs as they are not fatal and do not restore the sockopt values. I don't see this part. Do we really need this pr?

Fixed the description. PTAL

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 75.00% and project coverage change: +0.28% :tada:

Comparison is base (988a5f4) 70.36% compared to head (46cdba4) 70.64%.

:exclamation: Current head 46cdba4 differs from pull request most recent head 4a06322. Consider uploading reports for the commit 4a06322 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## criu-dev #2233 +/- ## ============================================ + Coverage 70.36% 70.64% +0.28% ============================================ Files 134 133 -1 Lines 34039 33315 -724 ============================================ - Hits 23952 23536 -416 + Misses 10087 9779 -308 ``` | [Files Changed](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2233?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [criu/include/util.h](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2233?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y3JpdS9pbmNsdWRlL3V0aWwuaA==) | `90.00% <ø> (-10.00%)` | :arrow_down: | | [criu/util.c](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2233?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y3JpdS91dGlsLmM=) | `63.29% <ø> (-0.10%)` | :arrow_down: | | [criu/page-xfer.c](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2233?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-Y3JpdS9wYWdlLXhmZXIuYw==) | `79.35% <75.00%> (+1.33%)` | :arrow_up: | ... and [12 files with indirect coverage changes](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2233/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.