Re: [PULL 1/1] Revert "configure: build ROMs with container-based cross compilers"

[Paolo Bonzini]

Il mar 11 ott 2022, 21:29 Alex Bennée  ha scritto:

> This reverts commit 730fe750fba63023e294ff0acf0f874369f1946f.
>
> Unconditionally building all the bios for all arches was a little too
> far too fast.
>

I would like to understand the issue better, because chances are that it is
preexisting and applies to the TCG tests as well.

Daniel, does building the TCG tests work for you? If not, I think we should
just disable containers by default.



> Signed-off-by: Alex Bennée 
> Cc: Paolo Bonzini 
> Reviewed-by: Daniel Henrique Barboza 
> Tested-by: Daniel Henrique Barboza 
> Message-Id: <2022103417.794841-4-alex.ben...@linaro.org>
>
> diff --git a/configure b/configure
> index baa69189f0..45ee6f4eb3 100755
> --- a/configure
> +++ b/configure
> @@ -2121,7 +2121,7 @@ probe_target_compiler() {
>  target_ranlib=
>  target_strip=
>fi
> -  test -n "$target_cc" || test -n "$container_image"
> +  test -n "$target_cc"
>  }
>
>  write_target_makefile() {
> @@ -2268,7 +2268,7 @@ if test "$targetos" != "darwin" && test "$targetos"
> != "sunos" && \
>  config_mak=pc-bios/optionrom/config.mak
>  echo "# Automatically generated by configure - do not modify" >
> $config_mak
>  echo "TOPSRC_DIR=$source_path" >> $config_mak
> -write_target_makefile pc-bios/optionrom/all >> $config_mak
> +write_target_makefile >> $config_mak
>  fi
>
>  if test "$softmmu" = yes && probe_target_compiler ppc-softmmu; then
> @@ -2276,31 +2276,25 @@ if test "$softmmu" = yes && probe_target_compiler
> ppc-softmmu; then
>  config_mak=pc-bios/vof/config.mak
>  echo "# Automatically generated by configure - do not modify" >
> $config_mak
>  echo "SRC_DIR=$source_path/pc-bios/vof" >> $config_mak
> -write_target_makefile pc-bios/vof/all >> $config_mak
> +write_target_makefile >> $config_mak
>  fi
>
>  # Only build s390-ccw bios if the compiler has -march=z900 or -march=z10
>  # (which is the lowest architecture level that Clang supports)
>  if test "$softmmu" = yes && probe_target_compiler s390x-softmmu; then
> -  got_cross_cc=no
> -  if test -n "$target_cc"; then
> -write_c_skeleton
> -do_compiler "$target_cc" $target_cc_cflags -march=z900 -o $TMPO -c
> $TMPC
> -has_z900=$?
> -if [ $has_z900 = 0 ] || do_compiler "$target_cc" $target_cc_cflags
> -march=z10 -msoft-float -Werror -o $TMPO -c $TMPC; then
> -  if [ $has_z900 != 0 ]; then
> -echo "WARNING: Your compiler does not support the z900!"
> -echo " The s390-ccw bios will only work with guest CPUs
> >= z10."
> -  fi
> -  got_cross_cc=yes
> +  write_c_skeleton
> +  do_compiler "$target_cc" $target_cc_cflags -march=z900 -o $TMPO -c $TMPC
> +  has_z900=$?
> +  if [ $has_z900 = 0 ] || do_compiler "$target_cc" $target_cc_cflags
> -march=z10 -msoft-float -Werror -o $TMPO -c $TMPC; then
> +if [ $has_z900 != 0 ]; then
> +  echo "WARNING: Your compiler does not support the z900!"
> +  echo " The s390-ccw bios will only work with guest CPUs >=
> z10."
>  fi
> -  fi
> -  if test "$got_cross_cc" = yes || test -n "$container_image"; then
>  roms="$roms pc-bios/s390-ccw"
>  config_mak=pc-bios/s390-ccw/config-host.mak
>  echo "# Automatically generated by configure - do not modify" >
> $config_mak
>  echo "SRC_PATH=$source_path/pc-bios/s390-ccw" >> $config_mak
> -write_target_makefile pc-bios/s390-ccw/all >> $config_mak
> +write_target_makefile >> $config_mak
>  # SLOF is required for building the s390-ccw firmware on s390x,
>  # since it is using the libnet code from SLOF for network booting.
>  git_submodules="${git_submodules} roms/SLOF"
> @@ -2488,7 +2482,7 @@ for target in $target_list; do
>;;
>esac
>
> -  if probe_target_compiler $target; then
> +  if probe_target_compiler $target || test -n "$container_image"; then
>test -n "$container_image" && build_static=y
>mkdir -p "tests/tcg/$target"
>config_target_mak=tests/tcg/$target/config-target.mak
> --
> 2.34.1
>
>

Re: [PATCH v5 11/18] tests/qtest: Support libqtest to build and run on Windows

[Marc-André Lureau]

Hi Bin

On Thu, Oct 6, 2022 at 8:09 PM Bin Meng  wrote:

> From: Bin Meng 
>
> At present the libqtest codes were written to depend on several
> POSIX APIs, including fork(), kill() and waitpid(). Unfortunately
> these APIs are not available on Windows.
>
> This commit implements the corresponding functionalities using
> win32 native APIs. With this change, all qtest cases can build
> successfully on a Windows host, and we can start qtest testing
> on Windows now.
>
> Signed-off-by: Xuzhou Cheng 
> Signed-off-by: Bin Meng 
> Reviewed-by: Marc-André Lureau 
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Move the enabling of building qtests on Windows to a separate
>   patch to keep bisectablity
> - Call socket_init() unconditionally
> - Add a missing CloseHandle() call
>
>  tests/qtest/libqtest.c | 95 +-
>  1 file changed, 93 insertions(+), 2 deletions(-)
>
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 54e5f64f20..ecd22cdb11 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -16,9 +16,11 @@
>
>  #include "qemu/osdep.h"
>
> +#ifndef _WIN32
>  #include 
>  #include 
>  #include 
> +#endif /* _WIN32 */
>  #ifdef __linux__
>  #include 
>  #endif /* __linux__ */
> @@ -27,6 +29,7 @@
>  #include "libqmp.h"
>  #include "qemu/ctype.h"
>  #include "qemu/cutils.h"
> +#include "qemu/sockets.h"
>  #include "qapi/qmp/qdict.h"
>  #include "qapi/qmp/qjson.h"
>  #include "qapi/qmp/qlist.h"
> @@ -35,6 +38,16 @@
>  #define MAX_IRQ 256
>  #define SOCKET_TIMEOUT 50
>
> +#ifndef _WIN32
> +# define CMD_EXEC   "exec "
> +# define DEV_STDERR "/dev/fd/2"
> +# define DEV_NULL   "/dev/null"
> +#else
> +# define CMD_EXEC   ""
> +# define DEV_STDERR "2"
> +# define DEV_NULL   "nul"
> +#endif
> +
>  typedef void (*QTestSendFn)(QTestState *s, const char *buf);
>  typedef void (*ExternalSendFn)(void *s, const char *buf);
>  typedef GString* (*QTestRecvFn)(QTestState *);
> @@ -118,10 +131,19 @@ bool qtest_probe_child(QTestState *s)
>  pid_t pid = s->qemu_pid;
>
>  if (pid != -1) {
> +#ifndef _WIN32
>  pid = waitpid(pid, &s->wstatus, WNOHANG);
>  if (pid == 0) {
>  return true;
>  }
> +#else
> +DWORD exit_code;
> +GetExitCodeProcess((HANDLE)pid, &exit_code);
> +if (exit_code == STILL_ACTIVE) {
> +return true;
> +}
> +CloseHandle((HANDLE)pid);
> +#endif
>  s->qemu_pid = -1;
>  }
>  return false;
> @@ -135,13 +157,23 @@ void qtest_set_expected_status(QTestState *s, int
> status)
>  void qtest_kill_qemu(QTestState *s)
>  {
>  pid_t pid = s->qemu_pid;
> +#ifndef _WIN32
>  int wstatus;
> +#else
> +DWORD ret, exit_code;
> +#endif
>
>  /* Skip wait if qtest_probe_child already reaped.  */
>  if (pid != -1) {
> +#ifndef _WIN32
>  kill(pid, SIGTERM);
>  TFR(pid = waitpid(s->qemu_pid, &s->wstatus, 0));
>  assert(pid == s->qemu_pid);
> +#else
> +TerminateProcess((HANDLE)pid, s->expected_status);
> +ret = WaitForSingleObject((HANDLE)pid, INFINITE);
> +assert(ret == WAIT_OBJECT_0);
> +#endif
>  s->qemu_pid = -1;
>  }
>
> @@ -149,6 +181,7 @@ void qtest_kill_qemu(QTestState *s)
>   * Check whether qemu exited with expected exit status; anything else
> is
>   * fishy and should be logged with as much detail as possible.
>   */
> +#ifndef _WIN32
>  wstatus = s->wstatus;
>  if (WIFEXITED(wstatus) && WEXITSTATUS(wstatus) != s->expected_status)
> {
>  fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
> @@ -165,6 +198,16 @@ void qtest_kill_qemu(QTestState *s)
>  __FILE__, __LINE__, sig, signame, dump);
>  abort();
>  }
> +#else
> +GetExitCodeProcess((HANDLE)pid, &exit_code);
>

That doesn't fulfill the doc expectations: "It is safe to call this
function multiple times". pid will be -1 after the first call. I think it
should save the "exit_code", similar to how "wstatus" is saved in posix.

please update the patch

+CloseHandle((HANDLE)pid);
> +if (exit_code != s->expected_status) {
> +fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
> +"process but encountered exit status %ld (expected %d)\n",
> +__FILE__, __LINE__, exit_code, s->expected_status);
> +abort();
> +}
> +#endif
>  }
>
>  static void kill_qemu_hook_func(void *s)
> @@ -243,6 +286,38 @@ static const char *qtest_qemu_binary(void)
>  return qemu_bin;
>  }
>
> +#ifdef _WIN32
> +static pid_t qtest_create_process(char *cmd)
> +{
> +STARTUPINFO si;
> +PROCESS_INFORMATION pi;
> +BOOL ret;
> +
> +ZeroMemory(&si, sizeof(si));
> +si.cb = sizeof(si);
> +ZeroMemory(&pi, sizeof(pi));
> +
> +ret = CreateProcess(NULL,   /* module name */
> +cmd,/* command line */
> +NULL,   /* process handle not inheritable */

Re: [PATCH] gitmodules: recurse by default

[Daniel P . Berrangé]

On Tue, Oct 11, 2022 at 06:32:40PM -0400, Michael S. Tsirkin wrote:
> On Fri, Oct 07, 2022 at 12:09:40PM +0100, Daniel P. Berrangé wrote:
> > On Fri, Oct 07, 2022 at 11:45:56AM +0100, Daniel P. Berrangé wrote:
> > > On Fri, Oct 07, 2022 at 06:11:25AM -0400, Michael S. Tsirkin wrote:
> > > > On Fri, Oct 07, 2022 at 09:07:17AM +0100, Daniel P. Berrangé wrote:
> > > > > On Thu, Oct 06, 2022 at 08:24:01PM -0400, Michael S. Tsirkin wrote:
> > > > > > On Thu, Oct 06, 2022 at 07:54:52PM +0100, Daniel P. Berrangé wrote:
> > > > > > > On Thu, Oct 06, 2022 at 07:39:07AM -0400, Michael S. Tsirkin 
> > > > > > > wrote:
> > > > > > > > The most commmon complaint about submodules is that
> > > > > > > > they don't follow when one switches branches in the
> > > > > > > > main repo. Enable recursing into submodules by default
> > > > > > > > to address that.
> > > > > > > > 
> > > > > > > > Signed-off-by: Michael S. Tsirkin 
> > > > > > > > ---
> > > > > > > >  .gitmodules | 23 +++
> > > > > > > >  1 file changed, 23 insertions(+)
> > 
> > snip
> > 
> > > > I just retested and it's not working for me either :(
> > > > I was sure it worked but I guess the testing wasn't done properly.
> > > > Back to the drawing board sorry.
> > > 
> > > I think the problem is that this setting doesn't apply in the context
> > > of .gitmodules. Various commands take a '--recurse-submodules' parameter,
> > > and like many params this can be set in the .git/config file. The
> > > problem is .git/config isn't a file we can influence automatically,
> > > it is upto the dev to set things for every clone they do :-(
> > 
> > With the correct setting in my .git/config, I've just discovered
> > an unexpected & undesirable consequence of using recurse=true.
> > It affects the 'push' command. If your submodule contains a hash
> > that is not present in the upstream of the submodule, then when
> > you try to push, it will also try to push the submodule change.
> > 
> > eg, I have a qemu.git branch 'work' and i made a change to
> > ui/keycodemapdb. If I try to push to my gitlab fork, whose
> > remote I called 'gitlab', then it will also try to push
> > ui/keycodemapdb to a fork called 'gitlab'.  Except I don't
> > have any such fork existing, so my attempt to push my qemu.git
> > changes fails because of the submodule.
> > 
> > This is going to be annoying to people who are working on branches
> > with updates to the git submodules if we were to set recurse=true
> > by default, as they'll have to also setup remotes for submodules
> > they work on.
> > 
> 
> Well this seems like a reasonable thing to do, no?
> 
> If you push qemu commit referring to hash 0xABC, you want
> that 0xABC to be available in the remote, no?
> Otherwise how will people fetching your tree check it out?

Don't assume I'm making it available for other people. I push to
remotes simply for moving code around for myself between machines.
I still have the submodule code I need elsewhere, so forcing me
to push the submodule & main repos so the same named remote is
getting in the way. 

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH v5 12/18] tests/qtest: migration-test: Make sure QEMU process "to" exited after migration is canceled

[Marc-André Lureau]

Hi

On Thu, Oct 6, 2022 at 8:12 PM Bin Meng  wrote:

> From: Xuzhou Cheng 
>
> Make sure QEMU process "to" exited before launching another target
> for migration in the test_multifd_tcp_cancel case.
>
> Signed-off-by: Xuzhou Cheng 
> Signed-off-by: Bin Meng 
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - Add a usleep(1) in the busy wait loop
>
> Changes in v2:
> - Change to a busy wait after migration is canceled
>
>  tests/qtest/migration-test.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index ef4427ff4d..e5ba0e21d2 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -2140,6 +2140,10 @@ static void test_multifd_tcp_cancel(void)
>  wait_for_migration_pass(from);
>
>  migrate_cancel(from);
> +/* Make sure QEMU process "to" exited */
> +while (qtest_probe_child(to)) {
> +usleep(1);
> +}
>
>
As discussed earlier, I think we can introduce a qtest_wait_qemu() instead,
something like that should work:


-void qtest_kill_qemu(QTestState *s)
+static void qtest_check_status(QTestState *s)
 {
-pid_t pid = s->qemu_pid;
-#ifndef _WIN32
-int wstatus;
-#else
-DWORD ret, exit_code;
-#endif
-
-/* Skip wait if qtest_probe_child already reaped.  */
-if (pid != -1) {
-#ifndef _WIN32
-kill(pid, SIGTERM);
-TFR(pid = waitpid(s->qemu_pid, &s->wstatus, 0));
-assert(pid == s->qemu_pid);
-#else
-TerminateProcess((HANDLE)pid, s->expected_status);
-ret = WaitForSingleObject((HANDLE)pid, INFINITE);
-assert(ret == WAIT_OBJECT_0);
-#endif
-s->qemu_pid = -1;
-}
-
 /*
  * Check whether qemu exited with expected exit status; anything else
is
  * fishy and should be logged with as much detail as possible.
  */
 #ifndef _WIN32
-wstatus = s->wstatus;
+int wstatus = s->wstatus;
 if (WIFEXITED(wstatus) && WEXITSTATUS(wstatus) != s->expected_status) {
 fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
 "process but encountered exit status %d (expected %d)\n",
@@ -208,17 +191,50 @@ void qtest_kill_qemu(QTestState *s)
 abort();
 }
 #else
-GetExitCodeProcess((HANDLE)pid, &exit_code);
-CloseHandle((HANDLE)pid);
-if (exit_code != s->expected_status) {
+if (s->exit_code != s->expected_status) {
 fprintf(stderr, "%s:%d: kill_qemu() tried to terminate QEMU "
 "process but encountered exit status %ld (expected %d)\n",
-__FILE__, __LINE__, exit_code, s->expected_status);
+__FILE__, __LINE__, s->exit_code, s->expected_status);
 abort();
 }
 #endif
 }

+void qtest_kill_qemu(QTestState *s)
+{
+/* Skip wait if qtest_probe_child already reaped.  */
+if (s->qemu_pid != -1) {
+#ifndef _WIN32
+kill(s->qemu_pid, SIGTERM);
+#else
+TerminateProcess((HANDLE)s->qemu_pid, s->expected_status);
+#endif
+qtest_wait_qemu(s);
+s->qemu_pid = -1;
+}
+
+qtest_check_status(s);
+}
+
+void qtest_wait_qemu(QTestState *s)
+{
+#ifndef _WIN32
+pid_t pid;
+
+TFR(pid = waitpid(s->qemu_pid, &s->wstatus, 0));
+assert(pid == s->qemu_pid);
+#else
+DWORD ret;
+
+ret = WaitForSingleObject((HANDLE)s->qemu_pid, INFINITE);
+assert(ret == WAIT_OBJECT_0);
+GetExitCodeProcess((HANDLE)s->qemu_pid, &s->exit_code);
+CloseHandle((HANDLE)s->qemu_pid);
+#endif
+
+qtest_check_status(s);
+}
+
 static void kill_qemu_hook_func(void *s)
 {
 qtest_kill_qemu(s);




-- 
Marc-André Lureau

Re: [PATCH v2 3/7] util: Introduce ThreadContext user-creatable object

[Markus Armbruster]

David Hildenbrand  writes:

>>> But note that due to dynamic library loading this example will not work
>>> before we actually make use of thread_context_create_thread() in QEMU
>>> code, because the type will otherwise not get registered.
>> 
>> What do you mean exactly by "not work"?  It's not "CLI option or HMP
>> command fails":
>
> For me, if I compile patch #1-#3 only, I get:
>
> $ ./build/qemu-system-x86_64 -S -display none -nodefaults -monitor stdio 
> -object thread-context,id=tc1,cpu-affinity=0-1,cpu-affinity=6-7
> qemu-system-x86_64: invalid object type: thread-context
>
>
> Reason is that, without a call to thread_context_create_thread(), we won't 
> trigger type_init(thread_context_register_types) and consequently, 
> the type won't be registered.
>
> Is it really different in your environment? Maybe it depends on the QEMU 
> config?

I just tested again, and get the same result as you.  I figure my
previous test was with the complete series.

PATCH 5 appears to make it work.  Suggest to say something like "The
commit after next will make this work".

>>  $ upstream-qemu -S -display none -nodefaults -monitor stdio -object 
>> thread-context,id=tc1,cpu-affinity=0-1,cpu-affinity=6-7
>>  QEMU 7.1.50 monitor - type 'help' for more information
>>  (qemu) qom-get tc1 cpu-affinity
>>  [
>>  0,
>>  1,
>>  6,
>>  7
>>  ]
>>  (qemu) info cpus
>>  * CPU #0: thread_id=1670613
>> 
>> Even though the affinities refer to nonexistent CPUs :)
>
> CPU affinities are CPU numbers on your system (host), not QEMU vCPU numbers. 
> I could talk about physical CPU numbers in the doc here, 
> although I am not sure if that really helps. What about "host CPU numbers" 
> and in patch #4 "host node numbers"?

I think this would reduce the confusion opportunities for noobs like me.

> Seems to match what we document for @MemoryBackendProperties: "@host-nodes: 
> the list of NUMA host nodes to bind the memory to"

Even better.

> But unrelated to that, pthread_setaffinity_np() won't bail out on CPUs that 
> are currently not available in the host -- because one might 
> online/hotplug them later. It only bails out if none of the CPUs is currently 
> available in the host:
>
> https://man7.org/linux/man-pages/man3/pthread_setaffinity_np.3.html
>
>
>EINVAL (pthread_setaffinity_np()) The affinity bit mask mask
>   contains no processors that are currently physically on
>   the system and permitted to the thread according to any
>   restrictions that may be imposed by the "cpuset" mechanism
>   described in cpuset(7).
>
> It will bail out on CPUs that cannot be available in the host though, because 
> it's impossible due to the kernel config:
>
>
>EINVAL (pthread_setaffinity_np()) cpuset specified a CPU that was
>   outside the set supported by the kernel.  (The kernel
>   configuration option CONFIG_NR_CPUS defines the range of
>   the set supported by the kernel data type used to
>   represent CPU sets.)
>
>
>>> A ThreadContext can be reused, simply by reconfiguring the CPU affinity.
>> 
>> So, when a thread is created, its affinity comes from its thread context
>> (if any).  When I later change the context's affinity, it does *not*
>> affect existing threads, only future ones.  Correct?
>
> Yes, that's the current state.

Thanks!

>>> Reviewed-by: Michal Privoznik 
>>> Signed-off-by: David Hildenbrand 

[...]

>>> diff --git a/qapi/qom.json b/qapi/qom.json
>>> index 80dd419b39..67d47f4051 100644
>>> --- a/qapi/qom.json
>>> +++ b/qapi/qom.json
>>> @@ -830,6 +830,21 @@
>>>   'reduced-phys-bits': 'uint32',
>>>   '*kernel-hashes': 'bool' } }
>>>   +##
>>> +# @ThreadContextProperties:
>>> +#
>>> +# Properties for thread context objects.
>>> +#
>>> +# @cpu-affinity: the list of CPU numbers used as CPU affinity for all 
>>> threads
>>> +#created in the thread context (default: QEMU main thread
>>> +#affinity)
>>
>> Another ignorant question: is the QEMU main thread affinity fixed or
>> configurable?  If configurable, how?
>
> AFAIK, it's only configurable externally, for example, via "virsh 
> emulatorpin". There is no QEMU interface to adjust that (because it 
> wouldn't work).
>
> Libvirt will essentially trigger "taskset" on the emulator thread to change 
> its CPU affinity.

I see.

QAPI schema
Acked-by: Markus Armbruster 

Re: [PATCH v2 4/7] util: Add write-only "node-affinity" property for ThreadContext

[Markus Armbruster]

David Hildenbrand  writes:

> On 11.10.22 08:03, Markus Armbruster wrote:
>> David Hildenbrand  writes:
>> 
>>> Let's make it easier to pin threads created via a ThreadContext to
>>> all CPUs currently belonging to a given set of NUMA nodes -- which is the
>>> common case.
>>>
>>> "node-affinity" is simply a shortcut for setting "cpu-affinity" manually
>>> to the list of CPUs belonging to the set of nodes. This property can only
>>> be written.
>>>
>>> A simple QEMU example to set the CPU affinity to Node 1 on a system with
>>> two NUMA nodes, 24 CPUs each:
>>>  qemu-system-x86_64 -S \
>>>-object thread-context,id=tc1,node-affinity=1
>>>
>>> And we can query the cpu-affinity via HMP/QMP:
>>>  (qemu) qom-get tc1 cpu-affinity
>>>  [
>>>  1,
>>>  3,
>>>  5,
>>>  7,
>>>  9,
>>>  11,
>>>  13,
>>>  15,
>>>  17,
>>>  19,
>>>  21,
>>>  23,
>>>  25,
>>>  27,
>>>  29,
>>>  31,
>>>  33,
>>>  35,
>>>  37,
>>>  39,
>>>  41,
>>>  43,
>>>  45,
>>>  47
>>>  ]
>> 
>> Double-checking my understanding: on this system, the even CPUs belong
>> to NUMA node 0, and the odd ones to node 1.  Setting node-affinity=1 is
>> therefore sugar for setting cpu-affinity to the set of even CPUs.
>> Correct?
>
> Yes!
>
> # lscpu
> ...
> NUMA:
>   NUMA node(s):  2
>   NUMA node0 CPU(s): 
> 0,2,4,6,8,10,12,14,16,18,20,22,24,26,28,30,32,34,36,38,40,42,44,46
>   NUMA node1 CPU(s): 
> 1,3,5,7,9,11,13,15,17,19,21,23,25,27,29,31,33,35,37,39,41,43,45,47

Thanks!

>>> We cannot query the node-affinity:
>>>  (qemu) qom-get tc1 node-affinity
>>>  Error: Insufficient permission to perform this operation
>> 
>> The error message is somewhat misleading.  "Insufficient permission"
>> suggests this could work if I had more "permission".  Not the case.  The
>> message comes from object_property_get(), i.e. it's not this patch's
>> fault.  I'll post a patch to improve it.
>
> I agree, thanks!

QAPI schema
Acked-by: Markus Armbruster 

Re: [PATCH] hw/block/nvme: re-enable NVMe PCI hotplug

[Klaus Jensen]

On Okt 12 08:24, Hannes Reinecke wrote:
> On 10/10/22 19:01, Daniel Wagner wrote:
> > On Tue, May 11, 2021 at 06:12:47PM +0200, Hannes Reinecke wrote:
> > > On 5/11/21 6:03 PM, Klaus Jensen wrote:
> > > > On May 11 16:54, Hannes Reinecke wrote:
> > > > > On 5/11/21 3:37 PM, Klaus Jensen wrote:
> > > > > > On May 11 15:12, Hannes Reinecke wrote:
> > > > > > > On 5/11/21 2:22 PM, Klaus Jensen wrote:
> > > > > [ .. ]
> > > > > > > > The hotplug fix looks good - I'll post a series that
> > > > > > > > tries to integrate
> > > > > > > > both.
> > > > > > > > 
> > > > > > > Ta.
> > > > > > > 
> > > > > > > The more I think about it, the more I think we should be looking 
> > > > > > > into
> > > > > > > reparenting the namespaces to the subsystem.
> > > > > > > That would have the _immediate_ benefit that 'device_del' and
> > > > > > > 'device_add' becomes symmetric (ie one doesn't have to do a 
> > > > > > > separate
> > > > > > > 'device_add nvme-ns'), as the nvme namespace is not affected by 
> > > > > > > the
> > > > > > > hotplug event.
> > > > > > > 
> > > > > > 
> > > > > > I have that working, but I'm struggling with a QEMU API 
> > > > > > technicality in
> > > > > > that I apparently cannot simply move the NvmeBus creation to the
> > > > > > nvme-subsys device. For some reason the bus is not available for the
> > > > > > nvme-ns devices. That is, if one does something like this:
> > > > > > 
> > > > > >    -device nvme-subsys,...
> > > > > >    -device nvme-ns,...
> > > > > > 
> > > > > > Then I get an error that "no 'nvme-bus' bus found for device 
> > > > > > 'nvme'ns".
> > > > > > This is probably just me not grok'ing the qdev well enough, so I'll 
> > > > > > keep
> > > > > > trying to fix that. What works now is to have the regular setup:
> > > > > > 
> > > > > _Normally_ the 'id' of the parent device spans a bus, so the syntax
> > > > > should be
> > > > > 
> > > > > -device nvme-subsys,id=subsys1,...
> > > > > -device nvme-ns,bus=subsys1,...
> > > > 
> > > > Yeah, I know, I just oversimplified the example. This *is* how I wanted
> > > > it to work ;)
> > > > 
> > > > > 
> > > > > As for the nvme device I would initially expose any namespace from the
> > > > > subsystem to the controller; the nvme spec has some concept of 
> > > > > 'active'
> > > > > or 'inactive' namespaces which would allow us to blank out individual
> > > > > namespaces on a per-controller basis, but I fear that's not easy to
> > > > > model with qdev and the structure above.
> > > > > 
> > > > 
> > > > The nvme-ns device already supports the boolean 'detached' parameter to
> > > > support the concept of an inactive namespace.
> > > > 
> > > Yeah, but that doesn't really work if we move the namespace to the
> > > subsystem; the 'detached' parameter is for the controller<->namespace
> > > relationship.
> > > That's why I meant we have to have some sort of NSID map for the 
> > > controller
> > > such that the controller knows with NSID to access.
> > > I guess we can copy the trick with the NSID array, and reverse the 
> > > operation
> > > we have now wrt subsystem; keep the main NSID array in the subsystem, and
> > > per-controller NSID arrays holding those which can be accessed.
> > > 
> > > And ignore the commandline for now; figure that one out later.
> > > 
> [..]
> > 
> > Sorry to ask but has there been any progress on this topic? Just run
> > into the same issue that adding nvme device during runtime is not
> > showing any namespace.
> > 
> I _thought_ that the pci hotplug fixes have now been merged with qemu
> upstream. Klaus?
> 

Yup. It's all upstream.


signature.asc
Description: PGP signature

Re: [PATCH v8 2/8] KVM: Extend the memslot to support fd-based private memory

[Jarkko Sakkinen]

