Re: [Qemu-devel] [RFC PATCH COLO v2 12/13] skip nbd_target when starting block replication

[Fam Zheng]

On Wed, 03/25 17:36, Wen Congyang wrote:
> Signed-off-by: Wen Congyang 
> Signed-off-by: zhanghailiang 
> Signed-off-by: Gonglei 
> ---
>  block.c | 18 ++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/block.c b/block.c
> index bd7fa9c..3af5ad4 100644
> --- a/block.c
> +++ b/block.c
> @@ -6368,6 +6368,12 @@ BlockAcctStats *bdrv_get_stats(BlockDriverState *bs)
>  void bdrv_start_replication(BlockDriverState *bs, COLOMode mode, Error 
> **errp)
>  {
>  BlockDriver *drv = bs->drv;
> +Error *local_err = NULL;
> +
> +if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKING_REFERENCE, &local_err)) 
> {
> +error_free(local_err);

local_err is set but not used, just pass NULL. Same below.

> +return;
> +}
>  
>  if (drv && drv->bdrv_start_replication) {
>  drv->bdrv_start_replication(bs, mode, errp);
> @@ -6381,6 +6387,12 @@ void bdrv_start_replication(BlockDriverState *bs, 
> COLOMode mode, Error **errp)
>  void bdrv_do_checkpoint(BlockDriverState *bs, Error **errp)
>  {
>  BlockDriver *drv = bs->drv;
> +Error *local_err = NULL;
> +
> +if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKING_REFERENCE, &local_err)) 
> {
> +error_free(local_err);
> +return;
> +}
>  
>  if (drv && drv->bdrv_do_checkpoint) {
>  drv->bdrv_do_checkpoint(bs, errp);
> @@ -6394,6 +6406,12 @@ void bdrv_do_checkpoint(BlockDriverState *bs, Error 
> **errp)
>  void bdrv_stop_replication(BlockDriverState *bs, Error **errp)
>  {
>  BlockDriver *drv = bs->drv;
> +Error *local_err = NULL;
> +
> +if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKING_REFERENCE, &local_err)) 
> {
> +error_free(local_err);
> +return;
> +}
>  
>  if (drv && drv->bdrv_stop_replication) {
>  drv->bdrv_stop_replication(bs, errp);
> -- 
> 2.1.0
> 

Re: [Qemu-devel] [RFC PATCH COLO v2 08/13] Allow creating backup jobs when opening BDS

[Wen Congyang]

On 03/26/2015 03:07 PM, Fam Zheng wrote:
> On Wed, 03/25 17:36, Wen Congyang wrote:
>> When opening BDS, we need to create backup jobs for
>> image-fleecing.
> 
> How does this commit message relate to the Makefile change?

Hmm, we need to use backup API in block.c, and block.o will
be used by qemu-img which doesn't use common-obj.

Thanks
Wen Congyang

> 
>>
>> Signed-off-by: Wen Congyang 
>> Signed-off-by: zhanghailiang 
>> Signed-off-by: Gonglei 
>> Cc: Jeff Cody 
>> ---
>>  block/Makefile.objs | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/block/Makefile.objs b/block/Makefile.objs
>> index db2933e..0950973 100644
>> --- a/block/Makefile.objs
>> +++ b/block/Makefile.objs
>> @@ -21,10 +21,10 @@ block-obj-$(CONFIG_ARCHIPELAGO) += archipelago.o
>>  block-obj-$(CONFIG_LIBSSH2) += ssh.o
>>  block-obj-y += accounting.o
>>  block-obj-y += write-threshold.o
>> +block-obj-y += backup.o
>>  
>>  common-obj-y += stream.o
>>  common-obj-y += commit.o
>> -common-obj-y += backup.o
>>  
>>  iscsi.o-cflags := $(LIBISCSI_CFLAGS)
>>  iscsi.o-libs   := $(LIBISCSI_LIBS)
>> -- 
>> 2.1.0
>>
> .
> 

Re: [Qemu-devel] [RFC PATCH COLO v2 12/13] skip nbd_target when starting block replication

[Wen Congyang]

On 03/26/2015 03:03 PM, Fam Zheng wrote:
> On Wed, 03/25 17:36, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang 
>> Signed-off-by: zhanghailiang 
>> Signed-off-by: Gonglei 
>> ---
>>  block.c | 18 ++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/block.c b/block.c
>> index bd7fa9c..3af5ad4 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -6368,6 +6368,12 @@ BlockAcctStats *bdrv_get_stats(BlockDriverState *bs)
>>  void bdrv_start_replication(BlockDriverState *bs, COLOMode mode, Error 
>> **errp)
>>  {
>>  BlockDriver *drv = bs->drv;
>> +Error *local_err = NULL;
>> +
>> +if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKING_REFERENCE, 
>> &local_err)) {
>> +error_free(local_err);
> 
> local_err is set but not used, just pass NULL. Same below.

Yes, will fix it in the next version.

Thanks
Wen Congyang

> 
>> +return;
>> +}
>>  
>>  if (drv && drv->bdrv_start_replication) {
>>  drv->bdrv_start_replication(bs, mode, errp);
>> @@ -6381,6 +6387,12 @@ void bdrv_start_replication(BlockDriverState *bs, 
>> COLOMode mode, Error **errp)
>>  void bdrv_do_checkpoint(BlockDriverState *bs, Error **errp)
>>  {
>>  BlockDriver *drv = bs->drv;
>> +Error *local_err = NULL;
>> +
>> +if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKING_REFERENCE, 
>> &local_err)) {
>> +error_free(local_err);
>> +return;
>> +}
>>  
>>  if (drv && drv->bdrv_do_checkpoint) {
>>  drv->bdrv_do_checkpoint(bs, errp);
>> @@ -6394,6 +6406,12 @@ void bdrv_do_checkpoint(BlockDriverState *bs, Error 
>> **errp)
>>  void bdrv_stop_replication(BlockDriverState *bs, Error **errp)
>>  {
>>  BlockDriver *drv = bs->drv;
>> +Error *local_err = NULL;
>> +
>> +if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKING_REFERENCE, 
>> &local_err)) {
>> +error_free(local_err);
>> +return;
>> +}
>>  
>>  if (drv && drv->bdrv_stop_replication) {
>>  drv->bdrv_stop_replication(bs, errp);
>> -- 
>> 2.1.0
>>
> .
> 

Re: [Qemu-devel] [RFC PATCH COLO v2 08/13] Allow creating backup jobs when opening BDS

[Fam Zheng]

On Wed, 03/25 17:36, Wen Congyang wrote:
> When opening BDS, we need to create backup jobs for
> image-fleecing.

How does this commit message relate to the Makefile change?

> 
> Signed-off-by: Wen Congyang 
> Signed-off-by: zhanghailiang 
> Signed-off-by: Gonglei 
> Cc: Jeff Cody 
> ---
>  block/Makefile.objs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index db2933e..0950973 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -21,10 +21,10 @@ block-obj-$(CONFIG_ARCHIPELAGO) += archipelago.o
>  block-obj-$(CONFIG_LIBSSH2) += ssh.o
>  block-obj-y += accounting.o
>  block-obj-y += write-threshold.o
> +block-obj-y += backup.o
>  
>  common-obj-y += stream.o
>  common-obj-y += commit.o
> -common-obj-y += backup.o
>  
>  iscsi.o-cflags := $(LIBISCSI_CFLAGS)
>  iscsi.o-libs   := $(LIBISCSI_LIBS)
> -- 
> 2.1.0
> 

Re: [Qemu-devel] [RFC PATCH COLO v2 04/13] Add new block driver interfaces to control block replication

[Fam Zheng]

On Wed, 03/25 17:36, Wen Congyang wrote:
> Signed-off-by: Wen Congyang 
> Signed-off-by: zhanghailiang 
> Signed-off-by: Gonglei 
> Cc: Luiz Capitulino 
> Cc: Michael Roth 
> ---
>  block.c   | 39 +++
>  include/block/block.h |  4 
>  include/block/block_int.h | 11 +++
>  qapi/block.json   | 16 
>  4 files changed, 70 insertions(+)
> 
> diff --git a/block.c b/block.c
> index 0fe97de..0ff5cf8 100644
> --- a/block.c
> +++ b/block.c
> @@ -6196,3 +6196,42 @@ BlockAcctStats *bdrv_get_stats(BlockDriverState *bs)
>  {
>  return &bs->stats;
>  }
> +
> +void bdrv_start_replication(BlockDriverState *bs, COLOMode mode, Error 
> **errp)
> +{
> +BlockDriver *drv = bs->drv;
> +
> +if (drv && drv->bdrv_start_replication) {
> +drv->bdrv_start_replication(bs, mode, errp);
> +} else if (bs->file) {
> +bdrv_start_replication(bs->file, mode, errp);
> +} else {
> +error_set(errp, QERR_UNSUPPORTED);

I think we should use error_setg in new code? (The same to following ones)

> +}
> +}
> +
> +void bdrv_do_checkpoint(BlockDriverState *bs, Error **errp)
> +{
> +BlockDriver *drv = bs->drv;
> +
> +if (drv && drv->bdrv_do_checkpoint) {
> +drv->bdrv_do_checkpoint(bs, errp);
> +} else if (bs->file) {
> +bdrv_do_checkpoint(bs->file, errp);
> +} else {
> +error_set(errp, QERR_UNSUPPORTED);
> +}
> +}
> +
> +void bdrv_stop_replication(BlockDriverState *bs, Error **errp)
> +{
> +BlockDriver *drv = bs->drv;
> +
> +if (drv && drv->bdrv_stop_replication) {
> +drv->bdrv_stop_replication(bs, errp);
> +} else if (bs->file) {
> +bdrv_stop_replication(bs->file, errp);
> +} else {
> +error_set(errp, QERR_UNSUPPORTED);
> +}
> +}
> diff --git a/include/block/block.h b/include/block/block.h
> index 4c57d63..68f3b1a 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -569,4 +569,8 @@ void bdrv_flush_io_queue(BlockDriverState *bs);
>  
>  BlockAcctStats *bdrv_get_stats(BlockDriverState *bs);
>  
> +void bdrv_start_replication(BlockDriverState *bs, COLOMode mode, Error 
> **errp);
> +void bdrv_do_checkpoint(BlockDriverState *bs, Error **errp);
> +void bdrv_stop_replication(BlockDriverState *bs, Error **errp);
> +
>  #endif
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index dccb092..08dd8ba 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -290,6 +290,17 @@ struct BlockDriver {
>   */
>  int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo);
>  
> +
> +void (*bdrv_start_replication)(BlockDriverState *bs, COLOMode mode,
> +   Error **errp);

Need some documentation, but I have a generic question:

Why is a single interface with modes better than different functions for each
mode (bdrv_start_replication_{primary,secondary}? Asking because the behavior
is very different between them, and I don't see much sharing -- you implement
primary operation in quorum, and secondary in qcow2+colo.

> +/* Drop Disk buffer when doing checkpoint. */
> +void (*bdrv_do_checkpoint)(BlockDriverState *bs, Error **errp);
> +/*
> + * After failover, we should flush Disk buffer into secondary disk
> + * and stop block replication.
> + */
> +void (*bdrv_stop_replication)(BlockDriverState *bs, Error **errp);
> +
>  QLIST_ENTRY(BlockDriver) list;
>  };
>  
> diff --git a/qapi/block.json b/qapi/block.json
> index e313465..e640566 100644
> --- a/qapi/block.json
> +++ b/qapi/block.json
> @@ -40,6 +40,22 @@
>'data': ['auto', 'none', 'lba', 'large', 'rechs']}
>  
>  ##
> +# @COLOMode
> +#
> +# An enumeration of COLO mode.
> +#
> +# @unprotected: COLO is not started or after failover
> +#
> +# @primary: Primary mode, the vm's state will be sent to secondary QEMU.
> +#
> +# @secondary: Secondary mode, receive the vm's state from primary QEMU.
> +#
> +# Since: 2.4
> +##
> +{ 'enum' : 'COLOMode',
> +  'data' : ['unprotected', 'primary', 'secondary']}

If split bdrv_start_replication, do we still need an enum? I can't find the
usage in QMP interface, is it in some other series?

Fam

> +
> +##
>  # @BlockdevSnapshotInternal
>  #
>  # @device: the name of the device to generate the snapshot from
> -- 
> 2.1.0
> 

Re: [Qemu-devel] [RFC PATCH COLO v2 01/13] docs: block replication's description

[Wen Congyang]

On 03/26/2015 02:31 PM, Fam Zheng wrote:
> On Wed, 03/25 17:36, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang 
>> Signed-off-by: Paolo Bonzini 
>> Signed-off-by: Yang Hongyang 
>> Signed-off-by: zhanghailiang 
>> Signed-off-by: Gonglei 
>> ---
>>  docs/block-replication.txt | 147 
>> +
>>  1 file changed, 147 insertions(+)
>>  create mode 100644 docs/block-replication.txt
>>
>> diff --git a/docs/block-replication.txt b/docs/block-replication.txt
>> new file mode 100644
>> index 000..874ed8e
>> --- /dev/null
>> +++ b/docs/block-replication.txt
>> @@ -0,0 +1,147 @@
>> +Block replication
>> +
>> +Copyright Fujitsu, Corp. 2015
>> +Copyright (c) 2015 Intel Corporation
>> +Copyright (c) 2015 HUAWEI TECHNOLOGIES CO.,LTD.
>> +
>> +This work is licensed under the terms of the GNU GPL, version 2 or later.
>> +See the COPYING file in the top-level directory.
>> +
>> +The block replication is used for continuous checkpoints. It is designed
>> +for COLO that Secondary VM is running. It can also be applied for FT/HA
>> +scene that Secondary VM is not running.
>> +
>> +This document gives an overview of block replication's design.
>> +
>> +== Background ==
>> +High availability solutions such as micro checkpoint and COLO will do
>> +consecutive checkpoint. The VM state of Primary VM and Secondary VM is
>> +identical right after a VM checkpoint, but becomes different as the VM
>> +executes till the next checkpoint. To support disk contents checkpoint,
>> +the modified disk contents in the Secondary VM must be buffered, and are
>> +only dropped at next checkpoint time. To reduce the network transportation
>> +effort at the time of checkpoint, the disk modification operations of
>> +Primary disk are asynchronously forwarded to the Secondary node.
>> +
>> +== Workflow ==
>> +The following is the image of block replication workflow:
>> +
>> ++--+++
>> +|Primary Write Requests||Secondary Write Requests|
>> ++--+++
>> +  |   |
>> +  |  (4)
>> +  |   V
>> +  |  /-\
>> +  |  Copy and Forward| |
>> +  |-(1)--+   | Disk Buffer |
>> +  |  |   | |
>> +  | (3)  \-/
>> +  | speculative  ^
>> +  |write through(2)
>> +  |  |   |
>> +  V  V   |
>> +   +--+   ++
>> +   | Primary Disk |   | Secondary Disk |
>> +   +--+   ++
>> +
>> +1) Primary write requests will be copied and forwarded to Secondary
>> +   QEMU.
>> +2) Before Primary write requests are written to Secondary disk, the
>> +   original sector content will be read from Secondary disk and
>> +   buffered in the Disk buffer, but it will not overwrite the existing
>> +   sector content in the Disk buffer.
> 
> Could you elaborate a bit about the "existing sector content" here? IIUC, it
> could be from either "Secondary Write Requests" or previous COW of "Primary
> Write Requests". Is that right?

Yes.

> 
>> +3) Primary write requests will be written to Secondary disk.
>> +4) Secondary write requests will be buffered in the Disk buffer and it
>> +   will overwrite the existing sector content in the buffer.
>> +
>> +== Architecture ==
>> +We are going to implement COLO block replication from many basic
>> +blocks that are already in QEMU.
>> +
>> + virtio-blk   ||
>> + ^||.--
>> + |||| Secondary
>> +1 Quorum  ||'--
>> + /  \ ||
>> +/\||
>> +   Primary  2 NBD  --->  2 NBD
>> + disk   client|| server 
>> virtio-blk
>> +  ||^   
>>  ^
>> +. |||   
>>  |
>> +Primary | ||  Secondary disk <- hidden-disk 4 
>> <- active-disk 3
>> +' |||  backing^   
>> backing
>> +  ||| |
>> +  ||  

Re: [Qemu-devel] [RFC PATCH COLO v2 08/13] Allow creating backup jobs when opening BDS

[Fam Zheng]

On Thu, 03/26 15:14, Wen Congyang wrote:
> On 03/26/2015 03:07 PM, Fam Zheng wrote:
> > On Wed, 03/25 17:36, Wen Congyang wrote:
> >> When opening BDS, we need to create backup jobs for
> >> image-fleecing.
> > 
> > How does this commit message relate to the Makefile change?
> 
> Hmm, we need to use backup API in block.c, and block.o will
> be used by qemu-img which doesn't use common-obj.

I see. How about adding the referenced functions to stubs/?

Fam

> 
> Thanks
> Wen Congyang
> 
> > 
> >>
> >> Signed-off-by: Wen Congyang 
> >> Signed-off-by: zhanghailiang 
> >> Signed-off-by: Gonglei 
> >> Cc: Jeff Cody 
> >> ---
> >>  block/Makefile.objs | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/block/Makefile.objs b/block/Makefile.objs
> >> index db2933e..0950973 100644
> >> --- a/block/Makefile.objs
> >> +++ b/block/Makefile.objs
> >> @@ -21,10 +21,10 @@ block-obj-$(CONFIG_ARCHIPELAGO) += archipelago.o
> >>  block-obj-$(CONFIG_LIBSSH2) += ssh.o
> >>  block-obj-y += accounting.o
> >>  block-obj-y += write-threshold.o
> >> +block-obj-y += backup.o
> >>  
> >>  common-obj-y += stream.o
> >>  common-obj-y += commit.o
> >> -common-obj-y += backup.o
> >>  
> >>  iscsi.o-cflags := $(LIBISCSI_CFLAGS)
> >>  iscsi.o-libs   := $(LIBISCSI_LIBS)
> >> -- 
> >> 2.1.0
> >>
> > .
> > 
> 

Re: [Qemu-devel] [RFC PATCH COLO v2 04/13] Add new block driver interfaces to control block replication

[Wen Congyang]

On 03/26/2015 03:12 PM, Fam Zheng wrote:
> On Wed, 03/25 17:36, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang 
>> Signed-off-by: zhanghailiang 
>> Signed-off-by: Gonglei 
>> Cc: Luiz Capitulino 
>> Cc: Michael Roth 
>> ---
>>  block.c   | 39 +++
>>  include/block/block.h |  4 
>>  include/block/block_int.h | 11 +++
>>  qapi/block.json   | 16 
>>  4 files changed, 70 insertions(+)
>>
>> diff --git a/block.c b/block.c
>> index 0fe97de..0ff5cf8 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -6196,3 +6196,42 @@ BlockAcctStats *bdrv_get_stats(BlockDriverState *bs)
>>  {
>>  return &bs->stats;
>>  }
>> +
>> +void bdrv_start_replication(BlockDriverState *bs, COLOMode mode, Error 
>> **errp)
>> +{
>> +BlockDriver *drv = bs->drv;
>> +
>> +if (drv && drv->bdrv_start_replication) {
>> +drv->bdrv_start_replication(bs, mode, errp);
>> +} else if (bs->file) {
>> +bdrv_start_replication(bs->file, mode, errp);
>> +} else {
>> +error_set(errp, QERR_UNSUPPORTED);
> 
> I think we should use error_setg in new code? (The same to following ones)

Hmm, do you mean that don't use QERR_UNSUPPORTED here?

> 
>> +}
>> +}
>> +
>> +void bdrv_do_checkpoint(BlockDriverState *bs, Error **errp)
>> +{
>> +BlockDriver *drv = bs->drv;
>> +
>> +if (drv && drv->bdrv_do_checkpoint) {
>> +drv->bdrv_do_checkpoint(bs, errp);
>> +} else if (bs->file) {
>> +bdrv_do_checkpoint(bs->file, errp);
>> +} else {
>> +error_set(errp, QERR_UNSUPPORTED);
>> +}
>> +}
>> +
>> +void bdrv_stop_replication(BlockDriverState *bs, Error **errp)
>> +{
>> +BlockDriver *drv = bs->drv;
>> +
>> +if (drv && drv->bdrv_stop_replication) {
>> +drv->bdrv_stop_replication(bs, errp);
>> +} else if (bs->file) {
>> +bdrv_stop_replication(bs->file, errp);
>> +} else {
>> +error_set(errp, QERR_UNSUPPORTED);
>> +}
>> +}
>> diff --git a/include/block/block.h b/include/block/block.h
>> index 4c57d63..68f3b1a 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -569,4 +569,8 @@ void bdrv_flush_io_queue(BlockDriverState *bs);
>>  
>>  BlockAcctStats *bdrv_get_stats(BlockDriverState *bs);
>>  
>> +void bdrv_start_replication(BlockDriverState *bs, COLOMode mode, Error 
>> **errp);
>> +void bdrv_do_checkpoint(BlockDriverState *bs, Error **errp);
>> +void bdrv_stop_replication(BlockDriverState *bs, Error **errp);
>> +
>>  #endif
>> diff --git a/include/block/block_int.h b/include/block/block_int.h
>> index dccb092..08dd8ba 100644
>> --- a/include/block/block_int.h
>> +++ b/include/block/block_int.h
>> @@ -290,6 +290,17 @@ struct BlockDriver {
>>   */
>>  int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo);
>>  
>> +
>> +void (*bdrv_start_replication)(BlockDriverState *bs, COLOMode mode,
>> +   Error **errp);
> 
> Need some documentation, but I have a generic question:
> 
> Why is a single interface with modes better than different functions for each
> mode (bdrv_start_replication_{primary,secondary}? Asking because the behavior
> is very different between them, and I don't see much sharing -- you implement
> primary operation in quorum, and secondary in qcow2+colo.

No special reason.

> 
>> +/* Drop Disk buffer when doing checkpoint. */
>> +void (*bdrv_do_checkpoint)(BlockDriverState *bs, Error **errp);
>> +/*
>> + * After failover, we should flush Disk buffer into secondary disk
>> + * and stop block replication.
>> + */
>> +void (*bdrv_stop_replication)(BlockDriverState *bs, Error **errp);
>> +
>>  QLIST_ENTRY(BlockDriver) list;
>>  };
>>  
>> diff --git a/qapi/block.json b/qapi/block.json
>> index e313465..e640566 100644
>> --- a/qapi/block.json
>> +++ b/qapi/block.json
>> @@ -40,6 +40,22 @@
>>'data': ['auto', 'none', 'lba', 'large', 'rechs']}
>>  
>>  ##
>> +# @COLOMode
>> +#
>> +# An enumeration of COLO mode.
>> +#
>> +# @unprotected: COLO is not started or after failover
>> +#
>> +# @primary: Primary mode, the vm's state will be sent to secondary QEMU.
>> +#
>> +# @secondary: Secondary mode, receive the vm's state from primary QEMU.
>> +#
>> +# Since: 2.4
>> +##
>> +{ 'enum' : 'COLOMode',
>> +  'data' : ['unprotected', 'primary', 'secondary']}
> 
> If split bdrv_start_replication, do we still need an enum? I can't find the
> usage in QMP interface, is it in some other series?

I will check it.

Thanks
Wen Congyang

> 
> Fam
> 
>> +
>> +##
>>  # @BlockdevSnapshotInternal
>>  #
>>  # @device: the name of the device to generate the snapshot from
>> -- 
>> 2.1.0
>>
> .
> 

Re: [Qemu-devel] [RFC PATCH COLO v2 08/13] Allow creating backup jobs when opening BDS

[Wen Congyang]

On 03/26/2015 03:18 PM, Fam Zheng wrote:
> On Thu, 03/26 15:14, Wen Congyang wrote:
>> On 03/26/2015 03:07 PM, Fam Zheng wrote:
>>> On Wed, 03/25 17:36, Wen Congyang wrote:
 When opening BDS, we need to create backup jobs for
 image-fleecing.
>>>
>>> How does this commit message relate to the Makefile change?
>>
>> Hmm, we need to use backup API in block.c, and block.o will
>> be used by qemu-img which doesn't use common-obj.
> 
> I see. How about adding the referenced functions to stubs/?

Good idea. I will try it.

Thanks
Wen Congyang

> 
> Fam
> 
>>
>> Thanks
>> Wen Congyang
>>
>>>

 Signed-off-by: Wen Congyang 
 Signed-off-by: zhanghailiang 
 Signed-off-by: Gonglei 
 Cc: Jeff Cody 
 ---
  block/Makefile.objs | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/block/Makefile.objs b/block/Makefile.objs
 index db2933e..0950973 100644
 --- a/block/Makefile.objs
 +++ b/block/Makefile.objs
 @@ -21,10 +21,10 @@ block-obj-$(CONFIG_ARCHIPELAGO) += archipelago.o
  block-obj-$(CONFIG_LIBSSH2) += ssh.o
  block-obj-y += accounting.o
  block-obj-y += write-threshold.o
 +block-obj-y += backup.o
  
  common-obj-y += stream.o
  common-obj-y += commit.o
 -common-obj-y += backup.o
  
  iscsi.o-cflags := $(LIBISCSI_CFLAGS)
  iscsi.o-libs   := $(LIBISCSI_LIBS)
 -- 
 2.1.0

>>> .
>>>
>>
> .
> 

Re: [Qemu-devel] [RFC PATCH COLO v2 06/13] NBD client: implement block driver interfaces for block replication

[Fam Zheng]

