[Qemu-devel] block migration and dirty bitmap reset

2018-03-07 Thread Peter Lieven
Hi,

while looking at the code I wonder if the blk_aio_preadv and the 
bdrv_reset_dirty_bitmap order must
be swapped in mig_save_device_bulk:

qemu_mutex_lock_iothread();
aio_context_acquire(blk_get_aio_context(bmds->blk));
blk->aiocb = blk_aio_preadv(bb, cur_sector * BDRV_SECTOR_SIZE, &blk->qiov,
0, blk_mig_read_cb, blk);

bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, cur_sector * BDRV_SECTOR_SIZE,
nr_sectors * BDRV_SECTOR_SIZE);
aio_context_release(blk_get_aio_context(bmds->blk));
qemu_mutex_unlock_iothread();

In mig_save_device_dirty we first reset the dirty bitmap and read then which 
shoulds like
a better idea. Maybe it doesn't matter while we acquire the aioctx and the 
iothread lock...

Peter




[Qemu-devel] [PATCH v2] iotests: Update 051 and 186 after commit 1454509726719e0933c

2018-03-07 Thread Alberto Garcia
SCSI controllers are no longer created automatically for
-drive if=scsi, so this patch updates the tests that relied
on that.

Signed-off-by: Alberto Garcia 
---
 tests/qemu-iotests/051.pc.out | 20 
 tests/qemu-iotests/186|  4 
 tests/qemu-iotests/186.out| 28 
 3 files changed, 52 deletions(-)

diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
index 830c11880a..b01f9a90d7 100644
--- a/tests/qemu-iotests/051.pc.out
+++ b/tests/qemu-iotests/051.pc.out
@@ -117,20 +117,10 @@ Testing: -drive if=ide,media=cdrom
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) quit
 
-Testing: -drive if=scsi,media=cdrom
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -drive if=scsi,media=cdrom: warning: bus=0,unit=0 is 
deprecated with this machine type
-quit
-
 Testing: -drive if=ide
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) QEMU_PROG: Initialization of device ide-hd failed: Device needs media, 
but drive is empty
 
-Testing: -drive if=scsi
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -drive if=scsi: warning: bus=0,unit=0 is deprecated with 
this machine type
-QEMU_PROG: -drive if=scsi: Device needs media, but drive is empty
-
 Testing: -drive if=virtio
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) QEMU_PROG: -drive if=virtio: Device needs media, but drive is empty
@@ -170,20 +160,10 @@ Testing: -drive 
file=TEST_DIR/t.qcow2,if=ide,media=cdrom,readonly=on
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) quit
 
-Testing: -drive file=TEST_DIR/t.qcow2,if=scsi,media=cdrom,readonly=on
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -drive 
file=TEST_DIR/t.qcow2,if=scsi,media=cdrom,readonly=on: warning: bus=0,unit=0 is 
deprecated with this machine type
-quit
-
 Testing: -drive file=TEST_DIR/t.qcow2,if=ide,readonly=on
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) QEMU_PROG: Initialization of device ide-hd failed: Block node is 
read-only
 
-Testing: -drive file=TEST_DIR/t.qcow2,if=scsi,readonly=on
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -drive file=TEST_DIR/t.qcow2,if=scsi,readonly=on: warning: 
bus=0,unit=0 is deprecated with this machine type
-quit
-
 Testing: -drive file=TEST_DIR/t.qcow2,if=virtio,readonly=on
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) quit
diff --git a/tests/qemu-iotests/186 b/tests/qemu-iotests/186
index 44cc01ed87..9687243d34 100755
--- a/tests/qemu-iotests/186
+++ b/tests/qemu-iotests/186
@@ -133,10 +133,6 @@ check_info_block -drive if=ide,driver=null-co
 check_info_block -drive if=ide,media=cdrom
 check_info_block -drive if=ide,driver=null-co,media=cdrom
 
-check_info_block -drive if=scsi,driver=null-co
-check_info_block -drive if=scsi,media=cdrom
-check_info_block -drive if=scsi,driver=null-co,media=cdrom
-
 check_info_block -drive if=virtio,driver=null-co
 
 check_info_block -drive if=pflash,driver=null-co,size=1M
diff --git a/tests/qemu-iotests/186.out b/tests/qemu-iotests/186.out
index c8377fe146..ec75c0fc60 100644
--- a/tests/qemu-iotests/186.out
+++ b/tests/qemu-iotests/186.out
@@ -442,34 +442,6 @@ ide0-cd0 (NODE_NAME): null-co:// (null-co, read-only)
 Cache mode:   writeback
 (qemu) quit
 
-Testing: -drive if=scsi,driver=null-co
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -drive if=scsi,driver=null-co: warning: bus=0,unit=0 is 
deprecated with this machine type
-info block
-scsi0-hd0 (NODE_NAME): null-co:// (null-co)
-Attached to:  /machine/unattached/device[27]/scsi.0/legacy[0]
-Cache mode:   writeback
-(qemu) quit
-
-Testing: -drive if=scsi,media=cdrom
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -drive if=scsi,media=cdrom: warning: bus=0,unit=0 is 
deprecated with this machine type
-info block
-scsi0-cd0: [not inserted]
-Attached to:  /machine/unattached/device[27]/scsi.0/legacy[0]
-Removable device: not locked, tray closed
-(qemu) quit
-
-Testing: -drive if=scsi,driver=null-co,media=cdrom
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) QEMU_PROG: -drive if=scsi,driver=null-co,media=cdrom: warning: 
bus=0,unit=0 is deprecated with this machine type
-info block
-scsi0-cd0 (NODE_NAME): null-co:// (null-co, read-only)
-Attached to:  /machine/unattached/device[27]/scsi.0/legacy[0]
-Removable device: not locked, tray closed
-Cache mode:   writeback
-(qemu) quit
-
 Testing: -drive if=virtio,driver=null-co
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) info block
-- 
2.11.0




[Qemu-devel] [Bug 1658120] Re: building with gcc-aarch64-linux-gnu

2018-03-07 Thread Cao Van Dong
Hello everyone!!

I am having a issue when build qemu using gcc aarch64-linux-gnu-* on
ubuntu 16.04:

dong02@dong:~/qemu$ ./configure 
 \
> --prefix=/usr --cross-prefix=/usr/bin/aarch64-linux-gnu-   \
> --target-list=aarch64-softmmu  \
> --enable-attr   --enable-fdt   --enable-kvm\
> --enable-sdl--enable-system--enable-tools  \
> --audio-drv-list=  \
> --disable-bluez --disable-brlapi   --disable-bsd-user  \
> --disable-cap-ng--disable-curl --disable-curses\
> --disable-docs  --disable-libiscsi --disable-linux-aio \
> --disable-rbd   --disable-seccomp  --disable-slirp \
> --disable-sparse--disable-spice--disable-strip \
> --disable-usb-redir --disable-vde  --disable-virtfs\
> --disable-vnc   --disable-werror   --disable-xen

ERROR: zlib check failed
   Make sure to have the zlib libs and headers installed.

I installed zlib library: sudo apt-get install zlib1g-dev. However, result no 
change
Please help me!!

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

Title:
  building with gcc-aarch64-linux-gnu

Status in QEMU:
  Invalid

Bug description:
  Hi, while trying to build qemu v2.8.0 with gcc-aarch64-linux-gnu
  cross-compiler I'm getting the following :

  
  In file included from /usr/include/x86_64-linux-gnu/sys/syscall.h:31:0,
   from /root/qemu/util/compatfd.c:21:
  /root/qemu/util/compatfd.c: In function 'qemu_signalfd':
  /root/qemu/util/compatfd.c:103:19: error: '__NR_signalfd' undeclared (first 
use in this function)
   ret = syscall(SYS_signalfd, -1, mask, _NSIG / 8);
 ^
  /root/qemu/util/compatfd.c:103:19: note: each undeclared identifier is 
reported only once for each function it appears in
  /root/qemu/rules.mak:59: recipe for target 'util/compatfd.o' failed
  make: *** [util/compatfd.o] Error 1

  
  I had configured it with :

  ../configure --target-list=x86_64-linux-user --static --cpu=aarch64

  And I'm on :

  Linux ubuntu-512mb-fra1-01 4.4.0-59-generic #80-Ubuntu SMP Fri Jan 6
  17:47:47 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

  Thanks

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



[Qemu-devel] [PATCH] hw/ppc/prep: Fix implicit creation of "-drive if=scsi" devices

2018-03-07 Thread Thomas Huth
The global hack for creating SCSI devices has recently been removed,
but this apparently broke SCSI devices on some boards that were not
ready for this change yet. For the 40p machine you now get:

$ ppc64-softmmu/qemu-system-ppc64 -M 40p -cdrom x.iso
qemu-system-ppc64: -cdrom x.iso: machine type does not support 
if=scsi,bus=0,unit=2

Fix it by providing a lsi53c810_create() function that takes care
of calling scsi_bus_legacy_handle_cmdline() after creating the
corresponding SCSI controller.

Fixes: 1454509726719e0933c800fad00d6999752688ea
Signed-off-by: Thomas Huth 
---
 hw/ppc/prep.c| 2 +-
 hw/scsi/lsi53c895a.c | 7 +++
 include/hw/pci/pci.h | 1 +
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 096d4d4..3361509 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -788,7 +788,7 @@ static void ibm_40p_init(MachineState *machine)
 qdev_prop_set_uint32(dev, "equipment", 0xc0);
 qdev_init_nofail(dev);
 
-pci_create_simple(pci_bus, PCI_DEVFN(1, 0), "lsi53c810");
+lsi53c810_create(pci_bus, PCI_DEVFN(1, 0));
 
 /* XXX: s3-trio at PCI_DEVFN(2, 0) */
 pci_vga_init(pci_bus);
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index f3d4c4d..160657f 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -2279,3 +2279,10 @@ void lsi53c895a_create(PCIBus *bus)
 
 scsi_bus_legacy_handle_cmdline(&s->bus);
 }
+
+void lsi53c810_create(PCIBus *bus, int devfn)
+{
+LSIState *s = LSI53C895A(pci_create_simple(bus, devfn, "lsi53c810"));
+
+scsi_bus_legacy_handle_cmdline(&s->bus);
+}
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index d8c18c7..e255941 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -708,6 +708,7 @@ PCIDevice *pci_create(PCIBus *bus, int devfn, const char 
*name);
 PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name);
 
 void lsi53c895a_create(PCIBus *bus);
+void lsi53c810_create(PCIBus *bus, int devfn);
 
 qemu_irq pci_allocate_irq(PCIDevice *pci_dev);
 void pci_set_irq(PCIDevice *pci_dev, int level);
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH] slirp: disable Nagle in outgoing connections

2018-03-07 Thread Kamil Rytarowski
This patch is correct. LLDB expects 1sec for reply, GDB by default 2.

Debuggers use this option to disable Nagle algorithm in order to quickly
transfer messages between gdb-server and gdb-client. It's also fairy
portable across systems.

On 04.01.2018 18:56, Andreas Gustafsson wrote:
> slirp: disable Nagle in outgoing connections
> 
> When setting up an outgoing user mode networking TCP connection,
> disable the Nagle algorithm in the host-side connection.  Either the
> guest is already doing Nagle, in which case there is no point in doing
> it twice, or it has chosen to disable it, in which case we should
> respect that choice.
> 
> This change speeds up GDB remote debugging over TCP over user mode
> networking (with GDB runing on the guest) by multiple orders of
> magnitude, and has been part of the local patches applied by pkgsrc
> since 2012 with no reported ill effects.
> 
> Signed-off-by: Andreas Gustafsson 

Signed-off-by: Kamil Rytarowski 

> ---
>  slirp/tcp_subr.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
> index da0d53743f..8d0f94b75f 100644
> --- a/slirp/tcp_subr.c
> +++ b/slirp/tcp_subr.c
> @@ -416,6 +416,8 @@ int tcp_fconnect(struct socket *so, unsigned short af)
>  socket_set_fast_reuse(s);
>  opt = 1;
>  qemu_setsockopt(s, SOL_SOCKET, SO_OOBINLINE, &opt, sizeof(opt));
> +opt = 1;
> +qemu_setsockopt(s, IPPROTO_TCP, TCP_NODELAY, &opt, sizeof(opt));
>  
>  addr = so->fhost.ss;
>  DEBUG_CALL(" connect()ing")
> 




signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH] hw/mips/jazz: Fix implicit creation of "-drive if=scsi" devices

2018-03-07 Thread Thomas Huth
The global hack for creating SCSI devices has recently been removed,
but this apparently broke SCSI devices on some boards that were not
ready for this change yet. For the pica61 machine you now get:

$ mips64-softmmu/qemu-system-mips64 -M pica61 -cdrom x.iso
qemu-system-mips64: -cdrom x.iso: machine type does not support 
if=scsi,bus=0,unit=2

Fix it by calling scsi_bus_legacy_handle_cmdline() after creating the
corresponding SCSI controller.

Fixes: 1454509726719e0933c800fad00d6999752688ea
Signed-off-by: Thomas Huth 
---
 hw/mips/mips_jazz.c   |  7 ---
 hw/scsi/esp.c | 12 +++-
 include/hw/scsi/esp.h | 10 +-
 3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index b09871a..bde2c9b 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -147,6 +147,7 @@ static void mips_jazz_init(MachineState *machine,
 MemoryRegion *ram = g_new(MemoryRegion, 1);
 MemoryRegion *bios = g_new(MemoryRegion, 1);
 MemoryRegion *bios2 = g_new(MemoryRegion, 1);
+ESPState *esp;
 
 /* init CPUs */
 cpu = MIPS_CPU(cpu_create(machine->cpu_type));
@@ -278,9 +279,9 @@ static void mips_jazz_init(MachineState *machine,
 }
 
 /* SCSI adapter */
-esp_init(0x80002000, 0,
- rc4030_dma_read, rc4030_dma_write, dmas[0],
- qdev_get_gpio_in(rc4030, 5), &esp_reset, &dma_enable);
+esp = esp_init(0x80002000, 0, rc4030_dma_read, rc4030_dma_write, dmas[0],
+   qdev_get_gpio_in(rc4030, 5), &esp_reset, &dma_enable);
+scsi_bus_legacy_handle_cmdline(&esp->bus);
 
 /* Floppy */
 for (n = 0; n < MAX_FD; n++) {
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 45975c2..64ec285 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -618,11 +618,11 @@ static const MemoryRegionOps sysbus_esp_mem_ops = {
 .valid.accepts = esp_mem_accepts,
 };
 
-void esp_init(hwaddr espaddr, int it_shift,
-  ESPDMAMemoryReadWriteFunc dma_memory_read,
-  ESPDMAMemoryReadWriteFunc dma_memory_write,
-  void *dma_opaque, qemu_irq irq, qemu_irq *reset,
-  qemu_irq *dma_enable)
+ESPState *esp_init(hwaddr espaddr, int it_shift,
+   ESPDMAMemoryReadWriteFunc dma_memory_read,
+   ESPDMAMemoryReadWriteFunc dma_memory_write,
+   void *dma_opaque, qemu_irq irq, qemu_irq *reset,
+   qemu_irq *dma_enable)
 {
 DeviceState *dev;
 SysBusDevice *s;
@@ -644,6 +644,8 @@ void esp_init(hwaddr espaddr, int it_shift,
 sysbus_mmio_map(s, 0, espaddr);
 *reset = qdev_get_gpio_in(dev, 0);
 *dma_enable = qdev_get_gpio_in(dev, 1);
+
+return esp;
 }
 
 static const struct SCSIBusInfo esp_scsi_info = {
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index 3b160f8..93fdace 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -7,11 +7,6 @@
 /* esp.c */
 #define ESP_MAX_DEVS 7
 typedef void (*ESPDMAMemoryReadWriteFunc)(void *opaque, uint8_t *buf, int len);
-void esp_init(hwaddr espaddr, int it_shift,
-  ESPDMAMemoryReadWriteFunc dma_memory_read,
-  ESPDMAMemoryReadWriteFunc dma_memory_write,
-  void *dma_opaque, qemu_irq irq, qemu_irq *reset,
-  qemu_irq *dma_enable);
 
 #define ESP_REGS 16
 #define TI_BUFSZ 16
@@ -136,6 +131,11 @@ typedef struct {
 #define TCHI_FAS100A 0x4
 #define TCHI_AM53C974 0x12
 
+ESPState *esp_init(hwaddr espaddr, int it_shift,
+   ESPDMAMemoryReadWriteFunc dma_memory_read,
+   ESPDMAMemoryReadWriteFunc dma_memory_write,
+   void *dma_opaque, qemu_irq irq, qemu_irq *reset,
+   qemu_irq *dma_enable);
 void esp_dma_enable(ESPState *s, int irq, int level);
 void esp_request_cancelled(SCSIRequest *req);
 void esp_command_complete(SCSIRequest *req, uint32_t status, size_t resid);
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH] slirp: disable Nagle in outgoing connections

2018-03-07 Thread Philippe Mathieu-Daudé
On 03/07/2018 06:13 AM, Kamil Rytarowski wrote:
> This patch is correct. LLDB expects 1sec for reply, GDB by default 2.
> 
> Debuggers use this option to disable Nagle algorithm in order to quickly
> transfer messages between gdb-server and gdb-client. It's also fairy
> portable across systems.
> 
> On 04.01.2018 18:56, Andreas Gustafsson wrote:
>> slirp: disable Nagle in outgoing connections
>>
>> When setting up an outgoing user mode networking TCP connection,
>> disable the Nagle algorithm in the host-side connection.  Either the
>> guest is already doing Nagle, in which case there is no point in doing
>> it twice, or it has chosen to disable it, in which case we should
>> respect that choice.
>>
>> This change speeds up GDB remote debugging over TCP over user mode
>> networking (with GDB runing on the guest) by multiple orders of
>> magnitude, and has been part of the local patches applied by pkgsrc
>> since 2012 with no reported ill effects.
>>
>> Signed-off-by: Andreas Gustafsson 
> 
> Signed-off-by: Kamil Rytarowski 

I suppose you meant "Reviewed-by: Kamil Rytarowski "

Reviewed-by: Philippe Mathieu-Daudé 

> 
>> ---
>>  slirp/tcp_subr.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
>> index da0d53743f..8d0f94b75f 100644
>> --- a/slirp/tcp_subr.c
>> +++ b/slirp/tcp_subr.c
>> @@ -416,6 +416,8 @@ int tcp_fconnect(struct socket *so, unsigned short af)
>>  socket_set_fast_reuse(s);
>>  opt = 1;
>>  qemu_setsockopt(s, SOL_SOCKET, SO_OOBINLINE, &opt, sizeof(opt));
>> +opt = 1;
>> +qemu_setsockopt(s, IPPROTO_TCP, TCP_NODELAY, &opt, sizeof(opt));
>>  
>>  addr = so->fhost.ss;
>>  DEBUG_CALL(" connect()ing")
>>
> 
> 



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH] hw/sparc/sun4m: Fix implicit creation of "-drive if=scsi" devices

2018-03-07 Thread Thomas Huth
The global hack for creating SCSI devices has recently been removed,
but this apparently broke SCSI devices on some boards that were not
ready for this change yet. For the sun4m machines you now get:

$ sparc-softmmu/qemu-system-sparc -boot d -cdrom x.iso
qemu-system-sparc: -cdrom x.iso: machine type does not support 
if=scsi,bus=0,unit=2

Fix it by calling scsi_bus_legacy_handle_cmdline() after creating the
corresponding SCSI controller.

Reported-by: Mark Cave-Ayland 
Fixes: 1454509726719e0933c800fad00d6999752688ea
Signed-off-by: Thomas Huth 
---
 hw/sparc/sun4m.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 61eb424..0f5804b 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -324,6 +324,7 @@ static void *sparc32_dma_init(hwaddr dma_base,
 
 esp = ESP_STATE(object_resolve_path_component(OBJECT(espdma), "esp"));
 sysbus_mmio_map(SYS_BUS_DEVICE(esp), 0, esp_base);
+scsi_bus_legacy_handle_cmdline(&esp->esp.bus);
 
 ledma = SPARC32_LEDMA_DEVICE(object_resolve_path_component(
  OBJECT(dma), "ledma"));
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH] slirp: disable Nagle in outgoing connections

2018-03-07 Thread Kamil Rytarowski
On 07.03.2018 10:38, Philippe Mathieu-Daudé wrote:
> On 03/07/2018 06:13 AM, Kamil Rytarowski wrote:
>> This patch is correct. LLDB expects 1sec for reply, GDB by default 2.
>>
>> Debuggers use this option to disable Nagle algorithm in order to quickly
>> transfer messages between gdb-server and gdb-client. It's also fairy
>> portable across systems.
>>
>> On 04.01.2018 18:56, Andreas Gustafsson wrote:
>>> slirp: disable Nagle in outgoing connections
>>>
>>> When setting up an outgoing user mode networking TCP connection,
>>> disable the Nagle algorithm in the host-side connection.  Either the
>>> guest is already doing Nagle, in which case there is no point in doing
>>> it twice, or it has chosen to disable it, in which case we should
>>> respect that choice.
>>>
>>> This change speeds up GDB remote debugging over TCP over user mode
>>> networking (with GDB runing on the guest) by multiple orders of
>>> magnitude, and has been part of the local patches applied by pkgsrc
>>> since 2012 with no reported ill effects.
>>>
>>> Signed-off-by: Andreas Gustafsson 
>>
>> Signed-off-by: Kamil Rytarowski 
> 
> I suppose you meant "Reviewed-by: Kamil Rytarowski "
> 

Correct!

> Reviewed-by: Philippe Mathieu-Daudé 
>



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v4 0/3] s390x/sclp: 64 bit event masks

2018-03-07 Thread Cornelia Huck
On Fri, 23 Feb 2018 18:42:55 +0100
Claudio Imbrenda  wrote:

> Until 67915de9f0383ccf4a ("s390x/event-facility: variable-length event masks")
> we only supported 32bit sclp event masks, even though the archiecture
> allows the guests to set up sclp event masks up to 1021 bytes in length.
> With that patch the behaviour was almost compliant, but some issues were
> still remaining, in particular regarding the handling of selective reads
> and migration.
> 
> This patchset fixes migration and the handling of selective reads, and
> puts in place the support for 64-bit sclp event masks internally.
> 
> A new property of the sclp-event device switches between the 32bit masks
> and the compliant behaviour. The property is bound to the machine
> version, so older machines keep the old broken behaviour, allowing for
> migration, but the default is the compliant implementation.
> 
> Fixes: 67915de9f0383ccf4a ("s390x/event-facility: variable-length event 
> masks")

Thanks, applied 1+2 (only patch 1 is needed for the fix.)



Re: [Qemu-devel] [Qemu-block] block migration and MAX_IN_FLIGHT_IO

2018-03-07 Thread Stefan Hajnoczi
On Wed, Mar 7, 2018 at 7:55 AM, Peter Lieven  wrote:
> Am 06.03.2018 um 17:35 schrieb Peter Lieven:
>> Am 06.03.2018 um 17:07 schrieb Stefan Hajnoczi:
>>> On Mon, Mar 05, 2018 at 02:52:16PM +, Dr. David Alan Gilbert wrote:
 * Peter Lieven (p...@kamp.de) wrote:
> Am 05.03.2018 um 12:45 schrieb Stefan Hajnoczi:
>> On Thu, Feb 22, 2018 at 12:13:50PM +0100, Peter Lieven wrote:
>>> I stumbled across the MAX_INFLIGHT_IO field that was introduced in 2015 
>>> and was curious what was the reason
>>> to choose 512MB as readahead? The question is that I found that the 
>>> source VM gets very unresponsive I/O wise
>>> while the initial 512MB are read and furthermore seems to stay 
>>> unreasponsive if we choose a high migration speed
>>> and have a fast storage on the destination VM.
>>>
>>> In our environment I modified this value to 16MB which seems to work 
>>> much smoother. I wonder if we should make
>>> this a user configurable value or define a different rate limit for the 
>>> block transfer in bulk stage at least?
>> I don't know if benchmarks were run when choosing the value.  From the
>> commit description it sounds like the main purpose was to limit the
>> amount of memory that can be consumed.
>>
>> 16 MB also fulfills that criteria :), but why is the source VM more
>> responsive with a lower value?
>>
>> Perhaps the issue is queue depth on the storage device - the block
>> migration code enqueues up to 512 MB worth of reads, and guest I/O has
>> to wait?
> That is my guess. Especially if the destination storage is faster we 
> basically alsways have
> 512 I/Os in flight on the source storage.
>
> Does anyone mind if the reduce that value to 16MB or do we need a better 
> mechanism?
 We've got migration-parameters these days; you could connect it to one
 of those fairly easily I think.
 Try: grep -i 'cpu[-_]throttle[-_]initial'  for an example of one that's
 already there.
 Then you can set it to whatever you like.
>>> It would be nice to solve the performance problem without adding a
>>> tuneable.
>>>
>>> On the other hand, QEMU has no idea what the queue depth of the device
>>> is.  Therefore it cannot prioritize guest I/O over block migration I/O.
>>>
>>> 512 parallel requests is much too high.  Most parallel I/O benchmarking
>>> is done at 32-64 queue depth.
>>>
>>> I think that 16 parallel requests is a reasonable maximum number for a
>>> background job.
>>>
>>> We need to be clear though that the purpose of this change is unrelated
>>> to the original 512 MB memory footprint goal.  It just happens to touch
>>> the same constant but the goal is now to submit at most 16 I/O requests
>>> in parallel to avoid monopolizing the I/O device.
>> I think we should really look at this. The variables that control if we stay 
>> in the while loop or not are incremented and decremented
>> at the following places:
>>
>> mig_save_device_dirty:
>> mig_save_device_bulk:
>> block_mig_state.submitted++;
>>
>> blk_mig_read_cb:
>> block_mig_state.submitted--;
>> block_mig_state.read_done++;
>>
>> flush_blks:
>> block_mig_state.read_done--;
>>
>> The condition of the while loop is:
>> (block_mig_state.submitted +
>> block_mig_state.read_done) * BLOCK_SIZE <
>>qemu_file_get_rate_limit(f) &&
>>(block_mig_state.submitted +
>> block_mig_state.read_done) <
>>MAX_INFLIGHT_IO)
>>
>> At first I wonder if we ever reach the rate-limit because we put the read 
>> buffers onto f AFTER we exit the while loop?
>>
>> And even if we reach the limit we constantly maintain 512 I/Os in parallel 
>> because we immediately decrement read_done
>> when we put the buffers to f in flush_blks. In the next iteration of the 
>> while loop we then read again until we have 512 in-flight I/Os.
>>
>> And shouldn't we have a time limit to limit the time we stay in the while 
>> loop? I think we artificially delay sending data to f?
>
> Thinking about it for a while I would propose the following:
>
> a) rename MAX_INFLIGHT_IO to MAX_IO_BUFFERS
> b) add MAX_PARALLEL_IO with a value of 16
> c) compare qemu_file_get_rate_limit only with block_mig_state.read_done
>
> This would yield in the following condition for the while loop:
>
> (block_mig_state.read_done * BLOCK_SIZE < qemu_file_get_rate_limit(f) &&
>  (block_mig_state.submitted + block_mig_state.read_done) < MAX_IO_BUFFERS &&
>  block_mig_state.submitted < MAX_PARALLEL_IO)
>
> Sounds that like a plan?

That sounds good to me.

Stefan



[Qemu-devel] [Bug 1658120] Re: building with gcc-aarch64-linux-gnu

2018-03-07 Thread philmd
Hi Cao Van Dong,

you need to install zlib1g-dev:arm64, see:

https://github.com/qemu/qemu/blob/master/tests/docker/dockerfiles
/debian-arm64-cross.docker

Regards,

Phil.

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

Title:
  building with gcc-aarch64-linux-gnu

Status in QEMU:
  Invalid

Bug description:
  Hi, while trying to build qemu v2.8.0 with gcc-aarch64-linux-gnu
  cross-compiler I'm getting the following :

  
  In file included from /usr/include/x86_64-linux-gnu/sys/syscall.h:31:0,
   from /root/qemu/util/compatfd.c:21:
  /root/qemu/util/compatfd.c: In function 'qemu_signalfd':
  /root/qemu/util/compatfd.c:103:19: error: '__NR_signalfd' undeclared (first 
use in this function)
   ret = syscall(SYS_signalfd, -1, mask, _NSIG / 8);
 ^
  /root/qemu/util/compatfd.c:103:19: note: each undeclared identifier is 
reported only once for each function it appears in
  /root/qemu/rules.mak:59: recipe for target 'util/compatfd.o' failed
  make: *** [util/compatfd.o] Error 1

  
  I had configured it with :

  ../configure --target-list=x86_64-linux-user --static --cpu=aarch64

  And I'm on :

  Linux ubuntu-512mb-fra1-01 4.4.0-59-generic #80-Ubuntu SMP Fri Jan 6
  17:47:47 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

  Thanks

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



Re: [Qemu-devel] [Bug 1658120] Re: building with gcc-aarch64-linux-gnu

2018-03-07 Thread Alex Bennée

Cao Van Dong <1658...@bugs.launchpad.net> writes:

> Hello everyone!!
>
> I am having a issue when build qemu using gcc aarch64-linux-gnu-* on
> ubuntu 16.04:
>
> dong02@dong:~/qemu$ ./configure   
>\
>> --prefix=/usr --cross-prefix=/usr/bin/aarch64-linux-gnu-   \
>> --target-list=aarch64-softmmu  \
>> --enable-attr   --enable-fdt   --enable-kvm\
>> --enable-sdl--enable-system--enable-tools  \
>> --audio-drv-list=  \
>> --disable-bluez --disable-brlapi   --disable-bsd-user  \
>> --disable-cap-ng--disable-curl --disable-curses\
>> --disable-docs  --disable-libiscsi --disable-linux-aio \
>> --disable-rbd   --disable-seccomp  --disable-slirp \
>> --disable-sparse--disable-spice--disable-strip \
>> --disable-usb-redir --disable-vde  --disable-virtfs\
>> --disable-vnc   --disable-werror   --disable-xen
>
> ERROR: zlib check failed
>Make sure to have the zlib libs and headers installed.
>
> I installed zlib library: sudo apt-get install zlib1g-dev. However, result no 
> change
> Please help me!!

You will have installed the x86 version of the zlib1g-dev libraries.
Unfortunately headers are not uniform across all architectures.

If you want to ensure you have all the appropriate headers for cross
compiling QEMU do:

  apt-get build-dep -a arm64 qemu

But you may well run into problems if the distribution isn't fully
multi-arch clean. This is one of the reasons we use docker to isolate
our various cross build environments:

  make docker-test-build@debian-arm64-cross J=9 TARGET_LIST=aarch64-softmmu

--
Alex Bennée



Re: [Qemu-devel] [PATCH] HMP: Initialize err before using

2018-03-07 Thread Dr. David Alan Gilbert
* Zhangjixiang (jixiang_zh...@h3c.com) wrote:
> When bdrv_snapshot_delete return fail, the errp will not be
> assigned a valid value in error_propagate as errp didn't be
> initialized in hmp_delvm, then error_reportf_err will use an
> uninitialized value(call by hmp_delvm), and qemu crash.
> 
> Signed-off-by: zhangjixiang 

Thanks; it's already noted to go in my next HMP pull.

(And this time the mail is the right format, thanks).

Dave

