date:20130620

On Tue, Jun 18, 2013 at 8:25 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
 On Thu, Jun 13, 2013 at 05:03:02PM +0800, Liu Ping Fan wrote:
 @@ -67,6 +67,10 @@ struct NetClientState {
  NetClientInfo *info;
  int link_down;
  QTAILQ_ENTRY(NetClientState) next;
 +/* protect the race access of peer only between reader and writer.
 + * to resolve the writer's race condition, resort on biglock.
 + */

 Indentation

Will fix.
 @@ -301,6 +303,38 @@ static void qemu_free_net_client(NetClientState *nc)
  }
  }

 +/* elimate the reference and sync with exit of rx/tx action.

 s/elimate/Eliminate/

Will fix
 + * And flush out peer's queue.
 + */
 +static void qemu_net_client_detach_flush(NetClientState *nc)
 +{
 +NetClientState *peer;
 +
 +/* reader of self's peer field , fixme? the deleters are not concurrent,
 + * so this pair lock can save.
 + */

 Indentation, also please resolve the fixme.

So, here can I take the assumption that the deleters are serialized by
biglock, and remove the lock following this comment?

 @@ -394,6 +433,28 @@ int qemu_can_send_packet(NetClientState *sender)
  return 1;
  }

 +int qemu_can_send_packet(NetClientState *sender)
 +{
 +int ret = 1;
 +
 +qemu_mutex_lock(sender-peer_lock);
 +if (!sender-peer) {
 +goto unlock;
 +}
 +
 +if (sender-peer-receive_disabled) {
 +ret = 0;
 +goto unlock;
 +} else if (sender-peer-info-can_receive 
 +   !sender-peer-info-can_receive(sender-peer)) {
 +ret = 0;
 +goto unlock;
 +}

 Just call qemu_can_send_packet_nolock() instead of duplicating code?

Yes.

Thx  regards,
pingfan

Re: [Qemu-devel] [PATCH v2 4/6] net: force NetQue opaque to be NetClientState

On Tue, Jun 18, 2013 at 8:47 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
 On Thu, Jun 13, 2013 at 05:03:04PM +0800, Liu Ping Fan wrote:
 From: Liu Ping Fan pingf...@linux.vnet.ibm.com

 qemu_net_client_setup() is the only user of qemu_new_net_queue(), which
 will pass in NetClientState. By forcing it be a NetClientState, we
 can ref/unref NetQueue's owner

 Please s/opaque/nc/ in net/queue.[hc] since it's no longer opaque :).

Ok, I will.
 Also, qemu_deliver_packet()/qemu_deliver_packet_iov() can take an
 NetClientState *nc instead of void *opaque.

 Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com

 pingfank here and pingfanl in the From: header.  Are both okay and which
 do you prefer to use?

Change my disk, totally re-install my system, and mix up with internal
mail address. Will fix it.

Thx  regards,
Pingfan

Re: [Qemu-devel] [PATCH v2 5/6] net: defer nested call to BH

On Tue, Jun 18, 2013 at 8:57 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
 On Thu, Jun 13, 2013 at 05:03:05PM +0800, Liu Ping Fan wrote:
 From: Liu Ping Fan pingf...@linux.vnet.ibm.com

 Nested call caused by -receive() will raise issue like deadlock,
 so postphone it to BH.

 Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
 ---
  net/queue.c | 40 ++--
  1 file changed, 38 insertions(+), 2 deletions(-)

 Does this patch belong before the netqueue lock patch?  The commit
 history should be bisectable without temporary failures/deadlocks.

Ok.
 diff --git a/net/queue.c b/net/queue.c
 index 58222b0..9c343ab 100644
 --- a/net/queue.c
 +++ b/net/queue.c
 @@ -24,6 +24,8 @@
  #include net/queue.h
  #include qemu/queue.h
  #include net/net.h
 +#include block/aio.h
 +#include qemu/main-loop.h

  /* The delivery handler may only return zero if it will call
   * qemu_net_queue_flush() when it determines that it is once again able
 @@ -183,6 +185,22 @@ static ssize_t qemu_net_queue_deliver_iov(NetQueue 
 *queue,
  return ret;
  }

 +typedef struct NetQueBH {

 This file uses Queue consistently, please don't add Que here.

 @@ -192,8 +210,17 @@ ssize_t qemu_net_queue_send(NetQueue *queue,
  {
  ssize_t ret;

 -if (queue-delivering || !qemu_can_send_packet_nolock(sender)) {
 +if (queue-delivering || !qemu_can_send_packet_nolock(sender)
 +|| sender-send_queue-delivering) {

 Not sure this is safe, we're only holding one NetClientState-peer_lock
 and one NetQueue-lock.  How can we access both queue-delivering and
 sender-send_queue-delivering safely?

Yes, you are right, it is not safely. The queue-delivering is
protected by peer_lock and we do not take the verse direction lock .
So finally the above code can not tell out the nested calling
A--B--A  from  A--B,  B--A (where A, B stands for a
NetClientState).
What about using TLS to trace the nested calling?  With it, we can
avoid AB-BA lock problem.

Thx  regards,
Pingfan

[Qemu-devel] [PATCH v2 0/2] libqtest leak fix cleanup

v2: qtest_start() function comment

Markus Armbruster (2):
  libqtest: Plug fd and memory leaks in qtest_quit()
  libqtest: New qtest_end() to go with qtest_start()

 tests/fdc-test.c|  2 +-
 tests/hd-geo-test.c |  8 
 tests/ide-test.c|  2 +-
 tests/libqtest.c|  4 
 tests/libqtest.h| 12 
 5 files changed, 22 insertions(+), 6 deletions(-)

-- 
1.7.11.7

[Qemu-devel] [PATCH v2 1/2] libqtest: Plug fd and memory leaks in qtest_quit()

Reviewed-by: Anthony Liguori aligu...@us.ibm.com
Signed-off-by: Markus Armbruster arm...@redhat.com
---
 tests/libqtest.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 879ffe9..bb82069 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -171,12 +171,16 @@ void qtest_quit(QTestState *s)
 waitpid(pid, status, 0);
 }
 
+close(s-fd);
+close(s-qmp_fd);
+g_string_free(s-rx, true);
 unlink(s-pid_file);
 unlink(s-socket_path);
 unlink(s-qmp_socket_path);
 g_free(s-pid_file);
 g_free(s-socket_path);
 g_free(s-qmp_socket_path);
+g_free(s);
 }
 
 static void socket_sendf(int fd, const char *fmt, va_list ap)
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 2/2] libqtest: New qtest_end() to go with qtest_start()


Signed-off-by: Markus Armbruster arm...@redhat.com
---
 tests/fdc-test.c|  2 +-
 tests/hd-geo-test.c |  8 
 tests/ide-test.c|  2 +-
 tests/libqtest.h| 12 
 4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index 4b0301d..fd198dc 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -556,7 +556,7 @@ int main(int argc, char **argv)
 ret = g_test_run();
 
 /* Cleanup */
-qtest_quit(global_qtest);
+qtest_end();
 unlink(test_image);
 
 return ret;
diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index 9a31e85..b72042e 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -244,7 +244,7 @@ static void test_ide_none(void)
 setup_common(argv, ARRAY_SIZE(argv));
 qtest_start(g_strjoinv( , argv));
 test_cmos();
-qtest_quit(global_qtest);
+qtest_end();
 }
 
 static void test_ide_mbr(bool use_device, MBRcontents mbr)
@@ -262,7 +262,7 @@ static void test_ide_mbr(bool use_device, MBRcontents mbr)
 }
 qtest_start(g_strjoinv( , argv));
 test_cmos();
