Re: [PATCH] util: qemu_get_thread_id for OpenBSD

[Thomas Huth]

On 14/07/2020 23.26, David CARLIER wrote:
> From 9c7f54c67d40fae0174ba795fbaad829cd59c264 Mon Sep 17 00:00:00 2001
> From: David Carlier 
> Date: Tue, 14 Jul 2020 23:23:55 +0100
> Subject: [PATCH] util: qemu_get_thread_id implementation for OpenBSD.
> 
> ussage of getthrid syscall.
> 
> Signed-off-by: David Carlier 
> ---
>  util/oslib-posix.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 72907d4d7f..b4f7de83c8 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -100,6 +100,8 @@ int qemu_get_thread_id(void)
>  return (int)tid;
>  #elif defined(__NetBSD__)
>  return _lwp_self();
> +#elif defined(__OpenBSD__)
> +return getthrid();
>  #else
>  return getpid();
>  #endif
> 

 Brad,

since you're listed as OpenBSD maintainer, could you please review above
patch?

 Thanks,
  Thomas

[Bug 1886811] Re: systemd complains Failed to enqueue loopback interface start request: Operation not supported

[Ryutaroh Matsumoto]

https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1887606

** No longer affects: qemu (Ubuntu)

** Also affects: qemu (Ubuntu)
   Importance: Undecided
   Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1886811

Title:
  systemd complains Failed to enqueue loopback interface start request:
  Operation not supported

Status in QEMU:
  In Progress
Status in qemu package in Ubuntu:
  New
Status in qemu package in Debian:
  Confirmed

Bug description:
  This symptom seems similar to
  https://bugs.launchpad.net/qemu/+bug/1823790

  Host Linux: Debian 11 Bullseye (testing) on x84-64 architecture
  qemu version: latest git of git commit hash 
eb2c66b10efd2b914b56b20ae90655914310c925
  compiled with "./configure --static --disable-system" 

  Down stream bug report at 
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=964289
  Bug report (closed) to systemd: 
https://github.com/systemd/systemd/issues/16359

  systemd in armhf and armel (both little endian 32-bit) containers fail to 
start with
  Failed to enqueue loopback interface start request: Operation not supported

  How to reproduce on Debian (and probably Ubuntu):
  mmdebstrap --components="main contrib non-free" --architectures=armhf 
--variant=important bullseye /var/lib/machines/armhf-bullseye
  systemd-nspawn -D /var/lib/machines/armhf-bullseye -b

  When "armhf" architecture is replaced with "mips" (32-bit big endian) or 
"ppc64"
  (64-bit big endian), the container starts up fine.

  The same symptom is also observed with "powerpc" (32-bit big endian)
  architecture.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1886811/+subscriptions

[Bug 1887604] [NEW] Forward host UNIX socket to guest TCP port

[Ralph G]

Public bug reported:

Hello. I've been racking my brain trying to work out if this is
possible.

I would like to be able to forward to a guest TCP port, via a host UNIX
socket to avoid opening a TCP port on the host. For example:

qemu-system-i386 [...] -nic user,hostfwd=unix:/path/to/socket-:22

and then connect to the VM like:

ssh -o "ProxyCommand socat - unix-connect:/path/to/socket" user@0.0.0.0

QEMU, as versatile as it is, doesn't appreciate my intuited syntax
"hostfwd=unix:...". It is also unhappy with:

qemu-system-i386 [...] \
-chardev socket,id=foo,path=/path/to/socket,server,nowait \
-nic user,hostfwd=chardev:foo-:22

And:

qemu-system-i386 [...] \
-nic user \
-chardev socket,id=foo,path=/path/to/socket,server,nowait \
-chardev socket,id=foo,host=10.0.2.15,port=22

I already found out how to connect in the opposite direction, **from**
guest TCP to host UNIX, via guestfwd -> cmd -> socat. So I feel like
there ought to be a way.

If this is not yet a feature I would like to request it, and if it is,
please tell me how!

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: feature-request

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1887604

Title:
  Forward host UNIX socket to guest TCP port

Status in QEMU:
  New

Bug description:
  Hello. I've been racking my brain trying to work out if this is
  possible.

  I would like to be able to forward to a guest TCP port, via a host
  UNIX socket to avoid opening a TCP port on the host. For example:

  qemu-system-i386 [...] -nic user,hostfwd=unix:/path/to/socket-:22

  and then connect to the VM like:

  ssh -o "ProxyCommand socat - unix-connect:/path/to/socket"
  user@0.0.0.0

  QEMU, as versatile as it is, doesn't appreciate my intuited syntax
  "hostfwd=unix:...". It is also unhappy with:

  qemu-system-i386 [...] \
  -chardev socket,id=foo,path=/path/to/socket,server,nowait \
  -nic user,hostfwd=chardev:foo-:22

  And:

  qemu-system-i386 [...] \
  -nic user \
  -chardev socket,id=foo,path=/path/to/socket,server,nowait \
  -chardev socket,id=foo,host=10.0.2.15,port=22

  I already found out how to connect in the opposite direction, **from**
  guest TCP to host UNIX, via guestfwd -> cmd -> socat. So I feel like
  there ought to be a way.

  If this is not yet a feature I would like to request it, and if it is,
  please tell me how!

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1887604/+subscriptions

[PATCH] docs/nvdimm: add 'pmem=on' for the device dax backend file

[Jingqi Liu]

At the end of live migration, QEMU uses msync() to flush the data to
the backend storage. When the backend file is a character device dax,
the pages explicitly avoid the page cache. It will return failure from msync().
The following warning is output.

"warning: qemu_ram_msync: failed to sync memory range“

So we add 'pmem=on' to avoid calling msync(), use the QEMU command line:

-object memory-backend-file,id=mem1,pmem=on,mem-path=/dev/dax0.0,size=4G

Signed-off-by: Jingqi Liu 
---
 docs/nvdimm.txt | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt
index c2c6e441b3..31048aff5e 100644
--- a/docs/nvdimm.txt
+++ b/docs/nvdimm.txt
@@ -243,6 +243,13 @@ use the QEMU command line:
 
 -object memory-backend-file,id=nv_mem,mem-path=/XXX/yyy,size=4G,pmem=on
 
+At the end of live migration, QEMU uses msync() to flush the data to the
+backend storage. When the backend file is a character device dax, the pages
+explicitly avoid the page cache. It will return failure from msync().
+So we add 'pmem=on' to avoid calling msync(), use the QEMU command line:
+
+-object memory-backend-file,id=mem1,pmem=on,mem-path=/dev/dax0.0,size=4G
+
 References
 --
 
-- 
2.17.1

Re: [RFC 05/65] target/riscv: remove vsll.vi, vsrl.vi, vsra.vi insns from using gvec

[LIU Zhiwei]

On 2020/7/14 21:59, Frank Chang wrote:
On Tue, Jul 14, 2020 at 9:21 PM Richard Henderson 
mailto:richard.hender...@linaro.org>> 
wrote:


On 7/13/20 7:59 PM, Frank Chang wrote:
> The latest spec specified:
>
> Only the low *lg2(SEW) bits* are read to obtain the shift amount
from a
> *register value*.
> The *immediate* is treated as an *unsigned shift amount*, with a
*maximum shift
> amount of 31*.

Which, I hope you will agree is underspecified, and should be
reported as a bug
in the manual.


Yes, you're correct.
I found out I missed the MASK part in GEN_VEXT_SHIFT_VV() macro,
which this macro is shared between OPIVV and OPIVI format of instructions.
So the same masking methodology should be applied to shift amounts 
coming from both register and immediate value.

Spike also does something like:
/vs2 >> (zimm5 & (sew - 1) & 0x1f);/ for SEW = 8.

I think spec is kind of misleading to the reader by the way it expresses.


> Looks like the shift amount in the immediate value is not
relevant with SEW
> setting.

How can it not be?  It is when the value comes from a register...

> If so, is it better to just use do_opivi_gvec() and implement
the logic by our
> own rather than using gvec IR?

No, it is not.  What is the logic you would apply on your own? 
There should be
a right answer.


I was talking about just calling GEN_OPIVI_TRANS() to generate the 
helper functions

defined in vector_helper.c as what I'm doing in the original patch.
But as long as the immediate value should also apply *lg2(SEW) bits* 
truncating,

I think I should update GEN_OPIVI_GVEC_TRANS() to utilize gvec IR.


If the answer is that out-of-range shift produces zero, which some
architectures use, then you can look at the immediate value, see
that you must
supply zero, and then fill the vector with zeros from translate. 
You need not
call a helper to perform N shifts when you know the result a-priori.

If the answer is that shift values are truncated, which riscv uses
*everywhere
else*, then you should truncate the immediate value during translate.


I think vsll.vi , vsrl.vi  and vsra.vi 
 truncate the out-of-range shift as other riscv 
instructions.

I will double confirm that, thanks for the advice.

Perhaps the reason is that the assembler can't identify if an imm is 
legal according to log(SEW),

because the assembler can't get the SEW.

To make more compliance with assembler directly users'  intuition, just 
let the imm encoding to insn as itself(truncate to 31)

and work as itself.

Zhiwei

Frank Chang



r~

Re: [PATCH-for-5.1 3/4] qemu-common: Document qemu_find_file()

[Li Qiang]

Philippe Mathieu-Daudé  于2020年7月15日周三 上午12:48写道：
>
> Document qemu_find_file(), in particular the returned
> value which must be freed.
>
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Li Qiang 

> ---
>  include/qemu-common.h | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index d0142f29ac..d6a08259d3 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -110,6 +110,20 @@ const char *qemu_get_vm_name(void);
>
>  #define QEMU_FILE_TYPE_BIOS   0
>  #define QEMU_FILE_TYPE_KEYMAP 1
> +/**
> + * qemu_find_file:
> + * @type: QEMU_FILE_TYPE_BIOS (for BIOS, VGA BIOS)
> + *or QEMU_FILE_TYPE_KEYMAP (for keymaps).
> + * @name: File name
> + *
> + * Search for @name file in the data directories, either configured at
> + * build time (DATADIR) or registered with the -L command line option.
> + *
> + * The caller must use g_free() to free the returned data when it is
> + * no longer required.
> + *
> + * Returns: absolute path to the file or NULL on error.
> + */
>  char *qemu_find_file(int type, const char *name);
>
>  /* OS specific functions */
> --
> 2.21.3
>
>

Re: [PATCH v5 04/11] hw/arm: Add NPCM730 and NPCM750 SoC models

[Havard Skinnemoen]

On Tue, Jul 14, 2020 at 10:11 AM Philippe Mathieu-Daudé  wrote:
>
> On 7/14/20 6:01 PM, Markus Armbruster wrote:
> > Philippe Mathieu-Daudé  writes:
> >
> >> +Markus
> >>
> >> On 7/14/20 2:44 AM, Havard Skinnemoen wrote:
> >>> On Mon, Jul 13, 2020 at 8:02 AM Cédric Le Goater  wrote:
> 
>  On 7/9/20 2:36 AM, Havard Skinnemoen wrote:
> > The Nuvoton NPCM7xx SoC family are used to implement Baseboard
> > Management Controllers in servers. While the family includes four SoCs,
> > this patch implements limited support for two of them: NPCM730 (targeted
> > for Data Center applications) and NPCM750 (targeted for Enterprise
> > applications).
> >
> > This patch includes little more than the bare minimum needed to boot a
> > Linux kernel built with NPCM7xx support in direct-kernel mode:
> >
> >   - Two Cortex-A9 CPU cores with built-in periperhals.
> >   - Global Configuration Registers.
> >   - Clock Management.
> >   - 3 Timer Modules with 5 timers each.
> >   - 4 serial ports.
> >
> > The chips themselves have a lot more features, some of which will be
> > added to the model at a later stage.
> >
> > Reviewed-by: Tyrone Ting 
> > Reviewed-by: Joel Stanley 
> > Signed-off-by: Havard Skinnemoen 
> > ---
> >> ...
> >>
> > +static void npcm7xx_realize(DeviceState *dev, Error **errp)
> > +{
> > +NPCM7xxState *s = NPCM7XX(dev);
> > +NPCM7xxClass *nc = NPCM7XX_GET_CLASS(s);
> > +int i;
> > +
> > +/* CPUs */
> > +for (i = 0; i < nc->num_cpus; i++) {
> > +object_property_set_int(OBJECT(>cpu[i]),
> > +arm_cpu_mp_affinity(i, 
> > NPCM7XX_MAX_NUM_CPUS),
> > +"mp-affinity", _abort);
> > +object_property_set_int(OBJECT(>cpu[i]), 
> > NPCM7XX_GIC_CPU_IF_ADDR,
> > +"reset-cbar", _abort);
> > +object_property_set_bool(OBJECT(>cpu[i]), true,
> > + "reset-hivecs", _abort);
> > +
> > +/* Disable security extensions. */
> > +object_property_set_bool(OBJECT(>cpu[i]), false, "has_el3",
> > + _abort);
> > +
> > +qdev_realize(DEVICE(>cpu[i]), NULL, _abort);
> 
>  I would check the error:
> 
>  if (!qdev_realize(DEVICE(>cpu[i]), NULL, errp)) {
>  return;
>  }
> 
>  same for the sysbus_realize() below.
> >>>
> >>> Hmm, I used to propagate these errors until Philippe told me not to
> >>> (or at least that's how I understood it).
> >>
> >> It was before Markus simplification API were merged, you had to
> >> propagate after each call, since this is a non hot-pluggable SoC
> >> I suggested to use _abort to simplify.
> >>
> >>> I'll be happy to do it
> >>> either way (and the new API makes it really easy to propagate errors),
> >>> but I worry that I don't fully understand when to propagate errors and
> >>> when not to.
> >>
> >> Markus explained it on the mailing list recently (as I found the doc
> >> not obvious). I can't find the thread. I suppose once the work result
> >> after the "Questionable aspects of QEMU Error's design" discussion is
> >> merged, the documentation will be clarified.
> >
> > The Error API evolved recently.  Please peruse the big comment in
> > include/qapi/error.h.  If still unsure, don't hesitate to ask here.
> >
> >> My rule of thumb so far is:
> >> - programming error (can't happen) -> _abort
> >
> > Correct.  Quote the big comment:
> >
> >  * Call a function aborting on errors:
> >  * foo(arg, _abort);
> >  * This is more concise and fails more nicely than
> >  * Error *err = NULL;
> >  * foo(arg, );
> >  * assert(!err); // don't do this
> >
> >> - everything triggerable by user or management layer (via QMP command)
> >>   -> _fatal, as we can't risk loose the user data, we need to
> >>   shutdown gracefully.
> >
> > Quote the big comment:
> >
> >  * Call a function treating errors as fatal:
> >  * foo(arg, _fatal);
> >  * This is more concise than
> >  * Error *err = NULL;
> >  * foo(arg, );
> >  * if (err) { // don't do this
> >  * error_report_err(err);
> >  * exit(1);
> >  * }
> >
> > Terminating the process is generally fine during initial startup,
> > i.e. before the guest runs.
> >
> > It's generally not fine once the guest runs.  Errors need to be handled
> > more gracefully then.  A QMP command, for instance, should fail cleanly,
> > propagating the error to the monitor core, which then sends it to the
> > QMP client, and loops to process the next command.
> >
> >>> It makes sense to me to propagate errors from *_realize() and
> >>> error_abort on failure to set simple properties, but I'd like to know
> >>> if Philippe is on board with that.
> >
> > Realize methods must not use 

[PATCH] ppc/spapr: Fix 32 bit logical memory block size assumptions

[Anton Blanchard]

When testing large LMB sizes (eg 4GB), I found a couple of places
that assume they are 32bit in size.

Signed-off-by: Anton Blanchard 
---
 hw/ppc/spapr.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a1b06defe6..0ba2526215 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -558,7 +558,8 @@ static int 
spapr_dt_dynamic_reconfiguration_memory(SpaprMachineState *spapr,
 int nb_numa_nodes = machine->numa_state->num_nodes;
 int ret, i, offset;
 uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
-uint32_t prop_lmb_size[] = {0, cpu_to_be32(lmb_size)};
+uint32_t prop_lmb_size[] = {cpu_to_be32(lmb_size >> 32),
+cpu_to_be32(lmb_size & 0x)};
 uint32_t *int_buf, *cur_index, buf_len;
 int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
 MemoryDeviceInfoList *dimms = NULL;
@@ -899,7 +900,8 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void 
*fdt)
 uint32_t lrdr_capacity[] = {
 cpu_to_be32(max_device_addr >> 32),
 cpu_to_be32(max_device_addr & 0x),
-0, cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE),
+cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE >> 32),
+cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE & 0x),
 cpu_to_be32(ms->smp.max_cpus / ms->smp.threads),
 };
 uint32_t maxdomain = cpu_to_be32(spapr->gpu_numa_id > 1 ? 1 : 0);
-- 
2.26.2

[PULL 18/19] python/qmp.py: add casts to JSON deserialization

[Philippe Mathieu-Daudé]

From: John Snow 

mypy and python type hints are not powerful enough to properly describe
JSON messages in Python 3.6. The best we can do, generally, is describe
them as Dict[str, Any].

Add casts to coerce this type for static analysis; but do NOT enforce
this type at runtime in any way.

Note: Python 3.8 adds a TypedDict construct which allows for the
description of more arbitrary Dictionary shapes. There is a third-party
module, "Pydantic", which is compatible with 3.6 that can be used
instead of the JSON library that parses JSON messages to fully-typed
Python objects, and may be preferable in some cases.

(That is well beyond the scope of this commit or series.)

Signed-off-by: John Snow 
Reviewed-by: Kevin Wolf 
Message-Id: <20200710052220.3306-6-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 python/qemu/qmp.py | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/python/qemu/qmp.py b/python/qemu/qmp.py
index ef3c919b76..1ae36050a4 100644
--- a/python/qemu/qmp.py
+++ b/python/qemu/qmp.py
@@ -13,6 +13,7 @@
 import logging
 from typing import (
 Any,
+cast,
 Dict,
 Optional,
 TextIO,
@@ -130,7 +131,10 @@ def __json_read(self, only_event=False):
 data = self.__sockfile.readline()
 if not data:
 return None
-resp = json.loads(data)
+# By definition, any JSON received from QMP is a QMPMessage,
+# and we are asserting only at static analysis time that it
+# has a particular shape.
+resp: QMPMessage = json.loads(data)
 if 'event' in resp:
 self.logger.debug("<<< %s", resp)
 self.__events.append(resp)
@@ -262,7 +266,7 @@ def command(self, cmd, **kwds):
 ret = self.cmd(cmd, kwds)
 if 'error' in ret:
 raise QMPResponseError(ret)
-return ret['return']
+return cast(QMPReturnValue, ret['return'])
 
 def pull_event(self, wait=False):
 """
-- 
2.21.3

[PULL 19/19] python/qmp.py: add QMPProtocolError

[Philippe Mathieu-Daudé]

From: John Snow 

In the case that we receive a reply but are unable to understand it,
use this exception name to indicate that case.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Kevin Wolf 
Message-Id: <20200710052220.3306-7-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 python/qemu/qmp.py | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/python/qemu/qmp.py b/python/qemu/qmp.py
index 1ae36050a4..7935dababb 100644
--- a/python/qemu/qmp.py
+++ b/python/qemu/qmp.py
@@ -62,6 +62,12 @@ class QMPTimeoutError(QMPError):
 """
 
 
+class QMPProtocolError(QMPError):
+"""
+QMP protocol error; unexpected response
+"""
+
+
 class QMPResponseError(QMPError):
 """
 Represents erroneous QMP monitor reply
@@ -266,6 +272,10 @@ def command(self, cmd, **kwds):
 ret = self.cmd(cmd, kwds)
 if 'error' in ret:
 raise QMPResponseError(ret)
+if 'return' not in ret:
+raise QMPProtocolError(
+"'return' key not found in QMP response '{}'".format(str(ret))
+)
 return cast(QMPReturnValue, ret['return'])
 
 def pull_event(self, wait=False):
-- 
2.21.3

[PULL 15/19] iotests.py: use qemu.qmp type aliases

[Philippe Mathieu-Daudé]

From: John Snow 

iotests.py should use the type definitions from qmp.py instead of its
own.

Signed-off-by: John Snow 
Reviewed-by: Kevin Wolf 
Message-Id: <20200710052220.3306-3-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/qemu-iotests/iotests.py | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 8b760405ee..3590ed78a0 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -35,13 +35,10 @@
 # pylint: disable=import-error, wrong-import-position
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu import qtest
+from qemu.qmp import QMPMessage
 
 assert sys.version_info >= (3, 6)
 
-# Type Aliases
-QMPResponse = Dict[str, Any]
-
-
 # Use this logger for logging messages directly from the iotests module
 logger = logging.getLogger('qemu.iotests')
 logger.addHandler(logging.NullHandler())
@@ -561,7 +558,7 @@ def add_incoming(self, addr):
 self._args.append(addr)
 return self
 
-def hmp(self, command_line: str, use_log: bool = False) -> QMPResponse:
+def hmp(self, command_line: str, use_log: bool = False) -> QMPMessage:
 cmd = 'human-monitor-command'
 kwargs = {'command-line': command_line}
 if use_log:
@@ -582,7 +579,7 @@ def resume_drive(self, drive: str) -> None:
 self.hmp(f'qemu-io {drive} "remove_break bp_{drive}"')
 
 def hmp_qemu_io(self, drive: str, cmd: str,
-use_log: bool = False) -> QMPResponse:
+use_log: bool = False) -> QMPMessage:
 """Write to a given drive using an HMP command"""
 return self.hmp(f'qemu-io {drive} "{cmd}"', use_log=use_log)
 
-- 
2.21.3

[PULL 12/19] python/machine.py: re-add sigkill warning suppression

[Philippe Mathieu-Daudé]

From: John Snow 

If the user kills QEMU on purpose, we don't need to warn
them about that having happened: they know already.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Cleber Rosa 
Message-Id: <20200710050649.32434-12-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 python/qemu/machine.py | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index a955e3f221..736a3c906f 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -22,6 +22,7 @@
 import os
 import subprocess
 import shutil
+import signal
 import socket
 import tempfile
 from typing import Optional, Type
@@ -133,6 +134,7 @@ def __init__(self, binary, args=None, wrapper=None, 
name=None,
 self._console_address = None
 self._console_socket = None
 self._remove_files = []
+self._user_killed = False
 self._console_log_path = console_log
 if self._console_log_path:
 # In order to log the console, buffering needs to be enabled.
@@ -327,7 +329,8 @@ def _post_shutdown(self):
 self._remove_if_exists(self._remove_files.pop())
 
 exitcode = self.exitcode()
-if exitcode is not None and exitcode < 0:
+if (exitcode is not None and exitcode < 0
+and not (self._user_killed and exitcode == -signal.SIGKILL)):
 msg = 'qemu received signal %i; command: "%s"'
 if self._qemu_full_args:
 command = ' '.join(self._qemu_full_args)
@@ -335,6 +338,7 @@ def _post_shutdown(self):
 command = ''
 LOG.warning(msg, -int(exitcode), command)
 
+self._user_killed = False
 self._launched = False
 
 def launch(self):
@@ -469,6 +473,7 @@ def shutdown(self, has_quit: bool = False,
 
 try:
 if hard:
+self._user_killed = True
 self._hard_shutdown()
 else:
 self._do_shutdown(has_quit, timeout=timeout)
-- 
2.21.3

[PULL 16/19] python/qmp.py: re-absorb MonitorResponseError

[Philippe Mathieu-Daudé]

From: John Snow 

When I initially split this out, I considered this more of a machine
error than a QMP protocol error, but I think that's misguided.

Move this back to qmp.py and name it QMPResponseError. Convert
qmp.command() to use this exception type.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Kevin Wolf 
Message-Id: <20200710052220.3306-4-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 python/qemu/machine.py| 15 +--
 python/qemu/qmp.py| 17 +++--
 scripts/render_block_graph.py |  7 +--
 3 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 69055189bd..80c4d4a8b6 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -56,19 +56,6 @@ class AbnormalShutdown(QEMUMachineError):
 """
 
 
-class MonitorResponseError(qmp.QMPError):
-"""
-Represents erroneous QMP monitor reply
-"""
-def __init__(self, reply):
-try:
-desc = reply["error"]["desc"]
-except KeyError:
-desc = reply
-super().__init__(desc)
-self.reply = reply
-
-
 class QEMUMachine:
 """
 A QEMU VM
@@ -533,7 +520,7 @@ def command(self, cmd, conv_keys=True, **args):
 if reply is None:
 raise qmp.QMPError("Monitor is closed")
 if "error" in reply:
-raise MonitorResponseError(reply)
+raise qmp.QMPResponseError(reply)
 return reply["return"]
 
 def get_qmp_event(self, wait=False):
diff --git a/python/qemu/qmp.py b/python/qemu/qmp.py
index 8388c7b603..aa8a666b8a 100644
--- a/python/qemu/qmp.py
+++ b/python/qemu/qmp.py
@@ -61,6 +61,19 @@ class QMPTimeoutError(QMPError):
 """
 
 
+class QMPResponseError(QMPError):
+"""
+Represents erroneous QMP monitor reply
+"""
+def __init__(self, reply: QMPMessage):
+try:
+desc = reply['error']['desc']
+except KeyError:
+desc = reply
+super().__init__(desc)
+self.reply = reply
+
+
 class QEMUMonitorProtocol:
 """
 Provide an API to connect to QEMU via QEMU Monitor Protocol (QMP) and then
@@ -251,8 +264,8 @@ def command(self, cmd, **kwds):
 Build and send a QMP command to the monitor, report errors if any
 """
 ret = self.cmd(cmd, kwds)
-if "error" in ret:
-raise Exception(ret['error']['desc'])
+if 'error' in ret:
+raise QMPResponseError(ret)
 return ret['return']
 
 def pull_event(self, wait=False):
diff --git a/scripts/render_block_graph.py b/scripts/render_block_graph.py
index 409b4321f2..da6acf050d 100755
--- a/scripts/render_block_graph.py
+++ b/scripts/render_block_graph.py
@@ -25,7 +25,10 @@
 from graphviz import Digraph
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', 'python'))
-from qemu.machine import MonitorResponseError
+from qemu.qmp import (
+QEMUMonitorProtocol,
+QMPResponseError,
+)
 
 
 def perm(arr):
@@ -102,7 +105,7 @@ def command(self, cmd):
 reply = json.loads(subprocess.check_output(ar))
 
 if 'error' in reply:
-raise MonitorResponseError(reply)
+raise QMPResponseError(reply)
 
 return reply['return']
 
-- 
2.21.3

[PULL 17/19] python/qmp.py: Do not return None from cmd_obj

[Philippe Mathieu-Daudé]

From: John Snow 

This makes typing the qmp library difficult, as it necessitates wrapping
Optional[] around the type for every return type up the stack. At some
point, it becomes difficult to discern or remember why it's None instead
of the expected object.

Use the python exception system to tell us exactly why we didn't get an
object. Remove this special-cased return.

Signed-off-by: John Snow 
Reviewed-by: Kevin Wolf 
Message-Id: <20200710052220.3306-5-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 python/qemu/qmp.py | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/python/qemu/qmp.py b/python/qemu/qmp.py
index aa8a666b8a..ef3c919b76 100644
--- a/python/qemu/qmp.py
+++ b/python/qemu/qmp.py
@@ -225,22 +225,18 @@ def accept(self, timeout=15.0):
 self.__sockfile = self.__sock.makefile(mode='r')
 return self.__negotiate_capabilities()
 
-def cmd_obj(self, qmp_cmd):
+def cmd_obj(self, qmp_cmd: QMPMessage) -> QMPMessage:
 """
 Send a QMP command to the QMP Monitor.
 
 @param qmp_cmd: QMP command to be sent as a Python dict
-@return QMP response as a Python dict or None if the connection has
-been closed
+@return QMP response as a Python dict
 """
 self.logger.debug(">>> %s", qmp_cmd)
-try:
-self.__sock.sendall(json.dumps(qmp_cmd).encode('utf-8'))
-except OSError as err:
-if err.errno == errno.EPIPE:
-return None
-raise err
+self.__sock.sendall(json.dumps(qmp_cmd).encode('utf-8'))
 resp = self.__json_read()
+if resp is None:
+raise QMPConnectError("Unexpected empty reply from server")
 self.logger.debug("<<< %s", resp)
 return resp
 
-- 
2.21.3

[PULL 13/19] python/machine.py: change default wait timeout to 3 seconds

[Philippe Mathieu-Daudé]

From: John Snow 

Machine.wait() does not appear to be used except in the acceptance tests,
and an infinite timeout by default in a test suite is not the most helpful.

Change it to 3 seconds, like the default shutdown timeout.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-Id: <20200710050649.32434-13-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 python/qemu/machine.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 736a3c906f..69055189bd 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -486,12 +486,12 @@ def kill(self):
 """
 self.shutdown(hard=True)
 
-def wait(self, timeout: Optional[int] = None) -> None:
+def wait(self, timeout: Optional[int] = 3) -> None:
 """
 Wait for the VM to power off and perform post-shutdown cleanup.
 
 :param timeout: Optional timeout in seconds.
-Default None, an infinite wait.
+Default 3 seconds, A value of None is an infinite wait.
 """
 self.shutdown(has_quit=True, timeout=timeout)
 
-- 
2.21.3

[PULL 03/19] python/machine.py: Close QMP socket in cleanup

[Philippe Mathieu-Daudé]

From: John Snow 

It's not important to do this before waiting for the process to exit, so
it can be done during generic post-shutdown cleanup.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-Id: <20200710050649.32434-3-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 python/qemu/machine.py | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index ca1f2114e6..d3faa9a84c 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -294,6 +294,10 @@ def _post_launch(self):
 self._qmp.accept()
 
 def _post_shutdown(self):
+if self._qmp:
+self._qmp.close()
+self._qmp = None
+
 self._load_io_log()
 
 if self._qemu_log_file is not None:
@@ -366,8 +370,6 @@ def wait(self):
 Wait for the VM to power off
 """
 self._popen.wait()
-if self._qmp:
-self._qmp.close()
 self._post_shutdown()
 
 def shutdown(self, has_quit=False, hard=False):
@@ -388,7 +390,6 @@ def shutdown(self, has_quit=False, hard=False):
 try:
 if not has_quit:
 self._qmp.cmd('quit')
-self._qmp.close()
 self._popen.wait(timeout=3)
 except:
 self._popen.kill()
-- 
2.21.3

[PULL 09/19] tests/acceptance: wait() instead of shutdown() where appropriate

[Philippe Mathieu-Daudé]

From: John Snow 

When issuing 'reboot' to a VM with the no-reboot option, that VM will
exit. When then issuing a shutdown command, the cleanup may race.

Add calls to vm.wait() which will gracefully mark the VM as having
exited. Subsequent vm.shutdown() calls in generic tearDown code will not
race when called after completion of the call.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-Id: <20200710050649.32434-9-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/acceptance/boot_linux_console.py   | 10 ++
 tests/acceptance/linux_ssh_mips_malta.py |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/tests/acceptance/boot_linux_console.py 
b/tests/acceptance/boot_linux_console.py
index 3d02519660..5867ef760c 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -191,6 +191,8 @@ def test_mips_malta_cpio(self):
 'Debian')
 exec_command_and_wait_for_pattern(self, 'reboot',
 'reboot: Restarting system')
+# Wait for VM to shut down gracefully
+self.vm.wait()
 
 @skipUnless(os.getenv('AVOCADO_ALLOW_UNTRUSTED_CODE'), 'untrusted code')
 def test_mips64el_malta_5KEc_cpio(self):
@@ -231,6 +233,8 @@ def test_mips64el_malta_5KEc_cpio(self):
 '3.19.3.mtoman.20150408')
 exec_command_and_wait_for_pattern(self, 'reboot',
 'reboot: Restarting system')
