Re: [Qemu-devel] [PATCH v11 09/14] target-mips: Add ASE DSP bit/manipulation instructions

2012-10-17 Thread Aurelien Jarno
On Wed, Oct 17, 2012 at 11:44:41AM +0800, Jia Liu wrote:
 Hi Aurelien,
 
  +#ifdef TARGET_MIPS64
  +case OPC_ABSQ_S_QH_DSP:
  +switch (op2) {
  +case OPC_REPL_OB:
  +check_dsp(ctx);
  +{
  +target_long temp;
  +
  +imm = (ctx-opcode  16)  0xFF;
  +temp = imm;
  +temp = (temp  8) | temp;
  +temp = (temp  16) | temp;
  +temp = (temp  32) | temp;
  +tcg_gen_movi_tl(cpu_gpr[ret], temp);
  +break;
  +}
 
  This hasn't been fixed, and thus is still wrong.
 
 
 Thank you for check this again.
 May you give me more comment about this please? I'm not sure what
 should do here.
 

The instruction is defined as:

| Replicate a immediate byte into all elements of an eight byte vector.
|
| Description: rd ← immediate || immediate || immediate || immediate || 
immediate ||
|   immediate || immediate || immediate

In your code, I only see the value replicated 4 times, not 8 times.


-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH v11 06/14] target-mips: Add ASE DSP arithmetic instructions

2012-10-17 Thread Aurelien Jarno
On Wed, Oct 17, 2012 at 12:54:45PM +0800, Jia Liu wrote:
  +if (ret == 0) {
  +/* Treat as NOP. */
  +MIPS_DEBUG(NOP);
  +return;
  +}
  +
  +if (v1 == 0) {
  +tcg_gen_movi_tl(v1_t, 0);
  +} else {
  +gen_load_gpr(v1_t, v1);
  +}
  +
  +if (v2 == 0) {
  +tcg_gen_movi_tl(v2_t, 0);
  +} else {
  +gen_load_gpr(v2_t, v2);
  +}
 
  You don't need to check if v1 or v2 == 0, this is already done in
  gen_load_gpr().
 
  Sorry I'm a little confused here, just remove the check code here is OK?
 

Yes, gen_load_gpr() is defined as:

| static inline void gen_load_gpr (TCGv t, int reg)
| {
| if (reg == 0)
| tcg_gen_movi_tl(t, 0);
| else
| tcg_gen_mov_tl(t, cpu_gpr[reg]);
| }

so the test for reg == 0 is redundant.

 Is this code OK?
 
 if (ret == 0) {
 /* Treat as NOP. */
 MIPS_DEBUG(NOP);
 return;
 }
 
 v1_t = tcg_temp_new();
 v2_t = tcg_temp_new();
 
 gen_load_gpr(v1_t, v1);
 gen_load_gpr(v2_t, v2);
 

Yes, it looks fine to me.
 
 
  +
  +switch (op1) {
  +/* OPC_MULT_G_2E is equal OPC_ADDUH_QB_DSP */
  +case OPC_MULT_G_2E:
  +check_dspr2(ctx);
  +switch (op2) {
  +case OPC_ADDUH_QB:
  +gen_helper_adduh_qb(cpu_gpr[ret], v1_t, v2_t);
  +break;
  +case OPC_ADDUH_R_QB:
  +gen_helper_adduh_r_qb(cpu_gpr[ret], v1_t, v2_t);
  +break;
  +case OPC_ADDQH_PH:
  +gen_helper_addqh_ph(cpu_gpr[ret], v1_t, v2_t);
  +break;
  +case OPC_ADDQH_R_PH:
  +gen_helper_addqh_r_ph(cpu_gpr[ret], v1_t, v2_t);
  +break;
  +case OPC_ADDQH_W:
  +gen_helper_addqh_w(cpu_gpr[ret], v1_t, v2_t);
  +break;
  +case OPC_ADDQH_R_W:
  +gen_helper_addqh_r_w(cpu_gpr[ret], v1_t, v2_t);
  +break;
  +case OPC_SUBUH_QB:
  +gen_helper_subuh_qb(cpu_gpr[ret], v1_t, v2_t);
  +break;
  +case OPC_SUBUH_R_QB:
  +gen_helper_subuh_r_qb(cpu_gpr[ret], v1_t, v2_t);
  +break;
  +case OPC_SUBQH_PH:
  +gen_helper_subqh_ph(cpu_gpr[ret], v1_t, v2_t);
  +break;
  +case OPC_SUBQH_R_PH:
  +gen_helper_subqh_r_ph(cpu_gpr[ret], v1_t, v2_t);
  +break;
  +case OPC_SUBQH_W:
  +gen_helper_subqh_w(cpu_gpr[ret], v1_t, v2_t);
  +break;
  +case OPC_SUBQH_R_W:
  +gen_helper_subqh_r_w(cpu_gpr[ret], v1_t, v2_t);
  +break;
  +}
  +break;
  +case OPC_ABSQ_S_PH_DSP:
  +switch (op2) {
  +case OPC_ABSQ_S_QB:
  +check_dspr2(ctx);
  +gen_helper_absq_s_qb(cpu_gpr[ret], v2_t, cpu_env);
  +break;
  +case OPC_ABSQ_S_PH:
  +check_dsp(ctx);
  +gen_helper_absq_s_ph(cpu_gpr[ret], v2_t, cpu_env);
  +break;
  +case OPC_ABSQ_S_W:
  +check_dsp(ctx);
  +gen_helper_absq_s_w(cpu_gpr[ret], v2_t, cpu_env);
  +break;
  +case OPC_PRECEQ_W_PHL:
  +{
  +TCGv temp = tcg_temp_new();;
  +check_dsp(ctx);
  +tcg_gen_andi_tl(temp, v2_t, 0x);
  +tcg_gen_ext32s_tl(cpu_gpr[ret], temp);
 
  Small optimization; you don't need the temp here, andi can directly
  write to cpu_gpr[ret].
 
  +tcg_temp_free(temp);
  +break;
  +}
  +case OPC_PRECEQ_W_PHR:
  +{
  +TCGv temp = tcg_temp_new();;
  +check_dsp(ctx);
  +tcg_gen_andi_tl(temp, v2_t, 0x);
 
  This is not needed due to the shift and the ext below.
 
  +tcg_gen_shli_tl(temp, temp, 16);
  +tcg_gen_ext32s_tl(cpu_gpr[ret], temp);
  +tcg_temp_free(temp);
  +break;
  +}
  +case OPC_PRECEQU_PH_QBL:
  +check_dsp(ctx);
  +gen_helper_precequ_ph_qbl(cpu_gpr[ret], v2_t);
  +break;
  +case OPC_PRECEQU_PH_QBR:
  +check_dsp(ctx);
  +gen_helper_precequ_ph_qbr(cpu_gpr[ret], v2_t);
  +break;
  +case OPC_PRECEQU_PH_QBLA:
  +check_dsp(ctx);
  +gen_helper_precequ_ph_qbla(cpu_gpr[ret], v2_t);
  +break;
  +case OPC_PRECEQU_PH_QBRA:
  +check_dsp(ctx);
  +gen_helper_precequ_ph_qbra(cpu_gpr[ret], v2_t);
  +break;
  +case OPC_PRECEU_PH_QBL:
  +check_dsp(ctx);
  +gen_helper_preceu_ph_qbl(cpu_gpr[ret], v2_t);
  +break;
  +case OPC_PRECEU_PH_QBR:
  +check_dsp(ctx);
  +gen_helper_preceu_ph_qbr(cpu_gpr[ret], v2_t);
  +break;
  +case 

Re: [Qemu-devel] [PATCH 1/2] hmp: fix info cpus for sparc targets

2012-10-17 Thread Aurelien Jarno
On Tue, Oct 16, 2012 at 11:01:45PM -0300, Luiz Capitulino wrote:
 On Wed, 17 Oct 2012 01:28:34 +0200
 Aurelien Jarno aurel...@aurel32.net wrote:
 
  On sparc targets, info cpus returns this kind of output:
  
  | info cpus
  | * CPU #0: 
  pc=0x00424d18pc=0x00424d18npc=0x00424d1c 
  thread_id=19460
  
  pc is printed twice, there is no space between pc, pc and npc. As npc
  implies pc, don't set has_pc on sparc, and add a space between pc and
  npc.
  
  Cc: Luiz Capitulino lcapitul...@redhat.com
  Cc: Markus Armbruster arm...@redhat.com
  Cc: Blue Swirl blauwir...@gmail.com
  Signed-off-by: Aurelien Jarno aurel...@aurel32.net
  ---
   cpus.c |2 --
   hmp.c  |2 +-
   2 files changed, 1 insertion(+), 3 deletions(-)
  
  diff --git a/cpus.c b/cpus.c
  index 750a76f..cbc8556 100644
  --- a/cpus.c
  +++ b/cpus.c
  @@ -1216,8 +1216,6 @@ CpuInfoList *qmp_query_cpus(Error **errp)
   info-value-has_nip = true;
   info-value-nip = env-nip;
   #elif defined(TARGET_SPARC)
  -info-value-has_pc = true;
  -info-value-pc = env-pc;
   info-value-has_npc = true;
   info-value-npc = env-npc;
   #elif defined(TARGET_MIPS)
  diff --git a/hmp.c b/hmp.c
  index 70bdec2..64b6ab5 100644
  --- a/hmp.c
  +++ b/hmp.c
  @@ -242,7 +242,7 @@ void hmp_info_cpus(Monitor *mon)
   monitor_printf(mon, nip=0x%016 PRIx64, cpu-value-nip);
   }
   if (cpu-value-has_npc) {
  -monitor_printf(mon, pc=0x%016 PRIx64, cpu-value-pc);
  +monitor_printf(mon, pc=0x%016  PRIx64, cpu-value-pc);
 
 This patch changes qmp_query_cpus() to never set pc, so why do you print it?

Well the print part of the code isn't change beside adding a space at
the end. With the current code, has_npc implies has_pc, that's why I
have removed the latter from qmp_query_cpus() in the sparc case.

If you prefer to change this behavior and not have has_npc implying
has_pc, I can send another patch.

   monitor_printf(mon, npc=0x%016 PRIx64, cpu-value-npc);
   }
   if (cpu-value-has_PC) {
 
 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH v2 0/5] target-mips: Preparations for CPUState part 4b series

2012-10-17 Thread Aurelien Jarno
On Fri, Oct 12, 2012 at 12:56:32AM +0200, Andreas Färber wrote:
 Hello Aurélien,
 
 This series picks up some preparatory patches for QOM CPUState refactoring,
 originally posted in May. They still applied cleanly, but I optimized them
 a bit and expanded the explanations.
 
 In short it is about MIPSCPU vs. CPUMIPSState; more fields will be moved
 from CPU_COMMON macro to CPUState struct, and to access CPUState we need
 a MIPSCPU. Thus MIPSCPU is preferable for arguments of static helpers
 (not TCG helpers) because we save some redundant accessor/cast macros.
 
 Can you please ack/apply and keep in mind for the current patch review?
 
 Available for testing from:
 git://github.com/afaerber/qemu-cpu.git qom-cpu-mips
 https://github.com/afaerber/qemu-cpu/commits/qom-cpu-mips
 
 Regards,
 Andreas
 
 Cc: Aurélien Jarno aurel...@aurel32.net
 
 Cc: Jia Liu pro...@gmail.com
 Cc: Eric Johnson er...@mips.com
 Cc: Richard Henderson r...@twiddle.net
 
 v1 - v2:
 * Cherry-picked from my CPUState part 4 series
 * Avoided calling mips_env_get_cpu() in both if branches of 
 helper_m[t]tc0_tchalt()
 * Prepended patch to clean up resulting variable naming mess
 * Placed variable declarations in the closest block (requested by Alex 
 elsewhere)
 
 Andreas Färber (5):
   target-mips: Clean up other_cpu in helper_{d,e}vpe()
   target-mips: Pass MIPSCPU to mips_tc_wake()
   target-mips: Pass MIPSCPU to mips_vpe_is_wfi()
   target-mips: Pass MIPSCPU to mips_tc_sleep()
   target-mips: Pass MIPSCPU to mips_vpe_sleep()
 
  target-mips/op_helper.c |   63 
 +--
  1 Datei geändert, 39 Zeilen hinzugefügt(+), 24 Zeilen entfernt(-)
 
 -- 
 1.7.10.4

Thanks, all applied.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH v11 09/14] target-mips: Add ASE DSP bit/manipulation instructions

2012-10-17 Thread Jia Liu
Hi Aurelien,

On Wed, Oct 17, 2012 at 2:05 PM, Aurelien Jarno aurel...@aurel32.net wrote:
 On Wed, Oct 17, 2012 at 11:44:41AM +0800, Jia Liu wrote:
 Hi Aurelien,

  +#ifdef TARGET_MIPS64
  +case OPC_ABSQ_S_QH_DSP:
  +switch (op2) {
  +case OPC_REPL_OB:
  +check_dsp(ctx);
  +{
  +target_long temp;
  +
  +imm = (ctx-opcode  16)  0xFF;
  +temp = imm;
  +temp = (temp  8) | temp;
  +temp = (temp  16) | temp;
  +temp = (temp  32) | temp;
  +tcg_gen_movi_tl(cpu_gpr[ret], temp);
  +break;
  +}
 
  This hasn't been fixed, and thus is still wrong.
 

 Thank you for check this again.
 May you give me more comment about this please? I'm not sure what
 should do here.


 The instruction is defined as:

 | Replicate a immediate byte into all elements of an eight byte vector.
 |
 | Description: rd ← immediate || immediate || immediate || immediate || 
 immediate ||
 |   immediate || immediate || immediate

 In your code, I only see the value replicated 4 times, not 8 times.


Thank you very much, is this code OK?

case OPC_REPL_OB:
check_dsp(ctx);
{
target_long temp;

imm = (ctx-opcode  16)  0xFF;
temp = ((uint64_t)imm  56) | ((uint64_t)imm  48) |
((uint64_t)imm  40) | ((uint64_t)imm  32) |
((uint64_t)imm  24) | ((uint64_t)imm  16) |
((uint64_t)imm  8) | (uint64_t)imm;
tcg_gen_movi_tl(cpu_gpr[ret], temp);
break;
}


 --
 Aurelien Jarno  GPG: 1024D/F1BCDB73
 aurel...@aurel32.net http://www.aurel32.net

Regards,
Jia.



[Qemu-devel] [PATCH 4/7] serial: add 2x + 4x pci variant

2012-10-17 Thread Gerd Hoffmann
Add multiport serial card implementation, with two variants, one
featuring two and one featuring four ports.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 hw/serial-pci.c |  149 +++
 1 files changed, 149 insertions(+), 0 deletions(-)

diff --git a/hw/serial-pci.c b/hw/serial-pci.c
index 6fcf117..badd297 100644
--- a/hw/serial-pci.c
+++ b/hw/serial-pci.c
@@ -26,11 +26,23 @@
 #include serial.h
 #include pci.h
 
+#define PCI_SERIAL_MAX_PORTS 4
+
 typedef struct PCISerialState {
 PCIDevice dev;
 SerialState state;
 } PCISerialState;
 
+typedef struct PCIMultiSerialState {
+PCIDevicedev;
+MemoryRegion iobar;
+uint32_t ports;
+char *name[PCI_SERIAL_MAX_PORTS];
+SerialState  state[PCI_SERIAL_MAX_PORTS];
+uint32_t level[PCI_SERIAL_MAX_PORTS];
+qemu_irq *irqs;
+} PCIMultiSerialState;
+
 static int serial_pci_init(PCIDevice *dev)
 {
 PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
@@ -47,6 +59,56 @@ static int serial_pci_init(PCIDevice *dev)
 return 0;
 }
 
+static void multi_serial_irq_mux(void *opaque, int n, int level)
+{
+PCIMultiSerialState *pci = opaque;
+int i, pending = 0;
+
+pci-level[n] = level;
+for (i = 0; i  pci-ports; i++) {
+if (pci-level[i]) {
+pending = 1;
+}
+}
+qemu_set_irq(pci-dev.irq[0], pending);
+}
+
+static int multi_serial_pci_init(PCIDevice *dev)
+{
+PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
+PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev);
+SerialState *s;
+int i;
+
+switch (pc-device_id) {
+case 0x0003:
+pci-ports = 2;
+break;
+case 0x0004:
+pci-ports = 4;
+break;
+}
+assert(pci-ports  0);
+assert(pci-ports = PCI_SERIAL_MAX_PORTS);
+
+pci-dev.config[PCI_INTERRUPT_PIN] = 0x01;
+memory_region_init(pci-iobar, multiserial, 8 * pci-ports);
+pci_register_bar(pci-dev, 0, PCI_BASE_ADDRESS_SPACE_IO, pci-iobar);
+pci-irqs = qemu_allocate_irqs(multi_serial_irq_mux, pci,
+   pci-ports);
+
+for (i = 0; i  pci-ports; i++) {
+s = pci-state + i;
+s-baudbase = 115200;
+serial_init_core(s);
+s-irq = pci-irqs[i];
+pci-name[i] = g_strdup_printf(uart #%d, i+1);
+memory_region_init_io(s-io, serial_io_ops, s, pci-name[i], 8);
+memory_region_add_subregion(pci-iobar, 8 * i, s-io);
+}
+return 0;
+}
+
 static void serial_pci_exit(PCIDevice *dev)
 {
 PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
@@ -56,6 +118,22 @@ static void serial_pci_exit(PCIDevice *dev)
 memory_region_destroy(s-io);
 }
 
+static void multi_serial_pci_exit(PCIDevice *dev)
+{
+PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev);
+SerialState *s;
+int i;
+
+for (i = 0; i  pci-ports; i++) {
+s = pci-state + i;
+serial_exit_core(s);
+memory_region_destroy(s-io);
+g_free(pci-name[i]);
+}
+memory_region_destroy(pci-iobar);
+qemu_free_irqs(pci-irqs);
+}
+
 static const VMStateDescription vmstate_pci_serial = {
 .name = pci-serial,
 .version_id = 1,
@@ -67,11 +145,38 @@ static const VMStateDescription vmstate_pci_serial = {
 }
 };
 
