Re: [Qemu-devel] [RFC][PATCH 05/14 v9] Add API to get memory mapping

[HATAYAMA Daisuke]

From: Wen Congyang we...@cn.fujitsu.com
Subject: [RFC][PATCH 05/14 v9] Add API to get memory mapping
Date: Wed, 14 Mar 2012 10:07:48 +0800

 Add API to get all virtual address and physical address mapping.
 If the guest doesn't use paging, the virtual address is equal to the phyical
 address. The virtual address and physical address mapping is for gdb's user, 
 and
 it does not include the memory that is not referenced by the page table. So if
 you want to use crash to anaylze the vmcore, please do not specify -p option.

It's necessary to write the reason why the -p option is not default
explicitly: guest machine in a catastrophic state can have corrupted
memory, which we cannot trust.

Thanks.
HATAYAMA, Daisuke

Re: [Qemu-devel] [RFC][PATCH 11/14 v9] introduce a new monitor command 'dump' to dump guest's memory

[Wen Congyang]

At 03/16/2012 11:23 AM, HATAYAMA Daisuke Wrote:
 From: Wen Congyang we...@cn.fujitsu.com
 Subject: [RFC][PATCH 11/14 v9] introduce a new monitor command 'dump' to dump 
 guest's memory
 Date: Wed, 14 Mar 2012 10:11:35 +0800
 
 +/*
 + * QEMU dump
 + *
 + * Copyright Fujitsu, Corp. 2011
 + *
 
 Now 2012.

On, I forgot to update it.

 
 +/*
 + * calculate phdr_num
 + *
 + * the type of phdr-num is uint16_t, so we should avoid overflow
 
 e_phnum is correct.

Yes
 
 + */
 +s-phdr_num = 1; /* PT_NOTE */
 +if (s-list.num  (1  16) - 2) {
 
 s-list.num  UINT16_MAX is better.
 
 +s-phdr_num += s-list.num;
 +s-have_section = false;
 +} else {
 +s-have_section = true;
 +s-phdr_num = PN_XNUM;
 +
 +/* the type of shdr-sh_info is uint32_t, so we should avoid 
 overflow */
 +if (s-list.num  (1ULL  32) - 2) {
 
 s-list.num  UINT32_MAX is better.
 
 +s-sh_info = 0x;
 
 UINT32_MAX is better. Is it rough around here?
 
 +} else {
 +s-sh_info += s-list.num;
 +}
 +}
 
 Now orders of processings in positive and negative cases for e_phnum
 and sh_info are different. It's better to make them sorted in the same
 order.
 
   if (phdr_num not overflow?) {
 not overflow case;
   } else {
 overflow case;
 if (sh_info not overflow?) {
   not overflow case;
 } else {
   overflow case;
 }
   }
 
 is better.

OK

Thanks
Wen Congyang
 
 Thanks.
 HATAYAMA, Daisuke
 
 

Re: [Qemu-devel] [RFC][PATCH 08/14 v9] target-i386: Add API to write cpu status to core file

[Wen Congyang]

At 03/16/2012 09:48 AM, HATAYAMA Daisuke Wrote:
 From: Wen Congyang we...@cn.fujitsu.com
 Subject: [RFC][PATCH 08/14 v9] target-i386: Add API to write cpu status to 
 core file
 Date: Wed, 14 Mar 2012 10:09:26 +0800
 
 +memset(note, 0, note_size);
 +if (type == 0) {
 +note32 = note;
 +note32-n_namesz = cpu_to_le32(name_size);
 +note32-n_descsz = cpu_to_le32(descsz);
 +note32-n_type = 0;
 +} else {
 +note64 = note;
 +note64-n_namesz = cpu_to_le32(name_size);
 +note64-n_descsz = cpu_to_le32(descsz);
 +note64-n_type = 0;
 +}
 
 Why not give new type for this note information an explicit name?
 Like NT_QEMUCPUSTATE? There might be another type in the future. This
 way there's also a merit that we can know all the existing notes
 relevant to qemu dump by looking at the names in a header file.

Hmm, how to add a new type? Does someont manage this?

Thanks
Wen Congyang

 
 Thanks.
 HATAYAMA, Daisuke
 
 

Re: [Qemu-devel] [RFC][PATCH 05/14 v9] Add API to get memory mapping

[Wen Congyang]

At 03/16/2012 11:52 AM, HATAYAMA Daisuke Wrote:
 From: Wen Congyang we...@cn.fujitsu.com
 Subject: [RFC][PATCH 05/14 v9] Add API to get memory mapping
 Date: Wed, 14 Mar 2012 10:07:48 +0800
 
  }
 +
 +int qemu_get_guest_memory_mapping(MemoryMappingList *list)
 +{
 +CPUState *env;
 +RAMBlock *block;
 +ram_addr_t offset, length;
 +int ret;
 +bool paging_mode;
 +
 +#if defined(CONFIG_HAVE_GET_MEMORY_MAPPING)
 +paging_mode = cpu_paging_enabled(first_cpu);
 +if (paging_mode) {
 +for (env = first_cpu; env != NULL; env = env-next_cpu) {
 +ret = cpu_get_memory_mapping(list, env);
 +if (ret  0) {
 +return -1;
 +}
 +}
 +return 0;
 +}
 +#else
 +return -2;
 +#endif
 
 Is it better to define the below somewhere else?
 
 #ifndef CONFIG_HAVE_GET_MEMORY_MAPPING
 static inline int qemu_get_guest_memory_mapping(MemoryMappingList *list)
 {
   return -2;
 }
 #endif

Yes

Thanks
Wen Congyang

 
 Thanks.
 HATAYAMA, Daisuke
 
 

Re: [Qemu-devel] [RFC][PATCH 05/14 v9] Add API to get memory mapping

[Wen Congyang]

At 03/16/2012 02:38 PM, HATAYAMA Daisuke Wrote:
 From: Wen Congyang we...@cn.fujitsu.com
 Subject: [RFC][PATCH 05/14 v9] Add API to get memory mapping
 Date: Wed, 14 Mar 2012 10:07:48 +0800
 
 Add API to get all virtual address and physical address mapping.
 If the guest doesn't use paging, the virtual address is equal to the phyical
 address. The virtual address and physical address mapping is for gdb's user, 
 and
 it does not include the memory that is not referenced by the page table. So 
 if
 you want to use crash to anaylze the vmcore, please do not specify -p option.
 
 It's necessary to write the reason why the -p option is not default
 explicitly: guest machine in a catastrophic state can have corrupted
 memory, which we cannot trust.

Yes

Thanks
Wen Congyang

 
 Thanks.
 HATAYAMA, Daisuke
 
 

Re: [Qemu-devel] qemu gdb issue

[Jacques]

Hi Mulyadi,

I see what you mean. How do I know if this is happening? When I do 'x/i
$eip' I get a completely sane result with exactly the instructions I want.

On 03/15/2012 07:13 PM, Mulyadi Santosa wrote:
 Hi...
 
 On Thu, Mar 15, 2012 at 23:03, Jacques jacq...@rambo-mes.net wrote:
 I'm running an application in qemu through the userspace qemu-i386 and
 attaching to the process with gdb. I have pygdb scripts that then
 interact with gdb.

 The issue is that at some point I want to change $eip and redirect
 instruction flow. I then set $eip to the value I need which gives me the
 following:

 Program received signal SIGSEGV, Segmentation fault.
 0x46367046 in ?? ()
 
 I am not keen in this kind of situation,but I think you hit non
 existing EIP. By that, I mean  maybe you think such EIP truly exist
 (based on ELF info perhaps?), but in reality since qemu user mode do
 dynamic translations and not really following ELF offset, you got
 segfault.
 
 


0x0B03082C.asc
Description: application/pgp-keys

[Qemu-devel] [Bug 918791] Re: qemu-kvm dies when using vmvga driver and unity in the guest

[Martin Pitt]

Is this still an issue in the -vmware package now?

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

Title:
  qemu-kvm dies when using vmvga driver and unity in the guest

Status in QEMU:
  New
Status in “qemu-kvm” package in Ubuntu:
  Fix Released
Status in “xserver-xorg-video-vmware” package in Ubuntu:
  Confirmed
Status in “qemu-kvm” source package in Oneiric:
  New
Status in “xserver-xorg-video-vmware” source package in Oneiric:
  New
Status in “qemu-kvm” source package in Precise:
  Fix Released
Status in “xserver-xorg-video-vmware” source package in Precise:
  Confirmed

Bug description:
  =
  SRU Justification:
  1. impact: kvm crashes
  2. Development fix: don't allow attempts to set_bit to negative offsets
  3. Stable fix: same as development fix
  4. Test case (see below)
  5. Regression potential: if the patch is wrong, graphics for vmware vga over 
vnc could get messed up
  =

  12.04's qemu-kvm has been unstable for me and Marc Deslauriers and I
  figured out it has something to do with the interaction of qemu-kvm,
  unity and the vmvga driver. This is a regression over qemu-kvm in
  11.10.

  TEST CASE:
  1. start a VM that uses unity (eg, 11.04, 11.10 or 12.04). My tests use 
unity-2d on an amd64 host and amd64 guests
  2. on 11.04 and 11.10, open empathy via the messaging indicator and click 
'Chat'. On 12.04, open empathy via the messaging indicator and click 'Chat', 
close the empathy wizard, move the empathy window over the unity luancher (so 
it autohides), then do 'ctrl+alt+t' to open a terminal

  When the launcher tries to auto(un)hide, qemu-kvm dies with this:
  [10574.958149] do_general_protection: 132 callbacks suppressed
  [10574.958154] kvm[13192] general protection ip:7fab9680ea0f sp:74440148 
error:0 in qemu-system-x86_64[7fab966c4000+2c9000]

  Relevant libvirt xml:
  video
    model type='vmvga' vram='9216' heads='1'/
    address type='pci' domain='0x' bus='0x00' slot='0x02' 
function='0x0'/
  /video

  If I change to using 'cirrus', then qemu-kvm no longer crashes. Eg:
  video
    model type='cirrus' vram='9216' heads='1'/
    alias name='video0'/
    address type='pci' domain='0x' bus='0x00' slot='0x02' 
function='0x0'/
  /video

  The workaround is therefore to use the cirrus driver instead of vmvga,
  however being able to kill qemu-kvm in this manner is not ideal. Also,
  unfortunately unity-2d does not run with with cirrus driver under
  11.04, so the security and SRU teams are unable to properly test
  updates in GUI applications under unity when using the current 12.04
  qemu-kvm.

  I tried to report this via apport, but apport complained about a CRC
  error, so I could not.

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

Re: [Qemu-devel] [PATCH 0/2 v3] kvm: notify host when guest panicked

[Wen Congyang]

At 03/15/2012 07:46 PM, Avi Kivity Wrote:
 On 03/15/2012 01:25 PM, Jan Kiszka wrote:

 There was such vm exit (KVM_EXIT_HYPERCALL), but it was deemed to be a
 bad idea.

 BTW, this would help a lot in emulating hypercalls of other hypervisors
 (or of KVM's VAPIC in the absence of in-kernel irqchip - I had to jump
 through hoops therefore) in user space. Not all those hypercall handlers
 actually have to reside in the KVM module.

 
 That is true.  On the other hand the hypercall ABI might go to pieces if
 there was no central implementation.
 

I prefer this:
host - guest kernel: use hypercall

host - guest userspace: use virtio-serial(or other way that not modify kernel)

Thanks
Wen Congyang

Re: [Qemu-devel] [PATCH 2/2] Drop obsolete nographic timer

[Jan Kiszka]

On 2012-03-16 01:52, Marek Vasut wrote:
 Dear Jan Kiszka,
 
 On 2012-03-15 19:12, Marek Vasut wrote:
 Dear Marek Vasut,

 Dear Jan Kiszka,

 On 2012-03-10 07:19, Marek Vasut wrote:
 Dear Jan Kiszka,

 We flush coalesced MMIO in the device models now, and VNC - for which
 this was once introduced - is also fine without it as it has its own
 refresh timer.

 Signed-off-by: Jan Kiszka jan.kis...@siemens.com
 ---

  vl.c |   13 -
  1 files changed, 0 insertions(+), 13 deletions(-)

 This patch seems to break QEMU/PXA emulation for me on the ZipitZ2
 platform. The serial console became very slow, but only when I type
 something in. The output from the device to the console is ok. After
 reverting this particular one, the console behaves normally.

 Removing that timer removal likely just revealed some formerly hidden
 bug. Can you share the command line used for starting and some test
 image?

 Jan

 qemu-system-arm -M z2 -pflash flash.img -serial null -serial null
 -serial stdio -display none

 flash.img attached (XZ-ed)

 Bump?

 I had a brief look: The guest is apparently polling the uart, IRQ
 delivery is disabled.
 
 Yes, it is.
 
 But it also receives no timer ticks. I haven't
 checked how it progresses, maybe there is some loop counter used.
 
 What do you mean? How does the timer implementation in pxa-uboot work? 
 udelay() 
 calls on the guest (pxa-uboot) simply poll the timer until it reaches certain 
 value. There's no paralelism going on at all.

I'm starting to understand the issue: The serial port can only accept a
single byte. Once this arrived, serial_can_receive1 returns 0, and the
backend fd is not longer polled by the io-thread. This should change
again as soon as the guest read that byte. But qemu_chr_accept_input,
which is properly called by the serial device, didn't kick the io-thread
in some way. I solved it this way now:

diff --git a/qemu-char.c b/qemu-char.c
index 9a5be75..a589a84 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -177,6 +177,7 @@ void qemu_chr_accept_input(CharDriverState *s)
 {
 if (s-chr_accept_input)
 s-chr_accept_input(s);
+qemu_notify_event();
 }

 void qemu_chr_fe_printf(CharDriverState *s, const char *fmt, ...)


But I'm not yet sure if this is correct. Comments welcome!

Jan



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] pci-assign can not work

[Jan Kiszka]

On 2012-03-16 03:38, Wen Congyang wrote:
 At 03/15/2012 06:21 PM, Wen Congyang Wrote:
 Hi all

 When I use pci-assign, I meet the following error:

 Failed to assign irq for hostdev0: Input/output error
 Perhaps you are assigning a device that shares an IRQ with another device?

 Is it a bug or I miss something?
 
 Hi, Jan
 
 This problem is caused by your patch:
 commit 6919115a8715c34cd80baa08422d90496f11f5d7
 Author: Jan Kiszka jan.kis...@siemens.com
 Date:   Thu Mar 8 11:10:27 2012 +0100
 
 pci_assign: Flip defaults of prefer_msi and share_intx
 
 INTx sharing is a bit more expensive than exclusive host interrupts, but
 this channel is not supposed to be used for high-performance scenarios
 anyway. Modern devices support MSI/MSI-X and do not depend on using INTx
 under critical workload, real old devices do not support INTx sharing
 anyway.
 
 For those in the middle, the user experience is much better if they just
 work even when IRQ sharing is required. If there is nothing to share,
 share_intx=off can still be applied as tuning parameter.
 
 With INTx sharing as default, the primary reason for prefer_msi=on is
 gone. Make it default off, specifically as it is known to cause troubles
 with devices that have incomplete/broken MSI support or otherwise
 stumble if host IRQ configuration does not match guest driver
 expectation.
 
 Acked-by: Alex Williamson alex.william...@redhat.com
 Signed-off-by: Jan Kiszka jan.kis...@siemens.com
 Signed-off-by: Marcelo Tosatti mtosa...@redhat.com
 
 If I revert this commit. qemu can work.
 

This should be solvable by passing prefer_msi=on to the pci-assign
device, or likely by updating your host kernel to latest kvm.git (to
enable INTx sharing).

Hmm, unfortunate. We needed a conditional default for the prefer_msi
property here. If INTx sharing doesn't work for some reason AND the user
did not ask for disabling the host-side MSI usage, we should fall back
to it again.

Markus, is there some easy way to find out if a specific qdev property
was set due to a command line switch or was defined by the default value?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] pci-assign can not work

[Wen Congyang]

At 03/16/2012 04:27 PM, Jan Kiszka Wrote:
 On 2012-03-16 03:38, Wen Congyang wrote:
 At 03/15/2012 06:21 PM, Wen Congyang Wrote:
 Hi all

 When I use pci-assign, I meet the following error:

 Failed to assign irq for hostdev0: Input/output error
 Perhaps you are assigning a device that shares an IRQ with another device?

 Is it a bug or I miss something?

 Hi, Jan

 This problem is caused by your patch:
 commit 6919115a8715c34cd80baa08422d90496f11f5d7
 Author: Jan Kiszka jan.kis...@siemens.com
 Date:   Thu Mar 8 11:10:27 2012 +0100

 pci_assign: Flip defaults of prefer_msi and share_intx
 
 INTx sharing is a bit more expensive than exclusive host interrupts, but
 this channel is not supposed to be used for high-performance scenarios
 anyway. Modern devices support MSI/MSI-X and do not depend on using INTx
 under critical workload, real old devices do not support INTx sharing
 anyway.
 
 For those in the middle, the user experience is much better if they just
 work even when IRQ sharing is required. If there is nothing to share,
 share_intx=off can still be applied as tuning parameter.
 
 With INTx sharing as default, the primary reason for prefer_msi=on is
 gone. Make it default off, specifically as it is known to cause troubles
 with devices that have incomplete/broken MSI support or otherwise
 stumble if host IRQ configuration does not match guest driver
 expectation.
 
 Acked-by: Alex Williamson alex.william...@redhat.com
 Signed-off-by: Jan Kiszka jan.kis...@siemens.com
 Signed-off-by: Marcelo Tosatti mtosa...@redhat.com

 If I revert this commit. qemu can work.

 
 This should be solvable by passing prefer_msi=on to the pci-assign
 device, or likely by updating your host kernel to latest kvm.git (to
 enable INTx sharing).

Is there some way to find out if the kernel supports to enable INTx
sharing?

Thanks
Wen Congyang

 
 Hmm, unfortunate. We needed a conditional default for the prefer_msi
 property here. If INTx sharing doesn't work for some reason AND the user
 did not ask for disabling the host-side MSI usage, we should fall back
 to it again.
 
 Markus, is there some easy way to find out if a specific qdev property
 was set due to a command line switch or was defined by the default value?
 
 Jan
 

Re: [Qemu-devel] [PATCH 2/2] Drop obsolete nographic timer

[Paolo Bonzini]

Il 16/03/2012 09:17, Jan Kiszka ha scritto:
 I'm starting to understand the issue: The serial port can only accept a
 single byte. Once this arrived, serial_can_receive1 returns 0, and the
 backend fd is not longer polled by the io-thread. This should change
 again as soon as the guest read that byte. But qemu_chr_accept_input,
 which is properly called by the serial device, didn't kick the io-thread
 in some way. I solved it this way now:
 
 diff --git a/qemu-char.c b/qemu-char.c
 index 9a5be75..a589a84 100644
 --- a/qemu-char.c
 +++ b/qemu-char.c
 @@ -177,6 +177,7 @@ void qemu_chr_accept_input(CharDriverState *s)
  {
  if (s-chr_accept_input)
  s-chr_accept_input(s);
 +qemu_notify_event();
  }
 
  void qemu_chr_fe_printf(CharDriverState *s, const char *fmt, ...)
 
 
 But I'm not yet sure if this is correct. Comments welcome!

I think so.  qemu_chr_accept_input signals that qemu_chr_be_can_write
could have changed, which means that the can_read handler could have
changed and has to be reevaluated.  qemu_notify_event is the right way
to do so.

Paolo

[Qemu-devel] [V5 PATCH 1/4] net: announce self after vm start

[Jason Wang]

qemu_announce_self() were moved to vm_start(). This is because we may
want to let guest to send the gratuitous packets. After this change,
we need to check the previous run state (RUN_STATE_INMIGRATE) to
decide whether an announcement is needed.

Signed-off-by: Jason Wang jasow...@redhat.com
---
 migration.c |1 -
 vl.c|4 
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/migration.c b/migration.c
index 00fa1e3..1ce6b5c 100644
--- a/migration.c
+++ b/migration.c
@@ -88,7 +88,6 @@ void process_incoming_migration(QEMUFile *f)
 fprintf(stderr, load of migration failed\n);
 exit(0);
 }
-qemu_announce_self();
 DPRINTF(successfully loaded vm state\n);
 
 /* Make sure all file formats flush their mutable metadata */
diff --git a/vl.c b/vl.c
index 65f11f2..4742b1b 100644
--- a/vl.c
+++ b/vl.c
@@ -1261,11 +1261,15 @@ void vm_state_notify(int running, RunState state)
 void vm_start(void)
 {
 if (!runstate_is_running()) {
+RunState prev_run_state = current_run_state;
 cpu_enable_ticks();
 runstate_set(RUN_STATE_RUNNING);
 vm_state_notify(1, RUN_STATE_RUNNING);
 resume_all_vcpus();
 monitor_protocol_event(QEVENT_RESUME, NULL);
+if (prev_run_state == RUN_STATE_INMIGRATE) {
+qemu_announce_self();
+}
 }
 }
 

[Qemu-devel] [V5 PATCH 0/4] Send gratuitous packets by guest

[Jason Wang]

This an update of series that let guest and qemu to be co-operated to
send gratuitous packets when needed such as after migration, loadvm
and continuing.

As it's hard for qemu to track the network configuration in guest such
as bondings, vlans or ipv6. So current gratuitous may not work under
those situations.

The series first introduce a model specific function in order to let
nic models to use a device specific way to announce the link
presence. With this, virtio-net backend were modified to notify the
guest (through config update interrupt) and let guest send the
gratuitous packet when needed.

Changes from V4:

- keep the old behavior that send the gratuitous packets only after
  migration
- decide whether to send gratuitous packets by previous runstate
  instead of a dedicated parameter
- check virtio_net_started() instead of VIRTIO_NET_S_LINK_UP before
  issue the config update interrupt
- move VIRTIO_NET_S_ANNOUNCE to 0x100 and supress guest config write to RO bits
- cleanups suggested by Michael

---

Jason Wang (4):
  net: announce self after vm start
  net: model specific announcing support
  virtio-net: notify guest to annouce itself
  virtio-net: compat guest announce support.


 hw/pc_piix.c|   35 +++
 hw/virtio-net.c |   19 +++
 hw/virtio-net.h |3 +++
 migration.c |1 -
 net.h   |2 ++
 savevm.c|8 +---
 vl.c|4 
 7 files changed, 68 insertions(+), 4 deletions(-)

-- 
Jason Wang

[Qemu-devel] [V5 PATCH 3/4] virtio-net: notify guest to annouce itself

[Jason Wang]

It's hard to track all mac addresses and their usage (vlan, bondings,
ipv6) in qemu to send proper gratuitous packet. The better choice is
to let guest to send them.

So, this patch introduces a new rw config status bit of virtio-net,
VIRTIO_NET_S_ANNOUNCE which is used to notify guest to announce
presence of its link through config update interrupt. When gust have
done the announcement, it should clear that bit. The feature is negotiated by
a new feature bit VIRTIO_NET_F_ANNOUNCE.

Signed-off-by: Jason Wang jasow...@redhat.com
---
 hw/virtio-net.c |   19 +++
 hw/virtio-net.h |3 +++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index bc5e3a8..c1dbd49 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -95,6 +95,10 @@ static void virtio_net_set_config(VirtIODevice *vdev, const 
uint8_t *config)
 memcpy(n-mac, netcfg.mac, ETH_ALEN);
 qemu_format_nic_info_str(n-nic-nc, n-mac);
 }
+
+/* ignore write to RO byte */
+memcpy((uint8_t *)n-status + 1, (uint8_t *)netcfg.status + 1,
+   sizeof(uint8_t));
 }
 
 static bool virtio_net_started(VirtIONet *n, uint8_t status)
@@ -983,6 +987,20 @@ static void virtio_net_cleanup(VLANClientState *nc)
 n-nic = NULL;
 }
 
+static int virtio_net_announce(VLANClientState *nc)
+{
+VirtIONet *n = DO_UPCAST(NICState, nc, nc)-opaque;
+
+if (n-vdev.guest_features  (0x1  VIRTIO_NET_F_GUEST_ANNOUNCE)
+ virtio_net_started(n, n-vdev.status)) {
+n-status |= VIRTIO_NET_S_ANNOUNCE;
+virtio_notify_config(n-vdev);
+return 0;
+}
+
+return -1;
+}
+
 static NetClientInfo net_virtio_info = {
 .type = NET_CLIENT_TYPE_NIC,
 .size = sizeof(NICState),
@@ -990,6 +1008,7 @@ static NetClientInfo net_virtio_info = {
 .receive = virtio_net_receive,
 .cleanup = virtio_net_cleanup,
 .link_status_changed = virtio_net_set_link_status,
+.announce = virtio_net_announce,
 };
 
 VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 4468741..f3acbd1 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -44,8 +44,10 @@
 #define VIRTIO_NET_F_CTRL_RX18  /* Control channel RX mode support */
 #define VIRTIO_NET_F_CTRL_VLAN  19  /* Control channel VLAN filtering */
 #define VIRTIO_NET_F_CTRL_RX_EXTRA 20   /* Extra RX mode control support */
+#define VIRTIO_NET_F_GUEST_ANNOUNCE 21  /* Guest can announce itself */
 
 #define VIRTIO_NET_S_LINK_UP1   /* Link is up */
+#define VIRTIO_NET_S_ANNOUNCE   0x100   /* Announcement is needed */
 
 #define TX_TIMER_INTERVAL 15 /* 150 us */
 
@@ -176,6 +178,7 @@ struct virtio_net_ctrl_mac {
 DEFINE_PROP_BIT(guest_tso6, _state, _field, VIRTIO_NET_F_GUEST_TSO6, 
true), \
 DEFINE_PROP_BIT(guest_ecn, _state, _field, VIRTIO_NET_F_GUEST_ECN, 
true), \
 DEFINE_PROP_BIT(guest_ufo, _state, _field, VIRTIO_NET_F_GUEST_UFO, 
true), \
+DEFINE_PROP_BIT(guest_announce, _state, _field, 
VIRTIO_NET_F_GUEST_ANNOUNCE, true), \
 DEFINE_PROP_BIT(host_tso4, _state, _field, VIRTIO_NET_F_HOST_TSO4, 
true), \
 DEFINE_PROP_BIT(host_tso6, _state, _field, VIRTIO_NET_F_HOST_TSO6, 
true), \
 DEFINE_PROP_BIT(host_ecn, _state, _field, VIRTIO_NET_F_HOST_ECN, 
true), \

[Qemu-devel] [V5 PATCH 4/4] virtio-net: compat guest announce support.

[Jason Wang]

Disable guest announce for compat machine types.

Signed-off-by: Jason Wang jasow...@redhat.com
---
 hw/pc_piix.c |   35 +++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 6c5c40f..780b607 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -389,6 +389,11 @@ static QEMUMachine pc_machine_v1_0 = {
 .property = check_media_rate,
 .value= off,
 },
+{
+.driver   = virtio-net-pci,
+.property = guest_announce,
+.value= off,
+},
 { /* end of list */ }
 },
 };
