[Qemu-devel] [PATCH] hw/i386/pc: reject to boot a wrong header magic kernel

2013-03-27 Thread liguang
if head magic is missing or wrong unexpectedly, we'd
better to reject booting.
e.g.
I make a mistake to boot a vmlinuz for MIPS(which
I think it's for x86) like this:
qemu-system-x86_64 -kernel vmlinuz -initrd demord
then qemu report:
qemu: linux kernel too old to load a ram disk
that's misleading.

Signed-off-by: liguang lig.f...@cn.fujitsu.com
---
 hw/i386/pc.c |4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index b1e06fa..2b78dfc 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -683,8 +683,10 @@ static void load_linux(void *fw_cfg,
 if (load_multiboot(fw_cfg, f, kernel_filename, initrd_filename,
kernel_cmdline, kernel_size, header)) {
 return;
+} else {
+fprintf(stderr, please assure specicified kernel is for x86!\n);
+exit(1);
 }
-protocol = 0;
 }
 
 if (protocol  0x200 || !(header[0x211]  0x01)) {
-- 
1.7.2.5




Re: [Qemu-devel] [PATCH v3] block: Add support for Secure Shell (ssh) block device.

2013-03-27 Thread Stefan Hajnoczi
On Tue, Mar 26, 2013 at 07:34:43PM +, Richard W.M. Jones wrote:
 
 BTW, I'd be interested in tips on what software/techniques people use
 for benchmarking qemu block devices.
 
 So far I've been testing this by booting various guests and timing how
 long they take to reach various stages (eg. graphical login), but this
 has not proven to be reproducible.

Running fio inside the guest is the way to go for real benchmarking:
http://freecode.com/projects/fio

For quick benchmarks you could use one more more dd(1) processes:
dd if=/dev/vdb of=/dev/null iflag=direct bs=64k

Stefan



Re: [Qemu-devel] [PATCH 1/4] net: ensure socket backend uses non-blocking fds

2013-03-27 Thread Stefan Hajnoczi
On Tue, Mar 26, 2013 at 05:25:50PM +0100, Juan Quintela wrote:
 Stefan Hajnoczi stefa...@redhat.com wrote:
  There are several code paths in net_init_socket() depending on how the
  socket is created: file descriptor passing, UDP multicast, TCP, or UDP.
  Some of these support both listen and connect.
 
  Not all code paths set the socket to non-blocking.  This patch addresses
  the file descriptor passing and UDP cases which were missing
  socket_set_nonblock(fd) calls.
 
  I considered moving socket_set_nonblock(fd) to a central location but it
  turns out the code paths are different enough to require non-blocking at
  different places.
 
  Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
  ---
   net/socket.c | 2 ++
   1 file changed, 2 insertions(+)
 
  diff --git a/net/socket.c b/net/socket.c
  index 6c3752b..4a88142 100644
  --- a/net/socket.c
  +++ b/net/socket.c
  @@ -674,6 +674,7 @@ static int net_socket_udp_init(NetClientState *peer,
   closesocket(fd);
   return -1;
   }
  +socket_set_nonblock(fd);
   
   s = net_socket_fd_init(peer, model, name, fd, 0);
   if (!s) {
  @@ -712,6 +713,7 @@ int net_init_socket(const NetClientOptions *opts, const 
  char *name,
   int fd;
   
   fd = monitor_handle_fd_param(cur_mon, sock-fd);
  +socket_set_nonblock(fd);
   if (fd == -1 || !net_socket_fd_init(peer, socket, name, fd, 1)) {
 
 Shouldn't be better to refactor the code to _not_ call
 socket_set_nonblock() in the case that fd == -1?  It will just clobber
 errno?

I'll send a fixed version.

Stefan



Re: [Qemu-devel] [PATCH 1/4] net: ensure socket backend uses non-blocking fds

2013-03-27 Thread Stefan Hajnoczi
On Tue, Mar 26, 2013 at 10:33:45AM -0600, Eric Blake wrote:
 On 03/26/2013 10:07 AM, Stefan Hajnoczi wrote:
  There are several code paths in net_init_socket() depending on how the
  socket is created: file descriptor passing, UDP multicast, TCP, or UDP.
  Some of these support both listen and connect.
  
  Not all code paths set the socket to non-blocking.  This patch addresses
  the file descriptor passing and UDP cases which were missing
  socket_set_nonblock(fd) calls.
  
  I considered moving socket_set_nonblock(fd) to a central location but it
  turns out the code paths are different enough to require non-blocking at
  different places.
 
 Is it worth rearranging patch 3 first, so that you don't have to churn
 on these newly-added lines?

Will do that in v2.

Stefan



Re: [Qemu-devel] [PATCH 0/5] Add some tracepoints for clarification of the cause of troubles

2013-03-27 Thread Kazuya Saito
(2013/03/26 16:15), Paolo Bonzini wrote:
 Il 26/03/2013 08:13, Kazuya Saito ha scritto:

 I'm not sure 4-5 are that useful, but the first 3 patches are definitely
 good stuff.
 Thanks. I'll modify the patch you pointed out about CPU number and
 re-post it.
 I'd like to add tracepoints to the virtual device creation/deletion
 part.  I had an issue that when a Windows guest OS booted up, it didn't
 have a virtual NIC device which it should have had.  It took us time to
 figure out which the guest OS or the QEMU had the issue.  If I have
 had the tracepoints in the virtual device creation/deletion should have
 eased the situation much better.
 
 Wouldn't you get the same information from the command line?

I think the information you said is different from what I meant. The
information I wanted to know is whether QEMU creates/deletes a device
successfully or not. We cannot get it from the command line.
I was sure I specified a NIC device for the guest, but the Windows guest
didn't have it when it booted.  There were two possibilities.  One was
QEMU failed to create the NIC device, and the other was QEMU created it
successfully and the Windows guest failed to detect it.  Since QEMU
output limited message at the moment, it was difficult to prove that
it was not QEMU's issue.  I took a coredump of the QEMU and proved the
QEMU must have a valid structures for the NIC device.  I believe the
tracepoints could have allowed me to figure out where the issue existed
a lot faster and easier.  (And it turned out it was the Windows' issue.)

Kazuya





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

2013-03-27 Thread Wenchao Xia
Hi, Luiz
  Personally I hope reduce the dynamic allocated buffer which brings
fragments and unexpected memory grow. Instead, how about sacrifice
some time to wait output complete, since monitor is not time critical?
in this case static buffer's size can decide how many work can be
postponded. Following is my suggestion:

--- a/monitor.c
+++ b/monitor.c
@@ -293,17 +293,28 @@ static void monitor_puts(Monitor *mon, const char
*str)
 {
 char c;

+/* if mux do not put in any thing to buffer */
+if (mon-mux_out) {
+return;
+}
+
 for(;;) {
-assert(mon-outbuf_index  sizeof(mon-outbuf) - 1);
+if (mon-outbuf_index = sizeof(mon-outbuf) - 1) {
+/* when buffer is full, flush it and retry. If buffer is
bigger, more
+   work can be postponed. */
+monitor_flush(mon);
+usleep(1);
+continue;
+}
 c = *str++;
 if (c == '\0')
 break;
 if (c == '\n')
 mon-outbuf[mon-outbuf_index++] = '\r';
 mon-outbuf[mon-outbuf_index++] = c;
-if (mon-outbuf_index = (sizeof(mon-outbuf) - 1)
-|| c == '\n')
+if (c == '\n') {
 monitor_flush(mon);
+}
 }
 }

 Commit f628926bb423fa8a7e0b114511400ea9df38b76a changed monitor_flush()
 to retry on qemu_chr_fe_write() errors. However, the Monitor's output
 buffer can keep growing while the retry is not issued and this can
 cause the buffer to overflow.
 
 To reproduce this issue, just start qemu and type on the Monitor:
 
 (qemu) ?
 
 This will cause the assertion to trig.
 
 To fix this problem this commit makes the Monitor buffer dynamic,
 which means that it can grow as much as needed.
 
 Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
 ---
   monitor.c | 42 +-
   1 file changed, 25 insertions(+), 17 deletions(-)
 




Re: [Qemu-devel] [PATCH 2/2] qtest: use synchronous I/O for char device

2013-03-27 Thread Wenchao Xia
Reviewed-by: Wenchao Xia xiaw...@linux.vnet.ibm.com

 Peter reported that rtc-test would periodically hang.  It turns out
 this was due to an EAGAIN occurring on qemu_chr_fe_write.
 
 Instead of heavily refactoring qtest, just use a synchronous version
 of the write operation for qemu_chr_fe_write to address this problem.
 
 Reported-by: Peter Maydell peter.mayd...@linaro.org
 Signed-off-by: Anthony Liguori aligu...@us.ibm.com
 ---
   qtest.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/qtest.c b/qtest.c
 index 5e0e9ec..b03b68a 100644
 --- a/qtest.c
 +++ b/qtest.c
 @@ -191,7 +191,7 @@ static void GCC_FMT_ATTR(2, 3) qtest_send(CharDriverState 
 *chr,
   len = vsnprintf(buffer, sizeof(buffer), fmt, ap);
   va_end(ap);
 
 -qemu_chr_fe_write(chr, (uint8_t *)buffer, len);
 +qemu_chr_fe_write_all(chr, (uint8_t *)buffer, len);
   if (qtest_log_fp  qtest_opened) {
   fprintf(qtest_log_fp, %s, buffer);
   }
 


-- 
Best Regards

Wenchao Xia




Re: [Qemu-devel] [PATCH 1/2] char: introduce a blocking version of qemu_chr_fe_write

2013-03-27 Thread Wenchao Xia

于 2013-3-26 23:21, Peter Maydell 写道:

On 26 March 2013 15:11, Anthony Liguori aligu...@us.ibm.com wrote:

+int qemu_chr_fe_write_all(CharDriverState *s, const uint8_t *buf, int len)
+{
+int offset = 0;
+int res;
+
+while (offset  len) {
+do {
+res = s-chr_write(s, buf + offset, len - offset);
+if (res == -1  errno == EAGAIN) {
+g_usleep(100);
+}
+} while (res == -1  errno == EAGAIN);


for (;;) {
res = s-chr_write(s, buf + offset, len - offset);
if (!(res == -1  errno == EAGAIN)) {
break;
}
g_usleep(100);
}

would avoid the duplication of the retry condition.

-- PMM


I think adjust like following make code easier to read:

while (offset  len) {
res = s-chr_write(s, buf + offset, len - offset);
if (res == -1  errno == EAGAIN) {
g_usleep(100)
continue;
}
.
}
--
Best Regards

Wenchao Xia




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

2013-03-27 Thread li guang
在 2013-03-26二的 11:20 -0300,Eduardo Habkost写道:
 On Wed, Mar 20, 2013 at 02:18:00PM +0800, li guang wrote:
  在 2013-01-09三的 01:08 +0100,Andreas Färber写道:
   Am 18.12.2012 13:41, schrieb Vasilis Liaskovitis:
Because dimm layout needs to be configured on machine-boot, all dimm 
devices
need to be specified on startup command line (either with populated=on 
or with
populated=off). The dimm information is stored in dimm configuration 
structures.

After machine startup, dimms are hot-added or removed with normal 
device_add
and device_del operations e.g.:
Hot-add syntax: device_add dimm,id=mydimm0,bus=membus.0
Hot-remove syntax: device_del dimm,id=mydimm0
   
   This sounds contradictory: Either all devices need to be specified on
   the command line, or they can be hot-added via monitor.
   
   Assuming a fixed layout at startup, I wonder if there is another clever
   way to model this... For CPU hotplug Anthony had suggested to have a
   fixed set of linkSocket properties that get set to a CPU socket as
   needed. Might a similar strategy work for memory, i.e. a
   startup-configured amount of linkDIMMs on /machine/dimm[n] that point
   to a QOM DIMM object or NULL if unpopulated? Hot(un)plug would then
   simply work via QMP qom-set command. (CC'ing some people)
  
  
  Sorry, what's link, did it adopted by cpu-QOM?
 
 link... is a QOM construct, allowing properties that point to other
 objects. We don't use it on the CPU objects yet.
 
  can you give some hints?
 
 Look for mentions of link in the doc comments at include/qom/object.h.
 

OK, I see, Thanks!




Re: [Qemu-devel] Qemu-devel Digest, Vol 120, Issue 887

2013-03-27 Thread Haitham Zuriq





Sent from Samsung tablet

 Original message 
From: qemu-devel-requ...@nongnu.org 
Date: 27/03/2013  9:12 AM  (GMT+03:00) 
To: qemu-devel@nongnu.org 
Subject: Qemu-devel Digest, Vol 120, Issue 887 
 
Send Qemu-devel mailing list submissions to
qemu-devel@nongnu.org

To subscribe or unsubscribe via the World Wide Web, visit
https://lists.nongnu.org/mailman/listinfo/qemu-devel
or, via email, send a message with subject or body 'help' to
qemu-devel-requ...@nongnu.org

You can reach the person managing the list at
qemu-devel-ow...@nongnu.org

When replying, please edit your Subject line so it is more specific
than Re: Contents of Qemu-devel digest...


Today's Topics:

   1. Re: coroutine: hung when using gthread backend (Wenchao Xia)
   2. Re: [RFC PATCH v4 00/30] ACPI memory hotplug (li guang)
   3. Re: [RFC PATCH v4 00/30] ACPI memory hotplug (li guang)
   4. Re: [RFC] qmp interface for save vmstate to image (Wenchao Xia)
   5. [Bug 1158912] Re: QEMU Version 1.4.0 - SLIRP hangs VM
  (Kenneth Salerno)
   6. [PATCH] hw/i386/pc: reject to boot a wrong header magic
  kernel (liguang)


--

Message: 1
Date: Wed, 27 Mar 2013 10:11:57 +0800
From: Wenchao Xia xiaw...@linux.vnet.ibm.com
To: Peter Maydell peter.mayd...@linaro.org
Cc: Stefan Hajnoczi stefa...@gmail.com, qemu-devel
qemu-devel@nongnu.org,Paolo Bonzini pbonz...@redhat.com
Subject: Re: [Qemu-devel] coroutine: hung when using gthread backend
Message-ID: 5152556d.4050...@linux.vnet.ibm.com
Content-Type: text/plain; charset=UTF-8; format=flowed

? 2013-3-26 17:56, Peter Maydell ??:
 On 26 March 2013 09:54, Stefan Hajnoczi stefa...@gmail.com wrote:
 On Tue, Mar 26, 2013 at 08:03:50AM +0100, Paolo Bonzini wrote:
 coroutine backend gthread hardly works for qemu, only qemu-io and qemu-img.

 Do you know why it doesn't work?

 Because nobody tests it?

 -- PMM

   It is not enabled by default in configure, so missed in tests. I feel
a full regression test suit covering different configure case is
missing.

-- 
Best Regards

Wenchao Xia




--

Message: 2
Date: Wed, 27 Mar 2013 10:42:00 +0800
From: li guang lig.f...@cn.fujitsu.com
To: Vasilis Liaskovitis vasilis.liaskovi...@profitbricks.com
Cc: g...@redhat.com, stefa...@gmail.com, seab...@seabios.org,
qemu-devel@nongnu.org, blauwir...@gmail.com, ke...@koconnor.net,Gerd
Hoffmann kra...@redhat.com, anth...@codemonkey.ws
Subject: Re: [Qemu-devel] [RFC PATCH v4 00/30] ACPI memory hotplug
Message-ID: 1364352120.31713.23.ca...@liguang.fnst.cn.fujitsu.com
Content-Type: text/plain; charset=UTF-8

? 2013-03-26?? 17:58 +0100?Vasilis Liaskovitis???
 Hi,
 
 On Tue, Mar 19, 2013 at 02:30:25PM +0800, li guang wrote:
  ? 2013-01-10?? 19:57 +0100?Vasilis Liaskovitis???
 
 IIRC q35 supports memory hotplug natively (picked up in some
 discussion).  Is that correct?
 
From previous discussion I also understand that q35 supports native 
hotplug. 
Sections 5.1 and 5.2 of the spec describe the MCH registers but the 
native
memory hotplug specifics are not yet clear to me. Any pointers from the
spec are welcome.
   
   Ping. Could anyone who's familiar with the q35 spec provide some pointers 
   on
   native memory hotplug details in the spec? I see pcie hotplug registers 
   but can't
   find memory hotplug interface details. If I am not mistaken, the spec is 
   here:
   http://www.intel.com/design/chipsets/datashts/316966.htm
   
   Is the q35 memory hotplug support supposed to be an shpc-like interface 
   geared
   towards memory slots instead of pci slots?
   
  
  seems there's no so-called q35-native support
 
 that was also my first impression when scanning the specification. Wasn't 
 native
 memory hotplug capabilities one of the reasons that q35 got picked as the next
 pc chipset?

Um, I can't find the original statement of q35,
but I think if we can't find in intel's official
SPEC, then we have to say 'there's no q35-native support'.





--

Message: 3
Date: Wed, 27 Mar 2013 10:54:05 +0800
From: li guang lig.f...@cn.fujitsu.com
To: Vasilis Liaskovitis vasilis.liaskovi...@profitbricks.com
Cc: g...@redhat.com, stefa...@gmail.com, seab...@seabios.org,
qemul...@gmail.com, qemu-devel@nongnu.org, blauwir...@gmail.com,
ke...@koconnor.net, Erlon Cruz sombra...@gmail.com,
kra...@redhat.com, anth...@codemonkey.ws
Subject: Re: [Qemu-devel] [RFC PATCH v4 00/30] ACPI memory hotplug
Message-ID: 1364352845.31713.26.ca...@liguang.fnst.cn.fujitsu.com
Content-Type: text/plain; charset=UTF-8

? 2013-03-26?? 17:43 +0100?Vasilis Liaskovitis???
 Hi,
 
 On Tue, Mar 19, 2013 at 03:28:38PM +0800, li guang wrote:
 [...]
 This is v4 of the ACPI memory hotplug functionality. Only x86_64 
 target is
 supported (both i440fx and q35). There are still several issues, but 
 it's
 been a while since v3 and I wanted to get some more 

Re: [Qemu-devel] [patch]Make GTK build on OS X

2013-03-27 Thread Paolo Bonzini
Il 26/03/2013 23:16, C.W. Betts ha scritto:
 This patch makes the GTK UI build on OS X by including the right headers.
 
 
 
 From b5cc84343f479d4870961c82fc7b384637e9616c Mon Sep 17 00:00:00 2001
 From: C.W. Betts computer...@hotmail.com
 Date: Sun, 24 Mar 2013 11:24:05 -0600
 Subject: [PATCH 1/3] Make the GTK UI build on OS X.
 
 ---
  ui/gtk.c | 5 +
  1 file changed, 5 insertions(+)
 
 diff --git a/ui/gtk.c b/ui/gtk.c
 index 305940d..e2948d7 100644
 --- a/ui/gtk.c
 +++ b/ui/gtk.c
 @@ -54,7 +54,12 @@
  #include sys/socket.h
  #include sys/un.h
  #include sys/wait.h
 +#ifdef __APPLE__
 +#include termios.h
 +#include util.h
 +#else
  #include pty.h
 +#endif
  #include math.h
  
  #include ui/console.h
 

termios.h can be included unconditionally.  For util.h and pty.h, there
is already similar code in qemu-char.c:

#if defined(__GLIBC__)
#include pty.h
#elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) ||
defined(__DragonFly__)
#include libutil.h
#else
#include util.h
#endif

Please move it to include/qemu-common.h instead so that there is no
duplication.

Paolo



Re: [Qemu-devel] [PATCH 0/4] monitor: do not rely on O_NONBLOCK for passed file descriptors

2013-03-27 Thread liu ping fan
On Wed, Mar 27, 2013 at 12:07 AM, Stefan Hajnoczi stefa...@redhat.com wrote:
 There are several places where QEMU accidentally relies on the O_NONBLOCK 
 state
 of passed file descriptors.  Exposing O_NONBLOCK state makes it part of the 
 QMP

If in future, we push more backend on their dedicated thread, will the
related fd be block?
 API whenever getfd or fdset_add_fd are used!

 Whether or not QEMU will use O_NONBLOCK is an implementation detail and should
 be hidden from QMP clients.

 This patch series addresses this in 3 steps:

 1. Fix callers of monitor_handle_fd_param(), monitor_fdset_get_fd(), and
monitor_get_fd() that depend on O_NONBLOCK being set.  Luckily there are
only two instances and they are fixed in Patches 1  2.

 2. Rename socket_set_nonblock() to qemu_set_nonblock() just like
qemu_set_cloexec().  This makes code cleaner when working with arbitrary
file descriptors that may not be sockets.  See Patch 3.

 3. Clear O_NONBLOCK when a chardev receives file descriptors.  From now on 
 QEMU
can assume that passed file descriptors are in blocking mode.  Simply use
qemu_set_nonblock(fd) if you want to enable O_NONBLOCK.  See Patch 4.

 This fixes live migration with recent libvirt.  Libvirt checks if QEMU 
 supports
 file descriptor passing and, if yes, hands QEMU a socket with O_NONBLOCK set.
 The migrate fd:foo code assumes the socket is in blocking mode.  The result
 is a corrupted migration stream.  For more info on this bug, see:

 https://bugzilla.redhat.com/show_bug.cgi?id=923124

 Note that Michal Privoznik mpriv...@redhat.com also sent a libvirt patch so
 that old QEMUs work with new libvirts:

 https://www.redhat.com/archives/libvir-list/2013-March/msg01486.html

 My patch series fixes the QMP API and allows old libvirts to work again with
 new QEMUs.

 Stefan Hajnoczi (4):
   net: ensure socket backend uses non-blocking fds
   qemu-socket: set passed fd non-blocking in socket_connect()
   oslib-posix: rename socket_set_nonblock() to qemu_set_nonblock()
   chardev: clear O_NONBLOCK on SCM_RIGHTS file descriptors

  block/nbd.c|  2 +-
  block/sheepdog.c   |  2 +-
  include/qemu/sockets.h |  4 ++--
  migration.c|  2 +-
  nbd.c  |  8 
  net/socket.c   |  8 +---
  qemu-char.c| 11 +++
  savevm.c   |  2 +-
  slirp/misc.c   |  2 +-
  slirp/tcp_subr.c   |  4 ++--
  ui/vnc.c   |  2 +-
  util/oslib-posix.c |  4 ++--
  util/oslib-win32.c |  4 ++--
  util/qemu-sockets.c|  5 +++--
  14 files changed, 33 insertions(+), 27 deletions(-)

 --
 1.8.1.4





Re: [Qemu-devel] [PATCH 0/5] Add some tracepoints for clarification of the cause of troubles

2013-03-27 Thread Paolo Bonzini

  Wouldn't you get the same information from the command line?
 
 I think the information you said is different from what I meant. The
 information I wanted to know is whether QEMU creates/deletes a device
 successfully or not.

Failing to create a device will always exit QEMU.  If you cannot assume
that, you're really in debugging territory and your tools should
be the info monitor commands (info qtree, info pci) or gdb...


 We cannot get it from the command line.
 I was sure I specified a NIC device for the guest, but the Windows
 guest didn't have it when it booted.  There were two possibilities.  One
 was QEMU failed to create the NIC device, and the other was QEMU created
 it successfully and the Windows guest failed to detect it.  Since QEMU
 output limited message at the moment, it was difficult to prove that
 it was not QEMU's issue.  I took a coredump of the QEMU and proved
 the QEMU must have a valid structures for the NIC device.  I believe the
 tracepoints could have allowed me to figure out where the issue
 existed a lot faster and easier. 

Had you tried info qtree or info pci?

Paolo



Re: [Qemu-devel] [RFC 1/4] block: fix I/O throttling accounting blind spot

2013-03-27 Thread Zhi Yong Wu
On Thu, Mar 21, 2013 at 10:49 PM, Stefan Hajnoczi stefa...@redhat.com wrote:
 I/O throttling relies on bdrv_acct_done() which is called when a request
 completes.  This leaves a blind spot since we only charge for completed
 requests, not submitted requests.

 For example, if there is 1 operation remaining in this time slice the
 guest could submit 3 operations and they will all be submitted
 successfully since they don't actually get accounted for until they
 complete.

 Originally we probably thought this is okay since the requests will be
 accounted when the time slice is extended.  In practice it causes
 fluctuations since the guest can exceed its I/O limit and it will be
 punished for this later on.

 Account for I/O upon submission so that I/O limits are enforced
 properly.

 Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
 ---
  block.c   | 19 ---
  include/block/block_int.h |  2 +-
  2 files changed, 9 insertions(+), 12 deletions(-)

 diff --git a/block.c b/block.c
 index 0a062c9..31fb0b0 100644
 --- a/block.c
 +++ b/block.c
 @@ -141,7 +141,6 @@ void bdrv_io_limits_disable(BlockDriverState *bs)
  bs-slice_start = 0;
  bs-slice_end   = 0;
  bs-slice_time  = 0;
 -memset(bs-io_base, 0, sizeof(bs-io_base));
If we try some concussive operations, enable I/O throttling at first,
then disable it, and then enable it, how about? I guess that
bs-slice_submitted will maybe be not correct.
  }

  static void bdrv_block_timer(void *opaque)
 @@ -1329,8 +1328,8 @@ static void bdrv_move_feature_fields(BlockDriverState 
 *bs_dest,
  bs_dest-slice_time = bs_src-slice_time;
  bs_dest-slice_start= bs_src-slice_start;
  bs_dest-slice_end  = bs_src-slice_end;
 +bs_dest-slice_submitted= bs_src-slice_submitted;
  bs_dest-io_limits  = bs_src-io_limits;
 -bs_dest-io_base= bs_src-io_base;
  bs_dest-throttled_reqs = bs_src-throttled_reqs;
  bs_dest-block_timer= bs_src-block_timer;
  bs_dest-io_limits_enabled  = bs_src-io_limits_enabled;
 @@ -3665,9 +3664,9 @@ static bool bdrv_exceed_bps_limits(BlockDriverState 
 *bs, int nb_sectors,
  slice_time = bs-slice_end - bs-slice_start;
  slice_time /= (NANOSECONDS_PER_SECOND);
  bytes_limit = bps_limit * slice_time;
 -bytes_base  = bs-nr_bytes[is_write] - bs-io_base.bytes[is_write];
 +bytes_base  = bs-slice_submitted.bytes[is_write];
  if (bs-io_limits.bps[BLOCK_IO_LIMIT_TOTAL]) {
 -bytes_base += bs-nr_bytes[!is_write] - bs-io_base.bytes[!is_write];
 +bytes_base += bs-slice_submitted.bytes[!is_write];
  }

  /* bytes_base: the bytes of data which have been read/written; and
 @@ -3683,6 +3682,7 @@ static bool bdrv_exceed_bps_limits(BlockDriverState 
 *bs, int nb_sectors,
  *wait = 0;
  }

 +bs-slice_submitted.bytes[is_write] += bytes_res;
  return false;
  }

 @@ -3725,9 +3725,9 @@ static bool bdrv_exceed_iops_limits(BlockDriverState 
 *bs, bool is_write,
  slice_time = bs-slice_end - bs-slice_start;
  slice_time /= (NANOSECONDS_PER_SECOND);
  ios_limit  = iops_limit * slice_time;
 -ios_base   = bs-nr_ops[is_write] - bs-io_base.ios[is_write];
 +ios_base   = bs-slice_submitted.ios[is_write];
  if (bs-io_limits.iops[BLOCK_IO_LIMIT_TOTAL]) {
 -ios_base += bs-nr_ops[!is_write] - bs-io_base.ios[!is_write];
 +ios_base += bs-slice_submitted.ios[!is_write];
  }

  if (ios_base + 1 = ios_limit) {
 @@ -3735,6 +3735,7 @@ static bool bdrv_exceed_iops_limits(BlockDriverState 
 *bs, bool is_write,
  *wait = 0;
  }

 +bs-slice_submitted.ios[is_write]++;
  return false;
  }

 @@ -3772,11 +3773,7 @@ static bool bdrv_exceed_io_limits(BlockDriverState 
 *bs, int nb_sectors,
  bs-slice_start = now;
  bs-slice_end   = now + bs-slice_time;

 -bs-io_base.bytes[is_write]  = bs-nr_bytes[is_write];
 -bs-io_base.bytes[!is_write] = bs-nr_bytes[!is_write];
 -
 -bs-io_base.ios[is_write]= bs-nr_ops[is_write];
 -bs-io_base.ios[!is_write]   = bs-nr_ops[!is_write];
 +memset(bs-slice_submitted, 0, sizeof(bs-slice_submitted));
  }

  elapsed_time  = now - bs-slice_start;
 diff --git a/include/block/block_int.h b/include/block/block_int.h
 index ce0aa26..b461764 100644
 --- a/include/block/block_int.h
 +++ b/include/block/block_int.h
 @@ -251,7 +251,7 @@ struct BlockDriverState {
  int64_t slice_start;
  int64_t slice_end;
  BlockIOLimit io_limits;
 -BlockIOBaseValue  io_base;
 +BlockIOBaseValue slice_submitted;
  CoQueue  throttled_reqs;
  QEMUTimer*block_timer;
  bool io_limits_enabled;
 --
 1.8.1.4





-- 
Regards,

Zhi Yong Wu



Re: [Qemu-devel] [PATCH 0/4] monitor: do not rely on O_NONBLOCK for passed file descriptors

2013-03-27 Thread Stefan Hajnoczi
On Wed, Mar 27, 2013 at 04:37:52PM +0800, liu ping fan wrote:
 On Wed, Mar 27, 2013 at 12:07 AM, Stefan Hajnoczi stefa...@redhat.com wrote:
  There are several places where QEMU accidentally relies on the O_NONBLOCK 
  state
  of passed file descriptors.  Exposing O_NONBLOCK state makes it part of the 
  QMP
 
 If in future, we push more backend on their dedicated thread, will the
 related fd be block?

This series is not related to threading in QEMU.  The convention it
establishes is that passed fds are blocking.  If QEMU wants to use
nonblocking it must call qemu_set_block(fd).  This works whether it is
done from a traditional QEMU thread or a data plane thread.

Stefan



[Qemu-devel] [PATCH v2 0/4] monitor: do not rely on O_NONBLOCK for passed file descriptors

2013-03-27 Thread Stefan Hajnoczi
There are several places where QEMU accidentally relies on the O_NONBLOCK state
of passed file descriptors.  Exposing O_NONBLOCK state makes it part of the QMP
API whenever getfd or fdset_add_fd are used!

Whether or not QEMU will use O_NONBLOCK is an implementation detail and should
be hidden from QMP clients.

This patch series addresses this in 3 steps:

1. Fix callers of monitor_handle_fd_param(), monitor_fdset_get_fd(), and
   monitor_get_fd() that depend on O_NONBLOCK being set.  Luckily there are
   only two instances and they are fixed in Patches 1  2.

2. Rename socket_set_nonblock() to qemu_set_nonblock() just like
   qemu_set_cloexec().  This makes code cleaner when working with arbitrary
   file descriptors that may not be sockets.  See Patch 3.

3. Clear O_NONBLOCK when a chardev receives file descriptors.  From now on QEMU
   can assume that passed file descriptors are in blocking mode.  Simply use
   qemu_set_nonblock(fd) if you want to enable O_NONBLOCK.  See Patch 4.

This fixes live migration with recent libvirt.  Libvirt checks if QEMU supports
file descriptor passing and, if yes, hands QEMU a socket with O_NONBLOCK set.
The migrate fd:foo code assumes the socket is in blocking mode.  The result
is a corrupted migration stream.  For more info on this bug, see:

https://bugzilla.redhat.com/show_bug.cgi?id=923124

Note that Michal Privoznik mpriv...@redhat.com also sent a libvirt patch so
that old QEMUs work with new libvirts:

https://www.redhat.com/archives/libvir-list/2013-March/msg01486.html

My patch series fixes the QMP API and allows old libvirts to work again with
new QEMUs.

v2:
 * Rename socket_set_nonblock() in Patch 1 to avoid code churn [eblake]
 * Avoid qemu_set_block(-1) calls that clobber errno [quintela]

Stefan Hajnoczi (4):
  oslib-posix: rename socket_set_nonblock() to qemu_set_nonblock()
  net: ensure socket backend uses non-blocking fds
  qemu-socket: set passed fd non-blocking in socket_connect()
  chardev: clear O_NONBLOCK on SCM_RIGHTS file descriptors

 block/nbd.c|  2 +-
 block/sheepdog.c   |  2 +-
 include/qemu/sockets.h |  4 ++--
 migration.c|  2 +-
 nbd.c  |  8 
 net/socket.c   | 13 +
 qemu-char.c| 11 +++
 savevm.c   |  2 +-
 slirp/misc.c   |  2 +-
 slirp/tcp_subr.c   |  4 ++--
 ui/vnc.c   |  2 +-
 util/oslib-posix.c |  4 ++--
 util/oslib-win32.c |  4 ++--
 util/qemu-sockets.c|  5 +++--
 14 files changed, 37 insertions(+), 28 deletions(-)

-- 
1.8.1.4




[Qemu-devel] [PATCH v2 1/4] oslib-posix: rename socket_set_nonblock() to qemu_set_nonblock()

2013-03-27 Thread Stefan Hajnoczi
The fcntl(fd, F_SETFL, O_NONBLOCK) flag is not specific to sockets.
Rename to qemu_set_nonblock() just like qemu_set_cloexec().

Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
 block/nbd.c| 2 +-
 block/sheepdog.c   | 2 +-
 include/qemu/sockets.h | 4 ++--
 migration.c| 2 +-
 nbd.c  | 8 
 net/socket.c   | 6 +++---
 qemu-char.c| 8 
 savevm.c   | 2 +-
 slirp/misc.c   | 2 +-
 slirp/tcp_subr.c   | 4 ++--
 ui/vnc.c   | 2 +-
 util/oslib-posix.c | 4 ++--
 util/oslib-win32.c | 4 ++--
 util/qemu-sockets.c| 4 ++--
 14 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 3d711b2..eff683c 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -415,7 +415,7 @@ static int nbd_establish_connection(BlockDriverState *bs)
 
 /* Now that we're connected, set the socket to be non-blocking and
  * kick the reply mechanism.  */
-socket_set_nonblock(sock);
+qemu_set_nonblock(sock);
 qemu_aio_set_fd_handler(sock, nbd_reply_ready, NULL,
 nbd_have_request, s);
 
diff --git a/block/sheepdog.c b/block/sheepdog.c
index bb67c4c..987018e 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -471,7 +471,7 @@ static int connect_to_sdog(BDRVSheepdogState *s)
 qerror_report_err(err);
 error_free(err);
 } else {
-socket_set_nonblock(fd);
+qemu_set_nonblock(fd);
 }
 
 return fd;
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index d225f6d..c5174d7 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -37,8 +37,8 @@ int qemu_socket(int domain, int type, int protocol);
 int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
 int socket_set_cork(int fd, int v);
 int socket_set_nodelay(int fd);
-void socket_set_block(int fd);
-void socket_set_nonblock(int fd);
+void qemu_set_block(int fd);
+void qemu_set_nonblock(int fd);
 int send_all(int fd, const void *buf, int len1);
 int recv_all(int fd, void *buf, int len1, bool single_read);
 
diff --git a/migration.c b/migration.c
index 7fb2147..3b4b467 100644
--- a/migration.c
+++ b/migration.c
@@ -121,7 +121,7 @@ void process_incoming_migration(QEMUFile *f)
 int fd = qemu_get_fd(f);
 
 assert(fd != -1);
-socket_set_nonblock(fd);
+qemu_set_nonblock(fd);
 qemu_coroutine_enter(co, f);
 }
 
diff --git a/nbd.c b/nbd.c
index d1a67ee..85187ff 100644
--- a/nbd.c
+++ b/nbd.c
@@ -386,7 +386,7 @@ static int nbd_send_negotiate(NBDClient *client)
 [28 .. 151]   reserved (0)
  */
 
-socket_set_block(csock);
+qemu_set_block(csock);
 rc = -EINVAL;
 
 TRACE(Beginning negotiation.);
@@ -429,7 +429,7 @@ static int nbd_send_negotiate(NBDClient *client)
 TRACE(Negotiation succeeded.);
 rc = 0;
 fail:
-socket_set_nonblock(csock);
+qemu_set_nonblock(csock);
 return rc;
 }
 
@@ -443,7 +443,7 @@ int nbd_receive_negotiate(int csock, const char *name, 
uint32_t *flags,
 
 TRACE(Receiving negotiation.);
 
-socket_set_block(csock);
+qemu_set_block(csock);
 rc = -EINVAL;
 
 if (read_sync(csock, buf, 8) != 8) {
@@ -558,7 +558,7 @@ int nbd_receive_negotiate(int csock, const char *name, 
uint32_t *flags,
 rc = 0;
 
 fail:
-socket_set_nonblock(csock);
+qemu_set_nonblock(csock);
 return rc;
 }
 
diff --git a/net/socket.c b/net/socket.c
index 6c3752b..b5c8e65 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -308,7 +308,7 @@ static int net_socket_mcast_create(struct sockaddr_in 
*mcastaddr, struct in_addr
 }
 }
 
-socket_set_nonblock(fd);
+qemu_set_nonblock(fd);
 return fd;
 fail:
 if (fd = 0)
@@ -519,7 +519,7 @@ static int net_socket_listen_init(NetClientState *peer,
 perror(socket);
 return -1;
 }
-socket_set_nonblock(fd);
+qemu_set_nonblock(fd);
 
 /* allow fast reuse */
 val = 1;
@@ -565,7 +565,7 @@ static int net_socket_connect_init(NetClientState *peer,
 perror(socket);
 return -1;
 }
-socket_set_nonblock(fd);
+qemu_set_nonblock(fd);
 
 connected = 0;
 for(;;) {
diff --git a/qemu-char.c b/qemu-char.c
index 936150f..500a582 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2564,7 +2564,7 @@ static int tcp_chr_add_client(CharDriverState *chr, int 
fd)
 if (s-fd != -1)
return -1;
 
-socket_set_nonblock(fd);
+qemu_set_nonblock(fd);
 if (s-do_nodelay)
 socket_set_nodelay(fd);
 s-fd = fd;
@@ -2716,7 +2716,7 @@ static CharDriverState *qemu_chr_open_socket_fd(int fd, 
bool do_nodelay,
 printf(QEMU waiting for connection on: %s\n,
chr-filename);
 tcp_chr_accept(s-listen_chan, G_IO_IN, chr);
-socket_set_nonblock(s-listen_fd);
+qemu_set_nonblock(s-listen_fd);
 }
 return chr;
 }
@@ -2758,7 +2758,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts 

[Qemu-devel] [PATCH v2 3/4] qemu-socket: set passed fd non-blocking in socket_connect()

2013-03-27 Thread Stefan Hajnoczi
socket_connect() sets non-blocking on TCP or UNIX domain sockets if a
callback function is passed.  Do the same for file descriptor passing,
otherwise we could unexpectedly be using a blocking file descriptor.

Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
 util/qemu-sockets.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index b632a74..94581aa 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -910,6 +910,7 @@ int socket_connect(SocketAddress *addr, Error **errp,
 case SOCKET_ADDRESS_KIND_FD:
 fd = monitor_get_fd(cur_mon, addr-fd-str, errp);
 if (callback) {
+qemu_set_nonblock(fd);
 callback(fd, opaque);
 }
 break;
-- 
1.8.1.4




[Qemu-devel] [PATCH v2 4/4] chardev: clear O_NONBLOCK on SCM_RIGHTS file descriptors

2013-03-27 Thread Stefan Hajnoczi
When we receive a file descriptor over a UNIX domain socket the
O_NONBLOCK flag is preserved.  Clear the O_NONBLOCK flag and rely on
QEMU file descriptor users like migration, SPICE, VNC, block layer, and
others to set non-blocking only when necessary.

This change ensures we don't accidentally expose O_NONBLOCK in the QMP
API.  QMP clients should not need to get the non-blocking state
correct.

A recent real-world example was when libvirt passed a non-blocking TCP
socket for migration where we expected a blocking socket.  The source
QEMU produced a corrupted migration stream since its code did not cope
with non-blocking sockets.

Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
 qemu-char.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/qemu-char.c b/qemu-char.c
index 500a582..091f2dc 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2434,6 +2434,9 @@ static void unix_process_msgfd(CharDriverState *chr, 
struct msghdr *msg)
 if (fd  0)
 continue;
 
+/* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
+qemu_set_block(fd);
+
 #ifndef MSG_CMSG_CLOEXEC
 qemu_set_cloexec(fd);
 #endif
-- 
1.8.1.4




[Qemu-devel] [PATCH v2 2/4] net: ensure socket backend uses non-blocking fds

2013-03-27 Thread Stefan Hajnoczi
There are several code paths in net_init_socket() depending on how the
socket is created: file descriptor passing, UDP multicast, TCP, or UDP.
Some of these support both listen and connect.

Not all code paths set the socket to non-blocking.  This patch addresses
the file descriptor passing and UDP cases which were missing
socket_set_nonblock(fd) calls.

I considered moving socket_set_nonblock(fd) to a central location but it
turns out the code paths are different enough to require non-blocking at
different places.

Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
 net/socket.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/socket.c b/net/socket.c
index b5c8e65..87af1d3 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -674,6 +674,7 @@ static int net_socket_udp_init(NetClientState *peer,
 closesocket(fd);
 return -1;
 }
+qemu_set_nonblock(fd);
 
 s = net_socket_fd_init(peer, model, name, fd, 0);
 if (!s) {
@@ -712,7 +713,11 @@ int net_init_socket(const NetClientOptions *opts, const 
char *name,
 int fd;
 
 fd = monitor_handle_fd_param(cur_mon, sock-fd);
-if (fd == -1 || !net_socket_fd_init(peer, socket, name, fd, 1)) {
+if (fd == -1) {
+return -1;
+}
+qemu_set_nonblock(fd);
+if (!net_socket_fd_init(peer, socket, name, fd, 1)) {
 return -1;
 }
 return 0;
-- 
1.8.1.4




Re: [Qemu-devel] [RFC 1/4] block: fix I/O throttling accounting blind spot

2013-03-27 Thread Stefan Hajnoczi
On Wed, Mar 27, 2013 at 9:50 AM, Zhi Yong Wu zwu.ker...@gmail.com wrote:
 On Thu, Mar 21, 2013 at 10:49 PM, Stefan Hajnoczi stefa...@redhat.com wrote:
 I/O throttling relies on bdrv_acct_done() which is called when a request
 completes.  This leaves a blind spot since we only charge for completed
 requests, not submitted requests.

 For example, if there is 1 operation remaining in this time slice the
 guest could submit 3 operations and they will all be submitted
 successfully since they don't actually get accounted for until they
 complete.

 Originally we probably thought this is okay since the requests will be
 accounted when the time slice is extended.  In practice it causes
 fluctuations since the guest can exceed its I/O limit and it will be
 punished for this later on.

 Account for I/O upon submission so that I/O limits are enforced
 properly.

 Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
 ---
  block.c   | 19 ---
  include/block/block_int.h |  2 +-
  2 files changed, 9 insertions(+), 12 deletions(-)

 diff --git a/block.c b/block.c
 index 0a062c9..31fb0b0 100644
 --- a/block.c
 +++ b/block.c
 @@ -141,7 +141,6 @@ void bdrv_io_limits_disable(BlockDriverState *bs)
  bs-slice_start = 0;
  bs-slice_end   = 0;
  bs-slice_time  = 0;
 -memset(bs-io_base, 0, sizeof(bs-io_base));
 If we try some concussive operations, enable I/O throttling at first,
 then disable it, and then enable it, how about? I guess that
 bs-slice_submitted will maybe be not correct.

The memset() was moved...

 @@ -3772,11 +3773,7 @@ static bool bdrv_exceed_io_limits(BlockDriverState 
 *bs, int nb_sectors,
  bs-slice_start = now;
  bs-slice_end   = now + bs-slice_time;

 -bs-io_base.bytes[is_write]  = bs-nr_bytes[is_write];
 -bs-io_base.bytes[!is_write] = bs-nr_bytes[!is_write];
 -
 -bs-io_base.ios[is_write]= bs-nr_ops[is_write];
 -bs-io_base.ios[!is_write]   = bs-nr_ops[!is_write];
 +memset(bs-slice_submitted, 0, sizeof(bs-slice_submitted));
  }

  elapsed_time  = now - bs-slice_start;

...here.

Since bs-slice_start = 0 when I/O throttling is disabled we will
start a new slice next time bdrv_exceed_io_limits() is called.

Therefore bs-slice_submitted is consistent across disable/enable.

Stefan



[Qemu-devel] [PATCH v3 1/6] virtio-balloon: add the virtio-balloon device.

2013-03-27 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

Create virtio-balloon which extends virtio-device, so it can be connected on
virtio-bus.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
---
 hw/virtio-balloon.c | 95 +
 hw/virtio-balloon.h |  4 +++
 2 files changed, 93 insertions(+), 6 deletions(-)

diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index 54a4372..7519971 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -29,6 +29,11 @@
 #include sys/mman.h
 #endif
 
+#include hw/virtio-bus.h
+
+/*
+ * Will be modified later in the serie.
+ */
 static VirtIOBalloon *to_virtio_balloon(VirtIODevice *vdev)
 {
 return (VirtIOBalloon *)vdev;
@@ -334,14 +339,27 @@ static int virtio_balloon_load(QEMUFile *f, void *opaque, 
int version_id)
 return 0;
 }
 
-VirtIODevice *virtio_balloon_init(DeviceState *dev)
+static VirtIODevice *virtio_balloon_common_init(DeviceState *dev,
+VirtIOBalloon **ps)
 {
-VirtIOBalloon *s;
+VirtIOBalloon *s = *ps;
 int ret;
 
-s = (VirtIOBalloon *)virtio_common_init(virtio-balloon,
-VIRTIO_ID_BALLOON,
-8, sizeof(VirtIOBalloon));
+/*
+ * We have two cases here: the old virtio-balloon-x device, and the
+ * refactored virtio-balloon.
+ * This will disappear later in the serie.
+ */
+int old_device = (s == NULL);
+if (s == NULL) {
+/* old virtio-balloon-pci or virtio-balloon-s390, no memory allocated 
*/
+s = (VirtIOBalloon *)virtio_common_init(virtio-balloon,
+VIRTIO_ID_BALLOON,
+8, sizeof(VirtIOBalloon));
+} else {
+/* new API virtio-balloon. (memory allocated by qdev) */
+virtio_init(VIRTIO_DEVICE(s), virtio-balloon, VIRTIO_ID_BALLOON, 8);
+}
 
 s-vdev.get_config = virtio_balloon_get_config;
 s-vdev.set_config = virtio_balloon_set_config;
@@ -349,10 +367,14 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev)
 
 ret = qemu_add_balloon_handler(virtio_balloon_to_target,
virtio_balloon_stat, s);
-if (ret  0) {
+if ((ret  0)  (old_device)) {
 virtio_cleanup(s-vdev);
 return NULL;
 }
+if (ret  0) {
+virtio_common_cleanup(VIRTIO_DEVICE(s));
+return NULL;
+}
 
 s-ivq = virtio_add_queue(s-vdev, 128, virtio_balloon_handle_output);
 s-dvq = virtio_add_queue(s-vdev, 128, virtio_balloon_handle_output);
@@ -373,6 +395,15 @@ VirtIODevice *virtio_balloon_init(DeviceState *dev)
 return s-vdev;
 }
 
+/*
+ * This two functions will be removed later in the serie.
+ */
+VirtIODevice *virtio_balloon_init(DeviceState *dev)
+{
+VirtIOBalloon *s = NULL;
+return virtio_balloon_common_init(dev, s);
+}
+
 void virtio_balloon_exit(VirtIODevice *vdev)
 {
 VirtIOBalloon *s = DO_UPCAST(VirtIOBalloon, vdev, vdev);
@@ -382,3 +413,55 @@ void virtio_balloon_exit(VirtIODevice *vdev)
 unregister_savevm(s-qdev, virtio-balloon, s);
 virtio_cleanup(vdev);
 }
+
+static int virtio_balloon_device_init(VirtIODevice *vdev)
+{
+DeviceState *qdev = DEVICE(vdev);
+VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
+if (virtio_balloon_common_init(qdev, s) == NULL) {
+return -1;
+}
+return 0;
+}
+
+static int virtio_balloon_device_exit(DeviceState *qdev)
+{
+VirtIOBalloon *s = VIRTIO_BALLOON(qdev);
+VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
+
+balloon_stats_destroy_timer(s);
+qemu_remove_balloon_handler(s);
+unregister_savevm(qdev, virtio-balloon, s);
+virtio_common_cleanup(vdev);
+return 0;
+}
+
+static Property virtio_balloon_properties[] = {
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void virtio_balloon_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+VirtioDeviceClass *vdc = VIRTIO_DEVICE_CLASS(klass);
+dc-exit = virtio_balloon_device_exit;
+dc-props = virtio_balloon_properties;
+vdc-init = virtio_balloon_device_init;
+vdc-get_config = virtio_balloon_get_config;
+vdc-set_config = virtio_balloon_set_config;
+vdc-get_features = virtio_balloon_get_features;
+}
+
+static const TypeInfo virtio_balloon_info = {
+.name = TYPE_VIRTIO_BALLOON,
+.parent = TYPE_VIRTIO_DEVICE,
+.instance_size = sizeof(VirtIOBalloon),
+.class_init = virtio_balloon_class_init,
+};
+
+static void virtio_register_types(void)
+{
+type_register_static(virtio_balloon_info);
+}
+
+type_init(virtio_register_types)
diff --git a/hw/virtio-balloon.h b/hw/virtio-balloon.h
index b007042..139eac3 100644
--- a/hw/virtio-balloon.h
+++ b/hw/virtio-balloon.h
@@ -18,6 +18,10 @@
 #include hw/virtio.h
 #include hw/pci/pci.h
 
+#define TYPE_VIRTIO_BALLOON virtio-balloon
+#define VIRTIO_BALLOON(obj) \
+

[Qemu-devel] [PATCH v3 0/6] virtio-balloon refactoring.

2013-03-27 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

This is the next part of virtio-refactoring.

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

You can checkout my branch here:

git://project.greensocs.com/qemu-virtio.git virtio-balloon-v3

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

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

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

Thanks,

Fred

KONRAD Frederic (6):
  virtio-balloon: add the virtio-balloon device.
  virtio-balloon-pci: switch to the new API.
  virtio-balloon-ccw: switch to the new API.
  virtio-balloon: cleanup: init and exit function.
  virtio-balloon: cleanup: QOM casts.
  virtio-balloon: cleanup: remove qdev field.

 hw/s390x/virtio-ccw.c |  25 +++-
 hw/s390x/virtio-ccw.h |  11 +
 hw/virtio-balloon.c   | 110 +++--
 hw/virtio-balloon.h   |   7 +++-
 hw/virtio-pci.c   | 111 +-
 hw/virtio-pci.h   |  14 +++
 6 files changed, 170 insertions(+), 108 deletions(-)

-- 
1.7.11.7




[Qemu-devel] [PATCH v3 2/6] virtio-balloon-pci: switch to the new API.

2013-03-27 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

Here the virtio-balloon-pci is modified for the new API. The device
virtio-balloon-pci extends virtio-pci. It creates and connects a
virtio-balloon during the init. The properties are not changed.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
---
 hw/virtio-pci.c | 111 
 hw/virtio-pci.h |  14 +++
 2 files changed, 69 insertions(+), 56 deletions(-)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 736a9bf..fb20722 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -22,6 +22,7 @@
 #include hw/virtio-net.h
 #include hw/virtio-serial.h
 #include hw/virtio-scsi.h
+#include hw/virtio-balloon.h
 #include hw/pci/pci.h
 #include qemu/error-report.h
 #include hw/pci/msi.h
@@ -1000,33 +1001,6 @@ static void virtio_net_exit_pci(PCIDevice *pci_dev)
 virtio_exit_pci(pci_dev);
 }
 
-static int virtio_balloon_init_pci(PCIDevice *pci_dev)
-{
-VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
-VirtIODevice *vdev;
-
-if (proxy-class_code != PCI_CLASS_OTHERS 
-proxy-class_code != PCI_CLASS_MEMORY_RAM) { /* qemu  1.1 */
-proxy-class_code = PCI_CLASS_OTHERS;
-}
-
-vdev = virtio_balloon_init(pci_dev-qdev);
-if (!vdev) {
-return -1;
-}
-virtio_init_pci(proxy, vdev);
-return 0;
-}
-
-static void virtio_balloon_exit_pci(PCIDevice *pci_dev)
-{
-VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
-
-virtio_pci_stop_ioeventfd(proxy);
-virtio_balloon_exit(proxy-vdev);
-virtio_exit_pci(pci_dev);
-}
-
 static int virtio_rng_init_pci(PCIDevice *pci_dev)
 {
 VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev);
@@ -1127,34 +1101,6 @@ static const TypeInfo virtio_serial_info = {
 .class_init= virtio_serial_class_init,
 };
 
-static Property virtio_balloon_properties[] = {
-DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
-DEFINE_PROP_HEX32(class, VirtIOPCIProxy, class_code, 0),
-DEFINE_PROP_END_OF_LIST(),
-};
-
-static void virtio_balloon_class_init(ObjectClass *klass, void *data)
-{
-DeviceClass *dc = DEVICE_CLASS(klass);
-PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-k-init = virtio_balloon_init_pci;
-k-exit = virtio_balloon_exit_pci;
-k-vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
-k-device_id = PCI_DEVICE_ID_VIRTIO_BALLOON;
-k-revision = VIRTIO_PCI_ABI_VERSION;
-k-class_id = PCI_CLASS_OTHERS;
-dc-reset = virtio_pci_reset;
-dc-props = virtio_balloon_properties;
-}
-
-static const TypeInfo virtio_balloon_info = {
-.name  = virtio-balloon-pci,
-.parent= TYPE_PCI_DEVICE,
-.instance_size = sizeof(VirtIOPCIProxy),
-.class_init= virtio_balloon_class_init,
-};
-
 static void virtio_rng_initfn(Object *obj)
 {
 PCIDevice *pci_dev = PCI_DEVICE(obj);
@@ -1461,6 +1407,59 @@ static const TypeInfo virtio_scsi_pci_info = {
 .class_init= virtio_scsi_pci_class_init,
 };
 
+/* virtio-balloon-pci */
+
+static Property virtio_balloon_pci_properties[] = {
+DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
+DEFINE_PROP_HEX32(class, VirtIOPCIProxy, class_code, 0),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static int virtio_balloon_pci_init(VirtIOPCIProxy *vpci_dev)
+{
+VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(vpci_dev);
+DeviceState *vdev = DEVICE(dev-vdev);
+
+if (vpci_dev-class_code != PCI_CLASS_OTHERS 
+vpci_dev-class_code != PCI_CLASS_MEMORY_RAM) { /* qemu  1.1 */
+vpci_dev-class_code = PCI_CLASS_OTHERS;
+}
+
+qdev_set_parent_bus(vdev, BUS(vpci_dev-bus));
+if (qdev_init(vdev)  0) {
+return -1;
+}
+return 0;
+}
+
+static void virtio_balloon_pci_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
+PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
+k-init = virtio_balloon_pci_init;
+dc-props = virtio_balloon_pci_properties;
+pcidev_k-vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
+pcidev_k-device_id = PCI_DEVICE_ID_VIRTIO_BALLOON;
+pcidev_k-revision = VIRTIO_PCI_ABI_VERSION;
+pcidev_k-class_id = PCI_CLASS_OTHERS;
+}
+
+static void virtio_balloon_pci_instance_init(Object *obj)
+{
+VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(obj);
+object_initialize(OBJECT(dev-vdev), TYPE_VIRTIO_BALLOON);
+object_property_add_child(obj, virtio-backend, OBJECT(dev-vdev), NULL);
+}
+
+static const TypeInfo virtio_balloon_pci_info = {
+.name  = TYPE_VIRTIO_BALLOON_PCI,
+.parent= TYPE_VIRTIO_PCI,
+.instance_size = sizeof(VirtIOBalloonPCI),
+.instance_init = virtio_balloon_pci_instance_init,
+.class_init= virtio_balloon_pci_class_init,
+};
+
 /* virtio-pci-bus */
 
 void virtio_pci_bus_new(VirtioBusState *bus, VirtIOPCIProxy *dev)
@@ -1501,7 +1500,6 @@ static void 

[Qemu-devel] [PATCH v3 5/6] virtio-balloon: cleanup: QOM casts.

2013-03-27 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

As the virtio-balloon-pci is switched to the new API, we can use QOM
casts.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
---
 hw/virtio-balloon.c | 43 ---
 hw/virtio-balloon.h |  2 +-
 2 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index 87278f5..38d2ee3 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -31,14 +31,6 @@
 
 #include hw/virtio-bus.h
 
-/*
- * Will be modified later in the serie.
- */
-static VirtIOBalloon *to_virtio_balloon(VirtIODevice *vdev)
-{
-return (VirtIOBalloon *)vdev;
-}
-
 static void balloon_page(void *addr, int deflate)
 {
 #if defined(__linux__)
@@ -74,7 +66,8 @@ static inline void reset_stats(VirtIOBalloon *dev)
 
 static bool balloon_stats_supported(const VirtIOBalloon *s)
 {
-return s-vdev.guest_features  (1  VIRTIO_BALLOON_F_STATS_VQ);
+VirtIODevice *vdev = VIRTIO_DEVICE(s);
+return vdev-guest_features  (1  VIRTIO_BALLOON_F_STATS_VQ);
 }
 
 static bool balloon_stats_enabled(const VirtIOBalloon *s)
@@ -100,6 +93,7 @@ static void balloon_stats_change_timer(VirtIOBalloon *s, int 
secs)
 static void balloon_stats_poll_cb(void *opaque)
 {
 VirtIOBalloon *s = opaque;
+VirtIODevice *vdev = VIRTIO_DEVICE(s);
 
 if (!balloon_stats_supported(s)) {
 /* re-schedule */
@@ -108,7 +102,7 @@ static void balloon_stats_poll_cb(void *opaque)
 }
 
 virtqueue_push(s-svq, s-stats_vq_elem, s-stats_vq_offset);
-virtio_notify(s-vdev, s-svq);
+virtio_notify(vdev, s-svq);
 }
 
 static void balloon_stats_get_all(Object *obj, struct Visitor *v,
@@ -186,7 +180,7 @@ static void balloon_stats_set_poll_interval(Object *obj, 
struct Visitor *v,
 
 static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
 {
-VirtIOBalloon *s = to_virtio_balloon(vdev);
+VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
 VirtQueueElement elem;
 MemoryRegionSection section;
 
@@ -220,7 +214,7 @@ static void virtio_balloon_handle_output(VirtIODevice 
*vdev, VirtQueue *vq)
 
 static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
 {
-VirtIOBalloon *s = DO_UPCAST(VirtIOBalloon, vdev, vdev);
+VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
 VirtQueueElement *elem = s-stats_vq_elem;
 VirtIOBalloonStat stat;
 size_t offset = 0;
@@ -262,7 +256,7 @@ out:
 
 static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data)
 {
-VirtIOBalloon *dev = to_virtio_balloon(vdev);
+VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
 struct virtio_balloon_config config;
 
 config.num_pages = cpu_to_le32(dev-num_pages);
@@ -274,7 +268,7 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, 
uint8_t *config_data)
 static void virtio_balloon_set_config(VirtIODevice *vdev,
   const uint8_t *config_data)
 {
-VirtIOBalloon *dev = to_virtio_balloon(vdev);
+VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
 struct virtio_balloon_config config;
 uint32_t oldactual = dev-actual;
 memcpy(config, config_data, 8);
@@ -300,22 +294,24 @@ static void virtio_balloon_stat(void *opaque, BalloonInfo 
*info)
 
 static void virtio_balloon_to_target(void *opaque, ram_addr_t target)
 {
-VirtIOBalloon *dev = opaque;
+VirtIOBalloon *dev = VIRTIO_BALLOON(opaque);
+VirtIODevice *vdev = VIRTIO_DEVICE(dev);
 
 if (target  ram_size) {
 target = ram_size;
 }
 if (target) {
 dev-num_pages = (ram_size - target)  VIRTIO_BALLOON_PFN_SHIFT;
-virtio_notify_config(dev-vdev);
+virtio_notify_config(vdev);
 }
 }
 
 static void virtio_balloon_save(QEMUFile *f, void *opaque)
 {
-VirtIOBalloon *s = opaque;
+VirtIOBalloon *s = VIRTIO_BALLOON(opaque);
+VirtIODevice *vdev = VIRTIO_DEVICE(s);
 
-virtio_save(s-vdev, f);
+virtio_save(vdev, f);
 
 qemu_put_be32(f, s-num_pages);
 qemu_put_be32(f, s-actual);
@@ -323,13 +319,14 @@ static void virtio_balloon_save(QEMUFile *f, void *opaque)
 
 static int virtio_balloon_load(QEMUFile *f, void *opaque, int version_id)
 {
-VirtIOBalloon *s = opaque;
+VirtIOBalloon *s = VIRTIO_BALLOON(opaque);
+VirtIODevice *vdev = VIRTIO_DEVICE(s);
 int ret;
 
 if (version_id != 1)
 return -EINVAL;
 
-ret = virtio_load(s-vdev, f);
+ret = virtio_load(vdev, f);
 if (ret) {
 return ret;
 }
@@ -347,9 +344,9 @@ static int virtio_balloon_device_init(VirtIODevice *vdev)
 
 virtio_init(vdev, virtio-balloon, VIRTIO_ID_BALLOON, 8);
 
-s-vdev.get_config = virtio_balloon_get_config;
-s-vdev.set_config = virtio_balloon_set_config;
-s-vdev.get_features = virtio_balloon_get_features;
+vdev-get_config = virtio_balloon_get_config;
+vdev-set_config = virtio_balloon_set_config;
+vdev-get_features = virtio_balloon_get_features;
 
 ret = 

[Qemu-devel] [PATCH v3 3/6] virtio-balloon-ccw: switch to the new API.

2013-03-27 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

Here the virtio-balloon-ccw is modified for the new API. The device
virtio-balloon-ccw extends virtio-ccw-device as before. It creates and
connects a virtio-balloon during the init. The properties are not modified.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
---
 hw/s390x/virtio-ccw.c | 25 ++---
 hw/s390x/virtio-ccw.h | 11 +++
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 7e79c57..5dce791 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -608,22 +608,24 @@ static int virtio_ccw_serial_exit(VirtioCcwDevice *dev)
 return virtio_ccw_exit(dev);
 }
 
-static int virtio_ccw_balloon_init(VirtioCcwDevice *dev)
+static int virtio_ccw_balloon_init(VirtioCcwDevice *ccw_dev)
 {
-VirtIODevice *vdev;
+VirtIOBalloonCcw *dev = VIRTIO_BALLOON_CCW(ccw_dev);
+DeviceState *vdev = DEVICE(dev-vdev);
 
-vdev = virtio_balloon_init((DeviceState *)dev);
-if (!vdev) {
+qdev_set_parent_bus(vdev, BUS(ccw_dev-bus));
+if (qdev_init(vdev)  0) {
 return -1;
 }
 
-return virtio_ccw_device_init(dev, vdev);
+return virtio_ccw_device_init(ccw_dev, VIRTIO_DEVICE(vdev));
 }
 
-static int virtio_ccw_balloon_exit(VirtioCcwDevice *dev)
+static void virtio_ccw_balloon_instance_init(Object *obj)
 {
-virtio_balloon_exit(dev-vdev);
-return virtio_ccw_exit(dev);
+VirtIOBalloonCcw *dev = VIRTIO_BALLOON_CCW(obj);
+object_initialize(OBJECT(dev-vdev), TYPE_VIRTIO_BALLOON);
+object_property_add_child(obj, virtio-backend, OBJECT(dev-vdev), NULL);
 }
 
 static int virtio_ccw_scsi_init(VirtioCcwDevice *ccw_dev)
@@ -820,15 +822,16 @@ static void virtio_ccw_balloon_class_init(ObjectClass 
*klass, void *data)
 VirtIOCCWDeviceClass *k = VIRTIO_CCW_DEVICE_CLASS(klass);
 
 k-init = virtio_ccw_balloon_init;
-k-exit = virtio_ccw_balloon_exit;
+k-exit = virtio_ccw_exit;
 dc-reset = virtio_ccw_reset;
 dc-props = virtio_ccw_balloon_properties;
 }
 
 static const TypeInfo virtio_ccw_balloon = {
-.name  = virtio-balloon-ccw,
+.name  = TYPE_VIRTIO_BALLOON_CCW,
 .parent= TYPE_VIRTIO_CCW_DEVICE,
-.instance_size = sizeof(VirtioCcwDevice),
+.instance_size = sizeof(VirtIOBalloonCcw),
+.instance_init = virtio_ccw_balloon_instance_init,
 .class_init= virtio_ccw_balloon_class_init,
 };
 
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index d9f7399..d580510 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -16,6 +16,7 @@
 #include hw/virtio-net.h
 #include hw/virtio-serial.h
 #include hw/virtio-scsi.h
+#include hw/virtio-balloon.h
 #include hw/virtio-rng.h
 #include hw/virtio-bus.h
 
@@ -115,6 +116,16 @@ typedef struct VirtIOBlkCcw {
 VirtIOBlkConf blk;
 } VirtIOBlkCcw;
 
+/* virtio-balloon-ccw */
+
+#define TYPE_VIRTIO_BALLOON_CCW virtio-balloon-ccw
+#define VIRTIO_BALLOON_CCW(obj) \
+OBJECT_CHECK(VirtIOBalloonCcw, (obj), TYPE_VIRTIO_BALLOON_CCW)
+
+typedef struct VirtIOBalloonCcw {
+VirtioCcwDevice parent_obj;
+VirtIOBalloon vdev;
+} VirtIOBalloonCcw;
 
 VirtualCssBus *virtual_css_bus_init(void);
 void virtio_ccw_device_update_status(SubchDev *sch);
-- 
1.7.11.7




[Qemu-devel] [PATCH v3 6/6] virtio-balloon: cleanup: remove qdev field.

2013-03-27 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

The qdev field is no longer needed, just drop it.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
---
 hw/virtio-balloon.c | 1 -
 hw/virtio-balloon.h | 1 -
 2 files changed, 2 deletions(-)

diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index 38d2ee3..b382bd4 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -360,7 +360,6 @@ static int virtio_balloon_device_init(VirtIODevice *vdev)
 s-dvq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
 s-svq = virtio_add_queue(vdev, 128, virtio_balloon_receive_stats);
 
-s-qdev = qdev;
 register_savevm(qdev, virtio-balloon, -1, 1,
 virtio_balloon_save, virtio_balloon_load, s);
 
diff --git a/hw/virtio-balloon.h b/hw/virtio-balloon.h
index 5f8fe02..d898315 100644
--- a/hw/virtio-balloon.h
+++ b/hw/virtio-balloon.h
@@ -67,7 +67,6 @@ typedef struct VirtIOBalloon {
 QEMUTimer *stats_timer;
 int64_t stats_last_update;
 int64_t stats_poll_interval;
-DeviceState *qdev;
 } VirtIOBalloon;
 
 #endif
-- 
1.7.11.7




[Qemu-devel] [PATCH v3 4/6] virtio-balloon: cleanup: init and exit function.

2013-03-27 Thread fred . konrad
From: KONRAD Frederic fred.kon...@greensocs.com

This remove old init and exit function as they are no longer needed.

Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
---
 hw/virtio-balloon.c | 73 ++---
 1 file changed, 13 insertions(+), 60 deletions(-)

diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index 7519971..87278f5 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -339,27 +339,13 @@ static int virtio_balloon_load(QEMUFile *f, void *opaque, 
int version_id)
 return 0;
 }
 
-static VirtIODevice *virtio_balloon_common_init(DeviceState *dev,
-VirtIOBalloon **ps)
+static int virtio_balloon_device_init(VirtIODevice *vdev)
 {
-VirtIOBalloon *s = *ps;
+DeviceState *qdev = DEVICE(vdev);
+VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
 int ret;
 
-/*
- * We have two cases here: the old virtio-balloon-x device, and the
- * refactored virtio-balloon.
- * This will disappear later in the serie.
- */
-int old_device = (s == NULL);
-if (s == NULL) {
-/* old virtio-balloon-pci or virtio-balloon-s390, no memory allocated 
*/
-s = (VirtIOBalloon *)virtio_common_init(virtio-balloon,
-VIRTIO_ID_BALLOON,
-8, sizeof(VirtIOBalloon));
-} else {
-/* new API virtio-balloon. (memory allocated by qdev) */
-virtio_init(VIRTIO_DEVICE(s), virtio-balloon, VIRTIO_ID_BALLOON, 8);
-}
+virtio_init(vdev, virtio-balloon, VIRTIO_ID_BALLOON, 8);
 
 s-vdev.get_config = virtio_balloon_get_config;
 s-vdev.set_config = virtio_balloon_set_config;
@@ -367,60 +353,27 @@ static VirtIODevice 
*virtio_balloon_common_init(DeviceState *dev,
 
 ret = qemu_add_balloon_handler(virtio_balloon_to_target,
virtio_balloon_stat, s);
-if ((ret  0)  (old_device)) {
-virtio_cleanup(s-vdev);
-return NULL;
-}
+
 if (ret  0) {
 virtio_common_cleanup(VIRTIO_DEVICE(s));
-return NULL;
+return -1;
 }
 
-s-ivq = virtio_add_queue(s-vdev, 128, virtio_balloon_handle_output);
-s-dvq = virtio_add_queue(s-vdev, 128, virtio_balloon_handle_output);
-s-svq = virtio_add_queue(s-vdev, 128, virtio_balloon_receive_stats);
+s-ivq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
+s-dvq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
+s-svq = virtio_add_queue(vdev, 128, virtio_balloon_receive_stats);
 
-s-qdev = dev;
-register_savevm(dev, virtio-balloon, -1, 1,
+s-qdev = qdev;
+register_savevm(qdev, virtio-balloon, -1, 1,
 virtio_balloon_save, virtio_balloon_load, s);
 
-object_property_add(OBJECT(dev), guest-stats, guest statistics,
+object_property_add(OBJECT(qdev), guest-stats, guest statistics,
 balloon_stats_get_all, NULL, NULL, s, NULL);
 
-object_property_add(OBJECT(dev), guest-stats-polling-interval, int,
+object_property_add(OBJECT(qdev), guest-stats-polling-interval, int,
 balloon_stats_get_poll_interval,
 balloon_stats_set_poll_interval,
 NULL, s, NULL);
-
-return s-vdev;
-}
-
-/*
- * This two functions will be removed later in the serie.
- */
-VirtIODevice *virtio_balloon_init(DeviceState *dev)
-{
-VirtIOBalloon *s = NULL;
-return virtio_balloon_common_init(dev, s);
-}
-
-void virtio_balloon_exit(VirtIODevice *vdev)
-{
-VirtIOBalloon *s = DO_UPCAST(VirtIOBalloon, vdev, vdev);
-
-balloon_stats_destroy_timer(s);
-qemu_remove_balloon_handler(s);
-unregister_savevm(s-qdev, virtio-balloon, s);
-virtio_cleanup(vdev);
-}
-
-static int virtio_balloon_device_init(VirtIODevice *vdev)
-{
-DeviceState *qdev = DEVICE(vdev);
-VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
-if (virtio_balloon_common_init(qdev, s) == NULL) {
-return -1;
-}
 return 0;
 }
 
-- 
1.7.11.7




Re: [Qemu-devel] [PATCH 01/12] target-i386: consolidate error propagation in x86_cpu_realizefn()

2013-03-27 Thread Paolo Bonzini
Il 21/03/2013 15:28, Igor Mammedov ha scritto:
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  target-i386/cpu.c |   17 ++---
  1 files changed, 10 insertions(+), 7 deletions(-)
 
 diff --git a/target-i386/cpu.c b/target-i386/cpu.c
 index a0640db..e905bcf 100644
 --- a/target-i386/cpu.c
 +++ b/target-i386/cpu.c
 @@ -2100,9 +2100,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
 **errp)
  X86CPU *cpu = X86_CPU(dev);
  X86CPUClass *xcc = X86_CPU_GET_CLASS(dev);
  CPUX86State *env = cpu-env;
 -#ifndef CONFIG_USER_ONLY
  Error *local_err = NULL;
 -#endif
  
  if (env-cpuid_7_0_ebx_features  env-cpuid_level  7) {
  env-cpuid_level = 7;
 @@ -2135,8 +2133,9 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
 **errp)
  #endif
  if (check_cpuid  kvm_check_features_against_host(cpu)
   enforce_cpuid) {
 -error_setg(errp, Host's CPU doesn't support requested 
 features);
 -return;
 +error_setg(local_err,
 +   Host's CPU doesn't support requested features);
 +goto out;
  }
  }
  
 @@ -2146,8 +2145,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
 **errp)
  if (cpu-env.cpuid_features  CPUID_APIC || smp_cpus  1) {
  x86_cpu_apic_init(cpu, local_err);
  if (local_err != NULL) {
 -error_propagate(errp, local_err);
 -return;
 +goto out;
  }
  }
  #endif
 @@ -2156,7 +2154,12 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
 **errp)
  qemu_init_vcpu(cpu-env);
  cpu_reset(CPU(cpu));
  
 -xcc-parent_realize(dev, errp);
 +xcc-parent_realize(dev, local_err);
 +out:
 +if (local_err != NULL) {
 +error_propagate(errp, local_err);
 +return;
 +}
  }
  
  /* Enables contiguous-apic-ID mode, for compatibility */
 

Reviewed-by: Paolo Bonzini pbonz...@redhat.com



Re: [Qemu-devel] [PATCH 06/12] target-i386: replace FROM_SYSBUS() with QOM type cast

2013-03-27 Thread Paolo Bonzini
Il 21/03/2013 15:28, Igor Mammedov ha scritto:
 ... and define type name and type cast macro for kvmvapic according
 to accepted convention.
 
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  hw/i386/kvmvapic.c |7 +--
  1 files changed, 5 insertions(+), 2 deletions(-)
 
 diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
 index c151c95..21551a5 100644
 --- a/hw/i386/kvmvapic.c
 +++ b/hw/i386/kvmvapic.c
 @@ -62,6 +62,9 @@ typedef struct VAPICROMState {
  bool rom_mapped_writable;
  } VAPICROMState;
  
 +#define TYPE_VAPIC_DEVICE kvmvapic
 +#define VAPIC_DEVICE(obj) OBJECT_CHECK(VAPICROMState, (obj), 
 TYPE_VAPIC_DEVICE)
 +
  #define TPR_INSTR_ABS_MODRM 0x1
  #define TPR_INSTR_MATCH_MODRM_REG   0x2
  
 @@ -692,7 +695,7 @@ static const MemoryRegionOps vapic_ops = {
  
  static int vapic_init(SysBusDevice *dev)
  {
 -VAPICROMState *s = FROM_SYSBUS(VAPICROMState, dev);
 +VAPICROMState *s = VAPIC_DEVICE(dev);
  
  memory_region_init_io(s-io, vapic_ops, s, kvmvapic, 2);
  sysbus_add_io(dev, VAPIC_IO_PORT, s-io);
 @@ -808,7 +811,7 @@ static void vapic_class_init(ObjectClass *klass, void 
 *data)
  }
  
  static const TypeInfo vapic_type = {
 -.name  = kvmvapic,
 +.name  = TYPE_VAPIC_DEVICE,
  .parent= TYPE_SYS_BUS_DEVICE,
  .instance_size = sizeof(VAPICROMState),
  .class_init= vapic_class_init,
 

Reviewed-by: Paolo Bonzini pbonz...@redhat.com



Re: [Qemu-devel] [PATCH v2 21/21] qcow2: Gather clusters in a looping loop

2013-03-27 Thread Stefan Hajnoczi
On Tue, Mar 26, 2013 at 05:50:13PM +0100, Kevin Wolf wrote:
 @@ -793,6 +793,13 @@ static int handle_dependencies(BlockDriverState *bs, 
 uint64_t guest_offset,
  bytes = 0;
  }
  
 +/* Stop if already an l2meta exists. We would have to consider
 + * locks held by it (l2_writeback_lock) and set m-next etc. */

l2_writeback_lock does not exist (yet).

 +if (bytes == 0  *m) {
 +*cur_bytes = 0;
 +return 0;
 +}



Re: [Qemu-devel] [PATCH v2 00/21] qcow2: Rework cluster allocation even more

2013-03-27 Thread Stefan Hajnoczi
On Tue, Mar 26, 2013 at 05:49:52PM +0100, Kevin Wolf wrote:
 This is the start of the latest Delayed COW series. As there seem to be 
 serious
 problems with the actual Delayed COW (which in fact exist today, but would
 become a bit easier to hit) and I don't have the time to complete this at the
 moment, here is another series with preparatory patches, for which there's no
 reason to not get them applied now.
 
 The result is a somewhat cleaner cluster allocation code in qcow2, and in
 theory some cases should show improved performance - in practice I think these
 cases are unlikely to occur, but as the optimisation falls out naturally, 
 let's
 just take it.
 
 Kevin Wolf (21):
   qemu-iotests: More concurrent allocation scenarios
   qcow2: Fix total clusters number in bdrv_check
   qcow2: Remove bogus unlock of s-lock
   qcow2: Handle dependencies earlier
   qcow2: Improve check for overlapping allocations
   qcow2: Change handle_dependency to byte granularity
   qcow2: Decouple cluster allocation from cluster reuse code
   qcow2: Factor out handle_alloc()
   qcow2: handle_alloc(): Get rid of nb_clusters parameter
   qcow2: handle_alloc(): Get rid of keep_clusters parameter
   qcow2: Finalise interface of handle_alloc()
   qcow2: Clean up handle_alloc()
   qcow2: Factor out handle_copied()
   qcow2: handle_copied(): Get rid of nb_clusters parameter
   qcow2: handle_copied(): Get rid of keep_clusters parameter
   qcow2: handle_copied(): Implement non-zero host_offset
   qcow2: Prepare handle_alloc/copied() for byte granularity
   qcow2: Use byte granularity in qcow2_alloc_cluster_offset()
   qcow2: Allow requests with multiple l2metas
   qcow2: Move cluster gathering to a non-looping loop
   qcow2: Gather clusters in a looping loop
 
  block/qcow2-cluster.c  | 507 
 -
  block/qcow2-refcount.c |   4 +-
  block/qcow2.c  |  16 +-
  block/qcow2.h  |  29 +++
  tests/qemu-iotests/038.out |  10 +-
  tests/qemu-iotests/044.out |   4 +-
  tests/qemu-iotests/046 |  49 -
  tests/qemu-iotests/046.out |  76 +++
  trace-events   |   2 +
  9 files changed, 534 insertions(+), 163 deletions(-)

Looks good except for an out-dated comment that I spotted.

Please either resend a fixed version or let me know the exact comment
fix you want so I can apply it while merging.

Thanks,
Stefan



Re: [Qemu-devel] [PATCH 11/12] qmp: add cpu_set qmp command

2013-03-27 Thread Paolo Bonzini
Il 21/03/2013 15:28, Igor Mammedov ha scritto:
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  include/sysemu/sysemu.h |2 ++
  qapi-schema.json|9 +
  qmp-commands.hx |   26 ++
  qmp.c   |   12 
  stubs/Makefile.objs |1 +
  stubs/do_cpu_hot_add.c  |7 +++
  6 files changed, 57 insertions(+), 0 deletions(-)
  create mode 100644 stubs/do_cpu_hot_add.c
 
 diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
 index 4b8f721..8bcaf26 100644
 --- a/include/sysemu/sysemu.h
 +++ b/include/sysemu/sysemu.h
 @@ -156,6 +156,8 @@ void drive_hot_add(Monitor *mon, const QDict *qdict);
  void qemu_register_cpu_add_notifier(Notifier *notifier);
  void qemu_system_cpu_hotplug_request(uint32_t id);
  
 +void do_cpu_hot_add(const int64_t id, Error **errp);
 +
  /* pcie aer error injection */
  void pcie_aer_inject_error_print(Monitor *mon, const QObject *data);
  int do_pcie_aer_inject_error(Monitor *mon,
 diff --git a/qapi-schema.json b/qapi-schema.json
 index fdaa9da..7acec6e 100644
 --- a/qapi-schema.json
 +++ b/qapi-schema.json
 @@ -1385,6 +1385,15 @@
  { 'command': 'cpu', 'data': {'index': 'int'} }
  
  ##
 +# @cpu_set
 +#
 +# Sets specified cpu to online/ofline mode
 +#
 +# Notes: semantics is : cpu_set id=x status=online|offline
 +##
 +{ 'command': 'cpu_set', 'data': {'id': 'int', 'status': 'str'} }
 +
 +##
  # @memsave:
  #
  # Save a portion of guest memory to a file.
 diff --git a/qmp-commands.hx b/qmp-commands.hx
 index b370060..283df76 100644
 --- a/qmp-commands.hx
 +++ b/qmp-commands.hx
 @@ -385,6 +385,32 @@ Note: CPUs' indexes are obtained with the 'query-cpus' 
 command.
  EQMP
  
  {
 +.name   = cpu_set,
 +.args_type  = id:i,status:s,
 +.mhandler.cmd_new = qmp_marshal_input_cpu_set,
 +},
 +
 +SQMP
 +cpu_set
 +---
 +
 +Sets virtual cpu to online/ofline state
 +
 +Arguments:
 +
 +- id: virtual cpu id (json-int)
 +- status: desired state of cpu, online/offline (json-string)
 +
 +Example:
 +
 +- { execute: cpu_set,
 + arguments: { id: 2,
 +status: online } }
 +- { return: {} }
 +
 +EQMP
 +
 +{
  .name   = memsave,
  .args_type  = val:l,size:i,filename:s,cpu:i?,
  .mhandler.cmd_new = qmp_marshal_input_memsave,
 diff --git a/qmp.c b/qmp.c
 index 55b056b..5b84779 100644
 --- a/qmp.c
 +++ b/qmp.c
 @@ -108,6 +108,18 @@ void qmp_cpu(int64_t index, Error **errp)
  /* Just do nothing */
  }
  
 +void qmp_cpu_set(int64_t id, const char *status, Error **errp)
 +{
 +if (!strcmp(status, online)) {
 +do_cpu_hot_add(id, errp);
 +} else if (!strcmp(status, offline)) {
 +error_setg(errp, Unplug is not implemented);
 +} else {
 +error_setg(errp, Invalid parameter '%s', status);
 +return;
 +}
 +}

Rather than adding stubs, what about the following:

1) put the QEMUMachineInitArgs in a global;

2) put a pointer to do_cpu_hot_add into the QEMUMachine;

3) call the callback from qmp_cpu_set if non-null, using the CPU model
from the QEMUMachineInitArgs

Paolo

  #ifndef CONFIG_VNC
  /* If VNC support is enabled, the true query-vnc command is
 defined in the VNC subsystem */
 diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
 index 6a492f5..4154a2b 100644
 --- a/stubs/Makefile.objs
 +++ b/stubs/Makefile.objs
 @@ -26,3 +26,4 @@ stub-obj-$(CONFIG_WIN32) += fd-register.o
  stub-obj-y += resume_vcpu.o
  stub-obj-y += get_icc_bus.o
  stub-obj-y += qemu_system_cpu_hotplug_request.o
 +stub-obj-y += do_cpu_hot_add.o
 diff --git a/stubs/do_cpu_hot_add.c b/stubs/do_cpu_hot_add.c
 new file mode 100644
 index 000..1f6d7a6
 --- /dev/null
 +++ b/stubs/do_cpu_hot_add.c
 @@ -0,0 +1,7 @@
 +#include qapi/error.h
 +#include sysemu/sysemu.h
 +
 +void do_cpu_hot_add(const int64_t id, Error **errp)
 +{
 +error_setg(errp, Not implemented);
 +}
 




[Qemu-devel] [PATCH v3 21/21] qcow2: Gather clusters in a looping loop

2013-03-27 Thread Kevin Wolf
Instead of just checking once in exactly this order if there are
dependendies, non-COW clusters and new allocation, this starts looping
around these. This way we can, for example, gather non-COW clusters after
new allocations as long as the host cluster offsets stay contiguous.

Once handle_dependencies() is extended so that COW areas of in-flight
allocations can be overwritten, this allows to continue with gathering
other clusters (we wouldn't be able to do that without this change
because we would have missed a possible second dependency in one of the
next clusters).

This means that in the typical sequential write case, we can combine the
COW overwrite of one cluster with the allocation of the next cluster as
soon as something like Delayed COW gets actually implemented. It is only
by avoiding splitting requests this way that Delayed COW actually starts
improving performance noticably.

Signed-off-by: Kevin Wolf kw...@redhat.com
---
 block/qcow2-cluster.c  | 74 +++---
 tests/qemu-iotests/044.out |  2 +-
 2 files changed, 44 insertions(+), 32 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 960d446..c71470a 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -770,7 +770,7 @@ out:
  *   must start over anyway, so consider *cur_bytes undefined.
  */
 static int handle_dependencies(BlockDriverState *bs, uint64_t guest_offset,
-uint64_t *cur_bytes)
+uint64_t *cur_bytes, QCowL2Meta **m)
 {
 BDRVQcowState *s = bs-opaque;
 QCowL2Meta *old_alloc;
@@ -793,6 +793,15 @@ static int handle_dependencies(BlockDriverState *bs, 
uint64_t guest_offset,
 bytes = 0;
 }
 
+/* Stop if already an l2meta exists. After yielding, it wouldn't
+ * be valid any more, so we'd have to clean up the old L2Metas
+ * and deal with requests depending on them before starting to
+ * gather new ones. Not worth the trouble. */
+if (bytes == 0  *m) {
+*cur_bytes = 0;
+return 0;
+}
+
 if (bytes == 0) {
 /* Wait for the dependency to complete. We need to recheck
  * the free/allocated clusters when we continue. */
@@ -1023,16 +1032,16 @@ static int handle_alloc(BlockDriverState *bs, uint64_t 
guest_offset,
 nb_clusters = count_cow_clusters(s, nb_clusters, l2_table, l2_index);
 }
 
+/* This function is only called when there were no non-COW clusters, so if
+ * we can't find any unallocated or COW clusters either, something is
+ * wrong with our code. */
+assert(nb_clusters  0);
+
 ret = qcow2_cache_put(bs, s-l2_table_cache, (void**) l2_table);
 if (ret  0) {
 return ret;
 }
 
-if (nb_clusters == 0) {
-*bytes = 0;
-return 0;
-}
-
 /* Allocate, if necessary at a given offset in the image file */
 alloc_cluster_offset = start_of_cluster(s, *host_offset);
 ret = do_alloc_cluster_offset(bs, guest_offset, alloc_cluster_offset,
@@ -1146,8 +1155,27 @@ again:
 remaining = (n_end - n_start)  BDRV_SECTOR_BITS;
 cluster_offset = 0;
 *host_offset = 0;
+cur_bytes = 0;
+*m = NULL;
 
 while (true) {
+
+if (!*host_offset) {
+*host_offset = start_of_cluster(s, cluster_offset);
+}
+
+assert(remaining = cur_bytes);
+
+start   += cur_bytes;
+remaining   -= cur_bytes;
+cluster_offset  += cur_bytes;
+
+if (remaining == 0) {
+break;
+}
+
+cur_bytes = remaining;
+
 /*
  * Now start gathering as many contiguous clusters as possible:
  *
@@ -1166,12 +1194,17 @@ again:
  * the right synchronisation between the in-flight request and
  * the new one.
  */
-cur_bytes = remaining;
-ret = handle_dependencies(bs, start, cur_bytes);
+ret = handle_dependencies(bs, start, cur_bytes, m);
 if (ret == -EAGAIN) {
+/* Currently handle_dependencies() doesn't yield if we already had
+ * an allocation. If it did, we would have to clean up the L2Meta
+ * structs before starting over. */
+assert(*m == NULL);
 goto again;
 } else if (ret  0) {
 return ret;
+} else if (cur_bytes == 0) {
+break;
 } else {
 /* handle_dependencies() may have decreased cur_bytes (shortened
  * the allocations below) so that the next dependency is processed
@@ -1185,24 +1218,11 @@ again:
 if (ret  0) {
 return ret;
 } else if (ret) {
-if (!*host_offset) {
-*host_offset = start_of_cluster(s, cluster_offset);
-}
-
-start   += cur_bytes;
-remaining   -= cur_bytes;
-cluster_offset  += 

Re: [Qemu-devel] [PATCH 10/12] acpi_piix4: add infrastructure to send CPU hot-plug GPE to guest

2013-03-27 Thread Paolo Bonzini
Il 21/03/2013 15:28, Igor Mammedov ha scritto:
 * introduce processor status bitmask visible to guest at 0xaf00 addr,
   where Seabios expects it
 * set bit corresponding to APIC ID in processor status bitmask on
   receiving CPU hot-plug notification
 * trigger CPU hot-plug SCI, expected by Seabios on receiving CPU
   hot-plug notification
 
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  hw/acpi_piix4.c |  107 +-
  1 files changed, 105 insertions(+), 2 deletions(-)
 
 diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
 index 7a4b712..ddb3981 100644
 --- a/hw/acpi_piix4.c
 +++ b/hw/acpi_piix4.c
 @@ -48,19 +48,28 @@
  #define PCI_EJ_BASE 0xae08
  #define PCI_RMV_BASE 0xae0c
  
 +#define PROC_BASE 0xaf00
 +#define PROC_LEN 32
 +
  #define PIIX4_PCI_HOTPLUG_STATUS 2
 +#define PIIX4_CPU_HOTPLUG_STATUS 4
  
  struct pci_status {
  uint32_t up; /* deprecated, maintained for migration compatibility */
  uint32_t down;
  };
  
 +struct cpu_status {
 +uint8_t sts[PROC_LEN];
 +};
 +
  typedef struct PIIX4PMState {
  PCIDevice dev;
  
  MemoryRegion io;
  MemoryRegion io_gpe;
  MemoryRegion io_pci;
 +MemoryRegion io_cpu;
  ACPIREGS ar;
  
  APMState apm;
 @@ -82,6 +91,9 @@ typedef struct PIIX4PMState {
  uint8_t disable_s3;
  uint8_t disable_s4;
  uint8_t s4_val;
 +
 +struct cpu_status gpe_cpu;
 +Notifier cpu_add_notifier;
  } PIIX4PMState;
  
  static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
 @@ -100,8 +112,8 @@ static void pm_update_sci(PIIX4PMState *s)
 ACPI_BITMASK_POWER_BUTTON_ENABLE |
 ACPI_BITMASK_GLOBAL_LOCK_ENABLE |
 ACPI_BITMASK_TIMER_ENABLE)) != 0) ||
 -(((s-ar.gpe.sts[0]  s-ar.gpe.en[0])
 -   PIIX4_PCI_HOTPLUG_STATUS) != 0);
 +(((s-ar.gpe.sts[0]  s-ar.gpe.en[0]) 
 +  (PIIX4_PCI_HOTPLUG_STATUS | PIIX4_CPU_HOTPLUG_STATUS)) != 0);
  
  qemu_set_irq(s-irq, sci_level);
  /* schedule a timer interruption if needed */
 @@ -257,6 +269,17 @@ static int acpi_load_old(QEMUFile *f, void *opaque, int 
 version_id)
  return ret;
  }
  
 +#define VMSTATE_CPU_STATUS_ARRAY(_field, _state) 
 \
 + {   
 \
 + .name   = (stringify(_field)),  
 \
 + .version_id = 0,
 \
 + .num= PROC_LEN, 
 \
 + .info   = vmstate_info_uint8,  
 \
 + .size   = sizeof(uint8_t),  
 \
 + .flags  = VMS_ARRAY,
 \
 + .offset = vmstate_offset_array(_state, _field, uint8_t, PROC_LEN),  
 \
 + }
 +
  /* qemu-kvm 1.2 uses version 3 but advertised as 2
   * To support incoming qemu-kvm 1.2 migration, change version_id
   * and minimum_version_id to 2 below (which breaks migration from
 @@ -281,6 +304,7 @@ static const VMStateDescription vmstate_acpi = {
  VMSTATE_STRUCT(ar.gpe, PIIX4PMState, 2, vmstate_gpe, ACPIGPE),
  VMSTATE_STRUCT(pci0_status, PIIX4PMState, 2, vmstate_pci_status,
 struct pci_status),
 +VMSTATE_CPU_STATUS_ARRAY(gpe_cpu.sts, PIIX4PMState),
  VMSTATE_END_OF_LIST()
  }

You need to either bump the version, or add a subsection.   The
subsection is not needed until the first hot-(un)plug action.

  };
 @@ -585,6 +609,78 @@ static const MemoryRegionOps piix4_pci_ops = {
  },
  };
  
 +static uint64_t cpu_status_read(void *opaque, hwaddr addr, unsigned width)
 +{
 +PIIX4PMState *s = opaque;
 +struct cpu_status *cpus = s-gpe_cpu;
 +uint64_t val = cpus-sts[addr];
 +return val;
 +}
 +
 +static void cpu_status_write(void *opaque, hwaddr addr, uint64_t data,
 + unsigned int size)
 +{
 +/* TODO: implement VCPU removal on guest signal that CPU can be removed 
 */
 +}
 +
 +static const MemoryRegionOps cpu_hotplug_ops = {
 +.read = cpu_status_read,
 +.write = cpu_status_write,
 +.endianness = DEVICE_LITTLE_ENDIAN,
 +.valid = {
 +.min_access_size = 1,
 +.max_access_size = 1,
 +},
 +};
 +
 +typedef enum {
 +PLUG,
 +UNPLUG,
 +} HotplugEventType;
 +
 +static void piix4_cpu_hotplug_req(PIIX4PMState *s, uint32_t cpu,
 +   HotplugEventType action)
 +{
 +struct cpu_status *g = s-gpe_cpu;
 +ACPIGPE *gpe = s-ar.gpe;
 +
 +assert(s != NULL);
 +*gpe-sts = *gpe-sts | PIIX4_CPU_HOTPLUG_STATUS;
 +if (action == PLUG) {
 +g-sts[cpu / 8] |= (1  (cpu % 8));
 +} else {
 +g-sts[cpu / 8] = ~(1  (cpu % 8));
 +}
 +pm_update_sci(s);
 +}
 +
 +static void piix4_cpu_add_req(Notifier *n, void *opaque)
 +{
 + 

Re: [Qemu-devel] [PATCH 07/12] target-i386: Add ICC_BUS and attach apic, kvmvapic and cpu to it

2013-03-27 Thread Paolo Bonzini
Il 21/03/2013 15:28, Igor Mammedov ha scritto:
 Introduce hot-pluggable ICC_BUS and convert APIC devices from SysBusDevice
 to ICCDevice wich has ICC_BUS as default one.
 
  * attach APIC and kvmvapic to ICC_BUS during creation
  * use memory API directly instead of using SysBus proxy functions
  * introduce get_icc_bus() for getting access to system wide ICC_BUS
  * make ICC_BUS default one for X86CPU class, so that device_add would
set it for CPU instead of SysBus
  * attach cold-pluged CPUs to icc_bus in softmmu mode, so they could be
uplugged in future
  * if kvmvapic init() fails return -1 instead of aborting.
 
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  hw/apic_common.c  |   14 ++---
  hw/apic_internal.h|6 ++--
  hw/i386/Makefile.objs |2 +-
  hw/i386/kvmvapic.c|   15 +
  hw/icc_bus.c  |   75 
 +
  hw/icc_bus.h  |   47 ++
  stubs/Makefile.objs   |1 +
  stubs/get_icc_bus.c   |6 
  target-i386/cpu.c |   16 --
  9 files changed, 162 insertions(+), 20 deletions(-)
  create mode 100644 hw/icc_bus.c
  create mode 100644 hw/icc_bus.h
  create mode 100644 stubs/get_icc_bus.c
 
 diff --git a/hw/apic_common.c b/hw/apic_common.c
 index d0c2616..61599d4 100644
 --- a/hw/apic_common.c
 +++ b/hw/apic_common.c
 @@ -21,6 +21,7 @@
  #include hw/apic_internal.h
  #include trace.h
  #include sysemu/kvm.h
 +#include qdev.h
  
  static int apic_irq_delivered;
  bool apic_report_tpr_access;
 @@ -282,9 +283,10 @@ static int apic_load_old(QEMUFile *f, void *opaque, int 
 version_id)
  return 0;
  }
  
 -static int apic_init_common(SysBusDevice *dev)
 +static int apic_init_common(ICCDevice *dev)
  {
  APICCommonState *s = APIC_COMMON(dev);
 +DeviceState *d = DEVICE(dev);
  APICCommonClass *info;
  static DeviceState *vapic;
  static int apic_no;
 @@ -297,12 +299,14 @@ static int apic_init_common(SysBusDevice *dev)
  info = APIC_COMMON_GET_CLASS(s);
  info-init(s);
  
 -sysbus_init_mmio(dev, s-io_memory);
  
  /* Note: We need at least 1M to map the VAPIC option ROM */
  if (!vapic  s-vapic_control  VAPIC_ENABLE_MASK 
  ram_size = 1024 * 1024) {
 -vapic = sysbus_create_simple(kvmvapic, -1, NULL);
 +vapic = qdev_try_create(d-parent_bus, kvmvapic);
 +if ((vapic == NULL) || (qdev_init(vapic) != 0)) {
 +return -1;
 +}
  }
  s-vapic = vapic;
  if (apic_report_tpr_access  info-enable_tpr_reporting) {
 @@ -375,7 +379,7 @@ static Property apic_properties_common[] = {
  
  static void apic_common_class_init(ObjectClass *klass, void *data)
  {
 -SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass);
 +ICCDeviceClass *sc = ICC_DEVICE_CLASS(klass);
  DeviceClass *dc = DEVICE_CLASS(klass);
  
  dc-vmsd = vmstate_apic_common;
 @@ -387,7 +391,7 @@ static void apic_common_class_init(ObjectClass *klass, 
 void *data)
  
  static const TypeInfo apic_common_type = {
  .name = TYPE_APIC_COMMON,
 -.parent = TYPE_SYS_BUS_DEVICE,
 +.parent = TYPE_ICC_DEVICE,
  .instance_size = sizeof(APICCommonState),
  .class_size = sizeof(APICCommonClass),
  .class_init = apic_common_class_init,
 diff --git a/hw/apic_internal.h b/hw/apic_internal.h
 index 578241f..e7b378e 100644
 --- a/hw/apic_internal.h
 +++ b/hw/apic_internal.h
 @@ -21,7 +21,7 @@
  #define QEMU_APIC_INTERNAL_H
  
  #include exec/memory.h
 -#include hw/sysbus.h
 +#include hw/icc_bus.h
  #include qemu/timer.h
  
  /* APIC Local Vector Table */
 @@ -80,7 +80,7 @@ typedef struct APICCommonState APICCommonState;
  
  typedef struct APICCommonClass
  {
 -SysBusDeviceClass parent_class;
 +ICCDeviceClass parent_class;
  
  void (*init)(APICCommonState *s);
  void (*set_base)(APICCommonState *s, uint64_t val);
 @@ -94,7 +94,7 @@ typedef struct APICCommonClass
  } APICCommonClass;
  
  struct APICCommonState {
 -SysBusDevice busdev;
 +ICCDevice busdev;
  
  MemoryRegion io_memory;
  X86CPU *cpu;
 diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
 index a78c0b2..316f999 100644
 --- a/hw/i386/Makefile.objs
 +++ b/hw/i386/Makefile.objs
 @@ -1,5 +1,5 @@
  obj-y += mc146818rtc.o
 -obj-y += apic_common.o apic.o
 +obj-y += apic_common.o apic.o icc_bus.o
  obj-y += sga.o ioapic_common.o ioapic.o piix_pci.o
  obj-y += vmport.o
  obj-y += pci/pci-hotplug.o wdt_ib700.o
 diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
 index 21551a5..7de75db 100644
 --- a/hw/i386/kvmvapic.c
 +++ b/hw/i386/kvmvapic.c
 @@ -12,6 +12,8 @@
  #include sysemu/cpus.h
  #include sysemu/kvm.h
  #include hw/apic_internal.h
 +#include migration/vmstate.h
 +#include exec/address-spaces.h
  
  #define APIC_DEFAULT_ADDRESS0xfee0
  
 @@ -49,7 +51,7 @@ typedef struct GuestROMState {
  } QEMU_PACKED GuestROMState;
  
  typedef struct VAPICROMState {
 -SysBusDevice busdev;
 +

Re: [Qemu-devel] [PATCH 05/12] target-i386: push hot-plugged VCPU state to KVM and unstop it

2013-03-27 Thread Paolo Bonzini
Il 21/03/2013 15:28, Igor Mammedov ha scritto:
  * synchronize CPU state to KVM
  * resume CPU, it makes CPU be able to handle interrupts in TCG mode
and/or with user-space APIC.
 
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  cpus.c|   11 ---
  include/sysemu/cpus.h |3 +++
  stubs/Makefile.objs   |1 +
  stubs/resume_vcpu.c   |6 ++
  target-i386/cpu.c |5 +
  5 files changed, 23 insertions(+), 3 deletions(-)
  create mode 100644 stubs/resume_vcpu.c
 
 diff --git a/cpus.c b/cpus.c
 index e919dd7..ae479bf 100644
 --- a/cpus.c
 +++ b/cpus.c
 @@ -973,6 +973,13 @@ void pause_all_vcpus(void)
  }
  }
  
 +void resume_vcpu(CPUState *cpu)
 +{
 +cpu-stop = false;
 +cpu-stopped = false;
 +qemu_cpu_kick(cpu);
 +}
 +
  void resume_all_vcpus(void)
  {
  CPUArchState *penv = first_cpu;
 @@ -980,9 +987,7 @@ void resume_all_vcpus(void)
  qemu_clock_enable(vm_clock, true);
  while (penv) {
  CPUState *pcpu = ENV_GET_CPU(penv);
 -pcpu-stop = false;
 -pcpu-stopped = false;
 -qemu_cpu_kick(pcpu);
 +resume_vcpu(pcpu);
  penv = penv-next_cpu;
  }
  }
 diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
 index 6502488..9437df5 100644
 --- a/include/sysemu/cpus.h
 +++ b/include/sysemu/cpus.h
 @@ -1,11 +1,14 @@
  #ifndef QEMU_CPUS_H
  #define QEMU_CPUS_H
  
 +#include qom/cpu.h
 +
  /* cpus.c */
  void qemu_init_cpu_loop(void);
  void resume_all_vcpus(void);
  void pause_all_vcpus(void);
  void cpu_stop_current(void);
 +void resume_vcpu(CPUState *cpu);
  
  void cpu_synchronize_all_states(void);
  void cpu_synchronize_all_post_reset(void);
 diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
 index 9c55b34..28fb4f8 100644
 --- a/stubs/Makefile.objs
 +++ b/stubs/Makefile.objs
 @@ -23,3 +23,4 @@ stub-obj-y += sysbus.o
  stub-obj-y += vm-stop.o
  stub-obj-y += vmstate.o
  stub-obj-$(CONFIG_WIN32) += fd-register.o
 +stub-obj-y += resume_vcpu.o
 diff --git a/stubs/resume_vcpu.c b/stubs/resume_vcpu.c
 new file mode 100644
 index 000..383b71a
 --- /dev/null
 +++ b/stubs/resume_vcpu.c
 @@ -0,0 +1,6 @@
 +#include qemu-common.h
 +#include sysemu/cpus.h
 +
 +void resume_vcpu(CPUState *cpu)
 +{
 +}
 diff --git a/target-i386/cpu.c b/target-i386/cpu.c
 index d03ff73..631bcd8 100644
 --- a/target-i386/cpu.c
 +++ b/target-i386/cpu.c
 @@ -2260,6 +2260,11 @@ out:
  error_propagate(errp, local_err);
  return;
  }
 +
 +if (dev-hotplugged) {
 +cpu_synchronize_post_init(env);
 +resume_vcpu(CPU(cpu));
 +}

I think the resume_vcpu should not be here, but in the superclass.

Paolo

  }
  
  /* Enables contiguous-apic-ID mode, for compatibility */
 




Re: [Qemu-devel] [PATCH 4/5] qdev: add qdev_{create, free} tracepoints

2013-03-27 Thread Andreas Färber
Am 22.03.2013 09:29, schrieb Kazuya Saito:
 This patch adds tracepoints at creating and removing virtual
 devices. It is useful for investigation of trouble related to virtual
 devices.
 
 Signed-off-by: Kazuya Saito saito.kaz...@jp.fujitsu.com

I would prefer not to do this. I had previously posted a patch to remove
qdev_free() in favor of using the QOM function object_unparent()
directly, which adding stuff to qdev_free() would interfere with. And
you should rather add a tracepoint to object_new() or better to
object_initialize() than into the legacy qdev_create() - which doesn't
cover qdev_try_create() btw. Either way, adding new tracepoints with the
legacy qdev in the name is ugly.

Regards,
Andreas

P.S. Your patches arrived in HTML format, please check your workflow.

 ---
  hw/qdev.c|3 +++
  trace-events |4 
  2 files changed, 7 insertions(+), 0 deletions(-)
 
 diff --git a/hw/qdev.c b/hw/qdev.c
 index 0b20280..0fda23e 100644
 --- a/hw/qdev.c
 +++ b/hw/qdev.c
 @@ -30,6 +30,7 @@
  #include qapi/error.h
  #include qapi/qmp/qerror.h
  #include qapi/visitor.h
 +#include trace.h
 
  int qdev_hotplug = 0;
  static bool qdev_hot_added = false;
 @@ -124,6 +125,7 @@ DeviceState *qdev_create(BusState *bus, const char *name)
  }
  }
 
 +trace_qdev_create(dev, dev-parent_bus);
  return dev;
  }
 
 @@ -268,6 +270,7 @@ void qdev_init_nofail(DeviceState *dev)
  /* Unlink device from bus and free the structure.  */
  void qdev_free(DeviceState *dev)
  {
 +trace_qdev_free(dev, dev-parent_bus);
  object_unparent(OBJECT(dev));
  }
 
 diff --git a/trace-events b/trace-events
 index c691ce4..235b978 100644
 --- a/trace-events
 +++ b/trace-events
 @@ -1102,3 +1102,7 @@ kvm_ioctl(int type) type %d
  kvm_vm_ioctl(int type) type %d
  kvm_vcpu_ioctl(int type) type %d
  kvm_run_exit(uint32_t reason) reason %d
 +
 +# qdev.c
 +qdev_create(void *dev, void *bus) dev %p, bus %p
 +qdev_free(void *dev, void *bus) dev %p, bus %p
 


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



Re: [Qemu-devel] [PATCH 08/12] introduce CPU hot-plug notifier

2013-03-27 Thread Paolo Bonzini
Il 21/03/2013 15:28, Igor Mammedov ha scritto:
 hot-added CPU id (APIC ID) will be distributed to acpi_piix4 and rtc_cmos
 
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  include/sysemu/sysemu.h |4 
  stubs/Makefile.objs |1 +
  stubs/qemu_system_cpu_hotplug_request.c |5 +
  vl.c|   14 ++
  4 files changed, 24 insertions(+), 0 deletions(-)
  create mode 100644 stubs/qemu_system_cpu_hotplug_request.c
 
 diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
 index 6578782..4b8f721 100644
 --- a/include/sysemu/sysemu.h
 +++ b/include/sysemu/sysemu.h
 @@ -152,6 +152,10 @@ void do_pci_device_hot_remove(Monitor *mon, const QDict 
 *qdict);
  /* generic hotplug */
  void drive_hot_add(Monitor *mon, const QDict *qdict);
  
 +/* CPU hotplug */
 +void qemu_register_cpu_add_notifier(Notifier *notifier);
 +void qemu_system_cpu_hotplug_request(uint32_t id);
 +
  /* pcie aer error injection */
  void pcie_aer_inject_error_print(Monitor *mon, const QObject *data);
  int do_pcie_aer_inject_error(Monitor *mon,
 diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
 index 9741e16..6a492f5 100644
 --- a/stubs/Makefile.objs
 +++ b/stubs/Makefile.objs
 @@ -25,3 +25,4 @@ stub-obj-y += vmstate.o
  stub-obj-$(CONFIG_WIN32) += fd-register.o
  stub-obj-y += resume_vcpu.o
  stub-obj-y += get_icc_bus.o
 +stub-obj-y += qemu_system_cpu_hotplug_request.o

You're adding one stub per patch.  I think this is a sign that something
can be abstracted at a higher level (e.g. put something in cpus.c if it
is softmmu-specific), or that it is added at the wrong place.

For example, this notifier can go in qom/cpu.c.

(Besides, I noticed now the get_icc_bus stub.  I didn't understand why
it's used, but anyway adding CPU-specific stuff to libqemustub is
absolutely a no-no).

Paolo

 diff --git a/stubs/qemu_system_cpu_hotplug_request.c 
 b/stubs/qemu_system_cpu_hotplug_request.c
 new file mode 100644
 index 000..ad4f394
 --- /dev/null
 +++ b/stubs/qemu_system_cpu_hotplug_request.c
 @@ -0,0 +1,5 @@
 +#include sysemu/sysemu.h
 +
 +void qemu_system_cpu_hotplug_request(uint32_t id)
 +{
 +}
 diff --git a/vl.c b/vl.c
 index aeed7f4..fd95e43 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -1723,6 +1723,20 @@ void vm_start(void)
  }
  }
  
 +/* CPU hot-plug notifiers */
 +static NotifierList cpu_add_notifiers =
 +NOTIFIER_LIST_INITIALIZER(cpu_add_notifiers);
 +
 +void qemu_register_cpu_add_notifier(Notifier *notifier)
 +{
 +notifier_list_add(cpu_add_notifiers, notifier);
 +}
 +
 +void qemu_system_cpu_hotplug_request(uint32_t id)
 +{
 +notifier_list_notify(cpu_add_notifiers, id);
 +}
 +
  /* reset/shutdown handler */
  
  typedef struct QEMUResetEntry {
 




Re: [Qemu-devel] [PATCH 12/12] target-i386: implement CPU hot-add

2013-03-27 Thread Paolo Bonzini
Il 21/03/2013 15:28, Igor Mammedov ha scritto:
 ... via do_cpu_hot_add() hook called by cpu_set QMP command,
 for x86 target.
 
 * add extra check that APIC ID is in allowed range
 * return error if CPU with requested APIC ID exists before creating
   a new instance. (CPU removal is broken now, will be fixed with CPU unplug)
 * call CPU add notifier as the last step of x86_cpu_realizefn() to
   update rtc_cmos and trigger CPU hot-plug ACPI GPE to notify guest.
   Doing it from x86_cpu_realizefn() will allow to do the same when
   it would be possible to add CPU using device_add.
 
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  hw/i386/pc.c  |   22 ++
  target-i386/cpu.c |1 +
  2 files changed, 23 insertions(+), 0 deletions(-)
 
 diff --git a/hw/i386/pc.c b/hw/i386/pc.c
 index 7481f73..e3ba9ee 100644
 --- a/hw/i386/pc.c
 +++ b/hw/i386/pc.c
 @@ -867,6 +867,19 @@ static void pc_new_cpu(const char *cpu_model, int64_t 
 apic_id, Error **errp)
  {
  X86CPU *cpu;
  
 +if (apic_id = pc_apic_id_limit(max_cpus)) {
 +error_setg(errp, Unable to add CPU with ID: 0x% PRIx64
 +   , max allowed ID: 0x%x, apic_id,
 +   pc_apic_id_limit(max_cpus) - 1);
 +return;
 +}

This seems the wrong place to do this check.  It should be done in
do_cpu_hot_add, simply comparing against max_cpus.  Here, instead, you
should _assert_ that the APIC ID is in range.

 +if (x86_cpu_is_cpu_exist(qdev_get_machine(), apic_id)) {

Similarly, can this be done in qmp_cpu_set?  And should it really be an
error?  Onlining an already-online CPU is fine.

Again, here you could assert that the CPU is not a duplicate, instead.

 +error_setg(errp, Unable to add CPU with ID: 0x% PRIx64
 +   , it's already exists, apic_id);
 +return;
 +}
  cpu = cpu_x86_create(cpu_model, errp);
  if (!cpu) {
  return;
 @@ -882,6 +895,8 @@ static void pc_new_cpu(const char *cpu_model, int64_t 
 apic_id, Error **errp)
  }
  }
  
 +static const char *saved_cpu_model;
 +
  void pc_cpus_init(const char *cpu_model)

Instead of using this global, see the approach in the previous patch.

  {
  int i;
 @@ -895,6 +910,8 @@ void pc_cpus_init(const char *cpu_model)
  cpu_model = qemu32;
  #endif
  }
 +saved_cpu_model = cpu_model;
 +

  for (i = 0; i  smp_cpus; i++) {
  pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i), error);
 @@ -906,6 +923,11 @@ void pc_cpus_init(const char *cpu_model)
  }
  }
  
 +void do_cpu_hot_add(const int64_t id, Error **errp)
 +{
 +pc_new_cpu(saved_cpu_model, id, errp);
 +}
 +

Missing x86_cpu_apic_id_from_index(id)?

  void pc_acpi_init(const char *default_dsdt)
  {
  char *filename = NULL, *arg = NULL;
 diff --git a/target-i386/cpu.c b/target-i386/cpu.c
 index ae46f81..d127141 100644
 --- a/target-i386/cpu.c
 +++ b/target-i386/cpu.c
 @@ -2271,6 +2271,7 @@ out:
  if (dev-hotplugged) {
  cpu_synchronize_post_init(env);
  resume_vcpu(CPU(cpu));
 +qemu_system_cpu_hotplug_request(env-cpuid_apic_id);

As mentioned earlier, this notifier should be invoked at the CPU level,
not X86CPU.

Paolo

  }
  }
  
 




Re: [Qemu-devel] [RFC PATCH 04/10] qemu-ga: Add Windows VSS provider to quiesce applications on fsfreeze

2013-03-27 Thread Vadim Rozenfeld
On Tue, 2013-03-26 at 16:30 +, Tomoki Sekiyama wrote:
 On 3/26/13 4:44 , Vadim Rozenfeld vroze...@redhat.com wrote:
 
 On Tue, 2013-03-26 at 08:09 +0100, Paolo Bonzini wrote:
  Il 25/03/2013 21:50, Tomoki Sekiyama ha scritto:
  Unfortunately, if I remove importlib(stdole2.tlb), generated .tlb
 seems
   rejected to register into Windows COM+ Catalog.
   
   Wine has stdole2.tlb in its fakedlls directory, but widl does not
 accept
   this by
   
 error: Wrong or unsupported typelib magic 405a4d
   
   Even if I copied native stdole2.tlb, widl fails with the same error.
   
   Do you have any idea about this error?
 
 Seems like your tlb has nonstandard identifying characters. Check first
 several bytes of the tlb file. Usually it should be something like '4D
 53 46 54', or '4D 5A 90'
 
 I found '4D 53 46 54' at offset 0x300 of stdole2.tlb.
 Wine's stdole2.tlb also have the signature at offset 0x738.
 

I rechecked stdole2.tlb provided by MS. All three versions (arm, x86,
and x64) begin with 'MSFT' (4D 53 46 54) and have 00 00 00 00  at offset
0x300

 (might have an additional header before the main part?)
 
 Widl accepted the wine's stdole2.tlb when I removed the header,
 but still, the generated qga-provider.tlb was not accepted by Windows...:(
 
  
  No, I suggest you ask on the Wine mailing lists.  In the meanwhile we
  can include a precompiled .tlb file in the QEMU repository and use midl.
  
  Paolo
 
 I will take this way so far.
 
 Thanks,
 
 Tomoki Sekiyama
 





Re: [Qemu-devel] Testing Linux user-mode with LTP

2013-03-27 Thread Peter Maydell
On 14 February 2013 11:22, Peter Maydell peter.mayd...@linaro.org wrote:
 On 8 February 2013 12:32, Peter Maydell peter.mayd...@linaro.org wrote:
 Hi; I spent yesterday working out how to run the Linux Test
 Project's syscall test suite under QEMU's linux-user mode,
 and I wrote up the instructions here for other peoples'
 benefit:
   http://wiki.qemu.org/Testing/LTP

 Thanks to Fathi Boudra, Linaro is now running armhf LTP tests
 automatically nightly based on qemu upstream master: no pretty
 webpages or notification of progression/regression

I spent a little time yesterday getting the LTP results into a
JUnit xml format that jenkins liked, so now we have graphs of
pass/fail over time:

https://ci.linaro.org/jenkins/job/qemu-ltp/

-- PMM



Re: [Qemu-devel] [PATCH v2 0/4] monitor: do not rely on O_NONBLOCK for passed file descriptors

2013-03-27 Thread Eric Blake
On 03/27/2013 03:10 AM, Stefan Hajnoczi wrote:
 There are several places where QEMU accidentally relies on the O_NONBLOCK 
 state
 of passed file descriptors.  Exposing O_NONBLOCK state makes it part of the 
 QMP
 API whenever getfd or fdset_add_fd are used!
 
 Whether or not QEMU will use O_NONBLOCK is an implementation detail and should
 be hidden from QMP clients.
 
 This patch series addresses this in 3 steps:
 
 1. Fix callers of monitor_handle_fd_param(), monitor_fdset_get_fd(), and
monitor_get_fd() that depend on O_NONBLOCK being set.  Luckily there are
only two instances and they are fixed in Patches 1  2.

Description is now off after rebase, but the cover letter isn't
committed, so no big deal.

 
 2. Rename socket_set_nonblock() to qemu_set_nonblock() just like
qemu_set_cloexec().  This makes code cleaner when working with arbitrary
file descriptors that may not be sockets.  See Patch 3.
 
 3. Clear O_NONBLOCK when a chardev receives file descriptors.  From now on 
 QEMU
can assume that passed file descriptors are in blocking mode.  Simply use
qemu_set_nonblock(fd) if you want to enable O_NONBLOCK.  See Patch 4.

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

 v2:
  * Rename socket_set_nonblock() in Patch 1 to avoid code churn [eblake]
  * Avoid qemu_set_block(-1) calls that clobber errno [quintela]
 
 Stefan Hajnoczi (4):
   oslib-posix: rename socket_set_nonblock() to qemu_set_nonblock()
   net: ensure socket backend uses non-blocking fds
   qemu-socket: set passed fd non-blocking in socket_connect()
   chardev: clear O_NONBLOCK on SCM_RIGHTS file descriptors

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v2] QOM-ify the TPM support

2013-03-27 Thread Paolo Bonzini
Il 26/03/2013 17:28, Stefan Berger ha scritto:
 QOM-ified the TPM support with much code borrowed from the rng implementation.
 
 What's missing may be that the tpm/tpm_passthrough.c be moved into backends/ .
 
 Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com
 
 ---
  v1-v2:
   - followed to git revision e769bdc26
 
 ---
  backends/Makefile.objs |2 
  backends/tpm.c |  154 
  include/qemu/tpm.h |  170 
 +

I think these should be tpm_backend.[ch] (with the include file in
include/tpm).  Can you rename the existing files with that name first?

Paolo

  include/tpm/tpm.h  |4 +
  tpm/tpm.c  |   11 ++-
  tpm/tpm_int.h  |   16 
  tpm/tpm_passthrough.c  |   94 +--
  tpm/tpm_tis.c  |   21 +++---
  8 files changed, 413 insertions(+), 59 deletions(-)
 
 Index: qemu-git.pt/backends/Makefile.objs
 ===
 --- qemu-git.pt.orig/backends/Makefile.objs
 +++ qemu-git.pt/backends/Makefile.objs
 @@ -4,3 +4,5 @@ common-obj-$(CONFIG_POSIX) += rng-random
  common-obj-y += msmouse.o
  common-obj-$(CONFIG_BRLAPI) += baum.o
  $(obj)/baum.o: QEMU_CFLAGS += $(SDL_CFLAGS) 
 +
 +common-obj-$(CONFIG_TPM) += tpm.o
 Index: qemu-git.pt/backends/tpm.c
 ===
 --- /dev/null
 +++ qemu-git.pt/backends/tpm.c
 @@ -0,0 +1,154 @@
 +/*
 + * QEMU TPM Backend
 + *
 + * Copyright IBM, Corp. 2013
 + *
 + * Authors:
 + *  Stefan Berger   stef...@us.ibm.com
 + *
 + * This work is licensed under the terms of the GNU GPL, version 2 or later.
 + * See the COPYING file in the top-level directory.
 + *
 + * Based on backends/rng.c by Anthony Liguori
 + */
 +
 +#include qemu/tpm.h
 +#include tpm/tpm_int.h
 +#include qapi/qmp/qerror.h
 +
 +enum TpmType tpm_backend_get_type(TPMBackend *s)
 +{
 +TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
 +
 +return k-ops-type;
 +}
 +
 +const char *tpm_backend_get_desc(TPMBackend *s)
 +{
 +TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
 +
 +return k-ops-desc();
 +}
 +
 +void tpm_backend_destroy(TPMBackend *s)
 +{
 +TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
 +
 +return k-ops-destroy(s);
 +}
 +
 +int tpm_backend_init(TPMBackend *s, TPMState *state,
 + TPMRecvDataCB *datacb)
 +{
 +TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
 +
 +return k-ops-init(s, state, datacb);
 +}
 +
 +int tpm_backend_startup_tpm(TPMBackend *s)
 +{
 +TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
 +
 +return k-ops-startup_tpm(s);
 +}
 +
 +bool tpm_backend_had_startup_error(TPMBackend *s)
 +{
 +TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
 +
 +return k-ops-had_startup_error(s);
 +}
 +
 +size_t tpm_backend_realloc_buffer(TPMBackend *s, TPMSizedBuffer *sb)
 +{
 +TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
 +
 +return k-ops-realloc_buffer(sb);
 +}
 +
 +void tpm_backend_deliver_request(TPMBackend *s)
 +{
 +TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
 +
 +k-ops-deliver_request(s);
 +}
 +
 +void tpm_backend_reset(TPMBackend *s)
 +{
 +TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
 +
 +k-ops-reset(s);
 +}
 +
 +void tpm_backend_cancel_cmd(TPMBackend *s)
 +{
 +TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
 +
 +k-ops-cancel_cmd(s);
 +}
 +
 +bool tpm_backend_get_tpm_established_flag(TPMBackend *s)
 +{
 +TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
 +
 +return k-ops-get_tpm_established_flag(s);
 +}
 +
 +static bool tpm_backend_prop_get_opened(Object *obj, Error **errp)
 +{
 +TPMBackend *s = TPM_BACKEND(obj);
 +
 +return s-opened;
 +}
 +
 +void tpm_backend_open(TPMBackend *s, Error **errp)
 +{
 +object_property_set_bool(OBJECT(s), true, opened, errp);
 +}
 +
 +static void tpm_backend_prop_set_opened(Object *obj, bool value, Error 
 **errp)
 +{
 +TPMBackend *s = TPM_BACKEND(obj);
 +TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s);
 +
 +if (value == s-opened) {
 +return;
 +}
 +
 +if (!value  s-opened) {
 +error_set(errp, QERR_PERMISSION_DENIED);
 +return;
 +}
 +
 +if (k-opened) {
 +k-opened(s, errp);
 +}
 +
 +if (!error_is_set(errp)) {
 +s-opened = value;
 +}
 +}
 +
 +static void tpm_backend_instance_init(Object *obj)
 +{
 +object_property_add_bool(obj, opened,
 + tpm_backend_prop_get_opened,
 + tpm_backend_prop_set_opened,
 + NULL);
 +}
 +
 +static const TypeInfo tpm_backend_info = {
 +.name = TYPE_TPM_BACKEND,
 +.parent = TYPE_OBJECT,
 +.instance_size = sizeof(TPMBackend),
 +.instance_init = tpm_backend_instance_init,
 +.class_size = sizeof(TPMBackendClass),
 +.abstract = true,
 +};
 +
 +static void register_types(void)
 +{
 +

Re: [Qemu-devel] [PATCH 05/12] target-i386: push hot-plugged VCPU state to KVM and unstop it

2013-03-27 Thread Igor Mammedov
On Wed, 27 Mar 2013 12:01:20 +0100
Paolo Bonzini pbonz...@redhat.com wrote:

 Il 21/03/2013 15:28, Igor Mammedov ha scritto:
   * synchronize CPU state to KVM
   * resume CPU, it makes CPU be able to handle interrupts in TCG mode
 and/or with user-space APIC.
  
  Signed-off-by: Igor Mammedov imamm...@redhat.com
  ---
   cpus.c|   11 ---
   include/sysemu/cpus.h |3 +++
   stubs/Makefile.objs   |1 +
   stubs/resume_vcpu.c   |6 ++
   target-i386/cpu.c |5 +
   5 files changed, 23 insertions(+), 3 deletions(-)
   create mode 100644 stubs/resume_vcpu.c
  
  diff --git a/cpus.c b/cpus.c
  index e919dd7..ae479bf 100644
  --- a/cpus.c
  +++ b/cpus.c
  @@ -973,6 +973,13 @@ void pause_all_vcpus(void)
   }
   }
   
  +void resume_vcpu(CPUState *cpu)
  +{
  +cpu-stop = false;
  +cpu-stopped = false;
  +qemu_cpu_kick(cpu);
  +}
  +
   void resume_all_vcpus(void)
   {
   CPUArchState *penv = first_cpu;
  @@ -980,9 +987,7 @@ void resume_all_vcpus(void)
   qemu_clock_enable(vm_clock, true);
   while (penv) {
   CPUState *pcpu = ENV_GET_CPU(penv);
  -pcpu-stop = false;
  -pcpu-stopped = false;
  -qemu_cpu_kick(pcpu);
  +resume_vcpu(pcpu);
   penv = penv-next_cpu;
   }
   }
  diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
  index 6502488..9437df5 100644
  --- a/include/sysemu/cpus.h
  +++ b/include/sysemu/cpus.h
  @@ -1,11 +1,14 @@
   #ifndef QEMU_CPUS_H
   #define QEMU_CPUS_H
   
  +#include qom/cpu.h
  +
   /* cpus.c */
   void qemu_init_cpu_loop(void);
   void resume_all_vcpus(void);
   void pause_all_vcpus(void);
   void cpu_stop_current(void);
  +void resume_vcpu(CPUState *cpu);
   
   void cpu_synchronize_all_states(void);
   void cpu_synchronize_all_post_reset(void);
  diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
  index 9c55b34..28fb4f8 100644
  --- a/stubs/Makefile.objs
  +++ b/stubs/Makefile.objs
  @@ -23,3 +23,4 @@ stub-obj-y += sysbus.o
   stub-obj-y += vm-stop.o
   stub-obj-y += vmstate.o
   stub-obj-$(CONFIG_WIN32) += fd-register.o
  +stub-obj-y += resume_vcpu.o
  diff --git a/stubs/resume_vcpu.c b/stubs/resume_vcpu.c
  new file mode 100644
  index 000..383b71a
  --- /dev/null
  +++ b/stubs/resume_vcpu.c
  @@ -0,0 +1,6 @@
  +#include qemu-common.h
  +#include sysemu/cpus.h
  +
  +void resume_vcpu(CPUState *cpu)
  +{
  +}
  diff --git a/target-i386/cpu.c b/target-i386/cpu.c
  index d03ff73..631bcd8 100644
  --- a/target-i386/cpu.c
  +++ b/target-i386/cpu.c
  @@ -2260,6 +2260,11 @@ out:
   error_propagate(errp, local_err);
   return;
   }
  +
  +if (dev-hotplugged) {
  +cpu_synchronize_post_init(env);
  +resume_vcpu(CPU(cpu));
  +}
 
 I think the resume_vcpu should not be here, but in the superclass.

Then I should move following parts to superclass:

if (dev-hotplugged) {
cpu_synchronize_post_init(env);
resume_vcpu(CPU(cpu));
}

because in case of KVM we should make sure that CPU in sane state before
allowing CPU to become run-able.

 Paolo
 
   }
   
   /* Enables contiguous-apic-ID mode, for compatibility */
  
 
 




Re: [Qemu-devel] [PATCH 05/12] target-i386: push hot-plugged VCPU state to KVM and unstop it

2013-03-27 Thread Andreas Färber
Am 27.03.2013 13:12, schrieb Igor Mammedov:
 On Wed, 27 Mar 2013 12:01:20 +0100
 Paolo Bonzini pbonz...@redhat.com wrote:
 
 Il 21/03/2013 15:28, Igor Mammedov ha scritto:
 diff --git a/target-i386/cpu.c b/target-i386/cpu.c
 index d03ff73..631bcd8 100644
 --- a/target-i386/cpu.c
 +++ b/target-i386/cpu.c
 @@ -2260,6 +2260,11 @@ out:
  error_propagate(errp, local_err);
  return;
  }
 +
 +if (dev-hotplugged) {
 +cpu_synchronize_post_init(env);
 +resume_vcpu(CPU(cpu));
 +}

 I think the resume_vcpu should not be here, but in the superclass.
 
 Then I should move following parts to superclass:
 
 if (dev-hotplugged) {
 cpu_synchronize_post_init(env);
 resume_vcpu(CPU(cpu));
 }
 
 because in case of KVM we should make sure that CPU in sane state before
 allowing CPU to become run-able.

That's not possible until we change cpu_synchronize_post_init() argument
to CPUState, which is somewhere down my TODO list. Currently I have
mostly flushed my refactorings out, so if you wanted to dive into that,
that would be appreciated. :)

Andreas

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



Re: [Qemu-devel] [PATCH v2] QOM-ify the TPM support

2013-03-27 Thread Stefan Berger

On 03/27/2013 08:06 AM, Paolo Bonzini wrote:

Il 26/03/2013 17:28, Stefan Berger ha scritto:

QOM-ified the TPM support with much code borrowed from the rng implementation.

What's missing may be that the tpm/tpm_passthrough.c be moved into backends/ .

Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com

---
  v1-v2:
   - followed to git revision e769bdc26

---
  backends/Makefile.objs |2
  backends/tpm.c |  154 
  include/qemu/tpm.h |  170 
+

I think these should be tpm_backend.[ch] (with the include file in
include/tpm).  Can you rename the existing files with that name first?


With the above file naming and directory placement I followed the pattern of

backends/rng.c
include/qemu/rng.h

So are you planning on having them renamed and moved as well?
My intention was to have tpm_passthrough moved into backends/.
There's a file tpm/tpm_backend.c -- you want me to rename this one even 
though its located in a different directory?


Since I am not running a git repository any move/rename would be a 
deletion of a file plus its addition.


   Stefan


Paolo






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

2013-03-27 Thread Luiz Capitulino
On Wed, 27 Mar 2013 14:45:53 +0800
Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:

 Hi, Luiz
   Personally I hope reduce the dynamic allocated buffer which brings
 fragments and unexpected memory grow. Instead, how about sacrifice
 some time to wait output complete, since monitor is not time critical?
 in this case static buffer's size can decide how many work can be
 postponded. Following is my suggestion:

I'd be fine with it if you find a good way to postpone work, but sleeping
on random timers is not a good way.

Also, QEMU allocates fragments of memory in so many places that I doubt
this is an valid argument. You're right that long QMP output can cause
unexpected memory growth, but I expected this to be really rare and if
this really bother us, we can ask monitor_puts() to flush at, say, every
4096 bytes.

 
 --- a/monitor.c
 +++ b/monitor.c
 @@ -293,17 +293,28 @@ static void monitor_puts(Monitor *mon, const char
 *str)
  {
  char c;
 
 +/* if mux do not put in any thing to buffer */
 +if (mon-mux_out) {
 +return;
 +}
 +
  for(;;) {
 -assert(mon-outbuf_index  sizeof(mon-outbuf) - 1);
 +if (mon-outbuf_index = sizeof(mon-outbuf) - 1) {
 +/* when buffer is full, flush it and retry. If buffer is
 bigger, more
 +   work can be postponed. */
 +monitor_flush(mon);
 +usleep(1);
 +continue;
 +}
  c = *str++;
  if (c == '\0')
  break;
  if (c == '\n')
  mon-outbuf[mon-outbuf_index++] = '\r';
  mon-outbuf[mon-outbuf_index++] = c;
 -if (mon-outbuf_index = (sizeof(mon-outbuf) - 1)
 -|| c == '\n')
 +if (c == '\n') {
  monitor_flush(mon);
 +}
  }
  }
 
  Commit f628926bb423fa8a7e0b114511400ea9df38b76a changed monitor_flush()
  to retry on qemu_chr_fe_write() errors. However, the Monitor's output
  buffer can keep growing while the retry is not issued and this can
  cause the buffer to overflow.
  
  To reproduce this issue, just start qemu and type on the Monitor:
  
  (qemu) ?
  
  This will cause the assertion to trig.
  
  To fix this problem this commit makes the Monitor buffer dynamic,
  which means that it can grow as much as needed.
  
  Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
  ---
monitor.c | 42 +-
1 file changed, 25 insertions(+), 17 deletions(-)
  
 




Re: [Qemu-devel] [PATCH v2 00/21] qcow2: Rework cluster allocation even more

2013-03-27 Thread Stefan Hajnoczi
On Tue, Mar 26, 2013 at 05:49:52PM +0100, Kevin Wolf wrote:
 This is the start of the latest Delayed COW series. As there seem to be 
 serious
 problems with the actual Delayed COW (which in fact exist today, but would
 become a bit easier to hit) and I don't have the time to complete this at the
 moment, here is another series with preparatory patches, for which there's no
 reason to not get them applied now.
 
 The result is a somewhat cleaner cluster allocation code in qcow2, and in
 theory some cases should show improved performance - in practice I think these
 cases are unlikely to occur, but as the optimisation falls out naturally, 
 let's
 just take it.
 
 Kevin Wolf (21):
   qemu-iotests: More concurrent allocation scenarios
   qcow2: Fix total clusters number in bdrv_check
   qcow2: Remove bogus unlock of s-lock
   qcow2: Handle dependencies earlier
   qcow2: Improve check for overlapping allocations
   qcow2: Change handle_dependency to byte granularity
   qcow2: Decouple cluster allocation from cluster reuse code
   qcow2: Factor out handle_alloc()
   qcow2: handle_alloc(): Get rid of nb_clusters parameter
   qcow2: handle_alloc(): Get rid of keep_clusters parameter
   qcow2: Finalise interface of handle_alloc()
   qcow2: Clean up handle_alloc()
   qcow2: Factor out handle_copied()
   qcow2: handle_copied(): Get rid of nb_clusters parameter
   qcow2: handle_copied(): Get rid of keep_clusters parameter
   qcow2: handle_copied(): Implement non-zero host_offset
   qcow2: Prepare handle_alloc/copied() for byte granularity
   qcow2: Use byte granularity in qcow2_alloc_cluster_offset()
   qcow2: Allow requests with multiple l2metas
   qcow2: Move cluster gathering to a non-looping loop
   qcow2: Gather clusters in a looping loop
 
  block/qcow2-cluster.c  | 507 
 -
  block/qcow2-refcount.c |   4 +-
  block/qcow2.c  |  16 +-
  block/qcow2.h  |  29 +++
  tests/qemu-iotests/038.out |  10 +-
  tests/qemu-iotests/044.out |   4 +-
  tests/qemu-iotests/046 |  49 -
  tests/qemu-iotests/046.out |  76 +++
  trace-events   |   2 +
  9 files changed, 534 insertions(+), 163 deletions(-)
 
 -- 
 1.8.1.4
 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan



Re: [Qemu-devel] [PATCH v3 0/6] virtio-balloon refactoring.

2013-03-27 Thread Cornelia Huck
On Wed, 27 Mar 2013 10:49:09 +0100
fred.kon...@greensocs.com wrote:

 From: KONRAD Frederic fred.kon...@greensocs.com
 
 This is the next part of virtio-refactoring.
 
 Basically it creates virtio-balloon device which extends virtio-device.
 Then a virtio-balloon can be connected on a virtio-bus.
 virtio-balloon-pci, virtio-balloon-ccw are created too, they extend
 respectively virtio-pci, virtio-ccw-device and have a virtio-balloon.
 
 You can checkout my branch here:
 
 git://project.greensocs.com/qemu-virtio.git virtio-balloon-v3
 
 Note that it is nearly the same series as virtio-blk and virtio-scsi
 refactoring.

Reviewed-by: Cornelia Huck cornelia.h...@de.ibm.com

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

For virtio-ccw on s390:

Tested-by: Cornelia Huck cornelia.h...@de.ibm.com

 
 Changes v2 - v3:
 * Added CCW device.
 * Rebased.
 
 Thanks,
 
 Fred
 
 KONRAD Frederic (6):
   virtio-balloon: add the virtio-balloon device.
   virtio-balloon-pci: switch to the new API.
   virtio-balloon-ccw: switch to the new API.
   virtio-balloon: cleanup: init and exit function.
   virtio-balloon: cleanup: QOM casts.
   virtio-balloon: cleanup: remove qdev field.
 
  hw/s390x/virtio-ccw.c |  25 +++-
  hw/s390x/virtio-ccw.h |  11 +
  hw/virtio-balloon.c   | 110 +++--
  hw/virtio-balloon.h   |   7 +++-
  hw/virtio-pci.c   | 111 
 +-
  hw/virtio-pci.h   |  14 +++
  6 files changed, 170 insertions(+), 108 deletions(-)
 




Re: [Qemu-devel] [RFC 1/4] block: fix I/O throttling accounting blind spot

2013-03-27 Thread Zhi Yong Wu
On Wed, Mar 27, 2013 at 5:14 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
 On Wed, Mar 27, 2013 at 9:50 AM, Zhi Yong Wu zwu.ker...@gmail.com wrote:
 On Thu, Mar 21, 2013 at 10:49 PM, Stefan Hajnoczi stefa...@redhat.com 
 wrote:
 I/O throttling relies on bdrv_acct_done() which is called when a request
 completes.  This leaves a blind spot since we only charge for completed
 requests, not submitted requests.

 For example, if there is 1 operation remaining in this time slice the
 guest could submit 3 operations and they will all be submitted
 successfully since they don't actually get accounted for until they
 complete.

 Originally we probably thought this is okay since the requests will be
 accounted when the time slice is extended.  In practice it causes
 fluctuations since the guest can exceed its I/O limit and it will be
 punished for this later on.

 Account for I/O upon submission so that I/O limits are enforced
 properly.

 Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
 ---
  block.c   | 19 ---
  include/block/block_int.h |  2 +-
  2 files changed, 9 insertions(+), 12 deletions(-)

 diff --git a/block.c b/block.c
 index 0a062c9..31fb0b0 100644
 --- a/block.c
 +++ b/block.c
 @@ -141,7 +141,6 @@ void bdrv_io_limits_disable(BlockDriverState *bs)
  bs-slice_start = 0;
  bs-slice_end   = 0;
  bs-slice_time  = 0;
 -memset(bs-io_base, 0, sizeof(bs-io_base));
 If we try some concussive operations, enable I/O throttling at first,
 then disable it, and then enable it, how about? I guess that
 bs-slice_submitted will maybe be not correct.

 The memset() was moved...

 @@ -3772,11 +3773,7 @@ static bool bdrv_exceed_io_limits(BlockDriverState 
 *bs, int nb_sectors,
  bs-slice_start = now;
  bs-slice_end   = now + bs-slice_time;

 -bs-io_base.bytes[is_write]  = bs-nr_bytes[is_write];
 -bs-io_base.bytes[!is_write] = bs-nr_bytes[!is_write];
 -
 -bs-io_base.ios[is_write]= bs-nr_ops[is_write];
 -bs-io_base.ios[!is_write]   = bs-nr_ops[!is_write];
 +memset(bs-slice_submitted, 0, sizeof(bs-slice_submitted));
  }

  elapsed_time  = now - bs-slice_start;

 ...here.

 Since bs-slice_start = 0 when I/O throttling is disabled we will
 start a new slice next time bdrv_exceed_io_limits() is called.

 Therefore bs-slice_submitted is consistent across disable/enable.
Yes, i also realized this just when i came home by subway.

 Stefan



-- 
Regards,

Zhi Yong Wu



Re: [Qemu-devel] [RFC 0/4] block: fix I/O throttling oscillations

2013-03-27 Thread Zhi Yong Wu
They look good to me, but i have not done any test on my dev box,
thanks for your fix.

On Thu, Mar 21, 2013 at 10:49 PM, Stefan Hajnoczi stefa...@redhat.com wrote:
 Benoît Canet ben...@irqsave.net reported that QEMU I/O throttling can
 oscillate under continuous I/O.  The test case runs 50 threads performing
 random writes and a -drive iops=150 limit is used.

 Since QEMU I/O throttling is implemented using 100 millisecond time slices,
 we'd expect 150 +/- 15 IOPS.  Anything outside that range indicates a problem
 with the I/O throttling algorithm.

 It turned out that even a single thread performing sequential I/O continuously
 is throttled outside the 150 +/- 15 IOPS range.  The continous stream of I/O
 slows down as time goes on but resets to 150 IOPS again when interrupted.  
 This
 can be tested with:

   $ iostat -d 1 -x /dev/vdb 
   $ dd if=/dev/vdb of=/dev/null bs=4096 iflag=direct

 This patches addresses these problems as follows:

 1. Account for I/O requests when they are submitted instead of completed.  
 This
ensures that we do not exceed the budget for this slice.  Exceeding the
budget leads to fluctuations since we have to make up for this later.

 2. Use constant 100 millisecond slice time.  Adjusting the slice time at
run-time led to oscillations.  Since the reason for adjusting slice time is
not clear, drop this behavior.

 I have also included two code clean-up patches.

 Benoît: Please let me know if this solves the problems you're seeing.

 Stefan Hajnoczi (4):
   block: fix I/O throttling accounting blind spot
   block: keep I/O throttling slice time constant
   block: drop duplicated slice extension code
   block: clean up I/O throttling wait_time code

  block.c   | 47 
 ---
  blockdev.c|  1 -
  include/block/block_int.h |  3 +--
  3 files changed, 21 insertions(+), 30 deletions(-)

 --
 1.8.1.4





-- 
Regards,

Zhi Yong Wu



Re: [Qemu-devel] [PATCH 11/12 v3] qmp: add cpu-add qmp command

2013-03-27 Thread Luiz Capitulino
On Tue, 26 Mar 2013 17:47:42 +0100
Igor Mammedov imamm...@redhat.com wrote:

 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
 v3:
   * it appears that 'online/offline' in cpu-set are confusing people
 with what command actually does and users might have to distinguish
 if 'offline' is not implemented by parsing error message. To simplify
 things replace cpu-set with cpu-add command to show more clear what
 command does and just add cpu-del when CPU remove is implemented.
 
 v2:
   * s/cpu_set/cpu-set/
   * qmp doc style fix
   * use bool type instead of opencodding online/offline string
  suggested-by: Eric Blake ebl...@redhat.com
 ---
  include/sysemu/sysemu.h |  2 ++
  qapi-schema.json| 11 +++
  qmp-commands.hx | 23 +++
  qmp.c   |  5 +
  stubs/Makefile.objs |  1 +
  stubs/do_cpu_hot_add.c  |  7 +++
  6 files changed, 49 insertions(+)
  create mode 100644 stubs/do_cpu_hot_add.c

Personally, I'd prefer this patch squashed into the next one, but anyway:

Acked-by: Luiz Capitulino lcapitul...@redhat.com

 
 diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
 index 4b8f721..8bcaf26 100644
 --- a/include/sysemu/sysemu.h
 +++ b/include/sysemu/sysemu.h
 @@ -156,6 +156,8 @@ void drive_hot_add(Monitor *mon, const QDict *qdict);
  void qemu_register_cpu_add_notifier(Notifier *notifier);
  void qemu_system_cpu_hotplug_request(uint32_t id);
  
 +void do_cpu_hot_add(const int64_t id, Error **errp);
 +
  /* pcie aer error injection */
  void pcie_aer_inject_error_print(Monitor *mon, const QObject *data);
  int do_pcie_aer_inject_error(Monitor *mon,
 diff --git a/qapi-schema.json b/qapi-schema.json
 index af499bd..3a2f273 100644
 --- a/qapi-schema.json
 +++ b/qapi-schema.json
 @@ -1385,6 +1385,17 @@
  { 'command': 'cpu', 'data': {'index': 'int'} }
  
  ##
 +# @cpu-add
 +#
 +# Adds CPU with specified id
 +#
 +# @id: cpu id of CPU to be created
 +#
 +# Returns: Nothing on success
 +##
 +{ 'command': 'cpu-add', 'data': {'id': 'int'} }
 +
 +##
  # @memsave:
  #
  # Save a portion of guest memory to a file.
 diff --git a/qmp-commands.hx b/qmp-commands.hx
 index 2051fcb..4876393 100644
 --- a/qmp-commands.hx
 +++ b/qmp-commands.hx
 @@ -407,6 +407,29 @@ Example:
  EQMP
  
  {
 +.name   = cpu-add,
 +.args_type  = id:i,
 +.mhandler.cmd_new = qmp_marshal_input_cpu_add,
 +},
 +
 +SQMP
 +cpu-add
 +---
 +
 +Adds virtual cpu
 +
 +Arguments:
 +
 +- id: cpu id (json-int)
 +
 +Example:
 +
 +- { execute: cpu-add, arguments: { id: 2 } }
 +- { return: {} }
 +
 +EQMP
 +
 +{
  .name   = memsave,
  .args_type  = val:l,size:i,filename:s,cpu:i?,
  .mhandler.cmd_new = qmp_marshal_input_memsave,
 diff --git a/qmp.c b/qmp.c
 index 55b056b..978d956 100644
 --- a/qmp.c
 +++ b/qmp.c
 @@ -108,6 +108,11 @@ void qmp_cpu(int64_t index, Error **errp)
  /* Just do nothing */
  }
  
 +void qmp_cpu_add(int64_t id, Error **errp)
 +{
 +do_cpu_hot_add(id, errp);
 +}
 +
  #ifndef CONFIG_VNC
  /* If VNC support is enabled, the true query-vnc command is
 defined in the VNC subsystem */
 diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
 index 6a492f5..4154a2b 100644
 --- a/stubs/Makefile.objs
 +++ b/stubs/Makefile.objs
 @@ -26,3 +26,4 @@ stub-obj-$(CONFIG_WIN32) += fd-register.o
  stub-obj-y += resume_vcpu.o
  stub-obj-y += get_icc_bus.o
  stub-obj-y += qemu_system_cpu_hotplug_request.o
 +stub-obj-y += do_cpu_hot_add.o
 diff --git a/stubs/do_cpu_hot_add.c b/stubs/do_cpu_hot_add.c
 new file mode 100644
 index 000..1f6d7a6
 --- /dev/null
 +++ b/stubs/do_cpu_hot_add.c
 @@ -0,0 +1,7 @@
 +#include qapi/error.h
 +#include sysemu/sysemu.h
 +
 +void do_cpu_hot_add(const int64_t id, Error **errp)
 +{
 +error_setg(errp, Not implemented);
 +}




Re: [Qemu-devel] [PATCH 0/4] monitor: do not rely on O_NONBLOCK for passed file descriptors

2013-03-27 Thread Juan Quintela
Stefan Hajnoczi stefa...@redhat.com wrote:
 On Wed, Mar 27, 2013 at 04:37:52PM +0800, liu ping fan wrote:
 On Wed, Mar 27, 2013 at 12:07 AM, Stefan Hajnoczi stefa...@redhat.com 
 wrote:
  There are several places where QEMU accidentally relies on the
  O_NONBLOCK state
  of passed file descriptors.  Exposing O_NONBLOCK state makes it
  part of the QMP
 
 If in future, we push more backend on their dedicated thread, will the
 related fd be block?

 This series is not related to threading in QEMU.  The convention it
 establishes is that passed fds are blocking.  If QEMU wants to use
 
You mean here non-blocking

 nonblocking it must call qemu_set_block(fd).  This works whether it is
   ^^
or here qemu_set_nonblock(fd)

no?

I guess the second one O:-)

Later, Juan.

 done from a traditional QEMU thread or a data plane thread.

 Stefan



[Qemu-devel] QCOW2 deduplication key value store

2013-03-27 Thread Benoît Canet

Hello,

I am starting this thread so we can discuss of the choice of a good key/value
store for the QCOW2 deduplication.

One of the main goal is to keep the ratio between the number of cluster written
and the number of dedup metadata io high.

I initially though about taking the first two stages of SILT
(http://goo.gl/x7UcI) to build the key value store.

The very nice thing about the two first stage of SILT is that the write io
involved are not random at all: The log is obviously not random and the
conversion into the second stage hash table can be done sequentially.
This makes the io ratio very high.

Kevin later thought about combining the first stage of SILT with a big hash
table as a second stage.
While this method is simpler I don't understand how we can inject the first
stage into this big table without generating a log of random ios.

Best regards

Benoît



Re: [Qemu-devel] [PATCH] VMXNET3: initialize rx_ridx to eliminate compile warning

2013-03-27 Thread Dmitry Fleytman
Hi, Wenchao

Thanks for fixing this!

Dmitry.


On Tue, Mar 26, 2013 at 4:24 AM, Wenchao Xia xiaw...@linux.vnet.ibm.comwrote:

   Gcc report hw/vmxnet3.c:972: error: ‘rx_ridx’ may be used
 uninitialized in this function, so fix it.

 Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
 ---
  hw/vmxnet3.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

 diff --git a/hw/vmxnet3.c b/hw/vmxnet3.c
 index 925be80..bdd256e 100644
 --- a/hw/vmxnet3.c
 +++ b/hw/vmxnet3.c
 @@ -969,7 +969,7 @@ vmxnet3_indicate_packet(VMXNET3State *s)
  struct Vmxnet3_RxDesc rxd;
  bool is_head = true;
  uint32_t rxd_idx;
 -uint32_t rx_ridx;
 +uint32_t rx_ridx = 0;

  struct Vmxnet3_RxCompDesc rxcd;
  uint32_t new_rxcd_gen = VMXNET3_INIT_GEN;
 --
 1.7.1





Re: [Qemu-devel] [PATCH v2 0/4] monitor: do not rely on O_NONBLOCK for passed file descriptors

2013-03-27 Thread Luiz Capitulino
On Wed, 27 Mar 2013 10:10:42 +0100
Stefan Hajnoczi stefa...@redhat.com wrote:

 There are several places where QEMU accidentally relies on the O_NONBLOCK 
 state
 of passed file descriptors.  Exposing O_NONBLOCK state makes it part of the 
 QMP
 API whenever getfd or fdset_add_fd are used!
 
 Whether or not QEMU will use O_NONBLOCK is an implementation detail and should
 be hidden from QMP clients.
 
 This patch series addresses this in 3 steps:

Nice series:

Applied to the qmp branch, thanks.

 
 1. Fix callers of monitor_handle_fd_param(), monitor_fdset_get_fd(), and
monitor_get_fd() that depend on O_NONBLOCK being set.  Luckily there are
only two instances and they are fixed in Patches 1  2.
 
 2. Rename socket_set_nonblock() to qemu_set_nonblock() just like
qemu_set_cloexec().  This makes code cleaner when working with arbitrary
file descriptors that may not be sockets.  See Patch 3.
 
 3. Clear O_NONBLOCK when a chardev receives file descriptors.  From now on 
 QEMU
can assume that passed file descriptors are in blocking mode.  Simply use
qemu_set_nonblock(fd) if you want to enable O_NONBLOCK.  See Patch 4.
 
 This fixes live migration with recent libvirt.  Libvirt checks if QEMU 
 supports
 file descriptor passing and, if yes, hands QEMU a socket with O_NONBLOCK set.
 The migrate fd:foo code assumes the socket is in blocking mode.  The result
 is a corrupted migration stream.  For more info on this bug, see:
 
 https://bugzilla.redhat.com/show_bug.cgi?id=923124
 
 Note that Michal Privoznik mpriv...@redhat.com also sent a libvirt patch so
 that old QEMUs work with new libvirts:
 
 https://www.redhat.com/archives/libvir-list/2013-March/msg01486.html
 
 My patch series fixes the QMP API and allows old libvirts to work again with
 new QEMUs.
 
 v2:
  * Rename socket_set_nonblock() in Patch 1 to avoid code churn [eblake]
  * Avoid qemu_set_block(-1) calls that clobber errno [quintela]
 
 Stefan Hajnoczi (4):
   oslib-posix: rename socket_set_nonblock() to qemu_set_nonblock()
   net: ensure socket backend uses non-blocking fds
   qemu-socket: set passed fd non-blocking in socket_connect()
   chardev: clear O_NONBLOCK on SCM_RIGHTS file descriptors
 
  block/nbd.c|  2 +-
  block/sheepdog.c   |  2 +-
  include/qemu/sockets.h |  4 ++--
  migration.c|  2 +-
  nbd.c  |  8 
  net/socket.c   | 13 +
  qemu-char.c| 11 +++
  savevm.c   |  2 +-
  slirp/misc.c   |  2 +-
  slirp/tcp_subr.c   |  4 ++--
  ui/vnc.c   |  2 +-
  util/oslib-posix.c |  4 ++--
  util/oslib-win32.c |  4 ++--
  util/qemu-sockets.c|  5 +++--
  14 files changed, 37 insertions(+), 28 deletions(-)
 




Re: [Qemu-devel] [PATCH 05/12] target-i386: push hot-plugged VCPU state to KVM and unstop it

2013-03-27 Thread Igor Mammedov
On Wed, 27 Mar 2013 13:17:45 +0100
Andreas Färber afaer...@suse.de wrote:
  Then I should move following parts to superclass:
  
  if (dev-hotplugged) {
  cpu_synchronize_post_init(env);
  resume_vcpu(CPU(cpu));
  }
  
  because in case of KVM we should make sure that CPU in sane state before
  allowing CPU to become run-able.
 
 That's not possible until we change cpu_synchronize_post_init() argument
 to CPUState, which is somewhere down my TODO list. Currently I have
 mostly flushed my refactorings out, so if you wanted to dive into that,
 that would be appreciated. :)
 
 Andreas

Would something like this be acceptable?
---
From 15502168009b7b5ae2b46d854c461e5ad031cdc6 Mon Sep 17 00:00:00 2001
From: Igor Mammedov imamm...@redhat.com
Date: Wed, 27 Mar 2013 14:21:20 +0100
Subject: [PATCH] cpu: Pass CPUState to *cpu_synchronize_post*()

Signed-off-by: Igor Mammedov imamm...@redhat.com
---
 cpus.c   |  4 ++--
 include/sysemu/kvm.h | 12 ++--
 kvm-all.c|  8 ++--
 kvm-stub.c   |  4 ++--
 4 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/cpus.c b/cpus.c
index e919dd7..9b9a32f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -419,7 +419,7 @@ void cpu_synchronize_all_post_reset(void)
 CPUArchState *cpu;
 
 for (cpu = first_cpu; cpu; cpu = cpu-next_cpu) {
-cpu_synchronize_post_reset(cpu);
+cpu_synchronize_post_reset(ENV_GET_CPU(cpu));
 }
 }
 
@@ -428,7 +428,7 @@ void cpu_synchronize_all_post_init(void)
 CPUArchState *cpu;
 
 for (cpu = first_cpu; cpu; cpu = cpu-next_cpu) {
-cpu_synchronize_post_init(cpu);
+cpu_synchronize_post_init(ENV_GET_CPU(cpu));
 }
 }
 
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index f2d97b5..495e6f8 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -250,8 +250,8 @@ int kvm_check_extension(KVMState *s, unsigned int 
extension);
 uint32_t kvm_arch_get_supported_cpuid(KVMState *env, uint32_t function,
   uint32_t index, int reg);
 void kvm_cpu_synchronize_state(CPUArchState *env);
-void kvm_cpu_synchronize_post_reset(CPUArchState *env);
-void kvm_cpu_synchronize_post_init(CPUArchState *env);
+void kvm_cpu_synchronize_post_reset(CPUState *cpu);
+void kvm_cpu_synchronize_post_init(CPUState *cpu);
 
 /* generic hooks - to be moved/refactored once there are more users */
 
@@ -262,17 +262,17 @@ static inline void cpu_synchronize_state(CPUArchState 
*env)
 }
 }
 
