Re: [Qemu-devel] [PATCH 16/26] qapi: add conditions to VNC type/commands/events on the schema

2017-08-17 Thread Markus Armbruster
Copying our resident VNC maintainer^Wodd fixer Gerd.

Also copying Dan for QCryptoCipherAlgorithm.

Gerd, Dan, this patch is about making VNC support visible in
query-qmp-schema, by having the QAPI generators generate suitable
ifdeffery.  Bonus: no need for QMP command stubs for
!defined(CONFIG_VNC).

Marc-André Lureau  writes:

> Add #if defined(CONFIG_VNC) in generated code, and adjust the
> qmp/hmp code accordingly.
>
> Signed-off-by: Marc-André Lureau 
> ---
>  qapi-schema.json | 34 ++
>  qapi/event.json  |  9 ++---
>  hmp.c| 14 +-
>  qmp.c| 30 --
>  4 files changed, 45 insertions(+), 42 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 9c6c3e1a53..829c66f9eb 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1660,7 +1660,8 @@
>'data': { 'host': 'str',
>  'service': 'str',
>  'family': 'NetworkAddressFamily',
> -'websocket': 'bool' } }
> +'websocket': 'bool' },
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @VncServerInfo:
> @@ -1674,7 +1675,8 @@
>  ##
>  { 'struct': 'VncServerInfo',
>'base': 'VncBasicInfo',
> -  'data': { '*auth': 'str' } }
> +  'data': { '*auth': 'str' },
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @VncClientInfo:
> @@ -1691,7 +1693,8 @@
>  ##
>  { 'struct': 'VncClientInfo',
>'base': 'VncBasicInfo',
> -  'data': { '*x509_dname': 'str', '*sasl_username': 'str' } }
> +  'data': { '*x509_dname': 'str', '*sasl_username': 'str' },
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @VncInfo:
> @@ -1732,7 +1735,8 @@
>  { 'struct': 'VncInfo',
>'data': {'enabled': 'bool', '*host': 'str',
> '*family': 'NetworkAddressFamily',
> -   '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']} 
> }
> +   '*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']},
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @VncPrimaryAuth:
> @@ -1743,7 +1747,8 @@
>  ##
>  { 'enum': 'VncPrimaryAuth',
>'data': [ 'none', 'vnc', 'ra2', 'ra2ne', 'tight', 'ultra',
> -'tls', 'vencrypt', 'sasl' ] }
> +'tls', 'vencrypt', 'sasl' ],
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @VncVencryptSubAuth:
> @@ -1757,7 +1762,8 @@
>  'tls-none',  'x509-none',
>  'tls-vnc',   'x509-vnc',
>  'tls-plain', 'x509-plain',
> -'tls-sasl',  'x509-sasl' ] }
> +'tls-sasl',  'x509-sasl' ],
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  
>  ##
> @@ -1775,7 +1781,8 @@
>  { 'struct': 'VncServerInfo2',
>'base': 'VncBasicInfo',
>'data': { 'auth'  : 'VncPrimaryAuth',
> -'*vencrypt' : 'VncVencryptSubAuth' } }
> +'*vencrypt' : 'VncVencryptSubAuth' },
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  
>  ##
> @@ -1808,7 +1815,8 @@
>  'clients'   : ['VncClientInfo'],
>  'auth'  : 'VncPrimaryAuth',
>  '*vencrypt' : 'VncVencryptSubAuth',
> -'*display'  : 'str' } }
> +'*display'  : 'str' },
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @query-vnc:
> @@ -1839,7 +1847,8 @@
>  #}
>  #
>  ##
> -{ 'command': 'query-vnc', 'returns': 'VncInfo' }
> +{ 'command': 'query-vnc', 'returns': 'VncInfo',
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @query-vnc-servers:
> @@ -1850,7 +1859,8 @@
>  #
>  # Since: 2.3
>  ##
> -{ 'command': 'query-vnc-servers', 'returns': ['VncInfo2'] }
> +{ 'command': 'query-vnc-servers', 'returns': ['VncInfo2'],
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @SpiceBasicInfo:
> @@ -3077,8 +3087,8 @@
>  # Notes:  An empty password in this command will set the password to the 
> empty
>  # string.  Existing clients are unaffected by executing this command.
>  ##
> -{ 'command': 'change-vnc-password', 'data': {'password': 'str'} }
> -
> +{ 'command': 'change-vnc-password', 'data': {'password': 'str'},
> +  'if': 'defined(CONFIG_VNC)' }
>  ##
>  # @change:
>  #
> diff --git a/qapi/event.json b/qapi/event.json
> index 6d22b025cc..c8b8e9f384 100644
> --- a/qapi/event.json
> +++ b/qapi/event.json
> @@ -263,7 +263,8 @@
>  ##
>  { 'event': 'VNC_CONNECTED',
>'data': { 'server': 'VncServerInfo',
> -'client': 'VncBasicInfo' } }
> +'client': 'VncBasicInfo' },
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @VNC_INITIALIZED:
> @@ -290,7 +291,8 @@
>  ##
>  { 'event': 'VNC_INITIALIZED',
>'data': { 'server': 'VncServerInfo',
> -'client': 'VncClientInfo' } }
> +'client': 'VncClientInfo' },
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @VNC_DISCONNECTED:
> @@ -316,7 +318,8 @@
>  ##
>  { 'event': 'VNC_DISCONNECTED',
>'data': { 'server': 'VncServerInfo',
> -'client': 'VncClientInfo' } }
> +'client': 'VncClientInfo' },
> +  'if': 'defined(CONFIG_VNC)' }
>  
>  ##
>  # @SPICE_CONNECTED:
> diff --git a/hmp.c b/hmp.c
> index fd80dce7

Re: [Qemu-devel] [PATCH 00/17] nbd client refactoring and fixing

2017-08-17 Thread Vladimir Sementsov-Ogievskiy

17.08.2017 00:21, Eric Blake wrote:

On 08/04/2017 10:14 AM, Vladimir Sementsov-Ogievskiy wrote:

A bit more refactoring and fixing before BLOCK_STATUS series.
I've tried to make individual patches simple enough, so there are
a lot of them.

Is your BLOCK_STATUS series something that is in good enough shape to
post a preliminary version of it (the version you posted back in
February is now horribly out-of-date, with all the good cleanups you
have been doing in the meantime).  I want to get a running start at
reviewing what I can to make sure we get improved NBD functionality into
2.11.


Every time I want to produce a new version of BLOCK_STATUS, I stumble on
something and new 10-20 patches refactoring series appears)



Also, please feel free to offer your Reviewed-by on other patches
(whether NBD-related or not).  Speaking as the NBD maintainer, I welcome
any help I can get.  And from personal experience, reviews tend to be
one of the largest bottlenecks in open source software - if you are
writing patches but not offering reviews, then you are adding to the
bottleneck so reviewers tend to set your patches aside for when they
have more time; while if you are actively offering reviews, then it is
obvious that you care about the project and your patch contributions
tend to have an easier time getting in.  My personal rule of thumb is to
try and review at least 2 other patches for every one that I send,
although that is a rather ambitious goal and there's nothing wrong if
you can't commit to theh same level of effort.


Thanks, I get the point, I'll try to do better.





Vladimir Sementsov-Ogievskiy (17):
   nbd/client: fix nbd_opt_go
   nbd/client: refactor nbd_read_eof
   nbd/client: refactor nbd_receive_reply
   nbd/client: fix nbd_send_request to return int
   block/nbd-client: get rid of ssize_t
   block/nbd-client: fix nbd_read_reply_entry
   block/nbd-client: refactor request send/receive
   block/nbd-client: rename nbd_recv_coroutines_enter_all
   block/nbd-client: move nbd_co_receive_reply content into
 nbd_co_request
   block/nbd-client: move nbd_coroutine_end content into nbd_co_request
   block/nbd-client: fix nbd_co_request: set s->reply.handle to 0 on
 error
   block/nbd-client: refactor nbd_co_request
   block/nbd-client: refactor NBDClientSession.recv_coroutine
   block/nbd-client: exit reply-reading coroutine on incorrect handle
   block/nbd-client: refactor reading reply
   block/nbd-client: drop reply field from NBDClientSession
   block/nbd-client: always return EIO on and after the first io channel
 error

Of course, parts of this will need rebasing based on what finally landed
in 2.10, but I can start reviewing what I can for this round.



--
Best regards,
Vladimir




Re: [Qemu-devel] [PATCH 17/26] qapi: add conditions to SPICE type/commands/events on the schema

2017-08-17 Thread Markus Armbruster
Copying our resident SPICE maintainer Gerd.

Marc-André Lureau  writes:

> Add #if defined(CONFIG_SPICE) in generated code, and adjust the
> qmp/hmp code accordingly.
>
> Signed-off-by: Marc-André Lureau 
> ---
>  qapi-schema.json | 28 ++--
>  qapi/event.json  | 12 
>  monitor.c|  3 ---
>  qmp.c| 16 
>  4 files changed, 26 insertions(+), 33 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 829c66f9eb..bcee3157b0 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1878,7 +1878,8 @@
>  { 'struct': 'SpiceBasicInfo',
>'data': { 'host': 'str',
>  'port': 'str',
> -'family': 'NetworkAddressFamily' } }
> +'family': 'NetworkAddressFamily' },
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @SpiceServerInfo:
> @@ -1891,7 +1892,8 @@
>  ##
>  { 'struct': 'SpiceServerInfo',
>'base': 'SpiceBasicInfo',
> -  'data': { '*auth': 'str' } }
> +  'data': { '*auth': 'str' },
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @SpiceChannel:
> @@ -1916,7 +1918,8 @@
>  { 'struct': 'SpiceChannel',
>'base': 'SpiceBasicInfo',
>'data': {'connection-id': 'int', 'channel-type': 'int', 'channel-id': 
> 'int',
> -   'tls': 'bool'} }
> +   'tls': 'bool'},
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @SpiceQueryMouseMode:
> @@ -1935,7 +1938,8 @@
>  # Since: 1.1
>  ##
>  { 'enum': 'SpiceQueryMouseMode',
> -  'data': [ 'client', 'server', 'unknown' ] }
> +  'data': [ 'client', 'server', 'unknown' ],
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @SpiceInfo:
> @@ -1972,7 +1976,8 @@
>  { 'struct': 'SpiceInfo',
>'data': {'enabled': 'bool', 'migrated': 'bool', '*host': 'str', '*port': 
> 'int',
> '*tls-port': 'int', '*auth': 'str', '*compiled-version': 'str',
> -   'mouse-mode': 'SpiceQueryMouseMode', '*channels': 
> ['SpiceChannel']} }
> +   'mouse-mode': 'SpiceQueryMouseMode', '*channels': 
> ['SpiceChannel']},
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @query-spice:
> @@ -2017,7 +2022,8 @@
>  #}
>  #
>  ##
> -{ 'command': 'query-spice', 'returns': 'SpiceInfo' }
> +{ 'command': 'query-spice', 'returns': 'SpiceInfo',
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @BalloonInfo:
> @@ -5067,7 +5073,8 @@
>  # Since: 1.5
>  ##
>  { 'struct': 'ChardevSpiceChannel', 'data': { 'type'  : 'str' },
> -  'base': 'ChardevCommon' }
> +  'base': 'ChardevCommon',
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @ChardevSpicePort:
> @@ -5079,7 +5086,8 @@
>  # Since: 1.5
>  ##
>  { 'struct': 'ChardevSpicePort', 'data': { 'fqdn'  : 'str' },
> -  'base': 'ChardevCommon' }
> +  'base': 'ChardevCommon',
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @ChardevVC:
> @@ -5133,8 +5141,8 @@
> 'testdev': 'ChardevCommon',
> 'stdio'  : 'ChardevStdio',
> 'console': 'ChardevCommon',
> -   'spicevmc' : 'ChardevSpiceChannel',
> -   'spiceport' : 'ChardevSpicePort',
> +   'spicevmc' : { 'type': 
> 'ChardevSpiceChannel', 'if': 'defined(CONFIG_SPICE)' },
> +   'spiceport' : { 'type': 
> 'ChardevSpicePort', 'if': 'defined(CONFIG_SPICE)' },
> 'vc' : 'ChardevVC',
> 'ringbuf': 'ChardevRingbuf',
> # next one is just for compatibility
> diff --git a/qapi/event.json b/qapi/event.json
> index c8b8e9f384..ff59551914 100644
> --- a/qapi/event.json
> +++ b/qapi/event.json
> @@ -344,7 +344,8 @@
>  ##
>  { 'event': 'SPICE_CONNECTED',
>'data': { 'server': 'SpiceBasicInfo',
> -'client': 'SpiceBasicInfo' } }
> +'client': 'SpiceBasicInfo' },
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @SPICE_INITIALIZED:
> @@ -372,7 +373,8 @@
>  ##
>  { 'event': 'SPICE_INITIALIZED',
>'data': { 'server': 'SpiceServerInfo',
> -'client': 'SpiceChannel' } }
> +'client': 'SpiceChannel' },
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @SPICE_DISCONNECTED:
> @@ -397,7 +399,8 @@
>  ##
>  { 'event': 'SPICE_DISCONNECTED',
>'data': { 'server': 'SpiceBasicInfo',
> -'client': 'SpiceBasicInfo' } }
> +'client': 'SpiceBasicInfo' },
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @SPICE_MIGRATE_COMPLETED:
> @@ -412,7 +415,8 @@
>  #  "event": "SPICE_MIGRATE_COMPLETED" }
>  #
>  ##
> -{ 'event': 'SPICE_MIGRATE_COMPLETED' }
> +{ 'event': 'SPICE_MIGRATE_COMPLETED',
> +  'if': 'defined(CONFIG_SPICE)' }
>  
>  ##
>  # @MIGRATION:
> diff --git a/monitor.c b/monitor.c
> index a1773d5bc7..4bf6a3ea2e 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -970,9 +970,6 @@ static void qmp_query_qmp_schema(QDict *qdict, QO

Re: [Qemu-devel] [PATCH 1/6] tests: Run filter-redirector and -mirror test only on POSIX systems

2017-08-17 Thread Cornelia Huck
On Thu, 17 Aug 2017 08:25:08 +0200
Thomas Huth  wrote:

> This way we can get rid of the ugly #ifdefs in the code which makes
> it easier to extend later.
> 
> Signed-off-by: Thomas Huth 
> ---
>  tests/Makefile.include |  8 
>  tests/test-filter-mirror.c |  5 -
>  tests/test-filter-redirector.c | 10 --
>  3 files changed, 4 insertions(+), 19 deletions(-)

Yes, that is much nicer.

Reviewed-by: Cornelia Huck 



Re: [Qemu-devel] [PATCH 2/6] tests: Add network filter tests to the check-qtest-s390x list

2017-08-17 Thread Cornelia Huck
On Thu, 17 Aug 2017 08:25:09 +0200
Thomas Huth  wrote:

> With some small modifications, we can also use the the netfilter,
> the fiter-mirror and the filter-redirector tests on s390x.

s/fiter/filter/

> 
> Signed-off-by: Thomas Huth 
> ---
>  tests/Makefile.include |  3 +++
>  tests/test-filter-mirror.c |  9 +++--
>  tests/test-filter-redirector.c | 22 --
>  tests/test-netfilter.c | 11 ++-
>  4 files changed, 36 insertions(+), 9 deletions(-)
> 

> diff --git a/tests/test-filter-mirror.c b/tests/test-filter-mirror.c
> index a1d5865..d569d27 100644
> --- a/tests/test-filter-mirror.c
> +++ b/tests/test-filter-mirror.c
> @@ -25,6 +25,11 @@ static void test_mirror(void)
>  char *recv_buf;
>  uint32_t size = sizeof(send_buf);
>  size = htonl(size);
> +const char *devstr = "e1000";
> +
> +if (g_str_equal(qtest_get_arch(), "s390x")) {
> +devstr = "virtio-net-ccw";
> +}

I'm wondering if we could unify selection of the network device
somehow. There's probably two cases:
- Test a specific device. This obviously needs to be decided
  individually.
- Just use a functional network device. For s390x, this will be
  virtio-net-ccw; for other architectures, this test uses e1000, while
  one of the tests below uses rtl8139 (why?). A helper for that may be
  useful.

>  
>  ret = socketpair(PF_UNIX, SOCK_STREAM, 0, send_sock);
>  g_assert_cmpint(ret, !=, -1);

> diff --git a/tests/test-filter-redirector.c b/tests/test-filter-redirector.c
> index 69c663b..3afd411 100644
> --- a/tests/test-filter-redirector.c
> +++ b/tests/test-filter-redirector.c
> @@ -57,6 +57,16 @@
>  #include "qemu/error-report.h"
>  #include "qemu/main-loop.h"
>  
> +static const char *get_devstr(void)
> +{
> +if (g_str_equal(qtest_get_arch(), "s390x")) {
> +return "virtio-net-ccw";
> +}
> +
> +return "rtl8139";

No problem with your patch, but I'm wondering why this does not use
e1000. Special capabilities of rtl8139?

> +}
> +
> +
>  static void test_redirector_tx(void)
>  {
>  int backend_sock[2], recv_sock;

> diff --git a/tests/test-netfilter.c b/tests/test-netfilter.c
> index 8b5a9b2..2506473 100644
> --- a/tests/test-netfilter.c
> +++ b/tests/test-netfilter.c
> @@ -182,6 +182,12 @@ static void remove_netdev_with_multi_netfilter(void)
>  int main(int argc, char **argv)
>  {
>  int ret;
> +char *args;
> +const char *devstr = "e1000";

It's our old friend again :)

> +
> +if (g_str_equal(qtest_get_arch(), "s390x")) {
> +devstr = "virtio-net-ccw";
> +}
>  
>  g_test_init(&argc, &argv, NULL);
>  qtest_add_func("/netfilter/addremove_one", add_one_netfilter);
> @@ -191,10 +197,13 @@ int main(int argc, char **argv)
>  qtest_add_func("/netfilter/remove_netdev_multi",
> remove_netdev_with_multi_netfilter);
>  
> -qtest_start("-netdev user,id=qtest-bn0 -device e1000,netdev=qtest-bn0");
> +args = g_strdup_printf("-netdev user,id=qtest-bn0 "
> +   "-device %s,netdev=qtest-bn0", devstr);
> +qtest_start(args);
>  ret = g_test_run();
>  
>  qtest_end();
> +g_free(args);
>  
>  return ret;
>  }

Even though I think we should deal with the questions above, having
more tests for s390x is certainly a good idea. Thus,

Reviewed-by: Cornelia Huck 



Re: [Qemu-devel] [PATCH 17/26] qapi: add conditions to SPICE type/commands/events on the schema

2017-08-17 Thread Markus Armbruster
Markus Armbruster  writes:

[...]
> Check for completeness by reviewing the case insensitive occurrences of
> SPICE in the schema not covered by your patch:
>
> * add_client
>
>   Same rationale as for VNC (see my review of PATCH 16).  Good.

Hmm, there's a difference to VNC, in qmp_add_client():

if (strcmp(protocol, "spice") == 0) {
if (!qemu_using_spice(errp)) {
close(fd);
return;
}
skipauth = has_skipauth ? skipauth : false;
tls = has_tls ? tls : false;
if (qemu_spice_display_add_client(fd, skipauth, tls) < 0) {
error_setg(errp, "spice failed to add client");
close(fd);
}
return;
#ifdef CONFIG_VNC
} else if (strcmp(protocol, "vnc") == 0) {
skipauth = has_skipauth ? skipauth : false;
vnc_display_add_client(NULL, fd, skipauth);
return;
#endif
} else if ((s = qemu_chr_find(protocol)) != NULL) {
if (qemu_chr_add_client(s, fd) < 0) {
error_setg(errp, "failed to add client");
close(fd);
return;
}
return;
}

error_setg(errp, "protocol '%s' is invalid", protocol);
close(fd);

Observe:

* Protocol "spice" is always recognized.  Without CONFIG_SPICE,
  using_spice is always false, and the command fails with
  qemu_using_spice()'s error "SPICE is not in use".

* Protocol "vnc" is only recognized with CONFIG_VNC.  Without, it's
  rejected with "protocol 'vnc' is invalid".

* If you name your character device "spice" or "vnc", you can't use it
  with add_client.  This is a design flaw.

  Except you can use one named "vnc" when !CONFIG_VNC.

  I'm afraid the command needs to be replaced.  Not in this series, of
  course.

[...]



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

2017-08-17 Thread Cornelia Huck
On Thu, 17 Aug 2017 08:25:10 +0200
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.

I don't think we should leak the aliasing stuff into tests, but rather
specify the transport on a per-architecture basis explicitly. (We might
want to test virtio-pci on s390x in the future as well -- in addition
to virtio-ccw, not replacing it.)

Also, the same question as for the previous patch: Is this supposed to
test virtio explicitly, or do we just want a reasonable device?

> 
> Signed-off-by: Thomas Huth 
> ---
>  tests/Makefile.include |  1 +
>  tests/drive_del-test.c | 13 +++--
>  2 files changed, 8 insertions(+), 6 deletions(-)



[Qemu-devel] [Bug 1711316] [NEW] fbsd strip(1) segfaults on aarch64

2017-08-17 Thread Gergely Czuczy
Public bug reported:

Hello,

During port builds ld.lld(devel/boost-libs, www/node), and strip
(textproc/libxml2 on xmlcatalog) are segfaulting. On physical boxes
these things do work, as they should:

root@build-aarch64:~ # strip xmlcatalog
Segmentation fault (core dumped)

All the cores fail at the same assembly instruction, here's strip's backtrace:
# lldb -c strip.core strip
(lldb) target create "strip" --core "strip.core"
Core file '/root/strip.core' (aarch64) was loaded.
(lldb) t 1
* thread #1, name = 'strip', stop reason = signal SIGSEGV
frame #0: 0x40312f40 libc.so.7`memcpy + 192
libc.so.7`memcpy:
->  0x40312f40 <+192>: ldpx4, x3, [x4, #-0x10]
0x40312f44 <+196>: stpx6, x7, [x0]
0x40312f48 <+200>: stpx8, x9, [x0, #0x10]
0x40312f4c <+204>: stpx10, x11, [x0, #0x20]
(lldb) bt
* thread #1, name = 'strip', stop reason = signal SIGSEGV
  * frame #0: 0x40312f40 libc.so.7`memcpy + 192
frame #1: 0x4017ac70 
libelf.so.2`_libelf_cvt_HALF_tom(dst=, dsz=, 
src=, count=, byteswap=) at 
libelf_convert.c:794
frame #2: 0x40177b34 libelf.so.2`elf_getdata(s=0x40f355a0, 
ed=) at elf_data.c:155
frame #3: 0x000283d4 strip`copy_data(s=0x40f36a40) at 
sections.c:1176
frame #4: 0x00027ea4 strip`copy_content(ecp=) at 
sections.c:594
frame #5: 0x00023ff4 strip`create_elf(ecp=0x40ed6000) at 
main.c:381
frame #6: 0x0002558c strip`create_file(ecp=, 
src=, dst=) at main.c:705
frame #7: 0x00024e20 strip`main [inlined] 
strip_main(argc=, argv=) at main.c:1192
frame #8: 0x00024cc8 strip`main(argc=, 
argv=) at main.c:1590
frame #9: 0x00020190 strip`__start + 376
frame #10: 0x40050018 ld-elf.so.1`.rtld_start at rtld_start.S:41

Host system information:
# uname -a
FreeBSD marvin.harmless.hu 11.0-STABLE FreeBSD 11.0-STABLE #0 r311927: Wed Jan 
11 14:53:55 CET 2017 t...@marvin.harmless.hu:/usr/obj/usr/src/sys/MARVIN  
amd64

# qemu-system-aarch64 --version
QEMU emulator version 2.9.0
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
# pkg info | grep qemu
qemu-2.9.0 QEMU CPU Emulator
I also had this happening with 2.8.0 and 2.8.1

Guest information:
# uname -a
FreeBSD build-aarch64.marvin.harmless.hu 12.0-CURRENT FreeBSD 12.0-CURRENT #0 
r322578: Wed Aug 16 18:08:43 CEST 2017 
t...@marvin.harmless.hu:/tank/rpi3/obj/arm64.aarch64/tank/rpi3/src/sys/GENERIC-NODEBUG
  arm64

Startup:
zdev=/dev/zvol/tank/rpi3/qemu-image
qemu-system-aarch64 -m 4096M -cpu cortex-a57 -M virt  \
-accel tcg,thread=single \
-bios QEMU_EFI.fd -serial telnet::,server -nographic \
-drive if=none,file=${image},id=hd0,format=raw \
-device virtio-blk-device,drive=hd0 \
-device e1000,netdev=net0 \
-netdev 
tap,id=net0,ifname=tap0,script=/tank/rpi3/build/qemu-ifup.sh

Tested with cortex A53 as well, does the same.

I've attached a test image to reproduce with (340MB), if it's too big for an 
attachment, it can be downloaded from here:
http://czg.harmless.hu/qemu-bug-stripsigsegv/image.gz

Reproduction steps:
1) log in as root, no password
2) strip xmlcatalog, it will segfault

For a full reproduction with ld.lld as well, you need a ports tree, it's 
suggested to attach a bigger volume to /usr/ports over NFS first (it might need 
more space than the image has). Steps for it:
1) portsnap fetch extract
2) make -C /usr/ports/devel/boost-libs package-recursive
3) make -C /usr/ports/textproc/libxml2 package-recursive
4) make -C /usr/ports/www/node package-recursive

Boost and node can take a day or more in a qemu VM. The build-time
options I've be set already, /var/db/ports is populated, so you
shouldn't have questions asked during the builds.

I've saved the core dumps, those can be found at /root/cores/ in the
image.

I hope I've submitted all the required information. If anything else is
needed, please let me know.

Best regards,
Gergely

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  fbsd strip(1) segfaults on aarch64

Status in QEMU:
  New

Bug description:
  Hello,

  During port builds ld.lld(devel/boost-libs, www/node), and strip
  (textproc/libxml2 on xmlcatalog) are segfaulting. On physical boxes
  these things do work, as they should:

  root@build-aarch64:~ # strip xmlcatalog
  Segmentation fault (core dumped)

  All the cores fail at the same assembly instruction, here's strip's backtrace:
  # lldb -c strip.core strip
  (lldb) target create "strip" --core "strip.core"
  Core file '/root/strip.core' (aarch64) was loaded.
  (lldb) t 1
  * thread #1, name = 'strip', stop reason = signal SIGSEGV
  frame #0: 0x40312f40 lib

Re: [Qemu-devel] [PATCH 16/26] qapi: add conditions to VNC type/commands/events on the schema

2017-08-17 Thread Markus Armbruster
Gerd, can we delete the vnc_init_func() stub?  Things still compile for
me when I do.

diff --git a/include/ui/console.h b/include/ui/console.h
index 7262bef..2c3b2cd 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -484,11 +484,6 @@ static inline QemuOpts *vnc_parse(const char *str, Error 
**errp)
 error_setg(errp, "VNC support is disabled");
 return NULL;
 }
-static inline int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
-{
-error_setg(errp, "VNC support is disabled");
-return -1;
-}
 #endif
 
 /* curses.c */



Re: [Qemu-devel] [PATCH 4/6] tests: Introduce generic device hot-plug/hot-unplug functions

2017-08-17 Thread Cornelia Huck
On Thu, 17 Aug 2017 08:25:11 +0200
Thomas Huth  wrote:

> A lot of tests provide code for adding and removing a device via the
> device_add and device_del QMP commands. Maintaining this code in so
> many places is cumbersome and error-prone (some of the code parts
> check the responses in an incorrect way, for example), so let's
> provide some proper generic qtest functions for adding and removing a
> device instead.

This sounds like a good idea.

> 
> Signed-off-by: Thomas Huth 
> ---
>  tests/libqos/pci.c | 19 ++-
>  tests/libqos/usb.c | 30 +--
>  tests/libqtest.c   | 60 
> ++
>  tests/libqtest.h   | 19 +++
>  tests/usb-hcd-uhci-test.c  | 26 ++--
>  tests/usb-hcd-xhci-test.c  | 51 ---
>  tests/virtio-scsi-test.c   | 24 ++-
>  tests/virtio-serial-test.c | 25 +++
>  8 files changed, 98 insertions(+), 156 deletions(-)
> 

> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index b9a1f18..4339d97 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -987,3 +987,63 @@ void qtest_cb_for_every_machine(void (*cb)(const char 
> *machine))
>  qtest_end();
>  QDECREF(response);
>  }
> +
> +/**
> + * Generic hot-plugging test via the device_add QMP command
> + */
> +void qtest_hot_plug_device(const char *driver, const char *id,
> +   const char *fmt, ...)
> +{
> +QDict *response;
> +char *cmd, *opts = NULL;
> +va_list va;
> +
> +if (fmt) {
> +va_start(va, fmt);
> +opts = g_strdup_vprintf(fmt, va);
> +va_end(va);
> +}
> +
> +cmd = g_strdup_printf("{'execute': 'device_add',"
> +  " 'arguments': { 'driver': '%s', 'id': '%s'%s%s 
> }}",
> +  driver, id, opts ? ", " : "", opts ? opts : "");
> +g_free(opts);
> +
> +response = qmp(cmd);
> +g_free(cmd);
> +g_assert(response);
> +while (qdict_haskey(response, "event")) {
> +/* We can get DEVICE_DELETED events in case something went wrong */
> +g_assert_cmpstr(qdict_get_str(response, "event"), !=, 
> "DEVICE_DELETED");

Is there other stuff we should check for?

> +QDECREF(response);
> +response = qmp("");
> +g_assert(response);
> +}
> +g_assert(!qdict_haskey(response, "error"));
> +QDECREF(response);
> +}
> +
> +/**
> + * Generic hot-unplugging test via the device_del QMP command
> + */
> +void qtest_hot_unplug_device(const char *id)
> +{
> +QDict *response;
> +char *cmd;
> +
> +cmd = g_strdup_printf("{'execute': 'device_del',"
> +  " 'arguments': { 'id': '%s' }}", id);
> +
> +response = qmp(cmd);
> +g_free(cmd);
> +g_assert(response);
> +while (qdict_haskey(response, "event")) {
> +/* We should get DEVICE_DELETED event first */
> +g_assert_cmpstr(qdict_get_str(response, "event"), ==, 
> "DEVICE_DELETED");

'should' does not sound assert-worthy to me :) Is this an expected event?

> +QDECREF(response);
> +response = qmp("");
> +g_assert(response);
> +}
> +g_assert(!qdict_haskey(response, "error"));
> +QDECREF(response);
> +}



Re: [Qemu-devel] [PATCH 5/6] tests: Add qvirtio_(un)plug_device_test wrapper functions

2017-08-17 Thread Cornelia Huck
On Thu, 17 Aug 2017 08:25:12 +0200
Thomas Huth  wrote:

> To support hot-plugging tests with virtio-ccw later, the current
> tests should become independent from PCI specific functions. Thus
> let's add some proper wrapper function for virtio device hot-plugging
> and -unplugging first.
> It also seems like device unplugging works fine on ppc64 when using
> the generic qtest_hot_unplug_device() function, so hot-unplugging
> is now tested on ppc64, too.

Yeah for more coverage!

> 
> Signed-off-by: Thomas Huth 
> ---
>  tests/Makefile.include  |  4 ++--
>  tests/libqos/virtio.c   | 27 +++
>  tests/libqos/virtio.h   |  5 +
>  tests/virtio-net-test.c |  8 ++--
>  tests/virtio-rng-test.c |  9 +++--
>  5 files changed, 39 insertions(+), 14 deletions(-)

> diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
> index 635b942..49e733d 100644
> --- a/tests/virtio-net-test.c
> +++ b/tests/virtio-net-test.c
> @@ -241,15 +241,11 @@ static void pci_basic(gconstpointer data)
>  
>  static void hotplug(void)
>  {
> -const char *arch = qtest_get_arch();
> -
>  qtest_start("-device virtio-net-pci");
>  
> -qpci_plug_device_test("virtio-net-pci", "net1", PCI_SLOT_HP, NULL);
> +qvirtio_plug_device_test("virtio-net", "net1", PCI_SLOT_HP, NULL);

Same comment as for a previous patch regarding implicit aliasing.

Also, 'PCI_SLOT_HP' sounds a bit pci-y to me?

>  
> -if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> -qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP);
> -}
> +qvirtio_unplug_device_test("net1", PCI_SLOT_HP);
>  
>  test_end();
>  }
> diff --git a/tests/virtio-rng-test.c b/tests/virtio-rng-test.c
> index dcecf77..04c4279 100644
> --- a/tests/virtio-rng-test.c
> +++ b/tests/virtio-rng-test.c
> @@ -10,6 +10,7 @@
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
>  #include "libqos/pci.h"
> +#include "libqos/virtio.h"
>  
>  #define PCI_SLOT_HP 0x06
>  
> @@ -20,13 +21,9 @@ static void pci_nop(void)
>  
>  static void hotplug(void)
>  {
> -const char *arch = qtest_get_arch();
> +qvirtio_plug_device_test("virtio-rng-pci", "rng1", PCI_SLOT_HP, NULL);

Dito.

>  
> -qpci_plug_device_test("virtio-rng-pci", "rng1", PCI_SLOT_HP, NULL);
> -
> -if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> -qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP);
> -}
> +qvirtio_unplug_device_test("rng1", PCI_SLOT_HP);
>  }
>  
>  int main(int argc, char **argv)




Re: [Qemu-devel] [PATCH 6/6] tests: Enable the simple virtio tests on s390x, too

2017-08-17 Thread Cornelia Huck
On Thu, 17 Aug 2017 08:25:13 +0200
Thomas Huth  wrote:

> Most of the simple virtio test can be used on virtio-ccw on
> s390x, too, by simply using the bus-independent alias names
> of the devices instead of the device names ending in "-pci".
> Hot-plugging can also be tested here - we just have to use
> the generic hot plug function instead of the PCI hot plug
> function in the qvirtio_plug_device_test() function.

Same comments as before regarding aliasing, and a possible future
desire to test virtio-pci on s390x as well.

But yes, we want the virtio tests on s390x as well.

> 
> Signed-off-by: Thomas Huth 
> ---
>  tests/Makefile.include  |  4 
>  tests/libqos/virtio.c   |  4 +++-
>  tests/virtio-balloon-test.c |  6 +++---
>  tests/virtio-console-test.c | 12 ++--
>  tests/virtio-rng-test.c | 10 +-
>  tests/virtio-serial-test.c  |  8 
>  6 files changed, 25 insertions(+), 19 deletions(-)



Re: [Qemu-devel] [PATCH for-2.11 0/6] Enable more qtests for s390x

2017-08-17 Thread Cornelia Huck
On Thu, 17 Aug 2017 08:25:07 +0200
Thomas Huth  wrote:

> We currently do not have many tests enabled for QEMU on s390x yet,
> so this series reworks some of the tests to be also usable on s390x
> to get some extended test coverage there.

More coverage on s390x is certainly desirable. I'm wondering how many
other tests are needlessly restricted to certain architectures as
well...

> 
> Along the way, this also cleans up some of the generic test code,
> e.g. by introducing generic functions to do hot-plugging with
> the device_add QMP command.

Cleanup also sounds good :)

> 
> Please review!
> 
>  Thanks,
>   Thomas
> 
> 
> Thomas Huth (6):
>   tests: Run filter-redirector and -mirror test only on POSIX systems
>   tests: Add network filter tests to the check-qtest-s390x list

I think these two are fine as-is.

>   tests: Enable the drive_del test also on s390x
>   tests: Introduce generic device hot-plug/hot-unplug functions
>   tests: Add qvirtio_(un)plug_device_test wrapper functions
>   tests: Enable the simple virtio tests on s390x, too

But these probably need some work (or at least, some thought.)

> 
>  tests/Makefile.include | 20 +-
>  tests/drive_del-test.c | 13 -
>  tests/libqos/pci.c | 19 ++---
>  tests/libqos/usb.c | 30 +
>  tests/libqos/virtio.c  | 29 
>  tests/libqos/virtio.h  |  5 
>  tests/libqtest.c   | 60 
> ++
>  tests/libqtest.h   | 19 +
>  tests/test-filter-mirror.c | 14 +-
>  tests/test-filter-redirector.c | 32 +++---
>  tests/test-netfilter.c | 11 +++-
>  tests/usb-hcd-uhci-test.c  | 26 ++
>  tests/usb-hcd-xhci-test.c  | 51 +++
>  tests/virtio-balloon-test.c|  6 ++---
>  tests/virtio-console-test.c| 12 -
>  tests/virtio-net-test.c|  8 ++
>  tests/virtio-rng-test.c| 17 +---
>  tests/virtio-scsi-test.c   | 24 ++---
>  tests/virtio-serial-test.c | 33 +--
>  19 files changed, 208 insertions(+), 221 deletions(-)
> 

The big question remains again: Who merges this? I can merge this
through the s390x tree if there are no other takers.



Re: [Qemu-devel] [PATCH] hw/s390x/ipl: The s390-ipl device is not hot-pluggable

2017-08-17 Thread David Hildenbrand
On 16.08.2017 07:30, Thomas Huth wrote:
> The s390-ipl device can not be created by the user, since it is meant only
> to  be instantiated once internally to load the ROMs and kernel. If the user
> tries to do a "device_add s390-ipl" via the monitor later, QEMU aborts with
> a "ROM images must be loaded at startup" error message.
> 
> Signed-off-by: Thomas Huth 
> ---
>  hw/s390x/ipl.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index cc36003..0d06fc1 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -442,6 +442,8 @@ static void s390_ipl_class_init(ObjectClass *klass, void 
> *data)
>  dc->reset = s390_ipl_reset;
>  dc->vmsd = &vmstate_ipl;
>  set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +/* Reason: Loads the ROMs and thus can only be used one time - 
> internally */
> +dc->user_creatable = false;
>  }
>  
>  static const TypeInfo s390_ipl_info = {
> 

Late but still

Reviewed-by: David Hildenbrand 

-- 

Thanks,

David



[Qemu-devel] [PATCH v7 0/4] Add shrink image for qcow2

2017-08-17 Thread Pavel Butsykin
This patch add shrinking of the image file for qcow2. As a result, this allows
us to reduce the virtual image size and free up space on the disk without
copying the image. Image can be fragmented and shrink is done by punching holes
in the image file.

# ./qemu-img create -f qcow2 image.qcow2 4G
Formatting 'image.qcow2', fmt=qcow2 size=4294967296 encryption=off 
cluster_size=65536 lazy_refcounts=off refcount_bits=16

# ./qemu-io -c "write -P 0x22 0 1G" image.qcow2
wrote 1073741824/1073741824 bytes at offset 0
1 GiB, 1 ops; 0:00:04.59 (222.886 MiB/sec and 0.2177 ops/sec)

# ./qemu-img resize image.qcow2 512M
warning: qemu-img: Shrinking an image will delete all data beyond the shrunken 
image's end. Before performing such an operation, make sure there is no 
important data there.
error: qemu-img: Use the --shrink option to perform a shrink operation.

# ./qemu-img resize --shrink image.qcow2 128M
Image resized.

# ./qemu-img info image.qcow2
image: image.qcow2
file format: qcow2
virtual size: 128M (134217728 bytes)
disk size: 128M
cluster_size: 65536
Format specific information:
compat: 1.1
lazy refcounts: false
refcount bits: 16
corrupt: false

# du -h image.qcow2
129Mimage.qcow2

Changes from v1:
- add --shrink flag for qemu-img resize
- add qcow2_cache_discard
- simplify qcow2_shrink_l1_table() to reduce the likelihood of image corruption
- add new qemu-iotests for shrinking images

Changes from v2:
- replace qprintf() on error_report() (1)
- rewrite warning messages (1)
- enforce --shrink flag for all formats except raw (1)
- split qcow2_cache_discard() (2)
- minor fixes according to comments (3)
- rewrite the last part of qcow2_shrink_reftable() to avoid
  qcow2_free_clusters() calls inside (3)
- improve test for shrinking image (4)

Changes from v3:
- rebase on "Implement a warning_report function" Alistair's patch-set (1)
- spelling fixes (1)
- the man page fix according to the discussion (1)
- add call qcow2_signal_corruption() in case of image corruption (3)

Changes from v4:
- rebase on https://github.com/XanClic/qemu/commits/block Max's block branch

Changes from v5:
- the condition refcount == 0 should be enough to evict the l2/refcount cluster
  from the cache (2)
- overwrite the l1/refcount table in memory with zeros, even if overwriting the
  l1/refcount table on disk has failed (3)
- replace g_try_malloc() on g_malloc() for allocation reftable_tmp (3)

Changes from v6:
- rebase on master 1f29673387

Pavel Butsykin (4):
  qemu-img: add --shrink flag for resize
  qcow2: add qcow2_cache_discard
  qcow2: add shrink image support
  qemu-iotests: add shrinking image test

 block/qcow2-cache.c|  26 +++
 block/qcow2-cluster.c  |  50 +
 block/qcow2-refcount.c | 140 -
 block/qcow2.c  |  43 +---
 block/qcow2.h  |  17 +
 qapi/block-core.json   |   3 +-
 qemu-img-cmds.hx   |   4 +-
 qemu-img.c |  23 ++
 qemu-img.texi  |   6 +-
 tests/qemu-iotests/102 |   4 +-
 tests/qemu-iotests/163 | 170 +
 tests/qemu-iotests/163.out |   5 ++
 tests/qemu-iotests/group   |   1 +
 13 files changed, 475 insertions(+), 17 deletions(-)
 create mode 100644 tests/qemu-iotests/163
 create mode 100644 tests/qemu-iotests/163.out

-- 
2.14.1




[Qemu-devel] [PATCH v7 1/4] qemu-img: add --shrink flag for resize

2017-08-17 Thread Pavel Butsykin
The flag is additional precaution against data loss. Perhaps in the future the
operation shrink without this flag will be blocked for all formats, but for now
we need to maintain compatibility with raw.

Signed-off-by: Pavel Butsykin 
Reviewed-by: Max Reitz 
---
 qemu-img-cmds.hx   |  4 ++--
 qemu-img.c | 23 +++
 qemu-img.texi  |  6 +-
 tests/qemu-iotests/102 |  4 ++--
 4 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index b47d409665..2fe31893cf 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -89,9 +89,9 @@ STEXI
 ETEXI
 
 DEF("resize", img_resize,
-"resize [--object objectdef] [--image-opts] [-q] filename [+ | -]size")
+"resize [--object objectdef] [--image-opts] [-q] [--shrink] filename [+ | 
-]size")
 STEXI
-@item resize [--object @var{objectdef}] [--image-opts] [-q] @var{filename} [+ 
| -]@var{size}
+@item resize [--object @var{objectdef}] [--image-opts] [-q] [--shrink] 
@var{filename} [+ | -]@var{size}
 ETEXI
 
 STEXI
diff --git a/qemu-img.c b/qemu-img.c
index 56ef49e214..b7b2386cbd 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -65,6 +65,7 @@ enum {
 OPTION_TARGET_IMAGE_OPTS = 263,
 OPTION_SIZE = 264,
 OPTION_PREALLOCATION = 265,
+OPTION_SHRINK = 266,
 };
 
 typedef enum OutputFormat {
@@ -3437,6 +3438,7 @@ static int img_resize(int argc, char **argv)
 },
 };
 bool image_opts = false;
+bool shrink = false;
 
 /* Remove size from argv manually so that negative numbers are not treated
  * as options by getopt. */
@@ -3455,6 +3457,7 @@ static int img_resize(int argc, char **argv)
 {"object", required_argument, 0, OPTION_OBJECT},
 {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 {"preallocation", required_argument, 0, OPTION_PREALLOCATION},
+{"shrink", no_argument, 0, OPTION_SHRINK},
 {0, 0, 0, 0}
 };
 c = getopt_long(argc, argv, ":f:hq",
@@ -3498,6 +3501,9 @@ static int img_resize(int argc, char **argv)
 return 1;
 }
 break;
+case OPTION_SHRINK:
+shrink = true;
+break;
 }
 }
 if (optind != argc - 1) {
@@ -3571,6 +3577,23 @@ static int img_resize(int argc, char **argv)
 goto out;
 }
 
+if (total_size < current_size && !shrink) {
+warn_report("Shrinking an image will delete all data beyond the "
+"shrunken image's end. Before performing such an "
+"operation, make sure there is no important data there.");
+
+if (g_strcmp0(bdrv_get_format_name(blk_bs(blk)), "raw") != 0) {
+error_report(
+  "Use the --shrink option to perform a shrink operation.");
+ret = -1;
+goto out;
+} else {
+warn_report("Using the --shrink option will suppress this message."
+"Note that future versions of qemu-img may refuse to "
+"shrink images without this option.");
+}
+}
+
 ret = blk_truncate(blk, total_size, prealloc, &err);
 if (!ret) {
 qprintf(quiet, "Image resized.\n");
diff --git a/qemu-img.texi b/qemu-img.texi
index 72dabd6b3e..ea5d04b873 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -536,7 +536,7 @@ qemu-img rebase -b base.img diff.qcow2
 At this point, @code{modified.img} can be discarded, since
 @code{base.img + diff.qcow2} contains the same information.
 
-@item resize [--preallocation=@var{prealloc}] @var{filename} [+ | -]@var{size}
+@item resize [--shrink] [--preallocation=@var{prealloc}] @var{filename} [+ | 
-]@var{size}
 
 Change the disk image as if it had been created with @var{size}.
 
@@ -544,6 +544,10 @@ Before using this command to shrink a disk image, you MUST 
use file system and
 partitioning tools inside the VM to reduce allocated file systems and partition
 sizes accordingly.  Failure to do so will result in data loss!
 
+When shrinking images, the @code{--shrink} option must be given. This informs
+qemu-img that the user acknowledges all loss of data beyond the truncated
+image's end.
+
 After using this command to grow a disk image, you must use file system and
 partitioning tools inside the VM to actually begin using the new space on the
 device.
diff --git a/tests/qemu-iotests/102 b/tests/qemu-iotests/102
index 87db1bb1bf..d7ad8d9840 100755
--- a/tests/qemu-iotests/102
+++ b/tests/qemu-iotests/102
@@ -54,7 +54,7 @@ _make_test_img $IMG_SIZE
 $QEMU_IO -c 'write 0 64k' "$TEST_IMG" | _filter_qemu_io
 # Remove data cluster from image (first cluster: image header, second: 
reftable,
 # third: refblock, fourth: L1 table, fifth: L2 table)
-$QEMU_IMG resize -f raw "$TEST_IMG" $((5 * 64 * 1024))
+$QEMU_IMG resize -f raw --shrink "$TEST_IMG" $((5 * 64 * 1024))
 
 $QEMU_IO -c map "$TEST_IMG"
 $QEMU_IMG map "$TEST_IMG"
@@ -69,7 +69,7 @@ $QEMU_IO -c 'write 0 64k' "$TEST_IMG" | _fil

Re: [Qemu-devel] [PATCH] hw/s390x/ipl: The s390-ipl device is not hot-pluggable

2017-08-17 Thread Cornelia Huck
On Thu, 17 Aug 2017 11:05:35 +0200
David Hildenbrand  wrote:

> On 16.08.2017 07:30, Thomas Huth wrote:
> > The s390-ipl device can not be created by the user, since it is meant only
> > to  be instantiated once internally to load the ROMs and kernel. If the user
> > tries to do a "device_add s390-ipl" via the monitor later, QEMU aborts with
> > a "ROM images must be loaded at startup" error message.
> > 
> > Signed-off-by: Thomas Huth 
> > ---
> >  hw/s390x/ipl.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> > index cc36003..0d06fc1 100644
> > --- a/hw/s390x/ipl.c
> > +++ b/hw/s390x/ipl.c
> > @@ -442,6 +442,8 @@ static void s390_ipl_class_init(ObjectClass *klass, 
> > void *data)
> >  dc->reset = s390_ipl_reset;
> >  dc->vmsd = &vmstate_ipl;
> >  set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> > +/* Reason: Loads the ROMs and thus can only be used one time - 
> > internally */
> > +dc->user_creatable = false;
> >  }
> >  
> >  static const TypeInfo s390_ipl_info = {
> >   
> 
> Late but still
> 
> Reviewed-by: David Hildenbrand 
> 

Not too late for me to add the tag :)

Thanks!



[Qemu-devel] [PATCH v7 2/4] qcow2: add qcow2_cache_discard

2017-08-17 Thread Pavel Butsykin
Whenever l2/refcount table clusters are discarded from the file we can
automatically drop unnecessary content of the cache tables. This reduces
the chance of eviction useful cache data and eliminates inconsistent data
in the cache with the data in the file.

Signed-off-by: Pavel Butsykin 
Reviewed-by: Max Reitz 
---
 block/qcow2-cache.c| 26 ++
 block/qcow2-refcount.c | 20 ++--
 block/qcow2.h  |  3 +++
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 1d25147392..75746a7f43 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -411,3 +411,29 @@ void qcow2_cache_entry_mark_dirty(BlockDriverState *bs, 
Qcow2Cache *c,
 assert(c->entries[i].offset != 0);
 c->entries[i].dirty = true;
 }
+
+void *qcow2_cache_is_table_offset(BlockDriverState *bs, Qcow2Cache *c,
+  uint64_t offset)
+{
+int i;
+
+for (i = 0; i < c->size; i++) {
+if (c->entries[i].offset == offset) {
+return qcow2_cache_get_table_addr(bs, c, i);
+}
+}
+return NULL;
+}
+
+void qcow2_cache_discard(BlockDriverState *bs, Qcow2Cache *c, void *table)
+{
+int i = qcow2_cache_get_table_idx(bs, c, table);
+
+assert(c->entries[i].ref == 0);
+
+c->entries[i].offset = 0;
+c->entries[i].lru_counter = 0;
+c->entries[i].dirty = false;
+
+qcow2_cache_table_release(bs, c, i, 1);
+}
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 168fc32e7b..8c17c0e3aa 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -861,8 +861,24 @@ static int QEMU_WARN_UNUSED_RESULT 
update_refcount(BlockDriverState *bs,
 }
 s->set_refcount(refcount_block, block_index, refcount);
 
-if (refcount == 0 && s->discard_passthrough[type]) {
-update_refcount_discard(bs, cluster_offset, s->cluster_size);
+if (refcount == 0) {
+void *table;
+
+table = qcow2_cache_is_table_offset(bs, s->refcount_block_cache,
+offset);
+if (table != NULL) {
+qcow2_cache_put(bs, s->refcount_block_cache, &refcount_block);
+qcow2_cache_discard(bs, s->refcount_block_cache, table);
+}
+
+table = qcow2_cache_is_table_offset(bs, s->l2_table_cache, offset);
+if (table != NULL) {
+qcow2_cache_discard(bs, s->l2_table_cache, table);
+}
+
+if (s->discard_passthrough[type]) {
+update_refcount_discard(bs, cluster_offset, s->cluster_size);
+}
 }
 }
 
diff --git a/block/qcow2.h b/block/qcow2.h
index 96a8d43c17..52c374e9ed 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -649,6 +649,9 @@ int qcow2_cache_get(BlockDriverState *bs, Qcow2Cache *c, 
uint64_t offset,
 int qcow2_cache_get_empty(BlockDriverState *bs, Qcow2Cache *c, uint64_t offset,
 void **table);
 void qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table);
+void *qcow2_cache_is_table_offset(BlockDriverState *bs, Qcow2Cache *c,
+  uint64_t offset);
+void qcow2_cache_discard(BlockDriverState *bs, Qcow2Cache *c, void *table);
 
 /* qcow2-bitmap.c functions */
 int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
-- 
2.14.1




[Qemu-devel] [PATCH v7 3/4] qcow2: add shrink image support

2017-08-17 Thread Pavel Butsykin
This patch add shrinking of the image file for qcow2. As a result, this allows
us to reduce the virtual image size and free up space on the disk without
copying the image. Image can be fragmented and shrink is done by punching holes
in the image file.

Signed-off-by: Pavel Butsykin 
Reviewed-by: Max Reitz 
---
 block/qcow2-cluster.c  |  50 +
 block/qcow2-refcount.c | 120 +
 block/qcow2.c  |  43 ++
 block/qcow2.h  |  14 ++
 qapi/block-core.json   |   3 +-
 5 files changed, 220 insertions(+), 10 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index f06c08f64c..0c7a9a920c 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -32,6 +32,56 @@
 #include "qemu/bswap.h"
 #include "trace.h"
 
+int qcow2_shrink_l1_table(BlockDriverState *bs, uint64_t exact_size)
+{
+BDRVQcow2State *s = bs->opaque;
+int new_l1_size, i, ret;
+
+if (exact_size >= s->l1_size) {
+return 0;
+}
+
+new_l1_size = exact_size;
+
+#ifdef DEBUG_ALLOC2
+fprintf(stderr, "shrink l1_table from %d to %d\n", s->l1_size, 
new_l1_size);
+#endif
+
+BLKDBG_EVENT(bs->file, BLKDBG_L1_SHRINK_WRITE_TABLE);
+ret = bdrv_pwrite_zeroes(bs->file, s->l1_table_offset +
+   new_l1_size * sizeof(uint64_t),
+ (s->l1_size - new_l1_size) * sizeof(uint64_t), 0);
+if (ret < 0) {
+goto fail;
+}
+
+ret = bdrv_flush(bs->file->bs);
+if (ret < 0) {
+goto fail;
+}
+
+BLKDBG_EVENT(bs->file, BLKDBG_L1_SHRINK_FREE_L2_CLUSTERS);
+for (i = s->l1_size - 1; i > new_l1_size - 1; i--) {
+if ((s->l1_table[i] & L1E_OFFSET_MASK) == 0) {
+continue;
+}
+qcow2_free_clusters(bs, s->l1_table[i] & L1E_OFFSET_MASK,
+s->cluster_size, QCOW2_DISCARD_ALWAYS);
+s->l1_table[i] = 0;
+}
+return 0;
+
+fail:
+/*
+ * If the write in the l1_table failed the image may contain partially
+ * overwritten the l1_table. In this case would be better to clear the
+ * l1_table in memory to avoid possible image corruption.
+ */
+memset(s->l1_table + exact_size, 0,
+   (s->l1_size - new_l1_size) * sizeof(uint64_t));
+return ret;
+}
+
 int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
 bool exact_size)
 {
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 8c17c0e3aa..15af9a795f 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -29,6 +29,7 @@
 #include "block/qcow2.h"
 #include "qemu/range.h"
 #include "qemu/bswap.h"
+#include "qemu/cutils.h"
 
 static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size);
 static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
@@ -3061,3 +3062,122 @@ done:
 qemu_vfree(new_refblock);
 return ret;
 }
