[Qemu-devel] buildbot failure in qemu on trivial-patches_i386_debian_6_0

2012-08-11 Thread qemu
The Buildbot has detected a new failure on builder 
trivial-patches_i386_debian_6_0 while building qemu.
Full details are available at:
 
http://buildbot.b1-systems.de/qemu/builders/trivial-patches_i386_debian_6_0/builds/356

Buildbot URL: http://buildbot.b1-systems.de/qemu/

Buildslave for this Build: yuzuki

Build Reason: The Nightly scheduler named 'nightly_trivial-patches' triggered 
this build
Build Source Stamp: [branch trivial-patches] HEAD
Blamelist: 

BUILD FAILED: failed git

sincerely,
 -The Buildbot



[Qemu-devel] buildbot failure in qemu on xen_i386_debian_6_0

2012-08-11 Thread qemu
The Buildbot has detected a new failure on builder xen_i386_debian_6_0 while 
building qemu.
Full details are available at:
 http://buildbot.b1-systems.de/qemu/builders/xen_i386_debian_6_0/builds/357

Buildbot URL: http://buildbot.b1-systems.de/qemu/

Buildslave for this Build: yuzuki

Build Reason: The Nightly scheduler named 'nightly_xen_next' triggered this 
build
Build Source Stamp: [branch xen-next] HEAD
Blamelist: 

BUILD FAILED: failed git

sincerely,
 -The Buildbot



[Qemu-devel] buildbot failure in qemu on xen_x86_64_debian_6_0

2012-08-11 Thread qemu
The Buildbot has detected a new failure on builder xen_x86_64_debian_6_0 while 
building qemu.
Full details are available at:
 http://buildbot.b1-systems.de/qemu/builders/xen_x86_64_debian_6_0/builds/356

Buildbot URL: http://buildbot.b1-systems.de/qemu/

Buildslave for this Build: yuzuki

Build Reason: The Nightly scheduler named 'nightly_xen_next' triggered this 
build
Build Source Stamp: [branch xen-next] HEAD
Blamelist: 

BUILD FAILED: failed git

sincerely,
 -The Buildbot



Re: [Qemu-devel] [PATCH v3 00/35]: add new error format

2012-08-11 Thread Markus Armbruster
Luiz Capitulino lcapitul...@redhat.com writes:

[...]
 This series implements the 'Plan for error handling in QMP' as described
 by Anthony in this email:

 http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg03764.html

 Basically, this replaces almost all error classes by GenericError (the
 exception are a few error classes used by libvirt) and drops the error
 data memeber. This also adds a free form string to error_set().

 On the wire, we go from:

 { error: { class: DeviceNotRemovable,
  data: { device: virtio0 },
  desc: Device 'virtio0' is not removable } }

 to:

  { error: { class: GenericError,
   desc: Device 'virtio0' is not removable } }

 Internally, we go from:

   void error_set(Error **err, const char *fmt, ...);

 to:

   void error_set(Error **err, ErrorClass err_class, const char *fmt, ...);

Glad to see this change in good shape in time for the release.  Thanks,
Luiz!

Reviewed-by: Markus Armbruster arm...@redhat.com



[Qemu-devel] buildbot failure in qemu on ppc-next_i386_debian_6_0

2012-08-11 Thread qemu
The Buildbot has detected a new failure on builder ppc-next_i386_debian_6_0 
while building qemu.
Full details are available at:
 http://buildbot.b1-systems.de/qemu/builders/ppc-next_i386_debian_6_0/builds/356

Buildbot URL: http://buildbot.b1-systems.de/qemu/

Buildslave for this Build: yuzuki

Build Reason: The Nightly scheduler named 'nightly_ppc-next' triggered this 
build
Build Source Stamp: [branch ppc-next] HEAD
Blamelist: 

BUILD FAILED: failed git

sincerely,
 -The Buildbot



[Qemu-devel] buildbot failure in qemu on ppc-next_x86_64_debian_6_0

2012-08-11 Thread qemu
The Buildbot has detected a new failure on builder ppc-next_x86_64_debian_6_0 
while building qemu.
Full details are available at:
 
http://buildbot.b1-systems.de/qemu/builders/ppc-next_x86_64_debian_6_0/builds/356

Buildbot URL: http://buildbot.b1-systems.de/qemu/

Buildslave for this Build: yuzuki

Build Reason: The Nightly scheduler named 'nightly_ppc-next' triggered this 
build
Build Source Stamp: [branch ppc-next] HEAD
Blamelist: 

BUILD FAILED: failed git

sincerely,
 -The Buildbot



Re: [Qemu-devel] [PATCH 11/34] qmp: query-block: add 'valid_encryption_key' field

2012-08-11 Thread Markus Armbruster
Luiz Capitulino lcapitul...@redhat.com writes:

 On Fri, 10 Aug 2012 19:17:22 +0200
 Markus Armbruster arm...@redhat.com wrote:

 Luiz Capitulino lcapitul...@redhat.com writes:
 
  On Fri, 10 Aug 2012 18:35:26 +0200
  Markus Armbruster arm...@redhat.com wrote:
 
  Luiz Capitulino lcapitul...@redhat.com writes:
  
   On Fri, 10 Aug 2012 09:56:11 +0200
   Markus Armbruster arm...@redhat.com wrote:
  
   Revisited this one on review of v2, replying here for context.
   
   Luiz Capitulino lcapitul...@redhat.com writes:
   
On Thu, 02 Aug 2012 13:35:54 +0200
Markus Armbruster arm...@redhat.com wrote:
   
Luiz Capitulino lcapitul...@redhat.com writes:

 Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
 ---
  block.c  | 1 +
  qapi-schema.json | 7 +--
  2 files changed, 6 insertions(+), 2 deletions(-)

 diff --git a/block.c b/block.c
 index b38940b..9c113b8 100644
 --- a/block.c
 +++ b/block.c
 @@ -2445,6 +2445,7 @@ BlockInfoList *qmp_query_block(Error **errp)
  info-value-inserted-ro = bs-read_only;
  info-value-inserted-drv = 
 g_strdup(bs-drv-format_name);
  info-value-inserted-encrypted = bs-encrypted;
 +info-value-inserted-valid_encryption_key = 
 bs-valid_key;
  if (bs-backing_file[0]) {
  info-value-inserted-has_backing_file = true;
  info-value-inserted-backing_file = 
 g_strdup(bs-backing_file);
 diff --git a/qapi-schema.json b/qapi-schema.json
 index bc55ed2..1b2d7f5 100644
 --- a/qapi-schema.json
 +++ b/qapi-schema.json
 @@ -400,6 +400,8 @@
  #
  # @encrypted: true if the backing device is encrypted
  #
 +# @valid_encryption_key: true if a valid encryption key has been 
 set
 +#
  # @bps: total throughput limit in bytes per second is specified
  #
  # @bps_rd: read throughput limit in bytes per second is specified
 @@ -419,8 +421,9 @@
  { 'type': 'BlockDeviceInfo',
'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str',
  '*backing_file': 'str', 'encrypted': 'bool',
 -'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
 -'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} }
 +'valid_encryption_key': 'bool', 'bps': 'int',
 +'bps_rd': 'int', 'bps_wr': 'int', 'iops': 'int',
 +'iops_rd': 'int', 'iops_wr': 'int'} }
  
  ##
  # @BlockDeviceIoStatus:

BlockDeviceInfo is API, isn't it?
   
Yes.
   
Note that bs-valid_key currently implies bs-encrypted.
bs-valid_key
 !bs-encrypted is impossible.  Should we make 
valid_encryption_key
only available when encrypted?
   
I don't think so. It's a bool, so it's ok for it to be false when
encrypted is false.
   
   What bothers me is encrypted=false, valid_encryption_key=true.
  
   Disappearing keys is worse, IMHO (assuming that that situation
   is impossible
   in practice, of course).
  
  It's fundamentally three states: unencrypted, encrypted-no-key,
  encrypted-got-key.  I'm fine with mapping these onto two bools, it's how
  the block layer does it.  You may want to consider a single enumeration
  instead.
 
  That's arguable. But I like the bools slightly better because they allow
  clients to do a true/false check vs. having to check against an enum value.
 
  Again, that's arguable.
 
valid_encryption_key is a bit long for my taste.  Yours may be
different.
   
We should choose more descriptive and self-documenting names for the
protocol. Besides, I can't think of anything shorter that won't get
cryptic.
   
Suggestions are always welcome though :)
   
   valid_encryption_key sounds like the value is the valid key.
  
   That's exactly what it is.
  
  Err, isn't the value bool?  The key value is a string...
 
  Ah, sorry, I read sounds like true means the key is valid even for an
  invalid key. I've renamed it to encryption_key_missing, should be better
  (although I could also do encryption_key_is_missing).
 
   got_crypt_key?  Also avoids valid.  Good, because current encrypted
   formats don't actually validate the key; they happily accept any key.
  
   That's a block layer bug, not QMP's.
  
   QMP clients are going to be misguided by valid_encryption_key
   the same way
   they are with the block_passwd command or how we suffer from it
   internally
   when calling bdrv_set_key() (which also manifests itself in HMP).
  
   Fixing the bug where it is will automatically fix all its instances.
  
  It's not fixable for existing image formats, and thus existing images.
 
  Why not? I'd expect that changing AES_set_decrypt_key() to fail for an
  invalid key wouldn't affect images, am I wrong?
 
 AES_set_decrypt_key() and AES_set_encrypt_key() accept any key with 128,
 192 or 256 bits.  Decrypting with an incorrect key 

[Qemu-devel] buildbot failure in qemu on s390-next_x86_64_debian_6_0

2012-08-11 Thread qemu
The Buildbot has detected a new failure on builder s390-next_x86_64_debian_6_0 
while building qemu.
Full details are available at:
 
http://buildbot.b1-systems.de/qemu/builders/s390-next_x86_64_debian_6_0/builds/357

Buildbot URL: http://buildbot.b1-systems.de/qemu/

Buildslave for this Build: yuzuki

Build Reason: The Nightly scheduler named 'nightly_s390-next' triggered this 
build
Build Source Stamp: [branch s390-next] HEAD
Blamelist: 

BUILD FAILED: failed git

sincerely,
 -The Buildbot



[Qemu-devel] buildbot failure in qemu on s390-next_i386_debian_6_0

2012-08-11 Thread qemu
The Buildbot has detected a new failure on builder s390-next_i386_debian_6_0 
while building qemu.
Full details are available at:
 
http://buildbot.b1-systems.de/qemu/builders/s390-next_i386_debian_6_0/builds/357

Buildbot URL: http://buildbot.b1-systems.de/qemu/

Buildslave for this Build: yuzuki

Build Reason: The Nightly scheduler named 'nightly_s390-next' triggered this 
build
Build Source Stamp: [branch s390-next] HEAD
Blamelist: 

BUILD FAILED: failed git

sincerely,
 -The Buildbot



[Qemu-devel] buildbot failure in qemu on monitor_x86_64_debian_6_0

2012-08-11 Thread qemu
The Buildbot has detected a new failure on builder monitor_x86_64_debian_6_0 
while building qemu.
Full details are available at:
 
http://buildbot.b1-systems.de/qemu/builders/monitor_x86_64_debian_6_0/builds/357

Buildbot URL: http://buildbot.b1-systems.de/qemu/

Buildslave for this Build: yuzuki

Build Reason: The Nightly scheduler named 'nightly_monitor' triggered this build
Build Source Stamp: [branch queue/monitor] HEAD
Blamelist: 

BUILD FAILED: failed git

sincerely,
 -The Buildbot



[Qemu-devel] buildbot failure in qemu on monitor_i386_debian_6_0

2012-08-11 Thread qemu
The Buildbot has detected a new failure on builder monitor_i386_debian_6_0 
while building qemu.
Full details are available at:
 http://buildbot.b1-systems.de/qemu/builders/monitor_i386_debian_6_0/builds/357

Buildbot URL: http://buildbot.b1-systems.de/qemu/

Buildslave for this Build: yuzuki

Build Reason: The Nightly scheduler named 'nightly_monitor' triggered this build
Build Source Stamp: [branch queue/monitor] HEAD
Blamelist: 

BUILD FAILED: failed git

sincerely,
 -The Buildbot



Re: [Qemu-devel] [PATCH 09/15] memory: prepare flatview and radix-tree for rcu style access

