[CRIU] [PATCH] Allow the veth-pair option to specify a bridge

[Saied Kazemi]

Hi Chris,

Thanks for your review; will send a new patch to address both
comments.  Re braces, CRIU's coding style does not use them when
they're not needed (but I'll add them).

--Saied



On Mon, Jan 5, 2015 at 5:48 AM, Christopher Covington
<cov at codeaurora.org> wrote:
> Hi Saied,
>
> On 12/30/2014 04:47 PM, Saied Kazemi wrote:
>> When restoring a pair of veth devices that had one end inside a namespace
>> or container and the other end outside, CRIU creates a new veth pair,
>> puts one end in the namespace/container, and names the other end from
>> what's specified in the --veth-pair IN=OUT command line option.
>>
>> This patch allows for appending a bridge name to the OUT string in the
>> form of OUT@<BRIDGE-NAME> in order for CRIU to move the outside veth to
>> the named bridge.  For example, --veth-pair eth0=veth1 at br0 tells CRIU
>> to name the peer of eth0 veth1 and move it to bridge br0.
>>
>> This is a simple and handy extension of the --veth-pair option that
>> obviates the need for an action script although one can still do the same
>> (and possibly more) if they prefer to use action scripts.
>>
>> Signed-off-by: Saied Kazemi <saied at google.com>
>> ---
>>  cr-restore.c  |  4 ++++
>>  crtools.c     |  1 +
>>  include/net.h |  2 ++
>>  net.c         | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  4 files changed, 58 insertions(+), 1 deletion(-)
>>
>> diff --git a/cr-restore.c b/cr-restore.c
>> index dc855f7..b4fd8f7 100644
>> --- a/cr-restore.c
>> +++ b/cr-restore.c
>> @@ -1777,6 +1777,10 @@ static int restore_root_task(struct pstree_item *init)
>>       if (ret < 0)
>>               goto out_kill;
>>
>> +     ret = move_veth_to_bridge();
>> +     if (ret < 0)
>> +             goto out_kill;
>> +
>>       ret = run_scripts(ACT_POST_RESTORE);
>>       if (ret != 0) {
>>               pr_err("Aborting restore due to script ret code %d\n", ret);
>> diff --git a/crtools.c b/crtools.c
>> index d23f302..4322e69 100644
>> --- a/crtools.c
>> +++ b/crtools.c
>> @@ -608,6 +608,7 @@ usage:
>>  "  --evasive-devices     use any path to a device file if the original one\n"
>>  "                        is inaccessible\n"
>>  "  --veth-pair IN=OUT    map inside veth device name to outside one\n"
>> +"                        can optionally append @<bridge-name> to move the outside veth to\n"
>
> Looks like you're missing:
>
> the named bridge.
>
>>  "  --link-remap          allow to link unlinked files back when possible\n"
>>  "  --action-script FILE  add an external action script\n"
>>  "  -j|--" OPT_SHELL_JOB "        allow to dump and restore shell jobs\n"
>> diff --git a/include/net.h b/include/net.h
>> index df9e67d..c03105c 100644
>> --- a/include/net.h
>> +++ b/include/net.h
>> @@ -12,6 +12,7 @@ struct veth_pair {
>>       struct list_head node;
>>       char *inside;
>>       char *outside;
>> +     char *bridge;
>>  };
>>
>>  extern int collect_net_namespaces(bool for_dump);
>> @@ -27,5 +28,6 @@ extern int read_ns_sys_file(char *path, char *buf, int len);
>>  extern int restore_link_parms(NetDeviceEntry *nde, int nlsk);
>>
>>  extern int veth_pair_add(char *in, char *out);
>> +extern int move_veth_to_bridge(void);
>>
>>  #endif /* __CR_NET_H__ */
>> diff --git a/net.c b/net.c
>> index d8abb36..0be171b 100644
>> --- a/net.c
>> +++ b/net.c
>> @@ -617,6 +617,7 @@ void network_unlock(void)
>>
>>  int veth_pair_add(char *in, char *out)
>>  {
>> +     char *aux;
>>       struct veth_pair *n;
>>
>>       n = xmalloc(sizeof(*n));
>> @@ -625,8 +626,22 @@ int veth_pair_add(char *in, char *out)
>>
>>       n->inside = in;
>>       n->outside = out;
>> +     /*
>> +      * Does the out string specify a bridge for
>> +      * moving the outside end of the veth pair to?
>> +      */
>> +     aux = strrchr(out, '@');
>> +     if (aux) {
>> +             *aux++ = '\0';
>> +             n->bridge = aux;
>> +     } else
>> +             n->bridge = NULL;
>
> I'm partial to using braces on the else if they've been used on the if.
>
>>       list_add(&n->node, &opts.veth_pairs);
>> -     pr_debug("Added %s:%s veth map\n", in, out);
>> +     if (n->bridge)
>> +             pr_debug("Added %s:%s@%s veth map\n", in, out, aux);
>> +     else
>> +             pr_debug("Added %s:%s veth map\n", in, out);
>>       return 0;
>>  }
>>
>
> Chris
>
> --
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project