Re: [Qemu-devel] [PATCH for-2.11 v2] file-posix: Clear out first sector in hdev_create

[Fam Zheng]

On Fri, 08/11 16:09, Fam Zheng wrote:
> People get surprised when, after "qemu-img create -f raw /dev/sdX", they
> still see qcow2 with "qemu-img info", if previously the bdev had a qcow2
> header. While this is natural because raw doesn't need to write any
> magic bytes during creation, hdev_create is free to clear out the first
> sector to make sure the stale qcow2 header doesn't cause such confusion.
> 
> Signed-off-by: Fam Zheng 

Gentle ping as a reminder for 2.11 as we have now released 2.10.

Fam

Re: [Qemu-devel] [PATCHv3 0/2] pci: allow PCI bus slots to be marked as reserved

[Mark Cave-Ayland]

On 16/07/17 21:27, Mark Cave-Ayland wrote:

> For some machines it is impossible to plug devices into a particular PCI bus
> slot, e.g. for a real Ultra 5 there are 2 PCI bridges attached to the root
> bus behind which all devices must be plugged. Ignoring this rule will cause
> problems with interrupt routing since the interrupt numbers are calculated
> based upon PCI bridge id and secondary PCI bus slot id.
> 
> This patchset adds a new slot_reserved_mask property to PCIBus which is a
> bitmask used to indicate whether PCI bus slots are reserved, i.e. they cannot
> be used for hot or cold plugging on a particular PCI bus.
> 
> Signed-off-by: Mark Cave-Ayland 
> 
> v3:
> - Rebase onto master
> - Simplify pci_bus_devfn_available() as suggested by Marcel
> - Also simplify pci_bus_devfn_reserved() in a similar manner
> 
> v2:
> - Rename dev_reserved_mask to slot_reserved_mask as suggested by Marcel
> - Squash patches 2 and 3 together
> 
> 
> Mark Cave-Ayland (2):
>   pci: move check for existing devfn into new pci_bus_devfn_available()
> helper
>   pci: add reserved slot check to do_pci_register_device()
> 
>  hw/pci/pci.c |   26 ++
>  include/hw/pci/pci_bus.h |1 +
>  2 files changed, 23 insertions(+), 4 deletions(-)

Ping? Is there any chance to get this in soon, as I ended up having to
drop a couple of sun4u patch series for 2.10 that were dependent upon
this :(

I have R-B tags from Marcel, however my understanding is that this patch
still needs review from Michael?

Here is the link to the patch that depends upon this:
https://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg03041.html


Many thanks,

Mark.

Re: [Qemu-devel] [PATCH v5 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux)

[no-reply]

Hi,

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

Message-id: 20170831064302.14427-1-f...@redhat.com
Subject: [Qemu-devel] [PATCH v5 00/12] tests: Add VM based build tests (for 
non-x86_64 and/or non-Linux)
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]
patchew/20170830163609.50260-1-pa...@linux.vnet.ibm.com -> 
patchew/20170830163609.50260-1-pa...@linux.vnet.ibm.com
 t [tag update]patchew/20170830215523.25278-1-f4...@amsat.org -> 
patchew/20170830215523.25278-1-f4...@amsat.org
 * [new tag]   patchew/20170831064302.14427-1-f...@redhat.com -> 
patchew/20170831064302.14427-1-f...@redhat.com
Switched to a new branch 'test'
7ad2e659ca docker: Use archive-source.py
2dc170d07c tests: Add README for vm tests
19170c4fd7 MAINTAINERS: Add tests/vm entry
77ab30467b Makefile: Add rules to run vm tests
4a41ba7fcd tests: Add OpenBSD image
d6586b4423 tests: Add NetBSD image
3f1a2d60c9 tests: Add FreeBSD image
b413ab4921 tests: Add ubuntu.i386 image
167e9915a4 tests: Add vm test lib
75824c3f84 scripts: Add archive-source.sh
35bff37d20 qemu.py: Add "wait()" method
e572b186d5 gitignore: Ignore vm test images

=== OUTPUT BEGIN ===
Checking PATCH 1/12: gitignore: Ignore vm test images...
Checking PATCH 2/12: qemu.py: Add "wait()" method...
Checking PATCH 3/12: scripts: Add archive-source.sh...
Checking PATCH 4/12: tests: Add vm test lib...
ERROR: line over 90 characters
#85: FILE: tests/vm/basevm.py:60:
+ssh-rsa 
B3NzaC1yc2EDAQABAAABAQCikC46WYtXotUd0UGPz9547Aj0KqC4gk+nt4BBJm86IHgCD9FygSGX9EFutXlhz9KZIPg9Okk7+IzXRHCWI2MNvhrcjyrezKREm71z08j9iwfxY3340fY2Mo+0khwpO7bzsgzkljHIHqcOg7MgttPInVMNH/EfqpgR8EDKJuWCB2Ny+EBFN/3dAiff0X/EvKle9PUrY70EkSycnyURS8HZReEqj8lN9J5kXzA8F6jBo/0Q42Ttv6e4k5YcaDrwmLrBWLra2PCXZLNyHqXEiFkGmdXtA1Eox9gc/p4jIXim6xrPNmpN6WyrrEjaCF5xYvNv8wXkD6uSWwbHYU24lIAn
 qemu-vm-key

WARNING: line over 80 characters
#99: FILE: tests/vm/basevm.py:74:
+self._tmpdir = tempfile.mkdtemp(prefix="vm-test-", suffix=".tmp", 
dir=".")

WARNING: line over 80 characters
#190: FILE: tests/vm/basevm.py:165:
+logging.debug("Creating archive %s for src_dir dir: %s", tarfile, 
src_dir)

WARNING: line over 80 characters
#195: FILE: tests/vm/basevm.py:170:
+"file=%s,if=none,id=%s,cache=writeback,format=raw" 
% \

WARNING: line over 80 characters
#198: FILE: tests/vm/basevm.py:173:
+"virtio-blk,drive=%s,serial=%s,bootindex=1" % 
(name, name)]

ERROR: line over 90 characters
#245: FILE: tests/vm/basevm.py:220:
+VM test utility.  Exit codes: 0 = success, 1 = command line error, 2 = 
environment initialization failed, 3 = test command failed""")

WARNING: line over 80 characters
#252: FILE: tests/vm/basevm.py:227:
+parser.add_option("--jobs", type=int, default=multiprocessing.cpu_count() 
/ 2,

total: 2 errors, 5 warnings, 276 lines checked

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

Checking PATCH 5/12: tests: Add ubuntu.i386 image...
Checking PATCH 6/12: tests: Add FreeBSD image...
Checking PATCH 7/12: tests: Add NetBSD image...
Checking PATCH 8/12: tests: Add OpenBSD image...
Checking PATCH 9/12: Makefile: Add rules to run vm tests...
Checking PATCH 10/12: MAINTAINERS: Add tests/vm entry...
Checking PATCH 11/12: tests: Add README for vm tests...
Checking PATCH 12/12: docker: Use archive-source.py...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-devel] [PATCH v2] qemu-img: Clarify about relative backing file options

[Fam Zheng]

On Fri, 08/04 22:36, Fam Zheng wrote:
> It's not too surprising when a user specifies the backing file relative
> to the current working directory instead of the top layer image. This
> causes error when they differ. Though the error message has enough
> information to infer the fact about the misunderstanding, it is better
> if we document this explicitly, so that users don't have to learn from
> mistakes.

Gentle ping as a reminder for 2.11, as 2.10 is now out of door.

Fam

Re: [Qemu-devel] [PATCHv5 01/03] qemu-iothread: IOThread supports the GMainContext event loop

[Fam Zheng]

On Thu, 08/31 10:18, Fam Zheng wrote:
> On Tue, 08/29 15:22, Wang yong wrote:
> > From: Wang Yong 
> > 
> > IOThread uses AioContext event loop and does not run a GMainContext.
> > Therefore,chardev cannot work in IOThread,such as the chardev is
> > used for colo-compare packets reception.
> > 
> > This patch makes the IOThread run the GMainContext event loop,
> > chardev and IOThread can work together.
> > 
> > Signed-off-by: Wang Yong 
> > Signed-off-by: Wang Guang 
> > ---
> >  include/sysemu/iothread.h |  4 
> >  iothread.c| 45 
> > +
> >  2 files changed, 49 insertions(+)
> > 
> > diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
> > index e6da1a4..d2985b3 100644
> > --- a/include/sysemu/iothread.h
> > +++ b/include/sysemu/iothread.h
> > @@ -24,6 +24,9 @@ typedef struct {
> >  
> >  QemuThread thread;
> >  AioContext *ctx;
> > +GMainContext *worker_context;
> > +GMainLoop *main_loop;
> > +GOnce once;
> >  QemuMutex init_done_lock;
> >  QemuCond init_done_cond;/* is thread initialization done? */
> >  bool stopping;
> > @@ -41,5 +44,6 @@ typedef struct {
> >  char *iothread_get_id(IOThread *iothread);
> >  AioContext *iothread_get_aio_context(IOThread *iothread);
> >  void iothread_stop_all(void);
> > +GMainContext *iothread_get_g_main_context(IOThread *iothread);
> >  
> >  #endif /* IOTHREAD_H */
> > diff --git a/iothread.c b/iothread.c
> > index beeb870..44c8944 100644
> > --- a/iothread.c
> > +++ b/iothread.c
> > @@ -57,6 +57,23 @@ static void *iothread_run(void *opaque)
> >  
> >  while (!atomic_read(&iothread->stopping)) {
> >  aio_poll(iothread->ctx, true);
> > +
> > +if (atomic_read(&iothread->worker_context)) {
> > +GMainLoop *loop;
> > +
> > +g_main_context_push_thread_default(iothread->worker_context);
> > +iothread->main_loop =
> > +g_main_loop_new(iothread->worker_context, TRUE);
> > +loop = iothread->main_loop;
> > +
> > +g_main_loop_run(iothread->main_loop);
> > +iothread->main_loop = NULL;
> > +g_main_loop_unref(loop);
> > +
> > +g_main_context_pop_thread_default(iothread->worker_context);
> > +g_main_context_unref(iothread->worker_context);
> > +iothread->worker_context = NULL;
> > +}
> >  }
> >  
> >  rcu_unregister_thread();
> > @@ -73,6 +90,9 @@ static int iothread_stop(Object *object, void *opaque)
> >  }
> >  iothread->stopping = true;
> >  aio_notify(iothread->ctx);
> > +if (atomic_read(&iothread->main_loop)) {
> > +g_main_loop_quit(iothread->main_loop);
> > +}
> >  qemu_thread_join(&iothread->thread);
> >  return 0;
> >  }
> > @@ -125,6 +145,7 @@ static void iothread_complete(UserCreatable *obj, Error 
> > **errp)
> >  
> >  qemu_mutex_init(&iothread->init_done_lock);
> >  qemu_cond_init(&iothread->init_done_cond);
> > +iothread->once = (GOnce) G_ONCE_INIT;
> 
> In last review I suggested removing this type cast, otherwise looks good. Drop
> it and please add

No, Yong is right, because this is not varialble initializer, type cast is
required by C. Sorry for the noise.

Fam

Re: [Qemu-devel] reduce write bandwidth of qcow2 driver while allocating new cluster

[Liu Qing]

On Wed, Aug 30, 2017 at 01:15:33PM +0300, Anton Nefedov wrote:
> 
> On 29/08/2017 05:56, Liu Qing wrote:
> >On Mon, Aug 28, 2017 at 10:46:34AM -0500, Eric Blake wrote:
> >>[adding qemu-block]
> >>
> >>On 08/28/2017 12:56 AM, Liu Qing wrote:
> >>>Dear list,
> >>>Recently I used fio to test qcow2 driver in the guest os, and found out
> >>>that when a new cluster is allocated the 4K IO will occupy 64K(default 
> >>>cluster
> >>>size) bandwith.
> >>>From the code qcow2 driver will fill the unused part of new allocated
> >>>cluster with 0 in perform_cow. These 0s are set in qcow2_co_readv when the 
> >>>read
> >>>destination is not allocated and it has no backing file. Could I forbidden 
> >>>any
> >>>further write in copy_sectors if the copy source is not allocated and it 
> >>>has
> >>>no backing file? So only the requested data is written to the cluster. 
> >>>Function
> >>>copy_sectors is only used by perform_cow in the master branch.
> >>
> >>There have already been discussions on optimizing COW writes in a manner
> >>similar to what you are describing; for example,
> >>
> >>https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg00109.html
> >Thanks Eric, this is what I am looking for.
> >The only concern I have is in patch '[Qemu-devel] [PATCH v4 12/15] qcow2: 
> >skip
> >writing zero buffers to empty' it says:
> >
> >It can be detected that
> >  1. COW alignment of a write request is zeroes
> >  2. Respective areas on the underlying BDS already read as zeroes
> > after being preallocated previously
> >  If both of these true, COW may be skipped
> >
> >Will writing zero be skipped if the disk is not preallocated? @Anton
> >
> 
> Hi,
> 
> In short, no, it will not (with my patches), but there might be some way
> if that's what you really need.
> 
> 
> First of all, this might be undesirable as you lose the cluster-size
> data locality: now the whole cluster is written at once and is expected
> to reside in the contiguous area on the physical drive.
> 
> Secondly, I think there is no guarantee that the underlying bs->file
> image reads back as zeroes if the cluster is unallocated on qcow2 level.
Why we need this guarantee? If the cluster is unallocated, it means no
one used these clusters previously. So why should these unallocated
clusters be read back as zeroes?
> 
> For example, the unallocated cluster could have been used earlier but
> then discarded. Discard passthrough is configurable so discard may not
> be passed down to the underlying image. And I guess that in general,
> even if it is passed, there is no strong requirement on reading back as
> zeroes - look at qcow2 discard handling - discard head and tail which do
> not cover full clusters are ignored.
> 
> _perhaps_, one may expect that there will be zeroes if the cluster is
> allocated at the end of file
> (see 'clusters_are_trailing' detection here
> https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg00122.html)
> 
> but I haven't thought about all corner cases here.
> 
> 
> /Anton
> 
> >BTW: why the code in the patch is a little different than the latest
> >master branch? For example I don't have the is_zero function but only
> >get is_zero_sectors. Is there something wrong with my settings?
> >
> >My repo:
> ># git remote -v
> >origin  git://git.qemu-project.org/qemu.git (fetch)
> >origin  git://git.qemu-project.org/qemu.git (push)
> >
> >Thanks.
> >>
> >>--
> >>Eric Blake, Principal Software Engineer
> >>Red Hat, Inc.   +1-919-301-3266
> >>Virtualization:  qemu.org | libvirt.org
> >>
> >
> >

Re: [Qemu-devel] [PATCH v2 2/3] block-jobs: Optionally unregister live block operations

[Markus Armbruster]

Eric Blake  writes:

> On 08/30/2017 12:24 PM, Eduardo Habkost wrote:
>> On Wed, Aug 30, 2017 at 01:01:41PM -0400, Jeff Cody wrote:
>>> From: Jeffrey Cody 
>>>
>>> If configured without live block operations enabled, unregister the
>>> live block operation commands.
>>>
>>> Signed-off-by: Jeff Cody 
>>> ---
>>>  monitor.c | 16 
>>>  1 file changed, 16 insertions(+)
>>>
>
>> 
>> I suggest using the new mechanisms added by:
>> 
>>   [PATCH 00/26] qapi: add #if pre-processor conditions to generated code
>
> Those haven't landed yet, but as both series are proposed for 2.11, I
> indeed agree that basing this series on top of that one will be a bit
> cleaner.

Rebasing shouldn't be hard.  However, we then have to hold it until the
QAPI series lands.  I don't think holding is necessary, as the conflicts
between the two are obvious, and should be straightforward to resolve.

[Qemu-devel] [PATCH v5 10/12] MAINTAINERS: Add tests/vm entry

[Fam Zheng]

Signed-off-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ccee28b12d..0ed607d003 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1881,6 +1881,7 @@ S: Maintained
 F: .travis.yml
 F: .shippable.yml
 F: tests/docker/
+F: tests/vm/
 W: https://travis-ci.org/qemu/qemu
 W: https://app.shippable.com/github/qemu/qemu
 W: http://patchew.org/QEMU/
-- 
2.13.5

[Qemu-devel] [PATCH v5 12/12] docker: Use archive-source.py

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 tests/docker/Makefile.include | 15 ++-
 tests/docker/run  |  8 +---
 2 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index aaab1a4208..7a027d5bd6 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -17,24 +17,13 @@ DOCKER_TOOLS := travis
 TESTS ?= %
 IMAGES ?= %
 
-# Make archive from git repo $1 to tar.gz $2
-make-archive-maybe = $(if $(wildcard $1/*), \
-   $(call quiet-command, \
-   (cd $1; if git diff-index --quiet HEAD -- &>/dev/null; then \
-   git archive -1 HEAD --format=tar.gz; \
-   else \
-   git archive -1 $$(git stash create) --format=tar.gz; \
-   fi) > $2, \
-   "ARCHIVE","$(notdir $2)"))
-
 CUR_TIME := $(shell date +%Y-%m-%d-%H.%M.%S.)
 DOCKER_SRC_COPY := docker-src.$(CUR_TIME)
 
 $(DOCKER_SRC_COPY):
@mkdir $@
-   $(call make-archive-maybe, $(SRC_PATH), $@/qemu.tgz)
-   $(call make-archive-maybe, $(SRC_PATH)/dtc, $@/dtc.tgz)
-   $(call make-archive-maybe, $(SRC_PATH)/pixman, $@/pixman.tgz)
+   $(call quiet-command, $(SRC_PATH)/scripts/archive-source.sh 
$@/qemu.tar, \
+   "GEN", "$@/qemu.tar")
$(call quiet-command, cp $(SRC_PATH)/tests/docker/run $@/run, \
"COPY","RUNNER")
 
diff --git a/tests/docker/run b/tests/docker/run
index c1e4513bce..9eb9165f76 100755
--- a/tests/docker/run
+++ b/tests/docker/run
@@ -32,13 +32,7 @@ export TEST_DIR=/tmp/qemu-test
 mkdir -p $TEST_DIR/{src,build,install}
 
 # Extract the source tarballs
-tar -C $TEST_DIR/src -xzf $BASE/qemu.tgz
-for p in dtc pixman; do
-if test -f $BASE/$p.tgz; then
-tar -C $TEST_DIR/src/$p -xzf $BASE/$p.tgz
-export FEATURES="$FEATURES $p"
-fi
-done
+tar -C $TEST_DIR/src -xf $BASE/qemu.tar
 
 if test -n "$SHOW_ENV"; then
 if test -f /packages.txt; then
-- 
2.13.5

Re: [Qemu-devel] [PATCHv5 01/03] qemu-iothread: IOThread supports theGMainContext event loop

[Fam Zheng]

On Thu, 08/31 11:27, wang.yong...@zte.com.cn wrote:
> >> From: Wang Yong 
> 
> >> 
> 
> >> IOThread uses AioContext event loop and does not run a GMainContext.
> 
> >> Therefore,chardev cannot work in IOThread,such as the chardev is
> 
> >> used for colo-compare packets reception.
> 
> >> 
> 
> >> This patch makes the IOThread run the GMainContext event loop,
> 
> >> chardev and IOThread can work together.
> 
> >> 
> 
> >> Signed-off-by: Wang Yong 
> 
> >> Signed-off-by: Wang Guang 
> 
> >> ---
> 
> >>  include/sysemu/iothread.h |  4 
> 
> >>  iothread.c| 45 
> >> +
> 
> >>  2 files changed, 49 insertions(+)
> 
> >> 
> 
> >> diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
> 
> >> index e6da1a4..d2985b3 100644
> 
> >> --- a/include/sysemu/iothread.h
> 
> >> +++ b/include/sysemu/iothread.h
> 
> >> @@ -24,6 +24,9 @@ typedef struct {
> 
> >>  
> 
> >>  QemuThread thread
> 
> >>  AioContext *ctx
> 
> >> +GMainContext *worker_context
> 
> >> +GMainLoop *main_loop
> 
> >> +GOnce once
> 
> >>  QemuMutex init_done_lock
> 
> >>  QemuCond init_done_cond/* is thread initialization done? */
> 
> >>  bool stopping
> 
> >> @@ -41,5 +44,6 @@ typedef struct {
> 
> >>  char *iothread_get_id(IOThread *iothread)
> 
> >>  AioContext *iothread_get_aio_context(IOThread *iothread)
> 
> >>  void iothread_stop_all(void)
> 
> >> +GMainContext *iothread_get_g_main_context(IOThread *iothread)
> 
> >>  
> 
> >>  #endif /* IOTHREAD_H */
> 
> >> diff --git a/iothread.c b/iothread.c
> 
> >> index beeb870..44c8944 100644
> 
> >> --- a/iothread.c
> 
> >> +++ b/iothread.c
> 
> >> @@ -57,6 +57,23 @@ static void *iothread_run(void *opaque)
> 
> >>  
> 
> >>  while (!atomic_read(&iothread->stopping)) {
> 
> >>  aio_poll(iothread->ctx, true)
> 
> >> +
> 
> >> +if (atomic_read(&iothread->worker_context)) {
> 
> >> +GMainLoop *loop
> 
> >> +
> 
> >> +g_main_context_push_thread_default(iothread->worker_context)
> 
> >> +iothread->main_loop =
> 
> >> +g_main_loop_new(iothread->worker_context, TRUE)
> 
> >> +loop = iothread->main_loop
> 
> >> +
> 
> >> +g_main_loop_run(iothread->main_loop)
> 
> >> +iothread->main_loop = NULL
> 
> >> +g_main_loop_unref(loop)
> 
> >> +
> 
> >> +g_main_context_pop_thread_default(iothread->worker_context)
> 
> >> +g_main_context_unref(iothread->worker_context)
> 
> >> +iothread->worker_context = NULL
> 
> >> +}
> 
> >>  }
> 
> >>  
> 
> >>  rcu_unregister_thread()
> 
> >> @@ -73,6 +90,9 @@ static int iothread_stop(Object *object, void *opaque)
> 
> >>  }
> 
> >>  iothread->stopping = true
> 
> >>  aio_notify(iothread->ctx)
> 
> >> +if (atomic_read(&iothread->main_loop)) {
> 
> >> +g_main_loop_quit(iothread->main_loop)
> 
> >> +}
> 
> >>  qemu_thread_join(&iothread->thread)
> 
> >>  return 0
> 
> >>  }
> 
> >> @@ -125,6 +145,7 @@ static void iothread_complete(UserCreatable *obj, 
> >> Error **errp)
> 
> >>  
> 
> >>  qemu_mutex_init(&iothread->init_done_lock)
> 
> >>  qemu_cond_init(&iothread->init_done_cond)
> 
> >> +iothread->once = (GOnce) G_ONCE_INIT
> 
> >
> 
> >In last review I suggested removing this type cast, otherwise looks good. 
> >Drop
> 
> >it and please add
> 
> >
> 
> >Reviewed-by: Fam Zheng 
> 
> 
> 
> 
> Sorry, There's something wrong with our company's mail format.

Yes, not only the format. There isn't "In-Reply-To" in the header in the other
message, which is a big issue because they won't be added to the original
thread. Please fix your email setup.

> 
> Please ignore my last reply mail.
> 
> 
> 
> 
> Hi Fam,
> 
> Here, iothread->once can't be initialized with G_ONCE_INIT directly, must use 
> type cast.
> 
> if  remove type cast , we will get an error.

Yes, you are right, thanks. I've also replied in the thread.

Fam

[Qemu-devel] [PATCH v5 09/12] Makefile: Add rules to run vm tests

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 Makefile  |  2 ++
 configure |  2 +-
 tests/vm/Makefile.include | 42 ++
 3 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 tests/vm/Makefile.include

diff --git a/Makefile b/Makefile
index 81447b1f08..2798a5ca69 100644
--- a/Makefile
+++ b/Makefile
@@ -813,6 +813,7 @@ endif
 -include $(wildcard *.d tests/*.d)
 
 include $(SRC_PATH)/tests/docker/Makefile.include
+include $(SRC_PATH)/tests/vm/Makefile.include
 
 .PHONY: help
 help:
@@ -836,6 +837,7 @@ help:
@echo  'Test targets:'
@echo  '  check   - Run all tests (check-help for details)'
@echo  '  docker  - Help about targets running tests inside 
Docker containers'
+   @echo  '  vm-test - Help about targets running tests inside VM'
@echo  ''
@echo  'Documentation targets:'
@echo  '  html info pdf txt'
diff --git a/configure b/configure
index dd73cce62f..9a3052e9ad 100755
--- a/configure
+++ b/configure
@@ -6544,7 +6544,7 @@ if test "$ccache_cpp2" = "yes"; then
 fi
 
 # build tree in object directory in case the source is not in the current 
directory
-DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos 
tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests"
+DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos 
tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests tests/vm"
 DIRS="$DIRS docs docs/interop fsdev"
 DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
 DIRS="$DIRS roms/seabios roms/vgabios"
diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
new file mode 100644
index 00..5daa2a3b73
--- /dev/null
+++ b/tests/vm/Makefile.include
@@ -0,0 +1,42 @@
+# Makefile for VM tests
+
+.PHONY: vm-build-all
+
+IMAGES := ubuntu.i386 freebsd netbsd openbsd
+IMAGE_FILES := $(patsubst %, tests/vm/%.img, $(IMAGES))
+
+.PRECIOUS: $(IMAGE_FILES)
+
+vm-test:
+   @echo "vm-test: Test QEMU in preconfigured virtual machines"
+   @echo
+   @echo "  vm-build-ubuntu.i386- Build QEMU in ubuntu i386 VM"
+   @echo "  vm-build-freebsd- Build QEMU in FreeBSD VM"
+   @echo "  vm-build-netbsd - Build QEMU in NetBSD VM"
+   @echo "  vm-build-openbsd- Build QEMU in OpenBSD VM"
+
+vm-build-all: $(addprefix vm-build-, $(IMAGES))
+
+tests/vm/%.img: $(SRC_PATH)/tests/vm/% \
+   $(SRC_PATH)/tests/vm/basevm.py \
+   $(SRC_PATH)/tests/vm/Makefile.include
+   $(call quiet-command, \
+   $< \
+   $(if $(V)$(DEBUG), --debug) \
+   --image "$@" \
+   --force \
+   --build-image $@, \
+   "  VM-IMAGE $*")
+
+
+# Build in VM $(IMAGE)
+vm-build-%: tests/vm/%.img
+   $(call quiet-command, \
+   $(SRC_PATH)/tests/vm/$* \
+   $(if $(V)$(DEBUG), --debug) \
+   $(if $(DEBUG), --interactive) \
+   $(if $(J),--jobs $(J)) \
+   --image "$<" \
+   --build-qemu $(SRC_PATH), \
+   "  VM-BUILD $*")
+
-- 
2.13.5

[Qemu-devel] [PATCH v5 06/12] tests: Add FreeBSD image

[Fam Zheng]

The image is prepared following instructions as in:

https://wiki.qemu.org/Hosts/BSD

Signed-off-by: Fam Zheng 
---
 tests/vm/freebsd | 42 ++
 1 file changed, 42 insertions(+)
 create mode 100755 tests/vm/freebsd

diff --git a/tests/vm/freebsd b/tests/vm/freebsd
new file mode 100755
index 00..6840da0bf0
--- /dev/null
+++ b/tests/vm/freebsd
@@ -0,0 +1,42 @@
+#!/usr/bin/env python
+#
+# FreeBSD VM image
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import subprocess
+import basevm
+
+class FreeBSDVM(basevm.BaseVM):
+name = "freebsd"
+BUILD_SCRIPT = """
+set -e;
+cd $(mktemp -d /var/tmp/qemu-test.XX);
+tar -xf /dev/vtbd1;
+./configure {configure_opts};
+gmake -j{jobs};
+gmake check;
+"""
+
+def build_image(self, img):
+cimg = 
self._download_with_cache("http://download.patchew.org/freebsd-11.1-amd64.img.xz";,
+
sha256sum='adcb771549b37bc63826c501f05121a206ed3d9f55f49145908f7e1432d65891')
+img_tmp_xz = img + ".tmp.xz"
+img_tmp = img + ".tmp"
+subprocess.check_call(["cp", "-f", cimg, img_tmp_xz])
+subprocess.check_call(["xz", "-df", img_tmp_xz])
+if os.path.exists(img):
+os.remove(img)
+os.rename(img_tmp, img)
+
+if __name__ == "__main__":
+sys.exit(basevm.main(FreeBSDVM))
-- 
2.13.5

[Qemu-devel] [PATCH v5 11/12] tests: Add README for vm tests

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 tests/vm/README | 63 +
 1 file changed, 63 insertions(+)
 create mode 100644 tests/vm/README

diff --git a/tests/vm/README b/tests/vm/README
new file mode 100644
index 00..7d2fe4ac8d
--- /dev/null
+++ b/tests/vm/README
@@ -0,0 +1,63 @@
+=== VM test suite to run build in guests ===
+
+== Intro ==
+
+This test suite contains scripts that bootstrap various guest images that have
+necessary packages to build QEMU. The basic usage is documented in Makefile
+help which is displayed with "make vm-test".
+
+== Quick start ==
+
+Run "make vm-test" to list available make targets.
+
+== Manual invocation ==
+
+Each guest script is an executable script with the same command line options.
+For example to work with the netbsd guest, use $QEMU_SRC/tests/vm/netbsd:
+
+$ cd $QEMU_SRC/tests/vm
+
+# To bootstrap the image
+$ ./netbsd --build-image --image /var/tmp/netbsd.img
+<...>
+
+# To run an arbitrary command in guest (the output will not be echoed 
unless
+# --debug is added)
+$ ./netbsd --debug --image /var/tmp/netbsd.img uname -a
+
+# To build QEMU in guest
+$ ./netbsd --debug --image /var/tmp/netbsd.img --build-qemu $QEMU_SRC
+
+# To get to an interactive shell
+$ ./netbsd --interactive --image /var/tmp/netbsd.img sh
+
+== Adding new guests ==
+
+Please look at existing guest scripts for how to add new guests.
+
+Most importantly, create a subclass of BaseVM and implement build_image()
+method and define BUILD_SCRIPT, then finally call basevm.main() from the
+script's main().
+
+  - Usually in build_image(), a template image is downloaded from a predefined
+URL. BaseVM._download_with_cache() takes care of the cache and the
+checksum, so consider using it.
+
+  - Once the image is downloaded, users, SSH server and QEMU build deps should
+be set up:
+
+* Root password set to BaseVM.ROOT_PASS
+* User BaseVM.GUEST_USER is created, and password set to BaseVM.GUEST_PASS
+* SSH service is enabled and started on boot, BaseVM.SSH_PUB_KEY is added
+  to authorized_keys of both root and the normal user
+* DHCP client service is enabled and started on boot, so that it can
+  automatically configure the virtio-net-pci NIC and communicate with QEMU
+  user net (10.0.2.2)
+* Necessary packages are installed to untar the source tarball and build
+  QEMU
+
+  - Write a proper BUILD_SCRIPT template, which should be a shell script that
+untars a raw virtio-blk block device, which is the tarball data blob of the
+QEMU source tree, then configure/build it. Running "make check" is also
+recommended.
+
-- 
2.13.5

[Qemu-devel] [PATCH v5 07/12] tests: Add NetBSD image

[Fam Zheng]

The image is prepared following instructions as in:

https://wiki.qemu.org/Hosts/BSD

Signed-off-by: Fam Zheng 
Reviewed-by: Kamil Rytarowski 
---
 tests/vm/netbsd | 42 ++
 1 file changed, 42 insertions(+)
 create mode 100755 tests/vm/netbsd

diff --git a/tests/vm/netbsd b/tests/vm/netbsd
new file mode 100755
index 00..559e89c8a6
--- /dev/null
+++ b/tests/vm/netbsd
@@ -0,0 +1,42 @@
+#!/usr/bin/env python
+#
+# NetBSD VM image
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import subprocess
+import basevm
+
+class NetBSDVM(basevm.BaseVM):
+name = "netbsd"
+BUILD_SCRIPT = """
+set -e;
+cd $(mktemp -d /var/tmp/qemu-test.XX);
+tar -xf /dev/rld1a;
+./configure --python=python2.7 {configure_opts};
+gmake -j{jobs};
+gmake check;
+"""
+
+def build_image(self, img):
+cimg = 
self._download_with_cache("http://download.patchew.org/netbsd-7.1-amd64.img.xz";,
+ 
sha256sum='b633d565b0eac3d02015cd0c81440bd8a7a8df8512615ac1ee05d318be015732')
+img_tmp_xz = img + ".tmp.xz"
+img_tmp = img + ".tmp"
+subprocess.check_call(["cp", "-f", cimg, img_tmp_xz])
+subprocess.check_call(["xz", "-df", img_tmp_xz])
+if os.path.exists(img):
+os.remove(img)
+os.rename(img_tmp, img)
+
+if __name__ == "__main__":
+sys.exit(basevm.main(NetBSDVM))
-- 
2.13.5

[Qemu-devel] [PATCH v5 08/12] tests: Add OpenBSD image

[Fam Zheng]

The image is prepared following instructions as in:

https://wiki.qemu.org/Hosts/BSD

Signed-off-by: Fam Zheng 
---
 tests/vm/openbsd | 43 +++
 1 file changed, 43 insertions(+)
 create mode 100755 tests/vm/openbsd

diff --git a/tests/vm/openbsd b/tests/vm/openbsd
new file mode 100755
index 00..57b10105f7
--- /dev/null
+++ b/tests/vm/openbsd
@@ -0,0 +1,43 @@
+#!/usr/bin/env python
+#
+# OpenBSD VM image
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import subprocess
+import basevm
+
+class OpenBSDVM(basevm.BaseVM):
+name = "openbsd"
+BUILD_SCRIPT = """
+set -e;
+cd $(mktemp -d /var/tmp/qemu-test.XX);
+tar -xf /dev/rsd1c;
+./configure --cc=x86_64-unknown-openbsd6.1-gcc-4.9.4 
--python=python2.7 {configure_opts};
+gmake -j{jobs};
+# XXX: "gmake check" seems to always hang or fail
+#gmake check;
+"""
+
+def build_image(self, img):
+cimg = 
self._download_with_cache("http://download.patchew.org/openbsd-6.1-amd64.img.xz";,
+
sha256sum='8c6cedc483e602cfee5e04f0406c64eb99138495e8ca580bc0293bcf0640c1bf')
+img_tmp_xz = img + ".tmp.xz"
+img_tmp = img + ".tmp"
+subprocess.check_call(["cp", "-f", cimg, img_tmp_xz])
+subprocess.check_call(["xz", "-df", img_tmp_xz])
+if os.path.exists(img):
+os.remove(img)
+os.rename(img_tmp, img)
+
+if __name__ == "__main__":
+sys.exit(basevm.main(OpenBSDVM))
-- 
2.13.5

[Qemu-devel] [PATCH v5 05/12] tests: Add ubuntu.i386 image

[Fam Zheng]

This adds a 32bit guest.

The official LTS cloud image is downloaded and initialized with
cloud-init.

Signed-off-by: Fam Zheng 
---
 tests/vm/ubuntu.i386 | 88 
 1 file changed, 88 insertions(+)
 create mode 100755 tests/vm/ubuntu.i386

diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386
new file mode 100755
index 00..1a55856d9c
--- /dev/null
+++ b/tests/vm/ubuntu.i386
@@ -0,0 +1,88 @@
+#!/usr/bin/env python
+#
+# Ubuntu i386 image
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import subprocess
+import basevm
+import time
+
+class UbuntuX86VM(basevm.BaseVM):
+name = "ubuntu.i386"
+BUILD_SCRIPT = """
+set -e;
+cd $(mktemp -d);
+sudo chmod a+r /dev/vdb;
+tar -xf /dev/vdb;
+./configure {configure_opts};
+make -j{jobs};
+make check;
+"""
+
+def _gen_cloud_init_iso(self):
+cidir = self._tmpdir
+mdata = open(os.path.join(cidir, "meta-data"), "w")
+mdata.writelines(["instance-id: ubuntu-vm-0\n",
+ "local-hostname: ubuntu-guest\n"])
+mdata.close()
+udata = open(os.path.join(cidir, "user-data"), "w")
+udata.writelines(["#cloud-config\n",
+  "chpasswd:\n",
+  "  list: |\n",
+  "root:%s\n" % self.ROOT_PASS,
+  "%s:%s\n" % (self.GUEST_USER, self.GUEST_PASS),
+  "  expire: False\n",
+  "users:\n",
+  "  - name: %s\n" % self.GUEST_USER,
+  "sudo: ALL=(ALL) NOPASSWD:ALL\n",
+  "ssh-authorized-keys:\n",
+  "- %s\n" % basevm.SSH_PUB_KEY,
+  "  - name: root\n",
+  "ssh-authorized-keys:\n",
+  "- %s\n" % basevm.SSH_PUB_KEY])
+udata.close()
+subprocess.check_call(["genisoimage", "-output", "cloud-init.iso",
+   "-volid", "cidata", "-joliet", "-rock",
+   "user-data", "meta-data"],
+   cwd=cidir,
+   stdin=self._devnull, stdout=self._stdout,
+   stderr=self._stdout)
+return os.path.join(cidir, "cloud-init.iso")
+
+def build_image(self, img):
+cimg = 
self._download_with_cache("https://cloud-images.ubuntu.com/releases/16.04/release/ubuntu-16.04-server-cloudimg-i386-disk1.img";)
+img_tmp = img + ".tmp"
+subprocess.check_call(["cp", "-f", cimg, img_tmp])
+subprocess.check_call(["qemu-img", "resize", img_tmp, "50G"])
+self.boot(img_tmp, extra_args = ["-cdrom", self._gen_cloud_init_iso()])
+self.wait_ssh()
+self.ssh_root_check("touch /etc/cloud/cloud-init.disabled")
+self.ssh_root_check("apt-get update")
+self.ssh_root_check("apt-get install -y cloud-initramfs-growroot")
+# Don't check the status in case the guest hang up too quickly
+self.ssh_root("sync && reboot")
+time.sleep(5)
+self.wait_ssh()
+# The previous update sometimes doesn't survive a reboot, so do it 
again
+self.ssh_root_check("apt-get update")
+self.ssh_root_check("apt-get build-dep -y qemu")
+self.ssh_root_check("apt-get install -y libfdt-dev")
+self.ssh_root("poweroff")
+self.wait()
+if os.path.exists(img):
+os.remove(img)
+os.rename(img_tmp, img)
+return 0
+
+if __name__ == "__main__":
+sys.exit(basevm.main(UbuntuX86VM))
-- 
2.13.5

[Qemu-devel] [PATCH v5 01/12] gitignore: Ignore vm test images

[Fam Zheng]

Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Fam Zheng 
---
 .gitignore | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.gitignore b/.gitignore
index cf65316863..643e23e515 100644
--- a/.gitignore
+++ b/.gitignore
@@ -52,6 +52,8 @@
 /vscclient
 /vhost-user-scsi
 /fsdev/virtfs-proxy-helper
+/tests/vm/*.img
+*.tmp
 *.[1-9]
 *.a
 *.aux
-- 
2.13.5

[Qemu-devel] [PATCH v5 03/12] scripts: Add archive-source.sh

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 scripts/archive-source.sh | 29 +
 1 file changed, 29 insertions(+)
 create mode 100755 scripts/archive-source.sh

diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
new file mode 100755
index 00..84e84961d4
--- /dev/null
+++ b/scripts/archive-source.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+#
+# Author: Fam Zheng 
+#
+# Create archive of source tree, including submodules
+#
+
+set -e
+
+if test $# -lt 1; then
+echo "Usage: $0 "
+exit 1
+fi
+
+submodules=$(git submodule foreach --recursive --quiet 'echo $name')
+
+if test -n "$submodules"; then
+{
+git ls-files
+for sm in $submodules; do
+(cd $sm; git ls-files) | sed "s:^:$sm/:"
+done
+} | grep -x -v $(for sm in $submodules; do echo "-e $sm"; done) > $1.list
+else
+git ls-files > $1.list
+fi
+
+tar -cf $1 -T $1.list
+rm $1.list
-- 
2.13.5

[Qemu-devel] [PATCH v5 00/12] tests: Add VM based build tests (for non-x86_64 and/or non-Linux)

[Fam Zheng]

v5: Generate source tar file with a script.
Fix tmpdir, use pwd.
Reduce default -j to half cores.

v4: Drop unused imports and parameters. [Cleber]
Use --exclude-vcs (still no --exclude-vcs-ignores because it's too new). 
[Philippe]
Use gtar if available. [Philippe, Kamil]
/dev/ld1a -> /dev/rld1a for netbsd. [Kamil]
Only use '-enable-kvm' if /dev/kvm is there. [Kamil]
Grammar fixes of README. [Stefan]
Rename image on the server to include version and arch. [Kamil]
Just ignore *.tmp. [Philippe]

v3: Drop RFC.
Add Stefan's and Kamil's reviewed-bys.
Use optparse. [Stefan]
Drop the VGA patch. [Paolo, Stefan]
Improve exit/exit code/doc. [Stefan]
Drop unused line from basevm.py. [Stefan]
Drop "--target-list" form Makefile.
More intelligent '-j'.
Add README. [Stefan]

v2: - Add docstring. [Stefan]
- Call self._load_io_lod. [Stefan]
- Use "info usernet" and dynamic ssh_port forwarding. [Stefan]
- Add image checksum.
- Use os.rename() and os.makedirs(). [Stefan]
- Fix NetBSD URL. [Kamil]

Build tests in one 32 bit Linux guest and three BSD images are defined in this
series. This is a more managable way than the manually maintained virtual
machines in patchew. Also, one big advantage of ephemeral VMs over long running
guests is the reduced RAM usage of host, which makes it possible to have one
host test all these BSD variants and probably more.

The BSD guest templates are manually prepared following

https://wiki.qemu.org/Hosts/BSD

as it is not easy to automate. (The ideal approach is like the ubuntu.i386
script, which configures the guest on top of an official released image, fully
automatically.)

Need for help: "gmake check" in the added OpenBSD image fails with -ENOMEM
errors, even if I change "-m 2G" to "-m 8G" when starting VM. Ideas? And there
is a warning from ./configure about OpenBSD going to be unsupported in coming
releases, is it still the case?

Fam

Fam Zheng (12):
  gitignore: Ignore vm test images
  qemu.py: Add "wait()" method
  scripts: Add archive-source.sh
  tests: Add vm test lib
  tests: Add ubuntu.i386 image
  tests: Add FreeBSD image
  tests: Add NetBSD image
  tests: Add OpenBSD image
  Makefile: Add rules to run vm tests
  MAINTAINERS: Add tests/vm entry
  tests: Add README for vm tests
  docker: Use archive-source.py

 .gitignore|   2 +
 MAINTAINERS   |   1 +
 Makefile  |   2 +
 configure |   2 +-
 scripts/archive-source.sh |  29 +
 scripts/qemu.py   |   7 ++
 tests/docker/Makefile.include |  15 +--
 tests/docker/run  |   8 +-
 tests/vm/Makefile.include |  42 +++
 tests/vm/README   |  63 ++
 tests/vm/basevm.py| 276 ++
 tests/vm/freebsd  |  42 +++
 tests/vm/netbsd   |  42 +++
 tests/vm/openbsd  |  43 +++
 tests/vm/ubuntu.i386  |  88 ++
 15 files changed, 641 insertions(+), 21 deletions(-)
 create mode 100755 scripts/archive-source.sh
 create mode 100644 tests/vm/Makefile.include
 create mode 100644 tests/vm/README
 create mode 100755 tests/vm/basevm.py
 create mode 100755 tests/vm/freebsd
 create mode 100755 tests/vm/netbsd
 create mode 100755 tests/vm/openbsd
 create mode 100755 tests/vm/ubuntu.i386

-- 
2.13.5

[Qemu-devel] [PATCH v5 02/12] qemu.py: Add "wait()" method

[Fam Zheng]

Signed-off-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
---
 scripts/qemu.py | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 880e3e8219..153f2d1564 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -143,6 +143,13 @@ class QEMUMachine(object):
 self._post_shutdown()
 raise
 
+def wait(self):
+'''Wait for the VM to power off'''
+self._popen.wait()
+self._qmp.close()
+self._load_io_log()
+self._post_shutdown()
+
 def shutdown(self):
 '''Terminate the VM and clean up'''
 if self.is_running():
-- 
2.13.5

[Qemu-devel] [PATCH v5 04/12] tests: Add vm test lib

[Fam Zheng]

This is the common code to implement a "VM test" to

  1) Download and initialize a pre-defined VM that has necessary
  dependencies to build QEMU and SSH access.

  2) Archive $SRC_PATH to a .tar file.

  3) Boot the VM, and pass the source tar file to the guest.

  4) SSH into the VM, untar the source tarball, build from the source.

Signed-off-by: Fam Zheng 
---
 tests/vm/basevm.py | 276 +
 1 file changed, 276 insertions(+)
 create mode 100755 tests/vm/basevm.py

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
new file mode 100755
index 00..9db91d61fa
--- /dev/null
+++ b/tests/vm/basevm.py
@@ -0,0 +1,276 @@
+#!/usr/bin/env python
+#
+# VM testing base class
+#
+# Copyright (C) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import os
+import sys
+import logging
+import time
+import datetime
+sys.path.append(os.path.join(os.path.dirname(__file__), "..", "..", "scripts"))
+from qemu import QEMUMachine
+import subprocess
+import hashlib
+import optparse
+import atexit
+import tempfile
+import shutil
+import multiprocessing
+import traceback
+
+SSH_KEY = """\
+-BEGIN RSA PRIVATE KEY-
+MIIEowIBAAKCAQEAopAuOlmLV6LVHdFBj8/eeOwI9CqguIJPp7eAQSZvOiB4Ag/R
+coEhl/RBbrV5Yc/SmSD4PTpJO/iM10RwliNjDb4a3I8q3sykRJu9c9PI/YsH8WN9
++NH2NjKPtJIcKTu287IM5JYxyB6nDoOzILbTyJ1TDR/xH6qYEfBAyiblggdjcvhA
+RTf93QIn39F/xLypXvT1K2O9BJEsnJ8lEUvB2UXhKo/JTfSeZF8wPBeowaP9EONk
+7b+nuJOWHGg68Ji6wVi62tjwl2Szch6lxIhZBpnV7QNRKMfYHP6eIyF4pusazzZq
+Telsq6xI2ghecWLzb/MF5A+rklsGx2FNuJSAJwIDAQABAoIBAHHi4o/8VZNivz0x
+cWXn8erzKV6tUoWQvW85Lj/2RiwJvSlsnYZDkx5af1CpEE2HA/pFT8PNRqsd+MWC
+7AEy710cVsM4BYerBFYQaYxwzblaoojo88LSjVPw3h5Z0iLM8+IMVd36nwuc9dpE
+R8TecMZ1+U4Tl6BgqkK+9xToZRdPKdjS8L5MoFhGN+xY0vRbbJbGaV9Q0IHxLBkB
+rEBV7T1mUynneCHRUQlJQEwJmKpT8MH3IjsUXlG5YvnuuvcQJSNTaW2iDLxuOKp8
+cxW8+qL88zpb1D5dppoIu6rlrugN0azSq70ruFJQPc/A8GQrDKoGgRQiagxNY3u+
+vHZzXlECgYEA0dKO3gfkSxsDBb94sQwskMScqLhcKhztEa8kPxTx6Yqh+x8/scx3
+XhJyOt669P8U1v8a/2Al+s81oZzzfQSzO1Q7gEwSrgBcRMSIoRBUw9uYcy02ngb/
+j/ng3DGivfJztjjiSJwb46FHkJ2JR8mF2UisC6UMXk3NgFY/3vWQx78CgYEAxlcG
+T3hfSWSmTgKRczMJuHQOX9ULfTBIqwP5VqkkkiavzigGRirzb5lgnmuTSPTpF0LB
+XVPjR2M4q+7gzP0Dca3pocrvLEoxjwIKnCbYKnyyvnUoE9qHv4Kr+vDbgWpa2LXG
+JbLmE7tgTCIp20jOPPT4xuDvlbzQZBJ5qCQSoZkCgYEAgrotSSihlCnAOFSTXbu4
+CHp3IKe8xIBBNENq0eK61kcJpOxTQvOha3sSsJsU4JAM6+cFaxb8kseHIqonCj1j
+bhOM/uJmwQJ4el/4wGDsbxriYOBKpyq1D38gGhDS1IW6kk3erl6VAb36WJ/OaGum
+eTpN9vNeQWM4Jj2WjdNx4QECgYAwTdd6mU1TmZCrJRL5ZG+0nYc2rbMrnQvFoqUi
+BvWiJovggHzur90zy73tNzPaq9Ls2FQxf5G1vCN8NCRJqEEjeYCR59OSDMu/EXc2
+CnvQ9SevHOdS1oEDEjcCWZCMFzPi3XpRih1gptzQDe31uuiHjf3cqcGPzTlPdfRt
+D8P92QKBgC4UaBvIRwREVJsdZzpIzm224Bpe8LOmA7DeTnjlT0b3lkGiBJ36/Q0p
+VhYh/6cjX4/iuIs7gJbGon7B+YPB8scmOi3fj0+nkJAONue1mMfBNkba6qQTc6Y2
+5mEKw2/O7/JpND7ucU3OK9plcw/qnrWDgHxl0Iz95+OzUIIagxne
+-END RSA PRIVATE KEY-
+"""
+SSH_PUB_KEY = """\
+ssh-rsa 
B3NzaC1yc2EDAQABAAABAQCikC46WYtXotUd0UGPz9547Aj0KqC4gk+nt4BBJm86IHgCD9FygSGX9EFutXlhz9KZIPg9Okk7+IzXRHCWI2MNvhrcjyrezKREm71z08j9iwfxY3340fY2Mo+0khwpO7bzsgzkljHIHqcOg7MgttPInVMNH/EfqpgR8EDKJuWCB2Ny+EBFN/3dAiff0X/EvKle9PUrY70EkSycnyURS8HZReEqj8lN9J5kXzA8F6jBo/0Q42Ttv6e4k5YcaDrwmLrBWLra2PCXZLNyHqXEiFkGmdXtA1Eox9gc/p4jIXim6xrPNmpN6WyrrEjaCF5xYvNv8wXkD6uSWwbHYU24lIAn
 qemu-vm-key
