[Devel] Re: Supporting patches for netns/netdev (v2)

[Serge E. Hallyn]

Quoting Dan Smith (danms at us.ibm.com):
> OL> Both look good.
> 
> Okay, but per Serge's suggestion shall we change the restore_obj() to
> the one included below? :)
> 
> -- 
> Dan Smith
> IBM Linux Technology Center
> email: danms at us.ibm.com
> 
> commit d0c71c159decd47c4a6f9778d02dc521b74ff414
> Author: Dan Smith <danms at us.ibm.com>
> Date:   Tue Feb 9 14:04:23 2010 -0800
> 
>     Make restore_obj() tolerate a preexisting object in the hash (v2)
>     
>     ... as long as the pointer is the same as that returned from the restore
>     function.  Also move the compulsory ref_drop() so that it only gets
>     done if we created the new object.
>     
>     The existing object tolerance is important for netdev restore because it
>     means that I can refer to a peer by its objref instead of needing the
>     (previously-rejected) veth_peer() function.  If this is not acceptable,
>     then I'll need to keep a separate list of pairs.
>     
>     Changes in v2:
>      - Check that the type of the object already in the hash matches that
>        of the objref header we're reading.
>      - Add a comment about why and how we might get into this sort of
>        situation.
>     
>     Signed-off-by: Dan Smith <danms at us.ibm.com>

Acked-by: Serge Hallyn <serue at us.ibm.com>

/me thinks he'd better send a patch defining CKPT_OBJ_LASTREF to help out
those poor ->obj_ref(x, 0) who so badly want to be more informative...

thanks,
-serge

> 
> diff --git a/checkpoint/objhash.c b/checkpoint/objhash.c
> index 0b06b06..4ca7799 100644
> --- a/checkpoint/objhash.c
> +++ b/checkpoint/objhash.c
> @@ -1059,16 +1059,29 @@ int restore_obj(struct ckpt_ctx *ctx, struct ckpt_hdr_ob
>         if (IS_ERR(ptr))
>                 return PTR_ERR(ptr);
> 
> -       if (obj_find_by_objref(ctx, h->objref))
> -               obj = ERR_PTR(-EINVAL);
> -       else
> +       obj = obj_find_by_objref(ctx, h->objref);
> +       if (!obj) {
>                 obj = obj_new(ctx, ptr, h->objref, h->objtype);
> -       /*
> -        * Drop an extra reference to the object returned by ops->restore:
> -        * On success, this clears the extra reference taken by obj_new(),
> -        * and on failure, this cleans up the object itself.
> -        */
> -       ops->ref_drop(ptr, 0);
> +               /*
> +                * Drop an extra reference to the object returned by
> +                * ops->restore: On success, this clears the extra
> +                * reference taken by obj_new(), and on failure, this
> +                * cleans up the object itself.
> +                */
> +               ops->ref_drop(ptr, 0);
> +       } else if ((obj->ptr != ptr) || (obj->ops->obj_type != h->objtype)) {
> +               /* Normally, we expect an object to not already exist
> +                * in the hash.  However, for some special scenarios
> +                * where we're restoring sets of objects that must be
> +                * co-allocated (such, as veth netdev pairs) we need
> +                * to tolerate this case if the second restore returns
> +                * the correct type and pointer, as specified in the
> +                * existing object.  If either of those don't match,
> +                * we fail.
> +                */
> +               obj = ERR_PTR(-EINVAL);
> +       }
> +
>         if (IS_ERR(obj)) {
>                 ops->ref_drop(ptr, 1);
>                 return PTR_ERR(obj);
> _______________________________________________
> Containers mailing list
> Containers at lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers