Re: [Libvirt-cim] [PATCH] SwitchService: make command configurable
Dear John, What's your opinion about my solution? If no problem, we'll develop patch as described. ver. 0.6.2 has to be released as soon as possible:-) Sincerely yours, Xu Wang From: cngesaint@outlook.com To: libvirt-cim@redhat.com Date: Wed, 10 Apr 2013 10:20:34 +0000 Subject: Re: [Libvirt-cim] [PATCH] SwitchService: make command configurable I think the "vsi_search_string be an array" problem could be solved as following steps: 1. An item such as (vsi_support_key_string="{supported forwarding mode: (0x40) reflective relay, supported capabilities: (0x7) RTE ECP VDP}";) is configured well in the .conf file. 2. The application read this value from .conf file as a long string. 3. The application executes supporting function about VSI and check whether the output returned is a substring of vsi_support_key_string. If the output contains in the vsi_support_key_string, supporting of VSI is proved. Sincerely yours, Xu Wang
On 04/09/2013 06:40 AM, Wenchao Xia wrote:
Hi, John
Sorry to interrupt but still need you to review this patch, which
make it work more flex on different version.
By the way, I got only 3 fail and they seems small problems in test
suit's code, do you think it is ready to release 0.6.2(with this patch
pushed)?
I don't have 'lldptool' installed on my system, so I cannot verify the output or the fix.
What concerns me though is the reliance on something that could (and
does) change between versions. >What you have is just "two" versions of differences. What if there's a 3rd version that had/has >something different? Is the output different on different archs/OS's?
Is there no way to make the vsi_search_string be an array in the
configuration file? Rather than two >constant strings.
I suppose what you have works, but it seems there has to be a better way
to do this. I also think the >comments in the .conf file could be beefed up so someone would know what to look for. Is that a >specific field in the output or is that string what is returned. Essentially how would someone know >what to look for in order to add a new/different string.
Call this a "luke-warm" ACK. It works, but I think it's a shortcut.
John
In RH6.4 lldptool query command and output changed a bit,
...... _______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
On 04/11/2013 03:15 AM, WangXu wrote:
Dear John, What's your opinion about my solution? If no problem, we'll develop patch as described. ver. 0.6.2 has to be released as soon as possible:-)
Sincerely yours, Xu Wang
I didn't realize you were looking/waiting for feedback. I think a non static length or number of strings is certainly better than just two. It provides for far more flexibility. Whether you do that by a comma (or some other delimiter) separated list or something that has a count and set of strings to look for doesn't really matter to me. The only concern about doing so in a list is choosing your element separator. As soon as you choose a comma is as soon as a comma shows up in the next round of output. For example, vsi_support_key_string_count=2 vsi_support_key_string[0]="String #1" vsi_support_key_string[1]="String #2" Beyond that adding something to the .conf file to describe what a "customer" would need to run and where to look in order to find the desired string that would be added to the existing list of strings would be helpful. John
------------------------------------------------------------------------ From: cngesaint@outlook.com To: libvirt-cim@redhat.com Date: Wed, 10 Apr 2013 10:20:34 +0000 Subject: Re: [Libvirt-cim] [PATCH] SwitchService: make command configurable
I think the "vsi_search_string be an array" problem could be solved as following steps: 1. An item such as (vsi_support_key_string="{supported forwarding mode: (0x40) reflective relay, supported capabilities: (0x7) RTE ECP VDP}";) is configured well in the .conf file. 2. The application read this value from .conf file as a long string. 3. The application executes supporting function about VSI and check whether the output returned is a substring of vsi_support_key_string. If the output contains in the vsi_support_key_string, supporting of VSI is proved.
Sincerely yours, Xu Wang
Hi, John Sorry to interrupt but still need you to review this patch, which make it work more flex on different version. By the way, I got only 3 fail and they seems small problems in test suit's code, do you think it is ready to release 0.6.2(with this patch pushed)? I don't have 'lldptool' installed on my system, so I cannot verify
On 04/09/2013 06:40 AM, Wenchao Xia wrote: the output or the fix.
What concerns me though is the reliance on something that could (and does) change between versions. >What you have is just "two" versions of differences. What if there's a 3rd version that had/has >something different? Is the output different on different archs/OS's?
Is there no way to make the vsi_search_string be an array in the configuration file? Rather than two >constant strings.
I suppose what you have works, but it seems there has to be a better way to do this. I also think the >comments in the .conf file could be beefed up so someone would know what to look for. Is that a >specific field in the output or is that string what is returned. Essentially how would someone know >what to look for in order to add a new/different string.
Call this a "luke-warm" ACK. It works, but I think it's a shortcut.
John
In RH6.4 lldptool query command and output changed a bit, ......
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
participants (2)
-
John Ferlan -
WangXu