+static const VMStateDescription vmstate_pci_multi_serial = {
+.name = pci-serial-multi,
+.version_id = 1,
+.minimum_version_id = 1,
+.fields  = (VMStateField[]) {
+VMSTATE_PCI_DEVICE(dev, PCIMultiSerialState),
+VMSTATE_STRUCT_ARRAY(state, PCIMultiSerialState, PCI_SERIAL_MAX_PORTS,
+ 0, vmstate_serial, SerialState),
+VMSTATE_UINT32_ARRAY(level, PCIMultiSerialState, PCI_SERIAL_MAX_PORTS),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static Property serial_pci_properties[] = {
 DEFINE_PROP_CHR(chardev,  PCISerialState, state.chr),
 DEFINE_PROP_END_OF_LIST(),
 };
 
+static Property multi_2x_serial_pci_properties[] = {
+DEFINE_PROP_CHR(chardev1,  PCIMultiSerialState, state[0].chr),
+DEFINE_PROP_CHR(chardev2,  PCIMultiSerialState, state[1].chr),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static Property multi_4x_serial_pci_properties[] = {
+DEFINE_PROP_CHR(chardev1,  PCIMultiSerialState, state[0].chr),
+DEFINE_PROP_CHR(chardev2,  PCIMultiSerialState, state[1].chr),
+DEFINE_PROP_CHR(chardev3,  PCIMultiSerialState, state[2].chr),
+DEFINE_PROP_CHR(chardev4,  PCIMultiSerialState, state[3].chr),
+DEFINE_PROP_END_OF_LIST(),
+};
+
 static void serial_pci_class_initfn(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
@@ -86,6 +191,34 @@ static void serial_pci_class_initfn(ObjectClass *klass, 
void *data)
 dc-props = serial_pci_properties;
 }
 
+static void multi_2x_serial_pci_class_initfn(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+PCIDeviceClass *pc = 

[Qemu-devel] [PATCH 6/7] usb-serial: don't magically zap chardev on umplug

2012-10-17 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 hw/usb/dev-serial.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 69b6e48..43214cd 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -421,7 +421,7 @@ static void usb_serial_handle_destroy(USBDevice *dev)
 {
 USBSerialState *s = (USBSerialState *)dev;
 
-qemu_chr_delete(s-cs);
+qemu_chr_add_handlers(s-cs, NULL, NULL, NULL, NULL);
 }
 
 static int usb_serial_can_read(void *opaque)
-- 
1.7.1




[Qemu-devel] [PULL v4 0/7] serial device hotplug patch series.

2012-10-17 Thread Gerd Hoffmann
  Hi,

Next round of the serial device hotplug series.  Left out chardev
patches due to confligting in-flight patches and ongoing discussions,
carrying only pci-serial and usb-serial bits.

Adressed Anthonys comments:  Added license to serial.h and moved
documentation to docs/specs/pci-serial.txt.

please pull,
  Gerd

The following changes since commit 6f4d6b09088ee161ff4be0e4db4e4c0962c79070:

  target-mips: Pass MIPSCPU to mips_vpe_sleep() (2012-10-17 01:32:11 +0200)

are available in the git repository at:
  git://git.kraxel.org/qemu serial.3

Gerd Hoffmann (7):
  serial: split serial.c
  serial: add pci variant
  serial: add windows inf file for the pci card to docs
  serial: add 2x + 4x pci variant
  serial: add pci-serial documentation
  usb-serial: don't magically zap chardev on umplug
  usb-serial: only expose device in guest when the chardev is open

 default-configs/pci.mak   |2 +
 docs/qemupciserial.inf|  109 +++
 docs/specs/pci-serial.txt |   34 ++
 hw/Makefile.objs  |3 +-
 hw/alpha_dp264.c  |1 +
 hw/kzm.c  |2 +-
 hw/mips_fulong2e.c|1 +
 hw/mips_jazz.c|1 +
 hw/mips_malta.c   |1 +
 hw/mips_mipssim.c |2 +-
 hw/mips_r4k.c |1 +
 hw/musicpal.c |2 +-
 hw/omap_uart.c|3 +-
 hw/openrisc_sim.c |3 +-
 hw/pc.c   |1 +
 hw/pc.h   |   27 -
 hw/pci_ids.h  |1 +
 hw/petalogix_ml605_mmu.c  |2 +-
 hw/ppc/e500.c |2 +-
 hw/ppc405_uc.c|2 +-
 hw/ppc440_bamboo.c|2 +-
 hw/ppc_prep.c |1 +
 hw/pxa2xx.c   |2 +-
 hw/serial-isa.c   |  130 +++
 hw/serial-pci.c   |  252 +
 hw/serial.c   |  149 ++-
 hw/serial.h   |   99 ++
 hw/sm501.c|2 +-
 hw/sun4u.c|1 +
 hw/usb/dev-serial.c   |   21 +++-
 hw/virtex_ml507.c |2 +-
 hw/xtensa_lx60.c  |3 +-
 32 files changed, 681 insertions(+), 183 deletions(-)
 create mode 100644 docs/qemupciserial.inf
 create mode 100644 docs/specs/pci-serial.txt
 create mode 100644 hw/serial-isa.c
 create mode 100644 hw/serial-pci.c
 create mode 100644 hw/serial.h



[Qemu-devel] [PATCH 3/7] serial: add windows inf file for the pci card to docs

2012-10-17 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 docs/qemupciserial.inf |  109 
 1 files changed, 109 insertions(+), 0 deletions(-)
 create mode 100644 docs/qemupciserial.inf

diff --git a/docs/qemupciserial.inf b/docs/qemupciserial.inf
new file mode 100644
index 000..3474310
--- /dev/null
+++ b/docs/qemupciserial.inf
@@ -0,0 +1,109 @@
+; qemupciserial.inf for QEMU, based on MSPORTS.INF
+
+; The driver itself is shipped with Windows (serial.sys).  This is
+; just a inf file to tell windows which pci id the serial pci card
+; emulated by qemu has, and to apply a name tag to it which windows
+; will show in the device manager.
+
+; Installing the driver: Go to device manager.  You should find a pci
+; serial card tagged with a yellow question mark.  Open properties.
+; Pick update driver.  Then select driver manually.  Pick Ports
+; (Com+Lpt) from the list.  Click Have a disk.  Select this file.
+; Procedure may vary a bit depending on the windows version.
+
+; FIXME: This file covers the single port version only.
+
+[Version]
+Signature=$CHICAGO$
+Class=Ports
+ClassGuid={4D36E978-E325-11CE-BFC1-08002BE10318}
+Provider=%QEMU%
+DriverVer=09/24/2012,1.3.0
+
+[SourceDisksNames]
+3426=windows cd
+
+[SourceDisksFiles]
+serial.sys = 3426
+serenum.sys= 3426
+
+[DestinationDirs]
+DefaultDestDir  = 11;LDID_SYS
+ComPort.NT.Copy = 12;DIRID_DRIVERS
+SerialEnumerator.NT.Copy=12 ;DIRID_DRIVERS
+
+; Drivers
+;--
+[Manufacturer]
+%QEMU%=QEMU,NTx86
+
+[QEMU.NTx86]
+%QEMU-PCI_SERIAL.DeviceDesc% = ComPort, PCI\VEN_1b36DEV_0002CC_0700
+
+; COM sections
+;--
+[ComPort.AddReg]
+HKR,,PortSubClass,1,01
+
+[ComPort.NT]
+AddReg=ComPort.AddReg, ComPort.NT.AddReg
+LogConfig=caa
+SyssetupPnPFlags = 1
+
+[ComPort.NT.HW]
+AddReg=ComPort.NT.HW.AddReg
+
+[ComPort.NT.AddReg]
+HKR,,EnumPropPages32,,MsPorts.dll,SerialPortPropPageProvider
+
+[ComPort.NT.HW.AddReg]
+HKR,,UpperFilters,0x0001,serenum
+
+;-- Service installation
+; Port Driver (function driver for this device)
+[ComPort.NT.Services]
+AddService = Serial, 0x0002, Serial_Service_Inst, Serial_EventLog_Inst
+AddService = Serenum,,Serenum_Service_Inst
+
+; -- Serial Port Driver install sections
+[Serial_Service_Inst]
+DisplayName= %Serial.SVCDESC%
+ServiceType= 1   ; SERVICE_KERNEL_DRIVER
+StartType  = 1   ; SERVICE_SYSTEM_START (this driver may do 
detection)
+ErrorControl   = 0   ; SERVICE_ERROR_IGNORE
+ServiceBinary  = %12%\serial.sys
+LoadOrderGroup = Extended base
+
+; -- Serenum Driver install section
+[Serenum_Service_Inst]
+DisplayName= %Serenum.SVCDESC%
+ServiceType= 1   ; SERVICE_KERNEL_DRIVER
+StartType  = 3   ; SERVICE_DEMAND_START
+ErrorControl   = 1   ; SERVICE_ERROR_NORMAL
+ServiceBinary  = %12%\serenum.sys
+LoadOrderGroup = PNP Filter
+
+[Serial_EventLog_Inst]
+AddReg = Serial_EventLog_AddReg
+
+[Serial_EventLog_AddReg]
+HKR,,EventMessageFile,0x0002,%%SystemRoot%%\System32\IoLogMsg.dll;%%SystemRoot%%\System32\drivers\serial.sys
+HKR,,TypesSupported,0x00010001,7
+
+; The following sections are COM port resource configs.
+; Section name format means:
+; Char 1 = c (COM port)
+; Char 2 = I/O config: 1 (3f8), 2 (2f8), 3 (3e8), 4 (2e8), a (any)
+; Char 3 = IRQ config: #, a (any)
+
+[caa]   ; Any base, any IRQ
+ConfigPriority=HARDRECONFIG
+IOConfig=8@100-%fff8(3ff::)
+IRQConfig=S:3,4,5,7,9,10,11,12,14,15
+
+[Strings]
+QEMU=QEMU
+QEMU-PCI_SERIAL.DeviceDesc=QEMU Serial PCI Card
+
+Serial.SVCDESC   = Serial port driver
+Serenum.SVCDESC = Serenum Filter Driver
-- 
1.7.1




[Qemu-devel] [PATCH 1/7] serial: split serial.c

2012-10-17 Thread Gerd Hoffmann
Split serial.c into serial.c, serial.h and serial-isa.c.  While being at
creating a serial.h header file move the serial prototypes from pc.h to
the new serial.h.  The latter leads to s/pc.h/serial.h/ in tons of
boards which just want the serial bits from pc.h

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 hw/Makefile.objs |2 +-
 hw/alpha_dp264.c |1 +
 hw/kzm.c |2 +-
 hw/mips_fulong2e.c   |1 +
 hw/mips_jazz.c   |1 +
 hw/mips_malta.c  |1 +
 hw/mips_mipssim.c|2 +-
 hw/mips_r4k.c|1 +
 hw/musicpal.c|2 +-
 hw/omap_uart.c   |3 +-
 hw/openrisc_sim.c|3 +-
 hw/pc.c  |1 +
 hw/pc.h  |   27 -
 hw/petalogix_ml605_mmu.c |2 +-
 hw/ppc/e500.c|2 +-
 hw/ppc405_uc.c   |2 +-
 hw/ppc440_bamboo.c   |2 +-
 hw/ppc_prep.c|1 +
 hw/pxa2xx.c  |2 +-
 hw/serial-isa.c  |  130 +
 hw/serial.c  |  143 ++
 hw/serial.h  |   98 +++
 hw/sm501.c   |2 +-
 hw/sun4u.c   |1 +
 hw/virtex_ml507.c|2 +-
 hw/xtensa_lx60.c |3 +-
 26 files changed, 257 insertions(+), 180 deletions(-)
 create mode 100644 hw/serial-isa.c
 create mode 100644 hw/serial.h

diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 854faa9..16e7a1e 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -20,7 +20,7 @@ common-obj-$(CONFIG_M48T59) += m48t59.o
 common-obj-$(CONFIG_ESCC) += escc.o
 common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
 
-common-obj-$(CONFIG_SERIAL) += serial.o
+common-obj-$(CONFIG_SERIAL) += serial.o serial-isa.o
 common-obj-$(CONFIG_PARALLEL) += parallel.o
 common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
 common-obj-$(CONFIG_PCSPK) += pcspk.o
diff --git a/hw/alpha_dp264.c b/hw/alpha_dp264.c
index 5ea04c7..8ce04e5 100644
--- a/hw/alpha_dp264.c
+++ b/hw/alpha_dp264.c
@@ -15,6 +15,7 @@
 #include mc146818rtc.h
 #include ide.h
 #include i8254.h
+#include serial.h
 
 #define MAX_IDE_BUS 2
 
diff --git a/hw/kzm.c b/hw/kzm.c
index 68cd1b4..1f3082b 100644
--- a/hw/kzm.c
+++ b/hw/kzm.c
@@ -21,7 +21,7 @@
 #include net.h
 #include sysemu.h
 #include boards.h
-#include pc.h /* for the FPGA UART that emulates a 16550 */
+#include serial.h
 #include imx.h
 
 /* Memory map for Kzm Emulation Baseboard:
diff --git a/hw/mips_fulong2e.c b/hw/mips_fulong2e.c
index d4a8672..a3cb3ab 100644
--- a/hw/mips_fulong2e.c
+++ b/hw/mips_fulong2e.c
@@ -20,6 +20,7 @@
 
 #include hw.h
 #include pc.h
+#include serial.h
 #include fdc.h
 #include net.h
 #include boards.h
diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c
index db927f1..d35cd54 100644
--- a/hw/mips_jazz.c
+++ b/hw/mips_jazz.c
@@ -26,6 +26,7 @@
 #include mips.h
 #include mips_cpudevs.h
 #include pc.h
+#include serial.h
 #include isa.h
 #include fdc.h
 #include sysemu.h
diff --git a/hw/mips_malta.c b/hw/mips_malta.c
index 632b466..8f73b1b 100644
--- a/hw/mips_malta.c
+++ b/hw/mips_malta.c
@@ -24,6 +24,7 @@
 
 #include hw.h
 #include pc.h
+#include serial.h
 #include fdc.h
 #include net.h
 #include boards.h
diff --git a/hw/mips_mipssim.c b/hw/mips_mipssim.c
index 830f635..0ee6756 100644
--- a/hw/mips_mipssim.c
+++ b/hw/mips_mipssim.c
@@ -27,7 +27,7 @@
 #include hw.h
 #include mips.h
 #include mips_cpudevs.h
-#include pc.h
+#include serial.h
 #include isa.h
 #include net.h
 #include sysemu.h
diff --git a/hw/mips_r4k.c b/hw/mips_r4k.c
index 967a76e..b3be80b 100644
--- a/hw/mips_r4k.c
+++ b/hw/mips_r4k.c
@@ -11,6 +11,7 @@
 #include mips.h
 #include mips_cpudevs.h
 #include pc.h
+#include serial.h
 #include isa.h
 #include net.h
 #include sysemu.h
diff --git a/hw/musicpal.c b/hw/musicpal.c
index f305e21..346fe41 100644
--- a/hw/musicpal.c
+++ b/hw/musicpal.c
@@ -15,7 +15,7 @@
 #include net.h
 #include sysemu.h
 #include boards.h
-#include pc.h
+#include serial.h
 #include qemu-timer.h
 #include ptimer.h
 #include block.h
diff --git a/hw/omap_uart.c b/hw/omap_uart.c
index 167d5c4..1c16a54 100644
--- a/hw/omap_uart.c
+++ b/hw/omap_uart.c
@@ -20,8 +20,7 @@
 #include qemu-char.h
 #include hw.h
 #include omap.h
-/* We use pc-style serial ports.  */
-#include pc.h
+#include serial.h
 #include exec-memory.h
 
 /* UARTs */
diff --git a/hw/openrisc_sim.c b/hw/openrisc_sim.c
index 55e97f0..e484613 100644
--- a/hw/openrisc_sim.c
+++ b/hw/openrisc_sim.c
@@ -21,7 +21,8 @@
 #include hw.h
 #include boards.h
 #include elf.h
-#include pc.h
+#include serial.h
+#include net.h
 #include loader.h
 #include exec-memory.h
 #include sysemu.h
diff --git a/hw/pc.c b/hw/pc.c
index 6c0722d..805e8ca 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -23,6 +23,7 @@
  */
 #include hw.h
 #include pc.h
+#include serial.h
 #include apic.h
 #include fdc.h
 #include ide.h
diff --git a/hw/pc.h b/hw/pc.h
index 

[Qemu-devel] [PATCH 7/7] usb-serial: only expose device in guest when the chardev is open

2012-10-17 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 hw/usb/dev-serial.c |   19 +--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 43214cd..a466f99 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -427,6 +427,10 @@ static void usb_serial_handle_destroy(USBDevice *dev)
 static int usb_serial_can_read(void *opaque)
 {
 USBSerialState *s = opaque;
+
+if (!s-dev.attached) {
+return 0;
+}
 return RECV_BUF - s-recv_used;
 }
 
@@ -469,8 +473,14 @@ static void usb_serial_event(void *opaque, int event)
 case CHR_EVENT_FOCUS:
 break;
 case CHR_EVENT_OPENED:
-usb_serial_reset(s);
-/* TODO: Reset USB port */
+if (!s-dev.attached) {
+usb_device_attach(s-dev);
+}
+break;
+case CHR_EVENT_CLOSED:
+if (s-dev.attached) {
+usb_device_detach(s-dev);
+}
 break;
 }
 }
@@ -481,6 +491,7 @@ static int usb_serial_initfn(USBDevice *dev)
 
 usb_desc_create_serial(dev);
 usb_desc_init(dev);
+dev-auto_attach = 0;
 
 if (!s-cs) {
 error_report(Property chardev is required);
@@ -490,6 +501,10 @@ static int usb_serial_initfn(USBDevice *dev)
 qemu_chr_add_handlers(s-cs, usb_serial_can_read, usb_serial_read,
   usb_serial_event, s);
 usb_serial_handle_reset(dev);
+
+if (s-cs-opened  !dev-attached) {
+usb_device_attach(dev);
+}
 return 0;
 }
 
-- 
1.7.1




[Qemu-devel] [PATCH 5/7] serial: add pci-serial documentation

2012-10-17 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 docs/specs/pci-serial.txt |   34 ++
 hw/serial-pci.c   |2 ++
 2 files changed, 36 insertions(+), 0 deletions(-)
 create mode 100644 docs/specs/pci-serial.txt

diff --git a/docs/specs/pci-serial.txt b/docs/specs/pci-serial.txt
new file mode 100644
index 000..66c761f
--- /dev/null
+++ b/docs/specs/pci-serial.txt
@@ -0,0 +1,34 @@
+
+QEMU pci serial devices
+===
+
+There is one single-port variant and two muliport-variants.  Linux
+guests out-of-the box with all cards.  There is a Windows inf file
+(docs/qemupciserial.inf) to setup the single-port card in Windows
+guests.
+
+
+single-port card
+
+
+Name:   pci-serial
+PCI ID: 1b36:0002
+
+PCI Region 0:
+   IO bar, 8 bytes long, with the 16550 uart mapped to it.
+   Interrupt is wired to pin A.
+
+
+multiport cards
+---
+
+Name:   pci-serial-2x
+PCI ID: 1b36:0003
+
+Name:   pci-serial-4x
+PCI ID: 1b36:0004
+
+PCI Region 0:
+   IO bar, with two/four 16550 uart mapped after each other.
+   The first is at offset 0, second at offset 8, ...
+   Interrupt is wired to pin A.
diff --git a/hw/serial-pci.c b/hw/serial-pci.c
index badd297..95dc5c8 100644
--- a/hw/serial-pci.c
+++ b/hw/serial-pci.c
@@ -23,6 +23,8 @@
  * THE SOFTWARE.
  */
 
+/* see docs/specs/pci-serial.txt */
+
 #include serial.h
 #include pci.h
 
-- 
1.7.1




Re: [Qemu-devel] [Patch]KVM: enabling per domain PLE

2012-10-17 Thread Hu, Xuekun
 
 The problem with this is that it requires an administrator to understand the
 workload, not only of the guest, but also of other guests on the machine.
 With low overcommit, a high PLE window reduces unneeded exits, but with
 high overcommit we need those exits to reduce spinning.
 
 In addition, most kvm hosts don't have an administrator.  They are controlled
 by a management system, which means we'll need some algorithm in
 userspace to control the PLE window.  Taking the two together, we need a
 dynamic (for changing workloads) algorithm.
 
 There are threads discussing this dynamic algorithm, we are making slow
 progress because it's such a difficult problem, but I think this is much more
 useful than anything requiring user intervention.

Avi, agreed that dynamic adaptive ple should be the best solution. However
currently it is a difficult problem like you said. Our solution just gives user
a choice who know how to set the two PLE values. So the solution is a compromise
solution, which should be better than nothing, for now? :-)

Your comments? 




Re: [Qemu-devel] [RFC PATCH 0/6] chardev: convert to QOM

2012-10-17 Thread Gerd Hoffmann
  Hi,

 I think the main decision point here is whether we introduce a separate
 chardev_add/chardev_del command or just use the qom-create command that has
 been posted previously.

Do you have a git tree with this series + qom-create to look at and play
with?

thanks,
  Gerd



[Qemu-devel] [PATCH 2/7] serial: add pci variant

2012-10-17 Thread Gerd Hoffmann
So we get a hot-pluggable 16550 uart.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 default-configs/pci.mak |2 +
 hw/Makefile.objs|1 +
 hw/pci_ids.h|1 +
 hw/serial-pci.c |  101 +++
 hw/serial.c |6 +++
 hw/serial.h |1 +
 6 files changed, 112 insertions(+), 0 deletions(-)
 create mode 100644 hw/serial-pci.c

diff --git a/default-configs/pci.mak b/default-configs/pci.mak
index 69e18f1..ae9d1eb 100644
--- a/default-configs/pci.mak
+++ b/default-configs/pci.mak
@@ -19,3 +19,5 @@ CONFIG_IDE_PCI=y
 CONFIG_AHCI=y
 CONFIG_ESP=y
 CONFIG_ESP_PCI=y
+CONFIG_SERIAL=y
+CONFIG_SERIAL_PCI=y
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 16e7a1e..af4ab0c 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -21,6 +21,7 @@ common-obj-$(CONFIG_ESCC) += escc.o
 common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
 
 common-obj-$(CONFIG_SERIAL) += serial.o serial-isa.o
+common-obj-$(CONFIG_SERIAL_PCI) += serial-pci.o
 common-obj-$(CONFIG_PARALLEL) += parallel.o
 common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o
 common-obj-$(CONFIG_PCSPK) += pcspk.o
diff --git a/hw/pci_ids.h b/hw/pci_ids.h
index 301bf1c..c017a79 100644
--- a/hw/pci_ids.h
+++ b/hw/pci_ids.h
@@ -37,6 +37,7 @@
 #define PCI_CLASS_BRIDGE_PCI 0x0604
 #define PCI_CLASS_BRIDGE_OTHER   0x0680
 
+#define PCI_CLASS_COMMUNICATION_SERIAL   0x0700
 #define PCI_CLASS_COMMUNICATION_OTHER0x0780
 
 #define PCI_CLASS_PROCESSOR_CO   0x0b40
diff --git a/hw/serial-pci.c b/hw/serial-pci.c
new file mode 100644
index 000..6fcf117
--- /dev/null
+++ b/hw/serial-pci.c
@@ -0,0 +1,101 @@
+/*
+ * QEMU 16550A UART emulation
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2008 Citrix Systems, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the Software), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include serial.h
+#include pci.h
+
+typedef struct PCISerialState {
+PCIDevice dev;
+SerialState state;
+} PCISerialState;
+
+static int serial_pci_init(PCIDevice *dev)
+{
+PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
+SerialState *s = pci-state;
+
+s-baudbase = 115200;
+serial_init_core(s);
+
+pci-dev.config[PCI_INTERRUPT_PIN] = 0x01;
+s-irq = pci-dev.irq[0];
+
+memory_region_init_io(s-io, serial_io_ops, s, serial, 8);
+pci_register_bar(pci-dev, 0, PCI_BASE_ADDRESS_SPACE_IO, s-io);
+return 0;
+}
+
+static void serial_pci_exit(PCIDevice *dev)
+{
+PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
+SerialState *s = pci-state;
+
+serial_exit_core(s);
+memory_region_destroy(s-io);
+}
+
+static const VMStateDescription vmstate_pci_serial = {
+.name = pci-serial,
+.version_id = 1,
+.minimum_version_id = 1,
+.fields  = (VMStateField[]) {
+VMSTATE_PCI_DEVICE(dev, PCISerialState),
+VMSTATE_STRUCT(state, PCISerialState, 0, vmstate_serial, SerialState),
+VMSTATE_END_OF_LIST()
+}
+};
+
+static Property serial_pci_properties[] = {
+DEFINE_PROP_CHR(chardev,  PCISerialState, state.chr),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void serial_pci_class_initfn(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
+pc-init = serial_pci_init;
+pc-exit = serial_pci_exit;
+pc-vendor_id = 0x1b36; /* Red Hat */
+pc-device_id = 0x0002;
+pc-revision = 1;
+pc-class_id = PCI_CLASS_COMMUNICATION_SERIAL;
+dc-vmsd = vmstate_pci_serial;
+dc-props = serial_pci_properties;
+}
+
+static TypeInfo serial_pci_info = {
+.name  = pci-serial,
+.parent= TYPE_PCI_DEVICE,
+.instance_size = sizeof(PCISerialState),
+.class_init= serial_pci_class_initfn,
+};
+
+static void serial_pci_register_types(void)
+{
+type_register_static(serial_pci_info);
+}
+
+type_init(serial_pci_register_types)
diff --git 

[Qemu-devel] udev 42-qemu-usb.rules considered harmful?

2012-10-17 Thread Michael Tokarev
Hello.

I noticed that some versions of linux guests shows quite
bad behavour of mouse cursor: it freezes after some idle
time, and when you move mouse, it jumps after some delay
to rather distant position.  So, mouse becomes freezy/jumpy,
so to say.

Quite some painful debugging pointed to the following
udev rule:

 /lib/udev/rules.d/42-qemu-usb.rules 
#
# Enable autosuspend for qemu emulated usb hid devices.
#
# Note that there are buggy qemu versions which advertise remote
# wakeup support but don't actually implement it correctly.  This
# is the reason why we need a match for the serial number here.
# The serial number 42 is used to tag the implementations where
# remote wakeup is working.
#
ACTION==add, SUBSYSTEM==usb, ATTR{product}==QEMU USB Tablet, 
ATTR{serial}==42, TEST==power/control, ATTR{power/control}=auto
 end 

(And a few other similar rules, for other QEMU USB devices).

Removing this rule restores correct mouse behavour.

As far as I can see, this rule is to reduce polling interrupt
frequency, but apparently it does not help: any usb device
connected to guest, and the host CPU usage rises anyway, with
or without this rule.

Now the question: should we remove this rule?  Why it is needed
to start with?

(qemu version 1.1, but the same bad behavour happens with other
versions too).

Thanks,

/mjt



Re: [Qemu-devel] udev 42-qemu-usb.rules considered harmful?

2012-10-17 Thread Gerd Hoffmann
On 10/17/12 10:41, Michael Tokarev wrote:
 Hello.
 
 I noticed that some versions of linux guests shows quite

What is some?

 As far as I can see, this rule is to reduce polling interrupt
 frequency, but apparently it does not help: any usb device
 connected to guest, and the host CPU usage rises anyway, with
 or without this rule.

No, it is for suspending the usb bus, so qemu stops doing 1000 wakeups
per second when the usb tablet is idle.  Works only in case all devices
on the usb bus support it, but for the typical usb use case (-usbdevice
tablet for the abs ptr) this is the case.

 Now the question: should we remove this rule?

No.  We should fix whatever is broken.

  Why it is needed
 to start with?

See above.

cheers,
  Gerd




Re: [Qemu-devel] udev 42-qemu-usb.rules considered harmful?

2012-10-17 Thread Michael Tokarev
On 17.10.2012 12:56, Gerd Hoffmann wrote:
 On 10/17/12 10:41, Michael Tokarev wrote:
 Hello.

 I noticed that some versions of linux guests shows quite
 
 What is some?

I guess it is almost all modern distros.  Debian wheezy,
Ubuntu 12.4, current Fedora (17 I think) to name some.

 As far as I can see, this rule is to reduce polling interrupt
 frequency, but apparently it does not help: any usb device
 connected to guest, and the host CPU usage rises anyway, with
 or without this rule.
 
 No, it is for suspending the usb bus, so qemu stops doing 1000 wakeups
 per second when the usb tablet is idle.  Works only in case all devices

This is very close to what I guessed, so I don't see why No ;)

 on the usb bus support it, but for the typical usb use case (-usbdevice
 tablet for the abs ptr) this is the case.

My command line:

  qemu-kvm -drive file=linux.img,if=virtio -usbdevice tablet

(plus a few other unrelated options).  That to say, there's nothing
more than just one usb tablet, it is exactly the case mentioned
above.

And indeed, the CPU utilization drops from - on my host - 12% to about
1.2% after a few secs of mouse being idle in the guest.  So I was
wrong here, and it actually works - it does what it was supposed to
do, it reduces CPU usage.

But the mouse is jumpy anyway.  I guess one can get used to this
behavour, given the tradeoff and that the behavour is not VERY annoying --
it is annoying, but just for a bit, it feels like old mechanical
mouse with bad sensor or dirty ball when you can move the mouse but
cursor sometimes stays in place and sometimes moves, depending on
the surface and direction of the move... ;)

 No.  We should fix whatever is broken.

Is it _ever_ possible to fix this?

What happens when the (device, bus) is suspended?  Does host still
sends interrupts periodically (but with much lower frequency)?  Or
is it the bus/hub/device which should signal attention at some point?

Sorry I'm not even a novice in USB internals ;)

/mjt



Re: [Qemu-devel] [RFC PATCH v3 06/19] Implement -dimm command line option

2012-10-17 Thread Vasilis Liaskovitis
On Sat, Oct 13, 2012 at 08:57:19AM +, Blue Swirl wrote:
 On Tue, Oct 9, 2012 at 5:04 PM, Vasilis Liaskovitis
 vasilis.liaskovi...@profitbricks.com wrote:
 
snip
  Maybe even the dimmbus device shouldn't exist by itself after all, or
  it should be pretty much invisible to users. On real HW, the memory
  controller or south bridge handles the memory. For i440fx, it's part
  of the same chipset. So I think we should just add qdev properties to
  i440fx to specify the sizes, nodes etc. Then i440fx should create the
  dimmbus device unconditionally using the properties. The default
  properties should create a sane configuration, otherwise -global
  i440fx.dimm_size=512M etc. could be used. Then the bus would be
  populated as before or with device_add.
 
  hmm the problem with using only i440fx properties, is that size/nodes look
  dimm specific to me, not chipset-memcontroller specific. Unless we only 
  allow
  uniform size dimms. Is it possible to have a dynamic list of sizes/nodes 
  pairs as
  properties of a qdev device?
 
 I don't think so, but probably there's a limit of DIMMs that real
 controllers have, something like 8 max.

In the case of i440fx specifically, do you mean that we should model the DRB
(Dram row boundary registers in section 3.2.19 of the i440fx spec) ?

The i440fx DRB registers only supports up to 8 DRAM rows (let's say 1 row
maps 1-1 to a DimmDevice for this discussion) and only supports up to 2GB of
memory afaict (bit 31 and above is ignored).

I 'd rather not model this part of the i440fx - having only 8 DIMMs seems too
restrictive. The rest of the patchset supports up to 255 DIMMs so it would be a
waste imho to model an old pc memory controller that only supports 8 DIMMs.

There was also an old discussion about i440fx modeling here:
https://lists.nongnu.org/archive/html/qemu-devel/2011-07/msg02705.html
the general direction was that i440fx is too old and we don't want to precisely
emulate the DRB registers, since they lack flexibility.

Possible solutions:

1) is there a newer and more flexible chipset that we could model?

2) model and document a generic (non-existent) i440fx that would support more
and larger DIMMs. E.g. support 255 DIMMs. If we want to use a description
similar to the i440fx DRB registers, the registers would take up a lot of space.
In i440fx there is one 8-bit DRB register per DIMM, and DRB[i] describes how
many 8MB chunks are contained in DIMMs 0...i. So, the register values are
cumulative (and total described memory cannot exceed 256x8MB = 2GB)

We could for example model: 
- an 8-bit non-cumulative register for each DIMM, denoting how many
128MB chunks it contains. This allowes 32GB for each DIMM, and with 255 DIMMs we
describe a bit less than 8TB. These registers require 255 bytes.
- a 16-bit cumulative register for each DIMM again for 128MB chunks. This allows
us to describe 8TB of memory (but the registers take up double the space, 
because
they describe cumulative memory amounts)

3) let everything be handled/abstracted by dimmbus - the chipset DRB modelling
is not done (at least for i440fx, other machines could). This is the least 
precise
in terms of emulation. On the other hand, if we are not really trying to emulate
the real (too restrictive) hardware, does it matter?

thanks,

- Vasilis



Re: [Qemu-devel] udev 42-qemu-usb.rules considered harmful?

2012-10-17 Thread Michael Tokarev
On 17.10.2012 13:15, Michael Tokarev wrote:
 But the mouse is jumpy anyway.  I guess one can get used to this
 behavour, given the tradeoff and that the behavour is not VERY annoying --
 it is annoying, but just for a bit, it feels like old mechanical
 mouse with bad sensor or dirty ball when you can move the mouse but
 cursor sometimes stays in place and sometimes moves, depending on
 the surface and direction of the move... ;)

It is more like the case when your desktop system is swapping
heavily, so when you move mouse, it needs to swap-in parts of
X or clients which are supposed to react to mouse movement events,
so the movement is just delayed, not ignored altogether like
with bad/dirty ball.  And when you leave it alone, these parts
gets swapped out again, to be swapped in when you move it the
next time.

/mjt



Re: [Qemu-devel] [PATCH v3 9/9] chardev: add hotplug support.

2012-10-17 Thread Gerd Hoffmann
  Hi,

 +- id: the chardev's ID, must be unique (json-string)
 +- backend: the chardev backend: file, socket, ... (json-string)
 +- path: file / device / unix socket path (json-string, optional)
 +- name: spice channel name (json-string, optional)
 +- host: host name (json-string, optional)
 +- port: port number (json-string, optional)
 +- server: create socket in server mode (json-bool, optional)
 +- wait: wait for connect (json-bool, optional)
 +- ipv4: force ipv4-only (json-bool, optional)
 +- ipv6: force ipv6-only (json-bool, optional)
 +- telnet: telnet negotiation (json-bool, optional)
 +
 
 As mentioned in another thread, should it support the mux argument
 for multiplexing mode?

I don't think so.  Basically the only use case for the mux chardev is
-nographic with serial line + monitor being multiplexed on stdio.

cheers,
  Gerd




Re: [Qemu-devel] [PATCH v3 0/4] block-commit fixes

2012-10-17 Thread Kevin Wolf
Am 16.10.2012 22:33, schrieb Eric Blake:
 On 10/16/2012 01:49 PM, Jeff Cody wrote:
 v2 - v3 differences:

 * Simplified code in patch 2, minor commit rewording
 * Made conditional check more valgrind-friendly

 v1 - v2 differences:

 * Updated changes to be protocol friendly
 * Updated block-commit tests to account for different error message
   wording
 * Updated block-commit tests to add relative backing filename tests
 * In bdrv_find_backing_image(), use g_malloc() instead of char arrays


 Jeff Cody (4):
   block: make bdrv_find_backing_image compare canonical filenames
   block: in commit, determine base image from the top image
   qemu-iotests: qemu-io tests update for block-commit (040)
   qemu-iotests: add relative backing file tests for block-commit (040)
 
 Should patch 3/4 be squashed into 2/4 to avoid 'git bisect' failures
 when running the testsuite?  Other than that, series:

Yes, I agree. Applied all and squashed 3/4 into 2/4.

Kevin



Re: [Qemu-devel] [RFC PATCH v3 06/19] Implement -dimm command line option

2012-10-17 Thread Avi Kivity
On 10/17/2012 11:19 AM, Vasilis Liaskovitis wrote:
 
 I don't think so, but probably there's a limit of DIMMs that real
 controllers have, something like 8 max.
 
 In the case of i440fx specifically, do you mean that we should model the DRB
 (Dram row boundary registers in section 3.2.19 of the i440fx spec) ?
 
 The i440fx DRB registers only supports up to 8 DRAM rows (let's say 1 row
 maps 1-1 to a DimmDevice for this discussion) and only supports up to 2GB of
 memory afaict (bit 31 and above is ignored).
 
 I 'd rather not model this part of the i440fx - having only 8 DIMMs seems too
 restrictive. The rest of the patchset supports up to 255 DIMMs so it would be 
 a
 waste imho to model an old pc memory controller that only supports 8 DIMMs.
 
 There was also an old discussion about i440fx modeling here:
 https://lists.nongnu.org/archive/html/qemu-devel/2011-07/msg02705.html
 the general direction was that i440fx is too old and we don't want to 
 precisely
 emulate the DRB registers, since they lack flexibility.
 
 Possible solutions:
 
 1) is there a newer and more flexible chipset that we could model?

