Re: [dpdk-dev] [PATCH v1 4/7] examples/power: add host channel to power manager

["Hunt, David" <david.hunt@intel.com>]

Hi Lei,


On 4/9/2018 8:31 AM, Yao, Lei A wrote:
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David Hunt
>> Sent: Thursday, August 30, 2018 6:54 PM
>> To: dev@dpdk.org
>> Cc: Mcnamara, John <john.mcnamara@intel.com>; Hunt, David
>> <david.hunt@intel.com>
>> Subject: [dpdk-dev] [PATCH v1 4/7] examples/power: add host channel to
>> power manager
>>
>> This patch adds a fifo channel to the vm_power_manager app through which
>> we can send commands and polices. Intended for sending JSON strings.
>> The fifo is at /tmp/powermonitor/fifo.0
>>
>> Signed-off-by: David Hunt <david.hunt@intel.com>
>> ---
>>   examples/vm_power_manager/channel_manager.c | 108
>> +++++++++++++++
>>   examples/vm_power_manager/channel_manager.h |  17 ++-
>>   examples/vm_power_manager/channel_monitor.c | 146
>> +++++++++++++++-----
>>   examples/vm_power_manager/main.c            |   2 +
>>   4 files changed, 238 insertions(+), 35 deletions(-)
>>
>> diff --git a/examples/vm_power_manager/channel_manager.c
>> b/examples/vm_power_manager/channel_manager.c
>> index 2bb8641d3..bcd106be1 100644
>> --- a/examples/vm_power_manager/channel_manager.c
>> +++ b/examples/vm_power_manager/channel_manager.c
>> @@ -13,6 +13,7 @@

--snip--

>>
>> @@ -160,8 +199,13 @@ update_policy(struct channel_packet *pkt)
>>   	unsigned int updated = 0;
>>   	int i;
>>
>> +
>> +	RTE_LOG(INFO, CHANNEL_MONITOR,
>> +			"Updating policy for %s\n", pkt->vm_name);
>> +
>>   	for (i = 0; i < MAX_VMS; i++) {
>>   		if (strcmp(policies[i].pkt.vm_name, pkt->vm_name) == 0) {
> I suggest add warning log here when no VM can match the policy name
> which we send through the fifo.0. Otherwise, the user can't aware the
> policy won't be applied.

There's already a flag here called "updated" that if it falls through 
this loop without finding the policy name, it adds a new one, so no need 
for the message.
I will however re-word the "Updating policy" message to read "Applying 
policy". "Applying" is less confusing.

Thanks,
Dave.


---snip---