-static inline void cpu_synchronize_post_reset(CPUArchState *env)
+static inline void cpu_synchronize_post_reset(CPUState *cpu)
 {
 if (kvm_enabled()) {
-kvm_cpu_synchronize_post_reset(env);
+kvm_cpu_synchronize_post_reset(cpu);
 }
 }
 
-static inline void cpu_synchronize_post_init(CPUArchState *env)
+static inline void cpu_synchronize_post_init(CPUState *cpu)
 {
 if (kvm_enabled()) {
-kvm_cpu_synchronize_post_init(env);
+kvm_cpu_synchronize_post_init(cpu);
 }
 }
 
diff --git a/kvm-all.c b/kvm-all.c
index 9b433d3..fc4e17c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1510,18 +1510,14 @@ void kvm_cpu_synchronize_state(CPUArchState *env)
 }
 }
 
-void kvm_cpu_synchronize_post_reset(CPUArchState *env)
+void kvm_cpu_synchronize_post_reset(CPUState *cpu)
 {
-CPUState *cpu = ENV_GET_CPU(env);
-
 kvm_arch_put_registers(cpu, KVM_PUT_RESET_STATE);
 cpu-kvm_vcpu_dirty = false;
 }
 
-void kvm_cpu_synchronize_post_init(CPUArchState *env)
+void kvm_cpu_synchronize_post_init(CPUState *cpu)
 {
-CPUState *cpu = ENV_GET_CPU(env);
-
 kvm_arch_put_registers(cpu, KVM_PUT_FULL_STATE);
 cpu-kvm_vcpu_dirty = false;
 }
