[libvirt] [PATCH 0/3] Replace vmware/esx argv string to VMX_CONFIG_FORMAT_ARGV

Han Han (3): vmx: Define macro VMX_CONFIG_FORMAT_ARGV for vmware-vmx esx: Use VMX_CONFIG_FORMAT_ARGV for esx naive argv vmware: Use VMX_CONFIG_FORMAT_ARGV for vmware naive argv src/esx/esx_driver.c | 4 ++-- src/vmware/vmware_conf.c | 2 +- src/vmware/vmware_driver.c | 2 +- src/vmx/vmx.h | 2 ++ 4 files changed, 6 insertions(+), 4 deletions(-) -- 2.20.1

Signed-off-by: Han Han <hhan@redhat.com> --- src/vmx/vmx.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/vmx/vmx.h b/src/vmx/vmx.h index cb99e84d18..8c068b4cb2 100644 --- a/src/vmx/vmx.h +++ b/src/vmx/vmx.h @@ -26,6 +26,8 @@ # include "virconf.h" # include "domain_conf.h" +# define VMX_CONFIG_FORMAT_ARGV "vmware-vmx" + typedef struct _virVMXContext virVMXContext; virDomainXMLOptionPtr virVMXDomainXMLConfInit(void); -- 2.20.1

Signed-off-by: Han Han <hhan@redhat.com> --- src/esx/esx_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 08d33b6f3b..d80fef0a58 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2710,7 +2710,7 @@ esxConnectDomainXMLFromNative(virConnectPtr conn, const char *nativeFormat, memset(&data, 0, sizeof(data)); - if (STRNEQ(nativeFormat, "vmware-vmx")) { + if (STRNEQ(nativeFormat, VMX_CONFIG_FORMAT_ARGV)) { virReportError(VIR_ERR_INVALID_ARG, _("Unsupported config format '%s'"), nativeFormat); return NULL; @@ -2755,7 +2755,7 @@ esxConnectDomainXMLToNative(virConnectPtr conn, const char *nativeFormat, memset(&data, 0, sizeof(data)); - if (STRNEQ(nativeFormat, "vmware-vmx")) { + if (STRNEQ(nativeFormat, VMX_CONFIG_FORMAT_ARGV)) { virReportError(VIR_ERR_INVALID_ARG, _("Unsupported config format '%s'"), nativeFormat); return NULL; -- 2.20.1

Signed-off-by: Han Han <hhan@redhat.com> --- src/vmware/vmware_conf.c | 2 +- src/vmware/vmware_driver.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index 0c1b1f9550..76645a2e0d 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -285,7 +285,7 @@ vmwareExtractVersion(struct vmware_driver *driver) break; case VMWARE_DRIVER_FUSION: - if (virAsprintf(&bin, "%s/%s", vmwarePath, "vmware-vmx") < 0) + if (virAsprintf(&bin, "%s/%s", vmwarePath, VMX_CONFIG_FORMAT_ARGV) < 0) goto cleanup; break; diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index f4b0989afd..1bc8a06c39 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -956,7 +956,7 @@ vmwareConnectDomainXMLFromNative(virConnectPtr conn, const char *nativeFormat, virCheckFlags(0, NULL); - if (STRNEQ(nativeFormat, "vmware-vmx")) { + if (STRNEQ(nativeFormat, VMX_CONFIG_FORMAT_ARGV)) { virReportError(VIR_ERR_INVALID_ARG, _("Unsupported config format '%s'"), nativeFormat); return NULL; -- 2.20.1

*native in the subject On 3/28/19 1:57 AM, Han Han wrote:
Signed-off-by: Han Han <hhan@redhat.com> --- src/vmware/vmware_conf.c | 2 +- src/vmware/vmware_driver.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index 0c1b1f9550..76645a2e0d 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -285,7 +285,7 @@ vmwareExtractVersion(struct vmware_driver *driver) break;
case VMWARE_DRIVER_FUSION: - if (virAsprintf(&bin, "%s/%s", vmwarePath, "vmware-vmx") < 0) + if (virAsprintf(&bin, "%s/%s", vmwarePath, VMX_CONFIG_FORMAT_ARGV) < 0) goto cleanup; break;
This usage is building a binary path like /usr/bin/vmware-vmx, so despite the strings being the same it's not the same type of usage. It shouldn't be replaced IMO Otherwise this series change makes sense, it's the same pattern used in other drivers. I suggest squashing it down to one patch though and send a v2 Thanks, Cole

On Thu, Apr 4, 2019 at 5:16 AM Cole Robinson <crobinso@redhat.com> wrote:
*native in the subject
Signed-off-by: Han Han <hhan@redhat.com> --- src/vmware/vmware_conf.c | 2 +- src/vmware/vmware_driver.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index 0c1b1f9550..76645a2e0d 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -285,7 +285,7 @@ vmwareExtractVersion(struct vmware_driver *driver) break;
case VMWARE_DRIVER_FUSION: - if (virAsprintf(&bin, "%s/%s", vmwarePath, "vmware-vmx") <
On 3/28/19 1:57 AM, Han Han wrote: 0)
+ if (virAsprintf(&bin, "%s/%s", vmwarePath, VMX_CONFIG_FORMAT_ARGV) < 0) goto cleanup; break;
This usage is building a binary path like /usr/bin/vmware-vmx, so despite the strings being the same it's not the same type of usage. It shouldn't be replaced IMO
Otherwise this series change makes sense, it's the same pattern used in other drivers. I suggest squashing it down to one patch though and send a v2
Thanks for review. v2 link: https://www.redhat.com/archives/libvir-list/2019-April/msg00609.html
Thanks, Cole
-- Best regards, ----------------------------------- Han Han Quality Engineer Redhat. Email: hhan@redhat.com Phone: +861065339333
participants (2)
-
Cole Robinson
-
Han Han