On Wed, 03/25 17:36, Wen Congyang wrote:
> Signed-off-by: Wen Congyang 
> Signed-off-by: zhanghailiang 
> Signed-off-by: Gonglei 
> ---
>  block/nbd.c | 49 +
>  1 file changed, 49 insertions(+)
> 
> diff --git a/block/nbd.c b/block/nbd.c
> index 3faf865..753b322 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -458,6 +458,52 @@ static void nbd_refresh_filename(BlockDriverState *bs)
>  bs->full_open_options = opts;
>  }
>  
> +static void nbd_start_replication(BlockDriverState *bs, COLOMode mode,
> +  Error **errp)
> +{
> +BDRVNBDState *s = bs->opaque;
> +
> +/*
> + * TODO: support COLO_SECONDARY_MODE if we allow secondary
> + * QEMU becoming primary QEMU.
> + */
> +if (mode != COLO_MODE_PRIMARY) {
> +error_set(errp, QERR_INVALID_PARAMETER, "mode");

Please use error_setg. (Please grep the whole series :)

Fam

Re: [Qemu-devel] [RFC PATCH COLO v2 06/13] NBD client: implement block driver interfaces for block replication

[Wen Congyang]

On 03/26/2015 03:21 PM, Fam Zheng wrote:
> On Wed, 03/25 17:36, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang 
>> Signed-off-by: zhanghailiang 
>> Signed-off-by: Gonglei 
>> ---
>>  block/nbd.c | 49 +
>>  1 file changed, 49 insertions(+)
>>
>> diff --git a/block/nbd.c b/block/nbd.c
>> index 3faf865..753b322 100644
>> --- a/block/nbd.c
>> +++ b/block/nbd.c
>> @@ -458,6 +458,52 @@ static void nbd_refresh_filename(BlockDriverState *bs)
>>  bs->full_open_options = opts;
>>  }
>>  
>> +static void nbd_start_replication(BlockDriverState *bs, COLOMode mode,
>> +  Error **errp)
>> +{
>> +BDRVNBDState *s = bs->opaque;
>> +
>> +/*
>> + * TODO: support COLO_SECONDARY_MODE if we allow secondary
>> + * QEMU becoming primary QEMU.
>> + */
>> +if (mode != COLO_MODE_PRIMARY) {
>> +error_set(errp, QERR_INVALID_PARAMETER, "mode");
> 
> Please use error_setg. (Please grep the whole series :)

Why? QERR_INVALID_PARAMETER includes ERROR_CLASS_GENERIC_ERROR.

Thanks
Wen Congyang

> 
> Fam
> .
> 

Re: [Qemu-devel] [RFC PATCH COLO v2 09/13] block: Parse "backing_reference" option to reference existing BDS

[Fam Zheng]

On Wed, 03/25 17:36, Wen Congyang wrote:
> Usage:
> -drive file=xxx,id=Y, \
> -drive 
> file=,id=X,backing_reference.drive_id=Y,backing_reference.hidden-disk.*
> 
> It will create such backing chain:
>{virtio-blk dev 'Y'}  
> {virtio-blk dev 'X'}
>  |
>   |
>  |
>   |
>  v
>   v
> 
> [base] <- [mid] <- ( Y )  <- (hidden target) 
> <--- ( X )
> 
>  v  ^
>  v  ^
>  v  ^
>  v  ^
>   drive-backup sync=none 
> 
> X's backing file is hidden-disk, and hidden-disk's backing file is Y.
> Disk Y may be opened or reopened in read-write mode, so A block backup
> job is automatically created: source is Y and target is hidden-disk.
> 
> Signed-off-by: Wen Congyang 
> Signed-off-by: zhanghailiang 
> Signed-off-by: Gonglei 
> ---
>  block.c| 145 
> -
>  include/block/block.h  |   1 +
>  include/block/block_int.h  |   1 +
>  tests/qemu-iotests/051 |  13 
>  tests/qemu-iotests/051.out |  13 
>  5 files changed, 170 insertions(+), 3 deletions(-)
> 
> diff --git a/block.c b/block.c
> index b4d629e..bd7fa9c 100644
> --- a/block.c
> +++ b/block.c
> @@ -1351,6 +1351,113 @@ free_exit:
>  return ret;
>  }
>  
> +static void backing_reference_completed(void *opaque, int ret)
> +{
> +BlockDriverState *hidden_disk = opaque;
> +
> +assert(!hidden_disk->backing_reference);
> +}
> +
> +static int bdrv_open_backing_reference_file(BlockDriverState *bs,
> +QDict *options, Error **errp)
> +{
> +const char *backing_name;
> +QDict *hidden_disk_options = NULL;
> +BlockDriverState *backing_hd, *hidden_disk;
> +BlockBackend *backing_blk;
> +Error *local_err = NULL;
> +int ret = 0;
> +
> +backing_name = qdict_get_try_str(options, "drive_id");
> +if (!backing_name) {
> +error_setg(errp, "Backing reference needs option drive_id");
> +ret = -EINVAL;
> +goto free_exit;
> +}
> +qdict_del(options, "drive_id");
> +
> +qdict_extract_subqdict(options, &hidden_disk_options, "hidden-disk.");
> +if (!qdict_size(hidden_disk_options)) {
> +error_setg(errp, "Backing reference needs option hidden-disk.*");
> +ret = -EINVAL;
> +goto free_exit;
> +}
> +
> +if (qdict_size(options)) {
> +const QDictEntry *entry = qdict_first(options);
> +error_setg(errp, "Backing reference used by '%s' doesn't support "
> +   "the option '%s'", bdrv_get_device_name(bs), entry->key);
> +ret = -EINVAL;
> +goto free_exit;
> +}
> +
> +backing_blk = blk_by_name(backing_name);
> +if (!backing_blk) {
> +error_set(errp, QERR_DEVICE_NOT_FOUND, backing_name);
> +ret = -ENOENT;
> +goto free_exit;
> +}
> +
> +backing_hd = blk_bs(backing_blk);
> +/* Backing reference itself? */
> +if (backing_hd == bs || bdrv_find_overlay(backing_hd, bs)) {
> +error_setg(errp, "Backing reference itself");
> +ret = -EINVAL;
> +goto free_exit;
> +}
> +
> +if (bdrv_op_is_blocked(backing_hd, BLOCK_OP_TYPE_BACKING_REFERENCE,
> +   errp)) {
> +ret = -EBUSY;
> +goto free_exit;
> +}
> +
> +/* hidden-disk is bs's backing file */
> +ret = bdrv_open_backing_file(bs, hidden_disk_options, errp);
> +hidden_disk_options = NULL;
> +if (ret < 0) {
> +goto free_exit;
> +}
> +
> +hidden_disk = bs->backing_hd;
> +if (!hidden_disk->drv || !hidden_disk->drv->supports_backing) {
> +ret = -EINVAL;
> +error_setg(errp, "Hidden disk's driver doesn't support backing 
> files");
> +goto free_exit;
> +}
> +
> +bdrv_set_backing_hd(hidden_disk, backing_hd);
> +bdrv_ref(backing_hd);
> +
> +/*
> + * backing hd may be opened or reopened in read-write mode, so we
> + * should backup backing hd to hidden disk
> + */
> +bdrv_op_unblock(hidden_disk, BLOCK_OP_TYPE_BACKUP_TARGET,
> +bs->backing_blocker);
> +bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_BACKUP_SOURCE,
> +hidden_disk->backing_blocker);
> +
> +bdrv_ref(hidden_disk);
> +backup_start(backing_hd, hidden_disk, 0, MIRROR_SYNC_MODE_NONE,
> + BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT,
> + backing_reference_completed, hidden_disk, &local_err);

We need t

[Qemu-devel] [PATCH] virtio-scsi-dataplane: fix memory leak for VirtIOSCSIVring

[Ting Wang]

VirtIOSCSIVring which allocated in virtio_scsi_vring_init
should be free when dataplane has been stopped or failed to start.

Signed-off-by: Ting Wang 
---
 hw/scsi/virtio-scsi-dataplane.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index c069cd7..5575648 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -182,13 +182,19 @@ static void virtio_scsi_vring_teardown(VirtIOSCSI *s)
 
 if (s->ctrl_vring) {
 vring_teardown(&s->ctrl_vring->vring, vdev, 0);
+g_slice_free(VirtIOSCSIVring, s->ctrl_vring);
+s->ctrl_vring = NULL;
 }
 if (s->event_vring) {
 vring_teardown(&s->event_vring->vring, vdev, 1);
+g_slice_free(VirtIOSCSIVring, s->event_vring);
+s->event_vring = NULL;
 }
 if (s->cmd_vrings) {
 for (i = 0; i < vs->conf.num_queues && s->cmd_vrings[i]; i++) {
 vring_teardown(&s->cmd_vrings[i]->vring, vdev, 2 + i);
+g_slice_free(VirtIOSCSIVring, s->cmd_vrings[i]);
+s->cmd_vrings[i] = NULL;
 }
 free(s->cmd_vrings);
 s->cmd_vrings = NULL;
-- 
1.8.5

Re: [Qemu-devel] [PATCH v8 0/9] rocker: add new rocker ethernet switch device

[Scott Feldman]

Ping

On Fri, Mar 13, 2015 at 9:09 PM,   wrote:
> From: Scott Feldman 
>
> v8:
>
>  - From Stefan Hajnoczi's net-pull-request v3, merge in these changes:
>  - Squash David Ahern's clang struct definition warnings fix
>  - Squash in Jiri's fix for rocker format string specifiers [Peter]
>  - Squash in Windows build fix [Peter]
>
>  - In addition, fix 2 compile warnings from net-pull-request v4:
>  - Fix compile issue for min glibc 2.12
>  - w32: fix 64 bit constant without ULL suffix
>
> v7:
>
>  - Per Stefan Hajnoczi comments:
>  - #ifdef CONFIG_ROCKER wrapper around qmp/hmp to fix compile when PCI is
>disabled or rocker is disabled.
>
> v6:
>
>  - Per Stefan Hajnoczi review:
>  - Move tests to tests/rocker
>  - Fix some mem leaks
>  - Fix doc grammer/spelling
>  - Per Eric Blake review:
>  - Add #optional to optional args comments in qmp interface
>  - Add "query-" prefix to qmp cmds
>  - Fix doc grammer/spelling
>
> v5:
>
>  - Per Jason Wang review:
>  - Fix some missing/wrong references in the rocker.txt spec
>  - mark rocker as unmigratable.
>
> v4:
>
>  - Per Paolo Bonzini review:
>  - move reg_guide.txt to docs/specs/rocker.txt
>  - fix some spelling/grammer mistakes in the rocker.txt doc
>  - fix some misleading/wrong statements in rocker.txt
>  - add double 4-byte access for 64-bit registers
>  - define new ROCKER_Exxx to replace usage of errno.h return codes
>  - Add patch from David Ahern to timestamp debug output
>
> v3:
>
>  - Per Stefan Hajnoczi review:
>  - move HMP rocker cmds to "info rocker"
>  - prefix QMP rocker cmds with query-
>  - tag QMP cmds as "Since 2.3"
>  - convert structs to typedef with CamelCase naming
>  - Remove SDHCI device ID move patch...Paolo Bonzini is addressing
>SDHCI move is separate patch.
>
> v2:
>
>  - Address reg_guide.txt review comments from Eric Blake
>  - Address QMP review comments from Eric Blake
>
> v1:
>
> [This is a collaboration between myself and Jiri Pirko].
>
> This patch set adds a new ethernet switch device, called rocker.  Rocker is
> intended to emulate HW features of switch ASICs found in today's
> data-center-class switch/routers.  The original motivation in creating a new
> device is to accelerate device driver development for ethernet switches in the
> Linux kernel.  A device driver for rocker already exists in the Linux 3.18
> kernel and loads against this device.  Basic L2 switching (bridging)
> functionality is offloaded to the device.  Work continues to enable offloading
> of L3 routing functions and ACLs, as well as support for a flow-based modes,
> such as OpenVSwitch with OpenFlow.  Future support for terminating L2-over-L3
> tunnels is also planned.
>
> The core network processing functions are based on the spec of a real device:
> Broadcom's OF-DPA.  Specifically, rocker borrows OF-DPA's network processing
> pipeline comprised of flow match and action tables.  Only the OF-DPA spec was
> used in constructing rocker.  The rocker developers do not have access to the
> real OF-DPA's software source code, so this is a clean-room, ground-up
> development.
>
> Each rocker device is a PCI device with a memory-mapped register space and
> MSI-X interrupts for command and event processing, as well as CPU-bound I/O.
> Each device can support up to 62 "front-panel" ports, which present themselves
> as -netdev attachment points.  The device is programmed using OF-DPA flow and
> group tables to setup the flow pipeline.  The programming defines the
> forwarding path for packets ingressing on 'front-panel' ports.  The forwarding
> path can look at L2/L3/L4 packet header to forward the packet to its
> destination.  For the performance path, packets would ingress and egress only
> on the device, and not be passed up to the device driver (or host OS).  The
> slow path for control packets will forward packets to the CPU via the device
> driver for host OS processing.
>
> A QMP/HMP interface is added to give inside into the device's internal port
> configuration and flow/group tables.
>
> A test directory is included with some basic sanity tests to verify the device
> and driver.
>
> David Ahern (1):
>   rocker: timestamp on the debug logs helps correlate with events in
> the VM
>
> Scott Feldman (8):
>   net: add MAC address string printer
>   virtio-net: use qemu_mac_strdup_printf
>   rocker: add register programming guide
>   pci: add rocker device ID
>   pci: add network device class 'other' for network switches
>   rocker: add new rocker switch device
>   rocker: add tests
>   MAINTAINERS: add rocker
>
>  MAINTAINERS   |6 +
>  default-configs/pci.mak   |1 +
>  docs/specs/pci-ids.txt|1 +
>  docs/specs/rocker.txt | 1009 ++
>  hw/net/Makefile.objs  |4 +
>  hw/net/rocker/rocker.c| 1480 ++
>  hw/net/rocker/rocker.h|   84 ++
>  hw/net/

Re: [Qemu-devel] [PATCH v5 01/28] qapi: Document type-safety considerations

[Markus Armbruster]

Copyright note question, copying Michael.

Eric Blake  writes:

> On 03/25/2015 12:31 PM, Markus Armbruster wrote:
>> Eric Blake  writes:
>>
>>> Go into more details about the various types of valid expressions
>>> in a qapi schema, including tweaks to document fixes being done
>>> later in the current patch series.  Also fix some stale and missing
>>> documentation in the QMP specification.
>>>
>>> Signed-off-by: Eric Blake 
>>>
>>> ---
>>>
>>> I'm not sure if it is okay to assert GPLv2+ licensing without an
>>> explicit Copyright, but as I am not the original author, I don't
>>> know who to attribute any original Copyright to.  Advice?  Should
>>> I split the license addition to a separate patch?
>
> No thoughts to this question?

Missed it.

I wish we didn't need to clutter copyright and licensing boiler plate
everywhere, but I accept it's the prudent thing to do in a tree with so
many differently licensed parts.

Making GPLv2+ explicit is obviously fine, because anything without an
explicit licensing note is GPLv2+ (see ./LICENSE).  That leaves the
copyright part, as you say.

According to git-log, the file was created by Michael Roth.  git-blame
blames 219 out of 590 lines in current master on his initial commit.

$ git-blame -w master docs/qapi-code-gen.txt | cut -c 11-28 | sort | uniq -c | 
sort -nr
219 Michael Roth
124 Markus Armbruster
116 Kevin Wolf
 95 Eric Blake
 21 Wenchao Xia
 10 Lluís Vilanova
  2 Stefan Weil
  2 Stefan Hajnoczi
  1 Benoît Canet

If you want to add a copyright note, I suggest to steal one from
Michael's work elsewhere, and update it for later major contributors.

Here's my stab at it:

= How to use the QAPI code generator =

Copyright IBM Corp. 2011
Copyright (C) 2012-2015 Red Hat, Inc.

Authors:
 Michael Roth 
 Kevin Wolf 
 Markus Armbruster 
 Eric Blake 

This work is licensed under the terms of the GNU GPL, version 2 or later.
See the COPYING file in the top-level directory.

== Introduction ==

QAPI is a native C API within QEMU which provides management-level
functionality to internal and external users. For external

The Authors paragraph is informational and could be omitted without
compromising the copyright note.

[...]

Re: [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough

[Chen, Tiejun]

Michael and Gerd,

I don't see any more comments for this revision, so what's next that I 
should do?


Thanks
Tiejun

On 2015/3/19 9:01, Chen, Tiejun wrote:

On 2015/3/18 18:21, Gerd Hoffmann wrote:

On Mi, 2015-03-18 at 17:06 +0800, Tiejun Chen wrote:

Implement a pci host bridge specific to passthrough. Actually
this just inherits the standard one. And we also just expose
a minimal real host bridge pci configuration subset.



+/* Here we just expose minimal host bridge offset subset. */
+static const IGDHostInfo igd_host_bridge_infos[] = {
+{0x08, 2},  /* revision id */
+{0x2c, 2},  /* sybsystem vendor id */
+{0x2e, 2},  /* sybsystem id */
+{0x50, 2},  /* SNB: processor graphics control register */
+{0x52, 2},  /* processor graphics control register */
+{0xa4, 4},  /* SNB: graphics base of stolen memory */
+{0xa8, 4},  /* SNB: base of GTT stolen memory */
+};


Hmm, no vendor/device id here?  Will the idg guest drivers happily read


These two emulated values are already fine.

And this is a long story. At the beginning, we were initially trying to
expose more,

+case 0x00:/* vendor id */
+case 0x02:/* device id */
+case 0x08:/* revision id */
+case 0x2c:/* sybsystem vendor id */
+case 0x2e:/* sybsystem id */
+case 0x50:/* SNB: processor graphics control register */
+case 0x52:/* processor graphics control register */
+case 0xa0:/* top of memory */
+case 0xb0:/* ILK: BSM: should read from dev 2 offset 0x5c */
+case 0x58:/* SNB: PAVPC Offset */
+case 0xa4:/* SNB: graphics base of stolen memory */
+case 0xa8:/* SNB: base of GTT stolen memory */

But this brought some discussions with Paolo and Michael, and then plus
our further experiment, now as everyone expect, this is a minimal real
host bridge pci configuration subset which we need to expose.


graphics control registers from i440fx even though this chipset never
existed in a igd variant?

[ just wondering, if it works that way fine, it certainly makes things


Yes, it works fine.


   easier for the firmware which uses host bridge pci id to figure
   whenever it is running @ i440fx or q35 ].


+static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
+{
+uint32_t val = 0;
+int rc, i, num;
+int pos, len;
+
+num = ARRAY_SIZE(igd_host_bridge_infos);
+for (i = 0; i < num; i++) {
+pos = igd_host_bridge_infos[i].offset;
+len = igd_host_bridge_infos[i].len;
+rc = host_pci_config_read(pos, len, val);
+if (rc) {
+return -ENODEV;
+}
+pci_default_write_config(pci_dev, pos, val, len);
+}
+
+return 0;
+}


Nothing i440fx specific in here, just copying some host bridge pci
config space bits.  I guess we can sub-class the q35 host bridge the
same way and even reuse the init function?


This is our another discussion long time ago :)

Currently Xen don't run with q35. If I remember those discussions
correctly, something is still restricted to Windows.




+#define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE
"xen-igd-passthrough-i440FX"


One xen leftover slipped through here.


Fixed.

Thanks
Tiejun

Re: [Qemu-devel] [PATCH v5 01/28] qapi: Document type-safety considerations

[Markus Armbruster]

Eric Blake  writes:

> On 03/25/2015 12:31 PM, Markus Armbruster wrote:
>> Eric Blake  writes:
>> 
>>> Go into more details about the various types of valid expressions
>>> in a qapi schema, including tweaks to document fixes being done
>>> later in the current patch series.  Also fix some stale and missing
>>> documentation in the QMP specification.
>>>
>>> Signed-off-by: Eric Blake 
>>>
>>> ---
>>>
>>> I'm not sure if it is okay to assert GPLv2+ licensing without an
>>> explicit Copyright, but as I am not the original author, I don't
>>> know who to attribute any original Copyright to.  Advice?  Should
>>> I split the license addition to a separate patch?
>
> No thoughts to this question?

Separate message.

>>>
>>> +There are six top-level expressions recognized by the parser:
>>> +'include', 'command', 'type', 'enum', 'union', and 'event'.  There are
>>> +several built-in types, such as 'int' and 'str'; additionally, the
>>> +top-level expressions can define complex types, enumeration types, and
>>> +several flavors of union types.  The 'command' expression can refer to
>>> +existing types by name, or list an anonymous type as a dictionary.
>> 
>> 'event' can do that, too.
>
> Yep. I can fix that on top if no respin is needed.
>
>
>>> +Types, commands, and events share a common namespace.  Therefore,
>>> +generally speaking, type definitions should always use CamelCase for
>>> +user-defined type names, while built-in types are lowercase. Type
>>> +definitions should not end in 'Kind', as this namespace is used for
>>> +creating implicit C enums for visiting union types.  Command names,
>>> +and field names within a type, should be all lower case with words
>>> +separated by a hyphen.  However, some existing older commands and
>>> +complex types use underscore; when extending such expressions,
>>> +consistency is preferred over blindly avoiding underscore.  Event
>>> +names should be ALL_CAPS with words separated by underscore.  The
>>> +special string '**' appears for some commands that manually perform
>>> +their own type checking rather than relying on the type-safe code
>>> +produced by the qapi code generators.
>> 
>> The generator generates C identifiers pretty thoughtlessly, and is
>> therefore prone to generate clashes.  Fixable, but we've got bigger fish
>> to fry, and this series is hefty enough as it is.  Documenting the mess
>> instead makes sense.
>
> And I actually DO tighten up the parser to flag some of these problems
> later in the series.
>
>
>>>  The QAPI schema definitions can be modularized using the 'include'
>>> directive:
>>>
>>>   { 'include': 'path/to/file.json'}
>> 
>> If you need to respin for some reason, insert a space before the closing
>> brace.
>
> Sure; added to my on-top-or-respin pile.
>
>
>>>
>>> -A complex type is a dictionary containing a single key whose value is a
>>> -dictionary.  This corresponds to a struct in C or an Object in JSON.  An
>>> -example of a complex type is:
>>> +Usage: { 'type': 'str', 'data': 'dict', '*base': 'complex-type-name' }
>> 
>> Here, the reader needs to get that 'type' and 'data' and 'base' are
>> literals, but 'str', 'dict' and 'complex-type-name' are placeholders.  I
>> like setting apart placeholders with typograhic conventions.  If we want
>> to do that, let's do it on top.
>
> I was trying to borrow from qapi syntax, in that the 'data' dictionary
> uses '*name':'type' for a literal "name" coupled with a
> value-of-that-type pair in the dictionary sent over the wire for
> "arguments".  As for a typographical convention, would either of these
> be any easier to read?
>
> Usage: { 'type': 'STR', 'data': 'DICT', '*base': 'COMPLEX-TYPE-NAME' }
>
> or
>
> Usage: { 'type': string, 'data': dict [, 'base': complex-type-name] }

Or

  Usage: { 'type': STRING, 'data': DICT [, 'base': COMPLEX-TYPE-NAME] }

Makes the placeholders stand out.  Unambiguous as long as we don't use
ALL-CAPS for anything else outside quotes.

Using QAPI's *-prefix instead of EBNF's brackets

  Usage: { 'type': STRING, 'data': DICT , '*base': COMPLEX-TYPE-NAME }

would also work, but I feel the *-prefix needs to be explained earlier
then.

>> 
>> '*base' is interesting.  It's a tacit use of the schema language's
>> "'*name' means member 'name' is optional" rule on meta-schema-level.  We
>> can take care of that when/if we clarify placeholders.
>> 
>>> +
>>> +A complex type is a dictionary containing a single 'data' key whose
>>> +value is a dictionary.  This corresponds to a struct in C or an Object
>>> +in JSON. Each value of the 'data' dictionary must be the name of a
>>> +complex, enum, union, or built-in type,
>> 
>> Are there any others?  If not, we can simplify to
>> 
>> Each value of the 'data' dictionary must be a type name
>
> Works for me, especially since the addition of 'alternate' later in this
> series is also a type that works in this context.
>
>> 
>>>  or a one-element array
>>> +containing a type name.  A

Re: [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough

[Michael S. Tsirkin]

On Thu, Mar 26, 2015 at 03:57:18PM +0800, Chen, Tiejun wrote:
> Michael and Gerd,
> 
> I don't see any more comments for this revision, so what's next that I
> should do?
> 
> Thanks
> Tiejun
> 
> On 2015/3/19 9:01, Chen, Tiejun wrote:

It's only been a week, and we are busy finalizing 2.3.  So please give
people time to review.

-- 
MST

Re: [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough

[Chen, Tiejun]

On 2015/3/26 16:15, Michael S. Tsirkin wrote:

On Thu, Mar 26, 2015 at 03:57:18PM +0800, Chen, Tiejun wrote:

Michael and Gerd,

I don't see any more comments for this revision, so what's next that I
should do?

Thanks
Tiejun

On 2015/3/19 9:01, Chen, Tiejun wrote:


It's only been a week, and we are busy finalizing 2.3.  So please give


Oh, understood.

Thanks
Tiejun

Re: [Qemu-devel] [Qemu-block] [RFC PATCH COLO v2 01/13] docs: block replication's description

[Wen Congyang]

On 03/25/2015 11:38 PM, Eric Blake wrote:
> On 03/25/2015 03:36 AM, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang 
>> Signed-off-by: Paolo Bonzini 
>> Signed-off-by: Yang Hongyang 
>> Signed-off-by: zhanghailiang 
>> Signed-off-by: Gonglei 
>> ---
>>  docs/block-replication.txt | 147 
>> +
>>  1 file changed, 147 insertions(+)
>>  create mode 100644 docs/block-replication.txt
>>
> 
> Grammar review only (I'll leave the technical review to others)
> 
>> diff --git a/docs/block-replication.txt b/docs/block-replication.txt
>> new file mode 100644
>> index 000..874ed8e
>> --- /dev/null
>> +++ b/docs/block-replication.txt
>> @@ -0,0 +1,147 @@
>> +Block replication
>> +
>> +Copyright Fujitsu, Corp. 2015
>> +Copyright (c) 2015 Intel Corporation
>> +Copyright (c) 2015 HUAWEI TECHNOLOGIES CO.,LTD.
> 
> Space after comma in English writing.

Yes, but I am not sure I can change it. HUAWEI always use this way.
You can find it in bootdevice.c

Thanks
Wen Congyang

> 
>> +
>> +This work is licensed under the terms of the GNU GPL, version 2 or later.
>> +See the COPYING file in the top-level directory.
>> +
>> +The block replication is used for continuous checkpoints. It is designed
> 
> Sounds better as either of:
> The block replication feature is...
> Block replication is...
> 
>> +for COLO that Secondary VM is running. It can also be applied for FT/HA
> 
> Please define COLO and FT/HA on first use (okay to abbreviate elsewhere
> in the document, but the first use should not assume the acronym is
> well-known)
> 
> s/for COLO that/for COLO (COurse-grain LOck-stepping), where the/
> 
>> +scene that Secondary VM is not running.
> 
> s/for FT/HA scene that/for the FT/HA (Fault-tolerance/High Assurance)
> scenario, where the/
> 
>> +
>> +This document gives an overview of block replication's design.
>> +
>> +== Background ==
>> +High availability solutions such as micro checkpoint and COLO will do
>> +consecutive checkpoint. The VM state of Primary VM and Secondary VM is
> 
> s/checkpoint/checkpoints/
> 
>> +identical right after a VM checkpoint, but becomes different as the VM
> ...
>> +
>> +4) The hidden-disk is created automatically. It buffers the original content
>> +that is modified by the primary VM. It should also be an empty disk, and
>> +the dirver supports bdrv_make_empty().
> 
> s/dirver/driver/
> 
>> +
>> +== New block driver interface ==
>> +We add three block driver interfaces to control block replication:
>> +a. bdrv_start_replication()
>> +   Start block replication, called in migration/checkpoint thread.
>> +   We must call bdrv_start_replication() in secondary QEMU before
>> +   calling bdrv_start_replication() in primary QEMU.
>> +b. bdrv_do_checkpoint()
>> +   This interface is called after all VM state is transfered to
> 
> s/transfered/transferred/
> 
>> +   Secondary QEMU. The Disk buffer will be dropped in this interface.
>> +   The caller must hold the I/O mutex lock if it is in migration/checkpoint
>> +   thread.
>> +c. bdrv_stop_replication()
>> +   It is called when failover. We will flush the Disk buffer into
> 
> s/when/on/
> 
>> +   Secondary Disk and stop block replication. The vm should be stopped
>> +   before calling it. The caller must hold the I/O mutex lock if it is
>> +   in migration/checkpoint thread.
>> +
>> +== Usage ==
>> +Primary:
>> +  -drive if=xxx,driver=quorum,read-pattern=fifo,\
>> + children.0.file.filename=1.raw,\
>> + children.0.driver=raw,\
>> + children.1.file.driver=nbd+colo,\
>> + children.1.file.host=xxx,\
>> + children.1.file.port=xxx,\
>> + children.1.file.export=xxx,\
>> + children.1.driver=raw,\
>> + children.1.ignore-errors=on
> 
> This command line looks like multiple arguments because of the leading
> whitespace on succeeding lines.  I don't know if there is any better way
> to format it, though, to make it obvious that it is all a single
> argument to -drive.
> 
>> +  Note:
>> +  1. NBD Client should not be the first child of quorum.
>> +  2. There should be only one NBD Client.
>> +  3. host is the secondary physical machine's hostname or IP
>> +  4. Each disk must have its own export name.
> 
> Maybe a note 5 to call out the formatting aspect of the command line?
> 
>> +
>> +Secondary:
>> +  -drive if=none,driver=raw,file=1.raw,id=nbd_target1 \
>> +  -drive if=xxx,driver=qcow2+colo,file=active_disk.qcow2,export=xxx,\
>> + backing_reference.drive_id=nbd_target1,\
>> + backing_reference.hidden-disk.file.filename=hidden_disk.qcow2,\
>> + backing_reference.hidden-disk.driver=qcow2,\
>> + backing_reference.hidden-disk.allow-write-backing-file=on
>> +  Then run qmp command:
>> +nbd_server_start host:port
>> +  Note:
>> +  1. The export name for the same disk must be the same in primary
>> + and secondary QEMU command line
>> +  2. The qmp command nbd_server_start mus

Re: [Qemu-devel] [PATCH v5 01/28] qapi: Document type-safety considerations

[Markus Armbruster]

Eric Blake  writes:

> On 03/25/2015 02:11 PM, Eric Blake wrote:
>
>>> The QObject types are QTYPE_NONE, QTYPE_QINT, QTYPE_QSTRING,
>>> QTYPE_QDICT, QTYPE_QLIST, QTYPE_QFLOAT, QTYPE_QBOOL, QTYPE_QERROR.
>>>
>>> The connections JSON string - QTYPE_QSTRING, JSON object - QTYPE_QDICT,
>>> JSON array - QTYPE_QLIST and JSON boolean - QTYPE_QBOOL are obvious
>>> enough.
>>>
>>> If I remember correctly, our JSON parser chokes on the JSON keyword
>>> null.
>
> Yep, that is sadly still the case [1]:
>
> $ qemu-kvm -qmp stdio -nodefaults
> {"QMP": {"version": {"qemu": {"micro": 90, "minor": 2, "major": 2},
> "package": " (qemu-2.3.0-0.1.rc0.fc21)"}, "capabilities": []}}
> {"execute":"qmp_capabilities","id":null}
> {"error": {"class": "GenericError", "desc": "Invalid JSON syntax"}}
>
> Patch 17 adds support for it to qapi schemas, but not to our JSON
> parser.  I guess that's yet another task to be solved before we turn on
> argument defaults and introspection (as returning 'null' would be a
> logical solution for introspecting the default value of an optional
> string argument.  On the other hand, we could argue that using 'null' in
> output of introspection still doesn't require the parser to accept
> 'null' on input;  the output direction of introspection is different
> than the input direction of parsing a 'null' in user commands).

Yes.

Our parser choking on null is simply a bug that happens not to matter
with our current usage.

>>>
>>> That leaves just JSON numbers - QTYPE_QINT or QTYPE_QFLOAT.  Can an
>>> anonymous union have a separate case for each of the two?
>
> Yes.  Don't know why we'd want it, but the code currently handles it.
> That is, this compiles just fine:
>
> { 'alternate': 'Foo', 'data': { 'a': 'int', 'b': 'number' } }
> { 'command': 'bar', 'data': { 'value': 'Foo' } }
>
> allowing:
>  {"execute":"bar", "arguments":{ "value":1 } }
>  {"execute":"bar", "arguments":{ "value":1.0 } }
> as operating on uint64_t vs. double (in practice, since '1' is also
> valid input as a number, a double is sufficient without needing the
> alternative of a uint64_t, unless you are simultaneously worrying about
> precise integral values larger than 2**53 that lose data when converted
> to double, while still allowing for inputs larger than 2**63 via double)

JSON leaves defining limits on range and precision of numbers to
implementations.

Many implementations limit them to IEEE double precision.  Integers that
double can't represent exactly get rounded.  Whether you write an
integer as string of digits or the same sting followed by a redundant
exponent or fraction such as .0 doesn't matter.

We limit differently.  If the JSON number has neither a fraction nor an
exponent part, and it's representable as int64_t, then we parse it as
that.  Else we parse it as double.  See the big comment in
parse_literal().  Yes, the code there fails error handling 101.

Buys us exact representation of more integers at the price of subtly
different interpretation of JSON numbers with neither fraction nor
exponent part.

A few examples:

JSONQTYPE_value  exact?
1   QINT  1  yes
1.0 QFLOAT1.0yes
10001.0 QFLOAT1.0no
10001   QINT  10001  yes
9223372036854775807 QINT  9223372036854775807yes
9223372036854775808 QFLOAT9223372036854775808.0  yes
9223372036854775809 QFLOAT9223372036854775808.0  no
   -9223372036854775808 QINT -9223372036854775808yes

Yes, that means the integers in [2^63,2^64-1] are parsed as double.
uint64 in the schema is a pious lie.

  The format of a success response is:

 -{ "return": json-object, "id": json-value }
 +{ "return": json-entity, "id": json-value }
>>>
>>> Unlike the other json-FOOs we use, "entity" isn't defined in RFC4627.
>>> "value" is, and we already use json-value.  What's the difference
>>> between the two?
>> 
>> Umm, when I wrote that, I was thinking "id":json-value meant integer, so
>> I wanted something that was not an integer.  But sure enough, json-value
>> is precisely the term I wanted to use:
>
> Well, given above at [1] that 'null' is a valid json-value but NOT
> accepted by our parser, I guess we are not quite accurate here.

I regard the parser choking on null as a bug.  Bugs are implementation
detail ;)

Re: [Qemu-devel] E5-2620v2 - emulation stop error

[Andrey Korolyov]

On Thu, Mar 26, 2015 at 5:47 AM, Bandan Das  wrote:
> Hi Andrey,
>
> Andrey Korolyov  writes:
>
>> On Mon, Mar 16, 2015 at 10:17 PM, Andrey Korolyov  wrote:
>>> For now, it looks like bug have a mixed Murphy-Heisenberg nature, as
>>> it appearance is very rare (compared to the number of actual launches)
>>> and most probably bounded to the physical characteristics of my
>>> production nodes. As soon as I reach any reproducible path for a
>>> regular workstation environment, I`ll let everyone know. Also I am
>>> starting to think that issue can belong to the particular motherboard
>>> firmware revision, despite fact that the CPU microcode is the same
>>> everywhere.
>
> I will take the risk and say this - "could it be a processor bug ?" :)
>
>>
>> Hello everyone, I`ve managed to reproduce this issue
>> *deterministically* with latest seabios with smp fix and 3.18.3. The
>> error occuring just *once* per vm until hypervisor reboots, at least
>> in my setup, this is definitely crazy...
>>
>> - launch two VMs (Centos 7 in my case),
>> - wait a little while they are booting,
>> - attach serial console (I am using virsh list for this exact purpose),
>> - issue acpi reboot or reset, does not matter,
>> - VM always hangs at boot, most times with sgabios initialization
>> string printed out [1], but sometimes it hangs a bit later [2],
>> - no matter how many times I try to relaunch the QEMU afterwards, the
>> issue does not appear on VM which experienced problem once;
>> - trace and sample args can be seen in [3] and [4] respectively.
>
> My system is a Dell R720 dual socket which has 2620v2s. I tried your
> setup but couldn't reproduce (my qemu cmdline isn't exactly the same
> as yours), although, if you could simplify your command line a bit,
> I can try again.
>
> Bandan
>
>> 1)
>> Google, Inc.
>> Serial Graphics Adapter 06/11/14
>> SGABIOS $Id: sgabios.S 8 2010-04-22 00:03:40Z nlaredo $
>> (pbuilder@zorak) Wed Jun 11 05:57:34 UTC 2014
>> Term: 211x62
>> 4 0
>>
>> 2)
>> Google, Inc.
>> Serial Graphics Adapter 06/11/14
>> SGABIOS $Id: sgabios.S 8 2010-04-22 00:03:40Z nlaredo $
>> (pbuilder@zorak) Wed Jun 11 05:57:34 UTC 2014
>> Term: 211x62
>> 4 0
>> [...empty screen...]
>> SeaBIOS (version 1.8.1-20150325_230423-testnode)
>> Machine UUID 3c78721f-7317-4f85-bcbe-f5ad46d293a1
>>
>>
>> iPXE (http://ipxe.org) 00:02.0 C100 PCI2.10 PnP PMM+3FF95BA0+3FEF5BA0 C10
>>
>> 3)
>>
>> KVM internal error. Suberror: 2
>> extra data[0]: 80ef
>> extra data[1]: 8b0d
>> EAX= EBX= ECX= EDX=
>> ESI= EDI= EBP= ESP=6d2c
>> EIP=d331 EFL=00010202 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0
>> ES =   9300
>> CS =f000 000f  9b00
>> SS =   9300
>> DS =   9300
>> FS =   9300
>> GS =   9300
>> LDT=   8200
>> TR =   8b00
>> GDT= 000f6cb0 0037
>> IDT=  03ff
>> CR0=0010 CR2= CR3= CR4=
>> DR0= DR1= DR2=
>> DR3=
>> DR6=0ff0 DR7=0400
>> EFER=
>> Code=66 c3 cd 02 cb cd 10 cb cd 13 cb cd 15 cb cd 16 cb cd 18 cb 
>> 19 cb cd 1c cb cd 4a cb fa fc 66 ba 47 d3 0f 00 e9 ad fe f3 90 f0 0f
>> ba 2d d4 fe fb 3f
>>
>> 4)
>> /usr/bin/qemu-system-x86_64 -name centos71 -S -machine
>> pc-i440fx-2.1,accel=kvm,usb=off -cpu SandyBridge,+kvm_pv_eoi -bios
>> /usr/share/seabios/bios.bin -m 1024 -realtime mlock=off -smp
>> 12,sockets=1,cores=12,threads=12 -uuid
>> 3c78721f-7317-4f85-bcbe-f5ad46d293a1 -nographic -no-user-config
>> -nodefaults -device sga -chardev
>> socket,id=charmonitor,path=/var/lib/libvirt/qemu/centos71.monitor,server,nowait
>> -mon chardev=charmonitor,id=monitor,mode=control -rtc
>> base=utc,driftfix=slew -global kvm-pit.lost_tick_policy=discard
>> -no-hpet -no-shutdown -global PIIX4_PM.disable_s3=1 -global
>> PIIX4_PM.disable_s4=1 -boot strict=on -device
>> nec-usb-xhci,id=usb,bus=pci.0,addr=0x3 -device
>> virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x4 -drive
>> file=rbd:dev-rack2/centos7-1.raw:id=qemukvm:key=XX:auth_supported=cephx\;none:mon_host=10.6.0.1\:6789\;10.6.0.3\:6789\;10.6.0.4\:6789,if=none,id=drive-virtio-disk0,format=raw,cache=writeback,aio=native
>> -device 
>> virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
>> -chardev pty,id=charserial0 -device
>> isa-serial,chardev=charserial0,id=serial0 -chardev
>> socket,id=charchannel0,path=/var/lib/libvirt/qemu/centos71.sock,server,nowait
>> -device 
>> virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.1
>> -msg timestamp=on

Hehe, 2.2 works just perfectly but 2.1 isn`t. I`ll bisect the issue in
a next couple of days and post the right commit (but as can remember
none of

Re: [Qemu-devel] Support for NetLogic XLP Processors

[Leon Alrae]

Hi Duarte,

On 25/03/2015 23:54, Duarte Silva wrote:
> On Wednesday 25 March 2015 17:33:59 Leon Alrae wrote:
>> On 25/03/2015 15:38, Duarte Silva wrote:
>>> On Wednesday 25 March 2015 14:54:41 Leon Alrae wrote:
 On 25/03/2015 14:44, Leon Alrae wrote:
> Hi Duarte,
>
> On 25/03/2015 14:20, Duarte Silva wrote:
>> On Wednesday 25 March 2015 13:13:14 James Hogan wrote:
>>> Hi Duarte,
>>>
>>> On 22/03/15 11:13, Duarte Silva wrote:
 Hi guys,

 I have been struggling to get some binaries compiled for NetLogic XLP
 processor to run under QEMU. I have tried a bunch of things (most
 going
 back and forth) and always get the following error message:

 qemu: uncaught target signal 4 (Illegal instruction) - core dumped
 Illegal instruction

 I tried to debug it using GDB but to no avail. Does anybody have
 ideas?
 I'm
 running QEMU 2.2.1.
>>>
>>> It sounds like the program had an instruction that QEMU doesn't
>>> recognise, or doesn't think should be allowed on the current CPU which
>>> you've set with -cpu. You might be able to find out what that
>>>
>>> instruction is by putting this on your qemu command line:
>>>  -singlestep -d in_asm
>>
>> Hi James,
>>
>> thanks for the help :) I have tried with all the CPU's available. None
>> of
>> them worked, so I just leave it as undefined. It seems the offending
>> instruction is "udi4".
>>
>> (...)
>> IN:
>> 0x765d1fa4:  udi4   a0,v0,zero,0x0
>
> According to this line you are trying to use MIPS32 CPU whereas I
> presume you would like MIPS64R2? Please try 5KEf CPU for example which
> is available in qemu-mips64 and qemu-mips64el QEMU binaries for big and
> little endian respectively.

 I just noticed the QEMU version you are using and it doesn't contain
 5KEf and 5KEc CPUs. Please try MIPS64R2-generic.

 Leon
>>>
>>> Hi Leon,
>>>
>>> have a look at the "binary-info.txt" file in the first e-Mail. It does use
>>> the ELF magic for 32 bits ELF, not the 64 bits, that's why I get the
>>> following:
>>>
>>> # chroot rootfs/ /usr/local/bin/qemu-mips64 -cpu MIPS64R2-generic /bin/sh
>>> /bin/sh: Invalid ELF image for this architecture
>>>
>>> Is there a way to force the execution of the binary even if the flag
>>> doesn't match?
>>>
>>> Also, if you have a look at the flags you get: noreorder, cpic, 32bitmode,
>>> unknown CPU, o32, mips64r2. So, is it 64 bits or 32 bits ELF file?
>>
>> I see, this mips64r2 binary has o32 ABI. It indeed would work in
>> qemu-mips provided there are no mips64r2-specific instructions. I think
>> I jumped a bit too quickly to the conclusion.
>>
>> QEMU's mips/disas doesn't help much in this case as it just indicates
>> User Defined Instruction. Presumably this instruction is specific to
>> this processor and is missing in QEMU. Are you able to get disassembly
>> of your program and look up what is under 0x765d1fa4 address which
>> caused the illegal instruction?
> 
> Hi Leon,
> 
> using IDA with a remote debug session to QEMU  I got the following 
> disassembly 
> (kept surrounding instructions to give some context). To IDA, this custom 
> instruction is also unknown.
> 
> MEMORY:765D1F90 sw  $v1, 4($v0)
> MEMORY:765D1F94 addu$a0, $a1
> MEMORY:765D1F98 sw  $a0, 0($v0)
> MEMORY:765D1F9C
> MEMORY:765D1F9C loc_765D1F9C:
> MEMORY:765D1F9C addiu   $a0, $s1, 0x51B0
> MEMORY:765D1FA0 move$v0, $zero
> MEMORY:765D1FA0  # ---
> MEMORY:765D1FA4 .byte 0x70  # p
> MEMORY:765D1FA5 .byte 0x82  # é
> MEMORY:765D1FA6 .byte0
> MEMORY:765D1FA7 .byte 0x14
> MEMORY:765D1FA8  # ---
> MEMORY:765D1FA8 slti$v0, 2
> MEMORY:765D1FAC beqz$v0, loc_765D204C
> MEMORY:765D1FB0 nop
> MEMORY:765D1FB4 lw  $ra, 0x24($sp)
> MEMORY:765D1FB8
> MEMORY:765D1FB8 loc_765D1FB8:
> MEMORY:765D1FB8 move$v0, $s0
> MEMORY:765D1FBC lw  $s1, 0x20($sp)
> MEMORY:765D1FC0 lw  $s0, 0x1C($sp)

According to binutils this is SWAPW which belongs to XLR:
{"swapw",  "t,b",  0x7014, 0xfc00,
MOD_1|RD_2|LM|SM,   0,  XLR,0,  0 },

I'm afraid you won't be able to run binaries built for NetLogic XLP
until someone implements these instructions in QEMU.

Regards,
Leon

Re: [Qemu-devel] Support for NetLogic XLP Processors

[James Hogan]

On 26/03/15 09:29, Leon Alrae wrote:
> Hi Duarte,
> 
> On 25/03/2015 23:54, Duarte Silva wrote:
>> On Wednesday 25 March 2015 17:33:59 Leon Alrae wrote:
>>> On 25/03/2015 15:38, Duarte Silva wrote:
 On Wednesday 25 March 2015 14:54:41 Leon Alrae wrote:
> On 25/03/2015 14:44, Leon Alrae wrote:
>> Hi Duarte,
>>
>> On 25/03/2015 14:20, Duarte Silva wrote:
>>> On Wednesday 25 March 2015 13:13:14 James Hogan wrote:
 Hi Duarte,

 On 22/03/15 11:13, Duarte Silva wrote:
> Hi guys,
>
> I have been struggling to get some binaries compiled for NetLogic XLP
> processor to run under QEMU. I have tried a bunch of things (most
> going
> back and forth) and always get the following error message:
>
> qemu: uncaught target signal 4 (Illegal instruction) - core dumped
> Illegal instruction
>
> I tried to debug it using GDB but to no avail. Does anybody have
> ideas?
> I'm
> running QEMU 2.2.1.

 It sounds like the program had an instruction that QEMU doesn't
 recognise, or doesn't think should be allowed on the current CPU which
 you've set with -cpu. You might be able to find out what that

 instruction is by putting this on your qemu command line:
  -singlestep -d in_asm
>>>
>>> Hi James,
>>>
>>> thanks for the help :) I have tried with all the CPU's available. None
>>> of
>>> them worked, so I just leave it as undefined. It seems the offending
>>> instruction is "udi4".
>>>
>>> (...)
>>> IN:
>>> 0x765d1fa4:  udi4   a0,v0,zero,0x0
>>
>> According to this line you are trying to use MIPS32 CPU whereas I
>> presume you would like MIPS64R2? Please try 5KEf CPU for example which
>> is available in qemu-mips64 and qemu-mips64el QEMU binaries for big and
>> little endian respectively.
>
> I just noticed the QEMU version you are using and it doesn't contain
> 5KEf and 5KEc CPUs. Please try MIPS64R2-generic.
>
> Leon

 Hi Leon,

 have a look at the "binary-info.txt" file in the first e-Mail. It does use
 the ELF magic for 32 bits ELF, not the 64 bits, that's why I get the
 following:

 # chroot rootfs/ /usr/local/bin/qemu-mips64 -cpu MIPS64R2-generic /bin/sh
 /bin/sh: Invalid ELF image for this architecture

 Is there a way to force the execution of the binary even if the flag
 doesn't match?

 Also, if you have a look at the flags you get: noreorder, cpic, 32bitmode,
 unknown CPU, o32, mips64r2. So, is it 64 bits or 32 bits ELF file?
>>>
>>> I see, this mips64r2 binary has o32 ABI. It indeed would work in
>>> qemu-mips provided there are no mips64r2-specific instructions. I think
>>> I jumped a bit too quickly to the conclusion.
>>>
>>> QEMU's mips/disas doesn't help much in this case as it just indicates
>>> User Defined Instruction. Presumably this instruction is specific to
>>> this processor and is missing in QEMU. Are you able to get disassembly
>>> of your program and look up what is under 0x765d1fa4 address which
>>> caused the illegal instruction?
>>
>> Hi Leon,
>>
>> using IDA with a remote debug session to QEMU  I got the following 
>> disassembly 
>> (kept surrounding instructions to give some context). To IDA, this custom 
>> instruction is also unknown.
>>
>> MEMORY:765D1F90 sw  $v1, 4($v0)
>> MEMORY:765D1F94 addu$a0, $a1
>> MEMORY:765D1F98 sw  $a0, 0($v0)
>> MEMORY:765D1F9C
>> MEMORY:765D1F9C loc_765D1F9C:
>> MEMORY:765D1F9C addiu   $a0, $s1, 0x51B0
>> MEMORY:765D1FA0 move$v0, $zero
>> MEMORY:765D1FA0  # ---
>> MEMORY:765D1FA4 .byte 0x70  # p
>> MEMORY:765D1FA5 .byte 0x82  # é
>> MEMORY:765D1FA6 .byte0
>> MEMORY:765D1FA7 .byte 0x14
>> MEMORY:765D1FA8  # ---
>> MEMORY:765D1FA8 slti$v0, 2
>> MEMORY:765D1FAC beqz$v0, loc_765D204C
>> MEMORY:765D1FB0 nop
>> MEMORY:765D1FB4 lw  $ra, 0x24($sp)
>> MEMORY:765D1FB8
>> MEMORY:765D1FB8 loc_765D1FB8:
>> MEMORY:765D1FB8 move$v0, $s0
>> MEMORY:765D1FBC lw  $s1, 0x20($sp)
>> MEMORY:765D1FC0 lw  $s0, 0x1C($sp)
> 
> According to binutils this is SWAPW which belongs to XLR:
> {"swapw",  "t,b",  0x7014, 0xfc00,
> MOD_1|RD_2|LM|SM,   0,  XLR,0,  0 },
> 
> I'm afraid you won't be able to run binaries built for NetLogic XLP
> until someone implements these instructions in QEMU.

Thanks Leon, you just beat me to it with that :-)

For reference, you can disassemble xlp specific code with objdump using
"-m mips:xlp":

$ cat test.S
.text
.word 0x70820014
$ mips-linux-gnu-gcc -o test.o -c test.S
$ mips-linux-gnu-objdump -d test.o -m mips:xlp

test.o: file format elf32-tradbigmips


Disassembly of section .text:

 <.text>:
   0:   70820014swapw   v0,a0
...

Cheers
James

Re: [Qemu-devel] [Qemu-discuss] error - Guest has not initialized the display yet.

[Peter Maydell]

On 25 March 2015 at 23:52, Programmingkid  wrote:
> I'm seeing this error also.

It's still not an error.

> When I try to boot Mac OS 10.2 in
> qemu-system-ppc, I just see a black window. When I switch from the monitor
> to the vga window, then I see "Guest has not initialized the display (yet)".
> It doesn't go away. OpenBIOS can't even be accessed anymore. I think I
> started seeing this problem around the 18th of March. I'm using Mac OS 10.6
> as my host.

Your symptoms are entirely different, and you have a different bug.

-- PMM

Re: [Qemu-devel] [PATCH v5 02/28] qapi: Fix generation of 'size' builtin type

[Markus Armbruster]

Eric Blake  writes:

> We were missing the 'size' builtin type (which means that QAPI using
> [ 'size' ] would fail to compile).

I suspect it chokes on 'size' in anonymous unions, too.

> Futhermore, there was some
> redundancy between builtin_types[] and builtin_type_qtypes{}.
>
> Signed-off-by: Eric Blake 

Split off "[PATCH v4 10/19] qapi: Better error messages for duplicated
expressions".  Good.

> ---
>  scripts/qapi-types.py   | 10 +-
>  scripts/qapi-visit.py   |  6 +++---
>  scripts/qapi.py |  9 ++---
>  tests/qapi-schema/qapi-schema-test.json |  3 ++-
>  tests/qapi-schema/qapi-schema-test.out  |  2 +-
>  5 files changed, 13 insertions(+), 17 deletions(-)
>
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index db87218..e400b03 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -182,8 +182,8 @@ const int %(name)s_qtypes[QTYPE_MAX] = {
>
>  for key in members:
>  qapi_type = members[key]
> -if builtin_type_qtypes.has_key(qapi_type):
> -qtype = builtin_type_qtypes[qapi_type]
> +if builtin_types.has_key(qapi_type):
> +qtype = builtin_types[qapi_type]
>  elif find_struct(qapi_type):
>  qtype = "QTYPE_QDICT"
>  elif find_union(qapi_type):

Took me a bit of digging to make the connection to "there was some
redundancy between builtin_types[] and builtin_type_qtypes{}", and to
the patch to qapi.py.

You could split this patch up: one part for folding _qtypes into _types,
and another part for fixing 'size'.  Not worth a respin by itself, so:

Reviewed-by: Markus Armbruster 

[...]

Re: [Qemu-devel] [PATCH v5 03/28] qapi: Require ASCII in schema

[Markus Armbruster]

Eric Blake  writes:

> On 03/24/2015 02:03 PM, Eric Blake wrote:
>> Python 2 and Python 3 have a wild history of whether strings
>> default to ascii or unicode, where Python 3 requires checking
>> instanceof(foo, basestr) to cover all strings, but where that
>> code is not portable to Python 2.  It's simpler to just state
>> that we don't care about Unicode strings, and to just always
>> use the simpler instanceof(foo, str) everywhere.
>
> And for all my proof-reading, I already have a commit message change:
>
> s/instanceof/isinstance/
>
> (you can tell I'm not that proficient in python...)
>
>> 
>> I'm no python expert, so I'm basing it on this conversation:
>> https://lists.gnu.org/archive/html/qemu-devel/2014-09/msg05278.html
>> 
>> Signed-off-by: Eric Blake 
>> ---
>>  scripts/qapi.py | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>
>> @@ -354,7 +354,7 @@ def parse_schema(input_file):
>>  return exprs
>> 
>>  def parse_args(typeinfo):
>> -if isinstance(typeinfo, basestring):
>> +if isinstance(typeinfo, str):
>
> at least the code is right.

Yup.  With the spelling fix:

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] Support for NetLogic XLP Processors

[Duarte Silva]

On Thursday 26 March 2015 09:34:14 James Hogan wrote:
> On 26/03/15 09:29, Leon Alrae wrote:
> > Hi Duarte,
> > 
> > On 25/03/2015 23:54, Duarte Silva wrote:
> >> On Wednesday 25 March 2015 17:33:59 Leon Alrae wrote:
> >>> On 25/03/2015 15:38, Duarte Silva wrote:
>  On Wednesday 25 March 2015 14:54:41 Leon Alrae wrote:
> > On 25/03/2015 14:44, Leon Alrae wrote:
> >> Hi Duarte,
> >> 
> >> On 25/03/2015 14:20, Duarte Silva wrote:
> >>> On Wednesday 25 March 2015 13:13:14 James Hogan wrote:
>  Hi Duarte,
>  
>  On 22/03/15 11:13, Duarte Silva wrote:
> > Hi guys,
> > 
> > I have been struggling to get some binaries compiled for NetLogic
> > XLP
> > processor to run under QEMU. I have tried a bunch of things (most
> > going
> > back and forth) and always get the following error message:
> > 
> > qemu: uncaught target signal 4 (Illegal instruction) - core dumped
> > Illegal instruction
> > 
> > I tried to debug it using GDB but to no avail. Does anybody have
> > ideas?
> > I'm
> > running QEMU 2.2.1.
>  
>  It sounds like the program had an instruction that QEMU doesn't
>  recognise, or doesn't think should be allowed on the current CPU
>  which
>  you've set with -cpu. You might be able to find out what that
>  
>  instruction is by putting this on your qemu command line:
>   -singlestep -d in_asm
> >>> 
> >>> Hi James,
> >>> 
> >>> thanks for the help :) I have tried with all the CPU's available.
> >>> None
> >>> of
> >>> them worked, so I just leave it as undefined. It seems the offending
> >>> instruction is "udi4".
> >>> 
> >>> (...)
> >>> IN:
> >>> 0x765d1fa4:  udi4   a0,v0,zero,0x0
> >> 
> >> According to this line you are trying to use MIPS32 CPU whereas I
> >> presume you would like MIPS64R2? Please try 5KEf CPU for example
> >> which
> >> is available in qemu-mips64 and qemu-mips64el QEMU binaries for big
> >> and
> >> little endian respectively.
> > 
> > I just noticed the QEMU version you are using and it doesn't contain
> > 5KEf and 5KEc CPUs. Please try MIPS64R2-generic.
> > 
> > Leon
>  
>  Hi Leon,
>  
>  have a look at the "binary-info.txt" file in the first e-Mail. It does
>  use
>  the ELF magic for 32 bits ELF, not the 64 bits, that's why I get the
>  following:
>  
>  # chroot rootfs/ /usr/local/bin/qemu-mips64 -cpu MIPS64R2-generic
>  /bin/sh
>  /bin/sh: Invalid ELF image for this architecture
>  
>  Is there a way to force the execution of the binary even if the flag
>  doesn't match?
>  
>  Also, if you have a look at the flags you get: noreorder, cpic,
>  32bitmode,
>  unknown CPU, o32, mips64r2. So, is it 64 bits or 32 bits ELF file?
> >>> 
> >>> I see, this mips64r2 binary has o32 ABI. It indeed would work in
> >>> qemu-mips provided there are no mips64r2-specific instructions. I think
> >>> I jumped a bit too quickly to the conclusion.
> >>> 
> >>> QEMU's mips/disas doesn't help much in this case as it just indicates
> >>> User Defined Instruction. Presumably this instruction is specific to
> >>> this processor and is missing in QEMU. Are you able to get disassembly
> >>> of your program and look up what is under 0x765d1fa4 address which
> >>> caused the illegal instruction?
> >> 
> >> Hi Leon,
> >> 
> >> using IDA with a remote debug session to QEMU  I got the following
> >> disassembly (kept surrounding instructions to give some context). To
> >> IDA, this custom instruction is also unknown.
> >> 
> >> MEMORY:765D1F90 sw  $v1, 4($v0)
> >> MEMORY:765D1F94 addu$a0, $a1
> >> MEMORY:765D1F98 sw  $a0, 0($v0)
> >> MEMORY:765D1F9C
> >> MEMORY:765D1F9C loc_765D1F9C:
> >> MEMORY:765D1F9C addiu   $a0, $s1, 0x51B0
> >> MEMORY:765D1FA0 move$v0, $zero
> >> MEMORY:765D1FA0  # ---
> >> MEMORY:765D1FA4 .byte 0x70  # p
> >> MEMORY:765D1FA5 .byte 0x82  # é
> >> MEMORY:765D1FA6 .byte0
> >> MEMORY:765D1FA7 .byte 0x14
> >> MEMORY:765D1FA8  # ---
> >> MEMORY:765D1FA8 slti$v0, 2
> >> MEMORY:765D1FAC beqz$v0, loc_765D204C
> >> MEMORY:765D1FB0 nop
> >> MEMORY:765D1FB4 lw  $ra, 0x24($sp)
> >> MEMORY:765D1FB8
> >> MEMORY:765D1FB8 loc_765D1FB8:
> >> MEMORY:765D1FB8 move$v0, $s0
> >> MEMORY:765D1FBC lw  $s1, 0x20($sp)
> >> MEMORY:765D1FC0 lw  $s0, 0x1C($sp)
> > 
> > According to binutils this is SWAPW which belongs to XLR:
> > {"swapw",  "t,b",  0x7014, 0xfc00,
> > MOD_1|RD_2|LM|SM,   0,  XLR,0,  0 },
> > 
> > I'm afraid you won't be able to run binaries built for NetLogic XLP
> > until someone implements these instructions in QEMU.
> 
> Thanks Leon, you just beat me to it with

Re: [Qemu-devel] [PATCH v5 04/28] qapi: Add some enum tests

[Markus Armbruster]

Eric Blake  writes:

> Demonstrate that the qapi generator doesn't deal well with enums
> that aren't up to par. Later patches will update the expected
> results as the generator is made stricter.
>
> Signed-off-by: Eric Blake 

This is v4 plus the new enum-union-clash test.

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [PATCH 1/2] object: Add can_be_deleted callback to TypeInfo and TypeImpl

[Igor Mammedov]

On Wed, 25 Mar 2015 23:47:46 +0800
Lin Ma  wrote:

> 
> 在 2015年03月23日 21:30, Igor Mammedov 写道:
> > On Mon, 23 Mar 2015 14:13:07 +0100
> > Andreas Färber  wrote:
> >
> >> Hi,
> >>
> >> For consistency in git-log, please use "qom:" rather than "object:".
> >>
> >> Am 23.03.2015 um 13:06 schrieb Paolo Bonzini:
> >>> On 23/03/2015 11:36, Peter Crosthwaite wrote:
>  I don't think TypeInfo is the right place for this. You can however
>  define function hooks for Object in ObjectClass. See the unparent
>  field of ObjectClass for a precedent.
> >> Agree.
> >>
> >>> In this case, the right place could be UserCreatable.
> >> Maybe, not so familiar with that interface myself. Does object_del allow
> >> to delete non-UserCreatable objects? Then it wouldn't help much.
> > object_del() works only with /objects children, and so far the only way
> > that child placed there is via object_add() which requires a new object
> > to have TYPE_USER_CREATABLE interface.
> > I'd go this way rather than overhaul object_unparent() in this case.
> What about these changes?
> 1. Add a callback function named 'ObjectCanBeDeleted *can_be_deleted' to 
> struct ObjectClass in object.h
> 2. Call the function in qmp_object_del of qmp.c, says:
>  ObjectClass *obj_class = object_get_class(obj);
>  if (obj_class->can_be_deleted)
>  if (!obj_class->can_be_deleted(obj))
>  error out
> 
> 3. Then implement can_be_deleted callback in backends, says:
>  static bool host_memory_backend_can_be_deleted(Object *obj) {..}
> 
>  static void host_memory_backend_class_init(ObjectClass *oc, void 
> *data) {
>  ..
>  oc->can_be_deleted = host_memory_backend_can_be_deleted;
>  }

all backends currently implement TYPE_USER_CREATABLE interface,
so I'd rather extend UserCreatableClass with:

 bool can_be_deleted(UserCreatable *uc)

callback, which backends could implement if needed.

> >
> >>>   Alternatively...
> >>>
>  But is a better way to do this to add error handling to
>  object_unparent API and override object_unparent for your device in
>  question to throw the error? Then your change doesn't have to be
>  limited to QMP.
> >>> ... this is also a good choice.
> >> Well, I have doubts about asking someone who's not ultimately familiar
> >> with that code to refactor the API. For instance, we wouldn't want QEMU
> >> on shutdown or in error cases refusing to unparent some object.
> >>
> >> Doing it at QMP level (ObjectClass/UserCreatable) seems safer, given
> >> that Chun Yan's trivial block option fix ended up respinning a QemuOpts
> >> refactoring some twenty times before it got merged.
> >>
> >> Regards,
> >> Andreas
> >>
> >
> >
> 

Re: [Qemu-devel] [v3][PATCH 2/2] libxl: introduce gfx_passthru_kind

[Ian Campbell]

On Thu, 2015-03-26 at 08:53 +0800, Chen, Tiejun wrote:
> > Hrm, OK. I suppose we can live with autodetect and igd both meaning igd
> > and whoever adds a new type will have to remember to add a check for
> > qemu-trad then.
> >
> 
> When we really have to introduce a new type, this means we probably need 
> to change something inside qemu codes. So a new type should just go into 
> that table to support qemu upstream since now we shouldn't refactor 
> anything in qemu-xen-traditional, right?

We'd want to error out on attempts to use qemu-xen-trad with non-IGD
passthru.

Ian.

Re: [Qemu-devel] [PATCH 1/2] object: Add can_be_deleted callback to TypeInfo and TypeImpl

[Andreas Färber]

Am 26.03.2015 um 11:05 schrieb Igor Mammedov:
> On Wed, 25 Mar 2015 23:47:46 +0800
> Lin Ma  wrote:
>> 在 2015年03月23日 21:30, Igor Mammedov 写道:
>>> On Mon, 23 Mar 2015 14:13:07 +0100
>>> Andreas Färber  wrote:
 Am 23.03.2015 um 13:06 schrieb Paolo Bonzini:
> On 23/03/2015 11:36, Peter Crosthwaite wrote:
>> I don't think TypeInfo is the right place for this. You can however
>> define function hooks for Object in ObjectClass. See the unparent
>> field of ObjectClass for a precedent.
 Agree.

> In this case, the right place could be UserCreatable.
 Maybe, not so familiar with that interface myself. Does object_del allow
 to delete non-UserCreatable objects? Then it wouldn't help much.
>>> object_del() works only with /objects children, and so far the only way
>>> that child placed there is via object_add() which requires a new object
>>> to have TYPE_USER_CREATABLE interface.
>>> I'd go this way rather than overhaul object_unparent() in this case.
>> What about these changes?
>> 1. Add a callback function named 'ObjectCanBeDeleted *can_be_deleted' to 
>> struct ObjectClass in object.h
>> 2. Call the function in qmp_object_del of qmp.c, says:
>>  ObjectClass *obj_class = object_get_class(obj);
>>  if (obj_class->can_be_deleted)
>>  if (!obj_class->can_be_deleted(obj))
>>  error out
>>
>> 3. Then implement can_be_deleted callback in backends, says:
>>  static bool host_memory_backend_can_be_deleted(Object *obj) {..}
>>
>>  static void host_memory_backend_class_init(ObjectClass *oc, void 
>> *data) {
>>  ..
>>  oc->can_be_deleted = host_memory_backend_can_be_deleted;
>>  }
> 
> all backends currently implement TYPE_USER_CREATABLE interface,
> so I'd rather extend UserCreatableClass with:
> 
>  bool can_be_deleted(UserCreatable *uc)
> 
> callback, which backends could implement if needed.

As for the implementation, could we simply look at Object::ref field?

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH v5 05/28] qapi: Better error messages for bad enums

[Markus Armbruster]

Eric Blake  writes:

> The previous commit demonstrated that the generator had several
> flaws with less-than-perfect enums:
> - an enum that listed the same string twice (or two variant
> strings that map to the same C enumerator) ended up generating
> an invalid C enum
> - because the generator adds a _MAX terminator to each enum,
> the use of an enum member 'max' can also cause this clash
> - if an enum omits 'data', the generator left a python stack
> trace rather than a graceful message
> - an enum that used a non-array 'data' was silently accepted by
> the parser
> - an enum that used non-string members in the 'data' member
> was silently accepted by the parser
>
> Add check_enum to cover these situations, and update testcases
> to match.  While valid .json files won't trigger any of these
> cases, we might as well be nicer to developers that make a typo
> while trying to add new QAPI code.
>
> Signed-off-by: Eric Blake 

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [v6 08/14] migration: Add the core code of multi-thread compression

[Juan Quintela]

