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

Re: [libvirt] [PATCH 1/1] conf, util: on restart of libvirt restart vepa callbacks



On 03/14/2012 05:00 AM, D. Herrendoerfer wrote:
> From: "D. Herrendoerfer" <d herrendoerfer herrendoerfer name>
>
> When libvirtd is restarted, also restart the netlink event
> message callbacks for existing VEPA connections and send
> a message to lldpad for these existing links, so it learns
> the new libvirtd pid.
>
> Note: This Patch provides fixes to a number of scenarios where
> lldpad, switch, and libvirt would loose syncronization.
>  1. Crash/Reboot of a switch.
>  2. lldpad crash.
>  3. Clearing of the local switch database. (cleanvms)
>  4. libvirtd crash or restart. 
> In any of the above events restarting libvirtd will restore normal
> operations of the VMs on their virtual network.
>
> Signed-off-by: D. Herrendoerfer <d herrendoerfer herrendoerfer name>
> ---
>  src/conf/domain_conf.c      |   18 ++++++
>  src/util/virnetdevmacvlan.c |  128 +++++++++++++++++++++++++++++++++---------
>  src/util/virnetdevmacvlan.h |    9 +++
>  3 files changed, 127 insertions(+), 28 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b994718..2ea3ea7 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -52,6 +52,7 @@
>  #include "secret_conf.h"
>  #include "netdev_vport_profile_conf.h"
>  #include "netdev_bandwidth_conf.h"
> +#include "virnetdevmacvlan.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_DOMAIN
>  
> @@ -4545,6 +4546,23 @@ virDomainNetDefParseXML(virCapsPtr caps,
>  
>          if ((flags & VIR_DOMAIN_XML_INACTIVE))
>              VIR_FREE(ifname);
> +        else {
> +            char *tmp;
> +            VIR_DEBUG("Device already active on %s in VEPA mode.",ifname);
> +
> +            tmp = virXPathString("string(../../uuid[1])", ctxt);
> +
> +            if (tmp) {
> +                VIR_DEBUG("Active VM UUID: %s. Restarting association and callback handler.",tmp);
> +                ignore_value(virNetDevMacVLanRestartWithVPortProfile(ifname,
> +                                                                     def->mac,
> +                                                                     def->data.direct.linkdev,
> +                                                                     tmp,
> +                                                                     def->data.direct.virtPortProfile,
> +                                                                     VIR_NETDEV_VPORT_PROFILE_OP_CREATE));
> +                VIR_FREE(tmp);
> +            }
> +        }

This is not the correct place to be taking action on the results of a
parse - the parse functions should just parse the xml into objects, and
not make assumptions about what will be done with them. Instead you need
to add code to each hypervisor driver's xxxReload() function just after
it has called virDomainLoadAllConfigs to reload all the "running" or
"status" configs. For example, for the qemu driver, you could add
something like the following to qemudStartup() just after the call to
qemuProcessReconnectAll() (around src/qemu/qemu_driver.c:679):

    virHashForEach(qemu_driver->domains.objs, qemuDomainNetsRestart, NULL);

(and somewhere above that:)

static void qemuDomainNetsReload(void *payload,
                                 const void *name ATTRIBUTE_UNUSED,
                                 void *data)
{
    int i;
    virDomainObjPtr vm = (virDomainObjPtr)payload;
    virDomainDefPtr def = vm->def;

    virDomainObjLock(vm);

    for (i = 0; i < def->nnets; i++) {
        virDomainNetDefPtr net = def->nets[i];
        if ("this net needs action taken") {
            "take action"
        }
    }

cleanup:
    virDomainObjUnlock(vm);
}

You'll need to do something similar for all the other hypervisor types
that support macvtap interfaces and port profiles (again, look for the
xxxReload() function of that hypervisor, most easily found by a tag
search for virDomainLoadAllConfigs).

Beyond this, I don't see any explicit action to remove the stale
registration from lldpad. Is there a way to do this? Or will lldpad
possibly remove it by itself after the first time it gets no reponse?


>  
>          break;
>  
> diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
> index 647679f..19daf57 100644
> --- a/src/util/virnetdevmacvlan.c
> +++ b/src/util/virnetdevmacvlan.c
> @@ -769,6 +769,49 @@ virNetDevMacVLanVPortProfileDestroyCallback(int watch ATTRIBUTE_UNUSED,
>      virNetlinkCallbackDataFree((virNetlinkCallbackDataPtr)opaque);
>  }
>  
> +static int
> +virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname,
> +                                             const unsigned char *macaddress,
> +                                             const char *linkdev,
> +                                             const unsigned char *vmuuid,
> +                                             virNetDevVPortProfilePtr virtPortProfile,
> +                                             enum virNetDevVPortProfileOp vmOp)
> +{
> +    virNetlinkCallbackDataPtr calld = NULL;
> +
> +    if (virtPortProfile && virNetlinkEventServiceIsRunning()) {
> +        if (VIR_ALLOC(calld) < 0)
> +            goto memory_error;
> +        if ((calld->cr_ifname = strdup(ifname)) == NULL)
> +            goto memory_error;
> +        if (VIR_ALLOC(calld->virtPortProfile) < 0)
> +            goto memory_error;
> +        memcpy(calld->virtPortProfile, virtPortProfile, sizeof(*virtPortProfile));
> +        if (VIR_ALLOC_N(calld->macaddress, VIR_MAC_BUFLEN) < 0)
> +            goto memory_error;
> +        memcpy(calld->macaddress, macaddress, VIR_MAC_BUFLEN);
> +        if ((calld->linkdev = strdup(linkdev)) == NULL)
> +            goto  memory_error;
> +        if (VIR_ALLOC_N(calld->vmuuid, VIR_UUID_BUFLEN) < 0)
> +            goto memory_error;
> +        memcpy(calld->vmuuid, vmuuid, VIR_UUID_BUFLEN);
> +
> +        calld->vmOp = vmOp;
> +
> +        virNetlinkEventAddClient(virNetDevMacVLanVPortProfileCallback,
> +                                 virNetDevMacVLanVPortProfileDestroyCallback,
> +                                 calld, macaddress);
> +    }
> +
> +    return 0;
> +
> + memory_error:
> +    virReportOOMError();
> +    virNetlinkCallbackDataFree(calld);
> +
> +    return -1;
> +}
> +
>  
>  /**
>   * virNetDevMacVLanCreateWithVPortProfile:
> @@ -810,7 +853,6 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
>      int retries, do_retry = 0;
>      uint32_t macvtapMode;
>      const char *cr_ifname;
> -    virNetlinkCallbackDataPtr calld = NULL;
>      int ret;
>      int vf = -1;
>  
> @@ -917,36 +959,12 @@ create_name:
>          goto disassociate_exit;
>      }
>  
> -    if (virtPortProfile && virNetlinkEventServiceIsRunning()) {
> -        if (VIR_ALLOC(calld) < 0)
> -            goto memory_error;
> -        if ((calld->cr_ifname = strdup(cr_ifname)) == NULL)
> -            goto memory_error;
> -        if (VIR_ALLOC(calld->virtPortProfile) < 0)
> -            goto memory_error;
> -        memcpy(calld->virtPortProfile, virtPortProfile, sizeof(*virtPortProfile));
> -        if (VIR_ALLOC_N(calld->macaddress, VIR_MAC_BUFLEN) < 0)
> -            goto memory_error;
> -        memcpy(calld->macaddress, macaddress, VIR_MAC_BUFLEN);
> -        if ((calld->linkdev = strdup(linkdev)) == NULL)
> -            goto  memory_error;
> -        if (VIR_ALLOC_N(calld->vmuuid, VIR_UUID_BUFLEN) < 0)
> -            goto memory_error;
> -        memcpy(calld->vmuuid, vmuuid, VIR_UUID_BUFLEN);
> -
> -        calld->vmOp = vmOp;
> -
> -        virNetlinkEventAddClient(virNetDevMacVLanVPortProfileCallback,
> -                                 virNetDevMacVLanVPortProfileDestroyCallback,
> -                                 calld, macaddress);
> -    }
> +    if (virNetDevMacVLanVPortProfileRegisterCallback(cr_ifname, macaddress,
> +                                         linkdev, vmuuid, virtPortProfile, vmOp) < 0 )
> +        goto disassociate_exit;
>  
>      return rc;
>  
> - memory_error:
> -    virReportOOMError();
> -    virNetlinkCallbackDataFree(calld);
> -
>  disassociate_exit:
>      ignore_value(virNetDevVPortProfileDisassociate(cr_ifname,
>                                                     virtPortProfile,
> @@ -1003,6 +1021,48 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname,
>      return ret;
>  }
>  
> +/**
> + * virNetDevMacVLanRestartWithVPortProfile:
> + * Register a port profile callback handler for a VM that
> + * is already running
> + * .
> + * @cr_ifname: Interface name that the macvtap has.
> + * @macaddress: The MAC address for the macvtap device
> + * @linkdev: The interface name of the NIC to connect to the external bridge
> + * @vmuuid: The UUID of the VM the macvtap belongs to
> + * @virtPortProfile: pointer to object holding the virtual port profile data
> + * @vmOp: Operation to use during setup of the association
> + *
> + * Returns 0; returns -1 on error.
> + */
> +int virNetDevMacVLanRestartWithVPortProfile(const char *cr_ifname,
> +                                           const unsigned char *macaddress,
> +                                           const char *linkdev,
> +                                           const unsigned char *vmuuid,
> +                                           virNetDevVPortProfilePtr virtPortProfile,
> +                                           enum virNetDevVPortProfileOp vmOp)
> +{
> +    int rc = 0;
> +
> +    rc = virNetDevMacVLanVPortProfileRegisterCallback(cr_ifname, macaddress,
> +                                                      linkdev, vmuuid,
> +                                                      virtPortProfile, vmOp);
> +    if (rc < 0)
> +        goto error;
> +
> +    ignore_value(virNetDevVPortProfileAssociate(cr_ifname,
> +                                                virtPortProfile,
> +                                                macaddress,
> +                                                linkdev,
> +                                                -1,
> +                                                vmuuid,
> +                                                vmOp, true));
> +
> +error:
> +    return rc;
> +
> +}
> +
>  #else /* ! WITH_MACVTAP */
>  int virNetDevMacVLanCreate(const char *ifname ATTRIBUTE_UNUSED,
>                             const char *type ATTRIBUTE_UNUSED,
> @@ -1052,4 +1112,16 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname ATTRIBUTE_UNUSED,
>                           _("Cannot create macvlan devices on this platform"));
>      return -1;
>  }
> +
> +int virNetDevMacVLanRestartWithVPortProfile(const char *cr_ifname,
> +                                           const unsigned char *macaddress,
> +                                           const char *linkdev,
> +                                           const unsigned char *vmuuid,
> +                                           virNetDevVPortProfilePtr virtPortProfile,
> +                                           enum virNetDevVPortProfileOp vmOp)

All of the args of these stub functions must be qualified with a
trailing "ATTRIBUTE_UNUSED" attribute (see the immediately preceding
function for an example).


> +{
> +    virReportSystemError(ENOSYS, "%s",
> +                         _("Cannot create macvlan devices on this platform"));
> +    return -1;
> +}
>  #endif /* ! WITH_MACVTAP */
> diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h
> index 130ecea..14640cf 100644
> --- a/src/util/virnetdevmacvlan.h+++ b/src/util/virnetdevmacvlan.h
> @@ -75,4 +75,13 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname,
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
>      ATTRIBUTE_NONNULL(6) ATTRIBUTE_RETURN_CHECK;
>  
> +int virNetDevMacVLanRestartWithVPortProfile(const char *cr_ifname,
> +                                           const unsigned char *macaddress,
> +                                           const char *linkdev,
> +                                           const unsigned char *vmuuid,
> +                                           virNetDevVPortProfilePtr virtPortProfile,
> +                                           enum virNetDevVPortProfileOp vmOp)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
> +    ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) ATTRIBUTE_RETURN_CHECK;
> +
>  #endif /* __UTIL_MACVTAP_H__ */



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