[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv4 1/3] qemu_migration: Add hooks to transport network data during migration



On 22.10.2012 23:30, Laine Stump wrote:
> From: Kyle Mestery <kmestery cisco com>
> 
> Add the ability for the Qemu V3 migration protocol to
> include transporting network configuration. A generic
> framework is proposed with this patch to allow for the
> transfer of opaque data.
> 
> Signed-off-by: Kyle Mestery <kmestery cisco com>
> ---
>  src/qemu/qemu_migration.c | 238 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 236 insertions(+), 2 deletions(-)

ACK but see my comments below.

> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 487182e..67276f0 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1,3 +1,4 @@
> +
>  /*
>   * qemu_migration.c: QEMU migration handling
>   *
> @@ -70,6 +71,7 @@ enum qemuMigrationCookieFlags {
>      QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS,
>      QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE,
>      QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT,
> +    QEMU_MIGRATION_COOKIE_FLAG_NETWORK,
>  
>      QEMU_MIGRATION_COOKIE_FLAG_LAST
>  };
> @@ -77,12 +79,13 @@ enum qemuMigrationCookieFlags {
>  VIR_ENUM_DECL(qemuMigrationCookieFlag);
>  VIR_ENUM_IMPL(qemuMigrationCookieFlag,
>                QEMU_MIGRATION_COOKIE_FLAG_LAST,
> -              "graphics", "lockstate", "persistent");
> +              "graphics", "lockstate", "persistent", "network");
>  
>  enum qemuMigrationCookieFeatures {
>      QEMU_MIGRATION_COOKIE_GRAPHICS  = (1 << QEMU_MIGRATION_COOKIE_FLAG_GRAPHICS),
>      QEMU_MIGRATION_COOKIE_LOCKSTATE = (1 << QEMU_MIGRATION_COOKIE_FLAG_LOCKSTATE),
>      QEMU_MIGRATION_COOKIE_PERSISTENT = (1 << QEMU_MIGRATION_COOKIE_FLAG_PERSISTENT),
> +    QEMU_MIGRATION_COOKIE_NETWORK = (1 << QEMU_MIGRATION_COOKIE_FLAG_NETWORK),
>  };
>  
>  typedef struct _qemuMigrationCookieGraphics qemuMigrationCookieGraphics;
> @@ -95,6 +98,27 @@ struct _qemuMigrationCookieGraphics {
>      char *tlsSubject;
>  };
>  
> +typedef struct _qemuMigrationCookieNetdata qemuMigrationCookieNetdata;
> +typedef qemuMigrationCookieNetdata *qemuMigrationCookieNetdataPtr;
> +struct _qemuMigrationCookieNetdata {
> +    int vporttype; /* enum virNetDevVPortProfile */
> +
> +    /*
> +     * Array of pointers to saved data. Each VIF will have it's own
> +     * data to transfer.
> +     */
> +    char *portdata;
> +};
> +
> +typedef struct _qemuMigrationCookieNetwork qemuMigrationCookieNetwork;
> +typedef qemuMigrationCookieNetwork *qemuMigrationCookieNetworkPtr;
> +struct _qemuMigrationCookieNetwork {
> +    /* How many virtual NICs are we saving data for? */
> +    int nnets;
> +
> +    qemuMigrationCookieNetdataPtr net;
> +};
> +
>  typedef struct _qemuMigrationCookie qemuMigrationCookie;
>  typedef qemuMigrationCookie *qemuMigrationCookiePtr;
>  struct _qemuMigrationCookie {
> @@ -120,6 +144,9 @@ struct _qemuMigrationCookie {
>  
>      /* If (flags & QEMU_MIGRATION_COOKIE_PERSISTENT) */
>      virDomainDefPtr persistent;
> +
> +    /* If (flags & QEMU_MIGRATION_COOKIE_NETWORK) */
> +    qemuMigrationCookieNetworkPtr network;
>  };
>  
>  static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap)
> @@ -132,6 +159,23 @@ static void qemuMigrationCookieGraphicsFree(qemuMigrationCookieGraphicsPtr grap)
>  }
>  
>  
> +static void
> +qemuMigrationCookieNetworkFree(qemuMigrationCookieNetworkPtr network)
> +{
> +    int i;
> +
> +    if (!network)
> +        return;
> +
> +    if (network->net) {
> +        for (i = 0; i < network->nnets; i++)
> +            VIR_FREE(network->net[i].portdata);
> +    }

You could have spared one level of nesting if you'd just only ... [1]

> +    VIR_FREE(network->net);
> +    VIR_FREE(network);
> +}
> +
> +
>  static void qemuMigrationCookieFree(qemuMigrationCookiePtr mig)
>  {
>      if (!mig)
> @@ -140,6 +184,10 @@ static void qemuMigrationCookieFree(qemuMigrationCookiePtr mig)
>      if (mig->flags & QEMU_MIGRATION_COOKIE_GRAPHICS)
>          qemuMigrationCookieGraphicsFree(mig->graphics);
>  
> +    if (mig->flags & QEMU_MIGRATION_COOKIE_NETWORK) {
> +        qemuMigrationCookieNetworkFree(mig->network);
> +    }

These curly braces aren't required. But it is not against coding style
neither.

> +
>      VIR_FREE(mig->localHostname);
>      VIR_FREE(mig->remoteHostname);
>      VIR_FREE(mig->name);
> @@ -256,6 +304,49 @@ error:
>  }
>  
>  
> +static qemuMigrationCookieNetworkPtr
> +qemuMigrationCookieNetworkAlloc(struct qemud_driver *driver ATTRIBUTE_UNUSED,
> +                                virDomainDefPtr def)
> +{
> +    qemuMigrationCookieNetworkPtr mig;
> +    int i;
> +
> +    if (VIR_ALLOC(mig) < 0)
> +        goto no_memory;
> +
> +    mig->nnets = def->nnets;

[1]: ... set this after the VIR_ALLOC_N below.

> +
> +    if (VIR_ALLOC_N(mig->net, def->nnets) <0)
> +        goto no_memory;
> +
> +    for (i = 0; i < def->nnets; i++) {
> +        virDomainNetDefPtr netptr;
> +        virNetDevVPortProfilePtr vport;
> +
> +        netptr = def->nets[i];
> +        vport = virDomainNetGetActualVirtPortProfile(netptr);
> +
> +        if (vport) {
> +            mig->net[i].vporttype = vport->virtPortType;
> +
> +            switch (vport->virtPortType) {
> +            case VIR_NETDEV_VPORT_PROFILE_NONE:
> +            case VIR_NETDEV_VPORT_PROFILE_8021QBG:
> +            case VIR_NETDEV_VPORT_PROFILE_8021QBH:
> +            case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
> +            default:
> +                break;
> +            }
> +        }
> +    }
> +    return mig;
> +
> +no_memory:
> +    virReportOOMError();
> +    qemuMigrationCookieNetworkFree(mig);
> +    return NULL;
> +}
> +
>  static qemuMigrationCookiePtr
>  qemuMigrationCookieNew(virDomainObjPtr dom)
>  {
> @@ -370,6 +461,27 @@ qemuMigrationCookieAddPersistent(qemuMigrationCookiePtr mig,
>  }
>  
>  
> +static int
> +qemuMigrationCookieAddNetwork(qemuMigrationCookiePtr mig,
> +                              struct qemud_driver *driver,
> +                              virDomainObjPtr dom)
> +{
> +    if (mig->flags & QEMU_MIGRATION_COOKIE_NETWORK) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Network migration data already present"));
> +        return -1;
> +    }
> +
> +    if (dom->def->nnets > 0) {
> +        mig->network = qemuMigrationCookieNetworkAlloc(driver, dom->def);
> +        if (!mig->network)
> +            return -1;
> +        mig->flags |= QEMU_MIGRATION_COOKIE_NETWORK;
> +    }
> +
> +    return 0;
> +}
> +
>  
>  static void qemuMigrationCookieGraphicsXMLFormat(virBufferPtr buf,
>                                                   qemuMigrationCookieGraphicsPtr grap)
> @@ -389,6 +501,32 @@ static void qemuMigrationCookieGraphicsXMLFormat(virBufferPtr buf,
>  }
>  
>  
> +static void
> +qemuMigrationCookieNetworkXMLFormat(virBufferPtr buf,
> +                                    qemuMigrationCookieNetworkPtr optr)
> +{
> +    int i;
> +
> +    virBufferAsprintf(buf, "  <network>\n");
> +    for (i = 0; i < optr->nnets; i++) {
> +        /* If optr->net.vporttype[i] is not set, there is nothing to transfer */

I believe '[i]' wants to be moved to 'net' to match the condition below.

> +        if (optr->net[i].vporttype != VIR_NETDEV_VPORT_PROFILE_NONE) {
> +            virBufferAsprintf(buf, "    <interface index='%d' vporttype='%s'",
> +                              i, virNetDevVPortTypeToString(optr->net[i].vporttype));
> +            if (optr->net[i].portdata) {
> +                virBufferAddLit(buf, ">\n");
> +                virBufferEscapeString(buf, "      <portdata>%s</portdata>\n",
> +                                      optr->net[i].portdata);
> +                virBufferAddLit(buf, "    </interface>\n");
> +            } else {
> +                virBufferAddLit(buf, "/>\n");
> +            }
> +        }
> +    }
> +    virBufferAddLit(buf, "  </network>\n");
> +}
> +
> +