On Mon, Oct 10, 2022 at 04:25:07PM +0800, Chao Peng wrote:
> On Sat, Oct 08, 2022 at 08:35:47PM +0300, Jarkko Sakkinen wrote:
> > On Sat, Oct 08, 2022 at 07:15:17PM +0300, Jarkko Sakkinen wrote:
> > > On Sat, Oct 08, 2022 at 12:54:32AM +0300, Jarkko Sakkinen wrote:
> > > > On Fri, Oct 07, 2022 at 02:58:54PM +, Sean Christopherson wrote:
> > > > > On Fri, Oct 07, 2022, Jarkko Sakkinen wrote:
> > > > > > On Thu, Oct 06, 2022 at 03:34:58PM +, Sean Christopherson wrote:
> > > > > > > On Thu, Oct 06, 2022, Jarkko Sakkinen wrote:
> > > > > > > > On Thu, Oct 06, 2022 at 05:58:03PM +0300, Jarkko Sakkinen wrote:
> > > > > > > > > On Thu, Sep 15, 2022 at 10:29:07PM +0800, Chao Peng wrote:
> > > > > > > > > > This new extension, indicated by the new flag 
> > > > > > > > > > KVM_MEM_PRIVATE, adds two
> > > > > > > > > > additional KVM memslot fields private_fd/private_offset to 
> > > > > > > > > > allow
> > > > > > > > > > userspace to specify that guest private memory provided 
> > > > > > > > > > from the
> > > > > > > > > > private_fd and guest_phys_addr mapped at the private_offset 
> > > > > > > > > > of the
> > > > > > > > > > private_fd, spanning a range of memory_size.
> > > > > > > > > > 
> > > > > > > > > > The extended memslot can still have the 
> > > > > > > > > > userspace_addr(hva). When use, a
> > > > > > > > > > single memslot can maintain both private memory through 
> > > > > > > > > > private
> > > > > > > > > > fd(private_fd/private_offset) and shared memory through
> > > > > > > > > > hva(userspace_addr). Whether the private or shared part is 
> > > > > > > > > > visible to
> > > > > > > > > > guest is maintained by other KVM code.
> > > > > > > > > 
> > > > > > > > > What is anyway the appeal of private_offset field, instead of 
> > > > > > > > > having just
> > > > > > > > > 1:1 association between regions and files, i.e. one memfd per 
> > > > > > > > > region?
> > > > > > > 
> > > > > > > Modifying memslots is slow, both in KVM and in QEMU (not sure 
> > > > > > > about Google's VMM).
> > > > > > > E.g. if a vCPU converts a single page, it will be forced to wait 
> > > > > > > until all other
> > > > > > > vCPUs drop SRCU, which can have severe latency spikes, e.g. if 
> > > > > > > KVM is faulting in
> > > > > > > memory.  KVM's memslot updates also hold a mutex for the entire 
> > > > > > > duration of the
> > > > > > > update, i.e. conversions on different vCPUs would be fully 
> > > > > > > serialized, exacerbating
> > > > > > > the SRCU problem.
> > > > > > > 
> > > > > > > KVM also has historical baggage where it "needs" to zap _all_ 
> > > > > > > SPTEs when any
> > > > > > > memslot is deleted.
> > > > > > > 
> > > > > > > Taking both a private_fd and a shared userspace address allows 
> > > > > > > userspace to convert
> > > > > > > between private and shared without having to manipulate memslots.
> > > > > > 
> > > > > > Right, this was really good explanation, thank you.
> > > > > > 
> > > > > > Still wondering could this possibly work (or not):
> > > > > > 
> > > > > > 1. Union userspace_addr and private_fd.
> > > > > 
> > > > > No, because userspace needs to be able to provide both userspace_addr 
> > > > > (shared
> > > > > memory) and private_fd (private memory) for a single memslot.
> > > > 
> > > > Got it, thanks for clearing my misunderstandings on this topic, and it
> > > > is quite obviously visible in 5/8 and 7/8. I.e. if I got it right,
> > > > memblock can be partially private, and you dig the shared holes with
> > > > KVM_MEMORY_ENCRYPT_UNREG_REGION. We have (in Enarx) ATM have memblock
> > > > per host mmap, I was looking into this dilated by that mindset but makes
> > > > definitely sense to support that.
> > > 
> > > For me the most useful reference with this feature is kvm_set_phys_mem()
> > > implementation in privmem-v8 branch. Took while to find it because I did
> > > not have much experience with QEMU code base. I'd even recommend to 
> > > mention
> > > that function in the cover letter because it is really good reference on
> > > how this feature is supposed to be used.
> 
> That's a good point, I can mention that if people find useful. 

Yeah, I did implementation for Enarx (https://www.enarx.dev/) using just
that part as a reference. It has all the essentials what you need to
consider when you are already using KVM API, and want to add private
regions.

BR, Jarkko

Re: [PULL 1/1] Revert "configure: build ROMs with container-based cross compilers"

[Daniel P . Berrangé]

On Wed, Oct 12, 2022 at 08:46:35AM +0200, Paolo Bonzini wrote:
> Il mar 11 ott 2022, 21:29 Alex Bennée  ha scritto:
> 
> > This reverts commit 730fe750fba63023e294ff0acf0f874369f1946f.
> >
> > Unconditionally building all the bios for all arches was a little too
> > far too fast.
> >
> 
> I would like to understand the issue better, because chances are that it is
> preexisting and applies to the TCG tests as well.
> 
> Daniel, does building the TCG tests work for you? If not, I think we should
> just disable containers by default.

I've never (knowingly) tried running TCG tests. IIUC, they are strictly
an opt-in test needing explicit 'make check-tcg', so any container usage
wouldn't be encountered by most contributors ?

Note, my objection wasn't that the containers are broken - it did
eventually work. Rather the issues I see were

 * Downloading and building containers as part of 'make' made
   the build insanely slow due to my limited 4G network connectivity.
   It took over 1 GB of downloads, which doesn't sound like much
   for those with reliably high speed internet, but was absolutely
   awful for me as my 4G was very degraded at the time.

 * Downloading and building the containers printed lots of
   verbose progress information that destroyed the progress
   output from meson when doing a parallel build

 * The containers being built were not even used by the build
   process, as I filtered the target list to only x86 and thus
   had no cause to build s390 / ppc64 firmware.

 * When the container rebuild failed, restarting seemed to use
   the downloaded image, that was previously considered stale,
   instead of trying the fresh rebuild again.

IOW, I'd like to see

 - Explicit opt-in at configure time for use of container
   downloads during 'make'
 - Tailor downloads wrt the target list configured
 - Supress the verbose output to preserve meson progress
   readability
 - Handle failure during container builds correctly


The problem of data downloads during 'make' arguably applies to
submodules too, but few submodules are needed when the distro
has provided the required deps in packages, and so those submodules
left are small and their download isn't noticably slow / large data
volumes.

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH v2 3/7] util: Introduce ThreadContext user-creatable object

[David Hildenbrand]

Thanks Markus!


I just tested again, and get the same result as you.  I figure my
previous test was with the complete series.

PATCH 5 appears to make it work.  Suggest to say something like "The
commit after next will make this work".


I'll phrase it like " We'll wire this up next to make it work."

[...]


So, when a thread is created, its affinity comes from its thread context
(if any).  When I later change the context's affinity, it does *not*
affect existing threads, only future ones.  Correct?


Yes, that's the current state.


Thanks!



I'm adding

"Note that the CPU affinity of previously created threads will not get 
adjusted."


and

"In general, the interface behaves like pthread_setaffinity_np(): host 
CPU numbers that are currently not available are ignored; only host CPU 
numbers that are impossible with the current kernel will fail. If the 
list of host CPU numbers does not include a single CPU that is 
available, setting the CPU affinity will fail."


--
Thanks,

David / dhildenb

Re: [PATCH v2 4/7] util: Add write-only "node-affinity" property for ThreadContext

[David Hildenbrand]

On 12.10.22 10:03, Markus Armbruster wrote:

Acked-by: Markus Armbruster


Thanks, I'll similarly adjust to use "host nodes" and "host CPUs".

--
Thanks,

David / dhildenb

[PATCH] target/i386: Switch back XFRM value

[Yang Zhong]

The previous patch wrongly replaced FEAT_XSAVE_XCR0_{LO|HI} with
FEAT_XSAVE_XSS_{LO|HI} in CPUID(EAX=12,ECX=1):ECX, which made SGX
enclave only supported SSE and x87 feature(xfrm=0x3).

Fixes: 301e90675c3f ("target/i386: Enable support for XSAVES based features")

Signed-off-by: Yang Zhong 
---
 target/i386/cpu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ad623d91e4..19aaed877b 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5584,8 +5584,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 } else {
 *eax &= env->features[FEAT_SGX_12_1_EAX];
 *ebx &= 0; /* ebx reserve */
-*ecx &= env->features[FEAT_XSAVE_XSS_LO];
-*edx &= env->features[FEAT_XSAVE_XSS_HI];
+*ecx &= env->features[FEAT_XSAVE_XCR0_LO];
+*edx &= env->features[FEAT_XSAVE_XCR0_HI];
 
 /* FP and SSE are always allowed regardless of XSAVE/XCR0. */
 *ecx |= XSTATE_FP_MASK | XSTATE_SSE_MASK;
-- 
2.30.2

[PATCH] tests/qtest/tpm: Clean up remainders of swtpm

[Thomas Huth]

After running "make check", there are remainders of the tpm
tests left in the /tmp directory, slowly filling it up.
Seems like "swtpm" leaves a ".lock" and a "tpm2-00.permall"
file behind, so that the g_rmdir() calls on the temporary
directories fail. Introduce a helper function to remove those
leftovers before doing the g_rmdir().

Signed-off-by: Thomas Huth 
---
 tests/qtest/tpm-util.h  |  1 +
 tests/qtest/tpm-crb-swtpm-test.c|  5 ++---
 tests/qtest/tpm-tis-device-swtpm-test.c |  5 ++---
 tests/qtest/tpm-tis-swtpm-test.c|  5 ++---
 tests/qtest/tpm-util.c  | 19 +++
 5 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/tests/qtest/tpm-util.h b/tests/qtest/tpm-util.h
index 3b97d69017..80720afac0 100644
--- a/tests/qtest/tpm-util.h
+++ b/tests/qtest/tpm-util.h
@@ -53,5 +53,6 @@ void tpm_util_migration_start_qemu(QTestState **src_qemu,
const char *machine_options);
 
 void tpm_util_wait_for_migration_complete(QTestState *who);
+void tpm_util_rmdir(const char *path);
 
 #endif /* TESTS_TPM_UTIL_H */
diff --git a/tests/qtest/tpm-crb-swtpm-test.c b/tests/qtest/tpm-crb-swtpm-test.c
index 55fdb5657d..40254f762f 100644
--- a/tests/qtest/tpm-crb-swtpm-test.c
+++ b/tests/qtest/tpm-crb-swtpm-test.c
@@ -13,7 +13,6 @@
  */
 
 #include "qemu/osdep.h"
-#include 
 
 #include "libqtest.h"
 #include "qemu/module.h"
@@ -62,9 +61,9 @@ int main(int argc, char **argv)
 tpm_crb_swtpm_migration_test);
 ret = g_test_run();
 
-g_rmdir(ts.dst_tpm_path);
+tpm_util_rmdir(ts.dst_tpm_path);
 g_free(ts.dst_tpm_path);
-g_rmdir(ts.src_tpm_path);
+tpm_util_rmdir(ts.src_tpm_path);
 g_free(ts.src_tpm_path);
 g_free(ts.uri);
 
diff --git a/tests/qtest/tpm-tis-device-swtpm-test.c 
b/tests/qtest/tpm-tis-device-swtpm-test.c
index 7b20035142..8c067fddd4 100644
--- a/tests/qtest/tpm-tis-device-swtpm-test.c
+++ b/tests/qtest/tpm-tis-device-swtpm-test.c
@@ -14,7 +14,6 @@
  */
 
 #include "qemu/osdep.h"
-#include 
 
 #include "libqtest.h"
 #include "qemu/module.h"
@@ -66,9 +65,9 @@ int main(int argc, char **argv)
 tpm_tis_swtpm_migration_test);
 ret = g_test_run();
 
-g_rmdir(ts.dst_tpm_path);
+tpm_util_rmdir(ts.dst_tpm_path);
 g_free(ts.dst_tpm_path);
-g_rmdir(ts.src_tpm_path);
+tpm_util_rmdir(ts.src_tpm_path);
 g_free(ts.src_tpm_path);
 g_free(ts.uri);
 
diff --git a/tests/qtest/tpm-tis-swtpm-test.c b/tests/qtest/tpm-tis-swtpm-test.c
index 90131cb3c4..11539c0a52 100644
--- a/tests/qtest/tpm-tis-swtpm-test.c
+++ b/tests/qtest/tpm-tis-swtpm-test.c
@@ -13,7 +13,6 @@
  */
 
 #include "qemu/osdep.h"
-#include 
 
 #include "libqtest.h"
 #include "qemu/module.h"
@@ -61,9 +60,9 @@ int main(int argc, char **argv)
 tpm_tis_swtpm_migration_test);
 ret = g_test_run();
 
-g_rmdir(ts.dst_tpm_path);
+tpm_util_rmdir(ts.dst_tpm_path);
 g_free(ts.dst_tpm_path);
-g_rmdir(ts.src_tpm_path);
+tpm_util_rmdir(ts.src_tpm_path);
 g_free(ts.src_tpm_path);
 g_free(ts.uri);
 
diff --git a/tests/qtest/tpm-util.c b/tests/qtest/tpm-util.c
index e0dc5da0af..a7efe2d0d2 100644
--- a/tests/qtest/tpm-util.c
+++ b/tests/qtest/tpm-util.c
@@ -13,6 +13,7 @@
  */
 
 #include "qemu/osdep.h"
+#include 
 
 #include "hw/acpi/tpm.h"
 #include "libqtest.h"
@@ -292,3 +293,21 @@ void tpm_util_migration_start_qemu(QTestState **src_qemu,
 g_free(src_qemu_args);
 g_free(dst_qemu_args);
 }
+
+/* Remove directory with remainders of swtpm */
+void tpm_util_rmdir(const char *path)
+{
+char *filename;
+int ret;
+
+filename = g_strdup_printf("%s/tpm2-00.permall", path);
+g_unlink(filename);
+g_free(filename);
+
+filename = g_strdup_printf("%s/.lock", path);
+g_unlink(filename);
+g_free(filename);
+
+ret = g_rmdir(path);
+g_assert(!ret);
+}
-- 
2.31.1

Re: [PATCH v2] gtk: Add show_menubar=on|off command line option.

[Markus Armbruster]

Cc'ing maintainers.  Doing that reduces the risk of maintainers missing
your patch.  You can use scripts/get_maintainer.pl to find them.

Bryce Mills  writes:

> The patch adds "show_menubar" command line option for GTK UI similar to
> "show_tabs". This option allows to hide menu bar initially, it still can
> be toggled by shortcut and other shortcuts still work.
>
> Signed-off-by: Bryce Mills 
> ---
>  qapi/ui.json|  5 -
>  qemu-options.hx |  3 +++
>  ui/gtk.c| 15 ++-
>  3 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 286c5731d1..0abba3e930 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1199,13 +1199,16 @@
>  #   interfaces (e.g. VGA and virtual console character devices)
>  #   by default.
>  #   Since 7.1
> +# @show-menubar: Display the main window menubar. Defaults to "on".
> +#Since 8.0
>  #
>  # Since: 2.12
>  ##
>  { 'struct'  : 'DisplayGTK',
>'data': { '*grab-on-hover' : 'bool',
>  '*zoom-to-fit'   : 'bool',
> -'*show-tabs' : 'bool'  } }
> +'*show-tabs' : 'bool',
> +'*show-menubar'  : 'bool'  } }
>  
>  ##
>  # @DisplayEGLHeadless:

QAPI schema
Acked-by: Markus Armbruster 

[...]

[PATCH] tests/unit/test-image-locking: Fix handling of temporary files

[Thomas Huth]

test-image-locking leaves some temporary files around - clean
them up. While we're at it, test-image-locking is a unit test,
so it should not use "qtest.*" for temporary file names. Give
them better names instead, so that it clear where the temporary
files come from.

Signed-off-by: Thomas Huth 
---
 tests/unit/test-image-locking.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/unit/test-image-locking.c b/tests/unit/test-image-locking.c
index a47299c247..2624cec6a0 100644
--- a/tests/unit/test-image-locking.c
+++ b/tests/unit/test-image-locking.c
@@ -79,7 +79,7 @@ static void test_image_locking_basic(void)
 g_autofree char *img_path = NULL;
 uint64_t perm, shared_perm;
 
-int fd = g_file_open_tmp("qtest.XX", &img_path, NULL);
+int fd = g_file_open_tmp("qemu-tst-img-lock.XX", &img_path, NULL);
 assert(fd >= 0);
 
 perm = BLK_PERM_WRITE | BLK_PERM_CONSISTENT_READ;
@@ -120,7 +120,7 @@ static void test_set_perm_abort(void)
 g_autofree char *img_path = NULL;
 uint64_t perm, shared_perm;
 int r;
-int fd = g_file_open_tmp("qtest.XX", &img_path, NULL);
+int fd = g_file_open_tmp("qemu-tst-img-lock.XX", &img_path, NULL);
 assert(fd >= 0);
 
 perm = BLK_PERM_WRITE | BLK_PERM_CONSISTENT_READ;
@@ -140,6 +140,8 @@ static void test_set_perm_abort(void)
 check_locked_bytes(fd, perm, ~shared_perm);
 blk_unref(blk1);
 blk_unref(blk2);
+close(fd);
+unlink(img_path);
 }
 
 int main(int argc, char **argv)
-- 
2.31.1

Re: [PULL 1/1] Revert "configure: build ROMs with container-based cross compilers"

[Paolo Bonzini]

On 10/12/22 10:14, Daniel P. Berrangé wrote:

Daniel, does building the TCG tests work for you? If not, I think we should
just disable containers by default.


I've never (knowingly) tried running TCG tests. IIUC, they are strictly
an opt-in test needing explicit 'make check-tcg', so any container usage
wouldn't be encountered by most contributors ?


Yeah, that is true.  But the problems below affect all container usage 
rather than just firmware builds, so they should be fixed there. 
configure is able to only run 'make check-tcg' for targets which have a 
compiler installed.



IOW, I'd like to see

  - Explicit opt-in at configure time for use of container
downloads during 'make'


This is what I'm proposing, and extending to all targets.


  - Tailor downloads wrt the target list configured


This is already done.


  - Suppress the verbose output to preserve meson progress
readability


That's in general a tradeoff with long-running tasks.  It's difficult to 
say which is better, for example "make check" also has a verbose output.


Paolo


  - Handle failure during container builds correctly


The problem of data downloads during 'make' arguably applies to
submodules too, but few submodules are needed when the distro
has provided the required deps in packages, and so those submodules
left are small and their download isn't noticably slow / large data
volumes.

Re: [PATCH] cirrus_vga: fix potential memory overflow

[Gerd Hoffmann]

On Thu, Sep 29, 2022 at 08:23:52PM +0800, luzhipeng wrote:
> From: lu zhipeng 
> 
> Signed-off-by: lu zhipeng 

> -copy_count = s->cirrus_srcptr_end - end_ptr;
> +copy_count = MIN(s->cirrus_srcptr_end - end_ptr, 
> CIRRUS_BLTBUFSIZE);

Added to patch queue.

thanks,
  Gerd

Re: [PATCH] ui/gtk-egl: egl context needs to be unbound in the end of gd_egl_switch

[Gerd Hoffmann]

On Wed, Sep 28, 2022 at 02:58:05PM -0700, Dongwon Kim wrote:
> A thread often fails to bind an egl context to itself after guest VM is
> rebooted because the context is still owned by another thread. It is not
> very clear what condition makes this happen but this can be prevented
> by unbinding the context from the thread in the end of gd_egl_switch.

Added to queue,

thanks,
  Gerd

Re: [PULL 1/1] Revert "configure: build ROMs with container-based cross compilers"

[Daniel P . Berrangé]

On Wed, Oct 12, 2022 at 10:59:54AM +0200, Paolo Bonzini wrote:
> On 10/12/22 10:14, Daniel P. Berrangé wrote:
> > > Daniel, does building the TCG tests work for you? If not, I think we 
> > > should
> > > just disable containers by default.
> > 
> > I've never (knowingly) tried running TCG tests. IIUC, they are strictly
> > an opt-in test needing explicit 'make check-tcg', so any container usage
> > wouldn't be encountered by most contributors ?
> 
> Yeah, that is true.  But the problems below affect all container usage
> rather than just firmware builds, so they should be fixed there. configure
> is able to only run 'make check-tcg' for targets which have a compiler
> installed.
> 
> > IOW, I'd like to see
> > 
> >   - Explicit opt-in at configure time for use of container
> > downloads during 'make'
> 
> This is what I'm proposing, and extending to all targets.

Ok, makes sense.

> >   - Tailor downloads wrt the target list configured
> 
> This is already done.

Where's the patch for that, I hadn't noticed it being posted yet ?

> >   - Suppress the verbose output to preserve meson progress
> > readability
> 
> That's in general a tradeoff with long-running tasks.  It's difficult to say
> which is better, for example "make check" also has a verbose output.

If 'make' was running with V=1, then also letting docker download be
verbose makes sense.

The plain 'make' though is intentionally quite terse, just giving a
list of files meson compiles. Right now, the output ends up looking
like this:

[2715/2945] Compiling C object tests/unit/test-xbzrle.p/test-xbzrle.c.o
[2716/2945] Linking target tests/unit/test-authz-pam
Copying blob bd159e379b3b skipped: already exists  
Copying blob fc8d65e34cd5 [>-] 9.3MiB / 
360.2MiB
Copying blob 13224e2971af [--] 1.1MiB / 
366.5MiB
[2720/2945] Linking target tests/unit/test-io-channel-tls
Copying blob bd159e379b3b skipped: already exists  
Copying blob fc8d65e34cd5 [>-] 9.4MiB / 
360.2MiB
Copying blob 13224e2971af [--] 1.2MiB / 
366.5MiB
[2724/2945] Linking target tests/unit/test-io-task
[2725/2945] Compiling C object 
tests/unit/test-util-sockets.p/test-util-sockets.c.o
[2726/2945] Compiling C object tests/unit/test-util-sockets.p/socket-helpers.c.o
[2727/2945] Linking target tests/unit/test-xbzrle
[2728/2945] Compiling C object tests/unit/test-base64.p/test-base64.c.o
[2729/2945] Linking target tests/unit/test-timed-average
[2730/2945] Compiling C object 
tests/unit/test-bufferiszero.p/test-bufferiszero.c.o
Copying blob bd159e379b3b skipped: already exists  
Copying blob 2a205c8a1d36 [>-] 4.4MiB / 
257.2MiB
Copying blob bd159e379b3b skipped: already exists  
Copying blob fc8d65e34cd5 [>-] 9.5MiB / 
360.2MiB
Copying blob 13224e2971af [--] 1.3MiB / 
366.5MiB
[2736/2945] Compiling C object tests/unit/test-yank.p/test-yank.c.o
[2737/2945] Compiling C object 
tests/unit/test-util-filemonitor.p/test-util-filemonitor.c.o
Copying blob bd159e379b3b skipped: already exists  
Copying blob fc8d65e34cd5 [>-] 9.6MiB / 
360.2MiB
Copying blob 13224e2971af [--] 1.3MiB / 
366.5MiB


which I feel is quite unplesant, especially when you then get
multiple parallel docker downloads concurrently refreshing the
screen and overwriting each others' output.

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[PATCH] qgraph: implement stack as a linked list

[Paolo Bonzini]

The stack used to visit the graph is implemented as a fixed-size array,
and the array is sized according to the maximum anticipated length of
a path on the graph.  However, the worst case for a depth-first search
is to push all nodes on the graph, and in fact stack overflows have
been observed in the past depending on the ordering of the constructors.

To fix the problem once and for all, use a QSLIST instead of the array,
allocating QOSStackElements from the heap.

Reviewed-by: Richard Henderson 
Signed-off-by: Paolo Bonzini 
---
 tests/qtest/libqos/qgraph.c | 35 +++
 1 file changed, 11 insertions(+), 24 deletions(-)

diff --git a/tests/qtest/libqos/qgraph.c b/tests/qtest/libqos/qgraph.c
index 0a2dddfafa..ff9d389f12 100644
--- a/tests/qtest/libqos/qgraph.c
+++ b/tests/qtest/libqos/qgraph.c
@@ -52,6 +52,7 @@ struct QOSStackElement {
 QOSStackElement *parent;
 QOSGraphEdge *parent_edge;
 int length;
+QSLIST_ENTRY(QOSStackElement) next;
 };
 
 /* Each enty in these hash table will consist of  pair. */
@@ -59,8 +60,7 @@ static GHashTable *edge_table;
 static GHashTable *node_table;
 
 /* stack used by the DFS algorithm to store the path from machine to test */
-static QOSStackElement qos_node_stack[QOS_PATH_MAX_ELEMENT_SIZE];
-static int qos_node_tos;
+static QSLIST_HEAD(, QOSStackElement) qos_node_stack;
 
 /**
  * add_edge(): creates an edge of type @type
@@ -325,40 +325,27 @@ static void qos_print_cb(QOSGraphNode *path, int length)
 static void qos_push(QOSGraphNode *el, QOSStackElement *parent,
  QOSGraphEdge *e)
 {
+QOSStackElement *elem = g_new(QOSStackElement, 1);
 int len = 0; /* root is not counted */
-if (qos_node_tos == QOS_PATH_MAX_ELEMENT_SIZE) {
-g_printerr("QOSStack: full stack, cannot push");
-abort();
-}
-
 if (parent) {
 len = parent->length + 1;
 }
-qos_node_stack[qos_node_tos++] = (QOSStackElement) {
+*elem = (QOSStackElement) {
 .node = el,
 .parent = parent,
 .parent_edge = e,
 .length = len,
 };
-}
-
-/* qos_tos(): returns the top of stack, without popping */
-static QOSStackElement *qos_tos(void)
-{
-return &qos_node_stack[qos_node_tos - 1];
+QSLIST_INSERT_HEAD(qos_node_stack, elem, next);
 }
 
 /* qos_pop(): pops an element from the tos, setting it unvisited*/
-static QOSStackElement *qos_pop(void)
+static void qos_pop(void)
 {
-if (qos_node_tos == 0) {
-g_printerr("QOSStack: empty stack, cannot pop");
-abort();
-}
-QOSStackElement *e = qos_tos();
+QOSStackElement *e = QSLIST_FIRST(qos_node_stack);
 e->node->visited = false;
-qos_node_tos--;
-return e;
+QSLIST_REMOVE_HEAD(qos_node_stack, next);
+g_free(e);
 }
 
 /**
@@ -400,8 +387,8 @@ static void qos_traverse_graph(QOSGraphNode *root, 
QOSTestCallback callback)
 
 qos_push(root, NULL, NULL);
 
-while (qos_node_tos > 0) {
-s_el = qos_tos();
+while (!QSLIST_EMPTY(qos_node_stack)) {
+s_el = QSLIST_HEAD(qos_node_stack);
 v = s_el->node;
 if (v->visited) {
 qos_pop();
-- 
2.37.3

[PATCH] build: disable container-based cross compilers by default

[Paolo Bonzini]

Container-based cross compilers have some issues which were overlooked
when they were only used for TCG tests, but are more visible since
firmware builds try to use them:

- Downloading and building containers as part of make adds a
  very long task to the build, unless you are on a fast network.
  Container images can be hundreds of MBs.

- Verbose progress information from the container builds
  is printed on stderr and messes up other output from
  make/ninja

- There seem to be some rough edges around failure too.

So, make container builds opt-in.

Reported-by: Daniel P. Berrangé 
Signed-off-by: Paolo Bonzini 
---
 .gitlab-ci.d/buildtest.yml   | 16 
 .gitlab-ci.d/crossbuilds.yml |  2 +-
 .../custom-runners/ubuntu-20.04-s390x.yml|  2 +-
 .../custom-runners/ubuntu-22.04-aarch64.yml  |  2 +-
 configure|  4 ++--
 5 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 6c05c46397..41742ae962 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -263,7 +263,7 @@ build-user:
 job: amd64-debian-user-cross-container
   variables:
 IMAGE: debian-all-test-cross
-CONFIGURE_ARGS: --disable-tools --disable-system
+CONFIGURE_ARGS: --disable-tools --disable-system --enable-containers
 MAKE_CHECK_ARGS: check-tcg
 
 build-user-static:
@@ -272,7 +272,7 @@ build-user-static:
 job: amd64-debian-user-cross-container
   variables:
 IMAGE: debian-all-test-cross
-CONFIGURE_ARGS: --disable-tools --disable-system --static
+CONFIGURE_ARGS: --disable-tools --disable-system --static 
--enable-containers
 MAKE_CHECK_ARGS: check-tcg
 
 # Because the hexagon cross-compiler takes so long to build we don't rely
@@ -286,7 +286,7 @@ build-user-hexagon:
   variables:
 IMAGE: debian-hexagon-cross
 TARGETS: hexagon-linux-user
-CONFIGURE_ARGS: --disable-tools --disable-docs --enable-debug-tcg
+CONFIGURE_ARGS: --disable-tools --disable-docs --enable-debug-tcg 
--enable-containers
 MAKE_CHECK_ARGS: check-tcg
 
 # Only build the softmmu targets we have check-tcg tests for
@@ -296,7 +296,7 @@ build-some-softmmu:
 job: amd64-debian-user-cross-container
   variables:
 IMAGE: debian-all-test-cross
-CONFIGURE_ARGS: --disable-tools --enable-debug
+CONFIGURE_ARGS: --disable-tools --enable-debug --enable-containers
 TARGETS: xtensa-softmmu arm-softmmu aarch64-softmmu alpha-softmmu
 MAKE_CHECK_ARGS: check-tcg
 
@@ -307,7 +307,7 @@ build-tricore-softmmu:
 job: tricore-debian-cross-container
   variables:
 IMAGE: debian-tricore-cross
-CONFIGURE_ARGS: --disable-tools --disable-fdt --enable-debug
+CONFIGURE_ARGS: --disable-tools --disable-fdt --enable-debug 
--enable-containers
 TARGETS: tricore-softmmu
 MAKE_CHECK_ARGS: check-tcg
 
@@ -317,7 +317,7 @@ clang-system:
 job: amd64-fedora-container
   variables:
 IMAGE: fedora
-CONFIGURE_ARGS: --cc=clang --cxx=clang++
+CONFIGURE_ARGS: --enable-containers --cc=clang --cxx=clang++
   --extra-cflags=-fsanitize=undefined 
--extra-cflags=-fno-sanitize-recover=undefined
 TARGETS: alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu
   ppc-softmmu s390x-softmmu
@@ -329,7 +329,7 @@ clang-user:
 job: amd64-debian-user-cross-container
   variables:
 IMAGE: debian-all-test-cross
-CONFIGURE_ARGS: --cc=clang --cxx=clang++ --disable-system
+CONFIGURE_ARGS: --enable-containers --cc=clang --cxx=clang++ 
--disable-system
   
--target-list-exclude=microblazeel-linux-user,aarch64_be-linux-user,i386-linux-user,m68k-linux-user,mipsn32el-linux-user,xtensaeb-linux-user
   --extra-cflags=-fsanitize=undefined 
--extra-cflags=-fno-sanitize-recover=undefined
 MAKE_CHECK_ARGS: check-unit check-tcg
@@ -523,7 +523,7 @@ build-tci:
 - TARGETS="aarch64 alpha arm hppa m68k microblaze ppc64 s390x x86_64"
 - mkdir build
 - cd build
-- ../configure --enable-tcg-interpreter
+- ../configure --enable-containers --enable-tcg-interpreter
 --target-list="$(for tg in $TARGETS; do echo -n ${tg}'-softmmu '; 
done)" || { cat config.log meson-logs/meson-log.txt && exit 1; }
 - make -j"$JOBS"
 - make tests/qtest/boot-serial-test tests/qtest/cdrom-test 
tests/qtest/pxe-test
diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index c4cd96433d..b2519ff2e0 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -67,7 +67,7 @@ cross-i386-tci:
   variables:
 IMAGE: fedora-i386-cross
 ACCEL: tcg-interpreter
-EXTRA_CONFIGURE_OPTS: 
--target-list=i386-softmmu,i386-linux-user,aarch64-softmmu,aarch64-linux-user,ppc-softmmu,ppc-linux-user
+EXTRA_CONFIGURE_OPTS: 
--target-list=i386-softmmu,i386-linux-user,aarch64-softmmu,aarch64-linux-user,ppc-softmmu,ppc-linux-user
 --enable-containers
 MAKE_CHECK_ARGS: check che

[PATCH 0/2] tcg/loongarch64: add neg tcg_op and direct jump support

[Qi Hu]

Hi,

This patch series add neg tcg_op and direct jump support into loongarch tcg 
backend.


Qi Hu (2):
  tcg/loongarch64: Implement INDEX_op_neg_i{32,64}
  tcg/loongarch64: Add direct jump support

 tcg/loongarch64/tcg-insn-defs.c.inc |  3 ++
 tcg/loongarch64/tcg-target.c.inc| 58 ++---
 tcg/loongarch64/tcg-target.h|  6 +--
 3 files changed, 59 insertions(+), 8 deletions(-)

-- 
2.37.3

[PATCH 2/2] tcg/loongarch64: Add direct jump support

[Qi Hu]

Similar to the ARM64, LoongArch has PC-relative instructions such as
PCADDU18I. These instructions can be used to support direct jump for
LoongArch. Additionally, if instruction "B offset" can cover the target
address, "tb_target_set_jmp_target" will only patch the "B offset".

Signed-off-by: Qi Hu 
---
 tcg/loongarch64/tcg-insn-defs.c.inc |  3 ++
 tcg/loongarch64/tcg-target.c.inc| 49 ++---
 tcg/loongarch64/tcg-target.h|  2 +-
 3 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/tcg/loongarch64/tcg-insn-defs.c.inc 
b/tcg/loongarch64/tcg-insn-defs.c.inc
index d162571856..f5869c6bb1 100644
--- a/tcg/loongarch64/tcg-insn-defs.c.inc
+++ b/tcg/loongarch64/tcg-insn-defs.c.inc
@@ -112,6 +112,9 @@ typedef enum {
 OPC_BLE = 0x6400,
 OPC_BGTU = 0x6800,
 OPC_BLEU = 0x6c00,
+/* pseudo-instruction */
+NOP = 0x0340,
+
 } LoongArchInsn;
 
 static int32_t __attribute__((unused))
diff --git a/tcg/loongarch64/tcg-target.c.inc b/tcg/loongarch64/tcg-target.c.inc
index f5a214a17f..3a7b1df081 100644
--- a/tcg/loongarch64/tcg-target.c.inc
+++ b/tcg/loongarch64/tcg-target.c.inc
@@ -1058,11 +1058,24 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
 break;
 
 case INDEX_op_goto_tb:
-assert(s->tb_jmp_insn_offset == 0);
-/* indirect jump method */
-tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_TMP0, TCG_REG_ZERO,
-   (uintptr_t)(s->tb_jmp_target_addr + a0));
-tcg_out_opc_jirl(s, TCG_REG_ZERO, TCG_REG_TMP0, 0);
+if (s->tb_jmp_insn_offset != NULL) {
+/* TCG_TARGET_HAS_direct_jump */
+/* Ensure that PCADD+JIRL are 8-byte aligned so that an atomic
+   write can be used to patch the target address. */
+if ((uintptr_t)s->code_ptr & 7) {
+tcg_out32(s, NOP);
+}
+s->tb_jmp_insn_offset[a0] = tcg_current_code_size(s);
+/* actual branch destination will be patched by
+   tb_target_set_jmp_target later. */
+tcg_out_opc_pcaddu18i(s, TCG_REG_TMP0, 0);
+tcg_out_opc_jirl(s, TCG_REG_ZERO, TCG_REG_TMP0, 0);
+} else {
+/* !TCG_TARGET_HAS_direct_jump */
+tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_TMP0, TCG_REG_ZERO,
+(uintptr_t)(s->tb_jmp_target_addr + a0));
+tcg_out_opc_jirl(s, TCG_REG_ZERO, TCG_REG_TMP0, 0);
+}
 set_jmp_reset_offset(s, a0);
 break;
 
@@ -1708,6 +1721,32 @@ static void tcg_target_init(TCGContext *s)
 tcg_regset_set_reg(s->reserved_regs, TCG_REG_RESERVED);
 }
 
