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

Re: [libvirt] [PATCH v4 2/4] event: introduce new event for tunable values



On 09/22/2014 05:50 PM, John Ferlan wrote:


On 09/18/2014 04:39 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 any tunable
values or add new tunable values. With typedParameters we don't have to
worry about creating some other events, we will just use this universal
event to inform user about updates.

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


Seems you covered previous code review comments just fine, although I
have a couple of questions

#1. Is virsh-domain.c relative here or perhaps later?  Or should the
verb/action be changed from "cpu-tune" to something more generic?
"tunable"?


That should be definitely changed to "tunable". Thanks for catching it.

#2. Should the REMOTE_CPUMAPS_MAX be replaced by something more name
(and size) relative to these events?  And secondarily, since event max
params is relevant to the size of the params buffer allocated for each
event series shouldn't that be passed along and eventually used as the
limit for the remoteDeserializeTypedParameters?  Although I'm not quite
clear/sure how that works with the protocol definition file.


And this one is also wrong and should definitely be REMOTE_DOMAIN_EVENT_TUNABLE_MAX and that means create a new const. About the size of the rpc message I should extend the size as in the future there could be some tunable staff that will require more memory than the cputune event.

Currently maximal size of any rpc message is 16MiB and I think that 8MiB should by more than enough limit for the tunable event, what do you think?

Pavel

I think this is ACK-able - just want to make sure of the above - #1
should be 'simple' right?  #2 is just one of those size things - the
current usage goes on a byte max, but is that relative to how this has
changed/evolved.

John


[...]


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