[Qemu-block] [PATCH 2/2] HACK: define NBD_SERVER_DEBUG to force malicious server

[Eric Blake]

---
 nbd/server.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/nbd/server.c b/nbd/server.c
index 82a78bf439..d6fbd46370 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -919,6 +919,17 @@ static int nbd_send_reply(QIOChannel *ioc, NBDReply 
*reply, Error **errp)
 stl_be_p(buf + 4, reply->error);
 stq_be_p(buf + 8, reply->handle);

+static int debug;
+static int count;
+if (!count++) {
+const char *str = getenv("NBD_SERVER_DEBUG");
+if (str) {
+debug = atoi(str);
+}
+}
+if (debug && !(count % debug)) {
+buf[0] = 0;
+}
 return nbd_write(ioc, buf, sizeof(buf), errp);
 }

-- 
2.13.4

[Qemu-block] [PATCH 1/2] nbd: Drop connection if broken server is detected

[Eric Blake]

As soon as the server is sending us garbage, we should quit
trying to send further messages to the server, and allow all
pending coroutines for any remaining replies to error out.
Failure to do so can let a malicious server cause the client
to hang, for example, if the server sends an invalid magic
number in its response.

Reported by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Eric Blake 
---
 block/nbd-client.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 25dd28406b..802d50b636 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -68,7 +68,8 @@ static void nbd_teardown_connection(BlockDriverState *bs)

 static coroutine_fn void nbd_read_reply_entry(void *opaque)
 {
-NBDClientSession *s = opaque;
+BlockDriverState *bs = opaque;
+NBDClientSession *s = nbd_get_client_session(bs);
 uint64_t i;
 int ret;
 Error *local_err = NULL;
@@ -107,8 +108,12 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
 qemu_coroutine_yield();
 }

+s->reply.handle = 0;
 nbd_recv_coroutines_enter_all(s);
 s->read_reply_co = NULL;
