[ovs-dev] [PATCH] xenserver: Update all external_ids in tap interfaces.

[Reid Price]

Trivial notes inline

On Wed, Jun 22, 2011 at 2:18 PM, Ethan Jackson <ethan at nicira.com> wrote:

> Commit 400430 "xenserver: Give tap devices iface-ids." copies the
> iface-id from vifs to their related tap device.  It turns out this
> is not sufficient, so this commit copies all relevant external_ids
> over.
>
> Requested-by: Pankaj Thakkar <thakkar at nicira.com>
> Signed-off-by: Ethan Jackson <ethan at nicira.com>
>
> Bug #5954.
> ---
>  .../usr_share_openvswitch_scripts_ovs-xapi-sync    |   31
> ++++++++++++++++++-
>  1 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync
> b/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync
> index 400693c..28acad8 100755
> --- a/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync
> +++ b/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync
> @@ -124,8 +124,12 @@ def call_vsctl(args):
>         s_log.warning("Couldn't call ovs-vsctl")
>
>  def set_external_id(table, record, key, value):
> -    col = 'external-ids:"' + key + '"="' + value + '"'
> -    call_vsctl(["set", table, record, col])
> +    if value:
> +        col = 'external-ids:"' + key + '"="' + value + '"'
>

This might be slightly more obvious as

  col = 'external-ids:"%s"="%s"' % (key, value)


> +        call_vsctl(["set", table, record, col])
> +    else:
> +        call_vsctl(["remove", table, record, "external-ids", key])
> +
>
>  # XenServer does not call interface-reconfigure on internal networks,
>  # which is where the fail-mode would normally be set.
> @@ -235,6 +239,27 @@ def handler(signum, frame):
>     if (signum == signal.SIGHUP):
>         force_run = True
>
> +def update_tap_from_vif(idl, tap_name, vif_name):
> +    ifaces = idl.data["Interface"]
> +    tap = None
> +    vif = None
> +
> +    for i in ifaces:

+        name = str(ifaces[i].name).strip('"')
> +        if name == tap_name:
> +            tap = ifaces[i]
> +        elif name == vif_name:
> +            vif = ifaces[i]
>

Minor note, could do ifaces.values(), and avoid the [i], since you don't use
it


> +
> +    if vif and tap:
> +        vxid = vif.external_ids
> +        txid = tap.external_ids
> +
> +        keys = ["attached-mac", "xs-network-uuid", "xs-vif-uuid",
> "xs-vm-uuid"]
> +        for k in keys:
> +            if vxid.get(k) != txid.get(k):
>
Is it possible (and would it matter) to have a condition where

  vxid = {'foo': None}
  txid = {}

In this case, txid would not get updated to have any value.  I seem to
remember
that this usually uses '[]' (never None) for empty values, which would be
fine.


> +                set_external_id("Interface", tap_name, k, vxid.get(k))
> +
>  def main(argv):
>     global force_run
>
> @@ -319,6 +344,8 @@ def main(argv):
>                 new_interfaces[vif]["iface-status"] = "inactive"
>                 new_interfaces[name]["iface-status"] = "active"
>
> +                update_tap_from_vif(idl, name, vif)
> +
>         if bridges != new_bridges:
>             for name,ids in new_bridges.items():
>                 if name not in bridges:
> --
> 1.7.5.2
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20110622/5a26b931/attachment-0003.html>