Re: [Libvirt-cim] [PATCH] SwitchService: make command configurable

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

> > _______________________________________________ Libvirt-cim mailing list Libvirt-cim(a)redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim