Re: [Qemu-block] [Qemu-devel] [PULL 1/9] gluster: Fix blockdev-add with server.N.type=unix

2018-04-03 Thread Jeff Cody 
On Tue, Apr 03, 2018 at 06:33:52PM +0200, Kevin Wolf wrote:
> The legacy command line interface gets the socket path from an option
> called 'socket'. QAPI in contract uses SocketAddress, where the
> corresponding option is called 'path'.
> 
> Fix the gluster block driver to accept both 'socket' and 'path', with
> 'path' being the preferred syntax.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1545155
> 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Kevin Wolf 
> Reviewed-by: Eric Blake 

Oops, you and I had a pull request collision on this patch (it is in both of
our PRs); Sorry, my fault, I didn't know you were pulling it in through your
tree.

> ---
>  block/gluster.c | 21 +
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/block/gluster.c b/block/gluster.c
> index 296e036b3d..4adc1a875b 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -167,7 +167,12 @@ static QemuOptsList runtime_unix_opts = {
>  {
>  .name = GLUSTER_OPT_SOCKET,
>  .type = QEMU_OPT_STRING,
> -.help = "socket file path)",
> +.help = "socket file path (legacy)",
> +},
> +{
> +.name = GLUSTER_OPT_PATH,
> +.type = QEMU_OPT_STRING,
> +.help = "socket file path (QAPI)",
>  },
>  { /* end of list */ }
>  },
> @@ -615,10 +620,18 @@ static int 
> qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
>  goto out;
>  }
>  
> -ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);
> +ptr = qemu_opt_get(opts, GLUSTER_OPT_PATH);
> +if (!ptr) {
> +ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);
> +} else if (qemu_opt_get(opts, GLUSTER_OPT_SOCKET)) {
> +error_setg(_err,
> +   "Conflicting parameters 'path' and 'socket'");
> +error_append_hint(_err, GERR_INDEX_HINT, i);
> +goto out;
> +}
>  if (!ptr) {
>  error_setg(_err, QERR_MISSING_PARAMETER,
> -   GLUSTER_OPT_SOCKET);
> +   GLUSTER_OPT_PATH);
>  error_append_hint(_err, GERR_INDEX_HINT, i);
>  goto out;
>  }
> @@ -684,7 +697,7 @@ static int qemu_gluster_parse(BlockdevOptionsGluster 
> *gconf,
>   "file.server.0.host=1.2.3.4,"
>   "file.server.0.port=24007,"
>   "file.server.1.transport=unix,"
> - "file.server.1.socket=/var/run/glusterd.socket 
> ..."
> + "file.server.1.path=/var/run/glusterd.socket 
> ..."
>   "\n");
>  return ret;
>  }
> -- 
> 2.13.6
> 
>

Re: [Qemu-block] [Qemu-devel] [PATCH] iotests: fix 169

2018-04-03 Thread John Snow 


On 04/03/2018 12:23 PM, Max Reitz wrote:
> On 2018-03-30 18:10, Vladimir Sementsov-Ogievskiy wrote:
>> Use MIGRATION events instead of RESUME. Also, make a TODO: enable
>> dirty-bitmaps capability for offline case.
>>
>> This (likely) fixes racy faults at least of the following types:
>>
>> - timeout on waiting for RESUME event
>> - sha256 mismatch on 136 (138 after this patch)
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
>> ---
>>
>> This patch is a true change for the test anyway. But I don't understand,
>> why (and do really) it fixes the things. And I'm not sure about do we
>> really have a bug in bitmap migration or persistence. So, it's up to you,
>> take it into 2.12... 
>>
>> It was already discussed, that "STOP" event is bad for tests. What about
>> "RESUME"? How can we miss it? And sha256 mismatch is really something
>> strange.
>>
>> Max, please check, do it fix 169 for you.
>>
>>  tests/qemu-iotests/169 | 44 +++-
>>  1 file changed, 23 insertions(+), 21 deletions(-)
> 
> This makes the test pass (thanks!), but it still leaves behind five cats...
> 
> Max
> 
> 

Hmm:

jhuston  14772  0.0  0.0   4296   784 pts/3S16:12   0:00 cat
/home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
jhuston  14796  0.0  0.0   4296   764 pts/3S16:12   0:00 cat
/home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
jhuston  14940  0.0  0.0   4296   788 pts/3S16:12   0:00 cat
/home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
jhuston  14964  0.0  0.0   4296   720 pts/3S16:12   0:00 cat
/home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file
jhuston  15052  0.0  0.0   4296   768 pts/3S16:12   0:00 cat
/home/bos/jhuston/src/qemu/bin/git/tests/qemu-iotests/scratch/mig_file

Why do these get left behind? Nothing to consume the data...?

[Qemu-block] [PULL 9/9] iotests: Test abnormally large size in compressed cluster descriptor

2018-04-03 Thread Kevin Wolf 
From: Alberto Garcia 

L2 entries for compressed clusters have a field that indicates the
number of sectors used to store the data in the image.

That's however not the size of the compressed data itself, just the
number of sectors where that data is located. The actual data size is
usually not a multiple of the sector size, and therefore cannot be
represented with this field.

The way it works is that QEMU reads all the specified sectors and
starts decompressing the data until there's enough to recover the
original uncompressed cluster. If there are any bytes left that
haven't been decompressed they are simply ignored.

One consequence of this is that even if the size field is larger than
it needs to be QEMU can handle it just fine: it will read more data
from disk but it will ignore the extra bytes.

This test creates an image with two compressed clusters that use 5
sectors (2.5 KB) each, increases the size field to the maximum (8192
sectors, or 4 MB) and verifies that the data can be read without
problems.

This test is important because while the decompressed data takes
exactly one cluster, the maximum value allowed in the compressed size
field is twice the cluster size. So although QEMU won't produce images
with such large values we need to make sure that it can handle them.

Another effect of increasing the size field is that it can make
it include data from the following host cluster(s). In this case
'qemu-img check' will detect that the refcounts are not correct, and
we'll need to rebuild them.

Additionally, this patch also tests that decreasing the size corrupts
the image since the original data can no longer be recovered. In this
case QEMU returns an error when trying to read the compressed data,
but 'qemu-img check' doesn't see anything wrong if the refcounts are
consistent.

One possible task for the future is to make 'qemu-img check' verify
the sizes of the compressed clusters, by trying to decompress the data
and checking that the size stored in the L2 entry is correct.

Signed-off-by: Alberto Garcia 
Message-id: 20180329120745.11154-1-be...@igalia.com
Reviewed-by: Eric Blake 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/122 | 47 ++
 tests/qemu-iotests/122.out | 33 
 2 files changed, 80 insertions(+)

diff --git a/tests/qemu-iotests/122 b/tests/qemu-iotests/122
index 45b359c2ba..6cf4fcb866 100755
--- a/tests/qemu-iotests/122
+++ b/tests/qemu-iotests/122
@@ -130,6 +130,53 @@ $QEMU_IO -c "read -P 01024k 1022k" "$TEST_IMG" 2>&1 | 
_filter_qemu_io | _fil
 
 
 echo
+echo "=== Corrupted size field in compressed cluster descriptor ==="
+echo
+# Create an empty image and fill half of it with compressed data.
+# The L2 entries of the two compressed clusters are located at
+# 0x80 and 0x88, their original values are 0x400800a0
+# and 0x400800a00802 (5 sectors for compressed data each).
+_make_test_img 8M -o cluster_size=2M
+$QEMU_IO -c "write -c -P 0x11 0 2M" -c "write -c -P 0x11 2M 2M" "$TEST_IMG" \
+ 2>&1 | _filter_qemu_io | _filter_testdir
+
+# Reduce size of compressed data to 4 sectors: this corrupts the image.
+poke_file "$TEST_IMG" $((0x80)) "\x40\x06"
+$QEMU_IO -c "read  -P 0x11 0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
+
+# 'qemu-img check' however doesn't see anything wrong because it
+# doesn't try to decompress the data and the refcounts are consistent.
+# TODO: update qemu-img so this can be detected.
+_check_test_img
+
+# Increase size of compressed data to the maximum (8192 sectors).
+# This makes QEMU read more data (8192 sectors instead of 5, host
+# addresses [0xa0, 0xdf]), but the decompression algorithm
+# stops once we have enough to restore the uncompressed cluster, so
+# the rest of the data is ignored.
+poke_file "$TEST_IMG" $((0x80)) "\x7f\xfe"
+# Do it also for the second compressed cluster (L2 entry at 0x88).
+# In this case the compressed data would span 3 host clusters
+# (host addresses: [0xa00802, 0xe00801])
+poke_file "$TEST_IMG" $((0x88)) "\x7f\xfe"
+
+# Here the image is too small so we're asking QEMU to read beyond the
+# end of the image.
+$QEMU_IO -c "read  -P 0x11  0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
+# But if we grow the image we won't be reading beyond its end anymore.
+$QEMU_IO -c "write -P 0x22 4M 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
+$QEMU_IO -c "read  -P 0x11  0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | 
_filter_testdir
+
+# The refcount data is however wrong because due to the increased size
+# of the compressed data it now reaches the following host clusters.
+# This can be repaired by qemu-img check by increasing the refcount of
+# those clusters.
+# TODO: update qemu-img to correct the compressed cluster size instead.
+_check_test_img -r all
+$QEMU_IO -c "read  -P 0x11  0 4M"

