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

Re: [libvirt] [PATCH v2 3/5] event: introduce new event for cputune



On 09/12/2014 09:46 PM, John Ferlan wrote:


On 09/10/2014 08:08 AM, Pavel Hrdina wrote:
This new event will use typedParameters to expose what has been actually
updated and the reason is that we can in the future extend the cputune.
With typedParameters we don't have to worry about creating some sort of
v2 of cputune event if there will be new features implemented for cputune.

Signed-off-by: Pavel Hrdina <phrdina redhat com>
---
  daemon/remote.c              | 45 ++++++++++++++++++++++++
  include/libvirt/libvirt.h.in | 19 ++++++++++
  src/conf/domain_event.c      | 84 ++++++++++++++++++++++++++++++++++++++++++++
  src/conf/domain_event.h      |  9 +++++
  src/libvirt_private.syms     |  2 ++
  src/remote/remote_driver.c   | 41 +++++++++++++++++++++
  src/remote/remote_protocol.x | 14 +++++++-
  src/remote_protocol-structs  |  9 +++++
  tools/virsh-domain.c         | 33 +++++++++++++++++
  9 files changed, 255 insertions(+), 1 deletion(-)


Looks OK to me - there one detail below...


[..]


  static void
+remoteDomainBuildEventCallbackCputune(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
+                                      virNetClientPtr client ATTRIBUTE_UNUSED,
+                                      void *evdata, void *opaque)
+{
+    virConnectPtr conn = opaque;
+    remote_domain_event_callback_cputune_msg *msg = evdata;
+    struct private_data *priv = conn->privateData;
+    virDomainPtr dom;
+    virTypedParameterPtr params = NULL;
+    int nparams = 0;
+    virObjectEventPtr event = NULL;
+
+    dom = get_nonnull_domain(conn, msg->dom);
+    if (!dom)
+        return;
+
+    if (remoteDeserializeTypedParameters(msg->params.params_val,
+                                         msg->params.params_len,
+                                         REMOTE_CPUMAPS_MAX,
+                                         &params, &nparams) < 0)
+        goto cleanup;
+
+    event = virDomainEventCputuneNewFromDom(dom, params, nparams);
+
+    remoteEventQueue(priv, event, msg->callbackID);
+
+ cleanup:
+    virDomainFree(dom);

Every other calling sequence as DomainFree followed by EventQueue - I
have to believe there's a reason - although nothing pops right out.

Like I said above in general ACK as I don't see anything obvious. It
certainly seems easier for me to add iothreadpin events into my code.

John


Yes that's true that every other event has virDomanFree followed by remoteEventQueue and the only reason I cant think of could be that virDomainFree is resetting last error, but I'm OK with changing the calling sequence.

Pavel


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