diff --git a/kvm-stub.c b/kvm-stub.c
index 760aadc..82875dd 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -42,11 +42,11 @@ void kvm_cpu_synchronize_state(CPUArchState *env)
 {
 }
 
-void kvm_cpu_synchronize_post_reset(CPUArchState *env)
+void kvm_cpu_synchronize_post_reset(CPUState *env)
 {
 }
 
-void kvm_cpu_synchronize_post_init(CPUArchState *env)
+void kvm_cpu_synchronize_post_init(CPUState *cpu)
 {
 }
 
-- 
1.7.11.7






[Qemu-devel] [PATCH] PPC: init_excp_7x0: fix hreset entry point.

2013-03-27 Thread Fabien Chouteau
From: gingold gingold@020d506d-db78-4e55-b2a8-6c851f84c332

According to the PowePC 750 user's manual, the vector offset for system
reset (both /HRESET and /SRESET) is 0x00100.

Signed-off-by: Fabien Chouteau chout...@adacore.com
---
 target-ppc/translate_init.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 781170f..a5bae1e 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -2885,7 +2885,7 @@ static void init_excp_7x0 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 env-hreset_excp_prefix = 0xUL;
 /* Hardware reset vector */
-env-hreset_vector = 0xFFFCUL;
+env-hreset_vector = 0xFFF00100UL;
 #endif
 }
 