2012-08-11 Thread liu ping fan
On Sat, Aug 11, 2012 at 9:58 AM, liu ping fan qemul...@gmail.com wrote:
 On Wed, Aug 8, 2012 at 5:41 PM, Avi Kivity a...@redhat.com wrote:
 On 08/08/2012 09:25 AM, Liu Ping Fan wrote:
 From: Liu Ping Fan pingf...@linux.vnet.ibm.com

 Flatview and radix view are all under the protection of pointer.
 And this make sure the change of them seem to be atomic!

 The mr accessed by radix-tree leaf or flatview will be reclaimed
 after the prev PhysMap not in use any longer


 IMO this cleverness should come much later.  Let's first take care of
 dropping the big qemu lock, then make swithcing memory maps more efficient.

 The initial paths could look like:

   lookup:
  take mem_map_lock
  lookup
  take ref
  drop mem_map_lock

   update:
  take mem_map_lock (in core_begin)
  do updates
  drop memo_map_lock

 Later we can replace mem_map_lock with either a rwlock or (real) rcu.



  #if !defined(CONFIG_USER_ONLY)

 -static void phys_map_node_reserve(unsigned nodes)
 +static void phys_map_node_reserve(PhysMap *map, unsigned nodes)
  {
 -if (phys_map_nodes_nb + nodes  phys_map_nodes_nb_alloc) {
 +if (map-phys_map_nodes_nb + nodes  map-phys_map_nodes_nb_alloc) {
  typedef PhysPageEntry Node[L2_SIZE];
 -phys_map_nodes_nb_alloc = MAX(phys_map_nodes_nb_alloc * 2, 16);
 -phys_map_nodes_nb_alloc = MAX(phys_map_nodes_nb_alloc,
 -  phys_map_nodes_nb + nodes);
 -phys_map_nodes = g_renew(Node, phys_map_nodes,
 - phys_map_nodes_nb_alloc);
 +map-phys_map_nodes_nb_alloc = MAX(map-phys_map_nodes_nb_alloc * 
 2,
 +
 16);
 +map-phys_map_nodes_nb_alloc = MAX(map-phys_map_nodes_nb_alloc,
 +  map-phys_map_nodes_nb + nodes);
 +map-phys_map_nodes = g_renew(Node, map-phys_map_nodes,
 + map-phys_map_nodes_nb_alloc);
  }
  }

 Please have a patch that just adds the map parameter to all these
 functions.  This makes the later patch, that adds the copy, easier to read.

 +
 +void cur_map_update(PhysMap *next)
 +{
 +qemu_mutex_lock(cur_map_lock);
 +physmap_put(cur_map);
 +cur_map = next;
 +smp_mb();
 +qemu_mutex_unlock(cur_map_lock);
 +}

 IMO this can be mem_map_lock.

 If we take my previous suggestion:

   lookup:
  take mem_map_lock
  lookup
  take ref
  drop mem_map_lock

   update:
  take mem_map_lock (in core_begin)
  do updates
  drop memo_map_lock

 And update it to


   update:
  prepare next_map (in core_begin)
  do updates
  take mem_map_lock (in core_commit)
  switch maps
  drop mem_map_lock
  free old map


 Note the lookup path copies the MemoryRegionSection instead of
 referencing it.  Thus we can destroy the old map without worrying; the
 only pointers will point to MemoryRegions, which will be protected by
 the refcounts on their Objects.

 Just find there may be a leak here. If mrs points to subpage, then the
 subpage_t  could be crashed by destroy.
 To avoid such situation, we can walk down the chain to pin us on the
 Object based mr, but then we must expose the address convert in
 subpage_read() right here. Right?

Oh, just read the code logic and I think walk down the chain is
enough. And subpage_read/write() is bypass, so no need for fold the
addr translation.

Regards,
pingfan

 Regards,
 pingfan

 This can be easily switched to rcu:

   update:
  prepare next_map (in core_begin)
  do updates
  switch maps - rcu_assign_pointer
  call_rcu(free old map) (or synchronize_rcu; free old maps)

 Again, this should be done after the simplictic patch that enables
 parallel lookup but keeps just one map.



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



[Qemu-devel] [Bug 1033494] Re: qemu-system-x86_64 segfaults with kernel 3.5.0

2012-08-11 Thread Michael Tokarev
** Changed in: qemu
   Status: New = Incomplete

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

Title:
  qemu-system-x86_64 segfaults with kernel 3.5.0

Status in QEMU:
  Incomplete

Bug description:
  qemu-kvm 1.1.1 stable is running fine for me with RHEL 6 2.6.32 based
  kernel.

  But with 3.5.0 kernel qemu-system-x86_64 segfaults while i'm trying to
  install ubuntu 12.04 server reproducable.

  You find three backtraces here:
  http://pastebin.com/raw.php?i=xCy2pEcP

  Stefan

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



Re: [Qemu-devel] Is the return address of get_page_addr_code guest physical address?

2012-08-11 Thread Laurent Desnogues
On Sat, Aug 11, 2012 at 5:41 AM, Steven wangwangk...@gmail.com wrote:
[...]
 I want to get the guest physical address of a pc. I note the part of
 the function cpu_x86_handle_mmu_fault will do something like page
 walking to convert a pc to its guest physical address. I think this is
 the guest physical address I need. However, there is no other function
 available to do this page walking.
 So I am thinking add a function to do the conversion.

If you want the translation of any guest virtual address to guest physical
address then cpu_x86_handle_mmu_fault is close to what you want.  Of
course you'd have to rewrite it as you probably don't want your function
to change the CPU env.

Perhaps cpu_get_phys_page_debug is even closer to what you need,
but I'm not familiar enough with x86 to say for sure.


Laurent



Re: [Qemu-devel] [qemu-devel] [PATCH V2 0/3] [RFC] libqblock draft code v2

2012-08-11 Thread Blue Swirl
On Fri, Aug 10, 2012 at 8:04 AM, Wenchao Xia xiaw...@linux.vnet.ibm.com wrote:
Thanks for your review, sorry I have forgot some fixing you
 mentioned before, will correct them this time.

 于 2012-8-10 1:12, Blue Swirl 写道:

 On Thu, Aug 9, 2012 at 10:10 AM, Wenchao Xia xiaw...@linux.vnet.ibm.com
 wrote:

This patch intrudce libqblock API, libqblock-test is used as a test
 case.

 V2:
Using struct_size and [xxx]_new [xxx]_free to keep ABI.
Using embbed structure to class format options in image creating.
Format specific options were brought to surface.
Image format was changed to enum interger instead of string.
Some API were renamed.
Internel error with errno was saved and with an API caller can get it.
ALL flags used were defined in libqblock.h.

 Something need discuss:
Embbed structure or union could make the model more friendly, but that
 make ABI more difficult, because we need to check every embbed
 structure's
 size and guess compiler's memory arrangement. This means #pragma pack(4)
 or struct_size, offset_next in every structure. Any better way to solve
 it?
 or make every structure a plain one?


 I'd still use accessor functions instead of structure passing, it
 would avoid these problems.

   Do you mean some function like :
   CreateOption_Filename_Set(const char *filename)
   CreateOption_Format_Set(const char *filename)

Something like this:
int qb_create_cow(struct QBlockState *qbs, const char *filename,
size_t virt_size, const char *backing_file, int backing_flag);
etc. for rest of the formats.

Alternatively, we could have more generic versions like you describe,
or even more generic still:

void qb_set_property(struct QBlockState *qbs, const char *prop_name,
const char *prop_value);

so the create sequence (ignoring error handling) would be:
s = qb_init();
qb_set_property(s, filename, c:autoexec.bat);
qb_set_property(s, format, cow);
qb_set_property(s, virt_size, 10GB);
//  use defaults for backing_file and backing_flag
qb_create(s);

Likewise for info structure:
char *qb_get_property(struct QBlockState *qbs, const char *prop_name);

foo = qb_get_property(s, format);
foo = qb_get_property(s, encrypted);
foo = qb_get_property(s, num_backing_files);
foo = qb_get_property(s, virt_size);

This would be helpful for the client to display parameters without
much understanding of their contents:
char **qb_list_properties(struct QBlockState *qbs); /* returns array
of property names available for this file, use get_property to
retrieve their contents */

But the clients can't be completely ignorant of all formats available,
for example a second UI dialog needs to be added for formats with
backing files, otherwise it won't be able to access some files at all.
Maybe by adding type descriptions for each property (type for
filename is path, for others string, bool, enum etc).


   It can solve the issue, with a cost of more small APIs in header
 files that user should use. Not sure if there is a good way to make
 it more friendly as an object language:
   oc.filename = name; automatically call CreateOption_Filename_Set,
 API CreateOption_Filename_Set is invisible to user.



 Packing can even introduce a new set of problems since we don't
 control the CFLAGS of the client of the library.


   indeed, I tried to handle the difference in function qboo_adjust_o2n,
 found that structure member alignment is hard to deal.


AIO is missing, need a prototype.

 Wenchao Xia (3):
adding libqblock
libqblock API
libqblock test case

   Makefile |3 +
   block.c  |2 +-
   block.h  |1 +
   libqblock-test.c |  197 
   libqblock.c  |  670
 ++
   libqblock.h  |  447 
   6 files changed, 1319 insertions(+), 1 deletions(-)
   create mode 100644 libqblock-test.c
   create mode 100644 libqblock.c
   create mode 100644 libqblock.h






 --
 Best Regards

 Wenchao Xia




Re: [Qemu-devel] [RFC 01/20] target-i386: return Error from cpu_x86_find_by_name()

2012-08-11 Thread Blue Swirl
On Fri, Aug 10, 2012 at 11:22 AM, Igor Mammedov imamm...@redhat.com wrote:
 it will allow to use property setters there later.

 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  target-i386/cpu.c | 18 +++---
  1 file changed, 15 insertions(+), 3 deletions(-)

 diff --git a/target-i386/cpu.c b/target-i386/cpu.c
 index 880cfea..ee25309 100644
 --- a/target-i386/cpu.c
 +++ b/target-i386/cpu.c
 @@ -858,7 +858,8 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor 
 *v, void *opaque,
  cpu-env.tsc_khz = value / 1000;
  }

 -static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char 
 *cpu_model)
 +static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
 +const char *cpu_model, Error **errp)
  {
  unsigned int i;
  x86_def_t *def;
 @@ -1003,6 +1004,11 @@ static int cpu_x86_find_by_name(x86_def_t 
 *x86_cpu_def, const char *cpu_model)
  fprintf(stderr, feature string `%s' not in format 
 (+feature|-feature|feature=xyz)\n, featurestr);
  goto error;
  }
 +
 +if (error_is_set(errp)) {
 +goto error;
 +}
 +
  featurestr = strtok(NULL, ,);
  }
  x86_cpu_def-features |= plus_features;
 @@ -1026,6 +1032,9 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
 const char *cpu_model)

  error:
  g_free(s);
 +if (!error_is_set(errp)) {
 +error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
 +}
  return -1;
  }

 @@ -1133,8 +1142,9 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)

  memset(def, 0, sizeof(*def));

 -if (cpu_x86_find_by_name(def, cpu_model)  0)
 -return -1;
 +if (cpu_x86_find_by_name(cpu, def, cpu_model, error)  0)

Please add braces.

 +goto out;
 +
  if (def-vendor1) {
  env-cpuid_vendor1 = def-vendor1;
  env-cpuid_vendor2 = def-vendor2;
 @@ -1173,6 +1183,8 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
  env-cpuid_svm_features = TCG_SVM_FEATURES;
  }
  object_property_set_str(OBJECT(cpu), def-model_id, model-id, error);
 +
 +out:
  if (error_is_set(error)) {
  error_free(error);
  return -1;
 --
 1.7.11.2




Re: [Qemu-devel] [PATCH] device_tree: load_device_tree(): Allow NULL sizep

2012-08-11 Thread Stefan Hajnoczi
On Sat, Aug 11, 2012 at 12:11 AM, Peter Crosthwaite
peter.crosthwa...@petalogix.com wrote:
 On Fri, Aug 10, 2012 at 11:42 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
 On Fri, Aug 10, 2012 at 01:54:26PM +1000, Peter A. G. Crosthwaite wrote:
 The sizep arg is populated with the size of the loaded device tree. Since 
 this
 is one of those informational please populate type arguments it should be
 optional. Guarded writes to *sizep against NULL accordingly.

 Signed-off-by: Peter A. G. Crosthwaite peter.crosthwa...@petalogix.com
 Acked-by: Alexander Graf ag...@suse.de
 ---
  device_tree.c |8 ++--
  1 files changed, 6 insertions(+), 2 deletions(-)

 diff --git a/device_tree.c b/device_tree.c
 index d7a9b6b..641a48a 100644
 --- a/device_tree.c
 +++ b/device_tree.c
 @@ -71,7 +71,9 @@ void *load_device_tree(const char *filename_path, int 
 *sizep)
  int ret;
  void *fdt = NULL;

 -*sizep = 0;
 +if (sizep) {
 +*sizep = 0;
 +}
  dt_size = get_image_size(filename_path);
  if (dt_size  0) {
  printf(Unable to get size of device tree file '%s'\n,
 @@ -104,7 +106,9 @@ void *load_device_tree(const char *filename_path, int 
 *sizep)
  filename_path);
  goto fail;
  }
 -*sizep = dt_size;
 +if (sizep) {
 +*sizep = dt_size;
 +}

 What can the caller do with this void* buffer without knowing its size?


 Sanity check the machine:

 dtb = load_device_tree( ... ); //dont care how big it is
 foo = fdt_gep_prop( dtb, ... );
 if (foo != object_get_prop(foo_device, foo_prop, ... )) {
 hw_error(your dtb is bad because ... !\n, ... );
 }

What happens if the fdt is corrupt or malicious?  I guess we'll access
memory beyond the end of blob.

This seems to be libfdt's fault.  I didn't see an API to validate the
blob's size.

I'm happy with this patch but if fdt's can ever come from untrusted
sources then we're in trouble.

Stefan



Re: [Qemu-devel] virtio-scsi - vhost multi lun/adapter performance results with 3.6-rc0

2012-08-11 Thread Stefan Hajnoczi
On Sat, Aug 11, 2012 at 12:23 AM, Nicholas A. Bellinger
n...@linux-iscsi.org wrote:
 Using a KVM guest with 32x vCPUs and 4G memory, the results for 4x
 random I/O now look like:

 workload | jobs | 25% write / 75% read | 75% write / 25% read
 -|--|--|-
 1x rd_mcp LUN|   8  | ~155K IOPs   |  ~145K IOPs
 16x rd_mcp LUNs  |  16  | ~315K IOPs   |  ~305K IOPs
 32x rd_mcp LUNs  |  16  | ~425K IOPs   |  ~410K IOPs

 The full fio randrw results for the six test cases are attached below.
 Also, using a workload of fio numjobs  16 currently makes performance
 start to fall off pretty sharply regardless of the number of vCPUs..

 So running a similar workload with loopback SCSI ports on bare-metal
 produces ~1M random IOPs with 12x LUNs + numjobs=32.  At numjobs=16 here
 with vhost the 16x LUN configuration ends up being in the range of ~310K
 IOPs for the current sweet spot..

This makes me wonder what a comparison against baremetal looks like
and the perf top, mpstat, and kvm_stat results on the host.

Stefan



[Qemu-devel] github mirror still stale

2012-08-11 Thread Peter Maydell
On 18 July 2012 10:28, Peter Maydell peter.mayd...@linaro.org wrote:
 On 12 March 2012 20:12, Stefan Weil s...@weilnetz.de wrote:
 We also need more resources for technical maintenance of the
 QEMU infrastructure. For example, the official mirror of the
 QEMU git repository (https://github.com/qemu/QEMU) is several
 months behind, http://git.savannah.gnu.org/cgit/qemu.git is
 even older.

 The github mirror is still wildly out of date -- can we
 get the link to it removed from http://wiki.qemu.org/Download
 please? (I'd do it myself but the page is 'locked'.)

Ping^2. It would be good to stop advertising stale git mirrors
before we make the 1.2 release...

-- PMM



[Qemu-devel] [PATCH v9 0/7] file descriptor passing using fd sets

2012-08-11 Thread Corey Bryant
libvirt's sVirt security driver provides SELinux MAC isolation for
Qemu guest processes and their corresponding image files.  In other
words, sVirt uses SELinux to prevent a QEMU process from opening
files that do not belong to it.

sVirt provides this support by labeling guests and resources with
security labels that are stored in file system extended attributes.
Some file systems, such as NFS, do not support the extended
attribute security namespace, and therefore cannot support sVirt
isolation.

A solution to this problem is to provide fd passing support, where
libvirt opens files and passes file descriptors to QEMU.  This,
along with SELinux policy to prevent QEMU from opening files, can
provide image file isolation for NFS files stored on the same NFS
mount.

This patch series adds the add-fd, remove-fd, and query-fdsets
QMP monitor commands, which allow file descriptors to be passed
via SCM_RIGHTS, and assigned to specified fd sets.  This allows
fd sets to be created per file with fds having, for example,
different access rights.  When QEMU needs to reopen a file with
different access rights, it can search for a matching fd in the
fd set.  Fd sets also allow for easy tracking of fds per file,
helping to prevent fd leaks.

Support is also added to the block layer to allow QEMU to dup an
fd from an fdset when the filename is of the /dev/fdset/nnn format,
where nnn is the fd set ID.

No new SELinux policy is required to prevent open of NFS files
(files with type nfs_t).  The virt_use_nfs boolean type simply
needs to be set to false, and open will be prevented (and dup will
be allowed).  For example:

# setsebool virt_use_nfs 0
# getsebool virt_use_nfs
virt_use_nfs -- off

Corey Bryant (7):
  qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg
  qapi: Introduce add-fd, remove-fd, query-fdsets
  block: Prevent detection of /dev/fdset/ as floppy
  block: Convert open calls to qemu_open
  block: Convert close calls to qemu_close
  block: Enable qemu_open/close to work with fd sets
  monitor: Clean up fd sets on monitor disconnect

 block/raw-posix.c |   46 +
 block/raw-win32.c |6 +-
 block/vdi.c   |5 +-
 block/vmdk.c  |   25 ++---
 block/vpc.c   |4 +-
 block/vvfat.c |   16 +--
 cutils.c  |5 +
 monitor.c |  287 +
 monitor.h |5 +
 osdep.c   |  113 +
 qapi-schema.json  |   98 ++
 qemu-char.c   |   12 ++-
 qemu-common.h |2 +
 qemu-tool.c   |   20 
 qmp-commands.hx   |  122 +++
 savevm.c  |4 +-
 16 files changed, 715 insertions(+), 55 deletions(-)

-- 
1.7.10.4




[Qemu-devel] [PATCH v9 3/7] block: Prevent detection of /dev/fdset/ as floppy

2012-08-11 Thread Corey Bryant
Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
---
v8
 -This patch is new in v8. It was reported on a prior fd passing
  approach and I realized it's needed in this series.
  (kw...@redhat.com)

v9
 -No changes

 block/raw-posix.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 0dce089..f606211 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1052,8 +1052,10 @@ static int floppy_probe_device(const char *filename)
 struct floppy_struct fdparam;
 struct stat st;
 
-if (strstart(filename, /dev/fd, NULL))
+if (strstart(filename, /dev/fd, NULL) 
+!strstart(filename, /dev/fdset/, NULL)) {
 prio = 50;
+}
 
 fd = open(filename, O_RDONLY | O_NONBLOCK);
 if (fd  0) {
-- 
1.7.10.4




[Qemu-devel] [PATCH v9 6/7] block: Enable qemu_open/close to work with fd sets

2012-08-11 Thread Corey Bryant
When qemu_open is passed a filename of the /dev/fdset/nnn
format (where nnn is the fdset ID), an fd with matching access
mode flags will be searched for within the specified monitor
fd set.  If the fd is found, a dup of the fd will be returned
from qemu_open.

Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
---
v2:
 -Get rid of file_open and move dup code to qemu_open
  (kw...@redhat.com)
 -Use strtol wrapper instead of atoi (kw...@redhat.com)

v3:
 -Add note about fd leakage (ebl...@redhat.com)

v4
 -Moved patch to be later in series (lcapitul...@redhat.com)
 -Update qemu_open to check access mode flags and set flags that
  can be set (ebl...@redhat.com, kw...@redhat.com)

v5:
 -This patch was overhauled quite a bit in this version, with
  the addition of fd set and refcount support.
 -Use qemu_set_cloexec() on dup'd fd (ebl...@redhat.com)
 -Modify flags set by fcntl on dup'd fd (ebl...@redhat.com)
 -Reduce syscalls when setting flags for dup'd fd (ebl...@redhat.com)
 -Fix O_RDWR, O_RDONLY, O_WRONLY checks (ebl...@redhat.com)

v6:
 -Pass only the fd to qemu_close() and keep track of dup fds per fd
  set. (kw...@redhat.com, ebl...@redhat.com)
 -Handle refcount incr/decr in new dup_fd_add/remove fd functions.
 -Use qemu_set_cloexec() appropriately in qemu_dup() (kw...@redhat.com)
 -Simplify setting of setfl_flags in qemu_dup() (kw...@redhat.com)
 -Add preprocessor checks for F_DUPFD_CLOEXEC (ebl...@redhat.com)
 -Simplify flag checking in monitor_fdset_get_fd() (kw...@redhat.com)

v7:
 -Minor updates to reference global mon_fdsets, and to remove
  default_mon usage in osdep.c. (kw...@redhat.com)

v8:
 -Use camel case for structures. (stefa...@linux.vnet.ibm.com)

v9:
 -Drop fdset refcount and check dup_fds instead. (ebl...@redhat.com)
 -Fix dupfd leak in qemu_dup(). (ebl...@redhat.com)
 -Always set O_CLOEXEC in qemu_dup(). (kw...@redhat.com)
 -Change name of qemu_dup() to qemu_dup_flags(). (kw...@redhat.com)

 cutils.c  |5 +++
 monitor.c |   83 +++-
 monitor.h |5 +++
 osdep.c   |  108 +
 qemu-common.h |1 +
 qemu-tool.c   |   20 +++
 6 files changed, 221 insertions(+), 1 deletion(-)

diff --git a/cutils.c b/cutils.c
index 9d4c570..8b0d2bb 100644
--- a/cutils.c
+++ b/cutils.c
@@ -382,3 +382,8 @@ int qemu_parse_fd(const char *param)
 }
 return fd;
 }
+
+int qemu_parse_fdset(const char *param)
+{
+return qemu_parse_fd(param);
+}
diff --git a/monitor.c b/monitor.c
index c1851f0..4f6d2ce 100644
--- a/monitor.c
+++ b/monitor.c
@@ -154,6 +154,7 @@ typedef struct MonFdset MonFdset;
 struct MonFdset {
 int64_t id;
 QLIST_HEAD(, MonFdsetFd) fds;
+QLIST_HEAD(, MonFdsetFd) dup_fds;
 QLIST_ENTRY(MonFdset) next;
 };
 
@@ -2421,7 +2422,7 @@ static void monitor_fdset_cleanup(MonFdset *mon_fdset)
 }
 }
 
-if (QLIST_EMPTY(mon_fdset-fds)) {
+if (QLIST_EMPTY(mon_fdset-fds)  QLIST_EMPTY(mon_fdset-dup_fds)) {
 QLIST_REMOVE(mon_fdset, next);
 g_free(mon_fdset);
 }
@@ -2574,6 +2575,86 @@ FdsetInfoList *qmp_query_fdsets(Error **errp)
 return fdset_list;
 }
 
+int monitor_fdset_get_fd(int64_t fdset_id, int flags)
+{
+MonFdset *mon_fdset;
+MonFdsetFd *mon_fdset_fd;
+int mon_fd_flags;
+
+QLIST_FOREACH(mon_fdset, mon_fdsets, next) {
+if (mon_fdset-id != fdset_id) {
+continue;
+}
+QLIST_FOREACH(mon_fdset_fd, mon_fdset-fds, next) {
+mon_fd_flags = fcntl(mon_fdset_fd-fd, F_GETFL);
+if (mon_fd_flags == -1) {
+return -1;
+}
+
+if ((flags  O_ACCMODE) == (mon_fd_flags  O_ACCMODE)) {
+return mon_fdset_fd-fd;
+}
+}
+errno = EACCES;
+return -1;
+}
+errno = ENOENT;
+return -1;
+}
+
+int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd)
+{
+MonFdset *mon_fdset;
+MonFdsetFd *mon_fdset_fd_dup;
+
+QLIST_FOREACH(mon_fdset, mon_fdsets, next) {
+if (mon_fdset-id != fdset_id) {
+continue;
+}
+QLIST_FOREACH(mon_fdset_fd_dup, mon_fdset-dup_fds, next) {
+if (mon_fdset_fd_dup-fd == dup_fd) {
+return -1;
+}
+}
+mon_fdset_fd_dup = g_malloc0(sizeof(*mon_fdset_fd_dup));
+mon_fdset_fd_dup-fd = dup_fd;
+QLIST_INSERT_HEAD(mon_fdset-dup_fds, mon_fdset_fd_dup, next);
+return 0;
+}
+return -1;
+}
+
+static int _monitor_fdset_dup_fd_find(int dup_fd, bool remove)
+{
+MonFdset *mon_fdset;
+MonFdsetFd *mon_fdset_fd_dup;
+
+QLIST_FOREACH(mon_fdset, mon_fdsets, next) {
+QLIST_FOREACH(mon_fdset_fd_dup, mon_fdset-dup_fds, next) {
+if (mon_fdset_fd_dup-fd == dup_fd) {
+if (remove) {
+QLIST_REMOVE(mon_fdset_fd_dup, next);
+if (QLIST_EMPTY(mon_fdset-dup_fds)) {
+   

[Qemu-devel] [PATCH v9 5/7] block: Convert close calls to qemu_close

2012-08-11 Thread Corey Bryant
This patch converts all block layer close calls, that correspond
to qemu_open calls, to qemu_close.

Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
---
v5:
 -This patch is new in v5. (kw...@redhat.com, ebl...@redhat.com)

v6-v9:
 -No changes

 block/raw-posix.c |   24 
 block/raw-win32.c |2 +-
 block/vmdk.c  |4 ++--
 block/vpc.c   |2 +-
 block/vvfat.c |   12 ++--
 osdep.c   |5 +
 qemu-common.h |1 +
 savevm.c  |4 ++--
 8 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 08b997e..6be20b1 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -271,7 +271,7 @@ static int raw_open_common(BlockDriverState *bs, const char 
*filename,
 out_free_buf:
 qemu_vfree(s-aligned_buf);
 out_close:
-close(fd);
+qemu_close(fd);
 return -errno;
 }
 
@@ -376,7 +376,7 @@ static void raw_close(BlockDriverState *bs)
 {
 BDRVRawState *s = bs-opaque;
 if (s-fd = 0) {
-close(s-fd);
+qemu_close(s-fd);
 s-fd = -1;
 if (s-aligned_buf != NULL)
 qemu_vfree(s-aligned_buf);
@@ -580,7 +580,7 @@ static int raw_create(const char *filename, 
QEMUOptionParameter *options)
 if (ftruncate(fd, total_size * BDRV_SECTOR_SIZE) != 0) {
 result = -errno;
 }
-if (close(fd) != 0) {
+if (qemu_close(fd) != 0) {
 result = -errno;
 }
 }
@@ -850,7 +850,7 @@ static int hdev_open(BlockDriverState *bs, const char 
*filename, int flags)
 if (fd  0) {
 bsdPath[strlen(bsdPath)-1] = '1';
 } else {
-close(fd);
+qemu_close(fd);
 }
 filename = bsdPath;
 }
@@ -889,7 +889,7 @@ static int fd_open(BlockDriverState *bs)
 last_media_present = (s-fd = 0);
 if (s-fd = 0 
 (get_clock() - s-fd_open_time) = FD_OPEN_TIMEOUT) {
-close(s-fd);
+qemu_close(s-fd);
 s-fd = -1;
 #ifdef DEBUG_FLOPPY
 printf(Floppy closed\n);
@@ -988,7 +988,7 @@ static int hdev_create(const char *filename, 
QEMUOptionParameter *options)
 else if (lseek(fd, 0, SEEK_END)  total_size * BDRV_SECTOR_SIZE)
 ret = -ENOSPC;
 
-close(fd);
+qemu_close(fd);
 return ret;
 }
 
@@ -1038,7 +1038,7 @@ static int floppy_open(BlockDriverState *bs, const char 
*filename, int flags)
 return ret;
 
 /* close fd so that we can reopen it as needed */
-close(s-fd);
+qemu_close(s-fd);
 s-fd = -1;
 s-fd_media_changed = 1;
 
@@ -1072,7 +1072,7 @@ static int floppy_probe_device(const char *filename)
 prio = 100;
 
 outc:
-close(fd);
+qemu_close(fd);
 out:
 return prio;
 }
@@ -1107,14 +1107,14 @@ static void floppy_eject(BlockDriverState *bs, bool 
eject_flag)
 int fd;
 
 if (s-fd = 0) {
-close(s-fd);
+qemu_close(s-fd);
 s-fd = -1;
 }
 fd = qemu_open(bs-filename, s-open_flags | O_NONBLOCK);
 if (fd = 0) {
 if (ioctl(fd, FDEJECT, 0)  0)
 perror(FDEJECT);
-close(fd);
+qemu_close(fd);
 }
 }
 
@@ -1175,7 +1175,7 @@ static int cdrom_probe_device(const char *filename)
 prio = 100;
 
 outc:
-close(fd);
+qemu_close(fd);
 out:
 return prio;
 }
@@ -1283,7 +1283,7 @@ static int cdrom_reopen(BlockDriverState *bs)
  * FreeBSD seems to not notice sometimes...
  */
 if (s-fd = 0)
-close(s-fd);
+qemu_close(s-fd);
 fd = qemu_open(bs-filename, s-open_flags, 0644);
 if (fd  0) {
 s-fd = -1;
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 8d7838d..c56bf83 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -261,7 +261,7 @@ static int raw_create(const char *filename, 
QEMUOptionParameter *options)
 return -EIO;
 set_sparse(fd);
 ftruncate(fd, total_size * 512);
-close(fd);
+qemu_close(fd);
 return 0;
 }
 
diff --git a/block/vmdk.c b/block/vmdk.c
index 557dc1b..daee426 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1258,7 +1258,7 @@ static int vmdk_create_extent(const char *filename, 
int64_t filesize,
 
 ret = 0;
  exit:
-close(fd);
+qemu_close(fd);
 return ret;
 }
 
@@ -1506,7 +1506,7 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options)
 }
 ret = 0;
 exit:
-close(fd);
+qemu_close(fd);
 return ret;
 }
 
diff --git a/block/vpc.c b/block/vpc.c
index 60ebf5a..c0b82c4 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -744,7 +744,7 @@ static int vpc_create(const char *filename, 
QEMUOptionParameter *options)
 }
 
  fail:
-close(fd);
+qemu_close(fd);
 return ret;
 }
 
diff --git a/block/vvfat.c b/block/vvfat.c
index 22b586a..59d3c5b 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1105,7 +1105,7 @@ static inline void 
vvfat_close_current_file(BDRVVVFATState *s)
 if(s-current_mapping) 

[Qemu-devel] [PATCH v9 4/7] block: Convert open calls to qemu_open

2012-08-11 Thread Corey Bryant
This patch converts all block layer open calls to qemu_open.

Note that this adds the O_CLOEXEC flag to the changed open paths
when the O_CLOEXEC macro is defined.

Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
---
v2:
 -Convert calls to qemu_open instead of file_open (kw...@redhat.com)
 -Mention introduction of O_CLOEXEC (kw...@redhat.com)

v3-v9:
 -No changes

 block/raw-posix.c |   18 +-
 block/raw-win32.c |4 ++--
 block/vdi.c   |5 +++--
 block/vmdk.c  |   21 +
 block/vpc.c   |2 +-
 block/vvfat.c |4 ++--
 6 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index f606211..08b997e 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -572,8 +572,8 @@ static int raw_create(const char *filename, 
QEMUOptionParameter *options)
 options++;
 }
 
-fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
-  0644);
+fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
+   0644);
 if (fd  0) {
 result = -errno;
 } else {
@@ -846,7 +846,7 @@ static int hdev_open(BlockDriverState *bs, const char 
*filename, int flags)
 if ( bsdPath[ 0 ] != '\0' ) {
 strcat(bsdPath,s0);
 /* some CDs don't have a partition 0 */
-fd = open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE);
+fd = qemu_open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE);
 if (fd  0) {
 bsdPath[strlen(bsdPath)-1] = '1';
 } else {
@@ -903,7 +903,7 @@ static int fd_open(BlockDriverState *bs)
 #endif
 return -EIO;
 }
-s-fd = open(bs-filename, s-open_flags  ~O_NONBLOCK);
+s-fd = qemu_open(bs-filename, s-open_flags  ~O_NONBLOCK);
 if (s-fd  0) {
 s-fd_error_time = get_clock();
 s-fd_got_error = 1;
@@ -977,7 +977,7 @@ static int hdev_create(const char *filename, 
QEMUOptionParameter *options)
 options++;
 }
 
-fd = open(filename, O_WRONLY | O_BINARY);
+fd = qemu_open(filename, O_WRONLY | O_BINARY);
 if (fd  0)
 return -errno;
 
@@ -1057,7 +1057,7 @@ static int floppy_probe_device(const char *filename)
 prio = 50;
 }
 
-fd = open(filename, O_RDONLY | O_NONBLOCK);
+fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
 if (fd  0) {
 goto out;
 }
@@ -1110,7 +1110,7 @@ static void floppy_eject(BlockDriverState *bs, bool 
eject_flag)
 close(s-fd);
 s-fd = -1;
 }
-fd = open(bs-filename, s-open_flags | O_NONBLOCK);
+fd = qemu_open(bs-filename, s-open_flags | O_NONBLOCK);
 if (fd = 0) {
 if (ioctl(fd, FDEJECT, 0)  0)
 perror(FDEJECT);
@@ -1160,7 +1160,7 @@ static int cdrom_probe_device(const char *filename)
 int prio = 0;
 struct stat st;
 
-fd = open(filename, O_RDONLY | O_NONBLOCK);
+fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
 if (fd  0) {
 goto out;
 }
@@ -1284,7 +1284,7 @@ static int cdrom_reopen(BlockDriverState *bs)
  */
 if (s-fd = 0)
 close(s-fd);
-fd = open(bs-filename, s-open_flags, 0644);
+fd = qemu_open(bs-filename, s-open_flags, 0644);
 if (fd  0) {
 s-fd = -1;
 return -EIO;
diff --git a/block/raw-win32.c b/block/raw-win32.c
index e4b0b75..8d7838d 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -255,8 +255,8 @@ static int raw_create(const char *filename, 
QEMUOptionParameter *options)
 options++;
 }
 
-fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
-  0644);
+fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
+   0644);
 if (fd  0)
 return -EIO;
 set_sparse(fd);
diff --git a/block/vdi.c b/block/vdi.c
index 57325d6..c4f1529 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -653,8 +653,9 @@ static int vdi_create(const char *filename, 
QEMUOptionParameter *options)
 options++;
 }
 
-fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
-  0644);
+fd = qemu_open(filename,
+   O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+   0644);
 if (fd  0) {
 return -errno;
 }
diff --git a/block/vmdk.c b/block/vmdk.c
index 18e9b4c..557dc1b 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1161,10 +1161,9 @@ static int vmdk_create_extent(const char *filename, 
int64_t filesize,
 VMDK4Header header;
 uint32_t tmp, magic, grains, gd_size, gt_size, gt_count;
 
-fd = open(
-filename,
-O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
-0644);
+fd = qemu_open(filename,
+   O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+   0644);
 if (fd  0) {
 return -errno;
 }
@@ -1484,15 +1483,13 @@ static int vmdk_create(const char 

[Qemu-devel] [PATCH v9 7/7] monitor: Clean up fd sets on monitor disconnect

2012-08-11 Thread Corey Bryant
Fd sets are shared by all monitor connections.  Fd sets are considered
to be in use while at least one monitor is connected.  When the last
monitor disconnects, all fds that are members of an fd set with no
outstanding dup references are closed.  This prevents any fd leakage
associated with a client disconnect prior to using a passed fd.

Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
---
v5:
 -This patch is new in v5.
 -This support addresses concerns from v4 regarding fd leakage
  if the client disconnects unexpectedly. (ebl...@redhat.com,
  kw...@redhat.com, dberra...@redhat.com)

v6:
 -No changes

v7:
 -Removed monitor_fdsets_set_in_use() function since we now use
  mon_refcount to determine if fdsets are in use.
 -Added monitor_fdsets_cleanup() function, and increment/decrement
  of mon_refcount when monitor connects/disconnects.

v8:
 -Use camel case for structures. (stefa...@linux.vnet.ibm.com)

v9:
 -Define mon_refcount and add corresponding logic to
  monitor_fdset_cleanup here rather than earlier patch.
  (kw...@redhat.com)

 monitor.c |   23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/monitor.c b/monitor.c
index 4f6d2ce..fc7ac9b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -230,6 +230,7 @@ static inline int mon_print_count_get(const Monitor *mon) { 
return 0; }
 
 static QLIST_HEAD(mon_list, Monitor) mon_list;
 static QLIST_HEAD(mon_fdsets, MonFdset) mon_fdsets;
+static int mon_refcount;
 
 static mon_cmd_t mon_cmds[];
 static mon_cmd_t info_cmds[];
@@ -2414,7 +2415,8 @@ static void monitor_fdset_cleanup(MonFdset *mon_fdset)
 MonFdsetFd *mon_fdset_fd_next;
 
 QLIST_FOREACH_SAFE(mon_fdset_fd, mon_fdset-fds, next, mon_fdset_fd_next) 
{
-if (mon_fdset_fd-removed) {
+if (mon_fdset_fd-removed ||
+(QLIST_EMPTY(mon_fdset-dup_fds)  mon_refcount == 0)) {
 close(mon_fdset_fd-fd);
 g_free(mon_fdset_fd-opaque);
 QLIST_REMOVE(mon_fdset_fd, next);
@@ -2428,6 +2430,16 @@ static void monitor_fdset_cleanup(MonFdset *mon_fdset)
 }
 }
 
+static void monitor_fdsets_cleanup(void)
+{
+MonFdset *mon_fdset;
+MonFdset *mon_fdset_next;
+
+QLIST_FOREACH_SAFE(mon_fdset, mon_fdsets, next, mon_fdset_next) {
+monitor_fdset_cleanup(mon_fdset);
+}
+}
+
 AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque,
   const char *opaque, Error **errp)
 {
@@ -4859,9 +4871,12 @@ static void monitor_control_event(void *opaque, int 
event)
 data = get_qmp_greeting();
 monitor_json_emitter(mon, data);
 qobject_decref(data);
+mon_refcount++;
 break;
 case CHR_EVENT_CLOSED:
 json_message_parser_destroy(mon-mc-parser);
+mon_refcount--;
+monitor_fdsets_cleanup();
 break;
 }
 }
@@ -4902,6 +4917,12 @@ static void monitor_event(void *opaque, int event)
 readline_show_prompt(mon-rs);
 }
 mon-reset_seen = 1;
+mon_refcount++;
+break;
+
+case CHR_EVENT_CLOSED:
+mon_refcount--;
+monitor_fdsets_cleanup();
 break;
 }
 }
-- 
1.7.10.4




[Qemu-devel] [PATCH v9 1/7] qemu-char: Add MSG_CMSG_CLOEXEC flag to recvmsg

2012-08-11 Thread Corey Bryant
Set the close-on-exec flag for the file descriptor received
via SCM_RIGHTS.

Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
---
v4
 -This patch is new in v4 (ebl...@redhat.com)

v5
 -Fallback to FD_CLOEXEC if MSG_CMSG_CLOEXEC is not available
  (ebl...@redhat.com, stefa...@linux.vnet.ibm.com)

v6
 -Set cloexec on correct fd (ebl...@redhat.com)

v7-v9
 -No changes

 qemu-char.c |   12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index c2aaaee..ab4a928 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2238,6 +2238,9 @@ static void unix_process_msgfd(CharDriverState *chr, 
struct msghdr *msg)
 if (fd  0)
 continue;
 
+#ifndef MSG_CMSG_CLOEXEC
+qemu_set_cloexec(fd);
+#endif
 if (s-msgfd != -1)
 close(s-msgfd);
 s-msgfd = fd;
@@ -2253,6 +2256,7 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char 
*buf, size_t len)
 struct cmsghdr cmsg;
 char control[CMSG_SPACE(sizeof(int))];
 } msg_control;
+int flags = 0;
 ssize_t ret;
 
 iov[0].iov_base = buf;
@@ -2263,9 +2267,13 @@ static ssize_t tcp_chr_recv(CharDriverState *chr, char 
*buf, size_t len)
 msg.msg_control = msg_control;
 msg.msg_controllen = sizeof(msg_control);
 
-ret = recvmsg(s-fd, msg, 0);
-if (ret  0  s-is_unix)
+#ifdef MSG_CMSG_CLOEXEC
+flags |= MSG_CMSG_CLOEXEC;
+#endif
+ret = recvmsg(s-fd, msg, flags);
+if (ret  0  s-is_unix) {
 unix_process_msgfd(chr, msg);
+}
 
 return ret;
 }
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH v9 5/7] block: Convert close calls to qemu_close

2012-08-11 Thread Blue Swirl
On Sat, Aug 11, 2012 at 1:14 PM, Corey Bryant cor...@linux.vnet.ibm.com wrote:
 This patch converts all block layer close calls, that correspond
 to qemu_open calls, to qemu_close.

 Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
 ---
 v5:
  -This patch is new in v5. (kw...@redhat.com, ebl...@redhat.com)

 v6-v9:
  -No changes

  block/raw-posix.c |   24 
  block/raw-win32.c |2 +-
  block/vmdk.c  |4 ++--
  block/vpc.c   |2 +-
  block/vvfat.c |   12 ++--
  osdep.c   |5 +
  qemu-common.h |1 +
  savevm.c  |4 ++--
  8 files changed, 30 insertions(+), 24 deletions(-)

 diff --git a/block/raw-posix.c b/block/raw-posix.c
 index 08b997e..6be20b1 100644
 --- a/block/raw-posix.c
 +++ b/block/raw-posix.c
 @@ -271,7 +271,7 @@ static int raw_open_common(BlockDriverState *bs, const 
 char *filename,
  out_free_buf:
  qemu_vfree(s-aligned_buf);
  out_close:
 -close(fd);
 +qemu_close(fd);
  return -errno;
  }

 @@ -376,7 +376,7 @@ static void raw_close(BlockDriverState *bs)
  {
  BDRVRawState *s = bs-opaque;
  if (s-fd = 0) {
 -close(s-fd);
 +qemu_close(s-fd);
  s-fd = -1;
  if (s-aligned_buf != NULL)
  qemu_vfree(s-aligned_buf);
 @@ -580,7 +580,7 @@ static int raw_create(const char *filename, 
 QEMUOptionParameter *options)
  if (ftruncate(fd, total_size * BDRV_SECTOR_SIZE) != 0) {
  result = -errno;
  }
 -if (close(fd) != 0) {
 +if (qemu_close(fd) != 0) {
  result = -errno;
  }
  }
 @@ -850,7 +850,7 @@ static int hdev_open(BlockDriverState *bs, const char 
 *filename, int flags)
  if (fd  0) {
  bsdPath[strlen(bsdPath)-1] = '1';
  } else {
 -close(fd);
 +qemu_close(fd);
  }
  filename = bsdPath;
  }
 @@ -889,7 +889,7 @@ static int fd_open(BlockDriverState *bs)
  last_media_present = (s-fd = 0);
  if (s-fd = 0 
  (get_clock() - s-fd_open_time) = FD_OPEN_TIMEOUT) {
 -close(s-fd);
 +qemu_close(s-fd);
  s-fd = -1;
  #ifdef DEBUG_FLOPPY
  printf(Floppy closed\n);
 @@ -988,7 +988,7 @@ static int hdev_create(const char *filename, 
 QEMUOptionParameter *options)
  else if (lseek(fd, 0, SEEK_END)  total_size * BDRV_SECTOR_SIZE)
  ret = -ENOSPC;

 -close(fd);
 +qemu_close(fd);
  return ret;
  }

 @@ -1038,7 +1038,7 @@ static int floppy_open(BlockDriverState *bs, const char 
 *filename, int flags)
  return ret;

  /* close fd so that we can reopen it as needed */
 -close(s-fd);
 +qemu_close(s-fd);
  s-fd = -1;
  s-fd_media_changed = 1;

 @@ -1072,7 +1072,7 @@ static int floppy_probe_device(const char *filename)
  prio = 100;

  outc:
 -close(fd);
 +qemu_close(fd);
  out:
  return prio;
  }
 @@ -1107,14 +1107,14 @@ static void floppy_eject(BlockDriverState *bs, bool 
 eject_flag)
  int fd;

  if (s-fd = 0) {
 -close(s-fd);
 +qemu_close(s-fd);
  s-fd = -1;
  }
  fd = qemu_open(bs-filename, s-open_flags | O_NONBLOCK);
  if (fd = 0) {
  if (ioctl(fd, FDEJECT, 0)  0)
  perror(FDEJECT);
 -close(fd);
 +qemu_close(fd);
  }
  }

 @@ -1175,7 +1175,7 @@ static int cdrom_probe_device(const char *filename)
  prio = 100;

  outc:
 -close(fd);
 +qemu_close(fd);
  out:
  return prio;
  }
 @@ -1283,7 +1283,7 @@ static int cdrom_reopen(BlockDriverState *bs)
   * FreeBSD seems to not notice sometimes...
   */
  if (s-fd = 0)
 -close(s-fd);
 +qemu_close(s-fd);
  fd = qemu_open(bs-filename, s-open_flags, 0644);
  if (fd  0) {
  s-fd = -1;
 diff --git a/block/raw-win32.c b/block/raw-win32.c
 index 8d7838d..c56bf83 100644
 --- a/block/raw-win32.c
 +++ b/block/raw-win32.c
 @@ -261,7 +261,7 @@ static int raw_create(const char *filename, 
 QEMUOptionParameter *options)
  return -EIO;
  set_sparse(fd);
  ftruncate(fd, total_size * 512);
 -close(fd);
 +qemu_close(fd);
  return 0;
  }

 diff --git a/block/vmdk.c b/block/vmdk.c
 index 557dc1b..daee426 100644
 --- a/block/vmdk.c
 +++ b/block/vmdk.c
 @@ -1258,7 +1258,7 @@ static int vmdk_create_extent(const char *filename, 
 int64_t filesize,

  ret = 0;
   exit:
 -close(fd);
 +qemu_close(fd);
  return ret;
  }

 @@ -1506,7 +1506,7 @@ static int vmdk_create(const char *filename, 
 QEMUOptionParameter *options)
  }
  ret = 0;
  exit:
 -close(fd);
 +qemu_close(fd);
  return ret;
  }

 diff --git a/block/vpc.c b/block/vpc.c
 index 60ebf5a..c0b82c4 100644
 --- a/block/vpc.c
 +++ b/block/vpc.c
 @@ -744,7 +744,7 @@ static int vpc_create(const char *filename, 
 QEMUOptionParameter *options)
  }

   fail:
 -close(fd);
 +qemu_close(fd);
  return 

[Qemu-devel] [PATCH v9 2/7] qapi: Introduce add-fd, remove-fd, query-fdsets

2012-08-11 Thread Corey Bryant
This patch adds support that enables passing of file descriptors
to the QEMU monitor where they will be stored in specified file
descriptor sets.

A file descriptor set can be used by a client like libvirt to
store file descriptors for the same file.  This allows the
client to open a file with different access modes (O_RDWR,
O_WRONLY, O_RDONLY) and add/remove the passed fds to/from an fd
set as needed.  This will allow QEMU to (in a later patch in this
series) open and reopen the same file by dup()ing the fd in
the fd set that corresponds to the file, where the fd has the
matching access mode flag that QEMU requests.

The new QMP commands are:
  add-fd: Add a file descriptor to an fd set
  remove-fd: Remove a file descriptor from an fd set
  query-fdsets: Return information describing all fd sets

Note: These commands are not compatible with the existing getfd
and closefd QMP commands.

Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
---
v5:
 -This patch is new in v5 and replaces the pass-fd QMP command
  from v4.
 -By grouping fds in fd sets, we ease managability with an fd
  set per file, addressing concerns raised in v4 about handling
  reopens and preventing fd leakage. (ebl...@redhat.com,
  kw...@redhat.com, dberra...@redhat.com)

v6
 -Make @fd optional for remove-fd (ebl...@redhat.com)
 -Make @fdset-id optional for add-fd (ebl...@redhat.com)

v7:
 -Share fd sets among all monitor connections (kw...@redhat.com)
 -Added mon_refcount to keep track of monitor connection count.

v8:
 -Add opaque string to add-fd/query-fdsets.
  (stefa...@linux.vnet.ibm.com)
 -Use camel case for structures. (stefa...@linux.vnet.ibm.com)
 -Don't return in-use and refcount from query-fdsets.
  (stefa...@linux.vnet.ibm.com)
 -Don't return removed fd's from query-fdsets.
  (stefa...@linux.vnet.ibm.com)
 -Use fdset-id rather than fdset_id. (ebl...@redhat.com)
 -Fix fd leak in qmp_add_fd(). (stefa...@linux.vnet.ibm.com)
 -Update QMP errors. (stefa...@linux.vnet.ibm.com, ebl...@redhat.com)

v9:
 -Use fdset-id rather than fdset_id. (ebl...@redhat.com)
 -Update example for query-fdsets. (ebl...@redhat.com)
 -Close fd immediately on remove-fd.
  (kw...@redhat.com, ebl...@redhat.com)
 -Drop fdset refcount, and check dup_fds instead (in a later patch).
  (ebl...@redhat.com)
 -Move mon_refcount code to a later patch. (kw...@redhat.com)

 monitor.c|  185 ++
 qapi-schema.json |   98 +
 qmp-commands.hx  |  122 +++
 3 files changed, 405 insertions(+)

diff --git a/monitor.c b/monitor.c
index 49dccfe..c1851f0 100644
--- a/monitor.c
+++ b/monitor.c
@@ -140,6 +140,23 @@ struct mon_fd_t {
 QLIST_ENTRY(mon_fd_t) next;
 };
 
+/* file descriptor associated with a file descriptor set */
+typedef struct MonFdsetFd MonFdsetFd;
+struct MonFdsetFd {
+int fd;
+bool removed;
+char *opaque;
+QLIST_ENTRY(MonFdsetFd) next;
+};
+
+/* file descriptor set containing fds passed via SCM_RIGHTS */
+typedef struct MonFdset MonFdset;
+struct MonFdset {
+int64_t id;
+QLIST_HEAD(, MonFdsetFd) fds;
+QLIST_ENTRY(MonFdset) next;
+};
+
 typedef struct MonitorControl {
 QObject *id;
 JSONMessageParser parser;
@@ -211,6 +228,7 @@ static inline int mon_print_count_get(const Monitor *mon) { 
return 0; }
 #define QMP_ACCEPT_UNKNOWNS 1
 
 static QLIST_HEAD(mon_list, Monitor) mon_list;
+static QLIST_HEAD(mon_fdsets, MonFdset) mon_fdsets;
 
 static mon_cmd_t mon_cmds[];
 static mon_cmd_t info_cmds[];
@@ -2389,6 +2407,173 @@ int monitor_get_fd(Monitor *mon, const char *fdname)
 return -1;
 }
 
+static void monitor_fdset_cleanup(MonFdset *mon_fdset)
+{
+MonFdsetFd *mon_fdset_fd;
+MonFdsetFd *mon_fdset_fd_next;
+
+QLIST_FOREACH_SAFE(mon_fdset_fd, mon_fdset-fds, next, mon_fdset_fd_next) 
{
+if (mon_fdset_fd-removed) {
+close(mon_fdset_fd-fd);
+g_free(mon_fdset_fd-opaque);
+QLIST_REMOVE(mon_fdset_fd, next);
+g_free(mon_fdset_fd);
+}
+}
+
+if (QLIST_EMPTY(mon_fdset-fds)) {
+QLIST_REMOVE(mon_fdset, next);
+g_free(mon_fdset);
+}
+}
+
+AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque,
+  const char *opaque, Error **errp)
+{
+int fd;
+Monitor *mon = cur_mon;
+MonFdset *mon_fdset;
+MonFdsetFd *mon_fdset_fd;
+AddfdInfo *fdinfo;
+
+fd = qemu_chr_fe_get_msgfd(mon-chr);
+if (fd == -1) {
+error_set(errp, QERR_FD_NOT_SUPPLIED);
+goto error;
+}
+
+if (has_fdset_id) {
+QLIST_FOREACH(mon_fdset, mon_fdsets, next) {
+if (mon_fdset-id == fdset_id) {
+break;
+}
+}
+if (mon_fdset == NULL) {
+error_set(errp, QERR_INVALID_PARAMETER_VALUE, fdset-id,
+  an existing fdset-id or no fdset-id);
+goto error;
+}
+} else {
+  

[Qemu-devel] building qemu on macos mountain lion

2012-08-11 Thread Peter Maydell
[Karl: I've cc'd you because I think you were the person asking about this
on #qemu IRC yesterday. Apologies if I have the wrong person...]

I just had a go at building qemu on macos X (mountain lion). The good news
is I got something working, but I had to fiddle with stuff a bit:
 * block/raw-posix.c includes IOKitLib.h -- unless we build with
  -DOS_OBJECT_USE_OBJC=0 the system headers end up trying
  to use objective C syntax and gcc barfs
 * the ui/cocoa.m file uses a system header which pulls in NSTask.h
   which uses Apple's non-standard Blocks objC extension. I worked
   round this by having rules.mak use clang to build all .m files (they
   don't use the major gcc-only-ism we need and they link fine with
   everything else)

[This is compiling with a fink build of mainline gcc-4.7.1]

Also 'make check' fails:

GTESTER tests/test-iov
send: Message too long
recv: Message too long
GTester: last random seed: R02Sf266e22f7524d5995dc2694b2a44c128

probably we're relying on a linuxism for sendmsg() splitting long messages,
haven't investigated yet.

I'll try to put together some patches for this but thought I'd send this
email to see if anybody else had already looked at them.

-- PMM



Re: [Qemu-devel] [PATCH v9 2/7] qapi: Introduce add-fd, remove-fd, query-fdsets

2012-08-11 Thread Eric Blake
On 08/11/2012 07:14 AM, Corey Bryant wrote:
 This patch adds support that enables passing of file descriptors
 to the QEMU monitor where they will be stored in specified file
 descriptor sets.
 

 
 v9:
  -Use fdset-id rather than fdset_id. (ebl...@redhat.com)
  -Update example for query-fdsets. (ebl...@redhat.com)
  -Close fd immediately on remove-fd.
   (kw...@redhat.com, ebl...@redhat.com)
  -Drop fdset refcount, and check dup_fds instead (in a later patch).
   (ebl...@redhat.com)
  -Move mon_refcount code to a later patch. (kw...@redhat.com)
 

 +AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque,
 +  const char *opaque, Error **errp)
 +{
 +int fd;
 +Monitor *mon = cur_mon;
 +MonFdset *mon_fdset;
 +MonFdsetFd *mon_fdset_fd;
 +AddfdInfo *fdinfo;
 +
 +fd = qemu_chr_fe_get_msgfd(mon-chr);
 +if (fd == -1) {
 +error_set(errp, QERR_FD_NOT_SUPPLIED);
 +goto error;
 +}
 +
 +if (has_fdset_id) {
 +QLIST_FOREACH(mon_fdset, mon_fdsets, next) {
 +if (mon_fdset-id == fdset_id) {
 +break;
 +}
 +}
 +if (mon_fdset == NULL) {
 +error_set(errp, QERR_INVALID_PARAMETER_VALUE, fdset-id,
 +  an existing fdset-id or no fdset-id);

The 'no fdset-id' portion of this error message doesn't make sense - it
can only be reached if has_fdset_id was true.

 +
 +void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
 +{
 +MonFdset *mon_fdset;
 +MonFdsetFd *mon_fdset_fd;
 +char fd_str[60];
 +
 +QLIST_FOREACH(mon_fdset, mon_fdsets, next) {
...

 +}
 +
 +error:
 +snprintf(fd_str, sizeof(fd_str),
 + fdset-id:% PRId64 , fd:% PRId64, fdset_id, fd);

Oops - fd is uninitialized if has_fd is false and the outer loop failed
to find fdset_id.  You need two separate error messages here, based on
whether fd was provided.

 +- { execute: query-fdsets }
 +- { return: [
 +   {
 + fds: [
 +   {
 + fd: 30,
 + opaque: rdonly:/path/to/file
 +   },
 +   {
 + fd: 24,
 + opaque: rdwr:/path/to/file
 +   }
 + ],
 + fdset-id: 1
 +   },
 +   {
 + fds: [
 +   {
 + fd: 28
 +   },
 +   {
 + fd: 29
 +   }
 + ],
 + fdset-id: 0
 +   },

No trailing comma here.

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v9 6/7] block: Enable qemu_open/close to work with fd sets

2012-08-11 Thread Eric Blake
On 08/11/2012 07:14 AM, Corey Bryant wrote:
 When qemu_open is passed a filename of the /dev/fdset/nnn
 format (where nnn is the fdset ID), an fd with matching access
 mode flags will be searched for within the specified monitor
 fd set.  If the fd is found, a dup of the fd will be returned
 from qemu_open.
 

 v9:
  -Drop fdset refcount and check dup_fds instead. (ebl...@redhat.com)
  -Fix dupfd leak in qemu_dup(). (ebl...@redhat.com)
  -Always set O_CLOEXEC in qemu_dup(). (kw...@redhat.com)
  -Change name of qemu_dup() to qemu_dup_flags(). (kw...@redhat.com)
 

 @@ -87,6 +146,40 @@ int qemu_open(const char *name, int flags, ...)
  int ret;
  int mode = 0;
  
 +#ifndef _WIN32
 +const char *fdset_id_str;
 +
 +/* Attempt dup of fd from fd set */
 +if (strstart(name, /dev/fdset/, fdset_id_str)) {
 +int64_t fdset_id;
 +int fd, dupfd;
 +
 +fdset_id = qemu_parse_fdset(fdset_id_str);
 +if (fdset_id == -1) {
 +errno = EINVAL;
 +return -1;
 +}
 +
 +fd = monitor_fdset_get_fd(fdset_id, flags);
 +if (fd == -1) {
 +return -1;
 +}
 +
 +dupfd = qemu_dup_flags(fd, flags);
 +if (fd == -1) {

Checking the wrong condition:
s/fd/dupfd/

 +return -1;
 +}
 +
 +ret = monitor_fdset_dup_fd_add(fdset_id, dupfd);
 +if (ret == -1) {
 +close(dupfd);
 +return -1;

This function appears to promise a reasonable errno on failure.
However, I don't think monitor_fdset_dup_fd_add guarantees a reasonable
errno, and even if it does, close() can corrupt errno.  I think that
prior to returning here, you either need an explicit errno=ENOMEM, or
fix monitor_fdset_dup_fd to guarantee a nice errno, plus a save and
restore of errno here.  Unless no one cares about errno on failure, in
which case your earlier errno=EINVAL can be dropped.

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] w64: Fix compiler warning [-Wformat]

2012-08-11 Thread Blue Swirl
On Fri, Aug 10, 2012 at 7:45 PM, Stefan Weil s...@weilnetz.de wrote:
 Glib2 uses __printf__ in macro G_GNUC_PRINTF for printf like
 functions. For MinGW, we want __gnu_printf__ because we use
 POSIX format specifiers instead of the MS format specifiers.

I get these warnings from glib:
  CCtrace/control.o
In file included from
/usr/local/i686-mingw32msvc/include/glib-2.0/glib/gthread.h:34,
 from
/usr/local/i686-mingw32msvc/include/glib-2.0/glib/gasyncqueue.h:34,
 from /usr/local/i686-mingw32msvc/include/glib-2.0/glib.h:34,
 from /src/qemu/qemu-common.h:40,
 from /src/qemu/trace/control.h:13,
 from /src/qemu/trace/default.c:10:
/usr/local/i686-mingw32msvc/include/glib-2.0/glib/gerror.h:46:
warning: '__gnu_printf__' is an unrecognized format function type
/usr/local/i686-mingw32msvc/include/glib-2.0/glib/gerror.h:70:
warning: '__gnu_printf__' is an unrecognized format function type
/usr/local/i686-mingw32msvc/include/glib-2.0/glib/gerror.h:88:
warning: '__gnu_printf__' is an unrecognized format function type
/usr/local/i686-mingw32msvc/include/glib-2.0/glib/gerror.h:94:
warning: '__gnu_printf__' is an unrecognized format function type


 Signed-off-by: Stefan Weil s...@weilnetz.de
 ---
  configure |2 ++
  1 file changed, 2 insertions(+)

 diff --git a/configure b/configure
 index 7cbf4b4..7e69574 100755
 --- a/configure
 +++ b/configure
 @@ -512,6 +512,8 @@ if test $mingw32 = yes ; then
QEMU_CFLAGS=-DWIN32_LEAN_AND_MEAN -DWINVER=0x501 $QEMU_CFLAGS
# enable C99/POSIX format strings (needs mingw32-runtime 3.15 or later)
QEMU_CFLAGS=-D__USE_MINGW_ANSI_STDIO=1 $QEMU_CFLAGS
 +  # GLIB2 and other libraries also support C99/POSIX format strings.
 +  QEMU_CFLAGS=-D__printf__=__gnu_printf__ $QEMU_CFLAGS
LIBS=-lwinmm -lws2_32 -liphlpapi $LIBS
  cat  $TMPC  EOF
  int main(void) { return 0; }
 --
 1.7.10





Re: [Qemu-devel] [PATCHv2 00/19] unicore32: Add unicore32-softmmu support

2012-08-11 Thread Blue Swirl
On Fri, Aug 10, 2012 at 6:42 AM,  g...@mprc.pku.edu.cn wrote:
 From: Guan Xuetao g...@mprc.pku.edu.cn

 These patches implement softmmu support on unicore32 architecture.

Thanks, applied all.


 v1-v2: Correct maybe-uninitialized warning in gpio/pm/dma handlers.

 UniCore32 CPU is embedded in PKUnity-3 SoC, so we add necessary puv3
 devices simulation codes together.
 Only minimal system control modules are simulated, to make linux kernel
 boot and busybox run in initramfs.

 Thanks Andreas Farber, Blue Swirl, Chen Weiren and Dunrong Huang for
 their priceless advice.

 Any advice is greatly appreciated.

 Thanks,

 Guan Xuetao
 ---
 Andreas Färber (1):
   target-unicore32: Drop UC32_CPUID macros

 Guan Xuetao (18):
   unicore32-softmmu: Add unicore32-softmmu build support
   unicore32-softmmu: Add coprocessor 0(sysctrl) and 1(ocd) instruction
 support
   unicore32-softmmu: Make UniCore32 cpuid  exceptions correct and
 runable
   unicore32-softmmu: Implement softmmu specific functions
   unicore32-softmmu: Make sure that kernel can access user space
   unicore32-softmmu: Add puv3 soc/board support
   unicore32-softmmu: Add puv3 interrupt support
   unicore32-softmmu: Add puv3 ostimer support
   unicore32-softmmu: Add puv3 gpio support
   unicore32-softmmu: Add puv3 pm support
   unicore32-softmmu: Add puv3 dma support
   unicore32-softmmu: Add ps2 support
   unicore32-softmmu: Add maintainer information for UniCore32 machine
   unicore32-softmmu: Add is_default setting for puv3 machine
   unicore32: Split UniCore-F64 instruction helpers from helper.c
   unicore32: Disintegrate cpu_dump_state_ucf64 function
   unicore32: Close dump-option of cpu_dump_state_ucf64 function
   unicore32-softmmu: Add a minimal curses screen support

  MAINTAINERS   |8 +
  arch_init.c   |2 +
  arch_init.h   |1 +
  configure |1 +
  cpu-exec.c|1 +
  default-configs/unicore32-softmmu.mak |4 +
  hw/Makefile.objs  |7 +
  hw/puv3.c |  131 +
  hw/puv3.h |   49 
  hw/puv3_dma.c |  109 +++
  hw/puv3_gpio.c|  141 +
  hw/puv3_intc.c|  135 +
  hw/puv3_ost.c |  151 ++
  hw/puv3_pm.c  |  149 ++
  hw/unicore32/Makefile.objs|6 +
  linux-user/main.c |3 +-
  target-unicore32/Makefile.objs|4 +-
  target-unicore32/cpu.c|   19 +-
  target-unicore32/cpu.h|   18 +-
  target-unicore32/helper.c |  511 
 +++--
  target-unicore32/helper.h |   17 +-
  target-unicore32/machine.c|   23 ++
  target-unicore32/op_helper.c  |   44 +++-
  target-unicore32/softmmu.c|  267 +
  target-unicore32/translate.c  |  159 +--
  target-unicore32/ucf64_helper.c   |  345 ++
  26 files changed, 1904 insertions(+), 401 deletions(-)
  create mode 100644 default-configs/unicore32-softmmu.mak
  create mode 100644 hw/puv3.c
  create mode 100644 hw/puv3.h
  create mode 100644 hw/puv3_dma.c
  create mode 100644 hw/puv3_gpio.c
  create mode 100644 hw/puv3_intc.c
  create mode 100644 hw/puv3_ost.c
  create mode 100644 hw/puv3_pm.c
  create mode 100644 hw/unicore32/Makefile.objs
  create mode 100644 target-unicore32/machine.c
  create mode 100644 target-unicore32/softmmu.c
  create mode 100644 target-unicore32/ucf64_helper.c




Re: [Qemu-devel] [PATCH] Makefile: add qapi.py dependencies

2012-08-11 Thread Blue Swirl
On Fri, Aug 10, 2012 at 1:08 PM, Stefan Hajnoczi
stefa...@linux.vnet.ibm.com wrote:
 Commit 427a1a2cb1d35b83b6302886f46289f6d617134d (qapi: avoid reserved
 keywords) modifies qapi.py, which is used by qapi-types.py and other
 Python scripts.  Because Makefile has no dependencies for qapi.py the
 qapi code generator will not be rerun and the following build error is
 produced:

   net/slirp.c: In function ‘net_init_slirp’:
   net/slirp.c:721:50: error: ‘NetdevUserOptions’ has no member named 
 ‘q_restrict’

 Fix this issue by adding the missing qapi.py dependencies.

Thanks, applied.


 Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
 ---
  Makefile |   14 --
  1 file changed, 8 insertions(+), 6 deletions(-)

 diff --git a/Makefile b/Makefile
 index 000b46c..d736ea5 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -181,24 +181,26 @@ ifneq ($(wildcard config-host.mak),)
  include $(SRC_PATH)/tests/Makefile
  endif

 +qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py
 +
  qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\
 -$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-types.py
 +$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-types.py 
 $(qapi-py)
 $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py 
 $(gen-out-type) -o qga/qapi-generated -p qga-  $,   GEN   $@)
  qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\
 -$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-visit.py
 +$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-visit.py 
 $(qapi-py)
 $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py 
 $(gen-out-type) -o qga/qapi-generated -p qga-  $,   GEN   $@)
  qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\
 -$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-commands.py
 +$(SRC_PATH)/qapi-schema-guest.json $(SRC_PATH)/scripts/qapi-commands.py 
 $(qapi-py)
 $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py 
 $(gen-out-type) -o qga/qapi-generated -p qga-  $,   GEN   $@)

  qapi-types.c qapi-types.h :\
 -$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py
 +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
 $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py 
 $(gen-out-type) -o .  $,   GEN   $@)
  qapi-visit.c qapi-visit.h :\
 -$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py
 +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
 $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py 
 $(gen-out-type) -o .   $,   GEN   $@)
  qmp-commands.h qmp-marshal.c :\
 -$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py
 +$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
 $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py 
 $(gen-out-type) -m -o .  $,   GEN   $@)

  QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h 
 qga-qapi-visit.h qga-qmp-commands.h)
 --
 1.7.10.4




Re: [Qemu-devel] [PATCH] exec.c: fix dirty bitmap reallocation

2012-08-11 Thread Blue Swirl
Thanks, applied.

On Fri, Aug 10, 2012 at 2:45 PM, Igor Mitsyanko i.mitsya...@samsung.com wrote:
 For each newly created RAM block, dirty bitmap is reallocated with g_realloc, 
 which doesn't
 make any promises on initial content of new extra data in returned buffer. In 
 theory,
 we initialize this new data with cpu_physical_memory_set_dirty_range() call. 
 The
 problem is, cpu_physical_memory_set_dirty_range() has a side effect of 
 incrementing
 ram_list.dirty_pages variable, but only for pages which are not already 
 dirty. And
 page cleanliness is determined using the same not yet uninitialized dirty 
 bitmap
 we've just reallocated. This results in inconsistency between real dirty page 
 number
 and value in ram_list.dirty_pages variable, which in turn could (and will) 
 result
 in errors during VM migration.
 Zero initialize new dirty bitmap bytes to fix this problem.

 Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com
 ---
  exec.c |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

 diff --git a/exec.c b/exec.c
 index a42a0b5..929db5c 100644
 --- a/exec.c
 +++ b/exec.c
 @@ -2550,6 +2550,8 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, 
 void *host,

  ram_list.phys_dirty = g_realloc(ram_list.phys_dirty,
 last_ram_offset()  
 TARGET_PAGE_BITS);
 +memset(ram_list.phys_dirty + (new_block-offset  TARGET_PAGE_BITS),
 +   0, size  TARGET_PAGE_BITS);
  cpu_physical_memory_set_dirty_range(new_block-offset, size, 0xff);

  if (kvm_enabled())
 --
 1.7.5.4




[Qemu-devel] [PATCH v2] configure: fix double check tests with Clang

2012-08-11 Thread Blue Swirl
Configuring with Clang compiler with -Werror would not work after
improved checks:
/tmp/qemu-conf--25992-.c:4:32: error: self-comparison always evaluates
to true [-Werror,-Wtautological-compare]
int main(void) { return preadv == preadv; }
/tmp/qemu-conf--25992-.c:13:26: error: self-comparison always
evaluates to true [-Werror,-Wtautological-compare]
return epoll_create1 == epoll_create1;
/tmp/qemu-conf--25992-.c:3:13: error: explicitly assigning a variable
of type 'char **' to itself [-Werror,-Wself-assign]
environ = environ;

Avoid the errors by adjusting the tests.

Signed-off-by: Blue Swirl blauwir...@gmail.com
---

v1-v2: remove void * casts.

---
 configure |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 12fdc22..f0dbc03 100755
--- a/configure
+++ b/configure
@@ -2256,7 +2256,7 @@ cat  $TMPC EOF
 #include sys/types.h
 #include sys/uio.h
 #include unistd.h
-int main(void) { return preadv == preadv; }
+int main(void) { return preadv(0, 0, 0, 0); }
 EOF
 preadv=no
 if compile_prog   ; then
@@ -2552,7 +2552,7 @@ int main(void)
  * warning but not an error, and will proceed to fail the
  * qemu compile where we compile with -Werror.)
  */
-return epoll_create1 == epoll_create1;
+return (int)(uintptr_t)epoll_create1;
 }
 EOF
 if compile_prog   ; then
@@ -2945,7 +2945,7 @@ has_environ=no
 cat  $TMPC  EOF
 #include unistd.h
 int main(void) {
-environ = environ;
+environ = 0;
 return 0;
 }
 EOF
-- 
1.7.2.5




[Qemu-devel] [Bug 1032828] Re: 64-bit capable PPC models should be supported in 32-bit usermode.

2012-08-11 Thread Samuel Bronson
** Changed in: qemu
   Status: Invalid = Fix Released

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

Title:
  64-bit capable PPC models should be supported in 32-bit usermode.

Status in QEMU:
  Fix Released

Bug description:
  While it makes perfect sense for qemu-system-ppc not to allow the use
  of PPC64-capable CPU models -- it would presumably be very confusing
  to the poor guest operating systems -- qemu-ppc (the user-only
  variants) is a different story.

  Sure, the most obvious PPC64 features are disabled when the processor
  is operating in 32-bit mode; as [PEM-64bit-3.0] says in section 1.1.1
  (p. 31):

   64-bit implementations/32-bit mode---For compatibility with
   32-bit implementations, 64-bit implementations can be
   configured to operate in 32-bit mode by clearing the MSR[SF]
   bit. In 32-bit mode, the effective address is treated as a
   32-bit address, condition bits, such as overflow and carry
   bits, are set based on 32-bit arithmetic (for example, integer
   overflow occurs when the result exceeds 32 bits), and the count
   register (CTR) is tested by branch conditional instructions
   following conventions for 32-bit implementations. All
   applications written for 32-bit implementations will run
   without modification on 64-bit processors running in 32-bit
   mode.

  However, this does NOT make the PPC64 instructions invalid, and at
  least one of them is actually useful.  I discovered this when my
  attempt to revive the darwin-user port reached the point where PPC
  guests would die on an std (store double word) instruction in the
  commpage.

  [ppc/cpu_capabilities.h] describes the commpage thus:

  /*
   * The shared kernel/user comm page(s):
   *
   * The last eight pages of every address space are reserved for the 
kernel/user
   * comm area.  Because they can be addressed via a sign-extended 16-bit 
field,
   * it is particularly efficient to access code or data in the comm area with
   * absolute branches (ba, bla, bca) or absolute load/stores (lwz 
r0,-4096(0)).
   * Because the comm area can be reached from anywhere, dyld is not needed.
   * Although eight pages are reserved, presently only two are populated and 
mapped.
   *
   * Routines on the comm page(s) can be thought of as the firmware for 
extended processor
   * instructions, whose opcodes are special forms of bla.  Ie, they are cpu
   * capabilities.  During system initialization, the kernel populates the comm 
page with
   * code customized for the particular processor and platform.
   *
   * Because Mach VM cannot map the last page of an address space, the max 
length of
   * the comm area is seven pages.
   */

  Since the darwin-user port doesn't actually support the guest_base
  feature (yet), and the commpage is at a fixed location on each
  architecture, darwin-user can't simply map a fake one when doing PPC-
  on-PPC or Intel-on-Intel; it therefore punts and hopes that the
  emulated CPU can cope with whatever code the HOST kernel has placed
  there based on the HOST CPU's capabilities.

  In my particular case, trying to run ppc-darwin-user/qemu-ppc
  /bin/ls on a G5 (ppc970) system running Mac OS X, gdb was helpful
  enough to reveal that the instruction on which the guest was choking
  was in the __bzero() function.  A bit of searching on fxr.watson.org
  revealed that, on this system, the kernel is using the implementation
  from [ppc/commpage/bzero_128.s]. Since I have no difficulty in
  runnning arch -arch ppc /bin/ls, it would appear that this works
  fine on a G5.

  I can think of four strategies for dealing with the problem:

  1. Allowing PPC64-capable processors for ppc user-mode targets.

  2. Somehow trapping calls into the commpage and treating them like a
  sort of syscall.

  3. Replacing the commpage with a version that will work fine on both
  host and guest.

  4. Cleaning up darwin-user to the point where guest_base actually
  works, and setting it to something like 0x2000 in this case.

  [PEM-64bit-3.0]: PowerPC® Microprocessor Family: The Programming
  Environments Manual for 64-bit Microprocessors, Version 3.0
  
https://www-01.ibm.com/chips/techlib/techlib.nsf/techdocs/F7E732FF811F783187256FDD004D3797/$file/pem_64bit_v3.0.2005jul15.pdf

  (The following sources are from xnu-1228, which is the closest
  fxr.watson.org has to the xnu-1228.15.4~1 reported by uname -v.)

  [ppc/cpu_capabilities.h]:
  http://fxr.watson.org/fxr/source/osfmk/ppc/cpu_capabilities.h?v=xnu-1228

  [ppc/commpage/bzero_128.s]:
  http://fxr.watson.org/fxr/source/osfmk/ppc/commpage/bzero_128.s?v=xnu-1228

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



Re: [Qemu-devel] [PATCH v2] configure: fix double check tests with Clang

2012-08-11 Thread Peter Maydell
On 11 August 2012 16:11, Blue Swirl blauwir...@gmail.com wrote:
 Configuring with Clang compiler with -Werror would not work after
 improved checks:
 /tmp/qemu-conf--25992-.c:4:32: error: self-comparison always evaluates
 to true [-Werror,-Wtautological-compare]
 int main(void) { return preadv == preadv; }
 /tmp/qemu-conf--25992-.c:13:26: error: self-comparison always
 evaluates to true [-Werror,-Wtautological-compare]
 return epoll_create1 == epoll_create1;
 /tmp/qemu-conf--25992-.c:3:13: error: explicitly assigning a variable
 of type 'char **' to itself [-Werror,-Wself-assign]
 environ = environ;

 Avoid the errors by adjusting the tests.

 Signed-off-by: Blue Swirl blauwir...@gmail.com

Reviewed-by: Peter Maydell peter.mayd...@linaro.org

-- PMM



Re: [Qemu-devel] [PATCH v5 08/15] ssd0323: abort() instead of exit(1) on error.

2012-08-11 Thread Peter Maydell
On 11 August 2012 00:31, Peter Crosthwaite
peter.crosthwa...@petalogix.com wrote:
 On Mon, Aug 6, 2012 at 7:41 PM, Peter Maydell peter.mayd...@linaro.org 
 wrote:
 On 6 August 2012 03:16, Peter A. G. Crosthwaite
 peter.crosthwa...@petalogix.com wrote:
 To be more consistent with the newer ways of error signalling. That and 
 SIGABT
 is easier to debug with than exit(1).

 Signed-off-by: Peter A. G. Crosthwaite peter.crosthwa...@petalogix.com

 Reviewed-by: Peter Maydell peter.mayd...@linaro.org


 This is independent of the rest of the series, do we want to en-queue
 to arm-devs and ill remove from series? One less diff for me and one
 less spam for the mailing list :)

Might as well. Applied to arm-devs.next.

-- PMM



Re: [Qemu-devel] [PATCH RFC for-1.2] arm: Move some ARM devices into libhw

2012-08-11 Thread Peter Maydell
On 10 August 2012 18:15, Andreas Färber afaer...@suse.de wrote:
 Am 02.08.2012 15:48, schrieb Peter Maydell:
 On 2 August 2012 02:16, Andreas Färber afaer...@suse.de wrote:
 +hw-obj-$(CONFIG_STELLARIS_ENET) += stellaris_enet.o

 Why just this stellaris device and not the others?

 So, turns out there was an actual reason:

   CClibhw64/hw/stellaris.o
 In file included from /home/andreas/QEMU/qemu-arm/hw/stellaris.c:12:0:
 /home/andreas/QEMU/qemu-arm/hw/arm-misc.h:19:28: error: unknown type
 name ‘ARMCPU’
 /home/andreas/QEMU/qemu-arm/hw/arm-misc.h:53:34: error: unknown type
 name ‘ARMCPU’
 /home/andreas/QEMU/qemu-arm/hw/arm-misc.h:55:38: error: unknown type
 name ‘ARMCPU’
 /home/andreas/QEMU/qemu-arm/hw/arm-misc.h:62:22: error: unknown type
 name ‘ARMCPU’
 make[1]: *** [hw/stellaris.o] Fehler 1
 make: *** [subdir-libhw64] Fehler 2

Yeah, although in fact this is just the result of arm-misc.h being a bit
of a grab-bag header: all stellaris.c needs is the prototype for armv7m_init()
and the system_clock_scale extern definition, not any of the ARMCPU
using things.

(system_clock_scale incidentally is pretty nasty -- this should be
done as some kind of QOM connection from the board model to the
armv7m and its systick device so the board model can feed the
right value in. This corresponds to hardware, where there is an external
set of signal lines for the board to pass this value in with.)

Also, stellaris.c is the board model, not a pure device, so you could
kind of construct a justification for only moving the other stellaris files
(ideally the devices wedged into stellaris.c at the moment would move
out of it, I guess.)

 Leave this part of the patch as is for now then?

If you don't want to try fixing this mess now I'd leave all the stellaris
bits where they are for the moment.

-- PMM



Re: [Qemu-devel] [PATCH 11/11] configure: Check for -Werror causing failures when compiling tests

2012-08-11 Thread Blue Swirl
Thanks, applied.


On Wed, Jul 18, 2012 at 2:10 PM, Peter Maydell peter.mayd...@linaro.org wrote:
 Add support for checking whether test case code can compile without
 warnings, by recompiling each successful test with -Werror. If the
 -Werror version doesn't pass, we bail out. This gives us the same
 level of visibility of warnings in test code as --enable-werror
 provides for the main compile.

 Signed-off-by: Peter Maydell peter.mayd...@linaro.org
 ---
  configure |   32 
  1 files changed, 28 insertions(+), 4 deletions(-)

 diff --git a/configure b/configure
 index 8140464..1939bdb 100755
 --- a/configure
 +++ b/configure
 @@ -27,16 +27,40 @@ printf  '%s' $0 $@  config.log
  echo  config.log
  echo #  config.log

 +do_cc() {
 +# Run the compiler, capturing its output to the log.
 +echo $cc $@  config.log
 +$cc $@  config.log 21 || return $?
 +# Test passed. If this is an --enable-werror build, rerun
 +# the test with -Werror and bail out if it fails. This
 +# makes warning-generating-errors in configure test code
 +# obvious to developers.
 +if test $werror != yes; then
 +return 0
 +fi
 +# Don't bother rerunning the compile if we were already using -Werror
 +case $* in
 +*-Werror*)
 +   return 0
 +;;
 +esac
 +echo $cc -Werror $@  config.log
 +$cc -Werror $@  config.log 21  return $?
 +echo ERROR: configure test passed without -Werror but failed with 
 -Werror.
 +echo This is probably a bug in the configure script. The failing 
 command
 +echo will be at the bottom of config.log.
 +echo You can run configure with --disable-werror to bypass this check.
 +exit 1
 +}
 +
  compile_object() {
 -  echo $cc $QEMU_CFLAGS -c -o $TMPO $TMPC  config.log
 -  $cc $QEMU_CFLAGS -c -o $TMPO $TMPC  config.log 21
 +  do_cc $QEMU_CFLAGS -c -o $TMPO $TMPC
  }

  compile_prog() {
local_cflags=$1
local_ldflags=$2
 -  echo $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags 
  config.log
 -  $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags  
 config.log 21
 +  do_cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags
  }

  # symbolically link $1 to $2.  Portable version of ln -sf.
 --
 1.7.5.4





Re: [Qemu-devel] [PATCH v2] configure: fix double check tests with Clang

2012-08-11 Thread Blue Swirl
On Sat, Aug 11, 2012 at 5:44 PM, Peter Maydell peter.mayd...@linaro.org wrote:
 On 11 August 2012 16:11, Blue Swirl blauwir...@gmail.com wrote:
 Configuring with Clang compiler with -Werror would not work after
 improved checks:
 /tmp/qemu-conf--25992-.c:4:32: error: self-comparison always evaluates
 to true [-Werror,-Wtautological-compare]
 int main(void) { return preadv == preadv; }
 /tmp/qemu-conf--25992-.c:13:26: error: self-comparison always
 evaluates to true [-Werror,-Wtautological-compare]
 return epoll_create1 == epoll_create1;
 /tmp/qemu-conf--25992-.c:3:13: error: explicitly assigning a variable
 of type 'char **' to itself [-Werror,-Wself-assign]
 environ = environ;

 Avoid the errors by adjusting the tests.

 Signed-off-by: Blue Swirl blauwir...@gmail.com

 Reviewed-by: Peter Maydell peter.mayd...@linaro.org

Thanks for the review, applied.


 -- PMM



Re: [Qemu-devel] [PATCH] w64: Fix compiler warning [-Wformat]

2012-08-11 Thread Stefan Weil

Am 11.08.2012 16:42, schrieb Blue Swirl:

On Fri, Aug 10, 2012 at 7:45 PM, Stefan Weil s...@weilnetz.de wrote:

Glib2 uses __printf__ in macro G_GNUC_PRINTF for printf like
functions. For MinGW, we want __gnu_printf__ because we use
POSIX format specifiers instead of the MS format specifiers.

I get these warnings from glib:
   CCtrace/control.o
In file included from
/usr/local/i686-mingw32msvc/include/glib-2.0/glib/gthread.h:34,
  from
/usr/local/i686-mingw32msvc/include/glib-2.0/glib/gasyncqueue.h:34,
  from /usr/local/i686-mingw32msvc/include/glib-2.0/glib.h:34,
  from /src/qemu/qemu-common.h:40,
  from /src/qemu/trace/control.h:13,
  from /src/qemu/trace/default.c:10:
/usr/local/i686-mingw32msvc/include/glib-2.0/glib/gerror.h:46:
warning: '__gnu_printf__' is an unrecognized format function type
/usr/local/i686-mingw32msvc/include/glib-2.0/glib/gerror.h:70:
warning: '__gnu_printf__' is an unrecognized format function type
/usr/local/i686-mingw32msvc/include/glib-2.0/glib/gerror.h:88:
warning: '__gnu_printf__' is an unrecognized format function type
/usr/local/i686-mingw32msvc/include/glib-2.0/glib/gerror.h:94:
warning: '__gnu_printf__' is an unrecognized format function type


Oh, sorry, I forgot that older versions of gcc (before 4.4)
don't support __gnu_printf__.

I'll have to modify my patch...

Regards,

Stefan W.




Re: [Qemu-devel] [PATCH v2 00/12] Portable thread-pool/AIO, Win32 emulated AIO

2012-08-11 Thread Stefan Weil

Am 07.08.2012 13:17, schrieb Paolo Bonzini:

This patch series is part 2 in my EventNotifier/AIO improvements
for QEMU 1.2.  It extends use of EventNotifier to the main loop
and AIO subsystems.  A new API using EventNotifier is added to aio.c
and a new portable thread pool is introduced (based on code from
posix-aio-compat.c, mostly) that uses this API.  raw-posix.c is
converted to use the new thread pool, and support for asynchronous
I/O is finally added to Win32 as well.

The network drivers (curl, libiscsi, nbd) have to be disabled
under Windows.  They are unlikely to have any users, since they
were broken until 1.0 and (unlike slirp) we never had any report.

I tested this under Wine, with a RHEL virtual machine booting just as
glacially as before.  info blockstats does show a slightly higher
overhead, so I would like this to be tested on real Windows hosts.
Even if the result is negative, I would prefer to keep the early
parts (i.e. drop only the last patch) since they are a prerequisite for
improvements to block/raw-posix.c (such as asynchronous discard
support) scheduled for 1.3.  The platform independent APIs introduced
by patches 4 and 5 are also useful for native AIO on Win32.


Paolo Bonzini (12):
   event_notifier: enable it to use pipes
   event_notifier: add Win32 implementation
   main-loop: use event notifiers
   aio: provide platform-independent API
   aio: add Win32 implementation
   linux-aio: use event notifiers
   qemu-thread: add QemuSemaphore
   aio: add generic thread-pool facility
   block: switch posix-aio-compat to threadpool
   raw: merge posix-aio-compat.c into block/raw-posix.c
   raw-posix: rename raw-posix-aio.h, hide unavailable prototypes
   raw-win32: add emulated AIO support

  Makefile.objs|  12 +-
  aio.c = aio-posix.c |   9 +
  aio-win32.c  | 177 +
  block/Makefile.objs  |   6 +-
  block/{raw-posix-aio.h = raw-aio.h} |  19 +-
  block/raw-posix.c| 297 ++-
  block/raw-win32.c| 187 +++---
  event_notifier-posix.c   | 118 ++
  event_notifier-win32.c   |  59 +++
  event_notifier.c |  67 
  event_notifier.h |  20 +-
  linux-aio.c  |  51 ++-
  main-loop.c  | 106 +-
  oslib-posix.c|  31 --
  posix-aio-compat.c   | 679 ---
  qemu-aio.h   |  19 +-
  qemu-common.h|   1 -
  qemu-thread-posix.c  |  74 
  qemu-thread-posix.h  |   5 +
  qemu-thread-win32.c  |  35 ++
  qemu-thread-win32.h  |   4 +
  qemu-thread.h|   7 +
  thread-pool.c| 279 ++
  thread-pool.h|  34 ++
  trace-events |   5 +
  25 file modificati, 1323 inserzioni(+), 978 rimozioni(-)
  rename aio.c = aio-posix.c (92%)
  create mode 100644 aio-win32.c
  rename block/{raw-posix-aio.h = raw-aio.h} (62%)
  create mode 100644 event_notifier-posix.c
  create mode 100644 event_notifier-win32.c
  delete mode 100644 event_notifier.c
  delete mode 100644 posix-aio-compat.c
  create mode 100644 thread-pool.c
  create mode 100644 thread-pool.h



Hi,

I needed these changes for compilation with MinGW32:

diff --git a/block/raw-posix.c b/block/raw-posix.c
index ad65604..c948f97 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -280,8 +280,10 @@ static int raw_open_common(BlockDriverState *bs, 
const char *filename,


 return 0;

+#ifdef CONFIG_LINUX_AIO
 out_free_buf:
 qemu_vfree(s-aligned_buf);
+#endif
 out_close:
 close(fd);
 return -errno;
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 7a5b86f..ef5e200 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -42,7 +42,7 @@ typedef struct RawWin32AIOData {
 struct iovec *aio_iov;
 int aio_niov;
 size_t aio_nbytes;
-off_t aio_offset;
+off64_t aio_offset;
 int aio_type;
 } RawWin32AIOData;


Label out_free_buf is only used with CONFIG_LINUX_AIO.

off_t is always 32 bit with MinGW header files, but we
want support for large files, so off64_t is a better choice
here. It also avoids a compiler warning when the value
is right shifted by 32 bits.

I noticed some messages from checkpatch.pl.
Some (but not all) of them were caused by the renamed file.
Maybe this file should be fixed before it is renamed.

Regards,
Stefan




[Qemu-devel] [PATCH] framebuffer: Fix spelling in comment (leight - height)

2012-08-11 Thread Stefan Weil
Signed-off-by: Stefan Weil s...@weilnetz.de
---
 hw/framebuffer.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/framebuffer.c b/hw/framebuffer.c
index f4747cd..85a00a5 100644
--- a/hw/framebuffer.c
+++ b/hw/framebuffer.c
@@ -28,7 +28,7 @@ void framebuffer_update_display(
 MemoryRegion *address_space,
 target_phys_addr_t base,
 int cols, /* Width in pixels.  */
-int rows, /* Leight in pixels.  */
+int rows, /* Height in pixels.  */
 int src_width, /* Length of source line, in bytes.  */
 int dest_row_pitch, /* Bytes between adjacent horizontal output pixels.  */
 int dest_col_pitch, /* Bytes between adjacent vertical output pixels.  */
-- 
1.7.10




[Qemu-devel] [PATCH] iov_send_recv(): Handle zero bytes case even if OS does not

2012-08-11 Thread Peter Maydell
POSIX allows sendmsg() and recvmsg() to fail EMSGSIZE if passed a zero
msg.msg_iovlen (in particular the MacOS X implementation will do this).
Handle the case where iov_send_recv() is passed a zero byte count
explicitly, to avoid accidentally depending on the OS to treat zero
msg_iovlen as a no-op.

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
This is what was causing 'make check' to fail on MacOS X.
The other option was to declare that a zero bytecount was illegal, I guess.

 iov.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/iov.c b/iov.c
index b333061..60705c7 100644
--- a/iov.c
+++ b/iov.c
@@ -146,6 +146,13 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, 
unsigned iov_cnt,
 {
 ssize_t ret;
 unsigned si, ei;/* start and end indexes */
+if (bytes == 0) {
+/* Catch the do-nothing case early, as otherwise we will pass an
+ * empty iovec to sendmsg/recvmsg(), and not all implementations
+ * accept this.
+ */
+return 0;
+}
 
 /* Find the start position, skipping `offset' bytes:
  * first, skip all full-sized vector elements, */
-- 
1.7.11.4




[Qemu-devel] [PATCH 2/2] Support using a different compiler for Objective-C files

2012-08-11 Thread Peter Maydell
MacOSX 10.8 (Mountain Lion) requires us to compile our one
Objective-C source file with clang even if the rest of QEMU
requires a real gcc, because the system headers we use make
use of Apple's Blocks extension to C/ObjC, and mainline
gcc doesn't support that. Since we only need to use a true
gcc for the parts of QEMU that use the fixed-register
env variable, we can simply use clang to build the ObjC
file: it will link to the gcc-built objects with no problems.

Add the necessary support for an OBJCC variable in the
makefile and configure machinery; we default to clang
if we have it, otherwise whatever CC is (since gcc
might be the Apple gcc which does support Blocks).

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
 configure | 12 
 rules.mak |  2 +-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index be4a2bb..bd62d2c 100755
--- a/configure
+++ b/configure
@@ -534,6 +534,13 @@ fi
 : ${python=${PYTHON-python}}
 : ${smbd=${SMBD-/usr/sbin/smbd}}
 
+# Default objcc to clang if available, otherwise use CC
+if has clang; then
+  objcc=clang
+else
+  objcc=$cc
+fi
+
 if test $mingw32 = yes ; then
   EXESUF=.exe
   QEMU_CFLAGS=-DWIN32_LEAN_AND_MEAN -DWINVER=0x501 $QEMU_CFLAGS
@@ -577,6 +584,8 @@ for opt do
   ;;
   --host-cc=*) host_cc=$optarg
   ;;
+  --objcc=*) objcc=$optarg
+  ;;
   --make=*) make=$optarg
   ;;
   --install=*) install=$optarg