Look for q35 on this list.

 
 2) model and document 
 ^--- the critical bit

 a generic (non-existent) i440fx that would support more
 and larger DIMMs. E.g. support 255 DIMMs. If we want to use a description
 similar to the i440fx DRB registers, the registers would take up a lot of 
 space.
 In i440fx there is one 8-bit DRB register per DIMM, and DRB[i] describes how
 many 8MB chunks are contained in DIMMs 0...i. So, the register values are
 cumulative (and total described memory cannot exceed 256x8MB = 2GB)

Our i440fx has already been extended by support for pci and cpu hotplug,
and I see no reason not to extend it for memory.  We can allocate extra
mmio space for registers if needed.  Usually I'm against this sort of
thing, but in this case we don't have much choice.

 
 We could for example model: 
 - an 8-bit non-cumulative register for each DIMM, denoting how many
 128MB chunks it contains. This allowes 32GB for each DIMM, and with 255 DIMMs 
 we
 describe a bit less than 8TB. These registers require 255 bytes.
 - a 16-bit cumulative register for each DIMM again for 128MB chunks. This 
 allows
 us to describe 8TB of memory (but the registers take up double the space, 
 because
 they describe cumulative memory amounts)

There is no reason to save space.  Why not have two 64-bit registers per
DIMM, one describing the size and the other the base address, both in
bytes?  Use a few low order bits for control.

 
 3) let everything be handled/abstracted by dimmbus - the chipset DRB modelling
 is not done (at least for i440fx, other machines could). This is the least 
 precise
 in terms of emulation. On the other hand, if we are not really trying to 
 emulate
 the real (too restrictive) hardware, does it matter?

We could emulate base memory using the chipset, and extra memory using
the scheme above.  This allows guests that are tied to the chipset to
work, and guests that have more awareness (seabios) to use the extra
features.

-- 
error compiling committee.c: too many arguments to function



[Qemu-devel] [PATCH 0/3] chardev hotplug patch series

2012-10-17 Thread Gerd Hoffmann
  Hi,

Here comes the updated chardev hotplug patch series.  Addressed most
review comments.  It's chardev-add and chardev-remove now.  Parameters
have been added to the schema.  Little nits and spell fixes here and
there.

Making 'backend' an enum is tricky given that the actual implementation
just turns the qdict into a QemuOpts, then goes piggyback on
qemu_chr_new_from_opts().

Not rebased (yet) on top of the qom queue as I expect more discussions.

please review,
  Gerd

Gerd Hoffmann (3):
  chardev: add error reporting for qemu_chr_new_from_opts
  chardev: fix QemuOpts lifecycle
  chardev: add hotplug support.

 hmp-commands.hx  |   32 +
 hmp.c|   23 +++
 hmp.h|2 +
 qapi-schema.json |   47 ++
 qemu-char.c  |   83 -
 qemu-char.h  |5 ++-
 qmp-commands.hx  |   61 +++
 vl.c |8 -
 8 files changed, 244 insertions(+), 17 deletions(-)




[Qemu-devel] [PATCH 1/3] chardev: add error reporting for qemu_chr_new_from_opts

2012-10-17 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 qemu-char.c |   24 +++-
 qemu-char.h |3 ++-
 vl.c|8 ++--
 3 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index b082bae..e2e1da8 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2755,19 +2755,20 @@ static const struct {
 };
 
 CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
-void (*init)(struct CharDriverState *s))
+void (*init)(struct CharDriverState *s),
+Error **errp)
 {
 CharDriverState *chr;
 int i;
 
 if (qemu_opts_id(opts) == NULL) {
-fprintf(stderr, chardev: no id specified\n);
+error_setg(errp, chardev: no id specified\n);
 return NULL;
 }
 
 if (qemu_opt_get(opts, backend) == NULL) {
-fprintf(stderr, chardev: \%s\ missing backend\n,
-qemu_opts_id(opts));
+error_setg(errp, chardev: \%s\ missing backend\n,
+   qemu_opts_id(opts));
 return NULL;
 }
 for (i = 0; i  ARRAY_SIZE(backend_table); i++) {
@@ -2775,15 +2776,15 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
 break;
 }
 if (i == ARRAY_SIZE(backend_table)) {
-fprintf(stderr, chardev: backend \%s\ not found\n,
-qemu_opt_get(opts, backend));
+error_setg(errp, chardev: backend \%s\ not found\n,
+   qemu_opt_get(opts, backend));
 return NULL;
 }
 
 chr = backend_table[i].open(opts);
 if (!chr) {
-fprintf(stderr, chardev: opening backend \%s\ failed\n,
-qemu_opt_get(opts, backend));
+error_setg(errp, chardev: opening backend \%s\ failed\n,
+   qemu_opt_get(opts, backend));
 return NULL;
 }
 
@@ -2813,6 +2814,7 @@ CharDriverState *qemu_chr_new(const char *label, const 
char *filename, void (*in
 const char *p;
 CharDriverState *chr;
 QemuOpts *opts;
+Error *err = NULL;
 
 if (strstart(filename, chardev:, p)) {
 return qemu_chr_find(p);
@@ -2822,7 +2824,11 @@ CharDriverState *qemu_chr_new(const char *label, const 
char *filename, void (*in
 if (!opts)
 return NULL;
 
-chr = qemu_chr_new_from_opts(opts, init);
+chr = qemu_chr_new_from_opts(opts, init, err);
+if (error_is_set(err)) {
+fprintf(stderr, %s\n, error_get_pretty(err));
+error_free(err);
+}
 if (chr  qemu_opt_get_bool(opts, mux, 0)) {
 monitor_init(chr, MONITOR_USE_READLINE);
 }
diff --git a/qemu-char.h b/qemu-char.h
index 486644b..adae13e 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -88,7 +88,8 @@ struct CharDriverState {
  * Returns: a new character backend
  */
 CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
-void (*init)(struct CharDriverState *s));
+void (*init)(struct CharDriverState *s),
+Error **errp);
 
 /**
  * @qemu_chr_new:
diff --git a/vl.c b/vl.c
index 5b357a3..d2f1c10 100644
--- a/vl.c
+++ b/vl.c
@@ -1940,10 +1940,14 @@ static int device_init_func(QemuOpts *opts, void 
*opaque)
 static int chardev_init_func(QemuOpts *opts, void *opaque)
 {
 CharDriverState *chr;
+Error *local_err = NULL;
 
-chr = qemu_chr_new_from_opts(opts, NULL);
-if (!chr)
+chr = qemu_chr_new_from_opts(opts, NULL, local_err);
+if (error_is_set(local_err)) {
+fprintf(stderr, %s\n, error_get_pretty(local_err));
+error_free(local_err);
 return -1;
+}
 return 0;
 }
 
-- 
1.7.1




[Qemu-devel] [PATCH 2/3] chardev: fix QemuOpts lifecycle

2012-10-17 Thread Gerd Hoffmann
qemu_chr_new_from_opts handles QemuOpts release now, so callers don't
have to worry.  It will either be saved in CharDriverState, then
released in qemu_chr_delete, or in the error case released instantly.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 qemu-char.c |   15 ++-
 qemu-char.h |1 +
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index e2e1da8..be4ec61 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2763,13 +2763,13 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
 
 if (qemu_opts_id(opts) == NULL) {
 error_setg(errp, chardev: no id specified\n);
-return NULL;
+goto err;
 }
 
 if (qemu_opt_get(opts, backend) == NULL) {
 error_setg(errp, chardev: \%s\ missing backend\n,
qemu_opts_id(opts));
-return NULL;
+goto err;
 }
 for (i = 0; i  ARRAY_SIZE(backend_table); i++) {
 if (strcmp(backend_table[i].name, qemu_opt_get(opts, backend)) == 0)
@@ -2778,14 +2778,14 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
 if (i == ARRAY_SIZE(backend_table)) {
 error_setg(errp, chardev: backend \%s\ not found\n,
qemu_opt_get(opts, backend));
-return NULL;
+goto err;
 }
 
 chr = backend_table[i].open(opts);
 if (!chr) {
 error_setg(errp, chardev: opening backend \%s\ failed\n,
qemu_opt_get(opts, backend));
-return NULL;
+goto err;
 }
 
 if (!chr-filename)
@@ -2806,7 +2806,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
 chr-avail_connections = 1;
 }
 chr-label = g_strdup(qemu_opts_id(opts));
+chr-opts = opts;
 return chr;
+
+err:
+qemu_opts_del(opts);
+return NULL;
 }
 
 CharDriverState *qemu_chr_new(const char *label, const char *filename, void 
(*init)(struct CharDriverState *s))
@@ -2832,7 +2837,6 @@ CharDriverState *qemu_chr_new(const char *label, const 
char *filename, void (*in
 if (chr  qemu_opt_get_bool(opts, mux, 0)) {
 monitor_init(chr, MONITOR_USE_READLINE);
 }
-qemu_opts_del(opts);
 return chr;
 }
 
@@ -2864,6 +2868,7 @@ void qemu_chr_delete(CharDriverState *chr)
 chr-chr_close(chr);
 g_free(chr-filename);
 g_free(chr-label);
+qemu_opts_del(chr-opts);
 g_free(chr);
 }
 
diff --git a/qemu-char.h b/qemu-char.h
index adae13e..99bc132 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -74,6 +74,7 @@ struct CharDriverState {
 char *filename;
 int opened;
 int avail_connections;
+QemuOpts *opts;
 QTAILQ_ENTRY(CharDriverState) next;
 };
 
-- 
1.7.1




Re: [Qemu-devel] [Patch]KVM: enabling per domain PLE

2012-10-17 Thread Avi Kivity
On 10/17/2012 10:02 AM, Hu, Xuekun wrote:
 
 The problem with this is that it requires an administrator to understand the
 workload, not only of the guest, but also of other guests on the machine.
 With low overcommit, a high PLE window reduces unneeded exits, but with
 high overcommit we need those exits to reduce spinning.
 
 In addition, most kvm hosts don't have an administrator.  They are controlled
 by a management system, which means we'll need some algorithm in
 userspace to control the PLE window.  Taking the two together, we need a
 dynamic (for changing workloads) algorithm.
 
 There are threads discussing this dynamic algorithm, we are making slow
 progress because it's such a difficult problem, but I think this is much more
 useful than anything requiring user intervention.
 
 Avi, agreed that dynamic adaptive ple should be the best solution. However
 currently it is a difficult problem like you said. Our solution just gives 
 user
 a choice who know how to set the two PLE values. So the solution is a 
 compromise
 solution, which should be better than nothing, for now? :-)

Let's see how the PLE thread works out.  Yes the patches give the user
control, but we need to make sure the user knows how to control it (in
fact your patch doesn't even update the documentation).  Just throwing
out a new ioctl, even if it is documented, doesn't mean that userspace
will begin to use it, or that users will exploit it.

Do you have a specific use case in mind?

-- 
error compiling committee.c: too many arguments to function



[Qemu-devel] [PATCH 3/3] chardev: add hotplug support.

2012-10-17 Thread Gerd Hoffmann
This patch adds chardev_add and chardev_remove monitor commands.

They work similar to the netdev_{add,del} commands.  The hmp version of
chardev_add accepts like the -chardev command line option does.  The qmp
version expects the arguments being passed as named parameters.

chardev_del just takes an id argument and zaps the chardev specified.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 hmp-commands.hx  |   32 
 hmp.c|   23 
 hmp.h|2 +
 qapi-schema.json |   47 +
 qemu-char.c  |   44 ++
 qemu-char.h  |1 +
 qmp-commands.hx  |   61 ++
 7 files changed, 210 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e0b537d..5207383 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1404,6 +1404,38 @@ passed since 1970, i.e. unix epoch.
 ETEXI
 
 {
+.name   = chardev_add,
+.args_type  = args:s,
+.params = args,
+.help   = add chardev,
+.mhandler.cmd = hmp_chardev_add,
+},
+
+STEXI
+@item chardev_add args
+@findex chardev_add
+
+chardev_add accepts the same parameters as the -chardev command line switch.
+
+ETEXI
+
+{
+.name   = chardev_remove,
+.args_type  = id:s,
+.params = id,
+.help   = remove chardev,
+.mhandler.cmd = hmp_chardev_remove,
+},
+
+STEXI
+@item chardev_remove id
+@findex chardev_remove
+
+Removes the chardev @var{id}.
+
+ETEXI
+
+{
 .name   = info,
 .args_type  = item:s?,
 .params = [subcommand],
diff --git a/hmp.c b/hmp.c
index 70bdec2..fc9d7bc 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1209,3 +1209,26 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
 qmp_screendump(filename, err);
 hmp_handle_error(mon, err);
 }
+
+void hmp_chardev_add(Monitor *mon, const QDict *qdict)
+{
+const char *args = qdict_get_str(qdict, args);
+Error *err = NULL;
+QemuOpts *opts;
+
+opts = qemu_opts_parse(qemu_find_opts(chardev), args, 1);
+if (opts == NULL) {
+error_setg(err, Parsing chardev args failed\n);
+} else {
+qemu_chr_new_from_opts(opts, NULL, err);
+}
+hmp_handle_error(mon, err);
+}
+
+void hmp_chardev_remove(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+qmp_chardev_remove(qdict_get_str(qdict, id),
+   err);
+hmp_handle_error(mon, err);
+}
diff --git a/hmp.h b/hmp.h
index 71ea384..cabe944 100644
--- a/hmp.h
+++ b/hmp.h
@@ -75,5 +75,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
 void hmp_send_key(Monitor *mon, const QDict *qdict);
 void hmp_screen_dump(Monitor *mon, const QDict *qdict);
+void hmp_chardev_add(Monitor *mon, const QDict *qdict);
+void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index f9dbdae..fab6bbc 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2796,3 +2796,50 @@
 # Since: 0.14.0
 ##
 { 'command': 'screendump', 'data': {'filename': 'str'} }
+
+##
+# @chardev-add:
+#
+# Add a chardev
+#
+# @id: the chardev's ID, must be unique
+# @backend: the chardev backend: file, socket, ...
+# @path: file / device / unix socket path
+# @name: spice channel name
+# @host: host name
+# @port: port number
+# @server: create socket in server mode
+# @wait: wait for connect
+# @ipv4: force ipv4-only
+# @ipv6: force ipv6-only
+# @telnet: telnet negotiation
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev-add', 'data': {'id'  : 'str',
+ 'backend' : 'str',
+ '*path'   : 'str',
+ '*name'   : 'str',
+ '*host'   : 'str',
+ '*port'   : 'str',
+ '*server' : 'bool',
+ '*wait'   : 'bool',
+ '*ipv4'   : 'bool',
+ '*ipv6'   : 'bool',
+ '*telnet' : 'bool' },
+  'gen': 'no' }
+
+##
+# @chardev-remove:
+#
+# Remove a chardev
+#
+# @id: the chardev's ID, must exist and not be in use
+#
+# Returns: Nothing on success
+#
+# Since: 1.3.0
+##
+{ 'command': 'chardev-remove', 'data': {'id': 'str'} }
diff --git a/qemu-char.c b/qemu-char.c
index be4ec61..cbfa24e 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2911,3 +2911,47 @@ CharDriverState *qemu_char_get_next_serial(void)
 return serial_hds[next_serial++];
 }
 
+int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret)
+{
+Error *err = NULL;
+QemuOptsList *opts_list;
+QemuOpts *opts;
+
+opts_list = 

Re: [Qemu-devel] udev 42-qemu-usb.rules considered harmful?

2012-10-17 Thread Gerd Hoffmann
  Hi,

 But the mouse is jumpy anyway.  I guess one can get used to this
 behavour, given the tradeoff and that the behavour is not VERY annoying --
 it is annoying, but just for a bit, it feels like old mechanical
 mouse with bad sensor or dirty ball when you can move the mouse but
 cursor sometimes stays in place and sometimes moves, depending on
 the surface and direction of the move... ;)

How long are the delays for you?

 What happens when the (device, bus) is suspended?  Does host still
 sends interrupts periodically (but with much lower frequency)?  Or
 is it the bus/hub/device which should signal attention at some point?

UHCI raises an IRQ, guest restarts USB bus in response.  Which takes a
bit of time, so the first mouse event of an idle tablet takes a little
bit longer.  As far I know linux schedules the first poll a bit in the
future, so this could be where the delay comes from.

Do you also see delays with xhci? (-device nec-pci-xhci -device
usb-tablet) ?

cheers,
  Gerd



Re: [Qemu-devel] [PATCH 10/10] tcg: Optimize mulu2

2012-10-17 Thread Avi Kivity
On 10/17/2012 03:09 AM, Richard Henderson wrote:
 On 2012-10-17 09:25, Aurelien Jarno wrote:
  +gen_opc_buf[op_index] = op = INDEX_op_mul_i32;
 Very minor nitpick: you probably don't need to set op there.
 
 
 Perhaps not, but I prefer to keep the variables in sync as we
 drop into common code...

The compiler should recognize the dead variable anyway.  How very meta.


-- 
error compiling committee.c: too many arguments to function



Re: [Qemu-devel] [PATCH 06/22] ehci: Speed up the timer of raising int from the async schedule

2012-10-17 Thread Gerd Hoffmann
On 10/15/12 15:00, Hans de Goede wrote:
 Hi,
 
 On 10/15/2012 01:17 PM, Gerd Hoffmann wrote:
 On 10/15/12 12:38, Hans de Goede wrote:
 Often the guest will queue up new packets in response to a packet, in
 the
 async schedule with its IOC flag set, completing. By speeding up the
 frame-timer, we notice these new packets earlier. This increases the
 speed (MB/s) of a Linux guest reading from a USB mass storage device
 by a
 factor of 1.15 on top of the Improve latency of interrupt delivery
 speed-ups, both with and without input pipelining enabled.

 Why not just set async_stepdown to 0?
 
 We already do that whenever we run a package completion (it get sets when
 we move to the executing stage). What this patch does is request the
 frame timer to run again in 500 usecs instead of after 1 ms, thus making
 us see and process async transfers faster when they are queued up in
 response to just completed packages (which we've told the guest about with
 the int interrupt). This makes the USB-bus / device idle time between
 any 2 transfers of the 3 transfer involving USB storage BOT time shorter,
 thereby speeding things up.

Don't feel like having two mechanisms for wakeup rate control.  Can't we
integrate this with async_stepdown?  Changing the baseline maybe, so
stepdown=0 doesn't mean 1000 Hz but 2000 Hz?

cheers,
  Gerd




Re: [Qemu-devel] [PATCH 13/22] usb: Add an int_req flag to USBPacket

2012-10-17 Thread Gerd Hoffmann
On 10/15/12 12:38, Hans de Goede wrote:
 -usb_packet_setup(xfer-packet, dir, ep, xfer-trbs[0].addr, false);
 +int_req = false;
 +for (i = 0; i  xfer-trb_count; i++) {
 +if (xfer-trbs[i].control  TRB_TR_IOC) {
 +int_req = true;
 +break;
 +}
 +}

Guess we better add a new int_req field to XHCITransfer and fill it when
walking the trb list _anyway_, so we don't have to loop again.

cheers,
  Gerd



Re: [Qemu-devel] [PATCH 06/22] ehci: Speed up the timer of raising int from the async schedule

2012-10-17 Thread Hans de Goede

Hi,

On 10/17/2012 01:01 PM, Gerd Hoffmann wrote:

On 10/15/12 15:00, Hans de Goede wrote:

Hi,

On 10/15/2012 01:17 PM, Gerd Hoffmann wrote:

On 10/15/12 12:38, Hans de Goede wrote:

Often the guest will queue up new packets in response to a packet, in
the
async schedule with its IOC flag set, completing. By speeding up the
frame-timer, we notice these new packets earlier. This increases the
speed (MB/s) of a Linux guest reading from a USB mass storage device
by a
factor of 1.15 on top of the Improve latency of interrupt delivery
speed-ups, both with and without input pipelining enabled.


Why not just set async_stepdown to 0?


We already do that whenever we run a package completion (it get sets when
we move to the executing stage). What this patch does is request the
frame timer to run again in 500 usecs instead of after 1 ms, thus making
us see and process async transfers faster when they are queued up in
response to just completed packages (which we've told the guest about with
the int interrupt). This makes the USB-bus / device idle time between
any 2 transfers of the 3 transfer involving USB storage BOT time shorter,
thereby speeding things up.


Don't feel like having two mechanisms for wakeup rate control.  Can't we
integrate this with async_stepdown?  Changing the baseline maybe, so
stepdown=0 doesn't mean 1000 Hz but 2000 Hz?


That is actually close to what I wanted to do at first (I wanted to use
stepdown=-1 for the faster wakeup case). But there are 2 problems with this:

1) It causes migration issues when migrating to / from an old version
2) We don't want to change the wakeup rate when the interrupt flag gets set
as pending, but when it actually gets committed, and we only want to change
the wakeup rate when the int was requested by an async packet, not when it
was requested by a periodic packet, so we will need the int_req_by_async
flag anyways, as which point this seemed the cleanest way.

Regards,

Hans



Re: [Qemu-devel] [PATCH 13/22] usb: Add an int_req flag to USBPacket

2012-10-17 Thread Hans de Goede

Hi,

On 10/17/2012 01:04 PM, Gerd Hoffmann wrote:

On 10/15/12 12:38, Hans de Goede wrote:

-usb_packet_setup(xfer-packet, dir, ep, xfer-trbs[0].addr, false);
+int_req = false;
+for (i = 0; i  xfer-trb_count; i++) {
+if (xfer-trbs[i].control  TRB_TR_IOC) {
+int_req = true;
+break;
+}
+}


Guess we better add a new int_req field to XHCITransfer and fill it when
walking the trb list _anyway_, so we don't have to loop again.


Ok, I will fix this for the next respin of the patchset.

Regards,

Hans



Re: [Qemu-devel] [PATCH 14/22] usb: Add packet combining functions

2012-10-17 Thread Gerd Hoffmann
  Hi,

 +/*
 + * Process / cancel combined packets, called from
 + * usb_ep_combine_input_packets() / usb_combined_packet_cancel().
 + * Only called for devices which call these functions themselves.
 + */
 +int (*handle_combined_data)(USBDevice *dev, USBPacket *p);
 +void (*cancel_combined_packet)(USBDevice *dev, USBPacket *p);

Do we really need these?  I think it isn't much work for the callers to
do that themself.  Saves them providing a callback.  And makes the code
flow easier to follow by removing a pointless indirection.

For handle_combined_data we probably must make
usb_ep_combine_input_packets return a status code.

cheers,
  Gerd




Re: [Qemu-devel] [PATCH 06/22] ehci: Speed up the timer of raising int from the async schedule

2012-10-17 Thread Gerd Hoffmann
  Hi,

 1) It causes migration issues when migrating to / from an old version

With -1 yes, shifting the scale shoudn't be a that big issue though as
it is just a optimization.

 2) We don't want to change the wakeup rate when the interrupt flag gets set
 as pending, but when it actually gets committed, and we only want to change
 the wakeup rate when the int was requested by an async packet, not when it
 was requested by a periodic packet, so we will need the int_req_by_async
 flag anyways, as which point this seemed the cleanest way.

Missed that little details, ok then.

cheers,
  Gerd



[Qemu-devel] [PATCH 1/2] microblaze: Support setting of TLS ptr

2012-10-17 Thread edgar . iglesias
From: Edgar E. Iglesias edgar.igles...@gmail.com

Signed-off-by: Edgar E. Iglesias edgar.igles...@gmail.com
---
 linux-user/syscall.c|2 ++
 target-microblaze/cpu.h |1 +
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 471d060..c6a6337 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6888,6 +6888,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 ret = get_errno(do_fork(cpu_env, arg1, arg2, arg3, arg5, arg4));
 #elif defined(TARGET_CRIS)
 ret = get_errno(do_fork(cpu_env, arg2, arg1, arg3, arg4, arg5));
+#elif defined(TARGET_MICROBLAZE)
+ret = get_errno(do_fork(cpu_env, arg1, arg2, arg4, arg6, arg5));
 #elif defined(TARGET_S390X)
 ret = get_errno(do_fork(cpu_env, arg2, arg1, arg3, arg5, arg4));
 #else
diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
index 4968c24..88430b5 100644
--- a/target-microblaze/cpu.h
+++ b/target-microblaze/cpu.h
@@ -345,6 +345,7 @@ static inline void cpu_clone_regs(CPUMBState *env, 
target_ulong newsp)
 
 static inline void cpu_set_tls(CPUMBState *env, target_ulong newtls)
 {
+env-regs[21] = newtls;
 }
 
 static inline int cpu_interrupts_enabled(CPUMBState *env)
-- 
1.7.8.6




[Qemu-devel] [PATCH 2/2] microblaze: Update PC before simulating syscall

2012-10-17 Thread edgar . iglesias
From: Edgar E. Iglesias edgar.igles...@gmail.com

Fixes a clone() emulation bug were the new thread starts
at the point of the syscall and thus clones in a loop.

Signed-off-by: Edgar E. Iglesias edgar.igles...@gmail.com
---
 linux-user/main.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index f4bbe69..5827ee6 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2527,6 +2527,7 @@ void cpu_loop(CPUMBState *env)
 case EXCP_BREAK:
 /* Return address is 4 bytes after the call.  */
 env-regs[14] += 4;
+env-sregs[SR_PC] = env-regs[14];
 ret = do_syscall(env, 
  env-regs[12], 
  env-regs[5], 
@@ -2537,7 +2538,6 @@ void cpu_loop(CPUMBState *env)
  env-regs[10],
  0, 0);
 env-regs[3] = ret;
-env-sregs[SR_PC] = env-regs[14];
 break;
 case EXCP_HW_EXCP:
 env-regs[17] = env-sregs[SR_PC] + 4;
-- 
1.7.8.6




[Qemu-devel] [PATCH v3 1/2] qemu-img: Add --backing-chain option to info command

2012-10-17 Thread Stefan Hajnoczi
The qemu-img info --backing-chain option enumerates the backing file
chain.  For example, for base.qcow2 - snap1.qcow2 - snap2.qcow2 the
output becomes:

  $ qemu-img info --backing-chain snap2.qcow2
  image: snap2.qcow2
  file format: qcow2
  virtual size: 100M (104857600 bytes)
  disk size: 196K
  cluster_size: 65536
  backing file: snap1.qcow2
  backing file format: qcow2

  image: snap1.qcow2
  file format: qcow2
  virtual size: 100M (104857600 bytes)
  disk size: 196K
  cluster_size: 65536
  backing file: base.qcow2
  backing file format: qcow2

  image: base.qcow2
  file format: qcow2
  virtual size: 100M (104857600 bytes)
  disk size: 136K
  cluster_size: 65536

Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
 qemu-img.c | 167 +++--
 1 file changed, 153 insertions(+), 14 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index f17f187..7261be9 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1108,6 +1108,23 @@ static void dump_snapshots(BlockDriverState *bs)
 g_free(sn_tab);
 }
 
+static void dump_json_image_info_list(ImageInfoList *list)
+{
+Error *errp = NULL;
+QString *str;
+QmpOutputVisitor *ov = qmp_output_visitor_new();
+QObject *obj;
+visit_type_ImageInfoList(qmp_output_get_visitor(ov),
+ list, NULL, errp);
+obj = qmp_output_get_qobject(ov);
+str = qobject_to_json_pretty(obj);
+assert(str != NULL);
+printf(%s\n, qstring_get_str(str));
+qobject_decref(obj);
+qmp_output_visitor_cleanup(ov);
+QDECREF(str);
+}
+
 static void collect_snapshots(BlockDriverState *bs , ImageInfo *info)
 {
 int i, sn_count;
@@ -1247,9 +1264,129 @@ static void dump_human_image_info(ImageInfo *info)
 printf(backing file format: %s\n, info-backing_filename_format);
 }
 }
+
+if (info-has_snapshots) {
+SnapshotInfoList *elem;
+char buf[256];
+
+printf(Snapshot list:\n);
+printf(%s\n, bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+
+/* Ideally bdrv_snapshot_dump() would operate on SnapshotInfoList but
+ * we convert to the block layer's native QEMUSnapshotInfo for now.
+ */
+for (elem = info-snapshots; elem; elem = elem-next) {
+QEMUSnapshotInfo sn = {
+.vm_state_size = elem-value-vm_state_size,
+.date_sec = elem-value-date_sec,
+.date_nsec = elem-value-date_nsec,
+.vm_clock_nsec = elem-value-vm_clock_sec * 10ULL +
+ elem-value-vm_clock_nsec,
+};
+
+pstrcpy(sn.id_str, sizeof(sn.id_str), elem-value-id);
+pstrcpy(sn.name, sizeof(sn.name), elem-value-name);
+printf(%s\n, bdrv_snapshot_dump(buf, sizeof(buf), sn));
+}
+}
+}
+
+static void dump_human_image_info_list(ImageInfoList *list)
+{
+ImageInfoList *elem;
+bool delim = false;
+
+for (elem = list; elem; elem = elem-next) {
+if (delim) {
+printf(\n);
+}
+delim = true;
+
+dump_human_image_info(elem-value);
+}
 }
 
-enum {OPTION_OUTPUT = 256};
+static gboolean str_equal_func(gconstpointer a, gconstpointer b)
+{
+return strcmp(a, b) == 0;
+}
+
+/**
+ * Open an image file chain and return an ImageInfoList
+ *
+ * @filename: topmost image filename
+ * @fmt: topmost image format (may be NULL to autodetect)
+ * @chain: true  - enumerate entire backing file chain
+ * false - only topmost image file
+ *
+ * Returns a list of ImageInfo objects or NULL if there was an error opening an
+ * image file.  If there was an error a message will have been printed to
+ * stderr.
+ */
+static ImageInfoList *collect_image_info_list(const char *filename,
+  const char *fmt,
+  bool chain)
+{
+ImageInfoList *head = NULL;
+ImageInfoList **last = head;
+GHashTable *filenames;
+
+filenames = g_hash_table_new_full(g_str_hash, str_equal_func, NULL, NULL);
+
+while (filename) {
+BlockDriverState *bs;
+ImageInfo *info;
+ImageInfoList *elem;
+
+if (g_hash_table_lookup_extended(filenames, filename, NULL, NULL)) {
+error_report(Backing file '%s' creates an infinite loop.,
+ filename);
+goto err;
+}
+g_hash_table_insert(filenames, (gpointer)filename, NULL);
+
+bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING,
+   false);
+if (!bs) {
+goto err;
+}
+
+info = g_new0(ImageInfo, 1);
+collect_image_info(bs, info, filename, fmt);
+collect_snapshots(bs, info);
+
+elem = g_new0(ImageInfoList, 1);
+elem-value = info;
+*last = elem;
+last = elem-next;
+
+bdrv_delete(bs);
+
+filename = fmt = NULL;
+   

[Qemu-devel] [PATCH v3 0/2] qemu-img: Add --backing-chain option to info command

2012-10-17 Thread Stefan Hajnoczi
This series adds the --backing-chain option for enumerating the backing file
chain.  Given the topmost image it will print qemu-img info information for
each image file in the chain.

Special care needs to be taken when image files form an infinite loop.  This is
very unusual, most like due to malicious image files.  Nevertheless, qemu-img
must be robust against invalid inputs so we explicit check for this.

v3:
 * Use ImageInfoList to avoid manually outputting JSON and to make the code
   more QAPI-friendly.
 * Solve the output vs error messages issues by printing a detailed error
   message but no image info.  To dig into a broken image chain, rerun without
   --backing-chain.  This is different from what I initially tried but is much
   cleaner because it doesn't produce confusing output.
 * rm -f $TEST_IMG.[123].base [Kevin]
 * Include --output=json in test case [Kevin]
 * Rename test case to free 043 number [Eric]

Stefan Hajnoczi (2):
  qemu-img: Add --backing-chain option to info command
  qemu-iotests: Add 043 backing file chain infinite loop test

 qemu-img.c   | 167 +++
 tests/qemu-iotests/043   |  94 
 tests/qemu-iotests/043.out   |  66 +
 tests/qemu-iotests/common.rc |  10 +++
 tests/qemu-iotests/group |   1 +
 5 files changed, 324 insertions(+), 14 deletions(-)
 create mode 100755 tests/qemu-iotests/043
 create mode 100644 tests/qemu-iotests/043.out

-- 
1.7.11.7




[Qemu-devel] [PATCH v3 2/2] qemu-iotests: Add 043 backing file chain infinite loop test

2012-10-17 Thread Stefan Hajnoczi
This new test verifies that qemu-img info --backing-chain safely aborts
when an image file has a backing file infinite loop.

Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
 tests/qemu-iotests/043   | 94 
 tests/qemu-iotests/043.out   | 66 +++
 tests/qemu-iotests/common.rc | 10 +
 tests/qemu-iotests/group |  1 +
 4 files changed, 171 insertions(+)
 create mode 100755 tests/qemu-iotests/043
 create mode 100644 tests/qemu-iotests/043.out

diff --git a/tests/qemu-iotests/043 b/tests/qemu-iotests/043
new file mode 100755
index 000..bf390ac
--- /dev/null
+++ b/tests/qemu-iotests/043
@@ -0,0 +1,94 @@
+#!/bin/bash
+#
+# Test that qemu-img info --backing-chain detects infinite loops
+#
+# Copyright (C) 2012 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see http://www.gnu.org/licenses/.
+#
+
+# creator
+owner=stefa...@redhat.com
+
+seq=`basename $0`
+echo QA output created by $seq
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+   _cleanup_test_img
+}
+trap _cleanup; exit \$status 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# Any format supporting backing files
+_supported_fmt qcow qcow2 vmdk qed
+_supported_proto generic
+_supported_os Linux
+
+
+size=128M
+_make_test_img $size
+$QEMU_IMG rebase -u -b $TEST_IMG $TEST_IMG
+
+echo
+echo == backing file references self ==
+_img_info --backing-chain
+
+_make_test_img $size
+mv $TEST_IMG $TEST_IMG.base
+_make_test_img -b $TEST_IMG.base $size
+$QEMU_IMG rebase -u -b $TEST_IMG $TEST_IMG.base
+
+echo
+echo == parent references self ==
+_img_info --backing-chain
+
+_make_test_img $size
+mv $TEST_IMG $TEST_IMG.1.base
+_make_test_img -b $TEST_IMG.1.base $size
+mv $TEST_IMG $TEST_IMG.2.base
+_make_test_img -b $TEST_IMG.2.base $size
+mv $TEST_IMG $TEST_IMG.3.base
+_make_test_img -b $TEST_IMG.3.base $size
+$QEMU_IMG rebase -u -b $TEST_IMG.2.base $TEST_IMG.1.base
+
+echo
+echo == ancestor references another ancestor ==
+_img_info --backing-chain
+
+_make_test_img $size
+mv $TEST_IMG $TEST_IMG.1.base
+_make_test_img -b $TEST_IMG.1.base $size
+mv $TEST_IMG $TEST_IMG.2.base
+_make_test_img -b $TEST_IMG.2.base $size
+
+echo
+echo == finite chain of length 3 (human) ==
+_img_info --backing-chain
+
+echo
+echo == finite chain of length 3 (json) ==
+_img_info --backing-chain --output=json
+
+# success, all done
+echo *** done
+rm -f $seq.full $TEST_IMG.[123].base
+status=0
diff --git a/tests/qemu-iotests/043.out b/tests/qemu-iotests/043.out
new file mode 100644
index 000..ad23337
--- /dev/null
+++ b/tests/qemu-iotests/043.out
@@ -0,0 +1,66 @@
+QA output created by 043
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
+
+== backing file references self ==
+qemu-img: Backing file 'TEST_DIR/t.IMGFMT' creates an infinite loop.
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
backing_file='TEST_DIR/t.IMGFMT.base' 
+
+== parent references self ==
+qemu-img: Backing file 'TEST_DIR/t.IMGFMT' creates an infinite loop.
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
backing_file='TEST_DIR/t.IMGFMT.1.base' 
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
backing_file='TEST_DIR/t.IMGFMT.2.base' 
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
backing_file='TEST_DIR/t.IMGFMT.3.base' 
+
+== ancestor references another ancestor ==
+qemu-img: Backing file 'TEST_DIR/t.IMGFMT.2.base' creates an infinite loop.
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
backing_file='TEST_DIR/t.IMGFMT.1.base' 
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 
backing_file='TEST_DIR/t.IMGFMT.2.base' 
+
+== finite chain of length 3 (human) ==
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 128M (134217728 bytes)
+cluster_size: 65536
+backing file: TEST_DIR/t.IMGFMT.2.base
+
+image: TEST_DIR/t.IMGFMT.2.base
+file format: IMGFMT
+virtual size: 128M (134217728 bytes)
+cluster_size: 65536
+backing file: TEST_DIR/t.IMGFMT.1.base
+
+image: TEST_DIR/t.IMGFMT.1.base
+file format: IMGFMT
+virtual size: 128M (134217728 bytes)
+cluster_size: 65536
+
+== finite chain of length 3 (json) ==
+[
+{

Re: [Qemu-devel] [PATCH 01/18] error: add error_set_errno and error_setg_errno

2012-10-17 Thread Markus Armbruster
Luiz Capitulino lcapitul...@redhat.com writes:

 On Thu, 04 Oct 2012 18:16:13 +0200
 Paolo Bonzini pbonz...@redhat.com wrote:

 Il 04/10/2012 18:14, Luiz Capitulino ha scritto:
   +void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
   + const char *fmt, ...)
  
  The function's name makes me expect that something else is done
  with os_errno
  
 
 Why something else? :)

 What I meant is that, it's not clear from the function's name how os_errno
 is used. Actually, it gives the impression you're storing errno, but I might
 be biased :)

 But again, I don't have any better suggestions.

