Re: [PATCH] cpu-driver: Fix the cross driver function call

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi again,

there were no replies or comments on my patch since nearly 2 weeks.

Please review this patch that is a preparation step to exploit the new QEMU CPU model support.

Thanks in advance.

Kind regards,
Daniel Hansel

On 20.11.2014 11:08, Daniel Hansel wrote:
> For Intel and PowerPC the implementation is calling a cpu driver
> function across driver layers (i.e. from qemu driver directly to cpu
> driver).
> The correct behavior is to use libvirt API functionality to perform such
> a inter-driver call.
> 
> This patch introduces a new cpu driver API function getModels() to
> retrieve the cpu models. The currect implementation to process the
> cpu_map XML content is transferred to the INTEL and PowerPC cpu driver
> specific API functions.
> Additionally processing the cpu_map XML file is not safe due to the fact
> that the cpu map does not exist for all architectures. Therefore it is
> better to encapsulate the processing in the architecture specific cpu
> drivers.
> 
> Signed-off-by: Daniel Hansel <daniel.hansel@xxxxxxxxxxxxxxxxxx>
> Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx>
> Reviewed-by: Viktor Mihajlovski <mihajlov@xxxxxxxxxxxxxxxxxx>
> ---
>  src/cpu/cpu.c         | 68 +++++++++------------------------------------------
>  src/cpu/cpu.h         |  4 +++
>  src/cpu/cpu_powerpc.c | 37 ++++++++++++++++++++++++++++
>  src/cpu/cpu_x86.c     | 33 +++++++++++++++++++++++++
>  4 files changed, 86 insertions(+), 56 deletions(-)
> 
> diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
> index 08bec5e..788f688 100644
> --- a/src/cpu/cpu.c
> +++ b/src/cpu/cpu.c
> @@ -724,43 +724,6 @@ cpuModelIsAllowed(const char *model,
>      return false;
>  }
> 
> -struct cpuGetModelsData
> -{
> -    char **data;
> -    size_t len;  /* It includes the last element of DATA, which is NULL. */
> -};
> -
> -static int
> -cpuGetArchModelsCb(cpuMapElement element,
> -                   xmlXPathContextPtr ctxt,
> -                   void *cbdata)
> -{
> -    char *name;
> -    struct cpuGetModelsData *data = cbdata;
> -    if (element != CPU_MAP_ELEMENT_MODEL)
> -        return 0;
> -
> -    name = virXPathString("string(@name)", ctxt);
> -    if (name == NULL)
> -        return -1;
> -
> -    if (!data->data) {
> -        VIR_FREE(name);
> -        data->len++;
> -        return 0;
> -    }
> -
> -    return VIR_INSERT_ELEMENT(data->data, data->len - 1, data->len, name);
> -}
> -
> -
> -static int
> -cpuGetArchModels(const char *arch, struct cpuGetModelsData *data)
> -{
> -    return cpuMapLoad(arch, cpuGetArchModelsCb, data);
> -}
> -
> -
>  /**
>   * cpuGetModels:
>   *
> @@ -774,18 +737,17 @@ cpuGetArchModels(const char *arch, struct cpuGetModelsData *data)
>  int
>  cpuGetModels(const char *archName, char ***models)
>  {
> -    struct cpuGetModelsData data;
> -    virArch arch;
>      struct cpuArchDriver *driver;
> -    data.data = NULL;
> -    data.len = 1;
> +    virArch arch;
> +
> +    VIR_DEBUG("arch=%s", archName);
> 
>      arch = virArchFromString(archName);
>      if (arch == VIR_ARCH_NONE) {
>          virReportError(VIR_ERR_INVALID_ARG,
>                         _("cannot find architecture %s"),
>                         archName);
> -        goto error;
> +        return -1;
>      }
> 
>      driver = cpuGetSubDriver(arch);
> @@ -793,21 +755,15 @@ cpuGetModels(const char *archName, char ***models)
>          virReportError(VIR_ERR_INVALID_ARG,
>                         _("cannot find a driver for the architecture %s"),
>                         archName);
> -        goto error;
> +        return -1;
>      }
> 
> -    if (models && VIR_ALLOC_N(data.data, data.len) < 0)
> -        goto error;
> -
> -    if (cpuGetArchModels(driver->name, &data) < 0)
> -        goto error;
> -
> -    if (models)
> -        *models = data.data;
> -
> -    return data.len - 1;
> +    if (!driver->getModels) {
> +        virReportError(VIR_ERR_NO_SUPPORT,
> +                       _("CPU driver for %s has no CPU model support"),
> +                       virArchToString(arch));
> +        return -1;
> +    }
> 
> - error:
> -    virStringFreeList(data.data);
> -    return -1;
> +    return driver->getModels(models);
>  }
> diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
> index 339964c..09e9538 100644
> --- a/src/cpu/cpu.h
> +++ b/src/cpu/cpu.h
> @@ -100,6 +100,9 @@ typedef char *
>  typedef virCPUDataPtr
>  (*cpuArchDataParse) (const char *xmlStr);
> 
> +typedef int
> +(*cpuArchGetModels) (char ***models);
> +
>  struct cpuArchDriver {
>      const char *name;
>      const virArch *arch;
> @@ -115,6 +118,7 @@ struct cpuArchDriver {
>      cpuArchHasFeature    hasFeature;
>      cpuArchDataFormat   dataFormat;
>      cpuArchDataParse    dataParse;
> +    cpuArchGetModels    getModels;
>  };
> 
> 
> diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c
> index 67cb9ff..155d93e 100644
> --- a/src/cpu/cpu_powerpc.c
> +++ b/src/cpu/cpu_powerpc.c
> @@ -649,6 +649,42 @@ ppcBaseline(virCPUDefPtr *cpus,
>      goto cleanup;
>  }
> 
> +static int
> +ppcGetModels(char ***models)
> +{
> +    struct ppc_map *map;
> +    struct ppc_model *model;
> +    char *name;
> +    size_t nmodels = 0;
> +
> +    if (!(map = ppcLoadMap()))
> +        goto error;
> +
> +    if (models && VIR_ALLOC_N(*models, 0) < 0)
> +        goto error;
> +
> +    model = map->models;
> +    while (model != NULL) {
> +        if (VIR_STRDUP(name, model->name) < 0)
> +            goto error;
> +
> +        if (VIR_INSERT_ELEMENT(*models, 0, nmodels, name) < 0)
> +            goto error;
> +
> +        model = model->next;
> +    }
> +
> + cleanup:
> +    ppcMapFree(map);
> +
> +    return nmodels;
> +
> + error:
> +    virStringFreeList(*models);
> +    nmodels = -1;
> +    goto cleanup;
> +}
> +
>  struct cpuArchDriver cpuDriverPowerPC = {
>      .name = "ppc64",
>      .arch = archs,
> @@ -662,4 +698,5 @@ struct cpuArchDriver cpuDriverPowerPC = {
>      .baseline   = ppcBaseline,
>      .update     = ppcUpdate,
>      .hasFeature = NULL,
> +    .getModels  = ppcGetModels,
>  };
> diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
> index 026b54e..f6dcba4 100644
> --- a/src/cpu/cpu_x86.c
> +++ b/src/cpu/cpu_x86.c
> @@ -2160,6 +2160,38 @@ x86HasFeature(const virCPUData *data,
>      return ret;
>  }
> 
> +static int
> +x86GetModels(char ***models)
> +{
> +    const struct x86_map *map;
> +    struct x86_model *model;
> +    char *name;
> +    size_t nmodels = 0;
> +
> +    if (!(map = virCPUx86GetMap()))
> +        return -1;
> +
> +    if (models && VIR_ALLOC_N(*models, 0) < 0)
> +        goto error;
> +
> +    model = map->models;
> +    while (model != NULL) {
> +        if (VIR_STRDUP(name, model->name) < 0)
> +            goto error;
> +
> +        if (VIR_INSERT_ELEMENT(*models, 0, nmodels, name) < 0)
> +            goto error;
> +
> +        model = model->next;
> +    }
> +
> +    return nmodels;
> +
> + error:
> +    virStringFreeList(*models);
> +    return -1;
> +}
> +
> 
>  struct cpuArchDriver cpuDriverX86 = {
>      .name = "x86",
> @@ -2180,4 +2212,5 @@ struct cpuArchDriver cpuDriverX86 = {
>      .hasFeature = x86HasFeature,
>      .dataFormat = x86CPUDataFormat,
>      .dataParse  = x86CPUDataParse,
> +    .getModels  = x86GetModels,
>  };
> 

-- 

Mit freundlichen Grüßen / Kind regards
Daniel Hansel

IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]