@@ -2931,7 +2931,7 @@ static void init_excp_750cx (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 env-hreset_excp_prefix = 0xUL;
 /* Hardware reset vector */
-env-hreset_vector = 0xFFFCUL;
+env-hreset_vector = 0xFFF00100UL;
 #endif
 }
 
@@ -2959,7 +2959,7 @@ static void init_excp_7x5 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 env-hreset_excp_prefix = 0xUL;
 /* Hardware reset vector */
-env-hreset_vector = 0xFFFCUL;
+env-hreset_vector = 0xFFF00100UL;
 #endif
 }
 
@@ -2985,7 +2985,7 @@ static void init_excp_7400 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 env-hreset_excp_prefix = 0xUL;
 /* Hardware reset vector */
-env-hreset_vector = 0xFFFCUL;
+env-hreset_vector = 0xFFF00100UL;
 #endif
 }
 
-- 
1.7.9.5




Re: [Qemu-devel] dataplane bug: fail to start Windows VM with dataplane enable

2013-03-27 Thread Stefan Hajnoczi
On Tue, Mar 26, 2013 at 11:10:35PM +0800, 张磊强 wrote:
 Hi,  Paolo  Stefan:
 
 When I test the dataplane feature with qemu master, I find that
 Windows (Windows 7 and Windows 2003) VM will hang if dataplane is
 enabled. But if I try to start a Fedora VM, it can start normally.
 
 The command I boot QEMU is:
 x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 1024 -smp 2 -drive
 file=win7.img,if=none,id=drive-virtio-disk,format=raw,cache=none,aio=native
 -device 
 virtio-blk-pci,config-wce=off,scsi=off,x-data-plane=on,drive=drive-virtio-disk,id=virtio-disk
 
 I found the similar bug has reported some days ago:
 http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg02200.html
 . And a patch for this bug has already committed by Paolo at Mar 13:
 http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg02200.html
 .
 
 But it cannot work under my environment. Could you give me some advise
 to debug this problem ? I can provide more information if need.