-qtest_quit(global_qtest);
+qtest_end();
 }
 
 /*
@@ -334,7 +334,7 @@ static void test_ide_drive_user(const char *dev, bool trans)
 g_free(opts);
 qtest_start(g_strjoinv( , argv));
 test_cmos();
-qtest_quit(global_qtest);
+qtest_end();
 }
 
 /*
@@ -387,7 +387,7 @@ static void test_ide_drive_cd_0(void)
 }
 qtest_start(g_strjoinv( , argv));
 test_cmos();
-qtest_quit(global_qtest);
+qtest_end();
 }
 
 int main(int argc, char **argv)
diff --git a/tests/ide-test.c b/tests/ide-test.c
index 7e2eb94..7307f1d 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -122,7 +122,7 @@ static void ide_test_start(const char *cmdline_fmt, ...)
 
 static void ide_test_quit(void)
 {
-qtest_quit(global_qtest);
+qtest_end();
 }
 
 static QPCIDevice *get_pci_device(uint16_t *bmdma_base)
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 437bda3..0f6aade 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -17,6 +17,7 @@
 #ifndef LIBQTEST_H
 #define LIBQTEST_H
 
+#include stddef.h
 #include stdint.h
 #include stdbool.h
 #include stdarg.h
@@ -319,6 +320,17 @@ static inline QTestState *qtest_start(const char *args)
 }
 
 /**
+ * qtest_end:
+ *
+ * Shut down the QEMU process started by qtest_start().
+ */
+static inline void qtest_end(void)
+{
+qtest_quit(global_qtest);
+global_qtest = NULL;
+}
+
+/**
  * qmp:
  * @fmt...: QMP message to send to qemu
  *
-- 
1.7.11.7

Re: [Qemu-devel] [RFC 06/13] qemu-thread: add TLS wrappers

2013-06-20 Thread Fam Zheng

On Fri, 06/14 11:48, Stefan Hajnoczi wrote:
 From: Paolo Bonzini pbonz...@redhat.com
 
 Fast TLS is not available on some platforms, but it is always nice to
 use it.  This wrapper implementation falls back to pthread_get/setspecific
 on POSIX systems that lack __thread, but uses the dynamic linker's TLS
 support on Linux and Windows.
 
 The user shall call alloc_foo() in every thread that needs to access the
 variable---exactly once and before any access.  foo is the name of the
 variable as passed to DECLARE_TLS and DEFINE_TLS.  Then, get_foo() will
 return the address of the variable.  It is guaranteed to remain the same
 across the lifetime of a thread, so you can cache it.

Would tls_alloc_foo() and tls_get_foo() be easier to read and less
possible for name conflict?

Fam

Re: [Qemu-devel] [Xen-devel] [PATCH] Remove hardcoded xen-platform device initialization

 -Original Message-
 From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com]
 Sent: 19 June 2013 17:28
 To: Paul Durrant
 Cc: Stefano Stabellini; Ian Campbell; Paolo Bonzini; xen-de...@lists.xen.org;
 qemu-devel@nongnu.org
 Subject: RE: [Qemu-devel] [Xen-devel] [PATCH] Remove hardcoded xen-
 platform device initialization

 On Wed, 19 Jun 2013, Paul Durrant wrote:
   -Original Message-
   From: qemu-devel-bounces+paul.durrant=citrix@nongnu.org
   [mailto:qemu-devel-bounces+paul.durrant=citrix@nongnu.org] On
   Behalf Of Stefano Stabellini
   Sent: 19 June 2013 14:53
   To: Ian Campbell
   Cc: Paolo Bonzini; Paul Durrant; xen-de...@lists.xen.org; qemu-
   de...@nongnu.org; Stefano Stabellini
   Subject: Re: [Qemu-devel] [Xen-devel] [PATCH] Remove hardcoded xen-
   platform device initialization

   On Wed, 19 Jun 2013, Ian Campbell wrote:
On Tue, 2013-06-18 at 19:56 +0100, Stefano Stabellini wrote:
 On Fri, 14 Jun 2013, Paul Durrant wrote:
   -Original Message-
   From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf
 Of
   Paolo
   Bonzini
   Sent: 14 June 2013 15:58
   To: Paul Durrant
   Cc: Ian Campbell; Stefano Stabellini; qemu-devel@nongnu.org;
 xen-
   de...@lists.xen.org
   Subject: Re: [Xen-devel] [PATCH] Remove hardcoded xen-
 platform
   device
   initialization

   Il 14/06/2013 10:11, Paul Durrant ha scritto:
I think we're still going to need -M xenpv, I think; it's quite
distinct from pc.

   Of course!  Even more: -M xenpv should be reused on ARM.

I guess we could use -M pc for HVM and gate the
accel code as you suggest but, if that's the way we're going, it
would seem more logical just to ditch the accel code for xenpv
completely (assuming we can do all we need from the machine
 init)
   and
then use -M pc -accel=xen for HVM guests going forward.

   There is common code between pv and fv, and that one definitely
   belongs
   in xen_init.  Most fv-only code probably should be in pc_init.  
   The
 rest
   should move to xen_init though, because it would apply just as
 well
   for
   example to Q35.  It's a bit ugly to have fv-only code there, but 
   it's
   better than having a Xen-specific machine type.  Xen/KVM/TCG
   should be
   as similar as possible at the QEMU level, any difference should be
   handled in the toolstack.

But that does
rather screw up my autodiscovery plans because I would not
 know,
   for
a given qemu binary, which machine type to use.

   There's no need for that.  4.4 can just use -M pc 
   unconditionally,
   =4.3 will just use -M xenfv unconditionally.

If I create a new
xenfv-2.0 machine type though I *can* do auto discovery... in
 which
case do we need the -accel=xen option at all?

   Yes.  Please try not do things differently from other 
   accelerators.

  Ok. I guess we can have the ability to override the machine type in
 the
   VM config, so you could still kick off an older qemu with a newer libxl -
 but it
   sounds like the auto-discovery idea is a no-go then.

 xenfv-2.0 is a bad idea, like Paolo wrote, it should be possible to 
 just
 use -M pc for HVM guests and retain -M xenpv for pv guests.

 However it seems to me that we also need a way in libxl to find out
 whether QEMU is new enough for us to be able to use -M pc.
 We can't just assume that users will be able to figure out the magic
 rune they need to write in the VM config file to solve their VM crash
 at
 boot problem.

What crash at boot problem?

   If you start QEMU as device model on Xen with the wrong machine
 option
   (for example -M pc on an old QEMU), QEMU would probably just abort
   during initialization.

 We could spawn an instance of QEMU just to figure out the QEMU
   version
 but we certainly cannot do that every time we start a new VM.
 Once we figure out the QEMU version the first time we could write it
 to
 xenstore so that the next time we don't have to go through the same
 process again.

Due to the device_model_override we might need to make this per-
 path.
You'd also likely need to store mtime or something in case qemu gets
upgraded, although perhaps that is getting unnecessarily picky...

   I think of device_model_override as an option for developers. People
   that use device_model_override can also override the QEMUMachine
   version.

  Are you suggesting we allow a freeform -machine option in libxl, or are you
 suggesting they point device_model_override at a script which drops the -M
 argument and inserts their new choice before invoking qemu?

 I am suggesting that we could have a qemu_machine_override option in
 QEMU, or maybe a

Re: [Qemu-devel] [PATCH v3 2/2] QEMUBH: make AioContext's bh re-entrant

On Thu, Jun 20, 2013 at 04:59:29AM +0800, Liu Ping Fan wrote:
 BH will be used outside big lock, so introduce lock to protect
 between the writers, ie, bh's adders and deleter. The lock only
 affects the writers and bh's callback does not take this extra lock.
 Note that for the same AioContext, aio_bh_poll() can not run in
 parallel yet.
 
 Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
 ---
  async.c | 22 ++
  include/block/aio.h |  5 +
  2 files changed, 27 insertions(+)

qemu_bh_cancel() and qemu_bh_delete() are not modified by this patch.

It seems that calling them from a thread is a little risky because there
is no guarantee that the BH is no longer invoked after a thread calls
these functions.

I think that's worth a comment or do you want them to take the lock so
they become safe?

The other thing I'm unclear on is the -idle assignment followed
immediately by a -scheduled assignment.  Without memory barriers
aio_bh_poll() isn't guaranteed to get an ordered view of these updates:
it may see an idle BH as a regular scheduled BH because -idle is still
0.

Stefan

Re: [Qemu-devel] [PATCH v2 2/6] net: introduce lock to protect NetClientState's peer's access

On Thu, Jun 20, 2013 at 02:30:30PM +0800, liu ping fan wrote:
 On Tue, Jun 18, 2013 at 8:25 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
  On Thu, Jun 13, 2013 at 05:03:02PM +0800, Liu Ping Fan wrote:
  + * And flush out peer's queue.
  + */
  +static void qemu_net_client_detach_flush(NetClientState *nc)
  +{
  +NetClientState *peer;
  +
  +/* reader of self's peer field , fixme? the deleters are not 
  concurrent,
  + * so this pair lock can save.
  + */
 
  Indentation, also please resolve the fixme.
 
 So, here can I take the assumption that the deleters are serialized by
 biglock, and remove the lock following this comment?

Ah, I understand the comment now.  Is there any advantage to dropping
the lock?  IMO it's clearer to take the lock consistently instead of
optimizing cases we think only get called from the main loop.

Re: [Qemu-devel] [Xen-devel] [PATCH] Add Xen platform PCI device version 2.

 -Original Message-
 From: Tim Deegan [mailto:t...@xen.org]
 Sent: 19 June 2013 21:15
 To: Matt Wilson
 Cc: Alex Bligh; Paul Durrant; xen-de...@lists.xen.org; Ian Campbell; qemu-
 de...@nongnu.org
 Subject: Re: [Xen-devel] [Qemu-devel] [PATCH] Add Xen platform PCI device
 version 2.

 At 11:21 -0700 on 19 Jun (1371640904), Matt Wilson wrote:
  On Wed, Jun 19, 2013 at 11:42:06AM +0100, Alex Bligh wrote:

   --On 19 June 2013 10:13:17 + Paul Durrant
   paul.durr...@citrix.com wrote:

   We obviously can't say to users Are you running Windows and are you
   running PV drivers = X.Y, if so set lever A to position B, otherwise if
   you are running some other OS or an earlier version of the Windows
 PV
   driver set it to position A.

   Why not? The device can be chosen on a per-VM basis.

   Not everyone knows what guest some random user will be running
   (consider cloud platforms).

  I agree. If this is really the only solution, we would need to have
  both versions presented to the guest so that old drivers continue to
  work without any intervention.

 I suspect that if we expose both, both sets of drivers try to run the
 same PV connections, and hilarity ensues.

Actually I think I can make that work, and it is the conclusion I came to after 
Alex's comment. I'll create a new patch which introduces a new device, let's 
call it citrix-pv-bus or somesuch, which will have the necessary device id and 
revision and will be a dedicate device purely for the Citrix PV drivers. Then, 
if someone wants to create a VM which will be able use Citrix PV drivers they 
add this device to their config but leave all other aspects of the config 
unchanged, thus not precluding using that VM with any drivers that bind to the 
xen platform device.
If someone has a VM that has the old Citrix drivers installed, or GPLPV, I 
think I should be able to spot this and make sure that the new bus driver 
quiesces itself to prevent strangeness ensuing. If and when said previous 
drivers are un-installed then the new bus driver can wake up and enumerate the 
device nodes for the other pv drivers and Windows Update can carry on doing its 
stuff.

  Paul

Re: [Qemu-devel] [PATCH v2 5/6] net: defer nested call to BH

On Thu, Jun 20, 2013 at 02:30:56PM +0800, liu ping fan wrote:
 On Tue, Jun 18, 2013 at 8:57 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
  On Thu, Jun 13, 2013 at 05:03:05PM +0800, Liu Ping Fan wrote:
  From: Liu Ping Fan pingf...@linux.vnet.ibm.com
 
  Nested call caused by -receive() will raise issue like deadlock,
  so postphone it to BH.
 
  Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
  ---
   net/queue.c | 40 ++--
   1 file changed, 38 insertions(+), 2 deletions(-)
 
  Does this patch belong before the netqueue lock patch?  The commit
  history should be bisectable without temporary failures/deadlocks.
 
 Ok.
  diff --git a/net/queue.c b/net/queue.c
  index 58222b0..9c343ab 100644
  --- a/net/queue.c
  +++ b/net/queue.c
  @@ -24,6 +24,8 @@
   #include net/queue.h
   #include qemu/queue.h
   #include net/net.h
  +#include block/aio.h
  +#include qemu/main-loop.h
 
   /* The delivery handler may only return zero if it will call
* qemu_net_queue_flush() when it determines that it is once again able
  @@ -183,6 +185,22 @@ static ssize_t qemu_net_queue_deliver_iov(NetQueue 
  *queue,
   return ret;
   }
 
  +typedef struct NetQueBH {
 
  This file uses Queue consistently, please don't add Que here.
 
  @@ -192,8 +210,17 @@ ssize_t qemu_net_queue_send(NetQueue *queue,
   {
   ssize_t ret;
 
  -if (queue-delivering || !qemu_can_send_packet_nolock(sender)) {
  +if (queue-delivering || !qemu_can_send_packet_nolock(sender)
  +|| sender-send_queue-delivering) {
 
  Not sure this is safe, we're only holding one NetClientState-peer_lock
  and one NetQueue-lock.  How can we access both queue-delivering and
  sender-send_queue-delivering safely?
 
 Yes, you are right, it is not safely. The queue-delivering is
 protected by peer_lock and we do not take the verse direction lock .
 So finally the above code can not tell out the nested calling
 A--B--A  from  A--B,  B--A (where A, B stands for a
 NetClientState).
 What about using TLS to trace the nested calling?  With it, we can
 avoid AB-BA lock problem.

I would take a step back and see if there's a way to avoid reaching into
inspect sender-send_queue-delivering here.

Stefan

Re: [Qemu-devel] [PATCH v2 0/5] qcow2: Discard freed clusters

On Wed, Jun 19, 2013 at 01:44:16PM +0200, Kevin Wolf wrote:
 This series adds options to make qcow2 discard freed clusters, in several
 categories. By default, only freed clusters related to snapshots (i.e. mainly
 snapshot deletion) are discarded.
 
 v2:
 - Removed leftover debug code
 - Don't discard after COW (overwriting compressed clusters)
 - Changed some commas into semicolons
 
 Kevin Wolf (5):
   Revert block: Disable driver-specific options for 1.5
   qcow2: Add refcount update reason to all callers
   qcow2: Options to enable discard for freed clusters
   qcow2: Batch discards
   block: Always enable discard on the protocol level
 
  block.c  |   2 +-
  block/qcow2-cluster.c|  41 ++
  block/qcow2-refcount.c   | 136 
 +++
  block/qcow2-snapshot.c   |   6 ++-
  block/qcow2.c|  30 ++-
  block/qcow2.h|  32 +--
  blockdev.c   | 118 ++--
  tests/qemu-iotests/group |   2 +-
  8 files changed, 214 insertions(+), 153 deletions(-)
 
 -- 
 1.8.1.4
 

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

Stefan

Re: [Qemu-devel] [PATCH 0/3] qapi: Top-level type reference for command definitions

On Wed, Jun 19, 2013 at 06:28:04PM +0200, Kevin Wolf wrote:
 Kevin Wolf (3):
   qapi.py: Move common code to evaluate()
   qapi.py: Allow top-level type reference for command definitions
   qapi-schema: Use BlockdevSnapshot type for blockdev-snapshot-sync
 
  qapi-schema.json |  3 +--
  scripts/qapi.py  | 43 +--
  2 files changed, 30 insertions(+), 16 deletions(-)

Nice, I'll use this for drive-backup.

Thanks,
Stefan

Re: [Qemu-devel] Java volatile vs. C11 seq_cst (was Re: [PATCH v2 1/2] add a header file for atomic operations)

Il 19/06/2013 22:25, Torvald Riegel ha scritto:
On Wed, 2013-06-19 at 17:14 +0200, Paolo Bonzini wrote:
(1) I don't care about relaxed RMW ops (loads/stores occur in hot paths,
but RMW shouldn't be that bad. I don't care if reference counting is a
little slower than it could be, for example);

I doubt relaxed RMW ops are sufficient even for reference counting.

They are enough on the increment side, or so says boost...

http://www.chaoticmind.net/~hcb/projects/boost.atomic/doc/atomic/usage_examples.html#boost_atomic.usage_examples.example_reference_counters

[An aside: Java guarantees that volatile stores are not reordered
with volatile loads. This is not guaranteed by just using release
stores and acquire stores, and is why IIUC acq_rel Java seq_cst].

Or maybe Java volatile is acq for loads and seq_cst for stores...

Perhaps (but I'm not 100% sure).

As long as you only have a producer and a consumer, C11 is fine, because
all you need is load-acquire/store-release. In fact, if it weren't for
the experience factor, C11 is easier than manually placing acquire and
release barriers. But as soon as two or more threads are reading _and_
writing the shared memory, it gets complicated and I want to provide
something simple that people can use. This is the reason for (2) above.

I can't quite follow you here. There is a total order for all
modifications to a single variable, and if you use acq/rel combined with
loads and stores on this variable, then you basically can make use of
the total order. (All loads that read-from a certain store get a
synchronized-with (and thus happens-before edge) with the store, and the
stores are in a total order.) This is independent of the number of
readers and writers. The difference starts once you want to sync with
more than one variable, and need to establish an order between those
accesses.

You're right of course. More specifically when there is a thread where
some variables are stored while others are loaded.

There will still be a few cases that need to be optimized, and here are
where the difficult requirements come:

(R1) the primitives *should* not be alien to people who know Linux.

(R2) those optimizations *must* be easy to do and review; at least as
easy as these things go.

The two are obviously related. Ease of review is why it is important to
make things familiar to people who know Linux.

In C11, relaxing SC loads and stores is complicated, and more
specifically hard to explain!

I can't see why that would be harder than reasoning about equally weaker
Java semantics. But you obviously know your community, and I don't :)

Because Java semantics are almost SC, and as Paul mentioned the
difference doesn't matter in practice (IRIW/RWC is where it matters, WRC
works even on Power; see
http://www.cl.cam.ac.uk/~pes20/ppc-supplemental/ppc051.html#toc5, row
WRC+lwsyncs). It hasn't ever mattered for Linux, at least.

By contrast, Java volatile semantics are easily converted to a sequence
of relaxed loads, relaxed stores, and acq/rel/sc fences.

The same holds for C11/C++11. If you look at either the standard or the
Batty model, you'll see that for every pair like store(rel)--load(acq),
there is also store(rel)--fence(acq)+load(relaxed),
store(relaxed)+fence(rel)--fence(acq)+load(relaxed), etc. defined,
giving the same semantics. Likewise for SC.

Do you have a pointer to that? It would help.

You can also build Dekker with SC stores and acq loads, if I'm not
mistaken. Typically one would probably use SC fences and relaxed
stores/loads.

Yes.

I guess so. But you also have to consider the legacy that you create.
I do think the C11/C++11 model will used widely, and more and more
people will used to it.

I don't think many people will learn how to use the various non-seqcst
modes... At least so far I punted. :)

But you already use similarly weaker orderings that the other
abstractions provide (e.g., Java), so you're half-way there :)

True. On the other hand you can treat Java like kinda SC but don't
worry, you won't see the difference. It is both worrisome and appealing...

Paolo

[Qemu-devel] [PATCH v2 2/8] exec: Clean up fall back when -mem-path allocation fails

With -mem-path, qemu_ram_alloc_from_ptr() first tries to allocate
accordingly, but when it fails, it falls back to normal allocation.

The fall back allocation code used to be effectively identical to the
-mem-path not given code, until it started to diverge in commit
432d268.  I believe the code still works, but clean it up anyway: drop
the special fall back allocation code, and fall back to the ordinary
-mem-path not given code instead.

Reviewed-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Markus Armbruster arm...@redhat.com
---
 exec.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/exec.c b/exec.c
index b424e12..56c31a9 100644
--- a/exec.c
+++ b/exec.c
@@ -1091,15 +1091,12 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, 
void *host,
 if (mem_path) {
 #if defined (__linux__)  !defined(TARGET_S390X)
 new_block-host = file_ram_alloc(new_block, size, mem_path);
-if (!new_block-host) {
-new_block-host = qemu_anon_ram_alloc(size);
-memory_try_enable_merging(new_block-host, size);
-}
 #else
 fprintf(stderr, -mem-path option unsupported\n);
 exit(1);
 #endif
-} else {
+}
+if (!new_block-host) {
 if (kvm_enabled()) {
 /* some s390/kvm configurations have special constraints */
 new_block-host = kvm_ram_alloc(size);
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 1/8] exec: Fix Xen RAM allocation with unusual options

Issues:

* We try to obey -mem-path even though it can't work with Xen.

* To implement -machine mem-merge, we call
  memory_try_enable_merging(new_block-host, size).  But with Xen,
  new_block-host remains null.  Oops.

Fix by separating Xen allocation from normal allocation.

Acked-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
Signed-off-by: Markus Armbruster arm...@redhat.com
---
 exec.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/exec.c b/exec.c