+
+static int qcow2_discard_refcount_block(BlockDriverState *bs,
+uint64_t discard_block_offs)
+{
+BDRVQcow2State *s = bs->opaque;
+uint64_t refblock_offs = get_refblock_offset(s, discard_block_offs);
+uint64_t cluster_index = discard_block_offs >> s->cluster_bits;
+uint32_t block_index = cluster_index & (s->refcount_block_size - 1);
+void *refblock;
+int ret;
+
+assert(discard_block_offs != 0);
+
+ret = qcow2_cache_get(bs, s->refcount_block_cache, refblock_offs,
+  &refblock);
+if (ret < 0) {
+return ret;
+}
+
+if (s->get_refcount(refblock, block_index) != 1) {
+qcow2_signal_corruption(bs, true, -1, -1, "Invalid refcount:"
+" refblock offset %#" PRIx64
+", reftable index %u"
+", block offset %#" PRIx64
+", refcount %#" PRIx64,
+refblock_offs,
+offset_to_reftable_index(s, 
discard_block_offs),
+discard_block_offs,
+s->get_refcount(refblock, block_index));
+qcow2_cache_put(bs, s->refcount_block_cache, &refblock);
+return -EINVAL;
+}
+s->set_refcount(refblock, block_index, 0);
+
+qcow2_cache_entry_mark_dirty(bs, s->refcount_block_cache, refblock);
+
+qcow2_cache_put(bs, s->refcount_block_cache, &refblock);
+
+if (cluster_index < s->free_cluster_index) {
+s->free_cluster_index = cluster_index;
+}
+
+refblock = qcow2_cache_is_table_offset(bs, s->refcount_block_cache,
+   discard_block_offs);
+if (refblock) {
+/* discard refblock from the cache if refblock is cached */
+qcow2_cache_discard(bs, s->refcount_block_cache, refblock);
+}
+update_refcount_discard(bs, discard_block_offs, s->cluster_size);
+
+ 

[Qemu-devel] [PATCH v7 4/4] qemu-iotests: add shrinking image test

2017-08-17 Thread Pavel Butsykin
Signed-off-by: Pavel Butsykin 
Reviewed-by: Max Reitz 
---
 tests/qemu-iotests/163 | 170 +
 tests/qemu-iotests/163.out |   5 ++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 176 insertions(+)
 create mode 100644 tests/qemu-iotests/163
 create mode 100644 tests/qemu-iotests/163.out

diff --git a/tests/qemu-iotests/163 b/tests/qemu-iotests/163
new file mode 100644
index 00..403842354e
--- /dev/null
+++ b/tests/qemu-iotests/163
@@ -0,0 +1,170 @@
+#!/usr/bin/env python
+#
+# Tests for shrinking images
+#
+# Copyright (c) 2016-2017 Parallels International GmbH
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import os, random, iotests, struct, qcow2
+from iotests import qemu_img, qemu_io, image_size
+
+test_img = os.path.join(iotests.test_dir, 'test.img')
+check_img = os.path.join(iotests.test_dir, 'check.img')
+
+def size_to_int(str):
+suff = ['B', 'K', 'M', 'G', 'T']
+return int(str[:-1]) * 1024**suff.index(str[-1:])
+
+class ShrinkBaseClass(iotests.QMPTestCase):
+image_len = '128M'
+shrink_size = '10M'
+chunk_size = '16M'
+refcount_bits = '16'
+
+def __qcow2_check(self, filename):
+entry_bits = 3
+entry_size = 1 << entry_bits
+l1_mask = 0x00fffe00
+div_roundup = lambda n, d: (n + d - 1) / d
+
+def split_by_n(data, n):
+for x in xrange(0, len(data), n):
+yield struct.unpack('>Q', data[x:x + n])[0] & l1_mask
+
+def check_l1_table(h, l1_data):
+l1_list = list(split_by_n(l1_data, entry_size))
+real_l1_size = div_roundup(h.size,
+   1 << (h.cluster_bits*2 - entry_size))
+used, unused = l1_list[:real_l1_size], l1_list[real_l1_size:]
+
+self.assertTrue(len(used) != 0, "Verifying l1 table content")
+self.assertFalse(any(unused), "Verifying l1 table content")
+
+def check_reftable(fd, h, reftable):
+for offset in split_by_n(reftable, entry_size):
+if offset != 0:
+fd.seek(offset)
+cluster = fd.read(1 << h.cluster_bits)
+self.assertTrue(any(cluster), "Verifying reftable content")
+
+with open(filename, "rb") as fd:
+h = qcow2.QcowHeader(fd)
+
+fd.seek(h.l1_table_offset)
+l1_table = fd.read(h.l1_size << entry_bits)
+
+fd.seek(h.refcount_table_offset)
+reftable = fd.read(h.refcount_table_clusters << h.cluster_bits)
+
+check_l1_table(h, l1_table)
+check_reftable(fd, h, reftable)
+
+def __raw_check(self, filename):
+pass
+
+image_check = {
+'qcow2' : __qcow2_check,
+'raw' : __raw_check
+}
+
+def setUp(self):
+if iotests.imgfmt == 'raw':
+qemu_img('create', '-f', iotests.imgfmt, test_img, self.image_len)
+qemu_img('create', '-f', iotests.imgfmt, check_img,
+ self.shrink_size)
+else:
+qemu_img('create', '-f', iotests.imgfmt,
+ '-o', 'cluster_size=' + self.cluster_size +
+ ',refcount_bits=' + self.refcount_bits,
+ test_img, self.image_len)
+qemu_img('create', '-f', iotests.imgfmt,
+ '-o', 'cluster_size=%s'% self.cluster_size,
+ check_img, self.shrink_size)
+qemu_io('-c', 'write -P 0xff 0 ' + self.shrink_size, check_img)
+
+def tearDown(self):
+os.remove(test_img)
+os.remove(check_img)
+
+def image_verify(self):
+self.assertEqual(image_size(test_img), image_size(check_img),
+ "Verifying image size")
+self.image_check[iotests.imgfmt](self, test_img)
+
+if iotests.imgfmt == 'raw':
+return
+self.assertEqual(qemu_img('check', test_img), 0,
+ "Verifying image corruption")
+
+def test_empty_image(self):
+qemu_img('resize',  '-f', iotests.imgfmt, '--shrink', test_img,
+ self.shrink_size)
+
+self.assertEqual(
+qemu_io('-c', 'read -P 0x00 %s'%self.shrink_size, test_img),
+qemu_io('-c', 'read -P 0x00 %s'%self.shrink_size, check_img),
+"Verifying image content")
+
+self.i

Re: [Qemu-devel] [PATCH 19/26] build-sys: move qapi variables in qapi.mak

2017-08-17 Thread Markus Armbruster
We don't move in qapi.mak we move *into* qapi.mak.

While we're talking: use a more common tag, and start the phrase with a
capital letter: "Makefile: Move qapi variables into qapi.mak".

Commit message then still misses the most important part: why?

Marc-André Lureau  writes:

> Signed-off-by: Marc-André Lureau 
> ---
>  Makefile | 43 +--
>  qapi.mak | 14 ++
>  2 files changed, 31 insertions(+), 26 deletions(-)
>  create mode 100644 qapi.mak
>
> diff --git a/Makefile b/Makefile
> index ef721480eb..8cd30fd88e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -50,6 +50,7 @@ endif
>  endif
>  
>  include $(SRC_PATH)/rules.mak
> +include $(SRC_PATH)/qapi.mak
>  
>  GENERATED_FILES = qemu-version.h config-host.h qemu-options.def
>  GENERATED_FILES += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
> @@ -390,56 +391,46 @@ qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx 
> $(SRC_PATH)/scripts/hxtool
>  qemu-ga$(EXESUF): LIBS = $(LIBS_QGA)
>  qemu-ga$(EXESUF): QEMU_CFLAGS += -I qga/qapi-generated
>  
> -gen-out-type = $(subst .,-,$(suffix $@))
> -
> -qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
> -
>  qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\
> -$(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
> +$(SRC_PATH)/qga/qapi-schema.json $(qapi-types-py)
>   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
> - $(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \
> + $(qapi-gen-type) -o qga/qapi-generated -p "qga-" $<, \
>   "GEN","$@")
>  qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\
> -$(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
> +$(SRC_PATH)/qga/qapi-schema.json $(qapi-visit-py)
>   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
> - $(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \
> + $(qapi-gen-type) -o qga/qapi-generated -p "qga-" $<, \
>   "GEN","$@")
>  qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\
> -$(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py 
> $(qapi-py)
> +$(SRC_PATH)/qga/qapi-schema.json $(qapi-commands-py)
>   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
> - $(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \
> + $(qapi-gen-type) -o qga/qapi-generated -p "qga-" $<, \
>   "GEN","$@")
>  
> -qapi-modules = $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi/common.json \
> -   $(SRC_PATH)/qapi/block.json $(SRC_PATH)/qapi/block-core.json \
> -   $(SRC_PATH)/qapi/event.json $(SRC_PATH)/qapi/introspect.json \
> -   $(SRC_PATH)/qapi/crypto.json $(SRC_PATH)/qapi/rocker.json \
> -   $(SRC_PATH)/qapi/trace.json
> -
>  qapi-types.c qapi-types.h :\
> -$(qapi-modules) $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
> +$(qapi-modules) $(qapi-types-py)
>   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
> - $(gen-out-type) -o "." -b $<, \
> + $(qapi-gen-type) -o "." -b $<, \
>   "GEN","$@")
>  qapi-visit.c qapi-visit.h :\
> -$(qapi-modules) $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
> +$(qapi-modules) $(qapi-visit-py)
>   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
> - $(gen-out-type) -o "." -b $<, \
> + $(qapi-gen-type) -o "." -b $<, \
>   "GEN","$@")
>  qapi-event.c qapi-event.h :\
> -$(qapi-modules) $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
> +$(qapi-modules) $(qapi-event-py)
>   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py \
> - $(gen-out-type) -o "." $<, \
> + $(qapi-gen-type) -o "." $<, \
>   "GEN","$@")
>  qmp-commands.h qmp-marshal.c :\
> -$(qapi-modules) $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
> +$(qapi-modules) $(qapi-commands-py)
>   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
> - $(gen-out-type) -o "." $<, \
> + $(qapi-gen-type) -o "." $<, \
>   "GEN","$@")
>  qmp-introspect.h qmp-introspect.c :\
> -$(qapi-modules) $(SRC_PATH)/scripts/qapi-introspect.py $(qapi-py)
> +$(qapi-modules) $(qapi-introspect-py)
>   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-introspect.py \
> - $(gen-out-type) -o "." $<, \
> + $(qapi-gen-type) -o "." $<, \
>   "GEN","$@")
>  
>  QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h 
> qga-qapi-visit.h qga-qmp-commands.h)
> diff --git a/qapi.mak b/qapi.mak
> new file mode 100644
> index 00..70196127d9
> --- /dev/null
> +++ b/qapi.mak
> @@ -0,0 +1,14 @@
> +qapi-gen-type = $(subst .,-,$(suffix $@))
> +
> +qapi-modules = $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi/common.json \
>

[Qemu-devel] [PATCH v1 for-2.11 01/10] target/s390x: move cc_name() to cc_helper.c

2017-08-17 Thread David Hildenbrand
While at it, move the translations into the function.

Signed-off-by: David Hildenbrand 
---
 target/s390x/cc_helper.c | 48 
 target/s390x/cpu.h   | 48 +---
 2 files changed, 49 insertions(+), 47 deletions(-)

diff --git a/target/s390x/cc_helper.c b/target/s390x/cc_helper.c
index 1cf8551..a4562e5 100644
--- a/target/s390x/cc_helper.c
+++ b/target/s390x/cc_helper.c
@@ -31,6 +31,54 @@
 #define HELPER_LOG(x...)
 #endif
 
+const char *cc_name(int cc_op)
+{
+static const char *cc_names[] = {
+[CC_OP_CONST0]= "CC_OP_CONST0",
+[CC_OP_CONST1]= "CC_OP_CONST1",
+[CC_OP_CONST2]= "CC_OP_CONST2",
+[CC_OP_CONST3]= "CC_OP_CONST3",
+[CC_OP_DYNAMIC]   = "CC_OP_DYNAMIC",
+[CC_OP_STATIC]= "CC_OP_STATIC",
+[CC_OP_NZ]= "CC_OP_NZ",
+[CC_OP_LTGT_32]   = "CC_OP_LTGT_32",
+[CC_OP_LTGT_64]   = "CC_OP_LTGT_64",
+[CC_OP_LTUGTU_32] = "CC_OP_LTUGTU_32",
+[CC_OP_LTUGTU_64] = "CC_OP_LTUGTU_64",
+[CC_OP_LTGT0_32]  = "CC_OP_LTGT0_32",
+[CC_OP_LTGT0_64]  = "CC_OP_LTGT0_64",
+[CC_OP_ADD_64]= "CC_OP_ADD_64",
+[CC_OP_ADDU_64]   = "CC_OP_ADDU_64",
+[CC_OP_ADDC_64]   = "CC_OP_ADDC_64",
+[CC_OP_SUB_64]= "CC_OP_SUB_64",
+[CC_OP_SUBU_64]   = "CC_OP_SUBU_64",
+[CC_OP_SUBB_64]   = "CC_OP_SUBB_64",
+[CC_OP_ABS_64]= "CC_OP_ABS_64",
+[CC_OP_NABS_64]   = "CC_OP_NABS_64",
+[CC_OP_ADD_32]= "CC_OP_ADD_32",
+[CC_OP_ADDU_32]   = "CC_OP_ADDU_32",
+[CC_OP_ADDC_32]   = "CC_OP_ADDC_32",
+[CC_OP_SUB_32]= "CC_OP_SUB_32",
+[CC_OP_SUBU_32]   = "CC_OP_SUBU_32",
+[CC_OP_SUBB_32]   = "CC_OP_SUBB_32",
+[CC_OP_ABS_32]= "CC_OP_ABS_32",
+[CC_OP_NABS_32]   = "CC_OP_NABS_32",
+[CC_OP_COMP_32]   = "CC_OP_COMP_32",
+[CC_OP_COMP_64]   = "CC_OP_COMP_64",
+[CC_OP_TM_32] = "CC_OP_TM_32",
+[CC_OP_TM_64] = "CC_OP_TM_64",
+[CC_OP_NZ_F32]= "CC_OP_NZ_F32",
+[CC_OP_NZ_F64]= "CC_OP_NZ_F64",
+[CC_OP_NZ_F128]   = "CC_OP_NZ_F128",
+[CC_OP_ICM]   = "CC_OP_ICM",
+[CC_OP_SLA_32]= "CC_OP_SLA_32",
+[CC_OP_SLA_64]= "CC_OP_SLA_64",
+[CC_OP_FLOGR] = "CC_OP_FLOGR",
+};
+
+return cc_names[cc_op];
+}
+
 static uint32_t cc_calc_ltgt_32(int32_t src, int32_t dst)
 {
 if (src == dst) {
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 29fdd5d..3e798ef 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -744,53 +744,7 @@ enum cc_op {
 CC_OP_MAX
 };
 
-static const char *cc_names[] = {
-[CC_OP_CONST0]= "CC_OP_CONST0",
-[CC_OP_CONST1]= "CC_OP_CONST1",
-[CC_OP_CONST2]= "CC_OP_CONST2",
-[CC_OP_CONST3]= "CC_OP_CONST3",
-[CC_OP_DYNAMIC]   = "CC_OP_DYNAMIC",
-[CC_OP_STATIC]= "CC_OP_STATIC",
-[CC_OP_NZ]= "CC_OP_NZ",
-[CC_OP_LTGT_32]   = "CC_OP_LTGT_32",
-[CC_OP_LTGT_64]   = "CC_OP_LTGT_64",
-[CC_OP_LTUGTU_32] = "CC_OP_LTUGTU_32",
-[CC_OP_LTUGTU_64] = "CC_OP_LTUGTU_64",
-[CC_OP_LTGT0_32]  = "CC_OP_LTGT0_32",
-[CC_OP_LTGT0_64]  = "CC_OP_LTGT0_64",
-[CC_OP_ADD_64]= "CC_OP_ADD_64",
-[CC_OP_ADDU_64]   = "CC_OP_ADDU_64",
-[CC_OP_ADDC_64]   = "CC_OP_ADDC_64",
-[CC_OP_SUB_64]= "CC_OP_SUB_64",
-[CC_OP_SUBU_64]   = "CC_OP_SUBU_64",
-[CC_OP_SUBB_64]   = "CC_OP_SUBB_64",
-[CC_OP_ABS_64]= "CC_OP_ABS_64",
-[CC_OP_NABS_64]   = "CC_OP_NABS_64",
-[CC_OP_ADD_32]= "CC_OP_ADD_32",
-[CC_OP_ADDU_32]   = "CC_OP_ADDU_32",
-[CC_OP_ADDC_32]   = "CC_OP_ADDC_32",
-[CC_OP_SUB_32]= "CC_OP_SUB_32",
-[CC_OP_SUBU_32]   = "CC_OP_SUBU_32",
-[CC_OP_SUBB_32]   = "CC_OP_SUBB_32",
-[CC_OP_ABS_32]= "CC_OP_ABS_32",
-[CC_OP_NABS_32]   = "CC_OP_NABS_32",
-[CC_OP_COMP_32]   = "CC_OP_COMP_32",
-[CC_OP_COMP_64]   = "CC_OP_COMP_64",
-[CC_OP_TM_32] = "CC_OP_TM_32",
-[CC_OP_TM_64] = "CC_OP_TM_64",
-[CC_OP_NZ_F32]= "CC_OP_NZ_F32",
-[CC_OP_NZ_F64]= "CC_OP_NZ_F64",
-[CC_OP_NZ_F128]   = "CC_OP_NZ_F128",
-[CC_OP_ICM]   = "CC_OP_ICM",
-[CC_OP_SLA_32]= "CC_OP_SLA_32",
-[CC_OP_SLA_64]= "CC_OP_SLA_64",
-[CC_OP_FLOGR] = "CC_OP_FLOGR",
-};
-
-static inline const char *cc_name(int cc_op)
-{
-return cc_names[cc_op];
-}
+const char *cc_name(int cc_op);
 
 static inline void setcc(S390CPU *cpu, uint64_t cc)
 {
-- 
2.9.4




[Qemu-devel] [PATCH v1 for-2.11 05/10] target/s390x: move get_per_in_range() to misc_helper.c

2017-08-17 Thread David Hildenbrand
Only used in that file.

Signed-off-by: David Hildenbrand 
---
 target/s390x/cpu.h | 11 ---
 target/s390x/misc_helper.c | 11 +++
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index d8bc134..ca10f0a 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -438,17 +438,6 @@ static inline uint8_t get_per_atmid(CPUS390XState *env)
((env->psw.mask & PSW_ASC_ACCREG)?(1 << 2) : 0);
 }
 
-/* Check if an address is within the PER starting address and the PER
-   ending address.  The address range might loop.  */
-static inline bool get_per_in_range(CPUS390XState *env, uint64_t addr)
-{
-if (env->cregs[10] <= env->cregs[11]) {
-return env->cregs[10] <= addr && addr <= env->cregs[11];
-} else {
-return env->cregs[10] <= addr || addr <= env->cregs[11];
-}
-}
-
 S390CPU *cpu_s390x_init(const char *cpu_model);
 S390CPU *s390x_new_cpu(const char *cpu_model, int64_t id, Error **errp);
 S390CPU *cpu_s390x_create(const char *cpu_model, Error **errp);
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index d23ffcd..ab0c53d 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -451,6 +451,17 @@ void HELPER(per_check_exception)(CPUS390XState *env)
 }
 }
 
+/* Check if an address is within the PER starting address and the PER
+   ending address.  The address range might loop.  */
+static inline bool get_per_in_range(CPUS390XState *env, uint64_t addr)
+{
+if (env->cregs[10] <= env->cregs[11]) {
+return env->cregs[10] <= addr && addr <= env->cregs[11];
+} else {
+return env->cregs[10] <= addr || addr <= env->cregs[11];
+}
+}
+
 void HELPER(per_branch)(CPUS390XState *env, uint64_t from, uint64_t to)
 {
 if ((env->cregs[9] & PER_CR9_EVENT_BRANCH)) {
-- 
2.9.4




Re: [Qemu-devel] [Qemu-block] [PATCH v6 0/4] Add shrink image for qcow2

2017-08-17 Thread Pavel Butsykin

On 17.08.2017 00:07, John Snow wrote:

Over a month with no replies and we're nearing the next QEMU release. If
this patchset is still applicable, can you rebase and resend for 2.11?


Thanks for digging up these patches :) I've sent the rebased version.


--js

On 07/14/2017 11:37 AM, Pavel Butsykin wrote:

This patch add shrinking of the image file for qcow2. As a result, this allows
us to reduce the virtual image size and free up space on the disk without
copying the image. Image can be fragmented and shrink is done by punching holes
in the image file.

# ./qemu-img create -f qcow2 image.qcow2 4G
Formatting 'image.qcow2', fmt=qcow2 size=4294967296 encryption=off 
cluster_size=65536 lazy_refcounts=off refcount_bits=16

# ./qemu-io -c "write -P 0x22 0 1G" image.qcow2
wrote 1073741824/1073741824 bytes at offset 0
1 GiB, 1 ops; 0:00:04.59 (222.886 MiB/sec and 0.2177 ops/sec)

# ./qemu-img resize image.qcow2 512M
warning: qemu-img: Shrinking an image will delete all data beyond the shrunken 
image's end. Before performing such an operation, make sure there is no 
important data there.
error: qemu-img: Use the --shrink option to perform a shrink operation.

# ./qemu-img resize --shrink image.qcow2 128M
Image resized.

# ./qemu-img info image.qcow2
image: image.qcow2
file format: qcow2
virtual size: 128M (134217728 bytes)
disk size: 128M
cluster_size: 65536
Format specific information:
 compat: 1.1
 lazy refcounts: false
 refcount bits: 16
 corrupt: false

# du -h image.qcow2
129Mimage.qcow2

Changes from v1:
- add --shrink flag for qemu-img resize
- add qcow2_cache_discard
- simplify qcow2_shrink_l1_table() to reduce the likelihood of image corruption
- add new qemu-iotests for shrinking images

Changes from v2:
- replace qprintf() on error_report() (1)
- rewrite warning messages (1)
- enforce --shrink flag for all formats except raw (1)
- split qcow2_cache_discard() (2)
- minor fixes according to comments (3)
- rewrite the last part of qcow2_shrink_reftable() to avoid
   qcow2_free_clusters() calls inside (3)
- improve test for shrinking image (4)

Changes from v3:
- rebase on "Implement a warning_report function" Alistair's patch-set (1)
- spelling fixes (1)
- the man page fix according to the discussion (1)
- add call qcow2_signal_corruption() in case of image corruption (3)

Changes from v4:
- rebase on https://github.com/XanClic/qemu/commits/block Max's block branch

Changes from v5:
- the condition refcount == 0 should be enough to evict the l2/refcount cluster
   from the cache (2)
- overwrite the l1/refcount table in memory with zeros, even if overwriting the
   l1/refcount table on disk has failed (3)
- replace g_try_malloc() on g_malloc() for allocation reftable_tmp (3)

Pavel Butsykin (4):
   qemu-img: add --shrink flag for resize
   qcow2: add qcow2_cache_discard
   qcow2: add shrink image support
   qemu-iotests: add shrinking image test

  block/qcow2-cache.c|  26 +++
  block/qcow2-cluster.c  |  50 +
  block/qcow2-refcount.c | 140 -
  block/qcow2.c  |  43 +---
  block/qcow2.h  |  17 +
  qapi/block-core.json   |   3 +-
  qemu-img-cmds.hx   |   4 +-
  qemu-img.c |  23 ++
  qemu-img.texi  |   6 +-
  tests/qemu-iotests/102 |   4 +-
  tests/qemu-iotests/163 | 170 +
  tests/qemu-iotests/163.out |   5 ++
  tests/qemu-iotests/group   |   1 +
  13 files changed, 475 insertions(+), 17 deletions(-)
  create mode 100644 tests/qemu-iotests/163
  create mode 100644 tests/qemu-iotests/163.out





[Qemu-devel] [PATCH v1 for-2.11 02/10] target/s390x: move cpu_mmu_idx_to_asc() to excp_helper.c

2017-08-17 Thread David Hildenbrand
Only used in that file.

Signed-off-by: David Hildenbrand 
---
 target/s390x/cpu.h | 14 --
 target/s390x/excp_helper.c | 14 ++
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 3e798ef..6567cfa 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -393,20 +393,6 @@ static inline int cpu_mmu_index(CPUS390XState *env, bool 
ifetch)
 }
 }
 
-static inline uint64_t cpu_mmu_idx_to_asc(int mmu_idx)
-{
-switch (mmu_idx) {
-case MMU_PRIMARY_IDX:
-return PSW_ASC_PRIMARY;
-case MMU_SECONDARY_IDX:
-return PSW_ASC_SECONDARY;
-case MMU_HOME_IDX:
-return PSW_ASC_HOME;
-default:
-abort();
-}
-}
-
 static inline void cpu_get_tb_cpu_state(CPUS390XState* env, target_ulong *pc,
 target_ulong *cs_base, uint32_t *flags)
 {
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index d183377..db86259 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -68,6 +68,20 @@ int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr address,
 
 #else /* !CONFIG_USER_ONLY */
 
+static inline uint64_t cpu_mmu_idx_to_asc(int mmu_idx)
+{
+switch (mmu_idx) {
+case MMU_PRIMARY_IDX:
+return PSW_ASC_PRIMARY;
+case MMU_SECONDARY_IDX:
+return PSW_ASC_SECONDARY;
+case MMU_HOME_IDX:
+return PSW_ASC_HOME;
+default:
+abort();
+}
+}
+
 int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr orig_vaddr,
   int rw, int mmu_idx)
 {
-- 
2.9.4




[Qemu-devel] [PATCH v1 for-2.11 07/10] target/s390x: move a couple of functions to cpu.c

2017-08-17 Thread David Hildenbrand
Prepare to move more stuff (especially KVM related) from cpu.h to
internal.h.

Reviewed-by: Richard Henderson 
Reviewed-by: Thomas Huth 
Signed-off-by: David Hildenbrand 
---
 target/s390x/cpu.c | 79 
 target/s390x/cpu.h | 89 ++
 2 files changed, 89 insertions(+), 79 deletions(-)

diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index dadd383..43c274d 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -392,6 +392,85 @@ unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU 
*cpu)
 
 return s390_count_running_cpus();
 }
+
+int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low)
+{
+if (kvm_enabled()) {
+return kvm_s390_get_clock(tod_high, tod_low);
+}
+/* Fixme TCG */
+*tod_high = 0;
+*tod_low = 0;
+return 0;
+}
+
+int s390_set_clock(uint8_t *tod_high, uint64_t *tod_low)
+{
+if (kvm_enabled()) {
+return kvm_s390_set_clock(tod_high, tod_low);
+}
+/* Fixme TCG */
+return 0;
+}
+
+int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit)
+{
+if (kvm_enabled()) {
+return kvm_s390_set_mem_limit(kvm_state, new_limit, hw_limit);
+}
+return 0;
+}
+
+void s390_cmma_reset(void)
+{
+if (kvm_enabled()) {
+kvm_s390_cmma_reset();
+}
+}
+
+int s390_cpu_restart(S390CPU *cpu)
+{
+if (kvm_enabled()) {
+return kvm_s390_cpu_restart(cpu);
+}
+return -ENOSYS;
+}
+
+int s390_get_memslot_count(KVMState *s)
+{
+if (kvm_enabled()) {
+return kvm_s390_get_memslot_count(s);
+} else {
+return MAX_AVAIL_SLOTS;
+}
+}
+
+int s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch_id,
+int vq, bool assign)
+{
+if (kvm_enabled()) {
+return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+} else {
+return 0;
+}
+}
+
+void s390_crypto_reset(void)
+{
+if (kvm_enabled()) {
+kvm_s390_crypto_reset();
+}
+}
+
+bool s390_get_squash_mcss(void)
+{
+if (object_property_get_bool(OBJECT(qdev_get_machine()), 
"s390-squash-mcss",
+ NULL)) {
+return true;
+}
+
+return false;
+}
 #endif
 
 static gchar *s390_gdb_arch_name(CPUState *cs)
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 1f20934..4318eb7 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -439,25 +439,8 @@ static inline void kvm_s390_access_exception(S390CPU *cpu, 
uint16_t code,
 }
 #endif
 
-static inline int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low)
-{
-if (kvm_enabled()) {
-return kvm_s390_get_clock(tod_high, tod_low);
-}
-/* Fixme TCG */
-*tod_high = 0;
-*tod_low = 0;
-return 0;
-}
-
-static inline int s390_set_clock(uint8_t *tod_high, uint64_t *tod_low)
-{
-if (kvm_enabled()) {
-return kvm_s390_set_clock(tod_high, tod_low);
-}
-/* Fixme TCG */
-return 0;
-}
+int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low);
+int s390_set_clock(uint8_t *tod_high, uint64_t *tod_low);
 
 S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
 unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu);
@@ -808,69 +791,17 @@ static inline void kvm_s390_crypto_reset(void)
 }
 #endif
 
-static inline int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit)
-{
-if (kvm_enabled()) {
-return kvm_s390_set_mem_limit(kvm_state, new_limit, hw_limit);
-}
-return 0;
-}
-
-static inline void s390_cmma_reset(void)
-{
-if (kvm_enabled()) {
-kvm_s390_cmma_reset();
-}
-}
-
-static inline int s390_cpu_restart(S390CPU *cpu)
-{
-if (kvm_enabled()) {
-return kvm_s390_cpu_restart(cpu);
-}
-return -ENOSYS;
-}
-
-static inline int s390_get_memslot_count(KVMState *s)
-{
-if (kvm_enabled()) {
-return kvm_s390_get_memslot_count(s);
-} else {
-return MAX_AVAIL_SLOTS;
-}
-}
-
+int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit);
+void s390_cmma_reset(void);
+int s390_cpu_restart(S390CPU *cpu);
+int s390_get_memslot_count(KVMState *s);
 void s390_io_interrupt(uint16_t subchannel_id, uint16_t subchannel_nr,
uint32_t io_int_parm, uint32_t io_int_word);
 void s390_crw_mchk(void);