@@ -1024,6 +1033,7 @@ echo   --cross-prefix=PREFIXuse PREFIX for compile 
tools [$cross_prefix]
 echo   --cc=CC  use C compiler CC [$cc]
 echo   --host-cc=CC use C compiler CC [$host_cc] for code run at
 echobuild time
+echo   --objcc=OBJCCuse Objective-C compiler OBJCC [$objcc]
 echo   --extra-cflags=CFLAGSappend extra C compiler flags QEMU_CFLAGS
 echo   --extra-ldflags=LDFLAGS  append extra linker flags LDFLAGS
 echo   --make=MAKE  use specified make [$make]
@@ -3054,6 +3064,7 @@ fi
 echo Source path   $source_path
 echo C compiler$cc
 echo Host C compiler   $host_cc
+echo Objective-C compiler $objcc
 echo CFLAGS$CFLAGS
 echo QEMU_CFLAGS   $QEMU_CFLAGS
 echo LDFLAGS   $LDFLAGS
@@ -3521,6 +3532,7 @@ echo PYTHON=$python  $config_host_mak
 echo CC=$cc  $config_host_mak
 echo CC_I386=$cc_i386  $config_host_mak
 echo HOST_CC=$host_cc  $config_host_mak
+echo OBJCC=$objcc  $config_host_mak
 echo AR=$ar  $config_host_mak
 echo OBJCOPY=$objcopy  $config_host_mak
 echo LD=$ld  $config_host_mak
