[libvirt] [PATCH 3/3] qemu_migration: Transport Open vSwitch per-port data during live migration

[Laine Stump]

On 09/21/2012 05:16 PM, Kyle Mestery wrote:
> Transport Open vSwitch per-port data during live
> migration by using the utility functions
> virNetDevOpenvswitchGetMigrateData() and
> virNetDevOpenvswitchSetMigrateData().

I like how the first part is all re-usable infrastructure, and the final
patch that actually makes things work is so small! That implies that it
will be easy to add other new functionality in the future...

>
> Signed-off-by: Kyle Mestery <kmestery at cisco.com>
> ---
>  src/qemu/qemu_migration.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 06981db..33cdb20 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -322,7 +322,7 @@ qemuMigrationCookieNetworkAlloc(struct qemud_driver *driver ATTRIBUTE_UNUSED,
>  {
>      qemuMigrationCookieNetworkPtr mig;
>      int i;
> -    virDomainNetDefPtr netptr ATTRIBUTE_UNUSED;
> +    virDomainNetDefPtr netptr;
>      const char *interfacetype;
>  
>      if (VIR_ALLOC(mig) < 0)
> @@ -357,7 +357,13 @@ qemuMigrationCookieNetworkAlloc(struct qemud_driver *driver ATTRIBUTE_UNUSED,
>                  mig->net[i]->portdata = NULL;
>                  break;
>              case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
> -                mig->net[i]->portdata = NULL;
> +                if (virNetDevOpenvswitchGetMigrateData(&mig->net[i]->portdata,
> +                                                       netptr->ifname) != 0) {
> +                        virReportSystemError(VIR_ERR_INTERNAL_ERROR,
> +                                             _("Unable to run command to get OVS port data for "
> +                                             "interface %s"), netptr->ifname);
> +                        goto error;
> +                }
>                  break;
>              default:
>                  mig->net[i]->portdata = NULL;
> @@ -366,6 +372,7 @@ qemuMigrationCookieNetworkAlloc(struct qemud_driver *driver ATTRIBUTE_UNUSED,
>          }
>      }
>  
> +error:
>      return mig;
>  
>  no_memory:
> @@ -1284,7 +1291,7 @@ qemuDomainMigrateOPDRelocate(struct qemud_driver *driver ATTRIBUTE_UNUSED,
>                               virDomainObjPtr vm,
>                               qemuMigrationCookiePtr cookie)
>  {
> -    virDomainNetDefPtr netptr ATTRIBUTE_UNUSED;
> +    virDomainNetDefPtr netptr;
>      int ret = 0;

These days we tend to 1) initialize "ret = -1" (assume failure), call
the early exit label "cleanup" rather than "error" (if a successful exit
will run through that label - it avoids confusion when someone reads the
code later), and put a "ret = 0;" on the line just before "cleanup:".
That way, you don't need to put in all the "ret = -1;" lines whenever
there is a failure.

ACK with that change (and after re-basing to account for the changes
requested in 1/3).

>      int i;
>      int interfacetype;
> @@ -1305,7 +1312,14 @@ qemuDomainMigrateOPDRelocate(struct qemud_driver *driver ATTRIBUTE_UNUSED,
>                  cookie->network->net[i]->portdata = NULL;
>                  break;
>              case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
> -                cookie->network->net[i]->portdata = NULL;
> +                if (virNetDevOpenvswitchSetMigrateData(cookie->network->net[i]->portdata,
> +                                                       netptr->ifname) != 0) {
> +                        virReportSystemError(VIR_ERR_INTERNAL_ERROR,
> +                                             _("Unable to run command to set OVS port data for "
> +                                             "interface %s"), netptr->ifname);
> +                        ret = -1;
> +                        goto error;
> +                }
>                  break;
>              default:
>                  cookie->network->net[i]->portdata = NULL;
> @@ -1314,6 +1328,7 @@ qemuDomainMigrateOPDRelocate(struct qemud_driver *driver ATTRIBUTE_UNUSED,
>          }
>      }
>  
> +error:
>      return ret;
>  }
>