-
-static inline int s390_assign_subch_ioeventfd(EventNotifier *notifier,
-  uint32_t sch_id, int vq,
-  bool assign)
-{
-if (kvm_enabled()) {
-return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
-} else {
-return 0;
-}
-}
-
-static inline void s390_crypto_reset(void)
-{
-if (kvm_enabled()) {
-kvm_s390_crypto_reset();
-}
-}
-
-static inline bool s390_get_squash_mcss(void)
-{
-if (object_property_get_bool(OBJECT(qdev_get_machine()), 
"s390-squash-mcss",
-

[Qemu-devel] [PATCH v1 for-2.11 04/10] target/s390x: move s390_do_cpu_reset() to diag.c

2017-08-17 Thread David Hildenbrand
Only used in that file. Also drop the comment, not really needed.

Signed-off-by: David Hildenbrand 
---
 target/s390x/cpu.h  | 7 ---
 target/s390x/diag.c | 7 +++
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 42d4ac0..d8bc134 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -490,13 +490,6 @@ static inline hwaddr decode_basedisp_s(CPUS390XState *env, 
uint32_t ipb,
 /* Base/displacement are at the same locations. */
 #define decode_basedisp_rs decode_basedisp_s
 
-/* helper functions for run_on_cpu() */
-static inline void s390_do_cpu_reset(CPUState *cs, run_on_cpu_data arg)
-{
-S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
-
-scc->cpu_reset(cs);
-}
 static inline void s390_do_cpu_full_reset(CPUState *cs, run_on_cpu_data arg)
 {
 cpu_reset(cs);
diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index 10ac845..1847cdb 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -39,6 +39,13 @@ static int modified_clear_reset(S390CPU *cpu)
 return 0;
 }
 
+static inline void s390_do_cpu_reset(CPUState *cs, run_on_cpu_data arg)
+{
+S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
+
+scc->cpu_reset(cs);
+}
+
 static int load_normal_reset(S390CPU *cpu)
 {
 S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
-- 
2.9.4




[Qemu-devel] [PATCH v1 for-2.11 00/10] target/s390x: cleanup cpu.h

2017-08-17 Thread David Hildenbrand
cpu.h is accessed outside of target/s390x. It should only contain
what is expected to be accessed outside of this folder. Therefore, create
internal.h and move a lot to that file. In addition, introduce
kvm-stub.c and kvm_390x.h for kvm specific functions.

Let's see what compilers think about this version.

RFC -> v1:
- (hopefully) fixed a compile error
- move some functions from cpu.h to the only c file they are used in
- move kvm function and stubs to kvm_s390x.h and kvm-stub.c
- smaller requested style fixes


David Hildenbrand (10):
  target/s390x: move cc_name() to cc_helper.c
  target/s390x: move cpu_mmu_idx_to_asc() to excp_helper.c
  target/s390x: move psw_key_valid() to mem_helper.c
  target/s390x: move s390_do_cpu_reset() to diag.c
  target/s390x: move get_per_in_range() to misc_helper.c
  target/s390x: introduce internal.h
  target/s390x: move a couple of functions to cpu.c
  s390x: avoid calling kvm_ functions outside of target/s390x/
  s390x/kvm: move KVM declarations and stubs to separate files
  target/s390x: cleanup cpu.h

 hw/s390x/s390-virtio-ccw.c |   4 +-
 target/s390x/Makefile.objs |   1 +
 target/s390x/arch_dump.c   |   1 +
 target/s390x/cc_helper.c   |  49 +++
 target/s390x/cpu.c |  87 ++
 target/s390x/cpu.h | 761 +
 target/s390x/cpu_models.c  |   1 +
 target/s390x/diag.c|   8 +
 target/s390x/excp_helper.c |  15 +
 target/s390x/fpu_helper.c  |   1 +
 target/s390x/gdbstub.c |   1 +
 target/s390x/helper.c  |   1 +
 target/s390x/int_helper.c  |   1 +
 target/s390x/internal.h| 391 +++
 target/s390x/interrupt.c   |   1 +
 target/s390x/ioinst.c  |   1 +
 target/s390x/kvm-stub.c| 111 +++
 target/s390x/kvm.c |   1 +
 target/s390x/kvm_s390x.h   |  51 +++
 target/s390x/machine.c |   1 +
 target/s390x/mem_helper.c  |  12 +
 target/s390x/misc_helper.c |  12 +
 target/s390x/mmu_helper.c  |   1 +
 target/s390x/translate.c   |   1 +
 24 files changed, 820 insertions(+), 694 deletions(-)
 create mode 100644 target/s390x/internal.h
 create mode 100644 target/s390x/kvm-stub.c
 create mode 100644 target/s390x/kvm_s390x.h

-- 
2.9.4




[Qemu-devel] [PATCH v1 for-2.11 03/10] target/s390x: move psw_key_valid() to mem_helper.c

2017-08-17 Thread David Hildenbrand
Only used in that file.

Signed-off-by: David Hildenbrand 
---
 target/s390x/cpu.h| 11 ---
 target/s390x/mem_helper.c | 11 +++
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 6567cfa..42d4ac0 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -366,17 +366,6 @@ void s390x_cpu_debug_excp_handler(CPUState *cs);
 #define MMU_SECONDARY_IDX   1
 #define MMU_HOME_IDX2
 
-static inline bool psw_key_valid(CPUS390XState *env, uint8_t psw_key)
-{
-uint16_t pkm = env->cregs[3] >> 16;
-
-if (env->psw.mask & PSW_MASK_PSTATE) {
-/* PSW key has range 0..15, it is valid if the bit is 1 in the PKM */
-return pkm & (0x80 >> psw_key);
-}
-return true;
-}
-
 static inline int cpu_mmu_index(CPUS390XState *env, bool ifetch)
 {
 switch (env->psw.mask & PSW_MASK_ASC) {
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index c71dce4..b91c740 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -56,6 +56,17 @@ void tlb_fill(CPUState *cs, target_ulong addr, MMUAccessType 
access_type,
 #define HELPER_LOG(x...)
 #endif
 
+static inline bool psw_key_valid(CPUS390XState *env, uint8_t psw_key)
+{
+uint16_t pkm = env->cregs[3] >> 16;
+
+if (env->psw.mask & PSW_MASK_PSTATE) {
+/* PSW key has range 0..15, it is valid if the bit is 1 in the PKM */
+return pkm & (0x80 >> psw_key);
+}
+return true;
+}
+
 /* Reduce the length so that addr + len doesn't cross a page boundary.  */
 static inline uint32_t adj_len_to_page(uint32_t len, uint64_t addr)
 {
-- 
2.9.4




[Qemu-devel] [PATCH v1 for-2.11 09/10] s390x/kvm: move KVM declarations and stubs to separate files

2017-08-17 Thread David Hildenbrand
Let's do it just like the other architectures. Introduce kvm-stub.c
for stubs and kvm_s390x.h for the declarations.

Add a fake declaration of struct kvm_s390_irq so we don't need other
ugly CONFIG_KVM checks.

Change license to GPL2+ and keep copyright notice.

Suggested-by: Thomas Huth 
Signed-off-by: David Hildenbrand 
---
 target/s390x/Makefile.objs |   1 +
 target/s390x/cpu.h | 121 +
 target/s390x/kvm-stub.c| 111 +
 target/s390x/kvm_s390x.h   |  51 +++
 4 files changed, 164 insertions(+), 120 deletions(-)
 create mode 100644 target/s390x/kvm-stub.c
 create mode 100644 target/s390x/kvm_s390x.h

diff --git a/target/s390x/Makefile.objs b/target/s390x/Makefile.objs
index f42cd1f..9615256 100644
--- a/target/s390x/Makefile.objs
+++ b/target/s390x/Makefile.objs
@@ -3,6 +3,7 @@ obj-$(CONFIG_TCG) += translate.o cc_helper.o excp_helper.o 
fpu_helper.o
 obj-$(CONFIG_TCG) += int_helper.o mem_helper.o misc_helper.o
 obj-$(CONFIG_SOFTMMU) += machine.o ioinst.o arch_dump.o mmu_helper.o diag.o
 obj-$(CONFIG_KVM) += kvm.o
+obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o
 
 # build and run feature list generator
 feat-src = $(SRC_PATH)/target/$(TARGET_BASE_ARCH)/
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 74d5b35..aeb730c 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -41,6 +41,7 @@
 #include "exec/cpu-all.h"
 
 #include "fpu/softfloat.h"
+#include "kvm_s390x.h"
 
 #define NB_MMU_MODES 3
 #define TARGET_INSN_START_EXTRA_WORDS 1
@@ -213,8 +214,6 @@ static inline S390CPU *s390_env_get_cpu(CPUS390XState *env)
 extern const struct VMStateDescription vmstate_s390_cpu;
 #endif
 
-#include "sysemu/kvm.h"
-
 /* distinguish between 24 bit and 31 bit addressing */
 #define HIGH_ORDER_BIT 0x8000
 
@@ -407,39 +406,6 @@ int cpu_s390x_signal_handler(int host_signum, void *pinfo,
 void s390_enable_css_support(S390CPU *cpu);
 int s390_virtio_hypercall(CPUS390XState *env);
 
-#ifdef CONFIG_KVM
-void kvm_s390_service_interrupt(uint32_t parm);
-void kvm_s390_vcpu_interrupt(S390CPU *cpu, struct kvm_s390_irq *irq);
-void kvm_s390_floating_interrupt(struct kvm_s390_irq *irq);
-int kvm_s390_inject_flic(struct kvm_s390_irq *irq);
-void kvm_s390_access_exception(S390CPU *cpu, uint16_t code, uint64_t te_code);
-int kvm_s390_mem_op(S390CPU *cpu, vaddr addr, uint8_t ar, void *hostbuf,
-int len, bool is_write);
-int kvm_s390_get_clock(uint8_t *tod_high, uint64_t *tod_clock);
-int kvm_s390_set_clock(uint8_t *tod_high, uint64_t *tod_clock);
-#else
-static inline void kvm_s390_service_interrupt(uint32_t parm)
-{
-}
-static inline int kvm_s390_get_clock(uint8_t *tod_high, uint64_t *tod_low)
-{
-return -ENOSYS;
-}
-static inline int kvm_s390_set_clock(uint8_t *tod_high, uint64_t *tod_low)
-{
-return -ENOSYS;
-}
-static inline int kvm_s390_mem_op(S390CPU *cpu, vaddr addr, uint8_t ar,
-  void *hostbuf, int len, bool is_write)
-{
-return -ENOSYS;
-}
-static inline void kvm_s390_access_exception(S390CPU *cpu, uint16_t code,
- uint64_t te_code)
-{
-}
-#endif
-
 int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low);
 int s390_set_clock(uint8_t *tod_high, uint64_t *tod_low);
 
@@ -706,91 +672,6 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, 
uint8_t ar, void *hostbuf,
 #define ILEN_AUTO   0xff
 void program_interrupt(CPUS390XState *env, uint32_t code, int ilen);
 
-#ifdef CONFIG_KVM
-void kvm_s390_program_interrupt(S390CPU *cpu, uint16_t code);
-void kvm_s390_io_interrupt(uint16_t subchannel_id,
-   uint16_t subchannel_nr, uint32_t io_int_parm,
-   uint32_t io_int_word);
-void kvm_s390_crw_mchk(void);
-void kvm_s390_enable_css_support(S390CPU *cpu);
-int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
-int vq, bool assign);
-int kvm_s390_cpu_restart(S390CPU *cpu);
-int kvm_s390_get_memslot_count(KVMState *s);
-int kvm_s390_cmma_active(void);
-void kvm_s390_cmma_reset(void);
-int kvm_s390_set_cpu_state(S390CPU *cpu, uint8_t cpu_state);
-void kvm_s390_reset_vcpu(S390CPU *cpu);
-int kvm_s390_set_mem_limit(KVMState *s, uint64_t new_limit, uint64_t 
*hw_limit);
-void kvm_s390_vcpu_interrupt_pre_save(S390CPU *cpu);
-int kvm_s390_vcpu_interrupt_post_load(S390CPU *cpu);
-int kvm_s390_get_ri(void);
-int kvm_s390_get_gs(void);
-void kvm_s390_crypto_reset(void);
-#else
-static inline void kvm_s390_program_interrupt(S390CPU *cpu, uint16_t code)
-{
-}
-static inline void kvm_s390_io_interrupt(uint16_t subchannel_id,
-uint16_t subchannel_nr,
-uint32_t io_int_parm,
-uint32_t io_int_word)
-{
-}
-static inline void kvm_s390_crw_mchk(void)
-{
-}
-static inline void kvm_s390_enable_

[Qemu-devel] [PATCH v1 for-2.11 06/10] target/s390x: introduce internal.h

2017-08-17 Thread David Hildenbrand
cpu.h should only contain what really has to be accessed outside of
target/s390x/. Add internal.h which can only be used inside target/s390x/.

Move everything that isn't fast enough to run away and restructure it
right away. We'll move all kvm_* stuff later.

Minor style fixes to avoid checkpatch warning to:
- struct Lowcore: "{" goes into same line as typedef
- struct LowCore: add spaces around "-" in array length calculations
- time2tod() and tod2time(): move "{" to separate line
- get_per_atmid(): add space between ")" and "?". Move cases by one char.
- get_per_atmid(): drop extra paremthesis around (1 << 6)

Change license of new file to GPL2+ and keep copyright notice.

Reviewed-by: Richard Henderson 
Signed-off-by: David Hildenbrand 
---
 target/s390x/arch_dump.c   |   1 +
 target/s390x/cc_helper.c   |   1 +
 target/s390x/cpu.c |   1 +
 target/s390x/cpu.h | 343 ---
 target/s390x/cpu_models.c  |   1 +
 target/s390x/diag.c|   1 +
 target/s390x/excp_helper.c |   1 +
 target/s390x/fpu_helper.c  |   1 +
 target/s390x/gdbstub.c |   1 +
 target/s390x/helper.c  |   1 +
 target/s390x/int_helper.c  |   1 +
 target/s390x/internal.h| 391 +
 target/s390x/interrupt.c   |   1 +
 target/s390x/ioinst.c  |   1 +
 target/s390x/kvm.c |   1 +
 target/s390x/machine.c |   1 +
 target/s390x/mem_helper.c  |   1 +
 target/s390x/misc_helper.c |   1 +
 target/s390x/mmu_helper.c  |   1 +
 target/s390x/translate.c   |   1 +
 20 files changed, 409 insertions(+), 343 deletions(-)
 create mode 100644 target/s390x/internal.h

diff --git a/target/s390x/arch_dump.c b/target/s390x/arch_dump.c
index 96c9fb9..a6a61ee 100644
--- a/target/s390x/arch_dump.c
+++ b/target/s390x/arch_dump.c
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
+#include "internal.h"
 #include "elf.h"
 #include "exec/cpu-all.h"
 #include "sysemu/dump.h"
diff --git a/target/s390x/cc_helper.c b/target/s390x/cc_helper.c
index a4562e5..9e23aa6 100644
--- a/target/s390x/cc_helper.c
+++ b/target/s390x/cc_helper.c
@@ -20,6 +20,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
+#include "internal.h"
 #include "exec/exec-all.h"
 #include "exec/helper-proto.h"
 #include "qemu/host-utils.h"
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 489bc25..dadd383 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -26,6 +26,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "cpu.h"
+#include "internal.h"
 #include "qemu-common.h"
 #include "qemu/cutils.h"
 #include "qemu/timer.h"
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index ca10f0a..1f20934 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -213,20 +213,6 @@ static inline S390CPU *s390_env_get_cpu(CPUS390XState *env)
 extern const struct VMStateDescription vmstate_s390_cpu;
 #endif
 
-void s390_cpu_do_interrupt(CPUState *cpu);
-bool s390_cpu_exec_interrupt(CPUState *cpu, int int_req);
-void s390_cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf,
- int flags);
-int s390_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
-  int cpuid, void *opaque);
-
-hwaddr s390_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
-hwaddr s390_cpu_get_phys_addr_debug(CPUState *cpu, vaddr addr);
-int s390_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
-int s390_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
-void s390_cpu_gdb_init(CPUState *cs);
-void s390x_cpu_debug_excp_handler(CPUState *cs);
-
 #include "sysemu/kvm.h"
 
 /* distinguish between 24 bit and 31 bit addressing */
@@ -390,26 +376,6 @@ static inline void cpu_get_tb_cpu_state(CPUS390XState* 
env, target_ulong *pc,
 *flags = (env->psw.mask >> FLAG_MASK_PSW_SHIFT) & FLAG_MASK_PSW;
 }
 
-#define MAX_ILEN 6
-
-/* While the PoO talks about ILC (a number between 1-3) what is actually
-   stored in LowCore is shifted left one bit (an even between 2-6).  As
-   this is the actual length of the insn and therefore more useful, that
-   is what we want to pass around and manipulate.  To make sure that we
-   have applied this distinction universally, rename the "ILC" to "ILEN".  */
-static inline int get_ilen(uint8_t opc)
-{
-switch (opc >> 6) {
-case 0:
-return 2;
-case 1:
-case 2:
-return 4;
-default:
-return 6;
-}
-}
-
 /* PER bits from control register 9 */
 #define PER_CR9_EVENT_BRANCH   0x8000
 #define PER_CR9_EVENT_IFETCH   0x4000
@@ -426,66 +392,17 @@ static inline int get_ilen(uint8_t opc)
 #define PER_CODE_EVENT_STORE_REAL  0x0800
 #define PER_CODE_EVENT_NULLIFICATION   0x0100
 
-/* Compute the ATMID field that is stored in the per_perc_atmid lowcore
-   entry when a PER exception is triggered.  */
-static inline uint8_t get_per_atmid(CPUS390XState *env)
-{
-return ((env->psw.mask & PSW_MASK_64) ?  (1 <

[Qemu-devel] [PATCH v1 for-2.11 08/10] s390x: avoid calling kvm_ functions outside of target/s390x/

2017-08-17 Thread David Hildenbrand
Let's just introduce an helper.

Reviewed-by: Richard Henderson 
Reviewed-by: Thomas Huth 
Signed-off-by: David Hildenbrand 
---
 hw/s390x/s390-virtio-ccw.c | 4 +---
 target/s390x/cpu.c | 7 +++
 target/s390x/cpu.h | 1 +
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 1c7af39..ac087ab 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -142,9 +142,7 @@ static void ccw_init(MachineState *machine)
 /* register hypercalls */
 virtio_ccw_register_hcalls();
 
-if (kvm_enabled()) {
-kvm_s390_enable_css_support(s390_cpu_addr2state(0));
-}
+s390_enable_css_support(s390_cpu_addr2state(0));
 /*
  * Non mcss-e enabled guests only see the devices from the default
  * css, which is determined by the value of the squash_mcss property.
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 43c274d..0aeab06 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -471,6 +471,13 @@ bool s390_get_squash_mcss(void)
 
 return false;
 }
+
+void s390_enable_css_support(S390CPU *cpu)
+{
+if (kvm_enabled()) {
+kvm_s390_enable_css_support(cpu);
+}
+}
 #endif
 
 static gchar *s390_gdb_arch_name(CPUState *cs)
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 4318eb7..74d5b35 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -404,6 +404,7 @@ int cpu_s390x_signal_handler(int host_signum, void *pinfo,
 
 #ifndef CONFIG_USER_ONLY
 
+void s390_enable_css_support(S390CPU *cpu);
 int s390_virtio_hypercall(CPUS390XState *env);
 
 #ifdef CONFIG_KVM
-- 
2.9.4




Re: [Qemu-devel] [PATCH 18/26] qapi: add conditions to REPLICATION type/commands on the schema

2017-08-17 Thread Markus Armbruster
Marc-André Lureau  writes:

> Add #if defined(CONFIG_REPLICATION) in generated code, and adjust the
> code accordingly.
>
> Signed-off-by: Marc-André Lureau 
> ---
>  qapi-schema.json | 12 
>  migration/colo.c | 14 ++
>  monitor.c|  5 -
>  3 files changed, 10 insertions(+), 21 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index bcee3157b0..2f4528c769 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -6337,7 +6337,8 @@
>  # Since: 2.9
>  ##
>  { 'command': 'xen-set-replication',
> -  'data': { 'enable': 'bool', 'primary': 'bool', '*failover' : 'bool' } }
> +  'data': { 'enable': 'bool', 'primary': 'bool', '*failover' : 'bool' },
> +  'if': 'defined(CONFIG_REPLICATION)' }
>  
>  ##
>  # @ReplicationStatus:
> @@ -6352,7 +6353,8 @@
>  # Since: 2.9
>  ##
>  { 'struct': 'ReplicationStatus',
> -  'data': { 'error': 'bool', '*desc': 'str' } }
> +  'data': { 'error': 'bool', '*desc': 'str' },
> +  'if': 'defined(CONFIG_REPLICATION)' }
>  
>  ##
>  # @query-xen-replication-status:
> @@ -6369,7 +6371,8 @@
>  # Since: 2.9
>  ##
>  { 'command': 'query-xen-replication-status',
> -  'returns': 'ReplicationStatus' }
> +  'returns': 'ReplicationStatus',
> +  'if': 'defined(CONFIG_REPLICATION)' }
>  
>  ##
>  # @xen-colo-do-checkpoint:
> @@ -6385,7 +6388,8 @@
>  #
>  # Since: 2.9
>  ##
> -{ 'command': 'xen-colo-do-checkpoint' }
> +{ 'command': 'xen-colo-do-checkpoint',
> +  'if': 'defined(CONFIG_REPLICATION)' }
>  
>  ##
>  # @GICCapability:
> diff --git a/migration/colo.c b/migration/colo.c
> index a4255432ac..3bff9fc9a4 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -147,11 +147,11 @@ void colo_do_failover(MigrationState *s)
>  }
>  }
>  
> +#ifdef CONFIG_REPLICATION
>  void qmp_xen_set_replication(bool enable, bool primary,
>   bool has_failover, bool failover,
>   Error **errp)
>  {
> -#ifdef CONFIG_REPLICATION
>  ReplicationMode mode = primary ?
> REPLICATION_MODE_PRIMARY :
> REPLICATION_MODE_SECONDARY;
> @@ -170,14 +170,10 @@ void qmp_xen_set_replication(bool enable, bool primary,
>  }
>  replication_stop_all(failover, failover ? NULL : errp);
>  }
> -#else
> -abort();
> -#endif
>  }
>  
>  ReplicationStatus *qmp_query_xen_replication_status(Error **errp)
>  {
> -#ifdef CONFIG_REPLICATION
>  Error *err = NULL;
>  ReplicationStatus *s = g_new0(ReplicationStatus, 1);
>  
> @@ -192,19 +188,13 @@ ReplicationStatus 
> *qmp_query_xen_replication_status(Error **errp)
>  
>  error_free(err);
>  return s;
> -#else
> -abort();
> -#endif
>  }
>  
>  void qmp_xen_colo_do_checkpoint(Error **errp)
>  {
> -#ifdef CONFIG_REPLICATION
>  replication_do_checkpoint_all(errp);
> -#else
> -abort();
> -#endif
>  }
> +#endif
>  
>  static void colo_send_message(QEMUFile *f, COLOMessage msg,
>Error **errp)
> diff --git a/monitor.c b/monitor.c
> index 4bf6a3ea2e..383c84d162 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -970,11 +970,6 @@ static void qmp_query_qmp_schema(QDict *qdict, QObject 
> **ret_data,
>   */
>  static void qmp_unregister_commands_hack(void)
>  {
> -#ifndef CONFIG_REPLICATION
> -qmp_unregister_command(&qmp_commands, "xen-set-replication");
> -qmp_unregister_command(&qmp_commands, "query-xen-replication-status");
> -qmp_unregister_command(&qmp_commands, "xen-colo-do-checkpoint");
> -#endif
>  #ifndef TARGET_I386
>  qmp_unregister_command(&qmp_commands, "rtc-reset-reinjection");
>  #endif

Same drill as for PATCH 16.

Commands you make conditional:

* xen-set-replication, query-xen-replication-status,
  xen-colo-do-checkpoint

  Before the patch, we first register the commands unconditionally in
  generated code (requires a stub), then conditionally unregister in
  qmp_unregister_commands_hack().

  Afterwards, we register only when CONFIG_REPLICATION.  The command
  fails exactly the same, with CommandNotFound.

  Improvement, because now query-qmp-schema is accurate, and we're one
  step close to killing qmp_unregister_commands_hack().

Check for completeness by reviewing the case insensitive occurrences of
replication in the schema not covered by your patch, and review uses of
CONFIG_REPLICATION for possible schema connections (I did that for
CONFIG_VNC [PATCH 16] and CONFIG_SPICE [PATCH 17], too):

* enum BlockdevDriver value "replication" in command blockdev-add

* Makefile.objs:block-obj-$(CONFIG_REPLICATION) += replication.o
  block/Makefile.objs:block-obj-$(CONFIG_REPLICATION) += replication.o

I think the following should be ifdef CONFIG_REPLICATION: enum
BlockdevDriver value @replication BlockdevOptions variant @replication,
BlockdevOptionsReplication, BlockdevOptionsReplicationMode.

Some of this analysis should perhaps be worked into the commit message.



[Qemu-devel] [PATCH v1 for-2.11 10/10] target/s390x: cleanup cpu.h

2017-08-17 Thread David Hildenbrand
Let's reshuffle the function prototypes so we get a cleaner outline
of the files.

Reviewed-by: Richard Henderson 
Signed-off-by: David Hildenbrand 
---
 target/s390x/cpu.h | 140 ++---
 1 file changed, 69 insertions(+), 71 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index aeb730c..3650cfc 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -391,48 +391,6 @@ static inline void cpu_get_tb_cpu_state(CPUS390XState* 
env, target_ulong *pc,
 #define PER_CODE_EVENT_STORE_REAL  0x0800
 #define PER_CODE_EVENT_NULLIFICATION   0x0100
 
-S390CPU *cpu_s390x_init(const char *cpu_model);
-S390CPU *s390x_new_cpu(const char *cpu_model, int64_t id, Error **errp);
-
-/* you can call this signal handler from your SIGBUS and SIGSEGV
-   signal handlers to inform the virtual CPU of exceptions. non zero
-   is returned if the signal was handled by the virtual CPU.  */
-int cpu_s390x_signal_handler(int host_signum, void *pinfo,
-   void *puc);
-
-
-#ifndef CONFIG_USER_ONLY
-
-void s390_enable_css_support(S390CPU *cpu);
-int s390_virtio_hypercall(CPUS390XState *env);
-
-int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low);
-int s390_set_clock(uint8_t *tod_high, uint64_t *tod_low);
-
-S390CPU *s390_cpu_addr2state(uint16_t cpu_addr);
-unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu);
-
-void gtod_save(QEMUFile *f, void *opaque);
-int gtod_load(QEMUFile *f, void *opaque, int version_id);
-
-/* service interrupts are floating therefore we must not pass an cpustate */
-void s390_sclp_extint(uint32_t parm);
-
-#else
-static inline unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu)
-{
-return 0;
-}
-#endif
-
-extern void subsystem_reset(void);
-
-#define cpu_init(model) CPU(cpu_s390x_init(model))
-#define cpu_signal_handler cpu_s390x_signal_handler
-
-void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf);
-#define cpu_list s390_cpu_list
-
 #define EXCP_EXT 1 /* external interrupt */
 #define EXCP_SVC 2 /* supervisor call (syscall) */
 #define EXCP_PGM 3 /* program interruption */
@@ -652,39 +610,10 @@ struct sysib_322 {
 /* SIGP order code mask corresponding to bit positions 56-63 */
 #define SIGP_ORDER_MASK 0x00ff
 
-int sclp_service_call(CPUS390XState *env, uint64_t sccb, uint32_t code);
-
-int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
- int len, bool is_write);
-
-#define s390_cpu_virt_mem_read(cpu, laddr, ar, dest, len)\
-s390_cpu_virt_mem_rw(cpu, laddr, ar, dest, len, false)
-#define s390_cpu_virt_mem_write(cpu, laddr, ar, dest, len)   \
-s390_cpu_virt_mem_rw(cpu, laddr, ar, dest, len, true)
-#define s390_cpu_virt_mem_check_write(cpu, laddr, ar, len)   \
-s390_cpu_virt_mem_rw(cpu, laddr, ar, NULL, len, true)
-
 /* from s390-virtio-ccw */
 #define MEM_SECTION_SIZE 0x1000UL
 #define MAX_AVAIL_SLOTS  32
 
-/* automatically detect the instruction length */
-#define ILEN_AUTO   0xff
-void program_interrupt(CPUS390XState *env, uint32_t code, int ilen);
-
-
-int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit);
-void s390_cmma_reset(void);
-int s390_cpu_restart(S390CPU *cpu);
-int s390_get_memslot_count(KVMState *s);
-void s390_io_interrupt(uint16_t subchannel_id, uint16_t subchannel_nr,
-   uint32_t io_int_parm, uint32_t io_int_word);
-void s390_crw_mchk(void);
-int s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch_id,
-int vq, bool assign);
-void s390_crypto_reset(void);
-bool s390_get_squash_mcss(void);
-
 /* machine check interruption code */
 
 /* subclasses */
@@ -730,4 +659,73 @@ bool s390_get_squash_mcss(void);
 #define MCIC_VB_CT 0x0002ULL
 #define MCIC_VB_CC 0x0001ULL
 
+
+/* cpu.c */
+int s390_get_clock(uint8_t *tod_high, uint64_t *tod_low);
+int s390_set_clock(uint8_t *tod_high, uint64_t *tod_low);
+void s390_crypto_reset(void);
+bool s390_get_squash_mcss(void);
+int s390_get_memslot_count(KVMState *s);
+int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit);
+void s390_cmma_reset(void);
+int s390_cpu_restart(S390CPU *cpu);
+void s390_enable_css_support(S390CPU *cpu);
+int s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch_id,
+int vq, bool assign);
+#ifndef CONFIG_USER_ONLY
+unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu);
+#else
+static inline unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu)
+{
+return 0;
+}
+#endif /* CONFIG_USER_ONLY */
+
+
+/* cpu_models.c */
+void s390_cpu_list(FILE *f, fprintf_function cpu_fprintf);
+#define cpu_list s390_cpu_list
+
+
+/* helper.c */
+S390CPU *cpu_s390x_init(const char *cpu_model);
+#define cpu_init(model) CPU(cpu_s390x_init(model))
+S390CPU *s390x_new_cpu(const char *cpu_model, int64_t id, Error **errp);
+/* y

Re: [Qemu-devel] [PATCH 16/26] qapi: add conditions to VNC type/commands/events on the schema

2017-08-17 Thread Dr. David Alan Gilbert
* Markus Armbruster (arm...@redhat.com) wrote:
> "Dr. David Alan Gilbert"  writes:
> 
> > * Marc-André Lureau (marcandre.lur...@redhat.com) wrote:
> >> Add #if defined(CONFIG_VNC) in generated code, and adjust the
> >> qmp/hmp code accordingly.
> >> 
> >> Signed-off-by: Marc-André Lureau 
> >
> >> diff --git a/hmp.c b/hmp.c
> >> index fd80dce758..9454c634bd 100644
> >> --- a/hmp.c
> >> +++ b/hmp.c
> >> @@ -605,6 +605,7 @@ void hmp_info_blockstats(Monitor *mon, const QDict 
> >> *qdict)
> >>  qapi_free_BlockStatsList(stats_list);
> >>  }
> >>  
> >> +#ifdef CONFIG_VNC
> >>  /* Helper for hmp_info_vnc_clients, _servers */
> >>  static void hmp_info_VncBasicInfo(Monitor *mon, VncBasicInfo *info,
> >>const char *name)
> >> @@ -692,6 +693,12 @@ void hmp_info_vnc(Monitor *mon, const QDict *qdict)
> >>  qapi_free_VncInfo2List(info2l);
> >>  
> >>  }
> >> +#else
> >> +void hmp_info_vnc(Monitor *mon, const QDict *qdict)
> >> +{
> >> +warn_report("VNC support is disabled");
> 
> error_report(), please (see below).
> 
> >> +}
> >> +#endif
> >
> > I'm OK with this, so
> >
> > Acked-by: Dr. David Alan Gilbert 
> >
> > although you might just be able to add a #ifdef in hmp-commands-info.hx
> > and avoid the is disabled function, or you might find that with the QMP
> > returning an error the HMP just passes that error on.
> 
> Let's compare failures when !CONFIG_VNC:
> 
> (a) Marc-André's patch as is:
> 
> (qemu) info vnc
> warning: VNC support is disabled
> 
> Drop the "warning: " (because it ain't; the command failed), and this
> is fine.
> 
> (b) Compiling them out completely (#ifdef in hmp-commands*.hx):
> 
> unknown command: 'vnc'
> 
> HMP bug; should be something like
> 
> Unknown command: 'info vnc'
> 
> but that's not this series' problem.

I'll fix that missing 'info'

Dave

> Good enough for me.
> 
> (c) Forwarding the QMP error verbatim
> 
> The command query-vnc has not been found
> 
> No good.
> 
> (d) Handling CommandNotFound
> 
> More work than (a) for the same result.
> 
> As far as I'm concerned, feel free to do (a) or (b).
> 
> [...]
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH 1/6] tests: Run filter-redirector and -mirror test only on POSIX systems

2017-08-17 Thread David Hildenbrand
On 17.08.2017 08:25, Thomas Huth wrote:
> This way we can get rid of the ugly #ifdefs in the code which makes
> it easier to extend later.
> 
> Signed-off-by: Thomas Huth 


Reviewed-by: David Hildenbrand 

-- 

Thanks,

David



Re: [Qemu-devel] [PATCH for-2.11] intel_iommu: fix missing BQL in pt fast path

2017-08-17 Thread Paolo Bonzini
On 17/08/2017 07:56, Peter Xu wrote:
> In vtd_switch_address_space() we did the memory region switch, however
> it's possible that the caller of it has not taken the BQL at all. Make
> sure we have it.
> 
> CC: Paolo Bonzini 
> CC: Jason Wang 
> CC: Michael S. Tsirkin 
> Signed-off-by: Peter Xu 
> ---
> 
> Paolo: I noticed this qemu_mutex_iothread_locked() function, which might
> simplify the fix, so I decided to use it. Using bottom half should be ok
> as well, but after a second thought it can be complicated: consider the
> case when guest firstly triggered the pt fast path then quickly
> re-enables the IOMMU region before the bottom half being executed. Then
> looks like we need special care on the sync of bottom half task as well.

No, we don't, because the bottom half (as you correctly do below) would
only have to cover vtd_switch_address_space.  So the worst that can
happen is that on of the two calls to vtd_switch_address_space does nothing.

The patch below is okay.  However, vtd_switch_address_space is
expensive, which is why I suggested the bottom half.

Paolo

> That's over-complicated I guess (if with that, I'd prefer to remove the
> pt fast path since it's even not really the default path when pt is
> used...). Please let me know if you don't think so.
> ---
>  hw/i386/intel_iommu.c | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index a7bf87a..3a5bb0b 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -957,6 +957,8 @@ static bool vtd_dev_pt_enabled(VTDAddressSpace *as)
>  static bool vtd_switch_address_space(VTDAddressSpace *as)
>  {
>  bool use_iommu;
> +/* Whether we need to take the BQL on our own */
> +bool take_bql = !qemu_mutex_iothread_locked();
>  
>  assert(as);
>  
> @@ -967,6 +969,15 @@ static bool vtd_switch_address_space(VTDAddressSpace *as)
> VTD_PCI_FUNC(as->devfn),
> use_iommu);
>  
> +/*
> + * It's possible that we reach here without BQL, e.g., when called
> + * from vtd_pt_enable_fast_path(). However the memory APIs need
> + * it. We'd better make sure we have had it already, or, take it.
> + */
> +if (take_bql) {
> +qemu_mutex_lock_iothread();
> +}
> +
>  /* Turn off first then on the other */
>  if (use_iommu) {
>  memory_region_set_enabled(&as->sys_alias, false);
> @@ -976,6 +987,10 @@ static bool vtd_switch_address_space(VTDAddressSpace *as)
>  memory_region_set_enabled(&as->sys_alias, true);
>  }
>  
> +if (take_bql) {
> +qemu_mutex_unlock_iothread();
> +}
> +
>  return use_iommu;
>  }
>  
> 




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

