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

Daniel P. Berrange wrote: > From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; > > 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 <berrange(a)redhat.com&gt; > --- > 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.

> 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 *xenStoreDomainGetOSType(virDomainPtr domain); > static void xenStoreWatchEvent(int watch, int fd, int events, void *data); > static void xenStoreWatchListFree(xenStoreWatchListPtr list); > > struct xenUnifiedDriver xenStoreDriver = { > - .xenDomainGetOSType = xenStoreDomainGetOSType, > .xenDomainGetMaxMemory = xenStoreDomainGetMaxMemory, > .xenDomainSetMemory = xenStoreDomainSetMemory, > .xenDomainGetInfo = xenStoreGetDomainInfo, > @@ -142,63 +140,6 @@ virDomainDoStoreWrite(virDomainPtr domain, const char *path, const char *value) > return ret; > } > > -/** > - * virDomainGetVM: > - * @domain: a domain object > - * > - * Internal API extracting a xenstore vm path. > - * > - * Returns the new string or NULL in case of error > - */ > -static char * > -virDomainGetVM(virDomainPtr domain) > -{ > - char *vm; > - char query[200]; > - unsigned int len; > - xenUnifiedPrivatePtr priv = domain->conn->privateData; > - > - if (priv->xshandle == NULL) > - return NULL; > - > - snprintf(query, 199, "/local/domain/%d/vm", virDomainGetID(domain)); > - query[199] = 0; > - > - vm = xs_read(priv->xshandle, 0, &query[0], &len); > - > - return vm; > -} > - > -/** > - * virDomainGetVMInfo: > - * @domain: a domain object > - * @vm: the xenstore vm path > - * @name: the value's path > - * > - * Internal API extracting one information the device used > - * by the domain from xensttore > - * > - * Returns the new string or NULL in case of error > - */ > -static char * > -virDomainGetVMInfo(virDomainPtr domain, const char *vm, const char *name) > -{ > - char s[256]; > - char *ret = NULL; > - unsigned int len = 0; > - xenUnifiedPrivatePtr priv = domain->conn->privateData; > - > - if (priv->xshandle == NULL) > - return NULL; > - > - snprintf(s, 255, "%s/%s", vm, name); > - s[255] = 0; > - > - ret = xs_read(priv->xshandle, 0, &s[0], &len); > - > - return ret; > -} > - > > /************************************************************************ > * * > @@ -579,32 +520,6 @@ xenStoreListDomains(virConnectPtr conn, int *ids, int maxids) > } > > > -/* > - * xenStoreDomainGetOSType: > - * @domain: a domain object > - * > - * Get the type of domain operation system. > - * > - * Returns the new string or NULL in case of error, the string must be > - * freed by the caller. > - */ > -static char * > -xenStoreDomainGetOSType(virDomainPtr domain) > -{ > - char *vm, *str = NULL; > - > - vm = virDomainGetVM(domain); > - if (vm) { > - xenUnifiedPrivatePtr priv = domain->conn->privateData; > - xenUnifiedLock(priv); > - str = virDomainGetVMInfo(domain, vm, "image/ostype"); > - xenUnifiedUnlock(priv); > - VIR_FREE(vm); > - } > - > - return str; > -} > - > /** > * xenStoreDomainGetVNCPort: > * @conn: the hypervisor connection > > -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list