> ---
> hmp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 7870d6a300..4a4da004e9 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1340,7 +1340,7 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
> void hmp_delvm(Monitor *mon, const QDict *qdict)
> {
>  BlockDriverState *bs;
> -Error *err;
> +Error *err = NULL;
>  const char *name = qdict_get_str(qdict, "name");
>  if (bdrv_all_delete_snapshot(name, &bs, &err) < 0) {
> --
> 2.11.0
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH] hw: Do not include "sysemu/block-backend.h" if it is not necessary

2018-03-07 Thread Thomas Huth
On 15.02.2018 09:55, Thomas Huth wrote:
> After reviewing a patch from Philippe that removes block-backend.h
> from hw/lm32/milkymist.c, I noticed that this header is included
> unnecessarily in a lot of other files, too. Remove those unneeded
> includes to speed up the compilation process a little bit.
> 
> Signed-off-by: Thomas Huth 
> ---
>  hw/arm/highbank.c  | 1 -
>  hw/arm/msf2-soc.c  | 1 -
>  hw/arm/realview.c  | 1 -
>  hw/arm/tosa.c  | 1 -
>  hw/i386/pc.c   | 2 --
>  hw/i386/pc_piix.c  | 1 -
>  hw/ide/ahci-allwinner.c| 1 -
>  hw/ide/cmd646.c| 1 -
>  hw/ide/ich.c   | 1 -
>  hw/ide/isa.c   | 1 -
>  hw/ide/microdrive.c| 1 -
>  hw/ide/mmio.c  | 1 -
>  hw/mips/mips_fulong2e.c| 1 -
>  hw/mips/mips_jazz.c| 1 -
>  hw/ppc/mac_newworld.c  | 1 -
>  hw/ppc/mac_oldworld.c  | 1 -
>  hw/ppc/prep.c  | 1 -
>  hw/scsi/mptendian.c| 1 -
>  hw/sd/core.c   | 1 -
>  hw/sparc/sun4m.c   | 1 -
>  hw/tricore/tricore_testboard.c | 2 --
>  21 files changed, 23 deletions(-)
> 
> diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
> index 287392b..1742cf6 100644
> --- a/hw/arm/highbank.c
> +++ b/hw/arm/highbank.c
> @@ -27,7 +27,6 @@
>  #include "sysemu/kvm.h"
>  #include "sysemu/sysemu.h"
>  #include "hw/boards.h"
> -#include "sysemu/block-backend.h"
>  #include "exec/address-spaces.h"
>  #include "qemu/error-report.h"
>  #include "hw/char/pl011.h"
> diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
> index a8ec2cd..f68df56 100644
> --- a/hw/arm/msf2-soc.c
> +++ b/hw/arm/msf2-soc.c
> @@ -29,7 +29,6 @@
>  #include "exec/address-spaces.h"
>  #include "hw/char/serial.h"
>  #include "hw/boards.h"
> -#include "sysemu/block-backend.h"
>  #include "qemu/cutils.h"
>  #include "hw/arm/msf2-soc.h"
>  #include "hw/misc/unimp.h"
> diff --git a/hw/arm/realview.c b/hw/arm/realview.c
> index 87cd1e5..2139a62 100644
> --- a/hw/arm/realview.c
> +++ b/hw/arm/realview.c
> @@ -20,7 +20,6 @@
>  #include "sysemu/sysemu.h"
>  #include "hw/boards.h"
>  #include "hw/i2c/i2c.h"
> -#include "sysemu/block-backend.h"
>  #include "exec/address-spaces.h"
>  #include "qemu/error-report.h"
>  #include "hw/char/pl011.h"
> diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c
> index a55b1a3..7a925fa 100644
> --- a/hw/arm/tosa.c
> +++ b/hw/arm/tosa.c
> @@ -22,7 +22,6 @@
>  #include "hw/boards.h"
>  #include "hw/i2c/i2c.h"
>  #include "hw/ssi/ssi.h"
> -#include "sysemu/block-backend.h"
>  #include "hw/sysbus.h"
>  #include "exec/address-spaces.h"
>  #include "sysemu/sysemu.h"
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 55e69d6..7670b45 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -50,8 +50,6 @@
>  #include "sysemu/qtest.h"
>  #include "kvm_i386.h"
>  #include "hw/xen/xen.h"
> -#include "sysemu/block-backend.h"
> -#include "hw/block/block.h"
>  #include "ui/qemu-spice.h"
>  #include "exec/memory.h"
>  #include "exec/address-spaces.h"
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 456dc9e..527c922 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -40,7 +40,6 @@
>  #include "sysemu/sysemu.h"
>  #include "hw/sysbus.h"
>  #include "sysemu/arch_init.h"
> -#include "sysemu/block-backend.h"
>  #include "hw/i2c/smbus.h"
>  #include "hw/xen/xen.h"
>  #include "exec/memory.h"
> diff --git a/hw/ide/ahci-allwinner.c b/hw/ide/ahci-allwinner.c
> index c3f1604..5397483 100644
> --- a/hw/ide/ahci-allwinner.c
> +++ b/hw/ide/ahci-allwinner.c
> @@ -18,7 +18,6 @@
>  #include "qemu/osdep.h"
>  #include "hw/hw.h"
>  #include "qemu/error-report.h"
> -#include "sysemu/block-backend.h"
>  #include "sysemu/dma.h"
>  #include "hw/ide/internal.h"
>  #include "hw/ide/ahci_internal.h"
> diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
> index 65aff51..6bb92d7 100644
> --- a/hw/ide/cmd646.c
> +++ b/hw/ide/cmd646.c
> @@ -26,7 +26,6 @@
>  #include "hw/hw.h"
>  #include "hw/pci/pci.h"
>  #include "hw/isa/isa.h"
> -#include "sysemu/block-backend.h"
>  #include "sysemu/sysemu.h"
>  #include "sysemu/dma.h"
>  
> diff --git a/hw/ide/ich.c b/hw/ide/ich.c
> index c01b24e..134478e 100644
> --- a/hw/ide/ich.c
> +++ b/hw/ide/ich.c
> @@ -65,7 +65,6 @@
>  #include "hw/pci/msi.h"
>  #include "hw/pci/pci.h"
>  #include "hw/isa/isa.h"
> -#include "sysemu/block-backend.h"
>  #include "sysemu/dma.h"
>  #include "hw/ide/pci.h"
>  #include "hw/ide/ahci_internal.h"
> diff --git a/hw/ide/isa.c b/hw/ide/isa.c
> index 9fb24fc..028bd61 100644
> --- a/hw/ide/isa.c
> +++ b/hw/ide/isa.c
> @@ -25,7 +25,6 @@
>  #include "qemu/osdep.h"
>  #include "hw/hw.h"
>  #include "hw/isa/isa.h"
> -#include "sysemu/block-backend.h"
>  #include "sysemu/dma.h"
>  
>  #include "hw/ide/internal.h"
> diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
> index 58e4f52..34bb98d 100644
> --- a/hw/ide/microdrive.c
> +++ b/hw/ide/microdrive.c
> @@ -25,7 +25,6 @@
>

[Qemu-devel] Using new TCG Vector infrastructure in PowerPC

2018-03-07 Thread Nikunj A Dadhania
Hi Richard,

I was working to get TCG vector support for PowerPC[1]. Started with
converting logical operations like vector AND/OR/XOR and compare
instructions. Found some inconsistency during my testing on x86 laptop
emulating PowerPC:

zero =   
max =   

1) tcg_gen_andc_vec - vandc in PPC

New API result:
andc(zero, max) -  (zero & ~max ) =  
andc(max, zero) -  (max & ~zero ) =  
andc(max, max)  -  (max & ~max ) =  -->WRONG
andc(zero, zero)-  (zero & ~zero ) =   

Expected result:
andc(zero, max)  (zero & ~max ) =   
andc(max, zero)  (max & ~zero ) =   
andc(max, max)   (max & ~max ) =   
andc(zero, zero) (zero & ~zero ) =   

2) tcg_gen_or_vec - vor in PPC

New API result:
(zero | max ) =     -> WRONG
(max | max ) =   
(zero | zero ) =  

Expected result:
(zero | max ) =   
(max | max ) =   
(zero | zero ) =   

3) tcg_gen_cmp_vec(TCG_COND_EQ) - vcmpequ* in PPC

New API result(all incorrect):

vcmpequb (zero == zero ) =   
vcmpequh (zero == zero ) =   
vcmpequw (zero == zero ) =   
vcmpequd (zero == zero ) =  

Expected result:
vcmpequb (zero == zero ) =   
vcmpequh (zero == zero ) =   
vcmpequw (zero == zero ) =   
vcmpequd (zero == zero ) =  

Do you see something that I am missing here ?

Regards,
Nikunj

1. PowerPC TCG vector infrastructure implementation
https://github.com/nikunjad/qemu/tree/ppc_vec_0




Re: [Qemu-devel] [Qemu-block] [PATCH 0/2] block/ssh: Implement .bdrv_refresh_filename()

2018-03-07 Thread Kevin Wolf
Am 06.03.2018 um 22:51 hat John Snow geschrieben:
> On 02/05/2018 03:22 PM, Max Reitz wrote:
> > This series implements .bdrv_refresh_filename() for the ssh block
> > driver, along with an appropriate .bdrv_dirname() so we don't chop off
> > query strings for backing files with relative filenames.
> > 
> > This series depends on my “block: Fix some filename generation issues”
> > series and on Pino's “ssh: switch from libssh2 to libssh” patch.
> > 
> > Based-on: 20180205151835.20812-1-mre...@redhat.com
> > Based-on: 20180118164439.2120-1-ptosc...@redhat.com
> > 
> > 
> > Max Reitz (2):
> >   block/ssh: Implement .bdrv_refresh_filename()
> >   block/ssh: Implement .bdrv_dirname()
> > 
> >  block/ssh.c | 72 
> > +++--
> >  1 file changed, 65 insertions(+), 7 deletions(-)
> 
> Did this one rot on the vine?
> 
> >1 month old.

The Based-on tags are the problem, in particular the first one. But yes,
we could possibly do more to review the dependencies...

Kevin



Re: [Qemu-devel] [PATCH v3 11/29] vhost+postcopy: Transmit 'listen' to client

2018-03-07 Thread Peter Xu
On Tue, Mar 06, 2018 at 11:20:56AM +, Dr. David Alan Gilbert wrote:
> * Peter Xu (pet...@redhat.com) wrote:
> > On Mon, Mar 05, 2018 at 05:42:42PM +, Dr. David Alan Gilbert wrote:
> > > * Peter Xu (pet...@redhat.com) wrote:
> > > > On Fri, Feb 16, 2018 at 01:16:07PM +, Dr. David Alan Gilbert (git) 
> > > > wrote:
> > > > 
> > > > [...]
> > > > 
> > > > >  typedef struct VuVirtqElement {
> > > > > diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
> > > > > index 621543e654..bdec9ec0e8 100644
> > > > > --- a/docs/interop/vhost-user.txt
> > > > > +++ b/docs/interop/vhost-user.txt
> > > > > @@ -682,6 +682,12 @@ Master message types
> > > > >the slave must open a userfaultfd for later use.
> > > > >Note that at this stage the migration is still in precopy mode.
> > > > >  
> > > > > + * VHOST_USER_POSTCOPY_LISTEN
> > > > > +  Id: 27
> > > > > +  Master payload: N/A
> > > > > +
> > > > > +  Master advises slave that a transition to postcopy mode has 
> > > > > happened.
> > > > 
> > > > Could we add something to explain why this listen needs to be
> > > > broadcasted to clients?  Since I failed to find it out quickly
> > > > myself. :(
> > > 
> > > I've changed this to:
> > > 
> > >  * VHOST_USER_POSTCOPY_LISTEN
> > >   Id: 29
> > >   Master payload: N/A
> > > 
> > >   Master advises slave that a transition to postcopy mode has 
> > > happened.
> > >   The slave must ensure that shared memory is registered with 
> > > userfaultfd
> > >   to cause faulting of non-present pages.
> > 
> > But shouldn't this be assured by the SET_MEM_TABLE call?
> 
> Yes, it is the set_mem_table that does the register - but it only
> registers it with userfaultfd if it's received a 'listen' notification,
> otherwise it assumes it's a normal precopy migration.

I think I got the picture now.  Please add this after the enhanced
document:

Reviewed-by: Peter Xu 

Thanks,

-- 
Peter Xu



Re: [Qemu-devel] [PATCH v5 05/11] linux-user: fix mmap/munmap/mprotect/mremap/shmat

2018-03-07 Thread Laurent Vivier
Le 07/03/2018 à 07:36, Max Filippov a écrit :
> In linux-user QEMU that runs for a target with TARGET_ABI_BITS bigger
> than L1_MAP_ADDR_SPACE_BITS an assertion in page_set_flags fires when
> mmap, munmap, mprotect, mremap or shmat is called for an address outside
> the guest address space. mmap and mprotect should return ENOMEM in such
> case.
> 
> Change definition of GUEST_ADDR_MAX to always be the last valid guest
> address. Account for this change in open_self_maps.
> Add macro guest_addr_valid that verifies if the guest address is valid.
> Add function guest_range_valid that verifies if address range is within
> guest address space and does not wrap around. Use that macro in
> mmap/munmap/mprotect/mremap/shmat for error checking.
> 
> Cc: qemu-sta...@nongnu.org
> Cc: Riku Voipio 
> Cc: Laurent Vivier 
> Signed-off-by: Max Filippov 
> ---
> Changes v4->v5:
> - change definition of GUEST_ADDR_MAX to always be the last valid guest
>   address. Account for this change in guest_addr_valid and open_self_maps.
> - turn guest_range_valid into a function.
> 
> Changes v3->v4:
> - change GUEST_ADDR_MAX and h2g_valid definitions as suggested by Laurent
>   Vivier.
> 
> Changes v2->v3:
> - fix comparison in guest_valid: it must be 'less' to preserve the existing
>   functionality, not 'less or equal'.
> - fix guest_range_valid: it may not use guest_valid, because single range
>   that occupies all of the guest address space is valid.
> 
>  include/exec/cpu-all.h  |  6 +-
>  include/exec/cpu_ldst.h | 19 ++-
>  linux-user/mmap.c   | 20 +++-
>  linux-user/syscall.c|  5 -
>  4 files changed, 34 insertions(+), 16 deletions(-)
> 
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index 0b141683f095..f4fa94e9669d 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -159,8 +159,12 @@ extern unsigned long guest_base;
>  extern int have_guest_base;
>  extern unsigned long reserved_va;
>  
> -#define GUEST_ADDR_MAX (reserved_va ? reserved_va : \
> +#if HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS
> +#define GUEST_ADDR_MAX (~0ul)
> +#else
> +#define GUEST_ADDR_MAX (reserved_va ? reserved_va - 1 : \
>  (1ul << TARGET_VIRT_ADDR_SPACE_BITS) - 1)
> +#endif
>  #else
>  
>  #include "exec/hwaddr.h"
> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
> index 191f2e962a3c..313664e9dae8 100644
> --- a/include/exec/cpu_ldst.h
> +++ b/include/exec/cpu_ldst.h
> @@ -51,15 +51,16 @@
>  /* All direct uses of g2h and h2g need to go away for usermode softmmu.  */
>  #define g2h(x) ((void *)((unsigned long)(target_ulong)(x) + guest_base))
>  
> -#if HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS
> -#define h2g_valid(x) 1
> -#else
> -#define h2g_valid(x) ({ \
> -unsigned long __guest = (unsigned long)(x) - guest_base; \
> -(__guest < (1ul << TARGET_VIRT_ADDR_SPACE_BITS)) && \
> -(!reserved_va || (__guest < reserved_va)); \
> -})
> -#endif
> +#define guest_addr_valid(x) ((x) <= GUEST_ADDR_MAX)
> +#define h2g_valid(x) guest_addr_valid((unsigned long)(x) - guest_base)
> +
> +static inline int guest_range_valid(unsigned long start, unsigned long len)
> +{
> +if (len)
> +return guest_addr_valid(len - 1) && start <= GUEST_ADDR_MAX - len + 
> 1;
> +else
> +return guest_addr_valid(start);
> +}

I think we can consider len == 0 is invalid and use only:

  return start + (len - 1) <= GUEST_ADDR_MAX;

Thanks,
Laurent



Re: [Qemu-devel] [PATCH v2 5/5] s390x/cpumodel: Set up CPU model for AP device support

2018-03-07 Thread Pierre Morel

On 06/03/2018 18:15, David Hildenbrand wrote:

1) ap=on is a guest ABI feature saying to the guest you can use AP
instructions

Indeed, that's what belongs into the CPU model.


2) How we provide AP instructions to the guest can be done in three
different ways:
- SIE Interpretation

AP instructions executed on the guest will be interpreted and passed
directly
through to a real AP device installed on the host according to the APQN
specified in the instruction, so the AP instructions must be available
on the
host.

- interception with VFIO

AP instructions executed on the guest will be intercepted, and
interpreted by
QEMU then forwarded to a real AP device installed on the host according
to how
the AP devices are configured in userspace - i.e., whether there is
remapping,
multiplexing or sharing of adapters/domains etc. This also requires that AP
instructions be available on the host.

See my other mail: I think this is a conflict with vSIE.


- interception with emulation

AP instructions executed on the guest will be intercepted, interpreted
by QEMU
and emulated. This will not require AP instructions be available on the
host.

See my other mail: I think this is a conflict with vSIE.


In all cases above, the need to set ECA_APIE is dependent upon the device
type configured for the guest.

Due to bad AP design we can't handle this like zPCI - completely in
QEMU. That's what should control it.


3) We implement this with a device in QEMU and a certain level kernel
support.

It seems possible to set or not ECA.28 , based on the type of kernel device:
- SIE interpretation -> MATRIX KVM device -> ECA.28
- Interception with VFIO and virtualization -> no ECA.28
- interception with emulation -> no ECA.28

I understand the concern with the vCPU but I think we can handle it with
an indirect variable
like:

SIE interpretation Device + KVM_S390_VM_CPU_FEAT_AP -> set the variable
ap_to_be_sie_interpreted=1
Then in vCPU initialization set ECA.28 based on this variable.

That's exactly why we have the cpu feature interface. E.g. CMMA -> if
not enabled, not interpreted by HW (however also not intercepted to user
space - no sense in doing that right now).

There are two factors at play here, the device type (i.e., -device
vfio_ap) and
the KVM_S390_VM_CPU_FEAT_AP feature. Setting ECA_APIE makes no sense if the
vfio_ap device is not configured. For the passthrough implementation, this
means that the AP bus will successfully load on the guest, but there will
be no AP devices detected. If the expectation is that ap=on will allow
access
to AP devices on the guest, that would be a mistaken assumption.

If down the road a new guest AP device is introduced that allows
multiplexing,
device remapping etc., then it will be necessary to intercept AP
instructions
executed on the guest. In this case, if ap=on results in setting ECA_APIE,
then the instructions will be interpreted and passed through to a device
that does not exist because it won't be defined in the guest's CRYCB.

Again, see my other mail, this discussion is superfluous if we can't get
vSIE to work with emulated devices. And it smells like this is the case.
But I don't have access to documentation.


Based on these two scenarios, I think what we are really saying with ap=on
is that the guest will require use of the AP instructions installed on the
host. Whether those instructions are executed as a result of interpretation
by the hardware or because they are executed by the device driver is a
separate matter. So, I am inclined to agree with Pierre for that reason.
ECA_APIE should be set only if ap=on (i.e., AP instructions are available
on the host) and the user of those instructions (i.e., the driver) indicate
an intent to use them.

ap=on -> set ECA_APIE is what I proposed.


True if we only support SIE interpretation, what you propose.
but
It is not what I meant.
What I mean is the reverse implication

ECA_APIE => ap=on

But you can have ap = on and ECA_APIE = off
This is interception or emulation.

and the second thing is that we need two QEMU cpu features
AP : guest API to say we provide AP instructions to the guest (what ever 
we do to provide it)

ECA_APIE : kernel will setup the SIE with interpretation

other said:
if( !ap)
    return -ENODEVICE
if(ECA_API)
    set_interpretation()
else
    set_interception()




--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany




Re: [Qemu-devel] [PATCH v3 1/5] pc-dimm: refactor qmp_pc_dimm_device_list

2018-03-07 Thread Igor Mammedov
On Mon,  5 Mar 2018 14:57:06 +0800
Haozhong Zhang  wrote:

I'd suggest following commit subj/msg:

-
 pc-dimm: make qmp_pc_dimm_device_list() sort devices by address

 Make qmp_pc_dimm_device_list() return sorted by start address
 list of devices so that it could be reused in places that
 would need sorted list*. Reuse existing pc_dimm_built_list()
 to get sorted list.

 While at it hide recursive callbacks from callers, so that:

   qmp_pc_dimm_device_list(qdev_get_machine(), &list);

 could be replaced with simpler:

   list = qmp_pc_dimm_device_list();

 * follow up patch will use it in build_srat()
-

with commit message updated:
  Reviewed-by: Igor Mammedov 

> Use pc_dimm_built_list to hide recursive callbacks from callers.
> 
> Signed-off-by: Haozhong Zhang 
> ---
>  hw/mem/pc-dimm.c | 83 
> +---
>  hw/ppc/spapr.c   |  3 +-
>  include/hw/mem/pc-dimm.h |  2 +-
>  numa.c   |  4 +--
>  qmp.c|  7 +---
>  stubs/qmp_pc_dimm.c  |  4 +--
>  6 files changed, 50 insertions(+), 53 deletions(-)
> 
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 6e74b61cb6..4d050fe2cd 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -162,45 +162,6 @@ uint64_t get_plugged_memory_size(void)
>  return pc_existing_dimms_capacity(&error_abort);
>  }
>  
> -int qmp_pc_dimm_device_list(Object *obj, void *opaque)
> -{
> -MemoryDeviceInfoList ***prev = opaque;
> -
> -if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
> -DeviceState *dev = DEVICE(obj);
> -
> -if (dev->realized) {
> -MemoryDeviceInfoList *elem = g_new0(MemoryDeviceInfoList, 1);
> -MemoryDeviceInfo *info = g_new0(MemoryDeviceInfo, 1);
> -PCDIMMDeviceInfo *di = g_new0(PCDIMMDeviceInfo, 1);
> -DeviceClass *dc = DEVICE_GET_CLASS(obj);
> -PCDIMMDevice *dimm = PC_DIMM(obj);
> -
> -if (dev->id) {
> -di->has_id = true;
> -di->id = g_strdup(dev->id);
> -}
> -di->hotplugged = dev->hotplugged;
> -di->hotpluggable = dc->hotpluggable;
> -di->addr = dimm->addr;
> -di->slot = dimm->slot;
> -di->node = dimm->node;
> -di->size = object_property_get_uint(OBJECT(dimm), 
> PC_DIMM_SIZE_PROP,
> -NULL);
> -di->memdev = object_get_canonical_path(OBJECT(dimm->hostmem));
> -
> -info->u.dimm.data = di;
> -elem->value = info;
> -elem->next = NULL;
> -**prev = elem;
> -*prev = &elem->next;
> -}
> -}
> -
> -object_child_foreach(obj, qmp_pc_dimm_device_list, opaque);
> -return 0;
> -}
> -
>  static int pc_dimm_slot2bitmap(Object *obj, void *opaque)
>  {
>  unsigned long *bitmap = opaque;
> @@ -276,6 +237,50 @@ static int pc_dimm_built_list(Object *obj, void *opaque)
>  return 0;
>  }
>  
> +MemoryDeviceInfoList *qmp_pc_dimm_device_list(void)
> +{
> +GSList *dimms = NULL, *item;
> +MemoryDeviceInfoList *list = NULL, *prev = NULL;
> +
> +object_child_foreach(qdev_get_machine(), pc_dimm_built_list, &dimms);
> +
> +for (item = dimms; item; item = g_slist_next(item)) {
> +PCDIMMDevice *dimm = PC_DIMM(item->data);
> +Object *obj = OBJECT(dimm);
> +MemoryDeviceInfoList *elem = g_new0(MemoryDeviceInfoList, 1);
> +MemoryDeviceInfo *info = g_new0(MemoryDeviceInfo, 1);
> +PCDIMMDeviceInfo *di = g_new0(PCDIMMDeviceInfo, 1);
> +DeviceClass *dc = DEVICE_GET_CLASS(obj);
> +DeviceState *dev = DEVICE(obj);
> +
> +if (dev->id) {
> +di->has_id = true;
> +di->id = g_strdup(dev->id);
> +}
> +di->hotplugged = dev->hotplugged;
> +di->hotpluggable = dc->hotpluggable;
> +di->addr = dimm->addr;
> +di->slot = dimm->slot;
> +di->node = dimm->node;
> +di->size = object_property_get_uint(obj, PC_DIMM_SIZE_PROP, NULL);
> +di->memdev = object_get_canonical_path(OBJECT(dimm->hostmem));
> +
> +info->u.dimm.data = di;
> +elem->value = info;
> +elem->next = NULL;
> +if (prev) {
> +prev->next = elem;
> +} else {
> +list = elem;
> +}
> +prev = elem;
> +}
> +
> +g_slist_free(dimms);
> +
> +return list;
> +}
> +
>  uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
> uint64_t address_space_size,
> uint64_t *hint, uint64_t align, uint64_t size,
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 83c9d66dd5..68a81e47d2 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -731,8 +731,7 @@ static int spapr_populate_drconf_memory(sPAPRMachineState 
> *spapr, void *fdt)
>  }
>  
>  if (hotplug_lmb_start) {
> -Memor

Re: [Qemu-devel] [PULL] RISC-V QEMU Port Submission v8.2

2018-03-07 Thread Richard W.M. Jones
On Wed, Mar 07, 2018 at 01:09:29PM +1300, Michael Clark wrote:
> Hopefully, this PR gets merged...

I hope so too.  We've been testing v8 (substantially the same as v8.2)
extensively, including SMP.  It's building hundreds of packages a day
in the autobuilder, and being used for manual builds by several
people.

Please don't forget about the softfloat fix!  Although it's not
specific to riscv-qemu, it is required for reliable operation and I
don't see if in the v8.2 tree.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v



Re: [Qemu-devel] [PATCH v3 20/29] postcopy: postcopy_notify_shared_wake

2018-03-07 Thread Peter Xu
On Tue, Mar 06, 2018 at 10:54:18AM +, Dr. David Alan Gilbert wrote:

[...]

> > Basically above was what I thought - to record the faulted addresses
> > with specific PostcopyFD when page fault happened, then we may know
> > which page(s) will a PostcopyFD need.  But when with that, we'll
> > possibly need a lock to protect the information (or any other sync
> > method).
> 
> OK, but I think you're suggesting building a whole new data structure to
> know which ones need notifying;  that sounds like a lot of extra
> complexity for not much gain.

Yes we may need a new structure (or just a list of addresses?), and
indeed I have no idea on how that would help us.  I think it depends
on how many useless wakeup we will have, and how expensive is each of
such a wakeup notification.  Again, I think current solution is good
enough as long as we don't see explicit blocker on performance side,
and we can rethink that when really needed.  Thanks,

-- 
Peter Xu



Re: [Qemu-devel] [PATCH] trace: only permit standard C types and fixed size integer types

2018-03-07 Thread Daniel P . Berrangé
On Wed, Mar 07, 2018 at 09:24:30AM +0800, Fam Zheng wrote:
> On Tue, 03/06 16:22, Daniel P. Berrangé wrote:
> > RFE for patchew...
> > 
> > Now we're sending separate email alerts for each build job, can we
> > make patchew include the job name in the subject line, so it is
> > immediately obvious which job failed without needing to read the
> > body.
> 
> Yes, doable. Do we still want the original subject (The "Re: ..." part)? If 
> so,
> any idea what is a good format for the full subject line?

Perhaps take the original Subject, and prefix it. eg something like
this:

 Subject: s390-build failed (Re: [Qemu-devel] [PATCH] trace: only permit 
standard C types and...)


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v3 2/5] qmp: distinguish PC-DIMM and NVDIMM in MemoryDeviceInfoList

2018-03-07 Thread Igor Mammedov
On Mon,  5 Mar 2018 14:57:07 +0800
Haozhong Zhang  wrote:

> It may need to treat PC-DIMM and NVDIMM differently, e.g., when
> deciding the necessity of non-volatile flag bit in SRAT memory
> affinity structures.
> 
> NVDIMMDeviceInfo, which inherits from PCDIMMDeviceInfo, is added to
> union type MemoryDeviceInfo to record information of NVDIMM devices.
> The NVDIMM-specific data is currently left empty and will be filled
> when necessary in the future.
Maybe add to commit message that:

It also fixes "info memory-devices"/query-memory-devices
which currently show nvdimm devices as dimm devices since
object_dynamic_cast(obj, TYPE_PC_DIMM) happily cast nvdimm
to TYPE_PC_DIMM which it's been inherited from.

Otherwise patch looks good,
I'll ack it after rebase with is necessary for this patch.