"Li, Liang Z"  wrote:
>> > --- a/arch_init.c
>> > +++ b/arch_init.c
>> > @@ -355,12 +355,33 @@ static DecompressParam *decomp_param;  static
>> > QemuThread *decompress_threads;  static uint8_t
>> *compressed_data_buf;
>> >
>> > +static int do_compress_ram_page(CompressParam *param);
>> > +
>> >  static void *do_data_compress(void *opaque)  {
>> > -while (!quit_comp_thread) {
>> > +CompressParam *param = opaque;
>> >
>> > -/* To be done */
>> What is the different with changing this loop to:
>> 
>> 
>> > +while (!quit_comp_thread) {
>> 
>> Here we don't have quit_comp_thread protected by anything.
>
> Yes, add a lock to protect quit_comp_thread is not hard and can make code
> more clean, but the lock will drop the performance. So I don't select to 
> protect
> it with a lock or something else. 
>
> Is there any problem to operate quit_comp_thread without protect?

You are not using atomic operations and you are using it from several
threads, so yes.  I still think that just ading another bool to the
parmas struct should be enough, and shouldn't affect a lot performance
(you are updating/looking at ->start near in all places that you touch it.)


>
>> 
>> > +qemu_mutex_lock(¶m->mutex);
>> > +/* Re-check the quit_comp_thread in case of
>> > + * terminate_compression_threads is called just before
>> > + * qemu_mutex_lock(¶m->mutex) and after
>> > + * while(!quit_comp_thread), re-check it here can make
>> > + * sure the compression thread terminate as expected.
>> > + */
>> > +while (!param->start && !quit_comp_thread) {
>> 
>> Here and next use is protected by param->mutex, but param is per
>> compression thread, so, it is not really protected.
>> 
>> > +qemu_cond_wait(¶m->cond, ¶m->mutex);
>> > +}
>> > +if (!quit_comp_thread) {
>> > +do_compress_ram_page(param);
>> > +}
>> > +param->start = false;
>> 
>> param->start is pretected by param->mutex everywhere
>> 
>> > +qemu_mutex_unlock(¶m->mutex);
>> >
>> > +qemu_mutex_lock(comp_done_lock);
>> > +param->done = true;
>> 
>> param->done protected by comp_done_lock
>> 
>> > +qemu_cond_signal(comp_done_cond);
>> > +qemu_mutex_unlock(comp_done_lock);
>> >  }
>> >
>> >  return NULL;
>> > @@ -368,9 +389,15 @@ static void *do_data_compress(void *opaque)
>> >
>> >  static inline void terminate_compression_threads(void)
>> >  {
>> > -quit_comp_thread = true;
>> > +int idx, thread_count;
>> >
>> > -/* To be done */
>> > +thread_count = migrate_compress_threads();
>> > +quit_comp_thread = true;
>> 
>> quite_comp_thread not protected again.
>> 
>> > +for (idx = 0; idx < thread_count; idx++) {
>> > +qemu_mutex_lock(&comp_param[idx].mutex);
>> > +qemu_cond_signal(&comp_param[idx].cond);
>> > +qemu_mutex_unlock(&comp_param[idx].mutex);
>> > +}
>> >  }
>> >
>> >  void migrate_compress_threads_join(void)
>> > @@ -420,6 +447,7 @@ void migrate_compress_threads_create(void)
>> >   * it's ops to empty.
>> >   */
>> >  comp_param[i].file = qemu_fopen_ops(NULL, &empty_ops);
>> > +comp_param[i].done = true;
>> >  qemu_mutex_init(&comp_param[i].mutex);
>> >  qemu_cond_init(&comp_param[i].cond);
>> >  qemu_thread_create(compress_threads + i, "compress", @@
>> > -829,6 +857,97 @@ static int ram_save_page(QEMUFile *f, RAMBlock*
>> block, ram_addr_t offset,
>> >  return pages;
>> >  }
>> >
>> > +static int do_compress_ram_page(CompressParam *param) {
>> > +int bytes_sent, blen;
>> > +uint8_t *p;
>> > +RAMBlock *block = param->block;
>> > +ram_addr_t offset = param->offset;
>> > +
>> > +p = memory_region_get_ram_ptr(block->mr) + (offset &
>> > + TARGET_PAGE_MASK);
>> > +
>> > +bytes_sent = save_page_header(param->file, block, offset |
>> > +  RAM_SAVE_FLAG_COMPRESS_PAGE);
>> > +blen = qemu_put_compression_data(param->file, p,
>> TARGET_PAGE_SIZE,
>> > + migrate_compress_level());
>> > +bytes_sent += blen;
>> > +atomic_inc(&acct_info.norm_pages);
>> > +
>> > +return bytes_sent;
>> > +}
>> > +
>> > +static inline void start_compression(CompressParam *param) {
>> > +param->done = false;
>> 
>> Not protected (well, its caller have protected it by comp_done_lock.
>> 
>> > +qemu_mutex_lock(¶m->mutex);
>> > +param->start = true;
>> > +qemu_cond_signal(¶m->cond);
>> > +qemu_mutex_unlock(¶m->mutex); }
>> > +
>> > +
>> > +static uint64_t bytes_transferred;
>> > +
>> > +static void flush_compressed_data(QEMUFile *f) {
>> > +int idx, len, thread_count;
>> > +
>> > +if (!migrate_use_compression()) {
>> > +return;
>> > +}
>> > +thread_count = migrate_compress_threads();
>> > +for (idx = 0; idx < thread_count; idx++) {
>> > +if (!comp_param[idx].done) {

Re: [Qemu-devel] [PATCH 1/2] object: Add can_be_deleted callback to TypeInfo and TypeImpl

[Igor Mammedov]

On Thu, 26 Mar 2015 11:07:27 +0100
Andreas Färber  wrote:

> Am 26.03.2015 um 11:05 schrieb Igor Mammedov:
> > On Wed, 25 Mar 2015 23:47:46 +0800
> > Lin Ma  wrote:
> >> 在 2015年03月23日 21:30, Igor Mammedov 写道:
> >>> On Mon, 23 Mar 2015 14:13:07 +0100
> >>> Andreas Färber  wrote:
>  Am 23.03.2015 um 13:06 schrieb Paolo Bonzini:
> > On 23/03/2015 11:36, Peter Crosthwaite wrote:
> >> I don't think TypeInfo is the right place for this. You can however
> >> define function hooks for Object in ObjectClass. See the unparent
> >> field of ObjectClass for a precedent.
>  Agree.
> 
> > In this case, the right place could be UserCreatable.
>  Maybe, not so familiar with that interface myself. Does object_del allow
>  to delete non-UserCreatable objects? Then it wouldn't help much.
> >>> object_del() works only with /objects children, and so far the only way
> >>> that child placed there is via object_add() which requires a new object
> >>> to have TYPE_USER_CREATABLE interface.
> >>> I'd go this way rather than overhaul object_unparent() in this case.
> >> What about these changes?
> >> 1. Add a callback function named 'ObjectCanBeDeleted *can_be_deleted' to 
> >> struct ObjectClass in object.h
> >> 2. Call the function in qmp_object_del of qmp.c, says:
> >>  ObjectClass *obj_class = object_get_class(obj);
> >>  if (obj_class->can_be_deleted)
> >>  if (!obj_class->can_be_deleted(obj))
> >>  error out
> >>
> >> 3. Then implement can_be_deleted callback in backends, says:
> >>  static bool host_memory_backend_can_be_deleted(Object *obj) {..}
> >>
> >>  static void host_memory_backend_class_init(ObjectClass *oc, void 
> >> *data) {
> >>  ..
> >>  oc->can_be_deleted = host_memory_backend_can_be_deleted;
> >>  }
> > 
> > all backends currently implement TYPE_USER_CREATABLE interface,
> > so I'd rather extend UserCreatableClass with:
> > 
> >  bool can_be_deleted(UserCreatable *uc)
> > 
> > callback, which backends could implement if needed.
> 
> As for the implementation, could we simply look at Object::ref field?
What value of ref, one would use to decide if deletion is possible?

In generic case object can have ref > 1 but still be eligible for deleting
via object-del.

> 
> Regards,
> Andreas
> 

Re: [Qemu-devel] [Qemu-block] [RFC PATCH COLO v2 01/13] docs: block replication's description

[Gonglei]

On 2015/3/26 16:58, Wen Congyang wrote:
> On 03/25/2015 11:38 PM, Eric Blake wrote:
>> > On 03/25/2015 03:36 AM, Wen Congyang wrote:
>>> >> Signed-off-by: Wen Congyang 
>>> >> Signed-off-by: Paolo Bonzini 
>>> >> Signed-off-by: Yang Hongyang 
>>> >> Signed-off-by: zhanghailiang 
>>> >> Signed-off-by: Gonglei 
>>> >> ---
>>> >>  docs/block-replication.txt | 147 
>>> >> +
>>> >>  1 file changed, 147 insertions(+)
>>> >>  create mode 100644 docs/block-replication.txt
>>> >>
>> > 
>> > Grammar review only (I'll leave the technical review to others)
>> > 
>>> >> diff --git a/docs/block-replication.txt b/docs/block-replication.txt
>>> >> new file mode 100644
>>> >> index 000..874ed8e
>>> >> --- /dev/null
>>> >> +++ b/docs/block-replication.txt
>>> >> @@ -0,0 +1,147 @@
>>> >> +Block replication
>>> >> +
>>> >> +Copyright Fujitsu, Corp. 2015
>>> >> +Copyright (c) 2015 Intel Corporation
>>> >> +Copyright (c) 2015 HUAWEI TECHNOLOGIES CO.,LTD.
>> > 
>> > Space after comma in English writing.
> Yes, but I am not sure I can change it. HUAWEI always use this way.
> You can find it in bootdevice.c

Good catch, Eric is right. I will change all of this writing way in Qemu at 2.4.

Thanks.

Regards,
-Gonglei

Re: [Qemu-devel] [Migration Bug? ] Occasionally, the content of VM's memory is inconsistent between Source and Destination of migration

[Juan Quintela]

Wen Congyang  wrote:
> On 03/25/2015 05:50 PM, Juan Quintela wrote:
>> zhanghailiang  wrote:
>>> Hi all,
>>>
>>> We found that, sometimes, the content of VM's memory is
>>> inconsistent between Source side and Destination side
>>> when we check it just after finishing migration but before VM continue to 
>>> Run.
>>>
>>> We use a patch like bellow to find this issue, you can find it from affix,
>>> and Steps to reprduce:
>>>
>>> (1) Compile QEMU:
>>>  ./configure --target-list=x86_64-softmmu  --extra-ldflags="-lssl" && make
>>>
>>> (2) Command and output:
>>> SRC: # x86_64-softmmu/qemu-system-x86_64 -enable-kvm -cpu
>>> qemu64,-kvmclock -netdev tap,id=hn0-device
>>> virtio-net-pci,id=net-pci0,netdev=hn0 -boot c -drive
>>> file=/mnt/sdb/pure_IMG/sles/sles11_sp3.img,if=none,id=drive-virtio-disk0,cache=unsafe
>>> -device
>>> virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0
>>> -vnc :7 -m 2048 -smp 2 -device piix3-usb-uhci -device usb-tablet
>>> -monitor stdio
>> 
>> Could you try to reproduce:
>> - without vhost
>> - without virtio-net
>> - cache=unsafe is going to give you trouble, but trouble should only
>>   happen after migration of pages have finished.
>
> If I use ide disk, it doesn't happen.
> Even if I use virtio-net with vhost=on, it still doesn't happen. I guess
> it is because I migrate the guest when it is booting. The virtio net
> device is not used in this case.

Kevin, Stefan, Michael, any great idea?

Thanks, Juan.

>
> Thanks
> Wen Congyang
>
>> 
>> What kind of load were you having when reproducing this issue?
>> Just to confirm, you have been able to reproduce this without COLO
>> patches, right?
>> 
>>> (qemu) migrate tcp:192.168.3.8:3004
>>> before saving ram complete
>>> ff703f6889ab8701e4e040872d079a28
>>> md_host : after saving ram complete
>>> ff703f6889ab8701e4e040872d079a28
>>>
>>> DST: # x86_64-softmmu/qemu-system-x86_64 -enable-kvm -cpu
>>> qemu64,-kvmclock -netdev tap,id=hn0,vhost=on -device
>>> virtio-net-pci,id=net-pci0,netdev=hn0 -boot c -drive
>>> file=/mnt/sdb/pure_IMG/sles/sles11_sp3.img,if=none,id=drive-virtio-disk0,cache=unsafe
>>> -device
>>> virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0
>>> -vnc :7 -m 2048 -smp 2 -device piix3-usb-uhci -device usb-tablet
>>> -monitor stdio -incoming tcp:0:3004
>>> (qemu) QEMU_VM_SECTION_END, after loading ram
>>> 230e1e68ece9cd4e769630e1bcb5ddfb
>>> md_host : after loading all vmstate
>>> 230e1e68ece9cd4e769630e1bcb5ddfb
>>> md_host : after cpu_synchronize_all_post_init
>>> 230e1e68ece9cd4e769630e1bcb5ddfb
>>>
>>> This happens occasionally, and it is more easy to reproduce when
>>> issue migration command during VM's startup time.
>> 
>> OK, a couple of things.  Memory don't have to be exactly identical.
>> Virtio devices in particular do funny things on "post-load".  There
>> aren't warantees for that as far as I know, we should end with an
>> equivalent device state in memory.
>> 
>>> We have done further test and found that some pages has been
>>> dirtied but its corresponding migration_bitmap is not set.
>>> We can't figure out which modules of QEMU has missed setting bitmap
>>> when dirty page of VM,
>>> it is very difficult for us to trace all the actions of dirtying VM's pages.
>> 
>> This seems to point to a bug in one of the devices.
>> 
>>> Actually, the first time we found this problem was in the COLO FT
>>> development, and it triggered some strange issues in
>>> VM which all pointed to the issue of inconsistent of VM's
>>> memory. (We have try to save all memory of VM to slave side every
>>> time
>>> when do checkpoint in COLO FT, and everything will be OK.)
>>>
>>> Is it OK for some pages that not transferred to destination when do
>>> migration ? Or is it a bug?
>> 
>> Pages transferred should be the same, after device state transmission is
>> when things could change.
>> 
>>> This issue has blocked our COLO development... :(
>>>
>>> Any help will be greatly appreciated!
>> 
>> Later, Juan.
>> 

Re: [Qemu-devel] [PATCH v5 30/45] Postcopy: Postcopy startup in migration thread

[Dr. David Alan Gilbert]

* David Gibson (da...@gibson.dropbear.id.au) wrote:
> On Wed, Feb 25, 2015 at 04:51:53PM +, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > Rework the migration thread to setup and start postcopy.
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  include/migration/migration.h |   3 +
> >  migration/migration.c | 161 
> > --
> >  trace-events  |   4 ++
> >  3 files changed, 164 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/migration/migration.h b/include/migration/migration.h
> > index 821d561..2c607e7 100644
> > --- a/include/migration/migration.h
> > +++ b/include/migration/migration.h
> > @@ -131,6 +131,9 @@ struct MigrationState
> >  /* Flag set once the migration has been asked to enter postcopy */
> >  bool start_postcopy;
> >  
> > +/* Flag set once the migration thread is running (and needs joining) */
> > +bool started_migration_thread;
> > +
> >  /* bitmap of pages that have been sent at least once
> >   * only maintained and used in postcopy at the moment
> >   * where it's used to send the dirtymap at the start
> > diff --git a/migration/migration.c b/migration/migration.c
> > index b1ad7b1..6bf9c8d 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -468,7 +468,10 @@ static void migrate_fd_cleanup(void *opaque)
> >  if (s->file) {
> >  trace_migrate_fd_cleanup();
> >  qemu_mutex_unlock_iothread();
> > -qemu_thread_join(&s->thread);
> > +if (s->started_migration_thread) {
> > +qemu_thread_join(&s->thread);
> > +s->started_migration_thread = false;
> > +}
> >  qemu_mutex_lock_iothread();
> >  
> >  qemu_fclose(s->file);
> > @@ -874,7 +877,6 @@ out:
> >  return NULL;
> >  }
> >  
> > -__attribute__ (( unused )) /* Until later in patch series */
> >  static int open_outgoing_return_path(MigrationState *ms)
> >  {
> >  
> > @@ -911,23 +913,141 @@ static void 
> > await_outgoing_return_path_close(MigrationState *ms)
> >  }
> >  
> >  /*
> > + * Switch from normal iteration to postcopy
> > + * Returns non-0 on error
> > + */
> > +static int postcopy_start(MigrationState *ms, bool *old_vm_running)
> > +{
> > +int ret;
> > +const QEMUSizedBuffer *qsb;
> > +int64_t time_at_stop = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> > +migrate_set_state(ms, MIG_STATE_ACTIVE, MIG_STATE_POSTCOPY_ACTIVE);
> > +
> > +trace_postcopy_start();
> > +qemu_mutex_lock_iothread();
> > +trace_postcopy_start_set_run();
> > +
> > +qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
> > +*old_vm_running = runstate_is_running();
> 
> I think that needs some explanation.  Why are you doing a wakeup on
> the source host?

This matches the existing code in migration_thread for the end of precopy;
Paolo's explanation of what it does is here:
https://lists.gnu.org/archive/html/qemu-devel/2014-08/msg04880.html

> > +ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
> > +
> > +if (ret < 0) {
> > +goto fail;
> > +}
> > +
> > +/*
> > + * in Finish migrate and with the io-lock held everything should
> > + * be quiet, but we've potentially still got dirty pages and we
> > + * need to tell the destination to throw any pages it's already 
> > received
> > + * that are dirty
> > + */
> > +if (ram_postcopy_send_discard_bitmap(ms)) {
> > +error_report("postcopy send discard bitmap failed");
> > +goto fail;
> > +}
> > +
> > +/*
> > + * send rest of state - note things that are doing postcopy
> > + * will notice we're in MIG_STATE_POSTCOPY_ACTIVE and not actually
> > + * wrap their state up here
> > + */
> > +qemu_file_set_rate_limit(ms->file, INT64_MAX);
> > +/* Ping just for debugging, helps line traces up */
> > +qemu_savevm_send_ping(ms->file, 2);
> > +
> > +/*
> > + * We need to leave the fd free for page transfers during the
> > + * loading of the device state, so wrap all the remaining
> > + * commands and state into a package that gets sent in one go
> > + */
> > +QEMUFile *fb = qemu_bufopen("w", NULL);
> > +if (!fb) {
> > +error_report("Failed to create buffered file");
> > +goto fail;
> > +}
> > +
> > +qemu_savevm_state_complete(fb);
> > +qemu_savevm_send_ping(fb, 3);
> > +
> > +qemu_savevm_send_postcopy_run(fb);
> > +
> > +/* <><> end of stuff going into the package */
> > +qsb = qemu_buf_get(fb);
> > +
> > +/* Now send that blob */
> > +if (qsb_get_length(qsb) > MAX_VM_CMD_PACKAGED_SIZE) {
> > +error_report("postcopy_start: Unreasonably large packaged state: 
> > %lu",
> > + (unsigned long)(qsb_get_length(qsb)));
> > +goto fail_closefb;
> > +}
> > +qemu_savevm_send_packaged(ms->file, qsb);
> > +qemu_fclose(fb);
> > 

Re: [Qemu-devel] [PATCH v11 2/4] hw/arm/boot: arm_load_kernel implemented as a machine init done notifier

[Alex Bennée]

Eric Auger  writes:

> Device tree nodes for the platform bus and its children dynamic sysbus
> devices are added in a machine init done notifier. To load the dtb once,
> after those latter nodes are built and before ROM freeze, the actual
> arm_load_kernel existing code is moved into a notifier notify function,
> arm_load_kernel_notify. arm_load_kernel now only registers the
> corresponding notifier.
>
> Machine files that do not support platform bus stay unchanged. Machine
> files willing to support dynamic sysbus devices must call arm_load_kernel
> before sysbus-fdt arm_register_platform_bus_fdt_creator to make sure
> dynamic sysbus device nodes are integrated in the dtb.
>
> Signed-off-by: Eric Auger 
> Reviewed-by: Shannon Zhao 
> Reviewed-by: Alexander Graf 

Reviewed-by: Alex Bennée 

>
> ---
>
> v8 -> v9:
> - fix compilation with arm-linux-user
> - reorder fields in ArmLoadKernelNotifier and use DO_UPCAST
>
> v7 -> v8:
> - Add Reviewed-by from Alex & Shannon
> - rebase on 2.2.0
>
> v6: creation of this patch file
> ---
>  hw/arm/boot.c| 14 +-
>  include/hw/arm/arm.h | 28 
>  2 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index a48d1b2..5ac5479 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -555,7 +555,7 @@ static void load_image_to_fw_cfg(FWCfgState *fw_cfg, 
> uint16_t size_key,
>  fw_cfg_add_bytes(fw_cfg, data_key, data, size);
>  }
>  
> -void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> +static void arm_load_kernel_notify(Notifier *notifier, void *data)
>  {
>  CPUState *cs;
>  int kernel_size;
> @@ -566,6 +566,11 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
> *info)
>  hwaddr entry, kernel_load_offset;
>  int big_endian;
>  static const ARMInsnFixup *primary_loader;
> +ArmLoadKernelNotifier *n = DO_UPCAST(ArmLoadKernelNotifier,
> + notifier, notifier);
> +ARMCPU *cpu = n->cpu;
> +struct arm_boot_info *info =
> +container_of(n, struct arm_boot_info, load_kernel_notifier);
>  
>  /* CPU objects (unlike devices) are not automatically reset on system
>   * reset, so we must always register a handler to do so. If we're
> @@ -773,3 +778,10 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
> *info)
>  ARM_CPU(cs)->env.boot_info = info;
>  }
>  }
> +
> +void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> +{
> +info->load_kernel_notifier.cpu = cpu;
> +info->load_kernel_notifier.notifier.notify = arm_load_kernel_notify;
> +
> qemu_add_machine_init_done_notifier(&info->load_kernel_notifier.notifier);
> +}
> diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
> index 5c940eb..760804c 100644
> --- a/include/hw/arm/arm.h
> +++ b/include/hw/arm/arm.h
> @@ -13,11 +13,21 @@
>  
>  #include "exec/memory.h"
>  #include "hw/irq.h"
> +#include "qemu/notify.h"
>  
>  /* armv7m.c */
>  qemu_irq *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
>const char *kernel_filename, const char *cpu_model);
>  
> +/*
> + * struct used as a parameter of the arm_load_kernel machine init
> + * done notifier
> + */
> +typedef struct {
> +Notifier notifier; /* actual notifier */
> +ARMCPU *cpu; /* handle to the first cpu object */
> +} ArmLoadKernelNotifier;
> +
>  /* arm_boot.c */
>  struct arm_boot_info {
>  uint64_t ram_size;
> @@ -64,6 +74,8 @@ struct arm_boot_info {
>   * the user it should implement this hook.
>   */
>  void (*modify_dtb)(const struct arm_boot_info *info, void *fdt);
> +/* machine init done notifier executing arm_load_dtb */
> +ArmLoadKernelNotifier load_kernel_notifier;
>  /* Used internally by arm_boot.c */
>  int is_linux;
>  hwaddr initrd_start;
> @@ -75,6 +87,22 @@ struct arm_boot_info {
>   */
>  bool firmware_loaded;
>  };
> +
> +/**
> + * arm_load_kernel - Loads memory with everything needed to boot
> + *
> + * @cpu: handle to the first CPU object
> + * @info: handle to the boot info struct
> + * Registers a machine init done notifier that copies to memory
> + * everything needed to boot, depending on machine and user options:
> + * kernel image, boot loaders, initrd, dtb. Also registers the CPU
> + * reset handler.
> + *
> + * In case the machine file supports the platform bus device and its
> + * dynamically instantiable sysbus devices, this function must be called
> + * before sysbus-fdt arm_register_platform_bus_fdt_creator. Indeed the
> + * machine init done notifiers are called in registration reverse order.
> + */
>  void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info);
>  
>  /* Multiplication factor to convert from system clock ticks to qemu timer

-- 
Alex Bennée

[Qemu-devel] [PATCH] vhost: fix typo in vq_index description

[Greg Kurz]

Signed-off-by: Greg Kurz 
---
 include/hw/virtio/vhost.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 71ef18f..88e1e56 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -36,7 +36,7 @@ struct vhost_dev {
 MemoryRegionSection *mem_sections;
 struct vhost_virtqueue *vqs;
 int nvqs;
-/* the first virtuque which would be used by this vhost dev */
+/* the first virtqueue which would be used by this vhost dev */
 int vq_index;
 unsigned long long features;
 unsigned long long acked_features;

[Qemu-devel] [PATCH v3 5/6] virtio-input: emulated devices

[Gerd Hoffmann]

This patch adds the virtio-input-hid base class and
virtio-{keyboard,mouse,tablet} subclasses building on the base class.
They are hooked up to the qemu input core and deliver input events
to the guest like all other hid devices (ps/2 kbd, usb tablet, ...).

Using them is as simple as adding "-device virtio-tablet-pci" to your
command line.  If you want add multiple devices but don't want waste
a pci slot for each you can compose a multifunction device this way:

qemu -device virtio-keyboard-pci,addr=0d.0,multifunction=on \
 -device virtio-tablet-pci,addr=0d.1,multifunction=on

Signed-off-by: Gerd Hoffmann 
---
 hw/input/Makefile.objs   |   1 +
 hw/input/virtio-input-hid.c  | 502 +++
 hw/virtio/virtio-pci.c   |  84 +++
 hw/virtio/virtio-pci.h   |  13 +
 include/hw/virtio/virtio-input.h |  21 ++
 5 files changed, 621 insertions(+)
 create mode 100644 hw/input/virtio-input-hid.c

diff --git a/hw/input/Makefile.objs b/hw/input/Makefile.objs
index ee8bba9..0dae710 100644
--- a/hw/input/Makefile.objs
+++ b/hw/input/Makefile.objs
@@ -10,6 +10,7 @@ common-obj-$(CONFIG_VMMOUSE) += vmmouse.o
 
 ifeq ($(CONFIG_LINUX),y)
 common-obj-$(CONFIG_VIRTIO) += virtio-input.o
+common-obj-$(CONFIG_VIRTIO) += virtio-input-hid.o
 endif
 
 obj-$(CONFIG_MILKYMIST) += milkymist-softusb.o
diff --git a/hw/input/virtio-input-hid.c b/hw/input/virtio-input-hid.c
new file mode 100644
index 000..32cc94a
--- /dev/null
+++ b/hw/input/virtio-input-hid.c
@@ -0,0 +1,502 @@
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#include "qemu/iov.h"
+
+#include "hw/qdev.h"
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-input.h"
+
+#undef CONFIG_CURSES
+#include "ui/console.h"
+
+#include "standard-headers/linux/input.h"
+
+#define VIRTIO_ID_NAME_KEYBOARD "QEMU Virtio Keyboard"
+#define VIRTIO_ID_NAME_MOUSE"QEMU Virtio Mouse"
+#define VIRTIO_ID_NAME_TABLET   "QEMU Virtio Tablet"
+
+/* - */
+
+static const unsigned int keymap_qcode[Q_KEY_CODE_MAX] = {
+[Q_KEY_CODE_ESC] = KEY_ESC,
+[Q_KEY_CODE_1]   = KEY_1,
+[Q_KEY_CODE_2]   = KEY_2,
+[Q_KEY_CODE_3]   = KEY_3,
+[Q_KEY_CODE_4]   = KEY_4,
+[Q_KEY_CODE_5]   = KEY_5,
+[Q_KEY_CODE_6]   = KEY_6,
+[Q_KEY_CODE_7]   = KEY_7,
+[Q_KEY_CODE_8]   = KEY_8,
+[Q_KEY_CODE_9]   = KEY_9,
+[Q_KEY_CODE_0]   = KEY_0,
+[Q_KEY_CODE_MINUS]   = KEY_MINUS,
+[Q_KEY_CODE_EQUAL]   = KEY_EQUAL,
+[Q_KEY_CODE_BACKSPACE]   = KEY_BACKSPACE,
+
+[Q_KEY_CODE_TAB] = KEY_TAB,
+[Q_KEY_CODE_Q]   = KEY_Q,
+[Q_KEY_CODE_W]   = KEY_W,
+[Q_KEY_CODE_E]   = KEY_E,
+[Q_KEY_CODE_R]   = KEY_R,
+[Q_KEY_CODE_T]   = KEY_T,
+[Q_KEY_CODE_Y]   = KEY_Y,
+[Q_KEY_CODE_U]   = KEY_U,
+[Q_KEY_CODE_I]   = KEY_I,
+[Q_KEY_CODE_O]   = KEY_O,
+[Q_KEY_CODE_P]   = KEY_P,
+[Q_KEY_CODE_BRACKET_LEFT]= KEY_LEFTBRACE,
+[Q_KEY_CODE_BRACKET_RIGHT]   = KEY_RIGHTBRACE,
+[Q_KEY_CODE_RET] = KEY_ENTER,
+
+[Q_KEY_CODE_CTRL]= KEY_LEFTCTRL,
+[Q_KEY_CODE_A]   = KEY_A,
+[Q_KEY_CODE_S]   = KEY_S,
+[Q_KEY_CODE_D]   = KEY_D,
+[Q_KEY_CODE_F]   = KEY_F,
+[Q_KEY_CODE_G]   = KEY_G,
+[Q_KEY_CODE_H]   = KEY_H,
+[Q_KEY_CODE_J]   = KEY_J,
+[Q_KEY_CODE_K]   = KEY_K,
+[Q_KEY_CODE_L]   = KEY_L,
+[Q_KEY_CODE_SEMICOLON]   = KEY_SEMICOLON,
+[Q_KEY_CODE_APOSTROPHE]  = KEY_APOSTROPHE,
+[Q_KEY_CODE_GRAVE_ACCENT]= KEY_GRAVE,
+
+[Q_KEY_CODE_SHIFT]   = KEY_LEFTSHIFT,
+[Q_KEY_CODE_BACKSLASH]   = KEY_BACKSLASH,
+[Q_KEY_CODE_LESS]= KEY_102ND,
+[Q_KEY_CODE_Z]   = KEY_Z,
+[Q_KEY_CODE_X]   = KEY_X,
+[Q_KEY_CODE_C]   = KEY_C,
+[Q_KEY_CODE_V]   = KEY_V,
+[Q_KEY_CODE_B]   = KEY_B,
+[Q_KEY_CODE_N]   = KEY_N,
+[Q_KEY_CODE_M]   = KEY_M,
+[Q_KEY_CODE_COMMA]   = KEY_COMMA,
+[Q_KEY_CODE_DOT] = KEY_DOT,
+[Q_KEY_CODE_SLASH]   = KEY_SLASH,
+[Q_KEY_CODE_SHIFT_R] = KEY_RIGHTSHIFT,
+
+[Q_KEY_CODE_ALT] = KEY_LEFTALT,
+[Q_KEY_CODE_SPC] 

[Qemu-devel] [PATCH v3 6/6] virtio-input: evdev passthrough

[Gerd Hoffmann]

This allows to assign host input devices to the guest:

qemu -device virto-input-host-pci,evdev=/dev/input/event

The guest gets exclusive access to the input device, so be careful
with assigning the keyboard if you have only one connected to your
machine.

Signed-off-by: Gerd Hoffmann 
---
 hw/input/Makefile.objs   |   1 +
 hw/input/virtio-input-host.c | 182 +++
 hw/virtio/virtio-pci.c   |  30 +++
 hw/virtio/virtio-pci.h   |  10 +++
 include/hw/virtio/virtio-input.h |  13 +++
 5 files changed, 236 insertions(+)
 create mode 100644 hw/input/virtio-input-host.c

diff --git a/hw/input/Makefile.objs b/hw/input/Makefile.objs
index 0dae710..624ba7e 100644
--- a/hw/input/Makefile.objs
+++ b/hw/input/Makefile.objs
@@ -11,6 +11,7 @@ common-obj-$(CONFIG_VMMOUSE) += vmmouse.o
 ifeq ($(CONFIG_LINUX),y)
 common-obj-$(CONFIG_VIRTIO) += virtio-input.o
 common-obj-$(CONFIG_VIRTIO) += virtio-input-hid.o
+common-obj-$(CONFIG_VIRTIO) += virtio-input-host.o
 endif
 
 obj-$(CONFIG_MILKYMIST) += milkymist-softusb.o
diff --git a/hw/input/virtio-input-host.c b/hw/input/virtio-input-host.c
new file mode 100644
index 000..b16cc4c
--- /dev/null
+++ b/hw/input/virtio-input-host.c
@@ -0,0 +1,182 @@
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#include "qemu-common.h"
+#include "qemu/sockets.h"
+
+#include "hw/qdev.h"
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-input.h"
+
+#include "standard-headers/linux/input.h"
+
+/* - */
+
+static struct virtio_input_config virtio_input_host_config[] = {
+{ /* empty list */ },
+};
+
+static void virtio_input_host_event(void *opaque)
+{
+VirtIOInputHost *vih = opaque;
+VirtIOInput *vinput = VIRTIO_INPUT(vih);
+struct virtio_input_event virtio;
+struct input_event evdev;
+int rc;
+
+for (;;) {
+rc = read(vih->fd, &evdev, sizeof(evdev));
+if (rc != sizeof(evdev)) {
+break;
+}
+
+virtio.type  = cpu_to_le16(evdev.type);
+virtio.code  = cpu_to_le16(evdev.code);
+virtio.value = cpu_to_le32(evdev.value);
+virtio_input_send(vinput, &virtio);
+}
+}
+
+static void virtio_input_bits_config(VirtIOInputHost *vih,
+ int type, int count)
+{
+virtio_input_config bits;
+int rc, i, size = 0;
+
+memset(&bits, 0, sizeof(bits));
+rc = ioctl(vih->fd, EVIOCGBIT(type, count/8), bits.u.bitmap);
+if (rc < 0) {
+return;
+}
+
+for (i = 0; i < count/8; i++) {
+if (bits.u.bitmap[i]) {
+size = i+1;
+}
+}
+if (size == 0) {
+return;
+}
+
+bits.select = VIRTIO_INPUT_CFG_EV_BITS;
+bits.subsel = type;
+bits.size   = size;
+virtio_input_add_config(VIRTIO_INPUT(vih), &bits);
+}
+
+static void virtio_input_host_realize(DeviceState *dev, Error **errp)
+{
+VirtIOInputHost *vih = VIRTIO_INPUT_HOST(dev);
+VirtIOInput *vinput = VIRTIO_INPUT(dev);
+virtio_input_config id;
+struct input_id ids;
+int rc, ver;
+
+if (!vih->evdev) {
+error_setg(errp, "evdev property is required");
+return;
+}
+
+vih->fd = open(vih->evdev, O_RDWR);
+if (vih->fd < 0)  {
+error_setg_file_open(errp, errno, vih->evdev);
+return;
+}
+qemu_set_nonblock(vih->fd);
+
+rc = ioctl(vih->fd, EVIOCGVERSION, &ver);
+if (rc < 0) {
+error_setg(errp, "%s: is not an evdev device", vih->evdev);
+goto err_close;
+}
+
+rc = ioctl(vih->fd, EVIOCGRAB, 1);
+if (rc < 0) {
+error_setg_errno(errp, errno, "%s: failed to get exclusive access",
+ vih->evdev);
+goto err_close;
+}
+
+memset(&id, 0, sizeof(id));
+ioctl(vih->fd, EVIOCGNAME(sizeof(id.u.string)-1), id.u.string);
+id.select = VIRTIO_INPUT_CFG_ID_NAME;
+id.size = strlen(id.u.string);
+virtio_input_add_config(vinput, &id);
+
+if (ioctl(vih->fd, EVIOCGID, &ids) == 0) {
+memset(&id, 0, sizeof(id));
+id.select = VIRTIO_INPUT_CFG_ID_DEVIDS;
+id.size = sizeof(struct virtio_input_devids);
+id.u.ids.bustype = cpu_to_le16(ids.bustype);
+id.u.ids.vendor  = cpu_to_le16(ids.vendor);
+id.u.ids.product = cpu_to_le16(ids.product);
+id.u.ids.version = cpu_to_le16(ids.version);
+virtio_input_add_config(vinput, &id);
+}
+
+virtio_input_bits_config(vih, EV_KEY, KEY_CNT);
+virtio_input_bits_config(vih, EV_REL, REL_CNT);
+virtio_input_bits_config(vih, EV_ABS, ABS_CNT);
+virtio_input_bits_config(vih, EV_MSC, MSC_CNT);
+virtio_input_bits_config(vih, EV_SW,  SW_CNT);
+
+qemu_set_fd_handler(vih->fd, virtio_input_host_event, NULL, vih);
+return;
+
+err_close:
+close(vih->fd);
+

[Qemu-devel] [PATCH v3 0/6] virtio: add input device

[Gerd Hoffmann]

This patch series adds virtio input devices.  It's basically sending
linux evdev events over virtio.  There is support for emulated hid
devices (i.e. send usual input to virtio device instead of usb or ps2
device) and for evdev device pass-through.

This depends on mst's virtio-1.0 patches.

Changes in v3:
 * sync header with v5 of the kernel driver patch.
   [ there was a abi change, testing this series requires v5 of the
 kernel patch, the branch below is up-to-date too ]
 * minor tweaks.

Changes in v2:
 * sync header files from linux & use them.
 * added virtio_input_devids struct, pass device ids from host
   (requested on guest driver review).

Guest driver:
  https://www.kraxel.org/cgit/linux/log/?h=virtio-input

Specification (slightly outdated, latest changes from kernel driver
review not added yet):
  https://www.kraxel.org/cgit/virtio-spec/log/?h=virtio-input
  https://www.kraxel.org/virtio/virtio-v1.0-csprd03-virtio-input.html#x1-2640007

Gerd Hoffmann (6):
  pci: add  PCI_CLASS_INPUT_*
  virtio-input: add virtio_input.h
  virtio-input: add linux/input.h
  virtio-input: core code & base class
  virtio-input: emulated devices
  virtio-input: evdev passthrough

 hw/input/Makefile.objs|6 +
 hw/input/virtio-input-hid.c   |  502 +++
 hw/input/virtio-input-host.c  |  182 
 hw/input/virtio-input.c   |  282 ++
 hw/virtio/virtio-pci.c|  150 
 hw/virtio/virtio-pci.h|   37 +
 include/hw/pci/pci_ids.h  |7 +
 include/hw/virtio/virtio-input.h  |  118 +++
 include/hw/virtio/virtio.h|1 +
 include/standard-headers/linux/input.h| 1198 +
 include/standard-headers/linux/virtio_ids.h   |1 +
 include/standard-headers/linux/virtio_input.h |   76 ++
 scripts/update-linux-headers.sh   |4 +-
 13 files changed, 2563 insertions(+), 1 deletion(-)
 create mode 100644 hw/input/virtio-input-hid.c
 create mode 100644 hw/input/virtio-input-host.c
 create mode 100644 hw/input/virtio-input.c
 create mode 100644 include/hw/virtio/virtio-input.h
 create mode 100644 include/standard-headers/linux/input.h
 create mode 100644 include/standard-headers/linux/virtio_input.h

-- 
1.8.3.1

[Qemu-devel] [PATCH v3 2/6] virtio-input: add virtio_input.h

[Gerd Hoffmann]

Using scripts/update-linux-headers.sh with linux-input kernel branch.

Signed-off-by: Gerd Hoffmann 
---
 include/standard-headers/linux/virtio_ids.h   |  1 +
 include/standard-headers/linux/virtio_input.h | 76 +++
 2 files changed, 77 insertions(+)
 create mode 100644 include/standard-headers/linux/virtio_input.h

diff --git a/include/standard-headers/linux/virtio_ids.h 
b/include/standard-headers/linux/virtio_ids.h
index 284fc3a..5f60aa4 100644
--- a/include/standard-headers/linux/virtio_ids.h
+++ b/include/standard-headers/linux/virtio_ids.h
@@ -39,5 +39,6 @@
 #define VIRTIO_ID_9P   9 /* 9p virtio console */
 #define VIRTIO_ID_RPROC_SERIAL 11 /* virtio remoteproc serial link */
 #define VIRTIO_ID_CAIF12 /* Virtio caif */
+#define VIRTIO_ID_INPUT18 /* virtio input */
 
 #endif /* _LINUX_VIRTIO_IDS_H */
diff --git a/include/standard-headers/linux/virtio_input.h 
b/include/standard-headers/linux/virtio_input.h
new file mode 100644
index 000..a98a797
--- /dev/null
+++ b/include/standard-headers/linux/virtio_input.h
@@ -0,0 +1,76 @@
+#ifndef _LINUX_VIRTIO_INPUT_H
+#define _LINUX_VIRTIO_INPUT_H
+/* This header is BSD licensed so anyone can use the definitions to implement
+ * compatible drivers/servers.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of IBM nor the names of its contributors
+ *may be used to endorse or promote products derived from this software
+ *without specific prior written permission.
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL IBM OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE. */
+
+#include "standard-headers/linux/types.h"
+
+enum virtio_input_config_select {
+   VIRTIO_INPUT_CFG_UNSET  = 0x00,
+   VIRTIO_INPUT_CFG_ID_NAME= 0x01,
+   VIRTIO_INPUT_CFG_ID_SERIAL  = 0x02,
+   VIRTIO_INPUT_CFG_ID_DEVIDS  = 0x03,
+   VIRTIO_INPUT_CFG_PROP_BITS  = 0x10,
+   VIRTIO_INPUT_CFG_EV_BITS= 0x11,
+   VIRTIO_INPUT_CFG_ABS_INFO   = 0x12,
+};
+
+struct virtio_input_absinfo {
+   uint32_t min;
+   uint32_t max;
+   uint32_t fuzz;
+   uint32_t flat;
+   uint32_t res;
+};
+
+struct virtio_input_devids {
+   uint16_t bustype;
+   uint16_t vendor;
+   uint16_t product;
+   uint16_t version;
+};
+
+struct virtio_input_config {
+   uint8_tselect;
+   uint8_tsubsel;
+   uint8_tsize;
+   uint8_treserved[5];
+   union {
+   char string[128];
+   uint8_t bitmap[128];
+   struct virtio_input_absinfo abs;
+   struct virtio_input_devids ids;
+   } u;
+};
+
+struct virtio_input_event {
+   uint16_t type;
+   uint16_t code;
+   uint32_t value;
+};
+
+#endif /* _LINUX_VIRTIO_INPUT_H */
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 4/6] virtio-input: core code & base class

[Gerd Hoffmann]

This patch adds virtio-input support to qemu.  It brings a abstract
base class providing core support, other classes can build on it to
actually implement input devices.

virtio-input basically sends linux input layer events (evdev) over
virtio.

Signed-off-by: Gerd Hoffmann 
---
 hw/input/Makefile.objs   |   4 +
 hw/input/virtio-input.c  | 282 +++
 hw/virtio/virtio-pci.c   |  36 +
 hw/virtio/virtio-pci.h   |  14 ++
 include/hw/virtio/virtio-input.h |  84 
 include/hw/virtio/virtio.h   |   1 +
 6 files changed, 421 insertions(+)
 create mode 100644 hw/input/virtio-input.c
 create mode 100644 include/hw/virtio/virtio-input.h

diff --git a/hw/input/Makefile.objs b/hw/input/Makefile.objs
index e8c80b9..ee8bba9 100644
--- a/hw/input/Makefile.objs
+++ b/hw/input/Makefile.objs
@@ -8,6 +8,10 @@ common-obj-$(CONFIG_STELLARIS_INPUT) += stellaris_input.o
 common-obj-$(CONFIG_TSC2005) += tsc2005.o
 common-obj-$(CONFIG_VMMOUSE) += vmmouse.o
 
+ifeq ($(CONFIG_LINUX),y)
+common-obj-$(CONFIG_VIRTIO) += virtio-input.o
+endif
+
 obj-$(CONFIG_MILKYMIST) += milkymist-softusb.o
 obj-$(CONFIG_PXA2XX) += pxa2xx_keypad.o
 obj-$(CONFIG_TSC210X) += tsc210x.o
diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c
new file mode 100644
index 000..17cbe97
--- /dev/null
+++ b/hw/input/virtio-input.c
@@ -0,0 +1,282 @@
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#include "qemu/iov.h"
+
+#include "hw/qdev.h"
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-input.h"
+
+#include "standard-headers/linux/input.h"
+
+/* - */
+
+void virtio_input_send(VirtIOInput *vinput, virtio_input_event *event)
+{
+VirtQueueElement elem;
+unsigned have, need;
+int i, len;
+
+/* queue up events ... */
+if (vinput->qindex == vinput->qsize) {
+vinput->qsize++;
+vinput->queue = realloc(vinput->queue, vinput->qsize *
+sizeof(virtio_input_event));
+}
+vinput->queue[vinput->qindex++] = *event;
+
+/* ... until we see a report sync ... */
+if (event->type != cpu_to_le16(EV_SYN) ||
+event->code != cpu_to_le16(SYN_REPORT)) {
+return;
+}
+
+/* ... then check available space ... */
+need = sizeof(virtio_input_event) * vinput->qindex;
+virtqueue_get_avail_bytes(vinput->evt, &have, NULL, need, 0);
+if (have < need) {
+vinput->qindex = 0;
+fprintf(stderr, "%s: ENOSPC in vq, dropping events\n", __func__);
+return;
+}
+
+/* ... and finally pass them to the guest */
+for (i = 0; i < vinput->qindex; i++) {
+if (!virtqueue_pop(vinput->evt, &elem)) {
+/* should not happen, we've checked for space beforehand */
+fprintf(stderr, "%s: Huh?  No vq elem available ...\n", __func__);
+return;
+}
+len = iov_from_buf(elem.in_sg, elem.in_num,
+   0, vinput->queue+i, sizeof(virtio_input_event));
+virtqueue_push(vinput->evt, &elem, len);
+}
+virtio_notify(VIRTIO_DEVICE(vinput), vinput->evt);
+vinput->qindex = 0;
+}
+
+static void virtio_input_handle_evt(VirtIODevice *vdev, VirtQueue *vq)
+{
+/* nothing */
+}
+
+static void virtio_input_handle_sts(VirtIODevice *vdev, VirtQueue *vq)
+{
+VirtIOInputClass *vic = VIRTIO_INPUT_GET_CLASS(vdev);
+VirtIOInput *vinput = VIRTIO_INPUT(vdev);
+virtio_input_event event;
+VirtQueueElement elem;
+int len;
+
+while (virtqueue_pop(vinput->sts, &elem)) {
+memset(&event, 0, sizeof(event));
+len = iov_to_buf(elem.out_sg, elem.out_num,
+ 0, &event, sizeof(event));
+if (vic->handle_status) {
+vic->handle_status(vinput, &event);
+}
+virtqueue_push(vinput->sts, &elem, len);
+}
+virtio_notify(vdev, vinput->sts);
+}
+
+static virtio_input_config *virtio_input_find_config(VirtIOInput *vinput,
+ uint8_t select,
+ uint8_t subsel)
+{
+VirtIOInputConfig *cfg;
+
+QTAILQ_FOREACH(cfg, &vinput->cfg_list, node) {
+if (select == cfg->config.select &&
+subsel == cfg->config.subsel) {
+return &cfg->config;
+}
+}
+return NULL;
+}
+
+void virtio_input_add_config(VirtIOInput *vinput,
+ virtio_input_config *config)
+{
+VirtIOInputConfig *cfg;
+
+if (virtio_input_find_config(vinput, config->select, config->subsel)) {
+/* should not happen */
+fprintf(stderr, "%s: duplicate config: %d/%d\n",
+__func__, config->select, config->subsel);
+abort();
+}
+
+cfg = g_new0(VirtIOInputConfig

[Qemu-devel] [PATCH v3 3/6] virtio-input: add linux/input.h

[Gerd Hoffmann]

Linux input layer (evdev) header file.

Signed-off-by: Gerd Hoffmann 
---
 include/standard-headers/linux/input.h | 1198 
 scripts/update-linux-headers.sh|4 +-
 2 files changed, 1201 insertions(+), 1 deletion(-)
 create mode 100644 include/standard-headers/linux/input.h

diff --git a/include/standard-headers/linux/input.h 
b/include/standard-headers/linux/input.h
new file mode 100644
index 000..b94d365
--- /dev/null
+++ b/include/standard-headers/linux/input.h
@@ -0,0 +1,1198 @@
+/*
+ * Copyright (c) 1999-2002 Vojtech Pavlik
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+#ifndef _INPUT_H
+#define _INPUT_H
+
+
+#include 
+#include 
+#include 
+#include "standard-headers/linux/types.h"
+
+
+/*
+ * The event structure itself
+ */
+
+struct input_event {
+   struct timeval time;
+   uint16_t type;
+   uint16_t code;
+   int32_t value;
+};
+
+/*
+ * Protocol version.
+ */
+
+#define EV_VERSION 0x010001
+
+/*
+ * IOCTLs (0x00 - 0x7f)
+ */
+
+struct input_id {
+   uint16_t bustype;
+   uint16_t vendor;
+   uint16_t product;
+   uint16_t version;
+};
+
+/**
+ * struct input_absinfo - used by EVIOCGABS/EVIOCSABS ioctls
+ * @value: latest reported value for the axis.
+ * @minimum: specifies minimum value for the axis.
+ * @maximum: specifies maximum value for the axis.
+ * @fuzz: specifies fuzz value that is used to filter noise from
+ * the event stream.
+ * @flat: values that are within this value will be discarded by
+ * joydev interface and reported as 0 instead.
+ * @resolution: specifies resolution for the values reported for
+ * the axis.
+ *
+ * Note that input core does not clamp reported values to the
+ * [minimum, maximum] limits, such task is left to userspace.
+ *
+ * Resolution for main axes (ABS_X, ABS_Y, ABS_Z) is reported in
+ * units per millimeter (units/mm), resolution for rotational axes
+ * (ABS_RX, ABS_RY, ABS_RZ) is reported in units per radian.
+ */
+struct input_absinfo {
+   int32_t value;
+   int32_t minimum;
+   int32_t maximum;
+   int32_t fuzz;
+   int32_t flat;
+   int32_t resolution;
+};
+
+/**
+ * struct input_keymap_entry - used by EVIOCGKEYCODE/EVIOCSKEYCODE ioctls
+ * @scancode: scancode represented in machine-endian form.
+ * @len: length of the scancode that resides in @scancode buffer.
+ * @index: index in the keymap, may be used instead of scancode
+ * @flags: allows to specify how kernel should handle the request. For
+ * example, setting INPUT_KEYMAP_BY_INDEX flag indicates that kernel
+ * should perform lookup in keymap by @index instead of @scancode
+ * @keycode: key code assigned to this scancode
+ *
+ * The structure is used to retrieve and modify keymap data. Users have
+ * option of performing lookup either by @scancode itself or by @index
+ * in keymap entry. EVIOCGKEYCODE will also return scancode or index
+ * (depending on which element was used to perform lookup).
+ */
+struct input_keymap_entry {
+#define INPUT_KEYMAP_BY_INDEX  (1 << 0)
+   uint8_t  flags;
+   uint8_t  len;
+   uint16_t index;
+   uint32_t keycode;
+   uint8_t  scancode[32];
+};
+
+#define EVIOCGVERSION  _IOR('E', 0x01, int)/* get 
driver version */
+#define EVIOCGID   _IOR('E', 0x02, struct input_id)/* get 
device ID */
+#define EVIOCGREP  _IOR('E', 0x03, unsigned int[2])/* get 
repeat settings */
+#define EVIOCSREP  _IOW('E', 0x03, unsigned int[2])/* set 
repeat settings */
+
+#define EVIOCGKEYCODE  _IOR('E', 0x04, unsigned int[2])/* get 
keycode */
+#define EVIOCGKEYCODE_V2   _IOR('E', 0x04, struct input_keymap_entry)
+#define EVIOCSKEYCODE  _IOW('E', 0x04, unsigned int[2])/* set 
keycode */
+#define EVIOCSKEYCODE_V2   _IOW('E', 0x04, struct input_keymap_entry)
+
+#define EVIOCGNAME(len)_IOC(_IOC_READ, 'E', 0x06, len) 
/* get device name */
+#define EVIOCGPHYS(len)_IOC(_IOC_READ, 'E', 0x07, len) 
/* get physical location */
+#define EVIOCGUNIQ(len)_IOC(_IOC_READ, 'E', 0x08, len) 
/* get unique identifier */
+#define EVIOCGPROP(len)_IOC(_IOC_READ, 'E', 0x09, len) 
/* get device properties */
+
+/**
+ * EVIOCGMTSLOTS(len) - get MT slot values
+ * @len: size of the data buffer in bytes
+ *
+ * The ioctl buffer argument should be binary equivalent to
+ *
+ * struct input_mt_request_layout {
+ * uint32_t code;
+ * int32_t values[num_slots];
+ * };
+ *
+ * where num_slots is the (arbitrary) number of MT slots to extract.
+ *
+ * The ioctl size argument (len) is the size of the buffer, which
+ * should satisfy len = (num_slots + 1) * sizeof(int32_t).  If len is
+ * too 

[Qemu-devel] [PATCH v3 1/6] pci: add PCI_CLASS_INPUT_*

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 include/hw/pci/pci_ids.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
index d7be386..dfccefc 100644
--- a/include/hw/pci/pci_ids.h
+++ b/include/hw/pci/pci_ids.h
@@ -46,6 +46,13 @@
 #define PCI_CLASS_COMMUNICATION_SERIAL   0x0700
 #define PCI_CLASS_COMMUNICATION_OTHER0x0780
 
+#define PCI_CLASS_INPUT_KEYBOARD 0x0900
+#define PCI_CLASS_INPUT_PEN  0x0901
+#define PCI_CLASS_INPUT_MOUSE0x0902
+#define PCI_CLASS_INPUT_SCANNER  0x0903
+#define PCI_CLASS_INPUT_GAMEPORT 0x0904
+#define PCI_CLASS_INPUT_OTHER0x0980
+
 #define PCI_CLASS_PROCESSOR_CO   0x0b40
 #define PCI_CLASS_PROCESSOR_POWERPC  0x0b20
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v11 4/4] hw/arm/virt: change indentation in a15memmap

[Alex Bennée]

Eric Auger  writes:

> Re-indent in a15memmap after VIRT_PLATFORM_BUS introduction
>
> Signed-off-by: Eric Auger 

Reviewed-by: Alex Bennée 

> ---
>  hw/arm/virt.c | 26 +-
>  1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index c3f4c54..e22a0fa 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -128,26 +128,26 @@ typedef struct {
>   */
>  static const MemMapEntry a15memmap[] = {
>  /* Space up to 0x800 is reserved for a boot ROM */
> -[VIRT_FLASH] =  {  0, 0x0800 },
> -[VIRT_CPUPERIPHS] = { 0x0800, 0x0002 },
> +[VIRT_FLASH] =  {  0, 0x0800 },
> +[VIRT_CPUPERIPHS] = { 0x0800, 0x0002 },
>  /* GIC distributor and CPU interfaces sit inside the CPU peripheral 
> space */
> -[VIRT_GIC_DIST] =   { 0x0800, 0x0001 },
> -[VIRT_GIC_CPU] ={ 0x0801, 0x0001 },
> -[VIRT_UART] =   { 0x0900, 0x1000 },
> -[VIRT_RTC] ={ 0x0901, 0x1000 },
> -[VIRT_FW_CFG] = { 0x0902, 0x000a },
> +[VIRT_GIC_DIST] =   { 0x0800, 0x0001 },
> +[VIRT_GIC_CPU] ={ 0x0801, 0x0001 },
> +[VIRT_UART] =   { 0x0900, 0x1000 },
> +[VIRT_RTC] ={ 0x0901, 0x1000 },
> +[VIRT_FW_CFG] = { 0x0902, 0x000a },
>  [VIRT_PLATFORM_BUS] =   { 0x0940, 0x0040 },
> -[VIRT_MMIO] =   { 0x0a00, 0x0200 },
> +[VIRT_MMIO] =   { 0x0a00, 0x0200 },
>  /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size 
> */
>  /*
>   * PCIE verbose map:
>   *
> - * MMIO window  { 0x1000, 0x2eff },
> - * PIO window   { 0x3eff, 0x0001 },
> - * ECAM { 0x3f00, 0x0100 },
> + * MMIO window  { 0x1000, 0x2eff },
> + * PIO window   { 0x3eff, 0x0001 },
> + * ECAM { 0x3f00, 0x0100 },
>   */
> -[VIRT_PCIE] =   { 0x1000, 0x3000 },
> -[VIRT_MEM] ={ 0x4000, 30ULL * 1024 * 1024 * 1024 },
> +[VIRT_PCIE] =   { 0x1000, 0x3000 },
> +[VIRT_MEM] ={ 0x4000, 30ULL * 1024 * 1024 * 1024 },
>  };
>  
>  static const int a15irqmap[] = {

-- 
Alex Bennée

Re: [Qemu-devel] [PATCH v11 3/4] hw/arm/virt: add dynamic sysbus device support

[Alex Bennée]

Eric Auger  writes:

> Allows sysbus devices to be instantiated from command line by
> using -device option. Machvirt creates a platform bus at init.
> The dynamic sysbus devices are attached to this platform bus device.
>
> The platform bus device registers a machine init done notifier
> whose role will be to bind the dynamic sysbus devices. Indeed
> dynamic sysbus devices are created after machine init.
>
> machvirt also registers a notifier that will build the device
> tree nodes for the platform bus and its children dynamic sysbus
> devices.
>
> Signed-off-by: Alexander Graf 
> Signed-off-by: Eric Auger 

Reviewed-by: Alex Bennée 

>
> ---
> v8 -> v9:
> - PLATFORM_BUS_NUM_IRQS set to 32 instead of 20
> - platform bus irq now start at 64 instead of 48
> - remove change of indentation in a15memmap
> - correct misc style issues
>
> v7 -> v8:
> - rebase on 2.2.0
> - in machvirt_init, create_platform_bus simply is added
>   after the arm_load_kernel call instead of moving this latter.
>   Related comment slighly reworded.
> - Due to those changes I dropped Alex and Shannon's Reviewed-by
>
> v6 -> v7:
> Take into account Shannon comments:
> - remove PLATFORM_BUS_FIRST_IRQ macro
> - correct platform bus size to 0x40
> - add an additional comment in a15irqmap related to
>   PLATFORM_BUS_NUM_IRQS
>
> v5 -> v6:
> - Take into account Peter's comments:
>   - platform_bus_params initialized from vbi->memmap and vbi->irqmap.
> As a consequence, const is removed. Also alignment in a15memmap
> is slightly changed.
>   - ARMPlatformBusSystemParams handle removed from create_platform_bus
> prototype
> - arm_load_kernel has become a machine init done notifier registration.
>   It must be called before platform_bus_create to guarantee the correct
>   notifier execution sequence
>
> v4 -> v5:
> - platform_bus_params becomes static const
> - reword comment in create_platform_bus
> - reword the commit message
>
> v3 -> v4:
> - use platform bus object, instantiated in create_platform_bus
> - device tree generation for platform bus and children dynamic
>   sysbus devices is no more handled at reset but in a
>   machine_init_done_notifier (due to the change in implementaion
>   of ARM load dtb using rom_add_blob_fixed).
> - device tree enhancement now takes into account the case of
>   user provided dtb. Before the user dtb was overwritten which
>   was wrong. However in case the dtb is provided by the user,
>   dynamic sysbus nodes are not added there.
> - renaming of MACHVIRT_PLATFORM defines
> - MACHVIRT_PLATFORM_PAGE_SHIFT and SIZE_PAGES not needed anymore,
>   hence removed.
> - DynSysbusParams struct renamed into ARMPlatformBusSystemParams
>   and above params removed.
> - separation of dt creation and QEMU binding is not mandated anymore
>   since the device tree is not created from scratch anymore. Instead
>   the modify_dtb function is used.
> - create_platform_bus registers another machine init done notifier
>   to start VFIO IRQ handling. This latter executes after the
>   dynamic sysbus device binding.
>
> v2 -> v3:
> - renaming of arm_platform_bus_create_devtree and arm_load_dtb
> - add copyright in hw/arm/dyn_sysbus_devtree.c
>
> v1 -> v2:
> - remove useless vfio-platform.h include file
> - s/MACHVIRT_PLATFORM_HOLE/MACHVIRT_PLATFORM_SIZE
> - use dyn_sysbus_binding and dyn_sysbus_devtree
> - dynamic sysbus platform buse size shrinked to 4MB and
>   moved between RTC and MMIO
>
> v1:
>
> Inspired from what Alex Graf did in ppc e500
> https://lists.gnu.org/archive/html/qemu-ppc/2014-07/msg00012.html
>
> Conflicts:
>   hw/arm/sysbus-fdt.c
>
> Conflicts:
>   hw/arm/virt.c
>
> Conflicts:
>   hw/arm/virt.c
> ---
>  hw/arm/virt.c | 59 
> +++
>  1 file changed, 59 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 69f51ac..c3f4c54 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -43,6 +43,8 @@
>  #include "qemu/bitops.h"
>  #include "qemu/error-report.h"
>  #include "hw/pci-host/gpex.h"
> +#include "hw/arm/sysbus-fdt.h"
> +#include "hw/platform-bus.h"
>  
>  #define NUM_VIRTIO_TRANSPORTS 32
>  
> @@ -60,6 +62,8 @@
>  #define GIC_FDT_IRQ_PPI_CPU_START 8
>  #define GIC_FDT_IRQ_PPI_CPU_WIDTH 8
>  
> +#define PLATFORM_BUS_NUM_IRQS 32
> +
>  enum {
>  VIRT_FLASH,
>  VIRT_MEM,
> @@ -71,8 +75,11 @@ enum {
>  VIRT_RTC,
>  VIRT_FW_CFG,
>  VIRT_PCIE,
> +VIRT_PLATFORM_BUS,
>  };
>  
> +static ARMPlatformBusSystemParams platform_bus_params;
> +
>  typedef struct MemMapEntry {
>  hwaddr base;
>  hwaddr size;
> @@ -129,6 +136,7 @@ static const MemMapEntry a15memmap[] = {
>  [VIRT_UART] =   { 0x0900, 0x1000 },
>  [VIRT_RTC] ={ 0x0901, 0x1000 },
>  [VIRT_FW_CFG] = { 0x0902, 0x000a },
> +[VIRT_PLATFORM_BUS] =   { 0x0940, 0x0040 },
>  [VIRT_MMIO] =   { 0x0a00, 0x0200 },
>  /* ...repeating for a total of NUM_VIRTIO_TRAN

Re: [Qemu-devel] [PATCH v5 20/45] Modify savevm handlers for postcopy

[Dr. David Alan Gilbert]

* David Gibson (da...@gibson.dropbear.id.au) wrote:
> On Wed, Mar 25, 2015 at 04:40:11PM +, Dr. David Alan Gilbert wrote:
> > * Dr. David Alan Gilbert (dgilb...@redhat.com) wrote:
> > > * David Gibson (da...@gibson.dropbear.id.au) wrote:
> > > > On Tue, Mar 24, 2015 at 08:04:14PM +, Dr. David Alan Gilbert wrote:
> > > > > * David Gibson (da...@gibson.dropbear.id.au) wrote:
> > > > > > On Fri, Mar 20, 2015 at 12:37:59PM +, Dr. David Alan Gilbert 
> > > > > > wrote:
> > > > > > > * David Gibson (da...@gibson.dropbear.id.au) wrote:
> > > > > > > > On Fri, Mar 13, 2015 at 10:19:54AM +, Dr. David Alan 
> > > > > > > > Gilbert wrote:
> > > > > > > > > * David Gibson (da...@gibson.dropbear.id.au) wrote:
> > > > > > > > > > On Wed, Feb 25, 2015 at 04:51:43PM +, Dr. David Alan 
> > > > > > > > > > Gilbert (git) wrote:
> > > > > > > > > > > From: "Dr. David Alan Gilbert" 
> > > > > > > > > > > 
> > > > > > > > > > > Modify save_live_pending to return separate postcopiable 
> > > > > > > > > > > and
> > > > > > > > > > > non-postcopiable counts.
> > > > > > > > > > > 
> > > > > > > > > > > Add 'can_postcopy' to allow a device to state if it can 
> > > > > > > > > > > postcopy
> > > > > > > > > > 
> > > > > > > > > > What's the purpose of the can_postcopy callback?  There are 
> > > > > > > > > > no callers
> > > > > > > > > > in this patch - is it still necessary with the change to
> > > > > > > > > > save_live_pending?
> > > > > > > > > 
> > > > > > > > > The patch 'qemu_savevm_state_complete: Postcopy changes' uses
> > > > > > > > > it in qemu_savevm_state_postcopy_complete and 
> > > > > > > > > qemu_savevm_state_complete
> > > > > > > > > to decide which devices must be completed at that point.
> > > > > > > > 
> > > > > > > > Couldn't they check for non-zero postcopiable state from
> > > > > > > > save_live_pending instead?
> > > > > > > 
> > > > > > > That would be a bit weird.
> > > > > > > 
> > > > > > > At the moment for each device we call the:
> > > > > > >save_live_setup method (from qemu_savevm_state_begin)
> > > > > > > 
> > > > > > >0...multiple times we call:
> > > > > > >save_live_pending
> > > > > > >save_live_iterate
> > > > > > > 
> > > > > > >and then we always call
> > > > > > >save_live_complete
> > > > > > > 
> > > > > > > 
> > > > > > > To my mind we have to call save_live_complete for any device
> > > > > > > that we've called save_live_setup on (maybe it allocated something
> > > > > > > in _setup that it clears up in _complete).
> > > > > > > 
> > > > > > > save_live_pending could perfectly well return 0 remaining at the 
> > > > > > > end of
> > > > > > > the migrate for our device, and thus if we used that then we 
> > > > > > > wouldn't
> > > > > > > call save_live_complete.
> > > > > > 
> > > > > > Um.. I don't follow.  I was suggesting that at the precopy->postcopy
> > > > > > transition point you call save_live_complete for everything that
> > > > > > reports 0 post-copiable state.
> > > > > > 
> > > > > > 
> > > > > > Then again, a different approach would be to split the
> > > > > > save_live_complete hook into (possibly NULL) "complete precopy" and
> > > > > > "complete postcopy" hooks.  The core would ensure that every chunk 
> > > > > > of
> > > > > > state has both completion hooks called (unless NULL).  That might 
> > > > > > also
> > > > > > address my concerns about the no longer entirely accurate
> > > > > > save_live_complete function name.
> > > > > 
> > > > > OK, that one I prefer.  Are you OK with:
> > > > > qemu_savevm_state_complete_precopy
> > > > >calls -> save_live_complete_precopy
> > > > > 
> > > > > qemu_savevm_state_complete_postcopy
> > > > >calls -> save_live_complete_postcopy
> > > > > 
> > > > > ?
> > > > 
> > > > Sounds ok to me.  Fwiw, I was thinking that both the complete_precopy
> > > > and complete_postcopy hooks should always be called.  For a
> > > > non-postcopy migration, the postcopy hooks would just be called
> > > > immediately after the precopy hooks.
> > > 
> > > OK, I've made the change as described in my last mail; but I haven't 
> > > called
> > > the complete_postcopy hook in the precopy case.  If it was as simple as 
> > > making
> > > all devices use one or the other then it would work, however there are
> > > existing (precopy) assumptions about ordering of device state on the wire 
> > > that
> > > I want to be careful not to alter; for example RAM must come first is the 
> > > one
> > > I know.
> > 
> > Actually, I spoke too soon; testing this found a bad breakage.
> > 
> > the functions in savevm.c add the per-section headers, and then call the 
> > _complete
> > methods on the devices.  Those _complete methods can't elect to do nothing, 
> > because
> > a header has already been planted.
> 
> Hrm.. couldn't you move the test for presence of the hook earlier so
> you don't sent the header if the hook is NULL?

There's two tests that you have to make:

Re: [Qemu-devel] [Migration Bug? ] Occasionally, the content of VM's memory is inconsistent between Source and Destination of migration

[Michael S. Tsirkin]

On Thu, Mar 26, 2015 at 11:29:43AM +0100, Juan Quintela wrote:
> Wen Congyang  wrote:
> > On 03/25/2015 05:50 PM, Juan Quintela wrote:
> >> zhanghailiang  wrote:
> >>> Hi all,
> >>>
> >>> We found that, sometimes, the content of VM's memory is
> >>> inconsistent between Source side and Destination side
> >>> when we check it just after finishing migration but before VM continue to 
> >>> Run.
> >>>
> >>> We use a patch like bellow to find this issue, you can find it from affix,
> >>> and Steps to reprduce:
> >>>
> >>> (1) Compile QEMU:
> >>>  ./configure --target-list=x86_64-softmmu  --extra-ldflags="-lssl" && make
> >>>
> >>> (2) Command and output:
> >>> SRC: # x86_64-softmmu/qemu-system-x86_64 -enable-kvm -cpu
> >>> qemu64,-kvmclock -netdev tap,id=hn0-device
> >>> virtio-net-pci,id=net-pci0,netdev=hn0 -boot c -drive
> >>> file=/mnt/sdb/pure_IMG/sles/sles11_sp3.img,if=none,id=drive-virtio-disk0,cache=unsafe
> >>> -device
> >>> virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0
> >>> -vnc :7 -m 2048 -smp 2 -device piix3-usb-uhci -device usb-tablet
> >>> -monitor stdio
> >> 
> >> Could you try to reproduce:
> >> - without vhost
> >> - without virtio-net
> >> - cache=unsafe is going to give you trouble, but trouble should only
> >>   happen after migration of pages have finished.
> >
> > If I use ide disk, it doesn't happen.
> > Even if I use virtio-net with vhost=on, it still doesn't happen. I guess
> > it is because I migrate the guest when it is booting. The virtio net
> > device is not used in this case.
> 
> Kevin, Stefan, Michael, any great idea?
> 
> Thanks, Juan.

If this is during boot from disk, we can more or less
rule out virtio-net/vhost-net.

> >
> > Thanks
> > Wen Congyang
> >
> >> 
> >> What kind of load were you having when reproducing this issue?
> >> Just to confirm, you have been able to reproduce this without COLO
> >> patches, right?
> >> 
> >>> (qemu) migrate tcp:192.168.3.8:3004
> >>> before saving ram complete
> >>> ff703f6889ab8701e4e040872d079a28
> >>> md_host : after saving ram complete
> >>> ff703f6889ab8701e4e040872d079a28
> >>>
> >>> DST: # x86_64-softmmu/qemu-system-x86_64 -enable-kvm -cpu
> >>> qemu64,-kvmclock -netdev tap,id=hn0,vhost=on -device
> >>> virtio-net-pci,id=net-pci0,netdev=hn0 -boot c -drive
> >>> file=/mnt/sdb/pure_IMG/sles/sles11_sp3.img,if=none,id=drive-virtio-disk0,cache=unsafe
> >>> -device
> >>> virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0
> >>> -vnc :7 -m 2048 -smp 2 -device piix3-usb-uhci -device usb-tablet
> >>> -monitor stdio -incoming tcp:0:3004
> >>> (qemu) QEMU_VM_SECTION_END, after loading ram
> >>> 230e1e68ece9cd4e769630e1bcb5ddfb
> >>> md_host : after loading all vmstate
> >>> 230e1e68ece9cd4e769630e1bcb5ddfb
> >>> md_host : after cpu_synchronize_all_post_init
> >>> 230e1e68ece9cd4e769630e1bcb5ddfb
> >>>
> >>> This happens occasionally, and it is more easy to reproduce when
> >>> issue migration command during VM's startup time.
> >> 
> >> OK, a couple of things.  Memory don't have to be exactly identical.
> >> Virtio devices in particular do funny things on "post-load".  There
> >> aren't warantees for that as far as I know, we should end with an
> >> equivalent device state in memory.
> >> 
> >>> We have done further test and found that some pages has been
> >>> dirtied but its corresponding migration_bitmap is not set.
> >>> We can't figure out which modules of QEMU has missed setting bitmap
> >>> when dirty page of VM,
> >>> it is very difficult for us to trace all the actions of dirtying VM's 
> >>> pages.
> >> 
> >> This seems to point to a bug in one of the devices.
> >> 
> >>> Actually, the first time we found this problem was in the COLO FT
> >>> development, and it triggered some strange issues in
> >>> VM which all pointed to the issue of inconsistent of VM's
> >>> memory. (We have try to save all memory of VM to slave side every
> >>> time
> >>> when do checkpoint in COLO FT, and everything will be OK.)
> >>>
> >>> Is it OK for some pages that not transferred to destination when do
> >>> migration ? Or is it a bug?
> >> 
> >> Pages transferred should be the same, after device state transmission is
> >> when things could change.
> >> 
> >>> This issue has blocked our COLO development... :(
> >>>
> >>> Any help will be greatly appreciated!
> >> 
> >> Later, Juan.
> >> 

Re: [Qemu-devel] [PATCH v5 40/45] Postcopy; Handle userfault requests

[Dr. David Alan Gilbert]

* David Gibson (da...@gibson.dropbear.id.au) wrote:
> On Wed, Feb 25, 2015 at 04:52:03PM +, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > userfaultfd is a Linux syscall that gives an fd that receives a stream
> > of notifications of accesses to pages registered with it and allows
> > the program to acknowledge those stalls and tell the accessing
> > thread to carry on.
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  include/migration/migration.h |   4 +
> >  migration/postcopy-ram.c  | 217 
> > --
> >  trace-events  |  12 +++
> >  3 files changed, 223 insertions(+), 10 deletions(-)
> > 
> > diff --git a/include/migration/migration.h b/include/migration/migration.h
> > index 139bb1b..cec064f 100644
> > --- a/include/migration/migration.h
> > +++ b/include/migration/migration.h
> > @@ -86,11 +86,15 @@ struct MigrationIncomingState {
> >  
> >  PostcopyState postcopy_state;
> >  
> > +bool   have_fault_thread;
> >  QemuThread fault_thread;
> >  QemuSemaphore  fault_thread_sem;
> >  
> >  /* For the kernel to send us notifications */
> >  intuserfault_fd;
> > +/* To tell the fault_thread to quit */
> > +intuserfault_quit_fd;
> > +
> >  QEMUFile *return_path;
> >  QemuMutex  rp_mutex;/* We send replies from multiple threads */
> >  PostcopyPMIpostcopy_pmi;
> > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > index 86fa5a0..abc039e 100644
> > --- a/migration/postcopy-ram.c
> > +++ b/migration/postcopy-ram.c
> > @@ -47,6 +47,8 @@ struct PostcopyDiscardState {
> >   */
> >  #if defined(__linux__)
> >  
> > +#include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -264,7 +266,7 @@ void postcopy_pmi_dump(MigrationIncomingState *mis)
> >  void postcopy_hook_early_receive(MigrationIncomingState *mis,
> >   size_t bitmap_index)
> >  {
> > -if (mis->postcopy_state == POSTCOPY_INCOMING_ADVISE) {
> > +if (postcopy_state_get(mis) == POSTCOPY_INCOMING_ADVISE) {
> 
> It kind of looks like that's a fix which should be folded into an
> earlier patch.

Thanks; gone.

> 
> >  /*
> >   * If we're in precopy-advise mode we need to track received pages 
> > even
> >   * though we don't need to place pages atomically yet.
> > @@ -489,15 +491,40 @@ int postcopy_ram_incoming_init(MigrationIncomingState 
> > *mis, size_t ram_pages)
> >   */
> >  int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
> >  {
> > -/* TODO: Join the fault thread once we're sure it will exit */
> > -if (qemu_ram_foreach_block(cleanup_area, mis)) {
> > -return -1;
> > +trace_postcopy_ram_incoming_cleanup_entry();
> > +
> > +if (mis->have_fault_thread) {
> > +uint64_t tmp64;
> > +
> > +if (qemu_ram_foreach_block(cleanup_area, mis)) {
> > +return -1;
> > +}
> > +/*
> > + * Tell the fault_thread to exit, it's an eventfd that should
> > + * currently be at 0, we're going to inc it to 1
> > + */
> > +tmp64 = 1;
> > +if (write(mis->userfault_quit_fd, &tmp64, 8) == 8) {
> > +trace_postcopy_ram_incoming_cleanup_join();
> > +qemu_thread_join(&mis->fault_thread);
> > +} else {
> > +/* Not much we can do here, but may as well report it */
> > +perror("incing userfault_quit_fd");
> > +}
> > +trace_postcopy_ram_incoming_cleanup_closeuf();
> > +close(mis->userfault_fd);
> > +close(mis->userfault_quit_fd);
> > +mis->have_fault_thread = false;
> >  }
> >  
> > +postcopy_state_set(mis, POSTCOPY_INCOMING_END);
> > +migrate_send_rp_shut(mis, qemu_file_get_error(mis->file) != 0);
> > +
> >  if (mis->postcopy_tmp_page) {
> >  munmap(mis->postcopy_tmp_page, getpagesize());
> >  mis->postcopy_tmp_page = NULL;
> >  }
> > +trace_postcopy_ram_incoming_cleanup_exit();
> >  return 0;
> >  }
> >  
> > @@ -531,36 +558,206 @@ static int ram_block_enable_notify(const char 
> > *block_name, void *host_addr,
> >  }
> >  
> >  /*
> > + * Tell the kernel that we've now got some memory it previously asked for.
> > + */
> > +static int ack_userfault(MigrationIncomingState *mis, void *start, size_t 
> > len)
> > +{
> > +struct uffdio_range range_struct;
> > +
> > +range_struct.start = (uint64_t)(uintptr_t)start;
> > +range_struct.len = (uint64_t)len;
> > +
> > +errno = 0;
> > +if (ioctl(mis->userfault_fd, UFFDIO_WAKE, &range_struct)) {
> > +int e = errno;
> > +
> > +if (e == ENOENT) {
> > +/* Kernel said it wasn't waiting - one case where this can
> > + * happen is where two threads triggered the userfault
> > + * and we receive the page and ack it just after we received
>

Re: [Qemu-devel] [PULL for v2.3 00/01] seccomp branch queue

[Peter Maydell]

On 25 March 2015 at 10:26, Eduardo Otubo  wrote:
> The following changes since commit 054903a832b865eb5432d79b5c9d1e1ff31b58d7:
>
>   Update version for v2.3.0-rc1 release (2015-03-24 16:34:16 +)
>
> are available in the git repository at:
>
>   git://github.com/otubo/qemu.git tags/pull-seccomp-20150325
>
> for you to fetch changes up to 8e27fc200457e3f2473d0069263774d4ba17bd85:
>
>   seccomp: update libseccomp version and remove arch restriction (2015-03-25 
> 11:03:27 +0100)
>
> 
> seccomp: update libseccomp version and remove arch restriction
> 
> Eduardo Otubo (1):
>   seccomp: update libseccomp version and remove arch restriction
>
>  configure | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v11 1/4] hw/arm/sysbus-fdt: helpers for platform bus nodes addition

[Alex Bennée]

Eric Auger  writes:

> This new C module will be used by ARM machine files to generate
> platform bus node and their dynamic sysbus device tree nodes.
>
> Dynamic sysbus device node addition is done in a machine init
> done notifier. arm_register_platform_bus_fdt_creator does the
> registration of this latter and is supposed to be called by
> ARM machine files that support platform bus and their dynamic
> sysbus. Addition of dynamic sysbus nodes is done only if the
> user did not provide any dtb.
>
> Signed-off-by: Alexander Graf 
> Signed-off-by: Eric Auger 
> Reviewed-by: Shannon Zhao 
> Reviewed-by: Alexander Graf 

Reviewed-by: Alex Bennée 

>
> ---
> v9 -> v10:
> - add assert and exit in add_fdt_node
>
> v8 -> v9:
> - s/Fdt/FDT in struct type names
> - reorder fields in PlatformBusFdtNotifierParams and use DO_UPCAST
>   instead of container_of
> - use assert() when relevant (board model issue)
> - g_free the ARMPlatformBusFDTParams and PlatformBusFDTNotifierParams
>   pointers in platform_bus_fdt_notify
>
> v7 -> v8:
> add Reviewed-by from Alex and Shannon
>
> v6 -> v7:
> - revert indentation in add_fdt_node_functions
>
> v5 -> v6:
> - add_all_platform_bus_fdt_nodes is not a modify_dtb function anymore
> - it now takes a handle to an ARMPlatformBusFdtParams.
> - fdt pointer is checked in case this notifier is executed after the
>   one that executes the load_dtb (this latter deallocates the fdt pointer)
> - check of fdt_filename moved in here.
> - upgrade_dtb is removed
> - copyright aligned between .h and .c
>
> v4 -> v5:
> - change indentation in add_fdt_node_functions. Also becomes a
>   static const.
> - ARMPlatformBusFdtParams.system_params becomes a pointer to
>   a const ARMPlatformBusSystemParams
> - removes platform-bus.h second inclusion
>
> v3 -> v4:
> - dyn_sysbus_devtree.c renamed into sysbus-fdt.c
> - use new PlatformBusDevice object
> - the dtb upgrade is done through modify_dtb. Before the fdt
>   was recreated from scratch. When the user provided a dtb this
>   latter was overwritten which was not correct.
> - an array contains the association between device type names
>   and their node creation function
> - I must aknowledge I did not find any cleaner way to implement
>   a FDT_BUILDER interface, as suggested by Paolo. The class method
>   would need to be initialized somewhere and since it cannot
>   happen in the device itself - according to Alex & Peter comments -,
>   I don't see when I shall associate the device type and its
>   interface implementation.
>
> v2 -> v3:
> - add arm_ prefix
> - arm_sysbus_device_create_devtree becomes static
>
> v1 -> v2:
> - Code moved in an arch specific file to accomodate architecture
>   dependent specificities.
> - remove platform_bus_base from PlatformDevtreeData
>
> v1: code originally written by Alex Graf in e500.c and reused for
> ARM [Eric Auger]
> ---
>  hw/arm/Makefile.objs|   1 +
>  hw/arm/sysbus-fdt.c | 174 
> 
>  include/hw/arm/sysbus-fdt.h |  60 +++
>  3 files changed, 235 insertions(+)
>  create mode 100644 hw/arm/sysbus-fdt.c
>  create mode 100644 include/hw/arm/sysbus-fdt.h
>
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 6088e53..0cc63e1 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -3,6 +3,7 @@ obj-$(CONFIG_DIGIC) += digic_boards.o
>  obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
>  obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
>  obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
> +obj-y += sysbus-fdt.o
>  
>  obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>  obj-$(CONFIG_DIGIC) += digic.o
> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
> new file mode 100644
> index 000..3038b94
> --- /dev/null
> +++ b/hw/arm/sysbus-fdt.c
> @@ -0,0 +1,174 @@
> +/*
> + * ARM Platform Bus device tree generation helpers
> + *
> + * Copyright (c) 2014 Linaro Limited
> + *
> + * Authors:
> + *  Alex Graf 
> + *  Eric Auger 
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program.  If not, see .
> + *
> + */
> +
> +#include "hw/arm/sysbus-fdt.h"
> +#include "qemu/error-report.h"
> +#include "sysemu/device_tree.h"
> +#include "hw/platform-bus.h"
> +#include "sysemu/sysemu.h"
> +
> +/*
> + * internal struct that contains the information to create dynamic
> + * sysbus device node
> + */
> +typedef

Re: [Qemu-devel] [Qemu-block] [RFC PATCH COLO v2 01/13] docs: block replication's description

[Eric Blake]

On 03/26/2015 04:28 AM, Gonglei wrote:

 Grammar review only (I'll leave the technical review to others)


>> +Copyright Fujitsu, Corp. 2015
>> +Copyright (c) 2015 Intel Corporation
>> +Copyright (c) 2015 HUAWEI TECHNOLOGIES CO.,LTD.

 Space after comma in English writing.
>> Yes, but I am not sure I can change it. HUAWEI always use this way.

Copyright lines are one thing that I am reluctant to change if it is not
my own line (some companies have policies on how their lines must look,
and I'm not in a position to argue the policy as I am not a lawyer).

>> You can find it in bootdevice.c

Such existing precedence is a strong argument to NOT changing it, at
least for a patch author that is not the copyright owner.

> 
> Good catch, Eric is right. I will change all of this writing way in Qemu at 
> 2.4.

So, sounds like such a change would be a separate patch (probably
through the -trivial tree), and cover all files in the repo in one go,
without affecting this series.  Such a patch by a copyright owner would
have no problem being accepted, if it is wanted.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-block] [RFC PATCH COLO v2 01/13] docs: block replication's description

[Gonglei]

On 2015/3/26 20:30, Eric Blake wrote:
> On 03/26/2015 04:28 AM, Gonglei wrote:
> 
> Grammar review only (I'll leave the technical review to others)
>
> 
>>> +Copyright Fujitsu, Corp. 2015
>>> +Copyright (c) 2015 Intel Corporation
>>> +Copyright (c) 2015 HUAWEI TECHNOLOGIES CO.,LTD.
>
> Space after comma in English writing.
>>> Yes, but I am not sure I can change it. HUAWEI always use this way.
> 
> Copyright lines are one thing that I am reluctant to change if it is not
> my own line (some companies have policies on how their lines must look,
> and I'm not in a position to argue the policy as I am not a lawyer).
> 
>>> You can find it in bootdevice.c
> 
> Such existing precedence is a strong argument to NOT changing it, at
> least for a patch author that is not the copyright owner.
> 
>>
>> Good catch, Eric is right. I will change all of this writing way in Qemu at 
>> 2.4.
> 
> So, sounds like such a change would be a separate patch (probably
> through the -trivial tree), and cover all files in the repo in one go,
> without affecting this series.  Such a patch by a copyright owner would
> have no problem being accepted, if it is wanted.
> 
Okay, will do, if it is not too later for rc2. :)

Thanks,
-Gonglei

Re: [Qemu-devel] [PATCH] configure: Add support for tcmalloc

[Kevin Wolf]

Am 26.03.2015 um 04:03 hat Fam Zheng geschrieben:
> This adds "--enable-tcmalloc" and "--disable-tcmalloc" to allow linking
> to libtcmalloc from gperftools.
> 
> tcmalloc is a malloc implementation that works well with threads and is
> fast, so it is good for performance.
> 
> It is disabled by default, because the MALLOC_PERTURB_ flag we use in
> tests doesn't work with tcmalloc. However we can enable tcmalloc
> specific heap checker and profilers later.
> 
> An IOPS gain can be observed with virtio-blk-dataplane, other parts of
> QEMU will directly benefit from it as well:
> 
> ==
>glibc malloc
> --
> rw bs iodepthbw iops   latency
> read   4k 1  15038511  24
> --
> 
> ==
>  tcmalloc
> --
> rw bs iodepthbw iops   latency
> read   4k 1  15639969  23
> --
> 
> Signed-off-by: Fam Zheng 
> ---
>  configure | 24 
>  1 file changed, 24 insertions(+)
> 
> diff --git a/configure b/configure
> index 589798e..03ba305 100755
> --- a/configure
> +++ b/configure
> @@ -336,6 +336,7 @@ libssh2=""
>  vhdx=""
>  quorum=""
>  numa=""
> +tcmalloc="no"
>  
>  # parse CC options first
>  for opt do
> @@ -1134,6 +1135,10 @@ for opt do
>;;
>--enable-numa) numa="yes"
>;;
> +  --disable-tcmalloc) tcmalloc="no"
> +  ;;
> +  --enable-tcmalloc) tcmalloc="yes"
> +  ;;
>*)
>echo "ERROR: unknown option $opt"
>echo "Try '$0 --help' for more information"
> @@ -1407,6 +1412,8 @@ Advanced options (experts only):
>--enable-quorum  enable quorum block filter support
>--disable-numa   disable libnuma support
>--enable-numaenable libnuma support
> +  --disable-tcmalloc   disable tcmalloc support
> +  --enable-numaenable tcmalloc support

Almost. :-)

Kevin

[Qemu-devel] [PATCH] misc: fix typos in copyright declaration

[arei.gonglei]

From: Gonglei 

Add a space after comma.

Signed-off-by: Gonglei 
---
 bootdevice.c  | 2 +-
 tests/usb-hcd-ohci-test.c | 2 +-
 tests/usb-hcd-uhci-test.c | 2 +-
 tests/usb-hcd-xhci-test.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/bootdevice.c b/bootdevice.c
index 673bfef..3cdc0d7 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -1,7 +1,7 @@
 /*
  * QEMU Boot Device Implement
  *
- * Copyright (c) 2014 HUAWEI TECHNOLOGIES CO.,LTD.
+ * Copyright (c) 2014 HUAWEI TECHNOLOGIES CO., LTD.
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to 
deal
diff --git a/tests/usb-hcd-ohci-test.c b/tests/usb-hcd-ohci-test.c
index 1160bde..fa592d4 100644
--- a/tests/usb-hcd-ohci-test.c
+++ b/tests/usb-hcd-ohci-test.c
@@ -1,7 +1,7 @@
 /*
  * QTest testcase for USB OHCI controller
  *
- * Copyright (c) 2014 HUAWEI TECHNOLOGIES CO.,LTD.
+ * Copyright (c) 2014 HUAWEI TECHNOLOGIES CO., LTD.
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
diff --git a/tests/usb-hcd-uhci-test.c b/tests/usb-hcd-uhci-test.c
index 6d631cf..a96b716 100644
--- a/tests/usb-hcd-uhci-test.c
+++ b/tests/usb-hcd-uhci-test.c
@@ -1,7 +1,7 @@
 /*
  * QTest testcase for USB UHCI controller
  *
- * Copyright (c) 2014 HUAWEI TECHNOLOGIES CO.,LTD.
+ * Copyright (c) 2014 HUAWEI TECHNOLOGIES CO., LTD.
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
diff --git a/tests/usb-hcd-xhci-test.c b/tests/usb-hcd-xhci-test.c
index adf261e..56ab367 100644
--- a/tests/usb-hcd-xhci-test.c
+++ b/tests/usb-hcd-xhci-test.c
@@ -1,7 +1,7 @@
 /*
  * QTest testcase for USB xHCI controller
  *
- * Copyright (c) 2014 HUAWEI TECHNOLOGIES CO.,LTD.
+ * Copyright (c) 2014 HUAWEI TECHNOLOGIES CO., LTD.
  *
  * This work is licensed under the terms of the GNU GPL, version 2 or later.
  * See the COPYING file in the top-level directory.
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH] misc: fix typos in copyright declaration

[Eric Blake]

On 03/26/2015 06:57 AM, arei.gong...@huawei.com wrote:
> From: Gonglei 
> 
> Add a space after comma.
> 
> Signed-off-by: Gonglei 
> ---
>  bootdevice.c  | 2 +-
>  tests/usb-hcd-ohci-test.c | 2 +-
>  tests/usb-hcd-uhci-test.c | 2 +-
>  tests/usb-hcd-xhci-test.c | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Eric Blake 

Safe for 2.3 if desired, as it has no impact on code.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] configure: Add support for tcmalloc

[Paolo Bonzini]

On 26/03/2015 13:53, Kevin Wolf wrote:
> Am 26.03.2015 um 04:03 hat Fam Zheng geschrieben:
>> This adds "--enable-tcmalloc" and "--disable-tcmalloc" to allow linking
>> to libtcmalloc from gperftools.
>>
>> tcmalloc is a malloc implementation that works well with threads and is
>> fast, so it is good for performance.
>>
>> It is disabled by default, because the MALLOC_PERTURB_ flag we use in
>> tests doesn't work with tcmalloc. However we can enable tcmalloc
>> specific heap checker and profilers later.
>>
>> An IOPS gain can be observed with virtio-blk-dataplane, other parts of
>> QEMU will directly benefit from it as well:
>>
>> ==
>>glibc malloc
>> --
>> rw bs iodepthbw iops   latency
>> read   4k 1  15038511  24
>> --
>>
>> ==
>>  tcmalloc
>> --
>> rw bs iodepthbw iops   latency
>> read   4k 1  15639969  23
>> --
>>
>> Signed-off-by: Fam Zheng 
>> ---
>>  configure | 24 
>>  1 file changed, 24 insertions(+)
>>
>> diff --git a/configure b/configure
>> index 589798e..03ba305 100755
>> --- a/configure
>> +++ b/configure
>> @@ -336,6 +336,7 @@ libssh2=""
>>  vhdx=""
>>  quorum=""
>>  numa=""
>> +tcmalloc="no"
>>  
>>  # parse CC options first
>>  for opt do
>> @@ -1134,6 +1135,10 @@ for opt do
>>;;
>>--enable-numa) numa="yes"
>>;;
>> +  --disable-tcmalloc) tcmalloc="no"
>> +  ;;
>> +  --enable-tcmalloc) tcmalloc="yes"
>> +  ;;
>>*)
>>echo "ERROR: unknown option $opt"
>>echo "Try '$0 --help' for more information"
>> @@ -1407,6 +1412,8 @@ Advanced options (experts only):
>>--enable-quorum  enable quorum block filter support
>>--disable-numa   disable libnuma support
>>--enable-numaenable libnuma support
>> +  --disable-tcmalloc   disable tcmalloc support
>> +  --enable-numaenable tcmalloc support
> 
> Almost. :-)

Will fix and apply for 2.4.

Paolo

Re: [Qemu-devel] [PATCH v5 06/28] qapi: Add some union tests

[Markus Armbruster]

Eric Blake  writes:

> Demonstrate that the qapi generator doesn't deal well with unions
> that aren't up to par. Later patches will update the expected
> reseults as the generator is made stricter.
>
> Of particular note, we currently allow 'base' without 'discriminator'
> as a way to create a simple union with a base class.  However, none
> of the existing QMP or QGA interfaces use it (it is exercised only
> in the testsuite).

qapi-code-gen.txt documents 'base' only for flat unions.  We should
either document its use in simple unions, or outlaw it.

> Meanwhile, it would be nice to allow
> 'discriminator':'EnumType' without 'base' for creating a simple union
> that is type-safe rather than open-coded to a generated enum (right
> now, we are only type-safe when using a flat union, but that uses
> a different syntax of 'discriminator':'member-name' which requires
> a base class containing a 'member-name' enum field).

I'm afraid I don't get you.  Can you give examples?

>   If both 'base'
> and 'discriminator' are optional, then converting a simple union
> with base class to a type-safe simple union with an enum discriminator
> would not be possible.  So my plan is to get rid of 'base' without
> 'discriminator' later in the series;

Aha: you're going to outlaw 'base' in simple unions.  Yes, please.

>  this will be no real loss, as any
> union that needs additional common fields can always use a flat
> union.

The mathematical concept behind unions is the sum type.

We have three implementations, and we call them simple, flat, anonymous.

Anonymous unions are implicitly tagged.  They come with the obvious
restrictions required to make implicit tagging work.

The other two are explicitly tagged.  The difference between them is
just notation.  I like my unions flat, because for me the extra wrapping
is just notational overhead.

In particular, I can define simple unions in terms of flat ones by
restricting all union cases to a single member named 'data'.  They're
not implemented that way, but that's a historical accident.  Simple
unions are a redundant concept.

This proves your "can always use a flat union" proposition.  The only
way they can earn their keep is making the schema easier to read.  I
haven't thought about that.

Another historical accident is how we express members common to all
union cases: base type.  Probably just because complex types already had
them.  Are you planning to change anything there?

[...]
> diff --git a/tests/qapi-schema/alternate-nested.err 
> b/tests/qapi-schema/alternate-nested.err
> new file mode 100644
> index 000..e69de29
> diff --git a/tests/qapi-schema/alternate-nested.exit 
> b/tests/qapi-schema/alternate-nested.exit
> new file mode 100644
> index 000..573541a
> --- /dev/null
> +++ b/tests/qapi-schema/alternate-nested.exit
> @@ -0,0 +1 @@
> +0
> diff --git a/tests/qapi-schema/alternate-nested.json 
> b/tests/qapi-schema/alternate-nested.json
> new file mode 100644
> index 000..d5812bf
> --- /dev/null
> +++ b/tests/qapi-schema/alternate-nested.json
> @@ -0,0 +1,7 @@
> +# FIXME: we should reject a nested anonymous union branch

Same reason we want to reject nested / anonymous complex types
elsewhere?  Or is there a deeper reason?

> +{ 'union': 'Union1',
> +  'discriminator': {},
> +  'data': { 'name': 'str', 'value': 'int' } }
> +{ 'union': 'Union2',
> +  'discriminator': {},
> +  'data': { 'nested': 'Union1' } }
> diff --git a/tests/qapi-schema/alternate-nested.out 
> b/tests/qapi-schema/alternate-nested.out
> new file mode 100644
> index 000..0137c1f
> --- /dev/null
> +++ b/tests/qapi-schema/alternate-nested.out
> @@ -0,0 +1,5 @@
> +[OrderedDict([('union', 'Union1'), ('discriminator', OrderedDict()), 
> ('data', OrderedDict([('name', 'str'), ('value', 'int')]))]),
> + OrderedDict([('union', 'Union2'), ('discriminator', OrderedDict()), 
> ('data', OrderedDict([('nested', 'Union1')]))])]
> +[{'enum_name': 'Union1Kind', 'enum_values': None},
> + {'enum_name': 'Union2Kind', 'enum_values': None}]
> +[]
[...]
> diff --git a/tests/qapi-schema/flat-union-bad-base.err 
> b/tests/qapi-schema/flat-union-bad-base.err
> new file mode 100644
> index 000..5962ff4
> --- /dev/null
> +++ b/tests/qapi-schema/flat-union-bad-base.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/flat-union-bad-base.json:9: Base 'OrderedDict([('enum1', 
> 'TestEnum'), ('kind', 'str')])' is not a valid type
> diff --git a/tests/qapi-schema/flat-union-bad-base.exit 
> b/tests/qapi-schema/flat-union-bad-base.exit
> new file mode 100644
> index 000..d00491f
> --- /dev/null
> +++ b/tests/qapi-schema/flat-union-bad-base.exit
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/qapi-schema/flat-union-bad-base.json 
> b/tests/qapi-schema/flat-union-bad-base.json
> new file mode 100644
> index 000..d69168f
> --- /dev/null
> +++ b/tests/qapi-schema/flat-union-bad-base.json
> @@ -0,0 +1,13 @@
> +# we

Re: [Qemu-devel] Bouncing maintainers

[Kevin Wolf]

Am 25.03.2015 um 16:45 hat John Snow geschrieben:
> 
> 
> On 03/25/2015 10:48 AM, Markus Armbruster wrote:
> >I just had another cc: to an address gotten from MAINTAINERS bounce with
> >"user unknown".
> >
> >How do we weed out dead MAINTAINERS entries?
> >
> 
> Automated spambot that runs once a release cycle and reports back to
> the list which entries were dead?

I don't need more email. Why can't we just send a patch to MAINTAINERS
dropping the line the first time someone actually notices it?

Kevin

Re: [Qemu-devel] [PATCH] virtio-scsi-dataplane: fix memory leak for VirtIOSCSIVring

[Paolo Bonzini]

On 26/03/2015 08:42, Ting Wang wrote:
> VirtIOSCSIVring which allocated in virtio_scsi_vring_init
> should be free when dataplane has been stopped or failed to start.
> 
> Signed-off-by: Ting Wang 
> ---
>  hw/scsi/virtio-scsi-dataplane.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
> index c069cd7..5575648 100644
> --- a/hw/scsi/virtio-scsi-dataplane.c
> +++ b/hw/scsi/virtio-scsi-dataplane.c
> @@ -182,13 +182,19 @@ static void virtio_scsi_vring_teardown(VirtIOSCSI *s)
>  
>  if (s->ctrl_vring) {
>  vring_teardown(&s->ctrl_vring->vring, vdev, 0);
> +g_slice_free(VirtIOSCSIVring, s->ctrl_vring);
> +s->ctrl_vring = NULL;
>  }
>  if (s->event_vring) {
>  vring_teardown(&s->event_vring->vring, vdev, 1);
> +g_slice_free(VirtIOSCSIVring, s->event_vring);
> +s->event_vring = NULL;
>  }
>  if (s->cmd_vrings) {
>  for (i = 0; i < vs->conf.num_queues && s->cmd_vrings[i]; i++) {
>  vring_teardown(&s->cmd_vrings[i]->vring, vdev, 2 + i);
> +g_slice_free(VirtIOSCSIVring, s->cmd_vrings[i]);
> +s->cmd_vrings[i] = NULL;
>  }
>  free(s->cmd_vrings);
>  s->cmd_vrings = NULL;
> 

Applied for 2.3.0-rc2, thanks.

Paolo

Re: [Qemu-devel] [PATCH v5 06/28] qapi: Add some union tests

[Markus Armbruster]

One more:

[...]
> diff --git a/tests/qapi-schema/alternate-conflict-string.json 
> b/tests/qapi-schema/alternate-conflict-string.json
> new file mode 100644
> index 000..5fd1a47
> --- /dev/null
> +++ b/tests/qapi-schema/alternate-conflict-string.json
> @@ -0,0 +1,8 @@
> +# FIXME: we should reject anonymous unions with multiple string-like branches
> +{ 'enum': 'Enum',
> +  'data': [ 'hello', 'world' ] }
> +{ 'union': 'MyUnion',
> +  'discriminator': {},
> +  'data': { 'one': 'str',
> + 'two': 'Enum' } }
> +

/home/armbru/work/qemu/.git/rebase-apply/patch:325: new blank line at EOF.

[...]

Re: [Qemu-devel] [PULL 0/3] pc, virtio bugfixes for 2.3

[Peter Maydell]

On 25 March 2015 at 15:30, Michael S. Tsirkin  wrote:
> The following changes since commit 054903a832b865eb5432d79b5c9d1e1ff31b58d7:
>
>   Update version for v2.3.0-rc1 release (2015-03-24 16:34:16 +)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to 7e0e736ecdfeac6d3517513d3a702304e4f6cf59:
>
>   virtio-net: validate backend queue numbers against bus limitation 
> (2015-03-25 13:39:25 +0100)
>
> 
> pc, virtio bugfixes for 2.3
>
> Several bugfixes, nothing stands out especially.
>
> Signed-off-by: Michael S. Tsirkin 

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH 1/2] object: Add can_be_deleted callback to TypeInfo and TypeImpl

[Paolo Bonzini]

On 26/03/2015 11:29, Igor Mammedov wrote:
> What value of ref, one would use to decide if deletion is possible?
> 
> In generic case object can have ref > 1 but still be eligible for deleting
> via object-del.

Right, for example devices are unparented with ref > 1.

Still, it could be a sane default implementation.  It doesn't even need
a function pointer until someone comes up with an object that has
different needs.

Paolo

Re: [Qemu-devel] [PULL for v2.3 01/01] seccomp: update libseccomp version and remove arch restriction

[Juan Quintela]

Eduardo Otubo  wrote:
> Libseccomp version updated to 2.2.0 and arch restriction to x86/x86_64
> is now removed. It's supposed to work on armv7l as well.
>
> Related bug: https://bugs.launchpad.net/qemu/+bug/1363641
>
> Signed-off-by: Eduardo Otubo 

This prevent compliation on x86_64 on Fedora 21.

(migration/next)$ make -j5 -C /scratch/tmp/next/all/
make: Entering directory '/scratch/tmp/next/all'
config-host.mak is out-of-date, running configure

ERROR: User requested feature libseccomp
   configure was not able to find it.
   Install libseccomp devel >= 2.2.0

Makefile:30: recipe for target 'config-host.mak' failed
make: *** [config-host.mak] Error 1
make: Leaving directory '/scratch/tmp/next/all'
(migration/next)$ rpm -qa | grep seccomp
libseccomp-2.1.1-5.fc21.x86_64
libseccomp-devel-2.1.1-5.fc21.x86_64
libseccomp-debuginfo-2.1.1-5.fc21.x86_64
(migration/next)$ 

This was compiling correctly until this patch got in.  And virt-test
uses seccomp by default.

Fedora 21 is less than 3 months old.  Do we really want to avoid
compilation there?

Thanks, Juan.


> ---
>  configure | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/configure b/configure
> index 589798e..cbe6495 100755
> --- a/configure
> +++ b/configure
> @@ -1848,14 +1848,13 @@ fi
>  # libseccomp check
>  
>  if test "$seccomp" != "no" ; then
> -if test "$cpu" = "i386" || test "$cpu" = "x86_64" &&
> -$pkg_config --atleast-version=2.1.1 libseccomp; then
> +if $pkg_config --atleast-version=2.2.0 libseccomp; then
>  libs_softmmu="$libs_softmmu `$pkg_config --libs libseccomp`"
>  QEMU_CFLAGS="$QEMU_CFLAGS `$pkg_config --cflags libseccomp`"
>   seccomp="yes"
>  else
>   if test "$seccomp" = "yes"; then
> -feature_not_found "libseccomp" "Install libseccomp devel >= 
> 2.1.1"
> +feature_not_found "libseccomp" "Install libseccomp devel >= 
> 2.2.0"
>   fi
>   seccomp="no"
>  fi

Re: [Qemu-devel] [PATCH v5 07/28] qapi: Simplify tests of simple unions

[Markus Armbruster]

Eric Blake  writes:

> None of the existing QMP or QGA interfaces uses a union with a
> base type but no discriminator; it is easier to avoid this in
> the generator to save room for other future extensions more likely
> to be useful (the previous commit added the test
> union-base-no-discriminator to ensure that we eventually give an
> error message).  Meanwhile, the tests of UserDefNativeListUnion
> serve to validate code generation of simple unions, except that it
> did not have full coverage in the strict test.
>
> Fix some indentation and long lines while at it.
>
> Signed-off-by: Eric Blake 

I figure I'd split this commit into two or three parts

* kill the undocumented "base in simple union" feature

* add the missing tests for UserDefNativeListUnion

* style cleanup, possibly squashed into previous

Your choice.

[...]
> diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
> index d5360c6..53134a1 100644
> --- a/tests/test-qmp-input-strict.c
> +++ b/tests/test-qmp-input-strict.c
[...]
> @@ -232,18 +232,19 @@ static void 
> test_validate_fail_list(TestInputVisitorData *data,
>  qapi_free_UserDefOneList(head);
>  }
>
> -static void test_validate_fail_union(TestInputVisitorData *data,
> -  const void *unused)
> +static void test_validate_fail_union_native_list(TestInputVisitorData *data,
> + const void *unused)
>  {
> -UserDefUnion *tmp = NULL;
> +UserDefNativeListUnion *tmp = NULL;
>  Error *err = NULL;
>  Visitor *v;
>
> -v = validate_test_init(data, "{ 'type': 'b', 'data' : { 'integer': 42 } 
> }");
> +v = validate_test_init(data,
> +   "{ 'type': 'integer', 'data' : [ \"string\" ] }");

Everywhere else we use ' instead of \".  Not worth a respin on its own,
clean up on top then.

>
> -visit_type_UserDefUnion(v, &tmp, NULL, &err);
> +visit_type_UserDefNativeListUnion(v, &tmp, NULL, &err);
>  g_assert(err);
> -qapi_free_UserDefUnion(tmp);
> +qapi_free_UserDefNativeListUnion(tmp);
>  }
>
>  static void test_validate_fail_union_flat(TestInputVisitorData *data,
[...]

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [PATCH v5 06/28] qapi: Add some union tests

[Eric Blake]

On 03/26/2015 07:23 AM, Markus Armbruster wrote:
> One more:
> 
> [...]
>> diff --git a/tests/qapi-schema/alternate-conflict-string.json 
>> b/tests/qapi-schema/alternate-conflict-string.json
>> new file mode 100644
>> index 000..5fd1a47
>> --- /dev/null
>> +++ b/tests/qapi-schema/alternate-conflict-string.json
>> @@ -0,0 +1,8 @@
>> +# FIXME: we should reject anonymous unions with multiple string-like 
>> branches
>> +{ 'enum': 'Enum',
>> +  'data': [ 'hello', 'world' ] }
>> +{ 'union': 'MyUnion',
>> +  'discriminator': {},
>> +  'data': { 'one': 'str',
>> +'two': 'Enum' } }
>> +
> 
> /home/armbru/work/qemu/.git/rebase-apply/patch:325: new blank line at EOF.

Huh. I thought I had git set up to reject me from making commits like
that locally, but obviously not.
http://wiki.qemu.org/Contribute/SubmitAPatch should probably mention the
magic one-time setup to use to turn this type of checking on...

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC PATCH COLO v2 08/13] Allow creating backup jobs when opening BDS

[Paolo Bonzini]

On 26/03/2015 08:23, Wen Congyang wrote:
>>> >> Hmm, we need to use backup API in block.c, and block.o will
>>> >> be used by qemu-img which doesn't use common-obj.
>> > 
>> > I see. How about adding the referenced functions to stubs/?
> Good idea. I will try it.

Even better would be to move the functions that need backup APIs to
another file.  block.c is big, and it is going to be split soon into
multiple files.

Paolo

Re: [Qemu-devel] [PULL for v2.3 01/01] seccomp: update libseccomp version and remove arch restriction

[Juan Quintela]

Juan Quintela  wrote:
> Eduardo Otubo  wrote:
>> Libseccomp version updated to 2.2.0 and arch restriction to x86/x86_64
>> is now removed. It's supposed to work on armv7l as well.
>>
>> Related bug: https://bugs.launchpad.net/qemu/+bug/1363641
>>
>> Signed-off-by: Eduardo Otubo 


Just in case anyone is interested, default virt-test output with current
qemu


15:05:01 ERROR| Qemu output:
15:05:01 ERROR| qemu-system-x86_64: -sandbox on: sandboxing request but seccomp 
is not compiled into this build
15:05:01 ERROR| 
15:05:01 ERROR| 
15:05:01 ERROR| FAIL type_specific.io-github-autotest-qemu.migrate.default.tcp 
-> VMStartError: VM 'virt-tests-vm1' failed to start: Qemu is defunct.
Qemu output:
qemu-system-x86_64: -sandbox on: sandboxing request but seccomp is not compiled 
into this build



Notice that you can "fix" the problem running with --qemu_sandbox=off,
but breaking the main test suite just before release looks wrong?

Later, Juan.

>
> This prevent compliation on x86_64 on Fedora 21.
>
> (migration/next)$ make -j5 -C /scratch/tmp/next/all/
> make: Entering directory '/scratch/tmp/next/all'
> config-host.mak is out-of-date, running configure
>
> ERROR: User requested feature libseccomp
>configure was not able to find it.
>Install libseccomp devel >= 2.2.0
>
> Makefile:30: recipe for target 'config-host.mak' failed
> make: *** [config-host.mak] Error 1
> make: Leaving directory '/scratch/tmp/next/all'
> (migration/next)$ rpm -qa | grep seccomp
> libseccomp-2.1.1-5.fc21.x86_64
> libseccomp-devel-2.1.1-5.fc21.x86_64
> libseccomp-debuginfo-2.1.1-5.fc21.x86_64
> (migration/next)$ 
>
> This was compiling correctly until this patch got in.  And virt-test
> uses seccomp by default.
>
> Fedora 21 is less than 3 months old.  Do we really want to avoid
> compilation there?
>
> Thanks, Juan.
>
>
>> ---
>>  configure | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 589798e..cbe6495 100755
>> --- a/configure
>> +++ b/configure
>> @@ -1848,14 +1848,13 @@ fi
>>  # libseccomp check
>>  
>>  if test "$seccomp" != "no" ; then
>> -if test "$cpu" = "i386" || test "$cpu" = "x86_64" &&
>> -$pkg_config --atleast-version=2.1.1 libseccomp; then
>> +if $pkg_config --atleast-version=2.2.0 libseccomp; then
>>  libs_softmmu="$libs_softmmu `$pkg_config --libs libseccomp`"
>>  QEMU_CFLAGS="$QEMU_CFLAGS `$pkg_config --cflags libseccomp`"
>>  seccomp="yes"
>>  else
>>  if test "$seccomp" = "yes"; then
>> -feature_not_found "libseccomp" "Install libseccomp devel >= 
>> 2.1.1"
>> +feature_not_found "libseccomp" "Install libseccomp devel >= 
>> 2.2.0"
>>  fi
>>  seccomp="no"
>>  fi

Re: [Qemu-devel] [PATCH 1/8] rc4030: create custom DMA address space

[Paolo Bonzini]

On 25/03/2015 20:10, Hervé Poussineau wrote:
> Le 25/03/2015 15:45, Paolo Bonzini a écrit :
>>
>>
>> On 05/03/2015 23:13, Hervé Poussineau wrote:
>>> Add a new memory region in system address space where DMA address space
>>> definition (the 'translation table') belongs, so we can update on the
>>> fly
>>> the DMA address space.
>>>
>>> Signed-off-by: Hervé Poussineau 
>>
>> Would it make sense to just use an IOMMU region for the DMA address
>> space?
> 
> I don't really know. Currently, the first user is the dp8393x network
> card (a sysbus device), which is connected through the DMA address space.
> It means that *every* read or write access needs to be translated, even
> those accessing 1 or 2 bytes. dp8393x has no direct connection to the
> system memory.
> On some other machines, like the m68k Quadra 800, the dp8393x is only
> connected to the system memory.
> 
> A second user is the ESP SCSI card in DMA mode, using the DMA address
> space with the help of the DMA controller.
> 
> As DMA address space mapping doesn't change much (except initialization
> phases), and that it is used for small (1 or 2 bytes) and big transfers
> (network packets and disk accesses), I thought that an address space was
> better.

Both are okay.  The IOMMU makes address space changes faster; your
scheme is basically a form of caching, it trades update performance for
improved translation performance.

The trick you're using with shadowing the RAM is nifty, but it leaks a
memory region.  I'll reply to the main patch.

Paolo

Re: [Qemu-devel] [PULL for v2.3 01/01] seccomp: update libseccomp version and remove arch restriction

[Peter Maydell]

On 26 March 2015 at 14:09, Juan Quintela  wrote:
> Notice that you can "fix" the problem running with --qemu_sandbox=off,
> but breaking the main test suite just before release looks wrong?

The main test suite is "make check" :-)

-- PMM

Re: [Qemu-devel] [PATCH 1/2] object: Add can_be_deleted callback to TypeInfo and TypeImpl

[Igor Mammedov]

On Thu, 26 Mar 2015 14:37:13 +0100
Paolo Bonzini  wrote:

> 
> 
> On 26/03/2015 11:29, Igor Mammedov wrote:
> > What value of ref, one would use to decide if deletion is possible?
> > 
> > In generic case object can have ref > 1 but still be eligible for deleting
> > via object-del.
> 
> Right, for example devices are unparented with ref > 1.
> 
> Still, it could be a sane default implementation.  It doesn't even need
> a function pointer until someone comes up with an object that has
> different needs.
Not in case of memory backend, its 'ref' in unused state is 2 since
it contains at least one child, backend object is owner of MemoryRegion.

> Paolo

Re: [Qemu-devel] [PATCH v5 08/28] qapi: Better error messages for bad unions

[Markus Armbruster]

Subject suggests you're just polishing error messages here.  In fact,
you're fixing the generator to detect errors.  Suggest something like

qapi: Tighten checking of unions

Eric Blake  writes:

> Previous commits demonstrated that the generator had several
> flaws with less-than-perfect unions:
> - make the use of a base without discriminator a hard error,
> since the previous patch removed all remaining uses of it
> - a simple union that listed the same branch twice (or two variant
> names that map to the same C enumerator, including the implicit
> MAX sentinel) ended up generating invalid C code
> - checking 'if discriminator' prior to 'if discriminator == {}'
> leads to dead code in python, and ended up processing anonymous
> unions as if they were simple unions
> - an anonymous union that listed two branches with the same qtype
> ended up generating invalid C code
> - the generator crashed on anonymous union attempts to use an
> array type
> - the generator was silently ignoring a base type for anonymous
> unions
> - the generator allowed unknown types or nested anonymous unions
> as a branch in an anonymous union
>
> Signed-off-by: Eric Blake 
> ---
[...]
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index f6fb930..2390887 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -181,17 +181,8 @@ const int %(name)s_qtypes[QTYPE_MAX] = {
>  name=name)
>
>  for key in members:
> -qapi_type = members[key]
> -if builtin_types.has_key(qapi_type):
> -qtype = builtin_types[qapi_type]
> -elif find_struct(qapi_type):
> -qtype = "QTYPE_QDICT"
> -elif find_union(qapi_type):
> -qtype = "QTYPE_QDICT"
> -elif find_enum(qapi_type):
> -qtype = "QTYPE_QSTRING"
> -else:
> -assert False, "Invalid anonymous union member"
> +qtype = find_anonymous_member_qtype(members[key])
> +assert qtype, "Invalid anonymous union member"
>
>  ret += mcgen('''
>  [ %(qtype)s ] = %(abbrev)s_KIND_%(enum)s,
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 3ce8c33..fc7b7f1 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -224,6 +224,23 @@ def find_base_fields(base):
>  return None
>  return base_struct_define['data']
>
> +# Return the qtype of an anonymous union branch, or None on error.
> +def find_anonymous_member_qtype(qapi_type):
> +if builtin_types.has_key(qapi_type):
> +return builtin_types[qapi_type]
> +elif find_struct(qapi_type):
> +return "QTYPE_QDICT"
> +elif find_enum(qapi_type):
> +return "QTYPE_QSTRING"
> +else:
> +union = find_union(qapi_type)
> +if union:
> +discriminator = union.get('discriminator')
> +if discriminator == {}:
> +return None
> +return "QTYPE_QDICT"
> +return None
> +
>  # Return the discriminator enum define if discriminator is specified as an
>  # enum type, otherwise return None.
>  def discriminator_find_enum_define(expr):
> @@ -258,24 +275,28 @@ def check_union(expr, expr_info):
>  base = expr.get('base')
>  discriminator = expr.get('discriminator')
>  members = expr['data']
> +values = { 'MAX': '(automatic)' }
> +types_seen = {}
>
> -# If the object has a member 'base', its value must name a complex type.
> -if base:
> -base_fields = find_base_fields(base)
> -if not base_fields:
> -raise QAPIExprError(expr_info,
> -"Base '%s' is not a valid type"
> -% base)
> +# Three types of unions, determined by discriminator.
>
> -# If the union object has no member 'discriminator', it's an
> -# ordinary union.
> -if not discriminator:
> +# If the value of member 'discriminator' is {}, it's an
> +# anonymous union, and must not have a base.
> +if discriminator == {}:
>  enum_define = None
> +if base:
> +raise QAPIExprError(expr_info,
> +"Anonymous union '%s' must not have a base"
> +% name)
>
> -# Else if the value of member 'discriminator' is {}, it's an
> -# anonymous union.
> -elif discriminator == {}:
> +# Else if the union object has no member 'discriminator', it's an
> +# ordinary union.  For now, it must not have a base.

Since you're touching this anyway, you could s/ordinary/simple/.

> +elif not discriminator:
>  enum_define = None
> +if base:
> +raise QAPIExprError(expr_info,
> +"Simple union '%s' must not have a base"
> +% name)
>
>  # Else, it's a flat union.
>  else:
> @@ -284,6 +305,12 @@ def check_union(expr, expr_info):
>  raise QAPIExprError(expr_info,
>  "Flat union '%s' must hav

Re: [Qemu-devel] [PATCH v5 09/28] qapi: Prepare for catching more semantic parse errors

[Markus Armbruster]

Eric Blake  writes:

> This patch widens the scope of a try block (with the attending
> reindentation required by Python) in preparation for a future
> patch adding more instances of QAPIExprError inside the block.
> It's easier to separate indentation from semantic changes, so
> this patch has no real behavior change.
>
> Signed-off-by: Eric Blake 

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [PATCH 1/8] rc4030: create custom DMA address space

[Paolo Bonzini]

On 05/03/2015 23:13, Hervé Poussineau wrote:
> +static const MemoryRegionOps rc4030_dma_tt_ops = {
> +.impl.min_access_size = 4,
> +.impl.max_access_size = 4,
> +.impl.max_access_size = 4,
> +};
> +
> +static void rc4030_dma_tt_update(rc4030State *s, uint32_t new_tl_base,
> + uint32_t new_tl_limit)
> +{
> +int entries, i;
> +dma_pagetable_entry *dma_tl_contents;
> +
> +if (s->dma_tl_limit) {
> +/* write old dma tl table to physical memory */
> +memory_region_del_subregion(get_system_memory(), &s->dma_tt);
> +cpu_physical_memory_write(s->dma_tl_limit & 0x7fff,
> +  memory_region_get_ram_ptr(&s->dma_tt),
> +  s->dma_tl_limit);

You would need object_unparent(&s->dma_tt) here.

However, this breaks the rules for memory region lifetime (see
docs/memory.txt).

One solution is to warn here for a large dma_tl_limit and always create
a 4K ROMD region.  Here you can create an alias into the actual dma_tt
region and add/remove/unparent that alias.  Aliases can be created and
unparented at will.

> @@ -733,7 +793,16 @@ static void rc4030_do_dma(void *opaque, int n, uint8_t 
> *buf, int len, int is_wri
>  dma_addr = s->dma_regs[n][DMA_REG_ADDRESS];
>  
>  /* Read/write data at right place */
> -rc4030_dma_memory_rw(opaque, dma_addr, buf, len, is_write);
> +for (i = 0; i < len; ) {
> +int ncpy = DMA_PAGESIZE - (dma_addr & (DMA_PAGESIZE - 1));
> +if (ncpy > len - i) {
> +ncpy = len - i;
> +}
> +address_space_rw(&s->dma_as, dma_addr, buf + i, ncpy, is_write);
> +
> +dma_addr += ncpy;
> +i += ncpy;
> +}
>  
>  s->dma_regs[n][DMA_REG_ENABLE] |= DMA_FLAG_TC_INTR;
>  s->dma_regs[n][DMA_REG_COUNT] -= len;

The loop should not be necessary, address_space_rw does the same.

Paolo

> @@ -800,6 +869,7 @@ void *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
>MemoryRegion *sysmem)
>  {
>  rc4030State *s;
> +int i;
>  
>  s = g_malloc0(sizeof(rc4030State));
>  
> @@ -821,5 +891,15 @@ void *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
>"rc4030.jazzio", 0x1000);
>  memory_region_add_subregion(sysmem, 0xf000, &s->iomem_jazzio);
>  
> +memory_region_init(&s->dma_tt, NULL, "dma_tt", 0);
> +memory_region_init(&s->dma_mr, NULL, "dma", INT32_MAX);
> +for (i = 0; i < MAX_TL_ENTRIES; ++i) {
> +memory_region_init_alias(&s->dma_mrs[i], NULL, "dma-alias",
> + get_system_memory(), 0, DMA_PAGESIZE);
> +memory_region_set_enabled(&s->dma_mrs[i], false);
> +memory_region_add_subregion(&s->dma_mr, i * DMA_PAGESIZE,
> +&s->dma_mrs[i]);
> +}
> +address_space_init(&s->dma_as, &s->dma_mr, "rc4030_dma");
>  return s;
>  }
> 

Re: [Qemu-devel] [RFC] optimization for qcow2 cache get/put

[Stefan Hajnoczi]

On Mon, Jan 26, 2015 at 09:20:00PM +0800, Zhang Haoyu wrote:
> Hi, all
> 
> Regarding too large qcow2 image, e.g., 2TB,
> so long disruption happened when performing snapshot,
> which was caused by cache update and IO wait.

I have CCed Kevin Wolf, the qcow2 maintainer.

> perf top data shown as below,
>PerfTop:2554 irqs/sec  kernel: 0.4%  exact:  0.0% [4000Hz cycles],  
> (target_pid: 34294)
> 
> 
> 33.80%  qemu-system-x86_64  [.] qcow2_cache_do_get
> 27.59%  qemu-system-x86_64  [.] qcow2_cache_put   
> 15.19%  qemu-system-x86_64  [.] qcow2_cache_entry_mark_dirty  
>  5.49%  qemu-system-x86_64  [.] update_refcount   
>  3.02%  libpthread-2.13.so  [.] pthread_getspecific   
>  2.26%  qemu-system-x86_64  [.] get_refcount  
>  1.95%  qemu-system-x86_64  [.] coroutine_get_thread_state
>  1.32%  qemu-system-x86_64  [.] qcow2_update_snapshot_refcount
>  1.20%  qemu-system-x86_64  [.] qemu_coroutine_self   
>  1.16%  libz.so.1.2.7   [.] 0x3018
>  0.95%  qemu-system-x86_64  [.] qcow2_update_cluster_refcount 
>  0.91%  qemu-system-x86_64  [.] qcow2_cache_get   
>  0.76%  libc-2.13.so[.] 0x00134e49
>  0.73%  qemu-system-x86_64  [.] bdrv_debug_event  
>  0.16%  qemu-system-x86_64  [.] pthread_getspecific@plt   
>  0.12%  [kernel][k] _raw_spin_unlock_irqrestore   
>  0.10%  qemu-system-x86_64  [.] vga_draw_line24_32
>  0.09%  [vdso]  [.] 0x060c
>  0.09%  qemu-system-x86_64  [.] qcow2_check_metadata_overlap  
>  0.08%  [kernel][k] do_blockdev_direct_IO  
> 
> If expand the cache table size, the IO will be decreased, 
> but the calculation time will be grown.
> so it's worthy to optimize qcow2 cache get and put algorithm.
> 
> My proposal:
> get:
> using ((use offset >> cluster_bits) % c->size) to locate the cache entry,
> raw implementation,
> index = (use offset >> cluster_bits) % c->size;
> if (c->entries[index].offset == offset) {
> goto found;
> }
> 
> replace:
> c->entries[use offset >> cluster_bits) % c->size].offset = offset;
> ...
> 
> put:
> using 64-entries cache table to cache
> the recently got c->entries, i.e., cache for cache,
> then during put process, firstly search the 64-entries cache,
> if not found, then the c->entries.
> 
> Any idea?
> 
> Thanks,
> Zhang Haoyu
> 


pgpudKdIpGDV6.pgp
Description: PGP signature

Re: [Qemu-devel] [PULL 2.3 0/5] ppc patch queue 2015-03-25 for 2.3

[Peter Maydell]

On 25 March 2015 at 21:54, Alexander Graf  wrote:
> Hi Peter,
>
> This is my current patch queue for ppc against 2.3.  Please pull.
>
> Alex
>
>
> The following changes since commit 054903a832b865eb5432d79b5c9d1e1ff31b58d7:
>
>   Update version for v2.3.0-rc1 release (2015-03-24 16:34:16 +)
>
> are available in the git repository at:
>
>   git://github.com/agraf/qemu.git tags/signed-ppc-for-upstream
>
> for you to fetch changes up to c6e765035bc5e0d73231c2c0fbd54620201d6655:
>
>   powerpc: fix -machine usb=no for newworld and pseries machines (2015-03-25 
> 22:49:47 +0100)
>
> 
> Patch queue for 2.3 ppc - 2015-03-25
>
> Just a few bug fixes before 2.3 gets released:
>
>   - pseries: Firmware update, bugfixes
>   - remove POWER5+ v0.0 that we incorrectly introduced in 2.3
>   - Fix -machine usb=no
>   - Fix -boot once=foo in pseries
>   - Add NULL pointer check in pseries machine init

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL for v2.3 01/01] seccomp: update libseccomp version and remove arch restriction

[Eduardo Otubo]

On Thu, Mar 26, 2015 at 02=38=55PM +0100, Juan Quintela wrote:
> Eduardo Otubo  wrote:
> > Libseccomp version updated to 2.2.0 and arch restriction to x86/x86_64
> > is now removed. It's supposed to work on armv7l as well.
> >
> > Related bug: https://bugs.launchpad.net/qemu/+bug/1363641
> >
> > Signed-off-by: Eduardo Otubo 
> 
> This prevent compliation on x86_64 on Fedora 21.
> 
> (migration/next)$ make -j5 -C /scratch/tmp/next/all/
> make: Entering directory '/scratch/tmp/next/all'
> config-host.mak is out-of-date, running configure
> 
> ERROR: User requested feature libseccomp
>configure was not able to find it.
>Install libseccomp devel >= 2.2.0
> 
> Makefile:30: recipe for target 'config-host.mak' failed
> make: *** [config-host.mak] Error 1
> make: Leaving directory '/scratch/tmp/next/all'
> (migration/next)$ rpm -qa | grep seccomp
> libseccomp-2.1.1-5.fc21.x86_64
> libseccomp-devel-2.1.1-5.fc21.x86_64
> libseccomp-debuginfo-2.1.1-5.fc21.x86_64
> (migration/next)$ 
> 
> This was compiling correctly until this patch got in.  And virt-test
> uses seccomp by default.
> 
> Fedora 21 is less than 3 months old.  Do we really want to avoid
> compilation there?

Hello Juan,

I completely understand your concern. Perhaps a ping on libseccomp
Fedora package maintainer would be a better way to tackle this issue
instead of reverting this commit. Libseccomp 2.2.0 is released since Feb
12th and I actually gave it a little time frame for other distros to
update their packages so we don't run into issues like this.

It's important to remember that this patch is also the proper fix for
this bug: https://bugs.launchpad.net/qemu/+bug/1363641

Regards,

-- 
Eduardo Otubo
ProfitBricks GmbH


signature.asc
Description: Digital signature

Re: [Qemu-devel] [PULL for v2.3 01/01] seccomp: update libseccomp version and remove arch restriction

[Eduardo Otubo]

On Thu, Mar 26, 2015 at 03=09=19PM +0100, Juan Quintela wrote:
> Juan Quintela  wrote:
> > Eduardo Otubo  wrote:
> >> Libseccomp version updated to 2.2.0 and arch restriction to x86/x86_64
> >> is now removed. It's supposed to work on armv7l as well.
> >>
> >> Related bug: https://bugs.launchpad.net/qemu/+bug/1363641
> >>
> >> Signed-off-by: Eduardo Otubo 
> 
> 
> Just in case anyone is interested, default virt-test output with current
> qemu
> 
> 
> 15:05:01 ERROR| Qemu output:
> 15:05:01 ERROR| qemu-system-x86_64: -sandbox on: sandboxing request but 
> seccomp 
> is not compiled into this build
> 15:05:01 ERROR| 
> 15:05:01 ERROR| 
> 15:05:01 ERROR| FAIL 
> type_specific.io-github-autotest-qemu.migrate.default.tcp -> VMStartError: VM 
> 'virt-tests-vm1' failed to start: Qemu is defunct.
> Qemu output:
> qemu-system-x86_64: -sandbox on: sandboxing request but seccomp is not 
> compiled into this build
> 
> 
> 
> Notice that you can "fix" the problem running with --qemu_sandbox=off,
> but breaking the main test suite just before release looks wrong?

Is it possible to temporarely disable this feature in your testing
environment while libseccomp is not updated?

Regards,

> 
> Later, Juan.
> 
> >
> > This prevent compliation on x86_64 on Fedora 21.
> >
> > (migration/next)$ make -j5 -C /scratch/tmp/next/all/
> > make: Entering directory '/scratch/tmp/next/all'
> > config-host.mak is out-of-date, running configure
> >
> > ERROR: User requested feature libseccomp
> >configure was not able to find it.
> >Install libseccomp devel >= 2.2.0
> >
> > Makefile:30: recipe for target 'config-host.mak' failed
> > make: *** [config-host.mak] Error 1
> > make: Leaving directory '/scratch/tmp/next/all'
> > (migration/next)$ rpm -qa | grep seccomp
> > libseccomp-2.1.1-5.fc21.x86_64
> > libseccomp-devel-2.1.1-5.fc21.x86_64
> > libseccomp-debuginfo-2.1.1-5.fc21.x86_64
> > (migration/next)$ 
> >
> > This was compiling correctly until this patch got in.  And virt-test
> > uses seccomp by default.
> >
> > Fedora 21 is less than 3 months old.  Do we really want to avoid
> > compilation there?
> >
> > Thanks, Juan.
> >
> >
> >> ---
> >>  configure | 5 ++---
> >>  1 file changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/configure b/configure
> >> index 589798e..cbe6495 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -1848,14 +1848,13 @@ fi
> >>  # libseccomp check
> >>  
> >>  if test "$seccomp" != "no" ; then
> >> -if test "$cpu" = "i386" || test "$cpu" = "x86_64" &&
> >> -$pkg_config --atleast-version=2.1.1 libseccomp; then
> >> +if $pkg_config --atleast-version=2.2.0 libseccomp; then
> >>  libs_softmmu="$libs_softmmu `$pkg_config --libs libseccomp`"
> >>  QEMU_CFLAGS="$QEMU_CFLAGS `$pkg_config --cflags libseccomp`"
> >>seccomp="yes"
> >>  else
> >>if test "$seccomp" = "yes"; then
> >> -feature_not_found "libseccomp" "Install libseccomp devel >= 
> >> 2.1.1"
> >> +feature_not_found "libseccomp" "Install libseccomp devel >= 
> >> 2.2.0"
> >>fi
> >>seccomp="no"
> >>  fi

-- 
Eduardo Otubo
ProfitBricks GmbH


signature.asc
Description: Digital signature

Re: [Qemu-devel] [PULL for v2.3 01/01] seccomp: update libseccomp version and remove arch restriction

[Peter Maydell]

On 26 March 2015 at 14:37, Eduardo Otubo  wrote:
> I completely understand your concern. Perhaps a ping on libseccomp
> Fedora package maintainer would be a better way to tackle this issue
> instead of reverting this commit. Libseccomp 2.2.0 is released since Feb
> 12th and I actually gave it a little time frame for other distros to
> update their packages so we don't run into issues like this.

Well, we shouldn't really be mandating latest-and-greatest versions
of our upstream dependencies unless the maintainer of those dependencies
feels the earlier versions are so badly broken that it would be better
to refuse to use them at all.

> It's important to remember that this patch is also the proper fix for
> this bug: https://bugs.launchpad.net/qemu/+bug/1363641

If that only applies to certain architectures we can make the
dependency version vary depending on which arch we're building
for, I suppose.

-- PMM

[Qemu-devel] [PULL 0/3] Migration pull request

[Juan Quintela]

Hi Peter,

In this pull requset:
- rdma fixes (Padmanabh)
- fix crash on multiple -incoming (dave)
- don't access last_sent_block outside migration_thread (me)
  It is needed by migration compression patches

Please apply.

Thanks, Juan.


The following changes since commit 087c4c9419d3086ac0a920899e4fed8ceaf9bb2b:

  Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging 
(2015-03-26 12:18:44 +)

are available in the git repository at:

  git://github.com/juanquintela/qemu.git tags/migration/20150326

for you to fetch changes up to 43edc0ed11a4d25f2fe67bb9d89a8a6a0a43b1e0:

  migration:  remove last_sent_block from save_page_header (2015-03-26 15:31:46 
+0100)


migration/next for 20150326


Dr. David Alan Gilbert (1):
  Avoid crashing on multiple -incoming

Juan Quintela (1):
  migration:  remove last_sent_block from save_page_header

Padmanabh Ratnakar (1):
  rdma: Fix cleanup in error paths

 arch_init.c  | 12 ++--
 migration/rdma.c | 22 --
 vl.c |  4 +++-
 3 files changed, 17 insertions(+), 21 deletions(-)

[Qemu-devel] [PULL 2/3] rdma: Fix cleanup in error paths

[Juan Quintela]

From: Padmanabh Ratnakar 

As part of commit e325b49a320b493cc5d69e263751ff716dc458fe,
order in which resources are destroyed was changed for fixing
a seg fault. Due to this change, CQ will never get destroyed as
CQ should be destroyed after QP destruction. Seg fault is caused
improper cleanup when connection fails. Fixing cleanup after
connection failure and order in which resources are destroyed
in qemu_rdma_cleanup() routine.

Signed-off-by: Meghana Cheripady 
Signed-off-by: Padmanabh Ratnakar 
Signed-off-by: Juan Quintela 
---
 migration/rdma.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index e6c3a67..77e3444 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -2194,6 +2194,10 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
 }
 }

+if (rdma->qp) {
+rdma_destroy_qp(rdma->cm_id);
+rdma->qp = NULL;
+}
 if (rdma->cq) {
 ibv_destroy_cq(rdma->cq);
 rdma->cq = NULL;
@@ -2206,18 +2210,14 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
 ibv_dealloc_pd(rdma->pd);
 rdma->pd = NULL;
 }
-if (rdma->listen_id) {
-rdma_destroy_id(rdma->listen_id);
-rdma->listen_id = NULL;
-}
 if (rdma->cm_id) {
-if (rdma->qp) {
-rdma_destroy_qp(rdma->cm_id);
-rdma->qp = NULL;
-}
 rdma_destroy_id(rdma->cm_id);
 rdma->cm_id = NULL;
 }
+if (rdma->listen_id) {
+rdma_destroy_id(rdma->listen_id);
+rdma->listen_id = NULL;
+}
 if (rdma->channel) {
 rdma_destroy_event_channel(rdma->channel);
 rdma->channel = NULL;
@@ -2309,8 +2309,6 @@ static int qemu_rdma_connect(RDMAContext *rdma, Error 
**errp)
 if (ret) {
 perror("rdma_connect");
 ERROR(errp, "connecting to destination!");
-rdma_destroy_id(rdma->cm_id);
-rdma->cm_id = NULL;
 goto err_rdma_source_connect;
 }

@@ -2319,8 +2317,6 @@ static int qemu_rdma_connect(RDMAContext *rdma, Error 
**errp)
 perror("rdma_get_cm_event after rdma_connect");
 ERROR(errp, "connecting to destination!");
 rdma_ack_cm_event(cm_event);
-rdma_destroy_id(rdma->cm_id);
-rdma->cm_id = NULL;
 goto err_rdma_source_connect;
 }

@@ -2328,8 +2324,6 @@ static int qemu_rdma_connect(RDMAContext *rdma, Error 
**errp)
 perror("rdma_get_cm_event != EVENT_ESTABLISHED after rdma_connect");
 ERROR(errp, "connecting to destination!");
 rdma_ack_cm_event(cm_event);
-rdma_destroy_id(rdma->cm_id);
-rdma->cm_id = NULL;
 goto err_rdma_source_connect;
 }
 rdma->connected = true;
-- 
2.1.0

[Qemu-devel] [PULL 3/3] migration: remove last_sent_block from save_page_header

[Juan Quintela]

Compression code (still not on tree) want to call this funtion from
outside the migration thread, so we can't write to last_sent_block.

Instead of reverting full patch:

[PULL 07/11] save_block_hdr: we can recalculate

Just revert the parts that touch last_sent_block.

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
---
 arch_init.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index fcfa328..4c8fcee 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -332,19 +332,14 @@ static size_t save_page_header(QEMUFile *f, RAMBlock 
*block, ram_addr_t offset)
 {
 size_t size;

-if (block == last_sent_block) {
-offset |= RAM_SAVE_FLAG_CONTINUE;
-}
-
 qemu_put_be64(f, offset);
 size = 8;

-if (block != last_sent_block) {
+if (!(offset & RAM_SAVE_FLAG_CONTINUE)) {
 qemu_put_byte(f, strlen(block->idstr));
 qemu_put_buffer(f, (uint8_t *)block->idstr,
 strlen(block->idstr));
 size += 1 + strlen(block->idstr);
-last_sent_block = block;
 }
 return size;
 }
@@ -644,6 +639,10 @@ static int ram_save_page(QEMUFile *f, RAMBlock* block, 
ram_addr_t offset,
 XBZRLE_cache_lock();

 current_addr = block->offset + offset;
+
+if (block == last_sent_block) {
+offset |= RAM_SAVE_FLAG_CONTINUE;
+}
 if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
 if (ret != RAM_SAVE_CONTROL_DELAYED) {
 if (bytes_xmit > 0) {
@@ -739,6 +738,7 @@ static int ram_find_and_save_block(QEMUFile *f, bool 
last_stage,

 /* if page is unmodified, continue to the next */
 if (pages > 0) {
+last_sent_block = block;
 break;
 }
 }
-- 
2.1.0

[Qemu-devel] [PULL 1/3] Avoid crashing on multiple -incoming

[Juan Quintela]

From: "Dr. David Alan Gilbert" 

Passing multiple -incoming options used to crash qemu (due to
an invalid state transition incoming->incoming).  Instead we now
take the last -incoming option, e.g.:

qemu-system-x86_64 -nographic -incoming tcp:: -incoming defer

ends up doing the defer.

Signed-off-by: Dr. David Alan Gilbert 
Reviewed-by: Eric Blake 
Reviewed-by: Amit Shah 
Signed-off-by: Juan Quintela 
---
 vl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index 75ec292..74c2681 100644
--- a/vl.c
+++ b/vl.c
@@ -3618,8 +3618,10 @@ int main(int argc, char **argv, char **envp)
 }
 break;
 case QEMU_OPTION_incoming:
+if (!incoming) {
+runstate_set(RUN_STATE_INMIGRATE);
+}
 incoming = optarg;
-runstate_set(RUN_STATE_INMIGRATE);
 break;
 case QEMU_OPTION_nodefaults:
 has_defaults = 0;
-- 
2.1.0

Re: [Qemu-devel] [PATCH v5 10/28] qapi: Segregate anonymous unions into alternates in generator

[Markus Armbruster]

Eric Blake  writes:

> Special-casing 'discriminator == {}' for handling anonymous unions
> is getting awkward; since this particular type is not always a
> dictionary on the wire, it is easier to treat it as a completely
> different class of type, "alternate", so that if a type is listed
> in the union_types array, we know it is not an anonymous union.
>
> This patch just further segregates union handling, to make sure that
> anonymous unions are not stored in union_types, and splitting up
> check_union() into separate functions.  A future patch will change
> the qapi grammar, and having the segregation already in place will
> make it easier to deal with the distinct meta-type.
>
> Signed-off-by: Eric Blake 
> ---
>  scripts/qapi-types.py |  6 ++--
>  scripts/qapi-visit.py |  4 +--
>  scripts/qapi.py   | 94 
> +--
>  3 files changed, 58 insertions(+), 46 deletions(-)
>
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index 2390887..c9e0201 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -170,7 +170,7 @@ typedef enum %(name)s
>
>  return lookup_decl + enum_decl
>
> -def generate_anon_union_qtypes(expr):
> +def generate_alternate_qtypes(expr):
>
>  name = expr['union']
>  members = expr['data']
> @@ -181,7 +181,7 @@ const int %(name)s_qtypes[QTYPE_MAX] = {
>  name=name)
>
>  for key in members:
> -qtype = find_anonymous_member_qtype(members[key])
> +qtype = find_alternate_member_qtype(members[key])
>  assert qtype, "Invalid anonymous union member"
>
>  ret += mcgen('''
> @@ -408,7 +408,7 @@ for expr in exprs:
>  fdef.write(generate_enum_lookup('%sKind' % expr['union'],
>  expr['data'].keys()))
>  if expr.get('discriminator') == {}:
> -fdef.write(generate_anon_union_qtypes(expr))
> +fdef.write(generate_alternate_qtypes(expr))
>  else:
>  continue
>  fdecl.write(ret)
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index 3f82bd4..77b0a1f 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -237,7 +237,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s *obj, const 
> char *name, Error **er
>  ''',
>   name=name)
>
> -def generate_visit_anon_union(name, members):
> +def generate_visit_alternate(name, members):
>  ret = mcgen('''
>
>  void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error 
> **errp)
> @@ -302,7 +302,7 @@ def generate_visit_union(expr):
>
>  if discriminator == {}:
>  assert not base
> -return generate_visit_anon_union(name, members)
> +return generate_visit_alternate(name, members)
>
>  enum_define = discriminator_find_enum_define(expr)
>  if enum_define:
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 39cc88b..17252e9 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -224,21 +224,16 @@ def find_base_fields(base):
>  return None
>  return base_struct_define['data']
>
> -# Return the qtype of an anonymous union branch, or None on error.
> -def find_anonymous_member_qtype(qapi_type):
> +# Return the qtype of an alternate branch, or None on error.
> +def find_alternate_member_qtype(qapi_type):
>  if builtin_types.has_key(qapi_type):
>  return builtin_types[qapi_type]
>  elif find_struct(qapi_type):
>  return "QTYPE_QDICT"
>  elif find_enum(qapi_type):
>  return "QTYPE_QSTRING"
> -else:
> -union = find_union(qapi_type)
> -if union:
> -discriminator = union.get('discriminator')
> -if discriminator == {}:
> -return None
> -return "QTYPE_QDICT"
> +elif find_union(qapi_type):
> +return "QTYPE_QDICT"
>  return None
>
>  # Return the discriminator enum define if discriminator is specified as an
> @@ -276,22 +271,13 @@ def check_union(expr, expr_info):
>  discriminator = expr.get('discriminator')
>  members = expr['data']
>  values = { 'MAX': '(automatic)' }
> -types_seen = {}
>
> -# Three types of unions, determined by discriminator.
> +# Two types of unions, determined by discriminator.
> +assert discriminator != {}
>
> -# If the value of member 'discriminator' is {}, it's an
> -# anonymous union, and must not have a base.
> -if discriminator == {}:
> -enum_define = None
> -if base:
> -raise QAPIExprError(expr_info,
> -"Anonymous union '%s' must not have a base"
> -% name)
> -
> -# Else if the union object has no member 'discriminator', it's an
> +# If the union object has no member 'discriminator', it's an
>  # ordinary union.  For now, it must not have a base.
> -elif not discriminator:
> +if not discriminator:
>  enum_define = None
> 

Re: [Qemu-devel] [PULL for v2.3 01/01] seccomp: update libseccomp version and remove arch restriction

[Juan Quintela]

Peter Maydell  wrote:
> On 26 March 2015 at 14:37, Eduardo Otubo  
> wrote:
>> I completely understand your concern. Perhaps a ping on libseccomp
>> Fedora package maintainer would be a better way to tackle this issue
>> instead of reverting this commit. Libseccomp 2.2.0 is released since Feb
>> 12th and I actually gave it a little time frame for other distros to
>> update their packages so we don't run into issues like this.
>
> Well, we shouldn't really be mandating latest-and-greatest versions
> of our upstream dependencies unless the maintainer of those dependencies
> feels the earlier versions are so badly broken that it would be better
> to refuse to use them at all.

+1

>> It's important to remember that this patch is also the proper fix for
>> this bug: https://bugs.launchpad.net/qemu/+bug/1363641
>
> If that only applies to certain architectures we can make the
> dependency version vary depending on which arch we're building
> for, I suppose.

I will go for this.

My concern is that virt-test will stop working until distributins gets
updated.  I can update it myself, or running with --qemu_sandbox=off,
that is what I just did for the pull request just sent.  But I feel bad
that people on recent distributions are not able to use important
features.

Thanks, Juan.

Re: [Qemu-devel] [PATCH v5 11/28] qapi: Rename anonymous union type in test

[Markus Armbruster]

Eric Blake  writes:

> Reduce churn in the future patch that replaces anonymous unions
> with a new metatype 'alternate' by changing 'AnonUnion' to
> 'Alternate'.
>
> Signed-off-by: Eric Blake 

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [GSoC] project proposal

[Paolo Bonzini]

On 18/03/2015 15:53, Catalin Vasile wrote:
> Hi,
> 
> My name is Catalin Vasile and I want to participate with a project for
> qemu at GSoC.
> From what I understand from the rules, I can participate with things I
> could also use for my college projects.
> This is my last bachelor year and I'm doing my diploma project, which is
> related to virtualization, more specific qemu-kvm.
> I'm trying to do a paravirtualized device using virtio and vhost.
> I've already done some work.
> To be more exact, I want to make a virtio-crypto device to emulate a
> virtual cryptographic offloading device that will send jobs from the
> guest to a vhost that will process the jobs. This mechanism will link
> CryptoAPI from the guest to the CryptoAPI from the host. This way,
> whatever it's beneath CryptoAPI from the host will be used as offloading
> for the guest.
> Is there a mentor interested in getting involved in this kind of project?

Hi Catalin, what accelerator are you targeting specifically?

Paolo

Re: [Qemu-devel] [PULL for v2.3 01/01] seccomp: update libseccomp version and remove arch restriction

[Peter Maydell]

On 26 March 2015 at 14:37, Eduardo Otubo  wrote:
> Libseccomp 2.2.0 is released since Feb
> 12th and I actually gave it a little time frame for other distros to
> update their packages so we don't run into issues like this.

Incidentally, "about two months" is actually a very short
time between upstream-release and mandating it for QEMU.
Consider that our glib dependency is 2.12, which was released
in 2006, and we're still debating whether to move forward
to 2.24 (released in 2010). Distro release schedules are
often on a scale of years, especially for long-term-support
distros, and we want to support as wide a range of them
as we reasonably can.

-- PMM

Re: [Qemu-devel] [PATCH v5 06/28] qapi: Add some union tests

[Eric Blake]

On 03/26/2015 07:18 AM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> Demonstrate that the qapi generator doesn't deal well with unions
>> that aren't up to par. Later patches will update the expected
>> reseults as the generator is made stricter.
>>
>> Of particular note, we currently allow 'base' without 'discriminator'
>> as a way to create a simple union with a base class.  However, none
>> of the existing QMP or QGA interfaces use it (it is exercised only
>> in the testsuite).
> 
> qapi-code-gen.txt documents 'base' only for flat unions.  We should
> either document its use in simple unions, or outlaw it.

I went with outlaw later in the series, and the rest of my commit
message was an attempt to explain my reasoning in that choice.  But I
can certainly try to improve the wording, if we need a respin.

> 
>> Meanwhile, it would be nice to allow
>> 'discriminator':'EnumType' without 'base' for creating a simple union
>> that is type-safe rather than open-coded to a generated enum (right
>> now, we are only type-safe when using a flat union, but that uses
>> a different syntax of 'discriminator':'member-name' which requires
>> a base class containing a 'member-name' enum field).
> 
> I'm afraid I don't get you.  Can you give examples?

Using this common code with the appropriate union for each example:
{ 'command':'foo', 'data':UNION }

Right now, we have flat unions which are required to be type-safe (all
branches MUST map back to the enum type of the discriminator, enforced
by the generator, so that if the enum later adds a member, the union
must also be updated to match):

[1]
{ 'union':'Safe', 'base':'Base', 'discriminator':'switch',
  'data':{ 'one':'str', 'two':'int' }}

{"execute":"foo", "arguments":{'switch':'one', 'data':'hello'}}

and simple unions which cannot be typesafe (the branches of the union
are open-coded - even if they correlate to an existing enum, there is
nothing enforcing that extensions to the enum be reflected into the union):

[2]
{ 'union':'SimpleButOpenCoded',
  'data':{ 'one': 'str', 'two':'int' }}

{"execute":"foo", "arguments":{'type':'one', 'data':'hello'}}

I'm hoping to add as a followup series a variant of simple unions that
is type-safe:

[3]
{ 'union':'SimpleAndSafe', 'discriminator':'MyEnum',
  'data':{ 'one':'str', 'two':'int' }}

{"execute":"foo", "arguments":{'type':'one', 'data':'hello'}}


But the existing, unused-except-in-testsuite, notion of a simple union
with a base class looks like:

[4]
{ 'type':'Shared', 'data':{'common':'int'}}
{ 'union':'SimpleWithBase', 'base':'Shared',
  'data':{ 'one':'str', 'two':'int' }}

{"execute":"foo", "arguments":{'common':1, 'type':'one', 'data':'hello'}}

If we were to allow the addition of 'discriminator':'EnumType' to a
simple union [3], but then add that discriminator to an existing case of
a simple union with base [4], it would look like:

{ 'type':'Shared', 'data':{'common':'int'}}
{ 'union':'SimpleWithBaseAndDiscriminator', 'base':'Shared',
  'discriminator':'MyEnum',
  'data':{ 'one':'str', 'two':'int' }}

Yuck.  That is indistinguishable from flat unions [1], except by whether
discriminator names an enum type or a member of the base class.

> 
>>   If both 'base'
>> and 'discriminator' are optional, then converting a simple union
>> with base class to a type-safe simple union with an enum discriminator
>> would not be possible.  So my plan is to get rid of 'base' without
>> 'discriminator' later in the series;
> 
> Aha: you're going to outlaw 'base' in simple unions.  Yes, please.

Okay, you came to my desired conclusion; it's just that my wording in
the middle didn't help.

> 
>>  this will be no real loss, as any
>> union that needs additional common fields can always use a flat
>> union.
> 
> The mathematical concept behind unions is the sum type.
> 
> We have three implementations, and we call them simple, flat, anonymous.
> 
> Anonymous unions are implicitly tagged.  They come with the obvious
> restrictions required to make implicit tagging work.

and get renamed to 'alternate' later in the series, so they are not
worth worrying about here.

> 
> The other two are explicitly tagged.  The difference between them is
> just notation.  I like my unions flat, because for me the extra wrapping
> is just notational overhead.
> 
> In particular, I can define simple unions in terms of flat ones by
> restricting all union cases to a single member named 'data'.  They're
> not implemented that way, but that's a historical accident.  Simple
> unions are a redundant concept.

Cool.  Or more concretely,

{ 'union': 'Simple', 'data': { 'one': 'str', 'two': 'int' } }

is identical on the wire to:

{ 'enum': 'MyEnum', 'data': ['one', 'two'] }
{ 'type': 'Base', 'data': { 'type': 'MyEnum' } }
{ 'type': 'Branch1', 'data': { 'data': 'str' } }
{ 'type': 'Branch2', 'data': { 'data': 'int' } }
{ 'union': 'Flat', 'base':

Re: [Qemu-devel] E5-2620v2 - emulation stop error

[Andrey Korolyov]

On Thu, Mar 26, 2015 at 12:18 PM, Andrey Korolyov  wrote:
> On Thu, Mar 26, 2015 at 5:47 AM, Bandan Das  wrote:
>> Hi Andrey,
>>
>> Andrey Korolyov  writes:
>>
>>> On Mon, Mar 16, 2015 at 10:17 PM, Andrey Korolyov  wrote:
 For now, it looks like bug have a mixed Murphy-Heisenberg nature, as
 it appearance is very rare (compared to the number of actual launches)
 and most probably bounded to the physical characteristics of my
 production nodes. As soon as I reach any reproducible path for a
 regular workstation environment, I`ll let everyone know. Also I am
 starting to think that issue can belong to the particular motherboard
 firmware revision, despite fact that the CPU microcode is the same
 everywhere.
>>
>> I will take the risk and say this - "could it be a processor bug ?" :)
>>
>>>
>>> Hello everyone, I`ve managed to reproduce this issue
>>> *deterministically* with latest seabios with smp fix and 3.18.3. The
>>> error occuring just *once* per vm until hypervisor reboots, at least
>>> in my setup, this is definitely crazy...
>>>
>>> - launch two VMs (Centos 7 in my case),
>>> - wait a little while they are booting,
>>> - attach serial console (I am using virsh list for this exact purpose),
>>> - issue acpi reboot or reset, does not matter,
>>> - VM always hangs at boot, most times with sgabios initialization
>>> string printed out [1], but sometimes it hangs a bit later [2],
>>> - no matter how many times I try to relaunch the QEMU afterwards, the
>>> issue does not appear on VM which experienced problem once;
>>> - trace and sample args can be seen in [3] and [4] respectively.
>>
>> My system is a Dell R720 dual socket which has 2620v2s. I tried your
>> setup but couldn't reproduce (my qemu cmdline isn't exactly the same
>> as yours), although, if you could simplify your command line a bit,
>> I can try again.
>>
>> Bandan
>>
>>> 1)
>>> Google, Inc.
>>> Serial Graphics Adapter 06/11/14
>>> SGABIOS $Id: sgabios.S 8 2010-04-22 00:03:40Z nlaredo $
>>> (pbuilder@zorak) Wed Jun 11 05:57:34 UTC 2014
>>> Term: 211x62
>>> 4 0
>>>
>>> 2)
>>> Google, Inc.
>>> Serial Graphics Adapter 06/11/14
>>> SGABIOS $Id: sgabios.S 8 2010-04-22 00:03:40Z nlaredo $
>>> (pbuilder@zorak) Wed Jun 11 05:57:34 UTC 2014
>>> Term: 211x62
>>> 4 0
>>> [...empty screen...]
>>> SeaBIOS (version 1.8.1-20150325_230423-testnode)
>>> Machine UUID 3c78721f-7317-4f85-bcbe-f5ad46d293a1
>>>
>>>
>>> iPXE (http://ipxe.org) 00:02.0 C100 PCI2.10 PnP PMM+3FF95BA0+3FEF5BA0 C10
>>>
>>> 3)
>>>
>>> KVM internal error. Suberror: 2
>>> extra data[0]: 80ef
>>> extra data[1]: 8b0d
>>> EAX= EBX= ECX= EDX=
>>> ESI= EDI= EBP= ESP=6d2c
>>> EIP=d331 EFL=00010202 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0
>>> ES =   9300
>>> CS =f000 000f  9b00
>>> SS =   9300
>>> DS =   9300
>>> FS =   9300
>>> GS =   9300
>>> LDT=   8200
>>> TR =   8b00
>>> GDT= 000f6cb0 0037
>>> IDT=  03ff
>>> CR0=0010 CR2= CR3= CR4=
>>> DR0= DR1= DR2=
>>> DR3=
>>> DR6=0ff0 DR7=0400
>>> EFER=
>>> Code=66 c3 cd 02 cb cd 10 cb cd 13 cb cd 15 cb cd 16 cb cd 18 cb 
>>> 19 cb cd 1c cb cd 4a cb fa fc 66 ba 47 d3 0f 00 e9 ad fe f3 90 f0 0f
>>> ba 2d d4 fe fb 3f
>>>
>>> 4)
>>> /usr/bin/qemu-system-x86_64 -name centos71 -S -machine
>>> pc-i440fx-2.1,accel=kvm,usb=off -cpu SandyBridge,+kvm_pv_eoi -bios
>>> /usr/share/seabios/bios.bin -m 1024 -realtime mlock=off -smp
>>> 12,sockets=1,cores=12,threads=12 -uuid
>>> 3c78721f-7317-4f85-bcbe-f5ad46d293a1 -nographic -no-user-config
>>> -nodefaults -device sga -chardev
>>> socket,id=charmonitor,path=/var/lib/libvirt/qemu/centos71.monitor,server,nowait
>>> -mon chardev=charmonitor,id=monitor,mode=control -rtc
>>> base=utc,driftfix=slew -global kvm-pit.lost_tick_policy=discard
>>> -no-hpet -no-shutdown -global PIIX4_PM.disable_s3=1 -global
>>> PIIX4_PM.disable_s4=1 -boot strict=on -device
>>> nec-usb-xhci,id=usb,bus=pci.0,addr=0x3 -device
>>> virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x4 -drive
>>> file=rbd:dev-rack2/centos7-1.raw:id=qemukvm:key=XX:auth_supported=cephx\;none:mon_host=10.6.0.1\:6789\;10.6.0.3\:6789\;10.6.0.4\:6789,if=none,id=drive-virtio-disk0,format=raw,cache=writeback,aio=native
>>> -device 
>>> virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
>>> -chardev pty,id=charserial0 -device
>>> isa-serial,chardev=charserial0,id=serial0 -chardev
>>> socket,id=charchannel0,path=/var/lib/libvirt/qemu/centos71.sock,server,nowait
>>> -device 
>>> virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.1

Re: [Qemu-devel] [PULL for v2.3 01/01] seccomp: update libseccomp version and remove arch restriction

[Eduardo Otubo]

On Thu, Mar 26, 2015 at 02=44=14PM +, Peter Maydell wrote:
> On 26 March 2015 at 14:37, Eduardo Otubo  
> wrote:
> > I completely understand your concern. Perhaps a ping on libseccomp
> > Fedora package maintainer would be a better way to tackle this issue
> > instead of reverting this commit. Libseccomp 2.2.0 is released since Feb
> > 12th and I actually gave it a little time frame for other distros to
> > update their packages so we don't run into issues like this.
> 
> Well, we shouldn't really be mandating latest-and-greatest versions
> of our upstream dependencies unless the maintainer of those dependencies
> feels the earlier versions are so badly broken that it would be better
> to refuse to use them at all.
> 
> > It's important to remember that this patch is also the proper fix for
> > this bug: https://bugs.launchpad.net/qemu/+bug/1363641
> 
> If that only applies to certain architectures we can make the
> dependency version vary depending on which arch we're building
> for, I suppose.
> 

This sounds more like a reasonable approach that could solve the above
mentioned problem and also making virt-test to be able to keep using
this feature as well -- which is also very important in order to get
more important system calls for the whitelist. I'll roll out a new patch
for that.

Thanks for the idea, Paul.
Sorry for the trouble on your side, Juan.

Regards,

-- 
Eduardo Otubo
ProfitBricks GmbH


signature.asc
Description: Digital signature

Re: [Qemu-devel] [PATCH v5 10/28] qapi: Segregate anonymous unions into alternates in generator

[Eric Blake]

On 03/26/2015 08:47 AM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> Special-casing 'discriminator == {}' for handling anonymous unions
>> is getting awkward; since this particular type is not always a
>> dictionary on the wire, it is easier to treat it as a completely
>> different class of type, "alternate", so that if a type is listed
>> in the union_types array, we know it is not an anonymous union.
>>
>> This patch just further segregates union handling, to make sure that
>> anonymous unions are not stored in union_types, and splitting up
>> check_union() into separate functions.  A future patch will change
>> the qapi grammar, and having the segregation already in place will
>> make it easier to deal with the distinct meta-type.
>>
>> Signed-off-by: Eric Blake 
>> ---

>> @@ -535,7 +546,8 @@ def find_struct(name):
>>
>>  def add_union(definition):
>>  global union_types
>> -union_types.append(definition)
>> +if definition.get('discriminator') != {}:
>> +union_types.append(definition)
>>
>>  def find_union(name):
>>  global union_types
> 
> This is the only unobvious hunk.
> 
> union_types is used only through find_union.  The hunk makes
> find_union(N) return None when N names an anonymous union.
> 
> find_union() is used in two places:
> 
> * find_alternate_member_qtype()
> 
>   Patched further up.  It really wants only non-anonymous unions, and
>   this change to find_union() renders its check for anonymous unions
>   superfluous.  Good.
> 
> * generate_visit_alternate()
> 
>   Asserts that each member's type is either a built-in type, a complex
>   type, a union type, or an enum type.
> 
>   The change relaxes the assertion not to trigger on anonymous union
>   types.  Why is that okay?

No, the change tightens the assertion so that it will now fail on an
anonymous union nested as a branch of another anonymous union (where
before it could silently pass the assertion), because the anonymous
union is no longer found by find_union().  And this is okay because the
earlier change to find_alternate_member_qtype means that we don't allow
nested anonymous unions, so making the assertion fail if an anonymous
union gets through anyway is correct.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL for v2.3 01/01] seccomp: update libseccomp version and remove arch restriction

[Juan Quintela]

Eduardo Otubo  wrote:
> On Thu, Mar 26, 2015 at 02=44=14PM +, Peter Maydell wrote:
>> On 26 March 2015 at 14:37, Eduardo Otubo  
>> wrote:
>> > I completely understand your concern. Perhaps a ping on libseccomp
>> > Fedora package maintainer would be a better way to tackle this issue
>> > instead of reverting this commit. Libseccomp 2.2.0 is released since Feb
>> > 12th and I actually gave it a little time frame for other distros to
>> > update their packages so we don't run into issues like this.
>> 
>> Well, we shouldn't really be mandating latest-and-greatest versions
>> of our upstream dependencies unless the maintainer of those dependencies
>> feels the earlier versions are so badly broken that it would be better
>> to refuse to use them at all.
>> 
>> > It's important to remember that this patch is also the proper fix for
>> > this bug: https://bugs.launchpad.net/qemu/+bug/1363641
>> 
>> If that only applies to certain architectures we can make the
>> dependency version vary depending on which arch we're building
>> for, I suppose.
>> 
>
> This sounds more like a reasonable approach that could solve the above
> mentioned problem and also making virt-test to be able to keep using
> this feature as well -- which is also very important in order to get
> more important system calls for the whitelist. I'll roll out a new patch
> for that.
>
> Thanks for the idea, Paul.
> Sorry for the trouble on your side, Juan.

You are welcome.  I was just trying to fix the prob0lem before somebody
else hit it O:-)

Later, Juan.

[Qemu-devel] [PATCH for-2.3 3/4] s390x: do not include ram_addr.h

[Cornelia Huck]

From: Paolo Bonzini 

ram_addr.h is an internal interface and it is not needed anyway by
hw/s390x/ipl.c.

Cc: Christian Borntraeger 
Signed-off-by: Paolo Bonzini 
Message-Id: <1427295389-5054-1-git-send-email-pbonz...@redhat.com>
Signed-off-by: Christian Borntraeger 
Signed-off-by: Cornelia Huck 
---
 hw/s390x/ipl.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 54d0835..5c86613 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -14,7 +14,6 @@
 #include "sysemu/sysemu.h"
 #include "cpu.h"
 #include "elf.h"
-#include "exec/ram_addr.h"
 #include "hw/loader.h"
 #include "hw/sysbus.h"
 #include "hw/s390x/virtio-ccw.h"
-- 
2.3.4

[Qemu-devel] [PATCH for-2.3 4/4] s390x/ipl: avoid sign extension

[Cornelia Huck]

Make s390_update_iplstate() return uint32_t to avoid sign extensions
for cssids > 127. While this doesn't matter in practice yet (as
nobody supports MCSS-E and thus won't see the real cssid), play safe.

Reported-by: Paolo Bonzini 
Reviewed-by: Jason J. Herne 
Signed-off-by: Cornelia Huck 
---
 hw/s390x/ipl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 5c86613..2e26d2a 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -218,7 +218,7 @@ static Property s390_ipl_properties[] = {
  * - -1 if no valid boot device was found
  * - ccw id of the boot device otherwise
  */
-static uint64_t s390_update_iplstate(CPUS390XState *env, S390IPLState *ipl)
+static uint32_t s390_update_iplstate(CPUS390XState *env, S390IPLState *ipl)
 {
 DeviceState *dev_st;
 
-- 
2.3.4

[Qemu-devel] [PATCH for-2.3 1/4] virtio-ccw: fix range check for SET_VQ

[Cornelia Huck]

VIRTIO_PCI_QUEUE_MAX is already too big; a malicious guest would be
able to trigger a write beyond the VirtQueue structure.

Cc: qemu-sta...@nongnu.org
Reviewed-by: David Hildenbrand 
Signed-off-by: Cornelia Huck 
---
 hw/s390x/virtio-ccw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 130535c..ceb6a45 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -266,7 +266,7 @@ static int virtio_ccw_set_vqs(SubchDev *sch, uint64_t addr, 
uint32_t align,
 {
 VirtIODevice *vdev = virtio_ccw_get_vdev(sch);
 
-if (index > VIRTIO_PCI_QUEUE_MAX) {
+if (index >= VIRTIO_PCI_QUEUE_MAX) {
 return -EINVAL;
 }
 
-- 
2.3.4

[Qemu-devel] [PATCH for-2.3 0/4] assorted s390x fixes

[Cornelia Huck]

Hi,

here's what I have pending for 2.3. Paolo noticed two small problems,
and I realized we have some bugs wrt accessing virtqueue indices
when I reviewed Jason's support for more virtqueues patchset.

I'm planning on sending a pull request on monday.

Cornelia Huck (3):
  virtio-ccw: fix range check for SET_VQ
  virtio-ccw: range check in READ_VQ_CONF
  s390x/ipl: avoid sign extension

Paolo Bonzini (1):
  s390x: do not include ram_addr.h

 hw/s390x/ipl.c| 3 +--
 hw/s390x/virtio-ccw.c | 6 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

-- 
2.3.4