Re: [Qemu-block] [PATCH v1 1/1] iotests: bypass s390x for case 200

[Christian Borntraeger]

Nack. This will be fixed by 

s390/ipl: only print boot menu error if -boot menu=on was specified

On 03/06/2018 08:54 AM, QingFeng Hao wrote:
> In s390x, the case 200 failed as:
>  === Starting QEMU VM ===
> 
> +QEMU_PROG: boot menu is not supported for this device type.
>  {"return": {}}
> 
>  === Sending stream/cancel, checking for SIGSEGV only ===
> Failures: 200
> Failed 1 of 1 tests
> 
> It was caused by the command which isn't supported by s390x now:
> qemu-system-s390x -device pci-bridge,id=bridge1,chassis_nr=1,bus=pci.0 
> -object iothread,id=iothread0 -device 
> virtio-scsi-pci,bus=bridge1,addr=0x1f,id=scsi0,iothread=iothread0 -drive 
> file=.../scratch/test.img,media=disk,if=none,cache=writeback,id=drive_sysdisk,format=qcow2
>  -device scsi-hd,drive=drive_sysdisk,bus=scsi0.0,id=sysdisk,bootindex=0 
> -nographic
> 
> Signed-off-by: QingFeng Hao 
> ---
>  tests/qemu-iotests/200 | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/qemu-iotests/200 b/tests/qemu-iotests/200
> index ddbdedc476..7e53bd7774 100755
> --- a/tests/qemu-iotests/200
> +++ b/tests/qemu-iotests/200
> @@ -45,6 +45,10 @@ _supported_fmt qcow2 qed
>  _supported_proto file
>  _supported_os Linux
> 
> +if [ "$QEMU_DEFAULT_MACHINE" != "pc" ]; then
> +_notrun "Requires a PC machine"
> +fi
> +
>  BACKING_IMG="${TEST_DIR}/backing.img"
>  TEST_IMG="${TEST_DIR}/test.img"
> 

[Qemu-block] [PATCH v1 1/1] iotests: bypass s390x for case 200

[QingFeng Hao]

In s390x, the case 200 failed as:
 === Starting QEMU VM ===

+QEMU_PROG: boot menu is not supported for this device type.
 {"return": {}}

 === Sending stream/cancel, checking for SIGSEGV only ===
Failures: 200
Failed 1 of 1 tests

It was caused by the command which isn't supported by s390x now:
qemu-system-s390x -device pci-bridge,id=bridge1,chassis_nr=1,bus=pci.0 -object 
iothread,id=iothread0 -device 
virtio-scsi-pci,bus=bridge1,addr=0x1f,id=scsi0,iothread=iothread0 -drive 
file=.../scratch/test.img,media=disk,if=none,cache=writeback,id=drive_sysdisk,format=qcow2
 -device scsi-hd,drive=drive_sysdisk,bus=scsi0.0,id=sysdisk,bootindex=0 
-nographic

Signed-off-by: QingFeng Hao 
---
 tests/qemu-iotests/200 | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/qemu-iotests/200 b/tests/qemu-iotests/200
index ddbdedc476..7e53bd7774 100755
--- a/tests/qemu-iotests/200
+++ b/tests/qemu-iotests/200
@@ -45,6 +45,10 @@ _supported_fmt qcow2 qed
 _supported_proto file
 _supported_os Linux
 
+if [ "$QEMU_DEFAULT_MACHINE" != "pc" ]; then
+_notrun "Requires a PC machine"
+fi
+
 BACKING_IMG="${TEST_DIR}/backing.img"
 TEST_IMG="${TEST_DIR}/test.img"
 
-- 
2.13.5

Re: [Qemu-block] [Qemu-devel] [PATCH] nbd/server: fix space read

[Eric Blake]

On 03/05/2018 01:47 PM, Eric Blake wrote:

On 03/05/2018 12:04 PM, Vladimir Sementsov-Ogievskiy wrote:


In the subject line: s/space/sparse/


In case of io error in nbd_co_send_sparse_read we should not
"goto reply:", as it is fatal error and common behavior is
disconnect in this case. We should not try to send client an
error reply, representing channel-io error on previous try to
send a reply.


Good catch.



Fix this by handle block-status error in nbd_co_send_sparse_read,
so nbd_co_send_sparse_read fails only on io error. Then just skip
common "reply:" code path in nbd_trip.

Note: nbd_co_send_structured_error is moved without changes to be
called from nbd_co_send_sparse_read.


Might be easier to read as two patches, one for the code motion, the 
other for using the new code.  But I'm not going to insist on a split.



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

Re: [Qemu-block] [Qemu-devel] [PULL 27/37] block: extract AIO_WAIT_WHILE() from BlockDriverState

[Eric Blake]

On 03/02/2018 12:54 PM, Kevin Wolf wrote:

From: Stefan Hajnoczi 

BlockDriverState has the BDRV_POLL_WHILE() macro to wait on event loop
activity while a condition evaluates to true.  This is used to implement
synchronous operations where it acts as a condvar between the IOThread
running the operation and the main loop waiting for the operation.  It
can also be called from the thread that owns the AioContext and in that
case it's just a nested event loop.

BlockBackend needs this behavior but doesn't always have a




Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Eric Blake 
Signed-off-by: Kevin Wolf 
---



+++ b/include/block/block.h
@@ -2,6 +2,7 @@
  #define BLOCK_H
  
  #include "block/aio.h"

+#include "block/aio-wait.h"
  #include "qapi-types.h"


Will need a minor rebase now that commit 9af23989 has renamed qapi-types.h.

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

Re: [Qemu-block] [PATCH] nbd/server: fix space read

[Eric Blake]

On 03/05/2018 12:04 PM, Vladimir Sementsov-Ogievskiy wrote:

In case of io error in nbd_co_send_sparse_read we should not
"goto reply:", as it is fatal error and common behavior is
disconnect in this case. We should not try to send client an
error reply, representing channel-io error on previous try to
send a reply.


Good catch.



Fix this by handle block-status error in nbd_co_send_sparse_read,
so nbd_co_send_sparse_read fails only on io error. Then just skip
common "reply:" code path in nbd_trip.

Note: nbd_co_send_structured_error is moved without changes to be
called from nbd_co_send_sparse_read.


Might be easier to read as two patches, one for the code motion, the 
other for using the new code.  But I'm not going to insist on a split.




Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

PS: this all shows that nbd_trip is tooo complex (and yes, I remember,
that its final semantics was made by myself :(.. It should be
refactored into several smaller parts. Do you have any ideas?

The complexity here is that we should handle channel errors (fatal) and
export errors(non fatal) in different ways, and both of error types may
have errp, which should be handled differently too..

May be, the best way is to make separate functions for each command,
avoiding code duplication by using helper-functions instead of common code
in nbd_trip.


Yes, splitting nbd_trip into smaller helper functions may be worthwhile.



  nbd/server.c | 64 +++-
  1 file changed, 37 insertions(+), 27 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index 4990a5826e..ea6b9467e4 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -21,6 +21,7 @@
  #include "qapi/error.h"
  #include "trace.h"
  #include "nbd-internal.h"
+#include "qemu/error-report.h"


I'm a bit worried about this one.  The server has previously not needed 
this, so it may be the wrong thing to pull it in now.




+
  static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
  uint64_t handle,
  uint64_t offset,


It doesn't help that we are lacking comments on the contract of this 
function.



@@ -1361,8 +1386,15 @@ static int coroutine_fn 
nbd_co_send_sparse_read(NBDClient *client,
  bool final;
  
  if (status < 0) {

-error_setg_errno(errp, -status, "unable to check for holes");
-return status;
+char *msg = g_strdup_printf("unable to check for holes: %s",
+  strerror(-status));


Indentation looks off.


+
+error_report("%s", msg);


Do we really need to report this on the server's stderr, or is sending 
the reply to the client good enough?



+
+ret = nbd_co_send_structured_error(client, handle, -status, msg,
+   errp);
+g_free(msg);
+return ret;


So if we have an early return here, then pre-patch, we were 
unconditionally setting errp and returning -1 (even if the client is 
still alive); post-patch errp is only set if we failed to do I/O with 
the client.  That change is right, but harder to see without comments 
giving a function contract.



@@ -1567,7 +1575,7 @@ static coroutine_fn void nbd_trip(void *opaque)
request.from, req->data, 
request.len,
_err);
  if (ret < 0) {
-goto reply;
+goto replied;
  }
  goto done;
  }
@@ -1664,6 +1672,8 @@ reply:
 req->data, reply_data_len, _err);
  }
  g_free(msg);
+
+replied:


This part makes sense.


  if (ret < 0) {
  error_prepend(_err, "Failed to send reply: ");
  goto disconnect;



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

Re: [Qemu-block] [PULL 0/6] Block patches

[Peter Maydell]

On 5 March 2018 at 09:40, Stefan Hajnoczi  wrote:
> The following changes since commit 136c67e07869227b21b3f627316e03679ce7b738:
>
>   Merge remote-tracking branch 
> 'remotes/bkoppelmann/tags/pull-tricore-2018-03-02' into staging (2018-03-02 
> 16:56:20 +)
>
> are available in the Git repository at:
>
>   git://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to 23500c6a9409efc80d696aede0629bfbe7556a90:
>
>   README: Document 'git-publish' workflow (2018-03-05 09:03:17 +)
>
> 
> Pull request
>
> Mostly patches that are only indirectly related to the block layer, but I've
> reviewed them and there is no maintainer.
>
> 
>
> Fam Zheng (2):
>   Add a git-publish configuration file
>   README: Document 'git-publish' workflow
>
> Su Hang (3):
>   util/uri.c: Coding style check, Only whitespace involved
>   util/uri.c: remove brackets that wrap `return` statement's content.
>   util/uri.c: wrap single statement blocks with braces {}
>
> Thomas Huth (1):
>   tests/libqos: Check for valid dev pointer when looking for PCI devices
>
>  tests/libqos/virtio-pci.c |4 +-
>  util/uri.c| 1733 
> -
>  .gitpublish   |   51 ++
>  README|   31 +-
>  4 files changed, 1014 insertions(+), 805 deletions(-)
>  create mode 100644 .gitpublish

Applied, thanks.

-- PMM

[Qemu-block] [PATCH] nbd/server: fix space read

[Vladimir Sementsov-Ogievskiy]

In case of io error in nbd_co_send_sparse_read we should not
"goto reply:", as it is fatal error and common behavior is
disconnect in this case. We should not try to send client an
error reply, representing channel-io error on previous try to
send a reply.

Fix this by handle block-status error in nbd_co_send_sparse_read,
so nbd_co_send_sparse_read fails only on io error. Then just skip
common "reply:" code path in nbd_trip.

Note: nbd_co_send_structured_error is moved without changes to be
called from nbd_co_send_sparse_read.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

PS: this all shows that nbd_trip is tooo complex (and yes, I remember,
that its final semantics was made by myself :(.. It should be
refactored into several smaller parts. Do you have any ideas?

The complexity here is that we should handle channel errors (fatal) and
export errors(non fatal) in different ways, and both of error types may
have errp, which should be handled differently too..

May be, the best way is to make separate functions for each command,
avoiding code duplication by using helper-functions instead of common code
in nbd_trip.

 nbd/server.c | 64 +++-
 1 file changed, 37 insertions(+), 27 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index 4990a5826e..ea6b9467e4 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -21,6 +21,7 @@
 #include "qapi/error.h"
 #include "trace.h"
 #include "nbd-internal.h"
+#include "qemu/error-report.h"
 
 static int system_errno_to_nbd_errno(int err)
 {
@@ -1341,6 +1342,30 @@ static int coroutine_fn 
nbd_co_send_structured_read(NBDClient *client,
 return nbd_co_send_iov(client, iov, 2, errp);
 }
 
+static int coroutine_fn nbd_co_send_structured_error(NBDClient *client,
+ uint64_t handle,
+ uint32_t error,
+ const char *msg,
+ Error **errp)
+{
+NBDStructuredError chunk;
+int nbd_err = system_errno_to_nbd_errno(error);
+struct iovec iov[] = {
+{.iov_base = , .iov_len = sizeof(chunk)},
+{.iov_base = (char *)msg, .iov_len = msg ? strlen(msg) : 0},
+};
+
+assert(nbd_err);
+trace_nbd_co_send_structured_error(handle, nbd_err,
+   nbd_err_lookup(nbd_err), msg ? msg : 
"");
+set_be_chunk(, NBD_REPLY_FLAG_DONE, NBD_REPLY_TYPE_ERROR, handle,
+ sizeof(chunk) - sizeof(chunk.h) + iov[1].iov_len);
+stl_be_p(, nbd_err);
+stw_be_p(_length, iov[1].iov_len);
+
+return nbd_co_send_iov(client, iov, 1 + !!iov[1].iov_len, errp);
+}
+
 static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
 uint64_t handle,
 uint64_t offset,
@@ -1361,8 +1386,15 @@ static int coroutine_fn 
nbd_co_send_sparse_read(NBDClient *client,
 bool final;
 
 if (status < 0) {
-error_setg_errno(errp, -status, "unable to check for holes");
-return status;
+char *msg = g_strdup_printf("unable to check for holes: %s",
+  strerror(-status));
+
+error_report("%s", msg);
+
+ret = nbd_co_send_structured_error(client, handle, -status, msg,
+   errp);
+g_free(msg);
+return ret;
 }
 assert(pnum && pnum <= size - progress);
 final = progress + pnum == size;
@@ -1400,30 +1432,6 @@ static int coroutine_fn 
nbd_co_send_sparse_read(NBDClient *client,
 return ret;
 }
 
-static int coroutine_fn nbd_co_send_structured_error(NBDClient *client,
- uint64_t handle,
- uint32_t error,
- const char *msg,
- Error **errp)
-{
-NBDStructuredError chunk;
-int nbd_err = system_errno_to_nbd_errno(error);
-struct iovec iov[] = {
-{.iov_base = , .iov_len = sizeof(chunk)},
-{.iov_base = (char *)msg, .iov_len = msg ? strlen(msg) : 0},
-};
-
-assert(nbd_err);
-trace_nbd_co_send_structured_error(handle, nbd_err,
-   nbd_err_lookup(nbd_err), msg ? msg : 
"");
-set_be_chunk(, NBD_REPLY_FLAG_DONE, NBD_REPLY_TYPE_ERROR, handle,
- sizeof(chunk) - sizeof(chunk.h) + iov[1].iov_len);
-stl_be_p(, nbd_err);
-stw_be_p(_length, iov[1].iov_len);
-
-return nbd_co_send_iov(client, iov, 1 + !!iov[1].iov_len, errp);
-}
-
 /* nbd_co_receive_request
  * Collect a client request. Return 0 if request looks valid, -EIO to drop
  * connection right away, and any other 

[Qemu-block] [PULL v2 00/38] Block layer patches

[Kevin Wolf]

The following changes since commit 86f4c7e05b1c44dbe1b329a51f311f10aef6ff34:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20180302' 
into staging (2018-03-02 14:37:10 +)

are available in the git repository at:

  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to bfe1a14c180ec44c033be12b9151252ffda69292:

  block: Fix NULL dereference on empty drive error (2018-03-05 18:45:32 +0100)


Block layer patches


Alberto Garcia (3):
  specs/qcow2: Fix documentation of the compressed cluster descriptor
  docs: document how to use the l2-cache-entry-size parameter
  qcow2: Replace align_offset() with ROUND_UP()

Anton Nefedov (2):
  block: fix write with zero flag set and iovector provided
  iotest 033: add misaligned write-zeroes test via truncate

Eric Blake (21):
  block: Add .bdrv_co_block_status() callback
  nvme: Drop pointless .bdrv_co_get_block_status()
  block: Switch passthrough drivers to .bdrv_co_block_status()
  file-posix: Switch to .bdrv_co_block_status()
  gluster: Switch to .bdrv_co_block_status()
  iscsi: Switch cluster_sectors to byte-based
  iscsi: Switch iscsi_allocmap_update() to byte-based
  iscsi: Switch to .bdrv_co_block_status()
  null: Switch to .bdrv_co_block_status()
  parallels: Switch to .bdrv_co_block_status()
  qcow: Switch to .bdrv_co_block_status()
  qcow2: Switch to .bdrv_co_block_status()
  qed: Switch to .bdrv_co_block_status()
  raw: Switch to .bdrv_co_block_status()
  sheepdog: Switch to .bdrv_co_block_status()
  vdi: Avoid bitrot of debugging code
  vdi: Switch to .bdrv_co_block_status()
  vmdk: Switch to .bdrv_co_block_status()
  vpc: Switch to .bdrv_co_block_status()
  vvfat: Switch to .bdrv_co_block_status()
  block: Drop unused .bdrv_co_get_block_status()

Kevin Wolf (3):
  block: test blk_aio_flush() with blk->root == NULL
  Merge remote-tracking branch 'mreitz/tags/pull-block-2018-03-02' into 
queue-block
  block: Fix NULL dereference on empty drive error

Max Reitz (4):
  qemu-img: Make resize error message more general
  block/ssh: Pull ssh_grow_file() from ssh_create()
  block/ssh: Make ssh_grow_file() blocking
  block/ssh: Add basic .bdrv_truncate()

Stefan Hajnoczi (6):
  aio: rename aio_context_in_iothread() to in_aio_context_home_thread()
  block: extract AIO_WAIT_WHILE() from BlockDriverState
  block: add BlockBackend->in_flight counter
  Revert "IDE: Do not flush empty CDROM drives"
  block: rename .bdrv_create() to .bdrv_co_create_opts()
  qcow2: make qcow2_co_create2() a coroutine_fn

 qapi/block-core.json   |   6 +-
 docs/interop/qcow2.txt |  16 -
 docs/qcow2-cache.txt   |  46 -
 block/qcow2.h  |   6 --
 include/block/aio-wait.h   | 116 
 include/block/aio.h|   7 +-
 include/block/block.h  |  54 ---
 include/block/block_int.h  |  61 ++---
 block.c|  11 ++-
 block/blkdebug.c   |  20 +++---
 block/block-backend.c  |  65 +++---
 block/commit.c |   2 +-
 block/crypto.c |   8 +--
 block/file-posix.c |  79 +++---
 block/file-win32.c |   5 +-
 block/gluster.c|  83 ---
 block/io.c |  98 +++
 block/iscsi.c  | 164 -
 block/mirror.c |   2 +-
 block/nfs.c|   5 +-
 block/null.c   |  23 ---
 block/nvme.c   |  14 
 block/parallels.c  |  28 +---
 block/qcow.c   |  32 +
 block/qcow2-bitmap.c   |   4 +-
 block/qcow2-cluster.c  |   4 +-
 block/qcow2-refcount.c |   4 +-
 block/qcow2-snapshot.c |  10 +--
 block/qcow2.c  |  60 +
 block/qed.c|  82 ---
 block/raw-format.c |  21 +++---
 block/rbd.c|   6 +-
 block/sheepdog.c   |  36 +-
 block/ssh.c|  66 +++---
 block/throttle.c   |   2 +-
 block/vdi.c|  50 +++---
 block/vhdx.c   |   5 +-
 block/vmdk.c   |  43 +---
 block/vpc.c|  50 +++---
 block/vvfat.c  |  16 ++---
 hw/ide/core.c  |  10 +--
 qemu-img.c |   2 +-
 tests/test-block-backend.c |  82 +++
 util/aio-wait.c|  40 +++
 tests/Makefile.include |   2 +
 tests/qemu-iotests/033 |  29 
 tests/qemu-iotests/033.out |  13 
 util/Makefile.objs |   2 +-
 48 files changed, 980 

Re: [Qemu-block] [PATCH] iotests: Skip test for ENOMEM error

[Max Reitz]

On 2018-03-01 02:14, Fam Zheng wrote:
> The AFL image is to exercise the code validating image size, which
> doesn't work on 32 bit or when out of memory (there is a large
> allocation before the interesting point). So check that and skip the
> test, instead of faking the result.
> 
> Signed-off-by: Fam Zheng 
> ---
>  tests/qemu-iotests/059 | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

Thanks!  Applied to my block branch:

https://github.com/XanClic/qemu/commits/block

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH v3] iotests: Tweak 030 in order to trigger a race condition with parallel jobs

[Max Reitz]

On 2018-03-02 12:20, Alberto Garcia wrote:
> This patch tweaks TestParallelOps in iotest 030 so it allocates data
> in smaller regions (256KB/512KB instead of 512KB/1MB) and the
> block-stream job in test_stream_commit() only needs to copy data that
> is at the very end of the image.
> 
> This way when the block-stream job is awakened it will finish right
> away without any chance of being stopped by block_job_sleep_ns(). This
> triggers the bug that was fixed by 3d5d319e1221082974711af1d09d82f07
> and is therefore a more useful test case for parallel block jobs.
> 
> After this patch the aforementiond bug can also be reproduced with the
> test_stream_parallel() test case.
> 
> Since with this change the stream job in test_stream_commit() finishes
> early, this patch introduces a similar test case where both jobs are
> slowed down so they can actually run in parallel.
> 
> Signed-off-by: Alberto Garcia 
> Reviewed-by: John Snow 
> ---
> 
> This patch was sent already in December but it seems to have been
> forgotten. v3 is the same as v2 but with a typo fixed in the commit
> message.
> 
> ---
>  tests/qemu-iotests/030 | 48 
> +++---
>  tests/qemu-iotests/030.out |  4 ++--
>  2 files changed, 43 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
> index 457984b8e9..44ad1e311f 100755
> --- a/tests/qemu-iotests/030
> +++ b/tests/qemu-iotests/030
> @@ -156,7 +156,7 @@ class TestSingleDrive(iotests.QMPTestCase):
>  class TestParallelOps(iotests.QMPTestCase):
>  num_ops = 4 # Number of parallel block-stream operations
>  num_imgs = num_ops * 2 + 1
> -image_len = num_ops * 1024 * 1024
> +image_len = num_ops * 512 * 1024
>  imgs = []
>  
>  def setUp(self):
> @@ -177,12 +177,12 @@ class TestParallelOps(iotests.QMPTestCase):
>  
>  # Put data into the images we are copying data from
>  for i in range(self.num_imgs / 2):
> -img_index = i * 2 + 1
> -# Alternate between 512k and 1M.
> +img_index = self.num_imgs - i * 2 - 2

First of all, I don't like this very much because it's not clear that
img_index is going to be odd.

I'd prefer something like

  reverse_i = self.num_imgs / 2 - 1 - 1
  img_index = reverse_i * 2 + 1

Secondly, I've reverted 3d5d319e1221082 to test this, and I could
reproduce failure exactly once.  Since then, no luck (in like 20
attempts, I think)...

Max

> +# Alternate between 256KB and 512KB.
>  # This way jobs will not finish in the same order they were 
> created
> -num_kb = 512 + 512 * (i % 2)
> +num_kb = 256 + 256 * (i % 2)
>  qemu_io('-f', iotests.imgfmt,
> -'-c', 'write -P %d %d %d' % (i, i*1024*1024, num_kb * 
> 1024),
> +'-c', 'write -P 0xFF %dk %dk' % (i * 512, num_kb),
>  self.imgs[img_index])
>  
>  # Attach the drive to the VM
> @@ -318,12 +318,14 @@ class TestParallelOps(iotests.QMPTestCase):
>  self.wait_until_completed(drive='commit-drive0')
>  
>  # Test a block-stream and a block-commit job in parallel
> -def test_stream_commit(self):
> +# Here the stream job is supposed to finish quickly in order to reproduce
> +# the scenario that triggers the bug fixed in 3d5d319e1221082974711af1d09
> +def test_stream_commit_1(self):
>  self.assertLessEqual(8, self.num_imgs)
>  self.assert_no_active_block_jobs()
>  
>  # Stream from node0 into node2
> -result = self.vm.qmp('block-stream', device='node2', job_id='node2')
> +result = self.vm.qmp('block-stream', device='node2', 
> base_node='node0', job_id='node2')
>  self.assert_qmp(result, 'return', {})
>  
>  # Commit from the active layer into node3
> @@ -348,6 +350,38 @@ class TestParallelOps(iotests.QMPTestCase):
>  
>  self.assert_no_active_block_jobs()
>  
> +# This is similar to test_stream_commit_1 but both jobs are slowed
> +# down so they can run in parallel for a little while.
> +def test_stream_commit_2(self):
> +self.assertLessEqual(8, self.num_imgs)
> +self.assert_no_active_block_jobs()
> +
> +# Stream from node0 into node4
> +result = self.vm.qmp('block-stream', device='node4', 
> base_node='node0', job_id='node4', speed=1024*1024)
> +self.assert_qmp(result, 'return', {})
> +
> +# Commit from the active layer into node5
> +result = self.vm.qmp('block-commit', device='drive0', 
> base=self.imgs[5], speed=1024*1024)
> +self.assert_qmp(result, 'return', {})
> +
> +# Wait for all jobs to be finished.
> +pending_jobs = ['node4', 'drive0']
> +while len(pending_jobs) > 0:
> +for event in self.vm.get_qmp_events(wait=True):
> +if event['event'] == 'BLOCK_JOB_COMPLETED':
> +

Re: [Qemu-block] [PATCH] block: Fix NULL dereference on empty drive error

[Kevin Wolf]

Am 05.03.2018 um 17:10 hat Eric Blake geschrieben:
> On 03/05/2018 09:05 AM, Kevin Wolf wrote:
> > blk_error_action() sends a BLOCK_IO_ERROR QMP event which includes the
> > node name of its root node. If the BlockBackend represents an empty
> > drive, there is no root node, so we should not try to access its node
> > name. Make the field optional in the event and include it only when
> > the BlockBackend isn't empty.
> > 
> > Signed-off-by: Kevin Wolf 
> > ---
> > 
> > Stefan, this is needed for your patch that reverts the workaround in the
> > IDE flush code. Without it, make check seems to succeed, but if you look
> > closer, qemu actually segfaults.
> > 
> > qapi/block-core.json  | 6 --
> >   block/block-backend.c | 5 +++--
> >   2 files changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 5c5921bfb7..00475f08d4 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -3676,7 +3676,8 @@
> >   #
> >   # @node-name: node name. Note that errors may be reported for the root 
> > node
> >   # that is directly attached to a guest device rather than for 
> > the
> > -# node where the error occurred. (Since: 2.8)
> > +# node where the error occurred. The node name is not present 
> > if
> > +# the drive is empty. (Since: 2.8)
> 
> Making an output field change from always present to sometimes absent
> might break older clients that expected to be able to parse the field
> unconditionally.  Would it be better to keep the 'node-name' field
> mandatory in the output but make it an empty string?

I considered that, but how likely is it that a client can handle an
empty string instead of a valid node name, but can't handle an absent
field? I assume that such clients would probably break either way. And
in that case I preferred to use the cleaner design.

> Then again, since the field was not present prior to 2.8, but the event
> itself is older, we can argue that clients of older qemu have to be prepared
> for the field to not be present.  So I think I can live with this change
> as-is.

Right, that too. If libvirt can deal with it (and I suppose it can
because it doesn't really use node names yet), we should be okay.
> 
> >   #
> >   # @operation: I/O operation
> >   #
> > @@ -3707,7 +3708,8 @@
> >   #
> >   ##
> >   { 'event': 'BLOCK_IO_ERROR',
> > -  'data': { 'device': 'str', 'node-name': 'str', 'operation': 
> > 'IoOperationType',
> > +  'data': { 'device': 'str', '*node-name': 'str',
> > +'operation': 'IoOperationType',
> >   'action': 'BlockErrorAction', '*nospace': 'bool',
> >   'reason': 'str' } }
> 
> Reviewed-by: Eric Blake 

Thanks.

Kevin

Re: [Qemu-block] [PATCH v2] iotests: Mark all tests executable

[Max Reitz]

On 2018-03-05 17:18, Eric Blake wrote:
> The majority of our iotests have the executable bit set; fix the
> few outliers for consistency.
> 
> Signed-off-by: Eric Blake 
> 
> ---
> v2: fix missing test [Max]
> 
> Note that checkpatch.pl complains that this doesn't look like a patch,
> because it is only changing mode bits.
> ---
>  tests/qemu-iotests/096 | 0
>  tests/qemu-iotests/124 | 0
>  tests/qemu-iotests/129 | 0
>  tests/qemu-iotests/132 | 0
>  tests/qemu-iotests/136 | 0
>  tests/qemu-iotests/139 | 0
>  tests/qemu-iotests/148 | 0
>  tests/qemu-iotests/152 | 0
>  tests/qemu-iotests/163 | 0
>  tests/qemu-iotests/205 | 0
>  10 files changed, 0 insertions(+), 0 deletions(-)
>  mode change 100644 => 100755 tests/qemu-iotests/096
>  mode change 100644 => 100755 tests/qemu-iotests/124
>  mode change 100644 => 100755 tests/qemu-iotests/129
>  mode change 100644 => 100755 tests/qemu-iotests/132
>  mode change 100644 => 100755 tests/qemu-iotests/136
>  mode change 100644 => 100755 tests/qemu-iotests/139
>  mode change 100644 => 100755 tests/qemu-iotests/148
>  mode change 100644 => 100755 tests/qemu-iotests/152
>  mode change 100644 => 100755 tests/qemu-iotests/163
>  mode change 100644 => 100755 tests/qemu-iotests/205

Thanks, applied to my block branch:

https://github.com/XanClic/qemu/commits/block

Max



signature.asc
Description: OpenPGP digital signature

[Qemu-block] [PATCH v2] iotests: Mark all tests executable

[Eric Blake]

The majority of our iotests have the executable bit set; fix the
few outliers for consistency.

Signed-off-by: Eric Blake 

---
v2: fix missing test [Max]

Note that checkpatch.pl complains that this doesn't look like a patch,
because it is only changing mode bits.
---
 tests/qemu-iotests/096 | 0
 tests/qemu-iotests/124 | 0
 tests/qemu-iotests/129 | 0
 tests/qemu-iotests/132 | 0
 tests/qemu-iotests/136 | 0
 tests/qemu-iotests/139 | 0
 tests/qemu-iotests/148 | 0
 tests/qemu-iotests/152 | 0
 tests/qemu-iotests/163 | 0
 tests/qemu-iotests/205 | 0
 10 files changed, 0 insertions(+), 0 deletions(-)
 mode change 100644 => 100755 tests/qemu-iotests/096
 mode change 100644 => 100755 tests/qemu-iotests/124
 mode change 100644 => 100755 tests/qemu-iotests/129
 mode change 100644 => 100755 tests/qemu-iotests/132
 mode change 100644 => 100755 tests/qemu-iotests/136
 mode change 100644 => 100755 tests/qemu-iotests/139
 mode change 100644 => 100755 tests/qemu-iotests/148
 mode change 100644 => 100755 tests/qemu-iotests/152
 mode change 100644 => 100755 tests/qemu-iotests/163
 mode change 100644 => 100755 tests/qemu-iotests/205

diff --git a/tests/qemu-iotests/096 b/tests/qemu-iotests/096
old mode 100644
new mode 100755
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
old mode 100644
new mode 100755
diff --git a/tests/qemu-iotests/129 b/tests/qemu-iotests/129
old mode 100644
new mode 100755
diff --git a/tests/qemu-iotests/132 b/tests/qemu-iotests/132
old mode 100644
new mode 100755
diff --git a/tests/qemu-iotests/136 b/tests/qemu-iotests/136
old mode 100644
new mode 100755
diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139
old mode 100644
new mode 100755
diff --git a/tests/qemu-iotests/148 b/tests/qemu-iotests/148
old mode 100644
new mode 100755
diff --git a/tests/qemu-iotests/152 b/tests/qemu-iotests/152
old mode 100644
new mode 100755
diff --git a/tests/qemu-iotests/163 b/tests/qemu-iotests/163
old mode 100644
new mode 100755
diff --git a/tests/qemu-iotests/205 b/tests/qemu-iotests/205
old mode 100644
new mode 100755
-- 
2.14.3

Re: [Qemu-block] [PATCH] block: Fix NULL dereference on empty drive error

[Eric Blake]

On 03/05/2018 09:05 AM, Kevin Wolf wrote:

blk_error_action() sends a BLOCK_IO_ERROR QMP event which includes the
node name of its root node. If the BlockBackend represents an empty
drive, there is no root node, so we should not try to access its node
name. Make the field optional in the event and include it only when
the BlockBackend isn't empty.

Signed-off-by: Kevin Wolf 
---

Stefan, this is needed for your patch that reverts the workaround in the
IDE flush code. Without it, make check seems to succeed, but if you look
closer, qemu actually segfaults.

qapi/block-core.json  | 6 --
  block/block-backend.c | 5 +++--
  2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5c5921bfb7..00475f08d4 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3676,7 +3676,8 @@
  #
  # @node-name: node name. Note that errors may be reported for the root node
  # that is directly attached to a guest device rather than for the
-# node where the error occurred. (Since: 2.8)
+# node where the error occurred. The node name is not present if
+# the drive is empty. (Since: 2.8)


Making an output field change from always present to sometimes absent 
might break older clients that expected to be able to parse the field 
unconditionally.  Would it be better to keep the 'node-name' field 
mandatory in the output but make it an empty string?


Then again, since the field was not present prior to 2.8, but the event 
itself is older, we can argue that clients of older qemu have to be 
prepared for the field to not be present.  So I think I can live with 
this change as-is.



  #
  # @operation: I/O operation
  #
@@ -3707,7 +3708,8 @@
  #
  ##
  { 'event': 'BLOCK_IO_ERROR',
-  'data': { 'device': 'str', 'node-name': 'str', 'operation': 
'IoOperationType',
+  'data': { 'device': 'str', '*node-name': 'str',
+'operation': 'IoOperationType',
  'action': 'BlockErrorAction', '*nospace': 'bool',
  'reason': 'str' } }
  


Reviewed-by: Eric Blake 

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

Re: [Qemu-block] [PATCH] qemu-iotests: fix 203 migration completion race

[Max Reitz]

On 2018-03-05 16:59, Stefan Hajnoczi wrote:
> There is a race between the test's 'query-migrate' QMP command after the
> QMP 'STOP' event and completing the migration:
> 
> The test case invokes 'query-migrate' upon receiving 'STOP'.  At this
> point the migration thread may still be in the process of completing.
> Therefore 'query-migrate' can return 'status': 'active' for a brief
> window of time instead of 'status': 'completed'.  This results in
> qemu-iotests 203 hanging.
> 
> Solve the race by enabling the 'events' migration capability, which
> causes QEMU to emit migration-specific QMP events that do not suffer
> from this race condition.  Wait for the QMP 'MIGRATION' event with
> 'status': 'completed'.
> 
> Reported-by: Max Reitz 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  tests/qemu-iotests/203 | 15 +++
>  tests/qemu-iotests/203.out |  5 +
>  2 files changed, 16 insertions(+), 4 deletions(-)

So much for "the ppoll() dungeon"...

Thanks!

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

[Qemu-block] [PATCH] qemu-iotests: fix 203 migration completion race

[Stefan Hajnoczi]

There is a race between the test's 'query-migrate' QMP command after the
QMP 'STOP' event and completing the migration:

The test case invokes 'query-migrate' upon receiving 'STOP'.  At this
point the migration thread may still be in the process of completing.
Therefore 'query-migrate' can return 'status': 'active' for a brief
window of time instead of 'status': 'completed'.  This results in
qemu-iotests 203 hanging.

Solve the race by enabling the 'events' migration capability, which
causes QEMU to emit migration-specific QMP events that do not suffer
from this race condition.  Wait for the QMP 'MIGRATION' event with
'status': 'completed'.

Reported-by: Max Reitz 
Signed-off-by: Stefan Hajnoczi 
---
 tests/qemu-iotests/203 | 15 +++
 tests/qemu-iotests/203.out |  5 +
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/203 b/tests/qemu-iotests/203
index 2c811917d8..4874a1a0d8 100755
--- a/tests/qemu-iotests/203
+++ b/tests/qemu-iotests/203
@@ -49,11 +49,18 @@ with iotests.FilePath('disk0.img') as disk0_img_path, \
node_name='drive1-node', iothread='iothread0',
force=True))
 
+iotests.log('Enabling migration QMP events...')
+iotests.log(vm.qmp('migrate-set-capabilities', capabilities=[
+{
+'capability': 'events',
+'state': True
+}
+]))
+
 iotests.log('Starting migration...')
 iotests.log(vm.qmp('migrate', uri='exec:cat >/dev/null'))
 while True:
-vm.get_qmp_event(wait=60.0)
-result = vm.qmp('query-migrate')
-status = result.get('return', {}).get('status', None)
-if status == 'completed':
+event = vm.event_wait('MIGRATION')
+iotests.log(event, filters=[iotests.filter_qmp_event])
+if event['data']['status'] == 'completed':
 break
diff --git a/tests/qemu-iotests/203.out b/tests/qemu-iotests/203.out
index 3f1ff900e4..1a11f0975c 100644
--- a/tests/qemu-iotests/203.out
+++ b/tests/qemu-iotests/203.out
@@ -2,5 +2,10 @@ Launching VM...
 Setting IOThreads...
 {u'return': {}}
 {u'return': {}}
+Enabling migration QMP events...
+{u'return': {}}
 Starting migration...
 {u'return': {}}
+{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': 
{u'status': u'setup'}, u'event': u'MIGRATION'}
+{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': 
{u'status': u'active'}, u'event': u'MIGRATION'}
+{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': 
{u'status': u'completed'}, u'event': u'MIGRATION'}
-- 
2.14.3

Re: [Qemu-block] [PATCH] iotests: Mark all tests executable

[Max Reitz]

On 2018-03-02 14:49, Eric Blake wrote:
> The majority of our iotests have the executable bit set; fix the
> few outliers for consistency.
> 
> Signed-off-by: Eric Blake 
> ---

I think 124 is missing.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH v2] iotests: Test creating overlay when guest running

[Max Reitz]

On 2017-12-25 03:51, Fam Zheng wrote:
> Signed-off-by: Fam Zheng 
> 
> ---
> 
> v2: Actually test the thing. [Kevin]
> ---
>  tests/qemu-iotests/153 | 8 +---
>  tests/qemu-iotests/153.out | 7 ---
>  2 files changed, 9 insertions(+), 6 deletions(-)

Thanks, applied to my block tree:

https://github.com/XanClic/qemu/commits/block

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] Intermittent failure of iotest 203

[Stefan Hajnoczi]

On Fri, Feb 23, 2018 at 02:03:15PM +0100, Max Reitz wrote:
> Hi,
> 
> iotest 203 relatively often fails for me, at least when run in parallel.
>  When I run the following concurrently on four shells:
> 
> $ while TEST_DIR=/tmp/t1 ./check -T -qcow2 203; do; done
> $ while TEST_DIR=/tmp/t2 ./check -T -qcow2 203; do; done
> $ while TEST_DIR=/tmp/t3 ./check -T -qcow2 203; do; done
> $ while TEST_DIR=/tmp/t4 ./check -T -qcow2 203; do; done
> 
> Very quickly (like under ten iterations), at least one of those starts
> to hang and then fails because of a timeout in vm.get_qmp_event().
> 
> Before digging deeper into the ppoll() dungeon* myself, I decided to
> report this so I wouldn't have to. :-)

Thanks for reporting it.  I will send a patch and CC you.

Stefan

> *Backtrace:
> 
> (gdb) bt
> #0  0x7f354137b4d6 in ppoll () at /lib64/libc.so.6
> #1  0x55b659144299 in ppoll (__ss=0x0, __timeout=0x7ffe4eaca230,
> __nfds=, __fds=) at
> /usr/include/bits/poll2.h:77
> #2  0x55b659144299 in qemu_poll_ns (fds=,
> nfds=, timeout=timeout@entry=39512999619000) at
> util/qemu-timer.c:334
> #3  0x55b6591450a3 in os_host_main_loop_wait (timeout= out>) at util/main-loop.c:255
> #4  0x55b6591450a3 in main_loop_wait (nonblocking=)
> at util/main-loop.c:515
> #5  0x55b658d4a253 in main_loop () at vl.c:1933
> #6  0x55b658d4a253 in main (argc=, argv= out>, envp=) at vl.c:4757
> 
> Max
> 





signature.asc
Description: PGP signature

Re: [Qemu-block] [RFC PATCH 1/2] migrate: Allow incoming migration without defer

[Richard Palethorpe]

hello David,

Dr. David Alan Gilbert writes:

> * Richard Palethorpe (rpaletho...@suse.com) wrote:
>> Allow a QEMU instance which has been started and used without the "-incoming"
>> flag to accept an incoming migration with the "migrate-incoming" QMP
>> command. This allows the user to dump the VM state to an external file then
>> revert to that state at a later time without restarting QEMU.
>> ---
>>  migration/migration.c | 12 +---
>>  vl.c  |  2 ++
>>  2 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 0aa596f867..05595a4cec 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -1321,14 +1321,14 @@ void migrate_del_blocker(Error *reason)
>>  void qmp_migrate_incoming(const char *uri, Error **errp)
>>  {
>>  Error *local_err = NULL;
>> -static bool once = true;
>>
>> -if (!deferred_incoming) {
>> -error_setg(errp, "For use with '-incoming defer'");
>> +if (runstate_check(RUN_STATE_INMIGRATE)) {
>> +error_setg(errp, "The incoming migration has already been started");
>>  return;
>>  }
>> -if (!once) {
>> -error_setg(errp, "The incoming migration has already been started");
>> +
>> +if (!deferred_incoming) {
>> +vm_stop(RUN_STATE_INMIGRATE);
>>  }
>>
>>  qemu_start_incoming_migration(uri, _err);
>> @@ -1337,8 +1337,6 @@ void qmp_migrate_incoming(const char *uri, Error 
>> **errp)
>>  error_propagate(errp, local_err);
>>  return;
>>  }
>> -
>> -once = false;
>>  }
>
> That's...interesting.  I think it will work, but I bet there's a whole
> bunch of corner cases [many of which loadvm will hit as well!].

It wouldn't surprise me if we have hit some of those corner cases with
our extensive use of loadvm :-) Unfortunately I don't have much data to
show what went wrong though.

> For starters there's probably devices that are active and still looking
> at RAM, even though vm_stop is in place (hopefully none of them are
> writing to it, but I wouldn't actually trust them).

I suppose that might be a showstopper.

> Also, you probably
> need to inactivate the block devices?

They are at least flushed by vm_stop. I would expect the user to
recreate them, or at least recreate the active layer before starting the
migration.

>
> Also, I think this means there's no protection against this happening
> during an outgoing migration (which has an existing check to make sure
> you can't start an outgoing migration while an incoming one is waiting).
>
>>  bool migration_is_blocked(Error **errp)
>> diff --git a/vl.c b/vl.c
>> index 9e7235df6d..a91eda039e 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -634,6 +634,7 @@ static const RunStateTransition 
>> runstate_transitions_def[] = {
>>  { RUN_STATE_PAUSED, RUN_STATE_POSTMIGRATE },
>>  { RUN_STATE_PAUSED, RUN_STATE_PRELAUNCH },
>>  { RUN_STATE_PAUSED, RUN_STATE_COLO},
>> +{ RUN_STATE_PAUSED, RUN_STATE_INMIGRATE },
>>
>>  { RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING },
>>  { RUN_STATE_POSTMIGRATE, RUN_STATE_FINISH_MIGRATE },
>> @@ -665,6 +666,7 @@ static const RunStateTransition 
>> runstate_transitions_def[] = {
>>  { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
>>  { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
>>  { RUN_STATE_RUNNING, RUN_STATE_COLO},
>> +{ RUN_STATE_RUNNING, RUN_STATE_INMIGRATE },
>
> Experience suggests that you'll find out the hard way that there's other
> states you'll need to allow or check for.
> For example, do you want to be able to loadvm from a GUEST_PANICKED or
> PRELAUNCH state?

Yes, I would add GUEST_PANICKED at least.

>
> Dave
>
>>  { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
>>
>> --
>> 2.16.2
>>


--
Thank you,
Richard.

[Qemu-block] [PATCH] block: Fix NULL dereference on empty drive error

[Kevin Wolf]

blk_error_action() sends a BLOCK_IO_ERROR QMP event which includes the
node name of its root node. If the BlockBackend represents an empty
drive, there is no root node, so we should not try to access its node
name. Make the field optional in the event and include it only when
the BlockBackend isn't empty.

Signed-off-by: Kevin Wolf 
---

Stefan, this is needed for your patch that reverts the workaround in the
IDE flush code. Without it, make check seems to succeed, but if you look
closer, qemu actually segfaults.

qapi/block-core.json  | 6 --
 block/block-backend.c | 5 +++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5c5921bfb7..00475f08d4 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3676,7 +3676,8 @@
 #
 # @node-name: node name. Note that errors may be reported for the root node
 # that is directly attached to a guest device rather than for the
-# node where the error occurred. (Since: 2.8)
+# node where the error occurred. The node name is not present if
+# the drive is empty. (Since: 2.8)
 #
 # @operation: I/O operation
 #
@@ -3707,7 +3708,8 @@
 #
 ##
 { 'event': 'BLOCK_IO_ERROR',
-  'data': { 'device': 'str', 'node-name': 'str', 'operation': 
'IoOperationType',
+  'data': { 'device': 'str', '*node-name': 'str',
+'operation': 'IoOperationType',
 'action': 'BlockErrorAction', '*nospace': 'bool',
 'reason': 'str' } }
 
diff --git a/block/block-backend.c b/block/block-backend.c
index a775a3dd2f..a4421252f8 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1615,10 +1615,11 @@ static void send_qmp_error_event(BlockBackend *blk,
  bool is_read, int error)
 {
 IoOperationType optype;
+BlockDriverState *bs = blk_bs(blk);
 
 optype = is_read ? IO_OPERATION_TYPE_READ : IO_OPERATION_TYPE_WRITE;
-qapi_event_send_block_io_error(blk_name(blk),
-   bdrv_get_node_name(blk_bs(blk)), optype,
+qapi_event_send_block_io_error(blk_name(blk), !!bs,
+   bs ? bdrv_get_node_name(bs) : NULL, optype,
action, blk_iostatus_is_enabled(blk),
error == ENOSPC, strerror(error),
_abort);
-- 
2.13.6

Re: [Qemu-block] block migration and MAX_IN_FLIGHT_IO

[Dr. David Alan Gilbert]

* Peter Lieven (p...@kamp.de) wrote:
> Am 05.03.2018 um 12:45 schrieb Stefan Hajnoczi:
> > On Thu, Feb 22, 2018 at 12:13:50PM +0100, Peter Lieven wrote:
> >> I stumbled across the MAX_INFLIGHT_IO field that was introduced in 2015 
> >> and was curious what was the reason
> >> to choose 512MB as readahead? The question is that I found that the source 
> >> VM gets very unresponsive I/O wise
> >> while the initial 512MB are read and furthermore seems to stay 
> >> unreasponsive if we choose a high migration speed
> >> and have a fast storage on the destination VM.
> >>
> >> In our environment I modified this value to 16MB which seems to work much 
> >> smoother. I wonder if we should make
> >> this a user configurable value or define a different rate limit for the 
> >> block transfer in bulk stage at least?
> > I don't know if benchmarks were run when choosing the value.  From the
> > commit description it sounds like the main purpose was to limit the
> > amount of memory that can be consumed.
> >
> > 16 MB also fulfills that criteria :), but why is the source VM more
> > responsive with a lower value?
> >
> > Perhaps the issue is queue depth on the storage device - the block
> > migration code enqueues up to 512 MB worth of reads, and guest I/O has
> > to wait?
> 
> That is my guess. Especially if the destination storage is faster we 
> basically alsways have
> 512 I/Os in flight on the source storage.
> 
> Does anyone mind if the reduce that value to 16MB or do we need a better 
> mechanism?

We've got migration-parameters these days; you could connect it to one
of those fairly easily I think.
Try: grep -i 'cpu[-_]throttle[-_]initial'  for an example of one that's
already there.
Then you can set it to whatever you like.

Dave

> Peter
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-block] block migration and MAX_IN_FLIGHT_IO

[Peter Lieven]

Am 05.03.2018 um 12:45 schrieb Stefan Hajnoczi:
> On Thu, Feb 22, 2018 at 12:13:50PM +0100, Peter Lieven wrote:
>> I stumbled across the MAX_INFLIGHT_IO field that was introduced in 2015 and 
>> was curious what was the reason
>> to choose 512MB as readahead? The question is that I found that the source 
>> VM gets very unresponsive I/O wise
>> while the initial 512MB are read and furthermore seems to stay unreasponsive 
>> if we choose a high migration speed
>> and have a fast storage on the destination VM.
>>
>> In our environment I modified this value to 16MB which seems to work much 
>> smoother. I wonder if we should make
>> this a user configurable value or define a different rate limit for the 
>> block transfer in bulk stage at least?
> I don't know if benchmarks were run when choosing the value.  From the
> commit description it sounds like the main purpose was to limit the
> amount of memory that can be consumed.
>
> 16 MB also fulfills that criteria :), but why is the source VM more
> responsive with a lower value?
>
> Perhaps the issue is queue depth on the storage device - the block
> migration code enqueues up to 512 MB worth of reads, and guest I/O has
> to wait?

That is my guess. Especially if the destination storage is faster we basically 
alsways have
512 I/Os in flight on the source storage.

Does anyone mind if the reduce that value to 16MB or do we need a better 
mechanism?

Peter

Re: [Qemu-block] [RFC PATCH 0/2] Increase usability of external snapshots

[Richard Palethorpe]

Hello Roman,

Roman Kagan writes:

> On Tue, Feb 27, 2018 at 12:56:49PM +0100, Richard Palethorpe wrote:
>> Following on from the discussion about creating savevm/loadvm QMP
>> equivalents. I decided to take the advice given that we should use external
>> snapshots. However reverting to a snapshot currently requires QEMU to be
>> restarted with "-incoming".  Restarting QEMU requires a fair bit of
>> book keeping to be done by the management application and may take
>> measurably more time.
>
> AFAICT "-incoming" is not the only reason for starting QEMU anew.  The
> block devices will need to be pointed at different nodes in the backing
> chains.  Moreover the snapshot you're reverting to may have been taken
> at a point where the VM had different configuration than it has now.

OK, so contrary to what I thought, it looks like there is no QMP command
to simply drop the current active layer and repoint the block device to
the previous node in the chain. We would have to recreate the block
device with the desired backing store and overlays or create a new
command. I am not sure that is a showstopper though.

>
> So the management application will need to do a lot of bookkeeping stuff
> anyway, and it'll probably have harder time applying all of the
> configuration changes to a live QEMU instance.

Well in our use case, configuration changes between snapshots would not
be possible. For other use cases the management app will have to restart
QEMU when necessary. AFAICT, with or without this patch it is possible for
someone to try accepting an incoming migration with an invalid
configuration. So I would hope that this is not introducing a new
problem?

>
> Is the cost of killing the old QEMU process and starting a new one big
> enough to be worth all the hassle?

I doubt I can justify it based on performance, atleast not for our use
case, but terminating the QEMU process will interrupt a number of socket
connections and such. I am sure this is a technically easier problem to
solve than modifying QEMU, but there is one QEMU and many management
apps so *maybe* the effort is better spent on QEMU if it avoids more work
in the management apps and is not introducing features to QEMU outside
of its scope. QEMU already supports reverting to internal snapshots in a
live instance, so I don't think I am increasing its scope.

>
> Thanks,
> Roman.

--
Thank you,
Richard.

Re: [Qemu-block] [PATCH v2 1/2] block/accounting: introduce latency histogram

[Vladimir Sementsov-Ogievskiy]

07.02.2018 15:50, Vladimir Sementsov-Ogievskiy wrote:

Introduce latency histogram statics for block devices.
For each accounted operation type latency region [0, +inf) is
divided into subregions by several points. Then, calculate
hits for each subregion.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  include/block/accounting.h |  9 +
  block/accounting.c | 97 ++
  2 files changed, 106 insertions(+)

diff --git a/include/block/accounting.h b/include/block/accounting.h
index b833d26d6c..9679020f64 100644
--- a/include/block/accounting.h
+++ b/include/block/accounting.h
@@ -45,6 +45,12 @@ struct BlockAcctTimedStats {
  QSLIST_ENTRY(BlockAcctTimedStats) entries;
  };
  
+typedef struct BlockLatencyHistogram {

+int size;
+uint64_t *points; /* @size-1 points here (all points, except 0 and +inf) */
+uint64_t *histogram[BLOCK_MAX_IOTYPE]; /* @size elements for each type */
+} BlockLatencyHistogram;
+
  struct BlockAcctStats {
  QemuMutex lock;
  uint64_t nr_bytes[BLOCK_MAX_IOTYPE];
@@ -57,6 +63,7 @@ struct BlockAcctStats {
  QSLIST_HEAD(, BlockAcctTimedStats) intervals;
  bool account_invalid;
  bool account_failed;
+BlockLatencyHistogram latency_histogram;
  };
  
  typedef struct BlockAcctCookie {

@@ -82,5 +89,7 @@ void block_acct_merge_done(BlockAcctStats *stats, enum 
BlockAcctType type,
  int64_t block_acct_idle_time_ns(BlockAcctStats *stats);
  double block_acct_queue_depth(BlockAcctTimedStats *stats,
enum BlockAcctType type);
+int block_latency_histogram_set(BlockAcctStats *stats, uint64List *latency);
+void block_latency_histogram_clear(BlockAcctStats *stats);
  
  #endif

diff --git a/block/accounting.c b/block/accounting.c
index 87ef5bbfaa..0051ff0c24 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -94,6 +94,100 @@ void block_acct_start(BlockAcctStats *stats, 
BlockAcctCookie *cookie,
  cookie->type = type;
  }
  
+/* block_latency_histogram_compare_func

+ * Compare @key with interval [@el, @el+1), where @el+1 is a next array element
+ * after @el.
+ * Return: -1 if @key < @el
+ *  0 if @key in [@el, @el+1)
+ * +1 if @key >= @el+1
+ */
+static int block_latency_histogram_compare_func(const void *key, const void 
*el)
+{
+uint64_t k = *(uint64_t *)key;
+uint64_t a = *(uint64_t *)el;
+uint64_t b = *((uint64_t *)el + 1);
+
+return k < a ? -1 : (k < b ? 0 : 1);
+}
+
+static void block_latency_histogram_account(BlockLatencyHistogram *hist,
+enum BlockAcctType type,
+int64_t latency_ns)
+{
+uint64_t *data, *pos;
+
+if (hist->points == NULL) {
+/* histogram disabled */
+return;
+}
+
+data = hist->histogram[type];
+
+if (latency_ns < hist->points[0]) {
+data[0]++;
+return;
+}
+
+if (latency_ns >= hist->points[hist->size - 2]) {
+data[hist->size - 1]++;
+return;
+}
+
+pos = bsearch(_ns, hist->points, hist->size - 2,
+  sizeof(hist->points[0]),
+  block_latency_histogram_compare_func);
+assert(pos != NULL);
+
+data[pos - hist->points + 1]++;
+}
+
+int block_latency_histogram_set(BlockAcctStats *stats, uint64List *latency)
+{
+BlockLatencyHistogram *hist = >latency_histogram;
+uint64List *entry;
+uint64_t *ptr;
+int i;
+uint64_t prev = 0;
+
+hist->size = 1;


bug here. we can fail, so separate variable new_size is needed to 
calculate new size.



+
+for (entry = latency; entry; entry = entry->next) {
+if (entry->value <= prev) {
+return -EINVAL;
+}
+hist->size++;
+prev = entry->value;
+}
+
+hist->points = g_renew(uint64_t, hist->points, hist->size - 1);
+for (entry = latency, ptr = hist->points; entry;
+ entry = entry->next, ptr++)
+{
+*ptr = entry->value;
+}
+
+for (i = 0; i < BLOCK_MAX_IOTYPE; i++) {
+hist->histogram[i] = g_renew(uint64_t, hist->histogram[i], hist->size);
+memset(hist->histogram[i], 0, hist->size * sizeof(uint64_t));
+}
+
+return 0;
+}
+
+void block_latency_histogram_clear(BlockAcctStats *stats)
+{
+BlockLatencyHistogram *hist = >latency_histogram;
+int i;
+
+g_free(hist->points);
+hist->points = NULL;
+
+for (i = 0; i < BLOCK_MAX_IOTYPE; i++) {
+g_free(hist->histogram[i]);
+hist->histogram[i] = NULL;
+}
+}
+
  static void block_account_one_io(BlockAcctStats *stats, BlockAcctCookie 
*cookie,
   bool failed)
  {
@@ -116,6 +210,9 @@ static void block_account_one_io(BlockAcctStats *stats, 
BlockAcctCookie *cookie,
  stats->nr_ops[cookie->type]++;
  }
  
+block_latency_histogram_account(>latency_histogram, cookie->type,

+

Re: [Qemu-block] [Qemu-devel] [PULL 00/37] Block layer patches

[Peter Maydell]

On 2 March 2018 at 18:54, Kevin Wolf  wrote:
> The following changes since commit 86f4c7e05b1c44dbe1b329a51f311f10aef6ff34:
>
>   Merge remote-tracking branch 
> 'remotes/pmaydell/tags/pull-target-arm-20180302' into staging (2018-03-02 
> 14:37:10 +)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 9d9b4b640f9e583ff4b24dc762630945f3ccc16d:
>
>   Merge remote-tracking branch 'mreitz/tags/pull-block-2018-03-02' into 
> queue-block (2018-03-02 18:45:03 +0100)
>
> 
> Block layer patches
>

This gives me a clang sanitizer error while running 'make check':

e104462:xenial:clang$ UBSAN_OPTIONS=print_stacktrace=1
QTEST_QEMU_BINARY=i386-softmmu/qemu-system-i386
QTEST_QEMU_IMG=qemu-img tests/ide-test
/i386/ide/identify: OK
/i386/ide/flush: OK
/i386/ide/bmdma/setup: OK
/i386/ide/bmdma/simple_rw: OK
/i386/ide/bmdma/trim: OK
/i386/ide/bmdma/short_prdt: OK
/i386/ide/bmdma/one_sector_short_prdt: OK
/i386/ide/bmdma/long_prdt: OK
/i386/ide/bmdma/no_busmaster: OK
/i386/ide/bmdma/teardown: OK
/i386/ide/flush/nodev: OK
/i386/ide/flush/empty_drive:
/home/petmay01/linaro/qemu-for-merges/block.c:3926:16: runtime error:
member access within null pointer of type 'const BlockDriverState'
(aka 'const struct BlockDriverState')
#0 0x5560b5776f48 in bdrv_get_node_name
/home/petmay01/linaro/qemu-for-merges/block.c:3926:16
#1 0x5560b587fc92 in send_qmp_error_event
/home/petmay01/linaro/qemu-for-merges/block/block-backend.c:1621:36
#2 0x5560b587fc92 in blk_error_action
/home/petmay01/linaro/qemu-for-merges/block/block-backend.c:1655
#3 0x5560b52619e6 in ide_handle_rw_error
/home/petmay01/linaro/qemu-for-merges/hw/ide/core.c:836:5
#4 0x5560b5278d7b in ide_flush_cb
/home/petmay01/linaro/qemu-for-merges/hw/ide/core.c:1067:13
#5 0x5560b58822ac in blk_aio_complete
/home/petmay01/linaro/qemu-for-merges/block/block-backend.c:1286:9
#6 0x5560b5a04417 in aio_bh_poll
/home/petmay01/linaro/qemu-for-merges/util/async.c:118:13
#7 0x5560b5a10cef in aio_dispatch
/home/petmay01/linaro/qemu-for-merges/util/aio-posix.c:436:5
#8 0x5560b5a062fa in aio_ctx_dispatch
/home/petmay01/linaro/qemu-for-merges/util/async.c:261:5
#9 0x7f9786667196 in g_main_context_dispatch
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4a196)
#10 0x5560b5a0dd08 in glib_pollfds_poll
/home/petmay01/linaro/qemu-for-merges/util/main-loop.c:214:9
#11 0x5560b5a0dd08 in os_host_main_loop_wait
/home/petmay01/linaro/qemu-for-merges/util/main-loop.c:261
#12 0x5560b5a0dd08 in main_loop_wait
/home/petmay01/linaro/qemu-for-merges/util/main-loop.c:515
#13 0x5560b4fdebf2 in main_loop
/home/petmay01/linaro/qemu-for-merges/vl.c:1942:9
#14 0x5560b4fdebf2 in main /home/petmay01/linaro/qemu-for-merges/vl.c:4772
#15 0x7f97843ec82f in __libc_start_main
/build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
#16 0x5560b4b456c8 in _start
(/home/petmay01/linaro/qemu-for-merges/build/clang/i386-softmmu/qemu-system-i386+0xe5c6c8)

OK
/i386/ide/flush/retry_pci: OK
/i386/ide/flush/retry_isa: OK
/i386/ide/cdrom/pio: OK
/i386/ide/cdrom/pio_large: OK
/i386/ide/cdrom/dma: OK


thanks
-- PMM

[Qemu-block] [PATCH v2 03/30] hw/block/nvme: include the "qemu/cutils.h" in the source file

[Philippe Mathieu-Daudé]

where it is used.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Thomas Huth 
---
 hw/block/nvme.h | 1 -
 hw/block/nvme.c | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 8f3981121d..cabcf20c32 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -1,6 +1,5 @@
 #ifndef HW_NVME_H
 #define HW_NVME_H
-#include "qemu/cutils.h"
 #include "block/nvme.h"
 
 typedef struct NvmeAsyncEvent {
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 85d2406400..811084b6a7 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -35,6 +35,7 @@
 #include "sysemu/block-backend.h"
 
 #include "qemu/log.h"
+#include "qemu/cutils.h"
 #include "trace.h"
 #include "nvme.h"
 
-- 
2.16.2

Re: [Qemu-block] [PULL 6/6] README: Document 'git-publish' workflow

[Fam Zheng]

On Mon, 03/05 11:51, Alberto Garcia wrote:
> On Mon 05 Mar 2018 10:40:06 AM CET, Stefan Hajnoczi wrote:
> > +A 'git-profile' utility was created to make above process less
> > +cumbersome, and is highly recommended for making regular
> > contributions,
> 
> A 'git-profile' utility ? Did you want to say 'git-publish' there?

Oops. Apparently, yes.

Fam

[Qemu-block] [PATCH v2 30/30] xen: use the BYTE-based definitions

[Philippe Mathieu-Daudé]

It eases code review, unit is explicit.

Patch generated using:

  $ git grep -E '(1024|2048|4096|8192|(<<|>>).?(10|20|30))' hw/ include/hw/

and modified manually.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alan Robinson 
---
 hw/block/xen_disk.c|  4 ++--
 hw/xenpv/xen_domainbuild.c | 10 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index f74fcd42d1..557005b5e5 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -1153,9 +1153,9 @@ static int blk_connect(struct XenDevice *xendev)
 }
 
 xen_pv_printf(xendev, 1, "type \"%s\", fileproto \"%s\", filename \"%s\","
-  " size %" PRId64 " (%" PRId64 " MB)\n",
+  " size %" PRId64 " (%llu MB)\n",
   blkdev->type, blkdev->fileproto, blkdev->filename,
-  blkdev->file_size, blkdev->file_size >> 20);
+  blkdev->file_size, blkdev->file_size / M_BYTE);
 
 /* Fill in number of sector size and number of sectors */
 xenstore_write_be_int(>xendev, "sector-size", blkdev->file_blk);
diff --git a/hw/xenpv/xen_domainbuild.c b/hw/xenpv/xen_domainbuild.c
index 027f76fad1..7c8bde20cd 100644
--- a/hw/xenpv/xen_domainbuild.c
+++ b/hw/xenpv/xen_domainbuild.c
@@ -75,9 +75,9 @@ int xenstore_domain_init1(const char *kernel, const char 
*ramdisk,
 xenstore_write_str(dom, "vm", vm);
 
 /* memory */
-xenstore_write_int(dom, "memory/target", ram_size >> 10);  // kB
-xenstore_write_int(vm, "memory", ram_size >> 20);  // MB
-xenstore_write_int(vm, "maxmem", ram_size >> 20);  // MB
+xenstore_write_int(dom, "memory/target", ram_size / K_BYTE);
+xenstore_write_int(vm, "memory", ram_size / M_BYTE);
+xenstore_write_int(vm, "maxmem", ram_size / M_BYTE);
 
 /* cpus */
 for (i = 0; i < smp_cpus; i++) {
@@ -260,7 +260,7 @@ int xen_domain_build_pv(const char *kernel, const char 
*ramdisk,
 }
 #endif
 
-rc = xc_domain_setmaxmem(xen_xc, xen_domid, ram_size >> 10);
+rc = xc_domain_setmaxmem(xen_xc, xen_domid, ram_size / K_BYTE);
 if (rc < 0) {
 fprintf(stderr, "xen: xc_domain_setmaxmem() failed\n");
 goto err;
@@ -269,7 +269,7 @@ int xen_domain_build_pv(const char *kernel, const char 
*ramdisk,
 xenstore_port = xc_evtchn_alloc_unbound(xen_xc, xen_domid, 0);
 console_port = xc_evtchn_alloc_unbound(xen_xc, xen_domid, 0);
 
-rc = xc_linux_build(xen_xc, xen_domid, ram_size >> 20,
+rc = xc_linux_build(xen_xc, xen_domid, ram_size / M_BYTE,
 kernel, ramdisk, cmdline,
 0, flags,
 xenstore_port, _mfn,
-- 
2.16.2

Re: [Qemu-block] block migration and MAX_IN_FLIGHT_IO

[Stefan Hajnoczi]

On Thu, Feb 22, 2018 at 12:13:50PM +0100, Peter Lieven wrote:
> I stumbled across the MAX_INFLIGHT_IO field that was introduced in 2015 and 
> was curious what was the reason
> to choose 512MB as readahead? The question is that I found that the source VM 
> gets very unresponsive I/O wise
> while the initial 512MB are read and furthermore seems to stay unreasponsive 
> if we choose a high migration speed
> and have a fast storage on the destination VM.
> 
> In our environment I modified this value to 16MB which seems to work much 
> smoother. I wonder if we should make
> this a user configurable value or define a different rate limit for the block 
> transfer in bulk stage at least?

I don't know if benchmarks were run when choosing the value.  From the
commit description it sounds like the main purpose was to limit the
amount of memory that can be consumed.

16 MB also fulfills that criteria :), but why is the source VM more
responsive with a lower value?

Perhaps the issue is queue depth on the storage device - the block
migration code enqueues up to 512 MB worth of reads, and guest I/O has
to wait?

Stefan


signature.asc
Description: PGP signature

[Qemu-block] [PATCH v2 16/30] hw/sh4: use the BYTE-based definitions

[Philippe Mathieu-Daudé]

It eases code review, unit is explicit.

Patch generated using:

  $ git grep -E '(1024|2048|4096|8192|(<<|>>).?(10|20|30))' hw/ include/hw/

and modified manually.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/block/tc58128.c | 2 +-
 hw/sh4/r2d.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/block/tc58128.c b/hw/block/tc58128.c
index 1d9f7ee000..3e658d509f 100644
--- a/hw/block/tc58128.c
+++ b/hw/block/tc58128.c
@@ -26,7 +26,7 @@ typedef struct {
 
 static tc58128_dev tc58128_devs[2];
 
-#define FLASH_SIZE (16*1024*1024)
+#define FLASH_SIZE (16 * M_BYTE)
 
 static void init_dev(tc58128_dev * dev, const char *filename)
 {
diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
index 458ed83297..720bd6ad04 100644
--- a/hw/sh4/r2d.c
+++ b/hw/sh4/r2d.c
@@ -292,7 +292,7 @@ static void r2d_init(MachineState *machine)
 dinfo = drive_get(IF_PFLASH, 0, 0);
 pflash_cfi02_register(0x0, NULL, "r2d.flash", FLASH_SIZE,
   dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-  (16 * 1024), FLASH_SIZE >> 16,
+  16 * K_BYTE, FLASH_SIZE >> 16,
   1, 4, 0x, 0x, 0x, 0x,
   0x555, 0x2aa, 0);
 
-- 
2.16.2

Re: [Qemu-block] [Qemu-devel] [PULL 0/6] Block patches

[no-reply]

Hi,

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

Type: series
Message-id: 20180305094006.21446-1-stefa...@redhat.com
Subject: [Qemu-devel] [PULL 0/6] Block patches

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

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

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

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   136c67e078..4f51e1d386  master -> master
 * [new tag]   
patchew/1520243276-30197-1-git-send-email-th...@redhat.com -> 
patchew/1520243276-30197-1-git-send-email-th...@redhat.com
 * [new tag]   patchew/20180305094006.21446-1-stefa...@redhat.com 
-> patchew/20180305094006.21446-1-stefa...@redhat.com
Switched to a new branch 'test'
20eeab9795 README: Document 'git-publish' workflow
333283c56b Add a git-publish configuration file
3a9752c776 tests/libqos: Check for valid dev pointer when looking for PCI 
devices
6863406cb9 util/uri.c: wrap single statement blocks with braces {}
906ebc14e5 util/uri.c: remove brackets that wrap `return` statement's content.
d0e12d0916 util/uri.c: Coding style check, Only whitespace involved

=== OUTPUT BEGIN ===
Checking PATCH 1/6: util/uri.c: Coding style check, Only whitespace involved...
ERROR: return is not a function, parentheses are not required
#171: FILE: util/uri.c:215:
+return (-1);

ERROR: return is not a function, parentheses are not required
#176: FILE: util/uri.c:219:
+return (2);

ERROR: return is not a function, parentheses are not required
#190: FILE: util/uri.c:229:
+return (0);

ERROR: braces {} are necessary for all arms of this statement
#212: FILE: util/uri.c:262:
+if (uri->cleanup & 2)
[...]
+else
[...]

ERROR: braces {} are necessary for all arms of this statement
#276: FILE: util/uri.c:359:
+if (uri->cleanup & 2)
[...]
+else
[...]

ERROR: return is not a function, parentheses are not required
#282: FILE: util/uri.c:365:
+return (0);

ERROR: return is not a function, parentheses are not required
#285: FILE: util/uri.c:367:
+return (1);

ERROR: return is not a function, parentheses are not required
#305: FILE: util/uri.c:389:
+return (1);

ERROR: braces {} are necessary for all arms of this statement
#306: FILE: util/uri.c:390:
+if (!ISA_DIGIT(cur + 1))
[...]
+else if ((*cur != '0') && (ISA_DIGIT(cur + 1)) && (!ISA_DIGIT(cur + 2)))
[...]
 else if ((*cur == '1') && (ISA_DIGIT(cur + 1)) && (ISA_DIGIT(cur + 2)))
[...]
-else if ((*cur == '2') && (*(cur + 1) >= '0') &&
[...]
-else if ((*cur == '2') && (*(cur + 1) == '5') &&
[...]
+cur += 3;
[...]

ERROR: braces {} are necessary for all arms of this statement
#308: FILE: util/uri.c:392:
+else if ((*cur != '0') && (ISA_DIGIT(cur + 1)) && (!ISA_DIGIT(cur + 2)))
[...]
 else if ((*cur == '1') && (ISA_DIGIT(cur + 1)) && (ISA_DIGIT(cur + 2)))
[...]
-else if ((*cur == '2') && (*(cur + 1) >= '0') &&
[...]
-else if ((*cur == '2') && (*(cur + 1) == '5') &&
[...]
+cur += 3;
[...]

ERROR: braces {} are necessary for all arms of this statement
#319: FILE: util/uri.c:396:
+else if ((*cur == '2') && (*(cur + 1) >= '0') && (*(cur + 1) <= '4') &&
[...]
+else if ((*cur == '2') && (*(cur + 1) == '5') && (*(cur + 2) >= '0') &&
[...]
 else
[...]

ERROR: braces {} are necessary for all arms of this statement
#322: FILE: util/uri.c:399:
+else if ((*cur == '2') && (*(cur + 1) == '5') && (*(cur + 2) >= '0') &&
[...]
 else
[...]

ERROR: return is not a function, parentheses are not required
#327: FILE: util/uri.c:403:
+return (1);

ERROR: return is not a function, parentheses are not required
#330: FILE: util/uri.c:405:
+return (0);

ERROR: braces {} are necessary even for single statement blocks
#354: FILE: util/uri.c:433:
+while ((*cur != ']') && (*cur != 0))
+cur++;

ERROR: braces {} are necessary for all arms of this statement
#356: FILE: util/uri.c:435:
+if (*cur != ']')
[...]

ERROR: return is not a function, parentheses are not required
#357: FILE: util/uri.c:436:
+return (1);

ERROR: braces {} are necessary for all arms of this statement
#371: FILE: util/uri.c:446:
+if (*cur != '.')
[...]

ERROR: braces {} are necessary for all arms of this statement
#379: FILE: util/uri.c:451:
+if (*cur != '.')
[...]

ERROR: braces {} are necessary for all arms of this statement
#386: FILE: util/uri.c:455:
+if (*cur != '.')
[...]

ERROR: braces {} 

[Qemu-block] [PULL 1/6] util/uri.c: Coding style check, Only whitespace involved

[Stefan Hajnoczi]

From: Su Hang 

Using `clang-format -i util/uri.c` first, then change back few code
manually, to make sure only whitespace involved.

Signed-off-by: Su Hang 
Reviewed-by: Thomas Huth 
Message-id: 1519533358-13759-2-git-send-email-suhan...@mails.ucas.ac.cn
Signed-off-by: Stefan Hajnoczi 
---
 util/uri.c | 1450 ++--
 1 file changed, 726 insertions(+), 724 deletions(-)

diff --git a/util/uri.c b/util/uri.c
index 21b1828170..cf09f41735 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -63,7 +63,6 @@ static void uri_clean(URI *uri);
  */
 #define IS_ALPHA(x) (IS_LOWALPHA(x) || IS_UPALPHA(x))
 
-
 /*
  * lowalpha = "a" | "b" | "c" | "d" | "e" | "f" | "g" | "h" | "i" | "j" |
  *"k" | "l" | "m" | "n" | "o" | "p" | "q" | "r" | "s" | "t" |
@@ -97,27 +96,27 @@ static void uri_clean(URI *uri);
  * mark = "-" | "_" | "." | "!" | "~" | "*" | "'" | "(" | ")"
  */
 
-#define IS_MARK(x) (((x) == '-') || ((x) == '_') || ((x) == '.') || \
-((x) == '!') || ((x) == '~') || ((x) == '*') || ((x) == '\'') ||\
+#define IS_MARK(x) (((x) == '-') || ((x) == '_') || ((x) == '.') ||
\
+((x) == '!') || ((x) == '~') || ((x) == '*') || ((x) == '\'') ||   
\
 ((x) == '(') || ((x) == ')'))
 
 /*
  * unwise = "{" | "}" | "|" | "\" | "^" | "`"
  */
 
-#define IS_UNWISE(p)\
-  (((*(p) == '{')) || ((*(p) == '}')) || ((*(p) == '|')) || \
-   ((*(p) == '\\')) || ((*(p) == '^')) || ((*(p) == '[')) ||\
-   ((*(p) == ']')) || ((*(p) == '`')))
+#define IS_UNWISE(p)   
\
+(((*(p) == '{')) || ((*(p) == '}')) || ((*(p) == '|')) ||  
\
+ ((*(p) == '\\')) || ((*(p) == '^')) || ((*(p) == '[')) || 
\
+ ((*(p) == ']')) || ((*(p) == '`')))
 /*
  * reserved = ";" | "/" | "?" | ":" | "@" | "&" | "=" | "+" | "$" | "," |
  *"[" | "]"
  */
 
-#define IS_RESERVED(x) (((x) == ';') || ((x) == '/') || ((x) == '?') || \
-((x) == ':') || ((x) == '@') || ((x) == '&') || ((x) == '=') || \
-((x) == '+') || ((x) == '$') || ((x) == ',') || ((x) == '[') || \
-((x) == ']'))
+#define IS_RESERVED(x) (((x) == ';') || ((x) == '/') || ((x) == '?') ||
\
+((x) == ':') || ((x) == '@') || ((x) == '&') || ((x) == '=') ||
\
+((x) == '+') || ((x) == '$') || ((x) == ',') || ((x) == '[') ||
\
+((x) == ']'))
 
 /*
  * unreserved = alphanum | mark
@@ -129,7 +128,7 @@ static void uri_clean(URI *uri);
  * Skip to next pointer char, handle escaped sequences
  */
 
-#define NEXT(p) ((*p == '%')? p += 3 : p++)
+#define NEXT(p) ((*p == '%') ? p += 3 : p++)
 
 /*
  * Productions from the spec.
@@ -141,37 +140,36 @@ static void uri_clean(URI *uri);
  * path  = [ abs_path | opaque_part ]
  */
 
-
 /
- * *
- * RFC 3986 parser *
- * *
+ *  *
+ * RFC 3986 parser  *
+ *  *
  /
 
 #define ISA_DIGIT(p) ((*(p) >= '0') && (*(p) <= '9'))
-#define ISA_ALPHA(p) (((*(p) >= 'a') && (*(p) <= 'z')) ||  \
+#define ISA_ALPHA(p) (((*(p) >= 'a') && (*(p) <= 'z')) ||  
\
   ((*(p) >= 'A') && (*(p) <= 'Z')))
-#define ISA_HEXDIG(p)  \
-   (ISA_DIGIT(p) || ((*(p) >= 'a') && (*(p) <= 'f')) ||\
-((*(p) >= 'A') && (*(p) <= 'F')))
+#define ISA_HEXDIG(p)  
\
+(ISA_DIGIT(p) || ((*(p) >= 'a') && (*(p) <= 'f')) ||   
\
+ ((*(p) >= 'A') && (*(p) <= 'F')))
 
 /*
  *sub-delims= "!" / "$" / "&" / "'" / "(" / ")"
  * / "*" / "+" / "," / ";" / "="
  */
-#define ISA_SUB_DELIM(p)   \
-  (((*(p) == '!')) || ((*(p) == '$')) || ((*(p) == '&')) ||
\
-   ((*(p) == '(')) || ((*(p) == ')')) || ((*(p) == '*')) ||
\
-   ((*(p) == '+')) || ((*(p) == ',')) || ((*(p) == ';')) ||
\
-   ((*(p) == '=')) || ((*(p) == '\'')))
+#define ISA_SUB_DELIM(p)   
\
+(((*(p) == '!')) || ((*(p) == '$')) || ((*(p) == '&')) ||  
\
+ ((*(p) == '(')) || ((*(p) == ')')) || ((*(p) == '*')) || 

[Qemu-block] [PULL 6/6] README: Document 'git-publish' workflow

[Stefan Hajnoczi]

From: Fam Zheng 

Signed-off-by: Fam Zheng 
Message-id: 20180226030326.20219-3-f...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 README | 31 ++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/README b/README
index 2c8e1c8cc4..7833b97365 100644
--- a/README
+++ b/README
@@ -56,7 +56,7 @@ The QEMU source code is maintained under the GIT version 
control system.
 
git clone git://git.qemu.org/qemu.git
 
-When submitting patches, the preferred approach is to use 'git
+When submitting patches, one common approach is to use 'git
 format-patch' and/or 'git send-email' to format & send the mail to the
 qemu-de...@nongnu.org mailing list. All patches submitted must contain
 a 'Signed-off-by' line from the author. Patches should follow the
@@ -73,6 +73,35 @@ The QEMU website is also maintained under source control.
   git clone git://git.qemu.org/qemu-web.git
   https://www.qemu.org/2017/02/04/the-new-qemu-website-is-up/
 
+A 'git-profile' utility was created to make above process less
+cumbersome, and is highly recommended for making regular contributions,
+or even just for sending consecutive patch series revisions. It also
+requires a working 'git send-email' setup, and by default doesn't
+automate everything, so you may want to go through the above steps
+manually for once.
+
+For installation instructions, please go to
+
+  https://github.com/stefanha/git-publish
+
+The workflow with 'git-publish' is:
+
+  $ git checkout master -b my-feature
+  $ # work on new commits, add your 'Signed-off-by' lines to each
+  $ git publish
+
+Your patch series will be sent and tagged as my-feature-v1 if you need to refer
+back to it in the future.
+
+Sending v2:
+
+  $ git checkout my-feature # same topic branch
+  $ # making changes to the commits (using 'git rebase', for example)
+  $ git publish
+
+Your patch series will be sent with 'v2' tag in the subject and the git tip
+will be tagged as my-feature-v2.
+
 Bug reporting
 =
 
-- 
2.14.3

[Qemu-block] [PULL 5/6] Add a git-publish configuration file

[Stefan Hajnoczi]

From: Fam Zheng 

git-publish [1] is a convenient tool to send patches and has been
popular among QEMU developers.  Recently it has been made available in
Fedora/Debian official repo.

One nice feature of the tool is a per-project configuration with
profiles, especially in which the cccmd option is a handy method to
create the Cc list.

[1]: https://github.com/stefanha/git-publish

Signed-off-by: Fam Zheng 
Message-id: 20180226030326.20219-2-f...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 .gitpublish | 51 +++
 1 file changed, 51 insertions(+)
 create mode 100644 .gitpublish

diff --git a/.gitpublish b/.gitpublish
new file mode 100644
index 00..a13f8c7c0e
--- /dev/null
+++ b/.gitpublish
@@ -0,0 +1,51 @@
+#
+# Common git-publish profiles that can be used to send patches to QEMU 
upstream.
+#
+# See https://github.com/stefanha/git-publish for more information
+#
+[gitpublishprofile "default"]
+base = master
+to = qemu-de...@nongnu.org
+cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit 
--nogit-fallback 2>/dev/null
+
+[gitpublishprofile "rfc"]
+base = master
+prefix = RFC PATCH
+to = qemu-de...@nongnu.org
+cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit 
--nogit-fallback 2>/dev/null
+
+[gitpublishprofile "stable"]
+base = master
+to = qemu-de...@nongnu.org
+cc = qemu-sta...@nongnu.org
+cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit 
--nogit-fallback 2>/dev/null
+
+[gitpublishprofile "trivial"]
+base = master
+to = qemu-de...@nongnu.org
+cc = qemu-triv...@nongnu.org
+cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit 
--nogit-fallback 2>/dev/null
+
+[gitpublishprofile "block"]
+base = master
+to = qemu-de...@nongnu.org
+cc = qemu-block@nongnu.org
+cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit 
--nogit-fallback 2>/dev/null
+
+[gitpublishprofile "arm"]
+base = master
+to = qemu-de...@nongnu.org
+cc = qemu-...@nongnu.org
+cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit 
--nogit-fallback 2>/dev/null
+
+[gitpublishprofile "s390"]
+base = master
+to = qemu-de...@nongnu.org
+cc = qemu-s...@nongnu.org
+cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit 
--nogit-fallback 2>/dev/null
+
+[gitpublishprofile "ppc"]
+base = master
+to = qemu-de...@nongnu.org
+cc = qemu-...@nongnu.org
+cccmd = scripts/get_maintainer.pl --noroles --norolestats --nogit 
--nogit-fallback 2>/dev/null
-- 
2.14.3

[Qemu-block] [PULL 0/6] Block patches

[Stefan Hajnoczi]

The following changes since commit 136c67e07869227b21b3f627316e03679ce7b738:

  Merge remote-tracking branch 
'remotes/bkoppelmann/tags/pull-tricore-2018-03-02' into staging (2018-03-02 
16:56:20 +)

are available in the Git repository at:

  git://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 23500c6a9409efc80d696aede0629bfbe7556a90:

  README: Document 'git-publish' workflow (2018-03-05 09:03:17 +)


Pull request

Mostly patches that are only indirectly related to the block layer, but I've
reviewed them and there is no maintainer.



Fam Zheng (2):
  Add a git-publish configuration file
  README: Document 'git-publish' workflow

Su Hang (3):
  util/uri.c: Coding style check, Only whitespace involved
  util/uri.c: remove brackets that wrap `return` statement's content.
  util/uri.c: wrap single statement blocks with braces {}

Thomas Huth (1):
  tests/libqos: Check for valid dev pointer when looking for PCI devices

 tests/libqos/virtio-pci.c |4 +-
 util/uri.c| 1733 -
 .gitpublish   |   51 ++
 README|   31 +-
 4 files changed, 1014 insertions(+), 805 deletions(-)
 create mode 100644 .gitpublish

-- 
2.14.3

[Qemu-block] [PULL 3/6] util/uri.c: wrap single statement blocks with braces {}

[Stefan Hajnoczi]

From: Su Hang 

For this patch, using curly braces to wrap `if` `while` `else` statements,
which only hold single statement. For example:
'''
if (cond)
statement;
'''
to
'''
if (cond) {
statement;
}
'''

And using tricks that compare the disassemblies before and after
code changes, to make sure code logic isn't changed:
'''
git checkout master
make util/uri.o
strip util/uri.o
objdump -Drx util/uri.o > /tmp/uri-master.txt
git checkout cleanupbranch
make util/uri.o
strip util/uri.o
objdump -Drx util/uri.o > /tmp/uri-cleanup.txt

Signed-off-by: Stefan Hajnoczi 
---
 util/uri.c | 463 +++--
 1 file changed, 294 insertions(+), 169 deletions(-)

diff --git a/util/uri.c b/util/uri.c
index bb2576cf21..93ecefdaaf 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -211,16 +211,19 @@ static int rfc3986_parse_scheme(URI *uri, const char 
**str)
 {
 const char *cur;
 
-if (str == NULL)
+if (str == NULL) {
 return -1;
+}
 
 cur = *str;
-if (!ISA_ALPHA(cur))
+if (!ISA_ALPHA(cur)) {
 return 2;
+}
 cur++;
 while (ISA_ALPHA(cur) || ISA_DIGIT(cur) || (*cur == '+') || (*cur == '-') 
||
-   (*cur == '.'))
+   (*cur == '.')) {
 cur++;
+}
 if (uri != NULL) {
 g_free(uri->scheme);
 uri->scheme = g_strndup(*str, cur - *str);
@@ -248,21 +251,24 @@ static int rfc3986_parse_fragment(URI *uri, const char 
**str)
 {
 const char *cur;
 
-if (str == NULL)
+if (str == NULL) {
 return -1;
+}
 
 cur = *str;
 
 while ((ISA_PCHAR(cur)) || (*cur == '/') || (*cur == '?') ||
(*cur == '[') || (*cur == ']') ||
-   ((uri != NULL) && (uri->cleanup & 1) && (IS_UNWISE(cur
+   ((uri != NULL) && (uri->cleanup & 1) && (IS_UNWISE(cur {
 NEXT(cur);
+}
 if (uri != NULL) {
 g_free(uri->fragment);
-if (uri->cleanup & 2)
+if (uri->cleanup & 2) {
 uri->fragment = g_strndup(*str, cur - *str);
-else
+} else {
 uri->fragment = uri_string_unescape(*str, cur - *str, NULL);
+}
 }
 *str = cur;
 return 0;
@@ -283,14 +289,16 @@ static int rfc3986_parse_query(URI *uri, const char **str)
 {
 const char *cur;
 
-if (str == NULL)
+if (str == NULL) {
 return -1;
+}
 
 cur = *str;
 
 while ((ISA_PCHAR(cur)) || (*cur == '/') || (*cur == '?') ||
-   ((uri != NULL) && (uri->cleanup & 1) && (IS_UNWISE(cur
+   ((uri != NULL) && (uri->cleanup & 1) && (IS_UNWISE(cur {
 NEXT(cur);
+}
 if (uri != NULL) {
 g_free(uri->query);
 uri->query = g_strndup(*str, cur - *str);
@@ -351,15 +359,17 @@ static int rfc3986_parse_user_info(URI *uri, const char 
**str)
 
 cur = *str;
 while (ISA_UNRESERVED(cur) || ISA_PCT_ENCODED(cur) || ISA_SUB_DELIM(cur) ||
-   (*cur == ':'))
+   (*cur == ':')) {
 NEXT(cur);
+}
 if (*cur == '@') {
 if (uri != NULL) {
 g_free(uri->user);
-if (uri->cleanup & 2)
+if (uri->cleanup & 2) {
 uri->user = g_strndup(*str, cur - *str);
-else
+} else {
 uri->user = uri_string_unescape(*str, cur - *str, NULL);
+}
 }
 *str = cur;
 return 0;
@@ -385,22 +395,24 @@ static int rfc3986_parse_dec_octet(const char **str)
 {
 const char *cur = *str;
 
-if (!(ISA_DIGIT(cur)))
+if (!(ISA_DIGIT(cur))) {
 return 1;
-if (!ISA_DIGIT(cur + 1))
+}
+if (!ISA_DIGIT(cur + 1)) {
 cur++;
-else if ((*cur != '0') && (ISA_DIGIT(cur + 1)) && (!ISA_DIGIT(cur + 2)))
+} else if ((*cur != '0') && (ISA_DIGIT(cur + 1)) && (!ISA_DIGIT(cur + 2))) 
{
 cur += 2;
-else if ((*cur == '1') && (ISA_DIGIT(cur + 1)) && (ISA_DIGIT(cur + 2)))
+} else if ((*cur == '1') && (ISA_DIGIT(cur + 1)) && (ISA_DIGIT(cur + 2))) {
 cur += 3;
-else if ((*cur == '2') && (*(cur + 1) >= '0') && (*(cur + 1) <= '4') &&
- (ISA_DIGIT(cur + 2)))
+} else if ((*cur == '2') && (*(cur + 1) >= '0') && (*(cur + 1) <= '4') &&
+ (ISA_DIGIT(cur + 2))) {
 cur += 3;
-else if ((*cur == '2') && (*(cur + 1) == '5') && (*(cur + 2) >= '0') &&
- (*(cur + 1) <= '5'))
+} else if ((*cur == '2') && (*(cur + 1) == '5') && (*(cur + 2) >= '0') &&
+ (*(cur + 1) <= '5')) {
 cur += 3;
-else
+} else {
 return 1;
+}
 *str = cur;
 return 0;
 }
@@ -430,10 +442,12 @@ static int rfc3986_parse_host(URI *uri, const char **str)
  */
 if (*cur == '[') {
 cur++;
-while ((*cur != ']') && (*cur != 0))
+while ((*cur != ']') && (*cur != 0)) {
 cur++;
-if (*cur != ']')
+}
+if (*cur != ']') {
 return 1;
+ 

[Qemu-block] [PULL 4/6] tests/libqos: Check for valid dev pointer when looking for PCI devices

[Stefan Hajnoczi]

From: Thomas Huth 

dev could be NULL if the PCI device can not be found due to some
reasons, so we must not dereference the pointer in this case.

Signed-off-by: Thomas Huth 
Message-id: 1519713884-2346-1-git-send-email-th...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/libqos/virtio-pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
index 7ac15c04e1..550dede0a2 100644
--- a/tests/libqos/virtio-pci.c
+++ b/tests/libqos/virtio-pci.c
@@ -315,7 +315,9 @@ QVirtioPCIDevice *qvirtio_pci_device_find(QPCIBus *bus, 
uint16_t device_type)
 qvirtio_pci_foreach(bus, device_type, false, 0,
 qvirtio_pci_assign_device, );
 
-dev->vdev.bus = _pci;
+if (dev) {
+dev->vdev.bus = _pci;
+}
 
 return dev;
 }
-- 
2.14.3

[Qemu-block] [PULL 2/6] util/uri.c: remove brackets that wrap `return` statement's content.

[Stefan Hajnoczi]

From: Su Hang 

only remove brackets that wrap `return` statements' content.

use `perl -pi -e "s/return \((.*?)\);/return \1;/g" util/uri.c`
to remove pattern like this: "return (1);"

Signed-off-by: Su Hang 
Reviewed-by: Thomas Huth 
Message-id: 1519533358-13759-3-git-send-email-suhan...@mails.ucas.ac.cn
Signed-off-by: Stefan Hajnoczi 
---
 util/uri.c | 160 ++---
 1 file changed, 80 insertions(+), 80 deletions(-)

diff --git a/util/uri.c b/util/uri.c
index cf09f41735..bb2576cf21 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -212,11 +212,11 @@ static int rfc3986_parse_scheme(URI *uri, const char 
**str)
 const char *cur;
 
 if (str == NULL)
-return (-1);
+return -1;
 
 cur = *str;
 if (!ISA_ALPHA(cur))
-return (2);
+return 2;
 cur++;
 while (ISA_ALPHA(cur) || ISA_DIGIT(cur) || (*cur == '+') || (*cur == '-') 
||
(*cur == '.'))
@@ -226,7 +226,7 @@ static int rfc3986_parse_scheme(URI *uri, const char **str)
 uri->scheme = g_strndup(*str, cur - *str);
 }
 *str = cur;
-return (0);
+return 0;
 }
 
 /**
@@ -249,7 +249,7 @@ static int rfc3986_parse_fragment(URI *uri, const char 
**str)
 const char *cur;
 
 if (str == NULL)
-return (-1);
+return -1;
 
 cur = *str;
 
@@ -265,7 +265,7 @@ static int rfc3986_parse_fragment(URI *uri, const char 
**str)
 uri->fragment = uri_string_unescape(*str, cur - *str, NULL);
 }
 *str = cur;
-return (0);
+return 0;
 }
 
 /**
@@ -284,7 +284,7 @@ static int rfc3986_parse_query(URI *uri, const char **str)
 const char *cur;
 
 if (str == NULL)
-return (-1);
+return -1;
 
 cur = *str;
 
@@ -296,7 +296,7 @@ static int rfc3986_parse_query(URI *uri, const char **str)
 uri->query = g_strndup(*str, cur - *str);
 }
 *str = cur;
-return (0);
+return 0;
 }
 
 /**
@@ -362,9 +362,9 @@ static int rfc3986_parse_user_info(URI *uri, const char 
**str)
 uri->user = uri_string_unescape(*str, cur - *str, NULL);
 }
 *str = cur;
-return (0);
+return 0;
 }
-return (1);
+return 1;
 }
 
 /**
@@ -386,7 +386,7 @@ static int rfc3986_parse_dec_octet(const char **str)
 const char *cur = *str;
 
 if (!(ISA_DIGIT(cur)))
-return (1);
+return 1;
 if (!ISA_DIGIT(cur + 1))
 cur++;
 else if ((*cur != '0') && (ISA_DIGIT(cur + 1)) && (!ISA_DIGIT(cur + 2)))
@@ -400,9 +400,9 @@ static int rfc3986_parse_dec_octet(const char **str)
  (*(cur + 1) <= '5'))
 cur += 3;
 else
-return (1);
+return 1;
 *str = cur;
-return (0);
+return 0;
 }
 /**
  * rfc3986_parse_host:
@@ -433,7 +433,7 @@ static int rfc3986_parse_host(URI *uri, const char **str)
 while ((*cur != ']') && (*cur != 0))
 cur++;
 if (*cur != ']')
-return (1);
+return 1;
 cur++;
 goto found;
 }
@@ -479,7 +479,7 @@ found:
 uri->server = NULL;
 }
 *str = cur;
-return (0);
+return 0;
 }
 
 /**
@@ -510,15 +510,15 @@ static int rfc3986_parse_authority(URI *uri, const char 
**str)
 cur++;
 ret = rfc3986_parse_host(uri, );
 if (ret != 0)
-return (ret);
+return ret;
 if (*cur == ':') {
 cur++;
 ret = rfc3986_parse_port(uri, );
 if (ret != 0)
-return (ret);
+return ret;
 }
 *str = cur;
-return (0);
+return 0;
 }
 
 /**
@@ -544,13 +544,13 @@ static int rfc3986_parse_segment(const char **str, char 
forbid, int empty)
 cur = *str;
 if (!ISA_PCHAR(cur)) {
 if (empty)
-return (0);
-return (1);
+return 0;
+return 1;
 }
 while (ISA_PCHAR(cur) && (*cur != forbid))
 NEXT(cur);
 *str = cur;
-return (0);
+return 0;
 }
 
 /**
@@ -576,7 +576,7 @@ static int rfc3986_parse_path_ab_empty(URI *uri, const char 
**str)
 cur++;
 ret = rfc3986_parse_segment(, 0, 1);
 if (ret != 0)
-return (ret);
+return ret;
 }
 if (uri != NULL) {
 g_free(uri->path);
@@ -590,7 +590,7 @@ static int rfc3986_parse_path_ab_empty(URI *uri, const char 
**str)
 }
 }
 *str = cur;
-return (0);
+return 0;
 }
 
 /**
@@ -613,7 +613,7 @@ static int rfc3986_parse_path_absolute(URI *uri, const char 
**str)
 cur = *str;
 
 if (*cur != '/')
-return (1);
+return 1;
 cur++;
 ret = rfc3986_parse_segment(, 0, 0);
 if (ret == 0) {
@@ -621,7 +621,7 @@ static int rfc3986_parse_path_absolute(URI *uri, const char 
**str)
 cur++;
 ret = rfc3986_parse_segment(, 0, 1);
 if (ret != 0)
-return (ret);
+