> Signed-off-by: Haozhong Zhang 
> ---
>  hmp.c| 14 +++---
>  hw/mem/pc-dimm.c | 20 ++--
>  numa.c   | 19 +--
>  qapi-schema.json | 18 +-
>  4 files changed, 59 insertions(+), 12 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index ae86bfbade..3f06407c5e 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -2413,7 +2413,18 @@ void hmp_info_memory_devices(Monitor *mon, const QDict 
> *qdict)
>  switch (value->type) {
>  case MEMORY_DEVICE_INFO_KIND_DIMM:
>  di = value->u.dimm.data;
> +break;
> +
> +case MEMORY_DEVICE_INFO_KIND_NVDIMM:
> +di = qapi_NVDIMMDeviceInfo_base(value->u.nvdimm.data);
> +break;
> +
> +default:
> +di = NULL;
> +break;
> +}
>  
> +if (di) {
>  monitor_printf(mon, "Memory device [%s]: \"%s\"\n",
> MemoryDeviceInfoKind_str(value->type),
> di->id ? di->id : "");
> @@ -2426,9 +2437,6 @@ void hmp_info_memory_devices(Monitor *mon, const QDict 
> *qdict)
> di->hotplugged ? "true" : "false");
>  monitor_printf(mon, "  hotpluggable: %s\n",
> di->hotpluggable ? "true" : "false");
> -break;
> -default:
> -break;
>  }
>  }
>  }
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 4d050fe2cd..866ecc699a 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -20,6 +20,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "hw/mem/pc-dimm.h"
> +#include "hw/mem/nvdimm.h"
>  #include "qapi/error.h"
>  #include "qemu/config-file.h"
>  #include "qapi/visitor.h"
> @@ -249,10 +250,19 @@ MemoryDeviceInfoList *qmp_pc_dimm_device_list(void)
>  Object *obj = OBJECT(dimm);
>  MemoryDeviceInfoList *elem = g_new0(MemoryDeviceInfoList, 1);
>  MemoryDeviceInfo *info = g_new0(MemoryDeviceInfo, 1);
> -PCDIMMDeviceInfo *di = g_new0(PCDIMMDeviceInfo, 1);
> +PCDIMMDeviceInfo *di;
> +NVDIMMDeviceInfo *ndi;
> +bool is_nvdimm = object_dynamic_cast(obj, TYPE_NVDIMM);
>  DeviceClass *dc = DEVICE_GET_CLASS(obj);
>  DeviceState *dev = DEVICE(obj);
>  
> +if (!is_nvdimm) {
> +di = g_new0(PCDIMMDeviceInfo, 1);
> +} else {
> +ndi = g_new0(NVDIMMDeviceInfo, 1);
> +di = qapi_NVDIMMDeviceInfo_base(ndi);
> +}
> +
>  if (dev->id) {
>  di->has_id = true;
>  di->id = g_strdup(dev->id);
> @@ -265,7 +275,13 @@ MemoryDeviceInfoList *qmp_pc_dimm_device_list(void)
>  di->size = object_property_get_uint(obj, PC_DIMM_SIZE_PROP, NULL);
>  di->memdev = object_get_canonical_path(OBJECT(dimm->hostmem));
>  
> -info->u.dimm.data = di;
> +if (!is_nvdimm) {
> +info->u.dimm.data = di;
> +info->type = MEMORY_DEVICE_INFO_KIND_DIMM;
> +} else {
> +info->u.nvdimm.data = ndi;
> +info->type = MEMORY_DEVICE_INFO_KIND_NVDIMM;
> +}
>  elem->value = info;
>  elem->next = NULL;
>  if (prev) {
> diff --git a/numa.c b/numa.c
> index c6734ceb8c..23c4371e51 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -529,18 +529,25 @@ static void numa_stat_memory_devices(NumaNodeMem 
> node_mem[])
>  
>  if (value) {
>  switch (value->type) {
> -case MEMORY_DEVICE_INFO_KIND_DIMM: {
> +case MEMORY_DEVICE_INFO_KIND_DIMM:
>  pcdimm_info = value->u.dimm.data;
> +break;
> +
> +case MEMORY_DEVICE_INFO_KIND_NVDIMM:
> +pcdimm_info = 
> qapi_NVDIMMDeviceInfo_base(value->u.nvdimm.data);
> +break;
> +
> +default:
> +pcdimm_info = NULL;
> +break;
> +}
> +
> +if (pcdimm_info) {
>  node_mem[pcdimm_info->node].node_mem += pcdimm_info->size;
>  if (pcdimm_inf

Re: [Qemu-devel] [PATCH v2 0/5] s390x: vfio-ap: guest dedicated crypto adapters

2018-03-07 Thread Pierre Morel

On 06/03/2018 18:10, David Hildenbrand wrote:

If L2 forward devices to L3 through SIE ECA.28 but no bit is set is in
the CRYCB of L2,
L3 will not see any device.

Exactly and this is the problem: How should L2 know that these devices
are special and cannot be forwarded.


This is what we call the nightmare of nested virtualization (see x86),
because we have to emulate L3 instructions in L1 - but even worse, not
even in L1 kernel space but in L1 user space.

As soon as one level begin to virtualize, all levels under it
must virtualize too so that L3 instructions will be handled in L2
which will issue instructions that will be handled in L1.

By virtualize I assume you mean emulate? If so, yes.


So we could never provide the AP feature reliably with the SIE feature.

I think we should change a little this sentence to:
We can not provide SIE interpretation to a guest from which
any guest level N-1 does not use SIE interpretation.

Exactly, and as said, there is no way to tell a guest that it has AP but
cannot use AP interpretation but has to intercept and handle manually.



vSIE must clear ECA28 during running of the guest if the host itself do 
not have ECA28 set.

Since ECA28 set for the host means AP instructions available for the host
then we can sum it up by: vSIE should never set ECA28 in the shadow SIE
if no AP instructions available.

Pierre





Nothing bad will occur for the host, the hardware or other guests,
but the guest will just not get any device.


We want to avoid interdependence between CPU features. (because
everything else makes CPU feature detection ugly - CMMA is a good
example and the only exception so far)


Long story even shorter:

No emulated AP devices with KVM.


I agree with: KVM should never set bits in CRYCB for emulated devices.

I think this is stronger: emulated AP devices should not be used with
KVM because it can potentially lead to architectural (v)SIE conflicts.

But the details are buried in some AP documentation not accessible to me.

Anyhow, if the scenario I described cannot be worked around via:

a) telling a guest that AP virtualization cannot be used - which doesn't
seem to be possible
b) provoking for selected devices a SIE exit when an AP instruction is
executed on these devices - and this is totally fine with the documented
AP architecture

I assume we would have to live with !emualted AP devices.



--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany




Re: [Qemu-devel] [PULL 00/30] ppc-for-2.12 queue 20180306

2018-03-07 Thread Daniel P . Berrangé
On Tue, Mar 06, 2018 at 05:56:03PM +, Mark Cave-Ayland wrote:
> On 06/03/18 17:47, Thomas Huth wrote:
> 
> > > It seems that the error is being raised when setting the property rather
> > > than during realize so I'm not sure what I can do to handle this. Any
> > > thoughts?
> > 
> > Does the device need to be hot-pluggable or even user_creatable at all?
> > It seems like it is also using serial_hds[] directly, so that is a good
> > indication that it is *not* user creatable. So maybe the easiest fix is
> > to simply set
> > 
> > dc->user_creatable = false;
> > 
> > in macio_class_init() ?
> 
> (Added Daniel to CC)
> 
> I believe it should fail anyhow during realize because both macio devices
> (newworld and oldworld) requires an object link to the PIC which won't be
> set when using device_add via the monitor as in your example.
> 
> But it still doesn't quite feel right that just setting a property value
> should abort() immediately. Daniel, any thoughts?

Setting user_creatable = false is important, as that flag is used to filter
the list of permitted devices when doing monitor readline completion for
'device_add' and when printing available devices with '-device ?'


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v3 3/5] hw/acpi-build: build SRAT memory affinity structures for DIMM devices

2018-03-07 Thread Igor Mammedov
On Mon,  5 Mar 2018 14:57:08 +0800
Haozhong Zhang  wrote:

> ACPI 6.2A Table 5-129 "SPA Range Structure" requires the proximity
> domain of a NVDIMM SPA range must match with corresponding entry in
> SRAT table.
> 
> The address ranges of vNVDIMM in QEMU are allocated from the
> hot-pluggable address space, which is entirely covered by one SRAT
> memory affinity structure. However, users can set the vNVDIMM
> proximity domain in NFIT SPA range structure by the 'node' property of
> '-device nvdimm' to a value different than the one in the above SRAT
> memory affinity structure.
> 
> In order to solve such proximity domain mismatch, this patch builds
> one SRAT memory affinity structure for each DIMM device present at
> boot time, including both PC-DIMM and NVDIMM, with the proximity
> domain specified in '-device pc-dimm' or '-device nvdimm'.
> 
> The remaining hot-pluggable address space is covered by one or multiple
> SRAT memory affinity structures with the proximity domain of the last
> node as before.
> 
> Signed-off-by: Haozhong Zhang 
> ---
>  hw/i386/acpi-build.c | 60 
> 
>  1 file changed, 56 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index deb440f286..2ca0317386 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2323,6 +2323,59 @@ build_tpm2(GArray *table_data, BIOSLinker *linker, 
> GArray *tcpalog)
>  #define HOLE_640K_START  (640 * 1024)
>  #define HOLE_640K_END   (1024 * 1024)
>  
> +static void build_srat_hotpluggable_memory(GArray *table_data, uint64_t base,
> +   uint64_t len, int default_node)
> +{
> +MemoryDeviceInfoList *info_list = qmp_pc_dimm_device_list();
> +MemoryDeviceInfoList *info;
> +MemoryDeviceInfo *mi;
> +PCDIMMDeviceInfo *di;
> +uint64_t end = base + len, cur, addr, size;
> +int node;
> +bool is_nvdimm;
> +AcpiSratMemoryAffinity *numamem;
> +MemoryAffinityFlags flags;
> +
> +for (cur = base, info = info_list;
> + cur < end;
> + cur += size, info = info->next) {
> +numamem = acpi_data_push(table_data, sizeof *numamem);
> +
> +if (!info) {
> +build_srat_memory(numamem, cur, end - cur, default_node,
> +  MEM_AFFINITY_HOTPLUGGABLE | 
> MEM_AFFINITY_ENABLED);
> +break;
> +}
> +
> +mi = info->value;
> +is_nvdimm = (mi->type == MEMORY_DEVICE_INFO_KIND_NVDIMM);
> +di = !is_nvdimm ? mi->u.dimm.data :
> +  qapi_NVDIMMDeviceInfo_base(mi->u.nvdimm.data);
> +
> +addr = di->addr;
maybe drop addr and use di->addr directly

> +if (cur < addr) {
> +build_srat_memory(numamem, cur, addr - cur, default_node,
> +  MEM_AFFINITY_HOTPLUGGABLE | 
> MEM_AFFINITY_ENABLED);
> +numamem = acpi_data_push(table_data, sizeof *numamem);
> +}
> +
> +size = di->size;
> +node = di->node;
the same wrt di->node

> +
> +flags = MEM_AFFINITY_ENABLED;
> +if (di->hotpluggable) {
> +flags |= MEM_AFFINITY_HOTPLUGGABLE;
> +}
> +if (is_nvdimm) {
> +flags |= MEM_AFFINITY_NON_VOLATILE;
> +}
> +
> +build_srat_memory(numamem, addr, size, node, flags);
> +}
> +
> +qapi_free_MemoryDeviceInfoList(info_list);
> +}
> +
>  static void
>  build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
>  {
> @@ -2434,10 +2487,9 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
> MachineState *machine)
>   * providing _PXM method if necessary.
>   */
>  if (hotplugabble_address_space_size) {
> -numamem = acpi_data_push(table_data, sizeof *numamem);
> -build_srat_memory(numamem, pcms->hotplug_memory.base,
> -  hotplugabble_address_space_size, pcms->numa_nodes 
> - 1,
> -  MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
> +build_srat_hotpluggable_memory(table_data, pcms->hotplug_memory.base,
> +   hotplugabble_address_space_size,
> +   pcms->numa_nodes - 1);
>  }
>  
>  build_header(linker, table_data,




Re: [Qemu-devel] [PATCH v3 4/5] tests/bios-tables-test: allow setting extra machine options

2018-03-07 Thread Igor Mammedov
On Mon,  5 Mar 2018 14:57:09 +0800
Haozhong Zhang  wrote:

> Some test cases may require extra machine options than those used in
> the current test_acpi_ones(), e.g., nvdimm test cases require the
> machine option 'nvdimm=on'.
> 
> Signed-off-by: Haozhong Zhang 
> ---
>  tests/bios-tables-test.c | 45 +
>  1 file changed, 29 insertions(+), 16 deletions(-)
> 
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index 65b271a173..d45181aa51 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -654,17 +654,22 @@ static void test_smbios_structs(test_data *data)
>  }
>  }
>  
> -static void test_acpi_one(const char *params, test_data *data)
> +static void test_acpi_one(const char *extra_machine_opts,
> +  const char *params, test_data *data)
>  {
>  char *args;
>  
>  /* Disable kernel irqchip to be able to override apic irq0. */
> -args = g_strdup_printf("-machine %s,accel=%s,kernel-irqchip=off "
> +args = g_strdup_printf("-machine %s,accel=%s,kernel-irqchip=off",
> +   data->machine, "kvm:tcg");
> +if (extra_machine_opts) {
> +args = g_strdup_printf("%s,%s", args, extra_machine_opts);
> +}
> +args = g_strdup_printf("%s "
wouldn't 2nd and the rest of "args = g_strdup_printf", leak memory?

btw you don't really need this patch, you can just add
 "-machine nvdimm=on" to params argument at call site.
it's allowed to have several -FOO options on CLI, since
they are merged together and applied to machine object in QEMU.


> "-net none -display none %s "
> "-drive id=hd0,if=none,file=%s,format=raw "
> "-device ide-hd,drive=hd0 ",
> -   data->machine, "kvm:tcg",
> -   params ? params : "", disk);
> +   args, params ? params : "", disk);
>  
>  qtest_start(args);
>  
> @@ -711,7 +716,7 @@ static void test_acpi_piix4_tcg(void)
>  data.machine = MACHINE_PC;
>  data.required_struct_types = base_required_struct_types;
>  data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
> -test_acpi_one(NULL, &data);
> +test_acpi_one(NULL, NULL, &data);
>  free_test_data(&data);
>  }
>  
> @@ -724,7 +729,7 @@ static void test_acpi_piix4_tcg_bridge(void)
>  data.variant = ".bridge";
>  data.required_struct_types = base_required_struct_types;
>  data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
> -test_acpi_one("-device pci-bridge,chassis_nr=1", &data);
> +test_acpi_one(NULL, "-device pci-bridge,chassis_nr=1", &data);
>  free_test_data(&data);
>  }
>  
> @@ -736,7 +741,7 @@ static void test_acpi_q35_tcg(void)
>  data.machine = MACHINE_Q35;
>  data.required_struct_types = base_required_struct_types;
>  data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
> -test_acpi_one(NULL, &data);
> +test_acpi_one(NULL, NULL, &data);
>  free_test_data(&data);
>  }
>  
> @@ -749,7 +754,7 @@ static void test_acpi_q35_tcg_bridge(void)
>  data.variant = ".bridge";
>  data.required_struct_types = base_required_struct_types;
>  data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
> -test_acpi_one("-device pci-bridge,chassis_nr=1",
> +test_acpi_one(NULL, "-device pci-bridge,chassis_nr=1",
>&data);
>  free_test_data(&data);
>  }
> @@ -761,7 +766,8 @@ static void test_acpi_piix4_tcg_cphp(void)
>  memset(&data, 0, sizeof(data));
>  data.machine = MACHINE_PC;
>  data.variant = ".cphp";
> -test_acpi_one("-smp 2,cores=3,sockets=2,maxcpus=6"
> +test_acpi_one(NULL,
> +  "-smp 2,cores=3,sockets=2,maxcpus=6"
>" -numa node -numa node"
>" -numa dist,src=0,dst=1,val=21",
>&data);
> @@ -775,7 +781,8 @@ static void test_acpi_q35_tcg_cphp(void)
>  memset(&data, 0, sizeof(data));
>  data.machine = MACHINE_Q35;
>  data.variant = ".cphp";
> -test_acpi_one(" -smp 2,cores=3,sockets=2,maxcpus=6"
> +test_acpi_one(NULL,
> +  " -smp 2,cores=3,sockets=2,maxcpus=6"
>" -numa node -numa node"
>" -numa dist,src=0,dst=1,val=21",
>&data);
> @@ -795,7 +802,8 @@ static void test_acpi_q35_tcg_ipmi(void)
>  data.variant = ".ipmibt";
>  data.required_struct_types = ipmi_required_struct_types;
>  data.required_struct_types_len = ARRAY_SIZE(ipmi_required_struct_types);
> -test_acpi_one("-device ipmi-bmc-sim,id=bmc0"
> +test_acpi_one(NULL,
> +  "-device ipmi-bmc-sim,id=bmc0"
>" -device isa-ipmi-bt,bmc=bmc0",
>&data);
>  free_test_data(&data);
> @@ -813,7 +821,8 @@ static void test_acpi_piix4_tcg_ipmi(void)
> 

Re: [Qemu-devel] [PATCH v2] pc-bios/s390-ccw: Move string arrays from bootmap header to .c file

2018-03-07 Thread Cornelia Huck
On Tue,  6 Mar 2018 07:18:01 +0100
Thomas Huth  wrote:

> bootmap.h can currently only be included once - otherwise the linker
> complains about multiple definitions of the "magic" strings. It's a
> bad style to define string arrays in header files, so let's better
> move these to the bootmap.c file instead where they are used.
> 
> Signed-off-by: Thomas Huth 
> ---
>  v2:
>  - Removed duplicated vol_desc_magic (copy-n-paste error)
> 
>  pc-bios/s390-ccw/bootmap.c | 20 
>  pc-bios/s390-ccw/bootmap.h | 19 ---
>  2 files changed, 20 insertions(+), 19 deletions(-)

Thanks, applied (without rebuild as this is no functional change).



[Qemu-devel] BUG? Using memory after freeing

2018-03-07 Thread Aleksey Kuleshov
Hello!

Explanation of what I saw is follows.

In hw/pci/pci_bridge.c function pci_bridge_update_mappings does follows:
```
void pci_bridge_update_mappings(PCIBridge *br)
{
PCIBridgeWindows *w = br->windows;

/* Make updates atomic to: handle the case of one VCPU updating the bridge
 * while another accesses an unaffected region. */
memory_region_transaction_begin();
pci_bridge_region_del(br, br->windows);
br->windows = pci_bridge_region_init(br);
memory_region_transaction_commit();
pci_bridge_region_cleanup(br, w);
}
```

It calls memory_region_transaction_commit which calls flatview_unref:
```
static void flatview_unref(FlatView *view)
{
if (atomic_fetch_dec(&view->ref) == 1) {
trace_flatview_destroy_rcu(view, view->root);
assert(view->root);
call_rcu(view, flatview_destroy, rcu);
}
}
```
As far as I understood, call_rcu can be considered as a deferred call to 
flatview_destroy.

Then in pci_bridge_update_mappings there is a call to pci_bridge_region_cleanup 
which does:
```
static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w)
{
object_unparent(OBJECT(&w->alias_io));
object_unparent(OBJECT(&w->alias_mem));
object_unparent(OBJECT(&w->alias_pref_mem));
object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_IO_LO]));
object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_IO_HI]));
object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_MEM]));
g_free(w);
}
```
Note g_free(w). "w" holds MemoryRegions, which are part of that FlatView
which is going to be destroyed some time later in the future.

When RCU thread kicks in, flatview_destroy is called on MemoryRegions which were
part of that "w" which is now freed and QEMU seg faults.



Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] iotests: add 208 nbd-server + blockdev-snapshot-sync test case

2018-03-07 Thread Stefan Hajnoczi
On Tue, Mar 6, 2018 at 11:25 PM, Stefano Panella  wrote:
> I have applied this patch and when I run the following qmp commands I I do
> not see the crash anymore but there is still something wrong because only
> /root/a is opened from qemu. It looks like nbd-server-stop is also getting
> rid of the nodes added with blockdev-snapshot-sync, therfore is than not
> possible to do blockdev-del on /root/d because node-name is not found

Nodes are reference counted.  If nothing holds a refcount then the
node is freed.

The blockdev-add command holds a reference to the node.  The node will
stay alive until blockdev-del, which releases that reference.

blockdev-snapshot-sync does not hold a reference.  Therefore snapshot
nodes are freed once nothing is using them anymore.  When the snapshot
node is created, the users of the parent node are updated to point to
the snapshot node instead.  This is why the NBD server switches to the
snapshot mode after blockdev-snapshot-sync.

This is why the snapshot nodes disappear after the NBD server is
stopped while /root/a stays alive.

I'm not sure if the current blockdev-snapshot-sync behavior is useful.
Perhaps the presence of the "snapshot-node-name" argument should cause
the snapshot node to be treated as monitor-owned, just like
blockdev-add.  This would introduce leaks for existing QMP clients
though, so it may be necessary to add yet another argument for this
behavior.

Anyway, I hope this explains the current behavior.  I don't see a
problem with it, but it's something the API users need to be aware of.

If it is a problem for your use case, please explain what you are trying to do.

Stefan



Re: [Qemu-devel] [PATCH v1 1/1] iotests: bypass s390x for case 200

2018-03-07 Thread QingFeng Hao



在 2018/3/6 15:56, Christian Borntraeger 写道:

Nack. This will be fixed by

s390/ipl: only print boot menu error if -boot menu=on was specified

You are right. After I applied that patch, the case is passed.
Please ignore this patch. Thanks


On 03/06/2018 08:54 AM, QingFeng Hao wrote:

In s390x, the case 200 failed as:
  === Starting QEMU VM ===

+QEMU_PROG: boot menu is not supported for this device type.
  {"return": {}}

  === Sending stream/cancel, checking for SIGSEGV only ===
Failures: 200
Failed 1 of 1 tests

It was caused by the command which isn't supported by s390x now:
qemu-system-s390x -device pci-bridge,id=bridge1,chassis_nr=1,bus=pci.0 -object 
iothread,id=iothread0 -device 
virtio-scsi-pci,bus=bridge1,addr=0x1f,id=scsi0,iothread=iothread0 -drive 
file=.../scratch/test.img,media=disk,if=none,cache=writeback,id=drive_sysdisk,format=qcow2
 -device scsi-hd,drive=drive_sysdisk,bus=scsi0.0,id=sysdisk,bootindex=0 
-nographic

Signed-off-by: QingFeng Hao 
---
  tests/qemu-iotests/200 | 4 
  1 file changed, 4 insertions(+)

diff --git a/tests/qemu-iotests/200 b/tests/qemu-iotests/200
index ddbdedc476..7e53bd7774 100755
--- a/tests/qemu-iotests/200
+++ b/tests/qemu-iotests/200
@@ -45,6 +45,10 @@ _supported_fmt qcow2 qed
  _supported_proto file
  _supported_os Linux

+if [ "$QEMU_DEFAULT_MACHINE" != "pc" ]; then
+_notrun "Requires a PC machine"
+fi
+
  BACKING_IMG="${TEST_DIR}/backing.img"
  TEST_IMG="${TEST_DIR}/test.img"



--
Regards
QingFeng Hao




[Qemu-devel] [PATCH v10 04/24] migration: Set the migration tcp port

2018-03-07 Thread Juan Quintela
We can set the port parameter as zero.  This patch lets us know what
port the system was choosen for us.  Now we can migrate to this place.

Signed-off-by: Juan Quintela 

--

This was migrate_set_uri(), but as we only need the tcp_port, change
to that one.
---
 migration/migration.c | 10 ++
 migration/migration.h |  2 ++
 migration/socket.c| 35 ++-
 3 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 31b16a335b..c398665de7 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -268,6 +268,16 @@ int migrate_send_rp_req_pages(MigrationIncomingState *mis, 
const char *rbname,
 return migrate_send_rp_message(mis, msg_type, msglen, bufc);
 }
 
+void migrate_set_port(const uint16_t port, Error **errp)
+{
+MigrateSetParameters p = {
+.has_x_tcp_port = true,
+.x_tcp_port = port,
+};
+
+qmp_migrate_set_parameters(&p, errp);
+}
+
 void qemu_start_incoming_migration(const char *uri, Error **errp)
 {
 const char *p;
diff --git a/migration/migration.h b/migration/migration.h
index 08c5d2ded1..f40014cf94 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -234,4 +234,6 @@ void migrate_send_rp_pong(MigrationIncomingState *mis,
 int migrate_send_rp_req_pages(MigrationIncomingState *mis, const char* rbname,
   ram_addr_t start, size_t len);
 
+void migrate_set_port(const uint16_t port, Error **errp);
+
 #endif
diff --git a/migration/socket.c b/migration/socket.c
index e090097077..08606c665d 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -15,6 +15,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 
 #include "qemu-common.h"
 #include "qemu/error-report.h"
@@ -160,17 +161,24 @@ out:
 }
 
 
-static void socket_start_incoming_migration(SocketAddress *saddr,
-Error **errp)
+static SocketAddress *socket_start_incoming_migration(SocketAddress *saddr,
+  Error **errp)
 {
 QIOChannelSocket *listen_ioc = qio_channel_socket_new();
+SocketAddress *address;
 
 qio_channel_set_name(QIO_CHANNEL(listen_ioc),
  "migration-socket-listener");
 
 if (qio_channel_socket_listen_sync(listen_ioc, saddr, errp) < 0) {
 object_unref(OBJECT(listen_ioc));
-return;
+return NULL;
+}
+
+address = qio_channel_socket_get_local_address(listen_ioc, errp);
+if (address < 0) {
+object_unref(OBJECT(listen_ioc));
+return NULL;
 }
 
 qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
@@ -178,14 +186,28 @@ static void socket_start_incoming_migration(SocketAddress 
*saddr,
   socket_accept_incoming_migration,
   listen_ioc,
   (GDestroyNotify)object_unref);
+return address;
 }
 
 void tcp_start_incoming_migration(const char *host_port, Error **errp)
 {
 Error *err = NULL;
 SocketAddress *saddr = tcp_build_address(host_port, &err);
+
 if (!err) {
-socket_start_incoming_migration(saddr, &err);
+SocketAddress *address = socket_start_incoming_migration(saddr, &err);
+
+if (address) {
+unsigned long long port;
+
+if (parse_uint_full(address->u.inet.port, &port, 10) < 0) {
+error_setg(errp, "error parsing port in '%s'",
+   address->u.inet.port);
+} else {
+migrate_set_port(port, errp);
+}
+qapi_free_SocketAddress(address);
+}
 }
 qapi_free_SocketAddress(saddr);
 error_propagate(errp, err);
@@ -194,6 +216,9 @@ void tcp_start_incoming_migration(const char *host_port, 
Error **errp)
 void unix_start_incoming_migration(const char *path, Error **errp)
 {
 SocketAddress *saddr = unix_build_address(path);
-socket_start_incoming_migration(saddr, errp);
+SocketAddress *address;
+
+address = socket_start_incoming_migration(saddr, errp);
+qapi_free_SocketAddress(address);
 qapi_free_SocketAddress(saddr);
 }
-- 
2.14.3




[Qemu-devel] [PATCH v10 09/24] migration: Set error state in case of error

2018-03-07 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 migration/ram.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/migration/ram.c b/migration/ram.c
index 3b6c077964..4a56a85d53 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -414,6 +414,16 @@ static void terminate_multifd_send_threads(Error *errp)
 {
 int i;
 
+if (errp) {
+MigrationState *s = migrate_get_current();
+migrate_set_error(s, errp);
+if (s->state == MIGRATION_STATUS_SETUP ||
+s->state == MIGRATION_STATUS_ACTIVE) {
+migrate_set_state(&s->state, s->state,
+  MIGRATION_STATUS_FAILED);
+}
+}
+
 for (i = 0; i < multifd_send_state->count; i++) {
 MultiFDSendParams *p = &multifd_send_state->params[i];
 
@@ -514,6 +524,16 @@ static void terminate_multifd_recv_threads(Error *errp)
 {
 int i;
 
+if (errp) {
+MigrationState *s = migrate_get_current();
+migrate_set_error(s, errp);
+if (s->state == MIGRATION_STATUS_SETUP ||
+s->state == MIGRATION_STATUS_ACTIVE) {
+migrate_set_state(&s->state, s->state,
+  MIGRATION_STATUS_FAILED);
+}
+}
+
 for (i = 0; i < multifd_recv_state->count; i++) {
 MultiFDRecvParams *p = &multifd_recv_state->params[i];
 
-- 
2.14.3




[Qemu-devel] [PATCH v10 12/24] migration: Reference counting recv channels correctly

2018-03-07 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 migration/socket.c | 11 +++
 migration/socket.h |  7 +++
 2 files changed, 18 insertions(+)

diff --git a/migration/socket.c b/migration/socket.c
index b12b0a462e..26110739cf 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -27,6 +27,17 @@
 #include "io/channel-socket.h"
 #include "trace.h"
 
+int socket_recv_channel_ref(QIOChannel *recv)
+{
+object_ref(OBJECT(recv));
+return 0;
+}
+
+int socket_recv_channel_unref(QIOChannel *recv)
+{
+object_unref(OBJECT(recv));
+return 0;
+}
 
 static SocketAddress *tcp_build_address(const char *host_port, Error **errp)
 {
diff --git a/migration/socket.h b/migration/socket.h
index 6b91e9db38..638a85255a 100644
--- a/migration/socket.h
+++ b/migration/socket.h
@@ -16,6 +16,13 @@
 
 #ifndef QEMU_MIGRATION_SOCKET_H
 #define QEMU_MIGRATION_SOCKET_H
+
+#include "io/channel.h"
+#include "io/task.h"
+
+int socket_recv_channel_ref(QIOChannel *recv);
+int socket_recv_channel_unref(QIOChannel *recv);
+
 void tcp_start_incoming_migration(const char *host_port, Error **errp);
 
 void tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
-- 
2.14.3




[Qemu-devel] [PATCH v10 06/24] tests: Add basic migration precopy tcp test

2018-03-07 Thread Juan Quintela
Not sharing code from precopy/unix because we have to read back the
tcp parameter.

Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Peter Xu 
---
 tests/migration-test.c | 58 +++---
 1 file changed, 55 insertions(+), 3 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index a8db55ef83..6b718bb5dd 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -277,8 +277,7 @@ static void cleanup(const char *filename)
 g_free(path);
 }
 
-static void migrate_check_parameter(QTestState *who, const char *parameter,
-const char *value)
+static char *migrate_get_parameter(QTestState *who, const char *parameter)
 {
 QDict *rsp, *rsp_return;
 char *result;
@@ -287,9 +286,18 @@ static void migrate_check_parameter(QTestState *who, const 
char *parameter,
 rsp_return = qdict_get_qdict(rsp, "return");
 result = g_strdup_printf("%" PRId64,
  qdict_get_try_int(rsp_return,  parameter, -1));
+QDECREF(rsp);
+return result;
+}
+
+static void migrate_check_parameter(QTestState *who, const char *parameter,
+const char *value)
+{
+char *result;
+
+result = migrate_get_parameter(who, parameter);
 g_assert_cmpstr(result, ==, value);
 g_free(result);
-QDECREF(rsp);
 }
 
 static void migrate_set_parameter(QTestState *who, const char *parameter,
@@ -655,6 +663,49 @@ static void test_xbzrle_unix(void)
 g_free(uri);
 }
 
+static void test_precopy_tcp(void)
+{
+char *uri;
+char *port;
+QTestState *from, *to;
+
+test_migrate_start(&from, &to, "tcp:127.0.0.1:0");
+
+/* We want to pick a speed slow enough that the test completes
+ * quickly, but that it doesn't complete precopy even on a slow
+ * machine, so also set the downtime.
+ */
+/* 1 ms should make it not converge*/
+migrate_set_parameter(from, "downtime-limit", "1");
+/* 1GB/s */
+migrate_set_parameter(from, "max-bandwidth", "10");
+
+/* Wait for the first serial output from the source */
+wait_for_serial("src_serial");
+
+port = migrate_get_parameter(to, "x-tcp-port");
+uri = g_strdup_printf("tcp:127.0.0.1:%s", port);
+
+migrate(from, uri);
+
+wait_for_migration_pass(from);
+
+/* 300ms should converge */
+migrate_set_parameter(from, "downtime-limit", "300");
+
+if (!got_stop) {
+qtest_qmp_eventwait(from, "STOP");
+}
+qtest_qmp_eventwait(to, "RESUME");
+
+wait_for_serial("dest_serial");
+wait_for_migration_complete(from);
+
+test_migrate_end(from, to, true);
+g_free(uri);
+g_free(port);
+}
+
 int main(int argc, char **argv)
 {
 char template[] = "/tmp/migration-test-XX";
@@ -678,6 +729,7 @@ int main(int argc, char **argv)
 qtest_add_func("/migration/deprecated", test_deprecated);
 qtest_add_func("/migration/bad_dest", test_baddest);
 qtest_add_func("/migration/precopy/unix", test_precopy_unix);
+qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
 qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
 
 ret = g_test_run();
-- 
2.14.3




[Qemu-devel] [PATCH v10 05/24] tests: Migration ppc now inlines its program

2018-03-07 Thread Juan Quintela
No need to write it to a file.  Just need a proper firmware O:-)

Signed-off-by: Juan Quintela 
CC: Laurent Vivier 
---
 tests/migration-test.c | 41 +
 1 file changed, 5 insertions(+), 36 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index fb67a88353..a8db55ef83 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -19,9 +19,6 @@
 #include "qemu/sockets.h"
 #include "chardev/char.h"
 #include "sysemu/sysemu.h"
-#include "hw/nvram/chrp_nvram.h"
-
-#define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */
 
 const unsigned start_address = 1024 * 1024;
 const unsigned end_address = 100 * 1024 * 1024;
@@ -90,36 +87,6 @@ static void init_bootfile_x86(const char *bootpath)
 fclose(bootfile);
 }
 
-static void init_bootfile_ppc(const char *bootpath)
-{
-FILE *bootfile;
-char buf[MIN_NVRAM_SIZE];
-ChrpNvramPartHdr *header = (ChrpNvramPartHdr *)buf;
-
-memset(buf, 0, MIN_NVRAM_SIZE);
-
-/* Create a "common" partition in nvram to store boot-command property */
-
-header->signature = CHRP_NVPART_SYSTEM;
-memcpy(header->name, "common", 6);
-chrp_nvram_finish_partition(header, MIN_NVRAM_SIZE);
-
-/* FW_MAX_SIZE is 4MB, but slof.bin is only 900KB,
- * so let's modify memory between 1MB and 100MB
- * to do like PC bootsector
- */
-
-sprintf(buf + 16,
-"boot-command=hex .\" _\" begin %x %x do i c@ 1 + i c! 1000 +loop "
-".\" B\" 0 until", end_address, start_address);
-
-/* Write partition to the NVRAM file */
-
-bootfile = fopen(bootpath, "wb");
-g_assert_cmpint(fwrite(buf, MIN_NVRAM_SIZE, 1, bootfile), ==, 1);
-fclose(bootfile);
-}
-
 /*
  * Wait for some output in the serial output file,
  * we get an 'A' followed by an endless string of 'B's
@@ -410,12 +377,14 @@ static void test_migrate_start(QTestState **from, 
QTestState **to,
 if (access("/sys/module/kvm_hv", F_OK)) {
 accel = "tcg";
 }
-init_bootfile_ppc(bootpath);
 cmd_src = g_strdup_printf("-machine accel=%s -m 256M"
   " -name source,debug-threads=on"
   " -serial file:%s/src_serial"
-  " -drive file=%s,if=pflash,format=raw",
-  accel, tmpfs, bootpath);
+  " -prom-env '"
+  "boot-command=hex .\" _\" begin %x %x "
+  "do i c@ 1 + i c! 1000 +loop .\" B\" 0 "
+  "until'",  accel, tmpfs, end_address,
+  start_address);
 cmd_dst = g_strdup_printf("-machine accel=%s -m 256M"
   " -name target,debug-threads=on"
   " -serial file:%s/dest_serial"
-- 
2.14.3




[Qemu-devel] [RFC v10 00/24] Multifd

2018-03-07 Thread Juan Quintela

Hi  this is the v10 version of the multifd patches,

Lots of changes from previous versions:
a - everything is sent now through the multifd channels, nothing is sent 
through main channel
b - locking is band new, I was getting into a hole with the previous approach, 
right now, there is a single way to
do locking (both source and destination)
   main thread : sets a ->sync variable for each thread and wakeps it
   multifd threads: clean the variable and signal (sem) back to main thread

using this for either:
- all threads have started
- we need to synchronize after each round through memory
- all threads have finished

c - I have to use a qio watcher for a thread to wait for ready data to read

d - lots of cleanups

e - to make things easier, I have included the missing tests stuff on
this round of patches, because they build on top of them

f - lots of traces, it is now much easier to follow what is happening

Now, why it is an RFC:

- in the last patch, there is still race between the whatcher, the
  ->quit of the threads and the last synchronization.  Techinically they
  are done in oder, but in practice, they are hanging sometimes.

- I *know* I can optimize the synchronization of the threads sending
  the "we start a new round" through the multifd channels, have to add a flag 
here.

- Not having a thread on the incoming side  is a mess, I can't block waiting 
for things to happen :-(

- When doing the synchronization, I need to optimize the sending of the "not 
finished packet" of pages, working on that.

please, take a look and review.

Thanks, Juan.

[v9]

This series is on top of my migration test series just sent, only reject should 
be on the test system, though.

On v9 series for you:
- qobject_unref() as requested by dan

  Yes he was right, I had a reference leak for _non_ multifd, I
  *thought* he mean for multifd, and that took a while to understand
  (and then find when/where).

- multifd page count: it is dropped for good
- uuid handling: we use the default qemu uuid of ...
- uuid handling: using and struct and sending the struct
  * idea is to add a size field and add more parameter after that
  * anyone has a good idea how to "ouptut" info
migrate_capabilities/parameters json into a string and how to read it back?
- changed how we test that all threads/channels are already created.
  Should be more robust.
- Add tests multifd.  Still not ported on top of migration-tests series sent 
early
  waiting for review on the ideas there.
- Rebase and remove al the integrated patches (back at 12)

Please, review.

Later, Juan.

[v8]
Things NOT done yet:

- drop x-multifd-page-count?  We can use performance to set a default value
- paolo suggestion of not having a control channel
  needs iyet more cleanups to be able to have more than one ramstate, trying it.
- still not performance done, but it has been very stable

On v8:
- use connect_async
- rename multifd-group to multifd-page-count (danp suggestion)
- rename multifd-threads to multifd-channels (danp suggestion)
- use new qio*channel functions
- Address rest of comments left


So, please review.

My idea will be to pull this changes and continue performance changes
for inside, basically everything is already reviewed.

Thanks, Juan.

On v7:
- tests fixed as danp wanted
- have to revert danp qio_*_all patches, as they break multifd, I have to 
investigate why.
- error_abort is gone.  After several tries about getting errors, I ended 
having a single error
  proceted by a lock and first error wins.
- Addressed basically all reviews (see on ToDo)
- Pointers to struct are done now
- fix lots of leaks
- lots of small fixes


[v6]
- Improve migration_ioc_porcess_incoming
- teach about G_SOURCE_REMOVE/CONTINUE
- Add test for migration_has_all_channels
- use DEFIN_PROP*
- change recv_state to use pointers to parameters
  make easier to receive channels out of order
- use g_strdup_printf()
- improve count of threads to know when we have to finish
- report channel id's on errors
- Use last_page parameter for multifd_send_page() sooner
- Improve commets for address
- use g_new0() instead of g_malloc()
- create MULTIFD_CONTINUE instead of using UINT16_MAX
- clear memory used by group of pages
  once there, pass everything to the global state variables instead of being
  local to the function.  This way it works if we cancel migration and start
  a new one
- Really wait to create the migration_thread until all channels are created
- split initial_bytes setup to make clearer following patches.
- createRAM_SAVE_FLAG_MULTIFD_SYNC macro, to make clear what we are doing
- move setting of need_flush to inside bitmap_sync
- Lots of other small changes & reorderings

Please, comment.


[v5]

- tests from qio functions (a.k.a. make danp happy)
- 1st message from one channel to the other contains:
multifd 
   This would allow us to create more channels as we want them.
   a.k.a. Making dave happy
- Waiting in reception 

[Qemu-devel] [PATCH v10 03/24] migration: Create tcp_port parameter

2018-03-07 Thread Juan Quintela
It will be used to store the uri tcp_port parameter.  This is the only
parameter than can change and we can need to be able to connect to it.

Signed-off-by: Juan Quintela 

--

This used to be uri parameter, but it has so many troubles to
reproduce that it don't just make sense.
---
 hmp.c |  3 +++
 migration/migration.c |  8 
 qapi/migration.json   | 19 ---
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/hmp.c b/hmp.c
index 016cb5c4f1..b37605d86a 100644
--- a/hmp.c
+++ b/hmp.c
@@ -355,6 +355,9 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict 
*qdict)
 monitor_printf(mon, "%s: %" PRIu64 "\n",
 MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
 params->xbzrle_cache_size);
+monitor_printf(mon, "%s: %d\n",
+MigrationParameter_str(MIGRATION_PARAMETER_X_TCP_PORT),
+params->x_tcp_port);
 }
 
 qapi_free_MigrationParameters(params);
diff --git a/migration/migration.c b/migration/migration.c
index e345d0cc7e..31b16a335b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -545,6 +545,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error 
**errp)
 params->x_multifd_page_count = s->parameters.x_multifd_page_count;
 params->has_xbzrle_cache_size = true;
 params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
+params->has_x_tcp_port = true;
+params->x_tcp_port = s->parameters.x_tcp_port;
 
 return params;
 }
@@ -912,6 +914,9 @@ static void migrate_params_test_apply(MigrateSetParameters 
*params,
 if (params->has_xbzrle_cache_size) {
 dest->xbzrle_cache_size = params->xbzrle_cache_size;
 }
+if (params->has_x_tcp_port) {
+dest->x_tcp_port = params->x_tcp_port;
+}
 }
 
 static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
@@ -984,6 +989,9 @@ static void migrate_params_apply(MigrateSetParameters 
*params, Error **errp)
 s->parameters.xbzrle_cache_size = params->xbzrle_cache_size;
 xbzrle_cache_resize(params->xbzrle_cache_size, errp);
 }
+if (params->has_x_tcp_port) {
+s->parameters.x_tcp_port = params->x_tcp_port;
+}
 }
 
 void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
diff --git a/qapi/migration.json b/qapi/migration.json
index 7f465a1902..b6ef193f47 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -490,6 +490,9 @@
 # and a power of 2
 # (Since 2.11)
 #
+# @x-tcp-port: Only used for tcp, to know what the real port is
+# (Since 2.12)
+#
 # Since: 2.4
 ##
 { 'enum': 'MigrationParameter',
@@ -498,7 +501,7 @@
'tls-creds', 'tls-hostname', 'max-bandwidth',
'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
'x-multifd-channels', 'x-multifd-page-count',
-   'xbzrle-cache-size' ] }
+   'xbzrle-cache-size', 'x-tcp-port' ] }
 
 ##
 # @MigrateSetParameters:
@@ -566,6 +569,10 @@
 # needs to be a multiple of the target page size
 # and a power of 2
 # (Since 2.11)
+#
+# @x-tcp-port: Only used for tcp, to know what the real port is
+# (Since 2.12)
+#
 # Since: 2.4
 ##
 # TODO either fuse back into MigrationParameters, or make
@@ -584,7 +591,8 @@
 '*block-incremental': 'bool',
 '*x-multifd-channels': 'int',
 '*x-multifd-page-count': 'int',
-'*xbzrle-cache-size': 'size' } }
+'*xbzrle-cache-size': 'size',
+'*x-tcp-port': 'uint16'} }
 
 ##
 # @migrate-set-parameters:
@@ -667,6 +675,10 @@
 # needs to be a multiple of the target page size
 # and a power of 2
 # (Since 2.11)
+#
+# @x-tcp-port: Only used for tcp, to know what the real port is
+# (Since 2.12)
+#
 # Since: 2.4
 ##
 { 'struct': 'MigrationParameters',
@@ -683,7 +695,8 @@
 '*block-incremental': 'bool' ,
 '*x-multifd-channels': 'uint8',
 '*x-multifd-page-count': 'uint32',
-'*xbzrle-cache-size': 'size' } }
+'*xbzrle-cache-size': 'size',
+'*x-tcp-port': 'uint16'} }
 
 ##
 # @query-migrate-parameters:
-- 
2.14.3




[Qemu-devel] [PATCH v10 14/24] migration: Be sure all recv channels are created

2018-03-07 Thread Juan Quintela
We need them before we start migration.

Signed-off-by: Juan Quintela 
---
 migration/migration.c |  6 +-
 migration/ram.c   | 11 +++
 migration/ram.h   |  1 +
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index 919343232e..c06c34ca0f 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -461,7 +461,11 @@ void migration_ioc_process_incoming(QIOChannel *ioc)
  */
 bool migration_has_all_channels(void)
 {
-return true;
+bool all_channels;
+
+all_channels = multifd_recv_all_channels_created();
+
+return all_channels;
 }
 
 /*
diff --git a/migration/ram.c b/migration/ram.c
index f48c74585f..e502be5dda 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -625,6 +625,17 @@ int multifd_load_setup(void)
 return 0;
 }
 
+bool multifd_recv_all_channels_created(void)
+{
+int thread_count = migrate_multifd_channels();
+
+if (!migrate_use_multifd()) {
+return true;
+}
+
+return thread_count == atomic_read(&multifd_recv_state->count);
+}
+
 void multifd_recv_new_channel(QIOChannel *ioc)
 {
 /* nothing to do yet */
diff --git a/migration/ram.h b/migration/ram.h
index a2031acf59..3daf074bcc 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -45,6 +45,7 @@ int multifd_save_setup(void);
 int multifd_save_cleanup(Error **errp);
 int multifd_load_setup(void);
 int multifd_load_cleanup(Error **errp);
+bool multifd_recv_all_channels_created(void);
 void multifd_recv_new_channel(QIOChannel *ioc);
 
 uint64_t ram_pagesize_summary(void);
-- 
2.14.3




[Qemu-devel] [PATCH v10 07/24] [RFH] tests: Add migration compress threads tests

2018-03-07 Thread Juan Quintela
Yeap, it is still not working. trying to learn how to debug threads
for guests running from the testt hardness.

For some reason, compression is not working at the moment, test is
disabled until I found why.

Signed-off-by: Juan Quintela 
---
 tests/migration-test.c | 52 ++
 1 file changed, 52 insertions(+)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 6b718bb5dd..6f9b4c8d7a 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -706,6 +706,55 @@ static void test_precopy_tcp(void)
 g_free(port);
 }
 
+static void test_compress(const char *uri)
+{
+QTestState *from, *to;
+
+test_migrate_start(&from, &to, uri);
+
+/* We want to pick a speed slow enough that the test completes
+ * quickly, but that it doesn't complete precopy even on a slow
+ * machine, so also set the downtime.
+ */
+/* 1 ms should make it not converge*/
+migrate_set_parameter(from, "downtime-limit", "1");
+/* 1GB/s */
+migrate_set_parameter(from, "max-bandwidth", "10");
+
+migrate_set_parameter(from, "compress-threads", "4");
+migrate_set_parameter(to, "decompress-threads", "3");
+
+migrate_set_capability(from, "compress", "true");
+migrate_set_capability(to, "compress", "true");
+/* Wait for the first serial output from the source */
+wait_for_serial("src_serial");
+
+migrate(from, uri);
+
+wait_for_migration_pass(from);
+
+/* 300ms should converge */
+migrate_set_parameter(from, "downtime-limit", "300");
+
+if (!got_stop) {
+qtest_qmp_eventwait(from, "STOP");
+}
+qtest_qmp_eventwait(to, "RESUME");
+
+wait_for_serial("dest_serial");
+wait_for_migration_complete(from);
+
+test_migrate_end(from, to, true);
+}
+
+static void test_compress_unix(void)
+{
+char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
+
+test_compress(uri);
+g_free(uri);
+}
+
 int main(int argc, char **argv)
 {
 char template[] = "/tmp/migration-test-XX";
@@ -731,6 +780,9 @@ int main(int argc, char **argv)
 qtest_add_func("/migration/precopy/unix", test_precopy_unix);
 qtest_add_func("/migration/precopy/tcp", test_precopy_tcp);
 qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
+if (0) {
+qtest_add_func("/migration/compress/unix", test_compress_unix);
+}
 
 ret = g_test_run();
 
-- 
2.14.3




[Qemu-devel] [PATCH v10 20/24] migration: Delay start of migration main routines

2018-03-07 Thread Juan Quintela
We need to make sure that we have started all the multifd threads.

Signed-off-by: Juan Quintela 
---
 migration/migration.c | 4 ++--
 migration/migration.h | 1 +
 migration/ram.c   | 3 +++
 migration/socket.c| 3 +++
 4 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index c06c34ca0f..a355618220 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -429,7 +429,7 @@ static void migration_incoming_setup(QEMUFile *f)
 qemu_file_set_blocking(f, false);
 }
 
-static void migration_incoming_process(void)
+void migration_incoming_process(void)
 {
 Coroutine *co = qemu_coroutine_create(process_incoming_migration_co, NULL);
 qemu_coroutine_enter(co);
@@ -447,7 +447,7 @@ void migration_ioc_process_incoming(QIOChannel *ioc)
 
 if (!mis->from_src_file) {
 QEMUFile *f = qemu_fopen_channel_input(ioc);
-migration_fd_process_incoming(f);
+migration_incoming_setup(f);
 return;
 }
 multifd_recv_new_channel(ioc);
diff --git a/migration/migration.h b/migration/migration.h
index f40014cf94..03a940831d 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -184,6 +184,7 @@ void migrate_set_state(int *state, int old_state, int 
new_state);
 
 void migration_fd_process_incoming(QEMUFile *f);
 void migration_ioc_process_incoming(QIOChannel *ioc);
+void migration_incoming_process(void);
 
 bool  migration_has_all_channels(void);
 
diff --git a/migration/ram.c b/migration/ram.c
index 7ef0c2b7e2..1aab96bd5e 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -743,6 +743,9 @@ void multifd_recv_new_channel(QIOChannel *ioc)
 qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
QEMU_THREAD_JOINABLE);
 atomic_inc(&multifd_recv_state->count);
+if (multifd_recv_state->count == migrate_multifd_channels()) {
+migration_incoming_process();
+}
 }
 
 /**
diff --git a/migration/socket.c b/migration/socket.c
index b3b5571ebb..deda193de7 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -189,6 +189,9 @@ static gboolean socket_accept_incoming_migration(QIOChannel 
*ioc,
 if (migration_has_all_channels()) {
 /* Close listening socket as its no longer needed */
 qio_channel_close(ioc, NULL);
+if (!migrate_use_multifd()) {
+migration_incoming_process();
+}
 return G_SOURCE_REMOVE;
 } else {
 return G_SOURCE_CONTINUE;
-- 
2.14.3




[Qemu-devel] [PATCH v10 08/24] migration: Add multifd test

2018-03-07 Thread Juan Quintela
We set the x-multifd-page-count and x-multifd-channels.

Signed-off-by: Juan Quintela 
---
 tests/migration-test.c | 50 ++
 1 file changed, 50 insertions(+)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 6f9b4c8d7a..97d35f979d 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -755,6 +755,55 @@ static void test_compress_unix(void)
 g_free(uri);
 }
 
+static void test_multifd_tcp(void)
+{
+char *uri;
+char *port;
+QTestState *from, *to;
+
+test_migrate_start(&from, &to, "tcp:127.0.0.1:0");
+
+/* We want to pick a speed slow enough that the test completes
+ * quickly, but that it doesn't complete precopy even on a slow
+ * machine, so also set the downtime.
+ */
+/* 1 ms should make it not converge*/
+migrate_set_parameter(from, "downtime-limit", "1");
+/* 1GB/s */
+migrate_set_parameter(from, "max-bandwidth", "10");
+
+migrate_set_parameter(from, "x-multifd-channels", "4");
+migrate_set_parameter(to, "x-multifd-channels", "4");
+
+migrate_set_parameter(from, "x-multifd-page-count", "64");
+migrate_set_parameter(to, "x-multifd-page-count", "64");
+
+migrate_set_capability(from, "x-multifd", "true");
+migrate_set_capability(to, "x-multifd", "true");
+/* Wait for the first serial output from the source */
+wait_for_serial("src_serial");
+
+port = migrate_get_parameter(to, "x-tcp-port");
+uri = g_strdup_printf("tcp:127.0.0.1:%s", port);
+
+migrate(from, uri);
+
+wait_for_migration_pass(from);
+
+/* 300ms it should converge */
+migrate_set_parameter(from, "downtime-limit", "300");
+
+if (!got_stop) {
+qtest_qmp_eventwait(from, "STOP");
+}
+qtest_qmp_eventwait(to, "RESUME");
+
+wait_for_serial("dest_serial");
+wait_for_migration_complete(from);
+
+test_migrate_end(from, to, true);
+}
+
 int main(int argc, char **argv)
 {
 char template[] = "/tmp/migration-test-XX";
@@ -783,6 +832,7 @@ int main(int argc, char **argv)
 if (0) {
 qtest_add_func("/migration/compress/unix", test_compress_unix);
 }
+qtest_add_func("/migration/multifd/tcp", test_multifd_tcp);
 
 ret = g_test_run();
 
-- 
2.14.3




[Qemu-devel] [PATCH v10 16/24] migration: Synchronize recv threads

2018-03-07 Thread Juan Quintela
We synchronize all threads each RAM_SAVE_FLAG_EOS.  Bitmap
synchronizations don't happen inside a  ram section, so we are safe
about two channels trying to overwrite the same memory.

Signed-off-by: Juan Quintela 
---
 migration/ram.c| 37 -
 migration/trace-events |  1 +
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index 153c7560cb..0266bd200c 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -551,6 +551,7 @@ struct MultiFDRecvParams {
 QemuMutex mutex;
 bool running;
 bool quit;
+bool sync;
 };
 typedef struct MultiFDRecvParams MultiFDRecvParams;
 
@@ -558,6 +559,8 @@ struct {
 MultiFDRecvParams *params;
 /* number of created threads */
 int count;
+/* syncs main thread and channels */
+QemuSemaphore sem_main;
 } *multifd_recv_state;
 
 static void terminate_multifd_recv_threads(Error *errp)
@@ -605,6 +608,7 @@ int multifd_load_cleanup(Error **errp)
 g_free(p->name);
 p->name = NULL;
 }
+qemu_sem_destroy(&multifd_recv_state->sem_main);
 g_free(multifd_recv_state->params);
 multifd_recv_state->params = NULL;
 g_free(multifd_recv_state);
@@ -613,18 +617,45 @@ int multifd_load_cleanup(Error **errp)
 return ret;
 }
 
+static void multifd_recv_sync_main(void)
+{
+int i;
+
+if (!migrate_use_multifd()) {
+return;
+}
+for (i = 0; i < migrate_multifd_channels(); i++) {
+MultiFDRecvParams *p = &multifd_recv_state->params[i];
+
+qemu_mutex_lock(&p->mutex);
+p->sync = true;
+qemu_mutex_unlock(&p->mutex);
+qemu_sem_post(&p->sem);
+}
+for (i = 0; i < migrate_multifd_channels(); i++) {
+qemu_sem_wait(&multifd_recv_state->sem_main);
+}
+trace_multifd_recv_sync_main();
+}
+
 static void *multifd_recv_thread(void *opaque)
 {
 MultiFDRecvParams *p = opaque;
 
 while (true) {
+qemu_sem_wait(&p->sem);
 qemu_mutex_lock(&p->mutex);
+if (p->sync) {
+p->sync = false;
+qemu_mutex_unlock(&p->mutex);
+qemu_sem_post(&multifd_recv_state->sem_main);
+continue;
+}
 if (p->quit) {
 qemu_mutex_unlock(&p->mutex);
 break;
 }
 qemu_mutex_unlock(&p->mutex);
-qemu_sem_wait(&p->sem);
 }
 
 return NULL;
@@ -642,6 +673,7 @@ int multifd_load_setup(void)
 multifd_recv_state = g_malloc0(sizeof(*multifd_recv_state));
 multifd_recv_state->params = g_new0(MultiFDRecvParams, thread_count);
 atomic_set(&multifd_recv_state->count, 0);
+qemu_sem_init(&multifd_recv_state->sem_main, 0);
 for (i = 0; i < thread_count; i++) {
 MultiFDRecvParams *p = &multifd_recv_state->params[i];
 
@@ -655,6 +687,7 @@ int multifd_load_setup(void)
QEMU_THREAD_JOINABLE);
 atomic_inc(&multifd_recv_state->count);
 }
+
 return 0;
 }
 
@@ -2868,6 +2901,7 @@ static int ram_load_postcopy(QEMUFile *f)
 break;
 case RAM_SAVE_FLAG_EOS:
 /* normal exit */
+multifd_recv_sync_main();
 break;
 default:
 error_report("Unknown combination of migration flags: %#x"
@@ -3053,6 +3087,7 @@ static int ram_load(QEMUFile *f, void *opaque, int 
version_id)
 break;
 case RAM_SAVE_FLAG_EOS:
 /* normal exit */
+multifd_recv_sync_main();
 break;
 default:
 if (flags & RAM_SAVE_FLAG_HOOK) {
diff --git a/migration/trace-events b/migration/trace-events
index 97b5ac564f..76075c26bc 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -78,6 +78,7 @@ ram_postcopy_send_discard_bitmap(void) ""
 ram_save_page(const char *rbname, uint64_t offset, void *host) "%s: offset: 
0x%" PRIx64 " host: %p"
 ram_save_queue_pages(const char *rbname, size_t start, size_t len) "%s: start: 
0x%zx len: 0x%zx"
 multifd_send_sync_main(void) ""
+multifd_recv_sync_main(void) ""
 
 # migration/migration.c
 await_return_path_close_on_source_close(void) ""
-- 
2.14.3




[Qemu-devel] [PATCH v10 10/24] migration: In case of error just end the migration

2018-03-07 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 migration/socket.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/migration/socket.c b/migration/socket.c
index 08606c665d..b12b0a462e 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -139,9 +139,8 @@ static gboolean socket_accept_incoming_migration(QIOChannel 
*ioc,
 sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc),
  &err);
 if (!sioc) {
-error_report("could not accept migration connection (%s)",
- error_get_pretty(err));
-goto out;
+migrate_set_error(migrate_get_current(), err);
+return G_SOURCE_REMOVE;
 }
 
 trace_migration_socket_incoming_accepted();
@@ -150,7 +149,6 @@ static gboolean socket_accept_incoming_migration(QIOChannel 
*ioc,
 migration_channel_process_incoming(QIO_CHANNEL(sioc));
 object_unref(OBJECT(sioc));
 
-out:
 if (migration_has_all_channels()) {
 /* Close listening socket as its no longer needed */
 qio_channel_close(ioc, NULL);
-- 
2.14.3




[Qemu-devel] [PATCH v10 23/24] migration: Create pages structure for reception

2018-03-07 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 migration/ram.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/migration/ram.c b/migration/ram.c
index df9646ed2e..264d2e462a 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -698,6 +698,7 @@ static void multifd_send_page(RAMBlock *block, ram_addr_t 
offset,
 }
 
 struct MultiFDRecvParams {
+/* not changed */
 uint8_t id;
 char *name;
 QemuThread thread;
@@ -705,8 +706,13 @@ struct MultiFDRecvParams {
 QemuSemaphore sem;
 QemuMutex mutex;
 bool running;
+/* protected by param mutex */
 bool quit;
 bool sync;
+/* how many patckets has recv this channel */
+uint32_t packets_recv;
+multifd_pages_t *pages;
+bool done;
 };
 typedef struct MultiFDRecvParams MultiFDRecvParams;
 
@@ -716,6 +722,7 @@ struct {
 int count;
 /* syncs main thread and channels */
 QemuSemaphore sem_main;
+multifd_pages_t *pages;
 } *multifd_recv_state;
 
 static void terminate_multifd_recv_threads(Error *errp)
@@ -764,10 +771,14 @@ int multifd_load_cleanup(Error **errp)
 qemu_sem_destroy(&p->sem);
 g_free(p->name);
 p->name = NULL;
+multifd_pages_clear(p->pages);
+p->pages = NULL;
 }
 qemu_sem_destroy(&multifd_recv_state->sem_main);
 g_free(multifd_recv_state->params);
 multifd_recv_state->params = NULL;
+multifd_pages_clear(multifd_recv_state->pages);
+multifd_recv_state->pages = NULL;
 g_free(multifd_recv_state);
 multifd_recv_state = NULL;
 
@@ -834,6 +845,9 @@ int multifd_load_setup(void)
 multifd_recv_state->params = g_new0(MultiFDRecvParams, thread_count);
 atomic_set(&multifd_recv_state->count, 0);
 qemu_sem_init(&multifd_recv_state->sem_main, 0);
+multifd_pages_init(&multifd_recv_state->pages,
+   migrate_multifd_page_count());
+
 for (i = 0; i < thread_count; i++) {
 MultiFDRecvParams *p = &multifd_recv_state->params[i];
 
@@ -842,6 +856,7 @@ int multifd_load_setup(void)
 p->quit = false;
 p->id = i;
 p->name = g_strdup_printf("multifdrecv_%d", i);
+multifd_pages_init(&p->pages, migrate_multifd_page_count());
 }
 
 return 0;
-- 
2.14.3




[Qemu-devel] [PATCH v10 11/24] migration: terminate_* can be called for other threads

2018-03-07 Thread Juan Quintela
Once there, make  count field to always be accessed with atomic
operations.  To make blocking operations, we need to know that the
thread is running, so create a bool to indicate that.

Signed-off-by: Juan Quintela 
---
 migration/ram.c | 30 --
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 4a56a85d53..977e675f46 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -400,6 +400,7 @@ struct MultiFDSendParams {
 QemuThread thread;
 QemuSemaphore sem;
 QemuMutex mutex;
+bool running;
 bool quit;
 };
 typedef struct MultiFDSendParams MultiFDSendParams;
@@ -424,7 +425,7 @@ static void terminate_multifd_send_threads(Error *errp)
 }
 }
 
-for (i = 0; i < multifd_send_state->count; i++) {
+for (i = 0; i < migrate_multifd_channels(); i++) {
 MultiFDSendParams *p = &multifd_send_state->params[i];
 
 qemu_mutex_lock(&p->mutex);
@@ -443,10 +444,13 @@ int multifd_save_cleanup(Error **errp)
 return 0;
 }
 terminate_multifd_send_threads(NULL);
-for (i = 0; i < multifd_send_state->count; i++) {
+for (i = 0; i < migrate_multifd_channels(); i++) {
 MultiFDSendParams *p = &multifd_send_state->params[i];
 
-qemu_thread_join(&p->thread);
+if (p->running) {
+qemu_thread_join(&p->thread);
+p->running = false;
+}
 qemu_mutex_destroy(&p->mutex);
 qemu_sem_destroy(&p->sem);
 g_free(p->name);
@@ -487,7 +491,7 @@ int multifd_save_setup(void)
 thread_count = migrate_multifd_channels();
 multifd_send_state = g_malloc0(sizeof(*multifd_send_state));
 multifd_send_state->params = g_new0(MultiFDSendParams, thread_count);
-multifd_send_state->count = 0;
+atomic_set(&multifd_send_state->count, 0);
 for (i = 0; i < thread_count; i++) {
 MultiFDSendParams *p = &multifd_send_state->params[i];
 
@@ -496,10 +500,11 @@ int multifd_save_setup(void)
 p->quit = false;
 p->id = i;
 p->name = g_strdup_printf("multifdsend_%d", i);
+p->running = true;
 qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
QEMU_THREAD_JOINABLE);
 
-multifd_send_state->count++;
+atomic_inc(&multifd_send_state->count);
 }
 return 0;
 }
@@ -510,6 +515,7 @@ struct MultiFDRecvParams {
 QemuThread thread;
 QemuSemaphore sem;
 QemuMutex mutex;
+bool running;
 bool quit;
 };
 typedef struct MultiFDRecvParams MultiFDRecvParams;
@@ -534,7 +540,7 @@ static void terminate_multifd_recv_threads(Error *errp)
 }
 }
 
-for (i = 0; i < multifd_recv_state->count; i++) {
+for (i = 0; i < migrate_multifd_channels(); i++) {
 MultiFDRecvParams *p = &multifd_recv_state->params[i];
 
 qemu_mutex_lock(&p->mutex);
@@ -553,10 +559,13 @@ int multifd_load_cleanup(Error **errp)
 return 0;
 }
 terminate_multifd_recv_threads(NULL);
-for (i = 0; i < multifd_recv_state->count; i++) {
+for (i = 0; i < migrate_multifd_channels(); i++) {
 MultiFDRecvParams *p = &multifd_recv_state->params[i];
 
-qemu_thread_join(&p->thread);
+if (p->running) {
+qemu_thread_join(&p->thread);
+p->running = false;
+}
 qemu_mutex_destroy(&p->mutex);
 qemu_sem_destroy(&p->sem);
 g_free(p->name);
@@ -598,7 +607,7 @@ int multifd_load_setup(void)
 thread_count = migrate_multifd_channels();
 multifd_recv_state = g_malloc0(sizeof(*multifd_recv_state));
 multifd_recv_state->params = g_new0(MultiFDRecvParams, thread_count);
-multifd_recv_state->count = 0;
+atomic_set(&multifd_recv_state->count, 0);
 for (i = 0; i < thread_count; i++) {
 MultiFDRecvParams *p = &multifd_recv_state->params[i];
 
@@ -607,9 +616,10 @@ int multifd_load_setup(void)
 p->quit = false;
 p->id = i;
 p->name = g_strdup_printf("multifdrecv_%d", i);
+p->running = true;
 qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
QEMU_THREAD_JOINABLE);
-multifd_recv_state->count++;
+atomic_inc(&multifd_recv_state->count);
 }
 return 0;
 }
-- 
2.14.3




[Qemu-devel] [PATCH v10 13/24] migration: Introduce multifd_recv_new_channel()

2018-03-07 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 migration/migration.c | 3 ++-
 migration/ram.c   | 6 ++
 migration/ram.h   | 2 ++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index c398665de7..919343232e 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -448,8 +448,9 @@ void migration_ioc_process_incoming(QIOChannel *ioc)
 if (!mis->from_src_file) {
 QEMUFile *f = qemu_fopen_channel_input(ioc);
 migration_fd_process_incoming(f);
+return;
 }
-/* We still only have a single channel.  Nothing to do here yet */
+multifd_recv_new_channel(ioc);
 }
 
 /**
diff --git a/migration/ram.c b/migration/ram.c
index 977e675f46..f48c74585f 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -36,6 +36,7 @@
 #include "xbzrle.h"
 #include "ram.h"
 #include "migration.h"
+#include "socket.h"
 #include "migration/register.h"
 #include "migration/misc.h"
 #include "qemu-file.h"
@@ -624,6 +625,11 @@ int multifd_load_setup(void)
 return 0;
 }
 
+void multifd_recv_new_channel(QIOChannel *ioc)
+{
+/* nothing to do yet */
+}
+
 /**
  * save_page_header: write page header to wire
  *
diff --git a/migration/ram.h b/migration/ram.h
index 53f0021c51..a2031acf59 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -32,6 +32,7 @@
 #include "qemu-common.h"
 #include "qapi/qapi-types-migration.h"
 #include "exec/cpu-common.h"
+#include "io/channel.h"
 
 extern MigrationStats ram_counters;
 extern XBZRLECacheStats xbzrle_counters;
@@ -44,6 +45,7 @@ int multifd_save_setup(void);
 int multifd_save_cleanup(Error **errp);
 int multifd_load_setup(void);
 int multifd_load_cleanup(Error **errp);
+void multifd_recv_new_channel(QIOChannel *ioc);
 
 uint64_t ram_pagesize_summary(void);
 int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len);
-- 
2.14.3




[Qemu-devel] [PATCH v10 01/24] tests: Add migration precopy test

2018-03-07 Thread Juan Quintela
Signed-off-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Peter Xu 
---
 tests/migration-test.c | 44 ++--
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 74f9361bdd..65ce3ea4ab 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -513,7 +513,7 @@ static void test_deprecated(void)
 qtest_quit(from);
 }
 
-static void test_migrate(void)
+static void test_postcopy(void)
 {
 char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
 QTestState *from, *to;
@@ -584,6 +584,45 @@ static void test_baddest(void)
 test_migrate_end(from, to, false);
 }
 
+static void test_precopy_unix(void)
+{
+char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
+QTestState *from, *to;
+
+test_migrate_start(&from, &to, uri);
+
+/* We want to pick a speed slow enough that the test completes
+ * quickly, but that it doesn't complete precopy even on a slow
+ * machine, so also set the downtime.
+ */
+/* 1 ms should make it not converge*/
+migrate_set_parameter(from, "downtime-limit", "1");
+/* 1GB/s */
+migrate_set_parameter(from, "max-bandwidth", "10");
+
+/* Wait for the first serial output from the source */
+wait_for_serial("src_serial");
+
+migrate(from, uri);
+
+wait_for_migration_pass(from);
+
+/* 300 ms should converge */
+migrate_set_parameter(from, "downtime-limit", "300");
+
+if (!got_stop) {
+qtest_qmp_eventwait(from, "STOP");
+}
+
+qtest_qmp_eventwait(to, "RESUME");
+
+wait_for_serial("dest_serial");
+wait_for_migration_complete(from);
+
+test_migrate_end(from, to, true);
+g_free(uri);
+}
+
 int main(int argc, char **argv)
 {
 char template[] = "/tmp/migration-test-XX";
@@ -603,9 +642,10 @@ int main(int argc, char **argv)
 
 module_call_init(MODULE_INIT_QOM);
 
-qtest_add_func("/migration/postcopy/unix", test_migrate);
+qtest_add_func("/migration/postcopy/unix", test_postcopy);
 qtest_add_func("/migration/deprecated", test_deprecated);
 qtest_add_func("/migration/bad_dest", test_baddest);
+qtest_add_func("/migration/precopy/unix", test_precopy_unix);
 
 ret = g_test_run();
 
-- 
2.14.3




[Qemu-devel] [PATCH v10 19/24] migration: Create multifd channels

2018-03-07 Thread Juan Quintela
In both sides.  We still don't transmit anything through them.

Signed-off-by: Juan Quintela 
---
 migration/ram.c | 52 ++--
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index b57d9fd667..7ef0c2b7e2 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -399,6 +399,7 @@ struct MultiFDSendParams {
 uint8_t id;
 char *name;
 QemuThread thread;
+QIOChannel *c;
 QemuSemaphore sem;
 QemuMutex mutex;
 bool running;
@@ -455,6 +456,8 @@ int multifd_save_cleanup(Error **errp)
 qemu_thread_join(&p->thread);
 p->running = false;
 }
+socket_send_channel_destroy(p->c);
+p->c = NULL;
 qemu_mutex_destroy(&p->mutex);
 qemu_sem_destroy(&p->sem);
 g_free(p->name);
@@ -514,6 +517,27 @@ static void *multifd_send_thread(void *opaque)
 return NULL;
 }
 
+static void multifd_new_send_channel_async(QIOTask *task, gpointer opaque)
+{
+MultiFDSendParams *p = opaque;
+QIOChannel *sioc = QIO_CHANNEL(qio_task_get_source(task));
+Error *local_err = NULL;
+
+if (qio_task_propagate_error(task, &local_err)) {
+if (multifd_save_cleanup(&local_err) != 0) {
+migrate_set_error(migrate_get_current(), local_err);
+}
+} else {
+p->c = QIO_CHANNEL(sioc);
+qio_channel_set_delay(p->c, false);
+p->running = true;
+qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
+   QEMU_THREAD_JOINABLE);
+
+atomic_inc(&multifd_send_state->count);
+}
+}
+
 int multifd_save_setup(void)
 {
 int thread_count;
@@ -536,11 +560,7 @@ int multifd_save_setup(void)
 p->quit = false;
 p->id = i;
 p->name = g_strdup_printf("multifdsend_%d", i);
-p->running = true;
-qemu_thread_create(&p->thread, p->name, multifd_send_thread, p,
-   QEMU_THREAD_JOINABLE);
-
-atomic_inc(&multifd_send_state->count);
+socket_send_channel_create(multifd_new_send_channel_async, p);
 }
 
 return 0;
@@ -550,6 +570,7 @@ struct MultiFDRecvParams {
 uint8_t id;
 char *name;
 QemuThread thread;
+QIOChannel *c;
 QemuSemaphore sem;
 QemuMutex mutex;
 bool running;
@@ -606,6 +627,8 @@ int multifd_load_cleanup(Error **errp)
 qemu_thread_join(&p->thread);
 p->running = false;
 }
+socket_recv_channel_unref(p->c);
+p->c = NULL;
 qemu_mutex_destroy(&p->mutex);
 qemu_sem_destroy(&p->sem);
 g_free(p->name);
@@ -688,10 +711,6 @@ int multifd_load_setup(void)
 p->quit = false;
 p->id = i;
 p->name = g_strdup_printf("multifdrecv_%d", i);
-p->running = true;
-qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
-   QEMU_THREAD_JOINABLE);
-atomic_inc(&multifd_recv_state->count);
 }
 
 return 0;