Hi,
I haven't gotten to the bottom of it yet but wanted to let you know that
I'm seeing a hang at the boot screen after the installer reboots.  The
Windows logo animation runs but the guest seems unable to make progress.

Will let you know when there is a fix.  The guest I'm testing is Windows
7 Professional 64-bit.

As a workaround you can set x-data-plane=off to boot the guest for the
first time.  Subsequent boots will succeed with x-data-plane=on.

Stefan



Re: [Qemu-devel] [PATCH] PPC: init_excp_7x0: fix hreset entry point.

2013-03-27 Thread Alexander Graf

On 27.03.2013, at 14:32, Fabien Chouteau wrote:

 From: gingold gingold@020d506d-db78-4e55-b2a8-6c851f84c332

What is this? :)


Alex

 
 According to the PowePC 750 user's manual, the vector offset for system
 reset (both /HRESET and /SRESET) is 0x00100.
 
 Signed-off-by: Fabien Chouteau chout...@adacore.com
 ---
 target-ppc/translate_init.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)
 
 diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
 index 781170f..a5bae1e 100644
 --- a/target-ppc/translate_init.c
 +++ b/target-ppc/translate_init.c
 @@ -2885,7 +2885,7 @@ static void init_excp_7x0 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 env-hreset_excp_prefix = 0xUL;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0xFFF00100UL;
 #endif
 }
 
 @@ -2931,7 +2931,7 @@ static void init_excp_750cx (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 env-hreset_excp_prefix = 0xUL;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0xFFF00100UL;
 #endif
 }
 
 @@ -2959,7 +2959,7 @@ static void init_excp_7x5 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 env-hreset_excp_prefix = 0xUL;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0xFFF00100UL;
 #endif
 }
 
 @@ -2985,7 +2985,7 @@ static void init_excp_7400 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 env-hreset_excp_prefix = 0xUL;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0xFFF00100UL;
 #endif
 }
 
 -- 
 1.7.9.5
 




