Re: [Qemu-devel] [PATCH for-1.7] atomic.h: Fix build with clang

[Paolo Bonzini]

Il 09/11/2013 19:09, Peter Maydell ha scritto:
> Ping! This is needed as a build-fix for MacOSX and didn't
> make it into 1.7-rc0. Paolo, can I get you to review this?

I thought I already had done that, anyway:

>> diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
>> index 0aa8913..492bce1 100644
>> --- a/include/qemu/atomic.h
>> +++ b/include/qemu/atomic.h
>> @@ -168,14 +168,14 @@
>>  #endif
>>
>>  #ifndef atomic_xchg
>> -#ifdef __ATOMIC_SEQ_CST
>> +#if defined(__clang__)
>> +#define atomic_xchg(ptr, i)__sync_swap(ptr, i)
>> +#elif defined(__ATOMIC_SEQ_CST)
>>  #define atomic_xchg(ptr, i)({   \
>>  typeof(*ptr) _new = (i), _old;  \
>>  __atomic_exchange(ptr, &_new, &_old, __ATOMIC_SEQ_CST); \
>>  _old;   \
>>  })
>> -#elif defined __clang__
>> -#define atomic_xchg(ptr, i)__sync_exchange(ptr, i)
>>  #else
>>  /* __sync_lock_test_and_set() is documented to be an acquire barrier only.  
>> */
>>  #define atomic_xchg(ptr, i)(smp_mb(), __sync_lock_test_and_set(ptr, i))

Reviewed-by: Paolo Bonzini 

[Qemu-devel] dump-guest-memory enhancement.

[Phi Debian]

Hi All,

I implemented guest-dump-memory for arm32, and bumped in something
strange, the PT_LOADs generated from dump.c are not target page
aligned. There are some advantage to get PT_LOAD aligned.

This mail is to ask advise about patch I could submit later if wanted.

Would it be desirable to get the PT_LOAD aligned on target page size
always, for all arch ?
If so I could fix dump.c, but may be people using dump-guest-memory on
existing current implementation may object, dunno if some of there
tools (debuggers) are dependent of the non aligned current setup (gdb
would nt complain)

If not wanted for existing arch (s390, ppc, i386), may be I could do
it for arm only.

Or ultimatly may be this is not wanted at all, in this case I keep my
dumper for myself :)

Cheers,
Phi

Re: [Qemu-devel] [PATCH 2/2] exec: make address spaces 64-bit wide

[Michael S. Tsirkin]

On Thu, Nov 07, 2013 at 05:14:37PM +0100, Paolo Bonzini wrote:
> As an alternative to commit 818f86b (exec: limit system memory
> size, 2013-11-04) let's just make all address spaces 64-bit wide.
> This eliminates problems with phys_page_find ignoring bits above
> TARGET_PHYS_ADDR_SPACE_BITS and address_space_translate_internal
> consequently messing up the computations.
> 
> In Luiz's reported crash, at startup gdb attempts to read from address
> 0xffe6 to 0x inclusive.  The region it gets
> is the newly introduced master abort region, which is as big as the PCI
> address space (see pci_bus_init).  Due to a typo that's only 2^63-1,
> not 2^64.  But we get it anyway because phys_page_find ignores the upper
> bits of the physical address.  In address_space_translate_internal then
> 
> diff = int128_sub(section->mr->size, int128_make64(addr));
> *plen = int128_get64(int128_min(diff, int128_make64(*plen)));
> 
> diff becomes negative, and int128_get64 booms.
> 
> The size of the PCI address space region should be fixed anyway.
> 
> Reported-by: Luiz Capitulino 
> Signed-off-by: Paolo Bonzini 

So this causes a 12% performance regression on some TCG
tests, I think we should look into a smarter
datastructure to solve the issues.

> ---
>  exec.c | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 9e2fc4b..d5ce3da 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -89,7 +89,7 @@ struct PhysPageEntry {
>  };
>  
>  /* Size of the L2 (and L3, etc) page tables.  */
> -#define ADDR_SPACE_BITS TARGET_PHYS_ADDR_SPACE_BITS
> +#define ADDR_SPACE_BITS 64
>  
>  #define P_L2_BITS 10
>  #define P_L2_SIZE (1 << P_L2_BITS)
> @@ -1750,11 +1750,7 @@ static void memory_map_init(void)
>  {
>  system_memory = g_malloc(sizeof(*system_memory));
>  
> -assert(ADDR_SPACE_BITS <= 64);
> -
> -memory_region_init(system_memory, NULL, "system",
> -   ADDR_SPACE_BITS == 64 ?
> -   UINT64_MAX : (0x1ULL << ADDR_SPACE_BITS));
> +memory_region_init(system_memory, NULL, "system", UINT64_MAX);
>  address_space_init(&address_space_memory, system_memory, "memory");
>  
>  system_io = g_malloc(sizeof(*system_io));
> -- 
> 1.8.4.2
> 

Re: [Qemu-devel] [RFC PATCH] i386: Add _PXM method to ACPI CPU objects

[Michael S. Tsirkin]

On Fri, Nov 08, 2013 at 06:33:12PM +0100, Igor Mammedov wrote:
> On Fri, 8 Nov 2013 12:22:12 +0200
> Vasilis Liaskovitis  wrote:
> 
> > Hi,
> > 
> > On Thu, Nov 07, 2013 at 03:03:42PM +0200, Michael S. Tsirkin wrote:
> > > On Thu, Nov 07, 2013 at 01:41:59PM +0100, Vasilis Liaskovitis wrote:
> > > > This patch adds a _PXM method to ACPI CPU objects for the pc machine. 
> > > > The _PXM
> > > > value is derived from the passed in guest info, same way as CPU SRAT 
> > > > entries.
> > > > 
> > > > The motivation for this patch is a CPU hot-unplug/hot-plug bug observed 
> > > > when
> > > > using a 3.11 linux guest kernel on a multi-NUMA node qemu/kvm VM. The 
> > > > linux
> > > > guest kernel parses the SRAT CPU entries at boot time and stores them 
> > > > in the
> > > > array __apicid_to_node. When a CPU is hot-removed, the linux guest 
> > > > kernel
> > > > resets the removed CPU's __apicid_to_node entry to NO_NUMA_NODE (kernel 
> > > > commit
> > > > c4c60524). When the removed cpu is hot-added again, the linux kernel 
> > > > looks up
> > > > the hot-added cpu object's _PXM method instead of somehow 
> > > > re-discovering the
> > > > SRAT entry info. With current qemu/seabios, the _PXM method is not 
> > > > found, and
> > > > the CPU is thus hot-plugged in the default NUMA node 0. (The problem 
> > > > does not
> > > > show up on initial hotplug of a cpu; the PXM method is still not found 
> > > > in this
> > > > case, but the kernel still has the correct proximity value from the 
> > > > CPU's SRAT
> > > > entry stored in __apicid_to_node)
> > > > 
> > > > ACPI spec mentions that the _PXM method is the correct way to determine
> > > > proximity information at hot-add time.
> > > 
> > > Where does it say this?
> > > I found this:
> > > If the Local APIC ID / Local SAPIC ID / Local x2APIC ID of a dynamically
> > > added processor is not present in the System Resource Affinity Table
> > > (SRAT), a _PXM object must exist for the processor’s device or one of
> > > its ancestors in the ACPI Namespace.
> > > 
> > > Does this mean that linux is buggy, and should be fixed up to look up
> > > the apic ID in SRAT?
> > 
> > The quote above suggests that if SRAT is absent, _PXM should be present.
> > Seabios/qemu provide SRAT entries, and  no _PXM. The fact that the kernel
> > resets the parse SRAT info on hot-remove time looks like a kernel problem.
> > 
> > But As Toshi Kani mentioned in the original thread, here is a quote from 
> > ACPI
> > 5.0, stating _PXM and only _PXM should be used at hot-plug time:
> > 
> > ===
> > 17.2.1 System Resource Affinity Table Definition
> > 
> > This optional System Resource Affinity Table (SRAT) provides the boot
> > time description of the processor and memory ranges belonging to a
> > system locality. OSPM will consume the SRAT only at boot time. OSPM
> > should use _PXM for any devices that are hot-added into the system after
> > boot up.
> > 
> > 
> > So in this sense, the kernel is correct (kernel only uses _PXM at hot-plug 
> > time)
> > , and qemu/Seabios should have _PXM methods for hot operations.
> 
> in terms of RFC SHOULD doesn't mean MUST, and in my interpretation of above is
> that SRAT parsed once but it doesn't mean that OS should forget data from it.

Well it says "OSPM will consume the SRAT only at boot time".
How do you interpret that?

> Anyway we surely can have both in QEMU.
> > 
> > > 
> > > > So far, qemu/seabios do not provide this
> > > > method for CPUs. So regardless of kernel behaviour, it is a good idea 
> > > > to add
> > > > this _PXM method. Since ACPI table generation has recently been moved 
> > > > from
> > > > seabios to qemu, we do this in qemu.
> > > > 
> > > > Note that the above hot-remove/hot-add scenario has been tested on an 
> > > > older
> > > > qemu + non-upstreamed patches for cpu hot-removal support, and not on 
> > > > qemu
> > > > master (since cpu-del support is still not on master). The only testing 
> > > > done
> > > > with qemu/seabios master and this patch, are successful boots of 
> > > > multi-node
> > > > linux and windows8 guests.
> > > > 
> > > > For the initial discussion on seabios and linux-acpi lists see
> > > > http://www.spinics.net/lists/linux-acpi/msg47058.html
> > > > 
> > > > Signed-off-by: Vasilis Liaskovitis 
> > > > 
> > > > Reviewed-by: Thilo Fromm 
> > > 
> > > Even if this is a linux bug, I have no issue with working around
> > > it in qemu.
> > > 
> > > But I think proper testing needs to be done with rebased upport for 
> > > cpu-del.
> > 
> > Ok, I can try to rebase cpu-del support for testing. If there are cpu-del 
> > bits
> > already somewhere (Igor?) and not merged yet, please point me to them.
> > 
> > > 
> > > > ---
> > > >  hw/i386/acpi-build.c  |2 ++
> > > >  hw/i386/ssdt-proc.dsl |2 ++
> > > >  2 files changed, 4 insertions(+)
> > > > 
> > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > > index 6cfa044..9373f5e 100644
> > > > --- a/hw/i386/acpi-build.c
> 

Re: [Qemu-devel] [PATCH] e1000/rtl8139: update HMP NIC when every bit is written

[Michael S. Tsirkin]

On Fri, Nov 08, 2013 at 02:42:27PM -0500, Vlad Yasevich wrote:
> What about this approach?  This only updates the monitory when all the
> bits have been written to.
> 
> -vlad


Thanks!
Some comments below.

> -- >8 --
> Subject: [PATCH] e1000/rtl8139: update HMP NIC when every bit is written
> 
> We currently just update the HMP NIC info when the last bit of macaddr
> is written. This assumes that guest driver will write all the macaddr
> from bit 0 to bit 5 when it changes the macaddr, this is the current
> behavior of linux driver (e1000/rtl8139cp), but we can't do this
> assumption.

I would rather say "it seems better not to make this assumption".
This does look somewhat safer than what Amos proposed.

> 
> The macaddr that is used for rx-filter will be updated when every bit
> is changed. This patch updates the e1000/rtl8139 nic to update HMP NIC
> info when every bit has been changed. It will be same as virtio-net.
> 
> Signed-off-by: Vlad Yasevich 
> ---
>  hw/net/e1000.c   | 17 -
>  hw/net/rtl8139.c | 14 +-
>  2 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 8387443..a5967ed 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -149,6 +149,10 @@ typedef struct E1000State_st {
>  #define E1000_FLAG_AUTONEG (1 << E1000_FLAG_AUTONEG_BIT)
>  #define E1000_FLAG_MIT (1 << E1000_FLAG_MIT_BIT)
>  uint32_t compat_flags;
> +uint32_t mac_changed;

Hmm why uint32_t? uint8_t is enough here isn't?

This new state has to be migrated then, and
we need to fallback to old behaviour if migrating to/from
an old version (see compat_flags for one way to
detect this compatibility mode).

> +#define E1000_RA0_CHANGED 0
> +#define E1000_RA1_CHANGED 1
> +#define E1000_RA_ALL_CHANGED (E1000_RA0_CHANGED|E1000_RA1_CHANGED)

I don't get it. So E1000_RA_ALL_CHANGED is 0 | 1 == 1.
it looks like the trigger is when E1000_RA1_CHANGED
so that's more or less equivalent.

>  } E1000State;
>  
>  #define TYPE_E1000 "e1000"
> @@ -402,6 +406,7 @@ static void e1000_reset(void *opaque)
>  d->mac_reg[RA + 1] |= (i < 2) ? macaddr[i + 4] << (8 * i) : 0;
>  }
>  qemu_format_nic_info_str(qemu_get_queue(d->nic), macaddr);
> +d->mac_changed = 0;
>  }
>  
>  static void
> @@ -1106,10 +,20 @@ mac_writereg(E1000State *s, int index, uint32_t val)
>  
>  s->mac_reg[index] = val;
>  
> -if (index == RA + 1) {
> +switch (index) {
> +case RA:
> +s->mac_changed |= E1000_RA0_CHANGED;
> +break;
> +case (RA + 1):
> +s->mac_changed |= E1000_RA1_CHANGED;
> +break;
> +}
> +
> +if (s->mac_changed ==  E1000_RA_ALL_CHANGED) {

Some whitespace damage here.

>  macaddr[0] = cpu_to_le32(s->mac_reg[RA]);
>  macaddr[1] = cpu_to_le32(s->mac_reg[RA + 1]);
>  qemu_format_nic_info_str(qemu_get_queue(s->nic), (uint8_t *)macaddr);
> + s->mac_changed = 0;

Need to use spaces for indent in qemu.

>  }
>  }
>  
> diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c

Best to split out in a separate commit.

> index 5329f44..6dac10c 100644
> --- a/hw/net/rtl8139.c
> +++ b/hw/net/rtl8139.c
> @@ -476,6 +476,8 @@ typedef struct RTL8139State {
>  
>  uint16_t CpCmd;
>  uint8_t  TxThresh;
> +uint8_t  mac_changed;

This new state has to be migrated then, and
we need to fallback to old behaviour if migrating to/from
an old version.

> +#define RTL8139_MAC_CHANGED_ALL 0x3F
>  
>  NICState *nic;
>  NICConf conf;
> @@ -1215,6 +1217,7 @@ static void rtl8139_reset(DeviceState *d)
>  /* restore MAC address */
>  memcpy(s->phys, s->conf.macaddr.a, 6);
>  qemu_format_nic_info_str(qemu_get_queue(s->nic), s->phys);
> +s->mac_changed = 0;
>  
>  /* reset interrupt mask */
>  s->IntrStatus = 0;
> @@ -2741,12 +2744,13 @@ static void rtl8139_io_writeb(void *opaque, uint8_t 
> addr, uint32_t val)
>  
>  switch (addr)
>  {
> -case MAC0 ... MAC0+4:
> -s->phys[addr - MAC0] = val;
> -break;
> -case MAC0+5:
> +case MAC0 ... MAC0+5:
>  s->phys[addr - MAC0] = val;
> -qemu_format_nic_info_str(qemu_get_queue(s->nic), s->phys);
> +s->mac_changed |= (1 << (addr - MAC0));

Better drop the external () here otherwise it starts looking like lisp :)

> +if (s->mac_changed == RTL8139_MAC_CHANGED_ALL) {
> +qemu_format_nic_info_str(qemu_get_queue(s->nic), s->phys);
> +s->mac_changed = 0;
> +}
>  break;
>  case MAC0+6 ... MAC0+7:
>  /* reserved */
> -- 
> 1.8.4.2

[Qemu-devel] [PATCH for-1.7 0/2] revert master abort related patches

[Marcel Apfelbaum]

The master-abort patch introduced a background memory region
covering all 64 bit pci address space, the visible parts
being the unused pci-holes addresses.

The patch revealed the following issues:
 1. Some memory regions have INT64_MAX size, but the size
was supposed to be UINT64_MAX (meaning that the
region covers all 64 bit address space). Having
a region that is not even a multiple of PAGE_SIZE
is really not what we want.
 2. exec.c does not support all the 64 bit address range
and when using an unsupported address, it leads to
page tables corruption.
 3. Some memory regions overlap and the visible region
is selected by chance (the algorithm implementation)
and not by the memory API:
- selecting a proper priority
- arrange the regions that are not supposed to overlap.

This series reverts this patch and another related patch
because the impact for 1.7 is too big.
After the issues above are solved, the patch can finally
be applied.

Marcel Apfelbaum (1):
  Revert "hw/pci: partially handle pci master abort"

Michael S. Tsirkin (1):
  Revert "exec: limit system memory size"

 exec.c   |  7 +--
 hw/pci/pci.c | 26 --
 include/hw/pci/pci_bus.h |  1 -
 3 files changed, 1 insertion(+), 33 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH for-1.7 1/2] Revert "hw/pci: partially handle pci master abort"

[Marcel Apfelbaum]

This reverts commit a53ae8e934cd54686875b5bcfc2f434244ee55d6.

The master-abort patch introduced a background memory region
covering all 64 bit pci address space, the visible parts
being the unused pci-holes addresses.

The patch revealed the following issues:
 1. Some memory regions have INT64_MAX size, but the size
was supposed to be UINT64_MAX (meaning that the
region covers all 64 bit address space). Having
a region that is not even a multiple of PAGE_SIZE
is really not what we want.
 2. exec.c does not support all the 64 bit address range
and when using an unsupported address, it leads to
page tables corruption.
 3. Some memory regions overlap and the visible region
is selected by chance (the algorithm implementation)
and not by the memory API:
- selecting a proper priority
- arrange the regions that are not supposed to overlap.

The patch is reverted because the impact for 1.7 is too big.
After the issues above are solved, the patch can finally
be applied.


Signed-off-by: Marcel Apfelbaum 
---
 hw/pci/pci.c | 26 --
 include/hw/pci/pci_bus.h |  1 -
 2 files changed, 27 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index a98c8a0..ed32059 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -283,24 +283,6 @@ const char *pci_root_bus_path(PCIDevice *dev)
 return rootbus->qbus.name;
 }
 
-static uint64_t master_abort_mem_read(void *opaque, hwaddr addr, unsigned size)
-{
-   return -1ULL;
-}
-
-static void master_abort_mem_write(void *opaque, hwaddr addr, uint64_t val,
-   unsigned size)
-{
-}
-
-static const MemoryRegionOps master_abort_mem_ops = {
-.read = master_abort_mem_read,
-.write = master_abort_mem_write,
-.endianness = DEVICE_LITTLE_ENDIAN,
-};
-
-#define MASTER_ABORT_MEM_PRIORITY INT_MIN
-
 static void pci_bus_init(PCIBus *bus, DeviceState *parent,
  const char *name,
  MemoryRegion *address_space_mem,
@@ -312,14 +294,6 @@ static void pci_bus_init(PCIBus *bus, DeviceState *parent,
 bus->address_space_mem = address_space_mem;
 bus->address_space_io = address_space_io;
 
-
-memory_region_init_io(&bus->master_abort_mem, OBJECT(bus),
-  &master_abort_mem_ops, bus, "pci-master-abort",
-  memory_region_size(bus->address_space_mem));
-memory_region_add_subregion_overlap(bus->address_space_mem,
-0, &bus->master_abort_mem,
-MASTER_ABORT_MEM_PRIORITY);
-
 /* host bridge */
 QLIST_INIT(&bus->child);
 
diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
index 2ad5edb..9df1788 100644
--- a/include/hw/pci/pci_bus.h
+++ b/include/hw/pci/pci_bus.h
@@ -23,7 +23,6 @@ struct PCIBus {
 PCIDevice *parent_dev;
 MemoryRegion *address_space_mem;
 MemoryRegion *address_space_io;
-MemoryRegion master_abort_mem;
 
 QLIST_HEAD(, PCIBus) child; /* this will be replaced by qdev later */
 QLIST_ENTRY(PCIBus) sibling;/* this will be replaced by qdev later */
-- 
1.8.3.1

[Qemu-devel] [PATCH for-1.7 2/2] Revert "exec: limit system memory size"

[Marcel Apfelbaum]

From: "Michael S. Tsirkin" 

This reverts commit 818f86b88394b7b2b59d313e51043fe15a8004db.

This was a work-around for bugs elsewhere in the system,
exposed by commit a53ae8e934cd54686875b5bcfc2f434244ee55d6:
"hw/pci: partially handle pci master abort"
since that's reverted now, the work-around is not required for 1.7
anymore.
The proper fix is supporting full 64 bit addresses in the radix tree.

Signed-off-by: Michael S. Tsirkin 
Signed-off-by: Marcel Apfelbaum 
---
 exec.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/exec.c b/exec.c
index 79610ce..b453713 100644
--- a/exec.c
+++ b/exec.c
@@ -1741,12 +1741,7 @@ void address_space_destroy_dispatch(AddressSpace *as)
 static void memory_map_init(void)
 {
 system_memory = g_malloc(sizeof(*system_memory));
-
-assert(TARGET_PHYS_ADDR_SPACE_BITS <= 64);
-
-memory_region_init(system_memory, NULL, "system",
-   TARGET_PHYS_ADDR_SPACE_BITS == 64 ?
-   UINT64_MAX : (0x1ULL << TARGET_PHYS_ADDR_SPACE_BITS));
+memory_region_init(system_memory, NULL, "system", INT64_MAX);
 address_space_init(&address_space_memory, system_memory, "memory");
 
 system_io = g_malloc(sizeof(*system_io));
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH for-1.7 0/2] revert master abort related patches

[Michael S. Tsirkin]

On Sun, Nov 10, 2013 at 02:15:23PM +0200, Marcel Apfelbaum wrote:
> The master-abort patch introduced a background memory region
> covering all 64 bit pci address space, the visible parts
> being the unused pci-holes addresses.
> 
> The patch revealed the following issues:
>  1. Some memory regions have INT64_MAX size, but the size
> was supposed to be UINT64_MAX (meaning that the
> region covers all 64 bit address space). Having
> a region that is not even a multiple of PAGE_SIZE
> is really not what we want.
>  2. exec.c does not support all the 64 bit address range
> and when using an unsupported address, it leads to
> page tables corruption.
>  3. Some memory regions overlap and the visible region
> is selected by chance (the algorithm implementation)
> and not by the memory API:
> - selecting a proper priority
> - arrange the regions that are not supposed to overlap.
> 
> This series reverts this patch and another related patch
> because the impact for 1.7 is too big.
> After the issues above are solved, the patch can finally
> be applied.

I edited the commit log slightly and applied this, thanks.

> Marcel Apfelbaum (1):
>   Revert "hw/pci: partially handle pci master abort"
> 
> Michael S. Tsirkin (1):
>   Revert "exec: limit system memory size"
> 
>  exec.c   |  7 +--
>  hw/pci/pci.c | 26 --
>  include/hw/pci/pci_bus.h |  1 -
>  3 files changed, 1 insertion(+), 33 deletions(-)
> 
> -- 
> 1.8.3.1

[Qemu-devel] [PATCH 2/2] trace: Remove trace.h from hw/usb/hcd-ehci.h (less dependencies)

[Stefan Weil]

This reduces the dependencies on trace.h.
Only one source file which needs hcd-ehci.h also needs trace.h.

Signed-off-by: Stefan Weil 
---

This patch reduces the number of .o files which depend on trace.h
in a default QEMU build from 382 to 379.

That's not a big reduction, but now there is no longer a .h file
which directly includes trace.h, so future improvements (splitting
trace headers, one header per source file, no longer compile 379
files after each change of trace-events) are easier to implement.

 hw/usb/hcd-ehci.c |1 +
 hw/usb/hcd-ehci.h |1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 22bdbf4..0ba38c9 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -28,6 +28,7 @@
  */
 
 #include "hw/usb/hcd-ehci.h"
+#include "trace.h"
 
 /* Capability Registers Base Address - section 2.2 */
 #define CAPLENGTH0x  /* 1-byte, 0x0001 reserved */
diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index 065c9fa..1ad4b96 100644
--- a/hw/usb/hcd-ehci.h
+++ b/hw/usb/hcd-ehci.h
@@ -21,7 +21,6 @@
 #include "qemu/timer.h"
 #include "hw/usb.h"
 #include "monitor/monitor.h"
-#include "trace.h"
 #include "sysemu/dma.h"
 #include "sysemu/sysemu.h"
 #include "hw/pci/pci.h"
-- 
1.7.9.5

[Qemu-devel] [PATCH 1/2] trace: Remove trace.h from console.h (less dependencies)

[Stefan Weil]

This reduces the dependencies on trace.h.
Only two source files which need console.h also need trace.h.

Signed-off-by: Stefan Weil 
---

Without this patch, 449 .o files depend on trace.h and must
be rebuild whenever the file trace-events is changed.

The patch reduces this number to 382 for the default QEMU build.

 hw/display/vmware_vga.c |1 +
 include/ui/console.h|1 -
 ui/console.c|1 +
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index a6a8cdc..aba292c 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -23,6 +23,7 @@
  */
 #include "hw/hw.h"
 #include "hw/loader.h"
+#include "trace.h"
 #include "ui/console.h"
 #include "hw/pci/pci.h"
 
diff --git a/include/ui/console.h b/include/ui/console.h
index 98edf41..4156a87 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -6,7 +6,6 @@
 #include "qapi/qmp/qdict.h"
 #include "qemu/notify.h"
 #include "monitor/monitor.h"
-#include "trace.h"
 #include "qapi-types.h"
 #include "qapi/error.h"
 
diff --git a/ui/console.c b/ui/console.c
index aad4fc9..11d5e6a 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -27,6 +27,7 @@
 #include "qemu/timer.h"
 #include "qmp-commands.h"
 #include "sysemu/char.h"
+#include "trace.h"
 
 //#define DEBUG_CONSOLE
 #define DEFAULT_BACKSCROLL 512
-- 
1.7.9.5

[Qemu-devel] [PATCH] console: Remove unused debug code

[Stefan Weil]

The local function console_print_text_attributes is no longer used since
commit 7d6ba01c3741bc32ae252bf64a5fd3f930c2df4f.

Signed-off-by: Stefan Weil 
---
 ui/console.c |   33 -
 1 file changed, 33 deletions(-)

diff --git a/ui/console.c b/ui/console.c
index 11d5e6a..61ed219 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -410,39 +410,6 @@ static const pixman_color_t color_table_rgb[2][8] = {
 }
 };
 
-#ifdef DEBUG_CONSOLE
-static void console_print_text_attributes(TextAttributes *t_attrib, char ch)
-{
-if (t_attrib->bold) {
-printf("b");
-} else {
-printf(" ");
-}
-if (t_attrib->uline) {
-printf("u");
-} else {
-printf(" ");
-}
-if (t_attrib->blink) {
-printf("l");
-} else {
-printf(" ");
-}
-if (t_attrib->invers) {
-printf("i");
-} else {
-printf(" ");
-}
-if (t_attrib->unvisible) {
-printf("n");
-} else {
-printf(" ");
-}
-
-printf(" fg: %d bg: %d ch:'%2X' '%c'\n", t_attrib->fgcol, t_attrib->bgcol, 
ch, ch);
-}
-#endif
-
 static void vga_putcharxy(QemuConsole *s, int x, int y, int ch,
   TextAttributes *t_attrib)
 {
-- 
1.7.10.4

[Qemu-devel] [PATCH] console: Replace conditional debug messages by trace methods

[Stefan Weil]

Signed-off-by: Stefan Weil 
---

This patch needs my previous patch which added trace.h, see
http://patchwork.ozlabs.org/patch/290045/.

 trace-events |2 ++
 ui/console.c |   11 +++
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/trace-events b/trace-events
index 8695e9e..d196234 100644
--- a/trace-events
+++ b/trace-events
@@ -1010,6 +1010,8 @@ dma_map_wait(void *dbs) "dbs=%p"
 
 # ui/console.c
 console_gfx_new(void) ""
+console_putchar_csi(int esc_param0, int esc_param1, int ch, int nb_esc_params) 
"escape sequence CSI%d;%d%c, %d parameters"
+console_putchar_unhandled(int ch) "unhandled escape character '%c'"
 console_txt_new(int w, int h) "%dx%d"
 console_select(int nr) "%d"
 console_refresh(int interval) "interval %d ms"
diff --git a/ui/console.c b/ui/console.c
index 61ed219..586fc6d 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -29,7 +29,6 @@
 #include "sysemu/char.h"
 #include "trace.h"
 
-//#define DEBUG_CONSOLE
 #define DEFAULT_BACKSCROLL 512
 #define MAX_CONSOLES 12
 #define CONSOLE_CURSOR_PERIOD 500
@@ -867,10 +866,8 @@ static void console_putchar(QemuConsole *s, int ch)
 s->nb_esc_params++;
 if (ch == ';')
 break;
-#ifdef DEBUG_CONSOLE
-fprintf(stderr, "escape sequence CSI%d;%d%c, %d parameters\n",
-s->esc_params[0], s->esc_params[1], ch, s->nb_esc_params);
-#endif
+trace_console_putchar_csi(s->esc_params[0], s->esc_params[1],
+  ch, s->nb_esc_params);
 s->state = TTY_STATE_NORM;
 switch(ch) {
 case 'A':
@@ -984,9 +981,7 @@ static void console_putchar(QemuConsole *s, int ch)
 s->y = s->y_saved;
 break;
 default:
-#ifdef DEBUG_CONSOLE
-fprintf(stderr, "unhandled escape character '%c'\n", ch);
-#endif
+trace_console_putchar_unhandled(ch);
 break;
 }
 break;
-- 
1.7.10.4

[Qemu-devel] [PATCH] gtk: Replace conditional debug messages by trace methods

[Stefan Weil]

Signed-off-by: Stefan Weil 
---
 trace-events |5 +
 ui/gtk.c |   19 +--
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/trace-events b/trace-events
index 8695e9e..360f684 100644
--- a/trace-events
+++ b/trace-events
@@ -1020,6 +1020,11 @@ displaychangelistener_register(void *dcl, const char 
*name) "%p [ %s ]"
 displaychangelistener_unregister(void *dcl, const char *name) "%p [ %s ]"
 ppm_save(const char *filename, void *display_surface) "%s surface=%p"
 
+# ui/gtk.c
+gd_switch(int width, int height) "width=%d, height=%d"
+gd_update(int x, int y, int w, int h) "x=%d, y=%d, w=%d, h=%d"
+gd_key_event(int gdk_keycode, int qemu_keycode, const char *action) 
"translated GDK keycode %d to QEMU keycode %d (%s)"
+
 # hw/display/vmware_vga.c
 vmware_value_read(uint32_t index, uint32_t value) "index %d, value 0x%x"
 vmware_value_write(uint32_t index, uint32_t value) "index %d, value 0x%x"
diff --git a/ui/gtk.c b/ui/gtk.c
index b5f4f0b..6316f5b 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -53,6 +53,7 @@
 #include 
 #include 
 
+#include "trace.h"
 #include "ui/console.h"
 #include "sysemu/sysemu.h"
 #include "qmp-commands.h"
@@ -60,14 +61,6 @@
 #include "keymaps.h"
 #include "sysemu/char.h"
 
-//#define DEBUG_GTK
-
-#ifdef DEBUG_GTK
-#define DPRINTF(fmt, ...) printf(fmt, ## __VA_ARGS__)
-#else
-#define DPRINTF(fmt, ...) do { } while (0)
-#endif
-
 #define MAX_VCS 10
 
 
@@ -302,7 +295,7 @@ static void gd_update(DisplayChangeListener *dcl,
 int fbw, fbh;
 int ww, wh;
 
-DPRINTF("update(x=%d, y=%d, w=%d, h=%d)\n", x, y, w, h);
+trace_gd_update(x, y, w, h);
 
 if (s->convert) {
 pixman_image_composite(PIXMAN_OP_SRC, s->ds->image, NULL, s->convert,
@@ -396,8 +389,7 @@ static void gd_switch(DisplayChangeListener *dcl,
 GtkDisplayState *s = container_of(dcl, GtkDisplayState, dcl);
 bool resized = true;
 
-DPRINTF("resize(width=%d, height=%d)\n",
-surface_width(surface), surface_height(surface));
+trace_gd_switch(surface_width(surface), surface_height(surface));
 
 if (s->surface) {
 cairo_surface_destroy(s->surface);
@@ -732,9 +724,8 @@ static gboolean gd_key_event(GtkWidget *widget, GdkEventKey 
*key, void *opaque)
 qemu_keycode = 0;
 }
 
-DPRINTF("translated GDK keycode %d to QEMU keycode %d (%s)\n",
-gdk_keycode, qemu_keycode,
-(key->type == GDK_KEY_PRESS) ? "down" : "up");
+trace_gd_key_event(gdk_keycode, qemu_keycode,
+   (key->type == GDK_KEY_PRESS) ? "down" : "up");
 
 for (i = 0; i < ARRAY_SIZE(modifier_keycode); i++) {
 if (qemu_keycode == modifier_keycode[i]) {
-- 
1.7.10.4

[Qemu-devel] Buildbot failures (XEN)

[Stefan Weil]

Hi Stefan, hi Alex,

could you please have a look at these buildbots (maybe others, too):

xen_i386_debian_6_0
xen_x86_64_debian_6_0

Both fail during the configuration:

ERROR: unknown option --disable-debug-info

Either the git repository should be updated (it is obviously more than a
year old), or configure must not use --disable-debug-info, or the
buildbots might be removed because they are no longer considered to be
useful.

Kind regards,
Stefan

[Qemu-devel] [Bug 685096] Re: USB Passthrough not working for Windows 7 guest

[debfreak]

I have the same problem. I tried it with qemu 1.4 and the last 1.6.0-dfsg-2 on 
a debian testing system. Win 7 says always "This device cannot start. (Code 
10)". I tried a lot of usb sticks but always the same...
I hope there will be sometime a solution for this :( I wait over a year in the 
hope that this will work.

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

Title:
  USB Passthrough not working for Windows 7 guest

Status in QEMU:
  Confirmed
Status in “qemu-kvm” package in Ubuntu:
  Confirmed

Bug description:
  USB Passthrough from host to guest is not working for a 32-bit Windows
  7 guest, while it works perfectly for a 32-bit Windows XP guest.

  The device appears in the device manager of Windows 7, but with "Error
  code 10: device cannot start". I have tried this with numerous USB
  thumbdrives and a USB wireless NIC, all with the same result. The
  device name and functionality is recognized, so at least some USB
  negotiation is taking place.

  I am trying this with the latest git-pull of QEMU-KVM.

  The command line to launch qemu-kvm for win7 is:
  sudo /home/user/local_install/bin/qemu-system-x86_64 -cpu core2duo -m 1024 
-smp 2 -vga std -hda ./disk_images/win7.qcow -vnc :1 -boot c -usb -usbdevice 
tablet -usbdevice host:0781:5150

  The command line to launch qemu-kvm for winxp is:
  sudo /home/user/local_install/bin/qemu-system-x86_64 -cpu core2duo -m 1024 
-smp 2 -usb -vga std -hda ./winxpsp3.qcow -vnc :0 -boot c -usbdevice tablet 
-usbdevice host:0781:5150

  Any help is appreciated.

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

[Qemu-devel] [PATCH for 1.7] qga: Fix compilation for old versions of MinGW

[Stefan Weil]

While MinGW-w64 can compile the qga code, MinGW from Ubuntu lenny
(gcc-mingw32 4.4.2-3) shows these errors:

In file included from qga/vss-win32.c:17:
qga/vss-win32/requester.h:31:
 error: expected »=«, »,«, »;«, »asm« or »__attribute__« before »requester_init«
qga/vss-win32/requester.h:32:
 error: expected »=«, »,«, »;«, »asm« or »__attribute__« before 
»requester_deinit«

The macro STDAPI is unknown, so add the missing include file which
defines it.

Signed-off-by: Stefan Weil 
---

This patch also fixes the same error on the MinGW buildbot.
I did not notice it earlier because I usually compile with MinGW-w64.

Regards,
Stefan

 qga/vss-win32/requester.h |1 +
 1 file changed, 1 insertion(+)

diff --git a/qga/vss-win32/requester.h b/qga/vss-win32/requester.h
index cffec01..374f9b8 100644
--- a/qga/vss-win32/requester.h
+++ b/qga/vss-win32/requester.h
@@ -13,6 +13,7 @@
 #ifndef VSS_WIN32_REQUESTER_H
 #define VSS_WIN32_REQUESTER_H
 
+#include/* STDAPI */
 #include "qemu/compiler.h"
 
 #ifdef __cplusplus
-- 
1.7.10.4

[Qemu-devel] Buildbot failures (MinGW)

[Stefan Weil]

Hello Gerd,

could you please install bison and flex on the buildbot machine for
default_mingw?
That would fix some errors in the dtc build:

 BISON dtc-parser.tab.c
make[1]: bison: Command not found
 LEX dtc-lexer.lex.c
make[1]: flex: Command not found

See
http://buildbot.b1-systems.de/qemu/builders/default_mingw32/builds/773/steps/compile/logs/stdio
for the context.

The build will still fail when compiling qga/vss-win32.c - I have
already sent a patch for that error which hopefully
will get merged soon.I'd appreciate if more of the buildbots would at
least compile QEMU 1.7.

Regards,
Stefan

[Qemu-devel] [PATCH] ui/sdl2 : initial port to SDL 2.0

[Dave Airlie]

From: Dave Airlie 

I've ported the SDL1.2 code over, and rewritten it to use the SDL2 interface.

The biggest changes were in the input handling, where SDL2 has done a major
overhaul, and I've had to include a generated translation file to get from
SDL2 codes back to qemu compatible ones. I'm still not sure how the keyboard
layout code works in qemu, so there may be further work if someone can point
me a test case that works with SDL1.2 and doesn't with SDL2.

Some SDL env vars we used to set are no longer used by SDL2,
Windows, OSX support is untested,

I don't think we can link to SDL1.2 and SDL2 at the same time, so I felt
using --with-sdlabi=2.0 to select the new code should be fine, like how
gtk does it.

Signed-off-by: Dave Airlie 
---
 configure|  23 +-
 ui/Makefile.objs |   4 +-
 ui/sdl.c |   3 +
 ui/sdl2.c| 889 +++
 ui/sdl2_scancode_translate.h | 260 +
 ui/sdl_keysym.h  |   3 +-
 6 files changed, 1175 insertions(+), 7 deletions(-)
 create mode 100644 ui/sdl2.c
 create mode 100644 ui/sdl2_scancode_translate.h

diff --git a/configure b/configure
index 9addff1..bf3be37 100755
--- a/configure
+++ b/configure
@@ -158,6 +158,7 @@ docs=""
 fdt=""
 pixman=""
 sdl=""
+sdlabi="1.2"
 virtfs=""
 vnc="yes"
 sparse="no"
@@ -310,6 +311,7 @@ query_pkg_config() {
 }
 pkg_config=query_pkg_config
 sdl_config="${SDL_CONFIG-${cross_prefix}sdl-config}"
+sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}"
 
 # default flags for all hosts
 QEMU_CFLAGS="-fno-strict-aliasing $QEMU_CFLAGS"
@@ -710,6 +712,8 @@ for opt do
   ;;
   --enable-sdl) sdl="yes"
   ;;
+  --with-sdlabi=*) sdlabi="$optarg"
+  ;;
   --disable-qom-cast-debug) qom_cast_debug="no"
   ;;
   --enable-qom-cast-debug) qom_cast_debug="yes"
@@ -1092,6 +1096,7 @@ echo "  --disable-strip  disable stripping 
binaries"
 echo "  --disable-werror disable compilation abort on warning"
 echo "  --disable-sdldisable SDL"
 echo "  --enable-sdl enable SDL"
+echo "  --with-sdlabiselect preferred SDL ABI 1.2 or 2.0"
 echo "  --disable-gtkdisable gtk UI"
 echo "  --enable-gtk enable gtk UI"
 echo "  --disable-virtfs disable VirtFS"
@@ -1751,12 +1756,22 @@ fi
 
 # Look for sdl configuration program (pkg-config or sdl-config).  Try
 # sdl-config even without cross prefix, and favour pkg-config over sdl-config.
-if test "`basename $sdl_config`" != sdl-config && ! has ${sdl_config}; then
-  sdl_config=sdl-config
+
+if test $sdlabi == "2.0"; then
+sdl_config=$sdl2_config
+sdlname=sdl2
+sdlconfigname=sdl2_config
+else
+sdlname=sdl
+sdlconfigname=sdl_config
+fi
+
+if test "`basename $sdl_config`" != $sdlconfigname && ! has ${sdl_config}; then
+  sdl_config=$sdlconfigname
 fi
 
-if $pkg_config sdl --exists; then
-  sdlconfig="$pkg_config sdl"
+if $pkg_config $sdlname --exists; then
+  sdlconfig="$pkg_config $sdlname"
   _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
 elif has ${sdl_config}; then
   sdlconfig="$sdl_config"
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index f33be47..721ad37 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -9,12 +9,12 @@ vnc-obj-y += vnc-jobs.o
 
 common-obj-y += keymaps.o console.o cursor.o input.o qemu-pixman.o
 common-obj-$(CONFIG_SPICE) += spice-core.o spice-input.o spice-display.o
-common-obj-$(CONFIG_SDL) += sdl.o sdl_zoom.o x_keymap.o
+common-obj-$(CONFIG_SDL) += sdl.o sdl_zoom.o x_keymap.o sdl2.o
 common-obj-$(CONFIG_COCOA) += cocoa.o
 common-obj-$(CONFIG_CURSES) += curses.o
 common-obj-$(CONFIG_VNC) += $(vnc-obj-y)
 common-obj-$(CONFIG_GTK) += gtk.o x_keymap.o
 
-$(obj)/sdl.o $(obj)/sdl_zoom.o: QEMU_CFLAGS += $(SDL_CFLAGS) 
+$(obj)/sdl.o $(obj)/sdl_zoom.o $(obj)/sdl2.o: QEMU_CFLAGS += $(SDL_CFLAGS) 
 
 $(obj)/gtk.o: QEMU_CFLAGS += $(GTK_CFLAGS) $(VTE_CFLAGS)
diff --git a/ui/sdl.c b/ui/sdl.c
index 9d8583c..736bb95 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -26,6 +26,8 @@
 #undef WIN32_LEAN_AND_MEAN
 
 #include 
+
+#if SDL_MAJOR_VERSION == 1
 #include 
 
 #include "qemu-common.h"
@@ -966,3 +968,4 @@ void sdl_display_init(DisplayState *ds, int full_screen, 
int no_frame)
 
 atexit(sdl_cleanup);
 }
+#endif
diff --git a/ui/sdl2.c b/ui/sdl2.c
new file mode 100644
index 000..1a71f59
--- /dev/null
+++ b/ui/sdl2.c
@@ -0,0 +1,889 @@
+/*
+ * QEMU SDL display driver
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ *
+ * 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 abov

Re: [Qemu-devel] dump-guest-memory enhancement.

[Laszlo Ersek]

On 11/10/13 10:10, Phi Debian wrote:
> Hi All,
> 
> I implemented guest-dump-memory for arm32, and bumped in something
> strange, the PT_LOADs generated from dump.c are not target page
> aligned. There are some advantage to get PT_LOAD aligned.
> 
> This mail is to ask advise about patch I could submit later if wanted.
> 
> Would it be desirable to get the PT_LOAD aligned on target page size
> always, for all arch ?

Can you please explain the problem in more detail? Preferably
illustrated by "readelf" output.

Is it about the placement of the PT_LOAD entries in the core file (eg.
their distance of the beginning of the file, or some such), or about the
contents of the PT_LOAD entries? (Ie. the target-phys ranges they
describe and where those are located in the core file.)

Thanks
Laszlo

> If so I could fix dump.c, but may be people using dump-guest-memory on
> existing current implementation may object, dunno if some of there
> tools (debuggers) are dependent of the non aligned current setup (gdb
> would nt complain)
> 
> If not wanted for existing arch (s390, ppc, i386), may be I could do
> it for arm only.
> 
> Or ultimatly may be this is not wanted at all, in this case I keep my
> dumper for myself :)
> 
> Cheers,
> Phi
> 

Re: [Qemu-devel] [PATCH V5 6/6] qemu-iotests: add test for qcow2 snapshot

[Wenchao Xia]

于 2013/11/9 4:50, Jeff Cody 写道:

On Tue, Nov 05, 2013 at 08:01:29AM +0800, Wenchao Xia wrote:

This test will focus on the low level procedure of qcow2 snapshot
operations, now it covers only the create operation. Overlap error
paths are not checked since no good way to trigger those errors.

Signed-off-by: Wenchao Xia 
---
  tests/qemu-iotests/070   |  214 ++
  tests/qemu-iotests/070.out   |   35 ++
  tests/qemu-iotests/common.filter |7 ++
  tests/qemu-iotests/group |1 +
  4 files changed, 257 insertions(+), 0 deletions(-)
  create mode 100755 tests/qemu-iotests/070
  create mode 100644 tests/qemu-iotests/070.out

diff --git a/tests/qemu-iotests/070 b/tests/qemu-iotests/070
new file mode 100755
index 000..37ada84
--- /dev/null
+++ b/tests/qemu-iotests/070
@@ -0,0 +1,214 @@
+#!/bin/bash
+#
+# qcow2 internal snapshot test
+#
+# Copyright (C) 2013 IBM, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+owner=xiaw...@linux.vnet.ibm.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+rm $TEST_DIR/blkdebug.conf


$TEST_DIR needs quoting (also in later uses in this file as well)


+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+# only test qcow2
+_supported_fmt qcow2
+_supported_proto generic
+# bind the errno correctly and filter the output of image check and qemu-img,
+# if you want to run it on other OS
+_supported_os Linux
+
+
+IMGOPTS="compat=1.1"
+
+CLUSTER_SIZE=65536
+
+SIZE=1G
+
+BLKDBG_TEST_IMG="blkdebug:$TEST_DIR/blkdebug.conf:$TEST_IMG"
+
+errno=5
+
+once=on
+
+imm=off
+
+
+# Start test, note that the injected errors are related to qcow2's snapshot
+# logic closely, see qcow2-snapshot.c for more details.
+
+# path 1: fail in L1 table allocation for snapshot
+echo
+echo "Path 1: fail in allocation of L1 table"
+
+_make_test_img $SIZE
+
+cat > $TEST_DIR/blkdebug.conf <

Both $BLKDB_TEST_IMG and $TEST_IMG need quoting (also in later uses in
this file)



 will fix that.

Re: [Qemu-devel] [PATCH V4 5/5] qemu-iotests: add test for snapshot in qemu-img convert

[Wenchao Xia]

于 2013/11/9 1:18, Jeff Cody 写道:

On Fri, Oct 11, 2013 at 10:33:31AM +0800, Wenchao Xia wrote:

Signed-off-by: Wenchao Xia 
---
  tests/qemu-iotests/058 |   19 ++-
  tests/qemu-iotests/058.out |   12 
  2 files changed, 30 insertions(+), 1 deletions(-)

diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058
index 5b821cf..d4987ae 100755
--- a/tests/qemu-iotests/058
+++ b/tests/qemu-iotests/058
@@ -1,6 +1,6 @@
  #!/bin/bash
  #
-# Test export internal snapshot by qemu-nbd.
+# Test export internal snapshot by qemu-nbd, convert it by qemu-img.
  #
  # Copyright (C) 2013 IBM, Inc.
  #
@@ -33,6 +33,8 @@ status=1  # failure is the default!
  nbd_snapshot_port=10850
  nbd_snapshot_img="nbd:127.0.0.1:$nbd_snapshot_port"

+converted_image=$TEST_IMG.converted
+
  _export_nbd_snapshot()
  {
  $QEMU_NBD -v -t -b 127.0.0.1 -p $nbd_snapshot_port "$TEST_IMG" -l $1 &
@@ -53,6 +55,7 @@ _cleanup()
  kill $NBD_SNAPSHOT_PID
  fi
  _cleanup_test_img
+rm -f $converted_image


Please quote $converted_image (especially with rm -f) - it is also
used unquoted later on in this file, as well.


  }
  trap "_cleanup; exit \$status" 0 1 2 3 15

@@ -96,6 +99,20 @@ echo "== verifying the exported snapshot with patterns =="
  $QEMU_IO -c 'read -P 0xa 0x1000 0x1000' $nbd_snapshot_img | _filter_qemu_io
  $QEMU_IO -c 'read -P 0xb 0x2000 0x1000' $nbd_snapshot_img | _filter_qemu_io

+$QEMU_IMG convert $TEST_IMG -l sn1 -O qcow2 $converted_image


$TEST_IMG needs quoting here, and again below



  Thanks for reviewing, will rebase with the quote issue fixed.


+
+echo
+echo "== verifying the converted snapshot with patterns =="
+$QEMU_IO -c 'read -P 0xa 0x1000 0x1000' $converted_image | _filter_qemu_io
+$QEMU_IO -c 'read -P 0xb 0x2000 0x1000' $converted_image | _filter_qemu_io
+
+$QEMU_IMG convert $TEST_IMG -l snapshot.name=sn1 -O qcow2 $converted_image
+
+echo
+echo "== verifying the converted snapshot with patterns =="
+$QEMU_IO -c 'read -P 0xa 0x1000 0x1000' $converted_image | _filter_qemu_io
+$QEMU_IO -c 'read -P 0xb 0x2000 0x1000' $converted_image | _filter_qemu_io
+
  # success, all done
  echo "*** done"
  rm -f $seq.full
diff --git a/tests/qemu-iotests/058.out b/tests/qemu-iotests/058.out
index cc4b8ca..a8381b9 100644
--- a/tests/qemu-iotests/058.out
+++ b/tests/qemu-iotests/058.out
@@ -29,4 +29,16 @@ read 4096/4096 bytes at offset 4096
  4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
  read 4096/4096 bytes at offset 8192
  4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verifying the converted snapshot with patterns ==
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verifying the converted snapshot with patterns ==
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
  *** done
--
1.7.1

[Qemu-devel] i386: pc: align gpa<->hpa on 1GB boundary (v5)

[Marcelo Tosatti]

v2: condition enablement of new mapping to new machine types (Paolo)
v3: fix changelog
v4: rebase
v5: ensure alignment of piecetwo on 2MB GPA (Igor)
do not register zero-sized piece-one(Igor)

-

Align guest physical address and host physical address
beyond guest 4GB on a 1GB boundary.

Otherwise 1GB TLBs cannot be cached for the range.

Signed-off-by: Marcelo Tosatti 

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 12c436e..abd6b81 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1156,8 +1156,9 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
 {
 int linux_boot, i;
 MemoryRegion *ram, *option_rom_mr;
-MemoryRegion *ram_below_4g, *ram_above_4g;
+MemoryRegion *ram_below_4g, *ram_above_4g, *ram_above_4g_piecetwo;
 FWCfgState *fw_cfg;
+uint64_t memsize, align_offset;
 
 linux_boot = (kernel_filename != NULL);
 
@@ -1166,8 +1167,12 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
  * with older qemus that used qemu_ram_alloc().
  */
 ram = g_malloc(sizeof(*ram));
-memory_region_init_ram(ram, NULL, "pc.ram",
-   below_4g_mem_size + above_4g_mem_size);
+
+memsize = ROUND_UP(below_4g_mem_size + above_4g_mem_size, 1UL << 21);
+align_offset = memsize - (below_4g_mem_size + above_4g_mem_size);
+
+memory_region_init_ram(ram, NULL, "pc.ram", memsize);
+
 vmstate_register_ram_global(ram);
 *ram_memory = ram;
 ram_below_4g = g_malloc(sizeof(*ram_below_4g));
@@ -1177,10 +1182,50 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory,
 e820_add_entry(0, below_4g_mem_size, E820_RAM);
 if (above_4g_mem_size > 0) {
 ram_above_4g = g_malloc(sizeof(*ram_above_4g));
-memory_region_init_alias(ram_above_4g, NULL, "ram-above-4g", ram,
- below_4g_mem_size, above_4g_mem_size);
-memory_region_add_subregion(system_memory, 0x1ULL,
+/*
+ *
+ * If 1GB hugepages are used to back guest RAM, map guest address
+ * space in the range [ramsize,ramsize+holesize] to the ram block
+ * range [holestart, 4GB]
+ *
+ *  0  h 4G [ramsize,ramsize+holesize]
+ *
+ * guest-addr-space [  ] [  ][xxx]
+ */--/
+ * contiguous-ram-block [  ][xxx][ ]
+ *
+ * So that memory beyond 4GB is aligned on a 1GB boundary,
+ * at the host physical address space.
+ *
+ */
+if (guest_info->gb_align) {
+uint64_t holesize = 0x1ULL - below_4g_mem_size;
+uint64_t piecetwosize = holesize - align_offset;
+
+assert(piecetwosize <= holesize);
+
+if ((above_4g_mem_size - piecetwosize) > 0) {
+memory_region_init_alias(ram_above_4g, NULL, "ram-above-4g",
+ ram, 0x1ULL,
+ above_4g_mem_size - piecetwosize);
+memory_region_add_subregion(system_memory, 0x1ULL,
+ ram_above_4g);
+}
+
+ram_above_4g_piecetwo = g_malloc(sizeof(*ram_above_4g_piecetwo));
+memory_region_init_alias(ram_above_4g_piecetwo, NULL,
+ "ram-above-4g-piecetwo", ram,
+ 0x1ULL - holesize, piecetwosize);
+memory_region_add_subregion(system_memory,
+0x1ULL +
+above_4g_mem_size - piecetwosize,
+ram_above_4g_piecetwo);
+} else {
+memory_region_init_alias(ram_above_4g, NULL, "ram-above-4g", ram,
+below_4g_mem_size, above_4g_mem_size);
+memory_region_add_subregion(system_memory, 0x1ULL,
 ram_above_4g);
+}
 e820_add_entry(0x1ULL, above_4g_mem_size, E820_RAM);
 }
 
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4fdb7b6..686736e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -60,6 +60,7 @@ static const int ide_irq[MAX_IDE_BUS] = { 14, 15 };
 static bool has_pvpanic;
 static bool has_pci_info = true;
 static bool has_acpi_build = true;
+static bool gb_align = true;
 
 /* PC hardware initialisation */
 static void pc_init1(QEMUMachineInitArgs *args,
@@ -128,6 +129,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
 
 guest_info->has_pci_info = has_pci_info;
 guest_info->isapc_ram_fw = !pci_enabled;
+guest_info->gb_align = gb_align;
 
 /* allocate ram and load rom/bios */
 if (!xen_enabled()) {
@@ -240,8 +242,14 @@ static void pc_init_pci(QEMUMachineInitArgs *args)
 pc_init1(args, 1, 1);
 }
 
+static void pc_compat_1_7(QEMUMachineInitArgs *args)
+{
+gb_al

Re: [Qemu-devel] dump-guest-memory enhancement.

[Phi Debian]

Hi All,

Sorry Laszlo for flooding your mailbox, I missed the 'reply to all' so
I redo the post here.

And I added some more comment at the end to answer your questions.
Phi

==
CU82$ /usr/bin/readelf -a vmcore
ELF Header:
  Magic:   7f 45 4c 46 01 01 01 00 00 00 00 00 00 00 00 00
  Class: ELF32
  Data:  2's complement, little endian
  Version:   1 (current)
  OS/ABI:UNIX - System V
  ABI Version:   0
  Type:  CORE (Core file)
  Machine:   ARM
  Version:   0x1
  Entry point address:   0x0
  Start of program headers:  52 (bytes into file)
  Start of section headers:  0 (bytes into file)
  Flags: 0x0
  Size of this header:   52 (bytes)
  Size of program headers:   32 (bytes)
  Number of program headers: 2
  Size of section headers:   0 (bytes)
  Number of section headers: 0
  Section header string table index: 0

There are no sections in this file.

There are no sections to group in this file.

Program Headers:
  Type Offset  VirtAddrPhysAddr   FileSizMemSiz  Flg Align
  NOTE   0x74 0x 0x 0x000a0 0x000a0 0
  LOAD   0x000114 0x6000 0x6000 0x4000 0x4000 0

There is no dynamic section in this file.

There are no relocations in this file.

No version information found in this file.

Notes at offset 0x0074 with length 0x00a0:
  Owner Data size   Description
  CORE 0x008c   NT_PRSTATUS (prstatus structure)




The Align fot the PT_LOAD is ZERO, then the offset is 0x114, having an
Align set to TARGET_PAGE_BITS, (or at least 4Kb) would provide a
chance for any debugger to do page align copy (either lseek/read, or
mmap) as they trip on the core, marginal detail, but may help.

As an example, a userland main(){abort();} kind of prog would produce
a core file like this.

CM01$ readelf -a core.2000
...
LOAD off0x1000 vaddr 0x0040 paddr
0x align 2**12
Program Headers:
  Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  NOTE   0x0001d4 0x 0x 0x001d8 0x0 0
  LOAD   0x001000 0x00a42000 0x 0x0 0x1b000 R E 0x1000
  LOAD   0x002000 0x00a5e000 0x 0x01000 0x01000 RW  0x1000

The align for LOAD's is 0x1000 thus the file offset is 0x01000, 0x2000 etc...

==

I guess dump-guest-memory is of a marginal use, yet it can be usefull
when kexec/kdump is broken or non existant on some new architecture
(os/arch bring up).

So to answer your question, the content of the PT_LOAD is ok, only its
offset is non aligned.

I got to precise I obtained this vmcore by implementing the arc_arm
part of the qemu dump-guest-memory, and planing to do the same for
arm64, I may have mis-used the QEMU API's, but for what I can read,
the align member is left non initialised after a memset(zero) of the
phdr/shdr i.e it is left at zero, and I got the impression that the
wayt the elf is produced, section/progs alignment was not in mind. So
I guess other arch are not aligned either, I did not test that.

Cheers,
Phi

[Qemu-devel] [PATCH] virtio-net: don't update mac_table in error state

[Amos Kong]

mac_table was always cleaned up first in handling
VIRTIO_NET_CTRL_MAC_TABLE_SET command, and we din't recover
mac_table content in error state, it's not correct.

This patch makes all the changes in temporal variables,
only update the real mac_table if everything is ok.
We won't change mac_table in error state, so rxfilter
notification isn't needed.

This patch also fixed same problame in
 http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg01188.html
 (not merge)

I will send patch for virtio spec to clarifying this change.

Signed-off-by: Amos Kong 
---
 hw/net/virtio-net.c | 34 --
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 22dbd05..880fa4a 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -610,11 +610,11 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t 
cmd,
 return VIRTIO_NET_ERR;
 }
 
-n->mac_table.in_use = 0;
-n->mac_table.first_multi = 0;
-n->mac_table.uni_overflow = 0;
-n->mac_table.multi_overflow = 0;
-memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN);
+int in_use = 0;
+int first_multi = 0;
+uint8_t uni_overflow = 0;
+uint8_t multi_overflow = 0;
+uint8_t *macs = g_malloc0(MAC_TABLE_ENTRIES * ETH_ALEN);
 
 s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries,
sizeof(mac_data.entries));
@@ -629,19 +629,19 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t 
cmd,
 }
 
 if (mac_data.entries <= MAC_TABLE_ENTRIES) {
-s = iov_to_buf(iov, iov_cnt, 0, n->mac_table.macs,
+s = iov_to_buf(iov, iov_cnt, 0, macs,
mac_data.entries * ETH_ALEN);
 if (s != mac_data.entries * ETH_ALEN) {
 goto error;
 }
-n->mac_table.in_use += mac_data.entries;
+in_use += mac_data.entries;
 } else {
-n->mac_table.uni_overflow = 1;
+uni_overflow = 1;
 }
 
 iov_discard_front(&iov, &iov_cnt, mac_data.entries * ETH_ALEN);
 
-n->mac_table.first_multi = n->mac_table.in_use;
+first_multi = in_use;
 
 s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries,
sizeof(mac_data.entries));
@@ -656,23 +656,29 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t 
cmd,
 goto error;
 }
 
-if (n->mac_table.in_use + mac_data.entries <= MAC_TABLE_ENTRIES) {
-s = iov_to_buf(iov, iov_cnt, 0, n->mac_table.macs,
+if (in_use + mac_data.entries <= MAC_TABLE_ENTRIES) {
+s = iov_to_buf(iov, iov_cnt, 0, &macs[in_use * ETH_ALEN],
mac_data.entries * ETH_ALEN);
 if (s != mac_data.entries * ETH_ALEN) {
 goto error;
 }
-n->mac_table.in_use += mac_data.entries;
+in_use += mac_data.entries;
 } else {
-n->mac_table.multi_overflow = 1;
+multi_overflow = 1;
 }
 
+n->mac_table.in_use = in_use;
+n->mac_table.first_multi = first_multi;
+n->mac_table.uni_overflow = uni_overflow;
+n->mac_table.multi_overflow = multi_overflow;
+memcpy(n->mac_table.macs, macs, MAC_TABLE_ENTRIES * ETH_ALEN);
+g_free(macs);
 rxfilter_notify(nc);
 
 return VIRTIO_NET_OK;
 
 error:
-rxfilter_notify(nc);
+g_free(macs);
 return VIRTIO_NET_ERR;
 }
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 2/4] apic: QOM'ify apic & icc_bus

[赵小强]

于 11/05/2013 04:51 PM, 赵小强 写道:

于 2013年11月05日 16:25, Chen Fan 写道:

On Tue, 2013-11-05 at 15:55 +0800, xiaoqiang zhao wrote:

changes includes:
1. use type constant for apic and kvm_apic
2. convert function 'init' to QOM's 'realize' for apic/kvm_apic
3. for consistency, also QOM'ify apic's parent bus 'icc_bus'
---
  hw/cpu/icc_bus.c|   14 ++
  hw/i386/kvm/apic.c  |   10 +++---
  hw/intc/apic.c  |   10 +++---
  hw/intc/apic_common.c   |   13 +++--
  include/hw/cpu/icc_bus.h|3 ++-
  include/hw/i386/apic_internal.h |5 +++--
  6 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
index 9a4ea7e..1cc64ac 100644
--- a/hw/cpu/icc_bus.c
+++ b/hw/cpu/icc_bus.c
@@ -43,15 +43,13 @@ static const TypeInfo icc_bus_info = {
static void icc_device_realize(DeviceState *dev, Error **errp)
  {
-ICCDevice *id = ICC_DEVICE(dev);
-ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(id);
-
-if (idc->init) {
-if (idc->init(id) < 0) {
-error_setg(errp, "%s initialization failed.",
-   object_get_typename(OBJECT(dev)));
-}
+ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(dev);
+
+/* convert to QOM */
+if (idc->realize) {
+idc->realize(dev, errp);
  }
+
  }
static void icc_device_class_init(ObjectClass *oc, void *data)
diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 84f6056..55f4a53 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -13,6 +13,8 @@
  #include "hw/pci/msi.h"
  #include "sysemu/kvm.h"
  +#define TYPE_KVM_APIC "kvm-apic"
+
  static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic,
  int reg_id, uint32_t val)
  {
@@ -171,8 +173,10 @@ static const MemoryRegionOps kvm_apic_io_ops = {
  .endianness = DEVICE_NATIVE_ENDIAN,
  };
  -static void kvm_apic_init(APICCommonState *s)
+static void kvm_apic_realize(DeviceState *dev, Error **errp)
  {
+APICCommonState *s = APIC_COMMON(dev);
+
  memory_region_init_io(&s->io_memory, NULL, &kvm_apic_io_ops, 
s, "kvm-apic-msi",

APIC_SPACE_SIZE);
  @@ -185,7 +189,7 @@ static void kvm_apic_class_init(ObjectClass 
*klass, void *data)

  {
  APICCommonClass *k = APIC_COMMON_CLASS(klass);
  -k->init = kvm_apic_init;
+k->realize = kvm_apic_realize;
  k->set_base = kvm_apic_set_base;
  k->set_tpr = kvm_apic_set_tpr;
  k->get_tpr = kvm_apic_get_tpr;
@@ -195,7 +199,7 @@ static void kvm_apic_class_init(ObjectClass 
*klass, void *data)

  }
static const TypeInfo kvm_apic_info = {
-.name = "kvm-apic",
+.name = TYPE_KVM_APIC,
  .parent = TYPE_APIC_COMMON,
  .instance_size = sizeof(APICCommonState),
  .class_init = kvm_apic_class_init,
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index b542628..2d7891d 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -32,6 +32,8 @@
  #define SYNC_TO_VAPIC   0x2
  #define SYNC_ISR_IRR_TO_VAPIC   0x4
  +#define TYPE_APIC "apic"
+
  static APICCommonState *local_apics[MAX_APICS + 1];
static void apic_set_irq(APICCommonState *s, int vector_num, int 
trigger_mode);

@@ -871,8 +873,10 @@ static const MemoryRegionOps apic_io_ops = {
  .endianness = DEVICE_NATIVE_ENDIAN,
  };
  -static void apic_init(APICCommonState *s)
+static void apic_realize(DeviceState *dev, Error **errp)
  {
+APICCommonState *s = APIC_COMMON(dev);
+
  memory_region_init_io(&s->io_memory, OBJECT(s), &apic_io_ops, 
s, "apic-msi",

APIC_SPACE_SIZE);
  @@ -886,7 +890,7 @@ static void apic_class_init(ObjectClass 
*klass, void *data)

  {
  APICCommonClass *k = APIC_COMMON_CLASS(klass);
  -k->init = apic_init;
+k->realize = apic_realize;
  k->set_base = apic_set_base;
  k->set_tpr = apic_set_tpr;
  k->get_tpr = apic_get_tpr;
@@ -897,7 +901,7 @@ static void apic_class_init(ObjectClass *klass, 
void *data)

  }
static const TypeInfo apic_info = {
-.name  = "apic",
+.name  = TYPE_APIC,
  .instance_size = sizeof(APICCommonState),
  .parent= TYPE_APIC_COMMON,
  .class_init= apic_class_init,
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index f3edf48..5a413cc 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -284,7 +284,7 @@ static int apic_load_old(QEMUFile *f, void 
*opaque, int version_id)

  return 0;
  }
  -static int apic_init_common(ICCDevice *dev)
+static void apic_common_realize(DeviceState *dev, Error **errp)
  {
  APICCommonState *s = APIC_COMMON(dev);
  APICCommonClass *info;
@@ -293,14 +293,16 @@ static int apic_init_common(ICCDevice *dev)
  static bool mmio_registered;
if (apic_no >= MAX_APICS) {
-return -1;
+error_setg(errp, "%s initialization failed.",

  ^^

+ object_get_typename(OBJECT(dev)));
+return;
  }

Inde

Re: [Qemu-devel] [PATCH] ui/sdl2 : initial port to SDL 2.0

[Anthony Liguori]

On Sun, Nov 10, 2013 at 3:15 PM, Dave Airlie  wrote:
> From: Dave Airlie 
>
> I've ported the SDL1.2 code over, and rewritten it to use the SDL2 interface.
>
> The biggest changes were in the input handling, where SDL2 has done a major
> overhaul, and I've had to include a generated translation file to get from
> SDL2 codes back to qemu compatible ones. I'm still not sure how the keyboard
> layout code works in qemu, so there may be further work if someone can point
> me a test case that works with SDL1.2 and doesn't with SDL2.
>
> Some SDL env vars we used to set are no longer used by SDL2,
> Windows, OSX support is untested,
>
> I don't think we can link to SDL1.2 and SDL2 at the same time, so I felt
> using --with-sdlabi=2.0 to select the new code should be fine, like how
> gtk does it.
>
> Signed-off-by: Dave Airlie 
> ---
>  configure|  23 +-
>  ui/Makefile.objs |   4 +-
>  ui/sdl.c |   3 +
>  ui/sdl2.c| 889 
> +++

Can we refactor this to not duplicate everything and instead have
function hooks or even #ifdefs for the things that are different?  We
try to guess the right SDL to use in configure.  See how we handle
GTK2 vs. GTK3.

It's very hard to review ATM due to the split.

Regarding the keycodes, danpb has a great write up on his blog:

https://www.berrange.com/posts/2010/07/04/a-summary-of-scan-code-key-codes-sets-used-in-the-pc-virtualization-stack/

Internally, we use a variant of raw XT scancodes.  We have a
keymapping routine that translates from a symbolic key (i.e. "capital
A") to the appropriate XT scancode.

SDL 1.x at least lets you get at raw X11 scancodes which are either
evdev or PS/2 codes depending on the version of X11.  So for SDL 1.x
we have two translations mechanisms 1) X11 scancodes to XT scancodes
and 2) SDL keysyms to internal QEMU keysyms.

>From what I can tell, SDL2 has moved from just passing through raw X11
scancodes (which like I said, can be different depending on your X
configuration) to having an intermediate translation layer.  See
comments inline:

>  ui/sdl2_scancode_translate.h | 260 +
>  ui/sdl_keysym.h  |   3 +-
>  6 files changed, 1175 insertions(+), 7 deletions(-)
>  create mode 100644 ui/sdl2.c
>  create mode 100644 ui/sdl2_scancode_translate.h
>
> diff --git a/configure b/configure
> index 9addff1..bf3be37 100755
> --- a/configure
> +++ b/configure
> @@ -158,6 +158,7 @@ docs=""
>  fdt=""
>  pixman=""
>  sdl=""
> +sdlabi="1.2"
>  virtfs=""
>  vnc="yes"
>  sparse="no"
> @@ -310,6 +311,7 @@ query_pkg_config() {
>  }
>  pkg_config=query_pkg_config
>  sdl_config="${SDL_CONFIG-${cross_prefix}sdl-config}"
> +sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}"
>
>  # default flags for all hosts
>  QEMU_CFLAGS="-fno-strict-aliasing $QEMU_CFLAGS"
> @@ -710,6 +712,8 @@ for opt do
>;;
>--enable-sdl) sdl="yes"
>;;
> +  --with-sdlabi=*) sdlabi="$optarg"
> +  ;;
>--disable-qom-cast-debug) qom_cast_debug="no"
>;;
>--enable-qom-cast-debug) qom_cast_debug="yes"
> @@ -1092,6 +1096,7 @@ echo "  --disable-strip  disable stripping 
> binaries"
>  echo "  --disable-werror disable compilation abort on warning"
>  echo "  --disable-sdldisable SDL"
>  echo "  --enable-sdl enable SDL"
> +echo "  --with-sdlabiselect preferred SDL ABI 1.2 or 2.0"
>  echo "  --disable-gtkdisable gtk UI"
>  echo "  --enable-gtk enable gtk UI"
>  echo "  --disable-virtfs disable VirtFS"
> @@ -1751,12 +1756,22 @@ fi
>
>  # Look for sdl configuration program (pkg-config or sdl-config).  Try
>  # sdl-config even without cross prefix, and favour pkg-config over 
> sdl-config.
> -if test "`basename $sdl_config`" != sdl-config && ! has ${sdl_config}; then
> -  sdl_config=sdl-config
> +
> +if test $sdlabi == "2.0"; then
> +sdl_config=$sdl2_config
> +sdlname=sdl2
> +sdlconfigname=sdl2_config
> +else
> +sdlname=sdl
> +sdlconfigname=sdl_config
> +fi
> +
> +if test "`basename $sdl_config`" != $sdlconfigname && ! has ${sdl_config}; 
> then
> +  sdl_config=$sdlconfigname
>  fi
>
> -if $pkg_config sdl --exists; then
> -  sdlconfig="$pkg_config sdl"
> +if $pkg_config $sdlname --exists; then
> +  sdlconfig="$pkg_config $sdlname"
>_sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
>  elif has ${sdl_config}; then
>sdlconfig="$sdl_config"
> diff --git a/ui/Makefile.objs b/ui/Makefile.objs
> index f33be47..721ad37 100644
> --- a/ui/Makefile.objs
> +++ b/ui/Makefile.objs
> @@ -9,12 +9,12 @@ vnc-obj-y += vnc-jobs.o
>
>  common-obj-y += keymaps.o console.o cursor.o input.o qemu-pixman.o
>  common-obj-$(CONFIG_SPICE) += spice-core.o spice-input.o spice-display.o
> -common-obj-$(CONFIG_SDL) += sdl.o sdl_zoom.o x_keymap.o
> +common-obj-$(CONFIG_SDL) += sdl.o sdl_zoom.o x_keymap.o sdl2.o
>  common-obj-$(CONFIG_COCOA) += cocoa.o
>  common-ob

Re: [Qemu-devel] [PATCH] qdev-properties-system.c: Allow vlan or netdev for -device, not both

[Jason Wang]

On 11/08/2013 10:13 AM, Vlad Yasevich wrote:
> It is currently possible to specify things like:
>   -device e1000,netdev=foo,vlan=1
> With this usage, whichever argument was specified last (vlan or netdev)
> overwrites what was previousely set and results in a non-working
> configuration.  Even worse, when used with multiqueue devices,
> it causes a segmentation fault on exit in qemu_free_net_client.
>
> That patch treates the above command line options as invalid and
> generates an error at start-up.
>
> Signed-off-by: Vlad Yasevich 
> ---
>  hw/core/qdev-properties-system.c | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/hw/core/qdev-properties-system.c 
> b/hw/core/qdev-properties-system.c
> index 0eada32..729efa8 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -205,6 +205,11 @@ static int parse_netdev(DeviceState *dev, const char 
> *str, void **ptr)
>  goto err;
>  }
>  
> +if (ncs[i]) {
> +ret = -EINVAL;
> +goto err;
> +}
> +
>  ncs[i] = peers[i];
>  ncs[i]->queue_index = i;
>  }
> @@ -301,6 +306,10 @@ static void set_vlan(Object *obj, Visitor *v, void 
> *opaque,
>  *ptr = NULL;
>  return;
>  }
> +if (*ptr) {
> +error_set_from_qdev_prop_error(errp, -EINVAL, dev, prop, name);
> +return;
> +}
>  
>  hubport = net_hub_port_find(id);
>  if (!hubport) {

Acked-by: Jason Wang 

[Qemu-devel] [PATCH V5 3/5] qemu-iotests: add 058 internal snapshot export with qemu-nbd case

[Wenchao Xia]

Signed-off-by: Wenchao Xia 
---
 tests/qemu-iotests/058 |  102 
 tests/qemu-iotests/058.out |   32 ++
 tests/qemu-iotests/check   |1 +
 tests/qemu-iotests/group   |1 +
 4 files changed, 136 insertions(+), 0 deletions(-)
 create mode 100755 tests/qemu-iotests/058
 create mode 100644 tests/qemu-iotests/058.out

diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058
new file mode 100755
index 000..2d50d97
--- /dev/null
+++ b/tests/qemu-iotests/058
@@ -0,0 +1,102 @@
+#!/bin/bash
+#
+# Test export internal snapshot by qemu-nbd.
+#
+# Copyright (C) 2013 IBM, Inc.
+#
+# Based on 029.
+#
+# 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 .
+#
+
+# creator
+owner=xiaw...@linux.vnet.ibm.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+nbd_snapshot_port=10850
+nbd_snapshot_img="nbd:127.0.0.1:$nbd_snapshot_port"
+
+_export_nbd_snapshot()
+{
+$QEMU_NBD -v -t -b 127.0.0.1 -p $nbd_snapshot_port "$TEST_IMG" -l $1 &
+NBD_SNAPSHOT_PID=$!
+sleep 1
+}
+
+_export_nbd_snapshot1()
+{
+$QEMU_NBD -v -t -b 127.0.0.1 -p $nbd_snapshot_port "$TEST_IMG" -l 
snapshot.name=$1 &
+NBD_SNAPSHOT_PID=$!
+sleep 1
+}
+
+_cleanup()
+{
+if [ -n "$NBD_SNAPSHOT_PID" ]; then
+kill $NBD_SNAPSHOT_PID
+fi
+_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+_supported_fmt qcow2
+_supported_proto generic
+
+echo
+echo "== preparing image =="
+_make_test_img 64M
+$QEMU_IO -c 'write -P 0xa 0x1000 0x1000' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c 'write -P 0xb 0x2000 0x1000' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -c sn1 "$TEST_IMG"
+$QEMU_IO -c 'write -P 0xc 0x1000 0x1000' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c 'write -P 0xd 0x2000 0x1000' "$TEST_IMG" | _filter_qemu_io
+_check_test_img
+
+echo
+echo "== verifying the image file with patterns =="
+$QEMU_IO -c 'read -P 0xc 0x1000 0x1000' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c 'read -P 0xd 0x2000 0x1000' "$TEST_IMG" | _filter_qemu_io
+
+_export_nbd_snapshot sn1
+
+echo
+echo "== verifying the exported snapshot with patterns =="
+$QEMU_IO -c 'read -P 0xa 0x1000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
+$QEMU_IO -c 'read -P 0xb 0x2000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
+
+kill $NBD_SNAPSHOT_PID
+sleep 1
+
+_export_nbd_snapshot1 sn1
+
+echo
+echo "== verifying the exported snapshot with patterns =="
+$QEMU_IO -c 'read -P 0xa 0x1000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
+$QEMU_IO -c 'read -P 0xb 0x2000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/058.out b/tests/qemu-iotests/058.out
new file mode 100644
index 000..cc4b8ca
--- /dev/null
+++ b/tests/qemu-iotests/058.out
@@ -0,0 +1,32 @@
+QA output created by 058
+
+== preparing image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+wrote 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+
+== verifying the image file with patterns ==
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verifying the exported snapshot with patterns ==
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verifying the exported snapshot with patterns ==
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+*** done
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index f5f328f..f879474 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -161,

[Qemu-devel] [PATCH V5 0/5] export internal snapshot by qemu-nbd

[Wenchao Xia]

This series allow user to read internal snapshot's contents without qemu-img
convert.

V2:
  Address Stefan's comments:
  02: add 'fall through' comments in the case statement.
  03: add doc about the difference of internal snapshot and backing chain
snapshot, which is used in previous '--snapshot' parameter.
  Other:
  01,04: rebased on upstream with conflict resolved.

v3:
  Address Paolo's comments:
  02: add parameter "-l snapshot_id_or_name", rename options
snapshot-load to load-snapshot, use QemuOpts.
  03: rename snapshot-load to load-snapshot.
  04: related change to test both -l and -L case.
  05-07: add similar parameter for qemu-img convert.
  Other:
  01: foldered original snapshot logic into function
bdrv_snapshot_load_tmp_by_id_or_name(), since multiple
caller present in this version. Refined error message from
", reason: %s" to ": %s".
  02: Refined error message from ", reason: %s" to ": %s".
  03: Rename PARAM to SNAPSHOT_PARAM.

v4:
  Address Eric's comments:
  01: typo fix.
  02: squashed patch. Use only one option -l and distiguish
the cases by the starting string.
  03: remove 'eval', remove '_supported_os Linux', remove the
comments that have typo.
  04: squashed patch. Add only one option -l and distiguish
the cases by the starting string.
  05: remove indentation.
  Address Paolo's comments:
  02: Use only one option -l and distiguish the cases by
the starting string.
  04: Use only one option -l and distiguish the cases by
the starting string, mark old -s as deprecated, added doc
for them.

v5:
  Address Jeff's comments:
  Quote image name in test cases.

Wenchao Xia (5):
  1 snapshot: distinguish id and name in load_tmp
  2 qemu-nbd: support internal snapshot export
  3 qemu-iotests: add 058 internal snapshot export with qemu-nbd case
  4 qemu-img: add -l for snapshot in convert
  5 qemu-iotests: add test for snapshot in qemu-img convert

 block/qcow2-snapshot.c |   16 +-
 block/qcow2.h  |5 ++-
 block/snapshot.c   |   78 +++-
 include/block/block_int.h  |4 +-
 include/block/snapshot.h   |   15 +-
 qemu-img-cmds.hx   |2 +-
 qemu-img.c |   45 ++---
 qemu-img.texi  |   12 +++--
 qemu-nbd.c |   47 +-
 qemu-nbd.texi  |8 +++-
 tests/qemu-iotests/058 |  119 
 tests/qemu-iotests/058.out |   44 
 tests/qemu-iotests/check   |1 +
 tests/qemu-iotests/group   |1 +
 14 files changed, 374 insertions(+), 23 deletions(-)
 create mode 100755 tests/qemu-iotests/058
 create mode 100644 tests/qemu-iotests/058.out

[Qemu-devel] [PATCH V5 2/5] qemu-nbd: support internal snapshot export

[Wenchao Xia]

Now it is possible to directly export an internal snapshot, which
can be used to probe the snapshot's contents without qemu-img
convert.

Signed-off-by: Wenchao Xia 
---
 block/snapshot.c |   18 +
 include/block/snapshot.h |8 +++
 qemu-nbd.c   |   47 -
 qemu-nbd.texi|8 ++-
 4 files changed, 78 insertions(+), 3 deletions(-)

diff --git a/block/snapshot.c b/block/snapshot.c
index e51a7db..7cc45fa 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -25,6 +25,24 @@
 #include "block/snapshot.h"
 #include "block/block_int.h"
 
+QemuOptsList internal_snapshot_opts = {
+.name = "snapshot",
+.head = QTAILQ_HEAD_INITIALIZER(internal_snapshot_opts.head),
+.desc = {
+{
+.name = SNAPSHOT_OPT_ID,
+.type = QEMU_OPT_STRING,
+.help = "snapshot id"
+},{
+.name = SNAPSHOT_OPT_NAME,
+.type = QEMU_OPT_STRING,
+.help = "snapshot name"
+},{
+/* end of list */
+}
+},
+};
+
 int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
const char *name)
 {
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index d05bea7..770d9bb 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -27,6 +27,14 @@
 
 #include "qemu-common.h"
 #include "qapi/error.h"
+#include "qemu/option.h"
+
+
+#define SNAPSHOT_OPT_BASE   "snapshot."
+#define SNAPSHOT_OPT_ID "snapshot.id"
+#define SNAPSHOT_OPT_NAME   "snapshot.name"
+
+extern QemuOptsList internal_snapshot_opts;
 
 typedef struct QEMUSnapshotInfo {
 char id_str[128]; /* unique snapshot id */
diff --git a/qemu-nbd.c b/qemu-nbd.c
index c26c98e..f934eaa 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -20,6 +20,7 @@
 #include "block/block.h"
 #include "block/nbd.h"
 #include "qemu/main-loop.h"
+#include "block/snapshot.h"
 
 #include 
 #include 
@@ -79,7 +80,14 @@ static void usage(const char *name)
 "\n"
 "Block device options:\n"
 "  -r, --read-only  export read-only\n"
-"  -s, --snapshot   use snapshot file\n"
+"  -s, --snapshot   use FILE as an external snapshot, create a temporary\n"
+"   file with backing_file=FILE, redirect the write to\n"
+"   the temporary one\n"
+"  -l, --load-snapshot=SNAPSHOT_PARAM\n"
+"   load an internal snapshot inside FILE and export it\n"
+"   as an read-only device, SNAPSHOT_PARAM format is\n"
+"   'snapshot.id=[ID],snapshot.name=[NAME]', or\n"
+"   '[ID_OR_NAME]'\n"
 "  -n, --nocachedisable host cache\n"
 "  --cache=MODE set cache mode (none, writeback, ...)\n"
 #ifdef CONFIG_LINUX_AIO
@@ -315,7 +323,9 @@ int main(int argc, char **argv)
 char *device = NULL;
 int port = NBD_DEFAULT_PORT;
 off_t fd_size;
-const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:t";
+QemuOpts *sn_opts = NULL;
+const char *sn_id_or_name = NULL;
+const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:";
 struct option lopt[] = {
 { "help", 0, NULL, 'h' },
 { "version", 0, NULL, 'V' },
@@ -328,6 +338,7 @@ int main(int argc, char **argv)
 { "connect", 1, NULL, 'c' },
 { "disconnect", 0, NULL, 'd' },
 { "snapshot", 0, NULL, 's' },
+{ "load-snapshot", 1, NULL, 'l' },
 { "nocache", 0, NULL, 'n' },
 { "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
 #ifdef CONFIG_LINUX_AIO
@@ -428,6 +439,18 @@ int main(int argc, char **argv)
 errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
 }
 break;
+case 'l':
+if (strncmp(optarg, SNAPSHOT_OPT_BASE, strlen(SNAPSHOT_OPT_BASE))
+== 0) {
+sn_opts = qemu_opts_parse(&internal_snapshot_opts, optarg, 0);
+if (!sn_opts) {
+errx(EXIT_FAILURE, "Failed in parsing snapshot param `%s'",
+ optarg);
+}
+} else {
+sn_id_or_name = optarg;
+}
+/* fall through */
 case 'r':
 nbdflags |= NBD_FLAG_READ_ONLY;
 flags &= ~BDRV_O_RDWR;
@@ -581,6 +604,22 @@ int main(int argc, char **argv)
 error_get_pretty(local_err));
 }
 
+if (sn_opts) {
+ret = bdrv_snapshot_load_tmp(bs,
+ qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID),
+ qemu_opt_get(sn_opts, SNAPSHOT_OPT_NAME),
+ &local_err);
+} else if (sn_id_or_name) {
+ret = bdrv_snapshot_load_tmp_by_id_or_name(bs, sn_id_or_name,
+   &local_err);
+}
+if (ret < 0) {
+errno = -ret;
+err(EXIT_FAILURE,
+"Failed to load snapshot: %s",
+   

[Qemu-devel] [PATCH V5 1/5] snapshot: distinguish id and name in load_tmp

[Wenchao Xia]

Since later this function will be used so improve it. The only caller of it
now is qemu-img, and it is not impacted by introduce function
bdrv_snapshot_load_tmp_by_id_or_name() that call bdrv_snapshot_load_tmp()
twice to keep old search logic. bdrv_snapshot_load_tmp_by_id_or_name() return
int to let caller know the errno, and errno will be used later.
Also fix a typo in comments of bdrv_snapshot_delete().

Signed-off-by: Wenchao Xia 
---
 block/qcow2-snapshot.c|   16 ++-
 block/qcow2.h |5 +++-
 block/snapshot.c  |   60 ++--
 include/block/block_int.h |4 ++-
 include/block/snapshot.h  |7 -
 qemu-img.c|8 -
 6 files changed, 90 insertions(+), 10 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 3529c68..aeb5efd 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -675,7 +675,10 @@ int qcow2_snapshot_list(BlockDriverState *bs, 
QEMUSnapshotInfo **psn_tab)
 return s->nb_snapshots;
 }
 
-int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name)
+int qcow2_snapshot_load_tmp(BlockDriverState *bs,
+const char *snapshot_id,
+const char *name,
+Error **errp)
 {
 int i, snapshot_index;
 BDRVQcowState *s = bs->opaque;
@@ -683,12 +686,17 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs, const 
char *snapshot_name)
 uint64_t *new_l1_table;
 int new_l1_bytes;
 int ret;
+const char *device = bdrv_get_device_name(bs);
 
 assert(bs->read_only);
 
 /* Search the snapshot */
-snapshot_index = find_snapshot_by_id_or_name(bs, snapshot_name);
+snapshot_index = find_snapshot_by_id_and_name(bs, snapshot_id, name);
 if (snapshot_index < 0) {
+error_setg(errp,
+   "Can't find a snapshot with ID '%s' and name '%s' "
+   "on device '%s'",
+   STR_OR_NULL(snapshot_id), STR_OR_NULL(name), device);
 return -ENOENT;
 }
 sn = &s->snapshots[snapshot_index];
@@ -699,6 +707,10 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs, const 
char *snapshot_name)
 
 ret = bdrv_pread(bs->file, sn->l1_table_offset, new_l1_table, 
new_l1_bytes);
 if (ret < 0) {
+error_setg(errp,
+   "Failed to read l1 table for snapshot with ID '%s' and name 
"
+   "'%s' on device '%s'",
+   sn->id_str, sn->name, device);
 g_free(new_l1_table);
 return ret;
 }
diff --git a/block/qcow2.h b/block/qcow2.h
index 922e190..303eb26 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -488,7 +488,10 @@ int qcow2_snapshot_delete(BlockDriverState *bs,
   const char *name,
   Error **errp);
 int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab);
-int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name);
+int qcow2_snapshot_load_tmp(BlockDriverState *bs,
+const char *snapshot_id,
+const char *name,
+Error **errp);
 
 void qcow2_free_snapshots(BlockDriverState *bs);
 int qcow2_read_snapshots(BlockDriverState *bs);
diff --git a/block/snapshot.c b/block/snapshot.c
index a05c0c0..e51a7db 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -194,7 +194,7 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
  * If only @snapshot_id is specified, delete the first one with id
  * @snapshot_id.
  * If only @name is specified, delete the first one with name @name.
- * if none is specified, return -ENINVAL.
+ * if none is specified, return -EINVAL.
  *
  * Returns: 0 on success, -errno on failure. If @bs is not inserted, return
  * -ENOMEDIUM. If @snapshot_id and @name are both NULL, return -EINVAL. If @bs
@@ -265,18 +265,72 @@ int bdrv_snapshot_list(BlockDriverState *bs,
 return -ENOTSUP;
 }
 
+/**
+ * Temporarily load an internal snapshot by @snapshot_id and @name.
+ * @bs: block device used in the operation
+ * @snapshot_id: unique snapshot ID, or NULL
+ * @name: snapshot name, or NULL
+ * @errp: location to store error
+ *
+ * If both @snapshot_id and @name are specified, load the first one with
+ * id @snapshot_id and name @name.
+ * If only @snapshot_id is specified, load the first one with id
+ * @snapshot_id.
+ * If only @name is specified, load the first one with name @name.
+ * if none is specified, return -EINVAL.
+ *
+ * Returns: 0 on success, -errno on fail. If @bs is not inserted, return
+ * -ENOMEDIUM. If @bs is not readonly, return -EINVAL. If @bs did not support
+ * internal snapshot, return -ENOTSUP. If qemu can't find one matching @id and
+ * @name, return -ENOENT. If @bs does not support parameter @snapshot_id or
+ * @name, return -EINVAL. If @errp != NULL, it will always be filled on
+ * failure.
+ */
 int bdrv_snapshot_load_tmp(BlockDriverState

[Qemu-devel] [PATCH V5 5/5] qemu-iotests: add test for snapshot in qemu-img convert

[Wenchao Xia]

Signed-off-by: Wenchao Xia 
---
 tests/qemu-iotests/058 |   19 ++-
 tests/qemu-iotests/058.out |   12 
 2 files changed, 30 insertions(+), 1 deletions(-)

diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058
index 2d50d97..9e28132 100755
--- a/tests/qemu-iotests/058
+++ b/tests/qemu-iotests/058
@@ -1,6 +1,6 @@
 #!/bin/bash
 #
-# Test export internal snapshot by qemu-nbd.
+# Test export internal snapshot by qemu-nbd, convert it by qemu-img.
 #
 # Copyright (C) 2013 IBM, Inc.
 #
@@ -33,6 +33,8 @@ status=1  # failure is the default!
 nbd_snapshot_port=10850
 nbd_snapshot_img="nbd:127.0.0.1:$nbd_snapshot_port"
 
+converted_image=$TEST_IMG.converted
+
 _export_nbd_snapshot()
 {
 $QEMU_NBD -v -t -b 127.0.0.1 -p $nbd_snapshot_port "$TEST_IMG" -l $1 &
@@ -53,6 +55,7 @@ _cleanup()
 kill $NBD_SNAPSHOT_PID
 fi
 _cleanup_test_img
+rm -f "$converted_image"
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
@@ -96,6 +99,20 @@ echo "== verifying the exported snapshot with patterns =="
 $QEMU_IO -c 'read -P 0xa 0x1000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
 $QEMU_IO -c 'read -P 0xb 0x2000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
 
+$QEMU_IMG convert "$TEST_IMG" -l sn1 -O qcow2 "$converted_image"
+
+echo
+echo "== verifying the converted snapshot with patterns =="
+$QEMU_IO -c 'read -P 0xa 0x1000 0x1000' "$converted_image" | _filter_qemu_io
+$QEMU_IO -c 'read -P 0xb 0x2000 0x1000' "$converted_image" | _filter_qemu_io
+
+$QEMU_IMG convert "$TEST_IMG" -l snapshot.name=sn1 -O qcow2 "$converted_image"
+
+echo
+echo "== verifying the converted snapshot with patterns =="
+$QEMU_IO -c 'read -P 0xa 0x1000 0x1000' "$converted_image" | _filter_qemu_io
+$QEMU_IO -c 'read -P 0xb 0x2000 0x1000' "$converted_image" | _filter_qemu_io
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/058.out b/tests/qemu-iotests/058.out
index cc4b8ca..a8381b9 100644
--- a/tests/qemu-iotests/058.out
+++ b/tests/qemu-iotests/058.out
@@ -29,4 +29,16 @@ read 4096/4096 bytes at offset 4096
 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 4096/4096 bytes at offset 8192
 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verifying the converted snapshot with patterns ==
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verifying the converted snapshot with patterns ==
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 *** done
-- 
1.7.1

[Qemu-devel] [PATCH V5 4/5] qemu-img: add -l for snapshot in convert

[Wenchao Xia]

Now qemu-img convert have similar options as qemu-nbd for internal
snapshot.

Signed-off-by: Wenchao Xia 
---
 qemu-img-cmds.hx |2 +-
 qemu-img.c   |   45 -
 qemu-img.texi|   12 
 3 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index da1d965..9a8153b 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -34,7 +34,7 @@ STEXI
 ETEXI
 
 DEF("convert", img_convert,
-"convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-O output_fmt] [-o 
options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] 
output_filename")
+"convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-O output_fmt] [-o 
options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename 
[filename2 [...]] output_filename")
 STEXI
 @item convert [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-O 
@var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S 
@var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
diff --git a/qemu-img.c b/qemu-img.c
index fe28119..e9f5e3a 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -93,6 +93,11 @@ static void help(void)
"  'options' is a comma separated list of format specific options 
in a\n"
"name=value format. Use -o ? for an overview of the options 
supported by the\n"
"used format\n"
+   "  'snapshot_param' is param used for internal snapshot, format\n"
+   "is 'snapshot.id=[ID],snapshot.name=[NAME]', or\n"
+   "'[ID_OR_NAME]'\n"
+   "  'snapshot_id_or_name' is deprecated, use 'snapshot_param'\n"
+   "instead\n"
"  '-c' indicates that target image must be compressed (qcow format 
only)\n"
"  '-u' enables unsafe rebasing. It is assumed that old and new 
backing file\n"
"   match exactly. The image doesn't need a working backing 
file before\n"
@@ -1140,6 +1145,7 @@ static int img_convert(int argc, char **argv)
 int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */
 bool quiet = false;
 Error *local_err = NULL;
+QemuOpts *sn_opts = NULL;
 
 fmt = NULL;
 out_fmt = "raw";
@@ -1148,7 +1154,7 @@ static int img_convert(int argc, char **argv)
 compress = 0;
 skip_create = 0;
 for(;;) {
-c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:qn");
+c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:qnl:");
 if (c == -1) {
 break;
 }
@@ -1183,6 +1189,19 @@ static int img_convert(int argc, char **argv)
 case 's':
 snapshot_name = optarg;
 break;
+case 'l':
+if (strncmp(optarg, SNAPSHOT_OPT_BASE, strlen(SNAPSHOT_OPT_BASE))
+== 0) {
+sn_opts = qemu_opts_parse(&internal_snapshot_opts, optarg, 0);
+if (!sn_opts) {
+error_report("Failed in parsing snapshot param '%s'",
+ optarg);
+return 1;
+}
+} else {
+snapshot_name = optarg;
+}
+break;
 case 'S':
 {
 int64_t sval;
@@ -1254,7 +1273,12 @@ static int img_convert(int argc, char **argv)
 total_sectors += bs_sectors;
 }
 
-if (snapshot_name != NULL) {
+if (sn_opts) {
+ret = bdrv_snapshot_load_tmp(bs[0],
+ qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID),
+ qemu_opt_get(sn_opts, SNAPSHOT_OPT_NAME),
+ &local_err);
+} else if (snapshot_name != NULL) {
 if (bs_n > 1) {
 error_report("No support for concatenating multiple snapshot");
 ret = -1;
@@ -1262,13 +1286,13 @@ static int img_convert(int argc, char **argv)
 }
 
 bdrv_snapshot_load_tmp_by_id_or_name(bs[0], snapshot_name, &local_err);
-if (error_is_set(&local_err)) {
-error_report("Failed to load snapshot: %s",
- error_get_pretty(local_err));
-error_free(local_err);
-ret = -1;
-goto out;
-}
+}
+if (error_is_set(&local_err)) {
+error_report("Failed to load snapshot: %s",
+ error_get_pretty(local_err));
+error_free(local_err);
+ret = -1;
+goto out;
 }
 
 /* Find driver and parse its options */
@@ -1559,6 +1583,9 @@ out:
 free_option_parameters(create_options);
 free_option_parameters(param);
 qemu_vfree(buf);
+if (sn_opts) {
+qemu_opts_del(sn_opts);
+}
 if (out_bs) {
 bdrv_unref(out_bs);
 }
diff --git a/qemu-img.texi b/qemu-img.texi
index 768054e..fbdbfa7 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -46,7 +46,11 @@ is the destination disk image filename
 is a comma separated list of format s

[Qemu-devel] [PATCH v3 2/6] target-ppc: make use of new -cpu suboptions handling

[Alexey Kardashevskiy]

This enables -cpu subobtions parser. No option is supported yet.

Signed-off-by: Alexey Kardashevskiy 
---
 hw/ppc/spapr.c  | 4 +++-
 target-ppc/translate_init.c | 5 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7e53a5f..2e18bdb 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1101,7 +1101,7 @@ static SaveVMHandlers savevm_htab_handlers = {
 static void ppc_spapr_init(QEMUMachineInitArgs *args)
 {
 ram_addr_t ram_size = args->ram_size;
-const char *cpu_model = args->cpu_model;
+const char *cpu_model;
 const char *kernel_filename = args->kernel_filename;
 const char *kernel_cmdline = args->kernel_cmdline;
 const char *initrd_filename = args->initrd_filename;
@@ -1121,6 +1121,8 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
 
 msi_supported = true;
 
+cpu_model = qemu_opt_get(qemu_get_cpu_opts(), "type");
+
 spapr = g_malloc0(sizeof(*spapr));
 QLIST_INIT(&spapr->phbs);
 
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index c030a20..9e4af56 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7381,6 +7381,10 @@ static void init_ppc_proc(PowerPCCPU *cpu)
 /* PowerPC implementation specific initialisations (SPRs, timers, ...) */
 (*pcc->init_proc)(env);
 
+if (cpu_parse_options(CPU(cpu))) {
+exit(1);
+}
+
 /* MSR bits & flags consistency checks */
 if (env->msr_mask & (1 << 25)) {
 switch (env->flags & (POWERPC_FLAG_SPE | POWERPC_FLAG_VRE)) {
@@ -8674,6 +8678,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void 
*data)
 #endif
 
 dc->fw_name = "PowerPC,UNKNOWN";
+cc->parse_options = cpu_default_parse_options_func;
 }
 
 static const TypeInfo ppc_cpu_type_info = {
-- 
1.8.4.rc4

[Qemu-devel] [PATCH v3 5/6] bitops: add BITNR macro

[Alexey Kardashevskiy]

This adds a macro to calculate the highest bit set. If used on constant
values, no code will be generated.

Signed-off-by: Alexey Kardashevskiy 
---
 include/qemu/bitops.h | 12 
 1 file changed, 12 insertions(+)

diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index 304c90c..98ba42a 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -23,6 +23,18 @@
 #define BIT_WORD(nr)   ((nr) / BITS_PER_LONG)
 #define BITS_TO_LONGS(nr)  DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
 
+#define __BITNR(m, n)  ((m) == ((m) & (1<<(n ? (n) :
+#define BITNR(m) \
+__BITNR((m), 31) __BITNR((m), 30) __BITNR((m), 29) __BITNR((m), 28) \
+__BITNR((m), 27) __BITNR((m), 26) __BITNR((m), 25) __BITNR((m), 24) \
+__BITNR((m), 23) __BITNR((m), 22) __BITNR((m), 21) __BITNR((m), 20) \
+__BITNR((m), 19) __BITNR((m), 18) __BITNR((m), 17) __BITNR((m), 16) \
+__BITNR((m), 15) __BITNR((m), 14) __BITNR((m), 13) __BITNR((m), 12) \
+__BITNR((m), 11) __BITNR((m), 10) __BITNR((m),  9) __BITNR((m),  8) \
+__BITNR((m),  7) __BITNR((m),  6) __BITNR((m),  5) __BITNR((m),  4) \
+__BITNR((m),  3) __BITNR((m),  2) __BITNR((m),  1) __BITNR((m),  0) \
+-1
+
 /**
  * set_bit - Set a bit in memory
  * @nr: the bit to set
-- 
1.8.4.rc4

[Qemu-devel] [PATCH v3 0/6] spapr: add "compat" machine option

[Alexey Kardashevskiy]

The further I go, more questions I get.

Here are 6 patches.

The first three is what I would like to have in QEMU to support "compat"
option for a CPU. The option now accepts "power6"/"power7" as after all
we will limit number of threads per core (not in this series though) and
since 2.05 does not limit number of threads at all, referring to actual
CPU models seems right.

The last three is what I would suggest doing if we needed ability to
enable/disable CPU features the way x86 does this. I used "VSX" as an example
but this is just an example so "-cpu host,-vsx,+vsx,vsx=on" works.
Using this, I suspect I could try converting x86's parser for "-cpu",
would it work?


btw I am sure there must be macro like BITNR (convert mask with 1 bit set
to a number of the bit which is set) but I failed to find it. What did I miss?


Please, comment. Thanks.


Alexey Kardashevskiy (6):
  cpu: add suboptions support
  target-ppc: make use of new -cpu suboptions handling
  target-ppc: add "compat" CPU option
  qemu-option: support +foo/-foo command line agruments
  bitops: add BITNR macro
  target-ppc: demonstrate new "vsx" property

 hw/ppc/spapr.c  | 13 +++-
 include/qemu/bitops.h   | 12 
 include/qom/cpu.h   | 29 ++
 include/sysemu/sysemu.h |  1 +
 qom/cpu.c   | 27 +
 target-ppc/cpu-models.h | 10 +++
 target-ppc/cpu.h|  4 +++
 target-ppc/translate_init.c | 73 +
 util/qemu-option.c  |  6 
 vl.c| 42 ++
 10 files changed, 216 insertions(+), 1 deletion(-)

-- 
1.8.4.rc4

[Qemu-devel] [PATCH v3 1/6] cpu: add suboptions support

[Alexey Kardashevskiy]

This adds suboptions support for -cpu.

Cc: Andreas Färber 
Signed-off-by: Alexey Kardashevskiy 
---
 include/qom/cpu.h   | 29 +
 include/sysemu/sysemu.h |  1 +
 qom/cpu.c   | 27 +++
 vl.c| 42 ++
 4 files changed, 99 insertions(+)

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 7739e00..7d3b043 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -124,6 +124,7 @@ typedef struct CPUClass {
 int cpuid, void *opaque);
 int (*write_elf32_qemunote)(WriteCoreDumpFunction f, CPUState *cpu,
 void *opaque);
+void (*parse_options)(CPUState *cpu, Error **errp);
 
 const struct VMStateDescription *vmsd;
 int gdb_num_core_regs;
@@ -327,6 +328,34 @@ static inline hwaddr cpu_get_phys_page_debug(CPUState 
*cpu, vaddr addr)
 #endif
 
 /**
+ * cpu_parse_options:
+ * @cpu: The CPU to set options for.
+ */
+static inline int cpu_parse_options(CPUState *cpu)
+{
+CPUClass *cc = CPU_GET_CLASS(cpu);
+Error *err = NULL;
+
+if (cc->parse_options) {
+cc->parse_options(cpu, &err);
+if (err) {
+return -1;
+}
+}
+
+/* No callback, let arch do it the old way */
+return 0;
+}
+
+/**
+ * cpu_default_parse_options_func:
+ * The default handler for CPUClass::parse_options
+ * @cpu: the CPU to set option for.
+ * @errp: the handling error descriptor.
+ */
+void cpu_default_parse_options_func(CPUState *cpu, Error **errp);
+
+/**
  * cpu_reset:
  * @cpu: The CPU whose state is to be reset.
  */
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index cd5791e..c6e3ea0 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -190,6 +190,7 @@ char *get_boot_devices_list(size_t *size);
 DeviceState *get_boot_device(uint32_t position);
 
 QemuOpts *qemu_get_machine_opts(void);
+QemuOpts *qemu_get_cpu_opts(void);
 
 bool usb_enabled(bool default_usb);
 
diff --git a/qom/cpu.c b/qom/cpu.c
index 818fb26..231dec5 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -24,6 +24,8 @@
 #include "qemu/notify.h"
 #include "qemu/log.h"
 #include "sysemu/sysemu.h"
+#include "qapi/qmp/qerror.h"
+#include "qemu/config-file.h"
 
 bool cpu_exists(int64_t id)
 {
@@ -186,6 +188,31 @@ void cpu_reset(CPUState *cpu)
 }
 }
 
+static int cpu_set_property(const char *name, const char *value, void *opaque)
+{
+Error *err = NULL;
+
+if (strcmp(name, "type") == 0) {
+return 0;
+}
+
+object_property_parse(opaque, value, name, &err);
+if (err != NULL) {
+qerror_report_err(err);
+error_free(err);
+return -1;
+}
+
+return 0;
+}
+
+void cpu_default_parse_options_func(CPUState *cpu, Error **errp)
+{
+if (qemu_opt_foreach(qemu_get_cpu_opts(), cpu_set_property, cpu, 1)) {
+error_setg(errp, "Bad option");
+}
+}
+
 static void cpu_common_reset(CPUState *cpu)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
diff --git a/vl.c b/vl.c
index 4ad15b8..09f8e8b 100644
--- a/vl.c
+++ b/vl.c
@@ -433,6 +433,16 @@ static QemuOptsList qemu_machine_opts = {
 },
 };
 
+static QemuOptsList qemu_cpu_opts = {
+.name = "cpu",
+.implied_opt_name = "type",
+.merge_lists = true,
+.head = QTAILQ_HEAD_INITIALIZER(qemu_cpu_opts.head),
+.desc = {
+{ /* End of list */ }
+},
+};
+
 static QemuOptsList qemu_boot_opts = {
 .name = "boot-opts",
 .implied_opt_name = "order",
@@ -548,6 +558,25 @@ QemuOpts *qemu_get_machine_opts(void)
 return opts;
 }
 
+/**
+ * Get CPU options
+ *
+ * Returns: CPU options (never null).
+ */
+QemuOpts *qemu_get_cpu_opts(void)
+{
+QemuOptsList *list;
+QemuOpts *opts;
+
+list = qemu_find_opts("cpu");
+assert(list);
+opts = qemu_opts_find(list, NULL);
+if (!opts) {
+opts = qemu_opts_create_nofail(list);
+}
+return opts;
+}
+
 const char *qemu_get_vm_name(void)
 {
 return qemu_name;
@@ -2877,6 +2906,7 @@ int main(int argc, char **argv, char **envp)
 qemu_add_opts(&qemu_trace_opts);
 qemu_add_opts(&qemu_option_rom_opts);
 qemu_add_opts(&qemu_machine_opts);
+qemu_add_opts(&qemu_cpu_opts);
 qemu_add_opts(&qemu_smp_opts);
 qemu_add_opts(&qemu_boot_opts);
 qemu_add_opts(&qemu_sandbox_opts);
@@ -2972,7 +3002,19 @@ int main(int argc, char **argv, char **envp)
 }
 case QEMU_OPTION_cpu:
 /* hw initialization will check this */
+
+/* Store cpu_model for old style parser */
 cpu_model = optarg;
+
+/*
+ * Parse and save options in the cpu options list
+ * for a new parser
+ */
+olist = qemu_find_opts("cpu");
+opts = qemu_opts_parse(olist, optarg, 1);
+if (!opts) {
+exit(1);
+}
 break;
   

[Qemu-devel] [PATCH v3 4/6] qemu-option: support +foo/-foo command line agruments

[Alexey Kardashevskiy]

This converts +foo/-foo to "foo=on"/"foo=off" respectively when
QEMU parser is used for the command line options.

"-cpu" parsers in x86 and other architectures should be unaffected
by this change.

Signed-off-by: Alexey Kardashevskiy 
---
 util/qemu-option.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/util/qemu-option.c b/util/qemu-option.c
index efcb5dc..6c8667c 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -890,6 +890,12 @@ static int opts_do_parse(QemuOpts *opts, const char 
*params,
 if (strncmp(option, "no", 2) == 0) {
 memmove(option, option+2, strlen(option+2)+1);
 pstrcpy(value, sizeof(value), "off");
+} else if (strncmp(option, "-", 1) == 0) {
+memmove(option, option+1, strlen(option+1)+1);
+pstrcpy(value, sizeof(value), "off");
+} else if (strncmp(option, "+", 1) == 0) {
+memmove(option, option+1, strlen(option+1)+1);
+pstrcpy(value, sizeof(value), "on");
 } else {
 pstrcpy(value, sizeof(value), "on");
 }
-- 
1.8.4.rc4

[Qemu-devel] [PATCH v3 6/6] target-ppc: demonstrate new "vsx" property

[Alexey Kardashevskiy]

This patch is to demonstrate a static property handling in PowerPC.
Running QEMU with -cpu host,-vsx disables VSX bit in
PowerPCCPU::env::flags.

Signed-off-by: Alexey Kardashevskiy 
---
 target-ppc/translate_init.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index df0d81c..60ea235 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -29,6 +29,7 @@
 #include "mmu-hash64.h"
 #include "qemu/error-report.h"
 #include "qapi/visitor.h"
+#include "hw/qdev-properties.h"
 
 //#define PPC_DUMP_CPU
 //#define PPC_DEBUG_SPR
@@ -8740,6 +8741,12 @@ static void ppc_cpu_class_init(ObjectClass *oc, void 
*data)
 
 dc->fw_name = "PowerPC,UNKNOWN";
 cc->parse_options = cpu_default_parse_options_func;
+
+static Property powerpc_properties[] = {
+DEFINE_PROP_BIT("vsx", PowerPCCPU, env.flags, BITNR(POWERPC_FLAG_VSX), 
false),
+DEFINE_PROP_END_OF_LIST(),
+};
+dc->props = powerpc_properties;
 }
 
 static const TypeInfo ppc_cpu_type_info = {
-- 
1.8.4.rc4

[Qemu-devel] [PATCH v3 3/6] target-ppc: add "compat" CPU option

[Alexey Kardashevskiy]

To be able to boot a guest on the hardware which is newer than the guest
actually supports, PowerISA defines a logical PVR per every PowerISA
specification version from 2.04.

This adds the "compat" option which takes values 205 or 206 and forces
QEMU to boot the guest with a logical PVR (CPU_POWERPC_LOGICAL_2_05 or
CPU_POWERPC_LOGICAL_2_06) which is stored in the "cpu-version" property of
CPU device nodes.

Cc: Andreas Färber 
Signed-off-by: Alexey Kardashevskiy 
---
Changes:
v3:
* "compat" option accepts "power6" and "power7" now
---
 hw/ppc/spapr.c  |  9 +++
 target-ppc/cpu-models.h | 10 
 target-ppc/cpu.h|  4 +++
 target-ppc/translate_init.c | 61 +
 4 files changed, 84 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 2e18bdb..9fcbd96 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -206,6 +206,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment 
*spapr)
 
 CPU_FOREACH(cpu) {
 DeviceClass *dc = DEVICE_GET_CLASS(cpu);
+CPUPPCState *env = &POWERPC_CPU(cpu)->env;
 uint32_t associativity[] = {cpu_to_be32(0x5),
 cpu_to_be32(0x0),
 cpu_to_be32(0x0),
@@ -238,6 +239,14 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment 
*spapr)
 if (ret < 0) {
 return ret;
 }
+
+if (env->compat) {
+ret = fdt_setprop(fdt, offset, "cpu-version",
+  &env->compat, sizeof(env->compat));
+if (ret < 0) {
+return ret;
+}
+}
 }
 return ret;
 }
diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
index 49ba4a4..d7c033c 100644
--- a/target-ppc/cpu-models.h
+++ b/target-ppc/cpu-models.h
@@ -583,6 +583,16 @@ enum {
 CPU_POWERPC_RS64II = 0x0034,
 CPU_POWERPC_RS64III= 0x0036,
 CPU_POWERPC_RS64IV = 0x0037,
+
+/* Logical CPUs */
+CPU_POWERPC_LOGICAL_MASK   = 0x,
+CPU_POWERPC_LOGICAL_2_04   = 0x0F01,
+CPU_POWERPC_LOGICAL_2_05   = 0x0F02,
+CPU_POWERPC_LOGICAL_2_06   = 0x0F03,
+CPU_POWERPC_LOGICAL_2_06_PLUS  = 0x0F13,
+CPU_POWERPC_LOGICAL_2_07   = 0x0F04,
+CPU_POWERPC_LOGICAL_2_08   = 0x0F05,
+
 #endif /* defined(TARGET_PPC64) */
 /* Original POWER */
 /* XXX: should be POWER (RIOS), RSC3308, RSC4608,
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index bb84767..8e30518 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1006,6 +1006,9 @@ struct CPUPPCState {
 /* Device control registers */
 ppc_dcr_t *dcr_env;
 
+/* Architecture compatibility mode PVR */
+uint32_t compat;
+
 int dcache_line_size;
 int icache_line_size;
 
@@ -1330,6 +1333,7 @@ static inline int cpu_mmu_index (CPUPPCState *env)
 #define SPR_BOOKE_DVC1(0x13E)
 #define SPR_BOOKE_DVC2(0x13F)
 #define SPR_BOOKE_TSR (0x150)
+#define SPR_PCR   (0x152)
 #define SPR_BOOKE_TCR (0x154)
 #define SPR_BOOKE_TLB0PS  (0x158)
 #define SPR_BOOKE_TLB1PS  (0x159)
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 9e4af56..df0d81c 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -28,6 +28,7 @@
 #include "mmu-hash32.h"
 #include "mmu-hash64.h"
 #include "qemu/error-report.h"
+#include "qapi/visitor.h"
 
 //#define PPC_DUMP_CPU
 //#define PPC_DEBUG_SPR
@@ -7135,8 +7136,60 @@ POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data)
  POWERPC_FLAG_BUS_CLK;
 }
 
+static void powerpc_get_compat(Object *obj, Visitor *v,
+   void *opaque, const char *name, Error **errp)
+{
+PowerPCCPU *cpu = POWERPC_CPU(obj);
+char *value = (char *)"";
+
+switch (cpu->env.compat) {
+case CPU_POWERPC_LOGICAL_2_05:
+value = (char *)"power6";
+break;
+case CPU_POWERPC_LOGICAL_2_06:
+value = (char *)"power7";
+break;
+case 0:
+break;
+default:
+error_setg(errp, "Internal error: compat is set to %x",
+   cpu->env.compat);
+break;
+}
+
+visit_type_str(v, &value, name, errp);
+}
+
+static void powerpc_set_compat(Object *obj, Visitor *v,
+   void *opaque, const char *name, Error **errp)
+{
+PowerPCCPU *cpu = POWERPC_CPU(obj);
+Error *error = NULL;
+/* TODO: Implement check for the value length */
+char *value = NULL;
+
+visit_type_str(v, &value, name, &error);
+if (error) {
+error_propagate(errp, error);
+return;
+}
+
+if (strcmp(value, "power6") == 0) {
+cpu->env.compat = CPU_POWERPC_LOGICAL_2_05;
+} else if (strcmp(value, "power7") == 0) {
+cpu->env.compat = CPU_POWERPC_LOGICAL_2_06;
+} else {
+error_setg(errp, "Invalid compatibilit

Re: [Qemu-devel] [PATCH for-1.7] qdev-properties-system.c: Allow vlan or netdev for -device, not both

[Paolo Bonzini]

Il 11/11/2013 06:18, Jason Wang ha scritto:
> On 11/08/2013 10:13 AM, Vlad Yasevich wrote:
>> It is currently possible to specify things like:
>>  -device e1000,netdev=foo,vlan=1
>> With this usage, whichever argument was specified last (vlan or netdev)
>> overwrites what was previousely set and results in a non-working
>> configuration.  Even worse, when used with multiqueue devices,
>> it causes a segmentation fault on exit in qemu_free_net_client.
>>
>> That patch treates the above command line options as invalid and
>> generates an error at start-up.
>>
>> Signed-off-by: Vlad Yasevich 
>> ---
>>  hw/core/qdev-properties-system.c | 9 +
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/hw/core/qdev-properties-system.c 
>> b/hw/core/qdev-properties-system.c
>> index 0eada32..729efa8 100644
>> --- a/hw/core/qdev-properties-system.c
>> +++ b/hw/core/qdev-properties-system.c
>> @@ -205,6 +205,11 @@ static int parse_netdev(DeviceState *dev, const char 
>> *str, void **ptr)
>>  goto err;
>>  }
>>  
>> +if (ncs[i]) {
>> +ret = -EINVAL;
>> +goto err;
>> +}
>> +
>>  ncs[i] = peers[i];
>>  ncs[i]->queue_index = i;
>>  }
>> @@ -301,6 +306,10 @@ static void set_vlan(Object *obj, Visitor *v, void 
>> *opaque,
>>  *ptr = NULL;
>>  return;
>>  }
>> +if (*ptr) {
>> +error_set_from_qdev_prop_error(errp, -EINVAL, dev, prop, name);
>> +return;
>> +}
>>  
>>  hubport = net_hub_port_find(id);
>>  if (!hubport) {
> 
> Acked-by: Jason Wang 

This patch is good for 1.7.

Paolo

[Qemu-devel] [PATCH V6 2/6] qcow2: add error message in qcow2_write_snapshots()

[Wenchao Xia]

The function still returns int since qcow2_snapshot_delete() will
return the number.

Signed-off-by: Wenchao Xia 
Reviewed-by: Max Reitz 
---
 block/qcow2-snapshot.c |   43 +--
 1 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 532b7f6..6a1d9de 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -152,7 +152,7 @@ fail:
 }
 
 /* add at the end of the file a new list of snapshots */
-static int qcow2_write_snapshots(BlockDriverState *bs)
+static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp)
 {
 BDRVQcowState *s = bs->opaque;
 QCowSnapshot *sn;
@@ -183,10 +183,15 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
 offset = snapshots_offset;
 if (offset < 0) {
 ret = offset;
+error_setg_errno(errp, -ret,
+ "Failed in allocation of cluster for snapshot list");
 goto fail;
 }
 ret = bdrv_flush(bs);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in flush after snapshot list cluster "
+ "allocation");
 goto fail;
 }
 
@@ -194,6 +199,10 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
  * must indeed be completely free */
 ret = qcow2_pre_write_overlap_check(bs, 0, offset, snapshots_size);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in overlap check for snapshot list cluster "
+ "at %" PRIi64 " with size %d",
+ offset, snapshots_size);
 goto fail;
 }
 
@@ -227,24 +236,40 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
 
 ret = bdrv_pwrite(bs->file, offset, &h, sizeof(h));
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in write of snapshot header at %"
+ PRIi64 " with size %d",
+ offset, (int)sizeof(h));
 goto fail;
 }
 offset += sizeof(h);
 
 ret = bdrv_pwrite(bs->file, offset, &extra, sizeof(extra));
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in write of extra snapshot data at %"
+ PRIi64 " with size %d",
+ offset, (int)sizeof(extra));
 goto fail;
 }
 offset += sizeof(extra);
 
 ret = bdrv_pwrite(bs->file, offset, sn->id_str, id_str_size);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in write of snapshot id string at %"
+ PRIi64 " with size %d",
+ offset, id_str_size);
 goto fail;
 }
 offset += id_str_size;
 
 ret = bdrv_pwrite(bs->file, offset, sn->name, name_size);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in write of snapshot name string at %"
+ PRIi64 " with size %d",
+ offset, name_size);
 goto fail;
 }
 offset += name_size;
@@ -256,6 +281,8 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
  */
 ret = bdrv_flush(bs);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in flush after snapshot list update");
 goto fail;
 }
 
@@ -268,6 +295,10 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
 ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, nb_snapshots),
&header_data, sizeof(header_data));
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in update of image header at %d with size %d",
+ (int)offsetof(QCowHeader, nb_snapshots),
+ (int)sizeof(header_data));
 goto fail;
 }
 
