Re: [PATCH v3 RESEND] qemu: Process RDMA GID state change event

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

 



On 12/24/18 11:15 AM, Yuval Shaia wrote:
> This event is emitted on the monitor when a GID table in pvrdma device
> is modified and the change needs to be propagate to the backend RDMA
> device's GID table.
> 
> The control over the RDMA device's GID table is done by updating the
> device's Ethernet function addresses.
> Usually the first GID entry is determine by the MAC address, the second
> by the first IPv6 address and the third by the IPv4 address. Other
> entries can be added by adding more IP addresses. The opposite is the
> same, i.e. whenever an address is removed, the corresponding GID entry
> is removed.
> 
> The process is done by the network and RDMA stacks. Whenever an address
> is added the ib_core driver is notified and calls the device driver's
> add_gid function which in turn update the device.
> 
> To support this in pvrdma device we need to hook into the create_bind
> and destroy_bind HW commands triggered by pvrdma driver in guest.
> Whenever a changed is made to the pvrdma device's GID table a special
> QMP messages is sent to be processed by libvirt to update the address of
> the backend Ethernet device.
> 
> Signed-off-by: Yuval Shaia <yuval.shaia@xxxxxxxxxx>
> ---
> (fixing mail subject from v2 to v3, rest is the same)
> Hi,
> 
> Corresponding qemu commit was merged to master as part of the following
> patch-set:
> https://www.mail-archive.com/qemu-devel@xxxxxxxxxx/msg583387.html
> 
> Appreciate if this patch can be reviewed and merged as well.
> 
> Thanks,
> Yuval
> 
> v1 -> v2:
>         * Address all comments from Michal Privoznik
> 
> v2 -> v3:
>         * Remove static initialization in processRdmaGidStatusChangedEvent
> ---
>  src/qemu/qemu_domain.c       |  3 +++
>  src/qemu/qemu_domain.h       |  1 +
>  src/qemu/qemu_driver.c       | 44 ++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor.c      | 27 +++++++++++++++++++
>  src/qemu/qemu_monitor.h      | 27 +++++++++++++++++++
>  src/qemu/qemu_monitor_json.c | 36 +++++++++++++++++++++++++
>  src/qemu/qemu_process.c      | 52 ++++++++++++++++++++++++++++++++++++
>  7 files changed, 190 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index ba3fff607a..8da54c7ee9 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -13479,6 +13479,9 @@ qemuProcessEventFree(struct qemuProcessEvent *event)
>      case QEMU_PROCESS_EVENT_GUESTPANIC:
>          qemuMonitorEventPanicInfoFree(event->data);
>          break;
> +    case QEMU_PROCESS_EVENT_RDMA_GID_STATUS_CHANGED:
> +        qemuMonitorEventRdmaGidStatusFree(event->data);
> +        break;
>      case QEMU_PROCESS_EVENT_WATCHDOG:
>      case QEMU_PROCESS_EVENT_DEVICE_DELETED:
>      case QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED:
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 80bd4bde91..64bceb9a98 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -487,6 +487,7 @@ typedef enum {
>      QEMU_PROCESS_EVENT_BLOCK_JOB,
>      QEMU_PROCESS_EVENT_MONITOR_EOF,
>      QEMU_PROCESS_EVENT_PR_DISCONNECT,
> +    QEMU_PROCESS_EVENT_RDMA_GID_STATUS_CHANGED,
>  
>      QEMU_PROCESS_EVENT_LAST
>  } qemuProcessEventType;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index a52e2495d5..5c6ab3c0ea 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4788,6 +4788,47 @@ processPRDisconnectEvent(virDomainObjPtr vm)
>  }
>  
>  
> +static void
> +processRdmaGidStatusChangedEvent(virDomainObjPtr vm,
> +                                 qemuMonitorRdmaGidStatusChangedPrivatePtr info)
> +{
> +    unsigned int prefix_len;
> +    virSocketAddr addr;
> +    int rc;
> +
> +    if (!virDomainObjIsActive(vm))
> +        return;
> +
> +    VIR_DEBUG("netdev=%s,gid_status=%d,subnet_prefix=0x%llx,interface_id=0x%llx",
> +              info->netdev, info->gid_status, info->subnet_prefix,
> +              info->interface_id);
> +
> +    if (info->subnet_prefix) {
> +        prefix_len = 64;
> +        uint32_t ipv6[4];
> +        memcpy(&ipv6[0], &info->subnet_prefix, sizeof(info->subnet_prefix));
> +        memcpy(&ipv6[2], &info->interface_id, sizeof(info->interface_id));
> +        virSocketAddrSetIPv6AddrNetOrder(&addr, ipv6);
> +    } else {
> +        prefix_len = 24;
> +        virSocketAddrSetIPv4AddrNetOrder(&addr, info->interface_id >> 32);
> +    }
> +
> +    if (info->gid_status) {
> +        VIR_DEBUG("Adding %s to %s", virSocketAddrFormat(&addr), info->netdev);

This leaks memory. virSocketAddrFormat() allocates return buffer and must be freed afterwards. Also, formatting it multiple times looks redundant.

> +        rc = virNetDevIPAddrAdd(info->netdev, &addr, NULL, prefix_len);
> +    } else {
> +        VIR_DEBUG("Removing %s from %s", virSocketAddrFormat(&addr),
> +                  info->netdev);
> +        rc = virNetDevIPAddrDel(info->netdev, &addr, prefix_len);
> +    }
> +
> +    if (rc < 0)
> +        VIR_WARN("Fail to update address %s to %s", virSocketAddrFormat(&addr),
> +                 info->netdev);
> +}
> +
> +
>  static void qemuProcessEventHandler(void *data, void *opaque)
>  {
>      struct qemuProcessEvent *processEvent = data;
> @@ -4828,6 +4869,9 @@ static void qemuProcessEventHandler(void *data, void *opaque)
>      case QEMU_PROCESS_EVENT_PR_DISCONNECT:
>          processPRDisconnectEvent(vm);
>          break;
> +    case QEMU_PROCESS_EVENT_RDMA_GID_STATUS_CHANGED:
> +        processRdmaGidStatusChangedEvent(vm, processEvent->data);
> +        break;
>      case QEMU_PROCESS_EVENT_LAST:
>          break;
>      }
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 7f7013e115..4bf71dbf8c 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -1686,6 +1686,22 @@ qemuMonitorEmitPRManagerStatusChanged(qemuMonitorPtr mon,
>  }
>  
>  
> +int
> +qemuMonitorEmitRdmaGidStatusChanged(qemuMonitorPtr mon, const char *netdev,
> +                                    bool gid_status, uint64_t subnet_prefix,
> +                                    uint64_t interface_id)
> +{
> +    int ret = -1;
> +    VIR_DEBUG("netdev=%s,gid_status=%d,subnet_prefix=0x%lx,interface_id=0x%lx",
> +              netdev, gid_status, subnet_prefix, interface_id);
> +
> +    QEMU_MONITOR_CALLBACK(mon, ret, domainRdmaGidStatusChanged, mon->vm, netdev,
> +                          gid_status, subnet_prefix, interface_id);
> +
> +    return ret;
> +}
> +
> +
>  int
>  qemuMonitorSetCapabilities(qemuMonitorPtr mon)
>  {
> @@ -4298,6 +4314,17 @@ qemuMonitorEventPanicInfoFree(qemuMonitorEventPanicInfoPtr info)
>  }
>  
>  
> +void
> +qemuMonitorEventRdmaGidStatusFree(qemuMonitorRdmaGidStatusChangedPrivatePtr info)
> +{
> +    if (!info)
> +        return;
> +
> +    VIR_FREE(info->netdev);
> +    VIR_FREE(info);
> +}
> +
> +
>  int
>  qemuMonitorSetWatchdogAction(qemuMonitorPtr mon,
>                               const char *action)
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 48b142a4f4..b639a0a9d2 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -109,8 +109,22 @@ struct _qemuMonitorEventPanicInfo {
>      } data;
>  };
>  
> +
> +typedef struct _qemuMonitorRdmaGidStatusChangedPrivate qemuMonitorRdmaGidStatusChangedPrivate;
> +typedef qemuMonitorRdmaGidStatusChangedPrivate *qemuMonitorRdmaGidStatusChangedPrivatePtr;
> +struct _qemuMonitorRdmaGidStatusChangedPrivate {
> +    virObject parent;
> +

Why is this ^^^ here? This is not a virObject nor you're handling it as such.

Also, the event is called RDMA_GID_STATUS_CHANGED so there's no need for 'Private' suffix in the name of struct. In fact, if 'Changed' is dropped too then this struct could be used more widely (if there is ever another RDMA event).

> +    char *netdev;
> +    bool gid_status;
> +    unsigned long long subnet_prefix;
> +    unsigned long long interface_id;
> +};
> +
> +
>  char *qemuMonitorGuestPanicEventInfoFormatMsg(qemuMonitorEventPanicInfoPtr info);
>  void qemuMonitorEventPanicInfoFree(qemuMonitorEventPanicInfoPtr info);
> +void qemuMonitorEventRdmaGidStatusFree(qemuMonitorRdmaGidStatusChangedPrivatePtr info);
>  
>  typedef void (*qemuMonitorDestroyCallback)(qemuMonitorPtr mon,
>                                             virDomainObjPtr vm,
> @@ -281,6 +295,14 @@ typedef int (*qemuMonitorDomainPRManagerStatusChangedCallback)(qemuMonitorPtr mo
>                                                                 bool connected,
>                                                                 void *opaque);
>  
> +typedef int (*qemuMonitorDomainRdmaGidStatusChangedCallback)(qemuMonitorPtr mon,
> +                                                             virDomainObjPtr vm,
> +                                                             const char *netdev,
> +                                                             bool gid_status,
> +                                                             uint64_t subnet_prefix,
> +                                                             uint64_t interface_id,
> +                                                             void *opaque);
> +
>  typedef struct _qemuMonitorCallbacks qemuMonitorCallbacks;
>  typedef qemuMonitorCallbacks *qemuMonitorCallbacksPtr;
>  struct _qemuMonitorCallbacks {
> @@ -314,6 +336,7 @@ struct _qemuMonitorCallbacks {
>      qemuMonitorDomainBlockThresholdCallback domainBlockThreshold;
>      qemuMonitorDomainDumpCompletedCallback domainDumpCompleted;
>      qemuMonitorDomainPRManagerStatusChangedCallback domainPRManagerStatusChanged;
> +    qemuMonitorDomainRdmaGidStatusChangedCallback domainRdmaGidStatusChanged;
>  };
>  
>  char *qemuMonitorEscapeArg(const char *in);
> @@ -448,6 +471,10 @@ int qemuMonitorEmitPRManagerStatusChanged(qemuMonitorPtr mon,
>                                            const char *prManager,
>                                            bool connected);
>  
> +int qemuMonitorEmitRdmaGidStatusChanged(qemuMonitorPtr mon, const char *netdev,
> +                                        bool gid_status, uint64_t subnet_prefix,
> +                                        uint64_t interface_id);
> +
>  int qemuMonitorStartCPUs(qemuMonitorPtr mon);
>  int qemuMonitorStopCPUs(qemuMonitorPtr mon);
>  
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 3de298c9e2..8df9b426ba 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -91,6 +91,7 @@ static void qemuMonitorJSONHandleAcpiOstInfo(qemuMonitorPtr mon, virJSONValuePtr
>  static void qemuMonitorJSONHandleBlockThreshold(qemuMonitorPtr mon, virJSONValuePtr data);
>  static void qemuMonitorJSONHandleDumpCompleted(qemuMonitorPtr mon, virJSONValuePtr data);
>  static void qemuMonitorJSONHandlePRManagerStatusChanged(qemuMonitorPtr mon, virJSONValuePtr data);
> +static void qemuMonitorJSONHandleRdmaGidStatusChanged(qemuMonitorPtr mon, virJSONValuePtr data);
>  
>  typedef struct {
>      const char *type;
> @@ -114,6 +115,7 @@ static qemuEventHandler eventHandlers[] = {
>      { "NIC_RX_FILTER_CHANGED", qemuMonitorJSONHandleNicRxFilterChanged, },
>      { "POWERDOWN", qemuMonitorJSONHandlePowerdown, },
>      { "PR_MANAGER_STATUS_CHANGED", qemuMonitorJSONHandlePRManagerStatusChanged, },
> +    { "RDMA_GID_STATUS_CHANGED", qemuMonitorJSONHandleRdmaGidStatusChanged, },
>      { "RESET", qemuMonitorJSONHandleReset, },
>      { "RESUME", qemuMonitorJSONHandleResume, },
>      { "RTC_CHANGE", qemuMonitorJSONHandleRTCChange, },
> @@ -1351,6 +1353,40 @@ static void qemuMonitorJSONHandlePRManagerStatusChanged(qemuMonitorPtr mon,
>  }
>  
>  
> +static void qemuMonitorJSONHandleRdmaGidStatusChanged(qemuMonitorPtr mon,
> +                                                      virJSONValuePtr data)
> +{
> +    const char *netdev;
> +    bool gid_status;
> +    unsigned long long subnet_prefix, interface_id;
> +
> +    if (!(netdev = virJSONValueObjectGetString(data, "netdev"))) {
> +        VIR_WARN("missing netdev in GID_STATUS_CHANGED event");
> +        return;
> +    }
> +
> +    if (virJSONValueObjectGetBoolean(data, "gid-status", &gid_status)) {
> +        VIR_WARN("missing gid-status in GID_STATUS_CHANGED event");
> +        return;
> +    }
> +
> +    if (virJSONValueObjectGetNumberUlong(data, "subnet-prefix",
> +                                         &subnet_prefix)) {
> +        VIR_WARN("missing subnet-prefix in GID_STATUS_CHANGED event");
> +        return;
> +    }
> +
> +    if (virJSONValueObjectGetNumberUlong(data, "interface-id",
> +                                         &interface_id)) {
> +        VIR_WARN("missing interface-id in GID_STATUS_CHANGED event");
> +        return;
> +    }
> +
> +    qemuMonitorEmitRdmaGidStatusChanged(mon, netdev, gid_status, subnet_prefix,
> +                                        interface_id);
> +}
> +
> +
>  int
>  qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon,
>                                    const char *cmd_str,
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 9cf971808c..6cf0ace5cf 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -1703,6 +1703,57 @@ qemuProcessHandlePRManagerStatusChanged(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>  }
>  
>  
> +static int
> +qemuProcessHandleRdmaGidStatusChanged(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
> +                                      virDomainObjPtr vm, const char *netdev,
> +                                      bool gid_status, uint64_t subnet_prefix,
> +                                      uint64_t interface_id, void *opaque)
> +{
> +    virQEMUDriverPtr driver = opaque;
> +    struct qemuProcessEvent *processEvent = NULL;
> +    qemuMonitorRdmaGidStatusChangedPrivatePtr rdmaGitStatusChangedPriv = NULL;
> +    int ret = -1;
> +
> +    virObjectLock(vm);
> +
> +    VIR_DEBUG("netdev=%s,gid_status=%d,subnet_prefix=0x%lx,interface_id=0x%lx",
> +              netdev, gid_status, subnet_prefix, interface_id);
> +
> +    if (VIR_ALLOC(rdmaGitStatusChangedPriv) < 0)
> +        goto out_unlock;
> +
> +    if (VIR_STRDUP(rdmaGitStatusChangedPriv->netdev, netdev) < 0)
> +        goto out_free;
> +
> +    rdmaGitStatusChangedPriv->gid_status = gid_status;
> +    rdmaGitStatusChangedPriv->subnet_prefix = subnet_prefix;
> +    rdmaGitStatusChangedPriv->interface_id = interface_id;
> +
> +    if (VIR_ALLOC(processEvent) < 0)
> +        goto out_free;
> +
> +    processEvent->eventType = QEMU_PROCESS_EVENT_RDMA_GID_STATUS_CHANGED;
> +    processEvent->vm = virObjectRef(vm);
> +    processEvent->data = rdmaGitStatusChangedPriv;
> +
> +    if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
> +        qemuProcessEventFree(processEvent);
> +        virObjectUnref(vm);
> +        goto out_free;
> +    }
> +
> +    ret = 0;
> +    goto out_unlock;
> +
> + out_free:
> +    qemuMonitorEventRdmaGidStatusFree(rdmaGitStatusChangedPriv);
> +
> + out_unlock:
> +    virObjectUnlock(vm);
> +    return ret;

Oh, we usually try to avoid too many labels. In fact we allow only 'cleanup' and 'error' lables (except for a very few exceptions). This can be rewritten to be much clearer. For instance, you have a double free here: rdmaGitStatusChangedPriv is allocated and then is passed to processEvent->data. However, if virThreadPoolSendJob() fails, then qemuProcessEventFree() is called which frees the @data and then you jump onto out_free which frees it again.

I'm squashing this in:

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 74ddc9e592..1d961707cc 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -4794,22 +4794,24 @@ processPRDisconnectEvent(virDomainObjPtr vm)
 
 static void
 processRdmaGidStatusChangedEvent(virDomainObjPtr vm,
-                                 qemuMonitorRdmaGidStatusChangedPrivatePtr info)
+                                 qemuMonitorRdmaGidStatusPtr info)
 {
     unsigned int prefix_len;
     virSocketAddr addr;
+    VIR_AUTOFREE(char *) addrStr = NULL;
     int rc;
 
     if (!virDomainObjIsActive(vm))
         return;
 
-    VIR_DEBUG("netdev=%s,gid_status=%d,subnet_prefix=0x%llx,interface_id=0x%llx",
+    VIR_DEBUG("netdev=%s, gid_status=%d, subnet_prefix=0x%llx, interface_id=0x%llx",
               info->netdev, info->gid_status, info->subnet_prefix,
               info->interface_id);
 
     if (info->subnet_prefix) {
+        uint32_t ipv6[4] = {0};
+
         prefix_len = 64;
-        uint32_t ipv6[4];
         memcpy(&ipv6[0], &info->subnet_prefix, sizeof(info->subnet_prefix));
         memcpy(&ipv6[2], &info->interface_id, sizeof(info->interface_id));
         virSocketAddrSetIPv6AddrNetOrder(&addr, ipv6);
@@ -4818,18 +4820,19 @@ processRdmaGidStatusChangedEvent(virDomainObjPtr vm,
         virSocketAddrSetIPv4AddrNetOrder(&addr, info->interface_id >> 32);
     }
 
+    if (!(addrStr = virSocketAddrFormat(&addr)))
+        return;
+
     if (info->gid_status) {
-        VIR_DEBUG("Adding %s to %s", virSocketAddrFormat(&addr), info->netdev);
+        VIR_DEBUG("Adding %s to %s", addrStr, info->netdev);
         rc = virNetDevIPAddrAdd(info->netdev, &addr, NULL, prefix_len);
     } else {
-        VIR_DEBUG("Removing %s from %s", virSocketAddrFormat(&addr),
-                  info->netdev);
+        VIR_DEBUG("Removing %s from %s", addrStr, info->netdev);
         rc = virNetDevIPAddrDel(info->netdev, &addr, prefix_len);
     }
 
     if (rc < 0)
-        VIR_WARN("Fail to update address %s to %s", virSocketAddrFormat(&addr),
-                 info->netdev);
+        VIR_WARN("Fail to update address %s to %s", addrStr, info->netdev);
 }
 
 
diff --git i/src/qemu/qemu_monitor.c w/src/qemu/qemu_monitor.c
index e27a17ee94..419ea66a6a 100644
--- i/src/qemu/qemu_monitor.c
+++ w/src/qemu/qemu_monitor.c
@@ -1685,16 +1685,18 @@ qemuMonitorEmitPRManagerStatusChanged(qemuMonitorPtr mon,
 
 
 int
-qemuMonitorEmitRdmaGidStatusChanged(qemuMonitorPtr mon, const char *netdev,
-                                    bool gid_status, uint64_t subnet_prefix,
+qemuMonitorEmitRdmaGidStatusChanged(qemuMonitorPtr mon,
+                                    const char *netdev,
+                                    bool gid_status,
+                                    uint64_t subnet_prefix,
                                     uint64_t interface_id)
 {
     int ret = -1;
-    VIR_DEBUG("netdev=%s,gid_status=%d,subnet_prefix=0x%lx,interface_id=0x%lx",
+    VIR_DEBUG("netdev=%s, gid_status=%d, subnet_prefix=0x%lx, interface_id=0x%lx",
               netdev, gid_status, subnet_prefix, interface_id);
 
-    QEMU_MONITOR_CALLBACK(mon, ret, domainRdmaGidStatusChanged, mon->vm, netdev,
-                          gid_status, subnet_prefix, interface_id);
+    QEMU_MONITOR_CALLBACK(mon, ret, domainRdmaGidStatusChanged, mon->vm,
+                          netdev, gid_status, subnet_prefix, interface_id);
 
     return ret;
 }
@@ -4332,7 +4334,7 @@ qemuMonitorEventPanicInfoFree(qemuMonitorEventPanicInfoPtr info)
 
 
 void
-qemuMonitorEventRdmaGidStatusFree(qemuMonitorRdmaGidStatusChangedPrivatePtr info)
+qemuMonitorEventRdmaGidStatusFree(qemuMonitorRdmaGidStatusPtr info)
 {
     if (!info)
         return;
diff --git i/src/qemu/qemu_monitor.h w/src/qemu/qemu_monitor.h
index c0e7789f1e..93804233db 100644
--- i/src/qemu/qemu_monitor.h
+++ w/src/qemu/qemu_monitor.h
@@ -107,11 +107,9 @@ struct _qemuMonitorEventPanicInfo {
 };
 
 
-typedef struct _qemuMonitorRdmaGidStatusChangedPrivate qemuMonitorRdmaGidStatusChangedPrivate;
-typedef qemuMonitorRdmaGidStatusChangedPrivate *qemuMonitorRdmaGidStatusChangedPrivatePtr;
-struct _qemuMonitorRdmaGidStatusChangedPrivate {
-    virObject parent;
-
+typedef struct _qemuMonitorRdmaGidStatus qemuMonitorRdmaGidStatus;
+typedef qemuMonitorRdmaGidStatus *qemuMonitorRdmaGidStatusPtr;
+struct _qemuMonitorRdmaGidStatus {
     char *netdev;
     bool gid_status;
     unsigned long long subnet_prefix;
@@ -121,7 +119,7 @@ struct _qemuMonitorRdmaGidStatusChangedPrivate {
 
 char *qemuMonitorGuestPanicEventInfoFormatMsg(qemuMonitorEventPanicInfoPtr info);
 void qemuMonitorEventPanicInfoFree(qemuMonitorEventPanicInfoPtr info);
-void qemuMonitorEventRdmaGidStatusFree(qemuMonitorRdmaGidStatusChangedPrivatePtr info);
+void qemuMonitorEventRdmaGidStatusFree(qemuMonitorRdmaGidStatusPtr info);
 
 typedef void (*qemuMonitorDestroyCallback)(qemuMonitorPtr mon,
                                            virDomainObjPtr vm,
diff --git i/src/qemu/qemu_process.c w/src/qemu/qemu_process.c
index e1241fd6ac..83a054663f 100644
--- i/src/qemu/qemu_process.c
+++ w/src/qemu/qemu_process.c
@@ -1717,13 +1717,16 @@ qemuProcessHandlePRManagerStatusChanged(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
 
 static int
 qemuProcessHandleRdmaGidStatusChanged(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
-                                      virDomainObjPtr vm, const char *netdev,
-                                      bool gid_status, uint64_t subnet_prefix,
-                                      uint64_t interface_id, void *opaque)
+                                      virDomainObjPtr vm,
+                                      const char *netdev,
+                                      bool gid_status,
+                                      uint64_t subnet_prefix,
+                                      uint64_t interface_id,
+                                      void *opaque)
 {
     virQEMUDriverPtr driver = opaque;
     struct qemuProcessEvent *processEvent = NULL;
-    qemuMonitorRdmaGidStatusChangedPrivatePtr rdmaGitStatusChangedPriv = NULL;
+    qemuMonitorRdmaGidStatusPtr info = NULL;
     int ret = -1;
 
     virObjectLock(vm);
@@ -1731,36 +1734,30 @@ qemuProcessHandleRdmaGidStatusChanged(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
     VIR_DEBUG("netdev=%s,gid_status=%d,subnet_prefix=0x%lx,interface_id=0x%lx",
               netdev, gid_status, subnet_prefix, interface_id);
 
-    if (VIR_ALLOC(rdmaGitStatusChangedPriv) < 0)
-        goto out_unlock;
+    if (VIR_ALLOC(info) < 0 ||
+        VIR_STRDUP(info->netdev, netdev) < 0)
+        goto cleanup;
 
-    if (VIR_STRDUP(rdmaGitStatusChangedPriv->netdev, netdev) < 0)
-        goto out_free;
-
-    rdmaGitStatusChangedPriv->gid_status = gid_status;
-    rdmaGitStatusChangedPriv->subnet_prefix = subnet_prefix;
-    rdmaGitStatusChangedPriv->interface_id = interface_id;
+    info->gid_status = gid_status;
+    info->subnet_prefix = subnet_prefix;
+    info->interface_id = interface_id;
 
     if (VIR_ALLOC(processEvent) < 0)
-        goto out_free;
+        goto cleanup;
 
     processEvent->eventType = QEMU_PROCESS_EVENT_RDMA_GID_STATUS_CHANGED;
     processEvent->vm = virObjectRef(vm);
-    processEvent->data = rdmaGitStatusChangedPriv;
+    VIR_STEAL_PTR(processEvent->data, info);
 
     if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
         qemuProcessEventFree(processEvent);
         virObjectUnref(vm);
-        goto out_free;
+        goto cleanup;
     }
 
     ret = 0;
-    goto out_unlock;
-
- out_free:
-    qemuMonitorEventRdmaGidStatusFree(rdmaGitStatusChangedPriv);
-
- out_unlock:
+ cleanup:
+    qemuMonitorEventRdmaGidStatusFree(info);
     virObjectUnlock(vm);
     return ret;
 }




ACKed and pushed. Congratulations on your first libvirt contribution!

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux