Skip to content

refactor: move ApiServer&ApiHandler to cpp file and overwrite config …#394

Merged
BigVan merged 1 commit intocontainerd:dev/snapshotfrom
chenbxun:main
Mar 16, 2026
Merged

refactor: move ApiServer&ApiHandler to cpp file and overwrite config …#394
BigVan merged 1 commit intocontainerd:dev/snapshotfrom
chenbxun:main

Conversation

@chenbxun
Copy link
Copy Markdown

@chenbxun chenbxun commented Mar 16, 2026

…after creating snapshot per review

What this PR does / why we need it:
Make the following adjustment according to the code review:

  • move the definition of ApiServer and ApiHandler from api_server.h to api_server.cpp
  • overwrite the config file in use when creating snapshot in case the old one is used again after the process restarts
  • use safe_delete instead of delete

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Please check the following list:

  • Does the affected code have corresponding tests, e.g. unit test, E2E test?
  • Does this change require a documentation update?
  • Does this introduce breaking changes that would require an announcement or bumping the major version?
  • Do all new files have an appropriate license header?

…after creating snapshot per review

Signed-off-by: Xun Chen <xunchen@hust.edu.cn>
@BigVan BigVan merged commit 9e319b5 into containerd:dev/snapshot Mar 16, 2026
1 check passed
api_handler.reset(new ApiHandler(this));
api_server = new ApiServer(global_conf.serviceConfig().address(), api_handler.get());
if(!api_server->ready)
auto ret = start_api_server(api_server, this, global_conf.serviceConfig().address());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

need 'delete ret' if init() fails?

LOG_ERRNO_RETURN(0, -1, "new localfs_adaptor failed");
}
DEFER(delete lfs);
lfs->rename(new_config_path, this->config_path.c_str());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

check return value... print an error log at least

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.

2 participants