@@ -283,6 +314,9 @@ fail:
 qcow2_free_clusters(bs, snapshots_offset, snapshots_size,
 QCOW2_DISCARD_ALWAYS);
 }
+if (errp) {
+g_assert(error_is_set(errp));
+}
 return ret;
 }
 
@@ -447,10 +481,8 @@ void qcow2_snapshot_create(BlockDriverState *bs,
 s->snapshots = new_snapshot_list;
 s->snapshots[s->nb_snapshots++] = *sn;
 
-ret = qcow2_write_snapshots(bs);
+ret = qcow2_write_snapshots(bs, errp);
 if (ret < 0) {
-/* Following line will be replaced with more detailed error later */
-error_setg(errp, "Failed in write of snapshot");
 g_free(s->snapshots);
 s->snapshots = old_snapshot_list;
 s->nb_snapshots--;
@@ -624,9 +656,8 @@ int qcow2_snapshot_delete(BlockDriverState *bs,
 s->snapshots + snapshot_index + 1,
 (s->nb_snapshots - snapshot_index - 1) * sizeof(sn));
 s->

[Qemu-devel] [PATCH V6 1/6] snapshot: add parameter *errp in snapshot create

[Wenchao Xia]

The return value is only used for error report before this patch,
so change the function protype to return void.

Signed-off-by: Wenchao Xia 
Reviewed-by: Max Reitz 
---
 block/qcow2-snapshot.c|   30 +-
 block/qcow2.h |4 +++-
 block/rbd.c   |   19 ++-
 block/sheepdog.c  |   28 ++--
 block/snapshot.c  |   19 +--
 blockdev.c|   10 --
 include/block/block_int.h |5 +++--
 include/block/snapshot.h  |5 +++--
 qemu-img.c|   10 ++
 savevm.c  |   12 
 10 files changed, 93 insertions(+), 49 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 3529c68..532b7f6 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -347,7 +347,9 @@ static int find_snapshot_by_id_or_name(BlockDriverState *bs,
 }
 
 /* if no id is provided, a new one is constructed */
-int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
+void qcow2_snapshot_create(BlockDriverState *bs,
+   QEMUSnapshotInfo *sn_info,
+   Error **errp)
 {
 BDRVQcowState *s = bs->opaque;
 QCowSnapshot *new_snapshot_list = NULL;
@@ -366,7 +368,8 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 
 /* Check that the ID is unique */
 if (find_snapshot_by_id_and_name(bs, sn_info->id_str, NULL) >= 0) {
-return -EEXIST;
+error_setg(errp, "Snapshot with id %s already exist", sn_info->id_str);
+return;
 }
 
 /* Populate sn with passed data */
@@ -382,7 +385,8 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 /* Allocate the L1 table of the snapshot and copy the current one there. */
 l1_table_offset = qcow2_alloc_clusters(bs, s->l1_size * sizeof(uint64_t));
 if (l1_table_offset < 0) {
-ret = l1_table_offset;
+error_setg_errno(errp, -l1_table_offset,
+ "Failed in allocation of snapshot L1 table");
 goto fail;
 }
 
@@ -397,12 +401,22 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 ret = qcow2_pre_write_overlap_check(bs, 0, sn->l1_table_offset,
 s->l1_size * sizeof(uint64_t));
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in overlap check for snapshot L1 table at %"
+ PRIu64 " with size %" PRIu64,
+ sn->l1_table_offset,
+ (uint64_t)(s->l1_size * sizeof(uint64_t)));
 goto fail;
 }
 
 ret = bdrv_pwrite(bs->file, sn->l1_table_offset, l1_table,
   s->l1_size * sizeof(uint64_t));
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in update of snapshot L1 table at %"
+ PRIu64 " with size %" PRIu64,
+ sn->l1_table_offset,
+ (uint64_t)(s->l1_size * sizeof(uint64_t)));
 goto fail;
 }
 