@@ -408,6 +413,11 @@ static QEMUMachine pc_machine_v0_15 = {
 .property = check_media_rate,
 .value= off,
 },
+{
+.driver   = virtio-net-pci,
+.property = guest_announce,
+.value= off,
+},
 { /* end of list */ }
 },
 };
@@ -452,6 +462,11 @@ static QEMUMachine pc_machine_v0_14 = {
 .property = rom_only,
 .value= stringify(1),
 },
+{
+.driver   = virtio-net-pci,
+.property = guest_announce,
+.value= off,
+},
 { /* end of list */ }
 },
 };
@@ -508,6 +523,11 @@ static QEMUMachine pc_machine_v0_13 = {
 .property = rom_only,
 .value= stringify(1),
 },
+{
+.driver   = virtio-net-pci,
+.property = guest_announce,
+.value= off,
+},
 { /* end of list */ }
 },
 };
@@ -568,6 +588,11 @@ static QEMUMachine pc_machine_v0_12 = {
 .property = rom_only,
 .value= stringify(1),
 },
+{
+.driver   = virtio-net-pci,
+.property = guest_announce,
+.value= off,
+},
 { /* end of list */ }
 }
 };
@@ -636,6 +661,11 @@ static QEMUMachine pc_machine_v0_11 = {
 .property = rom_only,
 .value= stringify(1),
 },
+{
+.driver   = virtio-net-pci,
+.property = guest_announce,
+.value= off,
+},
 { /* end of list */ }
 }
 };
@@ -716,6 +746,11 @@ static QEMUMachine pc_machine_v0_10 = {
 .property = rom_only,
 .value= stringify(1),
 },
+{
+.driver   = virtio-net-pci,
+.property = guest_announce,
+.value= off,
+},
 { /* end of list */ }
 },
 };

Re: [Qemu-devel] [PATCH 0/2] virtio-pci: fix abort when fail to allocate ioeventfd

[Amos Kong]

On 14/03/12 19:46, Stefan Hajnoczi wrote:

On Wed, Mar 14, 2012 at 10:46 AM, Avi Kivitya...@redhat.com  wrote:

On 03/14/2012 12:39 PM, Stefan Hajnoczi wrote:

On Wed, Mar 14, 2012 at 10:05 AM, Avi Kivitya...@redhat.com  wrote:

On 03/14/2012 11:59 AM, Stefan Hajnoczi wrote:

On Wed, Mar 14, 2012 at 9:22 AM, Avi Kivitya...@redhat.com  wrote:

On 03/13/2012 12:42 PM, Amos Kong wrote:

Boot up guest with 232 virtio-blk disk, qemu will abort for fail to
allocate ioeventfd. This patchset changes kvm_has_many_ioeventfds(),
and check if available ioeventfd exists. If not, virtio-pci will
fallback to userspace, and don't use ioeventfd for io notification.


How about an alternative way of solving this, within the memory core:
trap those writes in qemu and write to the ioeventfd yourself.  This way
ioeventfds work even without kvm:


  core: create eventfd
  core: install handler for memory address that writes to ioeventfd
  kvm (optional): install kernel handler for ioeventfd


Can you give some detail about this? I'm not familiar with Memory API.


btw, can we fix this problem by replacing abort() by a error note?
virtio-pci will auto fallback to userspace.

diff --git a/kvm-all.c b/kvm-all.c
index 3c6b4f0..cf23dbf 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -749,7 +749,8 @@ static void 
kvm_mem_ioeventfd_add(MemoryRegionSection *section,
 r = kvm_set_ioeventfd_mmio_long(fd, 
section-offset_within_address_space,

 data, true);
 if (r  0) {
-abort();
+fprintf(stderr, %s: unable to map ioeventfd: %s.\nFallback to 
+userspace (slower).\n, __func__, strerror(-r));
 }
 }

@@ -775,7 +776,8 @@ static void kvm_io_ioeventfd_add(MemoryRegionSection 
*section,
 r = kvm_set_ioeventfd_pio_word(fd, 
section-offset_within_address_space,

data, true);
 if (r  0) {
-abort();
+fprintf(stderr, %s: unable to map ioeventfd: %s.\nFallback to 
+userspace (slower).\n, __func__, strerror(-r));
 }
 }



even if the third step fails, the ioeventfd still works, it's just slower.


That approach will penalize guests with large numbers of disks - they
see an extra switch to vcpu thread instead of kvm.ko -  iothread.


It's only a failure path.  The normal path is expected to have a kvm
ioeventfd installed.


It's the normal path when you attach232 virtio-blk devices to a
guest (or 300 in the future).


Well, there's nothing we can do about it.

We'll increase the limit of course, but old kernels will remain out
there.  The right fix is virtio-scsi anyway.


   It
seems okay provided we can solve the limit in the kernel once and for
all by introducing a more dynamic data structure for in-kernel
devices.  That way future kernels will never hit an arbitrary limit
below their file descriptor rlimit.

Is there some reason why kvm.ko must use a fixed size array?  Would it
be possible to use a tree (maybe with a cache for recent lookups)?


It does use bsearch today IIRC.  We'll expand the limit, but there must
be a limit, and qemu must be prepared to deal with it.


Shouldn't the limit be the file descriptor rlimit?  If userspace
cannot create more eventfds then it cannot set up more ioeventfds.


You can use the same eventfd for multiple ioeventfds.  If you mean to
slave kvm's ioeventfd limit to the number of files the process can have,
that's a good idea.  Surely an ioeventfd occupies less resources than an
open file.


Yes.

Ultimately I guess you're right in that we still need to have an error
path and virtio-scsi will reduce the pressure on I/O eventfds for
storage.

Stefan


--
Amos.

[Qemu-devel] [RESEND PATCH] ioapic: fix build with DEBUG_IOAPIC

[Jason Wang]

ioapic.c:198: error: format ‘%08x’ expects type ‘unsigned int’, but argument 3 
has type ‘uint64_t’

Signed-off-by: Jason Wang jasow...@redhat.com
---
 hw/ioapic.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/ioapic.c b/hw/ioapic.c
index 3fee011..1ff31a1 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -195,7 +195,7 @@ ioapic_mem_write(void *opaque, target_phys_addr_t addr, 
uint64_t val,
 if (size != 4) {
 break;
 }
-DPRINTF(write: %08x = %08x\n, s-ioregsel, val);
+DPRINTF(write: %08x = % PRIx64 \n, s-ioregsel, val);
 switch (s-ioregsel) {
 case IOAPIC_REG_ID:
 s-id = (val  IOAPIC_ID_SHIFT)  IOAPIC_ID_MASK;

Re: [Qemu-devel] [PATCH v4 4/9] MSI-X state save/load invocations moved to PCI Device save/load callbacks to avoid code duplication in MSI-X-enabled devices that support live migration

[Dmitry Fleytman]

Michael,

Great. I believe higher level API if what really needed here.
I'll revert this patch and move msix_load/store invocations into the
device code.

Thanks.

On Fri, Mar 16, 2012 at 1:00 AM, Michael S. Tsirkin m...@redhat.com wrote:
 On Thu, Mar 15, 2012 at 11:09:03PM +0200, Dmitry Fleytman wrote:
 Signed-off-by: Dmitry Fleytman dmi...@daynix.com
 Signed-off-by: Yan Vugenfirer y...@daynix.com

 I'm working on a higher level API that will
 handle all capabilities. For now, pls just put
 these calls in your device.


 ---
  hw/pci.c        |    5 +
  hw/virtio-pci.c |    2 --
  2 files changed, 5 insertions(+), 2 deletions(-)

 diff --git a/hw/pci.c b/hw/pci.c
 index bf046bf..9146d3f 100644
 --- a/hw/pci.c
 +++ b/hw/pci.c
 @@ -31,6 +31,7 @@
  #include loader.h
  #include range.h
  #include qmp-commands.h
 +#include msix.h

  //#define DEBUG_PCI
  #ifdef DEBUG_PCI
 @@ -387,6 +388,8 @@ static int get_pci_irq_state(QEMUFile *f, void *pv, 
 size_t size)
          pci_set_irq_state(s, i, irq_state[i]);
      }

 +    msix_load(s, f);
 +
      return 0;
  }

 @@ -398,6 +401,8 @@ static void put_pci_irq_state(QEMUFile *f, void *pv, 
 size_t size)
      for (i = 0; i  PCI_NUM_PINS; ++i) {
          qemu_put_be32(f, pci_irq_state(s, i));
      }
 +
 +    msix_save(s, f);
  }

  static VMStateInfo vmstate_info_pci_irq_state = {
 diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
 index a0fb7c1..2f3cb1f 100644
 --- a/hw/virtio-pci.c
 +++ b/hw/virtio-pci.c
 @@ -110,7 +110,6 @@ static void virtio_pci_save_config(void * opaque, 
 QEMUFile *f)
  {
      VirtIOPCIProxy *proxy = opaque;
      pci_device_save(proxy-pci_dev, f);
 -    msix_save(proxy-pci_dev, f);
      if (msix_present(proxy-pci_dev))
          qemu_put_be16(f, proxy-vdev-config_vector);
  }
 @@ -130,7 +129,6 @@ static int virtio_pci_load_config(void * opaque, 
 QEMUFile *f)
      if (ret) {
          return ret;
      }
 -    msix_load(proxy-pci_dev, f);
      if (msix_present(proxy-pci_dev)) {
          qemu_get_be16s(f, proxy-vdev-config_vector);
      } else {
 --
 1.7.7.6

Re: [Qemu-devel] [PATCH v2] Man page: Add -global description

[Gerd Hoffmann]

  Hi,

 commit 82aff428155d469ab705294486cc26cb34947999
 Author: Anthony Liguori aligu...@us.ibm.com
 Date:   Fri Dec 23 11:30:45 2011 -0600
 
 qdev: don't allow globals to be set by bus name

 So I think we can safely break it :-)

There are compat properties using that (turn off new pci features on old
releases for all pci devices).

cheers,
  Gerd

Re: [Qemu-devel] Adding make check to the QEMU buildbot

[Gerd Hoffmann]

On 03/15/12 18:21, Stefan Weil wrote:
 Am 15.03.2012 09:06, schrieb Stefan Hajnoczi:
 QEMU has grown a number of sanity tests that can be run using make
 check. They are fast and do not require many resources.

 Is it possible to add make check after the build?

 We may have to deal with some failures in the beginning - either due to
 buildslave environment or legitimate broken platforms. But I think we
 can get all green pretty easily.

 Stefan
 
 You can combine build and check by running make  all check.

It is useful to keep them as separate steps though as the error mails
can tell you then which step failed.

cheers,
  Gerd

Re: [Qemu-devel] [RFC PATCH 06/17] block: use bdrv_{co, aio}_discard for write_zeroes operations

[Paolo Bonzini]

Il 16/03/2012 01:47, Richard Laager ha scritto:
 On Thu, 2012-03-15 at 10:36 +0100, Paolo Bonzini wrote:
 Changing across guest boots is a minor problem, but changing across
 migration must be avoided at all costs.

 BTW, after this discussion I think we can instead report
 discard_granularity = 512 and discard_zeroes_data=0 and get most of the
 benefit, at least on file-backed storage.
 
 Are you going to report that to guests all the time, or only when the
 host supports discard? If you don't report it all the time, you could
 still be changing across migration. If you do report it all the time,
 then you're incurring a performance penalty on systems that don't
 support discard, as the guest will be sending discard requests that QEMU
 has to throw away (but by which time some work has been wasted).

I don't think that should be that bad.  Discard requests should be
relatively rare.

 And either way, what you're proposing means that users with
 discard_zeros_data = 1 hosts can't get the (albeit small) benefits of
 that because some other QEMU user might want to do a migration across
 heterogeneous storage.

Yes, discard_zeroes_data can be made configurable on top, and either
rejected or emulated if the storage does not support it.

 Finally, I see your proposal of advertising fixed discard support

It does not really have to be fixed, it's just a default.

Paolo

Re: [Qemu-devel] [V5 PATCH 1/4] net: announce self after vm start

[Paolo Bonzini]

Il 16/03/2012 09:54, Jason Wang ha scritto:
 qemu_announce_self() were moved to vm_start(). This is because we may
 want to let guest to send the gratuitous packets. After this change,
 we need to check the previous run state (RUN_STATE_INMIGRATE) to
 decide whether an announcement is needed.
 
 Signed-off-by: Jason Wang jasow...@redhat.com
 ---
  migration.c |1 -
  vl.c|4 
  2 files changed, 4 insertions(+), 1 deletions(-)
 
 diff --git a/migration.c b/migration.c
 index 00fa1e3..1ce6b5c 100644
 --- a/migration.c
 +++ b/migration.c
 @@ -88,7 +88,6 @@ void process_incoming_migration(QEMUFile *f)
  fprintf(stderr, load of migration failed\n);
  exit(0);
  }
 -qemu_announce_self();
  DPRINTF(successfully loaded vm state\n);
  
  /* Make sure all file formats flush their mutable metadata */
 diff --git a/vl.c b/vl.c
 index 65f11f2..4742b1b 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -1261,11 +1261,15 @@ void vm_state_notify(int running, RunState state)
  void vm_start(void)
  {
  if (!runstate_is_running()) {
 +RunState prev_run_state = current_run_state;
  cpu_enable_ticks();
  runstate_set(RUN_STATE_RUNNING);
  vm_state_notify(1, RUN_STATE_RUNNING);
  resume_all_vcpus();
  monitor_protocol_event(QEVENT_RESUME, NULL);
 +if (prev_run_state == RUN_STATE_INMIGRATE) {
 +qemu_announce_self();
 +}
  }
  }
  
 

I tihnk this won't work with -S, did you test it?  Perhaps it's possible
simply to change

if (autostart) {
vm_start();
} else {
runstate_set(RUN_STATE_PRELAUNCH);
}

to remain in INMIGRATE state:

if (autostart) {
vm_start();
}

Otherwise looks good.

Paolo

Re: [Qemu-devel] [PATCH 0/7] block: convert VDI image format to coroutines

[Kevin Wolf]

Am 15.03.2012 22:28, schrieb Stefan Weil:
 Am 05.03.2012 18:40, schrieb Paolo Bonzini:
 Conversion to coroutines simplifies the code and removes the need to
 duplicate common features of the block layer. Each step in the conversion
 is detailed in the corresponding commit message.

 Tested with qemu-iotests.

 Paolo Bonzini (7):
 vdi: basic conversion to coroutines
 vdi: move end-of-I/O handling at the end
 vdi: merge aio_read_cb and aio_write_cb into callers
 vdi: move aiocb fields to locals
 vdi: leave bounce buffering to block layer
 vdi: do not create useless iovecs
 vdi: change goto to loop

 block/vdi.c | 421 +-
 2 files changed, 108 insertions(+), 317 deletions(-)
 
 Acked-by: Stefan Weil s...@weilnetz.de
 
 Kevin, could you please add Paolo's patches to your block queue?

Yes, I was just waiting for your Acked-by.

Thanks, applied all patches to the block branch.

Kevin

Re: [Qemu-devel] [PATCH 5/7] vdi: leave bounce buffering to block layer

[Kevin Wolf]

Am 05.03.2012 18:40, schrieb Paolo Bonzini:
 vdi.c really works as if it implemented bdrv_read and bdrv_write.
 However, because only vector I/O is supported by the asynchronous
 callbacks, it went through extra pain to bounce-buffer the I/O.
 With the conversion to coroutines bdrv_read and bdrv_write are now
 asynchronous, so they can be handled by the block layer now that the
 format is coroutine-based.
 
 Signed-off-by: Paolo Bonzini pbonz...@redhat.com

In the long run, the right thing to do would be to convert VDI to deal
with vectored I/O, of course. Stefan, maybe you want to do that on top.
It's not that hard, I did it for qcow2 a while ago.

Kevin

Re: [Qemu-devel] [PATCH] softfloat: fix for C99

[Andreas Färber]

Am 09.02.2012 17:38, schrieb Avi Kivity:
 On 02/09/2012 06:20 PM, Andreas Färber wrote:
 Am 27.12.2011 17:15, schrieb Andreas Färber:
 Am 27.12.2011 16:11, schrieb Avi Kivity:
 C99 appears to consider compound literals as non-constants, and complains
 when they are used in static initializers.  Switch to ordinary initializer
 syntax.


 Reported-by: Andreas Färber andreas.faer...@web.de

 Signed-off-by: Avi Kivity a...@redhat.com

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

 For the record, tested with --extra-cflags=-std=gnu99.

 diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
 index c5e2dab..4902450 100644
 --- a/fpu/softfloat-specialize.h
 +++ b/fpu/softfloat-specialize.h
 @@ -89,8 +89,8 @@ const float64 float64_default_nan = const_float64(LIT64( 
 0xFFF8 ));
  #define floatx80_default_nan_low  LIT64( 0xC000 )
  #endif
  
 -const floatx80 floatx80_default_nan = 
 make_floatx80(floatx80_default_nan_high,
 -
 floatx80_default_nan_low);
 +const floatx80 floatx80_default_nan
 += make_floatx80_init(floatx80_default_nan_high, 
 floatx80_default_nan_low);

 Calling it init_floatx80 would avoid the line break, but I'm okay with
 it either way.

 Ping! Avi, you didn't indicate whether you were going to simplify this
 patch or whether you're waiting for someone to apply it as is?

 
 Actually I forgot all about it.  If everyone's okay with it as is I'd
 like it to be applied, otherwise I'll respin.  Copying some maintainers
 completely at random.

Ping?

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] [RFC][PATCH 11/14 v9] introduce a new monitor command 'dump' to dump guest's memory

[Wen Congyang]

At 03/15/2012 01:18 AM, Luiz Capitulino Wrote:
 On Wed, 14 Mar 2012 10:11:35 +0800
 Wen Congyang we...@cn.fujitsu.com wrote:
 
 The command's usage:
dump [-p] file
 file should be start with file:(the file's path) or fd:(the fd's name).

 Note:
   1. If you want to use gdb to analyse the core, please specify -p option.
   2. This command doesn't support the fd that is is associated with a pipe,
  socket, or FIFO(lseek will fail with such fd).

 Signed-off-by: Wen Congyang we...@cn.fujitsu.com
 ---
  Makefile.target  |2 +-
  dump.c   |  714 
 ++
  elf.h|5 +
  hmp-commands.hx  |   21 ++
  hmp.c|   10 +
  hmp.h|1 +
  qapi-schema.json |   14 +
  qmp-commands.hx  |   34 +++
  8 files changed, 800 insertions(+), 1 deletions(-)
  create mode 100644 dump.c

 diff --git a/Makefile.target b/Makefile.target
 index c81c4fa..287fbe7 100644
 --- a/Makefile.target
 +++ b/Makefile.target
 @@ -213,7 +213,7 @@ obj-$(CONFIG_NO_KVM) += kvm-stub.o
  obj-$(CONFIG_VGA) += vga.o
  obj-y += memory.o savevm.o
  obj-y += memory_mapping.o
 -obj-$(CONFIG_HAVE_CORE_DUMP) += arch_dump.o
 +obj-$(CONFIG_HAVE_CORE_DUMP) += arch_dump.o dump.o
  LIBS+=-lz
  
  obj-i386-$(CONFIG_KVM) += hyperv.o
 diff --git a/dump.c b/dump.c
 new file mode 100644
 index 000..42e1681
 --- /dev/null
 +++ b/dump.c
 @@ -0,0 +1,714 @@
 +/*
 + * QEMU dump
 + *
 + * Copyright Fujitsu, Corp. 2011
 + *
 + * Authors:
 + * Wen Congyang we...@cn.fujitsu.com
 + *
 + * This work is licensed under the terms of the GNU GPL, version 2.  See
 + * the COPYING file in the top-level directory.
 + *
 + */
 +
 +#include qemu-common.h
 +#include unistd.h
 +#include elf.h
 +#include sys/procfs.h
 +#include glib.h
 +#include cpu.h
 +#include cpu-all.h
 +#include targphys.h
 +#include monitor.h
 +#include kvm.h
 +#include dump.h
 +#include sysemu.h
 +#include bswap.h
 +#include memory_mapping.h
 +#include error.h
 +#include qmp-commands.h
 +#include gdbstub.h
 +
 +static inline uint16_t cpu_convert_to_target16(uint16_t val, int endian)
 +{
 +if (endian == ELFDATA2LSB) {
 +val = cpu_to_le16(val);
 +} else {
 +val = cpu_to_be16(val);
 +}
 +
 +return val;
 +}
 +
 +static inline uint32_t cpu_convert_to_target32(uint32_t val, int endian)
 +{
 +if (endian == ELFDATA2LSB) {
 +val = cpu_to_le32(val);
 +} else {
 +val = cpu_to_be32(val);
 +}
 +
 +return val;
 +}
 +
 +static inline uint64_t cpu_convert_to_target64(uint64_t val, int endian)
 +{
 +if (endian == ELFDATA2LSB) {
 +val = cpu_to_le64(val);
 +} else {
 +val = cpu_to_be64(val);
 +}
 +
 +return val;
 +}
 +
 +enum {
 +DUMP_STATE_ERROR,
 +DUMP_STATE_SETUP,
 +DUMP_STATE_CANCELLED,
 +DUMP_STATE_ACTIVE,
 +DUMP_STATE_COMPLETED,
 +};
 +
 +typedef struct DumpState {
 +ArchDumpInfo dump_info;
 +MemoryMappingList list;
 +uint16_t phdr_num;
 +uint32_t sh_info;
 +bool have_section;
 +int state;
 +bool resume;
 +char *error;
 +target_phys_addr_t memory_offset;
 +write_core_dump_function f;
 +void (*cleanup)(void *opaque);
 +void *opaque;
 +} DumpState;
 +
 +static DumpState *dump_get_current(void)
 +{
 +static DumpState current_dump = {
 +.state = DUMP_STATE_SETUP,
 +};
 +
 +return current_dump;
 +}
 
 You just dropped a few asynchronous bits and resent this as a synchronous
 command, letting all the asynchronous infrastructure in. This is bad, as the
 command is more complex then it should be and doesn't make full use of the
 added infrastructure.
 
 For example, does the synchronous version really uses DumpState? If it 
 doesn't,
 let's just drop it and everything else which is not necessary.

I use this struct to avoid too many parameters...

I will try to make it simple and clean.

Thanks
Wen Congyang

 
 *However*, note that while it's fine with me to have this as a synchronous
 command we need a few more ACKs (from libvirt and Anthony and/or Jan). So, I
 wouldn't go too far on making changes before we get those ACKs.
 
 +
 +static int dump_cleanup(DumpState *s)
 +{
 +int ret = 0;
 +
 +memory_mapping_list_free(s-list);
 +s-cleanup(s-opaque);
 +if (s-resume) {
 +vm_start();
 +}
 +
 +return ret;
 +}
 +
 +static void dump_error(DumpState *s, const char *reason)
 +{
 +s-state = DUMP_STATE_ERROR;
 +s-error = g_strdup(reason);
 +dump_cleanup(s);
 +}
 +
 +static int write_elf64_header(DumpState *s)
 +{
 +Elf64_Ehdr elf_header;
 +int ret;
 +int endian = s-dump_info.d_endian;
 +
 +memset(elf_header, 0, sizeof(Elf64_Ehdr));
 +memcpy(elf_header, ELFMAG, 4);
 +elf_header.e_ident[EI_CLASS] = ELFCLASS64;
 +elf_header.e_ident[EI_DATA] = s-dump_info.d_endian;
 +elf_header.e_ident[EI_VERSION] = EV_CURRENT;
 +elf_header.e_type = cpu_convert_to_target16(ET_CORE, endian);
 +

Re: [Qemu-devel] [V5 PATCH 1/4] net: announce self after vm start

[Jason Wang]

On 03/16/2012 05:43 PM, Paolo Bonzini wrote:

Il 16/03/2012 09:54, Jason Wang ha scritto:

qemu_announce_self() were moved to vm_start(). This is because we may
want to let guest to send the gratuitous packets. After this change,
we need to check the previous run state (RUN_STATE_INMIGRATE) to
decide whether an announcement is needed.

Signed-off-by: Jason Wangjasow...@redhat.com
---
  migration.c |1 -
  vl.c|4 
  2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/migration.c b/migration.c
index 00fa1e3..1ce6b5c 100644
--- a/migration.c
+++ b/migration.c
@@ -88,7 +88,6 @@ void process_incoming_migration(QEMUFile *f)
  fprintf(stderr, load of migration failed\n);
  exit(0);
  }
-qemu_announce_self();
  DPRINTF(successfully loaded vm state\n);

  /* Make sure all file formats flush their mutable metadata */
diff --git a/vl.c b/vl.c
index 65f11f2..4742b1b 100644
--- a/vl.c
+++ b/vl.c
@@ -1261,11 +1261,15 @@ void vm_state_notify(int running, RunState state)
  void vm_start(void)
  {
  if (!runstate_is_running()) {
+RunState prev_run_state = current_run_state;
  cpu_enable_ticks();
  runstate_set(RUN_STATE_RUNNING);
  vm_state_notify(1, RUN_STATE_RUNNING);
  resume_all_vcpus();
  monitor_protocol_event(QEVENT_RESUME, NULL);
+if (prev_run_state == RUN_STATE_INMIGRATE) {
+qemu_announce_self();
+}
  }
  }



I tihnk this won't work with -S, did you test it?  Perhaps it's possible
simply to change


Yes, it does not work.


 if (autostart) {
 vm_start();
 } else {
 runstate_set(RUN_STATE_PRELAUNCH);
 }

to remain in INMIGRATE state:

 if (autostart) {
 vm_start();
 }

Otherwise looks good.

Paolo


The problem with staying in the INMIGRATE is that we can not figure out 
when the migration is completed when using '-S', so this kind of 
transition were forbidden by qmp_cont().


Looks like we need a new state such as RUN_STATE_MIGRATE_PRELAUNCH?

Re: [Qemu-devel] pci-assign can not work

[Jan Kiszka]

On 2012-03-16 09:38, Wen Congyang wrote:
 At 03/16/2012 04:27 PM, Jan Kiszka Wrote:
 On 2012-03-16 03:38, Wen Congyang wrote:
 At 03/15/2012 06:21 PM, Wen Congyang Wrote:
 Hi all

 When I use pci-assign, I meet the following error:

 Failed to assign irq for hostdev0: Input/output error
 Perhaps you are assigning a device that shares an IRQ with another device?

 Is it a bug or I miss something?

 Hi, Jan

 This problem is caused by your patch:
 commit 6919115a8715c34cd80baa08422d90496f11f5d7
 Author: Jan Kiszka jan.kis...@siemens.com
 Date:   Thu Mar 8 11:10:27 2012 +0100

 pci_assign: Flip defaults of prefer_msi and share_intx
 
 INTx sharing is a bit more expensive than exclusive host interrupts, but
 this channel is not supposed to be used for high-performance scenarios
 anyway. Modern devices support MSI/MSI-X and do not depend on using INTx
 under critical workload, real old devices do not support INTx sharing
 anyway.
 
 For those in the middle, the user experience is much better if they just
 work even when IRQ sharing is required. If there is nothing to share,
 share_intx=off can still be applied as tuning parameter.
 
 With INTx sharing as default, the primary reason for prefer_msi=on is
 gone. Make it default off, specifically as it is known to cause troubles
 with devices that have incomplete/broken MSI support or otherwise
 stumble if host IRQ configuration does not match guest driver
 expectation.
 
 Acked-by: Alex Williamson alex.william...@redhat.com
 Signed-off-by: Jan Kiszka jan.kis...@siemens.com
 Signed-off-by: Marcelo Tosatti mtosa...@redhat.com

 If I revert this commit. qemu can work.


 This should be solvable by passing prefer_msi=on to the pci-assign
 device, or likely by updating your host kernel to latest kvm.git (to
 enable INTx sharing).
 
 Is there some way to find out if the kernel supports to enable INTx
 sharing?