+"""
+
+class BaseVM(object):
+GUEST_USER = "qemu"
+GUEST_PASS = "qemupass"
+ROOT_PASS = "qemupass"
+
+# The script to run in the guest that builds QEMU
+BUILD_SCRIPT = ""
+# The guest name, to be overridden by subclasses
+name = "#base"
+def __init__(self, debug=False, vcpus=None):
+self._guest = None
+self._tmpdir = tempfile.mkdtemp(prefix="vm-test-", suffix=".tmp", 
dir=".")
+atexit.register(shutil.rmtree, self._tmpdir)
+
+self._ssh_key_file = os.path.join(self._tmpdir, "id_rsa")
+open(self._ssh_key_file, "w").write(SSH_KEY)
+subprocess.check_call(["chmod", "600", self._ssh_key_file])
+
+self._ssh_pub_key_file = os.path.join(self._tmpdir, "id_rsa.pub")
+open(self._ssh_pub_key_file, "w").write(SSH_PUB_KEY)
+
+self.debug = debug
+self._stderr = sys.stderr
+self._devnull = open("/dev/null", "w")
+if self.debug:
+self._stdout = sys.stdout
+else:
+self._stdout = self._devnull
+self._args = [ \
+"-nodefaults", "-m", "2G",
+"-cpu", "host",
+"-netdev", "user,id=vnet,hostfwd=:0.0.0.0:0-:22",
+"-device", "virtio-net-pci,netdev=vnet",
+"-vnc", ":0,to=20",
+"-serial", "file:%s" % os.path.join(self._tmpdir, "serial.out")]
+if vcpus:
+self._args += ["-smp", str(vcpus)]
+if os.access("/dev/kvm", os.R_OK | os.W_OK):
+self._args += ["-enable-kvm"]
+else:
+logging.info("KVM not available, not using -

Re: [Qemu-devel] [PATCH v1 08/11] s390x: allow only 1 CPU with TCG

[Cornelia Huck]

On Wed, 30 Aug 2017 21:06:55 +0200
Thomas Huth  wrote:

> On 30.08.2017 19:05, David Hildenbrand wrote:
> > Specifying more than 1 CPU (e.g. -smp 5) leads to SIGP errors (the
> > guest tries to bring these CPUs up but fails), because we don't support
> > multiple CPUs on s390x under TCG.
> > 
> > Let's bail out if more than 1 are specified, so we don't raise people's
> > hope.  
> 
> Aurelien recently posted a patch to add that basic SIGP support:
> 
>  https://patchwork.kernel.org/patch/9717489/
> 
> I think it would make more sense to get that included instead.

I'd look at it if it were reposted :)

[Qemu-devel] [PATCH 1/1] ppc: spapr: Move VCPU ID calculation into sPAPR

[Sam Bobroff]

Move the calculation of a CPU's VCPU ID out of the generic PPC code
(ppc_cpu_realizefn()) and into sPAPR specific code
(spapr_cpu_core_realize()) where it belongs.

Unfortunately, due to the way things are ordered, we still need to
default the VCPU ID in ppc_cpu_realizfn() but at least doing that
doesn't require any interaction with sPAPR.

Signed-off-by: Sam Bobroff 
---
This is follow up work arising from my work to clean up the way CPU VCPU IDs are
handled on PowerPC. It had looked like it would be difficult to move the actual
VCPU ID calculation out of generic code but it turned out to be OK.

It's based on dgibson/ppc-for-2.11.

Cheers,
Sam.

 hw/ppc/spapr_cpu_core.c | 11 +++
 target/ppc/translate_init.c | 18 +++---
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 5e319d9bbb..84dcc6e264 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -211,6 +211,7 @@ error:
 
 static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
 {
+sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
 sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
 sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev));
 CPUCore *cc = CPU_CORE(OBJECT(dev));
@@ -237,6 +238,16 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error 
**errp)
 cs = CPU(obj);
 cpu = POWERPC_CPU(cs);
 cs->cpu_index = cc->core_id + i;
+cpu->vcpu_id = (cc->core_id * spapr->vsmt / smp_threads) + i;
+if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->vcpu_id)) {
+error_setg(&local_err, "Can't create CPU with id %d in KVM",
+   cpu->vcpu_id);
+error_append_hint(&local_err, "Adjust the number of cpus to %d "
+  "or try to raise the number of threads per 
core\n",
+  cpu->vcpu_id * smp_threads / spapr->vsmt);
+goto err;
+}
+
 
 /* Set NUMA node for the threads belonged to core  */
 cpu->node_id = sc->node_id;
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index 7f6a349e43..1f7286c893 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -9903,28 +9903,15 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error 
**errp)
 PowerPCCPU *cpu = POWERPC_CPU(dev);
 PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
 Error *local_err = NULL;
-#if !defined(CONFIG_USER_ONLY)
-int max_smt = kvmppc_smt_threads();
-#endif
 
 cpu_exec_realizefn(cs, &local_err);
 if (local_err != NULL) {
 error_propagate(errp, local_err);
 return;
 }
-
-#if !defined(CONFIG_USER_ONLY)
-cpu->vcpu_id = (cs->cpu_index / smp_threads) * max_smt
-+ (cs->cpu_index % smp_threads);
-
-if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->vcpu_id)) {
-error_setg(errp, "Can't create CPU with id %d in KVM", cpu->vcpu_id);
-error_append_hint(errp, "Adjust the number of cpus to %d "
-  "or try to raise the number of threads per core\n",
-  cpu->vcpu_id * smp_threads / max_smt);
-goto unrealize;
+if (cpu->vcpu_id == UNASSIGNED_CPU_INDEX) {
+cpu->vcpu_id = cs->cpu_index;
 }
-#endif
 
 if (tcg_enabled()) {
 if (ppc_fixup_cpu(cpu) != 0) {
@@ -10625,6 +10612,7 @@ static void ppc_cpu_initfn(Object *obj)
 CPUPPCState *env = &cpu->env;
 
 cs->env_ptr = env;
+cpu->vcpu_id = UNASSIGNED_CPU_INDEX;
 
 env->msr_mask = pcc->msr_mask;
 env->mmu_model = pcc->mmu_model;
-- 
2.14.1.4.g334a7be4f

Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH

[Cornelia Huck]

On Thu, 31 Aug 2017 07:51:17 +0200
Thomas Huth  wrote:

> On 30.08.2017 18:36, Halil Pasic wrote:
> > The function ioinst_handle_xsch is presenting cc 2 when it's supposed to
> > present cc 1 and the other way around, because css_do_xsch has the error
> > codes mixed up. Fixing the error codes also fixes the priority.
> > 
> > Let us fix this.  
> 
> (Nit: In case you respin, I'd suggest to remove the last sentence. You
> already used "fix" two times in the previous one)

I can also remove it on applying, if Halil agrees (I have not yet
reviewed it, though).

> 
> > Signed-off-by: Halil Pasic 
> > Reported-by: Pierre Morel  
> 
> Space missing -^

And I can also add that space on applying :)

> 
> > ---
> >  hw/s390x/css.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> > index 1880b1a0ff..a50fb0727e 100644
> > --- a/hw/s390x/css.c
> > +++ b/hw/s390x/css.c
> > @@ -1281,12 +1281,12 @@ int css_do_xsch(SubchDev *sch)
> >  (!(s->ctrl &
> > (SCSW_ACTL_RESUME_PEND | SCSW_ACTL_START_PEND | 
> > SCSW_ACTL_SUSP))) ||
> >  (s->ctrl & SCSW_ACTL_SUBCH_ACTIVE)) {
> > -ret = -EINPROGRESS;
> > +ret = -EBUSY;
> >  goto out;
> >  }
> >  
> >  if (s->ctrl & SCSW_CTRL_MASK_STCTL) {
> > -ret = -EBUSY;
> > +ret = -EINPROGRESS;
> >  goto out;
> >  }  
> 
> Using both, EBUSY and EINPROGRESS as error codes sounds very confusing
> to me here ... what's the difference between busy and in-progress? So
> while you're at it, maybe you could replace the code for CC 2 ("CANCEL
> SUBCHANNEL not applicable") with a different error code?

IIRC, I used these two as they matched my idea of what happens best
(there's a difference between "subchannel is busy" and "the I/O is
already in progress, too late to cancel"). "xsch not applicable" is
very hard to translate to an Unix error code :/

I'll double-check with the PoP.

Re: [Qemu-devel] [PATCH] scripts: Support building with Python 3

[Markus Armbruster]

Stefan Hajnoczi  writes:

> On Mon, Aug 21, 2017 at 04:29:44PM +0200, Markus Armbruster wrote:
>> What is our Python 2 -> 3 migration strategy?
>> 
>> Don't support Python 3 until a flag day, then flip and don't support
>> Python 2?
>
> Add support for Python 3 so that both Python 2.6+ and Python 3 are
> supported.

Python 2.7 is the end of the Python 2 line of development (PEP 404).
It'll be maintained until 2020.  Downstream support for Python 2 may or
may not last longer.

> When Python 2 is EOL it will need to be dropped and the code becomes
> Python 3-only.
>
>> Keep the source in Python 2 and support 3 via conversion with 2to3?
>> 
>> Port the source to Python 3 and support 2 via conversion with 3to2?
>> 
>> Port to Python 3, but keep it working with Python 2 with help of the six
>> module?
>> 
>> Support both with ad hoc hackery (like this patch does)?
>
> Yes, please see for details:
> https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01758.html

So, first we'll invest in work-arounds to make both 2 and 3 work.  Once
2 is gone, we can invest some more to clean them up.  Which probably
won't happen, so we'll continue to carry work-arounds that no longer
make sense.

I maintain roughly one fourth of all Python code in qemu, and I'm not
looking forward to this hoop-jumping at all.

Are we really, really sure we want to go this way?  What exactly are we
hoping to accomplish by it?

Re: [Qemu-devel] [PATCH 3/9] s390x/css: be more consistent if broken beyond repair

[Thomas Huth]

On 30.08.2017 18:36, Halil Pasic wrote:
> If we detect that the internally manged state of the subchannel
> is broken beyond repair while in do_subchannel_work in case of
> virtual we just abort the operation and pretend all went well,
> while in case of pass-through we honor the situation with -ENODEV
> which results in cc 3 for the instruction whose handler triggered
> the call.
> 
> Let's be consistent on this and do the -ENODEV also for virtual
> subchannels.
> 
> Signed-off-by: Halil Pasic 
> Acked-by: Pierre Morel
> ---
>  hw/s390x/css.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index 0822538cde..bc47bf5b20 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -1078,7 +1078,7 @@ int do_subchannel_work_virtual(SubchDev *sch)
>  sch_handle_start_func_virtual(sch);
>  } else {
>  /* Cannot happen. */
> -return 0;
> +return -ENODEV;
>  }
>  css_inject_io_interrupt(sch);
>  return 0;

First, I think ENODEV is not really a good choice here since there
certainly was a device. So maybe use EINVAL or EBADR or something else
instead?

Second, while return an error code is certainly better than returning 0,
I think most errors will still go unnoticed here, since most callers of
do_subchannel_work() seem to ignore the return value ... so I wonder
whether we rather want to have another way to recognize this condition.
If the comment is right and this really can not happen, I think you
should use an g_assert_notreached() here instead. Otherwise the comment
should be changed and it's maybe a good idea to use a
qemu_log_mask(LOG_GUEST_ERROR, "subchannel in bad state bla bla...") here.

 Thomas

Re: [Qemu-devel] [PATCH for-2.11 5/6] ppc: simplify cpu model lookup by PVR

[David Gibson]

On Wed, Aug 30, 2017 at 12:50:33PM +0200, Igor Mammedov wrote:
> On Fri, 25 Aug 2017 19:32:29 +1000
> David Gibson  wrote:
> 
> > On Fri, Aug 25, 2017 at 09:40:37AM +0200, Igor Mammedov wrote:
> > > On Fri, 25 Aug 2017 14:16:44 +1000
> > > David Gibson  wrote:
> > >   
> > > > On Thu, Aug 24, 2017 at 10:21:50AM +0200, Igor Mammedov wrote:  
> > > > > Signed-off-by: Igor Mammedov 
> > > > > ---
> > > > >  target/ppc/translate_init.c | 27 +++
> > > > >  1 file changed, 11 insertions(+), 16 deletions(-)
> > > > > 
> > > > > diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> > > > > index f1a559d..ca9f1e3 100644
> > > > > --- a/target/ppc/translate_init.c
> > > > > +++ b/target/ppc/translate_init.c
> > > > > @@ -34,6 +34,7 @@
> > > > >  #include "hw/ppc/ppc.h"
> > > > >  #include "mmu-book3s-v3.h"
> > > > >  #include "sysemu/qtest.h"
> > > > > +#include "qemu/cutils.h"
> > > > >  
> > > > >  //#define PPC_DUMP_CPU
> > > > >  //#define PPC_DEBUG_SPR
> > > > > @@ -10203,22 +10204,16 @@ static ObjectClass 
> > > > > *ppc_cpu_class_by_name(const char *name)
> > > > >  char *cpu_model, *typename;
> > > > >  ObjectClass *oc;
> > > > >  const char *p;
> > > > > -int i, len;
> > > > > -
> > > > > -/* Check if the given name is a PVR */
> > > > > -len = strlen(name);
> > > > > -if (len == 10 && name[0] == '0' && name[1] == 'x') {
> > > > > -p = name + 2;
> > > > > -goto check_pvr;
> > > > > -} else if (len == 8) {
> > > > > -p = name;
> > > > > -check_pvr:
> > > > > -for (i = 0; i < 8; i++) {
> > > > > -if (!qemu_isxdigit(*p++))
> > > > > -break;
> > > > > -}
> > > > > -if (i == 8) {
> > > > > -return OBJECT_CLASS(ppc_cpu_class_by_pvr(strtoul(name, 
> > > > > NULL, 16)));
> > > > > +unsigned long pvr;
> > > > > +
> > > > > +/* Lookup by PVR if cpu_model is valid 8 digit hex number
> > > > > + * (excl: 0x prefix if present)
> > > > > + */
> > > > > +if (!qemu_strtoul(name, &p, 16, &pvr)) {
> > > > > +int len = p - name;
> > > > > +len = (len == 10) && (name[1] == 'x') ? len - 2 : len;
> > > > > +if (len == 8) {
> > > > > +return OBJECT_CLASS(ppc_cpu_class_by_pvr(pvr));
> > > > 
> > > > This doesn't look quite right.  A hex string of a PVR followed by
> > > > other stuff isn't a valid option, so if p (endptr) doesn't point to
> > > > the end of the string, then we shouldn't be using this path  
> > > yep, my mistake, I've lost somewhere *p == '\0' check when cleaning up 
> > > the patch  
> > 
> > Ok.
> > 
> > > >(from the
> > > > looks of qemu_strtoul() we can simply omit the endptr parameter to get
> > > > that behaviour).  I'm not sure what the messing around with len is
> > > > for, either.  If it's a valid hex string we can try this, I don't see
> > > > that we need further checks.  
> > > I've tried to keep current limitation here i.e. 8 hex symbols,
> > > but if any hex number is fine then doing
> > >  qemu_strtoul(name, NULL, 16, &pvr)
> > > is even better, so would you prefer to drop 8 symbol check altogether?  
> > 
> > Yeah, I don't see any value to the 8 character check.
> there are cpu models consisting of pure numbers => valid hex number
> so if check is dropped then it lookup will go PVR route,
> that will fail there.
> So check should be there to avoid regression.

Ah.  Good point.

> I'll fix whole string consumption check that I've missed before
> and respin with it.
> 
> 
> 

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH

[Thomas Huth]

On 30.08.2017 18:36, Halil Pasic wrote:
> The function ioinst_handle_xsch is presenting cc 2 when it's supposed to
> present cc 1 and the other way around, because css_do_xsch has the error
> codes mixed up. Fixing the error codes also fixes the priority.
> 
> Let us fix this.

(Nit: In case you respin, I'd suggest to remove the last sentence. You
already used "fix" two times in the previous one)

> Signed-off-by: Halil Pasic 
> Reported-by: Pierre Morel

Space missing -^

> ---
>  hw/s390x/css.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index 1880b1a0ff..a50fb0727e 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -1281,12 +1281,12 @@ int css_do_xsch(SubchDev *sch)
>  (!(s->ctrl &
> (SCSW_ACTL_RESUME_PEND | SCSW_ACTL_START_PEND | SCSW_ACTL_SUSP))) 
> ||
>  (s->ctrl & SCSW_ACTL_SUBCH_ACTIVE)) {
> -ret = -EINPROGRESS;
> +ret = -EBUSY;
>  goto out;
>  }
>  
>  if (s->ctrl & SCSW_CTRL_MASK_STCTL) {
> -ret = -EBUSY;
> +ret = -EINPROGRESS;
>  goto out;
>  }

Using both, EBUSY and EINPROGRESS as error codes sounds very confusing
to me here ... what's the difference between busy and in-progress? So
while you're at it, maybe you could replace the code for CC 2 ("CANCEL
SUBCHANNEL not applicable") with a different error code?

 Thomas

Re: [Qemu-devel] [PATCH for-2.11 v3 0/3] hw/ppc: CAS reset on early device hotplug

[David Gibson]

On Wed, Aug 30, 2017 at 03:21:38PM -0300, Daniel Henrique Barboza wrote:
> v3:
> - split into 3 patches, first 2 are fixes that are independent of the
> reboot code that can be applied separately:
> - patch 1: spapr_drc_needed fix
> - patch 2: clear pending_events on reboot, following David's
> suggestions on the v2 review.
> - patch 3: CAS reboot
> 
> v2:
> - rebased with ppc-for-2.11
> - function 'spapr_cas_completed' dropped
> - function 'spapr_drc_needed' made public and it's now used inside
>   'spapr_hotplugged_dev_before_cas'
> - 'spapr_drc_needed' was changed to support the migration of logical
>   DRCs with devs attached in UNUSED state
> - new function: 'spapr_clear_pending_events'. This function is used
>   inside ppc_spapr_reset to reset the pending_events QTAILQ

Applied to ppc-for-2.11.  The first two I'm also considering sending
off for the 2.10 stable branch.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 02/17] MAINTAINERS: add missing Versatile PB entry

[Thomas Huth]

On 30.08.2017 23:55, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b363e1b9c9..5b7891addc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -515,6 +515,7 @@ M: Peter Maydell 
>  L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/*/versatile*
> +F: hw/misc/arm_sysctl.c

I think you could also merge this into the previous patch. Anyway:

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH 1/7] serial: add serial_chr_nonnull() to use the null backend when none provided

[Thomas Huth]

On 31.08.2017 05:53, Philippe Mathieu-Daudé wrote:
> Suggested-by: Peter Maydell 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  include/hw/char/serial.h |  1 +
>  hw/char/serial.c | 13 +
>  2 files changed, 14 insertions(+)
> 
> diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
> index c4daf11a14..96bccb39e1 100644
> --- a/include/hw/char/serial.h
> +++ b/include/hw/char/serial.h
> @@ -93,6 +93,7 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
>  hwaddr base, int it_shift,
>  qemu_irq irq, int baudbase,
>  Chardev *chr, enum device_endian end);
> +Chardev *serial_chr_nonnull(Chardev *chr);

Why do you need the prototype? Please make the function static if
possible (otherwise please add some rationale in the patch description).

>  /* serial-isa.c */
>  #define TYPE_ISA_SERIAL "isa-serial"
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index 9aec6c60d8..7a100db107 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -1025,6 +1025,19 @@ static const MemoryRegionOps serial_mm_ops[3] = {
>  },
>  };
>  
> +Chardev *serial_chr_nonnull(Chardev *chr)
> +{
> +static int serial_id;
> +char *label;
> +
> +label = g_strdup_printf("discarding-serial%d", serial_id++);
> +chr = qemu_chr_new(label, "null");

That looks wrong - you're ignoring the input parameter and always open
the "null" device? Shouldn't there be a "if (chr) return chr;" in front
of this?

> +assert(chr);
> +g_free(label);
> +
> +return chr;
> +}

 Thomas

PS: I think you should also merge the two patches together, they are
small enough.

Re: [Qemu-devel] [PATCH v2 01/17] MAINTAINERS: add missing ARM entries

[Thomas Huth]

On 30.08.2017 23:55, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  MAINTAINERS | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ccee28b12d..b363e1b9c9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -380,6 +380,7 @@ M: Peter Maydell 
>  L: qemu-...@nongnu.org
>  S: Maintained
>  F: hw/char/pl011.c
> +F: include/hw/char/pl011.h
>  F: hw/display/pl110*
>  F: hw/dma/pl080.c
>  F: hw/dma/pl330.c
> @@ -403,13 +404,15 @@ F: hw/intc/gic_internal.h
>  F: hw/misc/a9scu.c
>  F: hw/misc/arm11scu.c
>  F: hw/timer/a9gtimer*
> -F: hw/timer/arm_*
> -F: include/hw/arm/arm.h
> +F: hw/timer/arm*
> +F: include/hw/arm/arm*.h
>  F: include/hw/intc/arm*
>  F: include/hw/misc/a9scu.h
>  F: include/hw/misc/arm11scu.h
>  F: include/hw/timer/a9gtimer.h
>  F: include/hw/timer/arm_mptimer.h
> +F: include/hw/timer/armv7m_systick.h
> +F: tests/test-arm-mptimer.c
>  
>  Exynos
>  M: Igor Mitsyanko 

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH v2 00/17] add missing entries in MAINTAINERS

[Philippe Mathieu-Daudé]

Cc'ing Markus since I forgot to include him.

On 08/30/2017 06:55 PM, Philippe Mathieu-Daudé wrote:

Hi,

I tried to have a more helpful ./scripts/get_maintainer.pl output, filling
missing entries in MAINTAINERS.

Regards,

Phil.

v2:
- add R-b & A-b
- clean ARM entries (Thomas Huth)
- moved files: comment since which commit (Eric Blake)
- drop inconsistent patches (default-configs.mak related to hw/machine, no CPU)
- drop unhappy patches (include/standard-headers/linux/*)
- drop unreplied patches

Philippe Mathieu-Daudé (17):
   MAINTAINERS: add missing ARM entries
   MAINTAINERS: add missing Versatile PB entry
   MAINTAINERS: add missing STM32 entry
   MAINTAINERS: add missing entry for vhost
   MAINTAINERS: add missing VMWare entry
   MAINTAINERS: add missing Guest Agent entries
   MAINTAINERS: add missing qcow2 entry
   MAINTAINERS: add missing USB entry
   MAINTAINERS: add missing PCI entries
   MAINTAINERS: add missing SSI entries
   MAINTAINERS: add missing entries for throttling infra
   MAINTAINERS: add missing megasas test entry
   MAINTAINERS: add missing AIO entry
   MAINTAINERS: add missing entry for Generic Loader
   MAINTAINERS: add missing Cryptography entry
   MAINTAINERS: update docs/devel/ entries
   MAINTAINERS: update docs/interop/ entries

  MAINTAINERS | 44 ++--
  1 file changed, 34 insertions(+), 10 deletions(-)

[Qemu-devel] [PATCH 5/7] hw/char/exynos4210_uart: use serial_chr_nonnull()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
This ARRAY_SIZE() first surprised me but was valid :)

 hw/char/exynos4210_uart.c | 14 ++
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/hw/char/exynos4210_uart.c b/hw/char/exynos4210_uart.c
index 3957e78abf..b6cdfc3006 100644
--- a/hw/char/exynos4210_uart.c
+++ b/hw/char/exynos4210_uart.c
@@ -27,6 +27,7 @@
 #include "chardev/char-serial.h"
 
 #include "hw/arm/exynos4210.h"
+#include "hw/char/serial.h"
 
 #undef DEBUG_UART
 #undef DEBUG_UART_EXTEND
@@ -589,9 +590,6 @@ DeviceState *exynos4210_uart_create(hwaddr addr,
 DeviceState  *dev;
 SysBusDevice *bus;
 
-const char chr_name[] = "serial";
-char label[ARRAY_SIZE(chr_name) + 1];
-
 dev = qdev_create(NULL, TYPE_EXYNOS4210_UART);
 
 if (!chr) {
@@ -600,15 +598,7 @@ DeviceState *exynos4210_uart_create(hwaddr addr,
  MAX_SERIAL_PORTS);
 exit(1);
 }
-chr = serial_hds[channel];
-if (!chr) {
-snprintf(label, ARRAY_SIZE(label), "%s%d", chr_name, channel);
-chr = qemu_chr_new(label, "null");
-if (!(chr)) {
-error_report("Can't assign serial port to UART%d", channel);
-exit(1);
-}
-}
+chr = serial_chr_nonnull(serial_hds[channel]);
 }
 
 qdev_prop_set_chr(dev, "chardev", chr);
-- 
2.14.1

[Qemu-devel] [PATCH 6/7] hw/char/omap_uart: serial_mm_init() already check for null chr

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/char/omap_uart.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/char/omap_uart.c b/hw/char/omap_uart.c
index 6fd1b9cf6b..1f0ac0a053 100644
--- a/hw/char/omap_uart.c
+++ b/hw/char/omap_uart.c
@@ -63,8 +63,7 @@ struct omap_uart_s *omap_uart_init(hwaddr base,
 s->irq = irq;
 s->serial = serial_mm_init(get_system_memory(), base, 2, irq,
omap_clk_getrate(fclk)/16,
-   chr ?: qemu_chr_new(label, "null"),
-   DEVICE_NATIVE_ENDIAN);
+   chr, DEVICE_NATIVE_ENDIAN);
 return s;
 }
 
@@ -183,6 +182,5 @@ void omap_uart_attach(struct omap_uart_s *s, Chardev *chr)
 /* TODO: Should reuse or destroy current s->serial */
 s->serial = serial_mm_init(get_system_memory(), s->base, 2, s->irq,
omap_clk_getrate(s->fclk) / 16,
-   chr ?: qemu_chr_new("null", "null"),
-   DEVICE_NATIVE_ENDIAN);
+   chr, DEVICE_NATIVE_ENDIAN);
 }
-- 
2.14.1

Re: [Qemu-devel] [PATCH v2 0/7] QOMify MIPS cpu

[Philippe Mathieu-Daudé]

Cc'ing Xiaoqiang Zhao, I forgot to include him.

On 08/30/2017 07:52 PM, Philippe Mathieu-Daudé wrote:

Hi,

This series is based on Igor's "complete cpu QOMification" [1] but only modify
the MIPS part. Igor posted an updated series [2], both series should apply
separately.

Igor suggested on IRC this series could enter via the Machine core tree, so
I added Eduardo and Marcel.

Regards,

Phil.

[1]: http://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg04414.html
[2]: http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg03364.html

v2:
- added Igor and James Tested-by
- squashed "!fixup mips: now than MIPSCPU is QOMified, mark it abstract"

PS: code movement somehow triggers a "binary vs unary operators" confusion
 in checkpatch: "ERROR: space prohibited after that '&' (ctx:WxW)"

Igor Mammedov (2):
   mips: MIPSCPU model subclasses
   mips: replace cpu_mips_init() with cpu_generic_init()

Philippe Mathieu-Daudé (5):
   mips: move hw/mips/cputimer.c to target/mips/
   mips: introduce internal.h and cleanup cpu.h
   mips: split cpu_mips_realize_env() out of cpu_mips_init()
   mips: call cpu_mips_realize_env() from mips_cpu_realizefn()
   mips: update mips_cpu_list() to use object_class_get_list()

  target/mips/cpu-qom.h |   1 +
  target/mips/cpu.h | 357 +-
  target/mips/internal.h| 422 ++
  hw/mips/cps.c |   2 +-
  hw/mips/mips_fulong2e.c   |   2 +-
  hw/mips/mips_jazz.c   |   2 +-
  hw/mips/mips_malta.c  |   2 +-
  hw/mips/mips_mipssim.c|   2 +-
  hw/mips/mips_r4k.c|   2 +-
  hw/mips/cputimer.c => target/mips/cp0_timer.c |   2 +-
  target/mips/cpu.c |  57 +++-
  target/mips/gdbstub.c |   1 +
  target/mips/helper.c  |  47 +++
  target/mips/kvm.c |   1 +
  target/mips/machine.c |   1 +
  target/mips/msa_helper.c  |   1 +
  target/mips/op_helper.c   |   1 +
  target/mips/translate.c   |  23 +-
  target/mips/translate_init.c  |  68 +
  hw/mips/Makefile.objs |   2 +-
  target/mips/Makefile.objs |   2 +-
  21 files changed, 549 insertions(+), 449 deletions(-)
  create mode 100644 target/mips/internal.h
  rename hw/mips/cputimer.c => target/mips/cp0_timer.c (99%)

[Qemu-devel] [PATCH 7/7] hw/xtensa: serial_mm_init() already check for null chr

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/xtensa/xtfpga.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index 635a4d4ec3..223f396902 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -263,10 +263,6 @@ static void lx_init(const LxBoardDesc *board, MachineState 
*machine)
 xtensa_get_extint(env, 1), nd_table);
 }
 
-if (!serial_hds[0]) {
-serial_hds[0] = qemu_chr_new("serial0", "null");
-}
-
 serial_mm_init(system_io, 0x0d050020, 2, xtensa_get_extint(env, 0),
 115200, serial_hds[0], DEVICE_NATIVE_ENDIAN);
 
-- 
2.14.1

[Qemu-devel] [PATCH 3/7] hw/arm/fsl_imx*: use serial_chr_nonnull()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/char/imx_serial.h |  1 +
 hw/arm/fsl-imx25.c   |  9 +
 hw/arm/fsl-imx31.c   |  9 +
 hw/arm/fsl-imx6.c| 10 +-
 4 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/include/hw/char/imx_serial.h b/include/hw/char/imx_serial.h
index baeec3183f..55139dc6ec 100644
--- a/include/hw/char/imx_serial.h
+++ b/include/hw/char/imx_serial.h
@@ -20,6 +20,7 @@
 
 #include "hw/sysbus.h"
 #include "chardev/char-fe.h"
+#include "hw/char/serial.h"
 
 #define TYPE_IMX_SERIAL "imx.serial"
 #define IMX_SERIAL(obj) OBJECT_CHECK(IMXSerialState, (obj), TYPE_IMX_SERIAL)
diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
index 3b97eceb3c..425a9edc36 100644
--- a/hw/arm/fsl-imx25.c
+++ b/hw/arm/fsl-imx25.c
@@ -120,14 +120,7 @@ static void fsl_imx25_realize(DeviceState *dev, Error 
**errp)
 if (i < MAX_SERIAL_PORTS) {
 Chardev *chr;
 
-chr = serial_hds[i];
-
-if (!chr) {
-char label[20];
-snprintf(label, sizeof(label), "imx31.uart%d", i);
-chr = qemu_chr_new(label, "null");
-}
-
+chr = serial_chr_nonnull(serial_hds[i]);
 qdev_prop_set_chr(DEVICE(&s->uart[i]), "chardev", chr);
 }
 
diff --git a/hw/arm/fsl-imx31.c b/hw/arm/fsl-imx31.c
index 0f2ebe8161..8d4535a536 100644
--- a/hw/arm/fsl-imx31.c
+++ b/hw/arm/fsl-imx31.c
@@ -109,14 +109,7 @@ static void fsl_imx31_realize(DeviceState *dev, Error 
**errp)
 if (i < MAX_SERIAL_PORTS) {
 Chardev *chr;
 
-chr = serial_hds[i];
-
-if (!chr) {
-char label[20];
-snprintf(label, sizeof(label), "imx31.uart%d", i);
-chr = qemu_chr_new(label, "null");
-}
-
+chr = serial_chr_nonnull(serial_hds[i]);
 qdev_prop_set_chr(DEVICE(&s->uart[i]), "chardev", chr);
 }
 
diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
index 26fd214004..7bc1aa1fbe 100644
--- a/hw/arm/fsl-imx6.c
+++ b/hw/arm/fsl-imx6.c
@@ -189,15 +189,7 @@ static void fsl_imx6_realize(DeviceState *dev, Error 
**errp)
 if (i < MAX_SERIAL_PORTS) {
 Chardev *chr;
 
-chr = serial_hds[i];
-
-if (!chr) {
-char *label = g_strdup_printf("imx6.uart%d", i + 1);
-chr = qemu_chr_new(label, "null");
-g_free(label);
-serial_hds[i] = chr;
-}
-
+chr = serial_chr_nonnull(serial_hds[i]);
 qdev_prop_set_chr(DEVICE(&s->uart[i]), "chardev", chr);
 }
 
-- 
2.14.1

[Qemu-devel] [PATCH 2/7] serial: use serial_chr_nonnull() in serial_mm_init()

[Philippe Mathieu-Daudé]

Suggested-by: Peter Maydell 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/char/serial.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index e4c38dc250..57e89468b4 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -1050,7 +1050,7 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
 s->it_shift = it_shift;
 s->irq = irq;
 s->baudbase = baudbase;
-qemu_chr_fe_init(&s->chr, chr, &error_abort);
+qemu_chr_fe_init(&s->chr, serial_chr_nonnull(chr), &error_abort);
 
 serial_realize_core(s, &error_fatal);
 vmstate_register(NULL, base, &vmstate_serial, s);
-- 
2.14.1

[Qemu-devel] [PATCH 0/7] serial: add serial_chr_nonnull()

[Philippe Mathieu-Daudé]

Hi,

This series add the serial_chr_nonnull() which connect to the "null" chardev
backend if none is provided.

Inspired by Peter's suggestion:
http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg05987.html
which also refers to this issue:
http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg03546.html

Some boards still check serial_hds[x] non null before calling serial_mm_init(),
this check could now be removed on the SoC which always have an UART mapped.

This might be a follow up if this series is accepted.

Regards,

Phil.

Philippe Mathieu-Daudé (7):
  serial: add serial_chr_nonnull() to use the null backend when none provided
  serial: use serial_chr_nonnull() in serial_mm_init()
  hw/arm/fsl_imx*: use serial_chr_nonnull()
  hw/mips/malta: use serial_chr_nonnull()
  hw/char/exynos4210_uart: use serial_chr_nonnull()
  hw/char/omap_uart: serial_mm_init() already check for null chr
  hw/xtensa: serial_mm_init() already check for null chr

 include/hw/char/imx_serial.h |  1 +
 include/hw/char/serial.h |  1 +
 hw/arm/fsl-imx25.c   |  9 +
 hw/arm/fsl-imx31.c   |  9 +
 hw/arm/fsl-imx6.c| 10 +-
 hw/char/exynos4210_uart.c| 14 ++
 hw/char/omap_uart.c  |  6 ++
 hw/char/serial.c | 15 ++-
 hw/mips/mips_malta.c |  6 +-
 hw/xtensa/xtfpga.c   |  4 
 10 files changed, 24 insertions(+), 51 deletions(-)

-- 
2.14.1

[Qemu-devel] [PATCH 4/7] hw/mips/malta: use serial_chr_nonnull()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/mips/mips_malta.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index af678f5784..8620e9c42c 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1035,11 +1035,7 @@ void mips_malta_init(MachineState *machine)
 
 /* Make sure the first 3 serial ports are associated with a device. */
 for(i = 0; i < 3; i++) {
-if (!serial_hds[i]) {
-char label[32];
-snprintf(label, sizeof(label), "serial%d", i);
-serial_hds[i] = qemu_chr_new(label, "null");
-}
+serial_hds[i] = serial_chr_nonnull(serial_hds[i]);
 }
 
 /* create CPU */
-- 
2.14.1

[Qemu-devel] [PATCH 1/7] serial: add serial_chr_nonnull() to use the null backend when none provided

[Philippe Mathieu-Daudé]

Suggested-by: Peter Maydell 
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/char/serial.h |  1 +
 hw/char/serial.c | 13 +
 2 files changed, 14 insertions(+)

diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
index c4daf11a14..96bccb39e1 100644
--- a/include/hw/char/serial.h
+++ b/include/hw/char/serial.h
@@ -93,6 +93,7 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
 hwaddr base, int it_shift,
 qemu_irq irq, int baudbase,
 Chardev *chr, enum device_endian end);
+Chardev *serial_chr_nonnull(Chardev *chr);
 
 /* serial-isa.c */
 #define TYPE_ISA_SERIAL "isa-serial"
diff --git a/hw/char/serial.c b/hw/char/serial.c
index 9aec6c60d8..7a100db107 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -1025,6 +1025,19 @@ static const MemoryRegionOps serial_mm_ops[3] = {
 },
 };
 
+Chardev *serial_chr_nonnull(Chardev *chr)
+{
+static int serial_id;
+char *label;
+
+label = g_strdup_printf("discarding-serial%d", serial_id++);
+chr = qemu_chr_new(label, "null");
+assert(chr);
+g_free(label);
+
+return chr;
+}
+
 SerialState *serial_mm_init(MemoryRegion *address_space,
 hwaddr base, int it_shift,
 qemu_irq irq, int baudbase,
-- 
2.14.1

Re: [Qemu-devel] [RFC v2 0/8] monitor: allow per-monitor thread

[Peter Xu]

On Wed, Aug 30, 2017 at 11:13:11AM +0100, Daniel P. Berrange wrote:
> On Wed, Aug 30, 2017 at 09:06:20AM +0200, Markus Armbruster wrote:
> > "Daniel P. Berrange"  writes:
> > 
> > > On Wed, Aug 23, 2017 at 02:51:03PM +0800, Peter Xu wrote:
> > 
> > >> However, even with the series, it does not mean that per-monitor
> > >> threads will never hang.  One example is that we can still run "info
> > >> vcpus" in per-monitor threads during a paused postcopy (in that state,
> > >> page faults are never handled, and "info cpus" will never return since
> > >> it tries to sync every vcpus).  So to make sure it does not hang, we
> > >> not only need the per-monitor thread, the user should be careful as
> > >> well on how to use it.
> > >> 
> > >> For postcopy recovery, we may need dedicated monitor channel for
> > >> recovery.  In other words, a destination VM that supports postcopy
> > >> recovery would possibly need:
> > >> 
> > >>   -qmp MAIN_CHANNEL -qmp RECOVERY_CHANNEL
> > 
> > Where RECOVERY_CHANNEL isn't necessarily just for postcopy, but for any
> > "emergency" QMP access.  If you use it only for commands that cannot
> > hang (i.e. terminate in bounded time), then you'll always be able to get
> > commands accepted there in bounded time.
> > 
> > > I think this is a really horrible thing to expose to management 
> > > applications.
> > > They should not need to be aware of fact that QEMU is buggy and thus 
> > > requires
> > > that certain commands be run on different monitors to work around the bug.
> > 
> > These are (serious) design limitations, not bugs in the narrow sense of
> > the word.
> > 
> > However, I quite agree that the need for clients to know whether a
> > monitor command can hang is impractical for the general case.  What
> > might be practical is a QMP monitor mode that accepts only known
> > hang-free commands.  Hang-free could be introspectable.
> > 
> > In case you consider that ugly: it's best to explore the design space
> > first, and recoil from "ugly" second.
> 
> Actually you slightly mis-interpreted me there. I think it is ok for
> applications to have knowledge about whether a particular command
> may hang or not. Given that knowledge it should *not*, however, require
> that the application issue such commands on separate monitor channels.
> It is entirely possible to handle hang-free commands on the existing
> channel.
> 
> > > I'd much prefer to see the problem described handled transparently inside
> > > QEMU. One approach is have a dedicated thread in QEMU responsible for all
> > > monitor I/O. This thread should never actually execute monitor commands
> > > though, it would simply parse the command request and put data onto a 
> > > queue
> > > of pending commands, thus it could never hang. The command queue could be
> > > processed by the main thread, or by another thread that is interested.
> > > eg the migration thread could process any queued commands related to
> > > migration directly.
> > 
> > The monitor itself can't hang then, but the thread(s) dequeuing parsed
> > commands can.
> 
> If certain commands are hang-free then you can have a dedicated thread
> that only de-queues & processes the hang-free commands. The approach I
> outlined is exactly how libvirt deals with its own RPC dispatch. We have
> certain commands that are guaranteed to not hang, which are processed by
> a dedicated pool of threads. So even if all normal RPC commands have
> hung, you can still run a subset of hang-free RPC commands.
> 
> > 
> > To maintain commands' synchronous semantics, their replies need to be
> > sent in order, which of course reintroduces the hangs.
> 
> The requirement for such ordering is just an arbitrary restriction that
> QEMU currently imposes. It is reasonable to allow arbitrary ordering of
> responses (which is what libvirt does in its RPC layer). Admittedly at
> this stage though, we would likely require some "opt in" handshake when
> initializing QMP for the app to tell QEMU it can cope with out of order
> replies. It would require that each command request has a unique serial
> number, which is included in the associated reply, so apps can match
> them up. We used to have that but iirc it was then removed.
> 
> There's other ways to deal with this, such as the job starting idea you
> mention below.
> 
> The key point though is that I don't think creating multiple monitor
> servers is a desirable approach - it is just a hack to avoid dealing
> with the root cause problems. 

Yeah I kindly agree.  It's not the root problem, but AFAIU that's the
simplest way for now to solve the problem.  But I think I understand
the major problem here - an extra channel is an interface change, and
it affects users of monitors.  So I agree we'd better be patient on
choosing a good enough interface, looks like we have two:

- dedicated "hang-able" and "hang-free" channel, or,

- async command handling, then we will have one single dedicated
  command parser (possibly as well in separate 

Re: [Qemu-devel] [PATCHv5 01/03] qemu-iothread: IOThread supports theGMainContext event loop

[wang.yong155]

>> From: Wang Yong 

>> 

>> IOThread uses AioContext event loop and does not run a GMainContext.

>> Therefore,chardev cannot work in IOThread,such as the chardev is

>> used for colo-compare packets reception.

>> 

>> This patch makes the IOThread run the GMainContext event loop,

>> chardev and IOThread can work together.

>> 

>> Signed-off-by: Wang Yong 

>> Signed-off-by: Wang Guang 

>> ---

>>  include/sysemu/iothread.h |  4 

>>  iothread.c| 45 +

>>  2 files changed, 49 insertions(+)

>> 

>> diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h

>> index e6da1a4..d2985b3 100644

>> --- a/include/sysemu/iothread.h

>> +++ b/include/sysemu/iothread.h

>> @@ -24,6 +24,9 @@ typedef struct {

>>  

>>  QemuThread thread

>>  AioContext *ctx

>> +GMainContext *worker_context

>> +GMainLoop *main_loop

>> +GOnce once

>>  QemuMutex init_done_lock

>>  QemuCond init_done_cond/* is thread initialization done? */

>>  bool stopping

>> @@ -41,5 +44,6 @@ typedef struct {

>>  char *iothread_get_id(IOThread *iothread)

>>  AioContext *iothread_get_aio_context(IOThread *iothread)

>>  void iothread_stop_all(void)

>> +GMainContext *iothread_get_g_main_context(IOThread *iothread)

>>  

>>  #endif /* IOTHREAD_H */

>> diff --git a/iothread.c b/iothread.c

>> index beeb870..44c8944 100644

>> --- a/iothread.c

>> +++ b/iothread.c

>> @@ -57,6 +57,23 @@ static void *iothread_run(void *opaque)

>>  

>>  while (!atomic_read(&iothread->stopping)) {

>>  aio_poll(iothread->ctx, true)

>> +

>> +if (atomic_read(&iothread->worker_context)) {

>> +GMainLoop *loop

>> +

>> +g_main_context_push_thread_default(iothread->worker_context)

>> +iothread->main_loop =

>> +g_main_loop_new(iothread->worker_context, TRUE)

>> +loop = iothread->main_loop

>> +

>> +g_main_loop_run(iothread->main_loop)

>> +iothread->main_loop = NULL

>> +g_main_loop_unref(loop)

>> +

>> +g_main_context_pop_thread_default(iothread->worker_context)

>> +g_main_context_unref(iothread->worker_context)

>> +iothread->worker_context = NULL

>> +}

>>  }

>>  

>>  rcu_unregister_thread()

>> @@ -73,6 +90,9 @@ static int iothread_stop(Object *object, void *opaque)

>>  }

>>  iothread->stopping = true

>>  aio_notify(iothread->ctx)

>> +if (atomic_read(&iothread->main_loop)) {

>> +g_main_loop_quit(iothread->main_loop)

>> +}

>>  qemu_thread_join(&iothread->thread)

>>  return 0

>>  }

>> @@ -125,6 +145,7 @@ static void iothread_complete(UserCreatable *obj, Error 
>> **errp)

>>  

>>  qemu_mutex_init(&iothread->init_done_lock)

>>  qemu_cond_init(&iothread->init_done_cond)

>> +iothread->once = (GOnce) G_ONCE_INIT

>

>In last review I suggested removing this type cast, otherwise looks good. Drop

>it and please add

>

>Reviewed-by: Fam Zheng 




Sorry, There's something wrong with our company's mail format.

Please ignore my last reply mail.




Hi Fam,

Here, iothread->once can't be initialized with G_ONCE_INIT directly, must use 
type cast.

if  remove type cast , we will get an error.




iothread.c: In function ‘iothread_complete’:

/usr/include/glib-2.0/glib/gthread.h:101:21: error: expected expression before 
‘{’ token

 #define G_ONCE_INIT { G_ONCE_STATUS_NOTCALLED, NULL }

 ^

iothread.c:149:22: note: in expansion of macro ‘G_ONCE_INIT’

 iothread->once = G_ONCE_INIT




Thanks

>

>>  

>>  /* This assumes we are called from a thread with useful CPU affinity 
>> for us

>>   * to inherit.

>> @@ -309,3 +330,27 @@ void iothread_stop_all(void)

>>  

>>  object_child_foreach(container, iothread_stop, NULL)

>>  }

>> +

>> +static gpointer iothread_g_main_context_init(gpointer opaque)

>> +{

>> +AioContext *ctx

>> +IOThread *iothread = opaque

>> +GSource *source

>> +

>> +iothread->worker_context = g_main_context_new()

>> +

>> +ctx = iothread_get_aio_context(iothread)

>> +source = aio_get_g_source(ctx)

>> +g_source_attach(source, iothread->worker_context)

>> +g_source_unref(source)

>> +

>> +aio_notify(iothread->ctx)

>> +return NULL

>> +}

>> +

>> +GMainContext *iothread_get_g_main_context(IOThread *iothread)

>> +{

>> +g_once(&iothread->once, iothread_g_main_context_init, iothread)

>> +

>> +return iothread->worker_context

>> +}

>> -- 

>> 1.8.3.1

>> 

>> 



原始邮件



发件人： 
收件人：王勇10170530
抄送人：
 王广10165992 
 
日 期 ：2017年08月31日 10:18
主 题 ：Re: [PATCHv5 01/03] qemu-iothread: IOThread supports theGMainContext event 
loop





On Tue, 08/29 15:22, Wang yong wrote:
> From: Wang Yong 
> 
> IOThread uses AioContext event loop and does not run a GMainContext.
> Therefore,chardev cannot work in IOThread,such as the ch

Re: [Qemu-devel] [PATCHv5 01/03] qemu-iothread: IOThread supports theGMainContext event loop

[wang.yong155]

>> From: Wang Yong >> >> IOThread uses AioContext 
>> event loop and does not run a GMainContext.>> Therefore,chardev cannot work 
>> in IOThread,such as the chardev is>> used for colo-compare packets 
>> reception.>> >> This patch makes the IOThread run the GMainContext event 
>> loop,>> chardev and IOThread can work together.>> >> Signed-off-by: Wang 
>> Yong >> Signed-off-by: Wang Guang 
>> >> --->>  include/sysemu/iothread.h |  4 >>  
>> iothread.c| 45 
>> +>>  2 files changed, 49 
>> insertions(+)>> >> diff --git a/include/sysemu/iothread.h 
>> b/include/sysemu/iothread.h>> index e6da1a4..d2985b3 100644>> --- 
>> a/include/sysemu/iothread.h>> +++ b/include/sysemu/iothread.h>> @@ -24,6 
>> +24,9 @@ typedef struct {>>  >>  QemuThread thread>>  AioContext 
>> *ctx>> +GMainContext *worker_context>> +GMainLoop *main_loop>> +
>> GOnce once>>  QemuMutex init_done_lock>>  QemuCond init_done_cond
>> /* is thread initialization done? */>>  bool stopping>> @@ -41,5 +44,6 
>> @@ typedef struct {>>  char *iothread_get_id(IOThread *iothread)>>  
>> AioContext *iothread_get_aio_context(IOThread *iothread)>>  void 
>> iothread_stop_all(void)>> +GMainContext 
>> *iothread_get_g_main_context(IOThread *iothread)>>  >>  #endif /* IOTHREAD_H 
>> */>> diff --git a/iothread.c b/iothread.c>> index beeb870..44c8944 100644>> 
>> --- a/iothread.c>> +++ b/iothread.c>> @@ -57,6 +57,23 @@ static void 
>> *iothread_run(void *opaque)>>  >>  while 
>> (!atomic_read(&iothread->stopping)) {>>  aio_poll(iothread->ctx, 
>> true)>> +>> +if (atomic_read(&iothread->worker_context)) {>> +   
>>  GMainLoop *loop>> +>> +
>> g_main_context_push_thread_default(iothread->worker_context)>> +
>> iothread->main_loop =>> +
>> g_main_loop_new(iothread->worker_context, TRUE)>> +loop = 
>> iothread->main_loop>> +>> +
>> g_main_loop_run(iothread->main_loop)>> +iothread->main_loop = 
>> NULL>> +g_main_loop_unref(loop)>> +>> +
>> g_main_context_pop_thread_default(iothread->worker_context)>> +
>> g_main_context_unref(iothread->worker_context)>> +
>> iothread->worker_context = NULL>> +}>>  }>>  >>  
>> rcu_unregister_thread()>> @@ -73,6 +90,9 @@ static int iothread_stop(Object 
>> *object, void *opaque)>>  }>>  iothread->stopping = true>>  
>> aio_notify(iothread->ctx)>> +if (atomic_read(&iothread->main_loop)) {>> 
>> +g_main_loop_quit(iothread->main_loop)>> +}>>  
>> qemu_thread_join(&iothread->thread)>>  return 0>>  }>> @@ -125,6 +145,7 
>> @@ static void iothread_complete(UserCreatable *obj, Error **errp)>>  >> 
>>  qemu_mutex_init(&iothread->init_done_lock)>>  
>> qemu_cond_init(&iothread->init_done_cond)>> +iothread->once = (GOnce) 
>> G_ONCE_INIT>>In last review I suggested removing this type cast, otherwise 
>> looks good. Drop>it and please add>>Reviewed-by: Fam Zheng >

Hi Fam,

Here, iothread->once can't be initialized with G_ONCE_INIT directly, must use 
type cast.

if  remove type cast , we will get an error.




iothread.c: In function ‘iothread_complete’:

/usr/include/glib-2.0/glib/gthread.h:101:21: error: expected expression before 
‘{’ token

 #define G_ONCE_INIT { G_ONCE_STATUS_NOTCALLED, NULL }

 ^

iothread.c:149:22: note: in expansion of macro ‘G_ONCE_INIT’

 iothread->once = G_ONCE_INIT




Thanks




>>  >>  /* This assumes we are called from a thread with useful CPU 
>> affinity for us>>   * to inherit.>> @@ -309,3 +330,27 @@ void 
>> iothread_stop_all(void)>>  >>  object_child_foreach(container, 
>> iothread_stop, NULL)>>  }>> +>> +static gpointer 
>> iothread_g_main_context_init(gpointer opaque)>> +{>> +AioContext *ctx>> 
>> +IOThread *iothread = opaque>> +GSource *source>> +>> +
>> iothread->worker_context = g_main_context_new()>> +>> +ctx = 
>> iothread_get_aio_context(iothread)>> +source = aio_get_g_source(ctx)>> + 
>>g_source_attach(source, iothread->worker_context)>> +
>> g_source_unref(source)>> +>> +aio_notify(iothread->ctx)>> +return 
>> NULL>> +}>> +>> +GMainContext *iothread_get_g_main_context(IOThread 
>> *iothread)>> +{>> +g_once(&iothread->once, iothread_g_main_context_init, 
>> iothread)>> +>> +return iothread->worker_context>> +}>> -- >> 1.8.3.1>> 
>> >> 



原始邮件



发件人： 
收件人：王勇10170530
抄送人：
 王广10165992 
 
日 期 ：2017年08月31日 10:18
主 题 ：Re: [PATCHv5 01/03] qemu-iothread: IOThread supports theGMainContext event 
loop





On Tue, 08/29 15:22, Wang yong wrote:
> From: Wang Yong 
> 
> IOThread uses AioContext event loop and does not run a GMainContext.
> Therefore,chardev cannot work in IOThread,such as the chardev is
> used for colo-compare packets reception.
> 
> This patch makes the IOThread run the GMainContext event loop,

Re: [Qemu-devel] [PATCHv5 01/03] qemu-iothread: IOThread supports the GMainContext event loop

[Fam Zheng]

On Tue, 08/29 15:22, Wang yong wrote:
> From: Wang Yong 
> 
> IOThread uses AioContext event loop and does not run a GMainContext.
> Therefore,chardev cannot work in IOThread,such as the chardev is
> used for colo-compare packets reception.
> 
> This patch makes the IOThread run the GMainContext event loop,
> chardev and IOThread can work together.
> 
> Signed-off-by: Wang Yong 
> Signed-off-by: Wang Guang 
> ---
>  include/sysemu/iothread.h |  4 
>  iothread.c| 45 +
>  2 files changed, 49 insertions(+)
> 
> diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
> index e6da1a4..d2985b3 100644
> --- a/include/sysemu/iothread.h
> +++ b/include/sysemu/iothread.h
> @@ -24,6 +24,9 @@ typedef struct {
>  
>  QemuThread thread;
>  AioContext *ctx;
> +GMainContext *worker_context;
> +GMainLoop *main_loop;
> +GOnce once;
>  QemuMutex init_done_lock;
>  QemuCond init_done_cond;/* is thread initialization done? */
>  bool stopping;
> @@ -41,5 +44,6 @@ typedef struct {
>  char *iothread_get_id(IOThread *iothread);
>  AioContext *iothread_get_aio_context(IOThread *iothread);
>  void iothread_stop_all(void);
> +GMainContext *iothread_get_g_main_context(IOThread *iothread);
>  
>  #endif /* IOTHREAD_H */
> diff --git a/iothread.c b/iothread.c
> index beeb870..44c8944 100644
> --- a/iothread.c
> +++ b/iothread.c
> @@ -57,6 +57,23 @@ static void *iothread_run(void *opaque)
>  
>  while (!atomic_read(&iothread->stopping)) {
>  aio_poll(iothread->ctx, true);
> +
> +if (atomic_read(&iothread->worker_context)) {
> +GMainLoop *loop;
> +
> +g_main_context_push_thread_default(iothread->worker_context);
> +iothread->main_loop =
> +g_main_loop_new(iothread->worker_context, TRUE);
> +loop = iothread->main_loop;
> +
> +g_main_loop_run(iothread->main_loop);
> +iothread->main_loop = NULL;
> +g_main_loop_unref(loop);
> +
> +g_main_context_pop_thread_default(iothread->worker_context);
> +g_main_context_unref(iothread->worker_context);
> +iothread->worker_context = NULL;
> +}
>  }
>  
>  rcu_unregister_thread();
> @@ -73,6 +90,9 @@ static int iothread_stop(Object *object, void *opaque)
>  }
>  iothread->stopping = true;
>  aio_notify(iothread->ctx);
> +if (atomic_read(&iothread->main_loop)) {
> +g_main_loop_quit(iothread->main_loop);
> +}
>  qemu_thread_join(&iothread->thread);
>  return 0;
>  }
> @@ -125,6 +145,7 @@ static void iothread_complete(UserCreatable *obj, Error 
> **errp)
>  
>  qemu_mutex_init(&iothread->init_done_lock);
>  qemu_cond_init(&iothread->init_done_cond);
> +iothread->once = (GOnce) G_ONCE_INIT;

In last review I suggested removing this type cast, otherwise looks good. Drop
it and please add

Reviewed-by: Fam Zheng 

>  
>  /* This assumes we are called from a thread with useful CPU affinity for 
> us
>   * to inherit.
> @@ -309,3 +330,27 @@ void iothread_stop_all(void)
>  
>  object_child_foreach(container, iothread_stop, NULL);
>  }
> +
> +static gpointer iothread_g_main_context_init(gpointer opaque)
> +{
> +AioContext *ctx;
> +IOThread *iothread = opaque;
> +GSource *source;
> +
> +iothread->worker_context = g_main_context_new();
> +
> +ctx = iothread_get_aio_context(iothread);
> +source = aio_get_g_source(ctx);
> +g_source_attach(source, iothread->worker_context);
> +g_source_unref(source);
> +
> +aio_notify(iothread->ctx);
> +return NULL;
> +}
> +
> +GMainContext *iothread_get_g_main_context(IOThread *iothread)
> +{
> +g_once(&iothread->once, iothread_g_main_context_init, iothread);
> +
> +return iothread->worker_context;
> +}
> -- 
> 1.8.3.1
> 
> 

Re: [Qemu-devel] [PATCH 3/3] Backup Tool: Test for Incremental Backup

[Fam Zheng]

On Thu, 08/31 00:45, Ishani Chugh wrote:
> This patch is the test for incremental backup implementation in Backup tool.
> The test employs two basic subtests:
> 1) Backing up an empty guest and comparing it with base image.
> 2) Writing a pattern to the guest, creating backup, writing
>a pattern again, creating backup and comparing with base image.
> 
> Signed-off-by: Ishani Chugh 
> ---
>  tests/qemu-iotests/193 | 86 
> ++
>  tests/qemu-iotests/193.out | 34 ++
>  tests/qemu-iotests/group   |  1 +
>  3 files changed, 121 insertions(+)
>  create mode 100755 tests/qemu-iotests/193
>  create mode 100644 tests/qemu-iotests/193.out
> 
> diff --git a/tests/qemu-iotests/193 b/tests/qemu-iotests/193
> new file mode 100755
> index 000..500e5df
> --- /dev/null
> +++ b/tests/qemu-iotests/193
> @@ -0,0 +1,86 @@
> +#!/bin/bash
> +#
> +# Test Incremental backup functionality of qemu-backup tool
> +#
> +# Copyright (C) 2009 Red Hat, Inc.

Year is off, probably the copyright holder too?

> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see .
> +#
> +
> +# creator
> +owner=chugh.ish...@research.iiit.ac.in
> +
> +seq=`basename $0`
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +status=1 # failure is the default!

s/\t/ /

> +
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +. ./common.qemu
> +
> +_supported_fmt generic
> +_supported_proto generic
> +_supported_os Linux
> +
> +
> +CONFIG_FILE=$TEST_DIR/backup-config
> +SOCKET=unix:$TEST_DIR/backup_socket
> +size=128M
> +
> +_make_test_img $size
> +export QEMU_BACKUP_CONFIG=$CONFIG_FILE
> +qemu_comm_method="monitor"
> +echo
> +_launch_qemu -drive if=virtio,file=$TEST_IMG -qmp $SOCKET,server,nowait
> +$PYTHON ../../contrib/backup/qemu-backup.py guest add --guest adad --qmp 
> $SOCKET
> +$PYTHON ../../contrib/backup/qemu-backup.py drive add --id virtio0 --guest 
> adad --target $TEST_DIR/virtio0
> +echo
> +echo "== Creating backup =="
> +$PYTHON ../../contrib/backup/qemu-backup.py backup --inc --guest adad
> +_send_qemu_cmd $QEMU_HANDLE 'quit' ''
> +wait=1 _cleanup_qemu
> +echo
> +echo "== Comparing images =="
> +$QEMU_IMG compare $TEST_DIR/virtio0_inc_0 $TEST_IMG
> +
> +rm $TEST_DIR/virtio0_inc_0
> +
> +_launch_qemu -drive if=virtio,id=virtio0,file=$TEST_IMG -qmp 
> $SOCKET,server,nowait
> +echo
> +echo "== Writing Pattern =="
> +_send_qemu_cmd $QEMU_HANDLE 'qemu-io virtio0 "write -P 0x22 0 1M"' "(qemu)" 
> | _filter_qemu_io
> +echo
> +
> +echo "== Creating backup =="
> +$PYTHON ../../contrib/backup/qemu-backup.py backup --inc --guest adad
> +
> +echo "== Writing Pattern =="
> +_send_qemu_cmd $QEMU_HANDLE 'qemu-io virtio0 "write -P 0x22 0 1M"' "(qemu)" 
> | _filter_qemu_io
> +echo
> +$PYTHON ../../contrib/backup/qemu-backup.py backup --inc --guest adad
> +_send_qemu_cmd $QEMU_HANDLE 'quit' ''
> +wait=1 _cleanup_qemu
> +echo
> +echo "== Comparing images =="
> +$QEMU_IMG compare $TEST_DIR/virtio0_inc_1 $TEST_IMG
> +rm $TEST_DIR/virtio0_inc_0
> +rm $TEST_DIR/virtio0_inc_1
> +rm $CONFIG_FILE
> +
> +echo "*** done"
> +status=0
> \ No newline at end of file
> diff --git a/tests/qemu-iotests/193.out b/tests/qemu-iotests/193.out
> new file mode 100644
> index 000..3a836c2
> --- /dev/null
> +++ b/tests/qemu-iotests/193.out
> @@ -0,0 +1,34 @@
> +QA output created by 193
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
> +
> +Successfully Added Guest
> +Successfully Added Drive
> +
> +== Creating backup ==
> +QEMU X.Y.Z monitor - type 'help' for more information
> +(qemu) Formatting 'TEST_DIR/virtio0_inc_0', fmt=qcow2 size=134217728 
> cluster_size=65536 lazy_refcounts=off refcount_bits=16
> +quit
> +
> +== Comparing images ==
> +Images are identical.
> +
> +== Writing Pattern ==
> +QEMU X.Y.Z monitor - type 'help' for more information
> +(qemu) qemu-io virtio0 "write -P 0x22 0 1M"
> +
> +== Creating backup ==
> +Initial Backup does not exist
> +== Writing Pattern ==
> +wrote 1048576/1048576 bytes at offset 0
> +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +(qemu) Formatting 'TEST_DIR/virtio0_inc_0', fmt=qcow2 size=134217728 
> cluster_size=65536 lazy_refcounts=off refcount_bits=16
> +
> +Formatting 
> '/home/ishani/Desktop/opw/qemu/tests/qemu-iotests/scratch/virtio0_inc_1', 
> fmt=qcow2 size=134217728 
> backing_file=/home

Re: [Qemu-devel] [PATCH 1/3] Backup Tool: Manpage for Incremental Backup

[Fam Zheng]

On Thu, 08/31 00:45, Ishani Chugh wrote:
> Adds command description to perform incremental backup and
> a full example for the same.
> 
> Signed-off-by: Ishani Chugh 
> ---
>  contrib/backup/qemu-backup.texi | 18 ++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/contrib/backup/qemu-backup.texi b/contrib/backup/qemu-backup.texi
> index 7ad266c..8e5cb86 100644
> --- a/contrib/backup/qemu-backup.texi
> +++ b/contrib/backup/qemu-backup.texi
> @@ -58,6 +58,7 @@ qemu-backup command [command options].
>  @item qemu-backup restore --guest guestname
>  @item qemu-backup guest remove --guest guestname
>  @item qemu-backup drive remove --guest guestname --id driveid
> +@item qemu-backup backup --inc --guest guestname
>  @end itemize
>  @node  Command Parameters
>  @chapter  Command Parameters
> @@ -67,6 +68,7 @@ qemu-backup command [command options].
>  @item --id: id of guest or drive.
>  @item --qmp: Path of qmp socket.
>  @item --target: Destination path on which you want your backup to be made.
> +@item --inc: incremental backup (Optional).
>  @end itemize
>  
>  @node  Command Descriptions
> @@ -119,6 +121,11 @@ This command helps remove a drive which is set for 
> backup in configuration of gi
>  
>  example: drive remove --guest=fedora --id=root
>  
> +@item qemu-backup backup --inc --guest guestname
> +This command is used for making incremental backup.
> +
> +example: qemu-backup backup --inc --guest fedora

Could you document the output file name generation?

Fam

> +
>  @item A full backup can be performed by following the given steps:
>  
>  Perform a full backup of 'vm001', which has one drive:
> @@ -129,6 +136,17 @@ qemu-backup add --id drive0 --guest vm001 --target 
> /backups/vm001-drive0.img
>  
>  qemu-backup backup --guest vm001
>  
> +@item An incremental backup can be performed by following the given steps:
> +
> +Perform an incremental backup of 'vm001', which has one drive:
> +
> +qemu-backup guest add --guest vm001 --qmp /path/to/vm001.sock
> +
> +qemu-backup add --id drive0 --guest vm001 --target /backups/vm001-drive0.img
> +
> +qemu-backup backup --inc --guest vm001
> +
> +qemu-backup backup --inc --guest vm001
>  
>  @end itemize
>  
> -- 
> 2.7.4
> 
> 

Re: [Qemu-devel] [PATCH 2/3] Backup Tool: Support for Incremental Backup

[Fam Zheng]

On Thu, 08/31 00:45, Ishani Chugh wrote:
> Adds incremental backup functionality.
> 
> Signed-off-by: Ishani Chugh 
> ---
>  contrib/backup/qemu-backup.py | 101 
> +-
>  1 file changed, 99 insertions(+), 2 deletions(-)
> 
> diff --git a/contrib/backup/qemu-backup.py b/contrib/backup/qemu-backup.py
> index 248ca9f..7a3077a 100644
> --- a/contrib/backup/qemu-backup.py
> +++ b/contrib/backup/qemu-backup.py
> @@ -24,11 +24,13 @@ from __future__ import print_function
>  from argparse import ArgumentParser
>  import os
>  import errno
> +from string import Template
>  from socket import error as socket_error
>  try:
>  import configparser
>  except ImportError:
>  import ConfigParser as configparser
> +from configparser import NoOptionError
>  import sys
>  sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..',
>   'scripts', 'qmp'))
> @@ -41,7 +43,6 @@ class BackupTool(object):
>   '/.config/qemu/qemu-backup-config'):
>  if "QEMU_BACKUP_CONFIG" in os.environ:
>  self.config_file = os.environ["QEMU_BACKUP_CONFIG"]
> -
>  else:
>  self.config_file = config_file
>  try:
> @@ -129,6 +130,97 @@ class BackupTool(object):
>  drive_list.remove(event['data']['device'])
>  print("Backup Complete")
>  
> +def _inc_backup(self, guest_name):
> +"""
> +Performs Incremental backup
> +"""
> +if guest_name not in self.config.sections():
> +print ("Cannot find specified guest", file=sys.stderr)

s/print (/print(/

> +exit(1)
> +
> +self.verify_guest_running(guest_name)
> +connection = QEMUMonitorProtocol(
> + self.get_socket_address(
> + self.config[guest_name]['qmp']))
> +connection.connect()
> +backup_cmd = {"execute": "transaction",
> +  "arguments": {"actions": [], "properties":
> +{"completion-mode": "grouped"}}}
> +bitmap_cmd = {"execute": "transaction", "arguments": {"actions": []}}
> +for key in self.config[guest_name]:

>From here on the indentation level is launched straight into outer space. 
>Please
either extract code blocks into functions, or at least try to rearrange it like:

> +if key.startswith("drive_"):

   if not key.startswith("drive_"):
   continue

   drive = ...
   ...
> +drive = key[len('drive_'):]
> +target = self.config.get(guest_name, key).rsplit('/', 1)[0]
> +inc_backup_pattern = Template('${drive}_inc_${N}')
> +bitmap = 'qemu_backup_'+guest_name

Whitespaces missing around +.

> +try:
> +query_block_cmd = {'execute': 'query-block'}
> +returned_json = connection.cmd_obj(query_block_cmd)
> +device_present = False
> +for device in returned_json['return']:
> +if device['device'] == drive:

   if device['device'] != drive:
   continue
   device_present = True
   ...
> +device_present = True
> +bitmap_present = False
> +for bitmaps in device['dirty-bitmaps']:
> +if bitmap == bitmaps['name']:

   if bitmap != bitmaps['name']:
   continue
   bitmap_present = True
   ...
> +bitmap_present = True
> +if os.path.isfile(self.config.get(
> +  guest_name,
> +  'inc_'+drive)) is 
> False:
> +print("Initial Backup does not 
> exist")
> +bitmap_remove = {"execute":
> + "block-dirty" +
> + "-bitmap-remove",

Please fix the indentation level and join the string: 
"block-dirty-bitmap-remove".

Even if you cannot control the indentation level, long line is still better than
cutting it into halves. It makes the code hard to read and unfriendly to grep.

> + "arguments":
> + {"node": drive,
> +  "name":
> +  "qemu_backup_" +
> +

Re: [Qemu-devel] [PATCH v7 07/11] qemu.py: include debug information on launch error

[Fam Zheng]

On Wed, 08/30 11:55, Cleber Rosa wrote:
> 
> 
> On 08/18/2017 01:05 PM, Amador Pahim wrote:
> > When launching a VM, if an exception happens and the VM is not
> > initiated, it might be useful to see the qemu command line and
> > the qemu command output.
> > 
> > This patch creates that message. Notice that self._iolog needs to be
> > cleaned up in the beginning of the launch() to make sure we will not
> > expose the qemu log from a previous launch if the current one fails.
> > 
> > Signed-off-by: Amador Pahim 
> > ---
> >  scripts/qemu.py | 8 
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/scripts/qemu.py b/scripts/qemu.py
> > index 0bcec4b3b1..29fd2469f9 100644
> > --- a/scripts/qemu.py
> > +++ b/scripts/qemu.py
> > @@ -147,6 +147,7 @@ class QEMUMachine(object):
> >  
> >  def launch(self):
> >  '''Launch the VM and establish a QMP connection'''
> > +self._iolog = None
> >  self._qemu_full_args = None
> >  devnull = open(os.path.devnull, 'rb')
> >  qemulog = open(self._qemu_log_path, 'wb')
> > @@ -162,6 +163,13 @@ class QEMUMachine(object):
> >  self._post_launch()
> >  except:
> >  self.shutdown()
> > +
> > +LOG.debug('Error launching VM')
> > +if self._qemu_full_args:
> > +LOG.debug('Command: %r', ' '.join(self._qemu_full_args))
> > +if self._iolog:
> > +LOG.debug('Output: %r', self._iolog)
> > +
> 
> Based on Fam's comment about the signal message being a warning (worth
> showing by default), I also think this deserves more than a "debug"
> classification.

Yes, LOG.error, please.

Fam

Re: [Qemu-devel] [PATCH] block: Cleanup BMDS in bdrv_close_all

[Fam Zheng]

On Wed, 08/30 16:04, Juan Quintela wrote:
> Fam Zheng  wrote:
> > On Wed, 08/30 13:49, Juan Quintela wrote:
> >> Fam Zheng  wrote:
> >> > This fixes the assertion due to op blockers added by BMDS:
> >> >
> >> > block.c:3248: bdrv_delete: Assertion `bdrv_op_blocker_is_empty(bs)' 
> >> > failed.
> >> >
> >> > Reproducer: simply start block migration and quit QEMU before it ends.
> >> >
> >> > Cc: qemu-sta...@nongnu.org
> >> > Signed-off-by: Fam Zheng 
> >> 
> >> No need for one stub, see later.
> >> 
> >> 
> >> > ---
> >> >  block.c | 2 ++
> >> >  migration/block.c   | 2 +-
> >> >  migration/block.h   | 1 +
> >> >  stubs/Makefile.objs | 1 +
> >> >  stubs/block-migration.c | 6 ++
> >> >  5 files changed, 11 insertions(+), 1 deletion(-)
> >> >  create mode 100644 stubs/block-migration.c
> >> >
> >> > diff --git a/block.c b/block.c
> >> > index 3308814bba..508a57274d 100644
> >> > --- a/block.c
> >> > +++ b/block.c
> >> > @@ -43,6 +43,7 @@
> >> >  #include "qemu/cutils.h"
> >> >  #include "qemu/id.h"
> >> >  #include "qapi/util.h"
> >> > +#include "migration/block.h"
> >> 
> >> this should be misc.h
> >> 
> >> >  
> >> >  #ifdef CONFIG_BSD
> >> >  #include 
> >> > @@ -3111,6 +3112,7 @@ static void bdrv_close(BlockDriverState *bs)
> >> >  
> >> >  void bdrv_close_all(void)
> >> >  {
> >> > +block_migration_cleanup_bmds();
> >> >  block_job_cancel_sync_all();
> >> >  nbd_export_close_all();
> >> >  
> >> 
> >> > diff --git a/migration/block.h b/migration/block.h
> >> > index 22ebe94259..8bae1cf55a 100644
> >> > --- a/migration/block.h
> >> > +++ b/migration/block.h
> >> > @@ -42,4 +42,5 @@ static inline uint64_t blk_mig_bytes_total(void)
> >> >  #endif /* CONFIG_LIVE_BLOCK_MIGRATION */
> >> >  
> >> >  void migrate_set_block_enabled(bool value, Error **errp);
> >> > +void block_migration_cleanup_bmds(void);
> >> >  #endif /* MIGRATION_BLOCK_H */
> >> > diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> >> > index e69c217aff..7540913767 100644
> >> > --- a/stubs/Makefile.objs
> >> > +++ b/stubs/Makefile.objs
> >> > @@ -19,6 +19,7 @@ stub-obj-y += is-daemonized.o
> >> >  stub-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
> >> >  stub-obj-y += machine-init-done.o
> >> >  stub-obj-y += migr-blocker.o
> >> > +stub-obj-y += block-migration.o
> >> >  stub-obj-y += change-state-handler.o
> >> >  stub-obj-y += monitor.o
> >> >  stub-obj-y += notify-event.o
> >> > diff --git a/stubs/block-migration.c b/stubs/block-migration.c
> >> > new file mode 100644
> >> > index 00..855f15c757
> >> > --- /dev/null
> >> > +++ b/stubs/block-migration.c
> >> > @@ -0,0 +1,6 @@
> >> > +#include "qemu/osdep.h"
> >> > +#include "migration/block.h"
> >> > +
> >> > +void block_migration_cleanup_bmds(void)
> >> > +{
> >> > +}
> >> 
> >> You can add this inside include/migration/misc.h
> >> 
> >> #ifdef CONFIG_LIVE_BLOCK_MIGRATION
> >> void blk_mig_init(void);
> >> #else
> >> static inline void blk_mig_init(void) {}
> >> 
> >> // And then you add the stub here?
> >
> > This doesn't work.  The function is not stubbed for 
> > !CONFIG_LIVE_BLOCK_MIGRATION
> > configs, but for tools that don't link to common-obj-y. For example with 
> > your
> > proposed change, I get:
> >
> >   LINKqemu-nbd
> > block.o: In function `bdrv_close_all':
> > /home/fam/work/qemu/block.c:3115: undefined reference to
> > `block_migration_cleanup_bmds'
> > collect2: error: ld returned 1 exit status
> > make: *** [/home/fam/work/qemu/rules.mak:121: qemu-nbd] Error 1
> > make: Leaving directory '/home/fam/work/q/build'
> 
> 
> This works for me, for both CONFIG_LIVE_BLOCK_MIGRATION enabled and not.
> For qemu-system-x86_64 and qemu-nbd.  Could you test?

I get the same error:

  LINKqemu-nbd
block.o: In function `bdrv_close_all':
/home/fam/work/qemu/block.c:3115: undefined reference to 
`block_migration_cleanup_bmds'
collect2: error: ld returned 1 exit status
make: *** [/home/fam/work/qemu/rules.mak:121: qemu-nbd] Error 1
make: Leaving directory '/home/fam/work/q/build'

(also applies to qemu-img etc.)

Fam

---

$ cat config.status 
#!/bin/sh
# Generated by configure.
# Run this file to recreate the current configuration.
# Compiler output produced by configure, useful for debugging
# configure, is in config.log if it exists.
exec '/home/fam/work/qemu/configure' 
'--prefix=/home/fam/work/q/install/bdrv_close_all-bmds' '--enable-debug' 
'--extra-cflags=-Wno-error=format-truncation' '--target-list=x86_64-softmmu' 
"$@"

$ grep CONFIG_LIVE_BLOCK_MIGRATION config-host.h
#define CONFIG_LIVE_BLOCK_MIGRATION 1

$ make qemu-nbd V=1
(cd /home/fam/work/qemu; printf '#define QEMU_PKGVERSION '; if test -n ""; then 
printf '""\n'; else if test -d .git; then printf '" ('; git describe --match 
'v*' 2>/dev/null | tr -d '\n'; if ! git diff-index --quiet HEAD &>/dev/null; 
then printf -- '-dirty'; fi; printf ')"\n'; else printf '""\n'; fi; fi) > 
qemu-version.h.tmp
if ! cmp -s qemu-version.h qemu-version.h.tmp; then mv qemu-version.h

Re: [Qemu-devel] [PATCH v3 0/3] QEMU Backup Tool

[Fam Zheng]

On Wed, 08/30 22:44, Ishani Chugh wrote:
> This patch series is intended to introduce QEMU Backup tool.
> qemu-backup will be a command-line tool for performing full and
> incremental disk backups on running VMs. It is intended as a
> reference implementation for management stack and backup developers
> to see QEMU's backup features in action.
> This patch series contains three patches,
>1) QEMU Backup command line tool.
>2) Test for full backup.
>3) Manpage for the tool.
> v3:
> * Added versioning in config file
> * Replace location by qemu-img convert in restore
> * Removed incremental backup documentation from manpage

Reviewed-by: Fam Zheng 

Re: [Qemu-devel] [PATCH] qcow2: allocate cluster_cache/cluster_data on demand

[Alexey Kardashevskiy]

On 31/08/17 03:20, Stefan Hajnoczi wrote:
> On Tue, Aug 22, 2017 at 02:56:00PM +1000, Alexey Kardashevskiy wrote:
>> On 19/08/17 12:46, Alexey Kardashevskiy wrote:
>>> On 19/08/17 01:18, Eric Blake wrote:
 On 08/18/2017 08:31 AM, Stefan Hajnoczi wrote:
> Most qcow2 files are uncompressed so it is wasteful to allocate (32 + 1)
> * cluster_size + 512 bytes upfront.  Allocate s->cluster_cache and
> s->cluster_data when the first read operation is performance on a
> compressed cluster.
>
> The buffers are freed in .bdrv_close().  .bdrv_open() no longer has any
> code paths that can allocate these buffers, so remove the free functions
> in the error code path.
>
> Reported-by: Alexey Kardashevskiy 
> Cc: Kevin Wolf 
> Signed-off-by: Stefan Hajnoczi 
> ---
> Alexey: Does this improve your memory profiling results?

 Is this a regression from earlier versions? 
>>>
>>> Hm, I have not thought about this.
>>>
>>> So. I did bisect and this started happening from
>>> 9a4c0e220d8a4f82b5665d0ee95ef94d8e1509d5
>>> "hw/virtio-pci: fix virtio behaviour"
>>>
>>> Before that, the very same command line would take less than 1GB of
>>> resident memory. That thing basically enforces virtio-1.0 for QEMU <=2.6
>>> which means that upstream with "-machine pseries-2.6" works fine (less than
>>> 1GB), "-machine pseries-2.7" does not (close to 7GB, sometime even 9GB).
>>>
>>> Then I tried bisecting again, with
>>> "scsi=off,disable-modern=off,disable-legacy=on" on my 150 virtio-block
>>> devices, started from
>>> e266d421490e0 "virtio-pci: add flags to enable/disable legacy/modern" (it
>>> added the disable-modern switch) which uses 2GB of memory.
>>>
>>> I ended up with ada434cd0b44 "virtio-pci: implement cfg capability".
>>>
>>> Then I removed proxy->modern_as on v2.10.0-rc3 (see below) and got 1.5GB of
>>> used memory (yay!)
>>>
>>> I do not really know how to reinterpret all of this, do you?
>>
>>
>> Anyone, ping? Should I move the conversation to the original thread? Any
>> hacks to try with libc?
> 
> I suggest a new top-level thread with Michael Tsirkin CCed.


I am continuing in the original "Memory use with >100 virtio devices"
thread and the problem is more generic than virtio, it is just easier to
reproduce it with virtio, that's all.



-- 
Alexey

Re: [Qemu-devel] [PATCH v2 7/9] AHCI: Rework IRQ constants

[John Snow]

On 08/29/2017 11:54 PM, Philippe Mathieu-Daudé wrote:
> On 08/29/2017 05:49 PM, John Snow wrote:
>> Create a new enum so that we can name the IRQ bits, which will make
>> debugging
>> them a little nicer if we can print them out. Not handled in this
>> patch, but
>> this will make it possible to get a nice debug printf detailing
>> exactly which
>> status bits are set, as it can be multiple at any given time.
>>
>> As a consequence of this patch, it is no longer possible to set
>> multiple IRQ
>> codes at once, but nothing was utilizing this ability anyway.
>>
>> Signed-off-by: John Snow 
>> ---
>>   hw/ide/ahci.c  | 49
>> ++---
>>   hw/ide/ahci_internal.h | 44
>> +++-
>>   hw/ide/trace-events|  2 +-
>>   3 files changed, 74 insertions(+), 21 deletions(-)
>>
>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>> index c60a000..a0a4dd6 100644
>> --- a/hw/ide/ahci.c
>> +++ b/hw/ide/ahci.c
>> @@ -56,6 +56,27 @@ static bool ahci_map_fis_address(AHCIDevice *ad);
>>   static void ahci_unmap_clb_address(AHCIDevice *ad);
>>   static void ahci_unmap_fis_address(AHCIDevice *ad);
>>   +static const char *AHCIPortIRQ_lookup[AHCI_PORT_IRQ__END] = {
>> +[AHCI_PORT_IRQ_BIT_DHRS] = "DHRS",
>> +[AHCI_PORT_IRQ_BIT_PSS]  = "PSS",
>> +[AHCI_PORT_IRQ_BIT_DSS]  = "DSS",
>> +[AHCI_PORT_IRQ_BIT_SDBS] = "SDBS",
>> +[AHCI_PORT_IRQ_BIT_UFS]  = "UFS",
>> +[AHCI_PORT_IRQ_BIT_DPS]  = "DPS",
>> +[AHCI_PORT_IRQ_BIT_PCS]  = "PCS",
>> +[AHCI_PORT_IRQ_BIT_DMPS] = "DMPS",
>> +[8 ... 21]   = "RESERVED",
>> +[AHCI_PORT_IRQ_BIT_PRCS] = "PRCS",
>> +[AHCI_PORT_IRQ_BIT_IPMS] = "IPMS",
>> +[AHCI_PORT_IRQ_BIT_OFS]  = "OFS",
>> +[25] = "RESERVED",
>> +[AHCI_PORT_IRQ_BIT_INFS] = "INFS",
>> +[AHCI_PORT_IRQ_BIT_IFS]  = "IFS",
>> +[AHCI_PORT_IRQ_BIT_HBDS] = "HBDS",
>> +[AHCI_PORT_IRQ_BIT_HBFS] = "HBFS",
>> +[AHCI_PORT_IRQ_BIT_TFES] = "TFES",
>> +[AHCI_PORT_IRQ_BIT_CPDS] = "CPDS"
>> +};
>> static uint32_t  ahci_port_read(AHCIState *s, int port, int offset)
>>   {
>> @@ -170,12 +191,18 @@ static void ahci_check_irq(AHCIState *s)
>>   }
>> static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d,
>> - int irq_type)
>> + enum AHCIPortIRQ irqbit)
>>   {
>> -DPRINTF(d->port_no, "trigger irq %#x -> %x\n",
>> -irq_type, d->port_regs.irq_mask & irq_type);
>> +g_assert(irqbit >= 0 && irqbit < 32);
> 
> I still think this assert is superfluous, anyway (and having hard time
> reading C99 statement before declarations - I need to grow):
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> 

Left in because of my distrust of compilers as explained in my reply to
#05. We'll get to the bottom of it ;)

Thank you for the reviews.

--js

Re: [Qemu-devel] [PATCH v2 5/9] IDE: replace DEBUG_AIO with trace events

[John Snow]

CCing Laszlo Ersek literally just for laughs, as he is the most
entertaining language lawyer I know of.

Laszlo, please feel free to ignore this if you don't care :P

On 08/29/2017 11:14 PM, Philippe Mathieu-Daudé wrote:
> Hi John,
> 
> On 08/29/2017 05:49 PM, John Snow wrote:
>> Signed-off-by: John Snow 
>> ---
>>   hw/ide/atapi.c|  5 +
>>   hw/ide/core.c | 17 ++---
>>   hw/ide/trace-events   |  3 +++
>>   include/hw/ide/internal.h |  6 --
>>   4 files changed, 18 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
>> index 37fa699..b8fc51e 100644
>> --- a/hw/ide/atapi.c
>> +++ b/hw/ide/atapi.c
>> @@ -416,10 +416,7 @@ static void ide_atapi_cmd_read_dma_cb(void
>> *opaque, int ret)
>>   s->io_buffer_size = n * 2048;
>>   data_offset = 0;
>>   }
>> -#ifdef DEBUG_AIO
>> -printf("aio_read_cd: lba=%u n=%d\n", s->lba, n);
>> -#endif
>> -
>> +trace_ide_atapi_cmd_read_dma_cb_aio(s, s->lba, n);
>>   s->bus->dma->iov.iov_base = (void *)(s->io_buffer + data_offset);
>>   s->bus->dma->iov.iov_len = n * ATAPI_SECTOR_SIZE;
>>   qemu_iovec_init_external(&s->bus->dma->qiov, &s->bus->dma->iov, 1);
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index 82a19b1..a1c90e9 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -58,6 +58,13 @@ static const int smart_attributes[][12] = {
>>   { 190,  0x03, 0x00, 0x45, 0x45, 0x1f, 0x00, 0x1f, 0x1f, 0x00,
>> 0x00, 0x32},
>>   };
>>   +const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT] = {
>> +[IDE_DMA_READ] = "DMA READ",
>> +[IDE_DMA_WRITE] = "DMA WRITE",
>> +[IDE_DMA_TRIM] = "DMA TRIM",
>> +[IDE_DMA_ATAPI] = "DMA ATAPI"
>> +};
>> +
>>   static void ide_dummy_transfer_stop(IDEState *s);
>> static void padstr(char *str, const char *src, int len)
>> @@ -860,10 +867,8 @@ static void ide_dma_cb(void *opaque, int ret)
>>   goto eot;
>>   }
>>   -#ifdef DEBUG_AIO
>> -printf("ide_dma_cb: sector_num=%" PRId64 " n=%d, cmd_cmd=%d\n",
>> -   sector_num, n, s->dma_cmd);
>> -#endif
>> +trace_ide_dma_cb(s, sector_num, n,
>> + IDE_DMA_CMD_lookup[s->dma_cmd]);
>> if ((s->dma_cmd == IDE_DMA_READ || s->dma_cmd ==
>> IDE_DMA_WRITE) &&
>>   !ide_sect_range_ok(s, sector_num, n)) {
>> @@ -2391,9 +2396,7 @@ void ide_bus_reset(IDEBus *bus)
>> /* pending async DMA */
>>   if (bus->dma->aiocb) {
>> -#ifdef DEBUG_AIO
>> -printf("aio_cancel\n");
>> -#endif
>> +trace_ide_bus_reset_aio();
>>   blk_aio_cancel(bus->dma->aiocb);
>>   bus->dma->aiocb = NULL;
>>   }
>> diff --git a/hw/ide/trace-events b/hw/ide/trace-events
>> index 8c79a6c..cc8949c 100644
>> --- a/hw/ide/trace-events
>> +++ b/hw/ide/trace-events
>> @@ -18,6 +18,8 @@ ide_cancel_dma_sync_remaining(void) "draining all
>> remaining requests"
>>   ide_sector_read(int64_t sector_num, int nsectors) "sector=%"PRId64"
>> nsectors=%d"
>>   ide_sector_write(int64_t sector_num, int nsectors) "sector=%"PRId64"
>> nsectors=%d"
>>   ide_reset(void *s) "IDEstate %p"
>> +ide_bus_reset_aio(void) "aio_cancel"
>> +ide_dma_cb(void *s, int64_t sector_num, int n, const char *dma)
>> "IDEState %p; sector_num=%"PRId64" n=%d cmd=%s"
>> # BMDMA HBAs:
>>   @@ -51,5 +53,6 @@ ide_atapi_cmd_reply_end_new(void *s, int status)
>> "IDEState: %p; new transfer sta
>>   ide_atapi_cmd_check_status(void *s) "IDEState: %p"
>>   ide_atapi_cmd_read(void *s, const char *method, int lba, int
>> nb_sectors) "IDEState: %p; read %s: LBA=%d nb_sectors=%d"
>>   ide_atapi_cmd(void *s, uint8_t cmd) "IDEState: %p; cmd: 0x%02x"
>> +ide_atapi_cmd_read_dma_cb_aio(void *s, int lba, int n) "IDEState: %p;
>> aio read: lba=%d n=%d"
>>   # Warning: Verbose
>>   ide_atapi_cmd_packet(void *s, uint16_t limit, const char *packet)
>> "IDEState: %p; limit=0x%x packet: %s"
>> diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
>> index 74efe8a..db9fde0 100644
>> --- a/include/hw/ide/internal.h
>> +++ b/include/hw/ide/internal.h
>> @@ -14,7 +14,6 @@
>>   #include "block/scsi.h"
>> /* debug IDE devices */
>> -//#define DEBUG_AIO
>>   #define USE_DMA_CDROM
>> typedef struct IDEBus IDEBus;
>> @@ -333,12 +332,15 @@ struct unreported_events {
>>   };
>> enum ide_dma_cmd {
>> -IDE_DMA_READ,
>> +IDE_DMA_READ = 0,
>>   IDE_DMA_WRITE,
>>   IDE_DMA_TRIM,
>>   IDE_DMA_ATAPI,
>> +IDE_DMA__COUNT
>>   };
>>   +extern const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT];
> 
> I recommend you to avoid this declaring extern const array with size, I
> remember some compilers (old GCC?) ignoring array size in extern. Eric
> will correct me!
> 
> It is much safer to use a getter:
> 

Well, whether or not the compiler ignores it, you're right that it's
safer to use a getter. I don't think the width being declared HURTS any
compiler though, does it?

> const char *IDE_DMA_CMD_lookup(enum ide_dma_cmd cmd)
> {
> static const char *IDE_DM

Re: [Qemu-devel] [PATCH v2 10/17] MAINTAINERS: add missing SSI entries

[Alistair Francis]

On Wed, Aug 30, 2017 at 2:55 PM, Philippe Mathieu-Daudé  wrote:
> Alistair Francis volunteered :)
>
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Alistair Francis 

Thanks,
Alistair

> ---
>  MAINTAINERS | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bbe1191883..0a3a50aa25 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -986,10 +986,13 @@ F: hw/scsi/lsi53c895a.c
>
>  SSI
>  M: Peter Crosthwaite 
> +M: Alistair Francis 
>  S: Maintained
>  F: hw/ssi/*
>  F: hw/block/m25p80.c
> +F: include/hw/ssi/ssi.h
>  X: hw/ssi/xilinx_*
> +F: tests/m25p80-test.c
>
>  Xilinx SPI
>  M: Alistair Francis 
> --
> 2.14.1
>
>

Re: [Qemu-devel] [PATCH v2 14/17] MAINTAINERS: add missing entry for Generic Loader

[Alistair Francis]

On Wed, Aug 30, 2017 at 2:55 PM, Philippe Mathieu-Daudé  wrote:
> Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Alistair Francis 

Thanks,
Alistair

> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a48f633cad..0b77590dc8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1164,6 +1164,7 @@ M: Alistair Francis 
>  S: Maintained
>  F: hw/core/generic-loader.c
>  F: include/hw/core/generic-loader.h
> +F: docs/generic-loader.txt
>
>  CHRP NVRAM
>  M: Thomas Huth 
> --
> 2.14.1
>
>

Re: [Qemu-devel] [PATCH v2 0/7] QOMify MIPS cpu

[no-reply]

Hi,

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

Message-id: 20170830225225.27925-1-f4...@amsat.org
Subject: [Qemu-devel] [PATCH v2 0/7] QOMify MIPS cpu
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]   patchew/20170830225225.27925-1-f4...@amsat.org -> 
patchew/20170830225225.27925-1-f4...@amsat.org
 t [tag update]patchew/cover.1504111803.git.jc...@redhat.com -> 
patchew/cover.1504111803.git.jc...@redhat.com
Switched to a new branch 'test'
2eba874055 mips: update mips_cpu_list() to use object_class_get_list()
c9170c27df mips: replace cpu_mips_init() with cpu_generic_init()
bd1c0e43a2 mips: MIPSCPU model subclasses
e77d1d74ea mips: call cpu_mips_realize_env() from mips_cpu_realizefn()
4f1073d8be mips: split cpu_mips_realize_env() out of cpu_mips_init()
22f783eb50 mips: introduce internal.h and cleanup cpu.h
f846c7bfc6 mips: move hw/mips/cputimer.c to target/mips/

=== OUTPUT BEGIN ===
Checking PATCH 1/7: mips: move hw/mips/cputimer.c to target/mips/...
Checking PATCH 2/7: mips: introduce internal.h and cleanup cpu.h...
ERROR: space prohibited after that '&' (ctx:WxW)
#727: FILE: target/mips/internal.h:230:
+if ((env->CP0_VPControl >> CP0VPCtl_DIS) & 1) {
  ^

ERROR: space prohibited after that '&' (ctx:WxW)
#735: FILE: target/mips/internal.h:238:
+((other_cpu->env.CP0_VPControl >> CP0VPCtl_DIS) & 1)) {
 ^

ERROR: space prohibited after that '&' (ctx:WxW)
#755: FILE: target/mips/internal.h:258:
+env->hflags |= (env->CP0_Status >> CP0St_KSU) & MIPS_HFLAG_KSU;
   ^

total: 3 errors, 0 warnings, 842 lines checked

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

Checking PATCH 3/7: mips: split cpu_mips_realize_env() out of cpu_mips_init()...
Checking PATCH 4/7: mips: call cpu_mips_realize_env() from 
mips_cpu_realizefn()...
Checking PATCH 5/7: mips: MIPSCPU model subclasses...
Checking PATCH 6/7: mips: replace cpu_mips_init() with cpu_generic_init()...
Checking PATCH 7/7: mips: update mips_cpu_list() to use 
object_class_get_list()...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

[Qemu-devel] [PATCH v2 6/7] mips: replace cpu_mips_init() with cpu_generic_init()

[Philippe Mathieu-Daudé]

From: Igor Mammedov 

now cpu_mips_init() reimplements subset of cpu_generic_init()
tasks, so just drop it and use cpu_generic_init() directly.

Signed-off-by: Igor Mammedov 
Reviewed-by: Hervé Poussineau 
Signed-off-by: Philippe Mathieu-Daudé 
[PMD: use internal.h instead of cpu.h]
Tested-by: James Hogan 
---
 target/mips/cpu.h   |  3 +--
 hw/mips/cps.c   |  2 +-
 hw/mips/mips_fulong2e.c |  2 +-
 hw/mips/mips_jazz.c |  2 +-
 hw/mips/mips_malta.c|  2 +-
 hw/mips/mips_mipssim.c  |  2 +-
 hw/mips/mips_r4k.c  |  2 +-
 target/mips/translate.c | 17 -
 8 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 2f81e0f950..66265e4eb6 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -737,10 +737,9 @@ enum {
  */
 #define CPU_INTERRUPT_WAKE CPU_INTERRUPT_TGT_INT_0
 
-MIPSCPU *cpu_mips_init(const char *cpu_model);
 int cpu_mips_signal_handler(int host_signum, void *pinfo, void *puc);
 
-#define cpu_init(cpu_model) CPU(cpu_mips_init(cpu_model))
+#define cpu_init(cpu_model) cpu_generic_init(TYPE_MIPS_CPU, cpu_model)
 bool cpu_supports_cps_smp(const char *cpu_model);
 bool cpu_supports_isa(const char *cpu_model, unsigned int isa);
 void cpu_set_exception_base(int vp_index, target_ulong address);
diff --git a/hw/mips/cps.c b/hw/mips/cps.c
index 4ef337d5c4..708899cf92 100644
--- a/hw/mips/cps.c
+++ b/hw/mips/cps.c
@@ -71,7 +71,7 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
 bool itu_present = false;
 
 for (i = 0; i < s->num_vp; i++) {
-cpu = cpu_mips_init(s->cpu_model);
+cpu = MIPS_CPU(cpu_generic_init(TYPE_MIPS_CPU, s->cpu_model));
 if (cpu == NULL) {
 error_setg(errp, "%s: CPU initialization failed",  __func__);
 return;
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index 3532399a13..5d9462ec35 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -280,7 +280,7 @@ static void mips_fulong2e_init(MachineState *machine)
 if (cpu_model == NULL) {
 cpu_model = "Loongson-2E";
 }
-cpu = cpu_mips_init(cpu_model);
+cpu = MIPS_CPU(cpu_generic_init(TYPE_MIPS_CPU, cpu_model));
 if (cpu == NULL) {
 fprintf(stderr, "Unable to find CPU definition\n");
 exit(1);
diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index df2262a2a8..c1402de1ce 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -151,7 +151,7 @@ static void mips_jazz_init(MachineState *machine,
 if (cpu_model == NULL) {
 cpu_model = "R4000";
 }
-cpu = cpu_mips_init(cpu_model);
+cpu = MIPS_CPU(cpu_generic_init(TYPE_MIPS_CPU, cpu_model));
 if (cpu == NULL) {
 fprintf(stderr, "Unable to find CPU definition\n");
 exit(1);
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index af678f5784..9ecdc818b1 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -931,7 +931,7 @@ static void create_cpu_without_cps(const char *cpu_model,
 int i;
 
 for (i = 0; i < smp_cpus; i++) {
-cpu = cpu_mips_init(cpu_model);
+cpu = MIPS_CPU(cpu_generic_init(TYPE_MIPS_CPU, cpu_model));
 if (cpu == NULL) {
 fprintf(stderr, "Unable to find CPU definition\n");
 exit(1);
diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
index 07fc4c2300..1166834f54 100644
--- a/hw/mips/mips_mipssim.c
+++ b/hw/mips/mips_mipssim.c
@@ -163,7 +163,7 @@ mips_mipssim_init(MachineState *machine)
 cpu_model = "24Kf";
 #endif
 }
-cpu = cpu_mips_init(cpu_model);
+cpu = MIPS_CPU(cpu_generic_init(TYPE_MIPS_CPU, cpu_model));
 if (cpu == NULL) {
 fprintf(stderr, "Unable to find CPU definition\n");
 exit(1);
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index 2f5ced7409..de212f5c13 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -193,7 +193,7 @@ void mips_r4k_init(MachineState *machine)
 cpu_model = "24Kf";
 #endif
 }
-cpu = cpu_mips_init(cpu_model);
+cpu = MIPS_CPU(cpu_generic_init(TYPE_MIPS_CPU, cpu_model));
 if (cpu == NULL) {
 fprintf(stderr, "Unable to find CPU definition\n");
 exit(1);
diff --git a/target/mips/translate.c b/target/mips/translate.c
index f7128bc91d..d16d879df7 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -20523,23 +20523,6 @@ void cpu_mips_realize_env(CPUMIPSState *env)
 mvp_init(env, env->cpu_model);
 }
 
-MIPSCPU *cpu_mips_init(const char *cpu_model)
-{
-ObjectClass *oc;
-MIPSCPU *cpu;
-
-oc = cpu_class_by_name(TYPE_MIPS_CPU, cpu_model);
-if (oc == NULL) {
-return NULL;
-}
-
-cpu = MIPS_CPU(object_new(object_class_get_name(oc)));
-
-object_property_set_bool(OBJECT(cpu), true, "realized", NULL);
-
-return cpu;
-}
-
 bool cpu_supports_cps_smp(const char *cpu_model)
 {
 const mips_def_t *def = cpu_mips_find_by_name(cpu_model);
-- 
2.14.1

[Qemu-devel] [PATCH v2 7/7] mips: update mips_cpu_list() to use object_class_get_list()

[Philippe Mathieu-Daudé]

while here, move it from translate_init.c to helper.c

Signed-off-by: Philippe Mathieu-Daudé 
Tested-by: Igor Mammedov 
Tested-by: James Hogan 
---
 target/mips/helper.c | 46 
 target/mips/translate_init.c | 10 --
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/target/mips/helper.c b/target/mips/helper.c
index ea076261af..8d12b0088a 100644
--- a/target/mips/helper.c
+++ b/target/mips/helper.c
@@ -1093,3 +1093,49 @@ void QEMU_NORETURN do_raise_exception_err(CPUMIPSState 
*env,
 
 cpu_loop_exit_restore(cs, pc);
 }
+
+/* Sort alphabetically by type name, except for "any". */
+static gint mips_cpu_list_compare(gconstpointer a, gconstpointer b)
+{
+ObjectClass *class_a = (ObjectClass *)a;
+ObjectClass *class_b = (ObjectClass *)b;
+const char *name_a, *name_b;
+
+name_a = object_class_get_name(class_a);
+name_b = object_class_get_name(class_b);
+if (strcmp(name_a, "any-" TYPE_MIPS_CPU) == 0) {
+return 1;
+} else if (strcmp(name_b, "any-" TYPE_MIPS_CPU) == 0) {
+return -1;
+} else {
+return strcmp(name_a, name_b);
+}
+}
+
+static void mips_cpu_list_entry(gpointer data, gpointer user_data)
+{
+ObjectClass *oc = data;
+CPUListState *s = user_data;
+const char *typename;
+char *name;
+
+typename = object_class_get_name(oc);
+name = g_strndup(typename, strlen(typename) - strlen("-" TYPE_MIPS_CPU));
+(*s->cpu_fprintf)(s->file, "  %s\n", name);
+g_free(name);
+}
+
+void mips_cpu_list(FILE *f, fprintf_function cpu_fprintf)
+{
+CPUListState s = {
+.file = f,
+.cpu_fprintf = cpu_fprintf,
+};
+GSList *list;
+
+list = object_class_get_list(TYPE_MIPS_CPU, false);
+list = g_slist_sort(list, mips_cpu_list_compare);
+(*cpu_fprintf)(f, "Available CPUs:\n");
+g_slist_foreach(list, mips_cpu_list_entry, &s);
+g_slist_free(list);
+}
diff --git a/target/mips/translate_init.c b/target/mips/translate_init.c
index 8bbded46c4..b75f4c9065 100644
--- a/target/mips/translate_init.c
+++ b/target/mips/translate_init.c
@@ -767,16 +767,6 @@ static const mips_def_t *cpu_mips_find_by_name (const char 
*name)
 return NULL;
 }
 
-void mips_cpu_list (FILE *f, fprintf_function cpu_fprintf)
-{
-int i;
-
-for (i = 0; i < ARRAY_SIZE(mips_defs); i++) {
-(*cpu_fprintf)(f, "MIPS '%s'\n",
-   mips_defs[i].name);
-}
-}
-
 #ifndef CONFIG_USER_ONLY
 static void no_mmu_init (CPUMIPSState *env, const mips_def_t *def)
 {
-- 
2.14.1

[Qemu-devel] [PATCH v2 2/7] mips: introduce internal.h and cleanup cpu.h

[Philippe Mathieu-Daudé]

no logical change, only code movement (and fix a comment typo).

Signed-off-by: Philippe Mathieu-Daudé 
Tested-by: Igor Mammedov 
Tested-by: James Hogan 
---

This patch triggers 3 positive falses from checkpatch:

ERROR: space prohibited after that '&' (ctx:WxW)
#664: FILE: target/mips/internal.h:230:
+if ((env->CP0_VPControl >> CP0VPCtl_DIS) & 1) {
  ^
#672: FILE: target/mips/internal.h:238:
+((other_cpu->env.CP0_VPControl >> CP0VPCtl_DIS) & 1)) {
 ^
#692: FILE: target/mips/internal.h:258:
+env->hflags |= (env->CP0_Status >> CP0St_KSU) & MIPS_HFLAG_KSU;
   ^
total: 3 errors, 0 warnings, 842 lines checked

This is a "binary vs unary operators" confusion.

 target/mips/cpu.h| 354 +
 target/mips/internal.h   | 362 +++
 target/mips/cp0_timer.c  |   1 +
 target/mips/cpu.c|   1 +
 target/mips/gdbstub.c|   1 +
 target/mips/helper.c |   1 +
 target/mips/kvm.c|   1 +
 target/mips/machine.c|   1 +
 target/mips/msa_helper.c |   1 +
 target/mips/op_helper.c  |   1 +
 target/mips/translate.c  |   1 +
 11 files changed, 372 insertions(+), 353 deletions(-)
 create mode 100644 target/mips/internal.h

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 74f6a5b098..2f81e0f950 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -1,8 +1,6 @@
 #ifndef MIPS_CPU_H
 #define MIPS_CPU_H
 
-//#define DEBUG_OP
-
 #define ALIGNED_ONLY
 
 #define CPUArchState struct CPUMIPSState
@@ -15,56 +13,11 @@
 
 struct CPUMIPSState;
 
-typedef struct r4k_tlb_t r4k_tlb_t;
-struct r4k_tlb_t {
-target_ulong VPN;
-uint32_t PageMask;
-uint16_t ASID;
-unsigned int G:1;
-unsigned int C0:3;
-unsigned int C1:3;
-unsigned int V0:1;
-unsigned int V1:1;
-unsigned int D0:1;
-unsigned int D1:1;
-unsigned int XI0:1;
-unsigned int XI1:1;
-unsigned int RI0:1;
-unsigned int RI1:1;
-unsigned int EHINV:1;
-uint64_t PFN[2];
-};
-
-#if !defined(CONFIG_USER_ONLY)
 typedef struct CPUMIPSTLBContext CPUMIPSTLBContext;
-struct CPUMIPSTLBContext {
-uint32_t nb_tlb;
-uint32_t tlb_in_use;
-int (*map_address) (struct CPUMIPSState *env, hwaddr *physical, int *prot, 
target_ulong address, int rw, int access_type);
-void (*helper_tlbwi)(struct CPUMIPSState *env);
-void (*helper_tlbwr)(struct CPUMIPSState *env);
-void (*helper_tlbp)(struct CPUMIPSState *env);
-void (*helper_tlbr)(struct CPUMIPSState *env);
-void (*helper_tlbinv)(struct CPUMIPSState *env);
-void (*helper_tlbinvf)(struct CPUMIPSState *env);
-union {
-struct {
-r4k_tlb_t tlb[MIPS_TLB_MAX];
-} r4k;
-} mmu;
-};
-#endif
 
 /* MSA Context */
 #define MSA_WRLEN (128)
 
-enum CPUMIPSMSADataFormat {
-DF_BYTE = 0,
-DF_HALF,
-DF_WORD,
-DF_DOUBLE
-};
-
 typedef union wr_t wr_t;
 union wr_t {
 int8_t  b[MSA_WRLEN/8];
@@ -682,40 +635,6 @@ static inline MIPSCPU *mips_env_get_cpu(CPUMIPSState *env)
 
 #define ENV_OFFSET offsetof(MIPSCPU, env)
 
-#ifndef CONFIG_USER_ONLY
-extern const struct VMStateDescription vmstate_mips_cpu;
-#endif
-
-void mips_cpu_do_interrupt(CPUState *cpu);
-bool mips_cpu_exec_interrupt(CPUState *cpu, int int_req);
-void mips_cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf,
- int flags);
-hwaddr mips_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
-int mips_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
-int mips_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
-void mips_cpu_do_unaligned_access(CPUState *cpu, vaddr addr,
-  MMUAccessType access_type,
-  int mmu_idx, uintptr_t retaddr);
-
-#if !defined(CONFIG_USER_ONLY)
-int no_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int *prot,
-target_ulong address, int rw, int access_type);
-int fixed_mmu_map_address (CPUMIPSState *env, hwaddr *physical, int *prot,
-   target_ulong address, int rw, int access_type);
-int r4k_map_address (CPUMIPSState *env, hwaddr *physical, int *prot,
- target_ulong address, int rw, int access_type);
-void r4k_helper_tlbwi(CPUMIPSState *env);
-void r4k_helper_tlbwr(CPUMIPSState *env);
-void r4k_helper_tlbp(CPUMIPSState *env);
-void r4k_helper_tlbr(CPUMIPSState *env);
-void r4k_helper_tlbinv(CPUMIPSState *env);
-void r4k_helper_tlbinvf(CPUMIPSState *env);
-
-void mips_cpu_unassigned_access(CPUState *cpu, hwaddr addr,
-bool is_write, bool is_exec, int unused,
-unsigned size);
-#endif
-
 void mips_cpu_list (FILE *f, fprintf_function cpu_fprintf);
 
 #define cpu_signal_handler cpu_mips_signal_handler

[Qemu-devel] [PATCH v2 3/7] mips: split cpu_mips_realize_env() out of cpu_mips_init()

[Philippe Mathieu-Daudé]

so it can be used in mips_cpu_realizefn() in the next commit

Signed-off-by: Philippe Mathieu-Daudé 
Tested-by: Igor Mammedov 
Tested-by: James Hogan 
---
 target/mips/internal.h  |  1 +
 target/mips/translate.c | 19 ---
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/target/mips/internal.h b/target/mips/internal.h
index 91c2df4537..cf4c9db427 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -132,6 +132,7 @@ void mips_tcg_init(void);
 
 /* TODO QOM'ify CPU reset and remove */
 void cpu_state_reset(CPUMIPSState *s);
+void cpu_mips_realize_env(CPUMIPSState *env);
 
 /* cp0_timer.c */
 uint32_t cpu_mips_get_random(CPUMIPSState *env);
diff --git a/target/mips/translate.c b/target/mips/translate.c
index f0febaf1b2..5fc7979ac5 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -20512,6 +20512,17 @@ void mips_tcg_init(void)
 
 #include "translate_init.c"
 
+void cpu_mips_realize_env(CPUMIPSState *env)
+{
+env->exception_base = (int32_t)0xBFC0;
+
+#ifndef CONFIG_USER_ONLY
+mmu_init(env, env->cpu_model);
+#endif
+fpu_init(env, env->cpu_model);
+mvp_init(env, env->cpu_model);
+}
+
 MIPSCPU *cpu_mips_init(const char *cpu_model)
 {
 MIPSCPU *cpu;
@@ -20524,13 +20535,7 @@ MIPSCPU *cpu_mips_init(const char *cpu_model)
 cpu = MIPS_CPU(object_new(TYPE_MIPS_CPU));
 env = &cpu->env;
 env->cpu_model = def;
-env->exception_base = (int32_t)0xBFC0;
-
-#ifndef CONFIG_USER_ONLY
-mmu_init(env, def);
-#endif
-fpu_init(env, def);
-mvp_init(env, def);
+cpu_mips_realize_env(env);
 
 object_property_set_bool(OBJECT(cpu), true, "realized", NULL);
 
-- 
2.14.1

[Qemu-devel] [PATCH v2 1/7] mips: move hw/mips/cputimer.c to target/mips/

[Philippe Mathieu-Daudé]

This timer is a required part of the MIPS32/MIPS64 System Control coprocessor
(CP0). Moving it with the other architecture related files will allow an opaque
use of CPUMIPSState* in the next commit (introduce "internal.h").

also remove it from 'user' targets, remove an unnecessary include.

Signed-off-by: Philippe Mathieu-Daudé 
Tested-by: Igor Mammedov 
Tested-by: James Hogan 
---
 hw/mips/cputimer.c => target/mips/cp0_timer.c | 1 -
 hw/mips/Makefile.objs | 2 +-
 target/mips/Makefile.objs | 2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)
 rename hw/mips/cputimer.c => target/mips/cp0_timer.c (99%)

diff --git a/hw/mips/cputimer.c b/target/mips/cp0_timer.c
similarity index 99%
rename from hw/mips/cputimer.c
rename to target/mips/cp0_timer.c
index 8a166b3ea7..a9a58c5604 100644
--- a/hw/mips/cputimer.c
+++ b/target/mips/cp0_timer.c
@@ -21,7 +21,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "hw/hw.h"
 #include "hw/mips/cpudevs.h"
 #include "qemu/timer.h"
 #include "sysemu/kvm.h"
diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs
index 48cd2ef50e..17a311aaba 100644
--- a/hw/mips/Makefile.objs
+++ b/hw/mips/Makefile.objs
@@ -1,5 +1,5 @@
 obj-y += mips_r4k.o mips_malta.o mips_mipssim.o
-obj-y += addr.o cputimer.o mips_int.o
+obj-y += addr.o mips_int.o
 obj-$(CONFIG_JAZZ) += mips_jazz.o
 obj-$(CONFIG_FULONG) += mips_fulong2e.o
 obj-y += gt64xxx_pci.o
diff --git a/target/mips/Makefile.objs b/target/mips/Makefile.objs
index bc5ed8511f..651f36f517 100644
--- a/target/mips/Makefile.objs
+++ b/target/mips/Makefile.objs
@@ -1,4 +1,4 @@
 obj-y += translate.o dsp_helper.o op_helper.o lmi_helper.o helper.o cpu.o
 obj-y += gdbstub.o msa_helper.o mips-semi.o
-obj-$(CONFIG_SOFTMMU) += machine.o
+obj-$(CONFIG_SOFTMMU) += machine.o cp0_timer.o
 obj-$(CONFIG_KVM) += kvm.o
-- 
2.14.1

[Qemu-devel] [PATCH v2 4/7] mips: call cpu_mips_realize_env() from mips_cpu_realizefn()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Tested-by: Igor Mammedov 
Tested-by: James Hogan 
---
 target/mips/cpu.c   | 3 +++
 target/mips/translate.c | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 68bf423e9d..e3ef835599 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -123,6 +123,7 @@ static void mips_cpu_disas_set_info(CPUState *s, 
disassemble_info *info) {
 static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
 {
 CPUState *cs = CPU(dev);
+MIPSCPU *cpu = MIPS_CPU(dev);
 MIPSCPUClass *mcc = MIPS_CPU_GET_CLASS(dev);
 Error *local_err = NULL;
 
@@ -132,6 +133,8 @@ static void mips_cpu_realizefn(DeviceState *dev, Error 
**errp)
 return;
 }
 
+cpu_mips_realize_env(&cpu->env);
+
 cpu_reset(cs);
 qemu_init_vcpu(cs);
 
diff --git a/target/mips/translate.c b/target/mips/translate.c
index 5fc7979ac5..94c38e8755 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -20535,7 +20535,6 @@ MIPSCPU *cpu_mips_init(const char *cpu_model)
 cpu = MIPS_CPU(object_new(TYPE_MIPS_CPU));
 env = &cpu->env;
 env->cpu_model = def;
-cpu_mips_realize_env(env);
 
 object_property_set_bool(OBJECT(cpu), true, "realized", NULL);
 
-- 
2.14.1

[Qemu-devel] [PATCH v2 5/7] mips: MIPSCPU model subclasses

[Philippe Mathieu-Daudé]

From: Igor Mammedov 

Register separate QOM types for each mips cpu model,
so it would be possible to reuse generic CPU creation
routines.

Signed-off-by: Igor Mammedov 
Signed-off-by: Philippe Mathieu-Daudé 
[PMD: use internal.h, use void* to hold cpu_def in MIPSCPUClass,
 mark MIPSCPU abstract]
Tested-by: James Hogan 
---
 target/mips/cpu-qom.h|  1 +
 target/mips/internal.h   | 59 
 target/mips/cpu.c| 53 ++-
 target/mips/translate.c  | 13 +-
 target/mips/translate_init.c | 58 ++-
 5 files changed, 120 insertions(+), 64 deletions(-)

diff --git a/target/mips/cpu-qom.h b/target/mips/cpu-qom.h
index 3f5bf23823..085711d8f9 100644
--- a/target/mips/cpu-qom.h
+++ b/target/mips/cpu-qom.h
@@ -49,6 +49,7 @@ typedef struct MIPSCPUClass {
 
 DeviceRealize parent_realize;
 void (*parent_reset)(CPUState *cpu);
+const void *cpu_def;
 } MIPSCPUClass;
 
 typedef struct MIPSCPU MIPSCPU;
diff --git a/target/mips/internal.h b/target/mips/internal.h
index cf4c9db427..45ded3484c 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -7,6 +7,65 @@
 #ifndef MIPS_INTERNAL_H
 #define MIPS_INTERNAL_H
 
+
+/* MMU types, the first four entries have the same layout as the
+   CP0C0_MT field.  */
+enum mips_mmu_types {
+MMU_TYPE_NONE,
+MMU_TYPE_R4000,
+MMU_TYPE_RESERVED,
+MMU_TYPE_FMT,
+MMU_TYPE_R3000,
+MMU_TYPE_R6000,
+MMU_TYPE_R8000
+};
+
+struct mips_def_t {
+const char *name;
+int32_t CP0_PRid;
+int32_t CP0_Config0;
+int32_t CP0_Config1;
+int32_t CP0_Config2;
+int32_t CP0_Config3;
+int32_t CP0_Config4;
+int32_t CP0_Config4_rw_bitmask;
+int32_t CP0_Config5;
+int32_t CP0_Config5_rw_bitmask;
+int32_t CP0_Config6;
+int32_t CP0_Config7;
+target_ulong CP0_LLAddr_rw_bitmask;
+int CP0_LLAddr_shift;
+int32_t SYNCI_Step;
+int32_t CCRes;
+int32_t CP0_Status_rw_bitmask;
+int32_t CP0_TCStatus_rw_bitmask;
+int32_t CP0_SRSCtl;
+int32_t CP1_fcr0;
+int32_t CP1_fcr31_rw_bitmask;
+int32_t CP1_fcr31;
+int32_t MSAIR;
+int32_t SEGBITS;
+int32_t PABITS;
+int32_t CP0_SRSConf0_rw_bitmask;
+int32_t CP0_SRSConf0;
+int32_t CP0_SRSConf1_rw_bitmask;
+int32_t CP0_SRSConf1;
+int32_t CP0_SRSConf2_rw_bitmask;
+int32_t CP0_SRSConf2;
+int32_t CP0_SRSConf3_rw_bitmask;
+int32_t CP0_SRSConf3;
+int32_t CP0_SRSConf4_rw_bitmask;
+int32_t CP0_SRSConf4;
+int32_t CP0_PageGrain_rw_bitmask;
+int32_t CP0_PageGrain;
+target_ulong CP0_EBaseWG_rw_bitmask;
+int insn_flags;
+enum mips_mmu_types mmu_type;
+};
+
+extern const struct mips_def_t mips_defs[];
+extern const int mips_defs_number;
+
 enum CPUMIPSMSADataFormat {
 DF_BYTE = 0,
 DF_HALF,
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index e3ef835599..84b6f8bf68 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -146,12 +146,37 @@ static void mips_cpu_initfn(Object *obj)
 CPUState *cs = CPU(obj);
 MIPSCPU *cpu = MIPS_CPU(obj);
 CPUMIPSState *env = &cpu->env;
+MIPSCPUClass *mcc = MIPS_CPU_GET_CLASS(obj);
 
 cs->env_ptr = env;
 
 if (tcg_enabled()) {
 mips_tcg_init();
 }
+
+if (mcc->cpu_def) {
+env->cpu_model = mcc->cpu_def;
+}
+}
+
+static char *mips_cpu_type_name(const char *cpu_model)
+{
+return g_strdup_printf("%s-" TYPE_MIPS_CPU, cpu_model);
+}
+
+static ObjectClass *mips_cpu_class_by_name(const char *cpu_model)
+{
+ObjectClass *oc;
+char *typename;
+
+if (cpu_model == NULL) {
+return NULL;
+}
+
+typename = mips_cpu_type_name(cpu_model);
+oc = object_class_by_name(typename);
+g_free(typename);
+return oc;
 }
 
 static void mips_cpu_class_init(ObjectClass *c, void *data)
@@ -166,6 +191,7 @@ static void mips_cpu_class_init(ObjectClass *c, void *data)
 mcc->parent_reset = cc->reset;
 cc->reset = mips_cpu_reset;
 
+cc->class_by_name = mips_cpu_class_by_name;
 cc->has_work = mips_cpu_has_work;
 cc->do_interrupt = mips_cpu_do_interrupt;
 cc->cpu_exec_interrupt = mips_cpu_exec_interrupt;
@@ -193,14 +219,39 @@ static const TypeInfo mips_cpu_type_info = {
 .parent = TYPE_CPU,
 .instance_size = sizeof(MIPSCPU),
 .instance_init = mips_cpu_initfn,
-.abstract = false,
+.abstract = true,
 .class_size = sizeof(MIPSCPUClass),
 .class_init = mips_cpu_class_init,
 };
 
+static void mips_cpu_cpudef_class_init(ObjectClass *oc, void *data)
+{
+MIPSCPUClass *mcc = MIPS_CPU_CLASS(oc);
+mcc->cpu_def = data;
+}
+
+static void mips_register_cpudef_type(const struct mips_def_t *def)
+{
+char *typename = mips_cpu_type_name(def->name);
+TypeInfo ti = {
+.name = typename,
+.parent = TYPE_MIPS_CPU,
+.class_init = mips_cpu_cpudef_class_init,
+.class_data = (void *)def,
+};
+
+ 

[Qemu-devel] [PATCH v2 0/7] QOMify MIPS cpu

[Philippe Mathieu-Daudé]

Hi,

This series is based on Igor's "complete cpu QOMification" [1] but only modify
the MIPS part. Igor posted an updated series [2], both series should apply
separately.

Igor suggested on IRC this series could enter via the Machine core tree, so
I added Eduardo and Marcel.

Regards,

Phil.

[1]: http://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg04414.html
[2]: http://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg03364.html

v2: 
- added Igor and James Tested-by
- squashed "!fixup mips: now than MIPSCPU is QOMified, mark it abstract"

PS: code movement somehow triggers a "binary vs unary operators" confusion
in checkpatch: "ERROR: space prohibited after that '&' (ctx:WxW)"

Igor Mammedov (2):
  mips: MIPSCPU model subclasses
  mips: replace cpu_mips_init() with cpu_generic_init()

Philippe Mathieu-Daudé (5):
  mips: move hw/mips/cputimer.c to target/mips/
  mips: introduce internal.h and cleanup cpu.h
  mips: split cpu_mips_realize_env() out of cpu_mips_init()
  mips: call cpu_mips_realize_env() from mips_cpu_realizefn()
  mips: update mips_cpu_list() to use object_class_get_list()

 target/mips/cpu-qom.h |   1 +
 target/mips/cpu.h | 357 +-
 target/mips/internal.h| 422 ++
 hw/mips/cps.c |   2 +-
 hw/mips/mips_fulong2e.c   |   2 +-
 hw/mips/mips_jazz.c   |   2 +-
 hw/mips/mips_malta.c  |   2 +-
 hw/mips/mips_mipssim.c|   2 +-
 hw/mips/mips_r4k.c|   2 +-
 hw/mips/cputimer.c => target/mips/cp0_timer.c |   2 +-
 target/mips/cpu.c |  57 +++-
 target/mips/gdbstub.c |   1 +
 target/mips/helper.c  |  47 +++
 target/mips/kvm.c |   1 +
 target/mips/machine.c |   1 +
 target/mips/msa_helper.c  |   1 +
 target/mips/op_helper.c   |   1 +
 target/mips/translate.c   |  23 +-
 target/mips/translate_init.c  |  68 +
 hw/mips/Makefile.objs |   2 +-
 target/mips/Makefile.objs |   2 +-
 21 files changed, 549 insertions(+), 449 deletions(-)
 create mode 100644 target/mips/internal.h
 rename hw/mips/cputimer.c => target/mips/cp0_timer.c (99%)

-- 
2.14.1

Re: [Qemu-devel] [Qemu-block] [PATCH v2 8/9] AHCI: pretty-print FIS to buffer instead of stderr

[John Snow]

On 08/30/2017 05:17 AM, Stefan Hajnoczi wrote:
> On Tue, Aug 29, 2017 at 04:49:33PM -0400, John Snow wrote:
>> The current FIS printing routines dump the FIS to screen. adjust this
>> such that it dumps to buffer instead, then use this ability to have
>> FIS dump mechanisms via trace-events instead of compiled defines.
>>
>> Signed-off-by: John Snow 
>> ---
>>  hw/ide/ahci.c   | 54 
>> +++--
>>  hw/ide/trace-events |  4 
>>  2 files changed, 48 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>> index a0a4dd6..2e75f9b 100644
>> --- a/hw/ide/ahci.c
>> +++ b/hw/ide/ahci.c
>> @@ -644,20 +644,45 @@ static void ahci_reset_port(AHCIState *s, int port)
>>  ahci_init_d2h(d);
>>  }
>>  
>> -static void debug_print_fis(uint8_t *fis, int cmd_len)
>> +/* Buffer pretty output based on a raw FIS structure. */
>> +static void ahci_pretty_buffer_fis(uint8_t *fis, int cmd_len, char **out)
> 
> Simplified function using GString:
> 
>   static char *ahci_pretty_buffer_fis(const uint8_t *fis, int cmd_len)
>   {
>   GString *s = g_string_new("FIS:");
>   int i;
> 
>   for (i = 0; i < cmd_len; i++) {
>   if (i % 16 == 0) {
>   g_string_append_printf(s, "\n0x%02x:", i);
> }
> 
>   g_string_append_printf(s, " %02x", fis[i]);
>   }
> 
>   g_string_append_c('\n');
>   return g_string_free(s, FALSE);
>   }
> 
> It's less efficient due to extra mallocs but a lot easier to read.
> 

eeeea I guess I don't need to be bleedingly efficient with debug
printfs...

And in this example I don't need to worry about the math being precisely
correct. I'll make the change :(

>>  {
>> -#if DEBUG_AHCI
>> +size_t bufsize;
>> +char *pbuf;
>> +char *pptr;
>> +size_t lines = DIV_ROUND_UP(cmd_len, 16);
>> +const char *preamble = "FIS:";
>>  int i;
>>  
>> -fprintf(stderr, "fis:");
>> +/* Total amount of memory to store FISes in HBA memory */
>> +g_assert_cmpint(cmd_len, <=, 0x100);
>> +g_assert(out);
>> +
>> +/* Printed like:
>> + * FIS:\n
>> + * 0x00: 00 11 22 33 44 55 66 77 88 99 aa bb cc dd ee \n
>> + * 0x10: ff \n
>> + * \0
>> + *
>> + * Four bytes for the preamble, seven for each line prefix (including a
>> + * newline to start a new line), three bytes for each source byte,
>> + * a trailing newline and a terminal null byte.
>> + */
>> +
>> +bufsize = strlen(preamble) + ((6 + 1) * lines) + (3 * cmd_len) + 1 + 1;
>> +pbuf = g_malloc(bufsize);
>> +pptr = pbuf;
>> +pptr += sprintf(pptr, "%s", preamble);
>>  for (i = 0; i < cmd_len; i++) {
>>  if ((i & 0xf) == 0) {
>> -fprintf(stderr, "\n%02x:",i);
>> +pptr += sprintf(pptr, "\n0x%02x: ", i);
>>  }
>> -fprintf(stderr, "%02x ",fis[i]);
>> +pptr += sprintf(pptr, "%02x ", fis[i]);
>>  }
>> -fprintf(stderr, "\n");
>> -#endif
>> +pptr += sprintf(pptr, "\n");
>> +pptr += 1; /* \0 */
>> +g_assert(pbuf + bufsize == pptr);
>> +*out = pbuf;
>>  }
>>  
>>  static bool ahci_map_fis_address(AHCIDevice *ad)
>> @@ -1201,7 +1226,12 @@ static void handle_reg_h2d_fis(AHCIState *s, int port,
>>   * table to ide_state->io_buffer */
>>  if (opts & AHCI_CMD_ATAPI) {
>>  memcpy(ide_state->io_buffer, &cmd_fis[AHCI_COMMAND_TABLE_ACMD], 
>> 0x10);
>> -debug_print_fis(ide_state->io_buffer, 0x10);
>> +if (TRACE_HANDLE_REG_H2D_FIS_DUMP_ENABLED) {
> 
> This should probably be:
> 
>   if (trace_event_get_state_backends(TRACE_HANDLE_REG_H2D_FIS_DUMP)) {
> 
> The difference is that TRACE_HANDLE_REG_H2D_FIS_DUMP_ENABLED is set at
> compile time while trace_event_get_state_backends() checks if the event
> is enabled at run-time.
> 
> Therefore TRACE_HANDLE_REG_H2D_FIS_DUMP_ENABLED causes the trace event
> to fire even when the user hasn't enabled the trace event yet.  That
> would be a waste of CPU.
> 

Oh, cool! Nice tip!

>> +char *pretty_fis;
>> +ahci_pretty_buffer_fis(ide_state->io_buffer, 0x10, &pretty_fis);
>> +trace_handle_reg_h2d_fis_dump(s, port, pretty_fis);
>> +g_free(pretty_fis);
>> +}
>>  s->dev[port].done_atapi_packet = false;
>>  /* XXX send PIO setup FIS */
>>  }
>> @@ -1256,8 +1286,12 @@ static int handle_cmd(AHCIState *s, int port, uint8_t 
>> slot)
>>  trace_handle_cmd_badmap(s, port, cmd_len);
>>  goto out;
>>  }
>> -debug_print_fis(cmd_fis, 0x80);
>> -
>> +if (TRACE_HANDLE_CMD_FIS_DUMP_ENABLED) {
> 
> Same here.
> 

Sure thing. There's probably a block in ATAPI that needs this treatment,
too.

>> +char *pretty_fis;
>> +ahci_pretty_buffer_fis(cmd_fis, 0x80, &pretty_fis);
>> +trace_handle_cmd_fis_dump(s, port, pretty_fis);
>> +g_free(pretty_fis);
>> +}
>>  switch (cmd_fis[0]) {
>>  case SATA_FIS_TYPE_REG

Re: [Qemu-devel] [PATCH v6 00/18] make dirty-bitmap byte-based

[John Snow]

On 08/30/2017 05:05 PM, Eric Blake wrote:
> There are patches floating around to add NBD_CMD_BLOCK_STATUS,
> but NBD wants to report status on byte granularity (even if the
> reporting will probably be naturally aligned to sectors or even
> much higher levels).  I've therefore started the task of
> converting our block status code to report at a byte granularity
> rather than sectors.
> 
> Now that 2.11 is open, I'm rebasing/reposting the remaining patches.
> 
> The overall conversion currently looks like:
> part 1: bdrv_is_allocated (merged in 2.10, commit 51b0a488)
> part 2: dirty-bitmap (this series, v5 was here [1])
> part 3: bdrv_get_block_status (v3 is posted [2] and is mostly reviewed, but
> needs a rebase)
> part 4: .bdrv_co_block_status (v2 is posted [3], but needs a rebase)
> 
> Available as a tag at:
> git fetch git://repo.or.cz/qemu/ericb.git nbd-byte-dirty-v6
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg03512.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg03853.html
> [3] https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg04370.html
> 
> Diff from v5:
> - add another patch (more for ease of bookkeeping, as it was previously
> posted independently)
> - drop bug fixes that were hoisted into 2.10 (v5 1/18, plus 14/18)
> 
> 001/18:[down] 'block: Make bdrv_img_create() size selection easier to read'
> 002/18:[] [--] 'hbitmap: Rename serialization_granularity to 
> serialization_align'
> 003/18:[] [--] 'qcow2: Ensure bitmap serialization is aligned'
> 004/18:[] [--] 'dirty-bitmap: Drop unused functions'
> 005/18:[] [--] 'dirty-bitmap: Change bdrv_dirty_bitmap_size() to report 
> bytes'
> 006/18:[] [--] 'dirty-bitmap: Change bdrv_dirty_bitmap_*serialize*() to 
> take bytes'
> 007/18:[] [--] 'qcow2: Switch sectors_covered_by_bitmap_cluster() to 
> byte-based'
> 008/18:[] [--] 'dirty-bitmap: Set iterator start by offset, not sector'
> 009/18:[] [--] 'dirty-bitmap: Change bdrv_dirty_iter_next() to report 
> byte offset'
> 010/18:[] [--] 'dirty-bitmap: Change bdrv_get_dirty_count() to report 
> bytes'
> 011/18:[] [--] 'dirty-bitmap: Change bdrv_get_dirty_locked() to take 
> bytes'
> 012/18:[] [--] 'dirty-bitmap: Change bdrv_[re]set_dirty_bitmap() to use 
> bytes'
> 013/18:[] [--] 'mirror: Switch mirror_dirty_init() to byte-based 
> iteration'
> 014/18:[0004] [FC] 'qcow2: Switch qcow2_measure() to byte-based iteration'
> 015/18:[] [--] 'qcow2: Switch load_bitmap_data() to byte-based iteration'
> 016/18:[] [--] 'qcow2: Switch store_bitmap_data() to byte-based iteration'
> 017/18:[] [--] 'dirty-bitmap: Switch bdrv_set_dirty() to bytes'
> 018/18:[] [--] 'dirty-bitmap: Convert internal hbitmap size/granularity'
> 
> Eric Blake (18):
>   block: Make bdrv_img_create() size selection easier to read
>   hbitmap: Rename serialization_granularity to serialization_align
>   qcow2: Ensure bitmap serialization is aligned
>   dirty-bitmap: Drop unused functions
>   dirty-bitmap: Change bdrv_dirty_bitmap_size() to report bytes
>   dirty-bitmap: Change bdrv_dirty_bitmap_*serialize*() to take bytes
>   qcow2: Switch sectors_covered_by_bitmap_cluster() to byte-based
>   dirty-bitmap: Set iterator start by offset, not sector
>   dirty-bitmap: Change bdrv_dirty_iter_next() to report byte offset
>   dirty-bitmap: Change bdrv_get_dirty_count() to report bytes
>   dirty-bitmap: Change bdrv_get_dirty_locked() to take bytes
>   dirty-bitmap: Change bdrv_[re]set_dirty_bitmap() to use bytes
>   mirror: Switch mirror_dirty_init() to byte-based iteration
>   qcow2: Switch qcow2_measure() to byte-based iteration
>   qcow2: Switch load_bitmap_data() to byte-based iteration
>   qcow2: Switch store_bitmap_data() to byte-based iteration
>   dirty-bitmap: Switch bdrv_set_dirty() to bytes
>   dirty-bitmap: Convert internal hbitmap size/granularity
> 
>  include/block/block_int.h|   2 +-
>  include/block/dirty-bitmap.h |  41 +-
>  include/qemu/hbitmap.h   |   8 +--
>  block/io.c   |   6 +-
>  block.c  |   2 +-
>  block/backup.c   |   7 +--
>  block/dirty-bitmap.c | 130 
> ++-
>  block/mirror.c   |  76 +++--
>  block/qcow2-bitmap.c |  57 +--
>  block/qcow2.c|  22 
>  migration/block.c|  12 ++--
>  tests/test-hbitmap.c |  10 ++--
>  util/hbitmap.c   |   8 +--
>  13 files changed, 154 insertions(+), 227 deletions(-)
> 

Should this go through the bitmap tree, or since it's touching qcow2,
I'll let Kevin/Max/Stefan stage it?

--js

Re: [Qemu-devel] [PATCH v3 1/5] qemu-iotests: set TEST_DIR to a unique dir for each test

[Jeff Cody]

On Wed, Aug 30, 2017 at 06:15:05PM -0400, John Snow wrote:
> 
> 
> On 08/30/2017 12:52 PM, Jeff Cody wrote:
> > Right now, all qemu-iotests output data into the same scratch directory,
> > and so each test needs to be responsible for cleaning up its own files.
> > 
> > Have each test use 'scratch/$seq' as its temp directory, so the check
> > script can do simple cleanup of removing the whole temporary directory.
> > 
> > Reviewed-by: Eric Blake 
> > Signed-off-by: Jeff Cody 
> > ---
> >  tests/qemu-iotests/check | 21 +
> >  1 file changed, 17 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> > index d504b6e..f6ca85d 100755
> > --- a/tests/qemu-iotests/check
> > +++ b/tests/qemu-iotests/check
> > @@ -243,6 +243,7 @@ seq="check"
> >  
> >  for seq in $list
> >  do
> > +TEST_DIR_SEQ=$TEST_DIR/$seq
> >  err=false
> >  printf %s "$seq"
> >  if [ -n "$TESTS_REMAINING_LOG" ] ; then
> > @@ -289,13 +290,23 @@ do
> >  fi
> >  export OUTPUT_DIR=$PWD
> >  if $debug; then
> > -(cd "$source_iotests";
> > +(
> > +export TEST_DIR=$TEST_DIR_SEQ
> > +. "$source_iotests/common.config"
> > +. "$source_iotests/common.rc"
> 
> What purpose do these serve?
>

This is setting $TEST_DIR according to the $seq number (test # being run) in
the bash subshell that the tests are being run from.  So that all the other
variables that are based on the $TEST_DIR are set appropriately, this also
sources them in the subshell prior to running the test.  That way their
environment is with $TEST_DIR_SEQ rather than the original base $TEST_DIR.

> > +cd "$source_iotests" &&
> >  MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(($RANDOM % 255 + 1))} \
> > -$run_command -d 2>&1 | tee $tmp.out)
> > +$run_command -d 2>&1 | tee $tmp.out
> > +)
> >  else
> > -(cd "$source_iotests";
> > +(
> > +export TEST_DIR=$TEST_DIR_SEQ
> > +. "$source_iotests/common.config"
> > +. "$source_iotests/common.rc"
> > + cd "$source_iotests" &&
> >  MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(($RANDOM % 255 + 1))} \
> > -$run_command >$tmp.out 2>&1)
> > +$run_command >$tmp.out 2>&1
> > +)
> >  fi
> >  sts=$?
> >  $timestamp && _timestamp
> > @@ -359,6 +370,8 @@ do
> >  fi
> >  fi
> >  
> > +rm -rf "$TEST_DIR_SEQ"
> > +
> >  fi
> >  
> >  # come here for each test, except when $showme is true
> > 
> 
> Seems OK to me, though I am not able to answer all doubts about exactly
> how this may effect the strange pipe/subshell arrangements that occur
> deeper in the bowels of the included files for launching QEMU and so
> on.. I suppose that might be related to the inclusion of those
> common.XYZ files?
> 
> Tested-by: John Snow 
> Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests attempt to leave intermediate files

[John Snow]

On 08/30/2017 06:35 PM, Eric Blake wrote:
> On 08/30/2017 05:28 PM, John Snow wrote:
> 
>> I'm a little iffy on this patch; I know that ./check can take care of
>> our temp files for us now, but because each python test is itself a
>> little mini-harness, I'm a little leery of moving the teardown to setup
>> and trying to pre-clean the confetti before the test begins.
>>
>> What's the benefit? We still have to clean up these files per-test, but
>> now it's slightly more error-prone and in a weird place.
>>
>> If we want to try to preserve the most-recent-failure-files, perhaps we
>> can define a setting in the python test-runner that allows us to
>> globally skip file cleanup.
> 
> On the other hand, since each test is a mini-harness, globally skipping
> cleanup will make a two-part test fail on the second because of garbage
> left behind by the first.
> 

subtext was to have per-subtest files.

> Patch 5 adds a comment with another possible solution: teach the python
> mini-harness to either clean all files in the directory, or to relocate
> the directory according to test name, so that each mini-test starts with
> a fresh location, and cleanup is then handled by the harness rather than
> spaghetti pre-cleanup.  But any solution is better than our current
> situation of nothing, so that's why I'm still okay with this patch as-is
> as offering more (even if not perfect) than before.
> 

I guess where I am unsure is really if this is better than what we
currently do, which is to (try) to clean up after each test as best as
we can. I don't see it as too different from trying to clean up before
each test.

It does give us the ability to leave behind a little detritus after a
failed run, but it's so imperfect that I wonder if it's worth shifting
this code around to change not much.

I won't die on this hill, it just strikes me a slightly less intuitive
use of the python unittest framework.

--js

Re: [Qemu-devel] [PATCH v3 5/5] qemu-iotests: add option to save temp files on error

[John Snow]

On 08/30/2017 12:52 PM, Jeff Cody wrote:
> Now that ./check takes care of cleaning up after each tests, it
> can also selectively not clean up.  Add option to leave all output from
> tests intact if that test encountered an error.
> 
> Note: this currently only works for bash tests, as the python tests
> still clean up after themselves manually.
> 
> Signed-off-by: Jeff Cody 
> ---
>  tests/qemu-iotests/check  | 10 +-
>  tests/qemu-iotests/common |  6 ++
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index f6ca85d..8a5fc0d 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -370,7 +370,15 @@ do
>  fi
>  fi
>  
> -rm -rf "$TEST_DIR_SEQ"
> +#TODO: There is some intial work to save intermediate files
> +#  in python tests, but it is imperfect.  Having each
> +#  test record its test name, and the tearDown function
> +#  just move intermediate images to a subdirectory with
> +#  the test name may prove more useful.
> +if [ "$save_on_err" != "true" ] || [ "$err" != "true" ]
> +then
> +rm -rf "$TEST_DIR_SEQ"
> +fi
>  
>  fi
>  
> diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
> index d34c11c..d08b233 100644
> --- a/tests/qemu-iotests/common
> +++ b/tests/qemu-iotests/common
> @@ -42,6 +42,7 @@ expunge=true
>  have_test_arg=false
>  randomize=false
>  cachemode=false
> +save_on_err=false
>  rm -f $tmp.list $tmp.tmp $tmp.sed
>  
>  export IMGFMT=raw
> @@ -172,6 +173,7 @@ other options
>  -T  output timestamps
>  -r  randomize test order
>  -c mode cache mode
> +-s  save test scratch directory on test failure
>  
>  testlist options
>  -g group[,group...]include tests from these groups
> @@ -349,6 +351,10 @@ testlist options
>  xgroup=true
>  xpand=false
>  ;;
> +-s)
> +save_on_err=true
> +xpand=false
> +;;
>  '[0-9][0-9][0-9] [0-9][0-9][0-9][0-9]')
>  echo "No tests?"
>  status=1
> 

This, however, is definitely awesome.

Tested-by: John Snow 
Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests attempt to leave intermediate files

[Eric Blake]

On 08/30/2017 05:28 PM, John Snow wrote:

> I'm a little iffy on this patch; I know that ./check can take care of
> our temp files for us now, but because each python test is itself a
> little mini-harness, I'm a little leery of moving the teardown to setup
> and trying to pre-clean the confetti before the test begins.
> 
> What's the benefit? We still have to clean up these files per-test, but
> now it's slightly more error-prone and in a weird place.
> 
> If we want to try to preserve the most-recent-failure-files, perhaps we
> can define a setting in the python test-runner that allows us to
> globally skip file cleanup.

On the other hand, since each test is a mini-harness, globally skipping
cleanup will make a two-part test fail on the second because of garbage
left behind by the first.

Patch 5 adds a comment with another possible solution: teach the python
mini-harness to either clean all files in the directory, or to relocate
the directory according to test name, so that each mini-test starts with
a fresh location, and cleanup is then handled by the harness rather than
spaghetti pre-cleanup.  But any solution is better than our current
situation of nothing, so that's why I'm still okay with this patch as-is
as offering more (even if not perfect) than before.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 4/5] qemu-iotests: make python tests attempt to leave intermediate files

[John Snow]

On 08/30/2017 02:33 PM, Eric Blake wrote:
> On 08/30/2017 11:52 AM, Jeff Cody wrote:
>> Now that 'check' will clean up after tests, try and make python
>> tests leave intermediate files so that they might be inspectable
>> on failure.
>>
>> This isn't perfect; the python unittest framework runs multiple
>> tests, even if previous tests failed.  So we need to make sure that
>> each test still begins with a "clean" slate, to prevent false
>> positives or tainted test runs.
>>
>> Rather than delete images in the unittest tearDown, invert this
>> and delete images to be used in that test at the beginning of the
>> setUp.  This is to make sure that the test run is not inadvertently
>> using file droppings from previous runs.  We must use 'blind_remove'
>> then for these, as the files might not exist yet, but we don't want
>> to throw an error for that.
>>
>> Signed-off-by: Jeff Cody 
>> ---
> 
>> +++ b/tests/qemu-iotests/030
>> @@ -21,7 +21,7 @@
>>  import time
>>  import os
>>  import iotests
>> -from iotests import qemu_img, qemu_io
>> +from iotests import qemu_img, qemu_io, blind_remove
>>  
>>  backing_img = os.path.join(iotests.test_dir, 'backing.img')
>>  mid_img = os.path.join(iotests.test_dir, 'mid.img')
>> @@ -31,6 +31,9 @@ class TestSingleDrive(iotests.QMPTestCase):
>>  image_len = 1 * 1024 * 1024 # MB
>>  
>>  def setUp(self):
>> +blind_remove(test_img)
>> +blind_remove(mid_img)
>> +blind_remove(backing_img)
> 
> Would it be any more pythonic to have support for:
> 
> blind_remove(test_img, mid_img, backing_img)
> 
> built into the previous patch?
> 

It should probably either take an iterable, or an arbitrary number of
arguments, or both, I dunno. I'm not a python.

>>  def tearDown(self):
>>  self.vm.shutdown()
>> -os.remove(self.test_img)
>> -os.remove(self.mid_img_abs)
>> -os.remove(self.backing_img_abs)
>> -try:
>> -os.rmdir(os.path.join(iotests.test_dir, self.dir1))
>> -os.rmdir(os.path.join(iotests.test_dir, self.dir3))
>> -os.rmdir(os.path.join(iotests.test_dir, self.dir2))
>> -except OSError as exception:
>> -if exception.errno != errno.EEXIST and exception.errno != 
>> errno.ENOTEMPTY:
>> -raise
> 
> The code removed here is using a syntax that differs from what you used
> in 3/5 when defining blind_remove; does that matter for 3/5?
> 
>> +++ b/tests/qemu-iotests/041
> 
>> +blind_remove(target_img)
>>  iotests.create_image(backing_img, self.image_len)
>>  qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % 
>> backing_img, test_img)
>>  self.vm = iotests.VM().add_drive(test_img, 
>> "node-name=top,backing.node-name=base")
>> @@ -49,12 +52,6 @@ class TestSingleDrive(iotests.QMPTestCase):
>>  
>>  def tearDown(self):
>>  self.vm.shutdown()
>> -os.remove(test_img)
>> -os.remove(backing_img)
>> -try:
>> -os.remove(target_img)
>> -except OSError:
>> -pass
> 
> You're changing failures other than ENOENT from ignored to explicit -
> nice little bug-fix along the way :)  I notice this pattern in multiple
> tests; is it worth mentioning in the commit message as intentional?
> 
>> @@ -797,6 +788,9 @@ class TestRepairQuorum(iotests.QMPTestCase):
>>  IMAGES = [ quorum_img1, quorum_img2, quorum_img3 ]
>>  
>>  def setUp(self):
>> +for i in self.IMAGES + [ quorum_repair_img, quorum_snapshot_file ]:
>> +blind_remove(i)
> 
> Again, would it be more pythonic if blind_remove() could take a list and
> automatically work on each element of the list, rather than having to
> make the caller iterate?
> 
>> +++ b/tests/qemu-iotests/057
>> @@ -23,7 +23,7 @@
>>  import time
>>  import os
>>  import iotests
>> -from iotests import qemu_img, qemu_io
>> +from iotests import qemu_img, qemu_io, blind_remove
>>  
>>  test_drv_base_name = 'drive'
>>  
>> @@ -36,6 +36,8 @@ class ImageSnapshotTestCase(iotests.QMPTestCase):
>>  
>>  def _setUp(self, test_img_base_name, image_num):
>>  self.vm = iotests.VM()
>> +for dev_expect in self.expect:
>> +blind_remove(dev_expect['image'])
> 
> Another place where python magic could make the caller nicer?
> 
>> +++ b/tests/qemu-iotests/118
> 
>> @@ -411,16 +411,16 @@ class TestFloppyInitiallyEmpty(TestInitiallyEmpty):
>>  
>>  class TestChangeReadOnly(ChangeBaseClass):
>>  def setUp(self):
>> -qemu_img('create', '-f', iotests.imgfmt, old_img, '1440k')
>> -qemu_img('create', '-f', iotests.imgfmt, new_img, '1440k')
>> -self.vm = iotests.VM()
>> -
>> -def tearDown(self):
>> -self.vm.shutdown()
>>  os.chmod(old_img, 0666)
>>  os.chmod(new_img, 0666)
>> -os.remove(old_img)
>> -os.remove(new_img)
>> +blind_remove(old_img)
>> +blind_remove(new_img)
>> +qemu_img('create', '-f', iot

Re: [Qemu-devel] [PATCH v3 3/5] qemu-iotests: add 'blind_remove' for python tests

[John Snow]

On 08/30/2017 02:13 PM, Eric Blake wrote:
> On 08/30/2017 11:52 AM, Jeff Cody wrote:
>> Add a function to attempt to 'blindly' remove a file, without
>> throwing an error if the file doesn't exist.
>>
>> Signed-off-by: Jeff Cody 
>> ---
>>  tests/qemu-iotests/iotests.py | 7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 7233983..a2088c7 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -57,6 +57,13 @@ qemu_default_machine = 
>> os.environ.get('QEMU_DEFAULT_MACHINE')
>>  socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper')
>>  debug = False
>>  
>> +def blind_remove(filename):
>> +try:
>> +os.remove(filename)
>> +except OSError, error:
> 
> I'm assuming this works for both python 2 and 3?
> 

Appears to be python2 specific syntax, actually. using "as error"
appears to work in both 2.7 and 3.whatever, and according to
http://python3porting.com/differences.html will work in 2.6 too.

>> +if error.errno != errno.ENOENT:
>> +raise
>> +
> 
> Weak, since I'm not the strongest at python, but you can add:
> Reviewed-by: Eric Blake 
> 

Re: [Qemu-devel] [PATCH v3 2/5] qemu-iotests: remove file cleanup from bash tests

[John Snow]

On 08/30/2017 12:52 PM, Jeff Cody wrote:
> All files for a given test are now self-contained in a subdirectory,
> and therefore the "./check" script can do all file-related cleanup
> without any help.
> 
> This removes file cleanups from the bash tests.  The only cleanup left
> is whatever is needed to kill any spawned processes; e.g. _cleanup_qemu.
> 
> Reviewed-by: Eric Blake 
> Signed-off-by: Jeff Cody 
> ---
>  tests/qemu-iotests/001 |  6 --
>  tests/qemu-iotests/002 |  6 --
>  tests/qemu-iotests/003 |  6 --
>  tests/qemu-iotests/004 |  6 --
>  tests/qemu-iotests/005 |  6 --
>  tests/qemu-iotests/007 |  7 ---
>  tests/qemu-iotests/008 |  6 --
>  tests/qemu-iotests/009 |  6 --
>  tests/qemu-iotests/010 |  6 --
>  tests/qemu-iotests/011 |  6 --
>  tests/qemu-iotests/012 |  6 --
>  tests/qemu-iotests/013 |  6 --
>  tests/qemu-iotests/014 |  6 --
>  tests/qemu-iotests/015 |  7 ---
>  tests/qemu-iotests/017 |  6 --
>  tests/qemu-iotests/018 |  6 --
>  tests/qemu-iotests/019 |  8 
>  tests/qemu-iotests/020 |  8 
>  tests/qemu-iotests/021 |  6 --
>  tests/qemu-iotests/022 |  6 --
>  tests/qemu-iotests/023 |  6 --
>  tests/qemu-iotests/024 |  8 
>  tests/qemu-iotests/025 |  6 --
>  tests/qemu-iotests/026 |  7 ---
>  tests/qemu-iotests/027 |  6 --
>  tests/qemu-iotests/028 |  8 
>  tests/qemu-iotests/029 |  7 ---
>  tests/qemu-iotests/031 |  6 --
>  tests/qemu-iotests/032 |  6 --
>  tests/qemu-iotests/033 |  6 --
>  tests/qemu-iotests/034 |  6 --
>  tests/qemu-iotests/035 |  6 --
>  tests/qemu-iotests/036 |  6 --
>  tests/qemu-iotests/037 |  6 --
>  tests/qemu-iotests/038 |  6 --
>  tests/qemu-iotests/039 |  6 --
>  tests/qemu-iotests/042 |  6 --
>  tests/qemu-iotests/043 |  7 ---
>  tests/qemu-iotests/046 |  6 --
>  tests/qemu-iotests/047 |  6 --
>  tests/qemu-iotests/048 |  8 
>  tests/qemu-iotests/048.out |  1 -
>  tests/qemu-iotests/049 |  6 --
>  tests/qemu-iotests/050 |  8 
>  tests/qemu-iotests/051 |  6 --
>  tests/qemu-iotests/052 |  6 --
>  tests/qemu-iotests/053 |  7 ---
>  tests/qemu-iotests/054 |  6 --
>  tests/qemu-iotests/058 |  8 +---
>  tests/qemu-iotests/059 |  7 ---
>  tests/qemu-iotests/060 |  6 --
>  tests/qemu-iotests/061 |  6 --
>  tests/qemu-iotests/062 |  6 --
>  tests/qemu-iotests/063 |  7 ---
>  tests/qemu-iotests/064 |  6 --
>  tests/qemu-iotests/066 |  6 --
>  tests/qemu-iotests/068 |  6 --
>  tests/qemu-iotests/069 |  6 --
>  tests/qemu-iotests/070 |  6 --
>  tests/qemu-iotests/071 |  6 --
>  tests/qemu-iotests/072 |  6 --
>  tests/qemu-iotests/073 |  6 --
>  tests/qemu-iotests/074 |  9 -
>  tests/qemu-iotests/074.out |  1 -
>  tests/qemu-iotests/075 |  6 --
>  tests/qemu-iotests/076 |  6 --
>  tests/qemu-iotests/077 |  6 --
>  tests/qemu-iotests/078 |  6 --
>  tests/qemu-iotests/079 |  6 --
>  tests/qemu-iotests/080 |  7 ---
>  tests/qemu-iotests/081 |  8 
>  tests/qemu-iotests/082 |  6 --
>  tests/qemu-iotests/084 |  6 --
>  tests/qemu-iotests/085 | 13 +
>  tests/qemu-iotests/086 |  6 --
>  tests/qemu-iotests/088 |  7 ---
>  tests/qemu-iotests/089 |  6 --
>  tests/qemu-iotests/090 |  6 --
>  tests/qemu-iotests/091 |  8 +---
>  tests/qemu-iotests/092 |  7 ---
>  tests/qemu-iotests/094 |  9 +
>  tests/qemu-iotests/095 |  8 +---
>  tests/qemu-iotests/097 |  7 ---
>  tests/qemu-iotests/098 |  7 ---
>  tests/qemu-iotests/099 |  6 --
>  tests/qemu-iotests/101 |  6 --
>  tests/qemu-iotests/102 |  7 +--
>  tests/qemu-iotests/103 |  6 --
>  tests/qemu-iotests/104 |  2 --
>  tests/qemu-iotests/105 |  6 --
>  tests/qemu-iotests/106 |  6 --
>  tests/qemu-iotests/107 |  6 --
>  tests/qemu-iotests/108 |  6 --
>  tests/qemu-iotests/109 |  8 +---
>  tests/qemu-iotests/110 |  6 --
>  tests/qemu-iotests/111 |  6 --
>  tests/qemu-iotests/112 |  6 --
>  tests/qemu-iotests/113 |  6 --
>  tests/qemu-iotests/114 |  6 --
>  tests/qemu-iotests/115 |  6 --
>  tests/qemu-iotests/116 |  6 --
>  tests/qemu-iotests/117 |  7 +--
>  tests/qemu-iotests/119 |  6 --
>  tests/qemu-iotests/120 |  6 --
>  tests/qemu-iotests/121 |  6 --
>  tests/qemu-iotests/122 |  7 ---
>  tests/qemu-iotests/123 |  7 ---
>  tests/qemu-iotests/125 |  6 --
>  test

Re: [Qemu-devel] [PATCH v3 1/5] qemu-iotests: set TEST_DIR to a unique dir for each test

[John Snow]

On 08/30/2017 12:52 PM, Jeff Cody wrote:
> Right now, all qemu-iotests output data into the same scratch directory,
> and so each test needs to be responsible for cleaning up its own files.
> 
> Have each test use 'scratch/$seq' as its temp directory, so the check
> script can do simple cleanup of removing the whole temporary directory.
> 
> Reviewed-by: Eric Blake 
> Signed-off-by: Jeff Cody 
> ---
>  tests/qemu-iotests/check | 21 +
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index d504b6e..f6ca85d 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -243,6 +243,7 @@ seq="check"
>  
>  for seq in $list
>  do
> +TEST_DIR_SEQ=$TEST_DIR/$seq
>  err=false
>  printf %s "$seq"
>  if [ -n "$TESTS_REMAINING_LOG" ] ; then
> @@ -289,13 +290,23 @@ do
>  fi
>  export OUTPUT_DIR=$PWD
>  if $debug; then
> -(cd "$source_iotests";
> +(
> +export TEST_DIR=$TEST_DIR_SEQ
> +. "$source_iotests/common.config"
> +. "$source_iotests/common.rc"

What purpose do these serve?

> +cd "$source_iotests" &&
>  MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(($RANDOM % 255 + 1))} \
> -$run_command -d 2>&1 | tee $tmp.out)
> +$run_command -d 2>&1 | tee $tmp.out
> +)
>  else
> -(cd "$source_iotests";
> +(
> +export TEST_DIR=$TEST_DIR_SEQ
> +. "$source_iotests/common.config"
> +. "$source_iotests/common.rc"
> + cd "$source_iotests" &&
>  MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(($RANDOM % 255 + 1))} \
> -$run_command >$tmp.out 2>&1)
> +$run_command >$tmp.out 2>&1
> +)
>  fi
>  sts=$?
>  $timestamp && _timestamp
> @@ -359,6 +370,8 @@ do
>  fi
>  fi
>  
> +rm -rf "$TEST_DIR_SEQ"
> +
>  fi
>  
>  # come here for each test, except when $showme is true
> 

Seems OK to me, though I am not able to answer all doubts about exactly
how this may effect the strange pipe/subshell arrangements that occur
deeper in the bowels of the included files for launching QEMU and so
on.. I suppose that might be related to the inclusion of those
common.XYZ files?

Tested-by: John Snow 
Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH v1 1/1] target/xtensa: Use the pre-defined MEMTXATTRS_UNSPECIFIED macro

[Peter Maydell]

On 30 August 2017 at 19:02, Alistair Francis
 wrote:
> Instead of using the hardcoded (MemTxAttrs){0} for no memory attributes
> let's use the already defined MEMTXATTRS_UNSPECIFIED macro instead.
>
> Signed-off-by: Alistair Francis 
> ---
>
>  target/xtensa/op_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/xtensa/op_helper.c b/target/xtensa/op_helper.c
> index 519fbeddd6..3d990c0caa 100644
> --- a/target/xtensa/op_helper.c
> +++ b/target/xtensa/op_helper.c
> @@ -1025,11 +1025,11 @@ void HELPER(ule_s)(CPUXtensaState *env, uint32_t br, 
> float32 a, float32 b)
>  uint32_t HELPER(rer)(CPUXtensaState *env, uint32_t addr)
>  {
>  return address_space_ldl(env->address_space_er, addr,
> - (MemTxAttrs){0}, NULL);
> + MEMTXATTRS_UNSPECIFIED, NULL);
>  }
>
>  void HELPER(wer)(CPUXtensaState *env, uint32_t data, uint32_t addr)
>  {
>  address_space_stl(env->address_space_er, addr, data,
> -  (MemTxAttrs){0}, NULL);
> +  MEMTXATTRS_UNSPECIFIED, NULL);
>  }

Might be worth noting in the commit that this is technically
a change of behaviour, because MEMTXATTRS_UNSPECIFIED
sets the 'unspecified' field to 1 whereas {0} doesn't.
I don't think anything actually checks that field, though.

thanks
-- PMM

[Qemu-devel] [PATCH v2 14/17] MAINTAINERS: add missing entry for Generic Loader

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a48f633cad..0b77590dc8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1164,6 +1164,7 @@ M: Alistair Francis 
 S: Maintained
 F: hw/core/generic-loader.c
 F: include/hw/core/generic-loader.h
+F: docs/generic-loader.txt
 
 CHRP NVRAM
 M: Thomas Huth 
-- 
2.14.1

[Qemu-devel] [PATCH v2 13/17] MAINTAINERS: add missing AIO entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Acked-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 20d65dca73..a48f633cad 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1225,6 +1225,7 @@ F: util/aio-*.c
 F: block/io.c
 F: migration/block*
 F: include/block/aio.h
+F: scripts/qemugdb/aio.py
 T: git git://github.com/stefanha/qemu.git block
 
 Block Jobs
-- 
2.14.1

[Qemu-devel] [PATCH v2 15/17] MAINTAINERS: add missing Cryptography entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0b77590dc8..7a0c00550e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1526,6 +1526,7 @@ S: Maintained
 F: crypto/
 F: include/crypto/
 F: tests/test-crypto-*
+F: tests/benchmark-crypto-*
 F: qemu.sasl
 
 Coroutines
-- 
2.14.1

[Qemu-devel] [PATCH v2 12/17] MAINTAINERS: add missing megasas test entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index fa74b7254b..20d65dca73 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1118,6 +1118,7 @@ L: qemu-bl...@nongnu.org
 S: Supported
 F: hw/scsi/megasas.c
 F: hw/scsi/mfi.h
+F: tests/megasas-test.c
 
 Network packet abstractions
 M: Dmitry Fleytman 
-- 
2.14.1

[Qemu-devel] [PATCH v2 07/17] MAINTAINERS: add missing qcow2 entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Acked-by: Kevin Wolf 
Reviewed-by: Stefan Hajnoczi 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f4e07173c8..eb20365fbb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1841,6 +1841,7 @@ M: Max Reitz 
 L: qemu-bl...@nongnu.org
 S: Supported
 F: block/qcow2*
+F: docs/interop/qcow2.txt
 
 qcow
 M: Kevin Wolf 
-- 
2.14.1

[Qemu-devel] [PATCH v2 17/17] MAINTAINERS: update docs/interop/ entries

[Philippe Mathieu-Daudé]

moved in commit 7746cf8aab68

Signed-off-by: Philippe Mathieu-Daudé 
Acked-by: Fam Zheng 
Acked-by: John Snow 
---
 MAINTAINERS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 452ccd71b4..833a7a6778 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1259,7 +1259,7 @@ F: block/dirty-bitmap.c
 F: include/qemu/hbitmap.h
 F: include/block/dirty-bitmap.h
 F: tests/test-hbitmap.c
-F: docs/bitmaps.md
+F: docs/interop/bitmaps.rst
 T: git git://github.com/famz/qemu.git bitmaps
 T: git git://github.com/jnsnow/qemu.git bitmaps
 
@@ -1828,7 +1828,7 @@ M: Denis V. Lunev 
 L: qemu-bl...@nongnu.org
 S: Supported
 F: block/parallels.c
-F: docs/specs/parallels.txt
+F: docs/interop/parallels.txt
 
 qed
 M: Stefan Hajnoczi 
-- 
2.14.1

[Qemu-devel] [PATCH v2 05/17] MAINTAINERS: add missing VMWare entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Thomas Huth 
Reviewed-by: Dmitry Fleytman 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 223591c231..aa185c1edd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1126,6 +1126,7 @@ M: Dmitry Fleytman 
 S: Maintained
 F: hw/net/vmxnet*
 F: hw/scsi/vmw_pvscsi*
+F: tests/vmxnet3-test.c
 
 Rocker
 M: Jiri Pirko 
-- 
2.14.1

[Qemu-devel] [PATCH v2 16/17] MAINTAINERS: update docs/devel/ entries

[Philippe Mathieu-Daudé]

moved in commit ac06724a7158

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Eric Blake 
---
 MAINTAINERS | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7a0c00550e..452ccd71b4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1414,7 +1414,7 @@ F: tests/test-qapi-*.c
 F: tests/test-qmp-*.c
 F: tests/test-visitor-serialization.c
 F: scripts/qapi*
-F: docs/qapi*
+F: docs/devel/qapi*
 T: git git://repo.or.cz/qemu/armbru.git qapi-next
 
 QAPI Schema
@@ -1466,7 +1466,7 @@ M: Markus Armbruster 
 S: Supported
 F: qmp.c
 F: monitor.c
-F: docs/*qmp-*
+F: docs/devel/*qmp-*
 F: scripts/qmp/
 F: tests/qmp-test.c
 T: git git://repo.or.cz/qemu/armbru.git qapi-next
@@ -1497,7 +1497,7 @@ S: Maintained
 F: trace/
 F: scripts/tracetool.py
 F: scripts/tracetool/
-F: docs/tracing.txt
+F: docs/devel/tracing.txt
 T: git git://github.com/stefanha/qemu.git tracing
 
 Checkpatch
@@ -1512,7 +1512,7 @@ F: include/migration/
 F: migration/
 F: scripts/vmstate-static-checker.py
 F: tests/vmstate-static-checker-data/
-F: docs/migration.txt
+F: docs/devel/migration.txt
 
 Seccomp
 M: Eduardo Otubo 
@@ -1914,5 +1914,5 @@ Documentation
 Build system architecture
 M: Daniel P. Berrange 
 S: Odd Fixes
-F: docs/build-system.txt
+F: docs/devel/build-system.txt
 
-- 
2.14.1

[Qemu-devel] [PATCH v2 11/17] MAINTAINERS: add missing entries for throttling infra

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Stefan Hajnoczi 
---
 MAINTAINERS | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0a3a50aa25..fa74b7254b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1558,8 +1558,10 @@ M: Alberto Garcia 
 S: Supported
 F: block/throttle-groups.c
 F: include/block/throttle-groups.h
-F: include/qemu/throttle.h
+F: include/qemu/throttle*.h
 F: util/throttle.c
+F: docs/throttle.txt
+F: tests/test-throttle.c
 L: qemu-bl...@nongnu.org
 
 UUID
-- 
2.14.1

[Qemu-devel] [PATCH v2 10/17] MAINTAINERS: add missing SSI entries

[Philippe Mathieu-Daudé]

Alistair Francis volunteered :)

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index bbe1191883..0a3a50aa25 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -986,10 +986,13 @@ F: hw/scsi/lsi53c895a.c
 
 SSI
 M: Peter Crosthwaite 
+M: Alistair Francis 
 S: Maintained
 F: hw/ssi/*
 F: hw/block/m25p80.c
+F: include/hw/ssi/ssi.h
 X: hw/ssi/xilinx_*
+F: tests/m25p80-test.c
 
 Xilinx SPI
 M: Alistair Francis 
-- 
2.14.1

[Qemu-devel] [PATCH v2 08/17] MAINTAINERS: add missing USB entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Thomas Huth 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index eb20365fbb..dd5e2c2b6b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1004,6 +1004,7 @@ F: docs/usb2.txt
 F: docs/usb-storage.txt
 F: include/hw/usb.h
 F: include/hw/usb/
+F: default-configs/usb.mak
 
 USB (serial adapter)
 M: Gerd Hoffmann 
-- 
2.14.1

[Qemu-devel] [PATCH v2 02/17] MAINTAINERS: add missing Versatile PB entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b363e1b9c9..5b7891addc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -515,6 +515,7 @@ M: Peter Maydell 
 L: qemu-...@nongnu.org
 S: Maintained
 F: hw/*/versatile*
+F: hw/misc/arm_sysctl.c
 
 Xilinx Zynq
 M: Edgar E. Iglesias 
-- 
2.14.1

[Qemu-devel] [PATCH v2 09/17] MAINTAINERS: add missing PCI entries

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index dd5e2c2b6b..bbe1191883 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -928,6 +928,8 @@ F: include/hw/pci/*
 F: hw/misc/pci-testdev.c
 F: hw/pci/*
 F: hw/pci-bridge/*
+F: docs/pci*
+F: docs/specs/*pci*
 
 ACPI/SMBIOS
 M: Michael S. Tsirkin 
-- 
2.14.1

[Qemu-devel] [PATCH v2 06/17] MAINTAINERS: add missing Guest Agent entries

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 4 
 1 file changed, 4 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index aa185c1edd..f4e07173c8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1434,6 +1434,10 @@ QEMU Guest Agent
 M: Michael Roth 
 S: Maintained
 F: qga/
+F: qemu-ga.texi
+F: scripts/qemu-guest-agent/
+F: tests/test-qga.c
+F: docs/interop/qemu-ga-ref.texi
 T: git git://github.com/mdroth/qemu.git qga
 
 QOM
-- 
2.14.1

[Qemu-devel] [PATCH v2 03/17] MAINTAINERS: add missing STM32 entry

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Thomas Huth 
Reviewed-by: Alistair Francis 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5b7891addc..8c75ce477a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -552,6 +552,7 @@ F: hw/char/stm32f2xx_usart.c
 F: hw/timer/stm32f2xx_timer.c
 F: hw/adc/*
 F: hw/ssi/stm32f2xx_spi.c
+F: include/hw/*/stm32*.h
 
 Netduino 2
 M: Alistair Francis 
-- 
2.14.1

[Qemu-devel] [PATCH v2 04/17] MAINTAINERS: add missing entry for vhost

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8c75ce477a..223591c231 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1029,6 +1029,7 @@ vhost
 M: Michael S. Tsirkin 
 S: Supported
 F: hw/*/*vhost*
+F: docs/interop/vhost-user.txt
 
 virtio
 M: Michael S. Tsirkin 
-- 
2.14.1

[Qemu-devel] [PATCH v2 00/17] add missing entries in MAINTAINERS

[Philippe Mathieu-Daudé]

Hi,

I tried to have a more helpful ./scripts/get_maintainer.pl output, filling
missing entries in MAINTAINERS.

Regards,

Phil.

v2:
- add R-b & A-b
- clean ARM entries (Thomas Huth)
- moved files: comment since which commit (Eric Blake)
- drop inconsistent patches (default-configs.mak related to hw/machine, no CPU)
- drop unhappy patches (include/standard-headers/linux/*)
- drop unreplied patches

Philippe Mathieu-Daudé (17):
  MAINTAINERS: add missing ARM entries
  MAINTAINERS: add missing Versatile PB entry
  MAINTAINERS: add missing STM32 entry
  MAINTAINERS: add missing entry for vhost
  MAINTAINERS: add missing VMWare entry
  MAINTAINERS: add missing Guest Agent entries
  MAINTAINERS: add missing qcow2 entry
  MAINTAINERS: add missing USB entry
  MAINTAINERS: add missing PCI entries
  MAINTAINERS: add missing SSI entries
  MAINTAINERS: add missing entries for throttling infra
  MAINTAINERS: add missing megasas test entry
  MAINTAINERS: add missing AIO entry
  MAINTAINERS: add missing entry for Generic Loader
  MAINTAINERS: add missing Cryptography entry
  MAINTAINERS: update docs/devel/ entries
  MAINTAINERS: update docs/interop/ entries

 MAINTAINERS | 44 ++--
 1 file changed, 34 insertions(+), 10 deletions(-)

-- 
2.14.1

[Qemu-devel] [PATCH v2 01/17] MAINTAINERS: add missing ARM entries

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index ccee28b12d..b363e1b9c9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -380,6 +380,7 @@ M: Peter Maydell 
 L: qemu-...@nongnu.org
 S: Maintained
 F: hw/char/pl011.c
+F: include/hw/char/pl011.h
 F: hw/display/pl110*
 F: hw/dma/pl080.c
 F: hw/dma/pl330.c
@@ -403,13 +404,15 @@ F: hw/intc/gic_internal.h
 F: hw/misc/a9scu.c
 F: hw/misc/arm11scu.c
 F: hw/timer/a9gtimer*
-F: hw/timer/arm_*
-F: include/hw/arm/arm.h
+F: hw/timer/arm*
+F: include/hw/arm/arm*.h
 F: include/hw/intc/arm*
 F: include/hw/misc/a9scu.h
 F: include/hw/misc/arm11scu.h
 F: include/hw/timer/a9gtimer.h
 F: include/hw/timer/arm_mptimer.h
+F: include/hw/timer/armv7m_systick.h
+F: tests/test-arm-mptimer.c
 
 Exynos
 M: Igor Mitsyanko 
-- 
2.14.1

Re: [Qemu-devel] [PATCH 3/6] tests: Enable the drive_del test also on s390x

[Cleber Rosa]

On 08/17/2017 02:25 AM, Thomas Huth wrote:
> By using the "virtio-xxx" device name aliases instead of the
> "virtio-xxx-pci" names, we can use this test on s390x, too,
> to check that adding and deleting also works fine with the
> virtio-ccw bus.
> 
> Signed-off-by: Thomas Huth 
> ---
>  tests/Makefile.include |  1 +
>  tests/drive_del-test.c | 13 +++--
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 0bb18b3..ff2a551 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -363,6 +363,7 @@ check-qtest-s390x-y = tests/boot-serial-test$(EXESUF)
>  check-qtest-s390x-$(CONFIG_SLIRP) += tests/test-netfilter$(EXESUF)
>  check-qtest-s390x-$(CONFIG_POSIX) += tests/test-filter-mirror$(EXESUF)
>  check-qtest-s390x-$(CONFIG_POSIX) += tests/test-filter-redirector$(EXESUF)
> +check-qtest-s390x-y += tests/drive_del-test$(EXESUF)
>  
>  check-qtest-generic-y += tests/qom-test$(EXESUF)
>  check-qtest-generic-y += tests/test-hmp$(EXESUF)
> diff --git a/tests/drive_del-test.c b/tests/drive_del-test.c
> index 2175139..efceb31 100644
> --- a/tests/drive_del-test.c
> +++ b/tests/drive_del-test.c
> @@ -65,12 +65,12 @@ static void test_after_failed_device_add(void)
>  
>  qtest_start("-drive if=none,id=drive0");
>  
> -/* Make device_add fail.  If this leaks the virtio-blk-pci device then a
> +/* Make device_add fail.  If this leaks the virtio-blk device then a
>   * reference to drive0 will also be held (via qdev properties).
>   */
>  response = qmp("{'execute': 'device_add',"
> " 'arguments': {"
> -   "   'driver': 'virtio-blk-pci',"
> +   "   'driver': 'virtio-blk',"
> "   'drive': 'drive0'"
> "}}");
>  g_assert(response);
> @@ -82,7 +82,7 @@ static void test_after_failed_device_add(void)
>  drive_del();
>  
>  /* Try to re-add the drive.  This fails with duplicate IDs if a leaked
> - * virtio-blk-pci exists that holds a reference to the old drive0.
> + * virtio-blk exists that holds a reference to the old drive0.
>   */
>  drive_add();
>  
> @@ -93,7 +93,7 @@ static void test_drive_del_device_del(void)
>  {
>  /* Start with a drive used by a device that unplugs instantaneously */
>  qtest_start("-drive if=none,id=drive0,file=null-co://,format=raw"
> -" -device virtio-scsi-pci"
> +" -device virtio-scsi"
>  " -device scsi-hd,drive=drive0,id=dev0");
>  
>  /*
> @@ -114,9 +114,10 @@ int main(int argc, char **argv)
>  
>  qtest_add_func("/drive_del/without-dev", test_drive_without_dev);
>  
> -/* TODO I guess any arch with PCI would do */
> +/* TODO I guess any arch with a hot-pluggable virtio bus would do */
>  if (!strcmp(arch, "i386") || !strcmp(arch, "x86_64") ||
> -!strcmp(arch, "ppc") || !strcmp(arch, "ppc64")) {
> +!strcmp(arch, "ppc") || !strcmp(arch, "ppc64") ||
> +!strcmp(arch, "s390x")) {
>  qtest_add_func("/drive_del/after_failed_device_add",
> test_after_failed_device_add);
>  qtest_add_func("/blockdev/drive_del_device_del",
> 

I honestly can't comment on the device to be used on this test (as
others have commented).  Putting that aside, and given the fact  that I
went through the lengths of testing it on the s390x target:

Tested-by: Cleber Rosa 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-block] Persistent bitmaps for non-qcow2 formats

[John Snow]

On 08/30/2017 09:45 AM, Daniel P. Berrange wrote:
> On Wed, Aug 30, 2017 at 02:36:11PM +0100, Stefan Hajnoczi wrote:
>> On Tue, Aug 22, 2017 at 03:07:04PM -0400, John Snow wrote:
>>> (3) Add either a new flag that turns qcow2's backing file into a full
>>> R/W backing file, or add a new extension to qcow2 entirely (bypassing
>>> the traditional backing file mechanism to avoid confusion for older
>>> tooling) that adds a new read-write backing file field.
>>>
>>> This RW backing file field will be used for all reads AND writes; the
>>> qcow2 in question becomes a metadata container on top of the BDS chain.
>>> We can re-use Vladimir's bitmap persistence extension to save bitmaps in
>>> a qcow2 shell.
>>>
>>> The qcow2 becomes effectively a metadata cache for a new (essentially)
>>> filter node that handles features such as bitmaps. This could also be
>>> used to provide allocation map data for RAW files and other goodies down
>>> the road.
>>>
>>> Hopefully this achieves our desire to not create new formats AND our
>>> desire to concentrate features (and debugging, testing, etc) into qcow2,
>>> while allowing users to "have bitmaps with raw files."
>>>
>>> Of course, in this scenario, users now have two files: a qcow2 wrapper
>>> and the actual raw file in question; but regardless of how we were going
>>> to solve this, a raw file necessitates an external file of some sort,
>>> else we give up the idea that it was a raw file.
>>
>> There is some complexity here for management tools:
>>
>> If the underlying image is resized, who resizes the qcow2 and how do
>> they know to do it?
>>
>> If QEMU's resize/truncate command it used, does first try to resize the
>> underlying image and then resize the qcow2?  This is probably the sanest
>> approach.
>>
>> If the underlying image is moved to a new location, does the qcow2 file
>> need to be modified and who does that?
>>
>> Management tools need to figure out how to represent manage this extra
>> qcow2 file.  The easiest solution is to punt it to the user and treat it
>> as part of a backing file chain.  If the management tool wants to
>> automatically manage the qcow2 so the user just specifies the underlying
>> image and enables the persistent bitmap checkbox, then it becomes more
>> complicated.
> 
> Indeed, I don't think it is practical to have libvirt / QEMU automagically
> create a qcow2 overlay on disk. Something has to decide where this would
> be stored. You might say just put it alongside the raw file, but it might
> not be a local file at all, it could be a NBD, or RBD raw "file". So do
> we create  local qcow2 file, or store a qcow2 file inside another RBD
> volume to hold the persistent bitmap. This kind of decision needs to be
> made by the mgmt app since only it knows about its storage mgmt model.

Oh, you mean to say mgmt app like VMM or something even above libvirt,
yes? Who currently makes the decision for where snapshot files and the
like goes, does libvirt not decide that, but the app using it?

> At this point you might as well just let the mgmt app take care of it
> all and not try to do anything magical with qcow2 overlays in libvirt/QEMU
> 

Might be the sanest, yes -- but say I don't offer an "automagic" way to
add a bitmap to a raw file to a running QEMU instance but rather offer a
way to qcow2ify an existing node:

We could create a wrapper:

qemu-img create -f qcow2 -o wrapper wrapper.qcow2

Then add a node:

blockdev_add driver=qcow2 node-name=foo file.driver=file
file.filename=wrapper.qcow2

(The size of the drive could perhaps remain as zero temporarily here)

Then some magic to make it the active target of whatever blockbackend
was using the old image, perhaps?:

blockdev_magic target=virtioblk0 node-name=foo

At this point, virtioblk0 is now backed by our new magic qcow2 node
which provides bitmap features for whatever kind of backing storage
virtioblk0 had, and nothing automagic happened -- we passed explicit
file references.

Any magic that we wish to provide can happen in libvirt or above.

--js

Re: [Qemu-devel] Persistent bitmaps for non-qcow2 formats

[John Snow]

On 08/30/2017 08:58 AM, Yaniv Lavi (Dary) wrote:
> 
> 
> We had no reason to switch to anything else so far and I'm sure this
> option was not available when we started supporting raw format. 
>  
> 

Yeah, they don't exist yet...! I've looped you in to see if the proposal
being discussed would alleviate the need for bitmaps for "other formats"
if we can offer a "raw mode qcow2."

At the moment I am going to still try to add bitmaps to other formats
(through the use of a qcow2 wrapper) but it sounds like a raw-layout
qcow2 might provide some benefits too.

-js

Re: [Qemu-devel] [PATCH 02/47] MAINTAINERS: add missing ARM entries

[Philippe Mathieu-Daudé]

On 07/28/2017 03:55 AM, Thomas Huth wrote:

On 28.07.2017 07:35, Philippe Mathieu-Daudé wrote:

Signed-off-by: Philippe Mathieu-Daudé 
---
  MAINTAINERS | 8 
  1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 972118e70b..795f89f709 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -120,6 +120,8 @@ F: include/hw/cpu/a*mpcore.h
  F: disas/arm.c
  F: disas/arm-a64.cc
  F: disas/libvixl/
+F: default-configs/arm-softmmu.mak
+F: default-configs/aarch64-softmmu.mak


You've added this to the TCG CPU core section, but strictly speaking
these files are also used for the machine emulation in general (and also
for KVM). So not sure whether this is a good fit here ... up to Peter to
decide.


You are right.




  CRIS
  M: Edgar E. Iglesias 
@@ -380,6 +382,7 @@ M: Peter Maydell 
  L: qemu-...@nongnu.org
  S: Maintained
  F: hw/char/pl011.c
+F: include/hw/char/pl011.h
  F: hw/display/pl110*
  F: hw/dma/pl080.c
  F: hw/dma/pl330.c
@@ -402,14 +405,19 @@ F: hw/intc/arm*
  F: hw/intc/gic_internal.h
  F: hw/misc/a9scu.c
  F: hw/misc/arm11scu.c
+F: hw/misc/arm_sysctl.c


According to a comment in that file, it is about RealView/Versatile
boards instead, so this is the wrong section here?


Wrong section indeed.




  F: hw/timer/a9gtimer*
  F: hw/timer/arm_*
+F: hw/timer/armv7m_systick.c


How about rather removing the underscore in the previous wildcard entry?


I also wondered, because I'm a slowly working branch where I try to boot 
some Marvell SoC, and I named the timer "armada_timer.c" following the 
Linux device-tree naming:


https://www.kernel.org/doc/Documentation/devicetree/bindings/timer/marvell%2Carmada-370-xp-timer.txt

But if I ever finish it I can add an exclude entry, so I'll follow your 
advice.





  F: include/hw/arm/arm.h
+F: include/hw/arm/armv7m*.h
  F: include/hw/intc/arm*
  F: include/hw/misc/a9scu.h
  F: include/hw/misc/arm11scu.h
  F: include/hw/timer/a9gtimer.h
  F: include/hw/timer/arm_mptimer.h
+F: include/hw/timer/armv7m_systick.h
+F: tests/test-arm-mptimer.c
  
  Exynos

  M: Igor Mitsyanko 



  Thomas

[Qemu-devel] [PATCH v6 17/18] dirty-bitmap: Switch bdrv_set_dirty() to bytes

[Eric Blake]

Both callers already had bytes available, but were scaling to
sectors.  Move the scaling to internal code.  In the case of
bdrv_aligned_pwritev(), we are now passing the exact offset
rather than a rounded sector-aligned value, but that's okay
as long as dirty bitmap widens start/bytes to granularity
boundaries.

Signed-off-by: Eric Blake 
Reviewed-by: John Snow 

---
v4: only context changes
v3: rebase to lock context changes, R-b kept
v2: no change
---
 include/block/block_int.h | 2 +-
 block/io.c| 6 ++
 block/dirty-bitmap.c  | 7 ---
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 7571c0aaaf..4d01dc3fa6 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -969,7 +969,7 @@ void blk_dev_eject_request(BlockBackend *blk, bool force);
 bool blk_dev_is_tray_open(BlockBackend *blk);
 bool blk_dev_is_medium_locked(BlockBackend *blk);

-void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int64_t nr_sect);
+void bdrv_set_dirty(BlockDriverState *bs, int64_t offset, int64_t bytes);
 bool bdrv_requests_pending(BlockDriverState *bs);

 void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out);
diff --git a/block/io.c b/block/io.c
index 26003814eb..926beebc8f 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1334,7 +1334,6 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild 
*child,
 bool waited;
 int ret;

-int64_t start_sector = offset >> BDRV_SECTOR_BITS;
 int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);
 uint64_t bytes_remaining = bytes;
 int max_transfer;
@@ -1409,7 +1408,7 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild 
*child,
 bdrv_debug_event(bs, BLKDBG_PWRITEV_DONE);

 atomic_inc(&bs->write_gen);
-bdrv_set_dirty(bs, start_sector, end_sector - start_sector);
+bdrv_set_dirty(bs, offset, bytes);

 stat64_max(&bs->wr_highest_offset, offset + bytes);

@@ -2412,8 +2411,7 @@ int coroutine_fn bdrv_co_pdiscard(BlockDriverState *bs, 
int64_t offset,
 ret = 0;
 out:
 atomic_inc(&bs->write_gen);
-bdrv_set_dirty(bs, req.offset >> BDRV_SECTOR_BITS,
-   req.bytes >> BDRV_SECTOR_BITS);
+bdrv_set_dirty(bs, req.offset, req.bytes);
 tracked_request_end(&req);
 bdrv_dec_in_flight(bs);
 return ret;
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 9821225523..b54eed46e4 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -629,10 +629,10 @@ void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap 
*bitmap)
 hbitmap_deserialize_finish(bitmap->bitmap);
 }

-void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
-int64_t nr_sectors)
+void bdrv_set_dirty(BlockDriverState *bs, int64_t offset, int64_t bytes)
 {
 BdrvDirtyBitmap *bitmap;
+int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);

 if (QLIST_EMPTY(&bs->dirty_bitmaps)) {
 return;
@@ -644,7 +644,8 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t 
cur_sector,
 continue;
 }
 assert(!bdrv_dirty_bitmap_readonly(bitmap));
-hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
+hbitmap_set(bitmap->bitmap, offset >> BDRV_SECTOR_BITS,
+end_sector - (offset >> BDRV_SECTOR_BITS));
 }
 bdrv_dirty_bitmaps_unlock(bs);
 }
-- 
2.13.5