diff --git a/rules.mak b/rules.mak
index a284946..1b173aa 100644
--- a/rules.mak
+++ b/rules.mak
@@ -29,7 +29,7 @@ endif
$(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) 
$(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $,  AS$(TARGET_DIR)$@)
 
 %.o: %.m
-   $(call quiet-command,$(CC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) 
$(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $,  OBJC  $(TARGET_DIR)$@)
+   $(call quiet-command,$(OBJCC) $(QEMU_INCLUDES) $(QEMU_CFLAGS) 
$(QEMU_DGFLAGS) $(CFLAGS) -c -o $@ $,  OBJC  $(TARGET_DIR)$@)
 
 LINK = $(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) $(LDFLAGS) -o $@ 
$(sort $(1)) $(LIBS),  LINK  $(TARGET_DIR)$@)
 
-- 
1.7.11.4




[Qemu-devel] [PATCH 1/2] configure: Define OS_OBJECT_USE_OBJC=0 for MacOSX builds

2012-08-11 Thread Peter Maydell
MacOSX 10.8 (Mountain Lion) defaults to trying to use automated
reference counting on certain objects.  This means that the system
header files will use some Objective C syntax constructs even when
compiling pure C, which confuses mainline gcc. Suppress this by
setting OS_OBJECT_USE_OBJC=0. This avoids a compile error like this:

In file included from
/System/Library/Frameworks/Foundation.framework/Headers/NSObject.h:5:0,
 from /usr/include/os/object.h:74,
 from /usr/include/dispatch/dispatch.h:48,
 from 
/System/Library/Frameworks/IOKit.framework/Headers/IOKitLib.h:56,
 from block/raw-posix.c:35:
/System/Library/Frameworks/Foundation.framework/Headers/NSObjCRuntime.h:409:1: 
error: stray ‘@’ in program
[with a large number of further run-on errors]

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
 configure | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/configure b/configure
index f0dbc03..be4a2bb 100755
--- a/configure
+++ b/configure
@@ -452,6 +452,9 @@ Darwin)
   audio_possible_drivers=coreaudio sdl fmod
   LDFLAGS=-framework CoreFoundation -framework IOKit $LDFLAGS
   libs_softmmu=-F/System/Library/Frameworks -framework Cocoa -framework IOKit 
$libs_softmmu
+  # Disable attempts to use ObjectiveC features in os/object.h since they
+  # won't work when we're compiling with gcc as a C compiler.
+  QEMU_CFLAGS=-DOS_OBJECT_USE_OBJC=0 $QEMU_CFLAGS
 ;;
 SunOS)
   solaris=yes
-- 
1.7.11.4




