date:20160830

[libvirt] [PATCH] util: hostcpu: improve CPU freq code for FreeBSD

2016-08-30 Thread Roman Bogorodskiy

Current implementation uses the dev.cpu.0.freq sysctl that is
provided by the cpufreq(4) framework and returns the actual
CPU frequency. However, there are environment where it's not available,
e.g. when running nested in KVM. In this case fall back to hw.clockrate
that reports CPU frequency at the boot time.

Resolves (hopefully):
https://bugzilla.redhat.com/show_bug.cgi?id=1369964
---
 src/util/virhostcpu.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
index 0f03ff8..b5a37a8 100644
--- a/src/util/virhostcpu.c
+++ b/src/util/virhostcpu.c
@@ -994,9 +994,16 @@ virHostCPUGetInfo(virArch hostarch ATTRIBUTE_UNUSED,
 *threads = 1;
 
 # ifdef __FreeBSD__
+/* dev.cpu.%d.freq reports current active CPU frequency. It is provided by
+ * the cpufreq(4) framework. However, it might be disabled or no driver
+ * available. In this case fallback to "hw.clockrate" which reports boot 
time
+ * CPU frequency. */
+
 if (sysctlbyname("dev.cpu.0.freq", _freq, _freq_len, NULL, 0) < 0) 
{
-virReportSystemError(errno, "%s", _("cannot obtain CPU freq"));
-return -1;
+if (sysctlbyname("hw.clockrate", _freq, _freq_len, NULL, 0) < 
0) {
+virReportSystemError(errno, "%s", _("cannot obtain CPU freq"));
+return -1;
+}
 }
 
 *mhz = cpu_freq;
-- 
2.7.4

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

Re: [libvirt] [PATCH 41/41] Move CMT feature filtering to QEMU driver

2016-08-30 Thread John Ferlan



On 08/12/2016 09:33 AM, Jiri Denemark wrote:
> It really doesn't belong to the generic CPU driver.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/cpu/cpu_x86.c| 16 ++--
>  src/qemu/qemu_capabilities.c | 16 +++-
>  2 files changed, 17 insertions(+), 15 deletions(-)
> 

Hmm... interesting is this something that the online perf add more stats
will need to also adjust, see (8/8):

http://www.redhat.com/archives/libvir-list/2016-August/msg00209.html

It doesn't seem so, but since I recognized the acronyms I figured I'd
check...

So here we are again at a summary - if I didn't comment on something
consider it an implicit ACK.

There's a couple of reviews that are simple and ACK'able - I think
they're obvious.

However, there's also a couple where I'm just looking for information. I
have no reason to not ACK, just wanted some clarity. I don't necessarily
need to see a whole new series.  I think it just the interaction noted
in patch 40, 35, and 26 (update and compare callbacks).

John


> diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
> index e0987c4..237fa40 100644
> --- a/src/cpu/cpu_x86.c
> +++ b/src/cpu/cpu_x86.c
> @@ -2558,22 +2558,10 @@ x86UpdateHostModel(virCPUDefPtr guest,
>  /* Remove non-migratable features by default */
>  updated->type = VIR_CPU_TYPE_GUEST;
>  updated->mode = VIR_CPU_MODE_CUSTOM;
> -if (virCPUDefCopyModel(updated, host, true) < 0)
> +if (virCPUDefCopyModelFilter(updated, host, true,
> + x86FeatureIsMigratable, map) < 0)
>  goto cleanup;
>  
> -i = 0;
> -while (i < updated->nfeatures) {
> -if (x86FeatureIsMigratable(updated->features[i].name, map) &&
> -STRNEQ(updated->features[i].name, "cmt") &&
> -STRNEQ(updated->features[i].name, "mbm_total") &&
> -STRNEQ(updated->features[i].name, "mbm_local")) {
> -i++;
> -} else {
> -VIR_FREE(updated->features[i].name);
> -VIR_DELETE_ELEMENT_INPLACE(updated->features, i, 
> updated->nfeatures);
> -}
> -}
> -
>  if (guest->vendor_id) {
>  VIR_FREE(updated->vendor_id);
>  if (VIR_STRDUP(updated->vendor_id, guest->vendor_id) < 0)
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index f70a36c..187d4c1 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -2859,6 +2859,19 @@ int virQEMUCapsProbeQMP(virQEMUCapsPtr qemuCaps,
>  }
>  
>  
> +static bool
> +virQEMUCapsCPUFilterFeatures(const char *name,
> + void *opaque ATTRIBUTE_UNUSED)
> +{
> +if (STREQ(name, "cmt") ||
> +STREQ(name, "mbm_total") ||
> +STREQ(name, "mbm_local"))
> +return false;
> +
> +return true;
> +}
> +
> +
>  void
>  virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
>  virCapsHostPtr host)
> @@ -2877,7 +2890,8 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
>  cpu->mode = VIR_CPU_MODE_CUSTOM;
>  cpu->match = VIR_CPU_MATCH_EXACT;
>  
> -if (virCPUDefCopyModel(cpu, host->cpu, true) < 0)
> +if (virCPUDefCopyModelFilter(cpu, host->cpu, true,
> + virQEMUCapsCPUFilterFeatures, NULL) < 0)
>  goto error;
>  }
>  
> 

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

Re: [libvirt] [PATCH 32/41] cpu: Make x86ModelFromCPU a bit smarter

2016-08-30 Thread John Ferlan



On 08/12/2016 09:33 AM, Jiri Denemark wrote:
> x86ModelFromCPU is used to provide CPUID data for features matching
> @policy. This patch allows callers to set @policy to -1 to get combined
> CPUID for all CPU features (including those implicitly provided a CPU
> model) specified in CPU def.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/cpu/cpu_x86.c | 44 +++-
>  1 file changed, 39 insertions(+), 5 deletions(-)
> 
> diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
> index 914352d..dcf9c25 100644
> --- a/src/cpu/cpu_x86.c
> +++ b/src/cpu/cpu_x86.c
> @@ -1011,6 +1011,15 @@ x86ModelFind(virCPUx86MapPtr map,
>  }
>  
>  
> +/*
> + * Computes CPU model data from a CPU definition associated with features
> + * matching @policy. If @policy equals -1, the computed model will describe
> + * all CPU features, i.e., it will contain:
> + *
> + *  features from model
> + *  + required and forced features
> + *  - disabled and forbidden features
> + */
>  static virCPUx86ModelPtr
>  x86ModelFromCPU(const virCPUDef *cpu,
>  virCPUx86MapPtr map,
> @@ -1023,10 +1032,11 @@ x86ModelFromCPU(const virCPUDef *cpu,
>   * just returns an empty model
>   */
>  if (cpu->type == VIR_CPU_TYPE_HOST &&
> -policy != VIR_CPU_FEATURE_REQUIRE)
> +policy != VIR_CPU_FEATURE_REQUIRE &&
> +policy != -1)
>  return x86ModelNew();
>  
> -if (policy == VIR_CPU_FEATURE_REQUIRE) {
> +if (policy == VIR_CPU_FEATURE_REQUIRE || policy == -1) {
>  if (!(model = x86ModelFind(map, cpu->model))) {
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Unknown CPU model %s"), cpu->model);
> @@ -1043,9 +1053,15 @@ x86ModelFromCPU(const virCPUDef *cpu,
>  
>  for (i = 0; i < cpu->nfeatures; i++) {
>  virCPUx86FeaturePtr feature;
> +virCPUFeaturePolicy fpol;
>  
> -if (cpu->type == VIR_CPU_TYPE_GUEST
> -&& cpu->features[i].policy != policy)
> +if (cpu->features[i].policy == -1)
> +fpol = VIR_CPU_FEATURE_REQUIRE;
> +else
> +fpol = cpu->features[i].policy;
> +
> +if ((policy == -1 && fpol == VIR_CPU_FEATURE_OPTIONAL) ||

[1]

> +(policy != -1 && fpol != policy))
>  continue;
>  
>  if (!(feature = x86FeatureFind(map, cpu->features[i].name))) {
> @@ -1054,8 +1070,26 @@ x86ModelFromCPU(const virCPUDef *cpu,
>  goto error;
>  }
>  
> -if (x86DataAdd(>data, >data))
> +if (policy == -1) {
> +switch (fpol) {
> +case VIR_CPU_FEATURE_FORCE:
> +case VIR_CPU_FEATURE_REQUIRE:
> +if (x86DataAdd(>data, >data) < 0)
> +goto error;
> +break;
> +
> +case VIR_CPU_FEATURE_DISABLE:
> +case VIR_CPU_FEATURE_FORBID:
> +x86DataSubtract(>data, >data);
> +break;
> +
> +case VIR_CPU_FEATURE_OPTIONAL:

[1]
Coverity happlily points out that OPTIONAL is a DEAD_CODE path
especially due to the prior check and continue;

I see no other sane way around it other than a comment before the case:

   /* coverity[dead_error_condition] */
   case VIR_CPU_FEATURE_OPTIONAL:


> +case VIR_CPU_FEATURE_LAST:
> +break;
> +}
> +} else if (x86DataAdd(>data, >data) < 0) {
>  goto error;
> +}
>  }
>  
>  return model;
> 

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

Re: [libvirt] [PATCH 40/41] qemu: Update guest CPU def in live XML

2016-08-30 Thread John Ferlan



On 08/12/2016 09:33 AM, Jiri Denemark wrote:
> Storing the updated CPU definition in the live domain definition saves
> us from having to update it over and over when we need it. Not to
> mention that we will soon further update the CPU definition according to
> QEMU once it's started.
> 
> A highly wanted side effect of this patch, libvirt will pass all CPU
> features explicitly specified in domain XML to QEMU, even those that are
> already included in the host model.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/qemu/qemu_command.c| 161 +++
>  src/qemu/qemu_domain.c |  10 +-
>  src/qemu/qemu_process.c| 178 
> +
>  .../qemuxml2argv-cpu-Haswell3.args |   2 +-
>  .../qemuxml2argv-cpu-exact2-nofallback.args|   3 +-
>  .../qemuxml2argvdata/qemuxml2argv-cpu-exact2.args  |   3 +-
>  .../qemuxml2argv-cpu-fallback.args |   2 +-
>  .../qemuxml2argv-cpu-host-model-cmt.args   |   2 +-
>  .../qemuxml2argv-cpu-host-model-fallback.args  |   2 +-
>  .../qemuxml2argv-cpu-minimum2.args |   2 +-
>  .../qemuxml2argvdata/qemuxml2argv-cpu-strict1.args |   3 +-
>  11 files changed, 147 insertions(+), 221 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 985b628..ebeeebe 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6432,62 +6432,18 @@ static int
>  qemuBuildCpuModelArgStr(virQEMUDriverPtr driver,
>  const virDomainDef *def,
>  virBufferPtr buf,
> -virQEMUCapsPtr qemuCaps,
> -bool *hasHwVirt,
> -bool migrating)
> +virQEMUCapsPtr qemuCaps)

This is a nice reduction!  So much more understandable.

>  {

[...]

> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index decbdd0..0b052ce 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3267,14 +3267,8 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver,
>  /* Update guest CPU requirements according to host CPU */
>  if ((flags & VIR_DOMAIN_XML_UPDATE_CPU) &&
>  def->cpu &&
> -(def->cpu->mode != VIR_CPU_MODE_CUSTOM || def->cpu->model)) {
> -if (!caps->host.cpu ||
> -!caps->host.cpu->model) {
> -virReportError(VIR_ERR_OPERATION_FAILED,
> -   "%s", _("cannot get host CPU capabilities"));
> -goto cleanup;
> -}
> -
> +(def->cpu->mode != VIR_CPU_MODE_CUSTOM ||
> + def->cpu->model)) {

It would seem this could be related to an earlier patch - that is the
cpuUpdate patch...  It can stay here, but it just feels like it could move.

>  if (virCPUUpdate(def->os.arch, def->cpu, caps->host.cpu) < 0)
>  goto cleanup;
>  }
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index d4269db..943704f 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4397,107 +4397,6 @@ qemuProcessStartValidateXML(virQEMUDriverPtr driver,
>  }
>  

[...]

>  
> +static int
> +qemuProcessUpdateGuestCPU(virDomainDefPtr def,
> +  virQEMUCapsPtr qemuCaps,
> +  virCapsPtr caps,
> +  unsigned int flags)
> +{
> +int ret = -1;
> +size_t nmodels = 0;
> +char **models = NULL;
> +
> +if (!def->cpu)
> +return 0;
> +
> +/* nothing to do if only topology part of CPU def is used */
> +if (def->cpu->mode == VIR_CPU_MODE_CUSTOM && !def->cpu->model)
> +return 0;
> +
> +/* Old libvirt added host CPU model to host-model CPUs for migrations,
> + * while new libvirt just turns host-model into custom mode. We need
> + * to fix the mode to maintain backward compatibility and to avoid
> + * the CPU model to be replaced in virCPUUpdate.
> + */
> +if (!(flags & VIR_QEMU_PROCESS_START_NEW) &&
> +ARCH_IS_X86(def->os.arch) &&
> +def->cpu->mode == VIR_CPU_MODE_HOST_MODEL &&
> +def->cpu->model) {
> +def->cpu->mode = VIR_CPU_MODE_CUSTOM;
> +}
> +
> +if (!virQEMUCapsIsCPUModeSupported(qemuCaps, caps, def->virtType,
> +   def->cpu->mode)) {

So this will fail for mode == VIR_CPU_MODE_HOST_MODEL if ->cpuModel
wasn't filled in, but it's just not clear enough to me (after 40
patches) whether the following checks could return NULL for the
GetHostModel. IIRC none of the failures are cores, just reported errors,
which is good and I supposed would be expected by this point in time!

> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("CPU mode '%s' for %s %s domain on %s host is not "
> + "supported by hypervisor"),
> +

Re: [libvirt] [PATCH 39/41] cpu: Rework cpuCompare* APIs

2016-08-30 Thread John Ferlan



On 08/12/2016 09:33 AM, Jiri Denemark wrote:
> Both cpuCompare* APIs are renamed to virCPUCompare*. And they should now
> work for any guest CPU definition, i.e., even for host-passthrough
> (trivial) and host-model CPUs. The implementation in x86 driver is
> enhanced to provide a hint about -noTSX Broadwell and Haswell models
> when appropriate.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/cpu/cpu.c| 37 -
>  src/cpu/cpu.h| 21 ++--
>  src/cpu/cpu_arm.c|  8 ++---
>  src/cpu/cpu_ppc64.c  | 15 +++--
>  src/cpu/cpu_x86.c| 84 
> 
>  src/libvirt_private.syms |  4 +--
>  src/qemu/qemu_driver.c   | 14 ++--
>  tests/cputest.c  |  4 +--
>  8 files changed, 119 insertions(+), 68 deletions(-)
> 
> diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
> index d6b0372..4ea192c 100644
> --- a/src/cpu/cpu.c
> +++ b/src/cpu/cpu.c
> @@ -91,8 +91,9 @@ cpuGetSubDriverByName(const char *name)
>  
>  
>  /**
> - * cpuCompareXML:
> + * virCPUCompareXML:
>   *
> + * @arch: CPU architecture
>   * @host: host CPU definition
>   * @xml: XML description of either guest or host CPU to be compared with 
> @host

Existing, @failIncompatible description is missing

>   *
> @@ -104,25 +105,26 @@ cpuGetSubDriverByName(const char *name)
>   * the @host CPU.
>   */
>  virCPUCompareResult
> -cpuCompareXML(virCPUDefPtr host,
> -  const char *xml,
> -  bool failIncompatible)
> +virCPUCompareXML(virArch arch,
> + virCPUDefPtr host,
> + const char *xml,
> + bool failIncompatible)
>  {
>  xmlDocPtr doc = NULL;
>  xmlXPathContextPtr ctxt = NULL;
>  virCPUDefPtr cpu = NULL;
>  virCPUCompareResult ret = VIR_CPU_COMPARE_ERROR;
>  
> -VIR_DEBUG("host=%p, xml=%s", host, NULLSTR(xml));
> +VIR_DEBUG("arch=%s, host=%p, xml=%s",
> +  virArchToString(arch), host, NULLSTR(xml));

The prototype changed such that 'host' and 'xml' could be passed as NULL
without a compiler complaint (ok a static analysis complaint).  Was that
by design?

>  
>  if (!(doc = virXMLParseStringCtxt(xml, _("(CPU_definition)"), )))

Having xml as NULL probably doesn't work well here.

>  goto cleanup;
>  
> -cpu = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_AUTO);
> -if (cpu == NULL)
> +if (!(cpu = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_AUTO)))
>  goto cleanup;
>  
> -ret = cpuCompare(host, cpu, failIncompatible);
> +ret = virCPUCompare(arch, host, cpu, failIncompatible);

Allowing host to be NULL will cause failure in PPC and X86 which perhaps
is expected.

>  
>   cleanup:
>  virCPUDefFree(cpu);
> @@ -134,8 +136,9 @@ cpuCompareXML(virCPUDefPtr host,
>  
>  
>  /**
> - * cpuCompare:
> + * virCPUCompare:
>   *
> + * @arch: CPU architecture
>   * @host: host CPU definition
>   * @cpu: either guest or host CPU to be compared with @host
>   *
> @@ -147,21 +150,23 @@ cpuCompareXML(virCPUDefPtr host,
>   * the @host CPU.
>   */
>  virCPUCompareResult
> -cpuCompare(virCPUDefPtr host,
> -   virCPUDefPtr cpu,
> -   bool failIncompatible)
> +virCPUCompare(virArch arch,
> +  virCPUDefPtr host,
> +  virCPUDefPtr cpu,
> +  bool failIncompatible)
>  {
>  struct cpuArchDriver *driver;
>  
> -VIR_DEBUG("host=%p, cpu=%p", host, cpu);
> +VIR_DEBUG("arch=%s, host=%p, cpu=%p",
> +  virArchToString(arch), host, cpu);
>  
> -if ((driver = cpuGetSubDriver(host->arch)) == NULL)
> +if (!(driver = cpuGetSubDriver(arch)))
>  return VIR_CPU_COMPARE_ERROR;
>  
> -if (driver->compare == NULL) {
> +if (!driver->compare) {
>  virReportError(VIR_ERR_NO_SUPPORT,
> _("cannot compare CPUs of %s architecture"),
> -   virArchToString(host->arch));
> +   virArchToString(arch));
>  return VIR_CPU_COMPARE_ERROR;
>  }
>  
> diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
> index 77ccb38..0e81e91 100644
> --- a/src/cpu/cpu.h
> +++ b/src/cpu/cpu.h
> @@ -45,7 +45,7 @@ struct _virCPUData {
>  
>  
>  typedef virCPUCompareResult
> -(*cpuArchCompare)   (virCPUDefPtr host,
> +(*virCPUArchCompare)(virCPUDefPtr host,
>   virCPUDefPtr cpu,
>   bool failIncompatible);
>  
> @@ -116,7 +116,7 @@ struct cpuArchDriver {
>  const char *name;
>  const virArch *arch;
>  unsigned int narch;
> -cpuArchCompare  compare;
> +virCPUArchCompare   compare;
>  cpuArchDecode   decode;
>  cpuArchEncode   encode;
>  cpuArchDataFree free;
> @@ -134,16 +134,17 @@ struct cpuArchDriver {
>  
>  
>  virCPUCompareResult
> -cpuCompareXML(virCPUDefPtr host,
> -  const char *xml,
> -  bool failIncompatible)
> -ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

Was

Re: [libvirt] [PATCH 38/41] cpu: Rework cpuHasFeature

2016-08-30 Thread John Ferlan



On 08/12/2016 09:33 AM, Jiri Denemark wrote:
> The function is renamed to virCPUDataCheckFeature and another function
> (virCPUCheckFeature) which works on CPU definition rather than raw CPU
> data is introduced.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/cpu/cpu.c | 55 
> ---
>  src/cpu/cpu.h | 23 ++
>  src/cpu/cpu_arm.c |  1 -
>  src/cpu/cpu_ppc64.c   |  1 -
>  src/cpu/cpu_s390.c|  1 -
>  src/cpu/cpu_x86.c | 30 ---
>  src/libvirt_private.syms  |  3 ++-
>  src/qemu/qemu_command.c   |  2 +-
>  src/qemu/qemu_domain.c|  7 --
>  src/qemu/qemu_parse_command.c |  2 +-
>  src/qemu/qemu_process.c   | 10 
>  src/vmware/vmware_conf.c  |  6 ++---
>  tests/cputest.c   |  6 -
>  13 files changed, 113 insertions(+), 34 deletions(-)
> 

Multiple things going on - too bad they couldn't be split...  Still not
clear why cputest changes, but it's been a long road to get this far so
I'm tired.

[...]


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

Re: [libvirt] [PATCH 35/41] cpu: Rework cpuUpdate

2016-08-30 Thread John Ferlan



On 08/12/2016 09:33 AM, Jiri Denemark wrote:
> The reworked API is now called virCPUUpdate and it should change the
> provided CPU definition into a one which can be consumed by QEMU command

s/by/by the/

> line builder:
> 
> - host-passthrough remains unchanged
> - host-model is turned into custom CPU with a model and features
>   copied from host
> - custom CPU with minimum match is converted similarly to host-model
> - optional features are updated according to host's CPU
> 
> Signed-off-by: Jiri Denemark 
> ---
>  po/POTFILES.in |   1 +
>  src/cpu/cpu.c  |  59 --
>  src/cpu/cpu.h  |  11 +-
>  src/cpu/cpu_arm.c  |  36 +++-
>  src/cpu/cpu_ppc64.c|  32 ++--
>  src/cpu/cpu_x86.c  | 212 
> -
>  src/libvirt_private.syms   |   2 +-
>  src/qemu/qemu_command.c|   2 +-
>  src/qemu/qemu_domain.c |   2 +-
>  src/qemu/qemu_process.c|   2 +-
>  tests/cputest.c|  14 +-
>  .../cputestdata/x86-host+host-model-nofallback.xml |   2 +-
>  tests/cputestdata/x86-host+host-model.xml  |   2 +-
>  .../x86-host+host-passthrough-features.xml |   4 +
>  tests/cputestdata/x86-host+host-passthrough.xml|  19 +-
>  tests/cputestdata/x86-host+min.xml |  27 +--
>  tests/cputestdata/x86-host+pentium3.xml|  39 ++--
>  tests/cputestdata/x86-host-invtsc+host-model.xml   |   2 +-
>  .../cputestdata/x86-host-passthrough-features.xml  |   4 +
>  19 files changed, 227 insertions(+), 245 deletions(-)
>  create mode 100644 tests/cputestdata/x86-host+host-passthrough-features.xml
>  create mode 100644 tests/cputestdata/x86-host-passthrough-features.xml
> 
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 25dbc84..1469240 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -43,6 +43,7 @@ src/conf/virchrdev.c
>  src/conf/virdomainobjlist.c
>  src/conf/virsecretobj.c
>  src/cpu/cpu.c
> +src/cpu/cpu_arm.c
>  src/cpu/cpu_map.c
>  src/cpu/cpu_ppc64.c
>  src/cpu/cpu_x86.c
> diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
> index fae3885..f3eb211 100644
> --- a/src/cpu/cpu.c
> +++ b/src/cpu/cpu.c
> @@ -579,38 +579,71 @@ cpuBaseline(virCPUDefPtr *cpus,
>  
>  
>  /**
> - * cpuUpdate:
> + * virCPUUpdate:

In a way I'd expect a "virCPUUpdate" API to go in src/util/...
somewhere. Not a problem to keep it here, but I guess when I see vir
prefixed functions I think src/util/...

>   *
> - * @guest: guest CPU definition
> + * @arch: CPU architecture
> + * @guest: guest CPU definition to be updated
>   * @host: host CPU definition
>   *
>   * Updates @guest CPU definition according to @host CPU. This is required to
> - * support guest CPU definition which are relative to host CPU, such as CPUs
> - * with VIR_CPU_MODE_CUSTOM and optional features or VIR_CPU_MATCH_MINIMUM, 
> or
> - * CPUs with non-custom mode (VIR_CPU_MODE_HOST_MODEL,
> - * VIR_CPU_MODE_HOST_PASSTHROUGH).
> + * support guest CPU definitions specified relatively to host CPU, such as
> + * CPUs with VIR_CPU_MODE_CUSTOM and optional features or
> + * VIR_CPU_MATCH_MINIMUM, or CPUs with VIR_CPU_MODE_HOST_MODEL.
> + * When the guest CPU was not specified relatively, the function does nothing
> + * and returns success.
>   *
>   * Returns 0 on success, -1 on error.
>   */
>  int
> -cpuUpdate(virCPUDefPtr guest,
> -  const virCPUDef *host)
> +virCPUUpdate(virArch arch,
> + virCPUDefPtr guest,
> + const virCPUDef *host)

Still not 100% clear whether if eventually (patch 40) it's possible to
enter here with 'host == NULL'... Keeping in mind from patch 26 that
virQEMUCapsInitHostCPUModel could have qemuCaps->cpuModel = NULL, thus
the virQEMUCapsGetHostModel could then return a NULL from
qemuProcessUpdateGuestCPU which calls this function. However, that
possibility is gated by whether virQEMUCapsIsCPUModeSupported succeeds
or not. Since you've added NULLSTR and host ? checks, I'm partially
assuming it's possible to get here with host == NULL. The intro comments
here don't help me determine that though.

I'm lost in the terminology between old/new that's described in patch
40. If your belief is things are going to be OK, then I'm fine with
that, but I figured I'd at least point out what I saw and what got
confusing eventually. Hopefully it's understandable.

>  {
>  struct cpuArchDriver *driver;
>  
> -VIR_DEBUG("guest=%p, host=%p", guest, host);
> +VIR_DEBUG("arch=%s, guest=%p mode=%s model=%s, host=%p model=%s",
> +  virArchToString(arch), guest, 
> virCPUModeTypeToString(guest->mode),
> +  NULLSTR(guest->model), host, NULLSTR(host ? host->model : 
> NULL));

The Coverity build

[libvirt] libvirt-2.2.0 Release Candidate 2 is out

2016-08-30 Thread Daniel Veillard


  It's tagged in git, with signed tarball and rpms at the usual place:

ftp://libvirt.org/libvirt/

but it looks broken to me, if I use virt manager and try to start a guest
it fails to render the grapical display of the guest. Need to kill virt-manager
as it blocks and become irresponsive, that's not good. virsh CLI still works 
fine
but there is a new issue on the integration of console. I doubt it was 
introduced
since RC1, 2.1.0 was fine.

 I would be tempted to delay the final release until this is analyzed and 
possibly
fixed (BTW I'm on standard Fedora 24 x86-64) assuming others can reproduce the 
issue.

  thanks,

Daniel

-- 
Daniel Veillard  | Open Source and Standards, Red Hat
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

Re: [libvirt] [PATCH] libxl: advertise support for migration V3

2016-08-30 Thread Joao Martins

On 08/30/2016 06:21 PM, Jim Fehlig wrote:
> On 08/30/2016 08:45 AM, Joao Martins wrote:
>> On 08/29/2016 06:20 PM, Jim Fehlig wrote:
>>> The libxl driver has long supported migration V3 but has never
>>> indicated so in the connectSupportsFeature API. 
>> Hmm, but IIRC it was only since "recent" commit 8db77b3 right?
> 
> The libxl driver has supported the VIR_DRV_FEATURE_MIGRATION_PARAMS V3 family 
> of
> migration functions since commit 9b8d6e1e in May 2014.
Indeed.

>>  Or rather Nikolay's
>> reworking top-level migration code, which effectively would convert to the 
>> params
>> versions accordingly. Before that rework I think one had to implement both 
>> params and
>> non-params variants. Would it be worth adding a comment mainly to help the 
>> reader?
> 
> I think you are right, but support for V3 is independent of the params vs
> non-params variants. virt-manager uses the generic virDomainMigrate() function
> in src/libvirt-domain.c, and at least as far back as libvirt 1.2.5 that 
> function
> tests whether both source and destination support 
> VIR_DRV_FEATURE_MIGRATION_V{1,2,3}. If the driver doesn't advertise support 
> for
> any of the versions, virDomainMigrate() returns VIR_ERR_NO_SUPPORT.
Hmm, OK. IIRC the problems I once observed where more around 
virDomainMigrateToURI -
which got addressed in the series containing the commit above. Anyhow sorry for 
the
noise!

> 
>>
>> (I vaguely remember this as I dropped my v3 patch as being no longer 
>> necessary)
>>
>>> As a result, apps
>>> such as virt-manager that use the more generic virDomainMigrate API
>>> fail with
>>>
>>> libvirtError: this function is not supported by the connection driver:
>>> virDomainMigrate
>>>
>>> Add VIR_DRV_FEATURE_MIGRATION_V3 to the list of features marked as
>>> supported in the connectSupportsFeature API.
>> FWIW and irrespective of the comment above:
>>
>> Reviewed-by: Joao Martins 
> 
> Thanks. Along with Cedric's ACK, and considering it is a trivial bug fix, I 
> plan
> to push this for RC2.

Cool.

Regards,
Joao

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

Re: [libvirt] [PATCH] libxl: advertise support for migration V3

2016-08-30 Thread Jim Fehlig

On 08/30/2016 12:52 AM, Cedric Bosdonnat wrote:
> On Mon, 2016-08-29 at 11:20 -0600, Jim Fehlig wrote:
>> The libxl driver has long supported migration V3 but has never
>> indicated so in the connectSupportsFeature API. As a result, apps
>> such as virt-manager that use the more generic virDomainMigrate API
>> fail with
>>
>> libvirtError: this function is not supported by the connection driver:
>> virDomainMigrate
>>
>> Add VIR_DRV_FEATURE_MIGRATION_V3 to the list of features marked as
>> supported in the connectSupportsFeature API.
>>
>> Signed-off-by: Jim Fehlig 
>> ---
>>  src/libxl/libxl_driver.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index a573c82..3ffaa74 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -5677,6 +5677,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int 
>> feature)
>>  return -1;
>>  
>>  switch (feature) {
>> +case VIR_DRV_FEATURE_MIGRATION_V3:
>>  case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
>>  case VIR_DRV_FEATURE_MIGRATION_PARAMS:
>>  case VIR_DRV_FEATURE_MIGRATION_P2P:
> ACK

Thanks. Since this is a trivial bug fix, and given a second review by Joao, I've
pushed this for RC2.

Regards,
Jim

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

Re: [libvirt] [PATCH] libxl: advertise support for migration V3

2016-08-30 Thread Jim Fehlig

On 08/30/2016 08:45 AM, Joao Martins wrote:
> On 08/29/2016 06:20 PM, Jim Fehlig wrote:
>> The libxl driver has long supported migration V3 but has never
>> indicated so in the connectSupportsFeature API. 
> Hmm, but IIRC it was only since "recent" commit 8db77b3 right?

The libxl driver has supported the VIR_DRV_FEATURE_MIGRATION_PARAMS V3 family of
migration functions since commit 9b8d6e1e in May 2014.

>  Or rather Nikolay's
> reworking top-level migration code, which effectively would convert to the 
> params
> versions accordingly. Before that rework I think one had to implement both 
> params and
> non-params variants. Would it be worth adding a comment mainly to help the 
> reader?

I think you are right, but support for V3 is independent of the params vs
non-params variants. virt-manager uses the generic virDomainMigrate() function
in src/libvirt-domain.c, and at least as far back as libvirt 1.2.5 that function
tests whether both source and destination support 
VIR_DRV_FEATURE_MIGRATION_V{1,2,3}. If the driver doesn't advertise support for
any of the versions, virDomainMigrate() returns VIR_ERR_NO_SUPPORT.

>
> (I vaguely remember this as I dropped my v3 patch as being no longer 
> necessary)
>
>> As a result, apps
>> such as virt-manager that use the more generic virDomainMigrate API
>> fail with
>>
>> libvirtError: this function is not supported by the connection driver:
>> virDomainMigrate
>>
>> Add VIR_DRV_FEATURE_MIGRATION_V3 to the list of features marked as
>> supported in the connectSupportsFeature API.
> FWIW and irrespective of the comment above:
>
> Reviewed-by: Joao Martins 

Thanks. Along with Cedric's ACK, and considering it is a trivial bug fix, I plan
to push this for RC2.

Regards,
Jim

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

Re: [libvirt] [PATCH v7 0/4] Add Mediated device support

2016-08-30 Thread Alex Williamson

Hi folks,

At KVM Forum we had a BoF session primarily around the mediated device
sysfs interface.  I'd like to share what I think we agreed on and the
"problem areas" that still need some work so we can get the thoughts
and ideas from those who weren't able to attend.

DanPB expressed some concern about the mdev_supported_types sysfs
interface, which exposes a flat csv file with fields like "type",
"number of instance", "vendor string", and then a bunch of type
specific fields like "framebuffer size", "resolution", "frame rate
limit", etc.  This is not entirely machine parsing friendly and sort of
abuses the sysfs concept of one value per file.  Example output taken
from Neo's libvirt RFC:

cat /sys/bus/pci/devices/:86:00.0/mdev_supported_types
# vgpu_type_id, vgpu_type, max_instance, num_heads, frl_config, framebuffer,
max_resolution
11  ,"GRID M60-0B",  16,   2,  45, 512M,2560x1600
12  ,"GRID M60-0Q",  16,   2,  60, 512M,2560x1600
13  ,"GRID M60-1B",   8,   2,  45,1024M,2560x1600
14  ,"GRID M60-1Q",   8,   2,  60,1024M,2560x1600
15  ,"GRID M60-2B",   4,   2,  45,2048M,2560x1600
16  ,"GRID M60-2Q",   4,   4,  60,2048M,2560x1600
17  ,"GRID M60-4Q",   2,   4,  60,4096M,3840x2160
18  ,"GRID M60-8Q",   1,   4,  60,8192M,3840x2160

The create/destroy then looks like this:

echo "$mdev_UUID:vendor_specific_argument_list" >
/sys/bus/pci/devices/.../mdev_create

echo "$mdev_UUID:vendor_specific_argument_list" >
/sys/bus/pci/devices/.../mdev_destroy

"vendor_specific_argument_list" is nebulous.

So the idea to fix this is to explode this into a directory structure,
something like:

├── mdev_destroy
└── mdev_supported_types
├── 11
│   ├── create
│   ├── description
│   └── max_instances
├── 12
│   ├── create
│   ├── description
│   └── max_instances
└── 13
├── create
├── description
└── max_instances

Note that I'm only exposing the minimal attributes here for simplicity,
the other attributes would be included in separate files and we would
require vendors to create standard attributes for common device classes.

For vGPUs like NVIDIA where we don't support multiple types
concurrently, this directory structure would update as mdev devices are
created, removing no longer available types.  I carried forward
max_instances here, but perhaps we really want to copy SR-IOV and
report a max and current allocation.  Creation and deletion is
simplified as we can simply "echo $UUID > create" per type.  I don't
understand why destroy had a parameter list, so here I imagine we can
simply do the same... in fact, I'd actually rather see a "remove" sysfs
entry under each mdev device, so we remove it at the device rather than
in some central location (any objections?).

We discussed how this might look with Intel devices which do allow
mixed vGPU types concurrently.  We believe, but need confirmation, that
the vendor driver could still make a finite set of supported types,
perhaps with additional module options to the vendor driver to enable
more "exotic" types.  So for instance if IGD vGPUs are based on
power-of-2 portions of the framebuffer size, then the vendor driver
could list types with 32MB, 64MB, 128MB, etc in useful and popular
sizes.  As vGPUs are allocated, the larger sizes may become unavailable.

We still don't have any way for the admin to learn in advance how the
available supported types will change once mdev devices start to be
created.  I'm not sure how we can create a specification for this, so
probing by creating devices may be the most flexible model.

The other issue is the start/stop requirement, which was revealed to
setup peer-to-peer resources between vGPUs which is a limited hardware
resource.  We'd really like to have these happen automatically on the
first open of a vfio mdev device file and final release.  So we
brainstormed how the open/release callbacks could know the other mdev
devices for a given user.  This is where the instance number came into
play previously.  This is an area that needs work.

There was a thought that perhaps on open() the vendor driver could look
at the user pid and use that to associate with other devices, but the
problem here is that we open and begin access to each device, so
devices do this discovery serially rather than in parallel as desired.
(we might not fault in mmio space yet though, so I wonder if open()
could set the association of mdev to pid, then the first mmio fault
would trigger the resource allocation?  Then all the "magic" would live
in the vendor driver.  open() could fail if the pid already has running
mdev devices and the vendor driver chooses not to support hotplug)

One comment was that for a GPU that only supports homogeneous vGPUs,
libvirt may choose to create all the vGPUs in

Re: [libvirt] [PATCH] libxl: add memory attach support

2016-08-30 Thread Joao Martins

Hey!

On 08/30/2016 11:00 AM, Bob Liu wrote:
> Support for VIR_DOMAIN_DEVICE_MEMORY on domainAttachDeviceFlags API in libxl
> driver, using libxl_set_memory_target in xen libxl.
> 
> With "virsh attach-device" command we can then hotplug memory to a domain:
> 
> 
> 128
> 0
> 
> 
> 
> $ virsh attach-device domain_name this.xml --live
> 
> Signed-off-by: Bob Liu 
See few very minor comments below, but overall LGTM.

> ---
>  src/libxl/libxl_domain.c |  1 +
>  src/libxl/libxl_driver.c | 29 +
>  2 files changed, 30 insertions(+)
> 
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index f529a2e..3924ba0 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -414,6 +414,7 @@ virDomainDefParserConfig libxlDomainDefParserConfig = {
>  .macPrefix = { 0x00, 0x16, 0x3e },
>  .devicesPostParseCallback = libxlDomainDeviceDefPostParse,
>  .domainPostParseCallback = libxlDomainDefPostParse,
> +.features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG
>  };
>  
>  
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index a34eb02..541ea3b 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -3085,6 +3085,30 @@ libxlDomainAttachHostUSBDevice(libxlDriverPrivatePtr 
> driver,
>  #endif
>  
>  static int
> +libxlDomainAttachMemory(libxlDriverPrivatePtr driver,
> +virDomainObjPtr vm,
> +virDomainMemoryDefPtr mem)
> +{
> +int res = 0;
I think you don't need to initialize the variable.

> +libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
> +
> +if (mem->targetNode != 0) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("unsupport non-zero target node for memory 
> device"));
Probably changing the message to "non-zero target node not supported" instead? 
The
messages sounds like you are removing support for it. But english is not my
mothertongue so may be it's just my wrong reading :)

Should we fail with other error as this patch, or VIR_WARN the user and ignore
targetNode. AFAIK libxl doesn't yet support ballooning for a specific node. 
What do
others think?

> +return -1;
> +}
> +
> +res = libxl_set_memory_target(cfg->ctx, vm->def->id, mem->size, 1, 1);
Should we unlock virDomainObj while ballooning, as similarly done in
libxlDomainSetMemory* ?

> +if (res < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("Failed to attach %lluKB memory for domain %d"),
> +   mem->size, vm->def->id);
> +return -1;
> +}
> +return 0;
> +}
> +
> +static int
>  libxlDomainAttachHostDevice(libxlDriverPrivatePtr driver,
>  virDomainObjPtr vm,
>  virDomainHostdevDefPtr hostdev)
> @@ -3284,6 +3308,11 @@ libxlDomainAttachDeviceLive(libxlDriverPrivatePtr 
> driver,
>  dev->data.hostdev = NULL;
>  break;
>  
> +case VIR_DOMAIN_DEVICE_MEMORY:
> +ret = libxlDomainAttachMemory(driver, vm, dev->data.memory);
> +dev->data.memory = NULL;
This should probably be:

ret = libxlDomainAttachMemory(driver, vm, dev->data.memory);
if (!ret)
dev->data.memory = NULL;

Right?

> +break;
> +
>  default:
>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("device type '%s' cannot be attached"),
> 

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

Re: [libvirt] [PATCH 30/41] qemu: Introduce virQEMUCapsIsCPUModeSupported

2016-08-30 Thread John Ferlan



On 08/12/2016 09:33 AM, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark 
> ---
>  src/qemu/qemu_capabilities.c | 55 
> ++--
>  src/qemu/qemu_capabilities.h |  4 
>  2 files changed, 47 insertions(+), 12 deletions(-)
> 

So another 10 down almost 75% done!  Let's consider ACK's again...

Still not sure about patch 20 w/r/t to the need for "unknown" printing,
the sorting by yes, no, and unknown, and whether LoadCache should "read"
what's been printed and validate against the current (if that matters).

As for 21-30, if there's no reply then an ACK can be implied.  A couple
of replies (23, 24) make suggestions for function name changes - your
choice on those. Two (25, 27) I've just left some thoughts - they don't
really require changes.

Patch 22 will need an adjustment for an ACK, but the build would fail
anyway, so it's obvious.

Patch 26 needs an adjust to fix a leak for an ACK, your choice on
renaming cpuModel, and I think the virResetLastError should be called.
I've seen "future" patches where returning NULL may come into play...

Patch 28 - your call on the domain page reference

That just leaves this one with one innocuous adjustment shown below. Not
required for an ACK

John


> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 670f944..f70a36c 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -2302,6 +2302,32 @@ virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps)
>  }
>  
>  
> +bool
> +virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps,
> +  virCapsPtr caps,
> +  virDomainVirtType type,
> +  virCPUMode mode)
> +{
> +switch (mode) {
> +case VIR_CPU_MODE_HOST_PASSTHROUGH:
> +return type == VIR_DOMAIN_VIRT_KVM &&
> +   virQEMUCapsGuestIsNative(caps->host.arch, qemuCaps->arch);
> +
> +case VIR_CPU_MODE_HOST_MODEL:
> +return !!qemuCaps->cpuModel;
> +
> +case VIR_CPU_MODE_CUSTOM:
> +return qemuCaps->cpuDefinitions &&
> +   qemuCaps->cpuDefinitions->count > 0;
> +
> +case VIR_CPU_MODE_LAST:
> +break;
> +}
> +
> +return false;
> +}
> +
> +
>  int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps,
> size_t *nmachines,
> virCapsGuestMachinePtr **machines)
> @@ -4260,22 +4286,27 @@ virQEMUCapsFillDomainCPUCaps(virCapsPtr caps,
>   virQEMUCapsPtr qemuCaps,
>   virDomainCapsPtr domCaps)
>  {
> -virDomainCapsCPUModelsPtr filtered = NULL;
> -char **models = NULL;
> -
> -if (domCaps->virttype == VIR_DOMAIN_VIRT_KVM &&
> -virQEMUCapsGuestIsNative(caps->host.arch, qemuCaps->arch))
> +if (virQEMUCapsIsCPUModeSupported(qemuCaps, caps, domCaps->virttype,
> +  VIR_CPU_MODE_HOST_PASSTHROUGH))
>  domCaps->cpu.hostPassthrough = true;
>  
> -domCaps->cpu.hostModel = virCPUDefCopy(qemuCaps->cpuModel);
> +if (virQEMUCapsIsCPUModeSupported(qemuCaps, caps, domCaps->virttype,
> +  VIR_CPU_MODE_HOST_MODEL))
> +domCaps->cpu.hostModel = virCPUDefCopy(qemuCaps->cpuModel);
>  
> -if (qemuCaps->cpuDefinitions &&
> -cpuGetModels(domCaps->arch, ) >= 0) {
> -filtered = virDomainCapsCPUModelsFilter(qemuCaps->cpuDefinitions,
> -models);
> -virStringFreeList(models);
> +if (virQEMUCapsIsCPUModeSupported(qemuCaps, caps, domCaps->virttype,
> +  VIR_CPU_MODE_CUSTOM)) {
> +virDomainCapsCPUModelsPtr filtered = NULL;
> +char **models = NULL;
> +
> +if (qemuCaps->cpuDefinitions &&

Redundant check for MODE_CUSTOM when ModeSupported is true

> +cpuGetModels(domCaps->arch, ) >= 0) {
> +filtered = virDomainCapsCPUModelsFilter(qemuCaps->cpuDefinitions,
> +models);
> +virStringFreeList(models);
> +}
> +domCaps->cpu.custom = filtered;
>  }
> -domCaps->cpu.custom = filtered;
>  
>  return 0;
>  }
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 1203073..2ea5849 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -425,6 +425,10 @@ int virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps,
>   char ***names,
>   size_t *count);
>  virCPUDefPtr virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps);
> +bool virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps,
> +   virCapsPtr caps,
> +   virDomainVirtType type,
> +   virCPUMode mode);
>

Re: [libvirt] [PATCH 28/41] Show host model in domain capabilities

2016-08-30 Thread John Ferlan



On 08/12/2016 09:33 AM, Jiri Denemark wrote:
> The domain capabilities XML is capable of showing whether each guest CPU
> mode is supported or not with a possibility to provide additional
> details. This patch enhances host-model capability to advertise the
> exact CPU model which will be used as a host-model:
> 
> 
> ...
> 
> Broadwell
> Intel
> 
> 
> 
> ...
> 
> 
> Signed-off-by: Jiri Denemark 
> ---
>  docs/formatdomaincaps.html.in   | 21 
> +++--
>  docs/schemas/domaincaps.rng | 10 ++
>  src/conf/domain_capabilities.c  | 16 +---
>  src/conf/domain_capabilities.h  |  2 +-
>  src/qemu/qemu_capabilities.c|  4 +---
>  tests/domaincapsschemadata/full.xml |  5 -
>  tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml|  4 +++-
>  .../qemu_2.6.0-gicv2-virt.aarch64.xml   |  2 +-
>  .../qemu_2.6.0-gicv3-virt.aarch64.xml   |  2 +-
>  tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml   |  2 +-
>  tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml   |  4 +++-
>  tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml|  4 +++-
>  tests/domaincapstest.c  | 10 --
>  13 files changed, 68 insertions(+), 18 deletions(-)
> 

Hmmm... I guess we are displaying the cpuModel data - it just wasn't
"obvious" previously...

> diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
> index 49ccbfc..34eb777 100644
> --- a/docs/formatdomaincaps.html.in
> +++ b/docs/formatdomaincaps.html.in
> @@ -154,7 +154,12 @@
>...
>cpu
>  mode name='host-passthrough' supported='yes'/
> -mode name='host-model' supported='yes'/
> +mode name='host-model' supported='yes'
> +  model fallback='allow'Broadwell/model
> +  vendorIntel/vendor
> +  feature policy='disable' name='aes'/
> +  feature policy='require' name='vmx'/
> +/mode
>  mode name='custom' supported='yes'
>model usable='no'Broadwell/model
>model usable='yes'Broadwell-noTSX/model
> @@ -177,7 +182,19 @@
>No mode specific details are provided.
>  
>host-model
> -  No mode specific details are provided yet.
> +  
> +If host-model is supported by the hypervisor, the
> +mode describes the guest CPU which will be used when
> +starting a domain with host-model CPU. The hypervisor
> +specifics (such as unsupported CPU models or features, machine type,
> +etc.) may be accounted for in this guest CPU specification and thus
> +the CPU can be different from the one shown in host capabilities XML.
> +This is indicated by the fallback attribute of the
> +model sub element: allow means not all
> +specifics were accounted for and thus the CPU a guest will see may
> +be different; forbid says that the CPU a guest will see

s/says/indicates

> +should match this CPU definition.
> +  

Should this make a reference to

   http://libvirt.org/formatdomain.html#elementsCPU

since that's the allow/forbid and 'host-model' for the domain description?

Too bad there weren't any easily crafted 'forbid' examples for test...

>  
>custom
>

[...]

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

Re: [libvirt] [PATCH] libxl: advertise support for migration V3

2016-08-30 Thread Joao Martins

On 08/29/2016 06:20 PM, Jim Fehlig wrote:
> The libxl driver has long supported migration V3 but has never
> indicated so in the connectSupportsFeature API. 

Hmm, but IIRC it was only since "recent" commit 8db77b3 right? Or rather 
Nikolay's
reworking top-level migration code, which effectively would convert to the 
params
versions accordingly. Before that rework I think one had to implement both 
params and
non-params variants. Would it be worth adding a comment mainly to help the 
reader?

(I vaguely remember this as I dropped my v3 patch as being no longer necessary)

> As a result, apps
> such as virt-manager that use the more generic virDomainMigrate API
> fail with
> 
> libvirtError: this function is not supported by the connection driver:
> virDomainMigrate
> 
> Add VIR_DRV_FEATURE_MIGRATION_V3 to the list of features marked as
> supported in the connectSupportsFeature API.

FWIW and irrespective of the comment above:

Reviewed-by: Joao Martins 

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

Re: [libvirt] [PATCH 27/41] cpu: Drop false support for ARM cpu-model

2016-08-30 Thread John Ferlan



On 08/12/2016 09:33 AM, Jiri Denemark wrote:
> The ARM CPU driver wrongly reported host CPU model as "host", which made
> host-model to be just an alias for host-passthrough. Let's drop this
> insanity.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/cpu/cpu_arm.c | 34 ++
>  1 file changed, 2 insertions(+), 32 deletions(-)
> 

I see cpuNodeData and cpuDecode are generally used together, but not so
in tests/cputest.c, which doesn't test arm anyway, so seemingly not a
problem...  Just making a note...

> diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
> index 6090253..a3aed6b 100644
> --- a/src/cpu/cpu_arm.c
> +++ b/src/cpu/cpu_arm.c
> @@ -37,36 +37,6 @@ static const virArch archs[] = {
>  VIR_ARCH_AARCH64,
>  };
>  
> -static virCPUDataPtr
> -armNodeData(virArch arch)
> -{
> -virCPUDataPtr data;
> -
> -if (VIR_ALLOC(data) < 0)
> -return NULL;
> -
> -data->arch = arch;
> -
> -return data;
> -}
> -
> -static int
> -armDecode(virCPUDefPtr cpu,
> -  const virCPUData *data ATTRIBUTE_UNUSED,
> -  const char **models ATTRIBUTE_UNUSED,
> -  unsigned int nmodels ATTRIBUTE_UNUSED,
> -  const char *preferred ATTRIBUTE_UNUSED,
> -  unsigned int flags)
> -{
> -virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1);
> -
> -if (cpu->model == NULL &&
> -VIR_STRDUP(cpu->model, "host") < 0)
> -return -1;
> -
> -return 0;
> -}
> -
>  static void
>  armDataFree(virCPUDataPtr data)
>  {
> @@ -128,10 +98,10 @@ struct cpuArchDriver cpuDriverArm = {
>  .arch = archs,
>  .narch = ARRAY_CARDINALITY(archs),
>  .compare = armCompare,
> -.decode = armDecode,
> +.decode = NULL,
>  .encode = NULL,
>  .free = armDataFree,
> -.nodeData = armNodeData,
> +.nodeData = NULL,
>  .guestData = armGuestData,
>  .baseline = armBaseline,
>  .update = armUpdate,
> 

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

Re: [libvirt] [PATCH 26/41] qemu: Store host-model CPU in qemu capabilities

2016-08-30 Thread John Ferlan



On 08/12/2016 09:33 AM, Jiri Denemark wrote:
> Host capabilities provide libvirt's view of the host CPU, but for a
> useful support for host-model CPUs we really need a hypervisor's view of
> the CPU. And since the view can be differ with emulator, qemu
> capabilities is the best place to store the host CPU model.
> 
> This patch just copies the CPU model from host capabilities, but this
> will change in the future.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/qemu/qemu_capabilities.c | 63 
> +---
>  src/qemu/qemu_capspriv.h |  7 -
>  tests/domaincapstest.c   | 10 +++
>  tests/qemucapabilitiestest.c |  2 +-
>  tests/qemuxml2argvtest.c |  7 +++--
>  tests/testutilsqemu.c|  5 ++--
>  tests/testutilsqemu.h|  3 ++-
>  7 files changed, 76 insertions(+), 21 deletions(-)
> 

I personally think this should be closer to patch 22 where the caps gets
introduced into virQEMUCapsNewForBinaryInternal, but it's not a review
requirement...

> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 8f55fcc..97dc877 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -374,6 +374,7 @@ struct _virQEMUCaps {
>  virArch arch;
>  
>  virDomainCapsCPUModelsPtr cpuDefinitions;
> +virCPUDefPtr cpuModel;

hostCpuModel ?

IOW: Some way to make it more clear to a casual reader and perhaps
easier to find via cscope

Also I note that this isn't being written out to the cache (e.g. no
change to virQEMUCapsFormatCache), so probably need to "note" that some
how. Furthermore, perhaps this should be moved after the gic stuff and
the note made that anything "after" this point isn't written out, rather
it's refetched during Load

>  
>  size_t nmachineTypes;
>  struct virQEMUCapsMachineType *machineTypes;
> @@ -2052,6 +2053,10 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr 
> qemuCaps)
>  goto error;
>  }
>  
> +if (qemuCaps->cpuModel &&
> +!(ret->cpuModel = virCPUDefCopy(qemuCaps->cpuModel))
> +goto error;
> +
>  if (VIR_ALLOC_N(ret->machineTypes, qemuCaps->nmachineTypes) < 0)
>  goto error;
>  ret->nmachineTypes = qemuCaps->nmachineTypes;
> @@ -2821,6 +2826,37 @@ int virQEMUCapsProbeQMP(virQEMUCapsPtr qemuCaps,
>  }
>  
>  
> +void
> +virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps,
> +virCapsHostPtr host)
> +{
> +virCPUDefPtr cpu = NULL;
> +
> +if (!virQEMUCapsGuestIsNative(host->arch, qemuCaps->arch))
> +goto error;
> +
> +if (host->cpu && host->cpu->model) {
> +if (VIR_ALLOC(cpu) < 0)
> +goto error;
> +
> +cpu->sockets = cpu->cores = cpu->threads = 0;
> +cpu->type = VIR_CPU_TYPE_GUEST;
> +cpu->mode = VIR_CPU_MODE_CUSTOM;
> +cpu->match = VIR_CPU_MATCH_EXACT;
> +
> +if (virCPUDefCopyModel(cpu, host->cpu, true) < 0)
> +goto error;
> +}
> +
> +qemuCaps->cpuModel = cpu;

This is leaked - eg. no virCPUDefFree in virQEMUCapsDispose

Since this is a void function it is possible to have a failure (above)
thus having qemuCaps->cpuModel be NULL... Store that knowledge away as
it becomes important later in patch 40 as processing will call
virQEMUCapsGetHostModel which returns this field.

> +return;
> +
> + error:
> +virCPUDefFree(cpu);
> +qemuCaps->cpuModel = NULL;

Should we virResetLastError() since we don't care about the failure in
the two callers?

> +}
> +
> +
>  /*
>   * Parsing a doc that looks like
>   *

[...]

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

Re: [libvirt] [PATCH 25/41] conf: Introduce virCPUDefCopyModelFilter

2016-08-30 Thread John Ferlan



On 08/12/2016 09:33 AM, Jiri Denemark wrote:
> The function filters all CPU features through a given callback while
> copying CPU model related parts of a CPU definition.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/conf/cpu_conf.c  | 31 ---
>  src/conf/cpu_conf.h  | 12 
>  src/libvirt_private.syms |  1 +
>  3 files changed, 37 insertions(+), 7 deletions(-)
> 
> diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
> index 13f3da3..b74c48b 100644
> --- a/src/conf/cpu_conf.c
> +++ b/src/conf/cpu_conf.c
> @@ -86,28 +86,45 @@ virCPUDefCopyModel(virCPUDefPtr dst,
> const virCPUDef *src,
> bool resetPolicy)
>  {
> +return virCPUDefCopyModelFilter(dst, src, resetPolicy, NULL, NULL);
> +}
> +
> +
> +int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)

Any particular advantage to putting the NONNULL in both the .h file and
here?  I see virCPUDefCopyModel is similar.

> +virCPUDefCopyModelFilter(virCPUDefPtr dst,
> + const virCPUDef *src,
> + bool resetPolicy,
> + virCPUDefFeatureFilter filter,
> + void *opaque)
> +{

[...]

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

Re: [libvirt] [PATCH 24/41] conf: Introduce virCPUDefMoveModel

2016-08-30 Thread John Ferlan



On 08/12/2016 09:33 AM, Jiri Denemark wrote:
> The function moves CPU model related parts from one CPU definition to
> another. It can be used to avoid unnecessary copies from a temporary CPU
> definitions which will be freed anyway.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/conf/cpu_conf.c  | 17 +
>  src/conf/cpu_conf.h  |  4 
>  src/libvirt_private.syms |  1 +
>  3 files changed, 22 insertions(+)
> 

Less "Move" and more "Take"... virCPUDefStealModel is a suggestion - of
course it's more than ->model, but close enough I think.

> diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
> index c6e847a..13f3da3 100644
> --- a/src/conf/cpu_conf.c
> +++ b/src/conf/cpu_conf.c
> @@ -115,6 +115,23 @@ virCPUDefCopyModel(virCPUDefPtr dst,
>  }
>  
>  
> +void
> +virCPUDefMoveModel(virCPUDefPtr dst,
> +   virCPUDefPtr src)
> +{
> +virCPUDefFreeModel(dst);
> +
> +VIR_STEAL_PTR(dst->model, src->model);
> +VIR_STEAL_PTR(dst->vendor, src->vendor);
> +VIR_STEAL_PTR(dst->vendor_id, src->vendor_id);
> +VIR_STEAL_PTR(dst->features, src->features);
> +dst->nfeatures_max = src->nfeatures_max;
> +src->nfeatures_max = 0;
> +dst->nfeatures = src->nfeatures;
> +src->nfeatures = 0;
> +}
> +
> +
>  virCPUDefPtr
>  virCPUDefCopyWithoutModel(const virCPUDef *cpu)
>  {
> diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
> index 2bbab9e..d866a89 100644
> --- a/src/conf/cpu_conf.h
> +++ b/src/conf/cpu_conf.h
> @@ -123,6 +123,10 @@ virCPUDefCopyModel(virCPUDefPtr dst,
> const virCPUDef *src,
> bool resetPolicy);
>  
> +void
> +virCPUDefMoveModel(virCPUDefPtr dst,
> +   virCPUDefPtr src);
> +
>  virCPUDefPtr
>  virCPUDefCopy(const virCPUDef *cpu);
>  
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 26f5bc8..1cfebd5 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -71,6 +71,7 @@ virCPUDefFormat;
>  virCPUDefFormatBuf;
>  virCPUDefFree;
>  virCPUDefFreeModel;
> +virCPUDefMoveModel;
>  virCPUDefParseXML;
>  virCPUDefUpdateFeature;
>  virCPUModeTypeToString;
> 

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

Re: [libvirt] [PATCH 23/41] conf: Introduce virCPUDefCopyWithoutModel

2016-08-30 Thread John Ferlan



On 08/12/2016 09:33 AM, Jiri Denemark wrote:
> Useful for copying a CPU definition without model related parts (i.e.,
> without model name, feature list, vendor).
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/conf/cpu_conf.c  | 16 +++-
>  src/conf/cpu_conf.h  |  3 +++
>  src/libvirt_private.syms |  1 +
>  3 files changed, 19 insertions(+), 1 deletion(-)
> '

suggestion - how about "virCPUDefSparseCopy" ?  Especially since nothing
that requires a VIR_STRDUP is copied (e.g. vendor, vendor_id, features).

> diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
> index b71528e..c6e847a 100644
> --- a/src/conf/cpu_conf.c
> +++ b/src/conf/cpu_conf.c
> @@ -114,8 +114,9 @@ virCPUDefCopyModel(virCPUDefPtr dst,
>  return 0;
>  }
>  
> +
>  virCPUDefPtr
> -virCPUDefCopy(const virCPUDef *cpu)
> +virCPUDefCopyWithoutModel(const virCPUDef *cpu)
>  {
>  virCPUDefPtr copy;
>  
> @@ -131,6 +132,18 @@ virCPUDefCopy(const virCPUDef *cpu)
>  copy->threads = cpu->threads;
>  copy->arch = cpu->arch;
>  
> +return copy;
> +}
> +
> +
> +virCPUDefPtr
> +virCPUDefCopy(const virCPUDef *cpu)
> +{
> +virCPUDefPtr copy;
> +
> +if (!(copy = virCPUDefCopyWithoutModel(cpu)))
> +return NULL;
> +
>  if (virCPUDefCopyModel(copy, cpu, false) < 0)
>  goto error;
>  
> @@ -141,6 +154,7 @@ virCPUDefCopy(const virCPUDef *cpu)
>  return NULL;
>  }
>  
> +
>  virCPUDefPtr
>  virCPUDefParseXML(xmlNodePtr node,
>xmlXPathContextPtr ctxt,
> diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h
> index 705ba6d..2bbab9e 100644
> --- a/src/conf/cpu_conf.h
> +++ b/src/conf/cpu_conf.h
> @@ -127,6 +127,9 @@ virCPUDefPtr
>  virCPUDefCopy(const virCPUDef *cpu);
>  
>  virCPUDefPtr
> +virCPUDefCopyWithoutModel(const virCPUDef *cpu);
> +
> +virCPUDefPtr
>  virCPUDefParseXML(xmlNodePtr node,
>xmlXPathContextPtr ctxt,
>virCPUType mode);
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 53d4e7f..26f5bc8 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -66,6 +66,7 @@ virCapabilitiesSetNetPrefix;
>  virCPUDefAddFeature;
>  virCPUDefCopy;
>  virCPUDefCopyModel;
> +virCPUDefCopyWithoutModel;
>  virCPUDefFormat;
>  virCPUDefFormatBuf;
>  virCPUDefFree;
> 

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

Re: [libvirt] [PATCH v5 0/8] perf: add more perf events support

2016-08-30 Thread John Ferlan



On 08/29/2016 10:00 PM, Ren, Qiaowei wrote:
> Hi John,
> 
> How about this patch series? 
> 

I made adjustments to your series, but I also made tweaks and those
needed to be OK'd/checked by someone such as you  Go back to the v4
series and read my comment. In particular from review of patch 2 and
more specifically code flow changes w/r/t the initialization which now
mostly become patch 7.

So please go through the series, make sure it "works" as you expect and
let me know.  Once that's done and 2.2 is released, I can push these. If
tweaks need to be made, then we can do so.

> Thanks,
> Qiaowei
> 
>> -Original Message-
>> From: John Ferlan [mailto:jfer...@redhat.com]
>> Sent: Thursday, August 4, 2016 6:31 AM
>> To: libvir-list@redhat.com
>> Cc: Ren, Qiaowei 
>> Subject: [PATCH v5 0/8] perf: add more perf events support
>>
>> v4: http://www.redhat.com/archives/libvir-list/2016-July/msg00607.html
>>
>> Reworked/reworded the series slightly.  The end result is mostly the same as 
>> the
>> original, but with the suggested tweaks.
>>
>>
>> John Ferlan (3):
>>   virsh: Add a forward reference to perf command from domstats --perf
>>   virsh: Rework the perf event names into a table.
>>   util: Move virPerfNew and virPerfFree
>>
>> Qiaowei Ren (5):
>>   perf: rename qemuDomainGetStatsPerfRdt()
>>   perf: Remove the switch from qemuDomainGetStatsPerf
>>   util: Add some comment details for virPerfEventType
>>   perf: Adjust the perf initialization
>>   perf: add more perf events support
>>
>>  docs/formatdomain.html.in   |  24 +++
>>  docs/schemas/domaincommon.rng   |   4 +
>>  include/libvirt/libvirt-domain.h|  39 +
>>  src/libvirt-domain.c|   9 ++
>>  src/qemu/qemu_driver.c  |  23 ++-
>>  src/util/virperf.c  | 230 
>> +---
>>  src/util/virperf.h  |  14 +-
>>  tests/genericxml2xmlindata/generic-perf.xml |   4 +
>>  tools/virsh.pod |  40 +++--
>>  9 files changed, 275 insertions(+), 112 deletions(-)
>>
>> --
>> 2.7.4
> 

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

[libvirt] [PATCH] libxl: add memory attach support

2016-08-30 Thread Bob Liu

Support for VIR_DOMAIN_DEVICE_MEMORY on domainAttachDeviceFlags API in libxl
driver, using libxl_set_memory_target in xen libxl.

With "virsh attach-device" command we can then hotplug memory to a domain:


128
0



$ virsh attach-device domain_name this.xml --live

Signed-off-by: Bob Liu 
---
 src/libxl/libxl_domain.c |  1 +
 src/libxl/libxl_driver.c | 29 +
 2 files changed, 30 insertions(+)

diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index f529a2e..3924ba0 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -414,6 +414,7 @@ virDomainDefParserConfig libxlDomainDefParserConfig = {
 .macPrefix = { 0x00, 0x16, 0x3e },
 .devicesPostParseCallback = libxlDomainDeviceDefPostParse,
 .domainPostParseCallback = libxlDomainDefPostParse,
+.features = VIR_DOMAIN_DEF_FEATURE_MEMORY_HOTPLUG
 };
 
 
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a34eb02..541ea3b 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3085,6 +3085,30 @@ libxlDomainAttachHostUSBDevice(libxlDriverPrivatePtr 
driver,
 #endif
 
 static int
+libxlDomainAttachMemory(libxlDriverPrivatePtr driver,
+virDomainObjPtr vm,
+virDomainMemoryDefPtr mem)
+{
+int res = 0;
+libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+
+if (mem->targetNode != 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("unsupport non-zero target node for memory device"));
+return -1;
+}
+
+res = libxl_set_memory_target(cfg->ctx, vm->def->id, mem->size, 1, 1);
+if (res < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Failed to attach %lluKB memory for domain %d"),
+   mem->size, vm->def->id);
+return -1;
+}
+return 0;
+}
+
+static int
 libxlDomainAttachHostDevice(libxlDriverPrivatePtr driver,
 virDomainObjPtr vm,
 virDomainHostdevDefPtr hostdev)
@@ -3284,6 +3308,11 @@ libxlDomainAttachDeviceLive(libxlDriverPrivatePtr driver,
 dev->data.hostdev = NULL;
 break;
 
+case VIR_DOMAIN_DEVICE_MEMORY:
+ret = libxlDomainAttachMemory(driver, vm, dev->data.memory);
+dev->data.memory = NULL;
+break;
+
 default:
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("device type '%s' cannot be attached"),
-- 
2.6.5

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

Re: [libvirt] Virtqueue size exceeded error when resuming VM

2016-08-30 Thread Moshe Levi

It seem that on Ubuntu they reverted the patch  See 
https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1612089

I tested on Ubuntu with  14.04 and it working 

ii  ipxe-qemu   1.0.0+git-2013.c3d1e78-2ubuntu1.1   
all  PXE boot firmware - ROM images for qemu
ii  qemu-keymaps2.0.0+dfsg-2ubuntu1.22  
all  QEMU keyboard maps
ii  qemu-kvm2.0.0+dfsg-2ubuntu1.27  
amd64QEMU Full virtualization
ii  qemu-system-common  2.0.0+dfsg-2ubuntu1.22  
amd64QEMU full system emulation binaries (common files)
ii  qemu-system-x86 2.0.0+dfsg-2ubuntu1.27 

I also test it with Ubuntu  16.04 and it working.
But on redhat  7.2  I still have the issue.
qemu-img-1.5.3-105.el7_2.7.x86_64
libvirt-daemon-driver-qemu-1.2.17-13.el7_2.5.x86_64
qemu-system-x86-2.0.0-1.el7.6.x86_64
ipxe-roms-qemu-20160127-1.git6366fa7a.el7.noarch
qemu-common-2.0.0-1.el7.6.x86_64
qemu-kvm-common-1.5.3-105.el7_2.7.x86_64
qemu-kvm-1.5.3-105.el7_2.7.x86_64
I didn't find new packages that revert the patch. Does anyone know what is the 
plan for RedHat?


> -Original Message-
> From: Moshe Levi
> Sent: Monday, August 08, 2016 2:50 PM
> To: Libvirt 
> Subject: Virtqueue size exceeded error when resuming VM
> 
> Hi,
> A new security fix [1],[2] and [3] merged to qemu.
> After updating the packages we started to get "qemu-system-x86_64:
> Virtqueue size exceeded", when resuming the guest.
> 
> Our environment is OpenStack master and we have Mellanox CI that test SR-
> IOV functionality.
> Ubuntu 14.04 with Qemu 2.0.0+dfsg-2ubuntu1.26 that contains the fixes see
> [2]
> ii  qemu-kvm2.0.0+dfsg-2ubuntu1.26
>   amd64QEMU
> Full virtualization
> ii  qemu-system-x86 2.0.0+dfsg-2ubuntu1.26
>   amd64
> QEMU full system emulation binaries (x86)
> ii  qemu-utils  2.0.0+dfsg-2ubuntu1.26
>   amd64QEMU
> utilities
> Our CI started to fail last week when this security packages released.
> 
> The scenarios is  as follows (sorry for the OpenStack commands :)) :
> 1. nova boot guest
> 2. nova suspend guest
> 3. nova resume guest
> 
> The result is that the guest is in poweroff state and when I power it on
> everything is working fine.
> 
> I tested in direct port (SR-IOV) and normal port (virtual port) and it happens
> in both cases.
> 
> 
> According to the [3] it prevent from malicious guest to submit more requests
> than the virtqueuesize permits.
> Our CI uses proprietary Cirros image with mlnx4_en driver.
> (http://13.69.151.247/images/mellanox_eth.img)
> I started to test it with other images to see if the problem with our image.
> I also tested with Ubuntu image - https://cloud-
> images.ubuntu.com/wily/current/wily-server-cloudimg-amd64-disk1.img
> And OpenStack Cirros image http://download.cirros-cloud.net/0.3.4/cirros-
> 0.3.4-x86_64-disk.img
> 
> The Ubuntu image had the same failure, but the  Cirros worked.
> 
> I wonder if there is a problem with the patch or with the images?
> What in these images can make them malicious guest?
> 
> 
> 
> [1] - https://access.redhat.com/security/cve/cve-2016-5403
> [2] - http://www.ubuntu.com/usn/usn-3047-1/
> [3] - https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg06257.html

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

[libvirt] [PATCH] docs: Add missing / to closing tag

2016-08-30 Thread Christophe Fergeau

The iothread example for virtio-scsi should be
 rather than 
for the XML to be valid.
---
I've already pushed this under the docs/trivial rule.

Christophe


 docs/formatdomain.html.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index f1dadc6..8c15a73 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3166,7 +3166,7 @@
   address type='pci' domain='0x' bus='0x00' slot='0x0a' 
function='0x0'/
 /controller
 controller type='scsi' index='0' model='virtio-scsi'
-  driver iothread='4'
+  driver iothread='4'/
   address type='pci' domain='0x' bus='0x00' slot='0x0b' 
function='0x0'/
 /controller
 ...
-- 
2.7.4

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

Re: [libvirt] [PATCH] libxl: advertise support for migration V3

2016-08-30 Thread Cedric Bosdonnat

On Mon, 2016-08-29 at 11:20 -0600, Jim Fehlig wrote:
> The libxl driver has long supported migration V3 but has never
> indicated so in the connectSupportsFeature API. As a result, apps
> such as virt-manager that use the more generic virDomainMigrate API
> fail with
> 
> libvirtError: this function is not supported by the connection driver:
> virDomainMigrate
> 
> Add VIR_DRV_FEATURE_MIGRATION_V3 to the list of features marked as
> supported in the connectSupportsFeature API.
> 
> Signed-off-by: Jim Fehlig 
> ---
>  src/libxl/libxl_driver.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index a573c82..3ffaa74 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -5677,6 +5677,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int 
> feature)
>  return -1;
>  
>  switch (feature) {
> +case VIR_DRV_FEATURE_MIGRATION_V3:
>  case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
>  case VIR_DRV_FEATURE_MIGRATION_PARAMS:
>  case VIR_DRV_FEATURE_MIGRATION_P2P:

ACK
--
Cedric

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

[libvirt] [PATCH] util: hostcpu: improve CPU freq code for FreeBSD

Re: [libvirt] [PATCH 41/41] Move CMT feature filtering to QEMU driver

Re: [libvirt] [PATCH 32/41] cpu: Make x86ModelFromCPU a bit smarter

Re: [libvirt] [PATCH 40/41] qemu: Update guest CPU def in live XML

Re: [libvirt] [PATCH 39/41] cpu: Rework cpuCompare* APIs

Re: [libvirt] [PATCH 38/41] cpu: Rework cpuHasFeature

Re: [libvirt] [PATCH 35/41] cpu: Rework cpuUpdate

[libvirt] libvirt-2.2.0 Release Candidate 2 is out

Re: [libvirt] [PATCH] libxl: advertise support for migration V3

Re: [libvirt] [PATCH] libxl: advertise support for migration V3

Re: [libvirt] [PATCH] libxl: advertise support for migration V3

Re: [libvirt] [PATCH v7 0/4] Add Mediated device support

Re: [libvirt] [PATCH] libxl: add memory attach support

Re: [libvirt] [PATCH 30/41] qemu: Introduce virQEMUCapsIsCPUModeSupported

Re: [libvirt] [PATCH 28/41] Show host model in domain capabilities

Re: [libvirt] [PATCH] libxl: advertise support for migration V3

Re: [libvirt] [PATCH 27/41] cpu: Drop false support for ARM cpu-model

Re: [libvirt] [PATCH 26/41] qemu: Store host-model CPU in qemu capabilities

Re: [libvirt] [PATCH 25/41] conf: Introduce virCPUDefCopyModelFilter

Re: [libvirt] [PATCH 24/41] conf: Introduce virCPUDefMoveModel

Re: [libvirt] [PATCH 23/41] conf: Introduce virCPUDefCopyWithoutModel

Re: [libvirt] [PATCH v5 0/8] perf: add more perf events support

[libvirt] [PATCH] libxl: add memory attach support

Re: [libvirt] Virtqueue size exceeded error when resuming VM

[libvirt] [PATCH] docs: Add missing / to closing tag

Re: [libvirt] [PATCH] libxl: advertise support for migration V3

26 matches

Site Navigation

Mail list logo

Footer information