date:20170103

Re: [Qemu-devel] vhost-user breaks after 96a3d98.

2017-01-03 Thread Jason Wang




On 2017年01月04日 11:26, Jason Wang wrote:



On 2017年01月04日 00:27, Michael S. Tsirkin wrote:

On Tue, Jan 03, 2017 at 06:28:18PM +0800, Jason Wang wrote:


On 2017年01月03日 11:09, Jason Wang wrote:


On 2016年12月30日 20:41, Flavio Leitner wrote:

Hi,

While I was testing vhost-user using OVS 2.5 and DPDK 2.2.0 in the
host and testpmd dpdk 2.2.0 in the guest, I found that the commit
below breaks the environment and no packets gets into the guest.

dpdk port --> OVS --> vhost-user --> guest --> testpmd
   ^--- drops here ^--- no packets 
here.


commit 96a3d98d2cdbd897ff5ab33427aa4cfb94077665
Author: Jason Wang 
Date:   Mon Aug 1 16:07:58 2016 +0800

  vhost: don't set vring call if no vector
   We used to set vring call fd unconditionally even if guest
driver does
  not use MSIX for this vritqueue at all. This will cause lots of
  unnecessary userspace access and other checks for drivers does
not use
  interrupt at all (e.g virtio-net pmd). So check and clean vring
call
  fd if guest does not use any vector for this virtqueue at
  all.
[...]

Thanks,

Hi Flavio:

Thanks for reporting this issue, could this be a bug of vhost-user? (I
believe virito-net pmd does not use interrupt for rx/tx at all)

Anyway, will try to reproduce it.

Could not reproduce this issue on similar setups (the only 
difference is I
don't create dpdk port) with dpdk 16.11 and ovs.git HEAD. Suspect an 
issue

dpdk. Will try OVS 2.5 + DPDK 2.2.0.

Thanks

Possibly dpdk assumed that call fd must be present unconditionally.
Limit this patch to when protocol is updated? add a new protocol flag?


If this is a bug of dpdk, I tend to fix it (or just disable this patch 
for vhost-user). I'm not sure whether or not it's worthwhile to add a 
new protocol flag which was used to tell qemu that bug X was fixed.


Thanks




Haven't tried but looking at vq_is_ready() in v2.2.0:

static int
vq_is_ready(struct vhost_virtqueue *vq)
{
return vq && vq->desc   &&
   vq->kickfd != -1 &&
   vq->callfd != -1;
}

Which assumes callfd must be set which seems wrong. And this has been 
fixed by


commit fb871d0a4dc1c038a381c524cdb86fe83d21d842
Author: Tetsuya Mukawa 
Date:   Mon Mar 14 17:53:32 2016 +0900

vhost: fix default value of kickfd and callfd

Currently, default values of kickfd and callfd are -1.
If the values are -1, current code guesses kickfd and callfd haven't
been initialized yet. Then vhost library will guess the virtqueue isn't
ready for processing.

But callfd and kickfd will be set as -1 when "--enable-kvm"
isn't specified in QEMU command line. It means we cannot treat -1 as
uninitialized state.

The patch defines -1 and -2 as VIRTIO_INVALID_EVENTFD and
VIRTIO_UNINITIALIZED_EVENTFD, and uses VIRTIO_UNINITIALIZED_EVENTFD for
the default values of kickfd and callfd.

Signed-off-by: Tetsuya Mukawa 
Acked-by: Yuanhan Liu 

Flavio, you could try to backport this to 2.2.0 to see if it fixes your 
issue.


Thanks

[Qemu-devel] [PATCH RFC v3 2/2] block/qapi: reduce the execution time of qmp_query_blockstats

2017-01-03 Thread Dou Liyang

In order to reduce the execution time, this patch optimize
the qmp_query_blockstats():
Remove the next_query_bds function.
Remove the bdrv_query_stats function.
Remove some judgement sentence.

The original qmp_query_blockstats calls next_query_bds to get
the next objects in each loops. In the next_query_bds, it checks
the query_nodes and blk. It also call bdrv_query_stats to get
the stats, In the bdrv_query_stats, it checks blk and bs each
times. This waste more times, which may stall the main loop a
bit. And if the disk is too many and donot use the dataplane
feature, this may affect the performance in main loop thread.

This patch removes that two functions, and makes the structure
clearly.

Signed-off-by: Dou Liyang 
---
 block/qapi.c | 72 +++-
 1 file changed, 28 insertions(+), 44 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index bc622cd..6e1e8bd 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -456,23 +456,6 @@ static BlockStats *bdrv_query_bds_stats(const 
BlockDriverState *bs,
 return s;
 }
 
-static BlockStats *bdrv_query_stats(BlockBackend *blk,
-const BlockDriverState *bs,
-bool query_backing)
-{
-BlockStats *s;
-
-s = bdrv_query_bds_stats(bs, query_backing);
-
-if (blk) {
-s->has_device = true;
-s->device = g_strdup(blk_name(blk));
-bdrv_query_blk_stats(s->stats, blk);
-}
-
-return s;
-}
-
 BlockInfoList *qmp_query_block(Error **errp)
 {
 BlockInfoList *head = NULL, **p_next = 
@@ -496,42 +479,43 @@ BlockInfoList *qmp_query_block(Error **errp)
 return head;
 }
 
-static bool next_query_bds(BlockBackend **blk, BlockDriverState **bs,
-   bool query_nodes)
-{
-if (query_nodes) {
-*bs = bdrv_next_node(*bs);
-return !!*bs;
-}
-
-*blk = blk_next(*blk);
-*bs = *blk ? blk_bs(*blk) : NULL;
-
-return !!*blk;
-}
-
 BlockStatsList *qmp_query_blockstats(bool has_query_nodes,
  bool query_nodes,
  Error **errp)
 {
 BlockStatsList *head = NULL, **p_next = 
-BlockBackend *blk = NULL;
-BlockDriverState *bs = NULL;
+BlockBackend *blk;
+BlockDriverState *bs;
 
 /* Just to be safe if query_nodes is not always initialized */
-query_nodes = has_query_nodes && query_nodes;
-
-while (next_query_bds(, , query_nodes)) {
-BlockStatsList *info = g_malloc0(sizeof(*info));
-AioContext *ctx = blk ? blk_get_aio_context(blk)
-  : bdrv_get_aio_context(bs);
+if (has_query_nodes && query_nodes) {
+for (bs = bdrv_next_node(NULL); bs; bs = bdrv_next_node(bs)) {
+BlockStatsList *info = g_malloc0(sizeof(*info));
+AioContext *ctx = bdrv_get_aio_context(bs);
 
-aio_context_acquire(ctx);
-info->value = bdrv_query_stats(blk, bs, !query_nodes);
-aio_context_release(ctx);
+aio_context_acquire(ctx);
+info->value = bdrv_query_bds_stats(bs, false);
+aio_context_release(ctx);
 
-*p_next = info;
-p_next = >next;
+*p_next = info;
+p_next = >next;
+}
+} else {
+for (blk = blk_next(NULL); blk; blk = blk_next(blk)) {
+BlockStatsList *info = g_malloc0(sizeof(*info));
+AioContext *ctx = blk_get_aio_context(blk);
+
+aio_context_acquire(ctx);
+BlockStats *s = bdrv_query_bds_stats(blk_bs(blk), true);
+s->has_device = true;
+s->device = g_strdup(blk_name(blk));
+bdrv_query_blk_stats(s->stats, blk);
+aio_context_release(ctx);
+
+info->value = s;
+*p_next = info;
+p_next = >next;
+}
 }
 
 return head;
-- 
2.5.5

[Qemu-devel] [PATCH RFC v3 1/2] block/qapi: reduce the coupling between the bdrv_query_stats and bdrv_query_bds_stats

2017-01-03 Thread Dou Liyang

the bdrv_query_stats and bdrv_query_bds_stats functions need to call
each other, that increases the coupling. it also makes the program
complicated and makes some unnecessary judgements.

remove the call from bdrv_query_bds_stats to bdrv_query_stats, just
take some recursion to make it clearly.

avoid judging whether the blk is NULL during querying the bds stats.
it is unnecessary.

Signed-off-by: Dou Liyang 
---
 block/qapi.c | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index a62e862..bc622cd 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -357,10 +357,6 @@ static void bdrv_query_info(BlockBackend *blk, BlockInfo 
**p_info,
 qapi_free_BlockInfo(info);
 }
 
-static BlockStats *bdrv_query_stats(BlockBackend *blk,
-const BlockDriverState *bs,
-bool query_backing);
-
 static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk)
 {
 BlockAcctStats *stats = blk_get_stats(blk);
@@ -428,9 +424,18 @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, 
BlockBackend *blk)
 }
 }
 
-static void bdrv_query_bds_stats(BlockStats *s, const BlockDriverState *bs,
+static BlockStats *bdrv_query_bds_stats(const BlockDriverState *bs,
  bool query_backing)
 {
+BlockStats *s = NULL;
+
+s = g_malloc0(sizeof(*s));
+s->stats = g_malloc0(sizeof(*s->stats));
+
+if (!bs) {
+return s;
+}
+
 if (bdrv_get_node_name(bs)[0]) {
 s->has_node_name = true;
 s->node_name = g_strdup(bdrv_get_node_name(bs));
@@ -440,14 +445,15 @@ static void bdrv_query_bds_stats(BlockStats *s, const 
BlockDriverState *bs,
 
 if (bs->file) {
 s->has_parent = true;
-s->parent = bdrv_query_stats(NULL, bs->file->bs, query_backing);
+s->parent = bdrv_query_bds_stats(bs->file->bs, query_backing);
 }
 
 if (query_backing && bs->backing) {
 s->has_backing = true;
-s->backing = bdrv_query_stats(NULL, bs->backing->bs, query_backing);
+s->backing = bdrv_query_bds_stats(bs->backing->bs, query_backing);
 }
 
+return s;
 }
 
 static BlockStats *bdrv_query_stats(BlockBackend *blk,
@@ -456,17 +462,13 @@ static BlockStats *bdrv_query_stats(BlockBackend *blk,
 {
 BlockStats *s;
 
-s = g_malloc0(sizeof(*s));
-s->stats = g_malloc0(sizeof(*s->stats));
+s = bdrv_query_bds_stats(bs, query_backing);
 
 if (blk) {
 s->has_device = true;
 s->device = g_strdup(blk_name(blk));
 bdrv_query_blk_stats(s->stats, blk);
 }
-if (bs) {
-bdrv_query_bds_stats(s, bs, query_backing);
-}
 
 return s;
 }
-- 
2.5.5

[Qemu-devel] [PATCH RFC v3 0/2] block/qapi: refactor and optimize the qmp_query_blockstats()

2017-01-03 Thread Dou Liyang

Change log v2 -> v3:
 1. Remove the unnecessary code for the bdrv_next_node().
 2. Remove the change of the locking rules.
Even if this change can improve the performance, but it may
effect the consistency.

For the multi-disks guest, we can use the dataplane feature to
hold performance does not drop, if we execute some slow monitor
commands, such as "info blockstats". But, without this feature, 
How to reduce the decline in performance?

These patches aim to refactor the qmp_query_blockstats() and
improve the performance by reducing the running time of it.

There are the two jobs:

1 For the performance:

1.1 the time it takes(ns) in each time:
the disk numbers | 10| 500
-
before these patches | 19429 | 667722 
after these patches  | 18536 | 627945

1.2 the I/O performance is degraded(%) during the monitor:

the disk numbers | 10| 500
-
before these patches | 1.3   | 14.2
after these patches  | 1.0   | 11.3

used the dd command likes this to test: 
dd if=date_1.dat of=date_2.dat conv=fsync oflag=direct bs=1k count=100k.

2 refactor qmp_query_blockstats():

From:

+--+  +-+
 | 1|  | 4.  |
 |next_query_bds|  |bdrv_query_bds_stats +---+
 |  |  | |   |
 +^-+  +-^---+   |
  |  |   |
+-+--+  ++---+   |
| 0. |  | 2. |   |
|qmp_query_blockstats+-->bdrv_query_stats<
||  ||
++  ++---+
 |
   +-v---+
   | 3.  |
   |bdrv_query_blk_stats |
   | |
   +-+

To:

+--+
|  |
   +v---+  |
   +--->  3.|  |
+---+  |   |bdrv_query_bds_stats+--+
| 1.+--+   ||
|   +  ++
|qmp_query_blockstats--+
|   |  |
+---+  |   ++
   |   | 2. |
   +--->|
   |bdrv_query_blk_stats|
   ||
   ++

Dou Liyang (2):
  block/qapi: reduce the coupling between the bdrv_query_stats and
bdrv_query_bds_stats
  block/qapi: reduce the execution time of qmp_query_blockstats

 block/qapi.c | 94 ++--
 1 file changed, 40 insertions(+), 54 deletions(-)

-- 
2.5.5

Re: [Qemu-devel] New Year's starting over ... bsd-user

2017-01-03 Thread Thomas Huth

On 03.01.2017 18:11, Sean Bruno wrote:
> 
> 
> On 01/03/17 09:18, Sean Bruno wrote:
>>
>> I'm pondering where to start with getting FreeBSD's bsd-user code into
>> shape so it could actually be reviewed and accepted now that its sort of
>> working again (signal handling fixed finally).
>>
>> I almost feel like the existing code should be purged, except that it
>> gives a good history (and this seems lazy to me).
>>
>> As a first pass, I guess, I'd like to at least make i386 user run on
>> x86_64.  What would you folks like to see in a first pass?
>>
>> sean
>>
>> ref: https://github.com/seanbruno/qemu-bsd-user/tree/bsd-user
>>
> 
> Primitive example of what I think I should base my patchset on.  Its
> invasive and large.
> 
> https://github.com/seanbruno/qemu-bsd-user/tree/merge1
> 
> That branch, is all the bsd-user changes that are pending in one large
> "splat".  It excludes the new architectures (arm, aarch64, mips, mips64)
> that we are actively using.  i386-bsd-user when compiled statically on
> x86_64 will run a static (rescue) sh ... so, I think that's good.
> x86_64 running on x86_64 just blows up.
> 
> As for sparc/sparc64 ... I'm tempted to delete them as nobody in freebsd
> is actively maintaining them nor do we have any expectation that they
> will work someday.

It's broken ... nobody maintains it ... and we've got too many
unmaintained bit-rotten files in the QEMU tree, so IMHO just go ahead
and send a patch to remove the bsud-user sparc support. If anybody ever
needs it again, they can revert the commit or simply submit a patch with
the fixed code.

 Thomas

Re: [Qemu-devel] [PATCH v2 3/4] migration: disallow migrate_add_blocker during migration

2017-01-03 Thread Ashijeet Acharya

On Fri, Dec 16, 2016 at 4:07 PM, Greg Kurz  wrote:
> On Fri, 16 Dec 2016 15:23:43 +0530
> Ashijeet Acharya  wrote:
>
>> If a migration is already in progress and somebody attempts
>> to add a migration blocker, this should rightly fail.
>>
>> Add an errp parameter and a retcode return value to migrate_add_blocker.
>>
>> Signed-off-by: John Snow 
>> Signed-off-by: Ashijeet Acharya 
>> ---
>>  block/qcow.c  |  6 +-
>>  block/vdi.c   |  6 +-
>>  block/vhdx.c  | 16 ++--
>>  block/vmdk.c  |  7 ++-
>>  block/vpc.c   | 10 +++---
>>  block/vvfat.c | 20 
>>  hw/9pfs/9p.c  | 16 
>>  hw/display/virtio-gpu.c   | 29 -
>>  hw/intc/arm_gic_kvm.c | 16 ++--
>>  hw/intc/arm_gicv3_its_kvm.c   | 18 +++---
>>  hw/intc/arm_gicv3_kvm.c   | 19 ---
>>  hw/misc/ivshmem.c | 11 +++
>>  hw/scsi/vhost-scsi.c  | 25 +++--
>>  hw/virtio/vhost.c |  8 +++-
>>  include/migration/migration.h |  6 +-
>>  migration/migration.c | 35 +--
>>  stubs/migr-blocker.c  |  3 ++-
>>  target-i386/kvm.c | 16 +---
>>  18 files changed, 192 insertions(+), 75 deletions(-)
>>
>> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
>> index faebd91..e72ef43 100644
>> --- a/hw/9pfs/9p.c
>> +++ b/hw/9pfs/9p.c
>> @@ -1013,20 +1013,28 @@ static void coroutine_fn v9fs_attach(void *opaque)
>>  goto out;
>>  }
>>  err += offset;
>> -memcpy(>root_qid, , sizeof(qid));
>> -trace_v9fs_attach_return(pdu->tag, pdu->id,
>> - qid.type, qid.version, qid.path);
>> +
>>  /*
>>   * disable migration if we haven't done already.
>>   * attach could get called multiple times for the same export.
>>   */
>>  if (!s->migration_blocker) {
>> +int ret;
>>  s->root_fid = fid;
>
> Since we may fail now, I guess ^^ should move...
>
>>  error_setg(>migration_blocker,
>> "Migration is disabled when VirtFS export path '%s' is 
>> mounted in the guest using mount_tag '%s'",
>> s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
>> -migrate_add_blocker(s->migration_blocker);
>> +ret = migrate_add_blocker(s->migration_blocker, NULL);
>> +if (ret < 0) {
>> +err = ret;
>> +clunk_fid(s, fid);
>
> Please don't rely on put_fid() to rollback what was done in this
> function. You need to do the following here:
> - error_free(s->migration_blocker)
> - s->migration_blocker = NULL

Okay, I was not aware of this. I have added these two now.

>
>> +goto out;
>> +}
>
>   s->root_fid = fid;
>
> ... here.

Right. I missed out on this one. I have moved it now.

Ashijeet
>
>>  }
>> +
>> +memcpy(>root_qid, , sizeof(qid));
>> +trace_v9fs_attach_return(pdu->tag, pdu->id,
>> + qid.type, qid.version, qid.path);
>>  out:
>>  put_fid(pdu, fidp);
>>  out_nofid:

Re: [Qemu-devel] [PATCH v4 8/8] arm: Add an RX8900 RTC to the ASpeed board

2017-01-03 Thread Andrew Jeffery

On Thu, 2016-12-15 at 16:48 +1100, Alastair D'Silva wrote:
> > From: Alastair D'Silva 
> 
> Connect an RX8900 RTC to i2c12 of the AST2500 SOC at address 0x32
> 
> > Signed-off-by: Alastair D'Silva 
> Signed-off-by: Chris Smart 

Reviewed-by: Andrew Jeffery 

> ---
>  hw/arm/aspeed.c | 29 +
>  1 file changed, 29 insertions(+)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 40c1383..ef63fd0 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -26,6 +26,12 @@ static struct arm_boot_info aspeed_board_binfo = {
>  .nb_cpus = 1,
>  };
>  
> +typedef struct AspeedI2CDevice {
> +const char *type;
> +uint8_t address;
> +int bus;
> +} AspeedI2CDevice;
> +
>  typedef struct AspeedBoardState {
>  AspeedSoCState soc;
>  MemoryRegion ram;
> @@ -37,6 +43,7 @@ typedef struct AspeedBoardConfig {
>  const char *fmc_model;
>  const char *spi_model;
>  uint32_t num_cs;
> +const AspeedI2CDevice *i2c_devices;
>  } AspeedBoardConfig;
>  
>  enum {
> @@ -80,6 +87,11 @@ enum {
>  SCU_AST2500_HW_STRAP_ACPI_ENABLE |  \
>  SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_MASTER))
>  
> +
> +static const AspeedI2CDevice ast2500_i2c_devices[] = {
> +{"rx8900", 0x32, 11}
> +};
> +
>  static const AspeedBoardConfig aspeed_boards[] = {
>  [PALMETTO_BMC] = {
>  .soc_name  = "ast2400-a1",
> @@ -94,6 +106,7 @@ static const AspeedBoardConfig aspeed_boards[] = {
>  .fmc_model = "n25q256a",
>  .spi_model = "mx25l25635e",
>  .num_cs= 1,
> +.i2c_devices = ast2500_i2c_devices,
>  },
>  [ROMULUS_BMC]  = {
>  .soc_name  = "ast2500-a1",
> @@ -104,6 +117,7 @@ static const AspeedBoardConfig aspeed_boards[] = {
>  },
>  };
>  
> +
>  static void aspeed_board_init_flashes(AspeedSMCState *s, const char 
> *flashtype,
>    Error **errp)
>  {
> @@ -130,6 +144,19 @@ static void aspeed_board_init_flashes(AspeedSMCState *s, 
> const char *flashtype,
>  }
>  }
>  
> +static void aspeed_i2c_init(AspeedBoardState *bmc,
> +const AspeedBoardConfig *cfg)
> +{
> +AspeedSoCState *soc = >soc;
> +const AspeedI2CDevice *dev;
> +
> +for (dev = cfg->i2c_devices; dev != NULL && dev->type != NULL; dev++) {
> +I2CBus *i2c_bus = aspeed_i2c_get_bus((DeviceState *)>i2c,
> + dev->bus);
> +(void)i2c_create_slave(i2c_bus, dev->type, dev->address);
> +}
> +}
> +
>  static void aspeed_board_init(MachineState *machine,
>    const AspeedBoardConfig *cfg)
>  {
> @@ -174,6 +201,8 @@ static void aspeed_board_init(MachineState *machine,
>  aspeed_board_binfo.ram_size = ram_size;
>  aspeed_board_binfo.loader_start = sc->info->sdram_base;
>  
> +aspeed_i2c_init(bmc, cfg);
> +
>  arm_load_kernel(ARM_CPU(first_cpu), _board_binfo);
>  }
>  

signature.asc
Description: This is a digitally signed message part

Re: [Qemu-devel] [PATCH v4 7/8] tests: Test all implemented RX8900 functionality

2017-01-03 Thread Andrew Jeffery

Hi Alastair,

Again, small comments below.

On Thu, 2016-12-15 at 16:48 +1100, Alastair D'Silva wrote:
> > From: Alastair D'Silva 
> 
> > Signed-off-by: Alastair D'Silva 
> ---
>  tests/Makefile.include |   2 +
>  tests/rx8900-test.c| 882 
> +
>  2 files changed, 884 insertions(+)
>  create mode 100644 tests/rx8900-test.c
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index e98d3b6..e52e355 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -300,6 +300,7 @@ check-qtest-sparc64-y = tests/endianness-test$(EXESUF)
>  
>  check-qtest-arm-y = tests/tmp105-test$(EXESUF)
>  check-qtest-arm-y += tests/ds1338-test$(EXESUF)
> +check-qtest-arm-y += tests/rx8900-test$(EXESUF)
>  check-qtest-arm-y += tests/m25p80-test$(EXESUF)
>  gcov-files-arm-y += hw/misc/tmp105.c
>  check-qtest-arm-y += tests/virtio-blk-test$(EXESUF)
> @@ -637,6 +638,7 @@ tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o 
> \
>  tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o $(libqos-obj-y)
>  tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
>  tests/ds1338-test$(EXESUF): tests/ds1338-test.o $(libqos-imx-obj-y)
> +tests/rx8900-test$(EXESUF): tests/rx8900-test.o $(libqos-imx-obj-y)
>  tests/m25p80-test$(EXESUF): tests/m25p80-test.o
>  tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
>  tests/q35-test$(EXESUF): tests/q35-test.o $(libqos-pc-obj-y)
> diff --git a/tests/rx8900-test.c b/tests/rx8900-test.c
> new file mode 100644
> index 000..1769659
> --- /dev/null
> +++ b/tests/rx8900-test.c
> @@ -0,0 +1,882 @@
> +/*
> + * QTest testcase for the Epson RX8900SA/CE RTC
> + *
> + * Copyright (c) 2016 IBM Corporation
> + * Authors:
> > + *  Alastair D'Silva 
> + *
> + * This code is licensed under the GPL version 2 or later.  See
> + * the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/timer/rx8900_regs.h"
> +#include "libqtest.h"
> +#include "libqos/i2c.h"
> +#include "qemu/timer.h"
> +
> +#define IMX25_I2C_0_BASE 0x43F8
> +#define RX8900_TEST_ID "rx8900-test"
> +#define RX8900_ADDR 0x32
> +#define RX8900_INTERRUPT_OUT "rx8900-interrupt-out"
> +#define RX8900_FOUT_ENABLE "rx8900-fout-enable"
> +#define RX8900_FOUT "rx8900-fout"
> +
> +static I2CAdapter *i2c;
> +static uint8_t addr;
> +
> +static inline uint8_t bcd2bin(uint8_t x)
> +{
> +return (x & 0x0f) + (x >> 4) * 10;
> +}
> +
> +static inline uint8_t bin2bcd(uint8_t x)
> +{
> +return (x / 10 << 4) | (x % 10);
> +}
> +
> +static void qmp_rx8900_set_temperature(const char *id, double value)
> +{
> +QDict *response;
> +
> +response = qmp("{ 'execute': 'qom-set', 'arguments': { 'path': %s, "
> +   "'property': 'temperature', 'value': %f } }", id, value);
> +g_assert(qdict_haskey(response, "return"));
> +QDECREF(response);
> +}
> +
> +static void qmp_rx8900_set_voltage(const char *id, double value)
> +{
> +QDict *response;
> +
> +response = qmp("{ 'execute': 'qom-set', 'arguments': { 'path': %s, "
> +   "'property': 'voltage', 'value': %f } }", id, value);
> +g_assert(qdict_haskey(response, "return"));
> +QDECREF(response);
> +}
> +
> +/**
> + * Read an RX8900 register
> + * @param reg the address of the register
> + * @return the value of the register
> + */
> +static uint8_t read_register(RX8900Addresses reg)
> +{
> +uint8_t val;
> +uint8_t reg_address = (uint8_t)reg;
> +
> +i2c_send(i2c, addr, _address, 1);
> +i2c_recv(i2c, addr, , 1);
> +
> +return val;
> +}
> +
> +/**
> + * Write to an RX8900 register
> + * @param reg the address of the register
> + * @param val the value to write
> + */
> +static uint8_t write_register(RX8900Addresses reg, uint8_t val)
> +{
> +uint8_t buf[2];
> +
> +buf[0] = reg;
> +buf[1] = val;
> +
> +i2c_send(i2c, addr, buf, 2);
> +
> +return val;
> +}
> +
> +/**
> + * Set bits in a register
> + * @param reg the address of the register
> + * @param mask a mask of the bits to set
> + */
> +static void set_bits_in_register(RX8900Addresses reg, uint8_t mask)
> +{
> +uint8_t value = read_register(reg);
> +value |= mask;
> +write_register(reg, value);
> +}
> +
> +/**
> + * Clear bits in a register
> + * @param reg the address of the register
> + * @param mask a mask of the bits to set
> + */
> +static void clear_bits_in_register(RX8900Addresses reg, uint8_t mask)
> +{
> +uint8_t value = read_register(reg);
> +value &= ~mask;
> +write_register(reg, value);
> +}
> +
> +/**
> + * Read a number of sequential RX8900 registers
> + * @param reg the address of the first register
> + * @param buf (out) an output buffer to stash the register values
> + * @param count the number of registers to read
> + */
> +static void read_registers(RX8900Addresses reg, uint8_t *buf, uint8_t count)
> +{

Re: [Qemu-devel] [PATCH v4 6/8] hw/timer: Add Epson RX8900 RTC support

2017-01-03 Thread Alastair D'Silva

On Wed, 2017-01-04 at 15:29 +1030, Andrew Jeffery wrote:

> Hi Alastair,
> 
> I have some mostly minor comments below.

Ok, I'm removing Chris from the CC list since he's left the team.

> 
> On Thu, 2016-12-15 at 16:48 +1100, Alastair D'Silva wrote:
> > > From: Alastair D'Silva 
> > 
> > This patch adds support for the Epson RX8900 I2C RTC.
> > 
> > The following chip features are implemented:
> >  - RTC (wallclock based, ptimer 10x oversampling to pick up
> > wallclock transitions)
> >  - Time update interrupt (per second/minute, wallclock based)
> >  - Alarms (wallclock based)
> >  - Temperature (set via a property)
> >  - Countdown timer (emulated clock via ptimer)
> >  - FOUT via GPIO (emulated clock via ptimer)
> > 
> > The following chip features are unimplemented:
> >  - Low voltage detection
> >  - i2c timeout
> > 
> > The implementation exports the following named GPIOs:
> > rx8900-interrupt-out
> > rx8900-fout-enable
> > rx8900-fout
> > 
> > > Signed-off-by: Alastair D'Silva 
> > > Signed-off-by: Chris Smart 
> > 
> > ---
> >  default-configs/arm-softmmu.mak |   1 +
> >  hw/timer/Makefile.objs  |   2 +
> >  hw/timer/rx8900.c   | 912
> > 
> >  hw/timer/rx8900_regs.h  | 141 +++
> >  hw/timer/trace-events   |  31 ++
> >  5 files changed, 1087 insertions(+)
> >  create mode 100644 hw/timer/rx8900.c
> >  create mode 100644 hw/timer/rx8900_regs.h
> > 
> > diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-
> > softmmu.mak
> > index 6de3e16..adb600e 100644
> > --- a/default-configs/arm-softmmu.mak
> > +++ b/default-configs/arm-softmmu.mak
> > @@ -29,6 +29,7 @@ CONFIG_SMC91C111=y
> >  CONFIG_ALLWINNER_EMAC=y
> >  CONFIG_IMX_FEC=y
> >  CONFIG_DS1338=y
> > +CONFIG_RX8900=y
> >  CONFIG_PFLASH_CFI01=y
> >  CONFIG_PFLASH_CFI02=y
> >  CONFIG_MICRODRIVE=y
> > diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> > index 7ba8c23..fa028ac 100644
> > --- a/hw/timer/Makefile.objs
> > +++ b/hw/timer/Makefile.objs
> > @@ -3,6 +3,7 @@ common-obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o
> >  common-obj-$(CONFIG_A9_GTIMER) += a9gtimer.o
> >  common-obj-$(CONFIG_CADENCE) += cadence_ttc.o
> >  common-obj-$(CONFIG_DS1338) += ds1338.o
> > +common-obj-$(CONFIG_RX8900) += rx8900.o
> >  common-obj-$(CONFIG_HPET) += hpet.o
> >  common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
> >  common-obj-$(CONFIG_M48T59) += m48t59.o
> > @@ -17,6 +18,7 @@ common-obj-$(CONFIG_IMX) += imx_epit.o
> >  common-obj-$(CONFIG_IMX) += imx_gpt.o
> >  common-obj-$(CONFIG_LM32) += lm32_timer.o
> >  common-obj-$(CONFIG_MILKYMIST) += milkymist-sysctl.o
> > +common-obj-$(CONFIG_RX8900) += rx8900.o
> >  
> >  obj-$(CONFIG_EXYNOS4) += exynos4210_mct.o
> >  obj-$(CONFIG_EXYNOS4) += exynos4210_pwm.o
> > diff --git a/hw/timer/rx8900.c b/hw/timer/rx8900.c
> > new file mode 100644
> > index 000..cb1a2c8
> > --- /dev/null
> > +++ b/hw/timer/rx8900.c
> > @@ -0,0 +1,912 @@
> > +/*
> > + * Epson RX8900SA/CE Realtime Clock Module
> > + *
> > + * Copyright (c) 2016 IBM Corporation
> > + * Authors:
> > > + *  Alastair D'Silva 
> > > + *  Chris Smart 
> > 
> > + *
> > + * This code is licensed under the GPL version 2 or later.  See
> > + * the COPYING file in the top-level directory.
> > + *
> > + * Datasheet available at:
> > + *  https://support.epson.biz/td/api/doc_check.php?dl=app_RX8900CE
> > =en
> > + *
> > + * Not implemented:
> > + *  Implement i2c timeout
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu-common.h"
> > +#include "hw/i2c/i2c.h"
> > +#include "hw/timer/rx8900_regs.h"
> > +#include "hw/ptimer.h"
> > +#include "qemu/main-loop.h"
> > +#include "qemu/bcd.h"
> > +#include "qemu/log.h"
> > +#include "qapi/error.h"
> > +#include "qapi/visitor.h"
> > +#include "trace.h"
> > +
> > +#define TYPE_RX8900 "rx8900"
> > +#define RX8900(obj) OBJECT_CHECK(RX8900State, (obj), TYPE_RX8900)
> > +
> > +typedef struct RX8900State {
> > +I2CSlave parent_obj;
> > +
> > +ptimer_state *sec_timer; /* triggered once per second */
> > +ptimer_state *fout_timer;
> > +ptimer_state *countdown_timer;
> > +bool fout_state;
> > +int64_t offset;
> > +uint8_t weekday; /* Saved for deferred offset calculation, 0-6 
> > */
> > +uint8_t wday_offset;
> > +uint8_t nvram[RX8900_NVRAM_SIZE];
> > +int32_t nvram_offset; /* Wrapped to stay within
> > RX8900_NVRAM_SIZE */
> > +bool addr_byte;
> > +uint8_t last_interrupt_seconds; /* The last time the second
> > timer ticked */
> > +/* the last minute the timer update interrupt was triggered
> > (if enabled) */
> > +uint8_t last_update_interrupt_minutes;
> > +double supply_voltage;
> > +qemu_irq interrupt_pin;
> > +qemu_irq fout_pin;
> > +} RX8900State;
> > +
> > +static const VMStateDescription vmstate_rx8900 = {
> > +.name = "rx8900",
>

Re: [Qemu-devel] [PATCH v4 6/8] hw/timer: Add Epson RX8900 RTC support

2017-01-03 Thread Andrew Jeffery

Hi Alastair,

I have some mostly minor comments below.

On Thu, 2016-12-15 at 16:48 +1100, Alastair D'Silva wrote:
> > From: Alastair D'Silva 
> 
> This patch adds support for the Epson RX8900 I2C RTC.
> 
> The following chip features are implemented:
>  - RTC (wallclock based, ptimer 10x oversampling to pick up
> wallclock transitions)
>  - Time update interrupt (per second/minute, wallclock based)
>  - Alarms (wallclock based)
>  - Temperature (set via a property)
>  - Countdown timer (emulated clock via ptimer)
>  - FOUT via GPIO (emulated clock via ptimer)
> 
> The following chip features are unimplemented:
>  - Low voltage detection
>  - i2c timeout
> 
> The implementation exports the following named GPIOs:
> rx8900-interrupt-out
> rx8900-fout-enable
> rx8900-fout
> 
> > Signed-off-by: Alastair D'Silva 
> > Signed-off-by: Chris Smart 
> ---
>  default-configs/arm-softmmu.mak |   1 +
>  hw/timer/Makefile.objs  |   2 +
>  hw/timer/rx8900.c   | 912 
> 
>  hw/timer/rx8900_regs.h  | 141 +++
>  hw/timer/trace-events   |  31 ++
>  5 files changed, 1087 insertions(+)
>  create mode 100644 hw/timer/rx8900.c
>  create mode 100644 hw/timer/rx8900_regs.h
> 
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 6de3e16..adb600e 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -29,6 +29,7 @@ CONFIG_SMC91C111=y
>  CONFIG_ALLWINNER_EMAC=y
>  CONFIG_IMX_FEC=y
>  CONFIG_DS1338=y
> +CONFIG_RX8900=y
>  CONFIG_PFLASH_CFI01=y
>  CONFIG_PFLASH_CFI02=y
>  CONFIG_MICRODRIVE=y
> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> index 7ba8c23..fa028ac 100644
> --- a/hw/timer/Makefile.objs
> +++ b/hw/timer/Makefile.objs
> @@ -3,6 +3,7 @@ common-obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o
>  common-obj-$(CONFIG_A9_GTIMER) += a9gtimer.o
>  common-obj-$(CONFIG_CADENCE) += cadence_ttc.o
>  common-obj-$(CONFIG_DS1338) += ds1338.o
> +common-obj-$(CONFIG_RX8900) += rx8900.o
>  common-obj-$(CONFIG_HPET) += hpet.o
>  common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
>  common-obj-$(CONFIG_M48T59) += m48t59.o
> @@ -17,6 +18,7 @@ common-obj-$(CONFIG_IMX) += imx_epit.o
>  common-obj-$(CONFIG_IMX) += imx_gpt.o
>  common-obj-$(CONFIG_LM32) += lm32_timer.o
>  common-obj-$(CONFIG_MILKYMIST) += milkymist-sysctl.o
> +common-obj-$(CONFIG_RX8900) += rx8900.o
>  
>  obj-$(CONFIG_EXYNOS4) += exynos4210_mct.o
>  obj-$(CONFIG_EXYNOS4) += exynos4210_pwm.o
> diff --git a/hw/timer/rx8900.c b/hw/timer/rx8900.c
> new file mode 100644
> index 000..cb1a2c8
> --- /dev/null
> +++ b/hw/timer/rx8900.c
> @@ -0,0 +1,912 @@
> +/*
> + * Epson RX8900SA/CE Realtime Clock Module
> + *
> + * Copyright (c) 2016 IBM Corporation
> + * Authors:
> > + *  Alastair D'Silva 
> > + *  Chris Smart 
> + *
> + * This code is licensed under the GPL version 2 or later.  See
> + * the COPYING file in the top-level directory.
> + *
> + * Datasheet available at:
> + *  https://support.epson.biz/td/api/doc_check.php?dl=app_RX8900CE=en
> + *
> + * Not implemented:
> + *  Implement i2c timeout
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "hw/i2c/i2c.h"
> +#include "hw/timer/rx8900_regs.h"
> +#include "hw/ptimer.h"
> +#include "qemu/main-loop.h"
> +#include "qemu/bcd.h"
> +#include "qemu/log.h"
> +#include "qapi/error.h"
> +#include "qapi/visitor.h"
> +#include "trace.h"
> +
> +#define TYPE_RX8900 "rx8900"
> +#define RX8900(obj) OBJECT_CHECK(RX8900State, (obj), TYPE_RX8900)
> +
> +typedef struct RX8900State {
> +I2CSlave parent_obj;
> +
> +ptimer_state *sec_timer; /* triggered once per second */
> +ptimer_state *fout_timer;
> +ptimer_state *countdown_timer;
> +bool fout_state;
> +int64_t offset;
> +uint8_t weekday; /* Saved for deferred offset calculation, 0-6 */
> +uint8_t wday_offset;
> +uint8_t nvram[RX8900_NVRAM_SIZE];
> +int32_t nvram_offset; /* Wrapped to stay within RX8900_NVRAM_SIZE */
> +bool addr_byte;
> +uint8_t last_interrupt_seconds; /* The last time the second timer ticked 
> */
> +/* the last minute the timer update interrupt was triggered (if enabled) 
> */
> +uint8_t last_update_interrupt_minutes;
> +double supply_voltage;
> +qemu_irq interrupt_pin;
> +qemu_irq fout_pin;
> +} RX8900State;
> +
> +static const VMStateDescription vmstate_rx8900 = {
> +.name = "rx8900",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.fields = (VMStateField[]) {
> +VMSTATE_I2C_SLAVE(parent_obj, RX8900State),
> +VMSTATE_PTIMER(sec_timer, RX8900State),
> +VMSTATE_PTIMER(fout_timer, RX8900State),
> +VMSTATE_PTIMER(countdown_timer, RX8900State),
> +VMSTATE_BOOL(fout_state, RX8900State),
> +VMSTATE_INT64(offset, RX8900State),
> +

[Qemu-devel] [PATCH] Crypto: fix leak in ivgen essiv init

2017-01-03 Thread Li Qiang

From: Li Qiang 

On error path, the 'salt' doesn't been freed thus leading
a memory leak. This patch avoid this.

Signed-off-by: Li Qiang 
---
 crypto/ivgen-essiv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/crypto/ivgen-essiv.c b/crypto/ivgen-essiv.c
index 634de63..cba20bd 100644
--- a/crypto/ivgen-essiv.c
+++ b/crypto/ivgen-essiv.c
@@ -48,6 +48,7 @@ static int qcrypto_ivgen_essiv_init(QCryptoIVGen *ivgen,
, ,
errp) < 0) {
 g_free(essiv);
+g_free(salt);
 return -1;
 }
 
-- 
1.8.3.1

[Qemu-devel] [Bug 1317603] Re: qemu-system-ppc does not terminate on VM exit

2017-01-03 Thread Launchpad Bug Tracker

[Expired for qemu (Ubuntu) because there has been no activity for 60
days.]

** Changed in: qemu (Ubuntu)
   Status: Incomplete => Expired

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

Title:
  qemu-system-ppc does not terminate on VM exit

Status in QEMU:
  Won't Fix
Status in qemu package in Ubuntu:
  Expired

Bug description:
  When a VM is created for a p4080-e500mc  ; the VM can not be  rebooted
  or terminated.

  The qemu-system-ppc process must be killed.

  ProblemType: Bug
  DistroRelease: Ubuntu 14.04
  Package: qemu-system-ppc 2.0.0~rc1+dfsg-0ubuntu3.1
  ProcVersionSignature: Ubuntu 3.13.0-24.46-powerpc-e500mc 3.13.9
  Uname: Linux 3.13.9+ ppc
  ApportVersion: 2.14.1-0ubuntu3
  Architecture: powerpc
  Date: Thu May  8 12:20:57 2014
  ProcEnviron:
   TERM=xterm
   PATH=(custom, no user)
   XDG_RUNTIME_DIR=
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  SourcePackage: qemu
  UpgradeStatus: Upgraded to trusty on 2014-04-29 (9 days ago)

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

[Qemu-devel] [Bug 1119686] Re: Incorrect handling of icebp

2017-01-03 Thread Launchpad Bug Tracker

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  Incorrect handling of icebp

Status in QEMU:
  Expired

Bug description:
  Wine conformance suite tests the behavior of various low-level Windows
  API functions. One of the tests involves checking the interaction of
  breakpoints and exceptions, and in particular the 'icebp' breakpoint.
  This test works on a Windows XP machine running either on the metal or
  in VMware ESX but fails when run in QEmu.

  To reproduce the issue grab the attached 'exception.exe' file and run
  it. If you get 'Test failed' lines like below then it means the
  problem is still present:

  exception.c:202: exception 0: 8004 flags:0 addr:003F
  exception.c:208: Test failed: 0: Wrong exception address 003F/003F0001
  exception.c:214: this is the last test seen before the exception
  exception: unhandled exception 8004 at 003F
  exception.c:202: exception 0: c027 flags:2 addr:7C80E0B9
  exception.c:205: Test failed: 0: Wrong exception code c027/8004
  exception.c:208: Test failed: 0: Wrong exception address 7C80E0B9/003F0001

  Note that this bug was not present in QEmu 1.1.2+dfsg-5 (Debian
  Testing) but is now present in 1.4.0~rc0+dfsg-1exp (Debian
  Experimental).

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

[Qemu-devel] [Bug 809912] Re: qemu-kvm -m bigger 4096 aborts with 'Bad ram offset'

2017-01-03 Thread Launchpad Bug Tracker

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  qemu-kvm -m bigger 4096 aborts with 'Bad ram offset'

Status in QEMU:
  Expired

Bug description:
  When I try to start a virtual machine (x86_64 guest on a x86_64 host
  that has 32GB memory, using kvm_amd module, both host and guest
  running linux-2.6.39 kernels) with "qemu-system-x86_64 -cpu host -smp
  2 -m 4096 ...", shortly after the guest kernel starts, qemu aborts
  with a message "Bad ram offset 11811c000".

  With e.g. "-m 3500" (or lower), the virtual machine runs fine.

  I experience this both using qemu-kvm 0.14.1 and a recent version from git
  commit 525e3df73e40290e95743d4c8f8b64d8d9cbe021
  Merge: d589310 75ef849
  Author: Avi Kivity 
  Date:   Mon Jul 4 13:36:06 2011 +0300

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

[Qemu-devel] [Bug 322602] Re: Snapshot usage makes qcow2 image unusable due to large tables

2017-01-03 Thread Launchpad Bug Tracker

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  Snapshot usage makes qcow2 image unusable due to large tables

Status in QEMU:
  Expired

Bug description:
  To reproduce with 0.9.1 and svn:
  - Create a 20G (or some size much greater than system RAM) qcow2 image
  - Inside VM, install some OS, formatting whole drive
  - Create snapshot with savevm
  - Inside VM, reformat and reinstall OS
  - Create snapshot with savevm
  [...]

  Eventually, qemu crashes, then neither qemu-img nor qemu can open the
  image because memory is exhausted.  The reason is that the whole
  refcount_table is loaded into memory, and this refcount_table has now
  become much bigger than the size of memory due to each snapshot taken
  after significant changes to the disk image.

  The refcount_table really needs to be loaded and used in fixed size
  chunks to avoid this problem.  It will only get worse as typical
  storage set modifications during work sessions between snapshots
  outpace the size of system RAM.

  Alternatively, there needs to be a way to "rollback" a snapshot
  without loading the whole disk image normally, so that a snapshot
  which has made the image unusable in this way can be reversed.

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

[Qemu-devel] [Bug 441672] Re: Windos XP BSOD with HP Photosmart usb device attached

2017-01-03 Thread Launchpad Bug Tracker

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  Windos XP BSOD with HP Photosmart usb device attached

Status in QEMU:
  Expired

Bug description:
  https://bugzilla.redhat.com/show_bug.cgi?id=524723 has all the details
  of the problem.

  I was just testing attaching a USB device to see if it really worked, and 
tried my HP Photosmart C5580 All-in-One
  printer/scanner, and the Windows XP box then started getting bluescreens and 
crashing at random
  (fairly short :-) intervals.

  My latest attempt was on a fedora rawhide system with pretty up to date 
software
  (qemu-kvm-0.11.0-2.fc12.x86_64), and the crashes still happen.

  A reply to that bugzilla recommended adding this upstream bug, so here
  it is.

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

[Qemu-devel] [Bug 612297] Re: qemu-kvm fails to detect mouse while Windows 95 setup

2017-01-03 Thread Launchpad Bug Tracker

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  qemu-kvm fails to detect mouse while Windows 95 setup

Status in QEMU:
  Expired

Bug description:
  CPU: AMD Phenom II X4 945
  KVM-Version: qemu-kvm-0.12.4+dfsg (Debian package)
  Kernel: linux-2.6.26.8-rt16
  arch: x86_64
  Guest: Windows 95 B

  I'm trying to install Windows 95 B on qemu-kvm with this options:

  kvm /var/mmn/vm/Win95/Win95.img -name "Windows 95" -M pc-0.12 -m 512M
  -rtc base=localtime -k de -soundhw sb16 -vga cirrus -net
  user,hostname=w95vm -net nic,model=ne2k_pci -boot a -fda
  /var/mmn/vm/floppy/win95B_Drive-D-boot.img -cdrom
  /var/mmn/vm/CD/win95-setup.iso -no-acpi -no-kvm -usb

  I've also tried some other option, but always the same: no ps/2 mouse
  detection. And usb mouse is not supported by Windows 95 B while setup
  process. This is only possible later by installing the extension
  usbsupp.exe after the system setup.

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

[Qemu-devel] [Bug 590552] Re: New default network card doesn't work with tap networking

2017-01-03 Thread Launchpad Bug Tracker

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  New default network card doesn't work with tap networking

Status in QEMU:
  Expired

Bug description:
  Unfortunately, I can provide very little information.

  Hope this will be useful anyway.

  I've upgraded qemu using debian apt to lastest unstable (QEMU PC
  emulator version 0.12.4 (Debian 0.12.4+dfsg-2), Copyright (c)
  2003-2008 Fabrice Bellard): looks like at some point the default
  network card for -net nic option was switched to intel gigabit instead
  of the good old ne2k_pci.

  I was using -net tap -net nic options and my network stopped working.
  When not working,
  - tcpdump on the host shows me taht all packets are sent and received fine 
from guest
  - tcpdump on guest shows that packets from host are NOT received

  obviously, both host tap interface and guest eth0 interfaces, routing
  tables, dns, firewall, etc... are well configured.

  Having banged my head for a while, I finally stopped the host and
  started it again using -net nic,model=ne2k_pci option, then my network
  magically started working again.

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

[Qemu-devel] [Bug 1102027] Re: QED Time travel

2017-01-03 Thread Launchpad Bug Tracker

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  QED Time travel

Status in QEMU:
  Expired

Bug description:
  This night after a reboot of a VM, it was back to 8 Oct. 2012, i've
  lost all data between 8 Oct 2012 and now. I've check the QED file and
  mount on another VM, all seems OK.

  This QED has a raw backfile with the base OS (debian) shared with many
  others QED. It has NO snapshot.

  QEMU emulator version 1.1.2

  Does anyone have a hint ?

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

[Qemu-devel] [Bug 1639225] Re: qcow2 - filesize 8.1Petabyte

2017-01-03 Thread Launchpad Bug Tracker

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  qcow2 - filesize 8.1Petabyte

Status in QEMU:
  Expired

Bug description:
  
  problem :

  Filesystem Size  Used Avail Use%
  Mounted on

  /dev/sdd1  120G   63G   57G  53%
  /storage/kvmstorage4ssd

  # pwd
  /storage/kvmstorage4ssd/images

  # qemu-img info vsys19_ssd1.qcow2 
  image: vsys19_ssd1.qcow2
  file format: qcow2
  virtual size: 20G (21474836480 bytes)
  disk size: 11G
  cluster_size: 65536
  Format specific information:
  compat: 1.1
  lazy refcounts: true
  refcount bits: 16
  corrupt: true
  # ls -lah vsys19_*
  -rw--- 1 root root 8.1P Nov  4 13:16 vsys19_ssd1.qcow2

  # ls -la vsys19_*
  -rw--- 1 root root 9007203702079488 Nov  4 13:16 vsys19_ssd1.qcow2

  # qemu-img check vsys19_ssd1.qcow2 
  qemu-img: Check failed: File too large

  # xfs_repair /dev/sdd1
  Phase 1 - find and verify superblock...
  Phase 2 - using internal log
  - zero log...
  - scan filesystem freespace and inode maps...
  - found root inode chunk
  Phase 3 - for each AG...
  - scan and clear agi unlinked lists...
  - process known inodes and perform inode discovery...
  - agno = 0
  - agno = 1
  - agno = 2
  - agno = 3
  - process newly discovered inodes...
  Phase 4 - check for duplicate blocks...
  - setting up duplicate extent list...
  - check for inodes claiming duplicate blocks...
  - agno = 0
  - agno = 1
  - agno = 2
  - agno = 3
  Phase 5 - rebuild AG headers and trees...
  - reset superblock...
  Phase 6 - check inode connectivity...
  - resetting contents of realtime bitmap and summary inodes
  - traversing filesystem ...
  - traversal finished ...
  - moving disconnected inodes to lost+found ...
  Phase 7 - verify and correct link counts...
  done

  
  # pwd
  /storage/kvmstorage4ssd/images

  




  

  
  guest OS:

  
  # uname -a
  Linux vsys19 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt11-1 (2015-05-24) x86_64 
GNU/Linux
  cat /etc/debian_version 
  stretch/sid

  Nov  4 01:23:26 vsys19 kernel: [7654313.024844] end_request: I/O error, dev 
vdk, sector 8691272
  Nov  4 01:23:26 vsys19 kernel: [7654313.025328] EXT4-fs warning (device 
dm-1): ext4_end_bio:317: I/O error -5 writing to inode 262305 (offset 0 size 
4202496 s
  tarting block 1085897)
  Nov  4 01:23:26 vsys19 kernel: [7654313.025334] Buffer I/O error on device 
dm-1, logical block 1085897
  Nov  4 01:23:26 vsys19 kernel: [7654313.025488] Buffer I/O error on device 
dm-1, logical block 1085898
  Nov  4 01:23:26 vsys19 kernel: [7654313.025632] Buffer I/O error on device 
dm-1, logical block 1085899
  Nov  4 01:23:26 vsys19 kernel: [7654313.025776] Buffer I/O error on device 
dm-1, logical block 1085900
  Nov  4 01:23:26 vsys19 kernel: [7654313.025920] Buffer I/O error on device 
dm-1, logical block 1085901
  Nov  4 01:23:26 vsys19 kernel: [7654313.026064] Buffer I/O error on device 
dm-1, logical block 1085902
  Nov  4 01:23:26 vsys19 kernel: [7654313.026207] Buffer I/O error on device 
dm-1, logical block 1085903
  Nov  4 01:23:26 vsys19 kernel: [7654313.026350] Buffer I/O error on device 
dm-1, logical block 1085904
  Nov  4 01:23:26 vsys19 kernel: [7654313.026500] Buffer I/O error on device 
dm-1, logical block 1085905
  Nov  4 01:23:26 vsys19 kernel: [7654313.028837] Buffer I/O error on device 
dm-1, logical block 1085906
  Nov  4 01:23:26 vsys19 kernel: [7654313.031122] end_request: I/O error, dev 
vdk, sector 8692280
  Nov  4 01:23:26 vsys19 kernel: [7654313.031325] EXT4-fs warning (device 
dm-1): ext4_end_bio:317: I/O error -5 writing to inode 262305 (offset 0 size 
4202496 starting block 1086023)
  Nov  4 01:23:26 vsys19 kernel: [7654313.031388] end_request: I/O error, dev 
vdk, sector 8693288
  Nov  4 01:23:26 vsys19 kernel: [7654313.031527] EXT4-fs warning (device 
dm-1): ext4_end_bio:317: I/O error -5 writing to inode 262305 (offset 0 size 
4202496 starting block 1086149)
  Nov  4 01:23:26 vsys19 kernel: [7654313.031598] end_request: I/O error, dev 
vdk, sector 8694296
  Nov  4 01:23:26 vsys19 kernel: [7654313.031736] EXT4-fs warning (device 
dm-1): ext4_end_bio:317: I/O error -5 writing to inode 262305 (offset 0 size 
4202496 starting block 1086275)
  Nov  4 01:23:26 vsys19 kernel: [7654313.031798] end_request: I/O error, dev 
vdk, sector 8695304
  Nov  4 01:23:26 vsys19 kernel: [7654313.031933] EXT4-fs warning (device 
dm-1): ext4_end_bio:317: I/O error -5 writing to inode 262305 (offset 0 size 
4202496 starting block 1086401)

  KVM host :
  # cat

Re: [Qemu-devel] [PATCH 0/4] QOM'ify work for ppc

2017-01-03 Thread David Gibson

On Tue, Jan 03, 2017 at 10:02:21PM +0800, 赵小强 wrote:
> Hi，david：
> 
>To my understanding，what must be put in the realize function  is
>code which depends on property values. What's the benefit of
>moving memory region initialization into realize function?  I can
>not figure out, can you make some explanations?

If nothing else it's better in realize() for consistency with other
devices.

I'm not familiar enough with the details to be sure, but I also think
it's not safe in instance_init.  Once memory regions are registered,
the device can potentially interact with other devices in the virtual
machine.  realize() is sequenced to expect that, instance_init is not.

> Thanks for your review.
> 
> Best wishes ！
> 
> > 在 2017年1月3日，06:28，David Gibson  写道：
> > 
> >> On Sat, Dec 31, 2016 at 09:18:27AM +0800, xiaoqiang zhao wrote:
> >> This is some QOM'ify work relate with ppc.
> >> See each commit message for details.
> >> 
> >> xiaoqiang zhao (4):
> >>  hw/gpio: QOM'ify mpc8xxx.c
> >>  hw/ppc: QOM'ify e500.c
> >>  hw/ppc: QOM'ify ppce500_spin.c
> >>  hw/ppc: QOM'ify spapr_vio.c
> >> 
> >> hw/gpio/mpc8xxx.c | 20 +++-
> >> hw/ppc/e500.c | 17 -
> >> hw/ppc/ppce500_spin.c | 18 --
> >> hw/ppc/spapr_vio.c|  2 --
> >> 4 files changed, 23 insertions(+), 34 deletions(-)
> > 
> > Patches 1-3 all have the same problem - they move memory region
> > initialization and similar to an instance_init function.  This is not
> > how things are generally done in the qdev model.  Instead that phase
> > of initialization should be done from a dc->realize() function.
> > 
> 
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.9 v3 0/5] Sheepdog cleanups

2017-01-03 Thread Jeff Cody

On Wed, Dec 21, 2016 at 03:07:07PM +0100, Paolo Bonzini wrote:
> 
> 
> On 29/11/2016 12:32, Paolo Bonzini wrote:
> > Cleaning up the code and removing duplication makes it simpler to
> > later adapt it for the multiqueue work.
> > 
> > Tested against sheepdog 1.0.  I also tested taking snapshots and reverting
> > to older snapshots, but the latter only worked with "dog vdi rollback".
> > Neither loadvm nor qemu-img worked for me.
> > 
> > Paolo
> > 
> > v1->v2: placate patchew
> > v2->v3: rebase
> > 
> > Paolo Bonzini (5):
> >   sheepdog: remove unused cancellation support
> >   sheepdog: reorganize coroutine flow
> >   sheepdog: do not use BlockAIOCB
> >   sheepdog: simplify inflight_aio_head management
> >   sheepdog: reorganize check for overlapping requests
> > 
> >  block/sheepdog.c | 289 
> > ---
> >  1 file changed, 83 insertions(+), 206 deletions(-)
> > 
> 
> 2.8 is now out, so ping.
>

I don't have a functional sheepdog setup at the moment; have you tested
these patches, or should I set up a test rig for them? (I am guessing I
should do the latter; either way, I'll pull the patches in once I or someone
else has tested them).

Re: [Qemu-devel] vhost-user breaks after 96a3d98.

2017-01-03 Thread Jason Wang




On 2017年01月04日 00:27, Michael S. Tsirkin wrote:

On Tue, Jan 03, 2017 at 06:28:18PM +0800, Jason Wang wrote:


On 2017年01月03日 11:09, Jason Wang wrote:


On 2016年12月30日 20:41, Flavio Leitner wrote:

Hi,

While I was testing vhost-user using OVS 2.5 and DPDK 2.2.0 in the
host and testpmd dpdk 2.2.0 in the guest, I found that the commit
below breaks the environment and no packets gets into the guest.

dpdk port --> OVS --> vhost-user --> guest --> testpmd
   ^--- drops here ^--- no packets here.

commit 96a3d98d2cdbd897ff5ab33427aa4cfb94077665
Author: Jason Wang 
Date:   Mon Aug 1 16:07:58 2016 +0800

  vhost: don't set vring call if no vector
   We used to set vring call fd unconditionally even if guest
driver does
  not use MSIX for this vritqueue at all. This will cause lots of
  unnecessary userspace access and other checks for drivers does
not use
  interrupt at all (e.g virtio-net pmd). So check and clean vring
call
  fd if guest does not use any vector for this virtqueue at
  all.
[...]

Thanks,

Hi Flavio:

Thanks for reporting this issue, could this be a bug of vhost-user? (I
believe virito-net pmd does not use interrupt for rx/tx at all)

Anyway, will try to reproduce it.


Could not reproduce this issue on similar setups (the only difference is I
don't create dpdk port) with dpdk 16.11 and ovs.git HEAD. Suspect an issue
dpdk. Will try OVS 2.5 + DPDK 2.2.0.

Thanks

Possibly dpdk assumed that call fd must be present unconditionally.
Limit this patch to when protocol is updated? add a new protocol flag?


If this is a bug of dpdk, I tend to fix it (or just disable this patch 
for vhost-user). I'm not sure whether or not it's worthwhile to add a 
new protocol flag which was used to tell qemu that bug X was fixed.


Thanks

[Qemu-devel] [PATCH v2] qemu-thread: fix qemu_thread_set_name() race in qemu_thread_create()

2017-01-03 Thread zhanghailiang

From: Caoxinhua 

QEMU will crash with the follow backtrace if the new created thread exited 
before
we call qemu_thread_set_name() for it.

  (gdb) bt
  #0 0x7f9a68b095d7 in __GI_raise (sig=sig@entry=6) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:56
  #1 0x7f9a68b0acc8 in __GI_abort () at abort.c:90
  #2 0x7f9a69cda389 in PAT_abort () from /usr/lib64/libuvpuserhotfix.so
  #3 0x7f9a69cdda0d in patchIllInsHandler () from 
/usr/lib64/libuvpuserhotfix.so
  #4 
  #5 pthread_setname_np (th=140298470549248, name=name@entry=0x8cc74a 
"io-task-worker") at ../nptl/sysdeps/unix/sysv/linux/pthread_setname.c:49
  #6 0x007f5f20 in qemu_thread_set_name 
(thread=thread@entry=0x7ffd2ac09680, name=name@entry=0x8cc74a "io-task-worker") 
at util/qemu_thread_posix.c:459
  #7 0x007f679e in qemu_thread_create 
(thread=thread@entry=0x7ffd2ac09680, name=name@entry=0x8cc74a 
"io-task-worker",start_routine=start_routine@entry=0x7c1300 
, arg=arg@entry=0x7f99b8001720, mode=mode@entry=1) at 
util/qemu_thread_posix.c:498
  #8 0x007c15b6 in qio_task_run_in_thread 
(task=task@entry=0x7f99b80033d0, worker=worker@entry=0x7bd920 
, opaque=0x7f99b8003370, destroy=0x7c6220 
) at io/task.c:133
  #9 0x007bda04 in qio_channel_socket_connect_async 
(ioc=0x7f99b80014c0, addr=0x37235d0, callback=callback@entry=0x54ad00 
, opaque=opaque@entry=0x38118b0, 
destroy=destroy@entry=0x0) at io/channel_socket.c:191
  #10 0x005487f6 in socket_reconnect_timeout (opaque=0x38118b0) at 
qemu_char.c:4402
  #11 0x7f9a6a1533b3 in g_timeout_dispatch () from 
/usr/lib64/libglib-2.0.so.0
  #12 0x7f9a6a15299a in g_main_context_dispatch () from 
/usr/lib64/libglib-2.0.so.0
  #13 0x00747386 in glib_pollfds_poll () at main_loop.c:227
  #14 0x00747424 in os_host_main_loop_wait (timeout=40400) at 
main_loop.c:272
  #15 0x00747575 in main_loop_wait (nonblocking=nonblocking@entry=0) at 
main_loop.c:520
  #16 0x00557d31 in main_loop () at vl.c:2170
  #17 0x0041c8b7 in main (argc=, argv=, 
envp=) at vl.c:5083

Let's detach the new thread after calling qemu_thread_set_name().

Signed-off-by: Caoxinhua 
---
v2:
 Fix missing title
---
 util/qemu-thread-posix.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index d20cdde..d31793d 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -481,12 +481,6 @@ void qemu_thread_create(QemuThread *thread, const char 
*name,
 if (err) {
 error_exit(err, __func__);
 }
-if (mode == QEMU_THREAD_DETACHED) {
-err = pthread_attr_setdetachstate(, PTHREAD_CREATE_DETACHED);
-if (err) {
-error_exit(err, __func__);
-}
-}
 
 /* Leave signal handling to the iothread.  */
 sigfillset();
@@ -499,6 +493,12 @@ void qemu_thread_create(QemuThread *thread, const char 
*name,
 qemu_thread_set_name(thread, name);
 }
 
+if (mode == QEMU_THREAD_DETACHED) {
+err = pthread_detach(thread->thread);
+if (err) {
+error_exit(err, __func__);
+}
+}
 pthread_sigmask(SIG_SETMASK, , NULL);
 
 pthread_attr_destroy();
-- 
1.8.3.1

Re: [Qemu-devel] [Resend PATCH] The QEMU crashes since invoking qemu_thread_set_name(), the backtrace is:

2017-01-03 Thread Hailiang Zhang


Oops, I deleted the original  title carelessly, will send V2. Thanks.

On 2017/1/4 0:06, Stefan Hajnoczi wrote:

Please follow the conventions for commit messages.  QEMU commit messages
start with a prefix that identifies the affected component.

(Use git-log(1) to check what prefix other people have used for the
file(s) you touched.)

The commit message should be brief and doesn't need to be a full
sentence.

For example:

   qemu-thread: fix qemu_thread_set_name() race in qemu_thread_create()

Thanks,
Stefan

Re: [Qemu-devel] [PATCH v14 0/2] virtio-crypto: virtio crypto device specification

2017-01-03 Thread Gonglei (Arei)

Hi Stefan,

>
> Subject: Re: [Qemu-devel] [PATCH v14 0/2] virtio-crypto: virtio crypto device
> specification
> 
> On Mon, Dec 26, 2016 at 02:38:29AM +, Gonglei (Arei) wrote:
> > Both Alex and Stefan mentioned that the process of create/close a session
> > makes we have a least one full round-trip cost from guest to host to guest
> > to be able to send any data for symmetric algorithms. It gets ourself into
> > synchronization troubles in some scenarios like a web server handling lots
> > of small requests whose algorithms and keys are different.
> >
> > Because the virtio crypto specification has not been voted yet and v15 is on
> the way.
> > I'd like to make some changes in order to support those scenarios better.
> That means
> > we will support one-blob request (no sessions) as well for symmetric
> > algorithms, including HASH, MAC services. The benefit is obvious for
> > HASH service because it's usually a one-blob operation.
> >
> > The main changes will be:
> >  1) using the flag property of struct virtio_crypto_op_header to identify 
> > the
> > type of crypto request. Aka Is it a session-based or non-session
> request?
> > The flag is not used currently, so we can make use of it.
> >
> >  2) extending virtio_crypto_*_para structures, for example, add the content
> of
> > struct virtio_crypto_cipher_session_para into struct
> virtio_crypto_cipher_para.
> > It's true that will increase the size of each crypto request after this
> change.
> >
> > Does it make sense? Thanks!
> 
> That sounds good.  Hopefully many crypto API users only use a single
> operation and can therefore benefit from this optimization.
> 
Thanks for your feedback. I'll start this work.

Regards,
-Gonglei

Re: [Qemu-devel] [PATCH V5 1/2] Add a new qmp command to start/stop replication

2017-01-03 Thread Stefano Stabellini

On Tue, 27 Dec 2016, Zhang Chen wrote:
> We can call this qmp command to start/stop replication outside of qemu.
> Like Xen colo need this function.
> 
> Signed-off-by: Zhang Chen 
> Signed-off-by: Wen Congyang 
> Reviewed-by: Eric Blake 

Reviewed-by: Stefano Stabellini 


> ---
>  docs/qmp-commands.txt | 18 ++
>  migration/colo.c  | 23 +++
>  qapi-schema.json  | 19 +++
>  3 files changed, 60 insertions(+)
> 
> diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt
> index abf210a..d182147 100644
> --- a/docs/qmp-commands.txt
> +++ b/docs/qmp-commands.txt
> @@ -432,6 +432,24 @@ Example:
>   "arguments": { "enable": true } }
>  <- { "return": {} }
>  
> +xen-set-replication
> +---
> +
> +Enable or disable replication.
> +
> +Arguments:
> +
> +- "enable": Enable it or disable it.
> +- "primary": True for primary or false for secondary.
> +- "failover": Enable failover when stopping replication, but cannot be
> +  specified if 'enable' is true (optional, default false).
> +
> +Example:
> +
> +-> { "execute": "xen-set-replication",
> + "arguments": {"enable": true, "primary": false} }
> +<- { "return": {} }
> +
>  migrate
>  ---
>  
> diff --git a/migration/colo.c b/migration/colo.c
> index 93c85c5..6fc2ade 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -19,6 +19,8 @@
>  #include "qemu/error-report.h"
>  #include "qapi/error.h"
>  #include "migration/failover.h"
> +#include "replication.h"
> +#include "qmp-commands.h"
>  
>  #define COLO_BUFFER_BASE_SIZE (4 * 1024 * 1024)
>  
> @@ -104,6 +106,27 @@ void colo_do_failover(MigrationState *s)
>  }
>  }
>  
> +void qmp_xen_set_replication(bool enable, bool primary,
> + bool has_failover, bool failover,
> + Error **errp)
> +{
> +ReplicationMode mode = primary ?
> +   REPLICATION_MODE_PRIMARY :
> +   REPLICATION_MODE_SECONDARY;
> +
> +if (has_failover && enable) {
> +error_setg(errp, "Parameter 'failover' is only for"
> +   " stopping replication");
> +return;
> +}
> +
> +if (enable) {
> +replication_start_all(mode, errp);
> +} else {
> +replication_stop_all(failover, failover ? NULL : errp);
> +}
> +}
> +
>  static void colo_send_message(QEMUFile *f, COLOMessage msg,
>Error **errp)
>  {
> diff --git a/qapi-schema.json b/qapi-schema.json
> index f3e9bfc..78802f4 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4676,6 +4676,25 @@
>  { 'command': 'xen-load-devices-state', 'data': {'filename': 'str'} }
>  
>  ##
> +# @xen-set-replication
> +#
> +# Enable or disable replication.
> +#
> +# @enable: true to enable, false to disable.
> +#
> +# @primary: true for primary or false for secondary.
> +#
> +# @failover: #optional true to do failover, false to stop. but cannot be
> +#specified if 'enable' is true. default value is false.
> +#
> +# Returns: nothing.
> +#
> +# Since: 2.9
> +##
> +{ 'command': 'xen-set-replication',
> +  'data': { 'enable': 'bool', 'primary': 'bool', '*failover' : 'bool' } }
> +
> +##
>  # @GICCapability:
>  #
>  # The struct describes capability for a specific GIC (Generic
> -- 
> 2.7.4
> 
> 
> 
>

Re: [Qemu-devel] [PATCH 2/6] vga: increase priority of 0xa0000 memory region

2017-01-03 Thread David Gibson

On Tue, Jan 03, 2017 at 11:37:20PM +0100, Hervé Poussineau wrote:
> Le 03/01/2017 à 00:02, David Gibson a écrit :
> > On Thu, Dec 29, 2016 at 11:12:12PM +0100, Hervé Poussineau wrote:
> > > VGA device registers vram as BAR 0. If this BAR is activated as a very 
> > > low address which
> > > crosses 0xa-0xb, low memory region is not accessible anymore.
> > > 
> > > This fixes display on PReP machine if we enable PCI mapping at
> > > address 0.
> > 
> > This commit message needs more information.  What exactly is the VGA
> > BAR colliding with?  Why does the other thing have higher priority?
> > Why is it safe for the VGA BAR to override the other thing?  Why is
> > this safe on all platforms?
> 
> VGA has (basically) two memory regions:
> - the legacy one, from 0xa to 0xb
> - a memory region describing the whole VRAM, configurable with PCI BAR 0.

Yes, that I know.  Note that both the fixed 0xa and the BAR value
are offsets with *PCI* memory space, which may not be the same as
addresses in system memory space.  PCI memory space is identity mapped
into system address space on x86, IIRC, but it's not on spapr.  AFAICT
it's not identity mapped on PReP either (see raven_pcihost_initfn()).
So the VGA legacy window will actually be at 0xc00a and BAR0 at
(0xc000 + BAR value) in the _system_ address space if I'm reading
the PReP PCI bridge code correctly.

> In QEMU, mapping PCI at address 0 is not permitted 
> (MachineClass->pci_allow_0_address is false by default),

I believe this is because of the identity map of PCI memory space on
x86 (well, PC, strictly speaking).  Because that's not the case on
PReP, it should probably set allow_0_address = true.

> except on arm/virt and ppc/spapr. So, this is usually not a problem as all 
> PCI BARs (including video PCI BAR 0) are not mapped at
> address 0, and so both memory regions can't collide.
> 
> When trying Linux on ppc/40p (introduced later in this patchset), I saw that 
> Linux assigns
> PCI BAR addresses in the order of PCI devices detection, and VGA PCI BAR 0 
> ends up at address 0.

Hmm.  You're saying that Linux is assigning BAR 0 so that it collides
with the legacy memory region?  That sounds like a Linux bug.
Or.. does Linux always assign BARs on PReP, or only if the firmware
doesn't?  Maybe the guest firmware needs to assign the BARs properly
to avoid this collision.

> However, Linux assumes that 0xa-0xb is still available to drive the 
> VGA card in text mode.
> Without changing the region priority, VGA output is garbled (as writes 
> directly go to VGA memory).
> When increasing the region priority, VGA output is restored (as writes go to 
> legacy address space).

Right.  But by increasing the priority, you're effectively making a
hole in BAR0.  That sounds like it will cause chaos if after boot you
load a "real" VGA driver which tries to use the full video RAM through
BAR0 instead of via the legacy area.

> arm/virt by default doesn't have a PCI VGA card.
> ppc/spapr may probably have the same problem as ppc/40p. However, as VGA PCI 
> BAR 0 and VGA legacy space
> both have a priority of 1, it may probably have a problem. I only need to 
> convince Linux to use address 0 for VGA PCI BAR.
> 
> Hervé
> 
> > 
> > > Signed-off-by: Hervé Poussineau 
> > > ---
> > >  hw/display/vga.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/display/vga.c b/hw/display/vga.c
> > > index 2a88b3c..c573f35 100644
> > > --- a/hw/display/vga.c
> > > +++ b/hw/display/vga.c
> > > @@ -2265,7 +2265,7 @@ void vga_init(VGACommonState *s, Object *obj, 
> > > MemoryRegion *address_space,
> > >  memory_region_add_subregion_overlap(address_space,
> > >  0x000a,
> > >  vga_io_memory,
> > > -1);
> > > +2);
> > >  memory_region_set_coalescing(vga_io_memory);
> > >  if (init_vga_ports) {
> > >  portio_list_init(>vga_port_list, obj, vga_ports, s, "vga");
> > 
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 4/6] prep: QOM'ify System I/O

2017-01-03 Thread David Gibson

On Tue, Jan 03, 2017 at 11:51:27PM +0100, Hervé Poussineau wrote:
> Le 03/01/2017 à 00:03, David Gibson a écrit :
> > On Thu, Dec 29, 2016 at 11:12:14PM +0100, Hervé Poussineau wrote:
> > > Part of the functionality is copied from hw/ppc/prep.c.
> > > Also add support for board identification/equipment registers.
> > 
> > Needs more detail in the commit message.  What is system I/O?  what is
> > it for?
> 
> System I/O is a PPC PReP device which allows access to motherboard devices, 
> living in the 0x800-0x8ff memory range.
> It is described in "PowerPC Reference platform Specification", available at
> ftp://ftp.software.ibm.com/rs6000/technology/spec/ (files srp1*) in section 
> 6.1.5: "I/O Device Mapping"
> 
> > 
> > The 1-line summary is also misleading; "QOM'ify" suggests you are
> > changing an existing device to use QOM conventions, but no existing
> > device is removed here.  Is this actually something new, or is it a
> > duplicate QOMified version of something else?  If so, what?
> 
> It is a partial duplicate of System I/O device available in hw/ppc/prep.c .
> The new one I'm adding doesn't have all the Motorola-specific registers, and 
> follows a known specification.
> 
> The existing one should be deprecated and removed with the 'prep'
> machine.


Ok, so fold this information into the commit message and resend.

> 
> Hervé
> 
> > > 
> > > Signed-off-by: Hervé Poussineau 
> > > ---
> > >  hw/ppc/Makefile.objs   |   1 +
> > >  hw/ppc/prep_systemio.c | 302 
> > > +
> > >  hw/ppc/trace-events|   4 +
> > >  3 files changed, 307 insertions(+)
> > >  create mode 100644 hw/ppc/prep_systemio.c
> > > 
> > > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> > > index 8025129..db72297 100644
> > > --- a/hw/ppc/Makefile.objs
> > > +++ b/hw/ppc/Makefile.objs
> > > @@ -16,6 +16,7 @@ obj-y += ppc405_boards.o ppc4xx_devs.o ppc405_uc.o 
> > > ppc440_bamboo.o
> > >  obj-y += ppc4xx_pci.o
> > >  # PReP
> > >  obj-$(CONFIG_PREP) += prep.o
> > > +obj-$(CONFIG_PREP) += prep_systemio.o
> > >  # OldWorld PowerMac
> > >  obj-$(CONFIG_MAC) += mac_oldworld.o
> > >  # NewWorld PowerMac
> > > diff --git a/hw/ppc/prep_systemio.c b/hw/ppc/prep_systemio.c
> > > new file mode 100644
> > > index 000..449056c
> > > --- /dev/null
> > > +++ b/hw/ppc/prep_systemio.c
> > > @@ -0,0 +1,302 @@
> > > +/*
> > > + * QEMU PReP System I/O emulation
> > > + *
> > > + * Copyright (c) 2016 Herve Poussineau
> > > + *
> > > + * Permission is hereby granted, free of charge, to any person obtaining 
> > > a copy
> > > + * of this software and associated documentation files (the "Software"), 
> > > to deal
> > > + * in the Software without restriction, including without limitation the 
> > > rights
> > > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
> > > sell
> > > + * copies of the Software, and to permit persons to whom the Software is
> > > + * furnished to do so, subject to the following conditions:
> > > + *
> > > + * The above copyright notice and this permission notice shall be 
> > > included in
> > > + * all copies or substantial portions of the Software.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> > > EXPRESS OR
> > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
> > > MERCHANTABILITY,
> > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT 
> > > SHALL
> > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
> > > OTHER
> > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
> > > ARISING FROM,
> > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> > > DEALINGS IN
> > > + * THE SOFTWARE.
> > > + */
> > > +
> > > +#include "qemu/osdep.h"
> > > +#include "hw/isa/isa.h"
> > > +#include "exec/address-spaces.h"
> > > +#include "qemu/error-report.h" /* for error_report() */
> > > +#include "sysemu/sysemu.h" /* for vm_stop() */
> > > +#include "cpu.h"
> > > +#include "trace.h"
> > > +
> > > +#define TYPE_PREP_SYSTEMIO "prep-systemio"
> > > +#define PREP_SYSTEMIO(obj) \
> > > +OBJECT_CHECK(PrepSystemIoState, (obj), TYPE_PREP_SYSTEMIO)
> > > +
> > > +/* Bit as defined in PowerPC Reference Plaform v1.1, sect. 6.1.5, p. 132 
> > > */
> > > +#define PREP_BIT(n) (1 << (7 - (n)))
> > > +
> > > +typedef struct PrepSystemIoState {
> > > +ISADevice parent_obj;
> > > +MemoryRegion ppc_parity_mem;
> > > +
> > > +qemu_irq non_contiguous_io_map_irq;
> > > +uint8_t sreset; /* 0x0092 */
> > > +uint8_t equipment; /* 0x080c */
> > > +uint8_t system_control; /* 0x081c */
> > > +uint8_t iomap_type; /* 0x0850 */
> > > +uint8_t ibm_planar_id; /* 0x0852 */
> > > +qemu_irq softreset_irq;
> > > +PortioList portio;
> > > +} PrepSystemIoState;
> > > +
> > > +/* PORT 0092 -- Special Port 92 (Read/Write) */
> > > +
> > > +enum {
> > > +PORT0092_SOFTRESET  = PREP_BIT(7),
>

Re: [Qemu-devel] [PATCH 5/6] prep: add IBM RS/6000 7020 (40p) memory controller

2017-01-03 Thread David Gibson

On Tue, Jan 03, 2017 at 11:55:10PM +0100, Hervé Poussineau wrote:
> Le 03/01/2017 à 05:57, David Gibson a écrit :
> > On Thu, Dec 29, 2016 at 11:12:15PM +0100, Hervé Poussineau wrote:
> > > Signed-off-by: Hervé Poussineau 
> > > ---
> > >  default-configs/ppc-softmmu.mak |   1 +
> > >  hw/ppc/Makefile.objs|   1 +
> > >  hw/ppc/rs6000_mc.c  | 232 
> > > 
> > >  hw/ppc/trace-events |   7 ++
> > >  4 files changed, 241 insertions(+)
> > >  create mode 100644 hw/ppc/rs6000_mc.c
> > > 
> > > diff --git a/default-configs/ppc-softmmu.mak 
> > > b/default-configs/ppc-softmmu.mak
> > > index d4d0f9b..e567658 100644
> > > --- a/default-configs/ppc-softmmu.mak
> > > +++ b/default-configs/ppc-softmmu.mak
> > > @@ -47,3 +47,4 @@ CONFIG_LIBDECNUMBER=y
> > >  # For PReP
> > >  CONFIG_MC146818RTC=y
> > >  CONFIG_ISA_TESTDEV=y
> > > +CONFIG_RS6000_MC=y
> > > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
> > > index db72297..0012934 100644
> > > --- a/hw/ppc/Makefile.objs
> > > +++ b/hw/ppc/Makefile.objs
> > > @@ -17,6 +17,7 @@ obj-y += ppc4xx_pci.o
> > >  # PReP
> > >  obj-$(CONFIG_PREP) += prep.o
> > >  obj-$(CONFIG_PREP) += prep_systemio.o
> > > +obj-${CONFIG_RS6000_MC} += rs6000_mc.o
> > >  # OldWorld PowerMac
> > >  obj-$(CONFIG_MAC) += mac_oldworld.o
> > >  # NewWorld PowerMac
> > > diff --git a/hw/ppc/rs6000_mc.c b/hw/ppc/rs6000_mc.c
> > > new file mode 100644
> > > index 000..c684421
> > > --- /dev/null
> > > +++ b/hw/ppc/rs6000_mc.c
> > > @@ -0,0 +1,232 @@
> > > +/*
> > > + * QEMU RS/6000 memory controller
> > > + *
> > > + * Copyright (c) 2016 Hervé Poussineau
> > > + *
> > > + * This program is free software: you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation, either version 2 of the License, or
> > > + * (at your option) version 3 or any later version.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > + * GNU General Public License for more details.
> > > + *
> > > + * You should have received a copy of the GNU General Public License
> > > + * along with this program.  If not, see .
> > > + */
> > > +
> > > +#include "qemu/osdep.h"
> > > +#include "hw/isa/isa.h"
> > > +#include "exec/address-spaces.h"
> > > +#include "hw/boards.h"
> > > +#include "qapi/error.h"
> > > +#include "trace.h"
> > > +
> > > +#define TYPE_RS6000MC "rs6000-mc"
> > > +#define RS6000MC_DEVICE(obj) \
> > > +OBJECT_CHECK(RS6000MCState, (obj), TYPE_RS6000MC)
> > > +
> > > +typedef struct RS6000MCState {
> > > +ISADevice parent_obj;
> > > +/* see US patent 5,684,979 for details (expired 2001-11-04) */
> > > +uint32_t ram_size;
> > > +bool autoconfigure;
> > > +MemoryRegion simm[6];
> > > +unsigned int simm_size[6];
> > > +uint32_t end_address[8];
> > > +uint8_t port0820_index;
> > > +PortioList portio;
> > > +} RS6000MCState;
> > > +
> > > +/* P0RT 0803 -- SIMM ID Register (32/8 MB) (Read Only) */
> > > +
> > > +static uint32_t rs6000mc_port0803_read(void *opaque, uint32_t addr)
> > > +{
> > > +RS6000MCState *s = opaque;
> > > +uint32_t val = 0;
> > > +int socket;
> > > +
> > > +/* (1 << socket) indicates 32 MB SIMM at given socket */
> > > +for (socket = 0; socket < 6; socket++) {
> > > +if (s->simm_size[socket] == 32) {
> > > +val |= (1 << socket);
> > > +}
> > > +}
> > > +
> > > +trace_rs6000mc_id_read(addr, val);
> > > +return val;
> > > +}
> > > +
> > > +/* PORT 0804 -- SIMM Presence Register (Read Only) */
> > > +
> > > +static uint32_t rs6000mc_port0804_read(void *opaque, uint32_t addr)
> > > +{
> > > +RS6000MCState *s = opaque;
> > > +uint32_t val = 0xff;
> > > +int socket;
> > > +
> > > +/* (1 << socket) indicates SIMM absence at given socket */
> > > +for (socket = 0; socket < 6; socket++) {
> > > +if (s->simm_size[socket]) {
> > > +val &= ~(1 << socket);
> > > +}
> > > +}
> > > +s->port0820_index = 0;
> > > +
> > > +trace_rs6000mc_presence_read(addr, val);
> > > +return val;
> > > +}
> > > +
> > > +/* Memory Controller Size Programming Register */
> > > +
> > > +static uint32_t rs6000mc_port0820_read(void *opaque, uint32_t addr)
> > > +{
> > > +RS6000MCState *s = opaque;
> > > +uint32_t val = s->end_address[s->port0820_index] & 0x1f;
> > > +s->port0820_index = (s->port0820_index + 1) & 7;
> > > +trace_rs6000mc_size_read(addr, val);
> > > +return val;
> > > +}
> > > +
> > > +static void rs6000mc_port0820_write(void *opaque, uint32_t addr, 
> > > uint32_t val)
> > > +{
> > > +RS6000MCState *s = opaque;
> > > +uint8_t

Re: [Qemu-devel] [PATCH 5/6] prep: add IBM RS/6000 7020 (40p) memory controller

2017-01-03 Thread Hervé Poussineau


Le 03/01/2017 à 05:57, David Gibson a écrit :

On Thu, Dec 29, 2016 at 11:12:15PM +0100, Hervé Poussineau wrote:

Signed-off-by: Hervé Poussineau 
---
 default-configs/ppc-softmmu.mak |   1 +
 hw/ppc/Makefile.objs|   1 +
 hw/ppc/rs6000_mc.c  | 232 
 hw/ppc/trace-events |   7 ++
 4 files changed, 241 insertions(+)
 create mode 100644 hw/ppc/rs6000_mc.c

diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index d4d0f9b..e567658 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -47,3 +47,4 @@ CONFIG_LIBDECNUMBER=y
 # For PReP
 CONFIG_MC146818RTC=y
 CONFIG_ISA_TESTDEV=y
+CONFIG_RS6000_MC=y
diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index db72297..0012934 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -17,6 +17,7 @@ obj-y += ppc4xx_pci.o
 # PReP
 obj-$(CONFIG_PREP) += prep.o
 obj-$(CONFIG_PREP) += prep_systemio.o
+obj-${CONFIG_RS6000_MC} += rs6000_mc.o
 # OldWorld PowerMac
 obj-$(CONFIG_MAC) += mac_oldworld.o
 # NewWorld PowerMac
diff --git a/hw/ppc/rs6000_mc.c b/hw/ppc/rs6000_mc.c
new file mode 100644
index 000..c684421
--- /dev/null
+++ b/hw/ppc/rs6000_mc.c
@@ -0,0 +1,232 @@
+/*
+ * QEMU RS/6000 memory controller
+ *
+ * Copyright (c) 2016 Hervé Poussineau
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) version 3 or any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "hw/isa/isa.h"
+#include "exec/address-spaces.h"
+#include "hw/boards.h"
+#include "qapi/error.h"
+#include "trace.h"
+
+#define TYPE_RS6000MC "rs6000-mc"
+#define RS6000MC_DEVICE(obj) \
+OBJECT_CHECK(RS6000MCState, (obj), TYPE_RS6000MC)
+
+typedef struct RS6000MCState {
+ISADevice parent_obj;
+/* see US patent 5,684,979 for details (expired 2001-11-04) */
+uint32_t ram_size;
+bool autoconfigure;
+MemoryRegion simm[6];
+unsigned int simm_size[6];
+uint32_t end_address[8];
+uint8_t port0820_index;
+PortioList portio;
+} RS6000MCState;
+
+/* P0RT 0803 -- SIMM ID Register (32/8 MB) (Read Only) */
+
+static uint32_t rs6000mc_port0803_read(void *opaque, uint32_t addr)
+{
+RS6000MCState *s = opaque;
+uint32_t val = 0;
+int socket;
+
+/* (1 << socket) indicates 32 MB SIMM at given socket */
+for (socket = 0; socket < 6; socket++) {
+if (s->simm_size[socket] == 32) {
+val |= (1 << socket);
+}
+}
+
+trace_rs6000mc_id_read(addr, val);
+return val;
+}
+
+/* PORT 0804 -- SIMM Presence Register (Read Only) */
+
+static uint32_t rs6000mc_port0804_read(void *opaque, uint32_t addr)
+{
+RS6000MCState *s = opaque;
+uint32_t val = 0xff;
+int socket;
+
+/* (1 << socket) indicates SIMM absence at given socket */
+for (socket = 0; socket < 6; socket++) {
+if (s->simm_size[socket]) {
+val &= ~(1 << socket);
+}
+}
+s->port0820_index = 0;
+
+trace_rs6000mc_presence_read(addr, val);
+return val;
+}
+
+/* Memory Controller Size Programming Register */
+
+static uint32_t rs6000mc_port0820_read(void *opaque, uint32_t addr)
+{
+RS6000MCState *s = opaque;
+uint32_t val = s->end_address[s->port0820_index] & 0x1f;
+s->port0820_index = (s->port0820_index + 1) & 7;
+trace_rs6000mc_size_read(addr, val);
+return val;
+}
+
+static void rs6000mc_port0820_write(void *opaque, uint32_t addr, uint32_t val)
+{
+RS6000MCState *s = opaque;
+uint8_t socket = val >> 5;
+uint32_t end_address = val & 0x1f;
+
+trace_rs6000mc_size_write(addr, val);
+s->end_address[socket] = end_address;
+if (socket > 0 && socket < 7) {
+if (s->simm_size[socket - 1]) {
+uint32_t size;
+uint32_t start_address = 0;
+if (socket > 1) {
+start_address = s->end_address[socket - 1];
+}
+
+size = end_address - start_address;
+memory_region_set_enabled(>simm[socket - 1], size != 0);
+memory_region_set_address(>simm[socket - 1],
+  start_address * 8 * 1024 * 1024);
+}
+}
+}
+
+/* Read Memory Parity Error */
+
+enum {
+PORT0841_NO_ERROR_DETECTED = 0x01,
+};
+
+static uint32_t rs6000mc_port0841_read(void *opaque, uint32_t addr)
+{
+uint32_t val = PORT0841_NO_ERROR_DETECTED;
+

Re: [Qemu-devel] [PATCH 4/6] prep: QOM'ify System I/O

2017-01-03 Thread Hervé Poussineau


Le 03/01/2017 à 00:03, David Gibson a écrit :

On Thu, Dec 29, 2016 at 11:12:14PM +0100, Hervé Poussineau wrote:

Part of the functionality is copied from hw/ppc/prep.c.
Also add support for board identification/equipment registers.


Needs more detail in the commit message.  What is system I/O?  what is
it for?


System I/O is a PPC PReP device which allows access to motherboard devices, 
living in the 0x800-0x8ff memory range.
It is described in "PowerPC Reference platform Specification", available at
ftp://ftp.software.ibm.com/rs6000/technology/spec/ (files srp1*) in section 6.1.5: 
"I/O Device Mapping"



The 1-line summary is also misleading; "QOM'ify" suggests you are
changing an existing device to use QOM conventions, but no existing
device is removed here.  Is this actually something new, or is it a
duplicate QOMified version of something else?  If so, what?


It is a partial duplicate of System I/O device available in hw/ppc/prep.c .
The new one I'm adding doesn't have all the Motorola-specific registers, and 
follows a known specification.

The existing one should be deprecated and removed with the 'prep' machine.

Hervé



Signed-off-by: Hervé Poussineau 
---
 hw/ppc/Makefile.objs   |   1 +
 hw/ppc/prep_systemio.c | 302 +
 hw/ppc/trace-events|   4 +
 3 files changed, 307 insertions(+)
 create mode 100644 hw/ppc/prep_systemio.c

diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index 8025129..db72297 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -16,6 +16,7 @@ obj-y += ppc405_boards.o ppc4xx_devs.o ppc405_uc.o 
ppc440_bamboo.o
 obj-y += ppc4xx_pci.o
 # PReP
 obj-$(CONFIG_PREP) += prep.o
+obj-$(CONFIG_PREP) += prep_systemio.o
 # OldWorld PowerMac
 obj-$(CONFIG_MAC) += mac_oldworld.o
 # NewWorld PowerMac
diff --git a/hw/ppc/prep_systemio.c b/hw/ppc/prep_systemio.c
new file mode 100644
index 000..449056c
--- /dev/null
+++ b/hw/ppc/prep_systemio.c
@@ -0,0 +1,302 @@
+/*
+ * QEMU PReP System I/O emulation
+ *
+ * Copyright (c) 2016 Herve Poussineau
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/isa/isa.h"
+#include "exec/address-spaces.h"
+#include "qemu/error-report.h" /* for error_report() */
+#include "sysemu/sysemu.h" /* for vm_stop() */
+#include "cpu.h"
+#include "trace.h"
+
+#define TYPE_PREP_SYSTEMIO "prep-systemio"
+#define PREP_SYSTEMIO(obj) \
+OBJECT_CHECK(PrepSystemIoState, (obj), TYPE_PREP_SYSTEMIO)
+
+/* Bit as defined in PowerPC Reference Plaform v1.1, sect. 6.1.5, p. 132 */
+#define PREP_BIT(n) (1 << (7 - (n)))
+
+typedef struct PrepSystemIoState {
+ISADevice parent_obj;
+MemoryRegion ppc_parity_mem;
+
+qemu_irq non_contiguous_io_map_irq;
+uint8_t sreset; /* 0x0092 */
+uint8_t equipment; /* 0x080c */
+uint8_t system_control; /* 0x081c */
+uint8_t iomap_type; /* 0x0850 */
+uint8_t ibm_planar_id; /* 0x0852 */
+qemu_irq softreset_irq;
+PortioList portio;
+} PrepSystemIoState;
+
+/* PORT 0092 -- Special Port 92 (Read/Write) */
+
+enum {
+PORT0092_SOFTRESET  = PREP_BIT(7),
+PORT0092_LE_MODE= PREP_BIT(6),
+};
+
+static void prep_port0092_write(void *opaque, uint32_t addr, uint32_t val)
+{
+PrepSystemIoState *s = opaque;
+
+trace_prep_systemio_write(addr, val);
+
+if ((val & PORT0092_SOFTRESET) != 0) {
+qemu_irq_raise(s->softreset_irq);
+s->sreset = 1;
+} else {
+qemu_irq_lower(s->softreset_irq);
+s->sreset = 0;
+}
+
+if ((val & PORT0092_LE_MODE) != 0) {
+/* XXX Not supported yet */
+error_report("little-endian mode not supported");
+vm_stop(RUN_STATE_PAUSED);
+} else {
+/* Nothing to do */
+}
+}
+
+static uint32_t prep_port0092_read(void *opaque, uint32_t addr)
+{
+PrepSystemIoState *s = opaque;
+/* XXX LE mode unsupported */
+

Re: [Qemu-devel] [PATCH 2/6] vga: increase priority of 0xa0000 memory region

2017-01-03 Thread Hervé Poussineau


Le 03/01/2017 à 00:02, David Gibson a écrit :

On Thu, Dec 29, 2016 at 11:12:12PM +0100, Hervé Poussineau wrote:

VGA device registers vram as BAR 0. If this BAR is activated as a very low 
address which
crosses 0xa-0xb, low memory region is not accessible anymore.

This fixes display on PReP machine if we enable PCI mapping at
address 0.


This commit message needs more information.  What exactly is the VGA
BAR colliding with?  Why does the other thing have higher priority?
Why is it safe for the VGA BAR to override the other thing?  Why is
this safe on all platforms?


VGA has (basically) two memory regions:
- the legacy one, from 0xa to 0xb
- a memory region describing the whole VRAM, configurable with PCI BAR 0.

In QEMU, mapping PCI at address 0 is not permitted 
(MachineClass->pci_allow_0_address is false by default),
except on arm/virt and ppc/spapr. So, this is usually not a problem as all PCI 
BARs (including video PCI BAR 0) are not mapped at
address 0, and so both memory regions can't collide.

When trying Linux on ppc/40p (introduced later in this patchset), I saw that 
Linux assigns
PCI BAR addresses in the order of PCI devices detection, and VGA PCI BAR 0 ends 
up at address 0.
However, Linux assumes that 0xa-0xb is still available to drive the VGA 
card in text mode.
Without changing the region priority, VGA output is garbled (as writes directly 
go to VGA memory).
When increasing the region priority, VGA output is restored (as writes go to 
legacy address space).

arm/virt by default doesn't have a PCI VGA card.
ppc/spapr may probably have the same problem as ppc/40p. However, as VGA PCI 
BAR 0 and VGA legacy space
both have a priority of 1, it may probably have a problem. I only need to 
convince Linux to use address 0 for VGA PCI BAR.

Hervé




Signed-off-by: Hervé Poussineau 
---
 hw/display/vga.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/vga.c b/hw/display/vga.c
index 2a88b3c..c573f35 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -2265,7 +2265,7 @@ void vga_init(VGACommonState *s, Object *obj, 
MemoryRegion *address_space,
 memory_region_add_subregion_overlap(address_space,
 0x000a,
 vga_io_memory,
-1);
+2);
 memory_region_set_coalescing(vga_io_memory);
 if (init_vga_ports) {
 portio_list_init(>vga_port_list, obj, vga_ports, s, "vga");

Re: [Qemu-devel] vhost-user breaks after 96a3d98.

2017-01-03 Thread Michael S. Tsirkin

On Tue, Jan 03, 2017 at 03:33:12PM -0200, Flavio Leitner wrote:
> On Tue, 3 Jan 2017 18:28:18 +0800
> Jason Wang  wrote:
> 
> > On 2017年01月03日 11:09, Jason Wang wrote:
> > >
> > >
> > > On 2016年12月30日 20:41, Flavio Leitner wrote:  
> > >> Hi,
> > >>
> > >> While I was testing vhost-user using OVS 2.5 and DPDK 2.2.0 in the
> > >> host and testpmd dpdk 2.2.0 in the guest, I found that the commit
> > >> below breaks the environment and no packets gets into the guest.
> > >>
> > >> dpdk port --> OVS --> vhost-user --> guest --> testpmd
> > >>   ^--- drops here ^--- no packets here.
> > >>
> > >> commit 96a3d98d2cdbd897ff5ab33427aa4cfb94077665
> > >> Author: Jason Wang 
> > >> Date:   Mon Aug 1 16:07:58 2016 +0800
> > >>
> > >>  vhost: don't set vring call if no vector
> > >>   We used to set vring call fd unconditionally even if guest 
> > >> driver does
> > >>  not use MSIX for this vritqueue at all. This will cause lots of
> > >>  unnecessary userspace access and other checks for drivers does 
> > >> not use
> > >>  interrupt at all (e.g virtio-net pmd). So check and clean vring 
> > >> call
> > >>  fd if guest does not use any vector for this virtqueue at
> > >>  all.
> > >> [...]
> > >>
> > >> Thanks,  
> > >
> > > Hi Flavio:
> > >
> > > Thanks for reporting this issue, could this be a bug of vhost-user? (I 
> > > believe virito-net pmd does not use interrupt for rx/tx at all)
> > >
> > > Anyway, will try to reproduce it.
> > >  
> > 
> > Could not reproduce this issue on similar setups (the only difference is 
> > I don't create dpdk port) with dpdk 16.11 and ovs.git HEAD. Suspect an 
> > issue dpdk. Will try OVS 2.5 + DPDK 2.2.0.
> 
> Yeah, that's the combo I am testing and seeing the issue.  I found the
> commit after bisecting qemu and then confirmed by testing up to the
> previous commit (works okay) and then the commit above (fails).
> 
> I still have my test environment available, so I would be able to test
> any patch you might have.
> 
> Thanks,
> -- 
> Flavio

Could you pls try to test dpdk git head and bisect that to find
what fixes the issue?

-- 
MST

Re: [Qemu-devel] [PATCH v3] intel_iommu: allow dynamic switch of IOMMU region

2017-01-03 Thread Alex Williamson

On Mon, 26 Dec 2016 10:45:55 +0800
Jason Wang  wrote:

> On 2016年12月23日 11:26, Peter Xu wrote:
> > On Thu, Dec 22, 2016 at 07:34:10PM +0800, Jason Wang wrote:  
> >>
> >> On 2016年12月22日 19:04, Peter Xu wrote:  
> >>> On Thu, Dec 22, 2016 at 05:52:58PM +0800, Jason Wang wrote:  
>  On 2016年12月22日 17:48, Peter Xu wrote:  
> >   /* Handle Translation Enable/Disable */
> >   static void vtd_handle_gcmd_te(IntelIOMMUState *s, bool en)
> >   {
> > +if (s->dmar_enabled == en) {
> > +return;
> > +}
> > +
> >   VTD_DPRINTF(CSR, "Translation Enable %s", (en ? "on" : "off"));
> >   if (en) {
> > @@ -1196,6 +1237,8 @@ static void vtd_handle_gcmd_te(IntelIOMMUState 
> > *s, bool en)
> >   /* Ok - report back to driver */
> >   vtd_set_clear_mask_long(s, DMAR_GSTS_REG, VTD_GSTS_TES, 0);
> >   }
> > +
> > +vtd_switch_address_space_all(s, en);
> >   }  
>  We may need something like notifier here to tell e.g vhost to stop device
>  IOTLB. (Since it's likely this series were applied after device IOTLB
>  patches)  
> >>> Yes, I missed vhost case.
> >>>
> >>> To notify vhost, IMO we should be able to use memory listeners just
> >>> like how vfio devices do (please refer to vfio_memory_listener).  
> >> Just for switching? This seems an overkill since we don't depends on it for
> >> all other things. Guest should setup correct mappings by explicitly notify
> >> device IOTLB. A quick glance at ATS spec, for enabling and disabling, looks
> >> like it was done through enable bit of ASTctl instead of here.
> >>
> >> So we are probably ok here :)  
> > So we need a more general way to notify ATS about this, right? (e.g.,
> > for future devices that support ATS as well, not only vhost)  
> 
> Yes, if we want to make ATS visible to guest. But looks like it needs 
> more e.g VFIO support for device IOTLB invalidation.

ATS is already visible for vfio-pci devices, it will be enabled if the
host IOMMU supports ATS.  If the host IOMMU does not support ATS,
enabling it on the device by the guest will generate unsupported
requests.  This is yet another case, along with address width, where
it's potentially really complicated and not amenable to hotplug to have
a guest IOMMU.  AFAICT, there's nothing to be added to vfio for device
iotlb invalidations, this occurs automatically between the host IOMMU
and device when a DMA unmap occurs.  Thanks,

Alex

[Qemu-devel] [PATCH] [m25p80] Abort in case we overrun the internal data buffer

2017-01-03 Thread Jean-Christophe Dubois

Signed-off-by: Jean-Christophe Dubois 
---
 hw/block/m25p80.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index d29ff4c..6c374cf 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -28,6 +28,7 @@
 #include "hw/ssi/ssi.h"
 #include "qemu/bitops.h"
 #include "qemu/log.h"
+#include "qemu/error-report.h"
 #include "qapi/error.h"
 
 #ifndef M25P80_ERR_DEBUG
@@ -376,6 +377,8 @@ typedef enum {
 MAN_GENERIC,
 } Manufacturer;
 
+#define _INTERNAL_DATA_SIZE 16
+
 typedef struct Flash {
 SSISlave parent_obj;
 
@@ -386,7 +389,7 @@ typedef struct Flash {
 int page_size;
 
 uint8_t state;
-uint8_t data[16];
+uint8_t data[_INTERNAL_DATA_SIZE];
 uint32_t len;
 uint32_t pos;
 uint8_t needed_bytes;
@@ -1114,6 +1117,12 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t 
tx)
 
 case STATE_COLLECTING_DATA:
 case STATE_COLLECTING_VAR_LEN_DATA:
+
+if (s->len >= _INTERNAL_DATA_SIZE) {
+error_report("Bug - Write overrun internal data buffer");
+abort();
+}
+
 s->data[s->len] = (uint8_t)tx;
 s->len++;
 
@@ -1123,6 +1132,12 @@ static uint32_t m25p80_transfer8(SSISlave *ss, uint32_t 
tx)
 break;
 
 case STATE_READING_DATA:
+
+if (s->pos >= _INTERNAL_DATA_SIZE) {
+error_report("Bug - Read overrun internal data buffer");
+abort();
+}
+
 r = s->data[s->pos];
 s->pos++;
 if (s->pos == s->len) {
@@ -1195,7 +1210,7 @@ static const VMStateDescription vmstate_m25p80 = {
 .pre_save = m25p80_pre_save,
 .fields = (VMStateField[]) {
 VMSTATE_UINT8(state, Flash),
-VMSTATE_UINT8_ARRAY(data, Flash, 16),
+VMSTATE_UINT8_ARRAY(data, Flash, _INTERNAL_DATA_SIZE),
 VMSTATE_UINT32(len, Flash),
 VMSTATE_UINT32(pos, Flash),
 VMSTATE_UINT8(needed_bytes, Flash),
-- 
2.9.3

[Qemu-devel] [PATCH v3] [i.MX] fix CS handling during SPI access.

2017-01-03 Thread Jean-Christophe Dubois

The i.MX SPI device was not de-asserting the CS line at the end of
memory access.

This triggered a SIGSEGV in Qemu when the sabrelite emulator was acessing
a SPI flash memory.

Whit this path the CS signal is correctly asserted and deasserted arround
memory access.

Assertion level is now based on SPI device configuration.

This was tested by:
* booting linux on Sabrelite Qemu emulator.
* booting xvisor on Sabrelite Qemu emultor.

Signed-off-by: Jean-Christophe Dubois 
---

Changes since v1:
* Fix coding style issue.

Changes since v2:
* get SS line polarity from config reg.

 hw/ssi/imx_spi.c | 42 ++
 include/hw/ssi/imx_spi.h |  2 ++
 2 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
index e4e395f..3cbf279 100644
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -141,6 +141,18 @@ static bool imx_spi_channel_is_master(IMXSPIState *s)
 return (mode & (1 << imx_spi_selected_channel(s))) ? true : false;
 }
 
+static uint8_t imx_spi_channel_pol(IMXSPIState *s, uint8_t channel)
+{
+uint8_t pol = EXTRACT(s->regs[ECSPI_CONFIGREG], ECSPI_CONFIGREG_SS_POL);
+
+return pol & (1 << imx_spi_selected_channel(s)) ? 1 : 0;
+}
+
+static uint8_t imx_spi_current_channel_pol(IMXSPIState *s)
+{
+return imx_spi_channel_pol(s, imx_spi_selected_channel(s));
+}
+
 static bool imx_spi_is_multiple_master_burst(IMXSPIState *s)
 {
 uint8_t wave = EXTRACT(s->regs[ECSPI_CONFIGREG], ECSPI_CONFIGREG_SS_CTL);
@@ -152,13 +164,16 @@ static bool imx_spi_is_multiple_master_burst(IMXSPIState 
*s)
 
 static void imx_spi_flush_txfifo(IMXSPIState *s)
 {
-uint32_t tx;
-uint32_t rx;
-
 DPRINTF("Begin: TX Fifo Size = %d, RX Fifo Size = %d\n",
 fifo32_num_used(>tx_fifo), fifo32_num_used(>rx_fifo));
 
+/* Activate the requested CS line */
+qemu_set_irq(s->cs_lines[imx_spi_selected_channel(s)],
+ imx_spi_current_channel_pol(s));
+
 while (!fifo32_is_empty(>tx_fifo)) {
+uint32_t tx;
+uint32_t rx = 0;
 int tx_burst = 0;
 int index = 0;
 
@@ -178,8 +193,6 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
 
 tx_burst = MIN(s->burst_length, 32);
 
-rx = 0;
-
 while (tx_burst) {
 uint8_t byte = tx & 0xff;
 
@@ -221,6 +234,12 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
 s->regs[ECSPI_STATREG] |= ECSPI_STATREG_TC;
 }
 
+/* Deselect all SS lines if transfert if completed */
+if (s->regs[ECSPI_STATREG] & ECSPI_STATREG_TC) {
+qemu_set_irq(s->cs_lines[imx_spi_selected_channel(s)],
+ !imx_spi_current_channel_pol(s));
+}
+
 /* TODO: We should also use TDR and RDR bits */
 
 DPRINTF("End: TX Fifo Size = %d, RX Fifo Size = %d\n",
@@ -230,6 +249,7 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
 static void imx_spi_reset(DeviceState *dev)
 {
 IMXSPIState *s = IMX_SPI(dev);
+uint32_t i;
 
 DPRINTF("\n");
 
@@ -243,6 +263,11 @@ static void imx_spi_reset(DeviceState *dev)
 imx_spi_update_irq(s);
 
 s->burst_length = 0;
+
+/* Disable all CS lines */
+for (i = 0; i < 4; i++) {
+qemu_set_irq(s->cs_lines[i], !imx_spi_channel_pol(s, i));
+}
 }
 
 static uint64_t imx_spi_read(void *opaque, hwaddr offset, unsigned size)
@@ -359,15 +384,8 @@ static void imx_spi_write(void *opaque, hwaddr offset, 
uint64_t value,
 }
 
 if (imx_spi_channel_is_master(s)) {
-int i;
-
 /* We are in master mode */
 
-for (i = 0; i < 4; i++) {
-qemu_set_irq(s->cs_lines[i],
- i == imx_spi_selected_channel(s) ? 0 : 1);
-}
-
 if ((value & change_mask & ECSPI_CONREG_SMC) &&
 !fifo32_is_empty(>tx_fifo)) {
 /* SMC bit is set and TX FIFO has some slots filled in */
diff --git a/include/hw/ssi/imx_spi.h b/include/hw/ssi/imx_spi.h
index 7103953..b9b9819 100644
--- a/include/hw/ssi/imx_spi.h
+++ b/include/hw/ssi/imx_spi.h
@@ -46,6 +46,8 @@
 /* ECSPI_CONFIGREG */
 #define ECSPI_CONFIGREG_SS_CTL_SHIFT 8
 #define ECSPI_CONFIGREG_SS_CTL_LENGTH 4
+#define ECSPI_CONFIGREG_SS_POL_SHIFT 12
+#define ECSPI_CONFIGREG_SS_POL_LENGTH 4
 
 /* ECSPI_INTREG */
 #define ECSPI_INTREG_TEEN (1 << 0)
-- 
2.9.3

Re: [Qemu-devel] [PATCH] [M25P80] Make sure not to overrun the internal data buffer.

2017-01-03 Thread Jean-Christophe DUBOIS


Le 03/01/2017 à 18:08, mar.krzeminski a écrit :


You may still want to harden the m25p80 code to make sure it doesn't 
overrun its internal buffer.


Yeap, for me it is a bug.
I understand you will not finish this patch, won't you ?:)


I could give it a shot.

JC

Re: [Qemu-devel] [PATCH v1 01/15] block: expose crypto option names / defs to other drivers

2017-01-03 Thread Eric Blake

On 01/03/2017 12:27 PM, Daniel P. Berrange wrote:
> The block/crypto.c defines a set of QemuOpts that provide
> parameters for encryption. This will also be needed by
> the qcow/qcow2 integration, so expose the relevant pieces
> in a new block/crypto.h header.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  block/crypto.c | 61 +++
>  block/crypto.h | 91 
> ++
>  2 files changed, 102 insertions(+), 50 deletions(-)
>  create mode 100644 block/crypto.h
> 

Reviewed-by: Eric Blake 

-- 
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] win32: use glib gpoll if glib >= 2.50

2017-01-03 Thread Stefan Weil


On 01/03/17 20:19, Marc-André Lureau wrote:

A fix has been committed in upstream glib commit
210a9796f78eb90f76f1bd6a304e9fea05e97617.
(See also related bug https://bugzilla.gnome.org/show_bug.cgi?id=764415)

It is desirable to use the glib version instead of qemu copy, since it
provides more debugging facilities (G_MAIN_POLL_DEBUG etc), and
hopefully has a better maintainance. Hopefully, we can drop the qemu
copy in a few years.

Signed-off-by: Marc-André Lureau 
---
 include/glib-compat.h | 2 +-
 util/oslib-win32.c| 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/glib-compat.h b/include/glib-compat.h
index acf254d2a0..0cd24ffbe9 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -39,7 +39,7 @@ static inline gint64 qemu_g_get_monotonic_time(void)
 #define g_get_monotonic_time() qemu_g_get_monotonic_time()
 #endif

-#ifdef _WIN32
+#if defined(_WIN32) && !GLIB_CHECK_VERSION(2, 50, 0)
 /*
  * g_poll has a problem on Windows when using
  * timeouts < 10ms, so use wrapper.
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index d09863cc9d..0b1890fd33 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -327,6 +327,7 @@ char *qemu_get_exec_dir(void)
 return g_strdup(exec_dir);
 }

+#if !GLIB_CHECK_VERSION(2, 50, 0)
 /*
  * The original implementation of g_poll from glib has a problem on Windows
  * when using timeouts < 10 ms.
@@ -530,6 +531,7 @@ gint g_poll(GPollFD *fds, guint nfds, gint timeout)

 return retval;
 }
+#endif

 int getpagesize(void)
 {



Thanks for this patch.
I think it can be applied via qemu-trivial (cc'ed).

Reviewed-by: Stefan Weil

Re: [Qemu-devel] [PATCH 2/2] vmxnet3: VMStatify rx/tx q_descr and int_state

2017-01-03 Thread Dr. David Alan Gilbert

* Dmitry Fleytman (dmi...@daynix.com) wrote:
> > 
> > On 16 Dec 2016, at 14:19 PM, Dr. David Alan Gilbert  
> > wrote:
> > 
> > * Dr. David Alan Gilbert (git) (dgilb...@redhat.com) wrote:
> >> From: "Dr. David Alan Gilbert" 
> >> 
> >> Fairly simple mechanical conversion of all fields.
> >> 
> >> TODO
> >> The problem is vmxnet3-ring size/cell_size/next are declared as size_t
> >> but written as 32bit.
> > 
> > Oops, I should have removed that warning in the commit message when
> > I added the 1st patch in.
> 
> 
> 
> Acked-by: Dmitry Fleytman 

Any reason for Acked-by rather than Reviewed-by ?
Are you going to pull those? 

Dave

> 
> > 
> > Dave
> > 
> >> Signed-off-by: Dr. David Alan Gilbert 
> >> ---
> >> hw/net/vmxnet3.c | 272 
> >> ++-
> >> 1 file changed, 90 insertions(+), 182 deletions(-)
> >> 
> >> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> >> index 7853174..4f7dbaf 100644
> >> --- a/hw/net/vmxnet3.c
> >> +++ b/hw/net/vmxnet3.c
> >> @@ -2403,155 +2403,87 @@ static const VMStateDescription 
> >> vmxstate_vmxnet3_mcast_list = {
> >> }
> >> };
> >> 
> >> -static void vmxnet3_get_ring_from_file(QEMUFile *f, Vmxnet3Ring *r)
> >> -{
> >> -r->pa = qemu_get_be64(f);
> >> -r->size = qemu_get_be32(f);
> >> -r->cell_size = qemu_get_be32(f);
> >> -r->next = qemu_get_be32(f);
> >> -r->gen = qemu_get_byte(f);
> >> -}
> >> -
> >> -static void vmxnet3_put_ring_to_file(QEMUFile *f, Vmxnet3Ring *r)
> >> -{
> >> -qemu_put_be64(f, r->pa);
> >> -qemu_put_be32(f, r->size);
> >> -qemu_put_be32(f, r->cell_size);
> >> -qemu_put_be32(f, r->next);
> >> -qemu_put_byte(f, r->gen);
> >> -}
> >> -
> >> -static void vmxnet3_get_tx_stats_from_file(QEMUFile *f,
> >> -struct UPT1_TxStats *tx_stat)
> >> -{
> >> -tx_stat->TSOPktsTxOK = qemu_get_be64(f);
> >> -tx_stat->TSOBytesTxOK = qemu_get_be64(f);
> >> -tx_stat->ucastPktsTxOK = qemu_get_be64(f);
> >> -tx_stat->ucastBytesTxOK = qemu_get_be64(f);
> >> -tx_stat->mcastPktsTxOK = qemu_get_be64(f);
> >> -tx_stat->mcastBytesTxOK = qemu_get_be64(f);
> >> -tx_stat->bcastPktsTxOK = qemu_get_be64(f);
> >> -tx_stat->bcastBytesTxOK = qemu_get_be64(f);
> >> -tx_stat->pktsTxError = qemu_get_be64(f);
> >> -tx_stat->pktsTxDiscard = qemu_get_be64(f);
> >> -}
> >> -
> >> -static void vmxnet3_put_tx_stats_to_file(QEMUFile *f,
> >> -struct UPT1_TxStats *tx_stat)
> >> -{
> >> -qemu_put_be64(f, tx_stat->TSOPktsTxOK);
> >> -qemu_put_be64(f, tx_stat->TSOBytesTxOK);
> >> -qemu_put_be64(f, tx_stat->ucastPktsTxOK);
> >> -qemu_put_be64(f, tx_stat->ucastBytesTxOK);
> >> -qemu_put_be64(f, tx_stat->mcastPktsTxOK);
> >> -qemu_put_be64(f, tx_stat->mcastBytesTxOK);
> >> -qemu_put_be64(f, tx_stat->bcastPktsTxOK);
> >> -qemu_put_be64(f, tx_stat->bcastBytesTxOK);
> >> -qemu_put_be64(f, tx_stat->pktsTxError);
> >> -qemu_put_be64(f, tx_stat->pktsTxDiscard);
> >> -}
> >> -
> >> -static int vmxnet3_get_txq_descr(QEMUFile *f, void *pv, size_t size,
> >> -VMStateField *field)
> >> -{
> >> -Vmxnet3TxqDescr *r = pv;
> >> -
> >> -vmxnet3_get_ring_from_file(f, >tx_ring);
> >> -vmxnet3_get_ring_from_file(f, >comp_ring);
> >> -r->intr_idx = qemu_get_byte(f);
> >> -r->tx_stats_pa = qemu_get_be64(f);
> >> -
> >> -vmxnet3_get_tx_stats_from_file(f, >txq_stats);
> >> -
> >> -return 0;
> >> -}
> >> -
> >> -static int vmxnet3_put_txq_descr(QEMUFile *f, void *pv, size_t size,
> >> - VMStateField *field, QJSON *vmdesc)
> >> -{
> >> -Vmxnet3TxqDescr *r = pv;
> >> -
> >> -vmxnet3_put_ring_to_file(f, >tx_ring);
> >> -vmxnet3_put_ring_to_file(f, >comp_ring);
> >> -qemu_put_byte(f, r->intr_idx);
> >> -qemu_put_be64(f, r->tx_stats_pa);
> >> -vmxnet3_put_tx_stats_to_file(f, >txq_stats);
> >> -
> >> -return 0;
> >> -}
> >> -
> >> -static const VMStateInfo txq_descr_info = {
> >> -.name = "txq_descr",
> >> -.get = vmxnet3_get_txq_descr,
> >> -.put = vmxnet3_put_txq_descr
> >> +static const VMStateDescription vmstate_vmxnet3_ring = {
> >> +.name = "vmxnet3-ring",
> >> +.version_id = 0,
> >> +.fields = (VMStateField[]) {
> >> +VMSTATE_UINT64(pa, Vmxnet3Ring),
> >> +VMSTATE_UINT32(size, Vmxnet3Ring),
> >> +VMSTATE_UINT32(cell_size, Vmxnet3Ring),
> >> +VMSTATE_UINT32(next, Vmxnet3Ring),
> >> +VMSTATE_UINT8(gen, Vmxnet3Ring),
> >> +VMSTATE_END_OF_LIST()
> >> +}
> >> };
> >> 
> >> -static void vmxnet3_get_rx_stats_from_file(QEMUFile *f,
> >> -struct UPT1_RxStats *rx_stat)
> >> -{
> >> -rx_stat->LROPktsRxOK = qemu_get_be64(f);
> >> -rx_stat->LROBytesRxOK = qemu_get_be64(f);
> >> -rx_stat->ucastPktsRxOK = qemu_get_be64(f);
> >> -rx_stat->ucastBytesRxOK = qemu_get_be64(f);
> >> -

Re: [Qemu-devel] [PATCH v2 5/5] iotests: chown LUKS device before qemu-io launches

2017-01-03 Thread Eric Blake

On 01/03/2017 11:04 AM, Daniel P. Berrange wrote:
> On some distros, whenever you close a block device file
> descriptor there is a udev rule that resets the file
> permissions. This can race with the test script when
> we run qemu-io multiple times against the same block
> device. Occassionally the second qemu-io invokation

s/Occassionally/Occasionally/
s/invokation/invocation/

> will find udev has reset the permissions causing failure.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  tests/qemu-iotests/149 |  13 +-
>  tests/qemu-iotests/149.out | 344 
> ++---
>  2 files changed, 178 insertions(+), 179 deletions(-)
> 

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] win32: use glib gpoll if glib >= 2.50

2017-01-03 Thread Marc-André Lureau

A fix has been committed in upstream glib commit
210a9796f78eb90f76f1bd6a304e9fea05e97617.
(See also related bug https://bugzilla.gnome.org/show_bug.cgi?id=764415)

It is desirable to use the glib version instead of qemu copy, since it
provides more debugging facilities (G_MAIN_POLL_DEBUG etc), and
hopefully has a better maintainance. Hopefully, we can drop the qemu
copy in a few years.

Signed-off-by: Marc-André Lureau 
---
 include/glib-compat.h | 2 +-
 util/oslib-win32.c| 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/glib-compat.h b/include/glib-compat.h
index acf254d2a0..0cd24ffbe9 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -39,7 +39,7 @@ static inline gint64 qemu_g_get_monotonic_time(void)
 #define g_get_monotonic_time() qemu_g_get_monotonic_time()
 #endif
 
-#ifdef _WIN32
+#if defined(_WIN32) && !GLIB_CHECK_VERSION(2, 50, 0)
 /*
  * g_poll has a problem on Windows when using
  * timeouts < 10ms, so use wrapper.
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index d09863cc9d..0b1890fd33 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -327,6 +327,7 @@ char *qemu_get_exec_dir(void)
 return g_strdup(exec_dir);
 }
 
+#if !GLIB_CHECK_VERSION(2, 50, 0)
 /*
  * The original implementation of g_poll from glib has a problem on Windows
  * when using timeouts < 10 ms.
@@ -530,6 +531,7 @@ gint g_poll(GPollFD *fds, guint nfds, gint timeout)
 
 return retval;
 }
+#endif
 
 int getpagesize(void)
 {
-- 
2.11.0

Re: [Qemu-devel] [PATCH v2 4/5] iotests: add more LUKS hash combination tests

2017-01-03 Thread Eric Blake

On 01/03/2017 11:04 AM, Daniel P. Berrange wrote:
> Add tests for sha224, sha512, sha384 and ripemd160 hash
> algorithms.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  tests/qemu-iotests/149 |  10 +-
>  tests/qemu-iotests/149.out | 482 
> -
>  2 files changed, 484 insertions(+), 8 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
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 v2 3/5] iotests: reduce PBKDF iterations when testing LUKS

2017-01-03 Thread Eric Blake

On 01/03/2017 11:04 AM, Daniel P. Berrange wrote:
> By default the PBKDF algorithm used with LUKS is tuned
> based on the number of iterations to produce 1 second
> of running time. This makes running the I/O test with
> the LUKS format orders of magnitude slower than with
> qcow2/raw formats.
> 
> When creating LUKS images, set the iteration time to
> a 10ms to reduce the time overhead for LUKS, since
> security does not matter in I/O tests.
> 
> Previsouly a full 'check -luks' would take

s/Previsouly/Previously/

-- 
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 v2 3/5] iotests: reduce PBKDF iterations when testing LUKS

2017-01-03 Thread Eric Blake

On 01/03/2017 11:04 AM, Daniel P. Berrange wrote:
> By default the PBKDF algorithm used with LUKS is tuned
> based on the number of iterations to produce 1 second
> of running time. This makes running the I/O test with
> the LUKS format orders of magnitude slower than with
> qcow2/raw formats.
> 
> When creating LUKS images, set the iteration time to
> a 10ms to reduce the time overhead for LUKS, since
> security does not matter in I/O tests.
> 
> Previsouly a full 'check -luks' would take
> 
>   $ time ./check -luks
>   Passed all 22 tests
> 
>   real  23m9.988s
>   user  21m46.223s
>   sys   0m22.841s
> 
> Now it takes
> 
>   $ time ./check -luks
>   Passed all 22 tests
> 
>   real  4m39.235s
>   user  3m29.590s
>   sys   0m24.234s
> 
> Still slow compared to qcow2/raw, but much improved
> none the less.
> 
> Signed-off-by: Daniel P. Berrange 
> ---

Reviewed-by: Eric Blake 

-- 
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 v2 2/5] iotests: fix remainining tests to work with LUKS

2017-01-03 Thread Eric Blake

On 01/03/2017 11:04 AM, Daniel P. Berrange wrote:
> The tests 033, 120, 140, 145 and 157 were all broken
> when run with LUKS, since they did not correctly use
> the required image opts args syntax to specify the
> decryption secret.
> 
> Signed-off-by: Daniel P. Berrange 
> ---

> +++ b/tests/qemu-iotests/120
> @@ -44,17 +44,36 @@ _supported_os Linux
>  
>  _make_test_img 64M
>  
> +if test "$IMGOPTSSYNTAX" = "true"
> +then
> +SYSEMU_DRIVE_ARG=id=drv,if=none,$TEST_IMG
> +SYSEMU_EXTRA_ARGS=""
> +IO_DRIVE_ARG="$TEST_IMG"
> +IO_EXTRA_ARGS="--image-opts"
> +if [ -n "$IMGKEYSECRET" ]; then
> + SECRET_ARG="secret,id=keysec0,data=$IMGKEYSECRET"
> + SYSEMU_EXTRA_ARGS="$SYSEMU_EXTRA_ARGS -object $SECRET_ARG"

Should we favor the '--object' spelling rather than '-object'?  But both
work (thanks to getopt_long_only()), so I'm not going to demand a respin.

> + IO_EXTRA_ARGS="$IO_EXTRA_ARGS --object $SECRET_ARG"
> +fi
> +else
> +
> SYSEMU_DRIVE_ARG=id=drv,if=none,file="$TEST_IMG",driver=raw,file.driver=$IMGFMT
> +SYSEMU_EXTRA_ARGS=""
> +IO_DRIVE_ARG="json:{'driver': 'raw', 'file': {'driver': '$IMGFMT', 
> 'file': {'filename': '$TEST_IMG'}}}"
> +IO_EXTRA_ARGS=""
> +fi
> +
> +
>  echo "{'execute': 'qmp_capabilities'}
>{'execute': 'human-monitor-command',
> 'arguments': {'command-line': 'qemu-io drv \"write -P 42 0 64k\"'}}
>{'execute': 'quit'}" \
> -| $QEMU -qmp stdio -nographic -nodefaults \
> --drive 
> id=drv,if=none,file="$TEST_IMG",driver=raw,file.driver=$IMGFMT \
> +| $QEMU -qmp stdio -nographic -nodefaults $SYSEMU_EXTRA_ARGS \
> +-drive $SYSEMU_DRIVE_ARG \
>  | _filter_qmp | _filter_qemu_io
>  $QEMU_IO -c 'read -P 42 0 64k' "$TEST_IMG" | _filter_qemu_io
>  
>  $QEMU_IO_PROG -c 'read -P 42 0 64k' \
> -"json:{'driver': 'raw', 'file': {'driver': '$IMGFMT', 'file': 
> {'filename': '$TEST_IMG'}}}" \
> +$IO_EXTRA_ARGS "$IO_DRIVE_ARG" \
>  | _filter_qemu_io
>  

Reviewed-by: Eric Blake 

-- 
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] win32: don't run subprocess tests on Mingw32 platform

2017-01-03 Thread Marc-André Lureau

Hi

On Tue, Jun 14, 2016 at 6:34 PM Daniel P. Berrange 
wrote:

> The g_test_trap_subprocess() method does not work on the
> Mingw32 platform, causing the test-qdev-global-props
> test case to abort
>
> (test-logging.exe:230): GLib-ERROR **: g_test_trap_subprocess()
> failed: Failed to execute helper program (No such file or directory)
>
> This failure was introduced a while ago in
>
>   commit 2177801a4899bf29108b3d471417a5b4d701ec29
>   Author: Eduardo Habkost 
>   Date:   Fri Aug 8 16:03:27 2014 -0300
>
> test-qdev-global-props: Run tests on subprocess
>
> Modify the configure time check to avoid enabling this feature
> on Mingw, rather than trying to rewrite the test to avoid this
> feature.
>
> Signed-off-by: Daniel P. Berrange 
> ---
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 8c2f90b..aa291e8 100755
> --- a/configure
> +++ b/configure
> @@ -5167,7 +5167,7 @@ if test "$bluez" = "yes" ; then
>echo "CONFIG_BLUEZ=y" >> $config_host_mak
>echo "BLUEZ_CFLAGS=$bluez_cflags" >> $config_host_mak
>  fi
> -if test "$glib_subprocess" = "yes" ; then
> +if test "$glib_subprocess" = "yes" && test "$mingw32" != "yes" ; then
>echo "CONFIG_HAS_GLIB_SUBPROCESS_TESTS=y" >> $config_host_mak
>  fi
>

Isn't it only because you are missing gspawn-win*-helper.exe?

This seems to be a glib installation issue to me. I can run the
subprocesses tests fine on win32 and wine. I suggest to revert commit
7ad9339e372fcd12d584684d7f52ac259604a4f4, I can send a patch if you agree.



 echo "GLIB_CFLAGS=$glib_cflags" >> $config_host_mak
> --
> 2.5.5
>
>
> --
Marc-André Lureau

[Qemu-devel] [PATCH v1 13/15] iotests: enable tests 134 and 158 to work with qcow (v1)

2017-01-03 Thread Daniel P. Berrange

The 138 and 158 iotests exercise the legacy qcow2 aes encryption
code path. With a few simple tweaks they can exercise the same
feature in qcow (v1).

Signed-off-by: Daniel P. Berrange 
---
 tests/qemu-iotests/134 | 10 +-
 tests/qemu-iotests/158 |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/134 b/tests/qemu-iotests/134
index dd080a2..23b7834 100755
--- a/tests/qemu-iotests/134
+++ b/tests/qemu-iotests/134
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.rc
 . ./common.filter
 
-_supported_fmt qcow2
+_supported_fmt qcow qcow2
 _supported_proto generic
 _supported_os Linux
 
@@ -55,19 +55,19 @@ QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT
 
 echo
 echo "== reading whole image =="
-$QEMU_IO --object $SECRET -c "read 0 $size" --image-opts $IMGSPEC | 
_filter_qemu_io | _filter_testdir
+$QEMU_IO --object $SECRET -c "read 0 $size" --image-opts $IMGSPEC | 
_filter_qemu_io | _filter_testdir | _filter_imgfmt
 
 echo
 echo "== rewriting whole image =="
-$QEMU_IO --object $SECRET -c "write -P 0xa 0 $size" --image-opts $IMGSPEC | 
_filter_qemu_io | _filter_testdir
+$QEMU_IO --object $SECRET -c "write -P 0xa 0 $size" --image-opts $IMGSPEC | 
_filter_qemu_io | _filter_testdir | _filter_imgfmt
 
 echo
 echo "== verify pattern =="
-$QEMU_IO --object $SECRET -c "read -P 0xa 0 $size"  --image-opts $IMGSPEC | 
_filter_qemu_io | _filter_testdir
+$QEMU_IO --object $SECRET -c "read -P 0xa 0 $size"  --image-opts $IMGSPEC | 
_filter_qemu_io | _filter_testdir | _filter_imgfmt
 
 echo
 echo "== verify pattern failure with wrong password =="
-$QEMU_IO --object $SECRETALT -c "read -P 0xa 0 $size" --image-opts $IMGSPEC | 
_filter_qemu_io | _filter_testdir
+$QEMU_IO --object $SECRETALT -c "read -P 0xa 0 $size" --image-opts $IMGSPEC | 
_filter_qemu_io | _filter_testdir | _filter_imgfmt
 
 
 # success, all done
diff --git a/tests/qemu-iotests/158 b/tests/qemu-iotests/158
index 7a1eb5c..2b53d9f 100755
--- a/tests/qemu-iotests/158
+++ b/tests/qemu-iotests/158
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.rc
 . ./common.filter
 
-_supported_fmt qcow2
+_supported_fmt qcow qcow2
 _supported_proto generic
 _supported_os Linux
 
-- 
2.9.3

[Qemu-devel] [PATCH v1 07/15] iotests: fix 097 when run with qcow

2017-01-03 Thread Daniel P. Berrange

The previous commit:

  commit a3e1505daec31ef56f0489f8c8fff1b8e4ca92bd
  Author: Eric Blake 
  Date:   Mon Dec 5 09:49:34 2016 -0600

qcow2: Don't strand clusters near 2G intervals during commit

extended the 097 test case so that it did two passes, once
with an internal snapshot, once without.

qcow (v1) does not support internal snapshots, so this change
broke test 097 when run against qcow.

This splits 097 in two, creating a new 173 that tests the
internal snapshot codepath, effectively putting 097 back
to its content before the above commit.

Signed-off-by: Daniel P. Berrange 
---
 tests/qemu-iotests/097 |  10 +---
 tests/qemu-iotests/097.out | 125 ++--
 tests/qemu-iotests/173 | 126 +
 tests/qemu-iotests/173.out | 119 ++
 tests/qemu-iotests/group   |   1 +
 5 files changed, 251 insertions(+), 130 deletions(-)
 create mode 100755 tests/qemu-iotests/173
 create mode 100644 tests/qemu-iotests/173.out

diff --git a/tests/qemu-iotests/097 b/tests/qemu-iotests/097
index 4c33e80..1d28aff 100755
--- a/tests/qemu-iotests/097
+++ b/tests/qemu-iotests/097
@@ -56,26 +56,19 @@ _supported_os Linux
 #  3: Two-layer backing chain, commit to lower backing file
 # (in this case, the top image will implicitly stay unchanged)
 #
-# Each pass is run twice, since qcow2 has different code paths for cleaning
-# an image depending on whether it has a snapshot.
-#
 # 020 already tests committing, so this only tests whether image chains are
 # working properly and that all images above the base are emptied; therefore,
 # no complicated patterns are necessary.  Check near the 2G mark, as qcow2
 # has been buggy at that boundary in the past.
 for i in 0 1 2 3; do
-for j in 0 1; do
 
 echo
-echo "=== Test pass $i.$j ==="
+echo "=== Test pass $i ==="
 echo
 
 TEST_IMG="$TEST_IMG.base" _make_test_img 2100M
 TEST_IMG="$TEST_IMG.itmd" _make_test_img -b "$TEST_IMG.base" 2100M
 _make_test_img -b "$TEST_IMG.itmd" 2100M
-if [ $j -eq 0 ]; then
-$QEMU_IMG snapshot -c snap "$TEST_IMG"
-fi
 
 $QEMU_IO -c 'write -P 1 0x7ffd 192k' "$TEST_IMG.base" | _filter_qemu_io
 $QEMU_IO -c 'write -P 2 0x7ffe 128k' "$TEST_IMG.itmd" | _filter_qemu_io
@@ -121,7 +114,6 @@ $QEMU_IMG map "$TEST_IMG.itmd" | _filter_qemu_img_map
 $QEMU_IMG map "$TEST_IMG" | _filter_qemu_img_map
 
 done
-done
 
 
 # success, all done
diff --git a/tests/qemu-iotests/097.out b/tests/qemu-iotests/097.out
index 8106cc9..81fc225 100644
--- a/tests/qemu-iotests/097.out
+++ b/tests/qemu-iotests/097.out
@@ -1,6 +1,6 @@
 QA output created by 097
 
-=== Test pass 0.0 ===
+=== Test pass 0 ===
 
 Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
 Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 
backing_file=TEST_DIR/t.IMGFMT.base
@@ -29,66 +29,7 @@ Offset  Length  File
 0x7ffd  0x1 TEST_DIR/t.IMGFMT.base
 0x7ffe  0x2 TEST_DIR/t.IMGFMT.itmd
 
-=== Test pass 0.1 ===
-
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 
backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 
backing_file=TEST_DIR/t.IMGFMT.itmd
-wrote 196608/196608 bytes at offset 2147287040
-192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 131072/131072 bytes at offset 2147352576
-128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset 2147418112
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Image committed.
-read 196608/196608 bytes at offset 2147287040
-192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 2147287040
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 2147352576
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 2147418112
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Offset  Length  File
-0x7ffd  0x3 TEST_DIR/t.IMGFMT.base
-Offset  Length  File
-0x7ffd  0x1 TEST_DIR/t.IMGFMT.base
-0x7ffe  0x2 TEST_DIR/t.IMGFMT.itmd
-Offset  Length  File
-0x7ffd  0x1 TEST_DIR/t.IMGFMT.base
-0x7ffe  0x2 TEST_DIR/t.IMGFMT.itmd
-
-=== Test pass 1.0 ===
-
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 
backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 
backing_file=TEST_DIR/t.IMGFMT.itmd
-wrote 196608/196608 bytes at offset 2147287040
-192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 131072/131072 bytes at offset 2147352576
-128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec

[Qemu-devel] [PATCH v1 15/15] block: remove all encryption handling APIs

2017-01-03 Thread Daniel P. Berrange

Now that all encryption keys must be provided upfront via
the QCryptoSecret API and associated block driver properties
there is no need for any explicit encryption handling APIs
in the block layer. Encryption can be handled transparently
within the block driver. We only retain an API for querying
whether an image is encrypted or not, since that is a
potentially useful piece of metadata to report to the user.

Signed-off-by: Daniel P. Berrange 
---
 block.c   | 77 +--
 block/crypto.c|  1 -
 block/qapi.c  |  2 +-
 block/qcow.c  |  1 -
 block/qcow2.c |  1 -
 blockdev.c| 37 ++-
 include/block/block.h |  3 --
 include/block/block_int.h |  1 -
 include/qapi/error.h  |  1 -
 qapi/common.json  |  5 +--
 10 files changed, 5 insertions(+), 124 deletions(-)

diff --git a/block.c b/block.c
index 39ddea3..27cca49 100644
--- a/block.c
+++ b/block.c
@@ -1865,15 +1865,7 @@ static BlockDriverState *bdrv_open_inherit(const char 
*filename,
 goto close_and_fail;
 }
 
-if (!bdrv_key_required(bs)) {
-bdrv_parent_cb_change_media(bs, true);
-} else if (!runstate_check(RUN_STATE_PRELAUNCH)
-   && !runstate_check(RUN_STATE_INMIGRATE)
-   && !runstate_check(RUN_STATE_PAUSED)) { /* HACK */
-error_setg(errp,
-   "Guest must be stopped for opening of encrypted image");
-goto close_and_fail;
-}
+bdrv_parent_cb_change_media(bs, true);
 
 QDECREF(options);
 
@@ -2354,7 +2346,6 @@ static void bdrv_close(BlockDriverState *bs)
 bs->backing_format[0] = '\0';
 bs->total_sectors = 0;
 bs->encrypted = false;
-bs->valid_key = false;
 bs->sg = false;
 QDECREF(bs->options);
 QDECREF(bs->explicit_options);
@@ -2723,72 +2714,6 @@ bool bdrv_is_encrypted(BlockDriverState *bs)
 return bs->encrypted;
 }
 
-bool bdrv_key_required(BlockDriverState *bs)
-{
-BdrvChild *backing = bs->backing;
-
-if (backing && backing->bs->encrypted && !backing->bs->valid_key) {
-return true;
-}
-return (bs->encrypted && !bs->valid_key);
-}
-
-int bdrv_set_key(BlockDriverState *bs, const char *key)
-{
-int ret;
-if (bs->backing && bs->backing->bs->encrypted) {
-ret = bdrv_set_key(bs->backing->bs, key);
-if (ret < 0)
-return ret;
-if (!bs->encrypted)
-return 0;
-}
-if (!bs->encrypted) {
-return -EINVAL;
-} else if (!bs->drv || !bs->drv->bdrv_set_key) {
-return -ENOMEDIUM;
-}
-ret = bs->drv->bdrv_set_key(bs, key);
-if (ret < 0) {
-bs->valid_key = false;
-} else if (!bs->valid_key) {
-/* call the change callback now, we skipped it on open */
-bs->valid_key = true;
-bdrv_parent_cb_change_media(bs, true);
-}
-return ret;
-}
-
-/*
- * Provide an encryption key for @bs.
- * If @key is non-null:
- * If @bs is not encrypted, fail.
- * Else if the key is invalid, fail.
- * Else set @bs's key to @key, replacing the existing key, if any.
- * If @key is null:
- * If @bs is encrypted and still lacks a key, fail.
- * Else do nothing.
- * On failure, store an error object through @errp if non-null.
- */
-void bdrv_add_key(BlockDriverState *bs, const char *key, Error **errp)
-{
-if (key) {
-if (!bdrv_is_encrypted(bs)) {
-error_setg(errp, "Node '%s' is not encrypted",
-  bdrv_get_device_or_node_name(bs));
-} else if (bdrv_set_key(bs, key) < 0) {
-error_setg(errp, QERR_INVALID_PASSWORD);
-}
-} else {
-if (bdrv_key_required(bs)) {
-error_set(errp, ERROR_CLASS_DEVICE_ENCRYPTED,
-  "'%s' (%s) is encrypted",
-  bdrv_get_device_or_node_name(bs),
-  bdrv_get_encrypted_filename(bs));
-}
-}
-}
-
 const char *bdrv_get_format_name(BlockDriverState *bs)
 {
 return bs->drv ? bs->drv->format_name : NULL;
diff --git a/block/crypto.c b/block/crypto.c
index 915ba70..2c4279c 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -380,7 +380,6 @@ static int block_crypto_open_generic(QCryptoBlockFormat 
format,
 }
 
 bs->encrypted = true;
-bs->valid_key = true;
 
 ret = 0;
  cleanup:
diff --git a/block/qapi.c b/block/qapi.c
index a62e862..68cab56 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -45,7 +45,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
 info->ro = bs->read_only;
 info->drv= g_strdup(bs->drv->format_name);
 info->encrypted  = bs->encrypted;
-info->encryption_key_missing = bdrv_key_required(bs);
+info->encryption_key_missing = false;
 
 info->cache = g_new(BlockdevCacheInfo, 1);
 *info->cache =

[Qemu-devel] [PATCH v1 06/15] iotests: skip 048 with qcow which doesn't support resize

2017-01-03 Thread Daniel P. Berrange

Test 048 is designed to verify data preservation during an
image resize. The qcow (v1) format impl has never supported
resize so always fails.

Signed-off-by: Daniel P. Berrange 
---
 tests/qemu-iotests/048 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/048 b/tests/qemu-iotests/048
index 203c04f..9ed04a0 100755
--- a/tests/qemu-iotests/048
+++ b/tests/qemu-iotests/048
@@ -46,7 +46,7 @@ _compare()
 . ./common.filter
 . ./common.pattern
 
-_supported_fmt raw qcow qcow2 qed luks
+_supported_fmt raw qcow2 qed luks
 _supported_proto file
 _supported_os Linux
 
-- 
2.9.3

[Qemu-devel] [PATCH v1 08/15] qcow: make encrypt_sectors encrypt in place

2017-01-03 Thread Daniel P. Berrange

Instead of requiring separate input/output buffers for
encrypting data, change encrypt_sectors() to assume
use of a single buffer, encrypting in place. One current
caller all uses the same buffer for input/output already
and the other two callers are easily converted todo so.

Signed-off-by: Daniel P. Berrange 
---
 block/qcow.c | 36 +++-
 1 file changed, 11 insertions(+), 25 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 8133fda..bc9fa2f 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -310,11 +310,10 @@ static int qcow_set_key(BlockDriverState *bs, const char 
*key)
 }
 
 /* The crypt function is compatible with the linux cryptoloop
-   algorithm for < 4 GB images. NOTE: out_buf == in_buf is
-   supported */
+   algorithm for < 4 GB images. */
 static int encrypt_sectors(BDRVQcowState *s, int64_t sector_num,
-   uint8_t *out_buf, const uint8_t *in_buf,
-   int nb_sectors, bool enc, Error **errp)
+   uint8_t *buf, int nb_sectors, bool enc,
+   Error **errp)
 {
 union {
 uint64_t ll[2];
@@ -333,14 +332,12 @@ static int encrypt_sectors(BDRVQcowState *s, int64_t 
sector_num,
 }
 if (enc) {
 ret = qcrypto_cipher_encrypt(s->cipher,
- in_buf,
- out_buf,
+ buf, buf,
  512,
  errp);
 } else {
 ret = qcrypto_cipher_decrypt(s->cipher,
- in_buf,
- out_buf,
+ buf, buf,
  512,
  errp);
 }
@@ -348,8 +345,7 @@ static int encrypt_sectors(BDRVQcowState *s, int64_t 
sector_num,
 return -1;
 }
 sector_num++;
-in_buf += 512;
-out_buf += 512;
+buf += 512;
 }
 return 0;
 }
@@ -469,13 +465,12 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
 uint64_t start_sect;
 assert(s->cipher);
 start_sect = (offset & ~(s->cluster_size - 1)) >> 9;
-memset(s->cluster_data + 512, 0x00, 512);
+memset(s->cluster_data, 0x00, 512);
 for(i = 0; i < s->cluster_sectors; i++) {
 if (i < n_start || i >= n_end) {
 Error *err = NULL;
 if (encrypt_sectors(s, start_sect + i,
-s->cluster_data,
-s->cluster_data + 512, 1,
+s->cluster_data, 1,
 true, ) < 0) {
 error_free(err);
 errno = EIO;
@@ -653,7 +648,7 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, 
int64_t sector_num,
 }
 if (bs->encrypted) {
 assert(s->cipher);
-if (encrypt_sectors(s, sector_num, buf, buf,
+if (encrypt_sectors(s, sector_num, buf,
 n, false, ) < 0) {
 goto fail;
 }
@@ -688,9 +683,7 @@ static coroutine_fn int qcow_co_writev(BlockDriverState 
*bs, int64_t sector_num,
 BDRVQcowState *s = bs->opaque;
 int index_in_cluster;
 uint64_t cluster_offset;
-const uint8_t *src_buf;
 int ret = 0, n;
-uint8_t *cluster_data = NULL;
 struct iovec hd_iov;
 QEMUIOVector hd_qiov;
 uint8_t *buf;
@@ -728,21 +721,15 @@ static coroutine_fn int qcow_co_writev(BlockDriverState 
*bs, int64_t sector_num,
 if (bs->encrypted) {
 Error *err = NULL;
 assert(s->cipher);
-if (!cluster_data) {
-cluster_data = g_malloc0(s->cluster_size);
-}
-if (encrypt_sectors(s, sector_num, cluster_data, buf,
+if (encrypt_sectors(s, sector_num, buf,
 n, true, ) < 0) {
 error_free(err);
 ret = -EIO;
 break;
 }
-src_buf = cluster_data;
-} else {
-src_buf = buf;
 }
 
-hd_iov.iov_base = (void *)src_buf;
+hd_iov.iov_base = (void *)buf;
 hd_iov.iov_len = n * 512;
 qemu_iovec_init_external(_qiov, _iov, 1);
 qemu_co_mutex_unlock(>lock);
@@ -764,7 +751,6 @@ static coroutine_fn int qcow_co_writev(BlockDriverState 
*bs, int64_t sector_num,
 if (qiov->niov > 1) {
 qemu_vfree(orig_buf);
 }
-g_free(cluster_data);
 
 return ret;
 }
--

[Qemu-devel] [PATCH v1 14/15] block: rip out all traces of password prompting

2017-01-03 Thread Daniel P. Berrange

Now that qcow & qcow2 are wired up to get encryption keys
via the QCryptoSecret object, nothing is relying on the
interactive prompting for passwords. All the code related
to password prompting can thus be ripped out.

Signed-off-by: Daniel P. Berrange 
---
 hmp.c | 31 -
 include/monitor/monitor.h |  7 -
 include/qemu/osdep.h  |  2 --
 monitor.c | 68 ---
 qapi-schema.json  | 10 +--
 qemu-img.c| 31 -
 qemu-io.c | 20 --
 qmp.c | 12 +
 util/oslib-posix.c| 66 -
 util/oslib-win32.c| 24 -
 10 files changed, 2 insertions(+), 269 deletions(-)

diff --git a/hmp.c b/hmp.c
index b869617..a1fe64e 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1070,37 +1070,12 @@ void hmp_ringbuf_read(Monitor *mon, const QDict *qdict)
 g_free(data);
 }
 
-static void hmp_cont_cb(void *opaque, int err)
-{
-if (!err) {
-qmp_cont(NULL);
-}
-}
-
-static bool key_is_missing(const BlockInfo *bdev)
-{
-return (bdev->inserted && bdev->inserted->encryption_key_missing);
-}
-
 void hmp_cont(Monitor *mon, const QDict *qdict)
 {
-BlockInfoList *bdev_list, *bdev;
 Error *err = NULL;
 
-bdev_list = qmp_query_block(NULL);
-for (bdev = bdev_list; bdev; bdev = bdev->next) {
-if (key_is_missing(bdev->value)) {
-monitor_read_block_device_key(mon, bdev->value->device,
-  hmp_cont_cb, NULL);
-goto out;
-}
-}
-
 qmp_cont();
 hmp_handle_error(mon, );
-
-out:
-qapi_free_BlockInfoList(bdev_list);
 }
 
 void hmp_system_wakeup(Monitor *mon, const QDict *qdict)
@@ -1536,12 +1511,6 @@ void hmp_change(Monitor *mon, const QDict *qdict)
 qmp_blockdev_change_medium(true, device, false, NULL, target,
!!arg, arg, !!read_only, read_only_mode,
);
-if (err &&
-error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
-error_free(err);
-monitor_read_block_device_key(mon, device, NULL, NULL);
-return;
-}
 }
 
 hmp_handle_error(mon, );
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 8cc532e..2183aac 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -22,13 +22,6 @@ void monitor_cleanup(void);
 int monitor_suspend(Monitor *mon);
 void monitor_resume(Monitor *mon);
 
-int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs,
-BlockCompletionFunc *completion_cb,
-void *opaque);
-int monitor_read_block_device_key(Monitor *mon, const char *device,
-  BlockCompletionFunc *completion_cb,
-  void *opaque);
-
 int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp);
 int monitor_fd_param(Monitor *mon, const char *fdname, Error **errp);
 
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 689f253..d79e9a5 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -396,8 +396,6 @@ void qemu_set_tty_echo(int fd, bool echo);
 
 void os_mem_prealloc(int fd, char *area, size_t sz, Error **errp);
 
-int qemu_read_password(char *buf, int buf_size);
-
 /**
  * qemu_get_pid_name:
  * @pid: pid of a process
diff --git a/monitor.c b/monitor.c
index 0841d43..7e3a855 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4033,74 +4033,6 @@ void monitor_cleanup(void)
 qemu_mutex_unlock(_lock);
 }
 
-static void bdrv_password_cb(void *opaque, const char *password,
- void *readline_opaque)
-{
-Monitor *mon = opaque;
-BlockDriverState *bs = readline_opaque;
-int ret = 0;
-Error *local_err = NULL;
-
-bdrv_add_key(bs, password, _err);
-if (local_err) {
-error_report_err(local_err);
-ret = -EPERM;
-}
-if (mon->password_completion_cb)
-mon->password_completion_cb(mon->password_opaque, ret);
-
-monitor_read_command(mon, 1);
-}
-
-int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs,
-BlockCompletionFunc *completion_cb,
-void *opaque)
-{
-int err;
-
-monitor_printf(mon, "%s (%s) is encrypted.\n", bdrv_get_device_name(bs),
-   bdrv_get_encrypted_filename(bs));
-
-mon->password_completion_cb = completion_cb;
-mon->password_opaque = opaque;
-
-err = monitor_read_password(mon, bdrv_password_cb, bs);
-
-if (err && completion_cb)
-completion_cb(opaque, err);
-
-return err;
-}
-
-int monitor_read_block_device_key(Monitor *mon, const char *device,
-  BlockCompletionFunc *completion_cb,
-

[Qemu-devel] [PATCH v1 12/15] qcow2: add support for LUKS encryption format

2017-01-03 Thread Daniel P. Berrange

This adds support for using LUKS as an encryption format
with the qcow2 file. The use of the existing 'encryption=on'
parameter is replaced by a new parameter 'encryption-format'
which takes the values 'aes' or 'luks'. e.g.

  # qemu-img create --object secret,data=123456,id=sec0 \
   -f qcow2 -o encryption-format=luks,luks-key-secret=sec0 \
   test.qcow2 10G

results in the creation of an image using the LUKS format.
Use of the legacy 'encryption=on' parameter still results
in creation of the old qcow2 AES format, and is equivalent
to the new 'encryption-format=aes'. e.g. the following are
equivalent:

  # qemu-img create --object secret,data=123456,id=sec0 \
   -f qcow2 -o encryption=on,aes-key-secret=sec0 \
   test.qcow2 10G

 # qemu-img create --object secret,data=123456,id=sec0 \
   -f qcow2 -o encryption-format=aes,aes-key-secret=sec0 \
   test.qcow2 10G

With the LUKS format it is necessary to store the LUKS
partition header and key material in the QCow2 file. This
data can be many MB in size, so cannot go into the QCow2
header region directly. Thus the spec defines a FDE
(Full Disk Encryption) header extension that specifies
the offset of a set of clusters to hold the FDE headers,
as well as the length of that region. The LUKS header is
thus stored in these extra allocated clusters before the
main image payload.

Aside from all the cryptographic differences implied by
use of the LUKS format, there is one further key difference
between the use of legacy AES and LUKS encryption in qcow2.
For LUKS, the initialiazation vectors are generated using
the host physical sector as the input, rather than the
guest virtual sector. This guarantees unique initialization
vectors for all sectors when qcow2 internal snapshots are
used, thus giving stronger protection against watermarking
attacks.

Signed-off-by: Daniel P. Berrange 
---
 block/qcow2-cluster.c  |   4 +-
 block/qcow2-refcount.c |  10 ++
 block/qcow2.c  | 236 ++---
 block/qcow2.h  |   9 ++
 docs/specs/qcow2.txt   |  99 +++
 qapi/block-core.json   |   6 +-
 tests/qemu-iotests/049 |   2 +-
 tests/qemu-iotests/082.out | 216 +
 tests/qemu-iotests/087 |  65 -
 tests/qemu-iotests/087.out |  22 -
 tests/qemu-iotests/134 |   4 +-
 tests/qemu-iotests/158 |   8 +-
 tests/qemu-iotests/174 |  76 +++
 tests/qemu-iotests/174.out |  19 
 tests/qemu-iotests/175 |  85 
 tests/qemu-iotests/175.out |  26 +
 tests/qemu-iotests/group   |   2 +
 17 files changed, 840 insertions(+), 49 deletions(-)
 create mode 100755 tests/qemu-iotests/174
 create mode 100644 tests/qemu-iotests/174.out
 create mode 100755 tests/qemu-iotests/175
 create mode 100644 tests/qemu-iotests/175.out

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index a2103dc..866b122 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -383,7 +383,9 @@ static int coroutine_fn do_perform_cow(BlockDriverState *bs,
 
 if (bs->encrypted) {
 Error *err = NULL;
-int64_t sector = (src_cluster_offset + offset_in_cluster)
+int64_t sector = (s->crypt_physical_offset ?
+  (cluster_offset + offset_in_cluster) :
+  (src_cluster_offset + offset_in_cluster))
  >> BDRV_SECTOR_BITS;
 assert((offset_in_cluster & ~BDRV_SECTOR_MASK) == 0);
 assert((bytes & ~BDRV_SECTOR_MASK) == 0);
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index cbfb3fe..afa4636 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1843,6 +1843,16 @@ static int calculate_refcounts(BlockDriverState *bs, 
BdrvCheckResult *res,
 return ret;
 }
 
+/* encryption */
+if (s->crypto_header.length) {
+ret = inc_refcounts(bs, res, refcount_table, nb_clusters,
+s->crypto_header.offset,
+s->crypto_header.length);
+if (ret < 0) {
+return ret;
+}
+}
+
 return check_refblocks(bs, res, fix, rebuild, refcount_table, nb_clusters);
 }
 
diff --git a/block/qcow2.c b/block/qcow2.c
index 5c9e196..b354914 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -66,6 +66,7 @@ typedef struct {
 #define  QCOW2_EXT_MAGIC_END 0
 #define  QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
 #define  QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
+#define  QCOW2_EXT_MAGIC_CRYPTO_HEADER 0x0537be77
 
 static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
 {
@@ -80,6 +81,73 @@ static int qcow2_probe(const uint8_t *buf, int buf_size, 
const char *filename)
 }
 
 
+static ssize_t qcow2_crypto_hdr_read_func(QCryptoBlock *block, size_t offset,
+  uint8_t *buf, size_t buflen,
+

[Qemu-devel] [PATCH v1 10/15] qcow2: make qcow2_encrypt_sectors encrypt in place

2017-01-03 Thread Daniel P. Berrange

Instead of requiring separate input/output buffers for
encrypting data, change qcow2_encrypt_sectors() to assume
use of a single buffer, encrypting in place. The current
callers all used the same buffer for input/output already.

Reviewed-by: Eric Blake 
Reviewed-by: Fam Zheng 
Signed-off-by: Daniel P. Berrange 
---
 block/qcow2-cluster.c | 17 ++---
 block/qcow2.c |  4 ++--
 block/qcow2.h |  3 +--
 3 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 928c1e2..907e869 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -346,11 +346,9 @@ static int count_contiguous_clusters_by_type(int 
nb_clusters,
 }
 
 /* The crypt function is compatible with the linux cryptoloop
-   algorithm for < 4 GB images. NOTE: out_buf == in_buf is
-   supported */
+   algorithm for < 4 GB images. */
 int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t sector_num,
-  uint8_t *out_buf, const uint8_t *in_buf,
-  int nb_sectors, bool enc,
+  uint8_t *buf, int nb_sectors, bool enc,
   Error **errp)
 {
 union {
@@ -370,14 +368,12 @@ int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t 
sector_num,
 }
 if (enc) {
 ret = qcrypto_cipher_encrypt(s->cipher,
- in_buf,
- out_buf,
+ buf, buf,
  512,
  errp);
 } else {
 ret = qcrypto_cipher_decrypt(s->cipher,
- in_buf,
- out_buf,
+ buf, buf,
  512,
  errp);
 }
@@ -385,8 +381,7 @@ int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t 
sector_num,
 return -1;
 }
 sector_num++;
-in_buf += 512;
-out_buf += 512;
+buf += 512;
 }
 return 0;
 }
@@ -434,7 +429,7 @@ static int coroutine_fn do_perform_cow(BlockDriverState *bs,
 assert(s->cipher);
 assert((offset_in_cluster & ~BDRV_SECTOR_MASK) == 0);
 assert((bytes & ~BDRV_SECTOR_MASK) == 0);
-if (qcow2_encrypt_sectors(s, sector, iov.iov_base, iov.iov_base,
+if (qcow2_encrypt_sectors(s, sector, iov.iov_base,
   bytes >> BDRV_SECTOR_BITS, true, ) < 0) {
 ret = -EIO;
 error_free(err);
diff --git a/block/qcow2.c b/block/qcow2.c
index 96fb8a8..3c14c86 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1530,7 +1530,7 @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState 
*bs, uint64_t offset,
 assert((cur_bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
 Error *err = NULL;
 if (qcow2_encrypt_sectors(s, offset >> BDRV_SECTOR_BITS,
-  cluster_data, cluster_data,
+  cluster_data,
   cur_bytes >> BDRV_SECTOR_BITS,
   false, ) < 0) {
 error_free(err);
@@ -1626,7 +1626,7 @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState 
*bs, uint64_t offset,
 qemu_iovec_to_buf(_qiov, 0, cluster_data, hd_qiov.size);
 
 if (qcow2_encrypt_sectors(s, offset >> BDRV_SECTOR_BITS,
-  cluster_data, cluster_data,
+  cluster_data,
   cur_bytes >>BDRV_SECTOR_BITS,
   true, ) < 0) {
 error_free(err);
diff --git a/block/qcow2.h b/block/qcow2.h
index 1823414..033d8c0 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -530,8 +530,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t 
min_size,
 int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index);
 int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset);
 int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t sector_num,
-  uint8_t *out_buf, const uint8_t *in_buf,
-  int nb_sectors, bool enc, Error **errp);
+  uint8_t *buf, int nb_sectors, bool enc, Error 
**errp);
 
 int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
  unsigned int *bytes, uint64_t *cluster_offset);
-- 
2.9.3

[Qemu-devel] [PATCH v1 11/15] qcow2: convert QCow2 to use QCryptoBlock for encryption

2017-01-03 Thread Daniel P. Berrange

This converts the qcow2 driver to make use of the QCryptoBlock
APIs for encrypting image content, using the legacyy QCow2 AES
scheme.

With this change it is now required to use the QCryptoSecret
object for providing passwords, instead of the current block
password APIs / interactive prompting.

  $QEMU \
-object secret,id=sec0,filename=/home/berrange/encrypted.pw \
-drive file=/home/berrange/encrypted.qcow2,aes-key-secret=sec0

Signed-off-by: Daniel P. Berrange 
---
 block/qcow2-cluster.c  |  47 +--
 block/qcow2.c  | 190 +
 block/qcow2.h  |   5 +-
 qapi/block-core.json   |   7 +-
 tests/qemu-iotests/049 |   2 +-
 tests/qemu-iotests/049.out |   4 +-
 tests/qemu-iotests/082.out |  27 +++
 tests/qemu-iotests/087 |  28 ++-
 tests/qemu-iotests/087.out |   6 +-
 tests/qemu-iotests/134 |  18 +++--
 tests/qemu-iotests/134.out |  10 +--
 tests/qemu-iotests/158 |  19 +++--
 tests/qemu-iotests/158.out |  14 +---
 13 files changed, 219 insertions(+), 158 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 907e869..a2103dc 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -345,47 +345,6 @@ static int count_contiguous_clusters_by_type(int 
nb_clusters,
 return i;
 }
 
-/* The crypt function is compatible with the linux cryptoloop
-   algorithm for < 4 GB images. */
-int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t sector_num,
-  uint8_t *buf, int nb_sectors, bool enc,
-  Error **errp)
-{
-union {
-uint64_t ll[2];
-uint8_t b[16];
-} ivec;
-int i;
-int ret;
-
-for(i = 0; i < nb_sectors; i++) {
-ivec.ll[0] = cpu_to_le64(sector_num);
-ivec.ll[1] = 0;
-if (qcrypto_cipher_setiv(s->cipher,
- ivec.b, G_N_ELEMENTS(ivec.b),
- errp) < 0) {
-return -1;
-}
-if (enc) {
-ret = qcrypto_cipher_encrypt(s->cipher,
- buf, buf,
- 512,
- errp);
-} else {
-ret = qcrypto_cipher_decrypt(s->cipher,
- buf, buf,
- 512,
- errp);
-}
-if (ret < 0) {
-return -1;
-}
-sector_num++;
-buf += 512;
-}
-return 0;
-}
-
 static int coroutine_fn do_perform_cow(BlockDriverState *bs,
uint64_t src_cluster_offset,
uint64_t cluster_offset,
@@ -426,11 +385,11 @@ static int coroutine_fn do_perform_cow(BlockDriverState 
*bs,
 Error *err = NULL;
 int64_t sector = (src_cluster_offset + offset_in_cluster)
  >> BDRV_SECTOR_BITS;
-assert(s->cipher);
 assert((offset_in_cluster & ~BDRV_SECTOR_MASK) == 0);
 assert((bytes & ~BDRV_SECTOR_MASK) == 0);
-if (qcow2_encrypt_sectors(s, sector, iov.iov_base,
-  bytes >> BDRV_SECTOR_BITS, true, ) < 0) {
+assert(s->crypto);
+if (qcrypto_block_encrypt(s->crypto, sector, iov.iov_base,
+  bytes, ) < 0) {
 ret = -EIO;
 error_free(err);
 goto out;
diff --git a/block/qcow2.c b/block/qcow2.c
index 3c14c86..5c9e196 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -37,6 +37,9 @@
 #include "qemu/option_int.h"
 #include "qemu/cutils.h"
 #include "qemu/bswap.h"
+#include "qapi/opts-visitor.h"
+#include "qapi-visit.h"
+#include "block/crypto.h"
 
 /*
   Differences with QCOW:
@@ -461,6 +464,7 @@ static QemuOptsList qcow2_runtime_opts = {
 .type = QEMU_OPT_NUMBER,
 .help = "Clean unused cache entries after this time (in seconds)",
 },
+BLOCK_CRYPTO_OPT_DEF_QCOW_KEY_SECRET("aes-"),
 { /* end of list */ }
 },
 };
@@ -578,6 +582,7 @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts 
*opts,
 }
 }
 
+
 typedef struct Qcow2ReopenState {
 Qcow2Cache *l2_table_cache;
 Qcow2Cache *refcount_block_cache;
@@ -585,6 +590,7 @@ typedef struct Qcow2ReopenState {
 int overlap_check;
 bool discard_passthrough[QCOW2_DISCARD_MAX];
 uint64_t cache_clean_interval;
+QCryptoBlockOpenOptions *crypto_opts; /* Disk encryption runtime options */
 } Qcow2ReopenState;
 
 static int qcow2_update_options_prepare(BlockDriverState *bs,
@@ -751,6 +757,23 @@ static int qcow2_update_options_prepare(BlockDriverState 
*bs,
 r->discard_passthrough[QCOW2_DISCARD_OTHER] =
 qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_OTHER, false);
 
+switch (s->crypt_method_header) {
+case QCOW_CRYPT_NONE:
+break;
+
+

[Qemu-devel] [PATCH v1 09/15] qcow: convert QCow to use QCryptoBlock for encryption

2017-01-03 Thread Daniel P. Berrange

This converts the qcow2 driver to make use of the QCryptoBlock
APIs for encrypting image content. This is only wired up to
permit use of the legacy QCow encryption format. Users who wish
to have the strong LUKS format should switch to qcow2 instead.

With this change it is now required to use the QCryptoSecret
object for providing passwords, instead of the current block
password APIs / interactive prompting.

  $QEMU \
-object secret,id=sec0,filename=/home/berrange/encrypted.pw \
-drive file=/home/berrange/encrypted.qcow,aes-key-secret=sec0

Signed-off-by: Daniel P. Berrange 
---
 block/crypto.c   |  10 +++
 block/crypto.h   |   9 +++
 block/qcow.c | 184 +++
 qapi/block-core.json |  17 -
 4 files changed, 117 insertions(+), 103 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index 1037c70..915ba70 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -250,6 +250,11 @@ block_crypto_open_opts_init(QCryptoBlockFormat format,
 v, >u.luks, _err);
 break;
 
+case Q_CRYPTO_BLOCK_FORMAT_QCOW:
+visit_type_QCryptoBlockOptionsQCow_members(
+v, >u.qcow, _err);
+break;
+
 default:
 error_setg(_err, "Unsupported block format %d", format);
 break;
@@ -307,6 +312,11 @@ block_crypto_create_opts_init(QCryptoBlockFormat format,
 v, >u.luks, _err);
 break;
 
+case Q_CRYPTO_BLOCK_FORMAT_QCOW:
+visit_type_QCryptoBlockOptionsQCow_members(
+v, >u.qcow, _err);
+break;
+
 default:
 error_setg(_err, "Unsupported block format %d", format);
 break;
diff --git a/block/crypto.h b/block/crypto.h
index e70e2f0..1d64676 100644
--- a/block/crypto.h
+++ b/block/crypto.h
@@ -21,6 +21,15 @@
 #ifndef BLOCK_CRYPTO_H__
 #define BLOCK_CRYPTO_H__
 
+#define BLOCK_CRYPTO_OPT_QCOW_KEY_SECRET "key-secret"
+
+#define BLOCK_CRYPTO_OPT_DEF_QCOW_KEY_SECRET(prefix)\
+{   \
+.name = prefix BLOCK_CRYPTO_OPT_QCOW_KEY_SECRET,\
+.type = QEMU_OPT_STRING,\
+.help = "ID of the secret that provides the AES encryption key", \
+}
+
 #define BLOCK_CRYPTO_OPT_LUKS_KEY_SECRET "key-secret"
 #define BLOCK_CRYPTO_OPT_LUKS_CIPHER_ALG "cipher-alg"
 #define BLOCK_CRYPTO_OPT_LUKS_CIPHER_MODE "cipher-mode"
diff --git a/block/qcow.c b/block/qcow.c
index bc9fa2f..6232ad8 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -31,8 +31,9 @@
 #include "qemu/bswap.h"
 #include 
 #include "qapi/qmp/qerror.h"
-#include "crypto/cipher.h"
+#include "crypto/block.h"
 #include "migration/migration.h"
+#include "block/crypto.h"
 
 /**/
 /* QEMU COW block driver with compression and encryption support */
@@ -77,7 +78,7 @@ typedef struct BDRVQcowState {
 uint8_t *cluster_cache;
 uint8_t *cluster_data;
 uint64_t cluster_cache_offset;
-QCryptoCipher *cipher; /* NULL if no key yet */
+QCryptoBlock *crypto; /* Disk encryption format driver */
 uint32_t crypt_method_header;
 CoMutex lock;
 Error *migration_blocker;
@@ -97,6 +98,15 @@ static int qcow_probe(const uint8_t *buf, int buf_size, 
const char *filename)
 return 0;
 }
 
+static QemuOptsList qcow_runtime_opts = {
+.name = "qcow",
+.head = QTAILQ_HEAD_INITIALIZER(qcow_runtime_opts.head),
+.desc = {
+BLOCK_CRYPTO_OPT_DEF_QCOW_KEY_SECRET("aes-"),
+{ /* end of list */ }
+},
+};
+
 static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
  Error **errp)
 {
@@ -104,6 +114,18 @@ static int qcow_open(BlockDriverState *bs, QDict *options, 
int flags,
 unsigned int len, i, shift;
 int ret;
 QCowHeader header;
+QemuOpts *opts = NULL;
+Error *local_err = NULL;
+QCryptoBlockOpenOptions *crypto_opts = NULL;
+unsigned int cflags = 0;
+
+opts = qemu_opts_create(_runtime_opts, NULL, 0, _abort);
+qemu_opts_absorb_qdict(opts, options, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+ret = -EINVAL;
+goto fail;
+}
 
 ret = bdrv_pread(bs->file, 0, , sizeof(header));
 if (ret < 0) {
@@ -148,17 +170,6 @@ static int qcow_open(BlockDriverState *bs, QDict *options, 
int flags,
 goto fail;
 }
 
-if (header.crypt_method > QCOW_CRYPT_AES) {
-error_setg(errp, "invalid encryption method in qcow header");
-ret = -EINVAL;
-goto fail;
-}
-if (!qcrypto_cipher_supports(QCRYPTO_CIPHER_ALG_AES_128,
- QCRYPTO_CIPHER_MODE_CBC)) {
-error_setg(errp, "AES cipher not available");
-ret = -EINVAL;
-goto fail;
-}
 s->crypt_method_header = header.crypt_method;
 if (s->crypt_method_header) {
 if

[Qemu-devel] [PATCH v1 05/15] iotests: skip 042 with qcow which dosn't support zero sized images

2017-01-03 Thread Daniel P. Berrange

Test 042 is designed to verify operation with zero sized images.
Such images are not supported with qcow (v1), so this test has
always failed.

Signed-off-by: Daniel P. Berrange 
---
 tests/qemu-iotests/042 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/042 b/tests/qemu-iotests/042
index 351b283..a53e7cb 100755
--- a/tests/qemu-iotests/042
+++ b/tests/qemu-iotests/042
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.rc
 . ./common.filter
 
-_supported_fmt qcow2 qcow qed
+_supported_fmt qcow2 qed
 _supported_proto file
 _supported_os Linux
 
-- 
2.9.3

[Qemu-devel] [PATCH v1 03/15] qcow: document another weakness of qcow AES encryption

2017-01-03 Thread Daniel P. Berrange

Document that use of guest virtual sector numbers as the basis for
the initialization vectors is a potential weakness, when combined
with internal snapshots or multiple images using the same passphrase.

Signed-off-by: Daniel P. Berrange 
---
 qemu-img.texi | 9 +
 1 file changed, 9 insertions(+)

diff --git a/qemu-img.texi b/qemu-img.texi
index 174aae3..8efcf89 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -554,6 +554,15 @@ change the passphrase to protect data in any qcow images. 
The files must
 be cloned, using a different encryption passphrase in the new file. The
 original file must then be securely erased using a program like shred,
 though even this is ineffective with many modern storage technologies.
+@item Initialization vectors used to encrypt sectors are based on the
+guest virtual sector number, instead of the host physical sector. When
+a disk image has multiple internal snapshots this means that data in
+multiple physical sectors is encrypted with the same initialization
+vector. With the CBC mode, this opens the possibility of watermarking
+attacks if the attack can collect multiple sectors encrypted with the
+same IV and some predictable data. Having multiple qcow2 images with
+the same passphrase also exposes this weakness since the passphrase
+is directly used as the key.
 @end itemize
 
 Use of qcow / qcow2 encryption is thus strongly discouraged. Users are
-- 
2.9.3

[Qemu-devel] [PATCH v1 04/15] qcow: require image size to be > 1 for new images

2017-01-03 Thread Daniel P. Berrange

The qcow driver refuses to open images which are less than
2 bytes in size, but will happily create such images. Add
a check in the create path to avoid this discrepancy.

Signed-off-by: Daniel P. Berrange 
---
 block/qcow.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/block/qcow.c b/block/qcow.c
index 7540f43..8133fda 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -799,6 +799,12 @@ static int qcow_create(const char *filename, QemuOpts 
*opts, Error **errp)
 /* Read out options */
 total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
   BDRV_SECTOR_SIZE);
+if (total_size <= 1) {
+error_setg(errp, "Image size is too small (must be at least 2 bytes)");
+ret = -EINVAL;
+goto cleanup;
+}
+
 backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
 if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
 flags |= BLOCK_FLAG_ENCRYPT;
-- 
2.9.3

[Qemu-devel] [PATCH v1 00/15] Convert QCow[2] to QCryptoBlock & add LUKS support

2017-01-03 Thread Daniel P. Berrange

This series is a continuation of previous work to support LUKS in
QEMU. The existing merged code supports LUKS as a standalone
driver which can be layered over/under any other QEMU block device
driver. This works well when using LUKS over protocol drivers (file,
rbd, iscsi, etc, etc), but has some downsides when combined with
format drivers like qcow2.

If you layer LUKS under qcow2 (eg qcow2 -> luks -> file) then you
cannot get any information about the qcow2 file without first
decrypting it, as both the header and payload are encrypted.

If you layer LUKS over qcow2 (eg luks -> qcow2 -> file) then you
cannot distinguish between a qcow2 file where the guest has done
LUKS encryption from a qcow2 file which qemu has done encryption.
More seriously, when encrypting sectors the guest virtual sector
is used as the input for deriving the initialization vectors.
When internal snapshots are used, this means that multiple sectors
in the qcow2 file may be encrypted with the same initialization
vector. This is a security weakness when combined with certain
cryptographic modes.

Integrating LUKS natively into qcow2 allows us to combine the
best aspects of both layering strategies above. In particular
the header remains unecrypted, but initialization vectors are
generated using physical sector numbers preserving security
when snapshots are used. This is a change from previous postings
of this work, where the IVs were (incorrectly) generated based
on the virtual disk sector.

In a previous posting of this work, Fam had suggested that we
do integration by layering luks over qcow2, but having QEMU
block layer automatically create the luks driver above qcow2
based on the qcow2 header crypt_method field. This is not
possible though, because such a scheme would suffer from the
problem of IVs being generated from the virtual disk sector
instead of physical disk sector. So having LUKS specific
code in the qcow2 block driver is unavoidable. In comparison
to the previous posting though, the amount of code in qcow2.c
has been reduced by allowing re-use of code from block/crypto.c
for handling QemuOpts -> QAPI conversion. So extra lines of
code in qcow2 to support LUKS is < 200.

I have also split the changes to qcow2 up into 2 patches. The
first patch simply introduces use of the QCryptoBlock framework
to qcow2 for the existing (deprecated) AES-CBC encryption method.
The second patch wires up the LUKS support for qcow2. This makes
it clearer which parts of the changes are related to plain code
refactoring, vs enabling the new features. Specifically we can
now see that the LUKS enablement in qcow2 has this footprint:

  block/qcow2-cluster.c  |   4 +-
  block/qcow2-refcount.c |  10 +
  block/qcow2.c  | 236 
+---
  block/qcow2.h  |   9 
  4 files changed, 224 insertions(+), 35 deletions(-)

Main changes since previous posting

 - Make sure iotests work for qcow v1
 - Fix bugs in qcow v1 conversion to QCryptoBlock which
   caused segv with legacy AES encryption
 - Split qcow2 conversion into two parts, one converting
   to QCryptoBlock just for legacy AES, one adding LUKS
   support
 - Enable more iotests for qcow to exercise legacy AES
   code path
 - Refactor generic LUKS block driver to allow code
   sharing with the qcow2 LUKS integration
 - Switch to use physical sector when generating
   initialization vectors for LUKS with qcow2 to
   avoid security weakness with snapshots.

Daniel P. Berrange (15):
  block: expose crypto option names / defs to other drivers
  block: add ability to set a prefix for opt names
  qcow: document another weakness of qcow AES encryption
  qcow: require image size to be > 1 for new images
  iotests: skip 042 with qcow which dosn't support zero sized images
  iotests: skip 048 with qcow which doesn't support resize
  iotests: fix 097 when run with qcow
  qcow: make encrypt_sectors encrypt in place
  qcow: convert QCow to use QCryptoBlock for encryption
  qcow2: make qcow2_encrypt_sectors encrypt in place
  qcow2: convert QCow2 to use QCryptoBlock for encryption
  qcow2: add support for LUKS encryption format
  iotests: enable tests 134 and 158 to work with qcow (v1)
  block: rip out all traces of password prompting
  block: remove all encryption handling APIs

 block.c|  77 +
 block/crypto.c | 166 ---
 block/crypto.h | 102 
 block/qapi.c   |   2 +-
 block/qcow.c   | 207 +++-
 block/qcow2-cluster.c  |  56 +--
 block/qcow2-refcount.c |  10 ++
 block/qcow2.c  | 385 +++--
 block/qcow2.h  |  17 +-
 blockdev.c |  37 +
 docs/specs/qcow2.txt   |  99 
 hmp.c  |  31 
 include/block/block.h  |   3 -
 include/block/block_int.h  |   1

[Qemu-devel] [PATCH v1 02/15] block: add ability to set a prefix for opt names

2017-01-03 Thread Daniel P. Berrange

When integrating the crypto support with qcow/qcow2, we don't
want to use the bare LUKS option names "hash-alg", "key-secret",
etc. We want to namespace them "luks-hash-alg", "luks-key-secret"
so that they don't clash with any general qcow options at a later
date.

Signed-off-by: Daniel P. Berrange 
---
 block/crypto.c | 110 +
 block/crypto.h |  42 +++---
 2 files changed, 118 insertions(+), 34 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index d281de6..1037c70 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -128,7 +128,7 @@ static QemuOptsList block_crypto_runtime_opts_luks = {
 .name = "crypto",
 .head = QTAILQ_HEAD_INITIALIZER(block_crypto_runtime_opts_luks.head),
 .desc = {
-BLOCK_CRYPTO_OPT_DEF_LUKS_KEY_SECRET,
+BLOCK_CRYPTO_OPT_DEF_LUKS_KEY_SECRET(""),
 { /* end of list */ }
 },
 };
@@ -143,31 +143,101 @@ static QemuOptsList block_crypto_create_opts_luks = {
 .type = QEMU_OPT_SIZE,
 .help = "Virtual disk size"
 },
-BLOCK_CRYPTO_OPT_DEF_LUKS_KEY_SECRET,
-BLOCK_CRYPTO_OPT_DEF_LUKS_CIPHER_ALG,
-BLOCK_CRYPTO_OPT_DEF_LUKS_CIPHER_MODE,
-BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_ALG,
-BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_HASH_ALG,
-BLOCK_CRYPTO_OPT_DEF_LUKS_HASH_ALG,
-BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME,
+BLOCK_CRYPTO_OPT_DEF_LUKS_KEY_SECRET(""),
+BLOCK_CRYPTO_OPT_DEF_LUKS_CIPHER_ALG(""),
+BLOCK_CRYPTO_OPT_DEF_LUKS_CIPHER_MODE(""),
+BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_ALG(""),
+BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_HASH_ALG(""),
+BLOCK_CRYPTO_OPT_DEF_LUKS_HASH_ALG(""),
+BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME(""),
 { /* end of list */ }
 },
 };
 
+static QemuOptsList empty_opts = {
+.name = "crypto-empty",
+.merge_lists = false,
+.head = QTAILQ_HEAD_INITIALIZER(empty_opts.head),
+.desc = {
+/* no elements => accept any params */
+{ /* end of list */ }
+},
+};
+
+
+struct BlockCryptoCopyData {
+QemuOpts *opts;
+const char *prefix;
+};
+
+static int block_crypto_copy_value(void *opaque, const char *name,
+   const char *value, Error **errp)
+{
+struct BlockCryptoCopyData *data = opaque;
+
+if (g_str_has_prefix(name, data->prefix)) {
+Error *local_err = NULL;
+const char *newname = name + strlen(data->prefix);
+
+qemu_opt_set(data->opts, newname, value, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+return 1;
+}
+}
+
+return 0;
+}
+
+/*
+ * Create a copy of @opts containing only the fields with
+ * a prefix of @prefix, stripping the prefix in the returned
+ * opts
+ */
+static QemuOpts *
+block_crypto_copy_opts(QemuOpts *opts,
+   const char *prefix,
+   Error **errp)
+{
+struct BlockCryptoCopyData data = {
+.opts = qemu_opts_create(_opts, NULL, false, errp),
+.prefix = prefix
+};
+if (!data.opts) {
+return NULL;
+}
+
+if (qemu_opt_foreach(opts, block_crypto_copy_value, , errp) != 0) {
+qemu_opts_del(data.opts);
+return NULL;
+}
+
+return data.opts;
+}
 
 QCryptoBlockOpenOptions *
 block_crypto_open_opts_init(QCryptoBlockFormat format,
 QemuOpts *opts,
+const char *prefix,
 Error **errp)
 {
-Visitor *v;
+Visitor *v = NULL;
 QCryptoBlockOpenOptions *ret = NULL;
 Error *local_err = NULL;
+QemuOpts *newopts = NULL;
 
 ret = g_new0(QCryptoBlockOpenOptions, 1);
 ret->format = format;
 
-v = opts_visitor_new(opts);
+if (prefix != NULL) {
+newopts = block_crypto_copy_opts(opts, prefix, _err);
+if (local_err) {
+goto out;
+}
+v = opts_visitor_new(newopts);
+} else {
+v = opts_visitor_new(opts);
+}
 
 visit_start_struct(v, NULL, NULL, 0, _err);
 if (local_err) {
@@ -196,6 +266,7 @@ block_crypto_open_opts_init(QCryptoBlockFormat format,
 qapi_free_QCryptoBlockOpenOptions(ret);
 ret = NULL;
 }
+qemu_opts_del(newopts);
 visit_free(v);
 return ret;
 }
@@ -204,16 +275,26 @@ block_crypto_open_opts_init(QCryptoBlockFormat format,
 QCryptoBlockCreateOptions *
 block_crypto_create_opts_init(QCryptoBlockFormat format,
   QemuOpts *opts,
+  const char *prefix,
   Error **errp)
 {
-Visitor *v;
+Visitor *v = NULL;
 QCryptoBlockCreateOptions *ret = NULL;
 Error *local_err = NULL;
+QemuOpts *newopts = NULL;
 
 ret = g_new0(QCryptoBlockCreateOptions, 1);
 ret->format = format;
 
-v = opts_visitor_new(opts);
+if (prefix != NULL) {
+newopts =

[Qemu-devel] [PATCH v1 01/15] block: expose crypto option names / defs to other drivers

2017-01-03 Thread Daniel P. Berrange

The block/crypto.c defines a set of QemuOpts that provide
parameters for encryption. This will also be needed by
the qcow/qcow2 integration, so expose the relevant pieces
in a new block/crypto.h header.

Signed-off-by: Daniel P. Berrange 
---
 block/crypto.c | 61 +++
 block/crypto.h | 91 ++
 2 files changed, 102 insertions(+), 50 deletions(-)
 create mode 100644 block/crypto.h

diff --git a/block/crypto.c b/block/crypto.c
index 7aa7eb5..d281de6 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -26,14 +26,7 @@
 #include "qapi/opts-visitor.h"
 #include "qapi-visit.h"
 #include "qapi/error.h"
-
-#define BLOCK_CRYPTO_OPT_LUKS_KEY_SECRET "key-secret"
-#define BLOCK_CRYPTO_OPT_LUKS_CIPHER_ALG "cipher-alg"
-#define BLOCK_CRYPTO_OPT_LUKS_CIPHER_MODE "cipher-mode"
-#define BLOCK_CRYPTO_OPT_LUKS_IVGEN_ALG "ivgen-alg"
-#define BLOCK_CRYPTO_OPT_LUKS_IVGEN_HASH_ALG "ivgen-hash-alg"
-#define BLOCK_CRYPTO_OPT_LUKS_HASH_ALG "hash-alg"
-#define BLOCK_CRYPTO_OPT_LUKS_ITER_TIME "iter-time"
+#include "block/crypto.h"
 
 typedef struct BlockCrypto BlockCrypto;
 
@@ -135,11 +128,7 @@ static QemuOptsList block_crypto_runtime_opts_luks = {
 .name = "crypto",
 .head = QTAILQ_HEAD_INITIALIZER(block_crypto_runtime_opts_luks.head),
 .desc = {
-{
-.name = BLOCK_CRYPTO_OPT_LUKS_KEY_SECRET,
-.type = QEMU_OPT_STRING,
-.help = "ID of the secret that provides the encryption key",
-},
+BLOCK_CRYPTO_OPT_DEF_LUKS_KEY_SECRET,
 { /* end of list */ }
 },
 };
@@ -154,47 +143,19 @@ static QemuOptsList block_crypto_create_opts_luks = {
 .type = QEMU_OPT_SIZE,
 .help = "Virtual disk size"
 },
-{
-.name = BLOCK_CRYPTO_OPT_LUKS_KEY_SECRET,
-.type = QEMU_OPT_STRING,
-.help = "ID of the secret that provides the encryption key",
-},
-{
-.name = BLOCK_CRYPTO_OPT_LUKS_CIPHER_ALG,
-.type = QEMU_OPT_STRING,
-.help = "Name of encryption cipher algorithm",
-},
-{
-.name = BLOCK_CRYPTO_OPT_LUKS_CIPHER_MODE,
-.type = QEMU_OPT_STRING,
-.help = "Name of encryption cipher mode",
-},
-{
-.name = BLOCK_CRYPTO_OPT_LUKS_IVGEN_ALG,
-.type = QEMU_OPT_STRING,
-.help = "Name of IV generator algorithm",
-},
-{
-.name = BLOCK_CRYPTO_OPT_LUKS_IVGEN_HASH_ALG,
-.type = QEMU_OPT_STRING,
-.help = "Name of IV generator hash algorithm",
-},
-{
-.name = BLOCK_CRYPTO_OPT_LUKS_HASH_ALG,
-.type = QEMU_OPT_STRING,
-.help = "Name of encryption hash algorithm",
-},
-{
-.name = BLOCK_CRYPTO_OPT_LUKS_ITER_TIME,
-.type = QEMU_OPT_NUMBER,
-.help = "Time to spend in PBKDF in milliseconds",
-},
+BLOCK_CRYPTO_OPT_DEF_LUKS_KEY_SECRET,
+BLOCK_CRYPTO_OPT_DEF_LUKS_CIPHER_ALG,
+BLOCK_CRYPTO_OPT_DEF_LUKS_CIPHER_MODE,
+BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_ALG,
+BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_HASH_ALG,
+BLOCK_CRYPTO_OPT_DEF_LUKS_HASH_ALG,
+BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME,
 { /* end of list */ }
 },
 };
 
 
-static QCryptoBlockOpenOptions *
+QCryptoBlockOpenOptions *
 block_crypto_open_opts_init(QCryptoBlockFormat format,
 QemuOpts *opts,
 Error **errp)
@@ -240,7 +201,7 @@ block_crypto_open_opts_init(QCryptoBlockFormat format,
 }
 
 
-static QCryptoBlockCreateOptions *
+QCryptoBlockCreateOptions *
 block_crypto_create_opts_init(QCryptoBlockFormat format,
   QemuOpts *opts,
   Error **errp)
diff --git a/block/crypto.h b/block/crypto.h
new file mode 100644
index 000..e42f20e
--- /dev/null
+++ b/block/crypto.h
@@ -0,0 +1,91 @@
+/*
+ * QEMU block full disk encryption
+ *
+ * Copyright (c) 2015-2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef BLOCK_CRYPTO_H__
+#define BLOCK_CRYPTO_H__
+
+#define BLOCK_CRYPTO_OPT_LUKS_KEY_SECRET "key-secret"
+#define BLOCK_CRYPTO_OPT_LUKS_CIPHER_ALG

[Qemu-devel] [PATCH v2 1/2] Do endian swapping.

2017-01-03 Thread Michal Suchanek

This allows running big endian and little endian guest side by side using
cut & paste between them.

There is a general design idea that swapping should come as close to
virtio_read/virtio_write as possible. In particular, the protocol
between vdagent and vdagentd is guest-specific and in native endian.
With muliple layers of headers this is a bit tricky. A few message types
have to be swapped fully before passing through vdagentd.

Signed-off-by: Michal Suchanek 

---
v2:
 - introduce helper functions to swap (a portion of) a message wholesale
 - pollute fewer places with swapping sometimes at the cost of slightly
   more verbose code
---
 src/vdagentd/vdagentd.c| 99 +-
 src/vdagentd/virtio-port.c | 35 ++--
 2 files changed, 102 insertions(+), 32 deletions(-)

diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
index a1faf23..991514e 100644
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -78,6 +78,34 @@ static int client_connected = 0;
 static int max_clipboard = -1;
 
 /* utility functions */
+static void virtio_msg_htole32(uint8_t *_msg, uint32_t size, uint32_t offset)
+{
+uint32_t i, *msg = (uint32_t *)(_msg + offset);
+
+/* offset - size % 4 should be 0 */
+for (i = 0; i < (size - offset) / 4; i++)
+msg[i] = htole32(msg[i]);
+}
+
+static void virtio_msg_le32toh(uint8_t *_msg, uint32_t size, uint32_t offset)
+{
+uint32_t i, *msg = (uint32_t *)(_msg + offset);
+
+/* offset - size % 4 should be 0 */
+for (i = 0; i < (size - offset) / 4; i++)
+msg[i] = le32toh(msg[i]);
+}
+
+static void virtio_msg_le16toh(uint8_t *_msg, uint32_t size, uint32_t offset)
+{
+uint32_t i;
+uint16_t *msg = (uint16_t *)(_msg + offset);
+
+/* offset - size % 2 should be 0 */
+for (i = 0; i < (size - offset) / 2; i++)
+msg[i] = le16toh(msg[i]);
+}
+
 /* vdagentd <-> spice-client communication handling */
 static void send_capabilities(struct vdagent_virtio_port *vport,
 uint32_t request)
@@ -102,6 +130,7 @@ static void send_capabilities(struct vdagent_virtio_port 
*vport,
 VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_GUEST_LINEEND_LF);
 VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_MAX_CLIPBOARD);
 VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_AUDIO_VOLUME_SYNC);
+virtio_msg_htole32((uint8_t *)caps, size, 0);
 
 vdagent_virtio_port_write(vport, VDP_CLIENT_PORT,
   VD_AGENT_ANNOUNCE_CAPABILITIES, 0,
@@ -151,8 +180,8 @@ static void do_client_monitors(struct vdagent_virtio_port 
*vport, int port_nr,
 (uint8_t *)mon_config, size);
 
 /* Acknowledge reception of monitors config to spice server / client */
-reply.type  = VD_AGENT_MONITORS_CONFIG;
-reply.error = VD_AGENT_SUCCESS;
+reply.type  = htole32(VD_AGENT_MONITORS_CONFIG);
+reply.error = htole32(VD_AGENT_SUCCESS);
 vdagent_virtio_port_write(vport, port_nr, VD_AGENT_REPLY, 0,
   (uint8_t *), sizeof(reply));
 }
@@ -255,8 +284,8 @@ static void send_file_xfer_status(struct 
vdagent_virtio_port *vport,
   const char *msg, uint32_t id, uint32_t 
xfer_status)
 {
 VDAgentFileXferStatusMessage status = {
-.id = id,
-.result = xfer_status,
+.id = htole32(id),
+.result = htole32(xfer_status),
 };
 syslog(LOG_WARNING, msg, id);
 if (vport)
@@ -333,6 +362,7 @@ static int virtio_port_read_complete(
 case VD_AGENT_MOUSE_STATE:
 if (message_header->size != sizeof(VDAgentMouseState))
 goto size_error;
+virtio_msg_le32toh(data, message_header->size, 0);
 vdagentd_uinput_do_mouse(, (VDAgentMouseState *)data);
 if (!uinput) {
 /* Try to re-open the tablet */
@@ -356,12 +386,14 @@ static int virtio_port_read_complete(
 case VD_AGENT_MONITORS_CONFIG:
 if (message_header->size < sizeof(VDAgentMonitorsConfig))
 goto size_error;
+virtio_msg_le32toh(data, message_header->size, 0);
 do_client_monitors(vport, port_nr, message_header,
 (VDAgentMonitorsConfig *)data);
 break;
 case VD_AGENT_ANNOUNCE_CAPABILITIES:
 if (message_header->size < sizeof(VDAgentAnnounceCapabilities))
 goto size_error;
+virtio_msg_le32toh(data, message_header->size, 0);
 do_client_capabilities(vport, message_header,
 (VDAgentAnnounceCapabilities *)data);
 break;
@@ -369,26 +401,50 @@ static int virtio_port_read_complete(
 case VD_AGENT_CLIPBOARD_REQUEST:
 case VD_AGENT_CLIPBOARD:
 case VD_AGENT_CLIPBOARD_RELEASE:
+if (VD_AGENT_HAS_CAPABILITY(capabilities, capabilities_size,
+VD_AGENT_CAP_CLIPBOARD_SELECTION))
+min_size += 4;
+uint32_t *data_type = (uint32_t *)(data + min_size);

[Qemu-devel] [PATCH v2 2/2] Quiet uninitialized variable warning.

2017-01-03 Thread Michal Suchanek

Signed-off-by: Michal Suchanek 
---
 src/vdagentd/vdagentd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
index 991514e..60a866e 100644
--- a/src/vdagentd/vdagentd.c
+++ b/src/vdagentd/vdagentd.c
@@ -334,6 +334,7 @@ static void do_client_file_xfer(struct vdagent_virtio_port 
*vport,
 id = d->id;
 break;
 }
+default: return; /* quiet uninitialized variable warning */
 }
 
 conn = g_hash_table_lookup(active_xfers, GUINT_TO_POINTER(id));
-- 
2.10.2

Re: [Qemu-devel] New Year's starting over ... bsd-user

2017-01-03 Thread Sean Bruno

On 01/03/17 09:18, Sean Bruno wrote:
> 
> I'm pondering where to start with getting FreeBSD's bsd-user code into
> shape so it could actually be reviewed and accepted now that its sort of
> working again (signal handling fixed finally).
> 
> I almost feel like the existing code should be purged, except that it
> gives a good history (and this seems lazy to me).
> 
> As a first pass, I guess, I'd like to at least make i386 user run on
> x86_64.  What would you folks like to see in a first pass?
> 
> sean
> 
> ref: https://github.com/seanbruno/qemu-bsd-user/tree/bsd-user
> 

Primitive example of what I think I should base my patchset on.  Its
invasive and large.

https://github.com/seanbruno/qemu-bsd-user/tree/merge1

That branch, is all the bsd-user changes that are pending in one large
"splat".  It excludes the new architectures (arm, aarch64, mips, mips64)
that we are actively using.  i386-bsd-user when compiled statically on
x86_64 will run a static (rescue) sh ... so, I think that's good.
x86_64 running on x86_64 just blows up.

As for sparc/sparc64 ... I'm tempted to delete them as nobody in freebsd
is actively maintaining them nor do we have any expectation that they
will work someday.

sean

signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] vhost-user breaks after 96a3d98.

2017-01-03 Thread Flavio Leitner

On Tue, 3 Jan 2017 18:28:18 +0800
Jason Wang  wrote:

> On 2017年01月03日 11:09, Jason Wang wrote:
> >
> >
> > On 2016年12月30日 20:41, Flavio Leitner wrote:  
> >> Hi,
> >>
> >> While I was testing vhost-user using OVS 2.5 and DPDK 2.2.0 in the
> >> host and testpmd dpdk 2.2.0 in the guest, I found that the commit
> >> below breaks the environment and no packets gets into the guest.
> >>
> >> dpdk port --> OVS --> vhost-user --> guest --> testpmd
> >>   ^--- drops here ^--- no packets here.
> >>
> >> commit 96a3d98d2cdbd897ff5ab33427aa4cfb94077665
> >> Author: Jason Wang 
> >> Date:   Mon Aug 1 16:07:58 2016 +0800
> >>
> >>  vhost: don't set vring call if no vector
> >>   We used to set vring call fd unconditionally even if guest 
> >> driver does
> >>  not use MSIX for this vritqueue at all. This will cause lots of
> >>  unnecessary userspace access and other checks for drivers does 
> >> not use
> >>  interrupt at all (e.g virtio-net pmd). So check and clean vring 
> >> call
> >>  fd if guest does not use any vector for this virtqueue at
> >>  all.
> >> [...]
> >>
> >> Thanks,  
> >
> > Hi Flavio:
> >
> > Thanks for reporting this issue, could this be a bug of vhost-user? (I 
> > believe virito-net pmd does not use interrupt for rx/tx at all)
> >
> > Anyway, will try to reproduce it.
> >  
> 
> Could not reproduce this issue on similar setups (the only difference is 
> I don't create dpdk port) with dpdk 16.11 and ovs.git HEAD. Suspect an 
> issue dpdk. Will try OVS 2.5 + DPDK 2.2.0.

Yeah, that's the combo I am testing and seeing the issue.  I found the
commit after bisecting qemu and then confirmed by testing up to the
previous commit (works okay) and then the commit above (fails).

I still have my test environment available, so I would be able to test
any patch you might have.

Thanks,
-- 
Flavio

Re: [Qemu-devel] [PATCH v2 1/5] iotests: skip 159 & 170 with luks format

2017-01-03 Thread Eric Blake

On 01/03/2017 11:04 AM, Daniel P. Berrange wrote:
> While the qemu-img dd command does accept --image-opts
> this is not sufficient to make it work with the LUKS
> image yet. This is because bdrv_create() still always
> requires the non-image-opts syntax.
> 
> Thus we must skip 159/170 with luks for now
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  tests/qemu-iotests/159 | 2 +-
>  tests/qemu-iotests/170 | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Eric Blake 

-- 
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 v2] iotests: record separate timings per format, protocol pair

2017-01-03 Thread Eric Blake

On 01/03/2017 10:05 AM, Daniel P. Berrange wrote:
> The 'check' program records timings for each test that
> is run. These timings are only valid, however, for a
> particular format/protocol combination. So if frequently
> running 'check' with a variety of different formats or
> protocols, the times printed can be very misleading.
> 
> Instead of having a single 'check.time' file, maintain
> multiple 'check.time-$IMGPROTO-$IMGFMT' files.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
> Changed in v2:
> 
>  - Use a separate file per format/protocol, instead of
>throwing away data each time format/protocol changes
>between runs (Eric)
> 

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v2 3/5] iotests: reduce PBKDF iterations when testing LUKS

2017-01-03 Thread Daniel P. Berrange

By default the PBKDF algorithm used with LUKS is tuned
based on the number of iterations to produce 1 second
of running time. This makes running the I/O test with
the LUKS format orders of magnitude slower than with
qcow2/raw formats.

When creating LUKS images, set the iteration time to
a 10ms to reduce the time overhead for LUKS, since
security does not matter in I/O tests.

Previsouly a full 'check -luks' would take

  $ time ./check -luks
  Passed all 22 tests

  real  23m9.988s
  user  21m46.223s
  sys   0m22.841s

Now it takes

  $ time ./check -luks
  Passed all 22 tests

  real  4m39.235s
  user  3m29.590s
  sys   0m24.234s

Still slow compared to qcow2/raw, but much improved
none the less.

Signed-off-by: Daniel P. Berrange 
---
 tests/qemu-iotests/149   |   3 +
 tests/qemu-iotests/149.out   | 118 +++
 tests/qemu-iotests/common.filter |   3 +-
 tests/qemu-iotests/common.rc |   3 +
 4 files changed, 67 insertions(+), 60 deletions(-)

diff --git a/tests/qemu-iotests/149 b/tests/qemu-iotests/149
index 8407251..f62e618 100755
--- a/tests/qemu-iotests/149
+++ b/tests/qemu-iotests/149
@@ -136,6 +136,7 @@ def cryptsetup_add_password(config, slot):
 args = ["luksAddKey", config.image_path(),
 "--key-slot", slot,
 "--key-file", "-",
+"--iter-time", "10",
 pwfile]
 
 cryptsetup(args, password)
@@ -164,6 +165,7 @@ def cryptsetup_format(config):
 args.extend(["--hash", config.hash])
 args.extend(["--key-slot", slot])
 args.extend(["--key-file", "-"])
+args.extend(["--iter-time", "10"])
 args.append(config.image_path())
 
 cryptsetup(args, password)
@@ -230,6 +232,7 @@ def qemu_img_create(config, size_mb):
 
 opts = [
 "key-secret=sec0",
+"iter-time=10",
 "cipher-alg=%s-%d" % (config.cipher, config.keylen),
 "cipher-mode=%s" % config.mode,
 "ivgen-alg=%s" % config.ivgen,
diff --git a/tests/qemu-iotests/149.out b/tests/qemu-iotests/149.out
index 90b5b55..b18c4e4 100644
--- a/tests/qemu-iotests/149.out
+++ b/tests/qemu-iotests/149.out
@@ -2,7 +2,7 @@
 # Create image
 truncate TEST_DIR/luks-aes-256-xts-plain64-sha1.img --size 4194304MB
 # Format image
-sudo cryptsetup -q -v luksFormat --cipher aes-xts-plain64 --key-size 512 
--hash sha1 --key-slot 0 --key-file - TEST_DIR/luks-aes-256-xts-plain64-sha1.img
+sudo cryptsetup -q -v luksFormat --cipher aes-xts-plain64 --key-size 512 
--hash sha1 --key-slot 0 --key-file - --iter-time 10 
TEST_DIR/luks-aes-256-xts-plain64-sha1.img
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-xts-plain64-sha1.img 
qiotest-145-aes-256-xts-plain64-sha1
 # Set dev owner
@@ -60,8 +60,8 @@ unlink TEST_DIR/luks-aes-256-xts-plain64-sha1.img
 
 # = qemu-img aes-256-xts-plain64-sha1 =
 # Create image
-qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,cipher-alg=aes-256,cipher-mode=xts,ivgen-alg=plain64,hash-alg=sha1
 TEST_DIR/luks-aes-256-xts-plain64-sha1.img 4194304M
-Formatting 'TEST_DIR/luks-aes-256-xts-plain64-sha1.img', fmt=luks 
size=4398046511104 key-secret=sec0 cipher-alg=aes-256 cipher-mode=xts 
ivgen-alg=plain64 hash-alg=sha1
+qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,iter-time=10,cipher-alg=aes-256,cipher-mode=xts,ivgen-alg=plain64,hash-alg=sha1
 TEST_DIR/luks-aes-256-xts-plain64-sha1.img 4194304M
+Formatting 'TEST_DIR/luks-aes-256-xts-plain64-sha1.img', fmt=luks 
size=4398046511104 key-secret=sec0 cipher-alg=aes-256 cipher-mode=xts 
ivgen-alg=plain64 hash-alg=sha1 iter-time=10
 
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-xts-plain64-sha1.img 
qiotest-145-aes-256-xts-plain64-sha1
@@ -122,7 +122,7 @@ unlink TEST_DIR/luks-aes-256-xts-plain64-sha1.img
 # Create image
 truncate TEST_DIR/luks-twofish-256-xts-plain64-sha1.img --size 4194304MB
 # Format image
-sudo cryptsetup -q -v luksFormat --cipher twofish-xts-plain64 --key-size 512 
--hash sha1 --key-slot 0 --key-file - 
TEST_DIR/luks-twofish-256-xts-plain64-sha1.img
+sudo cryptsetup -q -v luksFormat --cipher twofish-xts-plain64 --key-size 512 
--hash sha1 --key-slot 0 --key-file - --iter-time 10 
TEST_DIR/luks-twofish-256-xts-plain64-sha1.img
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-twofish-256-xts-plain64-sha1.img 
qiotest-145-twofish-256-xts-plain64-sha1
 # Set dev owner
@@ -180,8 +180,8 @@ unlink TEST_DIR/luks-twofish-256-xts-plain64-sha1.img
 
 # = qemu-img twofish-256-xts-plain64-sha1 =
 # Create image
-qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,cipher-alg=twofish-256,cipher-mode=xts,ivgen-alg=plain64,hash-alg=sha1
 TEST_DIR/luks-twofish-256-xts-plain64-sha1.img 4194304M
-Formatting 'TEST_DIR/luks-twofish-256-xts-plain64-sha1.img',

Re: [Qemu-devel] [PATCH 3/3] fix qmp/hmp query-memdev not repporting IDs of memory backends

2017-01-03 Thread Igor Mammedov

On Tue, 3 Jan 2017 09:34:40 -0600
Eric Blake  wrote:

> On 01/02/2017 09:44 AM, Igor Mammedov wrote:
> 
> s/repporting/reporting/ in the subject
> 
> > Do it by adding 'id' property to hostmem backends and fetch it
> > in query-memdev from object directly.
> > 
> > Signed-off-by: Igor Mammedov 
> > ---
> 
> > +++ b/qom/object_interfaces.c
> > @@ -4,6 +4,7 @@
> >  #include "qemu/module.h"
> >  #include "qapi-visit.h"
> >  #include "qapi/opts-visitor.h"
> > +#include "qapi/qmp/qstring.h"
> >  
> >  void user_creatable_complete(Object *obj, Error **errp)
> >  {
> > @@ -35,7 +36,7 @@ bool user_creatable_can_be_deleted(UserCreatable *uc, 
> > Error **errp)
> >  }
> >  
> >  Object *user_creatable_add_type(const char *type, const char *id,
> > -const QDict *qdict,
> > +QDict *qdict,
> >  Visitor *v, Error **errp)
> >  {
> >  Object *obj;
> > @@ -62,6 +63,9 @@ Object *user_creatable_add_type(const char *type, const 
> > char *id,
> >  
> >  assert(qdict);
> >  obj = object_new(type);
> > +if (object_property_find(obj, "id", NULL)) {
> > +qdict_put(qdict, "id", qstring_from_str(id));
> > +}
> 
> Wait. Isn't this going to inject an 'id' dict member to every use of
> user_creatable_add_type()?  But not all QAPI structs contain an id
> member.
it would 'inject' 'id' only for objects that have 'id' property.
or more exactly it would restore 'id' that is deleted earlier from
the same qdict. Look for qdict_del() usage in this file.
This way it'd be able to process both kinds of objects that implement
'id' property and ones that don't.
 
> Which means that you are now explicitly relying on the visitor
> to silently ignore garbage in the dictionary, rather than our desired
> goal of only validating if the dictionary exactly matches what the QAPI
> says it will match.
currently 'id' is mandatory, but not enforced by QAPI structs,
for all objects created with -object/object_add as it's used for
naming object in qom tree:
  object_property_add_child(object_get_objects_root(), id, obj, 
_err); 

> 
> I'm not sure if I like this hack, or if there is a better way to do
> things when using a strict (rather than relaxed) input visitor.
I'm not sure about a better way to do this.
Currently we use 2 visitors: opts (for cli/hmp) and qobject_input for qmp path,
which accept anything as target object could take different options.

The only things we currently enforce in code is presence of implicit 'qom-type' 
property
and 'id' property. The rest of properties are validated by object's property 
setters.

> 
> >  visit_start_struct(v, NULL, NULL, 0, _err);
> >  if (local_err) {
> >  goto out;
> > 
>

[Qemu-devel] [PATCH] scsi-block: fix direction of BYTCHK test for VERIFY commands

2017-01-03 Thread Paolo Bonzini

The direction is wrong; scsi_block_is_passthrough returns
false for commands that *can* use sglists.

Reported-by: Zhang Qian 
Fixes: 8fdc7839e40f43a426bc7e858cf1dbfe315a3804
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
---
 hw/scsi/scsi-disk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index bdd1e5f..c080888 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2701,7 +2701,7 @@ static bool scsi_block_is_passthrough(SCSIDiskState *s, 
uint8_t *buf)
  * for the number of logical blocks specified in the length
  * field).  For other modes, do not use scatter/gather operation.
  */
-if ((buf[1] & 6) != 2) {
+if ((buf[1] & 6) == 2) {
 return false;
 }
 break;
-- 
2.9.3

[Qemu-devel] [PATCH v2 4/5] iotests: add more LUKS hash combination tests

2017-01-03 Thread Daniel P. Berrange

Add tests for sha224, sha512, sha384 and ripemd160 hash
algorithms.

Signed-off-by: Daniel P. Berrange 
---
 tests/qemu-iotests/149 |  10 +-
 tests/qemu-iotests/149.out | 482 -
 2 files changed, 484 insertions(+), 8 deletions(-)

diff --git a/tests/qemu-iotests/149 b/tests/qemu-iotests/149
index f62e618..5faf585 100755
--- a/tests/qemu-iotests/149
+++ b/tests/qemu-iotests/149
@@ -457,8 +457,12 @@ configs = [
 
 
 # LUKS default but diff hash
+LUKSConfig("aes-256-xts-plain64-sha224",
+   "aes", 256, "xts", "plain64", None, "sha224"),
 LUKSConfig("aes-256-xts-plain64-sha256",
"aes", 256, "xts", "plain64", None, "sha256"),
+LUKSConfig("aes-256-xts-plain64-sha384",
+   "aes", 256, "xts", "plain64", None, "sha384"),
 LUKSConfig("aes-256-xts-plain64-sha512",
"aes", 256, "xts", "plain64", None, "sha512"),
 LUKSConfig("aes-256-xts-plain64-ripemd160",
@@ -504,12 +508,6 @@ blacklist = [
 
 # GCrypt doesn't support Twofish with 192 bit key
 "twofish-192-xts-plain64-sha1",
-
-# We don't have sha512 hash wired up yet
-"aes-256-xts-plain64-sha512",
-
-# We don't have ripemd160 hash wired up yet
-"aes-256-xts-plain64-ripemd160",
 ]
 
 whitelist = []
diff --git a/tests/qemu-iotests/149.out b/tests/qemu-iotests/149.out
index b18c4e4..2f0454f 100644
--- a/tests/qemu-iotests/149.out
+++ b/tests/qemu-iotests/149.out
@@ -1562,6 +1562,126 @@ unlink TEST_DIR/luks-serpent-192-xts-plain64-sha1.img
 
 Skipping cast6-128-xts-plain64-sha1 in blacklist
 Skipping cast6-192-xts-plain64-sha1 in blacklist
+# = dm-crypt aes-256-xts-plain64-sha224 =
+# Create image
+truncate TEST_DIR/luks-aes-256-xts-plain64-sha224.img --size 4194304MB
+# Format image
+sudo cryptsetup -q -v luksFormat --cipher aes-xts-plain64 --key-size 512 
--hash sha224 --key-slot 0 --key-file - --iter-time 10 
TEST_DIR/luks-aes-256-xts-plain64-sha224.img
+# Open dev
+sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-xts-plain64-sha224.img 
qiotest-145-aes-256-xts-plain64-sha224
+# Set dev owner
+sudo chown UID:GID /dev/mapper/qiotest-145-aes-256-xts-plain64-sha224
+# Write test pattern 0xa7
+qemu-io -c write -P 0xa7 100M 10M --image-opts 
driver=file,filename=/dev/mapper/qiotest-145-aes-256-xts-plain64-sha224
+wrote 10485760/10485760 bytes at offset 104857600
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+# Write test pattern 0x13
+qemu-io -c write -P 0x13 3145728M 10M --image-opts 
driver=file,filename=/dev/mapper/qiotest-145-aes-256-xts-plain64-sha224
+wrote 10485760/10485760 bytes at offset 3298534883328
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+# Close dev
+sudo cryptsetup -q -v luksClose qiotest-145-aes-256-xts-plain64-sha224
+# Read test pattern 0xa7
+qemu-io -c read -P 0xa7 100M 10M --object 
secret,id=sec0,data=MTIzNDU2,format=base64 --image-opts 
driver=luks,key-secret=sec0,file.filename=TEST_DIR/luks-aes-256-xts-plain64-sha224.img
+read 10485760/10485760 bytes at offset 104857600
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+# Read test pattern 0x13
+qemu-io -c read -P 0x13 3145728M 10M --object 
secret,id=sec0,data=MTIzNDU2,format=base64 --image-opts 
driver=luks,key-secret=sec0,file.filename=TEST_DIR/luks-aes-256-xts-plain64-sha224.img
+read 10485760/10485760 bytes at offset 3298534883328
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+# Write test pattern 0x91
+qemu-io -c write -P 0x91 100M 10M --object 
secret,id=sec0,data=MTIzNDU2,format=base64 --image-opts 
driver=luks,key-secret=sec0,file.filename=TEST_DIR/luks-aes-256-xts-plain64-sha224.img
+wrote 10485760/10485760 bytes at offset 104857600
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+# Write test pattern 0x5e
+qemu-io -c write -P 0x5e 3145728M 10M --object 
secret,id=sec0,data=MTIzNDU2,format=base64 --image-opts 
driver=luks,key-secret=sec0,file.filename=TEST_DIR/luks-aes-256-xts-plain64-sha224.img
+wrote 10485760/10485760 bytes at offset 3298534883328
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+# Open dev
+sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-xts-plain64-sha224.img 
qiotest-145-aes-256-xts-plain64-sha224
+# Set dev owner
+sudo chown UID:GID /dev/mapper/qiotest-145-aes-256-xts-plain64-sha224
+# Read test pattern 0x91
+qemu-io -c read -P 0x91 100M 10M --image-opts 
driver=file,filename=/dev/mapper/qiotest-145-aes-256-xts-plain64-sha224
+read 10485760/10485760 bytes at offset 104857600
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+# Read test pattern 0x5e
+qemu-io -c read -P 0x5e 3145728M 10M --image-opts 
driver=file,filename=/dev/mapper/qiotest-145-aes-256-xts-plain64-sha224
+read 10485760/10485760 bytes at offset 3298534883328
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+# Close dev
+sudo cryptsetup -q -v luksClose qiotest-145-aes-256-xts-plain64-sha224
+# Delete

Re: [Qemu-devel] [PATCH v2] [i.MX] fix CS handling during SPI access.

2017-01-03 Thread mar.krzeminski


Hello JC,

W dniu 02.01.2017 o 22:11, Jean-Christophe Dubois pisze:

The i.MX SPI device was not de-asserting the CS line at the end of
memory access.

This triggered a SIGSEGV in Qemu when the sabrelite emulator was acessing
a SPI flash memory.

Whit this path the CS signal is correctly asserted and deasserted arround
memory access.

This code assume, that iMX SPI does not support devices which
are active when CS signal is asserted. I have not read datasheet,
but if iMX SPI can support such devices I think it could be better to
implement such functionality in model - even if currently in Qemu you 
will not use it.
If you will stick in deasserted CS line as active, please see comments 
below.


This was tested by:
* booting linux on Sabrelite Qemu emulator.
* booting xvisor on Sabrelite Qemu emultor.

Signed-off-by: Jean-Christophe Dubois 
---

Changes since v1:
* Fix coding style issue.

  hw/ssi/imx_spi.c | 33 ++---
  1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c
index e4e395f..c2d293c 100644
--- a/hw/ssi/imx_spi.c
+++ b/hw/ssi/imx_spi.c
@@ -152,13 +152,20 @@ static bool imx_spi_is_multiple_master_burst(IMXSPIState 
*s)
  
  static void imx_spi_flush_txfifo(IMXSPIState *s)

  {
-uint32_t tx;
-uint32_t rx;
+uint32_t i;
  
  DPRINTF("Begin: TX Fifo Size = %d, RX Fifo Size = %d\n",

  fifo32_num_used(>tx_fifo), fifo32_num_used(>rx_fifo));
  
+/* Activate the requested CS line */

+for (i = 0; i < 4; i++) {
+qemu_set_irq(s->cs_lines[i],
+ i == imx_spi_selected_channel(s) ? 0 : 1);
+}

Since you are setting to 1 all cs lines in reset, this loop could be
removed, and only selected cs line could be de-asserted.

+
  while (!fifo32_is_empty(>tx_fifo)) {
+uint32_t tx;
+uint32_t rx = 0;
  int tx_burst = 0;
  int index = 0;
  
@@ -178,8 +185,6 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
  
  tx_burst = MIN(s->burst_length, 32);
  
-rx = 0;

-
  while (tx_burst) {
  uint8_t byte = tx & 0xff;
  
@@ -221,6 +226,13 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)

  s->regs[ECSPI_STATREG] |= ECSPI_STATREG_TC;
  }
  
+/* Deselect all SS lines if transfert if completed */

+if (s->regs[ECSPI_STATREG] & ECSPI_STATREG_TC) {
+for (i = 0; i < 4; i++) {
+qemu_set_irq(s->cs_lines[i], 1);
+}
+}
+

Same as above.

Thanks,
Marcin

  /* TODO: We should also use TDR and RDR bits */
  
  DPRINTF("End: TX Fifo Size = %d, RX Fifo Size = %d\n",

@@ -230,6 +242,7 @@ static void imx_spi_flush_txfifo(IMXSPIState *s)
  static void imx_spi_reset(DeviceState *dev)
  {
  IMXSPIState *s = IMX_SPI(dev);
+uint32_t i;
  
  DPRINTF("\n");
  
@@ -243,6 +256,11 @@ static void imx_spi_reset(DeviceState *dev)

  imx_spi_update_irq(s);
  
  s->burst_length = 0;

+
+/* Disable all CS lines */
+for (i = 0; i < 4; i++) {
+qemu_set_irq(s->cs_lines[i], 1);
+}
  }
  
  static uint64_t imx_spi_read(void *opaque, hwaddr offset, unsigned size)

@@ -359,15 +377,8 @@ static void imx_spi_write(void *opaque, hwaddr offset, 
uint64_t value,
  }
  
  if (imx_spi_channel_is_master(s)) {

-int i;
-
  /* We are in master mode */
  
-for (i = 0; i < 4; i++) {

-qemu_set_irq(s->cs_lines[i],
- i == imx_spi_selected_channel(s) ? 0 : 1);
-}
-
  if ((value & change_mask & ECSPI_CONREG_SMC) &&
  !fifo32_is_empty(>tx_fifo)) {
  /* SMC bit is set and TX FIFO has some slots filled in */

Re: [Qemu-devel] [PATCH] scsi-disk: fix crash on VERIFY command

2017-01-03 Thread Paolo Bonzini



On 03/01/2017 10:58, Zhang Qian wrote:
> 
> At 2017-01-03 17:38:49, Paolo Bonzini  wrote:
>>
>>
>>On 03/01/2017 09:12, Zhang Qian wrote:
>>> yes, you are right.
>>> The scenarios of problem is
>>> a scsi-disk object receives VERIFY command with BYTCHK bit being zero,
>>> scsi_block_is_passthrough returns false and finally scsi-block uses
>>> scsi_disk_dma_command for
>>> VERIFY. So the mode is set to SCSI_XFER_NONE.
>>> In scsi_req_continue, scsi_read_data function is called.
>>
>>Uhm, is the fix simply
>>
>>diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
>>index bdd1e5f..c080888 100644
>>--- a/hw/scsi/scsi-disk.c
>>+++ b/hw/scsi/scsi-disk.c
>>@@ -2701,7 +2701,7 @@ static bool scsi_block_is_passthrough(SCSIDiskState *s, 
>>uint8_t *buf)
>>  * for the number of logical blocks specified in the length
>>  * field).  For other modes, do not use scatter/gather operation.
>>  */
>>-if ((buf[1] & 6) != 2) {
>>+if ((buf[1] & 6) == 2) {
>> return false;
>> }
>> break;
>>
>>then?
> I verified your patch, it is ok.
> 
> but why not use (buf[1] & 2) == 2 ?

Isn't BYTCHK bits 1 and 2?

Paolo

Re: [Qemu-devel] [PATCH] [M25P80] Make sure not to overrun the internal data buffer.

2017-01-03 Thread mar.krzeminski




W dniu 02.01.2017 o 22:24, Jean-Christophe DUBOIS pisze:

Le 30/12/2016 à 19:09, mar.krzeminski a écrit :

Can you check spi controller model code?


I'll double check.

But why is the SPI memory/device even responding if CS is not set ?

Looking at ssi code it should not.
Flash (so the m25p80) is responding when CS line is low and it seem 
that this is default.


So I fixed the CS handling in the i.MX SPI device emulator and I don't 
experience crashes anymore. Thanks for your help.


Cool!


You may still want to harden the m25p80 code to make sure it doesn't 
overrun its internal buffer.


Yeap, for me it is a bug.
I understand you will not finish this patch, won't you ?:)

Thanks,
Marcin




Thanks,
Marcin





Thanks,
Marcin

[Qemu-devel] [PATCH v2 2/5] iotests: fix remainining tests to work with LUKS

2017-01-03 Thread Daniel P. Berrange

The tests 033, 120, 140, 145 and 157 were all broken
when run with LUKS, since they did not correctly use
the required image opts args syntax to specify the
decryption secret.

Signed-off-by: Daniel P. Berrange 
---
 tests/qemu-iotests/033 | 16 
 tests/qemu-iotests/120 | 25 ++---
 tests/qemu-iotests/140 | 15 ++-
 tests/qemu-iotests/145 | 18 +-
 tests/qemu-iotests/157 | 17 ++---
 tests/qemu-iotests/157.out | 16 
 6 files changed, 87 insertions(+), 20 deletions(-)

diff --git a/tests/qemu-iotests/033 b/tests/qemu-iotests/033
index 16edcf2..b690b0e 100755
--- a/tests/qemu-iotests/033
+++ b/tests/qemu-iotests/033
@@ -50,14 +50,22 @@ do_test()
local align=$1
local iocmd=$2
local img=$3
+   if test "$IMGOPTSSYNTAX" = "true"
+   then
+   IO_OPEN_ARG="$img"
+   IO_EXTRA_ARGS="--image-opts"
+   else
+   IO_OPEN_ARG="-o driver=$IMGFMT,file.align=$align blkdebug::$img"
+   IO_EXTRA_ARGS=""
+   fi
{
-   echo "open -o driver=$IMGFMT,file.align=$align blkdebug::$img"
+   echo "open $IO_OPEN_ARG"
echo $iocmd
-   } | $QEMU_IO
+   } | $QEMU_IO $IO_EXTRA_ARGS
 }
 
 for write_zero_cmd in "write -z" "aio_write -z"; do
-for align in 512 4k; do
+for align in 512 4k; do
echo
echo "== preparing image =="
do_test $align "write -P 0xa 0x200 0x400" "$TEST_IMG" | _filter_qemu_io
@@ -91,7 +99,7 @@ for align in 512 4k; do
do_test $align "read -P 0xb 0x400 0xc00" "$TEST_IMG" | _filter_qemu_io
 
echo
-done
+done
 done
 
 # success, all done
diff --git a/tests/qemu-iotests/120 b/tests/qemu-iotests/120
index 4f88a67..5f80517 100755
--- a/tests/qemu-iotests/120
+++ b/tests/qemu-iotests/120
@@ -44,17 +44,36 @@ _supported_os Linux
 
 _make_test_img 64M
 
+if test "$IMGOPTSSYNTAX" = "true"
+then
+SYSEMU_DRIVE_ARG=id=drv,if=none,$TEST_IMG
+SYSEMU_EXTRA_ARGS=""
+IO_DRIVE_ARG="$TEST_IMG"
+IO_EXTRA_ARGS="--image-opts"
+if [ -n "$IMGKEYSECRET" ]; then
+   SECRET_ARG="secret,id=keysec0,data=$IMGKEYSECRET"
+   SYSEMU_EXTRA_ARGS="$SYSEMU_EXTRA_ARGS -object $SECRET_ARG"
+   IO_EXTRA_ARGS="$IO_EXTRA_ARGS --object $SECRET_ARG"
+fi
+else
+
SYSEMU_DRIVE_ARG=id=drv,if=none,file="$TEST_IMG",driver=raw,file.driver=$IMGFMT
+SYSEMU_EXTRA_ARGS=""
+IO_DRIVE_ARG="json:{'driver': 'raw', 'file': {'driver': '$IMGFMT', 'file': 
{'filename': '$TEST_IMG'}}}"
+IO_EXTRA_ARGS=""
+fi
+
+
 echo "{'execute': 'qmp_capabilities'}
   {'execute': 'human-monitor-command',
'arguments': {'command-line': 'qemu-io drv \"write -P 42 0 64k\"'}}
   {'execute': 'quit'}" \
-| $QEMU -qmp stdio -nographic -nodefaults \
--drive 
id=drv,if=none,file="$TEST_IMG",driver=raw,file.driver=$IMGFMT \
+| $QEMU -qmp stdio -nographic -nodefaults $SYSEMU_EXTRA_ARGS \
+-drive $SYSEMU_DRIVE_ARG \
 | _filter_qmp | _filter_qemu_io
 $QEMU_IO -c 'read -P 42 0 64k' "$TEST_IMG" | _filter_qemu_io
 
 $QEMU_IO_PROG -c 'read -P 42 0 64k' \
-"json:{'driver': 'raw', 'file': {'driver': '$IMGFMT', 'file': {'filename': 
'$TEST_IMG'}}}" \
+$IO_EXTRA_ARGS "$IO_DRIVE_ARG" \
 | _filter_qemu_io
 
 # success, all done
diff --git a/tests/qemu-iotests/140 b/tests/qemu-iotests/140
index 49f9df4..afaa418 100755
--- a/tests/qemu-iotests/140
+++ b/tests/qemu-iotests/140
@@ -51,8 +51,21 @@ _make_test_img 64k
 
 $QEMU_IO -c 'write -P 42 0 64k' "$TEST_IMG" | _filter_qemu_io
 
+if test "$IMGOPTSSYNTAX" = "true"
+then
+SYSEMU_DRIVE_ARG=if=none,media=cdrom,id=drv,$TEST_IMG
+SYSEMU_EXTRA_ARGS=""
+if [ -n "$IMGKEYSECRET" ]; then
+   SECRET_ARG="secret,id=keysec0,data=$IMGKEYSECRET"
+   SYSEMU_EXTRA_ARGS="-object $SECRET_ARG"
+fi
+else
+
SYSEMU_DRIVE_ARG=if=none,media=cdrom,id=drv,file="$TEST_IMG",driver=raw,file.driver=$IMGFMT
+SYSEMU_EXTRA_ARGS=""
+fi
+
 keep_stderr=y \
-_launch_qemu -drive if=none,media=cdrom,id=drv,file="$TEST_IMG",format=$IMGFMT 
\
+_launch_qemu $SYSEMU_EXTRA_ARGS -drive $SYSEMU_DRIVE_ARG \
 2> >(_filter_nbd)
 
 _send_qemu_cmd $QEMU_HANDLE \
diff --git a/tests/qemu-iotests/145 b/tests/qemu-iotests/145
index 1eca0e8..ab54972 100755
--- a/tests/qemu-iotests/145
+++ b/tests/qemu-iotests/145
@@ -43,7 +43,23 @@ _supported_proto generic
 _supported_os Linux
 
 _make_test_img 1M
-echo quit | $QEMU -nographic -hda "$TEST_IMG" -incoming 'exec:true' -snapshot 
-serial none -monitor stdio | _filter_qemu
+
+if test "$IMGOPTSSYNTAX" = "true"
+then
+SYSEMU_DRIVE_ARG=if=none,$TEST_IMG
+SYSEMU_EXTRA_ARGS=""
+if [ -n "$IMGKEYSECRET" ]; then
+   SECRET_ARG="secret,id=keysec0,data=$IMGKEYSECRET"
+   SYSEMU_EXTRA_ARGS="-object $SECRET_ARG"
+fi
+else
+SYSEMU_DRIVE_ARG=if=none,file="$TEST_IMG",driver=raw,file.driver=$IMGFMT
+SYSEMU_EXTRA_ARGS=""
+fi
+
+echo

Re: [Qemu-devel] [PATCH] qdev: Make "hotplugged" property read-only

2017-01-03 Thread Eduardo Habkost

(CCing libvir-list and Laine)

On Tue, Jan 03, 2017 at 05:53:27PM +0100, Igor Mammedov wrote:
> On Tue, 3 Jan 2017 17:10:15 +0100
> Paolo Bonzini  wrote:
> 
> > 
> > 
> > On 03/01/2017 15:22, Eduardo Habkost wrote:
> > >> I didn't know that. Is this documented somewhere?
> > >> Is it actually used by any existing software?
> > > not that I know of. But users should be fixed if they are not using it.
> > > 
> > > I see. The problem is that the mechanism is undocumented,
> > > untested, and seems very likely to trigger bugs in device code.
> > 
> > I agree.  Why can't hotplugged be migrated?
> It's probably not migrated because of it's not runtime/guest modified
> state so we don't have to migrate it as it's know in advance.
> 
> For now it should set manually on CLI (-device) with the rest of
> hotplugged device properties.

As this recommendation has the potential to trigger hidden bugs
(and known to trigger a bug in QEMU <= 2.8), I would like it to
be properly documented, and the documentation/recommendations
reviewed following the usual patch review process.

While we don't do that, setting "hotplugged=true" on the
command-line is an unused, undocumented, untested (and
unsupported?) feature.

-- 
Eduardo

[Qemu-devel] [PATCH v2 0/5] Improve I/O tests coverage of LUKS

2017-01-03 Thread Daniel P. Berrange

The main goal of this series is to get the I/O tests passing
100% with LUKS when run with './check -luks'. It also adds a
few more combinations to the LUKS/dmcrypt interoperability
test.

To make LUKS testing not quite as slow, we drop the PBKDF
iteration count down to a very small value. This doesn't
remove all overhead, as formatting the volume will always
measure PBKDF timing over a 1 second interval.

Changed in v2:

 - Split off patch that change check.time recording since
   it was not a direct dependancy
 - Skip new 159 & 170 tests which don't work due to qemu-img
   dd limitations

Daniel P. Berrange (5):
  iotests: skip 159 & 170 with luks format
  iotests: fix remainining tests to work with LUKS
  iotests: reduce PBKDF iterations when testing LUKS
  iotests: add more LUKS hash combination tests
  iotests: chown LUKS device before qemu-io launches

 tests/qemu-iotests/033   |   16 +-
 tests/qemu-iotests/120   |   25 +-
 tests/qemu-iotests/140   |   15 +-
 tests/qemu-iotests/145   |   18 +-
 tests/qemu-iotests/149   |   26 +-
 tests/qemu-iotests/149.out   | 1002 --
 tests/qemu-iotests/157   |   17 +-
 tests/qemu-iotests/157.out   |   16 +-
 tests/qemu-iotests/159   |2 +-
 tests/qemu-iotests/170   |2 +-
 tests/qemu-iotests/common.filter |3 +-
 tests/qemu-iotests/common.rc |3 +
 12 files changed, 847 insertions(+), 298 deletions(-)

-- 
2.9.3

Re: [Qemu-devel] [PATCH] qemu-img: add support for --object with 'dd' command

2017-01-03 Thread Eric Blake

On 01/03/2017 11:00 AM, Daniel P. Berrange wrote:
> The qemu-img dd command added --image-opts support, but missed
> the corresponding --object support. This prevented passing
> secrets (eg auth passwords) needed by certain disk images.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  qemu-img.c | 16 
>  1 file changed, 16 insertions(+)

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] Performance about x-data-plane

2017-01-03 Thread Weiwei Jia

Hi Stefan,

Thanks for your reply.

On Tue, Jan 3, 2017 at 10:50 AM, Stefan Hajnoczi  wrote:
> On Thu, Dec 22, 2016 at 01:34:47AM -0500, Weiwei Jia wrote:
>> With QEMU x-data-plane, I find the performance has not been improved
>> very much. Please see following two settings.
>
> Using IOThreads improves scalability for SMP guests with many disks.  It
> does not improve performance for a single disk benchmark because there
> is nothing to spread across IOThreads.
>
>> Setting 1: I/O thread in host OS (VMM) reads 4KB each time from disk
>> (8GB in total). Pin the I/O thread to pCPU 5 which will serve I/O
>> thread dedicatedly. I find the performance is around 250 MB/s.
>
> 250 MB/s / 4 KB = 64k IOPS
>
> This seems like a reasonable result for single thread with a single
> disk.  I guess the benchmark queue depth setting is larger than 1 though
> because it would only allow 15 microseconds per request.

For this test, we have two disks as a RAID 0. Each of the disk is 2.5
inch and has around 130 MB/s bandwidth (sequential read) as
specification said.

>
>> Setting 2: I/O thread in guest OS (VMM) reads 4KB each time from
>> virtual disk (8GB in total). Pin the I/O thread to vCPU 5 and pin vCPU
>> 5 thread to pCPU5 so that vCPU 5 handles this I/O thread dedicatedly
>> and pCPU5 serve vCPU5 dedicatedly. In order to keep vCPU5 not to be
>> idle, I also pin one cpu intensive thread (while (1) {i++}) on vCPU 5
>> so that the I/O thread on it can be served without delay. For this
>> setting, I find the performance for this I/O thread is around 190
>> MB/s.
>
> 190 MB/s / 4 KB = 48k IOPS
>
> I worry that your while (1) {i++} thread may prevent achieving the best
> performance if the guest kernel scheduler allows it to use its time
> slice.
>
> Two options that might work better are:
> 1. idle=poll guest kernel command-line parameter
> 2. kvm.ko's halt_poll_ns host kernel module parameter

Yes, I add "idle=poll" currently but I did not try "kvm.ko's
halt_poll_ns" yet. I will try it. Actually, current days, I have made
a RAID 0 with four disks and each of them is 2.5 inch and has 130 MB/s
bandwidth (from specification) for sequential read. I also fix some
interrupts delay problem. See following for the latest experiment and
problems.

I run one I/O thread (sequential read 4 KB each time and read 8 GB in
total) in host OS, the throughput is around 420 MB/s. However, when I
run this I/O thread in one VM (no other VM is created and data-plane
is enabled) with dedicated hardware, the throughput will be around 350
MB/s. The VM's experiment setting is as follows.

In the VM, there are 15 vCPUs (vCPU0 - vCPU14); each vCPU is pinned to
corresponding dedicated pCPU (for example, vCPU 0 is pinned to pCPU0
... vCPU 14 is pinned to pCPU 14); "Idle=poll" is added in the VM's
boot grub so that the vCPU will not be idle; in the VM, all the
interrupts are pinned to vCPU0 to guarantee these interrupts can be
responded on time; the I/O thread is executed on one of the vCPU
except vCPU0.

In the host OS, there are 16 pCPUs (pCPU0 - pCPU15); pCPU 0 - pCPU 14
are used for vCPU0 - vCPU 14 in the VM dedicatedly; pCPU 15 is used by
QEMU IOthread (data-plane) to handle I/O read requests from VM
dedicatedly.

Kernel version: 3.16.39
QEMU version: 2.4.1

I don't know why there is 70 MB/s difference between host OS and guest
OS as above experiment. Does anyone have same experiences? Any
comments? Thank you in advance.

BTW, I have checked several times about my hardware configuration and
I think the throughput difference as above should be related to QEMU.
Maybe, I miss any configuration about QEMU.

>
>> NOTE: For setting 2, I also pin the QEMU dedicated IOthread
>> (x-data-plane) in host OS to pCPU to handle I/O requests from guest OS
>> dedicatedly.
>
> Which pCPU did you pin the dataplane thread to?  Did you try changing
> this?

No, it is pinned to pCPU 15 dedicatedly. Please see above latest
experiment and problem.

>
>> I think for setting 2, the performance of I/O thread should be almost
>> the same as setting 1. I cannot understand why it is 60 MB/s lower
>> than setting 1. I am wondering whether there are something wrong with
>> my x-data-plane setting or virtio setting for VM.  Would you please
>> give me some hints? Thank you.
>
> Ideally QEMU should achieve the same performance as bare metal.  In
> practice the overhead increases as IOPS increases.  You may be able to
> achieve 260 MB/s inside the guest with a larger request size since it
> involves fewer I/O requests.

Yes, I agree with you that it should achieve the performance as bare
metal. I will try a larger request size. Thank you.

>
> The expensive part is the virtqueue kick.  Recently we tried polling the
> virtqueue instead of waiting for the ioeventfd file descriptor and got
> double-digit performance improvements:
> https://lists.gnu.org/archive/html/qemu-devel/2016-12/msg00148.html
>
> If you want to understand the performance of your benchmark

[Qemu-devel] [PATCH v2 5/5] iotests: chown LUKS device before qemu-io launches

2017-01-03 Thread Daniel P. Berrange

On some distros, whenever you close a block device file
descriptor there is a udev rule that resets the file
permissions. This can race with the test script when
we run qemu-io multiple times against the same block
device. Occassionally the second qemu-io invokation
will find udev has reset the permissions causing failure.

Signed-off-by: Daniel P. Berrange 
---
 tests/qemu-iotests/149 |  13 +-
 tests/qemu-iotests/149.out | 344 ++---
 2 files changed, 178 insertions(+), 179 deletions(-)

diff --git a/tests/qemu-iotests/149 b/tests/qemu-iotests/149
index 5faf585..bc628ce 100755
--- a/tests/qemu-iotests/149
+++ b/tests/qemu-iotests/149
@@ -23,6 +23,7 @@
 import subprocess
 import os
 import os.path
+import time
 
 import base64
 
@@ -186,7 +187,7 @@ def chown(config):
 msg = proc.communicate()[0]
 
 if proc.returncode != 0:
-raise Exception("Cannot change owner on %s" % path)
+raise Exception(msg)
 
 
 def cryptsetup_open(config):
@@ -271,6 +272,8 @@ def qemu_io_image_args(config, dev=False):
 def qemu_io_write_pattern(config, pattern, offset_mb, size_mb, dev=False):
 """Write a pattern of data to a LUKS image or device"""
 
+if dev:
+chown(config)
 args = ["-c", "write -P 0x%x %dM %dM" % (pattern, offset_mb, size_mb)]
 args.extend(qemu_io_image_args(config, dev))
 iotests.log("qemu-io " + " ".join(args), filters=[iotests.filter_test_dir])
@@ -281,6 +284,8 @@ def qemu_io_write_pattern(config, pattern, offset_mb, 
size_mb, dev=False):
 def qemu_io_read_pattern(config, pattern, offset_mb, size_mb, dev=False):
 """Read a pattern of data to a LUKS image or device"""
 
+if dev:
+chown(config)
 args = ["-c", "read -P 0x%x %dM %dM" % (pattern, offset_mb, size_mb)]
 args.extend(qemu_io_image_args(config, dev))
 iotests.log("qemu-io " + " ".join(args), filters=[iotests.filter_test_dir])
@@ -331,9 +336,6 @@ def test_once(config, qemu_img=False):
 cryptsetup_open(config)
 
 try:
-iotests.log("# Set dev owner")
-chown(config)
-
 iotests.log("# Write test pattern 0xa7")
 qemu_io_write_pattern(config, 0xa7, lowOffsetMB, 10, dev=True)
 iotests.log("# Write test pattern 0x13")
@@ -365,9 +367,6 @@ def test_once(config, qemu_img=False):
 cryptsetup_open(config)
 
 try:
-iotests.log("# Set dev owner")
-chown(config)
-
 iotests.log("# Read test pattern 0x91")
 qemu_io_read_pattern(config, 0x91, lowOffsetMB, 10, dev=True)
 iotests.log("# Read test pattern 0x5e")
diff --git a/tests/qemu-iotests/149.out b/tests/qemu-iotests/149.out
index 2f0454f..5dea00b 100644
--- a/tests/qemu-iotests/149.out
+++ b/tests/qemu-iotests/149.out
@@ -5,14 +5,14 @@ truncate TEST_DIR/luks-aes-256-xts-plain64-sha1.img --size 
4194304MB
 sudo cryptsetup -q -v luksFormat --cipher aes-xts-plain64 --key-size 512 
--hash sha1 --key-slot 0 --key-file - --iter-time 10 
TEST_DIR/luks-aes-256-xts-plain64-sha1.img
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-xts-plain64-sha1.img 
qiotest-145-aes-256-xts-plain64-sha1
-# Set dev owner
-sudo chown UID:GID /dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 # Write test pattern 0xa7
+sudo chown UID:GID /dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 qemu-io -c write -P 0xa7 100M 10M --image-opts 
driver=file,filename=/dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 wrote 10485760/10485760 bytes at offset 104857600
 10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 # Write test pattern 0x13
+sudo chown UID:GID /dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 qemu-io -c write -P 0x13 3145728M 10M --image-opts 
driver=file,filename=/dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 wrote 10485760/10485760 bytes at offset 3298534883328
 10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -41,14 +41,14 @@ wrote 10485760/10485760 bytes at offset 3298534883328
 
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-xts-plain64-sha1.img 
qiotest-145-aes-256-xts-plain64-sha1
-# Set dev owner
-sudo chown UID:GID /dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 # Read test pattern 0x91
+sudo chown UID:GID /dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 qemu-io -c read -P 0x91 100M 10M --image-opts 
driver=file,filename=/dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 read 10485760/10485760 bytes at offset 104857600
 10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 # Read test pattern 0x5e
+sudo chown UID:GID /dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 qemu-io -c read -P 0x5e 3145728M 10M --image-opts 
driver=file,filename=/dev/mapper/qiotest-145-aes-256-xts-plain64-sha1
 read 10485760/10485760 bytes at offset 3298534883328
 10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -65,14 +65,14 @@ Formatting 'TEST_DIR/luks-aes-256-xts-plain64-sha1.img', 
fmt=luks

[Qemu-devel] [PATCH v2 1/5] iotests: skip 159 & 170 with luks format

2017-01-03 Thread Daniel P. Berrange

While the qemu-img dd command does accept --image-opts
this is not sufficient to make it work with the LUKS
image yet. This is because bdrv_create() still always
requires the non-image-opts syntax.

Thus we must skip 159/170 with luks for now

Signed-off-by: Daniel P. Berrange 
---
 tests/qemu-iotests/159 | 2 +-
 tests/qemu-iotests/170 | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/159 b/tests/qemu-iotests/159
index 825f05f..7636865 100755
--- a/tests/qemu-iotests/159
+++ b/tests/qemu-iotests/159
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.filter
 . ./common.pattern
 
-_supported_fmt generic
+_supported_fmt raw qcow qcow2 qed
 _supported_proto file
 _supported_os Linux
 
diff --git a/tests/qemu-iotests/170 b/tests/qemu-iotests/170
index 5b335db..6bd1c37 100755
--- a/tests/qemu-iotests/170
+++ b/tests/qemu-iotests/170
@@ -37,7 +37,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.filter
 . ./common.pattern
 
-_supported_fmt generic
+_supported_fmt raw qcow qcow2 qed
 _supported_proto file
 _supported_os Linux
 
-- 
2.9.3

[Qemu-devel] [PATCH] qemu-img: add support for --object with 'dd' command

2017-01-03 Thread Daniel P. Berrange

The qemu-img dd command added --image-opts support, but missed
the corresponding --object support. This prevented passing
secrets (eg auth passwords) needed by certain disk images.

Signed-off-by: Daniel P. Berrange 
---
 qemu-img.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c
index 6949b73..138e37b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3948,6 +3948,7 @@ static int img_dd(int argc, char **argv)
 };
 const struct option long_options[] = {
 { "help", no_argument, 0, 'h'},
+{ "object", required_argument, 0, OPTION_OBJECT},
 { "image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
 { 0, 0, 0, 0 }
 };
@@ -3970,6 +3971,14 @@ static int img_dd(int argc, char **argv)
 case 'h':
 help();
 break;
+case OPTION_OBJECT: {
+QemuOpts *opts;
+opts = qemu_opts_parse_noisily(_object_opts,
+   optarg, true);
+if (!opts) {
+return 1;
+}
+}   break;
 case OPTION_IMAGE_OPTS:
 image_opts = true;
 break;
@@ -4014,6 +4023,13 @@ static int img_dd(int argc, char **argv)
 ret = -1;
 goto out;
 }
+
+if (qemu_opts_foreach(_object_opts,
+  user_creatable_add_opts_foreach,
+  NULL, NULL)) {
+return 1;
+}
+
 blk1 = img_open(image_opts, in.filename, fmt, 0, false, false);
 
 if (!blk1) {
-- 
2.9.3

Re: [Qemu-devel] [PATCH] qdev: Make "hotplugged" property read-only

2017-01-03 Thread Igor Mammedov

On Tue, 3 Jan 2017 17:10:15 +0100
Paolo Bonzini  wrote:

> 
> 
> On 03/01/2017 15:22, Eduardo Habkost wrote:
> >> I didn't know that. Is this documented somewhere?
> >> Is it actually used by any existing software?
> > not that I know of. But users should be fixed if they are not using it.
> > 
> > I see. The problem is that the mechanism is undocumented,
> > untested, and seems very likely to trigger bugs in device code.
> 
> I agree.  Why can't hotplugged be migrated?
It's probably not migrated because of it's not runtime/guest modified
state so we don't have to migrate it as it's know in advance.

For now it should set manually on CLI (-device) with the rest of
hotplugged device properties.

> Paolo

Re: [Qemu-devel] [PATCH 1/3] cleanup: remove not used header

2017-01-03 Thread Igor Mammedov

On Tue, 3 Jan 2017 16:26:16 +0100
Andreas Färber  wrote:

> Am 03.01.2017 um 16:25 schrieb Eric Blake:
> > On 01/02/2017 09:44 AM, Igor Mammedov wrote:
> > 
> > perhaps s/not used/unused/ in the subject
> 
> ... and qom: instead of cleanup: if this gets re-spun.
sure

> 
> Andreas
> 
> > 
> > Reviewed-by: Eric Blake 
> > 
> >> Signed-off-by: Igor Mammedov 
> >> ---
> >>  qom/object_interfaces.c | 1 -
> >>  1 file changed, 1 deletion(-)
> >>
> >> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> >> index ded4d84..4b880d0 100644
> >> --- a/qom/object_interfaces.c
> >> +++ b/qom/object_interfaces.c
> >> @@ -3,7 +3,6 @@
> >>  #include "qom/object_interfaces.h"
> >>  #include "qemu/module.h"
> >>  #include "qapi-visit.h"
> >> -#include "qapi/qobject-output-visitor.h"
> >>  #include "qapi/opts-visitor.h"
> >>  
> >>  void user_creatable_complete(Object *obj, Error **errp)
> >>
> > 
> 
>

Re: [Qemu-devel] [RFC]virtio-blk: add disk-name device property

2017-01-03 Thread Eric Blake

On 12/29/2016 08:41 PM, Junkang Fu wrote:
>>From 74e913fc41ea98d1dde692175f1e3fb6729342aa Mon Sep 17 00:00:00 2001
> From: "junkang.fjk" 
> Date: Wed, 24 Aug 2016 19:36:53 +0800
> Subject: [PATCH] virtio-blk: add disk-name device property
> 
> Current virtio-blk disk name(ex. /dev/vdb) has nothing to do with the
> target dev
> name specified in libvirt xml file. For example, we may get disk name
> /dev/vdb in
> VM while target dev specified in libvirt xml is vdc.

It's not really libvirt's fault.  The libvirt XML names are for
convenience, but nothing on the host side requires the guest to pick the
same naming scheme as the host.

I guess your proposal is to enhance the virtio spec such that clients
that are new enough to honor the new addition to the virtio spec will
change their name-picking algorithm to use the name provided by the
host, rather than their current approach of picking whatever name they
feel like, and then enhance libvirt to pass the XML name on down to the
guest?  It might work, but as others have pointed out, it will require a
virtio spec change first.

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

signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] (no subject)

2017-01-03 Thread Stefan Hajnoczi

On Mon, Jan 02, 2017 at 09:03:55PM +0900, morgenlette madeBy wrote:
> I got problem using QEMU.
> 
> when i turn on virtual machine,
> 
> this message was shown,
> 
> 
> virsh: error while loading shared libraries: libapparmor.so.1: cannot open
> shared object file: No such file or directory
> 
> I have no idea about libapparmor.
> 
> 
> What should I do?

Hi,
Looks like the libvirt packages do not have all dependencies installed.

If you are using Ubuntu try "apt-get install libapparmor1" to install
the missing library.  I looked up the package that provides the
"libapparmor.so.1" filename here:
http://packages.ubuntu.com/search?searchon=contents=libapparmor.so.1=exactfilename=xenial=any

If you have further questions please try asking for help from your Linux
distribution (e.g. #ubuntu on irc.freenode.net) since this question is
not directly related to QEMU.

Good luck,
Stefan

signature.asc
Description: PGP signature

Re: [Qemu-devel] Speed menu for GTK interfaace

2017-01-03 Thread Programmingkid

On Jan 3, 2017, at 3:59 AM, Gerd Hoffmann wrote:

> On So, 2017-01-01 at 13:15 -0500, Programmingkid wrote:
>> Hello, I was wondering if you would accept a patch that added a Speed
>> menu to the GTK interface. This would allow the user to change how
>> much CPU time the emulated CPU would see. 
> 
> /me wonders what exactly would the menu entries do?
> 
> cheers,
>  Gerd

It is quite simple, there would be a 100% to a 1% menu item. It would look like
this:

Speed
---
100%
90%
80%
70%
60%
50%
40%
30%
20%
10%
1%

Each menu item would call cpu_throttle_set(). The value sent to this function 
would
be determined like this:
speed = -1 * menu_number + 100;

speed would be sent to the cpu_throttle_set() function. This function would 
reduce
the CPU usage of QEMU on the host. 

Why would someone want to slow down QEMU?
- The user is using a laptop and don't want it to heat up.
- The user wants to slow down a video game that is a little too challenging.
- The user wants to save energy.
- The user wants to conduct some kind of stress test on a program and see how
it handles under low cpu resources. 

Hope this helps. Thank you for responding.

Re: [Qemu-devel] [PATCH] ui/gtk: fix crash at startup when no console is available

2017-01-03 Thread Stefan Hajnoczi

On Sun, Jan 01, 2017 at 10:39:45AM +0100, Hervé Poussineau wrote:
> This patch fixes a segfault at QEMU startup, introduced in 
> a08156321ab9a7d2fed9ee77dbfeea2a61ffd153.
> gd_vc_find_current() return NULL, which is dereferenced without checking it.
> 
> While at it, disable the whole 'View' menu if no console exists.
> 
> Reproducer: qemu-system-i386 -M none -nodefaults
> 
> Signed-off-by: Hervé Poussineau 
> ---
>  ui/gtk.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Oops, sorry!

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [RFC]virtio-blk: add disk-name device property

2017-01-03 Thread Stefan Hajnoczi

On Fri, Dec 30, 2016 at 10:41:35AM +0800, Junkang Fu wrote:
> From 74e913fc41ea98d1dde692175f1e3fb6729342aa Mon Sep 17 00:00:00 2001
> From: "junkang.fjk" 
> Date: Wed, 24 Aug 2016 19:36:53 +0800
> Subject: [PATCH] virtio-blk: add disk-name device property
> 
> Current virtio-blk disk name(ex. /dev/vdb) has nothing to do with the
> target dev
> name specified in libvirt xml file. For example, we may get disk name
> /dev/vdb in
> VM while target dev specified in libvirt xml is vdc. This may lead to a
> little trouble
> to find out the relationship between the disk name in VM and somewhere out
> of
> VM, for example in the control board of Public cloud service providers. I
> suggest
> if Qemu could add a VIRTIO_BLK_F_DISK_NAME feature, with
> VIRTIO_BLK_F_DISK_NAME
> capable Qemu and virtio-blk frontend drivers, disk name in the vm can be
> specified
> as follows:
> -device virtio-blk-pci,disk-name=vdabc

Did you try -device virtio-blk-pci,serial=vdabc and
/dev/disk/by-id/virtio-vdabc inside a Linux guest?

> ---
>  hw/block/virtio-blk.c   | 5 +
>  include/hw/virtio/virtio-blk.h  | 1 +
>  include/standard-headers/linux/virtio_blk.h | 6 ++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 331d766..4039fb9 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -716,6 +716,8 @@ static void virtio_blk_update_config(VirtIODevice
> *vdev, uint8_t *config)
>  blkcfg.alignment_offset = 0;
>  blkcfg.wce = blk_enable_write_cache(s->blk);
>  virtio_stw_p(vdev, _queues, s->conf.num_queues);
> +if (s->disk_name)
> +strncpy((char *)blkcfg.disk_name, s->disk_name, DISK_NAME_LEN);
>  memcpy(config, , sizeof(struct virtio_blk_config));
>  }
> @@ -740,6 +742,8 @@ static uint64_t virtio_blk_get_features(VirtIODevice
> *vdev, uint64_t features,
>  virtio_add_feature(, VIRTIO_BLK_F_GEOMETRY);
>  virtio_add_feature(, VIRTIO_BLK_F_TOPOLOGY);
>  virtio_add_feature(, VIRTIO_BLK_F_BLK_SIZE);
> +virtio_add_feature(, VIRTIO_BLK_F_DISK_NAME);
> +
>  if (virtio_has_feature(features, VIRTIO_F_VERSION_1)) {
>  if (s->conf.scsi) {
>  error_setg(errp, "Please set scsi=off for virtio-blk devices
> in order to use virtio 1.0");
> @@ -970,6 +974,7 @@ static Property virtio_blk_properties[] = {
>  DEFINE_PROP_BIT("request-merging", VirtIOBlock, conf.request_merging,
> 0,
>  true),
>  DEFINE_PROP_UINT16("num-queues", VirtIOBlock, conf.num_queues, 1),
> +DEFINE_PROP_STRING("disk-name", VirtIOBlock, disk_name),
>  DEFINE_PROP_END_OF_LIST(),
>  };
> 
> diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
> index 180bd8d..003e810 100644
> --- a/include/hw/virtio/virtio-blk.h
> +++ b/include/hw/virtio/virtio-blk.h
> @@ -56,6 +56,7 @@ typedef struct VirtIOBlock {
>  bool dataplane_disabled;
>  bool dataplane_started;
>  struct VirtIOBlockDataPlane *dataplane;
> +char *disk_name;
>  } VirtIOBlock;
> 
>  typedef struct VirtIOBlockReq {
> diff --git a/include/standard-headers/linux/virtio_blk.h
> b/include/standard-headers/linux/virtio_blk.h
> index ab16ec5..1f5d89d 100644
> --- a/include/standard-headers/linux/virtio_blk.h
> +++ b/include/standard-headers/linux/virtio_blk.h
> @@ -38,6 +38,7 @@
>  #define VIRTIO_BLK_F_BLK_SIZE  6   /* Block size of disk is available*/
>  #define VIRTIO_BLK_F_TOPOLOGY  10  /* Topology information is available */
>  #define VIRTIO_BLK_F_MQ12  /* support more than one vq */
> +#define VIRTIO_BLK_F_DISK_NAME  13  /* specify /dev/xxx name */

These bits are defined in the VIRTIO specification.  In addition to
modifying QEMU and guest drivers you would also need to send a patch to
the VIRTIO Technical Committee to update the specification:
https://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-2050003

> 
>  /* Legacy feature bits */
>  #ifndef VIRTIO_BLK_NO_LEGACY
> @@ -51,6 +52,9 @@
> 
>  #define VIRTIO_BLK_ID_BYTES20  /* ID string length */
> 
> +/* micro defined in kernel genhd.h */
> +#define DISK_NAME_LEN 32
> +
>  struct virtio_blk_config {
> /* The capacity (in 512-byte sectors). */
> uint64_t capacity;
> @@ -84,6 +88,8 @@ struct virtio_blk_config {
> 
> /* number of vqs, only available when VIRTIO_BLK_F_MQ is set */
> uint16_t num_queues;
> +
> +   uint8_t disk_name[DISK_NAME_LEN];
>  } QEMU_PACKED;
> 
>  /*
> --
> 1.9.4




signature.asc
Description: PGP signature

Re: [Qemu-devel] vhost-user breaks after 96a3d98.

2017-01-03 Thread Michael S. Tsirkin

On Tue, Jan 03, 2017 at 06:28:18PM +0800, Jason Wang wrote:
> 
> 
> On 2017年01月03日 11:09, Jason Wang wrote:
> > 
> > 
> > On 2016年12月30日 20:41, Flavio Leitner wrote:
> > > Hi,
> > > 
> > > While I was testing vhost-user using OVS 2.5 and DPDK 2.2.0 in the
> > > host and testpmd dpdk 2.2.0 in the guest, I found that the commit
> > > below breaks the environment and no packets gets into the guest.
> > > 
> > > dpdk port --> OVS --> vhost-user --> guest --> testpmd
> > >   ^--- drops here ^--- no packets here.
> > > 
> > > commit 96a3d98d2cdbd897ff5ab33427aa4cfb94077665
> > > Author: Jason Wang 
> > > Date:   Mon Aug 1 16:07:58 2016 +0800
> > > 
> > >  vhost: don't set vring call if no vector
> > >   We used to set vring call fd unconditionally even if guest
> > > driver does
> > >  not use MSIX for this vritqueue at all. This will cause lots of
> > >  unnecessary userspace access and other checks for drivers does
> > > not use
> > >  interrupt at all (e.g virtio-net pmd). So check and clean vring
> > > call
> > >  fd if guest does not use any vector for this virtqueue at
> > >  all.
> > > [...]
> > > 
> > > Thanks,
> > 
> > Hi Flavio:
> > 
> > Thanks for reporting this issue, could this be a bug of vhost-user? (I
> > believe virito-net pmd does not use interrupt for rx/tx at all)
> > 
> > Anyway, will try to reproduce it.
> > 
> 
> Could not reproduce this issue on similar setups (the only difference is I
> don't create dpdk port) with dpdk 16.11 and ovs.git HEAD. Suspect an issue
> dpdk. Will try OVS 2.5 + DPDK 2.2.0.
> 
> Thanks

Possibly dpdk assumed that call fd must be present unconditionally.
Limit this patch to when protocol is updated? add a new protocol flag?

-- 
MST

[Qemu-devel] New Year's starting over ... bsd-user

2017-01-03 Thread Sean Bruno


I'm pondering where to start with getting FreeBSD's bsd-user code into
shape so it could actually be reviewed and accepted now that its sort of
working again (signal handling fixed finally).

I almost feel like the existing code should be purged, except that it
gives a good history (and this seems lazy to me).

As a first pass, I guess, I'd like to at least make i386 user run on
x86_64.  What would you folks like to see in a first pass?

sean

ref: https://github.com/seanbruno/qemu-bsd-user/tree/bsd-user



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] qdev: Make "hotplugged" property read-only

2017-01-03 Thread Paolo Bonzini



On 03/01/2017 15:22, Eduardo Habkost wrote:
>> I didn't know that. Is this documented somewhere?
>> Is it actually used by any existing software?
> not that I know of. But users should be fixed if they are not using it.
> 
> I see. The problem is that the mechanism is undocumented,
> untested, and seems very likely to trigger bugs in device code.

I agree.  Why can't hotplugged be migrated?

Paolo

Re: [Qemu-devel] [Resend PATCH] The QEMU crashes since invoking qemu_thread_set_name(), the backtrace is:

2017-01-03 Thread Stefan Hajnoczi

Please follow the conventions for commit messages.  QEMU commit messages
start with a prefix that identifies the affected component.

(Use git-log(1) to check what prefix other people have used for the
file(s) you touched.)

The commit message should be brief and doesn't need to be a full
sentence.

For example:

  qemu-thread: fix qemu_thread_set_name() race in qemu_thread_create()

Thanks,
Stefan


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH v2] iotests: record separate timings per format, protocol pair

2017-01-03 Thread Daniel P. Berrange

The 'check' program records timings for each test that
is run. These timings are only valid, however, for a
particular format/protocol combination. So if frequently
running 'check' with a variety of different formats or
protocols, the times printed can be very misleading.

Instead of having a single 'check.time' file, maintain
multiple 'check.time-$IMGPROTO-$IMGFMT' files.

Signed-off-by: Daniel P. Berrange 
---
Changed in v2:

 - Use a separate file per format/protocol, instead of
   throwing away data each time format/protocol changes
   between runs (Eric)

 tests/qemu-iotests/.gitignore |  2 +-
 tests/qemu-iotests/Makefile   |  2 +-
 tests/qemu-iotests/check  | 12 +++-
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/.gitignore b/tests/qemu-iotests/.gitignore
index 0711cbd..da62054 100644
--- a/tests/qemu-iotests/.gitignore
+++ b/tests/qemu-iotests/.gitignore
@@ -1,5 +1,5 @@
 check.log
-check.time
+check.time*
 common.env
 *.out.bad
 *.notrun
diff --git a/tests/qemu-iotests/Makefile b/tests/qemu-iotests/Makefile
index 2fb527c..27380e6 100644
--- a/tests/qemu-iotests/Makefile
+++ b/tests/qemu-iotests/Makefile
@@ -1,5 +1,5 @@
 
-CLEANFILES= *.out.bad *.notrun check.log check.time
+CLEANFILES= *.out.bad *.notrun check.log check.time*
 
 # no default target
 default:
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 4cba215..4b1c674 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -129,6 +129,8 @@ fi
 #exit 1
 #fi
 
+TIMESTAMP_FILE=check.time-$IMGPROTO-$IMGFMT
+
 tmp="${TEST_DIR}"/$$
 
 _wallclock()
@@ -155,9 +157,9 @@ _wrapup()
 :
 elif $needwrap
 then
-if [ -f check.time -a -f $tmp.time ]
+if [ -f $TIMESTAMP_FILE -a -f $tmp.time ]
 then
-cat check.time $tmp.time \
+cat $TIMESTAMP_FILE $tmp.time \
 | $AWK_PROG '
 { t[$1] = $2 }
 END{ if (NR > 0) {
@@ -165,7 +167,7 @@ END{ if (NR > 0) {
   }
 }' \
 | sort -n >$tmp.out
-mv $tmp.out check.time
+mv $tmp.out $TIMESTAMP_FILE
 fi
 
 if [ -f $tmp.expunged ]
@@ -223,7 +225,7 @@ echo "preamble" > "${TEST_DIR}"/check.sts
 # don't leave old full output behind on a clean run
 rm -f check.full
 
-[ -f check.time ] || touch check.time
+[ -f $TIMESTAMP_FILE ] || touch $TIMESTAMP_FILE
 
 FULL_IMGFMT_DETAILS=`_full_imgfmt_details`
 FULL_IMGPROTO_DETAILS=`_full_imgproto_details`
@@ -277,7 +279,7 @@ do
 # really going to try and run this one
 #
 rm -f $seq.out.bad
-lasttime=`sed -n -e "/^$seq /s/.* //p"

Re: [Qemu-devel] Proposal for 2.9 release schedule

2017-01-03 Thread Paolo Bonzini



On 03/01/2017 16:53, Stefan Hajnoczi wrote:
> On Fri, Dec 23, 2016 at 03:15:58PM +0100, Paolo Bonzini wrote:
>> Considering that Easter is on April 16th, we'd probably want to have the
>> release before that date even in case of a slip.
>>
>> On the other hand, the Christmas / New Year break here means that we'll
>> have to make the development time 1-2 week shorter in practice.
>>
>> 2016-02-21 2.9 soft freeze
>> 2016-03-07 hard freeze / rc0
>> 2016-03-28 rc3 (+3 weeks)
>> 2016-04-04 rc4 or release
>> 2016-04-11 release (if rc4)
>>
>> One possibility is to make soft freeze happen a few days later.
>> Peter/Stefan, how did the experiment go with the new rules for soft
>> freeze?  Is it worth repeating it for 2.9 and would it make sense to
>> shorten soft freeze given the new rules?
> 
> I would shorten the soft freeze by 1 week.
> 
> Overall the 2.8 release went smoothly.  We got unlucky right at the end
> with a release blocker but otherwise it was fine.

Then what about soft freeze on 2016-02-28?

Thanks,

Paolo

Re: [Qemu-devel] Qemu NVME ubuntu 16.04 setup problems

2017-01-03 Thread Stefan Hajnoczi

On Mon, Dec 26, 2016 at 01:59:13PM +0530, Gadre Nayan wrote:
> I am trying to test SPDK on a Virtual Machine with NVME support. I do not
> have NVME cards on the Host hence wanted to simulate the Hardware, I have
> install nvmeqemu (https://github.com/nvmeqemu/nvmeqemu) for that purpose.

That repo hasn't been updated since 2012.  NVMe support has been
available in upstream QEMU since 2013.

Please try using a modern QEMU.


signature.asc
Description: PGP signature

Re: [Qemu-devel] Proposal for 2.9 release schedule

2017-01-03 Thread Stefan Hajnoczi

On Fri, Dec 23, 2016 at 03:15:58PM +0100, Paolo Bonzini wrote:
> Considering that Easter is on April 16th, we'd probably want to have the
> release before that date even in case of a slip.
> 
> On the other hand, the Christmas / New Year break here means that we'll
> have to make the development time 1-2 week shorter in practice.
> 
> 2016-02-21 2.9 soft freeze
> 2016-03-07 hard freeze / rc0
> 2016-03-28 rc3 (+3 weeks)
> 2016-04-04 rc4 or release
> 2016-04-11 release (if rc4)
> 
> One possibility is to make soft freeze happen a few days later.
> Peter/Stefan, how did the experiment go with the new rules for soft
> freeze?  Is it worth repeating it for 2.9 and would it make sense to
> shorten soft freeze given the new rules?

I would shorten the soft freeze by 1 week.

Overall the 2.8 release went smoothly.  We got unlucky right at the end
with a release blocker but otherwise it was fine.

Stefan


signature.asc
Description: PGP signature

1 2 >

1 - 100 of 146 matches

Mail list logo