index 5b8b40d..b424e12 100644
--- a/exec.c
+++ b/exec.c
@@ -1081,6 +1081,12 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void 
*host,
 if (host) {
 new_block-host = host;
 new_block-flags |= RAM_PREALLOC_MASK;
+} else if (xen_enabled()) {
+if (mem_path) {
+fprintf(stderr, -mem-path not supported with Xen\n);
+exit(1);
+}
+xen_ram_alloc(new_block-offset, size, mr);
 } else {
 if (mem_path) {
 #if defined (__linux__)  !defined(TARGET_S390X)
@@ -1094,9 +1100,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void 
*host,
 exit(1);
 #endif
 } else {
-if (xen_enabled()) {
-xen_ram_alloc(new_block-offset, size, mr);
-} else if (kvm_enabled()) {
+if (kvm_enabled()) {
 /* some s390/kvm configurations have special constraints */
 new_block-host = kvm_ram_alloc(size);
 } else {
@@ -1174,6 +1178,8 @@ void qemu_ram_free(ram_addr_t addr)
 ram_list.version++;
 if (block-flags  RAM_PREALLOC_MASK) {
 ;
+} else if (xen_enabled()) {
+xen_invalidate_map_cache_entry(block-host);
 } else if (mem_path) {
 #if defined (__linux__)  !defined(TARGET_S390X)
 if (block-fd) {
@@ -1186,11 +1192,7 @@ void qemu_ram_free(ram_addr_t addr)
 abort();
 #endif
 } else {
-if (xen_enabled()) {
-xen_invalidate_map_cache_entry(block-host);
-} else {
-qemu_anon_ram_free(block-host, block-length);
-}
+qemu_anon_ram_free(block-host, block-length);
 }
 g_free(block);
 break;
@@ -1214,6 +1216,8 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
 vaddr = block-host + offset;
 if (block-flags  RAM_PREALLOC_MASK) {
 ;
+} else if (xen_enabled()) {
+abort();
 } else {
 flags = MAP_FIXED;
 munmap(vaddr, length);
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 8/8] pc_sysfw: Fix ISA BIOS init for ridiculously big flash

pc_isa_bios_init() suffers integer overflow for flash larger than
INT_MAX.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 hw/block/pc_sysfw.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/block/pc_sysfw.c b/hw/block/pc_sysfw.c
index 412d1b0..aebefc9 100644
--- a/hw/block/pc_sysfw.c
+++ b/hw/block/pc_sysfw.c
@@ -54,10 +54,7 @@ static void pc_isa_bios_init(MemoryRegion *rom_memory,
 flash_size = memory_region_size(flash_mem);
 
 /* map the last 128KB of the BIOS in ISA space */
-isa_bios_size = flash_size;
-if (isa_bios_size  (128 * 1024)) {
-isa_bios_size = 128 * 1024;
-}
+isa_bios_size = MIN(flash_size, 128 * 1024);
 isa_bios = g_malloc(sizeof(*isa_bios));
 memory_region_init_ram(isa_bios, isa-bios, isa_bios_size);
 vmstate_register_ram_global(isa_bios);
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 3/8] exec: Reduce ifdeffery around -mem-path

Instead of spreading its ifdeffery everywhere, confine it to
qemu_ram_alloc_from_ptr().  Everywhere else, simply test block-fd,
which is non-negative exactly when block uses -mem-path.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 exec.c | 37 ++---
 include/exec/cpu-all.h |  2 --
 2 files changed, 10 insertions(+), 29 deletions(-)

diff --git a/exec.c b/exec.c
index 56c31a9..4dbb0f1 100644
--- a/exec.c
+++ b/exec.c
@@ -1073,6 +1073,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void 
*host,
 
 size = TARGET_PAGE_ALIGN(size);
 new_block = g_malloc0(sizeof(*new_block));
+new_block-fd = -1;
 
 /* This assumes the iothread lock is taken here too.  */
 qemu_mutex_lock_ramlist();
@@ -1177,17 +1178,9 @@ void qemu_ram_free(ram_addr_t addr)
 ;
 } else if (xen_enabled()) {
 xen_invalidate_map_cache_entry(block-host);
-} else if (mem_path) {
-#if defined (__linux__)  !defined(TARGET_S390X)
-if (block-fd) {
-munmap(block-host, block-length);
-close(block-fd);
-} else {
-qemu_anon_ram_free(block-host, block-length);
-}
-#else
-abort();
-#endif
+} else if (block-fd = 0) {
+munmap(block-host, block-length);
+close(block-fd);
 } else {
 qemu_anon_ram_free(block-host, block-length);
 }
@@ -1218,25 +1211,15 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
 } else {
 flags = MAP_FIXED;
 munmap(vaddr, length);
-if (mem_path) {
-#if defined(__linux__)  !defined(TARGET_S390X)
-if (block-fd) {
+if (block-fd = 0) {
 #ifdef MAP_POPULATE
-flags |= mem_prealloc ? MAP_POPULATE | MAP_SHARED :
-MAP_PRIVATE;
+flags |= mem_prealloc ? MAP_POPULATE | MAP_SHARED :
+MAP_PRIVATE;
 #else
-flags |= MAP_PRIVATE;
-#endif
-area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
-flags, block-fd, offset);
-} else {
-flags |= MAP_PRIVATE | MAP_ANONYMOUS;
-area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
-flags, -1, 0);
-}
-#else
-abort();
+flags |= MAP_PRIVATE;
 #endif
+area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
+flags, block-fd, offset);
 } else {
 #if defined(TARGET_S390X)  defined(CONFIG_KVM)
 flags |= MAP_SHARED | MAP_ANONYMOUS;
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index e9c3717..c369b25 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -476,9 +476,7 @@ typedef struct RAMBlock {
  * Writes must take both locks.
  */
 QTAILQ_ENTRY(RAMBlock) next;
-#if defined(__linux__)  !defined(TARGET_S390X)
 int fd;
-#endif
 } RAMBlock;
 
 typedef struct RAMList {
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 4/8] exec: Simplify the guest physical memory allocation hook

Make it a generic hook rather than a KVM hook.  Less code and
ifdeffery.

Since the only user of the hook is old S390 KVM, there's hope we can
get rid of it some day.

Acked-by: Christian Borntraeger borntrae...@de.ibm.com
Signed-off-by: Markus Armbruster arm...@redhat.com
---
 exec.c  | 20 ++--
 include/exec/exec-all.h |  2 ++
 include/sysemu/kvm.h|  5 -
 kvm-all.c   | 13 -
 target-s390x/kvm.c  | 17 ++---
 5 files changed, 22 insertions(+), 35 deletions(-)

diff --git a/exec.c b/exec.c
index 4dbb0f1..c45eb33 100644
--- a/exec.c
+++ b/exec.c
@@ -685,6 +685,19 @@ typedef struct subpage_t {
 static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
  uint16_t section);
 static subpage_t *subpage_init(hwaddr base);
+
+static void *(*phys_mem_alloc)(ram_addr_t size) = qemu_anon_ram_alloc;
+
+/*
+ * Set a custom physical guest memory alloator.
+ * Accelerators with unusual needs may need this.  Hopefully, we can
+ * get rid of it eventually.
+ */
+void phys_mem_set_alloc(void *(*alloc)(ram_addr_t))
+{
+phys_mem_alloc = alloc;
+}
+
 static void destroy_page_desc(uint16_t section_index)
 {
 MemoryRegionSection *section = phys_sections[section_index];
@@ -1098,12 +,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void 
*host,
 #endif
 }
 if (!new_block-host) {
-if (kvm_enabled()) {
-/* some s390/kvm configurations have special constraints */
-new_block-host = kvm_ram_alloc(size);
-} else {
-new_block-host = qemu_anon_ram_alloc(size);
-}
+new_block-host = phys_mem_alloc(size);
 memory_try_enable_merging(new_block-host, size);
 }
 }
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index b2162a4..4921696 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -369,6 +369,8 @@ bool is_tcg_gen_code(uintptr_t pc_ptr);
 
 #if !defined(CONFIG_USER_ONLY)
 
+void phys_mem_set_alloc(void *(*alloc)(ram_addr_t));
+
 struct MemoryRegion *iotlb_to_region(hwaddr index);
 bool io_mem_read(struct MemoryRegion *mr, hwaddr addr,
  uint64_t *pvalue, unsigned size);
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 8b19322..e722027 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -151,11 +151,6 @@ int kvm_init_vcpu(CPUState *cpu);
 #ifdef NEED_CPU_H
 int kvm_cpu_exec(CPUArchState *env);
 
-#if !defined(CONFIG_USER_ONLY)
-void *kvm_ram_alloc(ram_addr_t size);
-void *kvm_arch_ram_alloc(ram_addr_t size);
-#endif
-
 void kvm_setup_guest_memory(void *start, size_t size);
 void kvm_flush_coalesced_mmio_buffer(void);
 
diff --git a/kvm-all.c b/kvm-all.c
index 405480e..f88c4ec 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1816,19 +1816,6 @@ int kvm_has_intx_set_mask(void)
 return kvm_state-intx_set_mask;
 }
 
-void *kvm_ram_alloc(ram_addr_t size)
-{
-#ifdef TARGET_S390X
-void *mem;
-
-mem = kvm_arch_ram_alloc(size);
-if (mem) {
-return mem;
-}
-#endif
-return qemu_anon_ram_alloc(size);
-}
-
 void kvm_setup_guest_memory(void *start, size_t size)
 {
 #ifdef CONFIG_VALGRIND_H
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 4d9ac4a..e7863d7 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -92,9 +92,15 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 
 static int cap_sync_regs;
 
+static void *legacy_s390_alloc(ram_addr_t size);
+
 int kvm_arch_init(KVMState *s)
 {
 cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
+if (!kvm_check_extension(s, KVM_CAP_S390_GMAP)
+|| !kvm_check_extension(s, KVM_CAP_S390_COW)) {
+phys_mem_set_alloc(legacy_s390_alloc);
+}
 return 0;
 }
 
@@ -332,17 +338,6 @@ static void *legacy_s390_alloc(ram_addr_t size)
 return mem;
 }
 
-void *kvm_arch_ram_alloc(ram_addr_t size)
-{
-/* Can we use the standard allocation ? */
-if (kvm_check_extension(kvm_state, KVM_CAP_S390_GMAP) 
-kvm_check_extension(kvm_state, KVM_CAP_S390_COW)) {
-return NULL;
-} else {
-return legacy_s390_alloc(size);
-}
-}
-
 int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
 {
 S390CPU *cpu = S390_CPU(cs);
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 0/8] Guest memory allocation fixes cleanup

All I wanted to do is exit(1) instead of abort() on guest memory
allocation failure [07/08].  But that lead me into a minor #ifdef bog,
and here's what I brought back.  Enjoy!

Testing:
* Christian Borntraeger reports v1 works fine under LPAR (new S390
  KVM, i.e. generic allocation) and as second guest under z/VM (old
  S390 KVM, i.e. legacy S390 allocation).  Thanks for testing, and for
  catching a stupid mistake.  v2 differs from v1 only in code that
  isn't reachable on S390.

Changes since v1:
* 5/8: Fix assertion in qemu_ram_remap() (Paolo)
* All other patches unchanged except for Acked-by in commit messages
Changes since RFC:
* 1-3+8/8 unchanged except for commit message tweaks
* 4+6/8 rewritten to address Paolo's review
* 5/8 rewritten: don't fix dead code, just assert it's dead
* 7/8 fix mistakes caught by Richard Henderson and Peter Maydell

Markus Armbruster (8):
  exec: Fix Xen RAM allocation with unusual options
  exec: Clean up fall back when -mem-path allocation fails
  exec: Reduce ifdeffery around -mem-path
  exec: Simplify the guest physical memory allocation hook
  exec: Drop incorrect  dead S390 code in qemu_ram_remap()
  exec: Clean up unnecessary S390 ifdeffery
  exec: Don't abort when we can't allocate guest memory
  pc_sysfw: Fix ISA BIOS init for ridiculously big flash

 exec.c  | 121 ++--
 hw/block/pc_sysfw.c |   5 +-
 include/exec/cpu-all.h  |   2 -
 include/exec/exec-all.h |   2 +
 include/sysemu/kvm.h|   5 --
 kvm-all.c   |  13 --
 target-s390x/kvm.c  |  23 +++--
 util/oslib-posix.c  |   4 +-
 util/oslib-win32.c  |   5 +-
 9 files changed, 78 insertions(+), 102 deletions(-)

-- 
1.7.11.7

[Qemu-devel] [PATCH v2 5/8] exec: Drop incorrect dead S390 code in qemu_ram_remap()

Old S390 KVM wants guest RAM mapped in a peculiar way.  Commit 6b02494
implemented that.

When qemu_ram_remap() got added in commit cd19cfa, its code carefully
mimicked the allocation code: peculiar way if defined(TARGET_S390X) 
defined(CONFIG_KVM), else normal way.

For new S390 KVM, we actually want the normal way.  Commit fdec991
changed qemu_ram_alloc_from_ptr() accordingly, but forgot to update
qemu_ram_remap().  If qemu_ram_alloc_from_ptr() maps RAM the normal
way, but qemu_ram_remap() remaps it the peculiar way, remapping
changes protection and flags, which it shouldn't.

Fortunately, this can't happen, as we never remap on S390.

Replace the incorrect code with an assertion.

Thanks to Christian Borntraeger for help with assessing the bug's
(non-)impact.

Acked-by: Christian Borntraeger borntrae...@de.ibm.com
Signed-off-by: Markus Armbruster arm...@redhat.com
---
 exec.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/exec.c b/exec.c
index c45eb33..366ac6a 100644
--- a/exec.c
+++ b/exec.c
@@ -1229,15 +1229,16 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
 area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
 flags, block-fd, offset);
 } else {
-#if defined(TARGET_S390X)  defined(CONFIG_KVM)
-flags |= MAP_SHARED | MAP_ANONYMOUS;
-area = mmap(vaddr, length, PROT_EXEC|PROT_READ|PROT_WRITE,
-flags, -1, 0);
-#else
+/*
+ * Remap needs to match alloc.  Accelerators that
+ * set phys_mem_alloc never remap.  If they did,
+ * we'd need a remap hook here.
+ */
+assert(phys_mem_alloc == qemu_anon_ram_alloc);
+
 flags |= MAP_PRIVATE | MAP_ANONYMOUS;
 area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
 flags, -1, 0);
-#endif
 }
 if (area != vaddr) {
 fprintf(stderr, Could not remap addr: 
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 6/8] exec: Clean up unnecessary S390 ifdeffery

Another issue missed in commit fdec991 is -mem-path: it needs to be
rejected only for old S390 KVM, not for any S390.  Not that I
personally care, but the ifdeffery in qemu_ram_alloc_from_ptr() annoys
me.

Note that this doesn't actually make -mem-path work, as the kernel
doesn't (yet?)  support large pages in the host for KVM guests.  Clean
it up anyway.

Thanks to Christian Borntraeger for pointing out the S390 kernel
limitations.

Signed-off-by: Markus Armbruster arm...@redhat.com
---
 exec.c | 25 +++--
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/exec.c b/exec.c
index 366ac6a..bf2a7d6 100644
--- a/exec.c
+++ b/exec.c
@@ -862,7 +862,7 @@ void qemu_mutex_unlock_ramlist(void)
 qemu_mutex_unlock(ram_list.mutex);
 }
 
-#if defined(__linux__)  !defined(TARGET_S390X)
+#ifdef __linux__
 
 #include sys/vfs.h
 