[Qemu-block] [PULL 0/9] Block layer patches for 2.12.0-rc2

2018-04-03 Thread Kevin Wolf 
The following changes since commit f184de7553272223d6af731d7d623a7cebf710b5:

  Merge remote-tracking branch 
'remotes/riscv/tags/riscv-qemu-2.12-critical-fixes' into staging (2018-03-31 
09:42:33 +0100)

are available in the git repository at:

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

for you to fetch changes up to 9c1386d3ff76c5983529884e3d8420df958c5a29:

  Merge remote-tracking branch 'mreitz/tags/pull-block-2018-04-03' into 
queue-block (2018-04-03 17:48:45 +0200)


Block layer patches


Alberto Garcia (3):
  iotests: Update 051 and 186 after commit 1454509726719e0933c
  iotests: Update 186 after commit ac64273c66ab136c44043259162
  iotests: Test abnormally large size in compressed cluster descriptor

Jeff Cody (1):
  block: handle invalid lseek returns gracefully

Kevin Wolf (2):
  gluster: Fix blockdev-add with server.N.type=unix
  Merge remote-tracking branch 'mreitz/tags/pull-block-2018-04-03' into 
queue-block

Lukáš Doktor (1):
  qemu-iotests: Use ppc64 qemu_arch on ppc64le host

Max Reitz (2):
  block/file-posix: Fix fully preallocated truncate
  iotests: Test preallocated truncate of 2G image

Vladimir Sementsov-Ogievskiy (1):
  iotests: fix 208 for luks format

 block/file-posix.c   | 19 +++--
 block/gluster.c  | 21 --
 tests/qemu-iotests/051.pc.out| 20 --
 tests/qemu-iotests/106   | 24 
 tests/qemu-iotests/106.out   | 10 +
 tests/qemu-iotests/122   | 47 ++
 tests/qemu-iotests/122.out   | 33 
 tests/qemu-iotests/186   |  6 +--
 tests/qemu-iotests/186.out   | 84 ++--
 tests/qemu-iotests/208   |  2 +-
 tests/qemu-iotests/check |  4 +-
 tests/qemu-iotests/common.config |  1 +
 tests/qemu-iotests/common.filter |  5 +++
 13 files changed, 184 insertions(+), 92 deletions(-)

[Qemu-block] [PULL 4/9] iotests: Update 186 after commit ac64273c66ab136c44043259162

2018-04-03 Thread Kevin Wolf 
From: Alberto Garcia 

Commit ac64273c66ab136c44 modified the output of iotest 186, changing
the QOM path of floppy drives from /machine/unattached/device[17] to
/machine/unattached/device[13].

Instead of updating the test output to reflect this change, this patch
adds a new filter that hides all QOM paths from the 'Attached to:'
line of the 'info block' command.

Signed-off-by: Alberto Garcia 
Cc: Philippe Mathieu-Daudé 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/186   |  2 +-
 tests/qemu-iotests/186.out   | 56 
 tests/qemu-iotests/common.filter |  5 
 3 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/tests/qemu-iotests/186 b/tests/qemu-iotests/186
index 9687243d34..0aa4395a57 100755
--- a/tests/qemu-iotests/186
+++ b/tests/qemu-iotests/186
@@ -64,7 +64,7 @@ function check_info_block()
 {
 echo "info block" |
 do_run_qemu "$@" | _filter_win32 | _filter_hmp | _filter_qemu |
-_filter_generated_node_ids
+_filter_generated_node_ids | _filter_qom_path
 }
 
 
diff --git a/tests/qemu-iotests/186.out b/tests/qemu-iotests/186.out
index ec75c0fc60..716b01ac3d 100644
--- a/tests/qemu-iotests/186.out
+++ b/tests/qemu-iotests/186.out
@@ -7,7 +7,7 @@ Testing: -device floppy
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) info block
 /machine/peripheral-anon/device[1]: [not inserted]
-Attached to:  /machine/peripheral-anon/device[1]
+Attached to:  PATH
 Removable device: not locked, tray closed
 (qemu) quit
 
@@ -23,7 +23,7 @@ Testing: -device ide-cd
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) info block
 /machine/peripheral-anon/device[1]: [not inserted]
-Attached to:  /machine/peripheral-anon/device[1]
+Attached to:  PATH
 Removable device: not locked, tray closed
 (qemu) quit
 
@@ -39,7 +39,7 @@ Testing: -device scsi-cd
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) info block
 /machine/peripheral-anon/device[1]: [not inserted]
-Attached to:  /machine/peripheral-anon/device[1]
+Attached to:  PATH
 Removable device: not locked, tray closed
 (qemu) quit
 
@@ -58,7 +58,7 @@ Testing: -blockdev driver=null-co,node-name=null -device 
ide-hd,drive=null
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) info block
 null: null-co:// (null-co)
-Attached to:  /machine/peripheral-anon/device[1]
+Attached to:  PATH
 Cache mode:   writeback
 (qemu) quit
 
@@ -74,7 +74,7 @@ Testing: -blockdev driver=null-co,node-name=null -device 
scsi-hd,drive=null
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) info block
 null: null-co:// (null-co)
-Attached to:  /machine/peripheral-anon/device[1]
+Attached to:  PATH
 Cache mode:   writeback
 (qemu) quit
 
@@ -90,7 +90,7 @@ Testing: -blockdev driver=null-co,node-name=null -device 
virtio-blk-pci,drive=nu
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) info block
 null: null-co:// (null-co)
-Attached to:  /machine/peripheral-anon/device[1]/virtio-backend
+Attached to:  PATH
 Cache mode:   writeback
 (qemu) quit
 
@@ -98,7 +98,7 @@ Testing: -blockdev driver=null-co,node-name=null -device 
virtio-blk-pci,drive=nu
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) info block
 null: null-co:// (null-co)
-Attached to:  /machine/peripheral/qdev_id/virtio-backend
+Attached to:  PATH
 Cache mode:   writeback
 (qemu) quit
 
@@ -106,7 +106,7 @@ Testing: -blockdev driver=null-co,node-name=null -device 
floppy,drive=null
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) info block
 null: null-co:// (null-co)
-Attached to:  /machine/peripheral-anon/device[1]
+Attached to:  PATH
 Removable device: not locked, tray closed
 Cache mode:   writeback
 (qemu) quit
@@ -124,7 +124,7 @@ Testing: -blockdev driver=null-co,node-name=null -device 
ide-cd,drive=null
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) info block
 null: null-co:// (null-co)
-Attached to:  /machine/peripheral-anon/device[1]
+Attached to:  PATH
 Removable device: not locked, tray closed
 Cache mode:   writeback
 (qemu) quit
@@ -142,7 +142,7 @@ Testing: -blockdev driver=null-co,node-name=null -device 
scsi-cd,drive=null
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) info block
 null: null-co:// (null-co)
-Attached to:  /machine/peripheral-anon/device[1]
+Attached to:  PATH
 Removable device: not locked, tray closed
 Cache mode:   writeback
 (qemu) quit
@@ -191,7 +191,7 @@ none0 (null): null-co:// (null-co)
 Cache mode:   writeback
 
 null: null-co:// (null-co)
