12:31 p.m.
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>
---
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;
--
1.7.1
12:40 p.m.
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>
---
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
12:54 p.m.
On Tue, Apr 09, 2013 at 06:40:34PM +0800, 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)?
didn't we want to have the testsuite patches pushed too before the
release ?
Daniel
> 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>
> ---
> 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
--
Daniel Veillard | Open Source and Standards, Red Hat
veillard(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
6:55 a.m.
于 2013-4-9 18:54, Daniel Veillard 写道:
On Tue, Apr 09, 2013 at 06:40:34PM +0800, 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)?
didn't we want to have the testsuite patches pushed too before the
release ?
Daniel
I think Cimtest still have some problems and patches need more
review, but works now,
maybe we can release libvirt-cim first getting it ready for yum
repositroy first
We can improve it later, there will be a new guys working for it.
6:06 p.m.
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,
> 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>
> ---
> 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;
>
10:29 a.m.
于 2013-4-10 0:06, John Ferlan 写道:
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?
I think it is
changed by lldptool itself, instead of OS, my commit
message is wrong. So to make it work better, it need to be configurababl
according to lldptool's behavior.
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.
Yep, I agree configuration file should tip more.
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
--
Best Regards
Wenchao Xia
4181
days inactive
4182
days old
5 comments
3 participants
participants (3)
-
Daniel Veillard -
John Ferlan -
Wenchao Xia