[Qemu-devel] [PATCH 0/2] Fix compilation on MacOS X 10.8 (Mountain Lion)

2012-08-11 Thread Peter Maydell
I had a go at compiling on my Macbook, which runs Apple's most recent
cat. I had to fix a couple of issues, which I think are down to new
stuff in the system headers:
 * the os/object.h header will try to use Objective-C syntax unless
   explicitly squashed
 * some of the headers we use in the cocoa ui frontend use the
   'Blocks' Apple extension, so won't build except with clang or
   the (now rapidly aging) Apple gcc, so to do a build with a
   modern gcc we need to support specifying the Objective-C compiler
   separately.

Peter Maydell (2):
  configure: Define OS_OBJECT_USE_OBJC=0 for MacOSX builds
  Support using a different compiler for Objective-C files

 configure | 15 +++
 rules.mak |  2 +-
 2 files changed, 16 insertions(+), 1 deletion(-)

-- 
1.7.11.4




Re: [Qemu-devel] github mirror still stale

2012-08-11 Thread Anthony Liguori
Peter Maydell peter.mayd...@linaro.org writes:

 On 18 July 2012 10:28, Peter Maydell peter.mayd...@linaro.org wrote:
 On 12 March 2012 20:12, Stefan Weil s...@weilnetz.de wrote:
 We also need more resources for technical maintenance of the
 QEMU infrastructure. For example, the official mirror of the
 QEMU git repository (https://github.com/qemu/QEMU) is several
 months behind, http://git.savannah.gnu.org/cgit/qemu.git is
 even older.

 The github mirror is still wildly out of date -- can we
 get the link to it removed from http://wiki.qemu.org/Download
 please? (I'd do it myself but the page is 'locked'.)

 Ping^2. It would be good to stop advertising stale git mirrors
 before we make the 1.2 release...

I messed up setting it up and had to delete the whole thing and start
over.

We now have a proper organization on github and which let me setup a
mirror from qemu.org that pushes every 30 minutes to github.

It should be active now.  I can also add other people to the github
organization.

Regards,

Anthony Liguori


 -- PMM



Re: [Qemu-devel] [PULL 0/7] last SCSI changes for 1.2

2012-08-11 Thread Anthony Liguori
Paolo Bonzini pbonz...@redhat.com writes:

 Anthony,

 The following changes since commit 0b8db8fe15d17a529a5ea90614c11e9f031dfee8:

   slirp: fix build on mingw32 (2012-08-06 19:31:55 -0500)

 are available in the git repository at:

   git://github.com/bonzini/qemu.git scsi-next

 for you to fetch changes up to 5222aaf223e52961cabeb7cabc579892ccd8bc59:

   scsi-disk: add support for the UNMAP command (2012-08-09 15:04:09 +0200)

Pulled. Thanks.

Regards,

Anthony Liguori


 
 Paolo Bonzini (6):
   iscsi: do not leak initiator_name
   iscsi: reorganize code for parse_initiator_name
   virtio-scsi: do not compare 32-bit QEMU tags against 64-bit virtio-scsi 
 tags
   scsi-disk: more assertions and resets for aiocb
   scsi-disk: improve out-of-range LBA detection for WRITE SAME
   scsi-disk: add support for the UNMAP command

 Ronnie Sahlberg (1):
   iscsi: Pick default initiator-name based on the name of the VM

  block/iscsi.c|  59 ++---
  hw/scsi-disk.c   | 112 
 ++-
  hw/virtio-scsi.c |  10 -
  qemu-common.h|   1 +
  qemu-doc.texi|   5 +++
  qemu-options.hx  |   8 
  qemu-tool.c  |   5 +++
  vl.c |   5 +++
  8 file modificati, 163 inserzioni(+), 42 rimozioni(-)
 -- 
 1.7.11.2



[Qemu-devel] [Bug 1013691] Re: ppc64 + virtio-scsi: only first scsi disk shows up in the guest

2012-08-11 Thread Benjamin Herrenschmidt
Only just saw this bug, I assume the problem still exist ? I have
somebody look at it next week

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

Title:
  ppc64 + virtio-scsi: only first scsi disk shows up in the guest

Status in QEMU:
  New

Bug description:
  When adding two virtio-scsi targets to a single guest, only the first
  disk is seen inside the guest.  For some unknown reason the guest
  doesn't enumerate the second disk.

  For full qemu-system-ppc64 command line and 'dmesg' output, see:

  http://lists.nongnu.org/archive/html/qemu-devel/2012-06/msg02430.html

  I have also tried this with Linus's git tree (3.5.0-rc2+ at time of writing),
  same thing.

  In both cases I'm using qemu from git.

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



Re: [Qemu-devel] [PATCH] block: Set cdrom device read only flag

2012-08-11 Thread Kevin Shanahan
On Thu, Aug 09, 2012 at 10:42:51AM +0200, Kevin Wolf wrote:
 Am 07.08.2012 10:47, schrieb Markus Armbruster:
  Kevin Wolf kw...@redhat.com writes:
  
  Am 02.08.2012 09:20, schrieb Kevin Shanahan:
  On Thu, Aug 02, 2012 at 02:49:52PM +0930, Kevin Shanahan wrote:
  On Thu, Aug 02, 2012 at 11:46:13AM +0930, Kevin Shanahan wrote:
  On Thu, Aug 02, 2012 at 11:02:42AM +0930, Kevin Shanahan wrote:
  Set the block driver read_only flag for cdrom devices so that
  qmp_change_blockdev does not attempt to open cdrom files in read-write
  mode when changing media.
 
  Hrm, this fixes my simple test case using the kvm monitor directly but
  changing media via libvirt still has the same issue (fails for RO
  files, succeeds for writable files).
 
  $ virsh attach-disk --type cdrom --mode readonly test1
  /srv/kvm/iso/ubuntu-12.04-server-amd64.iso hdc
  error: Failed to attach disk
  error: internal error unable to execute QEMU command 'change':
  Could not open '/srv/kvm/iso/ubuntu-12.04-server-amd64.iso'
 
  I'll keep looking into it.
 
  In the libvirt case, it seems libvirt is failing to add media=cdrom to
  the commandline, so in this case qemu is defaulting to media=disk and
  my proposed fix has no effect. Diving into libvirt now to see why no
  media=disk is getting added...
 
  Common test case has this xml (generated by virt-install):
 
  disk type='block' device='cdrom'
driver name='qemu' type='raw'/
target dev='hdc' bus='ide'/
readonly/
alias name='ide0-1-0'/
address type='drive' controller='0' bus='1' target='0' unit='0'/
  /disk
 
  Ok, looks like libvirt is intentionally leaving media=cdrom off the
  command line in the case that -device ide-cd,... is
  supported. Presumably by specifying the device this way, qemu is
  supposed to work out that the media type is cdrom automatically (but
  it doesn't, it defaults to disk).
 
  Libvirt wants to use:
 
  qemu-kvm ... \
   -drive if=none,id=drive-ide0-1-0,readonly=on,format=raw \
   -device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 ...
 
  If I hack qemu/qemu_command.c::qemuBuildDriveStr() to ignore the check
  for QEMU_CAPS_IDE_CD and always add media=cdrom, then (with my qemu as
  well patch) qemu will open cdrom media files read-only.
 
  There's probably a neater way to just get qemu to set the media type
  if -device ide-cd,... is used, but I haven't worked it out yet.
 
  Anyway, apologies for the rambling conversation with myself on your
  lists. Hope this is helpful in some way.
 
  Thanks, that's some good information.
 
  However, I don't think you should start from media=cdrom. libvirt
  already does specify readonly=on and that is the property you're really
  interested in. Since commit 528f7663 (released with 0.13) the 'change'
  command should keep the read-only flag for all kinds of media.
  
  Correct.
  
  Now I'm not sure where you lost the read-only flag. At least on git
  master it doesn't seem to reproduce for me.
  
  I can:
  
  $ qemu --enable-kvm -S -m 384 -vnc localhost:5500 -monitor stdio \
  -drive if=none,id=cd,readonly=on,format=raw \
  -device ide-cd,bus=ide.1,unit=0,drive=cd
  QEMU 1.1.50 monitor - type 'help' for more information
  (qemu) change cd r5.iso
  Could not open 'r5.iso'
  (qemu) q
  armbru@blackfin:~/work/images$ ls -l r5.iso 
  -r--r--r--. 1 armbru armbru 2872639488 Mar 31  2011 r5.iso
  
  Looks like a QEMU bug to me.
 
 Right, now it breaks for me as well. The difference is that when I tried
 on Monday, I didn't use an empty drive.
 
 Kevin

So qmp_change_blockdev uses bdrv_is_read_only() to check whether to
try and open the backing file read only, which uses the -read_only
member of struct BlockDriverState to decide whether to pass the
BDRV_O_RDRW flag to qmp_bdrv_open_encypted() and then bdrv_open().

I would assume we want to set this flag in drive_init() when the block
driver state is initialised. How about a patch like this instead?

diff --git a/blockdev.c b/blockdev.c
index 8669142..ba22064 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -526,6 +526,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
  if_name[type], mediastr, unit_id);
 }
 dinfo-bdrv = bdrv_new(dinfo-id);
+dinfo-bdrv-read_only = ro;
 dinfo-devaddr = devaddr;
 dinfo-type = type;
 dinfo-bus = bus_id;



[Qemu-devel] [Bug 1035572] [NEW] Bug in Qemu User Mode

2012-08-11 Thread Dietmar Stölting
Public bug reported:

Hi,
I make an interesting discovery.
My aim is to have a working qemu-i386 on Raspberry Pi.
After long searching in the dark what goes wrong with ANY Qemu version for User 
Mode until today,
I find the following: The bug must be in at least one function, that the 
program testclone
from the testpackage for i386 in linux-user-test-0.3 calls.
The wrong function is in the part, which enables more than one thread at the 
same time, NPTL.
Funny, how I find this out: All the programs from the tests in 
linux-user-test-0.3 I can now run succesfull with my new builded qemu-i386 for 
Raspi.
But the program testclone does not stop after it gives out all the right 
messages.
The program testclone stops on my Desktop computer with Debian Wheezy installed.
So, the error is not in the program testclone.
So I make a look, what is going on there with strace. With strace you get 
informations about all the values in the working program, here testclone.
I see, that the reason, why testclone not stops is in an infinite loop because 
of 
while (waitpid(pid1, status1, 0) != pid1);
while (waitpid(pid2, status2, 0) != pid2);
at its end is never fullfilled. 
This is the reason for the famous error message from Qemu User Mode 

qemu: uncaught target signal 11 (Segmentation fault) - core dumped 
Segmentation fault 

stack1 = malloc(STACK_SIZE);
pid1 = clone(thread1_func, stack1 + STACK_SIZE,
CLONE_VM | CLONE_FS | CLONE_FILES | SIGCHLD, hello1);

stack2 = malloc(STACK_SIZE);
pid2 = clone(thread2_func, stack2 + STACK_SIZE, 
CLONE_VM | CLONE_FS | CLONE_FILES | SIGCHLD, hello2);

The error happens early in the program testclone. Strace says, it is because no 
childprocess at all can be found. So, some basiccalculations in those four 
lines must be done wrong from Qemu.
I think, that the adressspace for each thread is calculated wrong, or overlapps.
Funny, it has nothing to do with the ARM processor. I get exact the same 
errormessages, when I run the program testclone on my desktopcompi i386 with a 
Wheezy in Qemu and then qemu-i386 testclone.
This is a good message, because it means it is an error, that belongs at least 
to the i386 family but I think, every processor in Qemu User Mode is involved, 
so until now NPTL does not work.
Today I make a hand by hand calculation with the source code from testclone and 
compare it with the values, that Qemu User Mode give. The handcalculated values 
should  be the same which my 
Desktop computer with Wheezy with tesclone produces, but who knows,
Dietmar

PS: I hope, that this is the right source code for testclone. Any help
is welcome:-)!


