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

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

On 09/23/2014 11:50 PM, Eric Blake wrote:
On 09/23/2014 03:26 PM, Pavel Hrdina wrote:

+    VIR_DEBUG("Relaying domain tunable event %s %d, callback %d",
+              dom->name, dom->id, callback->callbackID);

Might also be nice to log "%p %n", params, nparams

Yes, that would be probably nice, but since I've pushed this patch
already I can create a following patch with this small update?

Yes, a followup is fine.

+++ b/src/remote/remote_protocol.x
@@ -247,6 +247,9 @@ const REMOTE_NETWORK_DHCP_LEASES_MAX = 65536;
   /* Upper limit on count of parameters returned via bulk stats API */

+/* Upper limit of message size for tunable event. */

That feels excessive...

   /* UUID.  VIR_UUID_BUFLEN definition comes from libvirt.h */
   typedef opaque remote_uuid[VIR_UUID_BUFLEN];

@@ -2990,6 +2993,12 @@ struct remote_domain_event_block_job_2_msg {
       int status;

+struct remote_domain_event_callback_tunable_msg {
+    int callbackID;
+    remote_nonnull_domain dom;
+    remote_typed_param params<REMOTE_DOMAIN_EVENT_TUNABLE_MAX>;

...each param in the array will occupy multiple bytes.  I think that
something as low as 2048 for REMOTE_DOMAIN_EVENT_TUNABLE_MAX is still
plenty (we don't have that many tunables yet); even if each tunable
requires 64 bytes to transmit (mostly in the name of the parameter, but
also in the type and value), that's still well under a megabyte limit of
information passed on an instance of the event.

Well, yes and no :). Let's say, that we will add in the future (and I'm
planning to do it) blkiotune where you can update at the same time all
of the tunables for all host's disks where all params for now will be
only VIR_TYPED_PARAM_STRING and that could consume a lot of memory. I
know that it will probably never be that much, but I wanted to be sure
that we will have enough space for all possible tunable events.

Still, are you going to return 8 million separate strings?  Or just 8
million bytes but still contained within 2000 strings?  Seriously, I
think 2048 is a perfectly LARGE limit - there are not THAT many tunables
per domain.  The params<LIMIT> is not the overall size of the command,
but the number of parameters (each of which can be quite large if they
are type string)

Sigh, I should not work that late, because I've misunderstood the
meaning of the "LIMIT". I'll post a new value with patch for the
debug message.

Thanks, Pavel

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