[Qemu-devel] [PATCH v2 2/4] slirp: avoid use-after-free in slirp_pollfds_poll() if soread() returns an error

[Steven Luo]

From: Steven Luo 

Samuel Thibault pointed out that it's possible that slirp_pollfds_poll()
will try to use a socket even after soread() returns an error, resulting
in an use-after-free if the socket was removed while handling the error.
Avoid this by refusing to continue to work with the socket in this case.

Signed-off-by: Steven Luo 
---
It seems to me that it might be possible to trigger this use-after-free
even without the following patches, as tcp_sockclosed() (which soread()
currently calls when it gets an error in recv()) frees the socket in
certain cases.  The remaining patches in this series probably make this
much easier to trigger, though.

 slirp/slirp.c  | 12 +++-
 slirp/socket.c | 17 +++--
 slirp/socket.h |  2 +-
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/slirp/slirp.c b/slirp/slirp.c
index fef526c..9f4bea3 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -534,7 +534,12 @@ void slirp_pollfds_poll(GArray *pollfds, int select_error)
  * test for G_IO_IN below if this succeeds
  */
 if (revents & G_IO_PRI) {
-sorecvoob(so);
+ret = sorecvoob(so);
+if (ret < 0) {
+/* Socket error might have resulted in the socket being
+ * removed, do not try to do anything more with it. */
+continue;
+}
 }
 /*
  * Check sockets for reading
@@ -553,6 +558,11 @@ void slirp_pollfds_poll(GArray *pollfds, int select_error)
 if (ret > 0) {
 tcp_output(sototcpcb(so));
 }
+if (ret < 0) {
+/* Socket error might have resulted in the socket being
+ * removed, do not try to do anything more with it. */
+continue;
+}
 }
 
 /*
diff --git a/slirp/socket.c b/slirp/socket.c
index b836c42..7f022a6 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -260,10 +260,11 @@ err:
  * so when OOB data arrives, we soread() it and everything
  * in the send buffer is sent as urgent data
  */
-void
+int
 sorecvoob(struct socket *so)
 {
struct tcpcb *tp = sototcpcb(so);
+   int ret;
 
DEBUG_CALL("sorecvoob");
DEBUG_ARG("so = %p", so);
@@ -276,11 +277,15 @@ sorecvoob(struct socket *so)
 * urgent data, or the read() doesn't return all the
 * urgent data.
 */
-   soread(so);
-   tp->snd_up = tp->snd_una + so->so_snd.sb_cc;
-   tp->t_force = 1;
-   tcp_output(tp);
-   tp->t_force = 0;
+   ret = soread(so);
+   if (ret > 0) {
+   tp->snd_up = tp->snd_una + so->so_snd.sb_cc;
+   tp->t_force = 1;
+   tcp_output(tp);
+   tp->t_force = 0;
+   }
+
+   return ret;
 }
 
 /*
diff --git a/slirp/socket.h b/slirp/socket.h
index e9c9b05..7dca506 100644
--- a/slirp/socket.h
+++ b/slirp/socket.h
@@ -127,7 +127,7 @@ struct socket *solookup(struct socket **, struct socket *,
 struct socket *socreate(Slirp *);
 void sofree(struct socket *);
 int soread(struct socket *);
-void sorecvoob(struct socket *);
+int sorecvoob(struct socket *);
 int sosendoob(struct socket *);
 int sowrite(struct socket *);
 void sorecvfrom(struct socket *);
-- 
2.1.4

[Qemu-devel] [PATCH v2 4/4] slirp: handle deferred ECONNREFUSED on non-blocking TCP sockets

[Steven Luo]

From: Steven Luo 

slirp currently only handles ECONNREFUSED in the case where connect()
returns immediately with that error; since we use non-blocking sockets,
most of the time we won't receive the error until we later try to read
from the socket.  Ensure that we deliver the appropriate RST to the
guest in this case.

Signed-off-by: Steven Luo 
Reviewed-by: Edgar E. Iglesias 
---
v1->v2:
* added Reviewed-by line

 slirp/socket.c| 2 +-
 slirp/tcp_input.c | 6 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/slirp/socket.c b/slirp/socket.c
index 0d67b12..bd97b2d 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -188,7 +188,7 @@ soread(struct socket *so)
DEBUG_MISC((dfd, " --- soread() disconnected, nn = %d, 
errno = %d-%s\n", nn, errno,strerror(errno)));
sofcantrcvmore(so);
 
-   if (err == ECONNRESET
+   if (err == ECONNRESET || err == ECONNREFUSED
|| err == ENOTCONN || err == EPIPE) {
tcp_drop(sototcpcb(so), err);
} else {
diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c
index 1fcca30..5433e7f 100644
--- a/slirp/tcp_input.c
+++ b/slirp/tcp_input.c
@@ -725,6 +725,12 @@ findso:
so->so_ti = ti;
tp->t_timer[TCPT_KEEP] = TCPTV_KEEP_INIT;
tp->t_state = TCPS_SYN_RECEIVED;
+   /*
+* Initialize receive sequence numbers now so that we can send a
+* valid RST if the remote end rejects our connection.
+*/
+   tp->irs = ti->ti_seq;
+   tcp_rcvseqinit(tp);
tcp_template(tp);
  }
  return;
-- 
2.1.4

[Qemu-devel] [PATCH v2 3/4] slirp: Propagate host TCP RST to the guest.

[Steven Luo]

From: Edgar E. Iglesias 

When the host aborts (RST) it's side of a TCP connection we need to
propagate that RST to the guest. The current code can leave such guest
connections dangling forever. Spotted by Jason Wessel.

[ste...@steven676.net: coding style adjustments]
Signed-off-by: Steven Luo 
---
v1->v2:
* fix attribution

 slirp/socket.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/slirp/socket.c b/slirp/socket.c
index 7f022a6..0d67b12 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -176,9 +176,24 @@ soread(struct socket *so)
if (nn < 0 && (errno == EINTR || errno == EAGAIN))
return 0;
else {
+   int err;
+   socklen_t slen = sizeof err;
+
+   err = errno;
+   if (nn == 0) {
+   getsockopt(so->s, SOL_SOCKET, SO_ERROR,
+  , );
+   }
+
DEBUG_MISC((dfd, " --- soread() disconnected, nn = %d, 
errno = %d-%s\n", nn, errno,strerror(errno)));
sofcantrcvmore(so);
-   tcp_sockclosed(sototcpcb(so));
+
+   if (err == ECONNRESET
+   || err == ENOTCONN || err == EPIPE) {
+   tcp_drop(sototcpcb(so), err);
+   } else {
+   tcp_sockclosed(sototcpcb(so));
+   }
return -1;
}
}
-- 
2.1.4

[Qemu-devel] [PATCH v2 1/4] slirp: don't crash when tcp_sockclosed() is called with a NULL tp

[Steven Luo]

From: Steven Luo 

Signed-off-by: Steven Luo 
Reviewed-by: Edgar E. Iglesias 
---
v1->v2:
* added Reviewed-by line

 slirp/tcp_subr.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
index dbfd2c6..32ff452 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -356,6 +356,10 @@ tcp_sockclosed(struct tcpcb *tp)
DEBUG_CALL("tcp_sockclosed");
DEBUG_ARG("tp = %p", tp);
 
+   if (!tp) {
+   return;
+   }
+
switch (tp->t_state) {
 
case TCPS_CLOSED:
@@ -374,8 +378,7 @@ tcp_sockclosed(struct tcpcb *tp)
tp->t_state = TCPS_LAST_ACK;
break;
}
-   if (tp)
-   tcp_output(tp);
+   tcp_output(tp);
 }
 
 /*
-- 
2.1.4

[Qemu-devel] [PATCH v2 0/4] slirp: deliver received TCP RSTs to the guest

[Steven Luo]

Changes from v1:

* added patch 2, a fix for an use-after-free exposed by this series
  (thanks Samuel Thibault)
* incorporated Reviewed-by lines
* attributed patches correctly

-Steven Luo

===

QEMU's user-mode networking does not currently pass received TCP RSTs to
guests, meaning that applications in guests hang if the remote server
rejects their network connections.  This is particularly noticeable when
IPv6 is enabled, the guest is configured to prefer IPv6 and the remote
server rejects IPv6 connections (segment-data.zqtk.net is one example),
but the bug appears to be longstanding and affects TCP over IPv4 as
well.

There are four short patches in this series.  The first and second fix
crashes which would be exposed by the rest of the series.  The third,
which fixes delivery of an RST interrupting an already-established TCP
connection, was submitted by Edgar Iglesias in 2008 and appears to have
been missed then.  The last patch fixes the case where the remote end
sends RST in reply to our SYN (rejects our incoming connection attempt).

Lightly tested on a Linux host with Linux and Windows 7 guests.

Edgar E. Iglesias (1):
  slirp: Propagate host TCP RST to the guest.

Steven Luo (3):
  slirp: don't crash when tcp_sockclosed() is called with a NULL tp
  slirp: avoid use-after-free in slirp_pollfds_poll() if soread()
returns an error
  slirp: handle deferred ECONNREFUSED on non-blocking TCP sockets

 slirp/slirp.c | 12 +++-
 slirp/socket.c| 34 +++---
 slirp/socket.h|  2 +-
 slirp/tcp_input.c |  6 ++
 slirp/tcp_subr.c  |  7 +--
 5 files changed, 50 insertions(+), 11 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH V3] tap: vhost busy polling support

[Jason Wang]

This patch add the capability of basic vhost net busy polling which is
supported by recent kernel. User could configure the maximum number of
us that could be spent on busy polling through a new property of tap
"vhost-poll-us".

Signed-off-by: Jason Wang 
---
 hw/net/vhost_net.c|  2 +-
 hw/scsi/vhost-scsi.c  |  2 +-
 hw/virtio/vhost-backend.c |  8 
 hw/virtio/vhost.c | 40 ++-
 include/hw/virtio/vhost-backend.h |  3 +++
 include/hw/virtio/vhost.h |  3 ++-
 include/net/vhost_net.h   |  1 +
 net/tap.c | 10 --
 net/vhost-user.c  |  1 +
 qapi-schema.json  |  6 +-
 qemu-options.hx   |  3 +++
 11 files changed, 72 insertions(+), 7 deletions(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 6e1032f..1840c73 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -166,7 +166,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
 }
 
 r = vhost_dev_init(>dev, options->opaque,
-   options->backend_type);
+   options->backend_type, options->busyloop_timeout);
 if (r < 0) {
 goto fail;
 }
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 9261d51..2a00f2f 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -248,7 +248,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error 
**errp)
 s->dev.backend_features = 0;
 
 ret = vhost_dev_init(>dev, (void *)(uintptr_t)vhostfd,
- VHOST_BACKEND_TYPE_KERNEL);
+ VHOST_BACKEND_TYPE_KERNEL, 0);
 if (ret < 0) {
 error_setg(errp, "vhost-scsi: vhost initialization failed: %s",
strerror(-ret));
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index b358902..d62372e 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -138,6 +138,12 @@ static int vhost_kernel_set_vring_call(struct vhost_dev 
*dev,
 return vhost_kernel_call(dev, VHOST_SET_VRING_CALL, file);
 }
 
+static int vhost_kernel_set_vring_busyloop_timeout(struct vhost_dev *dev,
+   struct vhost_vring_state *s)
+{
+return vhost_kernel_call(dev, VHOST_SET_VRING_BUSYLOOP_TIMEOUT, s);
+}
+
 static int vhost_kernel_set_features(struct vhost_dev *dev,
  uint64_t features)
 {
@@ -185,6 +191,8 @@ static const VhostOps kernel_ops = {
 .vhost_get_vring_base = vhost_kernel_get_vring_base,
 .vhost_set_vring_kick = vhost_kernel_set_vring_kick,
 .vhost_set_vring_call = vhost_kernel_set_vring_call,
+.vhost_set_vring_busyloop_timeout =
+vhost_kernel_set_vring_busyloop_timeout,
 .vhost_set_features = vhost_kernel_set_features,
 .vhost_get_features = vhost_kernel_get_features,
 .vhost_set_owner = vhost_kernel_set_owner,
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 4400718..ebf8b08 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -964,6 +964,28 @@ static void vhost_eventfd_del(MemoryListener *listener,
 {
 }
 
+static int vhost_virtqueue_set_busyloop_timeout(struct vhost_dev *dev,
+int n, uint32_t timeout)
+{
+int vhost_vq_index = dev->vhost_ops->vhost_get_vq_index(dev, n);
+struct vhost_vring_state state = {
+.index = vhost_vq_index,
+.num = timeout,
+};
+int r;
+
+if (!dev->vhost_ops->vhost_set_vring_busyloop_timeout) {
+return -EINVAL;
+}
+
+r = dev->vhost_ops->vhost_set_vring_busyloop_timeout(dev, );
+if (r) {
+return r;
+}
+
+return 0;
+}
+
 static int vhost_virtqueue_init(struct vhost_dev *dev,
 struct vhost_virtqueue *vq, int n)
 {
@@ -994,7 +1016,7 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue 
*vq)
 }
 
 int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
-   VhostBackendType backend_type)
+   VhostBackendType backend_type, uint32_t busyloop_timeout)
 {
 uint64_t features;
 int i, r;
@@ -1035,6 +1057,17 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
 goto fail_vq;
 }
 }
+
+if (busyloop_timeout) {
+for (i = 0; i < hdev->nvqs; ++i) {
+r = vhost_virtqueue_set_busyloop_timeout(hdev, hdev->vq_index + i,
+ busyloop_timeout);
+if (r < 0) {
+goto fail_busyloop;
+}
+}
+}
+
 hdev->features = features;
 
 hdev->memory_listener = (MemoryListener) {
@@ -1077,6 +1110,11 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
 hdev->memory_changed = false;
 memory_listener_register(>memory_listener, 

Re: [Qemu-devel] [PATCH V2] tap: vhost busy polling support

[Jason Wang]

On 04/07/2016 01:16 AM, Greg Kurz wrote:
> On Wed, 6 Apr 2016 09:05:21 -0600
> Eric Blake  wrote:
>
>> On 04/06/2016 03:15 AM, Jason Wang wrote:
>>> This patch add the capability of basic vhost net busy polling which is
>>> supported by recent kernel. User could configure the maximum number of
>>> us that could be spent on busy polling through a new property of tap
>>> "vhost-poll-us".
>>>
>>> Signed-off-by: Jason Wang 
>>> ---  
>>> +++ b/qapi-schema.json
>>> @@ -2531,6 +2531,9 @@
>>>  #
>>>  # @queues: #optional number of queues to be created for multiqueue capable 
>>> tap
>>>  #
>>> +# @vhost-poll-us: #optional maximum number of microseconds that could
>>> +# be spent on busy polling for vhost net  
>> Missing a '(since 2.X)' designation (not sure if X should be 6 or 7)
>>
> According to another mail from Jason, this isn't 2.6 material.
>
> --
> Greg

Right, it's for 2.7.

Will post V3

Thanks

[Qemu-devel] [PATCH] doc: Revert swap of NBD_REP_SERVER fields in NBD_OPT_GO

[Eric Blake]

Commit 730c5830 included a reordering of the fields of
NBD_REP_SERVER, under the guise of putting the variable-sized
data last.  However, this makes life harder for sharing
code - the regular NBD_REP_SERVER used by NBD_OPT_LIST
already has _two_ variable-sized fields: name, and an optional
string of additional information, and the wording of the
surrounding text implied that NBD_OPT_GO was just re-purposing
the tail end as binary data instead of a string, not
reorganizing all the fields.

Furthermore, sticking the same fields last in both NBD_OPT_GO
and NBD_OPT_EXPORT_NAME means that any other future handshake
extension that sends a few more bytes of data behind the
transmission flags can do so in both places without reordering
the first half of NBD_REP_SERVER (such an extension is already
envisioned, where the client and server agree to have the server
advertise minimum, preferred, and maximum block sizes).

Signed-off-by: Eric Blake 
---

I'm trying to implement NBD_OPT_GO in qemu, and found the original
order easier to support than the swapped order.

 doc/proto.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/proto.md b/doc/proto.md
index 7b36be4..0775360 100644
--- a/doc/proto.md
+++ b/doc/proto.md
@@ -744,13 +744,13 @@ Therefore these commands share common documentation.
 in place of the default UTF-8 free-form string). The option reply length
 MUST be *length of name* + 14, and the option data has the following 
layout:

-- 64 bits, size of the export in bytes (unsigned)
-- 16 bits, transmission flags.
 - 32 bits, length of name (unsigned)
 - Name of the export. This name MAY be different from the one
   given in the `NBD_OPT_INFO` or `NBD_OPT_GO` option in case the
   server has multiple alternate names for a single export, or a
   default export was specified.
+- 64 bits, size of the export in bytes (unsigned)
+- 16 bits, transmission flags.

 The server MUST NOT fail an NBD_OPT_GO sent with the same parameters
 as a previous NBD_OPT_INFO which returned successfully (i.e. with
-- 
2.5.5

Re: [Qemu-devel] [PATCH] xen: piix reuse pci generic class init function

[Michael S. Tsirkin]

On Wed, Apr 06, 2016 at 04:56:12PM -0700, Stefano Stabellini wrote:
> On Sun, 3 Apr 2016, Michael S. Tsirkin wrote:
> > piix3_ide_xen_class_init is identical to piix3_ide_class_init
> > except it's buggy as it does not set exit and does not disable
> > hotplug properly.
> > 
> > Switch to the generic one.
> > 
> > Reviewed-by: Stefano Stabellini 
> > Signed-off-by: Michael S. Tsirkin 
> 
> Hey John,
> 
> are you going to take the patch or do you want me to handle it?
> 
> Cheers,
> 
> Stefano

it's in my tree already.

> 
> >  hw/ide/piix.c | 14 +-
> >  1 file changed, 1 insertion(+), 13 deletions(-)
> > 
> > diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> > index df46147..0a4cbcb 100644
> > --- a/hw/ide/piix.c
> > +++ b/hw/ide/piix.c
> > @@ -258,22 +258,10 @@ static const TypeInfo piix3_ide_info = {
> >  .class_init= piix3_ide_class_init,
> >  };
> >  
> > -static void piix3_ide_xen_class_init(ObjectClass *klass, void *data)
> > -{
> > -DeviceClass *dc = DEVICE_CLASS(klass);
> > -PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> > -
> > -k->realize = pci_piix_ide_realize;
> > -k->vendor_id = PCI_VENDOR_ID_INTEL;
> > -k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1;
> > -k->class_id = PCI_CLASS_STORAGE_IDE;
> > -set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
> > -}
> > -
> >  static const TypeInfo piix3_ide_xen_info = {
> >  .name  = "piix3-ide-xen",
> >  .parent= TYPE_PCI_IDE,
> > -.class_init= piix3_ide_xen_class_init,
> > +.class_init= piix3_ide_class_init,
> >  };
> >  
> >  static void piix4_ide_class_init(ObjectClass *klass, void *data)
> > -- 
> > MST
> > 

[Qemu-devel] [PATCH 3/3] use g_path_get_basename instead of basename

[Wei Jiangang]

Using g_strdup and g_basename to get the last component
of filename is not the best solution,
Only g_path_get_basename can achive the purpose we want.

Signed-off-by: Wei Jiangang 
---
 fsdev/virtfs-proxy-helper.c |  6 +-
 hw/9pfs/9p-local.c  |  6 +++---
 hw/vfio/pci.c   |  6 --
 hw/vfio/platform.c  |  6 --
 qemu-io.c   | 33 -
 qemu-nbd.c  |  4 +++-
 qga/commands-posix.c|  4 ++--
 7 files changed, 41 insertions(+), 24 deletions(-)

diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index 54f7ad1..a0d6118 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -787,6 +787,8 @@ error:
 
 static void usage(char *prog)
 {
+char *base_filename = g_path_get_basename(prog);
+
 fprintf(stderr, "usage: %s\n"
 " -p|--path  9p path to export\n"
 " {-f|--fd } socket file descriptor to be 
used\n"
@@ -795,7 +797,9 @@ static void usage(char *prog)
 " access to this socket\n"
 " \tNote: -s & -f can not be used together\n"
 " [-n|--nodaemon] Run as a normal program\n",
-basename(prog));
+base_filename);
+
+g_free(base_filename);
 }
 
 static int process_reply(int sock, int type,
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 16f45f4..4e6c17a 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -160,17 +160,17 @@ static int local_create_mapped_attr_dir(FsContext *ctx, 
const char *path)
 {
 int err;
 char *attr_dir;
-char *tmp_path = g_strdup(path);
+char *base_filename = g_path_get_basename(path);
 
 attr_dir = g_strdup_printf("%s/%s/%s",
- ctx->fs_root, dirname(tmp_path), VIRTFS_META_DIR);
+ ctx->fs_root, base_filename, VIRTFS_META_DIR);
 
 err = mkdir(attr_dir, 0700);
 if (err < 0 && errno == EEXIST) {
 err = 0;
 }
 g_free(attr_dir);
-g_free(tmp_path);
+g_free(base_filename);
 return err;
 }
 
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index d091d8c..d23b871 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2413,7 +2413,7 @@ static int vfio_initfn(PCIDevice *pdev)
 return -errno;
 }
 
-vdev->vbasedev.name = g_strdup(basename(vdev->vbasedev.sysfsdev));
+vdev->vbasedev.name = g_path_get_basename(vdev->vbasedev.sysfsdev);
 vdev->vbasedev.ops = _pci_ops;
 vdev->vbasedev.type = VFIO_DEVICE_TYPE_PCI;
 
@@ -2428,11 +2428,13 @@ static int vfio_initfn(PCIDevice *pdev)
 
 group_path[len] = 0;
 
-group_name = basename(group_path);
+group_name = g_path_get_basename(group_path);
 if (sscanf(group_name, "%d", ) != 1) {
 error_report("vfio: error reading %s: %m", group_path);
+g_free(group_name);
 return -errno;
 }
+g_free(group_name);
 
 trace_vfio_initfn(vdev->vbasedev.name, groupid);
 
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 1798a00..47134db 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -557,7 +557,7 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
 /* @sysfsdev takes precedence over @host */
 if (vbasedev->sysfsdev) {
 g_free(vbasedev->name);
-vbasedev->name = g_strdup(basename(vbasedev->sysfsdev));
+vbasedev->name = g_path_get_basename(vbasedev->sysfsdev);
 } else {
 if (!vbasedev->name || strchr(vbasedev->name, '/')) {
 return -EINVAL;
@@ -584,11 +584,13 @@ static int vfio_base_device_init(VFIODevice *vbasedev)
 
 group_path[len] = 0;
 
-group_name = basename(group_path);
+group_name = g_path_get_basename(group_path);
 if (sscanf(group_name, "%d", ) != 1) {
 error_report("vfio: error reading %s: %m", group_path);
+g_free(group_name);
 return -errno;
 }
+g_free(group_name);
 
 trace_vfio_platform_base_device_init(vbasedev->name, groupid);
 
diff --git a/qemu-io.c b/qemu-io.c
index 0a738f1..2f5c616 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -249,6 +249,12 @@ static char *get_prompt(void)
 return prompt;
 }
 
+static void cleanup_and_exit(int status)
+{
+g_free(progname);
+exit(status);
+}
+
 static void GCC_FMT_ATTR(2, 3) readline_printf_func(void *opaque,
 const char *fmt, ...)
 {
@@ -440,7 +446,7 @@ int main(int argc, char **argv)
 signal(SIGPIPE, SIG_IGN);
 #endif
 
-progname = basename(argv[0]);
+progname = g_path_get_basename(argv[0]);
 qemu_init_exec_dir(argv[0]);
 
 module_call_init(MODULE_INIT_QOM);
@@ -459,7 +465,7 @@ int main(int argc, char **argv)
 case 'd':
 if (bdrv_parse_discard_flags(optarg, ) < 0) {
 error_report("Invalid discard option: %s", optarg);
-exit(1);
+cleanup_and_exit(1);
 }
 break;
 case 'f':
@@ -480,26 +486,26 @@ int main(int 

[Qemu-devel] [PATCH 0/3] bitsized task for glib conversion

[Wei Jiangang]

The series used to change basename and dirname to
g_path_get_basename() and g_path_get_dirname() respectively.

Refer to http://wiki.qemu.org/BiteSizedTasks#API_conversion

*** BLURB HERE ***

Wei Jiangang (3):
  linux-user: complete omission of removing uses of strdup
  use g_path_get_dirname instead of dirname
  use g_path_get_basename instead of basename

 fsdev/virtfs-proxy-helper.c |  6 +-
 hw/9pfs/9p-local.c  |  6 +++---
 hw/vfio/pci.c   |  6 --
 hw/vfio/platform.c  |  6 --
 linux-user/elfload.c|  7 ++-
 os-posix.c  |  3 ++-
 qemu-io.c   | 33 -
 qemu-nbd.c  |  4 +++-
 qga/commands-posix.c|  4 ++--
 util/oslib-posix.c  |  4 +++-
 10 files changed, 48 insertions(+), 31 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH 1/3] linux-user: complete omission of removing uses of strdup

[Wei Jiangang]

The 900cfbc just removed two unchecked uses of strdup
in fill_psinfo and missed the rest in core_dump_filename.
This patch fixes it.

Signed-off-by: Wei Jiangang 
---
 linux-user/elfload.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index e47caff..6373320 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2718,7 +2718,6 @@ static int core_dump_filename(const TaskState *ts, char 
*buf,
   size_t bufsize)
 {
 char timestamp[64];
-char *filename = NULL;
 char *base_filename = NULL;
 struct timeval tv;
 struct tm tm;
@@ -2731,14 +2730,12 @@ static int core_dump_filename(const TaskState *ts, char 
*buf,
 return (-1);
 }
 
-filename = strdup(ts->bprm->filename);
-base_filename = strdup(basename(filename));
+base_filename = g_path_get_basename(ts->bprm->filename);
 (void) strftime(timestamp, sizeof (timestamp), "%Y%m%d-%H%M%S",
 localtime_r(_sec, ));
 (void) snprintf(buf, bufsize, "qemu_%s_%s_%d.core",
 base_filename, timestamp, (int)getpid());
-free(base_filename);
-free(filename);
+g_free(base_filename);
 
 return (0);
 }
-- 
1.9.3

[Qemu-devel] [PATCH 2/3] use g_path_get_dirname instead of dirname

[Wei Jiangang]

Use g_path_get_basename to get the directory components of
a file name, and free its return when no longer needed.

Signed-off-by: Wei Jiangang 
---
 os-posix.c | 3 ++-
 util/oslib-posix.c | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 107fde3..bcaef17 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -90,7 +90,7 @@ char *os_find_datadir(void)
 if (exec_dir == NULL) {
 return NULL;
 }
-dir = dirname(exec_dir);
+dir = g_path_get_dirname(exec_dir);
 
 max_len = strlen(dir) +
 MAX(strlen(SHARE_SUFFIX), strlen(BUILD_SUFFIX)) + 1;
@@ -104,6 +104,7 @@ char *os_find_datadir(void)
 }
 }
 
+g_free(dir);
 g_free(exec_dir);
 return res;
 }
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 20ca141..1d3248c 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -312,9 +312,11 @@ void qemu_init_exec_dir(const char *argv0)
 return;
 }
 }
-dir = dirname(p);
+dir = g_path_get_dirname(p);
 
 pstrcpy(exec_dir, sizeof(exec_dir), dir);
+
+g_free(dir);
 }
 
 char *qemu_get_exec_dir(void)
-- 
1.9.3

Re: [Qemu-devel] [PATCH qemu v15 15/17] spapr_pci: Get rid of dma_loibn

[David Gibson]

s/dma_loibn/dma_liobn/ in subject line.

On Mon, Apr 04, 2016 at 07:33:44PM +1000, Alexey Kardashevskiy wrote:
> We are going to have 2 DMA windows which LIOBNs are calculated from
> the PHB index and the window number using the SPAPR_PCI_LIOBN macro
> so there is no actual use for dma_liobn.
> 
> This replaces dma_liobn with SPAPR_PCI_LIOBN. This marks it as unused
> in the migration stream. This renames dma_liobn to _dma_liobn as we have
> to keep the property for the CLI compatibility and we need a storage
> for it, although it has never really been used.
> 
> Signed-off-by: Alexey Kardashevskiy 

