On 2025/9/3 16:29, ChengyuZhu6 wrote:
From: Chengyu Zhu <hudson...@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 <hudson...@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