-Attached to:  /machine/peripheral/qdev_id/virtio-backend
+Attached to:  PATH
 Cache mode:   writeback

[Qemu-block] [PULL 3/9] iotests: Update 051 and 186 after commit 1454509726719e0933c

2018-04-03 Thread Kevin Wolf 
From: Alberto Garcia 

SCSI controllers are no longer created automatically for
-drive if=scsi, so this patch updates the tests that relied
on that.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Cc: Thomas Huth 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/051.pc.out | 20 
 tests/qemu-iotests/186|  4 
 tests/qemu-iotests/186.out| 28 
 3 files changed, 52 deletions(-)

diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
index 830c11880a..b01f9a90d7 100644
--- a/tests/qemu-iotests/051.pc.out
+++ b/tests/qemu-iotests/051.pc.out
@@ -117,20 +117,10 @@ Testing: -drive if=ide,media=cdrom
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) quit
 
-Testing: -drive if=scsi,media=cdrom
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -drive if=scsi,media=cdrom: warning: bus=0,unit=0 is 
deprecated with this machine type
-quit
-
 Testing: -drive if=ide
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) QEMU_PROG: Initialization of device ide-hd failed: Device needs media, 
but drive is empty
 
-Testing: -drive if=scsi
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -drive if=scsi: warning: bus=0,unit=0 is deprecated with 
this machine type
-QEMU_PROG: -drive if=scsi: Device needs media, but drive is empty
-
 Testing: -drive if=virtio
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) QEMU_PROG: -drive if=virtio: Device needs media, but drive is empty
@@ -170,20 +160,10 @@ Testing: -drive 
file=TEST_DIR/t.qcow2,if=ide,media=cdrom,readonly=on
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) quit
 
-Testing: -drive file=TEST_DIR/t.qcow2,if=scsi,media=cdrom,readonly=on
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -drive 
file=TEST_DIR/t.qcow2,if=scsi,media=cdrom,readonly=on: warning: bus=0,unit=0 is 
deprecated with this machine type
-quit
-
 Testing: -drive file=TEST_DIR/t.qcow2,if=ide,readonly=on
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) QEMU_PROG: Initialization of device ide-hd failed: Block node is 
read-only
 
-Testing: -drive file=TEST_DIR/t.qcow2,if=scsi,readonly=on
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=scsi,readonly=on: warning: 
bus=0,unit=0 is deprecated with this machine type
-quit
-
 Testing: -drive file=TEST_DIR/t.qcow2,if=virtio,readonly=on
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) quit
diff --git a/tests/qemu-iotests/186 b/tests/qemu-iotests/186
index 44cc01ed87..9687243d34 100755
--- a/tests/qemu-iotests/186
+++ b/tests/qemu-iotests/186
@@ -133,10 +133,6 @@ check_info_block -drive if=ide,driver=null-co
 check_info_block -drive if=ide,media=cdrom
 check_info_block -drive if=ide,driver=null-co,media=cdrom
 
-check_info_block -drive if=scsi,driver=null-co
-check_info_block -drive if=scsi,media=cdrom
-check_info_block -drive if=scsi,driver=null-co,media=cdrom
-
 check_info_block -drive if=virtio,driver=null-co
 
 check_info_block -drive if=pflash,driver=null-co,size=1M
diff --git a/tests/qemu-iotests/186.out b/tests/qemu-iotests/186.out
index c8377fe146..ec75c0fc60 100644
--- a/tests/qemu-iotests/186.out
+++ b/tests/qemu-iotests/186.out
@@ -442,34 +442,6 @@ ide0-cd0 (NODE_NAME): null-co:// (null-co, read-only)
 Cache mode:   writeback
 (qemu) quit
 
-Testing: -drive if=scsi,driver=null-co
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -drive if=scsi,driver=null-co: warning: bus=0,unit=0 is 
deprecated with this machine type
-info block
-scsi0-hd0 (NODE_NAME): null-co:// (null-co)
-Attached to:  /machine/unattached/device[27]/scsi.0/legacy[0]
-Cache mode:   writeback
-(qemu) quit
-
-Testing: -drive if=scsi,media=cdrom
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -drive if=scsi,media=cdrom: warning: bus=0,unit=0 is 
deprecated with this machine type
-info block
-scsi0-cd0: [not inserted]
-Attached to:  /machine/unattached/device[27]/scsi.0/legacy[0]
-Removable device: not locked, tray closed
-(qemu) quit
-
-Testing: -drive if=scsi,driver=null-co,media=cdrom
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -drive if=scsi,driver=null-co,media=cdrom: warning: 
bus=0,unit=0 is deprecated with this machine type
-info block
-scsi0-cd0 (NODE_NAME): null-co:// (null-co, read-only)
-Attached to:  /machine/unattached/device[27]/scsi.0/legacy[0]
-Removable device: not locked, tray closed
-Cache mode:   writeback
-(qemu) quit
-
 Testing: -drive if=virtio,driver=null-co
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) info block
-- 
2.13.6

[Qemu-block] [PULL 2/9] block: handle invalid lseek returns gracefully

2018-04-03 Thread Kevin Wolf 
From: Jeff Cody 

In commit 223a23c198787328ae75bc65d84edf5fde33c0b6, we implemented a
workaround in the gluster driver to handle invalid values returned for
SEEK_DATA or SEEK_HOLE.

In some instances, these same invalid values can be seen in the posix
file handler as well - for example, it has been reported on FUSE gluster
mounts.

Calling assert() for these invalid values is overly harsh; we can safely
return -EIO and allow this case to be treated as a "learned nothing"
case (e.g., D4 / H4, as commented in the code).

This patch does the same thing that 223a23c198787 did for gluster.c,
except in file-posix.c

Signed-off-by: Jeff Cody 
Reviewed-by: Eric Blake 
Signed-off-by: Kevin Wolf 
---
 block/file-posix.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index d7fb772c14..a2f6d8a8c8 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2114,7 +2114,12 @@ static int find_allocation(BlockDriverState *bs, off_t 
start,
 if (offs < 0) {
 return -errno;  /* D3 or D4 */
 }
-assert(offs >= start);
+
+if (offs < start) {
+/* This is not a valid return by lseek().  We are safe to just return
+ * -EIO in this case, and we'll treat it like D4. */
+return -EIO;
+}
 
 if (offs > start) {
 /* D2: in hole, next data at offs */
@@ -2146,7 +2151,12 @@ static int find_allocation(BlockDriverState *bs, off_t 
start,
 if (offs < 0) {
 return -errno;  /* D1 and (H3 or H4) */
 }
-assert(offs >= start);
+
+if (offs < start) {
+/* This is not a valid return by lseek().  We are safe to just return
+ * -EIO in this case, and we'll treat it like H4. */
+return -EIO;
+}
 
 if (offs > start) {
 /*
-- 
2.13.6

[Qemu-block] [PULL 6/9] block/file-posix: Fix fully preallocated truncate

2018-04-03 Thread Kevin Wolf 
From: Max Reitz 

Storing the lseek() result in an int results in it overflowing when the
file is at least 2 GB big.  Then, we have a 50 % chance of the result
being "negative" and thus thinking an error occurred when actually
everything went just fine.

So we should use the correct type for storing the result: off_t.

Reported-by: Daniel P. Berrange 
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1549231
Cc: qemu-sta...@nongnu.org
Signed-off-by: Max Reitz 
Message-id: 20180228131315.30194-2-mre...@redhat.com
Reviewed-by: Eric Blake 
Signed-off-by: Max Reitz 
---
 block/file-posix.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index a2f6d8a8c8..3794c0007a 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1701,6 +1701,7 @@ static int raw_regular_truncate(int fd, int64_t offset, 
PreallocMode prealloc,
 case PREALLOC_MODE_FULL:
 {
 int64_t num = 0, left = offset - current_length;
+off_t seek_result;
 
 /*
  * Knowing the final size from the beginning could allow the file
@@ -1715,8 +1716,8 @@ static int raw_regular_truncate(int fd, int64_t offset, 
PreallocMode prealloc,
 
 buf = g_malloc0(65536);
 
-result = lseek(fd, current_length, SEEK_SET);
-if (result < 0) {
+seek_result = lseek(fd, current_length, SEEK_SET);
+if (seek_result < 0) {
 result = -errno;
 error_setg_errno(errp, -result,
  "Failed to seek to the old end of file");
-- 
2.13.6

[Qemu-block] [PULL 1/9] gluster: Fix blockdev-add with server.N.type=unix

2018-04-03 Thread Kevin Wolf 
The legacy command line interface gets the socket path from an option
called 'socket'. QAPI in contract uses SocketAddress, where the
corresponding option is called 'path'.

Fix the gluster block driver to accept both 'socket' and 'path', with
'path' being the preferred syntax.

https://bugzilla.redhat.com/show_bug.cgi?id=1545155

Cc: qemu-sta...@nongnu.org
Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 block/gluster.c | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index 296e036b3d..4adc1a875b 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -167,7 +167,12 @@ static QemuOptsList runtime_unix_opts = {
 {
 .name = GLUSTER_OPT_SOCKET,
 .type = QEMU_OPT_STRING,
-.help = "socket file path)",
+.help = "socket file path (legacy)",
+},
+{
+.name = GLUSTER_OPT_PATH,
+.type = QEMU_OPT_STRING,
+.help = "socket file path (QAPI)",
 },
 { /* end of list */ }
 },
@@ -615,10 +620,18 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster 
*gconf,
 goto out;
 }
 
-ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);
+ptr = qemu_opt_get(opts, GLUSTER_OPT_PATH);
+if (!ptr) {
+ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);
+} else if (qemu_opt_get(opts, GLUSTER_OPT_SOCKET)) {
+error_setg(_err,
+   "Conflicting parameters 'path' and 'socket'");
+error_append_hint(_err, GERR_INDEX_HINT, i);
+goto out;
+}
 if (!ptr) {
 error_setg(_err, QERR_MISSING_PARAMETER,
-   GLUSTER_OPT_SOCKET);
+   GLUSTER_OPT_PATH);
 error_append_hint(_err, GERR_INDEX_HINT, i);
 goto out;
 }
