Re: [Qemu-devel] [Qemu-stable] About qemu-kvm branch

[Fam Zheng]

On Tue, 09/15 14:51, tseng gogo wrote:
> Hi ,
> 
> Sorry in advance that I am not sure that if I ask this question here is
> appropriate. I am new to use KVM and want to do some research in qemu-kvm
> live migration.
> 
> In my environment, I install centos 7.0 with rpm install qemu-kvm
> 1.5.3, but what I want to do research is the source code. So I try to find
> qemu-kvm 1.5.3 on the Internet, but there is nothing I can find.
> 
> I tried to git clone to git://
> git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git, but the qemu-kvm which I git
> clone is version 1.2.5.
> 
> Can someone tell me that where can I find the qemu-kvm 1.5.3 source
> code branch? Also what I have googled saying that qemu-kvm fork is
> deprecated, is this mean that if we want to use the latest version
> qemu-kvm, we should download the qemu and configure it?
> 
> Thank everyone in advance!

Hi!

Nowadays qemu-kvm in CentOS is actually based on QEMU instead of the foregone
qemu-kvm fork, since qemu-kvm got merged back to QEMU years ago. The srpm is
not hard to find if you tried to google "how to download source rpm in centos
7":

https://wiki.centos.org/HowTos/RebuildSRPM

http://vault.centos.org/7.0.1406/os/Source/SPackages/qemu-kvm-1.5.3-60.el7.src.rpm

[BTW, qemu-stable and qemu-trivial are for particular patch contribution, not
for general discussion. Please just send to qemu-devel and/or qemu-discussion
next time.]

Fam

[Qemu-devel] [PATCH 0/7 v9] vhost-user multiple queue support

[Yuanhan Liu]

Hi,

Here is the updated patch set for enabling vhost-user multiple queue.

This patch set introduces 2 more vhost user messages: VHOST_USER_GET_QUEUE_NUM,
for querying how many queues the backend supports, and 
VHOST_USER_SET_VRING_ENABLE,
for enabling/disabling a specific virt queue.

Both of the two new messages are treated as vhost protocol extension,
and that's why Michaels's patch "vhost-user: add protocol feature
negotiation" is also included here.

Patch 1-5 are all prepare works for actually enabling multiple queue.

Patch 6 is the major patch for enabling multiple queue, which also tries
to address two major concerns from Michael: no feedback from backend if
it can't support # of requested queues, and all messages are sent N time.
It also fixes a hidden bug.

Patch 7 introduces the VHOST_USER_SET_VRING_ENABLE message, to enable
or disable a specific vring.

Note that I haven't done any formal test yet, it just passes build
test and basic functional test, such as it does exit when backend
doesn't support # of requested queues. Here I sent it out just for
more comments, and for avoiding spending too much effort on the wrong
track.


v9: - Per suggested by Jason Wang, patch 5 introduces a new vhost
  backend method: vhost_backend_get_vq_index().

- Use qemu_find_net_clients_except() at net/vhost-user.c for
  gathering all related ncs so that we could register chr dev
  event handler once. Which is also suggested by Jason Wang.


Thanks.

--yliu

---
Changchun Ouyang (2):
  vhost-user: add multiple queue support
  vhost-user: add a new message to disable/enable a specific virt queue.

Michael S. Tsirkin (1):
  vhost-user: add protocol feature negotiation

Yuanhan Liu (4):
  vhost-user: use VHOST_USER_XXX macro for switch statement
  vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE
  vhost-user: add VHOST_USER_GET_QUEUE_NUM message
  vhost: introduce vhost_backend_get_vq_index method

 docs/specs/vhost-user.txt |  75 +++-
 hw/net/vhost_net.c|  39 --
 hw/net/virtio-net.c   |   8 +++
 hw/virtio/vhost-backend.c |  10 ++-
 hw/virtio/vhost-user.c| 143 +++--
 hw/virtio/vhost.c |  20 +++---
 include/hw/virtio/vhost-backend.h |   4 ++
 include/hw/virtio/vhost.h |   2 +
 include/net/vhost_net.h   |   3 +
 linux-headers/linux/vhost.h   |   2 +-
 net/vhost-user.c  | 146 +++---
 qapi-schema.json  |   6 +-
 qemu-options.hx   |   5 +-
 tests/vhost-user-test.c   |   2 +-
 14 files changed, 380 insertions(+), 85 deletions(-)

-- 
1.9.0

[Qemu-devel] [PATCH 3/7] vhost: rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE

[Yuanhan Liu]

Quote from Michael:

We really should rename VHOST_RESET_OWNER to VHOST_RESET_DEVICE.

Suggested-by: Michael S. Tsirkin 
Signed-off-by: Yuanhan Liu 
---
 docs/specs/vhost-user.txt   | 4 ++--
 hw/net/vhost_net.c  | 2 +-
 hw/virtio/vhost-user.c  | 6 +++---
 linux-headers/linux/vhost.h | 2 +-
 tests/vhost-user-test.c | 2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 70da3b1..ccbbcbb 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -194,10 +194,10 @@ Message types
   as an owner of the session. This can be used on the Slave as a
   "session start" flag.
 
- * VHOST_USER_RESET_OWNER
+ * VHOST_USER_RESET_DEVICE
 
   Id: 4
-  Equivalent ioctl: VHOST_RESET_OWNER
+  Equivalent ioctl: VHOST_RESET_DEVICE
   Master payload: N/A
 
   Issued when a new connection is about to be closed. The Master will no
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index c864237..33b0e97 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -287,7 +287,7 @@ static void vhost_net_stop_one(struct vhost_net *net,
 } else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
 for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
 const VhostOps *vhost_ops = net->dev.vhost_ops;