@@ -416,6 +430,10 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
  */
 ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 
1);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in update of refcount for snapshot at %"
+ PRIu64 " with size %d",
+ s->l1_table_offset, s->l1_size);
 goto fail;
 }
 
@@ -431,6 +449,8 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 
 ret = qcow2_write_snapshots(bs);
 if (ret < 0) {
+/* Following line will be replaced with more detailed error later */
+error_setg(errp, "Failed in write of snapshot");
 g_free(s->snapshots);
 s->snapshots = old_snapshot_list;
 s->nb_snapshots--;
@@ -452,14 +472,14 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
   qcow2_check_refcounts(bs, &result, 0);
 }
 #endif
-return 0;
+return;
 
 fail:
 g_free(sn->id_str);
 g_free(sn->name);
 g_free(l1_table);
 
-return ret;
+return;
 }
 
 /* copy the snapshot 'snapshot_name' into the current disk image */
diff --git a/block/qcow2.h b/block/qcow2.h
index 922e190..c0a3d01 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -481,7 +481,9 @@ int qcow2_zero_clusters(BlockDriverState *bs, uint64_t 
offset, int nb_sectors);
 int qcow2_expand_zero_clusters(BlockDriverState *bs);
 
 /* qcow2-snapshot.c functions */
-int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info);
+void qcow2_snapshot_create(BlockDriverState *bs,
+   QEMUSnapshotInfo *sn_info,
+   Error **errp);
 int qcow2_snapshot_goto(BlockDriverState *bs, 

[Qemu-devel] [PATCH V6 6/6] qemu-iotests: add test for qcow2 snapshot

[Wenchao Xia]

This test will focus on the low level procedure of qcow2 snapshot
operations, now it covers only the create operation. Overlap error
paths are not checked since no good way to trigger those errors.

Signed-off-by: Wenchao Xia 
---
 tests/qemu-iotests/070   |  216 ++
 tests/qemu-iotests/070.out   |   35 ++
 tests/qemu-iotests/common.filter |7 ++
 tests/qemu-iotests/group |1 +
 4 files changed, 259 insertions(+), 0 deletions(-)
 create mode 100755 tests/qemu-iotests/070
 create mode 100644 tests/qemu-iotests/070.out

diff --git a/tests/qemu-iotests/070 b/tests/qemu-iotests/070
new file mode 100755
index 000..154acc4
--- /dev/null
+++ b/tests/qemu-iotests/070
@@ -0,0 +1,216 @@
+#!/bin/bash
+#
+# qcow2 internal snapshot test
+#
+# Copyright (C) 2013 IBM, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+owner=xiaw...@linux.vnet.ibm.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+BLKDEBUG_CONF="$TEST_DIR/blkdebug.conf"
+
+_cleanup()
+{
+_cleanup_test_img
+rm "$BLKDEBUG_CONF"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+# only test qcow2
+_supported_fmt qcow2
+_supported_proto generic
+# bind the errno correctly and filter the output of image check and qemu-img,
+# if you want to run it on other OS
+_supported_os Linux
+
+
+IMGOPTS="compat=1.1"
+
+CLUSTER_SIZE=65536
+
+SIZE=1G
+
+BLKDBG_TEST_IMG="blkdebug:$BLKDEBUG_CONF:$TEST_IMG"
+
+errno=5
+
+once=on
+
+imm=off
+
+
+# Start test, note that the injected errors are related to qcow2's snapshot
+# logic closely, see qcow2-snapshot.c for more details.
+
+# path 1: fail in L1 table allocation for snapshot
+echo
+echo "Path 1: fail in allocation of L1 table"
+
+_make_test_img $SIZE
+
+cat > "$BLKDEBUG_CONF" <&1
+
+
+# path 2: fail in update new L1 table
+echo
+echo "Path 2: fail in update new L1 table for snapshot"
+
+_make_test_img $SIZE
+
+cat > "$BLKDEBUG_CONF" <&1 | _filter_number
+$QEMU_IMG snapshot -l "$TEST_IMG"
+_check_test_img 2>&1
+
+# path 3: fail in update refcount block before write snapshot list
+echo
+echo "Path 3: fail in update refcount block before write snapshot list"
+
+_make_test_img $SIZE
+
+cat > "$BLKDEBUG_CONF" <&1 | _filter_number
+$QEMU_IMG snapshot -l "$TEST_IMG"
+_check_test_img 2>&1
+
+# path 4: fail in snapshot list allocation or its flush it is possible
+# qcow2_alloc_clusters() not fail immediately since cache hit, but in any
+# case, no error should be found in image check.
+echo
+echo "Path 4: fail in snapshot list allocation or its flush"
+
+_make_test_img $SIZE
+
+cat > "$BLKDEBUG_CONF" <&1 | grep "Failed" | 
grep "allocation" | grep "list"`
+if ! test -z "$err"
+then
+echo "Error happens as expected"
+fi
+$QEMU_IMG snapshot -l "$TEST_IMG"
+_check_test_img 2>&1
+
+
+# path 5: fail in snapshot list update
+echo
+echo "Path 5: fail in snapshot list update"
+
+_make_test_img $SIZE
+
+cat > "$BLKDEBUG_CONF" <&1 | _filter_number
+$QEMU_IMG snapshot -l "$TEST_IMG"
+_check_test_img 2>&1
+
+# path 6: fail in flush after snapshot list update, no good way to trigger it,
+# since the cache is empty and makes flush do nothing in that call, so leave
+# this path not tested
+
+# path 7: fail in update qcow2 header, it would have leaked cluster since not
+# discard the allocated ones for safe reason, see qcow2-snapshot.c.
+echo
+echo "Path 7: fail in update qcow2 header"
+
+_make_test_img $SIZE
+
+cat > "$BLKDEBUG_CONF" <&1 | _filter_number
+$QEMU_IMG snapshot -l "$TEST_IMG"
+_check_test_img 2>&1 | _filter_number
+
+# path 8: fail in overlap check before update L1 table for snapshot
+# path 9: fail in overlap check before update snapshot list
+# Since those clusters are allocated at runtime, there is no good way to
+# make them overlap in this script, so skip those two paths now.
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/070.out b/tests/qemu-iotests/070.out
new file mode 100644
index 000..1969e3f
--- /dev/null
+++ b/tests/qemu-iotests/070.out
@@ -0,0 +1,35 @@
+QA output created by 070
+
+Path 1: fail in allocation of L1 table
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
+qemu-img: Could not create snapsho

[Qemu-devel] [PATCH V6 3/6] qcow2: do not free clusters when fail in header update in qcow2_write_snapshots

[Wenchao Xia]

Signed-off-by: Wenchao Xia 
Reviewed-by: Max Reitz 
---
 block/qcow2-snapshot.c |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 6a1d9de..70e329e 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -299,6 +299,14 @@ static int qcow2_write_snapshots(BlockDriverState *bs, 
Error **errp)
  "Failed in update of image header at %d with size %d",
  (int)offsetof(QCowHeader, nb_snapshots),
  (int)sizeof(header_data));
+
+/*
+ * If the snapshot data part has been updated on disk, then the
+ * clusters at snapshot_offset may be used in next snapshot operation.
+ * If we free those clusters in fail path, they may be allocated and
+ * made dirty causing damage, so skip cluster free to be safe.
+ */
+snapshots_offset = 0;
 goto fail;
 }
 
-- 
1.7.1