@@ -684,7 +697,7 @@ static int qemu_gluster_parse(BlockdevOptionsGluster *gconf,
  "file.server.0.host=1.2.3.4,"
  "file.server.0.port=24007,"
  "file.server.1.transport=unix,"
- "file.server.1.socket=/var/run/glusterd.socket 
..."
+ "file.server.1.path=/var/run/glusterd.socket ..."
  "\n");
 return ret;
 }
-- 
2.13.6

Re: [Qemu-block] [PATCH] iotests: fix 169

2018-04-03 Thread Max Reitz 
On 2018-03-30 18:10, Vladimir Sementsov-Ogievskiy wrote:
> Use MIGRATION events instead of RESUME. Also, make a TODO: enable
> dirty-bitmaps capability for offline case.
> 
> This (likely) fixes racy faults at least of the following types:
> 
> - timeout on waiting for RESUME event
> - sha256 mismatch on 136 (138 after this patch)
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
> 
> This patch is a true change for the test anyway. But I don't understand,
> why (and do really) it fixes the things. And I'm not sure about do we
> really have a bug in bitmap migration or persistence. So, it's up to you,
> take it into 2.12... 
> 
> It was already discussed, that "STOP" event is bad for tests. What about
> "RESUME"? How can we miss it? And sha256 mismatch is really something
> strange.
> 
> Max, please check, do it fix 169 for you.
> 
>  tests/qemu-iotests/169 | 44 +++-
>  1 file changed, 23 insertions(+), 21 deletions(-)

This makes the test pass (thanks!), but it still leaves behind five cats...

Max

[Qemu-block] [PULL 1/3] blockjob: leak fix, remove from txn when failing early

2018-04-03 Thread Jeff Cody 
From: Marc-André Lureau 

This fixes leaks found by ASAN such as:
  GTESTER tests/test-blockjob
=
==31442==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x7f88483cba38 in __interceptor_calloc (/lib64/libasan.so.4+0xdea38)
#1 0x7f8845e1bd77 in g_malloc0 ../glib/gmem.c:129
#2 0x7f8845e1c04b in g_malloc0_n ../glib/gmem.c:360
#3 0x5584d2732498 in block_job_txn_new /home/elmarco/src/qemu/blockjob.c:172
#4 0x5584d2739b28 in block_job_create /home/elmarco/src/qemu/blockjob.c:973
#5 0x5584d270ae31 in mk_job /home/elmarco/src/qemu/tests/test-blockjob.c:34
#6 0x5584d270b1c1 in do_test_id 
/home/elmarco/src/qemu/tests/test-blockjob.c:57
#7 0x5584d270b65c in test_job_ids 
/home/elmarco/src/qemu/tests/test-blockjob.c:118
#8 0x7f8845e40b69 in test_case_run ../glib/gtestutils.c:2255
#9 0x7f8845e40f29 in g_test_run_suite_internal ../glib/gtestutils.c:2339
#10 0x7f8845e40fd2 in g_test_run_suite_internal ../glib/gtestutils.c:2351
#11 0x7f8845e411e9 in g_test_run_suite ../glib/gtestutils.c:2426
#12 0x7f8845e3fe72 in g_test_run ../glib/gtestutils.c:1692
#13 0x5584d270d6e2 in main /home/elmarco/src/qemu/tests/test-blockjob.c:377
#14 0x7f8843641f29 in __libc_start_main (/lib64/libc.so.6+0x20f29)

Add an assert to make sure that the job doesn't have associated txn before 
free().

[Jeff Cody: N.B., used updated patch provided by John Snow]

Signed-off-by: Marc-André Lureau 
Signed-off-by: Jeff Cody 
---
 blockjob.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index ef3ed69ff1..c510a9fde5 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -204,6 +204,15 @@ void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job)
 block_job_txn_ref(txn);
 }
 
+static void block_job_txn_del_job(BlockJob *job)
+{
+if (job->txn) {
+QLIST_REMOVE(job, txn_list);
+block_job_txn_unref(job->txn);
+job->txn = NULL;
+}
+}
+
 static void block_job_pause(BlockJob *job)
 {
 job->pause_count++;
@@ -232,6 +241,7 @@ void block_job_unref(BlockJob *job)
 {
 if (--job->refcnt == 0) {
 assert(job->status == BLOCK_JOB_STATUS_NULL);
+assert(!job->txn);
 BlockDriverState *bs = blk_bs(job->blk);
 QLIST_REMOVE(job, job_list);
 bs->job = NULL;
@@ -392,6 +402,7 @@ static void block_job_decommission(BlockJob *job)
 job->busy = false;
 job->paused = false;
 job->deferred_to_main_loop = true;
+block_job_txn_del_job(job);
 block_job_state_transition(job, BLOCK_JOB_STATUS_NULL);
 block_job_unref(job);
 }
@@ -481,8 +492,7 @@ static int block_job_finalize_single(BlockJob *job)
 }
 }
 
-QLIST_REMOVE(job, txn_list);
-block_job_txn_unref(job->txn);
+block_job_txn_del_job(job);
 block_job_conclude(job);
 return 0;
 }
-- 
2.13.6

[Qemu-block] [PULL 3/3] gluster: Fix blockdev-add with server.N.type=unix

2018-04-03 Thread Jeff Cody 
From: Kevin Wolf 

The legacy command line interface gets the socket path from an option
called 'socket'. QAPI in contract uses SocketAddress, where the
corresponding option is called 'path'.

Fix the gluster block driver to accept both 'socket' and 'path', with
'path' being the preferred syntax.

https://bugzilla.redhat.com/show_bug.cgi?id=1545155

Cc: qemu-sta...@nongnu.org
Signed-off-by: Kevin Wolf 
Message-id: 20180403110810.25624-1-kw...@redhat.com
Signed-off-by: Jeff Cody 
---
 block/gluster.c | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index 296e036b3d..4adc1a875b 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -167,7 +167,12 @@ static QemuOptsList runtime_unix_opts = {
 {
 .name = GLUSTER_OPT_SOCKET,
 .type = QEMU_OPT_STRING,
-.help = "socket file path)",
+.help = "socket file path (legacy)",
+},
+{
+.name = GLUSTER_OPT_PATH,
+.type = QEMU_OPT_STRING,
+.help = "socket file path (QAPI)",
 },
 { /* end of list */ }
 },