A bit long for my taste, but here goes anyway: error_set_with_errno()



Re: [Qemu-devel] [PATCH qom-cpu v2 1/7] target-i386: Inline APIC cpu_env property setting

2012-10-17 Thread Jan Kiszka
On 2012-10-16 18:02, Andreas Färber wrote:
 Am 12.10.2012 03:26, schrieb Andreas Färber:
 This prepares for changing the variable type from void*.

 Signed-off-by: Andreas Färber afaer...@suse.de
 Cc: Igor Mammedov imamm...@redhat.com
 
 Paolo, are you happy with getting rid of the pointer property this way?
 
 Jan, are you okay with accessing APICCommonState in target-i386/cpu.c as
 an interim solution?

Yes, I don't see it as a problem.

Jan

 
 Andreas
 
 ---
  hw/apic_common.c  |1 -
  target-i386/cpu.c |5 -
  2 Dateien geändert, 4 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)

 diff --git a/hw/apic_common.c b/hw/apic_common.c
 index 371f95d..a26a631 100644
 --- a/hw/apic_common.c
 +++ b/hw/apic_common.c
 @@ -368,7 +368,6 @@ static const VMStateDescription vmstate_apic_common = {
  
  static Property apic_properties_common[] = {
  DEFINE_PROP_UINT8(id, APICCommonState, id, -1),
 -DEFINE_PROP_PTR(cpu_env, APICCommonState, cpu_env),
  DEFINE_PROP_BIT(vapic, APICCommonState, vapic_control, 
 VAPIC_ENABLE_BIT,
  true),
  DEFINE_PROP_END_OF_LIST(),
 diff --git a/target-i386/cpu.c b/target-i386/cpu.c
 index e307b4e..0cce910 100644
 --- a/target-i386/cpu.c
 +++ b/target-i386/cpu.c
 @@ -41,6 +41,7 @@
  #ifndef CONFIG_USER_ONLY
  #include hw/xen.h
  #include hw/sysbus.h
 +#include hw/apic_internal.h
  #endif
  
  /* feature flags taken from Intel Processor Identification and the CPUID
 @@ -1884,6 +1885,7 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error 
 **errp)
  #ifndef CONFIG_USER_ONLY
  static int apic_mapped;
  CPUX86State *env = cpu-env;
 +APICCommonState *apic;
  const char *apic_type = apic;
  
  if (kvm_irqchip_in_kernel()) {
 @@ -1902,7 +1904,8 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error 
 **errp)
OBJECT(env-apic_state), NULL);
  qdev_prop_set_uint8(env-apic_state, id, env-cpuid_apic_id);
  /* TODO: convert to link */
 -qdev_prop_set_ptr(env-apic_state, cpu_env, env);
 +apic = APIC_COMMON(env-apic_state);
 +apic-cpu_env = env;
  
  if (qdev_init(env-apic_state)) {
  error_setg(errp, APIC device '%s' could not be initialized,

 




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 1/2] hmp: fix info cpus for sparc targets

2012-10-17 Thread Luiz Capitulino
On Wed, 17 Oct 2012 08:06:13 +0200
Aurelien Jarno aurel...@aurel32.net wrote:

 On Tue, Oct 16, 2012 at 11:01:45PM -0300, Luiz Capitulino wrote:
  On Wed, 17 Oct 2012 01:28:34 +0200
  Aurelien Jarno aurel...@aurel32.net wrote:
  
   On sparc targets, info cpus returns this kind of output:
   
   | info cpus
   | * CPU #0: 
   pc=0x00424d18pc=0x00424d18npc=0x00424d1c 
   thread_id=19460
   
   pc is printed twice, there is no space between pc, pc and npc. As npc
   implies pc, don't set has_pc on sparc, and add a space between pc and
   npc.
   
   Cc: Luiz Capitulino lcapitul...@redhat.com
   Cc: Markus Armbruster arm...@redhat.com
   Cc: Blue Swirl blauwir...@gmail.com
   Signed-off-by: Aurelien Jarno aurel...@aurel32.net
   ---
cpus.c |2 --
hmp.c  |2 +-
2 files changed, 1 insertion(+), 3 deletions(-)
   
   diff --git a/cpus.c b/cpus.c
   index 750a76f..cbc8556 100644
   --- a/cpus.c
   +++ b/cpus.c
   @@ -1216,8 +1216,6 @@ CpuInfoList *qmp_query_cpus(Error **errp)
info-value-has_nip = true;
info-value-nip = env-nip;
#elif defined(TARGET_SPARC)
   -info-value-has_pc = true;
   -info-value-pc = env-pc;
info-value-has_npc = true;
info-value-npc = env-npc;
#elif defined(TARGET_MIPS)
   diff --git a/hmp.c b/hmp.c
   index 70bdec2..64b6ab5 100644
   --- a/hmp.c
   +++ b/hmp.c
   @@ -242,7 +242,7 @@ void hmp_info_cpus(Monitor *mon)
monitor_printf(mon, nip=0x%016 PRIx64, cpu-value-nip);
}
if (cpu-value-has_npc) {
   -monitor_printf(mon, pc=0x%016 PRIx64, cpu-value-pc);
   +monitor_printf(mon, pc=0x%016  PRIx64, cpu-value-pc);
  
  This patch changes qmp_query_cpus() to never set pc, so why do you print it?
 
 Well the print part of the code isn't change beside adding a space at
 the end. With the current code, has_npc implies has_pc, that's why I
 have removed the latter from qmp_query_cpus() in the sparc case.

My point is that you dropped the pc assignment along:

-info-value-pc = env-pc;

So, value-pc will be zeroed.

 If you prefer to change this behavior and not have has_npc implying
 has_pc, I can send another patch.

If pc is going to contain relevant info, then we have to set has_pc even
if has_npc implies it. If your point is that pc is irrelevant, then we have
to set has_pc to false and drop its printing from HMP.

 
monitor_printf(mon, npc=0x%016 PRIx64, cpu-value-npc);
}
if (cpu-value-has_PC) {
  
  
 




Re: [Qemu-devel] [PATCH 01/18] error: add error_set_errno and error_setg_errno

2012-10-17 Thread Markus Armbruster
Paolo Bonzini pbonz...@redhat.com writes:

 These functions help maintaining homogeneous formatting of error
 messages that include strerror values.

 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  error.c | 28 
  error.h |  9 +
  2 file modificati, 37 inserzioni(+)

 diff --git a/error.c b/error.c
 index 1f05fc4..128d88c 100644
 --- a/error.c
 +++ b/error.c
 @@ -43,6 +43,34 @@ void error_set(Error **errp, ErrorClass err_class, const 
 char *fmt, ...)
  *errp = err;
  }
  
 +void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
 + const char *fmt, ...)
 +{
 +Error *err;
 +char *msg1;
 +va_list ap;
 +
 +if (errp == NULL) {
 +return;
 +}
 +assert(*errp == NULL);
 +
 +err = g_malloc0(sizeof(*err));
 +
 +va_start(ap, fmt);
 +msg1 = g_strdup_vprintf(fmt, ap);
 +if (os_errno != 0) {
 +err-msg = g_strdup_printf(%s: %s, msg1, strerror(os_errno));
 +g_free(msg1);
 +} else {
 +err-msg = msg1;
 +}
 +va_end(ap);
 +err-err_class = err_class;
 +
 +*errp = err;
 +}
 +

Duplicates error_set() code without need.  How about:

void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
 const char *fmt, ...)
{
char *msg;

va_start(ap, fmt);
msg = g_strdup_printf(fmt, ap);
va_end(ap);
if (os_errno) {
error_set(errp, err_class, %s: %s, msg, strerror(os_errno));
} else {
error_set(errp, err_class, %s, msg);
}
g_free(msg);
}

Sketch, not even compile-tested.

Or the other way round, implement error_set() in terms of the more
general error_set_errno():

void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
{
error_set(errp, err_class, 0, fmt, ...);
}

Except that's not C; real code needs verror_set_errno().

As usual, every variadic function sooner or later needs a buddy that
takes a va_list instead.

  Error *error_copy(const Error *err)
  {
  Error *err_new;
[...]



Re: [Qemu-devel] [PATCH 01/18] error: add error_set_errno and error_setg_errno

2012-10-17 Thread Paolo Bonzini
Il 17/10/2012 14:56, Markus Armbruster ha scritto:
 Duplicates error_set() code without need.  How about:
 
 void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
  const char *fmt, ...)
 {
 char *msg;
 
 va_start(ap, fmt);
 msg = g_strdup_printf(fmt, ap);
 va_end(ap);
 if (os_errno) {
 error_set(errp, err_class, %s: %s, msg, strerror(os_errno));
 } else {
 error_set(errp, err_class, %s, msg);
 }
 g_free(msg);
 }
 
 Sketch, not even compile-tested.
 
 Or the other way round, implement error_set() in terms of the more
 general error_set_errno():
 
 void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
 {
 error_set(errp, err_class, 0, fmt, ...);
 }
 
 Except that's not C; real code needs verror_set_errno().
 
 As usual, every variadic function sooner or later needs a buddy that
 takes a va_list instead.

Indeed... lazy me.

Added to my todo list.

Paolo




Re: [Qemu-devel] [PATCH 02/18] qemu-sockets: add Error ** to all functions

2012-10-17 Thread Markus Armbruster
Paolo Bonzini pbonz...@redhat.com writes:

 Il 04/10/2012 18:19, Luiz Capitulino ha scritto:
   
  -int unix_listen_opts(QemuOpts *opts)
  +int unix_listen_opts(QemuOpts *opts, Error **errp)
   {
  -fprintf(stderr, unix sockets are not available on windows\n);
  +error_setg(errp, unix sockets are not available on windows);

Just gave me a review WTF, too.

 As we've discussed in a previous series, this breaks error reporting
 for unix_listen_opts() until errp is passed _and_ the error is routed
 to the right direction (in this case stderr).
 
 Is this fixed later in this series? If it's not, when do you plan to fix it?

 It is (which is why the single patch became 18), and I can move this
 hunk at the end of the series.  Note that is just for Windows code and
 should really never run.

Fair enough, but a note in the commit message is in order.



[Qemu-devel] [PATCH 2/4] Setting -sandbox on as deafult

2012-10-17 Thread Eduardo Otubo
Now the seccomp filter will be set to on even if no argument
-sandbox is given.

Signed-off-by: Eduardo Otubo ot...@linux.vnet.ibm.com
---
 configure |  2 +-
 vl.c  | 38 +++---
 2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/configure b/configure
index 353d788..c613a51 100755
--- a/configure
+++ b/configure
@@ -220,7 +220,7 @@ guest_agent=yes
 want_tools=yes
 libiscsi=
 coroutine=
-seccomp=
+seccomp=yes
 glusterfs=
 
 # parse CC options first
diff --git a/vl.c b/vl.c
index 5b357a3..bec68cd 100644
--- a/vl.c
+++ b/vl.c
@@ -276,6 +276,10 @@ static int default_cdrom = 1;
 static int default_sdcard = 1;
 static int default_vga = 1;
 
+#ifdef CONFIG_SECCOMP
+bool seccomp_on = true;
+#endif
+
 static struct {
 const char *driver;
 int *flag;
@@ -770,23 +774,28 @@ static int bt_parse(const char *opt)
 return 1;
 }
 
-static int parse_sandbox(QemuOpts *opts, void *opaque)
+static int install_seccomp_filters(void)
 {
-/* FIXME: change this to true for 1.3 */
-if (qemu_opt_get_bool(opts, enable, false)) {
 #ifdef CONFIG_SECCOMP
-if (seccomp_start()  0) {
-qerror_report(ERROR_CLASS_GENERIC_ERROR,
-  failed to install seccomp syscall filter in the 
kernel);
-return -1;
-}
-#else
+if (seccomp_start()  0) {
 qerror_report(ERROR_CLASS_GENERIC_ERROR,
-  sandboxing request but seccomp is not compiled into 
this build);
+failed to install seccomp syscall filter in the kernel);
 return -1;
-#endif
 }
+#else
+qerror_report(ERROR_CLASS_GENERIC_ERROR,
+sandboxing requested but seccomp is not compiled into this 
build);
+return -1;
+#endif
+return 0;
+}
+
 
+static int parse_sandbox(QemuOpts *opts, void *opaque)
+{
+if (!qemu_opt_get_bool(opts, enable, true)) {
+seccomp_on = false;
+}
 return 0;
 }
 
@@ -3320,6 +3329,13 @@ int main(int argc, char **argv, char **envp)
 exit(1);
 }
 