@@ -965,6 +965,14 @@ static void *file_ram_alloc(RAMBlock *block,
 block-fd = fd;
 return area;
 }
+#else
+static void *file_ram_alloc(RAMBlock *block,
+ram_addr_t memory,
+const char *path)
+{
+fprintf(stderr, -mem-path not supported on this host\n);
+exit(1);
+}
 #endif
 
 static ram_addr_t find_ram_offset(ram_addr_t size)
@@ -1103,12 +,17 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, 
void *host,
 xen_ram_alloc(new_block-offset, size, mr);
 } else {
 if (mem_path) {
-#if defined (__linux__)  !defined(TARGET_S390X)
+if (phys_mem_alloc != qemu_anon_ram_alloc) {
+/*
+ * file_ram_alloc() needs to allocate just like
+ * phys_mem_alloc, but we haven't bothered to provide
+ * a hook there.
+ */
+fprintf(stderr,
+-mem-path not supported with this accelerator\n);
+exit(1);
+}
 new_block-host = file_ram_alloc(new_block, size, mem_path);
-#else
-fprintf(stderr, -mem-path option unsupported\n);
-exit(1);
-#endif
 }
 if (!new_block-host) {
 new_block-host = phys_mem_alloc(size);
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 7/8] exec: Don't abort when we can't allocate guest memory

We abort() on memory allocation failure.  abort() is appropriate for
programming errors.  Maybe most memory allocation failures are
programming errors, maybe not.  But guest memory allocation failure
isn't, and aborting when the user asks for more memory than we can
provide is not nice.  exit(1) instead, and do it in just one place, so
the error message is consistent.

Tested-by: Christian Borntraeger borntrae...@de.ibm.com
Signed-off-by: Markus Armbruster arm...@redhat.com
---
 exec.c | 5 +
 target-s390x/kvm.c | 6 +-
 util/oslib-posix.c | 4 +---
 util/oslib-win32.c | 5 +
 4 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/exec.c b/exec.c
index bf2a7d6..3f7fe29 100644
--- a/exec.c
+++ b/exec.c
@@ -1125,6 +1125,11 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void 
*host,
 }
 if (!new_block-host) {
 new_block-host = phys_mem_alloc(size);
+if (!new_block-host) {
+fprintf(stderr, Cannot set up guest memory '%s': %s\n,
+new_block-mr-name, strerror(errno));
+exit(1);
+}
 memory_try_enable_merging(new_block-host, size);
 }
 }
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index e7863d7..b1ffcea 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -331,11 +331,7 @@ static void *legacy_s390_alloc(ram_addr_t size)
 mem = mmap((void *) 0x8ULL, size,
PROT_EXEC|PROT_READ|PROT_WRITE,
MAP_SHARED | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
-if (mem == MAP_FAILED) {
-fprintf(stderr, Allocating RAM failed\n);
-abort();
-}
-return mem;
+return mem == MAP_FAILED ? NULL : mem;
 }
 
 int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 3dc8b1b..253bc3d 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -112,9 +112,7 @@ void *qemu_anon_ram_alloc(size_t size)
 size_t offset = QEMU_ALIGN_UP((uintptr_t)ptr, align) - (uintptr_t)ptr;
 
 if (ptr == MAP_FAILED) {
-fprintf(stderr, Failed to allocate %zu B: %s\n,
-size, strerror(errno));
-abort();
+return NULL;
 }
 
 ptr += offset;
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 961fbf5..983b7a2 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -65,10 +65,7 @@ void *qemu_anon_ram_alloc(size_t size)
 /* FIXME: this is not exactly optimal solution since VirtualAlloc
has 64Kb granularity, but at least it guarantees us that the
memory is page aligned. */
-if (!size) {
-abort();
-}
-ptr = qemu_oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
+ptr = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE);
 trace_qemu_anon_ram_alloc(size, ptr);
 return ptr;
 }
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH v3 1/2] pvpanic: initialization cleanup

2013-06-20 Thread Laszlo Ersek

On 06/19/13 17:02, Michael S. Tsirkin wrote:
 Avoid use of static variables: PC systems
 initialize pvpanic device through pvpanic_init,
 so we can simply create the fw_cfg file at that point.
 This also makes it possible to skip device
 creation completely if fw_cfg is not there, e.g. for xen -
 so the ports it reserves are not discoverable by guests.
 
 Also, make pvpanic_init void since callers ignore return
 status anyway.
 
 Cc: Stefano Stabellini stefano.stabell...@eu.citrix.com
 Cc: Laszlo Ersek ler...@redhat.com
 Cc: Paul Durrant paul.durr...@citrix.com
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
 Chanes from v2:
 skip device creation completely if !fw_cfg
 make pvpanic_init void
 Changes from v1:
 don't assert if !fw_cfg
 
 
  hw/misc/pvpanic.c| 30 --
  include/hw/i386/pc.h |  2 +-
  2 files changed, 17 insertions(+), 15 deletions(-)
 
 diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
 index 060099b..83ed226 100644
 --- a/hw/misc/pvpanic.c
 +++ b/hw/misc/pvpanic.c
 @@ -97,26 +97,28 @@ static void pvpanic_isa_realizefn(DeviceState *dev, Error 
 **errp)
  {
  ISADevice *d = ISA_DEVICE(dev);
  PVPanicState *s = ISA_PVPANIC_DEVICE(dev);
 -static bool port_configured;
 -FWCfgState *fw_cfg;
  
  isa_register_ioport(d, s-io, s-ioport);
 +}
  
 -if (!port_configured) {
 -fw_cfg = fw_cfg_find();
 -if (fw_cfg) {
 -fw_cfg_add_file(fw_cfg, etc/pvpanic-port,
 -g_memdup(s-ioport, sizeof(s-ioport)),
 -sizeof(s-ioport));
 -port_configured = true;
 -}
 -}
 +static void pvpanic_fw_cfg(ISADevice *dev, FWCfgState *fw_cfg)
 +{
 +PVPanicState *s = ISA_PVPANIC_DEVICE(dev);
 +
 +fw_cfg_add_file(fw_cfg, etc/pvpanic-port,
 +g_memdup(s-ioport, sizeof(s-ioport)),
 +sizeof(s-ioport));
  }
  
 -int pvpanic_init(ISABus *bus)
 +void pvpanic_init(ISABus *bus)
  {
 -isa_create_simple(bus, TYPE_ISA_PVPANIC_DEVICE);
 -return 0;
 +ISADevice *dev;
 +FWCfgState *fw_cfg = fw_cfg_find();
 +if (!fw_cfg) {
 +return;
 +}
 +dev = isa_create_simple (bus, TYPE_ISA_PVPANIC_DEVICE);
 +pvpanic_fw_cfg(dev, fw_cfg);
  }
  
  static Property pvpanic_isa_properties[] = {
 diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
 index ba9ba1a..458eded 100644
 --- a/include/hw/i386/pc.h
 +++ b/include/hw/i386/pc.h
 @@ -196,7 +196,7 @@ static inline bool isa_ne2000_init(ISABus *bus, int base, 
 int irq, NICInfo *nd)
  void pc_system_firmware_init(MemoryRegion *rom_memory);
  
  /* pvpanic.c */
 -int pvpanic_init(ISABus *bus);
 +void pvpanic_init(ISABus *bus);
  
  /* e820 types */
  #define E820_RAM1
 

series
Reviewed-by: Laszlo Ersek ler...@redhat.com

Re: [Qemu-devel] [PATCH v3 1/2] add a header file for atomic operations

Il 19/06/2013 22:44, Richard Henderson ha scritto:
  +/* Data must be read atomically. We don't really need barrier 
  semantics
  + * but it's easier to use atomic_* than roll our own. */
  +log = atomic_xchg(from, 0);
  
  If you really don't need any ordering guarantees / barriers here, then
  using a relaxed load should be fine.  But my gut feeling tells me you
  probably do need some barriers; either you are re-using another
  barrier (and then the comment should probably point out which), or it
  must be a case where it's either fine to read any value someone (else)
  wrote or there's no concurrent store after all.
  
 There is a store here, before and after.  Read the value, store zero.
 
 I suppose what the comment is saying is that the atomic operation doesn't need
 to be ordered with respect to the rest of the surrounding code, as the object
 being synchronized is just that one integer.

Exactly.  The items of the array can be read independently.

Paolo

Re: [Qemu-devel] [Xen-devel] [PATCH] Add Xen platform PCI device version 2.

2013-06-20 Thread Alex Bligh




--On 20 June 2013 07:47:12 + Paul Durrant paul.durr...@citrix.com 
wrote:



If someone has a VM that has the old Citrix drivers installed, or GPLPV,
I think I should be able to spot this and make sure that the new bus
driver quiesces itself to prevent strangeness ensuing. If and when said
previous drivers are un-installed then the new bus driver can wake up and
enumerate the device nodes for the other pv drivers and Windows Update
can carry on doing its stuff.


I have no clue about Windows device drivers, so this may be a silly
suggestion. If your suggestion above already requires a Xen code change,
one possibility might be copy the idea behind the PCI unplug logic. Either
if the new PCI device is used, it could unplug the old one, or vice versa.
Drivers magically unplugging themselves may not be ideal, but it beats
having 2 drivers fighting over the same device.

--
Alex Bligh

Re: [Qemu-devel] [PATCH v3 2/2] QEMUBH: make AioContext's bh re-entrant

Il 20/06/2013 09:39, Stefan Hajnoczi ha scritto:
 qemu_bh_cancel() and qemu_bh_delete() are not modified by this patch.
 
 It seems that calling them from a thread is a little risky because there
 is no guarantee that the BH is no longer invoked after a thread calls
 these functions.
 
 I think that's worth a comment or do you want them to take the lock so
 they become safe?

Taking the lock wouldn't help.  The invoking loop of aio_bh_poll runs
lockless.  I think a comment is better.

qemu_bh_cancel is inherently not thread-safe, there's not much you can
do about it.

qemu_bh_delete is safe as long as you wait for the bottom half to stop
before deleting the containing object.  Once we have RCU, deletion of
QOM objects will be RCU-protected.  Hence, a simple way could be to put
the first part of aio_bh_poll() within rcu_read_lock/unlock.

 The other thing I'm unclear on is the -idle assignment followed
 immediately by a -scheduled assignment.  Without memory barriers
 aio_bh_poll() isn't guaranteed to get an ordered view of these updates:
 it may see an idle BH as a regular scheduled BH because -idle is still
 0.

Right.  You need to order -idle writes before -scheduled writes, and
add memory barriers, or alternatively use two bits in -scheduled so
that you can assign both atomically.

Paolo

Re: [Qemu-devel] [Xen-devel] [PATCH] Add Xen platform PCI device version 2.

 -Original Message-
 From: Alex Bligh [mailto:a...@alex.org.uk]
 Sent: 20 June 2013 09:09
 To: Paul Durrant; Tim (Xen.org); Matt Wilson
 Cc: xen-de...@lists.xen.org; Ian Campbell; qemu-devel@nongnu.org; Alex
 Bligh
 Subject: RE: [Xen-devel] [Qemu-devel] [PATCH] Add Xen platform PCI device
 version 2.

 --On 20 June 2013 07:47:12 + Paul Durrant paul.durr...@citrix.com
 wrote:

  If someone has a VM that has the old Citrix drivers installed, or GPLPV,
  I think I should be able to spot this and make sure that the new bus
  driver quiesces itself to prevent strangeness ensuing. If and when said
  previous drivers are un-installed then the new bus driver can wake up and
  enumerate the device nodes for the other pv drivers and Windows Update
  can carry on doing its stuff.

 I have no clue about Windows device drivers, so this may be a silly
 suggestion. If your suggestion above already requires a Xen code change,
 one possibility might be copy the idea behind the PCI unplug logic. Either
 if the new PCI device is used, it could unplug the old one, or vice versa.
 Drivers magically unplugging themselves may not be ideal, but it beats
 having 2 drivers fighting over the same device.

Unfortunately, whilst it sounds good on the face of it, it's not as 
straightforward as that. The old Citrix PV drivers did not just bind to the Xen 
platform device, and make that device go away automagically would actually 
cause the system disk to disappear without any clean fallback to emulation.
As long as nothing actually breaks if and when Windows fetches the new PV bus 
driver from Windows Update then we can document the need to manually uninstall 
any other PV drivers.

  Paul

Re: [Qemu-devel] [RESEND PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci.

2013-06-20 Thread Frederic Konrad


On 18/06/2013 17:21, Michael S. Tsirkin wrote:

On Fri, Jun 14, 2013 at 08:13:29AM +0200, Frederic Konrad wrote:

On 13/06/2013 09:59, Michael S. Tsirkin wrote:

On Thu, Jun 13, 2013 at 09:34:30AM +0200, Frederic Konrad wrote:

On 13/06/2013 09:23, Michael S. Tsirkin wrote:

On Thu, Jun 13, 2013 at 04:46:09PM +1000, Alexey Kardashevskiy wrote:

On 06/13/2013 04:28 PM, Frederic Konrad wrote:

On 12/06/2013 13:21, Alexey Kardashevskiy wrote:

On 06/12/2013 07:16 PM, Michael S. Tsirkin wrote:

On Wed, Jun 12, 2013 at 07:04:48PM +1000, Alexey Kardashevskiy wrote:

On 06/12/2013 07:03 PM, Michael S. Tsirkin wrote:

On Wed, Jun 12, 2013 at 08:15:17AM +0200, fred.kon...@greensocs.com
wrote:

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

This fix a bug with scsi hotplug on virtio-scsi-pci:

As virtio-scsi-pci doesn't have any scsi bus, we need to forward
scsi-hot-add
to the virtio-scsi-device plugged on the virtio-bus.

Cc: qemu-sta...@nongnu.org
Reported-by: Alexey Kardashevskiy a...@ozlabs.ru
Reviewed-by: Andreas Färber afaer...@suse.de
Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com

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

Note: we don't seem to have any decent way to
add disks to devices: no QMP interface,
pci address is required instead of using an id ...

Anyone can be bothered to fix this?

Actually PCI address is not always required, this field (we are talking
about drive_add?) is ignored when if=none.


Then documentation in hmp-commands.hx is wrong, isn't it?
Add that to the list.

if=none can't be actually used to hot-add
a disk to a device, can it? It creates a disc and assumes you will
use it by a device created later.

Yep. I run QEMU with -device virtio-scsi-pci,id=device0 and then do in
console:
drive_add auto file=virtimg/fc18guest,if=none,id=bar1
device_add scsi-disk,bus=device0.0,drive=bar1

Pretty hot plug :)

I thought you use drive_add 0 if=scsi?

That's the other option, I posted a bug but I did not actually try the fix
till now :)