Code: Select all
#include stdlib.h
#include stdio.h
#include string.h
#include signal.h
#include unistd.h
#include inttypes.h
#include pthread.h
#include sys/wait.h
#include sched.h

int thread1_func(void *arg)
{
int i;
char buf[512];

for(i=0;i10;i++) {
snprintf(buf, sizeof(buf), thread1: %d %s\n, i, (char *)arg);
   write(1, buf, strlen(buf));
usleep(100 * 1000);
}
return 0;
}

int thread2_func(void *arg)
{
int i;
char buf[512];
for(i=0;i20;i++) {
snprintf(buf, sizeof(buf), thread2: %d %s\n, i, (char *)arg);
write(1, buf, strlen(buf));
usleep(120 * 1000);
}
return 0;
}

#define STACK_SIZE 16384

void test_clone(void)
{
uint8_t *stack1, *stack2;
int pid1, pid2, status1, status2;

stack1 = malloc(STACK_SIZE);
pid1 = clone(thread1_func, stack1 + STACK_SIZE, 
 CLONE_VM | CLONE_FS | CLONE_FILES | SIGCHLD, hello1);

stack2 = malloc(STACK_SIZE);
pid2 = clone(thread2_func, stack2 + STACK_SIZE, 
CLONE_VM | CLONE_FS | CLONE_FILES | SIGCHLD, hello2);

while (waitpid(pid1, status1, 0) != pid1);
while (waitpid(pid2, status2, 0) != pid2);
printf(status1=0x%x\n, status1);
printf(status2=0x%x\n, status2);
printf(End of clone test.\n);
}

int main(int argc, char **argv)
{
test_clone();
return 0;
}
Posts: 210
Joined: 04 Sep 2011 17:43

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  Bug in Qemu User Mode

Status in QEMU:
  New

Bug description:
  Hi,
  I make an interesting discovery.
  My aim is to have a working qemu-i386 on Raspberry Pi.
  After long searching in the dark what goes wrong with ANY Qemu version for 