[Qemu-devel] [PATCH] compiler: fix warning with GCC 4.8.0

2013-03-27 Thread Paolo Bonzini
GCC 4.8.0 introduces a new warning:

block/qcow2-snapshot.c: In function 'qcow2_write_snapshots’:
block/qcow2-snapshot.c:252:18: error: typedef 'qemu_build_bug_on__253'
  locally defined but not used [-Werror=unused-local-typedefs]
 QEMU_BUILD_BUG_ON(offsetof(QCowHeader, snapshots_offset) !=
  ^
cc1: all warnings being treated as errors

(Caret diagnostics aren't perfect yet with macros... :)) Work around it
with __attribute__((unused)).

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
 include/qemu/compiler.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 2f7998b..6175c24 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -33,7 +33,7 @@
 #define cat(x,y) x ## y
 #define cat2(x,y) cat(x,y)
 #define QEMU_BUILD_BUG_ON(x) \
-typedef char cat2(qemu_build_bug_on__,__LINE__)[(x)?-1:1];
+typedef char cat2(qemu_build_bug_on__,__LINE__)[(x)?-1:1] 
__attribute__((unused));
 
 #if defined __GNUC__
 # if !QEMU_GNUC_PREREQ(4, 4)
-- 
1.8.1.4




Re: [Qemu-devel] [PATCH] PPC: init_excp_7x0: fix hreset entry point.

2013-03-27 Thread Fabien Chouteau
On 03/27/2013 02:34 PM, Alexander Graf wrote:
 
 On 27.03.2013, at 14:32, Fabien Chouteau wrote:
 
 From: gingold gingold@020d506d-db78-4e55-b2a8-6c851f84c332
 
 What is this? :)
 

Oups :) Tristan Gingold is a colleague of mine. I'll fix this.

-- 
Fabien Chouteau



Re: [Qemu-devel] [PATCH 0/4] monitor: do not rely on O_NONBLOCK for passed file descriptors

2013-03-27 Thread Stefan Hajnoczi
On Wed, Mar 27, 2013 at 02:06:36PM +0100, Juan Quintela wrote:
 Stefan Hajnoczi stefa...@redhat.com wrote:
  On Wed, Mar 27, 2013 at 04:37:52PM +0800, liu ping fan wrote:
  On Wed, Mar 27, 2013 at 12:07 AM, Stefan Hajnoczi stefa...@redhat.com 
  wrote:
   There are several places where QEMU accidentally relies on the
   O_NONBLOCK state
   of passed file descriptors.  Exposing O_NONBLOCK state makes it
   part of the QMP
  
  If in future, we push more backend on their dedicated thread, will the
  related fd be block?
 
  This series is not related to threading in QEMU.  The convention it
  establishes is that passed fds are blocking.  If QEMU wants to use
  
 You mean here non-blocking
 
  nonblocking it must call qemu_set_block(fd).  This works whether it is
^^
 or here qemu_set_nonblock(fd)
 
 no?
 
 I guess the second one O:-)

Blocking by default, call qemu_set_nonblock(fd) if you need
non-blocking.

:)

Sorry for the mistake.

Stefan



[Qemu-devel] [PATCH] PPC: init_excp_7x0: fix hreset entry point.

2013-03-27 Thread Fabien Chouteau
According to the PowePC 750 user's manual, the vector offset for system
reset (both /HRESET and /SRESET) is 0x00100.

Signed-off-by: Fabien Chouteau chout...@adacore.com
---
 target-ppc/translate_init.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 781170f..a5bae1e 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -2885,7 +2885,7 @@ static void init_excp_7x0 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 env-hreset_excp_prefix = 0xUL;
 /* Hardware reset vector */
-env-hreset_vector = 0xFFFCUL;
+env-hreset_vector = 0xFFF00100UL;
 #endif
 }
 
@@ -2931,7 +2931,7 @@ static void init_excp_750cx (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 env-hreset_excp_prefix = 0xUL;
 /* Hardware reset vector */
-env-hreset_vector = 0xFFFCUL;
+env-hreset_vector = 0xFFF00100UL;
 #endif
 }
 
@@ -2959,7 +2959,7 @@ static void init_excp_7x5 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 env-hreset_excp_prefix = 0xUL;
 /* Hardware reset vector */
-env-hreset_vector = 0xFFFCUL;
+env-hreset_vector = 0xFFF00100UL;
 #endif
 }
 
@@ -2985,7 +2985,7 @@ static void init_excp_7400 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 env-hreset_excp_prefix = 0xUL;
 /* Hardware reset vector */
-env-hreset_vector = 0xFFFCUL;
+env-hreset_vector = 0xFFF00100UL;
 #endif
 }
 
-- 
1.7.9.5




Re: [Qemu-devel] [PATCH] PPC: init_excp_7x0: fix hreset entry point.

2013-03-27 Thread Alexander Graf

On 27.03.2013, at 14:50, Fabien Chouteau wrote:

 According to the PowePC 750 user's manual, the vector offset for system

PowerPC?

 reset (both /HRESET and /SRESET) is 0x00100.
 
 Signed-off-by: Fabien Chouteau chout...@adacore.com
 ---
 target-ppc/translate_init.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)
 
 diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
 index 781170f..a5bae1e 100644
 --- a/target-ppc/translate_init.c
 +++ b/target-ppc/translate_init.c
 @@ -2885,7 +2885,7 @@ static void init_excp_7x0 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 env-hreset_excp_prefix = 0xUL;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0xFFF00100UL;

As you properly explained above, the reset vector is 0x100 according to the 
spec. However, hreset_excp_prefix is 0x0. How do we end up getting to 
0xfff00100 here?


Alex

 #endif
 }
 
 @@ -2931,7 +2931,7 @@ static void init_excp_750cx (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 env-hreset_excp_prefix = 0xUL;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0xFFF00100UL;
 #endif
 }
 
 @@ -2959,7 +2959,7 @@ static void init_excp_7x5 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 env-hreset_excp_prefix = 0xUL;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0xFFF00100UL;
 #endif
 }
 
 @@ -2985,7 +2985,7 @@ static void init_excp_7400 (CPUPPCState *env)
 env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
 env-hreset_excp_prefix = 0xUL;
 /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0xFFF00100UL;
 #endif
 }
 
 -- 
 1.7.9.5
 




Re: [Qemu-devel] [Qemu-ppc] [PATCH] PPC: init_excp_7x0: fix hreset entry point.

2013-03-27 Thread Alexander Graf

On 27.03.2013, at 14:54, Alexander Graf wrote:

 
 On 27.03.2013, at 14:50, Fabien Chouteau wrote:
 
 According to the PowePC 750 user's manual, the vector offset for system
 
 PowerPC?
 
 reset (both /HRESET and /SRESET) is 0x00100.
 
 Signed-off-by: Fabien Chouteau chout...@adacore.com
 ---
 target-ppc/translate_init.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)
 
 diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
 index 781170f..a5bae1e 100644
 --- a/target-ppc/translate_init.c
 +++ b/target-ppc/translate_init.c
 @@ -2885,7 +2885,7 @@ static void init_excp_7x0 (CPUPPCState *env)
env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
env-hreset_excp_prefix = 0xUL;
/* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0xFFF00100UL;
 
 As you properly explained above, the reset vector is 0x100 according to the 
 spec. However, hreset_excp_prefix is 0x0. How do we end up getting to 
 0xfff00100 here?

According to 7xx_um.pdf (740 / 750 User manual), the IP bit in MSR (bit 25 in 
ppc notion) controls whether excp_prefix is 0xfff0 or 0x. The spec 
also says:

When either HRESET is negated or SRESET transitions to asserted, the processor 
attempts to fetch code from the system reset exception vector. The vector is 
located at offset 0x00100 from the exception prefix (all zeros or ones, 
depending on the setting of the exception prefix bit in the machine state 
register (MSR[IP]). The MSR[IP] bit is set for HRESET.

So on reset, MSR[IP] = 1. That means that hreset_excp_prefix is also wrong here.

Please add the respective logic that sets hreset_excp_prefix according to 
MSR[IP] on 740 / 750, otherwise whatever you're trying to execute will break as 
soon as it gets its first real exception :).


Alex




Re: [Qemu-devel] dataplane bug: fail to start Windows VM with dataplane enable

2013-03-27 Thread Stefan Hajnoczi
On Wed, Mar 27, 2013 at 02:32:19PM +0100, Stefan Hajnoczi wrote:
 On Tue, Mar 26, 2013 at 11:10:35PM +0800, 张磊强 wrote:
  Hi,  Paolo  Stefan:
  
  When I test the dataplane feature with qemu master, I find that
  Windows (Windows 7 and Windows 2003) VM will hang if dataplane is
  enabled. But if I try to start a Fedora VM, it can start normally.
  
  The command I boot QEMU is:
  x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 1024 -smp 2 -drive
  file=win7.img,if=none,id=drive-virtio-disk,format=raw,cache=none,aio=native
  -device 
  virtio-blk-pci,config-wce=off,scsi=off,x-data-plane=on,drive=drive-virtio-disk,id=virtio-disk
  
  I found the similar bug has reported some days ago:
  http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg02200.html
  . And a patch for this bug has already committed by Paolo at Mar 13:
  http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg02200.html
  .
  
  But it cannot work under my environment. Could you give me some advise
  to debug this problem ? I can provide more information if need.
 
 Hi,
 I haven't gotten to the bottom of it yet but wanted to let you know that
 I'm seeing a hang at the boot screen after the installer reboots.  The
 Windows logo animation runs but the guest seems unable to make progress.
 
 Will let you know when there is a fix.  The guest I'm testing is Windows
 7 Professional 64-bit.
 
 As a workaround you can set x-data-plane=off to boot the guest for the
 first time.  Subsequent boots will succeed with x-data-plane=on.

Next data point, it's not caused by the Paolo's AioContext conversion.
This means it's unrelated to the recent bug that you mentioned.

The boot gets stuck immediately after the switch from the BIOS
virtio-blk driver to the Windows viostor driver.  We get two requests
(VIRTIO_BLK_T_GET_ID and VIRTIO_BLK_T_READ) and then the guest stops.
The descriptor index of the second request has been incremented, perhaps
the guest is not receiving host-guest notifies.

Stefan



Re: [Qemu-devel] [Qemu-ppc] [PATCH] PPC: init_excp_7x0: fix hreset entry point.

2013-03-27 Thread Alexander Graf

On 27.03.2013, at 15:00, Alexander Graf wrote:

 
 On 27.03.2013, at 14:54, Alexander Graf wrote:
 
 
 On 27.03.2013, at 14:50, Fabien Chouteau wrote:
 
 According to the PowePC 750 user's manual, the vector offset for system
 
 PowerPC?
 
 reset (both /HRESET and /SRESET) is 0x00100.
 
 Signed-off-by: Fabien Chouteau chout...@adacore.com
 ---
 target-ppc/translate_init.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)
 
 diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
 index 781170f..a5bae1e 100644
 --- a/target-ppc/translate_init.c
 +++ b/target-ppc/translate_init.c
 @@ -2885,7 +2885,7 @@ static void init_excp_7x0 (CPUPPCState *env)
   env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
   env-hreset_excp_prefix = 0xUL;
   /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0xFFF00100UL;
 
 As you properly explained above, the reset vector is 0x100 according to the 
 spec. However, hreset_excp_prefix is 0x0. How do we end up getting to 
 0xfff00100 here?
 
 According to 7xx_um.pdf (740 / 750 User manual), the IP bit in MSR (bit 25 in 
 ppc notion) controls whether excp_prefix is 0xfff0 or 0x. The 
 spec also says:
 
 When either HRESET is negated or SRESET transitions to asserted, the 
 processor attempts to fetch code from the system reset exception vector. The 
 vector is located at offset 0x00100 from the exception prefix (all zeros or 
 ones, depending on the setting of the exception prefix bit in the machine 
 state register (MSR[IP]). The MSR[IP] bit is set for HRESET.
 
 So on reset, MSR[IP] = 1. That means that hreset_excp_prefix is also wrong 
 here.
 
 Please add the respective logic that sets hreset_excp_prefix according to 
 MSR[IP] on 740 / 750, otherwise whatever you're trying to execute will break 
 as soon as it gets its first real exception :).

While at it, on exception delivery ILE, ME and IP do not get modified according 
to the spec. Please verify that we don't accidentally set them to 0 when we 
deliver an interrupt. Except for machine check interrupts, where MSR.ME = 0.

Also, MSR.LE becomes the previous value of MSR.ILE. Not that we'd implement LE 
mode properly ;).


Alex




[Qemu-devel] [PATCH] chardev-frontends: Explicitly check, inc and dec avail_connections

2013-03-27 Thread Hans de Goede
chardev-frontends need to explictly check, increase and decrement the
avail_connections property of the chardev when they are not using a
qdev-chardev-property for the chardev.

This fixes things like:
qemu-kvm -chardev stdio,id=foo -device isa-serial,chardev=foo \
  -mon chardev=foo

Working, where they should fail. Most of the changes here are due to
old hardware emulation code which is using serial_hds directly rather then
a qdev-chardev-property.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 backends/rng-egd.c |  7 +++
 gdbstub.c  |  1 +
 hw/arm/pxa2xx.c|  9 -
 hw/bt-hci-csr.c|  1 +
 hw/ipoctal232.c|  1 +
 hw/ivshmem.c   |  1 +
 hw/mcf_uart.c  |  6 ++
 hw/serial.c| 16 
 hw/serial.h|  1 +
 hw/sh_serial.c |  9 -
 hw/xen_console.c   | 19 +++
 net/slirp.c|  1 +
 qemu-char.c| 14 +-
 vl.c   |  7 +++
 14 files changed, 86 insertions(+), 7 deletions(-)

diff --git a/backends/rng-egd.c b/backends/rng-egd.c
index 5e012e9..d8e9d63 100644
--- a/backends/rng-egd.c
+++ b/backends/rng-egd.c
@@ -149,6 +149,12 @@ static void rng_egd_opened(RngBackend *b, Error **errp)
 return;
 }
 
+if (s-chr-avail_connections  1) {
+error_set(errp, QERR_DEVICE_IN_USE, s-chr_name);
+return;
+}
+s-chr-avail_connections--;
+
 /* FIXME we should resubmit pending requests when the CDS reconnects. */
 qemu_chr_add_handlers(s-chr, rng_egd_chr_can_read, rng_egd_chr_read,
   NULL, s);
@@ -191,6 +197,7 @@ static void rng_egd_finalize(Object *obj)
 
 if (s-chr) {
 qemu_chr_add_handlers(s-chr, NULL, NULL, NULL, NULL);
+s-chr-avail_connections++;
 }
 
 g_free(s-chr_name);
diff --git a/gdbstub.c b/gdbstub.c
index a666cb5..83267e0 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -3025,6 +3025,7 @@ int gdbserver_start(const char *device)
 if (!chr)
 return -1;
 
+chr-avail_connections--;
 qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive,
   gdb_chr_event, NULL);
 }
diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index 7467cca..df4b458 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -1981,9 +1981,16 @@ static PXA2xxFIrState *pxa2xx_fir_init(MemoryRegion 
*sysmem,
 memory_region_init_io(s-iomem, pxa2xx_fir_ops, s, pxa2xx-fir, 0x1000);
 memory_region_add_subregion(sysmem, base, s-iomem);
 
-if (chr)
+if (chr) {
+if (chr-avail_connections  1) {
+fprintf(stderr, pxa2xx_fir_init error chardev %s already used\n,
+chr-label);
+exit(1);
+}
+chr-avail_connections--;
 qemu_chr_add_handlers(chr, pxa2xx_fir_is_empty,
 pxa2xx_fir_rx, pxa2xx_fir_event, s);
+}
 
 register_savevm(NULL, pxa2xx_fir, 0, 0, pxa2xx_fir_save,
 pxa2xx_fir_load, s);
diff --git a/hw/bt-hci-csr.c b/hw/bt-hci-csr.c
index e4ada3c..55c819b 100644
--- a/hw/bt-hci-csr.c
+++ b/hw/bt-hci-csr.c
@@ -439,6 +439,7 @@ CharDriverState *uart_hci_init(qemu_irq wakeup)
 s-chr.opaque = s;
 s-chr.chr_write = csrhci_write;
 s-chr.chr_ioctl = csrhci_ioctl;
+s-chr.avail_connections = 1;
 
 s-hci = qemu_next_hci();
 s-hci-opaque = s;
diff --git a/hw/ipoctal232.c b/hw/ipoctal232.c
index 1da6a99..f93ad5c 100644
--- a/hw/ipoctal232.c
+++ b/hw/ipoctal232.c
@@ -556,6 +556,7 @@ static int ipoctal_init(IPackDevice *ip)
 
 if (ch-dev) {
 index++;
+ch-dev-avail_connections--;
 qemu_chr_add_handlers(ch-dev, hostdev_can_receive,
   hostdev_receive, hostdev_event, ch);
 DPRINTF(Redirecting channel %u to %s (%s)\n,
diff --git a/hw/ivshmem.c b/hw/ivshmem.c
index 68a2cf2..82d34b7 100644
--- a/hw/ivshmem.c
+++ b/hw/ivshmem.c
@@ -292,6 +292,7 @@ static CharDriverState* create_eventfd_chr_device(void * 
opaque, EventNotifier *
 fprintf(stderr, creating eventfd for eventfd %d failed\n, eventfd);
 exit(-1);
 }
+chr-avail_connections--;
 
 /* if MSI is supported we need multiple interrupts */
 if (ivshmem_has_feature(s, IVSHMEM_MSI)) {
diff --git a/hw/mcf_uart.c b/hw/mcf_uart.c
index aacf0f0..079e776 100644
--- a/hw/mcf_uart.c
+++ b/hw/mcf_uart.c
@@ -280,6 +280,12 @@ void *mcf_uart_init(qemu_irq irq, CharDriverState *chr)
 s-chr = chr;
 s-irq = irq;
 if (chr) {
+if (chr-avail_connections  1) {
+fprintf(stderr, mcf_uart_init error chardev %s already used\n,
+chr-label);
+exit(1);
+}
+chr-avail_connections--;
 qemu_chr_add_handlers(chr, mcf_uart_can_receive, mcf_uart_receive,
   mcf_uart_event, s);
 }
diff --git a/hw/serial.c b/hw/serial.c
index 0ccc499..4e342fd 100644
--- 

Re: [Qemu-devel] [PATCH 07/11] qemu-char: Move incrementing of avail_connections to qdev-properties-system

2013-03-27 Thread Hans de Goede

Hi,

On 03/26/2013 02:50 PM, Paolo Bonzini wrote:

snip


1) For most problematic devices, the proper fix would be to make them
use a chardev qdev property for there chardev usage, and then this
would be automatically fixed, agreed?


At least on x86, all devices already use a chardev qdev property.


Yes on x86 maybe, but a lot of the other serial-port emulations are
still using serial_hds directly, making proper avail_connections tracking
a pain.

Anyways I've audited all frontends now, fixing things where necessary,
and where possible in a generic way.

I'll send a patch for this right after this mail.

Regards,

Hans



Re: [Qemu-devel] [PATCH 05/12] target-i386: push hot-plugged VCPU state to KVM and unstop it

2013-03-27 Thread Andreas Färber
Am 27.03.2013 14:27, schrieb Igor Mammedov:
 On Wed, 27 Mar 2013 13:17:45 +0100
 Andreas Färber afaer...@suse.de wrote:
 Then I should move following parts to superclass:

 if (dev-hotplugged) {
 cpu_synchronize_post_init(env);
 resume_vcpu(CPU(cpu));
 }

 because in case of KVM we should make sure that CPU in sane state before
 allowing CPU to become run-able.

 That's not possible until we change cpu_synchronize_post_init() argument
 to CPUState, which is somewhere down my TODO list. Currently I have
 mostly flushed my refactorings out, so if you wanted to dive into that,
 that would be appreciated. :)
 
 Would something like this be acceptable?
 ---
 From 15502168009b7b5ae2b46d854c461e5ad031cdc6 Mon Sep 17 00:00:00 2001
 From: Igor Mammedov imamm...@redhat.com
 Date: Wed, 27 Mar 2013 14:21:20 +0100
 Subject: [PATCH] cpu: Pass CPUState to *cpu_synchronize_post*()
 
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  cpus.c   |  4 ++--
  include/sysemu/kvm.h | 12 ++--
  kvm-all.c|  8 ++--
  kvm-stub.c   |  4 ++--
  4 files changed, 12 insertions(+), 16 deletions(-)

Looks good so far, did you grep for further occurrences?

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 04/10] qemu-ga: Add Windows VSS provider to quiesce applications on fsfreeze

2013-03-27 Thread Tomoki Sekiyama
On Tue, 2013-03-26 at 16:30 +, Tomoki Sekiyama wrote:
 On 3/26/13 4:44 , Vadim Rozenfeld vroze...@redhat.com wrote:

On Tue, 2013-03-26 at 08:09 +0100, Paolo Bonzini wrote:
 Il 25/03/2013 21:50, Tomoki Sekiyama ha scritto:
 Unfortunately, if I remove importlib(stdole2.tlb), generated .tlb
seems
  rejected to register into Windows COM+ Catalog.
 
  Wine has stdole2.tlb in its fakedlls directory, but widl does not
accept
  this by
 
error: Wrong or unsupported typelib magic 405a4d
 
  Even if I copied native stdole2.tlb, widl fails with the same error.
 
  Do you have any idea about this error?

Seems like your tlb has nonstandard identifying characters. Check first
several bytes of the tlb file. Usually it should be something like '4D
53 46 54', or '4D 5A 90'

 I found '4D 53 46 54' at offset 0x300 of stdole2.tlb.
 Wine's stdole2.tlb also have the signature at offset 0x738.


 I rechecked stdole2.tlb provided by MS. All three versions (arm, x86,
 and x64) begin with 'MSFT' (4D 53 46 54) and have 00 00 00 00  at offset
 0x300

Oops, I was watching stdole2.tlb picked up from system32 directory.
Thank you for rechecking.

Still, it is not available in POSIX environment...

 (might have an additional header before the main part?)

 Widl accepted the wine's stdole2.tlb when I removed the header,
 but still, the generated qga-provider.tlb was not accepted by Windows...:(

 
  No, I suggest you ask on the Wine mailing lists.  In the meanwhile we
  can include a precompiled .tlb file in the QEMU repository and use midl.
 
  Paolo

 I will take this way so far.

 Thanks,

 Tomoki Sekiyama




Re: [Qemu-devel] selecting a sparc framebuffer from command line

2013-03-27 Thread Bob Breuer
On 3/26/2013 12:24 PM, Artyom Tarasenko wrote:
 On Tue, Mar 26, 2013 at 4:08 PM, Bob Breuer breu...@mc.net wrote:
 On 3/26/2013 6:13 AM, Artyom Tarasenko wrote:
 It looks like we will have more framebuffers beside TCX in the near future.
 One way to use them would be to make new machines combining a base
 machine name with a framebuffer name, like ss5tcx and ss5cg3, but I
 guess this would produce too many machines if we have more than 2
 framebuffers.

 Should the -vga option be (mis)used for selecting a framebuffer?
 They are not called VGA in the wild, but maybe the name VGA is
 obvious enough for a modern user? After all there is already a -hda
 option in the SPARCStation-5 emulation command line, although in the
 non-emulating world no one calls scsi disk hda.

 Or should an architecture-specific option, like -framebuffer be added?


 I would like to see -device used.  With a better sbus framework, adding
 multiple video devices on sparc32 should be really easy to do.

 I just looked at how -device cirrus-vga works.  To get -device to
 override the default video, we need to add minimal support into vl.c to
 recognize each video device and handle the default_vga flag.  Then
 sparc32 could limit itself to handling either VGA_NONE or VGA_STD when
 it adds the default video.
 
 Oh, that's even better!
 And then how would the OpenBIOS find out what framebuffer to use?

For a single display device, can it do just like OBP?  Run all FCode
roms, then find the display device in the device tree.

For multiple devices, how to select the primary?  Possibly just use the
first one found.  OBP provides some control over this with
sbus-probe-list.  As a parallel question, for a PC with 2 PCI video
cards, which one gets used as the BIOS display?  Just trying
qemu-system-i386 -device cirrus-vga -device cirrus-vga gives an error
about RAMBlock already registered.

 Would -device cg3 add the FCode ROM? Or do we need another -device for that?

The FCode ROM should be integrated with the device, similar to a PCI
device with an expansion rom.  The SBUS specification defines a rom to
be at offset 0 of the slot.  Probably setup a memory region container
for each sbus slot so that the device addresses are relative to the slot
base and isolated from the system address space.

For the existing TCX, looks like it also has space for a rom at offset
0.  See ftp://ftp.rodents-montreal.org/pub/mouse/docs/Sun/S24/memory-map

So as a stepping stone, should we create an FCode ROM for TCX?

Bob



Re: [Qemu-devel] [PATCH 07/11] qemu-char: Move incrementing of avail_connections to qdev-properties-system

2013-03-27 Thread Paolo Bonzini
Il 27/03/2013 15:09, Hans de Goede ha scritto:
 Hi,
 
 On 03/26/2013 02:50 PM, Paolo Bonzini wrote:
 
 snip
 
 1) For most problematic devices, the proper fix would be to make them
 use a chardev qdev property for there chardev usage, and then this
 would be automatically fixed, agreed?

 At least on x86, all devices already use a chardev qdev property.
 
 Yes on x86 maybe, but a lot of the other serial-port emulations are
 still using serial_hds directly, making proper avail_connections tracking
 a pain.

serial_hds is still passed to most devices via a chardev qdev property.
 See for example sparc/leon3.c, which uses grlib_apbuart_create and that
function sets the chardev.

Luckily there are very few UART implementations, most boards use the
8250/16550, hence this is even true of boards that are generally not
qdev-ified (like OMAP).  There are exceptions, like mcf_uart.c and
bt-hci-csr.c.

Paolo

 Anyways I've audited all frontends now, fixing things where necessary,
 and where possible in a generic way.
 
 I'll send a patch for this right after this mail.
 
 Regards,
 
 Hans




Re: [Qemu-devel] [Qemu-ppc] [PATCH] PPC: init_excp_7x0: fix hreset entry point.

2013-03-27 Thread Fabien Chouteau
On 03/27/2013 03:04 PM, Alexander Graf wrote:
 On 27.03.2013, at 15:00, Alexander Graf wrote:
 On 27.03.2013, at 14:54, Alexander Graf wrote:
 On 27.03.2013, at 14:50, Fabien Chouteau wrote:
 diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
 index 781170f..a5bae1e 100644
 --- a/target-ppc/translate_init.c
 +++ b/target-ppc/translate_init.c
 @@ -2885,7 +2885,7 @@ static void init_excp_7x0 (CPUPPCState *env)
   env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
   env-hreset_excp_prefix = 0xUL;
   /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0xFFF00100UL;

 As you properly explained above, the reset vector is 0x100 according to the 
 spec. However, hreset_excp_prefix is 0x0. How do we end up getting to 
 0xfff00100 here?

 According to 7xx_um.pdf (740 / 750 User manual), the IP bit in MSR (bit 25 
 in ppc notion) controls whether excp_prefix is 0xfff0 or 0x. The 
 spec also says:

 When either HRESET is negated or SRESET transitions to asserted, the 
 processor attempts to fetch code from the system reset exception vector. The 
 vector is located at offset 0x00100 from the exception prefix (all zeros or 
 ones, depending on the setting of the exception prefix bit in the machine 
 state register (MSR[IP]). The MSR[IP] bit is set for HRESET.

 So on reset, MSR[IP] = 1. That means that hreset_excp_prefix is also wrong 
 here.

 Please add the respective logic that sets hreset_excp_prefix according to 
 MSR[IP] on 740 / 750, otherwise whatever you're trying to execute will break 
 as soon as it gets its first real exception :).
 

It's actually already implemented (helper_regs.h:96). The question is:
what is the value of MSR[IP] at reset?

Also, we might want to call hreg_store_msr() in ppc_cpu_reset() instead
of just setting the value env-msr, this way we don't need
hreset_excp_prefix as the MSR[IP] will be used to set the value of
env-excp_prefix. Something like:

+++ b/target-ppc/translate_init.c
@@ -8182,19 +8182,23 @@ static void ppc_cpu_reset(CPUState *s)
 msr |= (target_ulong)1  MSR_VR; /* Allow altivec usage */
 msr |= (target_ulong)1  MSR_SPE; /* Allow SPE usage */
 msr |= (target_ulong)1  MSR_PR;
-#else
-env-excp_prefix = env-hreset_excp_prefix;
-env-nip = env-hreset_vector | env-excp_prefix;
-if (env-mmu_model != POWERPC_MMU_REAL) {
-ppc_tlb_invalidate_all(env);
-}
 #endif
-env-msr = msr  env-msr_mask;
+
 #if defined(TARGET_PPC64)
 if (env-mmu_model  POWERPC_MMU_64) {
 env-msr |= (1ULL  MSR_SF);
 }
 #endif
+
+hreg_store_msr(env, msr, 1);
+
+#if !defined(CONFIG_USER_ONLY)
+env-nip = env-hreset_vector | env-excp_prefix;
+if (env-mmu_model != POWERPC_MMU_REAL) {
+ppc_tlb_invalidate_all(env);
+}
+#endif
+
 hreg_compute_hflags(env);
 env-reserve_addr = (target_ulong)-1ULL;
 /* Be sure no exception or interrupt is pending */




 While at it, on exception delivery ILE, ME and IP do not get modified 
 according to the spec. Please verify that we don't accidentally set them to 0 
 when we deliver an interrupt. 

They seems to be preserved.

 Except for machine check interrupts, where MSR.ME = 0.

This is done, excp_helper.c:148.


 Also, MSR.LE becomes the previous value of MSR.ILE. Not that we'd implement 
 LE mode properly ;).