@@ -615,10 +620,18 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster 
*gconf,
 goto out;
 }
 
-ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);
+ptr = qemu_opt_get(opts, GLUSTER_OPT_PATH);
+if (!ptr) {
+ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);
+} else if (qemu_opt_get(opts, GLUSTER_OPT_SOCKET)) {
+error_setg(_err,
+   "Conflicting parameters 'path' and 'socket'");
+error_append_hint(_err, GERR_INDEX_HINT, i);
+goto out;
+}
 if (!ptr) {
 error_setg(_err, QERR_MISSING_PARAMETER,
-   GLUSTER_OPT_SOCKET);
+   GLUSTER_OPT_PATH);
 error_append_hint(_err, GERR_INDEX_HINT, i);
 goto out;
 }
@@ -684,7 +697,7 @@ static int qemu_gluster_parse(BlockdevOptionsGluster *gconf,
  "file.server.0.host=1.2.3.4,"
  "file.server.0.port=24007,"
  "file.server.1.transport=unix,"
- "file.server.1.socket=/var/run/glusterd.socket 
..."
+ "file.server.1.path=/var/run/glusterd.socket ..."
  "\n");
 return ret;
 }
-- 
2.13.6

[Qemu-block] [PULL 2/3] blockjob: use qapi enum helpers

2018-04-03 Thread Jeff Cody 
From: Marc-André Lureau 

QAPI generator provide #define helpers for looking up enum string.

Signed-off-by: Marc-André Lureau 
Reviewed-by: John Snow 
Message-id: 20180327153011.29569-1-marcandre.lur...@redhat.com
Signed-off-by: Jeff Cody 
---
 blockjob.c | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index c510a9fde5..27f957e571 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -75,10 +75,8 @@ static void block_job_state_transition(BlockJob *job, 
BlockJobStatus s1)
 assert(s1 >= 0 && s1 <= BLOCK_JOB_STATUS__MAX);
 trace_block_job_state_transition(job, job->ret, BlockJobSTT[s0][s1] ?
  "allowed" : "disallowed",
- qapi_enum_lookup(_lookup,
-  s0),
- qapi_enum_lookup(_lookup,
-  s1));
+ BlockJobStatus_str(s0),
+ BlockJobStatus_str(s1));
 assert(BlockJobSTT[s0][s1]);
 job->status = s1;
 }
@@ -86,17 +84,15 @@ static void block_job_state_transition(BlockJob *job, 
BlockJobStatus s1)
 static int block_job_apply_verb(BlockJob *job, BlockJobVerb bv, Error **errp)
 {
 assert(bv >= 0 && bv <= BLOCK_JOB_VERB__MAX);
-trace_block_job_apply_verb(job, qapi_enum_lookup(_lookup,
- job->status),
-   qapi_enum_lookup(_lookup, bv),
+trace_block_job_apply_verb(job, BlockJobStatus_str(job->status),
+   BlockJobVerb_str(bv),
BlockJobVerbTable[bv][job->status] ?
"allowed" : "prohibited");
 if (BlockJobVerbTable[bv][job->status]) {
 return 0;
 }
 error_setg(errp, "Job '%s' in state '%s' cannot accept command verb '%s'",
-   job->id, qapi_enum_lookup(_lookup, job->status),
-   qapi_enum_lookup(_lookup, bv));
+   job->id, BlockJobStatus_str(job->status), BlockJobVerb_str(bv));
 return -EPERM;
 }
 
-- 
2.13.6

[Qemu-block] [PULL 0/3] Block patches for 2.12-rc2

2018-04-03 Thread Jeff Cody 
The following changes since commit f184de7553272223d6af731d7d623a7cebf710b5:

  Merge remote-tracking branch 
'remotes/riscv/tags/riscv-qemu-2.12-critical-fixes' into staging (2018-03-31 
09:42:33 +0100)

are available in the git repository at:

  git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request

for you to fetch changes up to 9dae635afa98f83688806861cefe77ff1b4d76a8:

  gluster: Fix blockdev-add with server.N.type=unix (2018-04-03 09:57:14 -0400)


Blockjob and Gluster patches


Kevin Wolf (1):
  gluster: Fix blockdev-add with server.N.type=unix

Marc-André Lureau (2):
  blockjob: leak fix, remove from txn when failing early
  blockjob: use qapi enum helpers

 block/gluster.c | 21 +
 blockjob.c  | 28 +---
 2 files changed, 34 insertions(+), 15 deletions(-)

-- 
2.13.6

Re: [Qemu-block] [PATCH v4 for 2.12 0/3] fix bitmaps migration through shared storage

2018-04-03 Thread Max Reitz 
On 2018-03-30 17:32, Vladimir Sementsov-Ogievskiy wrote:
> 30.03.2018 16:31, Vladimir Sementsov-Ogievskiy wrote:
>> 29.03.2018 18:09, Vladimir Sementsov-Ogievskiy wrote:
>>> 29.03.2018 17:03, Max Reitz wrote:
 On 2018-03-29 10:08, Vladimir Sementsov-Ogievskiy wrote:
> 28.03.2018 17:53, Max Reitz wrote:
>> On 2018-03-27 12:11, Vladimir Sementsov-Ogievskiy wrote:
 [...]