User Mode until today,
  I find the following: The bug must be in at least one function, that the 
program testclone
  from the testpackage for i386 in linux-user-test-0.3 calls.
  The wrong function is in the part, which enables more than one thread at the 
same time, NPTL.
  Funny, how I find this out: All the programs from the tests in 
linux-user-test-0.3 I can now run succesfull with my new builded qemu-i386 for 
Raspi.
  But the program testclone does not stop after it gives out all the 

[Qemu-devel] [Bug 1035572] Re: Bug in Qemu User Mode

2012-08-11 Thread Dietmar Stölting
I mean of course, that the tidptr from child and parent are different,

parent_tidptr=0x43076590   child_tidptr=0x42fcb788

Dietmar

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

Title:
  Bug in Qemu User Mode

Status in QEMU:
  New

Bug description:
  Hi,
  I make an interesting discovery.
  My aim is to have a working qemu-i386 on Raspberry Pi.
  After long searching in the dark what goes wrong with ANY Qemu version for 
User Mode until today,
  I find the following: The bug must be in at least one function, that the 
program testclone
  from the testpackage for i386 in linux-user-test-0.3 calls.
  The wrong function is in the part, which enables more than one thread at the 
same time, NPTL.
  Funny, how I find this out: All the programs from the tests in 
linux-user-test-0.3 I can now run succesfull with my new builded qemu-i386 for 
Raspi.
  But the program testclone does not stop after it gives out all the right 
messages.
  The program testclone stops on my Desktop computer with Debian Wheezy 
installed.
  So, the error is not in the program testclone.
  So I make a look, what is going on there with strace. With strace you get 
informations about all the values in the working program, here testclone.
  I see, that the reason, why testclone not stops is in an infinite loop 
because of 
  while (waitpid(pid1, status1, 0) != pid1);
  while (waitpid(pid2, status2, 0) != pid2);
  at its end is never fullfilled. 
  This is the reason for the famous error message from Qemu User Mode 

  qemu: uncaught target signal 11 (Segmentation fault) - core dumped 
  Segmentation fault 

  stack1 = malloc(STACK_SIZE);
  pid1 = clone(thread1_func, stack1 + STACK_SIZE,
  CLONE_VM | CLONE_FS | CLONE_FILES | SIGCHLD, hello1);

  stack2 = malloc(STACK_SIZE);
  pid2 = clone(thread2_func, stack2 + STACK_SIZE, 
  CLONE_VM | CLONE_FS | CLONE_FILES | SIGCHLD, hello2);

  The error happens early in the program testclone. Strace says, it is because 
no childprocess at all can be found. So, some basiccalculations in those four 
lines must be done wrong from Qemu.
  I think, that the adressspace for each thread is calculated wrong, or 
overlapps.
  Funny, it has nothing to do with the ARM processor. I get exact the same 
errormessages, when I run the program testclone on my desktopcompi i386 with a 
Wheezy in Qemu and then qemu-i386 testclone.
  This is a good message, because it means it is an error, that belongs at 
least to the i386 family but I think, every processor in Qemu User Mode is 
involved, so until now NPTL does not work.
  Today I make a hand by hand calculation with the source code from testclone 
and compare it with the values, that Qemu User Mode give. The handcalculated 
values should  be the same which my 
  Desktop computer with Wheezy with tesclone produces, but who knows,
  Dietmar

  PS: I hope, that this is the right source code for testclone. Any help
  is welcome:-)!

  
  Code: Select all
  #include stdlib.h
  #include stdio.h
  #include string.h
  #include signal.h
  #include unistd.h
  #include inttypes.h
  #include pthread.h
  #include sys/wait.h
  #include sched.h

  int thread1_func(void *arg)
  {
  int i;
  char buf[512];

  for(i=0;i10;i++) {
  snprintf(buf, sizeof(buf), thread1: %d %s\n, i, (char *)arg);
 write(1, buf, strlen(buf));
  usleep(100 * 1000);
  }
  return 0;
  }

  int thread2_func(void *arg)
  {
  int i;
  char buf[512];
  for(i=0;i20;i++) {
  snprintf(buf, sizeof(buf), thread2: %d %s\n, i, (char *)arg);
  write(1, buf, strlen(buf));
  usleep(120 * 1000);
  }
  return 0;
  }

  #define STACK_SIZE 16384

  void test_clone(void)
  {
  uint8_t *stack1, *stack2;
  int pid1, pid2, status1, status2;

  stack1 = malloc(STACK_SIZE);
  pid1 = clone(thread1_func, stack1 + STACK_SIZE, 
   CLONE_VM | CLONE_FS | CLONE_FILES | SIGCHLD, hello1);

  stack2 = malloc(STACK_SIZE);
  pid2 = clone(thread2_func, stack2 + STACK_SIZE, 
  CLONE_VM | CLONE_FS | CLONE_FILES | SIGCHLD, hello2);

  while (waitpid(pid1, status1, 0) != pid1);
  while (waitpid(pid2, status2, 0) != pid2);
  printf(status1=0x%x\n, status1);
  printf(status2=0x%x\n, status2);
  printf(End of clone test.\n);
  }

  int main(int argc, char **argv)
  {
  test_clone();
  return 0;
  }
  Posts: 210
  Joined: 04 Sep 2011 17:43

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



[Qemu-devel] [Bug 1035572] Re: Bug in Qemu User Mode

2012-08-11 Thread Dietmar Stölting
I just compiled the testclone new on debian Wheezy,
for to understand what is going on and name it clonemi.
Before I have always an endless loop on Raspberry Pi
because waitpid was never fullfilled.
So I commened the two waitpid lines out.
Also I enlarged the STACK_SIZE to 262144.
Enlarging of the stack does not help. But now the Raspberry Pi with
qemu-1386 -strace clonemi
showed the following output:

root@raspberrypi:/home/pi/raspidev# qemu-i386 clonemi
thread2: 0 hello2
thread1: 0 hello1
status1=0x80487ab
status2=0x42fd4788
End of clone test.

root@raspberrypi:/home/pi/raspidev# qemu-i386 -strace clonemi
1583 brk(NULL) = 0x0804a000
1583 uname(0x42fcb4ca) = 0
1583 access(/etc/ld.so.nohwcap,F_OK) = -1 errno=2 (No such file or directory)
1583 mmap2(NULL,8192,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS,-1,0) = 
0x43088000
1583 access(/etc/ld.so.preload,R_OK) = -1 errno=2 (No such file or directory)
1583 open(/etc/ld.so.cache,O_RDONLY) = 3
1583 fstat64(3,0x42fcb184) = 0
1583 mmap2(NULL,7566,PROT_READ,MAP_PRIVATE,3,0) = 0x4308a000
1583 close(3) = 0
1583 open(/lib/libc.so.6,O_RDONLY) = 3
1583 read(3,0x42fcb2b8,512) = 512
1583 fstat64(3,0x42fcb1d8) = 0
1583 mmap2(NULL,1345848,PROT_EXEC|PROT_READ,MAP_PRIVATE|MAP_DENYWRITE,3,0) = 
0x4308c000
1583 mprotect(0x431ce000,4096,PROT_NONE) = 0
1583 
mmap2(0x431cf000,12288,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_DENYWRITE|MAP_FIXED,3,0x142)
 = 0x431cf000
1583 
mmap2(0x431d2000,10552,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED,-1,0)
 = 0x431d2000
1583 close(3) = 0
1583 mmap2(NULL,4096,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS,-1,0) = 
0x431d5000
1583 set_thread_area(1123858048,1124618228,1125996224,1,0,1123858076) = 0
1583 mprotect(0x431cf000,8192,PROT_READ) = 0
1583 mprotect(0x43084000,4096,PROT_READ) = 0
1583 munmap(0x4308a000,7566) = 0
1583 mmap2(NULL,266240,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS,-1,0) = 
0x431d6000
1583 
clone(CLONE_VM|CLONE_FS|CLONE_FILES|0x11,child_stack=0x43215fe4,parent_tidptr=0x43076590,tls=0x08049a34,child_tidptr=0x42fcb788)
 = 1584
1583 mmap2(NULL,266240,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS,-1,0) = 
0x43303000
1583 
clone(CLONE_VM|CLONE_FS|CLONE_FILES|0x11,child_stack=0x43342fe4,parent_tidptr=0x43076590,tls=0x08049a34,child_tidptr=0x42fcb788)
 = 1585
1583 fstat64(1,0x42fcb09c) = 0
1583 mmap2(NULL,4096,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS,-1,0) = 
0x43344000
1583 write(1,0x43342ddc,18)thread2: 0 hello2
 = 18
1583 write(1,0x43215ddc,18)thread1: 0 hello1
 = 18
1583 nanosleep(1126260128,0,134514044,0,1809,1126260136)1583 
nanosleep(1127493024,0,134514175,0,1809,1127493032)1583 
write(1,0x43344000,18)status1=0x80487ab
 = 18
1583 write(1,0x43344000,19)status2=0x42fcb788
 = 19
1583 write(1,0x43344000,19)End of clone test.
 = 19
1583 exit_group(0)
root@raspberrypi:/home/pi/raspidev# 

You can see, that  
child_stack=0x43215fe4,parent_tidptr=0x43076590
is unequal.
Is this the mistake?

Nice to hear from you,
Dietmar

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

Title:
  Bug in Qemu User Mode

Status in QEMU:
  New

Bug description:
  Hi,
  I make an interesting discovery.
  My aim is to have a working qemu-i386 on Raspberry Pi.
  After long searching in the dark what goes wrong with ANY Qemu version for 
User Mode until today,
  I find the following: The bug must be in at least one function, that the 
program testclone
  from the testpackage for i386 in linux-user-test-0.3 calls.
  The wrong function is in the part, which enables more than one thread at the 
same time, NPTL.
  Funny, how I find this out: All the programs from the tests in 
linux-user-test-0.3 I can now run succesfull with my new builded qemu-i386 for 
Raspi.
  But the program testclone does not stop after it gives out all the right 
messages.
  The program testclone stops on my Desktop computer with Debian Wheezy 
installed.
  So, the error is not in the program testclone.
  So I make a look, what is going on there with strace. With strace you get 
informations about all the values in the working program, here testclone.
  I see, that the reason, why testclone not stops is in an infinite loop 
because of 
  while (waitpid(pid1, status1, 0) != pid1);
  while (waitpid(pid2, status2, 0) != pid2);
  at its end is never fullfilled. 
  This is the reason for the famous error message from Qemu User Mode 

  qemu: uncaught target signal 11 (Segmentation fault) - core dumped 
  Segmentation fault 

  stack1 = malloc(STACK_SIZE);
  pid1 = clone(thread1_func, stack1 + STACK_SIZE,
  CLONE_VM | CLONE_FS | CLONE_FILES | SIGCHLD, hello1);

  stack2 = malloc(STACK_SIZE);
  pid2 = clone(thread2_func, stack2 + STACK_SIZE, 
  CLONE_VM | CLONE_FS | CLONE_FILES | SIGCHLD, hello2);

  The error happens early in the program testclone. Strace says, it is because 
no childprocess at all can be found. So, some basiccalculations in those four 
lines must be done 

[Qemu-devel] [Bug 1035572] Re: Bug in Qemu User Mode

2012-08-11 Thread Dietmar Stölting
After studying the source code of testclone intensive,
I see only one possibility:
The implementation of the function [b]clone() [/b]in
Qemu User Mode must be wrong. 
This would indeed affect any configuration,
means it is NOT CPU depending,
Dietmar

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

Title:
  Bug in Qemu User Mode

Status in QEMU:
  New

Bug description:
  Hi,
  I make an interesting discovery.
  My aim is to have a working qemu-i386 on Raspberry Pi.
  After long searching in the dark what goes wrong with ANY Qemu version for 
User Mode until today,
  I find the following: The bug must be in at least one function, that the 
program testclone
  from the testpackage for i386 in linux-user-test-0.3 calls.
  The wrong function is in the part, which enables more than one thread at the 
same time, NPTL.
  Funny, how I find this out: All the programs from the tests in 
linux-user-test-0.3 I can now run succesfull with my new builded qemu-i386 for 
Raspi.
  But the program testclone does not stop after it gives out all the right 
messages.
  The program testclone stops on my Desktop computer with Debian Wheezy 
installed.
  So, the error is not in the program testclone.
  So I make a look, what is going on there with strace. With strace you get 
informations about all the values in the working program, here testclone.
  I see, that the reason, why testclone not stops is in an infinite loop 
because of 
  while (waitpid(pid1, status1, 0) != pid1);
  while (waitpid(pid2, status2, 0) != pid2);
  at its end is never fullfilled. 
  This is the reason for the famous error message from Qemu User Mode 

  qemu: uncaught target signal 11 (Segmentation fault) - core dumped 
  Segmentation fault 

  stack1 = malloc(STACK_SIZE);
  pid1 = clone(thread1_func, stack1 + STACK_SIZE,
  CLONE_VM | CLONE_FS | CLONE_FILES | SIGCHLD, hello1);

  stack2 = malloc(STACK_SIZE);
  pid2 = clone(thread2_func, stack2 + STACK_SIZE, 
  CLONE_VM | CLONE_FS | CLONE_FILES | SIGCHLD, hello2);

  The error happens early in the program testclone. Strace says, it is because 
no childprocess at all can be found. So, some basiccalculations in those four 
lines must be done wrong from Qemu.
  I think, that the adressspace for each thread is calculated wrong, or 
overlapps.
  Funny, it has nothing to do with the ARM processor. I get exact the same 
errormessages, when I run the program testclone on my desktopcompi i386 with a 
Wheezy in Qemu and then qemu-i386 testclone.
  This is a good message, because it means it is an error, that belongs at 
least to the i386 family but I think, every processor in Qemu User Mode is 
involved, so until now NPTL does not work.
  Today I make a hand by hand calculation with the source code from testclone 
and compare it with the values, that Qemu User Mode give. The handcalculated 
values should  be the same which my 
  Desktop computer with Wheezy with tesclone produces, but who knows,
  Dietmar

  PS: I hope, that this is the right source code for testclone. Any help
  is welcome:-)!

  
  Code: Select all
  #include stdlib.h
  #include stdio.h
  #include string.h
  #include signal.h
  #include unistd.h
  #include inttypes.h
  #include pthread.h
  #include sys/wait.h
  #include sched.h

  int thread1_func(void *arg)
  {
  int i;
  char buf[512];

  for(i=0;i10;i++) {
  snprintf(buf, sizeof(buf), thread1: %d %s\n, i, (char *)arg);
 write(1, buf, strlen(buf));
  usleep(100 * 1000);
  }
  return 0;
  }

  int thread2_func(void *arg)
  {
  int i;
  char buf[512];
  for(i=0;i20;i++) {
  snprintf(buf, sizeof(buf), thread2: %d %s\n, i, (char *)arg);
  write(1, buf, strlen(buf));
  usleep(120 * 1000);
  }
  return 0;
  }

  #define STACK_SIZE 16384

  void test_clone(void)
  {
  uint8_t *stack1, *stack2;
  int pid1, pid2, status1, status2;

  stack1 = malloc(STACK_SIZE);
  pid1 = clone(thread1_func, stack1 + STACK_SIZE, 
   CLONE_VM | CLONE_FS | CLONE_FILES | SIGCHLD, hello1);

  stack2 = malloc(STACK_SIZE);
  pid2 = clone(thread2_func, stack2 + STACK_SIZE, 
  CLONE_VM | CLONE_FS | CLONE_FILES | SIGCHLD, hello2);

  while (waitpid(pid1, status1, 0) != pid1);
  while (waitpid(pid2, status2, 0) != pid2);
  printf(status1=0x%x\n, status1);
  printf(status2=0x%x\n, status2);
  printf(End of clone test.\n);
  }

  int main(int argc, char **argv)
  {
  test_clone();
  return 0;
  }
  Posts: 210
  Joined: 04 Sep 2011 17:43

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



Re: [Qemu-devel] [PATCH] iov_send_recv(): Handle zero bytes case even if OS does not

2012-08-11 Thread Michael Tokarev
On 12.08.2012 01:24, Peter Maydell wrote:
 POSIX allows sendmsg() and recvmsg() to fail EMSGSIZE if passed a zero
 msg.msg_iovlen (in particular the MacOS X implementation will do this).

 Handle the case where iov_send_recv() is passed a zero byte count
 explicitly, to avoid accidentally depending on the OS to treat zero
 msg_iovlen as a no-op.

 Signed-off-by: Peter Maydell peter.mayd...@linaro.org
 ---
 This is what was causing 'make check' to fail on MacOS X.
 The other option was to declare that a zero bytecount was illegal, I guess.

I don't think sending/receiving zero bytes is a good idea in the
first place.  Which test were failed on MacOS?  Was it failing at
test-iov random I/O?

I thought I ensured that the test does not call any i/o function
with zero count argument.  Might be I was wrong, and in that
case THAT place should be fixed instead.

Can you provide a bit more details please?

The whole thing is actually interesting: this is indeed a system-
dependent corner case which should be handled in the code to make
the routine consistent.  But how to fix this is an open question
I think.  Your approach seems to be best, but we as well may
print a warning there...

Thank you!

/mjt

  iov.c | 7 +++
  1 file changed, 7 insertions(+)
 
 diff --git a/iov.c b/iov.c
 index b333061..60705c7 100644
 --- a/iov.c
 +++ b/iov.c
 @@ -146,6 +146,13 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, 
 unsigned iov_cnt,
  {
  ssize_t ret;
  unsigned si, ei;/* start and end indexes */
 +if (bytes == 0) {
 +/* Catch the do-nothing case early, as otherwise we will pass an
 + * empty iovec to sendmsg/recvmsg(), and not all implementations
 + * accept this.
 + */
 +return 0;
 +}
  
  /* Find the start position, skipping `offset' bytes:
   * first, skip all full-sized vector elements, */