It works now if I run QEMU with -device virtio-scsi-pci and do this in
qemu console:
drive_add 0 file=virtimg/fc18guest

No extra parameters or anything, cool, thanks, and :)

Tested-by: Alexey Kardashevskiy a...@ozlabs.ru


The only problem with it that it still wants PCI SCSI adapter while
spapr-vscsi is VIO device so if the guest kernel does not have virtio-scsi
support, I have to do what I described in the quote but this is a different
story.

Okay.  How about:
- document that pci_addr is optional in hmp
- if no pci_addr assume if=none
- add drive_add to qmp without the pci_addr and if options

We are left with the bus=device0.0 syntax for device_add which is also
gross - user asked for device0, the .0 part is qemu internals exposed to
users.
How about teaching qdev that if there's a single bus under a device,
naming the device itself should be identical?

Yes why not seems a good idea, but you'll pass it through bus= option?

This will solve the problem neatly without virtio specific hacks,
won't it?

The issue here is command line back-compatibility for pci_addr,
which won't be solved with
the single bus idea?

Why not? This code:
 scsibus = (SCSIBus *)
 object_dynamic_cast(OBJECT(QLIST_FIRST(adapter-child_bus)),
 TYPE_SCSI_BUS);
should be replaced with code from qdev that we'll write
that goes down the chain as long as there's 1 device
on each bus, looking for a device of the appropriate type.

Ok, understood what you mean :).

Why not if everybody is happy with it.

Fred

Ok so - want to try implementing this?


Ok, will try to look at it next week.

What about the stable release?
Wouldn't be safe to take this patch for the stable?

Fred



---
   hw/pci/pci-hotplug.c | 19 +--
   1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/hw/pci/pci-hotplug.c b/hw/pci/pci-hotplug.c
index 12287d1..c708752 100644
--- a/hw/pci/pci-hotplug.c
+++ b/hw/pci/pci-hotplug.c
@@ -30,6 +30,8 @@
   #include monitor/monitor.h
   #include hw/scsi/scsi.h
   #include hw/virtio/virtio-blk.h
+#include hw/virtio/virtio-scsi.h
+#include hw/virtio/virtio-pci.h
   #include qemu/config-file.h
   #include sysemu/blockdev.h
   #include qapi/error.h
@@ -79,13 +81,26 @@ static int scsi_hot_add(Monitor *mon, DeviceState
*adapter,
   {
   SCSIBus *scsibus;
   SCSIDevice *scsidev;
+VirtIOPCIProxy *virtio_proxy;
 scsibus = (SCSIBus *)
   object_dynamic_cast(OBJECT(QLIST_FIRST(adapter-child_bus)),
   TYPE_SCSI_BUS);
   if (!scsibus) {
-error_report(Device is not a SCSI adapter);
-return -1;
+/*
+ * Check if the adapter is a virtio-scsi-pci, and forward
scsi_hot_add
+ * to the virtio-scsi-device.
+ */
+if (!object_dynamic_cast(OBJECT(adapter),
TYPE_VIRTIO_SCSI_PCI)) {
+error_report(Device is not a SCSI adapter);
+return -1;
+}
+virtio_proxy = VIRTIO_PCI(adapter);
+

Re: [Qemu-devel] [RESEND PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci.

2013-06-20 Thread Michael S. Tsirkin

On Thu, Jun 20, 2013 at 10:26:18AM +0200, Frederic Konrad wrote:
 On 18/06/2013 17:21, Michael S. Tsirkin wrote:
 On Fri, Jun 14, 2013 at 08:13:29AM +0200, Frederic Konrad wrote:
 On 13/06/2013 09:59, Michael S. Tsirkin wrote:
 On Thu, Jun 13, 2013 at 09:34:30AM +0200, Frederic Konrad wrote:
 On 13/06/2013 09:23, Michael S. Tsirkin wrote:
 On Thu, Jun 13, 2013 at 04:46:09PM +1000, Alexey Kardashevskiy wrote:
 On 06/13/2013 04:28 PM, Frederic Konrad wrote:
 On 12/06/2013 13:21, Alexey Kardashevskiy wrote:
 On 06/12/2013 07:16 PM, Michael S. Tsirkin wrote:
 On Wed, Jun 12, 2013 at 07:04:48PM +1000, Alexey Kardashevskiy wrote:
 On 06/12/2013 07:03 PM, Michael S. Tsirkin wrote:
 On Wed, Jun 12, 2013 at 08:15:17AM +0200, fred.kon...@greensocs.com
 wrote:
 From: KONRAD Frederic fred.kon...@greensocs.com
 
 This fix a bug with scsi hotplug on virtio-scsi-pci:
 
 As virtio-scsi-pci doesn't have any scsi bus, we need to forward
 scsi-hot-add
 to the virtio-scsi-device plugged on the virtio-bus.
 
 Cc: qemu-sta...@nongnu.org
 Reported-by: Alexey Kardashevskiy a...@ozlabs.ru
 Reviewed-by: Andreas Färber afaer...@suse.de
 Signed-off-by: KONRAD Frederic fred.kon...@greensocs.com
 Acked-by: Michael S. Tsirkin m...@redhat.com
 
 Note: we don't seem to have any decent way to
 add disks to devices: no QMP interface,
 pci address is required instead of using an id ...
 
 Anyone can be bothered to fix this?
 Actually PCI address is not always required, this field (we are 
 talking
 about drive_add?) is ignored when if=none.
 
 Then documentation in hmp-commands.hx is wrong, isn't it?
 Add that to the list.
 
 if=none can't be actually used to hot-add
 a disk to a device, can it? It creates a disc and assumes you will
 use it by a device created later.
 Yep. I run QEMU with -device virtio-scsi-pci,id=device0 and then do 
 in
 console:
 drive_add auto file=virtimg/fc18guest,if=none,id=bar1
 device_add scsi-disk,bus=device0.0,drive=bar1
 
 Pretty hot plug :)
 I thought you use drive_add 0 if=scsi?
 That's the other option, I posted a bug but I did not actually try the 
 fix
 till now :)
 
 It works now if I run QEMU with -device virtio-scsi-pci and do this in
 qemu console:
 drive_add 0 file=virtimg/fc18guest
 
 No extra parameters or anything, cool, thanks, and :)
 
 Tested-by: Alexey Kardashevskiy a...@ozlabs.ru
 
 
 The only problem with it that it still wants PCI SCSI adapter while
 spapr-vscsi is VIO device so if the guest kernel does not have 
 virtio-scsi
 support, I have to do what I described in the quote but this is a 
 different
 story.
 Okay.  How about:
 - document that pci_addr is optional in hmp
 - if no pci_addr assume if=none
 - add drive_add to qmp without the pci_addr and if options
 
 We are left with the bus=device0.0 syntax for device_add which is also
 gross - user asked for device0, the .0 part is qemu internals exposed to
 users.
 How about teaching qdev that if there's a single bus under a device,
 naming the device itself should be identical?
 Yes why not seems a good idea, but you'll pass it through bus= option?
 This will solve the problem neatly without virtio specific hacks,
 won't it?
 The issue here is command line back-compatibility for pci_addr,
 which won't be solved with
 the single bus idea?
 Why not? This code:
  scsibus = (SCSIBus *)
  object_dynamic_cast(OBJECT(QLIST_FIRST(adapter-child_bus)),
  TYPE_SCSI_BUS);
 should be replaced with code from qdev that we'll write
 that goes down the chain as long as there's 1 device
 on each bus, looking for a device of the appropriate type.
 Ok, understood what you mean :).
 
 Why not if everybody is happy with it.
 
 Fred
 Ok so - want to try implementing this?
 
 Ok, will try to look at it next week.
 
 What about the stable release?
 Wouldn't be safe to take this patch for the stable?
 
 Fred

Yes. My ACK is for stable.

 
 ---
hw/pci/pci-hotplug.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
 
 diff --git a/hw/pci/pci-hotplug.c b/hw/pci/pci-hotplug.c
 index 12287d1..c708752 100644
 --- a/hw/pci/pci-hotplug.c
 +++ b/hw/pci/pci-hotplug.c
 @@ -30,6 +30,8 @@
#include monitor/monitor.h
#include hw/scsi/scsi.h
#include hw/virtio/virtio-blk.h
 +#include hw/virtio/virtio-scsi.h
 +#include hw/virtio/virtio-pci.h