>>> isn't it because a lot of cat processes? will check, update loop to
>>> i=0; while check -qcow2 169; do ((i++)); echo $i OK; killall -9 cat;
>>> done
>> Hmm...  I know I tried to kill all of the cats, but for some
>> reason that
>> didn't really help yesterday.  Seems to help now, for 2.12.0-rc0 at
>> least (that is, before this series).
> reproduced with killing... (without these series, just on master)
>
>> After the whole series, I still get a lot of failures in 169
>> (mismatching bitmap hash, mostly).
>>
>> And interestingly, if I add an abort():
>>
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index 486f3e83b7..9204c1c0ac 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -1481,6 +1481,7 @@ static int coroutine_fn
>> qcow2_do_open(BlockDriverState *bs, QDict *options, }
>>
>>    if (bdrv_dirty_bitmap_next(bs, NULL)) {
>> +    abort();
>>    /* It's some kind of reopen with already existing dirty
>> bitmaps. There
>>     * are no known cases where we need loading bitmaps in
>> such
>> situation,
>>     * so it's safer don't load them.
>>
>> Then this fires for a couple of test cases of 169 even without the
>> third
>> patch of this series.
>>
>> I guess bdrv_dirty_bitmap_next() reacts to some bitmaps that
>> migration
>> adds or something?  Then this would be the wrong condition, because I
>> guess we still want to load the bitmaps that are in the qcow2 file.
>>
>> I'm not sure whether bdrv_has_readonly_bitmaps() is the correct
>> condition then, either, though.  Maybe let's take a step back: We
>> want
>> to load all the bitmaps from the file exactly once, and that is
>> when it
>> is opened the first time.  Or that's what I would have thought...  Is
>> that even correct?
>>
>> Why do we load the bitmaps when the device is inactive anyway?
>> Shouldn't we load them only once the device is activated?
> Hmm, not sure. May be, we don't need. But we anyway need to load them,
> when opening read-only, and we should correspondingly reopen in
> this case.
 Yeah, well, yeah, but the current state seems just wrong. Apparently
 there are cases where a qcow2 node may have bitmaps before we try to
 load them from the file, so the current condition doesn't work.

 Furthermore, it seems like the current "state machine" is too
 complex so
 we don't know which cases are possible anymore and what to do when.

 So the first thing we could do is add a field to the BDRVQCow2State
 that
 tells us whether the bitmaps have been loaded already or not. If not,
 we invoke qcow2_load_dirty_bitmaps() and set the value to true. If the
 value was true already and the BDS is active and R/W now, we call
 qcow2_reopen_bitmaps_rw_hint().  That should solve one problem.
>>>
>>> good idea, will do.
>>>

 The other problem of course is the question whether we should call
 qcow2_load_dirty_bitmaps() at all while the drive is still inactive.
 You know the migration model better than me, so I'm asking this
 question
 to you.  We can phrase it differently: Do we need to load the bitmaps
 before the drive is activated?
>>>
>>> Now I think that we don't need. At least, we don't have such cases in
>>> Virtuozzo (I hope :).
>>>
>>> Why did I doubt:
>>>
>>> 1. We have cases, when we want to start vm as inactive, to be able to
>>> export it's drive as NBD export, push some data to it and then start
>>> the VM (which involves activating)
>>> 2. We have cases, when we want to start vm stopped and operate on
>>> dirty bitmaps.
>>>
>>> If just open all images in inactive mode until vm start, it looks
>>> like we need bitmaps in inactive mode (for 2.). But it looks like
>>> wrong approach anyway.
>>> Firstly, I tried to solve (1.) by simply inactivate_all() in case of
>>> start vm in paused mode, but it breaks at least (2.), so finally, I
>>> solved (1.) by an approach similar with "-incoming defer". So, we
>>> have inactive mode in two cases:
>>>  - incoming migration
>>>  - push data to vm before start
>>>
>>> and, in these cases, we don't need to load dirty-bitmaps.
>>>
>>> Also, inconsistency: now, we remove persistent bitmaps on inactivate.
>>> So, it is inconsistent to load the in inactive mode.
>>>
>>> Ok, I'll try to respin.
>>
>> finally, what cases we actually have for qcow2_do_open?
>>
>> 1. INACTIVE -> ACTIVE (through

Re: [Qemu-block] [PATCH for 2.12] iotests: fix 208 for luks format

2018-04-03 Thread Kevin Wolf 
Am 30.03.2018 um 16:44 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Support luks images creatins like in 205
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 

Thanks, applied to the block branch.

Kevin

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.12 v4 0/2] Update output of some iotests

2018-04-03 Thread Kevin Wolf 
Am 22.03.2018 um 15:45 hat Alberto Garcia geschrieben:
> I sent a patch a few days ago correction the output of iotests 051 and
> 186. I wanted to resend it again but I noticed that 186 needs now more
> changes due to commit ac64273c66ab136c44043259162, so I'm including
> those changes too.

[ Cc: qemu-block ]

Thanks, applied to the block branch.

Kevin

Re: [Qemu-block] [Qemu-devel] [PATCH v2 1/1] iotests: fix test case 185

2018-04-03 Thread Stefan Hajnoczi 
On Tue, Mar 27, 2018 at 11:32:00AM +0800, QingFeng Hao wrote:
> 
> 在 2018/3/23 18:04, Stefan Hajnoczi 写道:
> > On Fri, Mar 23, 2018 at 3:43 AM, QingFeng Hao  
> > wrote:
> > > Test case 185 failed since commit 4486e89c219 --- "vl: introduce 
> > > vm_shutdown()".
> > > It's because of the newly introduced function vm_shutdown calls 
> > > bdrv_drain_all,
> > > which is called later by bdrv_close_all. bdrv_drain_all resumes the jobs
> > > that doubles the speed and offset is doubled.
> > > Some jobs' status are changed as well.
> > > 
> > > The fix is to not resume the jobs that are already yielded and also change
> > > 185.out accordingly.
> > > 
> > > Suggested-by: Stefan Hajnoczi 
> > > Signed-off-by: QingFeng Hao 
> > > ---
> > >   blockjob.c | 10 +-
> > >   include/block/blockjob.h   |  5 +
> > >   tests/qemu-iotests/185.out | 11 +--
> > 
> > If drain no longer forces the block job to iterate, shouldn't the test
> > output remain the same?  (The means the test is fixed by the QEMU
> > patch.)
> > 
> > >   3 files changed, 23 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/blockjob.c b/blockjob.c
> > > index ef3ed69ff1..fa9838ac97 100644
> > > --- a/blockjob.c
> > > +++ b/blockjob.c
> > > @@ -206,11 +206,16 @@ void block_job_txn_add_job(BlockJobTxn *txn, 
> > > BlockJob *job)
> > > 
> > >   static void block_job_pause(BlockJob *job)
> > >   {
> > > -job->pause_count++;
> > > +if (!job->yielded) {
> > > +job->pause_count++;
> > > +}
> > 
> > The pause cannot be ignored.  This change introduces a bug.
> > 
> > Pause is not a synchronous operation that stops the job immediately.
> > Pause just remembers that the job needs to be paused.   When the job
> > runs again (e.g. timer callback, fd handler) it eventually reaches
> > block_job_pause_point() where it really pauses.
> > 
> > The bug in this patch is:
> > 
> > 1. The job has a timer pending.
> > 2. block_job_pause() is called during drain.
> > 3. The timer fires during drain but now the job doesn't know it needs
> > to pause, so it continues running!
> > 
> > Instead what needs to happen is that block_job_pause() remains
> > unmodified but block_job_resume if extended:
> > 
> > static void block_job_resume(BlockJob *job)
> > {
> >  assert(job->pause_count > 0);
> >  job->pause_count--;
> >  if (job->pause_count) {
> >  return;
> >  }
> > +if (job_yielded_before_pause_and_is_still_yielded) {
> Thanks a lot for your great comments! But I can't figure out how to check
> this.
> >  block_job_enter(job);
> > +}
> > }
> > 
> > This handles the case I mentioned above, where the yield ends before
> > pause ends (therefore resume must enter the job!).
> > 
> > To make this a little clearer, there are two cases to consider:
> > 
> > Case 1:
> > 1. Job yields
> > 2. Pause
> > 3. Job is entered from timer/fd callback
> How do I know that if job is entered from ...? thanks

Sorry, in order to answer your question properly I would have to study
the code and get the point where I could write the patch myself.

I have sent a patch to update the test output for the upcoming QEMU 2.12
release.  At this time in the release cycle it's the most appropriate
solution.

Stefan


signature.asc
Description: PGP signature

[Qemu-block] [PATCH] qemu-iotests: update 185 output

2018-04-03 Thread Stefan Hajnoczi 
Commit 4486e89c219c0d1b9bd8dfa0b1dd5b0d51ff2268 ("vl: introduce
vm_shutdown()") added a bdrv_drain_all() call.  As a side-effect of the
drain operation the block job iterates one more time than before.  The
185 output no longer matches and the test is failing now.

It may be possible to avoid the superfluous block job iteration, but
that type of patch is not suitable late in the QEMU 2.12 release cycle.

This patch simply updates the 185 output file.  The new behavior is
correct, just not optimal, so make the test pass again.

Fixes: 4486e89c219c0d1b9bd8dfa0b1dd5b0d51ff2268 ("vl: introduce vm_shutdown()")
Cc: Kevin Wolf 
Cc: QingFeng Hao 
Signed-off-by: Stefan Hajnoczi 
---
 tests/qemu-iotests/185.out | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/185.out b/tests/qemu-iotests/185.out
index 57eaf8d699..2c4b04de73 100644
--- a/tests/qemu-iotests/185.out
+++ b/tests/qemu-iotests/185.out
@@ -20,7 +20,7 @@ Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 
backing_file=TEST_DIR/t.q
 {"return": {}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 67108864, "offset": 
524288, "speed": 65536, "type": "commit"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 67108864, "offset": 
1048576, "speed": 65536, "type": "commit"}}
 
 === Start active commit job and exit qemu ===
 
@@ -28,16 +28,18 @@ Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 
backing_file=TEST_DIR/t.q
 {"return": {}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 4194304, "offset": 
4194304, "speed": 65536, "type": "commit"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_READY", "data": {"device": "disk", "len": 4194304, "offset": 
4194304, "speed": 65536, "type": "commit"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_COMPLETED", "data": {"device": "disk", "len": 4194304, "offset": 
4194304, "speed": 65536, "type": "commit"}}
 
 === Start mirror job and exit qemu ===
 
 {"return": {}}
 Formatting 'TEST_DIR/t.qcow2.copy', fmt=qcow2 size=67108864 cluster_size=65536 
lazy_refcounts=off refcount_bits=16
 {"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_READY", "data": {"device": "disk", "len": 4194304, "offset": 
4194304, "speed": 65536, "type": "mirror"}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 4194304, "offset": 
4194304, "speed": 65536, "type": "mirror"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_COMPLETED", "data": {"device": "disk", "len": 4194304, "offset": 
4194304, "speed": 65536, "type": "mirror"}}
 
 === Start backup job and exit qemu ===
 
@@ -46,7 +48,7 @@ Formatting 'TEST_DIR/t.qcow2.copy', fmt=qcow2 size=67108864 
cluster_size=65536 l
 {"return": {}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 67108864, "offset": 
65536, "speed": 65536, "type": "backup"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 67108864, "offset": 
131072, "speed": 65536, "type": "backup"}}
 
 === Start streaming job and exit qemu ===
 
@@ -54,6 +56,6 @@ Formatting 'TEST_DIR/t.qcow2.copy', fmt=qcow2 size=67108864 
cluster_size=65536 l
 {"return": {}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"SHUTDOWN", "data": {"guest": false}}
-{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 67108864, "offset": 
524288, "speed": 65536, "type": "stream"}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 67108864, "offset": 
1048576, "speed": 65536, "type": "stream"}}
 No errors were found on the image.
 *** done
-- 
2.14.3

Re: [Qemu-block] [PATCH for-2.12] gluster: Fix blockdev-add with server.N.type=unix

2018-04-03 Thread Jeff Cody 
On Tue, Apr 03, 2018 at 01:08:10PM +0200, Kevin Wolf wrote:
> The legacy command line interface gets the socket path from an option
> called 'socket'. QAPI in contract uses SocketAddress, where the
> corresponding option is called 'path'.
> 
> Fix the gluster block driver to accept both 'socket' and 'path', with
> 'path' being the preferred syntax.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1545155
> 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Kevin Wolf 
> ---
>  block/gluster.c | 21 +
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/block/gluster.c b/block/gluster.c
> index 296e036b3d..4adc1a875b 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -167,7 +167,12 @@ static QemuOptsList runtime_unix_opts = {
>  {
>  .name = GLUSTER_OPT_SOCKET,
>  .type = QEMU_OPT_STRING,
> -.help = "socket file path)",
> +.help = "socket file path (legacy)",
> +},
> +{
> +.name = GLUSTER_OPT_PATH,
> +.type = QEMU_OPT_STRING,
> +.help = "socket file path (QAPI)",
>  },
>  { /* end of list */ }
>  },
> @@ -615,10 +620,18 @@ static int 
> qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
>  goto out;
>  }
>  
> -ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);
> +ptr = qemu_opt_get(opts, GLUSTER_OPT_PATH);
> +if (!ptr) {
> +ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);
> +} else if (qemu_opt_get(opts, GLUSTER_OPT_SOCKET)) {
> +error_setg(_err,
> +   "Conflicting parameters 'path' and 'socket'");
> +error_append_hint(_err, GERR_INDEX_HINT, i);
> +goto out;
> +}
>  if (!ptr) {
>  error_setg(_err, QERR_MISSING_PARAMETER,
> -   GLUSTER_OPT_SOCKET);
> +   GLUSTER_OPT_PATH);
>  error_append_hint(_err, GERR_INDEX_HINT, i);
>  goto out;
>  }
> @@ -684,7 +697,7 @@ static int qemu_gluster_parse(BlockdevOptionsGluster 
> *gconf,
>   "file.server.0.host=1.2.3.4,"
>   "file.server.0.port=24007,"
>   "file.server.1.transport=unix,"
> - "file.server.1.socket=/var/run/glusterd.socket 
> ..."
> + "file.server.1.path=/var/run/glusterd.socket 
> ..."
>   "\n");
>  return ret;
>  }
> -- 
> 2.13.6
> 