QEMU does a feature check, but as a user you simply have to know which
kernel version includes it (will be 3.4 or 3.5). Of course, that's not
really handy.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH] Initial support for Ilumos build and Illumos-kvm

[Andreas Färber]

Am 16.03.2012 10:23, schrieb Lee Essen:
 This fixes a number of issues with the build process (namely ensuring
 the use of bash), adds specific support for the Illumos port of KVM
 and fixes a few general Solaris compatibility issues.

 There are still some things outstanding:

 - there's a duplicate smb_wmb() definition in qemu-barrier.h and the
 illumos kvm_x86.h which generates some warnings.
 - there's a repeated call to page_size() that should probably be fixed.
 - dtrace support needs to be fixed (-m64/32 option, reserved words and
 linking issues)
 - vnics need to be added
 - the original illumos code added another timer source (multiticks)
 - the issue with Linux needs to be resolved

 Other than that, this gets it to the point where it will build and run
 with illumos kvm, and works fine for Windows.

 It's my first patch to qemu, and most of the real kvm stuff has come
 from the original illumos-kvm-cmd tree, so be gentle with me!


 Signed-off-by: Lee Essen l
 mailto:da...@gibson.dropbear.id.auee.es...@nowonline.co.uk
 mailto:ee.es...@nowonline.co.uk

Your patch is HTML-formatted. Please use git-send-email to avoid that.

It is also doing way too many things at once. Properly using existing
$(SHELL) everywhere in Makefiles could be one patch, for instance,
adding $shell in configure another, same for functional
CONFIG_SOLARIS/__sun__ changes, KVM stuff in yet another. Making the
patches smaller and confined to subsystems or aspects will make it more
reviewable, especially since different maintainers are involved in the
files you touch.

LEE - todo doesn't sound too assuring. Either write it as a proper
TODO This and that needs to be done. so that someone can address it or
send it as an [RFC] rather than a [PATCH].
If this patch introduces an issue for Linux (you don't say which?) while
adding support for illumos, it won't be acceptable in the first place. A
[PATCH] is expected to be of regression-free quality for qemu.git.

Is there any SystemTap on illumos? If not, we don't need the .stp file
at all.

Please resubmit with those issues addressed. I just cc'ed you on the C99
fix mentioned yesterday and am rebasing my queue on OpenIndiana (oi_151a).

Regards,
Andreas

Re: [Qemu-devel] [RESEND PATCH] ioapic: fix build with DEBUG_IOAPIC

[Andreas Färber]

Am 16.03.2012 10:10, schrieb Jason Wang:
 ioapic.c:198: error: format ‘%08x’ expects type ‘unsigned int’, but argument 
 3 has type ‘uint64_t’
 
 Signed-off-by: Jason Wang jasow...@redhat.com

PRIx64 is indeed needed here. However, this drops the 08 without mention
in the commit message - was it intended?

Andreas

 ---
  hw/ioapic.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/hw/ioapic.c b/hw/ioapic.c
 index 3fee011..1ff31a1 100644
 --- a/hw/ioapic.c
 +++ b/hw/ioapic.c
 @@ -195,7 +195,7 @@ ioapic_mem_write(void *opaque, target_phys_addr_t addr, 
 uint64_t val,
  if (size != 4) {
  break;
  }
 -DPRINTF(write: %08x = %08x\n, s-ioregsel, val);
 +DPRINTF(write: %08x = % PRIx64 \n, s-ioregsel, val);
  switch (s-ioregsel) {
  case IOAPIC_REG_ID:
  s-id = (val  IOAPIC_ID_SHIFT)  IOAPIC_ID_MASK;

-- 
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] [V5 PATCH 1/4] net: announce self after vm start

[Paolo Bonzini]

Il 16/03/2012 11:13, Jason Wang ha scritto:
 The problem with staying in the INMIGRATE is that we can not figure out
 when the migration is completed when using '-S', so this kind of
 transition were forbidden by qmp_cont().
 
 Looks like we need a new state such as RUN_STATE_MIGRATE_PRELAUNCH?

Or just a global need_announce instead of looking at the runstate.

Paolo

Re: [Qemu-devel] [PATCH v3 1/9] net: introduce tcp_server_start()

[Amos Kong]

On 14/03/12 22:58, Michael Roth wrote:

On Wed, Mar 14, 2012 at 04:33:14PM +0800, Amos Kong wrote:

On 14/03/12 00:39, Michael Roth wrote:

On Wed, Mar 07, 2012 at 06:47:45AM +0800, Amos Kong wrote:

Introduce tcp_server_start() by moving original code in
tcp_start_incoming_migration().

Signed-off-by: Amos Kongak...@redhat.com
---
  net.c |   28 
  qemu_socket.h |2 ++
  2 files changed, 30 insertions(+), 0 deletions(-)

+int tcp_server_start(const char *str, int *fd)
+{



I would combine this with patch 2, since it provides context for why
this function is being added. Would also do the same for 3 and 4.

I see client the client implementation you need to pass fd back by
reference since ret can be set to EINPROGRESS/EWOULDBLOCK on success,


ret restores 0 or -socket_error()
  success: 0, -EINPROGRESS
  fail   : ret  0  ret !=-EINTR  ret != -EWOULDBLOCK

, it should be -EINPROGRESS


I see, I think I was confued by patch #4 where you do a

+ret = tcp_client_start(host_port,s-fd);
+if (ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
+DPRINTF(connect in progress);
+qemu_set_fd_handler2(s-fd, NULL, NULL, tcp_wait_for_connect, s);

If ret == EWOULDBLOCK is a failure (or if the call isn't supposed to
return EWOULDBLOCK), we should fail it there rather than passing it on to
tcp_wait_for_connect().


You are right, it should be :
   if (ret == -EINPROGRESS) {


Also, is there any reason we can't re-use
qemu-sockets.c:inet_listen()/qemu-sockets.c:inet_connect()? AFAICT they
serve the same purpose, and already include some of the work from your
PATCH #6.


We could not directly use it, there are some difference,
such as tcp_start_incoming_migration() doesn't set socket no-blocked,
but net_socket_listen_init() sets socket no-blocked.


I think adding a common function with blocking/non-blocking flag and
having inet_listen_opts()/socket_listen_opts() call it with a wrapper
would be reasonable.



A lot of code is being introduced here to solve problems that are
already handled in qemu-sockets.c. inet_listen()/inet_connect()
already handles backeted-enclosed ipv6 addrs, getting port numbers when
there's more than one colon, getaddrinfo()-based connections, and most
importantly it's had ipv6 support from day 1.



Not 100% sure it'll work for what you're doing, but qemu-sockets.c was
specifically added for this type of use-case and is heavilly used
currently (vnc, nbd, Chardev users), so I think we should use it unless
there's a good reason not to.


There are many special request for migration, which is not implemented in
inet_listen_opts()/socket_listen_opts(), but many codes can be reused,
I would re-write patches.

Thanks, Amos

Re: [Qemu-devel] [PATCH v6 2/6] Introduce save_devices

[Stefano Stabellini]

On Thu, 15 Mar 2012, Anthony Liguori wrote:
  diff --git a/qapi-schema.json b/qapi-schema.json
  index d0b6792..7f938ff 100644
  --- a/qapi-schema.json
  +++ b/qapi-schema.json
  @@ -1593,3 +1593,21 @@
{ 'command': 'qom-list-types',
  'data': { '*implements': 'str', '*abstract': 'bool' },
  'returns': [ 'ObjectTypeInfo' ] }
  +
  +##
  +# @save_devices:
  +#
  +# Save the state of all devices to file. The RAM and the block devices
  +# of the VM are not saved by this command.
  +#
  +# @filename: the file to save the state of the devices to as binary
  +# data. See save_devices.txt for a description of the binary format.
  +#
  +# Returns: Nothing on success
  +#  If @filename cannot be opened, OpenFileFailed
  +#  If an I/O error occurs while writing the file, IOError
  +#
  +# Since: 1.0
 
 Since: 1.1.
 
 Otherwise Reviewed-by: Anthony Liguori aligu...@us.ibm.com

Thanks!

Re: [Qemu-devel] [PATCH v6 2/6] Introduce save_devices

[Stefano Stabellini]

On Thu, 15 Mar 2012, Luiz Capitulino wrote:
 On Thu, 15 Mar 2012 15:16:15 -0500
 Anthony Liguori anth...@codemonkey.ws wrote:
 
  On 03/15/2012 01:19 PM, Stefano Stabellini wrote:
   - add an is_ram flag to SaveStateEntry;
  
   - register_savevm_live sets is_ram for live_savevm devices;
  
   - introduce a save_devices QAPI command that can be used to save
   the state of all devices, but not the RAM or the block devices of the
   VM.
  
   Changes in v6:
  
   - remove the is_ram parameter from register_savevm_live and sets is_ram
   if the device is a live_savevm device;
  
   - introduce save_devices as a QAPI command, write a better description
   for it;
  
   - fix CODING_STYLE;
  
   - introduce a new doc to explain the save format used by save_devices.
  
   Signed-off-by: Stefano Stabellinistefano.stabell...@eu.citrix.com
   CC: Anthony Liguorianth...@codemonkey.ws
   CC: Luiz Capitulinolcapitul...@redhat.com
   ---
 docs/save_devices.txt |   33 ++
 qapi-schema.json  |   18 
 qmp-commands.hx   |   25 +
 savevm.c  |   71 
   +
 4 files changed, 147 insertions(+), 0 deletions(-)
 create mode 100644 docs/save_devices.txt
  
   diff --git a/docs/save_devices.txt b/docs/save_devices.txt
   new file mode 100644
   index 000..79915d2
   --- /dev/null
   +++ b/docs/save_devices.txt
   @@ -0,0 +1,33 @@
   += Save Devices =
   +
   +QEMU has code to load/save the state of the guest that it is running.
   +These are two complementary operations.  Saving the state just does
   +that, saves the state for each device that the guest is running.
   +
   +These operations are normally used with migration (see migration.txt),
   +however it is also possible to save the state of all devices to file,
   +without saving the RAM or the block devices of the VM.
   +
   +This operation is called save_devices (see QMP/qmp-commands.txt).
   +
   +
   +The binary format used in the file is the following:
   +
   +
   +---
   +
   +32 bit big endian: QEMU_VM_FILE_MAGIC
   +32 bit big endian: QEMU_VM_FILE_VERSION
   +
   +for_each_device
   +{
   +8 bit:  QEMU_VM_SECTION_FULL
   +32 bit big endian:  section_id
   +8 bit:  idstr (ID string) length
   +string: idstr (ID string)
   +32 bit big endian:  instance_id
   +32 bit big endian:  version_id
   +buffer: device specific data
   +}
   +
   +8 bit: QEMU_VM_EOF
   diff --git a/qapi-schema.json b/qapi-schema.json
   index d0b6792..7f938ff 100644
   --- a/qapi-schema.json
   +++ b/qapi-schema.json
   @@ -1593,3 +1593,21 @@
 { 'command': 'qom-list-types',
   'data': { '*implements': 'str', '*abstract': 'bool' },
   'returns': [ 'ObjectTypeInfo' ] }
   +
   +##
   +# @save_devices:
   +#
   +# Save the state of all devices to file. The RAM and the block devices
   +# of the VM are not saved by this command.
   +#
   +# @filename: the file to save the state of the devices to as binary
   +# data. See save_devices.txt for a description of the binary format.
   +#
   +# Returns: Nothing on success
   +#  If @filename cannot be opened, OpenFileFailed
   +#  If an I/O error occurs while writing the file, IOError
   +#
   +# Since: 1.0
  
  Since: 1.1.
  
  Otherwise Reviewed-by: Anthony Liguori aligu...@us.ibm.com
 
 Looks fine to me FWIW, my only nitpick is that we use an hyphen instead of the
 underline in qmp command names, but I'd call this save-devices-state.

OK, I'll rename and resend.


 Don't you want this in HMP too, btw?

It is basically useless because the data saved by save_devices needs to
be packet together with other data before any tools can actually use it.

Re: [Qemu-devel] lsi_scsi: error: ORDERED queue not implemented

[Paolo Bonzini]

Il 15/03/2012 23:30, Marius Cirsta ha scritto:
 
 qemu-system-arm --version
 QEMU emulator version 1.0,1, Copyright (c) 2003-2008 Fabrice Bellard
 
 and I have a premade ARM image which I start with:
 
 qemu-system-arm -M versatilepb -m 256 -kernel
 vmlinuz-3.1-versatile-fw5 -hda initrd-arm.img -append root=/dev/sda
 ro quiet
 
 
 Every now and then I get this:
 
 lsi_scsi: error: ORDERED queue not implemented

This is only a heuristic in the Linux LSI driver.  Its lack is harmless,
in fact the feature is not anymore required by SCSI standards.

Paolo

Re: [Qemu-devel] [PATCH v4 9/9] VMXNET3 paravirtualized device integration. Interface type vmxnet3 added.

[Paolo Bonzini]

Il 15/03/2012 22:09, Dmitry Fleytman ha scritto:
 Signed-off-by: Dmitry Fleytman dmi...@daynix.com
 Signed-off-by: Yan Vugenfirer y...@daynix.com
 ---
  Makefile.objs   |1 +
  default-configs/pci.mak |1 +
  hw/pci.c|2 ++
  hw/pci.h|1 +
  net.c   |2 +-
  5 files changed, 6 insertions(+), 1 deletions(-)
 
 diff --git a/Makefile.objs b/Makefile.objs
 index 226b01d..1366e86 100644
 --- a/Makefile.objs
 +++ b/Makefile.objs
 @@ -284,6 +284,7 @@ hw-obj-$(CONFIG_PCNET_PCI) += pcnet-pci.o
  hw-obj-$(CONFIG_PCNET_COMMON) += pcnet.o
  hw-obj-$(CONFIG_E1000_PCI) += e1000.o
  hw-obj-$(CONFIG_RTL8139_PCI) += rtl8139.o
 +hw-obj-$(CONFIG_VMXNET3_PCI) += vmxnet3.o vmxnet_utils.o vmxnet_pkt.o
  
  hw-obj-$(CONFIG_SMC91C111) += smc91c111.o
  hw-obj-$(CONFIG_LAN9118) += lan9118.o
 diff --git a/default-configs/pci.mak b/default-configs/pci.mak
 index 21e4ccf..f8e6ee1 100644
 --- a/default-configs/pci.mak
 +++ b/default-configs/pci.mak
 @@ -13,6 +13,7 @@ CONFIG_PCNET_COMMON=y
  CONFIG_LSI_SCSI_PCI=y
  CONFIG_RTL8139_PCI=y
  CONFIG_E1000_PCI=y
 +CONFIG_VMXNET3_PCI=y
  CONFIG_IDE_CORE=y
  CONFIG_IDE_QDEV=y
  CONFIG_IDE_PCI=y

These parts should be included in part 8.

Paolo

 diff --git a/hw/pci.c b/hw/pci.c
 index 9146d3f..e2b0045 100644
 --- a/hw/pci.c
 +++ b/hw/pci.c
 @@ -1355,6 +1355,7 @@ static const char * const pci_nic_models[] = {
  e1000,
  pcnet,
  virtio,
 +vmxnet3,
  NULL
  };
  
 @@ -1367,6 +1368,7 @@ static const char * const pci_nic_names[] = {
  e1000,
  pcnet,
  virtio-net-pci,
 +vmxnet3,
  NULL
  };
  
 diff --git a/hw/pci.h b/hw/pci.h
 index 4f19fdb..fee8250 100644
 --- a/hw/pci.h
 +++ b/hw/pci.h
 @@ -60,6 +60,7 @@
  #define PCI_DEVICE_ID_VMWARE_NET 0x0720
  #define PCI_DEVICE_ID_VMWARE_SCSI0x0730
  #define PCI_DEVICE_ID_VMWARE_IDE 0x1729
 +#define PCI_DEVICE_ID_VMWARE_VMXNET3 0x07B0
  
  /* Intel (0x8086) */
  #define PCI_DEVICE_ID_INTEL_82551IT  0x1209
 diff --git a/net.c b/net.c
 index c34474f..e2f586c 100644
 --- a/net.c
 +++ b/net.c
 @@ -857,7 +857,7 @@ static const struct {
  }, {
  .name = model,
  .type = QEMU_OPT_STRING,
 -.help = device model (e1000, rtl8139, virtio etc.),
 +.help = device model (e1000, rtl8139, virtio, vmxnet3 
 etc.),
  }, {
  .name = addr,
  .type = QEMU_OPT_STRING,

Re: [Qemu-devel] [PATCH] Initial support for Ilumos build and Illumos-kvm

[Paolo Bonzini]

Il 16/03/2012 10:23, Lee Essen ha scritto:
 diff --git a/Makefile.objs b/Makefile.objs
 index 226b01d..c2a440a 100644
 --- a/Makefile.objs
 +++ b/Makefile.objs
 @@ -373,12 +373,12 @@ else
  trace.h: trace.h-timestamp
  endif
  trace.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
 -   $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool
 --$(TRACE_BACKEND) -h  $  $@,  GEN   trace.h)
 +   $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/tracetool
 --$(TRACE_BACKEND) -h  $  $@,  GEN   trace.h)
 @cmp -s $@ trace.h || cp $@ trace.h
  
  trace.c: trace.c-timestamp
  trace.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
 -   $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool
 --$(TRACE_BACKEND) -c  $  $@,  GEN   trace.c)
 +   $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/tracetool
 --$(TRACE_BACKEND) -c  $  $@,  GEN   trace.c)
 @cmp -s $@ trace.c || cp $@ trace.c
  
  trace.o: trace.c $(GENERATED_HEADERS)
 @@ -391,7 +391,7 @@ trace-dtrace.h: trace-dtrace.dtrace
  # rule file. So we use '.dtrace' instead
  trace-dtrace.dtrace: trace-dtrace.dtrace-timestamp
  trace-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events
 $(BUILD_DIR)/config-host.mak
 -   $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool
 --$(TRACE_BACKEND) -d  $  $@,  GEN   trace-dtrace.dtrace)
 +   $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/tracetool
 --$(TRACE_BACKEND) -d  $  $@,  GEN   trace-dtrace.dtrace)
 @cmp -s $@ trace-dtrace.dtrace || cp $@ trace-dtrace.dtrace
  
  trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS)
 diff --git a/Makefile.target b/Makefile.target
 index eb25941..d32afc9 100644
 --- a/Makefile.target
 +++ b/Makefile.target
 @@ -59,7 +59,7 @@ TARGET_TYPE=system
  endif
  
  $(QEMU_PROG).stp:
 -   $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool \
 +   $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/tracetool \
 --$(TRACE_BACKEND) \
 --binary $(bindir)/$(QEMU_PROG) \
 --target-arch $(TARGET_ARCH) \
 @@ -443,10 +443,10 @@ gdbstub-xml.c: $(TARGET_XML_FILES)
 $(SRC_PATH)/scripts/feature_to_c.sh
 $(call quiet-command,rm -f $@  $(SHELL)
 $(SRC_PATH)/scripts/feature_to_c.sh $@ $(TARGET_XML_FILES),  GEN  
 $(TARGET_DIR)$@)
  
  hmp-commands.h: $(SRC_PATH)/hmp-commands.hx
 -   $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h  $ 
 $@,  GEN   $(TARGET_DIR)$@)
 +   $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/hxtool -h  $
 $@,  GEN   $(TARGET_DIR)$@)
  
  qmp-commands-old.h: $(SRC_PATH)/qmp-commands.hx
 -   $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h  $ 
 $@,  GEN   $(TARGET_DIR)$@)
 +   $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/hxtool -h  $
 $@,  GEN   $(TARGET_DIR)$@)
  
  clean:
 rm -f *.o *.a *~ $(PROGS) nwfpe/*.o fpu/*.o
 diff --git a/configure b/configure
 index afe7395..601f77a 100755
 --- a/configure
 +++ b/configure
 @@ -101,6 +101,7 @@ audio_win_int=
  cc_i386=i386-pc-linux-gnu-gcc
  libs_qga=
  debug_info=yes
 +shell=sh
  
  target_list=
  
 @@ -442,6 +443,7 @@ SunOS)
# have to select again, because `uname -m` returns i86pc
# even on an x86_64 box.
solariscpu=`isainfo -k`
 +  shell=bash
if test ${solariscpu} = amd64 ; then
  cpu=x86_64
fi
 @@ -471,6 +473,7 @@ SunOS)
QEMU_CFLAGS=-D__EXTENSIONS__ $QEMU_CFLAGS
QEMU_CFLAGS=-std=gnu99 $QEMU_CFLAGS
LIBS=-lsocket -lnsl -lresolv $LIBS
 +  libs_qga=-lsocket -lxnet $lib_qga
  ;;
  AIX)
aix=yes
 @@ -1097,7 +1100,7 @@ echo   --disable-docs   disable
 documentation build
  echo   --disable-vhost-net  disable vhost-net acceleration support
  echo   --enable-vhost-net   enable vhost-net acceleration support
  echo   --enable-trace-backend=B Set trace backend
 -echoAvailable backends:
 $($source_path/scripts/tracetool --list-backends)
 +echoAvailable backends: $($shell
 $source_path/scripts/tracetool --list-backends)
  echo   --with-trace-file=NAME   Full PATH,NAME of file to store traces
  echoDefault:trace-pid
  echo   --disable-spice  disable spice
 @@ -2654,7 +2657,7 @@ fi
  ##
  # check if trace backend exists
  
 -sh $source_path/scripts/tracetool --$trace_backend --check-backend
 /dev/null 2 /dev/null
 +$shell $source_path/scripts/tracetool --$trace_backend
 --check-backend  /dev/null 2 /dev/null
  if test $? -ne 0 ; then
echo
echo Error: invalid trace backend
 @@ -3358,6 +3361,7 @@ echo LIBS+=$LIBS  $config_host_mak
  echo LIBS_TOOLS+=$libs_tools  $config_host_mak
  echo EXESUF=$EXESUF  $config_host_mak
  echo LIBS_QGA+=$libs_qga  $config_host_mak
 +echo SHELL=$shell  $config_host_mak
  
  # generate list of library paths for linker script
  
 diff --git a/cpus.c b/cpus.c

Everything up to here should be a separate patch, but it looks sane.

 index 25ba621..7a32ee6 

[Qemu-devel] [PATCH v7 0/6] save/restore on Xen

[Stefano Stabellini]

Hi all,
this is the seventh version of the Xen save/restore patch series.
We have been discussing this issue for quite a while on #qemu and
qemu-devel:


http://marc.info/?l=qemu-develm=132346828427314w=2
http://marc.info/?l=qemu-develm=132377734605464w=2


Please review the second patch: Introduce save_devices.


Changes in v7:

- rename save_devices to save-devices-state.


Changes in v6:

- remove the is_ram parameter from register_savevm_live and sets is_ram
if the device is a live_savevm device;  
  

- introduce save_devices as a QAPI command, write a better description
for it; 
  

- fix CODING_STYLE; 
  

- introduce a new doc to explain the save format used by save_devices.  
  


Changes in v5:

- rebased on b4bd0b168e9f4898b98308f4a8a089f647a86d16.


Changes in v4:

- following Anthony's suggestion I have introduced a new monitor command
to save the non-ram device state to file;

- I have also removed the hack not to reset the cirrus videoram on
restore, because it turns out that the videoram doesn't need to be
reset in the reset handler at all (tested on Win2K, where the problem
was found in the first place).



This is the list of patches with a diffstat:

Anthony PERARD (2):
  xen mapcache: check if memory region has moved.
  xen: do not allocate RAM during INMIGRATE runstate

Stefano Stabellini (4):
  cirrus_vga: do not reset videoram
  Introduce save-devices-state
  Set runstate to INMIGRATE earlier
  xen: record physmap changes to xenstore

 docs/save-devices-state.txt |   33 ++
 hw/cirrus_vga.c |4 --
 qapi-schema.json|   18 +++
 qmp-commands.hx |   25 ++
 savevm.c|   71 +
 vl.c|2 +-
 xen-all.c   |  104 ++-
 xen-mapcache.c  |   22 -
 xen-mapcache.h  |9 +++-
 9 files changed, 276 insertions(+), 12 deletions(-)


git://xenbits.xen.org/people/sstabellini/qemu-dm.git saverestore-7

Cheers,

Stefano

[Qemu-devel] [PATCH 1/5] configure to set shell type

[Lee Essen]

Adds support to configure for controlling which shell to use, defaults to sh 
as before
but adds bash for Solaris/Illumos builds. Plus ensures that tracetool is 
called with a
shell.

Signed-off-by: Lee Essen lee.es...@nowonline.co.uk

--

 configure |7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index afe7395..860c15d 100755
--- a/configure
+++ b/configure
@@ -101,6 +101,7 @@ audio_win_int=
 cc_i386=i386-pc-linux-gnu-gcc
 libs_qga=
 debug_info=yes
+shell=sh
 
 target_list=
 
@@ -442,6 +443,7 @@ SunOS)
   # have to select again, because `uname -m` returns i86pc
   # even on an x86_64 box.
   solariscpu=`isainfo -k`
+  shell=bash
   if test ${solariscpu} = amd64 ; then
 cpu=x86_64
   fi
@@ -1097,7 +1099,7 @@ echo   --disable-docs   disable documentation 
build
 echo   --disable-vhost-net  disable vhost-net acceleration support
 echo   --enable-vhost-net   enable vhost-net acceleration support
 echo   --enable-trace-backend=B Set trace backend
-echoAvailable backends: 
$($source_path/scripts/tracetool --list-backends)
+echoAvailable backends: $($shell 
$source_path/scripts/tracetool --list-backends)
 echo   --with-trace-file=NAME   Full PATH,NAME of file to store traces
 echoDefault:trace-pid
 echo   --disable-spice  disable spice
@@ -2654,7 +2656,7 @@ fi
 ##
 # check if trace backend exists
 
-sh $source_path/scripts/tracetool --$trace_backend --check-backend  
/dev/null 2 /dev/null
+$shell $source_path/scripts/tracetool --$trace_backend --check-backend  
/dev/null 2 /dev/null
 if test $? -ne 0 ; then
   echo
   echo Error: invalid trace backend
@@ -3358,6 +3360,7 @@ echo LIBS+=$LIBS  $config_host_mak
 echo LIBS_TOOLS+=$libs_tools  $config_host_mak
 echo EXESUF=$EXESUF  $config_host_mak
 echo LIBS_QGA+=$libs_qga  $config_host_mak
+echo SHELL=$shell  $config_host_mak
 
 # generate list of library paths for linker script
 

[Qemu-devel] [PATCH v7 1/6] cirrus_vga: do not reset videoram

[Stefano Stabellini]

There is no need to set the videoram to 0xff in cirrus_reset, because it
is the BIOS' job.

Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
Reviewed-by: Avi Kivity a...@redhat.com
---
 hw/cirrus_vga.c |4 
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 4edcb94..afedaa4 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2767,10 +2767,6 @@ static void cirrus_reset(void *opaque)
 }
 s-vga.cr[0x27] = s-device_id;
 
-/* Win2K seems to assume that the pattern buffer is at 0xff
-   initially ! */
-memset(s-vga.vram_ptr, 0xff, s-real_vram_size);
-
 s-cirrus_hidden_dac_lockindex = 5;
 s-cirrus_hidden_dac_data = 0;
 }
-- 
1.7.2.5

[Qemu-devel] [PATCH v7 6/6] xen: do not allocate RAM during INMIGRATE runstate

[Stefano Stabellini]

From: Anthony PERARD anthony.per...@citrix.com

Signed-off-by: Anthony PERARD anthony.per...@citrix.com
Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
---
 xen-all.c |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/xen-all.c b/xen-all.c
index 972cffd..10d53d1 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -190,6 +190,14 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, 
MemoryRegion *mr)
 xen_pfn_t *pfn_list;
 int i;
 
+if (runstate_check(RUN_STATE_INMIGRATE)) {
+/* RAM already populated in Xen */
+fprintf(stderr, %s: do not alloc RAM_ADDR_FMT
+ bytes of ram at RAM_ADDR_FMT when runstate is INMIGRATE\n,
+__func__, size, ram_addr); 
+return;
+}
+
 if (mr == ram_memory) {
 return;
 }
-- 
1.7.2.5

[Qemu-devel] [PATCH v7 5/6] xen mapcache: check if memory region has moved.

[Stefano Stabellini]

From: Anthony PERARD anthony.per...@citrix.com

This patch changes the xen_map_cache behavior. Before trying to map a guest
addr, mapcache will look into the list of range of address that have been moved
(physmap/set_memory). There is currently one memory space like this, the vram,
moved from were it's allocated to were the guest will look into.

This help to have a succefull migration.

Signed-off-by: Anthony PERARD anthony.per...@citrix.com
Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
---
 xen-all.c  |   18 +-
 xen-mapcache.c |   22 +++---
 xen-mapcache.h |9 +++--
 3 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/xen-all.c b/xen-all.c
index f2cad82..972cffd 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -225,6 +225,22 @@ static XenPhysmap *get_physmapping(XenIOState *state,
 return NULL;
 }
 
+static target_phys_addr_t xen_phys_offset_to_gaddr(target_phys_addr_t 
start_addr,
+   ram_addr_t size, void 
*opaque)
+{
+target_phys_addr_t addr = start_addr  TARGET_PAGE_MASK;
+XenIOState *xen_io_state = opaque;
+XenPhysmap *physmap = NULL;
+
+QLIST_FOREACH(physmap, xen_io_state-physmap, list) {
+if (range_covers_byte(physmap-phys_offset, physmap-size, addr)) {
+return physmap-start_addr;
+}
+}
+
+return start_addr;
+}
+
 #if CONFIG_XEN_CTRL_INTERFACE_VERSION = 340
 static int xen_add_to_physmap(XenIOState *state,
   target_phys_addr_t start_addr,
@@ -1043,7 +1059,7 @@ int xen_hvm_init(void)
 }
 
 /* Init RAM management */
-xen_map_cache_init();
+xen_map_cache_init(xen_phys_offset_to_gaddr, state);
 xen_ram_init(ram_size);
 
 qemu_add_vm_change_state_handler(xen_hvm_change_state_handler, state);
diff --git a/xen-mapcache.c b/xen-mapcache.c
index 585b559..a456479 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -78,6 +78,9 @@ typedef struct MapCache {
 uint8_t *last_address_vaddr;
 unsigned long max_mcache_size;
 unsigned int mcache_bucket_shift;
+
+phys_offset_to_gaddr_t phys_offset_to_gaddr;
+void *opaque;
 } MapCache;
 
 static MapCache *mapcache;
@@ -91,13 +94,16 @@ static inline int test_bits(int nr, int size, const 
unsigned long *addr)
 return 0;
 }
 