Maybe if all 'net[i].vporttype' are VIR_NETDEV_VPORT_PROFILE_NONE so
there is no <interface/> added, we don't need to add empty <network/>
neither. But that just cosmetics and I can live with this version as-is.

>  static int
>  qemuMigrationCookieXMLFormat(struct qemud_driver *driver,
>                               virBufferPtr buf,
> @@ -439,6 +577,9 @@ qemuMigrationCookieXMLFormat(struct qemud_driver *driver,
>          virBufferAdjustIndent(buf, -2);
>      }
>  
> +    if ((mig->flags & QEMU_MIGRATION_COOKIE_NETWORK) && mig->network)
> +        qemuMigrationCookieNetworkXMLFormat(buf, mig->network);
> +
>      virBufferAddLit(buf, "</qemu-migration>\n");
>      return 0;
>  }
> @@ -516,6 +657,58 @@ error:
>  }
>  
>  
> +static qemuMigrationCookieNetworkPtr
> +qemuMigrationCookieNetworkXMLParse(xmlXPathContextPtr ctxt)
> +{
> +    qemuMigrationCookieNetworkPtr optr;
> +    int i;
> +    int n;
> +    xmlNodePtr *interfaces = NULL;
> +    char *vporttype;
> +    xmlNodePtr save_ctxt = ctxt->node;
> +
> +    if (VIR_ALLOC(optr) < 0)
> +        goto no_memory;
> +
> +    if ((n = virXPathNodeSet("./network/interface", ctxt, &interfaces)) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s", _("missing interface information"));
> +        goto error;
> +    }
> +
> +    optr->nnets = n;
> +    if (VIR_ALLOC_N(optr->net, optr->nnets) <0)
> +        goto no_memory;
> +
> +    for (i = 0; i < n; i++) {
> +        /* portdata is optional, and may not exist */
> +        ctxt->node = interfaces[i];
> +        optr->net[i].portdata = virXPathString("string(./portdata[1])", ctxt);
> +
> +        if (!(vporttype = virXMLPropString(interfaces[i], "vporttype"))) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           "%s", _("missing vporttype attribute in migration data"));
> +            goto error;
> +        }
> +        optr->net[i].vporttype = virNetDevVPortTypeFromString(vporttype);
> +    }
> +
> +    VIR_FREE(interfaces);
> +
> +cleanup:
> +    ctxt->node = save_ctxt;
> +    return optr;
> +
> +no_memory:
> +    virReportOOMError();
> +error:
> +    VIR_FREE(interfaces);
> +    qemuMigrationCookieNetworkFree(optr);
> +    optr = NULL;
> +    goto cleanup;
> +}
> +
> +
>  static int
>  qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
>                              struct qemud_driver *driver,
> @@ -663,6 +856,11 @@ qemuMigrationCookieXMLParse(qemuMigrationCookiePtr mig,
>          VIR_FREE(nodes);
>      }
>  
> +    if ((flags & QEMU_MIGRATION_COOKIE_NETWORK) &&
> +        virXPathBoolean("count(./network) > 0", ctxt) &&
> +        (!(mig->network = qemuMigrationCookieNetworkXMLParse(ctxt))))
> +        goto error;
> +
>      return 0;
>  
>  error:
> @@ -722,6 +920,10 @@ qemuMigrationBakeCookie(qemuMigrationCookiePtr mig,
>          qemuMigrationCookieAddPersistent(mig, dom) < 0)
>          return -1;
>  
> +    if (flags & QEMU_MIGRATION_COOKIE_NETWORK &&
> +        qemuMigrationCookieAddNetwork(mig, driver, dom) < 0)
> +        return -1;
> +
>      if (!(*cookieout = qemuMigrationCookieXMLFormatStr(driver, mig)))
>          return -1;
>  
> @@ -1084,6 +1286,32 @@ qemuDomainMigrateGraphicsRelocate(struct qemud_driver *driver,
>  }
>  
>  
> +static int
> +qemuDomainMigrateOPDRelocate(struct qemud_driver *driver ATTRIBUTE_UNUSED,
> +                             virDomainObjPtr vm,
> +                             qemuMigrationCookiePtr cookie)
> +{
> +    virDomainNetDefPtr netptr ATTRIBUTE_UNUSED;
> +    int ret = 0;
> +    int i;
> +
> +    for (i = 0; i < cookie->network->nnets; i++) {
> +        netptr = vm->def->nets[i];
> +
> +        switch (cookie->network->net[i].vporttype) {
> +        case VIR_NETDEV_VPORT_PROFILE_NONE:
> +        case VIR_NETDEV_VPORT_PROFILE_8021QBG:
> +        case VIR_NETDEV_VPORT_PROFILE_8021QBH:
> +        case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
> +        default:
> +            break;
> +        }
> +    }
> +
> +    return ret;
> +}
> +
> +
>  /* This is called for outgoing non-p2p migrations when a connection to the
>   * client which initiated the migration was closed but we were waiting for it
>   * to follow up with the next phase, that is, in between
> @@ -2029,7 +2257,8 @@ cleanup:
>  
>      if (ret == 0 &&
>          qemuMigrationBakeCookie(mig, driver, vm, cookieout, cookieoutlen,
> -                                QEMU_MIGRATION_COOKIE_PERSISTENT ) < 0)
> +                                QEMU_MIGRATION_COOKIE_PERSISTENT |
> +                                QEMU_MIGRATION_COOKIE_NETWORK) < 0)
>          VIR_WARN("Unable to encode migration cookie");
>  
>      qemuMigrationCookieFree(mig);
> @@ -2963,6 +3192,7 @@ qemuMigrationFinish(struct qemud_driver *driver,
>  
>      qemuDomainCleanupRemove(vm, qemuMigrationPrepareCleanup);
>  
> +    cookie_flags = QEMU_MIGRATION_COOKIE_NETWORK;
>      if (flags & VIR_MIGRATE_PERSIST_DEST)
>          cookie_flags |= QEMU_MIGRATION_COOKIE_PERSISTENT;
>  
> @@ -2990,6 +3220,10 @@ qemuMigrationFinish(struct qemud_driver *driver,
>              goto endjob;
>          }
>  
> +        if (mig->network)
> +            if (qemuDomainMigrateOPDRelocate(driver, vm, mig) < 0)
> +                VIR_WARN("unable to provide network data for relocation");
> +
>          if (flags & VIR_MIGRATE_PERSIST_DEST) {
>              virDomainDefPtr vmdef;
>              if (vm->persistent)
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]