This doesn't quite make sense.  We can't really do that without
entirely removing support for PHBs without an 'index' value.
Basically the idea of the PHB config parameters what that you either
specified just "index" or you specified *all* the relevant addresses.
Removing option 2 might be a reasonable idea, but it shouldn't just be
done as a side effect of this other change.  With this patch the
"specify everything" approach still has code, but can't work, because
such a device will never get a reasonable liobn (or worse, it might
get a duplicate liobn, because the index isn't verified in this mode).

Then again.. the "index" approach has also bitten us with the problem
of the not-quite-big-enough MMIO space per-PHB, so I'm not entirely
sure that making it the only choice is the right way to go either.

The short term approach to handle DDW might be to instead add a
dma64_liobn property.

> ---
> Changes:
> v15:
> * new to the series
> ---
>  hw/ppc/spapr_pci.c  | 17 ++---
>  include/hw/pci-host/spapr.h |  2 +-
>  2 files changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index f864fde..d4bdb27 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1306,7 +1306,7 @@ static void spapr_phb_realize(DeviceState *dev, Error 
> **errp)
>  if (sphb->index != (uint32_t)-1) {
>  hwaddr windows_base;
>  
> -if ((sphb->buid != (uint64_t)-1) || (sphb->dma_liobn != (uint32_t)-1)
> +if ((sphb->buid != (uint64_t)-1)
>  || (sphb->mem_win_addr != (hwaddr)-1)
>  || (sphb->io_win_addr != (hwaddr)-1)) {
>  error_setg(errp, "Either \"index\" or other parameters must"
> @@ -1321,7 +1321,6 @@ static void spapr_phb_realize(DeviceState *dev, Error 
> **errp)
>  }
>  
>  sphb->buid = SPAPR_PCI_BASE_BUID + sphb->index;
> -sphb->dma_liobn = SPAPR_PCI_LIOBN(sphb->index, 0);
>  
>  windows_base = SPAPR_PCI_WINDOW_BASE
>  + sphb->index * SPAPR_PCI_WINDOW_SPACING;
> @@ -1334,11 +1333,6 @@ static void spapr_phb_realize(DeviceState *dev, Error 
> **errp)
>  return;
>  }
>  
> -if (sphb->dma_liobn == (uint32_t)-1) {
> -error_setg(errp, "LIOBN not specified for PHB");
> -return;
> -}
> -
>  if (sphb->mem_win_addr == (hwaddr)-1) {
>  error_setg(errp, "Memory window address not specified for PHB");
>  return;
> @@ -1453,7 +1447,7 @@ static void spapr_phb_realize(DeviceState *dev, Error 
> **errp)
>  }
>  }
>  
> -tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn);
> +tcet = spapr_tce_new_table(DEVICE(sphb), SPAPR_PCI_LIOBN(sphb->index, 
> 0));
>  if (!tcet) {
>  error_setg(errp, "Unable to create TCE table for %s",
> sphb->dtbusname);
> @@ -1479,7 +1473,8 @@ static int spapr_phb_children_reset(Object *child, void 
> *opaque)
>  
>  void spapr_phb_dma_reset(sPAPRPHBState *sphb)
>  {
> -sPAPRTCETable *tcet = spapr_tce_find_by_liobn(sphb->dma_liobn);
> +uint32_t liobn = SPAPR_PCI_LIOBN(sphb->index, 0);
> +sPAPRTCETable *tcet = spapr_tce_find_by_liobn(liobn);
>  
>  if (tcet && tcet->enabled) {
>  spapr_tce_table_disable(tcet);
> @@ -1507,7 +1502,7 @@ static void spapr_phb_reset(DeviceState *qdev)
>  static Property spapr_phb_properties[] = {
>  DEFINE_PROP_UINT32("index", sPAPRPHBState, index, -1),
>  DEFINE_PROP_UINT64("buid", sPAPRPHBState, buid, -1),
> -DEFINE_PROP_UINT32("liobn", sPAPRPHBState, dma_liobn, -1),
> +DEFINE_PROP_UINT32("liobn", sPAPRPHBState, _dma_liobn, -1),
>  DEFINE_PROP_UINT64("mem_win_addr", sPAPRPHBState, mem_win_addr, -1),
>  DEFINE_PROP_UINT64("mem_win_size", sPAPRPHBState, mem_win_size,
> SPAPR_PCI_MMIO_WIN_SIZE),
> @@ -1595,7 +1590,7 @@ static const VMStateDescription vmstate_spapr_pci = {
>  .post_load = spapr_pci_post_load,
>  .fields = (VMStateField[]) {
>  VMSTATE_UINT64_EQUAL(buid, sPAPRPHBState),
> -VMSTATE_UINT32_EQUAL(dma_liobn, sPAPRPHBState),
> +VMSTATE_UNUSED(4), /* former dma_liobn */
>  VMSTATE_UINT64_EQUAL(mem_win_addr, sPAPRPHBState),
>  VMSTATE_UINT64_EQUAL(mem_win_size, sPAPRPHBState),
>  VMSTATE_UINT64_EQUAL(io_win_addr, sPAPRPHBState),
> diff --git 

Re: [Qemu-devel] [PATCH qemu v15 14/17] spapr_iommu, vfio, memory: Notify IOMMU about starting/stopping being used by VFIO

[David Gibson]

On Mon, Apr 04, 2016 at 07:33:43PM +1000, Alexey Kardashevskiy wrote:
> The sPAPR TCE tables manage 2 copies when VFIO is using an IOMMU -
> a guest view of the table and a hardware TCE table. If there is no VFIO
> presense in the address space, then just the guest view is used, if
> this is the case, it is allocated in the KVM. However since there is no
> support yet for VFIO in KVM TCE hypercalls, when we start using VFIO,
> we need to move the guest view from KVM to the userspace; and we need
> to do this for every IOMMU on a bus with VFIO devices.
> 
> This adds vfio_start/vfio_stop callbacks in MemoryRegionIOMMUOps to
> notifiy IOMMU about changing environment so it can reallocate the table
> to/from KVM or (when available) hook the IOMMU groups with the logical
> bus (LIOBN) in the KVM.
> 
> This removes explicit spapr_tce_set_need_vfio() call from PCI hotplug
> path as the new callbacks do this better - they notify IOMMU at
> the exact moment when the configuration is changed, and this also
> includes the case of PCI hot unplug.
> 
> As there can be multiple containers attached to the same PHB/LIOBN,
> this replaces the @need_vfio flag in sPAPRTCETable with the counter
> of VFIO users.
> 
> Signed-off-by: Alexey Kardashevskiy 

This looks correct, but there's one remaining ugly.

> ---
> Changes:
> v15:
> * s/need_vfio/vfio-Users/g
> ---
>  hw/ppc/spapr_iommu.c   | 30 --
>  hw/ppc/spapr_pci.c |  6 --
>  hw/vfio/common.c   |  9 +
>  include/exec/memory.h  |  4 
>  include/hw/ppc/spapr.h |  2 +-
>  5 files changed, 34 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index c945dba..ea09414 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -155,6 +155,16 @@ static uint64_t spapr_tce_get_page_sizes(MemoryRegion 
> *iommu)
>  return 1ULL << tcet->page_shift;
>  }
>  
> +static void spapr_tce_vfio_start(MemoryRegion *iommu)
> +{
> +spapr_tce_set_need_vfio(container_of(iommu, sPAPRTCETable, iommu), true);
> +}
> +
> +static void spapr_tce_vfio_stop(MemoryRegion *iommu)
> +{
> +spapr_tce_set_need_vfio(container_of(iommu, sPAPRTCETable, iommu), 
> false);
> +}
> +
>  static void spapr_tce_table_do_enable(sPAPRTCETable *tcet);
>  static void spapr_tce_table_do_disable(sPAPRTCETable *tcet);
>  
> @@ -239,6 +249,8 @@ static const VMStateDescription vmstate_spapr_tce_table = 
> {
>  static MemoryRegionIOMMUOps spapr_iommu_ops = {
>  .translate = spapr_tce_translate_iommu,
>  .get_page_sizes = spapr_tce_get_page_sizes,
> +.vfio_start = spapr_tce_vfio_start,
> +.vfio_stop = spapr_tce_vfio_stop,

Ok, so AFAICT these callbacks are called whenever a VFIO context is
added / removed from the gIOMMU's address space, and it's up to the
gIOMMU code to ref count that to see if there are any current vfio
users.  That makes "vfio_start" and "vfio_stop" not great names.

But.. better than changing the names would be to move the refcounting
to the generic code if you can manage it, so the individual gIOMMU
backends don't need to - they just told when they need to start / stop
providing VFIO support.

>  };
>  
>  static int spapr_tce_table_realize(DeviceState *dev)
> @@ -248,7 +260,7 @@ static int spapr_tce_table_realize(DeviceState *dev)
>  char tmp[32];
>  
>  tcet->fd = -1;
> -tcet->need_vfio = false;
> +tcet->vfio_users = 0;
>  snprintf(tmp, sizeof(tmp), "tce-root-%x", tcet->liobn);
>  memory_region_init(>root, tcetobj, tmp, UINT64_MAX);
>  
> @@ -268,20 +280,18 @@ void spapr_tce_set_need_vfio(sPAPRTCETable *tcet, bool 
> need_vfio)
>  size_t table_size = tcet->nb_table * sizeof(uint64_t);
>  void *newtable;
>  
> -if (need_vfio == tcet->need_vfio) {
> -/* Nothing to do */
> -return;
> -}
> +tcet->vfio_users += need_vfio ? 1 : -1;
> +g_assert(tcet->vfio_users >= 0);
> +g_assert(tcet->table);
>  
> -if (!need_vfio) {
> +if (!tcet->vfio_users) {
>  /* FIXME: We don't support transition back to KVM accelerated
>   * TCEs yet */
>  return;
>  }
>  
> -tcet->need_vfio = true;
> -
> -if (tcet->fd < 0) {
> +if (tcet->vfio_users > 1) {
> +g_assert(tcet->fd < 0);
>  /* Table is already in userspace, nothing to be do */
>  return;
>  }
> @@ -327,7 +337,7 @@ static void spapr_tce_table_do_enable(sPAPRTCETable *tcet)
>  tcet->page_shift,
>  tcet->nb_table,
>  >fd,
> -tcet->need_vfio);
> +tcet->vfio_users != 0);
>  
>  memory_region_set_size(>iommu,
> (uint64_t)tcet->nb_table << tcet->page_shift);
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 5497a18..f864fde 100644
> --- 

Re: [Qemu-devel] [PATCH qemu v15 16/17] vfio/spapr: Use VFIO_SPAPR_TCE_v2_IOMMU

[David Gibson]

Subject doesn't seem quite right, since you added at least minimal
support for the SPAPRv2 IOMMU in the prereg patch.

On Mon, Apr 04, 2016 at 07:33:45PM +1000, Alexey Kardashevskiy wrote:
> New VFIO_SPAPR_TCE_v2_IOMMU type supports dynamic DMA window management.
> This adds ability to VFIO common code to dynamically allocate/remove
> DMA windows in the host kernel when new VFIO container is added/removed.
> 
> This adds VFIO_IOMMU_SPAPR_TCE_CREATE ioctl to vfio_listener_region_add
> and adds just created IOMMU into the host IOMMU list; the opposite
> action is taken in vfio_listener_region_del.
> 
> When creating a new window, this uses euristic to decide on the TCE table
> levels number.

"heuristic" has an 'h' (yes, English spelling is stupid[0]).

[0] The historical reasons for that are kind of fascinating, though.

> This should cause no guest visible change in behavior.
> 
> Signed-off-by: Alexey Kardashevskiy 
> ---
> Changes:
> v14:
> * new to the series
> 
> ---
> TODO:
> * export levels to PHB
> ---
>  hw/vfio/common.c | 115 
> ++-
>  trace-events |   2 +
>  2 files changed, 107 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 5e5b77c..57a51df 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -279,6 +279,14 @@ static int vfio_host_win_add(VFIOContainer *container,
>  return 0;
>  }
>  
> +static void vfio_host_win_del(VFIOContainer *container, hwaddr min_iova)
> +{
> +VFIOHostDMAWindow *hostwin = vfio_host_win_lookup(container, min_iova, 
> 1);
> +
> +g_assert(hostwin);
> +QLIST_REMOVE(hostwin, hiommu_next);
> +}
> +
>  static bool vfio_listener_skipped_section(MemoryRegionSection *section)
>  {
>  return (!memory_region_is_ram(section->mr) &&
> @@ -392,6 +400,63 @@ static void vfio_listener_region_add(MemoryListener 
> *listener,
>  }
>  end = int128_get64(llend);
>  
> +if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {

I think the "add region" path could do with being split out into a
different function - vfio_listener_region_add() is getting pretty
huge.

> +unsigned entries, pages, pagesize = qemu_real_host_page_size;
> +struct vfio_iommu_spapr_tce_create create = { .argsz = 
> sizeof(create) };
> +
> +trace_vfio_listener_region_add_iommu(iova, end - 1);
> +if (section->mr->iommu_ops) {
> +pagesize = section->mr->iommu_ops->get_page_sizes(section->mr);

Since you're querying the guest IOMMU here, I assume pagesize is
supposed to represent *guest* IOMMU pagesizes, in which case it should
default to TARGET_PAGE_SIZE, instead of qemu_real_host_page_size.
(didn't you already have a function which implemented that fallback?)

> +}
> +/*
> + * FIXME: For VFIO iommu types which have KVM acceleration to
> + * avoid bouncing all map/unmaps through qemu this way, this
> + * would be the right place to wire that up (tell the KVM
> + * device emulation the VFIO iommu handles to use).
> + */
> +create.window_size = int128_get64(section->size);
> +create.page_shift = ctz64(pagesize);
> +/*
> + * SPAPR host supports multilevel TCE tables, there is some
> + * euristic to decide how many levels we want for our table:

s/some euristic/a heuristic/

> + * 0..64 = 1; 65..4096 = 2; 4097..262144 = 3; 262145.. = 4
> + */
> +entries = create.window_size >> create.page_shift;
> +pages = (entries * sizeof(uint64_t)) / getpagesize();
> +create.levels = ctz64(pow2ceil(pages) - 1) / 6 + 1;
> +
> +if (vfio_host_win_lookup(container, create.start_addr,
> + create.start_addr + create.window_size - 
> 1)) {
> +goto fail;

Hmm.. if you successfully look up a host window, it seems to me you
shouldn't fail, but in fact don't even need to create a new window
(the removal path gets harder though, because you need to check if any
guest window requires that host window).

Requiring that the host windows exactly match the guest windows is
probably ok for a first version - except that in that case any overlap
should cause a failure, not just a complete inclusion.

> +}
> +
> +ret = ioctl(container->fd, VFIO_IOMMU_SPAPR_TCE_CREATE, );
> +if (ret) {
> +error_report("Failed to create a window, ret = %d (%m)", ret);
> +goto fail;
> +}
> +
> +if (create.start_addr != section->offset_within_address_space) {
> +struct vfio_iommu_spapr_tce_remove remove = {
> +.argsz = sizeof(remove),
> +.start_addr = create.start_addr
> +};
> +error_report("Host doesn't support DMA window at %"HWADDR_PRIx", 
> must be %"PRIx64,
> + section->offset_within_address_space,
> + 

Re: [Qemu-devel] Any progress with the Cortex-M4 emulation?

[Michael Davidsaver]

On 04/06/2016 06:23 PM, Liviu Ionescu wrote:
> 
>> On 07 Apr 2016, at 01:04, Peter Maydell  wrote:
>>
>> ... Somebody needs to do the necessary work to fix the
>> code review issues. ...
> 
> in this case I'll probably wait for this process to be completed and 
> reevaluate the situation by then.

Liviu,

I haven't had time to complete a revision of my patch set so far this year.  
I've been busy with other work (good for me, bad for qemu), and don't see this 
situation changing for an least the next two months.  It sounds like you have 
time and inclination to do part of what I've started.  I hope my having done 
half of a too big job won't keep you from finishing part of it.

If you decide to work on this problem, please don't hesitate to appropriate, or 
ignore, what I've done so far.

For what it's worth, I did start another revision back in February.  It does 
include the change Peter requested to the storage of priorities wrt. prigroup, 
but doesn't break up the big "rewrite NVIC" patch.

https://github.com/mdavidsaver/qemu/tree/fixirq2

Separately, I have a set of target test programs which I can run both with qemu 
and a real board.  They mostly agree.  Aside from test8.c (MPU) they might be 
of interest.

https://github.com/mdavidsaver/baremetal/tree/qemutest



Michael

Re: [Qemu-devel] [PATCH 07/10] tb hash: hash phys_pc, pc, and flags with xxhash

[Emilio G. Cota]

On Wed, Apr 06, 2016 at 20:23:42 +0200, Paolo Bonzini wrote:
> On 06/04/2016 19:44, Emilio G. Cota wrote:
> > I like this idea, because the ugliness of the sizeof checks is significant.
> > However, the quality of the resulting hash is not as good when always using 
> > func5.
> > For instance, when we'd otherwise use func3, two fifths of every input 
> > contain
> > exactly the same bits: all 0's. This inevitably leads to more collisions.

I take this back. I don't know anymore what I measured earlier today--it's
been a long day and was juggling quite a few things.

I essentially see the same chain lengths (within 0.2%) for either function, i.e.
func3 or func5 with the padded 0's when running arm-softmmu. So this
is good news :>

> Perhaps better is to always use a three-word xxhash, but pick the 64-bit
> version if any of phys_pc and pc are 64-bits.  The unrolling would be
> very effective, and the performance penalty not too important (64-bit on
> 32-bit is very slow anyway).

By "the 64-bit version" you mean what I called func5? That is:

if (sizeof(phys_pc) == sizeof(uint64_t) || sizeof(pc) == sizeof(uint64_t))
return tb_hash_func5();
return tb_hash_func3();

or do you mean xxhash64 (which I did not include in my patchset)?
My tests with xxhash64 suggest that the quality of the results do not
improve over xxhash32, and the computation takes longer (it's more
instructions); not much, but measurable.

So we should probably just go with func5 always, as you suggested
initially. If so, I'm ready to send a v2.

Thanks,

Emilio

Re: [Qemu-devel] [PATCH 0/3] slirp: deliver received TCP RSTs to the guest

[Samuel Thibault]

Hello,

Steven Luo, on Wed 06 Apr 2016 17:00:50 -0700, wrote:
> That said, sorecvoob() also calls soread(), so I'd guess we need to
> deal with the possibility that soread() frees the socket in that case
> as well?

Indeed, then sorecvoob() needs to return that information, so
slirp_pollfds_poll can avoid doing anything else with this socket.

> I could take care of this when I resend this patch series, if you
> prefer.

If you like, please do :)

> I think this should be "might have been removed"?  tcp_sockclosed()
> doesn't seem to call tcp_close() in every case, so we can get -1 from
> soread() without the socket being freed.

Right.

Samuel

Re: [Qemu-devel] [PATCH 0/3] slirp: deliver received TCP RSTs to the guest

[Steven Luo]

On Wed, Apr 06, 2016 at 02:57:43PM +0200, Samuel Thibault wrote:
> ste...@steven676.net, on Tue 05 Apr 2016 17:13:58 -0700, wrote:
> > The second,
> > which fixes delivery of an RST interrupting an already-established TCP
> > connection, was submitted by Edgar Iglesias in 2008 and appears to have
> > been missed then.  The last patch fixes the case where the remote end
> > sends RST in reply to our SYN (rejects our incoming connection attempt).
> 
> It seems I'm getting another crash with these: sowrite would be called
> too for the reseted socket, while the socket has been freed and is not
> even on the polling list any more, I had to additionally do the patch
> below, could you review it so I can push the whole series?

I can't reproduce the crash, but the !(so->so_state & SS_NOFDREF) test
immediately below would seem to be a use-after-free in this case, so I
figure we do need something like this.  That said, sorecvoob() also
calls soread(), so I'd guess we need to deal with the possibility that
soread() frees the socket in that case as well?  (I can't find any other
callers of soread(), but if they exist, they probably need to be fixed
too.)

I could take care of this when I resend this patch series, if you
prefer.

> It's actually quite easy: just reboot the server :) The new instance of
> the server will send a RST whenever the client sends more data.

Thanks for the hint -- I've verified that case works as well now.

> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index fef526c..b13b9af 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -553,6 +553,11 @@ void slirp_pollfds_poll(GArray *pollfds, int 
> select_error)
>  if (ret > 0) {
>  tcp_output(sototcpcb(so));
>  }
> +if (ret < 0) {
> +/* Socket error and thus removed, do not try to do
> + * anything more with it.  */

I think this should be "might have been removed"?  tcp_sockclosed()
doesn't seem to call tcp_close() in every case, so we can get -1 from
soread() without the socket being freed.

> +continue;
> +}
>  }
>  
>  /*

-Steven Luo

Re: [Qemu-devel] [PATCH] xen: piix reuse pci generic class init function

[Stefano Stabellini]

On Sun, 3 Apr 2016, Michael S. Tsirkin wrote:
> piix3_ide_xen_class_init is identical to piix3_ide_class_init
> except it's buggy as it does not set exit and does not disable
> hotplug properly.
> 
> Switch to the generic one.
> 
> Reviewed-by: Stefano Stabellini 
> Signed-off-by: Michael S. Tsirkin 

Hey John,

are you going to take the patch or do you want me to handle it?

Cheers,

Stefano


>  hw/ide/piix.c | 14 +-
>  1 file changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index df46147..0a4cbcb 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -258,22 +258,10 @@ static const TypeInfo piix3_ide_info = {
>  .class_init= piix3_ide_class_init,
>  };
>  
> -static void piix3_ide_xen_class_init(ObjectClass *klass, void *data)
> -{
> -DeviceClass *dc = DEVICE_CLASS(klass);
> -PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> -
> -k->realize = pci_piix_ide_realize;
> -k->vendor_id = PCI_VENDOR_ID_INTEL;
> -k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1;
> -k->class_id = PCI_CLASS_STORAGE_IDE;
> -set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
> -}
> -
>  static const TypeInfo piix3_ide_xen_info = {
>  .name  = "piix3-ide-xen",
>  .parent= TYPE_PCI_IDE,
> -.class_init= piix3_ide_xen_class_init,
> +.class_init= piix3_ide_class_init,
>  };
>  
>  static void piix4_ide_class_init(ObjectClass *klass, void *data)
> -- 
> MST
> 

Re: [Qemu-devel] [PATCH 1/2] qga: fix fd leak with guest-exec i/o channels

[Michael Roth]

Quoting Denis V. Lunev (2016-04-06 00:43:30)
> From: Yuriy Pudgorodskiy 
> 
> Signed-off-by: Yuriy Pudgorodskiy 
> Signed-off-by: Denis V. Lunev 
> CC: Michael Roth 
> ---
>  qga/commands.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/qga/commands.c b/qga/commands.c
> index e091ee1..9ad2f7d 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -446,6 +446,7 @@ GuestExec *qmp_guest_exec(const char *path,
>  g_io_channel_set_encoding(in_ch, NULL, NULL);
>  g_io_channel_set_buffered(in_ch, false);
>  g_io_channel_set_flags(in_ch, G_IO_FLAG_NONBLOCK, NULL);
> +g_io_channel_set_close_on_unref(in_ch, true);
>  g_io_add_watch(in_ch, G_IO_OUT, guest_exec_input_watch, >in);
>  }
> 
> @@ -461,6 +462,8 @@ GuestExec *qmp_guest_exec(const char *path,
>  g_io_channel_set_encoding(err_ch, NULL, NULL);
>  g_io_channel_set_buffered(out_ch, false);
>  g_io_channel_set_buffered(err_ch, false);
> +g_io_channel_set_close_on_unref(out_ch, true);
> +g_io_channel_set_close_on_unref(err_ch, true);

I don't seem any harm in adding these for safety, but don't the handles
get closed via the g_io_channel_shutdown(ch, ...) calls we make prior to
unref in guest_exec_{output,input}_watch()? Or is there another unref
path I'm missing?

>  g_io_add_watch(out_ch, G_IO_IN | G_IO_HUP,
>  guest_exec_output_watch, >out);
>  g_io_add_watch(err_ch, G_IO_IN | G_IO_HUP,
> -- 
> 2.1.4
> 

Re: [Qemu-devel] [PATCH 0/4] Fix QEMU crash on vhost-user socket disconnect.

[Ilya Maximets]

--- Original Message ---
Sender : Michael S. Tsirkin
Date : Apr 05, 2016 13:46 (GMT+03:00)
Title : Re: [PATCH 0/4] Fix QEMU crash on vhost-user socket disconnect.

> On Thu, Mar 31, 2016 at 09:02:01AM +0300, Ilya Maximets wrote:
> > On 30.03.2016 20:01, Michael S. Tsirkin wrote:
> > > On Wed, Mar 30, 2016 at 06:14:05PM +0300, Ilya Maximets wrote:
> > >> Currently QEMU always crashes in following scenario (assume that
> > >> vhost-user application is Open vSwitch with 'dpdkvhostuser' port):
> > > 
> > > In fact, wouldn't the right thing to do be stopping the VM?
> > 
> > I don't think that is reasonable to stop VM on failure of just one of
> > network interfaces.
> 
> We don't start QEMU until vhost user connects, either.
> Making guest run properly with a backend is a much bigger
> project, let's tackle this separately.
> 
> Also, I think handling graceful disconnect is the correct first step.
> Handling misbehaving clients is much harder as we have asserts on remote
> obeying protocol rules all over the place.
> 
> > There may be still working vhost-kernel interfaces.
> > Even connection can still be established if vhost-user interface was in
> > bonding with kernel interface.
> 
> Could not parse this.
>
> > Anyway user should be able to save all his data before QEMU restart.
>
> So reconnect a new backend, and VM will keep going.

We cant't do this because of 2 reasons:
1. Segmentation fault of QEMU on disconnect. (fixed by this patch-set)
2. There is no reconnect functionality in current QEMU version.

So, what are you talking about?

Best regards, Ilya Maximets.

[Qemu-devel] [Bug 1563887] Re: qemu-system-ppc64 freezes on starting image on ppc64le

[Serge Hallyn]

@leftyfb - what exactly is IBM asking to verify?  Whether kvm works
under powervm?  Did smoser's info help?

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

Title:
  qemu-system-ppc64 freezes on starting image on ppc64le

Status in QEMU:
  Confirmed
Status in linux package in Ubuntu:
  Confirmed
Status in livecd-rootfs package in Ubuntu:
  New
Status in qemu package in Ubuntu:
  Confirmed

Bug description:
  qemu-system-ppc64 running on Ubuntu 16.04 beta-2 fails to start an
  image as part of the certification process. This on an IBM ppc64le in
  PowerVM mode running Ubuntu 16.04 beta-2 deployed by MAAS 1.9.1. There
  is no error output.

  ubuntu@alpine01:~/kvm$ qemu-system-ppc64 -m 256 -display none -nographic -net 
nic -net user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine 
pseries -drive file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive 
file=seed.iso,if=virtio
  WARNING: Image format was not specified for 'seed.iso' and probing guessed 
raw.
   Automatically detecting the format is dangerous for raw images, 
write operations on block 0 will be restricted.
   Specify the 'raw' format explicitly to remove the restrictions.

  SLOF **
  QEMU Starting
   Build Date = Jan 29 2016 18:58:37
   FW Version = buildd@ release 20151103
   Press "s" to enter Open Firmware.

  Populating /vdevice methods
  Populating /vdevice/vty@7100
  Populating /vdevice/nvram@7101
  Populating /vdevice/l-lan@7102
  Populating /vdevice/v-scsi@7103
     SCSI: Looking for devices
    8200 CD-ROM   : "QEMU QEMU CD-ROM  2.5+"
  Populating /pci@8002000
   00 1800 (D) : 1af4 1001virtio [ block ]
   00 1000 (D) : 1af4 1001virtio [ block ]
   00 0800 (D) : 106b 003fserial bus [ usb-ohci ]
   00  (D) : 1234 qemu vga
  No NVRAM common partition, re-initializing...
  Installing QEMU fb

  Scanning USB
    OHCI: initializing
  USB Keyboard
  USB mouse
  No console specified using screen & keyboard

    Welcome to Open Firmware

    Copyright (c) 2004, 2011 IBM Corporation All rights reserved.
    This program and the accompanying materials are made available
    under the terms of the BSD License available at
    http://www.opensource.org/licenses/bsd-license.php

  Trying to load:  from: /pci@8002000/scsi@3 ...
  E3404: Not a bootable device!
  Trying to load:  from: /pci@8002000/scsi@2 ...   Successfully loaded
  Linux ppc64le
  #31-Ubuntu SMP F

  ProblemType: Bug
  DistroRelease: Ubuntu 16.04
  Package: qemu-system-ppc 1:2.5+dfsg-5ubuntu6
  ProcVersionSignature: Ubuntu 4.4.0-16.32-generic 4.4.6
  Uname: Linux 4.4.0-16-generic ppc64le
  ApportVersion: 2.20-0ubuntu3
  Architecture: ppc64el
  Date: Wed Mar 30 14:10:01 2016
  KvmCmdLine:
   COMMAND STAT  EUID  RUID   PID  PPID %CPU COMMAND
   kvm-irqfd-clean S<   0 0  1172 2  0.0 [kvm-irqfd-clean]
   qemu-nbdSsl  0 0 13467 1  0.0 qemu-nbd -c /dev/nbd0 
xenial-server-cloudimg-ppc64el-disk1.img
   qemu-system-ppc Sl+   1000  1000 18973 18896  101 qemu-system-ppc64 -m 256 
-display none -nographic -net nic -net 
user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine pseries -drive 
file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive 
file=seed.iso,if=virtio
  Lsusb: Error: command ['lsusb'] failed with exit code 1:
  ProcEnviron:
   TERM=xterm
   PATH=(custom, no user)
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinux-4.4.0-16-generic 
root=UUID=92d820c8-ab25-497b-9b1e-f1435992bbf3 ro
  ProcLoadAvg: 1.08 0.94 0.58 2/616 19571
  ProcLocks:
   1: POSIX  ADVISORY  WRITE 886 00:13:381 0 EOF
   2: POSIX  ADVISORY  WRITE 1339 00:13:528 0 EOF
   3: FLOCK  ADVISORY  WRITE 1284 00:13:522 0 EOF
   4: POSIX  ADVISORY  WRITE 2281 00:13:563 0 EOF
   5: POSIX  ADVISORY  WRITE 1331 00:13:536 0 EOF
  ProcSwaps:
   Filename TypeSizeUsedPriority
   /swap.img   file 8388544 0   -1
  ProcVersion: Linux version 4.4.0-16-generic (buildd@bos01-ppc64el-001) (gcc 
version 5.3.1 20160320 (Ubuntu/Linaro/IBM 5.3.1-12ubuntu4) ) #32-Ubuntu SMP Thu 
Mar 24 22:31:14 UTC 2016
  SourcePackage: qemu
  UpgradeStatus: No upgrade log present (probably fresh install)
  bootlist:
   /pci@8002011/pci1014,034A@0/sas/disk@4068402c40
   
/pci@8002018/ethernet@0:speed=auto,duplex=auto,csarch,000.000.000.000,,000.000.000.000,000.000.000.000,5,5,000.000.000.000,512
   
/pci@8002018/ethernet@0,1:speed=auto,duplex=auto,csarch,000.000.000.000,,000.000.000.000,000.000.000.000,5,5,000.000.000.000,512
   

Re: [Qemu-devel] [PATCH 06/10] include: add xxhash.h

[Emilio G. Cota]

On Wed, Apr 06, 2016 at 12:39:42 +0100, Alex Bennée wrote:
> 
> Emilio G. Cota  writes:
> 
> > xxhash is a fast, high-quality hashing function. The appended
> > brings in the 32-bit version of it, with the small modification that
> > it assumes the data to be hashed is made of 32-bit chunks; this increases
> > speed slightly for the use-case we care about, i.e. tb-hash.
> >
> > The original algorithm, as well as a 64-bit implementation, can be found at:
> >   https://github.com/Cyan4973/xxHash
> >
> > Signed-off-by: Emilio G. Cota 
> > ---
> >  include/qemu/xxhash.h | 106 
> > ++
> >  1 file changed, 106 insertions(+)
> >  create mode 100644 include/qemu/xxhash.h
> >
> > diff --git a/include/qemu/xxhash.h b/include/qemu/xxhash.h
> > new file mode 100644
> > index 000..a13a665
> > --- /dev/null
> > +++ b/include/qemu/xxhash.h
> > @@ -0,0 +1,106 @@
> 
> > +
> > +/* u32 hash of @n contiguous chunks of u32's */
> > +static inline uint32_t qemu_xxh32(const uint32_t *p, size_t n, uint32_t 
> > seed)
> > +{
> 
> What is the point of seed here? I looked on the original site to see if
> there was any guidance on tuning seed but couldn't find anything. I
> appreciate the compiler can inline the constant away but perhaps we
> should #define it and drop the parameter if we are not intending to
> modify it?

The seed value would only matter if we needed consistent hashes (we don't).

Any seed value should give similar performance, given xxhash's quality.

I'll set a define for this seed if it is used in more than one place.

> Also it might be helpful to wrap the call to avoid getting the
> boilerplate sizing wrong:
> 
>   #define qemu_xxh32(s) qemu_xxh32_impl((const uint32_t *)s, 
> sizeof(*s)/sizeof(uint32_t), 1)
> 
> Then calls become a little simpler for the user:
> 
>   return qemu_xxh32();
> 
> Do we need to include a compile time check for structures that don't
> neatly divide into uint32_t chunks?

I'll give the original byte-sized function a shot and come back to this
if necessary.

Thanks,

E.

[Qemu-devel] [PATCH v2 2/1 for-2.6] nbd: Don't kill server when client requests unknown option

[Eric Blake]

nbd-server.c currently fails to handle unsupported options properly.
If during option haggling the client sends an unknown request, the
server kills the connection instead of letting the client try to
fall back to something older.  This is precisely what advertising
NBD_FLAG_FIXED_NEWSTYLE was supposed to fix.

Signed-off-by: Eric Blake 
---

Turns out our server has a very similar bug to the client.
If desired, I can spin a v3 that moves the hunk in nbd/client.c
to the previous patch.

 nbd/client.c | 2 ++
 nbd/server.c | 5 -
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/nbd/client.c b/nbd/client.c
index 67116b9..fc382c5 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -109,6 +109,8 @@ static int nbd_handle_reply_err(QIOChannel *ioc, uint32_t 
opt, uint32_t type,

 switch (type) {
 case NBD_REP_ERR_UNSUP:
+TRACE("server doesn't understand request %d, attempting fallback",
+  opt);
 result = 0;
 goto cleanup;

diff --git a/nbd/server.c b/nbd/server.c
index b95571b..7843584 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -482,9 +482,12 @@ static int nbd_negotiate_options(NBDClient *client)
 return -EINVAL;
 default:
 TRACE("Unsupported option 0x%x", clientflags);
+if (nbd_negotiate_drop_sync(client->ioc, length) != length) {
+return -EIO;
+}
 nbd_negotiate_send_rep(client->ioc, NBD_REP_ERR_UNSUP,
clientflags);
-return -EINVAL;
+break;
 }
 } else {
 /*
-- 
2.5.5

Re: [Qemu-devel] Any progress with the Cortex-M4 emulation?

[Liviu Ionescu]

> On 07 Apr 2016, at 01:04, Peter Maydell  wrote:
> 
> ... Somebody needs to do the necessary work to fix the
> code review issues. ...

in this case I'll probably wait for this process to be completed and reevaluate 
the situation by then.


regards,

Liviu

Re: [Qemu-devel] Any progress with the Cortex-M4 emulation?

[Peter Maydell]

On 6 April 2016 at 22:54, Liviu Ionescu  wrote:
>
>> On 06 Apr 2016, at 17:42, Peter Maydell  wrote:
>>
>> On 6 April 2016 at 13:52, Liviu Ionescu  wrote:
>>>
>>> I also have on my TODO list to implement the SCB registers used
>>> during exception processing (MMFAR, BFAR, CFSR); I checked and
>>> in version 2.5.1 apparently they are still not implemented.
>>
>> Those I think are covered by Michael's patchset.
>
> I updated my git and browsed the log records, the only commits
> of Michael were from Mov 3, and apparently did not include this
> functionality.
>
>> This will clash very badly with Michael's in-flight
>> patchset.
>
> you mean Michael's code is not yet in the master branch?

Yes; patches were posted to the mailing list but there were
code review comments which haven't yet been addressed.
We went through a couple of rounds of review; it was
looking promising but still needed some more work. The
last posted patchset was:
https://lists.nongnu.org/archive/html/qemu-devel/2015-12/msg00504.html
(the second half of that is adding MPU support so you
can ignore it.)

>> I think it would be much better to get that
>> complete and upstream first, because otherwise you'll
>> probably end up having to redo a lot of work.
>
> my intent is to isolate the Cortex-M code from the
> existing ARM implementation, practically it should be
> a fresh start of the Cortex-M code, with a NVIC that no
> longer inherits from GIC.

Michael's patchset does that already.

> if you plan to further maintain the existing code, fain,
> but for me it is a bit too messy and I cannot rely on it,
> I need a clean implementation of the system peripherals,
> with properly processed exceptions.

Michael's patchset handles exceptions properly (at
any rate much closer to architecturally than our
current code).

> if Michael code is not in, when do you think it will be,
> so I can make a decision to either continue to patch the
> existing code or to redo the system code?

Somebody needs to do the necessary work to fix the
code review issues. I'm guessing from not having seen
any mails from Michael since December that he's been
busy with other things. If I were in your position I
would start with Michael's patchset and make the
necessary fixes to it to get it upstreamable. That's
likely going to be faster than starting the same
thing from scratch.

thanks
-- PMM

Re: [Qemu-devel] Any progress with the Cortex-M4 emulation?

[Liviu Ionescu]

> On 06 Apr 2016, at 17:42, Peter Maydell  wrote:
> 
> On 6 April 2016 at 13:52, Liviu Ionescu  wrote:
>> 
>> I also have on my TODO list to implement the SCB registers used
>> during exception processing (MMFAR, BFAR, CFSR); I checked and
>> in version 2.5.1 apparently they are still not implemented.
> 
> Those I think are covered by Michael's patchset.

I updated my git and browsed the log records, the only commits of Michael were 
from Mov 3, and apparently did not include this functionality.

> This will clash very badly with Michael's in-flight
> patchset.

you mean Michael's code is not yet in the master branch?

> I think it would be much better to get that
> complete and upstream first, because otherwise you'll
> probably end up having to redo a lot of work.

my intent is to isolate the Cortex-M code from the existing ARM implementation, 
practically it should be a fresh start of the Cortex-M code, with a NVIC that 
no longer inherits from GIC.

if you plan to further maintain the existing code, fain, but for me it is a bit 
too messy and I cannot rely on it, I need a clean implementation of the system 
peripherals, with properly processed exceptions.

if Michael code is not in, when do you think it will be, so I can make a 
decision to either continue to patch the existing code or to redo the system 
code?


regards,

Liviu

[Qemu-devel] [Bug 1563887] Re: qemu-system-ppc64 freezes on starting image on ppc64le

[Scott Moser]

Note, that Michael Roth's comments are probably valid.  I suspect that
if leftyfb ever saw this working on powerVM host, then it was through
the kvm_pr module.  In my experience, that works generally pretty well
(buit my testing has only ever been done powerNV host).

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

Title:
  qemu-system-ppc64 freezes on starting image on ppc64le

Status in QEMU:
  Confirmed
Status in linux package in Ubuntu:
  Confirmed
Status in livecd-rootfs package in Ubuntu:
  New
Status in qemu package in Ubuntu:
  Confirmed

Bug description:
  qemu-system-ppc64 running on Ubuntu 16.04 beta-2 fails to start an
  image as part of the certification process. This on an IBM ppc64le in
  PowerVM mode running Ubuntu 16.04 beta-2 deployed by MAAS 1.9.1. There
  is no error output.

  ubuntu@alpine01:~/kvm$ qemu-system-ppc64 -m 256 -display none -nographic -net 
nic -net user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine 
pseries -drive file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive 
file=seed.iso,if=virtio
  WARNING: Image format was not specified for 'seed.iso' and probing guessed 
raw.
   Automatically detecting the format is dangerous for raw images, 
write operations on block 0 will be restricted.
   Specify the 'raw' format explicitly to remove the restrictions.

  SLOF **
  QEMU Starting
   Build Date = Jan 29 2016 18:58:37
   FW Version = buildd@ release 20151103
   Press "s" to enter Open Firmware.

  Populating /vdevice methods
  Populating /vdevice/vty@7100
  Populating /vdevice/nvram@7101
  Populating /vdevice/l-lan@7102
  Populating /vdevice/v-scsi@7103
     SCSI: Looking for devices
    8200 CD-ROM   : "QEMU QEMU CD-ROM  2.5+"
  Populating /pci@8002000
   00 1800 (D) : 1af4 1001virtio [ block ]
   00 1000 (D) : 1af4 1001virtio [ block ]
   00 0800 (D) : 106b 003fserial bus [ usb-ohci ]
   00  (D) : 1234 qemu vga
  No NVRAM common partition, re-initializing...
  Installing QEMU fb

  Scanning USB
    OHCI: initializing
  USB Keyboard
  USB mouse
  No console specified using screen & keyboard

    Welcome to Open Firmware

    Copyright (c) 2004, 2011 IBM Corporation All rights reserved.
    This program and the accompanying materials are made available
    under the terms of the BSD License available at
    http://www.opensource.org/licenses/bsd-license.php

  Trying to load:  from: /pci@8002000/scsi@3 ...
  E3404: Not a bootable device!
  Trying to load:  from: /pci@8002000/scsi@2 ...   Successfully loaded
  Linux ppc64le
  #31-Ubuntu SMP F

  ProblemType: Bug
  DistroRelease: Ubuntu 16.04
  Package: qemu-system-ppc 1:2.5+dfsg-5ubuntu6
  ProcVersionSignature: Ubuntu 4.4.0-16.32-generic 4.4.6
  Uname: Linux 4.4.0-16-generic ppc64le
  ApportVersion: 2.20-0ubuntu3
  Architecture: ppc64el
  Date: Wed Mar 30 14:10:01 2016
  KvmCmdLine:
   COMMAND STAT  EUID  RUID   PID  PPID %CPU COMMAND
   kvm-irqfd-clean S<   0 0  1172 2  0.0 [kvm-irqfd-clean]
   qemu-nbdSsl  0 0 13467 1  0.0 qemu-nbd -c /dev/nbd0 
xenial-server-cloudimg-ppc64el-disk1.img
   qemu-system-ppc Sl+   1000  1000 18973 18896  101 qemu-system-ppc64 -m 256 
-display none -nographic -net nic -net 
user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine pseries -drive 
file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive 
file=seed.iso,if=virtio
  Lsusb: Error: command ['lsusb'] failed with exit code 1:
  ProcEnviron:
   TERM=xterm
   PATH=(custom, no user)
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinux-4.4.0-16-generic 
root=UUID=92d820c8-ab25-497b-9b1e-f1435992bbf3 ro
  ProcLoadAvg: 1.08 0.94 0.58 2/616 19571
  ProcLocks:
   1: POSIX  ADVISORY  WRITE 886 00:13:381 0 EOF
   2: POSIX  ADVISORY  WRITE 1339 00:13:528 0 EOF
   3: FLOCK  ADVISORY  WRITE 1284 00:13:522 0 EOF
   4: POSIX  ADVISORY  WRITE 2281 00:13:563 0 EOF
   5: POSIX  ADVISORY  WRITE 1331 00:13:536 0 EOF
  ProcSwaps:
   Filename TypeSizeUsedPriority
   /swap.img   file 8388544 0   -1
  ProcVersion: Linux version 4.4.0-16-generic (buildd@bos01-ppc64el-001) (gcc 
version 5.3.1 20160320 (Ubuntu/Linaro/IBM 5.3.1-12ubuntu4) ) #32-Ubuntu SMP Thu 
Mar 24 22:31:14 UTC 2016
  SourcePackage: qemu
  UpgradeStatus: No upgrade log present (probably fresh install)
  bootlist:
   /pci@8002011/pci1014,034A@0/sas/disk@4068402c40
   
/pci@8002018/ethernet@0:speed=auto,duplex=auto,csarch,000.000.000.000,,000.000.000.000,000.000.000.000,5,5,000.000.000.000,512
   

[Qemu-devel] [Bug 1563887] Re: qemu-system-ppc64 freezes on starting image on ppc64le

[Scott Moser]

ok. so after playing some, this all comes down to needing 'usb=off' on
the qemu command line.

works:
qemu-system-ppc64 -enable-kvm -machine usb=off -device 
virtio-net-pci,netdev=net00 -netdev type=user,id=net00 -drive 
if=virtio,file=xenial-server-cloudimg-ppc64el-disk1.img,format=qcow2 -drive 
if=virtio,file=my-seed.img,format=raw -snapshot -m 256 -nographic


doesnt work:
qemu-system-ppc64 -enable-kvm  virtio-net-pci,netdev=net00 -netdev 
type=user,id=net00 -drive 
if=virtio,file=xenial-server-cloudimg-ppc64el-disk1.img,format=qcow2 -drive 
if=virtio,file=my-seed.img,format=raw -snapshot -m 256 -nographic


the interesting bit of 'doesnt work' is that it *does* work, the user just 
thinks it doesnt because output is eaten up and doesnt go to console, but it 
*does* work fine.
http://paste.ubuntu.com/15660816/

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

Title:
  qemu-system-ppc64 freezes on starting image on ppc64le

Status in QEMU:
  Confirmed
Status in linux package in Ubuntu:
  Confirmed
Status in livecd-rootfs package in Ubuntu:
  New
Status in qemu package in Ubuntu:
  Confirmed

Bug description:
  qemu-system-ppc64 running on Ubuntu 16.04 beta-2 fails to start an
  image as part of the certification process. This on an IBM ppc64le in
  PowerVM mode running Ubuntu 16.04 beta-2 deployed by MAAS 1.9.1. There
  is no error output.

  ubuntu@alpine01:~/kvm$ qemu-system-ppc64 -m 256 -display none -nographic -net 
nic -net user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine 
pseries -drive file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive 
file=seed.iso,if=virtio
  WARNING: Image format was not specified for 'seed.iso' and probing guessed 
raw.
   Automatically detecting the format is dangerous for raw images, 
write operations on block 0 will be restricted.
   Specify the 'raw' format explicitly to remove the restrictions.

  SLOF **
  QEMU Starting
   Build Date = Jan 29 2016 18:58:37
   FW Version = buildd@ release 20151103
   Press "s" to enter Open Firmware.

  Populating /vdevice methods
  Populating /vdevice/vty@7100
  Populating /vdevice/nvram@7101
  Populating /vdevice/l-lan@7102
  Populating /vdevice/v-scsi@7103
     SCSI: Looking for devices
    8200 CD-ROM   : "QEMU QEMU CD-ROM  2.5+"
  Populating /pci@8002000
   00 1800 (D) : 1af4 1001virtio [ block ]
   00 1000 (D) : 1af4 1001virtio [ block ]
   00 0800 (D) : 106b 003fserial bus [ usb-ohci ]
   00  (D) : 1234 qemu vga
  No NVRAM common partition, re-initializing...
  Installing QEMU fb

  Scanning USB
    OHCI: initializing
  USB Keyboard
  USB mouse
  No console specified using screen & keyboard

    Welcome to Open Firmware

    Copyright (c) 2004, 2011 IBM Corporation All rights reserved.
    This program and the accompanying materials are made available
    under the terms of the BSD License available at
    http://www.opensource.org/licenses/bsd-license.php

  Trying to load:  from: /pci@8002000/scsi@3 ...
  E3404: Not a bootable device!
  Trying to load:  from: /pci@8002000/scsi@2 ...   Successfully loaded
  Linux ppc64le
  #31-Ubuntu SMP F

  ProblemType: Bug
  DistroRelease: Ubuntu 16.04
  Package: qemu-system-ppc 1:2.5+dfsg-5ubuntu6
  ProcVersionSignature: Ubuntu 4.4.0-16.32-generic 4.4.6
  Uname: Linux 4.4.0-16-generic ppc64le
  ApportVersion: 2.20-0ubuntu3
  Architecture: ppc64el
  Date: Wed Mar 30 14:10:01 2016
  KvmCmdLine:
   COMMAND STAT  EUID  RUID   PID  PPID %CPU COMMAND
   kvm-irqfd-clean S<   0 0  1172 2  0.0 [kvm-irqfd-clean]
   qemu-nbdSsl  0 0 13467 1  0.0 qemu-nbd -c /dev/nbd0 
xenial-server-cloudimg-ppc64el-disk1.img
   qemu-system-ppc Sl+   1000  1000 18973 18896  101 qemu-system-ppc64 -m 256 
-display none -nographic -net nic -net 
user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine pseries -drive 
file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive 
file=seed.iso,if=virtio
  Lsusb: Error: command ['lsusb'] failed with exit code 1:
  ProcEnviron:
   TERM=xterm
   PATH=(custom, no user)
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinux-4.4.0-16-generic 
root=UUID=92d820c8-ab25-497b-9b1e-f1435992bbf3 ro
  ProcLoadAvg: 1.08 0.94 0.58 2/616 19571
  ProcLocks:
   1: POSIX  ADVISORY  WRITE 886 00:13:381 0 EOF
   2: POSIX  ADVISORY  WRITE 1339 00:13:528 0 EOF
   3: FLOCK  ADVISORY  WRITE 1284 00:13:522 0 EOF
   4: POSIX  ADVISORY  WRITE 2281 00:13:563 0 EOF
   5: POSIX  ADVISORY  WRITE 1331 00:13:536 0 EOF
  ProcSwaps:
   Filename TypeSizeUsedPriority
   /swap.img 

[Qemu-devel] [Bug 1563887] Re: qemu-system-ppc64 freezes on starting image on ppc64le

[Mike Rushton]

1. the bug reporter is using a powervm partition. KVM cannot be used
there. This is not a KVM bug.

PowerVM mode is an LPAR. It is not a kvm instance trying to run an KVM
within. This was also working in 14.04 without issue and is being asked
of us from IBM to certify

2. the xenial cloud images have an outdated 4.2 kernel which doesn't
boot in kvm on powernv. A workaround is to use the isos which do boot.
This is a cloud-images bug.

All the Xenial daily images i've been using from MAAS deployments have
been the 4.4 kernel which is still not working.

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

Title:
  qemu-system-ppc64 freezes on starting image on ppc64le

Status in QEMU:
  Confirmed
Status in linux package in Ubuntu:
  Confirmed
Status in livecd-rootfs package in Ubuntu:
  New
Status in qemu package in Ubuntu:
  Confirmed

Bug description:
  qemu-system-ppc64 running on Ubuntu 16.04 beta-2 fails to start an
  image as part of the certification process. This on an IBM ppc64le in
  PowerVM mode running Ubuntu 16.04 beta-2 deployed by MAAS 1.9.1. There
  is no error output.

  ubuntu@alpine01:~/kvm$ qemu-system-ppc64 -m 256 -display none -nographic -net 
nic -net user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine 
pseries -drive file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive 
file=seed.iso,if=virtio
  WARNING: Image format was not specified for 'seed.iso' and probing guessed 
raw.
   Automatically detecting the format is dangerous for raw images, 
write operations on block 0 will be restricted.
   Specify the 'raw' format explicitly to remove the restrictions.

  SLOF **
  QEMU Starting
   Build Date = Jan 29 2016 18:58:37
   FW Version = buildd@ release 20151103
   Press "s" to enter Open Firmware.

  Populating /vdevice methods
  Populating /vdevice/vty@7100
  Populating /vdevice/nvram@7101
  Populating /vdevice/l-lan@7102
  Populating /vdevice/v-scsi@7103
     SCSI: Looking for devices
    8200 CD-ROM   : "QEMU QEMU CD-ROM  2.5+"
  Populating /pci@8002000
   00 1800 (D) : 1af4 1001virtio [ block ]
   00 1000 (D) : 1af4 1001virtio [ block ]
   00 0800 (D) : 106b 003fserial bus [ usb-ohci ]
   00  (D) : 1234 qemu vga
  No NVRAM common partition, re-initializing...
  Installing QEMU fb

  Scanning USB
    OHCI: initializing
  USB Keyboard
  USB mouse
  No console specified using screen & keyboard

    Welcome to Open Firmware

    Copyright (c) 2004, 2011 IBM Corporation All rights reserved.
    This program and the accompanying materials are made available
    under the terms of the BSD License available at
    http://www.opensource.org/licenses/bsd-license.php

  Trying to load:  from: /pci@8002000/scsi@3 ...
  E3404: Not a bootable device!
  Trying to load:  from: /pci@8002000/scsi@2 ...   Successfully loaded
  Linux ppc64le
  #31-Ubuntu SMP F

  ProblemType: Bug
  DistroRelease: Ubuntu 16.04
  Package: qemu-system-ppc 1:2.5+dfsg-5ubuntu6
  ProcVersionSignature: Ubuntu 4.4.0-16.32-generic 4.4.6
  Uname: Linux 4.4.0-16-generic ppc64le
  ApportVersion: 2.20-0ubuntu3
  Architecture: ppc64el
  Date: Wed Mar 30 14:10:01 2016
  KvmCmdLine:
   COMMAND STAT  EUID  RUID   PID  PPID %CPU COMMAND
   kvm-irqfd-clean S<   0 0  1172 2  0.0 [kvm-irqfd-clean]
   qemu-nbdSsl  0 0 13467 1  0.0 qemu-nbd -c /dev/nbd0 
xenial-server-cloudimg-ppc64el-disk1.img
   qemu-system-ppc Sl+   1000  1000 18973 18896  101 qemu-system-ppc64 -m 256 
-display none -nographic -net nic -net 
user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine pseries -drive 
file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive 
file=seed.iso,if=virtio
  Lsusb: Error: command ['lsusb'] failed with exit code 1:
  ProcEnviron:
   TERM=xterm
   PATH=(custom, no user)
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinux-4.4.0-16-generic 
root=UUID=92d820c8-ab25-497b-9b1e-f1435992bbf3 ro
  ProcLoadAvg: 1.08 0.94 0.58 2/616 19571
  ProcLocks:
   1: POSIX  ADVISORY  WRITE 886 00:13:381 0 EOF
   2: POSIX  ADVISORY  WRITE 1339 00:13:528 0 EOF
   3: FLOCK  ADVISORY  WRITE 1284 00:13:522 0 EOF
   4: POSIX  ADVISORY  WRITE 2281 00:13:563 0 EOF
   5: POSIX  ADVISORY  WRITE 1331 00:13:536 0 EOF
  ProcSwaps:
   Filename TypeSizeUsedPriority
   /swap.img   file 8388544 0   -1
  ProcVersion: Linux version 4.4.0-16-generic (buildd@bos01-ppc64el-001) (gcc 
version 5.3.1 20160320 (Ubuntu/Linaro/IBM 5.3.1-12ubuntu4) ) #32-Ubuntu SMP Thu 
Mar 24 22:31:14 UTC 2016
  SourcePackage: qemu
  

[Qemu-devel] [Bug 1563887] Re: qemu-system-ppc64 freezes on starting image on ppc64le

[Mike Rushton]

I have also shown that this issue has the exact same results on PowerNV
above.


** Changed in: qemu
   Status: Invalid => Confirmed

** Changed in: qemu (Ubuntu)
   Status: Invalid => Confirmed

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

Title:
  qemu-system-ppc64 freezes on starting image on ppc64le

Status in QEMU:
  Confirmed
Status in linux package in Ubuntu:
  Confirmed
Status in livecd-rootfs package in Ubuntu:
  New
Status in qemu package in Ubuntu:
  Confirmed

Bug description:
  qemu-system-ppc64 running on Ubuntu 16.04 beta-2 fails to start an
  image as part of the certification process. This on an IBM ppc64le in
  PowerVM mode running Ubuntu 16.04 beta-2 deployed by MAAS 1.9.1. There
  is no error output.

  ubuntu@alpine01:~/kvm$ qemu-system-ppc64 -m 256 -display none -nographic -net 
nic -net user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine 
pseries -drive file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive 
file=seed.iso,if=virtio
  WARNING: Image format was not specified for 'seed.iso' and probing guessed 
raw.
   Automatically detecting the format is dangerous for raw images, 
write operations on block 0 will be restricted.
   Specify the 'raw' format explicitly to remove the restrictions.

  SLOF **
  QEMU Starting
   Build Date = Jan 29 2016 18:58:37
   FW Version = buildd@ release 20151103
   Press "s" to enter Open Firmware.

  Populating /vdevice methods
  Populating /vdevice/vty@7100
  Populating /vdevice/nvram@7101
  Populating /vdevice/l-lan@7102
  Populating /vdevice/v-scsi@7103
     SCSI: Looking for devices
    8200 CD-ROM   : "QEMU QEMU CD-ROM  2.5+"
  Populating /pci@8002000
   00 1800 (D) : 1af4 1001virtio [ block ]
   00 1000 (D) : 1af4 1001virtio [ block ]
   00 0800 (D) : 106b 003fserial bus [ usb-ohci ]
   00  (D) : 1234 qemu vga
  No NVRAM common partition, re-initializing...
  Installing QEMU fb

  Scanning USB
    OHCI: initializing
  USB Keyboard
  USB mouse
  No console specified using screen & keyboard

    Welcome to Open Firmware

    Copyright (c) 2004, 2011 IBM Corporation All rights reserved.
    This program and the accompanying materials are made available
    under the terms of the BSD License available at
    http://www.opensource.org/licenses/bsd-license.php

  Trying to load:  from: /pci@8002000/scsi@3 ...
  E3404: Not a bootable device!
  Trying to load:  from: /pci@8002000/scsi@2 ...   Successfully loaded
  Linux ppc64le
  #31-Ubuntu SMP F

  ProblemType: Bug
  DistroRelease: Ubuntu 16.04
  Package: qemu-system-ppc 1:2.5+dfsg-5ubuntu6
  ProcVersionSignature: Ubuntu 4.4.0-16.32-generic 4.4.6
  Uname: Linux 4.4.0-16-generic ppc64le
  ApportVersion: 2.20-0ubuntu3
  Architecture: ppc64el
  Date: Wed Mar 30 14:10:01 2016
  KvmCmdLine:
   COMMAND STAT  EUID  RUID   PID  PPID %CPU COMMAND
   kvm-irqfd-clean S<   0 0  1172 2  0.0 [kvm-irqfd-clean]
   qemu-nbdSsl  0 0 13467 1  0.0 qemu-nbd -c /dev/nbd0 
xenial-server-cloudimg-ppc64el-disk1.img
   qemu-system-ppc Sl+   1000  1000 18973 18896  101 qemu-system-ppc64 -m 256 
-display none -nographic -net nic -net 
user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine pseries -drive 
file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive 
file=seed.iso,if=virtio
  Lsusb: Error: command ['lsusb'] failed with exit code 1:
  ProcEnviron:
   TERM=xterm
   PATH=(custom, no user)
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinux-4.4.0-16-generic 
root=UUID=92d820c8-ab25-497b-9b1e-f1435992bbf3 ro
  ProcLoadAvg: 1.08 0.94 0.58 2/616 19571
  ProcLocks:
   1: POSIX  ADVISORY  WRITE 886 00:13:381 0 EOF
   2: POSIX  ADVISORY  WRITE 1339 00:13:528 0 EOF
   3: FLOCK  ADVISORY  WRITE 1284 00:13:522 0 EOF
   4: POSIX  ADVISORY  WRITE 2281 00:13:563 0 EOF
   5: POSIX  ADVISORY  WRITE 1331 00:13:536 0 EOF
  ProcSwaps:
   Filename TypeSizeUsedPriority
   /swap.img   file 8388544 0   -1
  ProcVersion: Linux version 4.4.0-16-generic (buildd@bos01-ppc64el-001) (gcc 
version 5.3.1 20160320 (Ubuntu/Linaro/IBM 5.3.1-12ubuntu4) ) #32-Ubuntu SMP Thu 
Mar 24 22:31:14 UTC 2016
  SourcePackage: qemu
  UpgradeStatus: No upgrade log present (probably fresh install)
  bootlist:
   /pci@8002011/pci1014,034A@0/sas/disk@4068402c40
   
/pci@8002018/ethernet@0:speed=auto,duplex=auto,csarch,000.000.000.000,,000.000.000.000,000.000.000.000,5,5,000.000.000.000,512
   

[Qemu-devel] [Bug 1563887] Re: qemu-system-ppc64 freezes on starting image on ppc64le

[Scott Moser]

** Changed in: qemu
   Status: New => Invalid

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

Title:
  qemu-system-ppc64 freezes on starting image on ppc64le

Status in QEMU:
  Invalid
Status in linux package in Ubuntu:
  Confirmed
Status in livecd-rootfs package in Ubuntu:
  New
Status in qemu package in Ubuntu:
  Invalid

Bug description:
  qemu-system-ppc64 running on Ubuntu 16.04 beta-2 fails to start an
  image as part of the certification process. This on an IBM ppc64le in
  PowerVM mode running Ubuntu 16.04 beta-2 deployed by MAAS 1.9.1. There
  is no error output.

  ubuntu@alpine01:~/kvm$ qemu-system-ppc64 -m 256 -display none -nographic -net 
nic -net user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine 
pseries -drive file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive 
file=seed.iso,if=virtio
  WARNING: Image format was not specified for 'seed.iso' and probing guessed 
raw.
   Automatically detecting the format is dangerous for raw images, 
write operations on block 0 will be restricted.
   Specify the 'raw' format explicitly to remove the restrictions.

  SLOF **
  QEMU Starting
   Build Date = Jan 29 2016 18:58:37
   FW Version = buildd@ release 20151103
   Press "s" to enter Open Firmware.

  Populating /vdevice methods
  Populating /vdevice/vty@7100
  Populating /vdevice/nvram@7101
  Populating /vdevice/l-lan@7102
  Populating /vdevice/v-scsi@7103
     SCSI: Looking for devices
    8200 CD-ROM   : "QEMU QEMU CD-ROM  2.5+"
  Populating /pci@8002000
   00 1800 (D) : 1af4 1001virtio [ block ]
   00 1000 (D) : 1af4 1001virtio [ block ]
   00 0800 (D) : 106b 003fserial bus [ usb-ohci ]
   00  (D) : 1234 qemu vga
  No NVRAM common partition, re-initializing...
  Installing QEMU fb

  Scanning USB
    OHCI: initializing
  USB Keyboard
  USB mouse
  No console specified using screen & keyboard

    Welcome to Open Firmware

    Copyright (c) 2004, 2011 IBM Corporation All rights reserved.
    This program and the accompanying materials are made available
    under the terms of the BSD License available at
    http://www.opensource.org/licenses/bsd-license.php

  Trying to load:  from: /pci@8002000/scsi@3 ...
  E3404: Not a bootable device!
  Trying to load:  from: /pci@8002000/scsi@2 ...   Successfully loaded
  Linux ppc64le
  #31-Ubuntu SMP F

  ProblemType: Bug
  DistroRelease: Ubuntu 16.04
  Package: qemu-system-ppc 1:2.5+dfsg-5ubuntu6
  ProcVersionSignature: Ubuntu 4.4.0-16.32-generic 4.4.6
  Uname: Linux 4.4.0-16-generic ppc64le
  ApportVersion: 2.20-0ubuntu3
  Architecture: ppc64el
  Date: Wed Mar 30 14:10:01 2016
  KvmCmdLine:
   COMMAND STAT  EUID  RUID   PID  PPID %CPU COMMAND
   kvm-irqfd-clean S<   0 0  1172 2  0.0 [kvm-irqfd-clean]
   qemu-nbdSsl  0 0 13467 1  0.0 qemu-nbd -c /dev/nbd0 
xenial-server-cloudimg-ppc64el-disk1.img
   qemu-system-ppc Sl+   1000  1000 18973 18896  101 qemu-system-ppc64 -m 256 
-display none -nographic -net nic -net 
user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine pseries -drive 
file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive 
file=seed.iso,if=virtio
  Lsusb: Error: command ['lsusb'] failed with exit code 1:
  ProcEnviron:
   TERM=xterm
   PATH=(custom, no user)
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinux-4.4.0-16-generic 
root=UUID=92d820c8-ab25-497b-9b1e-f1435992bbf3 ro
  ProcLoadAvg: 1.08 0.94 0.58 2/616 19571
  ProcLocks:
   1: POSIX  ADVISORY  WRITE 886 00:13:381 0 EOF
   2: POSIX  ADVISORY  WRITE 1339 00:13:528 0 EOF
   3: FLOCK  ADVISORY  WRITE 1284 00:13:522 0 EOF
   4: POSIX  ADVISORY  WRITE 2281 00:13:563 0 EOF
   5: POSIX  ADVISORY  WRITE 1331 00:13:536 0 EOF
  ProcSwaps:
   Filename TypeSizeUsedPriority
   /swap.img   file 8388544 0   -1
  ProcVersion: Linux version 4.4.0-16-generic (buildd@bos01-ppc64el-001) (gcc 
version 5.3.1 20160320 (Ubuntu/Linaro/IBM 5.3.1-12ubuntu4) ) #32-Ubuntu SMP Thu 
Mar 24 22:31:14 UTC 2016
  SourcePackage: qemu
  UpgradeStatus: No upgrade log present (probably fresh install)
  bootlist:
   /pci@8002011/pci1014,034A@0/sas/disk@4068402c40
   
/pci@8002018/ethernet@0:speed=auto,duplex=auto,csarch,000.000.000.000,,000.000.000.000,000.000.000.000,5,5,000.000.000.000,512
   
/pci@8002018/ethernet@0,1:speed=auto,duplex=auto,csarch,000.000.000.000,,000.000.000.000,000.000.000.000,5,5,000.000.000.000,512
   

[Qemu-devel] [Bug 1563887] Re: qemu-system-ppc64 freezes on starting image on ppc64le

[Serge Hallyn]

Ok so if I'm following this right there are two issues:

1. the bug reporter is using a powervm partition.  KVM cannot be used
there.  This is not a KVM bug.

2. the xenial cloud images have an outdated 4.2 kernel which doesn't
boot in kvm on powernv.  A workaround is to use the isos which do boot.
This is a cloud-images bug.

AFAICS there is no qemu bug here, so marking invalid for that package.

** Changed in: qemu (Ubuntu)
   Status: Confirmed => Invalid

** Changed in: livecd-rootfs (Ubuntu)
   Importance: Undecided => High

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

Title:
  qemu-system-ppc64 freezes on starting image on ppc64le

Status in QEMU:
  New
Status in linux package in Ubuntu:
  Confirmed
Status in livecd-rootfs package in Ubuntu:
  New
Status in qemu package in Ubuntu:
  Invalid

Bug description:
  qemu-system-ppc64 running on Ubuntu 16.04 beta-2 fails to start an
  image as part of the certification process. This on an IBM ppc64le in
  PowerVM mode running Ubuntu 16.04 beta-2 deployed by MAAS 1.9.1. There
  is no error output.

  ubuntu@alpine01:~/kvm$ qemu-system-ppc64 -m 256 -display none -nographic -net 
nic -net user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine 
pseries -drive file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive 
file=seed.iso,if=virtio
  WARNING: Image format was not specified for 'seed.iso' and probing guessed 
raw.
   Automatically detecting the format is dangerous for raw images, 
write operations on block 0 will be restricted.
   Specify the 'raw' format explicitly to remove the restrictions.

  SLOF **
  QEMU Starting
   Build Date = Jan 29 2016 18:58:37
   FW Version = buildd@ release 20151103
   Press "s" to enter Open Firmware.

  Populating /vdevice methods
  Populating /vdevice/vty@7100
  Populating /vdevice/nvram@7101
  Populating /vdevice/l-lan@7102
  Populating /vdevice/v-scsi@7103
     SCSI: Looking for devices
    8200 CD-ROM   : "QEMU QEMU CD-ROM  2.5+"
  Populating /pci@8002000
   00 1800 (D) : 1af4 1001virtio [ block ]
   00 1000 (D) : 1af4 1001virtio [ block ]
   00 0800 (D) : 106b 003fserial bus [ usb-ohci ]
   00  (D) : 1234 qemu vga
  No NVRAM common partition, re-initializing...
  Installing QEMU fb

  Scanning USB
    OHCI: initializing
  USB Keyboard
  USB mouse
  No console specified using screen & keyboard

    Welcome to Open Firmware

    Copyright (c) 2004, 2011 IBM Corporation All rights reserved.
    This program and the accompanying materials are made available
    under the terms of the BSD License available at
    http://www.opensource.org/licenses/bsd-license.php

  Trying to load:  from: /pci@8002000/scsi@3 ...
  E3404: Not a bootable device!
  Trying to load:  from: /pci@8002000/scsi@2 ...   Successfully loaded
  Linux ppc64le
  #31-Ubuntu SMP F

  ProblemType: Bug
  DistroRelease: Ubuntu 16.04
  Package: qemu-system-ppc 1:2.5+dfsg-5ubuntu6
  ProcVersionSignature: Ubuntu 4.4.0-16.32-generic 4.4.6
  Uname: Linux 4.4.0-16-generic ppc64le
  ApportVersion: 2.20-0ubuntu3
  Architecture: ppc64el
  Date: Wed Mar 30 14:10:01 2016
  KvmCmdLine:
   COMMAND STAT  EUID  RUID   PID  PPID %CPU COMMAND
   kvm-irqfd-clean S<   0 0  1172 2  0.0 [kvm-irqfd-clean]
   qemu-nbdSsl  0 0 13467 1  0.0 qemu-nbd -c /dev/nbd0 
xenial-server-cloudimg-ppc64el-disk1.img
   qemu-system-ppc Sl+   1000  1000 18973 18896  101 qemu-system-ppc64 -m 256 
-display none -nographic -net nic -net 
user,net=10.0.0.0/8,host=10.0.0.1,hostfwd=tcp::-:22 -machine pseries -drive 
file=xenial-server-cloudimg-ppc64el-disk1.img,if=virtio -drive 
file=seed.iso,if=virtio
  Lsusb: Error: command ['lsusb'] failed with exit code 1:
  ProcEnviron:
   TERM=xterm
   PATH=(custom, no user)
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinux-4.4.0-16-generic 
root=UUID=92d820c8-ab25-497b-9b1e-f1435992bbf3 ro
  ProcLoadAvg: 1.08 0.94 0.58 2/616 19571
  ProcLocks:
   1: POSIX  ADVISORY  WRITE 886 00:13:381 0 EOF
   2: POSIX  ADVISORY  WRITE 1339 00:13:528 0 EOF
   3: FLOCK  ADVISORY  WRITE 1284 00:13:522 0 EOF
   4: POSIX  ADVISORY  WRITE 2281 00:13:563 0 EOF
   5: POSIX  ADVISORY  WRITE 1331 00:13:536 0 EOF
  ProcSwaps:
   Filename TypeSizeUsedPriority
   /swap.img   file 8388544 0   -1
  ProcVersion: Linux version 4.4.0-16-generic (buildd@bos01-ppc64el-001) (gcc 
version 5.3.1 20160320 (Ubuntu/Linaro/IBM 5.3.1-12ubuntu4) ) #32-Ubuntu SMP Thu 
Mar 24 22:31:14 UTC 2016
  SourcePackage: qemu
  UpgradeStatus: No upgrade log present 

[Qemu-devel] [PATCH v2] hw/i386: add device tree support

[Antonio Borneo]

With "-dtb" on command-line:
- append the device tree blob to the kernel image;
- pass the blob's pointer to the kernel through setup_data, as
  requested by upstream kernel commit da6b737b9ab7 ("x86: Add
  device tree support").

The device tree blob is passed as-is to the guest; none of its
fields is modified nor updated. This is not an issue; the kernel
commit above uses the device tree only as an extension to the
traditional kernel configuration.

To: "Michael S. Tsirkin" 
To: Paolo Bonzini 
To: Richard Henderson 
To: Eduardo Habkost 
Cc: qemu-devel@nongnu.org
Cc: Sebastian Andrzej Siewior 
Signed-off-by: Antonio Borneo 
---
v2: replace deprecated load_image() with load_image_size()

 hw/i386/pc.c | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2ac97c4..469f658 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -813,11 +813,26 @@ static long get_file_size(FILE *f)
 return size;
 }
 
+/* setup_data types */
+#define SETUP_NONE 0
+#define SETUP_E820_EXT 1
+#define SETUP_DTB  2
+#define SETUP_PCI  3
+#define SETUP_EFI  4
+
+struct setup_data {
+uint64_t next;
+uint32_t type;
+uint32_t len;
+uint8_t data[0];
+} __attribute__((packed));
+
 static void load_linux(PCMachineState *pcms,
FWCfgState *fw_cfg)
 {
 uint16_t protocol;
 int setup_size, kernel_size, initrd_size = 0, cmdline_size;
+int dtb_size, setup_data_offset;
 uint32_t initrd_max;
 uint8_t header[8192], *setup, *kernel, *initrd_data;
 hwaddr real_addr, prot_addr, cmdline_addr, initrd_addr = 0;
@@ -825,8 +840,10 @@ static void load_linux(PCMachineState *pcms,
 char *vmode;
 MachineState *machine = MACHINE(pcms);
 PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+struct setup_data *setup_data;
 const char *kernel_filename = machine->kernel_filename;
 const char *initrd_filename = machine->initrd_filename;
+const char *dtb_filename = machine->dtb;
 const char *kernel_cmdline = machine->kernel_cmdline;
 
 /* Align to 16 bytes as a paranoia measure */
@@ -989,6 +1006,35 @@ static void load_linux(PCMachineState *pcms,
 exit(1);
 }
 fclose(f);
+
+/* append dtb to kernel */
+if (dtb_filename) {
+if (protocol < 0x209) {
+fprintf(stderr, "qemu: Linux kernel too old to load a dtb\n");
+exit(1);
+}
+
+dtb_size = get_image_size(dtb_filename);
+if (dtb_size <= 0) {
+fprintf(stderr, "qemu: error reading dtb %s: %s\n",
+dtb_filename, strerror(errno));
+exit(1);
+}
+
+setup_data_offset = QEMU_ALIGN_UP(kernel_size, 16);
+kernel_size = setup_data_offset + sizeof(struct setup_data) + dtb_size;
+kernel = g_realloc(kernel, kernel_size);
+
+stq_p(header+0x250, prot_addr + setup_data_offset);
+
+setup_data = (struct setup_data *)(kernel + setup_data_offset);
+setup_data->next = 0;
+setup_data->type = cpu_to_le32(SETUP_DTB);
+setup_data->len = cpu_to_le32(dtb_size);
+
+load_image_size(dtb_filename, setup_data->data, dtb_size);
+}
+
 memcpy(setup, header, MIN(sizeof(header), setup_size));
 
 fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, prot_addr);
-- 
2.8.0

Re: [Qemu-devel] [PATCH] hw/i386: add device tree support

[Antonio Borneo]

On Tue, Apr 5, 2016 at 10:43 PM, Eduardo Habkost  wrote:
> [...]
>
> One small comment below:
>
> [...]
>> +
>> +load_image(dtb_filename, setup_data->data);
>
> load_image() is deprecated, please use
> load_image_size(dtb_filename, setup_data->data, dtb_size)
> instead.

Yes, correct! I missed it.

I'm going to send a V2.

Thanks,
Antonio

Re: [Qemu-devel] [PATCH 0/2] Add gpio_key and use it for ARM virt power button

[Peter Maydell]

On 6 April 2016 at 18:50, Emilio G. Cota  wrote:
> I might be doing something wrong, but aarch64-softmmu @ master only works for 
> me
> after reverting 94f02c5ea94 "ARM: Virt: Use gpio_key for power button". 
> Bisect log
> appended.
>
> This is what I get when booting aarch64 as per [1]:
> $ aarch64-softmmu/qemu-system-aarch64 -machine virt -cpu cortex-a57 -machine 
> type=virt \
>  -nographic -smp 1 -m 2048 -kernel 
> img/aarch64/aarch64-linux-3.15rc2-buildroot.img  \
> --append "console=ttyAMA0"
>
> qemu-system-aarch64: Unknown device 'gpio-key' for default sysbus
> Aborted (core dumped)

Your tree isn't building right (this is a dependency bug in our
makefiles somewhere) -- it hasn't built the new object file
with the gpio-key device in it, which means a runtime failure
when the device can't be found. You should be able to fix this
by deleting the aarch64-softmmu/config-devices.mak file from
your build tree and then doing a rebuild.

(One day I may get round to figuring out what happens here.
The oddity is that it doesn't cause problems for an
incremental rebuild, only for a build after a 'make clean'.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.6] block: Forbid I/O throttling on nodes with multiple parents for 2.6

[Alberto Garcia]

On Mon 04 Apr 2016 05:26:02 PM CEST, Kevin Wolf wrote:
> As the patches to move I/O throttling to BlockBackend didn't make it in
> time for the 2.6 release, but the release adds new ways of configuring
> VMs whose behaviour would change once the move is done, we need to
> outlaw such configurations temporarily.
>
> The problem exists whenever a BDS has more users than just its BB, for
> example it is used as a backing file for another node. (This wasn't
> possible in 2.5 yet as we introduced node references to specify a
> backing file only recently.) In these cases, the throttling would
> apply to these other users now, but after moving throttling to the
> BlockBackend the other users wouldn't be throttled any more.
>
> This patch both prevents making new references to a throttled node as
> well as using monitor commands to throttle a node with multiple parents.
>
> Compared to 2.5 this changes behaviour in some corner cases where
> references were allowed before, like bs->file or Quorum children. It
> seems reasonable to assume that users didn't use I/O throttling on such
> low level nodes. With the upcoming move of throttling into BlockBackend,
> such configurations won't be possible anyway.
>
> Signed-off-by: Kevin Wolf 

Reviewed-by: Alberto Garcia 

Berto

[Qemu-devel] [PATCH v3 12/14] iotests.py: Allow concurrent qemu instances

[Max Reitz]

By adding an optional suffix to the files used for communication with a
VM, we can launch multiple VM instances concurrently.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/iotests.py | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index b3c00dd..ef7e52f 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -157,10 +157,13 @@ def event_match(event, match=None):
 class VM(object):
 '''A QEMU VM'''
 
-def __init__(self):
-self._monitor_path = os.path.join(test_dir, 'qemu-mon.%d' % 
os.getpid())
-self._qemu_log_path = os.path.join(test_dir, 'qemu-log.%d' % 
os.getpid())
-self._qtest_path = os.path.join(test_dir, 'qemu-qtest.%d' % 
os.getpid())
+def __init__(self, path_suffix=''):
+self._monitor_path = os.path.join(test_dir, 'qemu-mon%s.%d' %
+(path_suffix, os.getpid()))
+self._qemu_log_path = os.path.join(test_dir, 'qemu-log%s.%d' %
+ (path_suffix, 
os.getpid()))
+self._qtest_path = os.path.join(test_dir, 'qemu-qtest%s.%d' %
+  (path_suffix, os.getpid()))
 self._args = qemu_args + ['-chardev',
  'socket,id=mon,path=' + self._monitor_path,
  '-mon', 'chardev=mon,mode=control',
-- 
2.8.0

Re: [Qemu-devel] [FreeBSD] Host build i386 failing to build aarch64 targets

[Alex Bennée]

Sean Bruno  writes:

> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA512
>
>
>
> On 03/20/16 12:20, Sean Bruno wrote:
>> aarch64 targets are now failing to build on i386 hosts due to
>> missing __atomic_load_8() calls since this commit:
>>
>> https://github.com/qemu/qemu/commit/a0aa44b488b3601415d55041e4619aef5f
> 3a4ba8#diff-c143d686899ae51d7b927d9c682e12fd
>>
>>  I'm unsure if Linux is disabling aarch64 targets for i386 hosts or
>> if this commit works "just fine" on Linux hosts right now, as it
>> doesn't work with clang or gcc.
>>
>> More or less, the code in question ends up looking like this bit
>> of test code:
>>
>> #include  #include  #include
>> 
>>
>> #define atomic_read(ptr)  \ ({
>> \ typeof(*ptr) _val;\
>> __atomic_load(ptr, &_val, __ATOMIC_RELAXED); \ _val;
>> \ })
>>
>> int main () { int foo; int64_t foo64;
>>
>> atomic_read(); atomic_read();
>>
>> return(0); }
>>
>>
>> This test code will manifest the same issue as the aarch64 target
>> building on FreeBSD i386 with the error:
>>
>> undefined reference to `__atomic_load_8'
>>
>>
>
>
> This seems to be fixed with the latest commits.  Thanks!

Sorry I thought I'd CC'd you. The patch needed re-spinning to get picked
up by Peter. Glad it is working for you now ;-)

>
> sean
> -BEGIN PGP SIGNATURE-
> Version: GnuPG v2
>
> iQF8BAEBCgBmBQJXBTUqXxSAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w
> ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRCQUFENDYzMkU3MTIxREU4RDIwOTk3REQx
> MjAxRUZDQTFFNzI3RTY0AAoJEBIB78oecn5kAoYH/3Ai7IdpyxyIiRtgYDWJQcg8
> GLmbu1NL5Xrh0af5DU933kCqkDlKK8qKYs89DzHMfay8TIyZFkqKF5pYy66JJAQ2
> qAB9eGTL567q3QPk9iYkRLju4Y4exmBL1ZSW1fUpPKjjjBlLR7VrHjEA/Ze0zYsM
> +MvRJcHSf8tNawr8WfOzVgFPf8pc2K0Ix8/VZCFEdf4FcATHj2nYXTmzQmTQuWSo
> tqDWe02TIov0BSBaA4uG6n02F4KWglGBE+bdsuTiwxxAjkcmHLgg28h7Wupkmatj
> 5zarlSLIhvv7j3KAS/r8aKtQ04ydXybTo0HnPLJ9JV/xz3bAbvvKDYLMZijpm0M=
> =OSgq
> -END PGP SIGNATURE-


--
Alex Bennée

[Qemu-devel] [PATCH v3 13/14] socket_scm_helper: Accept fd directly

[Max Reitz]

This gives us more freedom about the fd that is passed to qemu, allowing
us to e.g. pass sockets.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/socket_scm_helper.c | 29 ++---
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/tests/qemu-iotests/socket_scm_helper.c 
b/tests/qemu-iotests/socket_scm_helper.c
index 80cadf4..eb76d31 100644
--- a/tests/qemu-iotests/socket_scm_helper.c
+++ b/tests/qemu-iotests/socket_scm_helper.c
@@ -60,7 +60,7 @@ static int send_fd(int fd, int fd_to_send)
 }
 
 /* Convert string to fd number. */
-static int get_fd_num(const char *fd_str)
+static int get_fd_num(const char *fd_str, bool silent)
 {
 int sock;
 char *err;
@@ -68,12 +68,16 @@ static int get_fd_num(const char *fd_str)
 errno = 0;
 sock = strtol(fd_str, , 10);
 if (errno) {
-fprintf(stderr, "Failed in strtol for socket fd, reason: %s\n",
-strerror(errno));
+if (!silent) {
+fprintf(stderr, "Failed in strtol for socket fd, reason: %s\n",
+strerror(errno));
+}
 return -1;
 }
 if (!*fd_str || *err || sock < 0) {
-fprintf(stderr, "bad numerical value for socket fd '%s'\n", fd_str);
+if (!silent) {
+fprintf(stderr, "bad numerical value for socket fd '%s'\n", 
fd_str);
+}
 return -1;
 }
 
@@ -104,18 +108,21 @@ int main(int argc, char **argv, char **envp)
 }
 
 
-sock = get_fd_num(argv[1]);
+sock = get_fd_num(argv[1], false);
 if (sock < 0) {
 return EXIT_FAILURE;
 }
 
-/* Now only open a file in readonly mode for test purpose. If more precise
-   control is needed, use python script in file operation, which is
-   supposed to fork and exec this program. */
-fd = open(argv[2], O_RDONLY);
+fd = get_fd_num(argv[2], true);
 if (fd < 0) {
-fprintf(stderr, "Failed to open file '%s'\n", argv[2]);
-return EXIT_FAILURE;
+/* Now only open a file in readonly mode for test purpose. If more
+   precise control is needed, use python script in file operation, 
which
+   is supposed to fork and exec this program. */
+fd = open(argv[2], O_RDONLY);
+if (fd < 0) {
+fprintf(stderr, "Failed to open file '%s'\n", argv[2]);
+return EXIT_FAILURE;
+}
 }
 
 ret = send_fd(sock, fd);
-- 
2.8.0

Re: [Qemu-devel] [PATCH v3 0/7] bdrv_flush_io_queue removal, shared LinuxAioState

[Kevin Wolf]

Am 06.04.2016 um 20:28 hat Paolo Bonzini geschrieben:
> 
> 
> On 06/04/2016 20:19, Kevin Wolf wrote:
> >> > 
> >> > Great, I'll send v4 as soon as possible for inclusion in the block-next
> >> > branch.
> > Are you still working on v4 or did I miss it? All of the block-next
> > candidate series are waiting to be rebased on this one, and moving parts
> > at the bottom of a longish dependency chain are kind of nasty.
> 
> Yes, I am waiting for Fam's bdrv_co_drain patch, which is this one
> depends on.

Oh, I wasn't aware of that one. Thanks!

Kevin

[Qemu-devel] [PATCH v3 11/14] iotests.py: Add qemu_nbd function

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/iotests.py | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 8499e1b..b3c00dd 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -28,7 +28,7 @@ import qmp
 import qtest
 import struct
 
-__all__ = ['imgfmt', 'imgproto', 'test_dir' 'qemu_img', 'qemu_io',
+__all__ = ['imgfmt', 'imgproto', 'test_dir' 'qemu_img', 'qemu_io', 'qemu_nbd',
'VM', 'QMPTestCase', 'notrun', 'main', 'verify_image_format',
'verify_platform', 'filter_test_dir', 'filter_win32',
'filter_qemu_io', 'filter_chown', 'log']
@@ -43,6 +43,10 @@ qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
 if os.environ.get('QEMU_IO_OPTIONS'):
 qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
 
+qemu_nbd_args = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')]
+if os.environ.get('QEMU_NBD_OPTIONS'):
+qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ')
+
 qemu_args = [os.environ.get('QEMU_PROG', 'qemu')]
 if os.environ.get('QEMU_OPTIONS'):
 qemu_args += os.environ['QEMU_OPTIONS'].strip().split(' ')
@@ -91,6 +95,11 @@ def qemu_io(*args):
 sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' 
'.join(args)))
 return subp.communicate()[0]
 
+def qemu_nbd(*args):
+'''Run qemu-nbd in the background'''
+subp = subprocess.Popen(qemu_nbd_args + list(args))
+return subp
+
 def compare_images(img1, img2):
 '''Return True if two image files are identical'''
 return qemu_img('compare', '-f', imgfmt,
-- 
2.8.0

[Qemu-devel] [PATCH v3 05/14] block/nbd: Use qdict_put()

[Max Reitz]

Instead of inlining this nice macro (i.e. resorting to
qdict_put_obj(..., QOBJECT(...))), use it.

Signed-off-by: Max Reitz 
---
 block/nbd.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index efa5d3d..d12bcc6 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -437,7 +437,7 @@ static void nbd_refresh_filename(BlockDriverState *bs, 
QDict *options)
 port = stringify(NBD_DEFAULT_PORT);
 }
 
-qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("nbd")));
+qdict_put(opts, "driver", qstring_from_str("nbd"));
 
 if (path && export) {
 snprintf(bs->exact_filename, sizeof(bs->exact_filename),
@@ -454,16 +454,16 @@ static void nbd_refresh_filename(BlockDriverState *bs, 
QDict *options)
 }
 
 if (path) {
-qdict_put_obj(opts, "path", QOBJECT(qstring_from_str(path)));
+qdict_put(opts, "path", qstring_from_str(path));
 } else {
-qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(host)));
-qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(port)));
+qdict_put(opts, "host", qstring_from_str(host));
+qdict_put(opts, "port", qstring_from_str(port));
 }
 if (export) {
-qdict_put_obj(opts, "export", QOBJECT(qstring_from_str(export)));
+qdict_put(opts, "export", qstring_from_str(export));
 }
 if (tlscreds) {
-qdict_put_obj(opts, "tls-creds", QOBJECT(qstring_from_str(tlscreds)));
+qdict_put(opts, "tls-creds", qstring_from_str(tlscreds));
 }
 
 bs->full_open_options = opts;
-- 
2.8.0

[Qemu-devel] [PATCH v3 14/14] iotests: Add test for NBD's blockdev-add interface

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/147 | 194 +
 tests/qemu-iotests/147.out |   5 ++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 200 insertions(+)
 create mode 100755 tests/qemu-iotests/147
 create mode 100644 tests/qemu-iotests/147.out

diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
new file mode 100755
index 000..f31de69
--- /dev/null
+++ b/tests/qemu-iotests/147
@@ -0,0 +1,194 @@
+#!/usr/bin/env python
+#
+# Test case for the QMP 'change' command and all other associated
+# commands
+#
+# Copyright (C) 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import os
+import socket
+import stat
+import time
+import iotests
+from iotests import cachemode, imgfmt, qemu_img, qemu_nbd
+
+NBD_PORT = 10811
+
+test_img = os.path.join(iotests.test_dir, 'test.img')
+
+class NBDBlockdevAddBase(iotests.QMPTestCase):
+def blockdev_add_options(self, address, export=None):
+options = { 'id': 'drive0',
+'driver': 'raw',
+'file': {
+'driver': 'nbd',
+'address': address
+} }
+if export is not None:
+options['file']['export'] = export
+return options
+
+def client_test(self, filename, address, export=None):
+bao = self.blockdev_add_options(address, export)
+result = self.vm.qmp('blockdev-add', options=bao)
+self.assert_qmp(result, 'return', {})
+
+result = self.vm.qmp('query-block')
+self.assert_qmp(result, 'return[0]/inserted/image/filename', filename)
+
+result = self.vm.qmp('x-blockdev-del', id='drive0')
+self.assert_qmp(result, 'return', {})
+
+
+class QemuNBD(NBDBlockdevAddBase):
+def setUp(self):
+qemu_img('create', '-f', iotests.imgfmt, test_img, '64k')
+self.vm = iotests.VM()
+self.vm.launch()
+self.qemu_nbd = None
+
+def tearDown(self):
+self.vm.shutdown()
+if self.qemu_nbd is not None:
+self.qemu_nbd.wait()
+os.remove(test_img)
+
+def _server_up(self, *args):
+self.qemu_nbd = qemu_nbd('-f', imgfmt, test_img, *args)
+time.sleep(1)
+
+def test_inet(self):
+self._server_up('-p', str(NBD_PORT))
+address = { 'type': 'inet',
+'data': {
+'host': 'localhost',
+'port': str(NBD_PORT)
+} }
+self.client_test('nbd://localhost:%i' % NBD_PORT, address)
+
+def test_unix(self):
+socket = os.path.join(iotests.test_dir, 'qemu-nbd.socket')
+self._server_up('-k', socket)
+address = { 'type': 'unix',
+'data': { 'path': socket } }
+self.client_test('nbd+unix://?socket=' + socket, address)
+os.remove(socket)
+
+
+class BuiltinNBD(NBDBlockdevAddBase):
+def setUp(self):
+qemu_img('create', '-f', iotests.imgfmt, test_img, '64k')
+self.vm = iotests.VM()
+self.vm.launch()
+self.server = iotests.VM('.server')
+self.server.add_drive_raw('if=none,id=nbd-export,' +
+  'file=%s,' % test_img +
+  'format=%s,' % imgfmt +
+  'cache=%s' % cachemode)
+self.server.launch()
+
+def tearDown(self):
+self.vm.shutdown()
+self.server.shutdown()
+os.remove(test_img)
+
+def _server_up(self, address):
+result = self.server.qmp('nbd-server-start', addr=address)
+self.assert_qmp(result, 'return', {})
+
+result = self.server.qmp('nbd-server-add', device='nbd-export')
+self.assert_qmp(result, 'return', {})
+
+def _server_down(self):
+result = self.server.qmp('nbd-server-stop')
+self.assert_qmp(result, 'return', {})
+
+def test_inet(self):
+address = { 'type': 'inet',
+'data': {
+'host': 'localhost',
+'port': str(NBD_PORT)
+} }
+self._server_up(address)
+self.client_test('nbd://localhost:%i/nbd-export' % NBD_PORT,
+ address, 'nbd-export')
+self._server_down()
+
+def test_inet6(self):
+

[Qemu-devel] [PATCH v3 08/14] block/nbd: Accept SocketAddress

[Max Reitz]

Add a new option "address" to the NBD block driver which accepts a
SocketAddress.

"path", "host" and "port" are still supported as legacy options and are
mapped to their corresponding SocketAddress representation.

Signed-off-by: Max Reitz 
---
 block/nbd.c   | 97 ++-
 tests/qemu-iotests/051.out|  4 +-
 tests/qemu-iotests/051.pc.out |  4 +-
 3 files changed, 64 insertions(+), 41 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 3adf302..9ae66ba 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -32,6 +32,8 @@
 #include "qemu/uri.h"
 #include "block/block_int.h"
 #include "qemu/module.h"
+#include "qapi-visit.h"
+#include "qapi/qmp-input-visitor.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qjson.h"
 #include "qapi/qmp/qint.h"
@@ -128,7 +130,9 @@ static bool nbd_has_filename_options_conflict(QDict 
*options, Error **errp)
 if (!strcmp(e->key, "host")
 || !strcmp(e->key, "port")
 || !strcmp(e->key, "path")
-|| !strcmp(e->key, "export"))
+|| !strcmp(e->key, "export")
+|| !strcmp(e->key, "address")
+|| !strncmp(e->key, "address.", 8))
 {
 error_setg(errp, "Option '%s' cannot be used with a file name",
e->key);
@@ -202,46 +206,66 @@ out:
 g_free(file);
 }
 
+static bool nbd_process_legacy_socket_options(QDict *options, Error **errp)
+{
+if (qdict_haskey(options, "path") && qdict_haskey(options, "host")) {
+error_setg(errp, "path and host may not be used at the same time");
+return false;
+} else if (qdict_haskey(options, "path")) {
+if (qdict_haskey(options, "port")) {
+error_setg(errp, "port may not be used without host");
+return false;
+}
+
+qdict_put(options, "address.type", qstring_from_str("unix"));
+qdict_change_key(options, "path", "address.data.path");
+} else if (qdict_haskey(options, "host")) {
+qdict_put(options, "address.type", qstring_from_str("inet"));
+qdict_change_key(options, "host", "address.data.host");
+if (!qdict_change_key(options, "port", "address.data.port")) {
+qdict_put(options, "address.data.port",
+  qstring_from_str(stringify(NBD_DEFAULT_PORT)));
+}
+}
+
+return true;
+}
+
 static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, char 
**export,
  Error **errp)
 {
-SocketAddress *saddr;
+SocketAddress *saddr = NULL;
+QDict *addr = NULL;
+QObject *crumpled_addr;
+QmpInputVisitor *iv;
+Error *local_err = NULL;
 
-if (qdict_haskey(options, "path") == qdict_haskey(options, "host")) {
-if (qdict_haskey(options, "path")) {
-error_setg(errp, "path and host may not be used at the same time");
-} else {
-error_setg(errp, "one of path and host must be specified");
-}
-return NULL;
+if (!nbd_process_legacy_socket_options(options, errp)) {
+goto fail;
 }
-if (qdict_haskey(options, "port") && !qdict_haskey(options, "host")) {
-error_setg(errp, "port may not be used without host");
-return NULL;
+
+qdict_extract_subqdict(options, , "address.");
+if (!qdict_size(addr)) {
+error_setg(errp, "NBD server address missing");
+goto fail;
 }
 
-saddr = g_new0(SocketAddress, 1);
+crumpled_addr = qdict_crumple(addr, true, errp);
+if (!crumpled_addr) {
+goto fail;
+}
 
-if (qdict_haskey(options, "path")) {
-UnixSocketAddress *q_unix;
-saddr->type = SOCKET_ADDRESS_KIND_UNIX;
-q_unix = saddr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
-q_unix->path = g_strdup(qdict_get_str(options, "path"));
-qdict_del(options, "path");
-} else {
-InetSocketAddress *inet;
-saddr->type = SOCKET_ADDRESS_KIND_INET;
-inet = saddr->u.inet.data = g_new0(InetSocketAddress, 1);
-inet->host = g_strdup(qdict_get_str(options, "host"));
-if (!qdict_get_try_str(options, "port")) {
-inet->port = g_strdup_printf("%d", NBD_DEFAULT_PORT);
-} else {
-inet->port = g_strdup(qdict_get_str(options, "port"));
-}
-qdict_del(options, "host");
-qdict_del(options, "port");
+iv = qmp_input_visitor_new(crumpled_addr);
+visit_type_SocketAddress(qmp_input_get_visitor(iv), NULL, ,
+ _err);
+qmp_input_visitor_cleanup(iv);
+if (local_err) {
+error_propagate(errp, local_err);
+goto fail;
 }
 
+/* TODO: Detect superfluous (unused) options under in addr */
+
 s->client.is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;
 
 *export = g_strdup(qdict_get_try_str(options, "export"));
@@ -249,7 +273,12 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QDict 
*options, 

[Qemu-devel] [PATCH v3 04/14] block/nbd: Default port in nbd_refresh_filename()

[Max Reitz]

Instead of not emitting the port in nbd_refresh_filename(), just set it
to the default if the user did not specify it. This makes the logic a
bit simpler.

Signed-off-by: Max Reitz 
---
 block/nbd.c | 18 +++---
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 2112ec0..efa5d3d 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -433,6 +433,10 @@ static void nbd_refresh_filename(BlockDriverState *bs, 
QDict *options)
 const char *export = qdict_get_try_str(options, "export");
 const char *tlscreds = qdict_get_try_str(options, "tls-creds");
 
+if (host && !port) {
+port = stringify(NBD_DEFAULT_PORT);
+}
+
 qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("nbd")));
 
 if (path && export) {
@@ -441,27 +445,19 @@ static void nbd_refresh_filename(BlockDriverState *bs, 
QDict *options)
 } else if (path && !export) {
 snprintf(bs->exact_filename, sizeof(bs->exact_filename),
  "nbd+unix://?socket=%s", path);
-} else if (!path && export && port) {
+} else if (!path && export) {
 snprintf(bs->exact_filename, sizeof(bs->exact_filename),
  "nbd://%s:%s/%s", host, port, export);
-} else if (!path && export && !port) {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd://%s/%s", host, export);
-} else if (!path && !export && port) {
+} else if (!path && !export) {
 snprintf(bs->exact_filename, sizeof(bs->exact_filename),
  "nbd://%s:%s", host, port);
-} else if (!path && !export && !port) {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd://%s", host);
 }
 
 if (path) {
 qdict_put_obj(opts, "path", QOBJECT(qstring_from_str(path)));
-} else if (port) {
-qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(host)));
-qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(port)));
 } else {
 qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(host)));
+qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(port)));
 }
 if (export) {
 qdict_put_obj(opts, "export", QOBJECT(qstring_from_str(export)));
-- 
2.8.0

[Qemu-devel] [PATCH v3 10/14] qapi: Allow blockdev-add for NBD

[Max Reitz]

Signed-off-by: Max Reitz 
---
 qapi/block-core.json | 23 +--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1d09079..6e38ff0 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1632,13 +1632,14 @@
 # Drivers that are supported in block device operations.
 #
 # @host_device, @host_cdrom: Since 2.1
+# @nbd: Since 2.6
 #
 # Since: 2.0
 ##
 { 'enum': 'BlockdevDriver',
   'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
 'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',
-'http', 'https', 'luks', 'null-aio', 'null-co', 'parallels',
+'http', 'https', 'luks', 'nbd', 'null-aio', 'null-co', 'parallels',
 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp', 'vdi', 'vhdx',
 'vmdk', 'vpc', 'vvfat' ] }
 
@@ -2032,6 +2033,24 @@
 '*read-pattern': 'QuorumReadPattern' } }
 
 ##
+# @BlockdevOptionsNbd
+#
+# Driver specific block device options for NBD.
+#
+# @address: NBD server address
+#
+# @export:  #optional export name
+#
+# @tls-creds:   #optional TLS credentials ID
+#
+# Since: 2.6
+##
+{ 'struct': 'BlockdevOptionsNbd',
+  'data': { 'address': 'SocketAddress',
+'*export': 'str',
+'*tls-creds': 'str' } }
+
+##
 # @BlockdevOptions
 #
 # Options for creating a block device.  Many options are available for all
@@ -2101,7 +2120,7 @@
   'https':  'BlockdevOptionsFile',
 # TODO iscsi: Wait for structured options
   'luks':   'BlockdevOptionsLUKS',
-# TODO nbd: Should take InetSocketAddress for 'host'?
+  'nbd':'BlockdevOptionsNbd',
 # TODO nfs: Wait for structured options
   'null-aio':   'BlockdevOptionsNull',
   'null-co':'BlockdevOptionsNull',
-- 
2.8.0

[Qemu-devel] [PATCH v3 09/14] block/nbd: Use SocketAddress options

[Max Reitz]

Drop the use of legacy options in favor of the SocketAddress
representation, even for internal use (i.e. for storing the result of
the filename parsing).

Signed-off-by: Max Reitz 
---
 block/nbd.c | 34 +-
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 9ae66ba..82dcb5e 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -89,9 +89,13 @@ static int nbd_parse_uri(const char *filename, QDict 
*options)
 ret = -EINVAL;
 goto out;
 }
-qdict_put(options, "path", qstring_from_str(qp->p[0].value));
+qdict_put(options, "address.type", qstring_from_str("unix"));
+qdict_put(options, "address.data.path",
+  qstring_from_str(qp->p[0].value));
 } else {
 QString *host;
+char *port_str;
+
 /* nbd[+tcp]://host[:port]/export */
 if (!uri->server) {
 ret = -EINVAL;
@@ -106,12 +110,12 @@ static int nbd_parse_uri(const char *filename, QDict 
*options)
 host = qstring_from_str(uri->server);
 }
 
-qdict_put(options, "host", host);
-if (uri->port) {
-char* port_str = g_strdup_printf("%d", uri->port);
-qdict_put(options, "port", qstring_from_str(port_str));
-g_free(port_str);
-}
+qdict_put(options, "address.type", qstring_from_str("inet"));
+qdict_put(options, "address.data.host", host);
+
+port_str = g_strdup_printf("%d", uri->port ?: NBD_DEFAULT_PORT);
+qdict_put(options, "address.data.port", qstring_from_str(port_str));
+g_free(port_str);
 }
 
 out:
@@ -188,7 +192,8 @@ static void nbd_parse_filename(const char *filename, QDict 
*options,
 
 /* are we a UNIX or TCP socket? */
 if (strstart(host_spec, "unix:", )) {
-qdict_put(options, "path", qstring_from_str(unixpath));
+qdict_put(options, "address.type", qstring_from_str("unix"));
+qdict_put(options, "address.data.path", qstring_from_str(unixpath));
 } else {
 InetSocketAddress *addr = NULL;
 
@@ -197,8 +202,9 @@ static void nbd_parse_filename(const char *filename, QDict 
*options,
 goto out;
 }
 
-qdict_put(options, "host", qstring_from_str(addr->host));
-qdict_put(options, "port", qstring_from_str(addr->port));
+qdict_put(options, "address.type", qstring_from_str("inet"));
+qdict_put(options, "address.data.host", qstring_from_str(addr->host));
+qdict_put(options, "address.data.port", qstring_from_str(addr->port));
 qapi_free_InetSocketAddress(addr);
 }
 
@@ -528,10 +534,12 @@ static void nbd_refresh_filename(BlockDriverState *bs, 
QDict *options)
 }
 } else {
 if (path) {
-qdict_put(opts, "path", qstring_from_str(path));
+qdict_put(opts, "address.type", qstring_from_str("unix"));
+qdict_put(opts, "address.data.path", qstring_from_str(path));
 } else {
-qdict_put(opts, "host", qstring_from_str(host));
-qdict_put(opts, "port", qstring_from_str(port));
+qdict_put(opts, "address.type", qstring_from_str("inet"));
+qdict_put(opts, "address.data.host", qstring_from_str(host));
+qdict_put(opts, "address.data.port", qstring_from_str(port));
 }
 }
 if (export) {
-- 
2.8.0

[Qemu-devel] [PATCH v3 07/14] block/nbd: "address" in nbd_refresh_filename()

[Max Reitz]

As of a future patch, the NBD block driver will accept a SocketAddress
structure for a new "address" option. In order to support this,
nbd_refresh_filename() needs some changes.

The two TODOs introduced by this patch will be removed in the very next
one. They exist to explain that it is currently impossible for
nbd_refresh_filename() to emit an "address.*" option (which the NBD
block driver does not handle yet). The next patch will arm these code
paths, but it will also enable handling of these options.

Signed-off-by: Max Reitz 
---
 block/nbd.c | 84 -
 1 file changed, 61 insertions(+), 23 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 1736f68..3adf302 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -441,37 +441,75 @@ static void nbd_attach_aio_context(BlockDriverState *bs,
 static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
 {
 QDict *opts = qdict_new();
-const char *path   = qdict_get_try_str(options, "path");
-const char *host   = qdict_get_try_str(options, "host");
-const char *port   = qdict_get_try_str(options, "port");
+bool can_generate_filename = true;
+const char *path = NULL, *host = NULL, *port = NULL;
 const char *export = qdict_get_try_str(options, "export");
 const char *tlscreds = qdict_get_try_str(options, "tls-creds");
 
-if (host && !port) {
-port = stringify(NBD_DEFAULT_PORT);
+if (qdict_get_try_str(options, "address.type")) {
+/* This path will only be possible as of a future patch;
+ * TODO: Remove this note once it is */
+
+const char *type = qdict_get_str(options, "address.type");
+
+if (!strcmp(type, "unix")) {
+path = qdict_get_str(options, "address.data.path");
+} else if (!strcmp(type, "inet")) {
+host = qdict_get_str(options, "address.data.host");
+port = qdict_get_str(options, "address.data.port");
+
+can_generate_filename = !qdict_haskey(options, "address.data.to")
+ && !qdict_haskey(options, "address.data.ipv4")
+ && !qdict_haskey(options, 
"address.data.ipv6");
+} else {
+can_generate_filename = false;
+}
+} else {
+path = qdict_get_try_str(options, "path");
+host = qdict_get_try_str(options, "host");
+port = qdict_get_try_str(options, "port");
+
+if (host && !port) {
+port = stringify(NBD_DEFAULT_PORT);
+}
 }
 
 qdict_put(opts, "driver", qstring_from_str("nbd"));
 
-if (path && export) {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd+unix:///%s?socket=%s", export, path);
-} else if (path && !export) {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd+unix://?socket=%s", path);
-} else if (!path && export) {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd://%s:%s/%s", host, port, export);
-} else if (!path && !export) {
-snprintf(bs->exact_filename, sizeof(bs->exact_filename),
- "nbd://%s:%s", host, port);
-}
-
-if (path) {
-qdict_put(opts, "path", qstring_from_str(path));
+if (can_generate_filename) {
+if (path && export) {
+snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+ "nbd+unix:///%s?socket=%s", export, path);
+} else if (path && !export) {
+snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+ "nbd+unix://?socket=%s", path);
+} else if (!path && export) {
+snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+ "nbd://%s:%s/%s", host, port, export);
+} else if (!path && !export) {
+snprintf(bs->exact_filename, sizeof(bs->exact_filename),
+ "nbd://%s:%s", host, port);
+}
+}
+
+if (qdict_get_try_str(options, "address.type")) {
+/* This path will only be possible as of a future patch;
+ * TODO: Remove this note once it is */
+
+const QDictEntry *e;
+for (e = qdict_first(options); e; e = qdict_next(options, e)) {
+if (!strncmp(e->key, "address.", 8)) {
+qobject_incref(e->value);
+qdict_put_obj(opts, e->key, e->value);
+}
+}
 } else {
-qdict_put(opts, "host", qstring_from_str(host));
-qdict_put(opts, "port", qstring_from_str(port));
+if (path) {
+qdict_put(opts, "path", qstring_from_str(path));
+} else {
+qdict_put(opts, "host", qstring_from_str(host));
+qdict_put(opts, "port", qstring_from_str(port));
+}
 }
 if (export) {
 qdict_put(opts, "export", qstring_from_str(export));
-- 
2.8.0

[Qemu-devel] [PATCH v3 02/14] block/nbd: Drop trailing "." in error messages

[Max Reitz]

Signed-off-by: Max Reitz 
---
 block/nbd.c   | 4 ++--
 tests/qemu-iotests/051.out| 4 ++--
 tests/qemu-iotests/051.pc.out | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index f7ea3b3..d9b946f 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -195,9 +195,9 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QDict 
*options, char **export,
 
 if (qdict_haskey(options, "path") == qdict_haskey(options, "host")) {
 if (qdict_haskey(options, "path")) {
-error_setg(errp, "path and host may not be used at the same 
time.");
+error_setg(errp, "path and host may not be used at the same time");
 } else {
-error_setg(errp, "one of path and host must be specified.");
+error_setg(errp, "one of path and host must be specified");
 }
 return NULL;
 }
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index c1291ff..5b3e991 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -222,7 +222,7 @@ Testing: -drive driver=file
 QEMU_PROG: -drive driver=file: The 'file' block driver requires a file name
 
 Testing: -drive driver=nbd
-QEMU_PROG: -drive driver=nbd: one of path and host must be specified.
+QEMU_PROG: -drive driver=nbd: one of path and host must be specified
 
 Testing: -drive driver=raw
 QEMU_PROG: -drive driver=raw: Can't use 'raw' as a block driver for the 
protocol level
@@ -231,7 +231,7 @@ Testing: -drive file.driver=file
 QEMU_PROG: -drive file.driver=file: The 'file' block driver requires a file 
name
 
 Testing: -drive file.driver=nbd
-QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified.
+QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified
 
 Testing: -drive file.driver=raw
 QEMU_PROG: -drive file.driver=raw: Can't use 'raw' as a block driver for the 
protocol level
diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
index ec6d222..6395a30 100644
--- a/tests/qemu-iotests/051.pc.out
+++ b/tests/qemu-iotests/051.pc.out
@@ -316,7 +316,7 @@ Testing: -drive driver=file
 QEMU_PROG: -drive driver=file: The 'file' block driver requires a file name
 
 Testing: -drive driver=nbd
-QEMU_PROG: -drive driver=nbd: one of path and host must be specified.
+QEMU_PROG: -drive driver=nbd: one of path and host must be specified
 
 Testing: -drive driver=raw
 QEMU_PROG: -drive driver=raw: Can't use 'raw' as a block driver for the 
protocol level
@@ -325,7 +325,7 @@ Testing: -drive file.driver=file
 QEMU_PROG: -drive file.driver=file: The 'file' block driver requires a file 
name
 
 Testing: -drive file.driver=nbd
-QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified.
+QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified
 
 Testing: -drive file.driver=raw
 QEMU_PROG: -drive file.driver=raw: Can't use 'raw' as a block driver for the 
protocol level
-- 
2.8.0

[Qemu-devel] [PATCH v3 06/14] block/nbd: Add nbd_has_filename_options_conflict()

[Max Reitz]

Right now, we have four possible options that conflict with specifying
an NBD filename, and a future patch will add another one ("address").
This future option is a nested QDict that is flattened at this point,
requiring as to test each option whether its key has an "address."
prefix. Therefore, we will then need to iterate through all options.

Adding this iteration logic now will simplify adding the new option
later. A nice side effect is that the user will not receive a long list
of five options which are not supposed to be specified with a filename,
but we can actually print the problematic option.

Signed-off-by: Max Reitz 
---
 block/nbd.c | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index d12bcc6..1736f68 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -120,6 +120,25 @@ out:
 return ret;
 }
 
+static bool nbd_has_filename_options_conflict(QDict *options, Error **errp)
+{
+const QDictEntry *e;
+
+for (e = qdict_first(options); e; e = qdict_next(options, e)) {
+if (!strcmp(e->key, "host")
+|| !strcmp(e->key, "port")
+|| !strcmp(e->key, "path")
+|| !strcmp(e->key, "export"))
+{
+error_setg(errp, "Option '%s' cannot be used with a file name",
+   e->key);
+return true;
+}
+}
+
+return false;
+}
+
 static void nbd_parse_filename(const char *filename, QDict *options,
Error **errp)
 {
@@ -128,12 +147,7 @@ static void nbd_parse_filename(const char *filename, QDict 
*options,
 const char *host_spec;
 const char *unixpath;
 
-if (qdict_haskey(options, "host")
-|| qdict_haskey(options, "port")
-|| qdict_haskey(options, "path"))
-{
-error_setg(errp, "host/port/path and a file name may not be specified "
- "at the same time");
+if (nbd_has_filename_options_conflict(options, errp)) {
 return;
 }
 
-- 
2.8.0

[Qemu-devel] [PATCH v3 03/14] block/nbd: Reject port parameter without host

[Max Reitz]

This is better than the generic block layer finding out later that the
port parameter has not been used.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
---
 block/nbd.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/block/nbd.c b/block/nbd.c
index d9b946f..2112ec0 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -201,6 +201,10 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QDict 
*options, char **export,
 }
 return NULL;
 }
+if (qdict_haskey(options, "port") && !qdict_haskey(options, "host")) {
+error_setg(errp, "port may not be used without host");
+return NULL;
+}
 
 saddr = g_new0(SocketAddress, 1);
 
-- 
2.8.0

[Qemu-devel] [PATCH v3 for-2.7 00/14] qapi: Allow blockdev-add for NBD

[Max Reitz]

Turns out NBD is not so simple to do if you do it right. Anyway, this
series adds blockdev-add support for NBD clients.

Patch 1 adds a QDict function which I needed in later NBD patches for
handling legacy options (move "host" to "address.data.host" etc.).

Patches 2, 3, 4, and 5 are minor patches with no functional relation to
this series, other than later patches will touch the code they touch,
too.

Patches 6 and 7 prepare the code for the addition of a new option
prefix, which is "address.".

Patch 8 makes the NBD client accept a SocketAddress under the "address"
option (or rather, a flattened SocketAddress QDict with its keys
prefixed by "address."). The old options "host", "port", and "path" are
supported as legacy options and translated to the respective
SocketAddress representation.

Patch 9 drops usage of "host", "port", and "path" outside of
nbd_has_filename_options_conflict(),
nbd_process_legacy_socket_options(), and nbd_refresh_filename(), making
those options nothing but legacy.

Patch 10, the goal of this series, is again not very complicated.

Patches 11, 12, and 13 are required for the iotest added in patch 16. It
will invoke qemu-nbd, so patch 13 is required. Besides qemu-nbd, it will
launch an NBD server VM concurrently to the client VM, which is why
patch 14 is required. And finally, it will test whether we can add an
NBD BDS by passing it a file descriptor, which patch 15 is needed for
(so we use the socket_scm_helper to pass sockets to qemu).

Patch 14 then adds the iotest for NBD's blockdev-add interface.


*** This series requires Daniel's qdict_crumple() function (from his
"Provide a QOM-based authorization API" series). ***


v2:
- Dropped patches 2 and 3; use Daniel's qdict_crumple() instead.
- Patch 7: Not sure why the diff differs, seems functionally like the
  same patch to me. Maybe git changed something about its default diff
  algorithm.
- Patch 8: Use qdict_crumple() instead of qdict_unflatten()
- Patch 10: Rebase conflicts


git-backport-diff against v2:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/14:[] [--] 'qdict: Add qdict_change_key()'
002/14:[] [--] 'block/nbd: Drop trailing "." in error messages'
003/14:[] [--] 'block/nbd: Reject port parameter without host'
004/14:[] [--] 'block/nbd: Default port in nbd_refresh_filename()'
005/14:[] [--] 'block/nbd: Use qdict_put()'
006/14:[] [--] 'block/nbd: Add nbd_has_filename_options_conflict()'
007/14:[0008] [FC] 'block/nbd: "address" in nbd_refresh_filename()'
008/14:[0024] [FC] 'block/nbd: Accept SocketAddress'
009/14:[] [--] 'block/nbd: Use SocketAddress options'
010/14:[0004] [FC] 'qapi: Allow blockdev-add for NBD'
011/14:[] [-C] 'iotests.py: Add qemu_nbd function'
012/14:[] [--] 'iotests.py: Allow concurrent qemu instances'
013/14:[] [--] 'socket_scm_helper: Accept fd directly'
014/14:[] [-C] 'iotests: Add test for NBD's blockdev-add interface'


Max Reitz (14):
  qdict: Add qdict_change_key()
  block/nbd: Drop trailing "." in error messages
  block/nbd: Reject port parameter without host
  block/nbd: Default port in nbd_refresh_filename()
  block/nbd: Use qdict_put()
  block/nbd: Add nbd_has_filename_options_conflict()
  block/nbd: "address" in nbd_refresh_filename()
  block/nbd: Accept SocketAddress
  block/nbd: Use SocketAddress options
  qapi: Allow blockdev-add for NBD
  iotests.py: Add qemu_nbd function
  iotests.py: Allow concurrent qemu instances
  socket_scm_helper: Accept fd directly
  iotests: Add test for NBD's blockdev-add interface

 block/nbd.c| 235 ++---
 include/qapi/qmp/qdict.h   |   1 +
 qapi/block-core.json   |  23 +++-
 qobject/qdict.c|  23 
 tests/qemu-iotests/051.out |   4 +-
 tests/qemu-iotests/051.pc.out  |   4 +-
 tests/qemu-iotests/147 | 194 +++
 tests/qemu-iotests/147.out |   5 +
 tests/qemu-iotests/group   |   1 +
 tests/qemu-iotests/iotests.py  |  22 ++-
 tests/qemu-iotests/socket_scm_helper.c |  29 ++--
 11 files changed, 443 insertions(+), 98 deletions(-)
 create mode 100755 tests/qemu-iotests/147
 create mode 100644 tests/qemu-iotests/147.out

-- 
2.8.0

[Qemu-devel] [PATCH v3 01/14] qdict: Add qdict_change_key()

[Max Reitz]

This is a shorthand function for changing a QDict's entry's key.

Signed-off-by: Max Reitz 
---
 include/qapi/qmp/qdict.h |  1 +
 qobject/qdict.c  | 23 +++
 2 files changed, 24 insertions(+)

diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 8a3ac13..3e0d7df 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -39,6 +39,7 @@ size_t qdict_size(const QDict *qdict);
 void qdict_put_obj(QDict *qdict, const char *key, QObject *value);
 void qdict_del(QDict *qdict, const char *key);
 int qdict_haskey(const QDict *qdict, const char *key);
+bool qdict_change_key(QDict *qdict, const char *old_key, const char *new_key);
 QObject *qdict_get(const QDict *qdict, const char *key);
 QDict *qobject_to_qdict(const QObject *obj);
 void qdict_iter(const QDict *qdict,
diff --git a/qobject/qdict.c b/qobject/qdict.c
index ae6ad06..2eacb3d 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -170,6 +170,29 @@ int qdict_haskey(const QDict *qdict, const char *key)
 }
 
 /**
+ * qdict_change_key(): Changes an entry's key
+ *
+ * Removes the entry with the key 'old_key' and inserts its associated value as
+ * a new entry with the key 'new_key'.
+ *
+ * Returns false if 'old_key' does not exist, true otherwise.
+ */
+bool qdict_change_key(QDict *qdict, const char *old_key, const char *new_key)
+{
+QObject *value = qdict_get(qdict, old_key);
+
+if (!value) {
+return false;
+}
+
+qobject_incref(value);
+qdict_del(qdict, old_key);
+qdict_put_obj(qdict, new_key, value);
+
+return true;
+}
+
+/**
  * qdict_size(): Return the size of the dictionary
  */
 size_t qdict_size(const QDict *qdict)
-- 
2.8.0

Re: [Qemu-devel] [PATCH v3 0/7] bdrv_flush_io_queue removal, shared LinuxAioState

[Paolo Bonzini]

On 06/04/2016 20:19, Kevin Wolf wrote:
>> > 
>> > Great, I'll send v4 as soon as possible for inclusion in the block-next
>> > branch.
> Are you still working on v4 or did I miss it? All of the block-next
> candidate series are waiting to be rebased on this one, and moving parts
> at the bottom of a longish dependency chain are kind of nasty.

Yes, I am waiting for Fam's bdrv_co_drain patch, which is this one
depends on.

Paolo

Re: [Qemu-devel] [PATCH 07/10] tb hash: hash phys_pc, pc, and flags with xxhash

[Richard Henderson]

On 04/06/2016 11:23 AM, Paolo Bonzini wrote:
> Perhaps better is to always use a three-word xxhash, but pick the 64-bit
> version if any of phys_pc and pc are 64-bits.  The unrolling would be
> very effective, and the performance penalty not too important (64-bit on
> 32-bit is very slow anyway).

True.

It would also be interesting to put an average bucket chain length into the
"info jit" dump, so that it's easy to see how the hashing is performing for a
given guest.


r~

Re: [Qemu-devel] [PATCH 07/10] tb hash: hash phys_pc, pc, and flags with xxhash

[Paolo Bonzini]

On 06/04/2016 19:44, Emilio G. Cota wrote:
> I like this idea, because the ugliness of the sizeof checks is significant.
> However, the quality of the resulting hash is not as good when always using 
> func5.
> For instance, when we'd otherwise use func3, two fifths of every input contain
> exactly the same bits: all 0's. This inevitably leads to more collisions.

It doesn't necessarily lead to more collisions.  For a completely stupid
hash function "a+b+c+d+e", for example, adding zeros doesn't add more
collisions.

What if you rearrange the five words so that the "all 0" parts of the
input are all at the beginning, or all at the end?  Perhaps the problem
is that the odd words at the end are hashed less effectively.

Perhaps better is to always use a three-word xxhash, but pick the 64-bit
version if any of phys_pc and pc are 64-bits.  The unrolling would be
very effective, and the performance penalty not too important (64-bit on
32-bit is very slow anyway).

If you can fix the problems with the collisions, it also means that you
get good performance running 32-bit guests on qemu-system-x86_64 or
qemu-system-aarch64.  So, if you can do it, it's a very desirable property.

Paolo

Re: [Qemu-devel] [PATCH v3 0/7] bdrv_flush_io_queue removal, shared LinuxAioState

[Kevin Wolf]

Am 30.03.2016 um 15:17 hat Paolo Bonzini geschrieben:
> On 30/03/2016 15:06, Stefan Hajnoczi wrote:
> > Acked-by: Stefan Hajnoczi 
> 
> Great, I'll send v4 as soon as possible for inclusion in the block-next
> branch.

Are you still working on v4 or did I miss it? All of the block-next
candidate series are waiting to be rebased on this one, and moving parts
at the bottom of a longish dependency chain are kind of nasty.

Kevin

[Qemu-devel] [PATCH v2 for-2.7 6/8] block: Make bdrv_open() return a BDS

[Max Reitz]

There are no callers to bdrv_open() or bdrv_open_inherit() left that
pass a pointer to a non-NULL BDS pointer as the first argument of these
functions, so we can finally drop that parameter and just make them
return the new BDS.

Generally, the following pattern is applied:

bs = NULL;
ret = bdrv_open(, ..., _err);
if (ret < 0) {
error_propagate(errp, local_err);
...
}

by

bs = bdrv_open(..., errp);
if (!bs) {
ret = -EINVAL;
...
}

Of course, there are only a few instances where the pattern is really
pure.

Signed-off-by: Max Reitz 
---
 block.c   | 123 +-
 block/block-backend.c |   6 +--
 block/vvfat.c |   8 ++--
 blockdev.c|  38 ++--
 include/block/block.h |   4 +-
 5 files changed, 63 insertions(+), 116 deletions(-)

diff --git a/block.c b/block.c
index b6d1796..355c4be 100644
--- a/block.c
+++ b/block.c
@@ -64,10 +64,12 @@ static QTAILQ_HEAD(, BlockDriverState) all_bdrv_states =
 static QLIST_HEAD(, BlockDriver) bdrv_drivers =
 QLIST_HEAD_INITIALIZER(bdrv_drivers);
 
-static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
- const char *reference, QDict *options, int flags,
- BlockDriverState *parent,
- const BdrvChildRole *child_role, Error **errp);
+static BlockDriverState *bdrv_open_inherit(const char *filename,
+   const char *reference,
+   QDict *options, int flags,
+   BlockDriverState *parent,
+   const BdrvChildRole *child_role,
+   Error **errp);
 
 /* If non-zero, use only whitelisted block drivers */
 static int use_bdrv_whitelist;
@@ -1338,14 +1340,13 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
*parent_options,
 qdict_put(options, "driver", qstring_from_str(bs->backing_format));
 }
 
-backing_hd = NULL;
-ret = bdrv_open_inherit(_hd,
-*backing_filename ? backing_filename : NULL,
-reference, options, 0, bs, _backing,
-errp);
-if (ret < 0) {
+backing_hd = bdrv_open_inherit(*backing_filename ? backing_filename : NULL,
+   reference, options, 0, bs, _backing,
+   errp);
+if (!backing_hd) {
 bs->open_flags |= BDRV_O_NO_BACKING;
 error_prepend(errp, "Could not open backing file: ");
+ret = -EINVAL;
 goto free_exit;
 }
 
@@ -1385,7 +1386,6 @@ BdrvChild *bdrv_open_child(const char *filename,
 BdrvChild *c = NULL;
 BlockDriverState *bs;
 QDict *image_options;
-int ret;
 char *bdref_key_dot;
 const char *reference;
 
@@ -1405,10 +1405,9 @@ BdrvChild *bdrv_open_child(const char *filename,
 goto done;
 }
 
-bs = NULL;
-ret = bdrv_open_inherit(, filename, reference, image_options, 0,
-parent, child_role, errp);
-if (ret < 0) {
+bs = bdrv_open_inherit(filename, reference, image_options, 0,
+   parent, child_role, errp);
+if (!bs) {
 goto done;
 }
 
@@ -1429,7 +1428,6 @@ static BlockDriverState 
*bdrv_append_temp_snapshot(BlockDriverState *bs,
 int64_t total_size;
 QemuOpts *opts = NULL;
 BlockDriverState *bs_snapshot;
-Error *local_err = NULL;
 int ret;
 
 /* if snapshot, we create a temporary backing file and open it
@@ -1468,12 +1466,10 @@ static BlockDriverState 
*bdrv_append_temp_snapshot(BlockDriverState *bs,
 qdict_put(snapshot_options, "driver",
   qstring_from_str("qcow2"));
 
-bs_snapshot = NULL;
-ret = bdrv_open(_snapshot, NULL, NULL, snapshot_options,
-flags, _err);
+bs_snapshot = bdrv_open(NULL, NULL, snapshot_options, flags, errp);
 snapshot_options = NULL;
-if (ret < 0) {
-error_propagate(errp, local_err);
+if (!bs_snapshot) {
+ret = -EINVAL;
 goto out;
 }
 
@@ -1504,10 +1500,12 @@ out:
  * should be opened. If specified, neither options nor a filename may be given,
  * nor can an existing BDS be reused (that is, *pbs has to be NULL).
  */
-static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
- const char *reference, QDict *options, int flags,
- BlockDriverState *parent,
- const BdrvChildRole *child_role, Error **errp)
+static BlockDriverState *bdrv_open_inherit(const char *filename,
+   const char *reference,
+   QDict *options, int flags,
+   

[Qemu-devel] [PATCH v2 for-2.7 4/8] block: Drop blk_new_with_bs()

[Max Reitz]

Its only caller is blk_new_open(), so we can just inline it there.

Signed-off-by: Max Reitz 
---
 block/block-backend.c | 28 ++--
 1 file changed, 6 insertions(+), 22 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 66b8bad..4e8e8ab 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -175,26 +175,6 @@ BlockBackend *blk_new(Error **errp)
 }
 
 /*
- * Create a new BlockBackend with a new BlockDriverState attached.
- * Otherwise just like blk_new(), which see.
- */
-BlockBackend *blk_new_with_bs(Error **errp)
-{
-BlockBackend *blk;
-BlockDriverState *bs;
-
-blk = blk_new(errp);
-if (!blk) {
-return NULL;
-}
-
-bs = bdrv_new_root();
-blk->root = bdrv_root_attach_child(bs, "root", _root);
-blk->root->opaque = blk;
-return blk;
-}
-
-/*
  * Calls blk_new_with_bs() and then calls bdrv_open() on the BlockDriverState.
  *
  * Just as with bdrv_open(), after having called this function the reference to
@@ -210,21 +190,25 @@ BlockBackend *blk_new_open(const char *filename, const 
char *reference,
QDict *options, int flags, Error **errp)
 {
 BlockBackend *blk;
+BlockDriverState *bs;
 int ret;
 
-blk = blk_new_with_bs(errp);
+blk = blk_new(errp);
 if (!blk) {
 QDECREF(options);
 return NULL;
 }
 
-ret = bdrv_open(>root->bs, filename, reference, options, flags, errp);
+bs = NULL;
+ret = bdrv_open(, filename, reference, options, flags, errp);
 if (ret < 0) {
 blk_unref(blk);
 return NULL;
 }
 
 blk_set_enable_write_cache(blk, true);
+blk->root = bdrv_root_attach_child(bs, "root", _root);
+blk->root->opaque = blk;
 
 return blk;
 }
-- 
2.8.0

[Qemu-devel] [PATCH v2 for-2.7 3/8] tests: Drop BDS from test-throttle.c

[Max Reitz]

Now that throttling has been moved to the BlockBackend level, we do not
need to create a BDS along with the BB in the I/O throttling test.

Signed-off-by: Max Reitz 
---
 tests/test-throttle.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 5ec966c..d7fb0a6 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -578,9 +578,9 @@ static void test_groups(void)
 BlockBackend *blk1, *blk2, *blk3;
 BlockBackendPublic *blkp1, *blkp2, *blkp3;
 
-blk1 = blk_new_with_bs(_abort);
-blk2 = blk_new_with_bs(_abort);
-blk3 = blk_new_with_bs(_abort);
+blk1 = blk_new(_abort);
+blk2 = blk_new(_abort);
+blk3 = blk_new(_abort);
 
 blkp1 = blk_get_public(blk1);
 blkp2 = blk_get_public(blk2);
-- 
2.8.0

[Qemu-devel] [PATCH v2 for-2.7 2/8] block: Let bdrv_open_inherit() return the snapshot

[Max Reitz]

If bdrv_open_inherit() creates a snapshot BDS and *pbs is NULL, that
snapshot BDS should be returned instead of the BDS under it.

To this end, bdrv_append_temp_snapshot() now returns the snapshot BDS
instead of just appending it on top of the snapshotted BDS. Also, it
calls bdrv_ref() before bdrv_append() (which bdrv_open_inherit() has to
undo if not returning the overlay).

Signed-off-by: Max Reitz 
---
 block.c | 33 ++---
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index b6a452a..d27 100644
--- a/block.c
+++ b/block.c
@@ -1424,8 +1424,10 @@ done:
 return c;
 }
 
-static int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags,
- QDict *snapshot_options, Error **errp)
+static BlockDriverState *bdrv_append_temp_snapshot(BlockDriverState *bs,
+   int flags,
+   QDict *snapshot_options,
+   Error **errp)
 {
 /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
 char *tmp_filename = g_malloc0(PATH_MAX + 1);
@@ -1441,7 +1443,6 @@ static int bdrv_append_temp_snapshot(BlockDriverState 
*bs, int flags,
 /* Get the required size from the image */
 total_size = bdrv_getlength(bs);
 if (total_size < 0) {
-ret = total_size;
 error_setg_errno(errp, -total_size, "Could not get image size");
 goto out;
 }
@@ -1481,12 +1482,16 @@ static int bdrv_append_temp_snapshot(BlockDriverState 
*bs, int flags,
 goto out;
 }
 
+bdrv_ref(bs_snapshot);
 bdrv_append(bs_snapshot, bs);
 
+g_free(tmp_filename);
+return bs_snapshot;
+
 out:
 QDECREF(snapshot_options);
 g_free(tmp_filename);
-return ret;
+return NULL;
 }
 
 /*
@@ -1705,17 +1710,31 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
 }
 
 QDECREF(options);
-*pbs = bs;
 
 /* For snapshot=on, create a temporary qcow2 overlay. bs points to the
  * temporary snapshot afterwards. */
 if (snapshot_flags) {
-ret = bdrv_append_temp_snapshot(bs, snapshot_flags, snapshot_options,
-_err);
+BlockDriverState *snapshot_bs;
+snapshot_bs = bdrv_append_temp_snapshot(bs, snapshot_flags,
+snapshot_options, _err);
 snapshot_options = NULL;
 if (local_err) {
+ret = -EINVAL;
 goto close_and_fail;
 }
+if (!*pbs) {
+/* The reference is now held by the overlay BDS */
+bdrv_unref(bs);
+bs = snapshot_bs;
+} else {
+/* It is still referenced in the same way that *pbs was referenced,
+ * however that may be */
+bdrv_unref(snapshot_bs);
+}
+}
+
+if (!*pbs) {
+*pbs = bs;
 }
 
 return 0;
-- 
2.8.0

[Qemu-devel] [PATCH v2 for-2.7 7/8] block: Assert !bs->refcnt in bdrv_close()

[Max Reitz]

The only caller of bdrv_close() left is bdrv_delete(). We may as well
assert that, in a way (there are some things in bdrv_close() that make
more sense under that assumption, such as the call to
bdrv_release_all_dirty_bitmaps() which in turn assumes that no frozen
bitmaps are attached to the BDS).

In addition, being called only in bdrv_delete() means that we can drop
bdrv_close()'s forward declaration at the top of block.c.

Signed-off-by: Max Reitz 
Reviewed-by: Alberto Garcia 
---
 block.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 355c4be..963abcc 100644
--- a/block.c
+++ b/block.c
@@ -74,8 +74,6 @@ static BlockDriverState *bdrv_open_inherit(const char 
*filename,
 /* If non-zero, use only whitelisted block drivers */
 static int use_bdrv_whitelist;
 
-static void bdrv_close(BlockDriverState *bs);
-
 #ifdef _WIN32
 static int is_windows_drive_prefix(const char *filename)
 {
@@ -2106,6 +2104,7 @@ static void bdrv_close(BlockDriverState *bs)
 BdrvAioNotifier *ban, *ban_next;
 
 assert(!bs->job);
+assert(!bs->refcnt);
 
 bdrv_drained_begin(bs); /* complete I/O */
 bdrv_flush(bs);
-- 
2.8.0

[Qemu-devel] [PATCH v2 for-2.7 8/8] block: Drop bdrv_parent_cb_...() from bdrv_close()

[Max Reitz]

bdrv_close() now asserts that the BDS's refcount is 0, therefore it
cannot have any parents and the bdrv_parent_cb_change_media() call is a
no-op.

Signed-off-by: Max Reitz 
---
 block.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/block.c b/block.c
index 963abcc..7d7fd89 100644
--- a/block.c
+++ b/block.c
@@ -2113,8 +2113,6 @@ static void bdrv_close(BlockDriverState *bs)
 bdrv_release_named_dirty_bitmaps(bs);
 assert(QLIST_EMPTY(>dirty_bitmaps));
 
-bdrv_parent_cb_change_media(bs, false);
-
 if (bs->drv) {
 BdrvChild *child, *next;
 
-- 
2.8.0

[Qemu-devel] [PATCH v2 for-2.7 5/8] block: Drop bdrv_new_root()

[Max Reitz]

By now it has become just a wrapper around bdrv_new() and it has only a
single user. Use bdrv_new() there instead and drop this function.

Signed-off-by: Max Reitz 
Reviewed-by: Alberto Garcia 
---
 block.c   | 5 -
 include/block/block.h | 1 -
 2 files changed, 6 deletions(-)

diff --git a/block.c b/block.c
index d27..b6d1796 100644
--- a/block.c
+++ b/block.c
@@ -222,11 +222,6 @@ void bdrv_register(BlockDriver *bdrv)
 QLIST_INSERT_HEAD(_drivers, bdrv, list);
 }
 
-BlockDriverState *bdrv_new_root(void)
-{
-return bdrv_new();
-}
-
 BlockDriverState *bdrv_new(void)
 {
 BlockDriverState *bs;
diff --git a/include/block/block.h b/include/block/block.h
index 762984e..31fcd07 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -198,7 +198,6 @@ BlockDriver *bdrv_find_format(const char *format_name);
 int bdrv_create(BlockDriver *drv, const char* filename,
 QemuOpts *opts, Error **errp);
 int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp);
-BlockDriverState *bdrv_new_root(void);
 BlockDriverState *bdrv_new(void);
 void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
 void bdrv_replace_in_backing_chain(BlockDriverState *old,
-- 
2.8.0

[Qemu-devel] [PATCH v2 for-2.7 1/8] block: Drop useless bdrv_new() call

[Max Reitz]

bdrv_append_temp_snapshot() uses bdrv_new() to create an empty BDS
before invoking bdrv_open() on that BDS. This is probably a relict from
when it used to do some modifications on that empty BDS, but now that is
unnecessary, so we can just set bs_snapshot to NULL and let bdrv_open()
do the rest.

Signed-off-by: Max Reitz 
---
 block.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 1be887a..b6a452a 100644
--- a/block.c
+++ b/block.c
@@ -1472,8 +1472,7 @@ static int bdrv_append_temp_snapshot(BlockDriverState 
*bs, int flags,
 qdict_put(snapshot_options, "driver",
   qstring_from_str("qcow2"));
 
-bs_snapshot = bdrv_new();
-
+bs_snapshot = NULL;
 ret = bdrv_open(_snapshot, NULL, NULL, snapshot_options,
 flags, _err);
 snapshot_options = NULL;
-- 
2.8.0

[Qemu-devel] [PATCH v2 for-2.7 0/8] blockdev: (Nearly) free clean-up work

[Max Reitz]

After a lot has been restructed in the block layer in the past, we can
now reap at least one of the fruits: Make bdrv_open() return a BDS!


This series depends on the following series/patches:
- Revert "block: Forbid I/O throttling on nodes with multiple parents
  for 2.6"
  This is something I suppose Kevin will send when the 2.7 development
  window opens.
- "block: Move I/O throttling to BlockBackend" by Kevin
- "block: Remove BlockDriverState.blk" by Kevin


v2:
- Patch 1: bdrv_open_backing_file() has been changed already, so that
  part can be dropped from the patch
- Patch 2:
  - bdrv_append_temp_snapshot() now takes an additional argument
(snapshot_options)
  - Drop a superfluous assignment to ret [Berto]
  - The function is now also static already, so we don't need to do that
- Patch 3: Added because this test has been added in the meantime
- Patch 4: Rebase conflicts
- Patch 6:
  - Rebase conflicts
  - A whole lot of places we no longer need to touch because they use
blk_new_open() instead of bdrv_open()
- Patch 8: Added because patch 7 makes this possible.


git-backport-diff against v1:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/8:[down] 'block: Drop useless bdrv_new() call'
    Actually [0006] [FC] with a changed commit title
002/8:[0015] [FC] 'block: Let bdrv_open_inherit() return the snapshot'
003/8:[down] 'tests: Drop BDS from test-throttle.c'
004/8:[0019] [FC] 'block: Drop blk_new_with_bs()'
005/8:[] [-C] 'block: Drop bdrv_new_root()'
006/8:[0135] [FC] 'block: Make bdrv_open() return a BDS'
007/8:[] [-C] 'block: Assert !bs->refcnt in bdrv_close()'
008/8:[down] 'block: Drop bdrv_parent_cb_...() from bdrv_close()'


Max Reitz (8):
  block: Drop useless bdrv_new() call
  block: Let bdrv_open_inherit() return the snapshot
  tests: Drop BDS from test-throttle.c
  block: Drop blk_new_with_bs()
  block: Drop bdrv_new_root()
  block: Make bdrv_open() return a BDS
  block: Assert !bs->refcnt in bdrv_close()
  block: Drop bdrv_parent_cb_...() from bdrv_close()

 block.c   | 139 --
 block/block-backend.c |  30 +++
 block/vvfat.c |   8 +--
 blockdev.c|  38 +-
 include/block/block.h |   5 +-
 tests/test-throttle.c |   6 +--
 6 files changed, 83 insertions(+), 143 deletions(-)

-- 
2.8.0

Re: [Qemu-devel] [PATCH 0/2] Add gpio_key and use it for ARM virt power button

[Emilio G. Cota]

On Wed, Mar 23, 2016 at 16:12:46 +, Peter Maydell wrote:
> On 17 March 2016 at 13:25, Shannon Zhao  wrote:
> > From: Shannon Zhao 
> >
> > There is a problem for power button that it will not work if an early
> > system_powerdown request happens before guest gpio driver loads.
> >
> > Here we add the emulation of gpio_key and use it for ARM virt power
> > button.
> 
> I tweaked the type names to be 'gpio-key' rather than 'gpio_key',
> and added a comment to briefly describe what the device does:
> 
> + * Emulate a (human) keypress -- when the key is triggered by
> + * setting the incoming gpio line, the outbound irq line is
> + * raised for 100ms before being dropped again.
> 
> Applied to target-arm.next, thanks.

I might be doing something wrong, but aarch64-softmmu @ master only works for me
after reverting 94f02c5ea94 "ARM: Virt: Use gpio_key for power button". Bisect 
log
appended.

This is what I get when booting aarch64 as per [1]:
$ aarch64-softmmu/qemu-system-aarch64 -machine virt -cpu cortex-a57 -machine 
type=virt \
 -nographic -smp 1 -m 2048 -kernel 
img/aarch64/aarch64-linux-3.15rc2-buildroot.img  \
--append "console=ttyAMA0"

qemu-system-aarch64: Unknown device 'gpio-key' for default sysbus
Aborted (core dumped)

Thanks,

Emilio

[1] 
http://www.bennee.com/~alex/blog/2014/05/09/running-linux-in-qemus-aarch64-system-emulation-mode/

git bisect start
# good: [4829e0378dfb91d55af9dfd741bd09e8f2c4f91a] Merge remote-tracking branch 
'remotes/armbru/tags/pull-qapi-2016-03-18' into staging
git bisect good 4829e0378dfb91d55af9dfd741bd09e8f2c4f91a
# bad: [7acbff99c6c285b3070bf0e768d56f511e2bf346] Update version for v2.6.0-rc1 
release
git bisect bad 7acbff99c6c285b3070bf0e768d56f511e2bf346
# good: [9fd3c5d556b21e0020d98d4695c84a655aa056f0] 
tests/test-filter-redirector: Add unit test for filter-redirector
git bisect good 9fd3c5d556b21e0020d98d4695c84a655aa056f0
# bad: [e8710c2293c0f4652090b1434603715f2d9a410f] block: m25p80: Removed unused 
variable
git bisect bad e8710c2293c0f4652090b1434603715f2d9a410f
# good: [5481531154cf08ed53623a0184f7677a9b98d083] raw: Support BDRV_REQ_FUA
git bisect good 5481531154cf08ed53623a0184f7677a9b98d083
# good: [c98d3d79ee387ea6e8fb091299f8562b20022f10] target-mips: use CP0_CHECK 
for gen_m{f|t}hc0
git bisect good c98d3d79ee387ea6e8fb091299f8562b20022f10
# good: [f4e732a0a773c4e44c2c183a5d63cd850ffb57d1] iotests: Test qemu-img 
convert -S 0 behavior
git bisect good f4e732a0a773c4e44c2c183a5d63cd850ffb57d1
# good: [69bc7f5029db5ea55359f7905e9829777ae5a34f] Merge remote-tracking branch 
'remotes/berrange/tags/pull-qcrypto-2016-03-30-1' into staging
git bisect good 69bc7f5029db5ea55359f7905e9829777ae5a34f
# good: [489ef4c810033e63af570c8a430af8b9858bfa5f] Merge remote-tracking branch 
'remotes/lalrae/tags/mips-20160329-2' into staging
git bisect good 489ef4c810033e63af570c8a430af8b9858bfa5f
# bad: [94f02c5ea9430be935088b2574f72f2dcf902997] ARM: Virt: Use gpio_key for 
power button
git bisect bad 94f02c5ea9430be935088b2574f72f2dcf902997
# good: [e5a8152c9bfd3d40c37711ba5b704f277cbc0c54] hw/gpio: Add the emulation 
of gpio_key
git bisect good e5a8152c9bfd3d40c37711ba5b704f277cbc0c54
# first bad commit: [94f02c5ea9430be935088b2574f72f2dcf902997] ARM: Virt: Use 
gpio_key for power button

Re: [Qemu-devel] [PATCH 07/10] tb hash: hash phys_pc, pc, and flags with xxhash

[Emilio G. Cota]

On Wed, Apr 06, 2016 at 13:52:21 +0200, Paolo Bonzini wrote:
> 
> 
> On 06/04/2016 02:52, Emilio G. Cota wrote:
> > +static inline uint32_t tb_hash_func5(uint64_t a0, uint64_t b0, uint32_t e, 
> > int seed)
> 
> I would keep just this version and unconditionally zero-extend to
> 64-bits.  The compiler is able to detect the high 32 bits are zero, drop
> the more expensive multiplications and constant fold everything.
> 
> For example if you write
> 
> unsigned tb_hash_func(uint32_t phys_pc, uint32_t pc, int flags)
> {
> return tb_hash_func5(phys_pc, pc, flags, 1);
> }
> 
> and check the optimized code with -fdump-tree-optimized you'll see that
> the rotated v1, the rotated v3 and the 20 merge into a single constant
> 1733907856.

I like this idea, because the ugliness of the sizeof checks is significant.
However, the quality of the resulting hash is not as good when always using 
func5.
For instance, when we'd otherwise use func3, two fifths of every input contain
exactly the same bits: all 0's. This inevitably leads to more collisions.

Performance (for the debian arm bootup test) gets up to 15% worse.

Emilio

Re: [Qemu-devel] [PATCH 07/10] tb hash: hash phys_pc, pc, and flags with xxhash

[Richard Henderson]

On 04/06/2016 10:32 AM, Emilio G. Cota wrote:
> On Wed, Apr 06, 2016 at 08:06:57 +0200, Laurent Desnogues wrote:
>> On Tue, Apr 5, 2016 at 7:19 PM, Richard Henderson  wrote:
>>> On 04/05/2016 09:33 AM, Laurent Desnogues wrote:
 The 'flags' field is 64-bit.  You're thinking of cflags, I guess.
>>>
>>> Well that's silly.  Since it's filled in via
>>>
>>> static inline void cpu_get_tb_cpu_state(CPUMIPSState *env, target_ulong *pc,
>>> target_ulong *cs_base, int *flags)
>>>
>>> and passed back in to generate code with
>>>
>>> TranslationBlock *tb_gen_code(CPUState *cpu,
>>>   target_ulong pc, target_ulong cs_base, int 
>>> flags,
>>>   int cflags);
>>>
>>> So while TranslationBlock stores "uint64_t", the producer and consumer see 
>>> "int".
>>
>> I agree.  I guess TranslationBlock should be fixed to use uint32_t
>> (note several functions have to be changed from using int to uint32_t
>> or aarch64-softmmu will fail).
> 
> Can you please elaborate on this?

The arm port is using some high bits, including

#define ARM_TBFLAG_AARCH64_STATE_SHIFT 31
#define ARM_TBFLAG_AARCH64_STATE_MASK  (1U << ARM_TBFLAG_AARCH64_STATE_SHIFT)

So, I would certainly be ok switching everything to use uint32_t over int.


r~

Re: [Qemu-devel] [PATCH 07/10] tb hash: hash phys_pc, pc, and flags with xxhash

[Emilio G. Cota]

On Wed, Apr 06, 2016 at 08:06:57 +0200, Laurent Desnogues wrote:
> On Tue, Apr 5, 2016 at 7:19 PM, Richard Henderson  wrote:
> > On 04/05/2016 09:33 AM, Laurent Desnogues wrote:
> >> The 'flags' field is 64-bit.  You're thinking of cflags, I guess.
> >
> > Well that's silly.  Since it's filled in via
> >
> > static inline void cpu_get_tb_cpu_state(CPUMIPSState *env, target_ulong *pc,
> > target_ulong *cs_base, int *flags)
> >
> > and passed back in to generate code with
> >
> > TranslationBlock *tb_gen_code(CPUState *cpu,
> >   target_ulong pc, target_ulong cs_base, int 
> > flags,
> >   int cflags);
> >
> > So while TranslationBlock stores "uint64_t", the producer and consumer see 
> > "int".
> 
> I agree.  I guess TranslationBlock should be fixed to use uint32_t
> (note several functions have to be changed from using int to uint32_t
> or aarch64-softmmu will fail).

Can you please elaborate on this?

FWIW aarch64-softmmu boots OK for me with the patch below. I'm booting
it as per the instructions in
  
http://www.bennee.com/~alex/blog/2014/05/09/running-linux-in-qemus-aarch64-system-emulation-mode/

Thanks,

Emilio

commit e70474788fa37a85df21e1c63101a879103758f5
Author: Emilio G. Cota 
Date:   Tue Apr 5 13:55:16 2016 -0400

tb: consistently use 'int' type for tb->flags

Reported-by: Richard Henderson 
Signed-off-by: Emilio G. Cota 

diff --git a/cpu-exec.c b/cpu-exec.c
index bbfcbfb..5abbf57 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -220,7 +220,7 @@ static void cpu_exec_nocache(CPUState *cpu, int max_cycles,
 static TranslationBlock *tb_find_physical(CPUState *cpu,
   target_ulong pc,
   target_ulong cs_base,
-  uint64_t flags)
+  int flags)
 {
 CPUArchState *env = (CPUArchState *)cpu->env_ptr;
 TranslationBlock *tb, **ptb1;
@@ -271,7 +271,7 @@ static TranslationBlock *tb_find_physical(CPUState *cpu,
 static TranslationBlock *tb_find_slow(CPUState *cpu,
   target_ulong pc,
   target_ulong cs_base,
-  uint64_t flags)
+  int flags)
 {
 TranslationBlock *tb;
 
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 7362095..277e6f1 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -235,7 +235,7 @@ static inline void tlb_flush_by_mmuidx(CPUState *cpu, ...)
 struct TranslationBlock {
 target_ulong pc;   /* simulated PC corresponding to this block (EIP + CS 
base) */
 target_ulong cs_base; /* CS base for this block */
-uint64_t flags; /* flags defining in which context the code was generated 
*/
+int flags; /* flags defining in which context the code was generated */
 uint16_t size;  /* size of target code for this block (1 <=
size <= TARGET_PAGE_SIZE) */
 uint16_t icount;
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 1a1214d..4024ad4 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -8178,7 +8178,7 @@ void gen_intermediate_code(CPUX86State *env, 
TranslationBlock *tb)
 CPUState *cs = CPU(cpu);
 DisasContext dc1, *dc = 
 target_ulong pc_ptr;
-uint64_t flags;
+int flags;
 target_ulong pc_start;
 target_ulong cs_base;
 int num_insns;
diff --git a/translate-all.c b/translate-all.c
index 8329ea6..27b4d57 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1593,7 +1593,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
 TranslationBlock *tb;
 uint32_t n, cflags;
 target_ulong pc, cs_base;
-uint64_t flags;
+int flags;
 
 tb = tb_find_pc(retaddr);
 if (!tb) {

Re: [Qemu-devel] [PATCH V2] tap: vhost busy polling support

[Greg Kurz]

On Wed, 6 Apr 2016 09:05:21 -0600
Eric Blake  wrote:

> On 04/06/2016 03:15 AM, Jason Wang wrote:
> > This patch add the capability of basic vhost net busy polling which is
> > supported by recent kernel. User could configure the maximum number of
> > us that could be spent on busy polling through a new property of tap
> > "vhost-poll-us".
> > 
> > Signed-off-by: Jason Wang 
> > ---  
> 
> > +++ b/qapi-schema.json
> > @@ -2531,6 +2531,9 @@
> >  #
> >  # @queues: #optional number of queues to be created for multiqueue capable 
> > tap
> >  #
> > +# @vhost-poll-us: #optional maximum number of microseconds that could
> > +# be spent on busy polling for vhost net  
> 
> Missing a '(since 2.X)' designation (not sure if X should be 6 or 7)
> 

According to another mail from Jason, this isn't 2.6 material.

--
Greg

Re: [Qemu-devel] [PATCH] xen: write information about supported backends

[Stefano Stabellini]

On Mon, 4 Apr 2016, Juergen Gross wrote:
> On 01/04/16 16:56, Stefano Stabellini wrote:
> > On Wed, 30 Mar 2016, Juergen Gross wrote:
> >> Add a Xenstore directory for each supported pv backend. This will allow
> >> Xen tools to decide which backend type to use in case there are
> >> multiple possibilities.
> >>
> >> The information is added under
> >> /local/domain//device-model//backends
> >> before the "running" state is written to Xenstore. Using a directory
> >> for each directory enables us to add parameters for specific backends
> >> in the future.
> >>
> >> In order to reuse the Xenstore directory creation already present in
> >> hw/xen/xen_devconfig.c move the related functions to
> >> hw/xen/xen_backend.c where they fit better.
> > 
> > An interface change like this one, needs a new file under docs (in the
> > Xen repository) to document it.  Please add a reference to it, in the
> > commit message here.
> 
> Is the Xenstore path documentation enough? Or do you want something
> like docs/misc/qemu-deprivilege.txt to be added?

Something like qemu-deprivilege.txt would be acceptable.


> > In addition please make sure that this can work with QEMU running as
> > non-root. What are the permissions of the new xenstore directory?
> 
> Should work, as I'm using the same functions as any backend in qemu is
> using. The new directory is read-only for the qemu target domain (same
> as the backend directories).

All right, just make sure to have xen-qemuuser-shared or
xen-qemuuser-domid on your system when you test :-)

[Qemu-devel] [PATCH v2] nbd: Fix NBD unsupported options

[Eric Blake]

From: Alex Bligh 

nbd-client.c currently fails to handle unsupported options properly.
If during option haggling the server finds an option that is
unsupported, it returns an NBD_REP_ERR_UNSUP reply.

According to nbd's proto.md, the format for such a reply
should be:

  S: 64 bits, 0x3e889045565a9 (magic number for replies)
  S: 32 bits, the option as sent by the client to which this is a reply
  S: 32 bits, reply type (e.g., NBD_REP_ACK for successful completion,
 or NBD_REP_ERR_UNSUP to mark use of an option not known by this server
  S: 32 bits, length of the reply. This may be zero for some replies,
 in which case the next field is not sent
  S: any data as required by the reply (e.g., an export name in the case
 of NBD_REP_SERVER, or optional UTF-8 message for NBD_REP_ERR_*)

However, in nbd-client.c, the reply type was being read, and if it
contained an error, it was bailing out and issuing the next option
request without first reading the length. This meant that the
next option / handshake read had an extra 4 or more bytes of data in it.
In practice, this makes Qemu incompatible with servers that do not
support NBD_OPT_LIST.

To verify this isn't an error in the specification or my reading of
it, replies are sent by the reference implementation here:
 https://github.com/yoe/nbd/blob/66dfb35/nbd-server.c#L1232
and as is evident it always sends a 'datasize' (aka length) 32 bit
word. Unsupported elements are replied to here:
 https://github.com/yoe/nbd/blob/66dfb35/nbd-server.c#L1371

Signed-off-by: Alex Bligh 
Message-Id: <1459882500-24316-1-git-send-email-a...@alex.org.uk>
[rework to ALWAYS consume an optional UTF-8 message from the server]
Signed-off-by: Eric Blake 
---

v2: patch grew in size, but now handles more servers

 nbd/client.c | 53 +++--
 1 file changed, 43 insertions(+), 10 deletions(-)

diff --git a/nbd/client.c b/nbd/client.c
index d9b7a9b..67116b9 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -73,16 +73,44 @@ static QTAILQ_HEAD(, NBDExport) exports = 
QTAILQ_HEAD_INITIALIZER(exports);
 */


-static int nbd_handle_reply_err(uint32_t opt, uint32_t type, Error **errp)
+/* If type represents success, return 1 without further action.
+ * If type represents an error reply, consume the rest of the packet on ioc.
+ * Then return 0 for unsupported (so the client can fall back to
+ * other approaches), or -1 with errp set for other errors.
+ */
+static int nbd_handle_reply_err(QIOChannel *ioc, uint32_t opt, uint32_t type,
+Error **errp)
 {
+uint32_t len;
+char *msg = NULL;
+int result = -1;
+
 if (!(type & (1 << 31))) {
-return 0;
+return 1;
+}
+
+if (read_sync(ioc, , sizeof(len)) != sizeof(len)) {
+error_setg(errp, "failed to read option length");
+return -1;
+}
+len = be32_to_cpu(len);
+if (len) {
+if (len > NBD_MAX_BUFFER_SIZE) {
+error_setg(errp, "server's error message is too long");
+goto cleanup;
+}
+msg = g_malloc(len + 1);
+if (read_sync(ioc, msg, len) != len) {
+error_setg(errp, "failed to read option error message");
+goto cleanup;
+}
+msg[len] = '\0';
 }

 switch (type) {
 case NBD_REP_ERR_UNSUP:
-error_setg(errp, "Unsupported option type %x", opt);
-break;
+result = 0;
+goto cleanup;

 case NBD_REP_ERR_POLICY:
 error_setg(errp, "Denied by server for option %x", opt);
@@ -101,7 +129,13 @@ static int nbd_handle_reply_err(uint32_t opt, uint32_t 
type, Error **errp)
 break;
 }

-return -1;
+if (msg) {
+error_append_hint(errp, "%s\n", msg);
+}
+
+ cleanup:
+g_free(msg);
+return result;
 }

 static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
@@ -111,6 +145,7 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, 
Error **errp)
 uint32_t type;
 uint32_t len;
 uint32_t namelen;
+int error;

 *name = NULL;
 if (read_sync(ioc, , sizeof(magic)) != sizeof(magic)) {
@@ -138,11 +173,9 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, 
Error **errp)
 return -1;
 }
 type = be32_to_cpu(type);
-if (type == NBD_REP_ERR_UNSUP) {
-return 0;
-}
-if (nbd_handle_reply_err(opt, type, errp) < 0) {
-return -1;
+error = nbd_handle_reply_err(ioc, opt, type, errp);
+if (error <= 0) {
+return error;
 }

 if (read_sync(ioc, , sizeof(len)) != sizeof(len)) {
-- 
2.5.5

[Qemu-devel] [Bug 1563152] Re: general protection fault running VirtualBox in KVM guest

[Serge Hallyn]

** Changed in: qemu (Ubuntu)
   Importance: Undecided => Low

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

Title:
  general protection fault running VirtualBox in KVM guest

Status in QEMU:
  New
Status in qemu package in Ubuntu:
  New

Bug description:
  I'm trying to run nested VMs using qemu-kvm on the physical host and 
VirtualBox on the guest host:
    * physical host: Ubuntu 14.04 running Linux 4.2.0, qemu-kvm 2.0.0
    * guest host: Ubuntu 16.04 beta 2 running Linux 4.4.0, VirtualBox 5.0.16

  When I try to start up a VirtualBox VM in the guest host, I get a
  general protection fault (see below for dmesg output).  According to
  https://www.virtualbox.org/ticket/14965 this is caused by a bug in
  QEMU/KVM:

  The problem in more detail:  As written above, VirtualBox tries to
  read the MSR 0x9B (IA32_SMM_MONITOR_CTL).  This is an
  architectural MSR which is present if CPUID.01 / ECX bit 5 or bit
  6 are set (VMX or SMX).  As KVM has nested virtualization enabled
  and therefore pretends to support VT-x, this MSR must be
  accessible and reading from this MSR must not raise a
  #GP.  KVM/QEmu does not behave like real hardware in this case.

  dmesg output:

  SUPR0GipMap: fGetGipCpu=0x3
  general protection fault:  [#1] SMP
  Modules linked in: pci_stub vboxpci(OE) vboxnetadp(OE) vboxnetflt(OE) 
vboxdrv(OE) xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 
iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack 
xt_tcpudp bridge stp llc iptable_filter ip_tables x_tables ppdev kvm_intel kvm 
irqbypass snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core 
snd_hwdep snd_pcm snd_timer i2c_piix4 snd input_leds soundcore joydev 
8250_fintek mac_hid serio_raw pvpanic parport_pc parport ib_iser rdma_cm iw_cm 
ib_cm ib_sa ib_mad ib_core ib_addr iscsi_tcp libiscsi_tcp libiscsi 
scsi_transport_iscsi autofs4 btrfs raid10 raid456 async_raid6_recov 
async_memcpy async_pq async_xor async_tx xor raid6_pq libcrc32c raid1 raid0 
multipath linear crct10dif_pclmul crc32_pclmul qxl ttm drm_kms_helper 
syscopyarea sysfillrect aesni_intel sysimgblt fb_sys_fops aes_x86_64 lrw 
gf128mul glue_helper ablk_helper cryptd psmouse floppy drm pata_acpi
  CPU: 0 PID: 31507 Comm: EMT Tainted: G   OE   4.4.0-15-generic 
#31-Ubuntu
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
  task: 880034c0a580 ti: 880002e0 task.ti: 880002e0
  RIP: 0010:[]  [] 0xc067e506
  RSP: 0018:880002e03d70  EFLAGS: 00010206
  RAX: 06f0 RBX: ffdb RCX: 009b
  RDX:  RSI: 880002e03d00 RDI: 880002e03cc8
  RBP: 880002e03d90 R08: 0004 R09: 06f0
  R10: 49656e69 R11: 0f8bfbff R12: 0020
  R13:  R14: c957407c R15: c0645260
  FS:  7f89b8f6b700() GS:88007fc0() knlGS:
  CS:  0010 DS:  ES:  CR0: 80050033
  CR2: 7f89b8d1 CR3: 35ae1000 CR4: 06f0
  Stack:
      
   880002e03db0 c0693e93 c9574010 880035aae550
   880002e03e30 c060a3e7 880002e03e10 0282
  Call Trace:
   [] ? supdrvIOCtl+0x2de7/0x3250 [vboxdrv]
   [] ? VBoxDrvLinuxIOCtl_5_0_16+0x150/0x250 [vboxdrv]
   [] ? do_vfs_ioctl+0x29f/0x490
   [] ? __do_page_fault+0x1b4/0x400
   [] ? SyS_ioctl+0x79/0x90
   [] ? entry_SYSCALL_64_fastpath+0x16/0x71
  Code: 88 e4 fc ff ff b9 3a 00 00 00 0f 32 48 c1 e2 20 89 c0 48 09 d0 48 89 05 
f9 db 0e 00 0f 20 e0 b9 9b 00 00 00 48 89 05 d2 db 0e 00 <0f> 32 48 c1 e2 20 89 
c0 b9 80 00 00 c0 48 09 d0 48 89 05 cb db
  RIP  [] 0xc067e506
   RSP 
  ---[ end trace b3284b6520f49e0d ]---

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

Re: [Qemu-devel] [PATCH v3 0/7] virtio: aio handler API

[Cornelia Huck]

On Wed,  6 Apr 2016 12:16:21 +0200
Paolo Bonzini  wrote:

> This version removes patches 1 and 9, fixes some commit messages, and
> fixes some small in the formatting issues.
> 
> Michael S. Tsirkin (2):
>   virtio: add aio handler
>   virtio-blk: use aio handler for data plane
> 
> Paolo Bonzini (5):
>   virtio: make virtio_queue_notify_vq static
>   virtio-blk: fix disabled mode
>   virtio-scsi: fix disabled mode
>   virtio-scsi: use aio handler for data plane
>   virtio: merge virtio_queue_aio_set_host_notifier_handler with
> virtio_queue_set_aio
> 
>  hw/block/dataplane/virtio-blk.c | 23 ++
>  hw/block/virtio-blk.c   | 29 ++---
>  hw/scsi/virtio-scsi-dataplane.c | 47 +++-
>  hw/scsi/virtio-scsi.c   | 69 
> +++--
>  hw/virtio/virtio.c  | 39 +--
>  include/hw/virtio/virtio-blk.h  |  3 ++
>  include/hw/virtio/virtio-scsi.h |  7 ++---
>  include/hw/virtio/virtio.h  |  4 +--
>  8 files changed, 158 insertions(+), 63 deletions(-)

Seems to work fine so far. We'll try to run some more extensive tests
overnight.

[Qemu-devel] [PATCH v2] qcow2: Prevent backing file names longer than 1023

[Max Reitz]

We reject backing file names with a length of more than 1023 characters
when opening a qcow2 file, so we should not produce such files
ourselves.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Max Reitz 
---
v2:
- Move the check to qcow2_change_backing_file() so the BDS state will
  not be modified if that condition is not met [Kevin]
---
 block/qcow2.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 056525c..470734b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1986,6 +1986,10 @@ static int qcow2_change_backing_file(BlockDriverState 
*bs,
 {
 BDRVQcow2State *s = bs->opaque;
 
+if (backing_file && strlen(backing_file) > 1023) {
+return -EINVAL;
+}
+
 pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_file ?: "");
 pstrcpy(bs->backing_format, sizeof(bs->backing_format), backing_fmt ?: "");
 
-- 
2.8.0

Re: [Qemu-devel] [Qemu-block] [PATCH 6/7] qemu-iotests: 141: reduce likelihood of race condition on systems with fast IO

[Kevin Wolf]

Am 06.04.2016 um 18:15 hat Max Reitz geschrieben:
> On 05.04.2016 11:21, Sascha Silbe wrote:
> > On systems with fast IO, qemu-io may write more than 1 MiB before
> > receiving the block-job-cancel command, causing the test case to fail.
> > 
> > 141 is inherently racy, but we can at least reduce the likelihood of the
> > job completing before the cancel command arrives by bumping the size of
> > the data getting written; we'll try 32 MiB for a start.
> 
> Hm, interesting. I tried to prevent this by setting the block jobs'
> speed to 1, which should make it stop after the block job has processed
> the first block of data.
> 
> I won't oppose this patch, because if it fixes things for you, that's
> good. But I don't think it should be necessary.

We don't generally change test cases when they fail. Making a test case
pass doesn't "fix things" per se. It only helps when the failure is a
false positive.

In this case, it looks like there might be a problem with block job
throttling, so maybe we should look into that before changing the test?

Kevin

> > Once we actually move enough data around for the block job not to
> > complete prematurely, the test will still fail because the offset value
> > in the BLOCK_JOB_CANCELLED event will vary. Since this is more or less
> > inherent to the nature of this event, we just replace it with a fixed
> > value globally (in _filter_qmp), the same way we handle timestamps.
> > 
> > Signed-off-by: Sascha Silbe 
> > Reviewed-by: Bo Tu 
> > ---
> >  tests/qemu-iotests/141   | 11 ++-
> >  tests/qemu-iotests/141.out   | 24 
> >  tests/qemu-iotests/common.filter |  1 +
> >  3 files changed, 19 insertions(+), 17 deletions(-)
> 





pgppAdna15Xf_.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH] Fix NBD unsupported options

[Alex Bligh]

On 6 Apr 2016, at 17:14, Eric Blake  wrote:

> I just re-reviewed this patch. While what you have is a strict
> improvement, it is still buggy for a server that sends a UTF-8 message
> explaining the error:

Indeed. That should be fixed too. I missed that one.

--
Alex Bligh






signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [Qemu-devel] [V8 2/4] hw/i386: ACPI table for AMD IOMMU

[Igor Mammedov]

On Fri,  1 Apr 2016 21:39:52 +0300
David Kiarie  wrote:

> Add IVRS table for AMD IOMMU. Generate IVRS or DMAR
> depending on emulated IOMMU
> 
> Signed-off-by: David Kiarie 
> ---
>  hw/i386/acpi-build.c  | 98 
> ++-
>  include/hw/acpi/acpi-defs.h   | 55 
>  include/hw/i386/intel_iommu.h |  1 +
>  3 files changed, 143 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 35180ef..a54a22c 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -52,6 +52,7 @@
>  #include "hw/pci/pci_bus.h"
>  #include "hw/pci-host/q35.h"
>  #include "hw/i386/intel_iommu.h"
> +#include "hw/i386/amd_iommu.h"
>  #include "hw/timer/hpet.h"
>  
>  #include "hw/acpi/aml-build.h"
> @@ -118,6 +119,12 @@ typedef struct AcpiBuildPciBusHotplugState {
>  bool pcihp_bridge_en;
>  } AcpiBuildPciBusHotplugState;
>  
> +typedef enum iommu_type {
> +TYPE_AMD,
> +TYPE_INTEL,
> +TYPE_NONE
> +} iommu_type;
> +
>  static void acpi_get_pm_info(AcpiPmInfo *pm)
>  {
>  Object *piix = piix4_pm_find();
> @@ -2588,6 +2595,78 @@ build_dmar_q35(GArray *table_data, GArray *linker)
>   "DMAR", table_data->len - dmar_start, 1, NULL, NULL);
>  }
>  
> +static void
> +build_amd_iommu(GArray *table_data, GArray *linker)
> +{
> +int iommu_start = table_data->len;
> +bool iommu_ambig;
> +
> +AcpiAMDIOMMUIVRS *ivrs;
> +AcpiAMDIOMMUHardwareUnit *iommu;
> +
> +/* IVRS definition */
> +ivrs = acpi_data_push(table_data, sizeof(*ivrs));
> +ivrs->revision = cpu_to_le16(ACPI_IOMMU_IVRS_TYPE);
s/ACPI_IOMMU_IVRS_TYPE/1/ it's the only usage site and
well it's not type but revision number, 
Pls try to keep names close to spec, so that is would be easy
to review code.


> +ivrs->length = cpu_to_le16((sizeof(*ivrs) + sizeof(*iommu)));
you are expecting length be ^^^ from starters so push above should be for
this size.

> +ivrs->v_common_info = cpu_to_le64(AMD_IOMMU_HOST_ADDRESS_WIDTH << 8);
shouldn't above block go below (AMDIOMMUState *)object_resolve_path_type(...,
i.e. is there point in pushing empty table?

s/v_common_info/iv_info/ as in spec
also it's 32 bit in size so using here cpu_to_le64() is wrong.


> +
> +AMDIOMMUState *s = (AMDIOMMUState *)object_resolve_path_type("",
> +TYPE_AMD_IOMMU_DEVICE, _ambig);
Maybe it should assert on iommu_ambig == true
and get rid of if(!iommu_ambig) condition

> +
> +/* IVDB definition - type 10h */
s/IVDB definition - type 10h/Create IVHD entry/

> +iommu = acpi_data_push(table_data, sizeof(*iommu));
> +if (!iommu_ambig) {

> +iommu->type = cpu_to_le16(0x10);
> +/* IVHD flags */
> +iommu->flags = cpu_to_le16(iommu->flags);
> +iommu->flags = cpu_to_le16(IVHD_HT_TUNEN | IVHD_PPRSUP | 
> IVHD_IOTLBSUP
> +   | IVHD_PREFSUP);
> +iommu->length = cpu_to_le16(sizeof(*iommu));
> +iommu->device_id = cpu_to_le16(PCI_DEVICE_ID_RD890_IOMMU);
> +iommu->capability_offset = cpu_to_le16(s->capab_offset);
> +iommu->mmio_base = cpu_to_le64(s->mmio.addr);
> +iommu->pci_segment = 0;
> +iommu->interrupt_info = 0;
> +/* EFR features */
> +iommu->efr_register = cpu_to_le64(IVHD_EFR_GTSUP | IVHD_EFR_HATS
> +  | IVHD_EFR_GATS);
> +iommu->efr_register = cpu_to_le64(iommu->efr_register);
double assignment looks rather weird here, pls explain why are you doing it.

Actually I'd prefer if you'd use table declaration style for
creating IVRS table instead of AcpiAMDIOMMUIVRS/AcpiAMDIOMMUHardwareUnit
structures, that would simplify review greatly as it would be mostly
comparison with spec's tables.
You won't have to care about endianess, it also will help to avoid many
mistakes with usage of wrong cpu_to_le* functions as it's the most common
mistake in this patch.
something like this:

/* IVRS fields */
uint8_t *ivrs_tbl = acpi_data_push(table_data, sizeof(acpi_table_header)); /* 
reserve space for header */
build_append_int_noprefix(table_data, AMD_IOMMU_HOST_ADDRESS_WIDTH << 8, 4); /* 
IVinfo */
build_append_int_noprefix(table_data, 0, 8); /* Reserved */

/* build IVHD entry */
build_append_int_noprefix(table_data, 0x10 /* IVHD entry */, 1); /* Type */
build_append_int_noprefix(table_data, iommu->flags, 1); /* Flags */
build_append_int_noprefix(table_data, ivhd_len, 2); /* Length */
...
build_4byte_ivhd_device_entry(table_data,...); /* if it's only one entry no 
need in separate function, just inline it */
...
build_header(linker, table_data, (void *), ivrs_tbl,
 "IVRS", ivrs_tbl - table_data->data, 1 /* IVRS Revision */, NULL, 
NULL);

> +/* device entries */
> +memset(iommu->dev_entries, 0, 20);
> +/* Add device flags here
> + *  This is are 4-byte device entries currently 

Re: [Qemu-devel] [Qemu-block] [PATCH 7/7] qemu-iotests: iotests.py: get rid of __all__

[Max Reitz]

On 05.04.2016 11:21, Sascha Silbe wrote:
> The __all__ list contained a typo for as long as the iotests module
> existed. That typo prevented "from iotests import *" (which is the
> only case where iotests.__all__ is used at all) from ever working.

Hahaha :D

Nice one.

> The names used by iotests are highly prone to name collisions, so
> importing them all unconditionally is a bad idea anyway. Since __all__
> is not adding any value, let's just get rid of it.
> 
> Fixes: f345cfd0 ("qemu-iotests: add iotests Python module")
> Signed-off-by: Sascha Silbe 
> Reviewed-by: Bo Tu 
> ---
>  tests/qemu-iotests/iotests.py | 4 
>  1 file changed, 4 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] qcow2: Prevent backing file names longer than 1023

[Kevin Wolf]

Am 06.04.2016 um 17:14 hat Max Reitz geschrieben:
> We reject backing file names with a length of more than 1023 characters
> when opening a qcow2 file, so we should not produce such files
> ourselves.
> 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Max Reitz 
> ---
>  block/qcow2.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 056525c..011a0ae 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1957,6 +1957,11 @@ int qcow2_update_header(BlockDriverState *bs)
>  if (s->image_backing_file) {
>  size_t backing_file_len = strlen(s->image_backing_file);
>  
> +if (backing_file_len > 1023) {
> +ret = -EINVAL;
> +goto fail;
> +}
> +
>  if (buflen < backing_file_len) {
>  ret = -ENOSPC;
>  goto fail;

We should probably already check this in qcow2_change_backing_file(), so
that s->image_backing_file can never contain anything longer than that.
If you like, you can keep an assertion here.

The advantage is that in qcow2_change_backing_file() we can fail the
operation before all of the variables are updated and therefore become
inconsistent with the on-disk state of the image.

Kevin

Re: [Qemu-devel] [Qemu-block] [PATCH 6/7] qemu-iotests: 141: reduce likelihood of race condition on systems with fast IO

[Max Reitz]

On 05.04.2016 11:21, Sascha Silbe wrote:
> On systems with fast IO, qemu-io may write more than 1 MiB before
> receiving the block-job-cancel command, causing the test case to fail.
> 
> 141 is inherently racy, but we can at least reduce the likelihood of the
> job completing before the cancel command arrives by bumping the size of
> the data getting written; we'll try 32 MiB for a start.

Hm, interesting. I tried to prevent this by setting the block jobs'
speed to 1, which should make it stop after the block job has processed
the first block of data.

I won't oppose this patch, because if it fixes things for you, that's
good. But I don't think it should be necessary.

Max

> 
> Once we actually move enough data around for the block job not to
> complete prematurely, the test will still fail because the offset value
> in the BLOCK_JOB_CANCELLED event will vary. Since this is more or less
> inherent to the nature of this event, we just replace it with a fixed
> value globally (in _filter_qmp), the same way we handle timestamps.
> 
> Signed-off-by: Sascha Silbe 
> Reviewed-by: Bo Tu 
> ---
>  tests/qemu-iotests/141   | 11 ++-
>  tests/qemu-iotests/141.out   | 24 
>  tests/qemu-iotests/common.filter |  1 +
>  3 files changed, 19 insertions(+), 17 deletions(-)



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] Fix NBD unsupported options

[Eric Blake]

On 04/05/2016 12:55 PM, Alex Bligh wrote:
> nbd-client.c currently fails to handle unsupported options properly.
> If during option haggling the server finds an option that is
> unsupported, it returns an NBD_REP_ERR_UNSUP reply.
> 
> According to nbd's proto.md, the format for such a reply
> should be:
> 
>   S: 64 bits, 0x3e889045565a9 (magic number for replies)
>   S: 32 bits, the option as sent by the client to which this is a reply
>   S: 32 bits, reply type (e.g., NBD_REP_ACK for successful completion,
>  or NBD_REP_ERR_UNSUP to mark use of an option not known by this server
>   S: 32 bits, length of the reply. This may be zero for some replies,
>  in which case the next field is not sent
>   S: any data as required by the reply (e.g., an export name in the case
>  of NBD_REP_SERVER)

I just re-reviewed this patch. While what you have is a strict
improvement, it is still buggy for a server that sends a UTF-8 message
explaining the error:

> @@ -151,6 +145,13 @@ static int nbd_receive_list(QIOChannel *ioc, char 
> **name, Error **errp)
>  }
>  len = be32_to_cpu(len);
>  
> +if (type == NBD_REP_ERR_UNSUP) {
> +return 0;
> +}

That is, if len > 0, we are still leaving the server's UTF-8 message on
the wire, but returning early, which will still corrupt the next attempt
to send another option.

Paolo, can you revert this off your queue, and wait for me to send a v2
that fixes the situation in a cleaner manner?

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-block] [PATCH 5/7] qemu-iotests: 068: don't require KVM

[Max Reitz]

On 05.04.2016 11:21, Sascha Silbe wrote:
> None of the other test cases explicitly enable KVM and there's no
> obvious reason for 068 to require it. Drop this so all test cases can be
> executed in environments where KVM is not available (e.g. because the
> user doesn't have sufficient permissions to access /dev/kvm).
> 
> Signed-off-by: Sascha Silbe 
> Reviewed-by: Bo Tu 
> ---
>  tests/qemu-iotests/068 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [FreeBSD] Host build i386 failing to build aarch64 targets

[Sean Bruno]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512



On 03/20/16 12:20, Sean Bruno wrote:
> aarch64 targets are now failing to build on i386 hosts due to
> missing __atomic_load_8() calls since this commit:
> 
> https://github.com/qemu/qemu/commit/a0aa44b488b3601415d55041e4619aef5f
3a4ba8#diff-c143d686899ae51d7b927d9c682e12fd
>
>  I'm unsure if Linux is disabling aarch64 targets for i386 hosts or
> if this commit works "just fine" on Linux hosts right now, as it
> doesn't work with clang or gcc.
> 
> More or less, the code in question ends up looking like this bit
> of test code:
> 
> #include  #include  #include
> 
> 
> #define atomic_read(ptr)  \ ({
> \ typeof(*ptr) _val;\ 
> __atomic_load(ptr, &_val, __ATOMIC_RELAXED); \ _val;
> \ })
> 
> int main () { int foo; int64_t foo64;
> 
> atomic_read(); atomic_read();
> 
> return(0); }
> 
> 
> This test code will manifest the same issue as the aarch64 target 
> building on FreeBSD i386 with the error:
> 
> undefined reference to `__atomic_load_8'
> 
> 


This seems to be fixed with the latest commits.  Thanks!

sean
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iQF8BAEBCgBmBQJXBTUqXxSAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w
ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRCQUFENDYzMkU3MTIxREU4RDIwOTk3REQx
MjAxRUZDQTFFNzI3RTY0AAoJEBIB78oecn5kAoYH/3Ai7IdpyxyIiRtgYDWJQcg8
GLmbu1NL5Xrh0af5DU933kCqkDlKK8qKYs89DzHMfay8TIyZFkqKF5pYy66JJAQ2
qAB9eGTL567q3QPk9iYkRLju4Y4exmBL1ZSW1fUpPKjjjBlLR7VrHjEA/Ze0zYsM
+MvRJcHSf8tNawr8WfOzVgFPf8pc2K0Ix8/VZCFEdf4FcATHj2nYXTmzQmTQuWSo
tqDWe02TIov0BSBaA4uG6n02F4KWglGBE+bdsuTiwxxAjkcmHLgg28h7Wupkmatj
5zarlSLIhvv7j3KAS/r8aKtQ04ydXybTo0HnPLJ9JV/xz3bAbvvKDYLMZijpm0M=
=OSgq
-END PGP SIGNATURE-

Re: [Qemu-devel] [PATCH for-2.6 2/2] block/gluster: prevent data loss after i/o error

[Jeff Cody]

On Wed, Apr 06, 2016 at 04:47:32PM +0200, Niels de Vos wrote:
> On Wed, Apr 06, 2016 at 08:44:18AM -0400, Jeff Cody wrote:
> > On Wed, Apr 06, 2016 at 01:02:16PM +0200, Kevin Wolf wrote:
> > > [ Adding some CCs ]
> > > 
> > > Am 06.04.2016 um 05:29 hat Jeff Cody geschrieben:
> > > > Upon receiving an I/O error after an fsync, by default gluster will
> > > > dump its cache.  However, QEMU will retry the fsync, which is especially
> > > > useful when encountering errors such as ENOSPC when using the 
> > > > werror=stop
> > > > option.  When using caching with gluster, however, the last written data
> > > > will be lost upon encountering ENOSPC.  Using the cache xlator option of
> 
> Use "write-behind xlator" instead of "cache xlator". There are different
> caches in Gluster.
>

Thanks, I'll update the commit message.

> > > > 'resync-failed-syncs-after-fsync' should cause gluster to retain the
> > > > cached data after a failed fsync, so that ENOSPC and other transient
> > > > errors are recoverable.
> > > > 
> > > > Signed-off-by: Jeff Cody 
> > > > ---
> > > >  block/gluster.c | 27 +++
> > > >  configure   |  8 
> > > >  2 files changed, 35 insertions(+)
> > > > 
> > > > diff --git a/block/gluster.c b/block/gluster.c
> > > > index 30a827e..b1cf71b 100644
> > > > --- a/block/gluster.c
> > > > +++ b/block/gluster.c
> > > > @@ -330,6 +330,23 @@ static int qemu_gluster_open(BlockDriverState *bs, 
> > > >  QDict *options,
> > > >  goto out;
> > > >  }
> > > >  
> > > > +#ifdef CONFIG_GLUSTERFS_XLATOR_OPT
> > > > +/* Without this, if fsync fails for a recoverable reason (for 
> > > > instance,
> > > > + * ENOSPC), gluster will dump its cache, preventing retries.  This 
> > > > means
> > > > + * almost certain data loss.  Not all gluster versions support the
> > > > + * 'resync-failed-syncs-after-fsync' key value, but there is no 
> > > > way to
> > > > + * discover during runtime if it is supported (this api returns 
> > > > success for
> > > > + * unknown key/value pairs) */
> > > 
> > > Honestly, this sucks. There is apparently no way to operate gluster so
> > > we can safely recover after a failed fsync. "We hope everything is fine,
> > > but depending on your gluster version, we may now corrupt your image"
> > > isn't very good.
> > > 
> > > We need to consider very carefully if this is good enough to go on after
> > > an error. I'm currently leaning towards "no". That is, we should only
> > > enable this after Gluster provides us a way to make sure that the option
> > > is really set.
> 
> Unfortunately it is also possible to disable the write-behind xlator as
> well. This would cause setting the option to fail too :-/ At the moment
> there is no real useful way to detect if write-behind is disabled (it is
> enabled by default).
> 
> > > > +ret = glfs_set_xlator_option (s->glfs, "*-write-behind",
> > > > +   
> > > > "resync-failed-syncs-after-fsync",
> > > > +   "on");
> > > > +if (ret < 0) {
> > > > +error_setg_errno(errp, errno, "Unable to set xlator key/value 
> > > > pair");
> > > > +ret = -errno;
> > > > +goto out;
> > > > +}
> > > > +#endif
> > > 
> > > We also need to consider the case without CONFIG_GLUSTERFS_XLATOR_OPT.
> > > In this case (as well as theoretically in the case that the option
> > > didn't take effect - if only we could know about it), a failed
> > > glfs_fsync_async() is fatal and we need to stop operating on the image,
> > > i.e. set bs->drv = NULL like when we detect corruption in qcow2 images.
> > > The guest will see a broken disk that fails all I/O requests, but that's
> > > better than corrupting data.
> > >
> > 
> > Gluster versions that don't support CONFIG_GLUSTERFS_XLATOR_OPT will
> > also not have the gluster patch that removes the file descriptor
> > invalidation upon error (unless that was a relatively new
> > bug/feature).  But if that is the case, every operation on the file
> > descriptor in those versions will return error.  But it is also rather
> > old versions that don't support glfs_set_xlator_option() I believe.
> 
> Indeed, glfs_set_xlator_option() was introduced with glusterfs-3.4.0. We
> are currently on glusterfs-3.7, with the oldest supported version of
> 3.5. In ~2 months we hopefully have a 3.8 release and that will cause

Is it possible for 3.8 to be able to validate key/value pairs in
glfs_set_xlator_option()?  Or is it something that by design is decoupled
enough that it isn't feasible?

This is the second instance I know of that a lack of error information from
the gluster api has made it difficult to determine if a feature is
supported (the other instance being SEEK_DATA/SEEK_HOLE).  It'd be nice if
error returns were more consistent in 3.8 in the API, is possible.

> the end-of-life of 3.5. 3.4 has been EOL for about a year now, hopefully
> 

Re: [Qemu-devel] [Qemu-block] [PATCH 4/7] qemu-iotests: 148: properly skip test if quorum support is missing

[Max Reitz]

On 05.04.2016 11:21, Sascha Silbe wrote:
> qemu-iotests test case 148 already had some code for skipping the test
> if quorum support is missing, but it didn't work in all
> cases. TestQuorumEvents.setUp() gets run before the actual test class
> (which contains the skipping code) and tries to start qemu with a drive
> using the quorum driver. For some reason this works fine when using
> qcow2, but fails for raw.
> 
> As the entire test case requires quorum, just check for availability
> before even starting the test suite. Introduce a verify_quorum()
> function in iotests.py for this purpose so future test cases can make
> use of it.
> 
> Signed-off-by: Sascha Silbe 
> Reviewed-by: Bo Tu 
> ---
>  tests/qemu-iotests/148| 4 +---
>  tests/qemu-iotests/iotests.py | 5 +
>  2 files changed, 6 insertions(+), 3 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-2.6] block: Forbid I/O throttling on nodes with multiple parents for 2.6

[Paolo Bonzini]

On 06/04/2016 17:55, Alberto Garcia wrote:
>> > [BB] overlay
>> >  |
>> >  |
>> > [BDS] qcow2 [BB] backing
>> >  ||
>> >  ++
>> >  .|
>> >  . I/O throttling
>> >  .|
>> > [BDS] raw
>> >   .
>> >   .
>> >   .
> I see, thanks for the explanation!
> 
> But what's the use of having a BlockBackend on a node that is used as a
> backing image? Is it because "we allow this so let's prevent the user
> from doing weird things", or is there anything useful we can do with it?

The backing image can be the one that is visible to the guest, while the
overlay can be a block-backup target (with 'sync':'none) whose
BlockBackend is created by the NBD server.

Paolo

Re: [Qemu-devel] [Qemu-block] [PATCH 3/7] qemu-iotests: iotests.VM: remove qtest socket on error

[Max Reitz]

On 05.04.2016 11:21, Sascha Silbe wrote:
> On error, VM.launch() cleaned up the monitor unix socket, but left the
> qtest unix socket behind. This caused the remaining sub-tests to fail
> with EADDRINUSE:
> 
> +==
> +ERROR: testQuorum (__main__.TestFifoQuorumEvents)
> +--
> +Traceback (most recent call last):
> +  File "148", line 63, in setUp
> +self.vm.launch()
> +  File "/home6/silbe/qemu/tests/qemu-iotests/iotests.py", line 247, in launch
> +self._qmp.accept()
> +  File "/home6/silbe/qemu/tests/qemu-iotests/../../scripts/qmp/qmp.py", line 
> 141, in accept
> +return self.__negotiate_capabilities()
> +  File "/home6/silbe/qemu/tests/qemu-iotests/../../scripts/qmp/qmp.py", line 
> 57, in __negotiate_capabilities
> +raise QMPConnectError
> +QMPConnectError
> +
> +==
> +ERROR: testQuorum (__main__.TestQuorumEvents)
> +--
> +Traceback (most recent call last):
> +  File "148", line 63, in setUp
> +self.vm.launch()
> +  File "/home6/silbe/qemu/tests/qemu-iotests/iotests.py", line 244, in launch
> +self._qtest = qtest.QEMUQtestProtocol(self._qtest_path, server=True)
> +  File "/home6/silbe/qemu/tests/qemu-iotests/../../scripts/qtest.py", line 
> 33, in __init__
> +self._sock.bind(self._address)
> +  File "/usr/lib64/python2.7/socket.py", line 224, in meth
> +return getattr(self._sock,name)(*args)
> +error: [Errno 98] Address already in use
> 
> Fix this by cleaning up both the monitor socket and the qtest socket iff
> they exist.
> 
> Signed-off-by: Sascha Silbe 
> Reviewed-by: Bo Tu 
> ---
>  tests/qemu-iotests/iotests.py | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)

Not sure about the leading underscore (it appears to be the only such
function in iotests.py besides __init__()), but I guess it's a test so
it doesn't really matter anyway.

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-2.6] block: Forbid I/O throttling on nodes with multiple parents for 2.6

[Alberto Garcia]

On Wed 06 Apr 2016 11:59:47 AM CEST, Kevin Wolf wrote:
> Let me draw some ASCII art. I'm drawing I/O throttling like a filter
> to make the difference clear; in reality it's part of the BDS now or
> of the BB in 2.7, so don't let this confuse you.
>
> Let's assume these command line options:
>
> -drive file=backimg.img,if=none,id=backing,iops=1024
> -drive file=overlay.qcow2,id=overlay,backing=backing
>
> If we didn't catch this configuration and error out, this is what we
> would end up with in 2.6:
>
> [BB] overlay
>  |
>  |
> [BDS] qcow2 [BB] backing
>  ||
>  ++
>  .|
>  . I/O throttling
>  .|
> [BDS] raw
>   .
>   .
>   .

I see, thanks for the explanation!

But what's the use of having a BlockBackend on a node that is used as a
backing image? Is it because "we allow this so let's prevent the user
from doing weird things", or is there anything useful we can do with it?

Berto

Re: [Qemu-devel] [PATCH 05/10] include: add spinlock wrapper

[Alex Bennée]

Paolo Bonzini  writes:

> On 05/04/2016 07:30, Emilio G. Cota wrote:
>> Wrap pthread_spin on POSIX, or QemuMutex on Windows.
>>
>> AFAIK there are is no off-the-shelf spinlock implementation for
>> Windows, so we'll just use QemuMutex.
>
> It's much simpler to use a simple test-and-set spinlock.
>
> GitHub is down, but this should be the link
> http://github.com/bonzini/qemu/commit/e1361634 (it's in my mttcg
> branch).

In addition this version breaks on the OSX/clang build:

https://travis-ci.org/stsquad/qemu/jobs/121002991

>
> Paolo
>
>> Signed-off-by: Emilio G. Cota 
>> ---
>>  include/qemu/spinlock-posix.h | 60 
>> +++
>>  include/qemu/spinlock-win32.h | 33 
>>  include/qemu/spinlock.h   | 10 
>>  3 files changed, 103 insertions(+)
>>  create mode 100644 include/qemu/spinlock-posix.h
>>  create mode 100644 include/qemu/spinlock-win32.h
>>  create mode 100644 include/qemu/spinlock.h
>>
>> diff --git a/include/qemu/spinlock-posix.h b/include/qemu/spinlock-posix.h
>> new file mode 100644
>> index 000..51c2c08
>> --- /dev/null
>> +++ b/include/qemu/spinlock-posix.h
>> @@ -0,0 +1,60 @@
>> +#ifndef QEMU_SPINLOCK_POSIX_H
>> +#define QEMU_SPINLOCK_POSIX_H
>> +
>> +#include 
>> +#include 
>> +
>> +typedef pthread_spinlock_t QemuSpinLock;
>> +
>> +static inline void qemu_spinlock_error_exit(int err, const char *msg)
>> +{
>> +fprintf(stderr, "qemu: %s: %s\n", msg, strerror(err));
>> +abort();
>> +}
>> +
>> +static inline void qemu_spinlock_init(QemuSpinLock *lock)
>> +{
>> +int rc;
>> +
>> +rc = pthread_spin_init(lock, PTHREAD_PROCESS_SHARED);
>> +if (unlikely(rc)) {
>> +qemu_spinlock_error_exit(rc, __func__);
>> +}
>> +}
>> +
>> +static inline void qemu_spinlock_destroy(QemuSpinLock *lock)
>> +{
>> +int rc;
>> +
>> +rc = pthread_spin_destroy(lock);
>> +if (unlikely(rc)) {
>> +qemu_spinlock_error_exit(rc, __func__);
>> +}
>> +}
>> +
>> +static inline void qemu_spinlock_lock(QemuSpinLock *lock)
>> +{
>> +int rc;
>> +
>> +rc = pthread_spin_lock(lock);
>> +if (unlikely(rc)) {
>> +qemu_spinlock_error_exit(rc, __func__);
>> +}
>> +}
>> +
>> +static inline int qemu_spinlock_trylock(QemuSpinLock *lock)
>> +{
>> +return pthread_spin_trylock(lock);
>> +}
>> +
>> +static inline void qemu_spinlock_unlock(QemuSpinLock *lock)
>> +{
>> +int rc;
>> +
>> +rc = pthread_spin_unlock(lock);
>> +if (unlikely(rc)) {
>> +qemu_spinlock_error_exit(rc, __func__);
>> +}
>> +}
>> +
>> +#endif /* QEMU_SPINLOCK_POSIX_H */
>> diff --git a/include/qemu/spinlock-win32.h b/include/qemu/spinlock-win32.h
>> new file mode 100644
>> index 000..5a105fb
>> --- /dev/null
>> +++ b/include/qemu/spinlock-win32.h
>> @@ -0,0 +1,33 @@
>> +#ifndef QEMU_SPINLOCK_WIN32_H
>> +#define QEMU_SPINLOCK_WIN32_H
>> +
>> +#include 
>> +
>> +typedef QemuMutex QemuSpinLock;
>> +
>> +static inline void qemu_spinlock_init(QemuSpinLock *lock)
>> +{
>> +qemu_mutex_init(lock);
>> +}
>> +
>> +static inline void qemu_spinlock_destroy(QemuSpinLock *lock)
>> +{
>> +qemu_mutex_destroy(lock);
>> +}
>> +
>> +static inline void qemu_spinlock_lock(QemuSpinLock *lock)
>> +{
>> +qemu_mutex_lock(lock);
>> +}
>> +
>> +static inline int qemu_spinlock_trylock(QemuSpinLock *lock)
>> +{
>> +return qemu_mutex_trylock(lock);
>> +}
>> +
>> +static inline void qemu_spinlock_unlock(QemuSpinLock *lock)
>> +{
>> +qemu_mutex_unlock(lock);
>> +}
>> +
>> +#endif /* QEMU_SPINLOCK_WIN32_H */
>> diff --git a/include/qemu/spinlock.h b/include/qemu/spinlock.h
>> new file mode 100644
>> index 000..001b55e
>> --- /dev/null
>> +++ b/include/qemu/spinlock.h
>> @@ -0,0 +1,10 @@
>> +#ifndef QEMU_SPINLOCK_H
>> +#define QEMU_SPINLOCK_H
>> +
>> +#ifdef _WIN32
>> +#include "spinlock-win32.h"
>> +#else
>> +#include "spinlock-posix.h"
>> +#endif
>> +
>> +#endif /* QEMU_SPINLOCK_H */
>>


--
Alex Bennée

Re: [Qemu-devel] [Qemu-block] [PATCH 2/7] qemu-iotests: fix 051 on non-PC architectures

[Max Reitz]

On 05.04.2016 11:21, Sascha Silbe wrote:
> Commit 61de4c68 [block: Remove BDRV_O_CACHE_WB] updated the reference
> output for PCs, but neglected to do the same for the generic reference
> output file. Fix 051 on all non-PC architectures by applying the same
> change to the generic output file.
> 
> Fixes: 61de4c68 ("block: Remove BDRV_O_CACHE_WB")
> Signed-off-by: Sascha Silbe 
> Reviewed-by: Bo Tu 
> ---
>  tests/qemu-iotests/051.out | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)

Well, that's interesting. I did note that in
http://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg06123.html
and thus didn't give an R-b. But in v2,
(http://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg06353.html)
my R-b magically had appeared there without the patch being changed. I
guess I need to be more careful with reviewing revised patch series from
now on, whoever they are from. :-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Nbd] [PATCH] doc: Wording cleanups

[Alex Bligh]

On 6 Apr 2016, at 16:35, Eric Blake  wrote:

> Fix several document inconsistencies (missing references,
> rewrap long lines, address typos, improve grammar) introduced
> in recent patches.
> 
> Signed-off-by: Eric Blake 

Reviewed-by: Alex Bligh 

Most of that mess was mine - thanks for clearing it up.

Alex


> 
> ---
> Note that there is still ongoing discussion about the correct
> layout to use for NBD_REP_SERVER in response to NBD_OPT_INFO
> (and whether the layout should be a superset of the
> NBD_OPT_EXPORT_NAME reply) - cleaning that up will be a
> separate patch, once consensus is reached on the best layout.
> ---
> doc/proto.md | 51 +++
> 1 file changed, 31 insertions(+), 20 deletions(-)
> 
> diff --git a/doc/proto.md b/doc/proto.md
> index 4d63b23..3a70209 100644
> --- a/doc/proto.md
> +++ b/doc/proto.md
> @@ -445,8 +445,8 @@ during option haggling in the fixed newstyle negotiation.
>   encoded data suitable for direct display to a human being; with
>   no embedded or terminating NUL characters.
> 
> -The experimental `INFO` extension (see below) is a client
> -request where the extra data has a definition other than a
> +The experimental `INFO` extension (see below) adds two client
> +option requests where the extra data has a definition other than a
> UTF-8 message.
> 
> There are a number of error reply types, all of which are denoted by
> @@ -527,7 +527,7 @@ valid may depend on negotiation during the handshake 
> phase.
>   not in fact write data (for instance on an `NBD_CMD_TRIM` in a situation
>   where the command as a whole is ignored), the server MAY ignore this bit
>   being set on such a command.
> -- bit 1, `NBD_CMD_NO_HOLE`; defined by the experimental `WRITE_ZEROES`
> +- bit 1, `NBD_CMD_FLAG_NO_HOLE`; defined by the experimental `WRITE_ZEROES`
>   extension; see below.
> - bit 2, `NBD_CMD_FLAG_DF`; defined by the experimental `STRUCTURED_REPLY`
>   extension; see below
> @@ -608,7 +608,7 @@ The following request types exist:
> 
> After issuing this command, a client MUST NOT make any assumptions
> about the contents of the export affected by this command, until
> -overwriting it again with `NBD_CMD_WRITE`.
> +overwriting it again with `NBD_CMD_WRITE` or `NBD_CMD_WRITE_ZEROES`.
> 
> A client MUST NOT send a trim request unless `NBD_FLAG_SEND_TRIM`
> was set in the transmission flags field.
> @@ -700,15 +700,18 @@ Therefore these commands share common documentation.
> 
> * `NBD_OPT_INFO` and `NBD_OPT_GO`
> 
> -`NBD_OPT_INFO`: The client wishes to get details about export with the
> -given name for use in the transmission phase, but does not yet want
> -to move to the transmission phase.
> +`NBD_OPT_INFO`: The client wishes to get details about an export
> +with the given name for use in the transmission phase, but does
> +not yet want to move to the transmission phase.  When successful,
> +this option provides more details than `NBD_OPT_LIST`, but only
> +for a single export name.
> 
> -`NBD_OPT_GO`: The client wishes to terminate the negotiation phase and
> -progress to the transmission phase. This client MAY issue this command 
> after
> -an `NBD_OPT_INFO`, or MAY issue it without a previous `NBD_OPT_INFO`.
> -`NBD_OPT_GO` can thus be used as a substitute for `NBD_OPT_EXPORT_NAME`
> -that returns errors.
> +`NBD_OPT_GO`: The client wishes to terminate the handshake phase
> +and progress to the transmission phase. This client MAY issue this
> +command after an `NBD_OPT_INFO`, or MAY issue it without a
> +previous `NBD_OPT_INFO`.  `NBD_OPT_GO` can thus be used as an
> +improved version of `NBD_OPT_EXPORT_NAME` that is capable of
> +returning errors.
> 
> Data (both commands):
> 
> @@ -742,9 +745,9 @@ Therefore these commands share common documentation.
> - 16 bits, transmission flags.
> - 32 bits, length of name (unsigned)
> - Name of the export. This name MAY be different from the one
> -  given in the `NBD_OPT_INFO` or `NBD_OPT_GO` option in case the server 
> has
> -  multiple alternate names for a single export, or a default
> -  export was specified.
> +  given in the `NBD_OPT_INFO` or `NBD_OPT_GO` option in case the
> +  server has multiple alternate names for a single export, or a
> +  default export was specified.
> 
> The server MUST NOT fail an NDB_OPT_GO sent with the same parameters
> as a previous NBD_OPT_INFO which returned successfully (i.e. with
> @@ -754,7 +757,7 @@ Therefore these commands share common documentation.
> intervening time the client has negotiated other options.
> The values of the transmission flags MAY differ from what was sent
> earlier in response to an earlier `NBD_OPT_INFO` (if any), and/or
> -the server may fail the request, based on other options that 

[Qemu-devel] [PATCH 2/2] vhost-user: delete chardev on cleanup

[marcandre . lureau]

From: Marc-André Lureau 

Remove the chardev implicitely when cleaning up the netdev. This
prevents from reusing the chardev since it would be in an incorrect
state with the slave.

Signed-off-by: Marc-André Lureau 
---
 net/vhost-user.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 29fe097..2d7271a 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -144,6 +144,7 @@ static void vhost_user_cleanup(NetClientState *nc)
 qemu_purge_queued_packets(nc);
 
 qemu_chr_fe_release(s->chr);
+qemu_chr_delete(s->chr);
 }
 
 static bool vhost_user_has_vnet_hdr(NetClientState *nc)
-- 
2.5.5

[Qemu-devel] [PATCH 1/2] vhost-user: release chardev on cleanup

[marcandre . lureau]

From: Marc-André Lureau 

On init, vhost-user does qemu_chr_fe_claim_no_fail(). Do the opposite on
cleanup to allow removing or reusing the chardev.

Signed-off-by: Marc-André Lureau 
---
 net/vhost-user.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 1b9e73a..29fe097 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -142,6 +142,8 @@ static void vhost_user_cleanup(NetClientState *nc)
 }
 
 qemu_purge_queued_packets(nc);
+
+qemu_chr_fe_release(s->chr);
 }
 
 static bool vhost_user_has_vnet_hdr(NetClientState *nc)
-- 
2.5.5

[Qemu-devel] [PATCH 0/2] vhost-user: release chardev on cleanup

[marcandre . lureau]

From: Marc-André Lureau 

The following 2 one-liners remove the chardev from the vhost-user
netdev it is attached to. This prevents from further reuse of the
chardev that would likely fail to setup again with the slave.

Related to:
https://bugzilla.redhat.com/show_bug.cgi?id=1256619
https://bugzilla.redhat.com/show_bug.cgi?id=1256618

rfc->v1:
- rfc was sent a few months ago, no comments: rebased & resend as
  non-rfc this time.

Marc-André Lureau (2):
  vhost-user: release chardev on cleanup
  vhost-user: delete chardev on cleanup

 net/vhost-user.c | 3 +++
 1 file changed, 3 insertions(+)

-- 
2.5.5

Re: [Qemu-devel] [Qemu-block] [PATCH 1/7] qemu-iotests: check: don't place files with predictable names in /tmp

[Max Reitz]

On 05.04.2016 11:21, Sascha Silbe wrote:
> Placing files with predictable or even hard-coded names in /tmp is a
> security risk and can prevent or disturb operation on a multi-user
> machine. Place them inside the "scratch" directory instead, as we
> already do for most other test-related files.
> 
> Signed-off-by: Sascha Silbe 
> Reviewed-by: Bo Tu 
> ---
>  tests/qemu-iotests/check | 21 +++--
>  1 file changed, 11 insertions(+), 10 deletions(-)

Nice side effect: With this patch it's possible to run multiple
instances of the iotests in parallel (for different formats/protocols)
without them interfering with each other.

Grepping for '/tmp' in the iotests directory yields more occurrences,
however: Many tests set the tmp variable to /tmp/$$. Let's see whether
we can just remove that or have to replace it by "${TEST_DIR}"/$$.

"common.filter" evaluates $tmp, but the single filter that does so is
actually never used any more. Other than that, only "common" evaluates
it, but "common" is sourced by "check". Thus I think those tests setting
$tmp is superfluous and dropping it should be fine.

For this patch:

Reviewed-by: Max Reitz 

You decide whether you want to drop the tmp=/tmp/$$ lines in the tests
in a dedicated (follow-up) patch or include it here.

Max



signature.asc
Description: OpenPGP digital signature