+if (ret < 0) {
+nbd_teardown_connection(bs);
+}
 }

 static int nbd_co_send_request(BlockDriverState *bs,
@@ -416,7 +421,7 @@ int nbd_client_init(BlockDriverState *bs,
 /* Now that we're connected, set the socket to be non-blocking and
  * kick the reply mechanism.  */
 qio_channel_set_blocking(QIO_CHANNEL(sioc), false, NULL);
-client->read_reply_co = qemu_coroutine_create(nbd_read_reply_entry, 
client);
+client->read_reply_co = qemu_coroutine_create(nbd_read_reply_entry, bs);
 nbd_client_attach_aio_context(bs, bdrv_get_aio_context(bs));

 logout("Established connection with NBD server\n");
-- 
2.13.4

[Qemu-block] [PATCH] nbd: Fix trace message for disconnect

[Eric Blake]

NBD_CMD_DISC is a disconnect request, not a data discard request.

Signed-off-by: Eric Blake 
---

Although this is not 2.10 material in isolation (it is only a
bad trace message), I don't mind including it in a larger pull
request; I'm still planning to fix the issue of a client hanging
on a malicious server in time for -rc3 (whether via Vladimir's
patch or a smaller one that I'm testing locally).

 nbd/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nbd/common.c b/nbd/common.c
index a2f28f2eec..e288d1b972 100644
--- a/nbd/common.c
+++ b/nbd/common.c
@@ -182,7 +182,7 @@ const char *nbd_cmd_lookup(uint16_t cmd)
 case NBD_CMD_WRITE:
 return "write";
 case NBD_CMD_DISC:
-return "discard";
+return "disconnect";
 case NBD_CMD_FLUSH:
 return "flush";
 case NBD_CMD_TRIM:
-- 
2.13.4

Re: [Qemu-block] [Qemu-devel] block replication

[Xie Changlong]

在 8/10/2017 8:26 PM, Vladimir Sementsov-Ogievskiy 写道:

09.08.2017 17:11, Vladimir Sementsov-Ogievskiy wrote:

Hi Wen!

I'm trying to understand block/replication code and have a question.

Why should we block the region from intersecting cow requests when 
read? If I understand correctly


regardless of writes to the secondary-disk we have consistent view of 
it through hidden-disk:


Even if we are intersecting with some writes to secondary-disk (and 
corresponding cow-requests), the


data in secondary disk will not be updated until backed up to 
hidden-disk, therefore, for read we have two


options:

1. read old data from secondary-disk (unallocated region in 
hidden-disk means data in secondary-disk is not updated yet)


2. read backed-up data from hidden-disk (data in secondary-disk may be 
already updated but we don't care)


(the whole region to read may consists of parts, corresponding to 1 or 
2, but this should be ok too)


Where am I wrong?


Ok, now I think this is needed to prevent intersecting of writes and 
reads on hidden-disk. If it so, I think it is better to use serializing 


Hi, Vladimir. Pls refer 
https://lists.nongnu.org/archive/html/qemu-devel/2016-05/msg03025.html


BTW, wen's email has changed, also CC Zhang Hailiang, Zhang Chen, Li Zhijian


requests
mechanism (just serialize all requests on hidden-disk, and on write wait 
for all intersecting serializing requests, on read wait for intersecting 
serializing writes) - it may require additional
option for BlockDriverState, but it is more generic and more clear than 
export internal backup things to lock disk region. This also can be 
reused for image-fleecing scheme
(which is based on same pattern  [active-disk is backing for temp-disk, 
backup sync=none from active to temp, read from temp])





==

static coroutine_fn int replication_co_readv(BlockDriverState *bs,
 int64_t sector_num,
 int remaining_sectors,
 QEMUIOVector *qiov)
{
BDRVReplicationState *s = bs->opaque;
BdrvChild *child = s->secondary_disk;
BlockJob *job = NULL;
CowRequest req;
int ret;

if (s->mode == REPLICATION_MODE_PRIMARY) {
/* We only use it to forward primary write requests */
return -EIO;
}

ret = replication_get_io_status(s);
if (ret < 0) {
return ret;
}

if (child && child->bs) {
job = child->bs->job;
}

if (job) {
uint64_t remaining_bytes = remaining_sectors * BDRV_SECTOR_SIZE;

backup_wait_for_overlapping_requests(child->bs->job,
 sector_num * 
BDRV_SECTOR_SIZE,

remaining_bytes);
backup_cow_request_begin(, child->bs->job,
 sector_num * BDRV_SECTOR_SIZE,
remaining_bytes);
ret = bdrv_co_readv(bs->file, sector_num, remaining_sectors,
qiov);
backup_cow_request_end();
goto out;
}

ret = bdrv_co_readv(bs->file, sector_num, remaining_sectors, qiov);
out:
return replication_return_value(s, ret);
}





--
Thanks
-Xie

[Qemu-block] Block warning messages displayed while compiling

[Programmingkid]

Host info:
Operating system: Mac OS 10.12.5
Compiler: Apple LLVM version 8.1.0 (clang-802.0.42)
Command to reproduce: ./configure --target-list=ppc-softmmu,i386-softmmu && 
make -j 4

When compiling QEMU I see these warning messages:


  CC  block/vdi.o
block/qcow.c:138:19: warning: taking address of packed member 'magic' of class 
or
  structure 'QCowHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
be32_to_cpus();
  ^~~~
block/qcow.c:139:19: warning: taking address of packed member 'version' of 
class or
  structure 'QCowHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
be32_to_cpus();
  ^~
block/qcow.c:140:19: warning: taking address of packed member 
'backing_file_offset' of
  class or structure 'QCowHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
be64_to_cpus(_file_offset);
  ^~
block/qcow.c:141:19: warning: taking address of packed member 
'backing_file_size' of
  class or structure 'QCowHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
be32_to_cpus(_file_size);
  ^~~~
block/qcow.c:142:19: warning: taking address of packed member 'mtime' of class 
or
  structure 'QCowHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
be32_to_cpus();
  ^~~~
block/qcow.c:143:19: warning: taking address of packed member 'size' of class or
  structure 'QCowHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
be64_to_cpus();
  ^~~
block/qcow.c:144:19: warning: taking address of packed member 'crypt_method' of 
class or
  structure 'QCowHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
be32_to_cpus(_method);
  ^~~
block/qcow.c:145:19: warning: taking address of packed member 'l1_table_offset' 
of class
  or structure 'QCowHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
be64_to_cpus(_table_offset);
  ^~
block/vdi.c:182:19: warning: taking address of packed member 'signature' of 
class or
  structure 'VdiHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
le32_to_cpus(>signature);
  ^
block/vdi.c:183:19: warning: taking address of packed member 'version' of class 
or
  structure 'VdiHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
le32_to_cpus(>version);
  ^~~
block/vdi.c:184:19: warning: taking address of packed member 'header_size' of 
class or
  structure 'VdiHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
le32_to_cpus(>header_size);
  ^~~
block/vdi.c:185:19: warning: taking address of packed member 'image_type' of 
class or
  structure 'VdiHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
le32_to_cpus(>image_type);
  ^~
block/vdi.c:186:19: warning: taking address of packed member 'image_flags' of 
class or
  structure 'VdiHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
le32_to_cpus(>image_flags);
  ^~~
block/vdi.c:187:19: warning: taking address of packed member 'offset_bmap' of 
class or
  structure 'VdiHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
le32_to_cpus(>offset_bmap);
  ^~~
block/vdi.c:188:19: warning: taking address of packed member 'offset_data' of 
class or
  structure 'VdiHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
le32_to_cpus(>offset_data);
  ^~~
block/vdi.c:189:19: warning: taking address of packed member 'cylinders' of 
class or
  structure 'VdiHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
le32_to_cpus(>cylinders);
  ^
block/vdi.c:190:19: warning: taking address of packed member 'heads' of class or
  structure 'VdiHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
le32_to_cpus(>heads);
  ^
block/vdi.c:191:19: warning: taking address of packed member 'sectors' of class 
or
  structure 'VdiHeader' may result in an unaligned pointer value
  [-Waddress-of-packed-member]
le32_to_cpus(>sectors);
  ^~~
block/vdi.c:192:19: warning: taking address of packed member 'sector_size' of 
class or
  structure 'VdiHeader' may result in an 

Re: [Qemu-block] [PATCH 8/8] sockets: fix parsing of ipv4/ipv6 opts in parse_socket_addr

[Eric Blake]

On 08/10/2017 11:04 AM, Daniel P. Berrange wrote:
> The inet_parse() function looks for 'ipv4' and 'ipv6'
> flags, but only treats them as bare bool flags. The normal
> QemuOpts parsing would allow on/off values to be set too.
> 
> This updated inet_parse() so that its handling of the

s/updated/updates/ ?

> 'ipv4' and 'ipv6' flags matches that done by QemuOpts.

Do we have a regression compared to any previous version, such that this
patch might be considered 2.10 material?  Offhand, though, I think it's
fine as the end of your series, waiting for 2.11.

> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  tests/test-sockets-proto.c | 13 -
>  util/qemu-sockets.c| 36 
>  2 files changed, 32 insertions(+), 17 deletions(-)
> 

> +++ b/util/qemu-sockets.c
> @@ -616,6 +616,25 @@ err:
>  }
>  
>  /* compatibility wrapper */
> +static int inet_parse_flag(const char *flagname, const char *optstr, bool 
> *val,
> +   Error **errp)
> +{
> +char *end;
> +size_t len;
> +
> +end = strstr(optstr, ",");

Do we need to check that we are not landing on a ',,' escape that would
make QemuOpts behave differently?  [That is, ipv4=on,,garbage should be
parsed as setting ipv4 to 'on,garbage' (which is not valid), and NOT
setting 'ipv4=on' followed by the 'garbage' or ',garbage' key - while
the key named 'garbage' would fail, there might be other key names where
the distinction matters for catching command line typos]

But if this is unrelated to QemuOpts escape parsing, it seems okay.

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 4/8] blockdev: convert qemu-nbd server to QIONetListener

[Eric Blake]

On 08/10/2017 11:04 AM, Daniel P. Berrange wrote:
> Instead of creating a QIOChannelSocket directly for the NBD
> server socket, use a QIONetListener. This provides the ability
> to listen on multiple sockets at the same time, so enables
> full support for IPv4/IPv6 dual stack.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  qemu-nbd.c | 50 +-
>  1 file changed, 17 insertions(+), 33 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 3/8] blockdev: convert internal NBD server to QIONetListener

[Eric Blake]

On 08/10/2017 11:04 AM, Daniel P. Berrange wrote:
> Instead of creating a QIOChannelSocket directly for the NBD
> server socket, use a QIONetListener. This provides the ability
> to listen on multiple sockets at the same time, so enables
> full support for IPv4/IPv6 dual stack.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  blockdev-nbd.c | 50 --
>  1 file changed, 16 insertions(+), 34 deletions(-)

May need rebasing on top of other pending NBD cleanups; we'll see how
that plays out.

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 2/8] io: introduce a network socket listener API

[Eric Blake]

On 08/10/2017 11:04 AM, Daniel P. Berrange wrote:
> The existing QIOChannelSocket class provides the ability to
> listen on a single socket at a time. This patch introduces
> a QIONetListener class that provides a higher level API
> concept around listening for network services, allowing
> for listening on multiple sockets.
> 
> Signed-off-by: Daniel P. Berrange 
> ---

> +++ b/include/io/net-listener.h
> @@ -0,0 +1,174 @@
> +/*
> + * QEMU I/O network listener
> + *
> + * Copyright (c) 2016 Red Hat, Inc.

Want to add 2017?

At least it's covered by MAINTAINERS :)


> +/**
> + * qio_net_listener_is_disconnected:
> + * @listener: the network listener object
> + *
> + * Determine if the listener is connected to any socket
> + * channels
> + *
> + * Returns: TRUE if connected, FALSE otherwise
> + */
> +gboolean qio_net_listener_is_disconnected(QIONetListener *listener);
> +

Must it return gboolean, or is bool sufficient?

TRUE if connected for a function named 'is_disconnected' sounds
backwards.  Avoid the double negative, name it:

qio_net_listener_is_connected(), returning true if connected

> +++ b/io/net-listener.c
> @@ -0,0 +1,315 @@
> +/*
> + * QEMU network listener
> + *
> + * Copyright (c) 2016 Red Hat, Inc.

More 2017.  Probably for the whole series :)


> +static gboolean qio_net_listener_channel_func(QIOChannel *ioc,
> +  GIOCondition condition,
> +  gpointer opaque)
> +{

Again, can we use bool instead of gboolean?

> +int qio_net_listener_open_sync(QIONetListener *listener,
> +   SocketAddress *addr,
> +   Error **errp)
> +{
> +QIODNSResolver *resolver = qio_dns_resolver_get_instance();
> +SocketAddress **resaddrs;
> +size_t nresaddrs;
> +size_t i;
> +Error *err = NULL;
> +bool success = false;
> +
> +if (qio_dns_resolver_lookup_sync(resolver,
> + addr,
> + ,
> + ,
> + errp) < 0) {
> +return -1;
> +}
> +
> +for (i = 0; i < nresaddrs; i++) {
> +QIOChannelSocket *sioc = qio_channel_socket_new();
> +
> +if (qio_channel_socket_listen_sync(sioc, resaddrs[i],
> +   err ? NULL : ) == 0) {
> +success = true;
> +}

This says that as long as at least one address connected, we are
successful...

> +
> +qio_net_listener_add(listener, sioc);

...but this adds sioc as a listener regardless of whether listen_sync()
succeeded.  Is that right?


> +gboolean qio_net_listener_is_disconnected(QIONetListener *listener)
> +{
> +return listener->disconnected;

Documentation says it returns true on connected, but here you are
returning true on disconnected?

-- 
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 v8 0/8] Optimize VMDK I/O by allocating multiple clusters

[Stefan Hajnoczi]

On Thu, Aug 10, 2017 at 9:18 AM, Ashijeet Acharya
 wrote:
> On Thu, Aug 10, 2017 at 1:41 PM, Stefan Hajnoczi  wrote:
>>
>> On Thu, Jul 27, 2017 at 3:33 PM, Ashijeet Acharya
>>  wrote:
>> > Previously posted series patches:
>> > v1 -
>> > http://lists.nongnu.org/archive/html/qemu-devel/2017-03/msg02044.html
>> > v2 -
>> > http://lists.nongnu.org/archive/html/qemu-devel/2017-03/msg05080.html
>> > v3 -
>> > http://lists.nongnu.org/archive/html/qemu-devel/2017-04/msg00074.html
>> > v4 -
>> > http://lists.nongnu.org/archive/html/qemu-devel/2017-04/msg03851.html
>> > v5 -
>> > http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg00929.html
>> > v6 -
>> > http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg00947.html
>> > v7 -
>> > http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg06600.html
>> >
>> > This series helps to optimize the I/O performance of VMDK driver.
>> >
>> > Patch 1 helps us to move vmdk_find_offset_in_cluster.
>> >
>> > Patch 2 & 3 perform a simple function re-naming tasks.
>> >
>> > Patch 4 is used to factor out metadata loading code and implement it in
>> > separate
>> > functions. This will help us to avoid code duplication in future patches
>> > of this
>> > series.
>> >
>> > Patch 5 helps to set the upper limit of the bytes handled in one cycle.
>> >
>> > Patch 6 adds new functions to help us allocate multiple clusters
>> > according to
>> > the size requested, perform COW if required and return the offset of the
>> > first
>> > newly allocated cluster.
>> >
>> > Patch 7 changes the metadata update code to update the L2 tables for
>> > multiple
>> > clusters at once.
>> >
>> > Patch 8 helps us to finally change vmdk_get_cluster_offset() to find
>> > cluster
>> > offset only as cluster allocation task is now handled by
>> > vmdk_alloc_clusters()
>> >
>> > Optimization test results:
>> >
>> > This patch series improves 128 KB sequential write performance to an
>> > empty VMDK file by 54%
>> >
>> > Benchmark command: ./qemu-img bench -w -c 1024 -s 128K -d 1 -t none -f
>> > vmdk test.vmdk
>> >
>> > Changes in v8:
>> > - fix minor variable naming issue in patch 6
>>
>> Fam: Ping?
>>
>> Ashijeet: Feel free to send a ping reply if no one reviews your
>> patches within a few days.
>
>
> Hi Stefan,
>
> I had a chat with Fam on #qemu-block before submitting this series and he
> said he will be merging it soon when the freeze is over (I am not sure if it
> is yet) since all the patches are already reviewed :-)

Good to hear :).

QEMU 2.10 is scheduled to be released on 22nd or 29th of August.

Stefan

Re: [Qemu-block] [Qemu-devel] [PATCH 0/8] Enable full IPv4/IPv6 dual stack support

[no-reply]

Hi,

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

Message-id: 20170810160451.32723-1-berra...@redhat.com
Subject: [Qemu-devel] [PATCH 0/8] Enable full IPv4/IPv6 dual stack support
Type: series

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

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

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

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
c80d4b3d99 sockets: fix parsing of ipv4/ipv6 opts in parse_socket_addr
85193a29c1 ui: convert VNC server to QIONetListener
59dd1813c8 chardev: convert the socket server to QIONetListener
2304ec200f migration: convert socket server to QIONetListener
4f4c339d2c blockdev: convert qemu-nbd server to QIONetListener
9837cc0ccd blockdev: convert internal NBD server to QIONetListener
f3aa1d82c5 io: introduce a network socket listener API
81584cdc6f tests: add functional test validating ipv4/ipv6 address flag handling

=== OUTPUT BEGIN ===
Checking PATCH 1/8: tests: add functional test validating ipv4/ipv6 address 
flag handling...
Checking PATCH 2/8: io: introduce a network socket listener API...
Checking PATCH 3/8: blockdev: convert internal NBD server to QIONetListener...
Checking PATCH 4/8: blockdev: convert qemu-nbd server to QIONetListener...
WARNING: line over 80 characters
#41: FILE: qemu-nbd.c:348:
+static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, 
gpointer opaque)

total: 0 errors, 1 warnings, 99 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 5/8: migration: convert socket server to QIONetListener...
ERROR: line over 90 characters
#89: FILE: migration/socket.c:163:
+qio_net_listener_set_client_func(listener, 
socket_accept_incoming_migration, NULL, NULL);

total: 1 errors, 0 warnings, 82 lines checked

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

Checking PATCH 6/8: chardev: convert the socket server to QIONetListener...
WARNING: line over 80 characters
#57: FILE: chardev/char-socket.c:408:
+qio_net_listener_set_client_func(s->listener, tcp_chr_accept, chr, 
NULL);

WARNING: line over 80 characters
#165: FILE: chardev/char-socket.c:946:
+qio_net_listener_set_client_func(s->listener, tcp_chr_accept, 
chr, NULL);

total: 0 errors, 2 warnings, 168 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 7/8: ui: convert VNC server to QIONetListener...
WARNING: line over 80 characters
#51: FILE: ui/vnc.c:390:
+addr = qio_channel_socket_get_local_address(vd->listener->sioc[0], 
errp);

total: 0 errors, 1 warnings, 292 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 8/8: sockets: fix parsing of ipv4/ipv6 opts in 
parse_socket_addr...
=== OUTPUT END ===

Test command exited with code: 1


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

Re: [Qemu-block] [Qemu-devel] [PATCH 0/8] Enable full IPv4/IPv6 dual stack support

[no-reply]

Hi,

This series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Message-id: 20170810160451.32723-1-berra...@redhat.com
Subject: [Qemu-devel] [PATCH 0/8] Enable full IPv4/IPv6 dual stack support
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-quick@centos6
time make docker-test-build@min-glib
time make docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]   patchew/20170810160451.32723-1-berra...@redhat.com 
-> patchew/20170810160451.32723-1-berra...@redhat.com
Switched to a new branch 'test'
c80d4b3d99 sockets: fix parsing of ipv4/ipv6 opts in parse_socket_addr
85193a29c1 ui: convert VNC server to QIONetListener
59dd1813c8 chardev: convert the socket server to QIONetListener
2304ec200f migration: convert socket server to QIONetListener
4f4c339d2c blockdev: convert qemu-nbd server to QIONetListener
9837cc0ccd blockdev: convert internal NBD server to QIONetListener
f3aa1d82c5 io: introduce a network socket listener API
81584cdc6f tests: add functional test validating ipv4/ipv6 address flag handling

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-7o1dgdzo/src/dtc'...
Submodule path 'dtc': checked out '558cd81bdd432769b59bff01240c44f82cfb1a9d'
  BUILD   centos6
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-7o1dgdzo/src'
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPYRUNNER
RUN test-quick in qemu:centos6 
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
bison-2.4.1-5.el6.x86_64
ccache-3.1.6-2.el6.x86_64
epel-release-6-8.noarch
flex-2.5.35-9.el6.x86_64
gcc-4.4.7-18.el6.x86_64
git-1.7.1-8.el6.x86_64
glib2-devel-2.28.8-9.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
make-3.81-23.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
tar-1.23-15.el6_8.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=libfdt-devel ccache tar git make gcc g++ flex bison zlib-devel 
glib2-devel SDL-devel pixman-devel epel-release
HOSTNAME=638a1a17597e
TERM=xterm
MAKEFLAGS= -j8
HISTSIZE=1000
J=8
USER=root
LS_COLORS=rs=0:di=01;34:ln=01;36:mh=00:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:mi=01;05;37;41:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.tar=01;31:*.tgz=01;31:*.arj=01;31:*.taz=01;31:*.lzh=01;31:*.lzma=01;31:*.tlz=01;31:*.txz=01;31:*.zip=01;31:*.z=01;31:*.Z=01;31:*.dz=01;31:*.gz=01;31:*.lz=01;31:*.xz=01;31:*.bz2=01;31:*.tbz=01;31:*.tbz2=01;31:*.bz=01;31:*.tz=01;31:*.deb=01;31:*.rpm=01;31:*.jar=01;31:*.rar=01;31:*.ace=01;31:*.zoo=01;31:*.cpio=01;31:*.7z=01;31:*.rz=01;31:*.jpg=01;35:*.jpeg=01;35:*.gif=01;35:*.bmp=01;35:*.pbm=01;35:*.pgm=01;35:*.ppm=01;35:*.tga=01;35:*.xbm=01;35:*.xpm=01;35:*.tif=01;35:*.tiff=01;35:*.png=01;35:*.svg=01;35:*.svgz=01;35:*.mng=01;35:*.pcx=01;35:*.mov=01;35:*.mpg=01;35:*.mpeg=01;35:*.m2v=01;35:*.mkv=01;35:*.ogm=01;35:*.mp4=01;35:*.m4v=01;35:*.mp4v=01;35:*.vob=01;35:*.qt=01;35:*.nuv=01;35:*.wmv=01;35:*.asf=01;35:*.rm=01;35:*.rmvb=01;35:*.flc=01;35:*.avi=01;35:*.fli=01;35:*.flv=01;35:*.gl=01;35:*.dl=01;35:*.xcf=01;35:*.xwd=01;35:*.yuv=01;35:*.cgm=01;35:*.emf=01;35:*.axv=01;35:*.anx=01;35:*.ogv=01;35:*.ogx=01;35:*.aac=01;36:*.au=01;36:*.flac=01;36:*.mid=01;36:*.midi=01;36:*.mka=01;36:*.mp3=01;36:*.mpc=01;36:*.ogg=01;36:*.ra=01;36:*.wav=01;36:*.axa=01;36:*.oga=01;36:*.spx=01;36:*.xspf=01;36:
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
MAIL=/var/spool/mail/root
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
LANG=en_US.UTF-8
TARGET_LIST=
HISTCONTROL=ignoredups
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
LOGNAME=root
LESSOPEN=||/usr/bin/lesspipe.sh %s
FEATURES= dtc
DEBUG=
G_BROKEN_FILENAMES=1
CCACHE_HASHDIR=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu 
--prefix=/var/tmp/qemu-build/install
No C++ compiler available; disabling C++ specific optional code
Install prefix/var/tmp/qemu-build/install
BIOS directory/var/tmp/qemu-build/install/share/qemu
binary directory  /var/tmp/qemu-build/install/bin
library directory /var/tmp/qemu-build/install/lib
module directory  /var/tmp/qemu-build/install/lib/qemu
libexec directory /var/tmp/qemu-build/install/libexec
include directory /var/tmp/qemu-build/install/include
config directory  /var/tmp/qemu-build/install/etc
local state directory   /var/tmp/qemu-build/install/var
Manual directory  /var/tmp/qemu-build/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /tmp/qemu-test/src
C compilercc
Host C compiler   cc
C++ compiler  
Objective-C compiler cc

[Qemu-block] [PATCH 8/8] sockets: fix parsing of ipv4/ipv6 opts in parse_socket_addr

[Daniel P. Berrange]

The inet_parse() function looks for 'ipv4' and 'ipv6'
flags, but only treats them as bare bool flags. The normal
QemuOpts parsing would allow on/off values to be set too.

This updated inet_parse() so that its handling of the
'ipv4' and 'ipv6' flags matches that done by QemuOpts.

Signed-off-by: Daniel P. Berrange 
---
 tests/test-sockets-proto.c | 13 -
 util/qemu-sockets.c| 36 
 2 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/tests/test-sockets-proto.c b/tests/test-sockets-proto.c
index a92389bef6..5805d2be5f 100644
--- a/tests/test-sockets-proto.c
+++ b/tests/test-sockets-proto.c
@@ -69,7 +69,6 @@ typedef struct {
  */
 static QSocketsData test_data[] = {
 /* Migrate with "" address */
-/* XXX all settings with =off are disabled due to inet_parse() bug */
 { .ipv4 = 1, .ipv6 = 1, .error = false,
   .name = "/sockets/migrate/wildcard/all",
   .args = "-incoming tcp::9000" },
@@ -85,7 +84,6 @@ static QSocketsData test_data[] = {
 { .ipv4 = 0, .ipv6 = 1, .error = false,
   .name = "/sockets/migrate/wildcard/ipv6on",
   .args = "-incoming tcp::9000,ipv6=on" },
-/*
 { .ipv4 = 0, .ipv6 = 1, .error = false,
   .name = "/sockets/migrate/wildcard/ipv4off",
   .args = "-incoming tcp::9000,ipv4=off" },
@@ -98,15 +96,12 @@ static QSocketsData test_data[] = {
 { .ipv4 = 0, .ipv6 = 1, .error = false,
   .name = "/sockets/migrate/wildcard/ipv4offipv6on",
   .args = "-incoming tcp::9000,ipv4=off,ipv6=on" },
-*/
 { .ipv4 = 1, .ipv6 = 1, .error = false,
   .name = "/sockets/migrate/wildcard/ipv4onipv6on",
   .args = "-incoming tcp::9000,ipv4=on,ipv6=on" },
-/*
 { .ipv4 = 0, .ipv6 = 0, .error = true,
   .name = "/sockets/migrate/wildcard/ipv4offipv6off",
   .args = "-incoming tcp::9000,ipv4=off,ipv6=off" },
-*/
 
 /* Migrate with 0.0.0.0 address */
 { .ipv4 = 1, .ipv6 = 0, .error = false,
@@ -124,7 +119,6 @@ static QSocketsData test_data[] = {
 { .ipv4 = 0, .ipv6 = 0, .error = true,
   .name = "/sockets/migrate/0.0.0.0/ipv6on",
   .args = "-incoming tcp:0.0.0.0:9000,ipv6=on" },
-/*
 { .ipv4 = 0, .ipv6 = 0, .error = true,
   .name = "/sockets/migrate/0.0.0.0/ipv4off",
   .args = "-incoming tcp:0.0.0.0:9000,ipv4=off" },
@@ -137,15 +131,12 @@ static QSocketsData test_data[] = {
 { .ipv4 = 0, .ipv6 = 0, .error = true,
   .name = "/sockets/migrate/0.0.0.0/ipv4offipv6on",
   .args = "-incoming tcp:0.0.0.0:9000,ipv4=off,ipv6=on" },
-*/
 { .ipv4 = 1, .ipv6 = 0, .error = false,
   .name = "/sockets/migrate/0.0.0.0/ipv4onipv6on",
   .args = "-incoming tcp:0.0.0.0:9000,ipv4=on,ipv6=on" },
-/*
 { .ipv4 = 0, .ipv6 = 0, .error = true,
   .name = "/sockets/migrate/0.0.0.0/ipv4offipv6off",
   .args = "-incoming tcp:0.0.0.0:9000,ipv4=off,ipv6=off" },
-*/
 
 /* Migrate with :: address */
 { .ipv4 = 1, .ipv6 = 1, .error = false,
@@ -163,7 +154,6 @@ static QSocketsData test_data[] = {
 { .ipv4 = 0, .ipv6 = 1, .error = false,
   .name = "/sockets/migrate/::/ipv6on",
   .args = "-incoming tcp:[::]:9000,ipv6=on" },
-/*
 { .ipv4 = 0, .ipv6 = 1, .error = false,
   .name = "/sockets/migrate/::/ipv4off",
   .args = "-incoming tcp:[::]:9000,ipv4=off" },
@@ -176,15 +166,12 @@ static QSocketsData test_data[] = {
 { .ipv4 = 0, .ipv6 = 1, .error = false,
   .name = "/sockets/migrate/::/ipv4offipv6on",
   .args = "-incoming tcp:[::]:9000,ipv4=off,ipv6=on" },
-*/
 { .ipv4 = 1, .ipv6 = 1, .error = false,
   .name = "/sockets/migrate/::/ipv4onipv6on",
   .args = "-incoming tcp:[::]:9000,ipv4=on,ipv6=on" },
-/*
 { .ipv4 = 0, .ipv6 = 0, .error = true,
   .name = "/sockets/migrate/::/ipv4offipv6off",
   .args = "-incoming tcp:[::]:9000,ipv4=off,ipv6=off" },
-*/
 
 
 
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 1358c81bcc..76202949f5 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -616,6 +616,25 @@ err:
 }
 
 /* compatibility wrapper */
+static int inet_parse_flag(const char *flagname, const char *optstr, bool *val,
+   Error **errp)
+{
+char *end;
+size_t len;
+
+end = strstr(optstr, ",");
+len = end ? end - optstr : strlen(optstr);
+if (len == 0 || (len == 3 && strncmp(optstr, "=on", len) == 0)) {
+*val = true;
+} else if ((len == 4) && strncmp(optstr, "=off", len) == 0) {
+*val = false;
+} else {
+error_setg(errp, "error parsing '%s' flag '%s'", flagname, optstr);
+return -1;
+}
+return 0;
+}
+
 int inet_parse(InetSocketAddress *addr, const char *str, Error **errp)
 {
 const char *optstr, *h;
@@ -623,6 +642,7 @@ int inet_parse(InetSocketAddress *addr, const char *str, 
Error **errp)
 char port[33];
 int to;
 int pos;
+char *begin;
 
 memset(addr, 0, 

[Qemu-block] [PATCH 7/8] ui: convert VNC server to QIONetListener

[Daniel P. Berrange]

The VNC server already has the ability to listen on multiple sockets.
Converting it to use the QIONetListener APIs though, will reduce the
amount of code in the VNC server and improve the clarity of what is
left.

Signed-off-by: Daniel P. Berrange 
---
 ui/vnc.c | 194 ++-
 ui/vnc.h |   9 +--
 2 files changed, 57 insertions(+), 146 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 651cbb8606..5b78541575 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -227,12 +227,12 @@ static VncServerInfo *vnc_server_info_get(VncDisplay *vd)
 VncServerInfo *info;
 Error *err = NULL;
 
-if (!vd->nlsock) {
+if (!vd->listener || !vd->listener->nsioc) {
 return NULL;
 }
 
 info = g_malloc0(sizeof(*info));
-vnc_init_basic_info_from_server_addr(vd->lsock[0],
+vnc_init_basic_info_from_server_addr(vd->listener->sioc[0],
  qapi_VncServerInfo_base(info), );
 info->has_auth = true;
 info->auth = g_strdup(vnc_auth_name(vd));
@@ -378,7 +378,7 @@ VncInfo *qmp_query_vnc(Error **errp)
 VncDisplay *vd = vnc_display_find(NULL);
 SocketAddress *addr = NULL;
 
-if (vd == NULL || !vd->nlsock) {
+if (vd == NULL || !vd->listener || !vd->listener->nsioc) {
 info->enabled = false;
 } else {
 info->enabled = true;
@@ -387,11 +387,7 @@ VncInfo *qmp_query_vnc(Error **errp)
 info->has_clients = true;
 info->clients = qmp_query_client_list(vd);
 
-if (vd->lsock == NULL) {
-return info;
-}
-
-addr = qio_channel_socket_get_local_address(vd->lsock[0], errp);
+addr = qio_channel_socket_get_local_address(vd->listener->sioc[0], 
errp);
 if (!addr) {
 goto out_error;
 }
@@ -571,13 +567,14 @@ VncInfo2List *qmp_query_vnc_servers(Error **errp)
 info->has_display = true;
 info->display = g_strdup(dev->id);
 }
-for (i = 0; i < vd->nlsock; i++) {
+for (i = 0; vd->listener != NULL && i < vd->listener->nsioc; i++) {
 info->server = qmp_query_server_entry(
-vd->lsock[i], false, vd->auth, vd->subauth, info->server);
+vd->listener->sioc[i], false, vd->auth, vd->subauth,
+info->server);
 }
-for (i = 0; i < vd->nlwebsock; i++) {
+for (i = 0; vd->wslistener != NULL && i < vd->wslistener->nsioc; i++) {
 info->server = qmp_query_server_entry(
-vd->lwebsock[i], true, vd->ws_auth,
+vd->wslistener->sioc[i], true, vd->ws_auth,
 vd->ws_subauth, info->server);
 }
 
@@ -2991,36 +2988,18 @@ void vnc_start_protocol(VncState *vs)
 qemu_add_mouse_mode_change_notifier(>mouse_mode_notifier);
 }
 
-static gboolean vnc_listen_io(QIOChannel *ioc,
-  GIOCondition condition,
-  void *opaque)
+static void vnc_listen_io(QIONetListener *listener,
+  QIOChannelSocket *cioc,
+  void *opaque)
 {
 VncDisplay *vd = opaque;
-QIOChannelSocket *sioc = NULL;
-Error *err = NULL;
-bool isWebsock = false;
-size_t i;
-
-for (i = 0; i < vd->nlwebsock; i++) {
-if (ioc == QIO_CHANNEL(vd->lwebsock[i])) {
-isWebsock = true;
-break;
-}
-}
-
-sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc), );
-if (sioc != NULL) {
-qio_channel_set_name(QIO_CHANNEL(sioc),
- isWebsock ? "vnc-ws-server" : "vnc-server");
-qio_channel_set_delay(QIO_CHANNEL(sioc), false);
-vnc_connect(vd, sioc, false, isWebsock);
-object_unref(OBJECT(sioc));
-} else {
-/* client probably closed connection before we got there */
-error_free(err);
-}
+bool isWebsock = listener == vd->wslistener;
 
-return TRUE;
+qio_channel_set_name(QIO_CHANNEL(cioc),
+ isWebsock ? "vnc-ws-server" : "vnc-server");
+qio_channel_set_delay(QIO_CHANNEL(cioc), false);
+vnc_connect(vd, cioc, false, isWebsock);
+object_unref(OBJECT(cioc));
 }
 
 static const DisplayChangeListenerOps dcl_ops = {
@@ -3072,34 +3051,22 @@ void vnc_display_init(const char *id)
 
 static void vnc_display_close(VncDisplay *vd)
 {
-size_t i;
 if (!vd) {
 return;
 }
 vd->is_unix = false;
-for (i = 0; i < vd->nlsock; i++) {
-if (vd->lsock_tag[i]) {
-g_source_remove(vd->lsock_tag[i]);
-}
-object_unref(OBJECT(vd->lsock[i]));
+
+if (vd->listener) {
+qio_net_listener_disconnect(vd->listener);
+object_unref(OBJECT(vd->listener));
 }
-g_free(vd->lsock);
-g_free(vd->lsock_tag);
-vd->lsock = NULL;
-vd->lsock_tag = NULL;
-vd->nlsock = 0;
+vd->listener = NULL;
 
-for (i = 0; i < vd->nlwebsock; i++) {
-   

[Qemu-block] [PATCH 3/8] blockdev: convert internal NBD server to QIONetListener

[Daniel P. Berrange]

Instead of creating a QIOChannelSocket directly for the NBD
server socket, use a QIONetListener. This provides the ability
to listen on multiple sockets at the same time, so enables
full support for IPv4/IPv6 dual stack.

Signed-off-by: Daniel P. Berrange 
---
 blockdev-nbd.c | 50 --
 1 file changed, 16 insertions(+), 34 deletions(-)

diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 28f551a7b0..9e3c22109c 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -18,10 +18,10 @@
 #include "qmp-commands.h"
 #include "block/nbd.h"
 #include "io/channel-socket.h"
+#include "io/net-listener.h"
 
 typedef struct NBDServerData {
-QIOChannelSocket *listen_ioc;
-int watch;
+QIONetListener *listener;
 QCryptoTLSCreds *tlscreds;
 } NBDServerData;
 
@@ -32,27 +32,13 @@ static void nbd_blockdev_client_closed(NBDClient *client, 
bool ignored)
 nbd_client_put(client);
 }
 
-static gboolean nbd_accept(QIOChannel *ioc, GIOCondition condition,
-   gpointer opaque)
+static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
+   gpointer opaque)
 {
-QIOChannelSocket *cioc;
-
-if (!nbd_server) {
-return FALSE;
-}
-
-cioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc),
- NULL);
-if (!cioc) {
-return TRUE;
-}
-
 qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server");
 nbd_client_new(NULL, cioc,
nbd_server->tlscreds, NULL,
nbd_blockdev_client_closed);
-object_unref(OBJECT(cioc));
-return TRUE;
 }
 
 
@@ -62,10 +48,8 @@ static void nbd_server_free(NBDServerData *server)
 return;
 }
 
-if (server->watch != -1) {
-g_source_remove(server->watch);
-}
-object_unref(OBJECT(server->listen_ioc));
+qio_net_listener_disconnect(server->listener);
+object_unref(OBJECT(server->listener));
 if (server->tlscreds) {
 object_unref(OBJECT(server->tlscreds));
 }
@@ -112,12 +96,12 @@ void nbd_server_start(SocketAddress *addr, const char 
*tls_creds,
 }
 
 nbd_server = g_new0(NBDServerData, 1);
-nbd_server->watch = -1;
-nbd_server->listen_ioc = qio_channel_socket_new();
-qio_channel_set_name(QIO_CHANNEL(nbd_server->listen_ioc),
- "nbd-listener");
-if (qio_channel_socket_listen_sync(
-nbd_server->listen_ioc, addr, errp) < 0) {
+nbd_server->listener = qio_net_listener_new();
+
+qio_net_listener_set_name(nbd_server->listener,
+  "nbd-listener");
+
+if (qio_net_listener_open_sync(nbd_server->listener, addr, errp) < 0) {
 goto error;
 }
 
@@ -134,12 +118,10 @@ void nbd_server_start(SocketAddress *addr, const char 
*tls_creds,
 }
 }
 
-nbd_server->watch = qio_channel_add_watch(
-QIO_CHANNEL(nbd_server->listen_ioc),
-G_IO_IN,
-nbd_accept,
-NULL,
-NULL);
+qio_net_listener_set_client_func(nbd_server->listener,
+ nbd_accept,
+ NULL,
+ NULL);
 
 return;
 
-- 
2.13.3

[Qemu-block] [PATCH 4/8] blockdev: convert qemu-nbd server to QIONetListener

[Daniel P. Berrange]

Instead of creating a QIOChannelSocket directly for the NBD
server socket, use a QIONetListener. This provides the ability
to listen on multiple sockets at the same time, so enables
full support for IPv4/IPv6 dual stack.

Signed-off-by: Daniel P. Berrange 
---
 qemu-nbd.c | 50 +-
 1 file changed, 17 insertions(+), 33 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index b8666bb575..dcde7ac75c 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -38,6 +38,7 @@
 #include "qapi/qmp/qstring.h"
 #include "qom/object_interfaces.h"
 #include "io/channel-socket.h"
+#include "io/net-listener.h"
 #include "crypto/init.h"
 #include "trace/control.h"
 #include "qemu-version.h"
@@ -63,8 +64,7 @@ static int persistent = 0;
 static enum { RUNNING, TERMINATE, TERMINATING, TERMINATED } state;
 static int shared = 1;
 static int nb_fds;
-static QIOChannelSocket *server_ioc;
-static int server_watch = -1;
+static QIONetListener *server;
 static QCryptoTLSCreds *tlscreds;
 
 static void usage(const char *name)
@@ -345,44 +345,24 @@ static void nbd_client_closed(NBDClient *client, bool 
negotiated)
 nbd_client_put(client);
 }
 
-static gboolean nbd_accept(QIOChannel *ioc, GIOCondition cond, gpointer opaque)
+static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc, 
gpointer opaque)
 {
-QIOChannelSocket *cioc;
-
-cioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc),
- NULL);
-if (!cioc) {
-return TRUE;
-}
-
 if (state >= TERMINATE) {
-object_unref(OBJECT(cioc));
-return TRUE;
+return;
 }
 
 nb_fds++;
 nbd_update_server_watch();
 nbd_client_new(newproto ? NULL : exp, cioc,
tlscreds, NULL, nbd_client_closed);
-object_unref(OBJECT(cioc));
-
-return TRUE;
 }
 
 static void nbd_update_server_watch(void)
 {
 if (nbd_can_accept()) {
-if (server_watch == -1) {
-server_watch = qio_channel_add_watch(QIO_CHANNEL(server_ioc),
- G_IO_IN,
- nbd_accept,
- NULL, NULL);
-}
+qio_net_listener_set_client_func(server, nbd_accept, NULL, NULL);
 } else {
-if (server_watch != -1) {
-g_source_remove(server_watch);
-server_watch = -1;
-}
+qio_net_listener_set_client_func(server, NULL, NULL, NULL);
 }
 }
 
@@ -917,24 +897,28 @@ int main(int argc, char **argv)
 snprintf(sockpath, 128, SOCKET_PATH, basename(device));
 }
 
+server = qio_net_listener_new();
 if (socket_activation == 0) {
-server_ioc = qio_channel_socket_new();
 saddr = nbd_build_socket_address(sockpath, bindto, port);
-if (qio_channel_socket_listen_sync(server_ioc, saddr, _err) < 0) 
{
-object_unref(OBJECT(server_ioc));
+if (qio_net_listener_open_sync(server, saddr, _err) < 0) {
+object_unref(OBJECT(server));
 error_report_err(local_err);
-return 1;
+exit(EXIT_FAILURE);
 }
 } else {
+QIOChannelSocket *sioc;
 /* See comment in check_socket_activation above. */
 assert(socket_activation == 1);
-server_ioc = qio_channel_socket_new_fd(FIRST_SOCKET_ACTIVATION_FD,
-   _err);
-if (server_ioc == NULL) {
+sioc = qio_channel_socket_new_fd(FIRST_SOCKET_ACTIVATION_FD,
+ _err);
+if (sioc == NULL) {
+object_unref(OBJECT(server));
 error_report("Failed to use socket activation: %s",
  error_get_pretty(local_err));
 exit(EXIT_FAILURE);
 }
+qio_net_listener_add(server, sioc);
+object_unref(OBJECT(sioc));
 }
 
 if (qemu_init_main_loop(_err)) {
-- 
2.13.3

[Qemu-block] [PATCH 1/8] tests: add functional test validating ipv4/ipv6 address flag handling

[Daniel P. Berrange]

The semantics around handling ipv4=on|off & ipv6=on|off are quite
subtle to understand in combination with the various hostname addresses
and backend types. Introduce a massive test matrix that launches QEMU
and validates the ability to connect a client on each protocol as
appropriate.

The test requires that the host has ability to bind to both :: and
0.0.0.0, on port 9000. If either protocol is not available, or if
something is already listening on that port the test will skip.

Although it isn't using the QTest APIs, it expects the
QTEST_QEMU_BINARY env variable to be set.

Reviewed-by: Eric Blake 
Tested-by: Eric Blake 
Signed-off-by: Daniel P. Berrange 
---
 tests/.gitignore   |   1 +
 tests/Makefile.include |   3 +
 tests/test-sockets-proto.c | 924 +
 3 files changed, 928 insertions(+)
 create mode 100644 tests/test-sockets-proto.c

diff --git a/tests/.gitignore b/tests/.gitignore
index fed0189a5a..044183f4a0 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -78,6 +78,7 @@ test-qobject-output-visitor
 test-rcu-list
 test-replication
 test-shift128
+test-sockets-proto
 test-string-input-visitor
 test-string-output-visitor
 test-thread-pool
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 59e536bf0b..8caa5a7ae8 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -340,6 +340,7 @@ check-qtest-s390x-y = tests/boot-serial-test$(EXESUF)
 
 check-qtest-generic-y += tests/qom-test$(EXESUF)
 check-qtest-generic-y += tests/test-hmp$(EXESUF)
+check-qtest-generic-y += tests/test-sockets-proto$(EXESUF)
 
 qapi-schema += alternate-any.json
 qapi-schema += alternate-array.json
@@ -750,6 +751,8 @@ tests/usb-hcd-ehci-test$(EXESUF): tests/usb-hcd-ehci-test.o 
$(libqos-usb-obj-y)
 tests/usb-hcd-xhci-test$(EXESUF): tests/usb-hcd-xhci-test.o $(libqos-usb-obj-y)
 tests/pc-cpu-test$(EXESUF): tests/pc-cpu-test.o
 tests/postcopy-test$(EXESUF): tests/postcopy-test.o
+tests/test-sockets-proto$(EXESUF): tests/test-sockets-proto.o \
+   $(test-io-obj-y)
 tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o $(test-util-obj-y) \
$(qtest-obj-y) $(test-io-obj-y) $(libqos-virtio-obj-y) 
$(libqos-pc-obj-y) \
$(chardev-obj-y)
diff --git a/tests/test-sockets-proto.c b/tests/test-sockets-proto.c
new file mode 100644
index 00..1d6beda59f
--- /dev/null
+++ b/tests/test-sockets-proto.c
@@ -0,0 +1,924 @@
+/*
+ * QTest for IPv4/IPv6 protocol setup
+ *
+ * Copyright (c) 2017 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+
+#include "io/channel-socket.h"
+#include "qapi/error.h"
+#include "qemu/cutils.h"
+
+typedef struct {
+const char *name;
+const char *args;
+int ipv4; /* 0 -> disabled, 1 -> enabled */
+int ipv6; /* 0 -> disabled, 1 -> enabled, -1 -> check getaddrinfo() order 
*/
+bool error;
+} QSocketsData;
+
+/*
+ * This is the giant matrix of combinations we need to consider.
+ * There are 3 axes we deal with
+ *
+ * Axis 1: Protocol flags:
+ *
+ *  ipv4=unset, ipv6=unset  -> v4 & v6 clients ([1]
+ *  ipv4=unset, ipv6=off-> v4 clients only
+ *  ipv4=unset, ipv6=on -> v6 clients only
+ *  ipv4=off, ipv6=unset-> v6 clients only
+ *  ipv4=off, ipv6=off  -> error - can't disable both [2]
+ *  ipv4=off, ipv6=on   -> v6 clients only
+ *  ipv4=on, ipv6=unset -> v4 clients only
+ *  ipv4=on, ipv6=off   -> v4 clients only
+ *  ipv4=on, ipv6=on-> v4 & v6 clients [3]
+ *
+ * Depending on the listening address, some of those combinations
+ * may result in errors. eg ipv4=off,ipv6=on combined with 0.0.0.0
+ * is nonsensical.
+ *
+ * [1] Some backends only support a single socket listener, so
+ * will actually only allow v4 clients
+ * [2] QEMU should fail to startup in this case
+ * [3] If hostname is "" or "::", then we get a single listener
+ * on IPv6 and thus can also accept v4 clients. For all other
+ * hostnames, have same problem as [1].
+ *
+ * Axis 2: Listening address:
+ *
+ *  ""- resolves to 0.0.0.0 and ::, in that order
+ *  "0.0.0.0" - v4 clients only
+ *  "::"  - Mostly v6 clients only. Some scenarios should
+ *  permit v4 clients too.
+ *
+ * Axis 3: Backend type:
+ *
+ *  Migration - restricted to a single listener. Also relies
+ *  on buggy inet_parse() which can't accept
+ *  =off/=on parameters to ipv4/ipv6 flags
+ *  Chardevs  - restricted to a single listener.
+ *  VNC   - supports multiple listeners. Also supports
+ *  socket ranges, so has extra set of tests
+ *  in the matrix
+ *
+ */
+static QSocketsData test_data[] = {
+/* Migrate with "" address */
+/* XXX all settings with =off are disabled due to inet_parse() bug */
+/* XXX multilistener bug - 

[Qemu-block] [PATCH 2/8] io: introduce a network socket listener API

[Daniel P. Berrange]

The existing QIOChannelSocket class provides the ability to
listen on a single socket at a time. This patch introduces
a QIONetListener class that provides a higher level API
concept around listening for network services, allowing
for listening on multiple sockets.

Signed-off-by: Daniel P. Berrange 
---
 include/io/net-listener.h | 174 +
 io/Makefile.objs  |   1 +
 io/net-listener.c | 315 ++
 3 files changed, 490 insertions(+)
 create mode 100644 include/io/net-listener.h
 create mode 100644 io/net-listener.c

diff --git a/include/io/net-listener.h b/include/io/net-listener.h
new file mode 100644
index 00..0ac5c9cc72
--- /dev/null
+++ b/include/io/net-listener.h
@@ -0,0 +1,174 @@
+/*
+ * QEMU I/O network listener
+ *
+ * Copyright (c) 2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_NET_LISTENER_H
+#define QIO_NET_LISTENER_H
+
+#include "io/channel-socket.h"
+
+#define TYPE_QIO_NET_LISTENER "qio-net-listener"
+#define QIO_NET_LISTENER(obj)\
+OBJECT_CHECK(QIONetListener, (obj), TYPE_QIO_NET_LISTENER)
+#define QIO_NET_LISTENER_CLASS(klass)\
+OBJECT_CLASS_CHECK(QIONetListenerClass, klass, TYPE_QIO_NET_LISTENER)
+#define QIO_NET_LISTENER_GET_CLASS(obj)  \
+OBJECT_GET_CLASS(QIONetListenerClass, obj, TYPE_QIO_NET_LISTENER)
+
+typedef struct QIONetListener QIONetListener;
+typedef struct QIONetListenerClass QIONetListenerClass;
+
+typedef void (*QIONetListenerClientFunc)(QIONetListener *listener,
+ QIOChannelSocket *sioc,
+ gpointer data);
+
+/**
+ * QIONetListener:
+ *
+ * The QIONetListener object encapsulates the management of a
+ * listening socket. It is able to listen on multiple sockets
+ * concurrently, to deal with the scenario where IPv4 / IPv6
+ * needs separate sockets, or there is a need to listen on a
+ * subset of interface IP addresses, instead of the wildcard
+ * address.
+ */
+struct QIONetListener {
+Object parent;
+
+char *name;
+QIOChannelSocket **sioc;
+gulong *io_tag;
+size_t nsioc;
+
+gboolean disconnected;
+
+QIONetListenerClientFunc io_func;
+gpointer io_data;
+GDestroyNotify io_notify;
+};
+
+struct QIONetListenerClass {
+ObjectClass parent;
+};
+
+
+/**
+ * qio_net_listener_new:
+ *
+ * Create a new network listener service, which is not
+ * listening on any sockets initially.
+ *
+ * Returns: the new listener
+ */
+QIONetListener *qio_net_listener_new(void);
+
+
+/**
+ * qio_net_listener_set_name:
+ * @listener: the network listener object
+ * @name: the listener name
+ *
+ * Set the name of the listener. This is used as a debugging
+ * aid, to set names on any GSource instances associated
+ * with the listener
+ */
+void qio_net_listener_set_name(QIONetListener *listener,
+   const char *name);
+
+/**
+ * qio_net_listener_open_sync:
+ * @listener: the network listener object
+ * @addr: the address to listen on
+ * @errp: pointer to a NULL initialized error object
+ *
+ * Synchronously open a listening connection on all
+ * addresses associated with @addr. This method may
+ * also be invoked multiple times, in order to have a
+ * single listener on multiple distinct addresses.
+ */
+int qio_net_listener_open_sync(QIONetListener *listener,
+   SocketAddress *addr,
+   Error **errp);
+
+/**
+ * qio_net_listener_add:
+ * @listener: the network listener object
+ * @sioc: the socket I/O channel
+ *
+ * Associate a listening socket I/O channel with the
+ * listener. The listener will acquire an new reference
+ * on @sioc, so the caller should release its own reference
+ * if it no longer requires the object.
+ */
+void qio_net_listener_add(QIONetListener *listener,
+  QIOChannelSocket *sioc);
+
+/**
+ * qio_net_listener_set_client_func:
+ * @listener: the network listener object
+ * @func: the callback function
+ * @data: opaque data to pass to @func
+ * @notify: callback to free @data
+ *
+ * Register @func to be invoked whenever a new client
+ * connects to the listener. 

[Qemu-block] [PATCH 0/8] Enable full IPv4/IPv6 dual stack support

[Daniel P. Berrange]

Currently all the network listeners in QEMU, except the VNC server,
are restricted to listening on a single socket. This makes it
impossible to fully support IPv4/IPv6 dual stack. We're restricted
to using IPV6_V6ONLY=0 to listen on both protocols from a single
socket, but this doesn't work at all on OpenBSD, and even where
supported it is quite crude (only really works for localhost and
wildcard addresses).

This patch series introduces a new object QIONetListener, which
encapsulates multiple QIOChannelSocket listeners. This makes it
trivial to support multiple listening sockets in any part of
QEMU.

Daniel P. Berrange (8):
  tests: add functional test validating ipv4/ipv6 address flag handling
  io: introduce a network socket listener API
  blockdev: convert internal NBD server to QIONetListener
  blockdev: convert qemu-nbd server to QIONetListener
  migration: convert socket server to QIONetListener
  chardev: convert the socket server to QIONetListener
  ui: convert VNC server to QIONetListener
  sockets: fix parsing of ipv4/ipv6 opts in parse_socket_addr

 blockdev-nbd.c |  50 +--
 chardev/char-socket.c  |  70 ++--
 include/io/net-listener.h  | 174 +
 io/Makefile.objs   |   1 +
 io/net-listener.c  | 315 
 migration/socket.c |  44 +--
 qemu-nbd.c |  50 +--
 tests/.gitignore   |   1 +
 tests/Makefile.include |   3 +
 tests/test-sockets-proto.c | 906 +
 ui/vnc.c   | 194 +++---
 ui/vnc.h   |   9 +-
 util/qemu-sockets.c|  36 +-
 13 files changed, 1563 insertions(+), 290 deletions(-)
 create mode 100644 include/io/net-listener.h
 create mode 100644 io/net-listener.c
 create mode 100644 tests/test-sockets-proto.c

-- 
2.13.3

[Qemu-block] [PATCH 6/8] chardev: convert the socket server to QIONetListener

[Daniel P. Berrange]

Instead of creating a QIOChannelSocket directly for the chardev
server socket, use a QIONetListener. This provides the ability
to listen on multiple sockets at the same time, so enables
full support for IPv4/IPv6 dual stack.

Signed-off-by: Daniel P. Berrange 
---
 chardev/char-socket.c  | 70 ++
 tests/test-sockets-proto.c |  8 ++
 2 files changed, 29 insertions(+), 49 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 1ae730a4cb..96ff2a3ff4 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -25,6 +25,7 @@
 #include "chardev/char.h"
 #include "io/channel-socket.h"
 #include "io/channel-tls.h"
+#include "io/net-listener.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "qapi/clone-visitor.h"
@@ -40,8 +41,7 @@ typedef struct {
 Chardev parent;
 QIOChannel *ioc; /* Client I/O channel */
 QIOChannelSocket *sioc; /* Client master channel */
-QIOChannelSocket *listen_ioc;
-guint listen_tag;
+QIONetListener *listener;
 QCryptoTLSCreds *tls_creds;
 int connected;
 int max_size;
@@ -93,9 +93,9 @@ static void check_report_connect_error(Chardev *chr,
 qemu_chr_socket_restart_timer(chr);
 }
 
-static gboolean tcp_chr_accept(QIOChannel *chan,
-   GIOCondition cond,
-   void *opaque);
+static void tcp_chr_accept(QIONetListener *listener,
+   QIOChannelSocket *cioc,
+   void *opaque);
 
 static int tcp_chr_read_poll(void *opaque);
 static void tcp_chr_disconnect(Chardev *chr);
@@ -404,9 +404,8 @@ static void tcp_chr_disconnect(Chardev *chr)
 
 tcp_chr_free_connection(chr);
 
-if (s->listen_ioc) {
-s->listen_tag = qio_channel_add_watch(
-QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, NULL);
+if (s->listener) {
+qio_net_listener_set_client_func(s->listener, tcp_chr_accept, chr, 
NULL);
 }
 update_disconnected_filename(s);
 qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
@@ -704,9 +703,8 @@ static int tcp_chr_new_client(Chardev *chr, 
QIOChannelSocket *sioc)
 if (s->do_nodelay) {
 qio_channel_set_delay(s->ioc, false);
 }
-if (s->listen_tag) {
-g_source_remove(s->listen_tag);
-s->listen_tag = 0;
+if (s->listener) {
+qio_net_listener_set_client_func(s->listener, NULL, NULL, NULL);
 }
 
 if (s->tls_creds) {
@@ -738,24 +736,14 @@ static int tcp_chr_add_client(Chardev *chr, int fd)
 return ret;
 }
 
-static gboolean tcp_chr_accept(QIOChannel *channel,
-   GIOCondition cond,
-   void *opaque)
+static void tcp_chr_accept(QIONetListener *listener,
+   QIOChannelSocket *cioc,
+   void *opaque)
 {
 Chardev *chr = CHARDEV(opaque);
-QIOChannelSocket *sioc;
-
-sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(channel),
- NULL);
-if (!sioc) {
-return TRUE;
-}
-
-tcp_chr_new_client(chr, sioc);
 
-object_unref(OBJECT(sioc));
-
-return TRUE;
+tcp_chr_set_client_ioc_name(chr, cioc);
+tcp_chr_new_client(chr, cioc);
 }
 
 static int tcp_chr_wait_connected(Chardev *chr, Error **errp)
@@ -769,9 +757,10 @@ static int tcp_chr_wait_connected(Chardev *chr, Error 
**errp)
 if (s->is_listen) {
 info_report("QEMU waiting for connection on: %s",
 chr->filename);
-qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), true, NULL);
-tcp_chr_accept(QIO_CHANNEL(s->listen_ioc), G_IO_IN, chr);
-qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), false, NULL);
+sioc = qio_net_listener_wait_client(s->listener);
+tcp_chr_set_client_ioc_name(chr, sioc);
+tcp_chr_new_client(chr, sioc);
+object_unref(OBJECT(sioc));
 } else {
 sioc = qio_channel_socket_new();
 tcp_chr_set_client_ioc_name(chr, sioc);
@@ -799,12 +788,9 @@ static void char_socket_finalize(Object *obj)
 s->reconnect_timer = 0;
 }
 qapi_free_SocketAddress(s->addr);
-if (s->listen_tag) {
-g_source_remove(s->listen_tag);
-s->listen_tag = 0;
-}
-if (s->listen_ioc) {
-object_unref(OBJECT(s->listen_ioc));
+if (s->listener) {
+qio_net_listener_set_client_func(s->listener, NULL, NULL, NULL);
+object_unref(OBJECT(s->listener));
 }
 if (s->tls_creds) {
 object_unref(OBJECT(s->tls_creds));
@@ -937,29 +923,27 @@ static void qmp_chardev_open_socket(Chardev *chr,
 } else {
 if (s->is_listen) {
 char *name;
-sioc = qio_channel_socket_new();
+s->listener = qio_net_listener_new();
 
 name = g_strdup_printf("chardev-tcp-listener-%s", 

[Qemu-block] [PATCH 5/8] migration: convert socket server to QIONetListener

[Daniel P. Berrange]

Instead of creating a QIOChannelSocket directly for the migration
server socket, use a QIONetListener. This provides the ability
to listen on multiple sockets at the same time, so enables
full support for IPv4/IPv6 dual stack.

Signed-off-by: Daniel P. Berrange 
---
 migration/socket.c | 44 ++--
 tests/test-sockets-proto.c |  3 +--
 2 files changed, 15 insertions(+), 32 deletions(-)

diff --git a/migration/socket.c b/migration/socket.c
index 757d3821a1..34811addc5 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -24,6 +24,7 @@
 #include "migration.h"
 #include "qemu-file.h"
 #include "io/channel-socket.h"
+#include "io/net-listener.h"
 #include "trace.h"
 
 
@@ -130,53 +131,36 @@ void unix_start_outgoing_migration(MigrationState *s,
 }
 
 
-static gboolean socket_accept_incoming_migration(QIOChannel *ioc,
- GIOCondition condition,
- gpointer opaque)
+static void socket_accept_incoming_migration(QIONetListener *listener,
+ QIOChannelSocket *cioc,
+ gpointer opaque)
 {
-QIOChannelSocket *sioc;
-Error *err = NULL;
-
-sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc),
- );
-if (!sioc) {
-error_report("could not accept migration connection (%s)",
- error_get_pretty(err));
-goto out;
-}
-
 trace_migration_socket_incoming_accepted();
 
-qio_channel_set_name(QIO_CHANNEL(sioc), "migration-socket-incoming");
-migration_channel_process_incoming(QIO_CHANNEL(sioc));
-object_unref(OBJECT(sioc));
+qio_channel_set_name(QIO_CHANNEL(cioc), "migration-socket-incoming");
+migration_channel_process_incoming(QIO_CHANNEL(cioc));
 
-out:
 /* Close listening socket as its no longer needed */
-qio_channel_close(ioc, NULL);
-return FALSE; /* unregister */
+qio_net_listener_disconnect(listener);
+
+object_unref(OBJECT(listener));
 }
 
 
 static void socket_start_incoming_migration(SocketAddress *saddr,
 Error **errp)
 {
-QIOChannelSocket *listen_ioc = qio_channel_socket_new();
+QIONetListener *listener = qio_net_listener_new();
 
-qio_channel_set_name(QIO_CHANNEL(listen_ioc),
- "migration-socket-listener");
+qio_net_listener_set_name(listener, "migration-socket-listener");
 
-if (qio_channel_socket_listen_sync(listen_ioc, saddr, errp) < 0) {
-object_unref(OBJECT(listen_ioc));
+if (qio_net_listener_open_sync(listener, saddr, errp) < 0) {
+object_unref(OBJECT(listener));
 qapi_free_SocketAddress(saddr);
 return;
 }
 
-qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
-  G_IO_IN,
-  socket_accept_incoming_migration,
-  listen_ioc,
-  (GDestroyNotify)object_unref);
+qio_net_listener_set_client_func(listener, 
socket_accept_incoming_migration, NULL, NULL);
 qapi_free_SocketAddress(saddr);
 }
 
diff --git a/tests/test-sockets-proto.c b/tests/test-sockets-proto.c
index 1d6beda59f..1495369696 100644
--- a/tests/test-sockets-proto.c
+++ b/tests/test-sockets-proto.c
@@ -70,8 +70,7 @@ typedef struct {
 static QSocketsData test_data[] = {
 /* Migrate with "" address */
 /* XXX all settings with =off are disabled due to inet_parse() bug */
-/* XXX multilistener bug - should be .ipv6 = 1 */
-{ .ipv4 = 1, .ipv6 = -1, .error = false,
+{ .ipv4 = 1, .ipv6 = 1, .error = false,
   .name = "/sockets/migrate/wildcard/all",
   .args = "-incoming tcp::9000" },
 { .ipv4 = 1, .ipv6 = 0, .error = false,
-- 
2.13.3

Re: [Qemu-block] [Qemu-devel] [PATCH 2/2] file-posix: Do runtime check for ofd lock API

[Christian Ehrhardt]

On Fri, Jul 21, 2017 at 2:36 PM, Eric Blake  wrote:

> On 07/21/2017 05:20 AM, Fam Zheng wrote:
> > It is reported that on Windows Subsystem for Linux, ofd operations fail
> > with -EINVAL. In other words, QEMU binary built with system headers that
> > exports F_OFD_SETLK doesn't necessarily run in an environment that
> > actually supports it:
> >
> > $ qemu-system-aarch64 ... -drive file=test.vhdx,if=none,id=hd0 \
> > -device virtio-blk-pci,drive=hd0
> > qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to
> unlock byte 100
> > qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to
> unlock byte 100
> > qemu-system-aarch64: -drive file=test.vhdx,if=none,id=hd0: Failed to
> lock byte 100
> >
> > Let's do a runtime check to cope with that.
>
> You may want to mention that the same is possible on a system with old
> kernel but new glibc (ie. this issue is not necessarily specific to WSL).
>

I first thought that this combination hitting me as I run KVM in containers
which can diverge glibc (in container) a lot from kernel (in host).

My issue turned out to be an apparmor block instead.
But since I clearly see how my case could lead to the mentioned
old kernel but new glibc I wanted to ping here to refresh/reconsider
this change as well.

Also the reply might be worth as documentation if people search for the
error
message and get here that the following apparmor block leads to the same.

apparmor="DENIED" operation="file_lock"
namespace="root//lxd-testkvm-artful-from_"
profile="libvirt-f687a9b3-5bca-41bc-b206-6e616720cc5e"
name="/var/lib/uvtool/libvirt/images/kvmguest-artful-normal.qcow" pid=7001
comm="qemu-system-x86" requested_mask="k" denied_mask="k" fsuid=0 ouid=0

I'll work on libvirt's virt-aa-helper to generate a rule appropriate for
the new behavior.

Re: [Qemu-block] [PATCH for-2.10 v2 0/5] More bdrv_getlength() fixes

[Eric Blake]

On 08/10/2017 08:02 AM, Kevin Wolf wrote:
> Am 09.08.2017 um 22:38 hat Eric Blake geschrieben:
>> We already have a lot of bdrv_getlength() fixes in -rc2; so I think
>> this is still okay for -rc3.
>>
>> v1 was here (with a typo'd subject line):
>> https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01226.html
>>
>> Since v1:
>> - patch 1: fix error message capitalization (Kevin, R-b kept)
>> - fix locking bug in original patch 2 (Kevin)
>> - split original patch 2 into two parts: signature update, and
>> added error checking (Kevin)
>> - check for unlikely integer overflow before bdrv_truncate (Jeff)
>>
>> 001/5:[0002] [FC] 'vpc: Check failure of bdrv_getlength()'
>> 002/5:[down] 'qcow: Change signature of get_cluster_offset()'
>> 003/5:[0048] [FC] 'qcow: Check failure of bdrv_getlength() and 
>> bdrv_truncate()'
>> 004/5:[] [--] 'qcow2: Drop debugging dump_refcounts()'
>> 005/5:[] [--] 'qcow2: Check failure of bdrv_getlength()'
> 
> Looks good to me, but as the bug is far from being critical, I'd rather
> apply the more complex qcow1 patches only to block-next. The vpc and
> qcow2 parts seems a lot less risky, so 2.10 should be okay for them.
> 
> What do you think?

The argument for NOT doing the qcow changes (patches 2 and 3): the only
place where we are not checking for failures is part of
get_cluster_offset() - but in all likelihood, if we were unable to
determine or change the length of the backing file, we will have nearby
problems that will ultimately cause failure soon enough.  Furthermore,
it's not a regression (we've had several releases with the problem), and
qcow is not a good format (it's painfully slow, and we strongly
recommend qcow2 instead) - so no one will be hitting any actual bugs in
practice.

I'll trust your judgment as maintainer, so taking just 1, 4, and 5 in
2.10 is fine.

-- 
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 v4 4/7] block: convert ThrottleGroup to object with QOM

[Alberto Garcia]

On Wed 09 Aug 2017 12:07:31 PM CEST, Manos Pitsidianakis wrote:
> +/* unfix buckets to check validity */
> +throttle_get_config(>ts, cfg);
> +if (!throttle_is_valid(cfg, errp)) {
> +return;
> +}
> +/* fix buckets again */
> +throttle_config(>ts, tg->clock_type, cfg);

Ugh, this whole fix/unfix buckets should be transparent to outside users
of throttle.c.

You can leave this here for now but I'll think of a way to eliminate the
need to do this.

Berto

Re: [Qemu-block] [PATCH v4 5/7] block: add throttle block filter driver

[Manos Pitsidianakis]

On Thu, Aug 10, 2017 at 03:54:02PM +0200, Alberto Garcia wrote:

On Wed 09 Aug 2017 12:07:32 PM CEST, Manos Pitsidianakis wrote:

+/* Extract ThrottleConfig options. Assumes cfg is initialized and will be
+ * checked for validity.
+ *
+ * Returns -1 and sets errp if a burst_length value is over UINT_MAX.
+ */
+static int throttle_extract_options(QemuOpts *opts, ThrottleConfig *cfg,
+Error **errp)
+{
+#define IF_OPT_SET(rvalue, opt_name) \
+if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX opt_name)) { \
+rvalue = qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX opt_name, 0); }
+
+IF_OPT_SET(cfg->buckets[THROTTLE_BPS_TOTAL].avg, QEMU_OPT_BPS_TOTAL);
+IF_OPT_SET(cfg->buckets[THROTTLE_BPS_READ].avg, QEMU_OPT_BPS_READ);
+IF_OPT_SET(cfg->buckets[THROTTLE_BPS_WRITE].avg, QEMU_OPT_BPS_WRITE);
+IF_OPT_SET(cfg->buckets[THROTTLE_OPS_TOTAL].avg, QEMU_OPT_IOPS_TOTAL);
+IF_OPT_SET(cfg->buckets[THROTTLE_OPS_READ].avg, QEMU_OPT_IOPS_READ);
+IF_OPT_SET(cfg->buckets[THROTTLE_OPS_WRITE].avg, QEMU_OPT_IOPS_WRITE);
+IF_OPT_SET(cfg->buckets[THROTTLE_BPS_TOTAL].max, QEMU_OPT_BPS_TOTAL_MAX);

 [...]

This is all the code that I was saying that we'd save if we don't allow
setting limits here.


+static int throttle_configure_tgm(BlockDriverState *bs,
+  ThrottleGroupMember *tgm,
+  QDict *options, Error **errp)
+{
+int ret;
+ThrottleConfig cfg;
+const char *group_name = NULL;


No need to set it to NULL here.


I know, I do it out of habit!




+Error *local_err = NULL;
+QemuOpts *opts = qemu_opts_create(_opts, NULL, 0, 
_abort);

+
+qemu_opts_absorb_qdict(opts, options, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+goto err;
+}
+
+/* If group_name is NULL, an anonymous group will be created */
+group_name = qemu_opt_get(opts, QEMU_OPT_THROTTLE_GROUP_NAME);
+
+/* Register membership to group with name group_name */
+throttle_group_register_tgm(tgm, group_name, bdrv_get_aio_context(bs));
+
+/* Copy previous configuration */
+throttle_group_get_config(tgm, );
+
+/* Change limits if user has specified them */
+if (throttle_extract_options(opts, , errp) ||
+!throttle_is_valid(, errp)) {
+throttle_group_unregister_tgm(tgm);
+goto err;
+}
+/* Update group configuration */
+throttle_group_config(tgm, );


We'd also spare this, and this function would remain much simpler.


+
+ret = 0;
+goto fin;
+
+err:
+ret = -EINVAL;
+fin:
+qemu_opts_del(opts);
+return ret;
+}


If you set ret = -EINVAL before calling goto err you can simplify this
part as well, but feel free to ignore this suggestion.


+static int throttle_reopen_prepare(BDRVReopenState *reopen_state,
+   BlockReopenQueue *queue, Error **errp)
+{
+ThrottleGroupMember *tgm = NULL;
+
+assert(reopen_state != NULL);
+assert(reopen_state->bs != NULL);
+
+reopen_state->opaque = g_new0(ThrottleGroupMember, 1);
+tgm = reopen_state->opaque;
+
+return throttle_configure_tgm(reopen_state->bs, tgm, reopen_state->options,
+errp);
+}


I would rename 'reopen_state' as 'state' for consistency with the other
two functions.


The function signatures in block_int.h have reopen_state, so maybe for 
consistency I should change the other two to reopen_state as well, 
instead.





+static void throttle_reopen_commit(BDRVReopenState *state)
+{
+ThrottleGroupMember *tgm = state->bs->opaque;
+
+throttle_group_unregister_tgm(tgm);
+g_free(state->bs->opaque);
+state->bs->opaque = state->opaque;
+state->opaque = NULL;
+}


I also find the mixing of state->bs->opaque and tgm a bit confusing.
Here's a suggestion, but feel free to ignore it:


You're right, though it's only a few lines it might require a second 
read. I will rewrite those more clearly, too.


signature.asc
Description: PGP signature

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.10?] file-posix: Clear out first sector in hdev_create

[Eric Blake]

On 08/10/2017 03:01 AM, Fam Zheng wrote:
> People get surprised when, after "qemu-imc create -f raw /dev/sdX", they
> still see qcow2 with "qemu-img info", if previously the bdev had a qcow2
> header. While this is natural because raw doesn't need to write any
> magic bytes during creation, hdev_create is free to clear out the first
> sector to make sure the stale qcow2 header doesn't cause such a
> confusion.
> 
> Signed-off-by: Fam Zheng 
> ---
>  block/file-posix.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index f4de022ae0..1d8ef6f873 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -2703,6 +2703,17 @@ static int hdev_create(const char *filename, QemuOpts 
> *opts,
>  ret = -ENOSPC;
>  }
>  
> +if (total_size) {
> +int64_t zero_size = MIN(BDRV_SECTOR_SIZE, total_size);
> +uint8_t *buf;

Since BDRV_SECTOR_SIZE is small enough to stack-allocate, you could skip
the malloc by doing:

uint8_t buf[BDRV_SECTOR_SIZE] = "";

> +if (lseek(fd, 0, SEEK_SET) == -1) {
> +ret = -errno;
> +} else {
> +buf = g_malloc0(zero_size);
> +ret = qemu_write_full(fd, buf, zero_size);

Instead of doing lseek + qemu_write_full, can we just use
qemu_pwritev(fd, , 1, 0) with an iov set up to point to the
appropriate amount of buf?

At any rate, my ideas are micro-optimizations, so I can also live with
how you wrote it.

Reviewed-by: Eric Blake 

Are you arguing that this is a bug-fix worthy of inclusion in 2.10,
because it helps avoid user confusion? Or are you delaying it to 2.11,
because we've had the existing behavior for longer than one release, so
one release more won't hurt?

-- 
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 v4 5/7] block: add throttle block filter driver

[Alberto Garcia]

On Wed 09 Aug 2017 12:07:32 PM CEST, Manos Pitsidianakis wrote:
> +/* Extract ThrottleConfig options. Assumes cfg is initialized and will be
> + * checked for validity.
> + *
> + * Returns -1 and sets errp if a burst_length value is over UINT_MAX.
> + */
> +static int throttle_extract_options(QemuOpts *opts, ThrottleConfig *cfg,
> +Error **errp)
> +{
> +#define IF_OPT_SET(rvalue, opt_name) \
> +if (qemu_opt_get(opts, THROTTLE_OPT_PREFIX opt_name)) { \
> +rvalue = qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX opt_name, 0); 
> }
> +
> +IF_OPT_SET(cfg->buckets[THROTTLE_BPS_TOTAL].avg, QEMU_OPT_BPS_TOTAL);
> +IF_OPT_SET(cfg->buckets[THROTTLE_BPS_READ].avg, QEMU_OPT_BPS_READ);
> +IF_OPT_SET(cfg->buckets[THROTTLE_BPS_WRITE].avg, QEMU_OPT_BPS_WRITE);
> +IF_OPT_SET(cfg->buckets[THROTTLE_OPS_TOTAL].avg, QEMU_OPT_IOPS_TOTAL);
> +IF_OPT_SET(cfg->buckets[THROTTLE_OPS_READ].avg, QEMU_OPT_IOPS_READ);
> +IF_OPT_SET(cfg->buckets[THROTTLE_OPS_WRITE].avg, QEMU_OPT_IOPS_WRITE);
> +IF_OPT_SET(cfg->buckets[THROTTLE_BPS_TOTAL].max, QEMU_OPT_BPS_TOTAL_MAX);
  [...]

This is all the code that I was saying that we'd save if we don't allow
setting limits here.

> +static int throttle_configure_tgm(BlockDriverState *bs,
> +  ThrottleGroupMember *tgm,
> +  QDict *options, Error **errp)
> +{
> +int ret;
> +ThrottleConfig cfg;
> +const char *group_name = NULL;

No need to set it to NULL here.

> +Error *local_err = NULL;
> +QemuOpts *opts = qemu_opts_create(_opts, NULL, 0, _abort);
> +
> +qemu_opts_absorb_qdict(opts, options, _err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +goto err;
> +}
> +
> +/* If group_name is NULL, an anonymous group will be created */
> +group_name = qemu_opt_get(opts, QEMU_OPT_THROTTLE_GROUP_NAME);
> +
> +/* Register membership to group with name group_name */
> +throttle_group_register_tgm(tgm, group_name, bdrv_get_aio_context(bs));
> +
> +/* Copy previous configuration */
> +throttle_group_get_config(tgm, );
> +
> +/* Change limits if user has specified them */
> +if (throttle_extract_options(opts, , errp) ||
> +!throttle_is_valid(, errp)) {
> +throttle_group_unregister_tgm(tgm);
> +goto err;
> +}
> +/* Update group configuration */
> +throttle_group_config(tgm, );

We'd also spare this, and this function would remain much simpler.

> +
> +ret = 0;
> +goto fin;
> +
> +err:
> +ret = -EINVAL;
> +fin:
> +qemu_opts_del(opts);
> +return ret;
> +}

If you set ret = -EINVAL before calling goto err you can simplify this
part as well, but feel free to ignore this suggestion.

> +static int throttle_reopen_prepare(BDRVReopenState *reopen_state,
> +   BlockReopenQueue *queue, Error **errp)
> +{
> +ThrottleGroupMember *tgm = NULL;
> +
> +assert(reopen_state != NULL);
> +assert(reopen_state->bs != NULL);
> +
> +reopen_state->opaque = g_new0(ThrottleGroupMember, 1);
> +tgm = reopen_state->opaque;
> +
> +return throttle_configure_tgm(reopen_state->bs, tgm, 
> reopen_state->options,
> +errp);
> +}

I would rename 'reopen_state' as 'state' for consistency with the other
two functions.

> +static void throttle_reopen_commit(BDRVReopenState *state)
> +{
> +ThrottleGroupMember *tgm = state->bs->opaque;
> +
> +throttle_group_unregister_tgm(tgm);
> +g_free(state->bs->opaque);
> +state->bs->opaque = state->opaque;
> +state->opaque = NULL;
> +}

I also find the mixing of state->bs->opaque and tgm a bit confusing.
Here's a suggestion, but feel free to ignore it:

ThrottleGroupMember *old_tgm = state->bs->opaque;
ThrottleGroupMember *new_tgm = state->opaque;

throttle_group_unregister_tgm(old_tgm);
g_free(old_tgm);

state->bs->opaque = new_tgm;
state->opaque = NULL;

> +static void throttle_reopen_abort(BDRVReopenState *state)
> +{
> +ThrottleGroupMember *tgm = state->opaque;
> +
> +throttle_group_unregister_tgm(tgm);
> +g_free(state->opaque);
> +state->opaque = NULL;
> +}

Similar problem here (this one is simpler though).

Apart from the general question of whether we're parsing the limits in
this driver, the rest are just comments about the style. Otherwise the
code looks good, thanks!

Berto

Re: [Qemu-block] [PATCH] virtio-blk: handle blk_getlength() errors

[Stefan Hajnoczi]

On Tue, Aug 08, 2017 at 01:22:51PM +0100, Stefan Hajnoczi wrote:
> If blk_getlength() fails in virtio_blk_update_config() consider the disk
> image length to be 0 bytes.
> 
> Signed-off-by: Stefan Hajnoczi 
> ---
>  hw/block/virtio-blk.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-block] [PATCH for-2.10 v2 0/5] More bdrv_getlength() fixes

[Kevin Wolf]

Am 09.08.2017 um 22:38 hat Eric Blake geschrieben:
> We already have a lot of bdrv_getlength() fixes in -rc2; so I think
> this is still okay for -rc3.
> 
> v1 was here (with a typo'd subject line):
> https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01226.html
> 
> Since v1:
> - patch 1: fix error message capitalization (Kevin, R-b kept)
> - fix locking bug in original patch 2 (Kevin)
> - split original patch 2 into two parts: signature update, and
> added error checking (Kevin)
> - check for unlikely integer overflow before bdrv_truncate (Jeff)
> 
> 001/5:[0002] [FC] 'vpc: Check failure of bdrv_getlength()'
> 002/5:[down] 'qcow: Change signature of get_cluster_offset()'
> 003/5:[0048] [FC] 'qcow: Check failure of bdrv_getlength() and 
> bdrv_truncate()'
> 004/5:[] [--] 'qcow2: Drop debugging dump_refcounts()'
> 005/5:[] [--] 'qcow2: Check failure of bdrv_getlength()'

Looks good to me, but as the bug is far from being critical, I'd rather
apply the more complex qcow1 patches only to block-next. The vpc and
qcow2 parts seems a lot less risky, so 2.10 should be okay for them.

What do you think?

Kevin

Re: [Qemu-block] block replication

[Vladimir Sementsov-Ogievskiy]

09.08.2017 17:11, Vladimir Sementsov-Ogievskiy wrote:

Hi Wen!

I'm trying to understand block/replication code and have a question.

Why should we block the region from intersecting cow requests when 
read? If I understand correctly


regardless of writes to the secondary-disk we have consistent view of 
it through hidden-disk:


Even if we are intersecting with some writes to secondary-disk (and 
corresponding cow-requests), the


data in secondary disk will not be updated until backed up to 
hidden-disk, therefore, for read we have two


options:

1. read old data from secondary-disk (unallocated region in 
hidden-disk means data in secondary-disk is not updated yet)


2. read backed-up data from hidden-disk (data in secondary-disk may be 
already updated but we don't care)


(the whole region to read may consists of parts, corresponding to 1 or 
2, but this should be ok too)


Where am I wrong?


Ok, now I think this is needed to prevent intersecting of writes and 
reads on hidden-disk. If it so, I think it is better to use serializing 
requests
mechanism (just serialize all requests on hidden-disk, and on write wait 
for all intersecting serializing requests, on read wait for intersecting 
serializing writes) - it may require additional
option for BlockDriverState, but it is more generic and more clear than 
export internal backup things to lock disk region. This also can be 
reused for image-fleecing scheme
(which is based on same pattern  [active-disk is backing for temp-disk, 
backup sync=none from active to temp, read from temp])





==

static coroutine_fn int replication_co_readv(BlockDriverState *bs,
 int64_t sector_num,
 int remaining_sectors,
 QEMUIOVector *qiov)
{
BDRVReplicationState *s = bs->opaque;
BdrvChild *child = s->secondary_disk;
BlockJob *job = NULL;
CowRequest req;
int ret;

if (s->mode == REPLICATION_MODE_PRIMARY) {
/* We only use it to forward primary write requests */
return -EIO;
}

ret = replication_get_io_status(s);
if (ret < 0) {
return ret;
}

if (child && child->bs) {
job = child->bs->job;
}

if (job) {
uint64_t remaining_bytes = remaining_sectors * BDRV_SECTOR_SIZE;

backup_wait_for_overlapping_requests(child->bs->job,
 sector_num * 
BDRV_SECTOR_SIZE,

remaining_bytes);
backup_cow_request_begin(, child->bs->job,
 sector_num * BDRV_SECTOR_SIZE,
remaining_bytes);
ret = bdrv_co_readv(bs->file, sector_num, remaining_sectors,
qiov);
backup_cow_request_end();
goto out;
}

ret = bdrv_co_readv(bs->file, sector_num, remaining_sectors, qiov);
out:
return replication_return_value(s, ret);
}



--
Best regards,
Vladimir

Re: [Qemu-block] [PATCH 0/2] IDE: Do not flush empty drives

[Stefan Hajnoczi]

On Wed, Aug 09, 2017 at 05:02:10PM +0100, Stefan Hajnoczi wrote:
> John Snow is offline until Monday, QEMU 2.10-rc3 is due to be tagged on
> Tuesday, and so I've taken two patches John sent for 2.10 and addressed review
> comments.
> 
> Kevin Wolf (1):
>   IDE: test flush on empty CDROM
> 
> Stefan Hajnoczi (1):
>   IDE: Do not flush empty CDROM drives
> 
>  hw/ide/core.c| 10 +-
>  tests/ide-test.c | 19 +++
>  2 files changed, 28 insertions(+), 1 deletion(-)

John: I am sending a pull request to get this fix into -rc3 (Aug 15th).
Feel free to send a follow-up/revert if you want to make changes when
you get back online.

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-block] [Qemu-devel] [PATCH 01/12] qemu-iotests: remove dead code

[Paolo Bonzini]

On 10/08/2017 00:18, Eric Blake wrote:
> On 08/09/2017 04:54 PM, Paolo Bonzini wrote:
>> This includes shell function, shell variables, command line options
>> (randomize.awk does not exist) and conditions that can never be true
>> (./qemu does not exist anymore).
> 
> Can we point to a commit id where we stopped making ./qemu?  Is it still
> worth supporting a local symlink?
> 
>>
>> Signed-off-by: Paolo Bonzini 
>> ---
>>  tests/qemu-iotests/check | 36 +
>>  tests/qemu-iotests/common| 23 --
>>  tests/qemu-iotests/common.config | 26 ---
>>  tests/qemu-iotests/common.rc | 68 
>> 
>>  4 files changed, 1 insertion(+), 152 deletions(-)
>>
>> -#
>> -# - These can be added to $HOST_CONFIG_DIR (witch default to ./config)
> 
> Good riddance to the wrong word ('which' was intended)
> 
> Other than possibly still wanting ./qemu convenience, this looks like a
> reasonable cleanup.

In my reply yesterday, I forgot to note that the other current-directory
searches go away in patch 9 anyway. :)

Paolo

Re: [Qemu-block] [Qemu-devel] [PATCH 09/12] qemu-iotests: do not search for binaries in the current directory

[Markus Armbruster]

Paolo Bonzini  writes:

> Looking in the build root is enough.
>
> Signed-off-by: Paolo Bonzini 

It's actually *better*.  I hate it when tests pick up random garbage I
have lying around.

Re: [Qemu-block] [Qemu-devel] [PATCH 08/12] qemu-iotests: fix uninitialized variable

[Markus Armbruster]

Drive-by comment:

Paolo Bonzini  writes:

> The function is used in "common" but defined only after the file

"This variable"

> is sourced.
>
> Signed-off-by: Paolo Bonzini 

Re: [Qemu-block] [Qemu-devel] [RFC PATCH 01/56] qobject: Touch up comments to say @param instead of 'param'

[Markus Armbruster]

Eric Blake  writes:

> On 08/07/2017 09:45 AM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster 
>> ---
>>  qobject/qdict.c | 68 
>> -
>>  qobject/qlist.c |  2 +-
>>  2 files changed, 35 insertions(+), 35 deletions(-)
>> 
>> diff --git a/qobject/qdict.c b/qobject/qdict.c
>> index 576018e..d795079 100644
>> --- a/qobject/qdict.c
>> +++ b/qobject/qdict.c
>> @@ -116,13 +116,13 @@ static QDictEntry *qdict_find(const QDict *qdict,
>>  /**
>>   * qdict_put_obj(): Put a new QObject into the dictionary
>>   *
>> - * Insert the pair 'key:value' into 'qdict', if 'key' already exists
>> - * its 'value' will be replaced.
>> + * Insert the pair @key:@value into @qdict, if @key already exists
>> + * its value will be replaced.
>
> Maybe s/,/;/ while at it.

Or even *gasp* a period.

> Reviewed-by: Eric Blake 

Thanks!

Re: [Qemu-block] [PATCH v8 0/8] Optimize VMDK I/O by allocating multiple clusters

[Ashijeet Acharya]

On Thu, Aug 10, 2017 at 1:41 PM, Stefan Hajnoczi  wrote:

> On Thu, Jul 27, 2017 at 3:33 PM, Ashijeet Acharya
>  wrote:
> > Previously posted series patches:
> > v1 - http://lists.nongnu.org/archive/html/qemu-devel/2017-
> 03/msg02044.html
> > v2 - http://lists.nongnu.org/archive/html/qemu-devel/2017-
> 03/msg05080.html
> > v3 - http://lists.nongnu.org/archive/html/qemu-devel/2017-
> 04/msg00074.html
> > v4 - http://lists.nongnu.org/archive/html/qemu-devel/2017-
> 04/msg03851.html
> > v5 - http://lists.nongnu.org/archive/html/qemu-devel/2017-
> 06/msg00929.html
> > v6 - http://lists.nongnu.org/archive/html/qemu-devel/2017-
> 06/msg00947.html
> > v7 - http://lists.nongnu.org/archive/html/qemu-devel/2017-
> 06/msg06600.html
> >
> > This series helps to optimize the I/O performance of VMDK driver.
> >
> > Patch 1 helps us to move vmdk_find_offset_in_cluster.
> >
> > Patch 2 & 3 perform a simple function re-naming tasks.
> >
> > Patch 4 is used to factor out metadata loading code and implement it in
> separate
> > functions. This will help us to avoid code duplication in future patches
> of this
> > series.
> >
> > Patch 5 helps to set the upper limit of the bytes handled in one cycle.
> >
> > Patch 6 adds new functions to help us allocate multiple clusters
> according to
> > the size requested, perform COW if required and return the offset of the
> first
> > newly allocated cluster.
> >
> > Patch 7 changes the metadata update code to update the L2 tables for
> multiple
> > clusters at once.
> >
> > Patch 8 helps us to finally change vmdk_get_cluster_offset() to find
> cluster
> > offset only as cluster allocation task is now handled by
> vmdk_alloc_clusters()
> >
> > Optimization test results:
> >
> > This patch series improves 128 KB sequential write performance to an
> > empty VMDK file by 54%
> >
> > Benchmark command: ./qemu-img bench -w -c 1024 -s 128K -d 1 -t none -f
> > vmdk test.vmdk
> >
> > Changes in v8:
> > - fix minor variable naming issue in patch 6
>
> Fam: Ping?
>
> Ashijeet: Feel free to send a ping reply if no one reviews your
> patches within a few days.
>

Hi Stefan,

I had a chat with Fam on #qemu-block before submitting this series and he
said he will be merging it soon when the freeze is over (I am not sure if
it is yet) since all the patches are already reviewed :-)

Ashijeet

Re: [Qemu-block] [Qemu-devel] [PATCH v4 17/22] libqtest: Add qmp_args() helper

[Markus Armbruster]

Eric Blake  writes:

> On 08/09/2017 10:57 AM, Markus Armbruster wrote:
>
>> I don't like the qmp_args name.  It's not about arguments, it's about
>> sending a command with arguments.
>> 
>> I'm afraid I'm getting pretty thorougly confused by the evolving
>> interface.  So I stop and think how it should look like when done.
>
> At a bare minimum, I like your idea that we want:
>
> FOO_send() # sends a message, does not wait for a reply
> FOO_receive()  # reads one JSON object, returns QObject counterpart
> FOO() # combo of FOO_send() and FOO_receive()
>
> Then FOO_discard() is a convenient shorthand for QDECREF(FOO_receive()).

Yes.

Obvious names for the variations of FOO_send():

FOO_send()send a string exactly as is
FOO_sendf()   build a string from a template, interpolating from ...
FOO_vsendf()  ditto, interpolating from va_list

Since "send string exactly as is" is uncommon, using a longer name for
it would be okay.  Say FOO_send_string() or FOO_send_raw().

> Which FOO do we need? Right now, we have 'qmp' for anything using the
> global_qtest, 'qtest_qmp' for anything using an explicit QTestState (but
> we're arguing that is overkill), and 'qmp_fd' for anything using a raw
> fd (test-qga.c - and where I added 'qga' in 11/22, although that
> addition was local rather than in libqtest.h).

'qmp', 'qtest_qmp' and 'qmp_fd' sounds good.  I hope we can get rid of
'qtest_qmp'.

> I also just had an insight: it is possible to write a macro
> qmp_send(...) that will expand to qmp_send_cmd(name) if there is only
> one argument, but to qmp_send_cmdf(name, fmt, ...) if there is more than
> one argument (and later expose a public qmp_send_cmdv(name, fmt,
> va_list) as pair if it is needed, although right now it is not).
> Perhaps we can even make the one-arg form expand to qmp_send_cmdf(name,
> " "), if that's enough to silence gcc's -Wformat-zero-length, and if our
> underlying routines are smart enough to recognize a single-space
> argument as not being worthy of calling qobject_from_jsonf() (since
> qobject_from_jsonf() aborts rather than returning NULL when presented
> just whitespace).
>
> In fact, having a macro qmp_send(cmd, ...) expand to
> qmp_send_internal(cmd, " " __VA_ARGS__) might even do the trick without
> preprocessor magic of checking whether __VA_ARGS__ contains a comma
> (although it has the subtle limitation that if present, a format
> argument MUST be a string literal because we now rely on string
> concatenation with " " - although given gcc's -Wformat-nonliteral, we
> are probably okay with that limitation).
>
> So, with a nice macro, the bulk of the testsuite would just write in
> this style:
>
> qmp_send("foo");
> qmp_send("foo", "{literal_args...}");
> qmp_send("foo", "{'name':%s}", value);
>
> for send without waiting, or:
>
> obj = qmp("foo");
> obj = qmp(foo", "{literal_args...}");
> obj = qmp("foo", "{'name':%s}", value);
>
> where the result matters.  And the names we choose for internal
> implementation are no longer quite as important (although still helpful
> to get right).

Play with it, and we'll see how it turns out.

>> We need send and receive.  Convenience functions to do both, and to
>> receive and throw away are nice to have.
>
> Do we want both a convenience function to throw away a single read, AND
> a function to send a command + throw away a read? Prior to this series,
> we have qmp_discard_response() which is send + discard, but nothing that
> does plain discard; although plain discard is a better building block
> (precisely because then we don't have to duplicate all the different
> send forms) - the convenience of send+receive in one call should be
> limited to the most common case.

The interface should provide the basic building blocks.

Convenience functions are welcome when they help the interface' users
write clearer code.  Less code is often clearer code.

> Also, the qmp_eventwait() is a nice convenience function for wait in a
> loop until the received object matches a given name; whether we also
> need the convenience of qmp_eventwait_ref() is a bit more debatable
> (perhaps we could just as easily have qmp_event_wait() always return an
> object, and require the caller to QDECREF() it, for one less function to
> maintain).

See previous paragraph :)

>> We need qmp_raw().  We haven't found a need for a raw receive function.
>
> Yes, qmp_raw() (or maybe it should be named qmp_send_raw(), depending on
> whether the receive is coupled with it?) is our backdoor for sending
> JSON that does not match the normal {'execute':'name','arguments':{}}
> paradigm (and pretty much qmp-test.c, or maybe also test-qga.c, would be
> the only clients).

A "send raw and receive cooked" function feels slightly weird.  Since
it's used only rarely, we might want to avoid the weirdness and use a
pair of calls there.

Apropos raw vs. cooked: the "send cooked" functions convert single
quotes in the template to double quotes.  The "send raw" functions

Re: [Qemu-block] [PATCH v8 0/8] Optimize VMDK I/O by allocating multiple clusters

[Stefan Hajnoczi]

On Thu, Jul 27, 2017 at 3:33 PM, Ashijeet Acharya
 wrote:
> Previously posted series patches:
> v1 - http://lists.nongnu.org/archive/html/qemu-devel/2017-03/msg02044.html
> v2 - http://lists.nongnu.org/archive/html/qemu-devel/2017-03/msg05080.html
> v3 - http://lists.nongnu.org/archive/html/qemu-devel/2017-04/msg00074.html
> v4 - http://lists.nongnu.org/archive/html/qemu-devel/2017-04/msg03851.html
> v5 - http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg00929.html
> v6 - http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg00947.html
> v7 - http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg06600.html
>
> This series helps to optimize the I/O performance of VMDK driver.
>
> Patch 1 helps us to move vmdk_find_offset_in_cluster.
>
> Patch 2 & 3 perform a simple function re-naming tasks.
>
> Patch 4 is used to factor out metadata loading code and implement it in 
> separate
> functions. This will help us to avoid code duplication in future patches of 
> this
> series.
>
> Patch 5 helps to set the upper limit of the bytes handled in one cycle.
>
> Patch 6 adds new functions to help us allocate multiple clusters according to
> the size requested, perform COW if required and return the offset of the first
> newly allocated cluster.
>
> Patch 7 changes the metadata update code to update the L2 tables for multiple
> clusters at once.
>
> Patch 8 helps us to finally change vmdk_get_cluster_offset() to find cluster
> offset only as cluster allocation task is now handled by vmdk_alloc_clusters()
>
> Optimization test results:
>
> This patch series improves 128 KB sequential write performance to an
> empty VMDK file by 54%
>
> Benchmark command: ./qemu-img bench -w -c 1024 -s 128K -d 1 -t none -f
> vmdk test.vmdk
>
> Changes in v8:
> - fix minor variable naming issue in patch 6

Fam: Ping?

Ashijeet: Feel free to send a ping reply if no one reviews your
patches within a few days.

[Qemu-block] [PATCH] file-posix: Clear out first sector in hdev_create

[Fam Zheng]

People get surprised when, after "qemu-imc create -f raw /dev/sdX", they
still see qcow2 with "qemu-img info", if previously the bdev had a qcow2
header. While this is natural because raw doesn't need to write any
magic bytes during creation, hdev_create is free to clear out the first
sector to make sure the stale qcow2 header doesn't cause such a
confusion.

Signed-off-by: Fam Zheng 
---
 block/file-posix.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index f4de022ae0..1d8ef6f873 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2703,6 +2703,17 @@ static int hdev_create(const char *filename, QemuOpts 
*opts,
 ret = -ENOSPC;
 }
 
+if (total_size) {
+int64_t zero_size = MIN(BDRV_SECTOR_SIZE, total_size);
+uint8_t *buf;
+if (lseek(fd, 0, SEEK_SET) == -1) {
+ret = -errno;
+} else {
+buf = g_malloc0(zero_size);
+ret = qemu_write_full(fd, buf, zero_size);
+g_free(buf);
+}
+}
 qemu_close(fd);
 return ret;
 }
-- 
2.13.3