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

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)?

> In RH6.4 lldptool query command and output changed a bit, > to make it work on different version, the string used in search and > options used in lldptool command should be set in configuration file. > > Signed-off-by: Wenchao Xia <xiawenc(a)linux.vnet.ibm.com&gt; > --- > libvirt-cim.conf | 12 ++++++++++++ > libxkutil/misc_util.c | 27 +++++++++++++++++++++++++++ > libxkutil/misc_util.h | 4 +++- > src/Virt_SwitchService.c | 31 ++++++++++++++++++++++++------- > 4 files changed, 66 insertions(+), 8 deletions(-) > > diff --git a/libvirt-cim.conf b/libvirt-cim.conf > index 37d7b0f..40fc94a 100644 > --- a/libvirt-cim.conf > +++ b/libvirt-cim.conf > @@ -30,3 +30,15 @@ > # Default value: NULL, that is not set. > # > # migrate_ssh_temp_key = "/root/vm_migrate_tmp_id_rsa"; > + > +# lldptool_query_options (string) > +# Defines the command used in SwitchService to query VEPA support, will be > +# used as "lldptool -i [INTERFACE] [OPTIONS]" > +# > +# lldptool_query_options = "-t -g ncb -V evbcfg"; > + > +# vsi_search_string (string) > +# Defines the string used in SwitchService to search in lldptool's output > +# > +# vsi_search_string0 = "supported forwarding mode: (0x40) reflective relay"; > +# vsi_search_string1 = "supported capabilities: (0x7) RTE ECP VDP"; > diff --git a/libxkutil/misc_util.c b/libxkutil/misc_util.c > index 00eb4b1..1a9a1b4 100644 > --- a/libxkutil/misc_util.c > +++ b/libxkutil/misc_util.c > @@ -236,6 +236,33 @@ const char *get_mig_ssh_tmp_key(void) > return prop.value_string; > } > > +const char *get_lldptool_query_options(void) > +{ > + static LibvirtcimConfigProperty prop = { > + "lldptool_query_options", CONFIG_STRING, {0}, 0}; > + > + libvirt_cim_config_get(&prop); > + return prop.value_string; > +} > + > +const char *get_vsi_search_string0(void) > +{ > + static LibvirtcimConfigProperty prop = { > + "vsi_search_string0", CONFIG_STRING, {0}, 0}; > + > + libvirt_cim_config_get(&prop); > + return prop.value_string; > +} > + > +const char *get_vsi_search_string1(void) > +{ > + static LibvirtcimConfigProperty prop = { > + "vsi_search_string1", CONFIG_STRING, {0}, 0}; > + > + libvirt_cim_config_get(&prop); > + return prop.value_string; > +} > + > virConnectPtr connect_by_classname(const CMPIBroker *broker, > const char *classname, > CMPIStatus *s) > diff --git a/libxkutil/misc_util.h b/libxkutil/misc_util.h > index 0f52290..06a37cc 100644 > --- a/libxkutil/misc_util.h > +++ b/libxkutil/misc_util.h > @@ -154,7 +154,9 @@ int virt_set_status(const CMPIBroker *broker, > > /* get libvirt-cim config */ > const char *get_mig_ssh_tmp_key(void); > - > +const char *get_lldptool_query_options(void); > +const char *get_vsi_search_string0(void); > +const char *get_vsi_search_string1(void); > /* > * Local Variables: > * mode: C > diff --git a/src/Virt_SwitchService.c b/src/Virt_SwitchService.c > index 8991426..103f125 100644 > --- a/src/Virt_SwitchService.c > +++ b/src/Virt_SwitchService.c > @@ -46,13 +46,24 @@ static CMPIStatus check_vsi_support(char *command) > CMPIStatus s = {CMPI_RC_OK, NULL}; > char buff[MAX_LEN]; > FILE *stream = NULL; > - const char *searchStr[] = {" supported forwarding mode: " > - "(0x40) reflective relay", > - " supported capabilities: " > - "(0x07) RTE ECP VDP", > - NULL}; > + const char *string0 = get_vsi_search_string0(); > + const char *string1 = get_vsi_search_string1(); > + const char *searchStr[3]; > int matched = 0; > > + if (!string0) { > + string0 = " supported forwarding mode: " > + "(0x40) reflective relay"; > + } > + if (!string1) { > + string1 = " supported capabilities: " > + "(0x7) RTE ECP VDP"; > + } > + CU_DEBUG("vsi searching for string [%s] [%s]", string0, string1); > + searchStr[0] = string0; > + searchStr[1] = string1; > + searchStr[2] = NULL; > + > // Run lldptool command to find vsi support. > stream = popen(command, "r"); > if (stream == NULL) { > @@ -214,6 +225,7 @@ static CMPIStatus get_switchservice(const CMPIObjectPath *reference, > int i; > char **if_list; > char cmd[MAX_LEN]; > + const char *lldptool_query_options = NULL; > > *_inst = NULL; > conn = connect_by_classname(broker, CLASSNAME(reference), &s); > @@ -257,10 +269,15 @@ static CMPIStatus get_switchservice(const CMPIObjectPath *reference, > > CU_DEBUG("Found %d interfaces", count); > > + lldptool_query_options = get_lldptool_query_options(); > + if (!lldptool_query_options) { > + lldptool_query_options = "-t -g ncb -V evbcfg"; > + } > > for (i=0; i<count; i++) { > - sprintf(cmd, "lldptool -i %s -t -V evbcfg", if_list[i]); > - CU_DEBUG("running command %s ...", cmd); > + sprintf(cmd, "lldptool -i %s %s", > + if_list[i], lldptool_query_options); > + CU_DEBUG("running command [%s]", cmd); > s = check_vsi_support(cmd); > if (s.rc == CMPI_RC_OK) { > vsi = true; > -- Best Regards Wenchao Xia