+# Wait for VM to shut down gracefully
+self.vm.wait()
 
 def do_test_mips_malta32el_nanomips(self, kernel_url, kernel_hash):
 kernel_path_xz = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
@@ -506,6 +510,7 @@ def test_arm_cubieboard_initrd(self):
 'system-control@1c0')
 exec_command_and_wait_for_pattern(self, 'reboot',
 'reboot: Restarting system')
+# NB: Do not issue vm.wait() here, cubieboard's reboot does not exit!
 
 def test_arm_cubieboard_sata(self):
 """
@@ -550,6 +555,7 @@ def test_arm_cubieboard_sata(self):
 'sda')
 exec_command_and_wait_for_pattern(self, 'reboot',
 'reboot: Restarting system')
+# NB: Do not issue vm.wait() here, cubieboard's reboot does not exit!
 
 def test_arm_orangepi(self):
 """
@@ -615,6 +621,8 @@ def test_arm_orangepi_initrd(self):
 'system-control@1c0')
 exec_command_and_wait_for_pattern(self, 'reboot',
 'reboot: Restarting system')
+# Wait for VM to shut down gracefully
+self.vm.wait()
 
 def test_arm_orangepi_sd(self):
 """
@@ -662,6 +670,8 @@ def test_arm_orangepi_sd(self):
 '3 packets transmitted, 3 packets received, 0% packet loss')
 exec_command_and_wait_for_pattern(self, 'reboot',
 'reboot: Restarting system')
+# Wait for VM to shut down gracefully
+self.vm.wait()
 
 @skipUnless(os.getenv('AVOCADO_ALLOW_LARGE_STORAGE'), 'storage limited')
 @skipUnless(P7ZIP_AVAILABLE, '7z not installed')
diff --git a/tests/acceptance/linux_ssh_mips_malta.py 
b/tests/acceptance/linux_ssh_mips_malta.py
index 90d7f2f167..25c5c5f741 100644
--- a/tests/acceptance/linux_ssh_mips_malta.py
+++ b/tests/acceptance/linux_ssh_mips_malta.py
@@ -212,6 +212,8 @@ def check_mips_malta(self, uname_m, endianess):
 
 self.run_common_commands(wordsize)
 self.shutdown_via_ssh()
+# Wait for VM to shut down gracefully
+self.vm.wait()
 
 def test_mips_malta32eb_kernel3_2_0(self):
 """
-- 
2.21.3

[PULL 14/19] python/qmp.py: Define common types

[Philippe Mathieu-Daudé]

From: John Snow 

Define some common types that we'll need to annotate a lot of other
functions going forward.

Signed-off-by: John Snow 
Reviewed-by: Kevin Wolf 
Message-Id: <20200710052220.3306-2-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 python/qemu/qmp.py | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/python/qemu/qmp.py b/python/qemu/qmp.py
index e64b6b5faa..8388c7b603 100644
--- a/python/qemu/qmp.py
+++ b/python/qemu/qmp.py
@@ -12,13 +12,31 @@
 import socket
 import logging
 from typing import (
+Any,
+Dict,
 Optional,
 TextIO,
 Type,
+Tuple,
+Union,
 )
 from types import TracebackType
 
 
+# QMPMessage is a QMP Message of any kind.
+# e.g. {'yee': 'haw'}
+#
+# QMPReturnValue is the inner value of return values only.
+# {'return': {}} is the QMPMessage,
+# {} is the QMPReturnValue.
+QMPMessage = Dict[str, Any]
+QMPReturnValue = Dict[str, Any]
+
+InternetAddrT = Tuple[str, str]
+UnixAddrT = str
+SocketAddrT = Union[InternetAddrT, UnixAddrT]
+
+
 class QMPError(Exception):
 """
 QMP base exception
-- 
2.21.3

[PULL 10/19] tests/acceptance: Don't test reboot on cubieboard

[Philippe Mathieu-Daudé]

From: John Snow 

cubieboard does not have a functioning reboot, it halts and QEMU does
not exit.

vm.shutdown() is modified in a forthcoming patch that makes it less tolerant
of race conditions on shutdown; tests should consciously decide to WAIT
or to SHUTDOWN qemu.

So long as this test is attempting to reboot, the correct choice would
be to WAIT for the VM to exit. However, since that's broken, we should
SHUTDOWN instead.

SHUTDOWN is indeed what already happens when the test performs teardown,
however, if anyone fixes cubieboard reboot in the future, this test will
develop a new race condition that might be hard to debug.

Therefore: remove the reboot test and make it obvious that the VM is
still running when the test concludes, where the test teardown will do
the right thing.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-Id: <20200710050649.32434-10-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/acceptance/boot_linux_console.py | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/tests/acceptance/boot_linux_console.py 
b/tests/acceptance/boot_linux_console.py
index 5867ef760c..8b8b828bc5 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -508,9 +508,7 @@ def test_arm_cubieboard_initrd(self):
 'Allwinner sun4i/sun5i')
 exec_command_and_wait_for_pattern(self, 'cat /proc/iomem',
 'system-control@1c0')
-exec_command_and_wait_for_pattern(self, 'reboot',
-'reboot: Restarting system')
-# NB: Do not issue vm.wait() here, cubieboard's reboot does not exit!
+# cubieboard's reboot is not functioning; omit reboot test.
 
 def test_arm_cubieboard_sata(self):
 """
@@ -553,9 +551,7 @@ def test_arm_cubieboard_sata(self):
 'Allwinner sun4i/sun5i')
 exec_command_and_wait_for_pattern(self, 'cat /proc/partitions',
 'sda')
-exec_command_and_wait_for_pattern(self, 'reboot',
-'reboot: Restarting system')
-# NB: Do not issue vm.wait() here, cubieboard's reboot does not exit!
+# cubieboard's reboot is not functioning; omit reboot test.
 
 def test_arm_orangepi(self):
 """
-- 
2.21.3

[PULL 02/19] python/machine.py: consolidate _post_shutdown()

[Philippe Mathieu-Daudé]

From: John Snow 

Move more cleanup actions into _post_shutdown. As a change, if QEMU
should so happen to be terminated during a call to wait(), that event
will now be logged.

This is not likely to occur during normative use.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20200710050649.32434-2-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 python/qemu/machine.py | 27 +--
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index c25f0b42cf..ca1f2114e6 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -294,6 +294,8 @@ def _post_launch(self):
 self._qmp.accept()
 
 def _post_shutdown(self):
+self._load_io_log()
+
 if self._qemu_log_file is not None:
 self._qemu_log_file.close()
 self._qemu_log_file = None
@@ -307,6 +309,17 @@ def _post_shutdown(self):
 while len(self._remove_files) > 0:
 self._remove_if_exists(self._remove_files.pop())
 
+exitcode = self.exitcode()
+if exitcode is not None and exitcode < 0:
+msg = 'qemu received signal %i; command: "%s"'
+if self._qemu_full_args:
+command = ' '.join(self._qemu_full_args)
+else:
+command = ''
+LOG.warning(msg, -int(exitcode), command)
+
+self._launched = False
+
 def launch(self):
 """
 Launch the VM and make sure we cleanup and expose the
@@ -355,7 +368,6 @@ def wait(self):
 self._popen.wait()
 if self._qmp:
 self._qmp.close()
-self._load_io_log()
 self._post_shutdown()
 
 def shutdown(self, has_quit=False, hard=False):
@@ -382,21 +394,8 @@ def shutdown(self, has_quit=False, hard=False):
 self._popen.kill()
 self._popen.wait()
 
-self._load_io_log()
 self._post_shutdown()
 
-exitcode = self.exitcode()
-if exitcode is not None and exitcode < 0 and \
-not (exitcode == -9 and hard):
-msg = 'qemu received signal %i: %s'
-if self._qemu_full_args:
-command = ' '.join(self._qemu_full_args)
-else:
-command = ''
-LOG.warning(msg, -int(exitcode), command)
-
-self._launched = False
-
 def kill(self):
 self.shutdown(hard=True)
 
-- 
2.21.3

[PULL 06/19] python/machine.py: Prohibit multiple shutdown() calls

[Philippe Mathieu-Daudé]

From: John Snow 

If the VM is not launched, don't try to shut it down. As a change,
_post_shutdown now unconditionally also calls _early_cleanup in order to
offer comprehensive object cleanup in failure cases.

As a courtesy, treat it as a NOP instead of rejecting it as an
error. This is slightly nicer for acceptance tests where vm.shutdown()
is issued unconditionally in tearDown callbacks.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20200710050649.32434-6-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 python/qemu/machine.py | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 63e40879e2..c28957ee82 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -294,6 +294,13 @@ def _post_launch(self):
 self._qmp.accept()
 
 def _post_shutdown(self):
+"""
+Called to cleanup the VM instance after the process has exited.
+May also be called after a failed launch.
+"""
+# Comprehensive reset for the failed launch case:
+self._early_cleanup()
+
 if self._qmp:
 self._qmp.close()
 self._qmp = None
@@ -339,7 +346,7 @@ def launch(self):
 self._launch()
 self._launched = True
 except:
-self.shutdown()
+self._post_shutdown()
 
 LOG.debug('Error launching VM')
 if self._qemu_full_args:
@@ -368,6 +375,8 @@ def _launch(self):
 def _early_cleanup(self) -> None:
 """
 Perform any cleanup that needs to happen before the VM exits.
+
+Called additionally by _post_shutdown for comprehensive cleanup.
 """
 # If we keep the console socket open, we may deadlock waiting
 # for QEMU to exit, while QEMU is waiting for the socket to
@@ -388,6 +397,9 @@ def shutdown(self, has_quit=False, hard=False):
 """
 Terminate the VM and clean up
 """
+if not self._launched:
+return
+
 self._early_cleanup()
 
 if self.is_running():
-- 
2.21.3

[PULL 11/19] python/machine.py: split shutdown into hard and soft flavors

[Philippe Mathieu-Daudé]

From: John Snow 

This is done primarily to avoid the 'bare except' pattern, which
suppresses all exceptions during shutdown and can obscure errors.

Replace this with a pattern that isolates the different kind of shutdown
paradigms (_hard_shutdown and _soft_shutdown), and a new fallback shutdown
handler (_do_shutdown) that gracefully attempts one before the other.

This split now also ensures that no matter what happens,
_post_shutdown() is always invoked.

shutdown() changes in behavior such that if it attempts to do a graceful
shutdown and is unable to, it will now always raise an exception to
indicate this. This can be avoided by the test writer in three ways:

1. If the VM is expected to have already exited or is in the process of
exiting, wait() can be used instead of shutdown() to clean up resources
instead. This helps avoid race conditions in shutdown.

2. If a test writer is expecting graceful shutdown to fail, shutdown
should be called in a try...except block.

3. If the test writer has no interest in performing a graceful shutdown
at all, kill() can be used instead.

Handling shutdown in this way makes it much more explicit which type of
shutdown we want and allows the library to report problems with this
process.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-Id: <20200710050649.32434-11-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 python/qemu/machine.py | 98 +++---
 1 file changed, 83 insertions(+), 15 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 3f0b873f58..a955e3f221 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -49,6 +49,12 @@ class QEMUMachineAddDeviceError(QEMUMachineError):
 """
 
 
+class AbnormalShutdown(QEMUMachineError):
+"""
+Exception raised when a graceful shutdown was requested, but not performed.
+"""
+
+
 class MonitorResponseError(qmp.QMPError):
 """
 Represents erroneous QMP monitor reply
@@ -376,6 +382,7 @@ def _early_cleanup(self) -> None:
 """
 Perform any cleanup that needs to happen before the VM exits.
 
+May be invoked by both soft and hard shutdown in failover scenarios.
 Called additionally by _post_shutdown for comprehensive cleanup.
 """
 # If we keep the console socket open, we may deadlock waiting
@@ -385,32 +392,93 @@ def _early_cleanup(self) -> None:
 self._console_socket.close()
 self._console_socket = None
 
