[Qemu-devel] [PATCH arm-devs v1 14/15] xilinx_spips: lqspi: Push more data to tx-fifo

2013-04-02 Thread Peter Crosthwaite
Do 16 words per fifo flush. Increases performance and decreases
debug verbosity. This data depth has no real hardware analogue,
so just go with something that has reasonable performance.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---

 hw/xilinx_spips.c |   11 +++
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/xilinx_spips.c b/hw/xilinx_spips.c
index 5ac0f64..32d8db8 100644
--- a/hw/xilinx_spips.c
+++ b/hw/xilinx_spips.c
@@ -571,11 +571,14 @@ lqspi_read(void *opaque, hwaddr addr, unsigned int size)
 
 DB_PRINT_L(0, starting QSPI data read\n);
 
-for (i = 0; i  LQSPI_CACHE_SIZE / 4; ++i) {
-tx_data_bytes(s, 0, 4);
+while (cache_entry  LQSPI_CACHE_SIZE / 4) {
+for (i = 0; i  16; ++i) {
+tx_data_bytes(s, 0, 4);
+}
 xilinx_spips_flush_txfifo(s);
-rx_data_bytes(s, q-lqspi_buf[cache_entry], 4);
-cache_entry++;
+for (i = 0; i  16; ++i) {
+rx_data_bytes(s, q-lqspi_buf[cache_entry++], 4);
+}
 }
 
 s-regs[R_CONFIG] |= CS;
-- 
1.7.0.4




[Qemu-devel] [PATCH arm-devs v1 15/15] xilinx_spips: lqspi: Fix byte/misaligned access

2013-04-02 Thread Peter Crosthwaite
The LQSPI bus attachment supports byte/halfword and misaligned
accesses. Fixed. Refactored the LQSPI cache to be byte-wise
instead of word wise accordingly.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---

 hw/xilinx_spips.c |   31 +--
 1 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/hw/xilinx_spips.c b/hw/xilinx_spips.c
index 32d8db8..cb45a9c 100644
--- a/hw/xilinx_spips.c
+++ b/hw/xilinx_spips.c
@@ -160,7 +160,7 @@ typedef struct {
 typedef struct {
 XilinxSPIPS parent_obj;
 
-uint32_t lqspi_buf[LQSPI_CACHE_SIZE];
+uint8_t lqspi_buf[LQSPI_CACHE_SIZE];
 hwaddr lqspi_cached_addr;
 } XilinxQSPIPS;
 
@@ -366,14 +366,12 @@ static void xilinx_spips_flush_txfifo(XilinxSPIPS *s)
 }
 }
 
-static inline void rx_data_bytes(XilinxSPIPS *s, uint32_t *value, int max)
+static inline void rx_data_bytes(XilinxSPIPS *s, uint8_t *value, int max)
 {
 int i;
 
-*value = 0;
 for (i = 0; i  max  !fifo8_is_empty(s-rx_fifo); ++i) {
-uint32_t next = fifo8_pop(s-rx_fifo)  0xFF;
-*value |= next  8 * (s-regs[R_CONFIG]  ENDIAN ? 3-i : i);
+value[i] = fifo8_pop(s-rx_fifo);
 }
 }
 
@@ -383,6 +381,7 @@ static uint64_t xilinx_spips_read(void *opaque, hwaddr addr,
 XilinxSPIPS *s = opaque;
 uint32_t mask = ~0;
 uint32_t ret;
+uint8_t rx_buf[4];
 
 addr = 2;
 switch (addr) {
@@ -412,7 +411,10 @@ static uint64_t xilinx_spips_read(void *opaque, hwaddr 
addr,
 mask = 0;
 break;
 case R_RX_DATA:
-rx_data_bytes(s, ret, s-num_txrx_bytes);
+memset(rx_buf, 0, sizeof(rx_buf));
+rx_data_bytes(s, rx_buf, s-num_txrx_bytes);
+ret = s-regs[R_CONFIG]  ENDIAN ? cpu_to_be32(*(uint32_t *)rx_buf)
+: cpu_to_le32(*(uint32_t *)rx_buf);
 DB_PRINT_L(0, addr= TARGET_FMT_plx  = %x\n, addr * 4, ret);
 xilinx_spips_update_ixr(s);
 return ret;
@@ -525,7 +527,8 @@ lqspi_read(void *opaque, hwaddr addr, unsigned int size)
 
 if (addr = q-lqspi_cached_addr 
 addr = q-lqspi_cached_addr + LQSPI_CACHE_SIZE - 4) {
-ret = q-lqspi_buf[(addr - q-lqspi_cached_addr)  2];
+uint8_t *retp = q-lqspi_buf[addr - q-lqspi_cached_addr];
+ret = cpu_to_le32(*(uint32_t *)retp);
 DB_PRINT_L(1, addr: %08x, data: %08x\n, (unsigned)addr,
(unsigned)ret);
 return ret;
@@ -571,13 +574,13 @@ lqspi_read(void *opaque, hwaddr addr, unsigned int size)
 
 DB_PRINT_L(0, starting QSPI data read\n);
 
-while (cache_entry  LQSPI_CACHE_SIZE / 4) {
-for (i = 0; i  16; ++i) {
-tx_data_bytes(s, 0, 4);
+while (cache_entry  LQSPI_CACHE_SIZE) {
+for (i = 0; i  64; ++i) {
+tx_data_bytes(s, 0, 1);
 }
 xilinx_spips_flush_txfifo(s);
-for (i = 0; i  16; ++i) {
-rx_data_bytes(s, q-lqspi_buf[cache_entry++], 4);
+for (i = 0; i  64; ++i) {
+rx_data_bytes(s, q-lqspi_buf[cache_entry++], 1);
 }
 }
 
@@ -586,7 +589,7 @@ lqspi_read(void *opaque, hwaddr addr, unsigned int size)
 s-regs[R_CONFIG] = r_config_save;
 xilinx_spips_update_cs_lines(s);
 
-q-lqspi_cached_addr = addr;
+q-lqspi_cached_addr = flash_addr * num_effective_busses(s);
 return lqspi_read(opaque, addr, size);
 }
 }
@@ -595,7 +598,7 @@ static const MemoryRegionOps lqspi_ops = {
 .read = lqspi_read,
 .endianness = DEVICE_NATIVE_ENDIAN,
 .valid = {
-.min_access_size = 4,
+.min_access_size = 1,
 .max_access_size = 4
 }
 };
-- 
1.7.0.4




[Qemu-devel] [PATCH arm-devs v1 0/1] cadence_uart: U-boot driver

2013-04-02 Thread Peter Crosthwaite
Hi Peter,

This is a corner case bug discovered by U-boot driver for cadence GEM.

Regards,
Peter


Peter Crosthwaite (1):
  cadence_uart: Flush queued characters on reset

 hw/cadence_uart.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)




[Qemu-devel] [PATCH arm-devs v1 1/1] cadence_uart: Flush queued characters on reset

2013-04-02 Thread Peter Crosthwaite
Reset can be used to empty the rx-fifo. As the fifo full condition is
used to return false from can_receive, queued rx data should be flushed
on reset accordingly.

Cc: Wendy Liang jli...@xilinx.com
Cc: Jason Wu hua...@xilinx.com

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Reported-by: Jason Wu hua...@xilinx.com
---

 hw/cadence_uart.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/cadence_uart.c b/hw/cadence_uart.c
index 5426f10..421ec99 100644
--- a/hw/cadence_uart.c
+++ b/hw/cadence_uart.c
@@ -157,6 +157,7 @@ static void uart_rx_reset(UartState *s)
 {
 s-rx_wpos = 0;
 s-rx_count = 0;
+qemu_chr_accept_input(s-chr);
 
 s-r[R_SR] |= UART_SR_INTR_REMPTY;
 s-r[R_SR] = ~UART_SR_INTR_RFUL;
-- 
1.7.0.4




Re: [Qemu-devel] [PATCH V3 WIP 3/3] disable vhost_verify_ring_mappings check

2013-04-02 Thread Nicholas A. Bellinger
On Tue, 2013-04-02 at 21:04 -0700, Nicholas A. Bellinger wrote:
 On Tue, 2013-04-02 at 16:27 +0300, Michael S. Tsirkin wrote:
  On Mon, Apr 01, 2013 at 06:05:47PM -0700, Nicholas A. Bellinger wrote:
   On Fri, 2013-03-29 at 09:14 +0100, Paolo Bonzini wrote: 
Il 29/03/2013 03:53, Nicholas A. Bellinger ha scritto:
 On Thu, 2013-03-28 at 06:13 -0400, Paolo Bonzini wrote:
 I think it's the right thing to do, but maybe not the right place
 to do this, need to reset after all IO is done, before
 ring memory is write protected.

 Our emails are crossing each other unfortunately, but I want to
 reinforce this: ring memory is not write protected.
 
 Understood.  However, AFAICT the act of write protecting these ranges
 for ROM generates the offending callbacks to vhost_set_memory().
 
 The part that I'm missing is if ring memory is not being write 
 protected
 by make_bios_readonly_intel(), why are the vhost_set_memory() calls
 being invoked..?

Because mappings change for the region that contains the ring.  vhost
doesn't know yet that the changes do not affect ring memory,
vhost_set_memory() is called exactly to ascertain that.


SNIP

  
  Is it possible that what is going on here,
  is that we had a region at address 0x0 size 0x8000,
  and now a chunk from it is being made readonly,
  and to this end the whole old region is removed
  then new ones are added?
 
 Yes, I believe this is exactly what is happening..
 
  
  If yes maybe the problem is that we don't use the atomic
  begin/commit ops in the memory API.
  Maybe the following will help?
  Completely untested, posting just to give you the idea:
  
 
 Mmmm, one question on how vhost_region_del() + vhost_region_add() +
 vhost_commit() should work..
 
 Considering the following when the same seabios code snippet:
 
pci_config_writeb(0x31): bdf: 0x pam: 0x005b
 
 is executed to mark an pc.ram area 0xc as readonly:
 
 Entering vhost_begin 
 Entering vhost_region_del section: 0x7fd037a4bb60 offset_within_region: 
 0xc size: 2146697216 readonly: 0
 vhost_region_del: is_rom: 0, rom_device: 0
 vhost_region_del: readable: 1
 vhost_region_del: ram_addr 0x0, addr: 0x0 size: 2147483648
 vhost_region_del: name: pc.ram
 Entering vhost_set_memory, section: 0x7fd037a4bb60 add: 0, dev-started: 1
 vhost_set_memory: Setting dev-memory_changed = true for start_addr: 0xc
 Entering vhost_region_add section: 0x7fd037a4baa0 offset_within_region: 
 0xc size: 32768 readonly: 1
 vhost_region_add is readonly !!!
 vhost_region_add: is_rom: 0, rom_device: 0
 vhost_region_add: readable: 1
 vhost_region_add: ram_addr 0x, addr: 0x   0 size: 
 2147483648
 vhost_region_add: name: pc.ram
 Entering vhost_set_memory, section: 0x7fd037a4baa0 add: 1, dev-started: 1
 vhost_dev_assign_memory();  reg-guest_phys_addr: 
 0xc
 vhost_set_memory: Setting dev-memory_changed = true for start_addr: 0xc
 Entering vhost_region_add section: 0x7fd037a4baa0 offset_within_region: 
 0xc8000 size: 2146664448 readonly: 0
 vhost_region_add: is_rom: 0, rom_device: 0
 vhost_region_add: readable: 1
 vhost_region_add: ram_addr 0x, addr: 0x   0 size: 
 2147483648
 vhost_region_add: name: pc.ram
 Entering vhost_set_memory, section: 0x7fd037a4baa0 add: 1, dev-started: 1
 vhost_set_memory: Setting dev-memory_changed = true for start_addr: 0xc8000
 phys_page_find got PHYS_MAP_NODE_NIL ..
 Entering vhost_commit 
 
 Note that originally we'd see the cpu_physical_memory_map() failure in
 vhost_verify_ring_mappings() after the first -region_del() above.
 
 Adding a hardcoded cpu_physical_memory_map() testcase in vhost_commit()
 for phys_addr=0xed000, len=5124 (vq ring) does locate the correct
 *section from address_space_map(), which correct points to the section
 generated by the last vhost_region_add() above:
 
 Entering vhost_commit 
 address_space_map: addr: 0xed000, plen: 5124
 address_space_map: l: 4096, len: 5124
 address_space_map: section: 0x7f41b325f020 memory_region_is_ram: 1 readonly: 0
 address_space_map: section: 0x7f41b325f020 offset_within_region: 0xc8000 
 section size: 2146664448
 address_space_map: l: 4096, len: 1028
 address_space_map: section: 0x7f41b325f020 memory_region_is_ram: 1 readonly: 0
 address_space_map: section: 0x7f41b325f020 offset_within_region: 0xc8000 
 section size: 2146664448
 address_space_map: Calling qemu_ram_ptr_length: raddr: 0x   ed000 
 rlen: 5124
 address_space_map: After qemu_ram_ptr_length: raddr: 0x   ed000 rlen: 
 5124
 cpu_physical_memory_map(0xed000) got l: 5124
 
 So, does using a -commit callback for MemoryListener  mean that
 vhost_verify_ring_mappings() is OK to be called only from the final
 -commit callback, and not from each -region_del + -region_add
 callback..?   Eg: I seem to recall something about
 vhost_verify_ring_mappings() being called during each 

Re: [Qemu-devel] Regarding migration

2013-04-02 Thread Shehbaz Jaffer
All of migration code can be found in migrate.c file.

To understand migration, you can run qemu in gdb and place breakpoints in
do_savevm() function on server side and do_loadvm() function on client
side.

Migration consists of the following parts:
1. save device driver state (11 in number. this consists of keyboard, mouse
and other device drivers on server side)
2. RAM (this is also structured like other device drivers but there is a
special way in which this device driver is saved).
3. Disk (saved in the form of block device).

first you make a client to sleep over a port. i.e. client waits for server
to initiate migration.
on server, you give command to savevm.

the entry point is do_savevm() from which 3 things happen:
1.  qemu_savevm_state_begin() is called, ram is snapshotted for the first
time, all pages are marked dirty and thus saved.
2. qemu_savevm_state_iterate() is called, from which ram is snapshotted
time and again till either of the two things happen:
1. no of iterations go more than 30.
2. rate at which pages get dirty decreases in two successive iterations.
[NOTE that till here only RAM was being saved, the rest of the 12 device
drivers have not been saved yet]
==
 vm_stop() is called. this leads to halt of server vm(); no more ram pages
get dirty
==
3. qemu_savevm_state_complete() is called where rest of the 12 device
driver state is saved and 1 final iteration of saving ram is done.

this is device, ram and block(disk) state transferred via tcp / file to
client on a port / fd. (depending on what client was listening to)

At client end, the reverse sequence is followed i,e. first you load the
device driver, then the ram image and so on. and then vm_start() is called.

Hence, entry point for starting migration = do_savevm().  entry point for
receiving migration = do_loadvm().

more about live migration can be read in this amazing paper by Avi Kivity
::
www.linux-*kvm*.com/sites/default/files/*kivity*-Reprint.pdf

Regards,

Shehbaz Jaffer
Grad Student, IIT Delhi.

On Wed, Apr 3, 2013 at 3:07 AM, pavan pavu.ku...@gmail.com wrote:

 Hi.. I was wondering what are the entry points for the migration at
 source and destination.

 My understanding is that at source the method do_migrate in
 migrate.c file is the entry point and at the destination the method
 main in vl.c is the entry point.

 Is this correct ?

 --
Pavan Kumar M
Software Engineer.. not anymore though..
North Carolina State University




-- 
Shehbaz Jaffer
Graduate Student
Department of Computer Engineering
Indian Institute of Technology, Delhi


[Qemu-devel] [PATCH v5 00/16] Stream Patches

2013-04-02 Thread Peter Crosthwaite
Hi all. The Xilinx AXIEnet and DMA devices have two AXI stream connections
(control and data), only one of which is currently modelled (data). AXI stream
is modelled using the stream QOM interface described in stream.h. Unfortunately,
interfaces have no nice way of modelling multiple connections of the same type.
So to overcome this I created a secondary object which acts as a proxy for the
stream connection. Multiple connections can be implemented using multiple
proxies and stream masters link to the relevant proxy, rather than the ethernet
device itself. This Series changes AXI Enet and DMA to be connected as such.

Also changed the stream interface to implement flow control handshaking. This
is needed for the AXIEnet to be be able to implement the net can_receive() flow
control.

Patches 1-10 are low-impact cleanup of axienet/dma as per the current QOM
styling guidelines and can be cherry-picked off the front.

changed from v4:
resynchronized control flow stream API
reordered series for better consistency.
Folded patch 17 in 16 (app array length fix)
Fixed DMA tx path halted bit
changed from v3:
Changed from asynchronous flow control to synchronous (Edgar review)
changed from v2:
Reordered patches (from low impact - high impact)
Added styling refactoring of AXIDMA
Added asynchronous patches
Removed dummy second stream connection patch (former patch 8)
Added (functional) second stream connection
changed from v1:
Removed former P12 (already merged)
Address Andreas review
Refactor axienet to be more QOM friendly.


Peter Crosthwaite (16):
  xilinx_axienet: typedef XilinxAXIEnet struct
  xilinx_axienet: Defined and use type cast macro
  xilinx_axienet: Register reset properly
  xilinx_axienet: converted init-realize
  xilinx_axidma: typedef XilinxAXIDMA struct
  xilinx_axidma: Defined and use type cast macro
  xilinx_axidma: Register reset properly
  xilinx_axidma: converted init-realize
  petalogix_ml605_mmu: Fix machine node attachment
  petalogix_ml605_mmu: Attach ethernet to machine
  xilinx_axienet: Create Proxy object for stream
  xilinx_axidma: Create Proxy object for stream
  xilinx_axidma: Fix rx/tx halted bit.
  stream: Add flow control API
  xilinx_axienet/dma: Implement rx path flow control
  stream: Remove app argument hack

 hw/microblaze/petalogix_ml605_mmu.c |   28 +++-
 hw/stream.c |   15 ++-
 hw/stream.h |   36 -
 hw/xilinx.h |   21 ++-
 hw/xilinx_axidma.c  |  261 +++
 hw/xilinx_axienet.c |  255 +++---
 6 files changed, 483 insertions(+), 133 deletions(-)




[Qemu-devel] [PATCH v5 01/16] xilinx_axienet: typedef XilinxAXIEnet struct

2013-04-02 Thread Peter Crosthwaite
Typedef xilinx_axienets object state struct to shorten the repeated usages of
struct XilinxAXIEnet.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Reviewed-by: Andreas Färber afaer...@suse.de
Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com
---

 hw/xilinx_axienet.c |   44 +++-
 1 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c
index 5785290..c3c0663 100644
--- a/hw/xilinx_axienet.c
+++ b/hw/xilinx_axienet.c
@@ -306,6 +306,8 @@ struct TEMAC  {
 void *parent;
 };
 
+typedef struct XilinxAXIEnet XilinxAXIEnet;
+
 struct XilinxAXIEnet {
 SysBusDevice busdev;
 MemoryRegion iomem;
@@ -365,37 +367,37 @@ struct XilinxAXIEnet {
 uint8_t *rxmem;
 };
 
-static void axienet_rx_reset(struct XilinxAXIEnet *s)
+static void axienet_rx_reset(XilinxAXIEnet *s)
 {
 s-rcw[1] = RCW1_JUM | RCW1_FCS | RCW1_RX | RCW1_VLAN;
 }
 
-static void axienet_tx_reset(struct XilinxAXIEnet *s)
+static void axienet_tx_reset(XilinxAXIEnet *s)
 {
 s-tc = TC_JUM | TC_TX | TC_VLAN;
 }
 
-static inline int axienet_rx_resetting(struct XilinxAXIEnet *s)
+static inline int axienet_rx_resetting(XilinxAXIEnet *s)
 {
 return s-rcw[1]  RCW1_RST;
 }
 
-static inline int axienet_rx_enabled(struct XilinxAXIEnet *s)
+static inline int axienet_rx_enabled(XilinxAXIEnet *s)
 {
 return s-rcw[1]  RCW1_RX;
 }
 
-static inline int axienet_extmcf_enabled(struct XilinxAXIEnet *s)
+static inline int axienet_extmcf_enabled(XilinxAXIEnet *s)
 {
 return !!(s-regs[R_RAF]  RAF_EMCF_EN);
 }
 
-static inline int axienet_newfunc_enabled(struct XilinxAXIEnet *s)
+static inline int axienet_newfunc_enabled(XilinxAXIEnet *s)
 {
 return !!(s-regs[R_RAF]  RAF_NEWFUNC_EN);
 }
 
-static void axienet_reset(struct XilinxAXIEnet *s)
+static void axienet_reset(XilinxAXIEnet *s)
 {
 axienet_rx_reset(s);
 axienet_tx_reset(s);
@@ -406,7 +408,7 @@ static void axienet_reset(struct XilinxAXIEnet *s)
 s-emmc = EMMC_LINKSPEED_100MB;
 }
 
-static void enet_update_irq(struct XilinxAXIEnet *s)
+static void enet_update_irq(XilinxAXIEnet *s)
 {
 s-regs[R_IP] = s-regs[R_IS]  s-regs[R_IE];
 qemu_set_irq(s-irq, !!s-regs[R_IP]);
@@ -414,7 +416,7 @@ static void enet_update_irq(struct XilinxAXIEnet *s)
 
 static uint64_t enet_read(void *opaque, hwaddr addr, unsigned size)
 {
-struct XilinxAXIEnet *s = opaque;
+XilinxAXIEnet *s = opaque;
 uint32_t r = 0;
 addr = 2;
 
@@ -506,7 +508,7 @@ static uint64_t enet_read(void *opaque, hwaddr addr, 
unsigned size)
 static void enet_write(void *opaque, hwaddr addr,
uint64_t value, unsigned size)
 {
-struct XilinxAXIEnet *s = opaque;
+XilinxAXIEnet *s = opaque;
 struct TEMAC *t = s-TEMAC;
 
 addr = 2;
@@ -618,7 +620,7 @@ static const MemoryRegionOps enet_ops = {
 
 static int eth_can_rx(NetClientState *nc)
 {
-struct XilinxAXIEnet *s = qemu_get_nic_opaque(nc);
+XilinxAXIEnet *s = qemu_get_nic_opaque(nc);
 
 /* RX enabled?  */
 return !axienet_rx_resetting(s)  axienet_rx_enabled(s);
@@ -641,7 +643,7 @@ static int enet_match_addr(const uint8_t *buf, uint32_t f0, 
uint32_t f1)
 
 static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
 {
-struct XilinxAXIEnet *s = qemu_get_nic_opaque(nc);
+XilinxAXIEnet *s = qemu_get_nic_opaque(nc);
 static const unsigned char sa_bcast[6] = {0xff, 0xff, 0xff,
   0xff, 0xff, 0xff};
 static const unsigned char sa_ipmcast[3] = {0x01, 0x00, 0x52};
@@ -786,7 +788,7 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t 
*buf, size_t size)
 static void eth_cleanup(NetClientState *nc)
 {
 /* FIXME.  */
-struct XilinxAXIEnet *s = qemu_get_nic_opaque(nc);
+XilinxAXIEnet *s = qemu_get_nic_opaque(nc);
 g_free(s-rxmem);
 g_free(s);
 }
@@ -794,7 +796,7 @@ static void eth_cleanup(NetClientState *nc)
 static void
 axienet_stream_push(StreamSlave *obj, uint8_t *buf, size_t size, uint32_t *hdr)
 {
-struct XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj));
+XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj));
 
 /* TX enable ?  */
 if (!(s-tc  TC_TX)) {
@@ -844,7 +846,7 @@ static NetClientInfo net_xilinx_enet_info = {
 
 static int xilinx_enet_init(SysBusDevice *dev)
 {
-struct XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), dev);
+XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), dev);
 
 sysbus_init_irq(dev, s-irq);
 
@@ -869,7 +871,7 @@ static int xilinx_enet_init(SysBusDevice *dev)
 
 static void xilinx_enet_initfn(Object *obj)
 {
-struct XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj));
+XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj));
 Error *errp = NULL;
 
 object_property_add_link(obj, axistream-connected, TYPE_STREAM_SLAVE,
@@ -878,10 +880,10 @@ static void xilinx_enet_initfn(Object *obj)
 }
 
 static Property 

[Qemu-devel] [PATCH v5 02/16] xilinx_axienet: Defined and use type cast macro

2013-04-02 Thread Peter Crosthwaite
Standard QOM cast macro. Replaces usages of FROM_SYSBUS

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Reviewed-by: Andreas Färber afaer...@suse.de
Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com
---

 hw/xilinx_axienet.c |   11 ---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c
index c3c0663..4c7d390 100644
--- a/hw/xilinx_axienet.c
+++ b/hw/xilinx_axienet.c
@@ -32,6 +32,11 @@
 
 #define DPHY(x)
 
+#define TYPE_XILINX_AXI_ENET xlnx.axi-ethernet
+
+#define XILINX_AXI_ENET(obj) \
+ OBJECT_CHECK(XilinxAXIEnet, (obj), TYPE_XILINX_AXI_ENET)
+
 /* Advertisement control register. */
 #define ADVERTISE_10HALF0x0020  /* Try for 10mbps half-duplex  */
 #define ADVERTISE_10FULL0x0040  /* Try for 10mbps full-duplex  */
@@ -846,7 +851,7 @@ static NetClientInfo net_xilinx_enet_info = {
 
 static int xilinx_enet_init(SysBusDevice *dev)
 {
-XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), dev);
+XilinxAXIEnet *s = XILINX_AXI_ENET(dev);
 
 sysbus_init_irq(dev, s-irq);
 
@@ -871,7 +876,7 @@ static int xilinx_enet_init(SysBusDevice *dev)
 
 static void xilinx_enet_initfn(Object *obj)
 {
-XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj));
+XilinxAXIEnet *s = XILINX_AXI_ENET(obj);
 Error *errp = NULL;
 
 object_property_add_link(obj, axistream-connected, TYPE_STREAM_SLAVE,
@@ -899,7 +904,7 @@ static void xilinx_enet_class_init(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo xilinx_enet_info = {
-.name  = xlnx.axi-ethernet,
+.name  = TYPE_XILINX_AXI_ENET,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(XilinxAXIEnet),
 .class_init= xilinx_enet_class_init,
-- 
1.7.0.4




[Qemu-devel] [PATCH v5 03/16] xilinx_axienet: Register reset properly

2013-04-02 Thread Peter Crosthwaite
Register the reset function and the Device::reset function rather than
explicitly call it from the sysbus::init.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Reviewed-by: Andreas Färber afaer...@suse.de
Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com
---

 hw/xilinx_axienet.c |6 --
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c
index 4c7d390..35b4e4f 100644
--- a/hw/xilinx_axienet.c
+++ b/hw/xilinx_axienet.c
@@ -402,8 +402,10 @@ static inline int axienet_newfunc_enabled(XilinxAXIEnet *s)
 return !!(s-regs[R_RAF]  RAF_NEWFUNC_EN);
 }
 
-static void axienet_reset(XilinxAXIEnet *s)
+static void xilinx_axienet_reset(DeviceState *d)
 {
+XilinxAXIEnet *s = XILINX_AXI_ENET(d);
+
 axienet_rx_reset(s);
 axienet_tx_reset(s);
 
@@ -869,7 +871,6 @@ static int xilinx_enet_init(SysBusDevice *dev)
 s-TEMAC.parent = s;
 
 s-rxmem = g_malloc(s-c_rxmem);
-axienet_reset(s);
 
 return 0;
 }
@@ -900,6 +901,7 @@ static void xilinx_enet_class_init(ObjectClass *klass, void 
*data)
 
 k-init = xilinx_enet_init;
 dc-props = xilinx_enet_properties;
+dc-reset = xilinx_axienet_reset;
 ssc-push = axienet_stream_push;
 }
 
-- 
1.7.0.4




[Qemu-devel] [PATCH v5 04/16] xilinx_axienet: converted init-realize

2013-04-02 Thread Peter Crosthwaite
The prescribed transition from SysBusDevice::init to Device::realize. Im going
with Andreas suggestion to move the sysbus foo to Object::init for early IRQ
visibility.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Reviewed-by: Andreas Färber afaer...@suse.de
Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com
---
Changed from v3: Added missing include of qerror

 hw/xilinx_axidma.c  |1 +
 hw/xilinx_axienet.c |   24 +++-
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c
index 8db1a74..5d2b33a 100644
--- a/hw/xilinx_axidma.c
+++ b/hw/xilinx_axidma.c
@@ -27,6 +27,7 @@
 #include hw/ptimer.h
 #include qemu/log.h
 #include hw/qdev-addr.h
+#include qapi/qmp/qerror.h
 
 #include hw/stream.h
 
diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c
index 35b4e4f..ec1e893 100644
--- a/hw/xilinx_axienet.c
+++ b/hw/xilinx_axienet.c
@@ -851,18 +851,13 @@ static NetClientInfo net_xilinx_enet_info = {
 .cleanup = eth_cleanup,
 };
 
-static int xilinx_enet_init(SysBusDevice *dev)
+static void xilinx_enet_realize(DeviceState *dev, Error **errp)
 {
 XilinxAXIEnet *s = XILINX_AXI_ENET(dev);
 
-sysbus_init_irq(dev, s-irq);
-
-memory_region_init_io(s-iomem, enet_ops, s, enet, 0x4);
-sysbus_init_mmio(dev, s-iomem);
-
 qemu_macaddr_default_if_unset(s-conf.macaddr);
 s-nic = qemu_new_nic(net_xilinx_enet_info, s-conf,
-  object_get_typename(OBJECT(dev)), dev-qdev.id, s);
+  object_get_typename(OBJECT(dev)), dev-id, s);
 qemu_format_nic_info_str(qemu_get_queue(s-nic), s-conf.macaddr.a);
 
 tdk_init(s-TEMAC.phy);
@@ -871,18 +866,22 @@ static int xilinx_enet_init(SysBusDevice *dev)
 s-TEMAC.parent = s;
 
 s-rxmem = g_malloc(s-c_rxmem);
-
-return 0;
 }
 
