[PATCH v3 1/2] erofs-utils: refactor OCI code for better modularity
Gao Xiang
hsiangkao at linux.alibaba.com
Wed Sep 3 18:47:22 AEST 2025
On 2025/9/3 16:29, ChengyuZhu6 wrote:
> From: Chengyu Zhu <hudsonzhu at tencent.com>
>
> Refactor OCI code to improve code organization and maintainability:
>
> Key changes:
> - Add `ocierofs_get_api_registry()` and unify API endpoint selection.
> - Implement Bearer token discovery with Basic fallback; cache auth header.
> - Parse layer metadata (digest, mediaType, size) and add a proper free helper.
> - Split blob download from tar processing; process tar via a temp fd.
> - Rework init/teardown into `ocierofs_init()` and `ocierofs_ctx_cleanup()`.
> - Update mkfs to use `struct ocierofs_config` and new `--oci` parsing.
>
> Signed-off-by: Chengyu Zhu <hudsonzhu at tencent.com>
> ---
> lib/liberofs_oci.h | 84 ++---
> lib/remotes/oci.c | 885 ++++++++++++++++++++++++++-------------------
> mkfs/main.c | 192 ++--------
> 3 files changed, 566 insertions(+), 595 deletions(-)
>
> diff --git a/lib/liberofs_oci.h b/lib/liberofs_oci.h
> index 3a8108b..d119a2b 100644
> --- a/lib/liberofs_oci.h
> +++ b/lib/liberofs_oci.h
> @@ -8,85 +8,65 @@
>
> #include <stdbool.h>
>
> -#define DOCKER_REGISTRY "docker.io"
> -#define DOCKER_API_REGISTRY "registry-1.docker.io"
> -
> #ifdef __cplusplus
> extern "C" {
> #endif
>
> -struct erofs_inode;
> -struct CURL;
> struct erofs_importer;
>
> -/**
> - * struct erofs_oci_params - OCI configuration parameters
> - * @registry: registry hostname (e.g., "registry-1.docker.io")
> - * @repository: image repository (e.g., "library/ubuntu")
> - * @tag: image tag or digest (e.g., "latest" or sha256:...)
> +/*
> + * struct ocierofs_config - OCI configuration structure
> + * @image_ref: OCI image reference (e.g., "ubuntu:latest", "myregistry.com/app:v1.0")
> * @platform: target platform in "os/arch" format (e.g., "linux/amd64")
> * @username: username for authentication (optional)
> * @password: password for authentication (optional)
> * @layer_index: specific layer to extract (-1 for all layers)
> *
> - * Configuration structure for OCI image parameters including registry
> - * location, image identification, platform specification, and authentication
> - * credentials.
> */
> -struct erofs_oci_params {
> - char *registry;
> - char *repository;
> - char *tag;
> +struct ocierofs_config {
> + char *image_ref;
> char *platform;
> char *username;
> char *password;
> int layer_index;
> };
>
> -/**
> - * struct erofs_oci - Combined OCI client structure
> - * @curl: CURL handle for HTTP requests
> - * @params: OCI configuration parameters
> - *
> - * Main OCI client structure combining CURL HTTP client with
> - * OCI-specific configuration parameters.
> - */
> -struct erofs_oci {
> - struct CURL *curl;
> - struct erofs_oci_params params;
> +struct ocierofs_layer_info {
> + char *digest;
> + char *media_type;
> + u64 size;
> };
>
> -/*
> - * ocierofs_init - Initialize OCI client with default parameters
> - * @oci: OCI client structure to initialize
> - *
> - * Return: 0 on success, negative errno on failure
> - */
> -int ocierofs_init(struct erofs_oci *oci);
> +struct ocierofs_ctx {
> + struct {
> + struct CURL *curl;
> + char *auth_header;
> + bool using_basic;
> + } net;
Is it necessary to use two anonymous structures here?
Could we just unfold those fields?
...
> diff --git a/mkfs/main.c b/mkfs/main.c
> index bc895f1..5e8da7a 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -150,18 +150,18 @@ static void usage(int argc, char **argv)
> * "0-9,100-109" instead of a continuous "0-109", and to
> * state what those two subranges respectively mean. */
> printf("%s [,level=<0-9,100-109>]\t0-9=normal, 100-109=extreme (default=%i)\n",
> - spaces, s->c->default_level);
> + spaces, s->c->default_level);
> else
> printf("%s [,level=<0-%i>]\t\t(default=%i)\n",
> - spaces, s->c->best_level, s->c->default_level);
> + spaces, s->c->best_level, s->c->default_level);
> }
> if (s->c->setdictsize) {
> if (s->c->default_dictsize)
> printf("%s [,dictsize=<dictsize>]\t(default=%u, max=%u)\n",
> - spaces, s->c->default_dictsize, s->c->max_dictsize);
> + spaces, s->c->default_dictsize, s->c->max_dictsize);
> else
> printf("%s [,dictsize=<dictsize>]\t(default=<auto>, max=%u)\n",
> - spaces, s->c->max_dictsize);
> + spaces, s->c->max_dictsize);
Are those changes unnecessary?
> }
> }
> printf(
> @@ -272,7 +272,7 @@ static struct erofs_s3 s3cfg;
> #endif
>
> #ifdef OCIEROFS_ENABLED
> -static struct erofs_oci ocicfg;
> +static struct ocierofs_config ocicfg;
> static char *mkfs_oci_options;
> #endif
>
> @@ -689,21 +689,14 @@ static int mkfs_parse_s3_cfg(char *cfg_str)
> #endif
>
> #ifdef OCIEROFS_ENABLED
> -
> -
> -/**
> - * mkfs_parse_oci_options - Parse comma-separated OCI options string
> +/*
> + * mkfs_parse_oci_options - Parse OCI options for mkfs tool
> + * @cfg: OCI configuration structure to update
> * @options_str: comma-separated options string
> - *
> - * Parse OCI options string containing comma-separated key=value pairs.
> - * Supported options include platform, layer, username, and password.
> - *
> - * Return: 0 on success, negative errno on failure
> */
> -static int mkfs_parse_oci_options(char *options_str)
> +static int mkfs_parse_oci_options(struct ocierofs_config *oci_cfg, char *options_str)
> {
> char *opt, *q, *p;
> - int ret;
>
> if (!options_str)
> return 0;
> @@ -717,41 +710,38 @@ static int mkfs_parse_oci_options(char *options_str)
> p = strstr(opt, "platform=");
> if (p) {
> p += strlen("platform=");
> - ret = erofs_oci_params_set_string(&ocicfg.params.platform, p);
> - if (ret) {
> - erofs_err("failed to set platform");
> - return ret;
> - }
> + free(oci_cfg->platform);
> + oci_cfg->platform = strdup(p);
> + if (!oci_cfg->platform)
> + return -ENOMEM;
> } else {
> p = strstr(opt, "layer=");
> if (p) {
> p += strlen("layer=");
> - ocicfg.params.layer_index = atoi(p);
> - if (ocicfg.params.layer_index < 0) {
> + oci_cfg->layer_index = atoi(p);
> + if (oci_cfg->layer_index < 0) {
Could you just use `strtoul` since it can check invalid
arguments better?
> erofs_err("invalid layer index %d",
> - ocicfg.params.layer_index);
> + oci_cfg->layer_index);
> return -EINVAL;
> }
> } else {
> p = strstr(opt, "username=");
> if (p) {
> p += strlen("username=");
> - ret = erofs_oci_params_set_string(&ocicfg.params.username, p);
> - if (ret) {
> - erofs_err("failed to set username");
> - return ret;
> - }
> + free(oci_cfg->username);
> + oci_cfg->username = strdup(p);
> + if (!oci_cfg->username)
> + return -ENOMEM;
> } else {
> p = strstr(opt, "password=");
> if (p) {
> p += strlen("password=");
> - ret = erofs_oci_params_set_string(&ocicfg.params.password, p);
> - if (ret) {
> - erofs_err("failed to set password");
> - return ret;
> - }
> + free(oci_cfg->password);
> + oci_cfg->password = strdup(p);
> + if (!oci_cfg->password)
> + return -ENOMEM;
> } else {
> - erofs_err("invalid --oci value %s", opt);
> + erofs_err("mkfs: invalid --oci value %s", opt);
`mkfs:` prefix is unneeded.
Thanks,
Gao Xiang
More information about the Linux-erofs
mailing list