Re: [libvirt] [PATCH 15/40] Simplify the Xen domain get OS type driver method

2013-05-06 Thread Jim Fehlig 
Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com

 Make xenUnifiedDomainGetOSType directly call either the
 xenHypervisorDomainGetOSType or xenDaemonDomainGetOSType
 method depending on whether the domain is active or not.
   

Useful to add a note about removing the unused code in the xenstore
driver, as you did in the other patches.

 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/xen/xen_driver.c | 19 ++-
  src/xen/xen_driver.h |  1 -
  src/xen/xen_hypervisor.c |  5 +--
  src/xen/xend_internal.c  |  7 +---
  src/xen/xend_internal.h  |  2 ++
  src/xen/xs_internal.c| 85 
 
  6 files changed, 15 insertions(+), 104 deletions(-)
   

ACK.

Regards,
Jim

 diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
 index 7827d70..8ee3c4c 100644
 --- a/src/xen/xen_driver.c
 +++ b/src/xen/xen_driver.c
 @@ -782,18 +782,21 @@ static char *
  xenUnifiedDomainGetOSType(virDomainPtr dom)
  {
  xenUnifiedPrivatePtr priv = dom-conn-privateData;
 -int i;
 -char *ret;
  
 -for (i = 0; i  XEN_UNIFIED_NR_DRIVERS; ++i)
 -if (priv-opened[i]  drivers[i]-xenDomainGetOSType) {
 -ret = drivers[i]-xenDomainGetOSType(dom);
 -if (ret) return ret;
 +if (dom-id  0) {
 +if (priv-xendConfigVersion  XEND_CONFIG_VERSION_3_0_4) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(Unable to query OS type for inactive domain));
 +return NULL;
 +} else {
 +return xenDaemonDomainGetOSType(dom);
  }
 -
 -return NULL;
 +} else {
 +return xenHypervisorDomainGetOSType(dom);
 +}
  }
  
 +
  static unsigned long long
  xenUnifiedDomainGetMaxMemory(virDomainPtr dom)
  {
 diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
 index aff68f2..3ac2912 100644
 --- a/src/xen/xen_driver.h
 +++ b/src/xen/xen_driver.h
 @@ -94,7 +94,6 @@ extern int xenRegister (void);
   */
  struct xenUnifiedDriver {
  virDrvConnectGetHostname xenGetHostname;
 -virDrvDomainGetOSType xenDomainGetOSType;
  virDrvDomainGetMaxMemory xenDomainGetMaxMemory;
  virDrvDomainSetMaxMemory xenDomainSetMaxMemory;
  virDrvDomainSetMemory xenDomainSetMemory;
 diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
 index 244bdee..8636d52 100644
 --- a/src/xen/xen_hypervisor.c
 +++ b/src/xen/xen_hypervisor.c
 @@ -873,7 +873,6 @@ typedef struct xen_op_v2_dom xen_op_v2_dom;
  static unsigned long long xenHypervisorGetMaxMemory(virDomainPtr domain);
  
  struct xenUnifiedDriver xenHypervisorDriver = {
 -.xenDomainGetOSType = xenHypervisorDomainGetOSType,
  .xenDomainGetMaxMemory = xenHypervisorGetMaxMemory,
  .xenDomainSetMaxMemory = xenHypervisorSetMaxMemory,
  .xenDomainGetInfo = xenHypervisorGetDomainInfo,
 @@ -2613,9 +2612,7 @@ xenHypervisorDomainGetOSType(virDomainPtr dom)
  /* HV's earlier than 3.1.0 don't include the HVM flags in guests status*/
  if (hv_versions.hypervisor  2 ||
  hv_versions.dom_interface  4) {
 -virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 -   _(unsupported in dom interface  4));
 -return NULL;
 +return xenDaemonDomainGetOSType(dom);
  }
  
  XEN_GETDOMAININFO_CLEAR(dominfo);
 diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
 index c759636..75c1514 100644
 --- a/src/xen/xend_internal.c
 +++ b/src/xen/xend_internal.c
 @@ -1373,15 +1373,11 @@ xenDaemonDomainDestroy(virDomainPtr domain)
   * Returns the new string or NULL in case of error, the string must be
   * freed by the caller.
   */
 -static char *
 +char *
  xenDaemonDomainGetOSType(virDomainPtr domain)
  {
  char *type;
  struct sexpr *root;
 -xenUnifiedPrivatePtr priv = domain-conn-privateData;
 -
 -if (domain-id  0  priv-xendConfigVersion  
 XEND_CONFIG_VERSION_3_0_4)
 -return NULL;
  
  /* can we ask for a subset ? worth it ? */
  root = sexpr_get(domain-conn, /xend/domain/%s?detail=1, domain-name);
 @@ -3441,7 +3437,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
  }
  
  struct xenUnifiedDriver xenDaemonDriver = {
 -.xenDomainGetOSType = xenDaemonDomainGetOSType,
  .xenDomainGetMaxMemory = xenDaemonDomainGetMaxMemory,
  .xenDomainSetMaxMemory = xenDaemonDomainSetMaxMemory,
  .xenDomainSetMemory = xenDaemonDomainSetMemory,
 diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h
 index d393ec8..9681068 100644
 --- a/src/xen/xend_internal.h
 +++ b/src/xen/xend_internal.h
 @@ -108,6 +108,8 @@ char *xenDaemonDomainGetXMLDesc(virDomainPtr domain, 
 unsigned int flags,
  unsigned long long xenDaemonDomainGetMaxMemory(virDomainPtr domain);
  char **xenDaemonListDomainsOld(virConnectPtr xend);
  
 +char *xenDaemonDomainGetOSType(virDomainPtr domain);
 +
  virDomainPtr xenDaemonDomainDefineXML(virConnectPtr xend, const char *sexpr);
  int

Re: [libvirt] [PATCH 15/40] Simplify the Xen domain get OS type driver method

2013-05-06 Thread Jim Fehlig 
Jim Fehlig wrote:
 Daniel P. Berrange wrote:
   
 From: Daniel P. Berrange berra...@redhat.com

 Make xenUnifiedDomainGetOSType directly call either the
 xenHypervisorDomainGetOSType or xenDaemonDomainGetOSType
 method depending on whether the domain is active or not.
   
 

 Useful to add a note about removing the unused code in the xenstore
 driver, as you did in the other patches.

   
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/xen/xen_driver.c | 19 ++-
  src/xen/xen_driver.h |  1 -
  src/xen/xen_hypervisor.c |  5 +--
  src/xen/xend_internal.c  |  7 +---
  src/xen/xend_internal.h  |  2 ++
  src/xen/xs_internal.c| 85 
 
  6 files changed, 15 insertions(+), 104 deletions(-)
   
 

 ACK.
   

Forgot to mention one little nit below

   
 diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
 index 7827d70..8ee3c4c 100644
 --- a/src/xen/xen_driver.c
 +++ b/src/xen/xen_driver.c
 @@ -782,18 +782,21 @@ static char *
  xenUnifiedDomainGetOSType(virDomainPtr dom)
  {
  xenUnifiedPrivatePtr priv = dom-conn-privateData;
 -int i;
 -char *ret;
  
 -for (i = 0; i  XEN_UNIFIED_NR_DRIVERS; ++i)
 -if (priv-opened[i]  drivers[i]-xenDomainGetOSType) {
 -ret = drivers[i]-xenDomainGetOSType(dom);
 -if (ret) return ret;
 +if (dom-id  0) {
 +if (priv-xendConfigVersion  XEND_CONFIG_VERSION_3_0_4) {
 +virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 +   _(Unable to query OS type for inactive 
 domain));
 +return NULL;
 +} else {
 +return xenDaemonDomainGetOSType(dom);
  }
 -
 -return NULL;
 +} else {
 +return xenHypervisorDomainGetOSType(dom);
 +}
  }
  
 +
 

Spurious whitespace.

Regards,
Jim

  static unsigned long long
  xenUnifiedDomainGetMaxMemory(virDomainPtr dom)
  {
 diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
 index aff68f2..3ac2912 100644
 --- a/src/xen/xen_driver.h
 +++ b/src/xen/xen_driver.h
 @@ -94,7 +94,6 @@ extern int xenRegister (void);
   */
  struct xenUnifiedDriver {
  virDrvConnectGetHostname xenGetHostname;
 -virDrvDomainGetOSType xenDomainGetOSType;
  virDrvDomainGetMaxMemory xenDomainGetMaxMemory;
  virDrvDomainSetMaxMemory xenDomainSetMaxMemory;
  virDrvDomainSetMemory xenDomainSetMemory;
 diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
 index 244bdee..8636d52 100644
 --- a/src/xen/xen_hypervisor.c
 +++ b/src/xen/xen_hypervisor.c
 @@ -873,7 +873,6 @@ typedef struct xen_op_v2_dom xen_op_v2_dom;
  static unsigned long long xenHypervisorGetMaxMemory(virDomainPtr domain);
  
  struct xenUnifiedDriver xenHypervisorDriver = {
 -.xenDomainGetOSType = xenHypervisorDomainGetOSType,
  .xenDomainGetMaxMemory = xenHypervisorGetMaxMemory,
  .xenDomainSetMaxMemory = xenHypervisorSetMaxMemory,
  .xenDomainGetInfo = xenHypervisorGetDomainInfo,
 @@ -2613,9 +2612,7 @@ xenHypervisorDomainGetOSType(virDomainPtr dom)
  /* HV's earlier than 3.1.0 don't include the HVM flags in guests 
 status*/
  if (hv_versions.hypervisor  2 ||
  hv_versions.dom_interface  4) {
 -virReportError(VIR_ERR_INTERNAL_ERROR, %s,
 -   _(unsupported in dom interface  4));
 -return NULL;
 +return xenDaemonDomainGetOSType(dom);
  }
  
  XEN_GETDOMAININFO_CLEAR(dominfo);
 diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
 index c759636..75c1514 100644
 --- a/src/xen/xend_internal.c
 +++ b/src/xen/xend_internal.c
 @@ -1373,15 +1373,11 @@ xenDaemonDomainDestroy(virDomainPtr domain)
   * Returns the new string or NULL in case of error, the string must be
   * freed by the caller.
   */
 -static char *
 +char *
  xenDaemonDomainGetOSType(virDomainPtr domain)
  {
  char *type;
  struct sexpr *root;
 -xenUnifiedPrivatePtr priv = domain-conn-privateData;
 -
 -if (domain-id  0  priv-xendConfigVersion  
 XEND_CONFIG_VERSION_3_0_4)
 -return NULL;
  
  /* can we ask for a subset ? worth it ? */
  root = sexpr_get(domain-conn, /xend/domain/%s?detail=1, 
 domain-name);
 @@ -3441,7 +3437,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
  }
  
  struct xenUnifiedDriver xenDaemonDriver = {
 -.xenDomainGetOSType = xenDaemonDomainGetOSType,
  .xenDomainGetMaxMemory = xenDaemonDomainGetMaxMemory,
  .xenDomainSetMaxMemory = xenDaemonDomainSetMaxMemory,
  .xenDomainSetMemory = xenDaemonDomainSetMemory,
 diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h
 index d393ec8..9681068 100644
 --- a/src/xen/xend_internal.h
 +++ b/src/xen/xend_internal.h
 @@ -108,6 +108,8 @@ char *xenDaemonDomainGetXMLDesc(virDomainPtr domain, 
 unsigned int flags,
  unsigned long long xenDaemonDomainGetMaxMemory(virDomainPtr domain);
  char **xenDaemonListDomainsOld(virConnectPtr xend);
  
 +char

[libvirt] [PATCH 15/40] Simplify the Xen domain get OS type driver method

2013-05-02 Thread Daniel P. Berrange 
From: Daniel P. Berrange berra...@redhat.com

Make xenUnifiedDomainGetOSType directly call either the
xenHypervisorDomainGetOSType or xenDaemonDomainGetOSType
method depending on whether the domain is active or not.

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/xen/xen_driver.c | 19 ++-
 src/xen/xen_driver.h |  1 -
 src/xen/xen_hypervisor.c |  5 +--
 src/xen/xend_internal.c  |  7 +---
 src/xen/xend_internal.h  |  2 ++
 src/xen/xs_internal.c| 85 
 6 files changed, 15 insertions(+), 104 deletions(-)

diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 7827d70..8ee3c4c 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -782,18 +782,21 @@ static char *
 xenUnifiedDomainGetOSType(virDomainPtr dom)
 {
 xenUnifiedPrivatePtr priv = dom-conn-privateData;
-int i;
-char *ret;
 
-for (i = 0; i  XEN_UNIFIED_NR_DRIVERS; ++i)
-if (priv-opened[i]  drivers[i]-xenDomainGetOSType) {
-ret = drivers[i]-xenDomainGetOSType(dom);
-if (ret) return ret;
+if (dom-id  0) {
+if (priv-xendConfigVersion  XEND_CONFIG_VERSION_3_0_4) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Unable to query OS type for inactive domain));
+return NULL;
+} else {
+return xenDaemonDomainGetOSType(dom);
 }
-
-return NULL;
+} else {
+return xenHypervisorDomainGetOSType(dom);
+}
 }
 
+
 static unsigned long long
 xenUnifiedDomainGetMaxMemory(virDomainPtr dom)
 {
diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
index aff68f2..3ac2912 100644
--- a/src/xen/xen_driver.h
+++ b/src/xen/xen_driver.h
@@ -94,7 +94,6 @@ extern int xenRegister (void);
  */
 struct xenUnifiedDriver {
 virDrvConnectGetHostname xenGetHostname;
-virDrvDomainGetOSType xenDomainGetOSType;
 virDrvDomainGetMaxMemory xenDomainGetMaxMemory;
 virDrvDomainSetMaxMemory xenDomainSetMaxMemory;
 virDrvDomainSetMemory xenDomainSetMemory;
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index 244bdee..8636d52 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -873,7 +873,6 @@ typedef struct xen_op_v2_dom xen_op_v2_dom;
 static unsigned long long xenHypervisorGetMaxMemory(virDomainPtr domain);
 
 struct xenUnifiedDriver xenHypervisorDriver = {
-.xenDomainGetOSType = xenHypervisorDomainGetOSType,
 .xenDomainGetMaxMemory = xenHypervisorGetMaxMemory,
 .xenDomainSetMaxMemory = xenHypervisorSetMaxMemory,
 .xenDomainGetInfo = xenHypervisorGetDomainInfo,
@@ -2613,9 +2612,7 @@ xenHypervisorDomainGetOSType(virDomainPtr dom)
 /* HV's earlier than 3.1.0 don't include the HVM flags in guests status*/
 if (hv_versions.hypervisor  2 ||
 hv_versions.dom_interface  4) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
-   _(unsupported in dom interface  4));
-return NULL;
+return xenDaemonDomainGetOSType(dom);
 }
 
 XEN_GETDOMAININFO_CLEAR(dominfo);
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index c759636..75c1514 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -1373,15 +1373,11 @@ xenDaemonDomainDestroy(virDomainPtr domain)
  * Returns the new string or NULL in case of error, the string must be
  * freed by the caller.
  */
-static char *
+char *
 xenDaemonDomainGetOSType(virDomainPtr domain)
 {
 char *type;
 struct sexpr *root;
-xenUnifiedPrivatePtr priv = domain-conn-privateData;
-
-if (domain-id  0  priv-xendConfigVersion  XEND_CONFIG_VERSION_3_0_4)
-return NULL;
 
 /* can we ask for a subset ? worth it ? */
 root = sexpr_get(domain-conn, /xend/domain/%s?detail=1, domain-name);
@@ -3441,7 +3437,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain,
 }
 
 struct xenUnifiedDriver xenDaemonDriver = {
-.xenDomainGetOSType = xenDaemonDomainGetOSType,
 .xenDomainGetMaxMemory = xenDaemonDomainGetMaxMemory,
 .xenDomainSetMaxMemory = xenDaemonDomainSetMaxMemory,
 .xenDomainSetMemory = xenDaemonDomainSetMemory,
diff --git a/src/xen/xend_internal.h b/src/xen/xend_internal.h
index d393ec8..9681068 100644
--- a/src/xen/xend_internal.h
+++ b/src/xen/xend_internal.h
@@ -108,6 +108,8 @@ char *xenDaemonDomainGetXMLDesc(virDomainPtr domain, 
unsigned int flags,
 unsigned long long xenDaemonDomainGetMaxMemory(virDomainPtr domain);
 char **xenDaemonListDomainsOld(virConnectPtr xend);
 
+char *xenDaemonDomainGetOSType(virDomainPtr domain);
+
 virDomainPtr xenDaemonDomainDefineXML(virConnectPtr xend, const char *sexpr);
 int xenDaemonDomainCreate(virDomainPtr domain);
 int xenDaemonDomainUndefine(virDomainPtr domain);
diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c
index a7a8d15..40d0be2 100644
--- a/src/xen/xs_internal.c
+++ b/src/xen/xs_internal.c
@@ -53,12 +53,10 @@
 
 #define VIR_FROM_THIS VIR_FROM_XEN
 
-static char