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

2013-11-10 Thread 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 pbonz...@redhat.com




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

2013-11-10 Thread 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

2013-11-10 Thread 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 lcapitul...@redhat.com
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com

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

2013-11-10 Thread 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 vasilis.liaskovi...@profitbricks.com 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 
vasilis.liaskovi...@profitbricks.com
Reviewed-by: Thilo Fromm t...@thilo-fromm.de
   
   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
+++ b/hw/i386/acpi-build.c
@@ -603,6 +603,7 @@ static inline char acpi_get_hex(uint32_t val)
 #define ACPI_PROC_OFFSET_CPUHEX (*ssdt_proc_name - *ssdt_proc_start + 
2)
 #define ACPI_PROC_OFFSET_CPUID1 (*ssdt_proc_name - *ssdt_proc_start + 
4)
 #define 

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

2013-11-10 Thread 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 vyase...@redhat.com
 ---
  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

2013-11-10 Thread 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

2013-11-10 Thread 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 marce...@redhat.com
---
 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

2013-11-10 Thread Marcel Apfelbaum
From: Michael S. Tsirkin m...@redhat.com

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 m...@redhat.com
Signed-off-by: Marcel Apfelbaum marce...@redhat.com
---
 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

2013-11-10 Thread 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)

2013-11-10 Thread 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 s...@weilnetz.de
---

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)

2013-11-10 Thread 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 s...@weilnetz.de
---

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

2013-11-10 Thread Stefan Weil
The local function console_print_text_attributes is no longer used since
commit 7d6ba01c3741bc32ae252bf64a5fd3f930c2df4f.

Signed-off-by: Stefan Weil s...@weilnetz.de
---
 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

2013-11-10 Thread Stefan Weil
Signed-off-by: Stefan Weil s...@weilnetz.de
---

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

2013-11-10 Thread Stefan Weil
Signed-off-by: Stefan Weil s...@weilnetz.de
---
 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 vte/vte.h
 #include math.h
 
+#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)

2013-11-10 Thread 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

2013-11-10 Thread 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

2013-11-10 Thread 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 s...@weilnetz.de
---

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 basetyps.h   /* STDAPI */
 #include qemu/compiler.h
 
 #ifdef __cplusplus
-- 
1.7.10.4




[Qemu-devel] Buildbot failures (MinGW)

2013-11-10 Thread 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

2013-11-10 Thread Dave Airlie
From: Dave Airlie airl...@redhat.com

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 airl...@redhat.com
---
 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 SDL.h
+
+#if SDL_MAJOR_VERSION == 1
 #include SDL_syswm.h
 
 #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 above copyright 

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

2013-11-10 Thread 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

2013-11-10 Thread 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 xiaw...@linux.vnet.ibm.com
---
  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 http://www.gnu.org/licenses/.
+#
+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 EOF
+[inject-error]
+event = cluster_alloc
+errno = $errno
+immediately = $imm
+once = $once
+EOF
+
+$QEMU_IMG snapshot -c snap1 $BLKDBG_TEST_IMG
+$QEMU_IMG snapshot -l $TEST_IMG


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

2013-11-10 Thread 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 xiaw...@linux.vnet.ibm.com
---
  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)

2013-11-10 Thread 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 mtosa...@redhat.com

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)
+{
+

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

2013-11-10 Thread 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

2013-11-10 Thread 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 ak...@redhat.com
---
 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

2013-11-10 Thread 赵小强

于 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;
  }

Indentation looks bad.


  s-idx = 

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

2013-11-10 Thread Anthony Liguori
On Sun, Nov 10, 2013 at 3:15 PM, Dave Airlie airl...@gmail.com wrote:
 From: Dave Airlie airl...@redhat.com

 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 airl...@redhat.com
 ---
  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-obj-$(CONFIG_CURSES) += curses.o
  common-obj-$(CONFIG_VNC) += $(vnc-obj-y)
  common-obj-$(CONFIG_GTK) += gtk.o x_keymap.o


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

2013-11-10 Thread 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 vyase...@redhat.com
 ---
  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 jasow...@redhat.com



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

2013-11-10 Thread Wenchao Xia
Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
 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 http://www.gnu.org/licenses/.
+#
+
+# 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,6 +161,7 @@ cat EOF
 

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