This is also already done, excp_helper.c:615.


Thanks,

-- 
Fabien Chouteau



Re: [Qemu-devel] [PATCH 18/26] hw/usb/dev-smartcard-reader.c: copy atr's protocol to ccid's parameters (adds todo's)

2013-03-27 Thread Alon Levy
On Fri, Mar 22, 2013 at 03:23:15PM +0100, Marc-André Lureau wrote:
 Hi,
 
 
 On Mon, Mar 18, 2013 at 2:11 PM, Alon Levy al...@redhat.com wrote:
  +if (atr_protocol_num == 0) {
  +DPRINTF(s, D_WARN, %s: error: unimplemented ATR T0 parameters
  + setting\n, __func__);
  +t0-bmFindexDindex = 0;
  +t0-bmTCCKST0 = 0;
  +t0-bGuardTimeT0 = 0;
  +t0-bWaitingIntegerT0 = 0;
  +t0-bClockStop = 0;
  +} else {
  +if (atr_protocol_num != 1) {
  +DPRINTF(s, D_WARN, %s: error: unsupported ATR protocol %d\n,
  +__func__, atr_protocol_num);
  +} else {
  +DPRINTF(s, D_WARN, %s: error: unimplemented ATR T1 parameters
  + setting\n, __func__);
  +t1-bmFindexDindex = 0;
  +t1-bmTCCKST1 = 0;
  +t1-bGuardTimeT1 = 0;
  +t1-bWaitingIntegerT1 = 0;
  +t1-bClockStop = 0;
  +t1-bIFSC = 0;
  +t1-bNadValue = 0;
  +}
  +}
 
 Those blocks could be at the same indentation level, or perhaps use a switch?

A switch is a good idea.

 
 Is it sensible to warn in all cases?

Turning them into TODO comments instead (except for the now default
case).

 
 -- 
 Marc-André Lureau
 



Re: [Qemu-devel] [Qemu-ppc] [PATCH] PPC: init_excp_7x0: fix hreset entry point.

2013-03-27 Thread Fabien Chouteau
On 03/27/2013 03:59 PM, Fabien Chouteau wrote:

 It's actually already implemented (helper_regs.h:96). The question is:
 what is the value of MSR[IP] at reset?
 

Right now in QEMU, MSR[IP] is always set at reset (MSR_EP == MSR_IP):

translate_init.c:8174:
msr |= (target_ulong)1  MSR_EP; 


-- 
Fabien Chouteau



Re: [Qemu-devel] [Qemu-ppc] [PATCH] PPC: init_excp_7x0: fix hreset entry point.

2013-03-27 Thread Alexander Graf

On 27.03.2013, at 15:59, Fabien Chouteau wrote:

 On 03/27/2013 03:04 PM, Alexander Graf wrote:
 On 27.03.2013, at 15:00, Alexander Graf wrote:
 On 27.03.2013, at 14:54, Alexander Graf wrote:
 On 27.03.2013, at 14:50, Fabien Chouteau wrote:
 diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
 index 781170f..a5bae1e 100644
 --- a/target-ppc/translate_init.c
 +++ b/target-ppc/translate_init.c
 @@ -2885,7 +2885,7 @@ static void init_excp_7x0 (CPUPPCState *env)
  env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700;
  env-hreset_excp_prefix = 0xUL;
  /* Hardware reset vector */
 -env-hreset_vector = 0xFFFCUL;
 +env-hreset_vector = 0xFFF00100UL;
 
 As you properly explained above, the reset vector is 0x100 according to 
 the spec. However, hreset_excp_prefix is 0x0. How do we end up getting to 
 0xfff00100 here?
 
 According to 7xx_um.pdf (740 / 750 User manual), the IP bit in MSR (bit 25 
 in ppc notion) controls whether excp_prefix is 0xfff0 or 0x. 
 The spec also says:
 
 When either HRESET is negated or SRESET transitions to asserted, the 
 processor attempts to fetch code from the system reset exception vector. 
 The vector is located at offset 0x00100 from the exception prefix (all 
 zeros or ones, depending on the setting of the exception prefix bit in the 
 machine state register (MSR[IP]). The MSR[IP] bit is set for HRESET.
 
 So on reset, MSR[IP] = 1. That means that hreset_excp_prefix is also wrong 
 here.
 
 Please add the respective logic that sets hreset_excp_prefix according to 
 MSR[IP] on 740 / 750, otherwise whatever you're trying to execute will 
 break as soon as it gets its first real exception :).
 
 
 It's actually already implemented (helper_regs.h:96). The question is:
 what is the value of MSR[IP] at reset?

For 740 / 750, it's 1. All other bits are 0.

 Also, we might want to call hreg_store_msr() in ppc_cpu_reset() instead
 of just setting the value env-msr, this way we don't need
 hreset_excp_prefix as the MSR[IP] will be used to set the value of
 env-excp_prefix. Something like:

Sounds good :)


Alex

 
 +++ b/target-ppc/translate_init.c
 @@ -8182,19 +8182,23 @@ static void ppc_cpu_reset(CPUState *s)
 msr |= (target_ulong)1  MSR_VR; /* Allow altivec usage */
 msr |= (target_ulong)1  MSR_SPE; /* Allow SPE usage */
 msr |= (target_ulong)1  MSR_PR;
 -#else
 -env-excp_prefix = env-hreset_excp_prefix;
 -env-nip = env-hreset_vector | env-excp_prefix;
 -if (env-mmu_model != POWERPC_MMU_REAL) {
 -ppc_tlb_invalidate_all(env);
 -}
 #endif
 -env-msr = msr  env-msr_mask;
 +
 #if defined(TARGET_PPC64)
 if (env-mmu_model  POWERPC_MMU_64) {
 env-msr |= (1ULL  MSR_SF);
 }
 #endif
 +
 +hreg_store_msr(env, msr, 1);
 +
 +#if !defined(CONFIG_USER_ONLY)
 +env-nip = env-hreset_vector | env-excp_prefix;
 +if (env-mmu_model != POWERPC_MMU_REAL) {
 +ppc_tlb_invalidate_all(env);
 +}
 +#endif
 +
 hreg_compute_hflags(env);
 env-reserve_addr = (target_ulong)-1ULL;
 /* Be sure no exception or interrupt is pending */
 
 
 
 
 While at it, on exception delivery ILE, ME and IP do not get modified 
 according to the spec. Please verify that we don't accidentally set them to 
 0 when we deliver an interrupt. 
 
 They seems to be preserved.
 
 Except for machine check interrupts, where MSR.ME = 0.
 
 This is done, excp_helper.c:148.
 
 
 Also, MSR.LE becomes the previous value of MSR.ILE. Not that we'd implement 
 LE mode properly ;).
 
 
 This is also already done, excp_helper.c:615.
 
 
 Thanks,
 
 -- 
 Fabien Chouteau




Re: [Qemu-devel] [PATCH] chardev-frontends: Explicitly check, inc and dec avail_connections

2013-03-27 Thread Paolo Bonzini
Il 27/03/2013 15:10, Hans de Goede ha scritto:
 chardev-frontends need to explictly check, increase and decrement the
 avail_connections property of the chardev when they are not using a
 qdev-chardev-property for the chardev.
 
 This fixes things like:
 qemu-kvm -chardev stdio,id=foo -device isa-serial,chardev=foo \
   -mon chardev=foo
 
 Working, where they should fail. Most of the changes here are due to
 old hardware emulation code which is using serial_hds directly rather then
 a qdev-chardev-property.
 
 Signed-off-by: Hans de Goede hdego...@redhat.com
 ---
  backends/rng-egd.c |  7 +++
  gdbstub.c  |  1 +
  hw/arm/pxa2xx.c|  9 -
  hw/bt-hci-csr.c|  1 +
  hw/ipoctal232.c|  1 +
  hw/ivshmem.c   |  1 +
  hw/mcf_uart.c  |  6 ++
  hw/serial.c| 16 
  hw/serial.h|  1 +
  hw/sh_serial.c |  9 -
  hw/xen_console.c   | 19 +++
  net/slirp.c|  1 +
  qemu-char.c| 14 +-
  vl.c   |  7 +++
  14 files changed, 86 insertions(+), 7 deletions(-)
 
 diff --git a/backends/rng-egd.c b/backends/rng-egd.c
 index 5e012e9..d8e9d63 100644
 --- a/backends/rng-egd.c
 +++ b/backends/rng-egd.c
 @@ -149,6 +149,12 @@ static void rng_egd_opened(RngBackend *b, Error **errp)
  return;
  }
  
 +if (s-chr-avail_connections  1) {
 +error_set(errp, QERR_DEVICE_IN_USE, s-chr_name);
 +return;
 +}
 +s-chr-avail_connections--;
 +
  /* FIXME we should resubmit pending requests when the CDS reconnects. */
  qemu_chr_add_handlers(s-chr, rng_egd_chr_can_read, rng_egd_chr_read,
NULL, s);
 @@ -191,6 +197,7 @@ static void rng_egd_finalize(Object *obj)
  
  if (s-chr) {
  qemu_chr_add_handlers(s-chr, NULL, NULL, NULL, NULL);
 +s-chr-avail_connections++;
  }
  
  g_free(s-chr_name);

Ok, but please create wrappers for these (e.g. qemu_chr_be_start/stop
and qemu_chr_be_start_nofail) and use them throughout.

 diff --git a/gdbstub.c b/gdbstub.c
 index a666cb5..83267e0 100644
 --- a/gdbstub.c
 +++ b/gdbstub.c
 @@ -3025,6 +3025,7 @@ int gdbserver_start(const char *device)
  if (!chr)
  return -1;
  
 +chr-avail_connections--;
  qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive,
gdb_chr_event, NULL);
  }