@@ -710,7 +729,20 @@ bool multifd_recv_all_channels_created(void)
 
 void multifd_recv_new_channel(QIOChannel *ioc)
 {
-/* nothing to do yet */
+MultiFDRecvParams *p;
+/* we need to invent channels id's until we transmit */
+/* we will remove this on a later patch */
+static int i = 0;
+
+p = &multifd_recv_state->params[i];
+i++;
+p->c = ioc;
+socket_recv_channel_ref(ioc);
+
+p->running = true;
+qemu_thread_create(&p->thread, p->name, multifd_recv_thread, p,
+   QEMU_THREAD_JOINABLE);
+atomic_inc(&multifd_recv_state->count);
 }
 
 /**
-- 
2.14.3




[Qemu-devel] [PATCH v10 18/24] migration: Add multifd traces for start/end thread

2018-03-07 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 migration/ram.c| 6 ++
 migration/trace-events | 4 
 2 files changed, 10 insertions(+)

diff --git a/migration/ram.c b/migration/ram.c
index 0266bd200c..b57d9fd667 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -492,6 +492,8 @@ static void *multifd_send_thread(void *opaque)
 {
 MultiFDSendParams *p = opaque;
 
+trace_multifd_send_thread_start(p->id);
+
 while (true) {
 qemu_sem_wait(&p->sem);
 qemu_mutex_lock(&p->mutex);
@@ -507,6 +509,7 @@ static void *multifd_send_thread(void *opaque)
 }
 qemu_mutex_unlock(&p->mutex);
 }
+trace_multifd_send_thread_end(p->id);
 
 return NULL;
 }
@@ -642,6 +645,8 @@ static void *multifd_recv_thread(void *opaque)
 {
 MultiFDRecvParams *p = opaque;
 
+trace_multifd_recv_thread_start(p->id);
+
 while (true) {
 qemu_sem_wait(&p->sem);
 qemu_mutex_lock(&p->mutex);
@@ -658,6 +663,7 @@ static void *multifd_recv_thread(void *opaque)
 qemu_mutex_unlock(&p->mutex);
 }
 
+trace_multifd_recv_thread_end(p->id);
 return NULL;
 }
 
diff --git a/migration/trace-events b/migration/trace-events
index 76075c26bc..db88fa699f 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -79,6 +79,10 @@ ram_save_page(const char *rbname, uint64_t offset, void 
*host) "%s: offset: 0x%"
 ram_save_queue_pages(const char *rbname, size_t start, size_t len) "%s: start: 
0x%zx len: 0x%zx"
 multifd_send_sync_main(void) ""
 multifd_recv_sync_main(void) ""
+multifd_send_thread_start(int id) "%d"
+multifd_send_thread_end(int id) "%d"
+multifd_recv_thread_start(int id) "%d"
+multifd_recv_thread_end(int id) "%d"
 
 # migration/migration.c
 await_return_path_close_on_source_close(void) ""
-- 
2.14.3




[Qemu-devel] [PATCH v10 02/24] tests: Add migration xbzrle test

2018-03-07 Thread Juan Quintela
Signed-off-by: Juan Quintela 
Reviewed-by: Peter Xu 
---
 tests/migration-test.c | 64 ++
 1 file changed, 64 insertions(+)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 65ce3ea4ab..fb67a88353 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -501,6 +501,20 @@ static void deprecated_set_speed(QTestState *who, const 
char *value)
 migrate_check_parameter(who, "max-bandwidth", value);
 }
 
+static void deprecated_set_cache_size(QTestState *who, const char *value)
+{
+QDict *rsp;
+gchar *cmd;
+
+cmd = g_strdup_printf("{ 'execute': 'migrate-set-cache-size',"
+  "'arguments': { 'value': %s } }", value);
+rsp = qtest_qmp(who, cmd);
+g_free(cmd);
+g_assert(qdict_haskey(rsp, "return"));
+QDECREF(rsp);
+migrate_check_parameter(who, "xbzrle-cache-size", value);
+}
+
 static void test_deprecated(void)
 {
 QTestState *from;
@@ -509,6 +523,7 @@ static void test_deprecated(void)
 
 deprecated_set_downtime(from, 0.12345);
 deprecated_set_speed(from, "12345");
+deprecated_set_cache_size(from, "4096");
 
 qtest_quit(from);
 }
@@ -623,6 +638,54 @@ static void test_precopy_unix(void)
 g_free(uri);
 }
 
+static void test_xbzrle(const char *uri)
+{
+QTestState *from, *to;
+
+test_migrate_start(&from, &to, uri);
+
+/* We want to pick a speed slow enough that the test completes
+ * quickly, but that it doesn't complete precopy even on a slow
+ * machine, so also set the downtime.
+ */
+/* 1 ms should make it not converge*/
+migrate_set_parameter(from, "downtime-limit", "1");
+/* 1GB/s */
+migrate_set_parameter(from, "max-bandwidth", "10");
+
+migrate_set_parameter(from, "xbzrle-cache-size", "33554432");
+
+migrate_set_capability(from, "xbzrle", "true");
+migrate_set_capability(to, "xbzrle", "true");
+/* Wait for the first serial output from the source */
+wait_for_serial("src_serial");
+
+migrate(from, uri);
+
+wait_for_migration_pass(from);
+
+/* 300ms should converge */
+migrate_set_parameter(from, "downtime-limit", "300");
+
+if (!got_stop) {
+qtest_qmp_eventwait(from, "STOP");
+}
+qtest_qmp_eventwait(to, "RESUME");
+
+wait_for_serial("dest_serial");
+wait_for_migration_complete(from);
+
+test_migrate_end(from, to, true);
+}
+
+static void test_xbzrle_unix(void)
+{
+char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
+
+test_xbzrle(uri);
+g_free(uri);
+}
+
 int main(int argc, char **argv)
 {
 char template[] = "/tmp/migration-test-XX";
@@ -646,6 +709,7 @@ int main(int argc, char **argv)
 qtest_add_func("/migration/deprecated", test_deprecated);
 qtest_add_func("/migration/bad_dest", test_baddest);
 qtest_add_func("/migration/precopy/unix", test_precopy_unix);
+qtest_add_func("/migration/xbzrle/unix", test_xbzrle_unix);
 
 ret = g_test_run();
 
-- 
2.14.3




[Qemu-devel] [PATCH v10 21/24] migration: Transmit initial package through the multifd channels

2018-03-07 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 migration/ram.c | 50 +-
 1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 1aab96bd5e..4efac0c20c 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -52,6 +52,8 @@
 #include "qemu/rcu_queue.h"
 #include "migration/colo.h"
 #include "migration/block.h"
+#include "sysemu/sysemu.h"
+#include "qemu/uuid.h"
 
 /***/
 /* ram save/restore */
@@ -491,12 +493,30 @@ static void multifd_send_sync_main(void)
 trace_multifd_send_sync_main();
 }
 
