Jim Fehlig wrote:
> Daniel P. Berrange wrote:
>
>> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>> ---
>> 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 *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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list