Ok.

 diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
 index 7467cca..df4b458 100644
 --- a/hw/arm/pxa2xx.c
 +++ b/hw/arm/pxa2xx.c
 @@ -1981,9 +1981,16 @@ static PXA2xxFIrState *pxa2xx_fir_init(MemoryRegion 
 *sysmem,
  memory_region_init_io(s-iomem, pxa2xx_fir_ops, s, pxa2xx-fir, 
 0x1000);
  memory_region_add_subregion(sysmem, base, s-iomem);
  
 -if (chr)
 +if (chr) {
 +if (chr-avail_connections  1) {
 +fprintf(stderr, pxa2xx_fir_init error chardev %s already 
 used\n,
 +chr-label);
 +exit(1);
 +}
 +chr-avail_connections--;
  qemu_chr_add_handlers(chr, pxa2xx_fir_is_empty,
  pxa2xx_fir_rx, pxa2xx_fir_event, s);
 +}
  
  register_savevm(NULL, pxa2xx_fir, 0, 0, pxa2xx_fir_save,
  pxa2xx_fir_load, s);

Errors won't be reported, because serial_hds[] will always create its
own CharDriverState and avail_connections will always be 1.  Use a
wrapper and the code can ignore this.

 diff --git a/hw/bt-hci-csr.c b/hw/bt-hci-csr.c
 index e4ada3c..55c819b 100644
 --- a/hw/bt-hci-csr.c
 +++ b/hw/bt-hci-csr.c
 @@ -439,6 +439,7 @@ CharDriverState *uart_hci_init(qemu_irq wakeup)
  s-chr.opaque = s;
  s-chr.chr_write = csrhci_write;
  s-chr.chr_ioctl = csrhci_ioctl;
 +s-chr.avail_connections = 1;
  
  s-hci = qemu_next_hci();
  s-hci-opaque = s;

Ok.

 diff --git a/hw/ipoctal232.c b/hw/ipoctal232.c
 index 1da6a99..f93ad5c 100644
 --- a/hw/ipoctal232.c
 +++ b/hw/ipoctal232.c
 @@ -556,6 +556,7 @@ static int ipoctal_init(IPackDevice *ip)
  
  if (ch-dev) {
  index++;
 +ch-dev-avail_connections--;
  qemu_chr_add_handlers(ch-dev, hostdev_can_receive,
hostdev_receive, hostdev_event, ch);
  DPRINTF(Redirecting channel %u to %s (%s)\n,

Ouch.  WTF was I thinking when I reviewed this? :)  Please change this
to use DEFINE_PROP_CHARDEV.  I don't really care if it is
backwards-incompatible.

 diff --git a/hw/ivshmem.c b/hw/ivshmem.c
 index 68a2cf2..82d34b7 100644
 --- a/hw/ivshmem.c
 +++ b/hw/ivshmem.c
 @@ -292,6 +292,7 @@ static CharDriverState* create_eventfd_chr_device(void * 
 opaque, EventNotifier *
  fprintf(stderr, creating eventfd for eventfd %d failed\n, eventfd);
  exit(-1);
  }
 +chr-avail_connections--;
  
  /* if MSI is supported we need multiple interrupts */
  if (ivshmem_has_feature(s, IVSHMEM_MSI)) {

Ok.

 diff --git 

Re: [Qemu-devel] [PATCH v3] block: Add support for Secure Shell (ssh) block device.

2013-03-27 Thread Anthony Liguori
Richard W.M. Jones rjo...@redhat.com writes:

 From: Richard W.M. Jones rjo...@redhat.com

   qemu-system-x86_64 -drive file=ssh://hostname/some/image

 QEMU will ssh into 'hostname' and open '/some/image' which is made
 available as a standard block device.

 You can specify a username (ssh://user@host/...) and/or a port number
 (ssh://host:port/...).

 Current limitations:

 - Authentication must be done without passwords or passphrases, using
   ssh-agent.  Other authentication methods are not supported. (*)

 - New remote files cannot be created. (*)

 - Uses a single connection, instead of concurrent AIO with multiple
   SSH connections.

 (*) = potentially easy fix

 This is implemented using libssh2 on the client side.  The server just
 requires a regular ssh daemon with sftp-server support.  Most ssh
 daemons on Unix/Linux systems will work out of the box.

 Thanks: Stefan Hajnoczi, Kevin Wolf.

Curl actually supports sftp already.  In theory, we just need to add:

static BlockDriver bdrv_sftp = {
.format_name = sftp,
.protocol_name   = sftp,

.instance_size   = sizeof(BDRVCURLState),
.bdrv_file_open  = curl_open,
.bdrv_close  = curl_close,
.bdrv_getlength  = curl_getlength,

.bdrv_aio_readv  = curl_aio_readv,
};

To block/curl.c and it should Just Work.   Have you considered doing
this through curl?

Regards,

Anthony Liguori


 ---
  block/Makefile.objs |   1 +
  block/ssh.c | 750 
 
  configure   |  47 
  qemu-doc.texi   |  40 +++
  qemu-options.hx |  12 +
  5 files changed, 850 insertions(+)
  create mode 100644 block/ssh.c

 diff --git a/block/Makefile.objs b/block/Makefile.objs
 index c067f38..6c4b5bc 100644
 --- a/block/Makefile.objs
 +++ b/block/Makefile.objs
 @@ -13,6 +13,7 @@ block-obj-$(CONFIG_LIBISCSI) += iscsi.o
  block-obj-$(CONFIG_CURL) += curl.o
  block-obj-$(CONFIG_RBD) += rbd.o
  block-obj-$(CONFIG_GLUSTERFS) += gluster.o
 +block-obj-$(CONFIG_LIBSSH2) += ssh.o
  endif
  
  common-obj-y += stream.o
 diff --git a/block/ssh.c b/block/ssh.c
 new file mode 100644
 index 000..f7b8d32
 --- /dev/null
 +++ b/block/ssh.c
 @@ -0,0 +1,750 @@
 +/*
 + * Secure Shell (ssh) backend for QEMU.
 + *
 + * Copyright (C) 2013 Red Hat Inc., Richard W.M. Jones rjo...@redhat.com
 + *
 + * Permission is hereby granted, free of charge, to any person obtaining a 
 copy
 + * of this software and associated documentation files (the Software), to 
 deal
 + * in the Software without restriction, including without limitation the 
 rights
 + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
 + * copies of the Software, and to permit persons to whom the Software is
 + * furnished to do so, subject to the following conditions:
 + *
 + * The above copyright notice and this permission notice shall be included in
 + * all copies or substantial portions of the Software.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
 + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
 FROM,
 + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 + * THE SOFTWARE.
 + */
 +
 +#include stdio.h
 +#include stdlib.h
 +#include stdarg.h
 +
 +#ifndef _WIN32
 +#include pwd.h
 +#endif
 +
 +#include libssh2.h
 +#include libssh2_sftp.h
 +
 +#include block/block_int.h
 +#include qemu/sockets.h
 +#include qemu/uri.h
 +#include qapi/qmp/qint.h
 +
 +#define DEBUG_SSH 1
 +
 +#if DEBUG_SSH
 +#define DPRINTF(fmt,...)\
 +do {\
 +fprintf(stderr, ssh: %-15s  fmt \n, __func__, ##__VA_ARGS__); \
 +} while (0)
 +#else
 +#define DPRINTF(fmt,...) /* nothing */
 +#endif
 +
 +typedef struct BDRVSSHState {
 +CoMutex lock; /* coroutine lock */
 +
 +/* ssh connection */
 +int sock; /* socket */
 +LIBSSH2_SESSION *session; /* ssh session */
 +LIBSSH2_SFTP *sftp;   /* sftp session */
 +LIBSSH2_SFTP_HANDLE *sftp_handle; /* sftp remote file handle */
 +
 +/* file attributes (at open) */
 +LIBSSH2_SFTP_ATTRIBUTES attrs;
 +} BDRVSSHState;
 +
 +/* Wrappers around error_report which make sure to dump as much
 + * information from libssh2 as possible.
 + */
 +static void
 +session_error_report(BDRVSSHState *s, const char *fs, ...)
 +{
 +va_list args;
 +
 +va_start(args, fs);
 +
 +if ((s)-session) {
 +char *ssh_err;
 +int ssh_err_len = sizeof ssh_err;
 +int ssh_err_code;
 +
 +libssh2_session_last_error((s)-session,
 +   

Re: [Qemu-devel] [PATCH 07/11] qemu-char: Move incrementing of avail_connections to qdev-properties-system

2013-03-27 Thread Hans de Goede

Hi,

On 03/27/2013 03:58 PM, Paolo Bonzini wrote:

Il 27/03/2013 15:09, Hans de Goede ha scritto:

Hi,

On 03/26/2013 02:50 PM, Paolo Bonzini wrote:

snip


1) For most problematic devices, the proper fix would be to make them
use a chardev qdev property for there chardev usage, and then this
would be automatically fixed, agreed?


At least on x86, all devices already use a chardev qdev property.


Yes on x86 maybe, but a lot of the other serial-port emulations are
still using serial_hds directly, making proper avail_connections tracking
a pain.


serial_hds is still passed to most devices via a chardev qdev property.


Most, yes but not all, which is why I wrote using serial_hds *directly*,
anyways see the patch which I send a while back which tries to deal with
all the *direct* serial_hds users, as well as with the monitor, and some
code which does chardev creation completely on its own.

Regards,

Hans



Re: [Qemu-devel] dataplane bug: fail to start Windows VM with dataplane enable

2013-03-27 Thread Stefan Hajnoczi
On Wed, Mar 27, 2013 at 03:02:01PM +0100, Stefan Hajnoczi wrote:
 On Wed, Mar 27, 2013 at 02:32:19PM +0100, Stefan Hajnoczi wrote:
  On Tue, Mar 26, 2013 at 11:10:35PM +0800, 张磊强 wrote:
   Hi,  Paolo  Stefan:
   
   When I test the dataplane feature with qemu master, I find that
   Windows (Windows 7 and Windows 2003) VM will hang if dataplane is
   enabled. But if I try to start a Fedora VM, it can start normally.
   
   The command I boot QEMU is:
   x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 1024 -smp 2 -drive
   file=win7.img,if=none,id=drive-virtio-disk,format=raw,cache=none,aio=native
   -device 
   virtio-blk-pci,config-wce=off,scsi=off,x-data-plane=on,drive=drive-virtio-disk,id=virtio-disk
   
   I found the similar bug has reported some days ago:
   http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg02200.html
   . And a patch for this bug has already committed by Paolo at Mar 13:
   http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg02200.html
   .
   
   But it cannot work under my environment. Could you give me some advise
   to debug this problem ? I can provide more information if need.
  
  Hi,
  I haven't gotten to the bottom of it yet but wanted to let you know that
  I'm seeing a hang at the boot screen after the installer reboots.  The
  Windows logo animation runs but the guest seems unable to make progress.
  
  Will let you know when there is a fix.  The guest I'm testing is Windows
  7 Professional 64-bit.
  
  As a workaround you can set x-data-plane=off to boot the guest for the
  first time.  Subsequent boots will succeed with x-data-plane=on.
 
 Next data point, it's not caused by the Paolo's AioContext conversion.
 This means it's unrelated to the recent bug that you mentioned.
 
 The boot gets stuck immediately after the switch from the BIOS
 virtio-blk driver to the Windows viostor driver.  We get two requests
 (VIRTIO_BLK_T_GET_ID and VIRTIO_BLK_T_READ) and then the guest stops.
 The descriptor index of the second request has been incremented, perhaps
 the guest is not receiving host-guest notifies.

Okay, getting closer to the root cause now.  The guest driver has not
enabled MSI-X and the guest notifier that dataplane uses is not hooked
up to anything.  When MSI-X is enabled the guest notifier is hooked up
to irqfd and when MSI-X is disabled it is supposed to bounce back into
QEMU which issues an ioctl for the interrupt.  We've gotten into a state
where the guest notifier exists but nothing is listening to it :).

Therefore the guest is stuck waiting for virtio-blk I/O requests to
complete.

Stefan



Re: [Qemu-devel] [PATCH 05/12] target-i386: push hot-plugged VCPU state to KVM and unstop it

2013-03-27 Thread Igor Mammedov
On Wed, 27 Mar 2013 15:30:30 +0100
Andreas Färber afaer...@suse.de wrote:

 Am 27.03.2013 14:27, schrieb Igor Mammedov:
  On Wed, 27 Mar 2013 13:17:45 +0100
  Andreas Färber afaer...@suse.de wrote:
  Then I should move following parts to superclass:
 
  if (dev-hotplugged) {
  cpu_synchronize_post_init(env);
  resume_vcpu(CPU(cpu));
  }
 
  because in case of KVM we should make sure that CPU in sane state before
  allowing CPU to become run-able.
 
  That's not possible until we change cpu_synchronize_post_init() argument
  to CPUState, which is somewhere down my TODO list. Currently I have
  mostly flushed my refactorings out, so if you wanted to dive into that,
  that would be appreciated. :)
  
  Would something like this be acceptable?
  ---
  From 15502168009b7b5ae2b46d854c461e5ad031cdc6 Mon Sep 17 00:00:00 2001
  From: Igor Mammedov imamm...@redhat.com
  Date: Wed, 27 Mar 2013 14:21:20 +0100
  Subject: [PATCH] cpu: Pass CPUState to *cpu_synchronize_post*()
  
  Signed-off-by: Igor Mammedov imamm...@redhat.com
  ---
   cpus.c   |  4 ++--
   include/sysemu/kvm.h | 12 ++--
   kvm-all.c|  8 ++--
   kvm-stub.c   |  4 ++--
   4 files changed, 12 insertions(+), 16 deletions(-)
 
 Looks good so far, did you grep for further occurrences?
yep, I re-factored every *cpu_synchronize_post*() call,

but considering an intention to call cpu_synchronize_post_init() from
qom/cpu.c this patch won't work nice since it will pull with itself
kvm-stub.o to *-user target.

Due to qom/cpu.c is built only once for both softmmu and *-user targets, I
consider to move cpu_synchronize_post_init()  cpu_synchronize_post_reset()
from include/sysemu/kvm.h into include/sysemu/cpus.h with definition moved
into cpus.c + stubs for cpu_synchronize_post_init() resume_vcpu() in
libqemustub for *-user target.
Adding stubs to libqemustub could be avoided if resume_vcpu() and
cpu_synchronize_post_init() are called from x86_cpu_realizefn()
at the cost of some ifdeffenery in include/sysemu/cpus.h though.

But moving resume_vcpu()  cpu_synchronize_post_init() into qom/cpu.c looks
like good candidate for being reused by other targets. 

Paolo,
  would it be acceptable to add resume_vcpu()  cpu_synchronize_post_init()
  stubs into libqemustub? 


 Andreas
 




Re: [Qemu-devel] [PATCH 07/11] blockdev: flip default cache mode from writethrough to writeback

2013-03-27 Thread Artyom Tarasenko
On Fri, Aug 10, 2012 at 6:47 PM, Kevin Wolf kw...@redhat.com wrote:
 From: Paolo Bonzini pbonz...@redhat.com

 Now all major device models (IDE, SCSI, virtio) can choose between
 writethrough and writeback at run-time, and virtio will even revert
 to writethrough if the guest is not capable of sending flushes.  So
 we can change the default to writeback at last.

 Tested, for lack of a better idea, with a breakpoint on bdrv_open
 and all cache choices one by one.

This patch breaks shutting down of a sparc32 guest (or at least the
Debian-4 image I have):

$ sparc-softmmu/qemu-system-sparc -M SS-5 -nographic -hda  ../disk-debian-4
[...]
Debian GNU/Linux 4.0 debian ttyS0

debian login: root
Password:
Linux debian 2.6.18-6-sparc32 #1 Tue Nov 10 00:31:37 UTC 2009 sparc
# poweroff
[...]
Will now halt.
Synchronizing SCSI cache for disk sda:
esp0: Aborting command
esp0: dumping state
esp0: dma -- cond_rega4400010 addrf00b
esp0: SW [sreg03 sstep04 ireg10]
esp0: HW reread [sreg03 sstep00 ireg08]
esp0: current command [tgt00 lun00 pphaseMSGINDONE cphaseCLUELESS]
esp0: disconnected
esp0: Aborting command
esp0: dumping state
esp0: dma -- cond_rega4400010 addrf00b
esp0: SW [sreg03 sstep04 ireg10]
esp0: HW reread [sreg03 sstep04 ireg00]
esp0: current command [tgt00 lun00 pphaseUNISSUED cphaseUNISSUED]
esp0: disconnected
esp0: Resetting scsi bus
esp0: SCSI bus reset interrupt
esp0: no command in esp_handle()
Kernel panic - not syncing: esp_handle: current_SC == penguin within interrupt!
 0Press Stop-A (L1-A) to return to the boot prom


Without the patch, the line Synchronizing SCSI cache for disk sda
doesn't come up, so the patch probably just unveils a bug somewhere
else (esp?).

 Signed-off-by: Paolo Bonzini pbonz...@redhat.com
 Signed-off-by: Kevin Wolf kw...@redhat.com
 ---
  blockdev.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

 diff --git a/blockdev.c b/blockdev.c
 index 8669142..7c83baa 100644
 --- a/blockdev.c
 +++ b/blockdev.c
 @@ -377,6 +377,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
 }
  }

 +bdrv_flags |= BDRV_O_CACHE_WB;
  if ((buf = qemu_opt_get(opts, cache)) != NULL) {
  if (bdrv_parse_cache_flags(buf, bdrv_flags) != 0) {
  error_report(invalid cache option);
 --
 1.7.6.5





-- 
Regards,
Artyom Tarasenko

linux/sparc and solaris/sparc under qemu blog:
http://tyom.blogspot.com/search/label/qemu



Re: [Qemu-devel] [PATCH v4] Add option to mlock qemu and guest memory

2013-03-27 Thread Anthony Liguori
Satoru Moriya satoru.mor...@hds.com writes:

 In certain scenario, latency induced by paging is significant and
 memory locking is needed. Also, in the scenario with untrusted
 guests, latency improvement due to mlock is desired.

 This patch introduces a following new option to mlock guest and
 qemu memory:

 -realtime mlock=on|off

 Signed-off-by: Satoru Moriya satoru.mor...@hds.com
 Reviewed-by: Paolo Bonzini pbonz...@redhat.com
 Reviewed-by: Marcelo Tosatti mtosa...@redhat.com

This patch doesn't apply because you're sending it MIME encoded and the
patch has carriage returns in it.  I think your mailer is badly munging
the patch.

Please send it via git-send-email directly from the repository.

Regards,

Anthony Liguori

 ---
 ChangeLog:
 v4
  - Update commit message
 v3
  - Modify os_mlock() to return error code
  - Update configure_realtime() to handle return value from os_mlock()
  - Change the variable name from is_mlock to enable_mlock in 
 configure_realtime()
  - Rebase qemu version 1.4.50

 v2
  - Change option name from -mlock to -realtime mlock=on|off
  - Rebase qemu version 1.3.91
  - Update patch description

  include/sysemu/os-posix.h |  1 +
  include/sysemu/os-win32.h |  5 +
  os-posix.c| 12 
  qemu-options.hx   | 13 +
  vl.c  | 34 ++
  5 files changed, 65 insertions(+)

 diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
 index 7f198e4..25d0b2a 100644
 --- a/include/sysemu/os-posix.h
 +++ b/include/sysemu/os-posix.h
 @@ -31,6 +31,7 @@ void os_set_proc_name(const char *s);
  void os_setup_signal_handling(void);
  void os_daemonize(void);
  void os_setup_post(void);
 +int os_mlock(void);
  
  typedef struct timeval qemu_timeval;
  #define qemu_gettimeofday(tp) gettimeofday(tp, NULL)
 diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
 index 71f5fa0..bf8523a 100644
 --- a/include/sysemu/os-win32.h
 +++ b/include/sysemu/os-win32.h
 @@ -106,4 +106,9 @@ static inline bool is_daemonized(void)
  return false;
  }
  
 +static inline int os_mlock(void)
 +{
 +return -ENOSYS;
 +}
 +
  #endif
 diff --git a/os-posix.c b/os-posix.c
 index 5c64518..d39261d 100644
 --- a/os-posix.c
 +++ b/os-posix.c
 @@ -363,3 +363,15 @@ bool is_daemonized(void)
  {
  return daemonize;
  }
 +
 +int os_mlock(void)
 +{
 +int ret = 0;
 +
 +ret = mlockall(MCL_CURRENT | MCL_FUTURE);
 +if (ret  0) {
 +perror(mlockall);
 +}
 +
 +return ret;
 +}
 diff --git a/qemu-options.hx b/qemu-options.hx
 index 06dd565..1ec9541 100644
 --- a/qemu-options.hx
 +++ b/qemu-options.hx
 @@ -2569,6 +2569,19 @@ STEXI
  Do not start CPU at startup (you must type 'c' in the monitor).
  ETEXI
  
 +DEF(realtime, HAS_ARG, QEMU_OPTION_realtime,
 +-realtime [mlock=on|off]\n
 +run qemu with realtime features\n
 +mlock=on|off controls mlock support (default: on)\n,
 +QEMU_ARCH_ALL)
 +STEXI
 +@item -realtime mlock=on|off
 +@findex -realtime
 +Run qemu with realtime features.
 +mlocking qemu and guest memory can be enabled via @option{mlock=on}
 +(enabled by default).
 +ETEXI
 +
  DEF(gdb, HAS_ARG, QEMU_OPTION_gdb, \
  -gdb devwait for gdb connection on 'dev'\n, QEMU_ARCH_ALL)
  STEXI
 diff --git a/vl.c b/vl.c
 index aeed7f4..71bbcf1 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -521,6 +521,18 @@ static QemuOptsList qemu_tpmdev_opts = {
  },
  };
  
 +static QemuOptsList qemu_realtime_opts = {
 +.name = realtime,
 +.head = QTAILQ_HEAD_INITIALIZER(qemu_realtime_opts.head),
 +.desc = {
 +{
 +.name = mlock,
 +.type = QEMU_OPT_BOOL,
 +},
 +{ /* end of list */ }
 +},
 +};
 +
  const char *qemu_get_vm_name(void)
  {
  return qemu_name;
 @@ -1420,6 +1432,20 @@ static void smp_parse(const char *optarg)
  max_cpus = smp_cpus;
  }
  
 +static void configure_realtime(QemuOpts *opts)
 +{
 +bool enable_mlock;
 +
 +enable_mlock = qemu_opt_get_bool(opts, mlock, true);
 +
 +if (enable_mlock) {
 +if (os_mlock()  0) {
 +fprintf(stderr, qemu: locking memory failed\n);
 +exit(1);
 +}
 +}
 +}
 +
  /***/
  /* USB devices */
  
 @@ -2909,6 +2935,7 @@ int main(int argc, char **argv, char **envp)
  qemu_add_opts(qemu_add_fd_opts);
  qemu_add_opts(qemu_object_opts);
  qemu_add_opts(qemu_tpmdev_opts);
 +qemu_add_opts(qemu_realtime_opts);
  
  runstate_init();
  
 @@ -3878,6 +3905,13 @@ int main(int argc, char **argv, char **envp)
  exit(1);
  }
  break;
 +case QEMU_OPTION_realtime:
 +opts = qemu_opts_parse(qemu_find_opts(realtime), optarg, 
 0);
 +if (!opts) {
 +exit(1);
 +}
 +configure_realtime(opts);
 +  

Re: [Qemu-devel] [PATCH 07/11] blockdev: flip default cache mode from writethrough to writeback

2013-03-27 Thread Paolo Bonzini
Il 27/03/2013 16:16, Artyom Tarasenko ha scritto:
 This patch breaks shutting down of a sparc32 guest (or at least the
 Debian-4 image I have):
 
 $ sparc-softmmu/qemu-system-sparc -M SS-5 -nographic -hda  ../disk-debian-4
 [...]
 Debian GNU/Linux 4.0 debian ttyS0
 
 debian login: root
 Password:
 Linux debian 2.6.18-6-sparc32 #1 Tue Nov 10 00:31:37 UTC 2009 sparc
 # poweroff
 [...]
 Will now halt.
 Synchronizing SCSI cache for disk sda:
 esp0: Aborting command
 esp0: dumping state
 esp0: dma -- cond_rega4400010 addrf00b
 esp0: SW [sreg03 sstep04 ireg10]
 esp0: HW reread [sreg03 sstep00 ireg08]
 esp0: current command [tgt00 lun00 pphaseMSGINDONE cphaseCLUELESS]
 esp0: disconnected
 esp0: Aborting command
 esp0: dumping state
 esp0: dma -- cond_rega4400010 addrf00b
 esp0: SW [sreg03 sstep04 ireg10]
 esp0: HW reread [sreg03 sstep04 ireg00]
 esp0: current command [tgt00 lun00 pphaseUNISSUED cphaseUNISSUED]
 esp0: disconnected
 esp0: Resetting scsi bus
 esp0: SCSI bus reset interrupt
 esp0: no command in esp_handle()
 Kernel panic - not syncing: esp_handle: current_SC == penguin within 
 interrupt!
  0Press Stop-A (L1-A) to return to the boot prom
 
 
 Without the patch, the line Synchronizing SCSI cache for disk sda
 doesn't come up, so the patch probably just unveils a bug somewhere
 else (esp?).

It doesn't come up because, with a writethrough cache, there is no need
to flush the cache.  The bug should be reproducible before this patch
with -drive file=../disk-debian-4,cache=writeback.

Paolo



Re: [Qemu-devel] vNVRAM / blobstore design

2013-03-27 Thread Corey Bryant



On 03/25/2013 06:20 PM, Stefan Berger wrote:

On 03/25/2013 06:05 PM, Anthony Liguori wrote:

Stefan Berger stef...@linux.vnet.ibm.com writes:


[argh, just posted this to qemu-trivial -- it's not trivial]


Hello!

I am posting this message to revive the previous discussions about the
design of vNVRAM / blobstore cc'ing (at least) those that participated
in this discussion 'back then'.

The first goal of the implementation is to provide an vNVRAM storage for
a software implementation of a TPM to store its different blobs into.
Some of the data that the TPM writes into persistent memory needs to
survive a power down / power up cycle of a virtual machine, therefore
this type of persistent storage is needed. For the vNVRAM not to become
a road-block for VM migration, we would make use of block device
migration and layer the vNVRAM on top of the block device, therefore
using virtual machine images for storing the vNVRAM data.

Besides the TPM blobs the vNVRAM should of course also be able able to
accommodate other use cases where persistent data is stored into
NVRAM,

Well let's focus more on the blob store.  What are the semantics of
this?  Is there a max number of blobs?  Are the sizes fixed or variable?
How often are new blobs added/removed?


The max number of blobs and frequency of usage depends on the usage 
scenario and NVRAM size.  But that's probably obvious.


I think we should focus on worst case scenarios where NVRAM is filled up 
and used frequently.


One example is that an application can use TSS APIs to define, undefine, 
read, and write to the TPM's NVRAM storage.  (The TPM owner password is 
required to define NVRAM data.)  An application could potentially fill 
up NVRAM and frequently store, change, retrieve data in various places 
within NVRAM.  And the data could have various sizes.


For an example of total NVRAM size, Infineon's TPM has 16K of NVRAM.

--
Regards,
Corey Bryant



In case of TPM 1.2 there are 3 blobs that can be written at different
times for different reasons.

Examples: As with a real-world TPM users loading an owner-evict key into
the TPM will cause the TPM to write that owner-evict key into is own
NVRAM. This key survives a power-off of the machine. Further, the TPM
models its own NVRAM slots. Someone writing into this type of memory
will cause data to be written into the NVRAM. There are other commands
that the TPM offers that will cause data to be written into NVRAM which
users can invoke at any time.

The sizes of the NVRAM blobs of the TPM at least vary in size but I
handle this in the TPM emulation to pad them to fixed size. Depending on
how many such owner-evict keys are loaded into the TPM its permanent
state blob size may vary. Other devices may act differently.

We have a-priori knowledge about  the 3 different types of blobs the TPM
device produces. They are 'registered' once at the beginning (see API)
and are not 'removed' as such.

Regards,
 Stefan






Re: [Qemu-devel] [PATCH 05/12] target-i386: push hot-plugged VCPU state to KVM and unstop it

2013-03-27 Thread Paolo Bonzini
Il 27/03/2013 16:16, Igor Mammedov ha scritto:
 yep, I re-factored every *cpu_synchronize_post*() call,
 
 but considering an intention to call cpu_synchronize_post_init() from
 qom/cpu.c this patch won't work nice since it will pull with itself
 kvm-stub.o to *-user target.
 
 Due to qom/cpu.c is built only once for both softmmu and *-user targets, I
 consider to move cpu_synchronize_post_init()  cpu_synchronize_post_reset()
 from include/sysemu/kvm.h into include/sysemu/cpus.h with definition moved
 into cpus.c + stubs for cpu_synchronize_post_init() resume_vcpu() in
 libqemustub for *-user target.
 Adding stubs to libqemustub could be avoided if resume_vcpu() and
 cpu_synchronize_post_init() are called from x86_cpu_realizefn()
 at the cost of some ifdeffenery in include/sysemu/cpus.h though.
 
 But moving resume_vcpu()  cpu_synchronize_post_init() into qom/cpu.c looks
 like good candidate for being reused by other targets. 
 
 Paolo,
   would it be acceptable to add resume_vcpu()  cpu_synchronize_post_init()
   stubs into libqemustub? 

Can you instead add all of kvm-stub.c?

Paolo



Re: [Qemu-devel] vNVRAM / blobstore design

2013-03-27 Thread Corey Bryant



On 03/27/2013 11:17 AM, Corey Bryant wrote:



On 03/25/2013 06:20 PM, Stefan Berger wrote:

On 03/25/2013 06:05 PM, Anthony Liguori wrote:

Stefan Berger stef...@linux.vnet.ibm.com writes:


[argh, just posted this to qemu-trivial -- it's not trivial]


Hello!

I am posting this message to revive the previous discussions about the
design of vNVRAM / blobstore cc'ing (at least) those that participated
in this discussion 'back then'.

The first goal of the implementation is to provide an vNVRAM storage
for
a software implementation of a TPM to store its different blobs into.
Some of the data that the TPM writes into persistent memory needs to
survive a power down / power up cycle of a virtual machine, therefore
this type of persistent storage is needed. For the vNVRAM not to become
a road-block for VM migration, we would make use of block device
migration and layer the vNVRAM on top of the block device, therefore
using virtual machine images for storing the vNVRAM data.

Besides the TPM blobs the vNVRAM should of course also be able able to
accommodate other use cases where persistent data is stored into
NVRAM,

Well let's focus more on the blob store.  What are the semantics of
this?  Is there a max number of blobs?  Are the sizes fixed or variable?
How often are new blobs added/removed?


The max number of blobs and frequency of usage depends on the usage
scenario and NVRAM size.  But that's probably obvious.

I think we should focus on worst case scenarios where NVRAM is filled up
and used frequently.

One example is that an application can use TSS APIs to define, undefine,
read, and write to the TPM's NVRAM storage.  (The TPM owner password is
required to define NVRAM data.)  An application could potentially fill
up NVRAM and frequently store, change, retrieve data in various places
within NVRAM.  And the data could have various sizes.

For an example of total NVRAM size, Infineon's TPM has 16K of NVRAM.

--
Regards,
Corey Bryant



I just wanted to add that we could really use some direction on which 
way the community would prefer we go with this.  The 2 options that are 
on the table at the moment for encoding/decoding the vNVRAM byte stream 
are BER or JSON visitors.


--
Regards,
Corey Bryant



In case of TPM 1.2 there are 3 blobs that can be written at different
times for different reasons.

Examples: As with a real-world TPM users loading an owner-evict key into
the TPM will cause the TPM to write that owner-evict key into is own
NVRAM. This key survives a power-off of the machine. Further, the TPM
models its own NVRAM slots. Someone writing into this type of memory
will cause data to be written into the NVRAM. There are other commands
that the TPM offers that will cause data to be written into NVRAM which
users can invoke at any time.

The sizes of the NVRAM blobs of the TPM at least vary in size but I
handle this in the TPM emulation to pad them to fixed size. Depending on
how many such owner-evict keys are loaded into the TPM its permanent
state blob size may vary. Other devices may act differently.

We have a-priori knowledge about  the 3 different types of blobs the TPM
device produces. They are 'registered' once at the beginning (see API)
and are not 'removed' as such.

Regards,
 Stefan








Re: [Qemu-devel] [PATCH 08/12] introduce CPU hot-plug notifier

2013-03-27 Thread Igor Mammedov
On Wed, 27 Mar 2013 12:06:10 +0100
Paolo Bonzini pbonz...@redhat.com wrote:

 Il 21/03/2013 15:28, Igor Mammedov ha scritto:
  hot-added CPU id (APIC ID) will be distributed to acpi_piix4 and rtc_cmos
  
  Signed-off-by: Igor Mammedov imamm...@redhat.com
  ---
   include/sysemu/sysemu.h |4 
   stubs/Makefile.objs |1 +
   stubs/qemu_system_cpu_hotplug_request.c |5 +
   vl.c|   14 ++
   4 files changed, 24 insertions(+), 0 deletions(-)
   create mode 100644 stubs/qemu_system_cpu_hotplug_request.c
  
  diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
  index 6578782..4b8f721 100644
  --- a/include/sysemu/sysemu.h
  +++ b/include/sysemu/sysemu.h
  @@ -152,6 +152,10 @@ void do_pci_device_hot_remove(Monitor *mon, const
  QDict *qdict); /* generic hotplug */
   void drive_hot_add(Monitor *mon, const QDict *qdict);
   
  +/* CPU hotplug */
  +void qemu_register_cpu_add_notifier(Notifier *notifier);
  +void qemu_system_cpu_hotplug_request(uint32_t id);
  +
   /* pcie aer error injection */
   void pcie_aer_inject_error_print(Monitor *mon, const QObject *data);
   int do_pcie_aer_inject_error(Monitor *mon,
  diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
  index 9741e16..6a492f5 100644
  --- a/stubs/Makefile.objs
  +++ b/stubs/Makefile.objs
  @@ -25,3 +25,4 @@ stub-obj-y += vmstate.o
   stub-obj-$(CONFIG_WIN32) += fd-register.o
   stub-obj-y += resume_vcpu.o
   stub-obj-y += get_icc_bus.o
  +stub-obj-y += qemu_system_cpu_hotplug_request.o
 
 You're adding one stub per patch.  I think this is a sign that something
 can be abstracted at a higher level (e.g. put something in cpus.c if it
 is softmmu-specific), or that it is added at the wrong place.
I've put notifier in vl.c since most of them are there and it doesn't make
much difference if it is in cpus.c, but stub is to reduce ifdeffenery in
headers, although target-i386/cpu.c is build per target so I could use
ifdef in include/sysemu/cpus.h
===
#ifndef CONFIG_USER_ONLY
void qemu_system_cpu_hotplug_request(uint32_t id);
#esle
static inline void qemu_system_cpu_hotplug_request(uint32_t id)
{
}
#endif
===

 
 For example, this notifier can go in qom/cpu.c.
Yep there wouldn't be need for stub if notifier is in qom/cpu.c,
but I've figured out that people would object to put it there
since it's build only once for softmmu and *-user targets and
*-user target doesn't need it at all.

Andreas,
 would it be acceptable if notifier goes in qom/cpu.c, (it would
 add nop code to *-user target)?

 
 (Besides, I noticed now the get_icc_bus stub.  I didn't understand why
 it's used, but anyway adding CPU-specific stuff to libqemustub is
 absolutely a no-no).

True, If icc_bus was created at board level then there wouldn't be any need
for get_icc_bus(), it could be just looked up in qom tree. I'll try to do it.

BTW: is there any guidelines what might be added to libqemustub?
 
 Paolo




Re: [Qemu-devel] [PATCH v2] QOM-ify the TPM support

2013-03-27 Thread Paolo Bonzini
Il 27/03/2013 13:21, Stefan Berger ha scritto:
 
 With the above file naming and directory placement I followed the
 pattern of
 
 backends/rng.c
 include/qemu/rng.h
 
 So are you planning on having them renamed and moved as well?

Uff, we're really bad at consistent naming. :)

Given the above, I guess backends/tpm.c is fine.  Then let's do the
following:

   include/tpm/tpm.h - include/sysemu/tpm.h
   include/qemu/rng.h - include/backends/rng.h
   your new include - include/backends/tpm.h

 My intention was to have tpm_passthrough moved into backends/.

I'm not sure that is the right thing to do since tpm_passthrough has
dependencies on DeviceState.  It is not a pure backend, and if I
understand correctly it would likely not work with other TPM front-ends
than tpm_tis.c

 There's a file tpm/tpm_backend.c -- you want me to rename this one even
 though its located in a different directory?

tpm_backend.c and tpm_backend.h seem misnamed to begin with, so that
would be a separate but welcome change.

 Since I am not running a git repository any move/rename would be a
 deletion of a file plus its addition.

I don't understand.

Paolo



Re: [Qemu-devel] vNVRAM / blobstore design

2013-03-27 Thread Michael S. Tsirkin
On Wed, Mar 27, 2013 at 11:20:43AM -0400, Corey Bryant wrote:
 
 
 On 03/27/2013 11:17 AM, Corey Bryant wrote:
 
 
 On 03/25/2013 06:20 PM, Stefan Berger wrote:
 On 03/25/2013 06:05 PM, Anthony Liguori wrote:
 Stefan Berger stef...@linux.vnet.ibm.com writes:
 
 [argh, just posted this to qemu-trivial -- it's not trivial]
 
 
 Hello!
 
 I am posting this message to revive the previous discussions about the
 design of vNVRAM / blobstore cc'ing (at least) those that participated
 in this discussion 'back then'.
 
 The first goal of the implementation is to provide an vNVRAM storage
 for
 a software implementation of a TPM to store its different blobs into.
 Some of the data that the TPM writes into persistent memory needs to
 survive a power down / power up cycle of a virtual machine, therefore
 this type of persistent storage is needed. For the vNVRAM not to become
 a road-block for VM migration, we would make use of block device
 migration and layer the vNVRAM on top of the block device, therefore
 using virtual machine images for storing the vNVRAM data.
 
 Besides the TPM blobs the vNVRAM should of course also be able able to
 accommodate other use cases where persistent data is stored into
 NVRAM,
 Well let's focus more on the blob store.  What are the semantics of
 this?  Is there a max number of blobs?  Are the sizes fixed or variable?
 How often are new blobs added/removed?
 
 The max number of blobs and frequency of usage depends on the usage
 scenario and NVRAM size.  But that's probably obvious.
 
 I think we should focus on worst case scenarios where NVRAM is filled up
 and used frequently.
 
 One example is that an application can use TSS APIs to define, undefine,
 read, and write to the TPM's NVRAM storage.  (The TPM owner password is
 required to define NVRAM data.)  An application could potentially fill
 up NVRAM and frequently store, change, retrieve data in various places
 within NVRAM.  And the data could have various sizes.
 
 For an example of total NVRAM size, Infineon's TPM has 16K of NVRAM.
 
 --
 Regards,
 Corey Bryant
 
 
 I just wanted to add that we could really use some direction on
 which way the community would prefer we go with this.  The 2 options
 that are on the table at the moment for encoding/decoding the vNVRAM
 byte stream are BER or JSON visitors.
 
 -- 
 Regards,
 Corey Bryant

I think I like BER better. JSON seems like a bad fit for a
bunch of binary blobs.

 
 In case of TPM 1.2 there are 3 blobs that can be written at different
 times for different reasons.
 
 Examples: As with a real-world TPM users loading an owner-evict key into
 the TPM will cause the TPM to write that owner-evict key into is own
 NVRAM. This key survives a power-off of the machine. Further, the TPM
 models its own NVRAM slots. Someone writing into this type of memory
 will cause data to be written into the NVRAM. There are other commands
 that the TPM offers that will cause data to be written into NVRAM which
 users can invoke at any time.
 
 The sizes of the NVRAM blobs of the TPM at least vary in size but I
 handle this in the TPM emulation to pad them to fixed size. Depending on
 how many such owner-evict keys are loaded into the TPM its permanent
 state blob size may vary. Other devices may act differently.
 
 We have a-priori knowledge about  the 3 different types of blobs the TPM
 device produces. They are 'registered' once at the beginning (see API)
 and are not 'removed' as such.
 
 Regards,
  Stefan
 
 



Re: [Qemu-devel] [PATCH] chardev-frontends: Explicitly check, inc and dec avail_connections

2013-03-27 Thread Hans de Goede

Hi,

On 03/27/2013 04:11 PM, Paolo Bonzini wrote:

snip


diff --git a/backends/rng-egd.c b/backends/rng-egd.c
index 5e012e9..d8e9d63 100644
--- a/backends/rng-egd.c
+++ b/backends/rng-egd.c
@@ -149,6 +149,12 @@ static void rng_egd_opened(RngBackend *b, Error **errp)
  return;
  }

+if (s-chr-avail_connections  1) {
+error_set(errp, QERR_DEVICE_IN_USE, s-chr_name);
+return;
+}
+s-chr-avail_connections--;
+
  /* FIXME we should resubmit pending requests when the CDS reconnects. */
  qemu_chr_add_handlers(s-chr, rng_egd_chr_can_read, rng_egd_chr_read,
NULL, s);
@@ -191,6 +197,7 @@ static void rng_egd_finalize(Object *obj)

  if (s-chr) {
  qemu_chr_add_handlers(s-chr, NULL, NULL, NULL, NULL);
+s-chr-avail_connections++;
  }

  g_free(s-chr_name);


Ok, but please create wrappers for these (e.g. qemu_chr_be_start/stop
and qemu_chr_be_start_nofail) and use them throughout.


That would be fe_start fe_stop, ack otherwise.


diff --git a/gdbstub.c b/gdbstub.c
index a666cb5..83267e0 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -3025,6 +3025,7 @@ int gdbserver_start(const char *device)
  if (!chr)
  return -1;

+chr-avail_connections--;
  qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive,
gdb_chr_event, NULL);
  }


Ok.


diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index 7467cca..df4b458 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -1981,9 +1981,16 @@ static PXA2xxFIrState *pxa2xx_fir_init(MemoryRegion 
*sysmem,
  memory_region_init_io(s-iomem, pxa2xx_fir_ops, s, pxa2xx-fir, 
0x1000);
  memory_region_add_subregion(sysmem, base, s-iomem);

-if (chr)
+if (chr) {
+if (chr-avail_connections  1) {
+fprintf(stderr, pxa2xx_fir_init error chardev %s already used\n,
+chr-label);
+exit(1);
+}
+chr-avail_connections--;
  qemu_chr_add_handlers(chr, pxa2xx_fir_is_empty,
  pxa2xx_fir_rx, pxa2xx_fir_event, s);
+}

  register_savevm(NULL, pxa2xx_fir, 0, 0, pxa2xx_fir_save,
  pxa2xx_fir_load, s);


Errors won't be reported, because serial_hds[] will always create its
own CharDriverState and avail_connections will always be 1.  Use a
wrapper and the code can ignore this.


Unless some smartass adds, ie: -mon chardev=serial0 to the cmdline, then an
error will be reported.

I'll respin the patch taking your comments into account.

Regards,

Hans



Re: [Qemu-devel] [PATCH 08/12] introduce CPU hot-plug notifier

2013-03-27 Thread Paolo Bonzini
Il 27/03/2013 16:24, Igor Mammedov ha scritto:
 I've put notifier in vl.c since most of them are there

They are there, because the code that invokes them is also there.  In
fact, most calls of notifier_list_notify in vl.c are from static functions.

 Yep there wouldn't be need for stub if notifier is in qom/cpu.c,
 but I've figured out that people would object to put it there
 since it's build only once for softmmu and *-user targets and
 *-user target doesn't need it at all.
 
 Andreas,
  would it be acceptable if notifier goes in qom/cpu.c, (it would
  add nop code to *-user target)?

I think adding dead code to *-user is fine.

  (Besides, I noticed now the get_icc_bus stub.  I didn't understand why
  it's used, but anyway adding CPU-specific stuff to libqemustub is
  absolutely a no-no).
 True, If icc_bus was created at board level then there wouldn't be any need
 for get_icc_bus(), it could be just looked up in qom tree. I'll try to do it.
 
 BTW: is there any guidelines what might be added to libqemustub?

Nothing. :)

Seriously, you really should use it only when an entire subsystem does
not exist in tools or user-level emulation (monitor, vm_clock,
migration, slirp).  Everything else will probably be served better by
methods, in all likelihood.

Paolo



Re: [Qemu-devel] [PATCH] chardev-frontends: Explicitly check, inc and dec avail_connections

2013-03-27 Thread Paolo Bonzini
Il 27/03/2013 16:36, Hans de Goede ha scritto:
 Unless some smartass adds, ie: -mon chardev=serial0 to the cmdline, then an
 error will be reported.

Right. :)  For smartasses we can use qemu_chr_fe_start_nofail. :)

Paolo



Re: [Qemu-devel] dataplane bug: fail to start Windows VM with dataplane enable

2013-03-27 Thread Stefan Hajnoczi
On Tue, Mar 26, 2013 at 11:10:35PM +0800, 张磊强 wrote:
 Hi,  Paolo  Stefan:
 
 When I test the dataplane feature with qemu master, I find that
 Windows (Windows 7 and Windows 2003) VM will hang if dataplane is
 enabled. But if I try to start a Fedora VM, it can start normally.
 
 The command I boot QEMU is:
 x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 1024 -smp 2 -drive
 file=win7.img,if=none,id=drive-virtio-disk,format=raw,cache=none,aio=native
 -device 
 virtio-blk-pci,config-wce=off,scsi=off,x-data-plane=on,drive=drive-virtio-disk,id=virtio-disk
 
 I found the similar bug has reported some days ago:
 http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg02200.html
 . And a patch for this bug has already committed by Paolo at Mar 13:
 http://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg02200.html
 .
 
 But it cannot work under my environment. Could you give me some advise
 to debug this problem ? I can provide more information if need.

I sent a fix and CCed you on the patch.  Please test it if you have
time.

Stefan



Re: [Qemu-devel] [PATCH v3] block: Add support for Secure Shell (ssh) block device.

2013-03-27 Thread Richard W.M. Jones
On Wed, Mar 27, 2013 at 10:12:42AM -0500, Anthony Liguori wrote:
 Richard W.M. Jones rjo...@redhat.com writes:
 
  From: Richard W.M. Jones rjo...@redhat.com
 
qemu-system-x86_64 -drive file=ssh://hostname/some/image
 
  QEMU will ssh into 'hostname' and open '/some/image' which is made
  available as a standard block device.
 
  You can specify a username (ssh://user@host/...) and/or a port number
  (ssh://host:port/...).
 
  Current limitations:
 
  - Authentication must be done without passwords or passphrases, using
ssh-agent.  Other authentication methods are not supported. (*)
 
  - New remote files cannot be created. (*)
 
  - Uses a single connection, instead of concurrent AIO with multiple
SSH connections.
 
  (*) = potentially easy fix
 
  This is implemented using libssh2 on the client side.  The server just
  requires a regular ssh daemon with sftp-server support.  Most ssh
  daemons on Unix/Linux systems will work out of the box.
 
  Thanks: Stefan Hajnoczi, Kevin Wolf.
 
 Curl actually supports sftp already.  In theory, we just need to add:
 
 static BlockDriver bdrv_sftp = {
 .format_name = sftp,
 .protocol_name   = sftp,
 
 .instance_size   = sizeof(BDRVCURLState),
 .bdrv_file_open  = curl_open,
 .bdrv_close  = curl_close,
 .bdrv_getlength  = curl_getlength,
 
 .bdrv_aio_readv  = curl_aio_readv,
 };
 
 To block/curl.c and it should Just Work.   Have you considered doing
 this through curl?

Yes, and it doesn't work.  See:

http://www.mail-archive.com/qemu-devel@nongnu.org/msg162605.html
(curl-based patch: 
http://www.mail-archive.com/qemu-devel@nongnu.org/msg162253.html )

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/



Re: [Qemu-devel] [PATCH V14 5/5] VMXNET3 device implementation

2013-03-27 Thread Paolo Bonzini
Il 27/03/2013 15:49, Alexander Graf ha scritto:
  +#if defined(HOST_WORDS_BIGENDIAN)
  +#define const_cpu_to_le64(x) bswap_64(x)
  +#define __BIG_ENDIAN_BITFIELD
 Ah, sorry, I replied to the wrong version.
 
 ARE YOU KIDDING ME? BIG ENDIAN BITFIELD? BITFIELDS ARE _IMPLEMENTATION 
 SPECIFIC_!
 
 Can we please revert this whole patch set and send the authors back to school?

Can we please maintain a decent tone?

First, this file comes from Linux.  __BIG_ENDIAN_BITFIELD is a Linux
#define.  No doubt it is wrong to define it based on
HOST_WORDS_BIGENDIAN, it is better to use a configure check.  But it's
not the reason why PPC compilation fails.

Second, you haven't said _how_ it breaks PPC compilation.  Just
cut-and-paste from the compiler is enough.  Ok, I can guess it but not
always.

Third, there is no need to revert the patch set.  The const_cpu_to_le64
should simply be removed, since little-endian conversion is already done
in vmw_shmem_ld32.

Paolo



[Qemu-devel] [PATCH] virtio-pci: pass real with_irqfd to virtio_pci_set_guest_notifier()

2013-03-27 Thread Stefan Hajnoczi
virtio_pci_set_guest_notifiers() checks whether irqfd can be used and
whether MSI-X is enabled for the PCI adapter.  But then it calls
virtio_pci_set_guest_notifier() and passes kvm_msi_via_irqfd_enabled()
instead of with_irqfd.

When MSI-X is disabled but irqfd is allowed this means that
guest_notifier has neither irqfd nor a
virtio_queue_guest_notifier_read() handler.  Therefore the guest cannot
receive notifications.

This issue is triggered by a Windows 7 Professional 64-bit guest with
-device virtio-blk-pci,x-data-plane=on.  The guest driver does not
enable MSI-X and the guest gets stuck at the Windows boot screen since
it does not receive notifications.

Reported-by: 张磊强 (Leiqiang Zhang) leiqzh...@gmail.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
 hw/virtio-pci.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 736a9bf..84ece51 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -798,8 +798,7 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, 
int nvqs, bool assign)
 break;
 }
 
-r = virtio_pci_set_guest_notifier(d, n, assign,
-  kvm_msi_via_irqfd_enabled());
+r = virtio_pci_set_guest_notifier(d, n, assign, with_irqfd);
 if (r  0) {
 goto assign_error;
 }
-- 
1.8.1.4




  1   2   3   >