-void xen_map_cache_init(void)
+void xen_map_cache_init(phys_offset_to_gaddr_t f, void *opaque)
 {
 unsigned long size;
 struct rlimit rlimit_as;
 
 mapcache = g_malloc0(sizeof (MapCache));
 
+mapcache-phys_offset_to_gaddr = f;
+mapcache-opaque = opaque;
+
 QTAILQ_INIT(mapcache-locked_entries);
 mapcache-last_address_index = -1;
 
@@ -193,9 +199,14 @@ uint8_t *xen_map_cache(target_phys_addr_t phys_addr, 
target_phys_addr_t size,
uint8_t lock)
 {
 MapCacheEntry *entry, *pentry = NULL;
-target_phys_addr_t address_index  = phys_addr  MCACHE_BUCKET_SHIFT;
-target_phys_addr_t address_offset = phys_addr  (MCACHE_BUCKET_SIZE - 1);
+target_phys_addr_t address_index;
+target_phys_addr_t address_offset;
 target_phys_addr_t __size = size;
+bool translated = false;
+
+tryagain:
+address_index  = phys_addr  MCACHE_BUCKET_SHIFT;
+address_offset = phys_addr  (MCACHE_BUCKET_SIZE - 1);
 
 trace_xen_map_cache(phys_addr);
 
@@ -237,6 +248,11 @@ uint8_t *xen_map_cache(target_phys_addr_t phys_addr, 
target_phys_addr_t size,
 if(!test_bits(address_offset  XC_PAGE_SHIFT, size  XC_PAGE_SHIFT,
 entry-valid_mapping)) {
 mapcache-last_address_index = -1;
+if (!translated  mapcache-phys_offset_to_gaddr) {
+phys_addr = mapcache-phys_offset_to_gaddr(phys_addr, size, 
mapcache-opaque);
+translated = true;
+goto tryagain;
+}
 trace_xen_map_cache_return(NULL);
 return NULL;
 }
diff --git a/xen-mapcache.h b/xen-mapcache.h
index da874ca..70301a5 100644
--- a/xen-mapcache.h
+++ b/xen-mapcache.h
@@ -11,9 +11,13 @@
 
 #include stdlib.h
 
+typedef target_phys_addr_t (*phys_offset_to_gaddr_t)(target_phys_addr_t 
start_addr,
+ ram_addr_t size,
+ void *opaque);
 #ifdef CONFIG_XEN
 
-void xen_map_cache_init(void);
+void xen_map_cache_init(phys_offset_to_gaddr_t f,
+void *opaque);
 uint8_t *xen_map_cache(target_phys_addr_t phys_addr, target_phys_addr_t size,
uint8_t lock);
 ram_addr_t xen_ram_addr_from_mapcache(void *ptr);
@@ -22,7 +26,8 @@ void xen_invalidate_map_cache(void);
 
 #else
 
-static inline void xen_map_cache_init(void)
+static inline void xen_map_cache_init(phys_offset_to_gaddr_t f,
+  void *opaque)
 {
 }
 
-- 
1.7.2.5

Re: [Qemu-devel] [PATCHv4 00/11] cleanup/consolidate iovec functions

[Paolo Bonzini]

Il 15/03/2012 22:00, Michael Tokarev ha scritto:
 This is cleanup/consolidation of iovec-related low-level
 routines in qemu.
 
 The plan is to make library functions more understandable,
 consistent and useful, and to drop numerous implementations
 of the same thing.
 
 The patch changes prototypes of several iov and qiov functions
 to match each other, changes types of arguments for some
 functions, _swaps_ some function arguments with each other,
 and makes use of common code in r/w path.
 
 The result of all these changes.
 
 1. Most qiov-related (qemu_iovec_*) functions now accepts
  'offset' parameter to specify from which (byte) position to
  start operation.  This is added for _memset (removing
  _memset_skip), _from_buffer (allowing to copy a bounce-
  buffer to a middle of qiov).  Typical:
 
   size_t qemu_iovec_memset(QEMUIOVector *qiov, size_t offset,
int c, size_t bytes);
 
 2. All functions that accepts this `offset' argument does
  it in a similar manner, following the
iov, fromwhere, what, bytes
  pattern.  This is consistent with (updated) iov_send()
  and iov_recv() and friends, where `offset' and `bytes'
  arguments were _renamed_, with the following prototypes:
 
ssize_t iov_send(sockfd, iov, iov_cnt size_t offset, size_t bytes)
 
  instead of
 
int qemu_sendv(sockfd, iov, int len, int iov_offset)
 
  See how offset  bytes are used in the same way as for iov_*
  and qemu_iovec_*.   A few callers of these are verified and
  converted.
 
 3. Always use iov_cnt (number of iov entries) together with
   iov, to be able to verify that we don't go past the array.
   iov_send (former qemu_sendv) and iov_recv accepts one extra
   argument now, as are all other derivates (qemu_co_sendv etc).
 
 4. Used size_t instead of various variations for byte counts.
  Including qemu_iovec_copy which used uint64_t(!) type.
 
 5. Function arguments are renamed to better match with their
  actual meaning.  Compare new and original prototype of
  qemu_sendv() above: old prototype with `len' does not
  tell if `len' refers to number of iov elements (as
  regular writev() call) or to number of data bytes.
  Ditto for several usages of `count' for some qemu_iovec_*,
  which is also replaced to `bytes'.
 
 5. One implementation of the code remain. For example,
  qemu_iovec_from_buffer() uses iov_from_buf() directly,
  instead of repeating the same code.
 
 The resulting function usage is much more consistent, the
 functions themselves are nice and understandable, which
 means they're easier to use and less error-prone.
 
 This patchset also consolidates a few low-level sendrecv
 functions into one, since both versions were the same
 (and were finally calling common function anyway).
 This is done by exporting a common send_recv function
 with one extra bool argument, and making current sendrecv
 to be just #defines.
 
 And while at it all, also made some implementations shorter,
 cleaner and much easier to read/understand, and add some
 code comments.
 
 The readwrite consolidation has great potential for the
 block layer, as has been demonstrated before.
 Unification and generalization of qemu_iovec_* functions
 will let to optimize/simplify some more code in block/*,
 especially qemu_iovec_memset() and _from_buffer() (this
 optimization/simplification is already used in qcow2.c
 a bit).
 
 The resulting thing should behave like current/existing
 code, there should be no behavor changes, just some
 shuffling and a bit of sanity checks.  It has been tested
 slightly, by booting different guests out of different
 image formats and doing some i/o, after each of the 11
 patches, and it all works correctly so far.
 
 Changes since v3:
 
 - rename qemu_sendv(), qemu_sendv_recvv() to iov_send(),
   iov_send_recv() and move them from qemu-common.h and
   cutils.c to iov.h and iov.c.
 - add new argument, iov_cnt, to all send/recv-related
   functions (iov_send_recv(), qemu_co_sendv_recv() etc).
   This resulted in a bit more changes in other places,
   some of which, while simple, may still be non-obvious
   (block/nbd.c and block/sheepdog.c).
   Due to the rename above and to introduction of the
   new iov_cnt arg, the function signatures are changed
   so no old code using new function wrongly is possible.
 - patch that changes iov_from_buf()  iov_to_buf() is
   split into two halves: prototype changes and rewrite.
 - rewrite iov_send_recv() (former qemu_sendv_recvv())
   again, slightly, to use newly introduced iov_cnt arg.
 
 Changes since v2:
 
 - covered iov.[ch] with iov_*() functions which contained
   similar functionality
 - changed tabs to spaces as per suggestion by Kevin
 - explicitly allow to use large sizes for frombuf/tobuf
   functions and friends, stopping at the end of iovec
   in case more bytes requested when available, and
   _returning_ number of actual bytes processed, but
   made it extra clear what a return value will be.
 - used ssize_t for sendv/recvv 

[Qemu-devel] [PATCH 2/5] use SHELL in Makefiles

[Lee Essen]

Use the SHELL macro (set in configure) to ensure tracetool and hxtool are 
correctly called.

Signed-off-by: Lee Essen lee.es...@nowonline.co.uk

---

 Makefile.objs   |6 +++---
 Makefile.target |6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index 226b01d..c2a440a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -373,12 +373,12 @@ else
 trace.h: trace.h-timestamp
 endif
 trace.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
-   $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool 
--$(TRACE_BACKEND) -h  $  $@,  GEN   trace.h)
+   $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/tracetool 
--$(TRACE_BACKEND) -h  $  $@,  GEN   trace.h
@cmp -s $@ trace.h || cp $@ trace.h
 
 trace.c: trace.c-timestamp
 trace.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
-   $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool 
--$(TRACE_BACKEND) -c  $  $@,  GEN   trace.c)
+   $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/tracetool 
--$(TRACE_BACKEND) -c  $  $@,  GEN   trace.c
@cmp -s $@ trace.c || cp $@ trace.c
 
 trace.o: trace.c $(GENERATED_HEADERS)
@@ -391,7 +391,7 @@ trace-dtrace.h: trace-dtrace.dtrace
 # rule file. So we use '.dtrace' instead
 trace-dtrace.dtrace: trace-dtrace.dtrace-timestamp
 trace-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events 
$(BUILD_DIR)/config-host.mak
-   $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool 
--$(TRACE_BACKEND) -d  $  $@,  GEN   trace-dtrace.d
+   $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/tracetool 
--$(TRACE_BACKEND) -d  $  $@,  GEN   trace-dt
@cmp -s $@ trace-dtrace.dtrace || cp $@ trace-dtrace.dtrace
 
 trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS)
diff --git a/Makefile.target b/Makefile.target
index eb25941..d32afc9 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -59,7 +59,7 @@ TARGET_TYPE=system
 endif
 
 $(QEMU_PROG).stp:
-   $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool \
+   $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/tracetool \
--$(TRACE_BACKEND) \
--binary $(bindir)/$(QEMU_PROG) \
--target-arch $(TARGET_ARCH) \
@@ -443,10 +443,10 @@ gdbstub-xml.c: $(TARGET_XML_FILES) 
$(SRC_PATH)/scripts/feature_to_c.sh
$(call quiet-command,rm -f $@  $(SHELL) 
$(SRC_PATH)/scripts/feature_to_c.sh $@ $(TARGET_XML_FILES),  GEN  
 
 hmp-commands.h: $(SRC_PATH)/hmp-commands.hx
-   $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h  $  $@,  GEN  
 $(TARGET_DIR)$@)
+   $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/hxtool -h  $  $@, 
 GEN   $(TARGET_DIR)$@)
 
 qmp-commands-old.h: $(SRC_PATH)/qmp-commands.hx
-   $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h  $  $@,  GEN  
 $(TARGET_DIR)$@)
+   $(call quiet-command,$(SHELL) $(SRC_PATH)/scripts/hxtool -h  $  $@, 
 GEN   $(TARGET_DIR)$@)
 
 clean:
rm -f *.o *.a *~ $(PROGS) nwfpe/*.o fpu/*.o

Re: [Qemu-devel] [PATCH 1/5] configure to set shell type

[Andreas Färber]

Am 16.03.2012 13:02, schrieb Lee Essen:
 Adds support to configure for controlling which shell to use, defaults to 
 sh as before
 but adds bash for Solaris/Illumos builds. Plus ensures that tracetool is 
 called with a
 shell.
 
 Signed-off-by: Lee Essen lee.es...@nowonline.co.uk
 
 --
 
  configure |7 +--
  1 files changed, 5 insertions(+), 2 deletions(-)
 
 diff --git a/configure b/configure
 index afe7395..860c15d 100755
 --- a/configure
 +++ b/configure
 @@ -101,6 +101,7 @@ audio_win_int=
  cc_i386=i386-pc-linux-gnu-gcc
  libs_qga=
  debug_info=yes
 +shell=sh

This looks reasonable.

  
  target_list=
  
 @@ -442,6 +443,7 @@ SunOS)
# have to select again, because `uname -m` returns i86pc
# even on an x86_64 box.
solariscpu=`isainfo -k`
 +  shell=bash

Are you sure this is safe for Solaris 9+? In
https://bugs.launchpad.net/qemu/+bug/636315 we concluded that
/usr/xpg4/bin/sh were the best choice of POSIX-compliant shell provided
on Solaris 10. Blue's script fixes were never applied I believe...

if test ${solariscpu} = amd64 ; then
  cpu=x86_64
fi
 @@ -1097,7 +1099,7 @@ echo   --disable-docs   disable documentation 
 build
  echo   --disable-vhost-net  disable vhost-net acceleration support
  echo   --enable-vhost-net   enable vhost-net acceleration support
  echo   --enable-trace-backend=B Set trace backend
 -echoAvailable backends: 
 $($source_path/scripts/tracetool --list-backends)
 +echoAvailable backends: $($shell 
 $source_path/scripts/tracetool --list-backends)
  echo   --with-trace-file=NAME   Full PATH,NAME of file to store traces
  echoDefault:trace-pid
  echo   --disable-spice  disable spice
 @@ -2654,7 +2656,7 @@ fi
  ##
  # check if trace backend exists
  
 -sh $source_path/scripts/tracetool --$trace_backend --check-backend  
 /dev/null 2 /dev/null
 +$shell $source_path/scripts/tracetool --$trace_backend --check-backend  
 /dev/null 2 /dev/null
  if test $? -ne 0 ; then
echo
echo Error: invalid trace backend
 @@ -3358,6 +3360,7 @@ echo LIBS+=$LIBS  $config_host_mak
  echo LIBS_TOOLS+=$libs_tools  $config_host_mak
  echo EXESUF=$EXESUF  $config_host_mak
  echo LIBS_QGA+=$libs_qga  $config_host_mak
 +echo SHELL=$shell  $config_host_mak

Why?

  
  # generate list of library paths for linker script
  

Andreas

Re: [Qemu-devel] [PATCH 1/5] configure to set shell type

[Peter Maydell]

On 16 March 2012 12:02, Lee Essen lee.es...@nowonline.co.uk wrote:
 Adds support to configure for controlling which shell to use, defaults to 
 sh as before
 but adds bash for Solaris/Illumos builds. Plus ensures that tracetool is 
 called with a
 shell.

Ugh. If we have bashisms in our shell scripts/configure/makefiles etc we should
fix them, not paper over them.

If Solaris' /bin/sh isn't a POSIX sh that's a bug in Solaris :-)

 -echo                            Available backends: 
 $($source_path/scripts/tracetool --list-backends)
 +echo                            Available backends: $($shell 
 $source_path/scripts/tracetool --list-backends)

This shouldn't be necessary -- tracetool has a #!/bin/sh at the top.
If it needs bash then that should be fixed.

 -sh $source_path/scripts/tracetool --$trace_backend --check-backend  
 /dev/null 2 /dev/null
 +$shell $source_path/scripts/tracetool --$trace_backend --check-backend  
 /dev/null 2 /dev/null

...and we shouldn't need to use either 'sh' or '$shell' here...

-- PMM

Re: [Qemu-devel] [PATCH 1/5] configure to set shell type

[Lee Essen]

On 16 Mar 2012, at 12:14, Andreas Färber wrote:

 Am 16.03.2012 13:02, schrieb Lee Essen:
 
 target_list=
 
 @@ -442,6 +443,7 @@ SunOS)
   # have to select again, because `uname -m` returns i86pc
   # even on an x86_64 box.
   solariscpu=`isainfo -k`
 +  shell=bash
 
 Are you sure this is safe for Solaris 9+? In
 https://bugs.launchpad.net/qemu/+bug/636315 we concluded that
 /usr/xpg4/bin/sh were the best choice of POSIX-compliant shell provided
 on Solaris 10. Blue's script fixes were never applied I believe…
 

# /usr/xpg4/bin/sh scripts/tracetool 
scripts/tracetool: line 113: syntax error at line 126: `)' unexpected

tracetool seems to require bash … I'm happy to look at resolving that, perhaps 
thats a better fix.

 
 +echo SHELL=$shell  $config_host_mak
 
 Why?
 

See patch 2/7 … there are several calls to tracetool that hardcode sh in the 
Makefile … again if
the better solution is removing the bash requirement then this problem goes 
away completely.

Lee.

[Qemu-devel] [PATCH v7 3/6] Set runstate to INMIGRATE earlier

[Stefano Stabellini]

Set runstate to RUN_STATE_INMIGRATE as soon as we can on resume.

Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
Acked-by: Luiz Capitulino lcapitul...@redhat.com
---
 vl.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/vl.c b/vl.c
index 1d4c350..918177a 100644
--- a/vl.c
+++ b/vl.c
@@ -3099,6 +3099,7 @@ int main(int argc, char **argv, char **envp)
 break;
 case QEMU_OPTION_incoming:
 incoming = optarg;
+runstate_set(RUN_STATE_INMIGRATE);
 break;
 case QEMU_OPTION_nodefaults:
 default_serial = 0;
@@ -3596,7 +3597,6 @@ int main(int argc, char **argv, char **envp)
 }
 
 if (incoming) {
-runstate_set(RUN_STATE_INMIGRATE);
 int ret = qemu_start_incoming_migration(incoming);
 if (ret  0) {
 fprintf(stderr, Migration failed. Exit code %s(%d), exiting.\n,
-- 
1.7.2.5

Re: [Qemu-devel] [PATCH 1/5] configure to set shell type

[Lee Essen]

On 16 Mar 2012, at 12:20, Lee Essen wrote:

 
 On 16 Mar 2012, at 12:14, Andreas Färber wrote:
 
 Am 16.03.2012 13:02, schrieb Lee Essen:
 
 target_list=
 
 @@ -442,6 +443,7 @@ SunOS)
  # have to select again, because `uname -m` returns i86pc
  # even on an x86_64 box.
  solariscpu=`isainfo -k`
 +  shell=bash
 
 Are you sure this is safe for Solaris 9+? In
 https://bugs.launchpad.net/qemu/+bug/636315 we concluded that
 /usr/xpg4/bin/sh were the best choice of POSIX-compliant shell provided
 on Solaris 10. Blue's script fixes were never applied I believe…
 
 
 # /usr/xpg4/bin/sh scripts/tracetool 
 scripts/tracetool: line 113: syntax error at line 126: `)' unexpected
 
 tracetool seems to require bash … I'm happy to look at resolving that, 
 perhaps thats a better fix.
 

slap_wrist_for_not_looking_properly++;

It looks like a bug in tracetool (unless I completely misunderstand it), that 
bash doesn't care about…

# Get the format string including double quotes for a trace event
get_fmt()
{
puts ${1#*)}
}

So I will cancel the first two patches … and submit a fix for this instead.

Lee.

Re: [Qemu-devel] [PATCH 1/5] configure to set shell type

[Andreas Färber]

Am 16.03.2012 13:15, schrieb Peter Maydell:
 On 16 March 2012 12:02, Lee Essen lee.es...@nowonline.co.uk wrote:
 Adds support to configure for controlling which shell to use, defaults to 
 sh as before
 but adds bash for Solaris/Illumos builds. Plus ensures that tracetool is 
 called with a
 shell.
 
 Ugh. If we have bashisms in our shell scripts/configure/makefiles etc we 
 should
 fix them, not paper over them.
 
 If Solaris' /bin/sh isn't a POSIX sh that's a bug in Solaris :-)

Nah, Sun had really long support cycles and used to provide a POSIX sh
alongside their old sh for compatibility with themselves. ;-) We found
that actually documented in their man pages while investigating that in
response to my bug report. (Lee, don't forget to search the archives!)

 
 -echoAvailable backends: 
 $($source_path/scripts/tracetool --list-backends)
 +echoAvailable backends: $($shell 
 $source_path/scripts/tracetool --list-backends)
 
 This shouldn't be necessary -- tracetool has a #!/bin/sh at the top.
 If it needs bash then that should be fixed.

No, please. I'd be okay with setting shell=bash in a reasonably
limited environment (say, Solaris 11) but not with requiring bash for
all platforms.

The issue here is really just getting a fully POSIX-conformant shell.
And the way I expect this to work is by executing configure and make in
such a shell and by not having hardcoded /bin/sh creep in through some
shebang line.

Andreas

 -sh $source_path/scripts/tracetool --$trace_backend --check-backend  
 /dev/null 2 /dev/null
 +$shell $source_path/scripts/tracetool --$trace_backend --check-backend 
  /dev/null 2 /dev/null
 
 ...and we shouldn't need to use either 'sh' or '$shell' here...
 
 -- PMM

[Qemu-devel] [PATCH v7 2/6] Introduce save-devices-state

[Stefano Stabellini]

- add an is_ram flag to SaveStateEntry;

- register_savevm_live sets is_ram for live_savevm devices;

- introduce a save-devices-state QAPI command that can be used to save
the state of all devices, but not the RAM or the block devices of the
VM.

Changes in v7:

- rename save_devices to save-devices-state.

Changes in v6:

- remove the is_ram parameter from register_savevm_live and sets is_ram
if the device is a live_savevm device;

- introduce save_devices as a QAPI command, write a better description
for it;

- fix CODING_STYLE;

- introduce a new doc to explain the save format used by save_devices.

Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
Reviewed-by: Anthony Liguori aligu...@us.ibm.com
CC: Luiz Capitulino lcapitul...@redhat.com

---
 docs/save-devices-state.txt |   33 
 qapi-schema.json|   18 +++
 qmp-commands.hx |   25 +++
 savevm.c|   71 +++
 4 files changed, 147 insertions(+), 0 deletions(-)
 create mode 100644 docs/save-devices-state.txt

diff --git a/docs/save-devices-state.txt b/docs/save-devices-state.txt
new file mode 100644
index 000..c6b726c
--- /dev/null
+++ b/docs/save-devices-state.txt
@@ -0,0 +1,33 @@
+= Save Devices =
+
+QEMU has code to load/save the state of the guest that it is running.
+These are two complementary operations.  Saving the state just does
+that, saves the state for each device that the guest is running.
+
+These operations are normally used with migration (see migration.txt),
+however it is also possible to save the state of all devices to file,
+without saving the RAM or the block devices of the VM.
+
+This operation is called save-devices-state (see QMP/qmp-commands.txt)
+
+
+The binary format used in the file is the following:
+
+
+---
+
+32 bit big endian: QEMU_VM_FILE_MAGIC
+32 bit big endian: QEMU_VM_FILE_VERSION
+
+for_each_device
+{
+8 bit:  QEMU_VM_SECTION_FULL
+32 bit big endian:  section_id
+8 bit:  idstr (ID string) length
+string: idstr (ID string)
+32 bit big endian:  instance_id
+32 bit big endian:  version_id
+buffer: device specific data
+}
+
+8 bit: QEMU_VM_EOF
diff --git a/qapi-schema.json b/qapi-schema.json
index d0b6792..7dd0b74 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1593,3 +1593,21 @@
 { 'command': 'qom-list-types',
   'data': { '*implements': 'str', '*abstract': 'bool' },
   'returns': [ 'ObjectTypeInfo' ] }
+
+##
+# @save-devices-state:
+#
+# Save the state of all devices to file. The RAM and the block devices
+# of the VM are not saved by this command.
+#
+# @filename: the file to save the state of the devices to as binary
+# data. See save-devices-state.txt for a description of the binary format.
+#
+# Returns: Nothing on success
+#  If @filename cannot be opened, OpenFileFailed
+#  If an I/O error occurs while writing the file, IOError
+#
+# Since: 1.1
+##
+{ 'command': 'save-devices-state', 'data': {'filename': 'str'} }
+
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 705f704..8409d3f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -444,6 +444,31 @@ Note: inject-nmi is only supported for x86 guest 
currently, it will
 EQMP
 
 {
+.name   = save-devices-state,
+.args_type  = filename:F,
+.mhandler.cmd_new = qmp_marshal_input_save_devices_state,
+},
+
+SQMP
+save-devices-state
+---
+
+Save the state of all devices to file. The RAM and the block devices
+of the VM are not saved by this command.
+
+Arguments:
+
+- filename: the file to save the state of the devices to as binary
+data. See save-devices-state.txt for a description of the binary format.
+
+Example:
+
+- { execute: save-devices-state, arguments: { filename: /tmp/save } 
}
+- { return: {} }
+
+EQMP
+
+{
 .name   = migrate,
 .args_type  = detach:-d,blk:-b,inc:-i,uri:s,
 .params = [-d] [-b] [-i] uri,
diff --git a/savevm.c b/savevm.c
index 80be1ff..07ded39 100644
--- a/savevm.c
+++ b/savevm.c
@@ -84,6 +84,7 @@
 #include qemu-timer.h
 #include cpus.h
 #include memory.h
+#include qmp-commands.h
 
 #define SELF_ANNOUNCE_ROUNDS 5
 
@@ -1177,6 +1178,7 @@ typedef struct SaveStateEntry {
 void *opaque;
 CompatEntry *compat;
 int no_migrate;
+int is_ram;
 } SaveStateEntry;
 
 
@@ -1241,6 +1243,10 @@ int register_savevm_live(DeviceState *dev,
 se-opaque = opaque;
 se-vmsd = NULL;
 se-no_migrate = 0;
+/* if this is a live_savem then set is_ram */
+if (save_live_state != NULL) {
+se-is_ram = 1;
+}
 
 if (dev  dev-parent_bus  dev-parent_bus-info-get_dev_path) {
 char *id = dev-parent_bus-info-get_dev_path(dev);
@@ -1728,6 +1734,45 @@ out:
 return ret;
 }
 
+static int qemu_save_device_state(QEMUFile *f)
+{
+SaveStateEntry *se;
+
+

[Qemu-devel] [PATCH v7 4/6] xen: record physmap changes to xenstore

[Stefano Stabellini]

Write to xenstore any physmap changes so that the hypervisor can be
aware of them.
Read physmap changes from xenstore on boot.

Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
---
 xen-all.c |   78 -
 1 files changed, 77 insertions(+), 1 deletions(-)

diff --git a/xen-all.c b/xen-all.c
index b0ed1ed..f2cad82 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -65,7 +65,7 @@ static inline ioreq_t *xen_vcpu_ioreq(shared_iopage_t 
*shared_page, int vcpu)
 typedef struct XenPhysmap {
 target_phys_addr_t start_addr;
 ram_addr_t size;
-MemoryRegion *mr;
+char *name;
 target_phys_addr_t phys_offset;
 
 QLIST_ENTRY(XenPhysmap) list;
@@ -237,6 +237,7 @@ static int xen_add_to_physmap(XenIOState *state,
 XenPhysmap *physmap = NULL;
 target_phys_addr_t pfn, start_gpfn;
 target_phys_addr_t phys_offset = memory_region_get_ram_addr(mr);
+char path[80], value[17];
 
 if (get_physmapping(state, start_addr, size)) {
 return 0;
@@ -275,6 +276,7 @@ go_physmap:
 
 physmap-start_addr = start_addr;
 physmap-size = size;
+physmap-name = (char *)mr-name;
 physmap-phys_offset = phys_offset;
 
 QLIST_INSERT_HEAD(state-physmap, physmap, list);
@@ -283,6 +285,30 @@ go_physmap:
start_addr  TARGET_PAGE_BITS,
(start_addr + size)  TARGET_PAGE_BITS,
XEN_DOMCTL_MEM_CACHEATTR_WB);
+
+snprintf(path, sizeof(path),
+/local/domain/0/device-model/%d/physmap/%PRIx64/start_addr,
+xen_domid, (uint64_t)phys_offset);
+snprintf(value, sizeof(value), %PRIx64, (uint64_t)start_addr);
+if (!xs_write(state-xenstore, 0, path, value, strlen(value))) {
+return -1;
+}
+snprintf(path, sizeof(path),
+/local/domain/0/device-model/%d/physmap/%PRIx64/size,
+xen_domid, (uint64_t)phys_offset);
+snprintf(value, sizeof(value), %PRIx64, (uint64_t)size);
+if (!xs_write(state-xenstore, 0, path, value, strlen(value))) {
+return -1;
+}
+if (mr-name) {
+snprintf(path, sizeof(path),
+/local/domain/0/device-model/%d/physmap/%PRIx64/name,
+xen_domid, (uint64_t)phys_offset);
+if (!xs_write(state-xenstore, 0, path, mr-name, strlen(mr-name))) {
+return -1;
+}
+}
+
 return 0;
 }
 
@@ -911,6 +937,55 @@ int xen_init(void)
 return 0;
 }
 
+static void xen_read_physmap(XenIOState *state)
+{
+XenPhysmap *physmap = NULL;
+unsigned int len, num, i;
+char path[80], *value = NULL;
+char **entries = NULL;
+
+snprintf(path, sizeof(path),
+/local/domain/0/device-model/%d/physmap, xen_domid);
+entries = xs_directory(state-xenstore, 0, path, num);
+if (entries == NULL)
+return;
+
+for (i = 0; i  num; i++) {
+physmap = g_malloc(sizeof (XenPhysmap));
+physmap-phys_offset = strtoull(entries[i], NULL, 16);
+snprintf(path, sizeof(path),
+/local/domain/0/device-model/%d/physmap/%s/start_addr,
+xen_domid, entries[i]);
+value = xs_read(state-xenstore, 0, path, len);
+if (value == NULL) {
+free(physmap);
+continue;
+}
+physmap-start_addr = strtoull(value, NULL, 16);
+free(value);
+
+snprintf(path, sizeof(path),
+/local/domain/0/device-model/%d/physmap/%s/size,
+xen_domid, entries[i]);
+value = xs_read(state-xenstore, 0, path, len);
+if (value == NULL) {
+free(physmap);
+continue;
+}
+physmap-size = strtoull(value, NULL, 16);
+free(value);
+
+snprintf(path, sizeof(path),
+/local/domain/0/device-model/%d/physmap/%s/name,
+xen_domid, entries[i]);
+physmap-name = xs_read(state-xenstore, 0, path, len);
+
+QLIST_INSERT_HEAD(state-physmap, physmap, list);
+}
+free(entries);
+return;
+}
+
 int xen_hvm_init(void)
 {
 int i, rc;
@@ -986,6 +1061,7 @@ int xen_hvm_init(void)
 xen_be_register(console, xen_console_ops);
 xen_be_register(vkbd, xen_kbdmouse_ops);
 xen_be_register(qdisk, xen_blkdev_ops);
+xen_read_physmap(state);
 
 return 0;
 }
-- 
1.7.2.5

[Qemu-devel] [PATCH] Kick io-thread on qemu_chr_accept_input

[Jan Kiszka]

Once a chr frontend is able to receive input again, we need to inform
the io-thread about this fact. Otherwise, main_loop_wait may continue to
select without the related backend file descriptor in its set. This can
cause high input latencies if only low-rate events arrive otherwise.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---

/me wonders if a similar issue explains the slirp slowness under KVM
with in-kernel irqchip enabled. Need to check...

 qemu-char.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 9a5be75..a589a84 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -177,6 +177,7 @@ void qemu_chr_accept_input(CharDriverState *s)
 {
 if (s-chr_accept_input)
 s-chr_accept_input(s);
+qemu_notify_event();
 }
 
 void qemu_chr_fe_printf(CharDriverState *s, const char *fmt, ...)
-- 
1.7.3.4

Re: [Qemu-devel] Adding make check to the QEMU buildbot

[Anthony Liguori]

On 03/15/2012 12:21 PM, Stefan Weil wrote:

Am 15.03.2012 09:06, schrieb Stefan Hajnoczi:

QEMU has grown a number of sanity tests that can be run using make
check. They are fast and do not require many resources.

Is it possible to add make check after the build?

We may have to deal with some failures in the beginning - either due to
buildslave environment or legitimate broken platforms. But I think we
can get all green pretty easily.

Stefan


You can combine build and check by running make all check.
I don't expect much red (if any at all).

The iotests need some disk space (400 MB are sufficient, but I
don't know the lower limit). An incremental build and check
takes typically less than 3 minutes on my virtual server.
I nearly always compile without optimization because that
speeds compilation a lot, and errors caused by optimization
are very rare.

Non-Linux hosts currently won't run the iotests.

Cross builds cannot run the tests.


Once qtest gets merged, there will be a make check-report.html which has a nice 
HTML output.


Regards,

Anthony Liguori



Regards,

Stefan W.

[Qemu-devel] [PATCH] fix incorrect bracket in tracetool

[Lee Essen]

Signed-off-by: Lee Essen lee.es...@nowonline.co.uk

---

 scripts/tracetool |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/tracetool b/scripts/tracetool
index 65bd0a1..2e43d05 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -123,7 +123,7 @@ get_argc()
 # Get the format string including double quotes for a trace event
 get_fmt()
 {
-puts ${1#*)}
+puts ${1#*}
 }
 
 linetoh_begin_nop()

Re: [Qemu-devel] [PATCH 1/5] configure to set shell type

[Peter Maydell]

On 16 March 2012 12:24, Andreas Färber andreas.faer...@web.de wrote:
 Am 16.03.2012 13:15, schrieb Peter Maydell:
 This shouldn't be necessary -- tracetool has a #!/bin/sh at the top.
 If it needs bash then that should be fixed.

 No, please. I'd be okay with setting shell=bash in a reasonably
 limited environment (say, Solaris 11) but not with requiring bash for
 all platforms.

I meant, we should fix tracetool so that it really is a POSIX
conformant shell script, since that's what it claims to require.

 The issue here is really just getting a fully POSIX-conformant shell.
 And the way I expect this to work is by executing configure and make in
 such a shell and by not having hardcoded /bin/sh creep in through some
 shebang line.

The way I expect this to work is that /bin/sh should be a posix shell...

-- PMM

Re: [Qemu-devel] [PATCH 1/5] configure to set shell type

[Andreas Färber]

Am 16.03.2012 13:20, schrieb Lee Essen:
 
 On 16 Mar 2012, at 12:14, Andreas Färber wrote:
 
 Am 16.03.2012 13:02, schrieb Lee Essen:

 +echo SHELL=$shell  $config_host_mak

 Why?

 
 See patch 2/7 … there are several calls to tracetool that hardcode sh in 
 the Makefile … again if
 the better solution is removing the bash requirement then this problem goes 
 away completely.

My point is, there is already use of $(SHELL) in Makefile. So why do you
need to explicitly set it here? Such things need to be explained in the
commit message. Was the variable being used unassigned? Or are you
trying to use a non-GNU make?

The concept of using $(SHELL) in make seems valid. Dunno if there is an
easier, built-in way to execute a script in the current shell than
hardcoding some executable name that needs to be in the $PATH?

Andreas

Re: [Qemu-devel] [RFC PATCH 00/10] make qed and live migration usage safe

[Benoît Canet]

gentle ping :)

On Tue, Mar 6, 2012 at 6:32 PM, Benoît Canet benoit.ca...@gmail.com wrote:

 QED + live migration is an unsafe and disabled mix.

 This patchset make qed and live migration safe to use.

 The check of QED images is delayed during the incoming migration.
 After the migration complete the QED images are checked.

 Benoît Canet (10):
  block: Add new BDRV_O_INCOMING flag to notice incoming live migration
  block: add a function to set incoming live migration
  block: add a function to clear incoming live migration
  block: rename *_invalidate_cache_* to *_post_incoming_migration_*
  migration: inform the block layer of incoming live status
  block: open images with BDRV_O_INCOMING on incoming live migration
  qed: extract image checking into check_image_if_needed
  qed: add bdrv_post_incoming_migration operation checking the image
  qed: honor BDRV_O_INCOMING for incoming live migration
  qed: remove incoming live migration blocker

  block.c   |   35 +---
  block.h   |   13 +++--
  block/qcow2.c |7 -
  block/qed.c   |   81
 +++--
  block/qed.h   |2 -
  block_int.h   |4 +-
  migration.c   |9 +-
  vl.c  |5 +++
  8 files changed, 110 insertions(+), 46 deletions(-)

 --
 1.7.7.6

Re: [Qemu-devel] [PATCH] fix incorrect bracket in tracetool

[Andreas Färber]

Am 16.03.2012 13:29, schrieb Lee Essen:
 Signed-off-by: Lee Essen lee.es...@nowonline.co.uk

 ---

  scripts/tracetool |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

 diff --git a/scripts/tracetool b/scripts/tracetool
 index 65bd0a1..2e43d05 100755
 --- a/scripts/tracetool
 +++ b/scripts/tracetool
 @@ -123,7 +123,7 @@ get_argc()
  # Get the format string including double quotes for a trace event
  get_fmt()
  {
 -puts ${1#*)}
 +puts ${1#*}
  }
  
  linetoh_begin_nop()

Cc'ing the trace maintainer. I assume Lee forgot to look up the
maintainer, but Stefan, my checking MAINTAINERS indicates tracetool is
missing in the Tracing section too. Could you add it please?

Not being a shell expert I can't judge what this is actually trying to
do. Note that there is also an effort underway to rewrite tracetool as
tracetool.py.

Andreas

[Qemu-devel] Guest-sync-delimited and sentinel issue

[Michal Privoznik]

Hi guys,

I was just implementing support for guest-sync-delimited into libvirt. My 
intent is to issue this command prior any other command to determine if GA is 
available or not. The big advantage is - it doesn't change the state of the 
guest so from libvirt POV it's harmless. The other big advantage is this 
sentinel byte 0xFF which is supposed to flush all unprocessed (and possibly 
stale) data from previous unsuccessful tries.

As written in documentation, this command will output sentinel byte to the 
guest agent socket. This works perfectly. However, it is advised in the very 
same documentation to prepend this command with the sentinel as well allowing 
GA parser flush. But this doesn't work for me completely. All I can get is:

$ echo -e \xFF{\execute\:\guest-sync-delimited\, 
\arguments\:{\id\:1234}} | nc -U /tmp/ga.sock | hexdump -C
nc: using stream socket
  7b 22 65 72 72 6f 72 22  3a 20 7b 22 63 6c 61 73  |{error: {clas|
0010  73 22 3a 20 22 4a 53 4f  4e 50 61 72 73 69 6e 67  |s: JSONParsing|
0020  22 2c 20 22 64 61 74 61  22 3a 20 7b 7d 7d 7d 0a  |, data: {}}}.|
0030  ff 7b 22 72 65 74 75 72  6e 22 3a 20 31 32 33 34  |.{return: 1234|
0040  7d 0a |}.|
0042

The problem is - GA has difficulties with parsing sentinel, although the reply 
is correct, indeed.
Therefore my question is - should I just drop passing sentinel to GA? And even 
if this is fixed, How should I deal with older releases which have this bug?

Regards,
Michal

Re: [Qemu-devel] [PATCH] fix incorrect bracket in tracetool

[Lee Essen]

On 16 Mar 2012, at 12:44, Andreas Färber wrote:

 Am 16.03.2012 13:29, schrieb Lee Essen:
 Signed-off-by: Lee Essen lee.es...@nowonline.co.uk
 
 ---
 
 scripts/tracetool |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/scripts/tracetool b/scripts/tracetool
 index 65bd0a1..2e43d05 100755
 --- a/scripts/tracetool
 +++ b/scripts/tracetool
 @@ -123,7 +123,7 @@ get_argc()
 # Get the format string including double quotes for a trace event
 get_fmt()
 {
 -puts ${1#*)}
 +puts ${1#*}
 }
 
 linetoh_begin_nop()
 
 Cc'ing the trace maintainer. I assume Lee forgot to look up the
 maintainer, but Stefan, my checking MAINTAINERS indicates tracetool is
 missing in the Tracing section too. Could you add it please?
 
 Not being a shell expert I can't judge what this is actually trying to
 do. Note that there is also an effort underway to rewrite tracetool as
 tracetool.py.
 
 Andreas
 

Actually, I think I need to slow down a bit… there are more problems than just 
that bracket…

# make
  GEN   trace.h
/tmp/patch/qemu/scripts/tracetool[520]: local: not found [No such file or 
directory]
/tmp/patch/qemu/scripts/tracetool[66]: local: not found [No such file or 
directory]
/tmp/patch/qemu/scripts/tracetool[136]: local: not found [No such file or 
directory]
/tmp/patch/qemu/scripts/tracetool[55]: local: not found [No such file or 
directory]
/tmp/patch/qemu/scripts/tracetool[80]: local: not found [No such file or 
directory]
/tmp/patch/qemu/scripts/tracetool[55]: local: not found [No such file or 
directory]

From what I can see local isn't supported in posix ...
The POSIX standard supports functions, as shown above, but the semantics are 
weaker: functions do not have local traps or options, it is not possible to 
define local variables, and functions can't be exported.

So I could do with some advice now on how to proceed … is the goal to keep 
posix shell compliance? Wait for a tracetool.py version? Or should I go back to 
messing with SHELL?

Regards,

Lee.

Re: [Qemu-devel] [PATCH] Initial support for Ilumos build and Illumos-kvm

[Jan Kiszka]

On 2012-03-16 10:23, Lee Essen wrote:
 This fixes a number of issues with the build process (namely ensuring the use 
 of bash), adds specific support for the Illumos port of KVM and fixes a few 
 general Solaris compatibility issues.
 
 There are still some things outstanding:
 
 - there's a duplicate smb_wmb() definition in qemu-barrier.h and the illumos 
 kvm_x86.h which generates some warnings.
 - there's a repeated call to page_size() that should probably be fixed.
 - dtrace support needs to be fixed (-m64/32 option, reserved words and 
 linking issues)
 - vnics need to be added
 - the original illumos code added another timer source (multiticks)
 - the issue with Linux needs to be resolved
 
 Other than that, this gets it to the point where it will build and run with 
 illumos kvm, and works fine for Windows.
 
 It's my first patch to qemu, and most of the real kvm stuff has come from the 
 original illumos-kvm-cmd tree, so be gentle with me!
 
 
 Signed-off-by: Lee Essen lee.es...@nowonline.co.uk
 
 --
  Makefile.objs  |6 +-
  Makefile.target|6 +-
  configure  |8 +++-
  cpus.c |4 +-
  exec.c |   88 
 
  fpu/softfloat-specialize.h |4 ++
  hw/kvm/clock.c |4 ++
  kvm-all.c  |   81 -
  kvm.h  |   15 +++
  qemu-timer.c   |   10 ++--
  qga/channel-posix.c|   16 
  qga/commands-posix.c   |9 
  target-i386/hyperv.h   |4 ++
  target-i386/kvm.c  |   53 +-
  14 files changed, 291 insertions(+), 17 deletions(-)
 

...

 diff --git a/hw/kvm/clock.c b/hw/kvm/clock.c
 index 446bd62..3fd5e1e 100644
 --- a/hw/kvm/clock.c
 +++ b/hw/kvm/clock.c
 @@ -19,8 +19,12 @@
  #include hw/sysbus.h
  #include hw/kvm/clock.h
  
 +#ifdef __sun__
 +#include sys/kvm.h
 +#else
  #include linux/kvm.h
  #include linux/kvm_para.h
 +#endif

As Paolo already said, this should somehow be centralized.

Also, CONFIG_SOLARIS vs. __sun__: please use a consistent pattern.

  
  typedef struct KVMClockState {
  SysBusDevice busdev;
 diff --git a/kvm-all.c b/kvm-all.c
 index 42e5e23..27f3177 100644
 --- a/kvm-all.c
 +++ b/kvm-all.c
 @@ -18,7 +18,11 @@
  #include sys/mman.h
  #include stdarg.h
  
 +#ifdef __sun__
 +#include sys/kvm.h
 +#else
  #include linux/kvm.h
 +#endif
  
  #include qemu-common.h
  #include qemu-barrier.h
 @@ -176,12 +180,23 @@ int kvm_physical_memory_addr_from_host(KVMState *s, 
 void *ram,
  static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
  {
  struct kvm_userspace_memory_region mem;
 +#ifdef CONFIG_SOLARIS
 +caddr_t p;
 +char c;
 +#endif
  
  mem.slot = slot-slot;
  mem.guest_phys_addr = slot-start_addr;
  mem.memory_size = slot-memory_size;
  mem.userspace_addr = (unsigned long)slot-ram;
  mem.flags = slot-flags;
 +#ifdef CONFIG_SOLARIS
 +for (p = (caddr_t)mem.userspace_addr;
 +  p  (caddr_t)mem.userspace_addr + mem.memory_size;
 +  p += PAGE_SIZE)
 +c = *p;
 +#endif /* CONFIG_SOLARIS */
 +

I bet gcc will like this write-only pattern and bark at you.

  if (s-migration_log) {
  mem.flags |= KVM_MEM_LOG_DIRTY_PAGES;
  }
 @@ -200,6 +215,31 @@ int kvm_pit_in_kernel(void)
  return kvm_state-pit_in_kernel;
  }
  
 +#ifdef CONFIG_SOLARIS
 +static int kvm_vm_clone(KVMState *s)
 +{
 +struct stat stat;
 +int fd;
 +
 +if (fstat(s-fd, stat) != 0) {
 +return -errno;
 +}
 +
 +fd = qemu_open(/dev/kvm, O_RDWR);
 +
 +if (fd == -1) {
 +return -errno;
 +}
 +
 +if (ioctl(fd, KVM_CLONE, stat.st_rdev) == -1) {
 +close(fd);
 +return -errno;
 +}
 +
 +return fd;
 +}
 +#endif
 +
  int kvm_init_vcpu(CPUArchState *env)
  {
  KVMState *s = kvm_state;
 @@ -208,14 +248,29 @@ int kvm_init_vcpu(CPUArchState *env)
  
  DPRINTF(kvm_init_vcpu\n);
  
 +#ifdef CONFIG_SOLARIS
 +ret = kvm_vm_clone(kvm_state);
 +
 +if (ret  0) {
 +fprintf(stderr, kvm_init_vcpu could not clone fd: %m\n);
 +goto err;
 +}
 +env-kvm_fd = ret;
 +env-kvm_state = kvm_state;
 +
 +ret = ioctl(env-kvm_fd, KVM_CREATE_VCPU, env-cpu_index);

kvm_vcpu_ioctl

 +#else
  ret = kvm_vm_ioctl(s, KVM_CREATE_VCPU, env-cpu_index);
 +#endif

There is no chance to fix the Solaris KVM to do fd cloning in the kernel
and implement the same KVM_CREATE_VCPU ABI?

  if (ret  0) {
  DPRINTF(kvm_create_vcpu failed\n);
  goto err;
  }
  
 +#ifndef CONFIG_SOLARIS
  env-kvm_fd = ret;
  env-kvm_state = s;
 +#endif
  env-kvm_vcpu_dirty = 1;
  
  mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
 @@ -1021,6 +1076,9 @@ int kvm_init(void)
  ret = s-vmfd;
  goto err;
  }
 +#ifdef CONFIG_SOLARIS
 +s-vmfd = s-fd;
 +#endif
  
   

Re: [Qemu-devel] [PATCH] Kick io-thread on qemu_chr_accept_input

[Anthony Liguori]

On 03/16/2012 07:25 AM, Jan Kiszka wrote:

Once a chr frontend is able to receive input again, we need to inform
the io-thread about this fact. Otherwise, main_loop_wait may continue to
select without the related backend file descriptor in its set. This can
cause high input latencies if only low-rate events arrive otherwise.

Signed-off-by: Jan Kiszkajan.kis...@siemens.com


I'm not nacking this patch, but please note that this is a band-aid as not all 
char devices actually use qemu_chr_accept_input().


Regards,

Anthony Liguori


---

/me wonders if a similar issue explains the slirp slowness under KVM
with in-kernel irqchip enabled. Need to check...

  qemu-char.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 9a5be75..a589a84 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -177,6 +177,7 @@ void qemu_chr_accept_input(CharDriverState *s)
  {
  if (s-chr_accept_input)
  s-chr_accept_input(s);
+qemu_notify_event();
  }

  void qemu_chr_fe_printf(CharDriverState *s, const char *fmt, ...)

Re: [Qemu-devel] [PATCH] fix incorrect bracket in tracetool

[Paolo Bonzini]

Il 16/03/2012 13:29, Lee Essen ha scritto:
 @@ -123,7 +123,7 @@ get_argc()
  # Get the format string including double quotes for a trace event
  get_fmt()
  {
 -puts ${1#*)}
 +puts ${1#*}
  }
  

Eric, can you look at this?  Is it a bashism or a Solaris bug?

I would write it, to be entirely safe, as

   local fmt
   fmt=${1#*\)}
   puts $fmt

where I'm using the extra variable to avoid the ambiguity of quoting the
parentheses within quotes; variable assignments are always implicitly
quoted.

Paolo

Re: [Qemu-devel] [PATCH] Kick io-thread on qemu_chr_accept_input

[Jan Kiszka]

On 2012-03-16 14:16, Anthony Liguori wrote:
 On 03/16/2012 07:25 AM, Jan Kiszka wrote:
 Once a chr frontend is able to receive input again, we need to inform
 the io-thread about this fact. Otherwise, main_loop_wait may continue to
 select without the related backend file descriptor in its set. This can
 cause high input latencies if only low-rate events arrive otherwise.

 Signed-off-by: Jan Kiszkajan.kis...@siemens.com
 
 I'm not nacking this patch, but please note that this is a band-aid as not 
 all 
 char devices actually use qemu_chr_accept_input().

Then they have to be fixed. :)

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH] fix incorrect bracket in tracetool

[Paolo Bonzini]

Il 16/03/2012 14:00, Lee Essen ha scritto:
 /tmp/patch/qemu/scripts/tracetool[520]: local: not found [No such file or 
 directory]
 /tmp/patch/qemu/scripts/tracetool[66]: local: not found [No such file or 
 directory]
 /tmp/patch/qemu/scripts/tracetool[136]: local: not found [No such file or 
 directory]
 /tmp/patch/qemu/scripts/tracetool[55]: local: not found [No such file or 
 directory]
 /tmp/patch/qemu/scripts/tracetool[80]: local: not found [No such file or 
 directory]
 /tmp/patch/qemu/scripts/tracetool[55]: local: not found [No such file or 
 directory]
 
 From what I can see local isn't supported in posix ...
 The POSIX standard supports functions, as shown above, but the semantics are 
 weaker: functions do not have local traps or options, it is not possible to 
 define local variables, and functions can't be exported.
 
 So I could do with some advice now on how to proceed … is the goal to keep 
 posix shell compliance? Wait for a tracetool.py version? Or should I go back 
 to messing with SHELL?

I think #!/bin/bash is a better solution in the short-term.

Paolo

Re: [Qemu-devel] [PATCH] fix incorrect bracket in tracetool

[Andreas Färber]

Am 16.03.2012 14:00, schrieb Lee Essen:
 On 16 Mar 2012, at 12:44, Andreas Färber wrote:
 
 Am 16.03.2012 13:29, schrieb Lee Essen:
 Signed-off-by: Lee Essen lee.es...@nowonline.co.uk

 ---

 scripts/tracetool |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

 diff --git a/scripts/tracetool b/scripts/tracetool
 index 65bd0a1..2e43d05 100755
 --- a/scripts/tracetool
 +++ b/scripts/tracetool
 @@ -123,7 +123,7 @@ get_argc()
 # Get the format string including double quotes for a trace event
 get_fmt()
 {
 -puts ${1#*)}
 +puts ${1#*}
 }

 linetoh_begin_nop()

 Cc'ing the trace maintainer. I assume Lee forgot to look up the
 maintainer, but Stefan, my checking MAINTAINERS indicates tracetool is
 missing in the Tracing section too. Could you add it please?

 Not being a shell expert I can't judge what this is actually trying to
 do. Note that there is also an effort underway to rewrite tracetool as
 tracetool.py.
 
 Actually, I think I need to slow down a bit…

:) No need to rush, it's been broken for a while.

 there are more problems than just that bracket…
 
 # make
   GEN   trace.h
 /tmp/patch/qemu/scripts/tracetool[520]: local: not found [No such file or 
 directory]
 /tmp/patch/qemu/scripts/tracetool[66]: local: not found [No such file or 
 directory]
 /tmp/patch/qemu/scripts/tracetool[136]: local: not found [No such file or 
 directory]
 /tmp/patch/qemu/scripts/tracetool[55]: local: not found [No such file or 
 directory]
 /tmp/patch/qemu/scripts/tracetool[80]: local: not found [No such file or 
 directory]
 /tmp/patch/qemu/scripts/tracetool[55]: local: not found [No such file or 
 directory]
 
 From what I can see local isn't supported in posix ...
 The POSIX standard supports functions, as shown above, but the semantics are 
 weaker: functions do not have local traps or options, it is not possible to 
 define local variables, and functions can't be exported.

Hm, Blue's patch in the bug I referenced earlier still had local in it
and according to my comments worked with Solaris 10's /usr/xpg4/bin/sh,
and I don't have that issue on OpenIndiana (bash). What shell did you
test with on SmartOS?

 So I could do with some advice now on how to proceed … is the goal to keep 
 posix shell compliance? Wait for a tracetool.py version? Or should I go back 
 to messing with SHELL?

I'd recommend to evaluate what needs to be done to make the script(s)
POSIX-compliant. If the resulting patch is reasonable then IMO we should
apply it even if it gets replaced by a Python version later (it was
still feature-incomplete last time posted and has been floating around a
while already). Was just pointing it out for you in case it's easier to
get running on your system.

Andreas

Re: [Qemu-devel] [PATCH] fix incorrect bracket in tracetool

[Andreas Färber]

Am 16.03.2012 14:19, schrieb Paolo Bonzini:
 Il 16/03/2012 14:00, Lee Essen ha scritto:
 /tmp/patch/qemu/scripts/tracetool[520]: local: not found [No such file or 
 directory]
 /tmp/patch/qemu/scripts/tracetool[66]: local: not found [No such file or 
 directory]
 /tmp/patch/qemu/scripts/tracetool[136]: local: not found [No such file or 
 directory]
 /tmp/patch/qemu/scripts/tracetool[55]: local: not found [No such file or 
 directory]
 /tmp/patch/qemu/scripts/tracetool[80]: local: not found [No such file or 
 directory]
 /tmp/patch/qemu/scripts/tracetool[55]: local: not found [No such file or 
 directory]

 From what I can see local isn't supported in posix ...
 The POSIX standard supports functions, as shown above, but the semantics 
 are weaker: functions do not have local traps or options, it is not possible 
 to define local variables, and functions can't be exported.

 So I could do with some advice now on how to proceed … is the goal to keep 
 posix shell compliance? Wait for a tracetool.py version? Or should I go back 
 to messing with SHELL?
 
 I think #!/bin/bash is a better solution in the short-term.

Breaks if it's in /usr/gnu/bin or /opt/SUNWfoo/bin. :)

Just like Python scripts can't rely on #!/usr/bin/env python on
BeOS/Haiku in lack of /usr. The world is complicated.

Andreas

Re: [Qemu-devel] [PATCH] Initial support for Ilumos build and Illumos-kvm

[Lee Essen]

On 16 Mar 2012, at 13:14, Jan Kiszka wrote:

 On 2012-03-16 10:23, Lee Essen wrote:
 +#ifdef __sun__
 +#include sys/kvm.h
 +#else
 #include linux/kvm.h
 #include linux/kvm_para.h
 +#endif
 
 As Paolo already said, this should somehow be centralised.

Yep, fair point. I'll address this one.

 Also, CONFIG_SOLARIS vs. __sun__: please use a consistent pattern.

Hmmm … I was trying to be consistent with the existing style :-) … see 
__linux__ and CONFIG_LINUX as well. I'll see what I can do to make this a bit 
tidier.

 +#ifdef CONFIG_SOLARIS
 +for (p = (caddr_t)mem.userspace_addr;
 +  p  (caddr_t)mem.userspace_addr + mem.memory_size;
 +  p += PAGE_SIZE)
 +c = *p;
 +#endif /* CONFIG_SOLARIS */
 +
 
 I bet gcc will like this write-only pattern and bark at you.
 

It does indeed … this came from the original Joyent code, I must admit I did 
wonder whether gcc would optimise it away. I did consider adding something to 
stop gcc complaining, but I don't fully understand why this is necessary given 
the mlock() bit, so I thought it best to leave it alone.

Any suggestions?

 +#else
 ret = kvm_vm_ioctl(s, KVM_CREATE_VCPU, env-cpu_index);
 +#endif
 
 There is no chance to fix the Solaris KVM to do fd cloning in the kernel
 and implement the same KVM_CREATE_VCPU ABI?
 

I will raise this with the joyent guys, but they are pretty switched on and I 
suspect there is a reason.

My concern with the fix the kernel comments is that it would exclude the use 
of the newer qemu on existing installations, however I do understand the desire 
to not fill the code with workarounds that live forever.

How about a broken_solaris_kvm_abi option to configure with a suitable set of 
defines wrapping the code?

 
 #ifdef CONFIG_KVM
 +#ifdef __sun__
 +#include sys/kvm.h
 +/*
 + * it's a bit horrible to include these here, but the kvm_para.h include 
 file
 + * isn't public with the illumos kvm implementation
 
 Just provide a package of properly fixed kernel headers and let us carry
 them in solaris-headers or so, analogously to linux-headers.
 

Interestingly this is what I did originally but then thought it best to use the 
supplied headers, but actually thinking more about it, this does make much 
more sense.

Regards,

Lee.

[Qemu-devel] [PATCH] hw/qxl.c: Fix compilation failures on 32 bit hosts

[Peter Maydell]

Fix compilation failures on 32 bit hosts (cast from pointer to
integer of different size; %ld expects 'long int' not uint64_t).

Reported-by: Steve Langasek steve.langa...@canonical.com
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
 hw/qxl.c |   16 
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index e17b0e3..47dc383 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -149,7 +149,7 @@ void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t 
surface_id,
 } else {
 assert(cookie != NULL);
 spice_qxl_update_area_async(qxl-ssd.qxl, surface_id, area,
-clear_dirty_region, (uint64_t)cookie);
+clear_dirty_region, (uintptr_t)cookie);
 }
 }
 
@@ -171,7 +171,7 @@ static void qxl_spice_destroy_surface_wait(PCIQXLDevice 
*qxl, uint32_t id,
 cookie = qxl_cookie_new(QXL_COOKIE_TYPE_IO,
 QXL_IO_DESTROY_SURFACE_ASYNC);
 cookie-u.surface_id = id;
-spice_qxl_destroy_surface_async(qxl-ssd.qxl, id, (uint64_t)cookie);
+spice_qxl_destroy_surface_async(qxl-ssd.qxl, id, (uintptr_t)cookie);
 } else {
 qxl-ssd.worker-destroy_surface_wait(qxl-ssd.worker, id);
 qxl_spice_destroy_surface_wait_complete(qxl, id);
@@ -181,8 +181,8 @@ static void qxl_spice_destroy_surface_wait(PCIQXLDevice 
*qxl, uint32_t id,
 static void qxl_spice_flush_surfaces_async(PCIQXLDevice *qxl)
 {
 spice_qxl_flush_surfaces_async(qxl-ssd.qxl,
-(uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
- QXL_IO_FLUSH_SURFACES_ASYNC));
+(uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+  QXL_IO_FLUSH_SURFACES_ASYNC));
 }
 
 void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
@@ -213,8 +213,8 @@ static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl, 
qxl_async_io async)
 {
 if (async) {
 spice_qxl_destroy_surfaces_async(qxl-ssd.qxl,
-(uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
- QXL_IO_DESTROY_ALL_SURFACES_ASYNC));
+(uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+  QXL_IO_DESTROY_ALL_SURFACES_ASYNC));
 } else {
 qxl-ssd.worker-destroy_surfaces(qxl-ssd.worker);
 qxl_spice_destroy_surfaces_complete(qxl);
@@ -743,7 +743,7 @@ static void interface_async_complete_io(PCIQXLDevice *qxl, 
QXLCookie *cookie)
 }
 if (cookie  current_async != cookie-io) {
 fprintf(stderr,
-qxl: %s: error: current_async = %d != %ld = cookie-io\n,
+qxl: %s: error: current_async = %d != % PRId64  = 
cookie-io\n,
 __func__, current_async, cookie-io);
 }
 switch (current_async) {
@@ -812,7 +812,7 @@ static void interface_update_area_complete(QXLInstance *sin,
 static void interface_async_complete(QXLInstance *sin, uint64_t cookie_token)
 {
 PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
-QXLCookie *cookie = (QXLCookie *)cookie_token;
+QXLCookie *cookie = (QXLCookie *)(uintptr_t)cookie_token;
 
 switch (cookie-type) {
 case QXL_COOKIE_TYPE_IO:
-- 
1.7.9

[Qemu-devel] [Bug 918791] Re: qemu-kvm dies when using vmvga driver and unity in the guest

[Serge Hallyn]

@Martin,

No - Im still not sure about its responsibilities, so I don't know
whether it's deemed a bug that it is using negative values, but
certainly (virtualized) hardware shouldn't crash due to it, so that's
where we're fixing it.

So I'll mark the -vmware package part of the bug invalid.

Thanks.

** Changed in: xserver-xorg-video-vmware (Ubuntu Precise)
   Status: Confirmed = Invalid

** Changed in: xserver-xorg-video-vmware (Ubuntu Oneiric)
   Status: New = Invalid

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

Title:
  qemu-kvm dies when using vmvga driver and unity in the guest

Status in QEMU:
  New
Status in “qemu-kvm” package in Ubuntu:
  Fix Released
Status in “xserver-xorg-video-vmware” package in Ubuntu:
  Invalid
Status in “qemu-kvm” source package in Oneiric:
  New
Status in “xserver-xorg-video-vmware” source package in Oneiric:
  Invalid
Status in “qemu-kvm” source package in Precise:
  Fix Released
Status in “xserver-xorg-video-vmware” source package in Precise:
  Invalid

Bug description:
  =
  SRU Justification:
  1. impact: kvm crashes
  2. Development fix: don't allow attempts to set_bit to negative offsets
  3. Stable fix: same as development fix
  4. Test case (see below)
  5. Regression potential: if the patch is wrong, graphics for vmware vga over 
vnc could get messed up
  =

  12.04's qemu-kvm has been unstable for me and Marc Deslauriers and I
  figured out it has something to do with the interaction of qemu-kvm,
  unity and the vmvga driver. This is a regression over qemu-kvm in
  11.10.

  TEST CASE:
  1. start a VM that uses unity (eg, 11.04, 11.10 or 12.04). My tests use 
unity-2d on an amd64 host and amd64 guests
  2. on 11.04 and 11.10, open empathy via the messaging indicator and click 
'Chat'. On 12.04, open empathy via the messaging indicator and click 'Chat', 
close the empathy wizard, move the empathy window over the unity luancher (so 
it autohides), then do 'ctrl+alt+t' to open a terminal

  When the launcher tries to auto(un)hide, qemu-kvm dies with this:
  [10574.958149] do_general_protection: 132 callbacks suppressed
  [10574.958154] kvm[13192] general protection ip:7fab9680ea0f sp:74440148 
error:0 in qemu-system-x86_64[7fab966c4000+2c9000]

  Relevant libvirt xml:
  video
    model type='vmvga' vram='9216' heads='1'/
    address type='pci' domain='0x' bus='0x00' slot='0x02' 
function='0x0'/
  /video

  If I change to using 'cirrus', then qemu-kvm no longer crashes. Eg:
  video
    model type='cirrus' vram='9216' heads='1'/
    alias name='video0'/
    address type='pci' domain='0x' bus='0x00' slot='0x02' 
function='0x0'/
  /video

  The workaround is therefore to use the cirrus driver instead of vmvga,
  however being able to kill qemu-kvm in this manner is not ideal. Also,
  unfortunately unity-2d does not run with with cirrus driver under
  11.04, so the security and SRU teams are unable to properly test
  updates in GUI applications under unity when using the current 12.04
  qemu-kvm.

  I tried to report this via apport, but apport complained about a CRC
  error, so I could not.

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

[Qemu-devel] RFC: double free in qmp_output_visitor_cleanup()

[Laszlo Ersek]

Hi,

we seem to have found a double free in qmp_output_visitor_cleanup().
Please read the analysis below (that is based on commit e4e6aa14) and
please tell me if you'd like me to write a patch for solution (a) or
solution (b), as described at the bottom.

Paolo wrote a test case to trigger the problem, I'm attaching it.

Thank you,
Laszlo

 Original Message 
Date: Fri, 16 Mar 2012 13:55:48 +0100
From: Laszlo Ersek ler...@redhat.com

[...]

Consider the following example: we want to put an int (I0) in a dict
(D1) in a dict (D0). D0 is the root element.

When we start D0 with qmp_output_start_struct(), the stack is empty, so

qmp_output_start_struct()
- qmp_output_add_obj(D0)
   - qmp_output_push_obj(D0)[1]
- qmp_output_push(D0)   [2]

[1] After this step completes, we have a single entry (entry#0) in the
stack, and it points to D0. refcnt(D0) equals 1 (still from
qdict_new()). Entry#0 is an *owning* reference to D0 (aggregation,
contributes to refcounts).

[2] In this step we push D0 again, so entry#1 will point at D0 too.
refcnt(D0) still equals 1. Entry#1 is only a *navigation* reference
(weak reference etc.), it's only here so that we can continue adding
elements to D0 after we finished building a subtree below -- see step [6].

Then we start D1:

qmp_output_start_struct()
- qmp_output_add(D1)
   - qdict_put_obj(D0, D1)  [3]
- qmp_output_push(D1)   [4]

[3] refcnt(D1) == 1, from qdict_new(). qdict_put_obj() transfers
ownership of D1 to D0, without changing refcnt(D1), so that still equals
1. At the end of this step, D1 is *owned* by D0.

[4] Here we push D1 to the stack, without changing refcounts. Entry#2 is
only a *navigation* reference to D1.

Then we add I0:
qmp_output_type_int()
- qmp_output_add(I0)
   - qdict_put_obj(D1, I0)  [5]

[5] In this step we transfer ownership of I0 to D1, utilizing the
entry#2 *navigation* link to find D1. refcnt(I0) == 1, from
qint_from_int(). The status quo is as follows:

{Figure-1}

entry#0 --- owns [1] --+
   |
   v

entry#1 --- navigates-to [2] ---  D0 (refcnt=1)

   |
 owns [3]
   |
   v

entry#2 --- navigates-to [4] ---  D1 (refcnt=1)

   |
 owns [5]
   |
   v

   I0 (refcnt=1)

Then we close D1 and return to D0:

qmp_output_end_struct()
- qmp_output_pop()  [6]

[6] In this step we simply throw away (release) entry#2. We could
continue adding elements to D0 via the entry#1 navigation link, that was
set up in step [2].

Then we close D0 too:
qmp_output_end_struct()
- qmp_output_pop()  [7]

We drop entry#1.

We *never* drop entry #0 during this. After we issued an equal number of
pushes and pops, that is, when we're done, this is how it looks:

{Figure-2}

entry#0 --- owns [1] --+
   |
   v

   D0 (refcnt=1)

   |
 owns [3]
   |
   v

   D1 (refcnt=1)

   |
 owns [5]
   |
   v

   I0 (refcnt=1)


Now, consider running qmp_output_visitor_cleanup() on {Figure-2}. Since
there's only entry#0, we remove it, and issue

qobject_decref(D0)
- qdict_destroy_obj(D0)
   - qentry_destroy()
  - qobject_decref(D1)
 - qdict_destroy_obj(D1)
- qentry_destroy()
   - qobject_decref(I0)
  - qint_destroy_obj(I0)
 - qemu_free(I0)
- qemu_free(D1)
   - qemu_free(D0)

Everything's gone.

Consider instead running qmp_output_visitor_cleanup() on {Figure-1},
that is, in a state where there have been more qmp_output_start_struct()
calls than qmp_output_end_struct() calls:

{Figure-1 again}

entry#0 --- owns [1] --+
   |
   v

entry#1 --- navigates-to [2] ---  D0 (refcnt=1)

   |
 owns [3]
   |
   v

entry#2 --- navigates-to [4] ---  D1 (refcnt=1)

   |
 owns [5]
   |
   

Re: [Qemu-devel] [PATCH] hw/qxl.c: Fix compilation failures on 32 bit hosts

[Stefan Weil]

Am 16.03.2012 14:50, schrieb Peter Maydell:

Fix compilation failures on 32 bit hosts (cast from pointer to
integer of different size; %ld expects 'long int' not uint64_t).

Reported-by: Steve Langaseksteve.langa...@canonical.com
Signed-off-by: Peter Maydellpeter.mayd...@linaro.org
---
  hw/qxl.c |   16 
  1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index e17b0e3..47dc383 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -149,7 +149,7 @@ void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t 
surface_id,
  } else {
  assert(cookie != NULL);
  spice_qxl_update_area_async(qxl-ssd.qxl, surface_id, area,
-clear_dirty_region, (uint64_t)cookie);
+clear_dirty_region, (uintptr_t)cookie);
  }
  }

@@ -171,7 +171,7 @@ static void qxl_spice_destroy_surface_wait(PCIQXLDevice 
*qxl, uint32_t id,
  cookie = qxl_cookie_new(QXL_COOKIE_TYPE_IO,
  QXL_IO_DESTROY_SURFACE_ASYNC);
  cookie-u.surface_id = id;
-spice_qxl_destroy_surface_async(qxl-ssd.qxl, id, (uint64_t)cookie);
+spice_qxl_destroy_surface_async(qxl-ssd.qxl, id, (uintptr_t)cookie);
  } else {
  qxl-ssd.worker-destroy_surface_wait(qxl-ssd.worker, id);
  qxl_spice_destroy_surface_wait_complete(qxl, id);
@@ -181,8 +181,8 @@ static void qxl_spice_destroy_surface_wait(PCIQXLDevice 
*qxl, uint32_t id,
  static void qxl_spice_flush_surfaces_async(PCIQXLDevice *qxl)
  {
  spice_qxl_flush_surfaces_async(qxl-ssd.qxl,
-(uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
- QXL_IO_FLUSH_SURFACES_ASYNC));
+(uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+  QXL_IO_FLUSH_SURFACES_ASYNC));
  }

  void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
@@ -213,8 +213,8 @@ static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl, 
qxl_async_io async)
  {
  if (async) {
  spice_qxl_destroy_surfaces_async(qxl-ssd.qxl,
-(uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
- QXL_IO_DESTROY_ALL_SURFACES_ASYNC));
+(uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+  QXL_IO_DESTROY_ALL_SURFACES_ASYNC));
  } else {
  qxl-ssd.worker-destroy_surfaces(qxl-ssd.worker);
  qxl_spice_destroy_surfaces_complete(qxl);
@@ -743,7 +743,7 @@ static void interface_async_complete_io(PCIQXLDevice *qxl, 
QXLCookie *cookie)
  }
  if (cookie  current_async != cookie-io) {
  fprintf(stderr,
-qxl: %s: error: current_async = %d != %ld = cookie-io\n,
+qxl: %s: error: current_async = %d != % PRId64  = 
cookie-io\n,
  __func__, current_async, cookie-io);
   


current_async is uint32_t,  so maybe you can also replace %d by % PRIu32 .
Is io unsigned (I don't have the headers, but think it is)? Then PRIu64
would be better.

With these modifications you may add

Reviewed-by: Stefan Weil s...@weilnetz.de

Cheers,
Stefan


  }
  switch (current_async) {
@@ -812,7 +812,7 @@ static void interface_update_area_complete(QXLInstance *sin,
  static void interface_async_complete(QXLInstance *sin, uint64_t cookie_token)
  {
  PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
-QXLCookie *cookie = (QXLCookie *)cookie_token;
+QXLCookie *cookie = (QXLCookie *)(uintptr_t)cookie_token;

  switch (cookie-type) {
  case QXL_COOKIE_TYPE_IO:
   

Re: [Qemu-devel] Guest-sync-delimited and sentinel issue

[Michael Roth]

On Fri, Mar 16, 2012 at 01:47:42PM +0100, Michal Privoznik wrote:
 Hi guys,
 
 I was just implementing support for guest-sync-delimited into libvirt. My 
 intent is to issue this command prior any other command to determine if GA is 
 available or not. The big advantage is - it doesn't change the state of the 
 guest so from libvirt POV it's harmless. The other big advantage is this 
 sentinel byte 0xFF which is supposed to flush all unprocessed (and possibly 
 stale) data from previous unsuccessful tries.

Are you opening the qemu-ga socket prior to each command? Or just
once at startup? If you're only opening it once, it should be sufficient
to do the guest-sync/guest-sync-delimited exchange just once at that time,
since the streams are presumably synced at that point, and after that only if
you get a client-side timeout.

Issuing it prior to each command doesn't guarantee that the agent will
be available to handle the command, so you still need be prepared to
handle a timeout + re-sync. It does reduce the chances of timing out on
doing something that affects guest state though... but if that's the
intention here I would recommend just using 'guest-ping', which should
work reliably so long as you always re-sync on connect and after
client-side timeouts.

But it doesn't really matter either way, all I'm really getting at is that
scanning for the 0xFF delimiter in the response shouldn't be *necessary* in
this case. But there's no harm in using it that way. You don't need to
precede the request with 0xFF if you're just using it to probe for the
agent though, and you probably wouldn't want to given that it results in
2 responses:

 
 As written in documentation, this command will output sentinel byte to the 
 guest agent socket. This works perfectly. However, it is advised in the very 
 same documentation to prepend this command with the sentinel as well allowing 
 GA parser flush. But this doesn't work for me completely. All I can get is:
 
 $ echo -e \xFF{\execute\:\guest-sync-delimited\, 
 \arguments\:{\id\:1234}} | nc -U /tmp/ga.sock | hexdump -C
 nc: using stream socket
   7b 22 65 72 72 6f 72 22  3a 20 7b 22 63 6c 61 73  |{error: {clas|
 0010  73 22 3a 20 22 4a 53 4f  4e 50 61 72 73 69 6e 67  |s: JSONParsing|
 0020  22 2c 20 22 64 61 74 61  22 3a 20 7b 7d 7d 7d 0a  |, data: {}}}.|
 0030  ff 7b 22 72 65 74 75 72  6e 22 3a 20 31 32 33 34  |.{return: 1234|
 0040  7d 0a |}.|
 0042
 
 The problem is - GA has difficulties with parsing sentinel, although the 
 reply is correct, indeed.
 Therefore my question is - should I just drop passing sentinel to GA? And 
 even if this is fixed, How should I deal with older releases which have this 
 bug?

Sorry, I didn't document this properly. Haven't tested host-guest flush
in a while and got it in my head that it was handled silently, but what
you're seeing has actually always been the observed/intended behavior.

Depending on how you've implemented guest-sync-delimited it might not make
a difference though. If you're just ignoring any data up until you see
the 0xFF sentinel value then the error is silently thrown away as garbage. The
semantics of the command are that you may read garbage prior to the
sentinel+response, it's just that when preceeding the request with the
sentinel this is *always* the case.

Otherwise, if you're handling it like a normal request, when sending the 0xFF
you would treat it basically as a flush command that always returns a
JSONParsing error. It's not pretty because we're relying on the json
lexer/parser layer for this handling, but it should work reliably for all
current/previous versions of qemu-ga.

I'll make sure to fix up the documentation.
 
 Regards,
 Michal
 

Re: [Qemu-devel] [PATCH] hw/qxl.c: Fix compilation failures on 32 bit hosts

[Peter Maydell]

On 16 March 2012 14:37, Stefan Weil s...@weilnetz.de wrote:
          fprintf(stderr,
 -                qxl: %s: error: current_async = %d != %ld =
 cookie-io\n,
 +                qxl: %s: error: current_async = %d != % PRId64  =
 cookie-io\n,
                  __func__, current_async, cookie-io);



 current_async is uint32_t,  so maybe you can also replace %d by % PRIu32 .
 Is io unsigned (I don't have the headers, but think it is)? Then PRIu64
 would be better.

Yes, cookie-io is uint64_t, but I just want to fix compile warnings,
not make functional changes (it might be a semantically signed value
that we happen to be carrying in an unsigned type, for instance).

I agree that we could use PRId32 in theory, but on the other hand
it doesn't cause any compiler warnings and we also use %d for it
elsewhere in the function. I'd rather leave that kind of cleanup
for somebody who wants to deal with it more systematically.

-- PMM

Re: [Qemu-devel] [PATCH] fix incorrect bracket in tracetool

[Eric Blake]

On 03/16/2012 07:18 AM, Paolo Bonzini wrote:
 Il 16/03/2012 13:29, Lee Essen ha scritto:
 @@ -123,7 +123,7 @@ get_argc()
  # Get the format string including double quotes for a trace event
  get_fmt()
  {
 -puts ${1#*)}

This says to call puts with the first argument of get_fmt, except with
the shortest prefix ending in ) omitted.  Or are you complaining that
there is a shell treating this as a syntax error?

 +puts ${1#*}

This says to omit the shortest prefix that matches the glob '*', but
that is always the empty string, so you might as well write it $1.

  }
  
 
 Eric, can you look at this?  Is it a bashism or a Solaris bug?

Solaris /bin/sh lacks support for ${var#pattern}, but POSIX requires it.
 If this is using #!/bin/sh, you aren't portable.

 
 I would write it, to be entirely safe, as
 
local fmt

local is not portable.

fmt=${1#*\)}
puts $fmt

This looks reasonable, if you were hitting syntax errors on an unquoted
), and if you are sure that you have a POSIX rather than Solaris /bin/sh.

 
 where I'm using the extra variable to avoid the ambiguity of quoting the
 parentheses within quotes; variable assignments are always implicitly
 quoted.
 
 Paolo
 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] Guest-sync-delimited and sentinel issue

[Eric Blake]

On 03/16/2012 06:47 AM, Michal Privoznik wrote:
 Hi guys,
 
 I was just implementing support for guest-sync-delimited into libvirt. My 
 intent is to issue this command prior any other command to determine if GA is 
 available or not. The big advantage is - it doesn't change the state of the 
 guest so from libvirt POV it's harmless. The other big advantage is this 
 sentinel byte 0xFF which is supposed to flush all unprocessed (and possibly 
 stale) data from previous unsuccessful tries.
 
 As written in documentation, this command will output sentinel byte to the 
 guest agent socket. This works perfectly. However, it is advised in the very 
 same documentation to prepend this command with the sentinel as well allowing 
 GA parser flush. But this doesn't work for me completely. All I can get is:
 
 $ echo -e \xFF{\execute\:\guest-sync-delimited\, 
 \arguments\:{\id\:1234}} | nc -U /tmp/ga.sock | hexdump -C

side note - echo -e is non-portable; I would have written this as:

printf '\xff{execute:guest-sync-delimited, arguments:{id:1234}}'

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] virtio-spec: split virtio-net device status filed into ro and rw byte

[Jason Wang]

This patch splits the device status field of virtio-net into ro and rw
byte. This would simplify the implementation of both host and guest
and make the layout more clean. As VIRTIO_NET_S_ANNOUNCE is a rw bit,
it was moved to bit 8 (0x100).

btw. looks like there's no implementation that depends on
VIRTIO_NET_S_ANNOUNCE, so the move is safe.

Signed-off-by: Jason Wang jasow...@redhat.com
---
 virtio-0.9.4.lyx |   20 +++-
 1 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/virtio-0.9.4.lyx b/virtio-0.9.4.lyx
index 6c7bab1..ef3951c 100644
--- a/virtio-0.9.4.lyx
+++ b/virtio-0.9.4.lyx
@@ -58,6 +58,7 @@
 \html_be_strict false
 \author -608949062 Rusty Russell,,, 
 \author 1531152142 pbonzini 
+\author 2090695081 Jason 
 \end_header
 
 \begin_body
@@ -4012,8 +4013,19 @@ configuration
 layout Two configuration fields are currently defined.
  The mac address field always exists (though is only valid if VIRTIO_NET_F_MAC
  is set), and the status field only exists if VIRTIO_NET_F_STATUS is set.
+
+\change_inserted 2090695081 1331907586
+ The low byte of status field is read-only, guest write to this byte would
+ be ignored.
+ Currently only one bit is defined for this byte: VIRTIO_NET_S_LINK_UP.
+ The high byte of status field is read-writable.
+ Currently only one bit is defined for this byte: VIRTIO_NET_S_ANNOUNCE.
+
+\change_deleted 2090695081 1331907489
  Two bits are currently defined for the status field: VIRTIO_NET_S_LINK_UP
  and VIRTIO_NET_S_ANNOUNCE.
+
+\change_unchanged
  
 \begin_inset listings
 inline false
@@ -4026,7 +4038,13 @@ status open
 
 \begin_layout Plain Layout
 
-#define VIRTIO_NET_S_ANNOUNCE  2
+#define VIRTIO_NET_S_ANNOUNCE  
+\change_inserted 2090695081 1331907493
+0x100
+\change_deleted 2090695081 1331907491
+2
+\change_unchanged
+
 \end_layout
 
 \begin_layout Plain Layout

Re: [Qemu-devel] [V5 PATCH 1/4] net: announce self after vm start

[Jason Wang]

On 03/16/2012 06:45 PM, Paolo Bonzini wrote:

Il 16/03/2012 11:13, Jason Wang ha scritto:

  The problem with staying in the INMIGRATE is that we can not figure out
  when the migration is completed when using '-S', so this kind of
  transition were forbidden by qmp_cont().
  
  Looks like we need a new state such as RUN_STATE_MIGRATE_PRELAUNCH?

Or just a global need_announce instead of looking at the runstate.

Paolo

Then I think it's better for us introduce a parameter for vm_start() 
like what we've done in V4.

[Qemu-devel] [Bug 954099] Re: Assertion failed arp_table.c line 41 on raspberry pi fedora image boot up

[Akarsh]

This bug is related to ARM based processor emulation and has been fixed
in Linaro QEMU(qemu-linaro). You can install qemu-linaro to solve the
problem. This can be confirmed with the following link(Point 6) :
http://www.cnx-software.com/2012/03/08/instructions-to-run-raspberry-pi-
fedora-14-remix-in-qemu/

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

Title:
  Assertion failed arp_table.c line 41 on raspberry pi fedora image boot
  up

Status in QEMU:
  New

Bug description:
  OS Win XP pro, 32 bit SP3
  Intel Core Duo, 4G RAM.

  Qemu 1.0.1

  Launch command:
  qemu-system-arm.exe -M versatilepb -cpu arm1136-r2 -hda 
raspberrypi-fedora-remix-14-r1.img -kernel zImage-devtmpfs -m 192 -append 
root=/dev/sda2 -vga std -net nic -net user -localtime

  Starting HAL daemon: eth0: link up
  Assert fires :
  File : slirp\arp_table.c line 41
  Expression (ip_addr  htonl(~0xf  28))) 1=0

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

Re: [Qemu-devel] [PATCH v6 2/6] Introduce save_devices

[Eric Blake]

On 03/15/2012 02:34 PM, Luiz Capitulino wrote:
 On Thu, 15 Mar 2012 15:16:15 -0500
 Anthony Liguori anth...@codemonkey.ws wrote:
 
 On 03/15/2012 01:19 PM, Stefano Stabellini wrote:
 - add an is_ram flag to SaveStateEntry;

 - register_savevm_live sets is_ram for live_savevm devices;

 - introduce a save_devices QAPI command that can be used to save
 the state of all devices, but not the RAM or the block devices of the
 VM.


 +SQMP
 +save_devices
 +---
 +
 +Save the state of all devices to file. The RAM and the block devices
 +of the VM are not saved by this command.
 +
 +Arguments:
 +
 +- filename: the file to save the state of the devices to as binary
 +data. See save_devices.txt for a description of the binary format.
 +
 +Example:
 +
 +-  { execute: save_devices, arguments: { filename: /tmp/save } }

Why are we adding yet another command without support for passing in an
fd instead of having qemu open the file name?  Libvirt really does want
to move to an ability to pass in fds, and the more commands we add that
open files, the harder converting them all will be.

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [Bug 899961] Re: qemu/kvm locks up when run 32bit userspace with 64bit kernel

[Michael Tokarev]

Forgot to mention: tcg appears to be unaffected - so far anyway.

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

Title:
  qemu/kvm locks up when run 32bit userspace with 64bit kernel

Status in QEMU:
  New
Status in “qemu-kvm” package in Debian:
  Confirmed

Bug description:
  Only applies to qemu-kvm 1.0, and only when kernel is 64bit and
  userspace is 32bit, on x86.  Did not happen with previous released
  versions, such as 0.15.  Not all guests triggers this issue - so far,
  only (32bit) windows 7 guest shows it, but does that quite reliable:
  first boot of an old guest with new qemu-kvm, windows finds a new CPU
  and suggests rebooting - hit Reboot and in a few seconds it will be
  locked up (including the monitor), with no CPU usage whatsoever.
  Killable only with -9.

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

[Qemu-devel] [Bug 899961] Re: qemu/kvm locks up when run 32bit userspace with 64bit kernel

[Michael Tokarev]

After some more digging I found out that qemu also has this very issue,
but it happens a bit differently.  In particular, this very same winXP
test guest freezes in upstream qemu with -enable-kvm on _shutdown_, not
on restart.  In qemu-kvm it happens on restart but not on shutdown.

And bisecting plain qemu leads to this commit:

commit 12d4536f7d911b6d87a766ad7300482ea663cea2
Author: Anthony Liguori aligu...@us.ibm.com
Date:   Mon Aug 22 08:24:58 2011 -0500

main: force enabling of I/O thread

As far as I remember, qemu-kvm always had iothread enabled, that's why the bug 
initially was only reproducible on qemu-kvm.

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

Title:
  qemu/kvm locks up when run 32bit userspace with 64bit kernel

Status in QEMU:
  New
Status in “qemu-kvm” package in Debian:
  Confirmed

Bug description:
  Only applies to qemu-kvm 1.0, and only when kernel is 64bit and
  userspace is 32bit, on x86.  Did not happen with previous released
  versions, such as 0.15.  Not all guests triggers this issue - so far,
  only (32bit) windows 7 guest shows it, but does that quite reliable:
  first boot of an old guest with new qemu-kvm, windows finds a new CPU
  and suggests rebooting - hit Reboot and in a few seconds it will be
  locked up (including the monitor), with no CPU usage whatsoever.
  Killable only with -9.

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

Re: [Qemu-devel] [PATCH v7 2/6] Introduce save-devices-state

[Eric Blake]

On 03/16/2012 06:13 AM, Stefano Stabellini wrote:
 - add an is_ram flag to SaveStateEntry;
 
 - register_savevm_live sets is_ram for live_savevm devices;
 
 - introduce a save-devices-state QAPI command that can be used to save
 the state of all devices, but not the RAM or the block devices of the
 VM.
 

 +QEMU has code to load/save the state of the guest that it is running.
 +These are two complementary operations.  Saving the state just does
 +that, saves the state for each device that the guest is running.
 +
 +These operations are normally used with migration (see migration.txt),
 +however it is also possible to save the state of all devices to file,
 +without saving the RAM or the block devices of the VM.
 +
 +This operation is called save-devices-state (see QMP/qmp-commands.txt)

Is there a complimentary load-devices-state?

Just to make sure I'm clear, there are three things to save before you
have a complete picture of a running VM:

disk state (can be saved with 'savevm' to internal qcow2 snapshots, or
with 'transaction' and 'blockdev-snapshot-sync' by creating external
snapshots, or by 'migrate' to a file)

RAM state (can be saved with 'savevm' to internal qcow2 snapshot, or
with 'migrate' to a file; it is also possible to start qemu with a
command line parameter telling which backing file tracks RAM state)

device state (can be saved with 'savevm' to internal qcow2 snapshot, or
with 'migrate' to a file, and now with 'save-devices-state')

That is, 'migrate' does all three at once to an external file, 'savevm'
does all three at once to an internal qcow2 snapshot, and you are now
making it possible to do any one of the three independently to an
external file while the VM continues to run.

Is this something that libvirt should be exposing in the near future?

 +SQMP
 +save-devices-state
 +---
 +
 +Save the state of all devices to file. The RAM and the block devices
 +of the VM are not saved by this command.
 +
 +Arguments:
 +
 +- filename: the file to save the state of the devices to as binary
 +data. See save-devices-state.txt for a description of the binary format.
 +
 +Example:
 +
 +- { execute: save-devices-state, arguments: { filename: /tmp/save 
 } }

Can we at least reserve something for future extension to add things
like 'fd:name' to refer to a named fd received earlier via 'getfd'?  You
could do that by documenting up front that if filename does not start
with /, then any pattern of letters followed by a ':' as the initial
pattern of the filename is an error (and you avoid the error by using
./foo:... or an absolute path).

-- 
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 v7 2/6] Introduce save-devices-state

[Anthony Liguori]

On 03/16/2012 10:49 AM, Eric Blake wrote:

On 03/16/2012 06:13 AM, Stefano Stabellini wrote:

- add an is_ram flag to SaveStateEntry;

- register_savevm_live sets is_ram for live_savevm devices;

- introduce a save-devices-state QAPI command that can be used to save
the state of all devices, but not the RAM or the block devices of the
VM.




+QEMU has code to load/save the state of the guest that it is running.
+These are two complementary operations.  Saving the state just does
+that, saves the state for each device that the guest is running.
+
+These operations are normally used with migration (see migration.txt),
+however it is also possible to save the state of all devices to file,
+without saving the RAM or the block devices of the VM.
+
+This operation is called save-devices-state (see QMP/qmp-commands.txt)


Is there a complimentary load-devices-state?


Maybe we should call this xen-save-devices-state.

As far as I'm concerned, this is a Xen specific interface, not an interface for 
general consumption.


I would not expect libvirt to use this interface.  If libvirt wants an interface 
that only saves device state, I'd much rather do it properly through QMP such 
that what was returned was well structured and in JSON.


Regards,

Anthony Liguori



Just to make sure I'm clear, there are three things to save before you
have a complete picture of a running VM:

disk state (can be saved with 'savevm' to internal qcow2 snapshots, or
with 'transaction' and 'blockdev-snapshot-sync' by creating external
snapshots, or by 'migrate' to a file)

RAM state (can be saved with 'savevm' to internal qcow2 snapshot, or
with 'migrate' to a file; it is also possible to start qemu with a
command line parameter telling which backing file tracks RAM state)

device state (can be saved with 'savevm' to internal qcow2 snapshot, or
with 'migrate' to a file, and now with 'save-devices-state')

That is, 'migrate' does all three at once to an external file, 'savevm'
does all three at once to an internal qcow2 snapshot, and you are now
making it possible to do any one of the three independently to an
external file while the VM continues to run.

Is this something that libvirt should be exposing in the near future?


+SQMP
+save-devices-state
+---
+
+Save the state of all devices to file. The RAM and the block devices
+of the VM are not saved by this command.
+
+Arguments:
+
+- filename: the file to save the state of the devices to as binary
+data. See save-devices-state.txt for a description of the binary format.
+
+Example:
+
+-  { execute: save-devices-state, arguments: { filename: /tmp/save 
} }


Can we at least reserve something for future extension to add things
like 'fd:name' to refer to a named fd received earlier via 'getfd'?  You
could do that by documenting up front that if filename does not start
with /, then any pattern of letters followed by a ':' as the initial
pattern of the filename is an error (and you avoid the error by using
./foo:... or an absolute path).

Re: [Qemu-devel] [PATCH 1/5] configure to set shell type

[Eric Blake]

On 03/16/2012 06:35 AM, Peter Maydell wrote:
 The issue here is really just getting a fully POSIX-conformant shell.
 And the way I expect this to work is by executing configure and make in
 such a shell and by not having hardcoded /bin/sh creep in through some
 shebang line.
 
 The way I expect this to work is that /bin/sh should be a posix shell...

Then your expectations are wrong.  POSIX itself says that /bin/sh need
not be the POSIX shell, and merely requires that you can query for a
conforming PATH to use, and that the 'sh' found on that PATH search is
the POSIX shell (that is, 'command -p sh' will be conforming, but won't
necessarily be /bin/sh).  This weasel-wording is intentionally written
specifically for Solaris, since they refuse to make /bin/sh
POSIX-conforming (it might break 30-year-old legacy scripts - gasp!),
and instead set their conforming PATH to have /usr/xpg4/bin (or these
days, /usr/xpg6/bin:/usr/xpg4/bin) prior to /bin.

-- 
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 v6 2/6] Introduce save_devices

[Stefano Stabellini]

On Fri, 16 Mar 2012, Eric Blake wrote:
 On 03/15/2012 02:34 PM, Luiz Capitulino wrote:
  On Thu, 15 Mar 2012 15:16:15 -0500
  Anthony Liguori anth...@codemonkey.ws wrote:
  
  On 03/15/2012 01:19 PM, Stefano Stabellini wrote:
  - add an is_ram flag to SaveStateEntry;
 
  - register_savevm_live sets is_ram for live_savevm devices;
 
  - introduce a save_devices QAPI command that can be used to save
  the state of all devices, but not the RAM or the block devices of the
  VM.
 
 
  +SQMP
  +save_devices
  +---
  +
  +Save the state of all devices to file. The RAM and the block devices
  +of the VM are not saved by this command.
  +
  +Arguments:
  +
  +- filename: the file to save the state of the devices to as binary
  +data. See save_devices.txt for a description of the binary format.
  +
  +Example:
  +
  +-  { execute: save_devices, arguments: { filename: /tmp/save 
  } }
 
 Why are we adding yet another command without support for passing in an
 fd instead of having qemu open the file name?  Libvirt really does want
 to move to an ability to pass in fds, and the more commands we add that
 open files, the harder converting them all will be.

This command is not meant to be used by libvirt directly: it is meant to
be used by libxenlight. Libvirt is just going to call
libxl_domain_suspend and behind the scenes libxl is going to take care
of everything.

[Qemu-devel] [Bug 954099] Re: Assertion failed arp_table.c line 41 on raspberry pi fedora image boot up

[Peter Maydell]

It's probably also fixed in upstream qemu master since I haven't
deliberately put anything in to qemu-linaro to fix it -- we've almost
certainly just picked up the fix from upstream.

Incidentally -M versatilepb -cpu arm1136-r2 is veering slightly into
unsupported territory, since there's no such thing as a VersatilePB
board with an 1136 CPU in the real world. And did you really want
arm1136-r2? That's an r0p2, whereas arm1136 is the newer r1pX which is
probably what the RPi is actually using. (Yes, qemu's names for these
two CPU types are hopelessly confusing. Sorry.)

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

Title:
  Assertion failed arp_table.c line 41 on raspberry pi fedora image boot
  up

Status in QEMU:
  New

Bug description:
  OS Win XP pro, 32 bit SP3
  Intel Core Duo, 4G RAM.

  Qemu 1.0.1

  Launch command:
  qemu-system-arm.exe -M versatilepb -cpu arm1136-r2 -hda 
raspberrypi-fedora-remix-14-r1.img -kernel zImage-devtmpfs -m 192 -append 
root=/dev/sda2 -vga std -net nic -net user -localtime

  Starting HAL daemon: eth0: link up
  Assert fires :
  File : slirp\arp_table.c line 41
  Expression (ip_addr  htonl(~0xf  28))) 1=0

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

Re: [Qemu-devel] [PATCH v7 2/6] Introduce save-devices-state

[Stefano Stabellini]

On Fri, 16 Mar 2012, Eric Blake wrote:
 On 03/16/2012 06:13 AM, Stefano Stabellini wrote:
  - add an is_ram flag to SaveStateEntry;
  
  - register_savevm_live sets is_ram for live_savevm devices;
  
  - introduce a save-devices-state QAPI command that can be used to save
  the state of all devices, but not the RAM or the block devices of the
  VM.
  
 
  +QEMU has code to load/save the state of the guest that it is running.
  +These are two complementary operations.  Saving the state just does
  +that, saves the state for each device that the guest is running.
  +
  +These operations are normally used with migration (see migration.txt),
  +however it is also possible to save the state of all devices to file,
  +without saving the RAM or the block devices of the VM.
  +
  +This operation is called save-devices-state (see QMP/qmp-commands.txt)
 
 Is there a complimentary load-devices-state?

The generic loadvm function can be used with the device state on Xen,
see below.


 Just to make sure I'm clear, there are three things to save before you
 have a complete picture of a running VM:
 
 disk state (can be saved with 'savevm' to internal qcow2 snapshots, or
 with 'transaction' and 'blockdev-snapshot-sync' by creating external
 snapshots, or by 'migrate' to a file)
 
 RAM state (can be saved with 'savevm' to internal qcow2 snapshot, or
 with 'migrate' to a file; it is also possible to start qemu with a
 command line parameter telling which backing file tracks RAM state)
 
 device state (can be saved with 'savevm' to internal qcow2 snapshot, or
 with 'migrate' to a file, and now with 'save-devices-state')
 
 That is, 'migrate' does all three at once to an external file, 'savevm'
 does all three at once to an internal qcow2 snapshot, and you are now
 making it possible to do any one of the three independently to an
 external file while the VM continues to run.
 
 Is this something that libvirt should be exposing in the near future?

I don't think so. It is useful under Xen where the RAM state and the
disk state are saved elsewhere. Libvirt is going to be a consumer of
this interface but only through libxenlight.


 Can we at least reserve something for future extension to add things
 like 'fd:name' to refer to a named fd received earlier via 'getfd'?  You
 could do that by documenting up front that if filename does not start
 with /, then any pattern of letters followed by a ':' as the initial
 pattern of the filename is an error (and you avoid the error by using
 ./foo:... or an absolute path).
 
Yes, that can be done.

Re: [Qemu-devel] [PATCH v7 2/6] Introduce save-devices-state

[Eric Blake]

On 03/16/2012 09:58 AM, Anthony Liguori wrote:

 +These operations are normally used with migration (see migration.txt),
 +however it is also possible to save the state of all devices to file,
 +without saving the RAM or the block devices of the VM.
 +
 +This operation is called save-devices-state (see
 QMP/qmp-commands.txt)

 Is there a complimentary load-devices-state?
 
 Maybe we should call this xen-save-devices-state.
 
 As far as I'm concerned, this is a Xen specific interface, not an
 interface for general consumption.

Fair enough - and renaming it would certainly help.

 
 I would not expect libvirt to use this interface.

Good to know.  In that case, adding an fd: interface isn't a priority,
after all.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] Guest-sync-delimited and sentinel issue

[Michal Privoznik]

On 16.03.2012 15:49, Michael Roth wrote:
 On Fri, Mar 16, 2012 at 01:47:42PM +0100, Michal Privoznik wrote:
 Hi guys,

 I was just implementing support for guest-sync-delimited into libvirt. My 
 intent is to issue this command prior any other command to determine if GA 
 is available or not. The big advantage is - it doesn't change the state of 
 the guest so from libvirt POV it's harmless. The other big advantage is this 
 sentinel byte 0xFF which is supposed to flush all unprocessed (and possibly 
 stale) data from previous unsuccessful tries.
 
 Are you opening the qemu-ga socket prior to each command? Or just
 once at startup? If you're only opening it once, it should be sufficient
 to do the guest-sync/guest-sync-delimited exchange just once at that time,
 since the streams are presumably synced at that point, and after that only if
 you get a client-side timeout.

Only once at the guest startup or libvirt daemon startup if the guest is
already running.
 
 Issuing it prior to each command doesn't guarantee that the agent will
 be available to handle the command, so you still need be prepared to
 handle a timeout + re-sync. It does reduce the chances of timing out on
 doing something that affects guest state though... but if that's the
 intention here I would recommend just using 'guest-ping', which should
 work reliably so long as you always re-sync on connect and after
 client-side timeouts.

Since GA may come and go I don't see any guaranteed algorithm that would
work for 100%. And I try to avoid using timeouts for state changing
commands, because if I choose a timeout which can look reasonable now,
somebody will hit it sooner or later; take fs-freeze as example.
Moreover, if we timeout on a state changing command, libvirt would have
to keep track of such command anyway because if/when it returns, libvirt
must issue counter command right after: fs-freeze vs. fs-thaw. However,
only iff command returned successfully.
Things get complicated if two or more commands timeout, e.g. due to GA
not being running.
And since GA doesn't support passing ID into GA commands it's hard to
keep track which of expired commands succeeded and which failed. Okay,
nowdays GA executes commands sequentially, but that may change in the
future. We are not allowing timeouts on monitor neither.

Or maybe I am just too timid and see problems which are not really
problems :)

 
 But it doesn't really matter either way, all I'm really getting at is that
 scanning for the 0xFF delimiter in the response shouldn't be *necessary* in
 this case. But there's no harm in using it that way. You don't need to
 precede the request with 0xFF if you're just using it to probe for the
 agent though, and you probably wouldn't want to given that it results in
 2 responses:
 

 As written in documentation, this command will output sentinel byte to the 
 guest agent socket. This works perfectly. However, it is advised in the very 
 same documentation to prepend this command with the sentinel as well 
 allowing GA parser flush. But this doesn't work for me completely. All I can 
 get is:

 $ echo -e \xFF{\execute\:\guest-sync-delimited\, 
 \arguments\:{\id\:1234}} | nc -U /tmp/ga.sock | hexdump -C
 nc: using stream socket
   7b 22 65 72 72 6f 72 22  3a 20 7b 22 63 6c 61 73  |{error: 
 {clas|
 0010  73 22 3a 20 22 4a 53 4f  4e 50 61 72 73 69 6e 67  |s: 
 JSONParsing|
 0020  22 2c 20 22 64 61 74 61  22 3a 20 7b 7d 7d 7d 0a  |, data: 
 {}}}.|
 0030  ff 7b 22 72 65 74 75 72  6e 22 3a 20 31 32 33 34  |.{return: 
 1234|
 0040  7d 0a |}.|
 0042

 The problem is - GA has difficulties with parsing sentinel, although the 
 reply is correct, indeed.
 Therefore my question is - should I just drop passing sentinel to GA? And 
 even if this is fixed, How should I deal with older releases which have this 
 bug?
 
 Sorry, I didn't document this properly. Haven't tested host-guest flush
 in a while and got it in my head that it was handled silently, but what
 you're seeing has actually always been the observed/intended behavior.
 
 Depending on how you've implemented guest-sync-delimited it might not make
 a difference though. If you're just ignoring any data up until you see
 the 0xFF sentinel value then the error is silently thrown away as garbage. The
 semantics of the command are that you may read garbage prior to the
 sentinel+response, it's just that when preceeding the request with the
 sentinel this is *always* the case.
 
 Otherwise, if you're handling it like a normal request, when sending the 
 0xFF
 you would treat it basically as a flush command that always returns a
 JSONParsing error. It's not pretty because we're relying on the json
 lexer/parser layer for this handling, but it should work reliably for all
 current/previous versions of qemu-ga.
 
 I'll make sure to fix up the documentation.

 Regards,
 Michal

 

Anyway, thank you for clarify; I'll stick to guest-sync then.

Re: [Qemu-devel] [PATCHv4 01/11] virtio-serial-bus: use correct lengths in control_out() message

[Anthony Liguori]

On 03/15/2012 04:00 PM, Michael Tokarev wrote:

In case of more than one control message, the code will use
size of the largest message so far for all subsequent messages,
instead of using size of current one.  Fix it.

Signed-off-by: Michael Tokarevm...@tls.msk.ru
---
  hw/virtio-serial-bus.c |6 +++---
  1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index e22940e..abe48ec 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -454,7 +454,7 @@ static void control_out(VirtIODevice *vdev, VirtQueue *vq)
  len = 0;
  buf = NULL;
  while (virtqueue_pop(vq,elem)) {
-size_t cur_len, copied;
+size_t cur_len;

  cur_len = iov_size(elem.out_sg, elem.out_num);
  /*
@@ -467,9 +467,9 @@ static void control_out(VirtIODevice *vdev, VirtQueue *vq)
  buf = g_malloc(cur_len);
  len = cur_len;
  }
-copied = iov_to_buf(elem.out_sg, elem.out_num, buf, 0, len);
+iov_to_buf(elem.out_sg, elem.out_num, buf, 0, cur_len);


I don't understand what this is fixing.  copied = cur_len unless for some reason 
a full copy couldn't be done.


But you're assuming a full copy is done.  So I'm confused by what is being fixed 
here.


Regards,

Anthony Liguori


-handle_control_message(vser, buf, copied);
+handle_control_message(vser, buf, cur_len);
  virtqueue_push(vq,elem, 0);
  }
  g_free(buf);

Re: [Qemu-devel] [PATCHv4 02/11] change iov_* function prototypes to be more appropriate

[Anthony Liguori]

On 03/15/2012 04:00 PM, Michael Tokarev wrote:

Reorder arguments to be more natural, readable and
consistent with other iov_* functions, and change
argument names, from:
  iov_from_buf(iov, iov_cnt, buf, iov_off, size)
to
  iov_from_buf(iov, iov_cnt, offset, buf, bytes)

The result becomes natural English:


I don't think this is a good idea.  This is code churn for nothing but cosmetic 
reasons.  I don't think there's a lot of value in that.




  copy data to this `iov' vector with `iov_cnt'
  elements starting at byte offset `offset'
  from memory buffer `buf', processing `bytes'
  bytes max.

(Try to read the original prototype this way).

Also change iov_clear() to more general iov_memset()
(it uses memset() internally anyway).

While at it, add comments to the header file
describing what the routines actually does.

The patch only renames argumens in the header, but
keeps old names in the implementation.  The next
patch will touch actual code to match.

Now, it might look wrong to pay so much attention
to so small things.  But we've so many badly designed
interfaces already so the whole thing becomes rather
confusing or error prone.  One example of this is
previous commit and small discussion which emerged
from it, with an outcome that the utility functions
like these aren't well-understdandable, leading to
strange usage cases.  That's why I paid quite some
attention to this set of functions and a few
others in subsequent patches.

Signed-off-by: Michael Tokarevm...@tls.msk.ru
---
  hw/rtl8139.c   |2 +-
  hw/usb/core.c  |6 +++---
  hw/virtio-balloon.c|4 ++--
  hw/virtio-net.c|4 ++--
  hw/virtio-serial-bus.c |6 +++---
  iov.c  |   14 +++---
  iov.h  |   42 +-
  net.c  |2 +-
  8 files changed, 56 insertions(+), 24 deletions(-)

diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 05b8e1e..e837079 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -1799,7 +1799,7 @@ static void rtl8139_transfer_frame(RTL8139State *s, 
uint8_t *buf, int size,
  if (iov) {
  buf2_size = iov_size(iov, 3);
  buf2 = g_malloc(buf2_size);
-iov_to_buf(iov, 3, buf2, 0, buf2_size);
+iov_to_buf(iov, 3, 0, buf2, buf2_size);
  buf = buf2;
  }

diff --git a/hw/usb/core.c b/hw/usb/core.c
index a4048fe..4a213aa 100644
--- a/hw/usb/core.c
+++ b/hw/usb/core.c
@@ -516,10 +516,10 @@ void usb_packet_copy(USBPacket *p, void *ptr, size_t 
bytes)
  switch (p-pid) {
  case USB_TOKEN_SETUP:
  case USB_TOKEN_OUT:
-iov_to_buf(p-iov.iov, p-iov.niov, ptr, p-result, bytes);
+iov_to_buf(p-iov.iov, p-iov.niov, p-result, ptr, bytes);
  break;
  case USB_TOKEN_IN:
-iov_from_buf(p-iov.iov, p-iov.niov, ptr, p-result, bytes);
+iov_from_buf(p-iov.iov, p-iov.niov, p-result, ptr, bytes);
  break;
  default:
  fprintf(stderr, %s: invalid pid: %x\n, __func__, p-pid);
@@ -533,7 +533,7 @@ void usb_packet_skip(USBPacket *p, size_t bytes)
  assert(p-result= 0);
  assert(p-result + bytes= p-iov.size);
  if (p-pid == USB_TOKEN_IN) {
-iov_clear(p-iov.iov, p-iov.niov, p-result, bytes);
+iov_memset(p-iov.iov, p-iov.niov, p-result, 0, bytes);
  }
  p-result += bytes;
  }
diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index ce9d2c9..8f0cf33 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -77,7 +77,7 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, 
VirtQueue *vq)
  size_t offset = 0;
  uint32_t pfn;

-while (iov_to_buf(elem.out_sg, elem.out_num,pfn, offset, 4) == 4) {
+while (iov_to_buf(elem.out_sg, elem.out_num, offset,pfn, 4) == 4) {
  ram_addr_t pa;
  ram_addr_t addr;

@@ -118,7 +118,7 @@ static void virtio_balloon_receive_stats(VirtIODevice 
*vdev, VirtQueue *vq)
   */
  reset_stats(s);

-while (iov_to_buf(elem-out_sg, elem-out_num,stat, offset, sizeof(stat))
+while (iov_to_buf(elem-out_sg, elem-out_num, offset,stat, sizeof(stat))
 == sizeof(stat)) {
  uint16_t tag = tswap16(stat.tag);
  uint64_t val = tswap64(stat.val);
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index bc5e3a8..65a516f 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -656,8 +656,8 @@ static ssize_t virtio_net_receive(VLANClientState *nc, 
const uint8_t *buf, size_
  }

  /* copy in packet.  ugh */
-len = iov_from_buf(sg, elem.in_num,
-   buf + offset, 0, size - offset);
+len = iov_from_buf(sg, elem.in_num, 0,
+   buf + offset, size - offset);
  total += len;
  offset += len;
  /* If buffers can't be merged, at this point we
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index abe48ec..41a62d1 100644
--- 

Re: [Qemu-devel] [PATCHv4 03/11] rewrite iov_* functions

[Anthony Liguori]

On 03/15/2012 04:00 PM, Michael Tokarev wrote:

This changes implementations of all iov_*
functions, completing the previous step.

All iov_* functions now ensure that this offset
argument is within the iovec (using assertion),
but lets to specify `bytes' value larger than
actual length of the iovec - in this case they
stops at the actual end of iovec.  It is also
suggested to use convinient `-1' value as `bytes'
to mean just this -- up to the end.

There's one very minor semantic change here: new
requiriment is that `offset' points to inside of
iovec.  This is checked just at the end of functions
(assert()), it does not actually need to be enforced,
but using any of these functions with offset pointing
past the end of iovec is wrong anyway.

Note: the new code in iov.c uses arithmetic with
void pointers.  I thought this is not supported
everywhere and is a GCC extension (indeed, the C
standard does not define void arithmetic).  However,
the original code already use void arith in
iov_from_buf() function:
   (memcpy(..., buf + buf_off,...)
which apparently works well so far (it is this
way in qemu 1.0).  So I left it this way and used
it in other places.

Signed-off-by: Michael Tokarevm...@tls.msk.ru
---
  iov.c |   91 -
  iov.h |   12 +++-
  2 files changed, 49 insertions(+), 54 deletions(-)

diff --git a/iov.c b/iov.c
index bc58cab..fd518dd 100644
--- a/iov.c
+++ b/iov.c
@@ -7,6 +7,7 @@
   * Author(s):
   *  Anthony Liguorialigu...@us.ibm.com
   *  Amit Shahamit.s...@redhat.com
+ *  Michael Tokarevm...@tls.msk.ru
   *
   * This work is licensed under the terms of the GNU GPL, version 2.  See
   * the COPYING file in the top-level directory.
@@ -17,75 +18,61 @@

  #include iov.h

-size_t iov_from_buf(struct iovec *iov, unsigned int iov_cnt, size_t iov_off,
-const void *buf, size_t size)
+size_t iov_from_buf(struct iovec *iov, unsigned int iov_cnt,
+size_t offset, const void *buf, size_t bytes)
  {
-size_t iovec_off, buf_off;
+size_t done;
  unsigned int i;
-
-iovec_off = 0;
-buf_off = 0;
-for (i = 0; i  iov_cnt  size; i++) {
-if (iov_off  (iovec_off + iov[i].iov_len)) {
-size_t len = MIN((iovec_off + iov[i].iov_len) - iov_off, size);
-
-memcpy(iov[i].iov_base + (iov_off - iovec_off), buf + buf_off, 
len);
-
-buf_off += len;
-iov_off += len;
-size -= len;
+for (i = 0, done = 0; done  bytes  i  iov_cnt; i++) {
+if (offset  iov[i].iov_len) {
+size_t len = MIN(iov[i].iov_len - offset, bytes - done);
+memcpy(iov[i].iov_base + offset, buf + done, len);
+done += len;
+offset = 0;
+} else {
+offset -= iov[i].iov_len;
  }
-iovec_off += iov[i].iov_len;
  }
-return buf_off;
+assert(offset == 0);
+return done;
  }


It makes me nervous to make a change like this as it has wide impact on the rest 
of the code base.  Could you include a unit test that tests the various boundary 
conditions of this code?


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCHv4 03/11] rewrite iov_* functions

[Anthony Liguori]

On 03/15/2012 04:00 PM, Michael Tokarev wrote:

This changes implementations of all iov_*
functions, completing the previous step.

All iov_* functions now ensure that this offset
argument is within the iovec (using assertion),
but lets to specify `bytes' value larger than
actual length of the iovec - in this case they
stops at the actual end of iovec.  It is also
suggested to use convinient `-1' value as `bytes'
to mean just this -- up to the end.

There's one very minor semantic change here: new
requiriment is that `offset' points to inside of
iovec.  This is checked just at the end of functions
(assert()), it does not actually need to be enforced,
but using any of these functions with offset pointing
past the end of iovec is wrong anyway.

Note: the new code in iov.c uses arithmetic with
void pointers.  I thought this is not supported
everywhere and is a GCC extension (indeed, the C
standard does not define void arithmetic).  However,
the original code already use void arith in
iov_from_buf() function:
   (memcpy(..., buf + buf_off,...)
which apparently works well so far (it is this
way in qemu 1.0).  So I left it this way and used
it in other places.

Signed-off-by: Michael Tokarevm...@tls.msk.ru
---
  iov.c |   91 -
  iov.h |   12 +++-
  2 files changed, 49 insertions(+), 54 deletions(-)

diff --git a/iov.c b/iov.c
index bc58cab..fd518dd 100644
--- a/iov.c
+++ b/iov.c
@@ -7,6 +7,7 @@
   * Author(s):
   *  Anthony Liguorialigu...@us.ibm.com
   *  Amit Shahamit.s...@redhat.com
+ *  Michael Tokarevm...@tls.msk.ru
   *
   * This work is licensed under the terms of the GNU GPL, version 2.  See
   * the COPYING file in the top-level directory.
@@ -17,75 +18,61 @@

  #include iov.h

-size_t iov_from_buf(struct iovec *iov, unsigned int iov_cnt, size_t iov_off,
-const void *buf, size_t size)
+size_t iov_from_buf(struct iovec *iov, unsigned int iov_cnt,
+size_t offset, const void *buf, size_t bytes)
  {
-size_t iovec_off, buf_off;
+size_t done;
  unsigned int i;
-
-iovec_off = 0;
-buf_off = 0;
-for (i = 0; i  iov_cnt  size; i++) {
-if (iov_off  (iovec_off + iov[i].iov_len)) {
-size_t len = MIN((iovec_off + iov[i].iov_len) - iov_off, size);
-
-memcpy(iov[i].iov_base + (iov_off - iovec_off), buf + buf_off, 
len);
-
-buf_off += len;
-iov_off += len;
-size -= len;
+for (i = 0, done = 0; done  bytes  i  iov_cnt; i++) {
+if (offset  iov[i].iov_len) {
+size_t len = MIN(iov[i].iov_len - offset, bytes - done);
+memcpy(iov[i].iov_base + offset, buf + done, len);
+done += len;
+offset = 0;
+} else {
+offset -= iov[i].iov_len;
  }
-iovec_off += iov[i].iov_len;
  }
-return buf_off;
+assert(offset == 0);
+return done;
  }


It makes me nervous to make a change like this as it has wide impact on the rest 
of the code base.  Could you include a unit test that tests the various boundary 
conditions of this code?


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 1/5] configure to set shell type

[Peter Maydell]

On 16 March 2012 15:58, Eric Blake ebl...@redhat.com wrote:
 On 03/16/2012 06:35 AM, Peter Maydell wrote:
 The way I expect this to work is that /bin/sh should be a posix shell...

 Then your expectations are wrong.  POSIX itself says that /bin/sh need
 not be the POSIX shell, and merely requires that you can query for a
 conforming PATH to use, and that the 'sh' found on that PATH search is
 the POSIX shell (that is, 'command -p sh' will be conforming, but won't
 necessarily be /bin/sh).

Yes, well strictly POSIX says that If the first line of a file of shell
commands starts with the characters #! , the results are unspecified,
so we're already in the realm of undefined behaviour if we put anything
at the top of our shell scripts.

  This weasel-wording is intentionally written
 specifically for Solaris, since they refuse to make /bin/sh
 POSIX-conforming (it might break 30-year-old legacy scripts - gasp!),
 and instead set their conforming PATH to have /usr/xpg4/bin (or these
 days, /usr/xpg6/bin:/usr/xpg4/bin) prior to /bin.

Google suggests that Solaris 11 finally kicks the legacy bourne shell
out of /bin/sh...

(I'm mostly just pushing back a little at uglifying our build scripts
for the sake of a weirdo outlier if there's a different fix (eg in this
case it looks like the script really is a bash script) that's less ugly.)

-- PMM

Re: [Qemu-devel] [PATCHv4 05/11] allow qemu_iovec_from_buffer() to specify offset from which to start copying

[Anthony Liguori]

On 03/15/2012 04:00 PM, Michael Tokarev wrote:

Similar to
  qemu_iovec_memset(QEMUIOVector *qiov, size_t offset,
int c, size_t bytes);
the new prototype is:
  qemu_iovec_from_buf(QEMUIOVector *qiov, size_t offset,
  const void *buf, size_t bytes);

The processing starts at offset bytes within qiov.

This way, we may copy a bounce buffer directly to
a middle of qiov.

This is exactly the same function as iov_from_buf() from
iov.c, so use the existing implementation and rename it
to qemu_iovec_from_buf() to be shorter and to match the
utility function.

As with utility implementation, we now assert that the
offset is inside actual iovec.  Nothing changed for
current callers, because `offset' parameter is new.

While at it, stop using bounce-qiov in block/qcow2.c
and copy decrypted data directly from cluster_data
instead of recreating a temp qiov for doing that
(Cc'ing kwolf for this change).

Signed-off-by: Michael Tokarevm...@tls.msk.ru
Cc: Kevin Wolfkw...@redhat.com


Kevin, please Ack.

Regards,

Anthony Liguori


---
  block.c   |6 +++---
  block/curl.c  |6 +++---
  block/qcow.c  |2 +-
  block/qcow2.c |9 +++--
  block/vdi.c   |2 +-
  cutils.c  |   16 +++-
  qemu-common.h |3 ++-
  7 files changed, 16 insertions(+), 28 deletions(-)

diff --git a/block.c b/block.c
index b88ee90..b8db395 100644
--- a/block.c
+++ b/block.c
@@ -1696,8 +1696,8 @@ static int coroutine_fn 
bdrv_co_do_copy_on_readv(BlockDriverState *bs,
  }

  skip_bytes = (sector_num - cluster_sector_num) * BDRV_SECTOR_SIZE;
-qemu_iovec_from_buffer(qiov, bounce_buffer + skip_bytes,
-   nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_from_buf(qiov, 0, bounce_buffer + skip_bytes,
+nb_sectors * BDRV_SECTOR_SIZE);

  err:
  qemu_vfree(bounce_buffer);
@@ -3244,7 +3244,7 @@ static void bdrv_aio_bh_cb(void *opaque)
  BlockDriverAIOCBSync *acb = opaque;

  if (!acb-is_write)
-qemu_iovec_from_buffer(acb-qiov, acb-bounce, acb-qiov-size);
+qemu_iovec_from_buf(acb-qiov, 0, acb-bounce, acb-qiov-size);
  qemu_vfree(acb-bounce);
  acb-common.cb(acb-common.opaque, acb-ret);
  qemu_bh_delete(acb-bh);
diff --git a/block/curl.c b/block/curl.c
index e9102e3..cfc2ced 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -142,8 +142,8 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t 
nmemb, void *opaque)
  continue;

  if ((s-buf_off= acb-end)) {
-qemu_iovec_from_buffer(acb-qiov, s-orig_buf + acb-start,
-   acb-end - acb-start);
+qemu_iovec_from_buf(acb-qiov, 0, s-orig_buf + acb-start,
+acb-end - acb-start);
  acb-common.cb(acb-common.opaque, 0);
  qemu_aio_release(acb);
  s-acb[i] = NULL;
@@ -178,7 +178,7 @@ static int curl_find_buf(BDRVCURLState *s, size_t start, 
size_t len,
  {
  char *buf = state-orig_buf + (start - state-buf_start);

-qemu_iovec_from_buffer(acb-qiov, buf, len);
+qemu_iovec_from_buf(acb-qiov, 0, buf, len);
  acb-common.cb(acb-common.opaque, 0);

  return FIND_RET_OK;
diff --git a/block/qcow.c b/block/qcow.c
index b1cfe1f..562a19c 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -540,7 +540,7 @@ done:
  qemu_co_mutex_unlock(s-lock);

  if (qiov-niov  1) {
-qemu_iovec_from_buffer(qiov, orig_buf, qiov-size);
+qemu_iovec_from_buf(qiov, 0, orig_buf, qiov-size);
  qemu_vfree(orig_buf);
  }

diff --git a/block/qcow2.c b/block/qcow2.c
index 941a6a9..a24c0dc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -476,7 +476,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState 
*bs, int64_t sector_num,
  goto fail;
  }

-qemu_iovec_from_buffer(hd_qiov,
+qemu_iovec_from_buf(hd_qiov, 0,
  s-cluster_cache + index_in_cluster * 512,
  512 * cur_nr_sectors);
  } else {
@@ -514,11 +514,8 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState 
*bs, int64_t sector_num,
  if (s-crypt_method) {
  qcow2_encrypt_sectors(s, sector_num,  cluster_data,
  cluster_data, cur_nr_sectors, 0,s-aes_decrypt_key);
-qemu_iovec_reset(hd_qiov);
-qemu_iovec_copy(hd_qiov, qiov, bytes_done,
-cur_nr_sectors * 512);
-qemu_iovec_from_buffer(hd_qiov, cluster_data,
-512 * cur_nr_sectors);
+qemu_iovec_from_buf(qiov, bytes_done,
+cluster_data, 512 * cur_nr_sectors);
  }
  }

diff --git a/block/vdi.c b/block/vdi.c
index 6a0011f..24f4027 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -635,7 +635,7 @@ static void vdi_aio_read_cb(void *opaque, int ret)
  return;
  done:

Re: [Qemu-devel] [PATCHv4 09/11] export iov_send_recv() and use it in iov_send() and iov_recv()

[Anthony Liguori]

On 03/15/2012 04:00 PM, Michael Tokarev wrote:

Rename do_sendv_recvv() to iov_send_recv(), change its last arg
(do_send) from int to bool, export it in iov.h, and made the two
callers of it (iov_send() and iov_recv()) to be trivial #defines
just adding 5th arg.

iov_send_recv() will be used later.

Signed-off-by: Michael Tokarevm...@tls.msk.ru
---
  cutils.c |   17 +++--
  iov.h|   10 +++---
  2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/cutils.c b/cutils.c
index 2ad5fa3..cb6f638 100644
--- a/cutils.c
+++ b/cutils.c
@@ -376,9 +376,9 @@ int qemu_parse_fd(const char *param)
  return fd;
  }

-static ssize_t do_sendv_recvv(int sockfd, struct iovec *iov,
-  size_t offset, size_t bytes,
-  int do_sendv)
+ssize_t iov_send_recv(int sockfd, struct iovec *iov,
+  size_t offset, size_t bytes,
+  bool do_sendv)
  {
  int iovlen;
  ssize_t ret;
@@ -458,14 +458,3 @@ static ssize_t do_sendv_recvv(int sockfd, struct iovec 
*iov,
  last_iov-iov_len += diff;
  return ret;
  }
-
-ssize_t iov_recv(int sockfd, struct iovec *iov, size_t offset, size_t bytes)
-{
-return do_sendv_recvv(sockfd, iov, offset, bytes, 0);
-}
-
-ssize_t iov_send(int sockfd, struct iovec *iov, size_t offset, size_t bytes)
-{
-return do_sendv_recvv(sockfd, iov, offset, bytes, 1);
-}
-
diff --git a/iov.h b/iov.h
index 5aa2f45..9b6a883 100644
--- a/iov.h
+++ b/iov.h
@@ -60,7 +60,7 @@ size_t iov_memset(const struct iovec *iov, const unsigned int 
iov_cnt,
   * `offset' bytes in the beginning of iovec buffer are skipped and
   * next `bytes' bytes are used, which must be within data of iovec.
   *
- *   r = iov_send(sockfd, iov, offset, bytes);
+ *   r = iov_send_recv(sockfd, iov, offset, bytes, true);
   *
   * is logically equivalent to
   *
@@ -69,8 +69,12 @@ size_t iov_memset(const struct iovec *iov, const unsigned 
int iov_cnt,
   *   r = send(sockfd, buf, bytes, 0);
   *   free(buf);
   */
-ssize_t iov_recv(int sockfd, struct iovec *iov, size_t offset, size_t bytes);
-ssize_t iov_send(int sockfd, struct iovec *iov, size_t offset, size_t bytes);
+ssize_t iov_send_recv(int sockfd, struct iovec *iov,
+  size_t offset, size_t bytes, bool do_send);
+#define iov_recv(sockfd, iov, offset, bytes) \
+  iov_send_recv(sockfd, iov, offset, bytes, false)
+#define iov_send(sockfd, iov, offset, bytes) \
+  iov_send_recv(sockfd, iov, offset, bytes, true)


Please use a static inline instead of a macro.  Macros make compiler 
errors/warnings confusing.


Regards,

Anthony Liguori



  /**
   * Produce a text hexdump of iovec `iov' with `iov_cnt' number of elements

Re: [Qemu-devel] [PATCHv4 10/11] cleanup qemu_co_sendv(), qemu_co_recvv() and friends

[Anthony Liguori]

On 03/15/2012 04:00 PM, Michael Tokarev wrote:

The same as for non-coroutine versions in previous
patches: rename arguments to be more obvious, change
type of arguments from int to size_t where appropriate,
and use common code for send and receive paths (with
one extra argument) since these are exactly the same.
Use common iov_send_recv() directly.

qemu_co_sendv(), qemu_co_recvv(), and qemu_co_recv()
are now trivial #define's merely adding one extra arg.

qemu_co_sendv() and qemu_co_recvv() callers are
converted to different argument order and extra
`iov_cnt' argument.

Cc: MORITA Kazutakamorita.kazut...@lab.ntt.co.jp
Cc: Paolo Bonzinipbonz...@redhat.com
Signed-off-by: Michael Tokarevm...@tls.msk.ru


Same comments here re: macros.

Regards,

Anthony Liguori


---
  block/nbd.c |   16 +-
  block/sheepdog.c|6 ++--
  qemu-common.h   |   37 ++
  qemu-coroutine-io.c |   82 +++---
  4 files changed, 53 insertions(+), 88 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 161b299..c451a25 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -184,7 +184,7 @@ static void nbd_restart_write(void *opaque)
  }

  static int nbd_co_send_request(BDRVNBDState *s, struct nbd_request *request,
-   struct iovec *iov, int offset)
+   QEMUIOVector *qiov, int offset)
  {
  int rc, ret;

@@ -193,8 +193,8 @@ static int nbd_co_send_request(BDRVNBDState *s, struct 
nbd_request *request,
  qemu_aio_set_fd_handler(s-sock, nbd_reply_ready, nbd_restart_write,
  nbd_have_request, NULL, s);
  rc = nbd_send_request(s-sock, request);
-if (rc != -1  iov) {
-ret = qemu_co_sendv(s-sock, iov, request-len, offset);
+if (rc != -1  qiov) {
+ret = qemu_co_sendv(s-sock, qiov-iov, qiov-niov, offset, 
request-len);
  if (ret != request-len) {
  errno = -EIO;
  rc = -1;
@@ -209,7 +209,7 @@ static int nbd_co_send_request(BDRVNBDState *s, struct 
nbd_request *request,

  static void nbd_co_receive_reply(BDRVNBDState *s, struct nbd_request *request,
   struct nbd_reply *reply,
- struct iovec *iov, int offset)
+ QEMUIOVector *qiov, int offset)
  {
  int ret;

@@ -220,8 +220,8 @@ static void nbd_co_receive_reply(BDRVNBDState *s, struct 
nbd_request *request,
  if (reply-handle != request-handle) {
  reply-error = EIO;
  } else {
-if (iov  reply-error == 0) {
-ret = qemu_co_recvv(s-sock, iov, request-len, offset);
+if (qiov  reply-error == 0) {
+ret = qemu_co_recvv(s-sock, qiov-iov, qiov-niov, offset, 
request-len);
  if (ret != request-len) {
  reply-error = EIO;
  }
@@ -336,7 +336,7 @@ static int nbd_co_readv_1(BlockDriverState *bs, int64_t 
sector_num,
  if (nbd_co_send_request(s,request, NULL, 0) == -1) {
  reply.error = errno;
  } else {
-nbd_co_receive_reply(s,request,reply, qiov-iov, offset);
+nbd_co_receive_reply(s,request,reply, qiov, offset);
  }
  nbd_coroutine_end(s,request);
  return -reply.error;
@@ -360,7 +360,7 @@ static int nbd_co_writev_1(BlockDriverState *bs, int64_t 
sector_num,
  request.len = nb_sectors * 512;

  nbd_coroutine_start(s,request);
-if (nbd_co_send_request(s,request, qiov-iov, offset) == -1) {
+if (nbd_co_send_request(s,request, qiov, offset) == -1) {
  reply.error = errno;
  } else {
  nbd_co_receive_reply(s,request,reply, NULL, 0);
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 00276f6f..e688e49 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -657,8 +657,8 @@ static void coroutine_fn aio_read_response(void *opaque)
  }
  break;
  case AIOCB_READ_UDATA:
-ret = qemu_co_recvv(fd, acb-qiov-iov, rsp.data_length,
-aio_req-iov_offset);
+ret = qemu_co_recvv(fd, acb-qiov-iov, acb-qiov-niov,
+aio_req-iov_offset, rsp.data_length);
  if (ret  0) {
  error_report(failed to get the data, %s, strerror(errno));
  goto out;
@@ -924,7 +924,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState 
*s, AIOReq *aio_req,
  }

  if (wlen) {
-ret = qemu_co_sendv(s-fd, iov, wlen, aio_req-iov_offset);
+ret = qemu_co_sendv(s-fd, iov, niov, aio_req-iov_offset, wlen);
  if (ret  0) {
  qemu_co_mutex_unlock(s-lock);
  error_report(failed to send a data, %s, strerror(errno));
diff --git a/qemu-common.h b/qemu-common.h
index d4a0243..d9858d1 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -297,32 +297,29 @@ struct qemu_work_item {
  void qemu_init_vcpu(void *env);
  #endif

-/**
- * Sends an iovec (or optionally a part of it) down a