+/* We should install seccomp filters even if -sandbox on is not used. */
+if (seccomp_on) {
+if (install_seccomp_filters()  0) {
+exit(1);
+}
+}
+
 if (machine == NULL) {
 fprintf(stderr, No machine found.\n);
 exit(1);
-- 
1.7.12




[Qemu-devel] [PATCH 3/4] Support for double whitelist filters

2012-10-17 Thread Eduardo Otubo
This patch includes a second whitelist right before the main loop. It's
a smaller and more restricted whitelist, excluding execve() among many
others.

Signed-off-by: Eduardo Otubo ot...@linux.vnet.ibm.com
---
 qemu-seccomp.c | 94 --
 qemu-seccomp.h |  7 -
 vl.c   | 13 +++-
 3 files changed, 103 insertions(+), 11 deletions(-)

diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index a25f2fa..9c68af5 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -13,6 +13,7 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 #include stdio.h
+#include stdlib.h
 #include seccomp.h
 #include qemu-seccomp.h
 
@@ -21,7 +22,7 @@ struct QemuSeccompSyscall {
 uint8_t priority;
 };
 
-static const struct QemuSeccompSyscall seccomp_whitelist[] = {
+static const struct QemuSeccompSyscall seccomp_whitelist_init[] = {
 { SCMP_SYS(timer_settime), 255 },
 { SCMP_SYS(timer_gettime), 254 },
 { SCMP_SYS(futex), 253 },
@@ -118,27 +119,102 @@ static const struct QemuSeccompSyscall 
seccomp_whitelist[] = {
 { SCMP_SYS(accept4), 242 }
 };
 
-int seccomp_start(void)
+static const struct QemuSeccompSyscall seccomp_whitelist_main_loop[] = {
+{ SCMP_SYS(timer_settime), 255 },
+{ SCMP_SYS(timer_gettime), 254 },
+{ SCMP_SYS(futex), 253 },
+{ SCMP_SYS(select), 252 },
+{ SCMP_SYS(recvfrom), 251 },
+{ SCMP_SYS(sendto), 250 },
+{ SCMP_SYS(read), 249 },
+{ SCMP_SYS(brk), 248 },
+{ SCMP_SYS(mmap), 247 },
+#if defined(__i386__)
+{ SCMP_SYS(fcntl64), 245 },
+{ SCMP_SYS(fstat64), 245 },
+{ SCMP_SYS(stat64), 245 },
+{ SCMP_SYS(getgid32), 245 },
+{ SCMP_SYS(getegid32), 245 },
+{ SCMP_SYS(getuid32), 245 },
+{ SCMP_SYS(geteuid32), 245 },
+{ SCMP_SYS(sigreturn), 245 },
+{ SCMP_SYS(_newselect), 245 },
+{ SCMP_SYS(_llseek), 245 },
+{ SCMP_SYS(mmap2), 245},
+{ SCMP_SYS(sigprocmask), 245 },
+#endif
+{ SCMP_SYS(exit), 245 },
+{ SCMP_SYS(timer_delete), 245 },
+{ SCMP_SYS(exit_group), 245 },
+{ SCMP_SYS(rt_sigreturn), 245 },
+{ SCMP_SYS(madvise), 245 },
+{ SCMP_SYS(write), 244 },
+{ SCMP_SYS(fcntl), 243 },
+{ SCMP_SYS(tgkill), 242 },
+{ SCMP_SYS(rt_sigaction), 242 },
+{ SCMP_SYS(pipe2), 242 },
+{ SCMP_SYS(munmap), 242 },
+{ SCMP_SYS(mremap), 242 },
+{ SCMP_SYS(getsockname), 242 },
+{ SCMP_SYS(getpeername), 242 },
+{ SCMP_SYS(close), 242 },
+{ SCMP_SYS(accept4), 242 }
+};
+
+static int
+process_whitelist(const struct QemuSeccompSyscall *whitelist,
+  unsigned int size, scmp_filter_ctx *ctx)
 {
 int rc = 0;
+
 unsigned int i = 0;
-scmp_filter_ctx ctx;
+
+for (i = 0; i  size; i++) {
+rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, whitelist[i].num, 0);
+if (rc  0) {
+return -1;
+}
+
+rc = seccomp_syscall_priority(ctx, whitelist[i].num,
+  whitelist[i].priority);
+if (rc  0) {
+return -1;
+}
+}
+return 0;
+}
+
+int
+seccomp_start(enum whitelist_mode mode, scmp_filter_ctx *ctx)
+{
+int rc = 0;
 
 ctx = seccomp_init(SCMP_ACT_KILL);
 if (ctx == NULL) {
+rc = -1;
 goto seccomp_return;
 }
 
-for (i = 0; i  ARRAY_SIZE(seccomp_whitelist); i++) {
-rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, seccomp_whitelist[i].num, 
0);
-if (rc  0) {
+switch (mode) {
+case INIT:
+if (process_whitelist
+(seccomp_whitelist_init,
+ ARRAY_SIZE(seccomp_whitelist_init), ctx)  0) {
+rc = -1;
 goto seccomp_return;
 }
-rc = seccomp_syscall_priority(ctx, seccomp_whitelist[i].num,
-  seccomp_whitelist[i].priority);
-if (rc  0) {
+break;
+case MAIN_LOOP:
+if (process_whitelist
+(seccomp_whitelist_main_loop,
+ ARRAY_SIZE(seccomp_whitelist_main_loop), ctx)  0) {
+rc = -1;
 goto seccomp_return;
 }
+break;
+default:
+rc = -1;
+goto seccomp_return;
 }
 
 rc = seccomp_load(ctx);
diff --git a/qemu-seccomp.h b/qemu-seccomp.h
index b2fc3f8..1c97978 100644
--- a/qemu-seccomp.h
+++ b/qemu-seccomp.h
@@ -18,5 +18,10 @@
 #include seccomp.h
 #include osdep.h
 
-int seccomp_start(void);
+enum whitelist_mode {
+INIT = 0,
+MAIN_LOOP = 1,
+};
+
+int seccomp_start(enum whitelist_mode mode, scmp_filter_ctx *ctx);
 #endif
diff --git a/vl.c b/vl.c
index bec68cd..773d488 100644
--- a/vl.c
+++ b/vl.c
@@ -278,6 +278,7 @@ static int default_vga = 1;
 
 #ifdef CONFIG_SECCOMP
 bool seccomp_on = true;
+scmp_filter_ctx ctx;
 #endif
 
 static struct {
@@ -777,7 +778,7 @@ static int bt_parse(const char *opt)
 static int install_seccomp_filters(void)
 {
 #ifdef CONFIG_SECCOMP
-if (seccomp_start()  0) {
+if (seccomp_start(INIT, ctx)  0) {
 

[Qemu-devel] [PATCH 4/4] Warning messages on net devices hotplug

2012-10-17 Thread Eduardo Otubo
With the inclusion of the new double whitelist seccomp filter, Qemu
won't be able to execve() in runtime, thus, no hotplug net devices
allowed.

Signed-off-by: Eduardo Otubo ot...@linux.vnet.ibm.com
---
 hmp.c |  6 ++
 net.c | 13 +
 2 files changed, 19 insertions(+)

diff --git a/hmp.c b/hmp.c
index 70bdec2..f258338 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1091,6 +1091,12 @@ void hmp_netdev_add(Monitor *mon, const QDict *qdict)
 Error *err = NULL;
 QemuOpts *opts;
 
+#ifdef CONFIG_SECCOMP
+error_set(err, ERROR_CLASS_GENERIC_ERROR,
+Cannot hotplug TAP device when -sandbox is in effect);
+goto out;
+#endif
+
 opts = qemu_opts_from_qdict(qemu_find_opts(netdev), qdict, err);
 if (error_is_set(err)) {
 goto out;
diff --git a/net.c b/net.c
index ae4bc0d..a652ee9 100644
--- a/net.c
+++ b/net.c
@@ -752,6 +752,12 @@ void net_host_device_add(Monitor *mon, const QDict *qdict)
 Error *local_err = NULL;
 QemuOpts *opts;
 
+#ifdef CONFIG_SECCOMP
+error_set(local_err, ERROR_CLASS_GENERIC_ERROR,
+Cannot hotplug TAP device when -sandbox is in effect);
+goto out;
+#endif
+
 if (!net_host_check_device(device)) {
 monitor_printf(mon, invalid host network device %s\n, device);
 return;
@@ -765,6 +771,7 @@ void net_host_device_add(Monitor *mon, const QDict *qdict)
 qemu_opt_set(opts, type, device);
 
 net_client_init(opts, 0, local_err);
+out:
 if (error_is_set(local_err)) {
 qerror_report_err(local_err);
 error_free(local_err);
@@ -800,6 +807,12 @@ int qmp_netdev_add(Monitor *mon, const QDict *qdict, 
QObject **ret)
 QemuOptsList *opts_list;
 QemuOpts *opts;
 
+#ifdef CONFIG_SECCOMP
+error_set(local_err, ERROR_CLASS_GENERIC_ERROR,
+Cannot hotplug TAP device when -sandbox is in effect);
+goto exit_err;
+#endif
+
 opts_list = qemu_find_opts_err(netdev, local_err);
 if (error_is_set(local_err)) {
 goto exit_err;
-- 
1.7.12




Re: [Qemu-devel] [PATCH 03/18] qemu-sockets: unix_listen and unix_connect are portable

2012-10-17 Thread Markus Armbruster
Paolo Bonzini pbonz...@redhat.com writes:

 They are just wrappers and do not need a Win32-specific version.

This gave me pause (it's my thick-headed day, apparently), until I
realized that in addition to what the commit message says you also move
related code.  Please try harder to make review as easy as possible.



Re: [Qemu-devel] [PATCH 03/18] qemu-sockets: unix_listen and unix_connect are portable

2012-10-17 Thread Paolo Bonzini
Il 17/10/2012 15:17, Markus Armbruster ha scritto:
  They are just wrappers and do not need a Win32-specific version.
 This gave me pause (it's my thick-headed day, apparently), until I
 realized that in addition to what the commit message says you also move
 related code.  Please try harder to make review as easy as possible.

I don't, it's just git that decided to make the diff this way. :)

Paolo



[Qemu-devel] [PATCH 1/4] Adding new syscalls (bugzilla 855162)

2012-10-17 Thread Eduardo Otubo
According to the bug 855162[0] - there's the need of adding new syscalls
to the whitelist whenn using Qemu with Libvirt.

[1] - https://bugzilla.redhat.com/show_bug.cgi?id=855162

Reported-by: Paul Moore pmo...@redhat.com
Signed-off-by: Eduardo Otubo ot...@linux.vnet.ibm.com
---
 qemu-seccomp.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 64329a3..a25f2fa 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -45,6 +45,13 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = 
{
 { SCMP_SYS(access), 245 },
 { SCMP_SYS(prctl), 245 },
 { SCMP_SYS(signalfd), 245 },
+{ SCMP_SYS(getrlimit), 245 },
+{ SCMP_SYS(set_tid_address), 245 },
+{ SCMP_SYS(socketpair), 245 },
+{ SCMP_SYS(statfs), 245 },
+{ SCMP_SYS(unlink), 245 },
+{ SCMP_SYS(wait4), 245 },
+{ SCMP_SYS(getuid), 245 },
 #if defined(__i386__)
 { SCMP_SYS(fcntl64), 245 },
 { SCMP_SYS(fstat64), 245 },
@@ -107,7 +114,8 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] 
= {
 { SCMP_SYS(getsockname), 242 },
 { SCMP_SYS(getpeername), 242 },
 { SCMP_SYS(fdatasync), 242 },
-{ SCMP_SYS(close), 242 }
+{ SCMP_SYS(close), 242 },
+{ SCMP_SYS(accept4), 242 }
 };
 
 int seccomp_start(void)
-- 
1.7.12




[Qemu-devel] [PATCH 09/14] vga: stop direct access to DisplaySurface fields.

2012-10-17 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 hw/vga.c |9 +++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/vga.c b/hw/vga.c
index a0ba94d..4d34469 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1718,8 +1718,13 @@ static void vga_draw_graphic(VGACommonState *s, int 
full_update)
 s-last_depth = depth;
 full_update = 1;
 } else if (is_buffer_shared(s-ds-surface) 
-   (full_update || s-ds-surface-data != s-vram_ptr + 
(s-start_addr * 4))) {
-s-ds-surface-data = s-vram_ptr + (s-start_addr * 4);
+   (full_update || ds_get_data(s-ds) != s-vram_ptr
++ (s-start_addr * 4))) {
+qemu_free_displaysurface(s-ds);
+s-ds-surface = qemu_create_displaysurface_from(disp_width,
+height, depth,
+s-line_offset,
+s-vram_ptr + (s-start_addr * 4));
 dpy_gfx_setdata(s-ds);
 }
 
-- 
1.7.1




[Qemu-devel] [PATCH 04/14] pixman: helper functions

2012-10-17 Thread Gerd Hoffmann
Add some helper functions which will be put
into use by following patches.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 Makefile.objs |1 +
 qemu-pixman.c |   60 +
 qemu-pixman.h |   32 ++
 3 files changed, 93 insertions(+), 0 deletions(-)
 create mode 100644 qemu-pixman.c
 create mode 100644 qemu-pixman.h

diff --git a/Makefile.objs b/Makefile.objs
index 74b3542..73dd3bc 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -64,6 +64,7 @@ common-obj-y = $(block-obj-y) blockdev.o block/
 common-obj-y += net.o net/
 common-obj-y += qom/
 common-obj-y += readline.o console.o cursor.o
+common-obj-y += qemu-pixman.o
 common-obj-y += $(oslib-obj-y)
 common-obj-$(CONFIG_WIN32) += os-win32.o
 common-obj-$(CONFIG_POSIX) += os-posix.o
diff --git a/qemu-pixman.c b/qemu-pixman.c
new file mode 100644
index 000..7547ed7
--- /dev/null
+++ b/qemu-pixman.c
@@ -0,0 +1,60 @@
+#include qemu-pixman.h
+
+int qemu_pixman_get_type(int rshift, int gshift, int bshift)
+{
+int type = PIXMAN_TYPE_OTHER;
+
+if (rshift  gshift  gshift  bshift) {
+if (bshift == 0) {
+type = PIXMAN_TYPE_ARGB;
+} else {
+#if PIXMAN_VERSION = PIXMAN_VERSION_ENCODE(0, 21, 8)
+type = PIXMAN_TYPE_RGBA;
+#endif
+}
+} else if (rshift  gshift  gshift  bshift) {
+if (rshift == 0) {
+type = PIXMAN_TYPE_ABGR;
+} else {
+type = PIXMAN_TYPE_BGRA;
+}
+}
+return type;
+}
+
+pixman_format_code_t qemu_pixman_get_format(PixelFormat *pf)
+{
+pixman_format_code_t format;
+int type;
+
+type = qemu_pixman_get_type(pf-rshift, pf-gshift, pf-bshift);
+format = PIXMAN_FORMAT(pf-bits_per_pixel, type,
+   pf-abits, pf-rbits, pf-gbits, pf-bbits);
+if (!pixman_format_supported_source(format)) {
+return 0;
+}
+return format;
+}
+
+pixman_image_t *qemu_pixman_linebuf_create(pixman_format_code_t format,
+   int width)
+{
+pixman_image_t *image = pixman_image_create_bits(format, width, 1, NULL, 
0);
+assert(image != NULL);
+return image;
+}
+
+void qemu_pixman_linebuf_fill(pixman_image_t *linebuf, pixman_image_t *fb,
+  int width, int y)
+{
+pixman_image_composite(PIXMAN_OP_SRC, fb, NULL, linebuf,
+   0, y, 0, 0, 0, 0, width, 1);
+}
+
+void qemu_pixman_image_unref(pixman_image_t *image)
+{
+if (image == NULL) {
+return;
+}
+pixman_image_unref(image);
+}
diff --git a/qemu-pixman.h b/qemu-pixman.h
new file mode 100644
index 000..7652c41
--- /dev/null
+++ b/qemu-pixman.h
@@ -0,0 +1,32 @@
+#ifndef QEMU_PIXMAN_H
+#define QEMU_PIXMAN_H
+
+#include pixman.h
+
+#include console.h
+
+/*
+ * pixman image formats are defined to be native endian,
+ * that means host byte order on qemu.  So we go define
+ * fixed formats here for cases where it is needed, like
+ * feeding libjpeg / libpng and writing screenshots.
+ */
+
+#ifdef HOST_WORDS_BIGENDIAN
+# define PIXMAN_BE_r8g8b8 PIXMAN_r8g8b8
+#else
+# define PIXMAN_BE_r8g8b8 PIXMAN_b8g8r8
+#endif
+
+/*  */
+
+int qemu_pixman_get_type(int rshift, int gshift, int bshift);
+pixman_format_code_t qemu_pixman_get_format(PixelFormat *pf);
+
+pixman_image_t *qemu_pixman_linebuf_create(pixman_format_code_t format,
+   int width);
+void qemu_pixman_linebuf_fill(pixman_image_t *linebuf, pixman_image_t *fb,
+  int width, int y);
+void qemu_pixman_image_unref(pixman_image_t *image);
+
+#endif /* QEMU_PIXMAN_H */
-- 
1.7.1




[Qemu-devel] [PATCH 02/14] pixman: add submodule

2012-10-17 Thread Gerd Hoffmann
Add pixman submodule as fallback for old distros.
Picking version 0.18.4.  This is shipped by rhel6
and also the minimum version needed by spice so this
should serve well as baseline.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 .gitmodules |3 +++
 pixman  |1 +
 2 files changed, 4 insertions(+), 0 deletions(-)
 create mode 16 pixman

diff --git a/.gitmodules b/.gitmodules
index eca876f..cfa2af9 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -19,3 +19,6 @@
 [submodule roms/sgabios]
path = roms/sgabios
url = git://git.qemu.org/sgabios.git
+[submodule pixman]
+   path = pixman
+   url = git://anongit.freedesktop.org/pixman
diff --git a/pixman b/pixman
new file mode 16
index 000..97336fa
--- /dev/null
+++ b/pixman
@@ -0,0 +1 @@
+Subproject commit 97336fad32acf802003855cd8bd6477fa49a12e3
-- 
1.7.1




[Qemu-devel] [PATCH 06/14] console: make qemu_alloc_display static

2012-10-17 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 console.c |   48 
 console.h |2 --
 2 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/console.c b/console.c
index 5e1c5f5..48d88e4 100644
--- a/console.c
+++ b/console.c
@@ -1294,30 +1294,8 @@ static QemuConsole *new_console(DisplayState *ds, 
console_type_t console_type)
 return s;
 }
 
-DisplaySurface *qemu_create_displaysurface(DisplayState *ds,
-   int width, int height)
-{
-DisplaySurface *surface = g_new0(DisplaySurface, 1);
-
-int linesize = width * 4;
-qemu_alloc_display(surface, width, height, linesize,
-   qemu_default_pixelformat(32), 0);
-return surface;
-}
-
-DisplaySurface *qemu_resize_displaysurface(DisplayState *ds,
-   int width, int height)
-{
-int linesize = width * 4;
-
-trace_displaysurface_resize(ds, ds-surface, width, height);
-qemu_alloc_display(ds-surface, width, height, linesize,
-   qemu_default_pixelformat(32), 0);
-return ds-surface;
-}
-
-void qemu_alloc_display(DisplaySurface *surface, int width, int height,
-int linesize, PixelFormat pf, int newflags)
+static void qemu_alloc_display(DisplaySurface *surface, int width, int height,
+   int linesize, PixelFormat pf, int newflags)
 {
 surface-width = width;
 surface-height = height;
@@ -1342,6 +1320,28 @@ void qemu_alloc_display(DisplaySurface *surface, int 
width, int height,
 #endif
 }
 
+DisplaySurface *qemu_create_displaysurface(DisplayState *ds,
+   int width, int height)
+{
+DisplaySurface *surface = g_new0(DisplaySurface, 1);
+
+int linesize = width * 4;
+qemu_alloc_display(surface, width, height, linesize,
+   qemu_default_pixelformat(32), 0);
+return surface;
+}
+
+DisplaySurface *qemu_resize_displaysurface(DisplayState *ds,
+   int width, int height)
+{
+int linesize = width * 4;
+
+trace_displaysurface_resize(ds, ds-surface, width, height);
+qemu_alloc_display(ds-surface, width, height, linesize,
+   qemu_default_pixelformat(32), 0);
+return ds-surface;
+}
+
 DisplaySurface *qemu_create_displaysurface_from(int width, int height, int bpp,
 int linesize, uint8_t *data)
 {
diff --git a/console.h b/console.h
index c546e98..96ef165 100644
--- a/console.h
+++ b/console.h
@@ -190,8 +190,6 @@ void register_displaystate(DisplayState *ds);
 DisplayState *get_displaystate(void);
 DisplaySurface* qemu_create_displaysurface_from(int width, int height, int bpp,
 int linesize, uint8_t *data);
-void qemu_alloc_display(DisplaySurface *surface, int width, int height,
-int linesize, PixelFormat pf, int newflags);
 PixelFormat qemu_different_endianness_pixelformat(int bpp);
 PixelFormat qemu_default_pixelformat(int bpp);
 
-- 
1.7.1




[Qemu-devel] [PATCH 12/14] pixman/vnc: remove rgb_prepare_row* functions

2012-10-17 Thread Gerd Hoffmann
Let pixman do it instead.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 ui/vnc-enc-tight.c |   84 ++-
 1 files changed, 10 insertions(+), 74 deletions(-)

diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 8013c5c..9fd2556 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -1145,74 +1145,6 @@ static int send_palette_rect(VncState *vs, int x, int y,
 return (bytes = 0);
 }
 
-#if defined(CONFIG_VNC_JPEG) || defined(CONFIG_VNC_PNG)
-static void rgb_prepare_row24(VncState *vs, uint8_t *dst, int x, int y,
-  int count)
-{
-VncDisplay *vd = vs-vd;
-uint32_t *fbptr;
-uint32_t pix;
-
-fbptr = vnc_server_fb_ptr(vd, x, y);
-
-while (count--) {
-pix = *fbptr++;
-*dst++ = (uint8_t)(pix  vs-ds-surface-pf.rshift);
-*dst++ = (uint8_t)(pix  vs-ds-surface-pf.gshift);
-*dst++ = (uint8_t)(pix  vs-ds-surface-pf.bshift);
-}
-}
-
-#define DEFINE_RGB_GET_ROW_FUNCTION(bpp)\
-\
-static void \
-rgb_prepare_row##bpp(VncState *vs, uint8_t *dst,\
- int x, int y, int count)   \
-{   \
-VncDisplay *vd = vs-vd;\
-uint##bpp##_t *fbptr;   \
-uint##bpp##_t pix;  \
-int r, g, b;\
-\
-fbptr = vnc_server_fb_ptr(vd, x, y);\
-\
-while (count--) {   \
-pix = *fbptr++; \
-\
-r = (int)((pix  vs-ds-surface-pf.rshift)   \
-   vs-ds-surface-pf.rmax);  \
-g = (int)((pix  vs-ds-surface-pf.gshift)   \
-   vs-ds-surface-pf.gmax);  \
-b = (int)((pix  vs-ds-surface-pf.bshift)   \
-   vs-ds-surface-pf.bmax);  \
-\
-*dst++ = (uint8_t)((r * 255 + vs-ds-surface-pf.rmax / 2) \
-   / vs-ds-surface-pf.rmax); \
-*dst++ = (uint8_t)((g * 255 + vs-ds-surface-pf.gmax / 2) \
-   / vs-ds-surface-pf.gmax); \
-*dst++ = (uint8_t)((b * 255 + vs-ds-surface-pf.bmax / 2) \
-   / vs-ds-surface-pf.bmax); \
-}   \
-}
-
-DEFINE_RGB_GET_ROW_FUNCTION(16)
-DEFINE_RGB_GET_ROW_FUNCTION(32)
-
-static void rgb_prepare_row(VncState *vs, uint8_t *dst, int x, int y,
-int count)
-{
-if (VNC_SERVER_FB_BYTES == 4) {
-if (1) {
-rgb_prepare_row24(vs, dst, x, y, count);
-} else {
-rgb_prepare_row32(vs, dst, x, y, count);
-}
-} else {
-rgb_prepare_row16(vs, dst, x, y, count);
-}
-}
-#endif /* CONFIG_VNC_JPEG or CONFIG_VNC_PNG */
-
 /*
  * JPEG compression stuff.
  */
@@ -1257,6 +1189,7 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int 
w, int h, int quality)
 struct jpeg_compress_struct cinfo;
 struct jpeg_error_mgr jerr;
 struct jpeg_destination_mgr manager;
+pixman_image_t *linebuf;
 JSAMPROW row[1];
 uint8_t *buf;
 int dy;
@@ -1285,13 +1218,14 @@ static int send_jpeg_rect(VncState *vs, int x, int y, 
int w, int h, int quality)
 
 jpeg_start_compress(cinfo, true);
 
-buf = g_malloc(w * 3);
+linebuf = qemu_pixman_linebuf_create(PIXMAN_BE_r8g8b8, w);
+buf = (uint8_t *)pixman_image_get_data(linebuf);
 row[0] = buf;
 for (dy = 0; dy  h; dy++) {
-rgb_prepare_row(vs, buf, x, y + dy, w);
+qemu_pixman_linebuf_fill(linebuf, vs-vd-server, w, dy);
 jpeg_write_scanlines(cinfo, row, 1);
 }
-g_free(buf);
+qemu_pixman_image_unref(linebuf);
 
 jpeg_finish_compress(cinfo);
 jpeg_destroy_compress(cinfo);
@@ -1370,6 +1304,7 @@ static int send_png_rect(VncState *vs, int x, int y, int 
w, int h,
 png_structp png_ptr;
 png_infop info_ptr;
 png_colorp png_palette = NULL;
+pixman_image_t *linebuf;
 int level = tight_png_conf[vs-tight.compression].png_zlib_level;
 int filters = 

[Qemu-devel] [PATCH 14/14] pixman: drop obsolete fields from DisplaySurface

2012-10-17 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 console.c |9 -
 console.h |   23 +--
 2 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/console.c b/console.c
index d28b75e..048b48e 100644
--- a/console.c
+++ b/console.c
@@ -1297,14 +1297,10 @@ static QemuConsole *new_console(DisplayState *ds, 
console_type_t console_type)
 static void qemu_alloc_display(DisplaySurface *surface, int width, int height,
int linesize, PixelFormat pf, int newflags)
 {
-surface-width = width;
-surface-height = height;
-surface-linesize = linesize;
 surface-pf = pf;
 
 qemu_pixman_image_unref(surface-image);
 surface-image = NULL;
-surface-data = NULL;
 
 surface-format = qemu_pixman_get_format(pf);
 assert(surface-format != 0);
@@ -1313,7 +1309,6 @@ static void qemu_alloc_display(DisplaySurface *surface, 
int width, int height,
   NULL, linesize);
 assert(surface-image != NULL);
 
-surface-data = (uint8_t *)pixman_image_get_data(surface-image);
 surface-flags = newflags | QEMU_ALLOCATED_FLAG;
 #ifdef HOST_WORDS_BIGENDIAN
 surface-flags |= QEMU_BIG_ENDIAN_FLAG;
@@ -1347,9 +1342,6 @@ DisplaySurface *qemu_create_displaysurface_from(int 
width, int height, int bpp,
 {
 DisplaySurface *surface = g_new0(DisplaySurface, 1);
 
-surface-width = width;
-surface-height = height;
-surface-linesize = linesize;
 surface-pf = qemu_default_pixelformat(bpp);
 
 surface-format = qemu_pixman_get_format(surface-pf);
@@ -1362,7 +1354,6 @@ DisplaySurface *qemu_create_displaysurface_from(int 
width, int height, int bpp,
 #ifdef HOST_WORDS_BIGENDIAN
 surface-flags = QEMU_BIG_ENDIAN_FLAG;
 #endif
-surface-data = data;
 
 return surface;
 }
diff --git a/console.h b/console.h
index 96ef165..f66fbfc 100644
--- a/console.h
+++ b/console.h
@@ -123,10 +123,6 @@ struct DisplaySurface {
 pixman_format_code_t format;
 pixman_image_t *image;
 uint8_t flags;
-int width;
-int height;
-int linesize;/* bytes per line */
-uint8_t *data;
 
 struct PixelFormat pf;
 };
@@ -346,32 +342,39 @@ static inline bool dpy_cursor_define_supported(struct 
DisplayState *s)
 
 static inline int ds_get_linesize(DisplayState *ds)
 {
-return ds-surface-linesize;
+return pixman_image_get_stride(ds-surface-image);
 }
 
 static inline uint8_t* ds_get_data(DisplayState *ds)
 {
-return ds-surface-data;
+return (void *)pixman_image_get_data(ds-surface-image);
 }
 
 static inline int ds_get_width(DisplayState *ds)
 {
-return ds-surface-width;
+return pixman_image_get_width(ds-surface-image);
 }
 
 static inline int ds_get_height(DisplayState *ds)
 {
-return ds-surface-height;
+return pixman_image_get_height(ds-surface-image);
 }
 
 static inline int ds_get_bits_per_pixel(DisplayState *ds)
 {
-return ds-surface-pf.bits_per_pixel;
+int bits = PIXMAN_FORMAT_BPP(ds-surface-format);
+return bits;
 }
 
 static inline int ds_get_bytes_per_pixel(DisplayState *ds)
 {
-return ds-surface-pf.bytes_per_pixel;
+int bits = PIXMAN_FORMAT_BPP(ds-surface-format);
+return (bits + 7) / 8;
+}
+
+static inline pixman_format_code_t ds_get_format(DisplayState *ds)
+{
+return ds-surface-format;
 }
 
 #ifdef CONFIG_CURSES
-- 
1.7.1




Re: [Qemu-devel] [PATCH 04/25] qemu-sockets: add nonblocking connect for Unix sockets

2012-10-17 Thread Markus Armbruster
Paolo Bonzini pbonz...@redhat.com writes:

 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
 v1-v2: fixed connect_state memory leaks

  qemu-char.c|  2 +-
  qemu-sockets.c | 77 
 --
  qemu_socket.h  |  6 -
  3 file modificati, 70 inserzioni(+), 15 rimozioni(-)

 diff --git a/qemu-char.c b/qemu-char.c
 index 3cc6cb5..8ebd582 100644
 --- a/qemu-char.c
 +++ b/qemu-char.c
 @@ -2450,7 +2450,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts 
 *opts)
  if (is_listen) {
  fd = unix_listen_opts(opts, NULL);
  } else {
 -fd = unix_connect_opts(opts, NULL);
 +fd = unix_connect_opts(opts, NULL, NULL, NULL);
  }
  } else {
  if (is_listen) {
 diff --git a/qemu-sockets.c b/qemu-sockets.c
 index f7e67b6..6a49429 100644
 --- a/qemu-sockets.c
 +++ b/qemu-sockets.c
 @@ -252,16 +252,19 @@ static void wait_for_connect(void *opaque)
  }
  
  /* try to connect to the next address on the list */
 -while (s-current_addr-ai_next != NULL  s-fd  0) {
 -s-current_addr = s-current_addr-ai_next;
 -s-fd = inet_connect_addr(s-current_addr, in_progress, s);
 -/* connect in progress */
 -if (in_progress) {
 -return;
 +if (s-current_addr) {
 +while (s-current_addr-ai_next != NULL  s-fd  0) {
 +s-current_addr = s-current_addr-ai_next;
 +s-fd = inet_connect_addr(s-current_addr, in_progress, s);
 +/* connect in progress */
 +if (in_progress) {
 +return;
 +}
  }
 +
 +freeaddrinfo(s-addr_list);
  }
  
 -freeaddrinfo(s-addr_list);
  if (s-callback) {
  s-callback(s-fd, s-opaque);
  }
 @@ -701,11 +704,13 @@ err:
  return -1;
  }
  
 -int unix_connect_opts(QemuOpts *opts, Error **errp)
 +int unix_connect_opts(QemuOpts *opts, Error **errp,
 +  NonBlockingConnectHandler *callback, void *opaque)
  {
  struct sockaddr_un un;
  const char *path = qemu_opt_get(opts, path);
 -int sock;
 +ConnectState *connect_state = NULL;
 +int sock, rc;
  
  if (NULL == path) {
  fprintf(stderr, unix connect: no path specified\n);
 @@ -717,16 +722,44 @@ int unix_connect_opts(QemuOpts *opts, Error **errp)
  perror(socket(unix));
  return -1;
  }
 +if (callback != NULL) {
 +connect_state = g_malloc0(sizeof(*connect_state));
 +connect_state-callback = callback;
 +connect_state-opaque = opaque;
 +socket_set_nonblock(sock);
 +}
  
  memset(un, 0, sizeof(un));
  un.sun_family = AF_UNIX;
  snprintf(un.sun_path, sizeof(un.sun_path), %s, path);
 -if (connect(sock, (struct sockaddr*) un, sizeof(un))  0) {
 +
 +/* connect to peer */
 +do {
 +rc = 0;
 +if (connect(sock, (struct sockaddr *) un, sizeof(un))  0) {
 +rc = -socket_error();
 +}
 +} while (rc == -EINTR);

Isn't this a silent EINTR bug fix?

 +
 +if (connect_state != NULL  QEMU_SOCKET_RC_INPROGRESS(rc)) {
 +connect_state-fd = sock;
 +qemu_set_fd_handler2(sock, NULL, NULL, wait_for_connect,
 + connect_state);
 +return sock;
 +} else {
 +/* non blocking socket immediate success, call callback */
 +if (callback != NULL) {
 +callback(sock, opaque);
 +}
 +}
 +
 +if (rc  0) {
  fprintf(stderr, connect(unix:%s): %s\n, path, strerror(errno));
  close(sock);
 - return -1;
 +sock = -1;
  }
  
 +g_free(connect_state);
  return sock;
  }
  

Unlike inet_connect_opts(), this one runs callback() even when connect()
fails.  Are you sure that's what you want?

[...]



Re: [Qemu-devel] [PATCH 05/25] migration: avoid using error_is_set and thus relying on errp != NULL

2012-10-17 Thread Markus Armbruster
Paolo Bonzini pbonz...@redhat.com writes:

 The migration code is using errp to detect internal errors,
 this means that it relies on errp being non-NULL.

Impact?



Re: [Qemu-devel] [PATCH 05/25] migration: avoid using error_is_set and thus relying on errp != NULL

2012-10-17 Thread Paolo Bonzini
Il 17/10/2012 15:36, Markus Armbruster ha scritto:
  The migration code is using errp to detect internal errors,
  this means that it relies on errp being non-NULL.
 Impact?

None because so far our only QMP client is HMP and never passes a NULL
Error **.  But if we had others, it would make sure migration can work
with a NULL Error **.

Paolo



Re: [Qemu-devel] Any alternative to kqemu ?

2012-10-17 Thread Timothy Madden
On Mon, Oct 15, 2012 at 11:03 AM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 14/10/2012 12:52, Timothy Madden ha scritto:
 Hello

 Is kqemu deprecated ?

 It is simply not supported anymore.

 Is there an alternative to it ?

 No.

That is tough ... ! So my hardware is officially obsolete.

I had to get away with those other emulators provided by commercial
companies. VMWare said my hardware does not support virtualization
(much like qemu), but VirtualBox worked surprisingly well and uses the
CPU natively (no emulation, faster than my qemu without kvm).

 `qemu-system-i386 -net nic ...` keeps saying upon invocation that vlan0
 is not connected to host network. My `vconfig add eth1` command
 completed successfully, and I can `ifconfig eth1.0`, although I see no
 IP address on the new interface, only the mac address.

 QEMU VLANs have nothing to do with Linux VLANs.  Yes, that is confusing. :(

 If you install libvirt, you can use -net nic -net bridge,br=virbr0.

 Otherwise, you could configure a bridge yourself and just use -net nic
 -net bridge.

It turns out I had to say:
   ifconfig eth1.0 up

Which might be obvious for the people here, but is not that much
obvious when you first hear about vlans ...

Thank you for your responses,
Timothy Madden



Re: [Qemu-devel] [PATCH] hw/qxl: guest bug on primary create with stride %4 != 0

2012-10-17 Thread Gerd Hoffmann
On 10/15/12 14:54, Alon Levy wrote:
 Due to usage of pixman for rendering on all spice surfaces we have
 pixman's requirement that the stride be word aligned. A guest not
 honoring that can crash spice and qemu with it due to failure to create
 a surface (in spice-server). Avoid this early on in primary surface
 creation and offscreen surface creation.

Patch added to spice patch queue.

thanks,
  Gerd



Re: [Qemu-devel] [PATCH 04/25] qemu-sockets: add nonblocking connect for Unix sockets

2012-10-17 Thread Paolo Bonzini
Il 17/10/2012 15:33, Markus Armbruster ha scritto:
 Paolo Bonzini pbonz...@redhat.com writes:
 
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
 v1-v2: fixed connect_state memory leaks

  qemu-char.c|  2 +-
  qemu-sockets.c | 77 
 --
  qemu_socket.h  |  6 -
  3 file modificati, 70 inserzioni(+), 15 rimozioni(-)

 diff --git a/qemu-char.c b/qemu-char.c
 index 3cc6cb5..8ebd582 100644
 --- a/qemu-char.c
 +++ b/qemu-char.c
 @@ -2450,7 +2450,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts 
 *opts)
  if (is_listen) {
  fd = unix_listen_opts(opts, NULL);
  } else {
 -fd = unix_connect_opts(opts, NULL);
 +fd = unix_connect_opts(opts, NULL, NULL, NULL);
  }
  } else {
  if (is_listen) {
 diff --git a/qemu-sockets.c b/qemu-sockets.c
 index f7e67b6..6a49429 100644
 --- a/qemu-sockets.c
 +++ b/qemu-sockets.c
 @@ -252,16 +252,19 @@ static void wait_for_connect(void *opaque)
  }
  
  /* try to connect to the next address on the list */
 -while (s-current_addr-ai_next != NULL  s-fd  0) {
 -s-current_addr = s-current_addr-ai_next;
 -s-fd = inet_connect_addr(s-current_addr, in_progress, s);
 -/* connect in progress */
 -if (in_progress) {
 -return;
 +if (s-current_addr) {
 +while (s-current_addr-ai_next != NULL  s-fd  0) {
 +s-current_addr = s-current_addr-ai_next;
 +s-fd = inet_connect_addr(s-current_addr, in_progress, s);
 +/* connect in progress */
 +if (in_progress) {
 +return;
 +}
  }
 +
 +freeaddrinfo(s-addr_list);
  }
  
 -freeaddrinfo(s-addr_list);
  if (s-callback) {
  s-callback(s-fd, s-opaque);
  }
 @@ -701,11 +704,13 @@ err:
  return -1;
  }
  
 -int unix_connect_opts(QemuOpts *opts, Error **errp)
 +int unix_connect_opts(QemuOpts *opts, Error **errp,
 +  NonBlockingConnectHandler *callback, void *opaque)
  {
  struct sockaddr_un un;
  const char *path = qemu_opt_get(opts, path);
 -int sock;
 +ConnectState *connect_state = NULL;
 +int sock, rc;
  
  if (NULL == path) {
  fprintf(stderr, unix connect: no path specified\n);
 @@ -717,16 +722,44 @@ int unix_connect_opts(QemuOpts *opts, Error **errp)
  perror(socket(unix));
  return -1;
  }
 +if (callback != NULL) {
 +connect_state = g_malloc0(sizeof(*connect_state));
 +connect_state-callback = callback;
 +connect_state-opaque = opaque;
 +socket_set_nonblock(sock);
 +}
  
  memset(un, 0, sizeof(un));
  un.sun_family = AF_UNIX;
  snprintf(un.sun_path, sizeof(un.sun_path), %s, path);
 -if (connect(sock, (struct sockaddr*) un, sizeof(un))  0) {
 +
 +/* connect to peer */
 +do {
 +rc = 0;
 +if (connect(sock, (struct sockaddr *) un, sizeof(un))  0) {
 +rc = -socket_error();
 +}
 +} while (rc == -EINTR);
 
 Isn't this a silent EINTR bug fix?
 
 +
 +if (connect_state != NULL  QEMU_SOCKET_RC_INPROGRESS(rc)) {
 +connect_state-fd = sock;
 +qemu_set_fd_handler2(sock, NULL, NULL, wait_for_connect,
 + connect_state);
 +return sock;
 +} else {
 +/* non blocking socket immediate success, call callback */
 +if (callback != NULL) {
 +callback(sock, opaque);
 +}
 +}
 +
 +if (rc  0) {
  fprintf(stderr, connect(unix:%s): %s\n, path, strerror(errno));
  close(sock);
 -return -1;
 +sock = -1;
  }
  
 +g_free(connect_state);
  return sock;
  }
  
 
 Unlike inet_connect_opts(), this one runs callback() even when connect()
 fails.  Are you sure that's what you want?

I think it's harmless, but it's worth harmonizing it.

Paolo



Re: [Qemu-devel] [PATCH v3 2/4] monitor: Enable adding an inherited fd to an fd set

2012-10-17 Thread Kevin Wolf
Am 16.10.2012 20:08, schrieb Corey Bryant:
 qmp_add_fd() gets an fd that was received over a socket with
 SCM_RIGHTS and adds it to an fd set.  This patch adds support
 that will enable adding an fd that was inherited on the
 command line to an fd set.
 
 Note: All of the code added to monitor_fdset_add_fd(), with the
 exception of the error path for non-valid fdset-id, is code motion
 from qmp_add_fd().
 
 Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com

And the non-valid fdset-id error case leaks fd now, right?

Kevin



Re: [Qemu-devel] Any alternative to kqemu ?

2012-10-17 Thread Wei-Ren Chen
  Is there an alternative to it ?
 
  No.
 
 That is tough ... ! So my hardware is officially obsolete.
 
 I had to get away with those other emulators provided by commercial
 companies. VMWare said my hardware does not support virtualization
 (much like qemu), but VirtualBox worked surprisingly well and uses the
 CPU natively (no emulation, faster than my qemu without kvm).

  IIRC, VirtualBox take approach similar to kqemu does, i.e.,
translate privilege instructions only and run non-privilege
instructions on host cpu directly.

Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj



Re: [Qemu-devel] Any alternative to kqemu ?

2012-10-17 Thread Paolo Bonzini
Il 17/10/2012 15:39, Timothy Madden ha scritto:
 On Mon, Oct 15, 2012 at 11:03 AM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 14/10/2012 12:52, Timothy Madden ha scritto:
 Hello

 Is kqemu deprecated ?

 It is simply not supported anymore.

 Is there an alternative to it ?

 No.
 
 That is tough ... ! So my hardware is officially obsolete.

That is not because of obsoletion of hardware.  It's because kqemu was
holding off other enhancements to QEMU, and nobody stepped up to
maintain it.

 I had to get away with those other emulators provided by commercial
 companies. VMWare said my hardware does not support virtualization
 (much like qemu), but VirtualBox worked surprisingly well and uses the
 CPU natively (no emulation, faster than my qemu without kvm).

Yes, qemu without kvm can be quite slow because of the overhead of CPU
emulation.  One problem is that newer distros use SSE a lot, and this is
quite slow.

Paolo



Re: [Qemu-devel] nvram and boot order

2012-10-17 Thread Gleb Natapov
On Wed, Oct 17, 2012 at 07:07:53AM +1100, Benjamin Herrenschmidt wrote:
 On Tue, 2012-10-16 at 14:55 -0500, Anthony Liguori wrote:
  
  4) If -boot is specified, the parameter should alter the contents of
 NVRAM to change the boot order to what is specified by -boot.
  
  5) If ,bootorder is specified, it should take predence over -boot.
  
  6) ,bootorder= should also alter the contents of NVRAM to determine
  the
 boot order
 
 That's where I disagree. At least for us... I don't see why -boot or
 -bootorder should alter the nvram content.
 
 The plan is to have the nvram content essentially in control of SLOF
 (ie. the BIOS in x86 land). Qemu doesn't know much of anything appart
 from providing the storage for it.
 
 I see -boot and -bootorder as ways to *temporarily* override whatever
 setting was put in nvram by the guest. The nvram is typically populated
 by the distro installer (though sometimes by hand by the user). One may
 want to just temporarily boot from a CD (rescue for example, or to test
 a live CD or something ...), that doesn't mean the permanent setting
 should be altered.
 
 I would pass -boot and -bootorder to SLOF like we pass the current
 bootlist today and let it deal with it, I wouldn't touch the nvram.
 
Agree with you and David. This is not QEMU busyness to touch nvram
content. For all we know it may be encrypted by a key knows only to
the firmware.

--
Gleb.



Re: [Qemu-devel] [Bug 1066055] Re: Network performance regression with vde_switch

2012-10-17 Thread Stefan Hajnoczi
On Tue, Oct 16, 2012 at 2:23 PM, Edivaldo de Araujo Pereira
edivaldoapere...@yahoo.com.br wrote:
 Hi Stefan

 I finally could revert that commit in the latest snapshot; problem was I
 needed to revert one later, that modified hw/virtio-serial-bus.c
 accordingly; after that reversion, the regression in network performance
 went completely away; this confirms my previous identification of the
 commit that caused it.

 Additionally, I tested your last suggestion, to change '' to '=', and
 that didn't help; the problem was still there.

 By the way, the performance gain I observed ,of about 25~30 % when using
 a tun/tap, was in fact just apparent, because it was result of a greater
 use of cpu, so it was achieved only when the host was idle; when I
 stress the host, with a lot of concurrent guests generating traffic,
 there is no gain at all.

 Just for confirmation, this is the performance I get with latest
 snapshot (8b4a3df8081f3e6f1061ed5cbb303ad623ade66b) running wget in the
 guest:

 $ wget -O /dev/null http://172.18.1.1/bpd.iso
 --2012-10-16 09:10:18--  http://172.18.1.1/bpd.iso
 Connecting to 172.18.1.1:80... connected.
 HTTP request sent, awaiting response... 200 OK
 Length: 358979584 (342M) [application/x-iso9660-image]
 Saving to: `/dev/null'
 100%[==] 358.979.584 
 29,7M/s   in 11s
 2012-10-16 09:10:29 (30,3 MB/s) - `/dev/null' saved [358979584/358979584]

 The same wget, using the same snapshot, but with that commit reverted
 is:

 $ wget -O /dev/null http://172.18.1.1/bpd.iso
 --2012-10-16 09:15:12--  http://172.18.1.1/bpd.iso
 Connecting to 172.18.1.1:80... connected.
 HTTP request sent, awaiting response... 200 OK
 Length: 358979584 (342M) [application/x-iso9660-image]
 Saving to: `/dev/null'
 100%[==] 358.979.584  
 180M/s   in 1,9s
 2012-10-16 09:15:14 (180 MB/s) - `/dev/null' saved [358979584/358979584]

 So, as I can see, there is no doubt: that commit is the culprit; as it
 was intended to be related just to the quality of the source code (at
 least as I can see), but implied in such a cost in performance, I think
 it would be better to revert it.

Hi Amit,
Edivaldo has identified the following commit responsible for a network
performance regression he sees:

commit 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f
Author: Amit Shah amit.s...@redhat.com
Date:   Tue Sep 25 00:05:15 2012 +0530

virtio: Introduce virtqueue_get_avail_bytes()

I guess this is because we now iterate the entire descriptor chain to
check available space instead of returning early.

Do you want to propose a patch to fix it?

Stefan



Re: [Qemu-devel] [PATCH v3 4/4] qemu-config: Add new -add-fd command line option

2012-10-17 Thread Kevin Wolf
Am 17.10.2012 06:16, schrieb Eric Blake:
 I'm still seeing the corner case of:
 
 qemu-kvm -add-fd fd=3,set=1 -add-fd fd=4,set=2 4-
 
 where the dup(3) will populate fd 4 prior to the point where we get to
 process the -add-fd fd=4 command to notice that the user started
 qemu-kvm with fd 4 closed, and thus qemu will silently proceed to use
 the wrong fd.
 
 On the other hand, I'm not sure if that corner case is worth worrying
 about, or if we just chalk it up to user stupidity (aka libvirt
 programmer stupidity) if they did something like that (most likely,
 because the management app forgot to clear FD_CLOEXEC before exec()ing
 qemu-kvm).

If you specify an FD number that isn't actually open when qemu is
stared, you can get any FD that qemu opens internally. I think the
correct answer to this problem is then don't do that.

 Hmm, this makes me wonder if I can do something crazy like:
 
 qemu-kvm -add-fd fd=4,set=1 -qmp /dev/fdset/1
 
 to open a monitor on the fd I just passed in? 

I think so. At least on my side it was intended to allow this.

 And what if so, what then
 happens on that monitor if I request that fdset 1 be removed?

The same as with block devices: The fd stays open until the monitor
connection is closed. A closed monitor also triggers fd garbage
collection, so at this point the original fd would be closed (well,
assuming that you had only one monitor).

Kevin



[Qemu-devel] [PATCH 13/14] pixman/vnc: remove dead code.

2012-10-17 Thread Gerd Hoffmann
Switching the vnc server framebuffer to use 32bpp unconditionally
turns the code bits which handle 8 and 16 bpp into dead code.
Remove them.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 ui/vnc-enc-hextile.c |   32 -
 ui/vnc-enc-tight.c   |   64 +-
 ui/vnc.c |   18 --
 3 files changed, 27 insertions(+), 87 deletions(-)

diff --git a/ui/vnc-enc-hextile.c b/ui/vnc-enc-hextile.c
index 263a0ce..2e768fd 100644
--- a/ui/vnc-enc-hextile.c
+++ b/ui/vnc-enc-hextile.c
@@ -32,31 +32,11 @@ static void hextile_enc_cord(uint8_t *ptr, int x, int y, 
int w, int h)
 ptr[1] = (((w - 1)  0x0F)  4) | ((h - 1)  0x0F);
 }
 
-#define BPP 8
-#include vnc-enc-hextile-template.h
-#undef BPP
-
-#define BPP 16
-#include vnc-enc-hextile-template.h
-#undef BPP
-
 #define BPP 32
 #include vnc-enc-hextile-template.h
 #undef BPP
 
 #define GENERIC
-#define BPP 8
-#include vnc-enc-hextile-template.h
-#undef BPP
-#undef GENERIC
-
-#define GENERIC
-#define BPP 16
-#include vnc-enc-hextile-template.h
-#undef BPP
-#undef GENERIC
-
-#define GENERIC
 #define BPP 32
 #include vnc-enc-hextile-template.h
 #undef BPP
@@ -89,24 +69,12 @@ void vnc_hextile_set_pixel_conversion(VncState *vs, int 
generic)
 {
 if (!generic) {
 switch (VNC_SERVER_FB_BITS) {
-case 8:
-vs-hextile.send_tile = send_hextile_tile_8;
-break;
-case 16:
-vs-hextile.send_tile = send_hextile_tile_16;
-break;
 case 32:
 vs-hextile.send_tile = send_hextile_tile_32;
 break;
 }
 } else {
 switch (VNC_SERVER_FB_BITS) {
-case 8:
-vs-hextile.send_tile = send_hextile_tile_generic_8;
-break;
-case 16:
-vs-hextile.send_tile = send_hextile_tile_generic_16;
-break;
 case 32:
 vs-hextile.send_tile = send_hextile_tile_generic_32;
 break;
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 9fd2556..9ae4cab 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -671,41 +671,35 @@ DEFINE_GRADIENT_FILTER_FUNCTION(32)
  * that case new color will be stored in *colorPtr.
  */
 
-#define DEFINE_CHECK_SOLID_FUNCTION(bpp)\
-\
-static bool \
-check_solid_tile##bpp(VncState *vs, int x, int y, int w, int h, \
-  uint32_t* color, bool samecolor)  \
-{   \
-VncDisplay *vd = vs-vd;\
-uint##bpp##_t *fbptr;   \
-uint##bpp##_t c;\
-int dx, dy; \
-\
-fbptr = vnc_server_fb_ptr(vd, x, y);\
-\
-c = *fbptr; \
-if (samecolor  (uint32_t)c != *color) {   \
-return false;   \
-}   \
-\
-for (dy = 0; dy  h; dy++) {\
-for (dx = 0; dx  w; dx++) {\
-if (c != fbptr[dx]) {   \
-return false;   \
-}   \
-}   \
-fbptr = (uint##bpp##_t *)   \
-((uint8_t *)fbptr + vnc_server_fb_stride(vd));  \
-}   \
-\
-*color = (uint32_t)c;   \
-return true;\
+static bool
+check_solid_tile32(VncState *vs, int x, int y, int w, int h,
+   uint32_t *color, bool samecolor)
+{
+VncDisplay *vd = vs-vd;
+uint32_t *fbptr;
+uint32_t c;
+int dx, dy;
+
+fbptr = vnc_server_fb_ptr(vd, x, y);
+
+c = *fbptr;
+if (samecolor  (uint32_t)c != *color) {
+return false;
 }
 
-DEFINE_CHECK_SOLID_FUNCTION(32)
-DEFINE_CHECK_SOLID_FUNCTION(16)
-DEFINE_CHECK_SOLID_FUNCTION(8)
+for (dy = 0; dy  h; dy++) {
+

[Qemu-devel] [PATCH 03/14] pixman: windup in configure makefiles

2012-10-17 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 Makefile  |9 +
 configure |   38 ++
 2 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index a9c22bf..5699101 100644
--- a/Makefile
+++ b/Makefile
@@ -105,6 +105,15 @@ endif
 
 subdir-libcacard: $(oslib-obj-y) $(trace-obj-y) qemu-timer-common.o
 
+subdir-pixman: pixman/Makefile
+   $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C pixman V=$(V) 
all,)
+
+pixman/Makefile: $(SRC_PATH)/pixman/configure
+   (cd pixman; $(SRC_PATH)/pixman/configure --disable-shared 
--enable-static)
+
+$(SRC_PATH)/pixman/configure:
+   (cd $(SRC_PATH)/pixman; autoreconf -v --install)
+
 $(filter %-softmmu,$(SUBDIR_RULES)): $(universal-obj-y) $(trace-obj-y) 
$(common-obj-y) $(extra-obj-y) subdir-libdis
 
 $(filter %-user,$(SUBDIR_RULES)): $(universal-obj-y) $(trace-obj-y) 
subdir-libdis-user subdir-libuser
diff --git a/configure b/configure
index 353d788..4b916aa 100755
--- a/configure
+++ b/configure
@@ -147,6 +147,7 @@ curses=
 docs=
 fdt=
 nptl=
+pixman=
 sdl=
 virtfs=
 vnc=yes
@@ -642,6 +643,10 @@ for opt do
 # configure to be used by RPM and similar macros that set
 # lots of directory switches by default.
   ;;
+  --pixman-system) pixman=system
+  ;;
+  --pixman-internal) pixman=internal
+  ;;
   --disable-sdl) sdl=no
   ;;
   --enable-sdl) sdl=yes
@@ -2117,6 +2122,34 @@ else
 fi
 
 ##
+# pixman support probe
+
+if test $pixman = ; then
+  if $pkg_config pixman-1  /dev/null 21; then
+pixman=system
+  else
+pixman=internal
+  fi
+fi
+if test $pixman = system; then
+  pixman_cflags=`$pkg_config --cflags pixman-1 2/dev/null`
+  pixman_libs=`$pkg_config --libs pixman-1 2/dev/null`
+else
+  if test ! -d ${source_path}/pixman/pixman; then
+echo ERROR: pixman not present. Your options:
+echo   (1) Prefered: Install the pixman devel package (any recent
+echo   distro should have packages as Xorg needs pixman too).
+echo   (2) Fetch the pixman submodule, using:
+echo   git submodule update --init pixman
+exit 1
+  fi
+  pixman_cflags=-I${source_path}/pixman/pixman
+  pixman_libs=-Lpixman/pixman/.libs -lpixman-1
+fi
+QEMU_CFLAGS=$QEMU_CFLAGS $pixman_cflags
+libs_softmmu=$libs_softmmu $pixman_libs
+
+##
 # libcap probe
 
 if test $cap != no ; then
@@ -3147,6 +3180,7 @@ echo -Werror enabled   $werror
 if test $darwin = yes ; then
 echo Cocoa support $cocoa
 fi
+echo pixman$pixman
 echo SDL support   $sdl
 echo curses support$curses
 echo curl support  $curl
@@ -3909,6 +3943,9 @@ if test $target_softmmu = yes ; then
   if test $smartcard_nss = yes ; then
 echo subdir-$target: subdir-libcacard  $config_host_mak
   fi
+  if test $pixman = internal ; then
+echo subdir-$target: subdir-pixman  $config_host_mak
+  fi
   case $target_arch2 in
 i386|x86_64)
   echo CONFIG_HAVE_CORE_DUMP=y  $config_target_mak
@@ -4112,6 +4149,7 @@ DIRS=$DIRS pc-bios/optionrom pc-bios/spapr-rtas
 DIRS=$DIRS roms/seabios roms/vgabios
 DIRS=$DIRS qapi-generated
 DIRS=$DIRS libcacard libcacard/libcacard libcacard/trace
+DIRS=$DIRS pixman
 FILES=Makefile tests/tcg/Makefile qdict-test-data.txt
 FILES=$FILES tests/tcg/cris/Makefile tests/tcg/cris/.gdbinit
 FILES=$FILES tests/tcg/lm32/Makefile libcacard/Makefile
-- 
1.7.1




[Qemu-devel] [PATCH 08/14] qxl: stop direct access to DisplaySurface fields.

2012-10-17 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 hw/qxl-render.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index 47eb8b4..98ecb21 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -24,7 +24,7 @@
 static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
 {
 uint8_t *src;
-uint8_t *dst = qxl-vga.ds-surface-data;
+uint8_t *dst = ds_get_data(qxl-vga.ds);
 int len, i;
 
 if (is_buffer_shared(qxl-vga.ds-surface)) {
-- 
1.7.1




[Qemu-devel] [PATCHv2 1/2] signal: added a wrapper for sigprocmask function

2012-10-17 Thread Alex Barcelo
Create a wrapper for signal mask changes initiated by the guest;
this will give us a place to put code which prevents the guest
from changing the handling of signals used by QEMU itself
internally.

The wrapper is called from all the guest-initiated sigprocmask, but
is not called from internal qemu sigprocmask calls.

Signed-off-by: Alex Barcelo abarc...@ac.upc.edu
---
 linux-user/qemu.h|1 +
 linux-user/signal.c  |   10 ++
 linux-user/syscall.c |   14 +++---
 3 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index fc4cc00..e2dd6a6 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -237,6 +237,7 @@ int host_to_target_signal(int sig);
 long do_sigreturn(CPUArchState *env);
 long do_rt_sigreturn(CPUArchState *env);
 abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr, abi_ulong sp);
+int do_sigprocmask(int how, const sigset_t *set, sigset_t *oldset);
 
 #ifdef TARGET_I386
 /* vm86.c */
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 15bc4e8..3d25b7d 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -5461,6 +5461,16 @@ long do_rt_sigreturn(CPUArchState *env)
 
 #endif
 
+/* Wrapper for sigprocmask function
+ * Emulates a sigprocmask in a safe way for the guest. Note that set and oldset
+ * are host signal set, not guest ones. This wraps the sigprocmask host calls
+ * that should be protected (calls originated from guest)
+ */
+int do_sigprocmask(int how, const sigset_t *set, sigset_t *oldset)
+{
+return sigprocmask(how, set, oldset);
+}
+
 void process_pending_signals(CPUArchState *cpu_env)
 {
 int sig;
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 471d060..49c0beb 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5875,7 +5875,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 {
 sigset_t cur_set;
 abi_ulong target_set;
-sigprocmask(0, NULL, cur_set);
+do_sigprocmask(0, NULL, cur_set);
 host_to_target_old_sigset(target_set, cur_set);
 ret = target_set;
 }
@@ -5886,10 +5886,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 {
 sigset_t set, oset, cur_set;
 abi_ulong target_set = arg1;
-sigprocmask(0, NULL, cur_set);
+do_sigprocmask(0, NULL, cur_set);
 target_to_host_old_sigset(set, target_set);
 sigorset(set, set, cur_set);
-sigprocmask(SIG_SETMASK, set, oset);
+do_sigprocmask(SIG_SETMASK, set, oset);
 host_to_target_old_sigset(target_set, oset);
 ret = target_set;
 }
@@ -5920,7 +5920,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 mask = arg2;
 target_to_host_old_sigset(set, mask);
 
-ret = get_errno(sigprocmask(how, set, oldset));
+ret = get_errno(do_sigprocmask(how, set, oldset));
 if (!is_error(ret)) {
 host_to_target_old_sigset(mask, oldset);
 ret = mask;
@@ -5954,7 +5954,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 how = 0;
 set_ptr = NULL;
 }
-ret = get_errno(sigprocmask(how, set_ptr, oldset));
+ret = get_errno(do_sigprocmask(how, set_ptr, oldset));
 if (!is_error(ret)  arg3) {
 if (!(p = lock_user(VERIFY_WRITE, arg3, 
sizeof(target_sigset_t), 0)))
 goto efault;
@@ -5994,7 +5994,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 how = 0;
 set_ptr = NULL;
 }
-ret = get_errno(sigprocmask(how, set_ptr, oldset));
+ret = get_errno(do_sigprocmask(how, set_ptr, oldset));
 if (!is_error(ret)  arg3) {
 if (!(p = lock_user(VERIFY_WRITE, arg3, 
sizeof(target_sigset_t), 0)))
 goto efault;
@@ -7870,7 +7870,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 }
 mask = arg2;
 target_to_host_old_sigset(set, mask);
-sigprocmask(how, set, oldset);
+do_sigprocmask(how, set, oldset);
 host_to_target_old_sigset(mask, oldset);
 ret = mask;
 }
-- 
1.7.5.4




[Qemu-devel] [PATCHv2 0/2] Preparing safe sigprocmask wrapper on qemu-user

2012-10-17 Thread Alex Barcelo
qemu-user needs SIGSEGV (at least) for some internal use. If the guest
application masks it and does unsafe sigprocmask, then the application
crashes. Problems happen in applications with self-modifying code (who
also change the signal mask). Other guest applications may have related
problems if they use the SIGSEGV.

A way to be more safe is adding a wrapper for all sigprocmask calls from
the guest. The wrapper proposed here is quite simple, but the code can
be improved, here I try to ensure that the wrapper is set up properly.

Here, a test case where qemu-user goes wrong:


#include stdio.h
#include stdlib.h
#include string.h
#include sys/mman.h
#include malloc.h
#include signal.h

unsigned char *testfun;

int main ( void )
{
unsigned int ra;
testfun=memalign(getpagesize(),1024);
// We block the SIGSEGV signal, used by qemu-user
sigset_t set;
sigemptyset(set);
sigaddset(set, 11);
sigprocmask(SIG_BLOCK, set, NULL);
mprotect(testfun, 1024, PROT_READ|PROT_EXEC|PROT_WRITE);

//400687: b8 0d 00 00 00  mov$0xd,%eax
//40068d: c3  retq
testfun[ 0]=0xb8;
testfun[ 1]=0x0d;
testfun[ 2]=0x00;
testfun[ 3]=0x00;
testfun[ 4]=0x00;
testfun[ 5]=0xc3;
printf ( 0x%02X\n,
 ((unsigned int (*)())testfun)() );

//400687: b8 20 00 00 00  mov$0x20,%eax
//40068d: c3  retq
// This self-modifying code will break because of the sigsegv signal block
testfun[ 1]=0x20;
printf ( 0x%02X\n,
 ((unsigned int (*)())testfun)() );
}


On an i386 native host:
0x0D
0x20

On a non-patched qemu-i386:
0x0D
Segmentation fault

Alex Barcelo (2):
  signal: added a wrapper for sigprocmask function
  signal: sigsegv protection on do_sigprocmask

 linux-user/qemu.h|1 +
 linux-user/signal.c  |   27 +++
 linux-user/syscall.c |   14 +++---
 3 files changed, 35 insertions(+), 7 deletions(-)

-- 
1.7.5.4




[Qemu-devel] [PATCHv2 2/2] signal: sigsegv protection on do_sigprocmask

2012-10-17 Thread Alex Barcelo
Create a safe wrapper by protecting the signal mask.

Instead of doing a simple passthrough of the sigprocmask, the wrapper
manipulates the signal mask in a safe way for the qemu internal. This
is done by avoiding SIGSEGV bit mask manipulation from the guest.

We also return the same bit on the SIGSEGV. This is not required for
most applications, but if the application checks it, then it will see
that somethings fishy about it (and, in fact, maybe it should). If we
do not want the guest to be aware of those manipulations, then it should
be implemented in another way, but this seems quite clean and consistent.

The wrapper can be improved to add more features for better signal
managing, but this seems enough for simple self-modifying code.

Signed-off-by: Alex Barcelo abarc...@ac.upc.edu
---
 linux-user/signal.c |   19 ++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 3d25b7d..b965d89 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -5468,7 +5468,24 @@ long do_rt_sigreturn(CPUArchState *env)
  */
 int do_sigprocmask(int how, const sigset_t *set, sigset_t *oldset)
 {
-return sigprocmask(how, set, oldset);
+int ret;
+sigset_t val;
+sigset_t *temp;
+if (set) {
+val = *set;
+temp = val;
+sigdelset(temp, SIGSEGV);
+} else {
+temp = NULL;
+}
+ret = sigprocmask(how, temp, oldset);
+
+/* Force set state of SIGSEGV, may be best for some apps, maybe not so good
+ * This is not required for qemu to work */
+if (oldset) {
+sigaddset(oldset, SIGSEGV);
+}
+return ret;
 }
 
 void process_pending_signals(CPUArchState *cpu_env)
-- 
1.7.5.4




[Qemu-devel] [PATCH 00/14] pixman patch series.

2012-10-17 Thread Gerd Hoffmann
  Hi,

First round of pixman patches for qemu, so we have something concrete to
discuss.  Applies on top of the console cleanups patch series posted a
few days ago.

The long-term goal is to stop reinventing the wheel over and over when
it comes to pixel manipulation and to use pixman instead.  This patch
series is a start.  It switches a central data structure
(DisplaySurface) over to pixman, so the ui code can easily use pixman
to manipulate pixel data.  It also collects some low-hanging fruit to
show pixman in action.

Comments?

cheers,
  Gerd

Gerd Hoffmann (14):
  console: remove DisplayAllocator
  pixman: add submodule
  pixman: windup in configure  makefiles
  pixman: helper functions
  pixman: add pixman image to DisplaySurface
  console: make qemu_alloc_display static
  console: don't set PixelFormat alpha fields for 32bpp
  qxl: stop direct access to DisplaySurface fields.
  vga: stop direct access to DisplaySurface fields.
  pixman: switch screendump function.
  pixman/vnc: use pixman images in vnc.
  pixman/vnc: remove rgb_prepare_row* functions
  pixman/vnc: remove dead code.
  pixman: drop obsolete fields from DisplaySurface

 .gitmodules   |3 +
 Makefile  |9 ++
 Makefile.objs |1 +
 configure |   38 ++
 console.c |  107 +++-
 console.h |   62 --
 hw/qxl-render.c   |2 +-
 hw/vga.c  |   48 +++-
 pixman|1 +
 qemu-common.h |1 -
 qemu-pixman.c |   60 +
 qemu-pixman.h |   32 +
 ui/sdl.c  |  117 +
 ui/vnc-enc-hextile-template.h |   23 ++--
 ui/vnc-enc-hextile.c  |   53 ++---
 ui/vnc-enc-tight.c|  280 +++--
 ui/vnc-enc-zrle.c |   18 ++--
 ui/vnc-jobs.c |3 +-
 ui/vnc.c  |  239 ++-
 ui/vnc.h  |   19 +++-
 20 files changed, 508 insertions(+), 608 deletions(-)
 create mode 16 pixman
 create mode 100644 qemu-pixman.c
 create mode 100644 qemu-pixman.h




Re: [Qemu-devel] [PATCH 0/2] signal: Preparing safe sigprocmask wrapper on qemu-user

2012-10-17 Thread Alex Barcelo
Thanks Peter for all the feedback! I have just sent the v2 (have been
on holiday this weekend, a bit offline). I hope that this one is a
better patch.

Sorry Riku, I have doubleposted the new patch to you, I slipped on the
git send-email command, and sent the patch only to you at first (not
to the list). I corrected it but now you may have received twice. Next
time I will double check my git output :(

On Wed, Oct 10, 2012 at 5:37 PM, Peter Maydell peter.mayd...@linaro.org wrote:
 On 8 October 2012 19:42, Alex Barcelo abarc...@ac.upc.edu wrote:
 okay, now I see that this lacks a lot of presentation.

 Before sending a v2, is there something more that I should correct?

 Well, yes, your cover letter could be a little more verbose, but I
 think mostly it's just that nobody's got round to reviewing the
 patches yet. I'll have a look at them in a moment.

 One thing that is definitely missing and is critical is that
 you need to include a Signed-off-by: line in your patches'
 commit messages: we cannot commit them without one.
 (http://wiki.qemu.org/Contribute/SubmitAPatch mentions this and
 has some other hints, if you haven't read it.)

 And one netiquete question, test-breaking code should be in the
 description (cover, 00/00)? On a next post in the same thread? Another
 thread?

 In the cover letter is fine.

 -- PMM



Re: [Qemu-devel] [PATCH 03/14] pixman: windup in configure makefiles

2012-10-17 Thread Paolo Bonzini
Il 17/10/2012 15:29, Gerd Hoffmann ha scritto:
 Signed-off-by: Gerd Hoffmann kra...@redhat.com
 ---
  Makefile  |9 +
  configure |   38 ++
  2 files changed, 47 insertions(+), 0 deletions(-)
 
 diff --git a/Makefile b/Makefile
 index a9c22bf..5699101 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -105,6 +105,15 @@ endif
  
  subdir-libcacard: $(oslib-obj-y) $(trace-obj-y) qemu-timer-common.o
  
 +subdir-pixman: pixman/Makefile
 + $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C pixman V=$(V) 
 all,)
 +
 +pixman/Makefile: $(SRC_PATH)/pixman/configure
 + (cd pixman; $(SRC_PATH)/pixman/configure --disable-shared 
 --enable-static)
 +
 +$(SRC_PATH)/pixman/configure:
 + (cd $(SRC_PATH)/pixman; autoreconf -v --install)
 +
  $(filter %-softmmu,$(SUBDIR_RULES)): $(universal-obj-y) $(trace-obj-y) 
 $(common-obj-y) $(extra-obj-y) subdir-libdis
  
  $(filter %-user,$(SUBDIR_RULES)): $(universal-obj-y) $(trace-obj-y) 
 subdir-libdis-user subdir-libuser
 diff --git a/configure b/configure
 index 353d788..4b916aa 100755
 --- a/configure
 +++ b/configure
 @@ -147,6 +147,7 @@ curses=
  docs=
  fdt=
  nptl=
 +pixman=
  sdl=
  virtfs=
  vnc=yes
 @@ -642,6 +643,10 @@ for opt do
  # configure to be used by RPM and similar macros that set
  # lots of directory switches by default.
;;
 +  --pixman-system) pixman=system
 +  ;;
 +  --pixman-internal) pixman=internal
 +  ;;

Let's make this more autoconfy:
--with-system-pixman/--without-system-pixman.

Paolo

--disable-sdl) sdl=no
;;
--enable-sdl) sdl=yes
 @@ -2117,6 +2122,34 @@ else
  fi
  
  ##
 +# pixman support probe
 +
 +if test $pixman = ; then
 +  if $pkg_config pixman-1  /dev/null 21; then
 +pixman=system
 +  else
 +pixman=internal
 +  fi
 +fi
 +if test $pixman = system; then
 +  pixman_cflags=`$pkg_config --cflags pixman-1 2/dev/null`
 +  pixman_libs=`$pkg_config --libs pixman-1 2/dev/null`
 +else
 +  if test ! -d ${source_path}/pixman/pixman; then
 +echo ERROR: pixman not present. Your options:
 +echo   (1) Prefered: Install the pixman devel package (any recent
 +echo   distro should have packages as Xorg needs pixman too).
 +echo   (2) Fetch the pixman submodule, using:
 +echo   git submodule update --init pixman
 +exit 1
 +  fi
 +  pixman_cflags=-I${source_path}/pixman/pixman
 +  pixman_libs=-Lpixman/pixman/.libs -lpixman-1
 +fi
 +QEMU_CFLAGS=$QEMU_CFLAGS $pixman_cflags
 +libs_softmmu=$libs_softmmu $pixman_libs
 +
 +##
  # libcap probe
  
  if test $cap != no ; then
 @@ -3147,6 +3180,7 @@ echo -Werror enabled   $werror
  if test $darwin = yes ; then
  echo Cocoa support $cocoa
  fi
 +echo pixman$pixman
  echo SDL support   $sdl
  echo curses support$curses
  echo curl support  $curl
 @@ -3909,6 +3943,9 @@ if test $target_softmmu = yes ; then
if test $smartcard_nss = yes ; then
  echo subdir-$target: subdir-libcacard  $config_host_mak
fi
 +  if test $pixman = internal ; then
 +echo subdir-$target: subdir-pixman  $config_host_mak
 +  fi
case $target_arch2 in
  i386|x86_64)
echo CONFIG_HAVE_CORE_DUMP=y  $config_target_mak
 @@ -4112,6 +4149,7 @@ DIRS=$DIRS pc-bios/optionrom pc-bios/spapr-rtas
  DIRS=$DIRS roms/seabios roms/vgabios
  DIRS=$DIRS qapi-generated
  DIRS=$DIRS libcacard libcacard/libcacard libcacard/trace
 +DIRS=$DIRS pixman
  FILES=Makefile tests/tcg/Makefile qdict-test-data.txt
  FILES=$FILES tests/tcg/cris/Makefile tests/tcg/cris/.gdbinit
  FILES=$FILES tests/tcg/lm32/Makefile libcacard/Makefile
 




Re: [Qemu-devel] [PATCH 07/25] migration: use qemu-sockets to establish Unix sockets

2012-10-17 Thread Markus Armbruster
Paolo Bonzini pbonz...@redhat.com writes:

 This makes migration-unix.c again a cut-and-paste job from migration-tcp.c,
 exactly as it was in the beginning. :)

Neat!



[Qemu-devel] [PATCH 01/14] console: remove DisplayAllocator

2012-10-17 Thread Gerd Hoffmann
Causes [temporary] preformance regression with 24bpp vga modes @ sdl.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 console.c |   52 +
 console.h |   34 +++--
 qemu-common.h |1 -
 ui/sdl.c  |  117 +++--
 4 files changed, 31 insertions(+), 173 deletions(-)

diff --git a/console.c b/console.c
index 61812c7..71cc543 100644
--- a/console.c
+++ b/console.c
@@ -1294,9 +1294,10 @@ static QemuConsole *new_console(DisplayState *ds, 
console_type_t console_type)
 return s;
 }
 
-static DisplaySurface* defaultallocator_create_displaysurface(int width, int 
height)
+DisplaySurface *qemu_create_displaysurface(DisplayState *ds,
+   int width, int height)
 {
-DisplaySurface *surface = (DisplaySurface*) 
g_malloc0(sizeof(DisplaySurface));
+DisplaySurface *surface = g_new0(DisplaySurface, 1);
 
 int linesize = width * 4;
 qemu_alloc_display(surface, width, height, linesize,
@@ -1304,13 +1305,15 @@ static DisplaySurface* 
defaultallocator_create_displaysurface(int width, int hei
 return surface;
 }
 
-static DisplaySurface* defaultallocator_resize_displaysurface(DisplaySurface 
*surface,
-  int width, int height)
+DisplaySurface *qemu_resize_displaysurface(DisplayState *ds,
+   int width, int height)
 {
 int linesize = width * 4;
-qemu_alloc_display(surface, width, height, linesize,
+
+trace_displaysurface_resize(ds, ds-surface, width, height);
+qemu_alloc_display(ds-surface, width, height, linesize,
qemu_default_pixelformat(32), 0);
-return surface;
+return ds-surface;
 }
 
 void qemu_alloc_display(DisplaySurface *surface, int width, int height,
@@ -1323,7 +1326,7 @@ void qemu_alloc_display(DisplaySurface *surface, int 
width, int height,
 surface-pf = pf;
 if (surface-flags  QEMU_ALLOCATED_FLAG) {
 data = g_realloc(surface-data,
-surface-linesize * surface-height);
+ surface-linesize * surface-height);
 } else {
 data = g_malloc(surface-linesize * surface-height);
 }
@@ -1334,7 +1337,7 @@ void qemu_alloc_display(DisplaySurface *surface, int 
width, int height,
 #endif
 }
 
-DisplaySurface* qemu_create_displaysurface_from(int width, int height, int bpp,
+DisplaySurface *qemu_create_displaysurface_from(int width, int height, int bpp,
   int linesize, uint8_t *data)
 {
 DisplaySurface *surface = (DisplaySurface*) 
g_malloc0(sizeof(DisplaySurface));
@@ -1351,28 +1354,24 @@ DisplaySurface* qemu_create_displaysurface_from(int 
width, int height, int bpp,
 return surface;
 }
 
-static void defaultallocator_free_displaysurface(DisplaySurface *surface)
+void qemu_free_displaysurface(DisplayState *ds)
 {
-if (surface == NULL)
+trace_displaysurface_free(ds, ds-surface);
+if (ds-surface == NULL) {
 return;
-if (surface-flags  QEMU_ALLOCATED_FLAG)
-g_free(surface-data);
-g_free(surface);
+}
+if (ds-surface-flags  QEMU_ALLOCATED_FLAG) {
+g_free(ds-surface-data);
+}
+g_free(ds-surface);
 }
 
-static struct DisplayAllocator default_allocator = {
-defaultallocator_create_displaysurface,
-defaultallocator_resize_displaysurface,
-defaultallocator_free_displaysurface
-};
-
 static void dumb_display_init(void)
 {
 DisplayState *ds = g_malloc0(sizeof(DisplayState));
 int width = 640;
 int height = 480;
 
-ds-allocator = default_allocator;
 if (is_fixedsize_console()) {
 width = active_console-g_width;
 height = active_console-g_height;
@@ -1402,18 +1401,6 @@ DisplayState *get_displaystate(void)
 return display_state;
 }
 
-DisplayAllocator *register_displayallocator(DisplayState *ds, DisplayAllocator 
*da)
-{
-if(ds-allocator ==  default_allocator) {
-DisplaySurface *surf;
-surf = da-create_displaysurface(ds_get_width(ds), ds_get_height(ds));
-defaultallocator_free_displaysurface(ds-surface);
-ds-surface = surf;
-ds-allocator = da;
-}
-return ds-allocator;
-}
-
 DisplayState *graphic_console_init(vga_hw_update_ptr update,
vga_hw_invalidate_ptr invalidate,
vga_hw_screen_dump_ptr screen_dump,
@@ -1424,7 +1411,6 @@ DisplayState *graphic_console_init(vga_hw_update_ptr 
update,
 DisplayState *ds;
 
 ds = (DisplayState *) g_malloc0(sizeof(DisplayState));
-ds-allocator = default_allocator; 
 ds-surface = qemu_create_displaysurface(ds, 640, 480);
 
 s = new_console(ds, GRAPHIC_CONSOLE);
diff --git a/console.h b/console.h
index 78e842f..8f13668 100644
--- a/console.h
+++ b/console.h
@@ -107,7 +107,6 @@ void kbd_put_keysym(int keysym);
 
 #define QEMU_BIG_ENDIAN_FLAG

[Qemu-devel] [PATCH 10/14] pixman: switch screendump function.

2012-10-17 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 hw/vga.c |   39 +++
 1 files changed, 11 insertions(+), 28 deletions(-)

diff --git a/hw/vga.c b/hw/vga.c
index 4d34469..4ffd57c 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -2417,13 +2417,12 @@ void vga_init_vbe(VGACommonState *s, MemoryRegion 
*system_memory)
 
 void ppm_save(const char *filename, struct DisplaySurface *ds, Error **errp)
 {
+int width = pixman_image_get_width(ds-image);
+int height = pixman_image_get_height(ds-image);
 FILE *f;
-uint8_t *d, *d1;
-uint32_t v;
-int y, x;
-uint8_t r, g, b;
+int y;
 int ret;
-char *linebuf, *pbuf;
+pixman_image_t *linebuf;
 
 trace_ppm_save(filename, ds);
 f = fopen(filename, wb);
@@ -2432,33 +2431,17 @@ void ppm_save(const char *filename, struct 
DisplaySurface *ds, Error **errp)
strerror(errno));
 return;
 }
-ret = fprintf(f, P6\n%d %d\n%d\n, ds-width, ds-height, 255);
+ret = fprintf(f, P6\n%d %d\n%d\n, width, height, 255);
 if (ret  0) {
 linebuf = NULL;
 goto write_err;
 }
-linebuf = g_malloc(ds-width * 3);
-d1 = ds-data;
-for(y = 0; y  ds-height; y++) {
-d = d1;
-pbuf = linebuf;
-for(x = 0; x  ds-width; x++) {
-if (ds-pf.bits_per_pixel == 32)
-v = *(uint32_t *)d;
-else
-v = (uint32_t) (*(uint16_t *)d);
-/* Limited to 8 or fewer bits per channel: */
-r = ((v  ds-pf.rshift)  ds-pf.rmax)  (8 - ds-pf.rbits);
-g = ((v  ds-pf.gshift)  ds-pf.gmax)  (8 - ds-pf.gbits);
-b = ((v  ds-pf.bshift)  ds-pf.bmax)  (8 - ds-pf.bbits);
-*pbuf++ = r;
-*pbuf++ = g;
-*pbuf++ = b;
-d += ds-pf.bytes_per_pixel;
-}
-d1 += ds-linesize;
+linebuf = qemu_pixman_linebuf_create(PIXMAN_BE_r8g8b8, width);
+for (y = 0; y  height; y++) {
+qemu_pixman_linebuf_fill(linebuf, ds-image, width, y);
 clearerr(f);
-ret = fwrite(linebuf, 1, pbuf - linebuf, f);
+ret = fwrite(pixman_image_get_data(linebuf), 1,
+ pixman_image_get_stride(linebuf), f);
 (void)ret;
 if (ferror(f)) {
 goto write_err;
@@ -2466,7 +2449,7 @@ void ppm_save(const char *filename, struct DisplaySurface 
*ds, Error **errp)
 }
 
 out:
-g_free(linebuf);
+qemu_pixman_image_unref(linebuf);
 fclose(f);
 return;
 
-- 
1.7.1




[Qemu-devel] [PATCH 07/14] console: don't set PixelFormat alpha fields for 32bpp

2012-10-17 Thread Gerd Hoffmann
Currently it is inconstent, PixelFormat-amask is left unset whereas
abits and amax and ashift are filled.  As an alpha channel doesn't make
sense for the vga framebuffer leave all alpha fields clear.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 console.c |3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/console.c b/console.c
index 48d88e4..d28b75e 100644
--- a/console.c
+++ b/console.c
@@ -1715,18 +1715,15 @@ PixelFormat qemu_default_pixelformat(int bpp)
 pf.rmask = 0x00FF;
 pf.gmask = 0xFF00;
 pf.bmask = 0x00FF;
-pf.amax = 255;
 pf.rmax = 255;
 pf.gmax = 255;
 pf.bmax = 255;
-pf.ashift = 24;
 pf.rshift = 16;
 pf.gshift = 8;
 pf.bshift = 0;
 pf.rbits = 8;
 pf.gbits = 8;
 pf.bbits = 8;
-pf.abits = 8;
 break;
 default:
 break;
-- 
1.7.1




[Qemu-devel] [PATCH 05/14] pixman: add pixman image to DisplaySurface

2012-10-17 Thread Gerd Hoffmann
Surfaces are now allocated using pixman.  DisplaySurface gets new
struct fields with pixman image and data.  DisplayChangeListeners
can easily start using pixman now.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 console.c |   37 -
 console.h |3 +++
 2 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/console.c b/console.c
index 71cc543..5e1c5f5 100644
--- a/console.c
+++ b/console.c
@@ -1319,18 +1319,23 @@ DisplaySurface *qemu_resize_displaysurface(DisplayState 
*ds,
 void qemu_alloc_display(DisplaySurface *surface, int width, int height,
 int linesize, PixelFormat pf, int newflags)
 {
-void *data;
 surface-width = width;
 surface-height = height;
 surface-linesize = linesize;
 surface-pf = pf;
-if (surface-flags  QEMU_ALLOCATED_FLAG) {
-data = g_realloc(surface-data,
- surface-linesize * surface-height);
-} else {
-data = g_malloc(surface-linesize * surface-height);
-}
-surface-data = (uint8_t *)data;
+
+qemu_pixman_image_unref(surface-image);
+surface-image = NULL;
+surface-data = NULL;
+
+surface-format = qemu_pixman_get_format(pf);
+assert(surface-format != 0);
+surface-image = pixman_image_create_bits(surface-format,
+  width, height,
+  NULL, linesize);
+assert(surface-image != NULL);
+
+surface-data = (uint8_t *)pixman_image_get_data(surface-image);
 surface-flags = newflags | QEMU_ALLOCATED_FLAG;
 #ifdef HOST_WORDS_BIGENDIAN
 surface-flags |= QEMU_BIG_ENDIAN_FLAG;
@@ -1338,14 +1343,22 @@ void qemu_alloc_display(DisplaySurface *surface, int 
width, int height,
 }
 
 DisplaySurface *qemu_create_displaysurface_from(int width, int height, int bpp,
-  int linesize, uint8_t *data)
+int linesize, uint8_t *data)
 {
-DisplaySurface *surface = (DisplaySurface*) 
g_malloc0(sizeof(DisplaySurface));
+DisplaySurface *surface = g_new0(DisplaySurface, 1);
 
 surface-width = width;
 surface-height = height;
 surface-linesize = linesize;
 surface-pf = qemu_default_pixelformat(bpp);
+
+surface-format = qemu_pixman_get_format(surface-pf);
+assert(surface-format != 0);
+surface-image = pixman_image_create_bits(surface-format,
+  width, height,
+  (void *)data, linesize);
+assert(surface-image != NULL);
+
 #ifdef HOST_WORDS_BIGENDIAN
 surface-flags = QEMU_BIG_ENDIAN_FLAG;
 #endif
@@ -1360,9 +1373,7 @@ void qemu_free_displaysurface(DisplayState *ds)
 if (ds-surface == NULL) {
 return;
 }
-if (ds-surface-flags  QEMU_ALLOCATED_FLAG) {
-g_free(ds-surface-data);
-}
+qemu_pixman_image_unref(ds-surface-image);
 g_free(ds-surface);
 }
 
diff --git a/console.h b/console.h
index 8f13668..c546e98 100644
--- a/console.h
+++ b/console.h
@@ -2,6 +2,7 @@
 #define CONSOLE_H
 
 #include qemu-char.h
+#include qemu-pixman.h
 #include qdict.h
 #include notify.h
 #include monitor.h
@@ -119,6 +120,8 @@ struct PixelFormat {
 };
 
 struct DisplaySurface {
+pixman_format_code_t format;
+pixman_image_t *image;
 uint8_t flags;
 int width;
 int height;
-- 
1.7.1




Re: [Qemu-devel] [PATCH 14/22] usb: Add packet combining functions

2012-10-17 Thread Hans de Goede

Hi,

On 10/17/2012 01:29 PM, Gerd Hoffmann wrote:

   Hi,


+/*
+ * Process / cancel combined packets, called from
+ * usb_ep_combine_input_packets() / usb_combined_packet_cancel().
+ * Only called for devices which call these functions themselves.
+ */
+int (*handle_combined_data)(USBDevice *dev, USBPacket *p);
+void (*cancel_combined_packet)(USBDevice *dev, USBPacket *p);


Do we really need these?


For handle_combined_data, yes, as usb_ep_combine_input_packets can
cause multiple packets to get submitted, since if a combined packet
ends with a packet, which does not have its short_not_ok flag set,
another packet can be safely pipelined after it. This is not
useful for usb mass storage, but very usefull for usb serial
port converters.

I actually first did not have cancel_combined_packet :) Instead
I had usb_combined_packet_cancel() return a boolean indicating
if it had handled the cancel, or if the caller (which would be
a device's cancel method) needs to handle the cancel itself.

I personally find the cancel_combined_packet callback somewhat
cleaner, esp. since with the return boolean method,
after calling usb_combined_packet_cancel(p) p-combined will be
NULL, so if the device's cancel method needs access to p-combined
to deal with combined packets, things get more difficult.

But if you prefer getting rid of the callback we can do that.


I think it isn't much work for the callers to
do that themself.  Saves them providing a callback.  And makes the code
flow easier to follow by removing a pointless indirection.

For handle_combined_data we probably must make
usb_ep_combine_input_packets return a status code.


Regards,

Hans



[Qemu-devel] [PATCH 11/14] pixman/vnc: use pixman images in vnc.

2012-10-17 Thread Gerd Hoffmann
The vnc code uses *three* DisplaySurfaces:

First is the surface of the actual QemuConsole, usually the guest
screen, but could also be a text console (monitor/serial reachable via
Ctrl-Alt-nr keys).  This is left as-is.

Second is the current server's view of the screen content.  The vnc code
uses this to figure which parts of the guest screen did _really_ change
to reduce the amount of updates sent to the vnc clients.  It is also
used as data source when sending out the updates to the clients.  This
surface gets replaced by a pixman image.  The format changes too,
instead of using the guest screen format we'll use fixed 32bit rgb
framebuffer and convert the pixels on the fly when comparing and
updating the server framebuffer.

Third surface carries the format expected by the vnc client.  That isn't
used to store image data.  This surface is switched to PixelFormat and a
boolean for bigendian byte order.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 ui/vnc-enc-hextile-template.h |   23 ++--
 ui/vnc-enc-hextile.c  |   45 
 ui/vnc-enc-tight.c|  144 -
 ui/vnc-enc-zrle.c |   18 ++--
 ui/vnc-jobs.c |3 +-
 ui/vnc.c  |  235 +++--
 ui/vnc.h  |   19 +++-
 7 files changed, 259 insertions(+), 228 deletions(-)

diff --git a/ui/vnc-enc-hextile-template.h b/ui/vnc-enc-hextile-template.h
index a7310e1..d868d75 100644
--- a/ui/vnc-enc-hextile-template.h
+++ b/ui/vnc-enc-hextile-template.h
@@ -14,7 +14,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs,
  int *has_bg, int *has_fg)
 {
 VncDisplay *vd = vs-vd;
-uint8_t *row = vd-server-data + y * ds_get_linesize(vs-ds) + x * 
ds_get_bytes_per_pixel(vs-ds);
+uint8_t *row = vnc_server_fb_ptr(vd, x, y);
 pixel_t *irow = (pixel_t *)row;
 int j, i;
 pixel_t *last_bg = (pixel_t *)last_bg_;
@@ -25,7 +25,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs,
 int bg_count = 0;
 int fg_count = 0;
 int flags = 0;
-uint8_t data[(vs-clientds.pf.bytes_per_pixel + 2) * 16 * 16];
+uint8_t data[(vs-client_pf.bytes_per_pixel + 2) * 16 * 16];
 int n_data = 0;
 int n_subtiles = 0;
 
@@ -58,7 +58,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs,
}
if (n_colors  2)
break;
-   irow += ds_get_linesize(vs-ds) / sizeof(pixel_t);
+   irow += vnc_server_fb_stride(vd) / sizeof(pixel_t);
 }
 
 if (n_colors  1  fg_count  bg_count) {
@@ -106,7 +106,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs,
n_data += 2;
n_subtiles++;
}
-   irow += ds_get_linesize(vs-ds) / sizeof(pixel_t);
+   irow += vnc_server_fb_stride(vd) / sizeof(pixel_t);
}
break;
 case 3:
@@ -133,7 +133,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs,
has_color = 0;
 #ifdef GENERIC
 vnc_convert_pixel(vs, data + n_data, color);
-n_data += vs-clientds.pf.bytes_per_pixel;
+n_data += vs-client_pf.bytes_per_pixel;
 #else
memcpy(data + n_data, color, sizeof(color));
 n_data += sizeof(pixel_t);
@@ -153,7 +153,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs,
if (has_color) {
 #ifdef GENERIC
 vnc_convert_pixel(vs, data + n_data, color);
-n_data += vs-clientds.pf.bytes_per_pixel;
+n_data += vs-client_pf.bytes_per_pixel;
 #else
 memcpy(data + n_data, color, sizeof(color));
 n_data += sizeof(pixel_t);
@@ -162,7 +162,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs,
n_data += 2;
n_subtiles++;
}
-   irow += ds_get_linesize(vs-ds) / sizeof(pixel_t);
+   irow += vnc_server_fb_stride(vd) / sizeof(pixel_t);
}
 
/* A SubrectsColoured subtile invalidates the foreground color */
@@ -190,18 +190,17 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs,
 vnc_write_u8(vs, flags);
 if (n_colors  4) {
if (flags  0x02)
-   vs-write_pixels(vs, vd-server-pf, last_bg, sizeof(pixel_t));
+   vs-write_pixels(vs, last_bg, sizeof(pixel_t));
if (flags  0x04)
-   vs-write_pixels(vs, vd-server-pf, last_fg, sizeof(pixel_t));
+   vs-write_pixels(vs, last_fg, sizeof(pixel_t));
if (n_subtiles) {
vnc_write_u8(vs, n_subtiles);
vnc_write(vs, data, n_data);
}
 } else {
for (j = 0; j  h; j++) {
-   vs-write_pixels(vs, vd-server-pf, row,
- w * ds_get_bytes_per_pixel(vs-ds));
-   row += ds_get_linesize(vs-ds);
+   vs-write_pixels(vs, row, w * 4);
+   row += 

Re: [Qemu-devel] [PATCH 08/25] migration (outgoing): add error propagation for all protocols

2012-10-17 Thread Markus Armbruster
Paolo Bonzini pbonz...@redhat.com writes:

 Error propagation is already there for socket backends, even though
 it is (and remains) incomplete because no Error is passed to the
 NonBlockingConnectHandler.

Why is that a problem?

 Add it to other protocols, simplifying
 code that tests for errors that will never happen.

 With all protocols understanding Error, the code can be simplified
 further by removing the return value.

 Before:

 (qemu) migrate fd:
 migrate: An undefined error has occurred
 (qemu) info migrate
 (qemu)

 After:

 (qemu) migrate fd:
 migrate: File descriptor named '' has not been found
 (qemu) info migrate
 capabilities: xbzrle: off
 Migration status: failed
 total time: 0 milliseconds

 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
 v1-v2: turn bizarre DPRINTF into an assertion failure or just
 drop it for the failure test of O_NONBLOCK. Clean up after this
 change.

  migration-fd.c   | 19 ---
  migration-tcp.c  | 13 ++---
  migration-unix.c | 11 ++-
  migration.c  | 17 ++---
  migration.h  |  9 -
  6 file modificati, 22 inserzioni(+), 65 rimozioni(-)

 diff --git a/migration-exec.c b/migration-exec.c
 index 6c97db9..5f3f4b2 100644
 --- a/migration-exec.c
 +++ b/migration-exec.c
 @@ -60,22 +60,18 @@ static int exec_close(MigrationState *s)
  return ret;
  }
  
 -int exec_start_outgoing_migration(MigrationState *s, const char *command)
 +void exec_start_outgoing_migration(MigrationState *s, const char *command, 
 Error **errp)
  {
  FILE *f;
  
  f = popen(command, w);
  if (f == NULL) {
 -DPRINTF(Unable to popen exec target\n);
 -goto err_after_popen;
 +error_setg_errno(errp, errno, failed to popen the migration 
 target);
 +return;
  }
  
  s-fd = fileno(f);
 -if (s-fd == -1) {
 -DPRINTF(Unable to retrieve file descriptor for popen'd handle\n);
 -goto err_after_open;
 -}
 -
 +assert(s-fd != -1);

Okay, because fileno() can fail only if its argument is not a valid
stream, which really shouldn't be possible here.

  socket_set_nonblock(s-fd);
  
  s-opaque = qemu_popen(f, w);
 @@ -85,12 +81,6 @@ int exec_start_outgoing_migration(MigrationState *s, const 
 char *command)
  s-write = file_write;
  
  migrate_fd_connect(s);
 -return 0;
 -
 -err_after_open:
 -pclose(f);
 -err_after_popen:
 -return -1;
  }
  
  static void exec_accept_incoming_migration(void *opaque)
 diff --git a/migration-fd.c b/migration-fd.c
 index 7335167..a7c800a 100644
 --- a/migration-fd.c
 +++ b/migration-fd.c
 @@ -73,30 +73,19 @@ static int fd_close(MigrationState *s)
  return 0;
  }
  
 -int fd_start_outgoing_migration(MigrationState *s, const char *fdname)
 +void fd_start_outgoing_migration(MigrationState *s, const char *fdname, 
 Error **errp)
  {
 -s-fd = monitor_get_fd(cur_mon, fdname, NULL);
 +s-fd = monitor_get_fd(cur_mon, fdname, errp);
  if (s-fd == -1) {
 -DPRINTF(fd_migration: invalid file descriptor identifier\n);
 -goto err_after_get_fd;
 -}
 -
 -if (fcntl(s-fd, F_SETFL, O_NONBLOCK) == -1) {
 -DPRINTF(Unable to set nonblocking mode on file descriptor\n);
 -goto err_after_open;
 +return;
  }
  
 +fcntl(s-fd, F_SETFL, O_NONBLOCK);

Sure this can't fail?

  s-get_error = fd_errno;
  s-write = fd_write;
  s-close = fd_close;
  
  migrate_fd_connect(s);
 -return 0;
 -
 -err_after_open:
 -close(s-fd);
 -err_after_get_fd:
 -return -1;
  }
  
  static void fd_accept_incoming_migration(void *opaque)
[...]



Re: [Qemu-devel] [RFC PATCH 0/6] chardev: convert to QOM

2012-10-17 Thread Anthony Liguori
Gerd Hoffmann kra...@redhat.com writes:

   Hi,

 I think the main decision point here is whether we introduce a separate
 chardev_add/chardev_del command or just use the qom-create command that has
 been posted previously.

 Do you have a git tree with this series + qom-create to look at and play
 with?

https://github.com/aliguori/qemu/tree/chardev-qom.1

I haven't tried the qom-new command from Markus but I've also included
the -object option that I posted a while ago.  That seems to work.
Here's an example command line:

$ x86_64-softmmu/qemu-system-x86_64 \
 -object chardev-file,path=foo.txt,label=chr0,id=chr0,realized=on \
 -device isa-serial,index=0,chardev=chr0 -hda ~/images/linux.img \
 -snapshot -enable-kvm

A couple caveats:

- The label/id duplication is ugly.  The need to use label disappears
  though once we switch devices over to using links.

- Some error checking is needed to deal with trying to use an unrealized
  chardev.

- It may still make sense to have a chardev-add operation but I think
  that operation should work in terms of something like qom-new.
  Probably with some special casing to fix the container path to
  /backends/chardev and also to deal with automatically setting label
  based on id.  It may make sense to also always set realized explicitly
  to 'on' in chardev-add.

Regards,

Anthony Liguori


 thanks,
   Gerd




Re: [Qemu-devel] [PATCHv4] qemu-img rebase: use empty string to rebase without backing file

2012-10-17 Thread Kevin Wolf
Am 16.10.2012 14:46, schrieb Alex Bligh:
 This patch allows an empty filename to be passed as the new base image name
 for qemu-img rebase to mean base the image on no backing file (i.e.
 independent of any backing file). According to Eric Blake, qemu-img rebase
 already supports this when '-u' is used; this adds support when -u is not
 used.
 
 Signed-off-by: Alex Bligh a...@alex.org.uk
 ---
  qemu-img.c|   29 +++--
  qemu-img.texi |4 +++-
  2 files changed, 22 insertions(+), 11 deletions(-)
 
 Also obtainable from:
   https://github.com/abligh/qemu.git
 Commit at:
   
 https://github.com/abligh/qemu/commit/982ec105018f9901619663642de9d0a5de86799d
 
 diff --git a/qemu-img.c b/qemu-img.c
 index f17f187..2816f37 100644
 --- a/qemu-img.c
 +++ b/qemu-img.c
 @@ -1558,13 +1558,15 @@ static int img_rebase(int argc, char **argv)
  error_report(Could not open old backing file '%s', 
 backing_name);
  goto out;
  }
 -
 -bs_new_backing = bdrv_new(new_backing);
 -ret = bdrv_open(bs_new_backing, out_baseimg, BDRV_O_FLAGS,
 +if (out_baseimg[0]) {
 +bs_new_backing = bdrv_new(new_backing);
 +ret = bdrv_open(bs_new_backing, out_baseimg, BDRV_O_FLAGS,
  new_backing_drv);
 -if (ret) {
 -error_report(Could not open new backing file '%s', 
 out_baseimg);
 -goto out;
 +if (ret) {
 +error_report(Could not open new backing file '%s',
 + out_baseimg);
 +goto out;
 +}
  }
  }
  
 @@ -1580,7 +1582,7 @@ static int img_rebase(int argc, char **argv)
  if (!unsafe) {
  uint64_t num_sectors;
  uint64_t old_backing_num_sectors;
 -uint64_t new_backing_num_sectors;
 +uint64_t new_backing_num_sectors = 0;
  uint64_t sector;
  int n;
  uint8_t * buf_old;
 @@ -1592,7 +1594,9 @@ static int img_rebase(int argc, char **argv)
  
  bdrv_get_geometry(bs, num_sectors);
  bdrv_get_geometry(bs_old_backing, old_backing_num_sectors);
 -bdrv_get_geometry(bs_new_backing, new_backing_num_sectors);
 +if (bs_new_backing) {
 +bdrv_get_geometry(bs_new_backing, new_backing_num_sectors);
 +}
  
  local_progress = (float)100 /
  (num_sectors / MIN(num_sectors, IO_BUF_SIZE / 512));
 @@ -1629,7 +1633,7 @@ static int img_rebase(int argc, char **argv)
  }
  }
  
 -if (sector = new_backing_num_sectors) {
 +if (sector = new_backing_num_sectors || !bs_new_backing) {
  memset(buf_new, 0, n * BDRV_SECTOR_SIZE);
  } else {
  if (sector + n  new_backing_num_sectors) {
 @@ -1675,7 +1679,12 @@ static int img_rebase(int argc, char **argv)
   * backing file are overwritten in the COW file now, so the visible 
 content
   * doesn't change when we switch the backing file.
   */
 -ret = bdrv_change_backing_file(bs, out_baseimg, out_basefmt);
 +if (bs_new_backing) {

I think this needs to be out_baseimg, otherwise -u is broken. I've
updated the patch, please check if you agree with the fix.

Kevin



Re: [Qemu-devel] [PATCH v3 0/2] qemu-img: Add --backing-chain option to info command

2012-10-17 Thread Kevin Wolf
Am 17.10.2012 14:02, schrieb Stefan Hajnoczi:
 This series adds the --backing-chain option for enumerating the backing file
 chain.  Given the topmost image it will print qemu-img info information for
 each image file in the chain.
 
 Special care needs to be taken when image files form an infinite loop.  This 
 is
 very unusual, most like due to malicious image files.  Nevertheless, qemu-img
 must be robust against invalid inputs so we explicit check for this.
 
 v3:
  * Use ImageInfoList to avoid manually outputting JSON and to make the code
more QAPI-friendly.
  * Solve the output vs error messages issues by printing a detailed error
message but no image info.  To dig into a broken image chain, rerun without
--backing-chain.  This is different from what I initially tried but is much
cleaner because it doesn't produce confusing output.
  * rm -f $TEST_IMG.[123].base [Kevin]
  * Include --output=json in test case [Kevin]
  * Rename test case to free 043 number [Eric]
 
 Stefan Hajnoczi (2):
   qemu-img: Add --backing-chain option to info command
   qemu-iotests: Add 043 backing file chain infinite loop test
 
  qemu-img.c   | 167 
 +++
  tests/qemu-iotests/043   |  94 
  tests/qemu-iotests/043.out   |  66 +
  tests/qemu-iotests/common.rc |  10 +++
  tests/qemu-iotests/group |   1 +
  5 files changed, 324 insertions(+), 14 deletions(-)
  create mode 100755 tests/qemu-iotests/043
  create mode 100644 tests/qemu-iotests/043.out

Thanks applied both to the block branch.

Kevin



Re: [Qemu-devel] [PATCH] Memory-API: Make eventfd adhere to device endianness

2012-10-17 Thread Avi Kivity
On 10/15/2012 08:30 PM, Alexander Graf wrote:
 Our memory API MMIO regions know the concept of device endianness. This
 is used to automatically swap endianness between devices and host CPU,
 depending on whether buses in between would swizzle the bits.
 
 The ioeventfd value comparison does not adhere to that semantic though.
 Probably because nobody has been running ioeventfd on a BE platform and
 the only device implementing ioeventfd right now is LE (PCI) based.
 
 So add swizzling to ioeventfd registration / deletion to make the rest
 of the code as consistent as possible.
 
 Thanks a lot to Michael Tsirkin to point me towards the right direction.

Thanks, applied.


-- 
error compiling committee.c: too many arguments to function



Re: [Qemu-devel] [PATCH 08/25] migration (outgoing): add error propagation for all protocols

2012-10-17 Thread Paolo Bonzini
Il 17/10/2012 16:43, Markus Armbruster ha scritto:
  Error propagation is already there for socket backends, even though
  it is (and remains) incomplete because no Error is passed to the
  NonBlockingConnectHandler.
 Why is that a problem?

It means that the exact error message still cannot be sent to the user
if the OS reports it asynchronously via SO_ERROR.  If
NonBlockingConnectHandler received an Error**, we could for example
report the error class and/or message via a new field of the
query-migration command even if it is reported asynchronously.

Paolo




Re: [Qemu-devel] [PATCH 11/25] nbd: ask and print error information from qemu-sockets

2012-10-17 Thread Markus Armbruster
Paolo Bonzini pbonz...@redhat.com writes:

 Before:

 $ qemu-system-x86_64 nbd:localhost:12345
 inet_connect_opts: connect(ipv4,yakj.usersys.redhat.com,127.0.0.1,12345): 
 Connection refused
 qemu-system-x86_64: could not open disk image nbd:localhost:12345: 
 Connection refused

 After:

 $ x86_64-softmmu/qemu-system-x86_64 nbd:localhost:12345
 qemu-system-x86_64: Failed to connect to socket: Connection refused
 qemu-system-x86_64: could not open disk image nbd:localhost:12345: 
 Connection refused

 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 ---
  nbd.c | 39 +++
  1 file modificato, 31 inserzioni(+), 8 rimozioni(-)

 diff --git a/nbd.c b/nbd.c
 index f61a288..cec5a94 100644
 --- a/nbd.c
 +++ b/nbd.c
 @@ -208,7 +208,14 @@ int tcp_socket_outgoing(const char *address, uint16_t 
 port)
  
  int tcp_socket_outgoing_spec(const char *address_and_port)
  {
 -return inet_connect(address_and_port, NULL);
 +Error *local_err = NULL;
 +int fd = inet_connect(address_and_port, local_err);
 +
 +if (local_err != NULL) {
 +qerror_report_err(local_err);
 +error_free(local_err);
 +}
 +return fd;
  }
  
  int tcp_socket_incoming(const char *address, uint16_t port)
 @@ -220,22 +227,38 @@ int tcp_socket_incoming(const char *address, uint16_t 
 port)
  
  int tcp_socket_incoming_spec(const char *address_and_port)
  {
 -char *ostr  = NULL;
 -int olen = 0;
 -return inet_listen(address_and_port, ostr, olen, SOCK_STREAM, 0, NULL);
 +Error *local_err = NULL;
 +int fd = inet_listen(address_and_port, NULL, 0, SOCK_STREAM, 0, 
 local_err);
 +
 +if (local_err != NULL) {
 +qerror_report_err(local_err);
 +error_free(local_err);
 +}
 +return fd;
  }
  
  int unix_socket_incoming(const char *path)
  {
 -char *ostr = NULL;
 -int olen = 0;
 +Error *local_err = NULL;
 +int fd = unix_listen(path, NULL, 0, local_err);
  
 -return unix_listen(path, ostr, olen, NULL);
 +if (local_err != NULL) {
 +qerror_report_err(local_err);
 +error_free(local_err);
 +}
 +return fd;
  }
  
  int unix_socket_outgoing(const char *path)
  {
 -return unix_connect(path, NULL);
 +Error *local_err = NULL;
 +int fd = unix_connect(path, local_err);
 +
 +if (local_err != NULL) {
 +qerror_report_err(local_err);
 +error_free(local_err);
 +}
 +return fd;
  }
  
  /* Basic flow for negotiation

The boilerplate pattern

Error *local_err = NULL;
FOO(..., *local_err);
if (local_err) {
qerror_report_err(local_err);
error_free(local_err);
}

is spreading.  Not quite sure it's worth a macro.



Re: [Qemu-devel] [PATCH v3 0/2] qemu-img: Add --backing-chain option to info command

2012-10-17 Thread Eric Blake
On 10/17/2012 08:47 AM, Kevin Wolf wrote:
 Am 17.10.2012 14:02, schrieb Stefan Hajnoczi:
 This series adds the --backing-chain option for enumerating the backing file
 chain.  Given the topmost image it will print qemu-img info information for
 each image file in the chain.

 
 Thanks applied both to the block branch.

Now we also need the documentation:
https://lists.gnu.org/archive/html/qemu-devel/2012-10/msg02360.html

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 1/2] vnc: fix segfault due to failed handshake

2012-10-17 Thread Andreas Färber
Am 14.10.2012 15:08, schrieb Tim Hardeck:
 When the VNC server disconnects due to a failed handshake we don't have
 vs-bh allocated yet.
 
 Check for this case and don't delete it.
 
 Signed-off-by: Tim Hardeck thard...@suse.de
 ---
  ui/vnc.c |4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/ui/vnc.c b/ui/vnc.c
 index 01b2daf..656895a 100644
 --- a/ui/vnc.c
 +++ b/ui/vnc.c
 @@ -1055,7 +1055,9 @@ static void vnc_disconnect_finish(VncState *vs)
  vnc_unlock_output(vs);
  
  qemu_mutex_destroy(vs-output_mutex);
 -qemu_bh_delete(vs-bh);
 +if (vs-bh != NULL) {
 +qemu_bh_delete(vs-bh);
 +}
  buffer_free(vs-jobs_buffer);
  
  for (i = 0; i  VNC_STAT_ROWS; ++i) {

qemu_bh_delete() is not checking for a NULL argument, therefore this fix
looks good to me,

Acked-by: Andreas Färber afaer...@suse.de

Adding some CCs.

As a followup it might be a good idea to either assert or ignore a NULL
argument in qemu_bh_delete().

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH 11/25] nbd: ask and print error information from qemu-sockets

2012-10-17 Thread Paolo Bonzini
Il 17/10/2012 16:51, Markus Armbruster ha scritto:
   /* Basic flow for negotiation
 The boilerplate pattern
 
 Error *local_err = NULL;
 FOO(..., *local_err);
 if (local_err) {
 qerror_report_err(local_err);
 error_free(local_err);
 }
 
 is spreading.  Not quite sure it's worth a macro.

Actually this should not spread, but this one should:

Error *local_err = NULL;
FOO(..., *local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
}

Not quite sure how to macroize it though, at least without making the
code too ugly to see.

Paolo



Re: [Qemu-devel] [PATCH 2/2] qemu queue: fix uninitialized removals

2012-10-17 Thread Andreas Färber
Tim,

Am 14.10.2012 15:08, schrieb Tim Hardeck:
 When calling QTAILQ_REMOVE or QLIST_REMOVE on an unitialized list
 QEMU segfaults.

Can this be reproduced by a user today? Or is this just fixing the case
that a developer forgot to initialize a list?

Regards,
Andreas

 Check for this case specifically on item removal.
 
 Signed-off-by: Tim Hardeck thard...@suse.de
 ---
  qemu-queue.h |8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)
 
 diff --git a/qemu-queue.h b/qemu-queue.h
 index 9288cd8..47ed239 100644
 --- a/qemu-queue.h
 +++ b/qemu-queue.h
 @@ -141,7 +141,9 @@ struct {  
   \
  if ((elm)-field.le_next != NULL)   \
  (elm)-field.le_next-field.le_prev =   \
  (elm)-field.le_prev;   \
 -*(elm)-field.le_prev = (elm)-field.le_next;   \
 +if ((elm)-field.le_prev != NULL) { \
 +*(elm)-field.le_prev = (elm)-field.le_next;   \
 +}   \
  } while (/*CONSTCOND*/0)
  
  #define QLIST_FOREACH(var, head, field) \
 @@ -381,7 +383,9 @@ struct {  
   \
  (elm)-field.tqe_prev;  \
  else\
  (head)-tqh_last = (elm)-field.tqe_prev;   \
 -*(elm)-field.tqe_prev = (elm)-field.tqe_next; \
 +if ((elm)-field.tqe_prev != NULL) {\
 +*(elm)-field.tqe_prev = (elm)-field.tqe_next; \
 +}   \
  } while (/*CONSTCOND*/0)
  
  #define QTAILQ_FOREACH(var, head, field)\


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCHv2 1/2] signal: added a wrapper for sigprocmask function

2012-10-17 Thread Peter Maydell
On 17 October 2012 15:18, Alex Barcelo abarc...@ac.upc.edu wrote:
 Create a wrapper for signal mask changes initiated by the guest;
 this will give us a place to put code which prevents the guest
 from changing the handling of signals used by QEMU itself
 internally.

 The wrapper is called from all the guest-initiated sigprocmask, but
 is not called from internal qemu sigprocmask calls.

 Signed-off-by: Alex Barcelo abarc...@ac.upc.edu

In my comments on v1 of this patch I wrote:
I think all the uses of sigprocmask() in linux-user/signal.c also
need to be do_sigprocmask(), as they are the guest trying to control
its signal mask (via the mask it specifies for running signal handlers,
or the mask it passes back when restoring context on return from a
signal handler).

...but I don't see those changes here.

-- PMM



Re: [Qemu-devel] [PATCH v3 4/4] qemu-config: Add new -add-fd command line option

2012-10-17 Thread Eric Blake
On 10/17/2012 08:02 AM, Kevin Wolf wrote:
 Am 17.10.2012 06:16, schrieb Eric Blake:
 I'm still seeing the corner case of:

 qemu-kvm -add-fd fd=3,set=1 -add-fd fd=4,set=2 4-

 where the dup(3) will populate fd 4 prior to the point where we get to
 process the -add-fd fd=4 command to notice that the user started
 qemu-kvm with fd 4 closed, and thus qemu will silently proceed to use
 the wrong fd.

 On the other hand, I'm not sure if that corner case is worth worrying
 about, or if we just chalk it up to user stupidity (aka libvirt
 programmer stupidity) if they did something like that (most likely,
 because the management app forgot to clear FD_CLOEXEC before exec()ing
 qemu-kvm).
 
 If you specify an FD number that isn't actually open when qemu is
 stared, you can get any FD that qemu opens internally. I think the
 correct answer to this problem is then don't do that.

Overnight, I realized we do have one potential safety valve: we are
guaranteed that any fd inherited by the exec() of qemu-kvm has
FD_CLOEXEC clear, and we also strive to have qemu open/dup all of its
internal fds with FD_CLOEXEC set.  Therefore, it may be worth a sanity
check of fcntl(F_GETFD) to see if FD_CLOEXEC is set, and if so, the user
must have failed to pass in the fd, and we are now looking at a qemu
internal fd, and should therefore report failure.  But I'm not sure if
it's worth the extra code.

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



signature.asc
Description: OpenPGP digital signature


  1   2   3   >