2013-11-10 Thread 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

2013-11-10 Thread 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 xiaw...@linux.vnet.ibm.com
---
 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 stdarg.h
 #include stdio.h
@@ -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,
+error_get_pretty(local_err));
+}
+

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

2013-11-10 Thread 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 xiaw...@linux.vnet.ibm.com
---
 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 

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

2013-11-10 Thread Wenchao Xia
Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
 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

2013-11-10 Thread Wenchao Xia
Now qemu-img convert have similar options as qemu-nbd for internal
snapshot.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
 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 specific options in a
 

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

2013-11-10 Thread Alexey Kardashevskiy
This enables -cpu subobtions parser. No option is supported yet.

Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
 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

2013-11-10 Thread 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 a...@ozlabs.ru
---
 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

2013-11-10 Thread 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

2013-11-10 Thread Alexey Kardashevskiy
This adds suboptions support for -cpu.

Cc: Andreas Färber afaer...@suse.de
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
 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

2013-11-10 Thread 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 a...@ozlabs.ru
---
 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

2013-11-10 Thread 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 a...@ozlabs.ru
---
 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

2013-11-10 Thread 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 afaer...@suse.de
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
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 compatibility mode \%s\, only 

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

2013-11-10 Thread 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 vyase...@redhat.com
 ---
  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 jasow...@redhat.com

This patch is good for 1.7.

Paolo



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

2013-11-10 Thread Wenchao Xia
The function still returns int since qcow2_snapshot_delete() will
return the number.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
Reviewed-by: Max Reitz mre...@redhat.com
---
 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-nb_snapshots--;
-

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

2013-11-10 Thread 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 xiaw...@linux.vnet.ibm.com
Reviewed-by: Max Reitz mre...@redhat.com
---
 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

2013-11-10 Thread 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 xiaw...@linux.vnet.ibm.com
---
 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 http://www.gnu.org/licenses/.
+#
+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 EOF
+[inject-error]
+event = cluster_alloc
+errno = $errno
+immediately = $imm
+once = $once
+EOF
+
+$QEMU_IMG snapshot -c snap1 $BLKDBG_TEST_IMG
+$QEMU_IMG snapshot -l $TEST_IMG
+_check_test_img 21
+
+
+# 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 EOF
+[inject-error]
+event = snapshot_l1_update
+errno = $errno
+immediately = $imm
+once = $once
+EOF
+
+$QEMU_IMG snapshot -c snap1 $BLKDBG_TEST_IMG 21 | _filter_number
+$QEMU_IMG snapshot -l $TEST_IMG
+_check_test_img 21
+
+# 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 EOF
+[set-state]
+state = 1
+event = snapshot_l1_update
+new_state = 2
+
+[inject-error]
+state = 2
+event = refblock_update_part
+errno = $errno
+immediately = $imm
+once = $once
+
+[set-state]
+state = 2
+event = refblock_alloc
+new_state = 3
+EOF
+
+$QEMU_IMG snapshot -c snap1 $BLKDBG_TEST_IMG 21 | _filter_number
+$QEMU_IMG snapshot -l $TEST_IMG
+_check_test_img 21
+
+# 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 EOF
+[set-state]
+state = 1
+event = snapshot_l1_update
+new_state = 2
+
+[inject-error]
+state = 2
+event = cluster_alloc
+errno = $errno
+immediately = $imm
+once = $once
+EOF
+
+# fail directly or in flush, are both OK.
+err=`$QEMU_IMG snapshot -c snap1 $BLKDBG_TEST_IMG 21 | 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 21
+
+
+# path 5: fail in snapshot list update
+echo
+echo Path 5: fail in snapshot list update
+
+_make_test_img $SIZE
+
+cat  $BLKDEBUG_CONF EOF
+[inject-error]
+event = snapshot_list_update
+errno = $errno
+immediately = $imm
+once = $once
+EOF
+
+$QEMU_IMG snapshot -c snap1 $BLKDBG_TEST_IMG 21 | _filter_number
+$QEMU_IMG snapshot -l $TEST_IMG
+_check_test_img 21
+
+# 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 

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

2013-11-10 Thread Wenchao Xia
Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
Reviewed-by: Max Reitz mre...@redhat.com
---
 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