Re: [systemd-devel] [PATCH 4/5] import/pull: Tag replaced with reference

2015-06-17 Thread Pavel Odvody 
On Tue, 2015-06-16 at 19:53 +0200, Thomas H.P. Andersen wrote:
 On Thu, May 7, 2015 at 5:47 PM, Pavel Odvody podv...@redhat.com wrote:
  Signed-off-by: Pavel Odvody podv...@redhat.com
  ---
   src/import/pull.c | 28 +---
   1 file changed, 17 insertions(+), 11 deletions(-)
 
  diff --git a/src/import/pull.c b/src/import/pull.c
  index ef7b035..8054612 100644
  --- a/src/import/pull.c
  +++ b/src/import/pull.c
  @@ -227,7 +227,7 @@ static void on_dkr_finished(DkrPull *pull, int error, 
  void *userdata) {
   static int pull_dkr(int argc, char *argv[], void *userdata) {
   _cleanup_(dkr_pull_unrefp) DkrPull *pull = NULL;
   _cleanup_event_unref_ sd_event *event = NULL;
  -const char *name, *tag, *local;
  +const char *name, *reference, *local, *digest;
   int r;
 
   if (!arg_dkr_index_url) {
  @@ -240,13 +240,19 @@ static int pull_dkr(int argc, char *argv[], void 
  *userdata) {
   return -EINVAL;
   }
 
  -tag = strchr(argv[1], ':');
  -if (tag) {
  -name = strndupa(argv[1], tag - argv[1]);
  -tag++;
  +digest = strchr(argv[1], '@');
  +if (digest) {
  +reference = digest + 1;
  +name = strndupa(argv[1], digest - argv[1]);
  +}
  +
  +reference = strchr(argv[1], ':');
  +if (reference) {
  +name = strndupa(argv[1], reference - argv[1]);
  +reference++;
   } else {
   name = argv[1];
  -tag = latest;
  +reference = latest;
   }
 
 This part does not look correct. Any value that we set for
 reference/name in the digest part will be overwritten in the next
 block. I do not know the format of the string so I cannot create a
 patch for this. Can you take a look Pavel?
 

I think you're right, will take a look later today and submit a patch.

The digest reference looks like this:

fedora@sha256:cc47966101aeba8015c933f9c3854811a27363f93fa4e0e52e6c55181c56c66c


 
   if (!dkr_name_is_valid(name)) {
  @@ -254,8 +260,8 @@ static int pull_dkr(int argc, char *argv[], void 
  *userdata) {
   return -EINVAL;
   }
 
  -if (!dkr_tag_is_valid(tag)) {
  -log_error(Tag name '%s' is not valid., tag);
  +if (!dkr_ref_is_valid(reference)) {
  +log_error(Tag name '%s' is not valid., reference);
   return -EINVAL;
   }
 
  @@ -288,9 +294,9 @@ static int pull_dkr(int argc, char *argv[], void 
  *userdata) {
   }
   }
 
  -log_info(Pulling '%s' with tag '%s', saving as '%s'., 
  name, tag, local);
  +log_info(Pulling '%s' with reference '%s', saving as 
  '%s'., name, reference, local);
   } else
  -log_info(Pulling '%s' with tag '%s'., name, tag);
  +log_info(Pulling '%s' with reference '%s'., name, 
  reference);
 
   r = sd_event_default(event);
   if (r  0)
  @@ -304,7 +310,7 @@ static int pull_dkr(int argc, char *argv[], void 
  *userdata) {
   if (r  0)
   return log_error_errno(r, Failed to allocate puller: %m);
 
  -r = dkr_pull_start(pull, name, tag, local, arg_force);
  +r = dkr_pull_start(pull, name, reference, local, arg_force, 
  PULL_V2);
   if (r  0)
   return log_error_errno(r, Failed to pull image: %m);
 
  --
  2.1.0
 
 
 
 
  ___
  systemd-devel mailing list
  systemd-devel@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/systemd-devel
 


-- 
Pavel Odvody podv...@redhat.com
Software Engineer - EMEA ENG Developer Experience
5EC1 95C1 8E08 5BD9 9BBF 9241 3AFA 3A66 024F F68D
Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno



signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH v2 4/5] import/pull: Tag replaced with reference

2015-05-19 Thread Pavel Odvody 
Default pull version set to V2
---
 src/import/pull.c | 28 +---
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/src/import/pull.c b/src/import/pull.c
index ef7b035..0f2ad92 100644
--- a/src/import/pull.c
+++ b/src/import/pull.c
@@ -227,7 +227,7 @@ static void on_dkr_finished(DkrPull *pull, int error, void 
*userdata) {
 static int pull_dkr(int argc, char *argv[], void *userdata) {
 _cleanup_(dkr_pull_unrefp) DkrPull *pull = NULL;
 _cleanup_event_unref_ sd_event *event = NULL;
-const char *name, *tag, *local;
+const char *name, *reference, *local, *digest;
 int r;
 
 if (!arg_dkr_index_url) {
@@ -240,13 +240,19 @@ static int pull_dkr(int argc, char *argv[], void 
*userdata) {
 return -EINVAL;
 }
 
-tag = strchr(argv[1], ':');
-if (tag) {
-name = strndupa(argv[1], tag - argv[1]);
-tag++;
+digest = strchr(argv[1], '@');
+if (digest) {
+reference = digest + 1;
+name = strndupa(argv[1], digest - argv[1]);
+}
+
+reference = strchr(argv[1], ':');
+if (reference) {
+name = strndupa(argv[1], reference - argv[1]);
+reference++;
 } else {
 name = argv[1];
-tag = latest;
+reference = latest;
 }
 
 if (!dkr_name_is_valid(name)) {
@@ -254,8 +260,8 @@ static int pull_dkr(int argc, char *argv[], void *userdata) 
{
 return -EINVAL;
 }
 
-if (!dkr_tag_is_valid(tag)) {
-log_error(Tag name '%s' is not valid., tag);
+if (!dkr_ref_is_valid(reference)) {
+log_error(Tag name '%s' is not valid., reference);
 return -EINVAL;
 }
 
@@ -288,9 +294,9 @@ static int pull_dkr(int argc, char *argv[], void *userdata) 
{
 }
 }
 
-log_info(Pulling '%s' with tag '%s', saving as '%s'., name, 
tag, local);
+log_info(Pulling '%s' with reference '%s', saving as '%s'., 
name, reference, local);
 } else
-log_info(Pulling '%s' with tag '%s'., name, tag);
+log_info(Pulling '%s' with reference '%s'., name, reference);
 
 r = sd_event_default(event);
 if (r  0)
@@ -304,7 +310,7 @@ static int pull_dkr(int argc, char *argv[], void *userdata) 
{
 if (r  0)
 return log_error_errno(r, Failed to allocate puller: %m);
 
-r = dkr_pull_start(pull, name, tag, local, arg_force);
+r = dkr_pull_start(pull, name, reference, local, arg_force, 
DKR_PULL_V2);
 if (r  0)
 return log_error_errno(r, Failed to pull image: %m);
 
-- 
2.1.0


signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH v2 2/5] shared/json: Added DOM-like JSON parser

2015-05-19 Thread Pavel Odvody 
This makes working with complexly structured documents easy
and more reliable as the parser is not susceptible to
element re-ordering.

Also fixes a bug when the tokenizer would choke after reading
a number.
---
 src/shared/json.c | 432 --
 src/shared/json.h |  37 +
 2 files changed, 459 insertions(+), 10 deletions(-)

diff --git a/src/shared/json.c b/src/shared/json.c
index 45c8ece..f6c44f4 100644
--- a/src/shared/json.c
+++ b/src/shared/json.c
@@ -21,17 +21,171 @@
 
 #include sys/types.h
 #include math.h
-
 #include macro.h
-#include util.h
 #include utf8.h
 #include json.h
 
-enum {
-STATE_NULL,
-STATE_VALUE,
-STATE_VALUE_POST,
-};
+int json_variant_new(JsonVariant **ret, JsonVariantType type) {
+JsonVariant *v;
+v = new0(JsonVariant, 1);
+if (!v)
+return -ENOMEM;
+v-type = type;
+*ret = v;
+return 0;
+}
+
+static int json_variant_deep_copy(JsonVariant *ret, JsonVariant *variant) {
+assert(ret);
+assert(variant);
+
+ret-type = variant-type;
+ret-size = variant-size;
+
+if (variant-type == JSON_VARIANT_STRING) {
+ret-string = memdup(variant-string, variant-size+1);
+if (!ret-string)
+return -ENOMEM;
+} else if (variant-type == JSON_VARIANT_ARRAY || variant-type == 
JSON_VARIANT_OBJECT) {
+ret-objects = new0(JsonVariant, variant-size);
+if (!ret-objects)
+return -ENOMEM;
+
+for (unsigned i = 0; i  variant-size; ++i) {
+int r;
+r = json_variant_deep_copy(ret-objects[i], 
variant-objects[i]);
+if (r  0)
+return r;
+}
+}
+else
+ret-value = variant-value;
+
+return 0;
+}
+
+static JsonVariant *json_object_unref(JsonVariant *variant);
+
+static JsonVariant *json_variant_unref_inner(JsonVariant *variant) {
+if (!variant)
+return NULL;
+
+if (variant-type == JSON_VARIANT_ARRAY || variant-type == 
JSON_VARIANT_OBJECT)
+return json_object_unref(variant);
+
+else if (variant-type == JSON_VARIANT_STRING)
+free(variant-string);
+
+return NULL;
+}
+
+static JsonVariant *json_raw_unref(JsonVariant *variant, size_t size) {
+if (!variant)
+return NULL;
+
+for (size_t i = 0; i  size; ++i)
+json_variant_unref_inner(variant[i]);
+
+free(variant);
+return NULL;
+}
+
+static JsonVariant *json_object_unref(JsonVariant *variant) {
+assert(variant);
+if (!variant-objects)
+return NULL;
+
+for (unsigned i = 0; i  variant-size; ++i)
+json_variant_unref_inner(variant-objects[i]);
+
+free(variant-objects);
+return NULL;
+}
+
+static JsonVariant **json_variant_array_unref(JsonVariant **variant) {
+size_t i = 0;
+JsonVariant *p = NULL;
+
+if (!variant)
+return NULL;
+
+while((p = (variant[i++])) != NULL) {
+if (p-type == JSON_VARIANT_STRING)
+   free(p-string);
+free(p);
+}
+
+free(variant);
+
+return NULL;
+}
+DEFINE_TRIVIAL_CLEANUP_FUNC(JsonVariant **, json_variant_array_unref);
+
+JsonVariant *json_variant_unref(JsonVariant *variant) {
+if (!variant)
+return NULL;
+
+if (variant-type == JSON_VARIANT_ARRAY || variant-type == 
JSON_VARIANT_OBJECT)
+return json_object_unref(variant);
+
+else if (variant-type == JSON_VARIANT_STRING)
+free(variant-string);
+
+free(variant);
+
+return NULL;
+}
+
+char *json_variant_string(JsonVariant *variant){
+assert(variant);
+assert(variant-type == JSON_VARIANT_STRING);
+
+return variant-string;
+}
+
+bool json_variant_bool(JsonVariant *variant) {
+assert(variant);
+assert(variant-type == JSON_VARIANT_BOOLEAN);
+
+return variant-value.boolean;
+}
+
+intmax_t json_variant_integer(JsonVariant *variant) {
+assert(variant);
+assert(variant-type == JSON_VARIANT_INTEGER);
+
+return variant-value.integer;
+}
+
+double json_variant_real(JsonVariant *variant) {
+assert(variant);
+assert(variant-type == JSON_VARIANT_REAL);
+
+return variant-value.real;
+}
+
+JsonVariant *json_variant_element(JsonVariant *variant, unsigned index) {
+assert(variant);
+assert(variant-type == JSON_VARIANT_ARRAY || variant-type == 
JSON_VARIANT_OBJECT);
+assert(index  variant-size);
+assert(variant-objects);
+
+return variant-objects[index];
+}
+
+JsonVariant *json_variant_value(JsonVariant *variant, const

[systemd-devel] [PATCH v2 1/5] shared/import-util: Tag renamed to reference

2015-05-19 Thread Pavel Odvody 
Added (sha256) digest validation function
---
 src/shared/import-util.c | 21 +
 src/shared/import-util.h |  2 ++
 2 files changed, 23 insertions(+)

diff --git a/src/shared/import-util.c b/src/shared/import-util.c
index 660d92a..001a8a3 100644
--- a/src/shared/import-util.c
+++ b/src/shared/import-util.c
@@ -150,6 +150,27 @@ int raw_strip_suffixes(const char *p, char **ret) {
 return 0;
 }
 
+bool dkr_digest_is_valid(const char *digest) {
+/* 7 chars for prefix, 64 chars for the digest itself */
+if (strlen(digest) != 71)
+return false;
+
+return startswith(digest, sha256:)  in_charset(digest + 7, 
0123456789abcdef);
+}
+
+bool dkr_ref_is_valid(const char *ref) {
+const char *colon;
+
+if (isempty(ref))
+return false;
+
+colon = strchr(ref, ':');
+if (!colon)
+return filename_is_valid(ref);
+
+return dkr_digest_is_valid(ref);
+}
+
 bool dkr_name_is_valid(const char *name) {
 const char *slash, *p;
 
diff --git a/src/shared/import-util.h b/src/shared/import-util.h
index ff155b0..7bf7d4c 100644
--- a/src/shared/import-util.h
+++ b/src/shared/import-util.h
@@ -44,4 +44,6 @@ int raw_strip_suffixes(const char *name, char **ret);
 
 bool dkr_name_is_valid(const char *name);
 bool dkr_id_is_valid(const char *id);
+bool dkr_ref_is_valid(const char *ref);
+bool dkr_digest_is_valid(const char *digest);
 #define dkr_tag_is_valid(tag) filename_is_valid(tag)
-- 
2.1.0



signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH v2 3/5] test/test-json: Tests for the tokenizer bugfix and the DOM parser

2015-05-19 Thread Pavel Odvody 
The DOM parser tests are accompanied with structure and element analysis
---
 src/test/test-json.c | 97 
 1 file changed, 97 insertions(+)

diff --git a/src/test/test-json.c b/src/test/test-json.c
index 24dc700..c4b4a22 100644
--- a/src/test/test-json.c
+++ b/src/test/test-json.c
@@ -72,6 +72,98 @@ static void test_one(const char *data, ...) {
 va_end(ap);
 }
 
+typedef void (*Test)(JsonVariant *);
+
+static void test_file(const char *data, Test test) {
+JsonVariant *v = NULL;
+int r = json_parse(data, v);
+
+assert_se(r == 0);
+assert_se(v != NULL);
+assert_se(v-type == JSON_VARIANT_OBJECT);
+
+if (test)
+test(v);
+
+json_variant_unref(v);
+}
+
+static void test_1(JsonVariant *v) {
+JsonVariant *p, *q;
+unsigned i;
+
+/* 3 keys + 3 values */
+assert_se(v-size == 6);
+
+/* has k */
+p = json_variant_value(v, k);
+assert_se(p  p-type == JSON_VARIANT_STRING);
+
+/* k equals v */
+assert_se(streq(json_variant_string(p), v));
+
+/* has foo */
+p = json_variant_value(v, foo);
+assert_se(p  p-type == JSON_VARIANT_ARRAY  p-size == 3);
+
+/* check  foo[0] = 1, foo[1] = 2, foo[2] = 3 */
+for (i = 0; i  3; ++i) {
+q = json_variant_element(p, i);
+assert_se(q  q-type == JSON_VARIANT_INTEGER  
json_variant_integer(q) == (i+1));
+}
+
+/* has bar */
+p = json_variant_value(v, bar);
+assert_se(p  p-type == JSON_VARIANT_OBJECT  p-size == 2);
+
+/* zap is null */
+q = json_variant_value(p, zap);
+assert_se(q  q-type == JSON_VARIANT_NULL);
+}
+
+static void test_2(JsonVariant *v) {
+JsonVariant *p, *q;
+
+/* 2 keys + 2 values */
+assert_se(v-size == 4);
+
+/* has mutant */
+p = json_variant_value(v, mutant);
+assert_se(p  p-type == JSON_VARIANT_ARRAY  p-size == 4);
+
+/* mutant[0] == 1 */
+q = json_variant_element(p, 0);
+assert_se(q  q-type == JSON_VARIANT_INTEGER  
json_variant_integer(q) == 1);
+
+/* mutant[1] == null */
+q = json_variant_element(p, 1);
+assert_se(q  q-type == JSON_VARIANT_NULL);
+
+/* mutant[2] == 1 */
+q = json_variant_element(p, 2);
+assert_se(q  q-type == JSON_VARIANT_STRING  
streq(json_variant_string(q), 1));
+
+/* mutant[3] == JSON_VARIANT_OBJECT */
+q = json_variant_element(p, 3);
+assert_se(q  q-type == JSON_VARIANT_OBJECT  q-size == 2);
+
+/* has 1 */
+p = json_variant_value(q, 1);
+assert_se(p  p-type == JSON_VARIANT_ARRAY  p-size == 2);
+
+/* 1[0] == 1 */
+q = json_variant_element(p, 0);
+assert_se(q  q-type == JSON_VARIANT_INTEGER  
json_variant_integer(q) == 1);
+
+/* 1[1] == 1 */
+q = json_variant_element(p, 1);
+assert_se(q  q-type == JSON_VARIANT_STRING  
streq(json_variant_string(q), 1));
+
+/* has blah */
+p = json_variant_value(v, blah);
+assert_se(p  p-type == JSON_VARIANT_REAL  
fabs(json_variant_real(p) - 1.27)  0.001);
+}
+
 int main(int argc, char *argv[]) {
 
 test_one(x, -EINVAL);
@@ -102,5 +194,10 @@ int main(int argc, char *argv[]) {
 test_one(\\\udc00\\udc00\, -EINVAL);
 test_one(\\\ud801\\udc37\, JSON_STRING, \xf0\x90\x90\xb7, 
JSON_END);
 
+test_one([1, 2], JSON_ARRAY_OPEN, JSON_INTEGER, 1, JSON_COMMA, 
JSON_INTEGER, 2, JSON_ARRAY_CLOSE, JSON_END);
+
+test_file({\k\: \v\, \foo\: [1, 2, 3], \bar\: {\zap\: 
null}}, test_1);
+test_file({\mutant\: [1, null, \1\, {\1\: [1, \1\]}], 
\blah\: 1.27}, test_2);
+
 return 0;
 }
-- 
2.1.0




signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH v2 5/5] import/pull-dkr: V2 image specification and manifest support

2015-05-19 Thread Pavel Odvody 
The maximum number of layers changed to 127, as in Dkr.
---
 src/import/pull-dkr.c | 542 +-
 src/import/pull-dkr.h |   3 +-
 2 files changed, 491 insertions(+), 54 deletions(-)

diff --git a/src/import/pull-dkr.c b/src/import/pull-dkr.c
index 0eefec5..577bd5a 100644
--- a/src/import/pull-dkr.c
+++ b/src/import/pull-dkr.c
@@ -51,6 +51,9 @@ struct DkrPull {
 sd_event *event;
 CurlGlue *glue;
 
+char *index_protocol;
+char *index_address;
+
 char *index_url;
 char *image_root;
 
@@ -61,9 +64,10 @@ struct DkrPull {
 PullJob *layer_job;
 
 char *name;
-char *tag;
+char *reference;
 char *id;
 
+char *response_digest;
 char *response_token;
 char **response_registries;
 
@@ -87,9 +91,9 @@ struct DkrPull {
 #define PROTOCOL_PREFIX https://;
 
 #define HEADER_TOKEN X-Do /* the HTTP header for the auth token */ 
cker-Token:
-#define HEADER_REGISTRY X-Do /*the HTTP header for the registry */ 
cker-Endpoints:
-
-#define LAYERS_MAX 2048
+#define HEADER_REGISTRY X-Do /* the HTTP header for the registry */ 
cker-Endpoints:
+#define HEADER_DIGEST Do /* the HTTP header for the manifest digest */ 
cker-Content-Digest:
+#define LAYERS_MAX 127
 
 static void dkr_pull_job_on_finished(PullJob *j);
 
@@ -117,12 +121,13 @@ DkrPull* dkr_pull_unref(DkrPull *i) {
 }
 
 free(i-name);
-free(i-tag);
+free(i-reference);
 free(i-id);
 free(i-response_token);
-free(i-response_registries);
 strv_free(i-ancestry);
 free(i-final_path);
+free(i-index_address);
+free(i-index_protocol);
 free(i-index_url);
 free(i-image_root);
 free(i-local);
@@ -416,10 +421,27 @@ static int dkr_pull_add_token(DkrPull *i, PullJob *j) {
 return 0;
 }
 
+static int dkr_pull_add_bearer_token(DkrPull *i, PullJob *j) {
+const char *t = NULL;
+
+assert(i);
+assert(j);
+
+if (i-response_token)
+t = strjoina(Authorization: Bearer , i-response_token);
+else
+return -EINVAL;
+
+j-request_header = curl_slist_new(Accept: application/json, t, 
NULL);
+if (!j-request_header)
+return -ENOMEM;
+
+return 0;
+}
+
 static bool dkr_pull_is_done(DkrPull *i) {
 assert(i);
 assert(i-images_job);
-
 if (i-images_job-state != PULL_JOB_DONE)
 return false;
 
@@ -429,7 +451,7 @@ static bool dkr_pull_is_done(DkrPull *i) {
 if (!i-ancestry_job || i-ancestry_job-state != PULL_JOB_DONE)
 return false;
 
-if (!i-json_job || i-json_job-state != PULL_JOB_DONE)
+if (i-json_job  i-json_job-state != PULL_JOB_DONE)
 return false;
 
 if (i-layer_job  i-layer_job-state != PULL_JOB_DONE)
@@ -441,8 +463,9 @@ static bool dkr_pull_is_done(DkrPull *i) {
 return true;
 }
 
-static int dkr_pull_make_local_copy(DkrPull *i) {
+static int dkr_pull_make_local_copy(DkrPull *i, DkrPullVersion version) {
 int r;
+_cleanup_free_ char *p = NULL;
 
 assert(i);
 
@@ -455,10 +478,30 @@ static int dkr_pull_make_local_copy(DkrPull *i) {
 return log_oom();
 }
 
-r = pull_make_local_copy(i-final_path, i-image_root, i-local, 
i-force_local);
+if (version == DKR_PULL_V2) {
+r = path_get_parent(i-image_root, p);
+if (r  0)
+return r;
+}
+
+r = pull_make_local_copy(i-final_path, p ?: i-image_root, i-local, 
i-force_local);
 if (r  0)
 return r;
 
+if (version == DKR_PULL_V2) {
+char **k = NULL;
+STRV_FOREACH(k, i-ancestry) {
+_cleanup_free_ char *d = strjoin(i-image_root, 
/.dkr-, *k, NULL);
+r = btrfs_subvol_remove(d, false);
+if (r  0)
+   return r;
+}
+
+r = rmdir(i-image_root);
+if (r  0)
+return r;
+}
+
 return 0;
 }
 
@@ -516,6 +559,68 @@ static void dkr_pull_job_on_progress(PullJob *j) {

DKR_DOWNLOADING);
 }
 
+static void dkr_pull_job_on_finished_v2(PullJob *j);
+
+static int dkr_pull_pull_layer_v2(DkrPull *i) {
+_cleanup_free_ char *path = NULL;
+const char *url, *layer = NULL;
+int r;
+
+assert(i);
+assert(!i-layer_job);
+assert(!i-temp_path);
+assert(!i-final_path);
+
+for (;;) {
+layer = dkr_pull_current_layer(i);
+if (!layer)
+return 0; /* no more layers */
+
+path = strjoin(i-image_root, /.dkr-, layer, NULL);
+

Re: [systemd-devel] [PATCH 2/5] shared/json: JSON parser + number tokenizer bugfix

2015-05-18 Thread Pavel Odvody 
On Fri, 2015-05-15 at 17:11 +0200, Lennart Poettering wrote:
 On Fri, 15.05.15 17:05, Pavel Odvody (podv...@redhat.com) wrote:
 
  On Fri, 2015-05-15 at 16:12 +0200, Lennart Poettering wrote:
   On Fri, 15.05.15 16:03, Pavel Odvody (podv...@redhat.com) wrote:
   
On Fri, 2015-05-15 at 15:23 +0200, Lennart Poettering wrote:
 On Thu, 07.05.15 17:47, Pavel Odvody (podv...@redhat.com) wrote:
 
 Hmm, so if I grok this right, then this at's a DOM-like (object
 model) parser for json, where we previously hat a SAX-like (stream)
 parser only. What's the rationale for this? Why doesn't the stream
 parser suffice?
 
 I intentionally opted for a stream parser when I wrote the code, and
 that#s actually the primary reason why i roleld my own parser here,
 instead of using some existing library
 

Hmm, I'd call it lexer/tokenizer, since the burden of syntactic analysis
is on the user. The parser is actually rather thin wrapper around
json_tokenize.

Rationale: the v2 manifest (also) contains embedded JSON documents and
is itself versioned, so it will change sooner or later.
I believe that parsing the manifest, or any decently complex JSON
document, using the stream parser would yield equal or bigger chunk of
code than generic DOM parser + few lines that consume it's API.
   
   Can you give an example of these embedded JSON documents?
   
   Couldn't this part be handled nicely by providing a call that skips
   nicely over json objects we don't want to process?
   
   Lennart
   
  
  http://pastebin.com/rrkVxHzT
  
  Yes, it could be handled, but I wouldn't call it nicely :)
  Since there's a lot of nested objects / arrays I guess that you'd need
  to do the syntactic analysis anyway. It'd be even worse in case some
  values would shadow the key names, or some part of the document were
  re-ordered.
 
 Well, what I really don't like about object parsers is that they might
 take unbounded memory, which is much less a problem with stream
 parsers...
 
 If we do object model parsing we really need to be careful with
 enforcing limits on everything...
 
 Lennart
 

Hmm, I could add a function measuring the size of the resulting object.

  int json_parse_check(const char* data, size_t *size);

Which accepts a JSON string and outputs the final size on success.

What do you think?

-- 
Pavel Odvody podv...@redhat.com
Software Engineer - EMEA ENG Developer Experience
5EC1 95C1 8E08 5BD9 9BBF 9241 3AFA 3A66 024F F68D
Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno



signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 3/5] test/test-json: Tests for the JSON parser and the tokenizer bugfix

2015-05-18 Thread Pavel Odvody 
On Fri, 2015-05-15 at 17:24 +0200, Lennart Poettering wrote:
 On Thu, 07.05.15 17:47, Pavel Odvody (podv...@redhat.com) wrote:
 
  Signed-off-by: Pavel Odvody podv...@redhat.com
  ---
   src/test/test-json.c | 16 
   1 file changed, 16 insertions(+)
  
  diff --git a/src/test/test-json.c b/src/test/test-json.c
  index 24dc700..745eeb0 100644
  --- a/src/test/test-json.c
  +++ b/src/test/test-json.c
  @@ -72,6 +72,17 @@ static void test_one(const char *data, ...) {
   va_end(ap);
   }
   
  +static void test_file(const char *data) {
  +json_variant *v = NULL;
  +int r = json_parse(data, v);
  +
  +assert_se(r == 0);
  +assert_se(v != NULL);
  +assert_se(v-type == JSON_VARIANT_OBJECT);
  +
  +json_variant_unref(v);
  +}
  +
   int main(int argc, char *argv[]) {
   
   test_one(x, -EINVAL);
  @@ -102,5 +113,10 @@ int main(int argc, char *argv[]) {
   test_one(\\\udc00\\udc00\, -EINVAL);
   test_one(\\\ud801\\udc37\, JSON_STRING, \xf0\x90\x90\xb7, 
  JSON_END);
   
  +test_one([1, 2], JSON_ARRAY_OPEN, JSON_INTEGER, 1, JSON_COMMA, 
  JSON_INTEGER, 2, JSON_ARRAY_CLOSE, JSON_END);
  +
  +test_file({\k\: \v\, \foo\: [1, 2, 3], \bar\: {\zap\: 
  null}});
  +test_file({\mutant\: [1, null, \1\, {\1\: [1, \1\]}], 
  \blah\: 1.27});
  +
 
 Any chance you can extend the test to check the structure of the of
 the object parsed for validity here?
 
 Lennart
 

Yes, though I wonder how to write down the test specs.

Something like this for the first test case?

JSON_SCOPE, 
 JSON_KEY, k, JSON_VALUE, v,
 JSON_KEY, foo, JSON_VALUE, 
 JSON_SCOPE, 
  JSON_VALUE, 1, JSON_VALUE, 2, JSON_VALUE, 3,
 JSON_ENDSCOPE, 
 JSON_KEY, bar, JSON_VALUE,
 JSON_SCOPE,
  JSON_KEY, zap, JSON_VALUE, NULL
 JSON_ENDSCOPE.
JSON_ENDSCOPE


-- 
Pavel Odvody podv...@redhat.com
Software Engineer - EMEA ENG Developer Experience
5EC1 95C1 8E08 5BD9 9BBF 9241 3AFA 3A66 024F F68D
Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno



signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/5] shared/json: JSON parser + number tokenizer bugfix

2015-05-15 Thread Pavel Odvody 
On Fri, 2015-05-15 at 15:23 +0200, Lennart Poettering wrote:
 On Thu, 07.05.15 17:47, Pavel Odvody (podv...@redhat.com) wrote:
 
 Hmm, so if I grok this right, then this at's a DOM-like (object
 model) parser for json, where we previously hat a SAX-like (stream)
 parser only. What's the rationale for this? Why doesn't the stream
 parser suffice?
 
 I intentionally opted for a stream parser when I wrote the code, and
 that#s actually the primary reason why i roleld my own parser here,
 instead of using some existing library
 

Hmm, I'd call it lexer/tokenizer, since the burden of syntactic analysis
is on the user. The parser is actually rather thin wrapper around
json_tokenize.

Rationale: the v2 manifest (also) contains embedded JSON documents and
is itself versioned, so it will change sooner or later.
I believe that parsing the manifest, or any decently complex JSON
document, using the stream parser would yield equal or bigger chunk of
code than generic DOM parser + few lines that consume it's API.

So to sum it up:

1) Syntactic analysis
2) Maintainability
3) Easier consumption


  Signed-off-by: Pavel Odvody podv...@redhat.com
  ---
   src/shared/json.c | 437 
  --
   src/shared/json.h |  36 +
   2 files changed, 463 insertions(+), 10 deletions(-)
  
  diff --git a/src/shared/json.c b/src/shared/json.c
  index 45c8ece..00d5fce 100644
  --- a/src/shared/json.c
  +++ b/src/shared/json.c
  @@ -21,17 +21,173 @@
 
 Lennart
 


-- 
Pavel Odvody podv...@redhat.com
Software Engineer - EMEA ENG Developer Experience
5EC1 95C1 8E08 5BD9 9BBF 9241 3AFA 3A66 024F F68D
Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno



signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 1/5] shared/import-util: tag renamed to reference to support v2 pull by digest

2015-05-15 Thread Pavel Odvody 
On Fri, 2015-05-15 at 15:20 +0200, Lennart Poettering wrote:
 On Thu, 07.05.15 17:47, Pavel Odvody (podv...@redhat.com) wrote:
 
  Signed-off-by: Pavel Odvody podv...@redhat.com
 
 We dont do S-o-b in systemd.
 
oops.

  ---
   src/shared/import-util.c | 19 +++
   src/shared/import-util.h |  1 +
   2 files changed, 20 insertions(+)
  
  diff --git a/src/shared/import-util.c b/src/shared/import-util.c
  index 660d92a..f2fb6d0 100644
  --- a/src/shared/import-util.c
  +++ b/src/shared/import-util.c
  @@ -150,6 +150,25 @@ int raw_strip_suffixes(const char *p, char **ret) {
   return 0;
   }
   
  +bool dkr_ref_is_valid(const char *ref) {
  +const char *colon;
  +
  +if (isempty(ref))
  +return false;
  +
  +colon = strchr(ref, ':');
  +if (!colon)
  +return filename_is_valid(ref);
  +
  +else if (!startswith(ref, sha256))
  +return false;
  +
  +else if (!in_charset(colon + 1, 0123456789abcdef))
  +return false;
 
 
 Hmm, how precisely do this refs look, can you provide some examples?
 
 Right now you do not validate anything between sha256 and the first
 :, that's not intended, is it?
 
 Lennart
 

You're right, there's a blind spot, the digest reference looks like
this:

sha256:7266a84a67d01165f222eac5785fed00791eb3aec0fd8a18086b76310280d9da

Thanks for catching this.

-- 
Pavel Odvody podv...@redhat.com
Software Engineer - EMEA ENG Developer Experience
5EC1 95C1 8E08 5BD9 9BBF 9241 3AFA 3A66 024F F68D
Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno



signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/5] shared/json: JSON parser + number tokenizer bugfix

2015-05-15 Thread Pavel Odvody 
On Fri, 2015-05-15 at 16:12 +0200, Lennart Poettering wrote:
 On Fri, 15.05.15 16:03, Pavel Odvody (podv...@redhat.com) wrote:
 
  On Fri, 2015-05-15 at 15:23 +0200, Lennart Poettering wrote:
   On Thu, 07.05.15 17:47, Pavel Odvody (podv...@redhat.com) wrote:
   
   Hmm, so if I grok this right, then this at's a DOM-like (object
   model) parser for json, where we previously hat a SAX-like (stream)
   parser only. What's the rationale for this? Why doesn't the stream
   parser suffice?
   
   I intentionally opted for a stream parser when I wrote the code, and
   that#s actually the primary reason why i roleld my own parser here,
   instead of using some existing library
   
  
  Hmm, I'd call it lexer/tokenizer, since the burden of syntactic analysis
  is on the user. The parser is actually rather thin wrapper around
  json_tokenize.
  
  Rationale: the v2 manifest (also) contains embedded JSON documents and
  is itself versioned, so it will change sooner or later.
  I believe that parsing the manifest, or any decently complex JSON
  document, using the stream parser would yield equal or bigger chunk of
  code than generic DOM parser + few lines that consume it's API.
 
 Can you give an example of these embedded JSON documents?
 
 Couldn't this part be handled nicely by providing a call that skips
 nicely over json objects we don't want to process?
 
 Lennart
 

http://pastebin.com/rrkVxHzT

Yes, it could be handled, but I wouldn't call it nicely :)
Since there's a lot of nested objects / arrays I guess that you'd need
to do the syntactic analysis anyway. It'd be even worse in case some
values would shadow the key names, or some part of the document were
re-ordered.

-- 
Pavel Odvody podv...@redhat.com
Software Engineer - EMEA ENG Developer Experience
5EC1 95C1 8E08 5BD9 9BBF 9241 3AFA 3A66 024F F68D
Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno



signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 5/5] import/pull-dkr: V2 Image specification + manifest support

2015-05-15 Thread Pavel Odvody 
On Fri, 2015-05-15 at 15:54 +0200, Lennart Poettering wrote:
 On Thu, 07.05.15 17:47, Pavel Odvody (podv...@redhat.com) wrote:
 
   
   #define HEADER_TOKEN X-Do /* the HTTP header for the auth token */ 
  cker-Token:
  -#define HEADER_REGISTRY X-Do /*the HTTP header for the registry */ 
  cker-Endpoints:
  +#define HEADER_REGISTRY X-Do /* the HTTP header for the registry */ 
  cker-Endpoints:
  +#define HEADER_DIGEST Do /* the HTTP header for the manifest digest */ 
  cker-Content-Digest:
  +#define HEADER_USER_AGENT_V2 User-Agent: do /* otherwise we get
  load-balanced(!) to a V1 registyry */ cker/1.6.0
 
 I hope we can get rid of this. I'd prefer if we wouldn't have to 
 pretend to be other software.

As of today we can, kudos to Vincent for pushing this.

 
  +#define HEADER_BEARER_REALM https://auth.doc; /* URL which we query for a 
  bearer token */ ker.io/token
  +#define HEADER_BEARER_SERVICE registry.doc /* the service we
  query the token for */ ker.io
 
 These two are problematic, we really shouldn't hardcode them. First of
 all users might want to use other servers here. More importantly
 though, to my knowledge the service agreements of the company
 providing those server does not allow usage like this. IANAL, but I
 really would prefer staying out of any legal issues here. We can make
 the defaults for this configurable via ./configure switches but we
 should not carry these URLs by default. If downstream distros are
 willing to take the legal risk of encoding the URLs by default then,
 they are welcome to via the configure switches, but upstream I really
 don't want to see that...
 
 Also, can you quickly explain how the three servers relate to each
 other? Is this documented anywhere?
 

I'm currently in the process of rewriting these to use the same domain
name as is passed in --dkr-index-url, so they'll look like:
${PROTOCOL}auth.${ADDRESS}/[v2]/token
and registry.${ADDRESS} respectively.

These are somehow documented here:
https://docs.docker.com/registry/spec/auth/token/

   
  +if (strategy == PULL_V2) {
  +char **k = NULL;
  +STRV_FOREACH(k, i-ancestry) {
  +char *d = strjoina(i-image_root, /.dkr-,
  *k, NULL);
 
 Using alloca() (which strjoina() uses internally) within loops is not
 OK, since deallacation happens at the end of the function call, not at
 the end of the {} block. 
Oh, didn't know that, will change.

  +
  +r = json_parse((const char *)j-payload, doc);
 
 Why the cast? void, and non-const automatically upgrade to const and
 any type...

I think that j-payload is uint8_t* and the function expects const
char*, what do you thing about changing the signature of json_parse to

int json_parse(const void* payload, size_t size, json_variant **rv);

?

 
   
  +enum PullStrategy { PULL_V1, PULL_V2 };
 
 So far we typedef'ed enums for exported types, and prefixed both with
 a namespacing prefix. Also, why call this a strategy? Does dkr
 upstream call it that way? Shouldn't this be protocol or so?
 
 typedef enum DkrProtocolVersion {
DKR_PROTOCOL_V1,
DKR_PROTOCOL_V2,
 } DkrProtocolVersion;

Nope, entirely my choice of a name. Protocol sounds more like switching
between HTTP/S, FTP or something, while the underlying network protocols
has stayed the same, only the strategy how the image is pulled has
changed. But I have no problem with changing that.

 
  +
  +typedef struct DkrSignature {
  +char *curve;
  +
  +char *key_id;
  +char *key_type;
  +
  +char *x;
  +char *y;
  +
  +char *algorithm;
  +
  +char *signature;
  +char *protected;
  +} DkrSignature;
 
 This is not used so far, is it?
 
  +
  +//typedef struct DkrHistory {
  +//  char *image_id;
  +//  char *parent_id;
  +//} DkrHistory;
 
 
 And neither is this? Also, we don't use C++ comments in our code,
 i.e. /* this */ is preferred over // this...

Yep, I'll get rid of these.

 
 Lennart
 


-- 
Pavel Odvody podv...@redhat.com
Software Engineer - EMEA ENG Developer Experience
5EC1 95C1 8E08 5BD9 9BBF 9241 3AFA 3A66 024F F68D
Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno



signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 4/5] import/pull: Tag replaced with reference

2015-05-15 Thread Pavel Odvody 
On Fri, 2015-05-15 at 15:29 +0200, Lennart Poettering wrote:
 On Thu, 07.05.15 17:47, Pavel Odvody (podv...@redhat.com) wrote:
 
 Hmm, can you quickly explain how tags and refs and digests relate?
 
 If refs replace tags in v2, then I figure we can also drop the
 dkr_tag_is_valid() definition from the headers, no?
 
 Lennart
 

Both are references to an image - tags can be changed at any time, while
the digest is unique to the given manifest (immutable).

I wanted to keep dkr_tag_is_valid for the V1 compatibility as specifying
the digest in V1 workflow is an error.


-- 
Pavel Odvody podv...@redhat.com
Software Engineer - EMEA ENG Developer Experience
5EC1 95C1 8E08 5BD9 9BBF 9241 3AFA 3A66 024F F68D
Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno



signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 4/5] import/pull: Tag replaced with reference

2015-05-15 Thread Pavel Odvody 
On Fri, 2015-05-15 at 16:35 +0200, Lennart Poettering wrote:
 On Fri, 15.05.15 16:32, Pavel Odvody (podv...@redhat.com) wrote:
 
  On Fri, 2015-05-15 at 15:29 +0200, Lennart Poettering wrote:
   On Thu, 07.05.15 17:47, Pavel Odvody (podv...@redhat.com) wrote:
   
   Hmm, can you quickly explain how tags and refs and digests relate?
   
   If refs replace tags in v2, then I figure we can also drop the
   dkr_tag_is_valid() definition from the headers, no?
   
   Lennart
   
  
  Both are references to an image - tags can be changed at any time, while
  the digest is unique to the given manifest (immutable).
 
 And what is a ref precisely?
 
 Is ref supposed to be a generic term for digest or tag, the way
 git defines this?
 
 Lennart
 

Yep, short for reference.

-- 
Pavel Odvody podv...@redhat.com
Software Engineer - EMEA ENG Developer Experience
5EC1 95C1 8E08 5BD9 9BBF 9241 3AFA 3A66 024F F68D
Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno



signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 0/5] systemd-importd - support for pulling from V2 Dkr registries

2015-05-11 Thread Pavel Odvody 
On Mon, 2015-05-11 at 10:32 -0400, Vincent Batts wrote:
 On 09/05/15 00:31 +0200, Pavel Odvody wrote:
 On Fri, 2015-05-08 at 14:33 -0400, Vincent Batts wrote:
  On 08/05/15 11:31 +1000, Daurnimator wrote:
  On 8 May 2015 at 01:46, Pavel Odvody podv...@redhat.com wrote:
- To access the V2 registry we need to send a special User-Agent
  docker/1.6.0
  
  Is this really required?
  Can we request they change something server side?
 
  I would have to double check the behavior on their docker hub, but for
  local registries this user agent header is not required. It is the
  expectation that a docker registry can always be served as a static file
  tree (pull only).
 
  vb
 
 Hey,
 
 $ curl -XGET https://registry-1.docker.io/v2/library/node/manifests/latest
 !DOCTYPE HTML PUBLIC -//W3C//DTD HTML 3.2 Final//EN
 title404 Not Found/title
 h1Not Found/h1
 pThe requested URL was not found on the server.  If you entered the URL 
 manually please check your spelling and try again./p
 
 $ curl -XGET -HUser-Agent: docker/1.6.0 
 https://registry-1.docker.io/v2/library/node/manifests/latest
 {errors:[{code:UNAUTHORIZED,message:access to the requested 
 resource is not 
 authorized,detail:[{Type:repository,Name:library/node,Action:pull}]}]}
 
 I actually added a little clarification in my 5th patch:
 
 User-Agent: do /* otherwise we get load-balanced(!) to a V1 registyry */
 (I got this information from Andy G.)
 
 The second request obviously fails due to the bearer token not being 
 provided,
 but at least we can see that we're hitting the correct endpoint here.
 
 I think that this is the correct behavior, since the original systemd-pull
 workflow was to check the Hub first and obtain the token, which I'm simply
 following here, however the token is now obtained from a separate endpoint.
 
 The thing is that the argument is --dkr-index-url, so we're actually 
 specifying
 the Hub URL here and there's no way to specify a registry alone.
 (the mirror registry is received in HTTP headers from the Hub)
 
 Sounds like a topic for another patch?
 
 I hope that the pull-only policy will be relaxed soon :) A lot of roundtrips 
 ...
 
 I understand they've done this on their hub, to route client versions 
  1.6.0 which can not do the v2 api. There ought to be a way not no
 require UA headers. Will see what I can do.
 
 vb

I find that solution rather unfortunate, since the endpoints are already
versioned /v1/ and /v2/ respectively.
The clients before 1.6.0 have hardcoded the /v1/ endpoints so really no
additional reason for this. (oh, didn't 1.5.0 ship half-assed
implementation of v2 with old header names?)

-- 
Pavel Odvody podv...@redhat.com
Software Engineer - EMEA ENG Developer Experience
5EC1 95C1 8E08 5BD9 9BBF 9241 3AFA 3A66 024F F68D
Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno



signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 0/5] systemd-importd - support for pulling from V2 Dkr registries

2015-05-08 Thread Pavel Odvody 
On Fri, 2015-05-08 at 14:33 -0400, Vincent Batts wrote:
 On 08/05/15 11:31 +1000, Daurnimator wrote:
 On 8 May 2015 at 01:46, Pavel Odvody podv...@redhat.com wrote:
   - To access the V2 registry we need to send a special User-Agent
 docker/1.6.0
 
 Is this really required?
 Can we request they change something server side?
 
 I would have to double check the behavior on their docker hub, but for
 local registries this user agent header is not required. It is the
 expectation that a docker registry can always be served as a static file
 tree (pull only).
 
 vb

Hey,

$ curl -XGET https://registry-1.docker.io/v2/library/node/manifests/latest
!DOCTYPE HTML PUBLIC -//W3C//DTD HTML 3.2 Final//EN
title404 Not Found/title
h1Not Found/h1
pThe requested URL was not found on the server.  If you entered the URL 
manually please check your spelling and try again./p

$ curl -XGET -HUser-Agent: docker/1.6.0 
https://registry-1.docker.io/v2/library/node/manifests/latest   
   
{errors:[{code:UNAUTHORIZED,message:access to the requested resource 
is not 
authorized,detail:[{Type:repository,Name:library/node,Action:pull}]}]}

I actually added a little clarification in my 5th patch:

User-Agent: do /* otherwise we get load-balanced(!) to a V1 registyry */ 
(I got this information from Andy G.)

The second request obviously fails due to the bearer token not being provided,
but at least we can see that we're hitting the correct endpoint here.

I think that this is the correct behavior, since the original systemd-pull 
workflow was to check the Hub first and obtain the token, which I'm simply
following here, however the token is now obtained from a separate endpoint.

The thing is that the argument is --dkr-index-url, so we're actually specifying
the Hub URL here and there's no way to specify a registry alone.
(the mirror registry is received in HTTP headers from the Hub)

Sounds like a topic for another patch?

I hope that the pull-only policy will be relaxed soon :) A lot of roundtrips ...

-- 
Pavel Odvody podv...@redhat.com
Software Engineer - EMEA ENG Developer Experience
5EC1 95C1 8E08 5BD9 9BBF 9241 3AFA 3A66 024F F68D
Red Hat Czech s.r.o., Purkyňova 99/71, 612 45, Brno



signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 4/5] import/pull: Tag replaced with reference

2015-05-07 Thread Pavel Odvody 
Signed-off-by: Pavel Odvody podv...@redhat.com
---
 src/import/pull.c | 28 +---
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/src/import/pull.c b/src/import/pull.c
index ef7b035..8054612 100644
--- a/src/import/pull.c
+++ b/src/import/pull.c
@@ -227,7 +227,7 @@ static void on_dkr_finished(DkrPull *pull, int error, void 
*userdata) {
 static int pull_dkr(int argc, char *argv[], void *userdata) {
 _cleanup_(dkr_pull_unrefp) DkrPull *pull = NULL;
 _cleanup_event_unref_ sd_event *event = NULL;
-const char *name, *tag, *local;
+const char *name, *reference, *local, *digest;
 int r;
 
 if (!arg_dkr_index_url) {
@@ -240,13 +240,19 @@ static int pull_dkr(int argc, char *argv[], void 
*userdata) {
 return -EINVAL;
 }
 
-tag = strchr(argv[1], ':');
-if (tag) {
-name = strndupa(argv[1], tag - argv[1]);
-tag++;
+digest = strchr(argv[1], '@');
+if (digest) {
+reference = digest + 1;
+name = strndupa(argv[1], digest - argv[1]);
+}
+
+reference = strchr(argv[1], ':');
+if (reference) {
+name = strndupa(argv[1], reference - argv[1]);
+reference++;
 } else {
 name = argv[1];
-tag = latest;
+reference = latest;
 }
 
 if (!dkr_name_is_valid(name)) {
@@ -254,8 +260,8 @@ static int pull_dkr(int argc, char *argv[], void *userdata) 
{
 return -EINVAL;
 }
 
-if (!dkr_tag_is_valid(tag)) {
-log_error(Tag name '%s' is not valid., tag);
+if (!dkr_ref_is_valid(reference)) {
+log_error(Tag name '%s' is not valid., reference);
 return -EINVAL;
 }
 
@@ -288,9 +294,9 @@ static int pull_dkr(int argc, char *argv[], void *userdata) 
{
 }
 }
 
-log_info(Pulling '%s' with tag '%s', saving as '%s'., name, 
tag, local);
+log_info(Pulling '%s' with reference '%s', saving as '%s'., 
name, reference, local);
 } else
-log_info(Pulling '%s' with tag '%s'., name, tag);
+log_info(Pulling '%s' with reference '%s'., name, reference);
 
 r = sd_event_default(event);
 if (r  0)
@@ -304,7 +310,7 @@ static int pull_dkr(int argc, char *argv[], void *userdata) 
{
 if (r  0)
 return log_error_errno(r, Failed to allocate puller: %m);
 
-r = dkr_pull_start(pull, name, tag, local, arg_force);
+r = dkr_pull_start(pull, name, reference, local, arg_force, PULL_V2);
 if (r  0)
 return log_error_errno(r, Failed to pull image: %m);
 
-- 
2.1.0





signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 0/5] systemd-importd - support for pulling from V2 Dkr registries

2015-05-07 Thread Pavel Odvody 
Hi,

the attached series of patches add support for pulling from V2 docker
registries, so let me break down first what happened to the format since
V1
 - Image is now defined by a JSON manifest
  - contains fields like name, tag, schemaVersion ...
  - and fsLayers - which is an array of sha256 references to a
*content-addressable FS layers*
  - the manifest is now also signed using JWS/JWT (ECDSA p-256 mostly)
 - Authentication/Authorization now bearer token only
 - To access the V2 registry we need to send a special User-Agent
   docker/1.6.0
 - The whole manifest can be hashed using sha256 to obtain a 
   digest, which provides an immutable global identifier of the image,
   and can be used instead of a tag when pulling the image (the REST
   API endpoints are the same).

So far so good, now what's in the patches, besides the V2 workflow
 - lightweight JSON parser, written around json_tokenize
 - I've renamed 'tag' to 'reference' to accommodate for the digest
   semantics
 - all layers are saved in a directory .dkr-$imageid - image id is
   resolved from the v1 compatibility section of the manifest
  - since the layers are now CAS, we can't assume that the order, or
mere presence of certain layers will be preserved throughout
multitude of images/manifests, and therefore due to the
incremental nature of BTRFS snapshots we need to throw any
intermediary snapshots away.
 - small bugfix for the JSON tokenizer (it'd choke after reading 
   any digit)

This is the bare minimum to pullrun V2 images, since the signature is
now embedded in the manifest, it could now support --verify=signature. 
However, I've got one open question - how do we support V1/V2
concurrently (this patch makes V2 the default and only)? Docker first
pings the V2 endpoint and then falls back to V1, but I think that this is 
sub optimal, since --verify=signature makes sense only with V2, so I think 
something like
  
   --dkr-pull-strategy=v1|v2

as an argument would be the best?

Thanks,

Pavel

Pavel Odvody (5):
  shared/import-util: tag renamed to reference to support v2 pull by
digest
  shared/json: JSON parser + number tokenizer bugfix
  test/test-json: Tests for the JSON parser and the tokenizer bugfix
  import/pull: Tag replaced with reference
  import/pull-dkr: V2 Image specification + manifest support

 src/import/pull-dkr.c| 531 +--
 src/import/pull-dkr.h|  48 -
 src/import/pull.c|  28 ++-
 src/shared/import-util.c |  19 ++
 src/shared/import-util.h |   1 +
 src/shared/json.c| 437 +-
 src/shared/json.h|  36 
 src/test/test-json.c |  16 ++
 8 files changed, 1034 insertions(+), 82 deletions(-)

-- 
2.1.0




signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 3/5] test/test-json: Tests for the JSON parser and the tokenizer bugfix

2015-05-07 Thread Pavel Odvody 
Signed-off-by: Pavel Odvody podv...@redhat.com
---
 src/test/test-json.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/src/test/test-json.c b/src/test/test-json.c
index 24dc700..745eeb0 100644
--- a/src/test/test-json.c
+++ b/src/test/test-json.c
@@ -72,6 +72,17 @@ static void test_one(const char *data, ...) {
 va_end(ap);
 }
 
+static void test_file(const char *data) {
+json_variant *v = NULL;
+int r = json_parse(data, v);
+
+assert_se(r == 0);
+assert_se(v != NULL);
+assert_se(v-type == JSON_VARIANT_OBJECT);
+
+json_variant_unref(v);
+}
+
 int main(int argc, char *argv[]) {
 
 test_one(x, -EINVAL);
@@ -102,5 +113,10 @@ int main(int argc, char *argv[]) {
 test_one(\\\udc00\\udc00\, -EINVAL);
 test_one(\\\ud801\\udc37\, JSON_STRING, \xf0\x90\x90\xb7, 
JSON_END);
 
+test_one([1, 2], JSON_ARRAY_OPEN, JSON_INTEGER, 1, JSON_COMMA, 
JSON_INTEGER, 2, JSON_ARRAY_CLOSE, JSON_END);
+
+test_file({\k\: \v\, \foo\: [1, 2, 3], \bar\: {\zap\: 
null}});
+test_file({\mutant\: [1, null, \1\, {\1\: [1, \1\]}], 
\blah\: 1.27});
+
 return 0;
 }
-- 
2.1.0



signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/5] shared/import-util: tag renamed to reference to support v2 pull by digest

2015-05-07 Thread Pavel Odvody 
Signed-off-by: Pavel Odvody podv...@redhat.com
---
 src/shared/import-util.c | 19 +++
 src/shared/import-util.h |  1 +
 2 files changed, 20 insertions(+)

diff --git a/src/shared/import-util.c b/src/shared/import-util.c
index 660d92a..f2fb6d0 100644
--- a/src/shared/import-util.c
+++ b/src/shared/import-util.c
@@ -150,6 +150,25 @@ int raw_strip_suffixes(const char *p, char **ret) {
 return 0;
 }
 
+bool dkr_ref_is_valid(const char *ref) {
+const char *colon;
+
+if (isempty(ref))
+return false;
+
+colon = strchr(ref, ':');
+if (!colon)
+return filename_is_valid(ref);
+
+else if (!startswith(ref, sha256))
+return false;
+
+else if (!in_charset(colon + 1, 0123456789abcdef))
+return false;
+
+return true;
+}
+
 bool dkr_name_is_valid(const char *name) {
 const char *slash, *p;
 
diff --git a/src/shared/import-util.h b/src/shared/import-util.h
index ff155b0..8f47f91 100644
--- a/src/shared/import-util.h
+++ b/src/shared/import-util.h
@@ -44,4 +44,5 @@ int raw_strip_suffixes(const char *name, char **ret);
 
 bool dkr_name_is_valid(const char *name);
 bool dkr_id_is_valid(const char *id);
+bool dkr_ref_is_valid(const char *ref);
 #define dkr_tag_is_valid(tag) filename_is_valid(tag)
-- 
2.1.0



signature.asc
Description: This is a digitally signed message part
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/5] shared/json: JSON parser + number tokenizer bugfix

2015-05-07 Thread Pavel Odvody 
Signed-off-by: Pavel Odvody podv...@redhat.com
---
 src/shared/json.c | 437 --
 src/shared/json.h |  36 +
 2 files changed, 463 insertions(+), 10 deletions(-)

diff --git a/src/shared/json.c b/src/shared/json.c
index 45c8ece..00d5fce 100644
--- a/src/shared/json.c
+++ b/src/shared/json.c
@@ -21,17 +21,173 @@
 
 #include sys/types.h
 #include math.h
-
 #include macro.h
-#include util.h
 #include utf8.h
 #include json.h
 
-enum {
-STATE_NULL,
-STATE_VALUE,
-STATE_VALUE_POST,
-};
+int json_variant_new(json_variant **ret, int type) {
+json_variant *v;
+assert(!*ret);
+v = new0(json_variant, 1);
+if (!v)
+return -ENOMEM;
+v-type = type;
+v-size = 0;
+v-obj  = NULL;
+*ret = v;
+return 0;
+}
+
+static int json_variant_deep_copy(json_variant *ret, json_variant *variant) {
+assert(ret);
+assert(variant);
+
+ret-type = variant-type;
+ret-size = variant-size;
+
+if (variant-type == JSON_VARIANT_STRING) {
+ret-string = strndup(variant-string, variant-size);
+if (!ret-string)
+return -ENOMEM;
+} else if (variant-type == JSON_VARIANT_ARRAY || variant-type == 
JSON_VARIANT_OBJECT) {
+ret-obj = new0(json_variant, variant-size);
+if (!ret-obj)
+return -ENOMEM;
+
+for (unsigned i = 0; i  variant-size; ++i) {
+int r;
+r = json_variant_deep_copy(ret-obj[i], 
variant-obj[i]);
+if (r  0)
+return r;
+}
+}
+else
+ret-value = variant-value;
+
+return 0;
+}
+
+static json_variant *json_object_unref(json_variant *variant);
+
+static json_variant *json_variant_unref_inner(json_variant *variant) {
+if (!variant)
+return NULL;
+
+if (variant-type == JSON_VARIANT_ARRAY || variant-type == 
JSON_VARIANT_OBJECT)
+return json_object_unref(variant);
+
+else if (variant-type == JSON_VARIANT_STRING)
+free(variant-string);
+
+return NULL;
+}
+
+static json_variant *json_raw_unref(json_variant *variant, size_t size) {
+if (!variant)
+return NULL;
+
+for (size_t i = 0; i  size; ++i)
+json_variant_unref_inner(variant[i]);
+
+free(variant);
+return NULL;
+}
+
+static json_variant *json_object_unref(json_variant *variant) {
+assert(variant);
+if (!variant-obj)
+return NULL;
+
+for (unsigned i = 0; i  variant-size; ++i)
+json_variant_unref_inner(variant-obj[i]);
+
+free(variant-obj);
+return NULL;
+}
+
+static json_variant **json_variant_array_unref(json_variant **variant) {
+size_t i = 0;
+json_variant *p = NULL;
+
+if (!variant)
+return NULL;
+
+while((p = (variant[i++])) != NULL) {
+if (p-type == JSON_VARIANT_STRING)
+   free(p-string);
+free(p);
+}
+
+free(variant);
+
+return NULL;
+}
+DEFINE_TRIVIAL_CLEANUP_FUNC(json_variant **, json_variant_array_unref);
+
+json_variant *json_variant_unref(json_variant *variant) {
+if (!variant)
+return NULL;
+
+if (variant-type == JSON_VARIANT_ARRAY || variant-type == 
JSON_VARIANT_OBJECT)
+return json_object_unref(variant);
+
+else if (variant-type == JSON_VARIANT_STRING)
+free(variant-string);
+
+free(variant);
+
+return NULL;
+}
+
+char *json_variant_string(json_variant *variant){
+assert(variant);
+assert(variant-type == JSON_VARIANT_STRING);
+
+return variant-string;
+}
+
+bool json_variant_bool(json_variant *variant) {
+assert(variant);
+assert(variant-type == JSON_VARIANT_BOOLEAN);
+
+return variant-value.boolean;
+}
+
+intmax_t json_variant_integer(json_variant *variant) {
+assert(variant);
+assert(variant-type == JSON_VARIANT_INTEGER);
+
+return variant-value.integer;
+}
+
+double json_variant_real(json_variant *variant) {
+assert(variant);
+assert(variant-type == JSON_VARIANT_REAL);
+
+return variant-value.real;
+}
+
+json_variant *json_variant_element(json_variant *variant, unsigned index) {
+assert(variant);
+assert(variant-type == JSON_VARIANT_ARRAY || variant-type == 
JSON_VARIANT_OBJECT);
+assert(index  variant-size);
+assert(variant-obj);
+
+return variant-obj[index];
+}
+
+json_variant *json_variant_value(json_variant *variant, const char *key) {
+assert(variant);
+assert(variant-type == JSON_VARIANT_OBJECT

Re: [systemd-devel] [PATCH 5/5] import/pull-dkr: V2 Image specification + manifest support

2015-05-07 Thread Pavel Odvody 
On Thu, 2015-05-07 at 18:58 +0300, Reverend Homer wrote:
 
 
 07.05.2015 18:47, Pavel Odvody пишет:
 
  Signed-off-by: Pavel Odvody podv...@redhat.com
  ---
   src/import/pull-dkr.c | 531 
  --
   src/import/pull-dkr.h |  48 -
   2 files changed, 518 insertions(+), 61 deletions(-)
  
  diff --git a/src/import/pull-dkr.c b/src/import/pull-dkr.c
  index 0eefec5..408b795 100644
  --- a/src/import/pull-dkr.c
  +++ b/src/import/pull-dkr.c
  @@ -61,9 +61,10 @@ struct DkrPull {
   PullJob *layer_job;
   
   char *name;
  -char *tag;
  +char *reference;
   char *id;
   
  +char *response_digest;
   char *response_token;
   char **response_registries;
   
  @@ -87,7 +88,11 @@ struct DkrPull {
   #define PROTOCOL_PREFIX https://;
   
   #define HEADER_TOKEN X-Do /* the HTTP header for the auth token */ 
  cker-Token:
  -#define HEADER_REGISTRY X-Do /*the HTTP header for the registry */ 
  cker-Endpoints:
  +#define HEADER_REGISTRY X-Do /* the HTTP header for the registry */ 
  cker-Endpoints:
  +#define HEADER_DIGEST Do /* the HTTP header for the manifest digest */ 
  cker-Content-Digest:
  +#define HEADER_USER_AGENT_V2 User-Agent: do /* otherwise we get 
  load-balanced(!) to a V1 registyry */ cker/1.6.0
  +#define HEADER_BEARER_REALM https://auth.doc; /* URL which we query for a 
  bearer token */ ker.io/token
  +#define HEADER_BEARER_SERVICE registry.doc /* the service we query the 
  token for */ ker.io
   
   #define LAYERS_MAX 2048
   
  @@ -117,7 +122,7 @@ DkrPull* dkr_pull_unref(DkrPull *i) {
   }
   
   free(i-name);
  -free(i-tag);
  +free(i-reference);
   free(i-id);
   free(i-response_token);
   free(i-response_registries);
  @@ -416,10 +421,25 @@ static int dkr_pull_add_token(DkrPull *i, PullJob *j) 
  {
   return 0;
   }
   
  +static int dkr_pull_add_bearer_token(DkrPull *i, PullJob *j) {
  +const char *t = NULL;
  +
  +assert(i);
  +assert(j);
  +
  +if (i-response_token)
  +t = strjoina(Authorization: Bearer , i-response_token);
  +
  +j-request_header = curl_slist_new(HEADER_USER_AGENT_V2, Accept: 
  application/json, t, NULL);
  +if (!j-request_header)
  +return -ENOMEM;
  +
  +return 0;
  +}
  +
   static bool dkr_pull_is_done(DkrPull *i) {
   assert(i);
   assert(i-images_job);
  -
   if (i-images_job-state != PULL_JOB_DONE)
   return false;
   
  @@ -429,7 +449,7 @@ static bool dkr_pull_is_done(DkrPull *i) {
   if (!i-ancestry_job || i-ancestry_job-state != PULL_JOB_DONE)
   return false;
   
  -if (!i-json_job || i-json_job-state != PULL_JOB_DONE)
  +if (i-json_job  i-json_job-state != PULL_JOB_DONE)
   return false;
   
   if (i-layer_job  i-layer_job-state != PULL_JOB_DONE)
  @@ -441,8 +461,9 @@ static bool dkr_pull_is_done(DkrPull *i) {
   return true;
   }
   
  -static int dkr_pull_make_local_copy(DkrPull *i) {
  +static int dkr_pull_make_local_copy(DkrPull *i, enum PullStrategy 
  strategy) {
   int r;
  +_cleanup_free_ char *p = NULL;
   
   assert(i);
   
  @@ -455,10 +476,30 @@ static int dkr_pull_make_local_copy(DkrPull *i) {
   return log_oom();
   }
   
  -r = pull_make_local_copy(i-final_path, i-image_root, i-local, 
  i-force_local);
  +if (strategy == PULL_V2) {
  +r = path_get_parent(i-image_root, p);
  +if (r  0)
  +return r;
  +}
  +
  +r = pull_make_local_copy(i-final_path, p ?: i-image_root, 
  i-local, i-force_local);
   if (r  0)
   return r;
   
  +if (strategy == PULL_V2) {
  +char **k = NULL;
  +STRV_FOREACH(k, i-ancestry) {
  +char *d = strjoina(i-image_root, /.dkr-, *k, 
  NULL);
  +r = btrfs_subvol_remove(d, false);
  +if (r  0)
  +   return r;
  +}
  +
  +r = rmdir(i-image_root);
  +if (r  0)
  +return r;
  +}
  +
   return 0;
   }
   
  @@ -516,6 +557,68 @@ static void dkr_pull_job_on_progress(PullJob *j) {
  
  DKR_DOWNLOADING);
   }
   
  +static void dkr_pull_job_on_finished_v2(PullJob *j);
  +
  +static int dkr_pull_pull_layer_v2(DkrPull *i) {
  +_cleanup_free_ char *path = NULL;
  +const char *url, *layer = NULL;
  +int r;
  +
  +assert(i);
  +assert(!i-layer_job);
  +assert(!i-temp_path);
  +assert(!i-final_path);
  +
  +for (;;) {
  +layer

[systemd-devel] [PATCH 5/5] import/pull-dkr: V2 Image specification + manifest support

2015-05-07 Thread Pavel Odvody 
Signed-off-by: Pavel Odvody podv...@redhat.com
---
 src/import/pull-dkr.c | 531 --
 src/import/pull-dkr.h |  48 -
 2 files changed, 518 insertions(+), 61 deletions(-)

diff --git a/src/import/pull-dkr.c b/src/import/pull-dkr.c
index 0eefec5..408b795 100644
--- a/src/import/pull-dkr.c
+++ b/src/import/pull-dkr.c
@@ -61,9 +61,10 @@ struct DkrPull {
 PullJob *layer_job;
 
 char *name;
-char *tag;
+char *reference;
 char *id;
 
+char *response_digest;
 char *response_token;
 char **response_registries;
 
@@ -87,7 +88,11 @@ struct DkrPull {
 #define PROTOCOL_PREFIX https://;
 
 #define HEADER_TOKEN X-Do /* the HTTP header for the auth token */ 
cker-Token:
-#define HEADER_REGISTRY X-Do /*the HTTP header for the registry */ 
cker-Endpoints:
+#define HEADER_REGISTRY X-Do /* the HTTP header for the registry */ 
cker-Endpoints:
+#define HEADER_DIGEST Do /* the HTTP header for the manifest digest */ 
cker-Content-Digest:
+#define HEADER_USER_AGENT_V2 User-Agent: do /* otherwise we get 
load-balanced(!) to a V1 registyry */ cker/1.6.0
+#define HEADER_BEARER_REALM https://auth.doc; /* URL which we query for a 
bearer token */ ker.io/token
+#define HEADER_BEARER_SERVICE registry.doc /* the service we query the token 
for */ ker.io
 
 #define LAYERS_MAX 2048
 
@@ -117,7 +122,7 @@ DkrPull* dkr_pull_unref(DkrPull *i) {
 }
 
 free(i-name);
-free(i-tag);
+free(i-reference);
 free(i-id);
 free(i-response_token);
 free(i-response_registries);
@@ -416,10 +421,25 @@ static int dkr_pull_add_token(DkrPull *i, PullJob *j) {
 return 0;
 }
 
+static int dkr_pull_add_bearer_token(DkrPull *i, PullJob *j) {
+const char *t = NULL;
+
+assert(i);
+assert(j);
+
+if (i-response_token)
+t = strjoina(Authorization: Bearer , i-response_token);
+
+j-request_header = curl_slist_new(HEADER_USER_AGENT_V2, Accept: 
application/json, t, NULL);
+if (!j-request_header)
+return -ENOMEM;
+
+return 0;
+}
+
 static bool dkr_pull_is_done(DkrPull *i) {
 assert(i);
 assert(i-images_job);
-
 if (i-images_job-state != PULL_JOB_DONE)
 return false;
 
@@ -429,7 +449,7 @@ static bool dkr_pull_is_done(DkrPull *i) {
 if (!i-ancestry_job || i-ancestry_job-state != PULL_JOB_DONE)
 return false;
 
-if (!i-json_job || i-json_job-state != PULL_JOB_DONE)
+if (i-json_job  i-json_job-state != PULL_JOB_DONE)
 return false;
 
 if (i-layer_job  i-layer_job-state != PULL_JOB_DONE)
@@ -441,8 +461,9 @@ static bool dkr_pull_is_done(DkrPull *i) {
 return true;
 }
 
-static int dkr_pull_make_local_copy(DkrPull *i) {
+static int dkr_pull_make_local_copy(DkrPull *i, enum PullStrategy strategy) {
 int r;
+_cleanup_free_ char *p = NULL;
 
 assert(i);
 
@@ -455,10 +476,30 @@ static int dkr_pull_make_local_copy(DkrPull *i) {
 return log_oom();
 }
 
-r = pull_make_local_copy(i-final_path, i-image_root, i-local, 
i-force_local);
+if (strategy == PULL_V2) {
+r = path_get_parent(i-image_root, p);
+if (r  0)
+return r;
+}
+
+r = pull_make_local_copy(i-final_path, p ?: i-image_root, i-local, 
i-force_local);
 if (r  0)
 return r;
 
+if (strategy == PULL_V2) {
+char **k = NULL;
+STRV_FOREACH(k, i-ancestry) {
+char *d = strjoina(i-image_root, /.dkr-, *k, NULL);
+r = btrfs_subvol_remove(d, false);
+if (r  0)
+   return r;
+}
+
+r = rmdir(i-image_root);
+if (r  0)
+return r;
+}
+
 return 0;
 }
 
@@ -516,6 +557,68 @@ static void dkr_pull_job_on_progress(PullJob *j) {

DKR_DOWNLOADING);
 }
 
+static void dkr_pull_job_on_finished_v2(PullJob *j);
+
+static int dkr_pull_pull_layer_v2(DkrPull *i) {
+_cleanup_free_ char *path = NULL;
+const char *url, *layer = NULL;
+int r;
+
+assert(i);
+assert(!i-layer_job);
+assert(!i-temp_path);
+assert(!i-final_path);
+
+for (;;) {
+layer = dkr_pull_current_layer(i);
+if (!layer)
+return 0; /* no more layers */
+
+path = strjoin(i-image_root, /.dkr-, layer, NULL);
+if (!path)
+return log_oom();
+
+if (laccess(path, F_OK)  0) {
+if (errno == ENOENT)
+break;
+
+return