-int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_OWNER,
+int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_DEVICE,
   NULL);
 assert(r >= 0);
 }
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 43169a1..0ac8252 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -32,7 +32,7 @@ typedef enum VhostUserRequest {
 VHOST_USER_GET_FEATURES = 1,
 VHOST_USER_SET_FEATURES = 2,
 VHOST_USER_SET_OWNER = 3,
-VHOST_USER_RESET_OWNER = 4,
+VHOST_USER_RESET_DEVICE = 4,
 VHOST_USER_SET_MEM_TABLE = 5,
 VHOST_USER_SET_LOG_BASE = 6,
 VHOST_USER_SET_LOG_FD = 7,
@@ -98,7 +98,7 @@ static unsigned long int 
ioctl_to_vhost_user_request[VHOST_USER_MAX] = {
 VHOST_GET_FEATURES, /* VHOST_USER_GET_FEATURES */
 VHOST_SET_FEATURES, /* VHOST_USER_SET_FEATURES */
 VHOST_SET_OWNER,/* VHOST_USER_SET_OWNER */
-VHOST_RESET_OWNER,  /* VHOST_USER_RESET_OWNER */
+VHOST_RESET_DEVICE,  /* VHOST_USER_RESET_DEVICE */
 VHOST_SET_MEM_TABLE,/* VHOST_USER_SET_MEM_TABLE */
 VHOST_SET_LOG_BASE, /* VHOST_USER_SET_LOG_BASE */
 VHOST_SET_LOG_FD,   /* VHOST_USER_SET_LOG_FD */
@@ -222,7 +222,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned 
long int request,
 break;
 
 case VHOST_USER_SET_OWNER:
-case VHOST_USER_RESET_OWNER:
+case VHOST_USER_RESET_DEVICE:
 break;
 
 case VHOST_USER_SET_MEM_TABLE:
diff --git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
index ead86db..14a0160 100644
--- a/linux-headers/linux/vhost.h
+++ b/linux-headers/linux/vhost.h
@@ -78,7 +78,7 @@ struct vhost_memory {
 #define VHOST_SET_OWNER _IO(VHOST_VIRTIO, 0x01)
 /* Give up ownership, and reset the device to default values.
  * Allows subsequent call to VHOST_OWNER_SET to succeed. */
-#define VHOST_RESET_OWNER _IO(VHOST_VIRTIO, 0x02)
+#define VHOST_RESET_DEVICE _IO(VHOST_VIRTIO, 0x02)
 
 /* Set up/modify memory layout */
 #define VHOST_SET_MEM_TABLE_IOW(VHOST_VIRTIO, 0x03, struct vhost_memory)
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 75fedf0..e301db7 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -58,7 +58,7 @@ typedef enum VhostUserRequest {
 VHOST_USER_GET_FEATURES = 1,
 VHOST_USER_SET_FEATURES = 2,
 VHOST_USER_SET_OWNER = 3,
-VHOST_USER_RESET_OWNER = 4,
+VHOST_USER_RESET_DEVICE = 4,
 VHOST_USER_SET_MEM_TABLE = 5,
 VHOST_USER_SET_LOG_BASE = 6,
 VHOST_USER_SET_LOG_FD = 7,
-- 
1.9.0

[Qemu-devel] [PATCH 1/7] vhost-user: use VHOST_USER_XXX macro for switch statement

[Yuanhan Liu]

So that we could let vhost_user_call to handle extented requests,
such as VHOST_USER_GET/SET_PROTOCOL_FEATURES, instead of invoking
vhost_user_read/write and constructing the msg again by ourself.

Signed-off-by: Yuanhan Liu 
---
 hw/virtio/vhost-user.c | 38 ++
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index e7ab829..13677ac 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -193,27 +193,33 @@ static int vhost_user_call(struct vhost_dev *dev, 
unsigned long int request,
 
 assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
 
-msg_request = vhost_user_request_translate(request);
+/* only translate vhost ioctl requests */
+if (request > VHOST_USER_MAX) {
+msg_request = vhost_user_request_translate(request);
+} else {
+msg_request = request;
+}
+
 msg.request = msg_request;
 msg.flags = VHOST_USER_VERSION;
 msg.size = 0;
 
-switch (request) {
-case VHOST_GET_FEATURES:
+switch (msg_request) {
+case VHOST_USER_GET_FEATURES:
 need_reply = 1;
 break;
 
-case VHOST_SET_FEATURES:
-case VHOST_SET_LOG_BASE:
+case VHOST_USER_SET_FEATURES:
+case VHOST_USER_SET_LOG_BASE:
 msg.u64 = *((__u64 *) arg);
 msg.size = sizeof(m.u64);
 break;
 
-case VHOST_SET_OWNER:
-case VHOST_RESET_OWNER:
+case VHOST_USER_SET_OWNER:
+case VHOST_USER_RESET_OWNER:
 break;
 
-case VHOST_SET_MEM_TABLE:
+case VHOST_USER_SET_MEM_TABLE:
 for (i = 0; i < dev->mem->nregions; ++i) {
 struct vhost_memory_region *reg = dev->mem->regions + i;
 ram_addr_t ram_addr;
@@ -246,30 +252,30 @@ static int vhost_user_call(struct vhost_dev *dev, 
unsigned long int request,
 
 break;
 
-case VHOST_SET_LOG_FD:
+case VHOST_USER_SET_LOG_FD:
 fds[fd_num++] = *((int *) arg);
 break;
 
-case VHOST_SET_VRING_NUM:
-case VHOST_SET_VRING_BASE:
+case VHOST_USER_SET_VRING_NUM:
+case VHOST_USER_SET_VRING_BASE:
 memcpy(&msg.state, arg, sizeof(struct vhost_vring_state));
 msg.size = sizeof(m.state);
 break;
 
-case VHOST_GET_VRING_BASE:
+case VHOST_USER_GET_VRING_BASE:
 memcpy(&msg.state, arg, sizeof(struct vhost_vring_state));
 msg.size = sizeof(m.state);
 need_reply = 1;
 break;
 
-case VHOST_SET_VRING_ADDR:
+case VHOST_USER_SET_VRING_ADDR:
 memcpy(&msg.addr, arg, sizeof(struct vhost_vring_addr));
 msg.size = sizeof(m.addr);
 break;
 
-case VHOST_SET_VRING_KICK:
-case VHOST_SET_VRING_CALL:
-case VHOST_SET_VRING_ERR:
+case VHOST_USER_SET_VRING_KICK:
+case VHOST_USER_SET_VRING_CALL:
+case VHOST_USER_SET_VRING_ERR:
 file = arg;
 msg.u64 = file->index & VHOST_USER_VRING_IDX_MASK;
 msg.size = sizeof(m.u64);
-- 
1.9.0

[Qemu-devel] [PATCH 4/7] vhost-user: add VHOST_USER_GET_QUEUE_NUM message

[Yuanhan Liu]

This is for querying how many queues the backend supports if it has mq
support(when VHOST_USER_PROTOCOL_F_MQ flag is set from the quried
protocol features).

vhost_net_get_max_queues() is the interface to export that value, and
to tell if the backend supports # of queues user requested, which is
done in the following patch.

Signed-off-by: Yuanhan Liu 
---
 docs/specs/vhost-user.txt | 11 +++
 hw/net/vhost_net.c|  7 +++
 hw/virtio/vhost-user.c| 15 ++-
 include/hw/virtio/vhost.h |  1 +
 include/net/vhost_net.h   |  1 +
 5 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index ccbbcbb..43db9b4 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -301,3 +301,14 @@ Message types
   Bits (0-7) of the payload contain the vring index. Bit 8 is the
   invalid FD flag. This flag is set when there is no file descriptor
   in the ancillary data.
+
+ * VHOST_USER_GET_QUEUE_NUM
+
+  Id: 17
+  Equivalent ioctl: N/A
+  Master payload: N/A
+  Slave payload: u64
+
+  Query how many queues the backend supports. This request should be
+  sent only when VHOST_USER_PROTOCOL_F_MQ is set in quried protocol
+  features by VHOST_USER_GET_PROTOCOL_FEATURES.
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 33b0e97..f9441e9 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -122,6 +122,11 @@ void vhost_net_ack_features(struct vhost_net *net, 
uint64_t features)
 vhost_ack_features(&net->dev, vhost_net_get_feature_bits(net), features);
 }
 
+uint64_t vhost_net_get_max_queues(VHostNetState *net)
+{
+return net->dev.max_queues;
+}
+
 static int vhost_net_get_fd(NetClientState *backend)
 {
 switch (backend->info->type) {
@@ -144,6 +149,8 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
 goto fail;
 }
 
+net->dev.max_queues = 1;
+
 if (backend_kernel) {
 r = vhost_net_get_fd(options->net_backend);
 if (r < 0) {
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 0ac8252..8b4b36b 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -25,7 +25,9 @@
 
 #define VHOST_MEMORY_MAX_NREGIONS8
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
-#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x0ULL
+#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x1ULL
+
+#define VHOST_USER_PROTOCOL_F_MQ0
 
 typedef enum VhostUserRequest {
 VHOST_USER_NONE = 0,
@@ -45,6 +47,7 @@ typedef enum VhostUserRequest {
 VHOST_USER_SET_VRING_ERR = 14,
 VHOST_USER_GET_PROTOCOL_FEATURES = 15,
 VHOST_USER_SET_PROTOCOL_FEATURES = 16,
+VHOST_USER_GET_QUEUE_NUM = 17,
 VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -211,6 +214,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned 
long int request,
 switch (msg_request) {
 case VHOST_USER_GET_FEATURES:
 case VHOST_USER_GET_PROTOCOL_FEATURES:
+case VHOST_USER_GET_QUEUE_NUM:
 need_reply = 1;
 break;
 
@@ -315,6 +319,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned 
long int request,
 switch (msg_request) {
 case VHOST_USER_GET_FEATURES:
 case VHOST_USER_GET_PROTOCOL_FEATURES:
+case VHOST_USER_GET_QUEUE_NUM:
 if (msg.size != sizeof(m.u64)) {
 error_report("Received bad msg size.");
 return -1;
@@ -366,6 +371,14 @@ static int vhost_user_init(struct vhost_dev *dev, void 
*opaque)
 if (err < 0) {
 return err;
 }
+
+/* query the max queues we support if backend supports Multiple Queue 
*/
+if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)) {
+err = vhost_user_call(dev, VHOST_USER_GET_QUEUE_NUM, 
&dev->max_queues);
+if (err < 0) {
+return err;
+}
+}
 }
 
 return 0;
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 6467c73..c3758f3 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -48,6 +48,7 @@ struct vhost_dev {
 unsigned long long acked_features;
 unsigned long long backend_features;
 unsigned long long protocol_features;
+unsigned long long max_queues;
 bool started;
 bool log_enabled;
 unsigned long long log_size;
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 840d4b1..8db723e 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -13,6 +13,7 @@ typedef struct VhostNetOptions {
 void *opaque;
 } VhostNetOptions;
 
+uint64_t vhost_net_get_max_queues(VHostNetState *net);
 struct vhost_net *vhost_net_init(VhostNetOptions *options);
 
 int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, int total_queues);
-- 
1.9.0

[Qemu-devel] [PATCH 6/7] vhost-user: add multiple queue support

[Yuanhan Liu]

From: Changchun Ouyang 

This patch is initially based a patch from Nikolay Nikolaev.

Here is the latest version for adding vhost-user multiple queue support,
by creating a nc and vhost_net pair for each queue.

Qemu exits if find that the backend can't support number of requested
queues(by providing queues=# option). The max number is queried by a
new message, VHOST_USER_GET_QUEUE_NUM, and is sent only when protocol
feature VHOST_USER_PROTOCOL_F_MQ is present first.

The max queue check is done at vhost-user initiation stage. We initiate
one queue first, which, in the meantime, also gets the max_queues the
backend supports.

In older version, it was reported that some messages are sent more times
than necessary. Here we came an agreement with Michael that we could
categorize vhost user messages to 2 types: none-vring specific messages,
which should be sent only once, and vring specific messages, which should
be sent per queue.

Here I introduced a helper function vhost_user_one_time_request(), which
lists following messages as none-vring specific messages:

VHOST_USER_GET_FEATURES
VHOST_USER_SET_FEATURES
VHOST_USER_GET_PROTOCOL_FEATURES
VHOST_USER_SET_PROTOCOL_FEATURES
VHOST_USER_SET_OWNER
VHOST_USER_RESET_DEVICE
VHOST_USER_SET_MEM_TABLE
VHOST_USER_GET_QUEUE_NUM

For above messages, we simply ignore them when they are not sent the first
time.

v9: per suggested by Jason Wang, we could invoke qemu_chr_add_handlers()
once only, and invoke qemu_find_net_clients_except() at the handler
to gather all related ncs, for initiating all queues. Which addresses
a hidden bug in older verion in a more QEMU-like way.

v8: set net->dev.vq_index correctly inside vhost_net_init() based on the
value from net->nc->queue_index.

Signed-off-by: Nikolay Nikolaev 
Signed-off-by: Changchun Ouyang 
Signed-off-by: Yuanhan Liu 
---
 docs/specs/vhost-user.txt |  13 +
 hw/net/vhost_net.c|  10 ++--
 hw/virtio/vhost-user.c|  26 +
 hw/virtio/vhost.c |   5 +-
 net/vhost-user.c  | 146 +-
 qapi-schema.json  |   6 +-
 qemu-options.hx   |   5 +-
 7 files changed, 161 insertions(+), 50 deletions(-)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 43db9b4..acf5708 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -135,6 +135,19 @@ As older slaves don't support negotiating protocol 
features,
 a feature bit was dedicated for this purpose:
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 
+Multiple queue support
+---
+Multiple queue is treated as a protocol extension, hence the slave has to
+implement protocol features first. Multiple queues is supported only when
+the protocol feature VHOST_USER_PROTOCOL_F_MQ(bit 0) is set.
+
+The max number of queues the slave supports can be queried with message
+VHOST_USER_GET_PROTOCOL_FEATURES. Master should stop when the number of
+requested queues is bigger than that.
+
+As all queues share one connection, the master uses a unique index for each
+queue in the sent message to identify a specified queue.
+
 Message types
 -
 
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index f9441e9..c114304 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -148,8 +148,11 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
 fprintf(stderr, "vhost-net requires net backend to be setup\n");
 goto fail;
 }
+net->nc = options->net_backend;
 
 net->dev.max_queues = 1;
+net->dev.nvqs = 2;
+net->dev.vqs = net->vqs;
 
 if (backend_kernel) {
 r = vhost_net_get_fd(options->net_backend);
@@ -164,11 +167,10 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
 net->dev.backend_features = 0;
 net->dev.protocol_features = 0;
 net->backend = -1;
-}
-net->nc = options->net_backend;
 
-net->dev.nvqs = 2;
-net->dev.vqs = net->vqs;
+/* vhost-user needs vq_index to initiate a specific queue pair */
+net->dev.vq_index = net->nc->queue_index * net->dev.nvqs;
+}
 
 r = vhost_dev_init(&net->dev, options->opaque,
options->backend_type);
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 69b085b..dfb5a61 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -187,6 +187,23 @@ static int vhost_user_write(struct vhost_dev *dev, 
VhostUserMsg *msg,
 0 : -1;
 }
 
+static bool vhost_user_one_time_request(VhostUserRequest request)
+{
+switch (request) {
+case VHOST_USER_GET_FEATURES:
+case VHOST_USER_SET_FEATURES:
+case VHOST_USER_GET_PROTOCOL_FEATURES:
+case VHOST_USER_SET_PROTOCOL_FEATURES:
+case VHOST_USER_SET_OWNER:
+case VHOST_USER_RESET_DEVICE:
+case VHOST_USER_SET_MEM_TABLE:
+case VHOST_USER_GET_QUEUE_NUM:
+return true;
+default:
+retu

[Qemu-devel] [PATCH 1/2] vhost-user: add multiple queue support

[Yuanhan Liu]

From: Changchun Ouyang 

This patch is initially based a patch from Nikolay Nikolaev.

Here is the latest version for adding vhost-user multiple queue support,
by creating a nc and vhost_net pair for each queue.

What differs from last version is that this patch addresses two major
concerns from Michael, and fixes one hidden bug.

- Concern #1: no feedback when the backend can't support # of
  requested queues(by providing queues=# option).

  Here we address this issue by querying VHOST_USER_PROTOCOL_F_MQ
  protocol features first, if not set, it means the backend don't
  support mq feature, and let max_queues be 1. Otherwise, we send
  another message, VHOST_USER_GET_QUEUE_NUM, for getting the max_queues
  the backend supports.

  At vhost-user initiation stage(net_vhost_user_start), we then initiate
  one queue first, which, in the meantime, also gets the max_queues.
  We then do a simple compare: if requested_queues > max_queues, we
  exit(I guess it's safe to exit here, as the vm is not running yet).

- Concern #2: some messages are sent more times than necessary.

  We came an agreement with Michael that we could categorize vhost
  user messages to 2 types: none-vring specific messages, which should
  be sent only once, and vring specific messages, which should be sent
  per queue.

  Here I introduced a helper function vhost_user_one_time_request(),
  which lists following messages as none-vring specific messages:

VHOST_USER_GET_FEATURES
VHOST_USER_SET_FEATURES
VHOST_USER_GET_PROTOCOL_FEATURES
VHOST_USER_SET_PROTOCOL_FEATURES
VHOST_USER_SET_OWNER
VHOST_USER_RESET_DEVICE
VHOST_USER_SET_MEM_TABLE
VHOST_USER_GET_QUEUE_NUM

  For above messages, we simply ignore them when they are not sent the first
  time.

I also observed a hidden bug from last version. We register the char dev
event handler N times, which is not necessary, as well as buggy: A later
register overwrites the former one, as qemu_chr_add_handlers() will not
chain those handlers, but instead overwrites the old one. So, in theory,
invoking qemu_chr_add_handlers N times will not end up with calling the
handler N times.

However, the reason the handler is invoked N times is because we start
the backend(the connection server) first, and hence when net_vhost_user_init()
is executed, the connection is already established, and qemu_chr_add_handlers()
then invoke the handler immediately, which just looks like we invoke the
handler(net_vhost_user_event) directly from net_vhost_user_init().

The solution I came up with is to make VhostUserState as an upper level
structure, making it includes N nc and vhost_net pairs:

   typedef struct VhostUserNetPair {
   NetClientState *nc;
   VHostNetState  *vhost_net;
   } VhostUserNetPair;

   typedef struct VhostUserState {
   CharDriverState *chr;
   bool running;
   int queues;
   struct VhostUserNetPair pairs[];
   } VhostUserState;

v8: set net->dev.vq_index correctly inside vhost_net_init() based on the
value from net->nc->queue_index.

Signed-off-by: Nikolay Nikolaev 
Signed-off-by: Changchun Ouyang 
Signed-off-by: Yuanhan Liu 
---
 docs/specs/vhost-user.txt |  13 +
 hw/net/vhost_net.c|   5 +-
 hw/virtio/vhost-user.c|  32 ++-
 include/net/net.h |   1 +
 net/vhost-user.c  | 140 +-
 qapi-schema.json  |   6 +-
 qemu-options.hx   |   5 +-
 7 files changed, 157 insertions(+), 45 deletions(-)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 43db9b4..acf5708 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -135,6 +135,19 @@ As older slaves don't support negotiating protocol 
features,
 a feature bit was dedicated for this purpose:
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 
+Multiple queue support
+---
+Multiple queue is treated as a protocol extension, hence the slave has to
+implement protocol features first. Multiple queues is supported only when
+the protocol feature VHOST_USER_PROTOCOL_F_MQ(bit 0) is set.
+
+The max number of queues the slave supports can be queried with message
+VHOST_USER_GET_PROTOCOL_FEATURES. Master should stop when the number of
+requested queues is bigger than that.
+
+As all queues share one connection, the master uses a unique index for each
+queue in the sent message to identify a specified queue.
+
 Message types
 -
 
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index f9441e9..0eda2ed 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -148,6 +148,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
 fprintf(stderr, "vhost-net requires net backend to be setup\n");
 goto fail;
 }
+net->nc = options->net_backend;
 
 net->dev.max_queues = 1;
 
@@ -164,8 +165,10 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
 net->dev.backend_features = 0;

[Qemu-devel] [PATCH 7/7] vhost-user: add a new message to disable/enable a specific virt queue.

[Yuanhan Liu]

From: Changchun Ouyang 

Add a new message, VHOST_USER_SET_VRING_ENABLE, to enable or disable
a specific virt queue, which is similar to attach/detach queue for
tap device.

virtio driver on guest doesn't have to use max virt queue pair, it
could enable any number of virt queue ranging from 1 to max virt
queue pair.

Signed-off-by: Changchun Ouyang 
Signed-off-by: Yuanhan Liu 
---
 docs/specs/vhost-user.txt | 12 +++-
 hw/net/vhost_net.c| 18 ++
 hw/net/virtio-net.c   |  8 
 hw/virtio/vhost-user.c| 19 +++
 include/hw/virtio/vhost-backend.h |  2 ++
 include/net/vhost_net.h   |  2 ++
 6 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index acf5708..9c12875 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -146,7 +146,9 @@ VHOST_USER_GET_PROTOCOL_FEATURES. Master should stop when 
the number of
 requested queues is bigger than that.
 
 As all queues share one connection, the master uses a unique index for each
-queue in the sent message to identify a specified queue.
+queue in the sent message to identify a specified queue. One queue pairs
+is enabled initially. More queues are enabled dynamically, by sending
+message VHOST_USER_SET_VRING_ENABLE.
 
 Message types
 -
@@ -325,3 +327,11 @@ Message types
   Query how many queues the backend supports. This request should be
   sent only when VHOST_USER_PROTOCOL_F_MQ is set in quried protocol
   features by VHOST_USER_GET_PROTOCOL_FEATURES.
+
+ * VHOST_USER_SET_VRING_ENABLE
+
+  Id: 18
+  Equivalent ioctl: N/A
+  Master payload: vring state description
+
+  Signal slave to enable or disable corresponding vring.
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index c114304..b9266d7 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -422,6 +422,19 @@ VHostNetState *get_vhost_net(NetClientState *nc)
 
 return vhost_net;
 }
+
+int vhost_set_vring_enable(NetClientState *nc, int enable)
+{
+VHostNetState *net = get_vhost_net(nc);
+const VhostOps *vhost_ops = net->dev.vhost_ops;
+
+if (vhost_ops->vhost_backend_set_vring_enable) {
+return vhost_ops->vhost_backend_set_vring_enable(&net->dev, enable);
+}
+
+return 0;
+}
+
 #else
 struct vhost_net *vhost_net_init(VhostNetOptions *options)
 {
@@ -467,4 +480,9 @@ VHostNetState *get_vhost_net(NetClientState *nc)
 {
 return 0;
 }
+
+int vhost_set_vring_enable(NetClientState *nc, int enable)
+{
+return 0;
+}
 #endif
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 8d28e45..3d41e22 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -406,6 +406,10 @@ static int peer_attach(VirtIONet *n, int index)
 return 0;
 }
 
+if (nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
+vhost_set_vring_enable(nc->peer, 1);
+}
+
 if (nc->peer->info->type != NET_CLIENT_OPTIONS_KIND_TAP) {
 return 0;
 }
@@ -421,6 +425,10 @@ static int peer_detach(VirtIONet *n, int index)
 return 0;
 }
 
+if (nc->peer->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
+vhost_set_vring_enable(nc->peer, 0);
+}
+
 if (nc->peer->info->type !=  NET_CLIENT_OPTIONS_KIND_TAP) {
 return 0;
 }
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index dfb5a61..d525ab5 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -48,6 +48,7 @@ typedef enum VhostUserRequest {
 VHOST_USER_GET_PROTOCOL_FEATURES = 15,
 VHOST_USER_SET_PROTOCOL_FEATURES = 16,
 VHOST_USER_GET_QUEUE_NUM = 17,
+VHOST_USER_SET_VRING_ENABLE = 18,
 VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -294,6 +295,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned 
long int request,
 
 case VHOST_USER_SET_VRING_NUM:
 case VHOST_USER_SET_VRING_BASE:
+case VHOST_USER_SET_VRING_ENABLE:
 memcpy(&msg.state, arg, sizeof(struct vhost_vring_state));
 msg.size = sizeof(m.state);
 break;
@@ -410,6 +412,22 @@ static int vhost_user_init(struct vhost_dev *dev, void 
*opaque)
 return 0;
 }
 
+static int vhost_user_set_vring_enable(struct vhost_dev *dev, int enable)
+{
+struct vhost_vring_state state = {
+.index = dev->vq_index,
+.num   = enable,
+};
+
+assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
+
+if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ))) {
+return -1;
+}
+
+return vhost_user_call(dev, VHOST_USER_SET_VRING_ENABLE, &state);
+}
+
 static int vhost_user_cleanup(struct vhost_dev *dev)
 {
 assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
@@ -432,4 +450,5 @@ const VhostOps user_ops = {
 .vhost_backend_init = vhost_user_init,
 .vhost_backend_cleanup = vhost_user_cleanup,
 .vhost_backend_get_vq_index = vhost_user_get_v

Re: [Qemu-devel] [PATCH 1/9] shaders: initialize vertexes once

[Gerd Hoffmann]

> > Therefore, I'm not sure whether deleting the buffer is right here. Maybe
> > OpenGL uses reference counting here, too, so it remembers that the
> > buffer is still in use by the VAO, and so the glDeleteBuffers()
> > operation will only decrease its refcount, but not actually end up
> > deleting it.

Hmm, my docs sayed it's refcounted and as long as the vao uses it
everything should be fine ...

> Oh, and I just noticed: With glDeleteBuffers(), I get a segmentation
> fault when qemu exits (somewhere deep in fglrx_dri.so). Without, the
> segfault disappears.

... but appearently it isn't.  Ok, dropping the glDeleteBuffers call.
It isn't freed anyway until qemu exits, so at the end of the day it
doesn't matter much.

cheers,
  Gerd

[Qemu-devel] [PATCH 5/7] vhost: introduce vhost_backend_get_vq_index method

[Yuanhan Liu]

Minusing the idx with the base(dev->vq_index) for vhost-kernel, and
then adding it back for vhost-user doesn't seem right. Here introduces
a new method vhost_backend_get_vq_index() for getting the right vq
index for following vhost messages calls.

Suggested-by: Jason Wang 
Signed-off-by: Yuanhan Liu 
---
 hw/virtio/vhost-backend.c | 10 +-
 hw/virtio/vhost-user.c| 12 ++--
 hw/virtio/vhost.c | 15 ++-
 include/hw/virtio/vhost-backend.h |  2 ++
 4 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 4d68a27..72d1392 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -42,11 +42,19 @@ static int vhost_kernel_cleanup(struct vhost_dev *dev)
 return close(fd);
 }
 
+static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
+{
+assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
+
+return idx - dev->vq_index;
+}
+
 static const VhostOps kernel_ops = {
 .backend_type = VHOST_BACKEND_TYPE_KERNEL,
 .vhost_call = vhost_kernel_call,
 .vhost_backend_init = vhost_kernel_init,
-.vhost_backend_cleanup = vhost_kernel_cleanup
+.vhost_backend_cleanup = vhost_kernel_cleanup,
+.vhost_backend_get_vq_index = vhost_kernel_get_vq_index,
 };
 
 int vhost_set_backend_type(struct vhost_dev *dev, VhostBackendType 
backend_type)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 8b4b36b..69b085b 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -393,9 +393,17 @@ static int vhost_user_cleanup(struct vhost_dev *dev)
 return 0;
 }
 
+static int vhost_user_get_vq_index(struct vhost_dev *dev, int idx)
+{
+assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
+
+return idx;
+}
+
 const VhostOps user_ops = {
 .backend_type = VHOST_BACKEND_TYPE_USER,
 .vhost_call = vhost_user_call,
 .vhost_backend_init = vhost_user_init,
-.vhost_backend_cleanup = vhost_user_cleanup
-};
+.vhost_backend_cleanup = vhost_user_cleanup,
+.vhost_backend_get_vq_index = vhost_user_get_vq_index,
+};
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 2712c6f..717a0ba 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -719,7 +719,7 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
 {
 hwaddr s, l, a;
 int r;
-int vhost_vq_index = idx - dev->vq_index;
+int vhost_vq_index = dev->vhost_ops->vhost_backend_get_vq_index(dev, idx);
 struct vhost_vring_file file = {
 .index = vhost_vq_index
 };
@@ -728,7 +728,6 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
 };
 struct VirtQueue *vvq = virtio_get_queue(vdev, idx);
 
-assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
 
 vq->num = state.num = virtio_queue_get_num(vdev, idx);
 r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_NUM, &state);
@@ -822,12 +821,12 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
 struct vhost_virtqueue *vq,
 unsigned idx)
 {
-int vhost_vq_index = idx - dev->vq_index;
+int vhost_vq_index = dev->vhost_ops->vhost_backend_get_vq_index(dev, idx);
 struct vhost_vring_state state = {
 .index = vhost_vq_index,
 };
 int r;
-assert(idx >= dev->vq_index && idx < dev->vq_index + dev->nvqs);
+
 r = dev->vhost_ops->vhost_call(dev, VHOST_GET_VRING_BASE, &state);
 if (r < 0) {
 fprintf(stderr, "vhost VQ %d ring restore failed: %d\n", idx, r);
@@ -1066,17 +1065,15 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, 
VirtIODevice *vdev, int n,
 {
 struct VirtQueue *vvq = virtio_get_queue(vdev, n);
 int r, index = n - hdev->vq_index;
+struct vhost_vring_file file;
 
-assert(n >= hdev->vq_index && n < hdev->vq_index + hdev->nvqs);
-
-struct vhost_vring_file file = {
-.index = index
-};
 if (mask) {
 file.fd = event_notifier_get_fd(&hdev->vqs[index].masked_notifier);
 } else {
 file.fd = event_notifier_get_fd(virtio_queue_get_guest_notifier(vvq));
 }
+
+file.index = hdev->vhost_ops->vhost_backend_get_vq_index(hdev, n);
 r = hdev->vhost_ops->vhost_call(hdev, VHOST_SET_VRING_CALL, &file);
 assert(r >= 0);
 }
diff --git a/include/hw/virtio/vhost-backend.h 
b/include/hw/virtio/vhost-backend.h
index e472f29..e1dfc6d 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -24,12 +24,14 @@ typedef int (*vhost_call)(struct vhost_dev *dev, unsigned 
long int request,
  void *arg);
 typedef int (*vhost_backend_init)(struct vhost_dev *dev, void *opaque);
 typedef int (*vhost_backend_cleanup)(struct vhost_dev *dev);
+typedef int (*vhost_backend_get_vq_index)(struct vhost_dev *dev, int idx);
 
 typedef struct VhostOps {
 VhostBackendType backen

[Qemu-devel] [PATCH 2/7] vhost-user: add protocol feature negotiation

[Yuanhan Liu]

From: "Michael S. Tsirkin" 

Support a separate bitmask for vhost-user protocol features,
and messages to get/set protocol features.

Invoke them at init.

No features are defined yet.

v2: leverage vhost_user_call for request handling -- Yuanhan Liu

Signed-off-by: Michael S. Tsirkin 
Signed-off-by: Yuanhan Liu 
---
 docs/specs/vhost-user.txt | 37 +
 hw/net/vhost_net.c|  2 ++
 hw/virtio/vhost-user.c| 31 +++
 include/hw/virtio/vhost.h |  1 +
 4 files changed, 71 insertions(+)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 650bb18..70da3b1 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -113,6 +113,7 @@ message replies. Most of the requests don't require 
replies. Here is a list of
 the ones that do:
 
  * VHOST_GET_FEATURES
+ * VHOST_GET_PROTOCOL_FEATURES
  * VHOST_GET_VRING_BASE
 
 There are several messages that the master sends with file descriptors passed
@@ -127,6 +128,13 @@ in the ancillary data:
 If Master is unable to send the full message or receives a wrong reply it will
 close the connection. An optional reconnection mechanism can be implemented.
 
+Any protocol extensions are gated by protocol feature bits,
+which allows full backwards compatibility on both master
+and slave.
+As older slaves don't support negotiating protocol features,
+a feature bit was dedicated for this purpose:
+#define VHOST_USER_F_PROTOCOL_FEATURES 30
+
 Message types
 -
 
@@ -138,6 +146,8 @@ Message types
   Slave payload: u64
 
   Get from the underlying vhost implementation the features bitmask.
+  Feature bit VHOST_USER_F_PROTOCOL_FEATURES signals slave support for
+  VHOST_USER_GET_PROTOCOL_FEATURES and VHOST_USER_SET_PROTOCOL_FEATURES.
 
  * VHOST_USER_SET_FEATURES
 
@@ -146,6 +156,33 @@ Message types
   Master payload: u64
 
   Enable features in the underlying vhost implementation using a bitmask.
+  Feature bit VHOST_USER_F_PROTOCOL_FEATURES signals slave support for
+  VHOST_USER_GET_PROTOCOL_FEATURES and VHOST_USER_SET_PROTOCOL_FEATURES.
+
+ * VHOST_USER_GET_PROTOCOL_FEATURES
+
+  Id: 15
+  Equivalent ioctl: VHOST_GET_FEATURES
+  Master payload: N/A
+  Slave payload: u64
+
+  Get the protocol feature bitmask from the underlying vhost 
implementation.
+  Only legal if feature bit VHOST_USER_F_PROTOCOL_FEATURES is present in
+  VHOST_USER_GET_FEATURES.
+  Note: slave that reported VHOST_USER_F_PROTOCOL_FEATURES must support
+  this message even before VHOST_USER_SET_FEATURES was called.
+
+ * VHOST_USER_SET_PROTOCOL_FEATURES
+
+  Id: 16
+  Ioctl: VHOST_SET_FEATURES
+  Master payload: u64
+
+  Enable protocol features in the underlying vhost implementation.
+  Only legal if feature bit VHOST_USER_F_PROTOCOL_FEATURES is present in
+  VHOST_USER_GET_FEATURES.
+  Note: slave that reported VHOST_USER_F_PROTOCOL_FEATURES must support
+  this message even before VHOST_USER_SET_FEATURES was called.
 
  * VHOST_USER_SET_OWNER
 
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 5c1d11f..c864237 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -152,8 +152,10 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
 net->dev.backend_features = qemu_has_vnet_hdr(options->net_backend)
 ? 0 : (1ULL << VHOST_NET_F_VIRTIO_NET_HDR);
 net->backend = r;
+net->dev.protocol_features = 0;
 } else {
 net->dev.backend_features = 0;
+net->dev.protocol_features = 0;
 net->backend = -1;
 }
 net->nc = options->net_backend;
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 13677ac..43169a1 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -24,6 +24,8 @@
 #include 
 
 #define VHOST_MEMORY_MAX_NREGIONS8
+#define VHOST_USER_F_PROTOCOL_FEATURES 30
+#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x0ULL
 
 typedef enum VhostUserRequest {
 VHOST_USER_NONE = 0,
@@ -41,6 +43,8 @@ typedef enum VhostUserRequest {
 VHOST_USER_SET_VRING_KICK = 12,
 VHOST_USER_SET_VRING_CALL = 13,
 VHOST_USER_SET_VRING_ERR = 14,
+VHOST_USER_GET_PROTOCOL_FEATURES = 15,
+VHOST_USER_SET_PROTOCOL_FEATURES = 16,
 VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -206,11 +210,13 @@ static int vhost_user_call(struct vhost_dev *dev, 
unsigned long int request,
 
 switch (msg_request) {
 case VHOST_USER_GET_FEATURES:
+case VHOST_USER_GET_PROTOCOL_FEATURES:
 need_reply = 1;
 break;
 
 case VHOST_USER_SET_FEATURES:
 case VHOST_USER_SET_LOG_BASE:
+case VHOST_USER_SET_PROTOCOL_FEATURES:
 msg.u64 = *((__u64 *) arg);
 msg.size = sizeof(m.u64);
 break;
@@ -308,6 +314,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned 
long int request,
 
 switch (msg_request) {
 case VHOST_USER_GET_FEATURES:
+case VHOST_US

Re: [Qemu-devel] [PATCH RFC 2/4] arm64: kvm: enable trapping of read access to regs in TID3 group

[Tushar Jagad]

Hi Shannon,

On Tue, Sep 15, 2015 at 12:23:57PM +0800, Shannon Zhao wrote:
>
>
> On 2015/9/9 16:38, Tushar Jagad wrote:
> > This patch modifies the HCR_GUEST_FLAGS to enable trapping of
> > non secure read to registers under the HCR_EL2.TID3 group to EL2.
> >
> > We emulate the accesses to capability registers which list the number of
> > breakpoints, watchpoints, etc. These values are provided by the user when
> > starting the VM. The emulated values are constructed at runtime from the
> > trap handler.
> >
> > Signed-off-by: Tushar Jagad 
> > ---
> >  Documentation/virtual/kvm/api.txt |8 +
> >  arch/arm/kvm/arm.c|   50 -
> >  arch/arm64/include/asm/kvm_arm.h  |2 +-
> >  arch/arm64/include/asm/kvm_asm.h  |   38 +++-
> >  arch/arm64/include/asm/kvm_host.h |4 +-
> >  arch/arm64/include/uapi/asm/kvm.h |7 +
> >  arch/arm64/kvm/sys_regs.c |  443 
> > +
> >  7 files changed, 503 insertions(+), 49 deletions(-)
> >
> > diff --git a/Documentation/virtual/kvm/api.txt 
> > b/Documentation/virtual/kvm/api.txt
> > index a7926a9..b06c104 100644
> > --- a/Documentation/virtual/kvm/api.txt
> > +++ b/Documentation/virtual/kvm/api.txt
> > @@ -2561,6 +2561,14 @@ Possible features:
> >   Depends on KVM_CAP_ARM_EL1_32BIT (arm64 only).
> > - KVM_ARM_VCPU_PSCI_0_2: Emulate PSCI v0.2 for the CPU.
> >   Depends on KVM_CAP_ARM_PSCI_0_2.
> > +   - KVM_ARM_VCPU_NUM_BPTS: Number of supported h/w breakpoints
> > + This is a 4-bit value which defines number of hardware
> > + breakpoints supported on guest. If this is not sepecified or
> > + set to zero then the guest sees the value as is from the host.
> > +   - KVM_ARM_VCPU_NUM_WPTS: Number of supported h/w watchpoints
> > + This is a 4-bit value which defines number of hardware
> > + watchpoints supported on guest. If this is not sepecified or
> > + set to zero then the guest sees the value as is from the host.
> >
> >
> >  4.83 KVM_ARM_PREFERRED_TARGET
> > diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> > index bc738d2..8907d37 100644
> > --- a/arch/arm/kvm/arm.c
> > +++ b/arch/arm/kvm/arm.c
> > @@ -696,6 +696,8 @@ static int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
> >const struct kvm_vcpu_init *init)
> >  {
> > unsigned int i;
> > +   u64 aa64dfr;
> > +
> > int phys_target = kvm_target_cpu();
> >
> > if (init->target != phys_target)
> > @@ -708,6 +710,8 @@ static int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
> > if (vcpu->arch.target != -1 && vcpu->arch.target != init->target)
> > return -EINVAL;
> >
> > +   asm volatile("mrs %0, ID_AA64DFR0_EL1\n" : "=r" (aa64dfr));
> > +
> > /* -ENOENT for unknown features, -EINVAL for invalid combinations. */
> > for (i = 0; i < sizeof(init->features) * 8; i++) {
> > bool set = (init->features[i / 32] & (1 << (i % 32)));
> > @@ -715,6 +719,50 @@ static int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
> > if (set && i >= KVM_VCPU_MAX_FEATURES)
> > return -ENOENT;
> >
> > +   if (i == KVM_ARM_VCPU_NUM_BPTS) {
> > +   int h_bpts;
> > +   int g_bpts;
> > +
> > +   h_bpts = ((aa64dfr >> 12) & 0xf) + 1;
> > +   g_bpts = 
> > (init->features[KVM_ARM_VCPU_BPTS_FEATURES_IDX] &
> > +   KVM_ARM_VCPU_BPTS_MASK) >> 
> > KVM_ARM_VCPU_NUM_BPTS;
> > +
> > +   /*
> > +* We ensure that the host can support the requested
> > +* number of hardware breakpoints.
> > +*/
> > +   if (g_bpts > h_bpts)
> > +   return -EINVAL;
> > +
> This may not work. Assuming that the number of source host hardware
> breakpoints is 15 and userspace set the g_bpts to 15 as well, it's ok to
> create VM on the source host. But if the number of destination host
> hardware breakpoints is lees than 15 (e.g. 8), this will return -EINVAL
> and fail to create VM on the destination host and migrate failed.
>
> (P.S. I'm considering the guest PMU for cross-cpu type, so I have look
> at this patch)

We basically want to avoid migrating a guest to a host which lacks the
necessary support in the hardware. Thus consider a case where in there are
different platforms (with different CPU implementation capabilities) in a
cluster ie. few platforms support 2 h/w breakpoints/watchpoints, some platforms
support 4 h/w breakpoints/watchpoints, etc. In this case the least common
denominator of these implementation details should be considered before
starting a vm. So in the given scenario we will configure all vm's to have 2
h/w breakpoints/watchpoints which will avoid crashing of guest post migration.

For now these patches consider h/w breakpoint and h/w watchpoints but need to
expand to include PMU support.
--
Thanks,
Tushar

>
> > +   vcpu->arch

Re: [Qemu-devel] [PATCH 12/17] target-openrisc: Enable m[tf]spr from user mode

[Bastian Koppelmann]

On 09/14/2015 07:11 PM, Richard Henderson wrote:

On 09/13/2015 01:34 AM, Bastian Koppelmann wrote:

Looking at the article, user mode seems to be optional, so I'm not against it,
but it does look weird. How does ork1sim do it?

It's haphazard.  There are checks for supervisor in the l_mtspr and l_mfspr
insns, but not other uses of the sprs, such as l_rfe.
How about we make the optional user mode a feature bit? This way we are 
closer to ork1sim, but don't have the problem that testing is hindered.


Cheers,
Bastian

Re: [Qemu-devel] [PATCH 5/9] virtio-gpu: add 3d mode and virgl rendering support.

[Gerd Hoffmann]

  Hi,

> > +virtio_gpu_cleanup_mapping_iov(res_iovs, num_iovs);
> 
> Is res_iovs leaked here?

Oops, yes.  Moving the iov release to virtio_gpu_cleanup_mapping_iov (so
things is symmetrical to virtio_gpu_create_mapping_iov which allocates
it), that'll fix it.

> > +pixels = s->current_cursor->width * s->current_cursor->height;
> > +memcpy(s->current_cursor->data, data, pixels * sizeof(uint32_t));
> > +free(data);
> 
> width and height are unused; should they be compared against
> s->current_cursor->{width,height} to spot discrepancies?

Added.

> > +static void virtio_gpu_set_features(VirtIODevice *vdev, uint64_t features)
> > +{
> > +static const uint32_t virgl = (1 << VIRTIO_GPU_FEATURE_VIRGL);
> > +VirtIOGPU *g = VIRTIO_GPU(vdev);
> > +
> > +g->use_virgl_renderer = ((features & virgl) == virgl);
> 
> Could a non-well-behaving guest just set this feature bit even if it was
> not reported by virtio_gpu_get_features() because it has been disabled?

I'm pretty sure virtio core doesn't allow this.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v3 1/5] support nbd driver in blockdev-add

[Markus Armbruster]

Wen Congyang  writes:

> On 09/14/2015 11:47 PM, Eric Blake wrote:
>> On 09/14/2015 08:27 AM, Markus Armbruster wrote:
>>> Wen Congyang  writes:
>>>
 The NBD driver needs: filename, path or (host, port, exportname).
 It checks which key exists and decides use unix or inet socket.
 It doesn't recognize the key type, so we can't use union, and
 can't reuse InetSocketAddress.

 Signed-off-by: Wen Congyang 
 Signed-off-by: zhanghailiang 
 Signed-off-by: Gonglei 
 ---
  qapi/block-core.json | 42 --
  1 file changed, 40 insertions(+), 2 deletions(-)

>> 
  ##
 +# @BlockdevOptionsNBD
 +#
 +# Driver specific block device options for NBD
 +#
 +# @filename: #optional unix or inet path. The format is:
 +#unix: nbd+unix:///export?socket=path or
 +#  nbd:unix:path:exportname=export
 +#inet: nbd[+tcp]://host[:port]/export or
 +#  nbd:host[:port]:exportname=export
>> 
>> Yuck. You are passing structured data through a single 'str', when you
>> SHOULD be passing it through structured JSON.  Just because we have a
>> filename shorthand for convenience does NOT mean that we want to expose
>> that convenience in QMP.  Instead, we really want the breakdown of the
>> pieces (here, using abbreviated syntax of an inline base, since there
>> are pending qapi patches that will allow it):
>
> Do you mean that: there is no need to support filename?

Rule of thumb: if the QMP command handler needs to parse a string
argument, that argument should be a complex QAPI type instead.

Example: @filename needs to be parsed into its components, either

* protocol unix, socket path, export name, or
* protocol tcp, host, port, export name

Since there's an either/or, the complex QAPI type should be a union.

>> { 'enum': 'NBDTransport', 'data': ['unix', 'tcp', 'udp'] }
>
> NBD only uses tcp, it doesn't support udp.
>
>> { 'union': 'BlockdevOptionsNBD',
>>   'base': { 'transport': 'NBDTransport', 'export': 'str' },
>>   'discriminator': 'transport',
>>   'data': { 'unix': 'NBDUnix', 'tcp': 'NBDInet', 'udp': 'NBDInet' } }
>> { 'struct': 'NBDUnix', 'data': { 'socket': 'str' } }
>
> unix socket needs a path, and I think we can use UnixSocketAddress here.

Yes, we should try to reuse common types like SocketAddress,
InetSocketAddress, UnixSocketAddress.

Perhaps it could be as simple as

{ 'struct': 'BlockdevOptionsNBD',
  'data': { 'addr: 'SocketAddress', 'export': 'str' } }

Eric, what do you think?

>> { 'struct': 'NBDInet', 'data': { 'host': 'str', '*port': 'int',
>>  '*ipv4': 'bool', '*ipv6': 'bool' } }
>
> Thanks for the above, and I will try it.

[...]

Re: [Qemu-devel] [PATCH v3 4/5] qmp: add monitor command to add/remove a child

[Markus Armbruster]

Wen Congyang  writes:

> On 09/14/2015 10:36 PM, Markus Armbruster wrote:
>> Wen Congyang  writes:
>> 
>>> Signed-off-by: Wen Congyang 
>>> Signed-off-by: zhanghailiang 
>>> Signed-off-by: Gonglei 
>>> ---
>>>  blockdev.c   | 47 ++
>>>  qapi/block-core.json | 34 +
>>>  qmp-commands.hx  | 53 
>>> 
>>>  3 files changed, 134 insertions(+)
>>>
>>> diff --git a/blockdev.c b/blockdev.c
>>> index bd47756..0a40607 100644
>>> --- a/blockdev.c
>>> +++ b/blockdev.c
>>> @@ -3413,6 +3413,53 @@ fail:
>>>  qmp_output_visitor_cleanup(ov);
>>>  }
>>>  
>>> +void qmp_x_child_add(const char *parent, const char *child,
>>> + Error **errp)
>>> +{
>>> +BlockDriverState *parent_bs, *child_bs;
>>> +Error *local_err = NULL;
>>> +
>>> +parent_bs = bdrv_lookup_bs(parent, parent, &local_err);
>>> +if (!parent_bs) {
>>> +error_propagate(errp, local_err);
>>> +return;
>>> +}
>>> +
>>> +child_bs = bdrv_find_node(child);
>>> +if (!child_bs) {
>>> +error_setg(errp, "Node '%s' not found", child);
>>> +return;
>>> +}
>>> +
>>> +bdrv_add_child(parent_bs, child_bs, &local_err);
>>> +if (local_err) {
>>> +error_propagate(errp, local_err);
>>> +}
>>> +}
>>> +
>>> +void qmp_child_del(const char *parent, const char *child, Error **errp)
>>> +{
>>> +BlockDriverState *parent_bs, *child_bs;
>>> +Error *local_err = NULL;
>>> +
>>> +parent_bs = bdrv_lookup_bs(parent, parent, &local_err);
>>> +if (!parent_bs) {
>>> +error_propagate(errp, local_err);
>>> +return;
>>> +}
>>> +
>>> +child_bs = bdrv_find_node(child);
>>> +if (!child_bs) {
>>> +error_setg(errp, "Node '%s' not found", child);
>>> +return;
>>> +}
>>> +
>>> +bdrv_del_child(parent_bs, child_bs, &local_err);
>>> +if (local_err) {
>>> +error_propagate(errp, local_err);
>>> +}
>>> +}
>>> +
>>>  BlockJobInfoList *qmp_query_block_jobs(Error **errp)
>>>  {
>>>  BlockJobInfoList *head = NULL, **p_next = &head;
>>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>>> index e68a59f..b959577 100644
>>> --- a/qapi/block-core.json
>>> +++ b/qapi/block-core.json
>>> @@ -2272,3 +2272,37 @@
>>>  ##
>>>  { 'command': 'block-set-write-threshold',
>>>'data': { 'node-name': 'str', 'write-threshold': 'uint64' } }
>>> +
>>> +##
>>> +# @x-child-add
>>> +#
>>> +# Add a new child to the parent BDS. Currently only the Quorum driver
>>> +# implements this feature. This is useful to fix a broken quorum child.
>>> +#
>>> +# @parent: graph node name or id which the child will be added to.
>>> +#
>>> +# @child: graph node name that will be added.
>>> +#
>>> +# Note: this command is experimental, and not a stable API.
>>> +#
>>> +# Since: 2.5
>>> +##
>>> +{ 'command': 'x-child-add',
>>> +  'data' : { 'parent': 'str', 'child': 'str' } }
>>> +
>>> +##
>>> +# @child-del
>>> +#
>>> +# Remove a child from the parent BDS. Currently only the Quorum driver
>>> +# implements this feature. This is useful to fix a broken quorum child.
>>> +# Note, you can't remove a child if it would bring the quorum below its
>>> +# threshold.
>>> +#
>>> +# @parent: graph node name or id from which the child will removed.
>>> +#
>>> +# @child: graph node name that will be removed.
>>> +#
>>> +# Since: 2.5
>>> +##
>>> +{ 'command': 'child-del',
>>> +  'data' : { 'parent': 'str', 'child': 'str' } }
>> 
>> Why is x-child-add experimental, but child-del isn't?  Please explain
>> both in the schema and in the commit message.
>
> No special reason. Should I put child-del in experimental namespace?

I found the reason for x-child-add in your v2:

child-add


Add a child to a quorum node.

This command is still a work in progress. It doesn't support all
block drivers. Stay away from it unless you want it to help with
its development.

Eric suggested to rename it to x-child-add, and you did.  Good.  You
also shortened the "work in progress" note to just "Note: this command
is experimental, and not a stable API."  I'd like to have a more verbose
note explaining *why* the command is experimental, both here and in
qmp-commands.hx.  "It doesn't support all block drivers" is a reason.
Are the any others?

Is child-del similarly unfinished?  If yes, make it x-child-del to save
us from later grief.

If no: is child-del is only useful together with x-child-add?  Then make
it x-child-del regardless.

>>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>>> index 495670b..139a23b 100644
>>> --- a/qmp-commands.hx
>>> +++ b/qmp-commands.hx
>>> @@ -4053,6 +4053,59 @@ Example:
>>>  EQMP
>>>  
>>>  {
>>> +.name   = "x-child-add",
>>> +.args_type  = "parent:B,child:B",
>>> +.mhandler.cmd_new = qmp_marshal_input_x_child_add,
>>> +},
>>> +
>>> +SQMP
>>> +x-child-add
>

Re: [Qemu-devel] [PATCH RFC 2/4] arm64: kvm: enable trapping of read access to regs in TID3 group

[Shannon Zhao]

On 2015/9/15 15:18, Tushar Jagad wrote:
> 
> Hi Shannon,
> 
> On Tue, Sep 15, 2015 at 12:23:57PM +0800, Shannon Zhao wrote:
>>
>>
>> On 2015/9/9 16:38, Tushar Jagad wrote:
>>> This patch modifies the HCR_GUEST_FLAGS to enable trapping of
>>> non secure read to registers under the HCR_EL2.TID3 group to EL2.
>>>
>>> We emulate the accesses to capability registers which list the number of
>>> breakpoints, watchpoints, etc. These values are provided by the user when
>>> starting the VM. The emulated values are constructed at runtime from the
>>> trap handler.
>>>
>>> Signed-off-by: Tushar Jagad 
>>> ---
>>>  Documentation/virtual/kvm/api.txt |8 +
>>>  arch/arm/kvm/arm.c|   50 -
>>>  arch/arm64/include/asm/kvm_arm.h  |2 +-
>>>  arch/arm64/include/asm/kvm_asm.h  |   38 +++-
>>>  arch/arm64/include/asm/kvm_host.h |4 +-
>>>  arch/arm64/include/uapi/asm/kvm.h |7 +
>>>  arch/arm64/kvm/sys_regs.c |  443 
>>> +
>>>  7 files changed, 503 insertions(+), 49 deletions(-)
>>>
>>> diff --git a/Documentation/virtual/kvm/api.txt 
>>> b/Documentation/virtual/kvm/api.txt
>>> index a7926a9..b06c104 100644
>>> --- a/Documentation/virtual/kvm/api.txt
>>> +++ b/Documentation/virtual/kvm/api.txt
>>> @@ -2561,6 +2561,14 @@ Possible features:
>>>   Depends on KVM_CAP_ARM_EL1_32BIT (arm64 only).
>>> - KVM_ARM_VCPU_PSCI_0_2: Emulate PSCI v0.2 for the CPU.
>>>   Depends on KVM_CAP_ARM_PSCI_0_2.
>>> +   - KVM_ARM_VCPU_NUM_BPTS: Number of supported h/w breakpoints
>>> + This is a 4-bit value which defines number of hardware
>>> + breakpoints supported on guest. If this is not sepecified or
>>> + set to zero then the guest sees the value as is from the host.
>>> +   - KVM_ARM_VCPU_NUM_WPTS: Number of supported h/w watchpoints
>>> + This is a 4-bit value which defines number of hardware
>>> + watchpoints supported on guest. If this is not sepecified or
>>> + set to zero then the guest sees the value as is from the host.
>>>
>>>
>>>  4.83 KVM_ARM_PREFERRED_TARGET
>>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>>> index bc738d2..8907d37 100644
>>> --- a/arch/arm/kvm/arm.c
>>> +++ b/arch/arm/kvm/arm.c
>>> @@ -696,6 +696,8 @@ static int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
>>>const struct kvm_vcpu_init *init)
>>>  {
>>> unsigned int i;
>>> +   u64 aa64dfr;
>>> +
>>> int phys_target = kvm_target_cpu();
>>>
>>> if (init->target != phys_target)
>>> @@ -708,6 +710,8 @@ static int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
>>> if (vcpu->arch.target != -1 && vcpu->arch.target != init->target)
>>> return -EINVAL;
>>>
>>> +   asm volatile("mrs %0, ID_AA64DFR0_EL1\n" : "=r" (aa64dfr));
>>> +
>>> /* -ENOENT for unknown features, -EINVAL for invalid combinations. */
>>> for (i = 0; i < sizeof(init->features) * 8; i++) {
>>> bool set = (init->features[i / 32] & (1 << (i % 32)));
>>> @@ -715,6 +719,50 @@ static int kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
>>> if (set && i >= KVM_VCPU_MAX_FEATURES)
>>> return -ENOENT;
>>>
>>> +   if (i == KVM_ARM_VCPU_NUM_BPTS) {
>>> +   int h_bpts;
>>> +   int g_bpts;
>>> +
>>> +   h_bpts = ((aa64dfr >> 12) & 0xf) + 1;
>>> +   g_bpts = 
>>> (init->features[KVM_ARM_VCPU_BPTS_FEATURES_IDX] &
>>> +   KVM_ARM_VCPU_BPTS_MASK) >> 
>>> KVM_ARM_VCPU_NUM_BPTS;
>>> +
>>> +   /*
>>> +* We ensure that the host can support the requested
>>> +* number of hardware breakpoints.
>>> +*/
>>> +   if (g_bpts > h_bpts)
>>> +   return -EINVAL;
>>> +
>> This may not work. Assuming that the number of source host hardware
>> breakpoints is 15 and userspace set the g_bpts to 15 as well, it's ok to
>> create VM on the source host. But if the number of destination host
>> hardware breakpoints is lees than 15 (e.g. 8), this will return -EINVAL
>> and fail to create VM on the destination host and migrate failed.
>>
>> (P.S. I'm considering the guest PMU for cross-cpu type, so I have look
>> at this patch)
> 
> We basically want to avoid migrating a guest to a host which lacks the
> necessary support in the hardware. Thus consider a case where in there are
> different platforms (with different CPU implementation capabilities) in a
> cluster ie. few platforms support 2 h/w breakpoints/watchpoints, some 
> platforms
> support 4 h/w breakpoints/watchpoints, etc. In this case the least common
> denominator of these implementation details should be considered before
> starting a vm. So in the given scenario we will configure all vm's to have 2
> h/w breakpoints/watchpoints which will avoid crashing of guest post migration.
> 

Oh, I see. Using the minimum number of all the hardware bpts/wpts.

> For now t

Re: [Qemu-devel] [PATCH 6/9] sdl2/opengl: add opengl context and scanout support

[Gerd Hoffmann]

  Hi,

> > +scon->scanout_mode = scanout;
> > +if (!scon->scanout_mode) {
> > +if (scon->fbo_id) {
> > +glFramebufferTexture2DEXT(GL_FRAMEBUFFER_EXT,
> > +  GL_COLOR_ATTACHMENT0_EXT,
> > +  GL_TEXTURE_2D, 0, 0);
> > +glDeleteFramebuffers(1, &scon->fbo_id);
> 
> I'm not sure, but maybe the framebuffer itself should be unbound here,
> too? (i.e., glBindFramebuffer(GL_FRAMEBUFFER, 0))
> 
> I know it's deleted but I don't know whether that's enough.

Fixed.

> > +glBindFramebuffer(GL_READ_FRAMEBUFFER, scon->fbo_id);
> > +glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0);
> > +
> > +SDL_GetWindowSize(scon->real_window, &ww, &wh);
> > +glViewport(0, 0, ww, wh);
> > +y1 = scon->y0_top ? 0 : scon->h;
> > +y2 = scon->y0_top ? scon->h : 0;
> > +glBlitFramebuffer(0, y1, scon->w, y2,
> > +  0, 0, ww, wh,
> > +  GL_COLOR_BUFFER_BIT, GL_NEAREST);
> 
> Should we bind the FBO back to GL_DRAW_FRAMEBUFFER after the blit operation?

Isn't READ/DRAW used for framebuffer blits only?  Normal rendering goes
to GL_FRAMEBUFFER I think ...

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v3 4/5] qmp: add monitor command to add/remove a child

[Wen Congyang]

On 09/15/2015 03:49 PM, Markus Armbruster wrote:
> Wen Congyang  writes:
> 
>> On 09/14/2015 10:36 PM, Markus Armbruster wrote:
>>> Wen Congyang  writes:
>>>
 Signed-off-by: Wen Congyang 
 Signed-off-by: zhanghailiang 
 Signed-off-by: Gonglei 
 ---
  blockdev.c   | 47 ++
  qapi/block-core.json | 34 +
  qmp-commands.hx  | 53 
 
  3 files changed, 134 insertions(+)

 diff --git a/blockdev.c b/blockdev.c
 index bd47756..0a40607 100644
 --- a/blockdev.c
 +++ b/blockdev.c
 @@ -3413,6 +3413,53 @@ fail:
  qmp_output_visitor_cleanup(ov);
  }
  
 +void qmp_x_child_add(const char *parent, const char *child,
 + Error **errp)
 +{
 +BlockDriverState *parent_bs, *child_bs;
 +Error *local_err = NULL;
 +
 +parent_bs = bdrv_lookup_bs(parent, parent, &local_err);
 +if (!parent_bs) {
 +error_propagate(errp, local_err);
 +return;
 +}
 +
 +child_bs = bdrv_find_node(child);
 +if (!child_bs) {
 +error_setg(errp, "Node '%s' not found", child);
 +return;
 +}
 +
 +bdrv_add_child(parent_bs, child_bs, &local_err);
 +if (local_err) {
 +error_propagate(errp, local_err);
 +}
 +}
 +
 +void qmp_child_del(const char *parent, const char *child, Error **errp)
 +{
 +BlockDriverState *parent_bs, *child_bs;
 +Error *local_err = NULL;
 +
 +parent_bs = bdrv_lookup_bs(parent, parent, &local_err);
 +if (!parent_bs) {
 +error_propagate(errp, local_err);
 +return;
 +}
 +
 +child_bs = bdrv_find_node(child);
 +if (!child_bs) {
 +error_setg(errp, "Node '%s' not found", child);
 +return;
 +}
 +
 +bdrv_del_child(parent_bs, child_bs, &local_err);
 +if (local_err) {
 +error_propagate(errp, local_err);
 +}
 +}
 +
  BlockJobInfoList *qmp_query_block_jobs(Error **errp)
  {
  BlockJobInfoList *head = NULL, **p_next = &head;
 diff --git a/qapi/block-core.json b/qapi/block-core.json
 index e68a59f..b959577 100644
 --- a/qapi/block-core.json
 +++ b/qapi/block-core.json
 @@ -2272,3 +2272,37 @@
  ##
  { 'command': 'block-set-write-threshold',
'data': { 'node-name': 'str', 'write-threshold': 'uint64' } }
 +
 +##
 +# @x-child-add
 +#
 +# Add a new child to the parent BDS. Currently only the Quorum driver
 +# implements this feature. This is useful to fix a broken quorum child.
 +#
 +# @parent: graph node name or id which the child will be added to.
 +#
 +# @child: graph node name that will be added.
 +#
 +# Note: this command is experimental, and not a stable API.
 +#
 +# Since: 2.5
 +##
 +{ 'command': 'x-child-add',
 +  'data' : { 'parent': 'str', 'child': 'str' } }
 +
 +##
 +# @child-del
 +#
 +# Remove a child from the parent BDS. Currently only the Quorum driver
 +# implements this feature. This is useful to fix a broken quorum child.
 +# Note, you can't remove a child if it would bring the quorum below its
 +# threshold.
 +#
 +# @parent: graph node name or id from which the child will removed.
 +#
 +# @child: graph node name that will be removed.
 +#
 +# Since: 2.5
 +##
 +{ 'command': 'child-del',
 +  'data' : { 'parent': 'str', 'child': 'str' } }
>>>
>>> Why is x-child-add experimental, but child-del isn't?  Please explain
>>> both in the schema and in the commit message.
>>
>> No special reason. Should I put child-del in experimental namespace?
> 
> I found the reason for x-child-add in your v2:
> 
> child-add
> 
> 
> Add a child to a quorum node.
> 
> This command is still a work in progress. It doesn't support all
> block drivers. Stay away from it unless you want it to help with
> its development.
> 
> Eric suggested to rename it to x-child-add, and you did.  Good.  You
> also shortened the "work in progress" note to just "Note: this command
> is experimental, and not a stable API."  I'd like to have a more verbose
> note explaining *why* the command is experimental, both here and in
> qmp-commands.hx.  "It doesn't support all block drivers" is a reason.
> Are the any others?

Currently, it only for quorum. But in the future, we can use this command
to do more thing. For example: bs->file, bs->backing_hd, ...

> 
> Is child-del similarly unfinished?  If yes, make it x-child-del to save
> us from later grief.
> 
> If no: is child-del is only useful together with x-child-add?  Then make
> it x-child-del regardless.

child-del is only useful together with x-chil

Re: [Qemu-devel] [PATCH v3 1/5] support nbd driver in blockdev-add

[Wen Congyang]

On 09/15/2015 03:37 PM, Markus Armbruster wrote:
> Wen Congyang  writes:
> 
>> On 09/14/2015 11:47 PM, Eric Blake wrote:
>>> On 09/14/2015 08:27 AM, Markus Armbruster wrote:
 Wen Congyang  writes:

> The NBD driver needs: filename, path or (host, port, exportname).
> It checks which key exists and decides use unix or inet socket.
> It doesn't recognize the key type, so we can't use union, and
> can't reuse InetSocketAddress.
>
> Signed-off-by: Wen Congyang 
> Signed-off-by: zhanghailiang 
> Signed-off-by: Gonglei 
> ---
>  qapi/block-core.json | 42 --
>  1 file changed, 40 insertions(+), 2 deletions(-)
>
>>>
>  ##
> +# @BlockdevOptionsNBD
> +#
> +# Driver specific block device options for NBD
> +#
> +# @filename: #optional unix or inet path. The format is:
> +#unix: nbd+unix:///export?socket=path or
> +#  nbd:unix:path:exportname=export
> +#inet: nbd[+tcp]://host[:port]/export or
> +#  nbd:host[:port]:exportname=export
>>>
>>> Yuck. You are passing structured data through a single 'str', when you
>>> SHOULD be passing it through structured JSON.  Just because we have a
>>> filename shorthand for convenience does NOT mean that we want to expose
>>> that convenience in QMP.  Instead, we really want the breakdown of the
>>> pieces (here, using abbreviated syntax of an inline base, since there
>>> are pending qapi patches that will allow it):
>>
>> Do you mean that: there is no need to support filename?
> 
> Rule of thumb: if the QMP command handler needs to parse a string
> argument, that argument should be a complex QAPI type instead.
> 
> Example: @filename needs to be parsed into its components, either
> 
> * protocol unix, socket path, export name, or
> * protocol tcp, host, port, export name
> 
> Since there's an either/or, the complex QAPI type should be a union.
> 
>>> { 'enum': 'NBDTransport', 'data': ['unix', 'tcp', 'udp'] }
>>
>> NBD only uses tcp, it doesn't support udp.
>>
>>> { 'union': 'BlockdevOptionsNBD',
>>>   'base': { 'transport': 'NBDTransport', 'export': 'str' },
>>>   'discriminator': 'transport',
>>>   'data': { 'unix': 'NBDUnix', 'tcp': 'NBDInet', 'udp': 'NBDInet' } }
>>> { 'struct': 'NBDUnix', 'data': { 'socket': 'str' } }
>>
>> unix socket needs a path, and I think we can use UnixSocketAddress here.
> 
> Yes, we should try to reuse common types like SocketAddress,
> InetSocketAddress, UnixSocketAddress.
> 
> Perhaps it could be as simple as
> 
> { 'struct': 'BlockdevOptionsNBD',
>   'data': { 'addr: 'SocketAddress', 'export': 'str' } }

The problem is that: NBD doesn't use the fd.

Another question is: what key will we see in nbd_open()? "addr.host"
or "host"?

Thanks
Wen Congyang

> 
> Eric, what do you think?
> 
>>> { 'struct': 'NBDInet', 'data': { 'host': 'str', '*port': 'int',
>>>  '*ipv4': 'bool', '*ipv6': 'bool' } }
>>
>> Thanks for the above, and I will try it.
> 
> [...]
> .
> 

Re: [Qemu-devel] [PATCH v3 0/6] qemu: guest agent: implement guest-exec command

[Vasiliy Tolstov]

2015-06-15 10:07 GMT+03:00 Denis V. Lunev :
> _PING_


Any news about this feature? I'm need to run inside guest some scripts
and want non blocking exec, so this feature very useful for me.

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru

Re: [Qemu-devel] [PATCH v3 1/1] atapi: abort transfers with 0 byte limits

[Markus Armbruster]

John Snow  writes:

> We're supposed to abort on transfers like this, unless we fill
> Word 125 of our IDENTIFY data with a default transfer size, which
> we don't currently do.
>
> This is an ATA error, not a SCSI/ATAPI one.
> See ATA8-ACS3 sections 7.17.6.49 or 7.21.5.

Reading... yes, that's what the spec says.

> If we don't do this, QEMU will loop forever trying to transfer
> zero bytes, which isn't particularly useful.

Out of curiosity: which loop?

> Signed-off-by: John Snow 
> ---
>  hw/ide/atapi.c| 32 +++-
>  hw/ide/core.c |  2 +-
>  hw/ide/internal.h |  1 +
>  3 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index 79dd167..747f466 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -1169,20 +1169,28 @@ enum {
>   * 4.1.8)
>   */
>  CHECK_READY = 0x02,
> +
> +/*
> + * Commands flagged with NONDATA do not in any circumstances return
> + * any data via ide_atapi_cmd_reply. These commands are exempt from
> + * the normal byte_count_limit constraints.
> + * See ATA8-ACS3 "7.21.5 Byte Count Limit"

Aside: that section is bizarre even for ATA.

Missing piece: what tells you which commands are to be flagged NONDATA?

> + */
> +NONDATA = 0x04,
>  };
>  
>  static const struct {
>  void (*handler)(IDEState *s, uint8_t *buf);
>  int flags;
>  } atapi_cmd_table[0x100] = {
> -[ 0x00 ] = { cmd_test_unit_ready,   CHECK_READY },
> +[ 0x00 ] = { cmd_test_unit_ready,   CHECK_READY | NONDATA },
>  [ 0x03 ] = { cmd_request_sense, ALLOW_UA },
>  [ 0x12 ] = { cmd_inquiry,   ALLOW_UA },
> -[ 0x1b ] = { cmd_start_stop_unit,   0 }, /* [1] */
> -[ 0x1e ] = { cmd_prevent_allow_medium_removal,  0 },
> +[ 0x1b ] = { cmd_start_stop_unit,   NONDATA }, /* [1] */
> +[ 0x1e ] = { cmd_prevent_allow_medium_removal,  NONDATA },
>  [ 0x25 ] = { cmd_read_cdvd_capacity,CHECK_READY },
>  [ 0x28 ] = { cmd_read, /* (10) */   CHECK_READY },
> -[ 0x2b ] = { cmd_seek,  CHECK_READY },
> +[ 0x2b ] = { cmd_seek,  CHECK_READY | NONDATA },
>  [ 0x43 ] = { cmd_read_toc_pma_atip, CHECK_READY },
>  [ 0x46 ] = { cmd_get_configuration, ALLOW_UA },
>  [ 0x4a ] = { cmd_get_event_status_notification, ALLOW_UA },
> @@ -1190,7 +1198,7 @@ static const struct {
>  [ 0x5a ] = { cmd_mode_sense, /* (10) */ 0 },
>  [ 0xa8 ] = { cmd_read, /* (12) */   CHECK_READY },
>  [ 0xad ] = { cmd_read_dvd_structure,CHECK_READY },
> -[ 0xbb ] = { cmd_set_speed, 0 },
> +[ 0xbb ] = { cmd_set_speed, NONDATA },
>  [ 0xbd ] = { cmd_mechanism_status,  0 },
>  [ 0xbe ] = { cmd_read_cd,   CHECK_READY },
>  /* [1] handler detects and reports not ready condition itself */
> @@ -1251,6 +1259,20 @@ void ide_atapi_cmd(IDEState *s)
>  return;
>  }
>  
> +/* Nondata commands permit the byte_count_limit to be 0.
> + * If this is a data-transferring PIO command and BCL is 0,
> + * we abort at the /ATA/ level, not the ATAPI level.
> + * See ATA8 ACS3 section 7.17.6.49 and 7.21.5 */
> +if (!(atapi_cmd_table[s->io_buffer[0]].flags & NONDATA)) {
> +/* TODO: Check IDENTIFY data word 125 for default BCL (currently 0) 
> */
> +uint16_t byte_count_limit = s->lcyl | (s->hcyl << 8);

You might want to wrap s->lcyl | (s->hcyl << 8) in a helper function
some day.  Not in this patch, though.

> +if (!(byte_count_limit || s->atapi_dma)) {
> +/* TODO: Move abort back into core.c and make static inline 
> again */

Not sure about the inline part, but that's not this patch's to judge.

> +ide_abort_command(s);
> +return;
> +}
> +}
> +

Let's see whether I can slash through the negations here...

This is for a non-NONDATA command (outer conditional).  In other words,
we're expecting data.

Unless either byte_count_limit is non-zero or atapi_dma is true (inner
conditional), we abort the command.  In other words: if byte_count_limit
is non-zero, we'll be PIO-ing some data, so we're good.  If atapi_dma is
true, we'll be DMA-ing some data, so we're good.  Else, no data will be
coming, contradicting our expectation.  The command is invalid, and we
abort.

Correct?

>  /* Execute the command */
>  if (atapi_cmd_table[s->io_buffer[0]].handler) {
>  atapi_cmd_table[s->io_buffer[0]].handler(s, buf);
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 50449ca..28cf535 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -457,7 +457,7 @@ BlockAIOCB *ide_issue_trim(BlockBackend *blk,
>  return &iocb->common;
>  }
>  
> -static inline void ide_abort_command(IDEState *s)
> +void ide_abort

Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Fix issue for checking ptr in different address spaces in TARGET_CMSG_NXTHDR

[Peter Maydell]

On 15 September 2015 at 04:01,   wrote:
> From: Chen Gang 
>
> After fix this issue, qemu can run i386 wine notepad.exe successfully.
> But the initialization performance is not quite well.
>
> Signed-off-by: Chen Gang 
> ---
>  linux-user/syscall.c  | 30 +-
>  linux-user/syscall_defs.h | 20 +---
>  2 files changed, 26 insertions(+), 24 deletions(-)

Isn't this duplication of fixes in this patch currently on-list?

https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00680.html

thanks
-- PMM

Re: [Qemu-devel] [PATCH v11 4/5] cpu/apic: drop icc bus/bridge

[Igor Mammedov]

On Tue, 15 Sep 2015 08:52:34 +0800
Zhu Guihua  wrote:

> 
> On 09/14/2015 09:18 PM, Igor Mammedov wrote:
> > On Wed, 2 Sep 2015 17:36:21 +0800
> > Zhu Guihua  wrote:
> >
> >> From: Chen Fan 
> >>
> >> After CPU hotplug has been converted to BUS-less hot-plug infrastructure,
> >> the only function ICC bus performs is to propagate reset to LAPICs. However
> >> LAPIC could be reset by registering its reset handler after all device are
> >> initialized.
> >> Do so and drop ~200LOC of not needed anymore ICCBus related code.
> > this patch drops about 30LOCs only ...
> >
> >
> > PS:
> > Doesn't apply to current master, please rebase.
> 
> This patch rebased on my another patch
> '[PATCH v3] i386: keep cpu_model field in MachineState uptodate'
> https://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg03375.html
> 
> The patch has been reviewed by Eduardo, but not been merged.
> 
> Should I only rebase on current master?
you could pull dependency in this series, it's only one patch.

> 
> Thanks,
> Zhu
> 
> >> Signed-off-by: Chen Fan 
> >> Signed-off-by: Zhu Guihua 
> >> ---
> >>   hw/i386/pc.c| 19 ---
> >>   hw/i386/pc_piix.c   |  9 +
> >>   hw/i386/pc_q35.c|  9 +
> >>   hw/intc/apic_common.c   |  5 ++---
> >>   include/hw/i386/apic_internal.h |  7 ---
> >>   include/hw/i386/pc.h|  2 +-
> >>   target-i386/cpu.c   |  9 +
> >>   7 files changed, 14 insertions(+), 46 deletions(-)
> >>
> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> >> index 4b4a7f3..4c1d68a 100644
> >> --- a/hw/i386/pc.c
> >> +++ b/hw/i386/pc.c
> >> @@ -59,7 +59,6 @@
> >>   #include "qemu/error-report.h"
> >>   #include "hw/acpi/acpi.h"
> >>   #include "hw/acpi/cpu_hotplug.h"
> >> -#include "hw/cpu/icc_bus.h"
> >>   #include "hw/boards.h"
> >>   #include "hw/pci/pci_host.h"
> >>   #include "acpi-build.h"
> >> @@ -1052,23 +1051,16 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, 
> >> int level)
> >>   }
> >>   
> >>   static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
> >> -  DeviceState *icc_bridge, Error **errp)
> >> +  Error **errp)
> >>   {
> >>   X86CPU *cpu = NULL;
> >>   Error *local_err = NULL;
> >>   
> >> -if (icc_bridge == NULL) {
> >> -error_setg(&local_err, "Invalid icc-bridge value");
> >> -goto out;
> >> -}
> >> -
> >>   cpu = cpu_x86_create(cpu_model, &local_err);
> >>   if (local_err != NULL) {
> >>   goto out;
> >>   }
> >>   
> >> -qdev_set_parent_bus(DEVICE(cpu), qdev_get_child_bus(icc_bridge, 
> >> "icc"));
> >> -
> >>   object_property_set_int(OBJECT(cpu), apic_id, "apic-id", &local_err);
> >>   object_property_set_bool(OBJECT(cpu), true, "realized", &local_err);
> >>   
> >> @@ -1083,7 +1075,6 @@ out:
> >>   
> >>   void pc_hot_add_cpu(const int64_t id, Error **errp)
> >>   {
> >> -DeviceState *icc_bridge;
> >>   MachineState *machine = MACHINE(qdev_get_machine());
> >>   X86CPU *cpu;
> >>   int64_t apic_id = x86_cpu_apic_id_from_index(id);
> >> @@ -1113,9 +1104,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
> >>   return;
> >>   }
> >>   
> >> -icc_bridge = DEVICE(object_resolve_path_type("icc-bridge",
> >> - TYPE_ICC_BRIDGE, NULL));
> >> -cpu = pc_new_cpu(machine->cpu_model, apic_id, icc_bridge, &local_err);
> >> +cpu = pc_new_cpu(machine->cpu_model, apic_id, &local_err);
> >>   if (local_err) {
> >>   error_propagate(errp, local_err);
> >>   return;
> >> @@ -1123,7 +1112,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
> >>   object_unref(OBJECT(cpu));
> >>   }
> >>   
> >> -void pc_cpus_init(PCMachineState *pcms, DeviceState *icc_bridge)
> >> +void pc_cpus_init(PCMachineState *pcms)
> >>   {
> >>   int i;
> >>   X86CPU *cpu = NULL;
> >> @@ -1149,7 +1138,7 @@ void pc_cpus_init(PCMachineState *pcms, DeviceState 
> >> *icc_bridge)
> >>   
> >>   for (i = 0; i < smp_cpus; i++) {
> >>   cpu = pc_new_cpu(machine->cpu_model, 
> >> x86_cpu_apic_id_from_index(i),
> >> - icc_bridge, &error);
> >> + &error);
> >>   if (error) {
> >>   error_report_err(error);
> >>   exit(1);
> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> >> index fd6130d..3a97826 100644
> >> --- a/hw/i386/pc_piix.c
> >> +++ b/hw/i386/pc_piix.c
> >> @@ -39,7 +39,6 @@
> >>   #include "hw/kvm/clock.h"
> >>   #include "sysemu/sysemu.h"
> >>   #include "hw/sysbus.h"
> >> -#include "hw/cpu/icc_bus.h"
> >>   #include "sysemu/arch_init.h"
> >>   #include "sysemu/block-backend.h"
> >>   #include "hw/i2c/smbus.h"
> >> @@ -96,7 +95,6 @@ static void pc_init1(MachineState *machine)
> >>   MemoryRegion *ram_memory;
> >>   MemoryRegion *pci_memory;
> >>   MemoryRegion *rom_memory

Re: [Qemu-devel] [PATCH] ide: fix ATAPI command permissions

[Kevin Wolf]

Am 14.09.2015 um 20:49 hat John Snow geschrieben:
> On 09/14/2015 02:43 PM, Michael Tokarev wrote:
> > 14.09.2015 21:04, John Snow wrote:
> >> On 09/11/2015 02:56 AM, Michael Tokarev wrote:
> >>> 09.09.2015 19:28, John Snow wrote:
>  We're a little too lenient with what we'll let an ATAPI drive handle.
>  Clamp down on the IDE command execution table to remove CD_OK permissions
>  from commands that are not and have never been ATAPI commands.
> >>>
> >>> FWIW, this issue has been assigned CVE-2015-6855 identifier, which
> >>> can be reflected in the commit message when applying.  Since this
> >>> issue has security impact, it might be a good idea to add
> >>>
> >>> Cc: qemu-sta...@nongnu.org
> >>
> >> I'm still awaiting review/acks, but would you like me to re-send this
> >> patch to trivial, or just fwd/reply-to?
> > 
> > I think it is anything but trivial ;)  Well, the semantics is trivial,
> > but it isn't -trivial material per se.
> > 
> > I suggested add a Cc to qemu-stable, this can be done at commit,
> > together with mentioning the CVE# assigned meanwhile, and I don't
> > know who will commit it, Kevin?
> > 
> > Thanks,
> > 
> > /mjt
> > 
> 
> I'll be sending the pullreq through my tree, but I was waiting on at
> least an ACK or so before I went ahead.
> 
> I can add CC: qemu-stable to the patch on-pull if that's sufficient for you.

Ideally, stable patches should have a "Cc: qemu-sta...@nongnu.org" line
in the commit message, too. Just adding that on pull should be enough.

Also copying the list for this reply so that there is some trace of the
patch on it.

Kevin

Re: [Qemu-devel] [PATCH v3 0/6] qemu: guest agent: implement guest-exec command

[Vasiliy Tolstov]

2015-09-15 11:15 GMT+03:00 Denis V. Lunev :
> we have discussed new approach on KVM forum
> with a bit reduced set (guest-pipe-open/close
> code should not be in the first version) and in
> progress with a rework. I think that I'll post new version
> at the end of this week or next week.


Ok, i'm wait.

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru

Re: [Qemu-devel] [PATCH 3/9] ui/console: add opengl context and scanout support interfaces.

[Paolo Bonzini]

On 09/09/2015 13:20, Gerd Hoffmann wrote:
> +typedef void *qemu_gl_context;
> +
> +struct qemu_gl_params {
> +int major_ver;
> +int minor_ver;
> +};
> +

Please use QEMUGLContext and QEMUGLParams (with a typedef for the latter).

Paolo

Re: [Qemu-devel] [PATCH v3 0/6] qemu: guest agent: implement guest-exec command

[Denis V. Lunev]

On 09/15/2015 11:02 AM, Vasiliy Tolstov wrote:

2015-06-15 10:07 GMT+03:00 Denis V. Lunev :

_PING_


Any news about this feature? I'm need to run inside guest some scripts
and want non blocking exec, so this feature very useful for me.


we have discussed new approach on KVM forum
with a bit reduced set (guest-pipe-open/close
code should not be in the first version) and in
progress with a rework. I think that I'll post new version
at the end of this week or next week.

Den

Re: [Qemu-devel] [PATCH 5/9] virtio-gpu: add 3d mode and virgl rendering support.

[Paolo Bonzini]

On 09/09/2015 13:20, Gerd Hoffmann wrote:
> + * This work is licensed under the terms of the GNU GPL, version 2.
> + * See the COPYING file in the top-level directory.
> + */

GPLv2-only contributions are not allowed anymore, please upgrade this
file and hw/display/virtio-gpu*.c to v2+.

Paolo

[Qemu-devel] [PATCH] pci-assign: do not include sys/io.h

[Paolo Bonzini]

This file does not exist on bionic libc and the functions it defines
are in fact not used by pci-assign.c.  Remove it.

Reported-by: Houcheng Lin 
Signed-off-by: Paolo Bonzini 
---
 hw/i386/kvm/pci-assign.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 57f14f1..3df8e6c 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -22,7 +22,6 @@
  */
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
-- 
2.5.0

Re: [Qemu-devel] [PATCH v3 4/5] qmp: add monitor command to add/remove a child

[Alberto Garcia]

On Mon 14 Sep 2015 05:37:00 PM CEST, Kevin Wolf  wrote:

>> +{ 'command': 'x-child-add',
>> +  'data' : { 'parent': 'str', 'child': 'str' } }
>
> This is probably not future-proof and only made for the special case
> of quorum. Specifically, one thing I'm missing is some way to specfiy
> what kind of child the new node is when a node can take different
> types of children (e.g. bs->file and bs->backing_hd).

Children here are specified by child roles, aren't they?

We could have a ChildRole enum with 'file', 'format' and 'backing' that
would be passed to this command and use the same enumeration in
bdrv_open_image() rather than a pointer to BdrvChildRole.

Berto

Re: [Qemu-devel] [PATCH] ppc/spapr: Allow VIRTIO_VGA

[Gerd Hoffmann]

On Di, 2015-09-15 at 15:51 +1000, Benjamin Herrenschmidt wrote:
> It works fine with the Linux driver out of the box

Do you actually want the vga compatibility bits on pseries?

There also is virtio-gpu-pci (same thing as virtio-vga but without vga
compatibility), which should already be enabled on ppc64.

The linux kvm driver certainly doesn't need vga compatibility.

For slof support it might be useful though, the vga compatibility bits
in virtio-vga are fully compatible to stdvga, so slof support should be
as simple as adding a PCI ID ...

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v3 4/5] qmp: add monitor command to add/remove a child

[Kevin Wolf]

Am 15.09.2015 um 10:56 hat Alberto Garcia geschrieben:
> On Mon 14 Sep 2015 05:37:00 PM CEST, Kevin Wolf  wrote:
> 
> >> +{ 'command': 'x-child-add',
> >> +  'data' : { 'parent': 'str', 'child': 'str' } }
> >
> > This is probably not future-proof and only made for the special case
> > of quorum. Specifically, one thing I'm missing is some way to specfiy
> > what kind of child the new node is when a node can take different
> > types of children (e.g. bs->file and bs->backing_hd).
> 
> Children here are specified by child roles, aren't they?

Possibly, but actually, currently I'm inclined to think that they
aren't. Child roles are still relatively new, though, so it's hard to
say if this is just because we didn't use them in that way so far, or
because they are really the wrong tool.

What I can say is that traditionally children are identified by option
names. Block drivers assign a ChildRole when processing the option, and
both are equivalent only if the same ChildRole is never used for two
different children. I believe that that's currently true, but I'm
doubtful whether it will remain this way (even looking at blkverify,
which has one &child_file and one &child_format, things start to look a
bit confusing).

Possibly the right and consistent way to change the set of children
would be through a QMP command exposing bdrv_reopen(), which would also
be used for changing other options at runtime.

My current pull request adds the qemu-io (and therefore HMP) way of
doing this, but finding a good QMP interface will still be a challenge.

> We could have a ChildRole enum with 'file', 'format' and 'backing' that
> would be passed to this command and use the same enumeration in
> bdrv_open_image() rather than a pointer to BdrvChildRole.

Yes, that would be an option if ChildRole turned out to be enough.

Kevin

Re: [Qemu-devel] [Question] QEMU 2.3 Assertion with `existing->mr->subpage || existing->mr == &io_mem_unassigned' failed

[Gonglei]

On 2015/9/15 14:33, Gonglei wrote:
> On 2015/9/15 9:16, Gonglei wrote:
>> On 2015/9/14 17:28, Paolo Bonzini wrote:
>>>
>>>
>>> On 14/09/2015 10:01, Gonglei (Arei) wrote:
 [2015-09-11 13:42:44] domain is rebooting 
 qemu-kvm: /home/abuild/rpmbuild/BUILD/qemu-kvm-2.3.0/exec.c:1188: 
 register_subpage: Assertion `existing->mr->subpage || existing->mr == 
 &io_mem_unassigned' failed. 
 [2015-09-11 13:42:58]: shutting down

 Or
 qemu-kvm: 
 /home/abuild/rpmbuild/BUILD/qemu-kvm-2.3.0/include/qemu/int128.h:22: 
 int128_get64: Assertion `!a.hi' failed.
>>>
>>> You need to provide a backtrace.
>>>
>>> Paolo
>>>
>> Yup, I noticed that, but when I sent this email yesterday, I didn't get a 
>> backtrace :(
>> Fortunately,   I get a backtrace now:
>>
>> #0 int128_get64 (a=...) at /home/qemu/include/qemu/int128.h:27
>> #1 0x7f17ad7a7f1a in register_multipage (d=0x7f179c4f8480, 
>> section=0x7f17a323c3f0) at /home/qemu/exec.c:1215
>> #2 0x7f17ad7a8266 in mem_add (listener=0x7f17ae043968 
>> , section=0x7f17a323c730) at /home/qemu/exec.c:1250
>> #3 0x7f17ad7f843a in address_space_update_topology_pass 
>> (as=0x7f17ae043920 , old_view=0x7f179c1f8b50, 
>> new_view=0x7f179c523620, adding=true)
>> at /home/qemu/memory.c:739
>> #4 0x7f17ad7f8520 in address_space_update_topology (as=0x7f17ae043920 
>> ) at /home/qemu/memory.c:754
>> #5 0x7f17ad7f8660 in memory_region_transaction_commit () at 
>> /home/qemu/memory.c:794
>> #6 0x7f17ad9a690c in cirrus_update_memory_access (s=0x7f17b12873c0) at 
>> hw/display/cirrus_vga.c:2485
>> #7 0x7f17ad9a4dac in cirrus_vga_write_gr (s=0x7f17b12873c0, reg_index=9, 
>> reg_value=163) at hw/display/cirrus_vga.c:1524
>> #8 0x7f17ad9a6e47 in cirrus_vga_ioport_write (opaque=0x7f17b12873c0, 
>> addr=975, val=163, size=1) at hw/display/cirrus_vga.c:2672
>> #9 0x7f17ad7f6882 in memory_region_write_accessor (mr=0x7f17b1297d88, 
>> addr=31, value=0x7f17a323c968, size=1, shift=8, mask=255) at 
>> /home/qemu/memory.c:430
>> #10 0x7f17ad7f698b in access_with_adjusted_size (addr=30, 
>> value=0x7f17a323c968, size=2, access_size_min=1, access_size_max=1, 
>> access=0x7f17ad7f67fd , mr=0x7f17b1297d88)
>> at /home/qemu/memory.c:467
>> #11 0x7f17ad7f9311 in memory_region_dispatch_write (mr=0x7f17b1297d88, 
>> addr=30, data=41737, size=2) at /home/qemu/memory.c:1103
>> #12 0x7f17ad7fc22e in io_mem_write (mr=0x7f17b1297d88, addr=30, 
>> val=41737, size=2) at /home/qemu/memory.c:2003
>> #13 0x7f17ad7aafe4 in address_space_rw (as=0x7f17ae043920 
>> , addr=974, buf=0x7f17ad6f6000 "\t\243\320", len=2, 
>> is_write=true) at /home/qemu/exec.c:2533
>> #14 0x7f17ad7f3acf in kvm_handle_io (port=974, data=0x7f17ad6f6000, 
>> direction=1, size=2, count=1) at /home/qemu/kvm-all.c:1707
>> #15 0x7f17ad7f3fb5 in kvm_cpu_exec (cpu=0x7f17b05b7a20) at 
>> /home/qemu/kvm-all.c:1864
>> #16 0x7f17ad7db416 in qemu_kvm_cpu_thread_fn (arg=0x7f17b05b7a20) at 
>> /home/qemu/cpus.c:972
>> #17 0x7f17ac2cbdf5 in start_thread () from /lib64/libpthread.so.0
>> #18 0x7f17a73e31ad in clone () from /lib64/libc.so.6
>>
>> It seems that something wrong happened in vga memory updating.
>>
> 
> Another backtrace:
> 
> (gdb) bt
> #0 int128_get64 (a=...) at /home/qemu/include/qemu/int128.h:27
> #1 0x7f4cdefc1f6a in register_multipage (d=0x7f4cd012f1c0, 
> section=0x7f4cd4a562c0) at /home/qemu/exec.c:1215
> #2 0x7f4cdefc22b6 in mem_add (listener=0x7f4cdf85d968 
> , section=0x7f4cd4a56600) at /home/qemu/exec.c:1250
> #3 0x7f4cdf01248a in address_space_update_topology_pass 
> (as=0x7f4cdf85d920 , old_view=0x7f4cd0028d40, 
> new_view=0x7f4cd015f5f0, adding=true)
> at /home/qemu/memory.c:739
> #4 0x7f4cdf012570 in address_space_update_topology (as=0x7f4cdf85d920 
> ) at /home/qemu/memory.c:754
> #5 0x7f4cdf0126b0 in memory_region_transaction_commit () at 
> /home/qemu/memory.c:794
> #6 0x7f4cdf0151f0 in memory_region_del_subregion (mr=0x7f4ce01034e0, 
> subregion=0x7f4ce13873a0) at /home/qemu/memory.c:1698
> #7 0x7f4cdf21761d in pci_update_mappings (d=0x7f4ce1386f70) at 
> hw/pci/pci.c:1120
> #8 0x7f4cdf2179b0 in pci_default_write_config (d=0x7f4ce1386f70, addr=4, 
> val_in=256, l=2) at hw/pci/pci.c:1180
> #9 0x7f4cdf28d2d6 in virtio_write_config (pci_dev=0x7f4ce1386f70, 
> address=4, val=256, len=2) at hw/virtio/virtio-pci.c:430
> #10 0x7f4cdf220746 in pci_host_config_write_common 
> (pci_dev=0x7f4ce1386f70, addr=4, limit=256, val=256, len=2) at 
> hw/pci/pci_host.c:57
> #11 0x7f4cdf22084a in pci_data_write (s=0x7f4ce008afc0, addr=2147489796, 
> val=256, len=2) at hw/pci/pci_host.c:84
> #12 0x7f4cdf22096c in pci_host_data_write (opaque=0x7f4ce00896b0, addr=0, 
> val=256, len=2) at hw/pci/pci_host.c:137
> #13 0x7f4cdf0108d2 in memory_region_write_accessor (mr=0x7f4ce0089ab0, 
> addr=0, value=0x7f4cd4a56968, size=2, shift=0, mask=65535) at 
> /home/qemu/memory.c:430
> #14 0x7f4cdf0109db in acc

[Qemu-devel] [PATCH 7/9] ioapic_internal.h: added more constants

[Denis V. Lunev]

From: Pavel Butsykin 

Added the masks for easy  access to fields of the redirection table entry

Signed-off-by: Pavel Butsykin 
Signed-off-by: Denis V. Lunev 
CC: Andreas Färber 
CC: Paolo Bonzini 
---
 include/hw/i386/ioapic_internal.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/hw/i386/ioapic_internal.h 
b/include/hw/i386/ioapic_internal.h
index 3be3352..4f7764e 100644
--- a/include/hw/i386/ioapic_internal.h
+++ b/include/hw/i386/ioapic_internal.h
@@ -40,7 +40,12 @@
 #define IOAPIC_LVT_DELIV_MODE_SHIFT 8
 
 #define IOAPIC_LVT_MASKED   (1 << IOAPIC_LVT_MASKED_SHIFT)
+#define IOAPIC_LVT_TRIGGER_MODE (1 << IOAPIC_LVT_TRIGGER_MODE_SHIFT)
 #define IOAPIC_LVT_REMOTE_IRR   (1 << IOAPIC_LVT_REMOTE_IRR_SHIFT)
+#define IOAPIC_LVT_POLARITY (1 << IOAPIC_LVT_POLARITY_SHIFT)
+#define IOAPIC_LVT_DELIV_STATUS (1 << IOAPIC_LVT_DELIV_STATUS_SHIFT)
+#define IOAPIC_LVT_DEST_MODE(1 << IOAPIC_LVT_DEST_MODE_SHIFT)
+#define IOAPIC_LVT_DELIV_MODE   (7 << IOAPIC_LVT_DELIV_MODE_SHIFT)
 
 #define IOAPIC_TRIGGER_EDGE 0
 #define IOAPIC_TRIGGER_LEVEL1
-- 
2.1.4

[Qemu-devel] [PATCH 6/9] hmp: added local apic dump state

[Denis V. Lunev]

From: Pavel Butsykin 

Added the hmp command to query local apic registers state, may be
usefull after guest crashes to understand IRQ routing in guest.

For command name uses "apic-local" because it has to be grouped with
command "apic-io".

(qemu) info apic-local
apic.lvt00-timer   000300fd int=fd .H.EMP delmod=0:Fixed
apic.lvt00-thermal 0001 int=00 .H.EM. delmod=0:Fixed
apic.lvt00-perfmon 00fe int=fe .H.E.. delmod=0:Fixed
apic.lvt00-LINT0   0001001f int=1f .H.EM. delmod=0:Fixed
apic.lvt00-LINT1   04ff int=ff .H.E.. delmod=4:NMI
apic.lvt00-Error   00e3 int=e3 .H.E.. delmod=0:Fixed
apic.error  00 esr  S:... R:... .
apic.timer  00 DCR=000b(b) initial_count=19
apic.icr00 020c00d1: int=d1 delmod=0:Fixed P..E
shorthand=3:all dest=2
apic.prio   00 apr=00(0:0)  tpr=40(4:0)
apic.dest   00 dfr=f0(f)ldr=01(01)
apic.svr00 011f vec=1f on focus=off
apic.interrupt  00 065:R.E

Signed-off-by: Pavel Butsykin 
Signed-off-by: Denis V. Lunev 
CC: Andreas Färber 
CC: Paolo Bonzini 
---
 hmp-commands-info.hx |  16 
 include/monitor/monitor-common.h |   1 +
 target-i386/cpu.h|   3 +
 target-i386/helper.c | 155 +++
 target-i386/monitor.c|   6 ++
 5 files changed, 181 insertions(+)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 9f5a158..5ffc181 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -112,6 +112,22 @@ STEXI
 Show the cpu registers.
 ETEXI
 
+#if defined(TARGET_I386)
+{
+.name   = "apic-local",
+.args_type  = "",
+.params = "",
+.help   = "show local apic state",
+.mhandler.cmd = hmp_info_apic_local,
+},
+#endif
+
+STEXI
+@item info apic-local
+@findex apic-local
+Show local APIC state
+ETEXI
+
 {
 .name   = "cpus",
 .args_type  = "",
diff --git a/include/monitor/monitor-common.h b/include/monitor/monitor-common.h
index abd7a6c..462c35e 100644
--- a/include/monitor/monitor-common.h
+++ b/include/monitor/monitor-common.h
@@ -42,5 +42,6 @@ CPUState *mon_get_cpu(void);
 void hmp_info_mem(Monitor *mon, const QDict *qdict);
 void hmp_info_tlb(Monitor *mon, const QDict *qdict);
 void hmp_mce(Monitor *mon, const QDict *qdict);
+void hmp_info_apic_local(Monitor *mon, const QDict *qdict);
 
 #endif /* MONITOR_COMMON */
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index af97772..f37a9c6 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1347,4 +1347,7 @@ void enable_compat_apic_id_mode(void);
 #define APIC_DEFAULT_ADDRESS 0xfee0
 #define APIC_SPACE_SIZE  0x10
 
+void x86_cpu_dump_apic_local_state(CPUState *cs, FILE *f,
+   fprintf_function cpu_fprintf, int flags);
+
 #endif /* CPU_I386_H */
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 5480a96..8d883f5 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -23,6 +23,7 @@
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/sysemu.h"
 #include "monitor/monitor.h"
+#include "hw/i386/apic_internal.h"
 #endif
 
 static void cpu_x86_version(CPUX86State *env, int *family, int *model)
@@ -177,6 +178,160 @@ done:
 cpu_fprintf(f, "\n");
 }
 
+#ifndef CONFIG_USER_ONLY
+
+/* ARRAY_SIZE check is not required because
+ * DeliveryMode(dm) has a size of 3 bit.
+ */
+static inline const char *dm2str(uint32_t dm)
+{
+static const char *str[] = {
+"Fixed",
+"...",
+"SMI",
+"...",
+"NMI",
+"INIT",
+"...",
+"ExtINT"
+};
+return str[dm];
+}
+
+static void dump_apic_lvt(FILE *f, fprintf_function cpu_fprintf,
+  uint32_t cpu_idx, const char *name,
+  uint32_t lvt, bool is_timer)
+{
+uint32_t dm = (lvt & APIC_LVT_DELIV_MOD) >> APIC_LVT_DELIV_MOD_SHIFT;
+cpu_fprintf(f,
+"apic.lvt\t%02u-%-7s %08x int=%02x %c%c%c%c%c%c 
delmod=%x:%s\n",
+cpu_idx, name, lvt,
+lvt & APIC_VECTOR_MASK,
+lvt & APIC_LVT_DELIV_STS ? 'P' : '.',
+lvt & APIC_LVT_INT_POLARITY ? 'L' : 'H',
+lvt & APIC_LVT_REMOTE_IRR ? 'R' : '.',
+lvt & APIC_LVT_LEVEL_TRIGGER ? 'L' : 'E',
+lvt & APIC_LVT_MASKED ? 'M' : '.',
+!is_timer ? '.' : (lvt & APIC_LVT_TIMER_PERIODIC ? 'P' : 'S'),
+dm,
+dm2str(dm));
+
+}
+
+/* ARRAY_SIZE check is not required because
+ * destination shorthand has a size of 2 bit.
+ */
+static inline const char *shorthand2str(uint32_t shorthand)
+{
+const char *str[] = {
+"no", "self", "all-self", "all"
+};
+return str[shorthand];
+}
+
+void x86_cpu_dump_apic_local_state(CPUState *cs, FILE *f,
+   fprintf_function cpu_fprintf, int flags)
+{
+X86CPU *cpu = X86_CPU(cs);
+APICCommonState *s = 

[Qemu-devel] [PATCH 4/9] apic_internal.h: fix formatting and drop unused consts

[Denis V. Lunev]

From: Pavel Butsykin 

Fix formatting of local apic definitions and drop unused constant
APIC_INPUT_POLARITY, APIC_SEND_PENDING. Magic numbers in shifts are
replaced with constants defined just above.

Signed-off-by: Pavel Butsykin 
Signed-off-by: Denis V. Lunev 
CC: Andreas Färber 
CC: Paolo Bonzini 
---
 include/hw/i386/apic_internal.h | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index 3a3f8fc..36f6c08 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -61,12 +61,10 @@
 #define APIC_LVT_DELIV_STS_SHIFT12
 #define APIC_LVT_DELIV_MOD_SHIFT8
 
-#define APIC_LVT_TIMER_PERIODIC (1<<17)
-#define APIC_LVT_MASKED (1<<16)
-#define APIC_LVT_LEVEL_TRIGGER  (1<<15)
-#define APIC_LVT_REMOTE_IRR (1<<14)
-#define APIC_INPUT_POLARITY (1<<13)
-#define APIC_SEND_PENDING   (1<<12)
+#define APIC_LVT_TIMER_PERIODIC (1 << APIC_LVT_TIMER_SHIFT)
+#define APIC_LVT_MASKED (1 << APIC_LVT_MASKED_SHIFT)
+#define APIC_LVT_LEVEL_TRIGGER  (1 << APIC_LVT_LEVEL_TRIGGER_SHIFT)
+#define APIC_LVT_REMOTE_IRR (1 << APIC_LVT_REMOTE_IRR_SHIFT)
 #define APIC_LVT_INT_POLARITY   (1 << APIC_LVT_INT_POLARITY_SHIFT)
 #define APIC_LVT_DELIV_STS  (1 << APIC_LVT_DELIV_STS_SHIFT)
 #define APIC_LVT_DELIV_MOD  (7 << APIC_LVT_DELIV_MOD_SHIFT)
@@ -78,7 +76,7 @@
 #define APIC_ESR_SEND_ACCEPT_SHIFT  2
 #define APIC_ESR_RECV_CHECK_SUM_SHIFT   1
 
-#define APIC_ESR_ILLEGAL_ADDRESS(1 << 7)
+#define APIC_ESR_ILLEGAL_ADDRESS(1 << APIC_ESR_ILL_ADDRESS_SHIFT)
 #define APIC_ESR_RECV_ILLEGAL_VECT  (1 << APIC_ESR_RECV_ILL_VECT_SHIFT)
 #define APIC_ESR_SEND_ILLEGAL_VECT  (1 << APIC_ESR_SEND_ILL_VECT_SHIFT)
 #define APIC_ESR_RECV_ACCEPT(1 << APIC_ESR_RECV_ACCEPT_SHIFT)
@@ -113,8 +111,8 @@
 #define APIC_SPURIO_FOCUS   (1 << APIC_SPURIO_FOCUS_SHIFT)
 #define APIC_SPURIO_ENABLED (1 << APIC_SPURIO_ENABLED_SHIFT)
 
-#define APIC_SV_DIRECTED_IO (1<<12)
-#define APIC_SV_ENABLE  (1<<8)
+#define APIC_SV_DIRECTED_IO (1 << 12)
+#define APIC_SV_ENABLE  (1 << 8)
 
 #define VAPIC_ENABLE_BIT0
 #define VAPIC_ENABLE_MASK   (1 << VAPIC_ENABLE_BIT)
-- 
2.1.4

[Qemu-devel] [PATCH 1/9] apic_internal.h: move apic_get_bit(), apic_set_bit() to apic_internal.h

[Denis V. Lunev]

From: Pavel Butsykin 

It's necessary to work with bitmap isr, tmr, irr outside hw/intc/apic.c

Signed-off-by: Pavel Butsykin 
Signed-off-by: Denis V. Lunev 
CC: Andreas Färber 
CC: Paolo Bonzini 
---
 hw/intc/apic.c  | 16 
 include/hw/i386/apic_internal.h | 16 
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 77b639c..e4ee032 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -51,14 +51,6 @@ static int apic_ffs_bit(uint32_t value)
 return ctz32(value);
 }
 
-static inline void apic_set_bit(uint32_t *tab, int index)
-{
-int i, mask;
-i = index >> 5;
-mask = 1 << (index & 0x1f);
-tab[i] |= mask;
-}
-
 static inline void apic_reset_bit(uint32_t *tab, int index)
 {
 int i, mask;
@@ -67,14 +59,6 @@ static inline void apic_reset_bit(uint32_t *tab, int index)
 tab[i] &= ~mask;
 }
 
-static inline int apic_get_bit(uint32_t *tab, int index)
-{
-int i, mask;
-i = index >> 5;
-mask = 1 << (index & 0x1f);
-return !!(tab[i] & mask);
-}
-
 /* return -1 if no bit is set */
 static int get_highest_priority_int(uint32_t *tab)
 {
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index 26632ac..6a40156 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -147,4 +147,20 @@ void apic_enable_vapic(DeviceState *d, hwaddr paddr);
 void vapic_report_tpr_access(DeviceState *dev, CPUState *cpu, target_ulong ip,
  TPRAccess access);
 
+static inline void apic_set_bit(uint32_t *tab, int index)
+{
+int i, mask;
+i = index >> 5;
+mask = 1 << (index & 0x1f);
+tab[i] |= mask;
+}
+
+static inline int apic_get_bit(uint32_t *tab, int index)
+{
+int i, mask;
+i = index >> 5;
+mask = 1 << (index & 0x1f);
+return !!(tab[i] & mask);
+}
+
 #endif /* !QEMU_APIC_INTERNAL_H */
-- 
2.1.4

[Qemu-devel] [PATCH 2/9] apic_internal.h: rename ESR_ILLEGAL_ADDRESS to APIC_ESR_ILLEGAL_ADDRESS

[Denis V. Lunev]

From: Pavel Butsykin 

Added prefix APIC_ for determining the constant of a particular subsystem,
improve the overall readability and match other constant names.

Signed-off-by: Pavel Butsykin 
Signed-off-by: Denis V. Lunev 
CC: Andreas Färber 
CC: Paolo Bonzini 
---
 hw/intc/apic.c  | 4 ++--
 include/hw/i386/apic_internal.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index e4ee032..d64a0db 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -723,7 +723,7 @@ static uint32_t apic_mem_readl(void *opaque, hwaddr addr)
 val = s->divide_conf;
 break;
 default:
-s->esr |= ESR_ILLEGAL_ADDRESS;
+s->esr |= APIC_ESR_ILLEGAL_ADDRESS;
 val = 0;
 break;
 }
@@ -836,7 +836,7 @@ static void apic_mem_writel(void *opaque, hwaddr addr, 
uint32_t val)
 }
 break;
 default:
-s->esr |= ESR_ILLEGAL_ADDRESS;
+s->esr |= APIC_ESR_ILLEGAL_ADDRESS;
 break;
 }
 }
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index 6a40156..bc8d4e7 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -57,7 +57,7 @@
 #define APIC_INPUT_POLARITY (1<<13)
 #define APIC_SEND_PENDING   (1<<12)
 
-#define ESR_ILLEGAL_ADDRESS (1 << 7)
+#define APIC_ESR_ILLEGAL_ADDRESS(1 << 7)
 
 #define APIC_SV_DIRECTED_IO (1<<12)
 #define APIC_SV_ENABLE  (1<<8)
-- 
2.1.4

[Qemu-devel] [PATCH 9/9] hmp: implemented io apic dump state for TCG

[Denis V. Lunev]

From: Pavel Butsykin 

Added support emulator for the hmp command "info apic-io"

Signed-off-by: Pavel Butsykin 
Signed-off-by: Denis V. Lunev 
CC: Andreas Färber 
CC: Paolo Bonzini 
---
 hw/intc/ioapic.c  | 12 
 include/hw/i386/pc.h  |  1 +
 target-i386/monitor.c |  2 ++
 3 files changed, 15 insertions(+)

diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index b527932..8c3aeae 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -20,6 +20,7 @@
  * License along with this library; if not, see .
  */
 
+#include "monitor/monitor.h"
 #include "hw/hw.h"
 #include "hw/i386/pc.h"
 #include "hw/i386/ioapic.h"
@@ -137,6 +138,17 @@ void ioapic_eoi_broadcast(int vector)
 }
 }
 
+void ioapic_dump_state(Monitor *mon, const QDict *qdict)
+{
+int i;
+
+for (i = 0; i < MAX_IOAPICS; i++) {
+if (ioapics[i] != 0) {
+ioapic_print_redtbl(mon, ioapics[i]);
+}
+}
+}
+
 static uint64_t
 ioapic_mem_read(void *opaque, hwaddr addr, unsigned int size)
 {
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 539cf64..7c9f3a5 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -126,6 +126,7 @@ void hmp_info_irq(Monitor *mon, const QDict *qdict);
 /* ioapic.c */
 
 void kvm_ioapic_dump_state(Monitor *mon, const QDict *qdict);
+void ioapic_dump_state(Monitor *mon, const QDict *qdict);
 
 /* Global System Interrupts */
 
diff --git a/target-i386/monitor.c b/target-i386/monitor.c
index 797ea2d..f0774a3 100644
--- a/target-i386/monitor.c
+++ b/target-i386/monitor.c
@@ -505,5 +505,7 @@ void hmp_info_apic_io(Monitor *mon, const QDict *qdict)
 {
 if (kvm_irqchip_in_kernel()) {
 kvm_ioapic_dump_state(mon, qdict);
+} else {
+ioapic_dump_state(mon, qdict);
 }
 }
-- 
2.1.4

[Qemu-devel] [PATCH 3/9] apic_internal.h: added more constants

[Denis V. Lunev]

From: Pavel Butsykin 

These constants are needed for optimal access to
bit fields local apic registers without magic numbers.

Signed-off-by: Pavel Butsykin 
Signed-off-by: Denis V. Lunev 
CC: Andreas Färber 
CC: Paolo Bonzini 
---
 include/hw/i386/apic_internal.h | 54 +
 1 file changed, 54 insertions(+)

diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index bc8d4e7..3a3f8fc 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -50,14 +50,68 @@
 #define APIC_TRIGGER_EDGE   0
 #define APIC_TRIGGER_LEVEL  1
 
+#define APIC_VECTOR_MASK0xff
+#define APIC_DCR_MASK   0xf
+
+#define APIC_LVT_TIMER_SHIFT17
+#define APIC_LVT_MASKED_SHIFT   16
+#define APIC_LVT_LEVEL_TRIGGER_SHIFT15
+#define APIC_LVT_REMOTE_IRR_SHIFT   14
+#define APIC_LVT_INT_POLARITY_SHIFT 13
+#define APIC_LVT_DELIV_STS_SHIFT12
+#define APIC_LVT_DELIV_MOD_SHIFT8
+
 #define APIC_LVT_TIMER_PERIODIC (1<<17)
 #define APIC_LVT_MASKED (1<<16)
 #define APIC_LVT_LEVEL_TRIGGER  (1<<15)
 #define APIC_LVT_REMOTE_IRR (1<<14)
 #define APIC_INPUT_POLARITY (1<<13)
 #define APIC_SEND_PENDING   (1<<12)
+#define APIC_LVT_INT_POLARITY   (1 << APIC_LVT_INT_POLARITY_SHIFT)
+#define APIC_LVT_DELIV_STS  (1 << APIC_LVT_DELIV_STS_SHIFT)
+#define APIC_LVT_DELIV_MOD  (7 << APIC_LVT_DELIV_MOD_SHIFT)
+
+#define APIC_ESR_ILL_ADDRESS_SHIFT  7
+#define APIC_ESR_RECV_ILL_VECT_SHIFT6
+#define APIC_ESR_SEND_ILL_VECT_SHIFT5
+#define APIC_ESR_RECV_ACCEPT_SHIFT  3
+#define APIC_ESR_SEND_ACCEPT_SHIFT  2
+#define APIC_ESR_RECV_CHECK_SUM_SHIFT   1
 
 #define APIC_ESR_ILLEGAL_ADDRESS(1 << 7)
+#define APIC_ESR_RECV_ILLEGAL_VECT  (1 << APIC_ESR_RECV_ILL_VECT_SHIFT)
+#define APIC_ESR_SEND_ILLEGAL_VECT  (1 << APIC_ESR_SEND_ILL_VECT_SHIFT)
+#define APIC_ESR_RECV_ACCEPT(1 << APIC_ESR_RECV_ACCEPT_SHIFT)
+#define APIC_ESR_SEND_ACCEPT(1 << APIC_ESR_SEND_ACCEPT_SHIFT)
+#define APIC_ESR_RECV_CHECK_SUM (1 << APIC_ESR_RECV_CHECK_SUM_SHIFT)
+#define APIC_ESR_SEND_CHECK_SUM 1
+
+#define APIC_ICR_DEST_SHIFT 24
+#define APIC_ICR_DEST_SHORT_SHIFT   18
+#define APIC_ICR_TRIGGER_MOD_SHIFT  15
+#define APIC_ICR_LEVEL_SHIFT14
+#define APIC_ICR_DELIV_STS_SHIFT12
+#define APIC_ICR_DEST_MOD_SHIFT 11
+#define APIC_ICR_DELIV_MOD_SHIFT8
+
+#define APIC_ICR_DEST_SHORT (3 << APIC_ICR_DEST_SHORT_SHIFT)
+#define APIC_ICR_TRIGGER_MOD(1 << APIC_ICR_TRIGGER_MOD_SHIFT)
+#define APIC_ICR_LEVEL  (1 << APIC_ICR_LEVEL_SHIFT)
+#define APIC_ICR_DELIV_STS  (1 << APIC_ICR_DELIV_STS_SHIFT)
+#define APIC_ICR_DEST_MOD   (1 << APIC_ICR_DEST_MOD_SHIFT)
+#define APIC_ICR_DELIV_MOD  (7 << APIC_ICR_DELIV_MOD_SHIFT)
+
+#define APIC_PR_CLASS_SHIFT 4
+#define APIC_PR_SUB_CLASS   0xf
+
+#define APIC_LOGDEST_ID_SHIFT   4
+#define APIC_LOGDEST_APIC_ID0xf
+
+#define APIC_SPURIO_FOCUS_SHIFT 9
+#define APIC_SPURIO_ENABLED_SHIFT   8
+
+#define APIC_SPURIO_FOCUS   (1 << APIC_SPURIO_FOCUS_SHIFT)
+#define APIC_SPURIO_ENABLED (1 << APIC_SPURIO_ENABLED_SHIFT)
 
 #define APIC_SV_DIRECTED_IO (1<<12)
 #define APIC_SV_ENABLE  (1<<8)
-- 
2.1.4

[Qemu-devel] [PATCH 8/9] hmp: added io apic dump state

[Denis V. Lunev]

From: Pavel Butsykin 

Added the hmp command to query io apic state, may be usefull after guest
crashes to understand IRQ routing in guest.

Implementation is only for kvm here. The dump will look like
(qemu) info apic-io
ioapic ID=00 IRR= SEL=18
ioapic 00 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
ioapic 01 03000993: int=93 delmod=1:LowPri L.H.E. dest=3
...
ioapic 23 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0

Signed-off-by: Pavel Butsykin 
Signed-off-by: Denis V. Lunev 
CC: Andreas Färber 
CC: Paolo Bonzini 
---
 hmp-commands-info.hx  | 16 
 hw/i386/kvm/ioapic.c  | 10 ++
 hw/intc/ioapic_common.c   | 31 +++
 include/hw/i386/ioapic_internal.h |  2 ++
 include/hw/i386/pc.h  |  4 
 include/monitor/monitor-common.h  |  1 +
 target-i386/monitor.c |  9 +
 7 files changed, 73 insertions(+)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 5ffc181..c65b7d2 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -128,6 +128,22 @@ STEXI
 Show local APIC state
 ETEXI
 
+#if defined(TARGET_I386)
+{
+.name   = "apic-io",
+.args_type  = "",
+.params = "",
+.help   = "show io apic state",
+.mhandler.cmd = hmp_info_apic_io,
+},
+#endif
+
+STEXI
+@item info apic-io
+@findex apic-io
+Show io APIC state
+ETEXI
+
 {
 .name   = "cpus",
 .args_type  = "",
diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index d2a6c4c..b7390ca 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -10,6 +10,7 @@
  * See the COPYING file in the top-level directory.
  */
 
+#include "monitor/monitor.h"
 #include "hw/i386/pc.h"
 #include "hw/i386/ioapic_internal.h"
 #include "hw/i386/apic_internal.h"
@@ -110,6 +111,15 @@ static void kvm_ioapic_put(IOAPICCommonState *s)
 }
 }
 
+void kvm_ioapic_dump_state(Monitor *mon, const QDict *qdict)
+{
+IOAPICCommonState s;
+
+kvm_ioapic_get(&s);
+
+ioapic_print_redtbl(mon, &s);
+}
+
 static void kvm_ioapic_reset(DeviceState *dev)
 {
 IOAPICCommonState *s = IOAPIC_COMMON(dev);
diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index 8b7d118..83edf69 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -19,6 +19,7 @@
  * License along with this library; if not, see .
  */
 
+#include "monitor/monitor.h"
 #include "hw/i386/ioapic.h"
 #include "hw/i386/ioapic_internal.h"
 #include "hw/sysbus.h"
@@ -31,6 +32,36 @@
  */
 int ioapic_no;
 
+void ioapic_print_redtbl(Monitor *mon, IOAPICCommonState *s)
+{
+static const char *delm_str[] = {
+"Fixed", "LowPri", "SMI", "...", "NMI", "INIT", "...", "ExtINT"};
+int i;
+
+monitor_printf(mon, "ioapic ID=%02x IRR=%08x SEL=%02x\n",
+   s->id, s->irr, s->ioregsel);
+
+for (i = 0; i < IOAPIC_NUM_PINS; i++) {
+uint64_t entry = s->ioredtbl[i];
+uint32_t delm = (uint32_t)((entry & IOAPIC_LVT_DELIV_MODE) >>
+   IOAPIC_LVT_DELIV_MODE_SHIFT);
+monitor_printf(mon, "ioapic %02u %016jx: int=%02jx "
+   "delmod=%x:%-6s %c%c%c%c%c%c dest=%jx\n",
+   i, entry,
+   entry & IOAPIC_VECTOR_MASK,
+   delm,
+   delm_str[delm],
+   entry & IOAPIC_LVT_DEST_MODE ? 'L' : 'P',
+   entry & IOAPIC_LVT_DELIV_STATUS ? 'P' : '.',
+   entry & IOAPIC_LVT_POLARITY ? 'L' : 'H',
+   entry & IOAPIC_LVT_REMOTE_IRR ? 'R' : '.',
+   entry & IOAPIC_LVT_TRIGGER_MODE ? 'L' : 'E',
+   entry & IOAPIC_LVT_MASKED ? 'M' : '.',
+   (entry >> IOAPIC_LVT_DEST_SHIFT) &
+(entry & IOAPIC_LVT_DEST_MODE ? 0xff : 0xf));
+}
+}
+
 void ioapic_reset_common(DeviceState *dev)
 {
 IOAPICCommonState *s = IOAPIC_COMMON(dev);
diff --git a/include/hw/i386/ioapic_internal.h 
b/include/hw/i386/ioapic_internal.h
index 4f7764e..797ed47 100644
--- a/include/hw/i386/ioapic_internal.h
+++ b/include/hw/i386/ioapic_internal.h
@@ -105,4 +105,6 @@ struct IOAPICCommonState {
 
 void ioapic_reset_common(DeviceState *dev);
 
+void ioapic_print_redtbl(Monitor *mon, IOAPICCommonState *s);
+
 #endif /* !QEMU_IOAPIC_INTERNAL_H */
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 3e002c9..539cf64 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -123,6 +123,10 @@ int pic_get_output(DeviceState *d);
 void hmp_info_pic(Monitor *mon, const QDict *qdict);
 void hmp_info_irq(Monitor *mon, const QDict *qdict);
 
+/* ioapic.c */
+
+void kvm_ioapic_dump_state(Monitor *mon, const QDict *qdict);
+
 /* Global System Interrupts */
 
 #define GSI_NUM_PINS IOAPIC_NUM_PINS
diff --git a/include/monitor/monitor-common.h b/include/

[Qemu-devel] [PATCH 5/9] monitor: make monitor_fprintf and mon_get_cpu externally visible

[Denis V. Lunev]

From: Pavel Butsykin 

monitor_fprintf and mon_get_cpu will be used in the target-specific monitor,
so it is advisable to make it external.

Signed-off-by: Pavel Butsykin 
Signed-off-by: Denis V. Lunev 
CC: Andreas Färber 
CC: Paolo Bonzini 
---
 disas.c  | 10 --
 include/monitor/monitor-common.h |  1 +
 include/monitor/monitor.h|  1 +
 monitor.c|  5 ++---
 4 files changed, 4 insertions(+), 13 deletions(-)

diff --git a/disas.c b/disas.c
index 0ae70c2..45878fa 100644
--- a/disas.c
+++ b/disas.c
@@ -392,16 +392,6 @@ monitor_read_memory (bfd_vma memaddr, bfd_byte *myaddr, 
int length,
 return 0;
 }
 
-static int GCC_FMT_ATTR(2, 3)
-monitor_fprintf(FILE *stream, const char *fmt, ...)
-{
-va_list ap;
-va_start(ap, fmt);
-monitor_vprintf((Monitor *)stream, fmt, ap);
-va_end(ap);
-return 0;
-}
-
 /* Disassembler for the monitor.
See target_disas for a description of flags. */
 void monitor_disas(Monitor *mon, CPUState *cpu,
diff --git a/include/monitor/monitor-common.h b/include/monitor/monitor-common.h
index 7d1ec5a..abd7a6c 100644
--- a/include/monitor/monitor-common.h
+++ b/include/monitor/monitor-common.h
@@ -37,6 +37,7 @@ typedef struct MonitorDef {
 const MonitorDef *target_monitor_defs(void);
 
 CPUArchState *mon_get_cpu_env(void);
+CPUState *mon_get_cpu(void);
 
 void hmp_info_mem(Monitor *mon, const QDict *qdict);
 void hmp_info_tlb(Monitor *mon, const QDict *qdict);
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 9aff47e..f95a384 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -34,6 +34,7 @@ int monitor_fd_param(Monitor *mon, const char *fdname, Error 
**errp);
 void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
 GCC_FMT_ATTR(2, 0);
 void monitor_printf(Monitor *mon, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
+int monitor_fprintf(FILE *stream, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 void monitor_flush(Monitor *mon);
 int monitor_set_cpu(int cpu_index);
 int monitor_get_cpu_index(void);
diff --git a/monitor.c b/monitor.c
index e4a5653..989c685 100644
--- a/monitor.c
+++ b/monitor.c
@@ -372,8 +372,7 @@ void monitor_printf(Monitor *mon, const char *fmt, ...)
 va_end(ap);
 }
 
-static int GCC_FMT_ATTR(2, 3) monitor_fprintf(FILE *stream,
-  const char *fmt, ...)
+int monitor_fprintf(FILE *stream, const char *fmt, ...)
 {
 va_list ap;
 va_start(ap, fmt);
@@ -942,7 +941,7 @@ int monitor_set_cpu(int cpu_index)
 return 0;
 }
 
-static CPUState *mon_get_cpu(void)
+CPUState *mon_get_cpu(void)
 {
 if (!cur_mon->mon_cpu) {
 monitor_set_cpu(0);
-- 
2.1.4

[Qemu-devel] [PATCH v2 0/9] hmp command IO- and Local APIC dump state

[Denis V. Lunev]

Added the hmp command to query IO- and Local APIC registers state,
it can be very useful to identify problems related to the emulation devices.

(qemu) info apic-local
apic.lvt00-timer   000300fd int=fd .H.EMP delmod=0:Fixed
apic.lvt00-thermal 0001 int=00 .H.EM. delmod=0:Fixed
apic.lvt00-perfmon 00fe int=fe .H.E.. delmod=0:Fixed
apic.lvt00-LINT0   0001001f int=1f .H.EM. delmod=0:Fixed
apic.lvt00-LINT1   04ff int=ff .H.E.. delmod=4:NMI
apic.lvt00-Error   00e3 int=e3 .H.E.. delmod=0:Fixed
apic.error  00 esr  S:... R:... .
apic.timer  00 DCR=000b(b) initial_count=19
apic.icr00 020c00d1: int=d1 delmod=0:Fixed P..E shorthand=3:all 
dest=2
apic.prio   00 apr=00(0:0)  tpr=40(4:0)
apic.dest   00 dfr=f0(f)ldr=01(01)
apic.svr00 011f vec=1f on focus=off
apic.interrupt  00 065:R.E

(qemu) info apic-io
ioapic ID=00 IRR= SEL=18
ioapic 00 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
ioapic 01 03000993: int=93 delmod=1:LowPri L.H.E. dest=3
ioapic 02 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
ioapic 03 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
ioapic 04 03000992: int=92 delmod=1:LowPri L.H.E. dest=3
ioapic 05 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
ioapic 06 020109b3: int=b3 delmod=1:LowPri L.H.EM dest=2
ioapic 07 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
ioapic 08 010008d1: int=d1 delmod=0:Fixed  L.H.E. dest=1
ioapic 09 030089b1: int=b1 delmod=1:LowPri L.H.L. dest=3
ioapic 10 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
ioapic 11 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
ioapic 12 030009a3: int=a3 delmod=1:LowPri L.H.E. dest=3
ioapic 13 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
ioapic 14 03000962: int=62 delmod=1:LowPri L.H.E. dest=3
ioapic 15 03000982: int=82 delmod=1:LowPri L.H.E. dest=3
ioapic 16 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
ioapic 17 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
ioapic 18 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
ioapic 19 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
ioapic 20 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
ioapic 21 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
ioapic 22 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
ioapic 23 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0

Changes from v1:
 - implementation of hmp commands moved to the target-i386 part
 - the cpu_dump_apic_local_state interface moved to the target-i386 part

Signed-off-by: Pavel Butsykin 
Signed-off-by: Denis V. Lunev 
CC: Andreas Färber 
CC: Paolo Bonzini 

Pavel Butsykin (9):
  apic_internal.h: move apic_get_bit(), apic_set_bit() to
apic_internal.h
  apic_internal.h: rename ESR_ILLEGAL_ADDRESS to
APIC_ESR_ILLEGAL_ADDRESS
  apic_internal.h: added more constants
  apic_internal.h: fix formatting and drop unused consts
  monitor: make monitor_fprintf and mon_get_cpu externally visible
  hmp: added local apic dump state
  ioapic_internal.h: added more constants
  hmp: added io apic dump state
  hmp: implemented io apic dump state for TCG

 disas.c   |  10 ---
 hmp-commands-info.hx  |  32 
 hw/i386/kvm/ioapic.c  |  10 +++
 hw/intc/apic.c|  20 +
 hw/intc/ioapic.c  |  12 +++
 hw/intc/ioapic_common.c   |  31 
 include/hw/i386/apic_internal.h   |  90 +++---
 include/hw/i386/ioapic_internal.h |   7 ++
 include/hw/i386/pc.h  |   5 ++
 include/monitor/monitor-common.h  |   3 +
 include/monitor/monitor.h |   1 +
 monitor.c |   5 +-
 target-i386/cpu.h |   3 +
 target-i386/helper.c  | 155 ++
 target-i386/monitor.c |  17 +
 15 files changed, 359 insertions(+), 42 deletions(-)

-- 
2.1.4

Re: [Qemu-devel] [Qemu-block] [PATCH v3 4/5] qmp: add monitor command to add/remove a child

[Kevin Wolf]

Am 15.09.2015 um 11:20 hat Kevin Wolf geschrieben:
> Am 15.09.2015 um 10:56 hat Alberto Garcia geschrieben:
> > On Mon 14 Sep 2015 05:37:00 PM CEST, Kevin Wolf  wrote:
> > 
> > >> +{ 'command': 'x-child-add',
> > >> +  'data' : { 'parent': 'str', 'child': 'str' } }
> > >
> > > This is probably not future-proof and only made for the special case
> > > of quorum. Specifically, one thing I'm missing is some way to specfiy
> > > what kind of child the new node is when a node can take different
> > > types of children (e.g. bs->file and bs->backing_hd).
> > 
> > Children here are specified by child roles, aren't they?
> 
> Possibly, but actually, currently I'm inclined to think that they
> aren't. Child roles are still relatively new, though, so it's hard to
> say if this is just because we didn't use them in that way so far, or
> because they are really the wrong tool.
> 
> What I can say is that traditionally children are identified by option
> names. Block drivers assign a ChildRole when processing the option, and
> both are equivalent only if the same ChildRole is never used for two
> different children. I believe that that's currently true, but I'm
> doubtful whether it will remain this way (even looking at blkverify,
> which has one &child_file and one &child_format, things start to look a
> bit confusing).

What I wanted to mention, but forgot to do, is that obviously we could
use more specific ChildRoles than &child_file and &child_format,
essentially creating one ChildRole per option and making them equivalent
this way.

I'm not sure if that would be a good or a bad idea.

> Possibly the right and consistent way to change the set of children
> would be through a QMP command exposing bdrv_reopen(), which would also
> be used for changing other options at runtime.
> 
> My current pull request adds the qemu-io (and therefore HMP) way of
> doing this, but finding a good QMP interface will still be a challenge.
> 
> > We could have a ChildRole enum with 'file', 'format' and 'backing' that
> > would be passed to this command and use the same enumeration in
> > bdrv_open_image() rather than a pointer to BdrvChildRole.
> 
> Yes, that would be an option if ChildRole turned out to be enough.
> 
> Kevin
> 

Re: [Qemu-devel] [PULL v2 00/23] Block layer patches

[Peter Maydell]

On 14 September 2015 at 16:25, Kevin Wolf  wrote:
> The following changes since commit 2b750d9d261bda7f75b39dfc1e1e5f22502929d5:
>
>   Merge remote-tracking branch 'remotes/aurel/tags/pull-sh4-next-20150913' 
> into staging (2015-09-14 10:46:38 +0100)
>
> are available in the git repository at:
>
>
>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to 2ac01520be8717f3492b10a083c3e0e22cb52cda:
>
>   qcow2: Make qcow2_alloc_bytes() more explicit (2015-09-14 16:51:37 +0200)
>
> 
> Block layer patches (v2)

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH] target-mips: fix corner case in TLBWR causing QEMU to hang

[Leon Alrae]

On 14/09/15 20:21, Aurelien Jarno wrote:
> Note that this patch conflicts with the following one, that we might
> want to merge, even if the whole series is not ready:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg01171.html

Indeed, we should merge that patch as well.

Thanks,
Leon

Re: [Qemu-devel] [PATCH pic32 v3 02/16] pic32: use LCG algorithm for generated random index of TLBWR instruction

[Leon Alrae]

On 06/07/15 07:14, Serge Vakulenko wrote:
> The LFSR algorithm, used for generating random TLB indexes for TLBWR 
> instruction,
> was inclined to produce a degenerate sequence in some cases.
> For example, for 16-entry TLB size and Wired=1, it gives: 15, 6, 7, 2,
> 7, 2, 7, 2, 7, 2, 7, 2, 7, 2, 7, 2, 7, 2, 7, 2, 7, 2, 7, 2, 7, 2, 7, 2...
> When replaced with LCG algorithm from ISO/IEC 9899 standard, the sequence
> looks much better, with about the same computational effort needed.
> 
> Signed-off-by: Serge Vakulenko 
> ---
>  hw/mips/cputimer.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

I cherry-picked this patch from the series and applied to mips-next
queue, thanks.

Leon

Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Fix issue for checking ptr in different address spaces in TARGET_CMSG_NXTHDR

[Chen Gang]

yes, foe me, they are the same.  :-)

Thanks.

> From: peter.mayd...@linaro.org
> Date: Tue, 15 Sep 2015 09:10:40 +0100
> Subject: Re: [PATCH] linux-user/syscall.c: Fix issue for checking ptr in 
> different address spaces in TARGET_CMSG_NXTHDR
> To: gang.chen.5...@gmail.com
> CC: riku.voi...@iki.fi; r...@twiddle.net; xili_gchen_5...@hotmail.com; 
> qemu-devel@nongnu.org
> 
> On 15 September 2015 at 04:01,   wrote:
> > From: Chen Gang 
> >
> > After fix this issue, qemu can run i386 wine notepad.exe successfully.
> > But the initialization performance is not quite well.
> >
> > Signed-off-by: Chen Gang 
> > ---
> >  linux-user/syscall.c  | 30 +-
> >  linux-user/syscall_defs.h | 20 +---
> >  2 files changed, 26 insertions(+), 24 deletions(-)
> 
> Isn't this duplication of fixes in this patch currently on-list?
> 
> https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00680.html
> 
> thanks
> -- PMM
  

Re: [Qemu-devel] [PULL 0/19] xen-2015-09-08-tag

[Stefano Stabellini]

On Mon, 14 Sep 2015, Paolo Bonzini wrote:
> On 10/09/2015 12:29, Stefano Stabellini wrote:
> > +if (lseek(config_fd, pos, SEEK_SET) != pos) {
> > +return -errno;
> > +}
> >  do {
> > -rc = pread(config_fd, (uint8_t *)&val, len, pos);
> > +rc = read(config_fd, (uint8_t *)&val, len);
> >  } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
> 
> This leaks config_fd.

I don't follow, it leaks config_fd where?

Re: [Qemu-devel] [PATCH v16 16/35] host-utils: Add revbit functions

[Peter Maydell]

On 14 September 2015 at 23:43, Richard Henderson  wrote:
> Signed-off-by: Richard Henderson 
> ---
>  include/qemu/host-utils.h | 77 
> +++
>  1 file changed, 77 insertions(+)

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v16 17/35] target-arm: Use new revbit functions

[Peter Maydell]

On 14 September 2015 at 23:43, Richard Henderson  wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target-arm/helper-a64.c | 15 +--
>  target-arm/helper.c | 12 +---
>  2 files changed, 2 insertions(+), 25 deletions(-)

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v16 18/35] target-tilegx: Handle most bit manipulation instructions

[Peter Maydell]

On 14 September 2015 at 23:43, Richard Henderson  wrote:
> The crc instructions are omitted from this set.
>
> Signed-off-by: Richard Henderson 
> ---
>  target-tilegx/helper.c| 10 +++
>  target-tilegx/helper.h|  2 ++
>  target-tilegx/translate.c | 68 
> ++-
>  3 files changed, 79 insertions(+), 1 deletion(-)
>
> diff --git a/target-tilegx/helper.c b/target-tilegx/helper.c
> index 5b37a8c..a01bb8d 100644
> --- a/target-tilegx/helper.c
> +++ b/target-tilegx/helper.c
> @@ -40,6 +40,16 @@ uint64_t helper_cnttz(uint64_t arg)
>  return ctz64(arg);
>  }
>
> +uint64_t helper_pcnt(uint64_t arg)
> +{
> +return ctpop64(arg);
> +}
> +
> +uint64_t helper_revbits(uint64_t arg)
> +{
> +return revbit64(arg);
> +}
> +
>  /*
>   * Functional Description
>   * uint64_t a = rf[SrcA];
> diff --git a/target-tilegx/helper.h b/target-tilegx/helper.h
> index fd5517e..644d313 100644
> --- a/target-tilegx/helper.h
> +++ b/target-tilegx/helper.h
> @@ -1,4 +1,6 @@
>  DEF_HELPER_2(exception, noreturn, env, i32)
>  DEF_HELPER_FLAGS_1(cntlz, TCG_CALL_NO_RWG_SE, i64, i64)
>  DEF_HELPER_FLAGS_1(cnttz, TCG_CALL_NO_RWG_SE, i64, i64)
> +DEF_HELPER_FLAGS_1(pcnt, TCG_CALL_NO_RWG_SE, i64, i64)
> +DEF_HELPER_FLAGS_1(revbits, TCG_CALL_NO_RWG_SE, i64, i64)
>  DEF_HELPER_FLAGS_3(shufflebytes, TCG_CALL_NO_RWG_SE, i64, i64, i64, i64)
> diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
> index 6b5de55..c2f38f1 100644
> --- a/target-tilegx/translate.c
> +++ b/target-tilegx/translate.c
> @@ -177,6 +177,43 @@ static void gen_saturate_op(TCGv tdest, TCGv tsrca, TCGv 
> tsrcb,
>  tcg_temp_free(t0);
>  }
>
> +/* Shift the 128-bit value TSRCA:TSRCD riht by the number of bytes


Typo: "right".

Otherwise

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PULL 21/29] xen/pt: Sync up the dev.config and data values.

[Stefano Stabellini]

CC Konrad

On Mon, 14 Sep 2015, Paolo Bonzini wrote:
> On 10/09/2015 19:15, Stefano Stabellini wrote:
> > +
> > +switch (reg->size) {
> > +case 1: rc = xen_host_pci_get_byte(&s->real_device, offset, 
> > (uint8_t *)&val);
> 
> A bit ugly, and it relies on the host being little endian.
> 
> > +break;
> > +case 2: rc = xen_host_pci_get_word(&s->real_device, offset, 
> > (uint16_t *)&val);
> 
> Same here.

cpu_to_le32?

But in practice, Xen being little endian only, I doubt that xen_pt_config_init.c
would actually work on be.


> > +break;
> > +case 4: rc = xen_host_pci_get_long(&s->real_device, offset, &val);
> > +break;
> > +default: assert(1);
> 
> This should be assert(0) or, better, abort().

[Qemu-devel] [PATCH PULL v2 00/11] Extract TLS handling code from VNC server

[Daniel P. Berrange]

The following changes since commit 007e620a7576e4ce2ea6955541e87d8ae8ed32ae:

  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging 
(2015-09-14 18:51:09 +0100)

are available in the git repository at:

  git://github.com/berrange/qemu.git tags/vnc-crypto-v8-for-upstream

for you to fetch changes up to 63909633894a7d613aa80a32a827581da9bf5ad5:

  ui: convert VNC server to use QCryptoTLSSession (2015-09-15 11:08:52 +0100)


Merge vnc-crypto-v8


Daniel P. Berrange (11):
  qapi: allow override of default enum prefix naming
  tests: remove repetition in unit test object deps
  crypto: move crypto objects out of libqemuutil.la
  qom: allow QOM to be linked into tools binaries
  crypto: introduce new base module for TLS credentials
  crypto: introduce new module for TLS anonymous credentials
  crypto: introduce new module for TLS x509 credentials
  crypto: add sanity checking of TLS x509 credentials
  crypto: introduce new module for handling TLS sessions
  ui: fix return type for VNC I/O functions to be ssize_t
  ui: convert VNC server to use QCryptoTLSSession

 Makefile|   10 +-
 Makefile.objs   |   10 +-
 Makefile.target |4 +
 configure   |   53 +-
 crypto/Makefile.objs|   14 +-
 crypto/tlscreds.c   |  251 +++
 crypto/tlscredsanon.c   |  223 +++
 crypto/tlscredspriv.h   |   42 ++
 crypto/tlscredsx509.c   |  809 ++
 crypto/tlssession.c |  574 
 docs/qapi-code-gen.txt  |8 +
 include/crypto/tlscreds.h   |   68 ++
 include/crypto/tlscredsanon.h   |  112 
 include/crypto/tlscredsx509.h   |  113 
 include/crypto/tlssession.h |  322 +
 qapi-schema.json|3 +
 qapi/crypto.json|   21 +
 qemu-options.hx |   75 ++-
 qom/Makefile.objs   |7 +-
 scripts/qapi-types.py   |   16 +-
 scripts/qapi.py |   10 +-
 tests/.gitignore|7 +
 tests/Makefile  |  106 +--
 tests/crypto-tls-x509-helpers.c |  485 ++
 tests/crypto-tls-x509-helpers.h |  133 
 tests/pkix_asn1_tab.c   | 1104 +++
 tests/qapi-schema/enum-bad-prefix.err   |1 +
 tests/qapi-schema/enum-bad-prefix.exit  |1 +
 tests/qapi-schema/enum-bad-prefix.json  |2 +
 tests/qapi-schema/enum-bad-prefix.out   |0
 tests/qapi-schema/qapi-schema-test.json |5 +
 tests/qapi-schema/qapi-schema-test.out  |2 +
 tests/test-crypto-tlscredsx509.c|  731 
 tests/test-crypto-tlssession.c  |  535 +++
 trace-events|   18 +
 ui/Makefile.objs|2 +-
 ui/vnc-auth-sasl.c  |   36 +-
 ui/vnc-auth-vencrypt.c  |   80 ++-
 ui/vnc-tls.c|  474 -
 ui/vnc-tls.h|   69 --
 ui/vnc-ws.c |   84 +--
 ui/vnc-ws.h |2 -
 ui/vnc.c|  362 ++
 ui/vnc.h|   21 +-
 44 files changed, 6125 insertions(+), 880 deletions(-)
 create mode 100644 crypto/tlscreds.c
 create mode 100644 crypto/tlscredsanon.c
 create mode 100644 crypto/tlscredspriv.h
 create mode 100644 crypto/tlscredsx509.c
 create mode 100644 crypto/tlssession.c
 create mode 100644 include/crypto/tlscreds.h
 create mode 100644 include/crypto/tlscredsanon.h
 create mode 100644 include/crypto/tlscredsx509.h
 create mode 100644 include/crypto/tlssession.h
 create mode 100644 qapi/crypto.json
 create mode 100644 tests/crypto-tls-x509-helpers.c
 create mode 100644 tests/crypto-tls-x509-helpers.h
 create mode 100644 tests/pkix_asn1_tab.c
 create mode 100644 tests/qapi-schema/enum-bad-prefix.err
 create mode 100644 tests/qapi-schema/enum-bad-prefix.exit
 create mode 100644 tests/qapi-schema/enum-bad-prefix.json
 create mode 100644 tests/qapi-schema/enum-bad-prefix.out
 create mode 100644 tests/test-crypto-tlscredsx509.c
 create mode 100644 tests/test-crypto-tlssession.c
 delete mode 100644 ui/vnc-tls.c
 delete mode 100644 ui/vnc-tls.h

-- 
2.4.3

[Qemu-devel] [PATCH PULL v2 04/11] qom: allow QOM to be linked into tools binaries

[Daniel P. Berrange]

The qom objects are currently added to common-obj-y
which is only linked into the system emulators. The
later crypto patches will depend on QOM infrastructure
and will also be used from tools binaries. Thus the QOM
objects are moved into a new qom-obj-y variable which
can be referenced when linking tools, system emulators
and tests.

Signed-off-by: Daniel P. Berrange 
---
 Makefile  | 8 +---
 Makefile.objs | 5 +
 Makefile.target   | 2 ++
 qom/Makefile.objs | 7 ---
 tests/Makefile| 4 +---
 5 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index bb06c4f..8c096a7 100644
--- a/Makefile
+++ b/Makefile
@@ -153,6 +153,7 @@ dummy := $(call unnest-vars,, \
 block-obj-y \
 block-obj-m \
 crypto-obj-y \
+qom-obj-y \
 common-obj-y \
 common-obj-m)
 
@@ -175,6 +176,7 @@ SOFTMMU_SUBDIR_RULES=$(filter %-softmmu,$(SUBDIR_RULES))
 
 $(SOFTMMU_SUBDIR_RULES): $(block-obj-y)
 $(SOFTMMU_SUBDIR_RULES): $(crypto-obj-y)
+$(SOFTMMU_SUBDIR_RULES): $(qom-obj-y)
 $(SOFTMMU_SUBDIR_RULES): config-all-devices.mak
 
 subdir-%:
@@ -229,9 +231,9 @@ util/module.o-cflags = 
-D'CONFIG_BLOCK_MODULES=$(block-modules)'
 
 qemu-img.o: qemu-img-cmds.h
 
-qemu-img$(EXESUF): qemu-img.o $(block-obj-y) $(crypto-obj-y) libqemuutil.a 
libqemustub.a
-qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) $(crypto-obj-y) libqemuutil.a 
libqemustub.a
-qemu-io$(EXESUF): qemu-io.o $(block-obj-y) $(crypto-obj-y) libqemuutil.a 
libqemustub.a
+qemu-img$(EXESUF): qemu-img.o $(block-obj-y) $(crypto-obj-y) $(qom-obj-y) 
libqemuutil.a libqemustub.a
+qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) $(crypto-obj-y) $(qom-obj-y) 
libqemuutil.a libqemustub.a
+qemu-io$(EXESUF): qemu-io.o $(block-obj-y) $(crypto-obj-y) $(qom-obj-y) 
libqemuutil.a libqemustub.a
 
 qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
 
diff --git a/Makefile.objs b/Makefile.objs
index c7ed989..2ff4224 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -25,6 +25,11 @@ block-obj-m = block/
 
 crypto-obj-y = crypto/
 
+###
+# qom-obj-y is code used by both qemu system emulation and qemu-img
+
+qom-obj-y = qom/
+
 ##
 # smartcard
 
diff --git a/Makefile.target b/Makefile.target
index 8235202..4c8eaca 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -171,6 +171,7 @@ dummy := $(call unnest-vars,.., \
block-obj-y \
block-obj-m \
crypto-obj-y \
+   qom-obj-y \
common-obj-y \
common-obj-m)
 target-obj-y := $(target-obj-y-save)
@@ -178,6 +179,7 @@ all-obj-y += $(common-obj-y)
 all-obj-y += $(target-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(crypto-obj-y)
+all-obj-$(CONFIG_SOFTMMU) += $(qom-obj-y)
 
 $(QEMU_PROG_BUILD): config-devices.mak
 
diff --git a/qom/Makefile.objs b/qom/Makefile.objs
index 985003b..516349e 100644
--- a/qom/Makefile.objs
+++ b/qom/Makefile.objs
@@ -1,3 +1,4 @@
-common-obj-y = object.o container.o qom-qobject.o
-common-obj-y += cpu.o
-common-obj-y += object_interfaces.o
+qom-obj-y = object.o container.o qom-qobject.o
+qom-obj-y += object_interfaces.o
+
+common-obj-y = cpu.o
diff --git a/tests/Makefile b/tests/Makefile
index d77ccee..ddd5148 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -284,9 +284,7 @@ QEMU_CFLAGS += -I$(SRC_PATH)/tests
 
 # Deps that are common to various different sets of tests below
 test-util-obj-y = libqemuutil.a libqemustub.a
-test-qom-obj-y = qom/object.o qom/qom-qobject.o \
-   qom/container.o qom/object_interfaces.o \
-   $(test-util-obj-y)
+test-qom-obj-y = $(qom-obj-y) $(test-util-obj-y)
 test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \
tests/test-qapi-event.o \
$(test-qom-obj-y)
-- 
2.4.3

[Qemu-devel] [PATCH PULL v2 07/11] crypto: introduce new module for TLS x509 credentials

[Daniel P. Berrange]

Introduce a QCryptoTLSCredsX509 class which is used to
manage x509 certificate TLS credentials. This will be
the preferred credential type offering strong security
characteristics

Example CLI configuration:

 $QEMU -object tls-creds-x509,id=tls0,endpoint=server,\
   dir=/path/to/creds/dir,verify-peer=yes

The 'id' value in the -object args will be used to associate the
credentials with the network services. For example, when the VNC
server is later converted it would use

 $QEMU -object tls-creds-x509,id=tls0, \
   -vnc 127.0.0.1:1,tls-creds=tls0

Signed-off-by: Daniel P. Berrange 
Reviewed-by: Eric Blake 
---
 crypto/Makefile.objs  |   1 +
 crypto/tlscredsx509.c | 263 ++
 include/crypto/tlscredsx509.h | 112 ++
 qemu-options.hx   |  27 +
 trace-events  |   3 +
 5 files changed, 406 insertions(+)
 create mode 100644 crypto/tlscredsx509.c
 create mode 100644 include/crypto/tlscredsx509.h

diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 2080c5d..5894c88 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -5,3 +5,4 @@ crypto-obj-y += desrfb.o
 crypto-obj-y += cipher.o
 crypto-obj-y += tlscreds.o
 crypto-obj-y += tlscredsanon.o
+crypto-obj-y += tlscredsx509.o
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
new file mode 100644
index 000..6cc7b53
--- /dev/null
+++ b/crypto/tlscredsx509.c
@@ -0,0 +1,263 @@
+/*
+ * QEMU crypto TLS x509 credential support
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include "crypto/tlscredsx509.h"
+#include "crypto/tlscredspriv.h"
+#include "qom/object_interfaces.h"
+#include "trace.h"
+
+
+#ifdef CONFIG_GNUTLS
+
+
+static int
+qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
+Error **errp)
+{
+char *cacert = NULL, *cacrl = NULL, *cert = NULL,
+*key = NULL, *dhparams = NULL;
+int ret;
+int rv = -1;
+
+trace_qcrypto_tls_creds_x509_load(creds,
+creds->parent_obj.dir ? creds->parent_obj.dir : "");
+
+if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
+if (qcrypto_tls_creds_get_path(&creds->parent_obj,
+   QCRYPTO_TLS_CREDS_X509_CA_CERT,
+   true, &cacert, errp) < 0 ||
+qcrypto_tls_creds_get_path(&creds->parent_obj,
+   QCRYPTO_TLS_CREDS_X509_CA_CRL,
+   false, &cacrl, errp) < 0 ||
+qcrypto_tls_creds_get_path(&creds->parent_obj,
+   QCRYPTO_TLS_CREDS_X509_SERVER_CERT,
+   true, &cert, errp) < 0 ||
+qcrypto_tls_creds_get_path(&creds->parent_obj,
+   QCRYPTO_TLS_CREDS_X509_SERVER_KEY,
+   true, &key, errp) < 0 ||
+qcrypto_tls_creds_get_path(&creds->parent_obj,
+   QCRYPTO_TLS_CREDS_DH_PARAMS,
+   false, &dhparams, errp) < 0) {
+goto cleanup;
+}
+} else {
+if (qcrypto_tls_creds_get_path(&creds->parent_obj,
+   QCRYPTO_TLS_CREDS_X509_CA_CERT,
+   true, &cacert, errp) < 0 ||
+qcrypto_tls_creds_get_path(&creds->parent_obj,
+   QCRYPTO_TLS_CREDS_X509_CLIENT_CERT,
+   false, &cert, errp) < 0 ||
+qcrypto_tls_creds_get_path(&creds->parent_obj,
+   QCRYPTO_TLS_CREDS_X509_CLIENT_KEY,
+   false, &key, errp) < 0) {
+goto cleanup;
+}
+}
+
+ret = gnutls_certificate_allocate_credentials(&creds->data);
+if (ret < 0) {
+error_setg(errp, "Cannot allocate credentials: '%s'",
+   gnutls_strerror(ret));
+goto cleanup;
+}
+
+ret = gnutls_certificate_set_x509_trust_file(creds->data,
+ cacert,
+ GNUTLS_X509_FMT_PEM);
+if (ret < 0) {

[Qemu-devel] [PATCH PULL v2 01/11] qapi: allow override of default enum prefix naming

[Daniel P. Berrange]

The camel_to_upper() method applies some heuristics to turn
a mixed case type name into an all-uppercase name. This is
used for example, to generate enum constant name prefixes.

The heuristics don't also generate a satisfactory name
though. eg

  { 'enum': 'QCryptoTLSCredsEndpoint',
'data': ['client', 'server']}

Results in Q_CRYPTOTLS_CREDS_ENDPOINT_CLIENT. This has
an undesirable _ after the initial Q and is missing an
_ between the CRYPTO & TLS strings.

Rather than try to add more and more heuristics to try
to cope with this, simply allow the QAPI schema to
specify the desired enum constant prefix explicitly.

eg

  { 'enum': 'QCryptoTLSCredsEndpoint',
'prefix': 'QCRYPTO_TLS_CREDS_ENDPOINT',
'data': ['client', 'server']}

Now gives the QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT name.

Signed-off-by: Daniel P. Berrange 
---
 docs/qapi-code-gen.txt  |  8 
 scripts/qapi-types.py   | 16 +---
 scripts/qapi.py | 10 --
 tests/Makefile  |  3 ++-
 tests/qapi-schema/enum-bad-prefix.err   |  1 +
 tests/qapi-schema/enum-bad-prefix.exit  |  1 +
 tests/qapi-schema/enum-bad-prefix.json  |  2 ++
 tests/qapi-schema/enum-bad-prefix.out   |  0
 tests/qapi-schema/qapi-schema-test.json |  5 +
 tests/qapi-schema/qapi-schema-test.out  |  2 ++
 10 files changed, 38 insertions(+), 10 deletions(-)
 create mode 100644 tests/qapi-schema/enum-bad-prefix.err
 create mode 100644 tests/qapi-schema/enum-bad-prefix.exit
 create mode 100644 tests/qapi-schema/enum-bad-prefix.json
 create mode 100644 tests/qapi-schema/enum-bad-prefix.out

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index ff16df2..d960dc0 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -236,6 +236,7 @@ both fields like this:
 === Enumeration types ===
 
 Usage: { 'enum': STRING, 'data': ARRAY-OF-STRING }
+   { 'enum': STRING, '*prefix': STRING, 'data': ARRAY-OF-STRING }
 
 An enumeration type is a dictionary containing a single 'data' key
 whose value is a list of strings.  An example enumeration is:
@@ -247,6 +248,13 @@ useful.  The list of strings should be lower case; if an 
enum name
 represents multiple words, use '-' between words.  The string 'max' is
 not allowed as an enum value, and values should not be repeated.
 
+The enum constants will be named by using a heuristic to turn the
+type name into a set of underscore separated words. For the example
+above, 'MyEnum' will turn into 'MY_ENUM' giving a constant name
+of 'MY_ENUM_VALUE1' for the first value. If the default heuristic
+does not result in a desirable name, the optional 'prefix' field
+can be used when defining the enum.
+
 The enumeration values are passed as strings over the Client JSON
 Protocol, but are encoded as C enum integral values in generated code.
 While the C code starts numbering at 0, it is better to use explicit
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index f2428f3..a8453d1 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -101,20 +101,20 @@ struct %(name)s {
 
 return ret
 
-def generate_enum_lookup(name, values):
+def generate_enum_lookup(name, values, prefix=None):
 ret = mcgen('''
 
 const char *const %(name)s_lookup[] = {
 ''',
 name=c_name(name))
 for value in values:
-index = c_enum_const(name, value)
+index = c_enum_const(name, value, prefix)
 ret += mcgen('''
 [%(index)s] = "%(value)s",
 ''',
  index = index, value = value)
 
-max_index = c_enum_const(name, 'MAX')
+max_index = c_enum_const(name, 'MAX', prefix)
 ret += mcgen('''
 [%(max_index)s] = NULL,
 };
@@ -122,7 +122,7 @@ const char *const %(name)s_lookup[] = {
 max_index=max_index)
 return ret
 
-def generate_enum(name, values):
+def generate_enum(name, values, prefix=None):
 name = c_name(name)
 lookup_decl = mcgen('''
 
@@ -141,7 +141,7 @@ typedef enum %(name)s {
 
 i = 0
 for value in enum_values:
-enum_full_value = c_enum_const(name, value)
+enum_full_value = c_enum_const(name, value, prefix)
 enum_decl += mcgen('''
 %(enum_full_value)s = %(i)d,
 ''',
@@ -348,9 +348,11 @@ for expr in exprs:
 if expr.has_key('struct'):
 ret += generate_fwd_struct(expr['struct'])
 elif expr.has_key('enum'):
-ret += generate_enum(expr['enum'], expr['data'])
+ret += generate_enum(expr['enum'], expr['data'],
+ expr.get('prefix'))
 ret += generate_fwd_enum_struct(expr['enum'])
-fdef.write(generate_enum_lookup(expr['enum'], expr['data']))
+fdef.write(generate_enum_lookup(expr['enum'], expr['data'],
+expr.get('prefix')))
 elif expr.has_key('union'):
 ret += generate_fwd_struct(expr['union'])
 enum_define = discriminator_find_enum_define(expr)
diff --git a/scripts/qapi.py 

[Qemu-devel] [PATCH PULL v2 05/11] crypto: introduce new base module for TLS credentials

[Daniel P. Berrange]

Introduce a QCryptoTLSCreds class to act as the base class for
storing TLS credentials. This will be later subclassed to provide
handling of anonymous and x509 credential types. The subclasses
will be user creatable objects, so instances can be created &
deleted via 'object-add' and 'object-del' QMP commands respectively,
or via the -object command line arg.

If the credentials cannot be initialized an error will be reported
as a QMP reply, or on stderr respectively.

The idea is to make it possible to represent and manage TLS
credentials independently of the network service that is using
them. This will enable multiple services to use the same set of
credentials and minimize code duplication. A later patch will
convert the current VNC server TLS code over to use this object.

The representation of credentials will be functionally equivalent
to that currently implemented in the VNC server with one exception.
The new code has the ability to (optionally) load a pre-generated
set of diffie-hellman parameters, if the file dh-params.pem exists,
whereas the current VNC server will always generate them on startup.
This is beneficial for admins who wish to avoid the (small) time
sink of generating DH parameters at startup and/or avoid depleting
entropy.

Signed-off-by: Daniel P. Berrange 
---
 crypto/Makefile.objs  |   1 +
 crypto/tlscreds.c | 251 ++
 crypto/tlscredspriv.h |  42 
 include/crypto/tlscreds.h |  68 +
 qapi-schema.json  |   3 +
 qapi/crypto.json  |  21 
 tests/Makefile|   2 +-
 trace-events  |   4 +
 8 files changed, 391 insertions(+), 1 deletion(-)
 create mode 100644 crypto/tlscreds.c
 create mode 100644 crypto/tlscredspriv.h
 create mode 100644 include/crypto/tlscreds.h
 create mode 100644 qapi/crypto.json

diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 2cf5b70..ca90d1b 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -3,3 +3,4 @@ crypto-obj-y += hash.o
 crypto-obj-y += aes.o
 crypto-obj-y += desrfb.o
 crypto-obj-y += cipher.o
+crypto-obj-y += tlscreds.o
diff --git a/crypto/tlscreds.c b/crypto/tlscreds.c
new file mode 100644
index 000..5ec982c
--- /dev/null
+++ b/crypto/tlscreds.c
@@ -0,0 +1,251 @@
+/*
+ * QEMU crypto TLS credential support
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include "crypto/tlscredspriv.h"
+#include "trace.h"
+
+#define DH_BITS 2048
+
+#ifdef CONFIG_GNUTLS
+int
+qcrypto_tls_creds_get_dh_params_file(QCryptoTLSCreds *creds,
+ const char *filename,
+ gnutls_dh_params_t *dh_params,
+ Error **errp)
+{
+int ret;
+
+trace_qcrypto_tls_creds_load_dh(creds, filename ? filename : 
"");
+
+if (filename == NULL) {
+ret = gnutls_dh_params_init(dh_params);
+if (ret < 0) {
+error_setg(errp, "Unable to initialize DH parameters: %s",
+   gnutls_strerror(ret));
+return -1;
+}
+ret = gnutls_dh_params_generate2(*dh_params, DH_BITS);
+if (ret < 0) {
+gnutls_dh_params_deinit(*dh_params);
+*dh_params = NULL;
+error_setg(errp, "Unable to generate DH parameters: %s",
+   gnutls_strerror(ret));
+return -1;
+}
+} else {
+GError *gerr = NULL;
+gchar *contents;
+gsize len;
+gnutls_datum_t data;
+if (!g_file_get_contents(filename,
+ &contents,
+ &len,
+ &gerr)) {
+
+error_setg(errp, "%s", gerr->message);
+g_error_free(gerr);
+return -1;
+}
+data.data = (unsigned char *)contents;
+data.size = len;
+ret = gnutls_dh_params_init(dh_params);
+if (ret < 0) {
+g_free(contents);
+error_setg(errp, "Unable to initialize DH parameters: %s",
+   gnutls_strerror(ret));
+return -1;
+}
+ret = gnutls_dh_params_import_pkcs3(*dh_params,
+&data,
+ 

[Qemu-devel] [PATCH PULL v2 09/11] crypto: introduce new module for handling TLS sessions

[Daniel P. Berrange]

Introduce a QCryptoTLSSession object that will encapsulate
all the code for setting up and using a client/sever TLS
session. This isolates the code which depends on the gnutls
library, avoiding #ifdefs in the rest of the codebase, as
well as facilitating any possible future port to other TLS
libraries, if desired. It makes use of the previously
defined QCryptoTLSCreds object to access credentials to
use with the session. It also includes further unit tests
to validate the correctness of the TLS session handshake
and certificate validation. This is functionally equivalent
to the current TLS session handling code embedded in the
VNC server, and will obsolete it.

Signed-off-by: Daniel P. Berrange 
---
 crypto/Makefile.objs   |   1 +
 crypto/tlssession.c| 574 +
 include/crypto/tlssession.h| 322 +++
 tests/.gitignore   |   4 +
 tests/Makefile |   3 +
 tests/test-crypto-tlssession.c | 535 ++
 trace-events   |   3 +
 7 files changed, 1442 insertions(+)
 create mode 100644 crypto/tlssession.c
 create mode 100644 include/crypto/tlssession.h
 create mode 100644 tests/test-crypto-tlssession.c

diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 5894c88..5567a51 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -6,3 +6,4 @@ crypto-obj-y += cipher.o
 crypto-obj-y += tlscreds.o
 crypto-obj-y += tlscredsanon.o
 crypto-obj-y += tlscredsx509.o
+crypto-obj-y += tlssession.o
diff --git a/crypto/tlssession.c b/crypto/tlssession.c
new file mode 100644
index 000..ffc5c47
--- /dev/null
+++ b/crypto/tlssession.c
@@ -0,0 +1,574 @@
+/*
+ * QEMU crypto TLS session support
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include "crypto/tlssession.h"
+#include "crypto/tlscredsanon.h"
+#include "crypto/tlscredsx509.h"
+#include "qemu/acl.h"
+#include "trace.h"
+
+#ifdef CONFIG_GNUTLS
+
+
+#include 
+
+
+struct QCryptoTLSSession {
+QCryptoTLSCreds *creds;
+gnutls_session_t handle;
+char *hostname;
+char *aclname;
+bool handshakeComplete;
+QCryptoTLSSessionWriteFunc writeFunc;
+QCryptoTLSSessionReadFunc readFunc;
+void *opaque;
+char *peername;
+};
+
+
+void
+qcrypto_tls_session_free(QCryptoTLSSession *session)
+{
+if (!session) {
+return;
+}
+
+gnutls_deinit(session->handle);
+g_free(session->hostname);
+g_free(session->peername);
+g_free(session->aclname);
+object_unref(OBJECT(session->creds));
+g_free(session);
+}
+
+
+static ssize_t
+qcrypto_tls_session_push(void *opaque, const void *buf, size_t len)
+{
+QCryptoTLSSession *session = opaque;
+
+if (!session->writeFunc) {
+errno = EIO;
+return -1;
+};
+
+return session->writeFunc(buf, len, session->opaque);
+}
+
+
+static ssize_t
+qcrypto_tls_session_pull(void *opaque, void *buf, size_t len)
+{
+QCryptoTLSSession *session = opaque;
+
+if (!session->readFunc) {
+errno = EIO;
+return -1;
+};
+
+return session->readFunc(buf, len, session->opaque);
+}
+
+
+QCryptoTLSSession *
+qcrypto_tls_session_new(QCryptoTLSCreds *creds,
+const char *hostname,
+const char *aclname,
+QCryptoTLSCredsEndpoint endpoint,
+Error **errp)
+{
+QCryptoTLSSession *session;
+int ret;
+
+session = g_new0(QCryptoTLSSession, 1);
+trace_qcrypto_tls_session_new(
+session, creds, hostname ? hostname : "",
+aclname ? aclname : "", endpoint);
+
+if (hostname) {
+session->hostname = g_strdup(hostname);
+}
+if (aclname) {
+session->aclname = g_strdup(aclname);
+}
+session->creds = creds;
+object_ref(OBJECT(creds));
+
+if (creds->endpoint != endpoint) {
+error_setg(errp, "Credentials endpoint doesn't match session");
+goto error;
+}
+
+if (endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
+ret = gnutls_init(&session->handle, GNUTLS_SERVER);
+} else {
+ret = gnutls_init(&session->handle, GNUTLS_CLIENT);
+}
+if (ret < 0) {
+error_setg(errp, "Cannot initialize TLS session: %s",
+   gn

[Qemu-devel] [PATCH PULL v2 02/11] tests: remove repetition in unit test object deps

[Daniel P. Berrange]

Most of the unit tests have identical sets of object deps.
For example all block unit tests need to depend on

 $(block-obj-y) libqemuutil.a libqemustub.a

Currently each unit test repeats this list of test deps.
This list of deps will grow as future patches add more
modules to the build, so define some common variables
that can be used by all unit tests to remove the
repetition.

Signed-off-by: Daniel P. Berrange 
---
 tests/Makefile | 96 +++---
 1 file changed, 51 insertions(+), 45 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index 1951b1d..0fcf148 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -1,5 +1,7 @@
 export SRC_PATH
 
+qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
+
 # Get the list of all supported sysemu targets
 SYSEMU_TARGET_LIST := $(subst -softmmu.mak,,$(notdir \
$(wildcard $(SRC_PATH)/default-configs/*-softmmu.mak)))
@@ -276,47 +278,51 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o 
tests/check-qdict.o \
tests/test-opts-visitor.o tests/test-qmp-event.o \
tests/rcutorture.o tests/test-rcu-list.o
 
-test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \
- tests/test-qapi-event.o
-
 $(test-obj-y): QEMU_INCLUDES += -Itests
 QEMU_CFLAGS += -I$(SRC_PATH)/tests
-qom-core-obj = qom/object.o qom/qom-qobject.o qom/container.o 
qom/object_interfaces.o
-
-tests/check-qint$(EXESUF): tests/check-qint.o libqemuutil.a
-tests/check-qstring$(EXESUF): tests/check-qstring.o libqemuutil.a
-tests/check-qdict$(EXESUF): tests/check-qdict.o libqemuutil.a
-tests/check-qlist$(EXESUF): tests/check-qlist.o libqemuutil.a
-tests/check-qfloat$(EXESUF): tests/check-qfloat.o libqemuutil.a
-tests/check-qjson$(EXESUF): tests/check-qjson.o libqemuutil.a libqemustub.a
-tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o 
$(qom-core-obj) libqemuutil.a libqemustub.a
-tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o $(qom-core-obj) 
libqemuutil.a libqemustub.a
-tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(block-obj-y) 
libqemuutil.a libqemustub.a
-tests/test-aio$(EXESUF): tests/test-aio.o $(block-obj-y) libqemuutil.a 
libqemustub.a
-tests/test-rfifolock$(EXESUF): tests/test-rfifolock.o libqemuutil.a 
libqemustub.a
-tests/test-throttle$(EXESUF): tests/test-throttle.o $(block-obj-y) 
libqemuutil.a libqemustub.a
-tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(block-obj-y) 
libqemuutil.a libqemustub.a
-tests/test-iov$(EXESUF): tests/test-iov.o libqemuutil.a
-tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o libqemuutil.a libqemustub.a
+
+
+# Deps that are common to various different sets of tests below
+test-util-obj-y = libqemuutil.a libqemustub.a
+test-qom-obj-y = qom/object.o qom/qom-qobject.o \
+   qom/container.o qom/object_interfaces.o \
+   $(test-util-obj-y)
+test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \
+   tests/test-qapi-event.o \
+   $(test-qom-obj-y)
+test-block-obj-y = $(block-obj-y) $(test-util-obj-y)
+
+tests/check-qint$(EXESUF): tests/check-qint.o $(test-util-obj-y)
+tests/check-qstring$(EXESUF): tests/check-qstring.o $(test-util-obj-y)
+tests/check-qdict$(EXESUF): tests/check-qdict.o $(test-util-obj-y)
+tests/check-qlist$(EXESUF): tests/check-qlist.o $(test-util-obj-y)
+tests/check-qfloat$(EXESUF): tests/check-qfloat.o $(test-util-obj-y)
+tests/check-qjson$(EXESUF): tests/check-qjson.o $(test-util-obj-y)
+tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o 
$(test-qom-obj-y)
+tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o $(test-qom-obj-y)
+tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(test-block-obj-y)
+tests/test-aio$(EXESUF): tests/test-aio.o $(test-block-obj-y)
+tests/test-rfifolock$(EXESUF): tests/test-rfifolock.o $(test-util-obj-y)
+tests/test-throttle$(EXESUF): tests/test-throttle.o $(test-block-obj-y)
+tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(test-block-obj-y)
+tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y)
+tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y)
 tests/test-x86-cpuid$(EXESUF): tests/test-x86-cpuid.o
-tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o 
page_cache.o libqemuutil.a
+tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o migration/xbzrle.o 
page_cache.o $(test-util-obj-y)
 tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o
 tests/test-int128$(EXESUF): tests/test-int128.o
-tests/rcutorture$(EXESUF): tests/rcutorture.o libqemuutil.a libqemustub.a
-tests/test-rcu-list$(EXESUF): tests/test-rcu-list.o libqemuutil.a libqemustub.a
+tests/rcutorture$(EXESUF): tests/rcutorture.o $(test-util-obj-y)
+tests/test-rcu-list$(EXESUF): tests/test-rcu-list.o $(test-util-obj-y)
 
 tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
hw/core/qdev.o hw/core/qdev-properties.o hw/core/hotplug.o\
hw/core/irq.o \
hw/core/

[Qemu-devel] [PATCH PULL v2 08/11] crypto: add sanity checking of TLS x509 credentials

[Daniel P. Berrange]

If the administrator incorrectly sets up their x509 certificates,
the errors seen at runtime during connection attempts are very
obscure and difficult to diagnose. This has been a particular
problem for people using openssl to generate their certificates
instead of the gnutls certtool, because the openssl tools don't
turn on the various x509 extensions that gnutls expects to be
present by default.

This change thus adds support in the TLS credentials object to
sanity check the certificates when QEMU first loads them. This
gives the administrator immediate feedback for the majority of
common configuration mistakes, reducing the pain involved in
setting up TLS. The code is derived from equivalent code that
has been part of libvirt's TLS support and has been seen to be
valuable in assisting admins.

It is possible to disable the sanity checking, however, via
the new 'sanity-check' property on the tls-creds object type,
with a value of 'no'.

Unit tests are included in this change to verify the correctness
of the sanity checking code in all the key scenarios it is
intended to cope with. As part of the test suite, the pkix_asn1_tab.c
from gnutls is imported. This file is intentionally copied from the
(long since obsolete) gnutls 1.6.3 source tree, since that version
was still under GPLv2+, rather than the GPLv3+ of gnutls >= 2.0.

Signed-off-by: Daniel P. Berrange 
---
 configure|   22 +
 crypto/tlscredsx509.c|  546 +++
 include/crypto/tlscredsx509.h|1 +
 tests/.gitignore |3 +
 tests/Makefile   |5 +
 tests/crypto-tls-x509-helpers.c  |  485 +
 tests/crypto-tls-x509-helpers.h  |  133 +
 tests/pkix_asn1_tab.c| 1104 ++
 tests/test-crypto-tlscredsx509.c |  731 +
 trace-events |5 +
 10 files changed, 3035 insertions(+)
 create mode 100644 tests/crypto-tls-x509-helpers.c
 create mode 100644 tests/crypto-tls-x509-helpers.h
 create mode 100644 tests/pkix_asn1_tab.c
 create mode 100644 tests/test-crypto-tlscredsx509.c

diff --git a/configure b/configure
index d7c24cd..bdd302c 100755
--- a/configure
+++ b/configure
@@ -416,6 +416,9 @@ if test "$debug_info" = "yes"; then
 LDFLAGS="-g $LDFLAGS"
 fi
 
+test_cflags=""
+test_libs=""
+
 # make source path absolute
 source_path=`cd "$source_path"; pwd`
 
@@ -2249,6 +2252,19 @@ if test "$gnutls_nettle" != "no"; then
 fi
 fi
 
+##
+# libtasn1 - only for the TLS creds/session test suite
+
+tasn1=yes
+if $pkg_config --exists "libtasn1"; then
+tasn1_cflags=`$pkg_config --cflags libtasn1`
+tasn1_libs=`$pkg_config --libs libtasn1`
+test_cflags="$test_cflags $tasn1_cflags"
+test_libs="$test_libs $tasn1_libs"
+else
+tasn1=no
+fi
+
 
 ##
 # VTE probe
@@ -4574,6 +4590,7 @@ echo "GNUTLS support$gnutls"
 echo "GNUTLS hash   $gnutls_hash"
 echo "GNUTLS gcrypt $gnutls_gcrypt"
 echo "GNUTLS nettle $gnutls_nettle ${gnutls_nettle+($nettle_version)}"
+echo "libtasn1  $tasn1"
 echo "VTE support   $vte"
 echo "curses support$curses"
 echo "curl support  $curl"
@@ -4945,6 +4962,9 @@ if test "$gnutls_nettle" = "yes" ; then
   echo "CONFIG_GNUTLS_NETTLE=y" >> $config_host_mak
   echo "CONFIG_NETTLE_VERSION_MAJOR=${nettle_version%%.*}" >> $config_host_mak
 fi
+if test "$tasn1" = "yes" ; then
+  echo "CONFIG_TASN1=y" >> $config_host_mak
+fi
 if test "$vte" = "yes" ; then
   echo "CONFIG_VTE=y" >> $config_host_mak
   echo "VTE_CFLAGS=$vte_cflags" >> $config_host_mak
@@ -5268,6 +5288,8 @@ echo "EXESUF=$EXESUF" >> $config_host_mak
 echo "DSOSUF=$DSOSUF" >> $config_host_mak
 echo "LDFLAGS_SHARED=$LDFLAGS_SHARED" >> $config_host_mak
 echo "LIBS_QGA+=$libs_qga" >> $config_host_mak
+echo "TEST_LIBS=$test_libs" >> $config_host_mak
+echo "TEST_CFLAGS=$test_cflags" >> $config_host_mak
 echo "POD2MAN=$POD2MAN" >> $config_host_mak
 echo "TRANSLATE_OPT_CFLAGS=$TRANSLATE_OPT_CFLAGS" >> $config_host_mak
 if test "$gcov" = "yes" ; then
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 6cc7b53..dc46bc4 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -26,6 +26,516 @@
 
 #ifdef CONFIG_GNUTLS
 
+#include 
+
+
+static int
+qcrypto_tls_creds_check_cert_times(gnutls_x509_crt_t cert,
+   const char *certFile,
+   bool isServer,
+   bool isCA,
+   Error **errp)
+{
+time_t now = time(NULL);
+
+if (now == ((time_t)-1)) {
+error_setg_errno(errp, errno, "cannot get current time");
+return -1;
+}
+
+if (gnutls_x509_crt_get_expiration_time(cert) < now) {
+error_setg(errp,
+   (isCA ?
+"The CA certificate %s has expired" :
+(isSer

[Qemu-devel] [PATCH PULL v2 10/11] ui: fix return type for VNC I/O functions to be ssize_t

[Daniel P. Berrange]

Various VNC server I/O functions return 'long' and then
also pass this to a method accepting 'int'. All these
should be ssize_t to match the signature of read/write
APIs and thus avoid potential for integer truncation /
wraparound.

Signed-off-by: Daniel P. Berrange 
Reviewed-by: Eric Blake 
---
 ui/vnc.c | 36 ++--
 ui/vnc.h |  6 +++---
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index caf82f5..19ad088 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1268,7 +1268,7 @@ void vnc_disconnect_finish(VncState *vs)
 g_free(vs);
 }
 
-int vnc_client_io_error(VncState *vs, int ret, int last_errno)
+ssize_t vnc_client_io_error(VncState *vs, ssize_t ret, int last_errno)
 {
 if (ret == 0 || ret == -1) {
 if (ret == -1) {
@@ -1284,7 +1284,7 @@ int vnc_client_io_error(VncState *vs, int ret, int 
last_errno)
 }
 }
 
-VNC_DEBUG("Closing down client sock: ret %d, errno %d\n",
+VNC_DEBUG("Closing down client sock: ret %zd, errno %d\n",
   ret, ret < 0 ? last_errno : 0);
 vnc_disconnect_start(vs);
 
@@ -1301,11 +1301,11 @@ void vnc_client_error(VncState *vs)
 }
 
 #ifdef CONFIG_VNC_TLS
-static long vnc_client_write_tls(gnutls_session_t *session,
- const uint8_t *data,
- size_t datalen)
+static ssize_t vnc_client_write_tls(gnutls_session_t *session,
+const uint8_t *data,
+size_t datalen)
 {
-long ret = gnutls_write(*session, data, datalen);
+ssize_t ret = gnutls_write(*session, data, datalen);
 if (ret < 0) {
 if (ret == GNUTLS_E_AGAIN) {
 errno = EAGAIN;
@@ -1333,9 +1333,9 @@ static long vnc_client_write_tls(gnutls_session_t 
*session,
  * the requested 'datalen' if the socket would block. Returns
  * -1 on error, and disconnects the client socket.
  */
-long vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen)
+ssize_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen)
 {
-long ret;
+ssize_t ret;
 #ifdef CONFIG_VNC_TLS
 if (vs->tls.session) {
 ret = vnc_client_write_tls(&vs->tls.session, data, datalen);
@@ -1360,9 +1360,9 @@ long vnc_client_write_buf(VncState *vs, const uint8_t 
*data, size_t datalen)
  * the buffered output data if the socket would block. Returns
  * -1 on error, and disconnects the client socket.
  */
-static long vnc_client_write_plain(VncState *vs)
+static ssize_t vnc_client_write_plain(VncState *vs)
 {
-long ret;
+ssize_t ret;
 
 #ifdef CONFIG_VNC_SASL
 VNC_DEBUG("Write Plain: Pending output %p size %zd offset %zd. Wait SSF 
%d\n",
@@ -1436,10 +1436,10 @@ void vnc_read_when(VncState *vs, VncReadEvent *func, 
size_t expecting)
 }
 
 #ifdef CONFIG_VNC_TLS
-static long vnc_client_read_tls(gnutls_session_t *session, uint8_t *data,
-size_t datalen)
+static ssize_t vnc_client_read_tls(gnutls_session_t *session, uint8_t *data,
+   size_t datalen)
 {
-long ret = gnutls_read(*session, data, datalen);
+ssize_t ret = gnutls_read(*session, data, datalen);
 if (ret < 0) {
 if (ret == GNUTLS_E_AGAIN) {
 errno = EAGAIN;
@@ -1467,9 +1467,9 @@ static long vnc_client_read_tls(gnutls_session_t 
*session, uint8_t *data,
  * the requested 'datalen' if the socket would block. Returns
  * -1 on error, and disconnects the client socket.
  */
-long vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen)
+ssize_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen)
 {
-long ret;
+ssize_t ret;
 #ifdef CONFIG_VNC_TLS
 if (vs->tls.session) {
 ret = vnc_client_read_tls(&vs->tls.session, data, datalen);
@@ -1492,9 +1492,9 @@ long vnc_client_read_buf(VncState *vs, uint8_t *data, 
size_t datalen)
  * Returns the number of bytes read. Returns -1 on error, and
  * disconnects the client socket.
  */
-static long vnc_client_read_plain(VncState *vs)
+static ssize_t vnc_client_read_plain(VncState *vs)
 {
-int ret;
+ssize_t ret;
 VNC_DEBUG("Read plain %p size %zd offset %zd\n",
   vs->input.buffer, vs->input.capacity, vs->input.offset);
 buffer_reserve(&vs->input, 4096);
@@ -1520,7 +1520,7 @@ static void vnc_jobs_bh(void *opaque)
 void vnc_client_read(void *opaque)
 {
 VncState *vs = opaque;
-long ret;
+ssize_t ret;
 
 #ifdef CONFIG_VNC_SASL
 if (vs->sasl.conn && vs->sasl.runSSF)
diff --git a/ui/vnc.h b/ui/vnc.h
index 814d720..194ccf7 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -513,8 +513,8 @@ enum {
 void vnc_client_read(void *opaque);
 void vnc_client_write(void *opaque);
 
-long vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen);
-long vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen);
+ssize_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t dat

[Qemu-devel] [PATCH PULL v2 03/11] crypto: move crypto objects out of libqemuutil.la

[Daniel P. Berrange]

Future patches will be adding more crypto related APIs which
rely on QOM infrastructure. This creates a problem, because
QOM relies on library constructors to register objects. When
you have a file in a static .a library though which is only
referenced by a constructor the linker is dumb and will drop
that file when linking to the final executable :-( The only
workaround for this is to link the .a library to the executable
using the -Wl,--whole-archive flag, but this creates its own
set of problems because QEMU is relying on lazy linking for
libqemuutil.a. Using --whole-archive majorly increases the
size of final executables as they now contain a bunch of
object code they don't actually use.

The least bad option is to thus not include the crypto objects
in libqemuutil.la, and instead define a crypto-obj-y variable
that is referenced directly by all the executables that need
this code (tools + softmmu, but not qemu-ga).

Signed-off-by: Daniel P. Berrange 
---
 Makefile |  8 +---
 Makefile.objs|  5 -
 Makefile.target  |  2 ++
 crypto/Makefile.objs | 10 +-
 tests/Makefile   |  7 ---
 5 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/Makefile b/Makefile
index 9ce3972..bb06c4f 100644
--- a/Makefile
+++ b/Makefile
@@ -152,6 +152,7 @@ dummy := $(call unnest-vars,, \
 qga-vss-dll-obj-y \
 block-obj-y \
 block-obj-m \
+crypto-obj-y \
 common-obj-y \
 common-obj-m)
 
@@ -173,6 +174,7 @@ SUBDIR_RULES=$(patsubst %,subdir-%, $(TARGET_DIRS))
 SOFTMMU_SUBDIR_RULES=$(filter %-softmmu,$(SUBDIR_RULES))
 
 $(SOFTMMU_SUBDIR_RULES): $(block-obj-y)
+$(SOFTMMU_SUBDIR_RULES): $(crypto-obj-y)
 $(SOFTMMU_SUBDIR_RULES): config-all-devices.mak
 
 subdir-%:
@@ -227,9 +229,9 @@ util/module.o-cflags = 
-D'CONFIG_BLOCK_MODULES=$(block-modules)'
 
 qemu-img.o: qemu-img-cmds.h
 
-qemu-img$(EXESUF): qemu-img.o $(block-obj-y) libqemuutil.a libqemustub.a
-qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) libqemuutil.a libqemustub.a
-qemu-io$(EXESUF): qemu-io.o $(block-obj-y) libqemuutil.a libqemustub.a
+qemu-img$(EXESUF): qemu-img.o $(block-obj-y) $(crypto-obj-y) libqemuutil.a 
libqemustub.a
+qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) $(crypto-obj-y) libqemuutil.a 
libqemustub.a
+qemu-io$(EXESUF): qemu-io.o $(block-obj-y) $(crypto-obj-y) libqemuutil.a 
libqemustub.a
 
 qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
 
diff --git a/Makefile.objs b/Makefile.objs
index f094eff..c7ed989 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -2,7 +2,6 @@
 # Common libraries for tools and emulators
 stub-obj-y = stubs/
 util-obj-y = util/ qobject/ qapi/ qapi-types.o qapi-visit.o qapi-event.o
-util-obj-y += crypto/
 
 ###
 # block-obj-y is code used by both qemu system emulation and qemu-img
@@ -21,6 +20,10 @@ block-obj-y += coroutine-$(CONFIG_COROUTINE_BACKEND).o
 
 block-obj-m = block/
 
+###
+# crypto-obj-y is code used by both qemu system emulation and qemu-img
+
+crypto-obj-y = crypto/
 
 ##
 # smartcard
diff --git a/Makefile.target b/Makefile.target
index dc32294..8235202 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -170,12 +170,14 @@ target-obj-y-save := $(target-obj-y)
 dummy := $(call unnest-vars,.., \
block-obj-y \
block-obj-m \
+   crypto-obj-y \
common-obj-y \
common-obj-m)
 target-obj-y := $(target-obj-y-save)
 all-obj-y += $(common-obj-y)
 all-obj-y += $(target-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y)
+all-obj-$(CONFIG_SOFTMMU) += $(crypto-obj-y)
 
 $(QEMU_PROG_BUILD): config-devices.mak
 
diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index b050138..2cf5b70 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -1,5 +1,5 @@
-util-obj-y += init.o
-util-obj-y += hash.o
-util-obj-y += aes.o
-util-obj-y += desrfb.o
-util-obj-y += cipher.o
+crypto-obj-y = init.o
+crypto-obj-y += hash.o
+crypto-obj-y += aes.o
+crypto-obj-y += desrfb.o
+crypto-obj-y += cipher.o
diff --git a/tests/Makefile b/tests/Makefile
index 0fcf148..d77ccee 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -290,7 +290,8 @@ test-qom-obj-y = qom/object.o qom/qom-qobject.o \
 test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \
tests/test-qapi-event.o \
$(test-qom-obj-y)
-test-block-obj-y = $(block-obj-y) $(test-util-obj-y)
+test-crypto-obj-y = $(crypto-obj-y) $(test-util-obj-y)
+test-block-obj-y = $(block-obj-y) $(test-crypto-obj-y)
 
 tests/check-qint$(EXESUF): tests/check-qint.o $(test-util-obj-y)
 tests/check-qstring$(EXESUF): tests/check-qstring.o $(test-util-obj-y)
@@ -357,8 +358,8 @@ tests/test-opts-visitor$(EXESUF): tests/test-opts-visitor.o 
$(test-qapi-obj-y)
 
 tests

[Qemu-devel] [PATCH PULL v2 06/11] crypto: introduce new module for TLS anonymous credentials

[Daniel P. Berrange]

Introduce a QCryptoTLSCredsAnon class which is used to
manage anonymous TLS credentials. Use of this class is
generally discouraged since it does not offer strong
security, but it is required for backwards compatibility
with the current VNC server implementation.

Simple example CLI configuration:

 $QEMU -object tls-creds-anon,id=tls0,endpoint=server

Example using pre-created diffie-hellman parameters

 $QEMU -object tls-creds-anon,id=tls0,endpoint=server,\
   dir=/path/to/creds/dir

The 'id' value in the -object args will be used to associate the
credentials with the network services. For example, when the VNC
server is later converted it would use

 $QEMU -object tls-creds-anon,id=tls0, \
   -vnc 127.0.0.1:1,tls-creds=tls0

Signed-off-by: Daniel P. Berrange 
Reviewed-by: Eric Blake 
---
 crypto/Makefile.objs  |   1 +
 crypto/tlscredsanon.c | 223 ++
 include/crypto/tlscredsanon.h | 112 +
 qemu-options.hx   |  20 
 trace-events  |   3 +
 5 files changed, 359 insertions(+)
 create mode 100644 crypto/tlscredsanon.c
 create mode 100644 include/crypto/tlscredsanon.h

diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index ca90d1b..2080c5d 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -4,3 +4,4 @@ crypto-obj-y += aes.o
 crypto-obj-y += desrfb.o
 crypto-obj-y += cipher.o
 crypto-obj-y += tlscreds.o
+crypto-obj-y += tlscredsanon.o
diff --git a/crypto/tlscredsanon.c b/crypto/tlscredsanon.c
new file mode 100644
index 000..c3fcdaf
--- /dev/null
+++ b/crypto/tlscredsanon.c
@@ -0,0 +1,223 @@
+/*
+ * QEMU crypto TLS anonymous credential support
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include "crypto/tlscredsanon.h"
+#include "crypto/tlscredspriv.h"
+#include "qom/object_interfaces.h"
+#include "trace.h"
+
+
+#ifdef CONFIG_GNUTLS
+
+
+static int
+qcrypto_tls_creds_anon_load(QCryptoTLSCredsAnon *creds,
+Error **errp)
+{
+char *dhparams = NULL;
+int ret;
+int rv = -1;
+
+trace_qcrypto_tls_creds_anon_load(creds,
+creds->parent_obj.dir ? creds->parent_obj.dir : "");
+
+if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER) {
+if (qcrypto_tls_creds_get_path(&creds->parent_obj,
+   QCRYPTO_TLS_CREDS_DH_PARAMS,
+   false, &dhparams, errp) < 0) {
+goto cleanup;
+}
+
+ret = gnutls_anon_allocate_server_credentials(&creds->data.server);
+if (ret < 0) {
+error_setg(errp, "Cannot allocate credentials: %s",
+   gnutls_strerror(ret));
+goto cleanup;
+}
+
+if (qcrypto_tls_creds_get_dh_params_file(&creds->parent_obj, dhparams,
+ &creds->parent_obj.dh_params,
+ errp) < 0) {
+goto cleanup;
+}
+
+gnutls_anon_set_server_dh_params(creds->data.server,
+ creds->parent_obj.dh_params);
+} else {
+ret = gnutls_anon_allocate_client_credentials(&creds->data.client);
+if (ret < 0) {
+error_setg(errp, "Cannot allocate credentials: %s",
+   gnutls_strerror(ret));
+goto cleanup;
+}
+}
+
+rv = 0;
+ cleanup:
+g_free(dhparams);
+return rv;
+}
+
+
+static void
+qcrypto_tls_creds_anon_unload(QCryptoTLSCredsAnon *creds)
+{
+if (creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT) {
+if (creds->data.client) {
+gnutls_anon_free_client_credentials(creds->data.client);
+creds->data.client = NULL;
+}
+} else {
+if (creds->data.server) {
+gnutls_anon_free_server_credentials(creds->data.server);
+creds->data.server = NULL;
+}
+}
+if (creds->parent_obj.dh_params) {
+gnutls_dh_params_deinit(creds->parent_obj.dh_params);
+creds->parent_obj.dh_params = NULL;
+}
+}
+
+#else /* ! CONFIG_GNUTLS */
+
+
+static void
+qcrypto_tls_creds_anon_load(QCryptoTLSCredsAnon *creds G_GNUC_UNUSED,
+Err

[Qemu-devel] [PATCH PULL v2 11/11] ui: convert VNC server to use QCryptoTLSSession

[Daniel P. Berrange]

Switch VNC server over to using the QCryptoTLSSession object
for the TLS session. This removes the direct use of gnutls
from the VNC server code. It also removes most knowledge
about TLS certificate handling from the VNC server code.
This has the nice effect that all the CONFIG_VNC_TLS
conditionals go away and the user gets an actual error
message when requesting TLS instead of it being silently
ignored.

With this change, the existing configuration options for
enabling TLS with -vnc are deprecated.

Old syntax for anon-DH credentials:

  -vnc hostname:0,tls

New syntax:

  -object tls-creds-anon,id=tls0,endpoint=server \
  -vnc hostname:0,tls-creds=tls0

Old syntax for x509 credentials, no client certs:

  -vnc hostname:0,tls,x509=/path/to/certs

New syntax:

  -object 
tls-creds-x509,id=tls0,dir=/path/to/certs,endpoint=server,verify-peer=no \
  -vnc hostname:0,tls-creds=tls0

Old syntax for x509 credentials, requiring client certs:

  -vnc hostname:0,tls,x509verify=/path/to/certs

New syntax:

  -object 
tls-creds-x509,id=tls0,dir=/path/to/certs,endpoint=server,verify-peer=yes \
  -vnc hostname:0,tls-creds=tls0

This aligns VNC with the way TLS credentials are to be
configured in the future for chardev, nbd and migration
backends. It also has the benefit that the same TLS
credentials can be shared across multiple VNC server
instances, if desired.

If someone uses the deprecated syntax, it will internally
result in the creation of a 'tls-creds' object with an ID
based on the VNC server ID. This allows backwards compat
with the CLI syntax, while still deleting all the original
TLS code from the VNC server.

Signed-off-by: Daniel P. Berrange 
---
 configure  |  31 
 qemu-options.hx|  28 ++-
 ui/Makefile.objs   |   2 +-
 ui/vnc-auth-sasl.c |  36 ++--
 ui/vnc-auth-vencrypt.c |  80 +
 ui/vnc-tls.c   | 474 -
 ui/vnc-tls.h   |  69 ---
 ui/vnc-ws.c|  84 +
 ui/vnc-ws.h|   2 -
 ui/vnc.c   | 340 ++-
 ui/vnc.h   |  15 +-
 11 files changed, 362 insertions(+), 799 deletions(-)
 delete mode 100644 ui/vnc-tls.c
 delete mode 100644 ui/vnc-tls.h

diff --git a/configure b/configure
index bdd302c..018ba0b 100755
--- a/configure
+++ b/configure
@@ -242,7 +242,6 @@ vnc="yes"
 sparse="no"
 uuid=""
 vde=""
-vnc_tls=""
 vnc_sasl=""
 vnc_jpeg=""
 vnc_png=""
@@ -883,10 +882,6 @@ for opt do
   ;;
   --disable-strip) strip_opt="no"
   ;;
-  --disable-vnc-tls) vnc_tls="no"
-  ;;
-  --enable-vnc-tls) vnc_tls="yes"
-  ;;
   --disable-vnc-sasl) vnc_sasl="no"
   ;;
   --enable-vnc-sasl) vnc_sasl="yes"
@@ -2409,28 +2404,6 @@ EOF
   fi
 fi
 
-##
-# VNC TLS/WS detection
-if test "$vnc" = "yes" -a "$vnc_tls" != "no" ; then
-  cat > $TMPC <
-int main(void) { gnutls_session_t s; gnutls_init(&s, GNUTLS_SERVER); return 0; 
}
-EOF
-  vnc_tls_cflags=`$pkg_config --cflags gnutls 2> /dev/null`
-  vnc_tls_libs=`$pkg_config --libs gnutls 2> /dev/null`
-  if compile_prog "$vnc_tls_cflags" "$vnc_tls_libs" ; then
-if test "$vnc_tls" != "no" ; then
-  vnc_tls=yes
-fi
-libs_softmmu="$vnc_tls_libs $libs_softmmu"
-QEMU_CFLAGS="$QEMU_CFLAGS $vnc_tls_cflags"
-  else
-if test "$vnc_tls" = "yes" ; then
-  feature_not_found "vnc-tls" "Install gnutls devel"
-fi
-vnc_tls=no
-  fi
-fi
 
 ##
 # VNC SASL detection
@@ -4601,7 +4574,6 @@ echo "Block whitelist (ro) $block_drv_ro_whitelist"
 echo "VirtFS support$virtfs"
 echo "VNC support   $vnc"
 if test "$vnc" = "yes" ; then
-echo "VNC TLS support   $vnc_tls"
 echo "VNC SASL support  $vnc_sasl"
 echo "VNC JPEG support  $vnc_jpeg"
 echo "VNC PNG support   $vnc_png"
@@ -4810,9 +4782,6 @@ echo "CONFIG_BDRV_RO_WHITELIST=$block_drv_ro_whitelist" 
>> $config_host_mak
 if test "$vnc" = "yes" ; then
   echo "CONFIG_VNC=y" >> $config_host_mak
 fi
-if test "$vnc_tls" = "yes" ; then
-  echo "CONFIG_VNC_TLS=y" >> $config_host_mak
-fi
 if test "$vnc_sasl" = "yes" ; then
   echo "CONFIG_VNC_SASL=y" >> $config_host_mak
 fi
diff --git a/qemu-options.hx b/qemu-options.hx
index 3f2e25b..7e147b8 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1217,8 +1217,9 @@ By definition the Websocket port is 5700+@var{display}. 
If @var{host} is
 specified connections will only be allowed from this host.
 As an alternative the Websocket port could be specified by using
 @code{websocket}=@var{port}.
-TLS encryption for the Websocket connection is supported if the required
-certificates are specified with the VNC option @option{x509}.
+If no TLS credentials are provided, the websocket connection runs in
+unencrypted mode. If TLS credentials are provided, the websocket connection
+requires encrypted client connections.
 
 @item password
 
@@ -1239,6 +1240,20 @@ date and time).
 You can also use keywords "now" or "never

Re: [Qemu-devel] [PATCH v3] ppc/spapr: Implement H_RANDOM hypercall in QEMU

[Amit Shah]

On (Tue) 15 Sep 2015 [14:26:06], David Gibson wrote:
> On Mon, Sep 14, 2015 at 08:32:36AM +0200, Thomas Huth wrote:
> > On 14/09/15 04:15, David Gibson wrote:
> > > On Fri, Sep 11, 2015 at 11:17:01AM +0200, Thomas Huth wrote:
> > >> The PAPR interface defines a hypercall to pass high-quality
> > >> hardware generated random numbers to guests. Recent kernels can
> > >> already provide this hypercall to the guest if the right hardware
> > >> random number generator is available. But in case the user wants
> > >> to use another source like EGD, or QEMU is running with an older
> > >> kernel, we should also have this call in QEMU, so that guests that
> > >> do not support virtio-rng yet can get good random numbers, too.
> > >>
> > >> This patch now adds a new pseude-device to QEMU that either
> > >> directly provides this hypercall to the guest or is able to
> > >> enable the in-kernel hypercall if available. The in-kernel
> > >> hypercall can be enabled with the use-kvm property, e.g.:
> > >>
> > >>  qemu-system-ppc64 -device spapr-rng,use-kvm=true
> > >>
> > >> For handling the hypercall in QEMU instead, a RngBackend is required
> > >> since the hypercall should provide "good" random data instead of
> > >> pseudo-random (like from a "simple" library function like rand()
> > >> or g_random_int()). Since there are multiple RngBackends available,
> > >> the user must select an appropriate backend via the "backend"
> > >> property of the device, e.g.:
> > >>
> > >>  qemu-system-ppc64 -object rng-random,filename=/dev/hwrng,id=rng0 \
> > >>-device spapr-rng,backend=rng0 ...

(snip)

> > > More importantly, this should probably be called "rng" not "backend"
> > > to match virtio-rng.
> > 
> > Since the device is already called "spapr-rng", i.e. has "rng" in its
> > name, I'd rather like to keep this as "backend" to make it clear that
> > you specify the backend this way.
> 
> Hm, personally I'd weigh consistency with virtio-rng higher than the
> slightly confusing name.

Agreed.

Amit

Re: [Qemu-devel] [PATCH 6/9] hmp: added local apic dump state

[Paolo Bonzini]

On 15/09/2015 11:23, Denis V. Lunev wrote:
> From: Pavel Butsykin 
> 
> Added the hmp command to query local apic registers state, may be
> usefull after guest crashes to understand IRQ routing in guest.
> 
> For command name uses "apic-local" because it has to be grouped with
> command "apic-io".

I would call them "info lapic" and "info ioapic"
> 
> (qemu) info apic-local
> apic.lvt00-timer   000300fd int=fd .H.EMP delmod=0:Fixed
> apic.lvt00-thermal 0001 int=00 .H.EM. delmod=0:Fixed
> apic.lvt00-perfmon 00fe int=fe .H.E.. delmod=0:Fixed
> apic.lvt00-LINT0   0001001f int=1f .H.EM. delmod=0:Fixed
> apic.lvt00-LINT1   04ff int=ff .H.E.. delmod=4:NMI
> apic.lvt00-Error   00e3 int=e3 .H.E.. delmod=0:Fixed
> apic.error  00 esr  S:... R:... .
> apic.timer  00 DCR=000b(b) initial_count=19
> apic.icr00 020c00d1: int=d1 delmod=0:Fixed P..E
> shorthand=3:all dest=2
> apic.prio   00 apr=00(0:0)  tpr=40(4:0)
> apic.dest   00 dfr=f0(f)ldr=01(01)
> apic.svr00 011f vec=1f on focus=off
> apic.interrupt  00 065:R.E

This is not very readable :)

What about:

Dumping local APIC state for CPU 0

LVT0  0x0001001f activehi edge masked   Fixed (vec 31)
LVT1  0x04ff activehi edge  NMI
LVTPC 0x00fe activehi edge  Fixed (vec 254)
LVTERR0x00e3 activehi edge  Fixed (vec 243)
LVTTHMR   0x0001 activehi edge masked   Fixed (vec 0)
LVTT  0x000300fd activehi edge masked  periodic Fixed (vec 253)
Timer DCR=0xb (divide by 1), initial count = 19
SPIV  0x011f APIC enabled, focus=off, spurious vec 31
ICR   0x000c00d1 physical edge deassert all
ICR2  0x0200
ESR   0x
ISR   31(level) 65
IRR   31(level) 42(level)

APR 0x00  TPR 0x20  DFR 0xf0  LDR 0x01  PPR 0x40

Some notes:

- no need to detail ESR, it's never used

- no need to detail ICR2 if ICR uses a shorthand, otherwise could be one of
  these formats

  "cpu NN" if this APIC is in xapic flat mode
  "cluster NN mask " if this APIC is in xapic cluster mode
  "cluster NN mask " if this APIC is in x2apic mode

- the empty space after "masked" is for "pending"

- please add support for TSC deadline mode too

Paolo

> 
> Signed-off-by: Pavel Butsykin 
> Signed-off-by: Denis V. Lunev 
> CC: Andreas Färber 
> CC: Paolo Bonzini 
> ---
>  hmp-commands-info.hx |  16 
>  include/monitor/monitor-common.h |   1 +
>  target-i386/cpu.h|   3 +
>  target-i386/helper.c | 155 
> +++
>  target-i386/monitor.c|   6 ++
>  5 files changed, 181 insertions(+)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 9f5a158..5ffc181 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -112,6 +112,22 @@ STEXI
>  Show the cpu registers.
>  ETEXI
>  
> +#if defined(TARGET_I386)
> +{
> +.name   = "apic-local",
> +.args_type  = "",
> +.params = "",
> +.help   = "show local apic state",
> +.mhandler.cmd = hmp_info_apic_local,
> +},
> +#endif
> +
> +STEXI
> +@item info apic-local
> +@findex apic-local
> +Show local APIC state
> +ETEXI
> +
>  {
>  .name   = "cpus",
>  .args_type  = "",
> diff --git a/include/monitor/monitor-common.h 
> b/include/monitor/monitor-common.h
> index abd7a6c..462c35e 100644
> --- a/include/monitor/monitor-common.h
> +++ b/include/monitor/monitor-common.h
> @@ -42,5 +42,6 @@ CPUState *mon_get_cpu(void);
>  void hmp_info_mem(Monitor *mon, const QDict *qdict);
>  void hmp_info_tlb(Monitor *mon, const QDict *qdict);
>  void hmp_mce(Monitor *mon, const QDict *qdict);
> +void hmp_info_apic_local(Monitor *mon, const QDict *qdict);
>  
>  #endif /* MONITOR_COMMON */
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index af97772..f37a9c6 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -1347,4 +1347,7 @@ void enable_compat_apic_id_mode(void);
>  #define APIC_DEFAULT_ADDRESS 0xfee0
>  #define APIC_SPACE_SIZE  0x10
>  
> +void x86_cpu_dump_apic_local_state(CPUState *cs, FILE *f,
> +   fprintf_function cpu_fprintf, int flags);
> +
>  #endif /* CPU_I386_H */
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 5480a96..8d883f5 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -23,6 +23,7 @@
>  #ifndef CONFIG_USER_ONLY
>  #include "sysemu/sysemu.h"
>  #include "monitor/monitor.h"
> +#include "hw/i386/apic_internal.h"
>  #endif
>  
>  static void cpu_x86_version(CPUX86State *env, int *family, int *model)
> @@ -177,6 +178,160 @@ done:
>  cpu_fprintf(f, "\n");
>  }
>  
> +#ifndef CONFIG_USER_ONLY
> +
> +/* ARRAY_SIZE check is not required because
> + * DeliveryMode(dm) has a size o

Re: [Qemu-devel] [PATCH] migration: Use g_new() & friends where that makes obvious sense

[Amit Shah]

On (Mon) 14 Sep 2015 [13:51:31], Markus Armbruster wrote:
> g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
> for two reasons.  One, it catches multiplication overflowing size_t.
> Two, it returns T * rather than void *, which lets the compiler catch
> more type errors.
> 
> This commit only touches allocations with size arguments of the form
> sizeof(T).  Same Coccinelle semantic patch as in commit b45c03f.
> 
> Signed-off-by: Markus Armbruster 

Reviewed-by: Amit Shah 

Amit

Re: [Qemu-devel] [PULL 0/4] update to 35c53797 to 4e03af8

[Gerd Hoffmann]

On Do, 2015-09-03 at 14:32 +0100, Peter Maydell wrote:
> On 3 September 2015 at 14:02, Gerd Hoffmann  wrote:
> >   Hi,
> >
> > Here comes the ipxe update pull.  Thanks to Stefan all bits are upstream
> > now.  Also upstream has a named configuration for qemu.  That simplifies
> > the whole process, I could drop some patches and so there are only 4 of
> > them left, two of them being the actual ipxe update (one submodule, one
> > binaries).
> >
> > Note that this is NOT current git master.  Michael Brown added support
> > for EFI_PXE_BASE_CODE_PROTOCOL recently, and I tried to pick the last
> > commit before that work started to have a stable and known-good base.
> >
> > I'll have a look a the new code shortly though, so stay tuned for more
> > ipxe updates.
> 
> These patches appear to be not merely trivial "update submodule",
> but some other changes too. I'd prefer them to appear on the list
> as patches first for review, rather than making their first
> appearance in a pull request.

Patches have been on the list now for a week, without comments.
Pull now?

cheers,
  Gerd

[Qemu-devel] [PULL 0/6] gtk: misc grab tweaks, locale fix.

[Gerd Hoffmann]

  Hi,

gtk patch queue, with a bunch of grab code tweaks.
Also brings the locale fix (use LC_MESSAGES only).

please pull,
  Gerd

The following changes since commit 007e620a7576e4ce2ea6955541e87d8ae8ed32ae:

  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging 
(2015-09-14 18:51:09 +0100)

are available in the git repository at:


  git://git.kraxel.org/qemu tags/pull-gtk-20150915-1

for you to fetch changes up to 2cb5d2a47c655331bcf0ab16bab8fe4701182c58:

  gtk: use setlocale() for LC_MESSAGES only (2015-09-15 12:27:41 +0200)


gtk: misc grab tweaks, locale fix.


Alberto Garcia (1):
  gtk: use setlocale() for LC_MESSAGES only

Gerd Hoffmann (5):
  gtk: check for existing grabs in gd_grab_{pointer,keyboard}
  gtk: move gd_update_caption calls to gd_{grab,ungrab}_{pointer,keyboard}
  gtk: trace input grab reason
  gtk: set free_scale when setting zoom_fit
  gtk: don't grab input when entering fullscreen.

 trace-events |  3 ++-
 ui/gtk.c | 84 
 2 files changed, 58 insertions(+), 29 deletions(-)

[Qemu-devel] [PULL 6/6] gtk: use setlocale() for LC_MESSAGES only

[Gerd Hoffmann]

From: Alberto Garcia 

The QEMU code is not internationalized and assumes that it runs under
the C locale, but if we use the GTK+ UI we'll end up importing the
locale settings from the environment. This can break things, such as
the JSON generator and iotest 120 in locales that use a decimal comma.

We do however have translations for a few simple strings for the GTK+
menu items, so in order to run QEMU using the C locale, and yet have a
translated UI let's use setlocale() for LC_MESSAGES only.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Alberto Garcia 
Signed-off-by: Gerd Hoffmann 
---
 ui/gtk.c | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index a17b1d1..187de74 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1950,7 +1950,8 @@ void gtk_display_init(DisplayState *ds, bool full_screen, 
bool grab_on_hover)
 
 s->free_scale = FALSE;
 
-setlocale(LC_ALL, "");
+/* LC_MESSAGES only. See early_gtk_display_init() for details */
+setlocale(LC_MESSAGES, "");
 bindtextdomain("qemu", CONFIG_QEMU_LOCALEDIR);
 textdomain("qemu");
 
@@ -2019,6 +2020,24 @@ void gtk_display_init(DisplayState *ds, bool 
full_screen, bool grab_on_hover)
 
 void early_gtk_display_init(int opengl)
 {
+/* The QEMU code relies on the assumption that it's always run in
+ * the C locale. Therefore it is not prepared to deal with
+ * operations that produce different results depending on the
+ * locale, such as printf's formatting of decimal numbers, and
+ * possibly others.
+ *
+ * Since GTK+ calls setlocale() by default -importing the locale
+ * settings from the environment- we must prevent it from doing so
+ * using gtk_disable_setlocale().
+ *
+ * QEMU's GTK+ UI, however, _does_ have translations for some of
+ * the menu items. As a trade-off between a functionally correct
+ * QEMU and a fully internationalized UI we support importing
+ * LC_MESSAGES from the environment (see the setlocale() call
+ * earlier in this file). This allows us to display translated
+ * messages leaving everything else untouched.
+ */
+gtk_disable_setlocale();
 gtkinit = gtk_init_check(NULL, NULL);
 if (!gtkinit) {
 /* don't exit yet, that'll break -help */
-- 
1.8.3.1

[Qemu-devel] [PULL 3/6] gtk: trace input grab reason

[Gerd Hoffmann]

Add a reason to grab calls and trace points,
so it is easier to debug grab related ui issues.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Marc-André Lureau 
---
 trace-events |  3 ++-
 ui/gtk.c | 26 +-
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/trace-events b/trace-events
index 1927c76..3d1857b 100644
--- a/trace-events
+++ b/trace-events
@@ -1150,7 +1150,8 @@ ppm_save(const char *filename, void *display_surface) "%s 
surface=%p"
 gd_switch(const char *tab, int width, int height) "tab=%s, width=%d, height=%d"
 gd_update(const char *tab, int x, int y, int w, int h) "tab=%s, x=%d, y=%d, 
w=%d, h=%d"
 gd_key_event(const char *tab, int gdk_keycode, int qemu_keycode, const char 
*action) "tab=%s, translated GDK keycode %d to QEMU keycode %d (%s)"
-gd_grab(const char *tab, const char *device, bool on) "tab=%s, %s %d"
+gd_grab(const char *tab, const char *device, const char *reason) "tab=%s, 
dev=%s, reason=%s"
+gd_ungrab(const char *tab, const char *device) "tab=%s, dev=%s"
 
 # ui/vnc.c
 vnc_key_guest_leds(bool caps, bool num, bool scroll) "caps %d, num %d, scroll 
%d"
diff --git a/ui/gtk.c b/ui/gtk.c
index 5f87475..322d112 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -165,9 +165,9 @@ struct GtkDisplayState {
 bool ignore_keys;
 };
 
-static void gd_grab_pointer(VirtualConsole *vc);
+static void gd_grab_pointer(VirtualConsole *vc, const char *reason);
 static void gd_ungrab_pointer(GtkDisplayState *s);
-static void gd_grab_keyboard(VirtualConsole *vc);
+static void gd_grab_keyboard(VirtualConsole *vc, const char *reason);
 static void gd_ungrab_keyboard(GtkDisplayState *s);
 
 /** Utility Functions **/
@@ -855,7 +855,7 @@ static gboolean gd_button_event(GtkWidget *widget, 
GdkEventButton *button,
 gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
TRUE);
 } else {
-gd_grab_pointer(vc);
+gd_grab_pointer(vc, "relative-mode-click");
 }
 return TRUE;
 }
@@ -1092,7 +1092,7 @@ static gboolean gd_win_grab(void *opaque)
 if (vc->s->ptr_owner) {
 gd_ungrab_pointer(vc->s);
 } else {
-gd_grab_pointer(vc);
+gd_grab_pointer(vc, "user-request-detached-tab");
 }
 return TRUE;
 }
@@ -1256,7 +1256,7 @@ static void gd_grab_devices(VirtualConsole *vc, bool grab,
 }
 #endif
 
-static void gd_grab_keyboard(VirtualConsole *vc)
+static void gd_grab_keyboard(VirtualConsole *vc, const char *reason)
 {
 if (vc->s->kbd_owner) {
 if (vc->s->kbd_owner == vc) {
@@ -1277,7 +1277,7 @@ static void gd_grab_keyboard(VirtualConsole *vc)
 #endif
 vc->s->kbd_owner = vc;
 gd_update_caption(vc->s);
-trace_gd_grab(vc->label, "kbd", true);
+trace_gd_grab(vc->label, "kbd", reason);
 }
 
 static void gd_ungrab_keyboard(GtkDisplayState *s)
@@ -1295,10 +1295,10 @@ static void gd_ungrab_keyboard(GtkDisplayState *s)
 gdk_keyboard_ungrab(GDK_CURRENT_TIME);
 #endif
 gd_update_caption(s);
-trace_gd_grab(vc->label, "kbd", false);
+trace_gd_ungrab(vc->label, "kbd");
 }
 
-static void gd_grab_pointer(VirtualConsole *vc)
+static void gd_grab_pointer(VirtualConsole *vc, const char *reason)
 {
 GdkDisplay *display = gtk_widget_get_display(vc->gfx.drawing_area);
 
@@ -1337,7 +1337,7 @@ static void gd_grab_pointer(VirtualConsole *vc)
 #endif
 vc->s->ptr_owner = vc;
 gd_update_caption(vc->s);
-trace_gd_grab(vc->label, "ptr", true);
+trace_gd_grab(vc->label, "ptr", reason);
 }
 
 static void gd_ungrab_pointer(GtkDisplayState *s)
@@ -1363,7 +1363,7 @@ static void gd_ungrab_pointer(GtkDisplayState *s)
  vc->s->grab_x_root, vc->s->grab_y_root);
 #endif
 gd_update_caption(s);
-trace_gd_grab(vc->label, "ptr", false);
+trace_gd_ungrab(vc->label, "ptr");
 }
 
 static void gd_menu_grab_input(GtkMenuItem *item, void *opaque)
@@ -1372,8 +1372,8 @@ static void gd_menu_grab_input(GtkMenuItem *item, void 
*opaque)
 VirtualConsole *vc = gd_vc_find_current(s);
 
 if (gd_is_grab_active(s)) {
-gd_grab_keyboard(vc);
-gd_grab_pointer(vc);
+gd_grab_keyboard(vc, "user-request-main-window");
+gd_grab_pointer(vc, "user-request-main-window");
 } else {
 gd_ungrab_keyboard(s);
 gd_ungrab_pointer(s);
@@ -1432,7 +1432,7 @@ static gboolean gd_enter_event(GtkWidget *widget, 
GdkEventCrossing *crossing,
 GtkDisplayState *s = vc->s;
 
 if (gd_grab_on_hover(s)) {
-gd_grab_keyboard(vc);
+gd_grab_keyboard(vc, "grab-on-hover");
 }
 return TRUE;
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 8/9] hmp: added io apic dump state

[Paolo Bonzini]

On 15/09/2015 11:23, Denis V. Lunev wrote:
> 
> Implementation is only for kvm here. The dump will look like
> (qemu) info apic-io
> ioapic ID=00 IRR= SEL=18
> ioapic 00 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0
> ioapic 01 03000993: int=93 delmod=1:LowPri L.H.E. dest=3
> ...
> ioapic 23 000100ff: int=ff delmod=0:Fixed  P.H.EM dest=0

I would call the command "info ioapic" and would use a format like

ioapic id=0x00 sel=0x18 (redir[4])
pin 0  0x000100ff dest=0 vec=255 activehi edge masked fixed  physical
pin 1  0x03000993 dest=3 vec=147 activehi edgelowest logical
...
pin 23 0x000100ff dest=0 vec=255 activehi edge masked fixed  physical
IRR (none)

(both suggestions for the format are inspired by the KVM trace events)

Paolo

[Qemu-devel] [PULL 1/6] gtk: check for existing grabs in gd_grab_{pointer, keyboard}

[Gerd Hoffmann]

If a grab is already active for our window, do nothing.
If a grab is already active for another window, release it.

Cleanup some checks and ungrab calls in the code which are
not needed any more.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Marc-André Lureau 
---
 ui/gtk.c | 25 -
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index df2a79e..24a1edb 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -167,6 +167,8 @@ struct GtkDisplayState {
 
 static void gd_grab_pointer(VirtualConsole *vc);
 static void gd_ungrab_pointer(GtkDisplayState *s);
+static void gd_grab_keyboard(VirtualConsole *vc);
+static void gd_ungrab_keyboard(GtkDisplayState *s);
 
 /** Utility Functions **/
 
@@ -849,7 +851,6 @@ static gboolean gd_button_event(GtkWidget *widget, 
GdkEventButton *button,
 /* implicitly grab the input at the first click in the relative mode */
 if (button->button == 1 && button->type == GDK_BUTTON_PRESS &&
 !qemu_input_is_absolute() && s->ptr_owner != vc) {
-gd_ungrab_pointer(s);
 if (!vc->window) {
 gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
TRUE);
@@ -1259,6 +1260,14 @@ static void gd_grab_devices(VirtualConsole *vc, bool 
grab,
 
 static void gd_grab_keyboard(VirtualConsole *vc)
 {
+if (vc->s->kbd_owner) {
+if (vc->s->kbd_owner == vc) {
+return;
+} else {
+gd_ungrab_keyboard(vc->s);
+}
+}
+
 #if GTK_CHECK_VERSION(3, 0, 0)
 gd_grab_devices(vc, true, GDK_SOURCE_KEYBOARD,
GDK_KEY_PRESS_MASK | GDK_KEY_RELEASE_MASK,
@@ -1292,6 +1301,15 @@ static void gd_ungrab_keyboard(GtkDisplayState *s)
 static void gd_grab_pointer(VirtualConsole *vc)
 {
 GdkDisplay *display = gtk_widget_get_display(vc->gfx.drawing_area);
+
+if (vc->s->ptr_owner) {
+if (vc->s->ptr_owner == vc) {
+return;
+} else {
+gd_ungrab_pointer(vc->s);
+}
+}
+
 #if GTK_CHECK_VERSION(3, 0, 0)
 GdkDeviceManager *mgr = gdk_display_get_device_manager(display);
 gd_grab_devices(vc, true, GDK_SOURCE_MOUSE,
@@ -1352,9 +1370,7 @@ static void gd_menu_grab_input(GtkMenuItem *item, void 
*opaque)
 VirtualConsole *vc = gd_vc_find_current(s);
 
 if (gd_is_grab_active(s)) {
-if (!gd_grab_on_hover(s)) {
-gd_grab_keyboard(vc);
-}
+gd_grab_keyboard(vc);
 gd_grab_pointer(vc);
 } else {
 gd_ungrab_keyboard(s);
@@ -1415,7 +1431,6 @@ static gboolean gd_enter_event(GtkWidget *widget, 
GdkEventCrossing *crossing,
 GtkDisplayState *s = vc->s;
 
 if (gd_grab_on_hover(s)) {
-gd_ungrab_keyboard(s);
 gd_grab_keyboard(vc);
 gd_update_caption(s);
 }
-- 
1.8.3.1

[Qemu-devel] [PULL 5/6] gtk: don't grab input when entering fullscreen.

[Gerd Hoffmann]

Kick off all grabbing logic from fullscreen mode.  In the current state
it seems to create more problems than it solves.  Try running qemu/gtk
fullscreen on one head of a multihead host for example ...

There probably was a reason the grab-on-fullscreen logic was added in
the first place.  So please test and report any issues so we can try to
find a sane way to handle it.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Marc-André Lureau 
---
 ui/gtk.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 2629d97..a17b1d1 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1140,10 +1140,6 @@ static void gd_menu_full_screen(GtkMenuItem *item, void 
*opaque)
 gtk_widget_hide(s->menu_bar);
 if (vc->type == GD_VC_GFX) {
 gtk_widget_set_size_request(vc->gfx.drawing_area, -1, -1);
-if (qemu_console_is_graphic(vc->gfx.dcl.con)) {
-gtk_check_menu_item_set_active
-(GTK_CHECK_MENU_ITEM(s->grab_item), TRUE);
-}
 }
 gtk_window_fullscreen(GTK_WINDOW(s->window));
 s->full_screen = TRUE;
@@ -1156,8 +1152,6 @@ static void gd_menu_full_screen(GtkMenuItem *item, void 
*opaque)
 vc->gfx.scale_x = 1.0;
 vc->gfx.scale_y = 1.0;
 gd_update_windowsize(vc);
-gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(s->grab_item),
-   FALSE);
 }
 }
 
-- 
1.8.3.1

[Qemu-devel] [PULL 4/6] gtk: set free_scale when setting zoom_fit

[Gerd Hoffmann]

free_scale field tracks zoom-fit menu toggle state,
so we should keep them in sync ...

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Marc-André Lureau 
---
 ui/gtk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index 322d112..2629d97 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1782,6 +1782,7 @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, 
VirtualConsole *vc,
 
 if (dpy_ui_info_supported(vc->gfx.dcl.con)) {
 gtk_menu_item_activate(GTK_MENU_ITEM(s->zoom_fit_item));
+s->free_scale = true;
 }
 
 return group;
-- 
1.8.3.1

[Qemu-devel] [PULL 2/6] gtk: move gd_update_caption calls to gd_{grab, ungrab}_{pointer, keyboard}

[Gerd Hoffmann]

Then we don't have to pair the grab/ungrab calls with update_caption
calls any more because things happen automatically ;)

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Marc-André Lureau 
---
 ui/gtk.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 24a1edb..5f87475 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -856,7 +856,6 @@ static gboolean gd_button_event(GtkWidget *widget, 
GdkEventButton *button,
TRUE);
 } else {
 gd_grab_pointer(vc);
-gd_update_caption(s);
 }
 return TRUE;
 }
@@ -1095,7 +1094,6 @@ static gboolean gd_win_grab(void *opaque)
 } else {
 gd_grab_pointer(vc);
 }
-gd_update_caption(vc->s);
 return TRUE;
 }
 
@@ -1278,6 +1276,7 @@ static void gd_grab_keyboard(VirtualConsole *vc)
   GDK_CURRENT_TIME);
 #endif
 vc->s->kbd_owner = vc;
+gd_update_caption(vc->s);
 trace_gd_grab(vc->label, "kbd", true);
 }
 
@@ -1295,6 +1294,7 @@ static void gd_ungrab_keyboard(GtkDisplayState *s)
 #else
 gdk_keyboard_ungrab(GDK_CURRENT_TIME);
 #endif
+gd_update_caption(s);
 trace_gd_grab(vc->label, "kbd", false);
 }
 
@@ -1336,6 +1336,7 @@ static void gd_grab_pointer(VirtualConsole *vc)
 &vc->s->grab_x_root, &vc->s->grab_y_root, NULL);
 #endif
 vc->s->ptr_owner = vc;
+gd_update_caption(vc->s);
 trace_gd_grab(vc->label, "ptr", true);
 }
 
@@ -1361,6 +1362,7 @@ static void gd_ungrab_pointer(GtkDisplayState *s)
  gtk_widget_get_screen(vc->gfx.drawing_area),
  vc->s->grab_x_root, vc->s->grab_y_root);
 #endif
+gd_update_caption(s);
 trace_gd_grab(vc->label, "ptr", false);
 }
 
@@ -1377,7 +1379,6 @@ static void gd_menu_grab_input(GtkMenuItem *item, void 
*opaque)
 gd_ungrab_pointer(s);
 }
 
-gd_update_caption(s);
 gd_update_cursor(vc);
 }
 
@@ -1432,7 +1433,6 @@ static gboolean gd_enter_event(GtkWidget *widget, 
GdkEventCrossing *crossing,
 
 if (gd_grab_on_hover(s)) {
 gd_grab_keyboard(vc);
-gd_update_caption(s);
 }
 return TRUE;
 }
@@ -1445,7 +1445,6 @@ static gboolean gd_leave_event(GtkWidget *widget, 
GdkEventCrossing *crossing,
 
 if (gd_grab_on_hover(s)) {
 gd_ungrab_keyboard(s);
-gd_update_caption(s);
 }
 return TRUE;
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH RFC 0/3] Checkpoint-assisted migration proposal

[Amit Shah]

Hi,

On (Fri) 17 Apr 2015 [14:12:59], Bohdan Trach wrote:
> From: Bohdan Trach 
> 
> This patchset contains a checkpoint-assisted migration feature as
> proposed earlier on this list [1]. It allows reusing existing memory
> snapshots of guests to speed up migration of VMs between physical
> hosts.
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg01555.html

Could you please include a file in the docs/ directory that documents
how this works, so it's easier to comment on the general idea?

>From 'checkpointing', I was afraid this was going to use some
checkpoint-restore framework, but instead it's a new checkpointing
method that you're adding to qemu.

Can you describe when checkpoints are taken, and what is checkpointed?
How is it stored on the disk?

I'm sure the patches have all the details, but it's easier to check
the soundness of the idea if there's a high-level doc that explains
this, and then we can discuss the finer points over patches.

Overall, I think this approach can benefit some workloads, and since
it's not affecting a lot of common code, we could look at adding it.

Also, apologies for not getting to this earlier.

Thanks,

Amit

Re: [Qemu-devel] [PATCH 1/5] migration/ram.c: Use RAMBlock rather than MemoryRegion

[Amit Shah]

On (Thu) 13 Aug 2015 [11:51:30], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> RAM migration mainly works on RAMBlocks but in a few places
> uses data from MemoryRegions to access the same information that's
> already held in RAMBlocks; clean it up just to avoid the
> MemoryRegion use.
> 
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: Amit Shah 

Amit

Re: [Qemu-devel] [PATCH COLO-Frame v9 00/32] COarse-grain LOck-stepping(COLO) Virtual Machines for Non-stop Service (FT)

[zhanghailiang]

Ping again ...

On 2015/9/2 16:22, zhanghailiang wrote:

This is the 9th version of COLO.

Please Note that, this version is very different from the previous versions.
since we have decided to realize proxy in qemu, which based on slirp in qemu.
We dropped all the original colo proxy related part.

It will be a long time for proxy to be ready for merging, so here we extract
the basic periodic checkpoint part that not depend on proxy into this series.
Actually, the 'periodic' mode is also what we want to support in COLO, it is
based on Yang Hongyang's netfilter series. and this mode is very like
MicroCheckpointing and Remus.

You can find the discussion about why & how to realize the colo proxy in qemu
from the follow link:
http://lists.nongnu.org/archive/html/qemu-devel/2015-07/msg04069.html

As usual, here is only COLO frame part, you can get the whole codes from github:
https://github.com/coloft/qemu/commits/colo-v2.0-periodic-mode

Compared with previous versions, this version is more easy to test.

Test procedure:
1. Startup qemu
Primary side:
# x86_64-softmmu/qemu-system-x86_64 -enable-kvm -netdev tap,id=bn0 -netfilter 
buffer,id=f0,netdev=bn0,chain=in -device virtio-net-pci,id=net-pci0,netdev=bn0 
-boot c -drive 
if=virtio,id=disk1,driver=quorum,read-pattern=fifo,cache=none,aio=native,children.0.file.filename=/mnt/sdd/pure_IMG/linux/redhat/rhel_6.5_64_2U_ide,children.0.driver=raw
 -vnc :7 -m 2048 -smp 2 -device piix3-usb-uhci -device usb-tablet -monitor 
stdio -S

Secondary side:
# x86_64-softmmu/qemu-system-x86_64 -enable-kvm -netdev tap,id=bn0 -device 
virtio-net-pci,id=net-pci0,netdev=bn0 -drive 
if=none,driver=raw,file=/mnt/sdd/pure_IMG/linux/redhat/rhel_6.5_64_2U_ide,id=colo1,cache=none,aio=native
 -drive 
if=virtio,driver=replication,mode=secondary,throttling.bps-total=7000,file.file.filename=/mnt/ramfs/active_disk.img,file.driver=qcow2,file.backing.file.filename=/mnt/ramfs/hidden_disk.img,file.backing.driver=qcow2,file.backing.backing.backing_reference=colo1,file.backing.allow-write-backing-file=on
 -vnc :7 -m 2048 -smp 2 -device piix3-usb-uhci -device usb-table -monitor stdio 
-incoming tcp:0:

2. On Secondary VM's QEMU monitor, issue command
(qemu) nbd_server_start 192.168.2.88:8889
(qemu) nbd_server_add -w colo1

3. On Primary VM's QEMU monitor, issue command:
(qemu) child_add disk1 
child.driver=replication,child.mode=primary,child.file.host=192.168.2.88,child.file.port=8889,child.file.export=colo1,child.file.driver=nbd,child.ignore-errors=on
(qemu) migrate_set_capability colo on
(qemu) migrate tcp:192.168.2.88:

4. After the above steps, you will see, whenever you make changes to PVM, SVM 
will be synced.
You can by issue command "migrate_set_parameter checkpoint-delay 2000"
to change the checkpoint period time.

5. Failover test
You can kill PVM  and run 'colo_lost_heartbeat' in SVM's
monitor at the same time, then SVM will failover and client will not feel this 
change.

COLO is a totally new feature which is still in early stage,
your comments and feedback are warmly welcomed.

TODO:
1. checkpoint based on proxy in qemu
2. The capability of continuous FT

v9:
- Drop colo proxy related part (colo-nic.c file)
- Convert COLO protocol name definition to QAPI
- Smash failover related patch (patch 19/20/23)
- Fix colo exit event according Eric's comments.
- Fix some typos from Eric's comments
- Fix bug 'invalid runstate transition: 'colo' -> 'prelaunch' reported
   by Dave (patch 27)
- Use migrate_set_parameter intead of ecolo-set-checkpoint-period to set
   checkpoint delay time (patch 25)
- Add new patch (patch 29/30) to seperate the process of saving/loading
   device and state during checkpoint. which will reduce the data size
   for sending and also reduce the qsb size used in checkpoint.

Wen Congyang (1):
   COLO: Add block replication into colo process

zhanghailiang (31):
   configure: Add parameter for configure to enable/disable COLO support
   migration: Introduce capability 'colo' to migration
   COLO: migrate colo related info to slave
   migration: Add state records for migration incoming
   migration: Integrate COLO checkpoint process into migration
   migration: Integrate COLO checkpoint process into loadvm
   migration: Rename the'file' member of MigrationState and
 MigrationIncomingState
   COLO/migration: establish a new communication path from destination to
 source
   COLO: Implement colo checkpoint protocol
   COLO: Add a new RunState RUN_STATE_COLO
   QEMUSizedBuffer: Introduce two help functions for qsb
   COLO: Save PVM state to secondary side when do checkpoint
   COLO: Load PVM's dirty pages into SVM's RAM cache temporarily
   COLO: Load VMState into qsb before restore it
   COLO: Flush PVM's cached RAM into SVM's memory
   COLO: synchronize PVM's state to SVM periodically
   COLO failover: Introduce a new command to trigger a failover
   COLO failover: Introduce state to record failover process
   COLO: Implement failover work for Primary VM

Re: [Qemu-devel] [PATCH v16 02/35] linux-user: Support tilegx architecture in linux-user

[Peter Maydell]

On 14 September 2015 at 23:43, Richard Henderson  wrote:
> From: Chen Gang 
>
> Add main working flow feature, system call processing feature, and elf64
> tilegx binary loading feature, based on Linux kernel tilegx 64-bit
> implementation.
>
> [rth: Moved all of the implementation of atomic instructions to a later 
> patch.]
>
> Signed-off-by: Chen Gang 
> Reviewed-by: Peter Maydell 
> Message-Id: 
> Signed-off-by: Richard Henderson 

> +static void gen_sigsegv_mapper(CPUTLGState *env, target_ulong addr)
> +{
> +target_siginfo_t info;
> +
> +info.si_signo = TARGET_SIGSEGV;
> +info.si_errno = 0;
> +info.si_code = TARGET_SEGV_MAPERR;
> +info._sifields._sigfault._addr = addr;
> +queue_signal(env, info.si_signo, &info);
> +}

Missed this in my earlier review, but there's a typo
in this function name -- it should be gen_sigsegv_maperr,
to match the si_code value.

(You can retain my R-by and fix the typo.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v7 09/26] qapi: De-duplicate enum code generation

[Daniel P. Berrange]

On Mon, Sep 14, 2015 at 07:57:40PM +0200, Markus Armbruster wrote:
> Duplicated in commit 21cd70d.  Yes, we can't import qapi-types, but
> that's no excuse.  Move the helpers from qapi-types.py to qapi.py, and
> replace the duplicates in qapi-event.py.
> 
> The generated event enumeration type's lookup table becomes
> const-correct (see commit 2e4450f), and uses explicit indexes instead
> of relying on order (see commit 912ae9c).
> 
> Signed-off-by: Markus Armbruster 
> Reviewed-by: Eric Blake 
> ---
>  docs/qapi-code-gen.txt |  9 ---
>  scripts/qapi-event.py  | 67 
> +++---
>  scripts/qapi-types.py  | 55 -
>  scripts/qapi.py| 55 +
>  4 files changed, 64 insertions(+), 122 deletions(-)

Reviewed-by: Daniel P. Berrange 


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v16 32/35] target-tilegx: Handle atomic instructions

[Peter Maydell]

On 14 September 2015 at 23:43, Richard Henderson  wrote:
> Signed-off-by: Richard Henderson 
> ---

> +static void do_exch(CPUTLGState *env, bool quad, bool cmp)
> +{
> +target_ulong addr;
> +target_long val, sprval;
> +
> +start_exclusive();
> +
> +addr = env->atomic_srca;
> +if (quad ? get_user_s64(val, addr) : get_user_s32(val, addr)) {
> +goto sigsegv_mapper;
> +}
> +
> +if (cmp) {
> +if (quad) {
> +sprval = env->spregs[TILEGX_SPR_CMPEXCH];
> +} else {
> +sprval = sextract64(env->spregs[TILEGX_SPR_CMPEXCH], 0, 32);
> +}
> +}
> +
> +if (!cmp || val == sprval) {
> +target_long valb = env->atomic_srcb;
> +if (quad ? put_user_u64(valb, addr) : put_user_u32(valb, addr)) {
> +goto sigsegv_mapper;
> +}
> +}
> +
> +set_regval(env, env->atomic_dstr, val);
> +end_exclusive();
> +return;
> +
> + sigsegv_mapper:

This label (and the one in do_fetch) should be "sigsegv_maperr"

> +end_exclusive();
> +gen_sigsegv_mapper(env, addr);

(and I just noticed this function has a typo in its name too).

Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH v7 10/26] qapi-event: Eliminate global variable event_enum_value

[Daniel P. Berrange]

On Mon, Sep 14, 2015 at 07:57:41PM +0200, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster 
> Reviewed-by: Eric Blake 
> ---
>  scripts/qapi-event.py | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrange 


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v7 12/26] qapi: Replace dirty is_c_ptr() by method c_null()

[Daniel P. Berrange]

On Mon, Sep 14, 2015 at 07:57:43PM +0200, Markus Armbruster wrote:
> is_c_ptr() looks whether the end of the C text for the type looks like
> a pointer.  Works, but is fragile.
> 
> We now have a better tool: use QAPISchemaType method c_null().  The
> initializers for non-pointers become prettier: 0, false or the
> enumeration constant with the value 0 instead of {0}.
> 
> Signed-off-by: Markus Armbruster 
> Reviewed-by: Eric Blake 
> ---
>  scripts/qapi-commands.py | 16 +---
>  scripts/qapi.py  |  3 ---
>  2 files changed, 5 insertions(+), 14 deletions(-)

Reviewed-by: Daniel P. Berrange 


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [edk2] Windows does not support DataTableRegion at all [was: docs: describe QEMU's VMGenID design]

[Laszlo Ersek]

On 09/14/15 23:12, Bill Paul wrote:

[snip -- see my other email too]

> Also, if you want to talk business cases, I'm assuming that somewhere 
> Microsoft claims ACPI compliance.

Correct; this page:



states

Version 5.0 of the Microsoft ACPI source language (ASL) compiler
supports the features in the Advanced Configuration and Power
Interface Specification, Revision 5.0 [...]

And the separately downloadable ASL.exe that I tried starts with a banner that 
claims ACPI 4.0 compliance:

> C:\Program Files (x86)\Microsoft ASL Compiler v4.0>asl.exe x.dsl
> Microsoft ACPI Source Language Assembler Version 4.0.0NT [Aug 28 2009, 
> 18:36:36]
> 
> Copyright (c) 1996,2009 Microsoft Corporation
> Compliant with the ACPI 4.0 Specification
> 
> [...]

The DataTableRegion() operator is from ACPI 2.0.

In ACPI 6.0 (the most recent release), it is still there.

(And, logically, if you can compile DataTableRegion() from ASL to AML (-> 
DefDataRegion), then your AML interpreter should also be able to parse and 
execute the binary DefDataRegion encoding codified by the standard.)

> If so, then clearly that claim is untrue,

Let's say, "not precise".

I think such gaps in support for various industry standards are not uncommon, 
but I believe they should be publicly documented. Using Google, I couldn't find 
a trace of this limitation. If there had been public documentation about this 
(or maybe a public bug tracker, or just a tech support forum post...), it would 
have saved us many many hours, at minimum.

(At this point though, the best for us would be if Microsoft decided to 
implement DataTableRegion() in ACPI.SYS, and push it out as KBx.)
 
> and I'd be willing to bet this isn't the only place where their 
> implementation 
> deviates from the spec either.

I assume that's likely, yes.

> It's bad for business to claim compliance with 
> an industry standard and then very obviously (and maybe even deliberately) 
> not 
> comply with it.

The inaccurate claim about compliance is certainly confusing.

Establishing the non-compliance (from the oustide anyway) was anything but 
obvious. But, now that we've seen the evidence, it's quite factual.

> (If, as you say, this has already been reported and Microsoft 
> decided not to fix it, then it's no longer just a good faith mistake.)

I didn't try to state that as a fact, I just opined that in the 15 years since 
the release of the ACPI 2.0 revision, someone must surely have tried 
DataTableRegion(). Assuming that programmer worked for a big BIOS company 
(which I think is a safe assumption -- before virtualization has become 
commonplace, who else looked into *writing* ACPI tables?), it seems to follow 
that a bug would be reported in some way.

> It's 
> even worse to do that while also being a prominent member of the very same 
> industry committee responsible for defining the standard in the first place.

Right, it's strange.

> In any case, bugs are bugs. You shouldn't need a justification to fix them, 
> especially with iron-clad proof of their existence like you have here.

Bugfixing has costs. That's what I'm worried about a little bit. I don't know 
what to put in the other side of the balance. "Improving compliance" should 
have marketing value, minimally. Then, "helping QEMU developers implement 
Microsoft's VMGENID spec, thereby improving Windows guest utility on QEMU" 
should ultimately translate to wider Windows guest adoption.

> All that aside, I don't know who to report it to either. Maybe this is a good 
> time to establish a way to do that, because I doubt this will be the last 
> time 
> it will be necessary.

I'm hopeful about the ASWG connection.

Thanks!
Laszlo

> -Bill
> 
>> Thanks
>> Laszlo
>>
>>> -Bill
>>>
> I suggest we go back to the last Gal's series
> which is though not universal but a simple and
> straightforward solution.
> That series with comments addressed probably
> is what we need in the end.

 I agree (I commented the same on the RHBZ too). The only one requirement
 we might not satisfy with that is that the page containing the
 generation ID must always be mapped as cacheable. In practice it doesn't
 seem to cause issues.

 We tried to play nice, but given that (a) the vmgenid doc doesn't
 mention a real requirement about the _CRS -- ie. no IO descriptors are
 allowed to be in it --, (b) Windows doesn't support DataTableRegion(), I
 doubt we could cover our bases 100% anyway. There can be any number of
 further hidden requirements, and hidden gaps in ACPI support too, so
 it's just business as usual with Windows: whatever works, works, don't
 ask why.

 Just my opinion of course.

 Laszlo

>> The only crazy thing you didn't try is to use
>> an XSDT instead of the DSDT.
>> I find it unlikely that this will help ...

 __

Re: [Qemu-devel] [edk2] Windows does not support DataTableRegion at all [was: docs: describe QEMU's VMGenID design]

[Laszlo Ersek]

On 09/14/15 19:23, Walz, Michael C wrote:
> Who are the Microsoft representatives in the ASWG? Shouldn't this 
> conversation be happening with them?

I asked my colleague Al Stone @RH for help; he said he'd bring this up
at the next ASWG meeting. Thank you Al, and thank you Michael for the
suggestion.

Thanks!
Laszlo

> 
> -Original Message-
> From: Moore, Robert 
> Sent: September 14, 2015 10:14
> To: Bill Paul; edk2-de...@ml01.01.org
> Cc: Laszlo Ersek; Igor Mammedov; Michael S. Tsirkin; Josh Triplett; Gal 
> Hammer; edk2-devel-01; qemu-devel@nongnu.org; Paolo Bonzini; Gough, Robert; 
> Walz, Michael C
> Subject: RE: [edk2] [Qemu-devel] Windows does not support DataTableRegion at 
> all [was: docs: describe QEMU's VMGenID design]
> 
> The Windows ACPI implementation doesn't even fully support ACPI 2.0 AFAIK, 
> let alone other new things.
> 
> Rob Gough or Michael may know better than I do.
> 
> 
>> -Original Message-
>> From: Bill Paul [mailto:wp...@windriver.com]
>> Sent: Monday, September 14, 2015 9:53 AM
>> To: edk2-de...@ml01.01.org
>> Cc: Laszlo Ersek; Igor Mammedov; Michael S. Tsirkin; Josh Triplett; 
>> Gal Hammer; edk2-devel-01; Moore, Robert; qemu-devel@nongnu.org; Paolo 
>> Bonzini
>> Subject: Re: [edk2] [Qemu-devel] Windows does not support 
>> DataTableRegion at all [was: docs: describe QEMU's VMGenID design]
>>
>> Of all the gin joints in all the towns in all the world, Laszlo Ersek 
>> had to walk into mine at 03:24:42 on Monday 14 September 2015 and say:
>>
>>> On 09/14/15 10:24, Igor Mammedov wrote:
 On Sun, 13 Sep 2015 15:34:51 +0300

 "Michael S. Tsirkin"  wrote:
> On Sun, Sep 13, 2015 at 01:56:44PM +0200, Laszlo Ersek wrote:
>> As the subject suggests, I have terrible news.
>>
>> I'll preserve the full context here, so that it's easy to scroll 
>> back to the ASL for reference.
>>
>> I'm also CC'ing edk2-devel, because a number of BIOS developers 
>> should be congregating there.
>
> Wow, bravo! It does look like we need to go back to the drawing 
> board.
>>
>> I read your original post on this with great interest, and I applaud 
>> your determination in tracking this down. Nice job. Sadly, it seems 
>> you too have fallen victim to the "If It Works With Windows, It Must Be Ok"
>> syndrome.
>>
>> Now, I realize that as far as this particular situation is concerned, 
>> even if Microsoft decided to add support for DataTableRegion() 
>> tomorrow, it wouldn't really help because there are too many different 
>> versions of Windows in the field and there's no way to retroactively patch 
>> them all.
>> (Gee, that sounds
>> familiar...)
>>
>> Nevertheless, am I correct in saying that this is in fact a bug in 
>> Microsoft's ACPI implementation (both in their ASL compiler and in the 
>> AML parser)? Unless
>> DataTableRegion() is specified to be optional in some way (I don't 
>> know if it is or not, but I doubt it), this sounds like an clear cut 
>> case of non- compliance with the ACPI spec. And if that's true, isn't 
>> there any way to get Microsoft to fix it?
>>
>> -Bill
>>
 I suggest we go back to the last Gal's series which is though not 
 universal but a simple and straightforward solution.
 That series with comments addressed probably is what we need in 
 the end.
>>>
>>> I agree (I commented the same on the RHBZ too). The only one 
>>> requirement we might not satisfy with that is that the page 
>>> containing the generation ID must always be mapped as cacheable. In 
>>> practice it doesn't seem to cause issues.
>>>
>>> We tried to play nice, but given that (a) the vmgenid doc doesn't 
>>> mention a real requirement about the _CRS -- ie. no IO descriptors 
>>> are allowed to be in it --, (b) Windows doesn't support 
>>> DataTableRegion(), I doubt we could cover our bases 100% anyway. 
>>> There can be any number of further hidden requirements, and hidden 
>>> gaps in ACPI support too, so it's just business as usual with 
>>> Windows: whatever works, works, don't ask why.
>>>
>>> Just my opinion of course.
>>>
>>> Laszlo
>>>
> The only crazy thing you didn't try is to use an XSDT instead of 
> the DSDT.
> I find it unlikely that this will help ...
>>>
>>> ___
>>> edk2-devel mailing list
>>> edk2-de...@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>>
>> --
>> ==
>> 
>> ===
>> -Bill Paul(510) 749-2329 | Senior Member of Technical Staff,
>>  wp...@windriver.com | Master of Unix-Fu - Wind River 
>> Systems 
>> ==
>> 
>> ===
>>"I put a dollar in a change machine. Nothing changed." - George 
>> Carlin 
>> ==
>> 
>> ===

Re: [Qemu-devel] [PATCH v7 13/26] qapi: Clean up after recent conversions to QAPISchemaVisitor

[Daniel P. Berrange]

On Mon, Sep 14, 2015 at 07:57:44PM +0200, Markus Armbruster wrote:
> Generate just 'FOO' instead of 'struct FOO' when possible.
> 
> Drop helper functions that are now unused.
> 
> Make pep8 and pylint reasonably happy.
> 
> Rename generate_FOO() functions to gen_FOO() for consistency.
> 
> Use more consistent and sensible variable names.
> 
> Consistently use c_ for mapping keys when their value is a C
> identifier or type.
> 
> Simplify gen_enum() and gen_visit_union()
> 
> Consistently use single quotes for C text string literals.

I would have preferred to see one type of change per
patch  to make review easier, but this has been through
enough iterations now that no point changing again.

> 
> Signed-off-by: Markus Armbruster 
> Reviewed-by: Eric Blake 
> ---
>  docs/qapi-code-gen.txt   |   2 +-
>  scripts/qapi-commands.py | 140 
> ++-
>  scripts/qapi-event.py| 122 -
>  scripts/qapi-types.py|  79 ++
>  scripts/qapi-visit.py| 127 ++
>  scripts/qapi.py  | 131 +---
>  6 files changed, 274 insertions(+), 327 deletions(-)

Reviewed-by: Daniel P. Berrange 


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v7 14/26] qapi-visit: Rearrange code a bit

[Daniel P. Berrange]

On Mon, Sep 14, 2015 at 07:57:45PM +0200, Markus Armbruster wrote:
> Move gen_visit_decl() to a better place.  Inline
> generate_visit_struct_body().
> 
> Signed-off-by: Markus Armbruster 
> Reviewed-by: Eric Blake 
> ---
>  scripts/qapi-visit.py | 50 --
>  1 file changed, 20 insertions(+), 30 deletions(-)

Reviewed-by: Daniel P. Berrange 


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v7 15/26] qapi-commands: Rearrange code

[Daniel P. Berrange]

On Mon, Sep 14, 2015 at 07:57:46PM +0200, Markus Armbruster wrote:
> Rename gen_marshal_input() to gen_marshal(), because the generated
> function marshals both arguments and results.
> 
> Rename gen_visitor_input_containers_decl() to gen_marshal_vars(), and
> move the other variable declarations there, too.
> 
> Rename gen_visitor_input_block() to gen_marshal_input_visit(), and
> rearrange its code slightly.
> 
> Rename gen_marshal_input_decl() to gen_marshal_proto(), because the
> result isn't a full declaration, unlike gen_command_decl()'s.
> 
> New gen_marshal_decl() actually returns a full declaration.
> 
> Signed-off-by: Markus Armbruster 
> Reviewed-by: Eric Blake 
> ---
>  scripts/qapi-commands.py | 87 
> ++--
>  1 file changed, 39 insertions(+), 48 deletions(-)

Reviewed-by: Daniel P. Berrange 


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v7 16/26] qapi: Rename qmp_marshal_input_FOO() to qmp_marshal_FOO()

[Daniel P. Berrange]

On Mon, Sep 14, 2015 at 07:57:47PM +0200, Markus Armbruster wrote:
> These functions marshal both input and output.
> 
> Signed-off-by: Markus Armbruster 
> Reviewed-by: Eric Blake 
> ---
>  docs/qapi-code-gen.txt|   4 +-
>  docs/writing-qmp-commands.txt |   8 +-
>  monitor.c |   2 +-
>  qmp-commands.hx   | 242 
> +-
>  qmp.c |   6 +-
>  scripts/qapi-commands.py  |   4 +-
>  6 files changed, 133 insertions(+), 133 deletions(-)

Reviewed-by: Daniel P. Berrange 


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v7 17/26] qapi: De-duplicate parameter list generation

[Daniel P. Berrange]

On Mon, Sep 14, 2015 at 07:57:48PM +0200, Markus Armbruster wrote:
> Generated qapi-event.[ch] lose line breaks.  No change otherwise.
> 
> Signed-off-by: Markus Armbruster 
> Reviewed-by: Eric Blake 
> ---
>  scripts/qapi-commands.py | 11 ++-
>  scripts/qapi-event.py| 18 +++---
>  scripts/qapi.py  | 16 
>  3 files changed, 21 insertions(+), 24 deletions(-)

Reviewed-by: Daniel P. Berrange 

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v7 00/11] Fix exceptions handling for MIPS, PowerPC, and i386

[Aurelien Jarno]

On 2015-09-14 15:55, Richard Henderson wrote:
> On 07/10/2015 02:56 AM, Pavel Dovgalyuk wrote:
> >Pavel Dovgalyuk (11):
> >   softmmu: add helper function to pass through retaddr
> >   softmmu: remove now unused functions
> >   cpu-exec: introduce loop exit with restore function
> 
> These first three have been merged now.
> 
> >   target-mips: improve exception handling
> >   target-i386: introduce new raise_exception functions
> >   target-i386: exception handling for FPU instructions
> >   target-i386: exception handling for div instructions
> >   target-i386: exception handling for memory helpers
> >   target-i386: exception handling for seg_helper functions
> >   target-i386: exception handling for other helper functions
> >   target-ppc: exceptions handling in icount mode
> 
> And I've applied the target-i386 patches to my tgt-i386-next branch.
> They'll be merged soon.  That leaves the mips and ppc patches; I hope that
> Leon and Alex will review these.

AFAIK Leon already started to look at the mips patch. The problem is
that there introduced a serious performance regression in user mode:

https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg01620.html

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PULL 0/19] xen-2015-09-08-tag

[Paolo Bonzini]

On 15/09/2015 11:55, Stefano Stabellini wrote:
> On Mon, 14 Sep 2015, Paolo Bonzini wrote:
>> > On 10/09/2015 12:29, Stefano Stabellini wrote:
>>> > > +if (lseek(config_fd, pos, SEEK_SET) != pos) {
>>> > > +return -errno;
>>> > > +}
>>> > >  do {
>>> > > -rc = pread(config_fd, (uint8_t *)&val, len, pos);
>>> > > +rc = read(config_fd, (uint8_t *)&val, len);
>>> > >  } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
>> > 
>> > This leaks config_fd.
> I don't follow, it leaks config_fd where?

Where lseek returns -errno (and IIRC in other places in the same function).

Paolo

Re: [Qemu-devel] [PATCH v7 18/26] qapi-commands: De-duplicate output marshaling functions

[Daniel P. Berrange]

On Mon, Sep 14, 2015 at 07:57:49PM +0200, Markus Armbruster wrote:
> gen_marshal_output() uses its parameter name only for name of the
> generated function.  Name it after the type being marshaled instead of
> its caller, and drop duplicates.
> 
> Saves 7 copies of qmp_marshal_output_int() in qemu-ga, and one copy of
> qmp_marshal_output_str() in qemu-system-*.
> 
> Signed-off-by: Markus Armbruster 
> Reviewed-by: Eric Blake 
> ---
>  docs/qapi-code-gen.txt   |  4 ++--
>  scripts/qapi-commands.py | 17 ++---
>  2 files changed, 12 insertions(+), 9 deletions(-)

Reviewed-by: Daniel P. Berrange 

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v16 35/35] target-tilegx: Handle v1shl, v1shru, v1shrs

[Peter Maydell]

On 14 September 2015 at 23:43, Richard Henderson  wrote:
> Signed-off-by: Richard Henderson 

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/5] Split out end of migration code from migration_thread

[Amit Shah]

On (Thu) 13 Aug 2015 [11:51:31], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> The code that gets run at the end of the migration process
> is getting large, and I'm about to add more for postcopy.
> Split it into a separate function.
> 
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: Amit Shah 

> ---
>  migration/migration.c | 75 
> ---
>  trace-events  |  2 ++
>  2 files changed, 49 insertions(+), 28 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 662e77e..46bb410 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -913,6 +913,50 @@ int64_t migrate_xbzrle_cache_size(void)
>  return s->xbzrle_cache_size;
>  }
>  
> +/**
> + * migration_completion: Used by migration_thread when there's not much left.
> + *   The caller 'breaks' the loop when this returns.
> + *
> + * @s: Current migration state
> + * @*old_vm_running: Pointer to old_vm_running flag
> + * @*start_time: Pointer to time to update
> + */
> +static void migration_completion(MigrationState *s, bool *old_vm_running,
> + int64_t *start_time)
> +{
> +int ret;
> +
> +qemu_mutex_lock_iothread();
> +*start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> +qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
> +*old_vm_running = runstate_is_running();
> +
> +ret = global_state_store();
> +if (!ret) {
> +ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
> +if (ret >= 0) {
> +qemu_file_set_rate_limit(s->file, INT64_MAX);
> +qemu_savevm_state_complete(s->file);
> +}
> +}

The case where ret >= 0 and where s->file has an error didn't set
status to failed nor break'ed in the removed context below.  But it
happened a few lines outside the context anyway.

Now it just happens in this function, which is cleaner and easier to
understand.

Amit

Re: [Qemu-devel] [PATCH v3 1/5] support nbd driver in blockdev-add

[Markus Armbruster]

Wen Congyang  writes:

> On 09/15/2015 03:37 PM, Markus Armbruster wrote:
>> Wen Congyang  writes:
>> 
>>> On 09/14/2015 11:47 PM, Eric Blake wrote:
 On 09/14/2015 08:27 AM, Markus Armbruster wrote:
> Wen Congyang  writes:
>
>> The NBD driver needs: filename, path or (host, port, exportname).
>> It checks which key exists and decides use unix or inet socket.
>> It doesn't recognize the key type, so we can't use union, and
>> can't reuse InetSocketAddress.
>>
>> Signed-off-by: Wen Congyang 
>> Signed-off-by: zhanghailiang 
>> Signed-off-by: Gonglei 
>> ---
>>  qapi/block-core.json | 42 --
>>  1 file changed, 40 insertions(+), 2 deletions(-)
>>

>>  ##
>> +# @BlockdevOptionsNBD
>> +#
>> +# Driver specific block device options for NBD
>> +#
>> +# @filename: #optional unix or inet path. The format is:
>> +#unix: nbd+unix:///export?socket=path or
>> +#  nbd:unix:path:exportname=export
>> +#inet: nbd[+tcp]://host[:port]/export or
>> +#  nbd:host[:port]:exportname=export

 Yuck. You are passing structured data through a single 'str', when you
 SHOULD be passing it through structured JSON.  Just because we have a
 filename shorthand for convenience does NOT mean that we want to expose
 that convenience in QMP.  Instead, we really want the breakdown of the
 pieces (here, using abbreviated syntax of an inline base, since there
 are pending qapi patches that will allow it):
>>>
>>> Do you mean that: there is no need to support filename?
>> 
>> Rule of thumb: if the QMP command handler needs to parse a string
>> argument, that argument should be a complex QAPI type instead.
>> 
>> Example: @filename needs to be parsed into its components, either
>> 
>> * protocol unix, socket path, export name, or
>> * protocol tcp, host, port, export name
>> 
>> Since there's an either/or, the complex QAPI type should be a union.
>> 
 { 'enum': 'NBDTransport', 'data': ['unix', 'tcp', 'udp'] }
>>>
>>> NBD only uses tcp, it doesn't support udp.
>>>
 { 'union': 'BlockdevOptionsNBD',
   'base': { 'transport': 'NBDTransport', 'export': 'str' },
   'discriminator': 'transport',
   'data': { 'unix': 'NBDUnix', 'tcp': 'NBDInet', 'udp': 'NBDInet' } }
 { 'struct': 'NBDUnix', 'data': { 'socket': 'str' } }
>>>
>>> unix socket needs a path, and I think we can use UnixSocketAddress here.
>> 
>> Yes, we should try to reuse common types like SocketAddress,
>> InetSocketAddress, UnixSocketAddress.
>> 
>> Perhaps it could be as simple as
>> 
>> { 'struct': 'BlockdevOptionsNBD',
>>   'data': { 'addr: 'SocketAddress', 'export': 'str' } }
>
> The problem is that: NBD doesn't use the fd.

Is that fundamental, or just a matter of implementation?

> Another question is: what key will we see in nbd_open()? "addr.host"
> or "host"?

As long as nbd_config() looks for "host" in the options QDict, we need
to put "host".

> Thanks
> Wen Congyang
>
>> 
>> Eric, what do you think?
>> 
 { 'struct': 'NBDInet', 'data': { 'host': 'str', '*port': 'int',
  '*ipv4': 'bool', '*ipv6': 'bool' } }
>>>
>>> Thanks for the above, and I will try it.
>> 
>> [...]
>> .
>> 

Re: [Qemu-devel] [PATCH v7 19/26] qapi: Improve built-in type documentation

[Daniel P. Berrange]

On Mon, Sep 14, 2015 at 07:57:50PM +0200, Markus Armbruster wrote:
> Clarify how they map to JSON.  Add how they map to C.  Fix the
> reference to StringInputVisitor.
> 
> Signed-off-by: Markus Armbruster 
> Reviewed-by: Eric Blake 
> ---
>  docs/qapi-code-gen.txt | 29 ++---
>  1 file changed, 18 insertions(+), 11 deletions(-)

Reviewed-by: Daniel P. Berrange 


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|