2017-08-17 Thread David Hildenbrand
On 17.08.2017 10:53, Cornelia Huck wrote:
> On Thu, 17 Aug 2017 08:25:10 +0200
> 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.
> 
> I don't think we should leak the aliasing stuff into tests, but rather
> specify the transport on a per-architecture basis explicitly. (We might
> want to test virtio-pci on s390x in the future as well -- in addition
> to virtio-ccw, not replacing it.)

I also remember that using virtio aliases should be avoided (e.g. we are
not supposed to introduce new ones)

> 
> Also, the same question as for the previous patch: Is this supposed to
> test virtio explicitly, or do we just want a reasonable device?


-- 

Thanks,

David



Re: [Qemu-devel] [PATCH 4/6] tests: Introduce generic device hot-plug/hot-unplug functions

2017-08-17 Thread David Hildenbrand
On 17.08.2017 08:25, Thomas Huth wrote:
> A lot of tests provide code for adding and removing a device via the
> device_add and device_del QMP commands. Maintaining this code in so
> many places is cumbersome and error-prone (some of the code parts
> check the responses in an incorrect way, for example), so let's
> provide some proper generic qtest functions for adding and removing a
> device instead.
> 
> Signed-off-by: Thomas Huth 
> ---
>  tests/libqos/pci.c | 19 ++-
>  tests/libqos/usb.c | 30 +--
>  tests/libqtest.c   | 60 
> ++
>  tests/libqtest.h   | 19 +++
>  tests/usb-hcd-uhci-test.c  | 26 ++--
>  tests/usb-hcd-xhci-test.c  | 51 ---
>  tests/virtio-scsi-test.c   | 24 ++-
>  tests/virtio-serial-test.c | 25 +++
>  8 files changed, 98 insertions(+), 156 deletions(-)
> 
> diff --git a/tests/libqos/pci.c b/tests/libqos/pci.c
> index 2dcdead..aada753 100644
> --- a/tests/libqos/pci.c
> +++ b/tests/libqos/pci.c
> @@ -394,21 +394,6 @@ QPCIBar qpci_legacy_iomap(QPCIDevice *dev, uint16_t addr)
>  void qpci_plug_device_test(const char *driver, const char *id,
> uint8_t slot, const char *opts)
>  {
> -QDict *response;
> -char *cmd;
> -
> -cmd = g_strdup_printf("{'execute': 'device_add',"
> -  " 'arguments': {"
> -  "   'driver': '%s',"
> -  "   'addr': '%d',"
> -  "   %s%s"
> -  "   'id': '%s'"
> -  "}}", driver, slot,
> -  opts ? opts : "", opts ? "," : "",
> -  id);
> -response = qmp(cmd);
> -g_free(cmd);
> -g_assert(response);
> -g_assert(!qdict_haskey(response, "error"));
> -QDECREF(response);
> +qtest_hot_plug_device(driver, id, "'addr': '%d'%s%s", slot,
> +  opts ? ", " : "", opts ? opts : "");
>  }
> diff --git a/tests/libqos/usb.c b/tests/libqos/usb.c
> index 0cdfaec..f8d0190 100644
> --- a/tests/libqos/usb.c
> +++ b/tests/libqos/usb.c
> @@ -40,34 +40,16 @@ void uhci_port_test(struct qhc *hc, int port, uint16_t 
> expect)
>  void usb_test_hotplug(const char *hcd_id, const int port,
>void (*port_check)(void))
>  {
> -QDict *response;
> -char  *cmd;
> +char  *id = g_strdup_printf("usbdev%d", port);
>  
> -cmd = g_strdup_printf("{'execute': 'device_add',"
> -  " 'arguments': {"
> -  "   'driver': 'usb-tablet',"
> -  "   'port': '%d',"
> -  "   'bus': '%s.0',"
> -  "   'id': 'usbdev%d'"
> -  "}}", port, hcd_id, port);
> -response = qmp(cmd);
> -g_free(cmd);
> -g_assert(response);
> -g_assert(!qdict_haskey(response, "error"));
> -QDECREF(response);
> +qtest_hot_plug_device("usb-tablet", id, "'port': '%d', 'bus': '%s.0'",
> +  port, hcd_id);
>  
>  if (port_check) {
>  port_check();
>  }
>  
> -cmd = g_strdup_printf("{'execute': 'device_del',"
> -   " 'arguments': {"
> -   "   'id': 'usbdev%d'"
> -   "}}", port);
> -response = qmp(cmd);
> -g_free(cmd);
> -g_assert(response);
> -g_assert(qdict_haskey(response, "event"));
> -g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED"));
> -QDECREF(response);
> +qtest_hot_unplug_device(id);
> +
> +g_free(id);
>  }
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index b9a1f18..4339d97 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -987,3 +987,63 @@ void qtest_cb_for_every_machine(void (*cb)(const char 
> *machine))
>  qtest_end();
>  QDECREF(response);
>  }
> +
> +/**
> + * Generic hot-plugging test via the device_add QMP command
> + */
> +void qtest_hot_plug_device(const char *driver, const char *id,
> +   const char *fmt, ...)
> +{
> +QDict *response;
> +char *cmd, *opts = NULL;
> +va_list va;
> +
> +if (fmt) {
> +va_start(va, fmt);
> +opts = g_strdup_vprintf(fmt, va);
> +va_end(va);
> +}
> +
> +cmd = g_strdup_printf("{'execute': 'device_add',"
> +  " 'arguments': { 'driver': '%s', 'id': '%s'%s%s 
> }}",
> +  driver, id, opts ? ", " : "", opts ? opts : "");
> +g_free(opts);
> +
> +response = qmp(cmd);
> +g_free(cmd);
> +g_assert(response);
> +while (qdict_haskey(response, "event")) {
> +/* We can get DEVICE_DELETED events in case something went wrong */
> +g_assert_cmpstr(qdict_get_str(response, "event"), !=, 
> "DEVICE_DELETED");
> +QDECRE

Re: [Qemu-devel] [PATCH 1/6] tests: Run filter-redirector and -mirror test only on POSIX systems

2017-08-17 Thread Zhang Chen



On 08/17/2017 02:25 PM, Thomas Huth wrote:

This way we can get rid of the ugly #ifdefs in the code which makes
it easier to extend later.

Signed-off-by: Thomas Huth 


Reviewed-by: Zhang Chen

It's better for my code.

Thanks
Zhang Chen


---
  tests/Makefile.include |  8 
  tests/test-filter-mirror.c |  5 -
  tests/test-filter-redirector.c | 10 --
  3 files changed, 4 insertions(+), 19 deletions(-)



--
Thanks
Zhang Chen






Re: [Qemu-devel] [PATCH RFC v2 01/10] gitignore: Ignore vm test images

2017-08-17 Thread Stefan Hajnoczi
On Thu, Aug 17, 2017 at 10:47:37AM +0800, Fam Zheng wrote:
> Signed-off-by: Fam Zheng 
> ---
>  .gitignore | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH RFC v2 03/10] qemu.py: Add "wait()" method

2017-08-17 Thread Stefan Hajnoczi
On Thu, Aug 17, 2017 at 10:47:39AM +0800, Fam Zheng wrote:
> Signed-off-by: Fam Zheng 
> ---
>  scripts/qemu.py | 7 +++
>  1 file changed, 7 insertions(+)

It would be nice to unify this with shutdown() but it's okay for now.

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH RFC v2 02/10] qemu.py: Add variable vga type

2017-08-17 Thread Stefan Hajnoczi
On Thu, Aug 17, 2017 at 10:47:38AM +0800, Fam Zheng wrote:
> Some guests behave differently when no VGA is detected. Add a variable
> to allow override the "none" default.
> 
> Signed-off-by: Fam Zheng 
> ---
>  scripts/qemu.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH RFC v2 04/10] tests: Add vm test lib

2017-08-17 Thread Fam Zheng
> +self._args = [ \
> +"-nodefaults", "-enable-kvm", "-m", "2G",
> +"-smp", os.environ.get("J", "4"), "-cpu", "host",

Forgot to change "J" to a command argument. Will fix when dropping RFC.

Fam



Re: [Qemu-devel] [Qemu-arm] [PATCH 4/8] boards.h: Define new flag ignore_memory_transaction_failures

2017-08-17 Thread Peter Maydell
On 5 August 2017 at 11:13, Peter Maydell  wrote:
> On 4 August 2017 at 20:23, Richard Henderson
>  wrote:
>> On 08/04/2017 11:09 AM, Philippe Mathieu-Daudé wrote:
>>> Since create_unimplemented_device() register overlapped with low priority, 
>>> why
>>> not register it as default device directly, over the whole address space?
>>
>> That's a good suggestion.  It makes more sense to me than adding a flag on 
>> the
>> MachineClass.
>
> Yeah, I did think about implementing it that way, but...
>
> That wouldn't handle the case of a device model directly
> returning a MEMTX_ERROR, or a transaction dispatched to
> a memory region whose MemoryRegionOps valid settings
> prohibit it (eg byte accesses to a word-access-only device),
> or accesses to a MemoryRegion that was created by passing
> a NULL MemoryRegionOps pointer to memory_region_init_io
> (I dunno why you'd do that but some code does).
>
> In short, there are lots of ways the memory subsystem might
> end up returning a transaction error -- this mechanism
> ensures that none of them start generating exceptions
> when they previously did not, and is (I hope) easy to
> review in the sense of being sure that it does what it
> intends to do without the need to audit a lot of corner
> cases.

So, this question (should we have a board flag to disable reporting
of tx failures to the CPU hook, or use unimplemented_device as a
sort of background region) seems to be the main unanswered question
for this series. I think (as outlined above) that the board flag
is simpler and safer; are people happy for me to put this series
in target-arm.next with that approach, or should I rethink this bit?

thanks
-- PMM



Re: [Qemu-devel] [PATCH v3 1/5] ppc: spapr: Register and handle HCALL to receive updated RTAS region

2017-08-17 Thread Nikunj A Dadhania
David Gibson  writes:

> On Wed, Aug 16, 2017 at 02:42:13PM +0530, Aravinda Prasad wrote:
>> Receive updates from SLOF about the updated rtas-base.
>> A separate patch for SLOF [1] adds functionality to invoke
>> a private HCALL whenever OS issues instantiate-rtas with
>> a new rtas-base.
>> 
>> This is required as QEMU needs to know the updated rtas-base
>> as it allocates error reporting structure in RTAS space upon
>> a machine check exception.
>> 
>> [1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-August/120386.html
>> 
>> Signed-off-by: Aravinda Prasad 
>> Reviewed-by: David Gibson 
>
> Actually, I take back this R-b, see below.
>
> In any case I'm not willing to apply the patches which depend on this
> until the corresponding SLOF update is merged as well.

We already have this sitting around in SLOF for a while

https://git.qemu.org/gitweb.cgi?p=SLOF.git;a=commit;f=lib/libhvcall/libhvcall.h;h=f9a60de30492863811c2cdf6f28988c9e8a2c3d9

Regards
Nikunj




[Qemu-devel] [PATCH] scsi-block: Add qdev error properties

2017-08-17 Thread Fam Zheng
This makes the werror/rerror options available on the scsi-block device,
to allow user specify error handling policy in the same way as scsi-hd
etc.

Signed-off-by: Fam Zheng 
---
 hw/scsi/scsi-disk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 5f1e5e8070..27d917f7c3 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2972,6 +2972,7 @@ static const TypeInfo scsi_cd_info = {
 
 #ifdef __linux__
 static Property scsi_block_properties[] = {
+DEFINE_BLOCK_ERROR_PROPERTIES(SCSIDiskState, qdev.conf), \
 DEFINE_PROP_DRIVE("drive", SCSIDiskState, qdev.conf.blk),
 DEFINE_PROP_END_OF_LIST(),
 };
-- 
2.13.4




Re: [Qemu-devel] [PATCH] hw/arm: use defined type name instead of hard-coded string

2017-08-17 Thread Peter Maydell
On 31 July 2017 at 13:09, Peter Maydell  wrote:
> On 27 July 2017 at 14:48, Philippe Mathieu-Daudé  wrote:
>> On 07/27/2017 09:36 AM, Peter Maydell wrote:
>>>
>>> On 17 July 2017 at 14:30, Philippe Mathieu-Daudé  wrote:

 Signed-off-by: Philippe Mathieu-Daudé 
 ---

 Hi Peter, this patch is waiting the IDE queue to enters since it depends
 of
 commit de09efcc7b67ada3593354711b12a03dea23a9d0 to fix AHCI includes.
>>>
>>>
>>> There's no commit with that hash in master -- do you have
>>> a commit subject or something that I can use to find out
>>> whether it's landed or not?
>>
>>
>> Oops I guess the IDE queue was rebased between this patch and the merge, or
>> I messed my copy/paste :)
>>
>> The dependent commit is 70e2337030f and has how entered /master, do you want
>> me to send an v2 with fixed message? Anyway I delayed this patch for 2.11
>> since this is not a bugfix.
>
> You don't need to resend, I'll just keep it on my list of things
> to deal with after 2.11. (Anything below the '---' isn't really
> in the commit message anyway, it gets stripped out when the
> patch is applied.)

Patch now in target-arm.next ready for post-2.11 pullreq.

thanks
-- PMM



Re: [Qemu-devel] [PATCH v2 0/4] ARM: KVM: Enable in-kernel PMU with user space gic

2017-08-17 Thread Peter Maydell
On 8 August 2017 at 14:23, Andrew Jones  wrote:
>
> Peter,
>
> ping?

I've applied this to target-arm.next ready for the first 2.11
ARM pullreq. Sorry for the delay, I'd put not-for-2.10 patch
review onto the back burner since I knew nothing was going to
go into master til after the release...

thanks
-- PMM



[Qemu-devel] [PATCH 1/2] hmp: Missing handle_errors

2017-08-17 Thread Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" 

hmp_info_memdev && hmp_info_memory_devices were missing
hmp_handle_error calls.  Add them.

Signed-off-by: Dr. David Alan Gilbert 
---
 hmp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hmp.c b/hmp.c
index 10400386e0..dc6cf7d583 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2395,6 +2395,7 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
 monitor_printf(mon, "\n");
 
 qapi_free_MemdevList(memdev_list);
+hmp_handle_error(mon, &err);
 }
 
 void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)
@@ -2433,6 +2434,7 @@ void hmp_info_memory_devices(Monitor *mon, const QDict 
*qdict)
 }
 
 qapi_free_MemoryDeviceInfoList(info_list);
+hmp_handle_error(mon, &err);
 }
 
 void hmp_info_iothreads(Monitor *mon, const QDict *qdict)
-- 
2.13.5




[Qemu-devel] [PATCH 2/2] hmp: Fix unknown command for subtable

2017-08-17 Thread Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" 

(qemu) info foo
unknown command: 'foo'

fix this to:
(qemu) info foo
unknown command: 'info foo'

Reported-by: Markus Armbruster 
Signed-off-by: Dr. David Alan Gilbert 
---
 monitor.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/monitor.c b/monitor.c
index e0f880107f..0695bd59d4 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2693,6 +2693,7 @@ static const mon_cmd_t *search_dispatch_table(const 
mon_cmd_t *disp_table,
  * the command is found in a sub-command table.
  */
 static const mon_cmd_t *monitor_parse_command(Monitor *mon,
+  const char *cmdp_start,
   const char **cmdp,
   mon_cmd_t *table)
 {
@@ -2708,7 +2709,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
*mon,
 cmd = search_dispatch_table(table, cmdname);
 if (!cmd) {
 monitor_printf(mon, "unknown command: '%.*s'\n",
-   (int)(p - *cmdp), *cmdp);
+   (int)(p - cmdp_start), cmdp_start);
 return NULL;
 }
 
@@ -2720,7 +2721,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
*mon,
 *cmdp = p;
 /* search sub command */
 if (cmd->sub_table != NULL && *p != '\0') {
-return monitor_parse_command(mon, cmdp, cmd->sub_table);
+return monitor_parse_command(mon, cmdp_start, cmdp, cmd->sub_table);
 }
 
 return cmd;
@@ -3104,7 +3105,7 @@ static void handle_hmp_command(Monitor *mon, const char 
*cmdline)
 
 trace_handle_hmp_command(mon, cmdline);
 
-cmd = monitor_parse_command(mon, &cmdline, mon->cmd_table);
+cmd = monitor_parse_command(mon, cmdline, &cmdline, mon->cmd_table);
 if (!cmd) {
 return;
 }
-- 
2.13.5




[Qemu-devel] [PATCH 0/2 for 2.11] Minor HMP fixes

2017-08-17 Thread Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" 

A couple of minor HMP fixes

Dr. David Alan Gilbert (2):
  hmp: Missing handle_errors
  hmp: Fix unknown command for subtable

 hmp.c | 2 ++
 monitor.c | 7 ---
 2 files changed, 6 insertions(+), 3 deletions(-)

-- 
2.13.5




Re: [Qemu-devel] [PATCH for 2.11 v2 0/2] wdt_aspeed: Support reset width patterns

2017-08-17 Thread Peter Maydell
On 9 August 2017 at 07:28, Andrew Jeffery  wrote:
> Hello,
>
> These two patches add support for the reset width configuration register in 
> the
> Aspeed watchdog. Initially this was just one patch[1], but I've reworked it as
> two to explicitly support the varying capabilities between Aspeed SoC 
> versions.
>
> Andrew
>
> [1] http://patchwork.ozlabs.org/patch/796039/
>
> Andrew Jeffery (2):
>   watchdog: wdt_aspeed: Add support for the reset width register
>   aspeed_soc: Propagate silicon-rev to watchdog



Applied to target-arm.next ready for when 2.11 development opens, thanks.

-- PMM



Re: [Qemu-devel] [PATCH] hw/ppc: disable hotplug before CAS is completed

2017-08-17 Thread David Gibson
On Tue, Aug 15, 2017 at 05:28:46PM -0300, Daniel Henrique Barboza wrote:
> This patch is a follow up on the discussions that started with
> Laurent's patch series "spapr: disable hotplugging without OS" [1]
> and discussions made at patch "spapr: reset DRCs on migration
> pre_load" [2].
> 
> At this moment, we do not support CPU/memory hotplug in early
> boot stages, before CAS. The reason is that the hotplug event
> can't be handled at SLOF level (or even in PRELAUNCH runstate) and
> at the same time can't be canceled. This leads to devices being
> unable to be hot unplugged and, in some cases, guest kernel Ooops.
> After CAS, with the FDT in place, the guest can handle the hotplug
> events and everything works as usual.
> 
> An attempt to try to support hotplug before CAS was made, but not
> successful. The key difference in the current code flow between a
> coldplugged and a hotplugged device, in the PRELAUNCH state, is that
> the coldplugged device is registered at the base FDT, allowing its
> DRC to go straight to CONFIGURED state. In theory, this can also be
> done with a hotplugged device if we can add it to the base of the
> existing FDT. However, tampering with the FDT after writing in the
> guest memory, besides being a dubitable idea, is also not
> possible. The FDT is written in ppc_spapr_reset and there is no
> way to retrieve it - we can calculate the fdt_address but the
> fdt_size isn't stored. Storing the fdt_size to allow for
> later retrieval is yet another state that would need to be
> migrated. In short, it is not worth the trouble.
> 
> All this said, this patch opted to disable CPU/mem hotplug at early
> boot stages. CAS detection is made by checking if there are
> any bits set in ov5_cas to avoid adding an extra state that
> would need tracking/migration. The patch also makes sure that
> it doesn't interfere with hotplug in the INMIGRATE state.
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg05226.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg01989.html
> 
> Signed-off-by: Daniel Henrique Barboza 

I don't think this is a good idea.

1) After my DRC cleanups, early hotplug works just fine for me.  I'm
not sure why it isn't for you: we need to understand that before
proceeding.

2) libvirt actually uses early hotplug fairly often (before even
starting the firmware).  At the moment this works - at least in some
cases (see above), though there are some wrinkles to work out.  This
will break it completely and require an entirely different approach to
fix again.

3) There's no fundamental reason early hotplug shouldn't work - the
event will just be queued until the OS boots and processes it.

I know I suggested disabling early hotplug earlier, but that was
before I'd dug into the DRC layer and properly understood what was
going on here.

-- 
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 02/28] mips: MIPSCPU model subclasses

2017-08-17 Thread Igor Mammedov
On Thu, 17 Aug 2017 00:38:59 -0300
Philippe Mathieu-Daudé  wrote:

> Hi Igor,
> 
> On 07/15/2017 06:48 PM, Philippe Mathieu-Daudé wrote:
> > On 07/14/2017 10:51 AM, Igor Mammedov wrote:  
> >> 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   
> > 
> > Reviewed-by: Philippe Mathieu-Daudé 
> >   
> >> ---
> >> CC: Aurelien Jarno 
> >> CC: Yongbok Kim 
> >> ---
> >>   target/mips/cpu-qom.h|  2 ++
> >>   target/mips/cpu.h| 57 
> >> +++-
> >>   target/mips/cpu.c| 51 
> >> +++
> >>   target/mips/translate.c  | 13 +-
> >>   target/mips/translate_init.c | 57 
> >> ++--
> >>   5 files changed, 117 insertions(+), 63 deletions(-)
> >>
> >> diff --git a/target/mips/cpu-qom.h b/target/mips/cpu-qom.h
> >> index 3f5bf23..4b32401 100644
> >> --- a/target/mips/cpu-qom.h
> >> +++ b/target/mips/cpu-qom.h
> >> @@ -35,6 +35,7 @@
> >>   #define MIPS_CPU_GET_CLASS(obj) \
> >>   OBJECT_GET_CLASS(MIPSCPUClass, (obj), TYPE_MIPS_CPU)
> >> +typedef struct mips_def_t mips_def_t;
> >>   /**
> >>* MIPSCPUClass:
> >>* @parent_realize: The parent class' realize handler.
> >> @@ -49,6 +50,7 @@ typedef struct MIPSCPUClass {
> >>   DeviceRealize parent_realize;
> >>   void (*parent_reset)(CPUState *cpu);
> >> +const mips_def_t *cpu_def;
> >>   } MIPSCPUClass;
> >>   typedef struct MIPSCPU MIPSCPU;
> >> diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> >> index 9c32228..7c2e0bf 100644
> >> --- a/target/mips/cpu.h
> >> +++ b/target/mips/cpu.h
> >> @@ -161,7 +161,62 @@ struct CPUMIPSMVPContext {
> >>   #define CP0MVPC1_PCP10
> >>   };
> >> -typedef struct mips_def_t mips_def_t;
> >> +/* 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;
> >> +int insn_flags;
> >> +enum mips_mmu_types mmu_type;
> >> +};
> >> +
> >> +extern const struct mips_def_t mips_defs[];
> >> +extern const int mips_defs_number;
> >>   #define MIPS_SHADOW_SET_MAX 16
> >>   #define MIPS_TC_MAX 5
> >> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> >> index 82afdaa..111b5ae 100644
> >> --- a/target/mips/cpu.c
> >> +++ b/target/mips/cpu.c
> >> @@ -151,12 +151,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)
> >> @@ -171,6 +196,7 @@ static void mips_cpu_class_init(ObjectClass

Re: [Qemu-devel] [PATCH 4/6] tests: Introduce generic device hot-plug/hot-unplug functions

2017-08-17 Thread Thomas Huth
On 17.08.2017 11:53, David Hildenbrand wrote:
> On 17.08.2017 08:25, Thomas Huth wrote:
>> A lot of tests provide code for adding and removing a device via the
>> device_add and device_del QMP commands. Maintaining this code in so
>> many places is cumbersome and error-prone (some of the code parts
>> check the responses in an incorrect way, for example), so let's
>> provide some proper generic qtest functions for adding and removing a
>> device instead.
>>
>> Signed-off-by: Thomas Huth 
>> ---
[...]
>> diff --git a/tests/libqos/pci.c b/tests/libqos/pci.c
>> index 2dcdead..aada753 100644
>> --- a/tests/libqos/pci.c
>> +++ b/tests/libqos/pci.c
>> @@ -394,21 +394,6 @@ QPCIBar qpci_legacy_iomap(QPCIDevice *dev, uint16_t 
>> addr)
>>  void qpci_plug_device_test(const char *driver, const char *id,
>> uint8_t slot, const char *opts)
>>  {
>> -QDict *response;
>> -char *cmd;
>> -
>> -cmd = g_strdup_printf("{'execute': 'device_add',"
>> -  " 'arguments': {"
>> -  "   'driver': '%s',"
>> -  "   'addr': '%d',"
>> -  "   %s%s"
>> -  "   'id': '%s'"
>> -  "}}", driver, slot,
>> -  opts ? opts : "", opts ? "," : "",
>> -  id);
>> -response = qmp(cmd);
>> -g_free(cmd);
>> -g_assert(response);
>> -g_assert(!qdict_haskey(response, "error"));
>> -QDECREF(response);
>> +qtest_hot_plug_device(driver, id, "'addr': '%d'%s%s", slot,
>> +  opts ? ", " : "", opts ? opts : "");

While PCI uses 'addr' as additional parameter here...

[...]
>> +qtest_hot_plug_device("usb-tablet", id, "'port': '%d', 'bus': '%s.0'",
>> +  port, hcd_id);

... USB uses 'port' and 'bus' ...

[...]
>
> Not sure if it would be better to avoid the fmt and provide more
> parameters instead. Specifying a parameter as NULL will ifgnore it.
> 
> void qtest_hot_plug_device(const char *driver, const char *id,
>const char *drive, const char *port,
>const char *bus)
> 
> that should cover all cases and callers don't have to build strings in a
> special format.

... so I'm afraid, but no, this does not work. The additional parameters
are specific to the device type, so there is no way to get around the
format string if we want to stay flexible here.

 Thomas



[Qemu-devel] [PATCHv3 03/03] colo-compare: Update the COLO document to add the IOThread configuration

2017-08-17 Thread Wang yong
From: Wang Yong 

Update colo-proxy.txt,add IOThread configuration.
Later we have to configure IOThread,if not COLO can not work.

Signed-off-by: Wang Yong 
Signed-off-by: Wang Guang 
---
 docs/colo-proxy.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/docs/colo-proxy.txt b/docs/colo-proxy.txt
index c4941de..ce3f783 100644
--- a/docs/colo-proxy.txt
+++ b/docs/colo-proxy.txt
@@ -170,10 +170,11 @@ Primary(ip:3.3.3.3):
 -chardev socket,id=compare0-0,host=3.3.3.3,port=9001
 -chardev socket,id=compare_out,host=3.3.3.3,port=9005,server,nowait
 -chardev socket,id=compare_out0,host=3.3.3.3,port=9005
+-object iothread,id=iothread1
 -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0
 -object filter-redirector,netdev=hn0,id=redire0,queue=rx,indev=compare_out
 -object filter-redirector,netdev=hn0,id=redire1,queue=rx,outdev=compare0
--object 
colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0
+-object 
colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,outdev=compare_out0,iothread=iothread1
 
 Secondary(ip:3.3.3.8):
 -netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,down script=/etc/qemu-ifdown
-- 
1.8.3.1





[Qemu-devel] [PATCHv3 00/03] Replace the COLO comparing thread with IOThread

2017-08-17 Thread Wang yong
From: Wang Yong 

It's a good idea to use IOThread instead of COLO comparing thread.
comparing thread can be completely replaced by IOThread, so this idea came.

This series of updates mainly include IOThread supports the GMainContext
event loop, then the old packet regularly check and primary/secondary network
packets compare all into the IOThread processing.

Please review,thanks.

wangyong(3):
qemu-iothread: IOThread supports the GMainContext event loop
colo-compare: Use IOThread to Check old packet regularly and
 Process pactkets of the primary
colo-compare: Update the COLO document to add the IOThread
 configuration

 include/sysemu/iothread.h | 10 +
 iothread.c| 54 +++
 net/colo-compare.c | 75 --
 docs/colo-proxy.txt | 3 ++- 
 4 file changed, 86 insertions(+), 37 deletions(-)

--
1.8.3.1




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

2017-08-17 Thread Wang yong
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| 41 +
 2 files changed, 45 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..80ef830 100644
--- a/iothread.c
+++ b/iothread.c
@@ -57,6 +57,20 @@ static void *iothread_run(void *opaque)
 
 while (!atomic_read(&iothread->stopping)) {
 aio_poll(iothread->ctx, true);
+
+if (atomic_read(&iothread->worker_context)) {
+g_main_context_push_thread_default(iothread->worker_context);
+iothread->main_loop =
+g_main_loop_new(iothread->worker_context, TRUE);
+g_main_loop_run(iothread->main_loop);
+
+g_main_loop_unref(iothread->main_loop);
+iothread->main_loop = NULL;
+
+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 +87,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 +142,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;
 
 /* This assumes we are called from a thread with useful CPU affinity for us
  * to inherit.
@@ -309,3 +327,26 @@ 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);
+
+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 v1 for-2.11 01/10] target/s390x: move cc_name() to cc_helper.c

2017-08-17 Thread Thomas Huth
On 17.08.2017 11:22, David Hildenbrand wrote:
> While at it, move the translations into the function.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/cc_helper.c | 48 
> 
>  target/s390x/cpu.h   | 48 
> +---
>  2 files changed, 49 insertions(+), 47 deletions(-)

Reviewed-by: Thomas Huth 



[Qemu-devel] [PATCHv3 02/03] colo-compare: Use IOThread to Check old packet regularly and Process pactkets of the primary

2017-08-17 Thread Wang yong
From: Wang Yong 

Remove the task which check old packet in the comparing thread,
then use IOthread context timer to handle it.

Process pactkets in the IOThread which arrived over the socket.
we use iothread_get_g_main_context to create a new g_main_loop in
the IOThread.then the packets from the primary and the secondary
are processed in the IOThread.

Finally remove the colo-compare thread using the IOThread instead.

Signed-off-by: Wang Yong 
Signed-off-by: Wang Guang 
---
 net/colo-compare.c | 75 --
 1 file changed, 39 insertions(+), 36 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 5fe8e3f..69cb16e 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -29,6 +29,7 @@
 #include "qemu/sockets.h"
 #include "qapi-visit.h"
 #include "net/colo.h"
+#include "sysemu/iothread.h"
 
 #define TYPE_COLO_COMPARE "colo-compare"
 #define COLO_COMPARE(obj) \
@@ -82,11 +83,10 @@ typedef struct CompareState {
 GQueue conn_list;
 /* hashtable to save connection */
 GHashTable *connection_track_table;
-/* compare thread, a thread for each NIC */
-QemuThread thread;
 
+IOThread *iothread;
 GMainContext *worker_context;
-GMainLoop *compare_loop;
+QEMUTimer *packet_check_timer;
 } CompareState;
 
 typedef struct CompareClass {
@@ -597,22 +597,40 @@ static void compare_sec_chr_in(void *opaque, const 
uint8_t *buf, int size)
  * Check old packet regularly so it can watch for any packets
  * that the secondary hasn't produced equivalents of.
  */
-static gboolean check_old_packet_regular(void *opaque)
+static void check_old_packet_regular(void *opaque)
 {
 CompareState *s = opaque;
 
 /* if have old packet we will notify checkpoint */
 colo_old_packet_check(s);
+timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+REGULAR_PACKET_CHECK_MS);
+}
+
+static void colo_compare_timer_init(CompareState *s)
+{
+AioContext *ctx = iothread_get_aio_context(s->iothread);
 
-return TRUE;
+s->packet_check_timer = aio_timer_new(ctx, QEMU_CLOCK_VIRTUAL,
+SCALE_MS, check_old_packet_regular,
+s);
+timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
+REGULAR_PACKET_CHECK_MS);
 }
 
-static void *colo_compare_thread(void *opaque)
+static void colo_compare_timer_del(CompareState *s)
 {
-CompareState *s = opaque;
-GSource *timeout_source;
+if (s->packet_check_timer) {
+timer_del(s->packet_check_timer);
+timer_free(s->packet_check_timer);
+s->packet_check_timer = NULL;
+}
+ }
 
-s->worker_context = g_main_context_new();
+static void colo_compare_iothread(CompareState *s)
+{
+object_ref(OBJECT(s->iothread));
+s->worker_context = iothread_get_g_main_context(s->iothread);
 
 qemu_chr_fe_set_handlers(&s->chr_pri_in, compare_chr_can_read,
  compare_pri_chr_in, NULL, NULL,
@@ -621,20 +639,7 @@ static void *colo_compare_thread(void *opaque)
  compare_sec_chr_in, NULL, NULL,
  s, s->worker_context, true);
 
-s->compare_loop = g_main_loop_new(s->worker_context, FALSE);
-
-/* To kick any packets that the secondary doesn't match */
-timeout_source = g_timeout_source_new(REGULAR_PACKET_CHECK_MS);
-g_source_set_callback(timeout_source,
-  (GSourceFunc)check_old_packet_regular, s, NULL);
-g_source_attach(timeout_source, s->worker_context);
-
-g_main_loop_run(s->compare_loop);
-
-g_source_unref(timeout_source);
-g_main_loop_unref(s->compare_loop);
-g_main_context_unref(s->worker_context);
-return NULL;
+colo_compare_timer_init(s);
 }
 
 static char *compare_get_pri_indev(Object *obj, Error **errp)
@@ -759,12 +764,10 @@ static void colo_compare_complete(UserCreatable *uc, 
Error **errp)
 {
 CompareState *s = COLO_COMPARE(uc);
 Chardev *chr;
-char thread_name[64];
-static int compare_id;
 
-if (!s->pri_indev || !s->sec_indev || !s->outdev) {
+if (!s->pri_indev || !s->sec_indev || !s->outdev || !s->iothread) {
 error_setg(errp, "colo compare needs 'primary_in' ,"
-   "'secondary_in','outdev' property set");
+   "'secondary_in','outdev','iothread' property set");
 return;
 } else if (!strcmp(s->pri_indev, s->outdev) ||
!strcmp(s->sec_indev, s->outdev) ||
@@ -799,12 +802,7 @@ static void colo_compare_complete(UserCreatable *uc, Error 
**errp)
   g_free,
   connection_destroy);
 
-sprintf(thread_name, "colo-compare %d", compare_id);
-qemu_thread_create(&s->thread, thread_name,
-   colo_compare_thread, s,
-   QEMU_THREAD_JOINABLE);
-compare

Re: [Qemu-devel] [PATCH v1 for-2.11 02/10] target/s390x: move cpu_mmu_idx_to_asc() to excp_helper.c

2017-08-17 Thread Thomas Huth
On 17.08.2017 11:22, David Hildenbrand wrote:
> Only used in that file.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/cpu.h | 14 --
>  target/s390x/excp_helper.c | 14 ++
>  2 files changed, 14 insertions(+), 14 deletions(-)

Reviewed-by: Thomas Huth 



Re: [Qemu-devel] Guest init virtio-serial port failed randomly, qemu-kvm report error about guest failure in adding device virtio-serial0.0

2017-08-17 Thread Dr. David Alan Gilbert
* 皮智 (pizhi0...@gmail.com) wrote:
> Hi,I meet a problem with vm(windows10), my environment is qemu-kvm-2.3,
> virtio-win-0.1.126, host(centos7.2), guest(windows10).
> I add four virtio-serial port to vm, if I hard reboot the vm(windows10), it
> may happen as follows randomly:

I'm not too sure how virtio-win version numbering works; but:
https://bugzilla.redhat.com/show_bug.cgi?id=819412

looks like the same symptom, and says it was fixed in ~2013.

Dave

> 
> 2017-08-14T07:51:18.401008Z qemu-kvm: virtio-serial-bus: Unexpected port id
> 1122983520 for device virtio-serial0.0
> 2017-08-14T07:51:18.956482Z qemu-kvm: virtio-serial-bus: Guest failure in
> adding device virtio-serial0.0
> 2017-08-14T07:51:29.176795Z qemu-kvm: virtio-serial-bus: Guest failure in
> adding device virtio-serial0.0
> 2017-08-14T07:51:34.454819Z qemu-kvm: virtio-serial-bus: Unexpected port id
> 2042451128 for device virtio-serial0.0
> 2017-08-14T07:51:44.655068Z qemu-kvm: virtio-serial-bus: Guest failure in
> adding device virtio-serial0.0
> 2017-08-14T07:51:49.764201Z qemu-kvm: virtio-serial-bus: Unexpected port id
> 1122983520 for device virtio-serial0.0
> 2017-08-14T07:51:59.969836Z qemu-kvm: virtio-serial-bus: Guest failure in
> adding device virtio-serial0.0
> 2017-08-14T07:52:21.564461Z qemu-kvm: virtio-serial-bus: Unexpected port id
> 1122983520 for device virtio-serial0.0
> 2017-08-14T07:52:47.059452Z qemu-kvm: virtio-serial-bus: Guest failure in
> adding device virtio-serial0.0
> 2017-08-14T07:53:22.710677Z qemu-kvm: virtio-serial-bus: Unexpected port id
> 1122983520 for device virtio-serial0.0
> 2017-08-14T07:53:32.781204Z qemu-kvm: virtio-serial-bus: Guest failure in
> adding device virtio-serial0.0
> 2017-08-14T07:53:48.040513Z qemu-kvm: virtio-serial-bus: Unexpected port id
> 3179968584 for device virtio-serial0.0
> 2017-08-14T07:54:23.547393Z qemu-kvm: virtio-serial-bus: Unexpected port id
> 1122983520 for device virtio-serial0.0
> 2017-08-14T07:55:54.350754Z qemu-kvm: virtio-serial-bus: Unexpected port id
> 1122983520 for device virtio-serial0.0
> 2017-08-14T07:56:09.393056Z qemu-kvm: virtio-serial-bus: Unexpected port id
> 1122983520 for device virtio-serial0.0
> 2017-08-14T07:57:39.670850Z qemu-kvm: virtio-serial-bus: Unexpected port id
> 1122983520 for device virtio-serial0.0
> 2017-08-14T07:58:09.867650Z qemu-kvm: virtio-serial-bus: Unexpected port id
> 1185792 for device virtio-serial0.0
> 2017-08-14T07:59:10.053208Z qemu-kvm: virtio-serial-bus: Unexpected port id
> 1122983520 for device virtio-serial0.0
> 2017-08-14T08:00:20.235162Z qemu-kvm: virtio-serial-bus: Guest failure in
> adding device virtio-serial0.0
> 2017-08-14T08:01:20.394196Z qemu-kvm: virtio-serial-bus: Guest failure in
> adding device virtio-serial0.0
> 
> If qemu-kvm report upper error, it will cause guest failed to init
> virtio-serial port.
> Thanks.
> 
> Looking forward to your reply.
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH v7 3/4] qcow2: add shrink image support

2017-08-17 Thread Eric Blake
On 08/17/2017 04:15 AM, Pavel Butsykin wrote:
> This patch add shrinking of the image file for qcow2. As a result, this allows
> us to reduce the virtual image size and free up space on the disk without
> copying the image. Image can be fragmented and shrink is done by punching 
> holes
> in the image file.
> 
> Signed-off-by: Pavel Butsykin 
> Reviewed-by: Max Reitz 
> ---

> +++ b/qapi/block-core.json
> @@ -2495,7 +2495,8 @@
>  'cluster_alloc_bytes', 'cluster_free', 'flush_to_os',
>  'flush_to_disk', 'pwritev_rmw_head', 'pwritev_rmw_after_head',
>  'pwritev_rmw_tail', 'pwritev_rmw_after_tail', 'pwritev',
> -'pwritev_zero', 'pwritev_done', 'empty_image_prepare' ] }
> +'pwritev_zero', 'pwritev_done', 'empty_image_prepare',
> +'l1_shrink_write_table', 'l1_shrink_free_l2_clusters' ] }

Missing documentation of the new enum members (at a minimum, something
that says 'since 2.11').

-- 
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 v1 for-2.11 04/10] target/s390x: move s390_do_cpu_reset() to diag.c

2017-08-17 Thread Thomas Huth
On 17.08.2017 11:22, David Hildenbrand wrote:
> Only used in that file. Also drop the comment, not really needed.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/cpu.h  | 7 ---
>  target/s390x/diag.c | 7 +++
>  2 files changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Thomas Huth 





Re: [Qemu-devel] [PATCH v1 for-2.11 03/10] target/s390x: move psw_key_valid() to mem_helper.c

2017-08-17 Thread Thomas Huth
On 17.08.2017 11:22, David Hildenbrand wrote:
> Only used in that file.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/cpu.h| 11 ---
>  target/s390x/mem_helper.c | 11 +++
>  2 files changed, 11 insertions(+), 11 deletions(-)

Reviewed-by: Thomas Huth 




Re: [Qemu-devel] [PATCH v1 for-2.11 05/10] target/s390x: move get_per_in_range() to misc_helper.c

2017-08-17 Thread Thomas Huth
On 17.08.2017 11:22, David Hildenbrand wrote:
> Only used in that file.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/cpu.h | 11 ---
>  target/s390x/misc_helper.c | 11 +++
>  2 files changed, 11 insertions(+), 11 deletions(-)

Reviewed-by: Thomas Huth 



Re: [Qemu-devel] [PATCH 1/2] hmp: Missing handle_errors

2017-08-17 Thread Eric Blake
On 08/17/2017 05:42 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> hmp_info_memdev && hmp_info_memory_devices were missing
> hmp_handle_error calls.  Add them.
> 
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  hmp.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Eric Blake 

-- 
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 2/2] hmp: Fix unknown command for subtable

2017-08-17 Thread Eric Blake
On 08/17/2017 05:42 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> (qemu) info foo
> unknown command: 'foo'
> 
> fix this to:
> (qemu) info foo
> unknown command: 'info foo'
> 
> Reported-by: Markus Armbruster 
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  monitor.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
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 02/28] mips: MIPSCPU model subclasses

2017-08-17 Thread Philippe Mathieu-Daudé

On 08/17/2017 07:53 AM, Igor Mammedov wrote:

On Thu, 17 Aug 2017 00:38:59 -0300
Philippe Mathieu-Daudé  wrote:


Hi Igor,

On 07/15/2017 06:48 PM, Philippe Mathieu-Daudé wrote:

On 07/14/2017 10:51 AM, Igor Mammedov wrote:

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 


Reviewed-by: Philippe Mathieu-Daudé 
   

---

[...]

+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)
@@ -171,6 +196,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;


Now than I'm reading again...


   cc->has_work = mips_cpu_has_work;
   cc->do_interrupt = mips_cpu_do_interrupt;
   cc->cpu_exec_interrupt = mips_cpu_exec_interrupt;