+typedef struct {
+uint32_t version;
+unsigned char uuid[16]; /* QemuUUID */
+uint8_t id;
+} __attribute__((packed)) MultiFDInit_t;
+
 static void *multifd_send_thread(void *opaque)
 {
 MultiFDSendParams *p = opaque;
+MultiFDInit_t msg;
+Error *local_err = NULL;
+size_t ret;
 
 trace_multifd_send_thread_start(p->id);
 
+msg.version = 1;
+msg.id = p->id;
+memcpy(msg.uuid, &qemu_uuid.data, sizeof(msg.uuid));
+ret = qio_channel_write_all(p->c, (char *)&msg, sizeof(msg), &local_err);
+if (ret != 0) {
+terminate_multifd_send_threads(local_err);
+return NULL;
+}
+
 while (true) {
 qemu_sem_wait(&p->sem);
 qemu_mutex_lock(&p->mutex);
@@ -730,12 +750,32 @@ bool multifd_recv_all_channels_created(void)
 void multifd_recv_new_channel(QIOChannel *ioc)
 {
 MultiFDRecvParams *p;
-/* we need to invent channels id's until we transmit */
-/* we will remove this on a later patch */
-static int i = 0;
+MultiFDInit_t msg;
+Error *local_err = NULL;
+size_t ret;
 
-p = &multifd_recv_state->params[i];
-i++;
+ret = qio_channel_read_all(ioc, (char *)&msg, sizeof(msg), &local_err);
+if (ret != 0) {
+terminate_multifd_recv_threads(local_err);
+return;
+}
+
+if (memcmp(msg.uuid, &qemu_uuid, sizeof(qemu_uuid))) {
+char *uuid = qemu_uuid_unparse_strdup(&qemu_uuid);
+error_setg(&local_err, "multifd: received uuid '%s' and expected "
+   "uuid '%s' for channel %hhd", msg.uuid, uuid, msg.id);
+g_free(uuid);
+terminate_multifd_recv_threads(local_err);
+return;
+}
+
+p = &multifd_recv_state->params[msg.id];
+if (p->c != NULL) {
+error_setg(&local_err, "multifd: received id '%d' already setup'",
+   msg.id);
+terminate_multifd_recv_threads(local_err);
+return;
+}
 p->c = ioc;
 socket_recv_channel_ref(ioc);
 
-- 
2.14.3




[Qemu-devel] [PATCH v10 15/24] migration: Synchronize send threads

2018-03-07 Thread Juan Quintela
We synchronize all threads each RAM_SAVE_FLAG_EOS.  Bitmap
synchronizations don't happen inside a  ram section, so we are safe
about two channels trying to overwrite the same memory.

Signed-off-by: Juan Quintela 
---
 migration/ram.c| 38 +-
 migration/trace-events |  1 +
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index e502be5dda..153c7560cb 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -403,6 +403,7 @@ struct MultiFDSendParams {
 QemuMutex mutex;
 bool running;
 bool quit;
+bool sync;
 };
 typedef struct MultiFDSendParams MultiFDSendParams;
 
@@ -410,6 +411,8 @@ struct {
 MultiFDSendParams *params;
 /* number of created threads */
 int count;
+/* syncs main thread and channels */
+QemuSemaphore sem_main;
 } *multifd_send_state;
 
 static void terminate_multifd_send_threads(Error *errp)
@@ -457,6 +460,7 @@ int multifd_save_cleanup(Error **errp)
 g_free(p->name);
 p->name = NULL;
 }
+qemu_sem_destroy(&multifd_send_state->sem_main);
 g_free(multifd_send_state->params);
 multifd_send_state->params = NULL;
 g_free(multifd_send_state);
@@ -464,18 +468,44 @@ int multifd_save_cleanup(Error **errp)
 return ret;
 }
 
+static void multifd_send_sync_main(void)
+{
+int i;
+
+if (!migrate_use_multifd()) {
+return;
+}
+for (i = 0; i < migrate_multifd_channels(); i++) {
+MultiFDSendParams *p = &multifd_send_state->params[i];
+qemu_mutex_lock(&p->mutex);
+p->sync = true;
+qemu_mutex_unlock(&p->mutex);
+qemu_sem_post(&p->sem);
+}
+for (i = 0; i < migrate_multifd_channels(); i++) {
+qemu_sem_wait(&multifd_send_state->sem_main);
+}
+trace_multifd_send_sync_main();
+}
+
 static void *multifd_send_thread(void *opaque)
 {
 MultiFDSendParams *p = opaque;
 
 while (true) {
+qemu_sem_wait(&p->sem);
 qemu_mutex_lock(&p->mutex);
+if (p->sync) {
+p->sync = false;
+qemu_mutex_unlock(&p->mutex);
+qemu_sem_post(&multifd_send_state->sem_main);
+continue;
+}
 if (p->quit) {
 qemu_mutex_unlock(&p->mutex);
 break;
 }
 qemu_mutex_unlock(&p->mutex);
-qemu_sem_wait(&p->sem);
 }
 
 return NULL;
@@ -493,6 +523,8 @@ int multifd_save_setup(void)
 multifd_send_state = g_malloc0(sizeof(*multifd_send_state));
 multifd_send_state->params = g_new0(MultiFDSendParams, thread_count);
 atomic_set(&multifd_send_state->count, 0);
+qemu_sem_init(&multifd_send_state->sem_main, 0);
+
 for (i = 0; i < thread_count; i++) {
 MultiFDSendParams *p = &multifd_send_state->params[i];
 
@@ -507,6 +539,7 @@ int multifd_save_setup(void)
 
 atomic_inc(&multifd_send_state->count);
 }
+
 return 0;
 }
 
@@ -2283,6 +2316,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
 ram_control_before_iterate(f, RAM_CONTROL_SETUP);
 ram_control_after_iterate(f, RAM_CONTROL_SETUP);
 
+multifd_send_sync_main();
 qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
 
 return 0;
@@ -2351,6 +2385,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
  */
 ram_control_after_iterate(f, RAM_CONTROL_ROUND);
 
+multifd_send_sync_main();
 qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
 ram_counters.transferred += 8;
 
@@ -2403,6 +2438,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
 
 rcu_read_unlock();
 
+multifd_send_sync_main();
 qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
 
 return 0;
diff --git a/migration/trace-events b/migration/trace-events
index 93961dea16..97b5ac564f 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -77,6 +77,7 @@ ram_load_postcopy_loop(uint64_t addr, int flags) "@%" PRIx64 
" %x"
 ram_postcopy_send_discard_bitmap(void) ""
 ram_save_page(const char *rbname, uint64_t offset, void *host) "%s: offset: 
0x%" PRIx64 " host: %p"
 ram_save_queue_pages(const char *rbname, size_t start, size_t len) "%s: start: 
0x%zx len: 0x%zx"
+multifd_send_sync_main(void) ""
 
 # migration/migration.c
 await_return_path_close_on_source_close(void) ""
-- 
2.14.3




Re: [Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup

2018-03-07 Thread Kevin Wolf
Am 21.12.2017 um 18:25 hat Jack Schwartz geschrieben:
> Properly account for the possibility of multiboot kernels with a zero
> bss_end_addr.  The Multiboot Specification, section 3.1.3 allows for
> kernels without a bss section, by allowing a zeroed bss_end_addr multiboot
> header field.
> 
> Do some cleanup to multiboot.c as well:
> - Remove some unused variables.
> - Use more intuitive header names when displaying fields in messages.
> - Change fprintf(stderr...) to error_report
> 
> Testing:
>   1) Ran the "make check" test suite.
>   2) Booted multiboot kernel with bss_end_addr=0.  (I rolled my own
>  grub multiboot.elf test "kernel" by modifying source.)  Verified
>  with gdb that new code that reads addresses/offsets from multiboot
>  header was accessed.
>   3) Booted multiboot kernel with non-zero bss_end_addr.
>   4) Uncommented DEBUG_MULTIBOOT in multiboot.c and verified messages worked.
>   5) Code has soaked in an internal repo for two months.
> 
>   Thanks,
>   Jack

Thanks, applied to my multiboot branch.

Kevin



[Qemu-devel] [PATCH v10 22/24] migration: Create ram_multifd_page

2018-03-07 Thread Juan Quintela
The function still don't use multifd, but we have simplified
ram_save_page, xbzrle and RDMA stuff is gone.  We have added a new
counter.

Signed-off-by: Juan Quintela 

--
Add last_page parameter
Add commets for done and address
Remove multifd field, it is the same than normal pages
Merge next patch, now we send multiple pages at a time
Remove counter for multifd pages, it is identical to normal pages
Use iovec's instead of creating the equivalent.
Clear memory used by pages (dave)
Use g_new0(danp)
define MULTIFD_CONTINUE
now pages member is a pointer
Fix off-by-one in number of pages in one packet
Remove RAM_SAVE_FLAG_MULTIFD_PAGE
---
 migration/ram.c| 144 -
 migration/trace-events |   3 +-
 2 files changed, 144 insertions(+), 3 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 4efac0c20c..df9646ed2e 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -54,6 +54,7 @@
 #include "migration/block.h"
 #include "sysemu/sysemu.h"
 #include "qemu/uuid.h"
+#include "qemu/iov.h"
 
 /***/
 /* ram save/restore */
@@ -397,7 +398,19 @@ static void compress_threads_save_setup(void)
 
 /* Multiple fd's */
 
+typedef struct {
+/* number of used pages */
+uint32_t used;
+/* number of allocated pages */
+uint32_t allocated;
+/* global number of generated multifd packets */
+uint32_t seq;
+struct iovec *iov;
+RAMBlock *block;
+} multifd_pages_t;
+
 struct MultiFDSendParams {
+/* not changed */
 uint8_t id;
 char *name;
 QemuThread thread;
@@ -405,8 +418,15 @@ struct MultiFDSendParams {
 QemuSemaphore sem;
 QemuMutex mutex;
 bool running;
+/* protected by param mutex */
 bool quit;
 bool sync;
+multifd_pages_t *pages;
+/* how many patches has sent this channel */
+uint32_t packets_sent;
+/* protected by multifd mutex */
+/* has the thread finish the last submitted job */
+bool done;
 };
 typedef struct MultiFDSendParams MultiFDSendParams;
 
@@ -416,8 +436,31 @@ struct {
 int count;
 /* syncs main thread and channels */
 QemuSemaphore sem_main;
+QemuMutex mutex;
+QemuSemaphore sem;
+multifd_pages_t *pages;
 } *multifd_send_state;
 
+static void multifd_pages_init(multifd_pages_t **ppages, size_t size)
+{
+multifd_pages_t *pages = g_new0(multifd_pages_t, 1);
+
+pages->allocated = size;
+pages->iov = g_new0(struct iovec, size);
+*ppages = pages;
+}
+
+static void multifd_pages_clear(multifd_pages_t *pages)
+{
+pages->used = 0;
+pages->allocated = 0;
+pages->seq = 0;
+pages->block = NULL;
+g_free(pages->iov);
+pages->iov = NULL;
+g_free(pages);
+}
+
 static void terminate_multifd_send_threads(Error *errp)
 {
 int i;
@@ -464,10 +507,14 @@ int multifd_save_cleanup(Error **errp)
 qemu_sem_destroy(&p->sem);
 g_free(p->name);
 p->name = NULL;
+multifd_pages_clear(p->pages);
+p->pages = NULL;
 }
 qemu_sem_destroy(&multifd_send_state->sem_main);
 g_free(multifd_send_state->params);
 multifd_send_state->params = NULL;
+multifd_pages_clear(multifd_send_state->pages);
+multifd_send_state->pages = NULL;
 g_free(multifd_send_state);
 multifd_send_state = NULL;
 return ret;
@@ -516,6 +563,7 @@ static void *multifd_send_thread(void *opaque)
 terminate_multifd_send_threads(local_err);
 return NULL;
 }
+qemu_sem_post(&multifd_send_state->sem);
 
 while (true) {
 qemu_sem_wait(&p->sem);
@@ -530,9 +578,23 @@ static void *multifd_send_thread(void *opaque)
 qemu_mutex_unlock(&p->mutex);
 break;
 }
+if (p->pages->used) {
+p->pages->used = 0;
+qemu_mutex_unlock(&p->mutex);
+
+trace_multifd_send(p->id, p->pages->seq, p->pages->used);
+/* ToDo: send page here */
+
+qemu_mutex_lock(&multifd_send_state->mutex);
+p->done = true;
+p->packets_sent++;
+qemu_mutex_unlock(&multifd_send_state->mutex);
+qemu_sem_post(&multifd_send_state->sem);
+continue;
+}
 qemu_mutex_unlock(&p->mutex);
 }
-trace_multifd_send_thread_end(p->id);
+trace_multifd_send_thread_end(p->id, p->packets_sent);
 
 return NULL;
 }
@@ -571,7 +633,10 @@ int multifd_save_setup(void)
 multifd_send_state->params = g_new0(MultiFDSendParams, thread_count);
 atomic_set(&multifd_send_state->count, 0);
 qemu_sem_init(&multifd_send_state->sem_main, 0);
-
+qemu_mutex_init(&multifd_send_state->mutex);
+qemu_sem_init(&multifd_send_state->sem, 0);
+multifd_pages_init(&multifd_send_state->pages,
+   migrate_multifd_page_count());
 for (i = 0; i < thread_count; i++) {
 MultiFDSendParams *p = &multifd_send_state->params[i];
 
@@ -579,6 +644,8 @@ int multifd_save_

[Qemu-devel] [PULL 2/4] multiboot: Remove unused variables from multiboot.c

2018-03-07 Thread Kevin Wolf
From: Jack Schwartz 

Remove unused variables: mh_mode_type, mh_width, mh_height, mh_depth

Signed-off-by: Jack Schwartz 
Reviewed-by: Daniel Kiper 
Reviewed-by: Prasad J Pandit 
Signed-off-by: Kevin Wolf 
---
 hw/i386/multiboot.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index bb8d8e4629..7d59cbe523 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -255,12 +255,6 @@ int load_multiboot(FWCfgState *fw_cfg,
 mb_kernel_size = mb_load_size;
 }
 
-/* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_VBE.
-uint32_t mh_mode_type = ldl_p(header+i+32);
-uint32_t mh_width = ldl_p(header+i+36);
-uint32_t mh_height = ldl_p(header+i+40);
-uint32_t mh_depth = ldl_p(header+i+44); */
-
 mb_debug("multiboot: mh_header_addr = %#x\n", mh_header_addr);
 mb_debug("multiboot: mh_load_addr = %#x\n", mh_load_addr);
 mb_debug("multiboot: mh_load_end_addr = %#x\n", mh_load_end_addr);
-- 
2.13.6




[Qemu-devel] [PATCH v10 17/24] migration: Export functions to create send channels

2018-03-07 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 migration/socket.c | 28 +++-
 migration/socket.h |  3 +++
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/migration/socket.c b/migration/socket.c
index 26110739cf..b3b5571ebb 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -39,6 +39,28 @@ int socket_recv_channel_unref(QIOChannel *recv)
 return 0;
 }
 