Thanks,

Applied to my block branch:

git://github.com/codyprime/qemu-kvm-jtc block

-Jeff

Re: [Qemu-block] [PATCH for-2.12] gluster: Fix blockdev-add with server.N.type=unix

2018-04-03 Thread Jeff Cody 
On Tue, Apr 03, 2018 at 01:08:10PM +0200, Kevin Wolf wrote:
> The legacy command line interface gets the socket path from an option
> called 'socket'. QAPI in contract uses SocketAddress, where the
> corresponding option is called 'path'.
> 
> Fix the gluster block driver to accept both 'socket' and 'path', with
> 'path' being the preferred syntax.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1545155
> 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Kevin Wolf 

Reviewed-by: Jeff Cody 

> ---
>  block/gluster.c | 21 +
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/block/gluster.c b/block/gluster.c
> index 296e036b3d..4adc1a875b 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -167,7 +167,12 @@ static QemuOptsList runtime_unix_opts = {
>  {
>  .name = GLUSTER_OPT_SOCKET,
>  .type = QEMU_OPT_STRING,
> -.help = "socket file path)",
> +.help = "socket file path (legacy)",
> +},
> +{
> +.name = GLUSTER_OPT_PATH,
> +.type = QEMU_OPT_STRING,
> +.help = "socket file path (QAPI)",
>  },
>  { /* end of list */ }
>  },
> @@ -615,10 +620,18 @@ static int 
> qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
>  goto out;
>  }
>  
> -ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);
> +ptr = qemu_opt_get(opts, GLUSTER_OPT_PATH);
> +if (!ptr) {
> +ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);
> +} else if (qemu_opt_get(opts, GLUSTER_OPT_SOCKET)) {
> +error_setg(_err,
> +   "Conflicting parameters 'path' and 'socket'");
> +error_append_hint(_err, GERR_INDEX_HINT, i);
> +goto out;
> +}
>  if (!ptr) {
>  error_setg(_err, QERR_MISSING_PARAMETER,
> -   GLUSTER_OPT_SOCKET);
> +   GLUSTER_OPT_PATH);
>  error_append_hint(_err, GERR_INDEX_HINT, i);
>  goto out;
>  }
> @@ -684,7 +697,7 @@ static int qemu_gluster_parse(BlockdevOptionsGluster 
> *gconf,
>   "file.server.0.host=1.2.3.4,"
>   "file.server.0.port=24007,"
>   "file.server.1.transport=unix,"
> - "file.server.1.socket=/var/run/glusterd.socket 
> ..."
> + "file.server.1.path=/var/run/glusterd.socket 
> ..."
>   "\n");
>  return ret;
>  }
> -- 
> 2.13.6
>

Re: [Qemu-block] [PATCH 1/3] iotests.py: improve verify_image_format helper

2018-04-03 Thread Kevin Wolf 
Am 30.03.2018 um 17:16 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Add an assert (we don't want set both arguments) and remove
> duplication.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  tests/qemu-iotests/iotests.py | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index b5d7945..83c454d 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -532,9 +532,9 @@ def notrun(reason):
>  sys.exit(0)
>  
>  def verify_image_format(supported_fmts=[], unsupported_fmts=[]):
> -if supported_fmts and (imgfmt not in supported_fmts):
> -notrun('not suitable for this image format: %s' % imgfmt)
> -if unsupported_fmts and (imgfmt in unsupported_fmts):
> +assert not (supported_fmts and unsupported_fmts)
> +not_sup = supported_fmts and (imgfmt not in supported_fmts)
> +if not_sup or (imgfmt in unsupported_fmts):
>  notrun('not suitable for this image format: %s' % imgfmt)

Before the change, we accepted None for both parameters. Now None is
still accepted for supported_fmts, but not for unsupported_fmts any
more.

I don't think we actually make use of None for either, so I don't really
mind whether we allow it or not, but we should be consistent between
both parameters.

Kevin

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.12] block: handle invalid lseek returns gracefully

2018-04-03 Thread Jeff Cody 
On Tue, Apr 03, 2018 at 07:57:14AM -0500, Eric Blake wrote:
> On 04/02/2018 11:37 PM, Jeff Cody wrote:
> > In commit 223a23c198787328ae75bc65d84edf5fde33c0b6, we implemented a
> > workaround in the gluster driver to handle invalid values returned for
> > SEEK_DATA or SEEK_HOLE.
> > 
> > In some instances, these same invalid values can be seen in the posix
> > file handler as well - for example, it has been reported on FUSE gluster
> > mounts.
> 
> Yuck - that should be reported to the FUSE and gluster folks, as it does
> not scale to have everyone else work around their bug.  But in the
> meantime, working around it here is acceptable.
> 

Yes - there is a bug report on gluster still open for the lseek issue, I'll
make sure to add on it that it affects FUSE as well, so that they are aware.

-Jeff

Re: [Qemu-block] [PATCH 3/3] iotests: blacklist bochs and cloop for 205 and 208

