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

Re: [libvirt] [PATCH v2] 802.1Qbg: use pre-associate state at beginning of incoming migration



Hi Stefan,

Sorry about the delay. I verified your patch for 802.1Qbh. Its all fine. No
side-effects.

Thanks,
Roopa



On 11/24/10 5:39 AM, "Stefan Berger" <stefanb linux vnet ibm com> wrote:

> -v2:
>      - changing the vmOpStr array and vmOperation enum to use
> VIR_ENUM_IMPL and VIR_ENUM_DECL. Renaming the enum to virVMOperationType
> and adapting the name of the enum constants.
> 
> This patch introduces the usage of the pre-associate state of the IEEE
> 802.1Qbg standard on incoming VM migration on the target host. It is in
> response to bugzilla entry 632750.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=632750
> 
> For being able to differentiate the exact reason as to why a macvtap
> device is being created, either due to a VM creation or an incoming VM
> migration, I needed to pass that reason as a parameter from wherever
> qemudStartVMDaemon is being called in order to determine whether to send
> an ASSOCIATE (VM creation) or a PRE-ASSOCIATE (incoming VM migration)
> towards lldpad.
> 
> I am also fixing a problem with the virsh domainxml-to-native call on
> the way.
> 
> Gerhard successfully tested the patch with a recent blade network
> 802.1Qbg-compliant switch.
> 
> The patch should not have any side-effects on the 802.1Qbh support in
> libvirt, but Roopa (cc'ed) may want to verify this.
> 
> Signed-off-by: Stefan Berger <stefanb us ibm com>
> Signed-off-by: Gerhard Stenzel <gerhard stenzel de ibm com>
> 
> ---
>   src/libvirt_macvtap.syms |    2 +
>   src/qemu/qemu_conf.c     |   12 +++++--
>   src/qemu/qemu_conf.h     |    7 +++-
>   src/qemu/qemu_driver.c   |   72
> +++++++++++++++++++++++++++++++++++++----------
>   src/util/macvtap.c       |   54 ++++++++++++++++++++++++++++++-----
>   src/util/macvtap.h       |   20 +++++++++++--
>   6 files changed, 137 insertions(+), 30 deletions(-)
> 
> Index: libvirt-acl/src/qemu/qemu_conf.c
> ===================================================================
> --- libvirt-acl.orig/src/qemu/qemu_conf.c
> +++ libvirt-acl/src/qemu/qemu_conf.c
> @@ -1642,7 +1642,8 @@ qemudPhysIfaceConnect(virConnectPtr conn
>                         struct qemud_driver *driver,
>                         virDomainNetDefPtr net,
>                         unsigned long long qemuCmdFlags,
> -                      const unsigned char *vmuuid)
> +                      const unsigned char *vmuuid,
> +                      enum virVMOperationType vmop)
>   {
>       int rc;
>   #if WITH_MACVTAP
> @@ -1656,7 +1657,8 @@ qemudPhysIfaceConnect(virConnectPtr conn
> 
>       rc = openMacvtapTap(net->ifname, net->mac, net->data.direct.linkdev,
>                           net->data.direct.mode, vnet_hdr, vmuuid,
> - &net->data.direct.virtPortProfile, &res_ifname);
> + &net->data.direct.virtPortProfile, &res_ifname,
> +                        vmop);
>       if (rc >= 0) {
>           VIR_FREE(net->ifname);
>           net->ifname = res_ifname;
> @@ -3953,7 +3955,8 @@ int qemudBuildCommandLine(virConnectPtr
>                             int **vmfds,
>                             int *nvmfds,
>                             const char *migrateFrom,
> -                          virDomainSnapshotObjPtr current_snapshot)
> +                          virDomainSnapshotObjPtr current_snapshot,
> +                          enum virVMOperationType vmop)
>   {
>       int i;
>       char memory[50];
> @@ -4796,7 +4799,8 @@ int qemudBuildCommandLine(virConnectPtr
>               } else if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
>                   int tapfd = qemudPhysIfaceConnect(conn, driver, net,
>                                                     qemuCmdFlags,
> -                                                  def->uuid);
> +                                                  def->uuid,
> +                                                  vmop);
>                   if (tapfd < 0)
>                       goto error;
> 
> Index: libvirt-acl/src/qemu/qemu_conf.h
> ===================================================================
> --- libvirt-acl.orig/src/qemu/qemu_conf.h
> +++ libvirt-acl/src/qemu/qemu_conf.h
> @@ -40,6 +40,7 @@
>   # include "cpu_conf.h"
>   # include "driver.h"
>   # include "bitmap.h"
> +# include "macvtap.h"
> 
>   # define qemudDebug(fmt, ...) do {} while(0)
> 
> @@ -238,7 +239,8 @@ int         qemudBuildCommandLine
>                                            int **vmfds,
>                                            int *nvmfds,
>                                            const char *migrateFrom,
> -                                         virDomainSnapshotObjPtr
> current_snapshot)
> +                                         virDomainSnapshotObjPtr
> current_snapshot,
> +                                         enum virVMOperationType vmop)
>       ATTRIBUTE_NONNULL(1);
> 
>   /* With vlan == -1, use netdev syntax, else old hostnet */
> @@ -317,7 +319,8 @@ int qemudPhysIfaceConnect(virConnectPtr
>                             struct qemud_driver *driver,
>                             virDomainNetDefPtr net,
>                             unsigned long long qemuCmdFlags,
> -                          const unsigned char *vmuuid);
> +                          const unsigned char *vmuuid,
> +                          enum virVMOperationType vmop);
> 
>   int         qemudProbeMachineTypes      (const char *binary,
>                                            virCapsGuestMachinePtr
> **machines,
> Index: libvirt-acl/src/qemu/qemu_driver.c
> ===================================================================
> --- libvirt-acl.orig/src/qemu/qemu_driver.c
> +++ libvirt-acl/src/qemu/qemu_driver.c
> @@ -163,7 +163,8 @@ static int qemudStartVMDaemon(virConnect
>                                 const char *migrateFrom,
>                                 bool start_paused,
>                                 int stdin_fd,
> -                              const char *stdin_path);
> +                              const char *stdin_path,
> +                              enum virVMOperationType vmop);
> 
>   static void qemudShutdownVMDaemon(struct qemud_driver *driver,
>                                     virDomainObjPtr vm,
> @@ -3864,7 +3865,8 @@ static int qemudStartVMDaemon(virConnect
>                                 const char *migrateFrom,
>                                 bool start_paused,
>                                 int stdin_fd,
> -                              const char *stdin_path) {
> +                              const char *stdin_path,
> +                              enum virVMOperationType vmop) {
>       const char **argv = NULL, **tmp;
>       const char **progenv = NULL;
>       int i, ret, runflags;
> @@ -4065,7 +4067,7 @@ static int qemudStartVMDaemon(virConnect
>       if (qemudBuildCommandLine(conn, driver, vm->def, priv->monConfig,
>                                 priv->monJSON, qemuCmdFlags, &argv,
> &progenv,
> &vmfds, &nvmfds, migrateFrom,
> -                              vm->current_snapshot) < 0)
> +                              vm->current_snapshot, vmop) < 0)
>           goto cleanup;
> 
>       if (qemuDomainSnapshotSetInactive(vm, driver->snapshotDir) < 0)
> @@ -4879,7 +4881,7 @@ static virDomainPtr qemudDomainCreate(vi
> 
>       if (qemudStartVMDaemon(conn, driver, vm, NULL,
>                              (flags & VIR_DOMAIN_START_PAUSED) != 0,
> -                           -1, NULL) < 0) {
> +                           -1, NULL, VIR_VM_OP_CREATE) < 0) {
>           qemuDomainStartAudit(vm, "booted", false);
>           if (qemuDomainObjEndJob(vm) > 0)
>               virDomainRemoveInactive(&driver->domains,
> @@ -7015,7 +7017,8 @@ qemudDomainSaveImageStartVM(virConnectPt
>       }
> 
>       /* Set the migration source and start it up. */
> -    ret = qemudStartVMDaemon(conn, driver, vm, "stdio", true, fd, path);
> +    ret = qemudStartVMDaemon(conn, driver, vm, "stdio", true, fd, path,
> +                             VIR_VM_OP_RESTORE);
> 
>       if (intermediate_pid != -1) {
>           /* Wait for intermediate process to exit */
> @@ -7334,14 +7337,15 @@ static char *qemuDomainXMLToNative(virCo
>       if (!def)
>           goto cleanup;
> 
> -    /* Since we're just exporting args, we can't do bridge/network
> -     * setups, since libvirt will normally create TAP devices
> +    /* Since we're just exporting args, we can't do bridge/network/direct
> +     * setups, since libvirt will normally create TAP/macvtap devices
>        * directly. We convert those configs into generic 'ethernet'
>        * config and assume the user has suitable 'ifup-qemu' scripts
>        */
>       for (i = 0 ; i < def->nnets ; i++) {
>           virDomainNetDefPtr net = def->nets[i];
> -        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
> +        if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK ||
> +            net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
>               VIR_FREE(net->data.network.name);
> 
>               memset(net, 0, sizeof *net);
> @@ -7397,7 +7401,8 @@ static char *qemuDomainXMLToNative(virCo
> &monConfig, 0, qemuCmdFlags,
> &retargv, &retenv,
>                                 NULL, NULL, /* Don't want it to create
> TAP devices */
> -                              NULL, NULL) < 0) {
> +                              NULL, NULL,
> +                              VIR_VM_OP_NO_OP) < 0) {
>           goto cleanup;
>       }
> 
> @@ -7484,7 +7489,8 @@ static int qemudDomainObjStart(virConnec
>               goto cleanup;
>       }
> 
> -    ret = qemudStartVMDaemon(conn, driver, vm, NULL, start_paused, -1,
> NULL);
> +    ret = qemudStartVMDaemon(conn, driver, vm, NULL, start_paused, -1,
> NULL,
> +                             VIR_VM_OP_CREATE);
>       qemuDomainStartAudit(vm, "booted", ret >= 0);
>       if (ret >= 0) {
>           virDomainEventPtr event =
> @@ -8327,7 +8333,8 @@ static int qemudDomainAttachNetDevice(vi
> 
>           if ((tapfd = qemudPhysIfaceConnect(conn, driver, net,
>                                              qemuCmdFlags,
> -                                           vm->def->uuid)) < 0)
> +                                           vm->def->uuid,
> +                                           VIR_VM_OP_CREATE)) < 0)
>               return -1;
>       }
> 
> @@ -11001,7 +11008,7 @@ qemudDomainMigratePrepareTunnel(virConne
>        * -incoming unix:/path/to/file or exec:nc -U /path/to/file
>        */
>       internalret = qemudStartVMDaemon(dconn, driver, vm, migrateFrom, true,
> -                                     -1, NULL);
> +                                     -1, NULL, VIR_VM_OP_MIGRATE_IN_START);
>       VIR_FREE(migrateFrom);
>       if (internalret < 0) {
>           qemuDomainStartAudit(vm, "migrated", false);
> @@ -11247,7 +11254,7 @@ qemudDomainMigratePrepare2 (virConnectPt
>        */
>       snprintf (migrateFrom, sizeof (migrateFrom), "tcp:0.0.0.0:%d",
> this_port);
>       if (qemudStartVMDaemon (dconn, driver, vm, migrateFrom, true,
> -                            -1, NULL) < 0) {
> +                            -1, NULL, VIR_VM_OP_MIGRATE_IN_START) < 0) {
>           qemuDomainStartAudit(vm, "migrated", false);
>           /* Note that we don't set an error here because qemudStartVMDaemon
>            * should have already done that.
> @@ -11862,6 +11869,41 @@ cleanup:
>       return ret;
>   }
> 
> +static void
> +qemudVPAssociatePortProfiles(virDomainDefPtr def) {
> +    int i;
> +    int last_good_net = -1;
> +    virDomainNetDefPtr net;
> +
> +    for (i = 0; i < def->nnets; i++) {
> +        net = def->nets[i];
> +        if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
> +            if (vpAssociatePortProfileId(net->ifname,
> +                                         net->mac,
> +                                         net->data.direct.linkdev,
> + &net->data.direct.virtPortProfile,
> +                                         def->uuid,
> +                                         VIR_VM_OP_MIGRATE_IN_FINISH) != 0)
> +                goto err_exit;
> +        }
> +        last_good_net = i;
> +    }
> +
> +    return;
> +
> +err_exit:
> +    for (i = 0; i < last_good_net; i++) {
> +        net = def->nets[i];
> +        if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
> +            vpDisassociatePortProfileId(net->ifname,
> +                                        net->mac,
> +                                        net->data.direct.linkdev,
> + &net->data.direct.virtPortProfile,
> +                                        VIR_VM_OP_MIGRATE_IN_FINISH);
> +        }
> +    }
> +}
> +
>   /* Finish is the third and final step, and it runs on the destination
> host. */
>   static virDomainPtr
>   qemudDomainMigrateFinish2 (virConnectPtr dconn,
> @@ -11922,6 +11964,8 @@ qemudDomainMigrateFinish2 (virConnectPtr
>               goto cleanup;
>           }
> 
> +        qemudVPAssociatePortProfiles(vm->def);
> +
>           if (flags & VIR_MIGRATE_PERSIST_DEST) {
>               if (vm->persistent)
>                   newVM = 0;
> @@ -12814,7 +12858,7 @@ static int qemuDomainRevertToSnapshot(vi
>                   goto endjob;
> 
>               rc = qemudStartVMDaemon(snapshot->domain->conn, driver,
> vm, NULL,
> -                                    false, -1, NULL);
> +                                    false, -1, NULL, VIR_VM_OP_CREATE);
>               qemuDomainStartAudit(vm, "from-snapshot", rc >= 0);
>               if (qemuDomainSnapshotSetInactive(vm, driver->snapshotDir)
> < 0)
>                   goto endjob;
> Index: libvirt-acl/src/util/macvtap.c
> ===================================================================
> --- libvirt-acl.orig/src/util/macvtap.c
> +++ libvirt-acl/src/util/macvtap.c
> @@ -77,9 +77,21 @@
>   # define LLDPAD_PID_FILE  "/var/run/lldpad.pid"
> 
> 
> +VIR_ENUM_IMPL(virVMOperation, VIR_VM_OP_LAST,
> +    "create",
> +    "save",
> +    "restore",
> +    "destroy",
> +    "migrate out",
> +    "migrate in start",
> +    "migrate in finish",
> +    "no-op")
> +
> +
>   enum virVirtualPortOp {
>       ASSOCIATE = 0x1,
>       DISASSOCIATE = 0x2,
> +    PREASSOCIATE = 0x3,
>   };
> 
> 
> @@ -551,7 +563,8 @@ openMacvtapTap(const char *tgifname,
>                  int vnet_hdr,
>                  const unsigned char *vmuuid,
>                  virVirtualPortProfileParamsPtr virtPortProfile,
> -               char **res_ifname)
> +               char **res_ifname,
> +               enum virVMOperationType vmOp)
>   {
>       const char *type = "macvtap";
>       int c, rc;
> @@ -563,6 +576,8 @@ openMacvtapTap(const char *tgifname,
> 
>       *res_ifname = NULL;
> 
> +    VIR_DEBUG("%s: VM OPERATION: %s", __FUNCTION__,
> virVMOperationTypeToString(vmOp));
> +
>       if (tgifname) {
>           if(ifaceGetIndex(false, tgifname, &ifindex) == 0) {
>               if (STRPREFIX(tgifname,
> @@ -601,7 +616,7 @@ create_name:
>                                    macaddress,
>                                    linkdev,
>                                    virtPortProfile,
> -                                 vmuuid) != 0) {
> +                                 vmuuid, vmOp) != 0) {
>           rc = -1;
>           goto link_del_exit;
>       }
> @@ -634,7 +649,8 @@ disassociate_exit:
>       vpDisassociatePortProfileId(cr_ifname,
>                                   macaddress,
>                                   linkdev,
> -                                virtPortProfile);
> +                                virtPortProfile,
> +                                vmOp);
> 
>   link_del_exit:
>       link_del(cr_ifname);
> @@ -662,7 +678,8 @@ delMacvtap(const char *ifname,
>       if (ifname) {
>           vpDisassociatePortProfileId(ifname, macaddr,
>                                       linkdev,
> -                                    virtPortProfile);
> +                                    virtPortProfile,
> +                                    VIR_VM_OP_DESTROY);
>           link_del(ifname);
>       }
>   }
> @@ -1320,6 +1337,9 @@ doPortProfileOp8021Qbg(const char *ifnam
>       portVsi.vsi_type_id[0] = virtPort->u.virtPort8021Qbg.typeID;
> 
>       switch (virtPortOp) {
> +    case PREASSOCIATE:
> +        op = PORT_REQUEST_PREASSOCIATE;
> +        break;
>       case ASSOCIATE:
>           op = PORT_REQUEST_ASSOCIATE;
>           break;
> @@ -1484,6 +1504,7 @@ err_exit:
>    * @macvtap_ifname: The name of the macvtap device
>    * @virtPort: pointer to the object holding port profile parameters
>    * @vmuuid : the UUID of the virtual machine
> + * @vmOp : The VM operation (i.e., create, no-op)
>    *
>    * Associate a port on a swtich with a profile. This function
>    * may notify a kernel driver or an external daemon to run
> @@ -1499,13 +1520,19 @@ vpAssociatePortProfileId(const char *mac
>                            const unsigned char *macvtap_macaddr,
>                            const char *linkdev,
>                            const virVirtualPortProfileParamsPtr virtPort,
> -                         const unsigned char *vmuuid)
> +                         const unsigned char *vmuuid,
> +                         enum virVMOperationType vmOp)
>   {
>       int rc = 0;
> 
>       VIR_DEBUG("Associating port profile '%p' on link device '%s'",
>                 virtPort, macvtap_ifname);
> 
> +    VIR_DEBUG("%s: VM OPERATION: %s", __FUNCTION__,
> virVMOperationTypeToString(vmOp));
> +
> +    if (vmOp == VIR_VM_OP_NO_OP)
> +        return 0;
> +
>       switch (virtPort->virtPortType) {
>       case VIR_VIRTUALPORT_NONE:
>       case VIR_VIRTUALPORT_TYPE_LAST:
> @@ -1513,10 +1540,16 @@ vpAssociatePortProfileId(const char *mac
> 
>       case VIR_VIRTUALPORT_8021QBG:
>           rc = doPortProfileOp8021Qbg(macvtap_ifname, macvtap_macaddr,
> -                                    virtPort, ASSOCIATE);
> +                                    virtPort,
> +                                    (vmOp == VIR_VM_OP_MIGRATE_IN_START)
> +                                      ? PREASSOCIATE
> +                                      : ASSOCIATE);
>           break;
> 
>       case VIR_VIRTUALPORT_8021QBH:
> +        /* avoid associating twice */
> +        if (vmOp == VIR_VM_OP_MIGRATE_IN_FINISH)
> +            break;
>           rc = doPortProfileOp8021Qbh(linkdev, virtPort,
>                                       vmuuid,
>                                       ASSOCIATE);
> @@ -1542,13 +1575,16 @@ int
>   vpDisassociatePortProfileId(const char *macvtap_ifname,
>                               const unsigned char *macvtap_macaddr,
>                               const char *linkdev,
> -                            const virVirtualPortProfileParamsPtr virtPort)
> +                            const virVirtualPortProfileParamsPtr virtPort,
> +                            enum virVMOperationType vmOp)
>   {
>       int rc = 0;
> 
>       VIR_DEBUG("Disassociating port profile id '%p' on link device '%s' ",
>                 virtPort, macvtap_ifname);
> 
> +    VIR_DEBUG("%s: VM OPERATION: %s", __FUNCTION__,
> virVMOperationTypeToString(vmOp));
> +
>       switch (virtPort->virtPortType) {
>       case VIR_VIRTUALPORT_NONE:
>       case VIR_VIRTUALPORT_TYPE_LAST:
> @@ -1560,6 +1596,9 @@ vpDisassociatePortProfileId(const char *
>           break;
> 
>       case VIR_VIRTUALPORT_8021QBH:
> +        /* avoid disassociating twice */
> +        if (vmOp == VIR_VM_OP_MIGRATE_IN_FINISH)
> +            break;
>           rc = doPortProfileOp8021Qbh(linkdev, virtPort,
>                                       NULL,
>                                       DISASSOCIATE);
> Index: libvirt-acl/src/util/macvtap.h
> ===================================================================
> --- libvirt-acl.orig/src/util/macvtap.h
> +++ libvirt-acl/src/util/macvtap.h
> @@ -62,6 +62,19 @@ struct _virVirtualPortProfileParams {
> 
>   #  include "internal.h"
> 
> +enum virVMOperationType {
> +    VIR_VM_OP_CREATE,
> +    VIR_VM_OP_SAVE,
> +    VIR_VM_OP_RESTORE,
> +    VIR_VM_OP_DESTROY,
> +    VIR_VM_OP_MIGRATE_OUT,
> +    VIR_VM_OP_MIGRATE_IN_START,
> +    VIR_VM_OP_MIGRATE_IN_FINISH,
> +    VIR_VM_OP_NO_OP,
> +
> +    VIR_VM_OP_LAST
> +};
> +
>   int openMacvtapTap(const char *ifname,
>                      const unsigned char *macaddress,
>                      const char *linkdev,
> @@ -69,7 +82,8 @@ int openMacvtapTap(const char *ifname,
>                      int vnet_hdr,
>                      const unsigned char *vmuuid,
>                      virVirtualPortProfileParamsPtr virtPortProfile,
> -                   char **res_ifname);
> +                   char **res_ifname,
> +                   enum virVMOperationType vmop);
> 
>   void delMacvtap(const char *ifname,
>                   const unsigned char *macaddress,
> @@ -86,13 +100,16 @@ int vpAssociatePortProfileId(const char
>                                const unsigned char *macvtap_macaddr,
>                                const char *linkdev,
>                                const virVirtualPortProfileParamsPtr
> virtPort,
> -                             const unsigned char *vmuuid);
> +                             const unsigned char *vmuuid,
> +                             enum virVMOperationType vmOp);
> 
>   int vpDisassociatePortProfileId(const char *macvtap_ifname,
>                                   const unsigned char *macvtap_macaddr,
>                                   const char *linkdev,
> -                                const virVirtualPortProfileParamsPtr
> virtPort);
> +                                const virVirtualPortProfileParamsPtr
> virtPort,
> +                                enum virVMOperationType vmOp);
> 
>   VIR_ENUM_DECL(virVirtualPort)
> +VIR_ENUM_DECL(virVMOperation)
> 
>   #endif /* __UTIL_MACVTAP_H__ */
> Index: libvirt-acl/src/libvirt_macvtap.syms
> ===================================================================
> --- libvirt-acl.orig/src/libvirt_macvtap.syms
> +++ libvirt-acl/src/libvirt_macvtap.syms
> @@ -3,3 +3,7 @@
>   # macvtap.h
>   openMacvtapTap;
>   delMacvtap;
> +vpAssociatePortProfileId;
> +vpDisassociatePortProfileId;
> +virVMOperationTypeToString;
> +virVMOperationTypeFromString;
> 
> 


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