+struct SocketOutgoingArgs {
+SocketAddress *saddr;
+} outgoing_args;
+
+void socket_send_channel_create(void (*f)(QIOTask *, gpointer), void *data)
+{
+QIOChannelSocket *sioc = qio_channel_socket_new();
+qio_channel_socket_connect_async(sioc, outgoing_args.saddr,
+ f, data, NULL);
+}
+
+int socket_send_channel_destroy(QIOChannel *send)
+{
+/* Remove channel */
+object_unref(OBJECT(send));
+if (outgoing_args.saddr) {
+qapi_free_SocketAddress(outgoing_args.saddr);
+outgoing_args.saddr = NULL;
+}
+return 0;
+}
+
 static SocketAddress *tcp_build_address(const char *host_port, Error **errp)
 {
 SocketAddress *saddr;
@@ -106,6 +128,11 @@ static void socket_start_outgoing_migration(MigrationState 
*s,
 struct SocketConnectData *data = g_new0(struct SocketConnectData, 1);
 
 data->s = s;
+
+/* in case previous migration leaked it */
+qapi_free_SocketAddress(outgoing_args.saddr);
+outgoing_args.saddr = saddr;
+
 if (saddr->type == SOCKET_ADDRESS_TYPE_INET) {
 data->hostname = g_strdup(saddr->u.inet.host);
 }
@@ -116,7 +143,6 @@ static void socket_start_outgoing_migration(MigrationState 
*s,
  socket_outgoing_migration,
  data,
  socket_connect_data_free);
-qapi_free_SocketAddress(saddr);
 }
 
 void tcp_start_outgoing_migration(MigrationState *s,
diff --git a/migration/socket.h b/migration/socket.h
index 638a85255a..cbdb8d64c3 100644
--- a/migration/socket.h
+++ b/migration/socket.h
@@ -23,6 +23,9 @@
 int socket_recv_channel_ref(QIOChannel *recv);
 int socket_recv_channel_unref(QIOChannel *recv);
 
+void socket_send_channel_create(void (*f)(QIOTask *, gpointer), void *data);
+int socket_send_channel_destroy(QIOChannel *send);
+
 void tcp_start_incoming_migration(const char *host_port, Error **errp);
 
 void tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
-- 
2.14.3




[Qemu-devel] [PULL 4/4] multiboot: fprintf(stderr...) -> error_report()

2018-03-07 Thread Kevin Wolf
From: Jack Schwartz 

Change all fprintf(stderr...) calls in hw/i386/multiboot.c to call
error_report() instead, including the mb_debug macro.  Remove the "\n"
from strings passed to all modified calls, since error_report() appends
one.

Signed-off-by: Jack Schwartz 
Reviewed-by: Daniel Kiper 
Signed-off-by: Kevin Wolf 
---
 hw/i386/multiboot.c | 55 -
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index 07fdccb84a..b9064264d8 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -31,12 +31,13 @@
 #include "hw/loader.h"
 #include "elf.h"
 #include "sysemu/sysemu.h"
+#include "qemu/error-report.h"
 
 /* Show multiboot debug output */
 //#define DEBUG_MULTIBOOT
 
 #ifdef DEBUG_MULTIBOOT
-#define mb_debug(a...) fprintf(stderr, ## a)
+#define mb_debug(a...) error_report(a)
 #else
 #define mb_debug(a...)
 #endif
@@ -137,7 +138,7 @@ static void mb_add_mod(MultibootState *s,
 stl_p(p + MB_MOD_END, end);
 stl_p(p + MB_MOD_CMDLINE, cmdline_phys);
 
-mb_debug("mod%02d: "TARGET_FMT_plx" - "TARGET_FMT_plx"\n",
+mb_debug("mod%02d: "TARGET_FMT_plx" - "TARGET_FMT_plx,
  s->mb_mods_count, start, end);
 
 s->mb_mods_count++;
@@ -179,12 +180,12 @@ int load_multiboot(FWCfgState *fw_cfg,
 if (!is_multiboot)
 return 0; /* no multiboot */
 
-mb_debug("qemu: I believe we found a multiboot image!\n");
+mb_debug("qemu: I believe we found a multiboot image!");
 memset(bootinfo, 0, sizeof(bootinfo));
 memset(&mbs, 0, sizeof(mbs));
 
 if (flags & 0x0004) { /* MULTIBOOT_HEADER_HAS_VBE */
-fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
+error_report("qemu: multiboot knows VBE. we don't.");
 }
 if (!(flags & 0x0001)) { /* MULTIBOOT_HEADER_HAS_ADDR */
 uint64_t elf_entry;
@@ -193,7 +194,7 @@ int load_multiboot(FWCfgState *fw_cfg,
 fclose(f);
 
 if (((struct elf64_hdr*)header)->e_machine == EM_X86_64) {
-fprintf(stderr, "Cannot load x86-64 image, give a 32bit one.\n");
+error_report("Cannot load x86-64 image, give a 32bit one.");
 exit(1);
 }
 
@@ -201,7 +202,7 @@ int load_multiboot(FWCfgState *fw_cfg,
&elf_low, &elf_high, 0, I386_ELF_MACHINE,
0, 0);
 if (kernel_size < 0) {
-fprintf(stderr, "Error while loading elf kernel\n");
+error_report("Error while loading elf kernel");
 exit(1);
 }
 mh_load_addr = elf_low;
@@ -210,12 +211,13 @@ int load_multiboot(FWCfgState *fw_cfg,
 
 mbs.mb_buf = g_malloc(mb_kernel_size);
 if (rom_copy(mbs.mb_buf, mh_load_addr, mb_kernel_size) != 
mb_kernel_size) {
-fprintf(stderr, "Error while fetching elf kernel from rom\n");
+error_report("Error while fetching elf kernel from rom");
 exit(1);
 }
 
-mb_debug("qemu: loading multiboot-elf kernel (%#x bytes) with entry 
%#zx\n",
-  mb_kernel_size, (size_t)mh_entry_addr);
+mb_debug("qemu: loading multiboot-elf kernel "
+ "(%#x bytes) with entry %#zx",
+ mb_kernel_size, (size_t)mh_entry_addr);
 } else {
 /* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_ADDR. */
 uint32_t mh_header_addr = ldl_p(header+i+12);
@@ -224,7 +226,7 @@ int load_multiboot(FWCfgState *fw_cfg,
 
 mh_load_addr = ldl_p(header+i+16);
 if (mh_header_addr < mh_load_addr) {
-fprintf(stderr, "invalid load_addr address\n");
+error_report("invalid load_addr address");
 exit(1);
 }
 
@@ -234,20 +236,20 @@ int load_multiboot(FWCfgState *fw_cfg,
 
 if (mh_load_end_addr) {
 if (mh_load_end_addr < mh_load_addr) {
-fprintf(stderr, "invalid load_end_addr address\n");
+error_report("invalid load_end_addr address");
 exit(1);
 }
 mb_load_size = mh_load_end_addr - mh_load_addr;
 } else {
 if (kernel_file_size < mb_kernel_text_offset) {
-fprintf(stderr, "invalid kernel_file_size\n");
+error_report("invalid kernel_file_size");
 exit(1);
 }
 mb_load_size = kernel_file_size - mb_kernel_text_offset;
 }
 if (mh_bss_end_addr) {
 if (mh_bss_end_addr < (mh_load_addr + mb_load_size)) {
-fprintf(stderr, "invalid bss_end_addr address\n");
+error_report("invalid bss_end_addr address");
 exit(1);
 }
 mb_kernel_size = mh_bss_end_addr - mh_load_addr;
@@ -255,17 +257,17 @@ int load_multiboot(FWCfgState *fw_cfg,
 mb_kernel_size = mb_load_size;
 }
 
-mb_debug("multiboot: header_addr = %#x\n", mh_header_addr);
-   

[Qemu-devel] [PULL 0/4] Multiboot patches

2018-03-07 Thread Kevin Wolf
The following changes since commit f32408f3b472a088467474ab152be3b6285b2d7b:

  misc: don't use hwaddr as a type in trace events (2018-03-06 14:24:30 +)

are available in the git repository at:

  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to 4b9006a41ea8818f2385ae5228e07f211bb4a33d:

  multiboot: fprintf(stderr...) -> error_report() (2018-03-07 11:53:37 +0100)


Multiboot patches


Jack Schwartz (4):
  multiboot: bss_end_addr can be zero
  multiboot: Remove unused variables from multiboot.c
  multiboot: Use header names when displaying fields
  multiboot: fprintf(stderr...) -> error_report()

 hw/i386/multiboot.c | 77 ++---
 1 file changed, 38 insertions(+), 39 deletions(-)



[Qemu-devel] [PULL 3/4] multiboot: Use header names when displaying fields

2018-03-07 Thread Kevin Wolf
From: Jack Schwartz 

Refer to field names when displaying fields in printf and debug statements.

Signed-off-by: Jack Schwartz 
Reviewed-by: Daniel Kiper 
Signed-off-by: Kevin Wolf 
---
 hw/i386/multiboot.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index 7d59cbe523..07fdccb84a 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -224,7 +224,7 @@ int load_multiboot(FWCfgState *fw_cfg,
 
 mh_load_addr = ldl_p(header+i+16);
 if (mh_header_addr < mh_load_addr) {
-fprintf(stderr, "invalid mh_load_addr address\n");
+fprintf(stderr, "invalid load_addr address\n");
 exit(1);
 }
 
@@ -234,7 +234,7 @@ int load_multiboot(FWCfgState *fw_cfg,
 
 if (mh_load_end_addr) {
 if (mh_load_end_addr < mh_load_addr) {
-fprintf(stderr, "invalid mh_load_end_addr address\n");
+fprintf(stderr, "invalid load_end_addr address\n");
 exit(1);
 }
 mb_load_size = mh_load_end_addr - mh_load_addr;
@@ -247,7 +247,7 @@ int load_multiboot(FWCfgState *fw_cfg,
 }
 if (mh_bss_end_addr) {
 if (mh_bss_end_addr < (mh_load_addr + mb_load_size)) {
-fprintf(stderr, "invalid mh_bss_end_addr address\n");
+fprintf(stderr, "invalid bss_end_addr address\n");
 exit(1);
 }
 mb_kernel_size = mh_bss_end_addr - mh_load_addr;
@@ -255,10 +255,10 @@ int load_multiboot(FWCfgState *fw_cfg,
 mb_kernel_size = mb_load_size;
 }
 
-mb_debug("multiboot: mh_header_addr = %#x\n", mh_header_addr);
-mb_debug("multiboot: mh_load_addr = %#x\n", mh_load_addr);
-mb_debug("multiboot: mh_load_end_addr = %#x\n", mh_load_end_addr);
-mb_debug("multiboot: mh_bss_end_addr = %#x\n", mh_bss_end_addr);
+mb_debug("multiboot: header_addr = %#x\n", mh_header_addr);
+mb_debug("multiboot: load_addr = %#x\n", mh_load_addr);
+mb_debug("multiboot: load_end_addr = %#x\n", mh_load_end_addr);
+mb_debug("multiboot: bss_end_addr = %#x\n", mh_bss_end_addr);
 mb_debug("qemu: loading multiboot kernel (%#x bytes) at %#x\n",
  mb_load_size, mh_load_addr);
 
@@ -361,7 +361,7 @@ int load_multiboot(FWCfgState *fw_cfg,
 stl_p(bootinfo + MBI_BOOT_DEVICE, 0x8000); /* XXX: use the -boot 
switch? */
 stl_p(bootinfo + MBI_MMAP_ADDR,   ADDR_E820_MAP);
 
-mb_debug("multiboot: mh_entry_addr = %#x\n", mh_entry_addr);
+mb_debug("multiboot: entry_addr = %#x\n", mh_entry_addr);
 mb_debug("   mb_buf_phys   = "TARGET_FMT_plx"\n", mbs.mb_buf_phys);
 mb_debug("   mod_start = "TARGET_FMT_plx"\n", mbs.mb_buf_phys 
+ mbs.offset_mods);
 mb_debug("   mb_mods_count = %d\n", mbs.mb_mods_count);
-- 
2.13.6




[Qemu-devel] [PULL 1/4] multiboot: bss_end_addr can be zero

2018-03-07 Thread Kevin Wolf
From: Jack Schwartz 

The multiboot spec (https://www.gnu.org/software/grub/manual/multiboot/),
section 3.1.3, allows for bss_end_addr to be zero.

A zero bss_end_addr signifies there is no .bss section.

Suggested-by: Daniel Kiper 
Signed-off-by: Jack Schwartz 
Reviewed-by: Daniel Kiper 
Reviewed-by: Prasad J Pandit 
Signed-off-by: Kevin Wolf 
---
 hw/i386/multiboot.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index 46d9c68bf5..bb8d8e4629 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -233,12 +233,6 @@ int load_multiboot(FWCfgState *fw_cfg,
 mh_entry_addr = ldl_p(header+i+28);
 
 if (mh_load_end_addr) {
-if (mh_bss_end_addr < mh_load_addr) {
-fprintf(stderr, "invalid mh_bss_end_addr address\n");
-exit(1);
-}
-mb_kernel_size = mh_bss_end_addr - mh_load_addr;
-
 if (mh_load_end_addr < mh_load_addr) {
 fprintf(stderr, "invalid mh_load_end_addr address\n");
 exit(1);
@@ -249,8 +243,16 @@ int load_multiboot(FWCfgState *fw_cfg,
 fprintf(stderr, "invalid kernel_file_size\n");
 exit(1);
 }
-mb_kernel_size = kernel_file_size - mb_kernel_text_offset;
-mb_load_size = mb_kernel_size;
+mb_load_size = kernel_file_size - mb_kernel_text_offset;
+}
+if (mh_bss_end_addr) {
+if (mh_bss_end_addr < (mh_load_addr + mb_load_size)) {
+fprintf(stderr, "invalid mh_bss_end_addr address\n");
+exit(1);
+}
+mb_kernel_size = mh_bss_end_addr - mh_load_addr;
+} else {
+mb_kernel_size = mb_load_size;
 }
 
 /* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_VBE.
-- 
2.13.6




[Qemu-devel] [PATCH v10 24/24] [RFC] migration: Send pages through the multifd channels

2018-03-07 Thread Juan Quintela
Migration ends correctly, but there is still a race between clean up
and last synchronization.

Signed-off-by: Juan Quintela 
---
 migration/ram.c| 132 ++---
 migration/trace-events |   3 +-
 2 files changed, 126 insertions(+), 9 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 264d2e462a..577b448db3 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -398,6 +398,19 @@ static void compress_threads_save_setup(void)
 
 /* Multiple fd's */
 
+#define MULTIFD_MAGIC 0x112233d
+#define MULTIFD_VERSION 1
+
+typedef struct {
+uint32_t magic;
+uint32_t version;
+uint32_t size;
+uint32_t used;
+uint32_t seq;
+char ramblock[256];
+ram_addr_t offset[];
+} __attribute__((packed)) MultiFDPacket_t;
+
 typedef struct {
 /* number of used pages */
 uint32_t used;
@@ -407,6 +420,8 @@ typedef struct {
 uint32_t seq;
 struct iovec *iov;
 RAMBlock *block;
+uint32_t packet_len;
+MultiFDPacket_t *packet;
 } multifd_pages_t;
 
 struct MultiFDSendParams {
@@ -447,6 +462,8 @@ static void multifd_pages_init(multifd_pages_t **ppages, 
size_t size)
 
 pages->allocated = size;
 pages->iov = g_new0(struct iovec, size);
+pages->packet_len = sizeof(MultiFDPacket_t) + sizeof(ram_addr_t) * size;
+pages->packet = g_malloc0(pages->packet_len);
 *ppages = pages;
 }
 
@@ -458,6 +475,9 @@ static void multifd_pages_clear(multifd_pages_t *pages)
 pages->block = NULL;
 g_free(pages->iov);
 pages->iov = NULL;
+pages->packet_len = 0;
+g_free(pages->packet);
+pages->packet = NULL;
 g_free(pages);
 }
 
@@ -499,7 +519,6 @@ int multifd_save_cleanup(Error **errp)
 
 if (p->running) {
 qemu_thread_join(&p->thread);
-p->running = false;
 }
 socket_send_channel_destroy(p->c);
 p->c = NULL;
@@ -535,7 +554,16 @@ static void multifd_send_sync_main(void)
 qemu_sem_post(&p->sem);
 }
 for (i = 0; i < migrate_multifd_channels(); i++) {
-qemu_sem_wait(&multifd_send_state->sem_main);
+MultiFDSendParams *p = &multifd_send_state->params[i];
+bool wait = true;
+
+qemu_mutex_lock(&p->mutex);
+wait = p->running;
+qemu_mutex_unlock(&p->mutex);
+
+if (wait) {
+qemu_sem_wait(&multifd_send_state->sem_main);
+}
 }
 trace_multifd_send_sync_main();
 }
@@ -575,16 +603,37 @@ static void *multifd_send_thread(void *opaque)
 continue;
 }
 if (p->quit) {
+p->running = false;
 qemu_mutex_unlock(&p->mutex);
 break;
 }
 if (p->pages->used) {
+MultiFDPacket_t *packet = p->pages->packet;
+Error *local_err = NULL;
+size_t ret;
+
+packet->used = p->pages->used;
 p->pages->used = 0;
 qemu_mutex_unlock(&p->mutex);
+packet->magic = MULTIFD_MAGIC;
+packet->version = MULTIFD_VERSION;
 
-trace_multifd_send(p->id, p->pages->seq, p->pages->used);
-/* ToDo: send page here */
-
+strncpy(packet->ramblock, p->pages->block->idstr, 256);
+packet->size = migrate_multifd_page_count();
+packet->seq = p->pages->seq;
+ret = qio_channel_write_all(p->c, (void *)packet,
+p->pages->packet_len, &local_err);
+if (ret != 0) {
+terminate_multifd_send_threads(local_err);
+return NULL;
+}
+trace_multifd_send(p->id, p->pages->seq, packet->used);
+ret = qio_channel_writev_all(p->c, p->pages->iov,
+ packet->used, &local_err);
+if (ret != 0) {
+terminate_multifd_send_threads(local_err);
+return NULL;
+}
 qemu_mutex_lock(&multifd_send_state->mutex);
 p->done = true;
 p->packets_sent++;
@@ -763,7 +812,6 @@ int multifd_load_cleanup(Error **errp)
 
 if (p->running) {
 qemu_thread_join(&p->thread);
-p->running = false;
 }
 socket_recv_channel_unref(p->c);
 p->c = NULL;
@@ -801,17 +849,48 @@ static void multifd_recv_sync_main(void)
 qemu_sem_post(&p->sem);
 }
 for (i = 0; i < migrate_multifd_channels(); i++) {
-qemu_sem_wait(&multifd_recv_state->sem_main);
+MultiFDRecvParams *p = &multifd_recv_state->params[i];
+bool wait = true;
+
+qemu_mutex_lock(&p->mutex);
+wait = p->running && !p->quit;
+qemu_mutex_unlock(&p->mutex);
+
+if (wait) {
+qemu_sem_wait(&multifd_recv_state->sem_main);
+}
 }
 trace_multifd_recv_sync_main();
 }
 
+static gboolean recv_channel_ready(QIOChannel *ioc,
+   GIOCondition condition,
+   

Re: [Qemu-devel] Outreachy 2017-DecemberMarch Aspirant for Vulkan-ize_virgl Project

2018-03-07 Thread Marc-André Lureau
Hi Anusha

On Wed, Mar 7, 2018 at 4:54 AM, Anusha Srivastava
 wrote:
> Hi Stefan,
>
> I have not been able to contact with Marc-Andre.
>
> Could you suggest someone else who could help with this ?

Both Dave Airlie and I have discussed with you a few months ago, but
you didn't reply. Did you receive those mails? I suggested some
approaches, and Dave outlined some bigger problems wrt interoperating
GL & vulkan. I suggested to ignore the bigger problems for now. It's a
difficult project nonetheless. Part of the SoC is also for you to
study and propose solutions. What experience do you have with Vulkan?
Are you familiar with Virgl? If you study both technologies, you could
have enough materials to start doing some experimentations and
sketching some plans for the summer to get a chance to be selected.

If you have more specific problems, I can try to help you.

cheers

-- 
Marc-André Lureau



[Qemu-devel] [PULL 6/6] qio: non-default context for TLS handshake

2018-03-07 Thread Daniel P . Berrangé
From: Peter Xu 

A new parameter "context" is added to qio_channel_tls_handshake() is to
allow the TLS to be run on a non-default context.  Still, no functional
change.

Signed-off-by: Peter Xu 
Signed-off-by: Daniel P. Berrangé 
---
 chardev/char-socket.c   |  1 +
 include/io/channel-tls.h|  5 -
 io/channel-tls.c| 45 ++---
 migration/tls.c |  2 ++
 nbd/client.c|  1 +
 nbd/server.c|  1 +
 tests/test-io-channel-tls.c |  2 ++
 ui/vnc-auth-vencrypt.c  |  1 +
 ui/vnc-ws.c |  1 +
 9 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index b0d11387f3..58e11c6f4c 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -703,6 +703,7 @@ static void tcp_chr_tls_init(Chardev *chr)
 qio_channel_tls_handshake(tioc,
   tcp_chr_tls_handshake,
   chr,
+  NULL,
   NULL);
 }
 
diff --git a/include/io/channel-tls.h b/include/io/channel-tls.h
index d157eb10e8..87fcaf9146 100644
--- a/include/io/channel-tls.h
+++ b/include/io/channel-tls.h
@@ -116,6 +116,8 @@ qio_channel_tls_new_client(QIOChannel *master,
  * @func: the callback to invoke when completed
  * @opaque: opaque data to pass to @func
  * @destroy: optional callback to free @opaque
+ * @context: the context that TLS handshake will run with. If %NULL,
+ *   the default context will be used
  *
  * Perform the TLS session handshake. This method
  * will return immediately and the handshake will
@@ -126,7 +128,8 @@ qio_channel_tls_new_client(QIOChannel *master,
 void qio_channel_tls_handshake(QIOChannelTLS *ioc,
QIOTaskFunc func,
gpointer opaque,
-   GDestroyNotify destroy);
+   GDestroyNotify destroy,
+   GMainContext *context);
 
 /**
  * qio_channel_tls_get_session:
diff --git a/io/channel-tls.c b/io/channel-tls.c
index 6182702dab..9628e6fa47 100644
--- a/io/channel-tls.c
+++ b/io/channel-tls.c
@@ -140,13 +140,19 @@ qio_channel_tls_new_client(QIOChannel *master,
 return NULL;
 }
 
+struct QIOChannelTLSData {
+QIOTask *task;
+GMainContext *context;
+};
+typedef struct QIOChannelTLSData QIOChannelTLSData;
 
 static gboolean qio_channel_tls_handshake_io(QIOChannel *ioc,
  GIOCondition condition,
  gpointer user_data);
 
 static void qio_channel_tls_handshake_task(QIOChannelTLS *ioc,
-   QIOTask *task)
+   QIOTask *task,
+   GMainContext *context)
 {
 Error *err = NULL;
 QCryptoTLSSessionHandshakeStatus status;
@@ -171,6 +177,15 @@ static void qio_channel_tls_handshake_task(QIOChannelTLS 
*ioc,
 qio_task_complete(task);
 } else {
 GIOCondition condition;
+QIOChannelTLSData *data = g_new0(typeof(*data), 1);
+
+data->task = task;
+data->context = context;
+
+if (context) {
+g_main_context_ref(context);
+}
+
 if (status == QCRYPTO_TLS_HANDSHAKE_SENDING) {
 condition = G_IO_OUT;
 } else {
@@ -178,11 +193,12 @@ static void qio_channel_tls_handshake_task(QIOChannelTLS 
*ioc,
 }
 
 trace_qio_channel_tls_handshake_pending(ioc, status);
-qio_channel_add_watch(ioc->master,
-  condition,
-  qio_channel_tls_handshake_io,
-  task,
-  NULL);
+qio_channel_add_watch_full(ioc->master,
+   condition,
+   qio_channel_tls_handshake_io,
+   data,
+   NULL,
+   context);
 }
 }
 
@@ -191,12 +207,18 @@ static gboolean qio_channel_tls_handshake_io(QIOChannel 
*ioc,
  GIOCondition condition,
  gpointer user_data)
 {
-QIOTask *task = user_data;
+QIOChannelTLSData *data = user_data;
+QIOTask *task = data->task;
+GMainContext *context = data->context;
 QIOChannelTLS *tioc = QIO_CHANNEL_TLS(
 qio_task_get_source(task));
 
-qio_channel_tls_handshake_task(
-   tioc, task);
+g_free(data);
+qio_channel_tls_handshake_task(tioc, task, context);
+
+if (context) {
+g_main_context_unref(context);
+}
 
 return FALSE;
 }
@@ -204,7 +226,8 @@ static gboolean qio_channel_tls_handshake_io(QIOChannel 
*ioc,
 void qio_channel_tls_handshake(QIOChannelTLS *ioc,
  

[Qemu-devel] [PULL 2/6] qio: introduce qio_channel_add_watch_{full|source}

2018-03-07 Thread Daniel P . Berrangé
From: Peter Xu 

Firstly, introduce an internal qio_channel_add_watch_full(), which
enhances qio_channel_add_watch() that context can be specified.

Then add a new API wrapper qio_channel_add_watch_source() to return a
GSource pointer rather than a tag ID.

Note that the _source() call will keep a reference of GSource so that
callers need to unref them explicitly when finished using the GSource.

Signed-off-by: Peter Xu 
Signed-off-by: Daniel P. Berrangé 
---
 include/io/channel.h | 44 
 io/channel.c | 40 ++--
 2 files changed, 78 insertions(+), 6 deletions(-)

diff --git a/include/io/channel.h b/include/io/channel.h
index 3995e243a3..e8cdadb0b0 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -648,6 +648,50 @@ guint qio_channel_add_watch(QIOChannel *ioc,
 gpointer user_data,
 GDestroyNotify notify);
 
+/**
+ * qio_channel_add_watch_full:
+ * @ioc: the channel object
+ * @condition: the I/O condition to monitor
+ * @func: callback to invoke when the source becomes ready
+ * @user_data: opaque data to pass to @func
+ * @notify: callback to free @user_data
+ * @context: the context to run the watch source
+ *
+ * Similar as qio_channel_add_watch(), but allows to specify context
+ * to run the watch source.
+ *
+ * Returns: the source ID
+ */
+guint qio_channel_add_watch_full(QIOChannel *ioc,
+ GIOCondition condition,
+ QIOChannelFunc func,
+ gpointer user_data,
+ GDestroyNotify notify,
+ GMainContext *context);
+
+/**
+ * qio_channel_add_watch_source:
+ * @ioc: the channel object
+ * @condition: the I/O condition to monitor
+ * @func: callback to invoke when the source becomes ready
+ * @user_data: opaque data to pass to @func
+ * @notify: callback to free @user_data
+ * @context: gcontext to bind the source to
+ *
+ * Similar as qio_channel_add_watch(), but allows to specify context
+ * to run the watch source, meanwhile return the GSource object
+ * instead of tag ID, with the GSource referenced already.
+ *
+ * Note: callers is responsible to unref the source when not needed.
+ *
+ * Returns: the source pointer
+ */
+GSource *qio_channel_add_watch_source(QIOChannel *ioc,
+  GIOCondition condition,
+  QIOChannelFunc func,
+  gpointer user_data,
+  GDestroyNotify notify,
+  GMainContext *context);
 
 /**
  * qio_channel_attach_aio_context:
diff --git a/io/channel.c b/io/channel.c
index ec4b86de7c..8dd0684f5d 100644
--- a/io/channel.c
+++ b/io/channel.c
@@ -299,11 +299,12 @@ void qio_channel_set_aio_fd_handler(QIOChannel *ioc,
 klass->io_set_aio_fd_handler(ioc, ctx, io_read, io_write, opaque);
 }
 
-guint qio_channel_add_watch(QIOChannel *ioc,
-GIOCondition condition,
-QIOChannelFunc func,
-gpointer user_data,
-GDestroyNotify notify)
+guint qio_channel_add_watch_full(QIOChannel *ioc,
+ GIOCondition condition,
+ QIOChannelFunc func,
+ gpointer user_data,
+ GDestroyNotify notify,
+ GMainContext *context)
 {
 GSource *source;
 guint id;
@@ -312,12 +313,39 @@ guint qio_channel_add_watch(QIOChannel *ioc,
 
 g_source_set_callback(source, (GSourceFunc)func, user_data, notify);
 
-id = g_source_attach(source, NULL);
+id = g_source_attach(source, context);
 g_source_unref(source);
 
 return id;
 }
 
+guint qio_channel_add_watch(QIOChannel *ioc,
+GIOCondition condition,
+QIOChannelFunc func,
+gpointer user_data,
+GDestroyNotify notify)
+{
+return qio_channel_add_watch_full(ioc, condition, func,
+  user_data, notify, NULL);
+}
+
+GSource *qio_channel_add_watch_source(QIOChannel *ioc,
+  GIOCondition condition,
+  QIOChannelFunc func,
+  gpointer user_data,
+  GDestroyNotify notify,
+  GMainContext *context)
+{
+GSource *source;
+guint id;
+
+id = qio_channel_add_watch_full(ioc, condition, func,
+user_data, notify, context);
+source = g_main_context_find_source_by_id(context, id);
+g_source_ref(source);
+return source;
+}
+
 
 int qio_channel_shutd

[Qemu-devel] [PULL 5/6] qio: non-default context for async conn

2018-03-07 Thread Daniel P . Berrangé
From: Peter Xu 

We have worked on qio_task_run_in_thread() already.  Further, let
all the qio channel APIs use that context.

Signed-off-by: Peter Xu 
Signed-off-by: Daniel P. Berrangé 
---
 chardev/char-socket.c  |  4 ++--
 include/io/channel-socket.h| 15 ---
 io/channel-socket.c| 15 +--
 migration/socket.c |  3 ++-
 tests/test-io-channel-socket.c |  4 ++--
 5 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 22f65971a1..b0d11387f3 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -867,7 +867,7 @@ static gboolean socket_reconnect_timeout(gpointer opaque)
 tcp_chr_set_client_ioc_name(chr, sioc);
 qio_channel_socket_connect_async(sioc, s->addr,
  qemu_chr_socket_connected,
- chr, NULL);
+ chr, NULL, NULL);
 
 return false;
 }
@@ -951,7 +951,7 @@ static void qmp_chardev_open_socket(Chardev *chr,
 tcp_chr_set_client_ioc_name(chr, sioc);
 qio_channel_socket_connect_async(sioc, s->addr,
  qemu_chr_socket_connected,
- chr, NULL);
+ chr, NULL, NULL);
 } else {
 if (s->is_listen) {
 char *name;
diff --git a/include/io/channel-socket.h b/include/io/channel-socket.h
index 53801f6042..d7134d2cd6 100644
--- a/include/io/channel-socket.h
+++ b/include/io/channel-socket.h
@@ -101,6 +101,8 @@ int qio_channel_socket_connect_sync(QIOChannelSocket *ioc,
  * @callback: the function to invoke on completion
  * @opaque: user data to pass to @callback
  * @destroy: the function to free @opaque
+ * @context: the context to run the async task. If %NULL, the default
+ *   context will be used.
  *
  * Attempt to connect to the address @addr. This method
  * will run in the background so the caller will regain
@@ -113,7 +115,8 @@ void qio_channel_socket_connect_async(QIOChannelSocket *ioc,
   SocketAddress *addr,
   QIOTaskFunc callback,
   gpointer opaque,
-  GDestroyNotify destroy);
+  GDestroyNotify destroy,
+  GMainContext *context);
 
 
 /**
@@ -138,6 +141,8 @@ int qio_channel_socket_listen_sync(QIOChannelSocket *ioc,
  * @callback: the function to invoke on completion
  * @opaque: user data to pass to @callback
  * @destroy: the function to free @opaque
+ * @context: the context to run the async task. If %NULL, the default
+ *   context will be used.
  *
  * Attempt to listen to the address @addr. This method
  * will run in the background so the caller will regain
@@ -150,7 +155,8 @@ void qio_channel_socket_listen_async(QIOChannelSocket *ioc,
  SocketAddress *addr,
  QIOTaskFunc callback,
  gpointer opaque,
- GDestroyNotify destroy);
+ GDestroyNotify destroy,
+ GMainContext *context);
 
 
 /**
@@ -179,6 +185,8 @@ int qio_channel_socket_dgram_sync(QIOChannelSocket *ioc,
  * @callback: the function to invoke on completion
  * @opaque: user data to pass to @callback
  * @destroy: the function to free @opaque
+ * @context: the context to run the async task. If %NULL, the default
+ *   context will be used.
  *
  * Attempt to initialize a datagram socket bound to
  * @localAddr and communicating with peer @remoteAddr.
@@ -194,7 +202,8 @@ void qio_channel_socket_dgram_async(QIOChannelSocket *ioc,
 SocketAddress *remoteAddr,
 QIOTaskFunc callback,
 gpointer opaque,
-GDestroyNotify destroy);
+GDestroyNotify destroy,
+GMainContext *context);
 
 
 /**
diff --git a/io/channel-socket.c b/io/channel-socket.c
index b4d914b767..57cfb4d3a6 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -174,7 +174,8 @@ void qio_channel_socket_connect_async(QIOChannelSocket *ioc,
   SocketAddress *addr,
   QIOTaskFunc callback,
   gpointer opaque,
-  GDestroyNotify destroy)
+  GDestroyNotify destroy,
+  GMainContext *context)
 {
 QIOTask *task = qio_task_new(
 OBJECT(ioc), callback, opaque, destroy);
@@ -189,7 +190,7 @@ void qio_channel_socket_con

[Qemu-devel] [PULL 4/6] qio: non-default context for threaded qtask

2018-03-07 Thread Daniel P . Berrangé
From: Peter Xu 

qio_task_run_in_thread() allows main thread to run blocking operations
in the background. However it has an assumption on that it's always
working with the default context. This patch tries to allow the threaded
QIO task framework to run with non-default gcontext.

Currently no functional change so far, so the QIOTasks are still always
running on main context.

Reviewed-by: Daniel P. Berrange 
Signed-off-by: Peter Xu 
Signed-off-by: Daniel P. Berrangé 
---
 include/io/task.h|  7 +--
 io/channel-socket.c  |  9 ++---
 io/dns-resolver.c|  3 ++-
 io/task.c| 20 ++--
 tests/test-io-task.c |  2 ++
 5 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/include/io/task.h b/include/io/task.h
index 6021f51336..9e09b95b2e 100644
--- a/include/io/task.h
+++ b/include/io/task.h
@@ -227,15 +227,18 @@ QIOTask *qio_task_new(Object *source,
  * @worker: the function to invoke in a thread
  * @opaque: opaque data to pass to @worker
  * @destroy: function to free @opaque
+ * @context: the context to run the complete hook. If %NULL, the
+ *   default context will be used.
  *
  * Run a task in a background thread. When @worker
  * returns it will call qio_task_complete() in
- * the main event thread context.
+ * the event thread context that provided.
  */
 void qio_task_run_in_thread(QIOTask *task,
 QIOTaskWorker worker,
 gpointer opaque,
-GDestroyNotify destroy);
+GDestroyNotify destroy,
+GMainContext *context);
 
 /**
  * qio_task_complete:
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 8359b6683a..b4d914b767 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -188,7 +188,8 @@ void qio_channel_socket_connect_async(QIOChannelSocket *ioc,
 qio_task_run_in_thread(task,
qio_channel_socket_connect_worker,
addrCopy,
-   (GDestroyNotify)qapi_free_SocketAddress);
+   (GDestroyNotify)qapi_free_SocketAddress,
+   NULL);
 }
 
 
@@ -246,7 +247,8 @@ void qio_channel_socket_listen_async(QIOChannelSocket *ioc,
 qio_task_run_in_thread(task,
qio_channel_socket_listen_worker,
addrCopy,
-   (GDestroyNotify)qapi_free_SocketAddress);
+   (GDestroyNotify)qapi_free_SocketAddress,
+   NULL);
 }
 
 
@@ -322,7 +324,8 @@ void qio_channel_socket_dgram_async(QIOChannelSocket *ioc,
 qio_task_run_in_thread(task,
qio_channel_socket_dgram_worker,
data,
-   qio_channel_socket_dgram_worker_free);
+   qio_channel_socket_dgram_worker_free,
+   NULL);
 }
 
 
diff --git a/io/dns-resolver.c b/io/dns-resolver.c
index 8c924071c4..187f725665 100644
--- a/io/dns-resolver.c
+++ b/io/dns-resolver.c
@@ -234,7 +234,8 @@ void qio_dns_resolver_lookup_async(QIODNSResolver *resolver,
 qio_task_run_in_thread(task,
qio_dns_resolver_lookup_worker,
data,
-   qio_dns_resolver_lookup_data_free);
+   qio_dns_resolver_lookup_data_free,
+   NULL);
 }
 
 
diff --git a/io/task.c b/io/task.c
index 1a0a1c7185..2886a2c1bc 100644
--- a/io/task.c
+++ b/io/task.c
@@ -77,6 +77,7 @@ struct QIOTaskThreadData {
 QIOTaskWorker worker;
 gpointer opaque;
 GDestroyNotify destroy;
+GMainContext *context;
 };
 
 
@@ -91,6 +92,10 @@ static gboolean qio_task_thread_result(gpointer opaque)
 data->destroy(data->opaque);
 }
 
+if (data->context) {
+g_main_context_unref(data->context);
+}
+
 g_free(data);
 
 return FALSE;
@@ -100,6 +105,7 @@ static gboolean qio_task_thread_result(gpointer opaque)
 static gpointer qio_task_thread_worker(gpointer opaque)
 {
 struct QIOTaskThreadData *data = opaque;
+GSource *idle;
 
 trace_qio_task_thread_run(data->task);
 data->worker(data->task, data->opaque);
@@ -110,7 +116,11 @@ static gpointer qio_task_thread_worker(gpointer opaque)
  * the worker results
  */
 trace_qio_task_thread_exit(data->task);
-g_idle_add(qio_task_thread_result, data);
+
+idle = g_idle_source_new();
+g_source_set_callback(idle, qio_task_thread_result, data, NULL);
+g_source_attach(idle, data->context);
+
 return NULL;
 }
 
@@ -118,15 +128,21 @@ static gpointer qio_task_thread_worker(gpointer opaque)
 void qio_task_run_in_thread(QIOTask *task,
 QIOTaskWorker worker,
 gpointer opaque,
-GDestroyNotify destroy)
+GDestroyNotify destroy,
+

[Qemu-devel] [PULL 3/6] qio: store gsources for net listeners

2018-03-07 Thread Daniel P . Berrangé
From: Peter Xu 

Originally we were storing the GSources tag IDs.  That'll be not enough
if we are going to support non-default gcontext for QIO code.  Switch to
GSources without changing anything real.  Now we still always pass in
NULL, which means the default gcontext.

Signed-off-by: Peter Xu 
Signed-off-by: Daniel P. Berrangé 
---
 include/io/net-listener.h | 22 --
 io/net-listener.c | 58 +--
 2 files changed, 56 insertions(+), 24 deletions(-)

diff --git a/include/io/net-listener.h b/include/io/net-listener.h
index 56d6da7a76..8081ac58a2 100644
--- a/include/io/net-listener.h
+++ b/include/io/net-listener.h
@@ -53,7 +53,7 @@ struct QIONetListener {
 
 char *name;
 QIOChannelSocket **sioc;
-gulong *io_tag;
+GSource **io_source;
 size_t nsioc;
 
 bool connected;
@@ -120,17 +120,35 @@ void qio_net_listener_add(QIONetListener *listener,
   QIOChannelSocket *sioc);
 
 /**
- * qio_net_listener_set_client_func:
+ * qio_net_listener_set_client_func_full:
  * @listener: the network listener object
  * @func: the callback function
  * @data: opaque data to pass to @func
  * @notify: callback to free @data
+ * @context: the context that the sources will be bound to.  If %NULL,
+ *   the default context will be used.
  *
  * Register @func to be invoked whenever a new client
  * connects to the listener. @func will be invoked
  * passing in the QIOChannelSocket instance for the
  * client.
  */
+void qio_net_listener_set_client_func_full(QIONetListener *listener,
+   QIONetListenerClientFunc func,
+   gpointer data,
+   GDestroyNotify notify,
+   GMainContext *context);
+
+/**
+ * qio_net_listener_set_client_func:
+ * @listener: the network listener object
+ * @func: the callback function
+ * @data: opaque data to pass to @func
+ * @notify: callback to free @data
+ *
+ * Wrapper of qio_net_listener_set_client_func_full(), only that the
+ * sources will always be bound to default main context.
+ */
 void qio_net_listener_set_client_func(QIONetListener *listener,
   QIONetListenerClientFunc func,
   gpointer data,
diff --git a/io/net-listener.c b/io/net-listener.c
index de38dfae99..555e8acaa4 100644
--- a/io/net-listener.c
+++ b/io/net-listener.c
@@ -118,29 +118,32 @@ void qio_net_listener_add(QIONetListener *listener,
 
 listener->sioc = g_renew(QIOChannelSocket *, listener->sioc,
  listener->nsioc + 1);
-listener->io_tag = g_renew(gulong, listener->io_tag, listener->nsioc + 1);
+listener->io_source = g_renew(typeof(listener->io_source[0]),
+  listener->io_source,
+  listener->nsioc + 1);
 listener->sioc[listener->nsioc] = sioc;
-listener->io_tag[listener->nsioc] = 0;
+listener->io_source[listener->nsioc] = NULL;
 
 object_ref(OBJECT(sioc));
 listener->connected = true;
 
 if (listener->io_func != NULL) {
 object_ref(OBJECT(listener));
-listener->io_tag[listener->nsioc] = qio_channel_add_watch(
+listener->io_source[listener->nsioc] = qio_channel_add_watch_source(
 QIO_CHANNEL(listener->sioc[listener->nsioc]), G_IO_IN,
 qio_net_listener_channel_func,
-listener, (GDestroyNotify)object_unref);
+listener, (GDestroyNotify)object_unref, NULL);
 }
 
 listener->nsioc++;
 }
 
 
-void qio_net_listener_set_client_func(QIONetListener *listener,
-  QIONetListenerClientFunc func,
-  gpointer data,
-  GDestroyNotify notify)
+void qio_net_listener_set_client_func_full(QIONetListener *listener,
+   QIONetListenerClientFunc func,
+   gpointer data,
+   GDestroyNotify notify,
+   GMainContext *context)
 {
 size_t i;
 
@@ -152,23 +155,32 @@ void qio_net_listener_set_client_func(QIONetListener 
*listener,
 listener->io_notify = notify;
 
 for (i = 0; i < listener->nsioc; i++) {
-if (listener->io_tag[i]) {
-g_source_remove(listener->io_tag[i]);
-listener->io_tag[i] = 0;
+if (listener->io_source[i]) {
+g_source_destroy(listener->io_source[i]);
+g_source_unref(listener->io_source[i]);
+listener->io_source[i] = NULL;
 }
 }
 
 if (listener->io_func != NULL) {
 for (i = 0; i < listener->nsioc; i++) {
 object_ref(OBJECT(listener));
-listener->io_tag[i] = qio_channel_add_watch(
+li

Re: [Qemu-devel] [PATCH v2 0/9] chardev: qio related non-default context support

2018-03-07 Thread Stefan Hajnoczi
On Tue, Mar 06, 2018 at 01:33:11PM +0800, Peter Xu wrote:
> Based-on: <20180305064324.9238-1-pet...@redhat.com>
> 
> This series is based on the QIO part:
>   [PATCH v3 0/6] qio: general non-default GMainContext support
> 
> v2:
> - fix the reported problem by patchew in patch 5
> - added some r-bs from Marc-Andre
> 
> Please review, thanks.
> 
> Peter Xu (9):
>   vl: export machine_init_done
>   chardev: fix leak in tcp_chr_telnet_init_io()
>   chardev: update net listener gcontext
>   chardev: allow telnet gsource to switch gcontext
>   chardev: introduce chr_machine_done hook
>   chardev: use chardev's gcontext for async connect
>   chardev: tcp: postpone async connection setup
>   chardev: tcp: let TLS run on chardev context
>   chardev: tcp: postpone TLS work until machine done
> 
>  chardev/char-mux.c |  33 --
>  chardev/char-socket.c  | 153 
> ++---
>  chardev/char.c |  43 +
>  include/chardev/char-mux.h |   2 -
>  include/chardev/char.h |   2 +
>  include/sysemu/sysemu.h|   2 +
>  stubs/machine-init-done.c  |   2 +
>  tests/test-char.c  |   1 -
>  vl.c   |   4 +-
>  9 files changed, 169 insertions(+), 73 deletions(-)

I'm not very familiar with chardev or qio, so I defer this to Daniel
Berrange:

Acked-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature


[Qemu-devel] [PULL 0/6] Qio next patches

2018-03-07 Thread Daniel P . Berrangé
The following changes since commit f2bb2d14c2958f3f5aef456bd2cdb1ff99f4a562:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into 
staging (2018-03-05 16:41:20 +)

are available in the Git repository at:

  https://github.com/berrange/qemu tags/qio-next-pull-request

for you to fetch changes up to 1939ccdaa61ce6a1f57d83277b3d41d3a9ad3c58:

  qio: non-default context for TLS handshake (2018-03-06 10:19:07 +)





Peter Xu (6):
  qio: rename qio_task_thread_result
  qio: introduce qio_channel_add_watch_{full|source}
  qio: store gsources for net listeners
  qio: non-default context for threaded qtask
  qio: non-default context for async conn
  qio: non-default context for TLS handshake

 chardev/char-socket.c  |  5 ++--
 include/io/channel-socket.h| 15 ---
 include/io/channel-tls.h   |  5 +++-
 include/io/channel.h   | 44 
 include/io/net-listener.h  | 22 ++--
 include/io/task.h  |  7 +++--
 io/channel-socket.c| 18 -
 io/channel-tls.c   | 45 
 io/channel.c   | 40 -
 io/dns-resolver.c  |  3 ++-
 io/net-listener.c  | 58 ++
 io/task.c  | 22 +---
 migration/socket.c |  3 ++-
 migration/tls.c|  2 ++
 nbd/client.c   |  1 +
 nbd/server.c   |  1 +
 tests/test-io-channel-socket.c |  4 +--
 tests/test-io-channel-tls.c|  2 ++
 tests/test-io-task.c   |  2 ++
 ui/vnc-auth-vencrypt.c |  1 +
 ui/vnc-ws.c|  1 +
 21 files changed, 239 insertions(+), 62 deletions(-)

-- 
2.14.3




[Qemu-devel] [PULL 1/6] qio: rename qio_task_thread_result

2018-03-07 Thread Daniel P . Berrangé
From: Peter Xu 

It is strange that it was called gio_task_thread_result.  Rename it to
follow the naming rule of the file.

Reviewed-by: Daniel P. Berrange 
Signed-off-by: Peter Xu 
Signed-off-by: Daniel P. Berrangé 
---
 io/task.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/io/task.c b/io/task.c
index 3ce556017c..1a0a1c7185 100644
--- a/io/task.c
+++ b/io/task.c
@@ -80,7 +80,7 @@ struct QIOTaskThreadData {
 };
 
 
-static gboolean gio_task_thread_result(gpointer opaque)
+static gboolean qio_task_thread_result(gpointer opaque)
 {
 struct QIOTaskThreadData *data = opaque;
 
@@ -110,7 +110,7 @@ static gpointer qio_task_thread_worker(gpointer opaque)
  * the worker results
  */
 trace_qio_task_thread_exit(data->task);
-g_idle_add(gio_task_thread_result, data);
+g_idle_add(qio_task_thread_result, data);
 return NULL;
 }
 
-- 
2.14.3




Re: [Qemu-devel] [PATCH] block: make BDRV_POLL_WHILE() re-entrancy safe

2018-03-07 Thread Stefan Hajnoczi
On Tue, Mar 06, 2018 at 12:08:44PM +0100, Kevin Wolf wrote:
> Am 06.03.2018 um 11:53 hat Stefan Hajnoczi geschrieben:
> > Nested BDRV_POLL_WHILE() calls can occur.  Currently
> > assert(!bs_->wakeup) will fail when this happens.
> > 
> > This patch converts bs->wakeup from bool to a counter.
> > 
> > Nesting works correctly because outer BDRV_POLL_WHILE() callers evaluate
> > the condition again after the inner caller completes (invoking the inner
> > caller counts as aio_poll() progress).
> > 
> > Reported-by: "fuweiwei (C)" 
> > Cc: Paolo Bonzini 
> > Signed-off-by: Stefan Hajnoczi 
> 
> Doesn't this conflict with your own AIO_WAIT_WHILE() patch?

Yes, I wanted this patch to be easy for Weiwei to test without
dependencies.

AIO_WAIT_WHILE() has just hit qemu.git/master, so I'll rebase and send a
v2.

Stefan


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH v10 03/24] migration: Create tcp_port parameter

2018-03-07 Thread Daniel P . Berrangé
On Wed, Mar 07, 2018 at 11:59:49AM +0100, Juan Quintela wrote:
> It will be used to store the uri tcp_port parameter.  This is the only
> parameter than can change and we can need to be able to connect to it.
> 
> Signed-off-by: Juan Quintela 
> 
> --
> 
> This used to be uri parameter, but it has so many troubles to
> reproduce that it don't just make sense.
> ---
>  hmp.c |  3 +++
>  migration/migration.c |  8 
>  qapi/migration.json   | 19 ---
>  3 files changed, 27 insertions(+), 3 deletions(-)

> diff --git a/qapi/migration.json b/qapi/migration.json
> index 7f465a1902..b6ef193f47 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -490,6 +490,9 @@
>  # and a power of 2
>  # (Since 2.11)
>  #
> +# @x-tcp-port: Only used for tcp, to know what the real port is
> +# (Since 2.12)
> +#
>  # Since: 2.4
>  ##
>  { 'enum': 'MigrationParameter',
> @@ -498,7 +501,7 @@
> 'tls-creds', 'tls-hostname', 'max-bandwidth',
> 'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
> 'x-multifd-channels', 'x-multifd-page-count',
> -   'xbzrle-cache-size' ] }
> +   'xbzrle-cache-size', 'x-tcp-port' ] }
>  
>  ##
>  # @MigrateSetParameters:
> @@ -566,6 +569,10 @@
>  # needs to be a multiple of the target page size
>  # and a power of 2
>  # (Since 2.11)
> +#
> +# @x-tcp-port: Only used for tcp, to know what the real port is
> +# (Since 2.12)
> +#
>  # Since: 2.4
>  ##
>  # TODO either fuse back into MigrationParameters, or make
> @@ -584,7 +591,8 @@
>  '*block-incremental': 'bool',
>  '*x-multifd-channels': 'int',
>  '*x-multifd-page-count': 'int',
> -'*xbzrle-cache-size': 'size' } }
> +'*xbzrle-cache-size': 'size',
> +'*x-tcp-port': 'uint16'} }

This should not exist - this exposes this parameter in migate-set-parameters
as a end user settable property, which is definitely not desirable.

It is only something we should report with 'query-migrate' / 'info migrate'

>  # @migrate-set-parameters:
> @@ -667,6 +675,10 @@
>  # needs to be a multiple of the target page size
>  # and a power of 2
>  # (Since 2.11)
> +#
> +# @x-tcp-port: Only used for tcp, to know what the real port is
> +# (Since 2.12)
> +#
>  # Since: 2.4
>  ##
>  { 'struct': 'MigrationParameters',
> @@ -683,7 +695,8 @@
>  '*block-incremental': 'bool' ,
>  '*x-multifd-channels': 'uint8',
>  '*x-multifd-page-count': 'uint32',
> -'*xbzrle-cache-size': 'size' } }
> +'*xbzrle-cache-size': 'size',
> +'*x-tcp-port': 'uint16'} }

As mentioned in previous review, IMHO we should be reporting the full
socket address, and allow an array of them, since we're going to have
more than one address available. i.e.

   '*socket-address': ['SocketAddress']

It doesn't cover non-socket based URIs, but that's fine, because for those
the mgmt app already knows how the channel is setup. We just need the
array of SocketAddress, because for socket URIs, the hostname, gets turned
into an array of addresses, and the mgmt app can't discover them.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PULL 00/34] Misc patches for 2018-03-06

2018-03-07 Thread Thomas Huth
On 06.03.2018 14:18, Paolo Bonzini wrote:
> The following changes since commit 58e2e17dba49b43f4ac9de19468aeae1c787dcc2:
> 
>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging 
> (2018-03-06 11:20:44 +)
> 
> are available in the git repository at:
> 
> 
>   git://github.com/bonzini/qemu.git tags/for-upstream
[...]
> Paolo Bonzini (9):
>   g364fb: fix DirtyBitmapSnapshot leak
>   openpic_kvm: drop address_space_to_flatview call
>   memory: inline some performance-sensitive accessors
>   address_space_write: address_space_to_flatview needs RCU lock
>   address_space_read: address_space_to_flatview needs RCU lock
>   address_space_access_valid: address_space_to_flatview needs RCU lock
>   address_space_map: address_space_to_flatview needs RCU lock
>   address_space_rw: address_space_to_flatview needs RCU lock
>   Revert "build-sys: compile with -Og or -O1 when --enable-debug"

 Hi Paolo,

something in this PULL request caused a regression with the 40p machine,
it crashes now when QEMU is quit:

$ ppc-softmmu/qemu-system-ppc -M 40p -monitor stdio
QEMU 2.11.50 monitor - type 'help' for more information
(qemu) quit
qemu-system-ppc: include/qemu/rcu.h:89: rcu_read_unlock: Assertion 
`p_rcu_reader->depth != 0' failed.
Aborted (core dumped)

(gdb) bt
#0  0x71e841b7 in raise () at /lib64/libc.so.6
#1  0x71e858a8 in abort () at /lib64/libc.so.6
#2  0x71e7cfd6 in __assert_fail_base () at /lib64/libc.so.6
#3  0x71e7d082 in  () at /lib64/libc.so.6
#4  0x5583f786 in cpu_exec () at include/qemu/rcu.h:89
#5  0x5583f786 in cpu_exec (cpu=cpu@entry=0x77dff010) at 
accel/tcg/cpu-exec.c:740
#6  0x55811e0d in qemu_tcg_rr_cpu_thread_fn (cpu=0x77dff010) at 
cpus.c:1341
#7  0x55811e0d in qemu_tcg_rr_cpu_thread_fn (arg=) at 
cpus.c:1435
#8  0x770d8dd5 in start_thread () at /lib64/libpthread.so.0
#9  0x71f4caed in clone () at /lib64/libc.so.6

Any ideas how to fix this?

 Thomas



Re: [Qemu-devel] [PATCH V4 4/7] CAN bus Kvaser PCI CAN-S (single SJA1000 channel) emulation added.

2018-03-07 Thread Paolo Bonzini
On 06/03/2018 21:52, Pavel Pisa wrote:
> Hello Thomas,
> 
> thanks for report but I at this time I am and
> can be some time in condition which does not allow
> me to access e-mail and normal work
> 
> On Tuesday 06 of March 2018 16:29:19 Thomas Huth wrote:
>> On 14.01.2018 21:14, p...@cmp.felk.cvut.cz wrote:
>>> From: Pavel Pisa 
>>>
>>> Signed-off-by: Pavel Pisa 
>>> ---
>>>  default-configs/pci.mak |   1 +
>>>  hw/can/Makefile.objs|   1 +
>>>  hw/can/can_kvaser_pci.c | 375
>>>  3 files changed, 377
>>> insertions(+)
>>>  create mode 100644 hw/can/can_kvaser_pci.c
>>
>>  Hi,
>>
>> the kvaser_pci device introduced a new way to crash QEMU, e.g.:
>>
>> mips64el-softmmu/qemu-system-mips64el -M malta,accel=qtest \
>>   -device kvaser_pci
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> 0x55a6e2ec in can_bus_insert_client (bus=0x0,
>> client=client@entry=0x570c4018) at
>> /home/thuth/devel/qemu/net/can/can_core.c:50
>> 50   QTAILQ_INSERT_TAIL(&bus->clients, client, next);
> 
> The reason is that parameters canbus0 and canbus1 are required.
> 
>   -object can-bus,id=canbus0 \
>   -device kvaser_pci,canbus0=canbus0
> 
> This could be be fast fix but plead somebody else to send regular
> patch.
> 
> --- a/net/can/can_host.c
> +++ b/net/can/can_host.c
> @@ -57,6 +57,10 @@ static void can_host_connect(CanHostState *ch, Error 
> **errp)
>  return;
>  }
> 
> +if (ch->bus_client == NULL) {
> +error_setg(errp, "bus is not specified for given device.");
> +return;
> +}
>  can_bus_insert_client(ch->bus, &ch->bus_client);
>  }

Ok, I'll check it out.

Thanks Thomas for the report!

Paolo



Re: [Qemu-devel] [PATCH v10 04/24] migration: Set the migration tcp port

2018-03-07 Thread Daniel P . Berrangé
On Wed, Mar 07, 2018 at 11:59:50AM +0100, Juan Quintela wrote:
> We can set the port parameter as zero.  This patch lets us know what
> port the system was choosen for us.  Now we can migrate to this place.
> 
> Signed-off-by: Juan Quintela 
> 
> --
> 
> This was migrate_set_uri(), but as we only need the tcp_port, change
> to that one.
> ---
>  migration/migration.c | 10 ++
>  migration/migration.h |  2 ++
>  migration/socket.c| 35 ++-
>  3 files changed, 42 insertions(+), 5 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 31b16a335b..c398665de7 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -268,6 +268,16 @@ int migrate_send_rp_req_pages(MigrationIncomingState 
> *mis, const char *rbname,
>  return migrate_send_rp_message(mis, msg_type, msglen, bufc);
>  }
>  
> +void migrate_set_port(const uint16_t port, Error **errp)
> +{
> +MigrateSetParameters p = {
> +.has_x_tcp_port = true,
> +.x_tcp_port = port,
> +};
> +
> +qmp_migrate_set_parameters(&p, errp);
> +}

This is really not nice - it is requiring the QMP  'migrate-set-parameters'
command to accept an extra field that is never something we want the end
user to be allowed to set. We should not use the public QMP schema for
data items we are just passing between 2 internal pieces of code.

>  void qemu_start_incoming_migration(const char *uri, Error **errp)
>  {
>  const char *p;
> diff --git a/migration/migration.h b/migration/migration.h
> index 08c5d2ded1..f40014cf94 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -234,4 +234,6 @@ void migrate_send_rp_pong(MigrationIncomingState *mis,
>  int migrate_send_rp_req_pages(MigrationIncomingState *mis, const char* 
> rbname,
>ram_addr_t start, size_t len);
>  
> +void migrate_set_port(const uint16_t port, Error **errp);
> +
>  #endif
> diff --git a/migration/socket.c b/migration/socket.c
> index e090097077..08606c665d 100644
> --- a/migration/socket.c
> +++ b/migration/socket.c
> @@ -15,6 +15,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
>  
>  #include "qemu-common.h"
>  #include "qemu/error-report.h"
> @@ -160,17 +161,24 @@ out:
>  }
>  
>  
> -static void socket_start_incoming_migration(SocketAddress *saddr,
> -Error **errp)
> +static SocketAddress *socket_start_incoming_migration(SocketAddress *saddr,
> +  Error **errp)
>  {
>  QIOChannelSocket *listen_ioc = qio_channel_socket_new();
> +SocketAddress *address;
>  
>  qio_channel_set_name(QIO_CHANNEL(listen_ioc),
>   "migration-socket-listener");
>  
>  if (qio_channel_socket_listen_sync(listen_ioc, saddr, errp) < 0) {
>  object_unref(OBJECT(listen_ioc));
> -return;
> +return NULL;
> +}
> +
> +address = qio_channel_socket_get_local_address(listen_ioc, errp);
> +if (address < 0) {
> +object_unref(OBJECT(listen_ioc));
> +return NULL;
>  }
>  
>  qio_channel_add_watch(QIO_CHANNEL(listen_ioc),
> @@ -178,14 +186,28 @@ static void 
> socket_start_incoming_migration(SocketAddress *saddr,
>socket_accept_incoming_migration,
>listen_ioc,
>(GDestroyNotify)object_unref);
> +return address;
>  }
>  
>  void tcp_start_incoming_migration(const char *host_port, Error **errp)
>  {
>  Error *err = NULL;
>  SocketAddress *saddr = tcp_build_address(host_port, &err);
> +
>  if (!err) {
> -socket_start_incoming_migration(saddr, &err);
> +SocketAddress *address = socket_start_incoming_migration(saddr, 
> &err);
> +
> +if (address) {
> +unsigned long long port;
> +
> +if (parse_uint_full(address->u.inet.port, &port, 10) < 0) {
> +error_setg(errp, "error parsing port in '%s'",
> +   address->u.inet.port);
> +} else {
> +migrate_set_port(port, errp);
> +}
> +qapi_free_SocketAddress(address);
> +}
>  }
>  qapi_free_SocketAddress(saddr);
>  error_propagate(errp, err);
> @@ -194,6 +216,9 @@ void tcp_start_incoming_migration(const char *host_port, 
> Error **errp)
>  void unix_start_incoming_migration(const char *path, Error **errp)
>  {
>  SocketAddress *saddr = unix_build_address(path);
> -socket_start_incoming_migration(saddr, errp);
> +SocketAddress *address;
> +
> +address = socket_start_incoming_migration(saddr, errp);
> +qapi_free_SocketAddress(address);
>  qapi_free_SocketAddress(saddr);
>  }
> -- 
> 2.14.3
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|

[Qemu-devel] [PATCH] virtio-blk: dataplane: Don't batch notifications if EVENT_IDX is present

2018-03-07 Thread Sergio Lopez
Commit 5b2ffbe4d99843fd8305c573a100047a8c962327 ("virtio-blk: dataplane:
notify guest as a batch") deferred guest notification to a BH in order
batch notifications, with purpose of avoiding flooding the guest with
interruptions.

This optimization came with a cost. The average latency perceived in the
guest is increased by a few microseconds, but also when multiple IO
operations finish at the same time, the guest won't be notified until
all completions from each operation has been run. On the contrary,
virtio-scsi issues the notification at the end of each completion.

On the other hand, nowadays we have the EVENT_IDX feature that allows a
better coordination between QEMU and the Guest OS to avoid sending
unnecessary interruptions.

With this change, virtio-blk/dataplane only batches notifications if the
EVENT_IDX feature is not present.

Some numbers obtained with fio (ioengine=sync, iodepth=1, direct=1):
 - Test specs:
   * fio-3.4 (ioengine=sync, iodepth=1, direct=1)
   * qemu master
   * virtio-blk with a dedicated iothread (default poll-max-ns)
   * backend: null_blk nr_devices=1 irqmode=2 completion_nsec=28
   * 8 vCPUs pinned to isolated physical cores
   * Emulator and iothread also pinned to separate isolated cores
   * variance between runs < 1%

 - Not patched
   * numjobs=1:  lat_avg=327.32  irqs=29998
   * numjobs=4:  lat_avg=337.89  irqs=29073
   * numjobs=8:  lat_avg=342.98  irqs=28643

 - Patched:
   * numjobs=1:  lat_avg=323.92  irqs=30262
   * numjobs=4:  lat_avg=332.65  irqs=29520
   * numjobs=8:  lat_avg=335.54  irqs=29323

Signed-off-by: Sergio Lopez 
---
 hw/block/dataplane/virtio-blk.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 2cb990997e..c46253a924 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -34,6 +34,7 @@ struct VirtIOBlockDataPlane {
 VirtIODevice *vdev;
 QEMUBH *bh; /* bh for guest notification */
 unsigned long *batch_notify_vqs;
+bool batch_notifications;
 
 /* Note that these EventNotifiers are assigned by value.  This is
  * fine as long as you do not call event_notifier_cleanup on them
@@ -47,8 +48,12 @@ struct VirtIOBlockDataPlane {
 /* Raise an interrupt to signal guest, if necessary */
 void virtio_blk_data_plane_notify(VirtIOBlockDataPlane *s, VirtQueue *vq)
 {
-set_bit(virtio_get_queue_index(vq), s->batch_notify_vqs);
-qemu_bh_schedule(s->bh);
+if (s->batch_notifications) {
+set_bit(virtio_get_queue_index(vq), s->batch_notify_vqs);
+qemu_bh_schedule(s->bh);
+} else {
+virtio_notify_irqfd(s->vdev, vq);
+}
 }
 
 static void notify_guest_bh(void *opaque)
@@ -177,6 +182,12 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
 
 s->starting = true;
 
+if (!virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
+s->batch_notifications = true;
+} else {
+s->batch_notifications = false;
+}
+
 /* Set up guest notifier (irq) */
 r = k->set_guest_notifiers(qbus->parent, nvqs, true);
 if (r != 0) {
-- 
2.14.3




Re: [Qemu-devel] [PATCH v10 10/24] migration: In case of error just end the migration

2018-03-07 Thread Daniel P . Berrangé
On Wed, Mar 07, 2018 at 11:59:56AM +0100, Juan Quintela wrote:
> Signed-off-by: Juan Quintela 
> ---
>  migration/socket.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)

Reviewed-by: Daniel P. Berrangé 

> diff --git a/migration/socket.c b/migration/socket.c
> index 08606c665d..b12b0a462e 100644
> --- a/migration/socket.c
> +++ b/migration/socket.c
> @@ -139,9 +139,8 @@ static gboolean 
> socket_accept_incoming_migration(QIOChannel *ioc,
>  sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(ioc),
>   &err);
>  if (!sioc) {
> -error_report("could not accept migration connection (%s)",
> - error_get_pretty(err));
> -goto out;
> +migrate_set_error(migrate_get_current(), err);
> +return G_SOURCE_REMOVE;

It will only return NULL if a client connected & then went away. This should
not happen with a "normal" mgmt app usage. On the flip side this allows a
malicious network attacker to inflict a denial of service on the migration
by simply connecting to target QEMU & immediately exiting.

Our "authentication" for migration relies on being able to validate the TLS
certs during TLS handshake. So in general we ought to allow repeated incoming
connections until we get a successful handshake. 

So in fact, I think a better fix here is to simply remove the original
'error_report' line, and ensure we return G_SOURCE_CONTINUE to wait for
another incoming connection from the real mgmt app.

>  }
>  
>  trace_migration_socket_incoming_accepted();
> @@ -150,7 +149,6 @@ static gboolean 
> socket_accept_incoming_migration(QIOChannel *ioc,
>  migration_channel_process_incoming(QIO_CHANNEL(sioc));
>  object_unref(OBJECT(sioc));
>  
> -out:
>  if (migration_has_all_channels()) {
>  /* Close listening socket as its no longer needed */
>  qio_channel_close(ioc, NULL);
> -- 
> 2.14.3
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v10 12/24] migration: Reference counting recv channels correctly

2018-03-07 Thread Daniel P . Berrangé
On Wed, Mar 07, 2018 at 11:59:58AM +0100, Juan Quintela wrote:
> Signed-off-by: Juan Quintela 
> ---
>  migration/socket.c | 11 +++
>  migration/socket.h |  7 +++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/migration/socket.c b/migration/socket.c
> index b12b0a462e..26110739cf 100644
> --- a/migration/socket.c
> +++ b/migration/socket.c
> @@ -27,6 +27,17 @@
>  #include "io/channel-socket.h"
>  #include "trace.h"
>  
> +int socket_recv_channel_ref(QIOChannel *recv)
> +{
> +object_ref(OBJECT(recv));
> +return 0;
> +}
> +
> +int socket_recv_channel_unref(QIOChannel *recv)
> +{
> +object_unref(OBJECT(recv));
> +return 0;
> +}

These helpers don't really add any value IMHO  - just call object_ref/unref
directly where needed. We don't provide explicit qio_channel_ref() wrappers
around object_ref because they add no value.

>  
>  static SocketAddress *tcp_build_address(const char *host_port, Error **errp)
>  {
> diff --git a/migration/socket.h b/migration/socket.h
> index 6b91e9db38..638a85255a 100644
> --- a/migration/socket.h
> +++ b/migration/socket.h
> @@ -16,6 +16,13 @@
>  
>  #ifndef QEMU_MIGRATION_SOCKET_H
>  #define QEMU_MIGRATION_SOCKET_H
> +
> +#include "io/channel.h"
> +#include "io/task.h"
> +
> +int socket_recv_channel_ref(QIOChannel *recv);
> +int socket_recv_channel_unref(QIOChannel *recv);
> +
>  void tcp_start_incoming_migration(const char *host_port, Error **errp);
>  
>  void tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
> -- 
> 2.14.3
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v10 17/24] migration: Export functions to create send channels

2018-03-07 Thread Daniel P . Berrangé
On Wed, Mar 07, 2018 at 12:00:03PM +0100, Juan Quintela wrote:
> Signed-off-by: Juan Quintela 
> ---
>  migration/socket.c | 28 +++-
>  migration/socket.h |  3 +++
>  2 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/socket.c b/migration/socket.c
> index 26110739cf..b3b5571ebb 100644
> --- a/migration/socket.c
> +++ b/migration/socket.c
> @@ -39,6 +39,28 @@ int socket_recv_channel_unref(QIOChannel *recv)
>  return 0;
>  }
>  
> +struct SocketOutgoingArgs {
> +SocketAddress *saddr;
> +} outgoing_args;
> +
> +void socket_send_channel_create(void (*f)(QIOTask *, gpointer), void *data)

This should use the proper typedef

   socket_send_channel_create(QIOTaskFunc f, void *data)

> +{
> +QIOChannelSocket *sioc = qio_channel_socket_new();
> +qio_channel_socket_connect_async(sioc, outgoing_args.saddr,
> + f, data, NULL);
> +}
> +
> +int socket_send_channel_destroy(QIOChannel *send)
> +{
> +/* Remove channel */
> +object_unref(OBJECT(send));
> +if (outgoing_args.saddr) {
> +qapi_free_SocketAddress(outgoing_args.saddr);
> +outgoing_args.saddr = NULL;
> +}
> +return 0;
> +}
> +
>  static SocketAddress *tcp_build_address(const char *host_port, Error **errp)
>  {
>  SocketAddress *saddr;
> @@ -106,6 +128,11 @@ static void 
> socket_start_outgoing_migration(MigrationState *s,
>  struct SocketConnectData *data = g_new0(struct SocketConnectData, 1);
>  
>  data->s = s;
> +
> +/* in case previous migration leaked it */
> +qapi_free_SocketAddress(outgoing_args.saddr);
> +outgoing_args.saddr = saddr;
> +
>  if (saddr->type == SOCKET_ADDRESS_TYPE_INET) {
>  data->hostname = g_strdup(saddr->u.inet.host);
>  }
> @@ -116,7 +143,6 @@ static void 
> socket_start_outgoing_migration(MigrationState *s,
>   socket_outgoing_migration,
>   data,
>   socket_connect_data_free);
> -qapi_free_SocketAddress(saddr);
>  }
>  
>  void tcp_start_outgoing_migration(MigrationState *s,
> diff --git a/migration/socket.h b/migration/socket.h
> index 638a85255a..cbdb8d64c3 100644
> --- a/migration/socket.h
> +++ b/migration/socket.h
> @@ -23,6 +23,9 @@
>  int socket_recv_channel_ref(QIOChannel *recv);
>  int socket_recv_channel_unref(QIOChannel *recv);
>  
> +void socket_send_channel_create(void (*f)(QIOTask *, gpointer), void *data);

Again use the proper typedef for the callback


> +int socket_send_channel_destroy(QIOChannel *send);
> +
>  void tcp_start_incoming_migration(const char *host_port, Error **errp);
>  
>  void tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
> -- 
> 2.14.3
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v10 18/24] migration: Add multifd traces for start/end thread

2018-03-07 Thread Daniel P . Berrangé
On Wed, Mar 07, 2018 at 12:00:04PM +0100, Juan Quintela wrote:
> Signed-off-by: Juan Quintela 
> ---
>  migration/ram.c| 6 ++
>  migration/trace-events | 4 
>  2 files changed, 10 insertions(+)

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v3 5/5] tests/bios-tables-test: add test cases for DIMM proximity

2018-03-07 Thread Igor Mammedov
On Mon,  5 Mar 2018 14:57:10 +0800
Haozhong Zhang  wrote:

> QEMU now builds one SRAT memory affinity structure for each
> static-plugged PC-DIMM and NVDIMM device with the proximity domain
> specified in the device option 'node', rather than only one SRAT
> memory affinity structure covering the entire hotpluggable address
> space with the proximity domain of the last node.
> 
> Add test cases on PC and Q35 machines with 3 proximity domains, and
> one PC-DIMM and one NVDIMM attached to the second proximity domain.
> Check whether the QEMU-built SRAT tables match with the expected ones.
> 
> Signed-off-by: Haozhong Zhang 
> Suggested-by: Igor Mammedov 
> ---
>  tests/acpi-test-data/pc/APIC.dimmpxm  | Bin 0 -> 136 bytes
>  tests/acpi-test-data/pc/DSDT.dimmpxm  | Bin 0 -> 6710 bytes
>  tests/acpi-test-data/pc/NFIT.dimmpxm  | Bin 0 -> 224 bytes
>  tests/acpi-test-data/pc/SRAT.dimmpxm  | Bin 0 -> 416 bytes
>  tests/acpi-test-data/pc/SSDT.dimmpxm  | Bin 0 -> 685 bytes
>  tests/acpi-test-data/q35/APIC.dimmpxm | Bin 0 -> 136 bytes
>  tests/acpi-test-data/q35/DSDT.dimmpxm | Bin 0 -> 9394 bytes
>  tests/acpi-test-data/q35/NFIT.dimmpxm | Bin 0 -> 224 bytes
>  tests/acpi-test-data/q35/SRAT.dimmpxm | Bin 0 -> 416 bytes
>  tests/acpi-test-data/q35/SSDT.dimmpxm | Bin 0 -> 685 bytes
>  tests/bios-tables-test.c  |  33 +
>  11 files changed, 33 insertions(+)
>  create mode 100644 tests/acpi-test-data/pc/APIC.dimmpxm
>  create mode 100644 tests/acpi-test-data/pc/DSDT.dimmpxm
>  create mode 100644 tests/acpi-test-data/pc/NFIT.dimmpxm
>  create mode 100644 tests/acpi-test-data/pc/SRAT.dimmpxm
>  create mode 100644 tests/acpi-test-data/pc/SSDT.dimmpxm
>  create mode 100644 tests/acpi-test-data/q35/APIC.dimmpxm
>  create mode 100644 tests/acpi-test-data/q35/DSDT.dimmpxm
>  create mode 100644 tests/acpi-test-data/q35/NFIT.dimmpxm
>  create mode 100644 tests/acpi-test-data/q35/SRAT.dimmpxm
>  create mode 100644 tests/acpi-test-data/q35/SSDT.dimmpxm
drop binary blobs from patch,
just add in patch description which tables new and differ
from default ones, like

Note to maintainer, please update/add following tables
  tests/acpi-test-data/pc/APIC.dimmpxm
  tests/acpi-test-data/pc/NFIT.dimmpxm
  tests/acpi-test-data/pc/SRAT.dimmpxm
  ...

If you wish to simplify testing for reviewers it's ok post
an additional patch with blobs and "DO NOT APPLY" note in subj.

> 
> diff --git a/tests/acpi-test-data/pc/APIC.dimmpxm 
> b/tests/acpi-test-data/pc/APIC.dimmpxm
> new file mode 100644
> index 
> ..658d7e748e37540ff85a02f4391efc7eaae3c8b4
> GIT binary patch
> literal 136
> zcmZ<^@O18AU|?W8>g4b25v<@85#a0y6k`O6f!H9Lf#JbFFwFr}2jX%tGD2u3CJ@cY
> q0}?#&4@5F?0WpXHVzIIUX 
> literal 0
> HcmV?d1
> 
> diff --git a/tests/acpi-test-data/pc/DSDT.dimmpxm 
> b/tests/acpi-test-data/pc/DSDT.dimmpxm
> new file mode 100644
> index 
> ..20e6433725bb3e70085cf6227f981106772bdaea
> GIT binary patch
> literal 6710
> zcmcgxUvJyi6~C9H9O_E4DVt54IBf){ZQ8C)^e1&vY#1z&vZYv*8BxwMFc>Mv!Q`St  
> z2satx2E`NwaMQjOT80hSgA(XD{s`Mg=tt z@44sR%jlNgUOGbv-KeZ zy|S@N|Jrr`;=1>~aB0UQo6nV}n;q}*6L*s!=>De17&eqe$ErAXf5Fu1dD*Ge^>q0g  
> zCdy7(ZxPwqsOwZQos1~+PE+aPH|zWFglB>Rzq^4yJTQ_q<#-N~s-
> zj@2#`4|`k>yE>n_OmTchclv|4EF zh=H6)gzOUKt&8Xlx@-4On>Pz3-`BKAD7a!4N}52}fwG(!gK1LTDmwuV1{QIb^P0e1
> z2JXK7yNk$zZxT|wL{2o!YLk+yMAXXI5VZ>YQM7ZHL~a<_?EVL>wg#lZkfmU-(BFCX  
> z+A8&kM-*X^&{euac8D;wOYHuYwTd3WMNv)qqY?$`zvvQ|P z=*5}2$rojoSR@Jp%kqk@MU!|U^k{+2uhQ?t??fW4(jUYhV4xP4$$OH|U07+DWj@&}
> zdVMyh5SC!;EKk`!6WCkuZc2@Li_7qbw4aa{12zLM14YM8jDiL))
> zn0g#icQ^&pJtEJfC}xFaR_O!rfhfz1J>Q?Iq^%nTKBx&AWFV)(35lb5DZUhmyr}pz
> zD@aqEpkYG912Y=SBfJ!VM+P3HCLbnI&(y3oO_3K&h7?CZgB;w*!9&m4J*#>RmZJOu
> zGb)9GR>@bdfuhnhS~R5u3KX zkBHKQB~)>gHHK-g>dY})ZQ{)eJ=Y_h=auBs4(oZJb(laly@xxUO~OQSd#DU<11Jg0  
> zrqNu}$=2}A!EHLs4mwPVx-GKxEE7p(0A&ZanGp3 zVF&Cz*`mbTdg6A-{m#g>AH8`=L~n7e;A*30>v~>>M*$y2e3WE$u6`Xxb(nm}dR z$q`GbOZDqoD+z#BK0D1)Urv#vpKQD2E9_$lc-Duhr(KA-i|rA1+x^A~2osVySdeKb
> zAXuJc7%MA#lFfZNO_E{)vXsHU6#UyI>P)Z((Ft@<%{qGBBA;WM_57Y0T-9WRF8T5)  
> z$7)&ht8U;0RI^qc`$OxM3G0x*KiPU=%zDAMUI72btryhetrzURfw5~7)|v1#%ooSZ
> z7k%@^3G>Am^Tp7-K4~5{h@TvNY0P}dH(#1CUy3nbvis{2L~l)+mjd%Vg>V;vDd%N3
> zrB3;it)_x8MpvN=XIIg+V8hect;3>kwyKc{HsvQ*Ml&~ZwY&GcPwxLdw{z#yyZ3i)  
> z-}#i-R5KfEVfoE4wo1u9*{5l!(U4Sr71?KL`_Xw6$|R@ZhNIP+7S&qD4GIMzPl=>y
> zh7P4>7D1wBRU0`#>g9G$O*{2wUG@le+Wpn5xBMyvz6Abd%9>fv=L>oCAlT0n>N{F{  
> z<+s9+4Z37c%jfgk*reDjY!6d|E)%d_+*WH-P}<5#`~0m-65sDIbPNd#)MPjK;1PFt
> zW-zJ1pgcC?+82&!8fzn0H4+%;&oe|Pses{Fezi8OSz1$$3xm4P%c+42J2h0$Nm3a2
> z;i_~bAb)~j6er;@C)7LQ8K6DtK3kK9wWC!2G#^jJ#G_mQ2d?7-HImx8)lSC+dhC21
> zaTX%>wvUO+W5Q%FLO-7DgdsTAJNxmPgxLm54}OrikXrHx6AW_GD7UjICKDNtWS~ts  
> znE03!M4fgZs1!y z!>Spv34+g-j$DI7#7yKH6F47qFTgTfHLFfFUxbYE&x*JtF%6+WCdMj>Q8R;DnK7GY  
> zzMNlz!GNtQW8C?}-JRWB_eenrj+Q|sVX%MAV zxve{Y+{W@G8+x zc^H`31|EfA|33yT8d%4S_390HSXxe<^EcpOa)3U;;SlzJn;QPfs61uB(2+wOtX2Cw
> z>JAQ`|LUoWZ+I>e4(A4eby!ibq;0`u  
> ze8SYgk0~f$KH62w

Re: [Qemu-devel] [PATCH v10 21/24] migration: Transmit initial package through the multifd channels

2018-03-07 Thread Daniel P . Berrangé
On Wed, Mar 07, 2018 at 12:00:07PM +0100, Juan Quintela wrote:
> Signed-off-by: Juan Quintela 
> ---
>  migration/ram.c | 50 +-
>  1 file changed, 45 insertions(+), 5 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 1aab96bd5e..4efac0c20c 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -52,6 +52,8 @@
>  #include "qemu/rcu_queue.h"
>  #include "migration/colo.h"
>  #include "migration/block.h"
> +#include "sysemu/sysemu.h"
> +#include "qemu/uuid.h"
>  
>  /***/
>  /* ram save/restore */
> @@ -491,12 +493,30 @@ static void multifd_send_sync_main(void)
>  trace_multifd_send_sync_main();
>  }
>  
> +typedef struct {
> +uint32_t version;
> +unsigned char uuid[16]; /* QemuUUID */
> +uint8_t id;
> +} __attribute__((packed)) MultiFDInit_t;
> +
>  static void *multifd_send_thread(void *opaque)
>  {
>  MultiFDSendParams *p = opaque;
> +MultiFDInit_t msg;
> +Error *local_err = NULL;
> +size_t ret;
>  
>  trace_multifd_send_thread_start(p->id);
>  
> +msg.version = 1;

I'm thinking we should standardize byte-order for this, as you could be
migrating between qemu-system-x86_64 running TCG on PPC, to a another
qemu-system-x86_64 running TCG on AArch64, and so have mixed-endianness.

Just a 'msg.version = htonl(1) call to set network byte order ?

> +msg.id = p->id;
> +memcpy(msg.uuid, &qemu_uuid.data, sizeof(msg.uuid));
> +ret = qio_channel_write_all(p->c, (char *)&msg, sizeof(msg), &local_err);
> +if (ret != 0) {
> +terminate_multifd_send_threads(local_err);
> +return NULL;
> +}
> +
>  while (true) {
>  qemu_sem_wait(&p->sem);
>  qemu_mutex_lock(&p->mutex);
> @@ -730,12 +750,32 @@ bool multifd_recv_all_channels_created(void)
>  void multifd_recv_new_channel(QIOChannel *ioc)
>  {
>  MultiFDRecvParams *p;
> -/* we need to invent channels id's until we transmit */
> -/* we will remove this on a later patch */
> -static int i = 0;
> +MultiFDInit_t msg;
> +Error *local_err = NULL;
> +size_t ret;
>  
> -p = &multifd_recv_state->params[i];
> -i++;
> +ret = qio_channel_read_all(ioc, (char *)&msg, sizeof(msg), &local_err);
> +if (ret != 0) {
> +terminate_multifd_recv_threads(local_err);
> +return;
> +}
> +
> +if (memcmp(msg.uuid, &qemu_uuid, sizeof(qemu_uuid))) {
> +char *uuid = qemu_uuid_unparse_strdup(&qemu_uuid);
> +error_setg(&local_err, "multifd: received uuid '%s' and expected "
> +   "uuid '%s' for channel %hhd", msg.uuid, uuid, msg.id);
> +g_free(uuid);
> +terminate_multifd_recv_threads(local_err);
> +return;
> +}
> +
> +p = &multifd_recv_state->params[msg.id];

And  ntohl(msg.id) here

Also, since we're indexnig into an array using data off the network,
we should validate the index is in range to avoid out of bounds memory
access.

> +if (p->c != NULL) {
> +error_setg(&local_err, "multifd: received id '%d' already setup'",
> +   msg.id);
> +terminate_multifd_recv_threads(local_err);
> +return;
> +}
>  p->c = ioc;
>  socket_recv_channel_ref(ioc);


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v10 18/24] migration: Add multifd traces for start/end thread

2018-03-07 Thread Daniel P . Berrangé
On Wed, Mar 07, 2018 at 12:00:04PM +0100, Juan Quintela wrote:
> Signed-off-by: Juan Quintela 
> ---
>  migration/ram.c| 6 ++
>  migration/trace-events | 4 
>  2 files changed, 10 insertions(+)
> 

> diff --git a/migration/trace-events b/migration/trace-events
> index 76075c26bc..db88fa699f 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -79,6 +79,10 @@ ram_save_page(const char *rbname, uint64_t offset, void 
> *host) "%s: offset: 0x%"
>  ram_save_queue_pages(const char *rbname, size_t start, size_t len) "%s: 
> start: 0x%zx len: 0x%zx"
>  multifd_send_sync_main(void) ""
>  multifd_recv_sync_main(void) ""
> +multifd_send_thread_start(int id) "%d"
> +multifd_send_thread_end(int id) "%d"
> +multifd_recv_thread_start(int id) "%d"
> +multifd_recv_thread_end(int id) "%d"

Seems these should have been 'uint8_t' rather than 'int', avoiding the
change of type in later patches


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v10 22/24] migration: Create ram_multifd_page

2018-03-07 Thread Daniel P . Berrangé
On Wed, Mar 07, 2018 at 12:00:08PM +0100, Juan Quintela wrote:
> The function still don't use multifd, but we have simplified
> ram_save_page, xbzrle and RDMA stuff is gone.  We have added a new
> counter.
> 
> Signed-off-by: Juan Quintela 
> 
> --
> Add last_page parameter
> Add commets for done and address
> Remove multifd field, it is the same than normal pages
> Merge next patch, now we send multiple pages at a time
> Remove counter for multifd pages, it is identical to normal pages
> Use iovec's instead of creating the equivalent.
> Clear memory used by pages (dave)
> Use g_new0(danp)
> define MULTIFD_CONTINUE
> now pages member is a pointer
> Fix off-by-one in number of pages in one packet
> Remove RAM_SAVE_FLAG_MULTIFD_PAGE
> ---
>  migration/ram.c| 144 
> -
>  migration/trace-events |   3 +-
>  2 files changed, 144 insertions(+), 3 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 4efac0c20c..df9646ed2e 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -54,6 +54,7 @@
>  #include "migration/block.h"
>  #include "sysemu/sysemu.h"
>  #include "qemu/uuid.h"
> +#include "qemu/iov.h"
>  
>  /***/
>  /* ram save/restore */
> @@ -397,7 +398,19 @@ static void compress_threads_save_setup(void)
>  
>  /* Multiple fd's */
>  
> +typedef struct {
> +/* number of used pages */
> +uint32_t used;
> +/* number of allocated pages */
> +uint32_t allocated;
> +/* global number of generated multifd packets */
> +uint32_t seq;
> +struct iovec *iov;
> +RAMBlock *block;
> +} multifd_pages_t;
> +
>  struct MultiFDSendParams {
> +/* not changed */
>  uint8_t id;
>  char *name;
>  QemuThread thread;
> @@ -405,8 +418,15 @@ struct MultiFDSendParams {
>  QemuSemaphore sem;
>  QemuMutex mutex;
>  bool running;
> +/* protected by param mutex */
>  bool quit;
>  bool sync;
> +multifd_pages_t *pages;
> +/* how many patches has sent this channel */
> +uint32_t packets_sent;
> +/* protected by multifd mutex */
> +/* has the thread finish the last submitted job */
> +bool done;
>  };
>  typedef struct MultiFDSendParams MultiFDSendParams;
>  
> @@ -416,8 +436,31 @@ struct {
>  int count;
>  /* syncs main thread and channels */
>  QemuSemaphore sem_main;
> +QemuMutex mutex;
> +QemuSemaphore sem;
> +multifd_pages_t *pages;
>  } *multifd_send_state;
>  
> +static void multifd_pages_init(multifd_pages_t **ppages, size_t size)
> +{
> +multifd_pages_t *pages = g_new0(multifd_pages_t, 1);
> +
> +pages->allocated = size;
> +pages->iov = g_new0(struct iovec, size);
> +*ppages = pages;
> +}
> +
> +static void multifd_pages_clear(multifd_pages_t *pages)
> +{
> +pages->used = 0;
> +pages->allocated = 0;
> +pages->seq = 0;
> +pages->block = NULL;
> +g_free(pages->iov);
> +pages->iov = NULL;
> +g_free(pages);
> +}
> +
>  static void terminate_multifd_send_threads(Error *errp)
>  {
>  int i;
> @@ -464,10 +507,14 @@ int multifd_save_cleanup(Error **errp)
>  qemu_sem_destroy(&p->sem);
>  g_free(p->name);
>  p->name = NULL;
> +multifd_pages_clear(p->pages);
> +p->pages = NULL;
>  }
>  qemu_sem_destroy(&multifd_send_state->sem_main);
>  g_free(multifd_send_state->params);
>  multifd_send_state->params = NULL;
> +multifd_pages_clear(multifd_send_state->pages);
> +multifd_send_state->pages = NULL;
>  g_free(multifd_send_state);
>  multifd_send_state = NULL;
>  return ret;
> @@ -516,6 +563,7 @@ static void *multifd_send_thread(void *opaque)
>  terminate_multifd_send_threads(local_err);
>  return NULL;
>  }
> +qemu_sem_post(&multifd_send_state->sem);
>  
>  while (true) {
>  qemu_sem_wait(&p->sem);
> @@ -530,9 +578,23 @@ static void *multifd_send_thread(void *opaque)
>  qemu_mutex_unlock(&p->mutex);
>  break;
>  }
> +if (p->pages->used) {
> +p->pages->used = 0;
> +qemu_mutex_unlock(&p->mutex);
> +
> +trace_multifd_send(p->id, p->pages->seq, p->pages->used);
> +/* ToDo: send page here */
> +
> +qemu_mutex_lock(&multifd_send_state->mutex);
> +p->done = true;
> +p->packets_sent++;
> +qemu_mutex_unlock(&multifd_send_state->mutex);
> +qemu_sem_post(&multifd_send_state->sem);
> +continue;
> +}
>  qemu_mutex_unlock(&p->mutex);
>  }
> -trace_multifd_send_thread_end(p->id);
> +trace_multifd_send_thread_end(p->id, p->packets_sent);
>  
>  return NULL;
>  }
> @@ -571,7 +633,10 @@ int multifd_save_setup(void)
>  multifd_send_state->params = g_new0(MultiFDSendParams, thread_count);
>  atomic_set(&multifd_send_state->count, 0);
>  qemu_sem_init(&multifd_s

Re: [Qemu-devel] [PATCH v10 24/24] [RFC] migration: Send pages through the multifd channels

2018-03-07 Thread Daniel P . Berrangé
On Wed, Mar 07, 2018 at 12:00:10PM +0100, Juan Quintela wrote:
> Migration ends correctly, but there is still a race between clean up
> and last synchronization.
> 
> Signed-off-by: Juan Quintela 
> ---
>  migration/ram.c| 132 
> ++---
>  migration/trace-events |   3 +-
>  2 files changed, 126 insertions(+), 9 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 264d2e462a..577b448db3 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -398,6 +398,19 @@ static void compress_threads_save_setup(void)
>  
>  /* Multiple fd's */
>  
> +#define MULTIFD_MAGIC 0x112233d
> +#define MULTIFD_VERSION 1
> +
> +typedef struct {
> +uint32_t magic;
> +uint32_t version;
> +uint32_t size;
> +uint32_t used;
> +uint32_t seq;
> +char ramblock[256];
> +ram_addr_t offset[];
> +} __attribute__((packed)) MultiFDPacket_t;

Same question about byte ordering as earlier patches - if we have two
qemu-system-x86_64 binaries running TCG mode on hosts with different
endianness we need byte swapping

> +
>  typedef struct {
>  /* number of used pages */
>  uint32_t used;
> @@ -407,6 +420,8 @@ typedef struct {
>  uint32_t seq;
>  struct iovec *iov;
>  RAMBlock *block;
> +uint32_t packet_len;
> +MultiFDPacket_t *packet;
>  } multifd_pages_t;
>  
>  struct MultiFDSendParams {
> @@ -447,6 +462,8 @@ static void multifd_pages_init(multifd_pages_t **ppages, 
> size_t size)
>  
>  pages->allocated = size;
>  pages->iov = g_new0(struct iovec, size);
> +pages->packet_len = sizeof(MultiFDPacket_t) + sizeof(ram_addr_t) * size;
> +pages->packet = g_malloc0(pages->packet_len);
>  *ppages = pages;
>  }
>  
> @@ -458,6 +475,9 @@ static void multifd_pages_clear(multifd_pages_t *pages)
>  pages->block = NULL;
>  g_free(pages->iov);
>  pages->iov = NULL;
> +pages->packet_len = 0;
> +g_free(pages->packet);
> +pages->packet = NULL;
>  g_free(pages);
>  }
>  
> @@ -499,7 +519,6 @@ int multifd_save_cleanup(Error **errp)
>  
>  if (p->running) {
>  qemu_thread_join(&p->thread);
> -p->running = false;
>  }
>  socket_send_channel_destroy(p->c);
>  p->c = NULL;
> @@ -535,7 +554,16 @@ static void multifd_send_sync_main(void)
>  qemu_sem_post(&p->sem);
>  }
>  for (i = 0; i < migrate_multifd_channels(); i++) {
> -qemu_sem_wait(&multifd_send_state->sem_main);
> +MultiFDSendParams *p = &multifd_send_state->params[i];
> +bool wait = true;
> +
> +qemu_mutex_lock(&p->mutex);
> +wait = p->running;
> +qemu_mutex_unlock(&p->mutex);
> +
> +if (wait) {
> +qemu_sem_wait(&multifd_send_state->sem_main);
> +}
>  }
>  trace_multifd_send_sync_main();
>  }
> @@ -575,16 +603,37 @@ static void *multifd_send_thread(void *opaque)
>  continue;
>  }
>  if (p->quit) {
> +p->running = false;
>  qemu_mutex_unlock(&p->mutex);
>  break;
>  }
>  if (p->pages->used) {
> +MultiFDPacket_t *packet = p->pages->packet;
> +Error *local_err = NULL;
> +size_t ret;
> +
> +packet->used = p->pages->used;
>  p->pages->used = 0;
>  qemu_mutex_unlock(&p->mutex);
> +packet->magic = MULTIFD_MAGIC;
> +packet->version = MULTIFD_VERSION;
>  
> -trace_multifd_send(p->id, p->pages->seq, p->pages->used);
> -/* ToDo: send page here */
> -
> +strncpy(packet->ramblock, p->pages->block->idstr, 256);
> +packet->size = migrate_multifd_page_count();
> +packet->seq = p->pages->seq;
> +ret = qio_channel_write_all(p->c, (void *)packet,
> +p->pages->packet_len, &local_err);
> +if (ret != 0) {
> +terminate_multifd_send_threads(local_err);
> +return NULL;
> +}
> +trace_multifd_send(p->id, p->pages->seq, packet->used);
> +ret = qio_channel_writev_all(p->c, p->pages->iov,
> + packet->used, &local_err);
> +if (ret != 0) {
> +terminate_multifd_send_threads(local_err);
> +return NULL;
> +}
>  qemu_mutex_lock(&multifd_send_state->mutex);
>  p->done = true;
>  p->packets_sent++;
> @@ -763,7 +812,6 @@ int multifd_load_cleanup(Error **errp)
>  
>  if (p->running) {
>  qemu_thread_join(&p->thread);
> -p->running = false;
>  }
>  socket_recv_channel_unref(p->c);
>  p->c = NULL;
> @@ -801,17 +849,48 @@ static void multifd_recv_sync_main(void)
>  qemu_sem_post(&p->sem);
>  }
>  for (i = 0; i < migrate_multifd_channels(); i++) {
> -qemu_sem_w

[Qemu-devel] [PATCH] tests/boot-serial: Check the 40p machine, too

2018-03-07 Thread Thomas Huth
The "40p" machine is using the Open Hack'Ware BIOS, just like the "prep"
machine, so we can test it accordingly with the boot-serial tester, too.
While we're at it, also change the strings that we are using for the
"prep" machine, so that this test now also checks some CLI parameters.

Signed-off-by: Thomas Huth 
---
 NB: The 40p machine is currently broken - when the emulator is quit,
 there is a rcu_read_unlock assertion message printed out. This test
 here succeeds anyway since the qtest framework does not care about
 errors in the shutdown path yet.

 tests/boot-serial-test.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
index ece25c6..5b24cd2 100644
--- a/tests/boot-serial-test.c
+++ b/tests/boot-serial-test.c
@@ -75,11 +75,13 @@ typedef struct testdef {
 static testdef_t tests[] = {
 { "alpha", "clipper", "", "PCI:" },
 { "ppc", "ppce500", "", "U-Boot" },
-{ "ppc", "prep", "", "Open Hack'Ware BIOS" },
+{ "ppc", "prep", "-m 96", "Memory size: 96 MB" },
+{ "ppc", "40p", "-boot d", "Booting from device d" },
 { "ppc", "g3beige", "", "PowerPC,750" },
 { "ppc", "mac99", "", "PowerPC,G4" },
 { "ppc64", "ppce500", "", "U-Boot" },
-{ "ppc64", "prep", "", "Open Hack'Ware BIOS" },
+{ "ppc64", "prep", "-boot e", "Booting from device e" },
+{ "ppc64", "40p", "-m 192", "Memory size: 192 MB" },
 { "ppc64", "mac99", "", "PowerPC,970FX" },
 { "ppc64", "pseries", "", "Open Firmware" },
 { "ppc64", "powernv", "-cpu POWER8", "OPAL" },
-- 
1.8.3.1




Re: [Qemu-devel] [PULL] RISC-V QEMU Port Submission v8.2

2018-03-07 Thread Michael Clark
On Wed, 7 Mar 2018 at 11:11 PM, Richard W.M. Jones 
wrote:

> On Wed, Mar 07, 2018 at 01:09:29PM +1300, Michael Clark wrote:
> > Hopefully, this PR gets merged...
>
> I hope so too.  We've been testing v8 (substantially the same as v8.2)
> extensively, including SMP.  It's building hundreds of packages a day
> in the autobuilder, and being used for manual builds by several
> people.
>
> Please don't forget about the softfloat fix!  Although it's not
> specific to riscv-qemu, it is required for reliable operation and I
> don't see if in the v8.2 tree.


I’ll try to get it in. I’ve reviewed the new minmax code and it should not
be difficult to add IEEE-754 201x minimumNumber/maximumNumber. I just ran
out of time today.

It’s a relatively rare corner case where the RISC-V behaviour is different
when an sNaN operand and a valid operand are provided to fmin/fmax. The
IEEE-754 201x minimumNumber/maximumNumber always returns the valid operand
when passed anyNaN and a valid operand however an sNaN operand causes the
invalid flag to be raised, whereas iirc the IEEE-764 2008 minNum/maxNum
will raise the invalid flag and return the sNaN when one operand is an sNaN
and the other is valid. Both of their behaviours are the same when passed
qNaN and a valid operand.

I have the test suite which includes both signalling and quiet NaN
fmin/fmax test cases so it is easy to verify and RISC-V is the only QEMU
port to use the IEEE-754 201x  minimumNumber/maximumNumber semantics so we
can easily isolate and test the change without affecting existing code.

Michael


Re: [Qemu-devel] [Qemu-ppc] [PULL 00/30] ppc-for-2.12 queue 20180306

2018-03-07 Thread luigi burdo
HI,

i had test last git with Zoltan sam emultation but at start i recive this


./qemu-system-ppc -M sam460ex
qemu-system-ppc: 
/home/gigi/src/tags/ppc-for-2.12-20180306/include/qemu/rcu.h:89: 
rcu_read_unlock: asserzione "p_rcu_reader->depth != 0" non riuscita.
Annullato (core dump creato)


My machie is a Threadripper 1950x the distro is Ubuntu mate 17.10


Thanks

Luigi



Da: Qemu-ppc  per conto 
di Thomas Huth 
Inviato: martedì 6 marzo 2018 17:48
A: David Gibson; Mark Cave-Ayland
Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org
Oggetto: Re: [Qemu-ppc] [Qemu-devel] [PULL 00/30] ppc-for-2.12 queue 20180306

On 06.03.2018 05:01, David Gibson wrote:
> The following changes since commit f2bb2d14c2958f3f5aef456bd2cdb1ff99f4a562:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' 
> into staging (2018-03-05 16:41:20 +)
>
> are available in the Git repository at:
>
>   git://github.com/dgibson/qemu.git tags/ppc-for-2.12-20180306
>
> for you to fetch changes up to 21b786f607b11d888f90bbb8c3414500515d11e7:
>
>   PowerPC: Add TS bits into msr_mask (2018-03-06 13:16:29 +1100)
>
> 
> ppc patch queue 2018-03-06
>
> This pull request supersedes ppc-for-2.12-20180302 which had compile
> problems with some gcc versions.  It also contains a few additional
> patches.
>
> Highlights are:
> * New Sam460ex machine type
> * Yet more fixes related to vcpu id allocation for spapr
> * Numerous macio cleanupsr
> * Some enhancements to the Spectre/Meltdown fixes for pseries,
>   allowing use of a better mitigation for indirect branch based
>   exploits
> * New pseries machine types with Spectre/Meltdown mitigations
>   enabled (stop gap until libvirt and management understands the
>   machine options)
> * A handful of other fixes
>
> 
> BALATON Zoltan (5):
>   ppc440_uc: Fix unintialized variable warning with older gcc
>   ppc440: Add emulation of plb-pcix controller found in some 440 SoCs
>   roms: Added git submodule for u-boot-sam460 (firmware for sam460ex)
>   pc-bios: Added u-boot-sam460 firmware binary
>   ppc: Add aCube Sam460ex board
>
> David Engraf (1):
>   PPC: e500: Fix duplicate kernel load and device tree overlap
>
> Greg Kurz (3):
>   spapr: fix missing CPU core nodes in DT when running with TCG
>   spapr: register dummy ICPs later
>   spapr: harden code that depends on VSMT
>
> Mark Cave-Ayland (13):
>   macio: embed DBDMA device directly within macio
>   macio: move ESCC device within the macio device
>   heathrow: QOMify heathrow PIC
>   heathrow: convert to trace-events
>   heathrow: change heathrow_pic_init() to return the heathrow device
>   macio: move macio related structures and defines into separate macio.h 
> file
>   mac_oldworld: use object link to pass heathrow PIC object to macio
>   openpic: move KVM-specific declarations into separate openpic_kvm.h file
>   openpic: move OpenPIC state and related definitions to openpic.h
>   mac_newworld: use object link to pass OpenPIC object to macio
>   macio: move setting of CUDA timebase frequency to macio_common_realize()
>   macio: remove macio_init() function
>   adb: add trace-events for monitoring keyboard/mouse during bus 
> enumeration

Something in the recent commits introduced a new way to cause unexpected
aborts of QEMU:

$ ppc64-softmmu/qemu-system-ppc64 -monitor stdio
QEMU 2.11.50 monitor - type 'help' for more information
(qemu) device_add macio-newworld
Unexpected error in qemu_chr_fe_init() at
/home/thuth/devel/qemu/chardev/char-fe.c:222:
Device 'serial0' is in use
Aborted (core dumped)

Of course it does not make sense to add a macio-newworld device on the
pseries machine, but QEMU should not abort in this case - it should just
print an error message and continue afterwards. Any ideas how to fix this?

 Thomas



Re: [Qemu-devel] [Qemu-ppc] [PULL 00/30] ppc-for-2.12 queue 20180306

2018-03-07 Thread Thomas Huth
On 07.03.2018 13:16, luigi burdo wrote:
> HI,
> 
> i had test last git with Zoltan sam emultation but at start i recive this
> 
> ./qemu-system-ppc -M sam460ex
> qemu-system-ppc:
> /home/gigi/src/tags/ppc-for-2.12-20180306/include/qemu/rcu.h:89:
> rcu_read_unlock: asserzione "p_rcu_reader->depth != 0" non riuscita.
> Annullato (core dump creato)

I think that's a regression introduced with one of the latest commits.
Could you please try whether "git checkout 58e2e17dba49b43f4ac9de19468"
works better for you?

 Thomas



Re: [Qemu-devel] [PATCH v2 1/9] vl: export machine_init_done

2018-03-07 Thread Daniel P . Berrangé
On Tue, Mar 06, 2018 at 01:33:12PM +0800, Peter Xu wrote:
> We have that variable but not exported.  Export that so modules can have
> a way to poke on whether machine init has finished.
> 
> Meanwhile, set that up even before calling the notifiers, so that
> notifiers who may depend on this field will get a correct answer.
> 
> Suggested-by: Paolo Bonzini 
> Reviewed-by: Marc-André Lureau 
> Signed-off-by: Peter Xu 
> ---
>  include/sysemu/sysemu.h   | 2 ++
>  stubs/machine-init-done.c | 2 ++
>  vl.c  | 4 ++--
>  3 files changed, 6 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v3 1/3] migration: API to clear bits of guest free pages from the dirty bitmap

2018-03-07 Thread Dr. David Alan Gilbert
* Wei Wang (wei.w.w...@intel.com) wrote:
> This patch adds an API to clear bits corresponding to guest free pages
> from the dirty bitmap. Spilt the free page block if it crosses the QEMU
> RAMBlock boundary.
> 
> Signed-off-by: Wei Wang 
> CC: Dr. David Alan Gilbert 
> CC: Juan Quintela 
> CC: Michael S. Tsirkin 

Reviewed-by: Dr. David Alan Gilbert 

> ---
>  include/migration/misc.h |  2 ++
>  migration/ram.c  | 20 
>  2 files changed, 22 insertions(+)
> 
> diff --git a/include/migration/misc.h b/include/migration/misc.h
> index 77fd4f5..fae1acf 100644
> --- a/include/migration/misc.h
> +++ b/include/migration/misc.h
> @@ -14,11 +14,13 @@
>  #ifndef MIGRATION_MISC_H
>  #define MIGRATION_MISC_H
>  
> +#include "exec/cpu-common.h"
>  #include "qemu/notify.h"
>  
>  /* migration/ram.c */
>  
>  void ram_mig_init(void);
> +void qemu_guest_free_page_hint(void *addr, size_t len);
>  
>  /* migration/block.c */
>  
> diff --git a/migration/ram.c b/migration/ram.c
> index 5e33e5c..769a0f6 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2189,6 +2189,26 @@ static int ram_init_all(RAMState **rsp)
>  return 0;
>  }
>  
> +void qemu_guest_free_page_hint(void *addr, size_t len)
> +{
> +RAMBlock *block;
> +ram_addr_t offset;
> +size_t used_len, start, npages;
> +
> +for (used_len = len; len > 0; len -= used_len) {
> +block = qemu_ram_block_from_host(addr, false, &offset);
> +if (unlikely(offset + len > block->used_length)) {
> +used_len = block->used_length - offset;
> +addr += used_len;
> +}
> +
> +start = offset >> TARGET_PAGE_BITS;
> +npages = used_len >> TARGET_PAGE_BITS;
> +bitmap_clear(block->bmap, start, npages);
> +ram_state->migration_dirty_pages -= npages;
> +}
> +}
> +
>  /*
>   * Each of ram_save_setup, ram_save_iterate and ram_save_complete has
>   * long-running RCU critical section.  When rcu-reclaims in the code
> -- 
> 1.8.3.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH v2 3/9] chardev: update net listener gcontext

2018-03-07 Thread Daniel P . Berrangé
On Tue, Mar 06, 2018 at 01:33:14PM +0800, Peter Xu wrote:
> TCP chardevs can be using QIO network listeners working in the
> background when in listening mode.  However the network listeners are
> always running in main context.  This can race with chardevs that are
> running in non-main contexts.
> 
> To solve this, we need to re-setup the net listeners in
> tcp_chr_update_read_handler() with the newly cached gcontext.
> 
> Reviewed-by: Marc-André Lureau 
> Signed-off-by: Peter Xu 
> ---
>  chardev/char-socket.c | 26 --
>  1 file changed, 20 insertions(+), 6 deletions(-)

Reviewed-by: Daniel P. Berrangé 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v2 4/9] chardev: allow telnet gsource to switch gcontext

2018-03-07 Thread Daniel P . Berrangé
On Tue, Mar 06, 2018 at 01:33:15PM +0800, Peter Xu wrote:
> It was originally created by qio_channel_add_watch() so it's always
> assigning the task to main context.  Now we use the new API called
> qio_channel_add_watch_source() so that we get the GSource handle rather
> than the tag ID.
> 
> Meanwhile, caching the gsource and TCPChardevTelnetInit (which holds the
> handshake data) in SocketChardev.telnet_source so that we can also do
> dynamic context switch when update read handlers.
> 
> Signed-off-by: Peter Xu 
> ---
>  chardev/char-socket.c | 67 
> +++
>  1 file changed, 51 insertions(+), 16 deletions(-)

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v2 5/9] chardev: introduce chr_machine_done hook

2018-03-07 Thread Daniel P . Berrangé
On Tue, Mar 06, 2018 at 01:33:16PM +0800, Peter Xu wrote:
> Introduce ChardevClass.chr_machine_done() hook so that chardevs can run
> customized procedures after machine init.
> 
> There was an existing mux user already that did similar thing but used a
> raw machine done notifier.  Generalize it into a framework, and let the
> mux chardevs provide such a class-specific hook to achieve the same
> thing.  Then we can move the mux related code to the char-mux.c file.
> 
> Since at it, replace the mux_realized variable with the global
> machine_init_done varible.
> 
> This notifier framework will be further leverged by other type of
> chardevs soon.
> 
> Signed-off-by: Peter Xu 
> ---
>  chardev/char-mux.c | 33 +
>  chardev/char.c | 43 +--
>  include/chardev/char-mux.h |  2 --
>  include/chardev/char.h |  2 ++
>  tests/test-char.c  |  1 -
>  5 files changed, 48 insertions(+), 33 deletions(-)

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v3 3/3] migration: use the free page hint feature from balloon

2018-03-07 Thread Dr. David Alan Gilbert
* Wei Wang (wei.w.w...@intel.com) wrote:
> Start the free page optimization when the bulk stage starts. In case the
> guest is slow in reporting, actively stops it when the bulk stage ends.
> The optimization avoids sending guest free pages during the bulk stage.
> Currently, the optimization is added to precopy only.
> 
> Signed-off-by: Wei Wang 
> CC: Dr. David Alan Gilbert 
> CC: Juan Quintela 
> CC: Michael S. Tsirkin 

I think this is OK, but with the only problem being that postcopy will
break;  we need to disable the mechanism if postcopy is enabled.

Dave

> ---
>  migration/ram.c | 19 ++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 769a0f6..f6af7e6 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -51,6 +51,7 @@
>  #include "qemu/rcu_queue.h"
>  #include "migration/colo.h"
>  #include "migration/block.h"
> +#include "sysemu/balloon.h"
>  
>  /***/
>  /* ram save/restore */
> @@ -208,6 +209,8 @@ struct RAMState {
>  uint32_t last_version;
>  /* We are in the first round */
>  bool ram_bulk_stage;
> +/* The free pages optimization feature is supported */
> +bool free_page_support;
>  /* How many times we have dirty too many pages */
>  int dirty_rate_high_cnt;
>  /* these variables are used for bitmap sync */
> @@ -775,7 +778,7 @@ unsigned long migration_bitmap_find_dirty(RAMState *rs, 
> RAMBlock *rb,
>  unsigned long *bitmap = rb->bmap;
>  unsigned long next;
>  
> -if (rs->ram_bulk_stage && start > 0) {
> +if (rs->ram_bulk_stage && start > 0 && !rs->free_page_support) {
>  next = start + 1;
>  } else {
>  next = find_next_bit(bitmap, size, start);
> @@ -1225,6 +1228,10 @@ static bool find_dirty_block(RAMState *rs, 
> PageSearchStatus *pss, bool *again)
>  /* Flag that we've looped */
>  pss->complete_round = true;
>  rs->ram_bulk_stage = false;
> +if (rs->free_page_support) {
> +balloon_free_page_stop();
> +rs->free_page_support = false;
> +}
>  if (migrate_use_xbzrle()) {
>  /* If xbzrle is on, stop using the data compression at this
>   * point. In theory, xbzrle can do better than compression.
> @@ -1656,6 +1663,8 @@ static void ram_state_reset(RAMState *rs)
>  rs->last_page = 0;
>  rs->last_version = ram_list.version;
>  rs->ram_bulk_stage = true;
> +rs->free_page_support = balloon_free_page_support() &
> +!migration_in_postcopy();
>  }
>  
>  #define MAX_WAIT 50 /* ms, half buffered_file limit */
> @@ -2235,6 +2244,11 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>  return -1;
>  }
>  }
> +
> +if ((*rsp)->free_page_support) {
> +balloon_free_page_start();
> +}
> +
>  (*rsp)->f = f;
>  
>  rcu_read_lock();
> @@ -2329,6 +2343,9 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
>  
>  ret = qemu_file_get_error(f);
>  if (ret < 0) {
> +if (rs->ram_bulk_stage && rs->free_page_support) {
> +balloon_free_page_stop();
> +}
>  return ret;
>  }
>  
> -- 
> 1.8.3.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



Re: [Qemu-devel] [PATCH v2 6/9] chardev: use chardev's gcontext for async connect

2018-03-07 Thread Daniel P . Berrangé
On Tue, Mar 06, 2018 at 01:33:17PM +0800, Peter Xu wrote:
> Generalize the function to create the async QIO task connection.  Also,
> fix the context pointer to use the chardev's gcontext.
> 
> Reviewed-by: Paolo Bonzini 
> Signed-off-by: Peter Xu 
> ---
>  chardev/char-socket.c | 25 ++---
>  1 file changed, 14 insertions(+), 11 deletions(-)

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v2 8/9] chardev: tcp: let TLS run on chardev context

2018-03-07 Thread Daniel P . Berrangé
On Tue, Mar 06, 2018 at 01:33:19PM +0800, Peter Xu wrote:
> Now qio_channel_tls_handshake() is ready to receive the context.  Let
> socket chardev use it, then the TLS handshake of chardev will always be
> with the chardev's context.
> 
> Signed-off-by: Peter Xu 
> ---
>  chardev/char-socket.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



  1   2   3   4   >