Re: [Qemu-devel] [PATCH qemu v5 04/12] spapr_pci_vfio: Enable multiple groups per container

[David Gibson]

On Wed, Apr 08, 2015 at 01:45:19PM +1000, Alexey Kardashevskiy wrote:
> On 04/08/2015 12:01 PM, David Gibson wrote:
> >On Tue, Mar 31, 2015 at 04:28:39PM +1100, Alexey Kardashevskiy wrote:
> >>This enables multiple IOMMU groups in one VFIO container which means
> >>that multiple devices from different groups can share the same IOMMU
> >>table (or tables if DDW).
> >>
> >>This removes a group id from vfio_container_ioctl(). The kernel support
> >>is required for this; if the host kernel does not have the support,
> >>it will allow only one group per container. The PHB's "iommuid" property
> >>is ignored.
> >>
> >>This adds a sanity check that there is just one VFIO container per
> >>PHB address space.
> >>
> >>Signed-off-by: Alexey Kardashevskiy 
> >
> >[snip]
> >>diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> >>index b012620..99e1900 100644
> >>--- a/hw/vfio/common.c
> >>+++ b/hw/vfio/common.c
> >>@@ -915,21 +915,23 @@ void vfio_put_base_device(VFIODevice *vbasedev)
> >>  close(vbasedev->fd);
> >>  }
> >>
> >>-static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
> >>+static int vfio_container_do_ioctl(AddressSpace *as,
> >> int req, void *param)
> >>  {
> >>-VFIOGroup *group;
> >>  VFIOContainer *container;
> >>-int ret = -1;
> >>+int ret;
> >>+VFIOAddressSpace *space;
> >>
> >>-group = vfio_get_group(groupid, as);
> >>-if (!group) {
> >>-error_report("vfio: group %d not registered", groupid);
> >>-return ret;
> >>-}
> >>+space = vfio_get_address_space(as);
> >>+container = QLIST_FIRST(&space->containers);
> >
> >So getting the container handle from the address space, rather than
> >the group id certainly makes more sense to me.
> >
> >>-container = group->container;
> >>-if (group->container) {
> >>+if (!container) {
> >>+error_report("vfio: container is not set");
> >>+return -1;
> >>+} else if (QLIST_NEXT(container, next)) {
> >>+error_report("vfio: multiple containers per PHB are not 
> >>supported");
> >>+return -1;
> >
> >But if only one PHB per address space is possible, why is the
> >containers field a list in the first place?
> 
> 
> Historically the list was added in 3df3e0a5872 (the patch of yours
> :) ).

Heh.

> In theory we could implement spapr-pci-bridge (derived from pci-bridge) with
> isolation capability (i.e. its own LIOBN/DMA window), in this case there
> could be multiple containers per PHB address space. Other archs could want
> multiple containers for some other reason. It would help me a lot if you
> remembered why you kept the list at the first place :)

Ok, I've looked over the patch and it has jogged my memory a bit.  So
the dumb answer is that it's because the per address-space list was
replacing a global list of containers

The more useful answer is that I think it was because I was
anticipating the possibility of working around the
one-group-per-container limit by allowing a single VFIOAddressSpace in
qemu to be backed by several containers, whose mappings would be kept
in sync from the userspace side by duplicating all mappings.

Anyway, I think that means the right way to implement this is by
duplicating the ioctl() across all the attached containers, rather
than picking just one.

> For now I guess I'll move the next patch ("vfio: spapr: Move SPAPR-related
> code to a separate file") before this one, do s/vfio_container_do_ioctl/
> vfio_spapr_container_do_ioctl/ and move it to hw/vfio/spapr.c. Makes
> sense?

That sounds fine, though I don't see that it really addresses the
question here.


> 
> 
> >>+} else {
> >>  ret = ioctl(container->fd, req, param);
> >>  if (ret < 0) {
> >>  error_report("vfio: failed to ioctl %d to container: ret=%d, 
> >> %s",
> >>@@ -937,12 +939,10 @@ static int vfio_container_do_ioctl(AddressSpace *as, 
> >>int32_t groupid,
> >>  }
> >>  }
> >>
> >>-vfio_put_group(group);
> >>-
> >>  return ret;
> >>  }
> >>
> >>-int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
> >>+int vfio_container_ioctl(AddressSpace *as,
> >>   int req, void *param)
> >>  {
> >>  /* We allow only certain ioctls to the container */
> >>@@ -957,5 +957,5 @@ int vfio_container_ioctl(AddressSpace *as, int32_t 
> >>groupid,
> >>  return -1;
> >>  }
> >>
> >>-return vfio_container_do_ioctl(as, groupid, req, param);
> >>+return vfio_container_do_ioctl(as, req, param);
> >>  }
> >>diff --git a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h
> >>index 0b26cd8..76b5744 100644
> >>--- a/include/hw/vfio/vfio.h
> >>+++ b/include/hw/vfio/vfio.h
> >>@@ -3,7 +3,7 @@
> >>
> >>  #include "qemu/typedefs.h"
> >>
> >>-extern int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
> >>+extern int vfio_container_ioctl(AddressSpace *as,
> >>  int req, void *param);
> >>
> >>  #endif
> >
> 
> 

Re: [Qemu-devel] [Qemu-block] Migration sometimes fails with IDE and Qemu 2.2.1

[Peter Lieven]

Am 07.04.2015 um 22:05 schrieb Paolo Bonzini:


On 07/04/2015 20:44, Peter Lieven wrote:

Has the cdrom the power of taking down the bus?

IDE can only issue one command per bus, so hda/hdb can take down each
other, and hdc/hdd can take down each other.  However, hda cannot take
down hdc and vice versa---so likely the CDROM cannot take down the hard
disk.


Right confirmed that the machines use BMDMA and the CDROM is hdc while
the boot disk is hda. IDE driveres report as E-IDE Revision 7.0.0alpha2

Peter

Re: [Qemu-devel] [Qemu-block] Migration sometimes fails with IDE and Qemu 2.2.1

[Peter Lieven]

Am 07.04.2015 um 21:13 schrieb John Snow:



On 04/07/2015 03:02 PM, Peter Lieven wrote:

Am 07.04.2015 um 20:56 schrieb John Snow:



On 04/07/2015 02:44 PM, Peter Lieven wrote:

Am 07.04.2015 um 17:29 schrieb Dr. David Alan Gilbert:

* Peter Lieven (p...@kamp.de) wrote:

Hi David,

Am 07.04.2015 um 10:43 schrieb Dr. David Alan Gilbert:

Any particular workload or reproducer?

Workload is almost zero. I try to figure out if there is a way to trigger it.

Maybe playing a role: Machine type is -M pc1.2 and we set -kvmclock as
CPU flag since kvmclock seemed to be quite buggy in 2.6.16...

Exact cmdline is:
/usr/bin/qemu-2.2.1  -enable-kvm  -M pc-1.2 -nodefaults -netdev type=tap,id=guest2,script=no,downscript=no,ifname=tap2 -device e1000,netdev=guest2,mac=52:54:00:ff:00:65 -drive 
format=raw,file=iscsi://172.21.200.53/iqn.2001-05.com.equallogic:4-52aed6-88a7e99a4-d9e00040fdc509a3-XXX-hd0/0,if=ide,cache=writeback,aio=native -serial null  -parallel null  -m 1024 -smp 2,sockets=1,cores=2,threads=1  -monitor 
tcp:0:4003,server,nowait -vnc :3 -qmp tcp:0:3003,server,nowait -name 'XXX' -boot order=c,once=dc,menu=off  -drive index=2,media=cdrom,if=ide,cache=unsafe,aio=native,readonly=on -k de  -incoming tcp:0:5003  -pidfile /var/run/qemu/vm-146.pid  
-mem-path /hugepages -mem-prealloc  -rtc base=utc -usb -usbdevice tablet -no-hpet -vga cirrus  -cpu qemu64,-kvmclock


Exact kernel is:
2.6.16.46-0.12-smp (i think this is SLES10 or sth.)

The machine does not hang. It seems just I/O is hanging. So you can type at the 
console or ping the system, but no longer login.

Thank you,
Peter

Interesting observation: Migrating the vServer again seems to fix to problem 
(at least in one case I could test just now).

2.6.8-24-smp is also affected.

How often does it fail - you say 'sometimes' - is it a 1/10 or a 1/1000 ?

Its more often than 1/10 I would say.

OK, that's not too bad - it's the 1/1000 that are really nasty to find.
In your setup, how easy would it be for you to try :
  with either 2.1 or current head?
  with a newer machine-type?
  without the cdrom?


Its all possible. I can clone the system and try everything on my test systems. 
I hope
it reproduces there.

Has the cdrom the power of taking down the bus?

Peter



I don't know if CDROM could stall the entire bus, but I suspect the reason for asking is this: dgilbert and I had tracked down a problem previously where during migration, outstanding requests being handled by the ATAPI code can get lost during 
migration if, for instance, the user has only prepared the command (via bmdma) but has not yet written to the register to activate the command yet.


That sounds like it could be related.



So if something like this happens:

- User writes to the ATA registers to program a command
- Migration occurs
- User writes to the BMDMA register to initiate the command

We can lose some of the state and data of the request. David had checked in a 
workaround for at least ATAPI that simply coaxes the guest OS into trying the 
command again to unstick it.


Do you have the commit for me?



http://lists.gnu.org/archive/html/qemu-devel/2014-12/msg01109.html



I think we determined last time that we couldn't fix this problem without changing the migration format, so we opted not to do it for 2.3. We had also only noticed it with ATAPI drives, not HDDs, so a proper fix got kicked down the road since we 
thought the workaround was sufficient.


Maybe normally we use virtio nowadays and maybe the new kernel implementation 
(libata /dev/sdX) can't get locked? What I do not understand is how a second 
migration can unlock from this state?



IIRC our success rate with reproducing it was something on the order of 1/50, 
too.

If you can reproduce it without a CDROM but using the BMDMA interface, that's a 
good data point. If you can't reproduce it using the ISA interface, that's a 
phenomenal data point and implicates BMDMA pretty heavily.


To be 100% sure we are talking about the same? How would I use the ISA and how 
would I use the BMDMA interface?

Thanks,
Peter



BMDMA is the PCI HBA for IDE, I think it's the default for most machines that 
aren't using the AHCI HBA.

To get ISA, try launching with the machine "isapc" which will force it, or add the device 
manually, it's named "isa-ide".
The BMDMA PCI device is just named "ide".


I will start more debugging today I found that other SuSE servers which use the 
newer interface (presenting as /dev/sdX)
do not suffer from the problem.

Peter

Re: [Qemu-devel] [PATCH v4 05/20] hw/acpi/aml-build: Add aml_interrupt() term

[Shannon Zhao]

On 2015/4/8 22:57, Alex Bennée wrote:
> 
> Shannon Zhao  writes:
> 
>> From: Shannon Zhao 
>>
>> Add aml_interrupt() for describing device interrupt in resource template.
>> These can be used to generating DSDT table for ACPI on ARM.
>>
>> Signed-off-by: Shannon Zhao 
>> Signed-off-by: Shannon Zhao 
>> ---
>>  hw/acpi/aml-build.c | 18 ++
>>  include/hw/acpi/aml-build.h |  1 +
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> index fefe7c7..bd1713c 100644
>> --- a/hw/acpi/aml-build.c
>> +++ b/hw/acpi/aml-build.c
>> @@ -527,6 +527,24 @@ Aml *aml_memory32_fixed(uint64_t addr, uint64_t size, 
>> uint8_t rw_flag)
>>  return var;
>>  }
>>  
>> +/*
>> + * ACPI 1.0: 6.4.3.6 Interrupt (Interrupt Resource Descriptor Macro)
>> + */
>> +Aml *aml_interrupt(uint8_t irq_flags, int irq)
>> +{
>> +Aml *var = aml_alloc();
>> +build_append_byte(var->buf, 0x89); /* Extended irq descriptor */
>> +build_append_byte(var->buf, 6); /* Length, bits[7:0] minimum value = 6 
>> */
>> +build_append_byte(var->buf, 0); /* Length, bits[15:8] minimum value = 0 
>> */
>> +build_append_byte(var->buf, irq_flags); /* Interrupt Vector
>> Information. */
> 
> As the spec says [7:4] is RES0 we might want to assert this is the case.
> 

Yes, we should check although the probability is very small.
But the reserve bits are different in ACPI 5.1.

Bit[7:5] Reserved (must be 0)
Bit[4] Wake Capability, _WKC

>> +build_append_byte(var->buf, 0x01); /* Interrupt table length = 1 */
>> +build_append_byte(var->buf, irq & 0xff); /* Interrupt Number bits[7:0] 
>> */
>> +build_append_byte(var->buf, (irq >> 8) & 0xff); /* Interrupt Number 
>> bits[15:8] */
>> +build_append_byte(var->buf, (irq >> 16) & 0xff); /* Interrupt Number 
>> bits[23:16] */
>> +build_append_byte(var->buf, (irq >> 24) & 0xff); /* Interrupt
>> Number bits[31:24] */
> 
> Again extractNN bitops?
> 
>> +return var;
>> +}
>> +
>>  /* ACPI 1.0b: 6.4.2.5 I/O Port Descriptor */
>>  Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
>>  uint8_t aln, uint8_t len)
>> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
>> index baa0652..315c729 100644
>> --- a/include/hw/acpi/aml-build.h
>> +++ b/include/hw/acpi/aml-build.h
>> @@ -163,6 +163,7 @@ Aml *aml_call2(const char *method, Aml *arg1, Aml *arg2);
>>  Aml *aml_call3(const char *method, Aml *arg1, Aml *arg2, Aml *arg3);
>>  Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml 
>> *arg4);
>>  Aml *aml_memory32_fixed(uint64_t addr, uint64_t size, uint8_t rw_flag);
>> +Aml *aml_interrupt(uint8_t irq_flags, int irq);
>>  Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
>>  uint8_t aln, uint8_t len);
>>  Aml *aml_operation_region(const char *name, AmlRegionSpace rs,
> 

Re: [Qemu-devel] [PATCH v4 04/20] hw/acpi/aml-build: Add aml_memory32_fixed() term

[Shannon Zhao]

On 2015/4/8 22:54, Alex Bennée wrote:
> 
> Shannon Zhao  writes:
> 
>> From: Shannon Zhao 
>>
>> Add aml_memory32_fixed() for describing device mmio region in resource 
>> template.
>> These can be used to generating DSDT table for ACPI on ARM.
>>
>> Signed-off-by: Shannon Zhao 
>> Signed-off-by: Shannon Zhao 
>> ---
>>  hw/acpi/aml-build.c | 22 ++
>>  include/hw/acpi/aml-build.h |  1 +
>>  2 files changed, 23 insertions(+)
>>
>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> index 8d01959..fefe7c7 100644
>> --- a/hw/acpi/aml-build.c
>> +++ b/hw/acpi/aml-build.c
>> @@ -505,6 +505,28 @@ Aml *aml_call4(const char *method, Aml *arg1, Aml 
>> *arg2, Aml *arg3, Aml *arg4)
>>  return var;
>>  }
>>  
>> +/*
>> + * ACPI 1.0: 6.4.3.4 Memory32Fixed (Memory Resource Descriptor Macro)
>> + */
>> +Aml *aml_memory32_fixed(uint64_t addr, uint64_t size, uint8_t rw_flag)
>> +{
>> +Aml *var = aml_alloc();
> 
> This is more aimed at the ACPI maintainers but I wonder if there should
> be an aml_alloc_sized that pre-allocates the GArray? Otherwise we spend
> a lot of time realloc'ing while building these entries up. Or even a
> varidac build_append_bytes?
> 
>> +build_append_byte(var->buf, 0x86); /* Memory32Fixed Resource Descriptor 
>> */
>> +build_append_byte(var->buf, 9); /* Length, bits[7:0] value = 9 */
>> +build_append_byte(var->buf, 0); /* Length, bits[15:8] value = 0 */
>> +build_append_byte(var->buf, rw_flag); /* Write status, 1 rw 0 ro */
>> +build_append_byte(var->buf, addr & 0xff); /* Range base address 
>> bits[7:0] */
>> +build_append_byte(var->buf, (addr >> 8) & 0xff); /* Range base address 
>> bits[15:8] */
>> +build_append_byte(var->buf, (addr >> 16) & 0xff); /* Range base address 
>> bits[23:16] */
>> +build_append_byte(var->buf, (addr >> 24) & 0xff); /* Range base
>> address bits[31:24] */
> 
> I'm should point out we have handy utility functions for bit fiddling:
> 
> build_append_byte(var->buf, extract64(addr, 8, 8)); /* Range base address 
> bits[15:8] */
> 

Great, will use these utility functions. Same with the other patch.

>> +
>> +build_append_byte(var->buf, size & 0xff); /* Range length bits[7:0] */
>> +build_append_byte(var->buf, (size >> 8) & 0xff); /* Range length 
>> bits[15:8] */
>> +build_append_byte(var->buf, (size >> 16) & 0xff); /* Range length 
>> bits[23:16] */
>> +build_append_byte(var->buf, (size >> 24) & 0xff); /* Range length
>> bits[31:24] */
> 
> Hmm we seem to have two 64 bit inputs which we only use 32 bits worth
> of. Maybe the prototype should be fixed to avoid accidents of accidentally
> passing in 64 bit values.
> 

Thanks, will fix this.

> 
>> +return var;
>> +}
>> +
>>  /* ACPI 1.0b: 6.4.2.5 I/O Port Descriptor */
>>  Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
>>  uint8_t aln, uint8_t len)
>> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
>> index 1705001..baa0652 100644
>> --- a/include/hw/acpi/aml-build.h
>> +++ b/include/hw/acpi/aml-build.h
>> @@ -162,6 +162,7 @@ Aml *aml_call1(const char *method, Aml *arg1);
>>  Aml *aml_call2(const char *method, Aml *arg1, Aml *arg2);
>>  Aml *aml_call3(const char *method, Aml *arg1, Aml *arg2, Aml *arg3);
>>  Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml 
>> *arg4);
>> +Aml *aml_memory32_fixed(uint64_t addr, uint64_t size, uint8_t rw_flag);
>>  Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base,
>>  uint8_t aln, uint8_t len);
>>  Aml *aml_operation_region(const char *name, AmlRegionSpace rs,
> 

Re: [Qemu-devel] [PATCH v4 03/20] hw/arm/virt-acpi-build: Basic framework for building ACPI tables on ARM

[Shannon Zhao]

On 2015/4/8 22:37, Alex Bennée wrote:
> 
> Shannon Zhao  writes:
> 
>> From: Shannon Zhao 
>>
>> Introduce a preliminary framework in virt-acpi-build.c with the main
>> ACPI build functions. It exposes the generated ACPI contents to
>> guest over fw_cfg.
>>
>> The required ACPI v5.1 tables for ARM are:
>> - RSDP: Initial table that points to XSDT
>> - RSDT: Points to FADT GTDT MADT tables
>> - FADT: Generic information about the machine
>> - GTDT: Generic timer description table
>> - MADT: Multiple APIC description table
>> - DSDT: Holds all information about system devices/peripherals, pointed by 
>> FADT
>>
>> Signed-off-by: Shannon Zhao 
>> Signed-off-by: Shannon Zhao 
>> ---
>>  hw/arm/Makefile.objs |   1 +
>>  hw/arm/virt-acpi-build.c | 198 
>> +++
>>  include/hw/arm/virt-acpi-build.h |  65 +
>>  3 files changed, 264 insertions(+)
>>  create mode 100644 hw/arm/virt-acpi-build.c
>>  create mode 100644 include/hw/arm/virt-acpi-build.h
>>
>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>> index 2577f68..a1bfb19 100644
>> --- a/hw/arm/Makefile.objs
>> +++ b/hw/arm/Makefile.objs
>> @@ -3,6 +3,7 @@ obj-$(CONFIG_DIGIC) += digic_boards.o
>>  obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
>>  obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
>>  obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
>> +obj-$(CONFIG_ACPI) += virt-acpi-build.o
>>  obj-y += netduino2.o
>>  
>>  obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> new file mode 100644
>> index 000..388838a
>> --- /dev/null
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -0,0 +1,198 @@
>> +/* Support for generating ACPI tables and passing them to Guests
>> + *
>> + * ARM virt ACPI generation
>> + *
>> + * Copyright (C) 2008-2010  Kevin O'Connor 
>> + * Copyright (C) 2006 Fabrice Bellard
>> + * Copyright (C) 2013 Red Hat Inc
>> + *
>> + * Author: Michael S. Tsirkin 
>> + *
>> + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO.,LTD.
>> + *
>> + * Author: Shannon Zhao 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> +
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> +
>> + * You should have received a copy of the GNU General Public License along
>> + * with this program; if not, see .
>> + */
>> +
>> +#include "hw/arm/virt-acpi-build.h"
>> +#include 
>> +#include 
>> +#include "qemu-common.h"
>> +#include "qemu/bitmap.h"
>> +#include "qemu/osdep.h"
>> +#include "qemu/range.h"
>> +#include "qemu/error-report.h"
>> +#include "qom/cpu.h"
>> +#include "target-arm/cpu.h"
>> +#include "hw/acpi/acpi-defs.h"
>> +#include "hw/acpi/acpi.h"
>> +#include "hw/nvram/fw_cfg.h"
>> +#include "hw/acpi/bios-linker-loader.h"
>> +#include "hw/loader.h"
>> +#include "hw/hw.h"
>> +
>> +#include "hw/acpi/aml-build.h"
>> +
>> +#include "qapi/qmp/qint.h"
>> +#include "qom/qom-qobject.h"
>> +#include "exec/ram_addr.h"
>> +
>> +/* #define DEBUG_ACPI_BUILD */
>> +#ifdef DEBUG_ACPI_BUILD
>> +#define ACPI_BUILD_DPRINTF(fmt, ...)\
>> +do {printf("ACPI_BUILD: " fmt, ## __VA_ARGS__); } while (0)
>> +#else
>> +#define ACPI_BUILD_DPRINTF(fmt, ...)
>> +#endif
> 
> I'd be tempted to rename this to D or at a push VIRT_ACPI_DEBUG just to
> make it distinct from where it was copied from.
> 
> You could also consider something like:
> 
> printf("%s: " fmt, __func__, ##__VA_ARGS__);
> 
> So log statements are pre-pended with their source functions.
> 

Thanks, will fix it.

>> +
>> +typedef
>> +struct AcpiBuildState {
>> +/* Copy of table in RAM (for patching). */
>> +ram_addr_t table_ram;
>> +ram_addr_t rsdp_ram;
>> +ram_addr_t linker_ram;
>> +/* Is table patched? */
>> +uint8_t patched;
>> +VirtGuestInfo *guest_info;
>> +} AcpiBuildState;
>> +
>> +static
>> +void virt_acpi_build(VirtGuestInfo *guest_info, AcpiBuildTables *tables)
>> +{
>> +GArray *table_offsets;
>> +
>> +table_offsets = g_array_new(false, true /* clear */,
>> +sizeof(uint32_t));
>> +
>> +bios_linker_loader_alloc(tables->linker, ACPI_BUILD_TABLE_FILE,
>> + 64, false /* high memory */);
>> +
>> +/*
>> + * The ACPI v5.1 tables for Hardware-reduced ACPI platform are:
>> + * RSDP
>> + * RSDT
>> + * FADT
>> + * GTDT
>> + * MADT
>> + * DSDT
>> + */
>> +
>> +/* Cleanup memory that's no longer used. */
>> +g_arra

Re: [Qemu-devel] 64-bit build of qemu-system-arm with mingw-w64 not functional

[Stefan Weil]

Am 08.04.2015 um 22:27 schrieb Liviu Ionescu:

On 08 Apr 2015, at 09:20, Stefan Weil  wrote:

Here is my package list (from Debian Jessie):

ii  binutils-mingw-w64-i686 2.22-8+deb7u2+2+deb7u1amd64
Cross-binutils for Win32 (x86) using MinGW-w64
ii  binutils-mingw-w64-x86-64 2.22-8+deb7u2+2+deb7u1amd64
Cross-binutils for Win64 (x64) using MinGW-w64
ii  g++-mingw-w64 4.6.3-14+8all  GNU C++ compiler 
for MinGW-w64
ii  g++-mingw-w64-i686 4.6.3-14+8amd64GNU C++ 
compiler for MinGW-w64 targeting Win32
ii  g++-mingw-w64-x86-64 4.6.3-14+8amd64GNU C++ 
compiler for MinGW-w64 targeting Win64
ii  gcc-mingw-w64 4.6.3-14+8all  GNU C compiler for 
MinGW-w64
ii  gcc-mingw-w64-base 4.6.3-14+8amd64GNU Compiler 
Collection for MinGW-w64 (base package)
ii  gcc-mingw-w64-i686 4.6.3-14+8amd64GNU C 
compiler for MinGW-w64 targeting Win32
ii  gcc-mingw-w64-x86-64 4.6.3-14+8amd64GNU C 
compiler for MinGW-w64 targeting Win64
ii  gdb-mingw-w64 7.4.1-1.1+5   amd64Cross-debugger for 
Win32 and Win64 using MinGW-w64
ii  gdb-mingw-w64-target 7.4.1-1.1+5   all  
Cross-debugger server for Win32 and Win64 using MinGW-w64
ii  gtk-mingw-w64-x86-64 3.6.4-20131201-2  all  Converted 
tgz package
ii  gtk2.0-mingw-w64-i686 2.24.10-20120208-2all  Converted 
tgz package
ii  libfdt-mingw-w64-i686 1.4.0-2   all  Converted 
tgz package
ii  libfdt-mingw-w64-x86-64 1.4.0-2   all  
Converted tgz package
ii  libpthreads-mingw-w64 2.9.1+dfsg-1  all  POSIX 
threads library for 32- and 64-bit Windows
ii  mingw-w64 2.0.3-1   all  Development 
environment targetting 32- and 64-bit Windows
ii  mingw-w64-i686-dev 2.0.3-1   all  Development 
files for MinGW-w64 targeting Win32
ii  mingw-w64-tools 2.0.3-1   amd64Development 
tools for 32- and 64-bit Windows
ii  mingw-w64-x86-64-dev 2.0.3-1   all  Development 
files for MinGW-w64 targeting Win64


Stefan,

I'm afraid there is a small misunderstanding here, I checked and even without 
upgrading the packages, the Debian 8 (Jessie) does not include the packages you 
are referring above, the actual versions I identified are:


You are right, I was wrong: my production server qemu.weilnetz.de uses 
Wheezy,

not Jessie, so my list was for Wheezy.

Here is the list for my Jessie system:

ii  binutils-mingw-w64-i686 2.25-5+5.2  
amd64Cross-binutils for Win32 (x86) using MinGW-w64
ii  binutils-mingw-w64-x86-64 2.25-5+5.2  
amd64Cross-binutils for Win64 (x64) using MinGW-w64
ii  g++-mingw-w64 4.9.1-19+14.3   all  GNU 
C++ compiler for MinGW-w64
ii  g++-mingw-w64-i686 4.9.1-19+14.3   amd64
GNU C++ compiler for MinGW-w64 targeting Win32
ii  g++-mingw-w64-x86-64 4.9.1-19+14.3   
amd64GNU C++ compiler for MinGW-w64 targeting Win64
ii  gcc-mingw-w64 4.9.1-19+14.3   all  GNU C 
compiler for MinGW-w64
ii  gcc-mingw-w64-base 4.9.1-19+14.3   amd64
GNU Compiler Collection for MinGW-w64 (base package)
ii  gcc-mingw-w64-i686 4.9.1-19+14.3   amd64
GNU C compiler for MinGW-w64 targeting Win32
ii  gcc-mingw-w64-x86-64 4.9.1-19+14.3   
amd64GNU C compiler for MinGW-w64 targeting Win64
ii  gtk-mingw-w64-x86-64 3.6.4-20131201-2
all  Converted tgz package
ii  gtk2.0-mingw-w64-i686 2.24.10-20120208-2  
all  Converted tgz package
ii  libfdt-mingw-w64-i686 1.4.0-2 
all  Converted tgz package
ii  libfdt-mingw-w64-x86-64 1.4.0-2 
all  Converted tgz package
ii  mingw-w64 4.0~rc3-1   all  
Development environment targeting 32- and 64-bit Windows
ii  mingw-w64-common 4.0~rc3-1   all  
Common files for Mingw-w64
ii  mingw-w64-i686-dev 4.0~rc3-1   all  
Development files for MinGW-w64 targeting Win32
ii  mingw-w64-x86-64-dev 4.0~rc3-1   
all  Development files for MinGW-w64 targeting Win64


Both the Wheezy and the Jessie environment work.


and the question from the previous message is still open, how did you install the 
following packages? the comment "Converted tgz package" is a good sign these 
are custom packages.

ii  gtk-mingw-w64-x86-64 3.6.4-20131201-2  all  Converted 
tgz package
ii  gtk2.0-mingw-w64-i686 2.24.10-20120208

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH v2 19/23] spapr: CPU hot unplug support

[Bharata B Rao]

On Tue, Apr 07, 2015 at 04:45:17PM +1000, Alexey Kardashevskiy wrote:
> On 03/24/2015 12:36 AM, Bharata B Rao wrote:
> >Support hot removal of CPU for sPAPR guests by sending the hot
> >unplug notification to the guest via EPOW interrupt.
> >
> >Signed-off-by: Bharata B Rao 
> >---
> >  hw/ppc/spapr.c| 78 
> > ++-
> >  linux-headers/linux/kvm.h |  1 +
> >  target-ppc/kvm.c  |  7 +
> >  target-ppc/kvm_ppc.h  |  6 
> >  4 files changed, 91 insertions(+), 1 deletion(-)
> >
> >diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >index b48994b..7b8784d 100644
> >--- a/hw/ppc/spapr.c
> >+++ b/hw/ppc/spapr.c
> >@@ -1468,6 +1468,12 @@ static void spapr_cpu_init(PowerPCCPU *cpu)
> >  qemu_register_reset(spapr_cpu_reset, cpu);
> >  }
> >
> >+static void spapr_cpu_destroy(PowerPCCPU *cpu)
> >+{
> >+xics_cpu_destroy(spapr->icp, cpu);
> >+qemu_unregister_reset(spapr_cpu_reset, cpu);
> >+}
> >+
> >  /* pSeries LPAR / sPAPR hardware init */
> >  static void ppc_spapr_init(MachineState *machine)
> >  {
> >@@ -1880,6 +1886,18 @@ static void spapr_cpu_hotplug_add(DeviceState *dev, 
> >CPUState *cs, Error **errp)
> >  }
> >  }
> >
> >+static void spapr_cpu_hotplug_remove(DeviceState *dev, CPUState *cs,
> >+ Error **errp)
> >+{
> >+PowerPCCPU *cpu = POWERPC_CPU(cs);
> >+int id = ppc_get_vcpu_dt_id(cpu);
> >+sPAPRDRConnector *drc =
> >+spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, id);
> >+sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> >+
> >+drck->detach(drc, dev, NULL, NULL, errp);
> >+}
> >+
> >  static void spapr_cpu_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> >  Error **errp)
> >  {
> >@@ -1911,6 +1929,51 @@ static void spapr_cpu_plug(HotplugHandler 
> >*hotplug_dev, DeviceState *dev,
> >  return;
> >  }
> >
> >+static int spapr_cpu_unplug(Object *obj, void *opaque)
> >+{
> >+Error **errp = opaque;
> >+DeviceState *dev = DEVICE(obj);
> >+CPUState *cs = CPU(dev);
> >+PowerPCCPU *cpu = POWERPC_CPU(cs);
> >+int id = ppc_get_vcpu_dt_id(cpu);
> >+int smt = kvmppc_smt_threads();
> >+sPAPRDRConnector *drc =
> >+spapr_dr_connector_by_id(SPAPR_DR_CONNECTOR_TYPE_CPU, id);
> >+
> >+spapr_cpu_destroy(cpu);
> >+
> >+/*
> >+ * SMT threads return from here, only main thread (core) will
> >+ * continue and signal hot unplug event to the guest.
> >+ */
> >+if ((id % smt) != 0) {
> >+return 0;
> >+}
> >+g_assert(drc);
> >+
> >+spapr_cpu_hotplug_remove(dev, cs, errp);
> >+if (*errp) {
> >+return -1;
> >+}
> >+spapr_hotplug_req_remove_event(drc);
> >+
> >+return 0;
> >+}
> >+
> >+static int spapr_cpu_core_unplug(Object *obj, void *opaque)
> >+{
> >+Error **errp = opaque;
> >+
> >+object_child_foreach(obj, spapr_cpu_unplug, errp);
> >+return 0;
> >+}
> >+
> >+static void spapr_cpu_socket_unplug(HotplugHandler *hotplug_dev,
> >+DeviceState *dev, Error **errp)
> >+{
> >+object_child_foreach(OBJECT(dev), spapr_cpu_core_unplug, errp);
> >+}
> >+
> >  static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
> >DeviceState *dev, Error **errp)
> >  {
> >@@ -1926,10 +1989,21 @@ static void spapr_machine_device_plug(HotplugHandler 
> >*hotplug_dev,
> >  }
> >  }
> >
> >+static void spapr_machine_device_unplug(HotplugHandler *hotplug_dev,
> >+  DeviceState *dev, Error **errp)
> >+{
> >+if (object_dynamic_cast(OBJECT(dev), TYPE_CPU_SOCKET)) {
> >+if (dev->hotplugged && spapr->dr_cpu_enabled) {
> >+spapr_cpu_socket_unplug(hotplug_dev, dev, errp);
> >+}
> >+}
> >+}
> >+
> >  static HotplugHandler *spapr_get_hotpug_handler(MachineState *machine,
> >   DeviceState *dev)
> >  {
> >-if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> >+if (object_dynamic_cast(OBJECT(dev), TYPE_CPU) ||
> >+object_dynamic_cast(OBJECT(dev), TYPE_CPU_SOCKET)) {
> 
> 
> What is this change for? I mean why is not it always socket-only? Commit log
> would not hurt here...

In the hot add case (do_device_add), the CPU socket device is realized first
which will realize the CPU core devices. Core devices will realize the CPU
thread devices. So the ->plug() operation happens as part of CPU thread devices
and hence hotplug_handler is returned only for TYPE_CPU.

However in case of hot remove, qdev_unplug() directly does ->unplug() and
hence I need to return the hotplug_handler for TYPE_CPU_SOCKET also.
This ensures that ->unplug() gets called for socket object where I take
care of recursively walking down the core and thread objects and unplugging
the CPU thread object eventually.

Regards,
Bharata.

Re: [Qemu-devel] [PATCH for-2.3] cris: memory: Replace memory_region_init_ram with memory_region_allocate_system_memory

[Edgar E. Iglesias]

On Sat, Apr 04, 2015 at 02:15:10PM +0200, Dirk Müller wrote:
> Commit 0b183fc871:"memory: move mem_path handling to
> memory_region_allocate_system_memory" split memory_region_init_ram and
> memory_region_init_ram_from_file. Also it moved mem-path handling a step
> up from memory_region_init_ram to memory_region_allocate_system_memory.
> 
> Therefore for any board that uses memory_region_init_ram directly,
> -mem-path is not supported.
> 
> Fix this by replacing memory_region_init_ram with
> memory_region_allocate_system_memory.
> 
> Cc: Edgar E. Iglesias 
> Signed-off-by: Dirk Mueller 
> ---
>  hw/cris/axis_dev88.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)


Hi,

A question, should this only be done for one of the memories?
BTW, I'm having problems git am:ing this patch, not sure why...

Cheers,
Edgar


> 
> diff --git a/hw/cris/axis_dev88.c b/hw/cris/axis_dev88.c
> index 0479196..3cae480 100644
> --- a/hw/cris/axis_dev88.c
> +++ b/hw/cris/axis_dev88.c
> @@ -270,9 +270,8 @@ void axisdev88_init(MachineState *machine)
>  env = &cpu->env;
> 
>  /* allocate RAM */
> -memory_region_init_ram(phys_ram, NULL, "axisdev88.ram", ram_size,
> -   &error_abort);
> -vmstate_register_ram_global(phys_ram);
> +memory_region_allocate_system_memory(phys_ram, NULL, "axisdev88.ram",
> + ram_size);
>  memory_region_add_subregion(address_space_mem, 0x4000, phys_ram);
> 
>  /* The ETRAX-FS has 128Kb on chip ram, the docs refer to it as the
> -- 
> 2.0.4

Re: [Qemu-devel] [PULL 22/62] block: Support Archipelago as a QEMU block backend

[Andreas Färber]

Am 08.08.2014 um 19:39 schrieb Kevin Wolf:
> From: Chrysostomos Nanakos 
> 
> VM Image on Archipelago volume is specified like this:
> 
> file.driver=archipelago,file.volume=[,file.mport=[,
> file.vport=][,file.segment=]]
> 
> 'archipelago' is the protocol.
> 
> 'mport' is the port number on which mapperd is listening. This is optional
> and if not specified, QEMU will make Archipelago to use the default port.
> 
> 'vport' is the port number on which vlmcd is listening. This is optional
> and if not specified, QEMU will make Archipelago to use the default port.
> 
> 'segment' is the name of the shared memory segment Archipelago stack is using.
> This is optional and if not specified, QEMU will make Archipelago to use the
> default value, 'archipelago'.
> 
> Examples:
> 
> file.driver=archipelago,file.volume=my_vm_volume
> file.driver=archipelago,file.volume=my_vm_volume,file.mport=123
> file.driver=archipelago,file.volume=my_vm_volume,file.mport=123,
> file.vport=1234
> file.driver=archipelago,file.volume=my_vm_volume,file.mport=123,
> file.vport=1234,file.segment=my_segment
> 
> Signed-off-by: Chrysostomos Nanakos 
> Reviewed-by: Stefan Hajnoczi 
> Signed-off-by: Kevin Wolf 
> ---
>  MAINTAINERS |   6 +
>  block/Makefile.objs |   2 +
>  block/archipelago.c | 787 
> 
>  configure   |  40 +++
>  4 files changed, 835 insertions(+)
>  create mode 100644 block/archipelago.c

Judging by configure output in v2.3.0-rc2, QEMU seems to rely on
libxseg, which is GPL-3.0+: https://github.com/grnet/libxseg

How can anyone legally build this backend then? o.O

Any chance libxseg can be relicensed to GPL-2.0+?

Regards,
Andreas

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

[Qemu-devel] seccomp breakage on arm

[Andreas Färber]

Hello,

I am seeing the following build failure on openSUSE Tumbleweed armv7l
with --enable-seccomp in v2.3.0-rc2:

[  551s] In file included from qemu-seccomp.c:16:0:
[  551s] /usr/include/libseccomp/seccomp.h:177:23: error: '__NR_mmap'
undeclared here (not in a function)
[  551s]  #define SCMP_SYS(x)  (__NR_##x)
[  551s]^
[  551s] qemu-seccomp.c:36:7: note: in expansion of macro 'SCMP_SYS'
[  551s]  { SCMP_SYS(mmap), 247 },
[  551s]^
[  551s] /usr/include/libseccomp/seccomp.h:177:23: error:
'__NR_getrlimit' undeclared here (not in a function)
[  551s]  #define SCMP_SYS(x)  (__NR_##x)
[  551s]^
[  551s] qemu-seccomp.c:57:7: note: in expansion of macro 'SCMP_SYS'
[  551s]  { SCMP_SYS(getrlimit), 245 },
[  551s]^
[  551s] /home/abuild/rpmbuild/BUILD/qemu-2.3.0-rc2/rules.mak:57: recipe
for target 'qemu-seccomp.o' failed
[  551s] make: *** [qemu-seccomp.o] Error 1

Is this a problem with libseccomp 2.2.0 / master and needs to be fixed
in the library? Or do we need to #ifdef some syscalls in qemu-seccomp.c?

aarch64 builds fine. For ppc and ppc64 we're carrying a libseccomp patch
in openSUSE, those build okay then; ppc64le is still missing in libseccomp.

Regards,
Andreas

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

Re: [Qemu-devel] [PATCH v4 07/20] hw/arm/virt-acpi-build: Generate FADT table and update ACPI headers

[Shannon Zhao]

On 2015/4/9 2:53, Michael S. Tsirkin wrote:
> On Fri, Apr 03, 2015 at 06:03:39PM +0800, Shannon Zhao wrote:
>> @@ -135,6 +138,43 @@ struct AcpiFadtDescriptorRev1
>>  } QEMU_PACKED;
>>  typedef struct AcpiFadtDescriptorRev1 AcpiFadtDescriptorRev1;
>>  
>> +struct acpi_generic_address {
>> +uint8_t space_id;/* Address space where struct or register 
>> exists */
>> +uint8_t bit_width;   /* Size in bits of given register */
>> +uint8_t bit_offset;  /* Bit offset within the register */
>> +uint8_t access_width;/* Minimum Access size (ACPI 3.0) */
>> +uint64_t address;/* 64-bit address of struct or register */
>> +} QEMU_PACKED;
> 
> Pls use standard QEMU style for structs.
> There are more like this in the patchset, pls find and fix them.
> 

Ok, thanks.

> 
>> +
>> +struct AcpiFadtDescriptorRev5_1 {
>> +ACPI_FADT_COMMON_DEF
>> +uint16_t boot_flags; /* IA-PC Boot Architecture Flags (see below 
>> for individual flags) */
>> +uint8_t reserved;/* Reserved, must be zero */
>> +uint32_t flags;  /* Miscellaneous flag bits (see below for 
>> individual flags) */
>> +struct acpi_generic_address reset_register; /* 64-bit address of the 
>> Reset register */
>> +uint8_t reset_value; /* Value to write to the reset_register port 
>> to reset the system */
>> +uint16_t arm_boot_flags; /* ARM-Specific Boot Flags (see below for 
>> individual flags) (ACPI 5.1) */
>> +uint8_t minor_revision;  /* FADT Minor Revision (ACPI 5.1) */
>> +uint64_t Xfacs;  /* 64-bit physical address of FACS */
>> +uint64_t Xdsdt;  /* 64-bit physical address of DSDT */
>> +struct acpi_generic_address xpm1a_event_block;  /* 64-bit Extended 
>> Power Mgt 1a Event Reg Blk address */
>> +struct acpi_generic_address xpm1b_event_block;  /* 64-bit Extended 
>> Power Mgt 1b Event Reg Blk address */
>> +struct acpi_generic_address xpm1a_control_block;/* 64-bit Extended 
>> Power Mgt 1a Control Reg Blk address */
>> +struct acpi_generic_address xpm1b_control_block;/* 64-bit Extended 
>> Power Mgt 1b Control Reg Blk address */
>> +struct acpi_generic_address xpm2_control_block; /* 64-bit Extended 
>> Power Mgt 2 Control Reg Blk address */
>> +struct acpi_generic_address xpm_timer_block;/* 64-bit Extended 
>> Power Mgt Timer Ctrl Reg Blk address */
>> +struct acpi_generic_address xgpe0_block;/* 64-bit Extended General 
>> Purpose Event 0 Reg Blk address */
>> +struct acpi_generic_address xgpe1_block;/* 64-bit Extended General 
>> Purpose Event 1 Reg Blk address */
>> +struct acpi_generic_address sleep_control;  /* 64-bit Sleep Control 
>> register (ACPI 5.0) */
>> +struct acpi_generic_address sleep_status;   /* 64-bit Sleep Status 
>> register (ACPI 5.0) */
>> +} QEMU_PACKED;
> 
> empty line missing.
> 

ok.

>> +typedef struct AcpiFadtDescriptorRev5_1 AcpiFadtDescriptorRev5_1;
>> +
>> +enum {
>> +ACPI_FADT_ARM_USE_PSCI_G_0_2,
>> +ACPI_FADT_ARM_PSCI_USE_HVC,
>> +};
> 
> These are part of tables, are they not?

They are the values of arm_boot_flags in AcpiFadtDescriptorRev5_1.

> Pls add = 0, = 1, so we don't change them by mistake.

Ok, thanks.

> 
>> +
>>  /*
>>   * ACPI 1.0 Root System Description Table (RSDT)
>>   */
>> -- 
>> 2.0.4
>>
> 
> .
> 

Re: [Qemu-devel] [snabb-devel] Re: [PATCH v2] vhost-user: add multi queue support

[Ouyang, Changchun]

Hi guys,

> -Original Message-
> From: snabb-de...@googlegroups.com [mailto:snabb-
> de...@googlegroups.com] On Behalf Of Michael S. Tsirkin
> Sent: Monday, April 6, 2015 11:07 PM
> To: Nikolay Nikolaev
> Cc: Long, Thomas; snabb-de...@googlegroups.com; ebl...@redhat.com;
> qemu-devel@nongnu.org; t...@virtualopensystems.com
> Subject: [snabb-devel] Re: [PATCH v2] vhost-user: add multi queue support
> 
> On Sat, Jan 24, 2015 at 02:22:29PM +0200, Nikolay Nikolaev wrote:
> > Vhost-user will implement the multiqueueu support in a similar way to
> > what
> 
> multiqueue
> 
> > vhost already has - a separate thread for each queue.
> >
> > To enable the multiqueue funcionality - a new command line parameter
> > "queues" is introduced for the vhost-user netdev.
> >
> > Changes since v1:
> >  - use s->nc.info_str when bringing up/down the backend
> >
> > Signed-off-by: Nikolay Nikolaev 
> > ---
> >  docs/specs/vhost-user.txt |5 +
> >  hw/virtio/vhost-user.c|6 +-
> >  net/vhost-user.c  |   39 +--
> >  qapi-schema.json  |6 +-
> >  qemu-options.hx   |5 +++--
> >  5 files changed, 43 insertions(+), 18 deletions(-)
> >
> > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> > index 650bb18..d7b208c 100644
> > --- a/docs/specs/vhost-user.txt
> > +++ b/docs/specs/vhost-user.txt
> 
> I've been thinking that the protocol might be a useful addition to the virtio
> spec. For this, as a minimum you would have to submit this document as a
> comment to virtio TC with a proposal to include it in the virtio spec.
> See
> https://www.oasis-
> open.org/committees/comments/index.php?wg_abbrev=virtio
> 
> Can you do this?
> 
> We can take it from there, though I would encourage your company to join
> as a contributor.
> 
> 
> > @@ -127,6 +127,11 @@ in the ancillary data:
> >  If Master is unable to send the full message or receives a wrong
> > reply it will  close the connection. An optional reconnection mechanism can
> be implemented.
> >
> > +Multi queue suport
> > +-
> > +The protocol supports multiple queues by setting all index fields in
> > +the sent messages to a properly calculated value.
> > +
> 
> Something that's not clear from this document is what happens with control
> VQ.
> Can you clarify please?
> 
> 
> >  Message types
> >  -
> >
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index
> > aefe0bb..83ebcaa 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -253,17 +253,20 @@ static int vhost_user_call(struct vhost_dev *dev,
> unsigned long int request,
> >  case VHOST_SET_VRING_NUM:
> >  case VHOST_SET_VRING_BASE:
> >  memcpy(&msg.state, arg, sizeof(struct vhost_vring_state));
> > +msg.state.index += dev->vq_index;
> >  msg.size = sizeof(m.state);
> >  break;
> >
> >  case VHOST_GET_VRING_BASE:
> >  memcpy(&msg.state, arg, sizeof(struct vhost_vring_state));
> > +msg.state.index += dev->vq_index;
> >  msg.size = sizeof(m.state);
> >  need_reply = 1;
> >  break;
> >
> >  case VHOST_SET_VRING_ADDR:
> >  memcpy(&msg.addr, arg, sizeof(struct vhost_vring_addr));
> > +msg.addr.index += dev->vq_index;
> >  msg.size = sizeof(m.addr);
> >  break;
> >
> > @@ -271,7 +274,7 @@ static int vhost_user_call(struct vhost_dev *dev,
> unsigned long int request,
> >  case VHOST_SET_VRING_CALL:
> >  case VHOST_SET_VRING_ERR:
> >  file = arg;
> > -msg.u64 = file->index & VHOST_USER_VRING_IDX_MASK;
> > +msg.u64 = (file->index + dev->vq_index) &
> > + VHOST_USER_VRING_IDX_MASK;

I identify one vq_index issue here when it is the case of VHOST_SET_VRING_CALL,
The vq_index is not initialized before it is used here, so it could be a random 
value.
It leads to error in vhost, when this random value is passed to vhost and vhost 
use this random value to set the vring call.
 
I have a quick fix for this, code changes as the following:
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 4e3a061..2fbdb93 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -157,6 +157,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)

 net->dev.nvqs = 2;
 net->dev.vqs = net->vqs;
+net->dev.vq_index = net->nc->queue_index;

 r = vhost_dev_init(&net->dev, options->opaque,
options->backend_type, options->force);
diff --git a/net/vhost-user.c b/net/vhost-user.c
index a0b4af2..b27190f 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -152,6 +152,7 @@ static int net_vhost_user_init(NetClientState *peer, const 
char *device,
 s->nc.receive_disabled = 1;
 s->chr = chr;
 s->vhostforce = vhostforce;
+s->nc.queue_index = i;

 qemu_chr_add_handlers(s->chr, NULL, NULL, net_vhost_user_event, s);
 }

Would you guys have a look at

Re: [Qemu-devel] [PATCH] kvm: fix slot flags sync between Qemu and KVM

[Xiao Guangrong]

On 04/08/2015 06:46 PM, Paolo Bonzini wrote:



On 08/04/2015 08:34, Xiao Guangrong wrote:

We noticed that KVM keeps tracking dirty for the memslots when
live migration failed which causes bad performance due to huge
page mapping disallowed for this kind of memslot

It is caused by slot flags does not properly sync-ed between Qemu
and KVM. Current code doing slot update depends on slot->flags
which hopes to omit unnecessary ioctl. However, slot->flags only
reflects the stauts of corresponding memory region, vmsave and
live migration do dirty tracking which overset
KVM_MEM_LOG_DIRTY_PAGES for the slot. That causes the slot status
recorded in the flags does not exactly match the stauts in kernel.

We fixed it by introducing slot->is_dirty_logging which indicates
the dirty status in kernel so that it helps us to sync the status
between userspace and kernel

Wanpeng Li 
Signed-off-by: Xiao Guangrong 


Hi Xiao,

the patch looks good.

However, I am planning to remove s->migration_log completely from QEMU
2.4 and have slot->flags also track the migration state.  This has the
side effect of fixing this bug.  I'll Cc you on the patches when I post
them (next week probably).


Good to know it, look forward to your patches. Thank you, Paolo!

Re: [Qemu-devel] [PATCH] tcg/tcg-op.c: Fix ld/st of 64 bit values on 32-bit bigendian hosts

[Richard Henderson]

On 04/08/2015 12:57 PM, Peter Maydell wrote:
> Switch the ifdef back to HOST_WORDS_BIGENDIAN.
> 
> Signed-off-by: Peter Maydell 

Doh.

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v6 0/8] QEMU memory hot unplug support

[Zhu Guihua]

On 04/08/2015 06:47 PM, Paulo Ricardo Paz Vital wrote:

On Wed, 2015-04-08 at 11:52 +0200, Michael S. Tsirkin wrote:

On Wed, Apr 08, 2015 at 05:49:42PM +0800, Zhu Guihua wrote:

Ping...

It's only been 4 days.  We are finalizing 2.3 so pls sit tight.

I agree with Michael, it's time to close 2.3.
But I have a question. Is the patch counter correct? I didn't found the
patch 1/8 in my mailbox neither in qemu-devel archive.


My partners have received patch 1/8, but it is not in qemu-devel archive 
indeed. I don't

know what happened.

I will resend the series later.

Thanks,
Zhu




On 04/02/2015 05:50 PM, Zhu Guihua wrote:

This patchset adds support to hot unplug memory.

Memory hot unplug is complicated multi-stage process. Unplug request callback
sends remove request. After guest os processes ejection request, OSPM will
execute _EJ0 to signal qemu that a device eject will be to occur. Then qemu
will call unplug callback to eject the device.

v6:
  -improve documentation of memory hot unplug
  -add trace event for device deletion
  -put fix about "Memory device control fields" register in a separate patch

v5:
  -reorganize the patchset
  -add documentation to understand patch easily
  -add MEMORY_SLOT_EJECT for initiating device eject
  -add support to send qmp event to notify mgmt about memory unplug error

v4:
  -reorganize the patchset
  -drop the new API acpi_send_gpe_event()
  -update ssdt-mem

v3:
  -commit message changes
  -reorganize the patchset, squash and separate some patches
  -update specs about acpi_mem_hotplug
  -first cleanup external state, then un-map and un-register memory device

v2:
  -do a generic for acpi to send gpe event
  -unparent object by PC_MACHINE
  -update description in acpi_mem_hotplug.txt
  -combine the last two patches in the last version
  -cleanup external state in acpi_memory_unplug_cb

Tang Chen (3):
   acpi, mem-hotplug: add acpi_memory_slot_status() to get MemStatus
   acpi, mem-hotplug: add unplug request cb for memory device
   acpi, mem-hotplug: add unplug cb for memory device

Zhu Guihua (5):
   docs: update documentation for memory hot unplug
   acpi: extend aml_field() to support UpdateRule
   acpi: fix "Memory device control fields" register
   acpi: add hardware implementation for memory hot unplug
   qmp-event: add event notification for memory hot unplug error

  docs/memory-hotplug.txt   | 23 --
  docs/qmp/qmp-events.txt   | 17 +++
  docs/specs/acpi_mem_hotplug.txt   | 58 +--
  hw/acpi/aml-build.c   |  4 +-
  hw/acpi/ich9.c| 19 ++--
  hw/acpi/memory_hotplug.c  | 96 ---
  hw/acpi/piix4.c   | 17 +--
  hw/core/qdev.c|  2 +-
  hw/i386/acpi-build.c  | 25 --
  hw/i386/acpi-dsdt-mem-hotplug.dsl | 13 +-
  hw/i386/pc.c  | 62 +++--
  include/hw/acpi/aml-build.h   | 10 +++-
  include/hw/acpi/memory_hotplug.h  | 12 +
  include/hw/acpi/pc-hotplug.h  |  3 ++
  include/hw/qdev-core.h|  1 +
  monitor.c |  1 +
  qapi/event.json   | 14 ++
  trace-events  |  4 ++
  18 files changed, 346 insertions(+), 35 deletions(-)

[Qemu-devel] [Bug 1441775] Re: possible null pointer dereference in qemuDomainPinEmulator()

[Eric Blake]

** Project changed: qemu => libvirt

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

Title:
  possible null pointer dereference in qemuDomainPinEmulator()

Status in libvirt virtualization API:
  New

Bug description:
  In src/qemu/qemu_driver.c the qemuDomainPinEmulator() routine
  basically does this

   virDomainObjPtr vm;

   if (!(vm = qemuDomObjFromDomain(dom)))
   goto cleanup;

  cleanup:
   qemuDomObjEndAPI(&vm);

  
  If "vm" is null, then this will crash.

  The bug seems to have been added in commit 540c339a, which removed a null 
pointer check:
  -if (vm)
  -virObjectUnlock(vm);
  +qemuDomObjEndAPI(&vm);

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

Re: [Qemu-devel] [PATCH 3/3] block: add 'node-name' field to BLOCK_IMAGE_CORRUPTED

[Eric Blake]

On 04/08/2015 03:29 AM, Alberto Garcia wrote:
> Since this event can occur in nodes that cannot have a device name
> associated, include also a field with the node name.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  block/qcow2.c   |  8 ++--
>  docs/qmp/qmp-events.txt | 21 +
>  qapi/block-core.json| 17 +++--
>  3 files changed, 30 insertions(+), 16 deletions(-)
> 
>  
> -- "device": Device name (json-string)
> -- "msg":Informative message (e.g., reason for the corruption) 
> (json-string)
> -- "offset": If the corruption resulted from an image access, this is the 
> access
> -offset into the image (json-int)
> -- "size":   If the corruption resulted from an image access, this is the 
> access
> -size (json-int)
> +- "device":Device name (json-string)
> +- "node-name": Node name (json-string, optional)
> +- "msg":   Informative message (e.g., reason for the corruption)
> +   (json-string)
> +- "offset":If the corruption resulted from an image access, this
> +   is the access offset into the image (json-int)
> +- "size":  If the corruption resulted from an image access, this
> +   is the access size (json-int)

Not your fault (so don't worry about fixing it here), but I still find
this definition of 'offset' confusing - is it the guest's offset, or the
host's offset?  I'm going to assume the host's offset (remember, on
qcow2, the guest offset 0 is never at host offset 0, because that is
reserved for the qcow2 header - but we CAN encounter a read error while
reading the qcow2 header).

Reviewed-by: Eric Blake 

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

Re: [Qemu-devel] [PATCH] qemu-config: Accept empty option values

[Eric Blake]

On 04/08/2015 12:16 PM, Eduardo Habkost wrote:
> Currently it is impossible to set an option in a config file to an empty
> string, because the parser matches only lines containing non-empty
> strings between double-quotes.
> 
> As sscanf() "[" conversion specifier only matches non-empty strings, add
> a special case for empty strings.

I avoid sscanf() as a rule (as it's behavior on %d is undefined in the
face of malicious input), so I had to read the man page; but you are
right.  Libvirt is trying to completely ban use of *scanf for that
reason; but obviously qemu is not quite so opposed to it.

> 
> Signed-off-by: Eduardo Habkost 
> ---
>  util/qemu-config.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/util/qemu-config.c b/util/qemu-config.c
> index 2d32ce7..9f9577d 100644
> --- a/util/qemu-config.c
> +++ b/util/qemu-config.c
> @@ -413,7 +413,8 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, 
> const char *fname)
>  opts = qemu_opts_create(list, NULL, 0, &error_abort);
>  continue;
>  }
> -if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2) {
> +if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2 ||
> +(value[0] = '\0', sscanf(line, " %63s = \"\"", arg) == 1)) {

This is one of the few times I've seen sscanf used in a well-defined
manner (albeit still arbitrarily limiting, in that we have fixed-size
buffers) - but having to rely on a comma operator to get there makes
this look quite arcane.  I still wonder if hand-rolling a real scanner
would beat the compactness of sscanf by making the code intentions a
little more discernible, and have the benefits of avoiding my sscanf
red-flag checker.  But my wonder is not enough to stop me from accepting
this hack as-is.

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 v5 18/21] iotests: add QMP event waiting queue

[John Snow]

A filter is added to allow callers to request very specific
events to be pulled from the event queue, while leaving undesired
events still in the stream.

This allows to poll for completion data for multiple asynchronous
events in any arbitrary order.

A new timeout context is added to the qmp pull_event method's
wait parameter to allow tests to fail if they do not complete
within some expected period of time.

Signed-off-by: John Snow 
---
 scripts/qmp/qmp.py|  6 +-
 tests/qemu-iotests/iotests.py | 38 ++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
index 20b6ec7..7f9ac1b 100644
--- a/scripts/qmp/qmp.py
+++ b/scripts/qmp/qmp.py
@@ -140,7 +140,8 @@ class QEMUMonitorProtocol:
 """
 Get and delete the first available QMP event.
 
-@param wait: block until an event is available (bool)
+@param wait: block until an event is available. If a float is provided,
+ use this as a timeout value. (bool, float)
 """
 self.__sock.setblocking(0)
 try:
@@ -151,7 +152,10 @@ class QEMUMonitorProtocol:
 pass
 self.__sock.setblocking(1)
 if not self.__events and wait:
+if isinstance(wait, float):
+self.__sock.settimeout(wait)
 self.__json_read(only_event=True)
+self.__sock.settimeout(None)
 event = self.__events[0]
 del self.__events[0]
 return event
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 1402854..e93e623 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -78,6 +78,23 @@ def create_image(name, size):
 i = i + 512
 file.close()
 
+# Test if 'match' is a recursive subset of 'event'
+def event_match(event, match=None):
+if match is None:
+return True
+
+for key in match:
+if key in event:
+if isinstance(event[key], dict):
+if not event_match(event[key], match[key]):
+return False
+elif event[key] != match[key]:
+return False
+else:
+return False
+
+return True
+
 class VM(object):
 '''A QEMU VM'''
 
@@ -92,6 +109,7 @@ class VM(object):
  '-machine', 'accel=qtest',
  '-display', 'none', '-vga', 'none']
 self._num_drives = 0
+self._events = []
 
 # This can be used to add an unused monitor instance.
 def add_monitor_telnet(self, ip, port):
@@ -202,14 +220,34 @@ class VM(object):
 
 def get_qmp_event(self, wait=False):
 '''Poll for one queued QMP events and return it'''
+if len(self._events) > 0:
+return self._events.pop(0)
 return self._qmp.pull_event(wait=wait)
 
 def get_qmp_events(self, wait=False):
 '''Poll for queued QMP events and return a list of dicts'''
 events = self._qmp.get_events(wait=wait)
+events.extend(self._events)
+del self._events[:]
 self._qmp.clear_events()
 return events
 
+def event_wait(self, name='BLOCK_JOB_COMPLETED', timeout=60.0, match=None):
+# Search cached events
+for event in self._events:
+if (event['event'] == name) and event_match(event, match):
+self._events.remove(event)
+return event
+
+# Poll for new events
+while True:
+event = self._qmp.pull_event(wait=timeout)
+if (event['event'] == name) and event_match(event, match):
+return event
+self._events.append(event)
+
+return None
+
 index_re = re.compile(r'([^\[]+)\[([^\]]+)\]')
 
 class QMPTestCase(unittest.TestCase):
-- 
2.1.0

[Qemu-devel] [PATCH v5 19/21] iotests: add simple incremental backup case

[John Snow]

Signed-off-by: John Snow 
---
 tests/qemu-iotests/124 | 174 +++--
 tests/qemu-iotests/124.out |   4 +-
 2 files changed, 172 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 85675ec..5c3b434 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -29,6 +29,51 @@ def io_write_patterns(img, patterns):
 iotests.qemu_io('-c', 'write -P%s %s %s' % pattern, img)
 
 
+def try_remove(img):
+try:
+os.remove(img)
+except OSError:
+pass
+
+
+class Bitmap:
+def __init__(self, name, drive):
+self.name = name
+self.drive = drive
+self.num = 0
+self.backups = list()
+
+def base_target(self):
+return (self.drive['backup'], None)
+
+def new_target(self, num=None):
+if num is None:
+num = self.num
+self.num = num + 1
+base = os.path.join(iotests.test_dir,
+"%s.%s." % (self.drive['id'], self.name))
+suff = "%i.%s" % (num, self.drive['fmt'])
+target = base + "inc" + suff
+reference = base + "ref" + suff
+self.backups.append((target, reference))
+return (target, reference)
+
+def last_target(self):
+if self.backups:
+return self.backups[-1]
+return self.base_target()
+
+def del_target(self):
+for image in self.backups.pop():
+try_remove(image)
+self.num -= 1
+
+def cleanup(self):
+for backup in self.backups:
+for image in backup:
+try_remove(image)
+
+
 class TestIncrementalBackup(iotests.QMPTestCase):
 def setUp(self):
 self.bitmaps = list()
@@ -73,6 +118,128 @@ class TestIncrementalBackup(iotests.QMPTestCase):
 iotests.qemu_img('create', '-f', fmt, img, size)
 self.files.append(img)
 
+
+def do_qmp_backup(self, error='Input/output error', **kwargs):
+res = self.vm.qmp('drive-backup', **kwargs)
+self.assert_qmp(res, 'return', {})
+
+event = self.vm.event_wait(name="BLOCK_JOB_COMPLETED",
+   match={'data': {'device': 
kwargs['device']}})
+self.assertIsNotNone(event)
+
+try:
+failure = self.dictpath(event, 'data/error')
+except AssertionError:
+# Backup succeeded.
+self.assert_qmp(event, 'data/offset', event['data']['len'])
+return True
+else:
+# Backup failed.
+self.assert_qmp(event, 'data/error', error)
+return False
+
+
+def create_anchor_backup(self, drive=None):
+if drive is None:
+drive = self.drives[-1]
+res = self.do_qmp_backup(device=drive['id'], sync='full',
+ format=drive['fmt'], target=drive['backup'])
+self.assertTrue(res)
+self.files.append(drive['backup'])
+return drive['backup']
+
+
+def make_reference_backup(self, bitmap=None):
+if bitmap is None:
+bitmap = self.bitmaps[-1]
+_, reference = bitmap.last_target()
+res = self.do_qmp_backup(device=bitmap.drive['id'], sync='full',
+ format=bitmap.drive['fmt'], target=reference)
+self.assertTrue(res)
+
+
+def add_bitmap(self, name, drive):
+bitmap = Bitmap(name, drive)
+self.bitmaps.append(bitmap)
+result = self.vm.qmp('block-dirty-bitmap-add', node=drive['id'],
+ name=bitmap.name)
+self.assert_qmp(result, 'return', {})
+return bitmap
+
+
+def prepare_backup(self, bitmap=None, parent=None):
+if bitmap is None:
+bitmap = self.bitmaps[-1]
+if parent is None:
+parent, _ = bitmap.last_target()
+
+target, _ = bitmap.new_target()
+self.img_create(target, bitmap.drive['fmt'], parent=parent)
+return target
+
+
+def create_incremental(self, bitmap=None, parent=None,
+   parentFormat=None, validate=True):
+if bitmap is None:
+bitmap = self.bitmaps[-1]
+if parent is None:
+parent, _ = bitmap.last_target()
+
+target = self.prepare_backup(bitmap, parent)
+res = self.do_qmp_backup(device=bitmap.drive['id'],
+ sync='dirty-bitmap', bitmap=bitmap.name,
+ format=bitmap.drive['fmt'], target=target,
+ mode='existing')
+if not res:
+bitmap.del_target();
+self.assertFalse(validate)
+else:
+self.make_reference_backup(bitmap)
+return res
+
+
+def check_backups(self):
+for bitmap in self.bitmaps:
+for incremental, reference in bitmap.backups:
+self.assertTrue(iotests.compare_images(incremental, reference))
+las

[Qemu-devel] [PATCH v5 17/21] iotests: add invalid input incremental backup tests

[John Snow]

Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
Reviewed-by: Stefan Hajnoczi 
---
 tests/qemu-iotests/124 | 104 +
 tests/qemu-iotests/124.out |   5 +++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 110 insertions(+)
 create mode 100644 tests/qemu-iotests/124
 create mode 100644 tests/qemu-iotests/124.out

diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
new file mode 100644
index 000..85675ec
--- /dev/null
+++ b/tests/qemu-iotests/124
@@ -0,0 +1,104 @@
+#!/usr/bin/env python
+#
+# Tests for incremental drive-backup
+#
+# Copyright (C) 2015 John Snow for Red Hat, Inc.
+#
+# Based on 056.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import os
+import iotests
+
+
+def io_write_patterns(img, patterns):
+for pattern in patterns:
+iotests.qemu_io('-c', 'write -P%s %s %s' % pattern, img)
+
+
+class TestIncrementalBackup(iotests.QMPTestCase):
+def setUp(self):
+self.bitmaps = list()
+self.files = list()
+self.drives = list()
+self.vm = iotests.VM()
+self.err_img = os.path.join(iotests.test_dir, 'err.%s' % 
iotests.imgfmt)
+
+# Create a base image with a distinctive patterning
+drive0 = self.add_node('drive0')
+self.img_create(drive0['file'], drive0['fmt'])
+self.vm.add_drive(drive0['file'])
+io_write_patterns(drive0['file'], (('0x41', 0, 512),
+   ('0xd5', '1M', '32k'),
+   ('0xdc', '32M', '124k')))
+self.vm.launch()
+
+
+def add_node(self, node_id, fmt=iotests.imgfmt, path=None, backup=None):
+if path is None:
+path = os.path.join(iotests.test_dir, '%s.%s' % (node_id, fmt))
+if backup is None:
+backup = os.path.join(iotests.test_dir,
+  '%s.full.backup.%s' % (node_id, fmt))
+
+self.drives.append({
+'id': node_id,
+'file': path,
+'backup': backup,
+'fmt': fmt })
+return self.drives[-1]
+
+
+def img_create(self, img, fmt=iotests.imgfmt, size='64M',
+   parent=None, parentFormat=None):
+if parent:
+if parentFormat is None:
+parentFormat = fmt
+iotests.qemu_img('create', '-f', fmt, img, size,
+ '-b', parent, '-F', parentFormat)
+else:
+iotests.qemu_img('create', '-f', fmt, img, size)
+self.files.append(img)
+
+def test_sync_dirty_bitmap_missing(self):
+self.assert_no_active_block_jobs()
+self.files.append(self.err_img)
+result = self.vm.qmp('drive-backup', device=self.drives[0]['id'],
+ sync='dirty-bitmap', format=self.drives[0]['fmt'],
+ target=self.err_img)
+self.assert_qmp(result, 'error/class', 'GenericError')
+
+
+def test_sync_dirty_bitmap_not_found(self):
+self.assert_no_active_block_jobs()
+self.files.append(self.err_img)
+result = self.vm.qmp('drive-backup', device=self.drives[0]['id'],
+ sync='dirty-bitmap', bitmap='unknown',
+ format=self.drives[0]['fmt'], target=self.err_img)
+self.assert_qmp(result, 'error/class', 'GenericError')
+
+
+def tearDown(self):
+self.vm.shutdown()
+for filename in self.files:
+try:
+os.remove(filename)
+except OSError:
+pass
+
+
+if __name__ == '__main__':
+iotests.main(supported_fmts=['qcow2'])
diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
new file mode 100644
index 000..fbc63e6
--- /dev/null
+++ b/tests/qemu-iotests/124.out
@@ -0,0 +1,5 @@
+..
+--
+Ran 2 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 6262127..020f357 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -123,4 +123,5 @@
 116 rw auto quick
 121 rw auto
 123 rw auto quick
+124 rw auto backing
 128 rw auto quick
-- 
2.1.0

[Qemu-devel] [PATCH v5 20/21] iotests: add incremental backup failure recovery test

[John Snow]

Test the failure case for incremental backups.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/124 | 57 ++
 tests/qemu-iotests/124.out |  4 ++--
 2 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 5c3b434..95f6de5 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -240,6 +240,63 @@ class TestIncrementalBackup(iotests.QMPTestCase):
 self.check_backups()
 
 
+def test_incremental_failure(self):
+'''Test: Verify backups made after a failure are correct.
+
+Simulate a failure during an incremental backup block job,
+emulate additional writes, then create another incremental backup
+afterwards and verify that the backup created is correct.
+'''
+
+# Create a blkdebug interface to this img as 'drive1',
+# but don't actually create a new image.
+drive1 = self.add_node('drive1', self.drives[0]['fmt'],
+   path=self.drives[0]['file'],
+   backup=self.drives[0]['backup'])
+result = self.vm.qmp('blockdev-add', options={
+'id': drive1['id'],
+'driver': drive1['fmt'],
+'file': {
+'driver': 'blkdebug',
+'image': {
+'driver': 'file',
+'filename': drive1['file']
+},
+'set-state': [{
+'event': 'flush_to_disk',
+'state': 1,
+'new_state': 2
+}],
+'inject-error': [{
+'event': 'read_aio',
+'errno': 5,
+'state': 2,
+'immediately': False,
+'once': True
+}],
+}
+})
+self.assert_qmp(result, 'return', {})
+
+self.create_anchor_backup(self.drives[0])
+self.add_bitmap('bitmap0', drive1)
+# Note: at this point, during a normal execution,
+# Assume that the VM resumes and begins issuing IO requests here.
+
+self.hmp_io_writes(drive1['id'], (('0xab', 0, 512),
+  ('0xfe', '16M', '256k'),
+  ('0x64', '32736k', '64k')))
+
+result = self.create_incremental(validate=False)
+self.assertFalse(result)
+self.hmp_io_writes(drive1['id'], (('0x9a', 0, 512),
+  ('0x55', '8M', '352k'),
+  ('0x78', '15872k', '1M')))
+self.create_incremental()
+self.vm.shutdown()
+self.check_backups()
+
+
 def test_sync_dirty_bitmap_missing(self):
 self.assert_no_active_block_jobs()
 self.files.append(self.err_img)
diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
index 8d7e996..89968f3 100644
--- a/tests/qemu-iotests/124.out
+++ b/tests/qemu-iotests/124.out
@@ -1,5 +1,5 @@
-...
+
 --
-Ran 3 tests
+Ran 4 tests
 
 OK
-- 
2.1.0

[Qemu-devel] [PATCH v5 14/21] block: Ensure consistent bitmap function prototypes

[John Snow]

We often don't need the BlockDriverState for functions
that operate on bitmaps. Remove it.

Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
Reviewed-by: Stefan Hajnoczi 
---
 block.c   | 13 ++---
 block/backup.c|  2 +-
 block/mirror.c| 26 ++
 blockdev.c|  2 +-
 include/block/block.h | 11 +--
 migration/block.c |  7 +++
 6 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/block.c b/block.c
index 843b0bf..16209a2 100644
--- a/block.c
+++ b/block.c
@@ -5460,7 +5460,7 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState 
*bs, const char *name)
 return NULL;
 }
 
-void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
+void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap)
 {
 assert(!bdrv_dirty_bitmap_frozen(bitmap));
 g_free(bitmap->name);
@@ -5629,7 +5629,7 @@ BlockDirtyInfoList 
*bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
 BlockDirtyInfo *info = g_new0(BlockDirtyInfo, 1);
 BlockDirtyInfoList *entry = g_new0(BlockDirtyInfoList, 1);
-info->count = bdrv_get_dirty_count(bs, bm);
+info->count = bdrv_get_dirty_count(bm);
 info->granularity = bdrv_dirty_bitmap_granularity(bm);
 info->has_name = !!bm->name;
 info->name = g_strdup(bm->name);
@@ -5676,20 +5676,19 @@ uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap 
*bitmap)
 return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
 }
 
-void bdrv_dirty_iter_init(BlockDriverState *bs,
-  BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
+void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
 {
 hbitmap_iter_init(hbi, bitmap->bitmap, 0);
 }
 
-void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors)
 {
 assert(bdrv_dirty_bitmap_enabled(bitmap));
 hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
 }
 
-void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
  int64_t cur_sector, int nr_sectors)
 {
 assert(bdrv_dirty_bitmap_enabled(bitmap));
@@ -5735,7 +5734,7 @@ void bdrv_set_dirty_iter(HBitmapIter *hbi, int64_t offset)
 hbitmap_iter_init(hbi, hbi->hb, offset);
 }
 
-int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
+int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
 {
 return hbitmap_count(bitmap->bitmap);
 }
diff --git a/block/backup.c b/block/backup.c
index 8513917..cdd41c5 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -284,7 +284,7 @@ static int coroutine_fn 
backup_run_incremental(BackupBlockJob *job)
 
 granularity = bdrv_dirty_bitmap_granularity(job->sync_bitmap);
 clusters_per_iter = MAX((granularity / BACKUP_CLUSTER_SIZE), 1);
-bdrv_dirty_iter_init(bs, job->sync_bitmap, &hbi);
+bdrv_dirty_iter_init(job->sync_bitmap, &hbi);
 
 /* Find the next dirty sector(s) */
 while ((sector = hbitmap_iter_next(&hbi)) != -1) {
diff --git a/block/mirror.c b/block/mirror.c
index f89eccf..dcd6f65 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -125,11 +125,9 @@ static void mirror_write_complete(void *opaque, int ret)
 MirrorOp *op = opaque;
 MirrorBlockJob *s = op->s;
 if (ret < 0) {
-BlockDriverState *source = s->common.bs;
 BlockErrorAction action;
 
-bdrv_set_dirty_bitmap(source, s->dirty_bitmap, op->sector_num,
-  op->nb_sectors);
+bdrv_set_dirty_bitmap(s->dirty_bitmap, op->sector_num, op->nb_sectors);
 action = mirror_error_action(s, false, -ret);
 if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) {
 s->ret = ret;
@@ -143,11 +141,9 @@ static void mirror_read_complete(void *opaque, int ret)
 MirrorOp *op = opaque;
 MirrorBlockJob *s = op->s;
 if (ret < 0) {
-BlockDriverState *source = s->common.bs;
 BlockErrorAction action;
 
-bdrv_set_dirty_bitmap(source, s->dirty_bitmap, op->sector_num,
-  op->nb_sectors);
+bdrv_set_dirty_bitmap(s->dirty_bitmap, op->sector_num, op->nb_sectors);
 action = mirror_error_action(s, true, -ret);
 if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) {
 s->ret = ret;
@@ -170,10 +166,9 @@ static uint64_t coroutine_fn 
mirror_iteration(MirrorBlockJob *s)
 
 s->sector_num = hbitmap_iter_next(&s->hbi);
 if (s->sector_num < 0) {
-bdrv_dirty_iter_init(source, s->dirty_bitmap, &s->hbi);
+bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi);
 s->sector_num = hbitmap_iter_next(&s->hbi);
-trace_mirror_restart_iter(s,
-  bdrv_get_dirty_count(source, 
s->dirty_bit

[Qemu-devel] [PATCH v5 21/21] iotests: add incremental backup granularity tests

[John Snow]

Test what happens if you fiddle with the granularity.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/124 | 58 +-
 tests/qemu-iotests/124.out |  4 ++--
 2 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 95f6de5..3ee78cd 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -158,11 +158,11 @@ class TestIncrementalBackup(iotests.QMPTestCase):
 self.assertTrue(res)
 
 
-def add_bitmap(self, name, drive):
+def add_bitmap(self, name, drive, **kwargs):
 bitmap = Bitmap(name, drive)
 self.bitmaps.append(bitmap)
 result = self.vm.qmp('block-dirty-bitmap-add', node=drive['id'],
- name=bitmap.name)
+ name=bitmap.name, **kwargs)
 self.assert_qmp(result, 'return', {})
 return bitmap
 
@@ -212,16 +212,9 @@ class TestIncrementalBackup(iotests.QMPTestCase):
 self.vm.hmp_qemu_io(drive, 'flush')
 
 
-def test_incremental_simple(self):
-'''
-Test: Create and verify three incremental backups.
-
-Create a bitmap and a full backup before VM execution begins,
-then create a series of three incremental backups "during execution,"
-i.e.; after IO requests begin modifying the drive.
-'''
+def do_incremental_simple(self, **kwargs):
 self.create_anchor_backup()
-self.add_bitmap('bitmap0', self.drives[0])
+self.add_bitmap('bitmap0', self.drives[0], **kwargs)
 
 # Sanity: Create a "hollow" incremental backup
 self.create_incremental()
@@ -240,6 +233,37 @@ class TestIncrementalBackup(iotests.QMPTestCase):
 self.check_backups()
 
 
+def test_incremental_simple(self):
+'''
+Test: Create and verify three incremental backups.
+
+Create a bitmap and a full backup before VM execution begins,
+then create a series of three incremental backups "during execution,"
+i.e.; after IO requests begin modifying the drive.
+'''
+return self.do_incremental_simple()
+
+
+def test_small_granularity(self):
+'''
+Test: Create and verify backups made with a small granularity bitmap.
+
+Perform the same test as test_incremental_simple, but with a 
granularity
+of only 32KiB instead of the present default of 64KiB.
+'''
+return self.do_incremental_simple(granularity=32768)
+
+
+def test_large_granularity(self):
+'''
+Test: Create and verify backups made with a large granularity bitmap.
+
+Perform the same test as test_incremental_simple, but with a 
granularity
+of 128KiB instead of the present default of 64KiB.
+'''
+return self.do_incremental_simple(granularity=131072)
+
+
 def test_incremental_failure(self):
 '''Test: Verify backups made after a failure are correct.
 
@@ -315,6 +339,18 @@ class TestIncrementalBackup(iotests.QMPTestCase):
 self.assert_qmp(result, 'error/class', 'GenericError')
 
 
+def test_sync_dirty_bitmap_bad_granularity(self):
+'''
+Test: Test what happens if we provide an improper granularity.
+
+The granularity must always be a power of 2.
+'''
+self.assert_no_active_block_jobs()
+self.assertRaises(AssertionError, self.add_bitmap,
+  'bitmap0', self.drives[0],
+  granularity=64000)
+
+
 def tearDown(self):
 self.vm.shutdown()
 for bitmap in self.bitmaps:
diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
index 89968f3..2f7d390 100644
--- a/tests/qemu-iotests/124.out
+++ b/tests/qemu-iotests/124.out
@@ -1,5 +1,5 @@
-
+...
 --
-Ran 4 tests
+Ran 7 tests
 
 OK
-- 
2.1.0

[Qemu-devel] [PATCH v5 11/21] qmp: add block-dirty-bitmap-clear

[John Snow]

Add bdrv_clear_dirty_bitmap and a matching QMP command,
qmp_block_dirty_bitmap_clear that enables a user to reset
the bitmap attached to a drive.

This allows us to reset a bitmap in the event of a full
drive backup.

Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
Reviewed-by: Stefan Hajnoczi 
---
 block.c   |  8 
 blockdev.c| 34 ++
 include/block/block.h |  1 +
 qapi/block-core.json  | 14 ++
 qmp-commands.hx   | 29 +
 5 files changed, 86 insertions(+)

diff --git a/block.c b/block.c
index 2367311..c26ac64 100644
--- a/block.c
+++ b/block.c
@@ -62,6 +62,7 @@
 struct BdrvDirtyBitmap {
 HBitmap *bitmap;
 BdrvDirtyBitmap *successor;
+int64_t size;
 char *name;
 bool disabled;
 QLIST_ENTRY(BdrvDirtyBitmap) list;
@@ -5491,6 +5492,7 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs,
 }
 bitmap = g_new0(BdrvDirtyBitmap, 1);
 bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(sector_granularity) - 1);
+bitmap->size = bitmap_size;
 bitmap->name = g_strdup(name);
 bitmap->disabled = false;
 QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
@@ -5693,6 +5695,12 @@ void bdrv_reset_dirty_bitmap(BlockDriverState *bs, 
BdrvDirtyBitmap *bitmap,
 hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
 }
 
+void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+assert(bdrv_dirty_bitmap_enabled(bitmap));
+hbitmap_reset(bitmap->bitmap, 0, bitmap->size);
+}
+
 static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
int nr_sectors)
 {
diff --git a/blockdev.c b/blockdev.c
index 90ba5b6..df96959 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2078,6 +2078,40 @@ void qmp_block_dirty_bitmap_remove(const char *node, 
const char *name,
 aio_context_release(aio_context);
 }
 
+/**
+ * Completely clear a bitmap, for the purposes of synchronizing a bitmap
+ * immediately after a full backup operation.
+ */
+void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
+  Error **errp)
+{
+AioContext *aio_context;
+BdrvDirtyBitmap *bitmap;
+BlockDriverState *bs;
+
+bitmap = block_dirty_bitmap_lookup(node, name, &bs, &aio_context, errp);
+if (!bitmap || !bs) {
+return;
+}
+
+if (bdrv_dirty_bitmap_frozen(bitmap)) {
+error_setg(errp,
+   "Bitmap '%s' is currently frozen and cannot be modified",
+   name);
+goto out;
+} else if (!bdrv_dirty_bitmap_enabled(bitmap)) {
+error_setg(errp,
+   "Bitmap '%s' is currently disabled and cannot be cleared",
+   name);
+goto out;
+}
+
+bdrv_clear_dirty_bitmap(bitmap);
+
+ out:
+aio_context_release(aio_context);
+}
+
 int hmp_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 const char *id = qdict_get_str(qdict, "id");
diff --git a/include/block/block.h b/include/block/block.h
index 80ac2cc..0961b1e 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -478,6 +478,7 @@ void bdrv_set_dirty_bitmap(BlockDriverState *bs, 
BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
 void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
  int64_t cur_sector, int nr_sectors);
+void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 void bdrv_dirty_iter_init(BlockDriverState *bs,
   BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
 void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7f629e2..11d8e2c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1021,6 +1021,20 @@
   'data': 'BlockDirtyBitmap' }
 
 ##
+# @block-dirty-bitmap-clear
+#
+# Clear (reset) a dirty bitmap on the device
+#
+# Returns: nothing on success
+#  If @node is not a valid block device, DeviceNotFound
+#  If @name is not found, GenericError with an explanation
+#
+# Since 2.4
+##
+{ 'command': 'block-dirty-bitmap-clear',
+  'data': 'BlockDirtyBitmap' }
+
+##
 # @block_set_io_throttle:
 #
 # Change I/O throttle limits for a block drive.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index da750f0..f2dc66c 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1327,6 +1327,35 @@ Example:
 EQMP
 
 {
+.name   = "block-dirty-bitmap-clear",
+.args_type  = "node:B,name:s",
+.mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_clear,
+},
+
+SQMP
+
+block-dirty-bitmap-clear
+
+Since 2.4
+
+Reset the dirty bitmap associated with a node so that an incremental backup
+from this point in time forward will only backup clusters modified after this
+clear operation.
+
+Arguments:
+
+- "node": device/node on which to remove dirty bitmap (

[Qemu-devel] [PATCH v5 16/21] hbitmap: truncate tests

[John Snow]

The general approach is to set bits close to the boundaries of
where we are truncating and ensure that everything appears to
have gone OK.

We test growing and shrinking by different amounts:
- Less than the granularity
- Less than the granularity, but across a boundary
- Less than sizeof(unsigned long)
- Less than sizeof(unsigned long), but across a ulong boundary
- More than sizeof(unsigned long)

Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
Reviewed-by: Stefan Hajnoczi 
---
 tests/test-hbitmap.c | 255 +++
 1 file changed, 255 insertions(+)

diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
index 8c902f2..9f41b5f 100644
--- a/tests/test-hbitmap.c
+++ b/tests/test-hbitmap.c
@@ -11,6 +11,8 @@
 
 #include 
 #include 
+#include 
+#include 
 #include "qemu/hbitmap.h"
 
 #define LOG_BITS_PER_LONG  (BITS_PER_LONG == 32 ? 5 : 6)
@@ -23,6 +25,7 @@ typedef struct TestHBitmapData {
 HBitmap   *hb;
 unsigned long *bits;
 size_t size;
+size_t old_size;
 intgranularity;
 } TestHBitmapData;
 
@@ -91,6 +94,44 @@ static void hbitmap_test_init(TestHBitmapData *data,
 }
 }
 
+static inline size_t hbitmap_test_array_size(size_t bits)
+{
+size_t n = (bits + BITS_PER_LONG - 1) / BITS_PER_LONG;
+return n ? n : 1;
+}
+
+static void hbitmap_test_truncate_impl(TestHBitmapData *data,
+   size_t size)
+{
+size_t n;
+size_t m;
+data->old_size = data->size;
+data->size = size;
+
+if (data->size == data->old_size) {
+return;
+}
+
+n = hbitmap_test_array_size(size);
+m = hbitmap_test_array_size(data->old_size);
+data->bits = g_realloc(data->bits, sizeof(unsigned long) * n);
+if (n > m) {
+memset(&data->bits[m], 0x00, sizeof(unsigned long) * (n - m));
+}
+
+/* If we shrink to an uneven multiple of sizeof(unsigned long),
+ * scrub the leftover memory. */
+if (data->size < data->old_size) {
+m = size % (sizeof(unsigned long) * 8);
+if (m) {
+unsigned long mask = (1ULL << m) - 1;
+data->bits[n-1] &= mask;
+}
+}
+
+hbitmap_truncate(data->hb, size);
+}
+
 static void hbitmap_test_teardown(TestHBitmapData *data,
   const void *unused)
 {
@@ -369,6 +410,198 @@ static void test_hbitmap_iter_granularity(TestHBitmapData 
*data,
 g_assert_cmpint(hbitmap_iter_next(&hbi), <, 0);
 }
 
+static void hbitmap_test_set_boundary_bits(TestHBitmapData *data, ssize_t diff)
+{
+size_t size = data->size;
+
+/* First bit */
+hbitmap_test_set(data, 0, 1);
+if (diff < 0) {
+/* Last bit in new, shortened map */
+hbitmap_test_set(data, size + diff - 1, 1);
+
+/* First bit to be truncated away */
+hbitmap_test_set(data, size + diff, 1);
+}
+/* Last bit */
+hbitmap_test_set(data, size - 1, 1);
+if (data->granularity == 0) {
+hbitmap_test_check_get(data);
+}
+}
+
+static void hbitmap_test_check_boundary_bits(TestHBitmapData *data)
+{
+size_t size = MIN(data->size, data->old_size);
+
+if (data->granularity == 0) {
+hbitmap_test_check_get(data);
+hbitmap_test_check(data, 0);
+} else {
+/* If a granularity was set, note that every distinct
+ * (bit >> granularity) value that was set will increase
+ * the bit pop count by 2^granularity, not just 1.
+ *
+ * The hbitmap_test_check facility does not currently tolerate
+ * non-zero granularities, so test the boundaries and the population
+ * count manually.
+ */
+g_assert(hbitmap_get(data->hb, 0));
+g_assert(hbitmap_get(data->hb, size - 1));
+g_assert_cmpint(2 << data->granularity, ==, hbitmap_count(data->hb));
+}
+}
+
+/* Generic truncate test. */
+static void hbitmap_test_truncate(TestHBitmapData *data,
+  size_t size,
+  ssize_t diff,
+  int granularity)
+{
+hbitmap_test_init(data, size, granularity);
+hbitmap_test_set_boundary_bits(data, diff);
+hbitmap_test_truncate_impl(data, size + diff);
+hbitmap_test_check_boundary_bits(data);
+}
+
+static void test_hbitmap_truncate_nop(TestHBitmapData *data,
+  const void *unused)
+{
+hbitmap_test_truncate(data, L2, 0, 0);
+}
+
+/**
+ * Grow by an amount smaller than the granularity, without crossing
+ * a granularity alignment boundary. Effectively a NOP.
+ */
+static void test_hbitmap_truncate_grow_negligible(TestHBitmapData *data,
+  const void *unused)
+{
+size_t size = L2 - 1;
+size_t diff = 1;
+int granularity = 1;
+
+hbitmap_test_truncate(data, size, diff, granularity);
+}
+
+/**
+ * Shrink by an amount smaller than the granularity, without crossi

[Qemu-devel] [PATCH v5 10/21] qmp: Add support of "dirty-bitmap" sync mode for drive-backup

[John Snow]

For "dirty-bitmap" sync mode, the block job will iterate through the
given dirty bitmap to decide if a sector needs backup (backup all the
dirty clusters and skip clean ones), just as allocation conditions of
"top" sync mode.

Signed-off-by: Fam Zheng 
Signed-off-by: John Snow 
---
 block.c   |   9 +++
 block/backup.c| 156 +++---
 block/mirror.c|   4 ++
 blockdev.c|  18 +-
 hmp.c |   3 +-
 include/block/block.h |   1 +
 include/block/block_int.h |   2 +
 qapi/block-core.json  |  13 ++--
 qmp-commands.hx   |   7 ++-
 9 files changed, 180 insertions(+), 33 deletions(-)

diff --git a/block.c b/block.c
index 9d30379..2367311 100644
--- a/block.c
+++ b/block.c
@@ -5717,6 +5717,15 @@ static void bdrv_reset_dirty(BlockDriverState *bs, 
int64_t cur_sector,
 }
 }
 
+/**
+ * Advance an HBitmapIter to an arbitrary offset.
+ */
+void bdrv_set_dirty_iter(HBitmapIter *hbi, int64_t offset)
+{
+assert(hbi->hb);
+hbitmap_iter_init(hbi, hbi->hb, offset);
+}
+
 int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
 {
 return hbitmap_count(bitmap->bitmap);
diff --git a/block/backup.c b/block/backup.c
index 1c535b1..8513917 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -37,6 +37,8 @@ typedef struct CowRequest {
 typedef struct BackupBlockJob {
 BlockJob common;
 BlockDriverState *target;
+/* bitmap for sync=dirty-bitmap */
+BdrvDirtyBitmap *sync_bitmap;
 MirrorSyncMode sync_mode;
 RateLimit limit;
 BlockdevOnError on_source_error;
@@ -242,6 +244,92 @@ static void backup_complete(BlockJob *job, void *opaque)
 g_free(data);
 }
 
+static bool coroutine_fn yield_and_check(BackupBlockJob *job)
+{
+if (block_job_is_cancelled(&job->common)) {
+return true;
+}
+
+/* we need to yield so that qemu_aio_flush() returns.
+ * (without, VM does not reboot)
+ */
+if (job->common.speed) {
+uint64_t delay_ns = ratelimit_calculate_delay(&job->limit,
+  job->sectors_read);
+job->sectors_read = 0;
+block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, delay_ns);
+} else {
+block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, 0);
+}
+
+if (block_job_is_cancelled(&job->common)) {
+return true;
+}
+
+return false;
+}
+
+static int coroutine_fn backup_run_incremental(BackupBlockJob *job)
+{
+bool error_is_read;
+int ret = 0;
+int clusters_per_iter;
+uint32_t granularity;
+int64_t sector;
+int64_t cluster;
+int64_t end;
+int64_t last_cluster = -1;
+BlockDriverState *bs = job->common.bs;
+HBitmapIter hbi;
+
+granularity = bdrv_dirty_bitmap_granularity(job->sync_bitmap);
+clusters_per_iter = MAX((granularity / BACKUP_CLUSTER_SIZE), 1);
+bdrv_dirty_iter_init(bs, job->sync_bitmap, &hbi);
+
+/* Find the next dirty sector(s) */
+while ((sector = hbitmap_iter_next(&hbi)) != -1) {
+cluster = sector / BACKUP_SECTORS_PER_CLUSTER;
+
+/* Fake progress updates for any clusters we skipped */
+if (cluster != last_cluster + 1) {
+job->common.offset += ((cluster - last_cluster - 1) *
+   BACKUP_CLUSTER_SIZE);
+}
+
+for (end = cluster + clusters_per_iter; cluster < end; cluster++) {
+if (yield_and_check(job)) {
+return ret;
+}
+
+do {
+ret = backup_do_cow(bs, cluster * BACKUP_SECTORS_PER_CLUSTER,
+BACKUP_SECTORS_PER_CLUSTER, 
&error_is_read);
+if ((ret < 0) &&
+backup_error_action(job, error_is_read, -ret) ==
+BLOCK_ERROR_ACTION_REPORT) {
+return ret;
+}
+} while (ret < 0);
+}
+
+/* If the bitmap granularity is smaller than the backup granularity,
+ * we need to advance the iterator pointer to the next cluster. */
+if (granularity < BACKUP_CLUSTER_SIZE) {
+bdrv_set_dirty_iter(&hbi, cluster * BACKUP_SECTORS_PER_CLUSTER);
+}
+
+last_cluster = cluster - 1;
+}
+
+/* Play some final catchup with the progress meter */
+end = DIV_ROUND_UP(job->common.len, BACKUP_CLUSTER_SIZE);
+if (last_cluster + 1 < end) {
+job->common.offset += ((end - last_cluster - 1) * BACKUP_CLUSTER_SIZE);
+}
+
+return ret;
+}
+
 static void coroutine_fn backup_run(void *opaque)
 {
 BackupBlockJob *job = opaque;
@@ -259,8 +347,7 @@ static void coroutine_fn backup_run(void *opaque)
 qemu_co_rwlock_init(&job->flush_rwlock);
 
 start = 0;
-end = DIV_ROUND_UP(job->common.len / BDRV_SECTOR_SIZE,
-   BACKUP_SECTORS_PER_CLUSTER);
+end = DIV_ROUND_UP(job->common.len, BACKUP_CLUSTER_SIZE)

[Qemu-devel] [PATCH v5 15/21] block: Resize bitmaps on bdrv_truncate

[John Snow]

Signed-off-by: John Snow 
---
 block.c| 18 ++
 include/qemu/hbitmap.h | 10 ++
 util/hbitmap.c | 48 
 3 files changed, 76 insertions(+)

diff --git a/block.c b/block.c
index 16209a2..42839a0 100644
--- a/block.c
+++ b/block.c
@@ -113,6 +113,7 @@ static void bdrv_set_dirty(BlockDriverState *bs, int64_t 
cur_sector,
int nr_sectors);
 static void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector,
  int nr_sectors);
+static void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
 /* If non-zero, use only whitelisted block drivers */
 static int use_bdrv_whitelist;
 
@@ -3583,6 +3584,7 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
 ret = drv->bdrv_truncate(bs, offset);
 if (ret == 0) {
 ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
+bdrv_dirty_bitmap_truncate(bs);
 if (bs->blk) {
 blk_dev_resize_cb(bs->blk);
 }
@@ -5593,6 +5595,22 @@ BdrvDirtyBitmap 
*bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
 return parent;
 }
 
+/**
+ * Truncates _all_ bitmaps attached to a BDS.
+ */
+static void bdrv_dirty_bitmap_truncate(BlockDriverState *bs)
+{
+BdrvDirtyBitmap *bitmap;
+uint64_t size = bdrv_nb_sectors(bs);
+
+QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
+if (bdrv_dirty_bitmap_frozen(bitmap)) {
+continue;
+}
+hbitmap_truncate(bitmap->bitmap, size);
+}
+}
+
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
 {
 BdrvDirtyBitmap *bm, *next;
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index c19c1cb..a75157e 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -65,6 +65,16 @@ struct HBitmapIter {
 HBitmap *hbitmap_alloc(uint64_t size, int granularity);
 
 /**
+ * hbitmap_truncate:
+ * @hb: The bitmap to change the size of.
+ * @size: The number of elements to change the bitmap to accommodate.
+ *
+ * truncate or grow an existing bitmap to accommodate a new number of elements.
+ * This may invalidate existing HBitmapIterators.
+ */
+void hbitmap_truncate(HBitmap *hb, uint64_t size);
+
+/**
  * hbitmap_merge:
  * @a: The bitmap to store the result in.
  * @b: The bitmap to merge into @a.
diff --git a/util/hbitmap.c b/util/hbitmap.c
index ba11fd3..1ad3bf3 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -400,6 +400,54 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity)
 return hb;
 }
 
+void hbitmap_truncate(HBitmap *hb, uint64_t size)
+{
+bool shrink;
+unsigned i;
+uint64_t num_elements = size;
+uint64_t old;
+
+/* Size comes in as logical elements, adjust for granularity. */
+size = (size + (1ULL << hb->granularity) - 1) >> hb->granularity;
+assert(size <= ((uint64_t)1 << HBITMAP_LOG_MAX_SIZE));
+shrink = size < hb->size;
+
+/* bit sizes are identical; nothing to do. */
+if (size == hb->size) {
+return;
+}
+
+/* If we're losing bits, let's clear those bits before we invalidate all of
+ * our invariants. This helps keep the bitcount consistent, and will 
prevent
+ * us from carrying around garbage bits beyond the end of the map.
+ */
+if (shrink) {
+/* Don't clear partial granularity groups;
+ * start at the first full one. */
+uint64_t start = QEMU_ALIGN_UP(num_elements, 1 << hb->granularity);
+uint64_t fix_count = (hb->size << hb->granularity) - num_elements;
+
+assert(fix_count);
+hbitmap_reset(hb, start, fix_count);
+}
+
+hb->size = size;
+for (i = HBITMAP_LEVELS; i-- > 0; ) {
+size = MAX(BITS_TO_LONGS(size), 1);
+if (hb->sizes[i] == size) {
+break;
+}
+old = hb->sizes[i];
+hb->sizes[i] = size;
+hb->levels[i] = g_realloc(hb->levels[i], size * sizeof(unsigned long));
+if (!shrink) {
+memset(&hb->levels[i][old], 0x00,
+   (size - old) * sizeof(*hb->levels[i]));
+}
+}
+}
+
+
 /**
  * Given HBitmaps A and B, let A := A (BITOR) B.
  * Bitmap B will not be modified.
-- 
2.1.0

[Qemu-devel] [PATCH v5 02/21] qapi: Add optional field "name" to block dirty bitmap

[John Snow]

From: Fam Zheng 

This field will be set for user created dirty bitmap. Also pass in an
error pointer to bdrv_create_dirty_bitmap, so when a name is already
taken on this BDS, it can report an error message. This is not global
check, two BDSes can have dirty bitmap with a common name.

Implemented bdrv_find_dirty_bitmap to find a dirty bitmap by name, will
be used later when other QMP commands want to reference dirty bitmap by
name.

Add bdrv_dirty_bitmap_make_anon. This unsets the name of dirty bitmap.

Signed-off-by: Fam Zheng 
Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Stefan Hajnoczi 
---
 block.c   | 32 +++-
 block/mirror.c|  2 +-
 include/block/block.h |  7 ++-
 migration/block.c |  2 +-
 qapi/block-core.json  |  4 +++-
 5 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index f2f8ae7..0da35ae 100644
--- a/block.c
+++ b/block.c
@@ -53,6 +53,7 @@
 
 struct BdrvDirtyBitmap {
 HBitmap *bitmap;
+char *name;
 QLIST_ENTRY(BdrvDirtyBitmap) list;
 };
 
@@ -5435,7 +5436,28 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, 
QEMUIOVector *qiov)
 return true;
 }
 
-BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int 
granularity,
+BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *name)
+{
+BdrvDirtyBitmap *bm;
+
+assert(name);
+QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
+if (bm->name && !strcmp(name, bm->name)) {
+return bm;
+}
+}
+return NULL;
+}
+
+void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
+{
+g_free(bitmap->name);
+bitmap->name = NULL;
+}
+
+BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
+  int granularity,
+  const char *name,
   Error **errp)
 {
 int64_t bitmap_size;
@@ -5443,6 +5465,10 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
 
 assert((granularity & (granularity - 1)) == 0);
 
+if (name && bdrv_find_dirty_bitmap(bs, name)) {
+error_setg(errp, "Bitmap already exists: %s", name);
+return NULL;
+}
 granularity >>= BDRV_SECTOR_BITS;
 assert(granularity);
 bitmap_size = bdrv_nb_sectors(bs);
@@ -5453,6 +5479,7 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
 }
 bitmap = g_new0(BdrvDirtyBitmap, 1);
 bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
+bitmap->name = g_strdup(name);
 QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
 return bitmap;
 }
@@ -5464,6 +5491,7 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, 
BdrvDirtyBitmap *bitmap)
 if (bm == bitmap) {
 QLIST_REMOVE(bitmap, list);
 hbitmap_free(bitmap->bitmap);
+g_free(bitmap->name);
 g_free(bitmap);
 return;
 }
@@ -5482,6 +5510,8 @@ BlockDirtyInfoList 
*bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 info->count = bdrv_get_dirty_count(bs, bm);
 info->granularity =
 ((int64_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap));
+info->has_name = !!bm->name;
+info->name = g_strdup(bm->name);
 entry->value = info;
 *plist = entry;
 plist = &entry->next;
diff --git a/block/mirror.c b/block/mirror.c
index 4056164..f073ad7 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -703,7 +703,7 @@ static void mirror_start_job(BlockDriverState *bs, 
BlockDriverState *target,
 s->granularity = granularity;
 s->buf_size = MAX(buf_size, granularity);
 
-s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, errp);
+s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
 if (!s->dirty_bitmap) {
 return;
 }
diff --git a/include/block/block.h b/include/block/block.h
index 4c57d63..05e32f9 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -449,8 +449,13 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, 
QEMUIOVector *qiov);
 
 struct HBitmapIter;
 typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
-BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int 
granularity,
+BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
+  int granularity,
+  const char *name,
   Error **errp);
+BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
+const char *name);
+void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap 
*bitmap);
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
 int bdr

[Qemu-devel] [PATCH v5 13/21] block: add BdrvDirtyBitmap documentation

[John Snow]

Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
Reviewed-by: Stefan Hajnoczi 
---
 block.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index c104cdd..843b0bf 100644
--- a/block.c
+++ b/block.c
@@ -60,11 +60,11 @@
  * or enabled. A frozen bitmap can only abdicate() or reclaim().
  */
 struct BdrvDirtyBitmap {
-HBitmap *bitmap;
-BdrvDirtyBitmap *successor;
-int64_t size;
-char *name;
-bool disabled;
+HBitmap *bitmap;/* Dirty sector bitmap implementation */
+BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */
+char *name; /* Optional non-empty unique ID */
+int64_t size;   /* Size of the bitmap (Number of sectors) */
+bool disabled;  /* Bitmap is read-only */
 QLIST_ENTRY(BdrvDirtyBitmap) list;
 };
 
-- 
2.1.0

[Qemu-devel] [PATCH v5 07/21] hbitmap: add hbitmap_merge

[John Snow]

We add a bitmap merge operation to assist in error cases
where we wish to combine two bitmaps together.

This is algorithmically O(bits) provided HBITMAP_LEVELS remains
constant. For a full bitmap on a 64bit machine:
sum(bits/64^k, k, 0, HBITMAP_LEVELS) ~= 1.01587 * bits

We may be able to improve running speed for particularly sparse
bitmaps by using iterators, but the running time for dense maps
will be worse.

We present the simpler solution first, and we can refine it later
if needed.

Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
Reviewed-by: Stefan Hajnoczi 
---
 include/qemu/hbitmap.h | 11 +++
 util/hbitmap.c | 29 +
 2 files changed, 40 insertions(+)

diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 550d7ce..c19c1cb 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -65,6 +65,17 @@ struct HBitmapIter {
 HBitmap *hbitmap_alloc(uint64_t size, int granularity);
 
 /**
+ * hbitmap_merge:
+ * @a: The bitmap to store the result in.
+ * @b: The bitmap to merge into @a.
+ *
+ * Merge two bitmaps together.
+ * A := A (BITOR) B.
+ * B is left unmodified.
+ */
+bool hbitmap_merge(HBitmap *a, const HBitmap *b);
+
+/**
  * hbitmap_empty:
  * @hb: HBitmap to operate on.
  *
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 5b78613..ba11fd3 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -399,3 +399,32 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity)
 hb->levels[0][0] |= 1UL << (BITS_PER_LONG - 1);
 return hb;
 }
+
+/**
+ * Given HBitmaps A and B, let A := A (BITOR) B.
+ * Bitmap B will not be modified.
+ */
+bool hbitmap_merge(HBitmap *a, const HBitmap *b)
+{
+int i, j;
+
+if ((a->size != b->size) || (a->granularity != b->granularity)) {
+return false;
+}
+
+if (hbitmap_count(b) == 0) {
+return true;
+}
+
+/* This merge is O(size), as BITS_PER_LONG and HBITMAP_LEVELS are constant.
+ * It may be possible to improve running times for sparsely populated maps
+ * by using hbitmap_iter_next, but this is suboptimal for dense maps.
+ */
+for (i = HBITMAP_LEVELS - 1; i >= 0; i--) {
+for (j = 0; j < a->sizes[i]; j++) {
+a->levels[i][j] |= b->levels[i][j];
+}
+}
+
+return true;
+}
-- 
2.1.0

[Qemu-devel] [PATCH v5 12/21] qmp: Add dirty bitmap status field in query-block

[John Snow]

Add the "frozen" status booleans, to inform clients
when a bitmap is occupied doing a task.

Signed-off-by: Fam Zheng 
Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
Reviewed-by: Stefan Hajnoczi 
---
 block.c  | 1 +
 qapi/block-core.json | 5 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index c26ac64..c104cdd 100644
--- a/block.c
+++ b/block.c
@@ -5633,6 +5633,7 @@ BlockDirtyInfoList 
*bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 info->granularity = bdrv_dirty_bitmap_granularity(bm);
 info->has_name = !!bm->name;
 info->name = g_strdup(bm->name);
+info->frozen = bdrv_dirty_bitmap_frozen(bm);
 entry->value = info;
 *plist = entry;
 plist = &entry->next;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 11d8e2c..5a77cca 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -336,10 +336,13 @@
 #
 # @granularity: granularity of the dirty bitmap in bytes (since 1.4)
 #
+# @frozen: whether the dirty bitmap is frozen (Since 2.4)
+#
 # Since: 1.3
 ##
 { 'type': 'BlockDirtyInfo',
-  'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32'} }
+  'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
+   'frozen': 'bool'} }
 
 ##
 # @BlockInfo:
-- 
2.1.0

[Qemu-devel] [PATCH v5 09/21] block: Add bitmap successors

[John Snow]

A bitmap successor is an anonymous BdrvDirtyBitmap that is intended to
be created just prior to a sensitive operation (e.g. Incremental Backup)
that can either succeed or fail, but during the course of which we still
want a bitmap tracking writes.

On creating a successor, we "freeze" the parent bitmap which prevents
its deletion, enabling, anonymization, or creating a bitmap with the
same name.

On success, the parent bitmap can "abdicate" responsibility to the
successor, which will inherit its name. The successor will have been
tracking writes during the course of the backup operation. The parent
will be safely deleted.

On failure, we can "reclaim" the successor from the parent, unifying
them such that the resulting bitmap describes all writes occurring since
the last successful backup, for instance. Reclamation will thaw the
parent, but not explicitly re-enable it.

BdrvDirtyBitmap operations that target a single bitmap are protected
by assertions that the bitmap is not frozen and/or disabled.

BdrvDirtyBitmap operations that target a group of bitmaps, such as
bdrv_{set,reset}_dirty will ignore frozen/disabled drives with a
conditional instead.

QMP transactions that enable/disable bitmaps have extra error checking
surrounding them that prevent modifying bitmaps that are frozen.

Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
Reviewed-by: Stefan Hajnoczi 
---
 block.c   | 104 +-
 blockdev.c|   7 
 include/block/block.h |  10 +
 qapi/block-core.json  |   1 +
 4 files changed, 121 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index db742a9..9d30379 100644
--- a/block.c
+++ b/block.c
@@ -51,8 +51,17 @@
 #include 
 #endif
 
+/**
+ * A BdrvDirtyBitmap can be in three possible states:
+ * (1) successor is false and disabled is false: full r/w mode
+ * (2) successor is false and disabled is true: read only mode ("disabled")
+ * (3) successor is set: frozen mode.
+ * A frozen bitmap cannot be renamed, deleted, anonymized, cleared, set,
+ * or enabled. A frozen bitmap can only abdicate() or reclaim().
+ */
 struct BdrvDirtyBitmap {
 HBitmap *bitmap;
+BdrvDirtyBitmap *successor;
 char *name;
 bool disabled;
 QLIST_ENTRY(BdrvDirtyBitmap) list;
@@ -5452,6 +5461,7 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState 
*bs, const char *name)
 
 void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
 {
+assert(!bdrv_dirty_bitmap_frozen(bitmap));
 g_free(bitmap->name);
 bitmap->name = NULL;
 }
@@ -5487,9 +5497,98 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs,
 return bitmap;
 }
 
+bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
+{
+return bitmap->successor;
+}
+
 bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)
 {
-return !bitmap->disabled;
+return !(bitmap->disabled || bitmap->successor);
+}
+
+/**
+ * Create a successor bitmap destined to replace this bitmap after an 
operation.
+ * Requires that the bitmap is not frozen and has no successor.
+ */
+int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
+   BdrvDirtyBitmap *bitmap, Error **errp)
+{
+uint64_t granularity;
+BdrvDirtyBitmap *child;
+
+if (bdrv_dirty_bitmap_frozen(bitmap)) {
+error_setg(errp, "Cannot create a successor for a bitmap that is "
+   "currently frozen");
+return -1;
+}
+assert(!bitmap->successor);
+
+/* Create an anonymous successor */
+granularity = bdrv_dirty_bitmap_granularity(bitmap);
+child = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
+if (!child) {
+return -1;
+}
+
+/* Successor will be on or off based on our current state. */
+child->disabled = bitmap->disabled;
+
+/* Install the successor and freeze the parent */
+bitmap->successor = child;
+return 0;
+}
+
+/**
+ * For a bitmap with a successor, yield our name to the successor,
+ * Delete the old bitmap, and return a handle to the new bitmap.
+ */
+BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
+BdrvDirtyBitmap *bitmap,
+Error **errp)
+{
+char *name;
+BdrvDirtyBitmap *successor = bitmap->successor;
+
+if (successor == NULL) {
+error_setg(errp, "Cannot relinquish control if "
+   "there's no successor present");
+return NULL;
+}
+
+name = bitmap->name;
+bitmap->name = NULL;
+successor->name = name;
+bitmap->successor = NULL;
+bdrv_release_dirty_bitmap(bs, bitmap);
+
+return successor;
+}
+
+/**
+ * In cases of failure where we can no longer safely delete the parent,
+ * We may wish to re-join the parent and child/successor.
+ * The merged parent will be un-frozen, but not explicitly re-enabled.
+ */
+BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(

[Qemu-devel] [PATCH v5 04/21] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove

[John Snow]

The new command pair is added to manage a user created dirty bitmap. The
dirty bitmap's name is mandatory and must be unique for the same device,
but different devices can have bitmaps with the same names.

The granularity is an optional field. If it is not specified, we will
choose a default granularity based on the cluster size if available,
clamped to between 4K and 64K to mirror how the 'mirror' code was
already choosing granularity. If we do not have cluster size info
available, we choose 64K. This code has been factored out into a helper
shared with block/mirror.

This patch also introduces the 'block_dirty_bitmap_lookup' helper,
which takes a device name and a dirty bitmap name and validates the
lookup, returning NULL and setting errp if there is a problem with
either field. This helper will be re-used in future patches in this
series.

The types added to block-core.json will be re-used in future patches
in this series, see:
'qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable}'

Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
Reviewed-by: Stefan Hajnoczi 
---
 block.c   |  20 +
 block/mirror.c|  10 +
 blockdev.c| 117 ++
 include/block/block.h |   1 +
 qapi/block-core.json  |  55 
 qmp-commands.hx   |  56 
 6 files changed, 250 insertions(+), 9 deletions(-)

diff --git a/block.c b/block.c
index 2b609e7..ebea54d 100644
--- a/block.c
+++ b/block.c
@@ -5530,6 +5530,26 @@ int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap 
*bitmap, int64_t sector
 }
 }
 
+/**
+ * Chooses a default granularity based on the existing cluster size,
+ * but clamped between [4K, 64K]. Defaults to 64K in the case that there
+ * is no cluster size information available.
+ */
+uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs)
+{
+BlockDriverInfo bdi;
+uint32_t granularity;
+
+if (bdrv_get_info(bs, &bdi) >= 0 && bdi.cluster_size > 0) {
+granularity = MAX(4096, bdi.cluster_size);
+granularity = MIN(65536, granularity);
+} else {
+granularity = 65536;
+}
+
+return granularity;
+}
+
 void bdrv_dirty_iter_init(BlockDriverState *bs,
   BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
 {
diff --git a/block/mirror.c b/block/mirror.c
index f8aac33..1cb700e 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -668,15 +668,7 @@ static void mirror_start_job(BlockDriverState *bs, 
BlockDriverState *target,
 MirrorBlockJob *s;
 
 if (granularity == 0) {
-/* Choose the default granularity based on the target file's cluster
- * size, clamped between 4k and 64k.  */
-BlockDriverInfo bdi;
-if (bdrv_get_info(target, &bdi) >= 0 && bdi.cluster_size != 0) {
-granularity = MAX(4096, bdi.cluster_size);
-granularity = MIN(65536, granularity);
-} else {
-granularity = 65536;
-}
+granularity = bdrv_get_default_bitmap_granularity(target);
 }
 
 assert ((granularity & (granularity - 1)) == 0);
diff --git a/blockdev.c b/blockdev.c
index fbb3a79..7d71b81 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1164,6 +1164,68 @@ out_aio_context:
 return NULL;
 }
 
+/**
+ * block_dirty_bitmap_lookup:
+ * Return a dirty bitmap (if present), after validating
+ * the node reference and bitmap names.
+ *
+ * @node: The name of the BDS node to search for bitmaps
+ * @name: The name of the bitmap to search for
+ * @pbs: Output pointer for BDS lookup, if desired. Can be NULL.
+ * @paio: Output pointer for aio_context acquisition, if desired. Can be NULL.
+ * @errp: Output pointer for error information. Can be NULL.
+ *
+ * @return: A bitmap object on success, or NULL on failure.
+ */
+static BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node,
+  const char *name,
+  BlockDriverState **pbs,
+  AioContext **paio,
+  Error **errp)
+{
+BlockDriverState *bs;
+BdrvDirtyBitmap *bitmap;
+AioContext *aio_context;
+
+if (!node) {
+error_setg(errp, "Node cannot be NULL");
+return NULL;
+}
+if (!name) {
+error_setg(errp, "Bitmap name cannot be NULL");
+return NULL;
+}
+bs = bdrv_lookup_bs(node, node, NULL);
+if (!bs) {
+error_setg(errp, "Node '%s' not found", node);
+return NULL;
+}
+
+aio_context = bdrv_get_aio_context(bs);
+aio_context_acquire(aio_context);
+
+bitmap = bdrv_find_dirty_bitmap(bs, name);
+if (!bitmap) {
+error_setg(errp, "Dirty bitmap '%s' not found", name);
+goto fail;
+}
+
+if (pbs) {
+*pbs = bs;
+}
+if (paio) {
+*paio = aio_context;
+} else {
+aio_

[Qemu-devel] [PATCH v5 01/21] docs: incremental backup documentation

[John Snow]

Reviewed-by: Max Reitz 
Signed-off-by: John Snow 
---
 docs/bitmaps.md | 311 
 1 file changed, 311 insertions(+)
 create mode 100644 docs/bitmaps.md

diff --git a/docs/bitmaps.md b/docs/bitmaps.md
new file mode 100644
index 000..ad8c33b
--- /dev/null
+++ b/docs/bitmaps.md
@@ -0,0 +1,311 @@
+# Dirty Bitmaps and Incremental Backup
+
+* Dirty Bitmaps are objects that track which data needs to be backed up for the
+  next incremental backup.
+
+* Dirty bitmaps can be created at any time and attached to any node
+  (not just complete drives.)
+
+## Dirty Bitmap Names
+
+* A dirty bitmap's name is unique to the node, but bitmaps attached to 
different
+nodes can share the same name.
+
+## Bitmap Modes
+
+* A Bitmap can be "frozen," which means that it is currently in-use by a backup
+operation and cannot be deleted, renamed, written to, reset,
+etc.
+
+## Basic QMP Usage
+
+### Supported Commands ###
+
+* block-dirty-bitmap-add
+* block-dirty-bitmap-remove
+* block-dirty-bitmap-clear
+
+### Creation
+
+* To create a new bitmap, enabled, on the drive with id=drive0:
+
+```json
+{ "execute": "block-dirty-bitmap-add",
+  "arguments": {
+"node": "drive0",
+"name": "bitmap0"
+  }
+}
+```
+
+* This bitmap will have a default granularity that matches the cluster size of
+its associated drive, if available, clamped to between [4KiB, 64KiB].
+The current default for qcow2 is 64KiB.
+
+* To create a new bitmap that tracks changes in 32KiB segments:
+
+```json
+{ "execute": "block-dirty-bitmap-add",
+  "arguments": {
+"node": "drive0",
+"name": "bitmap0",
+"granularity": 32768
+  }
+}
+```
+
+### Deletion
+
+* Can be performed on a disabled bitmap, but not a frozen one.
+
+* Because bitmaps are only unique to the node to which they are attached,
+you must specify the node/drive name here, too.
+
+```json
+{ "execute": "block-dirty-bitmap-remove",
+  "arguments": {
+"node": "drive0",
+"name": "bitmap0"
+  }
+}
+```
+
+### Resetting
+
+* Resetting a bitmap will clear all information it holds.
+* An incremental backup created from an empty bitmap will copy no data,
+as if nothing has changed.
+
+```json
+{ "execute": "block-dirty-bitmap-clear",
+  "arguments": {
+"node": "drive0",
+"name": "bitmap0"
+  }
+}
+```
+
+## Transactions (Not yet implemented)
+
+* Transactional commands are forthcoming in a future version,
+  and are not yet available for use. This section serves as
+  documentation of intent for their design and usage.
+
+### Justification
+Bitmaps can be safely modified when the VM is paused or halted by using
+the basic QMP commands. For instance, you might perform the following actions:
+
+1. Boot the VM in a paused state.
+2. Create a full drive backup of drive0.
+3. Create a new bitmap attached to drive0.
+4. Resume execution of the VM.
+5. Incremental backups are ready to be created.
+
+At this point, the bitmap and drive backup would be correctly in sync,
+and incremental backups made from this point forward would be correctly aligned
+to the full drive backup.
+
+This is not particularly useful if we decide we want to start incremental
+backups after the VM has been running for a while, for which we will need to
+perform actions such as the following:
+
+1. Boot the VM and begin execution.
+2. Using a single transaction, perform the following operations:
+* Create bitmap0.
+* Create a full drive backup of drive0.
+3. Incremental backups are now ready to be created.
+
+### Supported Bitmap Transactions
+
+* block-dirty-bitmap-add
+* block-dirty-bitmap-clear
+
+The usages are identical to their respective QMP commands, but see below
+for examples.
+
+### Example: New Incremental Backup
+
+As outlined in the justification, perhaps we want to create a new incremental
+backup chain attached to a drive.
+
+```json
+{ "execute": "transaction",
+  "arguments": {
+"actions": [
+  {"type": "block-dirty-bitmap-add",
+   "data": {"node": "drive0", "name": "bitmap0"} },
+  {"type": "drive-backup",
+   "data": {"device": "drive0", "target": "/path/to/full_backup.img",
+"sync": "full", "format": "qcow2"} }
+]
+  }
+}
+```
+
+### Example: New Incremental Backup Anchor Point
+
+Maybe we just want to create a new full backup with an existing bitmap and
+want to reset the bitmap to track the new chain.
+
+```json
+{ "execute": "transaction",
+  "arguments": {
+"actions": [
+  {"type": "block-dirty-bitmap-clear",
+   "data": {"node": "drive0", "name": "bitmap0"} },
+  {"type": "drive-backup",
+   "data": {"device": "drive0", "target": "/path/to/new_full_backup.img",
+"sync": "full", "format": "qcow2"} }
+]
+  }
+}
+```
+
+## Incremental Backups
+
+The star of the show.
+
+**Nota Bene!** Only incremental backups of entire drives are supported for now.
+So despite the fact that you can attach a bitmap to any arbitrary node, they 
are
+only currentl

[Qemu-devel] [PATCH v5 08/21] block: Add bitmap disabled status

[John Snow]

Add a status indicating the enabled/disabled state of the bitmap.
A bitmap is by default enabled, but you can lock the bitmap into
a read-only state by setting disabled = true.

A previous version of this patch added a QMP interface for changing
the state of the bitmap, but it has since been removed for now until
a use case emerges where this state must be revealed to the user.

The disabled state WILL be used internally for bitmap migration and
bitmap persistence.

Signed-off-by: Fam Zheng 
Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
Reviewed-by: Stefan Hajnoczi 
---
 block.c   | 25 +
 include/block/block.h |  3 +++
 2 files changed, 28 insertions(+)

diff --git a/block.c b/block.c
index 41c5a67..db742a9 100644
--- a/block.c
+++ b/block.c
@@ -54,6 +54,7 @@
 struct BdrvDirtyBitmap {
 HBitmap *bitmap;
 char *name;
+bool disabled;
 QLIST_ENTRY(BdrvDirtyBitmap) list;
 };
 
@@ -5481,10 +5482,16 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs,
 bitmap = g_new0(BdrvDirtyBitmap, 1);
 bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(sector_granularity) - 1);
 bitmap->name = g_strdup(name);
+bitmap->disabled = false;
 QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
 return bitmap;
 }
 
+bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)
+{
+return !bitmap->disabled;
+}
+
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
 {
 BdrvDirtyBitmap *bm, *next;
@@ -5499,6 +5506,16 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, 
BdrvDirtyBitmap *bitmap)
 }
 }
 
+void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+bitmap->disabled = true;
+}
+
+void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+bitmap->disabled = false;
+}
+
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 {
 BdrvDirtyBitmap *bm;
@@ -5563,12 +5580,14 @@ void bdrv_dirty_iter_init(BlockDriverState *bs,
 void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors)
 {
+assert(bdrv_dirty_bitmap_enabled(bitmap));
 hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
 }
 
 void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
  int64_t cur_sector, int nr_sectors)
 {
+assert(bdrv_dirty_bitmap_enabled(bitmap));
 hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
 }
 
@@ -5577,6 +5596,9 @@ static void bdrv_set_dirty(BlockDriverState *bs, int64_t 
cur_sector,
 {
 BdrvDirtyBitmap *bitmap;
 QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
+if (!bdrv_dirty_bitmap_enabled(bitmap)) {
+continue;
+}
 hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
 }
 }
@@ -5586,6 +5608,9 @@ static void bdrv_reset_dirty(BlockDriverState *bs, 
int64_t cur_sector,
 {
 BdrvDirtyBitmap *bitmap;
 QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
+if (!bdrv_dirty_bitmap_enabled(bitmap)) {
+continue;
+}
 hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors);
 }
 }
diff --git a/include/block/block.h b/include/block/block.h
index 493b7c5..029a8a7 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -457,9 +457,12 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState 
*bs,
 const char *name);
 void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap 
*bitmap);
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
+void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
+void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
 uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
 uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
+bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
 int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t 
sector);
 void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
-- 
2.1.0

[Qemu-devel] [PATCH v5 03/21] qmp: Ensure consistent granularity type

[John Snow]

We treat this field with a variety of different types everywhere
in the code. Now it's just uint32_t.

Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Reviewed-by: Stefan Hajnoczi 
---
 block.c   | 11 ++-
 block/mirror.c|  4 ++--
 include/block/block.h |  2 +-
 include/block/block_int.h |  2 +-
 qapi/block-core.json  |  2 +-
 5 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/block.c b/block.c
index 0da35ae..2b609e7 100644
--- a/block.c
+++ b/block.c
@@ -5456,12 +5456,13 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, 
BdrvDirtyBitmap *bitmap)
 }
 
 BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
-  int granularity,
+  uint32_t granularity,
   const char *name,
   Error **errp)
 {
 int64_t bitmap_size;
 BdrvDirtyBitmap *bitmap;
+uint32_t sector_granularity;
 
 assert((granularity & (granularity - 1)) == 0);
 
@@ -5469,8 +5470,8 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs,
 error_setg(errp, "Bitmap already exists: %s", name);
 return NULL;
 }
-granularity >>= BDRV_SECTOR_BITS;
-assert(granularity);
+sector_granularity = granularity >> BDRV_SECTOR_BITS;
+assert(sector_granularity);
 bitmap_size = bdrv_nb_sectors(bs);
 if (bitmap_size < 0) {
 error_setg_errno(errp, -bitmap_size, "could not get length of device");
@@ -5478,7 +5479,7 @@ BdrvDirtyBitmap 
*bdrv_create_dirty_bitmap(BlockDriverState *bs,
 return NULL;
 }
 bitmap = g_new0(BdrvDirtyBitmap, 1);
-bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
+bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(sector_granularity) - 1);
 bitmap->name = g_strdup(name);
 QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
 return bitmap;
@@ -5509,7 +5510,7 @@ BlockDirtyInfoList 
*bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 BlockDirtyInfoList *entry = g_new0(BlockDirtyInfoList, 1);
 info->count = bdrv_get_dirty_count(bs, bm);
 info->granularity =
-((int64_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap));
+((uint32_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap));
 info->has_name = !!bm->name;
 info->name = g_strdup(bm->name);
 entry->value = info;
diff --git a/block/mirror.c b/block/mirror.c
index f073ad7..f8aac33 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -656,7 +656,7 @@ static const BlockJobDriver commit_active_job_driver = {
 
 static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
  const char *replaces,
- int64_t speed, int64_t granularity,
+ int64_t speed, uint32_t granularity,
  int64_t buf_size,
  BlockdevOnError on_source_error,
  BlockdevOnError on_target_error,
@@ -717,7 +717,7 @@ static void mirror_start_job(BlockDriverState *bs, 
BlockDriverState *target,
 
 void mirror_start(BlockDriverState *bs, BlockDriverState *target,
   const char *replaces,
-  int64_t speed, int64_t granularity, int64_t buf_size,
+  int64_t speed, uint32_t granularity, int64_t buf_size,
   MirrorSyncMode mode, BlockdevOnError on_source_error,
   BlockdevOnError on_target_error,
   BlockCompletionFunc *cb,
diff --git a/include/block/block.h b/include/block/block.h
index 05e32f9..5235086 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -450,7 +450,7 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, 
QEMUIOVector *qiov);
 struct HBitmapIter;
 typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
 BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
-  int granularity,
+  uint32_t granularity,
   const char *name,
   Error **errp);
 BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index dccb092..fb9e100 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -590,7 +590,7 @@ void commit_active_start(BlockDriverState *bs, 
BlockDriverState *base,
  */
 void mirror_start(BlockDriverState *bs, BlockDriverState *target,
   const char *replaces,
-  int64_t speed, int64_t granularity, int64_t buf_size,
+  int64_t speed, uint32_t granularity, int64_t buf_size,
   MirrorSyncMode mode, BlockdevOnError on_source_error,
   BlockdevOnError on_target_error,
 

[Qemu-devel] [PATCH v5 00/21] block: transactionless incremental backup series

[John Snow]

I've run out of cheeky jokes for my cover letters.

This patchset enables the in-memory part of the incremental backup
feature, without transactional support.

Support for transactions was separated into a separate series which
is also now available on-list. Getting this portion of the series
committed will help stabilize work on bitmap persistence and bitmap
migration.

Thanks to Fam Zheng for the original versions of this patchset,
And thanks to Max for reviewing 2,395 versions of it since.

===
v5:
===

10: Code has been moved into backup_run_incremental()
'polyrhythm' check is removed,
clusters_per_iter variable is introduced instead.
If the bitmap granularity is larger than the backup granularity,
loop over the backup_do_cow call multiple times.
If the bitmap granularity is smaller, skip the iterator ahead as
we had been doing previously.
14: Only whitespace changes caused by patch 10.
15: Changed my approach for clearing out data for the hbitmap
truncate shrink case, as suggested by Stefan
18: Added a proper timeout mechanism to qmp.pull_event():
  wait=False or wait=0.0 implies non-blocking.
  wait=True implies blocking.
  wait=60.0 implies a 60 second timeout.
  VM.event_wait() now uses a 60 second timeout by default.
19: Many things:
The big picture is to add a set of full backups alongside the
incremental backups created during the test to be able to test
the validity of each incremental at the conclusion of the test
when we can shut the VM down.

To do this, there are two basic changes:
(1) Keep a list of pairs of backup filenames (incremental, reference);
create a full reference backup for every incremental created.
(2) Refactor the backup helper functions a bit.
20: Naming fallout from 19
Added calls to vm.shutdown() and check_backups().
21: NEW, adds granularity tests that cover the changes in patch 10.

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

001/21:[] [--] 'docs: incremental backup documentation'
002/21:[] [--] 'qapi: Add optional field "name" to block dirty bitmap'
003/21:[] [--] 'qmp: Ensure consistent granularity type'
004/21:[] [--] 'qmp: Add block-dirty-bitmap-add and 
block-dirty-bitmap-remove'
005/21:[] [--] 'block: Introduce bdrv_dirty_bitmap_granularity()'
006/21:[] [--] 'hbitmap: cache array lengths'
007/21:[] [--] 'hbitmap: add hbitmap_merge'
008/21:[] [--] 'block: Add bitmap disabled status'
009/21:[] [--] 'block: Add bitmap successors'
010/21:[0092] [FC] 'qmp: Add support of "dirty-bitmap" sync mode for 
drive-backup'
011/21:[] [--] 'qmp: add block-dirty-bitmap-clear'
012/21:[] [--] 'qmp: Add dirty bitmap status field in query-block'
013/21:[] [--] 'block: add BdrvDirtyBitmap documentation'
014/21:[0004] [FC] 'block: Ensure consistent bitmap function prototypes'
015/21:[0012] [FC] 'block: Resize bitmaps on bdrv_truncate'
016/21:[] [--] 'hbitmap: truncate tests'
017/21:[] [--] 'iotests: add invalid input incremental backup tests'
018/21:[0014] [FC] 'iotests: add QMP event waiting queue'
019/21:[0141] [FC] 'iotests: add simple incremental backup case'
020/21:[0005] [FC] 'iotests: add incremental backup failure recovery test'
021/21:[down] 'iotests: add incremental backup granularity tests'

===
v4:
===

04: Some in-line documentation for block_dirty_bitmap_lookup
Changed behavior with respect to aio_context
  (always acquire, release if pbs == null, give to user otherwise)
10: Removed vestigial (currently nop) bdrv_enable_dirty_bitmap call
Kept R-B.
16: Added some comments to test_check_boundary_bits.
Kept R-B.
17: Folded in refactor from "incremental transactions v1" (Poor Kitty)
18: Pulled forward from "incremental transactions v1"
Kept R-B from that series.
19: Folded in refactor from "incremental transactions v1"
Added offset assertions into wait_incremental
20: Removed error tolerance from wait_until_completed, as
these patches no longer make use of that feature.

===
v3:
===

01: Removed enabled/disabled modes information.
Elaborated on events that can occur during error cases.
04: Added an AioContext out parameter to block_dirty_bitmap_lookup.
06: NEW:
Cache the array lengths for hbitmap.
07: hbitmap_merge now uses the cached array lengths.
11: block-dirty-bitmap-clear is edited for the new block_dirty_bitmap_lookup.
12: Removed the "disabled" status, leaving just "Frozen."
15: Moved bdrv_truncate_dirty_bitmap to be static local
Inlined dirty_bitmap_truncate function.
Removed size[] caching into new patch (06, above)
hbitmap_truncate now keeps correct bit population count
hbitmap_truncate now uses hbitmap_reset BEFORE the truncate,
to avoid tricky out-of-bounds usages.
Remove g_

[Qemu-devel] [PATCH v5 06/21] hbitmap: cache array lengths

[John Snow]

As a convenience: between incremental backups, bitmap migrations
and bitmap persistence we seem to need to recalculate these a lot.

Because the lengths are a little bit-twiddly, let's just solidly
cache them and be done with it.

Reviewed-by: Max Reitz 
Signed-off-by: John Snow 
---
 util/hbitmap.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/util/hbitmap.c b/util/hbitmap.c
index ab13971..5b78613 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -90,6 +90,9 @@ struct HBitmap {
  * bitmap will still allocate HBITMAP_LEVELS arrays.
  */
 unsigned long *levels[HBITMAP_LEVELS];
+
+/* The length of each levels[] array. */
+uint64_t sizes[HBITMAP_LEVELS];
 };
 
 /* Advance hbi to the next nonzero word and return it.  hbi->pos
@@ -384,6 +387,7 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity)
 hb->granularity = granularity;
 for (i = HBITMAP_LEVELS; i-- > 0; ) {
 size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
+hb->sizes[i] = size;
 hb->levels[i] = g_new0(unsigned long, size);
 }
 
-- 
2.1.0

[Qemu-devel] [PATCH v5 05/21] block: Introduce bdrv_dirty_bitmap_granularity()

[John Snow]

This returns the granularity (in bytes) of dirty bitmap,
which matches the QMP interface and the existing query
interface.

Signed-off-by: John Snow 
Reviewed-by: Max Reitz 
Reviewed-by: Eric Blake 
Reviewed-by: Stefan Hajnoczi 
---
 block.c   | 8 ++--
 include/block/block.h | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index ebea54d..41c5a67 100644
--- a/block.c
+++ b/block.c
@@ -5509,8 +5509,7 @@ BlockDirtyInfoList 
*bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 BlockDirtyInfo *info = g_new0(BlockDirtyInfo, 1);
 BlockDirtyInfoList *entry = g_new0(BlockDirtyInfoList, 1);
 info->count = bdrv_get_dirty_count(bs, bm);
-info->granularity =
-((uint32_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap));
+info->granularity = bdrv_dirty_bitmap_granularity(bm);
 info->has_name = !!bm->name;
 info->name = g_strdup(bm->name);
 entry->value = info;
@@ -5550,6 +5549,11 @@ uint32_t 
bdrv_get_default_bitmap_granularity(BlockDriverState *bs)
 return granularity;
 }
 
+uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap)
+{
+return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
+}
+
 void bdrv_dirty_iter_init(BlockDriverState *bs,
   BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
 {
diff --git a/include/block/block.h b/include/block/block.h
index 0f014a3..493b7c5 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -459,6 +459,7 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, 
BdrvDirtyBitmap *bitmap);
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
 uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
+uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
 int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t 
sector);
 void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
-- 
2.1.0

Re: [Qemu-devel] [PATCH 0/3] Add support for for GICv2m and MSIs to arm-virt

[Nikolay Nikolaev]

On Thu, Apr 9, 2015 at 12:20 AM, Christoffer Dall
 wrote:
> Now when we have a host generic PCIe controller in the virt board, it
> would be nice to be able to use MSIs so that we can eventually enable
> VHOST with KVM.
>
> With these patches you can use MSIs with TCG and with KVM, but you still
> need some fixes for the mapping of the IRQ index to the GSI number for
> IRQFD to work.  A separate patch series will follow this one to enable
> that.
>
> Tested with KVM on XGene and with TCG by configuring a virtio-pci
> network adapter for the guest and verifying MSIs going through as
> expected.

Christofer, have you measured the network performance difference with KVM.
Probably the next patchseries (with IRQFD) makes bigger difference.

In any case, sharing some numbers will be great.
Thanks.

regards,
Nikolay Nikolaev

>
> Christoffer Dall (3):
>   target-arm: Add GIC phandle to VirtBoardInfo
>   arm_gicv2m: Add GICv2m widget to support MSIs
>   target-arm: Add the GICv2m to the virt board
>
>  hw/arm/virt.c |  69 +++
>  hw/intc/Makefile.objs |   1 +
>  hw/intc/arm_gicv2m.c  | 180 
> ++
>  pixman|   2 +-
>  4 files changed, 237 insertions(+), 15 deletions(-)
>  create mode 100644 hw/intc/arm_gicv2m.c
>
> --
> 2.1.2.330.g565301e.dirty
>
>

[Qemu-devel] [PATCH] translate-all: remove redundant page_find from tb_invalidate_phys_page

[Emilio G. Cota]

The callers have just looked up the page descriptor, so there's no
point in searching again for it.

Signed-off-by: Emilio G. Cota 
---
 translate-all.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/translate-all.c b/translate-all.c
index 11763c6..4d05898 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1246,14 +1246,13 @@ void tb_invalidate_phys_page_fast(tb_page_addr_t start, 
int len)
 }
 }
 
 #if !defined(CONFIG_SOFTMMU)
-static void tb_invalidate_phys_page(tb_page_addr_t addr,
+static void tb_invalidate_phys_page(PageDesc *p, tb_page_addr_t addr,
 uintptr_t pc, void *puc,
 bool locked)
 {
 TranslationBlock *tb;
-PageDesc *p;
 int n;
 #ifdef TARGET_HAS_PRECISE_SMC
 TranslationBlock *current_tb = NULL;
 CPUState *cpu = current_cpu;
@@ -1264,12 +1263,8 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
 int current_flags = 0;
 #endif
 
 addr &= TARGET_PAGE_MASK;
-p = page_find(addr >> TARGET_PAGE_BITS);
-if (!p) {
-return;
-}
 tb = p->first_tb;
 #ifdef TARGET_HAS_PRECISE_SMC
 if (tb && pc != 0) {
 current_tb = tb_find_pc(pc);
@@ -1817,9 +1812,9 @@ void page_set_flags(target_ulong start, target_ulong end, 
int flags)
the code inside.  */
 if (!(p->flags & PAGE_WRITE) &&
 (flags & PAGE_WRITE) &&
 p->first_tb) {
-tb_invalidate_phys_page(addr, 0, NULL, false);
+tb_invalidate_phys_page(p, addr, 0, NULL, false);
 }
 p->flags = flags;
 }
 }
@@ -1911,9 +1906,9 @@ int page_unprotect(target_ulong address, uintptr_t pc, 
void *puc)
 prot |= p->flags;
 
 /* and since the content will be modified, we must invalidate
the corresponding translated code. */
-tb_invalidate_phys_page(addr, pc, puc, true);
+tb_invalidate_phys_page(p, addr, pc, puc, true);
 #ifdef DEBUG_TB_CHECK
 tb_invalidate_check(addr);
 #endif
 }
-- 
1.9.1

Re: [Qemu-devel] [PATCH 0/3] Add support for for GICv2m and MSIs to arm-virt

[Peter Maydell]

On 8 April 2015 at 22:20, Christoffer Dall  wrote:
> Now when we have a host generic PCIe controller in the virt board, it
> would be nice to be able to use MSIs so that we can eventually enable
> VHOST with KVM.
>
> With these patches you can use MSIs with TCG and with KVM, but you still
> need some fixes for the mapping of the IRQ index to the GSI number for
> IRQFD to work.  A separate patch series will follow this one to enable
> that.
>
> Tested with KVM on XGene and with TCG by configuring a virtio-pci
> network adapter for the guest and verifying MSIs going through as
> expected.
>
> Christoffer Dall (3):
>   target-arm: Add GIC phandle to VirtBoardInfo
>   arm_gicv2m: Add GICv2m widget to support MSIs
>   target-arm: Add the GICv2m to the virt board
>
>  hw/arm/virt.c |  69 +++
>  hw/intc/Makefile.objs |   1 +
>  hw/intc/arm_gicv2m.c  | 180 
> ++
>  pixman|   2 +-

Will review properly later, but what's pixman doing in your diffstat? :-)

-- PMM

[Qemu-devel] [PATCH 3/3] target-arm: Add the GICv2m to the virt board

[Christoffer Dall]

Adding the GICv2m to the virt board should allow us to enable MSIs on
the generic PCI host controller, in theory.

Signed-off-by: Christoffer Dall 
---
 hw/arm/virt.c | 47 ++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index e01676a..0c8dae9 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -45,6 +45,7 @@
 #include "hw/pci-host/gpex.h"
 
 #define NUM_VIRTIO_TRANSPORTS 32
+#define NUM_GICV2M_SPIS 64
 
 /* Number of external interrupt lines to configure the GIC with */
 #define NUM_IRQS 128
@@ -71,6 +72,7 @@ enum {
 VIRT_RTC,
 VIRT_FW_CFG,
 VIRT_PCIE,
+VIRT_GIC_V2M,
 };
 
 typedef struct MemMapEntry {
@@ -88,6 +90,7 @@ typedef struct VirtBoardInfo {
 int fdt_size;
 uint32_t clock_phandle;
 uint32_t gic_phandle;
+uint32_t v2m_phandle;
 } VirtBoardInfo;
 
 typedef struct {
@@ -127,6 +130,7 @@ static const MemMapEntry a15memmap[] = {
 /* GIC distributor and CPU interfaces sit inside the CPU peripheral space 
*/
 [VIRT_GIC_DIST] =   { 0x0800, 0x0001 },
 [VIRT_GIC_CPU] ={ 0x0801, 0x0001 },
+[VIRT_GIC_V2M] ={ 0x0802, 0x1000 },
 [VIRT_UART] =   { 0x0900, 0x1000 },
 [VIRT_RTC] ={ 0x0901, 0x1000 },
 [VIRT_FW_CFG] = { 0x0902, 0x000a },
@@ -148,6 +152,7 @@ static const int a15irqmap[] = {
 [VIRT_RTC] = 2,
 [VIRT_PCIE] = 3, /* ... to 6 */
 [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
+[VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
 };
 
 static VirtBoardInfo machines[] = {
@@ -323,9 +328,21 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
 }
 }
 
-static void fdt_add_gic_node(VirtBoardInfo *vbi)
+static void fdt_add_v2m_gic_node(VirtBoardInfo *vbi)
 {
+vbi->v2m_phandle = qemu_fdt_alloc_phandle(vbi->fdt);
+qemu_fdt_add_subnode(vbi->fdt, "/intc/v2m");
+qemu_fdt_setprop_string(vbi->fdt, "/intc/v2m", "compatible",
+"arm,gic-v2m-frame");
+qemu_fdt_setprop(vbi->fdt, "/intc/v2m", "msi-controller", NULL, 0);
+qemu_fdt_setprop_sized_cells(vbi->fdt, "/intc/v2m", "reg",
+ 2, vbi->memmap[VIRT_GIC_V2M].base,
+ 2, vbi->memmap[VIRT_GIC_V2M].size);
+qemu_fdt_setprop_cell(vbi->fdt, "/intc/v2m", "phandle", vbi->v2m_phandle);
+}
 
+static void fdt_add_gic_node(VirtBoardInfo *vbi)
+{
 vbi->gic_phandle = qemu_fdt_alloc_phandle(vbi->fdt);
 qemu_fdt_setprop_cell(vbi->fdt, "/", "interrupt-parent", vbi->gic_phandle);
 
@@ -340,9 +357,31 @@ static void fdt_add_gic_node(VirtBoardInfo *vbi)
  2, vbi->memmap[VIRT_GIC_DIST].size,
  2, vbi->memmap[VIRT_GIC_CPU].base,
  2, vbi->memmap[VIRT_GIC_CPU].size);
+qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#address-cells", 0x2);
+qemu_fdt_setprop_cell(vbi->fdt, "/intc", "#size-cells", 0x2);
+qemu_fdt_setprop(vbi->fdt, "/intc", "ranges", NULL, 0);
 qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", vbi->gic_phandle);
 }
 
+static void create_v2m(VirtBoardInfo *vbi, qemu_irq *pic)
+{
+int i;
+int irq = vbi->irqmap[VIRT_GIC_V2M];
+DeviceState *dev;
+
+dev = qdev_create(NULL, "gicv2m");
+sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vbi->memmap[VIRT_GIC_V2M].base);
+qdev_prop_set_uint32(dev, "base-spi", irq);
+qdev_prop_set_uint32(dev, "num-spi", NUM_GICV2M_SPIS);
+qdev_init_nofail(dev);
+
+for (i = 0; i < NUM_GICV2M_SPIS; i++) {
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]);
+}
+
+fdt_add_v2m_gic_node(vbi);
+}
+
 static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
 {
 /* We create a standalone GIC v2 */
@@ -391,6 +430,8 @@ static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
 }
 
 fdt_add_gic_node(vbi);
+
+create_v2m(vbi, pic);
 }
 
 static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
@@ -699,6 +740,10 @@ static void create_pcie(const VirtBoardInfo *vbi, qemu_irq 
*pic)
 qemu_fdt_setprop_cell(vbi->fdt, nodename, "#size-cells", 2);
 qemu_fdt_setprop_cells(vbi->fdt, nodename, "bus-range", 0,
nr_pcie_buses - 1);
+qemu_fdt_setprop_cells(vbi->fdt, nodename, "bus-range", 0,
+   nr_pcie_buses - 1);
+
+qemu_fdt_setprop_cells(vbi->fdt, nodename, "msi-parent", vbi->v2m_phandle);
 
 qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
  2, base_ecam, 2, size_ecam);
-- 
2.1.2.330.g565301e.dirty

Re: [Qemu-devel] [PATCH] tcg/tcg-op.c: Fix ld/st of 64 bit values on 32-bit bigendian hosts

[Andreas Färber]

Am 08.04.2015 um 21:57 schrieb Peter Maydell:
> Commit 951c6300f7 out-of-lined the 32-bit-host versions of
> tcg_gen_{ld,st}_i64, but in the process it inadvertently changed
> an #ifdef HOST_WORDS_BIGENDIAN to #ifdef TCG_TARGET_WORDS_BIGENDIAN.
> Since the latter doesn't get defined anywhere this meant we always
> took the "LE host" codepath, and stored the two halves of the value
> in the wrong order on BE hosts. This typically breaks any 64-bit
> guest on a 32-bit BE host completely, and will have possibly more
> subtle effects even for 32-bit guests.
> 
> Switch the ifdef back to HOST_WORDS_BIGENDIAN.

Doh, not the first time we've screwed up such an ifdef...

> 
> Signed-off-by: Peter Maydell 
> ---
> I checked that with this fix we no longer fail catastrophically
> on the first insn of the x86-64 BIOS image, but since I don't have
> a convenient way to display the graphical screen from the PPC
> box I have access to I didn't check that it continued to do
> sensible things thereafter. Andreas, could you confirm this fixes
> the breakage you're seeing?

It fixes the observed make check breakage on ppc. i586 still works, too.

Tested-by: Andreas Färber 

I'll check VNC tomorrow. I'm assuming this is for-2.3 material.

Thanks a lot,
Andreas

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

[Qemu-devel] [PATCH 2/3] arm_gicv2m: Add GICv2m widget to support MSIs

[Christoffer Dall]

The ARM GICv2m widget is a little device that handle MSI interrupt
writes to a trigger register and ties them to a range of interrupt lines
wires to the GIC.  It has a few status/id registers and the interrupt wires,
and that's about it.

A board instantiates the device by setting the base SPI number and
number SPIs for the frame.  The base-spi parameter is indexed in the SPI
number space only, so base-spi == 0, means IRQ number 32.  When a device
(the PCI host controller) writes to the trigger register, the payload is
the GIC IRQ number, so we have to subtract 32 from that and then index
into our frame of SPIs.

When instantiating a GICv2m device, tell PCI that we have instantiated
something that can deal with MSIs.  We rely on the board actually wiring
up the GICv2m to the PCI host controller.

Signed-off-by: Christoffer Dall 
---
 hw/intc/Makefile.objs |   1 +
 hw/intc/arm_gicv2m.c  | 180 ++
 2 files changed, 181 insertions(+)
 create mode 100644 hw/intc/arm_gicv2m.c

diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index 843864a..6b63dfc 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -15,6 +15,7 @@ common-obj-$(CONFIG_OPENPIC) += openpic.o
 
 obj-$(CONFIG_APIC) += apic.o apic_common.o
 obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o
+obj-$(CONFIG_ARM_GIC) += arm_gicv2m.o
 obj-$(CONFIG_STELLARIS) += armv7m_nvic.o
 obj-$(CONFIG_EXYNOS4) += exynos4210_gic.o exynos4210_combiner.o
 obj-$(CONFIG_GRLIB) += grlib_irqmp.o
diff --git a/hw/intc/arm_gicv2m.c b/hw/intc/arm_gicv2m.c
new file mode 100644
index 000..a80a16d
--- /dev/null
+++ b/hw/intc/arm_gicv2m.c
@@ -0,0 +1,180 @@
+/*
+ *  GICv2m extension for MSI/MSI-x support with a GICv2-based system
+ *
+ * Copyright (C) 2015 Linaro, All rights reserved.
+ *
+ * Author: Christoffer Dall 
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ */
+
+#include "hw/sysbus.h"
+#include "hw/pci/msi.h"
+
+#define TYPE_GICV2M "gicv2m"
+#define GICV2M(obj) OBJECT_CHECK(GICv2mState, (obj), TYPE_GICV2M)
+
+#define GICV2M_NUM_SPI_MAX 128
+
+#define V2M_MSI_TYPER   0x008
+#define V2M_MSI_SETSPI_NS   0x040
+#define V2M_MSI_IIDR0xFCC
+#define V2M_IIDR0   0xFD0
+
+#define PRODUCT_ID_QEMU 0x51 /* ASCII code Q */
+#define IMPLEMENTER_ARM 0x43b
+
+typedef struct GICv2mState {
+SysBusDevice parent_obj;
+
+MemoryRegion iomem;
+qemu_irq spi[GICV2M_NUM_SPI_MAX];
+
+uint32_t base_spi;
+uint32_t num_spi;
+} GICv2mState;
+
+static void gicv2m_set_irq(void *opaque, int irq)
+{
+GICv2mState *s = (GICv2mState *)opaque;
+
+qemu_irq_pulse(s->spi[irq]);
+}
+
+static uint64_t gicv2m_read(void *opaque, hwaddr offset,
+unsigned size)
+{
+GICv2mState *s = (GICv2mState *)opaque;
+uint32_t val;
+
+if (size != 4) {
+qemu_log_mask(LOG_GUEST_ERROR, "gicv2m_read: bad size %u\n", size);
+return 0;
+}
+
+switch (offset) {
+case V2M_MSI_TYPER:
+val = (s->base_spi + 32) << 16;
+val |= s->num_spi;
+return val;
+case V2M_MSI_IIDR:
+return (PRODUCT_ID_QEMU << 20) | IMPLEMENTER_ARM;
+default:
+if (offset > V2M_IIDR0 && offset <= 0xFFC) {
+return 0;
+}
+
+qemu_log_mask(LOG_GUEST_ERROR,
+  "gicv2m_read: Bad offset %x\n", (int)offset);
+return 0;
+}
+}
+
+static void gicv2m_write(void *opaque, hwaddr offset,
+uint64_t value, unsigned size)
+{
+GICv2mState *s = (GICv2mState *)opaque;
+
+if (size != 4) {
+qemu_log_mask(LOG_GUEST_ERROR, "gicv2m_write: bad size %u\n", size);
+return;
+}
+
+switch (offset) {
+case V2M_MSI_SETSPI_NS: {
+int spi;
+
+spi = (value & 0x3ff) - (s->base_spi + 32);
+if (spi < s->num_spi) {
+gicv2m_set_irq(s, spi);
+}
+return;
+}
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+  "gicv2m_write: Bad offset %x\n", (int)offset);
+return;
+}
+}
+
+static const MemoryRegionOps gicv2m_ops = {
+.read = gicv2m_read,
+.write = gicv2m_write,
+.endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+static void gicv2m_realize(DeviceState *dev, Error **errp)
+{
+GICv2mState *s = GICV2M(dev);
+int i;
+
+if

[Qemu-devel] [PATCH 0/3] Add support for for GICv2m and MSIs to arm-virt

[Christoffer Dall]

Now when we have a host generic PCIe controller in the virt board, it
would be nice to be able to use MSIs so that we can eventually enable
VHOST with KVM.

With these patches you can use MSIs with TCG and with KVM, but you still
need some fixes for the mapping of the IRQ index to the GSI number for
IRQFD to work.  A separate patch series will follow this one to enable
that.

Tested with KVM on XGene and with TCG by configuring a virtio-pci
network adapter for the guest and verifying MSIs going through as
expected.

Christoffer Dall (3):
  target-arm: Add GIC phandle to VirtBoardInfo
  arm_gicv2m: Add GICv2m widget to support MSIs
  target-arm: Add the GICv2m to the virt board

 hw/arm/virt.c |  69 +++
 hw/intc/Makefile.objs |   1 +
 hw/intc/arm_gicv2m.c  | 180 ++
 pixman|   2 +-
 4 files changed, 237 insertions(+), 15 deletions(-)
 create mode 100644 hw/intc/arm_gicv2m.c

-- 
2.1.2.330.g565301e.dirty

[Qemu-devel] [PATCH 1/3] target-arm: Add GIC phandle to VirtBoardInfo

[Christoffer Dall]

Instead of passing the GIC phandle around between functions, add it to
the VirtBoardInfo just like we do for the clock_phandle.  We are about
to add the v2m phandle as well, and it's easier not having to pass
around a bunch of phandles, return multiple values from functions, etc.

Signed-off-by: Christoffer Dall 
---
 hw/arm/virt.c | 26 +++---
 pixman|  2 +-
 2 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index febff22..e01676a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -87,6 +87,7 @@ typedef struct VirtBoardInfo {
 void *fdt;
 int fdt_size;
 uint32_t clock_phandle;
+uint32_t gic_phandle;
 } VirtBoardInfo;
 
 typedef struct {
@@ -322,12 +323,11 @@ static void fdt_add_cpu_nodes(const VirtBoardInfo *vbi)
 }
 }
 
-static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi)
+static void fdt_add_gic_node(VirtBoardInfo *vbi)
 {
-uint32_t gic_phandle;
 
-gic_phandle = qemu_fdt_alloc_phandle(vbi->fdt);
-qemu_fdt_setprop_cell(vbi->fdt, "/", "interrupt-parent", gic_phandle);
+vbi->gic_phandle = qemu_fdt_alloc_phandle(vbi->fdt);
+qemu_fdt_setprop_cell(vbi->fdt, "/", "interrupt-parent", vbi->gic_phandle);
 
 qemu_fdt_add_subnode(vbi->fdt, "/intc");
 /* 'cortex-a15-gic' means 'GIC v2' */
@@ -340,12 +340,10 @@ static uint32_t fdt_add_gic_node(const VirtBoardInfo *vbi)
  2, vbi->memmap[VIRT_GIC_DIST].size,
  2, vbi->memmap[VIRT_GIC_CPU].base,
  2, vbi->memmap[VIRT_GIC_CPU].size);
-qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", gic_phandle);
-
-return gic_phandle;
+qemu_fdt_setprop_cell(vbi->fdt, "/intc", "phandle", vbi->gic_phandle);
 }
 
-static uint32_t create_gic(const VirtBoardInfo *vbi, qemu_irq *pic)
+static void create_gic(VirtBoardInfo *vbi, qemu_irq *pic)
 {
 /* We create a standalone GIC v2 */
 DeviceState *gicdev;
@@ -392,7 +390,7 @@ static uint32_t create_gic(const VirtBoardInfo *vbi, 
qemu_irq *pic)
 pic[i] = qdev_get_gpio_in(gicdev, i);
 }
 
-return fdt_add_gic_node(vbi);
+fdt_add_gic_node(vbi);
 }
 
 static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
@@ -639,8 +637,7 @@ static void create_pcie_irq_map(const VirtBoardInfo *vbi, 
uint32_t gic_phandle,
0x7   /* PCI irq */);
 }
 
-static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic,
-uint32_t gic_phandle)
+static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic)
 {
 hwaddr base = vbi->memmap[VIRT_PCIE].base;
 hwaddr size = vbi->memmap[VIRT_PCIE].size;
@@ -712,7 +709,7 @@ static void create_pcie(const VirtBoardInfo *vbi, qemu_irq 
*pic,
  2, base_mmio, 2, size_mmio);
 
 qemu_fdt_setprop_cell(vbi->fdt, nodename, "#interrupt-cells", 1);
-create_pcie_irq_map(vbi, gic_phandle, irq, nodename);
+create_pcie_irq_map(vbi, vbi->gic_phandle, irq, nodename);
 
 g_free(nodename);
 }
@@ -734,7 +731,6 @@ static void machvirt_init(MachineState *machine)
 MemoryRegion *ram = g_new(MemoryRegion, 1);
 const char *cpu_model = machine->cpu_model;
 VirtBoardInfo *vbi;
-uint32_t gic_phandle;
 char **cpustr;
 
 if (!cpu_model) {
@@ -812,13 +808,13 @@ static void machvirt_init(MachineState *machine)
 
 create_flash(vbi);
 
-gic_phandle = create_gic(vbi, pic);
+create_gic(vbi, pic);
 
 create_uart(vbi, pic);
 
 create_rtc(vbi, pic);
 
-create_pcie(vbi, pic, gic_phandle);
+create_pcie(vbi, pic);
 
 /* Create mmio transports, so the user can create virtio backends
  * (which will be automatically plugged in to the transports). If
diff --git a/pixman b/pixman
index 87eea99..97336fa 16
--- a/pixman
+++ b/pixman
@@ -1 +1 @@
-Subproject commit 87eea99e443b389c978cf37efc52788bf03a0ee0
+Subproject commit 97336fad32acf802003855cd8bd6477fa49a12e3
-- 
2.1.2.330.g565301e.dirty

Re: [Qemu-devel] [PATCH v2] linux-user: Correct TARGET_NR_timerfd to TARGET_NR_timerfd_create.

[Peter Maydell]

On 8 April 2015 at 21:40, Timothy E Baldwin
 wrote:
> Misspelled system call name in macro was causing timerfd_create not
> to be supported for the ARM target.
>
> Signed-off-by: Timothy Edward Baldwin 

Thanks for the respin.

Reviewed-by: Peter Maydell 

thanks
-- PMM

[Qemu-devel] [PATCH v2] linux-user: Correct TARGET_NR_timerfd to TARGET_NR_timerfd_create.

[Timothy E Baldwin]

Misspelled system call name in macro was causing timerfd_create not
to be supported for the ARM target.

Signed-off-by: Timothy Edward Baldwin 
---
 linux-user/arm/syscall_nr.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/arm/syscall_nr.h b/linux-user/arm/syscall_nr.h
index 7d7be7c..53552be 100644
--- a/linux-user/arm/syscall_nr.h
+++ b/linux-user/arm/syscall_nr.h
@@ -354,7 +354,7 @@
 #define TARGET_NR_kexec_load   (347)
 #define TARGET_NR_utimensat(348)
 #define TARGET_NR_signalfd (349)
-#define TARGET_NR_timerfd  (350)
+#define TARGET_NR_timerfd_create   (350)
 #define TARGET_NR_eventfd  (351)
 #define TARGET_NR_fallocate(352)
 #define TARGET_NR_timerfd_settime  (353)
-- 
2.1.4

Re: [Qemu-devel] [PATCH] linux-user: Correct TARGET_NR_timerfd to TARGET_NR_timerfd_create.

[Peter Maydell]

On 8 April 2015 at 21:30, Peter Maydell  wrote:
> On 8 April 2015 at 21:26, Timothy E Baldwin
>  wrote:
>> Misspelled system call name in macro was causing timerfd_create not to be 
>> supported for the ARM target.
>> ---
>>  linux-user/arm/syscall_nr.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/linux-user/arm/syscall_nr.h b/linux-user/arm/syscall_nr.h
>> index 7d7be7c..53552be 100644
>> --- a/linux-user/arm/syscall_nr.h
>> +++ b/linux-user/arm/syscall_nr.h
>> @@ -354,7 +354,7 @@
>>  #define TARGET_NR_kexec_load   (347)
>>  #define TARGET_NR_utimensat(348)
>>  #define TARGET_NR_signalfd (349)
>> -#define TARGET_NR_timerfd  (350)
>> +#define TARGET_NR_timerfd_create   (350)
>>  #define TARGET_NR_eventfd  (351)
>>  #define TARGET_NR_fallocate(352)
>>  #define TARGET_NR_timerfd_settime  (353)
>
> Reviewed-by: Peter Maydell 

...except I've just noticed you haven't added a
Signed-off-by: line in your commit message. We need
this, or we can't apply your patch.

(It would also be good to linewrap your commit messages
at 70-75 columns or so.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] linux-user: Correct TARGET_NR_timerfd to TARGET_NR_timerfd_create.

[Peter Maydell]

On 8 April 2015 at 21:26, Timothy E Baldwin
 wrote:
> Misspelled system call name in macro was causing timerfd_create not to be 
> supported for the ARM target.
> ---
>  linux-user/arm/syscall_nr.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/linux-user/arm/syscall_nr.h b/linux-user/arm/syscall_nr.h
> index 7d7be7c..53552be 100644
> --- a/linux-user/arm/syscall_nr.h
> +++ b/linux-user/arm/syscall_nr.h
> @@ -354,7 +354,7 @@
>  #define TARGET_NR_kexec_load   (347)
>  #define TARGET_NR_utimensat(348)
>  #define TARGET_NR_signalfd (349)
> -#define TARGET_NR_timerfd  (350)
> +#define TARGET_NR_timerfd_create   (350)
>  #define TARGET_NR_eventfd  (351)
>  #define TARGET_NR_fallocate(352)
>  #define TARGET_NR_timerfd_settime  (353)

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] 64-bit build of qemu-system-arm with mingw-w64 not functional

[Liviu Ionescu]

> On 08 Apr 2015, at 09:20, Stefan Weil  wrote:
> 
> Here is my package list (from Debian Jessie):
> 
> ii  binutils-mingw-w64-i686 2.22-8+deb7u2+2+deb7u1amd64
> Cross-binutils for Win32 (x86) using MinGW-w64
> ii  binutils-mingw-w64-x86-64 2.22-8+deb7u2+2+deb7u1amd64
> Cross-binutils for Win64 (x64) using MinGW-w64
> ii  g++-mingw-w64 4.6.3-14+8all  GNU C++ compiler 
> for MinGW-w64
> ii  g++-mingw-w64-i686 4.6.3-14+8amd64GNU C++ 
> compiler for MinGW-w64 targeting Win32
> ii  g++-mingw-w64-x86-64 4.6.3-14+8amd64GNU C++ 
> compiler for MinGW-w64 targeting Win64
> ii  gcc-mingw-w64 4.6.3-14+8all  GNU C compiler 
> for MinGW-w64
> ii  gcc-mingw-w64-base 4.6.3-14+8amd64GNU 
> Compiler Collection for MinGW-w64 (base package)
> ii  gcc-mingw-w64-i686 4.6.3-14+8amd64GNU C 
> compiler for MinGW-w64 targeting Win32
> ii  gcc-mingw-w64-x86-64 4.6.3-14+8amd64GNU C 
> compiler for MinGW-w64 targeting Win64
> ii  gdb-mingw-w64 7.4.1-1.1+5   amd64Cross-debugger 
> for Win32 and Win64 using MinGW-w64
> ii  gdb-mingw-w64-target 7.4.1-1.1+5   all  
> Cross-debugger server for Win32 and Win64 using MinGW-w64
> ii  gtk-mingw-w64-x86-64 3.6.4-20131201-2  all  Converted 
> tgz package
> ii  gtk2.0-mingw-w64-i686 2.24.10-20120208-2all  
> Converted tgz package
> ii  libfdt-mingw-w64-i686 1.4.0-2   all  
> Converted tgz package
> ii  libfdt-mingw-w64-x86-64 1.4.0-2   all  
> Converted tgz package
> ii  libpthreads-mingw-w64 2.9.1+dfsg-1  all  POSIX 
> threads library for 32- and 64-bit Windows
> ii  mingw-w64 2.0.3-1   all  Development 
> environment targetting 32- and 64-bit Windows
> ii  mingw-w64-i686-dev 2.0.3-1   all  Development 
> files for MinGW-w64 targeting Win32
> ii  mingw-w64-tools 2.0.3-1   amd64Development 
> tools for 32- and 64-bit Windows
> ii  mingw-w64-x86-64-dev 2.0.3-1   all  
> Development files for MinGW-w64 targeting Win64
> 

Stefan,

I'm afraid there is a small misunderstanding here, I checked and even without 
upgrading the packages, the Debian 8 (Jessie) does not include the packages you 
are referring above, the actual versions I identified are:

# dpkg -l | grep mingw
ii  binutils-mingw-w64-i6862.25-5+5.2   amd64
Cross-binutils for Win32 (x86) using MinGW-w64
ii  binutils-mingw-w64-x86-64  2.25-5+5.2   amd64
Cross-binutils for Win64 (x64) using MinGW-w64
ii  g++-mingw-w64  4.9.1-19+14.3all  GNU C++ 
compiler for MinGW-w64
ii  g++-mingw-w64-i686 4.9.1-19+14.3amd64GNU C++ 
compiler for MinGW-w64 targeting Win32
ii  g++-mingw-w64-x86-64   4.9.1-19+14.3amd64GNU C++ 
compiler for MinGW-w64 targeting Win64
ii  gcc-mingw-w64  4.9.1-19+14.3all  GNU C 
compiler for MinGW-w64
ii  gcc-mingw-w64-base 4.9.1-19+14.3amd64GNU 
Compiler Collection for MinGW-w64 (base package)
ii  gcc-mingw-w64-i686 4.9.1-19+14.3amd64GNU C 
compiler for MinGW-w64 targeting Win32
ii  gcc-mingw-w64-x86-64   4.9.1-19+14.3amd64GNU C 
compiler for MinGW-w64 targeting Win64
ii  gdb-mingw-w64  7.6.2-1.1+9  amd64
Cross-debugger for Win32 and Win64 using MinGW-w64
ii  gdb-mingw-w64-target   7.6.2-1.1+9  all  
Cross-debugger server for Win32 and Win64 using MinGW-w64
ii  gfortran-mingw-w64 4.9.1-19+14.3all  GNU 
Fortran compiler for MinGW-w64
ii  gfortran-mingw-w64-i6864.9.1-19+14.3amd64GNU 
Fortran compiler for MinGW-w64 targeting Win32
ii  gfortran-mingw-w64-x86-64  4.9.1-19+14.3amd64GNU 
Fortran compiler for MinGW-w64 targeting Win64
ii  gnat-mingw-w64 4.9.1-3+14   all  GNU Ada 
compiler for MinGW-w64
ii  gnat-mingw-w64-base4.9.1-3+14   amd64GNU Ada 
compiler for MinGW-w64 (base package)
ii  gnat-mingw-w64-i6864.9.1-3+14   amd64GNU Ada 
compiler for MinGW-w64 targeting Win32
ii  gnat-mingw-w64-x86-64  4.9.1-3+14   amd64GNU Ada 
compiler for MinGW-w64 targeting Win64
ii  mingw-w64  3.2.0-2  all  
Development environment targeting 32- and 64-bit Windows
ii  mingw-w64-common   3.2.0-2  all  Common 
files for Mingw-w64
ii  mingw-w64-i686-dev 3.2.0-2  all  
Development f

[Qemu-devel] [PATCH] linux-user: Correct TARGET_NR_timerfd to TARGET_NR_timerfd_create.

[Timothy E Baldwin]

Misspelled system call name in macro was causing timerfd_create not to be 
supported for the ARM target.
---
 linux-user/arm/syscall_nr.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/arm/syscall_nr.h b/linux-user/arm/syscall_nr.h
index 7d7be7c..53552be 100644
--- a/linux-user/arm/syscall_nr.h
+++ b/linux-user/arm/syscall_nr.h
@@ -354,7 +354,7 @@
 #define TARGET_NR_kexec_load   (347)
 #define TARGET_NR_utimensat(348)
 #define TARGET_NR_signalfd (349)
-#define TARGET_NR_timerfd  (350)
+#define TARGET_NR_timerfd_create   (350)
 #define TARGET_NR_eventfd  (351)
 #define TARGET_NR_fallocate(352)
 #define TARGET_NR_timerfd_settime  (353)
-- 
2.1.4

Re: [Qemu-devel] [PATCH] qga/commands-posix.c: Use correct types with g_base64_decode()

[Michael Roth]

Quoting Peter Maydell (2015-04-08 15:02:49)
> The second argument of g_base64_decode() is a 'gsize *', not a
> 'size_t *'. Some compilation environments (like building 32-bit PPC
> binaries on a PPC64 system) will complain about the mismatch:
> 
>   CCqga/commands-posix.o
> qga/commands-posix.c: In function 'qmp_guest_set_user_password':
> qga/commands-posix.c:1908:5: error: passing argument 2 of 'g_base64_decode' 
> from incompatible pointer type [-Werror]
> In file included from /usr/include/glib-2.0/glib.h:37:0,
>  from qga/commands-posix.c:14:
> /usr/include/glib-2.0/glib/gbase64.h:49:9: note: expected ‘gsize *’ but 
> argument is of type ‘size_t *’
> 
> (We previously fixed errors of this type in commit 3d1bba20.)
> 
> Signed-off-by: Peter Maydell 

Reviewed-by: Michael Roth 

Thanks for the catch/patch! Assuming you're applying directly, but otherwise
let me know and I'll send a pull within the hour.

> ---
>  qga/commands-posix.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index ba8de62..9fde348 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -1901,7 +1901,7 @@ void qmp_guest_set_user_password(const char *username,
>  int status;
>  int datafd[2] = { -1, -1 };
>  char *rawpasswddata = NULL;
> -size_t rawpasswdlen;
> +gsize rawpasswdlen;
>  char *chpasswddata = NULL;
>  size_t chpasswdlen;
> 
> -- 
> 1.9.1

[Qemu-devel] [PATCH] qga/commands-posix.c: Use correct types with g_base64_decode()

[Peter Maydell]

The second argument of g_base64_decode() is a 'gsize *', not a
'size_t *'. Some compilation environments (like building 32-bit PPC
binaries on a PPC64 system) will complain about the mismatch:

  CCqga/commands-posix.o
qga/commands-posix.c: In function 'qmp_guest_set_user_password':
qga/commands-posix.c:1908:5: error: passing argument 2 of 'g_base64_decode' 
from incompatible pointer type [-Werror]
In file included from /usr/include/glib-2.0/glib.h:37:0,
 from qga/commands-posix.c:14:
/usr/include/glib-2.0/glib/gbase64.h:49:9: note: expected ‘gsize *’ but 
argument is of type ‘size_t *’

(We previously fixed errors of this type in commit 3d1bba20.)

Signed-off-by: Peter Maydell 
---
 qga/commands-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index ba8de62..9fde348 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1901,7 +1901,7 @@ void qmp_guest_set_user_password(const char *username,
 int status;
 int datafd[2] = { -1, -1 };
 char *rawpasswddata = NULL;
-size_t rawpasswdlen;
+gsize rawpasswdlen;
 char *chpasswddata = NULL;
 size_t chpasswdlen;
 
-- 
1.9.1

[Qemu-devel] [PATCH] tcg/tcg-op.c: Fix ld/st of 64 bit values on 32-bit bigendian hosts

[Peter Maydell]

Commit 951c6300f7 out-of-lined the 32-bit-host versions of
tcg_gen_{ld,st}_i64, but in the process it inadvertently changed
an #ifdef HOST_WORDS_BIGENDIAN to #ifdef TCG_TARGET_WORDS_BIGENDIAN.
Since the latter doesn't get defined anywhere this meant we always
took the "LE host" codepath, and stored the two halves of the value
in the wrong order on BE hosts. This typically breaks any 64-bit
guest on a 32-bit BE host completely, and will have possibly more
subtle effects even for 32-bit guests.

Switch the ifdef back to HOST_WORDS_BIGENDIAN.

Signed-off-by: Peter Maydell 
---
I checked that with this fix we no longer fail catastrophically
on the first insn of the x86-64 BIOS image, but since I don't have
a convenient way to display the graphical screen from the PPC
box I have access to I didn't check that it continued to do
sensible things thereafter. Andreas, could you confirm this fixes
the breakage you're seeing?

 tcg/tcg-op.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index f7a2767..2b6be75 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -808,7 +808,7 @@ void tcg_gen_ld_i64(TCGv_i64 ret, TCGv_ptr arg2, 
tcg_target_long offset)
 {
 /* Since arg2 and ret have different types,
they cannot be the same temporary */
-#ifdef TCG_TARGET_WORDS_BIGENDIAN
+#ifdef HOST_WORDS_BIGENDIAN
 tcg_gen_ld_i32(TCGV_HIGH(ret), arg2, offset);
 tcg_gen_ld_i32(TCGV_LOW(ret), arg2, offset + 4);
 #else
@@ -819,7 +819,7 @@ void tcg_gen_ld_i64(TCGv_i64 ret, TCGv_ptr arg2, 
tcg_target_long offset)
 
 void tcg_gen_st_i64(TCGv_i64 arg1, TCGv_ptr arg2, tcg_target_long offset)
 {
-#ifdef TCG_TARGET_WORDS_BIGENDIAN
+#ifdef HOST_WORDS_BIGENDIAN
 tcg_gen_st_i32(TCGV_HIGH(arg1), arg2, offset);
 tcg_gen_st_i32(TCGV_LOW(arg1), arg2, offset + 4);
 #else
-- 
1.9.1

Re: [Qemu-devel] bios-tables-test failing for x86_64 on ppc

[Peter Maydell]

On 8 April 2015 at 20:35, Peter Maydell  wrote:
> Looking at the generated TCG, I suspect the problem
> is that tcg_gen_st_i64() isn't writing the two i32
> input values in the right order for the case where the
> host is a 32-bit bigendian system...

Yep. Commit 951c6300f7 out-of-lined the 32-bit-host
versions of tcg_gen_{ld,st}_i64, and in the process
inadvertently changed an #ifdef HOST_WORDS_BIGENDIAN
to #ifdef TCG_TARGET_WORDS_BIGENDIAN. Since the latter
doesn't get defined anywhere we always get the LE host
behaviour even on BE hosts. Switching it back to
HOST_WORDS_BIGENDIAN fixes this. Patch coming up.

-- PMM

Re: [Qemu-devel] [Qemu-block] [PATCH] block/iscsi: handle zero events from iscsi_which_events

[Peter Lieven]

Am 08.04.2015 um 21:22 schrieb Stefan Hajnoczi:
> On Wed, Apr 8, 2015 at 6:08 PM, ronnie sahlberg
>  wrote:
>> The nice part with the current patch of Peter is that qemu and
>> libiscsi can be upgraded/downgraded independently.
> That's fine for avoiding hassles for existing apps, like QEMU, and I'm
> happy to merge the patch.
>
> For the library's API design it would be return the timeout so that
> new applications can avoid polling, but that's just a suggestion.

You are right. Paolo also brought up this idea, but as Ronnie said the
idea was not to bump the API version. The whole async reconnect stuff
works without a change in the client.

But polling every 250ms is magnitutes better than having a hung Qemu :-)
If you feel 250ms is too often we could also go to 1000ms. The timeout
we are talking about is in the order of 1-30 seconds. It only happens when
a reconnect is initiated either bei Qemu (due to NOP timeout) or libiscsi 
(protocol,
socket error etc.) and the first reconnect try is not successful.

Another idea I had was to encode the timeout in the return value of 
iscsi_which_events.
This would work with all Qemu versions, but I do not know how poll reacts if it 
gets
unknown events passed. So all users other than Qemu that pass the result of 
iscsi_which_events
directly to poll would need to mask the result. Thats not a problem for the 
tools provided with libiscsi,
but I do not know how much other real users except Qemu are out there?

Peter

Re: [Qemu-devel] bios-tables-test failing for x86_64 on ppc

[Peter Maydell]

On 8 April 2015 at 19:03, Andreas Färber  wrote:
> I've resorted to setting up a ppc chroot on ppc64, and I can reproduce
> that qemu-system-i386 shows SeaBIOS okay, but qemu-system-x86_64
> immediately aborts with the quoted error.

Yeah, I can repro on the gcc compile farm's fedora ppc64 box.
(I didn't need to use a chroot, it was enough to pass configure
"-cpu ppc" and then it uses -m32 in ldflags and cflags.)

Looks like we fail on the first insn:

0xfff0:  ljmp   $0xf000,$0xe05b

should take us to
0x000fe05b:  cmpl   $0x0,%cs:0x6ac8

but on ppc32 we end up at:
0x000fe05b:  add%al,(%bx,%si)

and then spend a long time galloping through page upon
page of "add%al,(%bx,%si)" before finally falling
off the end and aborting.

Looking at the generated TCG, I suspect the problem
is that tcg_gen_st_i64() isn't writing the two i32
input values in the right order for the case where the
host is a 32-bit bigendian system...

-- PMM

Re: [Qemu-devel] [Qemu-block] [PATCH] block/iscsi: handle zero events from iscsi_which_events

[Stefan Hajnoczi]

On Wed, Apr 8, 2015 at 6:08 PM, ronnie sahlberg
 wrote:
> The nice part with the current patch of Peter is that qemu and
> libiscsi can be upgraded/downgraded independently.

That's fine for avoiding hassles for existing apps, like QEMU, and I'm
happy to merge the patch.

For the library's API design it would be return the timeout so that
new applications can avoid polling, but that's just a suggestion.

Stefan

[Qemu-devel] [PATCH v2 4/4] scripts: x86-cpu-model-dump script

[Eduardo Habkost]

This is an example script that can be used to help generate a config
file that will reproduce a given CPU model from QEMU. The generated
config file can be loaded using "-readconfig" to make QEMU create CPUs
that will look exactly like the one used when cpu-model-dump was run.

A cpu-model-dump-selftest script is also provided, to help ensure that
the output of cpu-model-dump will produce the same config when run under
cpu-model-dump again.

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* Use "cpuid-" prefix instead of "feat-"
* Exit earlier if QEMU fails
* Exit code of the script will match QEMU or diff exit code
---
 scripts/x86-cpu-model-dump  | 221 
 scripts/x86-cpu-model-dump-selftest |  41 +++
 2 files changed, 262 insertions(+)
 create mode 100755 scripts/x86-cpu-model-dump
 create mode 100755 scripts/x86-cpu-model-dump-selftest

diff --git a/scripts/x86-cpu-model-dump b/scripts/x86-cpu-model-dump
new file mode 100755
index 000..38812ab
--- /dev/null
+++ b/scripts/x86-cpu-model-dump
@@ -0,0 +1,221 @@
+#!/usr/bin/env python2.7
+#
+# Script to dump CPU model information as a QEMU config file that can be loaded
+# using -readconfig
+#
+# Author: Eduardo Habkost 
+#
+# Copyright (c) 2015 Red Hat Inc.
+#
+# 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.
+
+
+import sys, os, signal, tempfile, re
+import xml.etree.ElementTree
+
+# Allow us to load the qmp/qmp.py module:
+sys.path.append(os.path.join(os.path.dirname(sys.argv[0]), 'qmp'))
+import qmp
+
+CPU_PATH = '/machine/icc-bridge/icc/child[0]'
+RE_PROPS = 
re.compile('x?level2?|vendor|family|model|stepping|cpuid-.*|model-id')
+CPU_MAP = '/usr/share/libvirt/cpu_map.xml'
+
+# features that may not be on cpu_map.xml:
+KNOWN_FEAT_NAMES = [
+(0x4001,0,'eax', [
+"kvmclock", "kvm-nopiodelay", "kvm-mmu", "kvmclock",
+"kvm-asyncpf", "kvm-steal-time", "kvm-pv-eoi", "kvm-pv-unhalt",
+None, None, None, None,
+None, None, None, None,
+None, None, None, None,
+None, None, None, None,
+"kvmclock-stable-bit", None, None, None,
+None, None, None, None,
+]),
+(0xd,1,'eax',[
+"xsaveopt", "xsavec", "xgetbv1", "xsaves",
+]),
+# CPU feature aliases don't have properties, add some special feature
+# names telling the script to ignore them:
+(0x8001,0,'edx',[
+"fpu-ALIAS", "vme-ALIAS", "de-ALIAS", "pse-ALIAS",
+"tsc-ALIAS", "msr-ALIAS", "pae-ALIAS", "mce-ALIAS",
+"cx8-ALIAS", "apic-ALIAS", None, None,
+"mtrr-ALIAS", "pge-ALIAS", "mca-ALIAS", "cmov-ALIAS",
+"pat-ALIAS", "pse36-ALIAS", None, None,
+None, None, None, "mmx-ALIAS",
+"fxsr-ALIAS", None, None, None,
+None, None, None, None,
+])
+]
+
+def value_to_string(ptype, v):
+"""Convert property value to string parseable by -global"""
+if ptype == "bool":
+return v and "on" or "off"
+elif ptype == 'string':
+return v
+elif ptype.startswith('int') or ptype.startswith('uint'):
+return str(v)
+else:
+raise Exception("Unsupported property type: %s", ptype)
+
+def load_feat_names(cpu_map):
+"""Load feature names from libvirt cpu_map.xml"""
+cpumap = xml.etree.ElementTree.parse(cpu_map)
+feat_names = {}
+
+for function,index,reg,names in KNOWN_FEAT_NAMES:
+for bitnr,name in enumerate(names):
+if name:
+feat_names[(function,index,reg,bitnr)] = name
+
+for f in cpumap.getroot().findall("./arch[@name='x86']/feature"):
+fname = f.attrib['name']
+for cpuid in f.findall('cpuid'):
+function=int(cpuid.attrib['function'], 0)
+index = 0
+for reg in 'abcd':
+regname = 'e%sx' % (reg)
+if regname in cpuid.attrib:
+v = int(cpuid.attrib[regname], 0)
+for bitnr in range(32):
+

[Qemu-devel] [PATCH v2 1/4] target-i386: Make "level" and "xlevel" properties static

[Eduardo Habkost]

Static properties require only 1 line of code, much simpler than the
existing code that requires writing new getters/setters.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 40 ++--
 1 file changed, 2 insertions(+), 38 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 03b33cf..2bbf01d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1618,38 +1618,6 @@ static void x86_cpuid_version_set_stepping(Object *obj, 
Visitor *v,
 env->cpuid_version |= value & 0xf;
 }
 
-static void x86_cpuid_get_level(Object *obj, Visitor *v, void *opaque,
-const char *name, Error **errp)
-{
-X86CPU *cpu = X86_CPU(obj);
-
-visit_type_uint32(v, &cpu->env.cpuid_level, name, errp);
-}
-
-static void x86_cpuid_set_level(Object *obj, Visitor *v, void *opaque,
-const char *name, Error **errp)
-{
-X86CPU *cpu = X86_CPU(obj);
-
-visit_type_uint32(v, &cpu->env.cpuid_level, name, errp);
-}
-
-static void x86_cpuid_get_xlevel(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
-{
-X86CPU *cpu = X86_CPU(obj);
-
-visit_type_uint32(v, &cpu->env.cpuid_xlevel, name, errp);
-}
-
-static void x86_cpuid_set_xlevel(Object *obj, Visitor *v, void *opaque,
- const char *name, Error **errp)
-{
-X86CPU *cpu = X86_CPU(obj);
-
-visit_type_uint32(v, &cpu->env.cpuid_xlevel, name, errp);
-}
-
 static char *x86_cpuid_get_vendor(Object *obj, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
@@ -2900,12 +2868,6 @@ static void x86_cpu_initfn(Object *obj)
 object_property_add(obj, "stepping", "int",
 x86_cpuid_version_get_stepping,
 x86_cpuid_version_set_stepping, NULL, NULL, NULL);
-object_property_add(obj, "level", "int",
-x86_cpuid_get_level,
-x86_cpuid_set_level, NULL, NULL, NULL);
-object_property_add(obj, "xlevel", "int",
-x86_cpuid_get_xlevel,
-x86_cpuid_set_xlevel, NULL, NULL, NULL);
 object_property_add_str(obj, "vendor",
 x86_cpuid_get_vendor,
 x86_cpuid_set_vendor, NULL);
@@ -2998,6 +2960,8 @@ static Property x86_cpu_properties[] = {
 DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, false),
 DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
 DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
+DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
+DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
 DEFINE_PROP_END_OF_LIST()
 };
 
-- 
2.1.0

[Qemu-devel] [PATCH v2 2/4] target-i386: X86CPU::xlevel2 QOM property

[Eduardo Habkost]

We already have "level" and "xlevel", only "xlevel2" is missing.

Signed-off-by: Eduardo Habkost 
Reviewed-by: Paolo Bonzini 
---
 target-i386/cpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 2bbf01d..e657f10 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2077,7 +2077,7 @@ static void x86_cpu_load_def(X86CPU *cpu, 
X86CPUDefinition *def, Error **errp)
 object_property_set_int(OBJECT(cpu), def->model, "model", errp);
 object_property_set_int(OBJECT(cpu), def->stepping, "stepping", errp);
 object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", errp);
-env->cpuid_xlevel2 = def->xlevel2;
+object_property_set_int(OBJECT(cpu), def->xlevel2, "xlevel2", errp);
 cpu->cache_info_passthrough = def->cache_info_passthrough;
 object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
 for (w = 0; w < FEATURE_WORDS; w++) {
@@ -2962,6 +2962,7 @@ static Property x86_cpu_properties[] = {
 DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
 DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
 DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
+DEFINE_PROP_UINT32("xlevel2", X86CPU, env.cpuid_xlevel2, 0),
 DEFINE_PROP_END_OF_LIST()
 };
 
-- 
2.1.0

[Qemu-devel] [PATCH v2 3/4] target-i386: Register QOM properties for feature flags

[Eduardo Habkost]

This uses the feature name arrays to register "feat-*" QOM properties
for feature flags. This simply adds the properties so they can be
configured using -global, but doesn't change x86_cpu_parse_featurestr()
to use them yet.

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
* Use "cpuid-" prefix instead of "feat-"
* Register release function for property
* Convert '_' to '-' on feature name before registering property
* Add dev->realized check to property setter
---
 target-i386/cpu.c | 113 ++
 1 file changed, 113 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e657f10..7099027 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2848,12 +2848,118 @@ out:
 }
 }
 
+typedef struct FeatureProperty {
+FeatureWord word;
+uint32_t mask;
+} FeatureProperty;
+
+
+static void x86_cpu_get_feature_prop(Object *obj,
+ struct Visitor *v,
+ void *opaque,
+ const char *name,
+ Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+FeatureProperty *fp = opaque;
+bool value = (env->features[fp->word] & fp->mask) == fp->mask;
+visit_type_bool(v, &value, name, errp);
+}
+
+static void x86_cpu_set_feature_prop(Object *obj,
+ struct Visitor *v,
+ void *opaque,
+ const char *name,
+ Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+DeviceState *dev = DEVICE(obj);
+CPUX86State *env = &cpu->env;
+FeatureProperty *fp = opaque;
+bool value;
+
+if (dev->realized) {
+qdev_prop_set_after_realize(dev, name, errp);
+return;
+}
+
+visit_type_bool(v, &value, name, errp);
+if (value) {
+env->features[fp->word] |= fp->mask;
+} else {
+env->features[fp->word] &= ~fp->mask;
+}
+}
+
+static void x86_cpu_release_feature_prop(Object *obj, const char *name,
+ void *opaque)
+{
+FeatureProperty *prop = opaque;
+g_free(prop);
+}
+
+/* Register a boolean feature-bits property.
+ * If mask has multiple bits, all must be set for the property to return true.
+ * The same property name can be registered multiple times to make it affect
+ * multiple bits in the same FeatureWord.
+ */
+static void x86_cpu_register_feature_prop(X86CPU *cpu,
+  const char *prop_name,
+  FeatureWord w,
+  uint32_t mask)
+{
+FeatureProperty *fp;
+ObjectProperty *op;
+op = object_property_find(OBJECT(cpu), prop_name, NULL);
+if (op) {
+fp = op->opaque;
+assert(fp->word == w);
+fp->mask |= mask;
+} else {
+fp = g_new0(FeatureProperty, 1);
+fp->word = w;
+fp->mask = mask;
+object_property_add(OBJECT(cpu), prop_name, "bool",
+x86_cpu_get_feature_prop,
+x86_cpu_set_feature_prop,
+x86_cpu_release_feature_prop, fp, &error_abort);
+}
+}
+
+static void x86_cpu_register_feature_bit_props(X86CPU *cpu,
+   FeatureWord w,
+   int bit)
+{
+int i;
+char **names;
+FeatureWordInfo *fi = &feature_word_info[w];
+
+if (!fi->feat_names) {
+return;
+}
+if (!fi->feat_names[bit]) {
+return;
+}
+
+names = g_strsplit(fi->feat_names[bit], "|", 0);
+for (i = 0; names[i]; i++) {
+char *feat_name = names[i];
+feat2prop(feat_name);
+char *prop_name = g_strdup_printf("cpuid-%s", feat_name);
+x86_cpu_register_feature_prop(cpu, prop_name, w, (1UL << bit));
+g_free(prop_name);
+}
+g_strfreev(names);
+}
+
 static void x86_cpu_initfn(Object *obj)
 {
 CPUState *cs = CPU(obj);
 X86CPU *cpu = X86_CPU(obj);
 X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
 CPUX86State *env = &cpu->env;
+FeatureWord w;
 static int inited;
 
 cs->env_ptr = env;
@@ -2894,6 +3000,13 @@ static void x86_cpu_initfn(Object *obj)
 cpu->apic_id = -1;
 #endif
 
+for (w = 0; w < FEATURE_WORDS; w++) {
+int bit;
+for (bit = 0; bit < 32; bit++) {
+x86_cpu_register_feature_bit_props(cpu, w, bit);
+}
+}
+
 x86_cpu_load_def(cpu, xcc->cpu_def, &error_abort);
 
 /* init various static tables used in TCG mode */
-- 
2.1.0

[Qemu-devel] [PATCH v2 0/4] target-i386: Feature properties, sample script for -global/-readconfig

[Eduardo Habkost]

This series adds feature flag properties to X86CPU, and adds a sample script to
demonstrate how the new properties can be used to give management software more
flexibility to control CPU features using -global and -readconfig.

While at it, simplify the code for "level" and "xlevel" properties, and add a
"xlevel2" property, that was still missing.

This series doesn't change x86_cpu_parse_featurestr() to use the new properties
yet, but that will be possible in the future (we just need to be careful about
the ordering requirements of the current host_features code).

Changes v1 -> v2:
* Use "cpuid-" prefix
* Register release function for properties
* Don't remove underscores from feature names on the feature array yet
  * Remove underscores on x86_cpu_register_feature_prop() instead,
so we don't need to touch x86_cpu_parse_featurestr() by now
* Make x86-cpu-model-dump return a reasonable exit code
* Add dev->realized check to property setter

Eduardo Habkost (4):
  target-i386: Make "level" and "xlevel" properties static
  target-i386: X86CPU::xlevel2 QOM property
  target-i386: Register QOM properties for feature flags
  scripts: x86-cpu-model-dump script

 scripts/x86-cpu-model-dump  | 221 
 scripts/x86-cpu-model-dump-selftest |  41 +++
 target-i386/cpu.c   | 156 ++---
 3 files changed, 379 insertions(+), 39 deletions(-)
 create mode 100755 scripts/x86-cpu-model-dump
 create mode 100755 scripts/x86-cpu-model-dump-selftest

-- 
2.1.0

Re: [Qemu-devel] [PATCH 01/10] qga: add guest-set-user-password command

[Peter Maydell]

On 17 February 2015 at 22:40, Michael Roth  wrote:
> From: "Daniel P. Berrange" 
>
> Add a new 'guest-set-user-password' command for changing the password

> +void qmp_guest_set_user_password(const char *username,
> + const char *password,
> + bool crypted,
> + Error **errp)
> +{
> +Error *local_err = NULL;
> +char *passwd_path = NULL;
> +pid_t pid;
> +int status;
> +int datafd[2] = { -1, -1 };
> +char *rawpasswddata = NULL;
> +size_t rawpasswdlen;
> +char *chpasswddata = NULL;
> +size_t chpasswdlen;
> +
> +rawpasswddata = (char *)g_base64_decode(password, &rawpasswdlen);

Hi. This seems to break certain kinds of compilation setups
(the one I noticed was a -m32 compile to build 32-bit PPC
binaries on a 64-bit PPC Fedora box):

  CCqga/commands-posix.o
/home/pm215/qemu/qga/commands-posix.c: In function
‘qmp_guest_set_user_password’:
/home/pm215/qemu/qga/commands-posix.c:1908:5: error: passing argument
2 of ‘g_base64_decode’ from incompatible pointer type [-Werror]
In file included from /usr/include/glib-2.0/glib.h:37:0,
 from /home/pm215/qemu/qga/commands-posix.c:14:
/usr/include/glib-2.0/glib/gbase64.h:49:9: note: expected ‘gsize *’
but argument is of type ‘size_t *’

It looks like you can't just assume that size_t and gsize
are the same thing. Compare commit 3d1bba20 which fixed some
previous instances of this.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 07/20] hw/arm/virt-acpi-build: Generate FADT table and update ACPI headers

[Michael S. Tsirkin]

On Fri, Apr 03, 2015 at 06:03:39PM +0800, Shannon Zhao wrote:
> @@ -135,6 +138,43 @@ struct AcpiFadtDescriptorRev1
>  } QEMU_PACKED;
>  typedef struct AcpiFadtDescriptorRev1 AcpiFadtDescriptorRev1;
>  
> +struct acpi_generic_address {
> +uint8_t space_id;/* Address space where struct or register 
> exists */
> +uint8_t bit_width;   /* Size in bits of given register */
> +uint8_t bit_offset;  /* Bit offset within the register */
> +uint8_t access_width;/* Minimum Access size (ACPI 3.0) */
> +uint64_t address;/* 64-bit address of struct or register */
> +} QEMU_PACKED;

Pls use standard QEMU style for structs.
There are more like this in the patchset, pls find and fix them.


> +
> +struct AcpiFadtDescriptorRev5_1 {
> +ACPI_FADT_COMMON_DEF
> +uint16_t boot_flags; /* IA-PC Boot Architecture Flags (see below for 
> individual flags) */
> +uint8_t reserved;/* Reserved, must be zero */
> +uint32_t flags;  /* Miscellaneous flag bits (see below for 
> individual flags) */
> +struct acpi_generic_address reset_register; /* 64-bit address of the 
> Reset register */
> +uint8_t reset_value; /* Value to write to the reset_register port to 
> reset the system */
> +uint16_t arm_boot_flags; /* ARM-Specific Boot Flags (see below for 
> individual flags) (ACPI 5.1) */
> +uint8_t minor_revision;  /* FADT Minor Revision (ACPI 5.1) */
> +uint64_t Xfacs;  /* 64-bit physical address of FACS */
> +uint64_t Xdsdt;  /* 64-bit physical address of DSDT */
> +struct acpi_generic_address xpm1a_event_block;  /* 64-bit Extended Power 
> Mgt 1a Event Reg Blk address */
> +struct acpi_generic_address xpm1b_event_block;  /* 64-bit Extended Power 
> Mgt 1b Event Reg Blk address */
> +struct acpi_generic_address xpm1a_control_block;/* 64-bit Extended 
> Power Mgt 1a Control Reg Blk address */
> +struct acpi_generic_address xpm1b_control_block;/* 64-bit Extended 
> Power Mgt 1b Control Reg Blk address */
> +struct acpi_generic_address xpm2_control_block; /* 64-bit Extended Power 
> Mgt 2 Control Reg Blk address */
> +struct acpi_generic_address xpm_timer_block;/* 64-bit Extended Power 
> Mgt Timer Ctrl Reg Blk address */
> +struct acpi_generic_address xgpe0_block;/* 64-bit Extended General 
> Purpose Event 0 Reg Blk address */
> +struct acpi_generic_address xgpe1_block;/* 64-bit Extended General 
> Purpose Event 1 Reg Blk address */
> +struct acpi_generic_address sleep_control;  /* 64-bit Sleep Control 
> register (ACPI 5.0) */
> +struct acpi_generic_address sleep_status;   /* 64-bit Sleep Status 
> register (ACPI 5.0) */
> +} QEMU_PACKED;

empty line missing.

> +typedef struct AcpiFadtDescriptorRev5_1 AcpiFadtDescriptorRev5_1;
> +
> +enum {
> +ACPI_FADT_ARM_USE_PSCI_G_0_2,
> +ACPI_FADT_ARM_PSCI_USE_HVC,
> +};

These are part of tables, are they not?
Pls add = 0, = 1, so we don't change them by mistake.

> +
>  /*
>   * ACPI 1.0 Root System Description Table (RSDT)
>   */
> -- 
> 2.0.4
> 

Re: [Qemu-devel] [PATCH v4 00/20] Generate ACPI v5.1 tables and expose it to guest over fw_cfg on ARM

[Michael S. Tsirkin]

On Tue, Apr 07, 2015 at 03:35:57PM +0200, Igor Mammedov wrote:
> On Tue, 7 Apr 2015 13:07:31 +0100
> Peter Maydell  wrote:
> 
> > On 7 April 2015 at 03:43, Shannon Zhao 
> > wrote:
> > > The ACPI table entry:
> > > Method (_CBA, 0, NotSerialized)  // _CBA: Configuration
> > > Base Address {
> > > Return (0x3F00)
> > > }
> > > Method (_CRS, 0, NotSerialized)  // _CRS: Current
> > > Resource Settings {
> > > Name (RBUF, ResourceTemplate ()
> > > {
> > > WordBusNumber (ResourceProducer, MinFixed,
> > > MaxFixed, PosDecode, 0x, // Granularity
> > > 0x, // Range Minimum
> > > 0x000F, // Range Maximum
> > > 0x, // Translation Offset
> > > 0x0010, // Length
> > > ,, )
> > > DWordMemory (ResourceProducer, PosDecode,
> > > MinFixed, MaxFixed, Cacheable, ReadWrite, 0x, //
> > > Granularity 0x1000, // Range Minimum
> > > 0x3EFF, // Range Maximum
> > > 0x, // Translation Offset
> > > 0x2EFF, // Length
> > 
> > In all the other sections, the Length entry is (rangemax - rangemin)
> > + 1, but in this one it is not, which suggests an error. Probably your
> > rangemax here is wrong, since 0x3eff is actually the first address
> > in the IO window.
> > 
> > (If ACPI is effectively describing the length of the range in
> > two separate places, it's a shame it doesn't sanity check that
> > they both agree...)
> According to spec Range Minimum & Range Maximum are
>lowest/highest possible base address
> and have nothing to do with range size.
> In x86 target that insanity was since the beginning, I guest it works
> because no OS tries to use anything other than Range Minimum. 
> For example If guest has mapped region at Range Maximum, then access to
> it would go beyond on real memory region mapped in QEMU.
> 
> In general Range Minimum & Range Maximum should be the same unless
> HW side covers access up to Range Maximum + Length.
>  

I think it's because the same structure can be used to define memory
resources for devices with programmable base addresses (e.g. like PCI).
Thus you can separately define range of base addresses
(legal BAR values) and separately - the length that is
claimed by the BAR (BAR size).

OTOH IIUC if base address is not programmable, min and max
must always be the same since it's the only valid value.

> > 
> > -- PMM
> > 

Re: [Qemu-devel] [PATCH v4 04/20] hw/acpi/aml-build: Add aml_memory32_fixed() term

[Michael S. Tsirkin]

On Wed, Apr 08, 2015 at 03:54:45PM +0100, Alex Bennée wrote:
> > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > index 8d01959..fefe7c7 100644
> > --- a/hw/acpi/aml-build.c
> > +++ b/hw/acpi/aml-build.c
> > @@ -505,6 +505,28 @@ Aml *aml_call4(const char *method, Aml *arg1, Aml 
> > *arg2, Aml *arg3, Aml *arg4)
> >  return var;
> >  }
> >  
> > +/*
> > + * ACPI 1.0: 6.4.3.4 Memory32Fixed (Memory Resource Descriptor Macro)
> > + */
> > +Aml *aml_memory32_fixed(uint64_t addr, uint64_t size, uint8_t rw_flag)
> > +{
> > +Aml *var = aml_alloc();
> 
> This is more aimed at the ACPI maintainers but I wonder if there should
> be an aml_alloc_sized that pre-allocates the GArray? Otherwise we spend
> a lot of time realloc'ing while building these entries up. Or even a
> varidac build_append_bytes?

Can you show measureable VM boot speedup from this?
If not, it's not worth bothering with.

-- 
MST

[Qemu-devel] [Bug 1441781] Re: qemuProcessSetEmulatorAffinity() not behaving as expected

[Chris Friesen]

** Summary changed:

- qemuProcessSetEmulatorAffinity() called before emulator process actually 
running
+ qemuProcessSetEmulatorAffinity() not behaving as expected

** Description changed:

  When running on a 24-CPU host and using vCPU and emulator pinning I've
  seen cases where the specified emulator pinning isn't applied as
  expected.
  
- It appears the bug was introduced by 411cea6 which moved both
+ It appears the bug was introduced/uncovered by 411cea6 which moved both
  qemuSetupCgroupForEmulator() and qemuProcessSetEmulatorAffinity() up
  before the call to qemuProcessWaitForMonitor().  Reverting this commit
  makes the problem go away.
+ 
+ It's not obvious why this makes a difference, since the pid had to have
+ been up and running already.

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

Title:
  qemuProcessSetEmulatorAffinity() not behaving as expected

Status in QEMU:
  New

Bug description:
  When running on a 24-CPU host and using vCPU and emulator pinning I've
  seen cases where the specified emulator pinning isn't applied as
  expected.

  It appears the bug was introduced/uncovered by 411cea6 which moved
  both qemuSetupCgroupForEmulator() and qemuProcessSetEmulatorAffinity()
  up before the call to qemuProcessWaitForMonitor().  Reverting this
  commit makes the problem go away.

  It's not obvious why this makes a difference, since the pid had to
  have been up and running already.

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

[Qemu-devel] [Bug 1441781] Re: qemuProcessSetEmulatorAffinity() called before emulator process actually running

[Chris Friesen]

** Description changed:

- In qemuProcessStart() the code looks like this:
+ When running on a 24-CPU host and using vCPU and emulator pinning I've
+ seen cases where the specified emulator pinning isn't applied as
+ expected.
  
- VIR_DEBUG("Setting cgroup for emulator (if required)");
- if (qemuSetupCgroupForEmulator(vm) < 0)
- goto cleanup;
- 
- VIR_DEBUG("Setting affinity of emulator threads");
- if (qemuProcessSetEmulatorAffinity(vm) < 0)
- goto cleanup;
- 
- VIR_DEBUG("Waiting for monitor to show up");
- if (qemuProcessWaitForMonitor(driver, vm, asyncJob, priv->qemuCaps, pos) 
< 0)
- goto cleanup;
- 
- 
- When running on a 24-CPU host and using vCPU and emulator pinning I've seen 
cases where the specified emulator pinning isn't applied as expected.
- 
- The issue appears to be due to libvirt racing with qemu.  Moving the
- qemuProcessSetEmulatorAffinity() call to after
- qemuProcessWaitForMonitor() returns seems to fix the problem.
- 
- Also, I suspect that qemuSetupCgroupForEmulator() should probably be
- moved as well.
+ It appears the bug was introduced by 411cea6 which moved both
+ qemuSetupCgroupForEmulator() and qemuProcessSetEmulatorAffinity() up
+ before the call to qemuProcessWaitForMonitor().  Reverting this commit
+ makes the problem go away.

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

Title:
  qemuProcessSetEmulatorAffinity() called before emulator process
  actually running

Status in QEMU:
  New

Bug description:
  When running on a 24-CPU host and using vCPU and emulator pinning I've
  seen cases where the specified emulator pinning isn't applied as
  expected.

  It appears the bug was introduced by 411cea6 which moved both
  qemuSetupCgroupForEmulator() and qemuProcessSetEmulatorAffinity() up
  before the call to qemuProcessWaitForMonitor().  Reverting this commit
  makes the problem go away.

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

[Qemu-devel] [PATCH] qemu-config: Accept empty option values

[Eduardo Habkost]

Currently it is impossible to set an option in a config file to an empty
string, because the parser matches only lines containing non-empty
strings between double-quotes.

As sscanf() "[" conversion specifier only matches non-empty strings, add
a special case for empty strings.

Signed-off-by: Eduardo Habkost 
---
 util/qemu-config.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/util/qemu-config.c b/util/qemu-config.c
index 2d32ce7..9f9577d 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -413,7 +413,8 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const 
char *fname)
 opts = qemu_opts_create(list, NULL, 0, &error_abort);
 continue;
 }
-if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2) {
+if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2 ||
+(value[0] = '\0', sscanf(line, " %63s = \"\"", arg) == 1)) {
 /* arg = value */
 if (opts == NULL) {
 error_report("no group defined");
-- 
2.1.0

[Qemu-devel] [Bug 1395217] Re: Networking in qemu 2.0.0 and beyond is not compatible with Open Solaris (Illumos) 5.11

[Cole Robinson]

FWIW there's some other hits on this:

Fedora bug: https://bugzilla.redhat.com/show_bug.cgi?id=1040500
Openstack mailing list: 
http://lists.openstack.org/pipermail/openstack-dev/2014-December/053478.html

** Bug watch added: Red Hat Bugzilla #1040500
   https://bugzilla.redhat.com/show_bug.cgi?id=1040500

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

Title:
  Networking in qemu 2.0.0 and beyond is not compatible with Open
  Solaris (Illumos) 5.11

Status in QEMU:
  New

Bug description:
  The networking code in qemu in versions 2.0.0 and beyond is non-
  functional with Solaris/Illumos 5.11 images.

  Building 1.7.1, 2.0.0, 2.0.2, 2.1.2,and 2.2.0rc1with the following
  standard Slackware config:

  # From Slackware build tree . . . 
  ./configure \
--prefix=/usr \
--libdir=/usr/lib64 \
--sysconfdir=/etc \
--localstatedir=/var \
--enable-gtk \
--enable-system \
--enable-kvm \
--disable-debug-info \
--enable-virtfs \
--enable-sdl \
--audio-drv-list=alsa,oss,sdl,esd \
--enable-libusb \
--disable-vnc \
--target-list=x86_64-linux-user,i386-linux-user,x86_64-softmmu,i386-softmmu 
\
--enable-spice \
--enable-usb-redir 

  
  And attempting to run the same VM image with the following command (or via 
virt-manager):

  macaddress="DE:AD:BE:EF:3F:A4"

  qemu-system-x86_64 nex4x -cdrom /dev/cdrom -name "Nex41" -cpu Westmere
  -machine accel=kvm -smp 2 -m 4000 -net nic,macaddr=$macaddress  -net 
bridge,br=b
  r0 -net dump,file=/usr1/tmp/ -drive file=nex4x_d1 -drive 
file=nex4x_d2
   -enable-kvm

  Gives success on 1.7.1, and a deaf VM on all subsequent versions.

  Notable in validating my config, is that a Windows 7 image runs
  cleanly with networking on *all* builds, so my configuration appears
  to be good - qemu just hates Solaris at this point.

  Watching with wireshark (as well as pulling network traces from qemu
  as noted above) it appears that the notable difference in the two
  configs is that for some reason, Solaris gets stuck arping for it's
  own interface on startup, and never really comes on line on the
  network.  If other hosts attempt to ping the Solaris instance, they
  can successfully arp the bad VM, but not the other way around.

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

[Qemu-devel] [Bug 1441781] [NEW] qemuProcessSetEmulatorAffinity() called before emulator process actually running

[Chris Friesen]

Public bug reported:

In qemuProcessStart() the code looks like this:

VIR_DEBUG("Setting cgroup for emulator (if required)");
if (qemuSetupCgroupForEmulator(vm) < 0)
goto cleanup;

VIR_DEBUG("Setting affinity of emulator threads");
if (qemuProcessSetEmulatorAffinity(vm) < 0)
goto cleanup;

VIR_DEBUG("Waiting for monitor to show up");
if (qemuProcessWaitForMonitor(driver, vm, asyncJob, priv->qemuCaps, pos) < 
0)
goto cleanup;


When running on a 24-CPU host and using vCPU and emulator pinning I've seen 
cases where the specified emulator pinning isn't applied as expected.

The issue appears to be due to libvirt racing with qemu.  Moving the
qemuProcessSetEmulatorAffinity() call to after
qemuProcessWaitForMonitor() returns seems to fix the problem.

Also, I suspect that qemuSetupCgroupForEmulator() should probably be
moved as well.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  qemuProcessSetEmulatorAffinity() called before emulator process
  actually running

Status in QEMU:
  New

Bug description:
  In qemuProcessStart() the code looks like this:

  VIR_DEBUG("Setting cgroup for emulator (if required)");
  if (qemuSetupCgroupForEmulator(vm) < 0)
  goto cleanup;

  VIR_DEBUG("Setting affinity of emulator threads");
  if (qemuProcessSetEmulatorAffinity(vm) < 0)
  goto cleanup;

  VIR_DEBUG("Waiting for monitor to show up");
  if (qemuProcessWaitForMonitor(driver, vm, asyncJob, priv->qemuCaps, pos) 
< 0)
  goto cleanup;

  
  When running on a 24-CPU host and using vCPU and emulator pinning I've seen 
cases where the specified emulator pinning isn't applied as expected.

  The issue appears to be due to libvirt racing with qemu.  Moving the
  qemuProcessSetEmulatorAffinity() call to after
  qemuProcessWaitForMonitor() returns seems to fix the problem.

  Also, I suspect that qemuSetupCgroupForEmulator() should probably be
  moved as well.

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

Re: [Qemu-devel] Last call for release-critical fixes for 2.3...

[Andreas Färber]

Am 08.04.2015 um 17:20 schrieb Peter Maydell:
> It looks like we have perhaps only a few more issues which need fixes
> to go into 2.3, so what I'd like to do is hold rc3 until we have them
> all fixed and then roll an rc3 which will with luck be our final
> release (bar the version number).
> 
> Can people reply to this mail if they have things that need to go in?
> So far I know about:
>  * Stefan intends to submit a pullreq with a few patches tomorrow
>  * some patches from Dirk to use memory_region_allocate_system_memory
>(though we've had several releases with this regression so it's
>not a huge deal if these don't make it)
> 
> Anything else?

I noticed (through a recent build-after-update failure) that the
Netduino2 machine is using a QOM type *_soc and wanted to fix that...
Don't find my patch right now, will post a new one then.

Regards,
Andreas

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

[Qemu-devel] [PATCH 4/5] ui/console : remove 'struct' from 'typedef struct' type

[Chih-Min Chao]

Signed-off-by: Chih-Min Chao 
---
 ui/console.c   | 4 ++--
 ui/spice-display.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ui/console.c b/ui/console.c
index b15ca87..d8d268c 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -269,7 +269,7 @@ void graphic_hw_invalidate(QemuConsole *con)
 }
 }
 
-static void ppm_save(const char *filename, struct DisplaySurface *ds,
+static void ppm_save(const char *filename, DisplaySurface *ds,
  Error **errp)
 {
 int width = pixman_image_get_width(ds->image);
@@ -1535,7 +1535,7 @@ void dpy_text_update(QemuConsole *con, int x, int y, int 
w, int h)
 void dpy_text_resize(QemuConsole *con, int w, int h)
 {
 DisplayState *s = con->ds;
-struct DisplayChangeListener *dcl;
+DisplayChangeListener *dcl;
 
 if (!qemu_console_is_visible(con)) {
 return;
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 1644185..6767d08 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -715,7 +715,7 @@ static void display_update(DisplayChangeListener *dcl,
 }
 
 static void display_switch(DisplayChangeListener *dcl,
-   struct DisplaySurface *surface)
+   DisplaySurface *surface)
 {
 SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
 qemu_spice_display_switch(ssd, surface);
-- 
1.9.1

Re: [Qemu-devel] [Qemu-block] [PATCH] block/iscsi: handle zero events from iscsi_which_events

[ronnie sahlberg]

On Wed, Apr 8, 2015 at 1:38 AM, Stefan Hajnoczi  wrote:
> On Tue, Apr 7, 2015 at 9:08 PM, Peter Lieven  wrote:
>
> Please CC qemu-devel@nongnu.org in the future.  All patches must be on
> the qemu-devel mailing list so they can be merged (for transparency).
> I have added qemu-devel to CC.
>
>> +/* newer versions of libiscsi may return zero events. In this
>> + * case start a timer to ensure we are able to return to service
>> + * once this situation changes. */
>> +if (!ev) {
>> +timer_mod(iscsilun->event_timer,
>> +  qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + EVENT_INTERVAL);
>>  }
>
> This suggests that libiscsi is waiting on its own internal timer.  It
> would be cleaner for libiscsi to provide a timeout value so the
> application doesn't need to poll the library.  Is there still scope to
> modify libiscsi to do this?

I think that returning a timeout value would be a bigger change in the
API that Peter wanted to avoid.
We discussed that as an option by my take from it was that this would
require that qemu and libiscsi
again would have to be updated in lockstep,

In this particular case with creating a delay between failed reconnect
attempts, i.e. the target is restarting and
returning a RST to the SYN request until it has fully restarted, the
correct amount of time to wait until re-trying is at best a guess
anyway.
I don't have any particular feelings on whether the decision on how
long to wait is done inside libiscsi and communicated to qemu,
or if it is done in qemu itself.

The nice part with the current patch of Peter is that qemu and
libiscsi can be upgraded/downgraded independently.

[Qemu-devel] [PATCH 2/5] ui/vnc : fix coding style

[Chih-Min Chao]

reported by checkpatch.pl

Signed-off-by: Chih-Min Chao 
---
 ui/vnc-auth-vencrypt.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/ui/vnc-auth-vencrypt.c b/ui/vnc-auth-vencrypt.c
index a420ccb..65f1afa 100644
--- a/ui/vnc-auth-vencrypt.c
+++ b/ui/vnc-auth-vencrypt.c
@@ -65,7 +65,8 @@ static void start_auth_vencrypt_subauth(VncState *vs)
 
 static void vnc_tls_handshake_io(void *opaque);
 
-static int vnc_start_vencrypt_handshake(struct VncState *vs) {
+static int vnc_start_vencrypt_handshake(struct VncState *vs)
+{
 int ret;
 
 if ((ret = gnutls_handshake(vs->tls.session)) < 0) {
@@ -100,7 +101,8 @@ static int vnc_start_vencrypt_handshake(struct VncState 
*vs) {
 return 0;
 }
 
-static void vnc_tls_handshake_io(void *opaque) {
+static void vnc_tls_handshake_io(void *opaque)
+{
 struct VncState *vs = (struct VncState *)opaque;
 
 VNC_DEBUG("Handshake IO continue\n");
-- 
1.9.1

[Qemu-devel] [PATCH 5/5] hw/display : remove 'struct' from 'typedef QXL struct'

[Chih-Min Chao]

Signed-off-by: Chih-Min Chao 
---
 hw/display/qxl.c   | 2 +-
 ui/spice-display.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index b6d65b9..0cd314c 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -696,7 +696,7 @@ static inline void qxl_push_free_res(PCIQXLDevice *d, int 
flush)
 
 /* called from spice server thread context only */
 static void interface_release_resource(QXLInstance *sin,
-   struct QXLReleaseInfoExt ext)
+   QXLReleaseInfoExt ext)
 {
 PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
 QXLReleaseRing *ring;
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 6767d08..32263a3 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -536,7 +536,7 @@ static void interface_get_init_info(QXLInstance *sin, 
QXLDevInitInfo *info)
 info->n_surfaces = ssd->num_surfaces;
 }
 
-static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
+static int interface_get_command(QXLInstance *sin, QXLCommandExt *ext)
 {
 SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
 SimpleSpiceUpdate *update;
@@ -563,7 +563,7 @@ static int interface_req_cmd_notification(QXLInstance *sin)
 }
 
 static void interface_release_resource(QXLInstance *sin,
-   struct QXLReleaseInfoExt rext)
+   QXLReleaseInfoExt rext)
 {
 SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
 SimpleSpiceUpdate *update;
@@ -586,7 +586,7 @@ static void interface_release_resource(QXLInstance *sin,
 }
 }
 
-static int interface_get_cursor_command(QXLInstance *sin, struct QXLCommandExt 
*ext)
+static int interface_get_cursor_command(QXLInstance *sin, QXLCommandExt *ext)
 {
 SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
 int ret;
-- 
1.9.1

[Qemu-devel] [PATCH 1/5] bitops : fix coding style

[Chih-Min Chao]

don't mix tab and space. The rule is 4 spaces

Signed-off-by: Chih-Min Chao 
---
 include/qemu/bitops.h | 61 ++-
 1 file changed, 31 insertions(+), 30 deletions(-)

diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index 90ca8df..8abdcf9 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -20,10 +20,10 @@
 #define BITS_PER_BYTE   CHAR_BIT
 #define BITS_PER_LONG   (sizeof (unsigned long) * BITS_PER_BYTE)
 
-#define BIT(nr)(1UL << (nr))
-#define BIT_MASK(nr)   (1UL << ((nr) % BITS_PER_LONG))
-#define BIT_WORD(nr)   ((nr) / BITS_PER_LONG)
-#define BITS_TO_LONGS(nr)  DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
+#define BIT(nr) (1UL << (nr))
+#define BIT_MASK(nr)(1UL << ((nr) % BITS_PER_LONG))
+#define BIT_WORD(nr)((nr) / BITS_PER_LONG)
+#define BITS_TO_LONGS(nr)   DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
 
 /**
  * set_bit - Set a bit in memory
@@ -32,10 +32,10 @@
  */
 static inline void set_bit(long nr, unsigned long *addr)
 {
-   unsigned long mask = BIT_MASK(nr);
-unsigned long *p = addr + BIT_WORD(nr);
+unsigned long mask = BIT_MASK(nr);
+unsigned long *p = addr + BIT_WORD(nr);
 
-   *p  |= mask;
+*p  |= mask;
 }
 
 /**
@@ -45,10 +45,10 @@ static inline void set_bit(long nr, unsigned long *addr)
  */
 static inline void clear_bit(long nr, unsigned long *addr)
 {
-   unsigned long mask = BIT_MASK(nr);
-unsigned long *p = addr + BIT_WORD(nr);
+unsigned long mask = BIT_MASK(nr);
+unsigned long *p = addr + BIT_WORD(nr);
 
-   *p &= ~mask;
+*p &= ~mask;
 }
 
 /**
@@ -58,10 +58,10 @@ static inline void clear_bit(long nr, unsigned long *addr)
  */
 static inline void change_bit(long nr, unsigned long *addr)
 {
-   unsigned long mask = BIT_MASK(nr);
-unsigned long *p = addr + BIT_WORD(nr);
+unsigned long mask = BIT_MASK(nr);
+unsigned long *p = addr + BIT_WORD(nr);
 
-   *p ^= mask;
+*p ^= mask;
 }
 
 /**
@@ -71,12 +71,12 @@ static inline void change_bit(long nr, unsigned long *addr)
  */
 static inline int test_and_set_bit(long nr, unsigned long *addr)
 {
-   unsigned long mask = BIT_MASK(nr);
-unsigned long *p = addr + BIT_WORD(nr);
-   unsigned long old = *p;
+unsigned long mask = BIT_MASK(nr);
+unsigned long *p = addr + BIT_WORD(nr);
+unsigned long old = *p;
 
-   *p = old | mask;
-   return (old & mask) != 0;
+*p = old | mask;
+return (old & mask) != 0;
 }
 
 /**
@@ -86,12 +86,12 @@ static inline int test_and_set_bit(long nr, unsigned long 
*addr)
  */
 static inline int test_and_clear_bit(long nr, unsigned long *addr)
 {
-   unsigned long mask = BIT_MASK(nr);
-unsigned long *p = addr + BIT_WORD(nr);
-   unsigned long old = *p;
+unsigned long mask = BIT_MASK(nr);
+unsigned long *p = addr + BIT_WORD(nr);
+unsigned long old = *p;
 
-   *p = old & ~mask;
-   return (old & mask) != 0;
+*p = old & ~mask;
+return (old & mask) != 0;
 }
 
 /**
@@ -101,12 +101,12 @@ static inline int test_and_clear_bit(long nr, unsigned 
long *addr)
  */
 static inline int test_and_change_bit(long nr, unsigned long *addr)
 {
-   unsigned long mask = BIT_MASK(nr);
-unsigned long *p = addr + BIT_WORD(nr);
-   unsigned long old = *p;
+unsigned long mask = BIT_MASK(nr);
+unsigned long *p = addr + BIT_WORD(nr);
+unsigned long old = *p;
 
-   *p = old ^ mask;
-   return (old & mask) != 0;
+*p = old ^ mask;
+return (old & mask) != 0;
 }
 
 /**
@@ -116,7 +116,7 @@ static inline int test_and_change_bit(long nr, unsigned 
long *addr)
  */
 static inline int test_bit(long nr, const unsigned long *addr)
 {
-   return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
+return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
 }
 
 /**
@@ -136,7 +136,8 @@ unsigned long find_last_bit(const unsigned long *addr,
  * @size: The bitmap size in bits
  */
 unsigned long find_next_bit(const unsigned long *addr,
-  unsigned long size, unsigned long offset);
+unsigned long size,
+unsigned long offset);
 
 /**
  * find_next_zero_bit - find the next cleared bit in a memory region
-- 
1.9.1

[Qemu-devel] [PATCH 0/5] fix coding style and use of typedef type

[Chih-Min Chao]

patch 1-2 
fix the coding style related to space indent and brance position

patch 3-5 
remove the 'struct' from the type which has been typedef


Chih-Min Chao (5):
  bitops : fix coding style
  ui/vnc : fix coding style
  ui/vnc : remove 'struct' of 'typedef struct'
  ui/console : remove 'struct' from 'typedef struct' type
  hw/display : remove 'struct' from 'typedef QXL struct'

 hw/display/qxl.c   |  2 +-
 include/qemu/bitops.h  | 61 +-
 ui/console.c   |  4 ++--
 ui/spice-display.c |  8 +++
 ui/vnc-auth-vencrypt.c |  8 ---
 ui/vnc-tls.c   | 10 -
 ui/vnc-ws.c|  4 ++--
 ui/vnc.c   |  2 +-
 8 files changed, 51 insertions(+), 48 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH 3/5] ui/vnc : remove 'struct' of 'typedef struct'

[Chih-Min Chao]

Signed-off-by: Chih-Min Chao 
---
 ui/vnc-auth-vencrypt.c |  4 ++--
 ui/vnc-tls.c   | 10 +-
 ui/vnc-ws.c|  4 ++--
 ui/vnc.c   |  2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/ui/vnc-auth-vencrypt.c b/ui/vnc-auth-vencrypt.c
index 65f1afa..03ea48a 100644
--- a/ui/vnc-auth-vencrypt.c
+++ b/ui/vnc-auth-vencrypt.c
@@ -65,7 +65,7 @@ static void start_auth_vencrypt_subauth(VncState *vs)
 
 static void vnc_tls_handshake_io(void *opaque);
 
-static int vnc_start_vencrypt_handshake(struct VncState *vs)
+static int vnc_start_vencrypt_handshake(VncState *vs)
 {
 int ret;
 
@@ -103,7 +103,7 @@ static int vnc_start_vencrypt_handshake(struct VncState *vs)
 
 static void vnc_tls_handshake_io(void *opaque)
 {
-struct VncState *vs = (struct VncState *)opaque;
+VncState *vs = (VncState *)opaque;
 
 VNC_DEBUG("Handshake IO continue\n");
 vnc_start_vencrypt_handshake(vs);
diff --git a/ui/vnc-tls.c b/ui/vnc-tls.c
index eddd39b..028fc4d 100644
--- a/ui/vnc-tls.c
+++ b/ui/vnc-tls.c
@@ -68,7 +68,7 @@ static int vnc_tls_initialize(void)
 static ssize_t vnc_tls_push(gnutls_transport_ptr_t transport,
 const void *data,
 size_t len) {
-struct VncState *vs = (struct VncState *)transport;
+VncState *vs = (VncState *)transport;
 int ret;
 
  retry:
@@ -85,7 +85,7 @@ static ssize_t vnc_tls_push(gnutls_transport_ptr_t transport,
 static ssize_t vnc_tls_pull(gnutls_transport_ptr_t transport,
 void *data,
 size_t len) {
-struct VncState *vs = (struct VncState *)transport;
+VncState *vs = (VncState *)transport;
 int ret;
 
  retry:
@@ -170,7 +170,7 @@ static gnutls_certificate_credentials_t 
vnc_tls_initialize_x509_cred(VncDisplay
 }
 
 
-int vnc_tls_validate_certificate(struct VncState *vs)
+int vnc_tls_validate_certificate(VncState *vs)
 {
 int ret;
 unsigned int status;
@@ -332,7 +332,7 @@ static int vnc_set_gnutls_priority(gnutls_session_t s, int 
x509)
 
 #endif
 
-int vnc_tls_client_setup(struct VncState *vs,
+int vnc_tls_client_setup(VncState *vs,
  int needX509Creds) {
 VNC_DEBUG("Do TLS setup\n");
 if (vnc_tls_initialize() < 0) {
@@ -410,7 +410,7 @@ int vnc_tls_client_setup(struct VncState *vs,
 }
 
 
-void vnc_tls_client_cleanup(struct VncState *vs)
+void vnc_tls_client_cleanup(VncState *vs)
 {
 if (vs->tls.session) {
 gnutls_deinit(vs->tls.session);
diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
index 62eb97f..38a1b8b 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -24,7 +24,7 @@
 #ifdef CONFIG_VNC_TLS
 #include "qemu/sockets.h"
 
-static int vncws_start_tls_handshake(struct VncState *vs)
+static int vncws_start_tls_handshake(VncState *vs)
 {
 int ret = gnutls_handshake(vs->tls.session);
 
@@ -63,7 +63,7 @@ static int vncws_start_tls_handshake(struct VncState *vs)
 
 void vncws_tls_handshake_io(void *opaque)
 {
-struct VncState *vs = (struct VncState *)opaque;
+VncState *vs = (VncState *)opaque;
 
 if (!vs->tls.session) {
 VNC_DEBUG("TLS Websocket setup\n");
diff --git a/ui/vnc.c b/ui/vnc.c
index cffb5b7..9f8ecd0 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1046,7 +1046,7 @@ static void vnc_dpy_cursor_define(DisplayChangeListener 
*dcl,
 }
 }
 
-static int find_and_clear_dirty_height(struct VncState *vs,
+static int find_and_clear_dirty_height(VncState *vs,
int y, int last_x, int x, int height)
 {
 int h;
-- 
1.9.1

Re: [Qemu-devel] bios-tables-test failing for x86_64 on ppc

[Andreas Färber]

Am 08.04.2015 um 17:22 schrieb Peter Maydell:
> On 8 April 2015 at 16:19, Andreas Färber  wrote:
>> Hello,
>>
>> With rcutorture fixed, I am still seeing the following failure in -rc2.
>> It is only for /x86_64/acpi/..., not for /i386/acpi/...; only on ppc
>> host, not on ppc64 or ppc64le (so not endianness), nor on i586 (so not
>> 32-bitness alone).
>>
>> Alex was suggesting it may be a ppc TCG problem, but I'm not seeing
>> anything suspicious in this QTest since v2.2.0:
>>
>> 3a9c86d tests: bios-tables-test: add support for testing bridges
>> 71f4be2 bios-tables-test: split piix4 and q35 tests
>> b8e665e qtests: Specify image format explicitly
> 
> This is basically the only test in "make check" that tries to
> actually execute any TCG code, so it's a bit of a canary for
> general TCG issues. If you try just running a random guest on
> this QEMU (eg qemu-system-i386 and see if it can at least execute
> the BIOS properly) does it work or crash?

I've resorted to setting up a ppc chroot on ppc64, and I can reproduce
that qemu-system-i386 shows SeaBIOS okay, but qemu-system-x86_64
immediately aborts with the quoted error.

Regards,
Andreas

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

[Qemu-devel] [Bug 1441775] [NEW] possible null pointer dereference in qemuDomainPinEmulator()

[Chris Friesen]

Public bug reported:

In src/qemu/qemu_driver.c the qemuDomainPinEmulator() routine basically
does this

 virDomainObjPtr vm;

 if (!(vm = qemuDomObjFromDomain(dom)))
 goto cleanup;

cleanup:
 qemuDomObjEndAPI(&vm);


If "vm" is null, then this will crash.

The bug seems to have been added in commit 540c339a, which removed a null 
pointer check:
-if (vm)
-virObjectUnlock(vm);
+qemuDomObjEndAPI(&vm);

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  possible null pointer dereference in qemuDomainPinEmulator()

Status in QEMU:
  New

Bug description:
  In src/qemu/qemu_driver.c the qemuDomainPinEmulator() routine
  basically does this

   virDomainObjPtr vm;

   if (!(vm = qemuDomObjFromDomain(dom)))
   goto cleanup;

  cleanup:
   qemuDomObjEndAPI(&vm);

  
  If "vm" is null, then this will crash.

  The bug seems to have been added in commit 540c339a, which removed a null 
pointer check:
  -if (vm)
  -virObjectUnlock(vm);
  +qemuDomObjEndAPI(&vm);

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

Re: [Qemu-devel] Last call for release-critical fixes for 2.3...

[John Snow]

On 04/08/2015 11:20 AM, Peter Maydell wrote:

It looks like we have perhaps only a few more issues which need fixes
to go into 2.3, so what I'd like to do is hold rc3 until we have them
all fixed and then roll an rc3 which will with luck be our final
release (bar the version number).

Can people reply to this mail if they have things that need to go in?
So far I know about:
  * Stefan intends to submit a pullreq with a few patches tomorrow
  * some patches from Dirk to use memory_region_allocate_system_memory
(though we've had several releases with this regression so it's
not a huge deal if these don't make it)

Anything else?

thanks
-- PMM



I am still working on testing it, but I think '[Qemu-devel] [PATCH] aio: 
strengthen memory barriers for bottom half scheduling' is likely 
something that should go in rc3 if it is to be our final RC.


I am seeing a lot more missed completion notifications in my testing 
right now than I have even a month ago, so I am a little concerned.


I'm trying to reproduce with this patch /right now/ to confirm it fixes 
my problem; it at least fixes Paul Leveille's.

Re: [Qemu-devel] [PATCH v2 for-2.3] arm: memory: Replace memory_region_init_ram with memory_region_allocate_system_memory

[Peter Maydell]

On 4 April 2015 at 13:24, Dirk Müller  wrote:
> Commit 0b183fc871:"memory: move mem_path handling to
> memory_region_allocate_system_memory" split memory_region_init_ram and
> memory_region_init_ram_from_file. Also it moved mem-path handling a step
> up from memory_region_init_ram to memory_region_allocate_system_memory.
>
> Therefore for any board that uses memory_region_init_ram directly,
> -mem-path is not supported.
>
> Fix this by replacing memory_region_init_ram with
> memory_region_allocate_system_memory.
>
> Signed-off-by: Dirk Mueller 

Applied to master, thanks. PS: you might want to check
your workflow for future patches, because this one had
a bunch of bad linewrapping that was a bit painful to
fix up manually. (Also your From and Signed-off-by have
different email addresses.)

PS: are you planning to submit another patch later for
the exynos boards?

It might also be nice if we could assert() if the board
init code never calls memory_region_allocate_system_memory();
then 'make check' would catch any new board models that
failed to do this, or old boards which we forgot to convert.

thanks
-- PMM

[Qemu-devel] [PATCH 2/2] MAINTAINERS: Change status of X86 to Maintained

[Eduardo Habkost]

"Odd Fixes" doesn't reflect the current status of target-i386. We have
people looking after it, now.

Signed-off-by: Eduardo Habkost 
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7db4c00..f714a98 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -173,7 +173,7 @@ X86
 M: Paolo Bonzini 
 M: Richard Henderson 
 M: Eduardo Habkost 
-S: Odd Fixes
+S: Maintained
 F: target-i386/
 F: hw/i386/
 
-- 
2.1.0

[Qemu-devel] [PATCH 0/2] MAINTAINERS: X86 update

[Eduardo Habkost]

Add myself as X86 maintainer, and change its status to "Maintained".

Eduardo Habkost (2):
  MAINTAINERS: Add myself to X86
  MAINTAINERS: Change status of X86 to Maintained

 MAINTAINERS | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
2.1.0

[Qemu-devel] [PATCH 1/2] MAINTAINERS: Add myself to X86

[Eduardo Habkost]

Signed-off-by: Eduardo Habkost 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d7e9ba2..7db4c00 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -172,6 +172,7 @@ F: hw/unicore32/
 X86
 M: Paolo Bonzini 
 M: Richard Henderson 
+M: Eduardo Habkost 
 S: Odd Fixes
 F: target-i386/
 F: hw/i386/
-- 
2.1.0

[Qemu-devel] [PATCH] MAINTAINERS: Add myself as NUMA code maintainer

[Eduardo Habkost]

The "srat" and "numa" keywords will help get_maintainer.pl catch
NUMA-related code in other files too.

Signed-off-by: Eduardo Habkost 
---
 MAINTAINERS | 9 +
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d7e9ba2..978e714 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -905,6 +905,15 @@ F: nbd.*
 F: qemu-nbd.c
 T: git git://github.com/bonzini/qemu.git nbd-next
 
+NUMA
+M: Eduardo Habkost 
+S: Maintained
+F: numa.c
+F: include/sysemu/numa.h
+K: numa|NUMA
+K: srat|SRAT
+T: git git://github.com/ehabkost/qemu.git numa
+
 QAPI
 M: Luiz Capitulino 
 M: Michael Roth 
-- 
2.1.0

Re: [Qemu-devel] [PATCH v6 02/36] qapi: Document type-safety considerations

[Eric Blake]

On 04/04/2015 10:07 PM, Eric Blake wrote:
> Go into more details about the various types of valid expressions
> in a qapi schema, including tweaks to document fixes being done
> later in the current patch series.  Also fix some stale and missing
> documentation in the QMP specification.
> 
> Signed-off-by: Eric Blake 
> 
> ---
> 
> v6: split copyright change into another patch; merge in 1/5, 3/5,
> and 5/5 of doc cleanup series; drop simple union with base docs,
> and mention relation between simple and flat unions; mention QMP
> is encoded as UTF-8; mention extensions of 'single \' quote' and
> "\'"; mention that repeating json-object (dict) keys is unspecified.

A bit of self-review:

> +++ b/docs/qmp/qmp-spec.txt
> @@ -13,8 +13,11 @@ Last revised in March 2015.
> @@ -62,7 +78,19 @@ The greeting message format is:
>  - The "version" member contains the Server's version information (the format
>is the same of the query-version command)
>  - The "capabilities" member specify the availability of features beyond the
> -  baseline specification
> +  baseline specification; the order of elements in this array has no
> +  particular significance, so a client must search the entire array
> +  when looking for a particular capability
> +
> +When first connecting to the server, the connection is in a capability
> +exchange mode, further documented below.

This sentence is redundant with the earlier sentence in the same section:

>> Right when connected the Server will issue a greeting message, which signals
>> that the connection has been successfully established and that the Server is
>> ready for capabilities negotiation (for more information refer to section
>> '4. Capabilities Negotiation').

I'll wait for other reviews before deciding whether it is easier to do
as a patch that a maintainer squashes in on commit of v6, as a v7
respin, or as an independent followup patch.

>  4. Capabilities Negotiation
> -
> +===
> 
>  When a Client successfully establishes a connection, the Server is in
>  Capabilities Negotiation mode.

Meanwhile, as this has just come up on the list, I'm now looking at how
easy it would be to make capability negotiation give saner error messages :)
https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg00833.html

Which of course may mean further touching of this file, and therefore
justifying a separate cleanup patch anyways.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] Getting VM state from outside QEMU?

[Eric Blake]

On 04/08/2015 10:21 AM, Erik Rull wrote:

>>> 172.17.48.45 ~ # telnet 127.0.0.1 
>>> {"QMP": {"version": {"qemu": {"micro": 0, "minor": 1, "major": 2},
>>> "package":
>>> ""}, "capabilities": []}}
>>
>> You HAVE to use {"execute":"qmp_capabilities"} (possibly with an
>> "id":...) as your first command on the monitor, before you can issue any
>> other command.  I really wish we could improve the error message:

> Thanks! That fixed it - even if I don't know why this was actually necessary 
> :-)

docs/qmp/qmp-spec.txt

It's part of the protocol, to allow us to make non-backwards-compatible
changes to how QMP is parsed while still keeping sane interactions
between mismatches in server vs. client (to date, we have wisely not
needed to add any capabilities).

-- 
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 1/3] block: add bdrv_get_device_or_node_name()

[Eric Blake]

On 04/08/2015 03:29 AM, Alberto Garcia wrote:
> This function gets the device name associated with a BlockDriverState,
> or its node name if the device name is empty.
> 
> Signed-off-by: Alberto Garcia 
> Reviewed-by: Max Reitz 
> ---
>  block.c   | 9 +
>  block/quorum.c| 5 +
>  include/block/block.h | 1 +
>  3 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/block.c b/block.c
> index f2f8ae7..24adfc1 100644
> --- a/block.c
> +++ b/block.c
> @@ -3953,6 +3953,15 @@ const char *bdrv_get_device_name(const 
> BlockDriverState *bs)
>  return bs->blk ? blk_name(bs->blk) : "";
>  }
>  
> +/* This can be used to identify nodes that might not have a device
> + * name associated. Since node and device names live in the same
> + * namespace, the result is unambiguous. The exception is if both are
> + * absent, then this returns an empty (non-null) string. */
> +const char *bdrv_get_device_or_node_name(const BlockDriverState *bs)
> +{
> +return bs->blk ? blk_name(bs->blk) : bs->node_name;

I had to check; bs->node_name is an array rather than a pointer, so it
always contains a non-null string (whether or not it is the empty
string), so your comment is correct.

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 2/3] block: use bdrv_get_device_or_node_name() in error messages

[Eric Blake]

On 04/08/2015 03:29 AM, Alberto Garcia wrote:
> There are several error messages that identify a BlockDriverState by
> its device name. However those errors can be produced in nodes that
> don't have a device name associated.
> 
> In those cases we should use bdrv_get_device_or_node_name() to fall
> back to the node name and produce a more meaningful message. The
> messages are also updated to use the more generic term 'node' instead
> of 'device'.
> 
> Signed-off-by: Alberto Garcia 
> Reviewed-by: Max Reitz 
> ---

> +++ b/block/snapshot.c
> @@ -246,9 +246,9 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
>  if (bs->file) {
>  return bdrv_snapshot_delete(bs->file, snapshot_id, name, errp);
>  }
> -error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
> -  drv->format_name, bdrv_get_device_name(bs),
> -  "internal snapshot deletion");
> +error_setg(errp, "Block format '%s' used by device '%s' "
> +   "does not support internal snapshot deletion",
> +   drv->format_name, bdrv_get_device_name(bs));
>  return -ENOTSUP;
>  }
>  
> @@ -329,9 +329,9 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
>  if (drv->bdrv_snapshot_load_tmp) {
>  return drv->bdrv_snapshot_load_tmp(bs, snapshot_id, name, errp);
>  }
> -error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
> -  drv->format_name, bdrv_get_device_name(bs),
> -  "temporarily load internal snapshot");
> +error_setg(errp, "Block format '%s' used by device '%s' "
> +   "does not support temporarily loading internal snapshots",
> +   drv->format_name, bdrv_get_device_name(bs));

Should these two messages use 'node' instead of 'device'?  After all, a
format is tied to a node (as a backing chain can involve multiple nodes
using different formats)


> +++ b/blockdev.c
> @@ -1248,13 +1248,14 @@ static void 
> internal_snapshot_prepare(BlkTransactionState *common,
>  }
>  
>  if (bdrv_is_read_only(bs)) {
> -error_set(errp, QERR_DEVICE_IS_READ_ONLY, device);
> +error_setg(errp, "Device '%s' is read only", device);
>  return;
>  }

This one is probably fine as device;

>  
>  if (!bdrv_can_snapshot(bs)) {
> -error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
> -  bs->drv->format_name, device, "internal snapshot");
> +error_setg(errp, "Block format '%s' used by device '%s' "
> +   "does not support internal snapshots",
> +   bs->drv->format_name, device);

but this is probably another one where node may be better.

But it's already a strict improvement, so I can live with:

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] Getting VM state from outside QEMU?

[Erik Rull]

Hi Eric,

> On April 8, 2015 at 6:16 PM Eric Blake  wrote:
> 
> 
> On 04/08/2015 10:10 AM, Erik Rull wrote:
> >>
> >> My suggestion is to create a script that sends the QMP command
> >> "query-status" an then parse the result. The syntax and output is:
> >>
> >> -> { "execute": "query-status" }
> >> <- { "return": { "running": true, "singlestep": false, "status":
> >> "running" } }
> >>
> > 
> > Sounds good - I tried that - but all attempts return that the command has
> > not
> > been found. I added the following command line snippet and the results are:
> > [...] -qmp tcp:localhost:,server,nowait [...]
> > 
> > 172.17.48.45 ~ # telnet 127.0.0.1 
> > {"QMP": {"version": {"qemu": {"micro": 0, "minor": 1, "major": 2},
> > "package":
> > ""}, "capabilities": []}}
> 
> You HAVE to use {"execute":"qmp_capabilities"} (possibly with an
> "id":...) as your first command on the monitor, before you can issue any
> other command.  I really wish we could improve the error message:
> 
> > 
> > { "execute": "query-status" }
> > {"error": {"class": "CommandNotFound", "desc": "The command query-status has
> > not
> > been found"}}
> 
> it would be a LOT nicer if we reported 'still in negotiation phase;
> "qmp_capabilities" expected' than a bland "CommandNotFound".  Of course,
> patches are welcome to improve the experience there!
> 
> Similarly, once you are NOT in capabilities negotiation, any subsequent
> use of "qmp_capabilities" fails.  That's also something where the error
> message could be improved.
> 
> -- 
> Eric Blake   eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org

Thanks! That fixed it - even if I don't know why this was actually necessary :-)

{"execute":"qmp_capabilities"}
{"return": {}}
{ "execute": "query-status" }
{"return": {"status": "running", "singlestep": false, "running": true}}

Best regards,

Erik

Re: [Qemu-devel] [RFC PATCH] vl.c: add -semihosting-config "arg" sub-argument

[Leon Alrae]

On 02/04/2015 17:47, Liviu Ionescu wrote:
> 
>> On 02 Apr 2015, at 17:27, Matthew Fortune  wrote:
>>
>> Liviu Ionescu  writes:
>>> for completeness:
>>>
>>> ilg-mbp:gnuarmeclipse-qemu.git ilg$ "/Applications/GNU ARM
>>> Eclipse/QEMU/2.2.91-20150402-dev/bin/qemu-system-gnuarmeclipse" -
>>> verbose -machine STM32-H103 -gdb tcp::1234 -semihosting-config
>>> enable=on,target=native,cmdline='n "1 a" 2 3'
>>
>> I see here that you have switched quotes because you know that you are
>> providing a double quoted argument within the string.
> 
> my code works both ways symmetrically:
> 
> ilg-mbp:~ ilg$ "/Applications/GNU ARM 
> Eclipse"/QEMU/2.2.91-20150402-dev/bin/qemu-system-gnuarmeclipse -verbose 
> -machine STM32-H103 -gdb tcp::1234 -semihosting-config 
> enable=on,target=native,cmdline="name 1 '2 a' 3"
> 
> GNU ARM Eclipse QEMU v2.2.91 (qemu-system-gnuarmeclipse).
> Board: 'STM32-H103' (Olimex Header Board for STM32F103RBT6 (Experimental)).
> Device: 'STM32F103RB' (cortex-m3, MPU), Flash: 128 KB, RAM: 20 KB.
> Command line: 'name 1 '2 a' 3' (14 bytes).
> GDB Server listening on: 'tcp::1234'...
> ... connection accepted from 127.0.0.1.
> 
> Execute 'mon system_reset'.
> main(argc=4, argv=["name", "1", "2 a", "3"]);
> Hello ARM World!
> 
> in other words, after being delivered by SYS_GET_CMDLINE to the application, 
> the startup parser uses either single or double quotes to split the string 
> into arguments, similarly to a real life shell.
> 
>> The root of all
>> argument passing issues tends to boil down to how to quote and or escape
>> characters appropriately. Because you know the arguments you can quote
>> correctly but for an unknown user-provided set of arguments (where they
>> do not know how their arguments will flow down to an application) then
>> the appropriate quoting and escaping becomes harder.
>>
>> The problem characters are obviously single and double quotes. Having
>> different quoting rules for any of the layers between a user and
>> the emulated program is always going to cause some confusion so when
>> quoting is necessary I have always found that following the standard
>> argument passing rules of the native application is the least
>> problematic.
> 
> I guess you are generally right, but in this case any quoting is perfectly 
> fine.
> 
>> qemu-system... -semihosting-config "arg=foo bar" -semihosting-config 
>> "arg=second"
>>
>> This should give an argv of ["foo bar", "second"]
> 
> yes, but for the casual user the above syntax is already not natural and even 
> more confusing: should I use "arg=foo bar" or arg="foo bar"? should I repeat 
> -semihosting-config for each argument, or I can group all together 
> (-semihosting-config arg="foo bar",arg="second")?
> 
> 
> if for your use cases this syntax is acceptable, ok, go for it.

Hmm... the -semihosting-config group would be indeed unusual comparing
to the other groups, but on the other hand this would solve also the
double-comma problem. For simple cases the casual user can just stick to
the cmdline.

> 
> 
> my first use of qemu is as a background GDB server started by an Eclipse 
> plug-in. the plug-in will have a single edit field, where the user can enter 
> the semihosting command line args. for args that include spaces, the user 
> should use either single or double quotes, as in any other field.
> 
> my preferred implemention for this would be the original -semihosting-cmdline 
> "some string".
> 
> both proposed implementations (-semihosting-config cmdline="some-string") and 
> even worse for the multiple (-semihosting-config arg=some-string), are way 
> more complicated to use in my Eclipse plug-in, requiring special precautions 
> for passing single/double quotes, commas and possibly equals.
> 
> with the -semihosting-cmdline some-string, the only thing I have to do is to 
> pass [-semihosting-cmdline] in one string array element, and the unmodified 
> [some-string], regardless the quoting used, as the next string array element, 
> and call exec().
> 
> given the above reasons, there are good chances that in my qemu fork (GNU ARM 
> Eclipse QEMU) I'll have to use the separate -semihosting-cmdline option; if 
> you'll accept my patches in the main trunk, ok, if not... no problem; 
> 
> as a conclusion, if you like arg=, go for it, but just be aware that this 
> solution does not cover all use cases properly.

I wasn't focussing on a specific use case, I just thought that having
only one flexible command line option used for passing input arguments
regardless of semi-hosting interface would be ideal. I do understand
however that in your particular case cmdline is more convenient, thus I
personally don’t mind having both options: the user-friendly cmdline and
more flexible arg.

Leon

Re: [Qemu-devel] Getting VM state from outside QEMU?

[Eric Blake]

On 04/08/2015 10:10 AM, Erik Rull wrote:
>>
>> My suggestion is to create a script that sends the QMP command
>> "query-status" an then parse the result. The syntax and output is:
>>
>> -> { "execute": "query-status" }
>> <- { "return": { "running": true, "singlestep": false, "status":
>> "running" } }
>>
> 
> Sounds good - I tried that - but all attempts return that the command has not
> been found. I added the following command line snippet and the results are:
> [...] -qmp tcp:localhost:,server,nowait [...]
> 
> 172.17.48.45 ~ # telnet 127.0.0.1 
> {"QMP": {"version": {"qemu": {"micro": 0, "minor": 1, "major": 2}, "package":
> ""}, "capabilities": []}}

You HAVE to use {"execute":"qmp_capabilities"} (possibly with an
"id":...) as your first command on the monitor, before you can issue any
other command.  I really wish we could improve the error message:

> 
> { "execute": "query-status" }
> {"error": {"class": "CommandNotFound", "desc": "The command query-status has 
> not
> been found"}}

it would be a LOT nicer if we reported 'still in negotiation phase;
"qmp_capabilities" expected' than a bland "CommandNotFound".  Of course,
patches are welcome to improve the experience there!

Similarly, once you are NOT in capabilities negotiation, any subsequent
use of "qmp_capabilities" fails.  That's also something where the error
message could be improved.

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

Re: [Qemu-devel] Getting VM state from outside QEMU?

[Erik Rull]

Hello Paulo,

> On April 7, 2015 at 3:42 PM Paulo Ricardo Paz Vital
>  wrote:
> 
> 
> On Tue, 2015-04-07 at 15:31 +0200, Erik Rull wrote:
> > Hi all,
> > 
> > I need a pretty simple way to get the current state of the VM running in
> > QEMU -
> > I only need the VM state (e.g. running, paused,...). Since my environment
> > does
> > not have any perl, python or other high level scripting capabilities, a
> > simple
> > way e.g. via a shell script would be nice. QEMU is running daemonized, so
> > interacting with the qemu console is not possible.
> > Are there any usable entries in /proc, /dev or /sys that could be used?
> > 
> 
> Hello Erik,
> 
> My suggestion is to create a script that sends the QMP command
> "query-status" an then parse the result. The syntax and output is:
> 
> -> { "execute": "query-status" }
> <- { "return": { "running": true, "singlestep": false, "status":
> "running" } }
> 

Sounds good - I tried that - but all attempts return that the command has not
been found. I added the following command line snippet and the results are:
[...] -qmp tcp:localhost:,server,nowait [...]

172.17.48.45 ~ # telnet 127.0.0.1 
{"QMP": {"version": {"qemu": {"micro": 0, "minor": 1, "major": 2}, "package":
""}, "capabilities": []}}

{ "execute": "query-status" }
{"error": {"class": "CommandNotFound", "desc": "The command query-status has not
been found"}}
{ "execute": "info status" }
{"error": {"class": "CommandNotFound", "desc": "The command info status has not
been found"}}
{ "execute": "info", "arguments": { "status": "" } }
{"error": {"class": "CommandNotFound", "desc": "The command info has not been
found"}}
{ "execute": "query-kvm" }
{"error": {"class": "CommandNotFound", "desc": "The command query-kvm has not
been found"}}

Any ideas what's wrong here?

Thanks.

Best regards,

Erik