2018-04-03 Thread Kevin Wolf 
Am 30.03.2018 um 17:16 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Blacklist these formats, as they don't support image creation, as they
> say:
> > ./qemu-img create -f bochs x 1m
> qemu-img: x: Format driver 'bochs' does not support image creation
> 
> > ./qemu-img create -f cloop x 1m
> qemu-img: x: Format driver 'cloop' does not support image creation
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 

We can take this for now, but I think I would actually prefer a solution
like in the bash tests, where the $IMGFMT_GENERIC environment variable
is checked for "_supported_fmt generic".

I suppose in Python test cases, we can assume that generic is meant when
neither supported_fmts nor unsupported_fmts are given (or both are empty
lists).

Kevin

Re: [Qemu-block] [PATCH for-2.12] block: handle invalid lseek returns gracefully

2018-04-03 Thread Kevin Wolf 
Am 03.04.2018 um 06:37 hat Jeff Cody geschrieben:
> In commit 223a23c198787328ae75bc65d84edf5fde33c0b6, we implemented a
> workaround in the gluster driver to handle invalid values returned for
> SEEK_DATA or SEEK_HOLE.
> 
> In some instances, these same invalid values can be seen in the posix
> file handler as well - for example, it has been reported on FUSE gluster
> mounts.
> 
> Calling assert() for these invalid values is overly harsh; we can safely
> return -EIO and allow this case to be treated as a "learned nothing"
> case (e.g., D4 / H4, as commented in the code).
> 
> This patch does the same thing that 223a23c198787 did for gluster.c,
> except in file-posix.c
> 
> Signed-off-by: Jeff Cody 

Thanks, applied to the block branch.

Kevin

Re: [Qemu-block] [PATCH for-2.12] gluster: Fix blockdev-add with server.N.type=unix

2018-04-03 Thread Eric Blake 
On 04/03/2018 06:08 AM, Kevin Wolf wrote:
> The legacy command line interface gets the socket path from an option
> called 'socket'. QAPI in contract uses SocketAddress, where the
> corresponding option is called 'path'.
> 
> Fix the gluster block driver to accept both 'socket' and 'path', with
> 'path' being the preferred syntax.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1545155
> 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Kevin Wolf 
> ---
>  block/gluster.c | 21 +
>  1 file changed, 17 insertions(+), 4 deletions(-)

Reviewed-by: Eric Blake 

Should we also add a deprecation warning for 'socket' and update the
deprecation documentation, so we can start the clock ticking on getting
rid of maintaining the back-compat glue forever?

> @@ -615,10 +620,18 @@ static int 
> qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
>  goto out;
>  }
>  
> -ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);
> +ptr = qemu_opt_get(opts, GLUSTER_OPT_PATH);
> +if (!ptr) {
> +ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);

Here's where we'd warn, if we want to deprecate things.

> +} else if (qemu_opt_get(opts, GLUSTER_OPT_SOCKET)) {
> +error_setg(_err,
> +   "Conflicting parameters 'path' and 'socket'");
> +error_append_hint(_err, GERR_INDEX_HINT, i);
> +goto out;
> +}
>  if (!ptr) {

-- 
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-block] [Qemu-devel] [PATCH for-2.12] block: handle invalid lseek returns gracefully

2018-04-03 Thread Eric Blake 
On 04/02/2018 11:37 PM, Jeff Cody wrote:
> In commit 223a23c198787328ae75bc65d84edf5fde33c0b6, we implemented a
> workaround in the gluster driver to handle invalid values returned for
> SEEK_DATA or SEEK_HOLE.
> 
> In some instances, these same invalid values can be seen in the posix
> file handler as well - for example, it has been reported on FUSE gluster
> mounts.

Yuck - that should be reported to the FUSE and gluster folks, as it does
not scale to have everyone else work around their bug.  But in the
meantime, working around it here is acceptable.

> 
> Calling assert() for these invalid values is overly harsh; we can safely
> return -EIO and allow this case to be treated as a "learned nothing"
> case (e.g., D4 / H4, as commented in the code).
> 
> This patch does the same thing that 223a23c198787 did for gluster.c,
> except in file-posix.c
> 
> Signed-off-by: Jeff Cody 
> ---
>  block/file-posix.c | 14 --
>  1 file changed, 12 insertions(+), 2 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-block] [PATCH] block: remove bdrv_dirty_bitmap_make_anon

2018-04-03 Thread Stefan Hajnoczi 
On Fri, Mar 23, 2018 at 05:42:53PM +0100, Paolo Bonzini wrote:
> All this function is doing will be repeated by
> bdrv_do_release_matching_dirty_bitmap_locked, except
> resetting bm->persistent.  But even that does not matter
> because the bitmap will be freed.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  block/dirty-bitmap.c | 9 -
>  blockdev.c   | 1 -
>  include/block/dirty-bitmap.h | 1 -
>  3 files changed, 11 deletions(-)

CCing Fam and John, who maintain block/dirty-bitmap.c.

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-block] [PATCH v2 1/1] qemu-iotests: Use ppc64 qemu_arch on ppc64le host

2018-04-03 Thread Kevin Wolf 
Am 29.03.2018 um 16:31 hat Max Reitz geschrieben:
> On 2018-03-29 13:20, Lukáš Doktor wrote:
> > The qemu target does not always correspond to the host machine type. For
> > example ppc64le machine target is ppc64. Let's introduce "qemu_arch"
> > variable to store the matching qemu architecture related to the current
> > architecture and use it when auto-detecting the default qemu binary.
> > 
> > Signed-off-by: Lukáš Doktor 
> > ---
> >  tests/qemu-iotests/check | 4 ++--
> >  tests/qemu-iotests/common.config | 1 +
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> Thanks, applied to my block branch:
> 
> https://github.com/XanClic/qemu/commits/block

Please don't forget to CC qemu-block and qemu-devel for patches to
qemu-iotests (the kvm list isn't really necessary, though). Adding it to
this reply so that people can see that something was merged.

Kevin

[Qemu-block] [PATCH for-2.12] gluster: Fix blockdev-add with server.N.type=unix

2018-04-03 Thread Kevin Wolf 
The legacy command line interface gets the socket path from an option
called 'socket'. QAPI in contract uses SocketAddress, where the
corresponding option is called 'path'.

Fix the gluster block driver to accept both 'socket' and 'path', with
'path' being the preferred syntax.

https://bugzilla.redhat.com/show_bug.cgi?id=1545155

Cc: qemu-sta...@nongnu.org
Signed-off-by: Kevin Wolf 
---
 block/gluster.c | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index 296e036b3d..4adc1a875b 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -167,7 +167,12 @@ static QemuOptsList runtime_unix_opts = {
 {
 .name = GLUSTER_OPT_SOCKET,
 .type = QEMU_OPT_STRING,
-.help = "socket file path)",
+.help = "socket file path (legacy)",
+},
+{
+.name = GLUSTER_OPT_PATH,
+.type = QEMU_OPT_STRING,
+.help = "socket file path (QAPI)",
 },
 { /* end of list */ }
 },
@@ -615,10 +620,18 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster 
*gconf,
 goto out;
 }
 
-ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);
+ptr = qemu_opt_get(opts, GLUSTER_OPT_PATH);
+if (!ptr) {
+ptr = qemu_opt_get(opts, GLUSTER_OPT_SOCKET);
+} else if (qemu_opt_get(opts, GLUSTER_OPT_SOCKET)) {
+error_setg(_err,
+   "Conflicting parameters 'path' and 'socket'");
+error_append_hint(_err, GERR_INDEX_HINT, i);
+goto out;
+}
 if (!ptr) {
 error_setg(_err, QERR_MISSING_PARAMETER,
-   GLUSTER_OPT_SOCKET);
+   GLUSTER_OPT_PATH);
 error_append_hint(_err, GERR_INDEX_HINT, i);
 goto out;
 }
@@ -684,7 +697,7 @@ static int qemu_gluster_parse(BlockdevOptionsGluster *gconf,
  "file.server.0.host=1.2.3.4,"
  "file.server.0.port=24007,"
  "file.server.1.transport=unix,"
- "file.server.1.socket=/var/run/glusterd.socket 
..."
+ "file.server.1.path=/var/run/glusterd.socket ..."
  "\n");
 return ret;
 }
-- 
2.13.6