race, storage: add independent AddNames and RemoveNames for images,layers and containers

[flouthoc]

Adds SetNameWithOptions so operations which are invoke in parallel
manner can use it without destroying names from storage.

For instance

We are deleting names which were already written in store.
This creates faulty behavior when builds are invoked in parallel manner, as
this removes names for other builds.

To fix this behavior we must append to already written names and
override if needed. But this should be optional and not break public API

Following patch will be used by parallel operations at podman or buildah end, directly or indirectly.

PR replaces: #1152

[vrothberg]

Would a {Add,Delete}Name API address the races?

[flouthoc]

Would a {Add,Delete}Name API address the races?

@vrothberg Bunch of code in common or podman or buildah needs to be instrumented slowly. So this seems most intuitive from what we discussed here #1152 .

There are some other things as well where we might have race but maybe that is part of a different PR.

[giuseppe]

code changes LGTM

[giuseppe]

Would a {Add,Delete}Name API address the races?

Wouldn't that be more difficult to handle for the caller? If it is adding multiple names and the operation fails middle way, should it then try to delete previously added ones?

[mtrmac]

If we need atomic name additions, we certainly also need atomic name removals. So I’d expect something like UpdateNames(id string, updates NameUpdates) with type NameUpdates struct { addNames, removeNames []string }

There was also an issue I can’t quickly find about store.CreateImage doing name updates and other changes atomically with other operations, that’s why I suggest a type NameUpdates that could be reused in that other feature.

[mtrmac]

@vrothberg Bunch of code in common or podman or buildah needs to be instrumented slowly.

What difference does that make? Every line calling SetNames should be edited either way, whatever form we design for the new API.

[mtrmac]

.

[mtrmac]

Also, if we know the old APIs are racy, they should be documented as deprecated.

[vrothberg]

@vrothberg Bunch of code in common or podman or buildah needs to be instrumented slowly.

What difference does that make? Every line calling SetNames should be edited either way, whatever form we design for the new API.

Yes, SetNames should not be used anymore. IIUIC, the race is in reading all data, followed by writing all data. An add(), delete() could just take care of synchronization and callers don't have to worry.

[mtrmac]

There was also an issue I can’t quickly find about store.CreateImage doing name updates and other changes atomically with other operations

#595

[flouthoc]

If we have a consensus on new API then i guess its cool. We can create a new API.

[vrothberg]

Not at all a thorough review