-static void xilinx_enet_initfn(Object *obj)
+static void xilinx_enet_init(Object *obj)
 {
 XilinxAXIEnet *s = XILINX_AXI_ENET(obj);
+SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 Error *errp = NULL;
 
 object_property_add_link(obj, axistream-connected, TYPE_STREAM_SLAVE,
  (Object **) s-tx_dev, errp);
 assert_no_error(errp);
+
+sysbus_init_irq(sbd, s-irq);
+
+memory_region_init_io(s-iomem, enet_ops, s, enet, 0x4);
+sysbus_init_mmio(sbd, s-iomem);
 }
 
 static Property xilinx_enet_properties[] = {
@@ -896,10 +895,9 @@ static Property xilinx_enet_properties[] = {
 static void xilinx_enet_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 StreamSlaveClass *ssc = STREAM_SLAVE_CLASS(klass);
 
-k-init = xilinx_enet_init;
+dc-realize = xilinx_enet_realize;
 dc-props = xilinx_enet_properties;
 dc-reset = xilinx_axienet_reset;
 ssc-push = axienet_stream_push;
@@ -910,7 +908,7 @@ static const TypeInfo xilinx_enet_info = {
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(XilinxAXIEnet),
 .class_init= xilinx_enet_class_init,
-.instance_init = xilinx_enet_initfn,
+.instance_init = xilinx_enet_init,
 .interfaces = (InterfaceInfo[]) {
 { TYPE_STREAM_SLAVE },
 { }
-- 
1.7.0.4




[Qemu-devel] [PATCH v5 05/16] xilinx_axidma: typedef XilinxAXIDMA struct

2013-04-02 Thread Peter Crosthwaite
Typedef xilinx_axidma's object state struct to shorten the repeated usages of
struct XilinxAXIDMA.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com
---

 hw/xilinx_axidma.c |   16 +---
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c
index 5d2b33a..d57538e 100644
--- a/hw/xilinx_axidma.c
+++ b/hw/xilinx_axidma.c
@@ -39,6 +39,8 @@
 #define R_TAILDESC  (0x10 / 4)
 #define R_MAX   (0x30 / 4)
 
+typedef struct XilinxAXIDMA XilinxAXIDMA;
+
 enum {
 DMACR_RUNSTOP = 1,
 DMACR_TAILPTR_MODE = 2,
@@ -354,7 +356,7 @@ static void stream_process_s2mem(struct Stream *s,
 static void
 axidma_push(StreamSlave *obj, unsigned char *buf, size_t len, uint32_t *app)
 {
-struct XilinxAXIDMA *d = FROM_SYSBUS(typeof(*d), SYS_BUS_DEVICE(obj));
+XilinxAXIDMA *d = FROM_SYSBUS(typeof(*d), SYS_BUS_DEVICE(obj));
 struct Stream *s = d-streams[1];
 
 if (!app) {
@@ -367,7 +369,7 @@ axidma_push(StreamSlave *obj, unsigned char *buf, size_t 
len, uint32_t *app)
 static uint64_t axidma_read(void *opaque, hwaddr addr,
 unsigned size)
 {
-struct XilinxAXIDMA *d = opaque;
+XilinxAXIDMA *d = opaque;
 struct Stream *s;
 uint32_t r = 0;
 int sid;
@@ -402,7 +404,7 @@ static uint64_t axidma_read(void *opaque, hwaddr addr,
 static void axidma_write(void *opaque, hwaddr addr,
  uint64_t value, unsigned size)
 {
-struct XilinxAXIDMA *d = opaque;
+XilinxAXIDMA *d = opaque;
 struct Stream *s;
 int sid;
 
@@ -460,7 +462,7 @@ static const MemoryRegionOps axidma_ops = {
 
 static int xilinx_axidma_init(SysBusDevice *dev)
 {
-struct XilinxAXIDMA *s = FROM_SYSBUS(typeof(*s), dev);
+XilinxAXIDMA *s = FROM_SYSBUS(typeof(*s), dev);
 int i;
 
 sysbus_init_irq(dev, s-streams[0].irq);
@@ -482,14 +484,14 @@ static int xilinx_axidma_init(SysBusDevice *dev)
 
 static void xilinx_axidma_initfn(Object *obj)
 {
-struct XilinxAXIDMA *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj));
+XilinxAXIDMA *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj));
 
 object_property_add_link(obj, axistream-connected, TYPE_STREAM_SLAVE,
  (Object **) s-tx_dev, NULL);
 }
 
 static Property axidma_properties[] = {
-DEFINE_PROP_UINT32(freqhz, struct XilinxAXIDMA, freqhz, 5000),
+DEFINE_PROP_UINT32(freqhz, XilinxAXIDMA, freqhz, 5000),
 DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -507,7 +509,7 @@ static void axidma_class_init(ObjectClass *klass, void 
*data)
 static const TypeInfo axidma_info = {
 .name  = xlnx.axi-dma,
 .parent= TYPE_SYS_BUS_DEVICE,
-.instance_size = sizeof(struct XilinxAXIDMA),
+.instance_size = sizeof(XilinxAXIDMA),
 .class_init= axidma_class_init,
 .instance_init = xilinx_axidma_initfn,
 .interfaces = (InterfaceInfo[]) {
-- 
1.7.0.4




[Qemu-devel] [PATCH v5 06/16] xilinx_axidma: Defined and use type cast macro

2013-04-02 Thread Peter Crosthwaite
Standard QOM cast macro. Replaces usages of FROM_SYSBUS

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com
---

 hw/xilinx_axidma.c |   13 +
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c
index d57538e..7cb40bd 100644
--- a/hw/xilinx_axidma.c
+++ b/hw/xilinx_axidma.c
@@ -33,6 +33,11 @@
 
 #define D(x)
 
+#define TYPE_XILINX_AXI_DMA xlnx.axi-dma
+
+#define XILINX_AXI_DMA(obj) \
+ OBJECT_CHECK(XilinxAXIDMA, (obj), TYPE_XILINX_AXI_DMA)
+
 #define R_DMACR (0x00 / 4)
 #define R_DMASR (0x04 / 4)
 #define R_CURDESC   (0x08 / 4)
@@ -356,7 +361,7 @@ static void stream_process_s2mem(struct Stream *s,
 static void
 axidma_push(StreamSlave *obj, unsigned char *buf, size_t len, uint32_t *app)
 {
-XilinxAXIDMA *d = FROM_SYSBUS(typeof(*d), SYS_BUS_DEVICE(obj));
+XilinxAXIDMA *d = XILINX_AXI_DMA(obj);
 struct Stream *s = d-streams[1];
 
 if (!app) {
@@ -462,7 +467,7 @@ static const MemoryRegionOps axidma_ops = {
 
 static int xilinx_axidma_init(SysBusDevice *dev)
 {
-XilinxAXIDMA *s = FROM_SYSBUS(typeof(*s), dev);
+XilinxAXIDMA *s = XILINX_AXI_DMA(dev);
 int i;
 
 sysbus_init_irq(dev, s-streams[0].irq);
@@ -484,7 +489,7 @@ static int xilinx_axidma_init(SysBusDevice *dev)
 
 static void xilinx_axidma_initfn(Object *obj)
 {
-XilinxAXIDMA *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj));
+XilinxAXIDMA *s = XILINX_AXI_DMA(obj);
 
 object_property_add_link(obj, axistream-connected, TYPE_STREAM_SLAVE,
  (Object **) s-tx_dev, NULL);
@@ -507,7 +512,7 @@ static void axidma_class_init(ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo axidma_info = {
-.name  = xlnx.axi-dma,
+.name  = TYPE_XILINX_AXI_DMA,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(XilinxAXIDMA),
 .class_init= axidma_class_init,
-- 
1.7.0.4




[Qemu-devel] [PATCH v5 07/16] xilinx_axidma: Register reset properly

2013-04-02 Thread Peter Crosthwaite
Register the reset function as the Device::reset function rather than
explicitly call it from the sysbus::init.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com
---

 hw/xilinx_axidma.c |   12 +++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c
index 7cb40bd..ac62245 100644
--- a/hw/xilinx_axidma.c
+++ b/hw/xilinx_axidma.c
@@ -358,6 +358,16 @@ static void stream_process_s2mem(struct Stream *s,
 }
 }
 
+static void xilinx_axidma_reset(DeviceState *dev)
+{
+int i;
+XilinxAXIDMA *s = XILINX_AXI_DMA(dev);
+
+for (i = 0; i  2; i++) {
+stream_reset(s-streams[i]);
+}
+}
+
 static void
 axidma_push(StreamSlave *obj, unsigned char *buf, size_t len, uint32_t *app)
 {
@@ -478,7 +488,6 @@ static int xilinx_axidma_init(SysBusDevice *dev)
 sysbus_init_mmio(dev, s-iomem);
 
 for (i = 0; i  2; i++) {
-stream_reset(s-streams[i]);
 s-streams[i].nr = i;
 s-streams[i].bh = qemu_bh_new(timer_hit, s-streams[i]);
 s-streams[i].ptimer = ptimer_init(s-streams[i].bh);
@@ -507,6 +516,7 @@ static void axidma_class_init(ObjectClass *klass, void 
*data)
 StreamSlaveClass *ssc = STREAM_SLAVE_CLASS(klass);
 
 k-init = xilinx_axidma_init;
+dc-reset = xilinx_axidma_reset;
 dc-props = axidma_properties;
 ssc-push = axidma_push;
 }
-- 
1.7.0.4




[Qemu-devel] [PATCH v5 08/16] xilinx_axidma: converted init-realize

2013-04-02 Thread Peter Crosthwaite
The prescribed transition from SysBusDevice::init to Device::realize. I'm going
with Andreas suggestion to move the sysbus foo to Object::init for early IRQ
visibility.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com
---

 hw/xilinx_axidma.c |   25 -
 1 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c
index ac62245..2c95765 100644
--- a/hw/xilinx_axidma.c
+++ b/hw/xilinx_axidma.c
@@ -475,33 +475,33 @@ static const MemoryRegionOps axidma_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static int xilinx_axidma_init(SysBusDevice *dev)
+static void xilinx_axidma_realize(DeviceState *dev, Error **errp)
 {
 XilinxAXIDMA *s = XILINX_AXI_DMA(dev);
 int i;
 
-sysbus_init_irq(dev, s-streams[0].irq);
-sysbus_init_irq(dev, s-streams[1].irq);
-
-memory_region_init_io(s-iomem, axidma_ops, s,
-  xlnx.axi-dma, R_MAX * 4 * 2);
-sysbus_init_mmio(dev, s-iomem);
-
 for (i = 0; i  2; i++) {
 s-streams[i].nr = i;
 s-streams[i].bh = qemu_bh_new(timer_hit, s-streams[i]);
 s-streams[i].ptimer = ptimer_init(s-streams[i].bh);
 ptimer_set_freq(s-streams[i].ptimer, s-freqhz);
 }
-return 0;
 }
 
-static void xilinx_axidma_initfn(Object *obj)
+static void xilinx_axidma_init(Object *obj)
 {
 XilinxAXIDMA *s = XILINX_AXI_DMA(obj);
+SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 
 object_property_add_link(obj, axistream-connected, TYPE_STREAM_SLAVE,
  (Object **) s-tx_dev, NULL);
+
+sysbus_init_irq(sbd, s-streams[0].irq);
+sysbus_init_irq(sbd, s-streams[1].irq);
+
+memory_region_init_io(s-iomem, axidma_ops, s,
+  xlnx.axi-dma, R_MAX * 4 * 2);
+sysbus_init_mmio(sbd, s-iomem);
 }
 
 static Property axidma_properties[] = {
@@ -512,10 +512,9 @@ static Property axidma_properties[] = {
 static void axidma_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
-SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 StreamSlaveClass *ssc = STREAM_SLAVE_CLASS(klass);
 
-k-init = xilinx_axidma_init;
+dc-realize = xilinx_axidma_realize,
 dc-reset = xilinx_axidma_reset;
 dc-props = axidma_properties;
 ssc-push = axidma_push;
@@ -526,7 +525,7 @@ static const TypeInfo axidma_info = {
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(XilinxAXIDMA),
 .class_init= axidma_class_init,
-.instance_init = xilinx_axidma_initfn,
+.instance_init = xilinx_axidma_init,
 .interfaces = (InterfaceInfo[]) {
 { TYPE_STREAM_SLAVE },
 { }
-- 
1.7.0.4




[Qemu-devel] [PATCH v5 09/16] petalogix_ml605_mmu: Fix machine node attachment

2013-04-02 Thread Peter Crosthwaite
Just attach devices straight to the root machine node, rather than the
unattached node

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Reviewed-by: Andreas Färber afaer...@suse.de
Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com
---
Suggested (indirectly) by Andreas if he wants to put his Suggested-by to it.

 hw/microblaze/petalogix_ml605_mmu.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/microblaze/petalogix_ml605_mmu.c 
b/hw/microblaze/petalogix_ml605_mmu.c
index cfc0220..ca918b9 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -134,8 +134,8 @@ petalogix_ml605_init(QEMUMachineInitArgs *args)
 dma = qdev_create(NULL, xlnx.axi-dma);
 
 /* FIXME: attach to the sysbus instead */
-object_property_add_child(container_get(qdev_get_machine(), /unattached),
-  xilinx-dma, OBJECT(dma), NULL);
+object_property_add_child(qdev_get_machine(), xilinx-dma, OBJECT(dma),
+  NULL);
 
 xilinx_axiethernet_init(eth0, nd_table[0], STREAM_SLAVE(dma),
0x8278, irq[3], 0x1000, 0x1000);
-- 
1.7.0.4




[Qemu-devel] [PATCH v5 10/16] petalogix_ml605_mmu: Attach ethernet to machine

2013-04-02 Thread Peter Crosthwaite
Explicitly make the ethernet a child of the machine. This is needed to set
and use links pre-realize. Also makes the ethernet initialization consistent
with its peer DMA.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Reviewed-by: Andreas Färber afaer...@suse.de
Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com
---

 hw/microblaze/petalogix_ml605_mmu.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/microblaze/petalogix_ml605_mmu.c 
b/hw/microblaze/petalogix_ml605_mmu.c
index ca918b9..482c21f 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -134,6 +134,8 @@ petalogix_ml605_init(QEMUMachineInitArgs *args)
 dma = qdev_create(NULL, xlnx.axi-dma);
 
 /* FIXME: attach to the sysbus instead */
+object_property_add_child(qdev_get_machine(), xilinx-eth, OBJECT(eth0),
+  NULL);
 object_property_add_child(qdev_get_machine(), xilinx-dma, OBJECT(dma),
   NULL);
 
-- 
1.7.0.4




[Qemu-devel] [PATCH v5 11/16] xilinx_axienet: Create Proxy object for stream

2013-04-02 Thread Peter Crosthwaite
Create a separate child object to proxy the stream slave connection. This is
setup for future work where a second stream slave connection is needed. The
new child object is created at qdev init time and is linked back to the parent
(the ethernet device itself) automatically.

Stream slave masters differentiate which slave connection they are connected to
by linking to the proxy object rather than the parent.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---
changed from v3:
Fixed function name s/axienet_data_stream_push/xilinx_axienet_data_stream_push
changed from v2:
got rid or overly defensive assert
change from v1:
renamed data-stream proxy link to axistream-connected-target
rebased ontop of realize conversion
reworked error return mechanism (Andreas review)
inlined child device state struct into parents (Andreas review)
replaced object_new() - object_initialize() (Andreas review)

 hw/microblaze/petalogix_ml605_mmu.c |5 ++-
 hw/xilinx_axienet.c |   60 --
 2 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/hw/microblaze/petalogix_ml605_mmu.c 
b/hw/microblaze/petalogix_ml605_mmu.c
index 482c21f..9240660 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -79,6 +79,7 @@ petalogix_ml605_init(QEMUMachineInitArgs *args)
 const char *cpu_model = args-cpu_model;
 MemoryRegion *address_space_mem = get_system_memory();
 DeviceState *dev, *dma, *eth0;
+Object *peer;
 MicroBlazeCPU *cpu;
 SysBusDevice *busdev;
 CPUMBState *env;
@@ -142,7 +143,9 @@ petalogix_ml605_init(QEMUMachineInitArgs *args)
 xilinx_axiethernet_init(eth0, nd_table[0], STREAM_SLAVE(dma),
0x8278, irq[3], 0x1000, 0x1000);
 
-xilinx_axidma_init(dma, STREAM_SLAVE(eth0), 0x8460, irq[1], irq[0],
+peer = object_property_get_link(OBJECT(eth0),
+axistream-connected-target, NULL);
+xilinx_axidma_init(dma, STREAM_SLAVE(peer), 0x8460, irq[1], irq[0],
100 * 100);
 
 {
diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c
index ec1e893..141d2ac 100644
--- a/hw/xilinx_axienet.c
+++ b/hw/xilinx_axienet.c
@@ -33,10 +33,15 @@
 #define DPHY(x)
 
 #define TYPE_XILINX_AXI_ENET xlnx.axi-ethernet
+#define TYPE_XILINX_AXI_ENET_DATA_STREAM xilinx-axienet-data-stream
 
 #define XILINX_AXI_ENET(obj) \
  OBJECT_CHECK(XilinxAXIEnet, (obj), TYPE_XILINX_AXI_ENET)
 
+#define XILINX_AXI_ENET_DATA_STREAM(obj) \
+ OBJECT_CHECK(XilinxAXIEnetStreamSlave, (obj),\
+ TYPE_XILINX_AXI_ENET_DATA_STREAM)
+
 /* Advertisement control register. */
 #define ADVERTISE_10HALF0x0020  /* Try for 10mbps half-duplex  */
 #define ADVERTISE_10FULL0x0040  /* Try for 10mbps full-duplex  */
@@ -311,13 +316,21 @@ struct TEMAC  {
 void *parent;
 };
 
+typedef struct XilinxAXIEnetStreamSlave XilinxAXIEnetStreamSlave;
 typedef struct XilinxAXIEnet XilinxAXIEnet;
 
+struct XilinxAXIEnetStreamSlave {
+Object parent;
+
+struct XilinxAXIEnet *enet;
+} ;
+
 struct XilinxAXIEnet {
 SysBusDevice busdev;
 MemoryRegion iomem;
 qemu_irq irq;
 StreamSlave *tx_dev;
+XilinxAXIEnetStreamSlave rx_data_dev;
 NICState *nic;
 NICConf conf;
 
@@ -801,9 +814,11 @@ static void eth_cleanup(NetClientState *nc)
 }
 
 static void
-axienet_stream_push(StreamSlave *obj, uint8_t *buf, size_t size, uint32_t *hdr)
+xilinx_axienet_data_stream_push(StreamSlave *obj, uint8_t *buf, size_t size,
+uint32_t *hdr)
 {
-XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj));
+XilinxAXIEnetStreamSlave *ds = XILINX_AXI_ENET_DATA_STREAM(obj);
+XilinxAXIEnet *s = ds-enet;
 
 /* TX enable ?  */
 if (!(s-tc  TC_TX)) {
@@ -854,6 +869,18 @@ static NetClientInfo net_xilinx_enet_info = {
 static void xilinx_enet_realize(DeviceState *dev, Error **errp)
 {
 XilinxAXIEnet *s = XILINX_AXI_ENET(dev);
+XilinxAXIEnetStreamSlave *ds = 
XILINX_AXI_ENET_DATA_STREAM(s-rx_data_dev);
+Error *local_errp = NULL;
+
+object_property_add_link(OBJECT(ds), enet, xlnx.axi-ethernet,
+ (Object **) ds-enet, local_errp);
+if (local_errp) {
+goto xilinx_enet_realize_fail;
+}
+object_property_set_link(OBJECT(ds), OBJECT(s), enet, local_errp);
+if (local_errp) {
+goto xilinx_enet_realize_fail;
+}
 
 qemu_macaddr_default_if_unset(s-conf.macaddr);
 s-nic = qemu_new_nic(net_xilinx_enet_info, s-conf,
@@ -866,6 +893,12 @@ static void xilinx_enet_realize(DeviceState *dev, Error 
**errp)
 s-TEMAC.parent = s;
 
 s-rxmem = g_malloc(s-c_rxmem);
+return;
+
+xilinx_enet_realize_fail:
+if (!*errp) {
+*errp = local_errp;
+}
 }
 
 static void xilinx_enet_init(Object *obj)
@@ -878,6 +911,11 @@ static void xilinx_enet_init(Object *obj)
  (Object **) 

Re: [Qemu-devel] [PATCH V11 05/17] block: add snapshot info query function bdrv_query_snapshot_info_list()

2013-04-02 Thread Wenchao Xia

于 2013-4-3 9:17, Eric Blake 写道:

On 04/02/2013 05:47 AM, Wenchao Xia wrote:

   This patch adds function bdrv_query_snapshot_info_list(), which will
retrieve snapshot info of an image in qmp object format. The implementation
is based on the code moved from qemu-img.c with modification to fit more
for qmp based block layer API.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
  block/qapi.c |   55 ++---
  include/block/qapi.h |4 ++-
  qemu-img.c   |5 +++-
  3 files changed, 49 insertions(+), 15 deletions(-)



+/*
+ * return 0 on success, @p_list will be set only on success, and caller need to


s/need/needs/


+ * check *p_list on success.


I wonder if this wording would be any better:

Returns 0 on success, with *p_list either set to describe snapshot
information, or NULL because there are no snapshots.  Returns -1 on
error, with *p_list untouched.


+ */
+int bdrv_query_snapshot_info_list(BlockDriverState *bs,
+  SnapshotInfoList **p_list,
+  Error **errp)
  {


At any rate, my only commentary was on grammar and a possible wording
for a comment, while the code itself is fine from my viewpoint; so feel
free to add:

Reviewed-by: Eric Blake ebl...@redhat.com


+++ b/qemu-img.c
@@ -1735,7 +1735,10 @@ static ImageInfoList *collect_image_info_list(const char 
*filename,

  info = g_new0(ImageInfo, 1);
  bdrv_collect_image_info(bs, info, filename);
-bdrv_collect_snapshots(bs, info);
+if (!bdrv_query_snapshot_info_list(bs, info-snapshots, NULL) 
+info-snapshots) {
+info-has_snapshots = true;
+}


Hmm.  info-snapshots starts life as NULL (thanks to g_new0), and is
untouched on error.  Since you are ignoring any errors, you technically
could write:

bdrv_query_snapshot_info_list(bs, info-snapshots, NULL);
if (info-snapshots) {
 info-has_snapshots = true;
}

for the same semantics.  That means that as of this commit, no caller
cares about the return value of bdrv_query_snapshot_info_list (they only
care about whether info-snapshots was changed to non-null), so it could
return void for a slightly simpler implementation.

But I don't know if any later patches in the series start to care about
which error was returned.


  Yes it would be cared in ImageInfo retrieving later. This section in
qemu-img.c would be replaced later so did not check strictly the
returned value.


--
Best Regards

Wenchao Xia




[Qemu-devel] [PATCH v5 12/16] xilinx_axidma: Create Proxy object for stream

2013-04-02 Thread Peter Crosthwaite
Create a separate child object to proxy the stream slave connection. This is
setup for future work where a second stream slave connection is needed. The
new child object is created at qdev init time and is linked back to the parent
(the ethernet device itself) automatically.

Stream slave masters differentiate which slave connection they are connected to
by linking to the proxy object rather than the parent.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---
changed since v3:
Rebased to occur before flow control patches

 hw/microblaze/petalogix_ml605_mmu.c |6 ++-
 hw/xilinx_axidma.c  |   63 ---
 2 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/hw/microblaze/petalogix_ml605_mmu.c 
b/hw/microblaze/petalogix_ml605_mmu.c
index 9240660..7581275 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -140,8 +140,10 @@ petalogix_ml605_init(QEMUMachineInitArgs *args)
 object_property_add_child(qdev_get_machine(), xilinx-dma, OBJECT(dma),
   NULL);
 
-xilinx_axiethernet_init(eth0, nd_table[0], STREAM_SLAVE(dma),
-   0x8278, irq[3], 0x1000, 0x1000);
+peer = object_property_get_link(OBJECT(dma),
+axistream-connected-target, NULL);
+xilinx_axiethernet_init(eth0, nd_table[0], STREAM_SLAVE(peer),
+0x8278, irq[3], 0x1000, 0x1000);
 
 peer = object_property_get_link(OBJECT(eth0),
 axistream-connected-target, NULL);
diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c
index 2c95765..02700ea 100644
--- a/hw/xilinx_axidma.c
+++ b/hw/xilinx_axidma.c
@@ -34,10 +34,15 @@
 #define D(x)
 
 #define TYPE_XILINX_AXI_DMA xlnx.axi-dma
+#define TYPE_XILINX_AXI_DMA_DATA_STREAM xilinx-axi-dma-data-stream
 
 #define XILINX_AXI_DMA(obj) \
  OBJECT_CHECK(XilinxAXIDMA, (obj), TYPE_XILINX_AXI_DMA)
 
+#define XILINX_AXI_DMA_DATA_STREAM(obj) \
+ OBJECT_CHECK(XilinxAXIDMAStreamSlave, (obj),\
+ TYPE_XILINX_AXI_DMA_DATA_STREAM)
+
 #define R_DMACR (0x00 / 4)
 #define R_DMASR (0x04 / 4)
 #define R_CURDESC   (0x08 / 4)
@@ -45,6 +50,7 @@
 #define R_MAX   (0x30 / 4)
 
 typedef struct XilinxAXIDMA XilinxAXIDMA;
+typedef struct XilinxAXIDMAStreamSlave XilinxAXIDMAStreamSlave;
 
 enum {
 DMACR_RUNSTOP = 1,
@@ -97,11 +103,18 @@ struct Stream {
 uint32_t regs[R_MAX];
 };
 
+struct XilinxAXIDMAStreamSlave {
+Object parent;
+
+struct XilinxAXIDMA *dma;
+};
+
 struct XilinxAXIDMA {
 SysBusDevice busdev;
 MemoryRegion iomem;
 uint32_t freqhz;
 StreamSlave *tx_dev;
+XilinxAXIDMAStreamSlave rx_data_dev;
 
 struct Stream streams[2];
 };
@@ -369,10 +382,11 @@ static void xilinx_axidma_reset(DeviceState *dev)
 }
 
 static void
-axidma_push(StreamSlave *obj, unsigned char *buf, size_t len, uint32_t *app)
+xilinx_axidma_data_stream_push(StreamSlave *obj, unsigned char *buf, size_t 
len,
+   uint32_t *app)
 {
-XilinxAXIDMA *d = XILINX_AXI_DMA(obj);
-struct Stream *s = d-streams[1];
+XilinxAXIDMAStreamSlave *ds = XILINX_AXI_DMA_DATA_STREAM(obj);
+struct Stream *s = ds-dma-streams[1];
 
 if (!app) {
 hw_error(No stream app data!\n);
@@ -478,6 +492,19 @@ static const MemoryRegionOps axidma_ops = {
 static void xilinx_axidma_realize(DeviceState *dev, Error **errp)
 {
 XilinxAXIDMA *s = XILINX_AXI_DMA(dev);
+XilinxAXIDMAStreamSlave *ds = XILINX_AXI_DMA_DATA_STREAM(s-rx_data_dev);
+Error *local_errp = NULL;
+
+object_property_add_link(OBJECT(ds), dma, TYPE_XILINX_AXI_DMA,
+ (Object **)ds-dma, local_errp);
+if (local_errp) {
+goto xilinx_axidma_realize_fail;
+}
+object_property_set_link(OBJECT(ds), OBJECT(s), dma, local_errp);
+if (local_errp) {
+goto xilinx_axidma_realize_fail;
+}
+
 int i;
 
 for (i = 0; i  2; i++) {
@@ -486,16 +513,28 @@ static void xilinx_axidma_realize(DeviceState *dev, Error 
**errp)
 s-streams[i].ptimer = ptimer_init(s-streams[i].bh);
 ptimer_set_freq(s-streams[i].ptimer, s-freqhz);
 }
+return;
+
+xilinx_axidma_realize_fail:
+if (!*errp) {
+*errp = local_errp;
+}
 }
 
 static void xilinx_axidma_init(Object *obj)
 {
 XilinxAXIDMA *s = XILINX_AXI_DMA(obj);
 SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+Error *errp = NULL;
 
 object_property_add_link(obj, axistream-connected, TYPE_STREAM_SLAVE,
  (Object **) s-tx_dev, NULL);
 
+object_initialize(s-rx_data_dev, TYPE_XILINX_AXI_DMA_DATA_STREAM);
+object_property_add_child(OBJECT(s), axistream-connected-target,
+  (Object *)s-rx_data_dev, errp);
+assert_no_error(errp);
+
 sysbus_init_irq(sbd, s-streams[0].irq);
 sysbus_init_irq(sbd, 

[Qemu-devel] [PATCH v5 14/16] stream: Add flow control API

2013-04-02 Thread Peter Crosthwaite
Add basic flow control to stream. A stream slave may return short, indicating
that it is not capable of accepting any more data at the present time. Polling
or a callback can be used via the can_push() function to determine when the
slave can receive again.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---

 hw/stream.c |   13 +++--
 hw/stream.h |   33 ++---
 hw/xilinx_axidma.c  |3 ++-
 hw/xilinx_axienet.c |8 +---
 4 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/hw/stream.c b/hw/stream.c
index a07d6a5..5397a8d 100644
--- a/hw/stream.c
+++ b/hw/stream.c
@@ -1,11 +1,20 @@
 #include hw/stream.h
 
-void
+size_t
 stream_push(StreamSlave *sink, uint8_t *buf, size_t len, uint32_t *app)
 {
 StreamSlaveClass *k =  STREAM_SLAVE_GET_CLASS(sink);
 
-k-push(sink, buf, len, app);
+return k-push(sink, buf, len, app);
+}
+
+bool
+stream_can_push(StreamSlave *sink, StreamCanPushNotifyFn notify,
+void *notify_opaque)
+{
+StreamSlaveClass *k =  STREAM_SLAVE_GET_CLASS(sink);
+
+return k-can_push ? k-can_push(sink, notify, notify_opaque) : true;
 }
 
 static const TypeInfo stream_slave_info = {
diff --git a/hw/stream.h b/hw/stream.h
index f6137d6..ff2cb14 100644
--- a/hw/stream.h
+++ b/hw/stream.h
@@ -18,14 +18,41 @@ typedef struct StreamSlave {
 Object Parent;
 } StreamSlave;
 
+typedef void (*StreamCanPushNotifyFn)(void *opaque);
+
 typedef struct StreamSlaveClass {
 InterfaceClass parent;
-
-void (*push)(StreamSlave *obj, unsigned char *buf, size_t len,
+/**
+ * can push - determine if a stream slave is capable of accepting at least
+ * one byte of data. Returns false if cannot accept. If not implemented, 
the
+ * slave is assumed to always be capable of recieveing.
+ * @notify: Optional callback that the slave will call when the slave is
+ * capable of recieving again. Only called if false is returned.
+ * @notify_opaque: opaque data to pass to notify call.
+ */
+bool (*can_push)(StreamSlave *obj, StreamCanPushNotifyFn notify,
+ void *notify_opaque);
+/**
+ * push - push data to a Stream slave. The number of bytes pushed is
+ * returned. If the slave short returns, the master must wait before trying
+ * again, the slave may continue to just return 0 waiting for the vm time 
to
+ * advance. The can_push() function can be used to trap the point in time
+ * where the slave is ready to recieve agai, otherwise polling on a QEMU
+ * timer will work.
+ * @obj: Stream slave to push to
+ * @buf: Data to write
+ * @len: Maximum number of bytes to write
+ */
+size_t (*push)(StreamSlave *obj, unsigned char *buf, size_t len,
 uint32_t *app);
 } StreamSlaveClass;
 
-void
+size_t
 stream_push(StreamSlave *sink, uint8_t *buf, size_t len, uint32_t *app);
 
+bool
+stream_can_push(StreamSlave *sink, StreamCanPushNotifyFn notify,
+void *notify_opaque);
+
+
 #endif /* STREAM_H */
diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c
index 2bbfea1..80ce57f 100644
--- a/hw/xilinx_axidma.c
+++ b/hw/xilinx_axidma.c
@@ -381,7 +381,7 @@ static void xilinx_axidma_reset(DeviceState *dev)
 }
 }
 
-static void
+static size_t
 xilinx_axidma_data_stream_push(StreamSlave *obj, unsigned char *buf, size_t 
len,
uint32_t *app)
 {
@@ -393,6 +393,7 @@ xilinx_axidma_data_stream_push(StreamSlave *obj, unsigned 
char *buf, size_t len,
 }
 stream_process_s2mem(s, buf, len, app);
 stream_update_irq(s);
+return len;
 }
 
 static uint64_t axidma_read(void *opaque, hwaddr addr,
diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c
index 141d2ac..5c0ea59 100644
--- a/hw/xilinx_axienet.c
+++ b/hw/xilinx_axienet.c
@@ -813,7 +813,7 @@ static void eth_cleanup(NetClientState *nc)
 g_free(s);
 }
 
-static void
+static size_t
 xilinx_axienet_data_stream_push(StreamSlave *obj, uint8_t *buf, size_t size,
 uint32_t *hdr)
 {
@@ -822,13 +822,13 @@ xilinx_axienet_data_stream_push(StreamSlave *obj, uint8_t 
*buf, size_t size,
 
 /* TX enable ?  */
 if (!(s-tc  TC_TX)) {
-return;
+return size;
 }
 
 /* Jumbo or vlan sizes ?  */
 if (!(s-tc  TC_JUM)) {
 if (size  1518  size = 1522  !(s-tc  TC_VLAN)) {
-return;
+return size;
 }
 }
 
@@ -856,6 +856,8 @@ xilinx_axienet_data_stream_push(StreamSlave *obj, uint8_t 
*buf, size_t size,
 s-stats.tx_bytes += size;
 s-regs[R_IS] |= IS_TX_COMPLETE;
 enet_update_irq(s);
+
+return size;
 }
 
 static NetClientInfo net_xilinx_enet_info = {
-- 
1.7.0.4




[Qemu-devel] [PATCH v5 13/16] xilinx_axidma: Fix rx/tx halted bit.

2013-04-02 Thread Peter Crosthwaite
If there is no DMA buffer descriptor, the DMA halts, not idles.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com
---
changed from v3:
Fixed for TX path as well as RX

 hw/xilinx_axidma.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c
index 02700ea..2bbfea1 100644
--- a/hw/xilinx_axidma.c
+++ b/hw/xilinx_axidma.c
@@ -276,7 +276,7 @@ static void stream_process_mem2s(struct Stream *s,
 stream_desc_load(s, s-regs[R_CURDESC]);
 
 if (s-desc.status  SDESC_STATUS_COMPLETE) {
-s-regs[R_DMASR] |= DMASR_IDLE;
+s-regs[R_DMASR] |= DMASR_HALTED;
 break;
 }
 
@@ -331,7 +331,7 @@ static void stream_process_s2mem(struct Stream *s,
 stream_desc_load(s, s-regs[R_CURDESC]);
 
 if (s-desc.status  SDESC_STATUS_COMPLETE) {
-s-regs[R_DMASR] |= DMASR_IDLE;
+s-regs[R_DMASR] |= DMASR_HALTED;
 break;
 }
 
-- 
1.7.0.4




[Qemu-devel] [PATCH v5 15/16] xilinx_axienet/dma: Implement rx path flow control

2013-04-02 Thread Peter Crosthwaite
Implement flow control for the RX data path from xilinx_axienet-xilinx_axidma.
On short return from axidma, then ethernet sets up the notify callback to resume
transfer from where it left off.

This also allows the ethernet to track whether there is an in progress 
transaction
and return false from ethernet can_receive() as appropriate.

If the DMA backs up or is disabled it waits for enablement. When the rx stream 
IO
region is touched, the can_push() notify function is called if set.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---

 hw/xilinx_axidma.c  |   49 +
 hw/xilinx_axienet.c |   28 +---
 2 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c
index 80ce57f..a5bf102 100644
--- a/hw/xilinx_axidma.c
+++ b/hw/xilinx_axidma.c
@@ -117,6 +117,9 @@ struct XilinxAXIDMA {
 XilinxAXIDMAStreamSlave rx_data_dev;
 
 struct Stream streams[2];
+
+StreamCanPushNotifyFn notify;
+void *notify_opaque;
 };
 
 /*
@@ -315,16 +318,16 @@ static void stream_process_mem2s(struct Stream *s,
 }
 }
 
-static void stream_process_s2mem(struct Stream *s,
- unsigned char *buf, size_t len, uint32_t *app)
+static size_t stream_process_s2mem(struct Stream *s, unsigned char *buf,
+   size_t len, uint32_t *app)
 {
 uint32_t prev_d;
 unsigned int rxlen;
-int pos = 0;
+size_t pos = 0;
 int sof = 1;
 
 if (!stream_running(s) || stream_idle(s)) {
-return;
+return 0;
 }
 
 while (len) {
@@ -369,6 +372,8 @@ static void stream_process_s2mem(struct Stream *s,
 break;
 }
 }
+
+return pos;
 }
 
 static void xilinx_axidma_reset(DeviceState *dev)
@@ -381,19 +386,37 @@ static void xilinx_axidma_reset(DeviceState *dev)
 }
 }
 
+static bool
+xilinx_axidma_data_stream_can_push(StreamSlave *obj,
+   StreamCanPushNotifyFn notify,
+   void *notify_opaque)
+{
+XilinxAXIDMAStreamSlave *ds = XILINX_AXI_DMA_DATA_STREAM(obj);
+struct Stream *s = ds-dma-streams[1];
+
+if (!stream_running(s) || stream_idle(s)) {
+ds-dma-notify = notify;
+ds-dma-notify_opaque = notify_opaque;
+return false;
+}
+
+return true;
+}
+
 static size_t
 xilinx_axidma_data_stream_push(StreamSlave *obj, unsigned char *buf, size_t 
len,
uint32_t *app)
 {
 XilinxAXIDMAStreamSlave *ds = XILINX_AXI_DMA_DATA_STREAM(obj);
 struct Stream *s = ds-dma-streams[1];
+size_t ret;
 
 if (!app) {
 hw_error(No stream app data!\n);
 }
-stream_process_s2mem(s, buf, len, app);
+ret = stream_process_s2mem(s, buf, len, app);
 stream_update_irq(s);
-return len;
+return ret;
 }
 
 static uint64_t axidma_read(void *opaque, hwaddr addr,
@@ -481,6 +504,10 @@ static void axidma_write(void *opaque, hwaddr addr,
 s-regs[addr] = value;
 break;
 }
+if (sid == 1  d-notify) {
+d-notify(d-notify_opaque);
+d-notify = NULL;
+}
 stream_update_irq(s);
 }
 
@@ -558,11 +585,17 @@ static void axidma_class_init(ObjectClass *klass, void 
*data)
 dc-props = axidma_properties;
 }
 
+static StreamSlaveClass xilinx_axidma_data_stream_class = {
+.push = xilinx_axidma_data_stream_push,
+.can_push = xilinx_axidma_data_stream_can_push,
+};
+
 static void xilinx_axidma_stream_class_init(ObjectClass *klass, void *data)
 {
 StreamSlaveClass *ssc = STREAM_SLAVE_CLASS(klass);
 
-ssc-push = data;
+ssc-push = ((StreamSlaveClass *)data)-push;
+ssc-can_push = ((StreamSlaveClass *)data)-can_push;
 }
 
 static const TypeInfo axidma_info = {
@@ -578,7 +611,7 @@ static const TypeInfo xilinx_axidma_data_stream_info = {
 .parent= TYPE_OBJECT,
 .instance_size = sizeof(struct XilinxAXIDMAStreamSlave),
 .class_init= xilinx_axidma_stream_class_init,
-.class_data= xilinx_axidma_data_stream_push,
+.class_data= xilinx_axidma_data_stream_class,
 .interfaces = (InterfaceInfo[]) {
 { TYPE_STREAM_SLAVE },
 { }
diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c
index 5c0ea59..e67a68b 100644
--- a/hw/xilinx_axienet.c
+++ b/hw/xilinx_axienet.c
@@ -383,6 +383,9 @@ struct XilinxAXIEnet {
 
 
 uint8_t *rxmem;
+uint32_t *rxapp;
+uint32_t rxsize;
+uint32_t rxpos;
 };
 
 static void axienet_rx_reset(XilinxAXIEnet *s)
@@ -643,7 +646,7 @@ static int eth_can_rx(NetClientState *nc)
 XilinxAXIEnet *s = qemu_get_nic_opaque(nc);
 
 /* RX enabled?  */
-return !axienet_rx_resetting(s)  axienet_rx_enabled(s);
+return !s-rxsize  !axienet_rx_resetting(s)  axienet_rx_enabled(s);
 }
 
 static int enet_match_addr(const uint8_t *buf, uint32_t f0, uint32_t f1)
@@ -661,6 +664,23 @@ static int enet_match_addr(const uint8_t *buf, 

[Qemu-devel] [PATCH v5 16/16] stream: Remove app argument hack

2013-04-02 Thread Peter Crosthwaite
The uint32_t *app argument doesn't exist in real hardware. It was a hack in
xilinx_axidma/enet to fake the (secondary) control stream connection. Removed
the argument and added the second stream to axienet/dma.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---
changed since v3:
Rebased against now synchronous stream control API
Macrofied stream control packet lengths

 hw/microblaze/petalogix_ml605_mmu.c |   25 +
 hw/stream.c |4 +-
 hw/stream.h |5 +-
 hw/xilinx.h |   21 +---
 hw/xilinx_axidma.c  |   99 --
 hw/xilinx_axienet.c |  100 +-
 6 files changed, 187 insertions(+), 67 deletions(-)

diff --git a/hw/microblaze/petalogix_ml605_mmu.c 
b/hw/microblaze/petalogix_ml605_mmu.c
index 7581275..341cd2e 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -79,7 +79,7 @@ petalogix_ml605_init(QEMUMachineInitArgs *args)
 const char *cpu_model = args-cpu_model;
 MemoryRegion *address_space_mem = get_system_memory();
 DeviceState *dev, *dma, *eth0;
-Object *peer;
+Object *ds, *cs;
 MicroBlazeCPU *cpu;
 SysBusDevice *busdev;
 CPUMBState *env;
@@ -140,15 +140,20 @@ petalogix_ml605_init(QEMUMachineInitArgs *args)
 object_property_add_child(qdev_get_machine(), xilinx-dma, OBJECT(dma),
   NULL);
 
-peer = object_property_get_link(OBJECT(dma),
-axistream-connected-target, NULL);
-xilinx_axiethernet_init(eth0, nd_table[0], STREAM_SLAVE(peer),
-0x8278, irq[3], 0x1000, 0x1000);
-
-peer = object_property_get_link(OBJECT(eth0),
-axistream-connected-target, NULL);
-xilinx_axidma_init(dma, STREAM_SLAVE(peer), 0x8460, irq[1], irq[0],
-   100 * 100);
+ds = object_property_get_link(OBJECT(dma),
+  axistream-connected-target, NULL);
+cs = object_property_get_link(OBJECT(dma),
+  axistream-control-connected-target, NULL);
+xilinx_axiethernet_init(eth0, nd_table[0], STREAM_SLAVE(ds),
+STREAM_SLAVE(cs), 0x8278, irq[3], 0x1000,
+0x1000);
+
+ds = object_property_get_link(OBJECT(eth0),
+  axistream-connected-target, NULL);
+cs = object_property_get_link(OBJECT(eth0),
+  axistream-control-connected-target, NULL);
+xilinx_axidma_init(dma, STREAM_SLAVE(ds), STREAM_SLAVE(cs), 0x8460,
+   irq[1], irq[0], 100 * 100);
 
 {
 SSIBus *spi;
diff --git a/hw/stream.c b/hw/stream.c
index 5397a8d..e6a05a5 100644
--- a/hw/stream.c
+++ b/hw/stream.c
@@ -1,11 +1,11 @@
 #include hw/stream.h
 
 size_t
-stream_push(StreamSlave *sink, uint8_t *buf, size_t len, uint32_t *app)
+stream_push(StreamSlave *sink, uint8_t *buf, size_t len)
 {
 StreamSlaveClass *k =  STREAM_SLAVE_GET_CLASS(sink);
 
-return k-push(sink, buf, len, app);
+return k-push(sink, buf, len);
 }
 
 bool
diff --git a/hw/stream.h b/hw/stream.h
index ff2cb14..36ae35d 100644
--- a/hw/stream.h
+++ b/hw/stream.h
@@ -43,12 +43,11 @@ typedef struct StreamSlaveClass {
  * @buf: Data to write
  * @len: Maximum number of bytes to write
  */
-size_t (*push)(StreamSlave *obj, unsigned char *buf, size_t len,
-uint32_t *app);
+size_t (*push)(StreamSlave *obj, unsigned char *buf, size_t len);
 } StreamSlaveClass;
 
 size_t
-stream_push(StreamSlave *sink, uint8_t *buf, size_t len, uint32_t *app);
+stream_push(StreamSlave *sink, uint8_t *buf, size_t len);
 
 bool
 stream_can_push(StreamSlave *sink, StreamCanPushNotifyFn notify,
diff --git a/hw/xilinx.h b/hw/xilinx.h
index 6c1ee21..0c0251a 100644
--- a/hw/xilinx.h
+++ b/hw/xilinx.h
@@ -55,16 +55,19 @@ xilinx_ethlite_create(NICInfo *nd, hwaddr base, qemu_irq 
irq,
 }
 
 static inline void
-xilinx_axiethernet_init(DeviceState *dev, NICInfo *nd, StreamSlave *peer,
-hwaddr base, qemu_irq irq, int txmem, int rxmem)
+xilinx_axiethernet_init(DeviceState *dev, NICInfo *nd, StreamSlave *ds,
+StreamSlave *cs, hwaddr base, qemu_irq irq, int txmem,
+int rxmem)
 {
 Error *errp = NULL;
 
 qdev_set_nic_properties(dev, nd);
 qdev_prop_set_uint32(dev, rxmem, rxmem);
 qdev_prop_set_uint32(dev, txmem, txmem);
-object_property_set_link(OBJECT(dev), OBJECT(peer), axistream-connected,
- errp);
+object_property_set_link(OBJECT(dev), OBJECT(ds),
+ axistream-connected, errp);
+object_property_set_link(OBJECT(dev), OBJECT(cs),
+ 

[Qemu-devel] [PATCH] hw/nand.c: Fix nand erase operation

2013-04-02 Thread Peter Crosthwaite
From: Wendy Liang wendy.li...@xilinx.com

Usually, nand erase operation has only 2 or 3 address cycles.
We need to mask s-addr to zero unset stale high-order bytes in the nand address
before using it as the erase address.

This fixes the NAND erase operation in Linux.

[PC: Generalised to work for any number of address cycles rather than just 3]

Signed-off-by: Wendy Liang jli...@xilinx.com
Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---
 hw/nand.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/nand.c b/hw/nand.c
index de3e502..6362093 100644
--- a/hw/nand.c
+++ b/hw/nand.c
@@ -297,6 +297,7 @@ static void nand_command(NANDFlashState *s)
 break;
 
 case NAND_CMD_BLOCKERASE2:
+s-addr = (1ull  s-addrlen * 8) - 1;
 if (nand_flash_ids[s-chip_id].options  NAND_SAMSUNG_LP)
 s-addr = 16;
 else
-- 
1.7.0.4




Re: [Qemu-devel] [PATCH 3/3] block: change rollback sequence in qmp_transaction

2013-04-02 Thread Wenchao Xia

于 2013-4-2 21:59, Kevin Wolf 写道:

Am 01.04.2013 um 12:01 hat Wenchao Xia geschrieben:

   Last operaton should be cancelled first.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com


Should it? This commit message does little to convince me.

Kevin


  I think so, if two request are snapshot_blkdev ide0 newimage.qcow2,
snapshot_blkdev_internal ide0 snap0, then in rollback delete of
snap0 would be wrong.

--
Best Regards

Wenchao Xia




Re: [Qemu-devel] [PATCH 2/3] block: adjust qmp_transaction to be extendable

2013-04-02 Thread Wenchao Xia

于 2013-4-2 21:55, Kevin Wolf 写道:

Am 01.04.2013 um 12:01 hat Wenchao Xia geschrieben:

   Now code for external snapshot are packaged as one case
in qmp_transaction, so later other operation could be added.
   The logic in qmp_transaction is changed a bit: Original code
tries to create all images first and then update all *bdrv
once together, new code create and update one *bdrv one time,
and use bdrv_deappend() to rollback on fail. This allows mixing
different kind of requests in qmp_transaction() later.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
  blockdev.c |  250 +---
  1 files changed, 153 insertions(+), 97 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 8cdc9ce..75416fb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -779,9 +779,155 @@ void qmp_blockdev_snapshot_sync(const char *device, const 
char *snapshot_file,


  /* New and old BlockDriverState structs for group snapshots */
-typedef struct BlkTransactionStates {
+typedef struct BdrvActionOps {
+int (*commit)(BlockdevAction *action, void **p_opaque, Error **errp);
+void (*rollback)(BlockdevAction *action, void *opaque);
+void (*clean)(BlockdevAction *action, void *opaque);
+} BdrvActionOps;


You don't really implement the transactional pattern that was used by
the original code (and is used elsewhere). It should work like this:

1. Prepare: This stage performs all operations that can fail. They are
not made active yet. For example with snapshotting, open a new
BlockDriver state, but don't change the backing file chain yet.

2. Commit: Activate the change. This operation can never fail. For this
reason, you never have to undo anything done here.

3. Rollback: Basically just free everything that prepare did up to the
error.

If you do it your way, you get into serious trouble if rollback involves
operations that can fail.

In the original code, here start the prepare:


  That is a clear comment, thanks. I changed the model since expecting
commit operation may need rollback, if not I am confident to keep
original model.
  Since bdrv_snapshot_delete() can fail, I guess assertion of its
success should be used in the rollback later.


@@ -806,125 +950,37 @@ void qmp_transaction(BlockdevActionList *dev_list, Error 
**errp)
  /* We don't do anything in this loop that commits us to the snapshot */
  while (NULL != dev_entry) {
  BlockdevAction *dev_info = NULL;
-BlockDriver *proto_drv;
-BlockDriver *drv;
-int flags;
-enum NewImageMode mode;
-const char *new_image_file;
-const char *device;
-const char *format = qcow2;
-
  dev_info = dev_entry-value;
  dev_entry = dev_entry-next;

  states = g_malloc0(sizeof(BlkTransactionStates));
  QSIMPLEQ_INSERT_TAIL(snap_bdrv_states, states, entry);

+states-action = dev_info;
  switch (dev_info-kind) {
  case BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC:
-device = dev_info-blockdev_snapshot_sync-device;
-if (!dev_info-blockdev_snapshot_sync-has_mode) {
-dev_info-blockdev_snapshot_sync-mode = 
NEW_IMAGE_MODE_ABSOLUTE_PATHS;
-}
-new_image_file = dev_info-blockdev_snapshot_sync-snapshot_file;
-if (dev_info-blockdev_snapshot_sync-has_format) {
-format = dev_info-blockdev_snapshot_sync-format;
-}
-mode = dev_info-blockdev_snapshot_sync-mode;
+states-ops = external_snapshot_ops;
  break;
  default:
  abort();
  }

-drv = bdrv_find_format(format);
-if (!drv) {
-error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
-goto delete_and_fail;
-}
-
-states-old_bs = bdrv_find(device);
-if (!states-old_bs) {
-error_set(errp, QERR_DEVICE_NOT_FOUND, device);
-goto delete_and_fail;
-}
-
-if (!bdrv_is_inserted(states-old_bs)) {
-error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
-goto delete_and_fail;
-}
-
-if (bdrv_in_use(states-old_bs)) {
-error_set(errp, QERR_DEVICE_IN_USE, device);
-goto delete_and_fail;
-}
-
-if (!bdrv_is_read_only(states-old_bs)) {
-if (bdrv_flush(states-old_bs)) {
-error_set(errp, QERR_IO_ERROR);
-goto delete_and_fail;
-}
-}
-
-flags = states-old_bs-open_flags;
-
-proto_drv = bdrv_find_protocol(new_image_file);
-if (!proto_drv) {
-error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
-goto delete_and_fail;
-}
-
-/* create new image w/backing file */
-if (mode != NEW_IMAGE_MODE_EXISTING) {
-bdrv_img_create(new_image_file, format,
-states-old_bs-filename,
-

Re: [Qemu-devel] [PATCH 2/5] hostmem: make hostmem global and RAM hotunplg safe

2013-04-02 Thread li guang
在 2013-04-01一的 16:20 +0800,Liu Ping Fan写道:
 From: Liu Ping Fan pingf...@linux.vnet.ibm.com
 
 The hostmem listener will translate gpa to hva quickly. Make it
 global, so other component can use it.
 Also this patch adopt MemoryRegionOps's ref/unref interface to
 make it survive the RAM hotunplug.
 
 Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
 ---
  hw/dataplane/hostmem.c |  130 +--
  hw/dataplane/hostmem.h |   19 ++--
  2 files changed, 95 insertions(+), 54 deletions(-)
 
 diff --git a/hw/dataplane/hostmem.c b/hw/dataplane/hostmem.c
 index 380537e..86c02cd 100644
 --- a/hw/dataplane/hostmem.c
 +++ b/hw/dataplane/hostmem.c
 @@ -13,6 +13,12 @@
  
  #include exec/address-spaces.h
  #include hostmem.h
 +#include qemu/main-loop.h
 +
 +/* lock to protect the access to cur_hostmem */
 +static QemuMutex hostmem_lock;
 +static HostMem *cur_hostmem;
 +static HostMem *next_hostmem;
  
  static int hostmem_lookup_cmp(const void *phys_, const void *region_)
  {
 @@ -28,16 +34,49 @@ static int hostmem_lookup_cmp(const void *phys_, const 
 void *region_)
  }
  }
  
 +static void hostmem_ref(HostMem *hostmem)
 +{
 +assert(__sync_add_and_fetch(hostmem-ref, 1)  0);
 +}
 +
 +void hostmem_unref(HostMem *hostmem)
 +{
 +int i;
 +HostMemRegion *hmr;
 +
 +if (!__sync_sub_and_fetch(hostmem-ref, 1)) {

maybe '__sync_sub_and_fetch(hostmem-ref, 1)  0' is more safer

 +for (i = 0; i  hostmem-num_current_regions; i++) {
 +hmr = hostmem-current_regions[i];
 +/* Fix me, when memory hotunplug implement
 + * assert(hmr-mr_ops-unref)
 + */
 +if (hmr-mr-ops  hmr-mr-ops-unref) {
 +hmr-mr-ops-unref();
 +}
 +}
 +g_free(hostmem-current_regions);
 +g_free(hostmem);
 +}
 +}
 +
  /**
   * Map guest physical address to host pointer
 + * also inc refcnt of *mr, caller need to unref later
   */
 -void *hostmem_lookup(HostMem *hostmem, hwaddr phys, hwaddr len, bool 
 is_write)
 +void *hostmem_lookup(hwaddr phys, hwaddr len, MemoryRegion **mr, bool 
 is_write)
  {
  HostMemRegion *region;
  void *host_addr = NULL;
  hwaddr offset_within_region;
 +HostMem *hostmem;
 +
 +assert(mr);
 +*mr = NULL;
 +qemu_mutex_lock(hostmem_lock);
 +hostmem = cur_hostmem;
 +hostmem_ref(hostmem);
 +qemu_mutex_unlock(hostmem_lock);
  
 -qemu_mutex_lock(hostmem-current_regions_lock);
  region = bsearch(phys, hostmem-current_regions,
   hostmem-num_current_regions,
   sizeof(hostmem-current_regions[0]),
 @@ -52,28 +91,33 @@ void *hostmem_lookup(HostMem *hostmem, hwaddr phys, 
 hwaddr len, bool is_write)
  if (len = region-size - offset_within_region) {
  host_addr = region-host_addr + offset_within_region;
  }
 -out:
 -qemu_mutex_unlock(hostmem-current_regions_lock);
 +*mr = region-mr;
 +memory_region_ref(*mr);
  
 +out:
 +hostmem_unref(hostmem);
  return host_addr;
  }
  
 +static void hostmem_listener_begin(MemoryListener *listener)
 +{
 +next_hostmem = g_new0(HostMem, 1);
 +next_hostmem-ref = 1;
 +}
 +
  /**
   * Install new regions list
   */
  static void hostmem_listener_commit(MemoryListener *listener)
  {
 -HostMem *hostmem = container_of(listener, HostMem, listener);
 +HostMem *tmp;
  
 -qemu_mutex_lock(hostmem-current_regions_lock);
 -g_free(hostmem-current_regions);
 -hostmem-current_regions = hostmem-new_regions;
 -hostmem-num_current_regions = hostmem-num_new_regions;
 -qemu_mutex_unlock(hostmem-current_regions_lock);
 +tmp = cur_hostmem;
 +qemu_mutex_lock(hostmem_lock);
 +cur_hostmem = next_hostmem;
 +qemu_mutex_unlock(hostmem_lock);
 +hostmem_unref(tmp);
  
 -/* Reset new regions list */
 -hostmem-new_regions = NULL;
 -hostmem-num_new_regions = 0;
  }
  
  /**
 @@ -83,23 +127,24 @@ static void hostmem_append_new_region(HostMem *hostmem,
MemoryRegionSection *section)
  {
  void *ram_ptr = memory_region_get_ram_ptr(section-mr);
 -size_t num = hostmem-num_new_regions;
 -size_t new_size = (num + 1) * sizeof(hostmem-new_regions[0]);
 +size_t num = hostmem-num_current_regions;
 +size_t new_size = (num + 1) * sizeof(hostmem-current_regions[0]);
  
 -hostmem-new_regions = g_realloc(hostmem-new_regions, new_size);
 -hostmem-new_regions[num] = (HostMemRegion){
 +hostmem-current_regions = g_realloc(hostmem-current_regions, new_size);
 +hostmem-current_regions[num] = (HostMemRegion){
  .host_addr = ram_ptr + section-offset_within_region,
  .guest_addr = section-offset_within_address_space,
 +.mr = section-mr,
  .size = section-size,
  .readonly = section-readonly,
  };
 -hostmem-num_new_regions++;
 +hostmem-num_current_regions++;
  }
  
 -static void 

Re: [Qemu-devel] [2/2] [vmxnet3] const_cpu_to_le64 wrapping for feature bits dropped

2013-04-02 Thread Alexander Graf

On 28.03.2013, at 09:53, Dmitry Fleytman wrote:

 Byte swap is redundant because shared memory reading functions
 already swap bytes when required
 
 Signed-off-by: Dmitry Fleytman dmi...@daynix.com

Acked-by: Alexander Graf ag...@suse.de

It might be worth to run an x86 guest with vmxnet3 on a big endian host machine 
to find potential additional breakage. I don't think it's worth caring about 
big endian guests with vmxnet3 at this point. Chances are quite low the guest 
driver would be correct :).


Alex

 ---
 hw/vmxnet3.h | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)
 
 diff --git a/hw/vmxnet3.h b/hw/vmxnet3.h
 index 7db0c8f..4eae7c7 100644
 --- a/hw/vmxnet3.h
 +++ b/hw/vmxnet3.h
 @@ -37,10 +37,8 @@
 #define __packed QEMU_PACKED
 
 #if defined(HOST_WORDS_BIGENDIAN)
 -#define const_cpu_to_le64(x) bswap_64(x)
 #define __BIG_ENDIAN_BITFIELD
 #else
 -#define const_cpu_to_le64(x) (x)
 #endif
 
 /*
 @@ -137,10 +135,10 @@ struct UPT1_RSSConf {
 
 /* features */
 enum {
 -UPT1_F_RXCSUM= const_cpu_to_le64(0x0001), /* rx csum 
 verification */
 -UPT1_F_RSS= const_cpu_to_le64(0x0002),
 -UPT1_F_RXVLAN= const_cpu_to_le64(0x0004), /* VLAN tag stripping 
 */
 -UPT1_F_LRO= const_cpu_to_le64(0x0008),
 +UPT1_F_RXCSUM= 0x0001, /* rx csum verification */
 +UPT1_F_RSS   = 0x0002,
 +UPT1_F_RXVLAN= 0x0004, /* VLAN tag stripping */
 +UPT1_F_LRO   = 0x0008,
 };
 
 /* all registers are 32 bit wide */
 @@ -752,7 +750,6 @@ struct Vmxnet3_DriverShared {
 #undef __le32
 #undef __le64
 #undef __packed
 -#undef const_cpu_to_le64
 #if defined(HOST_WORDS_BIGENDIAN)
 #undef __BIG_ENDIAN_BITFIELD
 #endif
 -- 
 1.8.1.4
 
 




Re: [Qemu-devel] [PATCH] target-ppc: Enable ISEL on POWER7

2013-04-02 Thread Alexander Graf

On 01.04.2013, at 17:06, Aurelien Jarno wrote:

 ISEL is a Power ISA 2.06 instruction and thus is available on POWER7.
 Given this is trapped and emulated by the Linux kernel, I guess it went
 unnoticed.
 
 Cc: Alexander Graf ag...@suse.de
 Signed-off-by: Aurelien Jarno aurel...@aurel32.net

Thanks, applied to ppc-next.

Alex

 ---
 target-ppc/translate_init.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
 index 781170f..aea7d27 100644
 --- a/target-ppc/translate_init.c
 +++ b/target-ppc/translate_init.c
 @@ -7004,7 +7004,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
 dc-desc = POWER7;
 pcc-init_proc = init_proc_POWER7;
 pcc-check_pow = check_pow_nocheck;
 -pcc-insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
 +pcc-insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
PPC_FLOAT_STFIWX |
 -- 
 1.7.10.4
 




Re: [Qemu-devel] [PATCH v3 17/27] tcg-ppc64: Implement bswap64

2013-04-02 Thread Alexander Graf

On 02.04.2013, at 06:23, Richard Henderson wrote:

 Reviewed-by: Aurelien Jarno aurel...@aurel32.net
 Signed-off-by: Richard Henderson r...@twiddle.net

Is this faster than a load/store with std/ldbrx?


Alex

 ---
 tcg/ppc64/tcg-target.c | 35 +++
 tcg/ppc64/tcg-target.h |  2 +-
 2 files changed, 36 insertions(+), 1 deletion(-)
 
 diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c
 index d8131ec..1806364 100644
 --- a/tcg/ppc64/tcg-target.c
 +++ b/tcg/ppc64/tcg-target.c
 @@ -1706,6 +1706,40 @@ static void tcg_out_op (TCGContext *s, TCGOpcode opc, 
 const TCGArg *args,
 }
 break;
 
 +case INDEX_op_bswap64_i64:
 +a0 = args[0], a1 = args[1], a2 = 0;
 +if (a0 == a1) {
 +a0 = 0;
 +a2 = a1;
 +}
 +
 +/* a1 = # abcd efgh */
 +/* a0 = rl32(a1, 8) #  fghe */
 +tcg_out_rlw(s, RLWINM, a0, a1, 8, 0, 31);
 +/* a0 = dep(a0, rl32(a1, 24), 0xff00) #  hghe */
 +tcg_out_rlw(s, RLWIMI, a0, a1, 24, 0, 7);
 +/* a0 = dep(a0, rl32(a1, 24), 0xff00) #  hgfe */
 +tcg_out_rlw(s, RLWIMI, a0, a1, 24, 16, 23);
 +
 +/* a0 = rl64(a0, 32) # hgfe  */
 +/* a2 = rl64(a1, 32) # efgh abcd */
 +tcg_out_rld(s, RLDICL, a0, a0, 32, 0);
 +tcg_out_rld(s, RLDICL, a2, a1, 32, 0);
 +
 +/* a0 = dep(a0, rl32(a2, 8), 0x)  # hgfe bcda */
 +tcg_out_rlw(s, RLWIMI, a0, a2, 8, 0, 31);
 +/* a0 = dep(a0, rl32(a2, 24), 0xff00) # hgfe dcda */
 +tcg_out_rlw(s, RLWIMI, a0, a2, 24, 0, 7);
 +/* a0 = dep(a0, rl32(a2, 24), 0xff00) # hgfe dcba */
 +tcg_out_rlw(s, RLWIMI, a0, a2, 24, 16, 23);
 +
 +if (a0 == 0) {
 +tcg_out_mov(s, TCG_TYPE_I64, args[0], a0);
 +/* Revert the source rotate that we performed above.  */
 +tcg_out_rld(s, RLDICL, a1, a1, 32, 0);
 +}
 +break;
 +
 default:
 tcg_dump_ops (s);
 tcg_abort ();
 @@ -1816,6 +1850,7 @@ static const TCGTargetOpDef ppc_op_defs[] = {
 { INDEX_op_bswap16_i64, { r, r } },
 { INDEX_op_bswap32_i32, { r, r } },
 { INDEX_op_bswap32_i64, { r, r } },
 +{ INDEX_op_bswap64_i64, { r, r } },
 
 { -1 },
 };
 diff --git a/tcg/ppc64/tcg-target.h b/tcg/ppc64/tcg-target.h
 index bd011b8..d8e1820 100644
 --- a/tcg/ppc64/tcg-target.h
 +++ b/tcg/ppc64/tcg-target.h
 @@ -102,7 +102,7 @@ typedef enum {
 #define TCG_TARGET_HAS_ext32u_i64   1
 #define TCG_TARGET_HAS_bswap16_i64  1
 #define TCG_TARGET_HAS_bswap32_i64  1
 -#define TCG_TARGET_HAS_bswap64_i64  0
 +#define TCG_TARGET_HAS_bswap64_i64  1
 #define TCG_TARGET_HAS_not_i64  1
 #define TCG_TARGET_HAS_neg_i64  1
 #define TCG_TARGET_HAS_andc_i64 0
 -- 
 1.8.1.4
 




Re: [Qemu-devel] [PATCH V10 12/17] hmp: add function hmp_info_snapshots()

2013-04-02 Thread Kevin Wolf
Am 30.03.2013 um 13:38 hat Wenchao Xia geschrieben:
 于 2013-3-30 7:04, Eric Blake 写道:
 On 03/22/2013 08:19 AM, Wenchao Xia wrote:
This function will simply call qmp interface qmp_query_snapshots()
 added in last commit and then dump information in monitor console.
To get snapshot info, Now qemu and qemu-img both call block layer
 function bdrv_query_snapshot_info_list() in their calling path, and
 then they just translate the qmp object got to strings in stdout or
 monitor console.
 
 Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
 ---
   hmp.c |   42 ++
   hmp.h |1 +
   2 files changed, 43 insertions(+), 0 deletions(-)
 
 diff --git a/hmp.c b/hmp.c
 index b0a861c..c475d65 100644
 --- a/hmp.c
 +++ b/hmp.c
 @@ -651,6 +651,48 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
   qapi_free_TPMInfoList(info_list);
   }
 
 +/* assume list is valid */
 +static void monitor_dump_snapshotinfolist(Monitor *mon, SnapshotInfoList 
 *list)
 +{
 +SnapshotInfoList *elem;
 +char buf[256];
 +
 +monitor_printf(mon, %s\n, bdrv_snapshot_dump(buf, sizeof(buf), 
 NULL));
 
 Are you sure that won't ever be truncated?  I'm pretty sure that I could
 come up with a scenario where I cause bdrv_snapshot_dump() to want to
 output more than 256 bytes of details.
 
   I hope not use dynamic buff for strings, how about check it as
 following:
 
 char buf[254] = 0;
 monitor_printf(mon, %s\n, bdrv_snapshot_dump(buf, sizeof(buf), NULL));
 if (buf[254]) {
  monitor_printf(mon, 'string truncated');
 }

About as useless as the truncated string alone. That something is
missing will be noticed by the user; the fix isn't to tell him that the
output is broken, but to give the full output.

Kevin



Re: [Qemu-devel] [PATCH V10 17/17] hmp: add parameter device and -b for info block

2013-04-02 Thread Kevin Wolf
Am 29.03.2013 um 03:48 hat Wenchao Xia geschrieben:
 于 2013-3-28 19:09, Kevin Wolf 写道:
 Am 22.03.2013 um 15:19 hat Wenchao Xia geschrieben:
With these parameters, user can choose the information to be showed,
 to avoid message flood in the montior.
 
 Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
 
 Aha, so here you actually introduce the 'device' parameter. If you can
 have this patch first, and only then patch 16, then limiting the new
 output to the form with a device specified should be trivial.
 
   It is a bit indirect that info block and info block device
 show different on single device, so I introduced additional parameter
 '-b' to filter out info. With more thinking, I think it should be
 {
 .name   = block,
 .help   = show the block devices,
 .args_type  = verbose:-v,device:B?,
 .params = [-v] [device],
 .help   = show info of one block device or all block devices 
   and detail of images with -v option,
 }
   Then by default info block so brief summary as old time. Since the
 -v parameter is filtering something out not present on patch 15,
 so I can't move this patch forward, hope you are OK with it.

'info block [-v] [device]' is okay with me. I would prefer if you
could arrange the patches so that without -v you never get verbose
output in any intermediate patch, but otherwise I don't mind about
the order of patches.

Kevin



Re: [Qemu-devel] [PATCH] PPC: e500: advertise 4.2 MPIC only if KVM supports EPR

2013-04-02 Thread Alexander Graf

On 30.03.2013, at 17:40, Stuart Yoder wrote:

 From: Stuart Yoder stuart.yo...@freescale.com

Missing patch description

 
 Signed-off-by: Stuart Yoder stuart.yo...@freescale.com
 ---
 hw/ppc/e500plat.c |5 +
 1 file changed, 5 insertions(+)
 
 diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
 index 25ac4b1..2cd7cad 100644
 --- a/hw/ppc/e500plat.c
 +++ b/hw/ppc/e500plat.c
 @@ -16,6 +16,7 @@
 #include sysemu/device_tree.h
 #include hw/pci/pci.h
 #include hw/openpic.h
 +#include sysemu/kvm.h
 
 static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt)
 {
 @@ -48,6 +49,10 @@ static void e500plat_init(QEMUMachineInitArgs *args)
 .mpic_version = OPENPIC_MODEL_FSL_MPIC_42,
 };
 

Missing comment

 +if (kvm_enabled()  !kvm_check_extension(kvm_state, KVM_CAP_PPC_EPR)) {

This should go through target-ppc/kvm_ppc.c. That way we don't need to have any 
kvm specific headers included here and guarantee that everything compiles just 
fine without kvm enabled.

I've fixed those up for you and applied the patch to ppc-next.


Alex

 +params.mpic_version = OPENPIC_MODEL_FSL_MPIC_20;
 +}
 +
 ppce500_init(params);
 }
 
 -- 
 1.7.9.7
 
 




Re: [Qemu-devel] [PATCH 1/2] PPC: Remvove env-hreset_excp_prefix

2013-04-02 Thread Alexander Graf

On 29.03.2013, at 13:06, Fabien Chouteau wrote:

 This value is not needed if we use correctly the MSR[IP] bit.
 
 excp_prefix is always 0x, except when the MSR[IP] bit is
 implemented and set to 1, in that case excp_prefix is 0xfff0.
 
 The handling of MSR[IP] was already implemented but not used at reset
 because the value of env-msr was changed manually.
 
 The patch uses the function hreg_store_msr() to set env-msr, this
 ensures a good handling of MSR[IP] at reset, and therefore a good value
 for excp_prefix.
 
 Signed-off-by: Fabien Chouteau chout...@adacore.com

Thanks, applied to ppc-next.


Alex

 ---
 hw/ppc/spapr.c  |6 --
 target-ppc/cpu.h|1 -
 target-ppc/machine.c|2 --
 target-ppc/translate_init.c |   42 --
 4 files changed, 16 insertions(+), 35 deletions(-)
 
 diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
 index 7b2a11f..6a12485 100644
 --- a/hw/ppc/spapr.c
 +++ b/hw/ppc/spapr.c
 @@ -801,8 +801,10 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
 /* Set time-base frequency to 512 MHz */
 cpu_ppc_tb_init(env, TIMEBASE_FREQ);
 
 -/* PAPR always has exception vectors in RAM not ROM */
 -env-hreset_excp_prefix = 0;
 +/* PAPR always has exception vectors in RAM not ROM. To ensure this,
 + * MSR[IP] should never be set.
 + */
 +env-msr_mask = ~(1  6);
 
 /* Tell KVM that we're in PAPR mode */
 if (kvm_enabled()) {
 diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
 index 42c36e2..99ebf7e 100644
 --- a/target-ppc/cpu.h
 +++ b/target-ppc/cpu.h
 @@ -1026,7 +1026,6 @@ struct CPUPPCState {
 /* Exception vectors */
 target_ulong excp_vectors[POWERPC_EXCP_NB];
 target_ulong excp_prefix;
 -target_ulong hreset_excp_prefix;
 target_ulong ivor_mask;
 target_ulong ivpr_mask;
 target_ulong hreset_vector;
 diff --git a/target-ppc/machine.c b/target-ppc/machine.c
 index 235b0d5..2d10adb 100644
 --- a/target-ppc/machine.c
 +++ b/target-ppc/machine.c
 @@ -78,7 +78,6 @@ void cpu_save(QEMUFile *f, void *opaque)
 for (i = 0; i  POWERPC_EXCP_NB; i++)
 qemu_put_betls(f, env-excp_vectors[i]);
 qemu_put_betls(f, env-excp_prefix);
 -qemu_put_betls(f, env-hreset_excp_prefix);
 qemu_put_betls(f, env-ivor_mask);
 qemu_put_betls(f, env-ivpr_mask);
 qemu_put_betls(f, env-hreset_vector);
 @@ -167,7 +166,6 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
 for (i = 0; i  POWERPC_EXCP_NB; i++)
 qemu_get_betls(f, env-excp_vectors[i]);
 qemu_get_betls(f, env-excp_prefix);
 -qemu_get_betls(f, env-hreset_excp_prefix);
 qemu_get_betls(f, env-ivor_mask);
 qemu_get_betls(f, env-ivpr_mask);
 qemu_get_betls(f, env-hreset_vector);
 diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
 index 781170f..ce5238b 100644
 --- a/target-ppc/translate_init.c
 +++ b/target-ppc/translate_init.c
 @@ -2593,7 +2593,6 @@ static void init_excp_4xx_real (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_FIT]  = 0x1010;
 env-excp_vectors[POWERPC_EXCP_WDT]  = 0x1020;
 env-excp_vectors[POWERPC_EXCP_DEBUG]= 0x2000;
 -env-hreset_excp_prefix = 0xUL;
 env-ivor_mask = 0xFFF0UL;
 env-ivpr_mask = 0xUL;
 /* Hardware reset vector */
 @@ -2618,7 +2617,6 @@ static void init_excp_4xx_softmmu (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_DTLB] = 0x1100;
 env-excp_vectors[POWERPC_EXCP_ITLB] = 0x1200;
 env-excp_vectors[POWERPC_EXCP_DEBUG]= 0x2000;
 -env-hreset_excp_prefix = 0xUL;
 env-ivor_mask = 0xFFF0UL;
 env-ivpr_mask = 0xUL;
 /* Hardware reset vector */
 @@ -2644,7 +2642,6 @@ static void init_excp_MPC5xx (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_IABR] = 0x1C00;
 env-excp_vectors[POWERPC_EXCP_MEXTBR]   = 0x1E00;
 env-excp_vectors[POWERPC_EXCP_NMEXTBR]  = 0x1F00;
 -env-hreset_excp_prefix = 0xUL;
 env-ivor_mask = 0xFFF0UL;
 env-ivpr_mask = 0xUL;
 /* Hardware reset vector */
 @@ -2676,7 +2673,6 @@ static void init_excp_MPC8xx (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_IABR] = 0x1C00;
 env-excp_vectors[POWERPC_EXCP_MEXTBR]   = 0x1E00;
 env-excp_vectors[POWERPC_EXCP_NMEXTBR]  = 0x1F00;
 -env-hreset_excp_prefix = 0xUL;
 env-ivor_mask = 0xFFF0UL;
 env-ivpr_mask = 0xUL;
 /* Hardware reset vector */
 @@ -2704,7 +2700,6 @@ static void init_excp_G2 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_DSTLB]= 0x1200;
 env-excp_vectors[POWERPC_EXCP_IABR] = 0x1300;
 env-excp_vectors[POWERPC_EXCP_SMI]  = 0x1400;
 -env-hreset_excp_prefix = 0xUL;
 /* Hardware reset vector */
 env-hreset_vector = 0xFFFCUL;
 #endif
 @@ -2733,7 +2728,6 @@ static void 

Re: [Qemu-devel] [PATCH 2/2] PPC: fix hreset_vector for 60x, 7x0, 7x5, G2, MPC8xx, MPC5xx, 7400 and 7450

2013-04-02 Thread Alexander Graf

On 29.03.2013, at 13:06, Fabien Chouteau wrote:

 According to the different user's manuals, the vector offset for system
 reset (both /HRESET and /SRESET) is 0x00100.
 
 This patch may break support of some executables, as the power-on start
 address may change. For a specific board, if the power-on start address
 is different than HRESET vector (i.e. 0x0100 or 0xfff00100), this
 should be fixed in board's initialization code.
 
 Signed-off-by: Fabien Chouteau chout...@adacore.com

What about the other users of hreset_vector? Can we just remove the variable 
altogether?


Alex

 ---
 target-ppc/translate_init.c |   22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)
 
 diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
 index ce5238b..2c9b477 100644
 --- a/target-ppc/translate_init.c
 +++ b/target-ppc/translate_init.c
 @@ -2645,7 +2645,7 @@ static void init_excp_MPC5xx (CPUPPCState *env)
 env-ivor_mask = 0xFFF0UL;
 env-ivpr_mask = 0xUL;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0x0100UL;
 #endif
 }
 
 @@ -2676,7 +2676,7 @@ static void init_excp_MPC8xx (CPUPPCState *env)
 env-ivor_mask = 0xFFF0UL;
 env-ivpr_mask = 0xUL;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0x0100UL;
 #endif
 }
 
 @@ -2701,7 +2701,7 @@ static void init_excp_G2 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_IABR] = 0x1300;
 env-excp_vectors[POWERPC_EXCP_SMI]  = 0x1400;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0x0100UL;
 #endif
 }
 
 @@ -2804,7 +2804,7 @@ static void init_excp_602 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_WDT]  = 0x1500;
 env-excp_vectors[POWERPC_EXCP_EMUL] = 0x1600;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0x0100UL;
 #endif
 }
 
 @@ -2828,7 +2828,7 @@ static void init_excp_603 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_IABR] = 0x1300;
 env-excp_vectors[POWERPC_EXCP_SMI]  = 0x1400;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0x0100UL;
 #endif
 }
 
 @@ -2873,7 +2873,7 @@ static void init_excp_7x0 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_SMI]  = 0x1400;
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0x0100UL;
 #endif
 }
 
 @@ -2895,7 +2895,7 @@ static void init_excp_750cl (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_IABR] = 0x1300;
 env-excp_vectors[POWERPC_EXCP_SMI]  = 0x1400;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0x0100UL;
 #endif
 }
 
 @@ -2917,7 +2917,7 @@ static void init_excp_750cx (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_IABR] = 0x1300;
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0x0100UL;
 #endif
 }
 
 @@ -2944,7 +2944,7 @@ static void init_excp_7x5 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_SMI]  = 0x1400;
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0x0100UL;
 #endif
 }
 
 @@ -2969,7 +2969,7 @@ static void init_excp_7400 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_VPUA] = 0x1600;
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0x0100UL;
 #endif
 }
 
 @@ -2996,7 +2996,7 @@ static void init_excp_7450 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_SMI]  = 0x1400;
 env-excp_vectors[POWERPC_EXCP_VPUA] = 0x1600;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0x0100UL;
 #endif
 }
 
 -- 
 1.7.9.5
 




Re: [Qemu-devel] [PATCH v2] target-ppc: fix nego and subf*o instructions

2013-04-02 Thread Alexander Graf

On 01.04.2013, at 02:33, Aurelien Jarno wrote:

 The overflow computation of nego and subf*o instructions has been broken
 in commit ffe30937. Contrary to other targets, the instruction is subtract
 from an not subtract on PowerPC.
 
 This patch fixes the issue by using the correct argument in the xor
 computation. Thanks to Peter Maydell for the hint.
 
 With this change the PPC emulation passes the Gwenole Beauchesne
 testsuite again.
 
 Cc: Alexander Graf ag...@suse.de
 Cc: Richard Henderson r...@twiddle.net
 Signed-off-by: Aurelien Jarno aurel...@aurel32.net

Thanks, applied to ppc-next.

Alex




Re: [Qemu-devel] [RFC PATCH 1/3] aio-context: if io_flush isn't provided, assume always busy

2013-04-02 Thread Kevin Wolf
Am 29.03.2013 um 00:37 hat Paolo Bonzini geschrieben:
 Il 28/03/2013 22:52, Anthony Liguori ha scritto:
  Today, all callers of qemu_aio_set_fd_handler() pass a valid io_flush
  function.
 
 Except one:
 
 aio_set_event_notifier(ctx, ctx-notifier,
(EventNotifierHandler *)
event_notifier_test_and_clear, NULL);
 
 This is the EventNotifier that is used by qemu_notify_event.
 
 It's quite surprising that this patch works and passes the tests. /me
 reads cover letter... ah, it is untested. :)
 
 But if you can eliminate the sole usage of aio_wait()'s return value (in
 bdrv_drain_all()), everything would be much simpler.  There is a
 relatively convenient
 
 assert(QLIST_EMPTY(bs-tracked_requests));
 
 that you can use as the exit condition instead.  Perhaps it's not
 trivial to do it efficiently, but it's not a fast path.

We just need to move to .bdrv_drain() for all block driver that
register an AioHandler. I'm pretty sure that each one has its own
data structures to manage in-flight requests (basically what is the
aio_flush handler today would become the .bdrv_drain callback).

Then bdrv_drain_all() can directly use the bdrv_drain() return value and
doesn't need to have it passed through aio_wait() any more.

Kevin



Re: [Qemu-devel] [RFC PATCH 2/3] sheepdog: pass NULL for io_flush

2013-04-02 Thread Kevin Wolf
Am 28.03.2013 um 22:52 hat Anthony Liguori geschrieben:
 Signed-off-by: Anthony Liguori aligu...@us.ibm.com
 ---
  block/sheepdog.c | 11 ++-
  1 file changed, 2 insertions(+), 9 deletions(-)
 
 diff --git a/block/sheepdog.c b/block/sheepdog.c
 index bb67c4c..2bccd9b 100644
 --- a/block/sheepdog.c
 +++ b/block/sheepdog.c
 @@ -503,13 +503,6 @@ static void restart_co_req(void *opaque)
  qemu_coroutine_enter(co, NULL);
  }
  
 -static int have_co_req(void *opaque)
 -{
 -/* this handler is set only when there is a pending request, so
 - * always returns 1. */

Now you return 1 even when no request is pending (which is the case in
which no io_flush handler would be set before). Why is this correct?
(This is actually a question about PATCH 1/3, I just noticed it here.
Are there more cases like this?)

Kevin



Re: [Qemu-devel] [PATCH V10 11/17] qmp: add ImageInfo in BlockDeviceInfo used by query-block

2013-04-02 Thread Kevin Wolf
Am 29.03.2013 um 03:35 hat Wenchao Xia geschrieben:
 于 2013-3-28 17:54, Kevin Wolf 写道:
 Am 22.03.2013 um 15:19 hat Wenchao Xia geschrieben:
Now image info will be retrieved as an embbed json object inside
 BlockDeviceInfo, backing chain info and all related internal snapshot
 info can be got in the enhanced recursive structure of ImageInfo.
 
 Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
 ---
   block/qapi.c |   39 ++--
   include/block/qapi.h |4 ++-
   qapi-schema.json |5 +++-
   qmp-commands.hx  |   67 
  +-
   4 files changed, 109 insertions(+), 6 deletions(-)
 
 diff --git a/block/qapi.c b/block/qapi.c
 index df73f5b..9051947 100644
 --- a/block/qapi.c
 +++ b/block/qapi.c
 @@ -200,9 +200,15 @@ int bdrv_query_image_info(BlockDriverState *bs,
   return 0;
   }
 
 -BlockInfo *bdrv_query_info(BlockDriverState *bs)
 +/* return 0 on success, and @p_info will be set only on success. */
 +int bdrv_query_info(BlockDriverState *bs,
 +BlockInfo **p_info,
 +Error **errp)
   {
   BlockInfo *info = g_malloc0(sizeof(*info));
 +BlockDriverState *bs0;
 +ImageInfo **p_image_info;
 +int ret = 0;
 
 ret is never changed, so this function always returns 0. I would suggest
 to drop ret and make the function return type void.
 
   My bad, I forgot to set its value, the interface is intend to return
 negative error number and errp both on fail.

Why do you need two separate error reporting mechanisms? Shouldn't only
errp be enough?

Kevin



Re: [Qemu-devel] [PATCH] append the terminating '\0' to bootorder string

2013-04-02 Thread Gerd Hoffmann
On 03/20/13 11:16, Amos Kong wrote:
 Problem was introduced in commit c8a6ae8b. The last terminating
 '\0' was lost, use the right length 5 (HALT\0).

Acked-by: Gerd Hoffmann kra...@redhat.com

cheers,
  Gerd



Re: [Qemu-devel] [PATCH 2/2] PPC: fix hreset_vector for 60x, 7x0, 7x5, G2, MPC8xx, MPC5xx, 7400 and 7450

2013-04-02 Thread Fabien Chouteau
On 04/02/2013 10:09 AM, Alexander Graf wrote:
 
 On 29.03.2013, at 13:06, Fabien Chouteau wrote:
 
 According to the different user's manuals, the vector offset for system
 reset (both /HRESET and /SRESET) is 0x00100.

 This patch may break support of some executables, as the power-on start
 address may change. For a specific board, if the power-on start address
 is different than HRESET vector (i.e. 0x0100 or 0xfff00100), this
 should be fixed in board's initialization code.

 Signed-off-by: Fabien Chouteau chout...@adacore.com
 
 What about the other users of hreset_vector? Can we just remove the variable 
 altogether?


For the CPUs that don't implement MSR[IP], the hreset_prefix is always
0x, so hreset_vector will be the reset address (i.e. 0xFFFC)

I guess all this can be converted to a CPU feature flag, telling if
reset is an exception or not.

-- 
Fabien Chouteau



Re: [Qemu-devel] [PATCH V10 11/17] qmp: add ImageInfo in BlockDeviceInfo used by query-block

2013-04-02 Thread Wenchao Xia

于 2013-4-2 16:09, Kevin Wolf 写道:

Am 29.03.2013 um 03:35 hat Wenchao Xia geschrieben:

于 2013-3-28 17:54, Kevin Wolf 写道:

Am 22.03.2013 um 15:19 hat Wenchao Xia geschrieben:

   Now image info will be retrieved as an embbed json object inside
BlockDeviceInfo, backing chain info and all related internal snapshot
info can be got in the enhanced recursive structure of ImageInfo.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
  block/qapi.c |   39 ++--
  include/block/qapi.h |4 ++-
  qapi-schema.json |5 +++-
  qmp-commands.hx  |   67 +-
  4 files changed, 109 insertions(+), 6 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index df73f5b..9051947 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -200,9 +200,15 @@ int bdrv_query_image_info(BlockDriverState *bs,
  return 0;
  }

-BlockInfo *bdrv_query_info(BlockDriverState *bs)
+/* return 0 on success, and @p_info will be set only on success. */
+int bdrv_query_info(BlockDriverState *bs,
+BlockInfo **p_info,
+Error **errp)
  {
  BlockInfo *info = g_malloc0(sizeof(*info));
+BlockDriverState *bs0;
+ImageInfo **p_image_info;
+int ret = 0;


ret is never changed, so this function always returns 0. I would suggest
to drop ret and make the function return type void.


   My bad, I forgot to set its value, the interface is intend to return
negative error number and errp both on fail.


Why do you need two separate error reporting mechanisms? Shouldn't only
errp be enough?

Kevin


  Returned value can tell caller what error it is, like -ENOMEDIUM.
Although it is not used by caller now, but I feel better to have it
just like

+int bdrv_query_snapshot_info_list(BlockDriverState *bs,
+  SnapshotInfoList **p_list,
+  Error **errp)

in patch 5/17.

--
Best Regards

Wenchao Xia




Re: [Qemu-devel] [PATCH 2/2] PPC: fix hreset_vector for 60x, 7x0, 7x5, G2, MPC8xx, MPC5xx, 7400 and 7450

2013-04-02 Thread Alexander Graf

On 02.04.2013, at 09:53, Fabien Chouteau wrote:

 On 04/02/2013 10:09 AM, Alexander Graf wrote:
 
 On 29.03.2013, at 13:06, Fabien Chouteau wrote:
 
 According to the different user's manuals, the vector offset for system
 reset (both /HRESET and /SRESET) is 0x00100.
 
 This patch may break support of some executables, as the power-on start
 address may change. For a specific board, if the power-on start address
 is different than HRESET vector (i.e. 0x0100 or 0xfff00100), this
 should be fixed in board's initialization code.
 
 Signed-off-by: Fabien Chouteau chout...@adacore.com
 
 What about the other users of hreset_vector? Can we just remove the variable 
 altogether?
 
 
 For the CPUs that don't implement MSR[IP], the hreset_prefix is always
 0x, so hreset_vector will be the reset address (i.e. 0xFFFC)
 
 I guess all this can be converted to a CPU feature flag, telling if
 reset is an exception or not.

Hrm, maybe. Not sure which way is cleaner.

I'll apply your patch for now. It's certainly a step into the right direction.


Alex




[Qemu-devel] qemu crashed when starting vm(kvm) with vnc connect

2013-04-02 Thread Zhanghaoyu (A)
I start a kvm VM with vnc(using the zrle protocol) connect, sometimes qemu 
program crashed during starting period, received signal SIGABRT.
Trying about 20 times, this crash may be reproduced.
I guess the cause memory corruption or double free.

The backtrace shown as below:

0x7f32eda3dd95 in raise () from /lib64/libc.so.6
(gdb) bt
#0  0x7f32eda3dd95 in raise () from /lib64/libc.so.6
#1  0x7f32eda3f2ab in abort () from /lib64/libc.so.6
#2  0x7f32eda77ece in __libc_message () from /lib64/libc.so.6
#3  0x7f32eda7dc06 in malloc_printerr () from /lib64/libc.so.6
#4  0x7f32eda7ecda in _int_free () from /lib64/libc.so.6
#5  0x7f32efd3452c in free_and_trace (mem=0x7f329cd0) at vl.c:2880
#6  0x7f32efd251a1 in buffer_free (buffer=0x7f32f0c82890) at ui/vnc.c:505
#7  0x7f32efd20c56 in vnc_zrle_clear (vs=0x7f32f0c762d0)
at ui/vnc-enc-zrle.c:364
#8  0x7f32efd26d07 in vnc_disconnect_finish (vs=0x7f32f0c762d0)
at ui/vnc.c:1050
#9  0x7f32efd275c5 in vnc_client_read (opaque=0x7f32f0c762d0)
at ui/vnc.c:1349
#10 0x7f32efcb397c in qemu_iohandler_poll (readfds=0x7f32f074d020,
writefds=0x7f32f074d0a0, xfds=0x7f32f074d120, ret=1) at iohandler.c:124
#11 0x7f32efcb46e8 in main_loop_wait (nonblocking=0) at main-loop.c:417
#12 0x7f32efd31159 in main_loop () at vl.c:2133
#13 0x7f32efd38070 in main (argc=46, argv=0x7fff7f5df178,
envp=0x7fff7f5df2f0) at vl.c:4481


[Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread

2013-04-02 Thread Peter Crosthwaite
Public bug: 1154328
Broken Commit: a29753f8aa79a34a324afebe340182a51a5aef11

ATM, the timeout from g_pollfds_fill is inhibiting unlocking of the
iothread. This is capable of causing a total deadlock when hw/serial
is used as a device. The bug manifests when you go -nographic -serial
mon:stdio and then paste 40 or more chars into the terminal.

My knowledge of this g_foo is vague at best, but my best working
theory is this:

- First 8 chars are recieved by the serial device no complaints.
- The next 32 chars, serial returns false for can_receive() so they
are buffered by the MuxDriver object - mux_chr_read()
- Buffer is full, so 41st char causes false return from Muxes own
can_read()
- This propagates all the way up to glib_pollfds_fill and manifests
as a timeout
- Timeout means no unlock of IOthread. Device land never sees any more
cycles so the serial port never progresses - no flushing of
buffer
- Deadlock

Tested on petalogix_ml605 microblazeel machine model, which was faulty
due to 1154328.

Fix by removing the conditions on unlocking the iothread. Don't know
what else this will break but the timeout is certainly the wrong
condition for the unlock. Probably the real solution is to have a more
selective unlock policy.

I'm happy for someone to take this patch off my hands, or educate me on
the correct implementation. For the peeps doing automated testing on
nographic platforms this will get your build working again.

Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
---
 main-loop.c |8 ++--
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/main-loop.c b/main-loop.c
index eb80ff3..a376898 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -194,15 +194,11 @@ static int os_host_main_loop_wait(uint32_t timeout)
 
 glib_pollfds_fill(timeout);
 
-if (timeout  0) {
-qemu_mutex_unlock_iothread();
-}
+qemu_mutex_unlock_iothread();
 
 ret = g_poll((GPollFD *)gpollfds-data, gpollfds-len, timeout);
 
-if (timeout  0) {
-qemu_mutex_lock_iothread();
-}
+qemu_mutex_lock_iothread();
 
 glib_pollfds_poll();
 return ret;
-- 
1.7.0.4




Re: [Qemu-devel] [SeaBIOS] [PATCH v16] Add pvpanic device driver

2013-04-02 Thread Gleb Natapov
On Mon, Apr 01, 2013 at 08:22:57PM -0400, Kevin O'Connor wrote:
 On Sun, Mar 31, 2013 at 05:34:10PM +0300, Gleb Natapov wrote:
  On Sat, Mar 30, 2013 at 09:20:09AM -0400, Kevin O'Connor wrote:
   On Fri, Mar 29, 2013 at 02:49:12PM +0100, Paolo Bonzini wrote:
Il 29/03/2013 14:33, Kevin O'Connor ha scritto:
 On Fri, Mar 29, 2013 at 04:18:44PM +0800, Hu Tao wrote:
 pvpanic device is used to notify host(qemu) when guest panic happens.
 
 Thanks.  However, we're planning a move of ACPI tables from SeaBIOS to
 QEMU.  I think this should wait until after the move.

The device should be in QEMU 1.5, and the SSDT probably will still be in
SeaBIOS by then (and might even be the last to move, since it's quite
complex and dynamic).  I don't think it is fair to block this patch on
those grounds...
   
   What is the user visible impact of not having a panic device?
   
   My main concern is that the patch creates a new fw_cfg channel between
   qemu and seabios thats sole purpose is to alter the OS visible ACPI
   tables.  These types of QEMU-SeaBIOS interfaces are fragile and are
   (in sum) quite complex.
   
  The patch uses existing channel between qemu and seabios, one
  romfile_loadint() is all it takes. We already have number of interfaces
  to change OS visible ACPI tables, that's why we want to move ACPI table
  creation to QEMU in the first place. It is unfortunate to start blocking
  features now before we have an alternative. When ACPI table creation
  will move into QEMU the code in this patch will be dropped along with
  all the other code that serves similar purpose.
 
 Hi Gleb,
 
 If there is a general consensus that this feature is important then
 we'll go forward with adding it as is.  To be clear though, my
 preference would be to go forward with moving ACPI tables into QEMU,
 and then add this stuff on top of that.  If no one beats me to it,
 I'll send some initial patches myself.
 
If we can accomplish the move before next major QEMU release we do not
need this new fw_cfg file obviously. Paolo thinks this is not feasible,
I haven't followed this work to close to have informed opinion.

 If we do merge this feature as is, it will also require a SeaBIOS
 release (followed by a SeaBIOS pull into QEMU).  There weren't any
 short-term plans for a new SeaBIOS release - if this feature demands
 that then we need to start planning it now.
 
It is always good to have latest BIOS with new QEMU release IMO, so lest
plan it regardless.

--
Gleb.



Re: [Qemu-devel] [PATCH 2/2] PPC: fix hreset_vector for 60x, 7x0, 7x5, G2, MPC8xx, MPC5xx, 7400 and 7450

2013-04-02 Thread Fabien Chouteau
On 04/02/2013 10:56 AM, Alexander Graf wrote:
 
 On 02.04.2013, at 09:53, Fabien Chouteau wrote:
 
 On 04/02/2013 10:09 AM, Alexander Graf wrote:

 What about the other users of hreset_vector? Can we just remove the 
 variable altogether?


 For the CPUs that don't implement MSR[IP], the hreset_prefix is always
 0x, so hreset_vector will be the reset address (i.e. 0xFFFC)

 I guess all this can be converted to a CPU feature flag, telling if
 reset is an exception or not.
 
 Hrm, maybe. Not sure which way is cleaner.
 
 I'll apply your patch for now. It's certainly a step into the right direction.


OK, thanks,

-- 
Fabien Chouteau



Re: [Qemu-devel] [RFC PATCH v4 00/30] ACPI memory hotplug

2013-04-02 Thread liu ping fan
On Fri, Jan 11, 2013 at 2:57 AM, Vasilis Liaskovitis
vasilis.liaskovi...@profitbricks.com wrote:
 
  IIRC q35 supports memory hotplug natively (picked up in some
  discussion).  Is that correct?
 
 From previous discussion I also understand that q35 supports native hotplug.
 Sections 5.1 and 5.2 of the spec describe the MCH registers but the native
 memory hotplug specifics are not yet clear to me. Any pointers from the
 spec are welcome.

 Ping. Could anyone who's familiar with the q35 spec provide some pointers on
 native memory hotplug details in the spec? I see pcie hotplug registers but 
 can't
 find memory hotplug interface details. If I am not mistaken, the spec is here:
 http://www.intel.com/design/chipsets/datashts/316966.htm

 Is the q35 memory hotplug support supposed to be an shpc-like interface geared
 towards memory slots instead of pci slots?

I think there is no dedicated pci express downstream port for memory
slots, so native plug is not available.


 thanks,

 - Vasilis




Re: [Qemu-devel] [PATCH 2/2] Monitor: Make output buffer dynamic

2013-04-02 Thread Gerd Hoffmann
On 03/27/13 07:45, Wenchao Xia wrote:
 Hi, Luiz
   Personally I hope reduce the dynamic allocated buffer which brings
 fragments and unexpected memory grow.

It's a tradeoff.  We can reduce the dynamic allocation, by simply
reusing the qstring instead of allocation a new one (after
complete/partial write).  At the cost of wasting some memory.

Given that qstrings grow expotentially I think the dynamic allocation
overhead isn't that bad.

 Instead, how about sacrifice
 some time to wait output complete, since monitor is not time critical?

The watch added recently will fire when there is space for writing.
Right now it will write out the next chunk from the buffer.

In theory we could change the monitor code to continue generating the
response when there is more output space, so we don't have to buffer it
in the first place.  But I think the complexity simply isn't worth it
for the memory savings we can gain.

cheers,
  Gerd





Re: [Qemu-devel] [PATCH 0/2] Monitor: make output buffer dynamic

2013-04-02 Thread Gerd Hoffmann
On 03/25/13 20:40, Luiz Capitulino wrote:
 This series fixes an easy to reproduce assertion in the Monitor code. The
 second patch contains all the relevant details.
 
 Gerd, I'd like a reviewed-by from you before merging this.

Series looks good to me.

Reviewed-by: Gerd Hoffmann kra...@redhat.com

cheers,
  Gerd



[Qemu-devel] [PATCH 1/2] po/Makefile: Fix dependency for %.mo

2013-04-02 Thread Kevin Wolf
Otherwise make will refuse to build updated .po files.

Signed-off-by: Kevin Wolf kw...@redhat.com
---
 po/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/po/Makefile b/po/Makefile
index 8297ab5..60ccd7d 100644
--- a/po/Makefile
+++ b/po/Makefile
@@ -30,7 +30,7 @@ install: $(OBJS)
$(INSTALL) -m644 $$obj 
$(DESTDIR)$(prefix)/share/locale/$$base/LC_MESSAGES/qemu.mo; \
done
 
-%.mo:
+%.mo: %.po
@msgfmt -o $@ $(SRC_PATH)/po/`basename $@ .mo`.po
 
 messages.po: $(SRC_PATH)/ui/gtk.c
-- 
1.8.1.4




[Qemu-devel] [PATCH 0/2] More translation improvements

2013-04-02 Thread Kevin Wolf
This series is meant to be applied on top of Aurélien's series.

Kevin Wolf (2):
  po/Makefile: Fix dependency for %.mo
  po: Update German translation

 po/Makefile |  2 +-
 po/de_DE.po | 12 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

-- 
1.8.1.4




[Qemu-devel] [PATCH 2/2] po: Update German translation

2013-04-02 Thread Kevin Wolf
Signed-off-by: Kevin Wolf kw...@redhat.com
---
 po/de_DE.po | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/po/de_DE.po b/po/de_DE.po
index 2566674..92c5df5 100644
--- a/po/de_DE.po
+++ b/po/de_DE.po
@@ -18,27 +18,27 @@ msgstr 
 
 #: ../ui/gtk.c:213
 msgid  - Press Ctrl+Alt+G to release grab
-msgstr 
+msgstr - Strg+Alt+G drücken, um Eingabegeräte freizugeben
 
 #: ../ui/gtk.c:217
 msgid  [Paused]
-msgstr 
+msgstr [Angehalten]
 
 #: ../ui/gtk.c:1250
 msgid _Machine
-msgstr 
+msgstr _Maschine
 
 #: ../ui/gtk.c:1252
 msgid _Pause
-msgstr 
+msgstr _Angehalten
 
 #: ../ui/gtk.c:1258
 msgid _Reset
-msgstr 
+msgstr _Reset
 
 #: ../ui/gtk.c:1261
 msgid Power _Down
-msgstr 
+msgstr _Herunterfahren
 
 #: ../ui/gtk.c:1276
 msgid _View
-- 
1.8.1.4




Re: [Qemu-devel] [PATCH V10 11/17] qmp: add ImageInfo in BlockDeviceInfo used by query-block

2013-04-02 Thread Kevin Wolf
Am 02.04.2013 um 10:54 hat Wenchao Xia geschrieben:
 于 2013-4-2 16:09, Kevin Wolf 写道:
 Am 29.03.2013 um 03:35 hat Wenchao Xia geschrieben:
 于 2013-3-28 17:54, Kevin Wolf 写道:
 Am 22.03.2013 um 15:19 hat Wenchao Xia geschrieben:
Now image info will be retrieved as an embbed json object inside
 BlockDeviceInfo, backing chain info and all related internal snapshot
 info can be got in the enhanced recursive structure of ImageInfo.
 
 Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
 ---
   block/qapi.c |   39 ++--
   include/block/qapi.h |4 ++-
   qapi-schema.json |5 +++-
   qmp-commands.hx  |   67 
  +-
   4 files changed, 109 insertions(+), 6 deletions(-)
 
 diff --git a/block/qapi.c b/block/qapi.c
 index df73f5b..9051947 100644
 --- a/block/qapi.c
 +++ b/block/qapi.c
 @@ -200,9 +200,15 @@ int bdrv_query_image_info(BlockDriverState *bs,
   return 0;
   }
 
 -BlockInfo *bdrv_query_info(BlockDriverState *bs)
 +/* return 0 on success, and @p_info will be set only on success. */
 +int bdrv_query_info(BlockDriverState *bs,
 +BlockInfo **p_info,
 +Error **errp)
   {
   BlockInfo *info = g_malloc0(sizeof(*info));
 +BlockDriverState *bs0;
 +ImageInfo **p_image_info;
 +int ret = 0;
 
 ret is never changed, so this function always returns 0. I would suggest
 to drop ret and make the function return type void.
 
My bad, I forgot to set its value, the interface is intend to return
 negative error number and errp both on fail.
 
 Why do you need two separate error reporting mechanisms? Shouldn't only
 errp be enough?
 
 Kevin
 
   Returned value can tell caller what error it is, like -ENOMEDIUM.
 Although it is not used by caller now, but I feel better to have it

No, let's remove it if there is no user. We can always add it back if we
ever need it. I doubt that we will.

Kevin



Re: [Qemu-devel] [RFC PATCH v2 0/4] port network layer onto glib

2013-04-02 Thread liu ping fan
On Thu, Mar 28, 2013 at 9:40 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
 On Thu, Mar 28, 2013 at 09:42:47AM +0100, Paolo Bonzini wrote:
 Il 28/03/2013 08:55, Liu Ping Fan ha scritto:
 3rd. block layer's AioContext will block other AioContexts on the same 
  thread.

 I cannot understand this.

 The plan is for BlockDriverState to be bound to an AioContext.  That
 means each thread is set up with one AioContext.  BlockDriverStates that
 are used in that thread will first be bound to its AioContext.

 It's not very useful to have multiple AioContext in the same thread.

But it can be the case that we detach and re-attach the different
device( AioContext) to the same thread.   I think that the design of
io_flush is to sync, but for NetClientState, we need something else.
So if we use AioContext, is it proper to extend readable/writeable
interface for qemu_aio_set_fd_handler()?

Thanks,
Pingfan

 Stefan



Re: [Qemu-devel] [PATCH] acpi: initialize s4_val used in s4 shutdown

2013-04-02 Thread Gerd Hoffmann
On 04/01/13 18:57, Bruce Rogers wrote:
 While investigating why a 32 bit Windows 2003 guest wasn't able to
 successfully perform a shutdown /h, it was discovered that commit
 afafe4bbe0cf7d3318e1ac7b40925561f86a6bd4 inadvertently dropped the
 initialization of the s4_val used to handle s4 shutdown.
 Initialize the value as before.

 --- a/hw/acpi.c
 +++ b/hw/acpi.c
 @@ -474,6 +474,7 @@ static const MemoryRegionOps acpi_pm_cnt_ops = {
  
  void acpi_pm1_cnt_init(ACPIREGS *ar, MemoryRegion *parent)
  {
 +ar-pm1.cnt.s4_val = 2;

The '2' used to come from PIIX4PMState-s4_val before commit
afafe4bbe0cf7d3318e1ac7b40925561f86a6bd4, see piix4_pm_properties, this
behavior should be maintained IMHO.

cheers,
  Gerd





Re: [Qemu-devel] [RFC PATCH 2/3] sheepdog: pass NULL for io_flush

2013-04-02 Thread Paolo Bonzini

  diff --git a/block/sheepdog.c b/block/sheepdog.c
  index bb67c4c..2bccd9b 100644
  --- a/block/sheepdog.c
  +++ b/block/sheepdog.c
  @@ -503,13 +503,6 @@ static void restart_co_req(void *opaque)
   qemu_coroutine_enter(co, NULL);
   }
   
  -static int have_co_req(void *opaque)
  -{
  -/* this handler is set only when there is a pending request, so
  - * always returns 1. */
 
 Now you return 1 even when no request is pending (which is the case in
 which no io_flush handler would be set before). Why is this correct?
 (This is actually a question about PATCH 1/3, I just noticed it here.
 Are there more cases like this?)

In the dataplane code, the ioeventfd uses an io_flush callback that returns
true.

Paolo



Re: [Qemu-devel] [PATCH 1/1] rng backend: open backend in blocking mode

2013-04-02 Thread Amit Shah
On (Mon) 01 Apr 2013 [09:02:46], Anthony Liguori wrote:
 Amit Shah amit.s...@redhat.com writes:
 
  Opening backends in non-blocking mode isn't necessary, we don't do
  anything while waiting for data.
 
  This also excuses us from checking for EAGAIN, which for the default
  random backend, is a very common return error type.
 
 It's not common...  It really shouldn't happen however.

EAGAIN is common when a file is opened in non-blocking mode.  Needs to
be made verbose?

   Starting the guest
  with '-device virtio-rng-pci', issuing a 'cat /dev/hwrng' in the guest
  while also doing 'cat /dev/random' on the host causes
 
 You are essentially cat'ing the same device twice.  What's happening is
 that there is entropy available in /dev/random so a select()
 notification happens but before we are able to read() it, the cat of
 /dev/hwrng ends up consuming that entropy.
 
 This would never happen with a socket, for instance.  /dev/random is
 special in there are multiple readers.
 
 
  backends/rng-random.c:44:entropy_available: assertion failed: (len != -1)
 
  without this fix.
 
 This fix would cause QEMU to block indefinitely which I don't think is
 very good behavior.  I think a better solution would be:
 
 diff --git a/backends/rng-random.c b/backends/rng-random.c
 index acd20af..9fde566 100644
 --- a/backends/rng-random.c
 +++ b/backends/rng-random.c
 @@ -41,6 +41,9 @@ static void entropy_available(void *opaque)
  ssize_t len;
  
  len = read(s-fd, buffer, s-size);
 +if (len == -1  errno == EINTR) {
 +return;
 +}

This has to be an additional fix on top of this one.  EAGAIN has to be
handled if we want to allow nonblocking reads, and there doesn't seem
to be any reason to have these reads be non-blocking.

OTOH, I also think we could use the glib functions for file IO, since
handling EINTR in each open-coded read call isn't always fun.

Amit



Re: [Qemu-devel] [PATCH 1/7] virtio-console: Also throttle when less was written then requested

2013-04-02 Thread Amit Shah
On (Mon) 01 Apr 2013 [18:00:55], Hans de Goede wrote:
 Hi,
 
 On 03/29/2013 10:31 AM, Amit Shah wrote:
 On (Thu) 28 Mar 2013 [14:28:11], Hans de Goede wrote:
 This is necessary so that we get properly woken up to write the rest.
 
 Signed-off-by: Hans de Goede hdego...@redhat.com
 Acked-by: Amit Shah amit.s...@redhat.com
 ---
   hw/virtio-console.c | 5 +++--
   1 file changed, 3 insertions(+), 2 deletions(-)
 
 diff --git a/hw/virtio-console.c b/hw/virtio-console.c
 index 284180f..015947c 100644
 --- a/hw/virtio-console.c
 +++ b/hw/virtio-console.c
 @@ -47,7 +47,7 @@ static ssize_t flush_buf(VirtIOSerialPort *port, const 
 uint8_t *buf, size_t len)
   ret = qemu_chr_fe_write(vcon-chr, buf, len);
   trace_virtio_console_flush_buf(port-id, len, ret);
 
 -if (ret = 0) {
 +if (ret  len) {
 
 Oops, there's a conversion bug here: len is size_t, and ret is
 ssize_t.  Both need to be the same type.
 
 Since we're not returning negative, ret should be changed to size_t.
 The function signature changes too, but that's alright.
 
 Erm changing ret to a size_t will break things, since qemu_chr_fe_write can
 return  0 and storing that into an unsigned will change it into a very
 large value instead ...

Oh, definitely.  I meant changing the return type of flush_buf(), but
that doesn't help matters much...

 A better fix would be to change len to ssize_t. Although that will cause
 issues if we ever get a single buf which is larger then 2^31 - 1. Note
 we already have that problem since qemu_chr_fe_write already cannot handle
 such large buffers...

Yep.  Do you want to do that as part of this series?

Amit



[Qemu-devel] [PATCH] vga.c: Debug messages go to stderr

2013-04-02 Thread Alex DAMIAN
From: Alexandru DAMIAN alexandru.dam...@intel.com

Move debug messages to stderr since often in vga code debug
stdio is used as serial console.

Signed-off-by: Alexandru DAMIAN alexandru.dam...@intel.com
---
 hw/vga.c |   24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/vga.c b/hw/vga.c
index 59bfb22..2e337b8 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -391,7 +391,7 @@ uint32_t vga_ioport_read(void *opaque, uint32_t addr)
 case VGA_SEQ_D:
 val = s-sr[s-sr_index];
 #ifdef DEBUG_VGA_REG
-printf(vga: read SR%x = 0x%02x\n, s-sr_index, val);
+fprintf(stderr, vga: read SR%x = 0x%02x\n, s-sr_index, val);
 #endif
 break;
 case VGA_PEL_IR:
@@ -419,7 +419,7 @@ uint32_t vga_ioport_read(void *opaque, uint32_t addr)
 case VGA_GFX_D:
 val = s-gr[s-gr_index];
 #ifdef DEBUG_VGA_REG
-printf(vga: read GR%x = 0x%02x\n, s-gr_index, val);
+fprintf(stderr, vga: read GR%x = 0x%02x\n, s-gr_index, val);
 #endif
 break;
 case VGA_CRT_IM:
@@ -430,7 +430,7 @@ uint32_t vga_ioport_read(void *opaque, uint32_t addr)
 case VGA_CRT_DC:
 val = s-cr[s-cr_index];
 #ifdef DEBUG_VGA_REG
-printf(vga: read CR%x = 0x%02x\n, s-cr_index, val);
+fprintf(stderr, vga: read CR%x = 0x%02x\n, s-cr_index, val);
 #endif
 break;
 case VGA_IS1_RM:
@@ -445,7 +445,7 @@ uint32_t vga_ioport_read(void *opaque, uint32_t addr)
 }
 }
 #if defined(DEBUG_VGA)
-printf(VGA: read addr=0x%04x data=0x%02x\n, addr, val);
+fprintf(stderr, VGA: read addr=0x%04x data=0x%02x\n, addr, val);
 #endif
 return val;
 }
@@ -462,7 +462,7 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t 
val)
 return;
 }
 #ifdef DEBUG_VGA
-printf(VGA: write addr=0x%04x data=0x%02x\n, addr, val);
+fprintf(stderr, VGA: write addr=0x%04x data=0x%02x\n, addr, val);
 #endif
 
 switch(addr) {
@@ -506,7 +506,7 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t 
val)
 break;
 case VGA_SEQ_D:
 #ifdef DEBUG_VGA_REG
-printf(vga: write SR%x = 0x%02x\n, s-sr_index, val);
+fprintf(stderr, vga: write SR%x = 0x%02x\n, s-sr_index, val);
 #endif
 s-sr[s-sr_index] = val  sr_mask[s-sr_index];
 if (s-sr_index == VGA_SEQ_CLOCK_MODE) {
@@ -537,7 +537,7 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t 
val)
 break;
 case VGA_GFX_D:
 #ifdef DEBUG_VGA_REG
-printf(vga: write GR%x = 0x%02x\n, s-gr_index, val);
+fprintf(stderr, vga: write GR%x = 0x%02x\n, s-gr_index, val);
 #endif
 s-gr[s-gr_index] = val  gr_mask[s-gr_index];
 vga_update_memory_access(s);
@@ -549,7 +549,7 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t 
val)
 case VGA_CRT_DM:
 case VGA_CRT_DC:
 #ifdef DEBUG_VGA_REG
-printf(vga: write CR%x = 0x%02x\n, s-cr_index, val);
+fprintf(stderr, vga: write CR%x = 0x%02x\n, s-cr_index, val);
 #endif
 /* handle CR0-7 protection */
 if ((s-cr[VGA_CRTC_V_SYNC_END]  VGA_CR11_LOCK_CR0_CR7) 
@@ -848,7 +848,7 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr, 
uint32_t val)
 uint32_t write_mask, bit_mask, set_mask;
 
 #ifdef DEBUG_VGA_MEM
-printf(vga: [0x TARGET_FMT_plx ] = 0x%02x\n, addr, val);
+fprintf(stderr, vga: [0x TARGET_FMT_plx ] = 0x%02x\n, addr, val);
 #endif
 /* convert to VGA memory offset */
 memory_map_mode = (s-gr[VGA_GFX_MISC]  2)  3;
@@ -881,7 +881,7 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr, 
uint32_t val)
 if (s-sr[VGA_SEQ_PLANE_WRITE]  mask) {
 s-vram_ptr[addr] = val;
 #ifdef DEBUG_VGA_MEM
-printf(vga: chain4: [0x TARGET_FMT_plx ]\n, addr);
+fprintf(stderr, vga: chain4: [0x TARGET_FMT_plx ]\n, addr);
 #endif
 s-plane_updated |= mask; /* only used to detect font change */
 memory_region_set_dirty(s-vram, addr, 1);
@@ -894,7 +894,7 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr, 
uint32_t val)
 addr = ((addr  ~1)  1) | plane;
 s-vram_ptr[addr] = val;
 #ifdef DEBUG_VGA_MEM
-printf(vga: odd/even: [0x TARGET_FMT_plx ]\n, addr);
+fprintf(stderr, vga: odd/even: [0x TARGET_FMT_plx ]\n, addr);
 #endif
 s-plane_updated |= mask; /* only used to detect font change */
 memory_region_set_dirty(s-vram, addr, 1);
@@ -969,7 +969,7 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr, 
uint32_t val)
 (((uint32_t *)s-vram_ptr)[addr]  ~write_mask) |
 (val  write_mask);
 #ifdef DEBUG_VGA_MEM
-printf(vga: latch: [0x TARGET_FMT_plx ] mask=0x%08x val=0x%08x\n,
+fprintf(stderr, vga: latch: [0x TARGET_FMT_plx ] mask=0x%08x 
val=0x%08x\n,
addr * 4, write_mask, val);
 #endif
 memory_region_set_dirty(s-vram, addr  2, sizeof(uint32_t));
-- 

Re: [Qemu-devel] [PATCH] [RFC] Xen PV backend: Move call to bdrv_new from blk_init to blk_connect

2013-04-02 Thread Stefano Stabellini
On Mon, 1 Apr 2013, Alex Bligh wrote:
 Stefano,
 
 --On 1 April 2013 16:44:05 +0100 Stefano Stabellini 
 stefano.stabell...@eu.citrix.com wrote:
 
  Note this patch is compile-tested only.
 
  I think the patch looks good, just a minor comment.
 
 Thanks. I guess I ought to actually test it works then :-)
 
  +/* fill info
  + * Temporarily write zero sectors as we won't know file size until
  + * bdrv_new has been called. blk_connect corrects this.
  + */
  +xenstore_write_be_int(blkdev-xendev, feature-flush-cache, 1);
  +xenstore_write_be_int(blkdev-xendev, feature-persistent, 1);
  +xenstore_write_be_int(blkdev-xendev, info, info);
  +xenstore_write_be_int(blkdev-xendev, sector-size, BLOCK_SIZE);
  +xenstore_write_be_int(blkdev-xendev, sectors, 0);
  +return 0;
 
  There is no need to fill the sector-size and sectors info here, you can
  do it later in blk_connect.
 
 My concern (not knowing how xenstore works) was the possibility of leaving
 them as uninitialized values. I'm taking it that if I don't initialise
 them, the key is just absent - correct?

Right


 I'll redo and move setting sector-size and sectors into blk_connect, and
 then add a second commit which removes BDRV_O_NOCACHE. Oh, and test it ...

Good :)



Re: [Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread

2013-04-02 Thread Paolo Bonzini
Il 02/04/2013 11:04, Peter Crosthwaite ha scritto:
 Public bug: 1154328
 Broken Commit: a29753f8aa79a34a324afebe340182a51a5aef11
 
 ATM, the timeout from g_pollfds_fill is inhibiting unlocking of the
 iothread. This is capable of causing a total deadlock when hw/serial
 is used as a device. The bug manifests when you go -nographic -serial
 mon:stdio and then paste 40 or more chars into the terminal.
 
 My knowledge of this g_foo is vague at best, but my best working
 theory is this:
 
 - First 8 chars are recieved by the serial device no complaints.
 - The next 32 chars, serial returns false for can_receive() so they
   are buffered by the MuxDriver object - mux_chr_read()
 - Buffer is full, so 41st char causes false return from Muxes own
   can_read()
 - This propagates all the way up to glib_pollfds_fill and manifests
   as a timeout

I suppose you mean manifests as timeout==0.  The question is *which*
GSource has a timeout of zero?  Not the mux's: if mux_chr_can_read()
returns zero, the prepare function returns FALSE without touching the
timeout at all...

static gboolean io_watch_poll_prepare(GSource *source, gint *timeout_)
{
IOWatchPoll *iwp = io_watch_poll_from_source(source);

iwp-max_size = iwp-fd_can_read(iwp-opaque);
if (iwp-max_size == 0) {
return FALSE;
}

return g_io_watch_funcs.prepare(source, timeout_);
}

 - Timeout means no unlock of IOthread. Device land never sees any more
   cycles so the serial port never progresses - no flushing of
   buffer

Still, this is plausible, so the patch looks correct.

Paolo

 - Deadlock
 
 Tested on petalogix_ml605 microblazeel machine model, which was faulty
 due to 1154328.
 
 Fix by removing the conditions on unlocking the iothread. Don't know
 what else this will break but the timeout is certainly the wrong
 condition for the unlock. Probably the real solution is to have a more
 selective unlock policy.
 
 I'm happy for someone to take this patch off my hands, or educate me on
 the correct implementation. For the peeps doing automated testing on
 nographic platforms this will get your build working again.
 
 Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
 ---
  main-loop.c |8 ++--
  1 files changed, 2 insertions(+), 6 deletions(-)
 
 diff --git a/main-loop.c b/main-loop.c
 index eb80ff3..a376898 100644
 --- a/main-loop.c
 +++ b/main-loop.c
 @@ -194,15 +194,11 @@ static int os_host_main_loop_wait(uint32_t timeout)
  
  glib_pollfds_fill(timeout);
  
 -if (timeout  0) {
 -qemu_mutex_unlock_iothread();
 -}
 +qemu_mutex_unlock_iothread();
  
  ret = g_poll((GPollFD *)gpollfds-data, gpollfds-len, timeout);
  
 -if (timeout  0) {
 -qemu_mutex_lock_iothread();
 -}
 +qemu_mutex_lock_iothread();
  
  glib_pollfds_poll();
  return ret;
 




Re: [Qemu-devel] [PATCH 13/16] IPMI: Add an external connection simulation interface

2013-04-02 Thread Zang Hongyong
What's the status of the IPMI patch? When will be merged into qemu?
With this patch, an external watchdog can be used for VM HA, even
through qemu is not healthy.
This is more attractive to qemu's own watchdog (ib700 or 6300esb).

On 2012/9/19 4:00, miny...@acm.org wrote:
 From: Corey Minyard cminy...@mvista.com

 This adds an interface for IPMI that connects to a remote
 BMC over a chardev (generally a TCP socket).  The OpenIPMI
 lanserv simulator describes this interface, see that for
 interface details.

 Signed-off-by: Corey Minyard cminy...@mvista.com
 ---
  default-configs/i386-softmmu.mak   |1 +
  default-configs/x86_64-softmmu.mak |1 +
  hw/Makefile.objs   |1 +
  hw/ipmi_extern.c   |  475 
 
  4 files changed, 478 insertions(+), 0 deletions(-)
  create mode 100644 hw/ipmi_extern.c

 diff --git a/default-configs/i386-softmmu.mak 
 b/default-configs/i386-softmmu.mak
 index 8c99d5d..325f92e 100644
 --- a/default-configs/i386-softmmu.mak
 +++ b/default-configs/i386-softmmu.mak
 @@ -12,6 +12,7 @@ CONFIG_ISA_IPMI=y
  CONFIG_IPMI_KCS=y
  CONFIG_IPMI_BT=y
  CONFIG_IPMI_LOCAL=y
 +CONFIG_IPMI_EXTERN=y
  CONFIG_SERIAL=y
  CONFIG_PARALLEL=y
  CONFIG_I8254=y
 diff --git a/default-configs/x86_64-softmmu.mak 
 b/default-configs/x86_64-softmmu.mak
 index 4d01883..2ac9177 100644
 --- a/default-configs/x86_64-softmmu.mak
 +++ b/default-configs/x86_64-softmmu.mak
 @@ -12,6 +12,7 @@ CONFIG_ISA_IPMI=y
  CONFIG_IPMI_KCS=y
  CONFIG_IPMI_BT=y
  CONFIG_IPMI_LOCAL=y
 +CONFIG_IPMI_EXTERN=y
  CONFIG_SERIAL=y
  CONFIG_PARALLEL=y
  CONFIG_I8254=y
 diff --git a/hw/Makefile.objs b/hw/Makefile.objs
 index 65f0ea1..a3f14ff 100644
 --- a/hw/Makefile.objs
 +++ b/hw/Makefile.objs
 @@ -25,6 +25,7 @@ hw-obj-$(CONFIG_ISA_IPMI) += isa_ipmi.o
  hw-obj-$(CONFIG_IPMI_KCS) += ipmi_kcs.o
  hw-obj-$(CONFIG_IPMI_BT) += ipmi_bt.o
  hw-obj-$(CONFIG_IPMI_LOCAL) += ipmi_sim.o
 +hw-obj-$(CONFIG_IPMI_EXTERN) += ipmi_extern.o
  
  hw-obj-$(CONFIG_SERIAL) += serial.o
  hw-obj-$(CONFIG_PARALLEL) += parallel.o
 diff --git a/hw/ipmi_extern.c b/hw/ipmi_extern.c
 new file mode 100644
 index 000..b31aea0
 --- /dev/null
 +++ b/hw/ipmi_extern.c
 @@ -0,0 +1,475 @@
 +/*
 + * IPMI BMC external connection
 + *
 + * Copyright (c) 2012 Corey Minyard, MontaVista Software, LLC
 + *
 + * Permission is hereby granted, free of charge, to any person obtaining a 
 copy
 + * of this software and associated documentation files (the Software), to 
 deal
 + * in the Software without restriction, including without limitation the 
 rights
 + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
 + * copies of the Software, and to permit persons to whom the Software is
 + * furnished to do so, subject to the following conditions:
 + *
 + * The above copyright notice and this permission notice shall be included in
 + * all copies or substantial portions of the Software.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
 + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
 FROM,
 + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 + * THE SOFTWARE.
 + */
 +
 +/*
 + * This is designed to connect with OpenIPMI's lanserv serial interface
 + * using the VM connection type.  See that for details.
 + */
 +
 +#include stdint.h
 +#include qemu-timer.h
 +#include qemu-char.h
 +#include ipmi.h
 +
 +#define VM_MSG_CHAR0xA0 /* Marks end of message */
 +#define VM_CMD_CHAR0xA1 /* Marks end of a command */
 +#define VM_ESCAPE_CHAR 0xAA /* Set bit 4 from the next byte to 0 */
 +
 +#define VM_PROTOCOL_VERSION1
 +#define VM_CMD_VERSION 0xff /* A version number byte follows */
 +#define VM_CMD_NOATTN  0x00
 +#define VM_CMD_ATTN0x01
 +#define VM_CMD_ATTN_IRQ0x02
 +#define VM_CMD_POWEROFF0x03
 +#define VM_CMD_RESET   0x04
 +#define VM_CMD_ENABLE_IRQ  0x05 /* Enable/disable the messaging irq 
 */
 +#define VM_CMD_DISABLE_IRQ 0x06
 +#define VM_CMD_SEND_NMI0x07
 +#define VM_CMD_CAPABILITIES0x08
 +#define   VM_CAPABILITIES_POWER0x01
 +#define   VM_CAPABILITIES_RESET0x02
 +#define   VM_CAPABILITIES_IRQ  0x04
 +#define   VM_CAPABILITIES_NMI  0x08
 +#define   VM_CAPABILITIES_ATTN 0x10
 +
 +#define IPMI_BMC_EXTERN(obj) OBJECT_CHECK(IPMIExternBmc, (obj), \
 +TYPE_IPMI_BMC_EXTERN)
 +typedef struct IPMIExternBmc {
 +IPMIBmc parent;
 +
 +int connected;
 +int is_listen;
 +
 +unsigned char inbuf[MAX_IPMI_MSG_SIZE + 2];
 +unsigned int inpos;
 +int in_escape;
 +int 

[Qemu-devel] [PATCH 0/2 V3] virtio-spec/net: dynamic network offloads configuration

2013-04-02 Thread Dmitry Fleytman
From: Dmitry Fleytman dfley...@redhat.com

V3 changes:
  1. Compat macro added
  2. Feature name beautification

V2 changes:
  1. _GUEST_ added to command and feature names
  2. Live migration logic fixed

Reported-by: Michael S. Tsirkin m...@redhat.com

One of recently introduced Windows features (RSC)
requires network driver to be able to enable and disable
HW LRO offload on the fly without device reinitialization.

Current Virtio specification doesn't support this requirement.
The solution proposed by following spec patch is to add
a new control command for this purpose.

The same solution may be used in Linux driver for ethtool interface
implementation.

Patch for QEMU mainline that implements this specification change
attached as well.

-- 
1.8.1.4




[Qemu-devel] [PATCH 1/2 V3] virtio-spec: dynamic network offloads configuration

2013-04-02 Thread Dmitry Fleytman
From: Dmitry Fleytman dfley...@redhat.com

Virtio-net driver currently negotiates network offloads
on startup via features mechanism and have no ability to
change offloads state later.
This patch introduced a new control command that allows
to configure device network offloads state dynamically.
The patch also introduces a new feature flag
VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.

Signed-off-by: Dmitry Fleytman dfley...@redhat.com
---
 virtio-spec.lyx | 143 
 1 file changed, 143 insertions(+)

diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index 3d2f485..fdba814 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -60,6 +60,7 @@
 \author -1930653948 Amos Kong 
 \author -608949062 Rusty Russell,,, 
 \author -385801441 Cornelia Huck cornelia.h...@de.ibm.com
+\author 460276516 Dmitry Fleytman dfley...@redhat.com
 \author 1112500848 Rusty Russell ru...@rustcorp.com.au
 \author 1531152142 Paolo Bonzini,,, 
 \author 1717892615 Alexey Zaytsev,,, 
@@ -4261,6 +4262,20 @@ VIRTIO_NET_F_GUEST_CSUM
 \end_inset
 
 (1) Guest handles packets with partial checksum
+\change_inserted 460276516 1363712169
+
+\end_layout
+
+\begin_layout Description
+
+\change_inserted 460276516 1363712334
+VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
+\begin_inset space ~
+\end_inset
+
+(2) Control channel offloads reconfiguration support.
+\change_unchanged
+
 \end_layout
 
 \begin_layout Description
@@ -5675,6 +5690,134 @@ virtqueue_pairs = 1
  
 \end_layout
 
+\begin_layout Subsection*
+
+\change_inserted 460276516 1363765850
+Offloads State Configuration
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 460276516 1363765861
+If the VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature is negotiated, the driver can
+ send control commands for dynamic offloads state configuration.
+\end_layout
+
+\begin_layout Subsubsection*
+
+\change_inserted 460276516 1363765928
+Setting offloads state
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 460276516 1363713225
+\begin_inset listings
+inline false
+status open
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363765996
+
+u32 offloads;
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363765997
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363766044
+
+#define VIRTIO_NET_OFFLOAD_GUEST_CSUM   1
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363766051
+
+#define VIRTIO_NET_OFFLOAD_GUEST_TSO4   2
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363766055
+
+#define VIRTIO_NET_OFFLOAD_GUEST_TSO6   4
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363766064
+
+#define VIRTIO_NET_OFFLOAD_GUEST_ECN8
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363766035
+
+#define VIRTIO_NET_OFFLOAD_GUEST_UFO   16
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363766031
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363765865
+
+#define VIRTIO_NET_CTRL_GUEST_OFFLOADS   5
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted 460276516 1363765867
+
+ #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET  0
+\end_layout
+
+\end_inset
+
+
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 460276516 1363766082
+The class VIRTIO_NET_CTRL_GUEST_OFFLOADS has one command: 
VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET
+ applies the new offloads configuration.
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 460276516 1363766435
+u32 value passed as command data is a bitmask, bits set define offloads
+ to be enabled, bits cleared - offloads to be disabled.
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 460276516 1363766757
+There is a corresponding device feature for each offload.
+ Upon feature negotiation corresponding offload gets enabled to preserve
+ backward compartibility.
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted 460276516 1363766720
+Corresponding feature must be negotiated at startup in order to allow dynamic
+ change of specific offload state.
+\end_layout
+
 \begin_layout Chapter*
 Appendix D: Block Device
 \end_layout
-- 
1.8.1.4




[Qemu-devel] [PATCH 2/2 V3] virtio-net: dynamic network offloads configuration

2013-04-02 Thread Dmitry Fleytman
From: Dmitry Fleytman dfley...@redhat.com

Virtio-net driver currently negotiates network offloads
on startup via features mechanism and have no ability to
change offloads state later.
This patch introduced a new control command that allows
to configure device network offloads state dynamically.
The patch also introduces a new feature flag
VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.

Signed-off-by: Dmitry Fleytman dfley...@redhat.com
---
 hw/pc.h |   4 ++
 hw/virtio-net.c | 111 
 hw/virtio-net.h |  19 ++
 3 files changed, 119 insertions(+), 15 deletions(-)

diff --git a/hw/pc.h b/hw/pc.h
index 8e1dd4c..7ca4698 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -221,6 +221,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
 .property = vectors,\
 /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\
 .value= stringify(0x),\
+},{ \
+.driver   = virtio-net-pci, \
+.property = ctrl_guest_offloads, \
+.value= off, \
 },{\
 .driver   = e1000,\
 .property = romfile,\
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 5917740..d040f96 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -360,6 +360,48 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
 return features;
 }
 
+static void virtio_net_apply_offloads(VirtIONet *n)
+{
+tap_set_offload(qemu_get_subqueue(n-nic, 0)-peer,
+!!(n-curr_offloads  VIRTIO_NET_OFFLOAD_GUEST_CSUM),
+!!(n-curr_offloads  VIRTIO_NET_OFFLOAD_GUEST_TSO4),
+!!(n-curr_offloads  VIRTIO_NET_OFFLOAD_GUEST_TSO6),
+!!(n-curr_offloads  VIRTIO_NET_OFFLOAD_GUEST_ECN),
+!!(n-curr_offloads  VIRTIO_NET_OFFLOAD_GUEST_UFO));
+}
+
+static uint32_t virtio_net_offloads_by_features(uint32_t features)
+{
+uint32_t offloads = 0;
+
+if ((1  VIRTIO_NET_F_GUEST_CSUM)  features) {
+offloads |= VIRTIO_NET_OFFLOAD_GUEST_CSUM;
+}
+
+if ((1  VIRTIO_NET_F_GUEST_TSO4)  features) {
+offloads |= VIRTIO_NET_OFFLOAD_GUEST_TSO4;
+}
+
+if ((1  VIRTIO_NET_F_GUEST_TSO6)  features) {
+offloads |= VIRTIO_NET_OFFLOAD_GUEST_TSO6;
+}
+
+if ((1  VIRTIO_NET_F_GUEST_ECN)  features) {
+offloads |= VIRTIO_NET_OFFLOAD_GUEST_ECN;
+}
+
+if ((1  VIRTIO_NET_F_GUEST_UFO)  features) {
+offloads |= VIRTIO_NET_OFFLOAD_GUEST_UFO;
+}
+
+return offloads;
+}
+
+static inline uint32_t virtio_net_supported_offloads(VirtIONet *n)
+{
+return virtio_net_offloads_by_features(n-vdev.guest_features);
+}
+
 static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
 {
 VirtIONet *n = to_virtio_net(vdev);
@@ -371,12 +413,8 @@ static void virtio_net_set_features(VirtIODevice *vdev, 
uint32_t features)
 virtio_net_set_mrg_rx_bufs(n, !!(features  (1  
VIRTIO_NET_F_MRG_RXBUF)));
 
 if (n-has_vnet_hdr) {
-tap_set_offload(qemu_get_subqueue(n-nic, 0)-peer,
-(features  VIRTIO_NET_F_GUEST_CSUM)  1,
-(features  VIRTIO_NET_F_GUEST_TSO4)  1,
-(features  VIRTIO_NET_F_GUEST_TSO6)  1,
-(features  VIRTIO_NET_F_GUEST_ECN)   1,
-(features  VIRTIO_NET_F_GUEST_UFO)   1);
+n-curr_offloads = virtio_net_offloads_by_features(features);
+virtio_net_apply_offloads(n);
 }
 
 for (i = 0;  i  n-max_queues; i++) {
@@ -422,6 +460,42 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t 
cmd,
 return VIRTIO_NET_OK;
 }
 
+static int virtio_net_handle_offloads(VirtIONet *n, uint8_t cmd,
+ struct iovec *iov, unsigned int iov_cnt)
+{
+uint32_t offloads;
+size_t s;
+
+if (!((1  VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)  n-vdev.guest_features)) {
+return VIRTIO_NET_ERR;
+}
+
+s = iov_to_buf(iov, iov_cnt, 0, offloads, sizeof(offloads));
+if (s != sizeof(offloads)) {
+return VIRTIO_NET_ERR;
+}
+
+if (cmd == VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET) {
+uint32_t supported_offloads;
+
+if (!n-has_vnet_hdr) {
+return VIRTIO_NET_ERR;
+}
+
+supported_offloads = virtio_net_supported_offloads(n);
+if (offloads  ~supported_offloads) {
+return VIRTIO_NET_ERR;
+}
+
+n-curr_offloads = offloads;
+virtio_net_apply_offloads(n);
+
+return VIRTIO_NET_OK;
+} else {
+return VIRTIO_NET_ERR;
+}
+}
+
 static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
  struct iovec *iov, unsigned int iov_cnt)
 {
@@ -591,6 +665,8 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, 
VirtQueue *vq)
 status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, iov_cnt);
 } else if (ctrl.class == VIRTIO_NET_CTRL_MQ) {

[Qemu-devel] [PATCH V11 03/17] qemu-img: remove unused parameter in collect_image_info()

2013-04-02 Thread Wenchao Xia
  Parameter *fmt was not used, so remove it.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Kevin Wolf kw...@redhat.com
---
 qemu-img.c |5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 31627b0..937ec01 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1640,8 +1640,7 @@ static void dump_json_image_info(ImageInfo *info)
 
 static void collect_image_info(BlockDriverState *bs,
ImageInfo *info,
-   const char *filename,
-   const char *fmt)
+   const char *filename)
 {
 uint64_t total_sectors;
 char backing_filename[1024];
@@ -1815,7 +1814,7 @@ static ImageInfoList *collect_image_info_list(const char 
*filename,
 }
 
 info = g_new0(ImageInfo, 1);
-collect_image_info(bs, info, filename, fmt);
+collect_image_info(bs, info, filename);
 collect_snapshots(bs, info);
 
 elem = g_new0(ImageInfoList, 1);
-- 
1.7.1





[Qemu-devel] [PATCH V11 01/17] block: move bdrv_snapshot_find() to block/snapshot.c

2013-04-02 Thread Wenchao Xia
  This patch adds block/snapshot.c and then moves the function
there. It also fixes small code style errors reported by check script.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Kevin Wolf kw...@redhat.com
---
 block/Makefile.objs  |1 +
 block/snapshot.c |   48 ++
 include/block/snapshot.h |   37 +++
 savevm.c |   23 +-
 4 files changed, 87 insertions(+), 22 deletions(-)
 create mode 100644 block/snapshot.c
 create mode 100644 include/block/snapshot.h

diff --git a/block/Makefile.objs b/block/Makefile.objs
index c067f38..60a4cd2 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -3,6 +3,7 @@ block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o 
qcow2-snapshot.o qcow2-c
 block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
 block-obj-y += qed-check.o
 block-obj-y += parallels.o blkdebug.o blkverify.o
+block-obj-y += snapshot.o
 block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
 block-obj-$(CONFIG_POSIX) += raw-posix.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
diff --git a/block/snapshot.c b/block/snapshot.c
new file mode 100644
index 000..c47a899
--- /dev/null
+++ b/block/snapshot.c
@@ -0,0 +1,48 @@
+/*
+ * Block layer snapshot related functions
+ *
+ * Copyright (c) 2003-2008 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 notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include block/snapshot.h
+
+int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
+   const char *name)
+{
+QEMUSnapshotInfo *sn_tab, *sn;
+int nb_sns, i, ret;
+
+ret = -ENOENT;
+nb_sns = bdrv_snapshot_list(bs, sn_tab);
+if (nb_sns  0) {
+return ret;
+}
+for (i = 0; i  nb_sns; i++) {
+sn = sn_tab[i];
+if (!strcmp(sn-id_str, name) || !strcmp(sn-name, name)) {
+*sn_info = *sn;
+ret = 0;
+break;
+}
+}
+g_free(sn_tab);
+return ret;
+}
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
new file mode 100644
index 000..4ad070c
--- /dev/null
+++ b/include/block/snapshot.h
@@ -0,0 +1,37 @@
+/*
+ * Block layer snapshot related functions
+ *
+ * Copyright (c) 2003-2008 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 notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef SNAPSHOT_H
+#define SNAPSHOT_H
+
+#include qemu-common.h
+/*
+ * block.h is needed for QEMUSnapshotInfo, it can be removed when define is
+ * moved here.
+ */
+#include block.h
+
+int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
+   const char *name);
+#endif
diff --git a/savevm.c b/savevm.c
index 406caa9..88acc38 100644
--- a/savevm.c
+++ b/savevm.c
@@ -40,6 +40,7 @@
 #include trace.h
 #include qemu/bitops.h
 #include qemu/iov.h
+#include 

[Qemu-devel] [PATCH V11 00/17] qmp/hmp interfaces for internal snapshot info

2013-04-02 Thread Wenchao Xia
  In the use of snapshot a way to retrieve related info at runtime is needed,
so this serial of patches will merge some code for qemu and qemu-img, and add
or enchance following interfaces for qemu:

1) qmp: query-block, show info for a block device include image info
Example:
- { execute: query-block }
- {
  return:[
 {
io-status: ok,
device:ide0-hd0,
locked:false,
removable:false,
inserted:{
   ro:false,
   drv:qcow2,
   encrypted:false,
   file:disks/test.img,
   backing_file_depth:1,
   bps:100,
   bps_rd:0,
   bps_wr:0,
   iops:100,
   iops_rd:0,
   iops_wr:0,
   image:{
  filename:disks/test.img,
  format:qcow2,
  virtual-size:2048000,
  backing_file:base.img,
  full-backing-filename:disks/base.img,
  backing-filename-format:qcow2,
  snapshots:[
 {
id: 1,
name: snapshot1,
vm-state-size: 0,
date-sec: 1200,
date-nsec: 12,
vm-clock-sec: 206,
vm-clock-nsec: 30
 }
  ],
  backing-image:{
  filename:disks/base.img,
  format:qcow2,
  virtual-size:2048000
  }
   }
},
type:unknown
 },
 {
io-status: ok,
device:ide1-cd0,
locked:false,
removable:true,
type:unknown
 },
 {
device:floppy0,
locked:false,
removable:true,
type:unknown
 },
 {
device:sd0,
locked:false,
removable:true,
type:unknown
 }
  ]
   }

2) qmp: query-snapshots, show avaiable vm snapshot info which can be used
in loadvm.
Example:
- { execute: query-snapshots }
- {
  return:[
 {
id: 1,
name: snapshot1,
vm-state-size: 0,
date-sec: 1200,
date-nsec: 12,
vm-clock-sec: 206,
vm-clock-nsec: 30
 }
  ]
}

3) hmp: info block, show what got in qmp query-block in monitor

  These patches follows the rule that use qmp to retieve information,
hmp layer just does a translation from qmp object it got.
  To make code graceful, snapshot and image info retrieving code in
qemu and qemu-img are merged into block layer, and some function name was
adjusted to make it tips better. For the part touch by the serial, it works as:

   qemu  qemu-img

dump_monitordump_stdout
 |--| 
|
   block/qapi.c

v2:
  Rename and adjusted qmp interface according to comments from Eric.
  Spelling fix.
  Information retrieving function in block layer goes to seperated patch.
  Free qmp object after usage in hmp.
  Added counterpart in qmp-commands.hx.
  Better tips in qmp-schema.json.

v3:
  Spelling fix in commit message, patch 03/11.
  Spelling fix in code, patch 06/11.
  Add comments that vm-state-size is in bytes, and change size of it in
example to a reasonable number, patch 08/11.

v4:
  02/13: in bdrv_get_filename(), add const to parameter *bs.
  03/13: new added, in which the function correct the behavior in info
retrieving.
  04/13: in bdrv_query_snapshot_infolist(), remove NULL check before call
err_setg(), added TODO comments that let block layer function set error instead
of this layer to tip better for errors, Split out patch about image info to
patch 05/13.
  05/13: new splitted, and it checks *bs by calling bdrv_can_read_snapshot()
before collect internal snasphot info to avoid *err is set unexpectly now.
  06/13: check if error happens after calling bdrv_query_image_info().
  08/13: rename info to image in DeviceImageInfo and make it optional,
when device is not inserted it will be empty, added error handling code
when met error in calling block layer API.
  09/13: distinguish *id and *name in bdrv_find_snapshots(), caller
can choose what to search with. id_wellformed() should be called in
new snapshot creation interface above this function in the future.
  10/13: now this interface have addtional parameter *device, which
enable showing internal snapshots on a single device. Also use
bdrv_can_read_snapshot() instead of bdrv_can_snapshot() now.
  11/13: this function goes to hmp.c so hmp_handler_error is not exported
any more, split out patch that switch snapshot info function to patch 12/13.
  12/13: new splitted.
  13/13: use qmp API instead of directly calling block layer API, now
all hmp function have correspond qmp funtion 

[Qemu-devel] [PATCH V11 02/17] block: distinguish id and name in bdrv_find_snapshot()

2013-04-02 Thread Wenchao Xia
  To make it clear about id and name in searching, the API is changed
a bit to distinguish them, and caller can choose to search by id or name.
Searching will be done with higher priority of id. This function also
returns negative value from bdrv_snapshot_list() instead of -ENOENT on
error now.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Kevin Wolf kw...@redhat.com
---
 block/snapshot.c |   46 ++
 include/block/snapshot.h |2 +-
 savevm.c |   10 +-
 3 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/block/snapshot.c b/block/snapshot.c
index c47a899..7b2026c 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -24,8 +24,20 @@
 
 #include block/snapshot.h
 
+/*
+ * Try find an internal snapshot with @id or @name, @id have higher priority
+ * in searching.
+ *
+ * @bs: block device to search on, must not be NULL.
+ * @sn_info: snapshot information to be filled in, must not be NULL.
+ * @id: snapshot id to search with, can be NULL.
+ * @name: snapshot name to search with, can be NULL.
+ *
+ * returns 0 and @sn_info is filled with related information if found,
+ * otherwise it returns negative value.
+ */
 int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
-   const char *name)
+   const char *id, const char *name)
 {
 QEMUSnapshotInfo *sn_tab, *sn;
 int nb_sns, i, ret;
@@ -33,16 +45,34 @@ int bdrv_snapshot_find(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info,
 ret = -ENOENT;
 nb_sns = bdrv_snapshot_list(bs, sn_tab);
 if (nb_sns  0) {
-return ret;
+return nb_sns;
 }
-for (i = 0; i  nb_sns; i++) {
-sn = sn_tab[i];
-if (!strcmp(sn-id_str, name) || !strcmp(sn-name, name)) {
-*sn_info = *sn;
-ret = 0;
-break;
+
+/* search by id */
+if (id) {
+for (i = 0; i  nb_sns; i++) {
+sn = sn_tab[i];
+if (!strcmp(sn-id_str, id)) {
+*sn_info = *sn;
+ret = 0;
+goto out;
+}
 }
 }
+
+/* search by name */
+if (name) {
+for (i = 0; i  nb_sns; i++) {
+sn = sn_tab[i];
+if (!strcmp(sn-name, name)) {
+*sn_info = *sn;
+ret = 0;
+goto out;
+}
+}
+}
+
+ out:
 g_free(sn_tab);
 return ret;
 }
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 4ad070c..a047a8e 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -33,5 +33,5 @@
 #include block.h
 
 int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
-   const char *name);
+   const char *id, const char *name);
 #endif
diff --git a/savevm.c b/savevm.c
index 88acc38..1d2da99 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2212,7 +2212,7 @@ static int del_existing_snapshots(Monitor *mon, const 
char *name)
 bs = NULL;
 while ((bs = bdrv_next(bs))) {
 if (bdrv_can_snapshot(bs) 
-bdrv_snapshot_find(bs, snapshot, name) = 0)
+bdrv_snapshot_find(bs, snapshot, name, name) = 0)
 {
 ret = bdrv_snapshot_delete(bs, name);
 if (ret  0) {
@@ -2272,7 +2272,7 @@ void do_savevm(Monitor *mon, const QDict *qdict)
 sn-vm_clock_nsec = qemu_get_clock_ns(vm_clock);
 
 if (name) {
-ret = bdrv_snapshot_find(bs, old_sn, name);
+ret = bdrv_snapshot_find(bs, old_sn, name, name);
 if (ret = 0) {
 pstrcpy(sn-name, sizeof(sn-name), old_sn-name);
 pstrcpy(sn-id_str, sizeof(sn-id_str), old_sn-id_str);
@@ -2363,7 +2363,7 @@ int load_vmstate(const char *name)
 }
 
 /* Don't even try to load empty VM states */
-ret = bdrv_snapshot_find(bs_vm_state, sn, name);
+ret = bdrv_snapshot_find(bs_vm_state, sn, name, name);
 if (ret  0) {
 return ret;
 } else if (sn.vm_state_size == 0) {
@@ -2387,7 +2387,7 @@ int load_vmstate(const char *name)
 return -ENOTSUP;
 }
 
-ret = bdrv_snapshot_find(bs, sn, name);
+ret = bdrv_snapshot_find(bs, sn, name, name);
 if (ret  0) {
 error_report(Device '%s' does not have the requested snapshot 
'%s',
bdrv_get_device_name(bs), name);
@@ -2493,7 +2493,7 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict)
 
 while ((bs1 = bdrv_next(bs1))) {
 if (bdrv_can_snapshot(bs1)  bs1 != bs) {
-ret = bdrv_snapshot_find(bs1, sn_info, sn-id_str);
+ret = bdrv_snapshot_find(bs1, sn_info, sn-id_str, NULL);
 if (ret  0) {
 available = 0;
 break;
-- 
1.7.1





[Qemu-devel] [PATCH V11 04/17] block: move collect_snapshots() and collect_image_info() to block/qapi.c

2013-04-02 Thread Wenchao Xia
  This patch adds block/qapi.c and moves the functions there. To avoid
conflict and tip better, macro in header file is BLOCK_QAPI_H instead
of QAPI_H. The moving is for making review easier, those functions
will be modified and renamed later.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Kevin Wolf kw...@redhat.com
---
 block/Makefile.objs  |2 +-
 block/qapi.c |  107 ++
 include/block/qapi.h |   35 
 qemu-img.c   |   86 +--
 4 files changed, 146 insertions(+), 84 deletions(-)
 create mode 100644 block/qapi.c
 create mode 100644 include/block/qapi.h

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 60a4cd2..fc27bed 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -3,7 +3,7 @@ block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o 
qcow2-snapshot.o qcow2-c
 block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
 block-obj-y += qed-check.o
 block-obj-y += parallels.o blkdebug.o blkverify.o
-block-obj-y += snapshot.o
+block-obj-y += snapshot.o qapi.o
 block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o
 block-obj-$(CONFIG_POSIX) += raw-posix.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
diff --git a/block/qapi.c b/block/qapi.c
new file mode 100644
index 000..e2b1b6b
--- /dev/null
+++ b/block/qapi.c
@@ -0,0 +1,107 @@
+/*
+ * Block layer qmp related functions
+ *
+ * Copyright (c) 2003-2008 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 notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include block/qapi.h
+#include block/block_int.h
+
+void bdrv_collect_snapshots(BlockDriverState *bs , ImageInfo *info)
+{
+int i, sn_count;
+QEMUSnapshotInfo *sn_tab = NULL;
+SnapshotInfoList *info_list, *cur_item = NULL;
+sn_count = bdrv_snapshot_list(bs, sn_tab);
+
+for (i = 0; i  sn_count; i++) {
+info-has_snapshots = true;
+info_list = g_new0(SnapshotInfoList, 1);
+
+info_list-value= g_new0(SnapshotInfo, 1);
+info_list-value-id= g_strdup(sn_tab[i].id_str);
+info_list-value-name  = g_strdup(sn_tab[i].name);
+info_list-value-vm_state_size = sn_tab[i].vm_state_size;
+info_list-value-date_sec  = sn_tab[i].date_sec;
+info_list-value-date_nsec = sn_tab[i].date_nsec;
+info_list-value-vm_clock_sec  = sn_tab[i].vm_clock_nsec / 10;
+info_list-value-vm_clock_nsec = sn_tab[i].vm_clock_nsec % 10;
+
+/* XXX: waiting for the qapi to support qemu-queue.h types */
+if (!cur_item) {
+info-snapshots = cur_item = info_list;
+} else {
+cur_item-next = info_list;
+cur_item = info_list;
+}
+
+}
+
+g_free(sn_tab);
+}
+
+void bdrv_collect_image_info(BlockDriverState *bs,
+ ImageInfo *info,
+ const char *filename)
+{
+uint64_t total_sectors;
+char backing_filename[1024];
+char backing_filename2[1024];
+BlockDriverInfo bdi;
+
+bdrv_get_geometry(bs, total_sectors);
+
+info-filename= g_strdup(filename);
+info-format  = g_strdup(bdrv_get_format_name(bs));
+info-virtual_size= total_sectors * 512;
+info-actual_size = bdrv_get_allocated_file_size(bs);
+info-has_actual_size = info-actual_size = 0;
+if (bdrv_is_encrypted(bs)) {
+info-encrypted = true;
+info-has_encrypted = true;
+}
+if (bdrv_get_info(bs, bdi) = 0) {
+if (bdi.cluster_size != 0) {
+info-cluster_size = bdi.cluster_size;
+info-has_cluster_size = true;
+}
+info-dirty_flag = bdi.is_dirty;
+info-has_dirty_flag = true;
+}
+bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
+if (backing_filename[0] != 

[Qemu-devel] [PATCH V11 05/17] block: add snapshot info query function bdrv_query_snapshot_info_list()

2013-04-02 Thread Wenchao Xia
  This patch adds function bdrv_query_snapshot_info_list(), which will
retrieve snapshot info of an image in qmp object format. The implementation
is based on the code moved from qemu-img.c with modification to fit more
for qmp based block layer API.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
 block/qapi.c |   55 ++---
 include/block/qapi.h |4 ++-
 qemu-img.c   |5 +++-
 3 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index e2b1b6b..03369a5 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -25,29 +25,56 @@
 #include block/qapi.h
 #include block/block_int.h
 
-void bdrv_collect_snapshots(BlockDriverState *bs , ImageInfo *info)
+/*
+ * return 0 on success, @p_list will be set only on success, and caller need to
+ * check *p_list on success.
+ */
+int bdrv_query_snapshot_info_list(BlockDriverState *bs,
+  SnapshotInfoList **p_list,
+  Error **errp)
 {
 int i, sn_count;
 QEMUSnapshotInfo *sn_tab = NULL;
-SnapshotInfoList *info_list, *cur_item = NULL;
+SnapshotInfoList *info_list, *cur_item = NULL, *head = NULL;
+SnapshotInfo *info;
+
 sn_count = bdrv_snapshot_list(bs, sn_tab);
+if (sn_count  0) {
+const char *dev = bdrv_get_device_name(bs);
+switch (sn_count) {
+case -ENOMEDIUM:
+error_setg(errp, Device '%s' is not inserted, dev);
+break;
+case -ENOTSUP:
+error_setg(errp,
+   Device '%s' does not support internal snapshots,
+   dev);
+break;
+default:
+error_setg_errno(errp, -sn_count,
+ Can't list snapshots of device '%s', dev);
+break;
+}
+return sn_count;
+}
 
 for (i = 0; i  sn_count; i++) {
-info-has_snapshots = true;
-info_list = g_new0(SnapshotInfoList, 1);
 
-info_list-value= g_new0(SnapshotInfo, 1);
-info_list-value-id= g_strdup(sn_tab[i].id_str);
-info_list-value-name  = g_strdup(sn_tab[i].name);
-info_list-value-vm_state_size = sn_tab[i].vm_state_size;
-info_list-value-date_sec  = sn_tab[i].date_sec;
-info_list-value-date_nsec = sn_tab[i].date_nsec;
-info_list-value-vm_clock_sec  = sn_tab[i].vm_clock_nsec / 10;
-info_list-value-vm_clock_nsec = sn_tab[i].vm_clock_nsec % 10;
+info = g_new0(SnapshotInfo, 1);
+info-id= g_strdup(sn_tab[i].id_str);
+info-name  = g_strdup(sn_tab[i].name);
+info-vm_state_size = sn_tab[i].vm_state_size;
+info-date_sec  = sn_tab[i].date_sec;
+info-date_nsec = sn_tab[i].date_nsec;
+info-vm_clock_sec  = sn_tab[i].vm_clock_nsec / 10;
+info-vm_clock_nsec = sn_tab[i].vm_clock_nsec % 10;
+
+info_list = g_new0(SnapshotInfoList, 1);
+info_list-value = info;
 
 /* XXX: waiting for the qapi to support qemu-queue.h types */
 if (!cur_item) {
-info-snapshots = cur_item = info_list;
+head = cur_item = info_list;
 } else {
 cur_item-next = info_list;
 cur_item = info_list;
@@ -56,6 +83,8 @@ void bdrv_collect_snapshots(BlockDriverState *bs , ImageInfo 
*info)
 }
 
 g_free(sn_tab);
+*p_list = head;
+return 0;
 }
 
 void bdrv_collect_image_info(BlockDriverState *bs,
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 4586578..91dc41b 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -28,7 +28,9 @@
 #include qapi-types.h
 #include block/block.h
 
-void bdrv_collect_snapshots(BlockDriverState *bs , ImageInfo *info);
+int bdrv_query_snapshot_info_list(BlockDriverState *bs,
+  SnapshotInfoList **p_list,
+  Error **errp);
 void bdrv_collect_image_info(BlockDriverState *bs,
  ImageInfo *info,
  const char *filename);
diff --git a/qemu-img.c b/qemu-img.c
index a020ccc..51043ef 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1735,7 +1735,10 @@ static ImageInfoList *collect_image_info_list(const char 
*filename,
 
 info = g_new0(ImageInfo, 1);
 bdrv_collect_image_info(bs, info, filename);
-bdrv_collect_snapshots(bs, info);
+if (!bdrv_query_snapshot_info_list(bs, info-snapshots, NULL) 
+info-snapshots) {
+info-has_snapshots = true;
+}
 
 elem = g_new0(ImageInfoList, 1);
 elem-value = info;
-- 
1.7.1





[Qemu-devel] [PATCH V11 07/17] block: add image info query function bdrv_query_image_info()

2013-04-02 Thread Wenchao Xia
  This patch adds function bdrv_query_image_info(), which will
retrieve image info in qmp object format. The implementation is
based on the code moved from qemu-img.c, but uses block layer
function to get snapshot info.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
 block/qapi.c |   41 ++---
 include/block/qapi.h |6 +++---
 qemu-img.c   |8 ++--
 3 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 19d4d93..176a479 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -122,18 +122,22 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
 return 0;
 }
 
-void bdrv_collect_image_info(BlockDriverState *bs,
- ImageInfo *info,
- const char *filename)
+/* return 0 on success, and @p_info will be set only on success. */
+int bdrv_query_image_info(BlockDriverState *bs,
+  ImageInfo **p_info,
+  Error **errp)
 {
 uint64_t total_sectors;
-char backing_filename[1024];
+const char *backing_filename;
 char backing_filename2[1024];
 BlockDriverInfo bdi;
+int ret;
+Error *err = NULL;
+ImageInfo *info = g_new0(ImageInfo, 1);
 
 bdrv_get_geometry(bs, total_sectors);
 
-info-filename= g_strdup(filename);
+info-filename= g_strdup(bs-filename);
 info-format  = g_strdup(bdrv_get_format_name(bs));
 info-virtual_size= total_sectors * 512;
 info-actual_size = bdrv_get_allocated_file_size(bs);
@@ -150,8 +154,8 @@ void bdrv_collect_image_info(BlockDriverState *bs,
 info-dirty_flag = bdi.is_dirty;
 info-has_dirty_flag = true;
 }
-bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
-if (backing_filename[0] != '\0') {
+backing_filename = bs-backing_file;
+if (backing_filename  backing_filename[0] != '\0') {
 info-backing_filename = g_strdup(backing_filename);
 info-has_backing_filename = true;
 bdrv_get_full_backing_filename(bs, backing_filename2,
@@ -168,4 +172,27 @@ void bdrv_collect_image_info(BlockDriverState *bs,
 info-has_backing_filename_format = true;
 }
 }
+
+ret = bdrv_query_snapshot_info_list(bs, info-snapshots, false, err);
+switch (ret) {
+case 0:
+if (info-snapshots) {
+info-has_snapshots = true;
+}
+break;
+/* recoverable error */
+case -ENOMEDIUM:
+error_free(err);
+break;
+case -ENOTSUP:
+error_free(err);
+break;
+default:
+error_propagate(errp, err);
+qapi_free_ImageInfo(info);
+return ret;
+}
+
+*p_info = info;
+return 0;
 }
diff --git a/include/block/qapi.h b/include/block/qapi.h
index fe66053..2c62fdf 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -32,7 +32,7 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
   SnapshotInfoList **p_list,
   bool vm_snapshot,
   Error **errp);
-void bdrv_collect_image_info(BlockDriverState *bs,
- ImageInfo *info,
- const char *filename);
+int bdrv_query_image_info(BlockDriverState *bs,
+  ImageInfo **p_info,
+  Error **errp);
 #endif
diff --git a/qemu-img.c b/qemu-img.c
index 261c277..1dd0a60 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1733,12 +1733,8 @@ static ImageInfoList *collect_image_info_list(const char 
*filename,
 goto err;
 }
 
-info = g_new0(ImageInfo, 1);
-bdrv_collect_image_info(bs, info, filename);
-if (!bdrv_query_snapshot_info_list(bs, info-snapshots,
-   false, NULL) 
-info-snapshots) {
-info-has_snapshots = true;
+if (bdrv_query_image_info(bs, info, NULL)) {
+goto err;
 }
 
 elem = g_new0(ImageInfoList, 1);
-- 
1.7.1





[Qemu-devel] [PATCH V11 08/17] block: move qmp_query_block() and bdrv_query_info() to block/qapi.c

2013-04-02 Thread Wenchao Xia
  This is a code move patch, except in qmp_query_block bdrv_next(bs)
is used instead of direct traverse of global array 'bdrv_states'.
  This patch also fix code style error reported by check script.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
Reviewed-by: Kevin Wolf kw...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
 block.c   |   76 
 block/qapi.c  |   77 +
 include/block/block.h |1 -
 include/block/qapi.h  |1 +
 4 files changed, 78 insertions(+), 77 deletions(-)

diff --git a/block.c b/block.c
index 0ae2e93..288cacd 100644
--- a/block.c
+++ b/block.c
@@ -3019,82 +3019,6 @@ int bdrv_is_allocated_above(BlockDriverState *top, 
BlockDriverState *base,
 return data.ret;
 }
 
-BlockInfo *bdrv_query_info(BlockDriverState *bs)
-{
-BlockInfo *info = g_malloc0(sizeof(*info));
-info-device = g_strdup(bs-device_name);
-info-type = g_strdup(unknown);
-info-locked = bdrv_dev_is_medium_locked(bs);
-info-removable = bdrv_dev_has_removable_media(bs);
-
-if (bdrv_dev_has_removable_media(bs)) {
-info-has_tray_open = true;
-info-tray_open = bdrv_dev_is_tray_open(bs);
-}
-
-if (bdrv_iostatus_is_enabled(bs)) {
-info-has_io_status = true;
-info-io_status = bs-iostatus;
-}
-
-if (bs-dirty_bitmap) {
-info-has_dirty = true;
-info-dirty = g_malloc0(sizeof(*info-dirty));
-info-dirty-count = bdrv_get_dirty_count(bs) * BDRV_SECTOR_SIZE;
-info-dirty-granularity =
-((int64_t) BDRV_SECTOR_SIZE  
hbitmap_granularity(bs-dirty_bitmap));
-}
-
-if (bs-drv) {
-info-has_inserted = true;
-info-inserted = g_malloc0(sizeof(*info-inserted));
-info-inserted-file = g_strdup(bs-filename);
-info-inserted-ro = bs-read_only;
-info-inserted-drv = g_strdup(bs-drv-format_name);
-info-inserted-encrypted = bs-encrypted;
-info-inserted-encryption_key_missing = bdrv_key_required(bs);
-
-if (bs-backing_file[0]) {
-info-inserted-has_backing_file = true;
-info-inserted-backing_file = g_strdup(bs-backing_file);
-}
-
-info-inserted-backing_file_depth = bdrv_get_backing_file_depth(bs);
-
-if (bs-io_limits_enabled) {
-info-inserted-bps =
-   bs-io_limits.bps[BLOCK_IO_LIMIT_TOTAL];
-info-inserted-bps_rd =
-   bs-io_limits.bps[BLOCK_IO_LIMIT_READ];
-info-inserted-bps_wr =
-   bs-io_limits.bps[BLOCK_IO_LIMIT_WRITE];
-info-inserted-iops =
-   bs-io_limits.iops[BLOCK_IO_LIMIT_TOTAL];
-info-inserted-iops_rd =
-   bs-io_limits.iops[BLOCK_IO_LIMIT_READ];
-info-inserted-iops_wr =
-   bs-io_limits.iops[BLOCK_IO_LIMIT_WRITE];
-}
-}
-return info;
-}
-
-BlockInfoList *qmp_query_block(Error **errp)
-{
-BlockInfoList *head = NULL, **p_next = head;
-BlockDriverState *bs;
-
-QTAILQ_FOREACH(bs, bdrv_states, list) {
-BlockInfoList *info = g_malloc0(sizeof(*info));
-info-value = bdrv_query_info(bs);
-
-*p_next = info;
-p_next = info-next;
-}
-
-return head;
-}
-
 BlockStats *bdrv_query_stats(const BlockDriverState *bs)
 {
 BlockStats *s;
diff --git a/block/qapi.c b/block/qapi.c
index 176a479..6e0c7c3 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -25,6 +25,7 @@
 #include block/qapi.h
 #include block/snapshot.h
 #include block/block_int.h
+#include qmp-commands.h
 
 /*
  * check whether the snapshot is valid for whole vm.
@@ -196,3 +197,79 @@ int bdrv_query_image_info(BlockDriverState *bs,
 *p_info = info;
 return 0;
 }
+
+BlockInfo *bdrv_query_info(BlockDriverState *bs)
+{
+BlockInfo *info = g_malloc0(sizeof(*info));
+info-device = g_strdup(bs-device_name);
+info-type = g_strdup(unknown);
+info-locked = bdrv_dev_is_medium_locked(bs);
+info-removable = bdrv_dev_has_removable_media(bs);
+
+if (bdrv_dev_has_removable_media(bs)) {
+info-has_tray_open = true;
+info-tray_open = bdrv_dev_is_tray_open(bs);
+}
+
+if (bdrv_iostatus_is_enabled(bs)) {
+info-has_io_status = true;
+info-io_status = bs-iostatus;
+}
+
+if (bs-dirty_bitmap) {
+info-has_dirty = true;
+info-dirty = g_malloc0(sizeof(*info-dirty));
+info-dirty-count = bdrv_get_dirty_count(bs) * BDRV_SECTOR_SIZE;
+info-dirty-granularity =
+ ((int64_t) BDRV_SECTOR_SIZE  hbitmap_granularity(bs-dirty_bitmap));
+}
+
+if (bs-drv) {
+info-has_inserted = true;
+info-inserted = g_malloc0(sizeof(*info-inserted));
+info-inserted-file = g_strdup(bs-filename);
+info-inserted-ro = bs-read_only;
+info-inserted-drv = 

[Qemu-devel] [PATCH V11 10/17] qmp: add recursive member in ImageInfo

2013-04-02 Thread Wenchao Xia
  New member *backing-image is added to reflect the backing chain
status.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
Reviewed-by: Kevin Wolf kw...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
 block/qapi.c |6 +-
 qapi-schema.json |5 -
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 5e91ab8..fa61c85 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -123,7 +123,11 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
 return 0;
 }
 
-/* return 0 on success, and @p_info will be set only on success. */
+/*
+ * return 0 on success, and @p_info will be set only on success,
+ * (*pinfo)-has_backing_image will be false and (*pinfo)-backing_image will
+ * be NULL.
+ */
 int bdrv_query_image_info(BlockDriverState *bs,
   ImageInfo **p_info,
   Error **errp)
diff --git a/qapi-schema.json b/qapi-schema.json
index 225afef..ad9dd82 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -233,6 +233,8 @@
 #
 # @snapshots: #optional list of VM snapshots
 #
+# @backing-image: #optional info of the backing image (since 1.5)
+#
 # Since: 1.3
 #
 ##
@@ -242,7 +244,8 @@
'*actual-size': 'int', 'virtual-size': 'int',
'*cluster-size': 'int', '*encrypted': 'bool',
'*backing-filename': 'str', '*full-backing-filename': 'str',
-   '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'] } 
}
+   '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'],
+   '*backing-image': 'ImageInfo' } }
 
 ##
 # @ImageCheck:
-- 
1.7.1





[Qemu-devel] [PATCH V11 06/17] block: add check for VM snapshot in bdrv_query_snapshot_info_list()

2013-04-02 Thread Wenchao Xia
  This patch adds a parameter to tell whether return valid snapshots
for whole VM only.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Kevin Wolf kw...@redhat.com
---
 block/qapi.c |   39 +--
 include/block/qapi.h |1 +
 qemu-img.c   |3 ++-
 3 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 03369a5..19d4d93 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -23,14 +23,47 @@
  */
 
 #include block/qapi.h
+#include block/snapshot.h
 #include block/block_int.h
 
 /*
+ * check whether the snapshot is valid for whole vm.
+ *
+ * @sn: snapshot info to be checked.
+ * @bs: where @sn was found.
+ *
+ * return true if the snapshot is consistent for the VM.
+ */
+static bool snapshot_valid_for_vm(const QEMUSnapshotInfo *sn,
+  BlockDriverState *bs)
+{
+BlockDriverState *bs1 = NULL;
+QEMUSnapshotInfo s, *sn_info = s;
+int ret;
+
+/* Check logic is connected with load_vmstate():
+   Only check the devices that can snapshot, other devices that can't
+   take snapshot, for example, readonly ones, will be ignored in
+   load_vmstate(). */
+while ((bs1 = bdrv_next(bs1))) {
+if (bs1 != bs  bdrv_can_snapshot(bs1)) {
+ret = bdrv_snapshot_find(bs1, sn_info, sn-id_str, NULL);
+if (ret  0) {
+return false;
+}
+}
+}
+return true;
+}
+
+/*
  * return 0 on success, @p_list will be set only on success, and caller need to
- * check *p_list on success.
+ * check *p_list on success. If @vm_snapshot is true, limit the results to
+ * snapshots valid for the whole VM.
  */
 int bdrv_query_snapshot_info_list(BlockDriverState *bs,
   SnapshotInfoList **p_list,
+  bool vm_snapshot,
   Error **errp)
 {
 int i, sn_count;
@@ -59,7 +92,9 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
 }
 
 for (i = 0; i  sn_count; i++) {
-
+if (vm_snapshot  !snapshot_valid_for_vm(sn_tab[i], bs)) {
+continue;
+}
 info = g_new0(SnapshotInfo, 1);
 info-id= g_strdup(sn_tab[i].id_str);
 info-name  = g_strdup(sn_tab[i].name);
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 91dc41b..fe66053 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -30,6 +30,7 @@
 
 int bdrv_query_snapshot_info_list(BlockDriverState *bs,
   SnapshotInfoList **p_list,
+  bool vm_snapshot,
   Error **errp);
 void bdrv_collect_image_info(BlockDriverState *bs,
  ImageInfo *info,
diff --git a/qemu-img.c b/qemu-img.c
index 51043ef..261c277 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1735,7 +1735,8 @@ static ImageInfoList *collect_image_info_list(const char 
*filename,
 
 info = g_new0(ImageInfo, 1);
 bdrv_collect_image_info(bs, info, filename);
-if (!bdrv_query_snapshot_info_list(bs, info-snapshots, NULL) 
+if (!bdrv_query_snapshot_info_list(bs, info-snapshots,
+   false, NULL) 
 info-snapshots) {
 info-has_snapshots = true;
 }
-- 
1.7.1





[Qemu-devel] [PATCH V11 11/17] qmp: add ImageInfo in BlockDeviceInfo used by query-block

2013-04-02 Thread Wenchao Xia
  Now image info will be retrieved as an embbed json object inside
BlockDeviceInfo, backing chain info and all related internal snapshot
info can be got in the enhanced recursive structure of ImageInfo.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
 block/qapi.c |   38 ++--
 include/block/qapi.h |4 ++-
 qapi-schema.json |5 +++-
 qmp-commands.hx  |   67 +-
 4 files changed, 108 insertions(+), 6 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index fa61c85..e98a028 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -202,9 +202,14 @@ int bdrv_query_image_info(BlockDriverState *bs,
 return 0;
 }
 
-BlockInfo *bdrv_query_info(BlockDriverState *bs)
+/* @p_info will be set only on success. */
+void bdrv_query_info(BlockDriverState *bs,
+ BlockInfo **p_info,
+ Error **errp)
 {
 BlockInfo *info = g_malloc0(sizeof(*info));
+BlockDriverState *bs0;
+ImageInfo **p_image_info;
 info-device = g_strdup(bs-device_name);
 info-type = g_strdup(unknown);
 info-locked = bdrv_dev_is_medium_locked(bs);
@@ -258,8 +263,28 @@ BlockInfo *bdrv_query_info(BlockDriverState *bs)
 info-inserted-iops_wr =
bs-io_limits.iops[BLOCK_IO_LIMIT_WRITE];
 }
+
+bs0 = bs;
+p_image_info = info-inserted-image;
+while (1) {
+if (bdrv_query_image_info(bs0, p_image_info, errp)) {
+goto err;
+}
+if (bs0-drv  bs0-backing_hd) {
+bs0 = bs0-backing_hd;
+(*p_image_info)-has_backing_image = true;
+p_image_info = ((*p_image_info)-backing_image);
+} else {
+break;
+}
+}
 }
-return info;
+
+*p_info = info;
+return;
+
+ err:
+qapi_free_BlockInfo(info);
 }
 
 SnapshotInfoList *qmp_query_snapshots(Error **errp)
@@ -286,11 +311,18 @@ BlockInfoList *qmp_query_block(Error **errp)
 
 while ((bs = bdrv_next(bs))) {
 BlockInfoList *info = g_malloc0(sizeof(*info));
-info-value = bdrv_query_info(bs);
+bdrv_query_info(bs, info-value, errp);
+if (error_is_set(errp)) {
+goto err;
+}
 
 *p_next = info;
 p_next = info-next;
 }
 
 return head;
+
+ err:
+qapi_free_BlockInfoList(head);
+return NULL;
 }
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 0039a70..e0fd0a5 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -35,5 +35,7 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
 int bdrv_query_image_info(BlockDriverState *bs,
   ImageInfo **p_info,
   Error **errp);
-BlockInfo *bdrv_query_info(BlockDriverState *bs);
+void bdrv_query_info(BlockDriverState *bs,
+ BlockInfo **p_info,
+ Error **errp);
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index ad9dd82..02dabc3 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -756,6 +756,8 @@
 #
 # @iops_wr: write I/O operations per second is specified
 #
+# @image: the info of image used (since: 1.5)
+#
 # Since: 0.14.0
 #
 # Notes: This interface is only found in @BlockInfo.
@@ -765,7 +767,8 @@
 '*backing_file': 'str', 'backing_file_depth': 'int',
 'encrypted': 'bool', 'encryption_key_missing': 'bool',
 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
-'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} }
+'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
+'image': 'ImageInfo' } }
 
 ##
 # @BlockDeviceIoStatus:
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 6b20684..b856be7 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1703,6 +1703,47 @@ Each json-object contain the following:
  - iops: limit total I/O operations per second (json-int)
  - iops_rd: limit read operations per second (json-int)
  - iops_wr: limit write operations per second (json-int)
+ - image: the detail of the image, it is a json-object containing
+the following:
+ - filename: image file name (json-string)
+ - format: image format (json-string)
+ - virtual-size: image capacity in bytes (json-int)
+ - dirty-flag: true if image is not cleanly closed, not present
+ means clean (json-bool, optional)
+ - actual-size: actual size on disk in bytes of the image, not
+  present when image does not support thin
+  provision (json-int, optional)
+ - cluster-size: size of a cluster in bytes, not present if image
+   format does not support it (json-int, optional)
+ - encrypted: true if the image is encrypted, not present means
+   

[Qemu-devel] [PATCH V11 12/17] block: move bdrv_snapshot_dump() and dump_human_image_info() to block/qapi.c

2013-04-02 Thread Wenchao Xia
  They are needed later in hmp command, dump_human_image_info()
is renamed to bdrv_image_info_dump().

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
 block.c   |   33 
 block/qapi.c  |  100 +
 include/block/block.h |1 -
 include/block/qapi.h  |2 +
 qemu-img.c|   69 +-
 savevm.c  |1 +
 6 files changed, 104 insertions(+), 102 deletions(-)

diff --git a/block.c b/block.c
index 288cacd..cd4bc7e 100644
--- a/block.c
+++ b/block.c
@@ -3433,39 +3433,6 @@ char *get_human_readable_size(char *buf, int buf_size, 
int64_t size)
 return buf;
 }
 
-char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn)
-{
-char buf1[128], date_buf[128], clock_buf[128];
-struct tm tm;
-time_t ti;
-int64_t secs;
-
-if (!sn) {
-snprintf(buf, buf_size,
- %-10s%-20s%7s%20s%15s,
- ID, TAG, VM SIZE, DATE, VM CLOCK);
-} else {
-ti = sn-date_sec;
-localtime_r(ti, tm);
-strftime(date_buf, sizeof(date_buf),
- %Y-%m-%d %H:%M:%S, tm);
-secs = sn-vm_clock_nsec / 10;
-snprintf(clock_buf, sizeof(clock_buf),
- %02d:%02d:%02d.%03d,
- (int)(secs / 3600),
- (int)((secs / 60) % 60),
- (int)(secs % 60),
- (int)((sn-vm_clock_nsec / 100) % 1000));
-snprintf(buf, buf_size,
- %-10s%-20s%7s%20s%15s,
- sn-id_str, sn-name,
- get_human_readable_size(buf1, sizeof(buf1), 
sn-vm_state_size),
- date_buf,
- clock_buf);
-}
-return buf;
-}
-
 /**/
 /* async I/Os */
 
diff --git a/block/qapi.c b/block/qapi.c
index e98a028..df63c97 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -326,3 +326,103 @@ BlockInfoList *qmp_query_block(Error **errp)
 qapi_free_BlockInfoList(head);
 return NULL;
 }
+
+char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn)
+{
+char buf1[128], date_buf[128], clock_buf[128];
+struct tm tm;
+time_t ti;
+int64_t secs;
+
+if (!sn) {
+snprintf(buf, buf_size,
+ %-10s%-20s%7s%20s%15s,
+ ID, TAG, VM SIZE, DATE, VM CLOCK);
+} else {
+ti = sn-date_sec;
+localtime_r(ti, tm);
+strftime(date_buf, sizeof(date_buf),
+ %Y-%m-%d %H:%M:%S, tm);
+secs = sn-vm_clock_nsec / 10;
+snprintf(clock_buf, sizeof(clock_buf),
+ %02d:%02d:%02d.%03d,
+ (int)(secs / 3600),
+ (int)((secs / 60) % 60),
+ (int)(secs % 60),
+ (int)((sn-vm_clock_nsec / 100) % 1000));
+snprintf(buf, buf_size,
+ %-10s%-20s%7s%20s%15s,
+ sn-id_str, sn-name,
+ get_human_readable_size(buf1, sizeof(buf1), 
sn-vm_state_size),
+ date_buf,
+ clock_buf);
+}
+return buf;
+}
+
+void bdrv_image_info_dump(ImageInfo *info)
+{
+char size_buf[128], dsize_buf[128];
+if (!info-has_actual_size) {
+snprintf(dsize_buf, sizeof(dsize_buf), unavailable);
+} else {
+get_human_readable_size(dsize_buf, sizeof(dsize_buf),
+info-actual_size);
+}
+get_human_readable_size(size_buf, sizeof(size_buf), info-virtual_size);
+printf(image: %s\n
+   file format: %s\n
+   virtual size: %s (% PRId64  bytes)\n
+   disk size: %s\n,
+   info-filename, info-format, size_buf,
+   info-virtual_size,
+   dsize_buf);
+
+if (info-has_encrypted  info-encrypted) {
+printf(encrypted: yes\n);
+}
+
+if (info-has_cluster_size) {
+printf(cluster_size: % PRId64 \n, info-cluster_size);
+}
+
+if (info-has_dirty_flag  info-dirty_flag) {
+printf(cleanly shut down: no\n);
+}
+
+if (info-has_backing_filename) {
+printf(backing file: %s, info-backing_filename);
+if (info-has_full_backing_filename) {
+printf( (actual path: %s), info-full_backing_filename);
+}
+putchar('\n');
+if (info-has_backing_filename_format) {
+printf(backing file format: %s\n, info-backing_filename_format);
+}
+}
+
+if (info-has_snapshots) {
+SnapshotInfoList *elem;
+char buf[256];
+
+printf(Snapshot list:\n);
+printf(%s\n, bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+
+/* Ideally bdrv_snapshot_dump() would operate on SnapshotInfoList but
+ * we convert to the block layer's native QEMUSnapshotInfo for now.
+ */
+for (elem = info-snapshots; elem; elem = elem-next) {
+QEMUSnapshotInfo sn = {
+

[Qemu-devel] [PATCH V11 17/17] hmp: add parameters device and -v for info block

2013-04-02 Thread Wenchao Xia
  With these parameters, user can choose the information to be showed,
to avoid message flood in the montior.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
 hmp.c |   34 --
 monitor.c |7 ---
 2 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/hmp.c b/hmp.c
index 6f93fcd..677bc7f 100644
--- a/hmp.c
+++ b/hmp.c
@@ -280,10 +280,15 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
 BlockInfoList *block_list, *info;
 ImageInfo *image_info;
 GString *buf = NULL;
+const char *device = qdict_get_try_str(qdict, device);
+int verbose = qdict_get_try_bool(qdict, verbose, 0);
 
 block_list = qmp_query_block(NULL);
 
 for (info = block_list; info; info = info-next) {
+if (device  strcmp(device, info-value-device)) {
+continue;
+}
 monitor_printf(mon, %s: removable=%d,
info-value-device, info-value-removable);
 
@@ -322,22 +327,23 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
 info-value-inserted-iops_rd,
 info-value-inserted-iops_wr);
 
-if (!buf) {
-buf = g_string_sized_new(2048);
-}
-monitor_printf(mon,  images:\n);
-image_info = info-value-inserted-image;
-while (1) {
-bdrv_image_info_dump(buf, image_info);
-monitor_printf(mon, %s, buf-str);
-g_string_set_size(buf, 0);
-if (image_info-has_backing_image) {
-image_info = image_info-backing_image;
-} else {
-break;
+if (verbose) {
+if (!buf) {
+buf = g_string_sized_new(2048);
+}
+monitor_printf(mon,  images:\n);
+image_info = info-value-inserted-image;
+while (1) {
+bdrv_image_info_dump(buf, image_info);
+monitor_printf(mon, %s, buf-str);
+g_string_set_size(buf, 0);
+if (image_info-has_backing_image) {
+image_info = image_info-backing_image;
+} else {
+break;
+}
 }
 }
-
 } else {
 monitor_printf(mon,  [not inserted]);
 }
diff --git a/monitor.c b/monitor.c
index 4720207..ae5a61e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2465,9 +2465,10 @@ static mon_cmd_t info_cmds[] = {
 },
 {
 .name   = block,
-.args_type  = ,
-.params = ,
-.help   = show the block devices,
+.args_type  = verbose:-v,device:B?,
+.params = [-v] [device],
+.help   = show info of one block device or all block devices 
+  (and details of images with -v option),
 .mhandler.cmd = hmp_info_block,
 },
 {
-- 
1.7.1





[Qemu-devel] [PATCH V11 14/17] hmp: add function hmp_info_snapshots()

2013-04-02 Thread Wenchao Xia
  This function will simply call qmp interface qmp_query_snapshots()
added in last commit and then dump information in monitor console.
  To get snapshot info, Now qemu and qemu-img both call block layer
function bdrv_query_snapshot_info_list() in their calling path, and
then they just translate the qmp object got to strings in stdout or
monitor console.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
 hmp.c |   48 
 hmp.h |1 +
 2 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/hmp.c b/hmp.c
index dbe9b90..89e1aaf 100644
--- a/hmp.c
+++ b/hmp.c
@@ -22,6 +22,7 @@
 #include qemu/sockets.h
 #include monitor/monitor.h
 #include ui/console.h
+#include block/qapi.h
 
 static void hmp_handle_error(Monitor *mon, Error **errp)
 {
@@ -653,6 +654,53 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
 qapi_free_TPMInfoList(info_list);
 }
 
+/* assume list is valid */
+static void monitor_dump_snapshotinfolist(Monitor *mon, SnapshotInfoList *list)
+{
+SnapshotInfoList *elem;
+GString *buf = g_string_sized_new(1024);
+
+bdrv_snapshot_dump(buf, NULL);
+g_string_append_printf(buf, \n);
+
+for (elem = list; elem; elem = elem-next) {
+QEMUSnapshotInfo sn = {
+.vm_state_size = elem-value-vm_state_size,
+.date_sec = elem-value-date_sec,
+.date_nsec = elem-value-date_nsec,
+.vm_clock_nsec = elem-value-vm_clock_sec * 10ULL +
+ elem-value-vm_clock_nsec,
+};
+pstrcpy(sn.id_str, sizeof(sn.id_str), elem-value-id);
+pstrcpy(sn.name, sizeof(sn.name), elem-value-name);
+bdrv_snapshot_dump(buf, sn);
+g_string_append_printf(buf, \n);
+}
+
+monitor_printf(mon, %s, buf-str);
+g_string_free(buf, true);
+}
+
+void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+SnapshotInfoList *list;
+
+list = qmp_query_snapshots(err);
+if (error_is_set(err)) {
+hmp_handle_error(mon, err);
+return;
+}
+
+if (list == NULL) {
+monitor_printf(mon, There is no suitable snapshot available\n);
+return;
+}
+
+monitor_dump_snapshotinfolist(mon, list);
+qapi_free_SnapshotInfoList(list);
+}
+
 void hmp_quit(Monitor *mon, const QDict *qdict)
 {
 monitor_suspend(mon);
diff --git a/hmp.h b/hmp.h
index 80e8b41..1172c47 100644
--- a/hmp.h
+++ b/hmp.h
@@ -37,6 +37,7 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict);
 void hmp_info_pci(Monitor *mon, const QDict *qdict);
 void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
 void hmp_info_tpm(Monitor *mon, const QDict *qdict);
+void hmp_info_snapshots(Monitor *mon, const QDict *qdict);
 void hmp_quit(Monitor *mon, const QDict *qdict);
 void hmp_stop(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
-- 
1.7.1





[Qemu-devel] [PATCH V11 13/17] block: dump to buffer for bdrv_snapshot_dump() and bdrv_image_info_dump()

2013-04-02 Thread Wenchao Xia
  This patch would allow hmp use them just like qemu-img, and avoid
string truncation.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
 block/qapi.c |   65 +++---
 include/block/qapi.h |4 +-
 qemu-img.c   |   22 
 savevm.c |   11 ++--
 4 files changed, 61 insertions(+), 41 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index df63c97..e27519e 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -327,7 +327,7 @@ BlockInfoList *qmp_query_block(Error **errp)
 return NULL;
 }
 
-char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn)
+void bdrv_snapshot_dump(GString *buf, QEMUSnapshotInfo *sn)
 {
 char buf1[128], date_buf[128], clock_buf[128];
 struct tm tm;
@@ -335,9 +335,8 @@ char *bdrv_snapshot_dump(char *buf, int buf_size, 
QEMUSnapshotInfo *sn)
 int64_t secs;
 
 if (!sn) {
-snprintf(buf, buf_size,
- %-10s%-20s%7s%20s%15s,
- ID, TAG, VM SIZE, DATE, VM CLOCK);
+g_string_append_printf(buf, %-10s%-20s%7s%20s%15s,
+   ID, TAG, VM SIZE, DATE, VM CLOCK);
 } else {
 ti = sn-date_sec;
 localtime_r(ti, tm);
@@ -350,17 +349,17 @@ char *bdrv_snapshot_dump(char *buf, int buf_size, 
QEMUSnapshotInfo *sn)
  (int)((secs / 60) % 60),
  (int)(secs % 60),
  (int)((sn-vm_clock_nsec / 100) % 1000));
-snprintf(buf, buf_size,
- %-10s%-20s%7s%20s%15s,
- sn-id_str, sn-name,
- get_human_readable_size(buf1, sizeof(buf1), 
sn-vm_state_size),
- date_buf,
- clock_buf);
+g_string_append_printf(buf,
+   %-10s%-20s%7s%20s%15s,
+   sn-id_str, sn-name,
+   get_human_readable_size(buf1, sizeof(buf1),
+   sn-vm_state_size),
+   date_buf,
+   clock_buf);
 }
-return buf;
 }
 
-void bdrv_image_info_dump(ImageInfo *info)
+void bdrv_image_info_dump(GString *buf, ImageInfo *info)
 {
 char size_buf[128], dsize_buf[128];
 if (!info-has_actual_size) {
@@ -370,43 +369,48 @@ void bdrv_image_info_dump(ImageInfo *info)
 info-actual_size);
 }
 get_human_readable_size(size_buf, sizeof(size_buf), info-virtual_size);
-printf(image: %s\n
-   file format: %s\n
-   virtual size: %s (% PRId64  bytes)\n
-   disk size: %s\n,
-   info-filename, info-format, size_buf,
-   info-virtual_size,
-   dsize_buf);
+g_string_append_printf(buf,
+   image: %s\n
+   file format: %s\n
+   virtual size: %s (% PRId64  bytes)\n
+   disk size: %s\n,
+   info-filename, info-format, size_buf,
+   info-virtual_size,
+   dsize_buf);
 
 if (info-has_encrypted  info-encrypted) {
-printf(encrypted: yes\n);
+g_string_append_printf(buf, encrypted: yes\n);
 }
 
 if (info-has_cluster_size) {
-printf(cluster_size: % PRId64 \n, info-cluster_size);
+g_string_append_printf(buf, cluster_size: % PRId64 \n,
+   info-cluster_size);
 }
 
 if (info-has_dirty_flag  info-dirty_flag) {
-printf(cleanly shut down: no\n);
+g_string_append_printf(buf, cleanly shut down: no\n);
 }
 
 if (info-has_backing_filename) {
-printf(backing file: %s, info-backing_filename);
+g_string_append_printf(buf, backing file: %s,
+   info-backing_filename);
 if (info-has_full_backing_filename) {
-printf( (actual path: %s), info-full_backing_filename);
+g_string_append_printf(buf,  (actual path: %s),
+   info-full_backing_filename);
 }
-putchar('\n');
+g_string_append_printf(buf, \n);
 if (info-has_backing_filename_format) {
-printf(backing file format: %s\n, info-backing_filename_format);
+g_string_append_printf(buf, backing file format: %s\n,
+   info-backing_filename_format);
 }
 }
 
 if (info-has_snapshots) {
 SnapshotInfoList *elem;
-char buf[256];
 
-printf(Snapshot list:\n);
-printf(%s\n, bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+g_string_append_printf(buf, Snapshot list:\n);
+bdrv_snapshot_dump(buf, NULL);
+g_string_append_printf(buf, \n);
 
 /* Ideally bdrv_snapshot_dump() would operate on SnapshotInfoList but
  * we convert to the block layer's native QEMUSnapshotInfo for now.
@@ -422,7 

[Qemu-devel] [PATCH V11 15/17] hmp: switch snapshot info function to qmp based one

2013-04-02 Thread Wenchao Xia
  This patch using new added function in last commit which retrieve
info from qmp for snapshot info.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
Reviewed-by: Kevin Wolf kw...@redhat.com
Reviewed-by: Eric Blake ebl...@redhat.com
---
 monitor.c |2 +-
 savevm.c  |   69 -
 2 files changed, 1 insertions(+), 70 deletions(-)

diff --git a/monitor.c b/monitor.c
index 4ec1db9..4720207 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2621,7 +2621,7 @@ static mon_cmd_t info_cmds[] = {
 .args_type  = ,
 .params = ,
 .help   = show the currently saved VM snapshots,
-.mhandler.cmd = do_info_snapshots,
+.mhandler.cmd = hmp_info_snapshots,
 },
 {
 .name   = status,
diff --git a/savevm.c b/savevm.c
index ce0bbe1..ca1e393 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2459,75 +2459,6 @@ void do_delvm(Monitor *mon, const QDict *qdict)
 }
 }
 
-void do_info_snapshots(Monitor *mon, const QDict *qdict)
-{
-BlockDriverState *bs, *bs1;
-QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = s;
-int nb_sns, i, ret, available;
-int total;
-int *available_snapshots;
-GString *buf = NULL;
-
-bs = bdrv_snapshots();
-if (!bs) {
-monitor_printf(mon, No available block device supports snapshots\n);
-return;
-}
-
-nb_sns = bdrv_snapshot_list(bs, sn_tab);
-if (nb_sns  0) {
-monitor_printf(mon, bdrv_snapshot_list: error %d\n, nb_sns);
-return;
-}
-
-if (nb_sns == 0) {
-monitor_printf(mon, There is no snapshot available.\n);
-return;
-}
-
-available_snapshots = g_malloc0(sizeof(int) * nb_sns);
-total = 0;
-for (i = 0; i  nb_sns; i++) {
-sn = sn_tab[i];
-available = 1;
-bs1 = NULL;
-
-while ((bs1 = bdrv_next(bs1))) {
-if (bdrv_can_snapshot(bs1)  bs1 != bs) {
-ret = bdrv_snapshot_find(bs1, sn_info, sn-id_str, NULL);
-if (ret  0) {
-available = 0;
-break;
-}
-}
-}
-
-if (available) {
-available_snapshots[total] = i;
-total++;
-}
-}
-
-if (total  0) {
-buf = g_string_new(NULL);
-bdrv_snapshot_dump(buf, NULL);
-g_string_append_printf(buf, \n);
-for (i = 0; i  total; i++) {
-sn = sn_tab[available_snapshots[i]];
-bdrv_snapshot_dump(buf, sn);
-g_string_append_printf(buf, \n);
-}
-monitor_printf(mon, %s, buf-str);
-g_string_free(buf, true);
-} else {
-monitor_printf(mon, There is no suitable snapshot available\n);
-}
-
-g_free(sn_tab);
-g_free(available_snapshots);
-
-}
-
 void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
 {
 qemu_ram_set_idstr(memory_region_get_ram_addr(mr)  TARGET_PAGE_MASK,
-- 
1.7.1





[Qemu-devel] [PATCH V11 16/17] hmp: show ImageInfo in 'info block'

2013-04-02 Thread Wenchao Xia
  Now human monitor can show image details include internal
snapshot info for every block device.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
 hmp.c |   22 ++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/hmp.c b/hmp.c
index 89e1aaf..6f93fcd 100644
--- a/hmp.c
+++ b/hmp.c
@@ -278,6 +278,8 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
 void hmp_info_block(Monitor *mon, const QDict *qdict)
 {
 BlockInfoList *block_list, *info;
+ImageInfo *image_info;
+GString *buf = NULL;
 
 block_list = qmp_query_block(NULL);
 
@@ -319,6 +321,23 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
 info-value-inserted-iops,
 info-value-inserted-iops_rd,
 info-value-inserted-iops_wr);
+
+if (!buf) {
+buf = g_string_sized_new(2048);
+}
+monitor_printf(mon,  images:\n);
+image_info = info-value-inserted-image;
+while (1) {
+bdrv_image_info_dump(buf, image_info);
+monitor_printf(mon, %s, buf-str);
+g_string_set_size(buf, 0);
+if (image_info-has_backing_image) {
+image_info = image_info-backing_image;
+} else {
+break;
+}
+}
+
 } else {
 monitor_printf(mon,  [not inserted]);
 }
@@ -326,6 +345,9 @@ void hmp_info_block(Monitor *mon, const QDict *qdict)
 monitor_printf(mon, \n);
 }
 
+if (buf) {
+g_string_free(buf, true);
+}
 qapi_free_BlockInfoList(block_list);
 }
 
-- 
1.7.1





Re: [Qemu-devel] [PATCH 0/2 V3] virtio-spec/net: dynamic network offloads configuration

2013-04-02 Thread Michael S. Tsirkin
On Tue, Apr 02, 2013 at 02:38:58PM +0300, Dmitry Fleytman wrote:
 From: Dmitry Fleytman dfley...@redhat.com
 
 V3 changes:
   1. Compat macro added
   2. Feature name beautification
 
 V2 changes:
   1. _GUEST_ added to command and feature names
   2. Live migration logic fixed
 
 Reported-by: Michael S. Tsirkin m...@redhat.com

Acked-by: Michael S. Tsirkin m...@redhat.com

Rusty?

 One of recently introduced Windows features (RSC)
 requires network driver to be able to enable and disable
 HW LRO offload on the fly without device reinitialization.
 
 Current Virtio specification doesn't support this requirement.
 The solution proposed by following spec patch is to add
 a new control command for this purpose.
 
 The same solution may be used in Linux driver for ethtool interface
 implementation.
 
 Patch for QEMU mainline that implements this specification change
 attached as well.
 
 -- 
 1.8.1.4



Re: [Qemu-devel] [PATCH] vga.c: Debug messages go to stderr

2013-04-02 Thread Gerd Hoffmann
On 04/02/13 12:53, Alex DAMIAN wrote:
 From: Alexandru DAMIAN alexandru.dam...@intel.com
 
 Move debug messages to stderr since often in vga code debug
 stdio is used as serial console.

Make them tracepoints would be even better ...

cheers,
  Gerd





[Qemu-devel] [PATCH V11 09/17] qmp: add interface query-snapshots

2013-04-02 Thread Wenchao Xia
  This interface returns info of valid internal snapshots for whole vm.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
Reviewed-by: Eric Blake ebl...@redhat.com
Reviewed-by: Kevin Wolf kw...@redhat.com
---
 block/qapi.c |   17 
 qapi-schema.json |   14 +
 qmp-commands.hx  |   55 ++
 3 files changed, 86 insertions(+), 0 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 6e0c7c3..5e91ab8 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -258,6 +258,23 @@ BlockInfo *bdrv_query_info(BlockDriverState *bs)
 return info;
 }
 
+SnapshotInfoList *qmp_query_snapshots(Error **errp)
+{
+BlockDriverState *bs;
+SnapshotInfoList *list = NULL;
+
+/* internal snapshot for whole vm */
+bs = bdrv_snapshots();
+if (!bs) {
+error_setg(errp, No available block device supports snapshots\n);
+return NULL;
+}
+
+bdrv_query_snapshot_info_list(bs, list, true, errp);
+
+return list;
+}
+
 BlockInfoList *qmp_query_block(Error **errp)
 {
 BlockInfoList *head = NULL, **p_next = head;
diff --git a/qapi-schema.json b/qapi-schema.json
index f629a24..225afef 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -841,6 +841,20 @@
 { 'command': 'query-block', 'returns': ['BlockInfo'] }
 
 ##
+# @query-snapshots:
+#
+# Get a list of internal snapshots for the whole virtual machine. Only valid
+# internal snapshots will be returned, inconsistent ones will be ignored
+#
+# Returns: a list of @SnapshotInfo describing all consistent virtual machine
+#  snapshots
+#
+# Since: 1.5
+##
+{ 'command': 'query-snapshots',
+  'returns': ['SnapshotInfo'] }
+
+##
 # @BlockDeviceStats:
 #
 # Statistics of a virtual block device or a block backing device.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 1e0e11e..6b20684 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1765,6 +1765,61 @@ EQMP
 },
 
 SQMP
+query-snapshots
+---
+
+Show the internal consistent snapshot information
+
+Each snapshot is represented by a json-object. The returned value
+is a json-array of all snapshots
+
+Each json-object contain the following:
+
+- id: unique snapshot id (json-string)
+- name: internal snapshot name (json-string)
+- vm-state-size: size of the VM state in bytes (json-int)
+- date-sec: UTC date of the snapshot in seconds (json-int)
+- date-nsec: fractional part in nanoseconds to be used with
+   date-sec(json-int)
+- vm-clock-sec: VM clock relative to boot in seconds (json-int)
+- vm-clock-nsec: fractional part in nanoseconds to be used with
+   vm-clock-sec (json-int)
+
+Example:
+
+- { execute: query-snapshots }
+- {
+  return:[
+ {
+id: 1,
+name: snapshot1,
+vm-state-size: 0,
+date-sec: 1200,
+date-nsec: 12,
+vm-clock-sec: 206,
+vm-clock-nsec: 30
+ },
+ {
+id: 2,
+name: snapshot2,
+vm-state-size: 3400,
+date-sec: 13000200,
+date-nsec: 32,
+vm-clock-sec: 406,
+vm-clock-nsec: 31
+ }
+  ]
+   }
+
+EQMP
+
+{
+.name   = query-snapshots,
+.args_type  = ,
+.mhandler.cmd_new = qmp_marshal_input_query_snapshots,
+},
+
+SQMP
 query-blockstats
 
 
-- 
1.7.1





Re: [Qemu-devel] [PATCH-v2 0/3] virtio/vhost: Add checks for uninitialized VQs

2013-04-02 Thread Michael S. Tsirkin
On Mon, Apr 01, 2013 at 11:58:21PM +, Nicholas A. Bellinger wrote:
 From: Nicholas Bellinger n...@linux-iscsi.org
 
 Hi folks,
 
 This series adds a virtio_queue_valid() for use by virtio-pci code in
 order to prevent opreations upon uninitialized VQs, which is currently
 expected to occur during seabios setup of virtio-scsi with in-flight
 vhost-scsi-pci device code.
 
 On the vhost side, it also adds virtio_queue_valid() sanity checks in
 vhost_virtqueue_[start,stop]() and vhost_verify_ring_mappings() in order
 to skip the same uninitialized VQs.
 
 Changes from v1:
   - Remove now unnecessary virtio_queue_get_num() calls in virtio-pci.c
   - Add virtio_queue_valid() calls in vhost_virtqueue_[start,stop]()
 
 Please review.
 
 --nab

Looks reasonable.
Acked-by: Michael S. Tsirkin m...@redhat.com

So - does this fix the issues you saw with vhost-scsi?

 Michael S. Tsirkin (1):
   virtio: add API to check that ring is setup
 
 Nicholas Bellinger (2):
   virtio-pci: Add virtio_queue_valid checks ahead of
 virtio_queue_get_num
   vhost: Skip uninitialized VQs in vhost_virtqueue_[start,stop]
 
  hw/vhost.c  |   12 
  hw/virtio-pci.c |   34 +++---
  hw/virtio.c |5 +
  hw/virtio.h |1 +
  4 files changed, 33 insertions(+), 19 deletions(-)
 
 -- 
 1.7.2.5



Re: [Qemu-devel] [PATCH v4 0/7] virtio-serial refactoring.

2013-04-02 Thread Peter Maydell
On 29 March 2013 09:02,  fred.kon...@greensocs.com wrote:
 From: KONRAD Frederic fred.kon...@greensocs.com

 This is the next part of virtio-refactoring.

 Basically it creates virtio-serial device which extends virtio-device.
 Then a virtio-serial can be connected on a virtio-bus.
 virtio-serial-pci, virtio-serial-s390 and virtio-serial-ccw are created too,
 they extend respectively virtio-pci, virtio-s390-device, virtio-ccw-device and
 have a virtio-serial.

Series:
Reviewed-by: Peter Maydell peter.mayd...@linaro.org

-- PMM



[Qemu-devel] [Patch] v2 fix /proc/self/maps output

2013-04-02 Thread Christophe Lyon
Add a space at end of line when there is no filename to print, to
conform to linux kernel format (see show_map_vma() in
fs/proc/task_mmu.c).

Signed-off-by: Christophe Lyon christophe.l...@linaro.org
---

Changes v1-v2:
Additional space is now part of the format string.

 linux-user/syscall.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a148d9f..c3c5c11 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5013,10 +5013,10 @@ static int open_self_maps(void *cpu_env, int fd)
 }
 if (h2g_valid(min)  h2g_valid(max)) {
 dprintf(fd, TARGET_ABI_FMT_lx - TARGET_ABI_FMT_lx
- %c%c%c%c %08 PRIx64  %02x:%02x %d%s%s\n,
+ %c%c%c%c %08 PRIx64  %02x:%02x %d %s%s\n,
 h2g(min), h2g(max), flag_r, flag_w,
 flag_x, flag_p, offset, dev_maj, dev_min, inode,
-path[0] ?: , path);
+path[0] ?   : , path);
 }
 }

--
1.7.10.4



Re: [Qemu-devel] vNVRAM / blobstore design

2013-04-02 Thread Michael S. Tsirkin
On Sun, Mar 31, 2013 at 04:48:24PM -0400, Kenneth Goldman wrote:
 Michael S. Tsirkin m...@redhat.com wrote on 03/31/2013 04:17:28 AM:
 
  You want to protect against someone who is able to
  manipulate some bits in the file (content) but not others (hash)?
  What's the attack you are trying to protect against here?
 
  I'm guessing the only result of extra checksums would be
  unbootable guests when qemu manages to corrupt the checksum
  without any help from attackers ...
 
 You are of course correct.  I advised an integrity value just to detect
 a hardware or software fault.  The check value would not protect against an
 attack.

Fair enough, but why protect these bits specifically?
E.g. disk corruption seems more likely (since it's bigger). Add
integrity at that level? Why even stop at detection, let's do error
correction ...

-- 
MST



[Qemu-devel] [Bug 1042388] Re: qemu: Unsupported syscall: 257 (timer_create)

2013-04-02 Thread LocutusOfBorg
Any news on this?

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

Title:
  qemu: Unsupported syscall: 257 (timer_create)

Status in QEMU:
  Confirmed

Bug description:
  Running qemu-arm-static for git HEAD. When I try to install ghc from
  debian into my arm chroot I get:

  Setting up ghc (7.4.1-4) ...
  qemu: Unsupported syscall: 257
  ghc: timer_create: Function not implemented
  qemu: Unsupported syscall: 257
  ghc-pkg: timer_create: Function not implemented
  dpkg: error processing ghc (--configure):
   subprocess installed post-installation script returned error exit status 1
  Errors were encountered while processing:
   ghc
  E: Sub-process /usr/bin/dpkg returned an error code (1)

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



Re: [Qemu-devel] [PATCH 1/1] rng backend: open backend in blocking mode

2013-04-02 Thread Anthony Liguori
Amit Shah amit.s...@redhat.com writes:

 On (Mon) 01 Apr 2013 [09:02:46], Anthony Liguori wrote:
 Amit Shah amit.s...@redhat.com writes:
 
  Opening backends in non-blocking mode isn't necessary, we don't do
  anything while waiting for data.
 
  This also excuses us from checking for EAGAIN, which for the default
  random backend, is a very common return error type.
 
 It's not common...  It really shouldn't happen however.

 EAGAIN is common when a file is opened in non-blocking mode.  Needs to
 be made verbose?

EAGAIN doesn't just happen randomly.  It only happens when you read from
an fd when no data is present.  Normally, that is something that is
predictable.

   Starting the guest
  with '-device virtio-rng-pci', issuing a 'cat /dev/hwrng' in the guest
  while also doing 'cat /dev/random' on the host causes
 
 You are essentially cat'ing the same device twice.  What's happening is
 that there is entropy available in /dev/random so a select()
 notification happens but before we are able to read() it, the cat of
 /dev/hwrng ends up consuming that entropy.
 
 This would never happen with a socket, for instance.  /dev/random is
 special in there are multiple readers.
 
 
  backends/rng-random.c:44:entropy_available: assertion failed: (len != -1)
 
  without this fix.
 
 This fix would cause QEMU to block indefinitely which I don't think is
 very good behavior.  I think a better solution would be:
 
 diff --git a/backends/rng-random.c b/backends/rng-random.c
 index acd20af..9fde566 100644
 --- a/backends/rng-random.c
 +++ b/backends/rng-random.c
 @@ -41,6 +41,9 @@ static void entropy_available(void *opaque)
  ssize_t len;
  
  len = read(s-fd, buffer, s-size);
 +if (len == -1  errno == EINTR) {
 +return;
 +}


That's a typo.  I meant s/EINTR/EAGAIN/g

Regards,

Anthony Liguori


 This has to be an additional fix on top of this one.  EAGAIN has to be
 handled if we want to allow nonblocking reads, and there doesn't seem
 to be any reason to have these reads be non-blocking.

 OTOH, I also think we could use the glib functions for file IO, since
 handling EINTR in each open-coded read call isn't always fun.

   Amit



Re: [Qemu-devel] [PATCH v4 0/7] virtio-serial refactoring.

2013-04-02 Thread Anthony Liguori
fred.kon...@greensocs.com writes:

 From: KONRAD Frederic fred.kon...@greensocs.com

 This is the next part of virtio-refactoring.

 Basically it creates virtio-serial device which extends virtio-device.
 Then a virtio-serial can be connected on a virtio-bus.
 virtio-serial-pci, virtio-serial-s390 and virtio-serial-ccw are created too,
 they extend respectively virtio-pci, virtio-s390-device, virtio-ccw-device and
 have a virtio-serial.

 You can checkout my branch here:

 git://project.greensocs.com/qemu-virtio.git virtio-serial-v4

 Note that it is nearly the same series as virtio-blk and virtio-scsi
 refactoring.

 I made basic tests (with linux guests) on:
  * qemu-system-i386

 CHanges v3 - v':
 * Removed serial configuration field ommited in VirtioCCWDevice structure.

 Changes v2 - v3:
 * Added CCW device.
 * Rebased.

Would like to at least see an Acked-by from Amit here...

Regards,

Anthony Liguori


 Thanks,

 Fred

 KONRAD Frederic (7):
   virtio-serial: add the virtio-serial device.
   virtio-serial-pci: switch to the new API.
   virtio-serial-s390: switch to the new API.
   virtio-serial-ccw: switch to the new API.
   virtio-serial: cleanup: init and exit functions.
   virtio-serial: cleanup: use QOM casts.
   virtio-serial: cleanup: remove qdev field.

  hw/s390x/s390-virtio-bus.c |  32 +
  hw/s390x/s390-virtio-bus.h |  12 +++-
  hw/s390x/virtio-ccw.c  |  30 +
  hw/s390x/virtio-ccw.h  |  12 +++-
  hw/virtio-pci.c| 130 ++---
  hw/virtio-pci.h|  14 +++-
  hw/virtio-serial-bus.c | 157 
 +++--
  hw/virtio-serial.h |  13 +++-
  8 files changed, 240 insertions(+), 160 deletions(-)

 -- 
 1.7.11.7




Re: [Qemu-devel] KVM call agenda for 2013-04-02

2013-04-02 Thread Juan Quintela
Juan Quintela quint...@redhat.com wrote:
 Hi

 Please send in any agenda topics you are interested in.

As there are no items, today call is cancelled.

Happy hacking.




Re: [Qemu-devel] vNVRAM / blobstore design

2013-04-02 Thread Kenneth Goldman
  You are of course correct.  I advised an integrity value just to 
detect
  a hardware or software fault.  The check value would not protect 
against an
  attack.
 
 Fair enough, but why protect these bits specifically?
 E.g. disk corruption seems more likely (since it's bigger). Add
 integrity at that level? Why even stop at detection, let's do error
 correction ...

Why ... just because it's a security device.  Whenever I code for 
security,
I add layers of protection, constantly looking for this should never 
happen
cases.

It might be just a small benefit, but hashing a few kbytes is a small part
of TPM startup time, and the function is already there.  Think of it as 
part
of the larger (and required) TPM self test that a TPM must do.


[Qemu-devel] [PATCH 2/2] xen-mapcache: pass the right size argument to test_bits

2013-04-02 Thread Stefano Stabellini
From: Hanweidong hanweid...@huawei.com

Compute the correct size for test_bits().
qemu_get_ram_ptr() and qemu_safe_ram_ptr() will call xen_map_cache()
with size is 0 if the requested address is in the RAM.  Then
xen_map_cache() will pass the size 0 to test_bits() for checking if the
corresponding pfn was mapped in cache. But test_bits() will always
return 1 when size is 0 without any bit testing. Actually, for this
case, test_bits should check one bit. So this patch introduced a
__test_bit_size which is greater than 0 and a multiple of XC_PAGE_SIZE,
then test_bits can work correctly with __test_bit_size
 XC_PAGE_SHIFT as its size.

Signed-off-by: Zhenguo Wang wangzhen...@huawei.com
Signed-off-by: Weidong Han hanweid...@huawei.com
Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
---
 xen-mapcache.c |   26 ++
 1 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/xen-mapcache.c b/xen-mapcache.c
index a80cbdb..5a626cd 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -200,6 +200,7 @@ uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr size,
 hwaddr address_index;
 hwaddr address_offset;
 hwaddr __size = size;
+hwaddr __test_bit_size = size;
 bool translated = false;
 
 tryagain:
@@ -208,9 +209,23 @@ tryagain:
 
 trace_xen_map_cache(phys_addr);
 
+/* __test_bit_size is always a multiple of XC_PAGE_SIZE */
+if (size) {
+__test_bit_size = size + (phys_addr  (XC_PAGE_SIZE - 1));
+
+if (__test_bit_size % XC_PAGE_SIZE) {
+__test_bit_size += XC_PAGE_SIZE - (__test_bit_size % XC_PAGE_SIZE);
+}
+} else {
+__test_bit_size = XC_PAGE_SIZE;
+}
+
 if (mapcache-last_entry != NULL 
 mapcache-last_entry-paddr_index == address_index 
-!lock  !__size) {
+!lock  !__size 
+test_bits(address_offset  XC_PAGE_SHIFT,
+  __test_bit_size  XC_PAGE_SHIFT,
+  mapcache-last_entry-valid_mapping)) {
 trace_xen_map_cache_return(mapcache-last_entry-vaddr_base + 
address_offset);
 return mapcache-last_entry-vaddr_base + address_offset;
 }
@@ -229,7 +244,8 @@ tryagain:
 
 while (entry  entry-lock  entry-vaddr_base 
 (entry-paddr_index != address_index || entry-size != __size ||
- !test_bits(address_offset  XC_PAGE_SHIFT, size  XC_PAGE_SHIFT,
+ !test_bits(address_offset  XC_PAGE_SHIFT,
+ __test_bit_size  XC_PAGE_SHIFT,
  entry-valid_mapping))) {
 pentry = entry;
 entry = entry-next;
@@ -241,13 +257,15 @@ tryagain:
 } else if (!entry-lock) {
 if (!entry-vaddr_base || entry-paddr_index != address_index ||
 entry-size != __size ||
-!test_bits(address_offset  XC_PAGE_SHIFT, size  
XC_PAGE_SHIFT,
+!test_bits(address_offset  XC_PAGE_SHIFT,
+__test_bit_size  XC_PAGE_SHIFT,
 entry-valid_mapping)) {
 xen_remap_bucket(entry, __size, address_index);
 }
 }
 
-if(!test_bits(address_offset  XC_PAGE_SHIFT, size  XC_PAGE_SHIFT,
+if(!test_bits(address_offset  XC_PAGE_SHIFT,
+__test_bit_size  XC_PAGE_SHIFT,
 entry-valid_mapping)) {
 mapcache-last_entry = NULL;
 if (!translated  mapcache-phys_offset_to_gaddr) {
-- 
1.7.2.5




[Qemu-devel] [PATCH 1/2] xen-mapcache: replace last_address_index with a last_entry pointer

2013-04-02 Thread Stefano Stabellini
Replace last_address_index and last_address_vaddr with a single pointer
to the last MapCacheEntry used.

Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
---
 xen-mapcache.c |   34 --
 1 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/xen-mapcache.c b/xen-mapcache.c
index dc6d1fa..a80cbdb 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -74,8 +74,7 @@ typedef struct MapCache {
 QTAILQ_HEAD(map_cache_head, MapCacheRev) locked_entries;
 
 /* For most cases (99.9%), the page address is the same. */
-hwaddr last_address_index;
-uint8_t *last_address_vaddr;
+MapCacheEntry *last_entry;
 unsigned long max_mcache_size;
 unsigned int mcache_bucket_shift;
 
@@ -105,7 +104,6 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f, void 
*opaque)
 mapcache-opaque = opaque;
 
 QTAILQ_INIT(mapcache-locked_entries);
-mapcache-last_address_index = -1;
 
 if (geteuid() == 0) {
 rlimit_as.rlim_cur = RLIM_INFINITY;
@@ -210,9 +208,11 @@ tryagain:
 
 trace_xen_map_cache(phys_addr);
 
-if (address_index == mapcache-last_address_index  !lock  !__size) {
-trace_xen_map_cache_return(mapcache-last_address_vaddr + 
address_offset);
-return mapcache-last_address_vaddr + address_offset;
+if (mapcache-last_entry != NULL 
+mapcache-last_entry-paddr_index == address_index 
+!lock  !__size) {
+trace_xen_map_cache_return(mapcache-last_entry-vaddr_base + 
address_offset);
+return mapcache-last_entry-vaddr_base + address_offset;
 }
 
 /* size is always a multiple of MCACHE_BUCKET_SIZE */
@@ -249,7 +249,7 @@ tryagain:
 
 if(!test_bits(address_offset  XC_PAGE_SHIFT, size  XC_PAGE_SHIFT,
 entry-valid_mapping)) {
-mapcache-last_address_index = -1;
+mapcache-last_entry = NULL;
 if (!translated  mapcache-phys_offset_to_gaddr) {
 phys_addr = mapcache-phys_offset_to_gaddr(phys_addr, size, 
mapcache-opaque);
 translated = true;
@@ -259,19 +259,18 @@ tryagain:
 return NULL;
 }
 
-mapcache-last_address_index = address_index;
-mapcache-last_address_vaddr = entry-vaddr_base;
+mapcache-last_entry = entry;
 if (lock) {
 MapCacheRev *reventry = g_malloc0(sizeof(MapCacheRev));
 entry-lock++;
-reventry-vaddr_req = mapcache-last_address_vaddr + address_offset;
-reventry-paddr_index = mapcache-last_address_index;
+reventry-vaddr_req = mapcache-last_entry-vaddr_base + 
address_offset;
+reventry-paddr_index = mapcache-last_entry-paddr_index;
 reventry-size = entry-size;
 QTAILQ_INSERT_HEAD(mapcache-locked_entries, reventry, next);
 }
 
-trace_xen_map_cache_return(mapcache-last_address_vaddr + address_offset);
-return mapcache-last_address_vaddr + address_offset;
+trace_xen_map_cache_return(mapcache-last_entry-vaddr_base + 
address_offset);
+return mapcache-last_entry-vaddr_base + address_offset;
 }
 
 ram_addr_t xen_ram_addr_from_mapcache(void *ptr)
@@ -338,9 +337,9 @@ void xen_invalidate_map_cache_entry(uint8_t *buffer)
 QTAILQ_REMOVE(mapcache-locked_entries, reventry, next);
 g_free(reventry);
 
-if (mapcache-last_address_index == paddr_index) {
-mapcache-last_address_index = -1;
-mapcache-last_address_vaddr = NULL;
+if (mapcache-last_entry != NULL 
+mapcache-last_entry-paddr_index == paddr_index) {
+mapcache-last_entry = NULL;
 }
 
 entry = mapcache-entry[paddr_index % mapcache-nr_buckets];
@@ -404,8 +403,7 @@ void xen_invalidate_map_cache(void)
 entry-valid_mapping = NULL;
 }
 
-mapcache-last_address_index = -1;
-mapcache-last_address_vaddr = NULL;
+mapcache-last_entry = NULL;
 
 mapcache_unlock();
 }
-- 
1.7.2.5




Re: [Qemu-devel] [PATCH V3 WIP 3/3] disable vhost_verify_ring_mappings check

2013-04-02 Thread Michael S. Tsirkin
On Mon, Apr 01, 2013 at 06:05:47PM -0700, Nicholas A. Bellinger wrote:
 On Fri, 2013-03-29 at 09:14 +0100, Paolo Bonzini wrote: 
  Il 29/03/2013 03:53, Nicholas A. Bellinger ha scritto:
   On Thu, 2013-03-28 at 06:13 -0400, Paolo Bonzini wrote:
   I think it's the right thing to do, but maybe not the right place
   to do this, need to reset after all IO is done, before
   ring memory is write protected.
  
   Our emails are crossing each other unfortunately, but I want to
   reinforce this: ring memory is not write protected.
   
   Understood.  However, AFAICT the act of write protecting these ranges
   for ROM generates the offending callbacks to vhost_set_memory().
   
   The part that I'm missing is if ring memory is not being write protected
   by make_bios_readonly_intel(), why are the vhost_set_memory() calls
   being invoked..?
  
  Because mappings change for the region that contains the ring.  vhost
  doesn't know yet that the changes do not affect ring memory,
  vhost_set_memory() is called exactly to ascertain that.
  
 
 Hi Paolo  Co,
 
 Here's a bit more information on what is going on with the same
 cpu_physical_memory_map() failure in vhost_verify_ring_mappings()..
 
 So as before, at the point that seabios is marking memory as readonly
 for ROM in src/shadow.c:make_bios_readonly_intel() with the following
 call:
 
 Calling pci_config_writeb(0x31): bdf: 0x pam: 0x005b
 
 the memory API update hook triggers back into vhost_region_del() code,
 and following occurs:
 
 Entering vhost_region_del section: 0x7fd30a213b60 offset_within_region: 
 0xc size: 2146697216 readonly: 0
 vhost_region_del: is_rom: 0, rom_device: 0
 vhost_region_del: readable: 1
 vhost_region_del: ram_addr 0x0, addr: 0x0 size: 2147483648
 vhost_region_del: name: pc.ram
 Entering vhost_set_memory, section: 0x7fd30a213b60 add: 0, dev-started: 1
 Entering verify_ring_mappings: start_addr 0x000c size: 2146697216
 verify_ring_mappings: ring_phys 0x0 ring_size: 0
 verify_ring_mappings: ring_phys 0x0 ring_size: 0
 verify_ring_mappings: ring_phys 0xed000 ring_size: 5124
 verify_ring_mappings: calling cpu_physical_memory_map ring_phys: 0xed000 l: 
 5124
 address_space_map: addr: 0xed000, plen: 5124
 address_space_map: l: 4096, len: 5124
 phys_page_find got PHYS_MAP_NODE_NIL ..
 address_space_map: section: 0x7fd30fabaed0 memory_region_is_ram: 0 readonly: 0
 address_space_map: section: 0x7fd30fabaed0 offset_within_region: 0x0 section 
 size: 18446744073709551615
 Unable to map ring buffer for ring 2, l: 4096
 
 So the interesting part is that phys_page_find() is not able to locate
 the corresponding page for vq-ring_phys: 0xed000 from the
 vhost_region_del() callback with section-offset_within_region:
 0xc..
 
 Is there any case where this would not be considered a bug..? 
 
 register_multipage : d: 0x7fd30f7d0ed0 section: 0x7fd30a2139b0
 register_multipage : d: 0x7fd30f7d0ed0 section: 0x7fd30a2139b0
 register_multipage : d: 0x7fd30f7d0ed0 section: 0x7fd30a2139b0
 Entering vhost_region_add section: 0x7fd30a213aa0 offset_within_region: 
 0xc size: 32768 readonly: 1
 vhost_region_add: is_rom: 0, rom_device: 0
 vhost_region_add: readable: 1
 vhost_region_add: ram_addr 0x, addr: 0x   0 size: 
 2147483648
 vhost_region_add: name: pc.ram
 Entering vhost_set_memory, section: 0x7fd30a213aa0 add: 1, dev-started: 1
 Entering verify_ring_mappings: start_addr 0x000c size: 32768
 verify_ring_mappings: ring_phys 0x0 ring_size: 0
 verify_ring_mappings: ring_phys 0x0 ring_size: 0
 verify_ring_mappings: ring_phys 0xed000 ring_size: 5124
 verify_ring_mappings: Got !ranges_overlap, skipping
 register_multipage : d: 0x7fd30f7d0ed0 section: 0x7fd30a2139b0
 Entering vhost_region_add section: 0x7fd30a213aa0 offset_within_region: 
 0xc8000 size: 2146664448 readonly: 0
 vhost_region_add: is_rom: 0, rom_device: 0
 vhost_region_add: readable: 1
 vhost_region_add: ram_addr 0x, addr: 0x   0 size: 
 2147483648
 vhost_region_add: name: pc.ram
 Entering vhost_set_memory, section: 0x7fd30a213aa0 add: 1, dev-started: 1
 Entering verify_ring_mappings: start_addr 0x000c8000 size: 2146664448
 verify_ring_mappings: ring_phys 0x0 ring_size: 0
 verify_ring_mappings: ring_phys 0x0 ring_size: 0
 verify_ring_mappings: ring_phys 0xed000 ring_size: 5124
 verify_ring_mappings: calling cpu_physical_memory_map ring_phys: 0xed000 l: 
 5124
 address_space_map: addr: 0xed000, plen: 5124
 address_space_map: l: 4096, len: 5124
 address_space_map: section: 0x7fd30fabb020 memory_region_is_ram: 1 readonly: 0
 address_space_map: section: 0x7fd30fabb020 offset_within_region: 0xc8000 
 section size: 2146664448
 address_space_map: l: 4096, len: 1028
 address_space_map: section: 0x7fd30fabb020 memory_region_is_ram: 1 readonly: 0
 address_space_map: section: 0x7fd30fabb020 offset_within_region: 0xc8000 
 section size: 2146664448
 address_space_map: Calling qemu_ram_ptr_length: raddr: 0x

Re: [Qemu-devel] [Xen-devel] frequently ballooning results in qemu exit

2013-04-02 Thread Stefano Stabellini
On Tue, 2 Apr 2013, Hanweidong wrote:
  -Original Message-
  From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com]
  Sent: 2013年4月1日 22:39
  To: Hanweidong
  Cc: Stefano Stabellini; Tim (Xen.org); George Dunlap; Andrew Cooper;
  Yanqiangjun; qemu-devel@nongnu.org; xen-de...@lists.xen.org; Gonglei
  (Arei); Anthony Perard; Wangzhenguo
  Subject: RE: [Qemu-devel] [Xen-devel] frequently ballooning results in
  qemu exit
  
  On Sat, 30 Mar 2013, Hanweidong wrote:
-Original Message-
From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com]
Sent: 2013年3月29日 20:37
To: Hanweidong
Cc: Tim (Xen.org); George Dunlap; Andrew Cooper; Yanqiangjun; qemu-
de...@nongnu.org; xen-de...@lists.xen.org; Gonglei (Arei); Anthony
Perard; Wangzhenguo
Subject: Re: [Qemu-devel] [Xen-devel] frequently ballooning results
  in
qemu exit
   
On Mon, 25 Mar 2013, Hanweidong wrote:
 We fixed this issue by below patch which computed the correct
  size
for test_bits(). qemu_get_ram_ptr() and qemu_safe_ram_ptr() will
  call
xen_map_cache() with size is 0 if the requested address is in the
  RAM.
Then xen_map_cache() will pass the size 0 to test_bits() for
  checking
if the corresponding pfn was mapped in cache. But test_bits() will
always return 1 when size is 0 without any bit testing. Actually,
  for
this case, test_bits should check one bit. So this patch introduced
  a
__test_bit_size which is greater than 0 and a multiple of
  XC_PAGE_SIZE,
then test_bits can work correctly with __test_bit_size 
  XC_PAGE_SHIFT
as its size.

 Signed-off-by: Zhenguo Wang wangzhen...@huawei.com
 Signed-off-by: Weidong Han hanweid...@huawei.com
   
Thanks for the patch and for debugging this difficult problem.
The reality is that size is never actually 0: when qemu_get_ram_ptr
calls xen_map_cache with size 0, it actually means map until the
  end
of
the page. As a consequence test_bits should always test at least 1
  bit,
like you wrote.
  
   Yes, for the case of size is 0, we can just simply set
  __test_bit_size 1. But for size  0, I think set __test_bit_size to
  size  XC_PAGE_SHIFT is incorrect. If size is not multiple of
  XC_PAGE_SIZE, then the part of (size % XC_PAGE_SIZE) also needs to test
  1 bit. For example size is XC_PAGE_SIZE + 1, then it needs to test 2
  bits, but size  XC_PAGE_SHIFT is only 1.
  
  
  I was assuming that the size is always page aligned.
  Looking through the code actually I think that it's better not to rely
  on this assumption.
  
  Looking back at your original patch:
  
  
  
   We fixed this issue by below patch which computed the correct size
  for test_bits(). qemu_get_ram_ptr() and qemu_safe_ram_ptr() will call
  xen_map_cache() with size is 0 if the requested address is in the RAM.
  Then xen_map_cache() will pass the size 0 to test_bits() for checking
  if the corresponding pfn was mapped in cache. But test_bits() will
  always return 1 when size is 0 without any bit testing. Actually, for
  this case, test_bits should check one bit. So this patch introduced a
  __test_bit_size which is greater than 0 and a multiple of XC_PAGE_SIZE,
  then test_bits can work correctly with __test_bit_size  XC_PAGE_SHIFT
  as its size.
  
   Signed-off-by: Zhenguo Wang wangzhen...@huawei.com
   Signed-off-by: Weidong Han hanweid...@huawei.com
  
   diff --git a/xen-mapcache.c b/xen-mapcache.c
   index 31c06dc..bd4a97f 100644
   --- a/xen-mapcache.c
   +++ b/xen-mapcache.c
   @@ -202,6 +202,7 @@ uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr
  size,
hwaddr address_index;
hwaddr address_offset;
hwaddr __size = size;
   +hwaddr __test_bit_size = size;
bool translated = false;
  
tryagain:
   @@ -210,7 +211,23 @@ tryagain:
  
trace_xen_map_cache(phys_addr);
  
   -if (address_index == mapcache-last_address_index  !lock
   !__size) {
   +entry = mapcache-entry[address_index % mapcache-nr_buckets];
  
  there is no need to move this line up here, see below
  
  
   +/* __test_bit_size is always a multiple of XC_PAGE_SIZE */
   +if (size) {
   +__test_bit_size = size + (phys_addr  (XC_PAGE_SIZE - 1));
   +
   +if (__test_bit_size % XC_PAGE_SIZE) {
   +__test_bit_size += XC_PAGE_SIZE - (__test_bit_size %
  XC_PAGE_SIZE);
   +}
   +} else {
   +__test_bit_size = XC_PAGE_SIZE;
   +}
  
  this is OK
  
  
   +if (address_index == mapcache-last_address_index  !lock
   !__size 
   +test_bits(address_offset  XC_PAGE_SHIFT,
   +  __test_bit_size  XC_PAGE_SHIFT,
   +  entry-valid_mapping)) {
trace_xen_map_cache_return(mapcache-last_address_vaddr +
  address_offset);
return mapcache-last_address_vaddr + address_offset;
}
  
  Unless I am missing something this change is unnecessary: if the
  mapping
  is 

  1   2   3   4   >