+void tb_target_set_jmp_target(uintptr_t tc_ptr, uintptr_t jmp_rx,
+  uintptr_t jmp_rw, uintptr_t addr)
+{
+tcg_insn_unit i1, i2;
+
+ptrdiff_t offset = addr - jmp_rx;
+
+if (offset == sextreg(offset, 0, 28)) {
+i1 = OPC_B | ((offset >> 18) & 0x3ff) | ((offset << 8) & 0x3fffc00);
+i2 = NOP;
+} else {
+offset >>= 2;
+
+ptrdiff_t upper, lower;
+upper = ((offset + (1 << 15)) >> 16) & 0xf;
+lower = (offset & 0x);
+/* patch pcaddu18i */
+i1 = OPC_PCADDU18I | upper << 5 | TCG_REG_T0;
+/* patch jirl */
+i2 = OPC_JIRL | lower << 10 | TCG_REG_T0 << 5;
+}
+uint64_t pair = (uint64_t)i2 << 32 | i1;
+qatomic_set((uint64_t *)jmp_rw, pair);
+flush_idcache_range(jmp_rx, jmp_rw, 8);
+}
+
 typedef struct {
 DebugFrameHeader h;
 uint8_t fde_def_cfa[4];
diff --git a/tcg/loongarch64/tcg-target.h b/tcg/loongarch64/tcg-target.h
index 67380b2432..0e552731f5 100644
--- a/tcg/loongarch64/tcg-target.h
+++ b/tcg/loongarch64/tcg-target.h
@@ -123,7 +123,7 @@ typedef enum {
 #define TCG_TARGET_HAS_clz_i32  1
 #define TCG_TARGET_HAS_ctz_i32  1
 #define TCG_TARGET_HAS_ctpop_i320
-#define TCG_TARGET_HAS_direct_jump  0
+#define TCG_TARGET_HAS_direct_jump  1
 #define TCG_TARGET_HAS_brcond2  0
 #define TCG_TARGET_HAS_setcond2 0
 #define TCG_TARGET_HAS_qemu_st8_i32 0
-- 
2.37.3

[PATCH 1/2] tcg/loongarch64: Implement INDEX_op_neg_i{32,64}

[Qi Hu]

Signed-off-by: Qi Hu 
---
 tcg/loongarch64/tcg-target.c.inc | 9 +
 tcg/loongarch64/tcg-target.h | 4 ++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/tcg/loongarch64/tcg-target.c.inc b/tcg/loongarch64/tcg-target.c.inc
index a3debf6da7..f5a214a17f 100644
--- a/tcg/loongarch64/tcg-target.c.inc
+++ b/tcg/loongarch64/tcg-target.c.inc
@@ -1125,6 +1125,13 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
 tcg_out_opc_nor(s, a0, a1, TCG_REG_ZERO);
 break;
 
+case INDEX_op_neg_i32:
+tcg_out_opc_sub_w(s, a0, TCG_REG_ZERO, a1);
+break;
+case INDEX_op_neg_i64:
+tcg_out_opc_sub_d(s, a0, TCG_REG_ZERO, a1);
+break;
+
 case INDEX_op_nor_i32:
 case INDEX_op_nor_i64:
 if (c2) {
@@ -1503,6 +1510,8 @@ static TCGConstraintSetIndex tcg_target_op_def(TCGOpcode 
op)
 case INDEX_op_ext_i32_i64:
 case INDEX_op_not_i32:
 case INDEX_op_not_i64:
+case INDEX_op_neg_i32:
+case INDEX_op_neg_i64:
 case INDEX_op_extract_i32:
 case INDEX_op_extract_i64:
 case INDEX_op_bswap16_i32:
diff --git a/tcg/loongarch64/tcg-target.h b/tcg/loongarch64/tcg-target.h
index d58a6162f2..67380b2432 100644
--- a/tcg/loongarch64/tcg-target.h
+++ b/tcg/loongarch64/tcg-target.h
@@ -114,7 +114,7 @@ typedef enum {
 #define TCG_TARGET_HAS_bswap16_i32  1
 #define TCG_TARGET_HAS_bswap32_i32  1
 #define TCG_TARGET_HAS_not_i32  1
-#define TCG_TARGET_HAS_neg_i32  0
+#define TCG_TARGET_HAS_neg_i32  1
 #define TCG_TARGET_HAS_andc_i32 1
 #define TCG_TARGET_HAS_orc_i32  1
 #define TCG_TARGET_HAS_eqv_i32  0
@@ -150,7 +150,7 @@ typedef enum {
 #define TCG_TARGET_HAS_bswap32_i64  1
 #define TCG_TARGET_HAS_bswap64_i64  1
 #define TCG_TARGET_HAS_not_i64  1
-#define TCG_TARGET_HAS_neg_i64  0
+#define TCG_TARGET_HAS_neg_i64  1
 #define TCG_TARGET_HAS_andc_i64 1
 #define TCG_TARGET_HAS_orc_i64  1
 #define TCG_TARGET_HAS_eqv_i64  0
-- 
2.37.3

[PATCH] tests/qtest/cxl-test: Remove temporary directories after testing

[Thomas Huth]

The cxl-test leaves some temporary directories behind. Let's
clean them up now!

Signed-off-by: Thomas Huth 
---
 tests/qtest/cxl-test.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/qtest/cxl-test.c b/tests/qtest/cxl-test.c
index cbe0fb549b..61f25a72b6 100644
--- a/tests/qtest/cxl-test.c
+++ b/tests/qtest/cxl-test.c
@@ -101,6 +101,7 @@ static void cxl_t3d(void)
 
 qtest_start(cmdline->str);
 qtest_end();
+rmdir(tmpfs);
 }
 
 static void cxl_1pxb_2rp_2t3d(void)
@@ -115,6 +116,7 @@ static void cxl_1pxb_2rp_2t3d(void)
 
 qtest_start(cmdline->str);
 qtest_end();
+rmdir(tmpfs);
 }
 
 static void cxl_2pxb_4rp_4t3d(void)
@@ -130,6 +132,7 @@ static void cxl_2pxb_4rp_4t3d(void)
 
 qtest_start(cmdline->str);
 qtest_end();
+rmdir(tmpfs);
 }
 #endif /* CONFIG_POSIX */
 
-- 
2.31.1

Re: [PATCH] tests/unit/test-image-locking: Fix handling of temporary files

[Marc-André Lureau]

Hi

On Wed, Oct 12, 2022 at 1:03 PM Thomas Huth  wrote:

> test-image-locking leaves some temporary files around - clean
> them up. While we're at it, test-image-locking is a unit test,
> so it should not use "qtest.*" for temporary file names. Give
> them better names instead, so that it clear where the temporary
> files come from.
>
> Signed-off-by: Thomas Huth 
> ---
>  tests/unit/test-image-locking.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tests/unit/test-image-locking.c
> b/tests/unit/test-image-locking.c
> index a47299c247..2624cec6a0 100644
> --- a/tests/unit/test-image-locking.c
> +++ b/tests/unit/test-image-locking.c
> @@ -79,7 +79,7 @@ static void test_image_locking_basic(void)
>  g_autofree char *img_path = NULL;
>  uint64_t perm, shared_perm;
>
> -int fd = g_file_open_tmp("qtest.XX", &img_path, NULL);
> +int fd = g_file_open_tmp("qemu-tst-img-lock.XX", &img_path, NULL);
>  assert(fd >= 0);
>
>  perm = BLK_PERM_WRITE | BLK_PERM_CONSISTENT_READ;
> @@ -120,7 +120,7 @@ static void test_set_perm_abort(void)
>  g_autofree char *img_path = NULL;
>  uint64_t perm, shared_perm;
>  int r;
> -int fd = g_file_open_tmp("qtest.XX", &img_path, NULL);
> +int fd = g_file_open_tmp("qemu-tst-img-lock.XX", &img_path, NULL);
>  assert(fd >= 0);
>
>  perm = BLK_PERM_WRITE | BLK_PERM_CONSISTENT_READ;
> @@ -140,6 +140,8 @@ static void test_set_perm_abort(void)
>  check_locked_bytes(fd, perm, ~shared_perm);
>  blk_unref(blk1);
>  blk_unref(blk2);
> +close(fd);
> +unlink(img_path);
>

Perhaps we should use g_unlink() instead for better portability? although
this is pre-existing.

otherwise, lgtm
Reviewed-by: Marc-André Lureau 


-- 
Marc-André Lureau

Re: [PATCH 1/2] tcg/loongarch64: Implement INDEX_op_neg_i{32,64}

[WANG Xuerui]

Hi,

On 2022/10/12 17:13, Qi Hu wrote:

Signed-off-by: Qi Hu 
---
  tcg/loongarch64/tcg-target.c.inc | 9 +
  tcg/loongarch64/tcg-target.h | 4 ++--
  2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/tcg/loongarch64/tcg-target.c.inc b/tcg/loongarch64/tcg-target.c.inc
index a3debf6da7..f5a214a17f 100644
--- a/tcg/loongarch64/tcg-target.c.inc
+++ b/tcg/loongarch64/tcg-target.c.inc
@@ -1125,6 +1125,13 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
  tcg_out_opc_nor(s, a0, a1, TCG_REG_ZERO);
  break;
  
+case INDEX_op_neg_i32:

+tcg_out_opc_sub_w(s, a0, TCG_REG_ZERO, a1);
+break;
+case INDEX_op_neg_i64:
+tcg_out_opc_sub_d(s, a0, TCG_REG_ZERO, a1);
+break;
+
  case INDEX_op_nor_i32:
  case INDEX_op_nor_i64:
  if (c2) {
@@ -1503,6 +1510,8 @@ static TCGConstraintSetIndex tcg_target_op_def(TCGOpcode 
op)
  case INDEX_op_ext_i32_i64:
  case INDEX_op_not_i32:
  case INDEX_op_not_i64:
+case INDEX_op_neg_i32:
+case INDEX_op_neg_i64:
  case INDEX_op_extract_i32:
  case INDEX_op_extract_i64:
  case INDEX_op_bswap16_i32:
diff --git a/tcg/loongarch64/tcg-target.h b/tcg/loongarch64/tcg-target.h
index d58a6162f2..67380b2432 100644
--- a/tcg/loongarch64/tcg-target.h
+++ b/tcg/loongarch64/tcg-target.h
@@ -114,7 +114,7 @@ typedef enum {
  #define TCG_TARGET_HAS_bswap16_i32  1
  #define TCG_TARGET_HAS_bswap32_i32  1
  #define TCG_TARGET_HAS_not_i32  1
-#define TCG_TARGET_HAS_neg_i32  0
+#define TCG_TARGET_HAS_neg_i32  1
  #define TCG_TARGET_HAS_andc_i32 1
  #define TCG_TARGET_HAS_orc_i32  1
  #define TCG_TARGET_HAS_eqv_i32  0
@@ -150,7 +150,7 @@ typedef enum {
  #define TCG_TARGET_HAS_bswap32_i64  1
  #define TCG_TARGET_HAS_bswap64_i64  1
  #define TCG_TARGET_HAS_not_i64  1
-#define TCG_TARGET_HAS_neg_i64  0
+#define TCG_TARGET_HAS_neg_i64  1
  #define TCG_TARGET_HAS_andc_i64 1
  #define TCG_TARGET_HAS_orc_i64  1
  #define TCG_TARGET_HAS_eqv_i64  0
The whole change is not necessary, if target doesn't have neg then the 
target-independent logic already makes sure a sub with the same 
semantics is emitted. This is explained in the commit message of that 
commit introducing add/sub support.

Re: [PATCH] tests/unit/test-image-locking: Fix handling of temporary files

[Thomas Huth]

On 12/10/2022 11.21, Marc-André Lureau wrote:

Hi

On Wed, Oct 12, 2022 at 1:03 PM Thomas Huth > wrote:


test-image-locking leaves some temporary files around - clean
them up. While we're at it, test-image-locking is a unit test,
so it should not use "qtest.*" for temporary file names. Give
them better names instead, so that it clear where the temporary
files come from.

Signed-off-by: Thomas Huth mailto:th...@redhat.com>>
---

[...]

@@ -140,6 +140,8 @@ static void test_set_perm_abort(void)
      check_locked_bytes(fd, perm, ~shared_perm);
      blk_unref(blk1);
      blk_unref(blk2);
+    close(fd);
+    unlink(img_path);


Perhaps we should use g_unlink() instead for better portability? although 
this is pre-existing.


I thought about that, too, but apparently you have to include an additional 
header file (gstdio.h) to get the prototype - so it seems to be more effort 
for no real gain (unless you want to use non-ASCII characters in the 
filename - which we are not doing here).


 Thomas

Re: [PATCH] build: disable container-based cross compilers by default

[Daniel P . Berrangé]

On Wed, Oct 12, 2022 at 11:08:55AM +0200, Paolo Bonzini wrote:
> Container-based cross compilers have some issues which were overlooked
> when they were only used for TCG tests, but are more visible since
> firmware builds try to use them:
> 
> - Downloading and building containers as part of make adds a
>   very long task to the build, unless you are on a fast network.
>   Container images can be hundreds of MBs.
> 
> - Verbose progress information from the container builds
>   is printed on stderr and messes up other output from
>   make/ninja
> 
> - There seem to be some rough edges around failure too.
> 
> So, make container builds opt-in.
> 
> Reported-by: Daniel P. Berrangé 
> Signed-off-by: Paolo Bonzini 
> ---
>  .gitlab-ci.d/buildtest.yml   | 16 
>  .gitlab-ci.d/crossbuilds.yml |  2 +-
>  .../custom-runners/ubuntu-20.04-s390x.yml|  2 +-
>  .../custom-runners/ubuntu-22.04-aarch64.yml  |  2 +-
>  configure|  4 ++--
>  5 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> index 6c05c46397..41742ae962 100644
> --- a/.gitlab-ci.d/buildtest.yml
> +++ b/.gitlab-ci.d/buildtest.yml
> @@ -263,7 +263,7 @@ build-user:
>  job: amd64-debian-user-cross-container
>variables:
>  IMAGE: debian-all-test-cross
> -CONFIGURE_ARGS: --disable-tools --disable-system
> +CONFIGURE_ARGS: --disable-tools --disable-system --enable-containers
>  MAKE_CHECK_ARGS: check-tcg

Are you sure these jobs wer using containers in the first place ?

A standard gitlab CI environment isn't able to use normal docker,
and the build jobs aren't configured with the docker-in-docker
service. So I'd be surprised if any were using the container
logic.

I guess we auto-detect if it works, so silently skip them, but
its probably misleading to add --enable-containers to an env
we don't expect to use them.


> diff --git a/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml 
> b/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml
> index 0c835939db..24bca3f995 100644
> --- a/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml
> +++ b/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml
> @@ -16,7 +16,7 @@ ubuntu-20.04-s390x-all-linux-static:
>   # --disable-glusterfs is needed because there's no static version of those 
> libs in distro supplied packages
>   - mkdir build
>   - cd build
> - - ../configure --enable-debug --static --disable-system --disable-glusterfs 
> --disable-libssh
> + - ../configure --enable-debug --static --disable-system --disable-glusterfs 
> --disable-libssh --enable-containers
> || { cat config.log meson-logs/meson-log.txt; exit 1; }
>   - make --output-sync -j`nproc`
>   - make --output-sync -j`nproc` check V=1
> diff --git a/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml 
> b/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml
> index ce0b18af6f..db0c919fab 100644
> --- a/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml
> +++ b/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml
> @@ -16,7 +16,7 @@ ubuntu-22.04-aarch64-all-linux-static:
>   - cd build
>   # Disable -static-pie due to build error with system libc:
>   # https://bugs.launchpad.net/ubuntu/+source/glibc/+bug/1987438
> - - ../configure --enable-debug --static --disable-system --disable-pie
> + - ../configure --enable-debug --static --disable-system --disable-pie 
> --enable-containers
> || { cat config.log meson-logs/meson-log.txt; exit 1; }
>   - make --output-sync -j`nproc --ignore=40`
>   - make --output-sync -j`nproc --ignore=40` check V=1


These changes are likely ok, as the custom runners are bare metal
so can use containers normally, provided docker/podman is installed.

> diff --git a/configure b/configure
> index baa69189f0..6fa158a0d4 100755
> --- a/configure
> +++ b/configure
> @@ -227,7 +227,7 @@ cross_prefix=""
>  host_cc="cc"
>  stack_protector=""
>  safe_stack=""
> -use_containers="yes"
> +use_containers="no"
>  gdb_bin=$(command -v "gdb-multiarch" || command -v "gdb")
>  
>  if test -e "$source_path/.git"
> @@ -1034,7 +1034,7 @@ Advanced options (experts only):
> ucontext, sigaltstack, windows
>--enable-plugins
> enable plugins via shared library loading
> -  --disable-containers don't use containers for cross-building
> +  --enable-containers  use containers for cross-building
>--gdb=GDB-path   gdb to use for gdbstub tests [$gdb_bin]
>  EOF
>meson_options_help
> -- 
> 2.37.3
> 

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH] target/i386: Switch back XFRM value

[Huang, Kai]

On Wed, 2022-10-12 at 04:26 -0400, Yang Zhong wrote:
> The previous patch wrongly replaced FEAT_XSAVE_XCR0_{LO|HI} with
> FEAT_XSAVE_XSS_{LO|HI} in CPUID(EAX=12,ECX=1):ECX, which made SGX
^

Nit: both ECX and EDX are wrongly set, but not only ECX.

> enclave only supported SSE and x87 feature(xfrm=0x3).

Is this true?  Perhaps I am missing something, but it seems env-
>features[FEAT_XSAVE_XCR0_LO] only includes LBR bit, which is bit 15.

/* Calculate XSAVE components based on the configured CPU feature flags */
static void x86_cpu_enable_xsave_components(X86CPU *cpu)
{
...
env->features[FEAT_XSAVE_XSS_LO] = mask & CPUID_XSTATE_XSS_MASK;
...
}

/* CPUID feature bits available in XSS */
#define CPUID_XSTATE_XSS_MASK(XSTATE_ARCH_LBR_MASK)

> 
> Fixes: 301e90675c3f ("target/i386: Enable support for XSAVES based features")
> 
> Signed-off-by: Yang Zhong 
> ---
>  target/i386/cpu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index ad623d91e4..19aaed877b 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -5584,8 +5584,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> uint32_t count,
>  } else {
>  *eax &= env->features[FEAT_SGX_12_1_EAX];
>  *ebx &= 0; /* ebx reserve */
> -*ecx &= env->features[FEAT_XSAVE_XSS_LO];
> -*edx &= env->features[FEAT_XSAVE_XSS_HI];
> +*ecx &= env->features[FEAT_XSAVE_XCR0_LO];
> +*edx &= env->features[FEAT_XSAVE_XCR0_HI];
>  
>  /* FP and SSE are always allowed regardless of XSAVE/XCR0. */
>  *ecx |= XSTATE_FP_MASK | XSTATE_SSE_MASK;

The code looks good:

Reviewed-by: Kai Huang 

Re: [PATCH 00/20] tests/9p: introduce declarative function calls

[Christian Schoenebeck]

On Dienstag, 4. Oktober 2022 22:56:44 CEST Christian Schoenebeck wrote:
> This series converts relevant 9p (test) client functions to use named
> function arguments. For instance
> 
> do_walk_expect_error(v9p, "non-existent", ENOENT);
> 
> becomes
> 
> twalk({
> .client = v9p, .path = "non-existent", .expectErr = ENOENT
> });
> 
> The intention is to make the actual 9p test code more readable, and easier
> to maintain on the long-term.
> 
> Not only makes it clear what a literal passed to a function is supposed to
> do, it also makes the order and selection of arguments very liberal, and
> allows to merge multiple, similar functions into one single function.
> 
> This is basically just refactoring, it does not change behaviour.

Too massive for review?

If so, then I'll probably just go ahead and prepare a PR early next week with 
this queued as well. It's just test code refactoring, so I am quite painless 
about these changes.

Best regards,
Christian Schoenebeck

> 
> PREREQUISITES
> =
> 
> This series requires the following additional patch to work correctly:
> 
> https://lore.kernel.org/all/e1odrya-0004fv...@lizzy.crudebyte.com/
> https://github.com/cschoenebeck/qemu/commit/23d01367fc7a4f27be323ed6d195c527
> bec9ede1
> 
> Christian Schoenebeck (20):
>   tests/9p: merge *walk*() functions
>   tests/9p: simplify callers of twalk()
>   tests/9p: merge v9fs_tversion() and do_version()
>   tests/9p: merge v9fs_tattach(), do_attach(), do_attach_rqid()
>   tests/9p: simplify callers of tattach()
>   tests/9p: convert v9fs_tgetattr() to declarative arguments
>   tests/9p: simplify callers of tgetattr()
>   tests/9p: convert v9fs_treaddir() to declarative arguments
>   tests/9p: simplify callers of treaddir()
>   tests/9p: convert v9fs_tlopen() to declarative arguments
>   tests/9p: simplify callers of tlopen()
>   tests/9p: convert v9fs_twrite() to declarative arguments
>   tests/9p: simplify callers of twrite()
>   tests/9p: convert v9fs_tflush() to declarative arguments
>   tests/9p: merge v9fs_tmkdir() and do_mkdir()
>   tests/9p: merge v9fs_tlcreate() and do_lcreate()
>   tests/9p: merge v9fs_tsymlink() and do_symlink()
>   tests/9p: merge v9fs_tlink() and do_hardlink()
>   tests/9p: merge v9fs_tunlinkat() and do_unlinkat()
>   tests/9p: remove unnecessary g_strdup() calls
> 
>  tests/qtest/libqos/virtio-9p-client.c | 569 +-
>  tests/qtest/libqos/virtio-9p-client.h | 408 --
>  tests/qtest/virtio-9p-test.c  | 529 
>  3 files changed, 1031 insertions(+), 475 deletions(-)

RE: [PATCH v2 0/2] vhost-user: Support vhost_dev_start

[Yajun Wu]

Michael,

Don't forget to merge. Thanks!

-Original Message-
From: Yajun Wu 
Sent: Monday, September 5, 2022 12:58 PM
To: Michael S. Tsirkin 
Cc: qemu-devel@nongnu.org; Parav Pandit 
Subject: RE: [PATCH v2 0/2] vhost-user: Support vhost_dev_start

Michael,

7.1 released, please merge.

-Original Message-
From: Michael S. Tsirkin 
Sent: Tuesday, July 26, 2022 10:56 PM
To: Yajun Wu 
Cc: qemu-devel@nongnu.org; Parav Pandit 
Subject: Re: [PATCH v2 0/2] vhost-user: Support vhost_dev_start

External email: Use caution opening links or attachments


On Tue, Jul 12, 2022 at 01:54:59PM +0300, Yajun Wu wrote:
> The motivation of adding vhost-user vhost_dev_start support is to 
> improve backend configuration speed and reduce live migration VM 
> downtime.
>
> Today VQ configuration is issued one by one. For virtio net with 
> multi-queue support, backend needs to update RSS (Receive side
> scaling) on every rx queue enable. Updating RSS is time-consuming 
> (typical time like 7ms).
>
> Implement already defined vhost status and message in the vhost 
> specification [1].
> (a) VHOST_USER_PROTOCOL_F_STATUS
> (b) VHOST_USER_SET_STATUS
> (c) VHOST_USER_GET_STATUS
>
> Send message VHOST_USER_SET_STATUS with VIRTIO_CONFIG_S_DRIVER_OK for 
> device start and reset(0) for device stop.
>
> On reception of the DRIVER_OK message, backend can apply the needed 
> setting only once (instead of incremental) and also utilize 
> parallelism on enabling queues.
>
> This improves QEMU's live migration downtime with vhost user backend 
> implementation by great margin, specially for the large number of VQs 
> of 64 from 800 msec to 250 msec.
>
> Another change is to move the device start routines after finishing 
> all the necessary device and VQ configuration, further aligning to the 
> virtio specification for "device initialization sequence".

I think it's best to merge this after the 7.1 release.
I've tagged this but pls ping me after the release just to make sure it's not 
lost. Thanks!

> [1]
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fqemu
> -project.gitlab.io%2Fqemu%2Finterop%2Fvhost-user.html%23introduction&a
> mp;data=05%7C01%7Cyajunw%40nvidia.com%7Cb526f8237f7a4531d6eb08da6f16fc
> e9%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C63791784266470%7CU
> nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
> WwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=zQ5%2F6pYP0riRzArdOWyaARNn
> 6s9NC8vIeIgj%2BB03EAM%3D&reserved=0
>
> v2:
> - add setting status bit VIRTIO_CONFIG_S_FEATURES_OK
> - avoid adding status bits already set
>
> Yajun Wu (2):
>   vhost: Change the sequence of device start
>   vhost-user: Support vhost_dev_start
>
>  hw/block/vhost-user-blk.c | 18 ++
>  hw/net/vhost_net.c| 12 +++
>  hw/virtio/vhost-user.c| 74 ++-
>  3 files changed, 90 insertions(+), 14 deletions(-)
>
> --
> 2.27.0

Re: [PATCH v2 3/7] util: Introduce ThreadContext user-creatable object

[Markus Armbruster]

David Hildenbrand  writes:

> Thanks Markus!
>
>> I just tested again, and get the same result as you.  I figure my
>> previous test was with the complete series.
>> PATCH 5 appears to make it work.  Suggest to say something like "The
>> commit after next will make this work".
>
> I'll phrase it like " We'll wire this up next to make it work."

Works for me!

> [...]
>
 So, when a thread is created, its affinity comes from its thread context
 (if any).  When I later change the context's affinity, it does *not*
 affect existing threads, only future ones.  Correct?
>>>
>>> Yes, that's the current state.
>> 
>> Thanks!
>
> I'm adding
>
> "Note that the CPU affinity of previously created threads will not get 
> adjusted."
>
> and
>
> "In general, the interface behaves like pthread_setaffinity_np(): host CPU 
> numbers that are currently not available are ignored; only host CPU 
> numbers that are impossible with the current kernel will fail. If the list of 
> host CPU numbers does not include a single CPU that is 
> available, setting the CPU affinity will fail."

This is one of the reasons why reviewing your work is such a pleasure:
not only do you answer my questions with clarity and patience, you
proactively improve your patches before I can even think to ask.

Thank you!

Re: [PATCH] ui/gtk: Fix the implicit mouse ungrabbing logic

[Gerd Hoffmann]

On Sat, Oct 08, 2022 at 11:01:16PM +0900, Akihiko Odaki wrote:
> Although the grab menu item represents the tabbed displays, the old
> implicit mouse ungrabbing logic changes the grab menu item even for
> an untabbed display.
> 
> Leave the grab menu item when implicitly ungrabbing mouse for an
> untabbed display. The new ungrabbing logic introduced in
> gd_mouse_mode_change() strictly follows the corresponding grabbing
> logic found in gd_button_event().

Added to queue.

thanks,
  Gerd

Re: [PULL 1/1] Revert "configure: build ROMs with container-based cross compilers"

[Daniel Henrique Barboza]

On 10/12/22 03:46, Paolo Bonzini wrote:



Il mar 11 ott 2022, 21:29 Alex Bennée mailto:alex.ben...@linaro.org>> ha scritto:

This reverts commit 730fe750fba63023e294ff0acf0f874369f1946f.

Unconditionally building all the bios for all arches was a little too
far too fast.


I would like to understand the issue better, because chances are that it is 
preexisting and applies to the TCG tests as well.

Daniel, does building the TCG tests work for you? If not, I think we should 
just disable containers by default.



'make check-tcg' never worked in this particular Xeon host I use. I never
had the curiosity to find out why because I have access to a Power9 host
that runs 'make check-tcg'.

Using this revert patch on top of master in this Xeon box makes 'make -j'
successful and 'make check-tcg' fails with the following error:


$ make -j
  GIT ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 
tests/fp/berkeley-softfloat-3 dtc
[1/24] Generating qemu-version.h with a custom command (wrapped by meson to 
capture output)

$ make check-tcg
  GIT ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 
tests/fp/berkeley-softfloat-3 dtc
  BUILD   debian-powerpc-test-cross
  BUILD   ppc64-linux-user guest-tests
Traceback (most recent call last):
  File "/home/danielhb/qemu/tests/docker/docker.py", line 683, in 
sys.exit(main())
  File "/home/danielhb/qemu/tests/docker/docker.py", line 679, in main
return args.cmdobj.run(args, argv)
  File "/home/danielhb/qemu/tests/docker/docker.py", line 657, in run
return Docker().run(cmd, False, quiet=args.quiet,
  File "/home/danielhb/qemu/tests/docker/docker.py", line 370, in run
ret = self._do_check(["run", "--rm", "--label",
  File "/home/danielhb/qemu/tests/docker/docker.py", line 247, in _do_check
return subprocess.check_call(self._command + cmd, **kwargs)
  File "/usr/lib64/python3.9/subprocess.py", line 373, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['podman', 'run', '--rm', '--label', 
'com.qemu.instance.uuid=cf15761c98884d0a9b4e37f631ba593f', '--userns=keep-id', 
'-u', '1005', '-w', '/home/danielhb/qemu/build/tests/tcg/ppc64-linux-user', 
'-v', 
'/home/danielhb/qemu/build/tests/tcg/ppc64-linux-user:/home/danielhb/qemu/build/tests/tcg/ppc64-linux-user:rw',
 '-v', '/home/danielhb/qemu:/home/danielhb/qemu:ro,z', 
'qemu/debian-powerpc-test-cross', 'powerpc64-linux-gnu-gcc-10', '-Wall', 
'-Werror', '-O0', '-g', '-fno-strict-aliasing', '-m64', '-mbig-endian', 
'/home/danielhb/qemu/tests/tcg/multiarch/float_convd.c', 
'/home/danielhb/qemu/tests/tcg/multiarch/libs/float_helpers.c', '-o', 
'float_convd', '-static', '-lm']' returned non-zero exit status 127.
filter=--filter=label=com.qemu.instance.uuid=cf15761c98884d0a9b4e37f631ba593f
make[1]: *** [/home/danielhb/qemu/tests/tcg/multiarch/Makefile.target:26: 
float_convd] Error 1
make: *** [/home/danielhb/qemu/tests/Makefile.include:50: 
build-tcg-tests-ppc64-linux-user] Error 2


This is very similar to the error message I get when running 'make -j' on 
mainline
without this revert.

So yeah, I guess we can say this is a preexisting condition that I always saw 
with
'make check-tcg' in this particular host, and 730fe750fba just made it manifest 
when
running a plain 'make'.


Thanks,


Daniel






Signed-off-by: Alex Bennée mailto:alex.ben...@linaro.org>>
Cc: Paolo Bonzini mailto:pbonz...@redhat.com>>
Reviewed-by: Daniel Henrique Barboza mailto:danielhb...@gmail.com>>
Tested-by: Daniel Henrique Barboza mailto:danielhb...@gmail.com>>
Message-Id: <2022103417.794841-4-alex.ben...@linaro.org 
>

diff --git a/configure b/configure
index baa69189f0..45ee6f4eb3 100755
--- a/configure
+++ b/configure
@@ -2121,7 +2121,7 @@ probe_target_compiler() {
      target_ranlib=
      target_strip=
    fi
-  test -n "$target_cc" || test -n "$container_image"
+  test -n "$target_cc"
  }

  write_target_makefile() {
@@ -2268,7 +2268,7 @@ if test "$targetos" != "darwin" && test "$targetos" != 
"sunos" && \
      config_mak=pc-bios/optionrom/config.mak
      echo "# Automatically generated by configure - do not modify" > 
$config_mak
      echo "TOPSRC_DIR=$source_path" >> $config_mak
-    write_target_makefile pc-bios/optionrom/all >> $config_mak
+    write_target_makefile >> $config_mak
  fi

  if test "$softmmu" = yes && probe_target_compiler ppc-softmmu; then
@@ -2276,31 +2276,25 @@ if test "$softmmu" = yes && probe_target_compiler 
ppc-softmmu; then
      config_mak=pc-bios/vof/config.mak
      echo "# Automatically generated by configure - do not modify" > 
$config_mak
      echo "SRC_DIR=$source_path/pc-bios/vof" >> $config_mak
-    write_target_makefile pc-bios/vof/all >> $config_mak
+    write_target_makefile >> $config_mak
  fi

  # Only build s390-ccw bios if 

[PATCH] migration/channel-block: fix return value for qio_channel_block_{readv, writev}

[Fiona Ebner]

in the error case. The documentation in include/io/channel.h states
that -1 or QIO_CHANNEL_ERR_BLOCK should be returned upon error. Simply
passing along the return value from the bdrv-functions has the
potential to confuse the call sides. Non-blocking mode is not
implemented currently, so -1 it is.

Signed-off-by: Fiona Ebner 
---
 migration/channel-block.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/migration/channel-block.c b/migration/channel-block.c
index c55c8c93ce..aabc4634a4 100644
--- a/migration/channel-block.c
+++ b/migration/channel-block.c
@@ -62,7 +62,8 @@ qio_channel_block_readv(QIOChannel *ioc,
 qemu_iovec_init_external(&qiov, (struct iovec *)iov, niov);
 ret = bdrv_readv_vmstate(bioc->bs, &qiov, bioc->offset);
 if (ret < 0) {
-return ret;
+error_setg(errp, "bdrv_readv_vmstate returned error %d", ret);
+return -1;
 }
 
 bioc->offset += qiov.size;
@@ -86,7 +87,8 @@ qio_channel_block_writev(QIOChannel *ioc,
 qemu_iovec_init_external(&qiov, (struct iovec *)iov, niov);
 ret = bdrv_writev_vmstate(bioc->bs, &qiov, bioc->offset);
 if (ret < 0) {
-return ret;
+error_setg(errp, "bdrv_writev_vmstate returned error %d", ret);
+return -1;
 }
 
 bioc->offset += qiov.size;
-- 
2.30.2

Re: [PATCH] gitmodules: recurse by default

[Michael S. Tsirkin]

On Wed, Oct 12, 2022 at 08:51:51AM +0100, Daniel P. Berrangé wrote:
> On Tue, Oct 11, 2022 at 06:32:40PM -0400, Michael S. Tsirkin wrote:
> > On Fri, Oct 07, 2022 at 12:09:40PM +0100, Daniel P. Berrangé wrote:
> > > On Fri, Oct 07, 2022 at 11:45:56AM +0100, Daniel P. Berrangé wrote:
> > > > On Fri, Oct 07, 2022 at 06:11:25AM -0400, Michael S. Tsirkin wrote:
> > > > > On Fri, Oct 07, 2022 at 09:07:17AM +0100, Daniel P. Berrangé wrote:
> > > > > > On Thu, Oct 06, 2022 at 08:24:01PM -0400, Michael S. Tsirkin wrote:
> > > > > > > On Thu, Oct 06, 2022 at 07:54:52PM +0100, Daniel P. Berrangé 
> > > > > > > wrote:
> > > > > > > > On Thu, Oct 06, 2022 at 07:39:07AM -0400, Michael S. Tsirkin 
> > > > > > > > wrote:
> > > > > > > > > The most commmon complaint about submodules is that
> > > > > > > > > they don't follow when one switches branches in the
> > > > > > > > > main repo. Enable recursing into submodules by default
> > > > > > > > > to address that.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Michael S. Tsirkin 
> > > > > > > > > ---
> > > > > > > > >  .gitmodules | 23 +++
> > > > > > > > >  1 file changed, 23 insertions(+)
> > > 
> > > snip
> > > 
> > > > > I just retested and it's not working for me either :(
> > > > > I was sure it worked but I guess the testing wasn't done properly.
> > > > > Back to the drawing board sorry.
> > > > 
> > > > I think the problem is that this setting doesn't apply in the context
> > > > of .gitmodules. Various commands take a '--recurse-submodules' 
> > > > parameter,
> > > > and like many params this can be set in the .git/config file. The
> > > > problem is .git/config isn't a file we can influence automatically,
> > > > it is upto the dev to set things for every clone they do :-(
> > > 
> > > With the correct setting in my .git/config, I've just discovered
> > > an unexpected & undesirable consequence of using recurse=true.
> > > It affects the 'push' command. If your submodule contains a hash
> > > that is not present in the upstream of the submodule, then when
> > > you try to push, it will also try to push the submodule change.
> > > 
> > > eg, I have a qemu.git branch 'work' and i made a change to
> > > ui/keycodemapdb. If I try to push to my gitlab fork, whose
> > > remote I called 'gitlab', then it will also try to push
> > > ui/keycodemapdb to a fork called 'gitlab'.  Except I don't
> > > have any such fork existing, so my attempt to push my qemu.git
> > > changes fails because of the submodule.
> > > 
> > > This is going to be annoying to people who are working on branches
> > > with updates to the git submodules if we were to set recurse=true
> > > by default, as they'll have to also setup remotes for submodules
> > > they work on.
> > > 
> > 
> > Well this seems like a reasonable thing to do, no?
> > 
> > If you push qemu commit referring to hash 0xABC, you want
> > that 0xABC to be available in the remote, no?
> > Otherwise how will people fetching your tree check it out?
> 
> Don't assume I'm making it available for other people. I push to
> remotes simply for moving code around for myself between machines.
> I still have the submodule code I need elsewhere, so forcing me
> to push the submodule & main repos so the same named remote is
> getting in the way. 
> 
> With regards,
> Daniel

So you are pushing this main tree but the submodule won't be
needed there? Sounds like an unusual enough thing that
it's reasonable to ask user to tweak config to enable it.

> -- 
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH 2/2] tcg/loongarch64: Add direct jump support

[WANG Xuerui]

Hi,

Thanks for the improvement! Some room for improvement though...

On 2022/10/12 17:13, Qi Hu wrote:

Similar to the ARM64, LoongArch has PC-relative instructions such as
PCADDU18I. These instructions can be used to support direct jump for
LoongArch. Additionally, if instruction "B offset" can cover the target
address, "tb_target_set_jmp_target" will only patch the "B offset".


"if the target is within +/- 28 bits range, a single "B offset" plus a 
nop will be used instead" might sound better?




Signed-off-by: Qi Hu 
---
  tcg/loongarch64/tcg-insn-defs.c.inc |  3 ++
  tcg/loongarch64/tcg-target.c.inc| 49 ++---
  tcg/loongarch64/tcg-target.h|  2 +-
  3 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/tcg/loongarch64/tcg-insn-defs.c.inc 
b/tcg/loongarch64/tcg-insn-defs.c.inc
index d162571856..f5869c6bb1 100644
--- a/tcg/loongarch64/tcg-insn-defs.c.inc
+++ b/tcg/loongarch64/tcg-insn-defs.c.inc
@@ -112,6 +112,9 @@ typedef enum {
  OPC_BLE = 0x6400,
  OPC_BGTU = 0x6800,
  OPC_BLEU = 0x6c00,
+/* pseudo-instruction */
+NOP = 0x0340,
+


You certainly saw the big fat comment block saying the file was 
auto-generated and thus shouldn't be touched, didn't you? ;-)


I saw your need for a NOP constant later though, and you can instead add 
`#define OPC_NOP OPC_ANDI` in tcg-target.c.inc, just below the include 
of tcg-insn-defs.c.inc.



  } LoongArchInsn;
  
  static int32_t __attribute__((unused))

diff --git a/tcg/loongarch64/tcg-target.c.inc b/tcg/loongarch64/tcg-target.c.inc
index f5a214a17f..3a7b1df081 100644
--- a/tcg/loongarch64/tcg-target.c.inc
+++ b/tcg/loongarch64/tcg-target.c.inc
@@ -1058,11 +1058,24 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
  break;
  
  case INDEX_op_goto_tb:

-assert(s->tb_jmp_insn_offset == 0);
-/* indirect jump method */
-tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_TMP0, TCG_REG_ZERO,
-   (uintptr_t)(s->tb_jmp_target_addr + a0));
-tcg_out_opc_jirl(s, TCG_REG_ZERO, TCG_REG_TMP0, 0);
+if (s->tb_jmp_insn_offset != NULL) {
+/* TCG_TARGET_HAS_direct_jump */
+/* Ensure that PCADD+JIRL are 8-byte aligned so that an atomic
+   write can be used to patch the target address. */
There isn't a "PCADD" in LoongArch, and it's possible for the 2 insns to 
be "B + NOP" as well. So better reword a bit like "Ensure that the 
8-byte direct jump fragment is aligned ..." (and add another space after 
the period at the end of the sentence).

+if ((uintptr_t)s->code_ptr & 7) {
+tcg_out32(s, NOP);
+}
+s->tb_jmp_insn_offset[a0] = tcg_current_code_size(s);
+/* actual branch destination will be patched by
+   tb_target_set_jmp_target later. */
Either make it a proper sentence by title-casing "actual" and adding 
another space after the trailing period, or remove the period.

+tcg_out_opc_pcaddu18i(s, TCG_REG_TMP0, 0);
+tcg_out_opc_jirl(s, TCG_REG_ZERO, TCG_REG_TMP0, 0);
+} else {
+/* !TCG_TARGET_HAS_direct_jump */
+tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_TMP0, TCG_REG_ZERO,
+(uintptr_t)(s->tb_jmp_target_addr + a0));
+tcg_out_opc_jirl(s, TCG_REG_ZERO, TCG_REG_TMP0, 0);
+}
We unconditionally support the direct jump method after the change, so 
do we need to retain this block any more? Note the aarch64 port 
currently does the same (declaring unconditional support for direct 
jumps but keeping both code paths), if we want to remove this code path 
then you may want to remove the aarch64 one respectively too.

  set_jmp_reset_offset(s, a0);
  break;
  
@@ -1708,6 +1721,32 @@ static void tcg_target_init(TCGContext *s)

  tcg_regset_set_reg(s->reserved_regs, TCG_REG_RESERVED);
  }
  
+void tb_target_set_jmp_target(uintptr_t tc_ptr, uintptr_t jmp_rx,

+  uintptr_t jmp_rw, uintptr_t addr)
+{


Move the function to somewhere above, like above the "/* Entrypoints */" 
section (and maybe introduce another section)? The various parts are 
more-or-less arranged in a particular order, so it's like going back to 
implementing concrete things after finishing everything and calling it a 
day.


Also you forgot to remove the now inappropriate comment about this 
helper in tcg-target.h.



+tcg_insn_unit i1, i2;
+
+ptrdiff_t offset = addr - jmp_rx;
+
+if (offset == sextreg(offset, 0, 28)) {
+i1 = OPC_B | ((offset >> 18) & 0x3ff) | ((offset << 8) & 0x3fffc00);
Use encode_sd10k16_insn instead. No need to juggle the bits yourself and 
you get nice range assertion for free.

+i2 = NOP;
+} else {
+offset >>= 2;
+
+ptrdiff_t upper, lower;
Promote the declaration to the top of the block (before offset 
shifting), IIRC that's the official coding style.

+upper = ((offset +

Re: [PATCH] migration/channel-block: fix return value for qio_channel_block_{readv, writev}

[Fiona Ebner]

Am 12.10.22 um 13:19 schrieb Fiona Ebner:
> in the error case. The documentation in include/io/channel.h states
> that -1 or QIO_CHANNEL_ERR_BLOCK should be returned upon error. Simply
> passing along the return value from the bdrv-functions has the
> potential to confuse the call sides. Non-blocking mode is not
> implemented currently, so -1 it is.
> 
> Signed-off-by: Fiona Ebner 
CC-ing dgilb...@redhat.com since I messed up when sending the mail.
Sorry about that!

[PATCH v2 2/2] audio: improve out.voices test

[Helge Konetzka]

Improve readability of audio out.voices test:
If 1 is logged and set after positive test, 1 should be tested.

Signed-off-by: Helge Konetzka 
Reviewed-by: Marc-André Lureau 
---
 audio/audio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/audio/audio.c b/audio/audio.c
index 8a0ade4052..912b456058 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1750,7 +1750,7 @@ static AudioState *audio_init(Audiodev *dev, const char 
*name)
 s->nb_hw_voices_out = audio_get_pdo_out(dev)->voices;
 s->nb_hw_voices_in = audio_get_pdo_in(dev)->voices;
 
-if (s->nb_hw_voices_out <= 0) {
+if (s->nb_hw_voices_out < 1) {
 dolog ("Bogus number of playback voices %d, setting to 1\n",
s->nb_hw_voices_out);
 s->nb_hw_voices_out = 1;
-- 
2.38.0

[PATCH v2 1/2] audio: fix in.voices test

[Helge Konetzka]

Calling qemu with valid -audiodev ...,in.voices=0 results in an obsolete
warning:
  audio: Bogus number of capture voices 0, setting to 0
This patch fixes the in.voices test.

Signed-off-by: Helge Konetzka 
Reviewed-by: Marc-André Lureau 
---
 audio/audio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/audio/audio.c b/audio/audio.c
index cfa4119c05..8a0ade4052 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1756,7 +1756,7 @@ static AudioState *audio_init(Audiodev *dev, const char 
*name)
 s->nb_hw_voices_out = 1;
 }
 
-if (s->nb_hw_voices_in <= 0) {
+if (s->nb_hw_voices_in < 0) {
 dolog ("Bogus number of capture voices %d, setting to 0\n",
s->nb_hw_voices_in);
 s->nb_hw_voices_in = 0;
-- 
2.38.0

[PATCH v2 0/2] Fix audio voices tests

[Helge Konetzka]

Changes for v2:
 * Sent by git send-email to keep correct format

Fix to remove obsolete warning on -audiodev ...,in.voices=0 and
improvement for better readability of audio out.voices test

Helge Konetzka (2):
  audio: fix in.voices test
  audio: improve out.voices test

 audio/audio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.38.0

Re: [PATCH 2/2] tcg/loongarch64: Add direct jump support

[WANG Xuerui]

On 2022/10/12 19:34, WANG Xuerui wrote:

+    tcg_insn_unit i1, i2;
+
+    ptrdiff_t offset = addr - jmp_rx;
+
+    if (offset == sextreg(offset, 0, 28)) {
+    i1 = OPC_B | ((offset >> 18) & 0x3ff) | ((offset << 8) & 
0x3fffc00);
Use encode_sd10k16_insn instead. No need to juggle the bits yourself 
and you get nice range assertion for free.
+    i2 = NOP; 


One more thing. I think there's a certain trend of making sure 
potentially bad things don't happen and stopping speculative insn 
fetch/execution by making the NOP here a BREAK instead. Although I'm not 
sure if LoongArch or TCG is affected by the various speculation 
vulnerabilities out there, to any degree, it might not hurt to just make 
it a BREAK? Performance shouldn't get affected in any way but one could 
probably sleep better with a guaranteed termination of execution flow 
after the B.


And you should change the "tcg_out32(s, NOP)" some lines above into just 
a "tcg_out_andi(s, 0, 0, 0)". This way you wouldn't have to define a NOP 
constant at all... Or, maybe better, make a "tcg_out_nop" helper right 
after the insn-defs include and after the "TCG intrinsics" section (it 
isn't one, but there isn't a better place), then use it here, so you get 
to include some more comments while still not having to define a NOP. Like:


static void tcg_out_nop(TCGContext *s)
{
/* Conventional NOP in LoongArch is `andi zero, zero, 0`.  */
tcg_out_opc_andi(s, 0, 0, 0);
}

Re: [PATCH] migration/channel-block: fix return value for qio_channel_block_{readv, writev}

[Daniel P . Berrangé]

On Wed, Oct 12, 2022 at 01:19:35PM +0200, Fiona Ebner wrote:
> in the error case. The documentation in include/io/channel.h states
> that -1 or QIO_CHANNEL_ERR_BLOCK should be returned upon error. Simply
> passing along the return value from the bdrv-functions has the
> potential to confuse the call sides. Non-blocking mode is not
> implemented currently, so -1 it is.

Opps, yes, my bad in writing this code.

> 
> Signed-off-by: Fiona Ebner 
> ---
>  migration/channel-block.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/channel-block.c b/migration/channel-block.c
> index c55c8c93ce..aabc4634a4 100644
> --- a/migration/channel-block.c
> +++ b/migration/channel-block.c
> @@ -62,7 +62,8 @@ qio_channel_block_readv(QIOChannel *ioc,
>  qemu_iovec_init_external(&qiov, (struct iovec *)iov, niov);
>  ret = bdrv_readv_vmstate(bioc->bs, &qiov, bioc->offset);
>  if (ret < 0) {
> -return ret;
> +error_setg(errp, "bdrv_readv_vmstate returned error %d", ret);

IIUC,  the bdrv functions return errno, so should use

error_setg_errno(errp, -ret, "bdrv_readv_vmstate returned error");


>  bioc->offset += qiov.size;
> @@ -86,7 +87,8 @@ qio_channel_block_writev(QIOChannel *ioc,
>  qemu_iovec_init_external(&qiov, (struct iovec *)iov, niov);
>  ret = bdrv_writev_vmstate(bioc->bs, &qiov, bioc->offset);
>  if (ret < 0) {
> -return ret;
> +error_setg(errp, "bdrv_writev_vmstate returned error %d", ret);
> +return -1;
>  }
>  
>  bioc->offset += qiov.size;
> -- 
> 2.30.2
> 
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[RFC PATCH 1/4] docs/devel: add a maintainers section to development process

[Alex Bennée]

We don't currently have a clear place in the documentation to describe
the rolls and responsibilities of a maintainer. Lets create one so we
can. I've moved a few small bits out of other files to try and keep
everything in one place.

Signed-off-by: Alex Bennée 
---
 docs/devel/code-of-conduct.rst   |  2 +
 docs/devel/index-process.rst |  1 +
 docs/devel/maintainers.rst   | 84 
 docs/devel/submitting-a-pull-request.rst | 12 ++--
 4 files changed, 91 insertions(+), 8 deletions(-)
 create mode 100644 docs/devel/maintainers.rst

diff --git a/docs/devel/code-of-conduct.rst b/docs/devel/code-of-conduct.rst
index 195444d1b4..f734ed0317 100644
--- a/docs/devel/code-of-conduct.rst
+++ b/docs/devel/code-of-conduct.rst
@@ -1,3 +1,5 @@
+.. _code_of_conduct:
+
 Code of Conduct
 ===
 
diff --git a/docs/devel/index-process.rst b/docs/devel/index-process.rst
index d0d7a200fd..d50dd74c3e 100644
--- a/docs/devel/index-process.rst
+++ b/docs/devel/index-process.rst
@@ -8,6 +8,7 @@ Notes about how to interact with the community and how and 
where to submit patch
 
code-of-conduct
conflict-resolution
+   maintainers
style
submitting-a-patch
trivial-patches
diff --git a/docs/devel/maintainers.rst b/docs/devel/maintainers.rst
new file mode 100644
index 00..e3c7003bfa
--- /dev/null
+++ b/docs/devel/maintainers.rst
@@ -0,0 +1,84 @@
+.. _maintainers:
+
+The Roll of Maintainers
+===
+
+Maintainers are a critical part of the projects contributor ecosystem.
+They come from a wide range of backgrounds from unpaid hobbyists
+working in their spare time to employees who work on the project as
+part of their job. Maintainer activities include:
+
+  - reviewing patches and suggesting changes
+  - preparing pull requests for their subsystems
+  - participating other project activities
+
+They are also human and subject to the same pressures as everyone else
+including overload and burn out. Like everyone else they are subject
+to projects :ref:`code_of_conduct`.
+
+The MAINTAINERS file
+
+
+The `MAINTAINERS
+`__
+file contains the canonical list of who is a maintainer. The file
+is machine readable so an appropriately configured git (see
+:ref:`cc_the_relevant_maintainer`) can automatically Cc them on
+patches that touch their area of code.
+
+The file also describes the status of the area of code to give an idea
+of how actively that section is maintained.
+
+.. list-table:: Meaning of support status in MAINTAINERS
+   :widths: 25 75
+   :header-rows: 1
+
+   * - Status
+ - Meaning
+   * - Supported
+ - Someone is actually paid to look after this.
+   * - Maintained
+ - Someone actually looks after it.
+   * - Odd Fixes
+ - It has a maintainer but they don't have time to do
+   much other than throw the odd patch in.
+   * - Orphan
+ - No current maintainer.
+   * - Obsolete
+ - Old obsolete code, should use something else.
+
+Please bare in mind that even if someone is paid to support something
+it does not mean they are paid to support you. This is open source and
+the code comes with no warranty and the project makes no guarantees
+about dealing with bugs or features requests.
+
+Becoming a maintainer
+-
+
+Maintainers are volunteers who put themselves forward to keep an eye
+on an area of code. They are generally accepted by the community to
+have a good understanding of the subsystem and able to make a positive
+contribution to the project.
+
+The process is simple - simply sent a patch to the list that updates
+the ``MAINTAINERS`` file. Sometimes this is done as part of a larger
+series when a new sub-system is being added to the code base. This can
+also be done by a retiring maintainer who nominates their replacement
+after discussion with other contributors.
+
+Once the patch is reviewed and merged the only other step is to make
+sure your GPG key is signed.
+
+.. _maintainer_keys:
+
+Maintainer GPG Keys
+~~~
+
+GPG is used to sign pull requests so they can be identified as really
+coming from the maintainer. If your key is not already signed by
+members of the QEMU community, you should make arrangements to attend
+a `KeySigningParty `__ (for
+example at KVM Forum) or make alternative arrangements to have your
+key signed by an attendee. Key signing requires meeting another
+community member **in person** so please make appropriate
+arrangements.
diff --git a/docs/devel/submitting-a-pull-request.rst 
b/docs/devel/submitting-a-pull-request.rst
index c9d1e8afd9..a4cd7ebbb6 100644
--- a/docs/devel/submitting-a-pull-request.rst
+++ b/docs/devel/submitting-a-pull-request.rst
@@ -53,14 +53,10 @@ series) and that "make check" passes before sending out the 
pull
 request. As a submaintainer you're one of QEMU's lines of defense
 against ba

[RFC PATCH 2/4] docs/devel: make language a little less code centric

[Alex Bennée]

We welcome all sorts of patches.

Signed-off-by: Alex Bennée 
---
 docs/devel/submitting-a-patch.rst | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/docs/devel/submitting-a-patch.rst 
b/docs/devel/submitting-a-patch.rst
index fec33ce148..fb1673e974 100644
--- a/docs/devel/submitting-a-patch.rst
+++ b/docs/devel/submitting-a-patch.rst
@@ -3,11 +3,11 @@
 Submitting a Patch
 ==
 
-QEMU welcomes contributions of code (either fixing bugs or adding new
-functionality). However, we get a lot of patches, and so we have some
-guidelines about submitting patches. If you follow these, you'll help
-make our task of code review easier and your patch is likely to be
-committed faster.
+QEMU welcomes contributions to fix bugs, add functionality or improve
+the documentation. However, we get a lot of patches, and so we have
+some guidelines about submitting patches. If you follow these, you'll
+help make our task of code review easier and your patch is likely to
+be committed faster.
 
 This page seems very long, so if you are only trying to post a quick
 one-shot fix, the bare minimum we ask is that:
-- 
2.34.1

[RFC PATCH 3/4] docs/devel: simplify the minimal checklist

[Alex Bennée]

The bullet points are quite long and contain process tips. Move those
bits of the bullet to the relevant sections and link to them. Use a
table for nicer formatting of the checklist.

Signed-off-by: Alex Bennée 
---
 docs/devel/submitting-a-patch.rst | 75 ---
 roms/qboot|  2 +-
 2 files changed, 50 insertions(+), 27 deletions(-)

diff --git a/docs/devel/submitting-a-patch.rst 
b/docs/devel/submitting-a-patch.rst
index fb1673e974..41771501bf 100644
--- a/docs/devel/submitting-a-patch.rst
+++ b/docs/devel/submitting-a-patch.rst
@@ -12,25 +12,18 @@ be committed faster.
 This page seems very long, so if you are only trying to post a quick
 one-shot fix, the bare minimum we ask is that:
 
--  You **must** provide a Signed-off-by: line (this is a hard
-   requirement because it's how you say "I'm legally okay to contribute
-   this and happy for it to go into QEMU", modeled after the `Linux kernel
-   
`__
-   policy.) ``git commit -s`` or ``git format-patch -s`` will add one.
--  All contributions to QEMU must be **sent as patches** to the
-   qemu-devel `mailing list `__.
-   Patch contributions should not be posted on the bug tracker, posted on
-   forums, or externally hosted and linked to. (We have other mailing lists 
too,
-   but all patches must go to qemu-devel, possibly with a Cc: to another
-   list.) ``git send-email`` (`step-by-step setup
-   guide `__ and `hints and
-   tips 
`__)
-   works best for delivering the patch without mangling it, but
-   attachments can be used as a last resort on a first-time submission.
--  You must read replies to your message, and be willing to act on them.
-   Note, however, that maintainers are often willing to manually fix up
-   first-time contributions, since there is a learning curve involved in
-   making an ideal patch submission.
+.. list-table:: Minimal Checklist for Patches
+   :widths: 35 65
+   :header-rows: 1
+
+   * - Check
+ - Reason
+   * - Patches contain Signed-off-by: Author Name 
+ - States you are legally able to contribute the code. See 
:ref:`patch_emails_must_include_a_signed_off_by_line`
+   * - Sent as patch emails to ``qemu-devel@nongnu.org``
+ - The project uses an email list based workflow. See 
:ref:`submitting_your_patches`
+   * - Be prepared to respond to review comments
+ - Code that doesn't pass review will not get merged. See 
:ref:`participating_in_code_review`
 
 You do not have to subscribe to post (list policy is to reply-to-all to
 preserve CCs and keep non-subscribers in the loop on the threads they
@@ -229,6 +222,19 @@ bisection doesn't land on a known-broken state.
 Submitting your Patches
 ---
 
+The QEMU project uses a public email based workflow for reviewing and
+merging patches. As a result all contributions to QEMU must be **sent
+as patches** to the qemu-devel `mailing list
+`__. Patch
+contributions should not be posted on the bug tracker, posted on
+forums, or externally hosted and linked to. (We have other mailing
+lists too, but all patches must go to qemu-devel, possibly with a Cc:
+to another list.) ``git send-email`` (`step-by-step setup guide
+`__ and `hints and tips
+`__)
+works best for delivering the patch without mangling it, but
+attachments can be used as a last resort on a first-time submission.
+
 .. _if_you_cannot_send_patch_emails:
 
 If you cannot send patch emails
@@ -314,10 +320,12 @@ git repository to fetch the original commit.
 Patch emails must include a ``Signed-off-by:`` line
 ~~~
 
-For more information see `SubmittingPatches 1.12
-`__.
-This is vital or we will not be able to apply your patch! Please use
-your real name to sign a patch (not an alias or acronym).
+Your patches **must** include a Signed-off-by: line. This is a hard
+requirement because it's how you say "I'm legally okay to contribute
+this and happy for it to go into QEMU". The process is modelled after
+the `Linux kernel
+`__
+policy.
 
 If you wrote the patch, make sure your "From:" and "Signed-off-by:"
 lines use the same spelling. It's okay if you subscribe or contribute to
@@ -327,6 +335,11 @@ include a "From:" line in the b

[RFC PATCH 4/4] docs/devel: try and improve the language around patch review

[Alex Bennée]

It is important that contributors take the review process seriously
and we collaborate in a respectful way while avoiding personal
attacks. Try and make this clear in the language.

Signed-off-by: Alex Bennée 
---
 docs/devel/submitting-a-patch.rst | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/docs/devel/submitting-a-patch.rst 
b/docs/devel/submitting-a-patch.rst
index 41771501bf..5b6dc7ddf1 100644
--- a/docs/devel/submitting-a-patch.rst
+++ b/docs/devel/submitting-a-patch.rst
@@ -434,14 +434,20 @@ developers will identify bugs, or suggest a cleaner 
approach, or even
 just point out code style issues or commit message typos. You'll need to
 respond to these, and then send a second version of your patches with
 the issues fixed. This takes a little time and effort on your part, but
-if you don't do it then your changes will never get into QEMU. It's also
-just polite -- it is quite disheartening for a developer to spend time
-reviewing your code and suggesting improvements, only to find that
-you're not going to do anything further and it was all wasted effort.
+if you don't do it then your changes will never get into QEMU.
+
+Remember that a maintainer is under no obligation to take your
+patches. If someone has spent the time reviewing your code and
+suggesting improvements and you simply re-post without either
+addressing the comment directly or providing additional justification
+for the change then it becomes wasted effort. You cannot demand others
+merge and then fix up your code after the fact.
 
 When replying to comments on your patches **reply to all and not just
 the sender** -- keeping discussion on the mailing list means everybody
-can follow it.
+can follow it. Remember the spirit of the :ref:`code_of_conduct` and
+keep discussions respectful and collaborative and avoid making
+personal comments.
 
 .. _pay_attention_to_review_comments:
 
-- 
2.34.1

Re: [RFC PATCH 3/4] docs/devel: simplify the minimal checklist

[Daniel P . Berrangé]

On Wed, Oct 12, 2022 at 01:11:51PM +0100, Alex Bennée wrote:
> The bullet points are quite long and contain process tips. Move those
> bits of the bullet to the relevant sections and link to them. Use a
> table for nicer formatting of the checklist.
> 
> Signed-off-by: Alex Bennée 
> ---
>  docs/devel/submitting-a-patch.rst | 75 ---
>  roms/qboot|  2 +-

git submodule surprise !


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[RFC PATCH 0/4] docs/devel suggestions for discussion

[Alex Bennée]

Hi,

This is an attempt to improve our processes documentation by:

 - adding an explicit section on maintainers
 - reducing the up-front verbiage in patch submission
 - emphasising the importance to respectful reviews

I'm sure the language could be improved further so I humbly submit
this RFC for discussion.

Alex Bennée (4):
  docs/devel: add a maintainers section to development process
  docs/devel: make language a little less code centric
  docs/devel: simplify the minimal checklist
  docs/devel: try and improve the language around patch review

 docs/devel/code-of-conduct.rst   |   2 +
 docs/devel/index-process.rst |   1 +
 docs/devel/maintainers.rst   |  84 +++
 docs/devel/submitting-a-patch.rst| 101 +++
 docs/devel/submitting-a-pull-request.rst |  12 +--
 roms/qboot   |   2 +-
 6 files changed, 157 insertions(+), 45 deletions(-)
 create mode 100644 docs/devel/maintainers.rst

-- 
2.34.1

Re: [PULL 1/1] Revert "configure: build ROMs with container-based cross compilers"

[Alex Bennée]

Daniel Henrique Barboza  writes:

> On 10/12/22 03:46, Paolo Bonzini wrote:
>> Il mar 11 ott 2022, 21:29 Alex Bennée > > ha scritto:
>> This reverts commit 730fe750fba63023e294ff0acf0f874369f1946f.
>> Unconditionally building all the bios for all arches was a
>> little too
>> far too fast.
>> I would like to understand the issue better, because chances are
>> that it is preexisting and applies to the TCG tests as well.
>> Daniel, does building the TCG tests work for you? If not, I think we
>> should just disable containers by default.
>
>
> 'make check-tcg' never worked in this particular Xeon host I use. I never
> had the curiosity to find out why because I have access to a Power9 host
> that runs 'make check-tcg'.
>
> Using this revert patch on top of master in this Xeon box makes 'make -j'
> successful and 'make check-tcg' fails with the following error:
>
>
> $ make -j
>   GIT ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 
> tests/fp/berkeley-softfloat-3 dtc
> [1/24] Generating qemu-version.h with a custom command (wrapped by meson to 
> capture output)
>
> $ make check-tcg
>   GIT ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 
> tests/fp/berkeley-softfloat-3 dtc
>   BUILD   debian-powerpc-test-cross
>   BUILD   ppc64-linux-user guest-tests
> Traceback (most recent call last):
>   File "/home/danielhb/qemu/tests/docker/docker.py", line 683, in 
> sys.exit(main())
>   File "/home/danielhb/qemu/tests/docker/docker.py", line 679, in main
> return args.cmdobj.run(args, argv)
>   File "/home/danielhb/qemu/tests/docker/docker.py", line 657, in run
> return Docker().run(cmd, False, quiet=args.quiet,
>   File "/home/danielhb/qemu/tests/docker/docker.py", line 370, in run
> ret = self._do_check(["run", "--rm", "--label",
>   File "/home/danielhb/qemu/tests/docker/docker.py", line 247, in _do_check
> return subprocess.check_call(self._command + cmd, **kwargs)
>   File "/usr/lib64/python3.9/subprocess.py", line 373, in check_call
> raise CalledProcessError(retcode, cmd)
> subprocess.CalledProcessError: Command '['podman', 'run', '--rm', '--label', 
> 'com.qemu.instance.uuid=cf15761c98884d0a9b4e37f631ba593f', 
> '--userns=keep-id', '-u', '1005', '-w', 
> '/home/danielhb/qemu/build/tests/tcg/ppc64-linux-user', '-v', 
> '/home/danielhb/qemu/build/tests/tcg/ppc64-linux-user:/home/danielhb/qemu/build/tests/tcg/ppc64-linux-user:rw',
>  '-v', '/home/danielhb/qemu:/home/danielhb/qemu:ro,z', 
> 'qemu/debian-powerpc-test-cross', 'powerpc64-linux-gnu-gcc-10', '-Wall', 
> '-Werror', '-O0', '-g', '-fno-strict-aliasing', '-m64', '-mbig-endian', 
> '/home/danielhb/qemu/tests/tcg/multiarch/float_convd.c', 
> '/home/danielhb/qemu/tests/tcg/multiarch/libs/float_helpers.c', '-o', 
> 'float_convd', '-static', '-lm']' returned non-zero exit status 127.
> filter=--filter=label=com.qemu.instance.uuid=cf15761c98884d0a9b4e37f631ba593f
> make[1]: *** [/home/danielhb/qemu/tests/tcg/multiarch/Makefile.target:26: 
> float_convd] Error 1
> make: *** [/home/danielhb/qemu/tests/Makefile.include:50: 
> build-tcg-tests-ppc64-linux-user] Error 2
>
>
> This is very similar to the error message I get when running 'make -j' on 
> mainline
> without this revert.
>
> So yeah, I guess we can say this is a preexisting condition that I always saw 
> with
> 'make check-tcg' in this particular host, and 730fe750fba just made it 
> manifest when
> running a plain 'make'.
>
>
> Thanks,
>
>
> Daniel
>
>
>> Signed-off-by: Alex Bennée > >
>> Cc: Paolo Bonzini mailto:pbonz...@redhat.com>>
>> Reviewed-by: Daniel Henrique Barboza > >
>> Tested-by: Daniel Henrique Barboza > >
>> Message-Id: <2022103417.794841-4-alex.ben...@linaro.org 
>> >
>> diff --git a/configure b/configure
>> index baa69189f0..45ee6f4eb3 100755
>> --- a/configure
>> +++ b/configure
>> @@ -2121,7 +2121,7 @@ probe_target_compiler() {
>>       target_ranlib=
>>       target_strip=
>>     fi
>> -  test -n "$target_cc" || test -n "$container_image"
>> +  test -n "$target_cc"
>>   }
>>   write_target_makefile() {
>> @@ -2268,7 +2268,7 @@ if test "$targetos" != "darwin" && test 
>> "$targetos" != "sunos" && \
>>       config_mak=pc-bios/optionrom/config.mak
>>       echo "# Automatically generated by configure - do not modify" > 
>> $config_mak
>>       echo "TOPSRC_DIR=$source_path" >> $config_mak
>> -    write_target_makefile pc-bios/optionrom/all >> $config_mak
>> +    write_target_makefile >> $config_mak
>>   fi
>>   if test "$softmmu" = yes && probe_target_compiler ppc-softmmu;
>> then
>> @@ -2276,31 +2276,25 @@ if test "$softmmu" = yes && 
>> probe_target_compiler ppc-softmmu; then
>>       config_mak=pc-bios/vof/config.mak
>>       echo "# Automatical

Re: [PATCH] build: disable container-based cross compilers by default

[Alex Bennée]

Paolo Bonzini  writes:

> Container-based cross compilers have some issues which were overlooked
> when they were only used for TCG tests, but are more visible since
> firmware builds try to use them:

We seem to have dropped our gating somewhere. Previously if a user did
not have docker or podman on their system none of the container stuff
would run.

-- 
Alex Bennée

[PATCH v2 0/2] Refactoring: expand usage of TFR() macro

[Nikita Ivanov]

At the moment, TFR() macro has a vague name and is not used
where it possibly could be. In order to make it more transparent
and useful, it was decided to refactor it to make it closer to
the similar one in glibc: TEMP_FAILURE_RETRY(). Now, macro
evaluates into an expression and is named RETRY_ON_EINTR(). All the
places where RETRY_ON_EINTR() macro code be applied were covered.

Nikita Ivanov (2):
  Refactoring: refactor TFR() macro to RETRY_ON_EINTR()
  error handling: Use RETRY_ON_EINTR() macro where applicable

 block/file-posix.c| 37 -
 chardev/char-fd.c |  2 +-
 chardev/char-pipe.c   |  8 +---
 chardev/char-pty.c|  4 +---
 hw/9pfs/9p-local.c|  8 ++--
 include/qemu/osdep.h  |  8 +++-
 net/l2tpv3.c  | 17 +
 net/socket.c  | 16 +++-
 net/tap-bsd.c |  6 +++---
 net/tap-linux.c   |  2 +-
 net/tap-solaris.c |  8 
 net/tap.c | 12 
 os-posix.c|  2 +-
 qga/commands-posix.c  |  4 +---
 semihosting/syscalls.c|  4 +---
 tests/qtest/libqtest.c| 14 ++
 tests/vhost-user-bridge.c |  4 +---
 util/main-loop.c  |  4 +---
 util/osdep.c  |  4 +---
 util/vfio-helpers.c   | 12 ++--
 20 files changed, 74 insertions(+), 102 deletions(-)

--
2.37.3

Re: [PATCH v2 3/7] util: Introduce ThreadContext user-creatable object

[David Hildenbrand]

Thanks!


I'm adding

"Note that the CPU affinity of previously created threads will not get 
adjusted."

and

"In general, the interface behaves like pthread_setaffinity_np(): host CPU 
numbers that are currently not available are ignored; only host CPU
numbers that are impossible with the current kernel will fail. If the list of 
host CPU numbers does not include a single CPU that is
available, setting the CPU affinity will fail."


This is one of the reasons why reviewing your work is such a pleasure:
not only do you answer my questions with clarity and patience, you
proactively improve your patches before I can even think to ask.

Thank you!


Thanks a lot Markus -- I have to say that your reviews are extremely 
helpful! You ask just the right questions that make one realize which 
parts of the documentation need improvement!


--
Thanks,

David / dhildenb

[PATCH v2 1/2] Refactoring: refactor TFR() macro to RETRY_ON_EINTR()

[Nikita Ivanov]

Rename macro name to more transparent one and refactor
it to expression.

Signed-off-by: Nikita Ivanov 
---
 chardev/char-fd.c| 2 +-
 chardev/char-pipe.c  | 8 +---
 include/qemu/osdep.h | 8 +++-
 net/tap-bsd.c| 6 +++---
 net/tap-linux.c  | 2 +-
 net/tap-solaris.c| 8 
 os-posix.c   | 2 +-
 7 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/chardev/char-fd.c b/chardev/char-fd.c
index cf78454841..d2c4923359 100644
--- a/chardev/char-fd.c
+++ b/chardev/char-fd.c
@@ -198,7 +198,7 @@ int qmp_chardev_open_file_source(char *src, int flags,
Error **errp)
 {
 int fd = -1;

-TFR(fd = qemu_open_old(src, flags, 0666));
+fd = RETRY_ON_EINTR(qemu_open_old(src, flags, 0666));
 if (fd == -1) {
 error_setg_file_open(errp, errno, src);
 }
diff --git a/chardev/char-pipe.c b/chardev/char-pipe.c
index 66d3b85091..5ad30bcc59 100644
--- a/chardev/char-pipe.c
+++ b/chardev/char-pipe.c
@@ -131,8 +131,8 @@ static void qemu_chr_open_pipe(Chardev *chr,

 filename_in = g_strdup_printf("%s.in", filename);
 filename_out = g_strdup_printf("%s.out", filename);
-TFR(fd_in = qemu_open_old(filename_in, O_RDWR | O_BINARY));
-TFR(fd_out = qemu_open_old(filename_out, O_RDWR | O_BINARY));
+fd_in = RETRY_ON_EINTR(qemu_open_old(filename_in, O_RDWR | O_BINARY));
+fd_out = RETRY_ON_EINTR(qemu_open_old(filename_out, O_RDWR |
O_BINARY));
 g_free(filename_in);
 g_free(filename_out);
 if (fd_in < 0 || fd_out < 0) {
@@ -142,7 +142,9 @@ static void qemu_chr_open_pipe(Chardev *chr,
 if (fd_out >= 0) {
 close(fd_out);
 }
-TFR(fd_in = fd_out = qemu_open_old(filename, O_RDWR | O_BINARY));
+fd_in = fd_out = RETRY_ON_EINTR(
+qemu_open_old(filename, O_RDWR | O_BINARY)
+);
 if (fd_in < 0) {
 error_setg_file_open(errp, errno, filename);
 return;
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index b1c161c035..a470905475 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -243,7 +243,13 @@ void QEMU_ERROR("code path is reachable")
 #define ESHUTDOWN 4099
 #endif

-#define TFR(expr) do { if ((expr) != -1) break; } while (errno == EINTR)
+#define RETRY_ON_EINTR(expr) \
+(__extension__  \
+({ typeof(expr) __result;   \
+   do { \
+__result = (typeof(expr)) (expr); \
+   } while (__result == -1L && errno == EINTR); \
+   __result; }))

 /* time_t may be either 32 or 64 bits depending on the host OS, and
  * can be either signed or unsigned, so we can't just hardcode a
diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index 005ce05c6e..4c98fdd337 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -56,7 +56,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
 } else {
 snprintf(dname, sizeof dname, "/dev/tap%d", i);
 }
-TFR(fd = open(dname, O_RDWR));
+fd = RETRY_ON_EINTR(open(dname, O_RDWR));
 if (fd >= 0) {
 break;
 }
@@ -111,7 +111,7 @@ static int tap_open_clone(char *ifname, int
ifname_size, Error **errp)
 int fd, s, ret;
 struct ifreq ifr;

-TFR(fd = open(PATH_NET_TAP, O_RDWR));
+fd = RETRY_ON_EINTR(open(PATH_NET_TAP, O_RDWR));
 if (fd < 0) {
 error_setg_errno(errp, errno, "could not open %s", PATH_NET_TAP);
 return -1;
@@ -159,7 +159,7 @@ int tap_open(char *ifname, int ifname_size, int
*vnet_hdr,
 if (ifname[0] != '\0') {
 char dname[100];
 snprintf(dname, sizeof dname, "/dev/%s", ifname);
-TFR(fd = open(dname, O_RDWR));
+fd = RETRY_ON_EINTR(open(dname, O_RDWR));
 if (fd < 0 && errno != ENOENT) {
 error_setg_errno(errp, errno, "could not open %s", dname);
 return -1;
diff --git a/net/tap-linux.c b/net/tap-linux.c
index 304ff45071..f54f308d35 100644
--- a/net/tap-linux.c
+++ b/net/tap-linux.c
@@ -45,7 +45,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
 int len = sizeof(struct virtio_net_hdr);
 unsigned int features;

-TFR(fd = open(PATH_NET_TUN, O_RDWR));
+fd = RETRY_ON_EINTR(open(PATH_NET_TUN, O_RDWR));
 if (fd < 0) {
 error_setg_errno(errp, errno, "could not open %s", PATH_NET_TUN);
 return -1;
diff --git a/net/tap-solaris.c b/net/tap-solaris.c
index a44f8805c2..38e15028bf 100644
--- a/net/tap-solaris.c
+++ b/net/tap-solaris.c
@@ -84,13 +84,13 @@ static int tap_alloc(char *dev, size_t dev_size, Error
**errp)
 if( ip_fd )
close(ip_fd);

-TFR(ip_fd = open("/dev/udp", O_RDWR, 0));
+ip_fd = RETRY_ON_EINTR(open("/dev/udp", O_RDWR, 0));
 if (ip_fd < 0) {
 error_setg(errp, "Can't open /dev/ip (actually /dev/udp)");
 return -1;
 }

-TFR(tap_fd = open("/dev/tap", O_RDWR, 0));
+tap_fd = R

[PATCH v2 2/2] error handling: Use RETRY_ON_EINTR() macro where applicable

[Nikita Ivanov]

There is a defined RETRY_ON_EINTR() macro in qemu/osdep.h which
handles the same while loop.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/415

Signed-off-by: Nikita Ivanov 
---
 block/file-posix.c| 37 -
 chardev/char-pty.c|  4 +---
 hw/9pfs/9p-local.c|  8 ++--
 net/l2tpv3.c  | 17 +
 net/socket.c  | 16 +++-
 net/tap.c | 12 
 qga/commands-posix.c  |  4 +---
 semihosting/syscalls.c|  4 +---
 tests/qtest/libqtest.c| 14 ++
 tests/vhost-user-bridge.c |  4 +---
 util/main-loop.c  |  4 +---
 util/osdep.c  |  4 +---
 util/vfio-helpers.c   | 12 ++--
 13 files changed, 52 insertions(+), 88 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 66fdb07820..c589cb489b 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1238,9 +1238,7 @@ static int hdev_get_max_segments(int fd, struct stat
*st)
 ret = -errno;
 goto out;
 }
-do {
-ret = read(sysfd, buf, sizeof(buf) - 1);
-} while (ret == -1 && errno == EINTR);
+ret = RETRY_ON_EINTR(read(sysfd, buf, sizeof(buf) - 1));
 if (ret < 0) {
 ret = -errno;
 goto out;
@@ -1388,9 +1386,9 @@ static int handle_aiocb_ioctl(void *opaque)
 RawPosixAIOData *aiocb = opaque;
 int ret;

-do {
-ret = ioctl(aiocb->aio_fildes, aiocb->ioctl.cmd, aiocb->ioctl.buf);
-} while (ret == -1 && errno == EINTR);
+ret = RETRY_ON_EINTR(
+ioctl(aiocb->aio_fildes, aiocb->ioctl.cmd, aiocb->ioctl.buf)
+);
 if (ret == -1) {
 return -errno;
 }
@@ -1472,18 +1470,17 @@ static ssize_t
handle_aiocb_rw_vector(RawPosixAIOData *aiocb)
 {
 ssize_t len;

-do {
-if (aiocb->aio_type & QEMU_AIO_WRITE)
-len = qemu_pwritev(aiocb->aio_fildes,
-   aiocb->io.iov,
-   aiocb->io.niov,
-   aiocb->aio_offset);
- else
-len = qemu_preadv(aiocb->aio_fildes,
-  aiocb->io.iov,
-  aiocb->io.niov,
-  aiocb->aio_offset);
-} while (len == -1 && errno == EINTR);
+len = RETRY_ON_EINTR(
+(aiocb->aio_type & QEMU_AIO_WRITE) ?
+qemu_pwritev(aiocb->aio_fildes,
+   aiocb->io.iov,
+   aiocb->io.niov,
+   aiocb->aio_offset) :
+qemu_preadv(aiocb->aio_fildes,
+  aiocb->io.iov,
+  aiocb->io.niov,
+  aiocb->aio_offset)
+);

 if (len == -1) {
 return -errno;
@@ -1908,9 +1905,7 @@ static int allocate_first_block(int fd, size_t
max_size)
 buf = qemu_memalign(max_align, write_size);
 memset(buf, 0, write_size);

-do {
-n = pwrite(fd, buf, write_size, 0);
-} while (n == -1 && errno == EINTR);
+n = RETRY_ON_EINTR(pwrite(fd, buf, write_size, 0));

 ret = (n == -1) ? -errno : 0;

diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index 53f25c6bbd..92fd33c854 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -93,9 +93,7 @@ static void pty_chr_update_read_handler(Chardev *chr)
 pfd.fd = fioc->fd;
 pfd.events = G_IO_OUT;
 pfd.revents = 0;
-do {
-rc = g_poll(&pfd, 1, 0);
-} while (rc == -1 && errno == EINTR);
+rc = RETRY_ON_EINTR(g_poll(&pfd, 1, 0));
 assert(rc >= 0);

 if (pfd.revents & G_IO_HUP) {
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index d42ce6d8b8..bb3187244f 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -470,9 +470,7 @@ static ssize_t local_readlink(FsContext *fs_ctx,
V9fsPath *fs_path,
 if (fd == -1) {
 return -1;
 }
-do {
-tsize = read(fd, (void *)buf, bufsz);
-} while (tsize == -1 && errno == EINTR);
+tsize = RETRY_ON_EINTR(read(fd, (void *)buf, bufsz));
 close_preserve_errno(fd);
 } else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) ||
(fs_ctx->export_flags & V9FS_SM_NONE)) {
@@ -908,9 +906,7 @@ static int local_symlink(FsContext *fs_ctx, const char
*oldpath,
 }
 /* Write the oldpath (target) to the file. */
 oldpath_size = strlen(oldpath);
-do {
-write_size = write(fd, (void *)oldpath, oldpath_size);
-} while (write_size == -1 && errno == EINTR);
+write_size = RETRY_ON_EINTR(write(fd, (void *)oldpath,
oldpath_size));
 close_preserve_errno(fd);

 if (write_size != oldpath_size) {
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index af373e5c30..e0726f5f89 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -240,9 +240,7 @@ static ssize_t
net_l2tpv3_receive_dgram_iov(NetClientState *nc,
 message.msg_control = NULL;
 message.m

Re: [PATCH] qemu-edid: Restrict input parameter -d to avoid division by zero

[Gerd Hoffmann]

On Tue, Oct 11, 2022 at 05:12:16PM +0200, Sebastian Mitterle wrote:
> A zero value for dpi will lead to a division by zero in qemu_edid_dpi_to_mm().
> Tested by runnig qemu-edid -dX, X = 0, 100.
> 
> Resolves: qemu-project/qemu#1249
> 
> Suggested-by: Thomas Huth 
> Signed-off-by: Sebastian Mitterle 

Added to queue.

thanks,
  Gerd

[PATCH v1] s390x/tod-kvm: don't save/restore the TOD in PV guests

[Nico Boehr]

Under PV, the guest's TOD clock is under control of the ultravisor and the
hypervisor cannot change it.

With upcoming kernel changes[1], the Linux kernel will reject QEMU's
request to adjust the guest's clock in this case, so don't attempt to set
the clock.

This avoids the following warning message on save/restore of a PV guest:

warning: Unable to set KVM guest TOD clock: Operation not supported

[1] https://lore.kernel.org/all/20221011160712.928239-2-...@linux.ibm.com/

Fixes: c3347ed0d2ee ("s390x: protvirt: Support unpack facility")
Signed-off-by: Nico Boehr 
---
 hw/s390x/tod-kvm.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/s390x/tod-kvm.c b/hw/s390x/tod-kvm.c
index 9d0cbfbce2bf..303bd67ee64f 100644
--- a/hw/s390x/tod-kvm.c
+++ b/hw/s390x/tod-kvm.c
@@ -13,6 +13,7 @@
 #include "qemu/module.h"
 #include "sysemu/runstate.h"
 #include "hw/s390x/tod.h"
+#include "hw/s390x/pv.h"
 #include "kvm/kvm_s390x.h"
 
 static void kvm_s390_get_tod_raw(S390TOD *tod, Error **errp)
@@ -84,6 +85,13 @@ static void kvm_s390_tod_vm_state_change(void *opaque, bool 
running,
 S390TODState *td = opaque;
 Error *local_err = NULL;
 
+/*
+ * Under PV, the clock is under ultravisor control, hence we cannot restore
+ * it on resume.
+ */
+if (s390_is_pv())
+return;
+
 if (running && td->stopped) {
 /* Set the old TOD when running the VM - start the TOD clock. */
 kvm_s390_set_tod_raw(&td->base, &local_err);
-- 
2.36.1

Re: [PATCH] tests/unit/test-image-locking: Fix handling of temporary files

[Bin Meng]

On Wed, Oct 12, 2022 at 4:59 PM Thomas Huth  wrote:
>
> test-image-locking leaves some temporary files around - clean
> them up. While we're at it, test-image-locking is a unit test,
> so it should not use "qtest.*" for temporary file names. Give
> them better names instead, so that it clear where the temporary
> files come from.
>
> Signed-off-by: Thomas Huth 
> ---
>  tests/unit/test-image-locking.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>

Fixes: aef96d7d4f0b ("tests: Add unit tests for image locking")

Reviewed-by: Bin Meng 

Re: [PULL 1/1] Revert "configure: build ROMs with container-based cross compilers"

[Daniel Henrique Barboza]

On 10/12/22 09:13, Alex Bennée wrote:


Daniel Henrique Barboza  writes:


On 10/12/22 03:46, Paolo Bonzini wrote:

Il mar 11 ott 2022, 21:29 Alex Bennée mailto:alex.ben...@linaro.org>> ha scritto:
 This reverts commit 730fe750fba63023e294ff0acf0f874369f1946f.
 Unconditionally building all the bios for all arches was a
little too
 far too fast.
I would like to understand the issue better, because chances are
that it is preexisting and applies to the TCG tests as well.
Daniel, does building the TCG tests work for you? If not, I think we
should just disable containers by default.



'make check-tcg' never worked in this particular Xeon host I use. I never
had the curiosity to find out why because I have access to a Power9 host
that runs 'make check-tcg'.

Using this revert patch on top of master in this Xeon box makes 'make -j'
successful and 'make check-tcg' fails with the following error:


$ make -j
   GIT ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 
tests/fp/berkeley-softfloat-3 dtc
[1/24] Generating qemu-version.h with a custom command (wrapped by meson to 
capture output)

$ make check-tcg
   GIT ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 
tests/fp/berkeley-softfloat-3 dtc
   BUILD   debian-powerpc-test-cross
   BUILD   ppc64-linux-user guest-tests
Traceback (most recent call last):
   File "/home/danielhb/qemu/tests/docker/docker.py", line 683, in 
 sys.exit(main())
   File "/home/danielhb/qemu/tests/docker/docker.py", line 679, in main
 return args.cmdobj.run(args, argv)
   File "/home/danielhb/qemu/tests/docker/docker.py", line 657, in run
 return Docker().run(cmd, False, quiet=args.quiet,
   File "/home/danielhb/qemu/tests/docker/docker.py", line 370, in run
 ret = self._do_check(["run", "--rm", "--label",
   File "/home/danielhb/qemu/tests/docker/docker.py", line 247, in _do_check
 return subprocess.check_call(self._command + cmd, **kwargs)
   File "/usr/lib64/python3.9/subprocess.py", line 373, in check_call
 raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['podman', 'run', '--rm', '--label', 
'com.qemu.instance.uuid=cf15761c98884d0a9b4e37f631ba593f', '--userns=keep-id', 
'-u', '1005', '-w', '/home/danielhb/qemu/build/tests/tcg/ppc64-linux-user', 
'-v', 
'/home/danielhb/qemu/build/tests/tcg/ppc64-linux-user:/home/danielhb/qemu/build/tests/tcg/ppc64-linux-user:rw',
 '-v', '/home/danielhb/qemu:/home/danielhb/qemu:ro,z', 
'qemu/debian-powerpc-test-cross', 'powerpc64-linux-gnu-gcc-10', '-Wall', 
'-Werror', '-O0', '-g', '-fno-strict-aliasing', '-m64', '-mbig-endian', 
'/home/danielhb/qemu/tests/tcg/multiarch/float_convd.c', 
'/home/danielhb/qemu/tests/tcg/multiarch/libs/float_helpers.c', '-o', 
'float_convd', '-static', '-lm']' returned non-zero exit status 127.
filter=--filter=label=com.qemu.instance.uuid=cf15761c98884d0a9b4e37f631ba593f
make[1]: *** [/home/danielhb/qemu/tests/tcg/multiarch/Makefile.target:26: 
float_convd] Error 1
make: *** [/home/danielhb/qemu/tests/Makefile.include:50: 
build-tcg-tests-ppc64-linux-user] Error 2


This is very similar to the error message I get when running 'make -j' on 
mainline
without this revert.

So yeah, I guess we can say this is a preexisting condition that I always saw 
with
'make check-tcg' in this particular host, and 730fe750fba just made it manifest 
when
running a plain 'make'.


Thanks,


Daniel



 Signed-off-by: Alex Bennée mailto:alex.ben...@linaro.org>>
 Cc: Paolo Bonzini mailto:pbonz...@redhat.com>>
 Reviewed-by: Daniel Henrique Barboza mailto:danielhb...@gmail.com>>
 Tested-by: Daniel Henrique Barboza mailto:danielhb...@gmail.com>>
 Message-Id: <2022103417.794841-4-alex.ben...@linaro.org 
>
 diff --git a/configure b/configure
 index baa69189f0..45ee6f4eb3 100755
 --- a/configure
 +++ b/configure
 @@ -2121,7 +2121,7 @@ probe_target_compiler() {
       target_ranlib=
       target_strip=
     fi
 -  test -n "$target_cc" || test -n "$container_image"
 +  test -n "$target_cc"
   }
   write_target_makefile() {
 @@ -2268,7 +2268,7 @@ if test "$targetos" != "darwin" && test "$targetos" != 
"sunos" && \
       config_mak=pc-bios/optionrom/config.mak
       echo "# Automatically generated by configure - do not modify" > 
$config_mak
       echo "TOPSRC_DIR=$source_path" >> $config_mak
 -    write_target_makefile pc-bios/optionrom/all >> $config_mak
 +    write_target_makefile >> $config_mak
   fi
   if test "$softmmu" = yes && probe_target_compiler ppc-softmmu;
then
 @@ -2276,31 +2276,25 @@ if test "$softmmu" = yes && probe_target_compiler 
ppc-softmmu; then
       config_mak=pc-bios/vof/config.mak
       echo "# Automatically generated by configure - do not modify" > 
$config_mak
       echo "SRC_DIR=$source_path/pc-bios/vof" >> $config_mak
 -    write_target_makefile pc-bios/vof/all

Re: [PATCH v2 00/11] iotests: use vm.cmd()

[Vladimir Sementsov-Ogievskiy]

ping

On 9/19/22 20:16, Vladimir Sementsov-Ogievskiy wrote:

ping

On 6/6/22 10:27, Vladimir Sementsov-Ogievskiy wrote:

Hi all!

Let's get rid of pattern

 result = self.vm.qmp(...)
 self.assert_qmp(result, 'return', {})

And switch to just

 self.vm.cmd(...)

Supersedes: <20220408170214.45585-1-vsement...@openvz.org>
    ([RFC 0/2] introduce QEMUMachind.cmd())

Vladimir Sementsov-Ogievskiy (11):
   python: rename QEMUMonitorProtocol.cmd() to cmd_raw()
   python/qemu: rename command() to cmd()
   python/machine.py: upgrade vm.cmd() method
   iotests: QemuStorageDaemon: add cmd() method like in QEMUMachine.
   iotests: add some missed checks of qmp result
   iotests: refactor some common qmp result checks into generic pattern
   iotests: drop some occasional semicolons
   iotests: drop some extra ** in qmp() call
   iotests.py: pause_job(): drop return value
   tests/vm/basevm.py: use cmd() instead of qmp()
   python: use vm.cmd() instead of vm.qmp() where appropriate

  docs/devel/testing.rst    |  10 +-
  python/qemu/machine/machine.py    |  20 +-
  python/qemu/qmp/legacy.py |  10 +-
  python/qemu/qmp/qmp_shell.py  |  13 +-
  python/qemu/utils/qemu_ga_client.py   |   2 +-
  python/qemu/utils/qom.py  |   8 +-
  python/qemu/utils/qom_common.py   |   2 +-
  python/qemu/utils/qom_fuse.py |   6 +-
  scripts/cpu-x86-uarch-abi.py  |   8 +-
  scripts/device-crash-test |   8 +-
  scripts/render_block_graph.py |   8 +-
  tests/avocado/avocado_qemu/__init__.py    |   4 +-
  tests/avocado/cpu_queries.py  |   4 +-
  tests/avocado/hotplug_cpu.py  |  10 +-
  tests/avocado/info_usernet.py |   4 +-
  tests/avocado/machine_arm_integratorcp.py |   6 +-
  tests/avocado/machine_m68k_nextcube.py    |   4 +-
  tests/avocado/machine_mips_malta.py   |   6 +-
  tests/avocado/machine_s390_ccw_virtio.py  |  28 +-
  tests/avocado/migration.py    |  10 +-
  tests/avocado/pc_cpu_hotplug_props.py |   2 +-
  tests/avocado/version.py  |   4 +-
  tests/avocado/virtio_check_params.py  |   6 +-
  tests/avocado/virtio_version.py   |   4 +-
  tests/avocado/vnc.py  |  16 +-
  tests/avocado/x86_cpu_model_versions.py   |  10 +-
  tests/migration/guestperf/engine.py   | 150 +++---
  tests/qemu-iotests/030    | 168 +++---
  tests/qemu-iotests/040    | 171 +++
  tests/qemu-iotests/041    | 482 --
  tests/qemu-iotests/045    |  15 +-
  tests/qemu-iotests/055    |  62 +--
  tests/qemu-iotests/056    |  77 ++-
  tests/qemu-iotests/093    |  42 +-
  tests/qemu-iotests/118    | 225 
  tests/qemu-iotests/124    | 102 ++--
  tests/qemu-iotests/129    |  14 +-
  tests/qemu-iotests/132    |   5 +-
  tests/qemu-iotests/139    |  45 +-
  tests/qemu-iotests/147    |  30 +-
  tests/qemu-iotests/151    |  56 +-
  tests/qemu-iotests/152    |   8 +-
  tests/qemu-iotests/155    |  55 +-
  tests/qemu-iotests/165    |   8 +-
  tests/qemu-iotests/196    |   3 +-
  tests/qemu-iotests/205    |   6 +-
  tests/qemu-iotests/218    | 105 ++--
  tests/qemu-iotests/245    | 245 -
  tests/qemu-iotests/256    |  34 +-
  tests/qemu-iotests/257    |  36 +-
  tests/qemu-iotests/264    |  31 +-
  tests/qemu-iotests/281    |  21 +-
  tests/qemu-iotests/295    |  16 +-
  tests/qemu-iotests/296    |  21 +-
  tests/qemu-iotests/298    |  13 +-
  tests/qemu-iotests/300    |  54 +-
  tests/qemu-iotests/iotests.py |  18 +-
  .../tests/export-incoming-iothread    |   6 +-
  .../qemu-iotests/tests/graph-changes-while-io |   6 +-
  tests/qemu-iotests/tests/image-fleecing   |   3 +-
  .../tests/migrate-bitmaps-postcopy-test   |  31 +-
  tests/qemu-iotests/tests/migrate-bitmaps-test |  45 +-
  .../qemu-iotests/tests/migrate-during-backup  |  41 +-
  .../qemu-iotests/tests/migration-permissions  |   9 +-
  .../tests/mirror-ready-cancel-error   |  74 ++-
  tests/qemu-iotests/tests/mirror-top-perms |  16 +-
  tests/qemu-iotests/tests/nbd-multiconn    |  12 +-
  tests/qemu-iotests/tests/reopen-file  |   3 +-
  .../qemu-iotests/tests/stream-error-on-reset  |   6 +-
  tests/vm/basevm.py

Re: [PATCH v7 for-7.2 00/15] block: cleanup backing and file handling

[Vladimir Sementsov-Ogievskiy]

ping

On 9/19/22 20:20, Vladimir Sementsov-Ogievskiy wrote:

ping. Seems, all patches are reviewed

On 7/26/22 23:11, Vladimir Sementsov-Ogievskiy wrote:

Hi all!

That's the first part of
"[PATCH v5 00/45] Transactional block-graph modifying API",
updated and is fully reviewed by Hanna.

v7: add r-bs and rebase on master

Vladimir Sementsov-Ogievskiy (15):
   block: BlockDriver: add .filtered_child_is_backing field
   block: introduce bdrv_open_file_child() helper
   block/blklogwrites: don't care to remove bs->file child on failure
   test-bdrv-graph-mod: update test_parallel_perm_update test case
   tests-bdrv-drain: bdrv_replace_test driver: declare supports_backing
   test-bdrv-graph-mod: fix filters to be filters
   block: document connection between child roles and
 bs->backing/bs->file
   block/snapshot: stress that we fallback to primary child
   Revert "block: Let replace_child_noperm free children"
   Revert "block: Let replace_child_tran keep indirect pointer"
   Revert "block: Restructure remove_file_or_backing_child()"
   Revert "block: Pass BdrvChild ** to replace_child_noperm"
   block: Manipulate bs->file / bs->backing pointers in .attach/.detach
   block/snapshot: drop indirection around bdrv_snapshot_fallback_ptr
   block: refactor bdrv_remove_file_or_backing_child to bdrv_remove_child

  block.c    | 435 ++---
  block/blkdebug.c   |   9 +-
  block/blklogwrites.c   |  11 +-
  block/blkreplay.c  |   7 +-
  block/blkverify.c  |   9 +-
  block/bochs.c  |   7 +-
  block/cloop.c  |   7 +-
  block/commit.c |   1 +
  block/copy-before-write.c  |   9 +-
  block/copy-on-read.c   |   9 +-
  block/crypto.c |  11 +-
  block/dmg.c    |   7 +-
  block/filter-compress.c    |   8 +-
  block/mirror.c |   1 +
  block/parallels.c  |   7 +-
  block/preallocate.c    |   9 +-
  block/qcow.c   |   6 +-
  block/qcow2.c  |   8 +-
  block/qed.c    |   8 +-
  block/raw-format.c |   4 +-
  block/replication.c    |   8 +-
  block/snapshot-access.c    |   6 +-
  block/snapshot.c   |  59 ++--
  block/throttle.c   |   8 +-
  block/vdi.c    |   7 +-
  block/vhdx.c   |   7 +-
  block/vmdk.c   |   7 +-
  block/vpc.c    |   7 +-
  include/block/block-common.h   |  39 +++
  include/block/block-global-state.h |   3 +
  include/block/block_int-common.h   |  29 +-
  tests/unit/test-bdrv-drain.c   |  11 +-
  tests/unit/test-bdrv-graph-mod.c   | 104 ---
  33 files changed, 389 insertions(+), 479 deletions(-)






--
Best regards,
Vladimir

Re: [PATCH] virtio: add VIRTQUEUE_ERROR QAPI event

[Vladimir Sementsov-Ogievskiy]

ping

On 9/19/22 22:48, Vladimir Sementsov-Ogievskiy wrote:

For now we only log the vhost device error, when virtqueue is actually
stopped. Let's add a QAPI event, which makes possible:

  - collect statistics of such errors
  - make immediate actions: take coredums or do some other debugging

The event could be reused for some other virtqueue problems (not only
for vhost devices) in future. For this it gets a generic name and
structure.

Signed-off-by: Vladimir Sementsov-Ogievskiy 


--
Best regards,
Vladimir

Re: [PATCH 00/20] tests/9p: introduce declarative function calls

[Greg Kurz]

On Wed, 12 Oct 2022 12:00:40 +0200
Christian Schoenebeck  wrote:

> On Dienstag, 4. Oktober 2022 22:56:44 CEST Christian Schoenebeck wrote:
> > This series converts relevant 9p (test) client functions to use named
> > function arguments. For instance
> > 
> > do_walk_expect_error(v9p, "non-existent", ENOENT);
> > 
> > becomes
> > 
> > twalk({
> > .client = v9p, .path = "non-existent", .expectErr = ENOENT
> > });
> > 
> > The intention is to make the actual 9p test code more readable, and easier
> > to maintain on the long-term.
> > 
> > Not only makes it clear what a literal passed to a function is supposed to
> > do, it also makes the order and selection of arguments very liberal, and
> > allows to merge multiple, similar functions into one single function.
> > 
> > This is basically just refactoring, it does not change behaviour.
> 
> Too massive for review?
> 

Yeah, sorry :-(

But since the approach you're taking here may be valuable elsewhere,
and this is qtest, it seems fair to ask Thomas and Laurent to have
a look :-)

> If so, then I'll probably just go ahead and prepare a PR early next week with 
> this queued as well. It's just test code refactoring, so I am quite painless 
> about these changes.
> 
> Best regards,
> Christian Schoenebeck
> 
> > 
> > PREREQUISITES
> > =
> > 
> > This series requires the following additional patch to work correctly:
> > 
> > https://lore.kernel.org/all/e1odrya-0004fv...@lizzy.crudebyte.com/
> > https://github.com/cschoenebeck/qemu/commit/23d01367fc7a4f27be323ed6d195c527
> > bec9ede1
> > 
> > Christian Schoenebeck (20):
> >   tests/9p: merge *walk*() functions
> >   tests/9p: simplify callers of twalk()
> >   tests/9p: merge v9fs_tversion() and do_version()
> >   tests/9p: merge v9fs_tattach(), do_attach(), do_attach_rqid()
> >   tests/9p: simplify callers of tattach()
> >   tests/9p: convert v9fs_tgetattr() to declarative arguments
> >   tests/9p: simplify callers of tgetattr()
> >   tests/9p: convert v9fs_treaddir() to declarative arguments
> >   tests/9p: simplify callers of treaddir()
> >   tests/9p: convert v9fs_tlopen() to declarative arguments
> >   tests/9p: simplify callers of tlopen()
> >   tests/9p: convert v9fs_twrite() to declarative arguments
> >   tests/9p: simplify callers of twrite()
> >   tests/9p: convert v9fs_tflush() to declarative arguments
> >   tests/9p: merge v9fs_tmkdir() and do_mkdir()
> >   tests/9p: merge v9fs_tlcreate() and do_lcreate()
> >   tests/9p: merge v9fs_tsymlink() and do_symlink()
> >   tests/9p: merge v9fs_tlink() and do_hardlink()
> >   tests/9p: merge v9fs_tunlinkat() and do_unlinkat()
> >   tests/9p: remove unnecessary g_strdup() calls
> > 
> >  tests/qtest/libqos/virtio-9p-client.c | 569 +-
> >  tests/qtest/libqos/virtio-9p-client.h | 408 --
> >  tests/qtest/virtio-9p-test.c  | 529 
> >  3 files changed, 1031 insertions(+), 475 deletions(-)
> 
> 
> 

Re: [PATCH 3/5] hw/mem/cxl_type: Generalize CDATDsmas initialization for Memory Regions

[Jonathan Cameron via]

On Tue, 11 Oct 2022 17:19:14 -0400
Gregory Price  wrote:

> This is a preparatory commit for enabling multiple memory regions within
> a single CXL Type-3 device.  We will need to initialize multiple CDAT
> DSMAS regions (and subsequent DSLBIS, and DSEMTS entries), so generalize
> the intialization into a function.
> 
> Signed-off-by: Gregory Price 

Hi Gregory,

Main comment here is that DOE isn't in yet.  Some of the changes
you have made in here should be review comments on that series rather
than here.

I'm also not keen on taking the various allocations to arrays,
particularly when seeing the hacky result in the free routine.

Just spin some more pointers and 3 more allocations (once persistent is
added).

Jonathan

> ---
>  hw/mem/cxl_type3.c | 275 +
>  1 file changed, 154 insertions(+), 121 deletions(-)
> 
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 282f274266..dda78704c2 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -24,145 +24,178 @@
>  #define UI64_NULL ~(0ULL)
>  #define DWORD_BYTE 4
>  
> +static int ct3_build_dsmas(CDATDsmas *dsmas,
> +   CDATDslbis *dslbis,
> +   CDATDsemts *dsemts,
> +   MemoryRegion *mr,
> +   int dsmad_handle,
> +   bool is_pmem,
> +   uint64_t dpa_base)

Rewrap this to be just under 80 characters per line.

This is building a lot more than DSMAS.
Could rename it, or could break it down into functions
that deal with each type  of entry.

> +{
> +int len = 0;
> +/* ttl_len should be incremented for every entry */

ttl_ ?

Given you now allocate outside of this function, probably
more appropriate to just add the entries up there.


> +
> +/* Device Scoped Memory Affinity Structure */
> +*dsmas = (CDATDsmas) {
> +.header = {
> +.type = CDAT_TYPE_DSMAS,
> +.length = sizeof(*dsmas),
> +},
> +.DSMADhandle = dsmad_handle,
> +.flags = (is_pmem ? CDAT_DSMAS_FLAG_NV : 0),
> +.DPA_base = dpa_base,
> +.DPA_length = int128_get64(mr->size),
> +};
> +len++;
> +
> +/* For now, no memory side cache, plausiblish numbers */
> +dslbis[0] = (CDATDslbis) {
> +.header = {
> +.type = CDAT_TYPE_DSLBIS,
> +.length = sizeof(*dslbis),
> +},
> +.handle = dsmad_handle,
> +.flags = HMAT_LB_MEM_MEMORY,
> +.data_type = HMAT_LB_DATA_READ_LATENCY,
> +.entry_base_unit = 1, /* 10ns base */
> +.entry[0] = 15, /* 150ns */
> +};
> +len++;
> +
> +dslbis[1] = (CDATDslbis) {
> +.header = {
> +.type = CDAT_TYPE_DSLBIS,
> +.length = sizeof(*dslbis),
> +},
> +.handle = dsmad_handle,
> +.flags = HMAT_LB_MEM_MEMORY,
> +.data_type = HMAT_LB_DATA_WRITE_LATENCY,
> +.entry_base_unit = 1,
> +.entry[0] = 25, /* 250ns */
> +};
> +len++;
> +
> +dslbis[2] = (CDATDslbis) {
> +.header = {
> +.type = CDAT_TYPE_DSLBIS,
> +.length = sizeof(*dslbis),
> +},
> +.handle = dsmad_handle,
> +.flags = HMAT_LB_MEM_MEMORY,
> +.data_type = HMAT_LB_DATA_READ_BANDWIDTH,
> +.entry_base_unit = 1000, /* GB/s */
> +.entry[0] = 16,
> +};
> +len++;
> +
> +dslbis[3] = (CDATDslbis) {
> +.header = {
> +.type = CDAT_TYPE_DSLBIS,
> +.length = sizeof(*dslbis),
> +},
> +.handle = dsmad_handle,
> +.flags = HMAT_LB_MEM_MEMORY,
> +.data_type = HMAT_LB_DATA_WRITE_BANDWIDTH,
> +.entry_base_unit = 1000, /* GB/s */
> +.entry[0] = 16,
> +};
> +len++;
> +
> +*dsemts = (CDATDsemts) {
> +.header = {
> +.type = CDAT_TYPE_DSEMTS,
> +.length = sizeof(*dsemts),
> +},
> +.DSMAS_handle = dsmad_handle,
> +/* EFI_MEMORY_NV implies EfiReservedMemoryType */
> +.EFI_memory_type_attr = is_pmem ? 2 : 0,
> +/* Reserved - the non volatile from DSMAS matters */
> +.DPA_offset = 0,
> +.DPA_length = int128_get64(mr->size),
> +};
> +len++;
> +return len;
> +}
> +
>  static int ct3_build_cdat_table(CDATSubHeader ***cdat_table,
>  void *priv)
>  {
> -g_autofree CDATDsmas *dsmas_nonvolatile = NULL;
> -g_autofree CDATDslbis *dslbis_nonvolatile = NULL;
> -g_autofree CDATDsemts *dsemts_nonvolatile = NULL;
> +g_autofree CDATDsmas *dsmas = NULL;
> +g_autofree CDATDslbis *dslbis = NULL;
> +g_autofree CDATDsemts *dsemts = NULL;
>  CXLType3Dev *ct3d = priv;
> -int len = 0;

There are changes in here that aren't necessary and just result
in a much harder diff to review.  Why rename len to cdat_len?

> -int i = 0;
> -int next_dsmad_han

Re: [PATCH] build: disable container-based cross compilers by default

[Paolo Bonzini]

On 10/12/22 14:17, Alex Bennée wrote:

Container-based cross compilers have some issues which were overlooked
when they were only used for TCG tests, but are more visible since
firmware builds try to use them:

We seem to have dropped our gating somewhere. Previously if a user did
not have docker or podman on their system none of the container stuff
would run.


It's still there:

container="no"
if test $use_containers = "yes"; then
case $($python "$source_path"/tests/docker/docker.py probe) in
*docker) container=docker ;;
podman) container=podman ;;
no) container=no ;;
esac
if test "$container" != "no"; then
docker_py="$python $source_path/tests/docker/docker.py --engine 
$container"
fi
fi

I think what's happening is that podman is there but there's no support
for rootless containers, so "podman run" fails.

Paolo

Re: ublk-qcow2: ublk-qcow2 is available

[Stefan Hajnoczi]

On Thu, 6 Oct 2022 at 06:14, Richard W.M. Jones  wrote:
>
> On Tue, Oct 04, 2022 at 09:53:32AM -0400, Stefan Hajnoczi wrote:
> > qemu-nbd doesn't use io_uring to handle the backend IO,
>
> Would this be fixed by your (not yet upstream) libblkio driver for
> qemu?

I was wrong, qemu-nbd has syntax to use io_uring:

  $ qemu-nbd ... --image-opts driver=file,filename=test.img,aio=io_uring

The new libblkio driver will also support io_uring, but QEMU's
built-in io_uring support is already available and can be used as
shown above.

Stefan

Re: [PATCH v2 1/2] Refactoring: refactor TFR() macro to RETRY_ON_EINTR()

[Marc-André Lureau]

On Wed, Oct 12, 2022 at 4:29 PM Nikita Ivanov 
wrote:

> Rename macro name to more transparent one and refactor
> it to expression.
>
> Signed-off-by: Nikita Ivanov 
>

Reviewed-by: Marc-André Lureau 



> ---
>  chardev/char-fd.c| 2 +-
>  chardev/char-pipe.c  | 8 +---
>  include/qemu/osdep.h | 8 +++-
>  net/tap-bsd.c| 6 +++---
>  net/tap-linux.c  | 2 +-
>  net/tap-solaris.c| 8 
>  os-posix.c   | 2 +-
>  7 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/chardev/char-fd.c b/chardev/char-fd.c
> index cf78454841..d2c4923359 100644
> --- a/chardev/char-fd.c
> +++ b/chardev/char-fd.c
> @@ -198,7 +198,7 @@ int qmp_chardev_open_file_source(char *src, int flags,
> Error **errp)
>  {
>  int fd = -1;
>
> -TFR(fd = qemu_open_old(src, flags, 0666));
> +fd = RETRY_ON_EINTR(qemu_open_old(src, flags, 0666));
>  if (fd == -1) {
>  error_setg_file_open(errp, errno, src);
>  }
> diff --git a/chardev/char-pipe.c b/chardev/char-pipe.c
> index 66d3b85091..5ad30bcc59 100644
> --- a/chardev/char-pipe.c
> +++ b/chardev/char-pipe.c
> @@ -131,8 +131,8 @@ static void qemu_chr_open_pipe(Chardev *chr,
>
>  filename_in = g_strdup_printf("%s.in", filename);
>  filename_out = g_strdup_printf("%s.out", filename);
> -TFR(fd_in = qemu_open_old(filename_in, O_RDWR | O_BINARY));
> -TFR(fd_out = qemu_open_old(filename_out, O_RDWR | O_BINARY));
> +fd_in = RETRY_ON_EINTR(qemu_open_old(filename_in, O_RDWR | O_BINARY));
> +fd_out = RETRY_ON_EINTR(qemu_open_old(filename_out, O_RDWR |
> O_BINARY));
>  g_free(filename_in);
>  g_free(filename_out);
>  if (fd_in < 0 || fd_out < 0) {
> @@ -142,7 +142,9 @@ static void qemu_chr_open_pipe(Chardev *chr,
>  if (fd_out >= 0) {
>  close(fd_out);
>  }
> -TFR(fd_in = fd_out = qemu_open_old(filename, O_RDWR | O_BINARY));
> +fd_in = fd_out = RETRY_ON_EINTR(
> +qemu_open_old(filename, O_RDWR | O_BINARY)
> +);
>  if (fd_in < 0) {
>  error_setg_file_open(errp, errno, filename);
>  return;
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index b1c161c035..a470905475 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -243,7 +243,13 @@ void QEMU_ERROR("code path is reachable")
>  #define ESHUTDOWN 4099
>  #endif
>
> -#define TFR(expr) do { if ((expr) != -1) break; } while (errno == EINTR)
> +#define RETRY_ON_EINTR(expr) \
> +(__extension__  \
> +({ typeof(expr) __result;   \
> +   do { \
> +__result = (typeof(expr)) (expr); \
> +   } while (__result == -1L && errno == EINTR); \
> +   __result; }))
>
>  /* time_t may be either 32 or 64 bits depending on the host OS, and
>   * can be either signed or unsigned, so we can't just hardcode a
> diff --git a/net/tap-bsd.c b/net/tap-bsd.c
> index 005ce05c6e..4c98fdd337 100644
> --- a/net/tap-bsd.c
> +++ b/net/tap-bsd.c
> @@ -56,7 +56,7 @@ int tap_open(char *ifname, int ifname_size, int
> *vnet_hdr,
>  } else {
>  snprintf(dname, sizeof dname, "/dev/tap%d", i);
>  }
> -TFR(fd = open(dname, O_RDWR));
> +fd = RETRY_ON_EINTR(open(dname, O_RDWR));
>  if (fd >= 0) {
>  break;
>  }
> @@ -111,7 +111,7 @@ static int tap_open_clone(char *ifname, int
> ifname_size, Error **errp)
>  int fd, s, ret;
>  struct ifreq ifr;
>
> -TFR(fd = open(PATH_NET_TAP, O_RDWR));
> +fd = RETRY_ON_EINTR(open(PATH_NET_TAP, O_RDWR));
>  if (fd < 0) {
>  error_setg_errno(errp, errno, "could not open %s", PATH_NET_TAP);
>  return -1;
> @@ -159,7 +159,7 @@ int tap_open(char *ifname, int ifname_size, int
> *vnet_hdr,
>  if (ifname[0] != '\0') {
>  char dname[100];
>  snprintf(dname, sizeof dname, "/dev/%s", ifname);
> -TFR(fd = open(dname, O_RDWR));
> +fd = RETRY_ON_EINTR(open(dname, O_RDWR));
>  if (fd < 0 && errno != ENOENT) {
>  error_setg_errno(errp, errno, "could not open %s", dname);
>  return -1;
> diff --git a/net/tap-linux.c b/net/tap-linux.c
> index 304ff45071..f54f308d35 100644
> --- a/net/tap-linux.c
> +++ b/net/tap-linux.c
> @@ -45,7 +45,7 @@ int tap_open(char *ifname, int ifname_size, int
> *vnet_hdr,
>  int len = sizeof(struct virtio_net_hdr);
>  unsigned int features;
>
> -TFR(fd = open(PATH_NET_TUN, O_RDWR));
> +fd = RETRY_ON_EINTR(open(PATH_NET_TUN, O_RDWR));
>  if (fd < 0) {
>  error_setg_errno(errp, errno, "could not open %s", PATH_NET_TUN);
>  return -1;
> diff --git a/net/tap-solaris.c b/net/tap-solaris.c
> index a44f8805c2..38e15028bf 100644
> --- a/net/tap-solaris.c
> +++ b/net/tap-solaris.c
> @@ -84,13 +84,13 @@ static int tap_alloc(char *dev, size_t dev_size, Er

Re: [PATCH 0/3] iothread and irqfd support

[Stefan Hajnoczi]

On Fri, 26 Aug 2022 at 07:18, Jinhao Fan  wrote:
>
> This patch series adds support for using a seperate iothread for NVMe
> IO emulation, which brings the potential of applying polling. The
> first two patches implements support for irqfd, which solves thread
> safety problems for interrupt emulation outside the main loop thread.
>
> Jinhao Fan (3):
>   hw/nvme: support irq(de)assertion with eventfd
>   hw/nvme: use KVM irqfd when available
>   hw/nvme: add iothread support

Hi,
What is the status of this series?

Stefan

[PULL 00/16] qtest patches (and one unit test and one avocado fix)

[Thomas Huth]

 Hi Stefan!

The following changes since commit 1dcdc92c72af5311666df64f5f04d6600af262ed:

  Merge tag 'pull-hex-20221003' of https://github.com/quic/qemu into staging 
(2022-10-05 10:17:32 -0400)

are available in the Git repository at:

  https://gitlab.com/thuth/qemu.git tags/pull-request-2022-10-12

for you to fetch changes up to 04c92d2654b130fd29597a57ae2d71e70895bf2b:

  tests/unit/test-image-locking: Fix handling of temporary files (2022-10-12 
12:48:53 +0200)


* Rework of qtests to support hot plugging tests on q35
* New VNC qtest
* Fixes related to temporary file handling in the tests
* Use signal() instead of sigaction() since the latter does not work on Windows
* Some other small clean-ups


Bin Meng (2):
  tests/qtest: migration-test: Avoid using hardcoded /tmp
  tests/qtest: libqtest: Install signal handler via signal()

Juan Quintela (1):
  qtest: "-display none" is set in qtest_init()

Marc-André Lureau (1):
  qtest: start a VNC test

Michael Labiuk (9):
  tests/x86: add helper qtest_qmp_device_del_send()
  tests/x86: Add subtest with 'q35' machine type to device-plug-test
  tests/x86: Refactor hot unplug hd-geo-test
  tests/x86: Add 'q35' machine type to override-tests in hd-geo-test
  tests/x86: Add 'q35' machine type to hotplug hd-geo-test
  tests/x86: Fix comment typo in drive_del-test
  tests/x86: replace snprint() by g_strdup_printf() in drive_del-test
  tests/x86: Add 'q35' machine type to drive_del-test
  tests/x86: Add 'q35' machine type to ivshmem-test

Peter Maydell (1):
  tests/avocado: Add missing require_netdev('user') checks

Thomas Huth (1):
  tests/unit/test-image-locking: Fix handling of temporary files

dinglimin (1):
  tests/migration: remove the unused local variable

 tests/qtest/libqtest.h  |  10 ++
 tests/qtest/bios-tables-test.c  |   2 +-
 tests/qtest/device-plug-test.c  |  56 ++--
 tests/qtest/drive_del-test.c| 125 +++--
 tests/qtest/fuzz-lsi53c895a-test.c  |   2 +-
 tests/qtest/fuzz-megasas-test.c |   2 +-
 tests/qtest/fuzz-sb16-test.c|   6 +-
 tests/qtest/fuzz-sdcard-test.c  |   6 +-
 tests/qtest/fuzz-virtio-scsi-test.c |   2 +-
 tests/qtest/fuzz-xlnx-dp-test.c |   2 +-
 tests/qtest/hd-geo-test.c   | 273 +++-
 tests/qtest/ivshmem-test.c  |  18 +++
 tests/qtest/libqos/pci-pc.c |   8 +-
 tests/qtest/libqtest.c  |  30 ++--
 tests/qtest/migration-test.c|  10 +-
 tests/qtest/vnc-display-test.c  | 103 ++
 tests/unit/test-image-locking.c |   6 +-
 tests/avocado/boot_linux_console.py |   4 +
 tests/avocado/machine_aspeed.py |   3 +
 tests/avocado/ppc_bamboo.py |   1 +
 tests/migration/guestperf/engine.py |   1 -
 tests/qtest/meson.build |   8 +-
 22 files changed, 544 insertions(+), 134 deletions(-)
 create mode 100644 tests/qtest/vnc-display-test.c

[PULL 05/16] tests/x86: Refactor hot unplug hd-geo-test

[Thomas Huth]

From: Michael Labiuk 

Moving common code to function.

Signed-off-by: Michael Labiuk 
Message-Id: <20220929223547.1429580-4-michael.lab...@virtuozzo.com>
Reviewed-by: Thomas Huth 
Signed-off-by: Thomas Huth 
---
 tests/qtest/hd-geo-test.c | 110 ++
 1 file changed, 40 insertions(+), 70 deletions(-)

diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
index ba772f4d7a..61f4c24b81 100644
--- a/tests/qtest/hd-geo-test.c
+++ b/tests/qtest/hd-geo-test.c
@@ -691,7 +691,8 @@ static void add_virtio_disk(TestArgs *args,
 args->n_virtio_disks++;
 }
 
-static void test_override(TestArgs *args, CHSResult expected[])
+static void test_override(TestArgs *args, const char *arch,
+  CHSResult expected[])
 {
 QTestState *qts;
 char *joined_args;
@@ -700,7 +701,7 @@ static void test_override(TestArgs *args, CHSResult 
expected[])
 
 joined_args = g_strjoinv(" ", args->argv);
 
-qts = qtest_initf("-machine pc %s", joined_args);
+qts = qtest_initf("-machine %s %s", arch, joined_args);
 fw_cfg = pc_fw_cfg_init(qts);
 
 read_bootdevices(fw_cfg, expected);
@@ -737,7 +738,7 @@ static void test_override_ide(void)
 add_ide_disk(args, 1, 0, 1, 9000, 120, 30);
 add_ide_disk(args, 2, 1, 0, 0, 1, 1);
 add_ide_disk(args, 3, 1, 1, 1, 0, 0);
-test_override(args, expected);
+test_override(args, "pc", expected);
 }
 
 static void test_override_scsi(void)
@@ -759,7 +760,7 @@ static void test_override_scsi(void)
 add_scsi_disk(args, 1, 0, 0, 1, 0, 9000, 120, 30);
 add_scsi_disk(args, 2, 0, 0, 2, 0, 1, 0, 0);
 add_scsi_disk(args, 3, 0, 0, 3, 0, 0, 1, 0);
-test_override(args, expected);
+test_override(args, "pc", expected);
 }
 
 static void test_override_scsi_2_controllers(void)
@@ -782,7 +783,7 @@ static void test_override_scsi_2_controllers(void)
 add_scsi_disk(args, 1, 0, 0, 1, 0, 9000, 120, 30);
 add_scsi_disk(args, 2, 1, 0, 0, 1, 1, 0, 0);
 add_scsi_disk(args, 3, 1, 0, 1, 2, 0, 1, 0);
-test_override(args, expected);
+test_override(args, "pc", expected);
 }
 
 static void test_override_virtio_blk(void)
@@ -797,7 +798,7 @@ static void test_override_virtio_blk(void)
 add_drive_with_mbr(args, empty_mbr, 1);
 add_virtio_disk(args, 0, "pci.0", 3, 1, 120, 30);
 add_virtio_disk(args, 1, "pci.0", 4, 9000, 120, 30);
-test_override(args, expected);
+test_override(args, "pc", expected);
 }
 
 static void test_override_zero_chs(void)
@@ -808,46 +809,28 @@ static void test_override_zero_chs(void)
 };
 add_drive_with_mbr(args, empty_mbr, 1);
 add_ide_disk(args, 0, 1, 1, 0, 0, 0);
-test_override(args, expected);
+test_override(args, "pc", expected);
 }
 
-static void test_override_scsi_hot_unplug(void)
+static void test_override_hot_unplug(TestArgs *args, const char *devid,
+ CHSResult expected[], CHSResult 
expected2[])
 {
 QTestState *qts;
 char *joined_args;
 QFWCFG *fw_cfg;
 QDict *response;
 int i;
-TestArgs *args = create_args();
-CHSResult expected[] = {
-{"/pci@i0cf8/scsi@2/channel@0/disk@0,0", {1, 120, 30} },
-{"/pci@i0cf8/scsi@2/channel@0/disk@1,0", {20, 20, 20} },
-{NULL, {0, 0, 0} }
-};
-CHSResult expected2[] = {
-{"/pci@i0cf8/scsi@2/channel@0/disk@1,0", {20, 20, 20} },
-{NULL, {0, 0, 0} }
-};
-add_drive_with_mbr(args, empty_mbr, 1);
-add_drive_with_mbr(args, empty_mbr, 1);
-add_scsi_controller(args, "virtio-scsi-pci", "pci.0", 2);
-add_scsi_disk(args, 0, 0, 0, 0, 0, 1, 120, 30);
-add_scsi_disk(args, 1, 0, 0, 1, 0, 20, 20, 20);
 
 joined_args = g_strjoinv(" ", args->argv);
 
-qts = qtest_initf("-machine pc %s", joined_args);
+qts = qtest_initf("%s", joined_args);
 fw_cfg = pc_fw_cfg_init(qts);
 
 read_bootdevices(fw_cfg, expected);
 
 /* unplug device an restart */
-response = qtest_qmp(qts,
- "{ 'execute': 'device_del',"
- "  'arguments': {'id': 'scsi-disk0' }}");
-g_assert(response);
-g_assert(!qdict_haskey(response, "error"));
-qobject_unref(response);
+qtest_qmp_device_del_send(qts, devid);
+
 response = qtest_qmp(qts,
  "{ 'execute': 'system_reset', 'arguments': { }}");
 g_assert(response);
@@ -872,13 +855,32 @@ static void test_override_scsi_hot_unplug(void)
 g_free(args);
 }
 
+static void test_override_scsi_hot_unplug(void)
+{
+TestArgs *args = create_args();
+CHSResult expected[] = {
+{"/pci@i0cf8/scsi@2/channel@0/disk@0,0", {1, 120, 30} },
+{"/pci@i0cf8/scsi@2/channel@0/disk@1,0", {20, 20, 20} },
+{NULL, {0, 0, 0} }
+};
+CHSResult expected2[] = {
+{"/pci@i0cf8/scsi@2/channel@0/disk@1,0", {20, 20, 20} },
+{NULL, {0, 0, 0} }
+};
+add_drive_with_mbr(args, empty_mbr, 1);
+add_drive_with_mbr(a

[PULL 03/16] tests/x86: add helper qtest_qmp_device_del_send()

[Thomas Huth]

From: Michael Labiuk 

Move sending 'device_del' command to separate function.
Function can be used in case of addition action is needed to start
actual removing device after sending command.

Signed-off-by: Michael Labiuk 
Message-Id: <20220929223547.1429580-2-michael.lab...@virtuozzo.com>
Reviewed-by: Thomas Huth 
[thuth: Fixed typo]
Signed-off-by: Thomas Huth 
---
 tests/qtest/libqtest.h | 10 ++
 tests/qtest/device-plug-test.c | 15 ++-
 tests/qtest/drive_del-test.c   |  6 +-
 tests/qtest/libqos/pci-pc.c|  8 +---
 tests/qtest/libqtest.c | 16 ++--
 5 files changed, 24 insertions(+), 31 deletions(-)

diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 3abc75964d..65c040e504 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -761,12 +761,22 @@ void qtest_qmp_device_add(QTestState *qts, const char 
*driver, const char *id,
 void qtest_qmp_add_client(QTestState *qts, const char *protocol, int fd);
 #endif /* _WIN32 */
 
+/**
+ * qtest_qmp_device_del_send:
+ * @qts: QTestState instance to operate on
+ * @id: Identification string
+ *
+ * Generic hot-unplugging test via the device_del QMP command.
+ */
+void qtest_qmp_device_del_send(QTestState *qts, const char *id);
+
 /**
  * qtest_qmp_device_del:
  * @qts: QTestState instance to operate on
  * @id: Identification string
  *
  * Generic hot-unplugging test via the device_del QMP command.
+ * Waiting for command completion event.
  */
 void qtest_qmp_device_del(QTestState *qts, const char *id);
 
diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
index e595b45b66..3841de1b8c 100644
--- a/tests/qtest/device-plug-test.c
+++ b/tests/qtest/device-plug-test.c
@@ -15,17 +15,6 @@
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qstring.h"
 
-static void device_del(QTestState *qtest, const char *id)
-{
-QDict *resp;
-
-resp = qtest_qmp(qtest,
- "{'execute': 'device_del', 'arguments': { 'id': %s } }", 
id);
-
-g_assert(qdict_haskey(resp, "return"));
-qobject_unref(resp);
-}
-
 static void system_reset(QTestState *qtest)
 {
 QDict *resp;
@@ -68,7 +57,7 @@ static void process_device_remove(QTestState *qtest, const 
char *id)
  * be processed. However during system reset, the removal will be
  * handled, removing the device.
  */
-device_del(qtest, id);
+qtest_qmp_device_del_send(qtest, id);
 system_reset(qtest);
 wait_device_deleted_event(qtest, id);
 }
@@ -112,7 +101,7 @@ static void test_ccw_unplug(void)
 {
 QTestState *qtest = qtest_initf("-device virtio-balloon-ccw,id=dev0");
 
-device_del(qtest, "dev0");
+qtest_qmp_device_del_send(qtest, "dev0");
 wait_device_deleted_event(qtest, "dev0");
 
 qtest_quit(qtest);
diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
index 5e6d58b4dd..467e752b0d 100644
--- a/tests/qtest/drive_del-test.c
+++ b/tests/qtest/drive_del-test.c
@@ -143,11 +143,7 @@ static void device_del(QTestState *qts, bool and_reset)
 {
 QDict *response;
 
-response = qtest_qmp(qts, "{'execute': 'device_del',"
- " 'arguments': { 'id': 'dev0' } }");
-g_assert(response);
-g_assert(qdict_haskey(response, "return"));
-qobject_unref(response);
+qtest_qmp_device_del_send(qts, "dev0");
 
 if (and_reset) {
 response = qtest_qmp(qts, "{'execute': 'system_reset' }");
diff --git a/tests/qtest/libqos/pci-pc.c b/tests/qtest/libqos/pci-pc.c
index 81c2c055ca..96046287ac 100644
--- a/tests/qtest/libqos/pci-pc.c
+++ b/tests/qtest/libqos/pci-pc.c
@@ -179,13 +179,7 @@ void qpci_free_pc(QPCIBus *bus)
 
 void qpci_unplug_acpi_device_test(QTestState *qts, const char *id, uint8_t 
slot)
 {
-QDict *response;
-
-response = qtest_qmp(qts, "{'execute': 'device_del',"
-  " 'arguments': {'id': %s}}", id);
-g_assert(response);
-g_assert(!qdict_haskey(response, "error"));
-qobject_unref(response);
+qtest_qmp_device_del_send(qts, id);
 
 qtest_outl(qts, ACPI_PCIHP_ADDR + PCI_EJ_BASE, 1 << slot);
 
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 4f4b2d6477..7b6152807b 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1371,15 +1371,19 @@ void qtest_qmp_add_client(QTestState *qts, const char 
*protocol, int fd)
  *
  * {"return": {}}
  */
-void qtest_qmp_device_del(QTestState *qts, const char *id)
+void qtest_qmp_device_del_send(QTestState *qts, const char *id)
 {
-QDict *rsp;
-
-rsp = qtest_qmp(qts, "{'execute': 'device_del', 'arguments': {'id': %s}}",
-id);
-
+QDict *rsp = qtest_qmp(qts, "{'execute': 'device_del', "
+"'arguments': {'id': %s}}", id);
+g_assert(rsp);
 g_assert(qdict_haskey(rsp, "return"));
+g_assert(!qdict_haskey(rsp, "error"));
 qobject_unref(rsp);
+}
+
+void qtest_qmp_device_del(QTestState *qts, const char *id)
+{
+qte

Re: [PULL 1/1] Revert "configure: build ROMs with container-based cross compilers"

[Paolo Bonzini]

On 10/12/22 11:08, Daniel P. Berrangé wrote:

   - Tailor downloads wrt the target list configured

This is already done.


Where's the patch for that, I hadn't noticed it being posted yet ?


Oh, I thought it was done already before the introduction of containers 
but only tests/tcg is filtered.  But it should be as simple as


diff --git a/configure b/configure
index 00acb69cc9..7a26ce39b3 100755
--- a/configure
+++ b/configure
@@ -1269,6 +1269,11 @@ probe_target_compiler() {
   container_cross_ranlib=
   container_cross_strip=

+  case " $target_list " in
+" $1 ") ;;
+*) return 1 ;;
+  esac
+
   target_arch=${1%%-*}
   case $target_arch in
 aarch64) container_hosts="x86_64 aarch64" ;;


Paolo

[PULL 06/16] tests/x86: Add 'q35' machine type to override-tests in hd-geo-test

[Thomas Huth]

From: Michael Labiuk 

Signed-off-by: Michael Labiuk 
Message-Id: <20220929223547.1429580-5-michael.lab...@virtuozzo.com>
Signed-off-by: Thomas Huth 
---
 tests/qtest/hd-geo-test.c | 97 +++
 1 file changed, 97 insertions(+)

diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
index 61f4c24b81..278464c379 100644
--- a/tests/qtest/hd-geo-test.c
+++ b/tests/qtest/hd-geo-test.c
@@ -741,6 +741,27 @@ static void test_override_ide(void)
 test_override(args, "pc", expected);
 }
 
+static void test_override_sata(void)
+{
+TestArgs *args = create_args();
+CHSResult expected[] = {
+{"/pci@i0cf8/pci8086,2922@1f,2/drive@0/disk@0", {1, 120, 30} },
+{"/pci@i0cf8/pci8086,2922@1f,2/drive@1/disk@0", {9000, 120, 30} },
+{"/pci@i0cf8/pci8086,2922@1f,2/drive@2/disk@0", {0, 1, 1} },
+{"/pci@i0cf8/pci8086,2922@1f,2/drive@3/disk@0", {1, 0, 0} },
+{NULL, {0, 0, 0} }
+};
+add_drive_with_mbr(args, empty_mbr, 1);
+add_drive_with_mbr(args, empty_mbr, 1);
+add_drive_with_mbr(args, empty_mbr, 1);
+add_drive_with_mbr(args, empty_mbr, 1);
+add_ide_disk(args, 0, 0, 0, 1, 120, 30);
+add_ide_disk(args, 1, 1, 0, 9000, 120, 30);
+add_ide_disk(args, 2, 2, 0, 0, 1, 1);
+add_ide_disk(args, 3, 3, 0, 1, 0, 0);
+test_override(args, "q35", expected);
+}
+
 static void test_override_scsi(void)
 {
 TestArgs *args = create_args();
@@ -763,6 +784,42 @@ static void test_override_scsi(void)
 test_override(args, "pc", expected);
 }
 
+static void setup_pci_bridge(TestArgs *args, const char *id, const char 
*rootid)
+{
+
+char *root, *br;
+root = g_strdup_printf("-device pcie-root-port,id=%s", rootid);
+br = g_strdup_printf("-device pcie-pci-bridge,bus=%s,id=%s", rootid, id);
+
+args->argc = append_arg(args->argc, args->argv, ARGV_SIZE, root);
+args->argc = append_arg(args->argc, args->argv, ARGV_SIZE, br);
+}
+
+static void test_override_scsi_q35(void)
+{
+TestArgs *args = create_args();
+CHSResult expected[] = {
+{   "/pci@i0cf8/pci-bridge@1/scsi@3/channel@0/disk@0,0",
+{1, 120, 30}
+},
+{"/pci@i0cf8/pci-bridge@1/scsi@3/channel@0/disk@1,0", {9000, 120, 30} 
},
+{"/pci@i0cf8/pci-bridge@1/scsi@3/channel@0/disk@2,0", {1, 0, 0} },
+{"/pci@i0cf8/pci-bridge@1/scsi@3/channel@0/disk@3,0", {0, 1, 0} },
+{NULL, {0, 0, 0} }
+};
+add_drive_with_mbr(args, empty_mbr, 1);
+add_drive_with_mbr(args, empty_mbr, 1);
+add_drive_with_mbr(args, empty_mbr, 1);
+add_drive_with_mbr(args, empty_mbr, 1);
+setup_pci_bridge(args, "pcie.0", "br");
+add_scsi_controller(args, "lsi53c895a", "br", 3);
+add_scsi_disk(args, 0, 0, 0, 0, 0, 1, 120, 30);
+add_scsi_disk(args, 1, 0, 0, 1, 0, 9000, 120, 30);
+add_scsi_disk(args, 2, 0, 0, 2, 0, 1, 0, 0);
+add_scsi_disk(args, 3, 0, 0, 3, 0, 0, 1, 0);
+test_override(args, "q35", expected);
+}
+
 static void test_override_scsi_2_controllers(void)
 {
 TestArgs *args = create_args();
@@ -801,6 +858,22 @@ static void test_override_virtio_blk(void)
 test_override(args, "pc", expected);
 }
 
+static void test_override_virtio_blk_q35(void)
+{
+TestArgs *args = create_args();
+CHSResult expected[] = {
+{"/pci@i0cf8/pci-bridge@1/scsi@3/disk@0,0", {1, 120, 30} },
+{"/pci@i0cf8/pci-bridge@1/scsi@4/disk@0,0", {9000, 120, 30} },
+{NULL, {0, 0, 0} }
+};
+add_drive_with_mbr(args, empty_mbr, 1);
+add_drive_with_mbr(args, empty_mbr, 1);
+setup_pci_bridge(args, "pcie.0", "br");
+add_virtio_disk(args, 0, "br", 3, 1, 120, 30);
+add_virtio_disk(args, 1, "br", 4, 9000, 120, 30);
+test_override(args, "q35", expected);
+}
+
 static void test_override_zero_chs(void)
 {
 TestArgs *args = create_args();
@@ -812,6 +885,17 @@ static void test_override_zero_chs(void)
 test_override(args, "pc", expected);
 }
 
+static void test_override_zero_chs_q35(void)
+{
+TestArgs *args = create_args();
+CHSResult expected[] = {
+{NULL, {0, 0, 0} }
+};
+add_drive_with_mbr(args, empty_mbr, 1);
+add_ide_disk(args, 0, 0, 0, 0, 0, 0);
+test_override(args, "q35", expected);
+}
+
 static void test_override_hot_unplug(TestArgs *args, const char *devid,
  CHSResult expected[], CHSResult 
expected2[])
 {
@@ -944,6 +1028,19 @@ int main(int argc, char **argv)
test_override_scsi_hot_unplug);
 qtest_add_func("hd-geo/override/virtio_hot_unplug",
test_override_virtio_hot_unplug);
+
+if (qtest_has_machine("q35")) {
+qtest_add_func("hd-geo/override/sata", test_override_sata);
+qtest_add_func("hd-geo/override/virtio_blk_q35",
+   test_override_virtio_blk_q35);
+qtest_add_func("hd-geo/override/zero_chs_q35",
+   test_ove

[PULL 09/16] tests/x86: replace snprint() by g_strdup_printf() in drive_del-test

[Thomas Huth]

From: Michael Labiuk 

Using g_autofree char* and  g_strdup_printf(...) instead of ugly
snprintf on stack array.

Signed-off-by: Michael Labiuk 
Message-Id: <20220929223547.1429580-8-michael.lab...@virtuozzo.com>
Reviewed-by: Thomas Huth 
Signed-off-by: Thomas Huth 
---
 tests/qtest/drive_del-test.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
index 44b9578801..106c613f4f 100644
--- a/tests/qtest/drive_del-test.c
+++ b/tests/qtest/drive_del-test.c
@@ -123,12 +123,10 @@ static const char *qvirtio_get_dev_type(void)
 
 static void device_add(QTestState *qts)
 {
-QDict *response;
-char driver[32];
-snprintf(driver, sizeof(driver), "virtio-blk-%s",
- qvirtio_get_dev_type());
-
-response = qtest_qmp(qts, "{'execute': 'device_add',"
+g_autofree char *driver = g_strdup_printf("virtio-blk-%s",
+  qvirtio_get_dev_type());
+QDict *response =
+   qtest_qmp(qts, "{'execute': 'device_add',"
   " 'arguments': {"
   "   'driver': %s,"
   "   'drive': 'drive0',"
-- 
2.31.1

[PULL 04/16] tests/x86: Add subtest with 'q35' machine type to device-plug-test

[Thomas Huth]

From: Michael Labiuk 

Configure pci bridge setting to plug pci device and unplug.

Signed-off-by: Michael Labiuk 
Message-Id: <20220929223547.1429580-3-michael.lab...@virtuozzo.com>
Reviewed-by: Thomas Huth 
Signed-off-by: Thomas Huth 
---
 tests/qtest/device-plug-test.c | 41 ++
 1 file changed, 41 insertions(+)

diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
index 3841de1b8c..3f44f731d1 100644
--- a/tests/qtest/device-plug-test.c
+++ b/tests/qtest/device-plug-test.c
@@ -79,6 +79,19 @@ static void test_pci_unplug_request(void)
 qtest_quit(qtest);
 }
 
+static void test_q35_pci_unplug_request(void)
+{
+
+QTestState *qtest = qtest_initf("-machine q35 "
+"-device pcie-root-port,id=p1 "
+"-device pcie-pci-bridge,bus=p1,id=b1 "
+"-device virtio-mouse-pci,bus=b1,id=dev0");
+
+process_device_remove(qtest, "dev0");
+
+qtest_quit(qtest);
+}
+
 static void test_pci_unplug_json_request(void)
 {
 const char *arch = qtest_get_arch();
@@ -97,6 +110,27 @@ static void test_pci_unplug_json_request(void)
 qtest_quit(qtest);
 }
 
+static void test_q35_pci_unplug_json_request(void)
+{
+const char *port = "-device '{\"driver\": \"pcie-root-port\", "
+  "\"id\": \"p1\"}'";
+
+const char *bridge = "-device '{\"driver\": \"pcie-pci-bridge\", "
+   "\"id\": \"b1\", "
+   "\"bus\": \"p1\"}'";
+
+const char *device = "-device '{\"driver\": \"virtio-mouse-pci\", "
+   "\"bus\": \"b1\", "
+   "\"id\": \"dev0\"}'";
+
+QTestState *qtest = qtest_initf("-machine q35 %s %s %s",
+port, bridge, device);
+
+process_device_remove(qtest, "dev0");
+
+qtest_quit(qtest);
+}
+
 static void test_ccw_unplug(void)
 {
 QTestState *qtest = qtest_initf("-device virtio-balloon-ccw,id=dev0");
@@ -176,5 +210,12 @@ int main(int argc, char **argv)
test_spapr_phb_unplug_request);
 }
 
+if (!strcmp(arch, "x86_64") && qtest_has_machine("q35")) {
+qtest_add_func("/device-plug/q35-pci-unplug-request",
+   test_q35_pci_unplug_request);
+qtest_add_func("/device-plug/q35-pci-unplug-json-request",
+   test_q35_pci_unplug_json_request);
+}
+
 return g_test_run();
 }
-- 
2.31.1

[PULL 01/16] qtest: "-display none" is set in qtest_init()

[Thomas Huth]

From: Juan Quintela 

So we don't need to set anywhere else.

Signed-off-by: Juan Quintela 
[thuth: Drop changes in tests/qtest/fuzz/ since the fuzzers still need this]
Message-Id: <20220902165126.1482-2-quint...@redhat.com>
Signed-off-by: Thomas Huth 
---
 tests/qtest/bios-tables-test.c  | 2 +-
 tests/qtest/fuzz-lsi53c895a-test.c  | 2 +-
 tests/qtest/fuzz-megasas-test.c | 2 +-
 tests/qtest/fuzz-sb16-test.c| 6 +++---
 tests/qtest/fuzz-sdcard-test.c  | 6 +++---
 tests/qtest/fuzz-virtio-scsi-test.c | 2 +-
 tests/qtest/fuzz-xlnx-dp-test.c | 2 +-
 7 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 2ebeb530b2..e6096e7f73 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -725,7 +725,7 @@ static char *test_acpi_create_args(test_data *data, const 
char *params,
 }
 } else {
 args = g_strdup_printf("-machine %s %s -accel tcg "
-"-net none -display none %s "
+"-net none %s "
 "-drive id=hd0,if=none,file=%s,format=raw "
 "-device %s,drive=hd0 ",
  data->machine, data->tcg_only ? "" : "-accel kvm",
diff --git a/tests/qtest/fuzz-lsi53c895a-test.c 
b/tests/qtest/fuzz-lsi53c895a-test.c
index 434c16bf42..392a7ae7ed 100644
--- a/tests/qtest/fuzz-lsi53c895a-test.c
+++ b/tests/qtest/fuzz-lsi53c895a-test.c
@@ -21,7 +21,7 @@ static void test_lsi_do_msgout_cancel_req(void)
 return;
 }
 
-s = qtest_init("-M q35 -m 2G -display none -nodefaults "
+s = qtest_init("-M q35 -m 2G -nodefaults "
"-device lsi53c895a,id=scsi "
"-device scsi-hd,drive=disk0 "
"-drive file=null-co://,id=disk0,if=none,format=raw");
diff --git a/tests/qtest/fuzz-megasas-test.c b/tests/qtest/fuzz-megasas-test.c
index 287fe19fc7..8d7ed3723a 100644
--- a/tests/qtest/fuzz-megasas-test.c
+++ b/tests/qtest/fuzz-megasas-test.c
@@ -40,7 +40,7 @@ static void test_lp1878263_megasas_zero_iov_cnt(void)
  */
 static void test_gitlab_issue521_megasas_sgl_ovf(void)
 {
-QTestState *s = qtest_init("-display none -m 32M -machine q35 "
+QTestState *s = qtest_init("-m 32M -machine q35 "
"-nodefaults -device megasas "
"-device scsi-cd,drive=null0 "
"-blockdev "
diff --git a/tests/qtest/fuzz-sb16-test.c b/tests/qtest/fuzz-sb16-test.c
index add2a2ad39..fc445b1871 100644
--- a/tests/qtest/fuzz-sb16-test.c
+++ b/tests/qtest/fuzz-sb16-test.c
@@ -15,7 +15,7 @@
  */
 static void test_fuzz_sb16_0x1c(void)
 {
-QTestState *s = qtest_init("-M q35 -display none "
+QTestState *s = qtest_init("-M q35 "
"-device sb16,audiodev=snd0 "
"-audiodev none,id=snd0");
 qtest_outw(s, 0x22c, 0x41);
@@ -27,7 +27,7 @@ static void test_fuzz_sb16_0x1c(void)
 
 static void test_fuzz_sb16_0x91(void)
 {
-QTestState *s = qtest_init("-M pc -display none "
+QTestState *s = qtest_init("-M pc "
"-device sb16,audiodev=none "
"-audiodev id=none,driver=none");
 qtest_outw(s, 0x22c, 0xf141);
@@ -43,7 +43,7 @@ static void test_fuzz_sb16_0x91(void)
  */
 static void test_fuzz_sb16_0xd4(void)
 {
-QTestState *s = qtest_init("-M pc -display none "
+QTestState *s = qtest_init("-M pc "
"-device sb16,audiodev=none "
"-audiodev id=none,driver=none");
 qtest_outb(s, 0x22c, 0x41);
diff --git a/tests/qtest/fuzz-sdcard-test.c b/tests/qtest/fuzz-sdcard-test.c
index e7fd818148..cd134cdf55 100644
--- a/tests/qtest/fuzz-sdcard-test.c
+++ b/tests/qtest/fuzz-sdcard-test.c
@@ -18,7 +18,7 @@ static void oss_fuzz_29225(void)
 {
 QTestState *s;
 
-s = qtest_init(" -display none -m 512m -nodefaults -nographic"
+s = qtest_init(" -m 512m -nodefaults -nographic"
" -device sdhci-pci,sd-spec-version=3"
" -device sd-card,drive=d0"
" -drive if=none,index=0,file=null-co://,format=raw,id=d0");
@@ -61,7 +61,7 @@ static void oss_fuzz_36217(void)
 {
 QTestState *s;
 
-s = qtest_init(" -display none -m 32 -nodefaults -nographic"
+s = qtest_init(" -m 32 -nodefaults -nographic"
" -device sdhci-pci,sd-spec-version=3 "
"-device sd-card,drive=d0 "
"-drive if=none,index=0,file=null-co://,format=raw,id=d0");
@@ -95,7 +95,7 @@ static void oss_fuzz_36391(void)
 {
 QTestState *s;
 
-s = qtest_init(" -display none -m 512M -nodefaults -nographic"
+s = qtest_init(" -m 512M -nodefaults -nographic"
" -device sdhci-pci,sd-spec-version=3"
" -device sd-card,drive=drv"
" -drive 
if=none,index=0,file=null-co://,format=raw,id=drv");
diff --git a

[PULL 07/16] tests/x86: Add 'q35' machine type to hotplug hd-geo-test

[Thomas Huth]

From: Michael Labiuk 

Add pci bridge setting to test hotplug.
Duplicate tests for plugging scsi and virtio devices for q35 machine type.

Signed-off-by: Michael Labiuk 
Message-Id: <20220929223547.1429580-6-michael.lab...@virtuozzo.com>
Signed-off-by: Thomas Huth 
---
 tests/qtest/hd-geo-test.c | 76 ++-
 1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
index 278464c379..4a7628077b 100644
--- a/tests/qtest/hd-geo-test.c
+++ b/tests/qtest/hd-geo-test.c
@@ -963,6 +963,42 @@ static void test_override_scsi_hot_unplug(void)
 test_override_hot_unplug(args, "scsi-disk0", expected, expected2);
 }
 
+static void test_override_scsi_hot_unplug_q35(void)
+{
+TestArgs *args = create_args();
+CHSResult expected[] = {
+{
+"/pci@i0cf8/pci-bridge@1/pci-bridge@0/scsi@2/channel@0/disk@0,0",
+{1, 120, 30}
+},
+{
+"/pci@i0cf8/pci-bridge@1/pci-bridge@0/scsi@2/channel@0/disk@1,0",
+{20, 20, 20}
+},
+{NULL, {0, 0, 0} }
+};
+CHSResult expected2[] = {
+{
+"/pci@i0cf8/pci-bridge@1/pci-bridge@0/scsi@2/channel@0/disk@1,0",
+{20, 20, 20}
+},
+{NULL, {0, 0, 0} }
+};
+
+args->argc = append_arg(args->argc, args->argv, ARGV_SIZE,
+g_strdup("-device pcie-root-port,id=p0 "
+ "-device pcie-pci-bridge,bus=p0,id=b1 "
+ "-machine q35"));
+
+add_drive_with_mbr(args, empty_mbr, 1);
+add_drive_with_mbr(args, empty_mbr, 1);
+add_scsi_controller(args, "virtio-scsi-pci", "b1", 2);
+add_scsi_disk(args, 0, 0, 0, 0, 0, 1, 120, 30);
+add_scsi_disk(args, 1, 0, 0, 1, 0, 20, 20, 20);
+
+test_override_hot_unplug(args, "scsi-disk0", expected, expected2);
+}
+
 static void test_override_virtio_hot_unplug(void)
 {
 TestArgs *args = create_args();
@@ -986,6 +1022,41 @@ static void test_override_virtio_hot_unplug(void)
 test_override_hot_unplug(args, "virtio-disk0", expected, expected2);
 }
 
+static void test_override_virtio_hot_unplug_q35(void)
+{
+TestArgs *args = create_args();
+CHSResult expected[] = {
+{
+"/pci@i0cf8/pci-bridge@1/pci-bridge@0/scsi@2/disk@0,0",
+{1, 120, 30}
+},
+{
+"/pci@i0cf8/pci-bridge@1/pci-bridge@0/scsi@3/disk@0,0",
+{20, 20, 20}
+},
+{NULL, {0, 0, 0} }
+};
+CHSResult expected2[] = {
+{
+"/pci@i0cf8/pci-bridge@1/pci-bridge@0/scsi@3/disk@0,0",
+{20, 20, 20}
+},
+{NULL, {0, 0, 0} }
+};
+
+args->argc = append_arg(args->argc, args->argv, ARGV_SIZE,
+g_strdup("-device pcie-root-port,id=p0 "
+ "-device pcie-pci-bridge,bus=p0,id=b1 "
+ "-machine q35"));
+
+add_drive_with_mbr(args, empty_mbr, 1);
+add_drive_with_mbr(args, empty_mbr, 1);
+add_virtio_disk(args, 0, "b1", 2, 1, 120, 30);
+add_virtio_disk(args, 1, "b1", 3, 20, 20, 20);
+
+test_override_hot_unplug(args, "virtio-disk0", expected, expected2);
+}
+
 int main(int argc, char **argv)
 {
 Backend i;
@@ -1035,11 +1106,14 @@ int main(int argc, char **argv)
test_override_virtio_blk_q35);
 qtest_add_func("hd-geo/override/zero_chs_q35",
test_override_zero_chs_q35);
-
 if (qtest_has_device("lsi53c895a")) {
 qtest_add_func("hd-geo/override/scsi_q35",
test_override_scsi_q35);
 }
+qtest_add_func("hd-geo/override/scsi_hot_unplug_q35",
+   test_override_scsi_hot_unplug_q35);
+qtest_add_func("hd-geo/override/virtio_hot_unplug_q35",
+   test_override_virtio_hot_unplug_q35);
 }
 } else {
 g_test_message("QTEST_QEMU_IMG not set or qemu-img missing; "
-- 
2.31.1

[PULL 08/16] tests/x86: Fix comment typo in drive_del-test

[Thomas Huth]

From: Michael Labiuk 

Signed-off-by: Michael Labiuk 
Message-Id: <20220929223547.1429580-7-michael.lab...@virtuozzo.com>
Reviewed-by: Thomas Huth 
Signed-off-by: Thomas Huth 
---
 tests/qtest/drive_del-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
index 467e752b0d..44b9578801 100644
--- a/tests/qtest/drive_del-test.c
+++ b/tests/qtest/drive_del-test.c
@@ -327,7 +327,7 @@ static void test_blockdev_add_device_add_and_del(void)
 qts = qtest_init(machine_addition);
 
 /*
- * blockdev_add/device_add and device_del.  The it drive is used by a
+ * blockdev_add/device_add and device_del. The drive is used by a
  * device that unplugs after reset, but it doesn't go away.
  */
 blockdev_add_with_media(qts);
-- 
2.31.1

[PULL 02/16] tests/migration: remove the unused local variable

[Thomas Huth]

From: dinglimin 

Remove the unused local variable "records".

Signed-off-by: dinglimin 
Reviewed-by: Ján Tomko 
Message-Id: <20220928080555.2263-1-dingli...@cmss.chinamobile.com>
Signed-off-by: Thomas Huth 
---
 tests/migration/guestperf/engine.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/migration/guestperf/engine.py 
b/tests/migration/guestperf/engine.py
index 87a6ab2009..59fca2c70b 100644
--- a/tests/migration/guestperf/engine.py
+++ b/tests/migration/guestperf/engine.py
@@ -65,7 +65,6 @@ def _vcpu_timing(self, pid, tid_list):
 return records
 
 def _cpu_timing(self, pid):
-records = []
 now = time.time()
 
 jiffies_per_sec = os.sysconf(os.sysconf_names['SC_CLK_TCK'])
-- 
2.31.1

[PULL 11/16] tests/x86: Add 'q35' machine type to ivshmem-test

[Thomas Huth]

From: Michael Labiuk 

Configure pci bridge setting to test ivshmem on 'q35'.

Signed-off-by: Michael Labiuk 
Message-Id: <20220929223547.1429580-10-michael.lab...@virtuozzo.com>
Reviewed-by: Thomas Huth 
Signed-off-by: Thomas Huth 
---
 tests/qtest/ivshmem-test.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/tests/qtest/ivshmem-test.c b/tests/qtest/ivshmem-test.c
index 9611d05eb5..cd550c8935 100644
--- a/tests/qtest/ivshmem-test.c
+++ b/tests/qtest/ivshmem-test.c
@@ -378,6 +378,20 @@ static void test_ivshmem_server(void)
 close(thread.pipe[0]);
 }
 
+static void test_ivshmem_hotplug_q35(void)
+{
+QTestState *qts = qtest_init("-object memory-backend-ram,size=1M,id=mb1 "
+ "-device pcie-root-port,id=p1 "
+ "-device pcie-pci-bridge,bus=p1,id=b1 "
+ "-machine q35");
+
+qtest_qmp_device_add(qts, "ivshmem-plain", "iv1",
+ "{'memdev': 'mb1', 'bus': 'b1'}");
+qtest_qmp_device_del_send(qts, "iv1");
+
+qtest_quit(qts);
+}
+
 #define PCI_SLOT_HP 0x06
 
 static void test_ivshmem_hotplug(void)
@@ -469,6 +483,7 @@ int main(int argc, char **argv)
 {
 int ret, fd;
 gchar dir[] = "/tmp/ivshmem-test.XX";
+const char *arch = qtest_get_arch();
 
 g_test_init(&argc, &argv, NULL);
 
@@ -494,6 +509,9 @@ int main(int argc, char **argv)
 qtest_add_func("/ivshmem/pair", test_ivshmem_pair);
 qtest_add_func("/ivshmem/server", test_ivshmem_server);
 }
+if (!strcmp(arch, "x86_64") && qtest_has_machine("q35")) {
+qtest_add_func("/ivshmem/hotplug-q35", test_ivshmem_hotplug_q35);
+}
 
 out:
 ret = g_test_run();
-- 
2.31.1

[PULL 13/16] qtest: start a VNC test

[Thomas Huth]

From: Marc-André Lureau 

This is some of the simplest test we could perform, it simply connects
to the VNC server via passed-in socket FDs and checks the connection can
be established.

Another series will make this test work on Windows as well.

As always, more tests can be added later! :)

Signed-off-by: Marc-André Lureau 
Message-Id: <20221006130513.2683873-1-marcandre.lur...@redhat.com>
Signed-off-by: Thomas Huth 
---
 tests/qtest/vnc-display-test.c | 103 +
 tests/qtest/meson.build|   8 ++-
 2 files changed, 110 insertions(+), 1 deletion(-)
 create mode 100644 tests/qtest/vnc-display-test.c

diff --git a/tests/qtest/vnc-display-test.c b/tests/qtest/vnc-display-test.c
new file mode 100644
index 00..e2a9d682bb
--- /dev/null
+++ b/tests/qtest/vnc-display-test.c
@@ -0,0 +1,103 @@
+/*
+ * VNC display tests
+ *
+ * Copyright (c) 2022 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/sockets.h"
+#include "libqtest.h"
+#include 
+#include 
+
+typedef struct Test {
+QTestState *qts;
+VncConnection *conn;
+GMainLoop *loop;
+} Test;
+
+static void on_vnc_error(VncConnection* self,
+ const char* msg)
+{
+g_error("vnc-error: %s", msg);
+}
+
+static void on_vnc_auth_failure(VncConnection *self,
+const char *msg)
+{
+g_error("vnc-auth-failure: %s", msg);
+}
+
+static bool
+test_setup(Test *test)
+{
+#ifdef WIN32
+g_test_skip("Not supported on Windows yet");
+return false;
+#else
+int pair[2];
+
+test->qts = qtest_init("-vnc none -name vnc-test");
+
+g_assert_cmpint(qemu_socketpair(AF_UNIX, SOCK_STREAM, 0, pair), ==, 0);
+
+qtest_qmp_add_client(test->qts, "vnc", pair[1]);
+
+test->conn = vnc_connection_new();
+g_signal_connect(test->conn, "vnc-error",
+ G_CALLBACK(on_vnc_error), NULL);
+g_signal_connect(test->conn, "vnc-auth-failure",
+ G_CALLBACK(on_vnc_auth_failure), NULL);
+vnc_connection_set_auth_type(test->conn, VNC_CONNECTION_AUTH_NONE);
+vnc_connection_open_fd(test->conn, pair[0]);
+
+test->loop = g_main_loop_new(NULL, FALSE);
+return true;
+#endif
+}
+
+static void
+test_vnc_basic_on_vnc_initialized(VncConnection *self,
+ Test *test)
+{
+const char *name = vnc_connection_get_name(test->conn);
+
+g_assert_cmpstr(name, ==, "QEMU (vnc-test)");
+g_main_loop_quit(test->loop);
+}
+
+static void
+test_vnc_basic(void)
+{
+Test test;
+
+if (!test_setup(&test)) {
+return;
+}
+
+g_signal_connect(test.conn, "vnc-initialized",
+ G_CALLBACK(test_vnc_basic_on_vnc_initialized), &test);
+
+g_main_loop_run(test.loop);
+
+qtest_quit(test.qts);
+g_object_unref(test.conn);
+g_main_loop_unref(test.loop);
+}
+
+int
+main(int argc, char **argv)
+{
+if (getenv("GTK_VNC_DEBUG")) {
+vnc_util_set_debug(true);
+}
+
+g_test_init(&argc, &argv, NULL);
+
+qtest_add_func("/vnc-display/basic", test_vnc_basic);
+
+return g_test_run();
+}
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 455f1bbb7e..c07a5b1a5f 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -306,8 +306,14 @@ qtests = {
   'vmgenid-test': files('boot-sector.c', 'acpi-utils.c'),
 }
 
+gvnc = dependency('gvnc-1.0', required: false)
+if gvnc.found()
+  qtests += {'vnc-display-test': [gvnc]}
+  qtests_generic += [ 'vnc-display-test' ]
+endif
+
 if dbus_display
-qtests += {'dbus-display-test': [dbus_display1, gio]}
+  qtests += {'dbus-display-test': [dbus_display1, gio]}
 endif
 
 qtest_executables = {}
-- 
2.31.1

[PULL 12/16] tests/avocado: Add missing require_netdev('user') checks

[Thomas Huth]

From: Peter Maydell 

Some avocado tests fail if QEMU was built without libslirp. Add
require_netdev('user') checks where necessary:

These tests try to ping 10.0.2.2 and expect it to succeed:
  boot_linux_console.py:BootLinuxConsole.test_arm_emcraft_sf2
  boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd
  ppc_bamboo.py:BambooMachine.test_ppc_bamboo

These tests run a commandline that includes '-net user':
  machine_aspeed.py:AST2x00Machine.test_arm_ast2500_evb_builroot
  (and others that use the do_test_arm_aspeed_buidroot_start()
  or do_test_arm_aspeed_sdk_start() helper functions)

These changes seem to be sufficient for 'make check-avocado'
to not fail on a --disable-slirp build.

Signed-off-by: Peter Maydell 
Message-Id: <20221001195224.2453581-1-peter.mayd...@linaro.org>
Reviewed-by: Thomas Huth 
Reviewed-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 tests/avocado/boot_linux_console.py | 4 
 tests/avocado/machine_aspeed.py | 3 +++
 tests/avocado/ppc_bamboo.py | 1 +
 3 files changed, 8 insertions(+)

diff --git a/tests/avocado/boot_linux_console.py 
b/tests/avocado/boot_linux_console.py
index f26e036ab5..ca9d09b0d7 100644
--- a/tests/avocado/boot_linux_console.py
+++ b/tests/avocado/boot_linux_console.py
@@ -381,6 +381,8 @@ def test_arm_emcraft_sf2(self):
 :avocado: tags=u-boot
 :avocado: tags=accel:tcg
 """
+self.require_netdev('user')
+
 uboot_url = ('https://raw.githubusercontent.com/'
  'Subbaraya-Sundeep/qemu-test-binaries/'
  'fe371d32e50ca682391e1e70ab98c2942aeffb01/u-boot')
@@ -779,6 +781,8 @@ def test_arm_orangepi_sd(self):
 :avocado: tags=machine:orangepi-pc
 :avocado: tags=device:sd
 """
+self.require_netdev('user')
+
 deb_url = ('https://apt.armbian.com/pool/main/l/'

'linux-5.10.16-sunxi/linux-image-current-sunxi_21.02.2_armhf.deb')
 deb_hash = '9fa84beda245cabf0b4fa84cf6eaa7738ead1da0'
diff --git a/tests/avocado/machine_aspeed.py b/tests/avocado/machine_aspeed.py
index 0f64eb636c..124649a24b 100644
--- a/tests/avocado/machine_aspeed.py
+++ b/tests/avocado/machine_aspeed.py
@@ -93,6 +93,8 @@ def test_arm_ast2500_romulus_openbmc_v2_9_0(self):
 self.do_test_arm_aspeed(image_path)
 
 def do_test_arm_aspeed_buidroot_start(self, image, cpu_id):
+self.require_netdev('user')
+
 self.vm.set_console()
 self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
  '-net', 'nic', '-net', 'user')
@@ -193,6 +195,7 @@ def wait_for_console_pattern(self, success_message, 
vm=None):
  vm=vm)
 
 def do_test_arm_aspeed_sdk_start(self, image, cpu_id):
+self.require_netdev('user')
 self.vm.set_console()
 self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
  '-net', 'nic', '-net', 'user')
diff --git a/tests/avocado/ppc_bamboo.py b/tests/avocado/ppc_bamboo.py
index 102ff252df..a81be3d608 100644
--- a/tests/avocado/ppc_bamboo.py
+++ b/tests/avocado/ppc_bamboo.py
@@ -23,6 +23,7 @@ def test_ppc_bamboo(self):
 :avocado: tags=accel:tcg
 """
 self.require_accelerator("tcg")
+self.require_netdev('user')
 tar_url = ('http://landley.net/aboriginal/downloads/binaries/'
'system-image-powerpc-440fp.tar.gz')
 tar_hash = '53e5f16414b195b82d2c70272f81c2eedb39bad9'
-- 
2.31.1

[PULL 14/16] tests/qtest: migration-test: Avoid using hardcoded /tmp

[Thomas Huth]

From: Bin Meng 

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_dir_make_tmp() for a portable implementation.

Signed-off-by: Bin Meng 
Reviewed-by: Marc-André Lureau 
Message-Id: <20221006151927.2079583-5-bmeng...@gmail.com>
Signed-off-by: Thomas Huth 
---
 tests/qtest/migration-test.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 0d153d6b5e..ef4427ff4d 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -102,7 +102,7 @@ static bool ufd_version_check(void)
 
 #endif
 
-static const char *tmpfs;
+static char *tmpfs;
 
 /* The boot file modifies memory area in [start_address, end_address)
  * repeatedly. It outputs a 'B' at a fixed rate while it's still running.
@@ -2451,10 +2451,10 @@ static bool kvm_dirty_ring_supported(void)
 
 int main(int argc, char **argv)
 {
-char template[] = "/tmp/migration-test-XX";
 const bool has_kvm = qtest_has_accel("kvm");
 const bool has_uffd = ufd_version_check();
 const char *arch = qtest_get_arch();
+g_autoptr(GError) err = NULL;
 int ret;
 
 g_test_init(&argc, &argv, NULL);
@@ -2479,9 +2479,10 @@ int main(int argc, char **argv)
 return g_test_run();
 }
 
-tmpfs = g_mkdtemp(template);
+tmpfs = g_dir_make_tmp("migration-test-XX", &err);
 if (!tmpfs) {
-g_test_message("g_mkdtemp on path (%s): %s", template, 
strerror(errno));
+g_test_message("g_dir_make_tmp on path (%s): %s", tmpfs,
+   err->message);
 }
 g_assert(tmpfs);
 
@@ -2612,6 +2613,7 @@ int main(int argc, char **argv)
 g_test_message("unable to rmdir: path (%s): %s",
tmpfs, strerror(errno));
 }
+g_free(tmpfs);
 
 return ret;
 }
-- 
2.31.1

[PULL 10/16] tests/x86: Add 'q35' machine type to drive_del-test

[Thomas Huth]

From: Michael Labiuk 

Configure pci bridge setting to run tests on 'q35' machine type.

Signed-off-by: Michael Labiuk 
Message-Id: <20220929223547.1429580-9-michael.lab...@virtuozzo.com>
Reviewed-by: Thomas Huth 
Signed-off-by: Thomas Huth 
---
 tests/qtest/drive_del-test.c | 107 +++
 1 file changed, 107 insertions(+)

diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
index 106c613f4f..9a750395a9 100644
--- a/tests/qtest/drive_del-test.c
+++ b/tests/qtest/drive_del-test.c
@@ -252,6 +252,27 @@ static void test_cli_device_del(void)
 qtest_quit(qts);
 }
 
+static void test_cli_device_del_q35(void)
+{
+QTestState *qts;
+
+/*
+ * -drive/-device and device_del.  Start with a drive used by a
+ * device that unplugs after reset.
+ */
+qts = qtest_initf("-drive if=none,id=drive0,file=null-co://,"
+  "file.read-zeroes=on,format=raw "
+  "-machine q35 -device pcie-root-port,id=p1 "
+  "-device pcie-pci-bridge,bus=p1,id=b1 "
+  "-device virtio-blk-%s,drive=drive0,bus=b1,id=dev0",
+  qvirtio_get_dev_type());
+
+device_del(qts, true);
+g_assert(!has_drive(qts));
+
+qtest_quit(qts);
+}
+
 static void test_empty_device_del(void)
 {
 QTestState *qts;
@@ -288,6 +309,43 @@ static void test_device_add_and_del(void)
 qtest_quit(qts);
 }
 
+static void device_add_q35(QTestState *qts)
+{
+g_autofree char *driver = g_strdup_printf("virtio-blk-%s",
+  qvirtio_get_dev_type());
+QDict *response =
+   qtest_qmp(qts, "{'execute': 'device_add',"
+  " 'arguments': {"
+  "   'driver': %s,"
+  "   'drive': 'drive0',"
+  "   'id': 'dev0',"
+  "   'bus': 'b1'"
+  "}}", driver);
+g_assert(response);
+g_assert(qdict_haskey(response, "return"));
+qobject_unref(response);
+}
+
+static void test_device_add_and_del_q35(void)
+{
+QTestState *qts;
+
+/*
+ * -drive/device_add and device_del.  Start with a drive used by a
+ * device that unplugs after reset.
+ */
+qts = qtest_initf("-machine q35 -device pcie-root-port,id=p1 "
+ "-device pcie-pci-bridge,bus=p1,id=b1 "
+ "-drive if=none,id=drive0,file=null-co://,"
+ "file.read-zeroes=on,format=raw");
+
+device_add_q35(qts);
+device_del(qts, true);
+g_assert(!has_drive(qts));
+
+qtest_quit(qts);
+}
+
 static void test_drive_add_device_add_and_del(void)
 {
 QTestState *qts;
@@ -312,6 +370,25 @@ static void test_drive_add_device_add_and_del(void)
 qtest_quit(qts);
 }
 
+static void test_drive_add_device_add_and_del_q35(void)
+{
+QTestState *qts;
+
+qts = qtest_init("-machine q35 -device pcie-root-port,id=p1 "
+ "-device pcie-pci-bridge,bus=p1,id=b1");
+
+/*
+ * drive_add/device_add and device_del.  The drive is used by a
+ * device that unplugs after reset.
+ */
+drive_add_with_media(qts);
+device_add_q35(qts);
+device_del(qts, true);
+g_assert(!has_drive(qts));
+
+qtest_quit(qts);
+}
+
 static void test_blockdev_add_device_add_and_del(void)
 {
 QTestState *qts;
@@ -336,6 +413,25 @@ static void test_blockdev_add_device_add_and_del(void)
 qtest_quit(qts);
 }
 
+static void test_blockdev_add_device_add_and_del_q35(void)
+{
+QTestState *qts;
+
+qts = qtest_init("-machine q35 -device pcie-root-port,id=p1 "
+ "-device pcie-pci-bridge,bus=p1,id=b1");
+
+/*
+ * blockdev_add/device_add and device_del. The drive is used by a
+ * device that unplugs after reset, but it doesn't go away.
+ */
+blockdev_add_with_media(qts);
+device_add_q35(qts);
+device_del(qts, true);
+g_assert(has_blockdev(qts));
+
+qtest_quit(qts);
+}
+
 int main(int argc, char **argv)
 {
 g_test_init(&argc, &argv, NULL);
@@ -357,6 +453,17 @@ int main(int argc, char **argv)
test_empty_device_del);
 qtest_add_func("/device_del/blockdev",
test_blockdev_add_device_add_and_del);
+
+if (qtest_has_machine("q35")) {
+qtest_add_func("/device_del/drive/cli_device_q35",
+   test_cli_device_del_q35);
+qtest_add_func("/device_del/drive/device_add_q35",
+   test_device_add_and_del_q35);
+qtest_add_func("/device_del/drive/drive_add_device_add_q35",
+   test_drive_add_device_add_and_del_q35);
+qtest_add_func("/device_del/blockdev_q35",
+   test_blockdev_add_device_add_and_del_q35);
+}
 }
 
 return g_test_run();
-- 
2.31.1

[PULL 16/16] tests/unit/test-image-locking: Fix handling of temporary files

[Thomas Huth]

test-image-locking leaves some temporary files around - clean
them up. While we're at it, test-image-locking is a unit test,
so it should not use "qtest.*" for temporary file names. Give
them better names instead, so that it clear where the temporary
files come from.

Message-Id: <20221012085932.799221-1-th...@redhat.com>
Reviewed-by: Marc-André Lureau 
Signed-off-by: Thomas Huth 
---
 tests/unit/test-image-locking.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/unit/test-image-locking.c b/tests/unit/test-image-locking.c
index a47299c247..2624cec6a0 100644
--- a/tests/unit/test-image-locking.c
+++ b/tests/unit/test-image-locking.c
@@ -79,7 +79,7 @@ static void test_image_locking_basic(void)
 g_autofree char *img_path = NULL;
 uint64_t perm, shared_perm;
 
-int fd = g_file_open_tmp("qtest.XX", &img_path, NULL);
+int fd = g_file_open_tmp("qemu-tst-img-lock.XX", &img_path, NULL);
 assert(fd >= 0);
 
 perm = BLK_PERM_WRITE | BLK_PERM_CONSISTENT_READ;
@@ -120,7 +120,7 @@ static void test_set_perm_abort(void)
 g_autofree char *img_path = NULL;
 uint64_t perm, shared_perm;
 int r;
-int fd = g_file_open_tmp("qtest.XX", &img_path, NULL);
+int fd = g_file_open_tmp("qemu-tst-img-lock.XX", &img_path, NULL);
 assert(fd >= 0);
 
 perm = BLK_PERM_WRITE | BLK_PERM_CONSISTENT_READ;
@@ -140,6 +140,8 @@ static void test_set_perm_abort(void)
 check_locked_bytes(fd, perm, ~shared_perm);
 blk_unref(blk1);
 blk_unref(blk2);
+close(fd);
+unlink(img_path);
 }
 
 int main(int argc, char **argv)
-- 
2.31.1

[PULL 15/16] tests/qtest: libqtest: Install signal handler via signal()

[Thomas Huth]

From: Bin Meng 

At present the codes uses sigaction() to install signal handler with
a flag SA_RESETHAND. Such usage can be covered by the signal() API
that is a simplified interface to the general sigaction() facility.

Update to use signal() to install the signal handler, as it is
available on Windows which we are going to support.

Signed-off-by: Bin Meng 
Reviewed-by: Marc-André Lureau 
Message-Id: <20221006151927.2079583-11-bmeng...@gmail.com>
Signed-off-by: Thomas Huth 
---
 tests/qtest/libqtest.c | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 7b6152807b..b23eb3edc3 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -66,7 +66,7 @@ struct QTestState
 };
 
 static GHookList abrt_hooks;
-static struct sigaction sigact_old;
+static void (*sighandler_old)(int);
 
 static int qtest_query_target_endianness(QTestState *s);
 
@@ -179,20 +179,12 @@ static void sigabrt_handler(int signo)
 
 static void setup_sigabrt_handler(void)
 {
-struct sigaction sigact;
-
-/* Catch SIGABRT to clean up on g_assert() failure */
-sigact = (struct sigaction){
-.sa_handler = sigabrt_handler,
-.sa_flags = SA_RESETHAND,
-};
-sigemptyset(&sigact.sa_mask);
-sigaction(SIGABRT, &sigact, &sigact_old);
+sighandler_old = signal(SIGABRT, sigabrt_handler);
 }
 
 static void cleanup_sigabrt_handler(void)
 {
-sigaction(SIGABRT, &sigact_old, NULL);
+signal(SIGABRT, sighandler_old);
 }
 
 static bool hook_list_is_empty(GHookList *hook_list)
-- 
2.31.1

Re: [PULL 0/1] testing: revert pc-bios build patch

[Stefan Hajnoczi]

Applied, thanks.

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


signature.asc
Description: PGP signature

Re: [PATCH 0/3] iothread and irqfd support

[Klaus Jensen]

On Okt 12 10:28, Stefan Hajnoczi wrote:
> On Fri, 26 Aug 2022 at 07:18, Jinhao Fan  wrote:
> >
> > This patch series adds support for using a seperate iothread for NVMe
> > IO emulation, which brings the potential of applying polling. The
> > first two patches implements support for irqfd, which solves thread
> > safety problems for interrupt emulation outside the main loop thread.
> >
> > Jinhao Fan (3):
> >   hw/nvme: support irq(de)assertion with eventfd
> >   hw/nvme: use KVM irqfd when available
> >   hw/nvme: add iothread support
> 
> Hi,
> What is the status of this series?
> 

I have been meaning to pick it up, but I got side-tracked. The polling
performance drop needs to be address as we discussed offline.

But the v4 looks pretty good and I can pick that up without the polling
support for now.


signature.asc
Description: PGP signature

Re: [RFC PATCH 0/4] docs/devel suggestions for discussion

[Stefan Hajnoczi]

On Wed, Oct 12, 2022 at 01:11:48PM +0100, Alex Bennée wrote:
> Hi,
> 
> This is an attempt to improve our processes documentation by:
> 
>  - adding an explicit section on maintainers
>  - reducing the up-front verbiage in patch submission
>  - emphasising the importance to respectful reviews
> 
> I'm sure the language could be improved further so I humbly submit
> this RFC for discussion.
> 
> Alex Bennée (4):
>   docs/devel: add a maintainers section to development process
>   docs/devel: make language a little less code centric
>   docs/devel: simplify the minimal checklist
>   docs/devel: try and improve the language around patch review
> 
>  docs/devel/code-of-conduct.rst   |   2 +
>  docs/devel/index-process.rst |   1 +
>  docs/devel/maintainers.rst   |  84 +++
>  docs/devel/submitting-a-patch.rst| 101 +++
>  docs/devel/submitting-a-pull-request.rst |  12 +--
>  roms/qboot   |   2 +-
>  6 files changed, 157 insertions(+), 45 deletions(-)
>  create mode 100644 docs/devel/maintainers.rst
> 
> -- 
> 2.34.1
> 

Modulo comments:

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [RFC PATCH 3/4] docs/devel: simplify the minimal checklist

[Stefan Hajnoczi]

On Wed, Oct 12, 2022 at 01:11:51PM +0100, Alex Bennée wrote:
> The bullet points are quite long and contain process tips. Move those
> bits of the bullet to the relevant sections and link to them. Use a
> table for nicer formatting of the checklist.
> 
> Signed-off-by: Alex Bennée 
> ---
>  docs/devel/submitting-a-patch.rst | 75 ---
>  roms/qboot|  2 +-
>  2 files changed, 50 insertions(+), 27 deletions(-)
> 
> diff --git a/docs/devel/submitting-a-patch.rst 
> b/docs/devel/submitting-a-patch.rst
> index fb1673e974..41771501bf 100644
> --- a/docs/devel/submitting-a-patch.rst
> +++ b/docs/devel/submitting-a-patch.rst
> @@ -12,25 +12,18 @@ be committed faster.
>  This page seems very long, so if you are only trying to post a quick
>  one-shot fix, the bare minimum we ask is that:
>  
> --  You **must** provide a Signed-off-by: line (this is a hard
> -   requirement because it's how you say "I'm legally okay to contribute
> -   this and happy for it to go into QEMU", modeled after the `Linux kernel
> -   
> `__
> -   policy.) ``git commit -s`` or ``git format-patch -s`` will add one.
> --  All contributions to QEMU must be **sent as patches** to the
> -   qemu-devel `mailing list 
> `__.
> -   Patch contributions should not be posted on the bug tracker, posted on
> -   forums, or externally hosted and linked to. (We have other mailing lists 
> too,
> -   but all patches must go to qemu-devel, possibly with a Cc: to another
> -   list.) ``git send-email`` (`step-by-step setup
> -   guide `__ and `hints and
> -   tips 
> `__)
> -   works best for delivering the patch without mangling it, but
> -   attachments can be used as a last resort on a first-time submission.
> --  You must read replies to your message, and be willing to act on them.
> -   Note, however, that maintainers are often willing to manually fix up
> -   first-time contributions, since there is a learning curve involved in
> -   making an ideal patch submission.
> +.. list-table:: Minimal Checklist for Patches
> +   :widths: 35 65
> +   :header-rows: 1
> +
> +   * - Check
> + - Reason
> +   * - Patches contain Signed-off-by: Author Name 
> + - States you are legally able to contribute the code. See 
> :ref:`patch_emails_must_include_a_signed_off_by_line`
> +   * - Sent as patch emails to ``qemu-devel@nongnu.org``
> + - The project uses an email list based workflow. See 
> :ref:`submitting_your_patches`
> +   * - Be prepared to respond to review comments
> + - Code that doesn't pass review will not get merged. See 
> :ref:`participating_in_code_review`
>  
>  You do not have to subscribe to post (list policy is to reply-to-all to
>  preserve CCs and keep non-subscribers in the loop on the threads they
> @@ -229,6 +222,19 @@ bisection doesn't land on a known-broken state.
>  Submitting your Patches
>  ---
>  
> +The QEMU project uses a public email based workflow for reviewing and
> +merging patches. As a result all contributions to QEMU must be **sent
> +as patches** to the qemu-devel `mailing list
> +`__. Patch
> +contributions should not be posted on the bug tracker, posted on
> +forums, or externally hosted and linked to. (We have other mailing
> +lists too, but all patches must go to qemu-devel, possibly with a Cc:
> +to another list.) ``git send-email`` (`step-by-step setup guide
> +`__ and `hints and tips
> +`__)
> +works best for delivering the patch without mangling it, but
> +attachments can be used as a last resort on a first-time submission.
> +
>  .. _if_you_cannot_send_patch_emails:
>  
>  If you cannot send patch emails
> @@ -314,10 +320,12 @@ git repository to fetch the original commit.
>  Patch emails must include a ``Signed-off-by:`` line
>  ~~~
>  
> -For more information see `SubmittingPatches 1.12
> -`__.
> -This is vital or we will not be able to apply your patch! Please use
> -your real name to sign a patch (not an alias or acronym).
> +Your patches **must** include a Signed-off-by: line. This is a hard
> +requirement because it's how you say "I'm legally okay to contribute
> +this and happy for it to go into QEMU". The process is modelled after
> +the `Linux kernel
> +

Re: [RFC PATCH 1/4] docs/devel: add a maintainers section to development process

[Stefan Hajnoczi]

On Wed, Oct 12, 2022 at 01:11:49PM +0100, Alex Bennée wrote:
> We don't currently have a clear place in the documentation to describe
> the rolls and responsibilities of a maintainer. Lets create one so we

s/roll/role/ in the commit description and the patch.

> can. I've moved a few small bits out of other files to try and keep
> everything in one place.
> 
> Signed-off-by: Alex Bennée 
> ---
>  docs/devel/code-of-conduct.rst   |  2 +
>  docs/devel/index-process.rst |  1 +
>  docs/devel/maintainers.rst   | 84 
>  docs/devel/submitting-a-pull-request.rst | 12 ++--
>  4 files changed, 91 insertions(+), 8 deletions(-)
>  create mode 100644 docs/devel/maintainers.rst
> 
> diff --git a/docs/devel/code-of-conduct.rst b/docs/devel/code-of-conduct.rst
> index 195444d1b4..f734ed0317 100644
> --- a/docs/devel/code-of-conduct.rst
> +++ b/docs/devel/code-of-conduct.rst
> @@ -1,3 +1,5 @@
> +.. _code_of_conduct:
> +
>  Code of Conduct
>  ===
>  
> diff --git a/docs/devel/index-process.rst b/docs/devel/index-process.rst
> index d0d7a200fd..d50dd74c3e 100644
> --- a/docs/devel/index-process.rst
> +++ b/docs/devel/index-process.rst
> @@ -8,6 +8,7 @@ Notes about how to interact with the community and how and 
> where to submit patch
>  
> code-of-conduct
> conflict-resolution
> +   maintainers
> style
> submitting-a-patch
> trivial-patches
> diff --git a/docs/devel/maintainers.rst b/docs/devel/maintainers.rst
> new file mode 100644
> index 00..e3c7003bfa
> --- /dev/null
> +++ b/docs/devel/maintainers.rst
> @@ -0,0 +1,84 @@
> +.. _maintainers:
> +
> +The Roll of Maintainers
> +===
> +
> +Maintainers are a critical part of the projects contributor ecosystem.

project's

> +They come from a wide range of backgrounds from unpaid hobbyists
> +working in their spare time to employees who work on the project as
> +part of their job. Maintainer activities include:
> +
> +  - reviewing patches and suggesting changes
> +  - preparing pull requests for their subsystems
> +  - participating other project activities
> +
> +They are also human and subject to the same pressures as everyone else
> +including overload and burn out. Like everyone else they are subject
> +to projects :ref:`code_of_conduct`.

to the project's

(Although "project's" can be dropped without changing the meaning.)

> +
> +The MAINTAINERS file
> +
> +
> +The `MAINTAINERS
> +`__
> +file contains the canonical list of who is a maintainer. The file
> +is machine readable so an appropriately configured git (see
> +:ref:`cc_the_relevant_maintainer`) can automatically Cc them on
> +patches that touch their area of code.
> +
> +The file also describes the status of the area of code to give an idea
> +of how actively that section is maintained.
> +
> +.. list-table:: Meaning of support status in MAINTAINERS
> +   :widths: 25 75
> +   :header-rows: 1
> +
> +   * - Status
> + - Meaning
> +   * - Supported
> + - Someone is actually paid to look after this.
> +   * - Maintained
> + - Someone actually looks after it.
> +   * - Odd Fixes
> + - It has a maintainer but they don't have time to do
> +   much other than throw the odd patch in.
> +   * - Orphan
> + - No current maintainer.
> +   * - Obsolete
> + - Old obsolete code, should use something else.
> +
> +Please bare in mind that even if someone is paid to support something
> +it does not mean they are paid to support you. This is open source and
> +the code comes with no warranty and the project makes no guarantees
> +about dealing with bugs or features requests.
> +
> +Becoming a maintainer
> +-
> +
> +Maintainers are volunteers who put themselves forward to keep an eye
> +on an area of code. They are generally accepted by the community to
> +have a good understanding of the subsystem and able to make a positive
> +contribution to the project.
> +
> +The process is simple - simply sent a patch to the list that updates
> +the ``MAINTAINERS`` file. Sometimes this is done as part of a larger
> +series when a new sub-system is being added to the code base. This can
> +also be done by a retiring maintainer who nominates their replacement
> +after discussion with other contributors.
> +
> +Once the patch is reviewed and merged the only other step is to make
> +sure your GPG key is signed.
> +
> +.. _maintainer_keys:
> +
> +Maintainer GPG Keys
> +~~~
> +
> +GPG is used to sign pull requests so they can be identified as really
> +coming from the maintainer. If your key is not already signed by
> +members of the QEMU community, you should make arrangements to attend
> +a `KeySigningParty `__ (for
> +example at KVM Forum) or make alternative arrangements to have your
> +key signed by an attendee. Key signing requires meeting another

Re: [PATCH v4 2/6] util: make do_send_recv work with partial send/recv

[Daniel P . Berrangé]

On Thu, Oct 06, 2022 at 03:36:53PM +0400, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> According to msdn documentation and Linux man pages, send() should try
> to send as much as possible in blocking mode, while recv() may return
> earlier with a smaller available amount, we should try to continue
> send/recv from there.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  util/iov.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH v4 6/6] tests/unit: make test-io-channel-command work on win32

[Daniel P . Berrangé]

On Thu, Oct 06, 2022 at 03:36:57PM +0400, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> This has been tested under msys2 & windows 11. I haven't tried to make
> it work with other environments yet, but that should be enough to
> validate the channel-command implementation anyway.
> 
> Here are the changes:
> - drop tests/ from fifo/pipe path, to avoid directory issues
> - use g_find_program() to lookup the socat executable (otherwise we
> would need to change ChanneCommand to use G_SPAWN_SEARCH_PATH, and deal
> with missing socat differently)
> - skip the "echo" test when socat is missing as well
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  tests/unit/test-io-channel-command.c | 37 ++--
>  1 file changed, 19 insertions(+), 18 deletions(-)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH v2 2/2] error handling: Use RETRY_ON_EINTR() macro where applicable

[Marc-André Lureau]

Hi

On Wed, Oct 12, 2022 at 4:32 PM Nikita Ivanov 
wrote:

> There is a defined RETRY_ON_EINTR() macro in qemu/osdep.h which
> handles the same while loop.
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/415
>
> Signed-off-by: Nikita Ivanov 
>

Reviewed-by: Marc-André Lureau 



> ---
>  block/file-posix.c| 37 -
>  chardev/char-pty.c|  4 +---
>  hw/9pfs/9p-local.c|  8 ++--
>  net/l2tpv3.c  | 17 +
>  net/socket.c  | 16 +++-
>  net/tap.c | 12 
>  qga/commands-posix.c  |  4 +---
>  semihosting/syscalls.c|  4 +---
>  tests/qtest/libqtest.c| 14 ++
>  tests/vhost-user-bridge.c |  4 +---
>  util/main-loop.c  |  4 +---
>  util/osdep.c  |  4 +---
>  util/vfio-helpers.c   | 12 ++--
>  13 files changed, 52 insertions(+), 88 deletions(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 66fdb07820..c589cb489b 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -1238,9 +1238,7 @@ static int hdev_get_max_segments(int fd, struct stat
> *st)
>  ret = -errno;
>  goto out;
>  }
> -do {
> -ret = read(sysfd, buf, sizeof(buf) - 1);
> -} while (ret == -1 && errno == EINTR);
> +ret = RETRY_ON_EINTR(read(sysfd, buf, sizeof(buf) - 1));
>  if (ret < 0) {
>  ret = -errno;
>  goto out;
> @@ -1388,9 +1386,9 @@ static int handle_aiocb_ioctl(void *opaque)
>  RawPosixAIOData *aiocb = opaque;
>  int ret;
>
> -do {
> -ret = ioctl(aiocb->aio_fildes, aiocb->ioctl.cmd,
> aiocb->ioctl.buf);
> -} while (ret == -1 && errno == EINTR);
> +ret = RETRY_ON_EINTR(
> +ioctl(aiocb->aio_fildes, aiocb->ioctl.cmd, aiocb->ioctl.buf)
> +);
>  if (ret == -1) {
>  return -errno;
>  }
> @@ -1472,18 +1470,17 @@ static ssize_t
> handle_aiocb_rw_vector(RawPosixAIOData *aiocb)
>  {
>  ssize_t len;
>
> -do {
> -if (aiocb->aio_type & QEMU_AIO_WRITE)
> -len = qemu_pwritev(aiocb->aio_fildes,
> -   aiocb->io.iov,
> -   aiocb->io.niov,
> -   aiocb->aio_offset);
> - else
> -len = qemu_preadv(aiocb->aio_fildes,
> -  aiocb->io.iov,
> -  aiocb->io.niov,
> -  aiocb->aio_offset);
> -} while (len == -1 && errno == EINTR);
> +len = RETRY_ON_EINTR(
> +(aiocb->aio_type & QEMU_AIO_WRITE) ?
> +qemu_pwritev(aiocb->aio_fildes,
> +   aiocb->io.iov,
> +   aiocb->io.niov,
> +   aiocb->aio_offset) :
> +qemu_preadv(aiocb->aio_fildes,
> +  aiocb->io.iov,
> +  aiocb->io.niov,
> +  aiocb->aio_offset)
> +);
>
>  if (len == -1) {
>  return -errno;
> @@ -1908,9 +1905,7 @@ static int allocate_first_block(int fd, size_t
> max_size)
>  buf = qemu_memalign(max_align, write_size);
>  memset(buf, 0, write_size);
>
> -do {
> -n = pwrite(fd, buf, write_size, 0);
> -} while (n == -1 && errno == EINTR);
> +n = RETRY_ON_EINTR(pwrite(fd, buf, write_size, 0));
>
>  ret = (n == -1) ? -errno : 0;
>
> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> index 53f25c6bbd..92fd33c854 100644
> --- a/chardev/char-pty.c
> +++ b/chardev/char-pty.c
> @@ -93,9 +93,7 @@ static void pty_chr_update_read_handler(Chardev *chr)
>  pfd.fd = fioc->fd;
>  pfd.events = G_IO_OUT;
>  pfd.revents = 0;
> -do {
> -rc = g_poll(&pfd, 1, 0);
> -} while (rc == -1 && errno == EINTR);
> +rc = RETRY_ON_EINTR(g_poll(&pfd, 1, 0));
>  assert(rc >= 0);
>
>  if (pfd.revents & G_IO_HUP) {
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index d42ce6d8b8..bb3187244f 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -470,9 +470,7 @@ static ssize_t local_readlink(FsContext *fs_ctx,
> V9fsPath *fs_path,
>  if (fd == -1) {
>  return -1;
>  }
> -do {
> -tsize = read(fd, (void *)buf, bufsz);
> -} while (tsize == -1 && errno == EINTR);
> +tsize = RETRY_ON_EINTR(read(fd, (void *)buf, bufsz));
>  close_preserve_errno(fd);
>  } else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) ||
> (fs_ctx->export_flags & V9FS_SM_NONE)) {
> @@ -908,9 +906,7 @@ static int local_symlink(FsContext *fs_ctx, const char
> *oldpath,
>  }
>  /* Write the oldpath (target) to the file. */
>  oldpath_size = strlen(oldpath);
> -do {
> -write_size = write(fd, (void *)oldpath, oldpath_size);
> -} while (write_size == -1 && errno == EINTR);
> +write_size = RETRY_ON_EINTR(write(fd, (v

Re: [PATCH v2 2/2] error handling: Use RETRY_ON_EINTR() macro where applicable

[Bin Meng]

Hi,

On Wed, Oct 12, 2022 at 8:32 PM Nikita Ivanov  wrote:
>
> There is a defined RETRY_ON_EINTR() macro in qemu/osdep.h which
> handles the same while loop.
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/415
>
> Signed-off-by: Nikita Ivanov 
> ---
>  block/file-posix.c| 37 -
>  chardev/char-pty.c|  4 +---
>  hw/9pfs/9p-local.c|  8 ++--
>  net/l2tpv3.c  | 17 +
>  net/socket.c  | 16 +++-
>  net/tap.c | 12 
>  qga/commands-posix.c  |  4 +---
>  semihosting/syscalls.c|  4 +---
>  tests/qtest/libqtest.c| 14 ++
>  tests/vhost-user-bridge.c |  4 +---
>  util/main-loop.c  |  4 +---
>  util/osdep.c  |  4 +---
>  util/vfio-helpers.c   | 12 ++--
>  13 files changed, 52 insertions(+), 88 deletions(-)
>

This patch has to be squashed into patch 1 for bisectability, as TFR
is already removed in patch 1.

Regards,
Bin

Re: [PATCH] build: disable container-based cross compilers by default

[Alex Bennée]

Paolo Bonzini  writes:

> On 10/12/22 14:17, Alex Bennée wrote:
>>> Container-based cross compilers have some issues which were overlooked
>>> when they were only used for TCG tests, but are more visible since
>>> firmware builds try to use them:
>> We seem to have dropped our gating somewhere. Previously if a user did
>> not have docker or podman on their system none of the container stuff
>> would run.
>
> It's still there:
>
> container="no"
> if test $use_containers = "yes"; then
> case $($python "$source_path"/tests/docker/docker.py probe) in
> *docker) container=docker ;;
> podman) container=podman ;;
> no) container=no ;;
> esac
> if test "$container" != "no"; then
> docker_py="$python $source_path/tests/docker/docker.py --engine 
> $container"
> fi
> fi
>
> I think what's happening is that podman is there but there's no support
> for rootless containers, so "podman run" fails.

Ahh so we could improve our probe code then? I'm afraid I don't have
much personal testing coverage for podman stuff - I thought rootless
support was the main reason Fedora had transitioned to it.

>
> Paolo


-- 
Alex Bennée

Re: [PATCH 0/3] iothread and irqfd support

[Jinhao Fan]

On 10/12/2022 10:39 PM, Klaus Jensen wrote:

I have been meaning to pick it up, but I got side-tracked. The polling
performance drop needs to be address as we discussed offline.

But the v4 looks pretty good and I can pick that up without the polling
support for now.


I've been using the v4 without polling for my daily work. It worked 
pretty well for my test workloads.


I'm not sure what needs to be done for the polling problem. I can try to 
add a completion batching mechanism with BH, similar to virtio-blk. Do 
you think this is the right direction?

[PATCH 3/4] qtest: Improve error messages when property can not be set right now

[Markus Armbruster]

When you try to set qtest property "log" while the qtest object is
active, the error message blames "insufficient permission":

$ qemu-system-x86_64 -S -display none -nodefaults -monitor stdio -chardev 
socket,id=chrqt0,path=qtest.socket,server=on,wait=off -object 
qtest,id=qt0,chardev=chrqt0,log=/dev/null
QEMU 7.1.50 monitor - type 'help' for more information
(qemu) qom-set /objects/qt0 log qtest.log
Error: Insufficient permission to perform this operation

This implies it could work with "sufficient permission".  It can't.
Change the error message to:

Error: Property 'log' can not be set now

Same for property "chardev".

Signed-off-by: Markus Armbruster 
---
 softmmu/qtest.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index f8acef2628..afea7693d0 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -977,7 +977,7 @@ static void qtest_set_log(Object *obj, const char *value, 
Error **errp)
 QTest *q = QTEST(obj);
 
 if (qtest == q) {
-error_setg(errp, QERR_PERMISSION_DENIED);
+error_setg(errp, "Property 'log' can not be set now");
 } else {
 g_free(q->log);
 q->log = g_strdup(value);
@@ -997,7 +997,7 @@ static void qtest_set_chardev(Object *obj, const char 
*value, Error **errp)
 Chardev *chr;
 
 if (qtest == q) {
-error_setg(errp, QERR_PERMISSION_DENIED);
+error_setg(errp, "Property 'chardev' can not be set now");
 return;
 }
 
-- 
2.37.2

[PATCH 4/4] qerror: QERR_PERMISSION_DENIED is no longer used, drop

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
---
 include/qapi/qmp/qerror.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 596fce0c54..87ca83b155 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -50,9 +50,6 @@
 #define QERR_MISSING_PARAMETER \
 "Parameter '%s' is missing"
 
-#define QERR_PERMISSION_DENIED \
-"Insufficient permission to perform this operation"
-
 #define QERR_PROPERTY_VALUE_BAD \
 "Property '%s.%s' doesn't take value '%s'"
 
-- 
2.37.2