@@ -203,9 +229,34 @@ static const TypeInfo mips_cpu_type_info = {


Shouldn't this class now be abstract?

it should,

I see your are fixing it in your version of series,
so I'll just drop mips from my series so that you could merge
your version of mips part separately via your tree,
assuming you can do it fast once merge window is open
as I have another series on top that does more extensive
generalization/clean up and depends on this series
(including mips being properly QOMified)


Ok I'll squash it there.



Re: [Qemu-devel] [PATCH 4/6] tests: Introduce generic device hot-plug/hot-unplug functions

2017-08-17 Thread David Hildenbrand
On 17.08.2017 12:57, Thomas Huth wrote:
> On 17.08.2017 11:53, David Hildenbrand wrote:
>> On 17.08.2017 08:25, Thomas Huth wrote:
>>> A lot of tests provide code for adding and removing a device via the
>>> device_add and device_del QMP commands. Maintaining this code in so
>>> many places is cumbersome and error-prone (some of the code parts
>>> check the responses in an incorrect way, for example), so let's
>>> provide some proper generic qtest functions for adding and removing a
>>> device instead.
>>>
>>> Signed-off-by: Thomas Huth 
>>> ---
> [...]
>>> diff --git a/tests/libqos/pci.c b/tests/libqos/pci.c
>>> index 2dcdead..aada753 100644
>>> --- a/tests/libqos/pci.c
>>> +++ b/tests/libqos/pci.c
>>> @@ -394,21 +394,6 @@ QPCIBar qpci_legacy_iomap(QPCIDevice *dev, uint16_t 
>>> addr)
>>>  void qpci_plug_device_test(const char *driver, const char *id,
>>> uint8_t slot, const char *opts)
>>>  {
>>> -QDict *response;
>>> -char *cmd;
>>> -
>>> -cmd = g_strdup_printf("{'execute': 'device_add',"
>>> -  " 'arguments': {"
>>> -  "   'driver': '%s',"
>>> -  "   'addr': '%d',"
>>> -  "   %s%s"
>>> -  "   'id': '%s'"
>>> -  "}}", driver, slot,
>>> -  opts ? opts : "", opts ? "," : "",
>>> -  id);
>>> -response = qmp(cmd);
>>> -g_free(cmd);
>>> -g_assert(response);
>>> -g_assert(!qdict_haskey(response, "error"));
>>> -QDECREF(response);
>>> +qtest_hot_plug_device(driver, id, "'addr': '%d'%s%s", slot,
>>> +  opts ? ", " : "", opts ? opts : "");
> 
> While PCI uses 'addr' as additional parameter here...

so addr would be needed in addition.

> 
> [...]
>>> +qtest_hot_plug_device("usb-tablet", id, "'port': '%d', 'bus': '%s.0'",
>>> +  port, hcd_id);
> 
> ... USB uses 'port' and 'bus' ...
> 
> [...]
>>
>> Not sure if it would be better to avoid the fmt and provide more
>> parameters instead. Specifying a parameter as NULL will ifgnore it.
>>
>> void qtest_hot_plug_device(const char *driver, const char *id,
>>const char *drive, const char *port,
>>const char *bus)
>>
>> that should cover all cases and callers don't have to build strings in a
>> special format.
> 
> ... so I'm afraid, but no, this does not work. The additional parameters
> are specific to the device type, so there is no way to get around the
> format string if we want to stay flexible here.

It will work, but yes it is not flexible. Just an idea because I don't
like having to specify json strings on a function.

> 
>  Thomas
> 


-- 

Thanks,

David



[Qemu-devel] [PATCH v2] spapr: fallback to raw mode if best compat mode cannot be set during CAS

2017-08-17 Thread Greg Kurz
KVM PR doesn't allow to set a compat mode. This causes ppc_set_compat_all()
to fail and we return H_HARDWARE to the guest right away.

This is excessive: even if we favor compat mode since commit 152ef803ceb19,
we should at least fallback to raw mode if the guest supports it.

This patch modifies cas_check_pvr() so that it also reports that the real
PVR was found in the table supplied by the guest. Note that this is only
makes sense if raw mode isn't explicitely disabled (ie, the user didn't
set the machine "max-cpu-compat" property). If this is the case, we can
simply ignore ppc_set_compat_all() failures, and let the guest run in raw
mode.

Signed-off-by: Greg Kurz 
---
v2: - initialize raw_mode_supported to silent patchew
---
 hw/ppc/spapr_hcall.c |   18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 07b3da8dc4cd..2f4c4f59e110 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1441,7 +1441,8 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
 }
 
 static uint32_t cas_check_pvr(sPAPRMachineState *spapr, PowerPCCPU *cpu,
-  target_ulong *addr, Error **errp)
+  target_ulong *addr, bool *raw_mode_supported,
+  Error **errp)
 {
 bool explicit_match = false; /* Matched the CPU's real PVR */
 uint32_t max_compat = spapr->max_compat_pvr;
@@ -1481,6 +1482,8 @@ static uint32_t cas_check_pvr(sPAPRMachineState *spapr, 
PowerPCCPU *cpu,
 return 0;
 }
 
+*raw_mode_supported = explicit_match;
+
 /* Parsing finished */
 trace_spapr_cas_pvr(cpu->compat_pvr, explicit_match, best_compat);
 
@@ -1499,8 +1502,9 @@ static target_ulong 
h_client_architecture_support(PowerPCCPU *cpu,
 sPAPROptionVector *ov1_guest, *ov5_guest, *ov5_cas_old, *ov5_updates;
 bool guest_radix;
 Error *local_err = NULL;
+bool raw_mode_supported = false;
 
-cas_pvr = cas_check_pvr(spapr, cpu, &addr, &local_err);
+cas_pvr = cas_check_pvr(spapr, cpu, &addr, &raw_mode_supported, 
&local_err);
 if (local_err) {
 error_report_err(local_err);
 return H_HARDWARE;
@@ -1510,8 +1514,14 @@ static target_ulong 
h_client_architecture_support(PowerPCCPU *cpu,
 if (cpu->compat_pvr != cas_pvr) {
 ppc_set_compat_all(cas_pvr, &local_err);
 if (local_err) {
-error_report_err(local_err);
-return H_HARDWARE;
+/* We fail to set compat mode (likely because running with KVM PR),
+ * but maybe we can fallback to raw mode if the guest supports it.
+ */
+if (!raw_mode_supported) {
+error_report_err(local_err);
+return H_HARDWARE;
+}
+local_err = NULL;
 }
 }
 




[Qemu-devel] [Bug 821078] Re: virtio-serial-bus: Unexpected port id

2017-08-17 Thread Dr. David Alan Gilbert
This looks like https://bugzilla.redhat.com/show_bug.cgi?id=819412
which is about the same age.

** Bug watch added: Red Hat Bugzilla #819412
   https://bugzilla.redhat.com/show_bug.cgi?id=819412

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

Title:
  virtio-serial-bus: Unexpected port id

Status in QEMU:
  Expired

Bug description:
  With qemu-kvm-0.15.0-rc1 virtio-serial-bus reports an error, and windows 
vdagent can not start.  qemu-0.15.0-rc1 behaves as expected, ie vdagent runs in 
the guest, mouse passes seamlessly between spicec and host and copy/paste works 
between guest and host.
  qemu-kvm has been configured with
  ./configure --target-list=x86_64-softmmu --disable-curses  --disable-curl 
--audio-drv-list=alsa --audio-card-list=sb16,ac97,hda --enable-vnc-thread 
--disable-bluez --enable-vhost-net --enable-spice
  and is started with
  qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid 
-drive file=/home/rick/qemu/hds/wxp.raw,if=virtio,aio=native -m 1536 -name 
WinXP -net nic,model -net user -localtime -usb -vga qxl -device 
virtio-serial-pci,id=virtio-serial0,max_ports=16,bus=pci.0,addr=0x5 -chardev 
spicevmc,name=vdagent,id=vdagent -device 
virtserialport,nr=1,bus=virtio-serial0.0,chardev=vdagent,name=com.redhat.spice.0
 -spice port=1234,disable-ticketing -monitor stdio

  I've also tried start qemu like
  qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid 
-drive file=/home/rick/qemu/hds/wxp.raw,if=virtio -m 768 -name WinXP -net 
nic,model=virtio -net user -localtime -usb -vga qxl -device virtio-serial 
-chardev spicevmc,name=vdagent,id=vdagent -device 
virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice 
port=1234,disable-ticketing -monitor stdio
  and observed the same results.

  the host runs 2.6.39.4 vanilla kernel.  the guest uses the most recent
  virtio-serial, vga-qxl and vdagent from spice-space.org

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



Re: [Qemu-devel] [PATCH RFC v2 07/10] tests: Add NetBSD image

2017-08-17 Thread Kamil Rytarowski
On 17.08.2017 04:47, Fam Zheng wrote:
> 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 | 45 +
>  1 file changed, 45 insertions(+)
>  create mode 100755 tests/vm/netbsd
> 
> diff --git a/tests/vm/netbsd b/tests/vm/netbsd
> new file mode 100755
> index 00..7d7dfe6586
> --- /dev/null
> +++ b/tests/vm/netbsd
> @@ -0,0 +1,45 @@
> +#!/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 logging
> +import subprocess
> +import tempfile
> +import time
> +import basevm
> +
> +class NetBSDVM(basevm.BaseVM):
> +name = "netbsd"
> +BUILD_SCRIPT = """
> +set -e;
> +cd $(mktemp -d /var/tmp/qemu-test.XX);
> +tar -xf /dev/ld1a;
> +./configure --python=python2.7 {configure_opts};
> +gmake -j{jobs};
> +gmake check;
> +"""
> +
> +def build_image(self, img, rebuild=False):
> +if os.path.exists(img) and not rebuild:
> +return
> +cimg = 
> self._download_with_cache("http://download.patchew.org/netbsd.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])
> +os.rename(img_tmp, img)
> +
> +if __name__ == "__main__":
> +sys.exit(basevm.main(NetBSDVM))
> 




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v1 for-2.11 00/10] target/s390x: cleanup cpu.h

2017-08-17 Thread Philippe Mathieu-Daudé

On 08/17/2017 06:22 AM, David Hildenbrand wrote:

cpu.h is accessed outside of target/s390x. It should only contain
what is expected to be accessed outside of this folder. Therefore, create
internal.h and move a lot to that file. In addition, introduce
kvm-stub.c and kvm_390x.h for kvm specific functions.

Let's see what compilers think about this version.

RFC -> v1:
- (hopefully) fixed a compile error
- move some functions from cpu.h to the only c file they are used in
- move kvm function and stubs to kvm_s390x.h and kvm-stub.c
- smaller requested style fixes


David Hildenbrand (10):
   target/s390x: move cc_name() to cc_helper.c
   target/s390x: move cpu_mmu_idx_to_asc() to excp_helper.c
   target/s390x: move psw_key_valid() to mem_helper.c
   target/s390x: move s390_do_cpu_reset() to diag.c
   target/s390x: move get_per_in_range() to misc_helper.c
   target/s390x: introduce internal.h
   target/s390x: move a couple of functions to cpu.c
   s390x: avoid calling kvm_ functions outside of target/s390x/
   s390x/kvm: move KVM declarations and stubs to separate files
   target/s390x: cleanup cpu.h


for this series but 9 "move KVM declarations and stubs to separate files":

Reviewed-by: Philippe Mathieu-Daudé 



Re: [Qemu-devel] [PATCH 21/26] build-sys: make qemu qapi objects per-target

2017-08-17 Thread Markus Armbruster
Marc-André Lureau  writes:

> The qapi schema has per-target definitions. Move qapi objects in the
> per-target build, so they can be configured at compile time.

Suggest something like:

QAPI can't do target-specific conditionals (the symbols are
poisoned), and the work-around is to pretend the target-specific
stuff is target-independent, with stubs for the other targets.
Makes the target-specifity invisible in introspection.

To unpoison the symbols, we need to move the generated QAPI code to
the per-target build.

> Keep qapi-types.o qapi-visit.o in util-obj as they are necessary for
> common code, but they will be overwritten during the target link.

--verbose: how are they supposed to even compile in the
target-independent build once we generate #if defined(TARGET_FOO) into
them?

>   Add
> some stubs for block events, in code shared by tools & qemu.

Sounds awkward.

> The following patch will configure the schema to conditionally remove
> per-target disabled features.

"The following patches", right?

> Signed-off-by: Marc-André Lureau 
> ---
>  stubs/qapi-event.c | 74 
> ++
>  tests/test-qobject-input-visitor.c |  1 -
>  Makefile.objs  |  9 +
>  Makefile.target|  4 +++
>  stubs/Makefile.objs|  1 +
>  trace/Makefile.objs|  2 +-
>  6 files changed, 81 insertions(+), 10 deletions(-)
>  create mode 100644 stubs/qapi-event.c
>
> diff --git a/stubs/qapi-event.c b/stubs/qapi-event.c
> new file mode 100644
> index 00..9415299f3a
> --- /dev/null
> +++ b/stubs/qapi-event.c
> @@ -0,0 +1,74 @@
> +#include "qemu/osdep.h"
> +#include "qapi-event.h"
> +
> +void qapi_event_send_device_tray_moved(const char *device, const char *id,
> +   bool tray_open, Error **errp)
> +{
> +}
> +
> +void qapi_event_send_quorum_report_bad(QuorumOpType type, bool has_error,
> +   const char *error, const char 
> *node_name,
> +   int64_t sector_num,
> +   int64_t sectors_count, Error **errp)
> +{
> +}
> +
> +void qapi_event_send_quorum_failure(const char *reference, int64_t 
> sector_num,
> +int64_t sectors_count, Error **errp)
> +{
> +}
> +
> +void qapi_event_send_block_job_cancelled(BlockJobType type, const char 
> *device,
> + int64_t len, int64_t offset,
> + int64_t speed, Error **errp)
> +{
> +}
> +
> +void qapi_event_send_block_job_completed(BlockJobType type, const char 
> *device,
> + int64_t len, int64_t offset,
> + int64_t speed, bool has_error,
> + const char *error, Error **errp)
> +{
> +}
> +
> +void qapi_event_send_block_job_error(const char *device,
> + IoOperationType operation,
> + BlockErrorAction action, Error **errp)
> +{
> +}
> +
> +void qapi_event_send_block_job_ready(BlockJobType type, const char *device,
> + int64_t len, int64_t offset, int64_t 
> speed,
> + Error **errp)
> +{
> +}
> +
> +void qapi_event_send_block_io_error(const char *device, const char 
> *node_name,
> +IoOperationType operation,
> +BlockErrorAction action, bool 
> has_nospace,
> +bool nospace, const char *reason,
> +Error **errp)
> +{
> +}
> +
> +void qapi_event_send_block_image_corrupted(const char *device,
> +   bool has_node_name,
> +   const char *node_name,
> +   const char *msg, bool has_offset,
> +   int64_t offset, bool has_size,
> +   int64_t size, bool fatal,
> +   Error **errp)
> +{
> +}
> +
> +void qapi_event_send_block_write_threshold(const char *node_name,
> +   uint64_t amount_exceeded,
> +   uint64_t write_threshold,
> +   Error **errp)
> +{
> +}
> +
> +void qapi_event_send_device_deleted(bool has_device, const char *device,
> +const char *path, Error **errp)
> +{
> +}

Yup, awkward.

> diff --git a/tests/test-qobject-input-visitor.c 
> b/tests/test-qobject-input-visitor.c
> index 4da5d02c35..0a9352c5c1 100644
> --- a/tests/test-qobject-input-visitor.c
> +++ b/

Re: [Qemu-devel] [PATCH] scsi-block: Add qdev error properties

2017-08-17 Thread Paolo Bonzini
On 17/08/2017 12:28, Fam Zheng wrote:
> This makes the werror/rerror options available on the scsi-block device,
> to allow user specify error handling policy in the same way as scsi-hd
> etc.
> 
> Signed-off-by: Fam Zheng 

I'm not sure this is enough, because you would only obey the
rerror/werror action if ioctl fails, not if the ioctl succeeds but the
command fails (the "r->status && *r->status" conditional in
scsi_disk_req_check_error).

Paolo

> ---
>  hw/scsi/scsi-disk.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index 5f1e5e8070..27d917f7c3 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -2972,6 +2972,7 @@ static const TypeInfo scsi_cd_info = {
>  
>  #ifdef __linux__
>  static Property scsi_block_properties[] = {
> +DEFINE_BLOCK_ERROR_PROPERTIES(SCSIDiskState, qdev.conf), \
>  DEFINE_PROP_DRIVE("drive", SCSIDiskState, qdev.conf.blk),
>  DEFINE_PROP_END_OF_LIST(),
>  };
> 




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

2017-08-17 Thread Paolo Bonzini
On 17/08/2017 12:58, 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| 41 +
>  2 files changed, 45 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..80ef830 100644
> --- a/iothread.c
> +++ b/iothread.c
> @@ -57,6 +57,20 @@ static void *iothread_run(void *opaque)
>  
>  while (!atomic_read(&iothread->stopping)) {
>  aio_poll(iothread->ctx, true);
> +
> +if (atomic_read(&iothread->worker_context)) {
> +g_main_context_push_thread_default(iothread->worker_context);
> +iothread->main_loop =
> +g_main_loop_new(iothread->worker_context, TRUE);
> +g_main_loop_run(iothread->main_loop);
> +
> +g_main_loop_unref(iothread->main_loop);
> +iothread->main_loop = NULL;
> +
> +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 +87,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 +142,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;
>  
>  /* This assumes we are called from a thread with useful CPU affinity for 
> us
>   * to inherit.
> @@ -309,3 +327,26 @@ 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);

You still need aio_notify, so that the aio_poll call in iothread_run exits.

Paolo

> +return NULL;
> +}
> +
> +GMainContext *iothread_get_g_main_context(IOThread *iothread)
> +{
> +g_once(&iothread->once, iothread_g_main_context_init, iothread);
> +
> +return iothread->worker_context;
> +}
> 




Re: [Qemu-devel] [PATCH 04/26] qapi: generate a literal qobject for introspection

2017-08-17 Thread Markus Armbruster
Marc-André Lureau  writes:

> Replace the generated json string with a literal qobject. The later is
> easier to deal with, at run time, as well as compile time: #if blocks
> can be more easily added than in a json string.
>
> Signed-off-by: Marc-André Lureau 
[...]
> diff --git a/tests/test-qobject-input-visitor.c 
> b/tests/test-qobject-input-visitor.c
> index bcf02617dc..1969733971 100644
> --- a/tests/test-qobject-input-visitor.c
> +++ b/tests/test-qobject-input-visitor.c
> @@ -1247,24 +1247,26 @@ static void 
> test_visitor_in_fail_alternate(TestInputVisitorData *data,
>  }
>  
>  static void do_test_visitor_in_qmp_introspect(TestInputVisitorData *data,
> -  const char *schema_json)
> +  const QLitObject *qlit)
>  {
>  SchemaInfoList *schema = NULL;
> +QObject *obj = qobject_from_qlit(qlit);
>  Visitor *v;
>  
> -v = visitor_input_test_init_raw(data, schema_json);
> +v = qobject_input_visitor_new(obj);
>  
>  visit_type_SchemaInfoList(v, NULL, &schema, &error_abort);
>  g_assert(schema);
>  
>  qapi_free_SchemaInfoList(schema);
> +qobject_decref(obj);
>  }

Are you leaking @v?

>  
>  static void test_visitor_in_qmp_introspect(TestInputVisitorData *data,
> const void *unused)
>  {
> -do_test_visitor_in_qmp_introspect(data, test_qmp_schema_json);
> -do_test_visitor_in_qmp_introspect(data, qmp_schema_json);
> +do_test_visitor_in_qmp_introspect(data, &test_qmp_schema_qlit);
> +do_test_visitor_in_qmp_introspect(data, &qmp_schema_qlit);
>  }
>  
>  int main(int argc, char **argv)
[...]



Re: [Qemu-devel] [PATCH 07/26] qapi: add 'if' condition on top-level schema elements

2017-08-17 Thread Markus Armbruster
Marc-André Lureau  writes:

> Add 'if' c-preprocessor condition on top-level schema elements:
> struct, enum, union, alternate, command, event.
>
> Variants objects types are created outside of #if blocks, since they
> may be shared by various types. Even if unused, this shouldn't be an
> issue, since those are internal types.
>
> Note that there is no checks in qapi scripts to verify that elements
> have compatible conditions (ex: if-struct used by a if-foo
> command). This may thus fail at C build time if they don't share the
> same subset of conditions.
>
> Signed-off-by: Marc-André Lureau 
> ---
>  scripts/qapi.py | 153 
> +---
>  scripts/qapi-commands.py|   4 +-
>  scripts/qapi-event.py   |   4 +-
>  scripts/qapi-introspect.py  |  46 ++
>  scripts/qapi-types.py   |  51 +++
>  scripts/qapi-visit.py   |  13 ++-
>  scripts/qapi2texi.py|  10 +--
>  tests/Makefile.include  |   1 +
>  tests/qapi-schema/bad-if.err|   1 +
>  tests/qapi-schema/bad-if.exit   |   1 +
>  tests/qapi-schema/bad-if.json   |   3 +
>  tests/qapi-schema/bad-if.out|   0
>  tests/qapi-schema/qapi-schema-test.json |  20 +
>  tests/qapi-schema/qapi-schema-test.out  |  31 +++
>  tests/qapi-schema/test-qapi.py  |  21 +++--
>  15 files changed, 272 insertions(+), 87 deletions(-)
>  create mode 100644 tests/qapi-schema/bad-if.err
>  create mode 100644 tests/qapi-schema/bad-if.exit
>  create mode 100644 tests/qapi-schema/bad-if.json
>  create mode 100644 tests/qapi-schema/bad-if.out

Missing: docs/devel/qapi-code-gen.txt update.

[...]



Re: [Qemu-devel] [PATCH for-2.9?] configure: Remove unused code (found by shellcheck)

2017-08-17 Thread Philippe Mathieu-Daudé

On 03/28/2017 03:49 PM, Stefan Weil wrote:

smartcard_cflags is no longer needed since commit
0b22ef0f57a8910d849602bef0940edcd0553d2c.


leftover from afd347ab387



Signed-off-by: Stefan Weil 


Reviewed-by: Philippe Mathieu-Daudé 


---

This is a very old patch which I just found in my patch queue.
It's not mandatory for 2.9, but I also think that there is
no risk to apply it.

Sorry for sending it so late.

Regards
Stefan

  configure | 1 -
  1 file changed, 1 deletion(-)

diff --git a/configure b/configure
index d1ce33bc79..b65ea2931c 100755
--- a/configure
+++ b/configure
@@ -4096,7 +4096,6 @@ EOF
  fi
  
  # check for smartcard support

-smartcard_cflags=""
  if test "$smartcard" != "no"; then
  if $pkg_config libcacard; then
  libcacard_cflags=$($pkg_config --cflags libcacard)





Re: [Qemu-devel] [PATCH 22/26] qapi: make rtc-reset-reinjection depend on TARGET_I386

2017-08-17 Thread Markus Armbruster
Marc-André Lureau  writes:

> Signed-off-by: Marc-André Lureau 
> ---
>  qapi-schema.json |  3 ++-
>  monitor.c| 10 --
>  2 files changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 2f4528c769..2361c13fc8 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -6270,7 +6270,8 @@
>  # <- { "return": {} }
>  #
>  ##
> -{ 'command': 'rtc-reset-reinjection' }
> +{ 'command': 'rtc-reset-reinjection',
> +  'if': ['defined(NEED_CPU_H)', 'defined(TARGET_I386)'] }

Aha, here' you use the list syntax.

And your strategy to keep things compiling also becomes clear: you wrap
uses of poisoned symbols like TARGET_I386 in #if defined(NEED_CPU_H).

Not exactly elegant, but looks workable.  But you need to explain this
solution in commit messages [PATCH 21, I guess] and document it in
qapi-code-gen.txt.  *Unless* we can find a better one.

>  
>  # Rocker ethernet network switch
>  { 'include': 'qapi/rocker.json' }
> diff --git a/monitor.c b/monitor.c
> index 383c84d162..f3dafafa22 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -970,9 +970,6 @@ static void qmp_query_qmp_schema(QDict *qdict, QObject 
> **ret_data,
>   */
>  static void qmp_unregister_commands_hack(void)
>  {
> -#ifndef TARGET_I386
> -qmp_unregister_command(&qmp_commands, "rtc-reset-reinjection");
> -#endif
>  #ifndef TARGET_S390X
>  qmp_unregister_command(&qmp_commands, "dump-skeys");
>  #endif
> @@ -4151,13 +4148,6 @@ QemuOptsList qemu_mon_opts = {
>  },
>  };
>  
> -#ifndef TARGET_I386
> -void qmp_rtc_reset_reinjection(Error **errp)
> -{
> -error_setg(errp, QERR_FEATURE_DISABLED, "rtc-reset-reinjection");
> -}
> -#endif
> -
>  #ifndef TARGET_S390X
>  void qmp_dump_skeys(const char *filename, Error **errp)
>  {



Re: [Qemu-devel] [PATCH v1 for-2.11 06/10] target/s390x: introduce internal.h

2017-08-17 Thread Thomas Huth
On 17.08.2017 11:22, David Hildenbrand wrote:
> cpu.h should only contain what really has to be accessed outside of
> target/s390x/. Add internal.h which can only be used inside target/s390x/.
> 
> Move everything that isn't fast enough to run away and restructure it
> right away. We'll move all kvm_* stuff later.
> 
> Minor style fixes to avoid checkpatch warning to:
> - struct Lowcore: "{" goes into same line as typedef
> - struct LowCore: add spaces around "-" in array length calculations
> - time2tod() and tod2time(): move "{" to separate line
> - get_per_atmid(): add space between ")" and "?". Move cases by one char.
> - get_per_atmid(): drop extra paremthesis around (1 << 6)
> 
> Change license of new file to GPL2+ and keep copyright notice.
> 
> Reviewed-by: Richard Henderson 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/arch_dump.c   |   1 +
>  target/s390x/cc_helper.c   |   1 +
>  target/s390x/cpu.c |   1 +
>  target/s390x/cpu.h | 343 ---
>  target/s390x/cpu_models.c  |   1 +
>  target/s390x/diag.c|   1 +
>  target/s390x/excp_helper.c |   1 +
>  target/s390x/fpu_helper.c  |   1 +
>  target/s390x/gdbstub.c |   1 +
>  target/s390x/helper.c  |   1 +
>  target/s390x/int_helper.c  |   1 +
>  target/s390x/internal.h| 391 
> +
>  target/s390x/interrupt.c   |   1 +
>  target/s390x/ioinst.c  |   1 +
>  target/s390x/kvm.c |   1 +
>  target/s390x/machine.c |   1 +
>  target/s390x/mem_helper.c  |   1 +
>  target/s390x/misc_helper.c |   1 +
>  target/s390x/mmu_helper.c  |   1 +
>  target/s390x/translate.c   |   1 +
>  20 files changed, 409 insertions(+), 343 deletions(-)
>  create mode 100644 target/s390x/internal.h

Reviewed-by: Thomas Huth 



Re: [Qemu-devel] [PATCH] scsi-block: Add qdev error properties

2017-08-17 Thread Fam Zheng
On Thu, 08/17 13:44, Paolo Bonzini wrote:
> On 17/08/2017 12:28, Fam Zheng wrote:
> > This makes the werror/rerror options available on the scsi-block device,
> > to allow user specify error handling policy in the same way as scsi-hd
> > etc.
> > 
> > Signed-off-by: Fam Zheng 
> 
> I'm not sure this is enough, because you would only obey the
> rerror/werror action if ioctl fails, not if the ioctl succeeds but the
> command fails (the "r->status && *r->status" conditional in
> scsi_disk_req_check_error).

Yes, I missed that.  Why isn't scsi_handle_rw_error called in that if block
currently?

Fam



Re: [Qemu-devel] [PATCH 23/26] qapi: make s390 commands depend on TARGET_S390X

2017-08-17 Thread Markus Armbruster
Marc-André Lureau  writes:

> Signed-off-by: Marc-André Lureau 
> ---
>  qapi-schema.json| 10 +++---
>  include/sysemu/arch_init.h  |  6 --
>  monitor.c   | 14 --
>  qmp.c   | 14 --
>  stubs/arch-query-cpu-model-baseline.c   | 12 
>  stubs/arch-query-cpu-model-comparison.c | 12 
>  target/s390x/cpu_models.c   |  4 ++--
>  stubs/Makefile.objs |  2 --
>  8 files changed, 9 insertions(+), 65 deletions(-)
>  delete mode 100644 stubs/arch-query-cpu-model-baseline.c
>  delete mode 100644 stubs/arch-query-cpu-model-comparison.c
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 2361c13fc8..278d7e2aa3 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3577,7 +3577,8 @@
>  #
>  ##
>  { 'command': 'dump-skeys',
> -  'data': { 'filename': 'str' } }
> +  'data': { 'filename': 'str' },
> +  'if': ['defined(NEED_CPU_H)', 'defined(TARGET_S390X)']}

Same technique as for TARGET_I386 in PATCH 22.  See my review of it for
how it works.

>  
>  ##
>  # @netdev_add:
> @@ -4621,7 +4622,9 @@
>  ##
>  { 'command': 'query-cpu-model-comparison',
>'data': { 'modela': 'CpuModelInfo', 'modelb': 'CpuModelInfo' },

Not your patch's fault: 'modela' sounds like the name of some high end
escort service or something.  The QAPI naming conventions want proper
words connected with dashes: 'model-a' and 'model-b'.  Although I
wouldn't mind just 'a' and 'b'.  This should've been caught in review.
Too late to fix now, I'm afraid.


> -  'returns': 'CpuModelCompareInfo' }
> +  'returns': 'CpuModelCompareInfo',
> +  'if': ['defined(NEED_CPU_H)', 'defined(TARGET_S390X)']}
> +
>  
>  ##
>  # @CpuModelBaselineInfo:
> @@ -4673,7 +4676,8 @@
>  { 'command': 'query-cpu-model-baseline',
>'data': { 'modela': 'CpuModelInfo',
>  'modelb': 'CpuModelInfo' },
> -  'returns': 'CpuModelBaselineInfo' }
> +  'returns': 'CpuModelBaselineInfo',
> +  'if': ['defined(NEED_CPU_H)', 'defined(TARGET_S390X)']}
>  
>  ##
>  # @AddfdInfo:
> diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
> index 8751c468ed..e9f1ea0cca 100644
> --- a/include/sysemu/arch_init.h
> +++ b/include/sysemu/arch_init.h
> @@ -35,11 +35,5 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error 
> **errp);
>  CpuModelExpansionInfo *arch_query_cpu_model_expansion(CpuModelExpansionType 
> type,
>CpuModelInfo *mode,
>Error **errp);
> -CpuModelCompareInfo *arch_query_cpu_model_comparison(CpuModelInfo *modela,
> - CpuModelInfo *modelb,
> - Error **errp);
> -CpuModelBaselineInfo *arch_query_cpu_model_baseline(CpuModelInfo *modela,
> -CpuModelInfo *modelb,
> -Error **errp);
>  
>  #endif
> diff --git a/monitor.c b/monitor.c
> index f3dafafa22..505ee5c58d 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -970,19 +970,12 @@ static void qmp_query_qmp_schema(QDict *qdict, QObject 
> **ret_data,
>   */
>  static void qmp_unregister_commands_hack(void)
>  {
> -#ifndef TARGET_S390X
> -qmp_unregister_command(&qmp_commands, "dump-skeys");
> -#endif
>  #ifndef TARGET_ARM
>  qmp_unregister_command(&qmp_commands, "query-gic-capabilities");
>  #endif
>  #if !defined(TARGET_S390X) && !defined(TARGET_I386)
>  qmp_unregister_command(&qmp_commands, "query-cpu-model-expansion");
>  #endif
> -#if !defined(TARGET_S390X)
> -qmp_unregister_command(&qmp_commands, "query-cpu-model-baseline");
> -qmp_unregister_command(&qmp_commands, "query-cpu-model-comparison");
> -#endif
>  #if !defined(TARGET_PPC) && !defined(TARGET_ARM) && !defined(TARGET_I386) \
>  && !defined(TARGET_S390X)
>  qmp_unregister_command(&qmp_commands, "query-cpu-definitions");
> @@ -4148,13 +4141,6 @@ QemuOptsList qemu_mon_opts = {
>  },
>  };
>  
> -#ifndef TARGET_S390X
> -void qmp_dump_skeys(const char *filename, Error **errp)
> -{
> -error_setg(errp, QERR_FEATURE_DISABLED, "dump-skeys");
> -}
> -#endif
> -
>  #ifndef TARGET_ARM
>  GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
>  {
> diff --git a/qmp.c b/qmp.c
> index 90816ba283..7b6861846f 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -553,20 +553,6 @@ CpuModelExpansionInfo 
> *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
>  return arch_query_cpu_model_expansion(type, model, errp);
>  }
>  
> -CpuModelCompareInfo *qmp_query_cpu_model_comparison(CpuModelInfo *modela,
> -CpuModelInfo *modelb,
> -Error **errp)
> -{
> -return arch_query_cpu_model_comparison(modela, modelb, errp);
> -}
> -
> -Cp

Re: [Qemu-devel] [PATCH] scsi-block: Add qdev error properties

2017-08-17 Thread Paolo Bonzini
On 17/08/2017 14:03, Fam Zheng wrote:
> On Thu, 08/17 13:44, Paolo Bonzini wrote:
>> On 17/08/2017 12:28, Fam Zheng wrote:
>>> This makes the werror/rerror options available on the scsi-block device,
>>> to allow user specify error handling policy in the same way as scsi-hd
>>> etc.
>>>
>>> Signed-off-by: Fam Zheng 
>>
>> I'm not sure this is enough, because you would only obey the
>> rerror/werror action if ioctl fails, not if the ioctl succeeds but the
>> command fails (the "r->status && *r->status" conditional in
>> scsi_disk_req_check_error).
> 
> Yes, I missed that.  Why isn't scsi_handle_rw_error called in that if block
> currently?

A recursive answer is because scsi-block doesn't have rerror and werror
(and it's the only one that sets r->status, see scsi_block_dma_command).

More precisely, scsi_handle_rw_error more or less assumes error != 0
(see the switch statement and blk_error_action), so some other changes
are needed to cover that case---at least not overwrite the sense and
ensure something sensible comes out of the QMP event.

Paolo



Re: [Qemu-devel] [PATCH v1 for-2.11 09/10] s390x/kvm: move KVM declarations and stubs to separate files

2017-08-17 Thread David Hildenbrand

>> +#include "qemu/osdep.h"
>> +#include "qemu-common.h"
>> +#include "cpu.h"
>> +#include "kvm_s390x.h"
> 
> here goes the:
> 
> struct kvm_s390_irq {};

As we don't have a stub using kvm_s390_irq, I guess I can just drop this
for now.
[...]

> 
> change by
> 
> typedef struct kvm_s390_irq kvm_s390_irq_t;
> 
>> +
>> +void kvm_s390_floating_interrupt(struct kvm_s390_irq *irq);
>> +void kvm_s390_service_interrupt(uint32_t parm);
>> +void kvm_s390_vcpu_interrupt(S390CPU *cpu, struct kvm_s390_irq *irq);
> 
> change these to use 'kvm_s390_irq_t *irq' arg
> 
>> +void kvm_s390_access_exception(S390CPU *cpu, uint16_t code, uint64_t 
>> te_code);
>> +int kvm_s390_mem_op(S390CPU *cpu, vaddr addr, uint8_t ar, void *hostbuf,
>> +int len, bool is_write);
>> +void kvm_s390_program_interrupt(S390CPU *cpu, uint16_t code);
>> +void kvm_s390_io_interrupt(uint16_t subchannel_id,
>> +   uint16_t subchannel_nr, uint32_t io_int_parm,
>> +   uint32_t io_int_word);
>> +void kvm_s390_crw_mchk(void);
>> +int kvm_s390_set_cpu_state(S390CPU *cpu, uint8_t cpu_state);
>> +void kvm_s390_vcpu_interrupt_pre_save(S390CPU *cpu);
>> +int kvm_s390_vcpu_interrupt_post_load(S390CPU *cpu);
>> +int kvm_s390_get_ri(void);
>> +int kvm_s390_get_gs(void);
>> +int kvm_s390_get_clock(uint8_t *tod_high, uint64_t *tod_clock);
>> +int kvm_s390_set_clock(uint8_t *tod_high, uint64_t *tod_clock);
>> +void kvm_s390_enable_css_support(S390CPU *cpu);
>> +int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>> +int vq, bool assign);
>> +int kvm_s390_cpu_restart(S390CPU *cpu);
>> +int kvm_s390_get_memslot_count(KVMState *s);
>> +int kvm_s390_cmma_active(void);
>> +void kvm_s390_cmma_reset(void);
>> +void kvm_s390_reset_vcpu(S390CPU *cpu);
>> +int kvm_s390_set_mem_limit(KVMState *s, uint64_t new_limit, uint64_t 
>> *hw_limit);
>> +void kvm_s390_crypto_reset(void);
>> +
>> +/* implemented outside of target/s390x/ */
>> +int kvm_s390_inject_flic(struct kvm_s390_irq *irq);
> 
> also change this to use 'kvm_s390_irq_t *irq' arg here and in hw/intc/
> 

Yes, this seems to compile!


-- 

Thanks,

David



Re: [Qemu-devel] [PATCH RFC v2 04/10] tests: Add vm test lib

2017-08-17 Thread Stefan Hajnoczi
On Thu, Aug 17, 2017 at 10:47:40AM +0800, Fam Zheng wrote:
> +self._args = [ \
> +"-nodefaults", "-enable-kvm", "-m", "2G",
> +"-smp", os.environ.get("J", "4"), "-cpu", "host",

I suggested making -j a command-line argument and constructor argument
instead of using a environment variable.  Your email reply seemed to
agree but this patch is still using an environment variable.  Did you
forget?

> +def boot(self, img, extra_args=[]):
> +args = self._args + [
> +"-drive", "file=%s,if=none,id=drive0,cache=writeback" % img,
> +"-device", "virtio-blk,drive=drive0,bootindex=0"]
> +args += self._data_args + extra_args
> +logging.debug("QEMU args: %s", " ".join(args))
> +guest = QEMUMachine(binary=os.environ.get("QEMU", 
> "qemu-system-x86_64"),
> +args=args)
> +guest._vga = "std"

_vga is a protected field.  We don't inherit from QEMUMachine so it
shouldn't be accessed directly.  One option is to add a vga argument to
the QEMUMachine() constructor.

> +guest.launch()
> +atexit.register(self.shutdown)
> +self._guest = guest
> +usernet_info = guest.qmp("human-monitor-command",
> + command_line="info usernet")
> +self.ssh_port = None
> +for l in usernet_info["return"].splitlines():
> +fields = l.split()
> +if "TCP[HOST_FORWARD]" in fields and "22" in fields:
> +self.ssh_port = l.split()[3]
> +if not self.ssh_port:
> +raise Exception("Cannot find ssh port from 'info usernet':\n%s" 
> % \
> +usernet_info)
> +
> +def wait_ssh(self, seconds=120):
> +guest_remote = self.GUEST_USER + "@127.0.0.1"

Please remove this unused variable.

> +def parse_args(vm_name):
> +parser = argparse.ArgumentParser()

Python 2.6 only has optparse, not argparse.  Perhaps we can raise the
minimum Python version requirement to 2.7 starting from QEMU 2.11.  For
the time being it would be necessary to use optparse instead.

> +parser.add_argument("--debug", "-D", action="store_true",
> +help="enable debug output")
> +parser.add_argument("--image", "-i", default="%s.img" % vm_name,
> +help="image file name")
> +parser.add_argument("--force", "-f", action="store_true",
> +help="force build image even if image exists")
> +parser.add_argument("--build-image", "-b", action="store_true",
> +help="build image")
> +parser.add_argument("--build-qemu",
> +help="build QEMU from source in guest")
> +parser.add_argument("--interactive", "-I", action="store_true",
> +help="Interactively run command")
> +return parser.parse_known_args()

Documentation would be really helpful.  I'm not sure what workflow you
have in mind for this command-line tool.  There are two scenarios that
would be good to document:
1. How to invoke and use existing images for tests.
2. How to create new images (e.g. add a guest operating system).

> +
> +def main(vmcls):
> +args, argv = parse_args(vmcls.name)
> +if not argv and not args.build_qemu:
> +print "Nothing to do?"
> +return 1
> +if args.debug:
> +logging.getLogger().setLevel(logging.DEBUG)
> +vm = vmcls(debug=args.debug)
> +if args.build_image:
> +if os.path.exists(args.image) and not args.force:
> +sys.stderr.writelines(["Image file exists: %s\n" % img,
> +  "Use --force option to overwrite\n"])
> +return 1
> +return vm.build_image(args.image)
> +if args.build_qemu:
> +vm.add_source_dir(args.build_qemu)
> +cmd = [vm.BUILD_SCRIPT.format(
> +   configure_opts = " ".join(argv),
> +   jobs=os.environ.get("J", "4"))]
> +else:
> +cmd = argv
> +vm.boot(args.image + ",snapshot=on")
> +vm.wait_ssh()

This method returns 2 on timeout.  Exit here?


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH RFC v2 04/10] tests: Add vm test lib

2017-08-17 Thread Paolo Bonzini
On 17/08/2017 14:21, Stefan Hajnoczi wrote:
>> +guest = QEMUMachine(binary=os.environ.get("QEMU", 
>> "qemu-system-x86_64"),
>> +args=args)
>> +guest._vga = "std"
> _vga is a protected field.  We don't inherit from QEMUMachine so it
> shouldn't be accessed directly.  One option is to add a vga argument to
> the QEMUMachine() constructor.

You can just use "-device VGA" here, since this code is x86-specific.
"-vga none -device VGA" works well, so patch 2 can be dropped.

Paolo




Re: [Qemu-devel] [PATCH 26/26] qapi: make query-cpu-definitions depend on specific targets

2017-08-17 Thread Markus Armbruster
Marc-André Lureau  writes:

> It depends on TARGET_PPC || TARGET_ARM || TARGET_I386 || TARGET_S390X.
>
> Signed-off-by: Marc-André Lureau 
> ---
>  qapi-schema.json|  4 +++-
>  include/sysemu/arch_init.h  |  2 --
>  monitor.c   | 22 --
>  qmp.c   |  5 -
>  stubs/arch-query-cpu-def.c  | 10 --
>  target/arm/helper.c |  3 ++-
>  target/i386/cpu.c   |  3 ++-
>  target/ppc/translate_init.c |  3 ++-
>  target/s390x/cpu_models.c   |  2 +-
>  stubs/Makefile.objs |  1 -
>  10 files changed, 10 insertions(+), 45 deletions(-)
>  delete mode 100644 stubs/arch-query-cpu-def.c
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index f5e1acff83..8e3949bca8 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4433,7 +4433,9 @@
>  #
>  # Since: 1.2.0
>  ##
> -{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
> +{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
> +  'if': ['defined(NEED_CPU_H)',
> + 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) 
> || defined(TARGET_S390X)'] }
>  
>  ##
>  # @CpuModelInfo:
> diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
> index fb3d20a1b8..e9721b9ce8 100644
> --- a/include/sysemu/arch_init.h
> +++ b/include/sysemu/arch_init.h
> @@ -31,6 +31,4 @@ extern const uint32_t arch_type;
>  int kvm_available(void);
>  int xen_available(void);
>  
> -CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp);
> -
>  #endif
> diff --git a/monitor.c b/monitor.c
> index b134c39144..6600819599 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -956,26 +956,6 @@ static void qmp_query_qmp_schema(QDict *qdict, QObject 
> **ret_data,
>  *ret_data = qobject_from_qlit(&qmp_schema_qlit);
>  }
>  
> -/*
> - * We used to define commands in qmp-commands.hx in addition to the
> - * QAPI schema.  This permitted defining some of them only in certain
> - * configurations.  query-commands has always reflected that (good,
> - * because it lets QMP clients figure out what's actually available),
> - * while query-qmp-schema never did (not so good).  This function is a
> - * hack to keep the configuration-specific commands defined exactly as
> - * before, even though qmp-commands.hx is gone.
> - *
> - * FIXME Educate the QAPI schema on configuration-specific commands,
> - * and drop this hack.
> - */
> -static void qmp_unregister_commands_hack(void)
> -{
> -#if !defined(TARGET_PPC) && !defined(TARGET_ARM) && !defined(TARGET_I386) \
> -&& !defined(TARGET_S390X)
> -qmp_unregister_command(&qmp_commands, "query-cpu-definitions");
> -#endif
> -}
> -

Very nice to see this gone.  Its removal could be made a separate commit
to highlight the achievement :)

There are a few more candidates:

* QERR_FEATURE_DISABLED leads me to
  - query-hotpluggable-cpus via monitor.c
  - x-colo-lost-heartbeat via colo-failover.c
  - query-rocker, query-rocker-ports, query-rocker-of-dpa-flows,
query-rocker-of-dpa-groups via qmp-norocker.c

* QERR_UNSUPPORTED leads me to
  - dump-guest-memory via dump_init() and stubs/dump.c
  - query-vm-generation-id via stubs/vmgenid.c
  - inject-nmi via nmi_monitor_handle() and s390_nmi()
  - query-pci via pci-stub.c

* grep error_set stubs/* doesn't find more

>  void monitor_init_qmp_commands(void)
>  {
>  /*
> @@ -995,8 +975,6 @@ void monitor_init_qmp_commands(void)
>  qmp_register_command(&qmp_commands, "netdev_add", qmp_netdev_add,
>   QCO_NO_OPTIONS);
>  
> -qmp_unregister_commands_hack();
> -
>  QTAILQ_INIT(&qmp_cap_negotiation_commands);
>  qmp_register_command(&qmp_cap_negotiation_commands, "qmp_capabilities",
>   qmp_marshal_qmp_capabilities, QCO_NO_OPTIONS);
> diff --git a/qmp.c b/qmp.c
> index afa266ec1e..d57ccf1251 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -541,11 +541,6 @@ DevicePropertyInfoList *qmp_device_list_properties(const 
> char *typename,
>  return prop_list;
>  }
>  
> -CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
> -{
> -return arch_query_cpu_definitions(errp);
> -}
> -
>  void qmp_add_client(const char *protocol, const char *fdname,
>  bool has_skipauth, bool skipauth, bool has_tls, bool tls,
>  Error **errp)
> diff --git a/stubs/arch-query-cpu-def.c b/stubs/arch-query-cpu-def.c
> deleted file mode 100644
> index cefe4beb82..00
> --- a/stubs/arch-query-cpu-def.c
> +++ /dev/null
> @@ -1,10 +0,0 @@
> -#include "qemu/osdep.h"
> -#include "qemu-common.h"
> -#include "sysemu/arch_init.h"
> -#include "qapi/qmp/qerror.h"
> -
> -CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
> -{
> -error_setg(errp, QERR_UNSUPPORTED);
> -return NULL;
> -}
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 4ed32c56b8..ec644f3930 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -15,6 +15,7 @@

Re: [Qemu-devel] [PATCH v1 for-2.11 09/10] s390x/kvm: move KVM declarations and stubs to separate files

2017-08-17 Thread Thomas Huth
On 17.08.2017 13:40, Philippe Mathieu-Daudé wrote:
> Hi David,
> 
> On 08/17/2017 06:22 AM, David Hildenbrand wrote:
>> Let's do it just like the other architectures. Introduce kvm-stub.c
>> for stubs and kvm_s390x.h for the declarations.
>>
>> Add a fake declaration of struct kvm_s390_irq so we don't need other
>> ugly CONFIG_KVM checks.
> 
> You can use an opaque pointer to avoid that ("bridge" design pattern).
> 
> It involves few more changes but looks safer.

There is maybe even a simpler solution than that, see below ...

[...]
>>   feat-src = $(SRC_PATH)/target/$(TARGET_BASE_ARCH)/
>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>> index 74d5b35..aeb730c 100644
>> --- a/target/s390x/cpu.h
>> +++ b/target/s390x/cpu.h
>> @@ -41,6 +41,7 @@
>>   #include "exec/cpu-all.h"
>> #include "fpu/softfloat.h"
>> +#include "kvm_s390x.h"

Do we still need that? cpu.h should theoretically be independent from
kvm now, shouldn't it? And for the .c files, it's likely better to
include kvm_s390x.h directly there if they require it.

[...]
>> diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
>> new file mode 100644
>> index 000..35db28c
>> --- /dev/null
>> +++ b/target/s390x/kvm_s390x.h
>> @@ -0,0 +1,51 @@
>> +/*
>> + * QEMU KVM support -- s390x specific functions.
>> + *
>> + * Copyright (c) 2009 Ulrich Hecht
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or
>> later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#ifndef KVM_S390X_H
>> +#define KVM_S390X_H
>> +
>> +#include "sysemu/kvm.h"
>> +
>> +#ifndef CONFIG_KVM
>> +struct kvm_s390_irq {};
>> +#endif /* CONFIG_KVM */
> 
> change by
> 
> typedef struct kvm_s390_irq kvm_s390_irq_t;

May I suggest to simply use this instead:

struct kvm_s390_irq;

No need to switch for a typedef here, you can simply use this anonymous
struct declaration, I think.

 Thomas



Re: [Qemu-devel] [PULL 7/7] hw/misc/mmio_interface: Return after error_setg() to avoid crash

2017-08-17 Thread KONRAD Frederic



On 08/14/2017 01:55 PM, Peter Maydell wrote:

On 14 August 2017 at 12:52, Thomas Huth  wrote:

On 14.08.2017 13:45, Peter Maydell wrote:

It seems like it should be an error to permit this to be
created from the command line at all



That's also what thought first ... but the commit message of commit
7cc2298c46a6afa4f4ff7e5cd262809c782d701b says that it "can be
hotplugged/hotunplugged" ? ... that's confusing ...


It means that memory.c creates and deletes instances of the
device on demand, not that it's hotplugged in the "controlled by
the user simulating PCI or other hotplug" sense.

thanks
-- PMM



Sorry, missed that I changed my email address recently.
I'll add some docs and fix the things we mentioned for 2.11.

For the context:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg446748.html

Thanks,
Fred



Re: [Qemu-devel] [PATCH v1 for-2.11 10/10] target/s390x: cleanup cpu.h

2017-08-17 Thread Thomas Huth
On 17.08.2017 11:22, David Hildenbrand wrote:
> Let's reshuffle the function prototypes so we get a cleaner outline
> of the files.
> 
> Reviewed-by: Richard Henderson 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/cpu.h | 140 
> ++---
>  1 file changed, 69 insertions(+), 71 deletions(-)

Not sure whether this code churn here is really worth the effort ... I'd
prefer to introduce file-specific headers for the prototypes instead,
e.g. helper.h, mmu_helper.h, etc. ... but that's just my 0.02 € ... I
also won't object if this patch gets included as is.

 Thomas



Re: [Qemu-devel] [PATCH 26/26] qapi: make query-cpu-definitions depend on specific targets

2017-08-17 Thread Marc-André Lureau
Hi

On Thu, Aug 17, 2017 at 2:30 PM Markus Armbruster  wrote:

> Marc-André Lureau  writes:
>
> > It depends on TARGET_PPC || TARGET_ARM || TARGET_I386 || TARGET_S390X.
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  qapi-schema.json|  4 +++-
> >  include/sysemu/arch_init.h  |  2 --
> >  monitor.c   | 22 --
> >  qmp.c   |  5 -
> >  stubs/arch-query-cpu-def.c  | 10 --
> >  target/arm/helper.c |  3 ++-
> >  target/i386/cpu.c   |  3 ++-
> >  target/ppc/translate_init.c |  3 ++-
> >  target/s390x/cpu_models.c   |  2 +-
> >  stubs/Makefile.objs |  1 -
> >  10 files changed, 10 insertions(+), 45 deletions(-)
> >  delete mode 100644 stubs/arch-query-cpu-def.c
> >
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index f5e1acff83..8e3949bca8 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -4433,7 +4433,9 @@
> >  #
> >  # Since: 1.2.0
> >  ##
> > -{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
> > +{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
> > +  'if': ['defined(NEED_CPU_H)',
> > + 'defined(TARGET_PPC) || defined(TARGET_ARM) ||
> defined(TARGET_I386) || defined(TARGET_S390X)'] }
> >
> >  ##
> >  # @CpuModelInfo:
> > diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
> > index fb3d20a1b8..e9721b9ce8 100644
> > --- a/include/sysemu/arch_init.h
> > +++ b/include/sysemu/arch_init.h
> > @@ -31,6 +31,4 @@ extern const uint32_t arch_type;
> >  int kvm_available(void);
> >  int xen_available(void);
> >
> > -CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp);
> > -
> >  #endif
> > diff --git a/monitor.c b/monitor.c
> > index b134c39144..6600819599 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -956,26 +956,6 @@ static void qmp_query_qmp_schema(QDict *qdict,
> QObject **ret_data,
> >  *ret_data = qobject_from_qlit(&qmp_schema_qlit);
> >  }
> >
> > -/*
> > - * We used to define commands in qmp-commands.hx in addition to the
> > - * QAPI schema.  This permitted defining some of them only in certain
> > - * configurations.  query-commands has always reflected that (good,
> > - * because it lets QMP clients figure out what's actually available),
> > - * while query-qmp-schema never did (not so good).  This function is a
> > - * hack to keep the configuration-specific commands defined exactly as
> > - * before, even though qmp-commands.hx is gone.
> > - *
> > - * FIXME Educate the QAPI schema on configuration-specific commands,
> > - * and drop this hack.
> > - */
> > -static void qmp_unregister_commands_hack(void)
> > -{
> > -#if !defined(TARGET_PPC) && !defined(TARGET_ARM) &&
> !defined(TARGET_I386) \
> > -&& !defined(TARGET_S390X)
> > -qmp_unregister_command(&qmp_commands, "query-cpu-definitions");
> > -#endif
> > -}
> > -
>
> Very nice to see this gone.  Its removal could be made a separate commit
> to highlight the achievement :)
>
> There are a few more candidates:
>
> * QERR_FEATURE_DISABLED leads me to
>   - query-hotpluggable-cpus via monitor.c
>   - x-colo-lost-heartbeat via colo-failover.c
>   - query-rocker, query-rocker-ports, query-rocker-of-dpa-flows,
> query-rocker-of-dpa-groups via qmp-norocker.c
>
> * QERR_UNSUPPORTED leads me to
>   - dump-guest-memory via dump_init() and stubs/dump.c
>   - query-vm-generation-id via stubs/vmgenid.c
>   - inject-nmi via nmi_monitor_handle() and s390_nmi()
>   - query-pci via pci-stub.c
>
> * grep error_set stubs/* doesn't find more
>

I have started to look at other opportunities in
https://github.com/elmarco/qemu/commits/qapi-if-more, but I have to revisit
that once we have that series ready. I guess this could be done case by
case later, and collaboratively.

Thanks for the review so far, I'll get back to it soon.


> >  void monitor_init_qmp_commands(void)
> >  {
> >  /*
> > @@ -995,8 +975,6 @@ void monitor_init_qmp_commands(void)
> >  qmp_register_command(&qmp_commands, "netdev_add", qmp_netdev_add,
> >   QCO_NO_OPTIONS);
> >
> > -qmp_unregister_commands_hack();
> > -
> >  QTAILQ_INIT(&qmp_cap_negotiation_commands);
> >  qmp_register_command(&qmp_cap_negotiation_commands,
> "qmp_capabilities",
> >   qmp_marshal_qmp_capabilities, QCO_NO_OPTIONS);
> > diff --git a/qmp.c b/qmp.c
> > index afa266ec1e..d57ccf1251 100644
> > --- a/qmp.c
> > +++ b/qmp.c
> > @@ -541,11 +541,6 @@ DevicePropertyInfoList
> *qmp_device_list_properties(const char *typename,
> >  return prop_list;
> >  }
> >
> > -CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
> > -{
> > -return arch_query_cpu_definitions(errp);
> > -}
> > -
> >  void qmp_add_client(const char *protocol, const char *fdname,
> >  bool has_skipauth, bool skipauth, bool has_tls,
> bool tls,
> >  Error **errp)
> > diff --git a/stubs/arch-query-cpu-def.c b/stubs/

Re: [Qemu-devel] [PATCH v1 for-2.11 09/10] s390x/kvm: move KVM declarations and stubs to separate files

2017-08-17 Thread Philippe Mathieu-Daudé

On 08/17/2017 09:35 AM, Thomas Huth wrote:

On 17.08.2017 13:40, Philippe Mathieu-Daudé wrote:

Hi David,

On 08/17/2017 06:22 AM, David Hildenbrand wrote:

Let's do it just like the other architectures. Introduce kvm-stub.c
for stubs and kvm_s390x.h for the declarations.

Add a fake declaration of struct kvm_s390_irq so we don't need other
ugly CONFIG_KVM checks.


You can use an opaque pointer to avoid that ("bridge" design pattern).

It involves few more changes but looks safer.


There is maybe even a simpler solution than that, see below ...

[...]

   feat-src = $(SRC_PATH)/target/$(TARGET_BASE_ARCH)/
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 74d5b35..aeb730c 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -41,6 +41,7 @@
   #include "exec/cpu-all.h"
 #include "fpu/softfloat.h"
+#include "kvm_s390x.h"


Do we still need that? cpu.h should theoretically be independent from
kvm now, shouldn't it? And for the .c files, it's likely better to
include kvm_s390x.h directly there if they require it.


Oh you totally right, I missed this include.

[...]

+#ifndef KVM_S390X_H
+#define KVM_S390X_H
+
+#include "sysemu/kvm.h"
+
+#ifndef CONFIG_KVM
+struct kvm_s390_irq {};
+#endif /* CONFIG_KVM */


change by

typedef struct kvm_s390_irq kvm_s390_irq_t;


May I suggest to simply use this instead:

struct kvm_s390_irq;

No need to switch for a typedef here, you can simply use this anonymous
struct declaration, I think.




Re: [Qemu-devel] [PATCH v1 for-2.11 09/10] s390x/kvm: move KVM declarations and stubs to separate files

2017-08-17 Thread Cornelia Huck
On Thu, 17 Aug 2017 14:35:53 +0200
Thomas Huth  wrote:

> On 17.08.2017 13:40, Philippe Mathieu-Daudé wrote:
> > Hi David,
> > 
> > On 08/17/2017 06:22 AM, David Hildenbrand wrote:  
> >> Let's do it just like the other architectures. Introduce kvm-stub.c
> >> for stubs and kvm_s390x.h for the declarations.
> >>
> >> Add a fake declaration of struct kvm_s390_irq so we don't need other
> >> ugly CONFIG_KVM checks.  
> > 
> > You can use an opaque pointer to avoid that ("bridge" design pattern).
> > 
> > It involves few more changes but looks safer.  
> 
> There is maybe even a simpler solution than that, see below ...
>
> >> diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
> >> new file mode 100644
> >> index 000..35db28c
> >> --- /dev/null
> >> +++ b/target/s390x/kvm_s390x.h
> >> @@ -0,0 +1,51 @@
> >> +/*
> >> + * QEMU KVM support -- s390x specific functions.
> >> + *
> >> + * Copyright (c) 2009 Ulrich Hecht
> >> + *
> >> + * This work is licensed under the terms of the GNU GPL, version 2 or
> >> later.
> >> + * See the COPYING file in the top-level directory.
> >> + */
> >> +
> >> +#ifndef KVM_S390X_H
> >> +#define KVM_S390X_H
> >> +
> >> +#include "sysemu/kvm.h"
> >> +
> >> +#ifndef CONFIG_KVM
> >> +struct kvm_s390_irq {};
> >> +#endif /* CONFIG_KVM */  
> > 
> > change by
> > 
> > typedef struct kvm_s390_irq kvm_s390_irq_t;  
> 
> May I suggest to simply use this instead:
> 
> struct kvm_s390_irq;
> 
> No need to switch for a typedef here, you can simply use this anonymous
> struct declaration, I think.

Yes, I think so.

I'm wondering if there are more cleanup opportunities...

We currently have the flic which is supposed to deal with floating
interrupts (in hw/intc/). It comes in a kvm (backed by a kvm device)
and a non-kvm flavour. Recent changes have introduced some adapter
interrupt handling in both the kvm device backing and in the non-kvm
flic.

Creation/injection of floating interrupts (like I/O interrupts) is
currently a bit scattered around the code, and the tcg code (which
predates the flic) does not have much in common with the kvm code. Can
we enhance the flic with one or more injection methods, move the tcg
interrupt creation into the non-kvm flic, and get rid of
kvm_s390_inject_flic()? Not sure how doable that is.



Re: [Qemu-devel] [PATCH v1 for-2.11 09/10] s390x/kvm: move KVM declarations and stubs to separate files

2017-08-17 Thread David Hildenbrand
On 17.08.2017 14:48, Cornelia Huck wrote:
> On Thu, 17 Aug 2017 14:35:53 +0200
> Thomas Huth  wrote:
> 
>> On 17.08.2017 13:40, Philippe Mathieu-Daudé wrote:
>>> Hi David,
>>>
>>> On 08/17/2017 06:22 AM, David Hildenbrand wrote:  
 Let's do it just like the other architectures. Introduce kvm-stub.c
 for stubs and kvm_s390x.h for the declarations.

 Add a fake declaration of struct kvm_s390_irq so we don't need other
 ugly CONFIG_KVM checks.  
>>>
>>> You can use an opaque pointer to avoid that ("bridge" design pattern).
>>>
>>> It involves few more changes but looks safer.  
>>
>> There is maybe even a simpler solution than that, see below ...
>>
 diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
 new file mode 100644
 index 000..35db28c
 --- /dev/null
 +++ b/target/s390x/kvm_s390x.h
 @@ -0,0 +1,51 @@
 +/*
 + * QEMU KVM support -- s390x specific functions.
 + *
 + * Copyright (c) 2009 Ulrich Hecht
 + *
 + * This work is licensed under the terms of the GNU GPL, version 2 or
 later.
 + * See the COPYING file in the top-level directory.
 + */
 +
 +#ifndef KVM_S390X_H
 +#define KVM_S390X_H
 +
 +#include "sysemu/kvm.h"
 +
 +#ifndef CONFIG_KVM
 +struct kvm_s390_irq {};
 +#endif /* CONFIG_KVM */  
>>>
>>> change by
>>>
>>> typedef struct kvm_s390_irq kvm_s390_irq_t;  
>>
>> May I suggest to simply use this instead:
>>
>> struct kvm_s390_irq;
>>
>> No need to switch for a typedef here, you can simply use this anonymous
>> struct declaration, I think.
> 
> Yes, I think so.
> 
> I'm wondering if there are more cleanup opportunities...
> 
> We currently have the flic which is supposed to deal with floating
> interrupts (in hw/intc/). It comes in a kvm (backed by a kvm device)
> and a non-kvm flavour. Recent changes have introduced some adapter
> interrupt handling in both the kvm device backing and in the non-kvm
> flic.
> 
> Creation/injection of floating interrupts (like I/O interrupts) is
> currently a bit scattered around the code, and the tcg code (which
> predates the flic) does not have much in common with the kvm code. Can
> we enhance the flic with one or more injection methods, move the tcg
> interrupt creation into the non-kvm flic, and get rid of
> kvm_s390_inject_flic()? Not sure how doable that is.
> 

There are many more possible cleanups, let's limit the scope of this
series. (it all started by wanting to move one function to cpu.h ...)

Thanks!

-- 

Thanks,

David



Re: [Qemu-devel] [PATCH v1 for-2.11 09/10] s390x/kvm: move KVM declarations and stubs to separate files

2017-08-17 Thread David Hildenbrand
On 17.08.2017 14:35, Thomas Huth wrote:
> On 17.08.2017 13:40, Philippe Mathieu-Daudé wrote:
>> Hi David,
>>
>> On 08/17/2017 06:22 AM, David Hildenbrand wrote:
>>> Let's do it just like the other architectures. Introduce kvm-stub.c
>>> for stubs and kvm_s390x.h for the declarations.
>>>
>>> Add a fake declaration of struct kvm_s390_irq so we don't need other
>>> ugly CONFIG_KVM checks.
>>
>> You can use an opaque pointer to avoid that ("bridge" design pattern).
>>
>> It involves few more changes but looks safer.
> 
> There is maybe even a simpler solution than that, see below ...
> 
> [...]
>>>   feat-src = $(SRC_PATH)/target/$(TARGET_BASE_ARCH)/
>>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>>> index 74d5b35..aeb730c 100644
>>> --- a/target/s390x/cpu.h
>>> +++ b/target/s390x/cpu.h
>>> @@ -41,6 +41,7 @@
>>>   #include "exec/cpu-all.h"
>>> #include "fpu/softfloat.h"
>>> +#include "kvm_s390x.h"
> 
> Do we still need that? cpu.h should theoretically be independent from
> kvm now, shouldn't it? And for the .c files, it's likely better to
> include kvm_s390x.h directly there if they require it.

It should work if:

a) we include "sysemu/kvm.h" in hw/s390x/s390-virtio-ccw.c
b) we include "target/s390x/kvm_s390x.h" in hw/intc/s390_flic_kvm.c
c) we include "kvm_s390x.h" in "internal.h"
d) we drop the "KVMState" parameter from kvm_s390_get_memslot_count()
(separate patch)

> 
> May I suggest to simply use this instead:
> 
> struct kvm_s390_irq;

That also seems to compile just fine.

> 
> No need to switch for a typedef here, you can simply use this anonymous
> struct declaration, I think.
> 
>  Thomas
> 


-- 

Thanks,

David



[Qemu-devel] [PATCH] follow-up path - " qcow2: add shrink image support"

2017-08-17 Thread Pavel Butsykin
Signed-off-by: Pavel Butsykin 
---
 qapi/block-core.json | 5 +
 1 file changed, 5 insertions(+)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index d6172bfe15..c55cd0c8db 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2479,6 +2479,11 @@
 #
 # Trigger events supported by blkdebug.
 #
+# @l1_shrink_write_table:  write zeros to the l1 table to shrink image.
+#  (since 2.11)
+#
+# @l1_shrink_free_l2_clusters: discard the l2 tables. (since 2.11)
+#
 # Since: 2.9
 ##
 { 'enum': 'BlkdebugEvent', 'prefix': 'BLKDBG',
-- 
2.14.1




Re: [Qemu-devel] [PATCH v1 for-2.11 09/10] s390x/kvm: move KVM declarations and stubs to separate files

2017-08-17 Thread Cornelia Huck
On Thu, 17 Aug 2017 14:53:08 +0200
David Hildenbrand  wrote:

> On 17.08.2017 14:48, Cornelia Huck wrote:
> > On Thu, 17 Aug 2017 14:35:53 +0200
> > Thomas Huth  wrote:
> >   
> >> On 17.08.2017 13:40, Philippe Mathieu-Daudé wrote:  
> >>> Hi David,
> >>>
> >>> On 08/17/2017 06:22 AM, David Hildenbrand wrote:
>  Let's do it just like the other architectures. Introduce kvm-stub.c
>  for stubs and kvm_s390x.h for the declarations.
> 
>  Add a fake declaration of struct kvm_s390_irq so we don't need other
>  ugly CONFIG_KVM checks.
> >>>
> >>> You can use an opaque pointer to avoid that ("bridge" design pattern).
> >>>
> >>> It involves few more changes but looks safer.
> >>
> >> There is maybe even a simpler solution than that, see below ...
> >>  
>  diff --git a/target/s390x/kvm_s390x.h b/target/s390x/kvm_s390x.h
>  new file mode 100644
>  index 000..35db28c
>  --- /dev/null
>  +++ b/target/s390x/kvm_s390x.h
>  @@ -0,0 +1,51 @@
>  +/*
>  + * QEMU KVM support -- s390x specific functions.
>  + *
>  + * Copyright (c) 2009 Ulrich Hecht
>  + *
>  + * This work is licensed under the terms of the GNU GPL, version 2 or
>  later.
>  + * See the COPYING file in the top-level directory.
>  + */
>  +
>  +#ifndef KVM_S390X_H
>  +#define KVM_S390X_H
>  +
>  +#include "sysemu/kvm.h"
>  +
>  +#ifndef CONFIG_KVM
>  +struct kvm_s390_irq {};
>  +#endif /* CONFIG_KVM */
> >>>
> >>> change by
> >>>
> >>> typedef struct kvm_s390_irq kvm_s390_irq_t;
> >>
> >> May I suggest to simply use this instead:
> >>
> >> struct kvm_s390_irq;
> >>
> >> No need to switch for a typedef here, you can simply use this anonymous
> >> struct declaration, I think.  
> > 
> > Yes, I think so.
> > 
> > I'm wondering if there are more cleanup opportunities...
> > 
> > We currently have the flic which is supposed to deal with floating
> > interrupts (in hw/intc/). It comes in a kvm (backed by a kvm device)
> > and a non-kvm flavour. Recent changes have introduced some adapter
> > interrupt handling in both the kvm device backing and in the non-kvm
> > flic.
> > 
> > Creation/injection of floating interrupts (like I/O interrupts) is
> > currently a bit scattered around the code, and the tcg code (which
> > predates the flic) does not have much in common with the kvm code. Can
> > we enhance the flic with one or more injection methods, move the tcg
> > interrupt creation into the non-kvm flic, and get rid of
> > kvm_s390_inject_flic()? Not sure how doable that is.
> >   
> 
> There are many more possible cleanups, let's limit the scope of this
> series. (it all started by wanting to move one function to cpu.h ...)

What, you don't want to rewrite everything? :)

No, that was just an idea for the future, no need to expand your series
further.



  1   2   3   >