+def _hard_shutdown(self) -> None:
+"""
+Perform early cleanup, kill the VM, and wait for it to terminate.
+
+:raise subprocess.Timeout: When timeout is exceeds 60 seconds
+waiting for the QEMU process to terminate.
+"""
+self._early_cleanup()
+self._popen.kill()
+self._popen.wait(timeout=60)
+
+def _soft_shutdown(self, has_quit: bool = False,
+   timeout: Optional[int] = 3) -> None:
+"""
+Perform early cleanup, attempt to gracefully shut down the VM, and wait
+for it to terminate.
+
+:param has_quit: When True, don't attempt to issue 'quit' QMP command
+:param timeout: Optional timeout in seconds for graceful shutdown.
+Default 3 seconds, A value of None is an infinite wait.
+
+:raise ConnectionReset: On QMP communication errors
+:raise subprocess.TimeoutExpired: When timeout is exceeded waiting for
+the QEMU process to terminate.
+"""
+self._early_cleanup()
+
+if self._qmp is not None:
+if not has_quit:
+# Might raise ConnectionReset
+self._qmp.cmd('quit')
+
+# May raise subprocess.TimeoutExpired
+self._popen.wait(timeout=timeout)
+
+def _do_shutdown(self, has_quit: bool = False,
+ timeout: Optional[int] = 3) -> None:
+"""
+Attempt to shutdown the VM gracefully; fallback to a hard shutdown.
+
+:param has_quit: When True, don't attempt to issue 'quit' QMP command
+:param timeout: Optional timeout in seconds for graceful shutdown.
+Default 3 seconds, A value of None is an infinite wait.
+
+:raise AbnormalShutdown: When the VM could not be shut down gracefully.
+The inner exception will likely be ConnectionReset or
+subprocess.TimeoutExpired. In rare cases, non-graceful termination
+may result in its own exceptions, likely subprocess.TimeoutExpired.
+"""
+try:
+self._soft_shutdown(has_quit, timeout)
+except Exception as exc:
+self._hard_shutdown()
+raise AbnormalShutdown("Could not perform graceful shutdown") \
+from exc
+
 def shutdown(self, has_quit: bool = False,
  hard: bool = False,
 

[PULL 01/19] scripts/performance: Add dissect.py script

[Philippe Mathieu-Daudé]

From: Ahmed Karaman 

Python script that dissects QEMU execution into three main phases:
code generation, JIT execution and helpers execution.

Syntax:
dissect.py [-h] --  [] \
  []

[-h] - Print the script arguments help message.

Example of usage:
dissect.py -- qemu-arm coulomb_double-arm

Example output:
Total Instructions:4,702,865,362

Code Generation: 115,819,309 2.463%
JIT Execution: 1,081,980,52823.007%
Helpers:   3,505,065,52574.530%

Signed-off-by: Ahmed Karaman 
Reviewed-by: Aleksandar Markovic 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20200709052055.2650-2-ahmedkhaledkara...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 scripts/performance/dissect.py | 166 +
 1 file changed, 166 insertions(+)
 create mode 100755 scripts/performance/dissect.py

diff --git a/scripts/performance/dissect.py b/scripts/performance/dissect.py
new file mode 100755
index 00..bf24f50922
--- /dev/null
+++ b/scripts/performance/dissect.py
@@ -0,0 +1,166 @@
+#!/usr/bin/env python3
+
+#  Print the percentage of instructions spent in each phase of QEMU
+#  execution.
+#
+#  Syntax:
+#  dissect.py [-h] --  [] \
+#[]
+#
+#  [-h] - Print the script arguments help message.
+#
+#  Example of usage:
+#  dissect.py -- qemu-arm coulomb_double-arm
+#
+#  This file is a part of the project "TCG Continuous Benchmarking".
+#
+#  Copyright (C) 2020  Ahmed Karaman 
+#  Copyright (C) 2020  Aleksandar Markovic 
+#
+#  This program is free software: you can redistribute it and/or modify
+#  it under the terms of the GNU General Public License as published by
+#  the Free Software Foundation, either version 2 of the License, or
+#  (at your option) any later version.
+#
+#  This program is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY; without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+#  GNU General Public License for more details.
+#
+#  You should have received a copy of the GNU General Public License
+#  along with this program. If not, see .
+
+import argparse
+import os
+import subprocess
+import sys
+import tempfile
+
+
+def get_JIT_line(callgrind_data):
+"""
+Search for the first instance of the JIT call in
+the callgrind_annotate output when ran using --tree=caller
+This is equivalent to the self number of instructions of JIT.
+
+Parameters:
+callgrind_data (list): callgrind_annotate output
+
+Returns:
+(int): Line number
+"""
+line = -1
+for i in range(len(callgrind_data)):
+if callgrind_data[i].strip('\n') and \
+callgrind_data[i].split()[-1] == "[???]":
+line = i
+break
+if line == -1:
+sys.exit("Couldn't locate the JIT call ... Exiting.")
+return line
+
+
+def main():
+# Parse the command line arguments
+parser = argparse.ArgumentParser(
+usage='dissect.py [-h] -- '
+' [] '
+' []')
+
+parser.add_argument('command', type=str, nargs='+', help=argparse.SUPPRESS)
+
+args = parser.parse_args()
+
+# Extract the needed variables from the args
+command = args.command
+
+# Insure that valgrind is installed
+check_valgrind = subprocess.run(
+["which", "valgrind"], stdout=subprocess.DEVNULL)
+if check_valgrind.returncode:
+sys.exit("Please install valgrind before running the script.")
+
+# Save all intermediate files in a temporary directory
+with tempfile.TemporaryDirectory() as tmpdirname:
+# callgrind output file path
+data_path = os.path.join(tmpdirname, "callgrind.data")
+# callgrind_annotate output file path
+annotate_out_path = os.path.join(tmpdirname, "callgrind_annotate.out")
+
+# Run callgrind
+callgrind = subprocess.run((["valgrind",
+ "--tool=callgrind",
+ "--callgrind-out-file=" + data_path]
++ command),
+   stdout=subprocess.DEVNULL,
+   stderr=subprocess.PIPE)
+if callgrind.returncode:
+sys.exit(callgrind.stderr.decode("utf-8"))
+
+# Save callgrind_annotate output
+with open(annotate_out_path, "w") as output:
+callgrind_annotate = subprocess.run(
+["callgrind_annotate", data_path, "--tree=caller"],
+stdout=output,
+stderr=subprocess.PIPE)
+if callgrind_annotate.returncode:
+sys.exit(callgrind_annotate.stderr.decode("utf-8"))
+
+# Read the callgrind_annotate output to callgrind_data[]
+callgrind_data = []
+with open(annotate_out_path, 'r') as data:
+callgrind_data = data.readlines()
+
+  

[PULL 07/19] python/machine.py: Add a configurable timeout to shutdown()

[Philippe Mathieu-Daudé]

From: John Snow 

Three seconds is hardcoded. Use it as a default parameter instead, and use that
value for both waits that may occur in the function.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Cleber Rosa 
Message-Id: <20200710050649.32434-7-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 python/qemu/machine.py | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index c28957ee82..e825f0bdc6 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -393,7 +393,9 @@ def wait(self):
 self._popen.wait()
 self._post_shutdown()
 
-def shutdown(self, has_quit=False, hard=False):
+def shutdown(self, has_quit: bool = False,
+ hard: bool = False,
+ timeout: Optional[int] = 3) -> None:
 """
 Terminate the VM and clean up
 """
@@ -409,10 +411,10 @@ def shutdown(self, has_quit=False, hard=False):
 try:
 if not has_quit:
 self._qmp.cmd('quit')
-self._popen.wait(timeout=3)
+self._popen.wait(timeout=timeout)
 except:
 self._popen.kill()
-self._popen.wait()
+self._popen.wait(timeout=timeout)
 
 self._post_shutdown()
 
-- 
2.21.3

[PULL 00/19] Python patches for 5.1

[Philippe Mathieu-Daudé]

The following changes since commit 1a53dfee92284d3016a579ef31d53367e84d9dd8:

  Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2020-07-13' into 
staging (2020-07-14 13:52:10 +0100)

are available in the Git repository at:

  https://gitlab.com/philmd/qemu.git tags/python-next-20200714

for you to fetch changes up to 84dcdf0887cdaaba7300442482c99e5064865a2d:

  python/qmp.py: add QMPProtocolError (2020-07-14 22:22:22 +0200)


Python patches for 5.1

- Reduce race conditions on QEMUMachine::shutdown()

 1. Remove the "bare except" pattern in the existing shutdown code,
which can mask problems and make debugging difficult.
 2. Ensure that post-shutdown cleanup is always performed, even when
graceful termination fails.
 3. Unify cleanup paths such that no matter how the VM is terminated,
the same functions and steps are always taken to reset the object
state.
 4. Rewrite shutdown() such that any error encountered when attempting
a graceful shutdown will be raised as an AbnormalShutdown exception.
The pythonic idiom is to allow the caller to decide if this is a
problem or not.

- Modify part of the python/qemu library to comply with:

  . mypy --strict
  . pylint
  . flake8

- Script for the TCG Continuous Benchmarking project that uses
  callgrind to dissect QEMU execution into three main phases:

  . code generation
  . JIT execution
  . helpers execution

CI jobs results:
. https://cirrus-ci.com/build/5421349961203712
. https://gitlab.com/philmd/qemu/-/pipelines/166556001
. https://travis-ci.org/github/philmd/qemu/builds/708102347


Ahmed Karaman (1):
  scripts/performance: Add dissect.py script

John Snow (18):
  python/machine.py: consolidate _post_shutdown()
  python/machine.py: Close QMP socket in cleanup
  python/machine.py: Add _early_cleanup hook
  python/machine.py: Perform early cleanup for wait() calls, too
  python/machine.py: Prohibit multiple shutdown() calls
  python/machine.py: Add a configurable timeout to shutdown()
  python/machine.py: Make wait() call shutdown()
  tests/acceptance: wait() instead of shutdown() where appropriate
  tests/acceptance: Don't test reboot on cubieboard
  python/machine.py: split shutdown into hard and soft flavors
  python/machine.py: re-add sigkill warning suppression
  python/machine.py: change default wait timeout to 3 seconds
  python/qmp.py: Define common types
  iotests.py: use qemu.qmp type aliases
  python/qmp.py: re-absorb MonitorResponseError
  python/qmp.py: Do not return None from cmd_obj
  python/qmp.py: add casts to JSON deserialization
  python/qmp.py: add QMPProtocolError

 python/qemu/machine.py   | 176 +--
 python/qemu/qmp.py   |  67 +++--
 scripts/performance/dissect.py   | 166 +
 scripts/render_block_graph.py|   7 +-
 tests/acceptance/boot_linux_console.py   |  14 +-
 tests/acceptance/linux_ssh_mips_malta.py |   2 +
 tests/qemu-iotests/iotests.py|   9 +-
 7 files changed, 369 insertions(+), 72 deletions(-)
 create mode 100755 scripts/performance/dissect.py

-- 
2.21.3

[PULL 08/19] python/machine.py: Make wait() call shutdown()

[Philippe Mathieu-Daudé]

From: John Snow 

At this point, shutdown(has_quit=True) and wait() do essentially the
same thing; they perform cleanup without actually instructing QEMU to
quit.

Define one in terms of the other.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-Id: <20200710050649.32434-8-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 python/qemu/machine.py | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index e825f0bdc6..3f0b873f58 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -385,14 +385,6 @@ def _early_cleanup(self) -> None:
 self._console_socket.close()
 self._console_socket = None
 
-def wait(self):
-"""
-Wait for the VM to power off
-"""
-self._early_cleanup()
-self._popen.wait()
-self._post_shutdown()
-
 def shutdown(self, has_quit: bool = False,
  hard: bool = False,
  timeout: Optional[int] = 3) -> None:
@@ -421,6 +413,15 @@ def shutdown(self, has_quit: bool = False,
 def kill(self):
 self.shutdown(hard=True)
 
+def wait(self, timeout: Optional[int] = None) -> None:
+"""
+Wait for the VM to power off and perform post-shutdown cleanup.
+
+:param timeout: Optional timeout in seconds.
+Default None, an infinite wait.
+"""
+self.shutdown(has_quit=True, timeout=timeout)
+
 def set_qmp_monitor(self, enabled=True):
 """
 Set the QMP monitor.
-- 
2.21.3

[PULL 05/19] python/machine.py: Perform early cleanup for wait() calls, too

[Philippe Mathieu-Daudé]

From: John Snow 

This is primarily for consistency, and is a step towards wait() and
shutdown() sharing the same implementation so that the two cleanup paths
cannot diverge.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-Id: <20200710050649.32434-5-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 python/qemu/machine.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 127926b276..63e40879e2 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -380,6 +380,7 @@ def wait(self):
 """
 Wait for the VM to power off
 """
+self._early_cleanup()
 self._popen.wait()
 self._post_shutdown()
 
-- 
2.21.3

[PULL 04/19] python/machine.py: Add _early_cleanup hook

[Philippe Mathieu-Daudé]

From: John Snow 

Some parts of cleanup need to occur prior to shutdown, otherwise
shutdown might break. Move this into a suitably named method/callback.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-Id: <20200710050649.32434-4-js...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 python/qemu/machine.py | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index d3faa9a84c..127926b276 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -365,6 +365,17 @@ def _launch(self):
close_fds=False)
 self._post_launch()
 
+def _early_cleanup(self) -> None:
+"""
+Perform any cleanup that needs to happen before the VM exits.
+"""
+# If we keep the console socket open, we may deadlock waiting
+# for QEMU to exit, while QEMU is waiting for the socket to
+# become writeable.
+if self._console_socket is not None:
+self._console_socket.close()
+self._console_socket = None
+
 def wait(self):
 """
 Wait for the VM to power off
@@ -376,12 +387,7 @@ def shutdown(self, has_quit=False, hard=False):
 """
 Terminate the VM and clean up
 """
-# If we keep the console socket open, we may deadlock waiting
-# for QEMU to exit, while QEMU is waiting for the socket to
-# become writeable.
-if self._console_socket is not None:
-self._console_socket.close()
-self._console_socket = None
+self._early_cleanup()
 
 if self.is_running():
 if hard:
-- 
2.21.3

[PATCH] virtio-rng: return available data with O_NONBLOCK

[mwilck]

From: Martin Wilck 

If a program opens /dev/hwrng with O_NONBLOCK and uses poll() and
non-blocking read() to retrieve random data, it ends up in a tight
loop with poll() always returning POLLIN and read() returning EAGAIN.
This repeats forever until some process makes a blocking read() call.
The reason is that virtio_read() always returns 0 in non-blocking mode,
even if data is available.

The following test program illustrates the behavior.

void loop(int fd)
{
struct pollfd pfd0 = { .fd = fd, .events  = POLLIN, };
int rc;
unsigned int n;

for (n = LOOPS; n > 0; n--) {
struct pollfd pfd = pfd0;
char buf[SIZE];

rc = poll(, 1, 1);
if (rc > 0) {
int rd = read(fd, buf, sizeof(buf));

if (rd == -1)
perror("read");
else
printf("read %d bytes\n", rd);
} else if (rc == -1)
perror("poll");
else
fprintf(stderr, "timeout\n");

}
}

int main(void)
{
int fd;

fd = open("/dev/hwrng", O_RDONLY|O_NONBLOCK);
if (fd == -1) {
perror("open");
return 1;
};
loop(fd);
close(fd);
return 0;
}

This can be observed in the real word e.g. with nested qemu/KVM virtual
machines, if both the "outer" and "inner" VMs have a virtio-rng device.
If the "inner" VM requests random data, qemu running in the "outer" VM
uses this device in a non-blocking manner like the test program above.

Fix it by returning available data if it exists.

Signed-off-by: Martin Wilck 
---
 drivers/char/hw_random/virtio-rng.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/char/hw_random/virtio-rng.c 
b/drivers/char/hw_random/virtio-rng.c
index 79a6e47b5fbc..94806308d814 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -59,6 +59,9 @@ static int virtio_read(struct hwrng *rng, void *buf, size_t 
size, bool wait)
if (vi->hwrng_removed)
return -ENODEV;
 
+   if (vi->data_avail >= size || (vi->data_avail && !wait))
+   return vi->data_avail;
+
if (!vi->busy) {
vi->busy = true;
reinit_completion(>have_data);
-- 
2.26.2

[PATCH] util: qemu_get_thread_id for OpenBSD

[David CARLIER]

>From 9c7f54c67d40fae0174ba795fbaad829cd59c264 Mon Sep 17 00:00:00 2001
From: David Carlier 
Date: Tue, 14 Jul 2020 23:23:55 +0100
Subject: [PATCH] util: qemu_get_thread_id implementation for OpenBSD.

ussage of getthrid syscall.

Signed-off-by: David Carlier 
---
 util/oslib-posix.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 72907d4d7f..b4f7de83c8 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -100,6 +100,8 @@ int qemu_get_thread_id(void)
 return (int)tid;
 #elif defined(__NetBSD__)
 return _lwp_self();
+#elif defined(__OpenBSD__)
+return getthrid();
 #else
 return getpid();
 #endif
-- 
2.27.0

Re: device compatibility interface for live migration with assigned devices

[Sean Mooney]

resending with full cc list since i had this typed up
i would blame my email provier but my email client does not seam to like long 
cc lists.
we probably want to continue on  alex's thread to not split the disscusion.
but i have responed inline with some example of  how openstack schdules and 
what i ment by different mdev_types


On Tue, 2020-07-14 at 20:29 +0100, Sean Mooney wrote:
> On Tue, 2020-07-14 at 11:01 -0600, Alex Williamson wrote:
> > On Tue, 14 Jul 2020 13:33:24 +0100
> > Sean Mooney  wrote:
> > 
> > > On Tue, 2020-07-14 at 11:21 +0100, Daniel P. Berrangé wrote:
> > > > On Tue, Jul 14, 2020 at 07:29:57AM +0800, Yan Zhao wrote:  
> > > > > hi folks,
> > > > > we are defining a device migration compatibility interface that helps 
> > > > > upper
> > > > > layer stack like openstack/ovirt/libvirt to check if two devices are
> > > > > live migration compatible.
> > > > > The "devices" here could be MDEVs, physical devices, or hybrid of the 
> > > > > two.
> > > > > e.g. we could use it to check whether
> > > > > - a src MDEV can migrate to a target MDEV,  
> > > 
> > > mdev live migration is completely possible to do but i agree with Dan 
> > > barrange's comments
> > > from the point of view of openstack integration i dont see calling out to 
> > > a vender sepecific
> > > tool to be an accpetable
> > 
> > As I replied to Dan, I'm hoping Yan was referring more to vendor
> > specific knowledge rather than actual tools.
> > 
> > > solutions for device compatiablity checking. the sys filesystem
> > > that describs the mdevs that can be created shoudl also
> > > contain the relevent infomation such
> > > taht nova could integrate it via libvirt xml representation or directly 
> > > retrive the
> > > info from
> > > sysfs.
> > > > > - a src VF in SRIOV can migrate to a target VF in SRIOV,  
> > > 
> > > so vf to vf migration is not possible in the general case as there is no 
> > > standarised
> > > way to transfer teh device state as part of the siorv specs produced by 
> > > the pci-sig
> > > as such there is not vender neutral way to support sriov live migration. 
> > 
> > We're not talking about a general case, we're talking about physical
> > devices which have vfio wrappers or hooks with device specific
> > knowledge in order to support the vfio migration interface.  The point
> > is that a discussion around vfio device migration cannot be limited to
> > mdev devices.
> 
> ok upstream in  openstack at least we do not plan to support generic 
> livemigration
> for passthough devivces. we cheat with network interfaces since in generaly 
> operating
> systems handel hotplug of a nic somewhat safely so wehre no abstraction layer 
> like
> an mdev is present or a macvtap device we hot unplug the nic before the 
> migration
> and attach a new one after.  for gpus or crypto cards this likely would not 
> be viable
> since you can bond generic hardware devices to hide the removal and readdtion 
> of a generic
> pci device. we were hoping that there would be a convergenca around MDEVs as 
> a way to provide
> that abstraction going forward for generic device or some other new 
> mechanisum in the future.
> > 
> > > > > - a src MDEV can migration to a target VF in SRIOV.  
> > > 
> > > that also makes this unviable
> > > > >   (e.g. SIOV/SRIOV backward compatibility case)
> > > > > 
> > > > > The upper layer stack could use this interface as the last step to 
> > > > > check
> > > > > if one device is able to migrate to another device before triggering 
> > > > > a real
> > > > > live migration procedure.  
> > > 
> > > well actully that is already too late really. ideally we would want to do 
> > > this compaiablity
> > > check much sooneer to avoid the migration failing. in an openstack 
> > > envionment  at least
> > > by the time we invoke libvirt (assuming your using the libvirt driver) to 
> > > do the migration we have alreaedy
> > > finished schduling the instance to the new host. if if we do the 
> > > compatiablity check at this point
> > > and it fails then the live migration is aborted and will not be retired. 
> > > These types of late check lead to a
> > > poor user experince as unless you check the migration detial it basically 
> > > looks like the migration was ignored
> > > as it start to migrate and then continuge running on the orgininal host.
> > > 
> > > when using generic pci passhotuhg with openstack, the pci alias is 
> > > intended to reference a single vendor
> > > id/product
> > > id so you will have 1+ alias for each type of device. that allows 
> > > openstack to schedule based on the availability
> > > of
> > > a
> > > compatibale device because we track inventories of pci devices and can 
> > > query that when selecting a host.
> > > 
> > > if we were to support mdev live migration in the future we would want to 
> > > take the same declarative approch.
> > > 1 interospec the capability of the deivce we manage
> > > 2 create inventories of the allocatable devices and 

Re: device compatibility interface for live migration with assigned devices

[Alex Williamson]

On Tue, 14 Jul 2020 18:19:46 +0100
"Dr. David Alan Gilbert"  wrote:

> * Alex Williamson (alex.william...@redhat.com) wrote:
> > On Tue, 14 Jul 2020 11:21:29 +0100
> > Daniel P. Berrangé  wrote:
> >   
> > > On Tue, Jul 14, 2020 at 07:29:57AM +0800, Yan Zhao wrote:  
> > > > hi folks,
> > > > we are defining a device migration compatibility interface that helps 
> > > > upper
> > > > layer stack like openstack/ovirt/libvirt to check if two devices are
> > > > live migration compatible.
> > > > The "devices" here could be MDEVs, physical devices, or hybrid of the 
> > > > two.
> > > > e.g. we could use it to check whether
> > > > - a src MDEV can migrate to a target MDEV,
> > > > - a src VF in SRIOV can migrate to a target VF in SRIOV,
> > > > - a src MDEV can migration to a target VF in SRIOV.
> > > >   (e.g. SIOV/SRIOV backward compatibility case)
> > > > 
> > > > The upper layer stack could use this interface as the last step to check
> > > > if one device is able to migrate to another device before triggering a 
> > > > real
> > > > live migration procedure.
> > > > we are not sure if this interface is of value or help to you. please 
> > > > don't
> > > > hesitate to drop your valuable comments.
> > > > 
> > > > 
> > > > (1) interface definition
> > > > The interface is defined in below way:
> > > > 
> > > >  __userspace
> > > >   /\  \
> > > >  / \write
> > > > / read  \
> > > >/__   ___\|/_
> > > >   | migration_version | | migration_version |-->check migration
> > > >   - -   compatibility
> > > >  device Adevice B
> > > > 
> > > > 
> > > > a device attribute named migration_version is defined under each 
> > > > device's
> > > > sysfs node. e.g. 
> > > > (/sys/bus/pci/devices/\:00\:02.0/$mdev_UUID/migration_version).
> > > > userspace tools read the migration_version as a string from the source 
> > > > device,
> > > > and write it to the migration_version sysfs attribute in the target 
> > > > device.
> > > > 
> > > > The userspace should treat ANY of below conditions as two devices not 
> > > > compatible:
> > > > - any one of the two devices does not have a migration_version attribute
> > > > - error when reading from migration_version attribute of one device
> > > > - error when writing migration_version string of one device to
> > > >   migration_version attribute of the other device
> > > > 
> > > > The string read from migration_version attribute is defined by device 
> > > > vendor
> > > > driver and is completely opaque to the userspace.
> > > > for a Intel vGPU, string format can be defined like
> > > > "parent device PCI ID" + "version of gvt driver" + "mdev type" + 
> > > > "aggregator count".
> > > > 
> > > > for an NVMe VF connecting to a remote storage. it could be
> > > > "PCI ID" + "driver version" + "configured remote storage URL"
> > > > 
> > > > for a QAT VF, it may be
> > > > "PCI ID" + "driver version" + "supported encryption set".
> > > > 
> > > > (to avoid namespace confliction from each vendor, we may prefix a 
> > > > driver name to
> > > > each migration_version string. e.g. i915-v1-8086-591d-i915-GVTg_V5_8-1) 
> > > >  
> > 
> > It's very strange to define it as opaque and then proceed to describe
> > the contents of that opaque string.  The point is that its contents
> > are defined by the vendor driver to describe the device, driver version,
> > and possibly metadata about the configuration of the device.  One
> > instance of a device might generate a different string from another.
> > The string that a device produces is not necessarily the only string
> > the vendor driver will accept, for example the driver might support
> > backwards compatible migrations.  
> 
> (As I've said in the previous discussion, off one of the patch series)
> 
> My view is it makes sense to have a half-way house on the opaqueness of
> this string; I'd expect to have an ID and version that are human
> readable, maybe a device ID/name that's human interpretable and then a
> bunch of other cruft that maybe device/vendor/version specific.
> 
> I'm thinking that we want to be able to report problems and include the
> string and the user to be able to easily identify the device that was
> complaining and notice a difference in versions, and perhaps also use
> it in compatibility patterns to find compatible hosts; but that does
> get tricky when it's a 'ask the device if it's compatible'.

In the reply I just sent to Dan, I gave this example of what a
"compatibility string" might look like represented as json:

{
  "device_api": "vfio-pci",
  "vendor": "vendor-driver-name",
  "version": {
"major": 0,
"minor": 1
  },
  "vfio-pci": { // Based on above device_api
"vendor": 0x1234, // Values for the exposed device
"device": 0x5678,
  // Possibly further parameters for a more 

Re: [PATCH] linux-user: Add strace support for printing arguments for ioctls used for terminals and serial lines

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20200714200439.11328-1-filip.boz...@syrmia.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20200714200439.11328-1-filip.boz...@syrmia.com
Subject: [PATCH] linux-user: Add strace support for printing arguments for 
ioctls used for terminals and serial lines

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
   d2628b1..8bfa25a  master -> master
Switched to a new branch 'test'
ae9f19a linux-user: Add strace support for printing arguments for ioctls used 
for terminals and serial lines

=== OUTPUT BEGIN ===
ERROR: storage class should be at the beginning of the declaration
#75: FILE: linux-user/strace.c:1196:
+UNUSED static struct flags termios_iflags[] = {

ERROR: storage class should be at the beginning of the declaration
#93: FILE: linux-user/strace.c:1214:
+UNUSED static struct flags termios_oflags[] = {

ERROR: storage class should be at the beginning of the declaration
#105: FILE: linux-user/strace.c:1226:
+UNUSED static struct flags termios_oflags_NLDLY[] = {

ERROR: storage class should be at the beginning of the declaration
#111: FILE: linux-user/strace.c:1232:
+UNUSED static struct flags termios_oflags_CRDLY[] = {

ERROR: storage class should be at the beginning of the declaration
#119: FILE: linux-user/strace.c:1240:
+UNUSED static struct flags termios_oflags_TABDLY[] = {

ERROR: storage class should be at the beginning of the declaration
#127: FILE: linux-user/strace.c:1248:
+UNUSED static struct flags termios_oflags_VTDLY[] = {

ERROR: storage class should be at the beginning of the declaration
#133: FILE: linux-user/strace.c:1254:
+UNUSED static struct flags termios_oflags_FFDLY[] = {

ERROR: storage class should be at the beginning of the declaration
#139: FILE: linux-user/strace.c:1260:
+UNUSED static struct flags termios_oflags_BSDLY[] = {

ERROR: storage class should be at the beginning of the declaration
#145: FILE: linux-user/strace.c:1266:
+UNUSED static struct flags termios_cflags_CBAUD[] = {

ERROR: storage class should be at the beginning of the declaration
#169: FILE: linux-user/strace.c:1290:
+UNUSED static struct flags termios_cflags_CSIZE[] = {

ERROR: storage class should be at the beginning of the declaration
#177: FILE: linux-user/strace.c:1298:
+UNUSED static struct flags termios_cflags[] = {

ERROR: storage class should be at the beginning of the declaration
#188: FILE: linux-user/strace.c:1309:
+UNUSED static struct flags termios_lflags[] = {

total: 12 errors, 0 warnings, 260 lines checked

Commit ae9f19a165a6 (linux-user: Add strace support for printing arguments for 
ioctls used for terminals and serial lines) has style problems, please review.  
If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200714200439.11328-1-filip.boz...@syrmia.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH v1] util: OpenBSD build fix

[Peter Maydell]

On Tue, 14 Jul 2020 at 20:45, David CARLIER  wrote:
>
> From e2103b86b031ab74ff4c8dd0a3944cb488c9333e Mon Sep 17 00:00:00 2001
> From: David Carlier 
> Date: Tue, 14 Jul 2020 21:34:59 +0100
> Subject: [PATCH] util: OpenBSD build fix.
>
> thread id implementation, using getthrid syscall.
> qemu_exec_dir implementation as beast as we can as
> path is not always possible to resolve this on this platform.

Hi; thanks for the patch.

These look like two separate changes, so they should be
in separate patches, please.

It would be useful to have a comment in the code documenting
what the limitations of the OpenBSD call are, and when
it's better than just using realpath() on the argv[0]
that we already have. (ie, in which cases is the argv[0]
which we get back via KERN_PROC_ARGV something other than
the argv[0] that was passed to the process?)

> Signed-off-by: David Carlier 
> ---
>  util/oslib-posix.c | 23 +++
>  1 file changed, 23 insertions(+)
>
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 72907d4d7f..4a0cce15b4 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -56,6 +56,10 @@
>  #include 
>  #endif
>
> +#ifdef __OpenBSD__
> +#include 
> +#endif
> +
>  #ifdef __APPLE__
>  #include 
>  #endif
> @@ -100,6 +104,8 @@ int qemu_get_thread_id(void)
>  return (int)tid;
>  #elif defined(__NetBSD__)
>  return _lwp_self();
> +#elif defined(__OpenBSD__)
> +return getthrid();
>  #else
>  return getpid();
>  #endif
> @@ -408,6 +414,23 @@ void qemu_init_exec_dir(const char *argv0)
>  }
>  }
>  }
> +#elif defined(__OpenBSD__)
> +{
> +
> +char **args;
> +size_t len;
> +int mib[4] = {CTL_KERN, KERN_PROC_ARGS, getpid(), KERN_PROC_ARGV};
> +
> +*buf = 0;
> +if (!sysctl(mib, ARRAY_SIZE(mib), NULL, , NULL, 0)) {
> +args = malloc(len);

If you want to use malloc() you need to check the return value.
But better to use g_malloc(), which can't return a failure value.

> +if (!sysctl(mib, ARRAY_SIZE(mib), args, , NULL, 0)) {
> +p = realpath(*args, buf);
> +}
> +
> +free(args);
> +}
> +}
>  #endif
>  /* If we don't have any way of figuring out the actual executable
> location then try argv[0].  */
> --

thanks
-- PMM

Re: device compatibility interface for live migration with assigned devices

[Alex Williamson]

On Tue, 14 Jul 2020 17:47:22 +0100
Daniel P. Berrangé  wrote:

> On Tue, Jul 14, 2020 at 10:16:16AM -0600, Alex Williamson wrote:
> > On Tue, 14 Jul 2020 11:21:29 +0100
> > Daniel P. Berrangé  wrote:
> >   
> > > On Tue, Jul 14, 2020 at 07:29:57AM +0800, Yan Zhao wrote:  
> > > > 
> > > > The string read from migration_version attribute is defined by device 
> > > > vendor
> > > > driver and is completely opaque to the userspace.
> > > > for a Intel vGPU, string format can be defined like
> > > > "parent device PCI ID" + "version of gvt driver" + "mdev type" + 
> > > > "aggregator count".
> > > > 
> > > > for an NVMe VF connecting to a remote storage. it could be
> > > > "PCI ID" + "driver version" + "configured remote storage URL"
> > > > 
> > > > for a QAT VF, it may be
> > > > "PCI ID" + "driver version" + "supported encryption set".
> > > > 
> > > > (to avoid namespace confliction from each vendor, we may prefix a 
> > > > driver name to
> > > > each migration_version string. e.g. i915-v1-8086-591d-i915-GVTg_V5_8-1) 
> > > >  
> > 
> > It's very strange to define it as opaque and then proceed to describe
> > the contents of that opaque string.  The point is that its contents
> > are defined by the vendor driver to describe the device, driver version,
> > and possibly metadata about the configuration of the device.  One
> > instance of a device might generate a different string from another.
> > The string that a device produces is not necessarily the only string
> > the vendor driver will accept, for example the driver might support
> > backwards compatible migrations.  
> 
> 
> > > IMHO there needs to be a mechanism for the kernel to report via sysfs
> > > what versions are supported on a given device. This puts the job of
> > > reporting compatible versions directly under the responsibility of the
> > > vendor who writes the kernel driver for it. They are the ones with the
> > > best knowledge of the hardware they've built and the rules around its
> > > compatibility.  
> > 
> > The version string discussed previously is the version string that
> > represents a given device, possibly including driver information,
> > configuration, etc.  I think what you're asking for here is an
> > enumeration of every possible version string that a given device could
> > accept as an incoming migration stream.  If we consider the string as
> > opaque, that means the vendor driver needs to generate a separate
> > string for every possible version it could accept, for every possible
> > configuration option.  That potentially becomes an excessive amount of
> > data to either generate or manage.
> > 
> > Am I overestimating how vendors intend to use the version string?  
> 
> If I'm interpreting your reply & the quoted text orrectly, the version
> string isn't really a version string in any normal sense of the word
> "version".
> 
> Instead it sounds like string encoding a set of features in some arbitrary
> vendor specific format, which they parse and do compatibility checks on
> individual pieces ? One or more parts may contain a version number, but
> its much more than just a version.
> 
> If that's correct, then I'd prefer we didn't call it a version string,
> instead call it a "capability string" to make it clear it is expressing
> a much more general concept, but...

I'd agree with that.  The intent of the previous proposal was to
provide and interface for reading a string and writing a string back in
where the result of that write indicated migration compatibility with
the device.  So yes, "version" is not the right term.
 
> > We'd also need to consider devices that we could create, for instance
> > providing the same interface enumeration prior to creating an mdev
> > device to have a confidence level that the new device would be a valid
> > target.
> > 
> > We defined the string as opaque to allow vendor flexibility and because
> > defining a common format is hard.  Do we need to revisit this part of
> > the discussion to define the version string as non-opaque with parsing
> > rules, probably with separate incoming vs outgoing interfaces?  Thanks,  
> 
> ..even if the huge amount of flexibility is technically relevant from the
> POV of the hardware/drivers, we should consider whether management apps
> actually want, or can use, that level of flexibility.
> 
> The task of picking which host to place a VM on has alot of factors to
> consider, and when there are a large number of hosts, the total amount
> of information to check gets correspondingly large.  The placement
> process is also fairly performance critical.
> 
> Running complex algorithmic logic to check compatibility of devices
> based on a arbitrary set of rules is likely to be a performance
> challenge. A flat list of supported strings is a much simpler
> thing to check as it reduces down to a simple set membership test.
> 
> IOW, even if there's some complex set of device type / vendor specific
> rules to check for compatibility, I fear apps 

[PATCH] hw/arm/aspeed: Add board model for Supermicro X11 BMC

[erik-smit]

Signed-off-by: erik-smit 
-- 
checkpatch was complaining about the length of
aspeed_machine_supermicrox11_bmc_class_init(ObjectClass *oc, void *data)

so I renamed it to aspeed_machine_smx11_bmc_class_init. Not sure if
that's the right way to go, since then it's out of sync with the machine
name "supermicrox11-bmc". But renaming the machine name to "smx11-bmc"
makes it less descriptive...

---
 hw/arm/aspeed.c | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 379f9672a5..e74a89f427 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -57,6 +57,20 @@ struct AspeedMachineState {
 SCU_HW_STRAP_VGA_SIZE_SET(VGA_16M_DRAM) |   \
 SCU_AST2400_HW_STRAP_BOOT_MODE(AST2400_SPI_BOOT))
 
+/* TODO: Find the actual hardware value */
+#define SUPERMICROX11_BMC_HW_STRAP1 (   \
+SCU_AST2400_HW_STRAP_DRAM_SIZE(DRAM_SIZE_128MB) |   \
+SCU_AST2400_HW_STRAP_DRAM_CONFIG(2) |   \
+SCU_AST2400_HW_STRAP_ACPI_DIS | \
+SCU_AST2400_HW_STRAP_SET_CLK_SOURCE(AST2400_CLK_48M_IN) |   \
+SCU_HW_STRAP_VGA_CLASS_CODE |   \
+SCU_HW_STRAP_LPC_RESET_PIN |\
+SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_M_S_EN) |\
+SCU_AST2400_HW_STRAP_SET_CPU_AHB_RATIO(AST2400_CPU_AHB_RATIO_2_1) | \
+SCU_HW_STRAP_SPI_WIDTH |\
+SCU_HW_STRAP_VGA_SIZE_SET(VGA_16M_DRAM) |   \
+SCU_AST2400_HW_STRAP_BOOT_MODE(AST2400_SPI_BOOT))
+
 /* AST2500 evb hardware value: 0xF100C2E6 */
 #define AST2500_EVB_HW_STRAP1 ((\
 AST2500_HW_STRAP1_DEFAULTS |\
@@ -600,6 +614,22 @@ static void aspeed_machine_palmetto_class_init(ObjectClass 
*oc, void *data)
 aspeed_soc_num_cpus(amc->soc_name);
 };
 
+static void aspeed_machine_smx11_bmc_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+mc->desc   = "Supermicro X11 BMC (ARM926EJ-S)";
+amc->soc_name  = "ast2400-a1";
+amc->hw_strap1 = SUPERMICROX11_BMC_HW_STRAP1;
+amc->fmc_model = "mx25l25635e";
+amc->spi_model = "mx25l25635e";
+amc->num_cs= 1;
+amc->macs_mask = ASPEED_MAC0_ON | ASPEED_MAC1_ON;
+amc->i2c_init  = palmetto_bmc_i2c_init;
+mc->default_ram_size = 256 * MiB;
+}
+
 static void aspeed_machine_ast2500_evb_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
@@ -728,6 +758,10 @@ static const TypeInfo aspeed_machine_types[] = {
 .name  = MACHINE_TYPE_NAME("palmetto-bmc"),
 .parent= TYPE_ASPEED_MACHINE,
 .class_init= aspeed_machine_palmetto_class_init,
+}, {
+.name  = MACHINE_TYPE_NAME("supermicrox11-bmc"),
+.parent= TYPE_ASPEED_MACHINE,
+.class_init= aspeed_machine_smx11_bmc_class_init,
 }, {
 .name  = MACHINE_TYPE_NAME("ast2500-evb"),
 .parent= TYPE_ASPEED_MACHINE,
-- 
2.25.1

Re: [PATCH v4 06/11] Update PowerPC AT_HWCAP2 definition

[Lijun Pan]

> On Jul 13, 2020, at 6:47 PM, David Gibson  wrote:
> 
> On Mon, Jul 13, 2020 at 02:20:20PM -0500, Lijun Pan wrote:
>> 
>> 
>>> On Jul 13, 2020, at 12:14 AM, David Gibson  
>>> wrote:
>>> 
>>> On Wed, Jul 01, 2020 at 06:43:41PM -0500, Lijun Pan wrote:
 Add PPC2_FEATURE2_ARCH_3_10 to the PowerPC AT_HWCAP2 definitions.
 
 Signed-off-by: Lijun Pan 
 ---
 v4: add missing changes, and split to 5/11, 6/11, 7/11
 v3: use tcg_gen_gvec_mul()
 v2: fix coding style
   use Power ISA 3.1 flag
 
 include/elf.h | 1 +
 1 file changed, 1 insertion(+)
 
 diff --git a/include/elf.h b/include/elf.h
 index 8fbfe60e09..1858b95acf 100644
 --- a/include/elf.h
 +++ b/include/elf.h
 @@ -554,6 +554,7 @@ typedef struct {
 #define PPC_FEATURE2_HTM_NOSC   0x0100
 #define PPC_FEATURE2_ARCH_3_00  0x0080
 #define PPC_FEATURE2_HAS_IEEE1280x0040
 +#define PPC_FEATURE2_ARCH_3_10  0x0020
 
 /* Bits present in AT_HWCAP for Sparc.  */
>>> 
>>> 
>>> Um.. in the corresponding #defines in the kernel 0x0020 is given
>>> to PPC_FEATURE2_DARN, and several more bits are allocated past that
>>> point.
>> 
>> Then what do you recommend to use?
> 
> This is part of exposed userland ABI, so it needs to be standardized
> at least semi-formally.  I'm not actually sure who allocates these in
> general.

Anton,

Do you know the allocation standard since you made the last change into elf.h?

Lijun

> 
> -- 
> David Gibson  | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | 
> minimalist, thank you.  NOT _the_ _other_
>   | _way_ _around_!
> http://www.ozlabs.org/~dgibson 

Re: [PULL 0/7] Linux user for 5.1 patches

[Peter Maydell]

On Tue, 14 Jul 2020 at 08:34, Laurent Vivier  wrote:
>
> The following changes since commit 5c65b1f135ff09d24827fa3a17e56a4f8a032cd5:
>
>   Merge remote-tracking branch 
> 'remotes/pmaydell/tags/pull-target-arm-20200713' into staging (2020-07-13 
> 15:14:48 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/vivier/qemu.git tags/linux-user-for-5.1-pull-request
>
> for you to fetch changes up to 42b16184d016d48d167229a1ddb89b3671c77440:
>
>   linux-user: fix print_syscall_err() when syscall returned value is negative 
> (2020-07-14 09:29:14 +0200)
>
> ----
> linux-user branch 20200714
>
> Fix strace errno management
> Fix Coverity erros in ioctl straces
> Fix some netlinks errors
> Fix semtimedop
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.

-- PMM

[PULL 2/3] target/mips: Fix ADD.S FPU instruction

[Philippe Mathieu-Daudé]

From: Alex Richardson 

After merging latest QEMU upstream into our CHERI fork,
I noticed that some of the FPU tests in our MIPS baremetal
testsuite [*] started failing.
It turns out commit 1ace099f2a accidentally changed add.s
into a subtract.

[*] https://github.com/CTSRD-CHERI/cheritest

Fixes: 1ace099f2a ("target/mips: fpu: Demacro ADD.")
Signed-off-by: Alex Richardson 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20200703161515.25966-1-alexander.richard...@cl.cam.ac.uk>
Signed-off-by: Aleksandar Markovic 
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/mips/fpu_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index 7a3a61cab3..56beda49d8 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1221,7 +1221,7 @@ uint32_t helper_float_add_s(CPUMIPSState *env,
 {
 uint32_t wt2;
 
-wt2 = float32_sub(fst0, fst1, >active_fpu.fp_status);
+wt2 = float32_add(fst0, fst1, >active_fpu.fp_status);
 update_fcr31(env, GETPC());
 return wt2;
 }
-- 
2.21.3

[PULL 1/3] target/mips: Remove identical if/else branches

[Philippe Mathieu-Daudé]

From: Aleksandar Markovic 

Remove the segment:

  if (other_tc == other->current_tc) {
  tccause = other->CP0_Cause;
  } else {
  tccause = other->CP0_Cause;
  }

Original contributor can't remember what was his intention.

Fixes: 5a25ce9487 ("mips: Hook in more reg accesses via mttr/mftr")
Buglink: https://bugs.launchpad.net/qemu/+bug/1885718
Signed-off-by: Aleksandar Markovic 
Message-Id: <20200701182559.28841-2-aleksandar.qemu.de...@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/mips/cp0_helper.c | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/target/mips/cp0_helper.c b/target/mips/cp0_helper.c
index bbf12e4a97..de64add038 100644
--- a/target/mips/cp0_helper.c
+++ b/target/mips/cp0_helper.c
@@ -375,16 +375,9 @@ target_ulong helper_mftc0_entryhi(CPUMIPSState *env)
 target_ulong helper_mftc0_cause(CPUMIPSState *env)
 {
 int other_tc = env->CP0_VPEControl & (0xff << CP0VPECo_TargTC);
-int32_t tccause;
 CPUMIPSState *other = mips_cpu_map_tc(env, _tc);
 
-if (other_tc == other->current_tc) {
-tccause = other->CP0_Cause;
-} else {
-tccause = other->CP0_Cause;
-}
-
-return tccause;
+return other->CP0_Cause;
 }
 
 target_ulong helper_mftc0_status(CPUMIPSState *env)
-- 
2.21.3

[PULL 0/3] MIPS patches for 5.1

[Philippe Mathieu-Daudé]

The following changes since commit 1a53dfee92284d3016a579ef31d53367e84d9dd8:

  Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2020-07-13' into 
staging (2020-07-14 13:52:10 +0100)

are available in the Git repository at:

  https://gitlab.com/philmd/qemu.git tags/mips-next-20200714

for you to fetch changes up to 15d983dee95edff1dc4c0bed71ce02fff877e766:

  MAINTAINERS: Adjust MIPS maintainership (add Huacai Chen & Jiaxun Yang) 
(2020-07-14 21:49:33 +0200)


MIPS patches for 5.1

- A pair of fixes,
- Add Huacai Chen as MIPS KVM maintainer,
- Add Jiaxun Yang as designated MIPS TCG reviewer.

CI jobs results:
. https://travis-ci.org/github/philmd/qemu/builds/708079271
. https://gitlab.com/philmd/qemu/-/pipelines/166528104
. https://cirrus-ci.com/build/6483996878045184



Aleksandar Markovic (2):
  target/mips: Remove identical if/else branches
  MAINTAINERS: Adjust MIPS maintainership (add Huacai Chen & Jiaxun
Yang)

Alex Richardson (1):
  target/mips: Fix ADD.S FPU instruction

 target/mips/cp0_helper.c | 9 +
 target/mips/fpu_helper.c | 2 +-
 MAINTAINERS  | 4 
 3 files changed, 6 insertions(+), 9 deletions(-)

-- 
2.21.3

[PULL 3/3] MAINTAINERS: Adjust MIPS maintainership (add Huacai Chen & Jiaxun Yang)

[Philippe Mathieu-Daudé]

From: Aleksandar Markovic 

Huacai Chen and Jiaxun Yang step in as new energy [1].

Aurelien Jarno comment [2]:

  It happens that I known Huacai Chen from the time he was
  upstreaming the Loongson 3 support to the kernel, I have been
  testing and reviewing his patches. I also know Jiaxun Yang from
  the #debian-mips IRC channel. I know that they are both very
  competent and have a good knowledge of the open source world.
  I therefore agree that they are good additions to maintain and/or
  review the MIPS part of QEMU.

[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg718434.html
[2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg718738.html

Signed-off-by: Aleksandar Markovic 
Message-Id: <20200701182559.28841-3-aleksandar.qemu.de...@gmail.com>
PMD: [Split patch, added Aurelien's comment]
Signed-off-by: Philippe Mathieu-Daudé 
Acked-by: Jiaxun Yang 
Acked-by: Huacai Chen 
---
 MAINTAINERS | 4 
 1 file changed, 4 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index fe8139f367..cdb5c6f171 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -222,6 +222,7 @@ F: disas/microblaze.c
 MIPS TCG CPUs
 M: Aleksandar Markovic 
 R: Aurelien Jarno 
+R: Jiaxun Yang 
 R: Aleksandar Rikalo 
 S: Maintained
 F: target/mips/
@@ -384,6 +385,7 @@ S: Maintained
 F: target/arm/kvm.c
 
 MIPS KVM CPUs
+M: Huacai Chen 
 M: Aleksandar Markovic 
 S: Odd Fixes
 F: target/mips/kvm.c
@@ -2743,6 +2745,8 @@ F: disas/i386.c
 MIPS TCG target
 M: Aleksandar Markovic 
 R: Aurelien Jarno 
+R: Huacai Chen 
+R: Jiaxun Yang 
 R: Aleksandar Rikalo 
 S: Maintained
 F: tcg/mips/
-- 
2.21.3

[PATCH] linux-user: Add strace support for printing arguments for ioctls used for terminals and serial lines

[Filip Bozuta]

Functions "print_ioctl()" and "print_syscall_ret_ioctl()" are used
to print arguments of "ioctl()" with "-strace". These functions
use "thunk_print()", which is defined in "thunk.c", to print the
contents of ioctl's third arguments that are not basic types.
However, this function doesn't handle ioctls of group ioctl_tty which
are used for terminals and serial lines. These ioctls use a type
"struct termios" which thunk type is defined in a non standard
way using "STRUCT_SPECIAL()". This means that this type is not decoded
regularly using "thunk_convert()" and uses special converting functions
"target_to_host_termios()" and "host_to_target_termios()", which are defined
in "syscall.c" to decode it's values. For simillar reasons, this type is
also not printed regularly using "thunk_print()". That is the reason
why a separate printing function "print_termios()" is defined in
file "strace.c". This function decodes and prints flag values of the
"termios" structure.

Implementation notes:

Function "print_termios()" was implemented in "strace.c" using
an existing function "print_flags()" to print flag values of
"struct termios" fields. These flag values were defined
using an existing macro "FLAG_TARGET()" that generates aproppriate
target flag values and string representations of these flags.
This function was declared in "qemu.h" so that it can be accessed
in "syscall.c". Type "StructEntry" defined in "exec/user/thunk.h"
contains information that is used to decode structure values.
Field "void print(void *arg)" was added in this structure as a
special print function. Also, function "thunk_print()" was changed
a little so that it uses this special print function in case
it is defined. This printing function was instantiated with the
defined "print_termios()" in "syscall.c" in "struct_termios_def".

Signed-off-by: Filip Bozuta 
---
 include/exec/user/thunk.h |   1 +
 linux-user/qemu.h |   2 +
 linux-user/strace.c   | 193 ++
 linux-user/syscall.c  |   1 +
 thunk.c   |  23 +++--
 5 files changed, 211 insertions(+), 9 deletions(-)

diff --git a/include/exec/user/thunk.h b/include/exec/user/thunk.h
index 7992475c9f..a5bbb2c733 100644
--- a/include/exec/user/thunk.h
+++ b/include/exec/user/thunk.h
@@ -55,6 +55,7 @@ typedef struct {
 int *field_offsets[2];
 /* special handling */
 void (*convert[2])(void *dst, const void *src);
+void (*print)(void *arg);
 int size[2];
 int align[2];
 const char *name;
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 5c964389c1..e51a0ededb 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -706,6 +706,8 @@ static inline uint64_t target_offset64(uint64_t word0, 
uint64_t word1)
 }
 #endif /* TARGET_ABI_BITS != 32 */
 
+extern void print_termios(void *arg);
+
 /**
  * preexit_cleanup: housekeeping before the guest exits
  *
diff --git a/linux-user/strace.c b/linux-user/strace.c
index 5235b2260c..7b5408cf4a 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1193,6 +1193,138 @@ UNUSED static struct flags falloc_flags[] = {
 #endif
 };
 
+UNUSED static struct flags termios_iflags[] = {
+FLAG_TARGET(IGNBRK),
+FLAG_TARGET(BRKINT),
+FLAG_TARGET(IGNPAR),
+FLAG_TARGET(PARMRK),
+FLAG_TARGET(INPCK),
+FLAG_TARGET(ISTRIP),
+FLAG_TARGET(INLCR),
+FLAG_TARGET(IGNCR),
+FLAG_TARGET(ICRNL),
+FLAG_TARGET(IUCLC),
+FLAG_TARGET(IXON),
+FLAG_TARGET(IXANY),
+FLAG_TARGET(IXOFF),
+FLAG_TARGET(IMAXBEL),
+FLAG_END,
+};
+
+UNUSED static struct flags termios_oflags[] = {
+FLAG_TARGET(OPOST),
+FLAG_TARGET(OLCUC),
+FLAG_TARGET(ONLCR),
+FLAG_TARGET(OCRNL),
+FLAG_TARGET(ONOCR),
+FLAG_TARGET(ONLRET),
+FLAG_TARGET(OFILL),
+FLAG_TARGET(OFDEL),
+FLAG_END,
+};
+
+UNUSED static struct flags termios_oflags_NLDLY[] = {
+FLAG_TARGET(NL0),
+FLAG_TARGET(NL1),
+FLAG_END,
+};
+
+UNUSED static struct flags termios_oflags_CRDLY[] = {
+FLAG_TARGET(CR0),
+FLAG_TARGET(CR1),
+FLAG_TARGET(CR2),
+FLAG_TARGET(CR3),
+FLAG_END,
+};
+
+UNUSED static struct flags termios_oflags_TABDLY[] = {
+FLAG_TARGET(TAB0),
+FLAG_TARGET(TAB1),
+FLAG_TARGET(TAB2),
+FLAG_TARGET(TAB3),
+FLAG_END,
+};
+
+UNUSED static struct flags termios_oflags_VTDLY[] = {
+FLAG_TARGET(VT0),
+FLAG_TARGET(VT1),
+FLAG_END,
+};
+
+UNUSED static struct flags termios_oflags_FFDLY[] = {
+FLAG_TARGET(FF0),
+FLAG_TARGET(FF1),
+FLAG_END,
+};
+
+UNUSED static struct flags termios_oflags_BSDLY[] = {
+FLAG_TARGET(BS0),
+FLAG_TARGET(BS1),
+FLAG_END,
+};
+
+UNUSED static struct flags termios_cflags_CBAUD[] = {
+FLAG_TARGET(B0),
+FLAG_TARGET(B50),
+FLAG_TARGET(B75),
+FLAG_TARGET(B110),
+FLAG_TARGET(B134),
+FLAG_TARGET(B150),
+FLAG_TARGET(B200),
+FLAG_TARGET(B300),
+FLAG_TARGET(B600),
+FLAG_TARGET(B1200),

Re: [PATCH v6] scripts/simplebench: compare write request performance

[Vladimir Sementsov-Ogievskiy]

14.07.2020 22:36, Eduardo Habkost wrote:

On Tue, Jul 14, 2020 at 07:05:05PM +0300, Vladimir Sementsov-Ogievskiy wrote:

Hi Eduardo!

Could you please stage this in your branch, to be pulled for 5.2 later?

Queued, thanks!



Thank you!

--
Best regards,
Vladimir

[PATCH v1] util: OpenBSD build fix

[David CARLIER]

>From e2103b86b031ab74ff4c8dd0a3944cb488c9333e Mon Sep 17 00:00:00 2001
From: David Carlier 
Date: Tue, 14 Jul 2020 21:34:59 +0100
Subject: [PATCH] util: OpenBSD build fix.

thread id implementation, using getthrid syscall.
qemu_exec_dir implementation as beast as we can as
path is not always possible to resolve this on this platform.

Signed-off-by: David Carlier 
---
 util/oslib-posix.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 72907d4d7f..4a0cce15b4 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -56,6 +56,10 @@
 #include 
 #endif

+#ifdef __OpenBSD__
+#include 
+#endif
+
 #ifdef __APPLE__
 #include 
 #endif
@@ -100,6 +104,8 @@ int qemu_get_thread_id(void)
 return (int)tid;
 #elif defined(__NetBSD__)
 return _lwp_self();
+#elif defined(__OpenBSD__)
+return getthrid();
 #else
 return getpid();
 #endif
@@ -408,6 +414,23 @@ void qemu_init_exec_dir(const char *argv0)
 }
 }
 }
+#elif defined(__OpenBSD__)
+{
+
+char **args;
+size_t len;
+int mib[4] = {CTL_KERN, KERN_PROC_ARGS, getpid(), KERN_PROC_ARGV};
+
+*buf = 0;
+if (!sysctl(mib, ARRAY_SIZE(mib), NULL, , NULL, 0)) {
+args = malloc(len);
+if (!sysctl(mib, ARRAY_SIZE(mib), args, , NULL, 0)) {
+p = realpath(*args, buf);
+}
+
+free(args);
+}
+}
 #endif
 /* If we don't have any way of figuring out the actual executable
location then try argv[0].  */
-- 
2.27.0

Re: [PATCH v6] scripts/simplebench: compare write request performance

[Eduardo Habkost]

On Tue, Jul 14, 2020 at 07:05:05PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi Eduardo!
> 
> Could you please stage this in your branch, to be pulled for 5.2 later?

Queued, thanks!

> 
> 14.07.2020 18:50, Andrey Shinkevich wrote:
> > The script 'bench_write_req.py' allows comparing performances of write
> > request for two qemu-img binary files.
> > An example with (qemu-img binary 1) and without (qemu-img binary 2) the
> > applied patch "qcow2: skip writing zero buffers to empty COW areas"
> > (git commit ID: c8bb23cbdbe32f5) has the following results:
> > 
> > SSD:
> >   ---  ---
> >  
> >0.10 +- 0.00 8.16 +- 0.65
> >   0.10 +- 0.00 7.37 +- 1.10
> >7.40 +- 1.08 21.97 +- 4.19
> >  2.14 +- 0.94 8.48 +- 1.66
> >   ---  ---
> > HDD:
> >   ---  ---
> >  
> >2.30 +- 0.01 6.19 +- 0.06
> >   2.20 +- 0.09 6.20 +- 0.06
> >8.32 +- 0.16 8.26 +- 0.14
> >  8.20 +- 0.05 6.26 +- 0.10
> >   ---  ---
> > 
> > Suggested-by: Denis V. Lunev 
> > Suggested-by: Vladimir Sementsov-Ogievskiy 
> > Signed-off-by: Andrey Shinkevich 
> > Reviewed-by: Vladimir Sementsov-Ogievskiy 
> > ---
> >   scripts/simplebench/bench_write_req.py | 170 
> > +
> >   1 file changed, 170 insertions(+)
> >   create mode 100755 scripts/simplebench/bench_write_req.py
> > 
> > diff --git a/scripts/simplebench/bench_write_req.py 
> > b/scripts/simplebench/bench_write_req.py
> > new file mode 100755
> > index 000..ca1178f
> > --- /dev/null
> > +++ b/scripts/simplebench/bench_write_req.py
> > @@ -0,0 +1,170 @@
> > +#!/usr/bin/env python3
> > +#
> > +# Test to compare performance of write requests for two qemu-img binary 
> > files.
> > +#
> > +# The idea of the test comes from intention to check the benefit of 
> > c8bb23cbdbe
> > +# "qcow2: skip writing zero buffers to empty COW areas".
> > +#
> > +# Copyright (c) 2020 Virtuozzo International GmbH.
> > +#
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 2 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program.  If not, see .
> > +#
> > +
> > +
> > +import sys
> > +import os
> > +import subprocess
> > +import simplebench
> > +
> > +
> > +def bench_func(env, case):
> > +""" Handle one "cell" of benchmarking table. """
> > +return bench_write_req(env['qemu_img'], env['image_name'],
> > +   case['block_size'], case['block_offset'],
> > +   case['cluster_size'])
> > +
> > +
> > +def qemu_img_pipe(*args):
> > +'''Run qemu-img and return its output'''
> > +subp = subprocess.Popen(list(args),
> > +stdout=subprocess.PIPE,
> > +stderr=subprocess.STDOUT,
> > +universal_newlines=True)
> > +exitcode = subp.wait()
> > +if exitcode < 0:
> > +sys.stderr.write('qemu-img received signal %i: %s\n'
> > + % (-exitcode, ' '.join(list(args
> > +return subp.communicate()[0]
> > +
> > +
> > +def bench_write_req(qemu_img, image_name, block_size, block_offset,
> > +cluster_size):
> > +"""Benchmark write requests
> > +
> > +The function creates a QCOW2 image with the given path/name. Then it 
> > runs
> > +the 'qemu-img bench' command and makes series of write requests on the
> > +image clusters. Finally, it returns the total time of the write 
> > operations
> > +on the disk.
> > +
> > +qemu_img -- path to qemu_img executable file
> > +image_name   -- QCOW2 image name to create
> > +block_size   -- size of a block to write to clusters
> > +block_offset -- offset of the block in clusters
> > +cluster_size -- size of the image cluster
> > +
> > +Returns {'seconds': int} on success and {'error': str} on failure.
> > +Return value is compatible with simplebench lib.
> > +"""
> > +
> > +if not os.path.isfile(qemu_img):
> > +print(f'File not found: {qemu_img}')
> > +sys.exit(1)
> > +
> > +image_dir = os.path.dirname(os.path.abspath(image_name))
> > +if not os.path.isdir(image_dir):
> > +print(f'Path not found: 

Re: [PATCH v3 4/9] host trust limitation: Rework the "memory-encryption" property

[Richard Henderson]

On 6/18/20 7:05 PM, David Gibson wrote:
> Currently the "memory-encryption" property is only looked at once we get to
> kvm_init().  Although protection of guest memory from the hypervisor isn't
> something that could really ever work with TCG, it's not conceptually tied
> to the KVM accelerator.
> 
> In addition, the way the string property is resolved to an object is
> almost identical to how a QOM link property is handled.
> 
> So, create a new "host-trust-limitation" link property which sets this QOM
> interface link directly in the machine.  For compatibility we keep the
> "memory-encryption" property, but now implemented in terms of the new
> property.
> 
> Signed-off-by: David Gibson 
> ---
>  accel/kvm/kvm-all.c | 23 +++
>  hw/core/machine.c   | 41 -
>  include/hw/boards.h |  2 +-
>  3 files changed, 44 insertions(+), 22 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [PATCH v5 10/12] python/machine.py: split shutdown into hard and soft flavors

[Philippe Mathieu-Daudé]

On 7/14/20 8:13 PM, John Snow wrote:
> 
> 
> On 7/14/20 12:13 AM, Cleber Rosa wrote:
>> On Fri, Jul 10, 2020 at 01:06:47AM -0400, John Snow wrote:
>>> This is done primarily to avoid the 'bare except' pattern, which
>>> suppresses all exceptions during shutdown and can obscure errors.
>>>
>>> Replace this with a pattern that isolates the different kind of shutdown
>>> paradigms (_hard_shutdown and _soft_shutdown), and a new fallback shutdown
>>> handler (_do_shutdown) that gracefully attempts one before the other.
>>>
>>> This split now also ensures that no matter what happens,
>>> _post_shutdown() is always invoked.
>>>
>>> shutdown() changes in behavior such that if it attempts to do a graceful
>>> shutdown and is unable to, it will now always raise an exception to
>>> indicate this. This can be avoided by the test writer in three ways:
>>>
>>> 1. If the VM is expected to have already exited or is in the process of
>>> exiting, wait() can be used instead of shutdown() to clean up resources
>>> instead. This helps avoid race conditions in shutdown.
>>>
>>> 2. If a test writer is expecting graceful shutdown to fail, shutdown
>>> should be called in a try...except block.
>>>
>>> 3. If the test writer has no interest in performing a graceful shutdown
>>> at all, kill() can be used instead.
>>>
>>>
>>> Handling shutdown in this way makes it much more explicit which type of
>>> shutdown we want and allows the library to report problems with this
>>> process.
>>>
>>> Signed-off-by: John Snow 
>>> ---
>>>  python/qemu/machine.py | 95 +++---
>>>  1 file changed, 80 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
>>> index aaa173f046..b24ce8a268 100644
>>> --- a/python/qemu/machine.py
>>> +++ b/python/qemu/machine.py
>>> @@ -48,6 +48,12 @@ class QEMUMachineAddDeviceError(QEMUMachineError):
>>>  """
>>>  
>>>  
>>> +class AbnormalShutdown(QEMUMachineError):
>>> +"""
>>> +Exception raised when a graceful shutdown was requested, but not 
>>> performed.
>>> +"""
>>> +
>>> +
>>>  class MonitorResponseError(qmp.QMPError):
>>>  """
>>>  Represents erroneous QMP monitor reply
>>> @@ -365,6 +371,7 @@ def _early_cleanup(self) -> None:
>>>  """
>>>  Perform any cleanup that needs to happen before the VM exits.
>>>  
>>> +May be invoked by both soft and hard shutdown in failover 
>>> scenarios.
>>>  Called additionally by _post_shutdown for comprehensive cleanup.
>>>  """
>>>  # If we keep the console socket open, we may deadlock waiting
>>> @@ -374,32 +381,90 @@ def _early_cleanup(self) -> None:
>>>  self._console_socket.close()
>>>  self._console_socket = None
>>>  
>>> +def _hard_shutdown(self) -> None:
>>> +"""
>>> +Perform early cleanup, kill the VM, and wait for it to terminate.
>>> +
>>> +:raise subprocess.Timeout: When timeout is exceeds 60 seconds
>>> +waiting for the QEMU process to terminate.
>>> +"""
>>> +self._early_cleanup()
>>
>> Like I commented on patch 5, I don't think the *current* type of
>> cleanup done is needed on a scenario like this...
>>
>>> +self._popen.kill()
>>
>> ... as I don't remember QEMU's SIGKILL handler to be susceptible to
>> the race condition that motivated the closing of the console file in
>> the first place.  But, I also can not prove it's not susceptible at
>> this time.
>>
> 
> It probably isn't. It was for consistency in when and where that hook is
> called, again. It does happen to be "pointless" here.
> 
>> Note: I have some old patches that added tests for QEMUMachine itself.
>> I intend to respin them on top of your work, so we may have a clearer
>> understanding of the QEMU behaviors we need to handle.  So, feel free
>> to take the prudent route here, and keep the early cleanup.
>>
> 
> Oh, adding formal tests to this folder would be incredible; especially
> if we wanted to package it on PyPI. Basically a necessity.
> 
>> Reviewed-by: Cleber Rosa 
>> Tested-by: Cleber Rosa 
>>
> 

Queuing with this hunk from jsnow:

-- >8 --
@@ -455,6 +444,9 @@ def shutdown(self, has_quit: bool = False,
 Terminate the VM (gracefully if possible) and perform cleanup.
 Cleanup will always be performed.

+If the VM has not yet been launched, or shutdown(), wait(), or
kill()
+have already been called, this method does nothing.
+
 :param has_quit: When true, do not attempt to issue 'quit' QMP
command.
 :param hard: When true, do not attempt graceful shutdown, and
  suppress the SIGKILL warning log message.
---

Re: [PULL v3 00/32] AVR port

[Philippe Mathieu-Daudé]

On 7/12/20 4:31 PM, Peter Maydell wrote:
> On Sat, 11 Jul 2020 at 10:07, Philippe Mathieu-Daudé  wrote:
>> 
>> 8bit AVR port from Michael Rolnik.
>>
>> Michael started to work on the AVR port few years ago [*] and kept
>> improving the code over various series.
...
> 
> 
> Applied, thanks.
> 
> Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
> for any user-visible changes.

Oh, Thomas already did that :) Thanks!

Phil.

Re: [PATCH] .travis.yml: skip ppc64abi32-linux-user with plugins

[Philippe Mathieu-Daudé]

On 7/14/20 7:55 PM, Alex Bennée wrote:
> We actually see failures on threadcount running without plugins:
> 
>   retry.py -n 1000 -c -- \
> ./ppc64abi32-linux-user/qemu-ppc64abi32 \
> ./tests/tcg/ppc64abi32-linux-user/threadcount
> 
> which reports:
> 
>   0: 978 times (97.80%), avg time 0.270 (0.01 varience/0.08 deviation)
>   -6: 21 times (2.10%), avg time 0.336 (0.01 varience/0.12 deviation)
>   -11: 1 times (0.10%), avg time 0.502 (0.00 varience/0.00 deviation)
>   Ran command 1000 times, 978 passes
> 
> But when running with plugins we hit the failure a lot more often:
> 
>   0: 91 times (91.00%), avg time 0.302 (0.04 varience/0.19 deviation)
>   -11: 9 times (9.00%), avg time 0.558 (0.01 varience/0.11 deviation)
>   Ran command 100 times, 91 passes
> 
> The crash occurs in guest code which is the same in both pass and fail
> cases. However we see various messages reported on the console about
> corrupted memory lists which seems to imply the guest memory allocation
> is corrupted. This lines up with the seg fault being in the guest
> __libc_free function. So we think this is a guest bug which is
> exacerbated by various modes of translation. If anyone has access to
> real hardware to soak test the test case we could prove this properly.
> 
> Signed-off-by: Alex Bennée 
> Cc: David Gibson 
> Cc: Philippe Mathieu-Daudé 

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 

> ---
>  .travis.yml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index ab429500fc..6695c0620f 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -350,9 +350,10 @@ jobs:
>  # Run check-tcg against linux-user (with plugins)
>  # we skip sparc64-linux-user until it has been fixed somewhat
>  # we skip cris-linux-user as it doesn't use the common run loop
> +# we skip ppc64abi32-linux-user as it seems to have a broken libc
>  - name: "GCC plugins check-tcg (user)"
>env:
> -- CONFIG="--disable-system --enable-plugins --enable-debug-tcg 
> --target-list-exclude=sparc64-linux-user,cris-linux-user"
> +- CONFIG="--disable-system --enable-plugins --enable-debug-tcg 
> --target-list-exclude=sparc64-linux-user,cris-linux-user,ppc64abi32-linux-user"
>  - TEST_BUILD_CMD="make build-tcg"
>  - TEST_CMD="make check-tcg"
>  - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
> 

Re: [PATCH v5 00/16] python: add mypy support to python/qemu

[Philippe Mathieu-Daudé]

On 7/10/20 7:22 AM, John Snow wrote:
> Based-on: 20200710050649.32434-1-js...@redhat.com
> 
> This series modifies the python/qemu library to comply with mypy --strict,
> pylint, and flake8.
> This requires my "refactor shutdown" patch as a pre-requisite.
> 
[...]
> 
> These should all 100% pass.
> 
> John Snow (16):
>   python/qmp.py: Define common types
>   iotests.py: use qemu.qmp type aliases
>   python/qmp.py: re-absorb MonitorResponseError
>   python/qmp.py: Do not return None from cmd_obj
>   python/qmp.py: add casts to JSON deserialization
>   python/qmp.py: add QMPProtocolError

I've applied patches 1 to 6 on python-next, because unfortunately
then patch 7 now conflicts.

We'll merge them in 5.2 with the 'make check-python' rule patch last,
to avoid regressions.

Thanks,

Phil.

>   python/machine.py: Fix monitor address typing
>   python/machine.py: reorder __init__
>   python/machine.py: Don't modify state in _base_args()
>   python/machine.py: Handle None events in events_wait
>   python/machine.py: use qmp.command
>   python/machine.py: Add _qmp access shim
>   python/machine.py: fix _popen access
>   python/qemu: make 'args' style arguments immutable
>   iotests.py: Adjust HMP kwargs typing
>   python/qemu: Add mypy type annotations

Re: [PATCH v3 1/9] host trust limitation: Introduce new host trust limitation interface

[Richard Henderson]

On 6/18/20 7:05 PM, David Gibson wrote:
> Several architectures have mechanisms which are designed to protect guest
> memory from interference or eavesdropping by a compromised hypervisor.  AMD
> SEV does this with in-chip memory encryption and Intel has a similar
> mechanism.  POWER's Protected Execution Framework (PEF) accomplishes a
> similar goal using an ultravisor and new memory protection features,
> instead of encryption.
> 
> To (partially) unify handling for these, this introduces a new
> HostTrustLimitation QOM interface.
> 
> Signed-off-by: David Gibson 
> ---
>  backends/Makefile.objs   |  2 ++
>  backends/host-trust-limitation.c | 29 
>  include/exec/host-trust-limitation.h | 33 
>  include/qemu/typedefs.h  |  1 +
>  4 files changed, 65 insertions(+)
>  create mode 100644 backends/host-trust-limitation.c
>  create mode 100644 include/exec/host-trust-limitation.h

Reviewed-by: Richard Henderson 

r~

Re: [PULL v3] Block layer patches

[Peter Maydell]

On Mon, 13 Jul 2020 at 15:33, Kevin Wolf  wrote:
>
> The following changes since commit 6c87d9f311dba0641bdc2df556056938a8bf2a12:
>
>   Merge remote-tracking branch 'remotes/elmarco/tags/chardev-pull-request' 
> into staging (2020-07-13 09:34:24 +0100)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 7637b225a8d59a3b9b0e31bbc4eb8a0788792ac5:
>
>   block: Avoid stale pointer dereference in blk_get_aio_context() (2020-07-13 
> 15:57:13 +0200)
>
> 
> Block layer patches:
>
> - file-posix: Mitigate file fragmentation with extent size hints
> - Tighten qemu-img rules on missing backing format
> - qemu-img map: Don't limit block status request size
> - Fix crash with virtio-scsi and iothreads
>

Applied (fixed version), thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.

-- PMM

Re: [PATCH v5 00/12] python/machine.py: refactor shutdown

[Philippe Mathieu-Daudé]

On 7/10/20 7:06 AM, John Snow wrote:
> v5: More or less rewritten.
> 
> This series is motivated by a desire to move python/qemu onto a strict
> mypy/pylint regime to help prevent regressions in the python codebase.
> 
> 1. Remove the "bare except" pattern in the existing shutdown code, which
>can mask problems and make debugging difficult.
> 
> 2. Ensure that post-shutdown cleanup is always performed, even when
>graceful termination fails.
> 
> 3. Unify cleanup paths such that no matter how the VM is terminated, the
>same functions and steps are always taken to reset the object state.
> 
> 4. Rewrite shutdown() such that any error encountered when attempting a
>graceful shutdown will be raised as an AbnormalShutdown exception.
>The pythonic idiom is to allow the caller to decide if this is a
>problem or not.
> 
> Previous versions of this series did not engage the fourth goal, and ran
> into race conditions. When I was trying to allow shutdown to succeed if
> QEMU was already closed, it became impossible to tell in which cases
> QEMU not being present was "OK" and in which cases it was evidence of a
> problem.
> 
> This refactoring is even more explicit. If a graceful shutdown is
> requested and cannot be performed, an exception /will/ be raised.
> 
> In cases where the test writer expects QEMU to already have exited,
> vm.wait() should be used in preference to vm.shutdown(). In cases where
> a graceful shutdown is not interesting or necessary to the test,
> vm.kill() should be used.
> 
> John Snow (12):
>   python/machine.py: consolidate _post_shutdown()
>   python/machine.py: Close QMP socket in cleanup
>   python/machine.py: Add _early_cleanup hook
>   python/machine.py: Perform early cleanup for wait() calls, too
>   python/machine.py: Prohibit multiple shutdown() calls
>   python/machine.py: Add a configurable timeout to shutdown()
>   python/machine.py: Make wait() call shutdown()
>   tests/acceptance: wait() instead of shutdown() where appropriate
>   tests/acceptance: Don't test reboot on cubieboard
>   python/machine.py: split shutdown into hard and soft flavors
>   python/machine.py: re-add sigkill warning suppression
>   python/machine.py: change default wait timeout to 3 seconds

Series (finally) queued on python-next, thanks.

Re: [PATCH 1/1] MAINTAINERS: Add Python library stanza

[John Snow]

On 7/13/20 9:35 AM, Alex Bennée wrote:
> 
> John Snow  writes:
> 
>> I'm proposing that I split the actual Python library off from the other
>> miscellaneous python scripts we have and declare it maintained. Add
>> myself as a maintainer of this folder, along with Cleber.
>>
>> Signed-off-by: John Snow 
>> ---
>>  MAINTAINERS | 9 -
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 6aa54f7f8f..fe1dcd5a76 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -2280,11 +2280,18 @@ S: Maintained
>>  F: include/sysemu/cryptodev*.h
>>  F: backends/cryptodev*.c
>>  
>> +Python library
>> +M: John Snow 
>> +M: Cleber Rosa 
>> +R: Eduardo Habkost 
>> +S: Maintained
>> +F: python/*
> 
> I don't think that's equivalent to what you drop bellow:
> 
> F:drivers/net/all files in and below drivers/net
> F:drivers/net/*   all files in drivers/net, but not below
> 
> So I think you should drop the *
> 
>> +T: git https://gitlab.com/jsnow/qemu.git python
>> +
>>  Python scripts
>>  M: Eduardo Habkost 
>>  M: Cleber Rosa 
>>  S: Odd fixes
>> -F: python/qemu/*py
>>  F: scripts/*.py
>>  F: tests/*.py
> 
> Otherwise:
> 
> Reviewed-by: Alex Bennée 
> 

ACK on the edit, I think otherwise I'm still technically waiting for
Cleber to approve this before I would ask for someone to stage it.

--js

Re: [PATCH v5 05/12] python/machine.py: Prohibit multiple shutdown() calls

[John Snow]

On 7/13/20 10:48 PM, Cleber Rosa wrote:
> On Fri, Jul 10, 2020 at 01:06:42AM -0400, John Snow wrote:
>> If the VM is not launched, don't try to shut it down. As a change,
>> _post_shutdown now unconditionally also calls _early_cleanup in order to
>> offer comprehensive object cleanup in failure cases.
>>
>> As a courtesy, treat it as a NOP instead of rejecting it as an
>> error. This is slightly nicer for acceptance tests where vm.shutdown()
>> is issued unconditionally in tearDown callbacks.
>>
>> Signed-off-by: John Snow 
>> ---
>>  python/qemu/machine.py | 14 +-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
>> index cac466fbe6..e66a7d66dd 100644
>> --- a/python/qemu/machine.py
>> +++ b/python/qemu/machine.py
>> @@ -283,6 +283,13 @@ def _post_launch(self):
>>  self._qmp.accept()
>>  
>>  def _post_shutdown(self):
>> +"""
>> +Called to cleanup the VM instance after the process has exited.
>> +May also be called after a failed launch.
>> +"""
>> +# Comprehensive reset for the failed launch case:
>> +self._early_cleanup()
>> +
> 
> I'm not following why this is needed here.  AFAICT, the closing of the
> console socket file was introduced when the QEMU process was alive,
> and doing I/O on the serial device attached to the console file (and
> was only necessary because of that).  In those scenarios, a race
> between the time of sending the QMP command to quit, and getting a
> response from QMP could occur.
> 
> But here, IIUC, there's no expectations for the QEMU process to be alive.
> To the best of my understanding and testing, this line did not contribute
> to the robustness of the shutdown behavior.
> 
> Finally, given that currently, only the closing of the console socket
> is done within _early_cleanup(), and that is conditional, this also does
> no harm AFAICT.  If more early cleanups operations were needed, then I
> would feel less bothered about calling it here.
> 
>>  if self._qmp:
>>  self._qmp.close()
>>  self._qmp = None
>> @@ -328,7 +335,7 @@ def launch(self):
>>  self._launch()
>>  self._launched = True
>>  except:
>> -self.shutdown()
>> +self._post_shutdown()
>>  
>>  LOG.debug('Error launching VM')
>>  if self._qemu_full_args:
>> @@ -357,6 +364,8 @@ def _launch(self):
>>  def _early_cleanup(self) -> None:
>>  """
>>  Perform any cleanup that needs to happen before the VM exits.
>> +
>> +Called additionally by _post_shutdown for comprehensive cleanup.
>>  """
>>  # If we keep the console socket open, we may deadlock waiting
>>  # for QEMU to exit, while QEMU is waiting for the socket to
>> @@ -377,6 +386,9 @@ def shutdown(self, has_quit=False, hard=False):
>>  """
>>  Terminate the VM and clean up
>>  """
>> +if not self._launched:
>> +return
>> +
> 
> Because of the additional logic, it'd be a good opportunity to make
> the docstring more accurate.  This method may now *not* do *any* of if
> termination and cleaning (while previously it would attempt those
> anyhow).
> 

The docstring gets hit with a polish beam later in the series, but still
neglects that it might do nothing.

I squashed in a change to patch #10 and pushed to my gitlab to add this
clarification to the docstring.

https://gitlab.com/jsnow/qemu/-/tree/python-package-shutdown

https://gitlab.com/jsnow/qemu/-/commit/b3f14deb730fda9bb2a28a9366a8096d3617f4f4

Re: [PATCH-for-5.1 3/4] qemu-common: Document qemu_find_file()

[Peter Maydell]

On Tue, 14 Jul 2020 at 17:43, Philippe Mathieu-Daudé  wrote:
>
> Document qemu_find_file(), in particular the returned
> value which must be freed.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  include/qemu-common.h | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index d0142f29ac..d6a08259d3 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -110,6 +110,20 @@ const char *qemu_get_vm_name(void);
>
>  #define QEMU_FILE_TYPE_BIOS   0
>  #define QEMU_FILE_TYPE_KEYMAP 1
> +/**
> + * qemu_find_file:
> + * @type: QEMU_FILE_TYPE_BIOS (for BIOS, VGA BIOS)
> + *or QEMU_FILE_TYPE_KEYMAP (for keymaps).
> + * @name: File name
> + *
> + * Search for @name file in the data directories, either configured at
> + * build time (DATADIR) or registered with the -L command line option.
> + *
> + * The caller must use g_free() to free the returned data when it is
> + * no longer required.
> + *
> + * Returns: absolute path to the file or NULL on error.
> + */
>  char *qemu_find_file(int type, const char *name);

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PULL 00/15] riscv-to-apply queue

[Peter Maydell]

On Tue, 14 Jul 2020 at 01:44, Alistair Francis  wrote:
>
> The following changes since commit 20c1df5476e1e9b5d3f5b94f9f3ce01d21f14c46:
>
>   Merge remote-tracking branch 
> 'remotes/kraxel/tags/fixes-20200713-pull-request' into staging (2020-07-13 
> 16:58:44 +0100)
>
> are available in the Git repository at:
>
>   g...@github.com:alistair23/qemu.git tags/pull-riscv-to-apply-20200713
>
> for you to fetch changes up to cfad709bceb629a4ebeb5d8a3acd1871b9a6436b:
>
>   target/riscv: Fix pmp NA4 implementation (2020-07-13 17:25:37 -0700)
>
> 
> This is a colection of bug fixes and small imrprovements for RISC-V.
>
> This includes some vector extensions fixes, a PMP bug fix, OpenTitan
> UART bug fix and support for OpenSBI dynamic firmware.
>
> 



Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.

-- PMM

Re: [PATCH v7 20/47] block: Iterate over children in refresh_limits

[Andrey Shinkevich]

On 25.06.2020 18:21, Max Reitz wrote:

Instead of looking at just bs->file and bs->backing, we should look at
all children that could end up receiving forwarded requests.

Signed-off-by: Max Reitz 
---
  block/io.c | 32 
  1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/block/io.c b/block/io.c
index c2af7711d6..37057f13e0 100644
--- a/block/io.c
+++ b/block/io.c
@@ -135,6 +135,8 @@ static void bdrv_merge_limits(BlockLimits *dst, const 
BlockLimits *src)
  void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
  {
  BlockDriver *drv = bs->drv;
+BdrvChild *c;
+bool have_limits;
  Error *local_err = NULL;
  
  memset(>bl, 0, sizeof(bs->bl));

@@ -149,14 +151,21 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error 
**errp)
  drv->bdrv_co_preadv_part) ? 1 : 512;
  
  /* Take some limits from the children as a default */

-if (bs->file) {
-bdrv_refresh_limits(bs->file->bs, _err);
-if (local_err) {
-error_propagate(errp, local_err);
-return;
+have_limits = false;
+QLIST_FOREACH(c, >children, next) {
+if (c->role & (BDRV_CHILD_DATA | BDRV_CHILD_FILTERED | BDRV_CHILD_COW))
+{
+bdrv_refresh_limits(c->bs, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+bdrv_merge_limits(>bl, >bs->bl);
+have_limits = true;
  }
-bdrv_merge_limits(>bl, >file->bs->bl);
-} else {
+}
+
+if (!have_limits) {



This conditioned piece of code worked with (bs->file == NULL) only.

Now, it works only if there are neither bs->file, nor bs->backing, nor 
else filtered children.


Is it OK and doesn't break the logic for all cases?

Andrey



  bs->bl.min_mem_alignment = 512;
  bs->bl.opt_mem_alignment = qemu_real_host_page_size;
  
@@ -164,15 +173,6 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)

  bs->bl.max_iov = IOV_MAX;
  }
  
-if (bs->backing) {

-bdrv_refresh_limits(bs->backing->bs, _err);
-if (local_err) {
-error_propagate(errp, local_err);
-return;
-}
-bdrv_merge_limits(>bl, >backing->bs->bl);
-}
-
  /* Then let the driver override it */
  if (drv->bdrv_refresh_limits) {
  drv->bdrv_refresh_limits(bs, errp);

Re: [PATCH 1/1] python: add check-python target

[John Snow]

On 7/14/20 12:30 AM, Cleber Rosa wrote:
> On Mon, Jul 13, 2020 at 09:30:26PM -0400, John Snow wrote:
>> Move pylintrc and flake8 up to the root of the python folder where
>> they're the most useful. Add a requirements.cqa.txt file to house
>> the requirements necessary to build a venv sufficient for running
>> code quality analysis on the python folder. Add a makefile that
>> will build the venv and run the tests.
>>
>> Signed-off-by: John Snow 
>> ---
>>  Makefile|  1 +
>>  python/{qemu => }/.flake8   |  0
>>  python/Makefile.include | 33 +
>>  python/{qemu => }/pylintrc  |  1 +
>>  python/requirements.cqa.txt |  3 +++
>>  5 files changed, 38 insertions(+)
>>  rename python/{qemu => }/.flake8 (100%)
>>  create mode 100644 python/Makefile.include
>>  rename python/{qemu => }/pylintrc (99%)
>>  create mode 100644 python/requirements.cqa.txt
>>
>> diff --git a/Makefile b/Makefile
>> index b1b8a5a6d0..41808be392 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -478,6 +478,7 @@ dummy := $(call unnest-vars,, \
>>  trace-obj-y)
>>  
>>  include $(SRC_PATH)/tests/Makefile.include
>> +include $(SRC_PATH)/python/Makefile.include
>>  
>>  all: $(DOCS) $(if $(BUILD_DOCS),sphinxdocs) $(TOOLS) $(HELPERS-y) 
>> recurse-all modules $(vhost-user-json-y)
>>  
>> diff --git a/python/qemu/.flake8 b/python/.flake8
>> similarity index 100%
>> rename from python/qemu/.flake8
>> rename to python/.flake8
>> diff --git a/python/Makefile.include b/python/Makefile.include
>> new file mode 100644
>> index 00..917808e2f1
>> --- /dev/null
>> +++ b/python/Makefile.include
>> @@ -0,0 +1,33 @@
>> +# -*- Mode: makefile -*-
>> +
>> +PYLIB_VENV_DIR=$(BUILD_DIR)/venv/cqa
>> +PYLIB_VENV_REQ=$(SRC_PATH)/python/requirements.cqa.txt
>> +
>> +$(PYLIB_VENV_DIR): $(PYLIB_VENV_REQ)
>> +$(call quiet-command, \
>> +$(PYTHON) -m venv $@, \
>> +VENV, $@)
>> +$(call quiet-command, \
>> +$(PYLIB_VENV_DIR)/bin/python3 -m pip -q install -r 
>> $(PYLIB_VENV_REQ), \
>> +PIP, $(PYLIB_VENV_REQ))
>> +$(call quiet-command, touch $@)
>> +
> 
> Maybe we should try to create a generic rule that takes a directory
> name and a requirements file and creates the venv accordingly, instead
> of duplicating the other similar rules under tests/Makefile.include?
> 

Maybe, but I have to admit that my Makefile prowess is lacking and would
be at the mercy of Somebody Else(tm) to do that.

Can I get away with saying "Patches welcome" here?

>> +pylib-venv: $(PYLIB_VENV_DIR)
>> +
>> +check-python: pylib-venv
>> +$(call quiet-command, cd $(SRC_PATH)/python && \
>> +$(PYLIB_VENV_DIR)/bin/python3 -m flake8 qemu, \
>> +FLAKE8, \
>> +$(SRC_PATH)/python/qemu \
> 
> I can see how this venv would be very useful to run the same checks on
> other Python code (for instance, the acceptance tests themselves), so
> we'd also need another "check-python"-like rule, or include those on
> the same call.
> 
> Ideas? :)
> 

Not good ones at the moment... this is still fuzzy in my head.

There's no reason to conflate development packages (mypy, flake8, and
pylint) with runtime packages (avocado-framework, pycdlib).

I consider the requirements.cqa.txt file I added the "development
requirements". I pinned them to specific versions for the purposes of CI
repeatability. (A future patch that may add a setup.py here would re-add
these packages without pinned versions.)

Since the acceptance test venv had a different use case (running the
code) versus this one (analyzing the code) I kept them separate.

That said; maybe it's not a problem to use the same actual venv, but use
different pip setup steps as-needed. We would need to be careful not to
pin conflicting versions between the two different directories!

We'd also then want a test (somewhere) that did nothing but installed
both sets of requirements and made sure it worked.


Lastly, I want to point out that with future plans to package the python
library as an independently installable entity I want to avoid putting
anything in that directory that references python code it "doesn't
manage". avocado-framework, for example, has no business being
referenced in python/ yet.

Ideally this folder would also have its own Makefile that ran the code
quality analysis checks by itself without the Qemu infrastructure
involved (e.g. you can just type 'make check' inside of ./qemu/python),
but then there's code duplication between Makefile and Makefile.include.

That got messy looking and stupid, so I opted for the top-level include
instead for now so that it could be invoked from the build directory,
but I'm not sure I made the right call.

> Thanks!
> - Cleber.
> 

Oh, and the final step that's needed is to add a gitlab CI job to run
check-python as a test step, but that should be easy after Dan's changes.

--js

Re: [PATCH for-5.1] i386: hvf: Explicitly set CR4 guest/host mask

[Paolo Bonzini]

Hi Roman, please ask Peter to apply it directly because I won't be able to
send a pull request in the next couple of weeks.

Paolo

Il mar 14 lug 2020, 12:39 Roman Bolshakov  ha
scritto:

> On Tue, Jul 14, 2020 at 12:07:27PM +0300, Roman Bolshakov wrote:
> > Removal of register reset omitted initialization of CR4 guest/host mask.
> > x86_64 guests aren't booting without it.
> >
> > Fixes: 5009ef22c6bb2 ("i386: hvf: Don't duplicate register reset")
> > Signed-off-by: Roman Bolshakov 
> >
>
> If one has a chance to test or review it, it'd be very helpful. That'd
> allow to include it in RC0.
>
> Thanks,
> Roman
>
>

Re: [RFC 0/3] x86: fix cpu hotplug with secure boot

[Laszlo Ersek]

On 07/10/20 18:17, Igor Mammedov wrote:
> CPU hotplug with Secure Boot was not really supported and firmware wasn't 
> aware
> of hotplugged CPUs (which might lead to guest crashes). During 4.2 we 
> introduced
> locked SMI handler RAM arrea to make sure that guest OS wasn't able to inject
> its own SMI handler and OVMF added initial CPU hotplug support.
>
> This series is QEMU part of that support [1] which lets QMVF tell QEMU that
> CPU hotplug with SMI broadcast enabled is supported so that QEMU would be able
> to prevent hotplug in case it's not supported and trigger SMI on hotplug when
> it's necessary.
>
> 1) CPU hotplug negotiation part was introduced later so it might not be
> in upstream OVMF yet or I might have missed the patch on edk2-devel
> (Laszlo will point out to it/post formal patch)
>
> Igor Mammedov (3):
>   x86: lpc9: let firmware negotiate CPU hotplug SMI feature
>   x86: cphp: prevent guest crash on CPU hotplug when broadcast SMI is in
> use
>   x68: acpi: trigger SMI before scanning for hotplugged CPUs
>
>  include/hw/acpi/cpu.h  |  1 +
>  include/hw/i386/ich9.h |  1 +
>  hw/acpi/cpu.c  |  6 ++
>  hw/acpi/ich9.c | 12 +++-
>  hw/i386/acpi-build.c   | 33 -
>  hw/i386/pc.c   | 15 ++-
>  hw/isa/lpc_ich9.c  | 10 ++
>  7 files changed, 75 insertions(+), 3 deletions(-)
>

I applied the series on top of QEMU commit 20c1df5476e1,
plus the unrelated build fix that I proposed in:

  Re: [PULL 14/31] target/i386: reimplement f2xm1 using floatx80
  operation

at:

  https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg04714.html
  (alternative link:
  http://mid.mail-archive.com/a3302e58-c470-9305-b106-a2b6b2c52d39@redhat.com>)

I used the following command for hotplug:

$ virsh setvcpu ovmf.fedora.q35 3 --enable --live

Results:

  OVMF SMM_REQUIRE  edk2machine type   result
    --  -  

  FALSE 9c6f3545aee0pc-i440fx-5.0  hotplug accepted [1]
  FALSE 9c6f3545aee0pc-i440fx-5.1  hotplug accepted [1]
  FALSE 9c6f3545aee0pc-q35-5.0 hotplug accepted [1]
  FALSE 9c6f3545aee0pc-q35-5.1 hotplug accepted [1]
  FALSE 9c6f3545aee0+patch  pc-i440fx-5.0  hotplug accepted [1]
  FALSE 9c6f3545aee0+patch  pc-i440fx-5.1  hotplug accepted [1]
  FALSE 9c6f3545aee0+patch  pc-q35-5.0 hotplug accepted [1]
  FALSE 9c6f3545aee0+patch  pc-q35-5.1 hotplug accepted [1]
  TRUE  9c6f3545aee0pc-q35-5.0 hotplug rejected [2]
  TRUE  9c6f3545aee0pc-q35-5.1 hotplug rejected [2]
  TRUE  9c6f3545aee0+BUGpc-q35-5.1 negotiation rejected [3]
  TRUE  9c6f3545aee0+patch  pc-q35-5.0 hotplug rejected [2]
  TRUE  9c6f3545aee0+patch  pc-q35-5.1 hotplug accepted [4] [5] 
[6]

[1] In this case, i.e., when OVMF is built without -D SMM_REQUIRE, the
firmware is not supposed to learn about CPU hotplug at OS runtime;
only the OS should care. No SMI is raised in ACPI. So this is a
regression-test for when SMM is not used at all.

[2] "error: internal error: unable to execute QEMU command 'device_add':
 cpu hotplug SMI was not enabled by firmware"

[3] In this test, I intentionally broke the firmware so that it would
negotiate "CPU hotplug with SMI", but not "SMI broadcast". In this
case, QEMU rejects the feature request, the firmware detects that,
and hangs (intentionally, as this is a programming error in the
firmware -- a failed assertion).

[4] Tested hotplug in Linux guest with "rdmsr -a 0x3a", "taskset -c X
efibootmgr", and S3 suspend/resume, as described at

https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt#tests-to-perform-in-the-installed-guest-fedora-26-guest

[5] Also tested hotplug with Windows Server 2012 R2; checking the CPU
count in the Task Manager | Logical Processors view, and in Device
Manager | Processors. Tested S3 suspend/resume too.

[6] The S3 suspend/resume test is relevant in two forms -- first, after
hot-adding CPUs, S3 suspend/resume continues to work.

Second, during S3 resume, the "S3 boot script" re-selects the same
SMI features originally negotiated during normal boot, so plugging a
new CPU works after S3 resume too.

Consider the following S3 boot script difference (executed during S3
resume), taken between pc-q35-5.0 and pc-q35-5.1:

 ExecuteBootScript - 7BB67037
 EFI_BOOT_SCRIPT_MEM_WRITE_OPCODE
 BootScriptExecuteMemoryWrite - 0x7BB66018, 0x0018, 0x
 S3BootScriptWidthUint8 - 0x7BB66018 (0x00)
 S3BootScriptWidthUint8 - 0x7BB66019 (0x2B)
 S3BootScriptWidthUint8 - 0x7BB6601A (0x00)
 

Re: [PATCH v2 1/3] hw/i386: Initialize topo_ids from CpuInstanceProperties

[Igor Mammedov]

On Tue, 14 Jul 2020 12:26:56 -0500
Babu Moger  wrote:

> > -Original Message-
> > From: Igor Mammedov 
> > Sent: Tuesday, July 14, 2020 11:42 AM
> > To: Moger, Babu 
> > Cc: qemu-devel@nongnu.org; pbonz...@redhat.com; ehabk...@redhat.com;
> > r...@twiddle.net
> > Subject: Re: [PATCH v2 1/3] hw/i386: Initialize topo_ids from
> > CpuInstanceProperties
> > 
> > On Mon, 13 Jul 2020 14:30:29 -0500
> > Babu Moger  wrote:
> >   
> > > > -Original Message-
> > > > From: Igor Mammedov 
> > > > Sent: Monday, July 13, 2020 12:32 PM
> > > > To: Moger, Babu 
> > > > Cc: pbonz...@redhat.com; r...@twiddle.net; ehabk...@redhat.com; qemu-
> > > > de...@nongnu.org
> > > > Subject: Re: [PATCH v2 1/3] hw/i386: Initialize topo_ids from
> > > > CpuInstanceProperties
> > > >
> > > > On Mon, 13 Jul 2020 11:43:33 -0500
> > > > Babu Moger  wrote:
> > > >  
> > > > > On 7/13/20 11:17 AM, Igor Mammedov wrote:  
> > > > > > On Mon, 13 Jul 2020 10:02:22 -0500 Babu Moger
> > > > > >  wrote:
> > > > > >  
> > > > > >>> -Original Message-
> > > > > >>> From: Igor Mammedov 
> > > > > >>> Sent: Monday, July 13, 2020 4:08 AM
> > > > > >>> To: Moger, Babu 
> > > > > >>> Cc: pbonz...@redhat.com; r...@twiddle.net; ehabk...@redhat.com;
> > > > > >>> qemu- de...@nongnu.org
> > > > > >>> Subject: Re: [PATCH v2 1/3] hw/i386: Initialize topo_ids from
> > > > > >>> CpuInstanceProperties  
> > > > > > [...]  
> > > > >  +
> > > > >  +/*
> > > > >  + * Initialize topo_ids from CpuInstanceProperties
> > > > >  + * node_id in CpuInstanceProperties(or in CPU device) is a
> > > > >  +sequential
> > > > >  + * number, but while building the topology  
> > > > > >>>  
> > > > >  we need to separate it for
> > > > >  + * each socket(mod nodes_per_pkg).  
> > > > > >>> could you clarify a bit more on why this is necessary?  
> > > > > >>
> > > > > >> If you have two sockets and 4 numa nodes, node_id in
> > > > > >> CpuInstanceProperties will be number sequentially as 0, 1, 2, 3.
> > > > > >> But in EPYC topology, it will be  0, 1, 0, 1( Basically mod %
> > > > > >> number of nodes  
> > > > per socket).  
> > > > > >
> > > > > > I'm confused, let's suppose we have 2 EPYC sockets with 2 nodes
> > > > > > per socket so APIC id woulbe be composed like:
> > > > > >
> > > > > >  1st socket
> > > > > >pkg_id(0) | node_id(0)
> > > > > >pkg_id(0) | node_id(1)
> > > > > >
> > > > > >  2nd socket
> > > > > >pkg_id(1) | node_id(0)
> > > > > >pkg_id(1) | node_id(1)
> > > > > >
> > > > > > if that's the case, then EPYC's node_id here doesn't look like a
> > > > > > NUMA node in the sense it's usually used (above config would
> > > > > > have 4 different memory controllers => 4 conventional NUMA nodes).  
> > > > >
> > > > > EPIC model uses combination of socket id and node id to identify
> > > > > the numa nodes. So, it internally uses all the information.  
> > > >
> > > > well with above values, EPYC's node_id doesn't look like it's
> > > > specifying a machine numa node, but rather a node index within
> > > > single socket. In which case, it doesn't make much sense calling it
> > > > NUMA node_id, it's rather some index within a socket. (it starts
> > > > looking like terminology is all mixed up)
> > > >
> > > > If you have access to a milti-socket EPYC machine, can you dump and
> > > > post here its apic ids, pls?  
> > >
> > > Here is the output from my EPYC machine with 2 sockets and totally 8
> > > nodes(SMT disabled). The cpus 0-31 are in socket 0 and  cpus 32-63 in
> > > socket 1.
> > >
> > > # lscpu
> > > Architecture:x86_64
> > > CPU op-mode(s):  32-bit, 64-bit
> > > Byte Order:  Little Endian
> > > CPU(s):  64
> > > On-line CPU(s) list: 0-63
> > > Thread(s) per core:  1
> > > Core(s) per socket:  32
> > > Socket(s):   2
> > > NUMA node(s):8
> > > Vendor ID:   AuthenticAMD
> > > CPU family:  23
> > > Model:   1
> > > Model name:  AMD Eng Sample: 1S1901A4VIHF5_30/19_N
> > > Stepping:2
> > > CPU MHz: 2379.233
> > > CPU max MHz: 1900.
> > > CPU min MHz: 1200.
> > > BogoMIPS:3792.81
> > > Virtualization:  AMD-V
> > > L1d cache:   32K
> > > L1i cache:   64K
> > > L2 cache:512K
> > > L3 cache:8192K
> > > NUMA node0 CPU(s):   0-7
> > > NUMA node1 CPU(s):   8-15
> > > NUMA node2 CPU(s):   16-23
> > > NUMA node3 CPU(s):   24-31
> > > NUMA node4 CPU(s):   32-39
> > > NUMA node5 CPU(s):   40-47
> > > NUMA node6 CPU(s):   48-55
> > > NUMA node7 CPU(s):   56-63
> > >
> > > Here is the output of #cpuid  -l 0x801e  -r
> > >
> > > You may want to refer
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> > >  
> > amd.com%2Fsystem%2Ffiles%2FTechDocs%2F54945_3.03_ppr_ZP_B2_pub.zip&
> > amp  
> > >  
> > ;data=02%7C01%7Cbabu.moger%40amd.com%7C8027127197c440bc097008d8
> > 2814e52  
> > >  
> > 

Re: [PATCH v5 12/12] python/machine.py: change default wait timeout to 3 seconds

[John Snow]

On 7/14/20 12:20 AM, Cleber Rosa wrote:
> On Fri, Jul 10, 2020 at 01:06:49AM -0400, John Snow wrote:
>> Machine.wait() does not appear to be used except in the acceptance tests,
>> and an infinite timeout by default in a test suite is not the most helpful.
>>
>> Change it to 3 seconds, like the default shutdown timeout.
>>
>> Signed-off-by: John Snow 
>> ---
>>  python/qemu/machine.py | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
> 
> Well, for the acceptance tests, there's usually a test wide timeout,
> but this is indeed a good idea!
> 
> Reviewed-by: Cleber Rosa 
> Tested-by: Cleber Rosa 
> 

Yes, there's a bigger timeout for acceptance tests, but iotests doesn't
have the same just yet.

In general, it helps for most of the python library methods to time out
by default to prevent hangs in the various test suites.

So, anticipating that iotest callers will probably want to use wait()
sooner or later, I just went ahead and made the change primarily for
consistency again.

--js

Re: [PATCH v5 10/12] python/machine.py: split shutdown into hard and soft flavors

[John Snow]

On 7/14/20 12:13 AM, Cleber Rosa wrote:
> On Fri, Jul 10, 2020 at 01:06:47AM -0400, John Snow wrote:
>> This is done primarily to avoid the 'bare except' pattern, which
>> suppresses all exceptions during shutdown and can obscure errors.
>>
>> Replace this with a pattern that isolates the different kind of shutdown
>> paradigms (_hard_shutdown and _soft_shutdown), and a new fallback shutdown
>> handler (_do_shutdown) that gracefully attempts one before the other.
>>
>> This split now also ensures that no matter what happens,
>> _post_shutdown() is always invoked.
>>
>> shutdown() changes in behavior such that if it attempts to do a graceful
>> shutdown and is unable to, it will now always raise an exception to
>> indicate this. This can be avoided by the test writer in three ways:
>>
>> 1. If the VM is expected to have already exited or is in the process of
>> exiting, wait() can be used instead of shutdown() to clean up resources
>> instead. This helps avoid race conditions in shutdown.
>>
>> 2. If a test writer is expecting graceful shutdown to fail, shutdown
>> should be called in a try...except block.
>>
>> 3. If the test writer has no interest in performing a graceful shutdown
>> at all, kill() can be used instead.
>>
>>
>> Handling shutdown in this way makes it much more explicit which type of
>> shutdown we want and allows the library to report problems with this
>> process.
>>
>> Signed-off-by: John Snow 
>> ---
>>  python/qemu/machine.py | 95 +++---
>>  1 file changed, 80 insertions(+), 15 deletions(-)
>>
>> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
>> index aaa173f046..b24ce8a268 100644
>> --- a/python/qemu/machine.py
>> +++ b/python/qemu/machine.py
>> @@ -48,6 +48,12 @@ class QEMUMachineAddDeviceError(QEMUMachineError):
>>  """
>>  
>>  
>> +class AbnormalShutdown(QEMUMachineError):
>> +"""
>> +Exception raised when a graceful shutdown was requested, but not 
>> performed.
>> +"""
>> +
>> +
>>  class MonitorResponseError(qmp.QMPError):
>>  """
>>  Represents erroneous QMP monitor reply
>> @@ -365,6 +371,7 @@ def _early_cleanup(self) -> None:
>>  """
>>  Perform any cleanup that needs to happen before the VM exits.
>>  
>> +May be invoked by both soft and hard shutdown in failover scenarios.
>>  Called additionally by _post_shutdown for comprehensive cleanup.
>>  """
>>  # If we keep the console socket open, we may deadlock waiting
>> @@ -374,32 +381,90 @@ def _early_cleanup(self) -> None:
>>  self._console_socket.close()
>>  self._console_socket = None
>>  
>> +def _hard_shutdown(self) -> None:
>> +"""
>> +Perform early cleanup, kill the VM, and wait for it to terminate.
>> +
>> +:raise subprocess.Timeout: When timeout is exceeds 60 seconds
>> +waiting for the QEMU process to terminate.
>> +"""
>> +self._early_cleanup()
> 
> Like I commented on patch 5, I don't think the *current* type of
> cleanup done is needed on a scenario like this...
> 
>> +self._popen.kill()
> 
> ... as I don't remember QEMU's SIGKILL handler to be susceptible to
> the race condition that motivated the closing of the console file in
> the first place.  But, I also can not prove it's not susceptible at
> this time.
> 

It probably isn't. It was for consistency in when and where that hook is
called, again. It does happen to be "pointless" here.

> Note: I have some old patches that added tests for QEMUMachine itself.
> I intend to respin them on top of your work, so we may have a clearer
> understanding of the QEMU behaviors we need to handle.  So, feel free
> to take the prudent route here, and keep the early cleanup.
> 

Oh, adding formal tests to this folder would be incredible; especially
if we wanted to package it on PyPI. Basically a necessity.

> Reviewed-by: Cleber Rosa 
> Tested-by: Cleber Rosa 
> 

RE: [EXTERNAL] Re: PATCH] WHPX: TSC get and set should be dependent on VM state

[Sunil Muthuswamy]

> >
> > Thanks. Ok, I am setup with GPG. Where should I be sending the pull 
> > requests to? Who is "Peter"? Do I have to send it to you?
> 
> Peter is Peter Maydell, but for now you can send them to me.  I'll get
> round to documenting the remaining steps.
> 
> Unfortunately all the scripts I have for this use the Unix shell, but
> they are just a few lines so I might try to rewrite them in Python.
> This way you can use them from Windows without needing to bring up WSL
> or anything like that.
> 
> Paolo

Paolo, just wanted to make sure that I understood your request. You are asking
me to submit my WHPX changes as signed PRs. But, are you also asking that I
maintain a QEMU fork for all WHPX changes (as WHPX maintainer) and merge
those occasionally? Or, should the other WHPX changes from others be submitted
directly upstream?

Re: [PATCH v5 05/12] python/machine.py: Prohibit multiple shutdown() calls

[John Snow]

On 7/13/20 10:48 PM, Cleber Rosa wrote:
> On Fri, Jul 10, 2020 at 01:06:42AM -0400, John Snow wrote:
>> If the VM is not launched, don't try to shut it down. As a change,
>> _post_shutdown now unconditionally also calls _early_cleanup in order to
>> offer comprehensive object cleanup in failure cases.
>>
>> As a courtesy, treat it as a NOP instead of rejecting it as an
>> error. This is slightly nicer for acceptance tests where vm.shutdown()
>> is issued unconditionally in tearDown callbacks.
>>
>> Signed-off-by: John Snow 
>> ---
>>  python/qemu/machine.py | 14 +-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/python/qemu/machine.py b/python/qemu/machine.py
>> index cac466fbe6..e66a7d66dd 100644
>> --- a/python/qemu/machine.py
>> +++ b/python/qemu/machine.py
>> @@ -283,6 +283,13 @@ def _post_launch(self):
>>  self._qmp.accept()
>>  
>>  def _post_shutdown(self):
>> +"""
>> +Called to cleanup the VM instance after the process has exited.
>> +May also be called after a failed launch.
>> +"""
>> +# Comprehensive reset for the failed launch case:
>> +self._early_cleanup()
>> +
> 
> I'm not following why this is needed here.  AFAICT, the closing of the
> console socket file was introduced when the QEMU process was alive,
> and doing I/O on the serial device attached to the console file (and
> was only necessary because of that).  In those scenarios, a race
> between the time of sending the QMP command to quit, and getting a
> response from QMP could occur.
> 
> But here, IIUC, there's no expectations for the QEMU process to be alive.
> To the best of my understanding and testing, this line did not contribute
> to the robustness of the shutdown behavior.
> 
> Finally, given that currently, only the closing of the console socket
> is done within _early_cleanup(), and that is conditional, this also does
> no harm AFAICT.  If more early cleanups operations were needed, then I
> would feel less bothered about calling it here.
> 

Sure, I can tie it up to be a little less abstract, but I'll go over
some of my reasons below to see if any of them resonate for you.

What I wanted was three things:

(1) A single call that comprehensively cleaned up the VM object back to
a known state, no matter what state it was in.

This is the answer to your first paragraph: I wanted a function to
always thoroughly reset an object back to base state. In the current
code, we won't have a console socket object here yet.

I felt this was good as it illustrates to other contributors what the
comprehensive steps to reset the object are.

(It might be considered bad in that it does perform potentially
unnecessary cleanup at times; but as a matter of taste I prefer this to
conditional inconsistency.)


(2) A guarantee that no matter how a VM object was terminated (either
via context handler, shutdown(), or wait()) that the exact same cleanup
steps would be performed for consistency

This is why wait() now points to shutdown(), and why _post_shutdown is
called outside of either of the shutdown handlers. No matter what,
_post_shutdown is getting called and it is going to clean everything.


(3) Avoiding special-casing certain cleanup actions open-coded in the
shutdown handlers.

It's #3 that wound up with me creating the "_early_cleanup" hook. I
created it because I had the idea to create "Console" and "QMP Socket"
as inheritable mixins to help keep the core class simpler.

In these cases, the mixins need a hook for "early cleanup."

No, I didn't finish that work and it's not ready. We could remove it for
now, but that's why this is here and why it's named like it is.

Long term, I think Machine Mixins are the only viable way to add more
features. The upstream version of the code now has more than 10
arguments and 20+ fields. It's getting unwieldy.


(all that said, we can still do it later. If you prefer, for now, I
can at least rename the function "_close_socket" to make it less abstract.

(I think it should still be called in _post_shutdown, though.))

>>  if self._qmp:
>>  self._qmp.close()
>>  self._qmp = None
>> @@ -328,7 +335,7 @@ def launch(self):
>>  self._launch()
>>  self._launched = True
>>  except:
>> -self.shutdown()
>> +self._post_shutdown()
>>  
>>  LOG.debug('Error launching VM')
>>  if self._qemu_full_args:
>> @@ -357,6 +364,8 @@ def _launch(self):
>>  def _early_cleanup(self) -> None:
>>  """
>>  Perform any cleanup that needs to happen before the VM exits.
>> +
>> +Called additionally by _post_shutdown for comprehensive cleanup.
>>  """
>>  # If we keep the console socket open, we may deadlock waiting
>>  # for QEMU to exit, while QEMU is waiting for the socket to
>> @@ -377,6 +386,9 @@ def shutdown(self, has_quit=False, hard=False):
>>  """
>>  

[PATCH] fuzz: Expect the cmdline in a freeable GString

[Alexander Bulekov]

In the initial FuzzTarget, get_init_cmdline returned a char *. With this
API, we had no guarantee about where the string came from. For example,
i440fx-qtest-reboot-fuzz simply returned a pointer to a string literal,
while the QOS-based targets build the arguments out in a GString an
return the gchar *str pointer. Since we did not try to free the cmdline,
we have a leak for any targets that do not simply return string
literals. Clean up this mess by forcing fuzz-targets to return
a GString, that we can free.

Signed-off-by: Alexander Bulekov 
---
 tests/qtest/fuzz/fuzz.c| 13 ++---
 tests/qtest/fuzz/fuzz.h|  6 +++---
 tests/qtest/fuzz/i440fx_fuzz.c |  4 ++--
 tests/qtest/fuzz/qos_fuzz.c|  6 +++---
 4 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
index 0b66e43409..6bc17ef313 100644
--- a/tests/qtest/fuzz/fuzz.c
+++ b/tests/qtest/fuzz/fuzz.c
@@ -199,16 +199,15 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char 
***envp)
 }
 
 /* Run QEMU's softmmu main with the fuzz-target dependent arguments */
-const char *init_cmdline = fuzz_target->get_init_cmdline(fuzz_target);
-init_cmdline = g_strdup_printf("%s -qtest /dev/null -qtest-log %s",
-   init_cmdline,
-   getenv("QTEST_LOG") ? "/dev/fd/2"
-   : "/dev/null");
-
+GString *cmd_line = fuzz_target->get_init_cmdline(fuzz_target);
+g_string_append_printf(cmd_line,
+   " -qtest /dev/null -qtest-log %s",
+   getenv("QTEST_LOG") ? "/dev/fd/2" : "/dev/null");
 
 /* Split the runcmd into an argv and argc */
 wordexp_t result;
-wordexp(init_cmdline, , 0);
+wordexp(cmd_line->str, , 0);
+g_string_free(cmd_line, true);
 
 qemu_init(result.we_wordc, result.we_wordv, NULL);
 
diff --git a/tests/qtest/fuzz/fuzz.h b/tests/qtest/fuzz/fuzz.h
index 72d5710f6c..9ca3d107c5 100644
--- a/tests/qtest/fuzz/fuzz.h
+++ b/tests/qtest/fuzz/fuzz.h
@@ -50,10 +50,10 @@ typedef struct FuzzTarget {
 
 
 /*
- * returns the arg-list that is passed to qemu/softmmu init()
- * Cannot be NULL
+ * Returns the arguments that are passed to qemu/softmmu init(). Freed by
+ * the caller.
  */
-const char* (*get_init_cmdline)(struct FuzzTarget *);
+GString *(*get_init_cmdline)(struct FuzzTarget *);
 
 /*
  * will run once, prior to running qemu/softmmu init.
diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c
index e2f31e56f9..bf966d478b 100644
--- a/tests/qtest/fuzz/i440fx_fuzz.c
+++ b/tests/qtest/fuzz/i440fx_fuzz.c
@@ -158,9 +158,9 @@ static void i440fx_fuzz_qos_fork(QTestState *s,
 
 static const char *i440fx_qtest_argv = TARGET_NAME " -machine accel=qtest"
" -m 0 -display none";
-static const char *i440fx_argv(FuzzTarget *t)
+static GString *i440fx_argv(FuzzTarget *t)
 {
-return i440fx_qtest_argv;
+return g_string_new(i440fx_qtest_argv);
 }
 
 static void fork_init(void)
diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
index 0c68f5361f..d52f3ebd83 100644
--- a/tests/qtest/fuzz/qos_fuzz.c
+++ b/tests/qtest/fuzz/qos_fuzz.c
@@ -66,7 +66,7 @@ void *qos_allocate_objects(QTestState *qts, QGuestAllocator 
**p_alloc)
 return allocate_objects(qts, current_path + 1, p_alloc);
 }
 
-static const char *qos_build_main_args(void)
+static GString *qos_build_main_args(void)
 {
 char **path = fuzz_path_vec;
 QOSGraphNode *test_node;
@@ -88,7 +88,7 @@ static const char *qos_build_main_args(void)
 /* Prepend the arguments that we need */
 g_string_prepend(cmd_line,
 TARGET_NAME " -display none -machine accel=qtest -m 64 ");
-return cmd_line->str;
+return cmd_line;
 }
 
 /*
@@ -189,7 +189,7 @@ static void walk_path(QOSGraphNode *orig_path, int len)
 g_free(path_str);
 }
 
-static const char *qos_get_cmdline(FuzzTarget *t)
+static GString *qos_get_cmdline(FuzzTarget *t)
 {
 /*
  * Set a global variable that we use to identify the qos_path for our
-- 
2.26.2

Re: [PATCH] tests: qmp-cmd-test: fix memory leak

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200714171531.83723-1-liq...@163.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  TESTcheck-unit: tests/test-crypto-secret
  TESTcheck-unit: tests/test-char
**
ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' should 
not be NULL
ERROR test-char - Bail out! 
ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' should 
not be NULL
make: *** [check-unit] Error 1
make: *** Waiting for unfinished jobs
  TESTiotest-qcow2: 029
qemu-system-aarch64: -accel kvm: invalid accelerator kvm
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
'--label', 'com.qemu.instance.uuid=40109892cb4947b592110f54dc387d3d', '-u', 
'1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 
'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
'/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
'/var/tmp/patchew-tester-tmp-fj3d95lf/src/docker-src.2020-07-14-13.45.46.10433:/var/tmp/qemu:z,ro',
 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit 
status 2.
filter=--filter=label=com.qemu.instance.uuid=40109892cb4947b592110f54dc387d3d
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-fj3d95lf/src'
make: *** [docker-run-test-quick@centos7] Error 2

real15m6.204s
user0m5.812s


The full log is available at
http://patchew.org/logs/20200714171531.83723-1-liq...@163.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH] fuzz: Expect the cmdline in a freeable GString

[Darren Kenny]

On Tuesday, 2020-07-14 at 13:46:16 -04, Alexander Bulekov wrote:
> In the initial FuzzTarget, get_init_cmdline returned a char *. With this
> API, we had no guarantee about where the string came from. For example,
> i440fx-qtest-reboot-fuzz simply returned a pointer to a string literal,
> while the QOS-based targets build the arguments out in a GString an
> return the gchar *str pointer. Since we did not try to free the cmdline,
> we have a leak for any targets that do not simply return string
> literals. Clean up this mess by forcing fuzz-targets to return
> a GString, that we can free.
>
> Signed-off-by: Alexander Bulekov 

Reviewed-by: Darren Kenny 

> ---
>  tests/qtest/fuzz/fuzz.c| 13 ++---
>  tests/qtest/fuzz/fuzz.h|  6 +++---
>  tests/qtest/fuzz/i440fx_fuzz.c |  4 ++--
>  tests/qtest/fuzz/qos_fuzz.c|  6 +++---
>  4 files changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
> index 0b66e43409..6bc17ef313 100644
> --- a/tests/qtest/fuzz/fuzz.c
> +++ b/tests/qtest/fuzz/fuzz.c
> @@ -199,16 +199,15 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char 
> ***envp)
>  }
>  
>  /* Run QEMU's softmmu main with the fuzz-target dependent arguments */
> -const char *init_cmdline = fuzz_target->get_init_cmdline(fuzz_target);
> -init_cmdline = g_strdup_printf("%s -qtest /dev/null -qtest-log %s",
> -   init_cmdline,
> -   getenv("QTEST_LOG") ? "/dev/fd/2"
> -   : "/dev/null");
> -
> +GString *cmd_line = fuzz_target->get_init_cmdline(fuzz_target);
> +g_string_append_printf(cmd_line,
> +   " -qtest /dev/null -qtest-log %s",
> +   getenv("QTEST_LOG") ? "/dev/fd/2" : "/dev/null");
>  
>  /* Split the runcmd into an argv and argc */
>  wordexp_t result;
> -wordexp(init_cmdline, , 0);
> +wordexp(cmd_line->str, , 0);
> +g_string_free(cmd_line, true);
>  
>  qemu_init(result.we_wordc, result.we_wordv, NULL);
>  
> diff --git a/tests/qtest/fuzz/fuzz.h b/tests/qtest/fuzz/fuzz.h
> index 72d5710f6c..9ca3d107c5 100644
> --- a/tests/qtest/fuzz/fuzz.h
> +++ b/tests/qtest/fuzz/fuzz.h
> @@ -50,10 +50,10 @@ typedef struct FuzzTarget {
>  
>  
>  /*
> - * returns the arg-list that is passed to qemu/softmmu init()
> - * Cannot be NULL
> + * Returns the arguments that are passed to qemu/softmmu init(). Freed by
> + * the caller.
>   */
> -const char* (*get_init_cmdline)(struct FuzzTarget *);
> +GString *(*get_init_cmdline)(struct FuzzTarget *);
>  
>  /*
>   * will run once, prior to running qemu/softmmu init.
> diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c
> index e2f31e56f9..bf966d478b 100644
> --- a/tests/qtest/fuzz/i440fx_fuzz.c
> +++ b/tests/qtest/fuzz/i440fx_fuzz.c
> @@ -158,9 +158,9 @@ static void i440fx_fuzz_qos_fork(QTestState *s,
>  
>  static const char *i440fx_qtest_argv = TARGET_NAME " -machine accel=qtest"
> " -m 0 -display none";
> -static const char *i440fx_argv(FuzzTarget *t)
> +static GString *i440fx_argv(FuzzTarget *t)
>  {
> -return i440fx_qtest_argv;
> +return g_string_new(i440fx_qtest_argv);
>  }
>  
>  static void fork_init(void)
> diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
> index 0c68f5361f..d52f3ebd83 100644
> --- a/tests/qtest/fuzz/qos_fuzz.c
> +++ b/tests/qtest/fuzz/qos_fuzz.c
> @@ -66,7 +66,7 @@ void *qos_allocate_objects(QTestState *qts, QGuestAllocator 
> **p_alloc)
>  return allocate_objects(qts, current_path + 1, p_alloc);
>  }
>  
> -static const char *qos_build_main_args(void)
> +static GString *qos_build_main_args(void)
>  {
>  char **path = fuzz_path_vec;
>  QOSGraphNode *test_node;
> @@ -88,7 +88,7 @@ static const char *qos_build_main_args(void)
>  /* Prepend the arguments that we need */
>  g_string_prepend(cmd_line,
>  TARGET_NAME " -display none -machine accel=qtest -m 64 ");
> -return cmd_line->str;
> +return cmd_line;
>  }
>  
>  /*
> @@ -189,7 +189,7 @@ static void walk_path(QOSGraphNode *orig_path, int len)
>  g_free(path_str);
>  }
>  
> -static const char *qos_get_cmdline(FuzzTarget *t)
> +static GString *qos_get_cmdline(FuzzTarget *t)
>  {
>  /*
>   * Set a global variable that we use to identify the qos_path for our
> -- 
> 2.26.2

[PATCH] .travis.yml: skip ppc64abi32-linux-user with plugins

[Alex Bennée]

We actually see failures on threadcount running without plugins:

  retry.py -n 1000 -c -- \
./ppc64abi32-linux-user/qemu-ppc64abi32 \
./tests/tcg/ppc64abi32-linux-user/threadcount

which reports:

  0: 978 times (97.80%), avg time 0.270 (0.01 varience/0.08 deviation)
  -6: 21 times (2.10%), avg time 0.336 (0.01 varience/0.12 deviation)
  -11: 1 times (0.10%), avg time 0.502 (0.00 varience/0.00 deviation)
  Ran command 1000 times, 978 passes

But when running with plugins we hit the failure a lot more often:

  0: 91 times (91.00%), avg time 0.302 (0.04 varience/0.19 deviation)
  -11: 9 times (9.00%), avg time 0.558 (0.01 varience/0.11 deviation)
  Ran command 100 times, 91 passes

The crash occurs in guest code which is the same in both pass and fail
cases. However we see various messages reported on the console about
corrupted memory lists which seems to imply the guest memory allocation
is corrupted. This lines up with the seg fault being in the guest
__libc_free function. So we think this is a guest bug which is
exacerbated by various modes of translation. If anyone has access to
real hardware to soak test the test case we could prove this properly.

Signed-off-by: Alex Bennée 
Cc: David Gibson 
Cc: Philippe Mathieu-Daudé 
---
 .travis.yml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index ab429500fc..6695c0620f 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -350,9 +350,10 @@ jobs:
 # Run check-tcg against linux-user (with plugins)
 # we skip sparc64-linux-user until it has been fixed somewhat
 # we skip cris-linux-user as it doesn't use the common run loop
+# we skip ppc64abi32-linux-user as it seems to have a broken libc
 - name: "GCC plugins check-tcg (user)"
   env:
-- CONFIG="--disable-system --enable-plugins --enable-debug-tcg 
--target-list-exclude=sparc64-linux-user,cris-linux-user"
+- CONFIG="--disable-system --enable-plugins --enable-debug-tcg 
--target-list-exclude=sparc64-linux-user,cris-linux-user,ppc64abi32-linux-user"
 - TEST_BUILD_CMD="make build-tcg"
 - TEST_CMD="make check-tcg"
 - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg"
-- 
2.20.1

Re: [PATCH v3 1/2] tpm: tpm_spapr: Exit on TPM backend failures

[Marc-André Lureau]

Hi

On Wed, Jul 8, 2020 at 12:17 AM Stefan Berger 
wrote:

> Exit on TPM backend failures in the same way as the TPM CRB and TIS device
> models do. With this change we now get an error report when the backend
> did not start up properly:
>
> error: internal error: qemu unexpectedly closed the monitor:
> 2020-07-07T12:49:28.333928Z qemu-system-ppc64: tpm-emulator: \
>   TPM result for CMD_INIT: 0x101 operation failed
>
> Signed-off-by: Stefan Berger 
> ---
>  hw/tpm/tpm_spapr.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/tpm/tpm_spapr.c b/hw/tpm/tpm_spapr.c
> index cb4dfd1e6a..8288ab0a15 100644
> --- a/hw/tpm/tpm_spapr.c
> +++ b/hw/tpm/tpm_spapr.c
> @@ -306,7 +306,10 @@ static void tpm_spapr_reset(SpaprVioDevice *dev)
>  TPM_SPAPR_BUFFER_MAX);
>
>  tpm_backend_reset(s->be_driver);
> -tpm_spapr_do_startup_tpm(s, s->be_buffer_size);
> +
> +if (tpm_spapr_do_startup_tpm(s, s->be_buffer_size) < 0) {
> +exit(1);
> +}
>

Not ideal, but consistent with CRB & TIS.

Reviewed-by: Marc-André Lureau 

 }
>
>  static enum TPMVersion tpm_spapr_get_version(TPMIf *ti)
> --
> 2.24.1
>
>
>

-- 
Marc-André Lureau

Re: [PATCH for 5.1] crypto: use a stronger private key for tests

[Philippe Mathieu-Daudé]

On 7/14/20 7:37 PM, Daniel P. Berrangé wrote:
> The unit tests using the x509 crypto functionality have started
> failing in Fedora 33 rawhide with a message like
> 
>   The certificate uses an insecure algorithm
> 
> This is result of Fedora changes to support strong crypto [1]. RSA
> with 1024 bit key is viewed as legacy and thus insecure. Generate
> a new private key then. Moreover, switch to EC which is not only
> shorter but also not deprecated that often as RSA. Generated
> using the following command [2]:
> 
> openssl genpkey --outform PEM --out privkey.pem \
>   --algorithm EC --pkeyopt ec_paramgen_curve:P-384 \
>   --pkeyopt ec_param_enc:named_curve
> 
> 1: https://fedoraproject.org/wiki/Changes/StrongCryptoSettings2
> 2: technically I just copied the key from libvirt git but that's
>how libvirt generated it
> 
> Signed-off-by: Daniel P. Berrangé 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  tests/crypto-tls-x509-helpers.c | 20 
>  1 file changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/tests/crypto-tls-x509-helpers.c b/tests/crypto-tls-x509-helpers.c
> index 9b669c2a4b..bdb72b9ed0 100644
> --- a/tests/crypto-tls-x509-helpers.c
> +++ b/tests/crypto-tls-x509-helpers.c
> @@ -39,22 +39,10 @@ ASN1_TYPE pkix_asn1;
>  gnutls_x509_privkey_t privkey;
>  # define PRIVATE_KEY  \
>  "-BEGIN PRIVATE KEY-\n"   \
> -"MIICdQIBADANBgkqhkiG9w0BAQEFAASCAl8wggJbAgEAAoGBALVcr\n" \
> -"BL40Tm6yq88FBhJNw1aaoCjmtg0l4dWQZ/e9Fimx4ARxFpT+ji4FE\n" \
> -"Cgl9s/SGqC+1nvlkm9ViSo0j7MKDbnDB+VRHDvMAzQhA2X7e8M0n9\n" \
> -"rPolUY2lIVC83q0BBaOBkCj2RSmT2xTEbbC2xLukSrg2WP/ihVOxc\n" \
> -"kXRuyFtzAgMBAAECgYB7slBexDwXrtItAMIH6m/U+LUpNe0Xx48OL\n" \
> -"IOn4a4whNgO/o84uIwygUK27ZGFZT0kAGAk8CdF9hA6ArcbQ62s1H\n" \
> -"myxrUbF9/mrLsQw1NEqpuUk9Ay2Tx5U/wPx35S3W/X2AvR/ZpTnCn\n" \
> -"2q/7ym9fyiSoj86drD7BTvmKXlOnOwQJBAPOFMp4mMa9NGpGuEssO\n" \
> -"m3Uwbp6lhcP0cA9MK+iOmeANpoKWfBdk5O34VbmeXnGYWEkrnX+9J\n" \
> -"bM4wVhnnBWtgBMCQQC+qAEmvwcfhauERKYznMVUVksyeuhxhCe7EK\n" \
> -"mPh+U2+g0WwdKvGDgO0PPt1gq0ILEjspMDeMHVdTwkaVBo/uMhAkA\n" \
> -"Z5SsZyCP2aTOPFDypXRdI4eqRcjaEPOUBq27r3uYb/jeboVb2weLa\n" \
> -"L1MmVuHiIHoa5clswPdWVI2y0em2IGoDAkBPSp/v9VKJEZabk9Frd\n" \
> -"a+7u4fanrM9QrEjY3KhduslSilXZZSxrWjjAJPyPiqFb3M8XXA26W\n" \
> -"nz1KYGnqYKhLcBAkB7dt57n9xfrhDpuyVEv+Uv1D3VVAhZlsaZ5Pp\n" \
> -"dcrhrkJn2sa/+O8OKvdrPSeeu/N5WwYhJf61+CPoenMp7IFci\n" \
> +"MIG2AgEAMBAGByqGSM49AgEGBSuBBAAiBIGeMIGbAgEBBDD39t6GRLeEmsYjRGR6\n" \
> +"iQiIN2S4zXsgLGS/2GloXdG7K+i/3vEJDt9celZ0DfCLcG6hZANiAAQTJIe13jy7\n" \
> +"k4KTXMkHQHEJa/asH263JaPL5kTbfRa6tMq3DS3pzWlOj+NHY/9JzthrKD+Ece+g\n" \
> +"2g/POHa0gfXRYXGiHTs8mY0AHFqNNmF38eIVGjOqobIi90MkyI3wx4g=\n" \
>  "-END PRIVATE KEY-\n"
>  
>  /*
> 

RE: [PATCH v2 1/3] hw/i386: Initialize topo_ids from CpuInstanceProperties

[Babu Moger]

> -Original Message-
> From: Igor Mammedov 
> Sent: Tuesday, July 14, 2020 11:42 AM
> To: Moger, Babu 
> Cc: qemu-devel@nongnu.org; pbonz...@redhat.com; ehabk...@redhat.com;
> r...@twiddle.net
> Subject: Re: [PATCH v2 1/3] hw/i386: Initialize topo_ids from
> CpuInstanceProperties
> 
> On Mon, 13 Jul 2020 14:30:29 -0500
> Babu Moger  wrote:
> 
> > > -Original Message-
> > > From: Igor Mammedov 
> > > Sent: Monday, July 13, 2020 12:32 PM
> > > To: Moger, Babu 
> > > Cc: pbonz...@redhat.com; r...@twiddle.net; ehabk...@redhat.com; qemu-
> > > de...@nongnu.org
> > > Subject: Re: [PATCH v2 1/3] hw/i386: Initialize topo_ids from
> > > CpuInstanceProperties
> > >
> > > On Mon, 13 Jul 2020 11:43:33 -0500
> > > Babu Moger  wrote:
> > >
> > > > On 7/13/20 11:17 AM, Igor Mammedov wrote:
> > > > > On Mon, 13 Jul 2020 10:02:22 -0500 Babu Moger
> > > > >  wrote:
> > > > >
> > > > >>> -Original Message-
> > > > >>> From: Igor Mammedov 
> > > > >>> Sent: Monday, July 13, 2020 4:08 AM
> > > > >>> To: Moger, Babu 
> > > > >>> Cc: pbonz...@redhat.com; r...@twiddle.net; ehabk...@redhat.com;
> > > > >>> qemu- de...@nongnu.org
> > > > >>> Subject: Re: [PATCH v2 1/3] hw/i386: Initialize topo_ids from
> > > > >>> CpuInstanceProperties
> > > > > [...]
> > > >  +
> > > >  +/*
> > > >  + * Initialize topo_ids from CpuInstanceProperties
> > > >  + * node_id in CpuInstanceProperties(or in CPU device) is a
> > > >  +sequential
> > > >  + * number, but while building the topology
> > > > >>>
> > > >  we need to separate it for
> > > >  + * each socket(mod nodes_per_pkg).
> > > > >>> could you clarify a bit more on why this is necessary?
> > > > >>
> > > > >> If you have two sockets and 4 numa nodes, node_id in
> > > > >> CpuInstanceProperties will be number sequentially as 0, 1, 2, 3.
> > > > >> But in EPYC topology, it will be  0, 1, 0, 1( Basically mod %
> > > > >> number of nodes
> > > per socket).
> > > > >
> > > > > I'm confused, let's suppose we have 2 EPYC sockets with 2 nodes
> > > > > per socket so APIC id woulbe be composed like:
> > > > >
> > > > >  1st socket
> > > > >pkg_id(0) | node_id(0)
> > > > >pkg_id(0) | node_id(1)
> > > > >
> > > > >  2nd socket
> > > > >pkg_id(1) | node_id(0)
> > > > >pkg_id(1) | node_id(1)
> > > > >
> > > > > if that's the case, then EPYC's node_id here doesn't look like a
> > > > > NUMA node in the sense it's usually used (above config would
> > > > > have 4 different memory controllers => 4 conventional NUMA nodes).
> > > >
> > > > EPIC model uses combination of socket id and node id to identify
> > > > the numa nodes. So, it internally uses all the information.
> > >
> > > well with above values, EPYC's node_id doesn't look like it's
> > > specifying a machine numa node, but rather a node index within
> > > single socket. In which case, it doesn't make much sense calling it
> > > NUMA node_id, it's rather some index within a socket. (it starts
> > > looking like terminology is all mixed up)
> > >
> > > If you have access to a milti-socket EPYC machine, can you dump and
> > > post here its apic ids, pls?
> >
> > Here is the output from my EPYC machine with 2 sockets and totally 8
> > nodes(SMT disabled). The cpus 0-31 are in socket 0 and  cpus 32-63 in
> > socket 1.
> >
> > # lscpu
> > Architecture:x86_64
> > CPU op-mode(s):  32-bit, 64-bit
> > Byte Order:  Little Endian
> > CPU(s):  64
> > On-line CPU(s) list: 0-63
> > Thread(s) per core:  1
> > Core(s) per socket:  32
> > Socket(s):   2
> > NUMA node(s):8
> > Vendor ID:   AuthenticAMD
> > CPU family:  23
> > Model:   1
> > Model name:  AMD Eng Sample: 1S1901A4VIHF5_30/19_N
> > Stepping:2
> > CPU MHz: 2379.233
> > CPU max MHz: 1900.
> > CPU min MHz: 1200.
> > BogoMIPS:3792.81
> > Virtualization:  AMD-V
> > L1d cache:   32K
> > L1i cache:   64K
> > L2 cache:512K
> > L3 cache:8192K
> > NUMA node0 CPU(s):   0-7
> > NUMA node1 CPU(s):   8-15
> > NUMA node2 CPU(s):   16-23
> > NUMA node3 CPU(s):   24-31
> > NUMA node4 CPU(s):   32-39
> > NUMA node5 CPU(s):   40-47
> > NUMA node6 CPU(s):   48-55
> > NUMA node7 CPU(s):   56-63
> >
> > Here is the output of #cpuid  -l 0x801e  -r
> >
> > You may want to refer
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> >
> amd.com%2Fsystem%2Ffiles%2FTechDocs%2F54945_3.03_ppr_ZP_B2_pub.zip&
> amp
> >
> ;data=02%7C01%7Cbabu.moger%40amd.com%7C8027127197c440bc097008d8
> 2814e52
> >
> 5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6373034174951581
> 22
> >
> sdata=ViF%2FGTPxDFV6KcjicGedyACbjQ6Ikkq0oWUWw18pGbU%3Dreser
> ved=0
> > (section 2.1.12.2.1.3 ApicId Enumeration Requirements).
> > Note that this is a general guideline. We tried to generalize in qemu
> > as much as possible. It is bit complex.
> Thanks, I'll look into it.
> 

[PATCH for 5.1] crypto: use a stronger private key for tests

[Daniel P . Berrangé]

The unit tests using the x509 crypto functionality have started
failing in Fedora 33 rawhide with a message like

  The certificate uses an insecure algorithm

This is result of Fedora changes to support strong crypto [1]. RSA
with 1024 bit key is viewed as legacy and thus insecure. Generate
a new private key then. Moreover, switch to EC which is not only
shorter but also not deprecated that often as RSA. Generated
using the following command [2]:

openssl genpkey --outform PEM --out privkey.pem \
  --algorithm EC --pkeyopt ec_paramgen_curve:P-384 \
  --pkeyopt ec_param_enc:named_curve

1: https://fedoraproject.org/wiki/Changes/StrongCryptoSettings2
2: technically I just copied the key from libvirt git but that's
   how libvirt generated it

Signed-off-by: Daniel P. Berrangé 
---
 tests/crypto-tls-x509-helpers.c | 20 
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/tests/crypto-tls-x509-helpers.c b/tests/crypto-tls-x509-helpers.c
index 9b669c2a4b..bdb72b9ed0 100644
--- a/tests/crypto-tls-x509-helpers.c
+++ b/tests/crypto-tls-x509-helpers.c
@@ -39,22 +39,10 @@ ASN1_TYPE pkix_asn1;
 gnutls_x509_privkey_t privkey;
 # define PRIVATE_KEY  \
 "-BEGIN PRIVATE KEY-\n"   \
-"MIICdQIBADANBgkqhkiG9w0BAQEFAASCAl8wggJbAgEAAoGBALVcr\n" \
-"BL40Tm6yq88FBhJNw1aaoCjmtg0l4dWQZ/e9Fimx4ARxFpT+ji4FE\n" \
-"Cgl9s/SGqC+1nvlkm9ViSo0j7MKDbnDB+VRHDvMAzQhA2X7e8M0n9\n" \
-"rPolUY2lIVC83q0BBaOBkCj2RSmT2xTEbbC2xLukSrg2WP/ihVOxc\n" \
-"kXRuyFtzAgMBAAECgYB7slBexDwXrtItAMIH6m/U+LUpNe0Xx48OL\n" \
-"IOn4a4whNgO/o84uIwygUK27ZGFZT0kAGAk8CdF9hA6ArcbQ62s1H\n" \
-"myxrUbF9/mrLsQw1NEqpuUk9Ay2Tx5U/wPx35S3W/X2AvR/ZpTnCn\n" \
-"2q/7ym9fyiSoj86drD7BTvmKXlOnOwQJBAPOFMp4mMa9NGpGuEssO\n" \
-"m3Uwbp6lhcP0cA9MK+iOmeANpoKWfBdk5O34VbmeXnGYWEkrnX+9J\n" \
-"bM4wVhnnBWtgBMCQQC+qAEmvwcfhauERKYznMVUVksyeuhxhCe7EK\n" \
-"mPh+U2+g0WwdKvGDgO0PPt1gq0ILEjspMDeMHVdTwkaVBo/uMhAkA\n" \
-"Z5SsZyCP2aTOPFDypXRdI4eqRcjaEPOUBq27r3uYb/jeboVb2weLa\n" \
-"L1MmVuHiIHoa5clswPdWVI2y0em2IGoDAkBPSp/v9VKJEZabk9Frd\n" \
-"a+7u4fanrM9QrEjY3KhduslSilXZZSxrWjjAJPyPiqFb3M8XXA26W\n" \
-"nz1KYGnqYKhLcBAkB7dt57n9xfrhDpuyVEv+Uv1D3VVAhZlsaZ5Pp\n" \
-"dcrhrkJn2sa/+O8OKvdrPSeeu/N5WwYhJf61+CPoenMp7IFci\n" \
+"MIG2AgEAMBAGByqGSM49AgEGBSuBBAAiBIGeMIGbAgEBBDD39t6GRLeEmsYjRGR6\n" \
+"iQiIN2S4zXsgLGS/2GloXdG7K+i/3vEJDt9celZ0DfCLcG6hZANiAAQTJIe13jy7\n" \
+"k4KTXMkHQHEJa/asH263JaPL5kTbfRa6tMq3DS3pzWlOj+NHY/9JzthrKD+Ece+g\n" \
+"2g/POHa0gfXRYXGiHTs8mY0AHFqNNmF38eIVGjOqobIi90MkyI3wx4g=\n" \
 "-END PRIVATE KEY-\n"
 
 /*
-- 
2.24.1

Re: [PATCH 0/2] Add list_fn_callees.py and list_helpers.py scripts

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20200714164156.9353-1-ahmedkhaledkara...@gmail.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20200714164156.9353-1-ahmedkhaledkara...@gmail.com
Subject: [PATCH 0/2] Add list_fn_callees.py and list_helpers.py scripts

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
   1a53dfe..beff47a  master -> master
Switched to a new branch 'test'
f9644e8 scripts/performance: Add list_helpers.py script
aa66eb0 scripts/performance: Add list_fn_callees.py script

=== OUTPUT BEGIN ===
1/2 Checking commit aa66eb05dbfb (scripts/performance: Add list_fn_callees.py 
script)
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 228 lines checked

Patch 1/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/2 Checking commit f9644e83658f (scripts/performance: Add list_helpers.py 
script)
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 207 lines checked

Patch 2/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200714164156.9353-1-ahmedkhaledkara...@gmail.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH v3 for-5.1 0/2] Fix crash due to NBD export leak

[Philippe Mathieu-Daudé]

Cc'ing Marc-André

On 7/14/20 6:49 PM, no-re...@patchew.org wrote:
> Patchew URL: 
> https://patchew.org/QEMU/20200714162234.13113-1-vsement...@virtuozzo.com/
...
> 
>   TESTiotest-qcow2: 022
>   TESTcheck-unit: tests/test-char
> **
> ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' 
> should not be NULL
> ERROR test-char - Bail out! 
> ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' 
> should not be NULL

Seems related to latest chardev-pull-request.

> make: *** [check-unit] Error 1
> make: *** Waiting for unfinished jobs
>   TESTiotest-qcow2: 024
>   TESTiotest-qcow2: 025
> ---
> raise CalledProcessError(retcode, cmd)
> subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
> '--label', 'com.qemu.instance.uuid=9dc2c7a6e3eb458c88fdd6be6a03c6eb', '-u', 
> '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
> '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', 
> '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
> '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
> '/var/tmp/patchew-tester-tmp-fzvixkio/src/docker-src.2020-07-14-12.32.53.19697:/var/tmp/qemu:z,ro',
>  'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit 
> status 2.
> filter=--filter=label=com.qemu.instance.uuid=9dc2c7a6e3eb458c88fdd6be6a03c6eb
> make[1]: *** [docker-run] Error 1
> make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-fzvixkio/src'
> make: *** [docker-run-test-quick@centos7] Error 2
> 
> real16m48.396s
> user0m8.741s
> 
> 
> The full log is available at
> http://patchew.org/logs/20200714162234.13113-1-vsement...@virtuozzo.com/testing.docker-quick@centos7/?type=message.

Re: device compatibility interface for live migration with assigned devices

[Dr. David Alan Gilbert]

* Alex Williamson (alex.william...@redhat.com) wrote:
> On Tue, 14 Jul 2020 11:21:29 +0100
> Daniel P. Berrangé  wrote:
> 
> > On Tue, Jul 14, 2020 at 07:29:57AM +0800, Yan Zhao wrote:
> > > hi folks,
> > > we are defining a device migration compatibility interface that helps 
> > > upper
> > > layer stack like openstack/ovirt/libvirt to check if two devices are
> > > live migration compatible.
> > > The "devices" here could be MDEVs, physical devices, or hybrid of the two.
> > > e.g. we could use it to check whether
> > > - a src MDEV can migrate to a target MDEV,
> > > - a src VF in SRIOV can migrate to a target VF in SRIOV,
> > > - a src MDEV can migration to a target VF in SRIOV.
> > >   (e.g. SIOV/SRIOV backward compatibility case)
> > > 
> > > The upper layer stack could use this interface as the last step to check
> > > if one device is able to migrate to another device before triggering a 
> > > real
> > > live migration procedure.
> > > we are not sure if this interface is of value or help to you. please don't
> > > hesitate to drop your valuable comments.
> > > 
> > > 
> > > (1) interface definition
> > > The interface is defined in below way:
> > > 
> > >  __userspace
> > >   /\  \
> > >  / \write
> > > / read  \
> > >/__   ___\|/_
> > >   | migration_version | | migration_version |-->check migration
> > >   - -   compatibility
> > >  device Adevice B
> > > 
> > > 
> > > a device attribute named migration_version is defined under each device's
> > > sysfs node. e.g. 
> > > (/sys/bus/pci/devices/\:00\:02.0/$mdev_UUID/migration_version).
> > > userspace tools read the migration_version as a string from the source 
> > > device,
> > > and write it to the migration_version sysfs attribute in the target 
> > > device.
> > > 
> > > The userspace should treat ANY of below conditions as two devices not 
> > > compatible:
> > > - any one of the two devices does not have a migration_version attribute
> > > - error when reading from migration_version attribute of one device
> > > - error when writing migration_version string of one device to
> > >   migration_version attribute of the other device
> > > 
> > > The string read from migration_version attribute is defined by device 
> > > vendor
> > > driver and is completely opaque to the userspace.
> > > for a Intel vGPU, string format can be defined like
> > > "parent device PCI ID" + "version of gvt driver" + "mdev type" + 
> > > "aggregator count".
> > > 
> > > for an NVMe VF connecting to a remote storage. it could be
> > > "PCI ID" + "driver version" + "configured remote storage URL"
> > > 
> > > for a QAT VF, it may be
> > > "PCI ID" + "driver version" + "supported encryption set".
> > > 
> > > (to avoid namespace confliction from each vendor, we may prefix a driver 
> > > name to
> > > each migration_version string. e.g. i915-v1-8086-591d-i915-GVTg_V5_8-1)
> 
> It's very strange to define it as opaque and then proceed to describe
> the contents of that opaque string.  The point is that its contents
> are defined by the vendor driver to describe the device, driver version,
> and possibly metadata about the configuration of the device.  One
> instance of a device might generate a different string from another.
> The string that a device produces is not necessarily the only string
> the vendor driver will accept, for example the driver might support
> backwards compatible migrations.

(As I've said in the previous discussion, off one of the patch series)

My view is it makes sense to have a half-way house on the opaqueness of
this string; I'd expect to have an ID and version that are human
readable, maybe a device ID/name that's human interpretable and then a
bunch of other cruft that maybe device/vendor/version specific.

I'm thinking that we want to be able to report problems and include the
string and the user to be able to easily identify the device that was
complaining and notice a difference in versions, and perhaps also use
it in compatibility patterns to find compatible hosts; but that does
get tricky when it's a 'ask the device if it's compatible'.

Dave

> > > (2) backgrounds
> > > 
> > > The reason we hope the migration_version string is opaque to the userspace
> > > is that it is hard to generalize standard comparing fields and comparing
> > > methods for different devices from different vendors.
> > > Though userspace now could still do a simple string compare to check if
> > > two devices are compatible, and result should also be right, it's still
> > > too limited as it excludes the possible candidate whose migration_version
> > > string fails to be equal.
> > > e.g. an MDEV with mdev_type_1, aggregator count 3 is probably compatible
> > > with another MDEV with mdev_type_3, aggregator count 1, even their
> > > migration_version strings 

Re: [PATCH] hmp: fix memory leak in qom_composition_compare()

[Li Qiang]

Philippe Mathieu-Daudé  于2020年7月15日周三 上午12:47写道：
>
> On 7/14/20 5:11 PM, Li Qiang wrote:
> > While 'make chekc', I got following error:
> >
> > root@ubuntu:~/qemu# ./tests/qtest/device-introspect-test
> > /x86_64/device/introspect/list: OK
> > /x86_64/device/introspect/list-fields: OK
> > /x86_64/device/introspect/none:
> > =
> > ==53741==ERROR: LeakSanitizer: detected memory leaks
> >
> > Direct leak of 212 byte(s) in 20 object(s) allocated from:
> > #0 0x7f3b6319cb40 in __interceptor_malloc 
> > (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
> > #1 0x7f3b62805ab8 in g_malloc 
> > (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x51ab8)
> >
> > SUMMARY: AddressSanitizer: 212 byte(s) leaked in 20 allocation(s).
> > tests/qtest/libqtest.c:166: kill_qemu() tried to terminate QEMU process but 
> > encountered exit status 1 (expected 0)
> > Aborted (core dumped)
> >
> > This is because the 'info qom-tree' path has a memory leak and qemu
> > exit 1. The leak is in 'qom_composition_compare'. This patch fixes this.
> >
> > Fixes: e8c9e65816f("qom: Make "info qom-tree" show children sorted")
> > Signed-off-by: Li Qiang 
> > ---
> >  qom/qom-hmp-cmds.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c
> > index 9ed8bb1c9f..3547d5ba4e 100644
> > --- a/qom/qom-hmp-cmds.c
> > +++ b/qom/qom-hmp-cmds.c
> > @@ -96,8 +96,10 @@ static void print_qom_composition(Monitor *mon, Object 
> > *obj, int indent);
> >
> >  static int qom_composition_compare(const void *a, const void *b, void 
> > *ignore)
> >  {
> > -return g_strcmp0(a ? object_get_canonical_path_component(a) : NULL,
> > - b ? object_get_canonical_path_component(b) : NULL);
> > +g_autofree char *t1 = a ? object_get_canonical_path_component(a) : 
> > NULL;
> > +g_autofree char *t2 = b ? object_get_canonical_path_component(b) : 
> > NULL;
> > +
> > +return g_strcmp0(t1, t2);
> >  }
> >
> >  static int insert_qom_composition_child(Object *obj, void *opaque)
> >
>
> Ah you won the race with Markus:
> https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg04740.html
>

Oh interesting, I just want to  begin to write the e1000e tx bh
discussed with Jason.
But found there are some memleak issue.

> Reviewed-by: Philippe Mathieu-Daudé 
>

Re: [PATCH v5 10/11] hw/arm: Wire up BMC boot flash for npcm750-evb and quanta-gsj

[Philippe Mathieu-Daudé]

On 7/14/20 6:21 PM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé  writes:
> 
>> + qemu-block experts.
>>
>> On 7/14/20 11:16 AM, Markus Armbruster wrote:
>>> Havard Skinnemoen  writes:
>>>
 On Mon, Jul 13, 2020 at 7:57 AM Cédric Le Goater  wrote:
>
> On 7/9/20 2:36 AM, Havard Skinnemoen wrote:
>> This allows these NPCM7xx-based boards to boot from a flash image, e.g.
>> one built with OpenBMC. For example like this:
>>
>> IMAGE=${OPENBMC}/build/tmp/deploy/images/gsj/image-bmc
>> qemu-system-arm -machine quanta-gsj -nographic \
>>   -bios ~/qemu/bootrom/npcm7xx_bootrom.bin \
>>   -drive file=${IMAGE},if=mtd,bus=0,unit=0,format=raw,snapshot=on
>>
>> Reviewed-by: Tyrone Ting 
>> Signed-off-by: Havard Skinnemoen 
>
> May be we don't need to create the flash object if dinfo is NULL.

 It's soldered on the board, so you can't really boot the board without
 it. But if you think it's better to remove it altogether if we don't
 have an image to load into it, I can do that.
>>>
>>> If a device is a fixed part of the physical board, it should be a fixed
>>> part of the virtual board, too.
>>
>> We agree so far but ... how to do it?
>>
>> I never used this API, does that makes sense?
>>
>> if (!dinfo) {
>> QemuOpts *opts;
>>
>> opts = qemu_opts_create(NULL, "spi-flash", 1, _abort);
>> qdict_put_str(opts, "format", "null-co");
>> qdict_put_int(opts, BLOCK_OPT_SIZE, 64 * MiB);
>> qdict_put_bool(opts, NULL_OPT_ZEROES, false); // XXX
>>
>> dinfo = drive_new(opts, IF_MTD, _abort);
>> qemu_opts_del(opts);
>> }
> 
> I believe existing code special-cases "no backend" instead of making one
> up.
> 
> Example: pflash_cfi0?.c
> 
> If ->blk is non-null, we read its contents into the memory buffer and
> write updates back, else we leave it blank and don't write updates back.
> 
> Making one up could be more elegant.  To find out, you have to try.

I'd rather avoid ad-hoc code in each device. I2C EEPROM do that too,
it is a source of head aches.

>From the emulation PoV I'd prefer to always use a block backend,
regardless the user provide a drive.

> 
> We make up a few default drives (i.e. drives the user doesn't specify):
> floppy, CD-ROM and SD card.  Ancient part of the user interface, uses
> DriveInfo.  I doubt we should create more of them.
> 
> I believe block backends we make up for internal use should stay away
> from DriveInfo.  Kevin, what do you think?  How would you make up a
> null-co block backend for a device's internal use?

I read 'DriveInfo' is the legacy interface, but all the code base use it
so it is confusing, I don't understand what is the correct interface to
use.

> 
>> We should probably add a public helper for that.
> 
> If we decide we want to make up backends, then yes, we should do that in
> a helper, not in each device.
> 
>> 'XXX' because NOR flashes erase content is when hardware bit
>> is set, so it would be more useful to return -1/0xff... rather
>> than zeroes.
> 
> 

[PATCH] tests: qmp-cmd-test: fix memory leak

[Li Qiang]

Fixes: 5b88849e7b9("tests/qmp-cmd-test: Add
qmp/object-add-failure-modes"

Signed-off-by: Li Qiang 
---
 tests/qtest/qmp-cmd-test.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
index c68f99f659..f7b1aa7fdc 100644
--- a/tests/qtest/qmp-cmd-test.c
+++ b/tests/qtest/qmp-cmd-test.c
@@ -230,6 +230,8 @@ static void test_object_add_failure_modes(void)
  " 'props': {'size': 1048576 } } }");
 g_assert_nonnull(resp);
 g_assert(qdict_haskey(resp, "return"));
+qobject_unref(resp);
+
 resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
  " {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
  " 'props': {'size': 1048576 } } }");
@@ -241,6 +243,7 @@ static void test_object_add_failure_modes(void)
  " {'id': 'ram1' } }");
 g_assert_nonnull(resp);
 g_assert(qdict_haskey(resp, "return"));
+qobject_unref(resp);
 
 /* attempt to create an object with a property of a wrong type */
 resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
@@ -249,17 +252,20 @@ static void test_object_add_failure_modes(void)
 g_assert_nonnull(resp);
 /* now do it right */
 qmp_assert_error_class(resp, "GenericError");
+
 resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
  " {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
  " 'props': {'size': 1048576 } } }");
 g_assert_nonnull(resp);
 g_assert(qdict_haskey(resp, "return"));
+qobject_unref(resp);
 
 /* delete ram1 object */
 resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
  " {'id': 'ram1' } }");
 g_assert_nonnull(resp);
 g_assert(qdict_haskey(resp, "return"));
+qobject_unref(resp);
 
 /* attempt to create an object without the id */
 resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
@@ -267,18 +273,21 @@ static void test_object_add_failure_modes(void)
  " 'props': {'size': 1048576 } } }");
 g_assert_nonnull(resp);
 qmp_assert_error_class(resp, "GenericError");
+
 /* now do it right */
 resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
  " {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
  " 'props': {'size': 1048576 } } }");
 g_assert_nonnull(resp);
 g_assert(qdict_haskey(resp, "return"));
+qobject_unref(resp);
 
 /* delete ram1 object */
 resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
  " {'id': 'ram1' } }");
 g_assert_nonnull(resp);
 g_assert(qdict_haskey(resp, "return"));
+qobject_unref(resp);
 
 /* attempt to set a non existing property */
 resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
@@ -286,23 +295,27 @@ static void test_object_add_failure_modes(void)
  " 'props': {'sized': 1048576 } } }");
 g_assert_nonnull(resp);
 qmp_assert_error_class(resp, "GenericError");
+
 /* now do it right */
 resp = qtest_qmp(qts, "{'execute': 'object-add', 'arguments':"
  " {'qom-type': 'memory-backend-ram', 'id': 'ram1',"
  " 'props': {'size': 1048576 } } }");
 g_assert_nonnull(resp);
 g_assert(qdict_haskey(resp, "return"));
+qobject_unref(resp);
 
 /* delete ram1 object without id */
 resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
  " {'ida': 'ram1' } }");
 g_assert_nonnull(resp);
+qobject_unref(resp);
 
 /* delete ram1 object */
 resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
  " {'id': 'ram1' } }");
 g_assert_nonnull(resp);
 g_assert(qdict_haskey(resp, "return"));
+qobject_unref(resp);
 
 /* delete ram1 object that does not exist anymore*/
 resp = qtest_qmp(qts, "{'execute': 'object-del', 'arguments':"
-- 
2.17.1

Re: [PATCH v5 04/11] hw/arm: Add NPCM730 and NPCM750 SoC models

[Philippe Mathieu-Daudé]

On 7/14/20 6:01 PM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé  writes:
> 
>> +Markus
>>
>> On 7/14/20 2:44 AM, Havard Skinnemoen wrote:
>>> On Mon, Jul 13, 2020 at 8:02 AM Cédric Le Goater  wrote:

 On 7/9/20 2:36 AM, Havard Skinnemoen wrote:
> The Nuvoton NPCM7xx SoC family are used to implement Baseboard
> Management Controllers in servers. While the family includes four SoCs,
> this patch implements limited support for two of them: NPCM730 (targeted
> for Data Center applications) and NPCM750 (targeted for Enterprise
> applications).
>
> This patch includes little more than the bare minimum needed to boot a
> Linux kernel built with NPCM7xx support in direct-kernel mode:
>
>   - Two Cortex-A9 CPU cores with built-in periperhals.
>   - Global Configuration Registers.
>   - Clock Management.
>   - 3 Timer Modules with 5 timers each.
>   - 4 serial ports.
>
> The chips themselves have a lot more features, some of which will be
> added to the model at a later stage.
>
> Reviewed-by: Tyrone Ting 
> Reviewed-by: Joel Stanley 
> Signed-off-by: Havard Skinnemoen 
> ---
>> ...
>>
> +static void npcm7xx_realize(DeviceState *dev, Error **errp)
> +{
> +NPCM7xxState *s = NPCM7XX(dev);
> +NPCM7xxClass *nc = NPCM7XX_GET_CLASS(s);
> +int i;
> +
> +/* CPUs */
> +for (i = 0; i < nc->num_cpus; i++) {
> +object_property_set_int(OBJECT(>cpu[i]),
> +arm_cpu_mp_affinity(i, 
> NPCM7XX_MAX_NUM_CPUS),
> +"mp-affinity", _abort);
> +object_property_set_int(OBJECT(>cpu[i]), 
> NPCM7XX_GIC_CPU_IF_ADDR,
> +"reset-cbar", _abort);
> +object_property_set_bool(OBJECT(>cpu[i]), true,
> + "reset-hivecs", _abort);
> +
> +/* Disable security extensions. */
> +object_property_set_bool(OBJECT(>cpu[i]), false, "has_el3",
> + _abort);
> +
> +qdev_realize(DEVICE(>cpu[i]), NULL, _abort);

 I would check the error:

 if (!qdev_realize(DEVICE(>cpu[i]), NULL, errp)) {
 return;
 }

 same for the sysbus_realize() below.
>>>
>>> Hmm, I used to propagate these errors until Philippe told me not to
>>> (or at least that's how I understood it).
>>
>> It was before Markus simplification API were merged, you had to
>> propagate after each call, since this is a non hot-pluggable SoC
>> I suggested to use _abort to simplify.
>>
>>> I'll be happy to do it
>>> either way (and the new API makes it really easy to propagate errors),
>>> but I worry that I don't fully understand when to propagate errors and
>>> when not to.
>>
>> Markus explained it on the mailing list recently (as I found the doc
>> not obvious). I can't find the thread. I suppose once the work result
>> after the "Questionable aspects of QEMU Error's design" discussion is
>> merged, the documentation will be clarified.
> 
> The Error API evolved recently.  Please peruse the big comment in
> include/qapi/error.h.  If still unsure, don't hesitate to ask here.
> 
>> My rule of thumb so far is:
>> - programming error (can't happen) -> _abort
> 
> Correct.  Quote the big comment:
> 
>  * Call a function aborting on errors:
>  * foo(arg, _abort);
>  * This is more concise and fails more nicely than
>  * Error *err = NULL;
>  * foo(arg, );
>  * assert(!err); // don't do this
> 
>> - everything triggerable by user or management layer (via QMP command)
>>   -> _fatal, as we can't risk loose the user data, we need to
>>   shutdown gracefully.
> 
> Quote the big comment:
> 
>  * Call a function treating errors as fatal:
>  * foo(arg, _fatal);
>  * This is more concise than
>  * Error *err = NULL;
>  * foo(arg, );
>  * if (err) { // don't do this
>  * error_report_err(err);
>  * exit(1);
>  * }
> 
> Terminating the process is generally fine during initial startup,
> i.e. before the guest runs.
> 
> It's generally not fine once the guest runs.  Errors need to be handled
> more gracefully then.  A QMP command, for instance, should fail cleanly,
> propagating the error to the monitor core, which then sends it to the
> QMP client, and loops to process the next command.
> 
>>> It makes sense to me to propagate errors from *_realize() and
>>> error_abort on failure to set simple properties, but I'd like to know
>>> if Philippe is on board with that.
> 
> Realize methods must not use _fatal.  Instead, they should clean
> up and fail.
> 
> "Clean up" is the part we often neglect.  The big advantage of
> _fatal is that you don't have to bother :)
> 
> Questions?

One on my side. So in this realize(), all _abort uses has
to be replaced by local_err + propagate ...:

static void 

Re: [PATCH for 5.1] docs: fix trace docs build with sphinx 3.1.1

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20200714162659.1017432-1-berra...@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  TESTiotest-qcow2: 024
  TESTcheck-unit: tests/test-char
**
ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' should 
not be NULL
ERROR test-char - Bail out! 
ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' should 
not be NULL
make: *** [check-unit] Error 1
make: *** Waiting for unfinished jobs
qemu-system-aarch64: -accel kvm: invalid accelerator kvm
qemu-system-aarch64: falling back to tcg
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
'--label', 'com.qemu.instance.uuid=ee90d27b35364342aac88818477b8e5c', '-u', 
'1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 
'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
'/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
'/var/tmp/patchew-tester-tmp-9f8ignz7/src/docker-src.2020-07-14-12.48.51.16596:/var/tmp/qemu:z,ro',
 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit 
status 2.
filter=--filter=label=com.qemu.instance.uuid=ee90d27b35364342aac88818477b8e5c
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-9f8ignz7/src'
make: *** [docker-run-test-quick@centos7] Error 2

real15m38.865s
user0m8.774s


The full log is available at
http://patchew.org/logs/20200714162659.1017432-1-berra...@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: sysbus_create_simple Vs qdev_create

[Philippe Mathieu-Daudé]

Hi Pratik,

On 7/14/20 6:17 PM, Pratik Parvati wrote:
> Here is a brief context that might help you.
> I am referring hw/arm/versatilepb.c
> 
> The ARM PrimeCell UART (PL011) device created as follows
> 
> |dev = qdev_create(NULL, "pl011"); s = SYS_BUS_DEVICE(dev);
> qdev_prop_set_chr(dev, "chardev", chr); qdev_init_nofail(dev);
> sysbus_mmio_map(s, 0, addr); sysbus_connect_irq(s, 0, irq); |
> 
> Whereas the PL031 RTC device is created as
> 
> |/* Add PL031 Real Time Clock. */ sysbus_create_simple("pl031",
> 0x101e8000, pic[10]); |
> 
> What is the difference between these two devices creation?

Both devices inherit SysBusDevice, which itself inherits QDev.

You can create QDev objects with the qdev API, and
SysBusDevice objects with the sysbus API.

sysbus_create_simple() is a condensed helper, but only allow you
to pass qemu_irq objects, not a 'chardev' property. So for this
case you have to use the qdev API instead.

> How do I know
> which method to use while creating an object?

SysBusDevice are plugged onto a bus. QDev aren't.
The sysbus API results in smaller code, easier to review.

> 
> Thanks & Regards,
> 
> Pratik
> 
> 
> On Tue, Jul 14, 2020 at 9:39 PM Pratik Parvati  > wrote:
> 
> Hi Support team,
> 
> Can someone please guide me to understand the difference between
> *sysbus_create_simple *and *qdev_create*?.
> 
> I understand that these two methods are used to create a new device.
> But, when to use these functions is not clear to me.
> 
> Regards,
> Pratik
> 

Re: [PATCH v3 for-5.1 0/2] Fix crash due to NBD export leak

[Vladimir Sementsov-Ogievskiy]

14.07.2020 19:49, no-re...@patchew.org wrote:

Patchew URL: 
https://patchew.org/QEMU/20200714162234.13113-1-vsement...@virtuozzo.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

   TESTiotest-qcow2: 022
   TESTcheck-unit: tests/test-char
**
ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' should 
not be NULL
ERROR test-char - Bail out! 
ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' should 
not be NULL
make: *** [check-unit] Error 1
make: *** Waiting for unfinished jobs
   TESTiotest-qcow2: 024
   TESTiotest-qcow2: 025
---
 raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
'--label', 'com.qemu.instance.uuid=9dc2c7a6e3eb458c88fdd6be6a03c6eb', '-u', 
'1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 
'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
'/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
'/var/tmp/patchew-tester-tmp-fzvixkio/src/docker-src.2020-07-14-12.32.53.19697:/var/tmp/qemu:z,ro',
 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit 
status 2.
filter=--filter=label=com.qemu.instance.uuid=9dc2c7a6e3eb458c88fdd6be6a03c6eb
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-fzvixkio/src'
make: *** [docker-run-test-quick@centos7] Error 2

real16m48.396s
user0m8.741s


The full log is available at
http://patchew.org/logs/20200714162234.13113-1-vsement...@virtuozzo.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com



something unrelated..

--
Best regards,
Vladimir

Re: [PULL 0/3] Migration patches

[Peter Maydell]

On Mon, 13 Jul 2020 at 18:55, Juan Quintela  wrote:
>
> The following changes since commit 9f526fce49c6ac48114ed04914b5a76e4db75785:
>
>   Merge remote-tracking branch 
> 'remotes/stsquad/tags/pull-testing-and-misc-110720-2' into staging 
> (2020-07-12 15:32:05 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/juanquintela/qemu.git tags/migration-pull-request
>
> for you to fetch changes up to eb9bd46ff658e05e2c0c71fc308f3b811afa87e1:
>
>   migration/migration.c: Remove superfluous breaks (2020-07-13 18:15:36 +0200)
>
> 
> Migration Pull request
>
> It includes several fixes:
>
> - fix qemu_fclose(denis)
> - remove superfluous breaks (liao)
> - fix memory leak (zheng)
>
> Please apply
>
> [v1 & v2]
>
> There was one error on the huawei address of the 1st patch and mail
> was bouncing.  Fixed.


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.

-- PMM

Re: [PATCH for 5.1] docs: fix trace docs build with sphinx 3.1.1

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20200714162659.1017432-1-berra...@redhat.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC  authz/trace.o
  CC  block/trace.o

Warning, treated as error:
../src/docs/qemu-option-trace.rst.inc:21:Duplicate explicit target name: 
"cmdoption-qemu-img-trace".
  CC  io/trace.o
  CC  nbd/trace.o
  CC  scsi/trace.o
  CC  audio/trace.o

Warning, treated as error:
../src/docs/qemu-option-trace.rst.inc:21:Duplicate explicit target name: 
"cmdoption-qemu-img-trace".
  CC  chardev/trace.o
  CC  hw/9pfs/trace.o
---
  CC  hw/hyperv/trace.o
  CC  hw/i2c/trace.o
  CC  hw/i386/trace.o
make: *** [Makefile:1102: docs/tools/index.html] Error 2
make: *** Waiting for unfinished jobs
make: *** [Makefile:1112: 
.docs_tools_qemu-img.1_docs_tools_qemu-nbd.8_docs_tools_qemu-trace-stap.1_docs_tools_virtiofsd.1_docs_tools_virtfs-proxy-helper.1.sentinel.]
 Error 2
make: *** Deleting file 
'.docs_tools_qemu-img.1_docs_tools_qemu-nbd.8_docs_tools_qemu-trace-stap.1_docs_tools_virtiofsd.1_docs_tools_virtfs-proxy-helper.1.sentinel.'

Warning, treated as error:
../src/docs/qemu-option-trace.rst.inc:21:Duplicate explicit target name: 
"cmdoption-trace".
make: *** [Makefile:1110: 
.docs_system_qemu.1_docs_system_qemu-block-drivers.7_docs_system_qemu-cpu-models.7.sentinel.]
 Error 2
make: *** Deleting file 
'.docs_system_qemu.1_docs_system_qemu-block-drivers.7_docs_system_qemu-cpu-models.7.sentinel.'

Warning, treated as error:
../src/docs/qemu-option-trace.rst.inc:21:Duplicate explicit target name: 
"cmdoption-trace".
make: *** [Makefile:1099: docs/system/index.html] Error 2
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 702, in 
sys.exit(main())
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
'--label', 'com.qemu.instance.uuid=74976b19cfaa4c07a430b88e4a00cdec', '-u', 
'1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 
'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
'/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
'/var/tmp/patchew-tester-tmp-um58z9k1/src/docker-src.2020-07-14-12.53.28.7929:/var/tmp/qemu:z,ro',
 'qemu/fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit 
status 2.
filter=--filter=label=com.qemu.instance.uuid=74976b19cfaa4c07a430b88e4a00cdec
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-um58z9k1/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real3m0.912s
user0m8.019s


The full log is available at
http://patchew.org/logs/20200714162659.1017432-1-berra...@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH v3 for-5.1 0/2] Fix crash due to NBD export leak

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20200714162234.13113-1-vsement...@virtuozzo.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  TESTiotest-qcow2: 022
  TESTcheck-unit: tests/test-char
**
ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' should 
not be NULL
ERROR test-char - Bail out! 
ERROR:/tmp/qemu-test/src/tests/test-char.c:1204:char_serial_test: 'chr' should 
not be NULL
make: *** [check-unit] Error 1
make: *** Waiting for unfinished jobs
  TESTiotest-qcow2: 024
  TESTiotest-qcow2: 025
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
'--label', 'com.qemu.instance.uuid=9dc2c7a6e3eb458c88fdd6be6a03c6eb', '-u', 
'1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 
'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
'/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
'/var/tmp/patchew-tester-tmp-fzvixkio/src/docker-src.2020-07-14-12.32.53.19697:/var/tmp/qemu:z,ro',
 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit 
status 2.
filter=--filter=label=com.qemu.instance.uuid=9dc2c7a6e3eb458c88fdd6be6a03c6eb
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-fzvixkio/src'
make: *** [docker-run-test-quick@centos7] Error 2

real16m48.396s
user0m8.741s


The full log is available at
http://patchew.org/logs/20200714162234.13113-1-vsement...@virtuozzo.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH] hmp: fix memory leak in qom_composition_compare()

[Philippe Mathieu-Daudé]

On 7/14/20 5:11 PM, Li Qiang wrote:
> While 'make chekc', I got following error:
> 
> root@ubuntu:~/qemu# ./tests/qtest/device-introspect-test
> /x86_64/device/introspect/list: OK
> /x86_64/device/introspect/list-fields: OK
> /x86_64/device/introspect/none:
> =
> ==53741==ERROR: LeakSanitizer: detected memory leaks
> 
> Direct leak of 212 byte(s) in 20 object(s) allocated from:
> #0 0x7f3b6319cb40 in __interceptor_malloc 
> (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb40)
> #1 0x7f3b62805ab8 in g_malloc 
> (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x51ab8)
> 
> SUMMARY: AddressSanitizer: 212 byte(s) leaked in 20 allocation(s).
> tests/qtest/libqtest.c:166: kill_qemu() tried to terminate QEMU process but 
> encountered exit status 1 (expected 0)
> Aborted (core dumped)
> 
> This is because the 'info qom-tree' path has a memory leak and qemu
> exit 1. The leak is in 'qom_composition_compare'. This patch fixes this.
> 
> Fixes: e8c9e65816f("qom: Make "info qom-tree" show children sorted")
> Signed-off-by: Li Qiang 
> ---
>  qom/qom-hmp-cmds.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c
> index 9ed8bb1c9f..3547d5ba4e 100644
> --- a/qom/qom-hmp-cmds.c
> +++ b/qom/qom-hmp-cmds.c
> @@ -96,8 +96,10 @@ static void print_qom_composition(Monitor *mon, Object 
> *obj, int indent);
>  
>  static int qom_composition_compare(const void *a, const void *b, void 
> *ignore)
>  {
> -return g_strcmp0(a ? object_get_canonical_path_component(a) : NULL,
> - b ? object_get_canonical_path_component(b) : NULL);
> +g_autofree char *t1 = a ? object_get_canonical_path_component(a) : NULL;
> +g_autofree char *t2 = b ? object_get_canonical_path_component(b) : NULL;
> +
> +return g_strcmp0(t1, t2);
>  }
>  
>  static int insert_qom_composition_child(Object *obj, void *opaque)
> 

Ah you won the race with Markus:
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg04740.html

Reviewed-by: Philippe Mathieu-Daudé 

Re: device compatibility interface for live migration with assigned devices

[Daniel P . Berrangé]

On Tue, Jul 14, 2020 at 10:16:16AM -0600, Alex Williamson wrote:
> On Tue, 14 Jul 2020 11:21:29 +0100
> Daniel P. Berrangé  wrote:
> 
> > On Tue, Jul 14, 2020 at 07:29:57AM +0800, Yan Zhao wrote:
> > > 
> > > The string read from migration_version attribute is defined by device 
> > > vendor
> > > driver and is completely opaque to the userspace.
> > > for a Intel vGPU, string format can be defined like
> > > "parent device PCI ID" + "version of gvt driver" + "mdev type" + 
> > > "aggregator count".
> > > 
> > > for an NVMe VF connecting to a remote storage. it could be
> > > "PCI ID" + "driver version" + "configured remote storage URL"
> > > 
> > > for a QAT VF, it may be
> > > "PCI ID" + "driver version" + "supported encryption set".
> > > 
> > > (to avoid namespace confliction from each vendor, we may prefix a driver 
> > > name to
> > > each migration_version string. e.g. i915-v1-8086-591d-i915-GVTg_V5_8-1)
> 
> It's very strange to define it as opaque and then proceed to describe
> the contents of that opaque string.  The point is that its contents
> are defined by the vendor driver to describe the device, driver version,
> and possibly metadata about the configuration of the device.  One
> instance of a device might generate a different string from another.
> The string that a device produces is not necessarily the only string
> the vendor driver will accept, for example the driver might support
> backwards compatible migrations.


> > IMHO there needs to be a mechanism for the kernel to report via sysfs
> > what versions are supported on a given device. This puts the job of
> > reporting compatible versions directly under the responsibility of the
> > vendor who writes the kernel driver for it. They are the ones with the
> > best knowledge of the hardware they've built and the rules around its
> > compatibility.
> 
> The version string discussed previously is the version string that
> represents a given device, possibly including driver information,
> configuration, etc.  I think what you're asking for here is an
> enumeration of every possible version string that a given device could
> accept as an incoming migration stream.  If we consider the string as
> opaque, that means the vendor driver needs to generate a separate
> string for every possible version it could accept, for every possible
> configuration option.  That potentially becomes an excessive amount of
> data to either generate or manage.
> 
> Am I overestimating how vendors intend to use the version string?

If I'm interpreting your reply & the quoted text orrectly, the version
string isn't really a version string in any normal sense of the word
"version".

Instead it sounds like string encoding a set of features in some arbitrary
vendor specific format, which they parse and do compatibility checks on
individual pieces ? One or more parts may contain a version number, but
its much more than just a version.

If that's correct, then I'd prefer we didn't call it a version string,
instead call it a "capability string" to make it clear it is expressing
a much more general concept, but...

> We'd also need to consider devices that we could create, for instance
> providing the same interface enumeration prior to creating an mdev
> device to have a confidence level that the new device would be a valid
> target.
> 
> We defined the string as opaque to allow vendor flexibility and because
> defining a common format is hard.  Do we need to revisit this part of
> the discussion to define the version string as non-opaque with parsing
> rules, probably with separate incoming vs outgoing interfaces?  Thanks,

..even if the huge amount of flexibility is technically relevant from the
POV of the hardware/drivers, we should consider whether management apps
actually want, or can use, that level of flexibility.

The task of picking which host to place a VM on has alot of factors to
consider, and when there are a large number of hosts, the total amount
of information to check gets correspondingly large.  The placement
process is also fairly performance critical.

Running complex algorithmic logic to check compatibility of devices
based on a arbitrary set of rules is likely to be a performance
challenge. A flat list of supported strings is a much simpler
thing to check as it reduces down to a simple set membership test.

IOW, even if there's some complex set of device type / vendor specific
rules to check for compatibility, I fear apps will ignore them and
just define a very simplified list of compatible string, and ignore
all the extra flexibility.

I'm sure OpenStack maintainers can speak to this more, as they've put
alot of work into their scheduling engine to optimize the way it places
VMs largely driven from simple structured data reported from hosts.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: 

[PATCH-for-5.1 3/4] qemu-common: Document qemu_find_file()

[Philippe Mathieu-Daudé]

Document qemu_find_file(), in particular the returned
value which must be freed.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/qemu-common.h | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index d0142f29ac..d6a08259d3 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -110,6 +110,20 @@ const char *qemu_get_vm_name(void);
 
 #define QEMU_FILE_TYPE_BIOS   0
 #define QEMU_FILE_TYPE_KEYMAP 1
+/**
+ * qemu_find_file:
+ * @type: QEMU_FILE_TYPE_BIOS (for BIOS, VGA BIOS)
+ *or QEMU_FILE_TYPE_KEYMAP (for keymaps).
+ * @name: File name
+ *
+ * Search for @name file in the data directories, either configured at
+ * build time (DATADIR) or registered with the -L command line option.
+ *
+ * The caller must use g_free() to free the returned data when it is
+ * no longer required.
+ *
+ * Returns: absolute path to the file or NULL on error.
+ */
 char *qemu_find_file(int type, const char *name);
 
 /* OS specific functions */
-- 
2.21.3

Re: [PATCH for-5.1 2/5] qom: Plug memory leak in "info qom-tree"

[Philippe Mathieu-Daudé]

On 7/14/20 6:01 PM, Markus Armbruster wrote:
> Commit e8c9e65816 "qom: Make "info qom-tree" show children sorted"
> created a memory leak, because I didn't realize
> object_get_canonical_path_component()'s value needs to be freed.
> 
> Reproducer:
> 
> $ qemu-system-x86_64 -nodefaults -display none -S -monitor stdio
> QEMU 5.0.50 monitor - type 'help' for more information
> (qemu) info qom-tree
> 
> This leaks some 4500 path components, 12-13 characters on average,
> i.e. roughly 100kBytes depending on the allocator.  A couple of
> hundred "info qom-tree" here, a couple of hundred there, and soon
> enough we're talking about real memory.
> 
> Plug the leak.
> 
> Fixes: e8c9e65816f5dbfe18ad3b2be938d0d8192d459a
> Signed-off-by: Markus Armbruster 
> ---
>  qom/qom-hmp-cmds.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c
> index 9ed8bb1c9f..aaacadacca 100644
> --- a/qom/qom-hmp-cmds.c
> +++ b/qom/qom-hmp-cmds.c
> @@ -96,8 +96,10 @@ static void print_qom_composition(Monitor *mon, Object 
> *obj, int indent);
>  
>  static int qom_composition_compare(const void *a, const void *b, void 
> *ignore)
>  {
> -return g_strcmp0(a ? object_get_canonical_path_component(a) : NULL,
> - b ? object_get_canonical_path_component(b) : NULL);
> +g_autofree char *ac = object_get_canonical_path_component(a);
> +g_autofree char *bc = object_get_canonical_path_component(b);
> +
> +return g_strcmp0(ac, bc);
>  }
>  
>  static int insert_qom_composition_child(Object *obj, void *opaque)
> 

Li Qiang set this too:
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg04732.html

[PATCH 2/2] scripts/performance: Add list_helpers.py script

[Ahmed Karaman]

Python script that prints executed helpers of a QEMU invocation.

Syntax:
list_helpers.py [-h] -- \
[] \
[]

[-h] - Print the script arguments help message.

Example of usage:
list_helpers.py -- qemu-mips coulomb_double-mips -n10

Example output:
Total number of instructions: 108,933,695

Executed QEMU Helpers:

No. Ins Percent  Calls Ins/Call Helper Name Source File
--- --- --- --  ---
  1 183,021  0.168%  1,305  140 helper_float_sub_d  
/target/mips/fpu_helper.c
  2 177,111  0.163%770  230 helper_float_madd_d 
/target/mips/fpu_helper.c
  3 171,537  0.157%  1,014  169 helper_float_mul_d  
/target/mips/fpu_helper.c
  4 157,298  0.144%  2,443   64 helper_lookup_tb_ptr
/accel/tcg/tcg-runtime.c
  5 138,123  0.127%897  153 helper_float_add_d  
/target/mips/fpu_helper.c
  6  47,083  0.043%207  227 helper_float_msub_d 
/target/mips/fpu_helper.c
  7  24,062  0.022%487   49 helper_cmp_d_lt 
/target/mips/fpu_helper.c
  8  22,910  0.021%150  152 helper_float_div_d  
/target/mips/fpu_helper.c
  9  15,497  0.014%321   48 helper_cmp_d_eq 
/target/mips/fpu_helper.c
 10   9,100  0.008% 52  175 helper_float_trunc_w_d  
/target/mips/fpu_helper.c
 11   7,059  0.006% 10  705 helper_float_sqrt_d 
/target/mips/fpu_helper.c
 12   3,000  0.003% 40   75 helper_cmp_d_ule
/target/mips/fpu_helper.c
 13   2,720  0.002% 20  136 helper_float_cvtd_w 
/target/mips/fpu_helper.c
 14   2,477  0.002% 27   91 helper_swl  
/target/mips/op_helper.c
 15   2,000  0.002% 40   50 helper_cmp_d_le 
/target/mips/fpu_helper.c
 16   1,800  0.002% 40   45 helper_cmp_d_un 
/target/mips/fpu_helper.c
 17   1,164  0.001% 12   97 helper_raise_exception_ 
/target/mips/op_helper.c
 18 720  0.001% 10   72 helper_cmp_d_ult
/target/mips/fpu_helper.c
 19 560  0.001%1404 helper_cfc1 
/target/mips/fpu_helper.c

Signed-off-by: Ahmed Karaman 
---
 scripts/performance/list_helpers.py | 207 
 1 file changed, 207 insertions(+)
 create mode 100755 scripts/performance/list_helpers.py

diff --git a/scripts/performance/list_helpers.py 
b/scripts/performance/list_helpers.py
new file mode 100755
index 00..a97c7ed4fe
--- /dev/null
+++ b/scripts/performance/list_helpers.py
@@ -0,0 +1,207 @@
+#!/usr/bin/env python3
+
+#  Print the executed helpers of a QEMU invocation.
+#
+#  Syntax:
+#  list_helpers.py [-h] -- \
+#  [] \
+#  []
+#
+#  [-h] - Print the script arguments help message.
+#
+#  Example of usage:
+#  list_helpers.py -- qemu-mips coulomb_double-mips
+#
+#  This file is a part of the project "TCG Continuous Benchmarking".
+#
+#  Copyright (C) 2020  Ahmed Karaman 
+#  Copyright (C) 2020  Aleksandar Markovic 
+#
+#  This program is free software: you can redistribute it and/or modify
+#  it under the terms of the GNU General Public License as published by
+#  the Free Software Foundation, either version 2 of the License, or
+#  (at your option) any later version.
+#
+#  This program is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY; without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+#  GNU General Public License for more details.
+#
+#  You should have received a copy of the GNU General Public License
+#  along with this program. If not, see .
+
+import argparse
+import os
+import subprocess
+import sys
+import tempfile
+
+
+def find_JIT_line(callgrind_data):
+"""
+Search for the line with the JIT call in the callgrind_annotate
+output when ran using --tre=calling.
+All the helpers should be listed after that line.
+
+Parameters:
+callgrind_data (list): callgrind_annotate output
+
+Returns:
+(int): Line number of JIT call
+"""
+line = -1
+for i in range(len(callgrind_data)):
+split_line = callgrind_data[i].split()
+if len(split_line) > 2 and \
+split_line[1] == "*" and \
+split_line[-1] == "[???]":
+line = i
+break
+return line
+
+
+def get_helpers(JIT_line, callgrind_data):
+"""
+Get all helpers data given the line number of the JIT call.
+
+Parameters:
+JIT_line (int): Line number of the JIT call
+callgrind_data (list): callgrind_annotate output
+
+Returns:
+(list):[[number_of_instructions(int), helper_name(str),
+ number_of_calls(int), source_file(str)]]
+"""
+helpers = []
+next_helper = JIT_line + 1
+while (callgrind_data[next_helper] != "\n"):
+split_line = callgrind_data[next_helper].split()
+number_of_instructions = 

[PATCH-for-5.1 1/4] qemu/osdep: Document os_find_datadir() return value

[Philippe Mathieu-Daudé]

Document os_find_datadir() returned data must be freed.

Signed-off-by: Philippe Mathieu-Daudé 
---
 os-posix.c | 3 +++
 os-win32.c | 7 ++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/os-posix.c b/os-posix.c
index b674b20b1b..3572db3f44 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -84,6 +84,9 @@ void os_setup_signal_handling(void)
  * Find a likely location for support files using the location of the binary.
  * When running from the build tree this will be "$bindir/../pc-bios".
  * Otherwise, this is CONFIG_QEMU_DATADIR.
+ *
+ * The caller must use g_free() to free the returned data when it is
+ * no longer required.
  */
 char *os_find_datadir(void)
 {
diff --git a/os-win32.c b/os-win32.c
index 6b86e022f0..c9c3afe648 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -57,7 +57,12 @@ void os_setup_early_signal_handling(void)
 atexit(os_undo_timer_resolution);
 }
 
-/* Look for support files in the same directory as the executable.  */
+/*
+ * Look for support files in the same directory as the executable.
+ *
+ * The caller must use g_free() to free the returned data when it is
+ * no longer required.
+ */
 char *os_find_datadir(void)
 {
 return qemu_get_exec_dir();
-- 
2.21.3

[PATCH-for-5.1 4/4] hw/avr/boot: Fix memory leak in avr_load_firmware()

[Philippe Mathieu-Daudé]

The value returned by qemu_find_file() must be freed.

This fixes Coverity issue CID 1430449, which points out
that the memory returned by qemu_find_file() is leaked.

Fixes: Coverity CID 1430449 (RESOURCE_LEAK)
Fixes: 7dd8f6fde4 ('hw/avr: Add support for loading ELF/raw binaries')
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/avr/boot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/avr/boot.c b/hw/avr/boot.c
index 6fbcde4061..151734f82d 100644
--- a/hw/avr/boot.c
+++ b/hw/avr/boot.c
@@ -60,7 +60,7 @@ static const char *avr_elf_e_flags_to_cpu_type(uint32_t flags)
 bool avr_load_firmware(AVRCPU *cpu, MachineState *ms,
MemoryRegion *program_mr, const char *firmware)
 {
-const char *filename;
+g_autofree char *filename;
 int bytes_loaded;
 uint64_t entry;
 uint32_t e_flags;
-- 
2.21.3

[PATCH 1/2] scripts/performance: Add list_fn_callees.py script

[Ahmed Karaman]

Python script that prints the callees of a given list of QEMU
functions.

Syntax:
list_fn_callees.py [-h] -f FUNCTION [FUNCTION ...] -- \
[] \
[]

[-h] - Print the script arguments help message.
-f FUNCTION [FUNCTION ...] - List of function names

Example of usage:
list_fn_callees.py -f helper_float_sub_d helper_float_mul_d -- \
  qemu-mips coulomb_double-mips -n10

Example output:
Total number of instructions: 108,952,851

Callees of helper_float_sub_d:

No. Instructions Percentage  Calls Ins/Call Function Name Source File
---  -- --  - ---
  1  153,160 0.141%  1,305 117  float64_sub   /fpu/softfloat.c

Callees of helper_float_mul_d:

No. Instructions Percentage  Calls Ins/Call Function Name Source File
---  -- --  - ---
  1  131,137 0.120%  1,014  129 float64_mul   /fpu/softfloat.c

Signed-off-by: Ahmed Karaman 
---
 scripts/performance/list_fn_callees.py | 228 +
 1 file changed, 228 insertions(+)
 create mode 100755 scripts/performance/list_fn_callees.py

diff --git a/scripts/performance/list_fn_callees.py 
b/scripts/performance/list_fn_callees.py
new file mode 100755
index 00..f0ec5c8e81
--- /dev/null
+++ b/scripts/performance/list_fn_callees.py
@@ -0,0 +1,228 @@
+#!/usr/bin/env python3
+
+#  Print the callees of a given list of QEMU functions.
+#
+#  Syntax:
+#  list_fn_callees.py [-h] -f FUNCTION [FUNCTION ...] -- \
+#  [] \
+#  []
+#
+#  [-h] - Print the script arguments help message.
+#  -f FUNCTION [FUNCTION ...] - List of function names
+#
+#  Example of usage:
+#  list_fn_callees.py -f helper_float_sub_d helper_float_mul_d -- \
+#qemu-mips coulomb_double-mips
+#
+#  This file is a part of the project "TCG Continuous Benchmarking".
+#
+#  Copyright (C) 2020  Ahmed Karaman 
+#  Copyright (C) 2020  Aleksandar Markovic 
+#
+#  This program is free software: you can redistribute it and/or modify
+#  it under the terms of the GNU General Public License as published by
+#  the Free Software Foundation, either version 2 of the License, or
+#  (at your option) any later version.
+#
+#  This program is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY; without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+#  GNU General Public License for more details.
+#
+#  You should have received a copy of the GNU General Public License
+#  along with this program. If not, see .
+
+import argparse
+import os
+import subprocess
+import sys
+import tempfile
+
+
+def find_function_lines(function_name, callgrind_data):
+"""
+Search for the line with the function name in the
+callgrind_annotate output when ran using --tre=calling.
+All the function callees should be listed after that line.
+
+Parameters:
+function_name (string): The desired function name to print its callees
+callgrind_data (list): callgrind_annotate output
+
+Returns:
+(list): List of function line numbers
+"""
+lines = []
+for i in range(len(callgrind_data)):
+split_line = callgrind_data[i].split()
+if len(split_line) > 2 and \
+split_line[1] == "*" and \
+split_line[2].split(":")[-1] == function_name:
+# Function might be in the callgrind_annotate output more than
+# once, so don't break after finding an instance
+if callgrind_data[i + 1] != "\n":
+# Only append the line number if the found instance has
+# callees
+lines.append(i)
+return lines
+
+
+def get_function_calles(function_lines, callgrind_data):
+"""
+Get all callees data for a function given its list of line numbers in
+callgrind_annotate output.
+
+Parameters:
+function_lines (list): Line numbers of the function to get its callees
+callgrind_data (list): callgrind_annotate output
+
+Returns:
+(list):[[number_of_instructions(int), callee_name(str),
+ number_of_calls(int), source_file(str)]]
+"""
+callees = []
+for function_line in function_lines:
+next_callee = function_line + 1
+while (callgrind_data[next_callee] != "\n"):
+split_line = callgrind_data[next_callee].split()
+number_of_instructions = int(split_line[0].replace(",", ""))
+source_file = split_line[2].split(":")[0]
+callee_name = split_line[2].split(":")[1]
+number_of_calls = int(split_line[3][1:-2])
+callees.append([number_of_instructions, callee_name,
+number_of_calls, source_file])
+next_callee += 1
+return sorted(callees, reverse=True)
+
+
+def main():
+# Parse the command line 

[PATCH-for-5.1 0/4] misc: Document qemu_find_file and fix memory leak in avr_load_firmware

[Philippe Mathieu-Daudé]

Fix the memory leak reported by Coverity (CID 1430449).

Philippe Mathieu-Daudé (4):
  qemu/osdep: Document os_find_datadir() return value
  qemu/osdep: Reword qemu_get_exec_dir() documentation
  qemu-common: Document qemu_find_file()
  hw/avr/boot: Fix memory leak in avr_load_firmware()

 include/qemu-common.h | 14 ++
 include/qemu/osdep.h  |  5 -
 hw/avr/boot.c |  2 +-
 os-posix.c|  3 +++
 os-win32.c|  7 ++-
 5 files changed, 28 insertions(+), 3 deletions(-)

-- 
2.21.3

Re: [PATCH v2 1/3] hw/i386: Initialize topo_ids from CpuInstanceProperties

[Igor Mammedov]

On Mon, 13 Jul 2020 14:30:29 -0500
Babu Moger  wrote:

> > -Original Message-
> > From: Igor Mammedov 
> > Sent: Monday, July 13, 2020 12:32 PM
> > To: Moger, Babu 
> > Cc: pbonz...@redhat.com; r...@twiddle.net; ehabk...@redhat.com; qemu-
> > de...@nongnu.org
> > Subject: Re: [PATCH v2 1/3] hw/i386: Initialize topo_ids from
> > CpuInstanceProperties
> > 
> > On Mon, 13 Jul 2020 11:43:33 -0500
> > Babu Moger  wrote:
> >   
> > > On 7/13/20 11:17 AM, Igor Mammedov wrote:  
> > > > On Mon, 13 Jul 2020 10:02:22 -0500
> > > > Babu Moger  wrote:
> > > >  
> > > >>> -Original Message-
> > > >>> From: Igor Mammedov 
> > > >>> Sent: Monday, July 13, 2020 4:08 AM
> > > >>> To: Moger, Babu 
> > > >>> Cc: pbonz...@redhat.com; r...@twiddle.net; ehabk...@redhat.com;
> > > >>> qemu- de...@nongnu.org
> > > >>> Subject: Re: [PATCH v2 1/3] hw/i386: Initialize topo_ids from
> > > >>> CpuInstanceProperties  
> > > > [...]  
> > >  +
> > >  +/*
> > >  + * Initialize topo_ids from CpuInstanceProperties
> > >  + * node_id in CpuInstanceProperties(or in CPU device) is a 
> > >  sequential
> > >  + * number, but while building the topology  
> > > >>>  
> > >  we need to separate it for
> > >  + * each socket(mod nodes_per_pkg).  
> > > >>> could you clarify a bit more on why this is necessary?  
> > > >>
> > > >> If you have two sockets and 4 numa nodes, node_id in
> > > >> CpuInstanceProperties will be number sequentially as 0, 1, 2, 3.
> > > >> But in EPYC topology, it will be  0, 1, 0, 1( Basically mod % number 
> > > >> of nodes  
> > per socket).  
> > > >
> > > > I'm confused, let's suppose we have 2 EPYC sockets with 2 nodes per
> > > > socket so APIC id woulbe be composed like:
> > > >
> > > >  1st socket
> > > >pkg_id(0) | node_id(0)
> > > >pkg_id(0) | node_id(1)
> > > >
> > > >  2nd socket
> > > >pkg_id(1) | node_id(0)
> > > >pkg_id(1) | node_id(1)
> > > >
> > > > if that's the case, then EPYC's node_id here doesn't look like a
> > > > NUMA node in the sense it's usually used (above config would have 4
> > > > different memory controllers => 4 conventional NUMA nodes).  
> > >
> > > EPIC model uses combination of socket id and node id to identify the
> > > numa nodes. So, it internally uses all the information.  
> > 
> > well with above values, EPYC's node_id doesn't look like it's specifying a
> > machine numa node, but rather a node index within single socket. In which 
> > case,
> > it doesn't make much sense calling it NUMA node_id, it's rather some index
> > within a socket. (it starts looking like terminology is all mixed up)
> > 
> > If you have access to a milti-socket EPYC machine, can you dump and post 
> > here
> > its apic ids, pls?  
> 
> Here is the output from my EPYC machine with 2 sockets and totally 8
> nodes(SMT disabled). The cpus 0-31 are in socket 0 and  cpus 32-63 in
> socket 1.
> 
> # lscpu
> Architecture:x86_64
> CPU op-mode(s):  32-bit, 64-bit
> Byte Order:  Little Endian
> CPU(s):  64
> On-line CPU(s) list: 0-63
> Thread(s) per core:  1
> Core(s) per socket:  32
> Socket(s):   2
> NUMA node(s):8
> Vendor ID:   AuthenticAMD
> CPU family:  23
> Model:   1
> Model name:  AMD Eng Sample: 1S1901A4VIHF5_30/19_N
> Stepping:2
> CPU MHz: 2379.233
> CPU max MHz: 1900.
> CPU min MHz: 1200.
> BogoMIPS:3792.81
> Virtualization:  AMD-V
> L1d cache:   32K
> L1i cache:   64K
> L2 cache:512K
> L3 cache:8192K
> NUMA node0 CPU(s):   0-7
> NUMA node1 CPU(s):   8-15
> NUMA node2 CPU(s):   16-23
> NUMA node3 CPU(s):   24-31
> NUMA node4 CPU(s):   32-39
> NUMA node5 CPU(s):   40-47
> NUMA node6 CPU(s):   48-55
> NUMA node7 CPU(s):   56-63
> 
> Here is the output of #cpuid  -l 0x801e  -r
> 
> You may want to refer
> https://www.amd.com/system/files/TechDocs/54945_3.03_ppr_ZP_B2_pub.zip
> (section 2.1.12.2.1.3 ApicId Enumeration Requirements).
> Note that this is a general guideline. We tried to generalize in qemu as
> much as possible. It is bit complex.
Thanks, I'll look into it.
Can you also dump SRAT table from that machine, please?
I'd like to see CPUs relation to NUMA nodes described in SRAT.

> 
> CPU 0:
>0x801e 0x00: eax=0x ebx=0x0100 ecx=0x0300
> edx=0x
> CPU 1:
>0x801e 0x00: eax=0x0002 ebx=0x0101 ecx=0x0300
> edx=0x
> CPU 2:
>0x801e 0x00: eax=0x0004 ebx=0x0102 ecx=0x0300
> edx=0x
> CPU 3:
>0x801e 0x00: eax=0x0006 ebx=0x0103 ecx=0x0300
> edx=0x
> CPU 4:
>0x801e 0x00: eax=0x0008 ebx=0x0104 ecx=0x0300
> edx=0x
> CPU 5:
>0x801e 0x00: eax=0x000a ebx=0x0105 ecx=0x0300
> edx=0x
> CPU 6:
>0x801e 0x00: eax=0x000c ebx=0x0106 ecx=0x0300
> edx=0x
> CPU 7:
>0x801e 

[PATCH 0/2] Add list_fn_callees.py and list_helpers.py scripts

[Ahmed Karaman]

Hi,

This series adds the two new scripts introduced in report 4 of the
"TCG Continuous Benchmarking" GSoC project.

"list_fn_callees.py" is used for printing the callees of a given list
of QEMU functions.

"list_helpers.py" is used for printing the executed helpers of a QEMU
invocation.

To learn more about how the scripts work and how they can be used for
analyzing the performance of different targets, please check the
"Listing QEMU Helpers and Function Callees" report.

Report link:
https://lists.nongnu.org/archive/html/qemu-devel/2020-07/msg04227.html

Best regards,
Ahmed Karaman

Ahmed Karaman (2):
  scripts/performance: Add list_fn_callees.py script
  scripts/performance: Add list_helpers.py script

 scripts/performance/list_fn_callees.py | 228 +
 scripts/performance/list_helpers.py| 207 ++
 2 files changed, 435 insertions(+)
 create mode 100755 scripts/performance/list_fn_callees.py
 create mode 100755 scripts/performance/list_helpers.py

-- 
2.17.1

[PATCH-for-5.1 2/4] qemu/osdep: Reword qemu_get_exec_dir() documentation

[Philippe Mathieu-Daudé]

This comment is confuse, reword it a bit.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/qemu/osdep.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 979a403984..a96849dd90 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -588,7 +588,10 @@ char *qemu_get_local_state_pathname(const char 
*relative_pathname);
 void qemu_init_exec_dir(const char *argv0);
 
 /* Get the saved exec dir.
- * Caller needs to release the returned string by g_free() */
+ *
+ * The caller is responsible for releasing the value returned with g_free()
+ * after use.
+ */
 char *qemu_get_exec_dir(void);
 
 /**
-- 
2.21.3