#include qemu/config-file.h
#include sysemu/blockdev.h
#include qapi/error.h
 @@ -79,13 +81,26 @@ static int scsi_hot_add(Monitor *mon, 
 DeviceState
 *adapter,
{
SCSIBus *scsibus;
SCSIDevice *scsidev;
 +VirtIOPCIProxy *virtio_proxy;
  scsibus = (SCSIBus *)

  object_dynamic_cast(OBJECT(QLIST_FIRST(adapter-child_bus)),
TYPE_SCSI_BUS);
if (!scsibus) {
 -error_report(Device is not a SCSI adapter);
 -return -1;
 +/*
 + * Check if the adapter is a virtio-scsi-pci, and forward
 scsi_hot_add
 + * to the virtio-scsi-device.
 + */

Re: [Qemu-devel] [PATCH] pseries: Fix compiler warning (conversion of pointer to integral value)

2013-06-20 Thread Alexander Graf



Am 20.06.2013 um 07:10 schrieb Michael Tokarev m...@tls.msk.ru:

 20.06.2013 01:40, Alexander Graf wrote:
 
 On 19.06.2013, at 23:08, Stefan Weil wrote:
 
 This kind of type cast must use uintptr_t or target_ulong to be portable
 for hosts with sizeof(void *) != sizeof(long).
 
 Here the value is assigned to a variable of type target_ulong.
 
 Signed-off-by: Stefan Weil s...@weilnetz.de
 
 Acked-by: Alexander Graf ag...@suse.de
 
 I suppose this one goes through the trivial tree?
 
 Anything which goes to -trivial can be applied directly or
 into some other subsystem tree first.  When I send a pull
 request I rebase trivial tree ontop of current master and
 filter out anything which has been already applied, so that's
 not an issue.  The only possible issue is when you applied
 it to some other tree, and -trivial pull request were handled
 before that other tree - how that will be handled by git?
 
 Will it complain (so that the situation should be resolved
 manually), will it apply nothing or will it appy an empty
 patch?  (the patch signature will be different, with
 different S-o-b.)  I think I've seen empty commits before
 in qemu tree, with the same subject/author as some previous
 commit.

It depends on how you handle the tree. I rebase ppc-next too, so the commit 
would simply vanish.

I'll apply it to ppc-next as well then.


Alex

Re: [Qemu-devel] [PATCH 6/9] vhost-scsi: new device supporting the tcm_vhost Linux kernel module

2013-06-20 Thread Libaiqing

Hi Asias,
Thanks for your config.
According to you config,I test booting from vhost device with upstream 
kernel and qemu,but failed.

1 installing guest from cdrom,ok.
2 booting vhost-scsi,guest fs error occurs. 
3 using fileio backstores,the error is same..
4 rebooting guest,a log printed:
 (qemu) hw/scsi/virtio-scsi.c:533:virtio_scsi_handle_event: Object 
0x7fccae7f2c88 is not an instance of type virtio-scsi-device
5 using upstream seabios,core dumped.
 
Could you give me some advise to debug this problem ? I can provide more 
information if need.

The qemu cmd:
[root@fedora121 x86_64-softmmu]# ./qemu-system-x86_64 -enable-kvm -name fedora  
 -M pc -m 1024 -smp 2   -drive file=/home/fedora18.iso,if=ide,media=cdrom 
-device vhost-scsi-pci,wwpn=naa.50014057133e25dc  -monitor stdio   -vga qxl  
-vnc :1

The vnc output:
Dracut-initqueue[189]:/dev/mapper/fedora-root:UNEXPECTED INCONSISTENCY;RUN FSCK 
MANUALLY.
Dracut-initqueue[189]: Warning: e2fsck returned with 4
Dracut-initqueue[189]: Warning: ***An error occurred during the file system 
check.

The guest kernel log:
Kernel: virtio-pci :00:04.0: irq 40 for MSI/MSI-X
Kernel: virtio-pci :00:04.0: irq 41 for MSI/MSI-X
Kernel: virtio-pci :00:04.0: irq 42 for MSI/MSI-X
Kernel: virtio-pci :00:04.0: irq 43 for MSI/MSI-X
Kernel: scsi2 : Virtio SCSI HBA
Kernel: scsi 2:0:1:0: Direct-Access LIO-ORG r0
Kernel: sd 2:0:1:0: Attached scsi generic sg1 type 0
Kernel: sd 2:0:1:0: [sda]1258912 512-byte logical .
Kernel: sd 2:0:1:0: [sda]write protect is off
Kernel: sd 2:0:1:0: [sda]Mode sense :43 00 00 08
Kernel: sd 2:0:1:0: [sda]write cache: disabled, read .
Kernel: sda sda1 sda2
Kernel: sd 2:0:1:0: [sda] Attached SCSI disk
Dracut-initqueue[189]: Scanning devices sda2 for LVM
Dracut-initqueue[189]: inactive '/dev/fedora/swap'...
Dracut-initqueue[189]: inactive '/dev/fedora/root'...

The info of host:
[root@fedora121 x86_64-softmmu]# uname -a
Linux fedora121 3.10.0-rc6 #1 SMP Wed Jun 19 19:34:24 CST 2013 x86_64 x86_64 
x86_64 GNU/Linux
[root@fedora121 x86_64-softmmu]# lsmod |grep vhost_scsi
vhost_scsi 49456  5
target_core_mod   282163  14 
target_core_iblock,target_core_pscsi,iscsi_target_mod,target_core_file,vhost_scsi
[root@fedora121 x86_64-softmmu]# targetcli
targetcli shell version v2.1.fb26
Copyright 2011 by RisingTide Systems LLC and others.
For help on commands, type 'help'.

/ ls
o- / 
.
 [...]
  o- backstores 
..
 [...]
  | o- block 
..
 [Storage Objects: 0]
  | o- fileio 
.
 [Storage Objects: 0]
  | o- pscsi 
..
 [Storage Objects: 0]
  | o- ramdisk 

 [Storage Objects: 1]
  |   o- r0 
...
 [(6.0GiB) activated]
  o- iscsi 

 [Targets: 0]
  o- loopback 
.
 [Targets: 0]
  o- vhost 

 [Targets: 1]
o- naa.50014057133e25dc 
..
 [TPGs: 1]
  o- tpg1 
...
 [naa.5001405a70ac3421]
o- acls 
..
 [ACLs: 0]
o- luns 
..
 [LUNs: 1]
  o- lun0 
.
 [ramdisk/r0]

Regards,
baiqing
 -Original Message-
 From: Asias He [mailto:as...@redhat.com]
 Sent: Thursday, June 20, 2013 9:34 AM
 To: Libaiqing
 Cc: Paolo Bonzini; Wenchao Xia; qemu-devel@nongnu.org;
 n...@linux-iscsi.org; Michael S. Tsirkin; Haofeng
 Subject: Re: [Qemu-devel] [PATCH 6/9] vhost-scsi: new device supporting the
 tcm_vhost Linux kernel module
 
 On Wed, Jun 19, 2013 at 12:55:10PM +, Libaiqing wrote:
  Hi paolo,
The vhost-scsi device can be used as boot device?
I tested with your config + 3.10 rc6 +

Re: [Qemu-devel] [RFC 06/13] qemu-thread: add TLS wrappers

Il 20/06/2013 09:26, Fam Zheng ha scritto:
 On Fri, 06/14 11:48, Stefan Hajnoczi wrote:
 From: Paolo Bonzini pbonz...@redhat.com

 Fast TLS is not available on some platforms, but it is always nice to
 use it.  This wrapper implementation falls back to pthread_get/setspecific
 on POSIX systems that lack __thread, but uses the dynamic linker's TLS
 support on Linux and Windows.

 The user shall call alloc_foo() in every thread that needs to access the
 variable---exactly once and before any access.  foo is the name of the
 variable as passed to DECLARE_TLS and DEFINE_TLS.  Then, get_foo() will
 return the address of the variable.  It is guaranteed to remain the same
 across the lifetime of a thread, so you can cache it.
 
 Would tls_alloc_foo() and tls_get_foo() be easier to read and less
 possible for name conflict?

Fine by me.

Paolo

Re: [Qemu-devel] [PATCH 06/12] spapr-vty: add copyright and license

Il 19/06/2013 22:40, Anthony Liguori ha scritto:
 If you are on CC, then please Ack this patch as you touched this
 file at some point in time.
 
 Cc: Alexey Kardashevskiy a...@ozlabs.ru
 Cc: Andreas Färber afaer...@suse.de
 Cc: David Gibson da...@gibson.dropbear.id.au
 Cc: Michael Ellerman mich...@ellerman.id.au
 Cc: Paolo Bonzini pbonz...@redhat.com
 Signed-off-by: Anthony Liguori aligu...@us.ibm.com
 ---
  hw/char/spapr_vty.c | 13 +
  1 file changed, 13 insertions(+)
 
 diff --git a/hw/char/spapr_vty.c b/hw/char/spapr_vty.c
 index 2993848..ecc2bb5 100644
 --- a/hw/char/spapr_vty.c
 +++ b/hw/char/spapr_vty.c
 @@ -1,3 +1,16 @@
 +/*
 + * QEMU PowerPC pSeries Logical Partition (aka sPAPR) hardware System 
 Emulator
 + *
 + * PAPR Inter-VM Logical Lan, aka ibmveth
 + *
 + * Copyright IBM, Corp. 2010-2013
 + *
 + * Authors:
 + *   David Gibson da...@gibson.dropbear.id.au
 + *
 + * This work is licensed under the terms of the GNU GPL, version 2 or later.
 + * See the COPYING file in the top-level directory.
 + */
  #include hw/qdev.h
  #include sysemu/char.h
  #include hw/ppc/spapr.h
 

ACK

Paolo

Re: [Qemu-devel] [Xen-devel] [PATCH] Add Xen platform PCI device version 2.

2013-06-20 Thread Tim Deegan

At 07:47 + on 20 Jun (1371714432), Paul Durrant wrote:
   I agree. If this is really the only solution, we would need to have
   both versions presented to the guest so that old drivers continue to
   work without any intervention.
  
  I suspect that if we expose both, both sets of drivers try to run the
  same PV connections, and hilarity ensues.
  
 
 Actually I think I can make that work, and it is the conclusion I came
 to after Alex's comment.

Ah, nice!  In that case, I'm a lot less worried -- we can just expose
both versions/devices by default and there's no need for a visible
control knob tied to driver version (except maybe for debugging).

It means an 'unsupported' device appearing on other/older OSes, which is
unfortunate, but ISTR only Windows really complains visibly about that.

Tim.

Re: [Qemu-devel] [PATCH v4 0/9] Make 'dump-guest-memory' dump in kdump-compressed format

On Thu, Jun 20, 2013 at 10:18:35AM +0800, Qiao Nuohan wrote:
 On 06/19/2013 09:49 PM, Stefan Hajnoczi wrote:
 Where does that code live that writes DISKDUMP files?  I can see the
 diskdump.[ch] code.
 
 Sorry, I cannot catch what do you mean here.

Please link to the code that writes DISKDUMP kdump files on a physical
machine.  I only see the crash utility code to read the DISKDUMP code
but I haven't found anything in the Linux kernel, the crash utility, or
the kexec-utils code to actually write a DISKDUMP file.

 
 The file format is pretty bad: we need 4 temporary files and a lot of
 data copying to write it out.
 
 Why not just compress an ELF file and teach the crash utility how to
 decompress while reading the ELF?
 
 Also, did you look into simply outputting the ELF file without zero
 pages?
 
 What I want is a dump file with smaller size. And compressed format and with
 zero pages excluded can make it. I choose kdump-compressed format because it 
 is
 a standard format and it can realize what I want.
 
 Why 4 temporary files are need? dump-guest-memory may be called with a fd 
 which
 is supposed to send data of dump to. If fd is opened on a pipe or etc which is
 unable to seek, then I need to cache the data.

I understand why you need temporary files, but my questions stand:

Have you looked at using ELF more efficiently instead of duplicating
kdump code into QEMU?  kdump is not a great format for the problem
you're trying to solve - you're not filling in the Linux-specific
metadata and it's a pain to write due to its layout.

Why can't you simply omit zero pages from the ELF?

Why can't you compress the entire ELF file and add straightforward
decompression to the crash utility?

Stefan

[Qemu-devel] [RFC] qemu-img: add option -d in convert

2013-06-20 Thread Wenchao Xia

Hi,
  This is a draft design which aimed for internal snapshot convert,
hope to get your comments:

  Internal snapshot is not as easy as external snapshot, to query and
convert. This patch will improve convertion side, which helps internal
/ external snapshot mixed case. With it user can treat internal
snapshot as lineraity relationship, use it like external ones with
tool qemu-img.


An detailed example, If there is a chain as following:

imageA(sn0)-imageB(sn0,sn1)-imageC(sn0)

The real relationship in it could be:
--imageA.qcow2imageB.qcow2-imageC.qcow2
|-imageA(sn0)   |-imageB(sn0)|-imageC(sn0)
 |-imageB(sn1)

To export it, two steps:
1. duplicate them to get an exactly same tree by:
qemu-img convert imageA.qcow2 -O export/imageA.qcow2 -f qcow2
qemu-img convert imageA.qcow2 -s sn0 -O export/imageA_sn0.qcow2
qemu-img convert imageB.qcow2 -O export/imageB.qcow2 -f qcow2 -o
backing_file=export/imageA.qcow2
qemu-img convert imageB.qcow2 -s sn0 -O export/imageB.qcow2 -f qcow2 -o
backing_file=export/imageB.qcow2
...

result at ./export:
--imageA.qcow2imageB.qcow2-imageC.qcow2
|-imageA_sn0.qcow2  |-imageB_sn0.qcow2   |-imageC_sn0.qcow2
 |-imageB_sn1.qcow2

2. change the relationship to linearity to save space(or by 3rd party
diff tool):
qemu-img create imageA_l.qcow2 -f qcow2 -p backing_file=imageA_qcow2
qemu-img rebase imageA_l.qcow2 -b imageA_sn0.qcow2
qemu-img rebase -u imageB.qcow2 -b imageA_l.qcow2
discard imageA.qcow2


result at ./export:
imageA_sn0.qcow2--imageA_l.qcow2--imageB_sn0.qcow2--imageB_sn1_l.qcow2-
-imageB_l.qcow2--imageC_sn0.qcow2--imageC_l.qcow2


This is a bit complexity, they can be merged into one step, to save
disk I/O and make procedure simple, add a parameter:
[-d [base_image=IMAGE,]snapshot=SNAPSHOT]

qemu-img convert imageA.qcow2 -s sn0 -O export/imageA_sn0.qcow2 -f qcow2
qemu-img convert imageA.qcow2 -d snapshot=sn0 -O export/imageA.qcow2 -f
qcow2 -o backing_file=export/imageA_sn0.qcow2
...

result at ./export:
imageA_sn0.qcow2--imageA.qcow2--imageB_sn0.qcow2--imageB_sn1.qcow2-
-imageB.qcow2--imageC_sn0.qcow2--imageC.qcow2

parameter base_image allow diff operation taken across image in the
backing chain.

Note:
  1 snapshot query can be added in qemu-nbd easily later.
  2 This is actually a work around by qemu-img and qemu-nbd. A better
way is to provide user snapshot_read() and snapshot_allocated()
interface, typically a library. But that need some adjust in block
level, especially thread, coroutine, and emulator cut off, so delay
that.

-- 
Best Regards

Wenchao Xia

[Qemu-devel] 1192847 : NMI watchdog fails to increment the NMI counter in /proc/interrupts

2013-06-20 Thread chandrashekar shastri


Hi All,

I have filed the following bug for watchdog:

NMI watchdog fails to increment the NMI counter in /proc/interrupts

Kernel Version: 3.10.0-rc5+
Libvirt Version: 1.0.6
Qemu Version: 1.5.50

Steps to reproduce the issue:

1. Booted the VM with :
qemu-system-x86_64 VM1.qcow2 -enable-kvm -watchdog i6300esb 
-watchdog-action reset -smp 2 -m 2000
2. Edit the /boot/grub/grub/grub.conf with nmi_watchdog = 1 before the 
initrd image.

3. Restart the guests, the NMI counter in /proc/interrupts was 0
4. Installed the watchdog rpm and ran chkconfig watchdog on
5. Restart the guest, even then the NMI counter did not increment
6. Changed the /boot/grub/grub/grub.conf with nmi_watchdog = 1 to 
/boot/grub/grub/grub.conf with nmi_watchdog = 2
and restarted the guest. Even then NMI conuter did not increment (The 
NMI counter was showing 0 all the time for all the above steps).


Please let me know if I am missing some steps to test the NMI.

Thanks,
Shastri

[Qemu-devel] Failed booting into OS after introduct the KVM_MEM_READONLY flag for regions

2013-06-20 Thread Zhangleiqiang

HI, Jordan:

By using the latest master of qemu, after booting the vnc view will 
continue to be just black, and the os cannot be start. After bisect, I found 
the problem was introduced by the commit: 
235e8982ad393e5611cb892df54881c872eea9e1 (kvm: support using KVM_MEM_READONLY 
flag for regions). There are plenty of medications in this commit, so I report 
this problem to you. 
The running environment:

Qemu: 4eda32f588086b6cd0ec2be6a7a6c131f8c2b427
Host： Fedora 18， kernel： 3.8.1-201.fc18.x86_64
Boot Cmd： 
./x86_64-softmmu/qemu-system-x86_64 -name ovirt -M pc-0.15 -m 1024 
-enable-kvm -boot d \
-drive file=/pkgs/imgs/win7.qcow2,format=qcow2,id=drive0,if=none \
-device ide-hd,drive=drive0,id=disk0 \
-vnc 186.100.8.144:0 -monitor stdio \
-net tap,ifname=tap0,downscript=no -net nic

I can provide more information if need.


--
Leiqzhang

Best Regards

Re: [Qemu-devel] [PATCH v2 2/2] QEMUBH: make AioContext's bh re-entrant

On Wed, Jun 19, 2013 at 11:27:25AM +0200, Paolo Bonzini wrote:
 Il 19/06/2013 00:26, mdroth ha scritto:
  On Tue, Jun 18, 2013 at 09:20:26PM +0200, Paolo Bonzini wrote:
  Il 18/06/2013 17:14, mdroth ha scritto:
  Could we possibly simplify this by introducing a recursive mutex that we
  could use to protect the whole list loop and hold even during the cb?
 
  If it is possible, we should avoid recursive locks.  It makes impossible
  to establish a lock hierarchy.  For example:
 
  I assume we can't hold the lock during the cb currently since we might
  try to reschedule, but if it's a recursive mutex would that simplify
  things?
 
  If you have two callbacks in two different AioContexts, both of which do
  bdrv_drain_all(), you get an AB-BA deadlock
  
  I think I see what you mean. That problem exists regardless of whether we
  introduce a recursive mutex though right?
 
 Without a recursive mutex, you only hold one lock at a time in each thread.
 
  I guess the main issue is the
  fact that we'd be encouraging sloppy locking practices without
  addressing the root problem?
 
 Yeah.  We're basically standing where the Linux kernel stood 10 years
 ago (let's say 2.2 timeframe).  If Linux got this far without recursive
 mutexes, we can at least try. :)

FWIW I was also looking into recursive mutexes for the block layer.
What scared me a little is that they make it tempting to stop thinking
about locks since you know you'll be able to reacquire locks you already
hold.

Especially when converting existing code, I think we need to be rigorous
about exploring every function and thinking about the locks it needs and
which child functions it calls.

Otherwise we'll have code paths hidden away somewhere that were never
truly thought through.

Stefan

Re: [Qemu-devel] [PATCH v3 2/2] QEMUBH: make AioContext's bh re-entrant

On Thu, Jun 20, 2013 at 4:16 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 20/06/2013 09:39, Stefan Hajnoczi ha scritto:
 qemu_bh_cancel() and qemu_bh_delete() are not modified by this patch.

 It seems that calling them from a thread is a little risky because there
 is no guarantee that the BH is no longer invoked after a thread calls
 these functions.

 I think that's worth a comment or do you want them to take the lock so
 they become safe?

 Taking the lock wouldn't help.  The invoking loop of aio_bh_poll runs
 lockless.  I think a comment is better.

Yes, will document it.
 qemu_bh_cancel is inherently not thread-safe, there's not much you can
 do about it.

 qemu_bh_delete is safe as long as you wait for the bottom half to stop
 before deleting the containing object.  Once we have RCU, deletion of
 QOM objects will be RCU-protected.  Hence, a simple way could be to put
 the first part of aio_bh_poll() within rcu_read_lock/unlock.

 The other thing I'm unclear on is the -idle assignment followed
 immediately by a -scheduled assignment.  Without memory barriers
 aio_bh_poll() isn't guaranteed to get an ordered view of these updates:
 it may see an idle BH as a regular scheduled BH because -idle is still
 0.

 Right.  You need to order -idle writes before -scheduled writes, and
 add memory barriers, or alternatively use two bits in -scheduled so
 that you can assign both atomically.

I think just shift the position of smp_rmb/wmb in _schedule and _poll,
we can acheive this (callbacks will not refer to -idle)

Regards,
Pingfan

 Paolo

Re: [Qemu-devel] [PATCH v2 3/6] net: make netclient re-entrant with refcnt

On Tue, Jun 18, 2013 at 8:41 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
 On Thu, Jun 13, 2013 at 05:03:03PM +0800, Liu Ping Fan wrote:
 From: Liu Ping Fan pingf...@linux.vnet.ibm.com

 With refcnt, NetClientState's user can run agaist deleter.

 Please split this into two patches:

 1. net_clients lock
 2. NetClientState refcount

Ok.

 Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com
 ---
  hw/core/qdev-properties-system.c | 14 
  include/net/net.h|  3 +++
  net/hub.c|  3 +++
  net/net.c| 47 
 +---
  net/slirp.c  |  3 ++-
  5 files changed, 66 insertions(+), 4 deletions(-)

 diff --git a/hw/core/qdev-properties-system.c 
 b/hw/core/qdev-properties-system.c
 index 0eada32..41cc7e6 100644
 --- a/hw/core/qdev-properties-system.c
 +++ b/hw/core/qdev-properties-system.c
 @@ -302,6 +302,7 @@ static void set_vlan(Object *obj, Visitor *v, void 
 *opaque,
  return;
  }

 +/* inc ref, released when unset property */
  hubport = net_hub_port_find(id);
  if (!hubport) {
  error_set(errp, QERR_INVALID_PARAMETER_VALUE,
 @@ -311,11 +312,24 @@ static void set_vlan(Object *obj, Visitor *v, void 
 *opaque,
  *ptr = hubport;
  }

 +static void release_vlan(Object *obj, const char *name, void *opaque)
 +{
 +DeviceState *dev = DEVICE(obj);
 +Property *prop = opaque;
 +NICPeers *peers_ptr = qdev_get_prop_ptr(dev, prop);
 +NetClientState **ptr = peers_ptr-ncs[0];
 +
 +if (*ptr) {
 +netclient_unref(*ptr);
 +}
 +}
 +
  PropertyInfo qdev_prop_vlan = {
  .name  = vlan,
  .print = print_vlan,
  .get   = get_vlan,
  .set   = set_vlan,
 +.release = release_vlan,
  };

  int qdev_prop_set_drive(DeviceState *dev, const char *name,

 What about the netdev property?  I don't see any refcount code there.

Yes, the release of netdev and vlan property should all free its
backend. Will add the code.
 @@ -1109,6 +1146,7 @@ void net_cleanup(void)
  qemu_del_net_client(nc);
  }
  }
 +qemu_mutex_destroy(net_clients_lock);

 Why is it okay to iterate over net_clients here without the lock?

 atexit(net_cleanup); So no other racers exist.

Thx  Regards,
Pingfan

Re: [Qemu-devel] [PATCH v2 2/6] net: introduce lock to protect NetClientState's peer's access

On Thu, Jun 20, 2013 at 3:46 PM, Stefan Hajnoczi stefa...@redhat.com wrote:
 On Thu, Jun 20, 2013 at 02:30:30PM +0800, liu ping fan wrote:
 On Tue, Jun 18, 2013 at 8:25 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
  On Thu, Jun 13, 2013 at 05:03:02PM +0800, Liu Ping Fan wrote:
  + * And flush out peer's queue.
  + */
  +static void qemu_net_client_detach_flush(NetClientState *nc)
  +{
  +NetClientState *peer;
  +
  +/* reader of self's peer field , fixme? the deleters are not 
  concurrent,
  + * so this pair lock can save.
  + */
 
  Indentation, also please resolve the fixme.
 
 So, here can I take the assumption that the deleters are serialized by
 biglock, and remove the lock following this comment?

 Ah, I understand the comment now.  Is there any advantage to dropping

:), only two atomic instruction in rare path.
 the lock?  IMO it's clearer to take the lock consistently instead of
 optimizing cases we think only get called from the main loop.

Reasonable, will keep them.

Re: [Qemu-devel] [PATCH v3 2/2] QEMUBH: make AioContext's bh re-entrant

Il 20/06/2013 11:12, liu ping fan ha scritto:
 Right.  You need to order -idle writes before -scheduled writes, and
 add memory barriers, or alternatively use two bits in -scheduled so
 that you can assign both atomically.

 I think just shift the position of smp_rmb/wmb in _schedule and _poll,
 we can acheive this (callbacks will not refer to -idle)

Yes, but you also need to swap -idle and -scheduled assignments
(aio_bh_poll reads scheduled before idle; qemu_bh_schedule* must write
idle before scheduled).

Paolo

Re: [Qemu-devel] [Xen-devel] [PATCH] Add Xen platform PCI device version 2.

 -Original Message-
 From: Tim Deegan [mailto:t...@xen.org]
 Sent: 20 June 2013 09:56
 To: Paul Durrant
 Cc: Matt Wilson; Alex Bligh; xen-de...@lists.xen.org; Ian Campbell; qemu-
 de...@nongnu.org
 Subject: Re: [Xen-devel] [Qemu-devel] [PATCH] Add Xen platform PCI device
 version 2.

 At 07:47 + on 20 Jun (1371714432), Paul Durrant wrote:
I agree. If this is really the only solution, we would need to have
both versions presented to the guest so that old drivers continue to
work without any intervention.

   I suspect that if we expose both, both sets of drivers try to run the
   same PV connections, and hilarity ensues.

  Actually I think I can make that work, and it is the conclusion I came
  to after Alex's comment.

 Ah, nice!  In that case, I'm a lot less worried -- we can just expose
 both versions/devices by default and there's no need for a visible
 control knob tied to driver version (except maybe for debugging).

 It means an 'unsupported' device appearing on other/older OSes, which is
 unfortunate, but ISTR only Windows really complains visibly about that.

Yes, I think only Windows complains and we should be able to post an article 
somewhere saying 'don't worry about it' :-)

  Paul

[Qemu-devel] [Bug 994378] Re: Nested-virt)L1 (kvm on kvm)guest panic with parameter “-cpu host” in qemu command line.

2013-06-20 Thread Kashyap Chamarthy

Short: I can't reproduce here with L1 guest having has host-passthrough
for CPU.

Long:
=

Version Info:
-

On Physical host:
~
$ uname -r; rpm -q libvirt-daemon-kvm qemu
3.10.0-0.rc2.git1.2.fc20.x86_64
qemu-1.4.2-3.fc19.x86_64
libvirt-daemon-kvm-1.0.5.2-1.fc19.x86_64
libguestfs-1.22.3-1.fc19.x86_64


On L1:
~~
$ uname -r; rpm -q libvirt-daemon-kvm qemu
3.10.0-0.rc3.git0.2.fc20.x86_64
libvirt-daemon-kvm-1.0.5.1-1.fc19.x86_64
qemu-1.4.2-2.fc19.x86_64
[root@dhcp47-209 ~]# 


L1 guest CLI:
-
[root@bare-metal ~]# ps -ef | grep qemu
qemu  7281 1 67 04:57 ?00:00:10 /usr/bin/qemu-system-x86_64 
-machine accel=kvm -name regular-guest -S -machine 
pc-i440fx-1.4,accel=kvm,usb=off -cpu host -m 10240 -smp 
4,sockets=4,cores=1,threads=1 -uuid 4ed9ac0b-7f72-dfcf-68b3-e6fe2ac588b2 
-nographic -no-user-config -nodefaults -chardev 
socket,id=charmonitor,path=/var/lib/libvirt/qemu/regular-guest.monitor,server,nowait
 -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown 
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -drive 
file=/home/test/vmimages/regular-guest.qcow2,if=none,id=drive-virtio-disk0,format=qcow2,cache=none
 -device 
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
 -netdev tap,fd=23,id=hostnet0,vhost=on,vhostfd=24 -device 
virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:80:c1:34,bus=pci.0,addr=0x3 
-chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 
-device usb-tablet,id=input0 -device 
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5


L2 guest CLI:
-
[root@regular-guest ~]# ps -ef | grep -i qemu
qemu  1138 1 88 05:18 ?00:00:07 /usr/bin/qemu-system-x86_64 
-machine accel=kvm -name nguest-01 -S -machine pc-i440fx-1.4,accel=kvm,usb=off 
-m 2048 -smp 2,sockets=2,cores=1,threads=1 -uuid 
b47c5cbb-b320-ce9d-c595-4e083b0e465d -nographic -no-user-config -nodefaults 
-chardev 
socket,id=charmonitor,path=/var/lib/libvirt/qemu/nguest-01.monitor,server,nowait
 -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown 
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -drive 
file=/home/test/vmimages/nguest-01.qcow2,if=none,id=drive-virtio-disk0,format=qcow2,cache=none
 -device 
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
 -netdev tap,fd=23,id=hostnet0,vhost=on,vhostfd=24 -device 
virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:be:d5:8e,bus=pci.0,addr=0x3 
-chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 
-device usb-tablet,id=input0 -device 
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5


A search for string 'error' in logs doesn't turn up anything:
[root@nguest-01 ~]# grep -i error /var/log/boot.log 
[root@nguest-01 ~]# grep -i error /var/log/messages
[root@nguest-01 ~]# 


Yongjie, can you please re-try?

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

Title:
  Nested-virt)L1 (kvm on kvm)guest panic with parameter “-cpu host” in
  qemu command line.

Status in QEMU:
  New

Bug description:
  Environment:
  
  Host OS (ia32/ia32e/IA64):ia32e
  Guest OS (ia32/ia32e/IA64):ia32e
  Guest OS Type (Linux/Windows):Linux
  kvm.git Commit:19853301ef3289bda2d5264c1093e74efddaeab9
  qemu-kvm Commit:69abebf20280152da8fa7c418a819ae51e862231
  Host Kernel Version:3.4.0-rc3
  Hardware:WSM-EP, Romley-EP

  
  Bug detailed description:
  --
  (KVM on KVM) L1 guest panic when starting the L1 guest with “-cpu host” 
parameter in qemu command line.

  Note:
  1. when creating guest with “-cpu qemu64,+vmx”, L1 guest and L2 guest can boot
  up. 
  2. This should be a qemu-kvm bug. using '-cpu host' parameter, the following 
is the result.
  Kvm+ qemu-kvm =result
  19853301 + 69abebf2  = bad
  19853301 + 44755ea3  = good
  3. when booting up the guest with  the good commit of 19853301 + 44755ea3, 
you can see some
  error info, but nested virt works fine. (L1 and L2 guest can boot up.)
  “error: feature i64 not available in set
  error: bad option value [extfeature_edx = i64 xd syscall]” 

  some logs 
  [root@vt-snb9 x86_64-softmmu]# ./qemu-system-x86_64 -m 2048 -net 
nic,model=rtl8139 -net tap,script=/etc/kvm/qemu-ifup -hda /root/nested-kvm.qcow 
-cpu host
  error: feature i64 not available in set
  error: bad option value [extfeature_edx = i64 xd syscall]
  error: feature i64 not available in set
  error: bad option value [extfeature_edx = i64 xd syscall]
  error: feature i64 not available in set
  error: bad option value [extfeature_edx = i64 syscall xd]
  error: feature i64 not available in set
  error: bad option value [extfeature_edx = i64 syscall xd]
  VNC server running on `::1:5900'


  Reproduce steps:
  
  1.start up a host with kvm (commit: 19853301)
  2.rmmod

Re: [Qemu-devel] [PATCH 6/9] vhost-scsi: new device supporting the tcm_vhost Linux kernel module

2013-06-20 Thread Asias He

On Thu, Jun 20, 2013 at 08:49:50AM +, Libaiqing wrote:
 Hi Asias,
 Thanks for your config.
 According to you config,I test booting from vhost device with upstream 
 kernel and qemu,but failed.
 
 1 installing guest from cdrom,ok.
 2 booting vhost-scsi,guest fs error occurs. 
 3 using fileio backstores,the error is same..
 4 rebooting guest,a log printed:
  (qemu) hw/scsi/virtio-scsi.c:533:virtio_scsi_handle_event: Object 
 0x7fccae7f2c88 is not an instance of type virtio-scsi-device

Paolo, I remember you fixed a similar issue?

 5 using upstream seabios,core dumped.
  
 Could you give me some advise to debug this problem ? I can provide more 
 information if need.

Can you show me qemu commit id you used? Can you verity that if using the
host kernel for guest helps? Does booting directly (without the install
and reboot process) work?

 The qemu cmd:
 [root@fedora121 x86_64-softmmu]# ./qemu-system-x86_64 -enable-kvm -name 
 fedora   -M pc -m 1024 -smp 2   -drive 
 file=/home/fedora18.iso,if=ide,media=cdrom -device 
 vhost-scsi-pci,wwpn=naa.50014057133e25dc  -monitor stdio   -vga qxl  -vnc :1
 
 The vnc output:
 Dracut-initqueue[189]:/dev/mapper/fedora-root:UNEXPECTED INCONSISTENCY;RUN 
 FSCK MANUALLY.
 Dracut-initqueue[189]: Warning: e2fsck returned with 4
 Dracut-initqueue[189]: Warning: ***An error occurred during the file system 
 check.
 
 The guest kernel log:
 Kernel: virtio-pci :00:04.0: irq 40 for MSI/MSI-X
 Kernel: virtio-pci :00:04.0: irq 41 for MSI/MSI-X
 Kernel: virtio-pci :00:04.0: irq 42 for MSI/MSI-X
 Kernel: virtio-pci :00:04.0: irq 43 for MSI/MSI-X
 Kernel: scsi2 : Virtio SCSI HBA
 Kernel: scsi 2:0:1:0: Direct-Access LIO-ORG r0
 Kernel: sd 2:0:1:0: Attached scsi generic sg1 type 0
 Kernel: sd 2:0:1:0: [sda]1258912 512-byte logical .
 Kernel: sd 2:0:1:0: [sda]write protect is off
 Kernel: sd 2:0:1:0: [sda]Mode sense :43 00 00 08
 Kernel: sd 2:0:1:0: [sda]write cache: disabled, read .
 Kernel: sda sda1 sda2
 Kernel: sd 2:0:1:0: [sda] Attached SCSI disk
 Dracut-initqueue[189]: Scanning devices sda2 for LVM
 Dracut-initqueue[189]: inactive '/dev/fedora/swap'...
 Dracut-initqueue[189]: inactive '/dev/fedora/root'...
 
 The info of host:
 [root@fedora121 x86_64-softmmu]# uname -a
 Linux fedora121 3.10.0-rc6 #1 SMP Wed Jun 19 19:34:24 CST 2013 x86_64 x86_64 
 x86_64 GNU/Linux
 [root@fedora121 x86_64-softmmu]# lsmod |grep vhost_scsi
 vhost_scsi 49456  5
 target_core_mod   282163  14 
 target_core_iblock,target_core_pscsi,iscsi_target_mod,target_core_file,vhost_scsi
 [root@fedora121 x86_64-softmmu]# targetcli
 targetcli shell version v2.1.fb26
 Copyright 2011 by RisingTide Systems LLC and others.
 For help on commands, type 'help'.
 
 / ls
 o- / 
 .
  [...]
   o- backstores 
 ..
  [...]
   | o- block 
 ..
  [Storage Objects: 0]
   | o- fileio 
 .
  [Storage Objects: 0]
   | o- pscsi 
 ..
  [Storage Objects: 0]
   | o- ramdisk 
 
  [Storage Objects: 1]
   |   o- r0 
 ...
  [(6.0GiB) activated]
   o- iscsi 
 
  [Targets: 0]
   o- loopback 
 .
  [Targets: 0]
   o- vhost 
 
  [Targets: 1]
 o- naa.50014057133e25dc 
 ..
  [TPGs: 1]
   o- tpg1 
 ...
  [naa.5001405a70ac3421]
 o- acls 
 ..
  [ACLs: 0]
 o- luns 
 ..
  [LUNs: 1]
   o- lun0 
 .
  [ramdisk/r0]
 
 Regards,
 baiqing
  -Original Message-
  From: Asias He [mailto:as...@redhat.com]
  Sent: Thursday, June 20, 2013 9:34 AM
  To: Libaiqing

Re: [Qemu-devel] [PATCH v3 2/2] QEMUBH: make AioContext's bh re-entrant

On Thu, Jun 20, 2013 at 4:16 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 20/06/2013 09:39, Stefan Hajnoczi ha scritto:
 qemu_bh_cancel() and qemu_bh_delete() are not modified by this patch.

 It seems that calling them from a thread is a little risky because there
 is no guarantee that the BH is no longer invoked after a thread calls
 these functions.

 I think that's worth a comment or do you want them to take the lock so
 they become safe?

 Taking the lock wouldn't help.  The invoking loop of aio_bh_poll runs
 lockless.  I think a comment is better.

 qemu_bh_cancel is inherently not thread-safe, there's not much you can
 do about it.

 qemu_bh_delete is safe as long as you wait for the bottom half to stop
 before deleting the containing object.  Once we have RCU, deletion of
 QOM objects will be RCU-protected.  Hence, a simple way could be to put
 the first part of aio_bh_poll() within rcu_read_lock/unlock.

In fact, I have some idea about this,  introduce another member -
Object for QEMUBH which will be refereed in cb, then we leave anything
to refcnt mechanism.
For qemu_bh_cancel(), I do not figure out whether it is important or
not to sync with caller.

diff --git a/async.c b/async.c
index 4b17eb7..60c35a1 100644
--- a/async.c
+++ b/async.c
@@ -61,6 +61,7 @@ int aio_bh_poll(AioContext *ctx)
 {
 QEMUBH *bh, **bhp, *next;
 int ret;
+int sched;

 {
 QEMUBH *bh, **bhp, *next;
 int ret;
+int sched;

 ctx-walking_bh++;

@@ -69,8 +70,10 @@ int aio_bh_poll(AioContext *ctx)
 /* Make sure fetching bh before accessing its members */
 smp_read_barrier_depends();
 next = bh-next;
-if (!bh-deleted  bh-scheduled) {
-bh-scheduled = 0;
+sched = 0;
+atomic_xchg(bh-scheduled, sched);
+if (!bh-deleted  sched) {
+//bh-scheduled = 0;
 if (!bh-idle)
 ret = 1;
 bh-idle = 0;
@@ -79,6 +82,9 @@ int aio_bh_poll(AioContext *ctx)
  */
 smp_rmb();
 bh-cb(bh-opaque);
+if (bh-obj) {
+object_unref(bh-obj);
+}
 }
 }

@@ -105,8 +111,12 @@ int aio_bh_poll(AioContext *ctx)

 void qemu_bh_schedule_idle(QEMUBH *bh)
 {
-if (bh-scheduled)
+int sched = 1;
+
+atomic_xchg( bh-scheduled, sched);
+if (sched) {
 return;
+}
 /* Make sure any writes that are needed by the callback are done
  * before the locations are read in the aio_bh_poll.
  */
@@ -117,25 +127,46 @@ void qemu_bh_schedule_idle(QEMUBH *bh)

 void qemu_bh_schedule(QEMUBH *bh)
 {
-if (bh-scheduled)
+int sched = 1;
+
+atomic_xchg( bh-scheduled, sched);
+if (sched) {
 return;
+}
 /* Make sure any writes that are needed by the callback are done
  * before the locations are read in the aio_bh_poll.
  */
 smp_wmb();
 bh-scheduled = 1;
+if (bh-obj) {
+object_ref(bh-obj);
+}
 bh-idle = 0;
 aio_notify(bh-ctx);
 }

 void qemu_bh_cancel(QEMUBH *bh)
 {
-bh-scheduled = 0;
+int sched = 0;
+
+atomic_xchg( bh-scheduled, sched);
+if (sched) {
+if (bh-obj) {
+object_ref(bh-obj);
+}
+}
 }

 void qemu_bh_delete(QEMUBH *bh)
 {
-bh-scheduled = 0;
+int sched = 0;
+
+atomic_xchg( bh-scheduled, sched);
+if (sched) {
+if (bh-obj) {
+object_ref(bh-obj);
+}
+}
 bh-deleted = 1;
 }

Regards,
Pingfan
 The other thing I'm unclear on is the -idle assignment followed
 immediately by a -scheduled assignment.  Without memory barriers
 aio_bh_poll() isn't guaranteed to get an ordered view of these updates:
 it may see an idle BH as a regular scheduled BH because -idle is still
 0.

 Right.  You need to order -idle writes before -scheduled writes, and
 add memory barriers, or alternatively use two bits in -scheduled so
 that you can assign both atomically.

 Paolo

Re: [Qemu-devel] [PATCH v3 2/2] QEMUBH: make AioContext's bh re-entrant

Il 20/06/2013 11:41, liu ping fan ha scritto:
 On Thu, Jun 20, 2013 at 4:16 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 20/06/2013 09:39, Stefan Hajnoczi ha scritto:
 qemu_bh_cancel() and qemu_bh_delete() are not modified by this patch.

 It seems that calling them from a thread is a little risky because there
 is no guarantee that the BH is no longer invoked after a thread calls
 these functions.

 I think that's worth a comment or do you want them to take the lock so
 they become safe?

 Taking the lock wouldn't help.  The invoking loop of aio_bh_poll runs
 lockless.  I think a comment is better.

 qemu_bh_cancel is inherently not thread-safe, there's not much you can
 do about it.

 qemu_bh_delete is safe as long as you wait for the bottom half to stop
 before deleting the containing object.  Once we have RCU, deletion of
 QOM objects will be RCU-protected.  Hence, a simple way could be to put
 the first part of aio_bh_poll() within rcu_read_lock/unlock.

 In fact, I have some idea about this,  introduce another member -
 Object for QEMUBH which will be refereed in cb, then we leave anything
 to refcnt mechanism.
 For qemu_bh_cancel(), I do not figure out whether it is important or
 not to sync with caller.

This is a separate patch anyway... and a long discussion to have before
too. :)

Let's concentrate on one thing at a time.

Paolo

 diff --git a/async.c b/async.c
 index 4b17eb7..60c35a1 100644
 --- a/async.c
 +++ b/async.c
 @@ -61,6 +61,7 @@ int aio_bh_poll(AioContext *ctx)
  {
  QEMUBH *bh, **bhp, *next;
  int ret;
 +int sched;
 
  {
  QEMUBH *bh, **bhp, *next;
  int ret;
 +int sched;
 
  ctx-walking_bh++;
 
 @@ -69,8 +70,10 @@ int aio_bh_poll(AioContext *ctx)
  /* Make sure fetching bh before accessing its members */
  smp_read_barrier_depends();
  next = bh-next;
 -if (!bh-deleted  bh-scheduled) {
 -bh-scheduled = 0;
 +sched = 0;
 +atomic_xchg(bh-scheduled, sched);

This is expensive.

 +if (!bh-deleted  sched) {
 +//bh-scheduled = 0;
  if (!bh-idle)
  ret = 1;
  bh-idle = 0;
 @@ -79,6 +82,9 @@ int aio_bh_poll(AioContext *ctx)
   */
  smp_rmb();
  bh-cb(bh-opaque);
 +if (bh-obj) {
 +object_unref(bh-obj);
 +}
  }
  }
 
 @@ -105,8 +111,12 @@ int aio_bh_poll(AioContext *ctx)
 
  void qemu_bh_schedule_idle(QEMUBH *bh)
  {
 -if (bh-scheduled)
 +int sched = 1;
 +
 +atomic_xchg( bh-scheduled, sched);
 +if (sched) {
  return;
 +}
  /* Make sure any writes that are needed by the callback are done
   * before the locations are read in the aio_bh_poll.
   */
 @@ -117,25 +127,46 @@ void qemu_bh_schedule_idle(QEMUBH *bh)
 
  void qemu_bh_schedule(QEMUBH *bh)
  {
 -if (bh-scheduled)
 +int sched = 1;
 +
 +atomic_xchg( bh-scheduled, sched);
 +if (sched) {
  return;
 +}
  /* Make sure any writes that are needed by the callback are done
   * before the locations are read in the aio_bh_poll.
   */
  smp_wmb();
  bh-scheduled = 1;
 +if (bh-obj) {
 +object_ref(bh-obj);
 +}
  bh-idle = 0;
  aio_notify(bh-ctx);
  }
 
  void qemu_bh_cancel(QEMUBH *bh)
  {
 -bh-scheduled = 0;
 +int sched = 0;
 +
 +atomic_xchg( bh-scheduled, sched);
 +if (sched) {
 +if (bh-obj) {
 +object_ref(bh-obj);
 +}
 +}
  }
 
  void qemu_bh_delete(QEMUBH *bh)
  {
 -bh-scheduled = 0;
 +int sched = 0;
 +
 +atomic_xchg( bh-scheduled, sched);
 +if (sched) {
 +if (bh-obj) {
 +object_ref(bh-obj);
 +}
 +}
  bh-deleted = 1;
  }
 
 Regards,
 Pingfan
 The other thing I'm unclear on is the -idle assignment followed
 immediately by a -scheduled assignment.  Without memory barriers
 aio_bh_poll() isn't guaranteed to get an ordered view of these updates:
 it may see an idle BH as a regular scheduled BH because -idle is still
 0.

 Right.  You need to order -idle writes before -scheduled writes, and
 add memory barriers, or alternatively use two bits in -scheduled so
 that you can assign both atomically.

 Paolo

Re: [Qemu-devel] Adding a persistent writeback cache to qemu

On Wed, Jun 19, 2013 at 10:28:53PM +0100, Alex Bligh wrote:
 --On 11 April 2013 11:25:48 +0200 Stefan Hajnoczi
 stefa...@gmail.com wrote:
 
 I'd like to experiment with adding persistent writeback cache to qemu.
 The use case here is where non-local storage is used (e.g. rbd, ceph)
 using the qemu drivers, together with a local cache as a file on
 a much faster locally mounted device, for instance an SSD (possibly
 replicated). This would I think give a similar performance boost to
 using an rbd block device plus flashcache/dm-cache/bcache, but without
 introducing all the context switches and limitations of having to
 use real block devices. I appreciate it would need to be live migration
 aware (worst case solution: flush and turn off caching during live
 migrate), and ideally be capable of replaying a dirty writeback cache
 in the event the host crashes.
 
 Is there any support for this already? Has anyone worked on this before?
 If not, would there be any interest in it?
 
 I'm concerned about the complexity this would introduce in QEMU.
 Therefore I'm a fan of using existing solutions like the Linux block
 layer instead of reimplementing this stuff in Linux.
 
 What concrete issues are there with using rbd plus
 flashcache/dm-cache/bcache?
 
 I'm not sure I understand the context switch problem since implementing
 it in user space will still require system calls to do all the actual
 cache I/O.
 
 I failed to see your reply and got distracted from this. Apologies.
 So several months later ...

Happens to me sometimes too ;-).

 The concrete problem here is that flashcache/dm-cache/bcache don't
 work with the rbd (librbd) driver, as flashcache/dm-cache/bcache
 cache access to block devices (in the host layer), and with rbd
 (for instance) there is no access to a block device at all. block/rbd.c
 simply calls librbd which calls librados etc.
 
 So the context switches etc. I am avoiding are the ones that would
 be introduced by using kernel rbd devices rather than librbd.

I understand the limitations with kernel block devices - their
setup/teardown is an extra step outside QEMU and privileges need to be
managed.  That basically means you need to use a management tool like
libvirt to make it usable.

But I don't understand the performance angle here.  Do you have profiles
that show kernel rbd is a bottleneck due to context switching?

We use the kernel page cache for -drive file=test.img,cache=writeback
and no one has suggested reimplementing the page cache inside QEMU for
better performance.

Also, how do you want to manage QEMU page cache with multiple guests
running?  They are independent and know nothing about each other.  Their
process memory consumption will be bloated and the kernel memory
management will end up having to sort out who gets to stay in physical
memory.

You can see I'm skeptical of this and think it's premature optimization,
but if there's really a case for it with performance profiles then I
guess it would be necessary.  But we should definitely get feedback from
the Ceph folks too.

I'd like to hear from Ceph folks what their position on kernel rbd vs
librados is.  Why one do they recommend for QEMU guests and what are the
pros/cons?

CCed Sage and Josh

Stefan

Re: [Qemu-devel] [PATCH v3] vl.c: Support multiple CPU ranges on -numa option

Il 20/06/2013 11:30, Igor Mammedov ha scritto:
So, basically the format seemed easier to work with if we are 
thinking 
of using QemuOpts for -numa. Using -cpu rather than cpus probably
makes it less ambiguous as well IMO. However, it's probably not a 
good idea
if the current syntax is well established ?
  
  libvirt uses the cpus option already, so we have to keep it working.
 Sure, we can leave it as it's now for some time while a new interface is
 introduced/adopted. And than later deprecate cpus.

So, you used a new name because the new behavior of -numa
node,cpus=1-2,cpus=3-4 would be incompatible with the old.

Personally I don't think that's a problem, but I remember a long
discussion in the past.  Igor/Eduardo, do you remember the conclusions?

Paolo

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries

2013-06-20 Thread Peter Maydell

On 20 June 2013 10:49, Paolo Bonzini pbonz...@redhat.com wrote:
 This only leaves Darwin.  I have no idea about that, and I don't have
 anymore a machine to test it.  Andreas or Peter, can you shed light?

If you have something concrete you'd like me to test I can test it.

 But still, libtool wouldn't be a particularly problematic dependency.
 We're already using it for libcacard.

...and we're already disabling the probe for libtool in configure
on MacOSX because MacOS libtool is something completely different...

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH 0/4] per-object libraries