Re: [Qemu-devel] [PATCH 15/18] qapi: implement support for variable argument list

2012-04-17 Thread Paolo Bonzini
Il 17/04/2012 22:42, Luiz Capitulino ha scritto:
> On Tue, 17 Apr 2012 22:26:55 +0200
> Paolo Bonzini  wrote:
> 
>> Il 17/04/2012 21:36, Luiz Capitulino ha scritto:
>>> +switch(qobject_type(obj)) {
>>> +case QTYPE_QSTRING:
>>> +qstring_append(arglist,
>>> +   qstring_get_str(qobject_to_qstring(obj)));
>>> +break;
>>
>> Does this escape commas correctly?
> 
> No, but does it have to? Does QemuOpts accept an option with a coma in it?

Yes, ",," is parsed as ",".

>> It seems much easier to use no_gen and qemu_opts_from_qdict...  Then
>> cmd_netdev_add can be
> 
> netdev_add/del is expected to be a stable interface, so we can't use no_gen.

You can have hmp_netdev_add and the no_gen qmp_netdev_add as front-ends
for the QAPI cmd_netdev_add.  I think it's fair when we have to take
into account backwards-compatibility.  The conversion gives correct
error propagation, so even though QemuOpts still leaks it's a step in
the right direction.

>>   void cmd_foo(QemuOpts *arglist, Error **errp);
> 
> Until now we're treating hmp.c like an external QMP C client, using QemuOpts
> this way will leak qemu internals to hmp.c...

True, but on the other hand it sounds strange to have QAPI clients
encoding options manually and escaping commas.

A KeyValueList (list of string->string associations) could be an
alternative, but I do think that ultimately we want to have a visitor
and remove QemuOpts altogether from net.c.  I can write a proof of
concept in a couple of weeks.  Again, we can proceed in steps.

Paolo



Re: [Qemu-devel] [PATCH v6 1/5] sockets: change inet_connect() to support nonblock socket

2012-04-17 Thread Orit Wasserman
On 04/17/2012 05:54 PM, Amos Kong wrote:
> Add a bool argument to inet_connect() to assign if set socket
> to block/nonblock, and delete original argument 'socktype'
> that is unused.
> 
> Retry to connect when following errors are got:
>   -EINTR
>   -EWOULDBLOCK (win32)
> Connect's successful for nonblock socket when following
> errors are got, user should wait for connecting by select():
>   -EINPROGRESS
>   -WSAEALREADY (win32)
> 
> Change nbd, vnc to use new interface.
> 
> Signed-off-by: Amos Kong 
> ---
>  nbd.c  |2 +-
>  qemu-sockets.c |   58 
> +++-
>  qemu_socket.h  |2 +-
>  ui/vnc.c   |2 +-
>  4 files changed, 48 insertions(+), 16 deletions(-)
> 
> diff --git a/nbd.c b/nbd.c
> index 406e555..b4e68a9 100644
> --- a/nbd.c
> +++ b/nbd.c
> @@ -146,7 +146,7 @@ int tcp_socket_outgoing(const char *address, uint16_t 
> port)
>  
>  int tcp_socket_outgoing_spec(const char *address_and_port)
>  {
> -return inet_connect(address_and_port, SOCK_STREAM);
> +return inet_connect(address_and_port, true);
>  }
>  
>  int tcp_socket_incoming(const char *address, uint16_t port)
> diff --git a/qemu-sockets.c b/qemu-sockets.c
> index 6bcb8e3..e886195 100644
> --- a/qemu-sockets.c
> +++ b/qemu-sockets.c
> @@ -51,6 +51,9 @@ static QemuOptsList dummy_opts = {
>  },{
>  .name = "ipv6",
>  .type = QEMU_OPT_BOOL,
> +},{
> +.name = "block",
> +.type = QEMU_OPT_BOOL,
>  },
>  { /* end if list */ }
>  },
> @@ -201,7 +204,8 @@ int inet_connect_opts(QemuOpts *opts)
>  const char *port;
>  char uaddr[INET6_ADDRSTRLEN+1];
>  char uport[33];
> -int sock,rc;
> +int sock, rc, err;
> +bool block;
>  
>  memset(&ai,0, sizeof(ai));
>  ai.ai_flags = AI_CANONNAME | AI_ADDRCONFIG;
> @@ -210,6 +214,7 @@ int inet_connect_opts(QemuOpts *opts)
>  
>  addr = qemu_opt_get(opts, "host");
>  port = qemu_opt_get(opts, "port");
> +block = qemu_opt_get_bool(opts, "block", 0);
>  if (addr == NULL || port == NULL) {
>  fprintf(stderr, "inet_connect: host and/or port not specified\n");
>  return -1;
> @@ -241,21 +246,44 @@ int inet_connect_opts(QemuOpts *opts)
>  continue;
>  }
>  setsockopt(sock,SOL_SOCKET,SO_REUSEADDR,(void*)&on,sizeof(on));
> -
> +if (!block) {
> +socket_set_nonblock(sock);
> +}
>  /* connect to peer */
> -if (connect(sock,e->ai_addr,e->ai_addrlen) < 0) {
> -if (NULL == e->ai_next)
> -fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n", 
> __FUNCTION__,
> -inet_strfamily(e->ai_family),
> -e->ai_canonname, uaddr, uport, strerror(errno));
> -closesocket(sock);
> -continue;
> +do {
> +err = 0;
> +if (connect(sock, e->ai_addr, e->ai_addrlen) < 0) {
> +err = -socket_error();
> +}
> +#ifndef _WIN32
> +} while (err == -EINTR || err == -EWOULDBLOCK);
> +#else
> +} while (err == -EINTR);
> +#endif

We shouldn't retry to connect for a blocking socket, please add a check for 
!block.
According to msn docs in WIN32 if we get EWOULDBLOCK , we should do select 
http://msdn.microsoft.com/en-us/library/windows/desktop/ms737625(v=vs.85).aspx
so I think we only need to retry for -EINTR.

> +
> +if (err >= 0) {
> +goto success;
> +} else if (!block && err == -EINPROGRESS) {
> +goto success;
> +#ifdef _WIN32
> +} else if (!block && err == -WSAEALREADY) {

Also EWOULDBLOCK
This is more a style comment as I feel to code doesn't need the go to.

Check for an error path so the rest of the function looks like:

if (err < 0) {
if ( block ||
#ifndef __WIN32
 err != -EINPROGRESS ) {
#else 
(err != -EWOULDBLOCK && err != -WASALREADY) ) {
#endif
if (NULL == e->ai_next) {
fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n", __FUNCTION__,
inet_strfamily(e->ai_family),
e->ai_canonname, uaddr, uport, strerror(errno));
}
closesocket(sock);
sock = -1;
}

freeaddrinfo(res);
return sock;
}

> +goto success;
> +#endif
>  }
> -freeaddrinfo(res);
> -return sock;
> +
> +if (NULL == e->ai_next) {
> +fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n", __FUNCTION__,
> +inet_strfamily(e->ai_family),
> +e->ai_canonname, uaddr, uport, strerror(errno));
> +}
> +closesocket(sock);
>  }
>  freeaddrinfo(res);
>  return -1;
> +
> +success:
> +freeaddrinfo(res);
> +return sock;
>  }
>  
>  int inet_dgram_opts(QemuOpts *opts)
> @@ -449,14 +477,18 @@ int inet_listen(const char *str, char *ostr, int olen,
>  return sock;
>  }
>  
> -int in

Re: [Qemu-devel] [PATCH v6 2/5] qerror: add five qerror strings

2012-04-17 Thread Orit Wasserman
On 04/17/2012 05:54 PM, Amos Kong wrote:
> Add five new qerror strings, they are about socket:
>   QERR_SOCKET_CONNECT_IN_PROGRESS
>   QERR_SOCKET_CONNECT_FAILED
>   QERR_SOCKET_LISTEN_FAILED
>   QERR_SOCKET_BIND_FAILED
>   QERR_SOCKET_CREATE_FAILED
> 
> Signed-off-by: Amos Kong 
> ---
>  qerror.c |   20 
>  qerror.h |   15 +++
>  2 files changed, 35 insertions(+), 0 deletions(-)
> 
> diff --git a/qerror.c b/qerror.c
> index 96fbe71..7afe1ac 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -304,6 +304,26 @@ static const QErrorStringTable qerror_table[] = {
>  .error_fmt = QERR_VNC_SERVER_FAILED,
>  .desc  = "Could not start VNC server on %(target)",
>  },
> +{
> +.error_fmt = QERR_SOCKET_CONNECT_IN_PROGRESS,
> +.desc  = "Connection cannot be completed immediately",
> +},
> +{
> +.error_fmt = QERR_SOCKET_CONNECT_FAILED,
> +.desc  = "Fail to connect socket",
> +},
> +{
> +.error_fmt = QERR_SOCKET_LISTEN_FAILED,
> +.desc  = "Fail to listen socket",
> +},
> +{
> +.error_fmt = QERR_SOCKET_BIND_FAILED,
> +.desc  = "Fail to bind socket",
> +},
> +{
> +.error_fmt = QERR_SOCKET_CREATE_FAILED,
> +.desc  = "Fail to create socket",
> +},
>  {}
>  };
>  
> diff --git a/qerror.h b/qerror.h
> index 5c23c1f..4cbba48 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -248,4 +248,19 @@ QError *qobject_to_qerror(const QObject *obj);
>  #define QERR_VNC_SERVER_FAILED \
>  "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
>  
> +#define QERR_SOCKET_CONNECT_IN_PROGRESS \
> +"{ 'class': 'SockConnectInprogress', 'data': {} }"
> +
> +#define QERR_SOCKET_CONNECT_FAILED \
> +"{ 'class': 'SockConnectFailed', 'data': {} }"
> +
> +#define QERR_SOCKET_LISTEN_FAILED \
> +"{ 'class': 'SockListenFailed', 'data': {} }"
> +
> +#define QERR_SOCKET_BIND_FAILED \
> +"{ 'class': 'SockBindFailed', 'data': {} }"
> +
> +#define QERR_SOCKET_CREATE_FAILED \
> +"{ 'class': 'SockCreateFailed', 'data': {} }"

For the FAILED error we will probably need more data , how about adding a 
string 
that can contain the strerror string ?

Orit
> +
>  #endif /* QERROR_H */
> 




[Qemu-devel] [Bug 984516] Re: should use sdl-config for static build not pkg-config

2012-04-17 Thread Stefan Weil
pkg-config supports --static, and QEMU uses it.

Please try whether

   pkg-config --libs --static sdl

gives the correct flags with your distribution. If not, that
distribution is buggy.

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

Title:
  should use sdl-config for static build not pkg-config

Status in QEMU:
  New

Bug description:
  In the configure script when a user wants to compile a static QEMU and
  enable SDL support (i.e. ./configure --static --enable-sdl):

  pkg-config does not have an option "--static-libs". For correct
  results (to find the static archive libSDL.a) you need to use sdl-
  config --static-libs.

  
  This is how I get it to work for me anyway:

  
  diff --git a/configure b/configure
  index 2d62d12..3de4c9b 100755
  --- a/configure
  +++ b/configure
  @@ -1548,7 +1548,7 @@ int main( void ) { return SDL_Init (SDL_INIT_VIDEO); }
   EOF
 sdl_cflags=`$sdlconfig --cflags 2> /dev/null`
 if test "$static" = "yes" ; then
  -sdl_libs=`$sdlconfig --static-libs 2>/dev/null`
  +sdl_libs=`${SDL_CONFIG-${cross_prefix}sdl-config} --static-libs`
 else
   sdl_libs=`$sdlconfig --libs 2> /dev/null`
 fi

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



[Qemu-devel] [Bug 984476] Re: "segmentaion" error when DMAing

2012-04-17 Thread Stefan Weil
** Changed in: qemu
   Status: New => Confirmed

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

Title:
  "segmentaion" error when DMAing

Status in QEMU:
  Confirmed

Bug description:
  When working with QEMU's PCI network card E1000 emulator, I
  accidentally put virtual addresses into the memory mapped registers
  when I should have put physical addresses. Short story is, the address
  was too large for the physical address space so when the network card
  tried to DMA the location it tossed a "segmentaion" error out to the
  console. That's right--not a "segmentation" error, but a "segmentaion"
  error. I just thought I'd let ya'll know about that little typo.

  My "qemu -version" gives "QEMU emulator version 0.15.0, Copyright (c)
  2003-2008 Fabrice Bellard" on linux version 2.6.32. I guess it might
  be an older version, dunno if the typo's still there.

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



[Qemu-devel] [Bug 984476] Re: "segmentaion" error when DMAing

2012-04-17 Thread Stefan Weil
Was it "TCP segmentaion Error"? Then it is still there.

Thanks for reporting. It will be fixed in latest QEMU.

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

Title:
  "segmentaion" error when DMAing

Status in QEMU:
  New

Bug description:
  When working with QEMU's PCI network card E1000 emulator, I
  accidentally put virtual addresses into the memory mapped registers
  when I should have put physical addresses. Short story is, the address
  was too large for the physical address space so when the network card
  tried to DMA the location it tossed a "segmentaion" error out to the
  console. That's right--not a "segmentation" error, but a "segmentaion"
  error. I just thought I'd let ya'll know about that little typo.

  My "qemu -version" gives "QEMU emulator version 0.15.0, Copyright (c)
  2003-2008 Fabrice Bellard" on linux version 2.6.32. I guess it might
  be an older version, dunno if the typo's still there.

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



Re: [Qemu-devel] [PATCH 0/7 V6] VMXNET3 paravirtual NIC device implementation

2012-04-17 Thread Gerhard Wiesinger
As already pretested this patch I got from Dmitry now I successfully 
tested it on Fedora 16 and Knoppix 6.7 (retested again successfully).


netio performance on localhost (ok):
Packet size  1k bytes:  33645 KByte/s Tx,  25279 KByte/s Rx.
Packet size  2k bytes:  45884 KByte/s Tx,  24854 KByte/s Rx.
Packet size  4k bytes:  80332 KByte/s Tx,  42068 KByte/s Rx.
Packet size  8k bytes:  117696 KByte/s Tx,  64489 KByte/s Rx.
Packet size 16k bytes:  150418 KByte/s Tx,  100288 KByte/s Rx.
Packet size 32k bytes:  182412 KByte/s Tx,  138779 KByte/s Rx.

Since hw passed also WHQL tests it is IHMO ready for commit :-)

Formal:
Tested-by: Gerhard Wiesinger 

Ciao,
Gerhard

On 17.04.2012 14:32, Dmitry Fleytman wrote:

From: Dmitry Fleytman

This set of patches implements VMWare VMXNET3 paravirtual NIC device.
The device supports of all the device features including offload capabilties,
VLANs and etc.
The device is tested on different OSes:
 Fedora 15
 Ubuntu 10.4
 Centos 6.2
 Windows 2008R2
 Windows 2008 64bit
 Windows 2008 32bit
 Windows 2003 64bit
 Windows 2003 32bit

Changes in V6:
Fixed most of problems pointed out by Michael S. Tsirkin
The only issue still open is creation of shared place
with generic network structures and functions. Currently
all generic network code introduced by VMXNET3 resides in
vmxnet_utils.c/h files. It could be moved to some shared location however
we believe it is a matter of separate refactoring as there are a lot of 
copy-pasted
definitions in almost every device and code cleanup efforts requred in order
to create truly shared codebase.

  Reported-by: Michael S. Tsirkin

Implemented suggestions by Anthony Liguori

  Reported-by: Anthony Liguori

Fixed incorrect checksum caclulation for some packets in SW offloads mode

  Reported-by: Gerhard Wiesinger

Changes in V5:
MSI-X save/load implemented in the device instead of pci bus as
suggested by Michael S. Tsirkin

  Reported-by: Michael S. Tsirkin

Patches regrouped as suggested by Paolo Bonzini

  Reported-by: Paolo Bonzini

Changes in V4:
Fixed a few problems uncovered by NETIO test suit
Assertion on failure to initialize MSI/MSI-X replaced with warning
message and fallback to Legacy/MSI respectively

  Reported-by: Gerhard Wiesinger

Various coding style adjustments and patch split-up as suggested by Anthony
Liguori

  Reported-by: Anthony Liguori

Live migration support added

Changes in V3:
Fixed crash when net device that is used as network fronted has no
virtio HDR support.
Task offloads emulation for cases when net device that is used as
network fronted has no virtio HDR support.

  Reported-by: Gerhard Wiesinger

Changes in V2:
License text changed accoring to community suggestions
Standard license header from GPLv2+ - licensed QEMU files used

Dmitry Fleytman (7):
   Adding missing flag VIRTIO_NET_HDR_F_DATA_VALID from Linux kernel
 source tree Reformatting comments according to checkpatch.pl
 requirements
   Adding utility function net_checksum_add_cont() that allows checksum
calculation of scattered data with odd chunk sizes
   Adding utility function iov_net_csum_add() for iovec checksum
 calculation Adding utility function iov_rebuild() for smart
 iovec copy
   Header with various utility functions shared by VMWARE SCSI and
 network devices
   Various utility functions used by VMWARE network devices
   Packet abstraction used by VMWARE network devices
   VMXNET3 paravirtualized device implementation Device "vmxnet3"
 added.

  Makefile.objs   |1 +
  default-configs/pci.mak |1 +
  hw/pci.h|1 +
  hw/virtio-net.h |   13 +-
  hw/vmware_utils.h   |  126 +++
  hw/vmxnet3.c| 2435 +++
  hw/vmxnet3.h|  762 +++
  hw/vmxnet_debug.h   |  121 +++
  hw/vmxnet_pkt.c |  776 +++
  hw/vmxnet_pkt.h |  311 ++
  hw/vmxnet_utils.c   |  219 +
  hw/vmxnet_utils.h   |  341 +++
  iov.c   |   53 +
  iov.h   |6 +
  net/checksum.c  |   13 +-
  net/checksum.h  |   14 +-
  16 files changed, 5180 insertions(+), 13 deletions(-)
  create mode 100644 hw/vmware_utils.h
  create mode 100644 hw/vmxnet3.c
  create mode 100644 hw/vmxnet3.h
  create mode 100644 hw/vmxnet_debug.h
  create mode 100644 hw/vmxnet_pkt.c
  create mode 100644 hw/vmxnet_pkt.h
  create mode 100644 hw/vmxnet_utils.c
  create mode 100644 hw/vmxnet_utils.h






Re: [Qemu-devel] [PATCH 4/7 V6] Header with various utility functions shared by VMWARE SCSI and network devices

2012-04-17 Thread Michael S. Tsirkin
On Tue, Apr 17, 2012 at 03:32:38PM +0300, Dmitry Fleytman wrote:
> From: Dmitry Fleytman 
> 
> Signed-off-by: Dmitry Fleytman 
> Signed-off-by: Yan Vugenfirer 

I can easily see how different vmware devices would
share some code. However:

> ---
>  hw/vmware_utils.h |  126 
> +
>  1 files changed, 126 insertions(+), 0 deletions(-)
>  create mode 100644 hw/vmware_utils.h
> 
> diff --git a/hw/vmware_utils.h b/hw/vmware_utils.h
> new file mode 100644
> index 000..0d261c0
> --- /dev/null
> +++ b/hw/vmware_utils.h
> @@ -0,0 +1,126 @@
> +/*
> + * QEMU VMWARE paravirtual devices - auxiliary code
> + *
> + * Copyright (c) 2012 Ravello Systems LTD (http://ravellosystems.com)
> + *
> + * Developed by Daynix Computing LTD (http://www.daynix.com)
> + *
> + * Authors:
> + * Dmitry Fleytman 
> + * Yan Vugenfirer 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef VMWARE_UTILS_H
> +#define VMWARE_UTILS_H
> +
> +#ifndef VMW_SHPRN
> +#define VMW_SHPRN(fmt, ...) do {} while (0)
> +#endif
> +
> +/* Shared memory access functions with byte swap support */
> +static inline void
> +vmw_shmem_read(target_phys_addr_t addr, void *buf, int len)
> +{
> +VMW_SHPRN("SHMEM r: %" PRIx64 ", len: %d to %p", addr, len, buf);
> +cpu_physical_memory_read(addr, buf, len);
> +}
> +
> +static inline void
> +vmw_shmem_write(target_phys_addr_t addr, void *buf, int len)
> +{
> +VMW_SHPRN("SHMEM w: %" PRIx64 ", len: %d to %p", addr, len, buf);
> +cpu_physical_memory_write(addr, buf, len);
> +}
> +
> +static inline void
> +vmw_shmem_rw(target_phys_addr_t addr, void *buf, int len, int is_write)
> +{
> +VMW_SHPRN("SHMEM r/w: %" PRIx64 ", len: %d (to %p), is write: %d",
> +  addr, len, buf, is_write);
> +
> +cpu_physical_memory_rw(addr, buf, len, is_write);
> +}
> +
> +static inline void
> +vmw_shmem_set(target_phys_addr_t addr, uint8 val, int len)
> +{
> +int i;
> +VMW_SHPRN("SHMEM set: %" PRIx64 ", len: %d (value 0x%X)", addr, len, 
> val);
> +
> +for (i = 0; i < len; i++) {
> +cpu_physical_memory_write(addr + i, &val, 1);
> +}
> +}
> +
> +static inline uint32_t
> +vmw_shmem_ld8(target_phys_addr_t addr)
> +{
> +uint8_t res = ldub_phys(addr);
> +VMW_SHPRN("SHMEM load8: %" PRIx64 " (value 0x%X)", addr, res);
> +return res;
> +}
> +
> +static inline void
> +vmw_shmem_st8(target_phys_addr_t addr, uint8_t value)
> +{
> +VMW_SHPRN("SHMEM store8: %" PRIx64 " (value 0x%X)", addr, value);
> +stb_phys(addr, value);
> +}
> +
> +static inline uint32_t
> +vmw_shmem_ld16(target_phys_addr_t addr)
> +{
> +uint16_t res = lduw_le_phys(addr);
> +VMW_SHPRN("SHMEM load16: %" PRIx64 " (value 0x%X)", addr, res);
> +return res;
> +}
> +
> +static inline void
> +vmw_shmem_st16(target_phys_addr_t addr, uint16_t value)
> +{
> +VMW_SHPRN("SHMEM store16: %" PRIx64 " (value 0x%X)", addr, value);
> +stw_le_phys(addr, value);
> +}
> +
> +static inline uint32_t
> +vmw_shmem_ld32(target_phys_addr_t addr)
> +{
> +uint32_t res = ldl_le_phys(addr);
> +VMW_SHPRN("SHMEM load32: %" PRIx64 " (value 0x%X)", addr, res);
> +return res;
> +}
> +
> +static inline void
> +vmw_shmem_st32(target_phys_addr_t addr, uint32_t value)
> +{
> +VMW_SHPRN("SHMEM store32: %" PRIx64 " (value 0x%X)", addr, value);
> +stl_le_phys(addr, value);
> +}
> +
> +static inline uint64_t
> +vmw_shmem_ld64(target_phys_addr_t addr)
> +{
> +uint64_t res = ldq_le_phys(addr);
> +VMW_SHPRN("SHMEM load64: %" PRIx64 " (value %" PRIx64 ")", addr, res);
> +return res;
> +}
> +
> +static inline void
> +vmw_shmem_st64(target_phys_addr_t addr, uint64_t value)
> +{
> +VMW_SHPRN("SHMEM store64: %" PRIx64 " (value %" PRIx64 ")", addr, value);
> +stq_le_phys(addr, value);
> +}
> +

Pls remove these wrappers.  These are just memory stores. Our codebase
is too large as it is without every driver wrapping all standard calls.


> +/* MACROS for simplification of operations on array-style registers */

UPPERCASE ABUSE

> +#define IS_MULTIREG_ADDR(addr, base, cnt, regsize) \
> +(((addr + 1) > (base)) && ((addr) < (base) + (cnt) * (regsize)))


Same as range_covers_byte(base, cnt * regsize, addr)?

> +
> +#define MULTIREG_IDX_BY_ADDR(addr, base, regsize)  \
> +(((addr) - (base)) / (regsize))
> +

Above two macros is all that's left. No objection but it does not say
what they do - want to add minimal documentation?
And please prefix with VMWARE_ or something.

> +#endif
> -- 
> 1.7.7.6



[Qemu-devel] [Bug 984476] Re: "segmentaion" error when DMAing

2012-04-17 Thread Rob
Yeah it was. Sorry, should have specified. Thanks!

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

Title:
  "segmentaion" error when DMAing

Status in QEMU:
  Confirmed

Bug description:
  When working with QEMU's PCI network card E1000 emulator, I
  accidentally put virtual addresses into the memory mapped registers
  when I should have put physical addresses. Short story is, the address
  was too large for the physical address space so when the network card
  tried to DMA the location it tossed a "segmentaion" error out to the
  console. That's right--not a "segmentation" error, but a "segmentaion"
  error. I just thought I'd let ya'll know about that little typo.

  My "qemu -version" gives "QEMU emulator version 0.15.0, Copyright (c)
  2003-2008 Fabrice Bellard" on linux version 2.6.32. I guess it might
  be an older version, dunno if the typo's still there.

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



Re: [Qemu-devel] [PATCH 6/7 V6] Packet abstraction used by VMWARE network devices

2012-04-17 Thread Michael S. Tsirkin
On Tue, Apr 17, 2012 at 03:32:40PM +0300, Dmitry Fleytman wrote:
> +/*=
> + 
> *=
> + *
> + *TX INTERFACE
> + *
> + 
> *=
> + 
> *===*/
> +
> +/* tx module context handle */
> +typedef void *VmxnetTxPktH;

This gets you zero type safety and makes debugging impossible.
Just use pointers like everyone does.

-- 
MST



[Qemu-devel] [PATCH] e1000: Fix spelling (segmentaion -> segmentation) in debug output

2012-04-17 Thread Stefan Weil
This was reported by https://bugs.launchpad.net/qemu/+bug/984476.

I also changed the case for 'error'.

Signed-off-by: Stefan Weil 
---
 hw/e1000.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/e1000.c b/hw/e1000.c
index 7babc0b..9c76462 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -481,7 +481,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
 } while (split_size -= bytes);
 } else if (!tp->tse && tp->cptse) {
 // context descriptor TSE is not set, while data descriptor TSE is set
-DBGOUT(TXERR, "TCP segmentaion Error\n");
+DBGOUT(TXERR, "TCP segmentation error\n");
 } else {
 split_size = MIN(sizeof(tp->data) - tp->size, split_size);
 pci_dma_read(&s->dev, addr, tp->data + tp->size, split_size);
-- 
1.7.9




Re: [Qemu-devel] [PATCH 0/3] switch to seavgabios

2012-04-17 Thread Gerhard Wiesinger

Negative also here:
Don't see anything on screen on startup...

From log, latest qemu-kvm git version:
Running option rom at c180:3d4e
Running option rom at c180:3da2
Running option rom at c180:3df6
Running option rom at c580:0003
qemu-system-x86_64: /root/download/qemu/git/qemu-kvm/exec.c:2641: 
register_subpage: Assertion `existing.mr->subpage || existing.mr == 
&io_mem_unassigned' failed.


Startup is done with the following command:
/root/download/qemu/git/qemu-kvm/x86_64-softmmu/qemu-system-x86_64
-drive file=1.img,media=disk,if=scsi,bus=0,unit=0
-drive file=2.img,media=disk,if=scsi,bus=0,unit=1
-drive file=3.img,media=disk,if=scsi,bus=0,unit=2
-boot order=cad,menu=on
-m 2048 -k de
-vga vmware -vnc :0
-bios /root/download/seabios/git/seabios/out/bios.bin -chardev 
stdio,id=seabios -device isa-debugcon,iobase=0x402,chardev=seabios

-option-rom BIOS/V4.19/8xx_64.rom
-device rtl8139,mac=00:02:44:92:87:6a,vlan=0,romfile= -net 
tap,ifname=tap0,script=no,downscript=no,vlan=0


Backtrace isn't valid.

Old integrated BIOS is valid and works well.

Ciao,
Gerhard

On 17.04.2012 12:11, Gerd Hoffmann wrote:

   Hi,

This patch series switches the vgabios binaries shipped by qemu from the
lgpl'ed vgabios to the seabios version.

There should be no guest-visible changes (especially no regressions) in
theory.  The only known (and intentional) exception is the vesa 2.0
protected mode interface which is not implemented by the seavgabios.
There are no known issues with linux and windows guests.  Testing with
other, more exotic guests is very welcome.

Note #1: Just pull from git to test, I've zapped the big binary patches
so you can't 'git am' them.

Note #2: This goes on top of the seabios-1.7.0 pull request just sent,
so pulling this will pull the seabios update too.

please give it a spin,
   Gerd

The following changes since commit 6b034aa138716a515c88f7894940d5d0aff2f3ed:

   seabios: update to 1.7.0 (2012-04-17 10:51:41 +0200)

are available in the git repository at:
   git://git.kraxel.org/qemu seavgabios-1.7.0

Gerd Hoffmann (3):
   Add seavgabios build rules to roms/Makefile
   Update vgabios binaries
   Switch isa vga to seavgabios too.

  hw/vga-isa.c   |2 +-
  hw/vga_int.h   |1 -
  pc-bios/vgabios-cirrus.bin |  Bin 35840 ->  37888 bytes
  pc-bios/vgabios-isavga.bin |  Bin 0 ->  37888 bytes
  pc-bios/vgabios-qxl.bin|  Bin 40448 ->  37888 bytes
  pc-bios/vgabios-stdvga.bin |  Bin 40448 ->  37888 bytes
  pc-bios/vgabios-vmware.bin |  Bin 40448 ->  37888 bytes
  roms/Makefile  |   13 +
  roms/config.vga.cirrus |3 +++
  roms/config.vga.isavga |3 +++
  roms/config.vga.qxl|6 ++
  roms/config.vga.stdvga |3 +++
  roms/config.vga.vmware |6 ++
  13 files changed, 35 insertions(+), 2 deletions(-)
  create mode 100644 pc-bios/vgabios-isavga.bin
  create mode 100644 roms/config.vga.cirrus
  create mode 100644 roms/config.vga.isavga
  create mode 100644 roms/config.vga.qxl
  create mode 100644 roms/config.vga.stdvga
  create mode 100644 roms/config.vga.vmware






[Qemu-devel] [PATCH] qemu kvm: Accept PCID feature

2012-04-17 Thread Mao, Junjie
This patch makes Qemu accept the PCID feature specified from configuration or 
command line options.

Signed-off-by: Mao, Junjie 
---
target-i386/cpuid.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index b9bfeaf..bc061b0 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -48,7 +48,7 @@ static const char *ext_feature_name[] = {
 "ds_cpl", "vmx", "smx", "est",
 "tm2", "ssse3", "cid", NULL,
 "fma", "cx16", "xtpr", "pdcm",
-NULL, NULL, "dca", "sse4.1|sse4_1",
+NULL, "pcid", "dca", "sse4.1|sse4_1",
 "sse4.2|sse4_2", "x2apic", "movbe", "popcnt",
 NULL, "aes", "xsave", "osxsave",
 "avx", NULL, NULL, "hypervisor",




Re: [Qemu-devel] qemu softmmu inlined lookup sequence

2012-04-17 Thread 陳韋任
On Tue, Apr 17, 2012 at 08:17:09PM +, Blue Swirl wrote:
> On Tue, Apr 17, 2012 at 05:40, Xin Tong  wrote:
> > that is possible. but if that is the case, why not split the tlb
> > walking and the tlb fill ? can anyone please confirm ?
> 
> I sent a patch earlier that did something like that but it wasn't very
> successful:
> http://lists.nongnu.org/archive/html/qemu-devel/2012-04/msg00992.html

  So functions like __ldb_mmu actually don't need to walk the TLB again?
Why you said the patch was't very successful? I don't others' comment
against the patch.

Regards,
chenwj

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



Re: [Qemu-devel] [PATCH 2/3] qtest: enable qtest for most targets

2012-04-17 Thread Max Filippov
On Wed, Apr 18, 2012 at 12:27 AM, Peter Maydell
 wrote:
> On 17 April 2012 21:19, Blue Swirl  wrote:
>> On Mon, Apr 16, 2012 at 21:58, Max Filippov  wrote:
>>> What is the use case of the default board?
>>
>> If an architecture has multiple boards, the default board can be used
>> without using -M option.
>
> Unless the Xtensa architecture has a particular board which
> is and will *always* be a primary choice for anybody using it
> (as "pc" is for x86) I'd recommend against setting a default
> board for it, because once you set one you can never change
> it without breaking command-line compatibility for users.

Ok, I got it. I'd say there's no such board. 'sim' could be the default board,
but it's useless without semihosting, and enabling semihosting by default
looks like a security issue.

-- 
Thanks.
-- Max



[Qemu-devel] [Bug 984476] [NEW] "segmentaion" error when DMAing

2012-04-17 Thread Rob
Public bug reported:

When working with QEMU's PCI network card E1000 emulator, I accidentally
put virtual addresses into the memory mapped registers when I should
have put physical addresses. Short story is, the address was too large
for the physical address space so when the network card tried to DMA the
location it tossed a "segmentaion" error out to the console. That's
right--not a "segmentation" error, but a "segmentaion" error. I just
thought I'd let ya'll know about that little typo.

My "qemu -version" gives "QEMU emulator version 0.15.0, Copyright (c)
2003-2008 Fabrice Bellard" on linux version 2.6.32. I guess it might be
an older version, dunno if the typo's still there.

** 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/984476

Title:
  "segmentaion" error when DMAing

Status in QEMU:
  New

Bug description:
  When working with QEMU's PCI network card E1000 emulator, I
  accidentally put virtual addresses into the memory mapped registers
  when I should have put physical addresses. Short story is, the address
  was too large for the physical address space so when the network card
  tried to DMA the location it tossed a "segmentaion" error out to the
  console. That's right--not a "segmentation" error, but a "segmentaion"
  error. I just thought I'd let ya'll know about that little typo.

  My "qemu -version" gives "QEMU emulator version 0.15.0, Copyright (c)
  2003-2008 Fabrice Bellard" on linux version 2.6.32. I guess it might
  be an older version, dunno if the typo's still there.

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



Re: [Qemu-devel] [PATCH v6 1/5] sockets: change inet_connect() to support nonblock socket

2012-04-17 Thread Amos Kong

On 18/04/12 10:10, Michael Roth wrote:

On Tue, Apr 17, 2012 at 10:54:01PM +0800, Amos Kong wrote:

Add a bool argument to inet_connect() to assign if set socket
to block/nonblock, and delete original argument 'socktype'
that is unused.

Retry to connect when following errors are got:
   -EINTR
   -EWOULDBLOCK (win32)
Connect's successful for nonblock socket when following
errors are got, user should wait for connecting by select():
   -EINPROGRESS
   -WSAEALREADY (win32)


How does the user access these? Via errno/socket_error()?


Yes, connect error is got by socket_error()


 I'm not sure
how that would work, I don't think we can rely on the value of errno
unless a function indicates an error, so for the case where we're
successful but have an EINPROGRESS, I don't think we have a way of
reliably obtaining that (errno value might be stale) except maybe
something like getsockopt().



I think it would be better to just merge this with your patch #3, which
handles all this nicely via Error.


We use Error to pass meaningful error, but we need to process original 
socket error,

which is got by socket_error().

In qemu, we used socket_error() to get socket error in many place. If 
you are confused
with how that would work, we need to fix socket_error. I don't want to 
fix this in the

migration patchset.

What's your opinion?


Change nbd, vnc to use new interface.

Signed-off-by: Amos Kong
---
  nbd.c  |2 +-
  qemu-sockets.c |   58 +++-
  qemu_socket.h  |2 +-
  ui/vnc.c   |2 +-
  4 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/nbd.c b/nbd.c
index 406e555..b4e68a9 100644
--- a/nbd.c
+++ b/nbd.c
@@ -146,7 +146,7 @@ int tcp_socket_outgoing(const char *address, uint16_t port)

  int tcp_socket_outgoing_spec(const char *address_and_port)
  {
-return inet_connect(address_and_port, SOCK_STREAM);
+return inet_connect(address_and_port, true);
  }

  int tcp_socket_incoming(const char *address, uint16_t port)
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 6bcb8e3..e886195 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -51,6 +51,9 @@ static QemuOptsList dummy_opts = {
  },{
  .name = "ipv6",
  .type = QEMU_OPT_BOOL,
+},{
+.name = "block",
+.type = QEMU_OPT_BOOL,
  },
  { /* end if list */ }
  },
@@ -201,7 +204,8 @@ int inet_connect_opts(QemuOpts *opts)
  const char *port;
  char uaddr[INET6_ADDRSTRLEN+1];
  char uport[33];
-int sock,rc;
+int sock, rc, err;
+bool block;

  memset(&ai,0, sizeof(ai));
  ai.ai_flags = AI_CANONNAME | AI_ADDRCONFIG;
@@ -210,6 +214,7 @@ int inet_connect_opts(QemuOpts *opts)

  addr = qemu_opt_get(opts, "host");
  port = qemu_opt_get(opts, "port");
+block = qemu_opt_get_bool(opts, "block", 0);
  if (addr == NULL || port == NULL) {
  fprintf(stderr, "inet_connect: host and/or port not specified\n");
  return -1;
@@ -241,21 +246,44 @@ int inet_connect_opts(QemuOpts *opts)
  continue;
  }
  setsockopt(sock,SOL_SOCKET,SO_REUSEADDR,(void*)&on,sizeof(on));
-
+if (!block) {
+socket_set_nonblock(sock);
+}
  /* connect to peer */
-if (connect(sock,e->ai_addr,e->ai_addrlen)<  0) {
-if (NULL == e->ai_next)
-fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n", __FUNCTION__,
-inet_strfamily(e->ai_family),
-e->ai_canonname, uaddr, uport, strerror(errno));
-closesocket(sock);
-continue;
+do {
+err = 0;
+if (connect(sock, e->ai_addr, e->ai_addrlen)<  0) {
+err = -socket_error();



connect error is got here



+}
+#ifndef _WIN32
+} while (err == -EINTR || err == -EWOULDBLOCK);
+#else
+} while (err == -EINTR);
+#endif
+
+if (err>= 0) {
+goto success;
+} else if (!block&&  err == -EINPROGRESS) {
+goto success;
+#ifdef _WIN32
+} else if (!block&&  err == -WSAEALREADY) {
+goto success;
+#endif
  }
-freeaddrinfo(res);
-return sock;
+
+if (NULL == e->ai_next) {
+fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n", __FUNCTION__,
+inet_strfamily(e->ai_family),
+e->ai_canonname, uaddr, uport, strerror(errno));
+}
+closesocket(sock);
  }
  freeaddrinfo(res);
  return -1;
+
+success:
+freeaddrinfo(res);
+return sock;
  }

  int inet_dgram_opts(QemuOpts *opts)
@@ -449,14 +477,18 @@ int inet_listen(const char *str, char *ostr, int olen,
  return sock;
  }

-int inet_connect(const char *str, int socktype)
+int inet_connect(const char *str, bool block)
  {
  QemuOpts *opts;
  int sock = -1;

  opts = qemu_opts_create(&dummy_opts, NULL, 0)

Re: [Qemu-devel] [PATCH v6 5/5] use inet_listen()/inet_connect() to support ipv6 migration

2012-04-17 Thread Amos Kong

On 18/04/12 10:17, Michael Roth wrote:

On Tue, Apr 17, 2012 at 10:54:39PM +0800, Amos Kong wrote:

Use help functions in qemu-socket.c for tcp migration,
which already support ipv6 addresses.

Currently errp will be set to UNDEFINED_ERROR when migration fails,
qemu would output "migration failed: ...", and current user can
see a message("An undefined error has occurred") in monitor.

This patch changed tcp_start_outgoing_migration()/inet_connect()
/inet_connect_opts(), socket error would be passed back,
then current user can see a meaningful err message in monitor.

Qemu will exit if listening fails, so output socket error
to qemu stderr.

For IPv6 brackets must be mandatory if you require a port.
Referencing to RFC5952, the recommended format is:
   [2312::8274]:5200

test status: Successed
listen side: qemu-kvm  -incoming tcp:[2312::8274]:5200
client side: qemu-kvm ...
  (qemu) migrate -d tcp:[2312::8274]:5200

Signed-off-by: Amos Kong
---
  migration-tcp.c |   74 +--
  migration.c |   14 ++
  migration.h |7 +++--
  vl.c|6 
  4 files changed, 35 insertions(+), 66 deletions(-)

diff --git a/migration-tcp.c b/migration-tcp.c
index 35a5781..1ecfd1e 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -79,45 +79,29 @@ static void tcp_wait_for_connect(void *opaque)
  }
  }

-int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
+int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
+ Error **errp)
  {
-struct sockaddr_in addr;
-int ret;
-
-ret = parse_host_port(&addr, host_port);
-if (ret<  0) {
-return ret;
-}
-
  s->get_error = socket_errno;
  s->write = socket_write;
  s->close = tcp_close;

-s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
-if (s->fd == -1) {
-DPRINTF("Unable to open socket");
-return -socket_error();
-}
-
-socket_set_nonblock(s->fd);
-
-do {
-ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
-if (ret == -1) {
-ret = -socket_error();
-}
-if (ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
-qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
-return 0;
-}
-} while (ret == -EINTR);
+s->fd = inet_connect(host_port, false, errp);

-if (ret<  0) {
+if (error_is_type(*errp, QERR_SOCKET_CONNECT_IN_PROGRESS)) {
+DPRINTF("connect in progress");
+qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);



In this condition(most of time), and message will be outputted to 
monitor, is this ok?

 "migrate: Connection cannot be completed immediately"


+} else if (error_is_type(*errp, QERR_SOCKET_CREATE_FAILED)) {
+DPRINTF("connect failed\n");
+return -1;


Shouldn't this be handled the same way as CONNECT_FAILED?


migrate_fd_error() is used to set migration error state,
we only set the state when connect fails.
If the socket is not create successfully (create fails
/parse issue/etc), we will not set the state, user can
retry to migrate.

This behavior is same as original.


+} else if (error_is_type(*errp, QERR_SOCKET_CONNECT_FAILED)) {
  DPRINTF("connect failed\n");
  migrate_fd_error(s);


 ^


-return ret;
+return -1;
+} else {
+migrate_fd_connect(s);
  }
-migrate_fd_connect(s);


...

--
Amos.



[Qemu-devel] [Bug 984516] [NEW] should use sdl-config for static build not pkg-config

2012-04-17 Thread Kenneth Salerno
Public bug reported:

In the configure script when a user wants to compile a static QEMU and
enable SDL support (i.e. ./configure --static --enable-sdl):

pkg-config does not have an option "--static-libs". For correct results
(to find the static archive libSDL.a) you need to use sdl-config
--static-libs.


This is how I get it to work for me anyway:


diff --git a/configure b/configure
index 2d62d12..3de4c9b 100755
--- a/configure
+++ b/configure
@@ -1548,7 +1548,7 @@ int main( void ) { return SDL_Init (SDL_INIT_VIDEO); }
 EOF
   sdl_cflags=`$sdlconfig --cflags 2> /dev/null`
   if test "$static" = "yes" ; then
-sdl_libs=`$sdlconfig --static-libs 2>/dev/null`
+sdl_libs=`${SDL_CONFIG-${cross_prefix}sdl-config} --static-libs`
   else
 sdl_libs=`$sdlconfig --libs 2> /dev/null`
   fi

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: sdl static

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

Title:
  should use sdl-config for static build not pkg-config

Status in QEMU:
  New

Bug description:
  In the configure script when a user wants to compile a static QEMU and
  enable SDL support (i.e. ./configure --static --enable-sdl):

  pkg-config does not have an option "--static-libs". For correct
  results (to find the static archive libSDL.a) you need to use sdl-
  config --static-libs.

  
  This is how I get it to work for me anyway:

  
  diff --git a/configure b/configure
  index 2d62d12..3de4c9b 100755
  --- a/configure
  +++ b/configure
  @@ -1548,7 +1548,7 @@ int main( void ) { return SDL_Init (SDL_INIT_VIDEO); }
   EOF
 sdl_cflags=`$sdlconfig --cflags 2> /dev/null`
 if test "$static" = "yes" ; then
  -sdl_libs=`$sdlconfig --static-libs 2>/dev/null`
  +sdl_libs=`${SDL_CONFIG-${cross_prefix}sdl-config} --static-libs`
 else
   sdl_libs=`$sdlconfig --libs 2> /dev/null`
 fi

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



[Qemu-devel] [Bug 984516] Re: should use sdl-config for static build not pkg-config

2012-04-17 Thread Kenneth Salerno
Sorry, I stripped out the "2>/dev/null" when I was debugging and forgot
to add it back in:


diff --git a/configure b/configure
index 2d62d12..3de4c9b 100755
--- a/configure
+++ b/configure
@@ -1548,7 +1548,7 @@ int main( void ) { return SDL_Init (SDL_INIT_VIDEO); }
 EOF
   sdl_cflags=`$sdlconfig --cflags 2> /dev/null`
   if test "$static" = "yes" ; then
- sdl_libs=`$sdlconfig --static-libs 2>/dev/null`
+ sdl_libs=`${SDL_CONFIG-${cross_prefix}sdl-config} --static-libs 2>/dev/null`
   else
 sdl_libs=`$sdlconfig --libs 2> /dev/null`
   fi

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

Title:
  should use sdl-config for static build not pkg-config

Status in QEMU:
  New

Bug description:
  In the configure script when a user wants to compile a static QEMU and
  enable SDL support (i.e. ./configure --static --enable-sdl):

  pkg-config does not have an option "--static-libs". For correct
  results (to find the static archive libSDL.a) you need to use sdl-
  config --static-libs.

  
  This is how I get it to work for me anyway:

  
  diff --git a/configure b/configure
  index 2d62d12..3de4c9b 100755
  --- a/configure
  +++ b/configure
  @@ -1548,7 +1548,7 @@ int main( void ) { return SDL_Init (SDL_INIT_VIDEO); }
   EOF
 sdl_cflags=`$sdlconfig --cflags 2> /dev/null`
 if test "$static" = "yes" ; then
  -sdl_libs=`$sdlconfig --static-libs 2>/dev/null`
  +sdl_libs=`${SDL_CONFIG-${cross_prefix}sdl-config} --static-libs`
 else
   sdl_libs=`$sdlconfig --libs 2> /dev/null`
 fi

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



Re: [Qemu-devel] [PATCH v6 5/5] use inet_listen()/inet_connect() to support ipv6 migration

2012-04-17 Thread Michael Roth
On Tue, Apr 17, 2012 at 10:54:39PM +0800, Amos Kong wrote:
> Use help functions in qemu-socket.c for tcp migration,
> which already support ipv6 addresses.
> 
> Currently errp will be set to UNDEFINED_ERROR when migration fails,
> qemu would output "migration failed: ...", and current user can
> see a message("An undefined error has occurred") in monitor.
> 
> This patch changed tcp_start_outgoing_migration()/inet_connect()
> /inet_connect_opts(), socket error would be passed back,
> then current user can see a meaningful err message in monitor.
> 
> Qemu will exit if listening fails, so output socket error
> to qemu stderr.
> 
> For IPv6 brackets must be mandatory if you require a port.
> Referencing to RFC5952, the recommended format is:
>   [2312::8274]:5200
> 
> test status: Successed
> listen side: qemu-kvm  -incoming tcp:[2312::8274]:5200
> client side: qemu-kvm ...
>  (qemu) migrate -d tcp:[2312::8274]:5200
> 
> Signed-off-by: Amos Kong 
> ---
>  migration-tcp.c |   74 
> +--
>  migration.c |   14 ++
>  migration.h |7 +++--
>  vl.c|6 
>  4 files changed, 35 insertions(+), 66 deletions(-)
> 
> diff --git a/migration-tcp.c b/migration-tcp.c
> index 35a5781..1ecfd1e 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -79,45 +79,29 @@ static void tcp_wait_for_connect(void *opaque)
>  }
>  }
> 
> -int tcp_start_outgoing_migration(MigrationState *s, const char *host_port)
> +int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
> + Error **errp)
>  {
> -struct sockaddr_in addr;
> -int ret;
> -
> -ret = parse_host_port(&addr, host_port);
> -if (ret < 0) {
> -return ret;
> -}
> -
>  s->get_error = socket_errno;
>  s->write = socket_write;
>  s->close = tcp_close;
> 
> -s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
> -if (s->fd == -1) {
> -DPRINTF("Unable to open socket");
> -return -socket_error();
> -}
> -
> -socket_set_nonblock(s->fd);
> -
> -do {
> -ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
> -if (ret == -1) {
> -ret = -socket_error();
> -}
> -if (ret == -EINPROGRESS || ret == -EWOULDBLOCK) {
> -qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
> -return 0;
> -}
> -} while (ret == -EINTR);
> +s->fd = inet_connect(host_port, false, errp);
> 
> -if (ret < 0) {
> +if (error_is_type(*errp, QERR_SOCKET_CONNECT_IN_PROGRESS)) {
> +DPRINTF("connect in progress");
> +qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
> +} else if (error_is_type(*errp, QERR_SOCKET_CREATE_FAILED)) {
> +DPRINTF("connect failed\n");
> +return -1;

Shouldn't this be handled the same way as CONNECT_FAILED?

> +} else if (error_is_type(*errp, QERR_SOCKET_CONNECT_FAILED)) {
>  DPRINTF("connect failed\n");
>  migrate_fd_error(s);
> -return ret;
> +return -1;
> +} else {
> +migrate_fd_connect(s);
>  }
> -migrate_fd_connect(s);
> +
>  return 0;
>  }
> 
> @@ -155,40 +139,18 @@ out2:
>  close(s);
>  }
> 
> -int tcp_start_incoming_migration(const char *host_port)
> +int tcp_start_incoming_migration(const char *host_port, Error **errp)
>  {
> -struct sockaddr_in addr;
> -int val;
>  int s;
> 
> -DPRINTF("Attempting to start an incoming migration\n");
> -
> -if (parse_host_port(&addr, host_port) < 0) {
> -fprintf(stderr, "invalid host/port combination: %s\n", host_port);
> -return -EINVAL;
> -}
> -
> -s = qemu_socket(PF_INET, SOCK_STREAM, 0);
> -if (s == -1) {
> -return -socket_error();
> -}
> -
> -val = 1;
> -setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (const char *)&val, sizeof(val));
> +s = inet_listen(host_port, NULL, 256, SOCK_STREAM, 0, errp);
> 
> -if (bind(s, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
> -goto err;
> -}
> -if (listen(s, 1) == -1) {
> -goto err;
> +if (s < 0) {
> +return -1;
>  }
> 
>  qemu_set_fd_handler2(s, NULL, tcp_accept_incoming_migration, NULL,
>   (void *)(intptr_t)s);
> 
>  return 0;
> -
> -err:
> -close(s);
> -return -socket_error();
>  }
> diff --git a/migration.c b/migration.c
> index 94f7839..6289bc7 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -60,13 +60,13 @@ static MigrationState *migrate_get_current(void)
>  return ¤t_migration;
>  }
> 
> -int qemu_start_incoming_migration(const char *uri)
> +int qemu_start_incoming_migration(const char *uri, Error **errp)
>  {
>  const char *p;
>  int ret;
> 
>  if (strstart(uri, "tcp:", &p))
> -ret = tcp_start_incoming_migration(p);
> +ret = tcp_start_incoming_migration(p, errp);
>  #if !defined

Re: [Qemu-devel] [PATCH v6 1/5] sockets: change inet_connect() to support nonblock socket

2012-04-17 Thread Michael Roth
On Tue, Apr 17, 2012 at 10:54:01PM +0800, Amos Kong wrote:
> Add a bool argument to inet_connect() to assign if set socket
> to block/nonblock, and delete original argument 'socktype'
> that is unused.
> 
> Retry to connect when following errors are got:
>   -EINTR
>   -EWOULDBLOCK (win32)
> Connect's successful for nonblock socket when following
> errors are got, user should wait for connecting by select():
>   -EINPROGRESS
>   -WSAEALREADY (win32)

How does the user access these? Via errno/socket_error()? I'm not sure
how that would work, I don't think we can rely on the value of errno
unless a function indicates an error, so for the case where we're
successful but have an EINPROGRESS, I don't think we have a way of
reliably obtaining that (errno value might be stale) except maybe
something like getsockopt().

I think it would be better to just merge this with your patch #3, which
handles all this nicely via Error.
> 
> Change nbd, vnc to use new interface.
> 
> Signed-off-by: Amos Kong 
> ---
>  nbd.c  |2 +-
>  qemu-sockets.c |   58 
> +++-
>  qemu_socket.h  |2 +-
>  ui/vnc.c   |2 +-
>  4 files changed, 48 insertions(+), 16 deletions(-)
> 
> diff --git a/nbd.c b/nbd.c
> index 406e555..b4e68a9 100644
> --- a/nbd.c
> +++ b/nbd.c
> @@ -146,7 +146,7 @@ int tcp_socket_outgoing(const char *address, uint16_t 
> port)
> 
>  int tcp_socket_outgoing_spec(const char *address_and_port)
>  {
> -return inet_connect(address_and_port, SOCK_STREAM);
> +return inet_connect(address_and_port, true);
>  }
> 
>  int tcp_socket_incoming(const char *address, uint16_t port)
> diff --git a/qemu-sockets.c b/qemu-sockets.c
> index 6bcb8e3..e886195 100644
> --- a/qemu-sockets.c
> +++ b/qemu-sockets.c
> @@ -51,6 +51,9 @@ static QemuOptsList dummy_opts = {
>  },{
>  .name = "ipv6",
>  .type = QEMU_OPT_BOOL,
> +},{
> +.name = "block",
> +.type = QEMU_OPT_BOOL,
>  },
>  { /* end if list */ }
>  },
> @@ -201,7 +204,8 @@ int inet_connect_opts(QemuOpts *opts)
>  const char *port;
>  char uaddr[INET6_ADDRSTRLEN+1];
>  char uport[33];
> -int sock,rc;
> +int sock, rc, err;
> +bool block;
> 
>  memset(&ai,0, sizeof(ai));
>  ai.ai_flags = AI_CANONNAME | AI_ADDRCONFIG;
> @@ -210,6 +214,7 @@ int inet_connect_opts(QemuOpts *opts)
> 
>  addr = qemu_opt_get(opts, "host");
>  port = qemu_opt_get(opts, "port");
> +block = qemu_opt_get_bool(opts, "block", 0);
>  if (addr == NULL || port == NULL) {
>  fprintf(stderr, "inet_connect: host and/or port not specified\n");
>  return -1;
> @@ -241,21 +246,44 @@ int inet_connect_opts(QemuOpts *opts)
>  continue;
>  }
>  setsockopt(sock,SOL_SOCKET,SO_REUSEADDR,(void*)&on,sizeof(on));
> -
> +if (!block) {
> +socket_set_nonblock(sock);
> +}
>  /* connect to peer */
> -if (connect(sock,e->ai_addr,e->ai_addrlen) < 0) {
> -if (NULL == e->ai_next)
> -fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n", 
> __FUNCTION__,
> -inet_strfamily(e->ai_family),
> -e->ai_canonname, uaddr, uport, strerror(errno));
> -closesocket(sock);
> -continue;
> +do {
> +err = 0;
> +if (connect(sock, e->ai_addr, e->ai_addrlen) < 0) {
> +err = -socket_error();
> +}
> +#ifndef _WIN32
> +} while (err == -EINTR || err == -EWOULDBLOCK);
> +#else
> +} while (err == -EINTR);
> +#endif
> +
> +if (err >= 0) {
> +goto success;
> +} else if (!block && err == -EINPROGRESS) {
> +goto success;
> +#ifdef _WIN32
> +} else if (!block && err == -WSAEALREADY) {
> +goto success;
> +#endif
>  }
> -freeaddrinfo(res);
> -return sock;
> +
> +if (NULL == e->ai_next) {
> +fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n", __FUNCTION__,
> +inet_strfamily(e->ai_family),
> +e->ai_canonname, uaddr, uport, strerror(errno));
> +}
> +closesocket(sock);
>  }
>  freeaddrinfo(res);
>  return -1;
> +
> +success:
> +freeaddrinfo(res);
> +return sock;
>  }
> 
>  int inet_dgram_opts(QemuOpts *opts)
> @@ -449,14 +477,18 @@ int inet_listen(const char *str, char *ostr, int olen,
>  return sock;
>  }
> 
> -int inet_connect(const char *str, int socktype)
> +int inet_connect(const char *str, bool block)
>  {
>  QemuOpts *opts;
>  int sock = -1;
> 
>  opts = qemu_opts_create(&dummy_opts, NULL, 0);
> -if (inet_parse(opts, str) == 0)
> +if (inet_parse(opts, str) == 0) {
> +if (block) {
> +qemu_opt_set(opts, "block", "on");
> +}
>  sock = inet_connect_opts(opts);
> +}
>  qemu_opt

Re: [Qemu-devel] [PATCH v6 3/5] sockets: use error class to pass connect error

2012-04-17 Thread Michael Roth
On Tue, Apr 17, 2012 at 10:54:20PM +0800, Amos Kong wrote:
> Add a new argument in inet_connect()/inet_connect_opts()
> to pass back connect error.
> 
> Change nbd, vnc to use new interface.

Looks good!

Reviewed-by: Michael Roth 

> 
> Signed-off-by: Amos Kong 
> ---
>  nbd.c  |2 +-
>  qemu-char.c|2 +-
>  qemu-sockets.c |   13 ++---
>  qemu_socket.h  |6 --
>  ui/vnc.c   |2 +-
>  5 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/nbd.c b/nbd.c
> index b4e68a9..bb71f00 100644
> --- a/nbd.c
> +++ b/nbd.c
> @@ -146,7 +146,7 @@ int tcp_socket_outgoing(const char *address, uint16_t 
> port)
> 
>  int tcp_socket_outgoing_spec(const char *address_and_port)
>  {
> -return inet_connect(address_and_port, true);
> +return inet_connect(address_and_port, true, NULL);
>  }
> 
>  int tcp_socket_incoming(const char *address, uint16_t port)
> diff --git a/qemu-char.c b/qemu-char.c
> index 74c60e1..09f990a 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2444,7 +2444,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts 
> *opts)
>  if (is_listen) {
>  fd = inet_listen_opts(opts, 0);
>  } else {
> -fd = inet_connect_opts(opts);
> +fd = inet_connect_opts_err(opts, NULL);
>  }
>  }
>  if (fd < 0) {
> diff --git a/qemu-sockets.c b/qemu-sockets.c
> index e886195..2bd87fa 100644
> --- a/qemu-sockets.c
> +++ b/qemu-sockets.c
> @@ -197,7 +197,7 @@ listen:
>  return slisten;
>  }
> 
> -int inet_connect_opts(QemuOpts *opts)
> +int inet_connect_opts(QemuOpts *opts, Error **errp)
>  {
>  struct addrinfo ai,*res,*e;
>  const char *addr;
> @@ -217,6 +217,7 @@ int inet_connect_opts(QemuOpts *opts)
>  block = qemu_opt_get_bool(opts, "block", 0);
>  if (addr == NULL || port == NULL) {
>  fprintf(stderr, "inet_connect: host and/or port not specified\n");
> +error_set(errp, QERR_SOCKET_CREATE_FAILED);
>  return -1;
>  }
> 
> @@ -229,6 +230,7 @@ int inet_connect_opts(QemuOpts *opts)
>  if (0 != (rc = getaddrinfo(addr, port, &ai, &res))) {
>  fprintf(stderr,"getaddrinfo(%s,%s): %s\n", addr, port,
>  gai_strerror(rc));
> +error_set(errp, QERR_SOCKET_CREATE_FAILED);
>   return -1;
>  }
> 
> @@ -254,6 +256,7 @@ int inet_connect_opts(QemuOpts *opts)
>  err = 0;
>  if (connect(sock, e->ai_addr, e->ai_addrlen) < 0) {
>  err = -socket_error();
> +error_set(errp, QERR_SOCKET_CONNECT_FAILED);
>  }
>  #ifndef _WIN32
>  } while (err == -EINTR || err == -EWOULDBLOCK);
> @@ -264,9 +267,11 @@ int inet_connect_opts(QemuOpts *opts)
>  if (err >= 0) {
>  goto success;
>  } else if (!block && err == -EINPROGRESS) {
> +error_set(errp, QERR_SOCKET_CONNECT_IN_PROGRESS);
>  goto success;
>  #ifdef _WIN32
>  } else if (!block && err == -WSAEALREADY) {
> +error_set(errp, QERR_SOCKET_CONNECT_IN_PROGRESS);
>  goto success;
>  #endif
>  }
> @@ -477,7 +482,7 @@ int inet_listen(const char *str, char *ostr, int olen,
>  return sock;
>  }
> 
> -int inet_connect(const char *str, bool block)
> +int inet_connect(const char *str, bool block, Error **errp)
>  {
>  QemuOpts *opts;
>  int sock = -1;
> @@ -487,7 +492,9 @@ int inet_connect(const char *str, bool block)
>  if (block) {
>  qemu_opt_set(opts, "block", "on");
>  }
> -sock = inet_connect_opts(opts);
> +sock = inet_connect_opts(opts, errp);
> +} else {
> +error_set(errp, QERR_SOCKET_CREATE_FAILED);
>  }
>  qemu_opts_del(opts);
>  return sock;
> diff --git a/qemu_socket.h b/qemu_socket.h
> index f73e26d..26998ef 100644
> --- a/qemu_socket.h
> +++ b/qemu_socket.h
> @@ -27,6 +27,8 @@ int inet_aton(const char *cp, struct in_addr *ia);
>  #endif /* !_WIN32 */
> 
>  #include "qemu-option.h"
> +#include "error.h"
> +#include "qerror.h"
> 
>  /* misc helpers */
>  int qemu_socket(int domain, int type, int protocol);
> @@ -40,8 +42,8 @@ int send_all(int fd, const void *buf, int len1);
>  int inet_listen_opts(QemuOpts *opts, int port_offset);
>  int inet_listen(const char *str, char *ostr, int olen,
>  int socktype, int port_offset);
> -int inet_connect_opts(QemuOpts *opts);
> -int inet_connect(const char *str, bool block);
> +int inet_connect_opts(QemuOpts *opts, Error **errp);
> +int inet_connect(const char *str, bool block, Error **errp);
>  int inet_dgram_opts(QemuOpts *opts);
>  const char *inet_strfamily(int family);
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 4a96153..3ae7704 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -3068,7 +3068,7 @@ int vnc_display_open(DisplayState *ds, const char 
> *display)
>  if (strncmp(display, "unix:", 5) == 0)
>  vs->lsock = unix_connect(display+5);
>  else
> -  

Re: [Qemu-devel] [PATCH v6 2/5] qerror: add five qerror strings

2012-04-17 Thread Michael Roth
On Tue, Apr 17, 2012 at 10:54:11PM +0800, Amos Kong wrote:
> Add five new qerror strings, they are about socket:
>   QERR_SOCKET_CONNECT_IN_PROGRESS
>   QERR_SOCKET_CONNECT_FAILED
>   QERR_SOCKET_LISTEN_FAILED
>   QERR_SOCKET_BIND_FAILED
>   QERR_SOCKET_CREATE_FAILED
> 
> Signed-off-by: Amos Kong 
> ---
>  qerror.c |   20 
>  qerror.h |   15 +++
>  2 files changed, 35 insertions(+), 0 deletions(-)
> 
> diff --git a/qerror.c b/qerror.c
> index 96fbe71..7afe1ac 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -304,6 +304,26 @@ static const QErrorStringTable qerror_table[] = {
>  .error_fmt = QERR_VNC_SERVER_FAILED,
>  .desc  = "Could not start VNC server on %(target)",
>  },
> +{
> +.error_fmt = QERR_SOCKET_CONNECT_IN_PROGRESS,
> +.desc  = "Connection cannot be completed immediately",
> +},
> +{
> +.error_fmt = QERR_SOCKET_CONNECT_FAILED,
> +.desc  = "Fail to connect socket",

"Failed to connect to socket"

> +},
> +{
> +.error_fmt = QERR_SOCKET_LISTEN_FAILED,
> +.desc  = "Fail to listen socket",

"Failed to set socket to listening mode"

> +},
> +{
> +.error_fmt = QERR_SOCKET_BIND_FAILED,
> +.desc  = "Fail to bind socket",

"Failed to bind socket"

> +},
> +{
> +.error_fmt = QERR_SOCKET_CREATE_FAILED,
> +.desc  = "Fail to create socket",

"Failed to create socket"

> +},
>  {}
>  };
> 
> diff --git a/qerror.h b/qerror.h
> index 5c23c1f..4cbba48 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -248,4 +248,19 @@ QError *qobject_to_qerror(const QObject *obj);
>  #define QERR_VNC_SERVER_FAILED \
>  "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
> 
> +#define QERR_SOCKET_CONNECT_IN_PROGRESS \
> +"{ 'class': 'SockConnectInprogress', 'data': {} }"
> +
> +#define QERR_SOCKET_CONNECT_FAILED \
> +"{ 'class': 'SockConnectFailed', 'data': {} }"
> +
> +#define QERR_SOCKET_LISTEN_FAILED \
> +"{ 'class': 'SockListenFailed', 'data': {} }"
> +
> +#define QERR_SOCKET_BIND_FAILED \
> +"{ 'class': 'SockBindFailed', 'data': {} }"
> +
> +#define QERR_SOCKET_CREATE_FAILED \
> +"{ 'class': 'SockCreateFailed', 'data': {} }"
> +
>  #endif /* QERROR_H */
> 
> 



[Qemu-devel] qemu physical address

2012-04-17 Thread Xin Tong
I am reading how qemu refill TLB working.

target-i386/helper.c

pte = pte & env->a20_mask;

/* Even if 4MB pages, we map only one 4KB page in the cache to
   avoid filling it too fast */
page_offset = (addr & TARGET_PAGE_MASK) & (page_size - 1);
paddr = (pte & TARGET_PAGE_MASK) + page_offset;
vaddr = virt_addr + page_offset;


How can the paddr be bigger than 4G even though i gave the machine
4096 MB of memory ( i.e. qemu -m 4096 ...). should not paddr be within
0 - 4G-1 ?

Thanks

Xin



[Qemu-devel] [PATCH 11/15] target-i386: Add property getter for CPU model-id

2012-04-17 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   17 -
 1 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0c98fcc..cc4f566 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -711,6 +711,21 @@ static void x86_cpuid_version_set_stepping(Object *obj, 
Visitor *v,
 env->cpuid_version |= value & 0xf;
 }
 
+static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+char *value;
+int i;
+
+value = g_malloc(48 + 1);
+for (i = 0; i < 48; i++) {
+value[i] = env->cpuid_model[i >> 2] >> (8 * (i & 3));
+}
+value[48] = '\0';
+return value;
+}
+
 static void x86_cpuid_set_model_id(Object *obj, const char *model_id,
Error **errp)
 {
@@ -1586,7 +1601,7 @@ static void x86_cpu_initfn(Object *obj)
 x86_cpuid_version_get_stepping,
 x86_cpuid_version_set_stepping, NULL, NULL, NULL);
 object_property_add_str(obj, "model-id",
-NULL,
+x86_cpuid_get_model_id,
 x86_cpuid_set_model_id, NULL);
 
 env->cpuid_apic_id = env->cpu_index;
-- 
1.7.7




Re: [Qemu-devel] [PATCH 5/7 V6] Various utility functions used by VMWARE network devices

2012-04-17 Thread Michael S. Tsirkin
On Tue, Apr 17, 2012 at 03:32:39PM +0300, Dmitry Fleytman wrote:
> diff --git a/hw/vmxnet_utils.h b/hw/vmxnet_utils.h
> new file mode 100644
> index 000..18d218d
> --- /dev/null
> +++ b/hw/vmxnet_utils.h
> @@ -0,0 +1,341 @@
> +/*
> + * QEMU VMWARE VMXNET* paravirtual NICs - network auxiliary code
> + *
> + * Copyright (c) 2012 Ravello Systems LTD (http://ravellosystems.com)
> + *
> + * Developed by Daynix Computing LTD (http://www.daynix.com)
> + *
> + * Authors:
> + * Dmitry Fleytman 
> + * Tamir Shomer 
> + * Yan Vugenfirer 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef _VMXNET_UTILS_H_
> +#define _VMXNET_UTILS_H_

Please do not start identifiers with _ followed by an uppercase
lattters.

> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct eth_header {
> +uint8_t  h_dest[ETH_ALEN];   /* destination eth addr */
> +uint8_t  h_source[ETH_ALEN]; /* source ether addr*/
> +uint16_t h_proto;/* packet type ID field */
> +};
> +

And fix struct definitions to
1. follow qemu coding style
2. start with vmxnet

I also don't really understand why are these
functions split out - vmxnet is the only user, no?




[Qemu-devel] [PATCH 05/15] target-i386: Add "model" property to X86CPU

2012-04-17 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   26 +++---
 1 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index aa0d328..f4eb24c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -626,10 +626,27 @@ static void x86_cpuid_version_set_family(Object *obj, 
Visitor *v, void *opaque,
 }
 }
 
-static void x86_cpuid_version_set_model(CPUX86State *env, int model)
+static void x86_cpuid_version_set_model(Object *obj, Visitor *v, void *opaque,
+const char *name, Error **errp)
 {
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+const int64_t min = 0;
+const int64_t max = 0xff;
+int64_t value;
+
+visit_type_int(v, &value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+if (value < min || value > max) {
+error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
+  name ? name : "null", value, min, max);
+return;
+}
+
 env->cpuid_version &= ~0xf00f0;
-env->cpuid_version |= ((model & 0xf) << 4) | ((model >> 4) << 16);
+env->cpuid_version |= ((value & 0xf) << 4) | ((value >> 4) << 16);
 }
 
 static void x86_cpuid_version_set_stepping(CPUX86State *env, int stepping)
@@ -948,7 +965,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 env->cpuid_vendor_override = def->vendor_override;
 env->cpuid_level = def->level;
 object_property_set_int(OBJECT(cpu), def->family, "family", &error);
-x86_cpuid_version_set_model(env, def->model);
+object_property_set_int(OBJECT(cpu), def->model, "model", &error);
 x86_cpuid_version_set_stepping(env, def->stepping);
 env->cpuid_features = def->features;
 env->cpuid_ext_features = def->ext_features;
@@ -1503,6 +1520,9 @@ static void x86_cpu_initfn(Object *obj)
 object_property_add(obj, "family", "int",
 NULL,
 x86_cpuid_version_set_family, NULL, NULL, NULL);
+object_property_add(obj, "model", "int",
+NULL,
+x86_cpuid_version_set_model, NULL, NULL, NULL);
 
 env->cpuid_apic_id = env->cpu_index;
 mce_init(cpu);
-- 
1.7.7




[Qemu-devel] [PATCH 14/15] target-i386: Prepare "vendor" property for X86CPU

2012-04-17 Thread Andreas Färber
Using it now would incur converting the three x86_def_t vendor words
into a string for object_property_set_str(), then back to three words
in the "vendor" setter.
The built-in CPU definitions use numeric preprocessor defines to
initialize the three words in a charset-safe way, so do not change the
fields to char[48] just to use the setter.

Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   44 
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 2785690..1ece541 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -777,6 +777,47 @@ static void x86_cpuid_set_xlevel(Object *obj, Visitor *v, 
void *opaque,
 cpu->env.cpuid_xlevel = value;
 }
 
+static char *x86_cpuid_get_vendor(Object *obj, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+char *value;
+int i;
+
+value = (char *)g_malloc(12 + 1);
+for (i = 0; i < 4; i++) {
+value[i] = env->cpuid_vendor1 >> (8 * i);
+value[i + 4] = env->cpuid_vendor2 >> (8 * i);
+value[i + 8] = env->cpuid_vendor3 >> (8 * i);
+}
+value[12] = '\0';
+return value;
+}
+
+static void x86_cpuid_set_vendor(Object *obj, const char *value,
+ Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+int i;
+
+if (strlen(value) != 12) {
+error_set(errp, QERR_PROPERTY_VALUE_BAD, "",
+  "vendor", value);
+return;
+}
+
+env->cpuid_vendor1 = 0;
+env->cpuid_vendor2 = 0;
+env->cpuid_vendor3 = 0;
+for (i = 0; i < 4; i++) {
+env->cpuid_vendor1 |= ((uint8_t)value[i]) << (8 * i);
+env->cpuid_vendor2 |= ((uint8_t)value[i + 4]) << (8 * i);
+env->cpuid_vendor3 |= ((uint8_t)value[i + 8]) << (8 * i);
+}
+env->cpuid_vendor_override = 1;
+}
+
 static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
@@ -1672,6 +1713,9 @@ static void x86_cpu_initfn(Object *obj)
 object_property_add(obj, "xlevel", "int",
 x86_cpuid_get_xlevel,
 x86_cpuid_set_xlevel, NULL, NULL, NULL);
+object_property_add_str(obj, "vendor",
+x86_cpuid_get_vendor,
+x86_cpuid_set_vendor, NULL);
 object_property_add_str(obj, "model-id",
 x86_cpuid_get_model_id,
 x86_cpuid_set_model_id, NULL);
-- 
1.7.7




[Qemu-devel] [PATCH 06/15] target-i386: Add "stepping" property to X86CPU

2012-04-17 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   27 ---
 1 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index f4eb24c..2e62a6f 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -649,10 +649,28 @@ static void x86_cpuid_version_set_model(Object *obj, 
Visitor *v, void *opaque,
 env->cpuid_version |= ((value & 0xf) << 4) | ((value >> 4) << 16);
 }
 
-static void x86_cpuid_version_set_stepping(CPUX86State *env, int stepping)
+static void x86_cpuid_version_set_stepping(Object *obj, Visitor *v,
+   void *opaque, const char *name,
+   Error **errp)
 {
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+const int64_t min = 0;
+const int64_t max = 0xf;
+int64_t value;
+
+visit_type_int(v, &value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+if (value < min || value > max) {
+error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
+  name ? name : "null", value, min, max);
+return;
+}
+
 env->cpuid_version &= ~0xf;
-env->cpuid_version |= stepping & 0xf;
+env->cpuid_version |= value & 0xf;
 }
 
 static void x86_cpuid_set_model_id(CPUX86State *env, const char *model_id)
@@ -966,7 +984,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 env->cpuid_level = def->level;
 object_property_set_int(OBJECT(cpu), def->family, "family", &error);
 object_property_set_int(OBJECT(cpu), def->model, "model", &error);
-x86_cpuid_version_set_stepping(env, def->stepping);
+object_property_set_int(OBJECT(cpu), def->stepping, "stepping", &error);
 env->cpuid_features = def->features;
 env->cpuid_ext_features = def->ext_features;
 env->cpuid_ext2_features = def->ext2_features;
@@ -1523,6 +1541,9 @@ static void x86_cpu_initfn(Object *obj)
 object_property_add(obj, "model", "int",
 NULL,
 x86_cpuid_version_set_model, NULL, NULL, NULL);
+object_property_add(obj, "stepping", "int",
+NULL,
+x86_cpuid_version_set_stepping, NULL, NULL, NULL);
 
 env->cpuid_apic_id = env->cpu_index;
 mce_init(cpu);
-- 
1.7.7




[Qemu-devel] [PATCH 03/15] target-i386: Add range check for -cpu , family=x

2012-04-17 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e95a1d8..d30185b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -693,7 +693,7 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 if (!strcmp(featurestr, "family")) {
 char *err;
 numvalue = strtoul(val, &err, 0);
-if (!*val || *err) {
+if (!*val || *err || numvalue > 0xff + 0xf) {
 fprintf(stderr, "bad numerical value %s\n", val);
 goto error;
 }
-- 
1.7.7




[Qemu-devel] [PATCH 10/15] target-i386: Add property getter for CPU stepping

2012-04-17 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   14 +-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 2beb3ab..0c98fcc 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -675,6 +675,18 @@ static void x86_cpuid_version_set_model(Object *obj, 
Visitor *v, void *opaque,
 env->cpuid_version |= ((value & 0xf) << 4) | ((value >> 4) << 16);
 }
 
+static void x86_cpuid_version_get_stepping(Object *obj, Visitor *v,
+   void *opaque, const char *name,
+   Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+int64_t value;
+
+value = env->cpuid_version & 0xf;
+visit_type_int(v, &value, name, errp);
+}
+
 static void x86_cpuid_version_set_stepping(Object *obj, Visitor *v,
void *opaque, const char *name,
Error **errp)
@@ -1571,7 +1583,7 @@ static void x86_cpu_initfn(Object *obj)
 x86_cpuid_version_get_model,
 x86_cpuid_version_set_model, NULL, NULL, NULL);
 object_property_add(obj, "stepping", "int",
-NULL,
+x86_cpuid_version_get_stepping,
 x86_cpuid_version_set_stepping, NULL, NULL, NULL);
 object_property_add_str(obj, "model-id",
 NULL,
-- 
1.7.7




[Qemu-devel] [PATCH 12/15] target-i386: Introduce "level" property for X86CPU

2012-04-17 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   38 +-
 1 files changed, 37 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index cc4f566..c9d5be5 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -711,6 +711,39 @@ static void x86_cpuid_version_set_stepping(Object *obj, 
Visitor *v,
 env->cpuid_version |= value & 0xf;
 }
 
+static void x86_cpuid_get_level(Object *obj, Visitor *v, void *opaque,
+const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+int64_t value;
+
+value = cpu->env.cpuid_level;
+/* TODO Use visit_type_uint32() once available */
+visit_type_int(v, &value, name, errp);
+}
+
+static void x86_cpuid_set_level(Object *obj, Visitor *v, void *opaque,
+const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+const int64_t min = 0;
+const int64_t max = UINT32_MAX;
+int64_t value;
+
+/* TODO Use visit_type_uint32() once available */
+visit_type_int(v, &value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+if (value < min || value > max) {
+error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
+  name ? name : "null", value, min, max);
+return;
+}
+
+cpu->env.cpuid_level = value;
+}
+
 static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
@@ -1037,7 +1070,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 env->cpuid_vendor3 = CPUID_VENDOR_INTEL_3;
 }
 env->cpuid_vendor_override = def->vendor_override;
-env->cpuid_level = def->level;
+object_property_set_int(OBJECT(cpu), def->level, "level", &error);
 object_property_set_int(OBJECT(cpu), def->family, "family", &error);
 object_property_set_int(OBJECT(cpu), def->model, "model", &error);
 object_property_set_int(OBJECT(cpu), def->stepping, "stepping", &error);
@@ -1600,6 +1633,9 @@ static void x86_cpu_initfn(Object *obj)
 object_property_add(obj, "stepping", "int",
 x86_cpuid_version_get_stepping,
 x86_cpuid_version_set_stepping, NULL, NULL, NULL);
+object_property_add(obj, "level", "int",
+x86_cpuid_get_level,
+x86_cpuid_set_level, NULL, NULL, NULL);
 object_property_add_str(obj, "model-id",
 x86_cpuid_get_model_id,
 x86_cpuid_set_model_id, NULL);
-- 
1.7.7




[Qemu-devel] [PATCH 02/15] target-i386: Pass X86CPU to cpu_x86_register()

2012-04-17 Thread Andreas Färber
Avoids an x86_env_get_cpu() call there, to work with QOM properties.

Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c|3 ++-
 target-i386/cpu.h|2 +-
 target-i386/helper.c |2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 80c1ca5..e95a1d8 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -907,8 +907,9 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, 
const char *optarg)
 }
 }
 
-int cpu_x86_register (CPUX86State *env, const char *cpu_model)
+int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 {
+CPUX86State *env = &cpu->env;
 x86_def_t def1, *def = &def1;
 
 memset(def, 0, sizeof(*def));
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 4bb4592..b5b9a50 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -901,7 +901,7 @@ int cpu_x86_signal_handler(int host_signum, void *pinfo,
 void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
uint32_t *eax, uint32_t *ebx,
uint32_t *ecx, uint32_t *edx);
-int cpu_x86_register (CPUX86State *env, const char *cpu_model);
+int cpu_x86_register(X86CPU *cpu, const char *cpu_model);
 void cpu_clear_apic_feature(CPUX86State *env);
 void host_cpuid(uint32_t function, uint32_t count,
 uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 87954f0..0b22582 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1176,7 +1176,7 @@ CPUX86State *cpu_x86_init(const char *cpu_model)
 cpu_set_debug_excp_handler(breakpoint_handler);
 #endif
 }
-if (cpu_x86_register(env, cpu_model) < 0) {
+if (cpu_x86_register(cpu, cpu_model) < 0) {
 object_delete(OBJECT(cpu));
 return NULL;
 }
-- 
1.7.7




[Qemu-devel] [PATCH 13/15] target-i386: Introduce "xlevel" property for X86CPU

2012-04-17 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   38 +-
 1 files changed, 37 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index c9d5be5..2785690 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -744,6 +744,39 @@ static void x86_cpuid_set_level(Object *obj, Visitor *v, 
void *opaque,
 cpu->env.cpuid_level = value;
 }
 
+static void x86_cpuid_get_xlevel(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+int64_t value;
+
+value = cpu->env.cpuid_xlevel;
+/* TODO Use visit_type_uint32() once available */
+visit_type_int(v, &value, name, errp);
+}
+
+static void x86_cpuid_set_xlevel(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+const int64_t min = 0;
+const int64_t max = UINT32_MAX;
+int64_t value;
+
+/* TODO Use visit_type_uint32() once available */
+visit_type_int(v, &value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+if (value < min || value > max) {
+error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
+  name ? name : "null", value, min, max);
+return;
+}
+
+cpu->env.cpuid_xlevel = value;
+}
+
 static char *x86_cpuid_get_model_id(Object *obj, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
@@ -1078,7 +,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 env->cpuid_ext_features = def->ext_features;
 env->cpuid_ext2_features = def->ext2_features;
 env->cpuid_ext3_features = def->ext3_features;
-env->cpuid_xlevel = def->xlevel;
+object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", &error);
 env->cpuid_kvm_features = def->kvm_features;
 env->cpuid_svm_features = def->svm_features;
 env->cpuid_ext4_features = def->ext4_features;
@@ -1636,6 +1669,9 @@ static void x86_cpu_initfn(Object *obj)
 object_property_add(obj, "level", "int",
 x86_cpuid_get_level,
 x86_cpuid_set_level, NULL, NULL, NULL);
+object_property_add(obj, "xlevel", "int",
+x86_cpuid_get_xlevel,
+x86_cpuid_set_xlevel, NULL, NULL, NULL);
 object_property_add_str(obj, "model-id",
 x86_cpuid_get_model_id,
 x86_cpuid_set_model_id, NULL);
-- 
1.7.7




[Qemu-devel] [PATCH 08/15] target-i386: Add property getter for CPU family

2012-04-17 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   16 +++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 78cb568..21041b5 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -599,6 +599,20 @@ static int check_features_against_host(x86_def_t 
*guest_def)
 return rv;
 }
 
+static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+int64_t value;
+
+value = (env->cpuid_version >> 8) & 0xf;
+if (value == 0xf) {
+value += (env->cpuid_version >> 20) & 0xff;
+}
+visit_type_int(v, &value, name, errp);
+}
+
 static void x86_cpuid_version_set_family(Object *obj, Visitor *v, void *opaque,
  const char *name, Error **errp)
 {
@@ -1539,7 +1553,7 @@ static void x86_cpu_initfn(Object *obj)
 cpu_exec_init(env);
 
 object_property_add(obj, "family", "int",
-NULL,
+x86_cpuid_version_get_family,
 x86_cpuid_version_set_family, NULL, NULL, NULL);
 object_property_add(obj, "model", "int",
 NULL,
-- 
1.7.7




[Qemu-devel] [PATCH 15/15] target-i386: Introduce "tsc-frequency" property for X86CPU

2012-04-17 Thread Andreas Färber
Use Hz as unit.

Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   37 -
 1 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1ece541..f26253f 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -857,6 +857,37 @@ static void x86_cpuid_set_model_id(Object *obj, const char 
*model_id,
 }
 }
 
+static void x86_cpuid_get_tsc_freq(Object *obj, Visitor *v, void *opaque,
+   const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+int64_t value;
+
+value = cpu->env.tsc_khz * 1000;
+visit_type_int(v, &value, name, errp);
+}
+
+static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
+   const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+const int64_t min = 0;
+const int64_t max = INT_MAX;
+int64_t value;
+
+visit_type_int(v, &value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+if (value < min || value > max) {
+error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
+  name ? name : "null", value, min, max);
+return;
+}
+
+cpu->env.tsc_khz = value / 1000;
+}
+
 static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
 {
 unsigned int i;
@@ -1157,7 +1188,8 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 env->cpuid_svm_features = def->svm_features;
 env->cpuid_ext4_features = def->ext4_features;
 env->cpuid_xlevel2 = def->xlevel2;
-env->tsc_khz = def->tsc_khz;
+object_property_set_int(OBJECT(cpu), (int64_t)def->tsc_khz * 1000,
+"tsc-frequency", &error);
 if (!kvm_enabled()) {
 env->cpuid_features &= TCG_FEATURES;
 env->cpuid_ext_features &= TCG_EXT_FEATURES;
@@ -1719,6 +1751,9 @@ static void x86_cpu_initfn(Object *obj)
 object_property_add_str(obj, "model-id",
 x86_cpuid_get_model_id,
 x86_cpuid_set_model_id, NULL);
+object_property_add(obj, "tsc-frequency", "int",
+x86_cpuid_get_tsc_freq,
+x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
 
 env->cpuid_apic_id = env->cpu_index;
 mce_init(cpu);
-- 
1.7.7




[Qemu-devel] [PATCH 04/15] target-i386: Add "family" property to X86CPU

2012-04-17 Thread Andreas Färber
Add the property early in the initfn so that it can be used in helpers
such as mce_init().

Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   38 +-
 1 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index d30185b..aa0d328 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -27,6 +27,8 @@
 #include "qemu-option.h"
 #include "qemu-config.h"
 
+#include "qapi/qapi-visit-core.h"
+
 #include "hyperv.h"
 
 /* feature flags taken from "Intel Processor Identification and the CPUID
@@ -597,13 +599,30 @@ static int check_features_against_host(x86_def_t 
*guest_def)
 return rv;
 }
 
-static void x86_cpuid_version_set_family(CPUX86State *env, int family)
+static void x86_cpuid_version_set_family(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
 {
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+const int64_t min = 0;
+const int64_t max = 0xff + 0xf;
+int64_t value;
+
+visit_type_int(v, &value, name, errp);
+if (error_is_set(errp)) {
+return;
+}
+if (value < min || value > max) {
+error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
+  name ? name : "null", value, min, max);
+return;
+}
+
 env->cpuid_version &= ~0xff00f00;
-if (family > 0x0f) {
-env->cpuid_version |= 0xf00 | ((family - 0x0f) << 20);
+if (value > 0x0f) {
+env->cpuid_version |= 0xf00 | ((value - 0x0f) << 20);
 } else {
-env->cpuid_version |= family << 8;
+env->cpuid_version |= value << 8;
 }
 }
 
@@ -911,6 +930,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 {
 CPUX86State *env = &cpu->env;
 x86_def_t def1, *def = &def1;
+Error *error = NULL;
 
 memset(def, 0, sizeof(*def));
 
@@ -927,7 +947,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 }
 env->cpuid_vendor_override = def->vendor_override;
 env->cpuid_level = def->level;
-x86_cpuid_version_set_family(env, def->family);
+object_property_set_int(OBJECT(cpu), def->family, "family", &error);
 x86_cpuid_version_set_model(env, def->model);
 x86_cpuid_version_set_stepping(env, def->stepping);
 env->cpuid_features = def->features;
@@ -952,6 +972,9 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 env->cpuid_svm_features &= TCG_SVM_FEATURES;
 }
 x86_cpuid_set_model_id(env, def->model_id);
+if (error_is_set(&error)) {
+return -1;
+}
 return 0;
 }
 
@@ -1476,6 +1499,11 @@ static void x86_cpu_initfn(Object *obj)
 CPUX86State *env = &cpu->env;
 
 cpu_exec_init(env);
+
+object_property_add(obj, "family", "int",
+NULL,
+x86_cpuid_version_set_family, NULL, NULL, NULL);
+
 env->cpuid_apic_id = env->cpu_index;
 mce_init(cpu);
 }
-- 
1.7.7




[Qemu-devel] [PATCH 00/15] QOM'ify x86 CPU, part 2: properties

2012-04-17 Thread Andreas Färber
Hello,

This series introduces some QOM properties for X86CPU, so that our built-in
init code exercises the same code paths as QMP, as suggested by Eduardo:
* "family",
* "model",
* "stepping" and
* "model-id" (rather than "model_id")
This QOM'ifies my previously introduced helper functions, adding getters.

In the same spirit I've also introduced numeric QOM properties for:
* "level"
* "xlevel"
* "tsc-frequency" (rather than "tsc_freq")
Being uint32_t, "level" and "xlevel" would benefit from fixed-width visitors,
as introduced in Michael's series. It seems his v4 was neither applied
nor commented on and only some parts were integrated into Paolo's large
rush-rush series... Would be nice to see Michael's full series merged soon!

Further I've prepared one QOM property that's currently unused:
* "vendor" (converting three words to string and back seemed too much overhead)

By constrast, the HyperV -cpu property "hv_spinlocks" and flags "hv_relaxed"
and "hv_vapic" do not seem to be per-CPU properties.

Note that these properties are still somewhat orthogonal to the feature flags
that Jinsong and Jan were discussing for machine compatibility IIUC. I'm hoping
that some x86 guru can come up with a sensible follow-up for that. :-)
Maybe bool properties per feature on an as-needed basis?

Available from:
git://github.com/afaerber/qemu-cpu.git qom-cpu-x86-prop.v1
https://github.com/afaerber/qemu-cpu/commits/qom-cpu-x86-prop.v1

Regards,
Andreas

Cc: Anthony Liguori 
Cc: Jan Kiszka 
Cc: Igor Mammedov 
Cc: Liu Jinsong 
Cc: Lai Jiangshan 
Cc: Vasilis Liaskovitis 
Cc: Eduardo Habkost 
Cc: Michael Roth 
Cc: Paolo Bonzini 
Cc: Vadim Rozenfeld 

Andreas Färber (15):
  target-i386: Fix x86_cpuid_set_model_id()
  target-i386: Pass X86CPU to cpu_x86_register()
  target-i386: Add range check for -cpu ,family=x
  target-i386: Add "family" property to X86CPU
  target-i386: Add "model" property to X86CPU
  target-i386: Add "stepping" property to X86CPU
  target-i386: Add "model-id" property to X86CPU
  target-i386: Add property getter for CPU family
  target-i386: Add property getter for CPU model
  target-i386: Add property getter for CPU stepping
  target-i386: Add property getter for CPU model-id
  target-i386: Introduce "level" property for X86CPU
  target-i386: Introduce "xlevel" property for X86CPU
  target-i386: Prepare "vendor" property for X86CPU
  target-i386: Introduce "tsc-frequency" property for X86CPU

 target-i386/cpu.c|  319 +++---
 target-i386/cpu.h|2 +-
 target-i386/helper.c |2 +-
 3 files changed, 303 insertions(+), 20 deletions(-)

-- 
1.7.7




Re: [Qemu-devel] [PATCH 14/15] target-i386: Prepare "vendor" property for X86CPU

2012-04-17 Thread Andreas Färber
Am 18.04.2012 01:11, schrieb Andreas Färber:
> Using it now would incur converting the three x86_def_t vendor words
> into a string for object_property_set_str(), then back to three words
> in the "vendor" setter.
> The built-in CPU definitions use numeric preprocessor defines to
> initialize the three words in a charset-safe way, so do not change the
> fields to char[48] just to use the setter.

Er, should've been char[12] - fixed on qom-cpu-x86 branch.

/-F

> 
> Signed-off-by: Andreas Färber 
> ---
>  target-i386/cpu.c |   44 
>  1 files changed, 44 insertions(+), 0 deletions(-)

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



Re: [Qemu-devel] [PATCH 2/3] qtest: enable qtest for most targets

2012-04-17 Thread Andreas Färber
Am 17.04.2012 22:51, schrieb Blue Swirl:
> On Tue, Apr 17, 2012 at 20:35, Peter Maydell  wrote:
>> On 17 April 2012 21:31, Blue Swirl  wrote:
>>> On Tue, Apr 17, 2012 at 20:27, Peter Maydell  
>>> wrote:
 ARM has a default board set, which is unfortunate because it's
 hardware that nobody uses now, so the QEMU default setting
 mostly serves to trip up users who try to run a kernel for
 some other machine on it by mistake.
>>>
>>> Then change it.
>>
>> As I say, I don't want to break command line compatibility.
> 
> v1.1: print a warning about using deprecated board, suggest -M integratorcp
> v1.2: change default board, when default board is used, print a notice
> that it's now -M z2, suggest -M integratorcp for old geezers
> v1.3: remove notice

That assumes end users actually launch it from the command line, as
opposed to some Eclipse UI (e.g., Sourcery CodeBench).

Anyhow, we currently don't have a way to detect whether we're using a
board because it's the default or due to an explicit -M argument.

Andreas

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



[Qemu-devel] [PATCH 09/15] target-i386: Add property getter for CPU model

2012-04-17 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   14 +-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 21041b5..2beb3ab 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -640,6 +640,18 @@ static void x86_cpuid_version_set_family(Object *obj, 
Visitor *v, void *opaque,
 }
 }
 
+static void x86_cpuid_version_get_model(Object *obj, Visitor *v, void *opaque,
+const char *name, Error **errp)
+{
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
+int64_t value;
+
+value = (env->cpuid_version >> 4) & 0xf;
+value |= (env->cpuid_version >> 16) & 0xf;
+visit_type_int(v, &value, name, errp);
+}
+
 static void x86_cpuid_version_set_model(Object *obj, Visitor *v, void *opaque,
 const char *name, Error **errp)
 {
@@ -1556,7 +1568,7 @@ static void x86_cpu_initfn(Object *obj)
 x86_cpuid_version_get_family,
 x86_cpuid_version_set_family, NULL, NULL, NULL);
 object_property_add(obj, "model", "int",
-NULL,
+x86_cpuid_version_get_model,
 x86_cpuid_version_set_model, NULL, NULL, NULL);
 object_property_add(obj, "stepping", "int",
 NULL,
-- 
1.7.7




[Qemu-devel] [PATCH 07/15] target-i386: Add "model-id" property to X86CPU

2012-04-17 Thread Andreas Färber
Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   10 --
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 2e62a6f..78cb568 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -673,8 +673,11 @@ static void x86_cpuid_version_set_stepping(Object *obj, 
Visitor *v,
 env->cpuid_version |= value & 0xf;
 }
 
-static void x86_cpuid_set_model_id(CPUX86State *env, const char *model_id)
+static void x86_cpuid_set_model_id(Object *obj, const char *model_id,
+   Error **errp)
 {
+X86CPU *cpu = X86_CPU(obj);
+CPUX86State *env = &cpu->env;
 int c, len, i;
 
 if (model_id == NULL) {
@@ -1006,7 +1009,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 env->cpuid_ext3_features &= TCG_EXT3_FEATURES;
 env->cpuid_svm_features &= TCG_SVM_FEATURES;
 }
-x86_cpuid_set_model_id(env, def->model_id);
+object_property_set_str(OBJECT(cpu), def->model_id, "model-id", &error);
 if (error_is_set(&error)) {
 return -1;
 }
@@ -1544,6 +1547,9 @@ static void x86_cpu_initfn(Object *obj)
 object_property_add(obj, "stepping", "int",
 NULL,
 x86_cpuid_version_set_stepping, NULL, NULL, NULL);
+object_property_add_str(obj, "model-id",
+NULL,
+x86_cpuid_set_model_id, NULL);
 
 env->cpuid_apic_id = env->cpu_index;
 mce_init(cpu);
-- 
1.7.7




[Qemu-devel] [PATCH 01/15] target-i386: Fix x86_cpuid_set_model_id()

2012-04-17 Thread Andreas Färber
Don't assume zero'ed cpuid_model[] fields.

Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3df53ca..80c1ca5 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -627,6 +627,9 @@ static void x86_cpuid_set_model_id(CPUX86State *env, const 
char *model_id)
 model_id = "";
 }
 len = strlen(model_id);
+for (i = 0; i < 12; i++) {
+env->cpuid_model[i] = 0;
+}
 for (i = 0; i < 48; i++) {
 if (i >= len) {
 c = '\0';
-- 
1.7.7




Re: [Qemu-devel] [PATCH] qemu-ga: generate missing stubs for fsfreeze

2012-04-17 Thread Michael Roth
On Tue, Apr 17, 2012 at 01:54:49PM -0300, Luiz Capitulino wrote:
> On Tue, 17 Apr 2012 10:15:03 -0500
> Michael Roth  wrote:
> 
> > On Tue, Apr 17, 2012 at 11:44:39AM -0300, Luiz Capitulino wrote:
> > > On Fri, 13 Apr 2012 21:07:36 -0500
> > > Michael Roth  wrote:
> > > 
> > > > When linux-specific commands (including guest-fsfreeze-*) were 
> > > > consolidated
> > > > under defined(__linux__), we forgot to account for the case where
> > > > defined(__linux__) && !defined(FIFREEZE). As a result stubs are no 
> > > > longer
> > > > being generated on linux hosts that don't have FIFREEZE support. Fix
> > > > this.
> > > > 
> > > > Signed-off-by: Michael Roth 
> > > 
> > > Looks good to me, but I'm honestly a bit confused with the number of 
> > > ifdefs we
> > > have in this file. I think it's possible to re-organize them, but maybe 
> > > it's
> > > best to move the linux specific stuff to its own file.
> > > 
> > 
> > Yah, it was actually pretty nice and organized prior to this patch.
> 
> I think there's room for improvement. Today we have:
> 
> #if defined(__linux__)
> 
> -> includes
> 
> #if defined(__linux__) && defined(FIREEZE)
> #defined CONFIG_FSFREEZE
> #endif
> #endif
> 
> #if defined(__linux__)
> static void reopen_fd_to_null(int fd);
> #endif
> 
> -> non linux-specific code
> 
> #if defined(__linux__)
> 
> #if defined(CONFIG_FSFREEZE)
> 
> -> fsfreeze functions
> 
> #endif CONFIG_FSFREEZE
> 
> -> linux specific functions
> 
> #else /* !defined(__linux__) */
> 
> -> fsfree & suspend stubs
> 
> #endif
> 
> 
> I think we could have:
> 
> 
> -> non-linux code
> 
> #if defined(__linux__)
> 
> -> includes

Personally I'd prefer grouping #includes together. I don't really have an
objection doing it this way though, but it doesn't bother me too much as
is.

> 
> static void reopen_fd_to_null(int fd)

I actually left reopen_fd_to_null() out of the linux-only grouping because
it should actually be used by some other functions that call fork()
(mainly: shutdown, which isn't linux-specific). There's a TODO I stuck in
there with the recent consolidation as a place-holder.

> 
> -> linux specific functions
> 
> #if defined(FIREEZE)
> 
> -> fsfreeze functions
> 
> #else /* ! defined(FIREEZE) */
> 
> -> fsfreeze stubs
> 
> #endif /* defined(FIREEZE) */

I assume you meant to add here an:

#else /* ! defined(__linux__) */

> 
> -> suspend stubs

We'd also need to copy/paste the fsfreeze stubs here if we did it this
way, which is why I ended up retaining the CONFIG_FSFREEZE macro and
generating those stubs on !CONFIG_FSFREEZE, outside of the
[!]defined(__linux__) blocks. CONFIG_FSFREEZE does obfuscate things a
bit though, and is somewhat redundant when used inside the linux-specific
block, so I agree we should drop it.

> 
> #endif /* defined(__linux__) */
> 



Re: [Qemu-devel] [PATCH 2/3] qtest: enable qtest for most targets

2012-04-17 Thread Anthony Liguori

On 04/17/2012 03:59 PM, Peter Maydell wrote:

On 17 April 2012 21:43, Blue Swirl  wrote:

On Tue, Apr 17, 2012 at 20:31, Peter Maydell  wrote:

Well, it could. But we should make that decision based on whether it
makes sense and has a use case for actual users of the board, not
because we're trying to get away with not having a setup that lets
us properly unit test devices.


I disagree. I see many benefits in qtest and very little downsides in
changes like these.

qtest is a tool to let developers test the changes they make to
devices, so breakages shouldn't be so common. This should improve the
development process in QEMU tremendously.


I'm entirely in favour of having a decent testing framework so we
can easily write unit tests for device models. What I don't understand
is why a developer only unit testing tool seems to require changes
to user visible behaviour across dozens of board models. Something
is wrong in its design somewhere, and I think that's what I'm
objecting to as much as to the specific detail of what's being
changed in this patch.




Kernel loading is a hack.  I'll go out on a limb and say that most non-x86 
boards are doing it completely wrong.  Messing around with CPU state has no 
business in machine init.  It creates horrible dependencies about RAM 
initialization order and problems for reset/live migration.


The kernel should be presented as a virtual device (an emulated flash or 
whatever) and there should be firmware that loads the kernel appropriately. 
Then we wouldn't need changes like this in the first place.




But now that that's out of my system, I don't think we should change every board 
that's doing direct kernel loading.  But this is why we need to make a change 
like this.  The boards are "wrong in its design somewhere".



(And I don't want us to add lots of tests
and/or changes to the code before we fix whatever the problem is.)


If you'd like to change all of the boards to behave in a way that's sensibly 
similar to how actual hardware would work, that's fine by me :-)


Regards,

Anthony Liguori


-- PMM






Re: [Qemu-devel] [PATCH 2/3] qtest: enable qtest for most targets

2012-04-17 Thread Peter Maydell
On 17 April 2012 21:43, Blue Swirl  wrote:
> On Tue, Apr 17, 2012 at 20:31, Peter Maydell  wrote:
>> Well, it could. But we should make that decision based on whether it
>> makes sense and has a use case for actual users of the board, not
>> because we're trying to get away with not having a setup that lets
>> us properly unit test devices.
>
> I disagree. I see many benefits in qtest and very little downsides in
> changes like these.
>
> qtest is a tool to let developers test the changes they make to
> devices, so breakages shouldn't be so common. This should improve the
> development process in QEMU tremendously.

I'm entirely in favour of having a decent testing framework so we
can easily write unit tests for device models. What I don't understand
is why a developer only unit testing tool seems to require changes
to user visible behaviour across dozens of board models. Something
is wrong in its design somewhere, and I think that's what I'm
objecting to as much as to the specific detail of what's being
changed in this patch. (And I don't want us to add lots of tests
and/or changes to the code before we fix whatever the problem is.)

-- PMM



[Qemu-devel] [PATCH 08/18] qemu-option: opts_do_parse(): use error_set()

2012-04-17 Thread Luiz Capitulino
The functions qemu_opts_do_parse() and opts_parse() both call
opts_do_parse(), but their callers expect QError semantics. Thus,
both functions call qerro_report_err() to keep the expected semantics.

Signed-off-by: Luiz Capitulino 
---
 qemu-option.c |   38 --
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index b409742..41e7a57 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -848,12 +848,11 @@ int qemu_opts_print(QemuOpts *opts, void *dummy)
 return 0;
 }
 
-static int opts_do_parse(QemuOpts *opts, const char *params,
- const char *firstname, bool prepend)
+static void opts_do_parse(QemuOpts *opts, const char *params,
+  const char *firstname, bool prepend, Error **errp)
 {
 char option[128], value[1024];
 const char *p,*pe,*pc;
-Error *local_err = NULL;
 
 for (p = params; *p != '\0'; p++) {
 pe = strchr(p, '=');
@@ -885,23 +884,29 @@ static int opts_do_parse(QemuOpts *opts, const char 
*params,
 }
 if (strcmp(option, "id") != 0) {
 /* store and parse */
-opt_set(opts, option, value, prepend, &local_err);
-if (error_is_set(&local_err)) {
-qerror_report_err(local_err);
-error_free(local_err);
-return -1;
+opt_set(opts, option, value, prepend, errp);
+if (error_is_set(errp)) {
+return;
 }
 }
 if (*p != ',') {
 break;
 }
 }
-return 0;
 }
 
 int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char 
*firstname)
 {
-return opts_do_parse(opts, params, firstname, false);
+Error *local_err = NULL;
+
+opts_do_parse(opts, params, firstname, false, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+return -1;
+}
+
+return 0;
 }
 
 static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
@@ -934,18 +939,23 @@ static QemuOpts *opts_parse(QemuOptsList *list, const 
char *params,
 }
 if (opts == NULL) {
 if (error_is_set(&local_err)) {
-qerror_report_err(local_err);
-error_free(local_err);
+goto exit_err;
 }
 return NULL;
 }
 
-if (opts_do_parse(opts, params, firstname, defaults) != 0) {
+opts_do_parse(opts, params, firstname, defaults, &local_err);
+if (error_is_set(&local_err)) {
 qemu_opts_del(opts);
-return NULL;
+goto exit_err;
 }
 
 return opts;
+
+exit_err:
+qerror_report_err(local_err);
+error_free(local_err);
+return NULL;
 }
 
 QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params,
-- 
1.7.9.2.384.g4a92a




Re: [Qemu-devel] [PATCH 2/3] qtest: enable qtest for most targets

2012-04-17 Thread Peter Maydell
On 17 April 2012 21:51, Blue Swirl  wrote:
> On Tue, Apr 17, 2012 at 20:35, Peter Maydell  wrote:
>> On 17 April 2012 21:31, Blue Swirl  wrote:
>>> On Tue, Apr 17, 2012 at 20:27, Peter Maydell  
>>> wrote:
 ARM has a default board set, which is unfortunate because it's
 hardware that nobody uses now, so the QEMU default setting
 mostly serves to trip up users who try to run a kernel for
 some other machine on it by mistake.
>>>
>>> Then change it.
>>
>> As I say, I don't want to break command line compatibility.
>
> v1.1: print a warning about using deprecated board, suggest -M integratorcp
> v1.2: change default board, when default board is used, print a notice
> that it's now -M z2, suggest -M integratorcp for old geezers
> v1.3: remove notice

If I were going to change, I'd switch to "no default board", because
there is no single right choice for everybody in the ARM world.

-- PMM



[Qemu-devel] [PATCH 18/18] qapi: convert netdev_del

2012-04-17 Thread Luiz Capitulino
Signed-off-by: Anthony Liguori 
Signed-off-by: Luiz Capitulino 
---
 hmp-commands.hx  |3 +--
 hmp.c|9 +
 hmp.h|1 +
 net.c|   11 +--
 net.h|1 -
 qapi-schema.json |   14 ++
 qmp-commands.hx  |5 +
 7 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 7381395..ae7d356 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1023,8 +1023,7 @@ ETEXI
 .args_type  = "id:s",
 .params = "id",
 .help   = "remove host network device",
-.user_print = monitor_user_noop,
-.mhandler.cmd_new = do_netdev_del,
+.mhandler.cmd = hmp_netdev_del,
 },
 
 STEXI
diff --git a/hmp.c b/hmp.c
index 2b78f8b..10109e7 100644
--- a/hmp.c
+++ b/hmp.c
@@ -976,3 +976,12 @@ void hmp_netdev_add(Monitor *mon, const QDict *qdict)
 hmp_handle_error(mon, &local_err);
 QDECREF(arglist);
 }
+
+void hmp_netdev_del(Monitor *mon, const QDict *qdict)
+{
+const char *id = qdict_get_str(qdict, "id");
+Error *err = NULL;
+
+qmp_netdev_del(id, &err);
+hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 2fc041a..ca005df 100644
--- a/hmp.h
+++ b/hmp.h
@@ -62,5 +62,6 @@ void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
 void hmp_migrate(Monitor *mon, const QDict *qdict);
 void hmp_device_del(Monitor *mon, const QDict *qdict);
 void hmp_netdev_add(Monitor *mon, const QDict *qdict);
+void hmp_netdev_del(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/net.c b/net.c
index c758a7c..a7355f6 100644
--- a/net.c
+++ b/net.c
@@ -1274,19 +1274,18 @@ exit_err:
 qemu_opts_del(opts);
 }
 
-int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
+void qmp_netdev_del(const char *id, Error **errp)
 {
-const char *id = qdict_get_str(qdict, "id");
 VLANClientState *vc;
 
 vc = qemu_find_netdev(id);
 if (!vc) {
-qerror_report(QERR_DEVICE_NOT_FOUND, id);
-return -1;
+error_set(errp, QERR_DEVICE_NOT_FOUND, id);
+return;
 }
+
 qemu_del_vlan_client(vc);
-qemu_opts_del(qemu_opts_find(qemu_find_opts("netdev"), id));
-return 0;
+qemu_opts_del(qemu_opts_find(qemu_find_opts_err("netdev", errp), id));
 }
 
 static void print_net_client(Monitor *mon, VLANClientState *vc)
diff --git a/net.h b/net.h
index 0dfa49a..bb5c28f 100644
--- a/net.h
+++ b/net.h
@@ -170,7 +170,6 @@ void net_check_clients(void);
 void net_cleanup(void);
 void net_host_device_add(Monitor *mon, const QDict *qdict);
 void net_host_device_remove(Monitor *mon, const QDict *qdict);
-int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
 #define DEFAULT_NETWORK_SCRIPT "/etc/qemu-ifup"
 #define DEFAULT_NETWORK_DOWN_SCRIPT "/etc/qemu-ifdown"
diff --git a/qapi-schema.json b/qapi-schema.json
index 8fec5f0..3b3a55f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1749,3 +1749,17 @@
 ##
 { 'command': 'netdev_add',
   'data': {'type': 'str', 'id': 'str', '*props': '**'} }
+
+##
+# @netdev_del:
+#
+# Remove a network backend.
+#
+# @id: the name of the network backend to remove
+#
+# Returns: Nothing on success
+#  If @id is not a valid network backend, DeviceNotFound
+#
+# Since: 0.14.0
+##
+{ 'command': 'netdev_del', 'data': {'id': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 5f73ac3..866e26c 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -635,10 +635,7 @@ EQMP
 {
 .name   = "netdev_del",
 .args_type  = "id:s",
-.params = "id",
-.help   = "remove host network device",
-.user_print = monitor_user_noop,
-.mhandler.cmd_new = do_netdev_del,
+.mhandler.cmd_new = qmp_marshal_input_netdev_del,
 },
 
 SQMP
-- 
1.7.9.2.384.g4a92a




Re: [Qemu-devel] [PATCH 2/3] qtest: enable qtest for most targets

2012-04-17 Thread Blue Swirl
On Tue, Apr 17, 2012 at 20:35, Peter Maydell  wrote:
> On 17 April 2012 21:31, Blue Swirl  wrote:
>> On Tue, Apr 17, 2012 at 20:27, Peter Maydell  
>> wrote:
>>> ARM has a default board set, which is unfortunate because it's
>>> hardware that nobody uses now, so the QEMU default setting
>>> mostly serves to trip up users who try to run a kernel for
>>> some other machine on it by mistake.
>>
>> Then change it.
>
> As I say, I don't want to break command line compatibility.

v1.1: print a warning about using deprecated board, suggest -M integratorcp
v1.2: change default board, when default board is used, print a notice
that it's now -M z2, suggest -M integratorcp for old geezers
v1.3: remove notice

>
> -- PMM



[Qemu-devel] [PATCH 06/18] qemu-option: qemu_opts_validate(): use error_set()

2012-04-17 Thread Luiz Capitulino
Signed-off-by: Luiz Capitulino 
---
 net.c |6 +-
 qemu-option.c |   16 +---
 qemu-option.h |2 +-
 3 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/net.c b/net.c
index 1922d8a..f5d9cc7 100644
--- a/net.c
+++ b/net.c
@@ -1136,10 +1136,14 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int 
is_netdev)
 for (i = 0; i < NET_CLIENT_TYPE_MAX; i++) {
 if (net_client_types[i].type != NULL &&
 !strcmp(net_client_types[i].type, type)) {
+Error *local_err = NULL;
 VLANState *vlan = NULL;
 int ret;
 
-if (qemu_opts_validate(opts, &net_client_types[i].desc[0]) == -1) {
+qemu_opts_validate(opts, &net_client_types[i].desc[0], &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
 return -1;
 }
 
diff --git a/qemu-option.c b/qemu-option.c
index 4829de5..5299a30 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -1047,7 +1047,7 @@ QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict)
 /* Validate parsed opts against descriptions where no
  * descriptions were provided in the QemuOptsList.
  */
-int qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc)
+void qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp)
 {
 QemuOpt *opt;
 Error *local_err = NULL;
@@ -1063,24 +1063,18 @@ int qemu_opts_validate(QemuOpts *opts, const 
QemuOptDesc *desc)
 }
 }
 if (desc[i].name == NULL) {
-error_set(&local_err, QERR_INVALID_PARAMETER, opt->name);
-goto exit_err;
+error_set(errp, QERR_INVALID_PARAMETER, opt->name);
+return;
 }
 
 opt->desc = &desc[i];
 
 qemu_opt_parse(opt, &local_err);
 if (error_is_set(&local_err)) {
-goto exit_err;
+error_propagate(errp, local_err);
+return;
 }
 }
-
-return 0;
-
-exit_err:
-qerror_report_err(local_err);
-error_free(local_err);
-return -1;
 }
 
 int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void 
*opaque,
diff --git a/qemu-option.h b/qemu-option.h
index 4d5b3d3..e9fbbb5 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -125,7 +125,7 @@ int qemu_opts_set(QemuOptsList *list, const char *id,
   const char *name, const char *value);
 const char *qemu_opts_id(QemuOpts *opts);
 void qemu_opts_del(QemuOpts *opts);
-int qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc);
+void qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp);
 int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char 
*firstname);
 QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, int 
permit_abbrev);
 void qemu_opts_set_defaults(QemuOptsList *list, const char *params,
-- 
1.7.9.2.384.g4a92a




Re: [Qemu-devel] [RFC 00/14] MAINTAINERS cleanups - please read

2012-04-17 Thread Blue Swirl
On Mon, Apr 16, 2012 at 21:47, Anthony Liguori  wrote:
> On 04/16/2012 04:24 PM, Peter Maydell wrote:
>>
>> On 16 April 2012 18:42, Anthony Liguori  wrote:
>>>
>>> On 04/16/2012 12:17 PM, Peter Maydell wrote:

 Here's my stab at it:
            Maintained:  Someone actually looks after it. The maintainer
                         will have a git subtree for this area and
 patches
                         are expected to go through it. Bug reports will
                         generally be investigated.
>>>
>>>
>>> * For something to be marked Maintained, there must be a person on M: and
>>> there must be a git tree for the subsystem.
>>
>>
>> Do you mean "there must be a git tree" or "there must be a git tree
>> listed under T: for this area" ? We have I think several subsystems
>> where things do come in via pullreq for a submaintainer tree but that
>> tree isn't officially public except in as much as the branch name
>> for the pullreq is always the same...
>
>
> I'd like to record T: as part of a way to validate pull requests.  I get
> slightly nervous about pull requests because it's an easy way to sneak code
> into the tree if you're malicious.

Wouldn't signed PULL requests help? They need a very recent git though.

>
> I'd prefer if we kept an official whitelist of trees in MAINTAINERS verses
> relying on my local .git/config.
>
>
>>
>> I don't particularly object to providing a T: line for
>> target-arm.next/arm-devs.next, but I'm not sure it's particularly useful,
>> since we don't have the same tendency the kernel does to having subtrees
>> which can diverge significantly because of large amounts of change waiting
>> for a merge window. I wouldn't expect people to base arm patches against
>> arm-devs.next rather than master, for instance. (Maybe I should??)
>> Anyway, I think if we have T: lines in MAINTAINERS it should be because
>> (and we should clearly say that) that is the tree that we expect patches
>> in that area to apply to.
>
>
> I think we should (and do already?) say that all patches on qemu-devel
> should be against qemu.git unless otherwise indicated in the patch subject.
>
> Regards,
>
> Anthony Liguori
>
>>
>> -- PMM
>>
>
>



[Qemu-devel] [PATCH 09/18] qemu-option: qemu_opts_do_parse(): convert error_set()

2012-04-17 Thread Luiz Capitulino
qemu_chr_parse_compat() calls qerror_report_err() because its callers
expect QError semantics.

Signed-off-by: Luiz Capitulino 
---
 qemu-char.c   |   12 ++--
 qemu-option.c |   10 --
 qemu-option.h |3 ++-
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 3a5d2b6..acb0c30 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2651,8 +2651,12 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const 
char *filename)
 qemu_opt_set(opts, "host", host);
 qemu_opt_set(opts, "port", port);
 if (p[pos] == ',') {
-if (qemu_opts_do_parse(opts, p+pos+1, NULL) != 0)
+qemu_opts_do_parse(opts, p+pos+1, NULL, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
 goto fail;
+}
 }
 if (strstart(filename, "telnet:", &p))
 qemu_opt_set(opts, "telnet", "on");
@@ -2683,8 +2687,12 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const 
char *filename)
 }
 if (strstart(filename, "unix:", &p)) {
 qemu_opt_set(opts, "backend", "socket");
-if (qemu_opts_do_parse(opts, p, "path") != 0)
+qemu_opts_do_parse(opts, p, "path", &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
 goto fail;
+}
 return opts;
 }
 if (strstart(filename, "/dev/parport", NULL) ||
diff --git a/qemu-option.c b/qemu-option.c
index 41e7a57..8a9d8d5 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -895,18 +895,16 @@ static void opts_do_parse(QemuOpts *opts, const char 
*params,
 }
 }
 
-int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char 
*firstname)
+void qemu_opts_do_parse(QemuOpts *opts, const char *params,
+const char *firstname, Error **errp)
 {
 Error *local_err = NULL;
 
 opts_do_parse(opts, params, firstname, false, &local_err);
 if (error_is_set(&local_err)) {
-qerror_report_err(local_err);
-error_free(local_err);
-return -1;
+error_propagate(errp, local_err);
+return;
 }
-
-return 0;
 }
 
 static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
diff --git a/qemu-option.h b/qemu-option.h
index e9fbbb5..4480c17 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -126,7 +126,8 @@ int qemu_opts_set(QemuOptsList *list, const char *id,
 const char *qemu_opts_id(QemuOpts *opts);
 void qemu_opts_del(QemuOpts *opts);
 void qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp);
-int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char 
*firstname);
+void qemu_opts_do_parse(QemuOpts *opts, const char *params,
+const char *firstname, Error **errp);
 QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, int 
permit_abbrev);
 void qemu_opts_set_defaults(QemuOptsList *list, const char *params,
 int permit_abbrev);
-- 
1.7.9.2.384.g4a92a




[Qemu-devel] [PATCH 11/18] qerror: introduce QERR_INVALID_OPTION_GROUP

2012-04-17 Thread Luiz Capitulino
Signed-off-by: Luiz Capitulino 
---
 qerror.c |4 
 qerror.h |3 +++
 2 files changed, 7 insertions(+)

diff --git a/qerror.c b/qerror.c
index 96fbe71..5d92096 100644
--- a/qerror.c
+++ b/qerror.c
@@ -156,6 +156,10 @@ static const QErrorStringTable qerror_table[] = {
 .desc  = "Invalid block format '%(name)'",
 },
 {
+.error_fmt = QERR_INVALID_OPTION_GROUP,
+.desc  = "There is no option group '%(group)'",
+},
+{
 .error_fmt = QERR_INVALID_PARAMETER,
 .desc  = "Invalid parameter '%(name)'",
 },
diff --git a/qerror.h b/qerror.h
index 5c23c1f..0d8bfd2 100644
--- a/qerror.h
+++ b/qerror.h
@@ -139,6 +139,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_INVALID_BLOCK_FORMAT \
 "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }"
 
+#define QERR_INVALID_OPTION_GROUP \
+"{ 'class': 'InvalidOptionGroup', 'data': { 'group': %s } }"
+
 #define QERR_INVALID_PARAMETER \
 "{ 'class': 'InvalidParameter', 'data': { 'name': %s } }"
 
-- 
1.7.9.2.384.g4a92a




Re: [Qemu-devel] [PATCH 2/3] qtest: enable qtest for most targets

2012-04-17 Thread Blue Swirl
On Tue, Apr 17, 2012 at 20:31, Peter Maydell  wrote:
> On 17 April 2012 21:24, Blue Swirl  wrote:
>> On Tue, Apr 17, 2012 at 07:33, Peter Maydell  
>> wrote:
>>> Just testing a device shouldn't require running a particular
>>> board model either, of course.
>>
>> The goal is obviously to make comprehensive tests for all devices in
>> all boards. Also, if a device is only used by a certain board then
>> yes, that particular board model is required.
>
> Device testing should be done by instantiating the device in
> a test harness and prodding it according to whatever tests
> you're doing. Trying to use the board as an ad-hoc test
> harness means you can't test aspects of the device which
> the board doesn't use, and if the board changes then half your
> test cases probably break.

You have a point. The unused features may be uninteresting though if
no board uses them.

>
 I think we should probably refactor the boards to do something more useful
 if a kernel isn't specified verses just buggering out.
>>>
>>> I don't inherently object if you can define something that's genuinely
>>> more useful. I'm not sure "start the CPU and have it execute through
>>> zeroed out RAM so the user gets no indication they've forgotten something"
>>> really counts, though...
>>
>> The error could be downgraded to a warning.
>
> Well, it could. But we should make that decision based on whether it
> makes sense and has a use case for actual users of the board, not
> because we're trying to get away with not having a setup that lets
> us properly unit test devices.

I disagree. I see many benefits in qtest and very little downsides in
changes like these.

qtest is a tool to let developers test the changes they make to
devices, so breakages shouldn't be so common. This should improve the
development process in QEMU tremendously.

It's also a tool to find bugs and improve code quality.

I value the above much higher than how user experience is slightly
(and also arguably) downgraded, because now they are able to shoot
themselves to a foot more easily.

>
> -- PMM



Re: [Qemu-devel] [PATCH 15/18] qapi: implement support for variable argument list

2012-04-17 Thread Luiz Capitulino
On Tue, 17 Apr 2012 22:26:55 +0200
Paolo Bonzini  wrote:

> Il 17/04/2012 21:36, Luiz Capitulino ha scritto:
> > +switch(qobject_type(obj)) {
> > +case QTYPE_QSTRING:
> > +qstring_append(arglist,
> > +   qstring_get_str(qobject_to_qstring(obj)));
> > +break;
> 
> Does this escape commas correctly?

No, but does it have to? Does QemuOpts accept an option with a coma in it?

> It seems much easier to use no_gen and qemu_opts_from_qdict...  Then
> cmd_netdev_add can be

netdev_add/del is expected to be a stable interface, so we can't use no_gen.

>   void cmd_foo(QemuOpts *arglist, Error **errp);

Until now we're treating hmp.c like an external QMP C client, using QemuOpts
this way will leak qemu internals to hmp.c...

> 
> and later on we could even replace the QemuOpts with a visitor for full
> QAPI-ness...
> 
> Paolo
> 




Re: [Qemu-devel] [PATCH 2/3] qtest: enable qtest for most targets

2012-04-17 Thread Peter Maydell
On 17 April 2012 21:31, Blue Swirl  wrote:
> On Tue, Apr 17, 2012 at 20:27, Peter Maydell  wrote:
>> ARM has a default board set, which is unfortunate because it's
>> hardware that nobody uses now, so the QEMU default setting
>> mostly serves to trip up users who try to run a kernel for
>> some other machine on it by mistake.
>
> Then change it.

As I say, I don't want to break command line compatibility.

-- PMM



[Qemu-devel] [PATCH 04/18] qemu-option: parse_option_size(): use error_set()

2012-04-17 Thread Luiz Capitulino
Signed-off-by: Luiz Capitulino 
---
 qemu-option.c |   20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index a8b50af..61354af 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -203,7 +203,8 @@ static void parse_option_number(const char *name, const 
char *value,
 }
 }
 
-static int parse_option_size(const char *name, const char *value, uint64_t 
*ret)
+static void parse_option_size(const char *name, const char *value,
+  uint64_t *ret, Error **errp)
 {
 char *postfix;
 double sizef;
@@ -229,16 +230,14 @@ static int parse_option_size(const char *name, const char 
*value, uint64_t *ret)
 *ret = (uint64_t) sizef;
 break;
 default:
-qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a size");
+error_set(errp, QERR_INVALID_PARAMETER_VALUE, name, "a size");
 error_printf_unless_qmp("You may use k, M, G or T suffixes for "
 "kilobytes, megabytes, gigabytes and terabytes.\n");
-return -1;
+return;
 }
 } else {
-qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a size");
-return -1;
+error_set(errp, QERR_INVALID_PARAMETER_VALUE, name, "a size");
 }
-return 0;
 }
 
 /*
@@ -291,8 +290,10 @@ int set_option_parameter(QEMUOptionParameter *list, const 
char *name,
 break;
 
 case OPT_SIZE:
-if (parse_option_size(name, value, &list->value.n) == -1)
-return -1;
+parse_option_size(name, value, &list->value.n, &local_err);
+if (error_is_set(&local_err)) {
+goto exit_err;
+}
 break;
 
 default:
@@ -602,7 +603,8 @@ static int qemu_opt_parse(QemuOpt *opt)
 &local_err);
 break;
 case QEMU_OPT_SIZE:
-return parse_option_size(opt->name, opt->str, &opt->value.uint);
+parse_option_size(opt->name, opt->str, &opt->value.uint, &local_err);
+break;
 default:
 abort();
 }
-- 
1.7.9.2.384.g4a92a




Re: [Qemu-devel] [PATCH 2/3] qtest: enable qtest for most targets

2012-04-17 Thread Blue Swirl
On Tue, Apr 17, 2012 at 20:27, Peter Maydell  wrote:
> On 17 April 2012 21:19, Blue Swirl  wrote:
>> On Mon, Apr 16, 2012 at 21:58, Max Filippov  wrote:
>>> What is the use case of the default board?
>>
>> If an architecture has multiple boards, the default board can be used
>> without using -M option.
>
> Unless the Xtensa architecture has a particular board which
> is and will *always* be a primary choice for anybody using it
> (as "pc" is for x86) I'd recommend against setting a default
> board for it, because once you set one you can never change
> it without breaking command-line compatibility for users.

IIRC we have changed the default board for PPC from oldworld to newworld model.

Xtensa is the only architecture out of 14 which does not have a default board.

>
> ARM has a default board set, which is unfortunate because it's
> hardware that nobody uses now, so the QEMU default setting
> mostly serves to trip up users who try to run a kernel for
> some other machine on it by mistake.

Then change it.

>
> -- PMM



[Qemu-devel] [PATCH 10/18] qemu-option: introduce qemu_opt_set_err()

2012-04-17 Thread Luiz Capitulino
This is like qemu_opt_set(), except that it takes an Error argument.

This new function allows for a incremental conversion of code using
qemu_opt_set().

Signed-off-by: Luiz Capitulino 
---
 qemu-option.c |6 ++
 qemu-option.h |2 ++
 2 files changed, 8 insertions(+)

diff --git a/qemu-option.c b/qemu-option.c
index 8a9d8d5..70e55df 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -670,6 +670,12 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const 
char *value)
 return 0;
 }
 
+void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value,
+  Error **errp)
+{
+opt_set(opts, name, value, false, errp);
+}
+
 int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val)
 {
 QemuOpt *opt;
diff --git a/qemu-option.h b/qemu-option.h
index 4480c17..ddd15ce 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -111,6 +111,8 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name, 
bool defval);
 uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t 
defval);
 uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval);
 int qemu_opt_set(QemuOpts *opts, const char *name, const char *value);
+void qemu_opt_set_err(QemuOpts *opts, const char *name, const char *value,
+  Error **errp);
 int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val);
 typedef int (*qemu_opt_loopfunc)(const char *name, const char *value, void 
*opaque);
 int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
-- 
1.7.9.2.384.g4a92a




Re: [Qemu-devel] [PATCH 2/3] qtest: enable qtest for most targets

2012-04-17 Thread Peter Maydell
On 17 April 2012 21:24, Blue Swirl  wrote:
> On Tue, Apr 17, 2012 at 07:33, Peter Maydell  wrote:
>> Just testing a device shouldn't require running a particular
>> board model either, of course.
>
> The goal is obviously to make comprehensive tests for all devices in
> all boards. Also, if a device is only used by a certain board then
> yes, that particular board model is required.

Device testing should be done by instantiating the device in
a test harness and prodding it according to whatever tests
you're doing. Trying to use the board as an ad-hoc test
harness means you can't test aspects of the device which
the board doesn't use, and if the board changes then half your
test cases probably break.

>>> I think we should probably refactor the boards to do something more useful
>>> if a kernel isn't specified verses just buggering out.
>>
>> I don't inherently object if you can define something that's genuinely
>> more useful. I'm not sure "start the CPU and have it execute through
>> zeroed out RAM so the user gets no indication they've forgotten something"
>> really counts, though...
>
> The error could be downgraded to a warning.

Well, it could. But we should make that decision based on whether it
makes sense and has a use case for actual users of the board, not
because we're trying to get away with not having a setup that lets
us properly unit test devices.

-- PMM



[Qemu-devel] [PATCH 07/18] qemu-option: opt_set(): use error_set()

2012-04-17 Thread Luiz Capitulino
The functions qemu_opt_set() and opts_do_parse() both call opt_set(),
but their callers expect QError semantics. Thus, both functions call
qerro_report_err() to keep the expected semantics.

Signed-off-by: Luiz Capitulino 
---
 qemu-option.c |   38 +-
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 5299a30..b409742 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -615,12 +615,11 @@ static void qemu_opt_del(QemuOpt *opt)
 g_free(opt);
 }
 
-static int opt_set(QemuOpts *opts, const char *name, const char *value,
-   bool prepend)
+static void opt_set(QemuOpts *opts, const char *name, const char *value,
+bool prepend, Error **errp)
 {
 QemuOpt *opt;
 const QemuOptDesc *desc = opts->list->desc;
-Error *local_err = NULL;
 int i;
 
 for (i = 0; desc[i].name != NULL; i++) {
@@ -632,8 +631,8 @@ static int opt_set(QemuOpts *opts, const char *name, const 
char *value,
 if (i == 0) {
 /* empty list -> allow any */;
 } else {
-error_set(&local_err, QERR_INVALID_PARAMETER, name);
-goto exit_err;
+error_set(errp, QERR_INVALID_PARAMETER, name);
+return;
 }
 }
 
@@ -651,23 +650,24 @@ static int opt_set(QemuOpts *opts, const char *name, 
const char *value,
 if (value) {
 opt->str = g_strdup(value);
 }
-qemu_opt_parse(opt, &local_err);
-if (error_is_set(&local_err)) {
+qemu_opt_parse(opt, errp);
+if (error_is_set(errp)) {
 qemu_opt_del(opt);
-goto exit_err;
 }
-
-return 0;
-
-exit_err:
-qerror_report_err(local_err);
-error_free(local_err);
-return -1;
 }
 
 int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
 {
-return opt_set(opts, name, value, false);
+Error *local_err = NULL;
+
+opt_set(opts, name, value, false, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+return -1;
+}
+
+return 0;
 }
 
 int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val)
@@ -853,6 +853,7 @@ static int opts_do_parse(QemuOpts *opts, const char *params,
 {
 char option[128], value[1024];
 const char *p,*pe,*pc;
+Error *local_err = NULL;
 
 for (p = params; *p != '\0'; p++) {
 pe = strchr(p, '=');
@@ -884,7 +885,10 @@ static int opts_do_parse(QemuOpts *opts, const char 
*params,
 }
 if (strcmp(option, "id") != 0) {
 /* store and parse */
-if (opt_set(opts, option, value, prepend) == -1) {
+opt_set(opts, option, value, prepend, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
 return -1;
 }
 }
-- 
1.7.9.2.384.g4a92a




Re: [Qemu-devel] [PATCH 2/3] qtest: enable qtest for most targets

2012-04-17 Thread Blue Swirl
On Tue, Apr 17, 2012 at 12:00, Anthony Liguori  wrote:
> On 04/17/2012 02:44 AM, Paolo Bonzini wrote:
>>
>> Il 17/04/2012 09:33, Peter Maydell ha scritto:

 I think the issue is that all of these machines mandate a -kernel
 option.
  qtest doesn't care if you pass a -kernel but requiring a kernel in
 order to
 test a device sucks especially if you don't possess the toolchain to
 build
 such a kernel.
>>>
>>>
>>> Just testing a device shouldn't require running a particular
>>> board model either, of course.
>>
>>
>> qtest isn't so much testing a device.  It is really testing a board with
>> improved determinism, debuggability and logging.
>>
>> That said, I think a simpler solution is to set kernel/initrd/dtb to
>> "/dev/null" if qtest is enabled.
>
>
> Why not refactor the check to be a QEMUMachine property (requires_kernel).
>  That way the check can be moved common code and that common code can have a
> single check where it sets the parameters to /dev/null if qtest is enabled.

The same problem exists in addition to kernel/initrd/dtb, also with
BIOS, other ROMs and blobs. Perhaps this could be handled at lower
level instead of higher: make load_elf/load_image fake success when
using qtest.

>
> Regards,
>
> Anthony Liguori
>
>>
>> Paolo
>>
>



Re: [Qemu-devel] [PATCH 2/3] qtest: enable qtest for most targets

2012-04-17 Thread Peter Maydell
On 17 April 2012 21:19, Blue Swirl  wrote:
> On Mon, Apr 16, 2012 at 21:58, Max Filippov  wrote:
>> What is the use case of the default board?
>
> If an architecture has multiple boards, the default board can be used
> without using -M option.

Unless the Xtensa architecture has a particular board which
is and will *always* be a primary choice for anybody using it
(as "pc" is for x86) I'd recommend against setting a default
board for it, because once you set one you can never change
it without breaking command-line compatibility for users.

ARM has a default board set, which is unfortunate because it's
hardware that nobody uses now, so the QEMU default setting
mostly serves to trip up users who try to run a kernel for
some other machine on it by mistake.

-- PMM



Re: [Qemu-devel] [PATCH 15/18] qapi: implement support for variable argument list

2012-04-17 Thread Paolo Bonzini
Il 17/04/2012 21:36, Luiz Capitulino ha scritto:
> +switch(qobject_type(obj)) {
> +case QTYPE_QSTRING:
> +qstring_append(arglist,
> +   qstring_get_str(qobject_to_qstring(obj)));
> +break;

Does this escape commas correctly?

It seems much easier to use no_gen and qemu_opts_from_qdict...  Then
cmd_netdev_add can be

  void cmd_foo(QemuOpts *arglist, Error **errp);

and later on we could even replace the QemuOpts with a visitor for full
QAPI-ness...

Paolo




Re: [Qemu-devel] [PATCH 2/3] qtest: enable qtest for most targets

2012-04-17 Thread Blue Swirl
On Tue, Apr 17, 2012 at 07:33, Peter Maydell  wrote:
> On 17 April 2012 02:16, Anthony Liguori  wrote:
>> On 04/16/2012 05:54 PM, Peter Maydell wrote:
>>> Yuck! Nack, this is way too invasive. Testing frameworks
>>> shouldn't require random pointless changes to every board
>>> model.
>>
>> I think the issue is that all of these machines mandate a -kernel option.
>>  qtest doesn't care if you pass a -kernel but requiring a kernel in order to
>> test a device sucks especially if you don't possess the toolchain to build
>> such a kernel.
>
> Just testing a device shouldn't require running a particular
> board model either, of course.

The goal is obviously to make comprehensive tests for all devices in
all boards. Also, if a device is only used by a certain board then
yes, that particular board model is required.

>> I think we should probably refactor the boards to do something more useful
>> if a kernel isn't specified verses just buggering out.
>
> I don't inherently object if you can define something that's genuinely
> more useful. I'm not sure "start the CPU and have it execute through
> zeroed out RAM so the user gets no indication they've forgotten something"
> really counts, though...

The error could be downgraded to a warning.

>
> -- PMM



Re: [Qemu-devel] [PATCH 2/3] qtest: enable qtest for most targets

2012-04-17 Thread Blue Swirl
On Mon, Apr 16, 2012 at 21:58, Max Filippov  wrote:
>> Skip ROM or kernel loading and TCG init for qtest.
>>
>> For Xtensa there is no default board and the
>
> What is the use case of the default board?

If an architecture has multiple boards, the default board can be used
without using -M option.

> Are there any specific requirements for it?

I don't think so.

>
> --
> Thanks.
> -- Max



Re: [Qemu-devel] qemu softmmu inlined lookup sequence

2012-04-17 Thread Blue Swirl
On Tue, Apr 17, 2012 at 05:40, Xin Tong  wrote:
> that is possible. but if that is the case, why not split the tlb
> walking and the tlb fill ? can anyone please confirm ?

I sent a patch earlier that did something like that but it wasn't very
successful:
http://lists.nongnu.org/archive/html/qemu-devel/2012-04/msg00992.html

>
> Xin
>
>
> 2012/4/16 陳韋任 :
>>> > If TLB miss, it will call something like __ldb_mmu (b). __ldb_mmu will 
>>> > try to
>>> > walk guest page table, then fill TLB entry if page table hit, or raise a 
>>> > guest
>>> > page fault exception if page table miss.
>>>
>>> Yep. that is what i was taught. the sequence of code above is an
>>> inlined assembly for walking the TLB.  In the __ldx_mmu, the tlb is
>>> walked again ? why ?
>>>
>>>     int index, shift;
>>>     target_phys_addr_t ioaddr;
>>>     unsigned long addend;
>>>     target_ulong tlb_addr, addr1, addr2;
>>>
>>>     index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
>>>  redo:
>>>     tlb_addr = env->tlb_table[mmu_idx][index].ADDR_READ;
>>>     if ((addr & TARGET_PAGE_MASK) == (tlb_addr & (TARGET_PAGE_MASK |
>>> TLB_INVALID_MASK))) {
>>>     ...
>>>  }
>>>
>>>   ...
>>>   ...
>>>   fill_tlb()
>>>   ...
>>>
>>> }
>>
>>  Perhaps __ldb_mmu is not only called in the TLB lookup sequence, I guess.
>> But I am not sure of it.
>>
>> Regards,
>> chenwj
>>
>>
>> --
>> Wei-Ren Chen (陳韋任)
>> Computer Systems Lab, Institute of Information Science,
>> Academia Sinica, Taiwan (R.O.C.)
>> Tel:886-2-2788-3799 #1667
>> Homepage: http://people.cs.nctu.edu.tw/~chenwj
>



[Qemu-devel] [PATCH 01/18] qemu-option: qemu_opts_create(): use error_set()

2012-04-17 Thread Luiz Capitulino
This commit converts qemu_opts_create() from qerror_report() to
error_set().

Currently, most calls to qemu_opts_create() can't fail, so most
callers don't need any changes.

The two cases where code checks for qemu_opts_create() erros are:

 1. Initialization code in vl.c. All of them print their own
error messages directly to stderr, no need to pass the Error
object

 2. The functions opts_parse(), qemu_opts_from_qdict() and
qemu_chr_parse_compat() make use of the error information and
they can be called from HMP or QMP. In this case, to allow for
incremental conversion, we propagate the error up using
qerror_report_err(), which keeps the QError semantics

Signed-off-by: Luiz Capitulino 
---
 blockdev.c   |2 +-
 hw/usb/dev-storage.c |2 +-
 hw/watchdog.c|2 +-
 qemu-char.c  |8 ++--
 qemu-config.c|6 +++---
 qemu-option.c|   37 +++--
 qemu-option.h|4 +++-
 qemu-sockets.c   |8 
 vl.c |   22 +-
 9 files changed, 59 insertions(+), 32 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 0c2440e..0a4acd3 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -569,7 +569,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
 break;
 case IF_VIRTIO:
 /* add virtio block device */
-opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0);
+opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL);
 if (arch_type == QEMU_ARCH_S390X) {
 qemu_opt_set(opts, "driver", "virtio-blk-s390");
 } else {
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index d865a5e..3e2dc69 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -580,7 +580,7 @@ static USBDevice *usb_msd_init(USBBus *bus, const char 
*filename)
 
 /* parse -usbdevice disk: syntax into drive opts */
 snprintf(id, sizeof(id), "usb%d", nr++);
-opts = qemu_opts_create(qemu_find_opts("drive"), id, 0);
+opts = qemu_opts_create(qemu_find_opts("drive"), id, 0, NULL);
 
 p1 = strchr(filename, ':');
 if (p1++) {
diff --git a/hw/watchdog.c b/hw/watchdog.c
index 4c18965..a42124d 100644
--- a/hw/watchdog.c
+++ b/hw/watchdog.c
@@ -66,7 +66,7 @@ int select_watchdog(const char *p)
 QLIST_FOREACH(model, &watchdog_list, entry) {
 if (strcasecmp(model->wdt_name, p) == 0) {
 /* add the device */
-opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0);
+opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0, NULL);
 qemu_opt_set(opts, "driver", p);
 return 0;
 }
diff --git a/qemu-char.c b/qemu-char.c
index 74c60e1..3a5d2b6 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2582,10 +2582,14 @@ QemuOpts *qemu_chr_parse_compat(const char *label, 
const char *filename)
 int pos;
 const char *p;
 QemuOpts *opts;
+Error *local_err = NULL;
 
-opts = qemu_opts_create(qemu_find_opts("chardev"), label, 1);
-if (NULL == opts)
+opts = qemu_opts_create(qemu_find_opts("chardev"), label, 1, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
 return NULL;
+}
 
 if (strstart(filename, "mon:", &p)) {
 filename = p;
diff --git a/qemu-config.c b/qemu-config.c
index be84a03..f876646 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -709,7 +709,7 @@ int qemu_global_option(const char *str)
 return -1;
 }
 
-opts = qemu_opts_create(&qemu_global_opts, NULL, 0);
+opts = qemu_opts_create(&qemu_global_opts, NULL, 0, NULL);
 qemu_opt_set(opts, "driver", driver);
 qemu_opt_set(opts, "property", property);
 qemu_opt_set(opts, "value", str+offset+1);
@@ -781,7 +781,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const 
char *fname)
 list = find_list(lists, group);
 if (list == NULL)
 goto out;
-opts = qemu_opts_create(list, id, 1);
+opts = qemu_opts_create(list, id, 1, NULL);
 continue;
 }
 if (sscanf(line, "[%63[^]]]", group) == 1) {
@@ -789,7 +789,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const 
char *fname)
 list = find_list(lists, group);
 if (list == NULL)
 goto out;
-opts = qemu_opts_create(list, NULL, 0);
+opts = qemu_opts_create(list, NULL, 0, NULL);
 continue;
 }
 if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2) {
diff --git a/qemu-option.c b/qemu-option.c
index 35cd609..9f531c8 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -30,6 +30,7 @@
 #include "qemu-error.h"
 #include "qemu-objects.h"
 #include "qemu-option.h"
+#include "error.h"
 #include "qerror.h"
 
 /*
@@ -729,20 +730,21 @@ static int id_wellformed(const char *id)
 return 1;
 }
 
-QemuOpts 

[Qemu-devel] [PATCH 15/18] qapi: implement support for variable argument list

2012-04-17 Thread Luiz Capitulino
The variable argument list allows QAPI functions to receive a
string in the format "name1=value1,name2=value2 ... nameN=valueN".

This is going to be used by QMP commands that accept options in
QemuOpts format, like netdev_add.

The variable argument list is represented by the QAPI type '**'. It's
declared like this in the schema:

  { 'command': 'cmd-foo', 'data': { 'arglist': '**' } }

and the code generator will generate the following signature for cmd-foo:

void cmd_foo(const char *arglist, Error **errp);

The argument list can be accessed in 'arglist'.

Signed-off-by: Luiz Capitulino 
---
 scripts/qapi-commands.py |   91 ++
 scripts/qapi.py  |2 +
 2 files changed, 93 insertions(+)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 0b4f0a0..1506263 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -122,6 +122,10 @@ bool has_%(argname)s = false;
 %(argtype)s %(argname)s;
 ''',
  argname=c_var(argname), argtype=c_type(argtype))
+if argtype == '**':
+ret += mcgen('''
+QString *arglist = NULL;
+''')
 
 pop_indent()
 return ret.rstrip()
@@ -146,6 +150,8 @@ v = qmp_input_get_visitor(mi);
  obj=obj)
 
 for argname, argtype, optional, structured in parse_args(args):
+if argtype == '**':
+continue
 if optional:
 ret += mcgen('''
 visit_start_optional(v, &has_%(c_name)s, "%(name)s", errp);
@@ -257,6 +263,86 @@ def gen_marshal_input(name, args, ret_type, middle_mode):
 if (error_is_set(errp)) {
 goto out;
 }
+''')
+
+# '**' implementation
+regular_args = []
+var_list_name = ""
+for argname, argtype, optional, structured in parse_args(args):
+if argtype == '**':
+var_list_name = argname
+else:
+regular_args.append(argname)
+
+if var_list_name:
+ret += mcgen('''
+else {
+const QDictEntry *entry;
+
+arglist = qstring_new();
+for (entry = qdict_first(args); entry;
+ entry = qdict_next(qdict, entry)) {
+const QObject *obj = qdict_entry_value(entry);
+char buf[32];
+int n;
+
+''')
+for argname in regular_args:
+ret += mcgen('''
+if (strcmp(qdict_entry_key(entry), "%(argname)s") == 0) {
+continue;
+}
+
+''', argname=argname)
+
+ret += mcgen('''
+if (qstring_len(arglist) > 0) {
+qstring_append(arglist, ",");
+}
+
+qstring_append(arglist, qdict_entry_key(entry));
+qstring_append(arglist, "=");
+
+switch(qobject_type(obj)) {
+case QTYPE_QSTRING:
+qstring_append(arglist,
+   qstring_get_str(qobject_to_qstring(obj)));
+break;
+case QTYPE_QINT:
+n = snprintf(buf, sizeof(buf), %(arg1)s,
+ qint_get_int(qobject_to_qint(obj)));
+assert(n < sizeof(buf));
+qstring_append(arglist, buf);
+break;
+case QTYPE_QFLOAT:
+n = snprintf(buf, sizeof(buf), %(arg2)s,
+ qfloat_get_double(qobject_to_qfloat(obj)));
+qstring_append(arglist, buf);
+break;
+case QTYPE_QBOOL:
+pstrcpy(buf, sizeof(buf),
+qbool_get_int(qobject_to_qbool(obj)) ? "on" : "off");
+qstring_append(arglist, buf);
+break;
+default:
+QDECREF(arglist);
+error_set(&local_err, QERR_INVALID_PARAMETER_TYPE,
+  qdict_entry_key(entry),
+  "string, int, float or bool");
+goto out;
+}
+}
+
+if (qstring_len(arglist) > 0) {
+has_%(var_list_name)s = true;
+%(var_list_name)s = qstring_get_str(arglist);
+}
+}
+
+''', var_list_name=c_var(var_list_name), arg1='"%" PRId64', arg2='"%.17g"',
+arg3='"%d"')
+
+ret += mcgen('''
 %(sync_call)s
 ''',
  sync_call=gen_sync_call(name, args, ret_type, indent=4))
@@ -270,6 +356,11 @@ out:
  visitor_input_block_cleanup=gen_visitor_input_block(args, 
None,
  
dealloc=True))
 
+if var_list_name:
+ret += mcgen('''
+QDECREF(arglist);
+''')
+
 if middle_mode:
 ret += mcgen('''
 
diff --git a/scripts/qapi.py b/scripts/qapi.py
index e062336..9bd6e95 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -163,6 +163,8 @@ def c_type(name):
 return 'bool'
 elif name == 'number':
 return 'double'
+elif name == '**':
+return 'const char *'
 elif type(name) == list:
 return

[Qemu-devel] [PATCH 05/18] qemu-option: qemu_opt_parse(): use error_set()

2012-04-17 Thread Luiz Capitulino
The functions opt_set() and qemu_opts_validate() both call qemu_opt_parse(),
but their callers expect QError semantics. Thus, both functions call
qerro_report_err() to keep the expected semantics.

Signed-off-by: Luiz Capitulino 
---
 qemu-option.c |   54 +-
 1 file changed, 29 insertions(+), 25 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 61354af..4829de5 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -584,38 +584,27 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char 
*name, uint64_t defval)
 return opt->value.uint;
 }
 
-static int qemu_opt_parse(QemuOpt *opt)
+static void qemu_opt_parse(QemuOpt *opt, Error **errp)
 {
-Error *local_err = NULL;
-
 if (opt->desc == NULL)
-return 0;
+return;
 
 switch (opt->desc->type) {
 case QEMU_OPT_STRING:
 /* nothing */
-return 0;
+return;
 case QEMU_OPT_BOOL:
-parse_option_bool(opt->name, opt->str, &opt->value.boolean, 
&local_err);
+parse_option_bool(opt->name, opt->str, &opt->value.boolean, errp);
 break;
 case QEMU_OPT_NUMBER:
-parse_option_number(opt->name, opt->str, &opt->value.uint,
-&local_err);
+parse_option_number(opt->name, opt->str, &opt->value.uint, errp);
 break;
 case QEMU_OPT_SIZE:
-parse_option_size(opt->name, opt->str, &opt->value.uint, &local_err);
+parse_option_size(opt->name, opt->str, &opt->value.uint, errp);
 break;
 default:
 abort();
 }
-
-if (error_is_set(&local_err)) {
-qerror_report_err(local_err);
-error_free(local_err);
-return -1;
-}
-
-return 0;
 }
 
 static void qemu_opt_del(QemuOpt *opt)
@@ -631,6 +620,7 @@ static int opt_set(QemuOpts *opts, const char *name, const 
char *value,
 {
 QemuOpt *opt;
 const QemuOptDesc *desc = opts->list->desc;
+Error *local_err = NULL;
 int i;
 
 for (i = 0; desc[i].name != NULL; i++) {
@@ -642,8 +632,8 @@ static int opt_set(QemuOpts *opts, const char *name, const 
char *value,
 if (i == 0) {
 /* empty list -> allow any */;
 } else {
-qerror_report(QERR_INVALID_PARAMETER, name);
-return -1;
+error_set(&local_err, QERR_INVALID_PARAMETER, name);
+goto exit_err;
 }
 }
 
@@ -661,11 +651,18 @@ static int opt_set(QemuOpts *opts, const char *name, 
const char *value,
 if (value) {
 opt->str = g_strdup(value);
 }
-if (qemu_opt_parse(opt) < 0) {
+qemu_opt_parse(opt, &local_err);
+if (error_is_set(&local_err)) {
 qemu_opt_del(opt);
-return -1;
+goto exit_err;
 }
+
 return 0;
+
+exit_err:
+qerror_report_err(local_err);
+error_free(local_err);
+return -1;
 }
 
 int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
@@ -1053,6 +1050,7 @@ QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict)
 int qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc)
 {
 QemuOpt *opt;
+Error *local_err = NULL;
 
 assert(opts->list->desc[0].name == NULL);
 
@@ -1065,18 +1063,24 @@ int qemu_opts_validate(QemuOpts *opts, const 
QemuOptDesc *desc)
 }
 }
 if (desc[i].name == NULL) {
-qerror_report(QERR_INVALID_PARAMETER, opt->name);
-return -1;
+error_set(&local_err, QERR_INVALID_PARAMETER, opt->name);
+goto exit_err;
 }
 
 opt->desc = &desc[i];
 
-if (qemu_opt_parse(opt) < 0) {
-return -1;
+qemu_opt_parse(opt, &local_err);
+if (error_is_set(&local_err)) {
+goto exit_err;
 }
 }
 
 return 0;
+
+exit_err:
+qerror_report_err(local_err);
+error_free(local_err);
+return -1;
 }
 
 int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void 
*opaque,
-- 
1.7.9.2.384.g4a92a




[Qemu-devel] [PATCH 12/18] qemu-config: find_list(): use error_set()

2012-04-17 Thread Luiz Capitulino
Note that qemu_find_opts() callers still expect automatic error reporting
with QError, so qemu_find_opts() calls qerror_report_err() to keep the
same semantics.

Signed-off-by: Luiz Capitulino 
---
 qemu-config.c |   32 +---
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index f876646..bdb381d 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -3,6 +3,7 @@
 #include "qemu-option.h"
 #include "qemu-config.h"
 #include "hw/qdev.h"
+#include "error.h"
 
 static QemuOptsList qemu_drive_opts = {
 .name = "drive",
@@ -631,7 +632,8 @@ static QemuOptsList *vm_config_groups[32] = {
 NULL,
 };
 
-static QemuOptsList *find_list(QemuOptsList **lists, const char *group)
+static QemuOptsList *find_list(QemuOptsList **lists, const char *group,
+   Error **errp)
 {
 int i;
 
@@ -640,14 +642,23 @@ static QemuOptsList *find_list(QemuOptsList **lists, 
const char *group)
 break;
 }
 if (lists[i] == NULL) {
-error_report("there is no option group \"%s\"", group);
+error_set(errp, QERR_INVALID_OPTION_GROUP, group);
 }
 return lists[i];
 }
 
 QemuOptsList *qemu_find_opts(const char *group)
 {
-return find_list(vm_config_groups, group);
+QemuOptsList *ret;
+Error *local_err = NULL;
+
+ret = find_list(vm_config_groups, group, &local_err);
+if (error_is_set(&local_err)) {
+error_report("%s\n", error_get_pretty(local_err));
+error_free(local_err);
+}
+
+return ret;
 }
 
 void qemu_add_opts(QemuOptsList *list)
@@ -762,6 +773,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const 
char *fname)
 char line[1024], group[64], id[64], arg[64], value[1024];
 Location loc;
 QemuOptsList *list = NULL;
+Error *local_err = NULL;
 QemuOpts *opts = NULL;
 int res = -1, lno = 0;
 
@@ -778,17 +790,23 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, 
const char *fname)
 }
 if (sscanf(line, "[%63s \"%63[^\"]\"]", group, id) == 2) {
 /* group with id */
-list = find_list(lists, group);
-if (list == NULL)
+list = find_list(lists, group, &local_err);
+if (error_is_set(&local_err)) {
+error_report("%s\n", error_get_pretty(local_err));
+error_free(local_err);
 goto out;
+}
 opts = qemu_opts_create(list, id, 1, NULL);
 continue;
 }
 if (sscanf(line, "[%63[^]]]", group) == 1) {
 /* group without id */
-list = find_list(lists, group);
-if (list == NULL)
+list = find_list(lists, group, &local_err);
+if (error_is_set(&local_err)) {
+error_report("%s\n", error_get_pretty(local_err));
+error_free(local_err);
 goto out;
+}
 opts = qemu_opts_create(list, NULL, 0, NULL);
 continue;
 }
-- 
1.7.9.2.384.g4a92a




[Qemu-devel] [PATCH 13/18] qemu-config: introduce qemu_find_opts_err()

2012-04-17 Thread Luiz Capitulino
This is like qemu_find_opts(), except that it takes an Error argument.

This new function allows for a incremental conversion of code using
qemu_find_opts().

Signed-off-by: Luiz Capitulino 
---
 qemu-config.c |5 +
 qemu-config.h |3 +++
 2 files changed, 8 insertions(+)

diff --git a/qemu-config.c b/qemu-config.c
index bdb381d..bb3bff4 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -661,6 +661,11 @@ QemuOptsList *qemu_find_opts(const char *group)
 return ret;
 }
 
+QemuOptsList *qemu_find_opts_err(const char *group, Error **errp)
+{
+return find_list(vm_config_groups, group, errp);
+}
+
 void qemu_add_opts(QemuOptsList *list)
 {
 int entries, i;
diff --git a/qemu-config.h b/qemu-config.h
index 20d707f..a093e3f 100644
--- a/qemu-config.h
+++ b/qemu-config.h
@@ -1,11 +1,14 @@
 #ifndef QEMU_CONFIG_H
 #define QEMU_CONFIG_H
 
+#include "error.h"
+
 extern QemuOptsList qemu_fsdev_opts;
 extern QemuOptsList qemu_virtfs_opts;
 extern QemuOptsList qemu_spice_opts;
 
 QemuOptsList *qemu_find_opts(const char *group);
+QemuOptsList *qemu_find_opts_err(const char *group, Error **errp);
 void qemu_add_opts(QemuOptsList *list);
 int qemu_set_option(const char *str);
 int qemu_global_option(const char *str);
-- 
1.7.9.2.384.g4a92a




[Qemu-devel] [PATCH 16/18] net: purge the monitor object from all init functions

2012-04-17 Thread Luiz Capitulino
The only backend that really uses it is the socket one, which calls
monitor_get_fd(). But it can use 'cur_mon' instead.

Signed-off-by: Luiz Capitulino 
---
 hw/pci-hotplug.c |2 +-
 hw/usb/dev-network.c |2 +-
 net.c|   18 +++---
 net.h|2 +-
 net/dump.c   |2 +-
 net/dump.h   |3 +--
 net/slirp.c  |5 +
 net/slirp.h  |5 +
 net/socket.c |8 +++-
 net/socket.h |3 +--
 net/tap-win32.c  |2 +-
 net/tap.c|9 -
 net/tap.h|5 ++---
 net/vde.c|2 +-
 14 files changed, 26 insertions(+), 42 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index c55d8b9..785eb3d 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -60,7 +60,7 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
 
 qemu_opt_set(opts, "type", "nic");
 
-ret = net_client_init(mon, opts, 0);
+ret = net_client_init(opts, 0);
 if (ret < 0)
 return NULL;
 if (nd_table[ret].devaddr) {
diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index cff55f2..6b89e66 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -1366,7 +1366,7 @@ static USBDevice *usb_net_init(USBBus *bus, const char 
*cmdline)
 qemu_opt_set(opts, "type", "nic");
 qemu_opt_set(opts, "model", "usb");
 
-idx = net_client_init(NULL, opts, 0);
+idx = net_client_init(opts, 0);
 if (idx == -1) {
 return NULL;
 }
diff --git a/net.c b/net.c
index f5d9cc7..8e9ca16 100644
--- a/net.c
+++ b/net.c
@@ -745,10 +745,7 @@ int net_handle_fd_param(Monitor *mon, const char *param)
 return fd;
 }
 
-static int net_init_nic(QemuOpts *opts,
-Monitor *mon,
-const char *name,
-VLANState *vlan)
+static int net_init_nic(QemuOpts *opts, const char *name, VLANState *vlan)
 {
 int idx;
 NICInfo *nd;
@@ -821,7 +818,6 @@ static int net_init_nic(QemuOpts *opts,
  }
 
 typedef int (*net_client_init_func)(QemuOpts *opts,
-Monitor *mon,
 const char *name,
 VLANState *vlan);
 
@@ -1085,7 +1081,7 @@ static const struct {
 #endif /* CONFIG_NET_BRIDGE */
 };
 
-int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev)
+int net_client_init(QemuOpts *opts, int is_netdev)
 {
 const char *name;
 const char *type;
@@ -1156,7 +1152,7 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int 
is_netdev)
 
 ret = 0;
 if (net_client_types[i].init) {
-ret = net_client_types[i].init(opts, mon, name, vlan);
+ret = net_client_types[i].init(opts, name, vlan);
 if (ret < 0) {
 /* TODO push error reporting into init() methods */
 qerror_report(QERR_DEVICE_INIT_FAILED, type);
@@ -1213,7 +1209,7 @@ void net_host_device_add(Monitor *mon, const QDict *qdict)
 
 qemu_opt_set(opts, "type", device);
 
-if (net_client_init(mon, opts, 0) < 0) {
+if (net_client_init(opts, 0) < 0) {
 monitor_printf(mon, "adding host network device %s failed\n", device);
 }
 }
@@ -1245,7 +1241,7 @@ int do_netdev_add(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
 return -1;
 }
 
-res = net_client_init(mon, opts, 1);
+res = net_client_init(opts, 1);
 if (res < 0) {
 qemu_opts_del(opts);
 }
@@ -1428,14 +1424,14 @@ void net_check_clients(void)
 
 static int net_init_client(QemuOpts *opts, void *dummy)
 {
-if (net_client_init(NULL, opts, 0) < 0)
+if (net_client_init(opts, 0) < 0)
 return -1;
 return 0;
 }
 
 static int net_init_netdev(QemuOpts *opts, void *dummy)
 {
-return net_client_init(NULL, opts, 1);
+return net_client_init(opts, 1);
 }
 
 int net_init_clients(void)
diff --git a/net.h b/net.h
index 64993b4..9d1ed93 100644
--- a/net.h
+++ b/net.h
@@ -163,7 +163,7 @@ struct HCIInfo *qemu_next_hci(void);
 extern const char *legacy_tftp_prefix;
 extern const char *legacy_bootp_filename;
 
-int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev);
+int net_client_init(QemuOpts *opts, int is_netdev);
 int net_client_parse(QemuOptsList *opts_list, const char *str);
 int net_init_clients(void);
 void net_check_clients(void);
diff --git a/net/dump.c b/net/dump.c
index 4b48d48..f835c51 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -144,7 +144,7 @@ static int net_dump_init(VLANState *vlan, const char 
*device,
 return 0;
 }
 
-int net_init_dump(QemuOpts *opts, Monitor *mon, const char *name, VLANState 
*vlan)
+int net_init_dump(QemuOpts *opts, const char *name, VLANState *vlan)
 {
 int len;
 const char *file;
diff --git a/net/dump.h b/net/dump.h
index fdc91ad..2b5d9ba 100644
--- a/net/dump.h
+++ b/net/dump.h
@@ -27,7 +27,6 @@
 #include "net.h"
 #include "qemu-common.h

[Qemu-devel] [PATCH 17/18] qapi: convert netdev_add

2012-04-17 Thread Luiz Capitulino
Signed-off-by: Anthony Liguori 
Signed-off-by: Luiz Capitulino 
---
 hmp-commands.hx  |3 +-
 hmp.c|   33 +++
 hmp.h|1 +
 hw/pci-hotplug.c |8 +++--
 hw/usb/dev-network.c |7 ++--
 net.c|   89 --
 net.h|3 +-
 qapi-schema.json |   28 
 qmp-commands.hx  |5 +--
 9 files changed, 141 insertions(+), 36 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index a6f5a84..7381395 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1009,8 +1009,7 @@ ETEXI
 .args_type  = "netdev:O",
 .params = "[user|tap|socket],id=str[,prop=value][,...]",
 .help   = "add host network device",
-.user_print = monitor_user_noop,
-.mhandler.cmd_new = do_netdev_add,
+.mhandler.cmd = hmp_netdev_add,
 },
 
 STEXI
diff --git a/hmp.c b/hmp.c
index f3e5163..2b78f8b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -943,3 +943,36 @@ void hmp_device_del(Monitor *mon, const QDict *qdict)
 qmp_device_del(id, &err);
 hmp_handle_error(mon, &err);
 }
+
+void hmp_netdev_add(Monitor *mon, const QDict *qdict)
+{
+const char *type = qdict_get_try_str(qdict, "type");
+const char *id = qdict_get_try_str(qdict, "id");
+QString *arglist = qstring_new();
+bool has_arglist = false;
+const QDictEntry *entry;
+Error *local_err = NULL;
+
+for (entry = qdict_first(qdict); entry;
+ entry = qdict_next(qdict, entry)) {
+const char *key = qdict_entry_key(entry);
+const char *value = 
qstring_get_str(qobject_to_qstring(qdict_entry_value(entry)));
+
+if (strcmp(key, "type") == 0 || strcmp(key, "id") == 0) {
+continue;
+}
+
+if (qstring_len(arglist) > 0) {
+qstring_append(arglist, ",");
+}
+
+qstring_append(arglist, key);
+qstring_append(arglist, "=");
+qstring_append(arglist, value);
+has_arglist = true;
+}
+
+qmp_netdev_add(type, id, has_arglist, qstring_get_str(arglist), 
&local_err);
+hmp_handle_error(mon, &local_err);
+QDECREF(arglist);
+}
diff --git a/hmp.h b/hmp.h
index 443b812..2fc041a 100644
--- a/hmp.h
+++ b/hmp.h
@@ -61,5 +61,6 @@ void hmp_block_job_set_speed(Monitor *mon, const QDict 
*qdict);
 void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
 void hmp_migrate(Monitor *mon, const QDict *qdict);
 void hmp_device_del(Monitor *mon, const QDict *qdict);
+void hmp_netdev_add(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 785eb3d..61257f4 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -39,6 +39,7 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
const char *devaddr,
const char *opts_str)
 {
+Error *local_err = NULL;
 QemuOpts *opts;
 PCIBus *bus;
 int ret, devfn;
@@ -60,9 +61,12 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
 
 qemu_opt_set(opts, "type", "nic");
 
-ret = net_client_init(opts, 0);
-if (ret < 0)
+ret = net_client_init(opts, 0, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
 return NULL;
+}
 if (nd_table[ret].devaddr) {
 monitor_printf(mon, "Parameter addr not supported\n");
 return NULL;
diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index 6b89e66..9dad76c 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -1355,6 +1355,7 @@ static int usb_net_initfn(USBDevice *dev)
 
 static USBDevice *usb_net_init(USBBus *bus, const char *cmdline)
 {
+Error *local_err = NULL;
 USBDevice *dev;
 QemuOpts *opts;
 int idx;
@@ -1366,8 +1367,10 @@ static USBDevice *usb_net_init(USBBus *bus, const char 
*cmdline)
 qemu_opt_set(opts, "type", "nic");
 qemu_opt_set(opts, "model", "usb");
 
-idx = net_client_init(opts, 0);
-if (idx == -1) {
+idx = net_client_init(opts, 0, &local_err);
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
 return NULL;
 }
 
diff --git a/net.c b/net.c
index 8e9ca16..c758a7c 100644
--- a/net.c
+++ b/net.c
@@ -1081,7 +1081,7 @@ static const struct {
 #endif /* CONFIG_NET_BRIDGE */
 };
 
-int net_client_init(QemuOpts *opts, int is_netdev)
+int net_client_init(QemuOpts *opts, int is_netdev, Error **errp)
 {
 const char *name;
 const char *type;
@@ -1089,7 +1089,7 @@ int net_client_init(QemuOpts *opts, int is_netdev)
 
 type = qemu_opt_get(opts, "type");
 if (!type) {
-qerror_report(QERR_MISSING_PARAMETER, "type");
+error_set(errp, QERR_MISSING_PARAMETER, "type");
 return -1;
 }
 
@@ -1105,21 +1105,21 @@ int net_client_init(QemuOpts *opts, int is_netdev)
 strcmp(type, "vde") !

[Qemu-devel] [PATCH 14/18] qstring: introduce qstring_len()

2012-04-17 Thread Luiz Capitulino
Signed-off-by: Luiz Capitulino 
---
 qstring.c |9 +
 qstring.h |1 +
 tests/check-qstring.c |   17 +
 3 files changed, 27 insertions(+)

diff --git a/qstring.c b/qstring.c
index b7e12e4..b1bf975 100644
--- a/qstring.c
+++ b/qstring.c
@@ -32,6 +32,15 @@ QString *qstring_new(void)
 }
 
 /**
+ * qstring_len(): Get string length
+ *
+ */
+size_t qstring_len(const QString *qstring)
+{
+return qstring->length;
+}
+
+/**
  * qstring_from_substr(): Create a new QString from a C string substring
  *
  * Return string reference
diff --git a/qstring.h b/qstring.h
index 84ccd96..d1c8a07 100644
--- a/qstring.h
+++ b/qstring.h
@@ -24,6 +24,7 @@ typedef struct QString {
 } QString;
 
 QString *qstring_new(void);
+size_t qstring_len(const QString *qstring);
 QString *qstring_from_str(const char *str);
 QString *qstring_from_substr(const char *str, int start, int end);
 const char *qstring_get_str(const QString *qstring);
diff --git a/tests/check-qstring.c b/tests/check-qstring.c
index addad6c..8e99c62 100644
--- a/tests/check-qstring.c
+++ b/tests/check-qstring.c
@@ -81,6 +81,22 @@ static void qstring_from_substr_test(void)
 QDECREF(qs);
 }
 
+static void qstring_len_test(void)
+{
+const char *str = "virtualization";
+QString *qs;
+
+qs = qstring_new();
+g_assert(qstring_len(qs) == 0);
+
+qstring_append(qs, "QEMU");
+g_assert(qstring_len(qs) == 4);
+QDECREF(qs);
+
+qs = qstring_from_str(str);
+g_assert(qstring_len(qs) == strlen(str));
+QDECREF(qs);
+}
 
 static void qobject_to_qstring_test(void)
 {
@@ -101,6 +117,7 @@ int main(int argc, char **argv)
 g_test_add_func("/public/get_str", qstring_get_str_test);
 g_test_add_func("/public/append_chr", qstring_append_chr_test);
 g_test_add_func("/public/from_substr", qstring_from_substr_test);
+g_test_add_func("/public/len", qstring_len_test);
 g_test_add_func("/public/to_qstring", qobject_to_qstring_test);
 
 return g_test_run();
-- 
1.7.9.2.384.g4a92a




[Qemu-devel] [PATCH 03/18] qemu-option: parse_option_bool(): use error_set()

2012-04-17 Thread Luiz Capitulino
Note that set_option_parameter() callers still expect automatic error
reporting with QError, so set_option_parameter() calls
qerror_report_err() to keep the same semantics.

Signed-off-by: Luiz Capitulino 
---
 qemu-option.c |   22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 72dcb56..a8b50af 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -169,7 +169,8 @@ QEMUOptionParameter 
*get_option_parameter(QEMUOptionParameter *list,
 return NULL;
 }
 
-static int parse_option_bool(const char *name, const char *value, bool *ret)
+static void parse_option_bool(const char *name, const char *value, bool *ret,
+  Error **errp)
 {
 if (value != NULL) {
 if (!strcmp(value, "on")) {
@@ -177,13 +178,11 @@ static int parse_option_bool(const char *name, const char 
*value, bool *ret)
 } else if (!strcmp(value, "off")) {
 *ret = 0;
 } else {
-qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "'on' or 'off'");
-return -1;
+error_set(errp,QERR_INVALID_PARAMETER_VALUE, name, "'on' or 
'off'");
 }
 } else {
 *ret = 1;
 }
-return 0;
 }
 
 static void parse_option_number(const char *name, const char *value,
@@ -263,6 +262,7 @@ int set_option_parameter(QEMUOptionParameter *list, const 
char *name,
 const char *value)
 {
 bool flag;
+Error *local_err = NULL;
 
 // Find a matching parameter
 list = get_option_parameter(list, name);
@@ -274,8 +274,10 @@ int set_option_parameter(QEMUOptionParameter *list, const 
char *name,
 // Process parameter
 switch (list->type) {
 case OPT_FLAG:
-if (parse_option_bool(name, value, &flag) == -1)
-return -1;
+parse_option_bool(name, value, &flag, &local_err);
+if (error_is_set(&local_err)) {
+goto exit_err;
+}
 list->value.n = flag;
 break;
 
@@ -299,6 +301,11 @@ int set_option_parameter(QEMUOptionParameter *list, const 
char *name,
 }
 
 return 0;
+
+exit_err:
+qerror_report_err(local_err);
+error_free(local_err);
+return -1;
 }
 
 /*
@@ -588,7 +595,8 @@ static int qemu_opt_parse(QemuOpt *opt)
 /* nothing */
 return 0;
 case QEMU_OPT_BOOL:
-return parse_option_bool(opt->name, opt->str, &opt->value.boolean);
+parse_option_bool(opt->name, opt->str, &opt->value.boolean, 
&local_err);
+break;
 case QEMU_OPT_NUMBER:
 parse_option_number(opt->name, opt->str, &opt->value.uint,
 &local_err);
-- 
1.7.9.2.384.g4a92a




[Qemu-devel] [PATCH 02/18] qemu-option: parse_option_number(): use error_set()

2012-04-17 Thread Luiz Capitulino
Note that qemu_opt_parse() callers still expect automatic error reporting
with QError, so qemu_opts_parse() calls qerror_report_err() to keep the
same semantics.

Signed-off-by: Luiz Capitulino 
---
 qemu-option.c |   26 +++---
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 9f531c8..72dcb56 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -186,7 +186,8 @@ static int parse_option_bool(const char *name, const char 
*value, bool *ret)
 return 0;
 }
 
-static int parse_option_number(const char *name, const char *value, uint64_t 
*ret)
+static void parse_option_number(const char *name, const char *value,
+uint64_t *ret, Error **errp)
 {
 char *postfix;
 uint64_t number;
@@ -194,15 +195,13 @@ static int parse_option_number(const char *name, const 
char *value, uint64_t *re
 if (value != NULL) {
 number = strtoull(value, &postfix, 0);
 if (*postfix != '\0') {
-qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a number");
-return -1;
+error_set(errp, QERR_INVALID_PARAMETER_VALUE, name, "a number");
+return;
 }
 *ret = number;
 } else {
-qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a number");
-return -1;
+error_set(errp, QERR_INVALID_PARAMETER_VALUE, name, "a number");
 }
-return 0;
 }
 
 static int parse_option_size(const char *name, const char *value, uint64_t 
*ret)
@@ -579,8 +578,11 @@ uint64_t qemu_opt_get_size(QemuOpts *opts, const char 
*name, uint64_t defval)
 
 static int qemu_opt_parse(QemuOpt *opt)
 {
+Error *local_err = NULL;
+
 if (opt->desc == NULL)
 return 0;
+
 switch (opt->desc->type) {
 case QEMU_OPT_STRING:
 /* nothing */
@@ -588,12 +590,22 @@ static int qemu_opt_parse(QemuOpt *opt)
 case QEMU_OPT_BOOL:
 return parse_option_bool(opt->name, opt->str, &opt->value.boolean);
 case QEMU_OPT_NUMBER:
-return parse_option_number(opt->name, opt->str, &opt->value.uint);
+parse_option_number(opt->name, opt->str, &opt->value.uint,
+&local_err);
+break;
 case QEMU_OPT_SIZE:
 return parse_option_size(opt->name, opt->str, &opt->value.uint);
 default:
 abort();
 }
+
+if (error_is_set(&local_err)) {
+qerror_report_err(local_err);
+error_free(local_err);
+return -1;
+}
+
+return 0;
 }
 
 static void qemu_opt_del(QemuOpt *opt)
-- 
1.7.9.2.384.g4a92a




[Qemu-devel] [PATCH 00/18]: qapi: convert netdev_add & netdev_del

2012-04-17 Thread Luiz Capitulino
As device_add depends on some qom stuff, I've switched to netdev_add/del as
most patches are common.

Will appreciate your review, specially the qemu-option changes (which are
boring and thus easy to make mistakes).


 blockdev.c   |2 +-
 hmp-commands.hx  |6 +-
 hmp.c|   42 +++
 hmp.h|2 +
 hw/pci-hotplug.c |8 ++-
 hw/usb/dev-network.c |7 +-
 hw/usb/dev-storage.c |2 +-
 hw/watchdog.c|2 +-
 net.c|  110 +++--
 net.h|4 +-
 net/dump.c   |2 +-
 net/dump.h   |3 +-
 net/slirp.c  |5 +-
 net/slirp.h  |5 +-
 net/socket.c |8 +--
 net/socket.h |3 +-
 net/tap-win32.c  |2 +-
 net/tap.c|9 ++-
 net/tap.h|5 +-
 net/vde.c|2 +-
 qapi-schema.json |   42 +++
 qemu-char.c  |   20 --
 qemu-config.c|   43 +---
 qemu-config.h|3 +
 qemu-option.c|  173 ++
 qemu-option.h|   11 ++-
 qemu-sockets.c   |8 +--
 qerror.c |4 ++
 qerror.h |3 +
 qmp-commands.hx  |   10 +--
 qstring.c|9 +++
 qstring.h|1 +
 scripts/qapi-commands.py |   91 
 scripts/qapi.py  |2 +
 tests/check-qstring.c|   17 +
 vl.c |   22 +++---
 36 files changed, 514 insertions(+), 174 deletions(-)



Re: [Qemu-devel] [PATCH] arm-dis: Fix spelling in comments (iff -> if)

2012-04-17 Thread Peter Maydell
On 17 April 2012 19:22, Andreas Färber  wrote:
> Am 17.04.2012 19:58, schrieb Stefan Weil:
>> The spelling 'iff' is sometimes used for 'if and only if'.
>> Even if that meaning could be applied here, it is not used
>> consistently. It is also quite unusual to use 'if and only if'
>> in technical documentation. Therefore a simple 'if' should be
>> preferred here.
>>
>> Signed-off-by: Stefan Weil 
>> ---
>>  arm-dis.c |   22 +++---
>>  1 files changed, 11 insertions(+), 11 deletions(-)
>
> Aren't the *-dis.c files imported from binutils? In that case I'd
> suggest to stay in sync with how they write/wrote it.

Yes, arm-dis.c is from binutils. These comments still read 'iff'
in current binutils, as they have done since first introduced
in binutils 2.5 in 1994.

-- PMM



Re: [Qemu-devel] [PATCH] arm-dis: Fix spelling in comments (iff -> if)

2012-04-17 Thread Andreas Färber
Am 17.04.2012 19:58, schrieb Stefan Weil:
> The spelling 'iff' is sometimes used for 'if and only if'.
> Even if that meaning could be applied here, it is not used
> consistently. It is also quite unusual to use 'if and only if'
> in technical documentation. Therefore a simple 'if' should be
> preferred here.
> 
> Signed-off-by: Stefan Weil 
> ---
>  arm-dis.c |   22 +++---
>  1 files changed, 11 insertions(+), 11 deletions(-)

Aren't the *-dis.c files imported from binutils? In that case I'd
suggest to stay in sync with how they write/wrote it.
(Would spelling fixes in comments be subject to the GPLv3?)

Andreas

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



[Qemu-devel] [PATCH BUILD FIX 2/2] xen: add a dummy xc_hvm_inject_msi for Xen < 4.2

2012-04-17 Thread Stefano Stabellini
xc_hvm_inject_msi is only available on Xen >= 4.2: add a dummy
compatibility function for Xen < 4.2.

Also enable msi support only on Xen >= 4.2.

Signed-off-by: Stefano Stabellini 
Acked-by: Anthony PERARD 
---
 hw/pc.c |2 +-
 hw/xen.h|   10 ++
 hw/xen_common.h |   15 +++
 xen-all.c   |2 +-
 4 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 1f5aacb..4d34a33 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -916,7 +916,7 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
 msi_supported = true;
 }
 
-if (xen_enabled()) {
+if (xen_msi_support()) {
 msi_supported = true;
 }
 
diff --git a/hw/xen.h b/hw/xen.h
index e5926b7..3ae4cd0 100644
--- a/hw/xen.h
+++ b/hw/xen.h
@@ -57,4 +57,14 @@ void xen_register_framebuffer(struct MemoryRegion *mr);
 #  define HVM_MAX_VCPUS 32
 #endif
 
+static inline int xen_msi_support(void)
+{
+#if defined(CONFIG_XEN_CTRL_INTERFACE_VERSION) \
+&& CONFIG_XEN_CTRL_INTERFACE_VERSION >= 420
+return xen_enabled();
+#else
+return 0;
+#endif
+}
+
 #endif /* QEMU_HW_XEN_H */
diff --git a/hw/xen_common.h b/hw/xen_common.h
index 0409ac7..7043c14 100644
--- a/hw/xen_common.h
+++ b/hw/xen_common.h
@@ -133,6 +133,21 @@ static inline int xc_fd(xc_interface *xen_xc)
 }
 #endif
 
+/* Xen before 4.2 */
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 420
+static inline int xen_xc_hvm_inject_msi(XenXC xen_xc, domid_t dom,
+uint64_t addr, uint32_t data)
+{
+return -ENOSYS;
+}
+#else
+static inline int xen_xc_hvm_inject_msi(XenXC xen_xc, domid_t dom,
+uint64_t addr, uint32_t data)
+{
+return xc_hvm_inject_msi(xen_xc, dom, addr, data);
+}
+#endif
+
 void destroy_hvm_domain(void);
 
 #endif /* QEMU_HW_XEN_COMMON_H */
diff --git a/xen-all.c b/xen-all.c
index a08eec0..bdf9c0f 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -129,7 +129,7 @@ void xen_piix_pci_write_config_client(uint32_t address, 
uint32_t val, int len)
 
 void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
 {
-xc_hvm_inject_msi(xen_xc, xen_domid, addr, data);
+xen_xc_hvm_inject_msi(xen_xc, xen_domid, addr, data);
 }
 
 static void xen_suspend_notifier(Notifier *notifier, void *data)
-- 
1.7.2.5




[Qemu-devel] [PATCH BUILD FIX 1/2] xen,configure: detect Xen 4.2

2012-04-17 Thread Stefano Stabellini
Xen 4.2 is the first to support xc_hvm_inject_msi: use it to determine
if we are running on it.

Signed-off-by: Stefano Stabellini 
Acked-by: Anthony PERARD 
---
 configure |   25 +
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 1d94acd..38f45f3 100755
--- a/configure
+++ b/configure
@@ -1398,6 +1398,31 @@ int main(void) {
   xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
   xc_gnttab_open(NULL, 0);
   xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
+  xc_hvm_inject_msi(xc, 0, 0xf000, 0x);
+  return 0;
+}
+EOF
+  compile_prog "" "$xen_libs"
+) ; then
+xen_ctrl_version=420
+xen=yes
+
+  elif (
+  cat > $TMPC <
+#include 
+#include 
+#include 
+#if !defined(HVM_MAX_VCPUS)
+# error HVM_MAX_VCPUS not defined
+#endif
+int main(void) {
+  xc_interface *xc;
+  xs_daemon_open();
+  xc = xc_interface_open(0, 0, 0);
+  xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
+  xc_gnttab_open(NULL, 0);
+  xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
   return 0;
 }
 EOF
-- 
1.7.2.5




[Qemu-devel] [PATCH BUILD FIX 0/2] build xc_hvm_inject_msi on Xen < 4.2

2012-04-17 Thread Stefano Stabellini
Hi all,
this small patch series fixes the build breakage introduced by
f1dbf015dfb0aa7f66f710a1f1bc58b662951de2 with Xen < 4.2.

The problem is that xc_hvm_inject_msi is only defined from Xen 4.2
onwards so we need to provide a compatibility function for older Xen
versions.


Stefano Stabellini (2):
  xen,configure: detect Xen 4.2
  xen: add a dummy xc_hvm_inject_msi for Xen < 4.2

 configure   |   25 +
 hw/pc.c |2 +-
 hw/xen.h|   10 ++
 hw/xen_common.h |   15 +++
 xen-all.c   |2 +-
 5 files changed, 52 insertions(+), 2 deletions(-)


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

Cheers,

Stefano



[Qemu-devel] [PATCH] arm-dis: Fix spelling in comments (iff -> if)

2012-04-17 Thread Stefan Weil
The spelling 'iff' is sometimes used for 'if and only if'.
Even if that meaning could be applied here, it is not used
consistently. It is also quite unusual to use 'if and only if'
in technical documentation. Therefore a simple 'if' should be
preferred here.

Signed-off-by: Stefan Weil 
---
 arm-dis.c |   22 +++---
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arm-dis.c b/arm-dis.c
index 6bc4d71..db15eca 100644
--- a/arm-dis.c
+++ b/arm-dis.c
@@ -130,8 +130,8 @@ struct opcode16
%zprint a double precision VFP reg
  Codes: 0=>Dm, 1=>Dd, 2=>Dn, 3=>multi-list
 
-   %'c   print specified char iff bitfield is all ones
-   %`c   print specified char iff bitfield is all zeroes
+   %'c   print specified char if bitfield is all ones
+   %`c   print specified char if bitfield is all zeroes
%?ab...select from array of values in big endian order
 
%L  print as an iWMMXt N/M width field.
@@ -522,8 +522,8 @@ static const struct opcode32 coprocessor_opcodes[] =
%Tn   print short scaled width limited by n
%Un   print long scaled width limited by n
 
-   %'c   print specified char iff bitfield is all ones
-   %`c   print specified char iff bitfield is all zeroes
+   %'c   print specified char if bitfield is all ones
+   %`c   print specified char if bitfield is all zeroes
%?ab...select from array of values in big endian order  */
 
 static const struct opcode32 neon_opcodes[] =
@@ -787,8 +787,8 @@ static const struct opcode32 neon_opcodes[] =
%c  print condition code (always bits 28-31)
%m  print register mask for ldm/stm instruction
%o  print operand2 (immediate or register + shift)
-   %p  print 'p' iff bits 12-15 are 15
-   %t  print 't' iff bit 21 set and bit 24 clear
+   %p  print 'p' if bits 12-15 are 15
+   %t  print 't' if bit 21 set and bit 24 clear
%B  print arm BLX(1) destination
%C  print the PSR sub type.
%U  print barrier type.
@@ -800,8 +800,8 @@ static const struct opcode32 neon_opcodes[] =
%xprint the bitfield in hex
%Xprint the bitfield as 1 hex digit without 
leading "0x"
 
-   %'c   print specified char iff bitfield is all ones
-   %`c   print specified char iff bitfield is all zeroes
+   %'c   print specified char if bitfield is all ones
+   %`c   print specified char if bitfield is all zeroes
%?ab...select from array of values in big endian order
 
%e   print arm SMI operand (bits 0..7,8..19).
@@ -1090,7 +1090,7 @@ static const struct opcode32 arm_opcodes[] =
%a print (bitfield * 4) as a pc-rel offset + decoded 
symbol
%B print Thumb branch destination (signed displacement)
%c print bitfield as a condition code
-   %'c print specified char iff bit is one
+   %'c print specified char if bit is one
%?abprint a if bit is one else print b.  */
 
 static const struct opcode16 thumb_opcodes[] =
@@ -1248,8 +1248,8 @@ static const struct opcode16 thumb_opcodes[] =
%rprint bitfield as an ARM register
%cprint bitfield as a condition code
 
-   %'c   print specified char iff bitfield is all ones
-   %`c   print specified char iff bitfield is all zeroes
+   %'c   print specified char if bitfield is all ones
+   %`c   print specified char if bitfield is all zeroes
%?ab... select from array of values in big endian order
 
With one exception at the bottom (done because BL and BLX(1) need
-- 
1.7.9




Re: [Qemu-devel] [Bug 891625] Re: [qemu-kvm] add vhost-net to kvm group udev rules 65-kvm.rules

2012-04-17 Thread Alon Bar-Lev
Well, at Gentoo we have kvm group, and I think that this comes from
upstream rule.
Doing vhost_net is good, anything that may be assigned to regular users.
Thanks!

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

Title:
  [qemu-kvm] add vhost-net to kvm group udev rules 65-kvm.rules

Status in QEMU:
  New

Bug description:
  Please consider authorizing the kvm group to access vhost-net device, similar 
to the kvm device.
  Thanks!

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



[Qemu-devel] [Bug 891625] Re: [qemu-kvm] add vhost-net to kvm group udev rules 65-kvm.rules

2012-04-17 Thread Michael Tokarev
I wonder if it really belongs to kvm group, -- maybe a separate
"vhost_net" group should be used instead.  Yes it can only be used with
qemu/kvm currently, but maybe some other tool will use it in the future,
and looking at how many security issues /dev/kvm access had, maybe
vhost_net shold be restricted more...

How other distributions are doing this?  I'm not sure we want to
introduce our own naming here...

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

Title:
  [qemu-kvm] add vhost-net to kvm group udev rules 65-kvm.rules

Status in QEMU:
  New

Bug description:
  Please consider authorizing the kvm group to access vhost-net device, similar 
to the kvm device.
  Thanks!

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



[Qemu-devel] [PATCH] block: Fix spelling in comment (ineffcient -> inefficient)

2012-04-17 Thread Stefan Weil
Signed-off-by: Stefan Weil 
---
 block/cow.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/block/cow.c b/block/cow.c
index 8d3c9f8..a5a00eb 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -103,7 +103,7 @@ static int cow_open(BlockDriverState *bs, int flags)
 }
 
 /*
- * XXX(hch): right now these functions are extremely ineffcient.
+ * XXX(hch): right now these functions are extremely inefficient.
  * We should just read the whole bitmap we'll need in one go instead.
  */
 static inline int cow_set_bit(BlockDriverState *bs, int64_t bitnum)
-- 
1.7.9




Re: [Qemu-devel] [PATCH] tci: GETPC() macro must return an uintptr_t

2012-04-17 Thread Eric Blake
On 04/17/2012 11:22 AM, Stefan Weil wrote:
> Change the data type of tci_tb_ptr, so GETPC() returns an
> uintptr_t now (like for all other TCG targets).
> 
> This completes commit 2050396801ca0c8359364d61eaadece951006057
> and fixes builds with TCI.
> 
> Signed-off-by: Stefan Weil 
> ---
>  exec-all.h |2 +-
>  tci.c  |4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/exec-all.h b/exec-all.h
> index a963fd4..585d44f 100644
> --- a/exec-all.h
> +++ b/exec-all.h
> @@ -283,7 +283,7 @@ extern int tb_invalidated_flag;
>  /* Alpha and SH4 user mode emulations and Softmmu call GETPC().
> For all others, GETPC remains undefined (which makes TCI a little faster. 
> */
>  # if defined(CONFIG_SOFTMMU) || defined(TARGET_ALPHA) || defined(TARGET_SH4)
> -extern void *tci_tb_ptr;
> +extern uintptr_t tci_tb_ptr;
>  #  define GETPC() tci_tb_ptr

Nice way to side-step the entire () in macro issue :)

Reviewed-by: Eric Blake 

-- 
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 2/2] qemu-ga: fix help output

2012-04-17 Thread Luiz Capitulino
On Tue, 17 Apr 2012 12:07:39 -0500
Michael Roth  wrote:

> 
> Signed-off-by: Michael Roth 

Reviewed-by: Luiz Capitulino 

> ---
>  qemu-ga.c |7 ---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/qemu-ga.c b/qemu-ga.c
> index d6f786e..74a1b02 100644
> --- a/qemu-ga.c
> +++ b/qemu-ga.c
> @@ -117,12 +117,13 @@ static gboolean register_signal_handlers(void)
>  static void usage(const char *cmd)
>  {
>  printf(
> -"Usage: %s -c \n"
> +"Usage: %s [-m  -p ] []\n"
>  "QEMU Guest Agent %s\n"
>  "\n"
>  "  -m, --method  transport method: one of unix-listen, virtio-serial, 
> or\n"
>  "isa-serial (virtio-serial is the default)\n"
> -"  -p, --pathdevice/socket path (%s is the default for 
> virtio-serial)\n"
> +"  -p, --pathdevice/socket path (the default for virtio-serial is:\n"
> +"%s)\n"
>  "  -l, --logfile set logfile path, logs to stderr by default\n"
>  "  -f, --pidfile specify pidfile (default is %s)\n"
>  "  -v, --verbose log extra debugging information\n"
> @@ -131,7 +132,7 @@ static void usage(const char *cmd)
>  #ifdef _WIN32
>  "  -s, --service service commands: install, uninstall\n"
>  #endif
> -"  -b, --blacklist   comma-separated list of RPCs to disable (no spaces, 
> \"?\""
> +"  -b, --blacklist   comma-separated list of RPCs to disable (no spaces, 
> \"?\"\n"
>  "to list available RPCs)\n"
>  "  -h, --helpdisplay this help and exit\n"
>  "\n"




Re: [Qemu-devel] [PATCH 1/2] qemu-ga: generate missing stubs for fsfreeze

2012-04-17 Thread Luiz Capitulino
On Tue, 17 Apr 2012 12:07:38 -0500
Michael Roth  wrote:

> When linux-specific commands (including guest-fsfreeze-*) were consolidated
> under defined(__linux__), we forgot to account for the case where
> defined(__linux__) && !defined(FIFREEZE). As a result stubs are no longer
> being generated on linux hosts that don't have FIFREEZE support. Fix
> this.
> 
> Tested-by: Andreas Färber 
> Signed-off-by: Michael Roth 

Reviewed-by: Luiz Capitulino 

> ---
>  qga/commands-posix.c |   36 
>  1 files changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index faf970d..087c3af 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -881,46 +881,50 @@ error:
>  
>  #else /* defined(__linux__) */
>  
> -GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
> +void qmp_guest_suspend_disk(Error **err)
>  {
>  error_set(err, QERR_UNSUPPORTED);
> -
> -return 0;
>  }
>  
> -int64_t qmp_guest_fsfreeze_freeze(Error **err)
> +void qmp_guest_suspend_ram(Error **err)
>  {
>  error_set(err, QERR_UNSUPPORTED);
> -
> -return 0;
>  }
>  
> -int64_t qmp_guest_fsfreeze_thaw(Error **err)
> +void qmp_guest_suspend_hybrid(Error **err)
>  {
>  error_set(err, QERR_UNSUPPORTED);
> -
> -return 0;
>  }
>  
> -void qmp_guest_suspend_disk(Error **err)
> +GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
>  {
> -error_set(err, QERR_UNSUPPORTED);
> +error_set(errp, QERR_UNSUPPORTED);
> +return NULL;
>  }
>  
> -void qmp_guest_suspend_ram(Error **err)
> +#endif
> +
> +#if !defined(CONFIG_FSFREEZE)
> +
> +GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
>  {
>  error_set(err, QERR_UNSUPPORTED);
> +
> +return 0;
>  }
>  
> -void qmp_guest_suspend_hybrid(Error **err)
> +int64_t qmp_guest_fsfreeze_freeze(Error **err)
>  {
>  error_set(err, QERR_UNSUPPORTED);
> +
> +return 0;
>  }
>  
> -GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
> +int64_t qmp_guest_fsfreeze_thaw(Error **err)
>  {
> -error_set(errp, QERR_UNSUPPORTED);
> -return NULL;
> +error_set(err, QERR_UNSUPPORTED);
> +
> +return 0;
>  }
>  
>  #endif




Re: [Qemu-devel] [PATCH 00/12 v11] introducing a new, dedicated guest memory dump mechanism

2012-04-17 Thread Luiz Capitulino
On Mon, 26 Mar 2012 17:58:46 +0800
Wen Congyang  wrote:

> Hi, all
> 
> 'virsh dump' can not work when host pci device is used by guest. We have
> discussed this issue here:
> http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg00736.html

No more review comments, will assume this is good enough.

Wen, can you post v12? I've lost the correct ordering due to the two
additional patches you posted.



[Qemu-devel] [PATCH] tci: GETPC() macro must return an uintptr_t

2012-04-17 Thread Stefan Weil
Change the data type of tci_tb_ptr, so GETPC() returns an
uintptr_t now (like for all other TCG targets).

This completes commit 2050396801ca0c8359364d61eaadece951006057
and fixes builds with TCI.

Signed-off-by: Stefan Weil 
---
 exec-all.h |2 +-
 tci.c  |4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/exec-all.h b/exec-all.h
index a963fd4..585d44f 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -283,7 +283,7 @@ extern int tb_invalidated_flag;
 /* Alpha and SH4 user mode emulations and Softmmu call GETPC().
For all others, GETPC remains undefined (which makes TCI a little faster. */
 # if defined(CONFIG_SOFTMMU) || defined(TARGET_ALPHA) || defined(TARGET_SH4)
-extern void *tci_tb_ptr;
+extern uintptr_t tci_tb_ptr;
 #  define GETPC() tci_tb_ptr
 # endif
 #elif defined(__s390__) && !defined(__s390x__)
diff --git a/tci.c b/tci.c
index 3abb52f..23a8368 100644
--- a/tci.c
+++ b/tci.c
@@ -58,7 +58,7 @@ CPUArchState *env;
 /* Targets which don't use GETPC also don't need tci_tb_ptr
which makes them a little faster. */
 #if defined(GETPC)
-void *tci_tb_ptr;
+uintptr_t tci_tb_ptr;
 #endif
 
 static tcg_target_ulong tci_reg[TCG_TARGET_NB_REGS];
@@ -450,7 +450,7 @@ tcg_target_ulong tcg_qemu_tb_exec(CPUArchState *cpustate, 
uint8_t *tb_ptr)
 
 for (;;) {
 #if defined(GETPC)
-tci_tb_ptr = tb_ptr;
+tci_tb_ptr = (uintptr_t)tb_ptr;
 #endif
 TCGOpcode opc = tb_ptr[0];
 #if !defined(NDEBUG)
-- 
1.7.0.4




Re: [Qemu-devel] [PATCH] qapi: g_hash_table_find() instead of GHashTableIter.

2012-04-17 Thread Luiz Capitulino
On Sat, 14 Apr 2012 00:01:49 +0900
"NODA, Kai"  wrote:

> From: "NODA, Kai" 
> 
> GHashTableIter was first introduced in glib 2.16.
> This patch removes it in favor of older g_hash_table_find()
> for better compatibility with RHEL5.

Missing signed-off, otherwise looks good.

> ---
>  qapi/qmp-input-visitor.c |   25 +
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
> index 74386b9..4cdc47d 100644
> --- a/qapi/qmp-input-visitor.c
> +++ b/qapi/qmp-input-visitor.c
> @@ -87,20 +87,29 @@ static void qmp_input_push(QmpInputVisitor *qiv, QObject 
> *obj, Error **errp)
>  qiv->nb_stack++;
>  }
>  
> +/** Only for qmp_input_pop. */
> +static gboolean always_true(gpointer key, gpointer val, gpointer user_pkey)
> +{
> +*(const char **)user_pkey = (const char *)key;
> +return TRUE;
> +}
> +
>  static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
>  {
> -GHashTableIter iter;
> -gpointer key;
> +assert(qiv->nb_stack > 0);
>  
> -if (qiv->strict && qiv->stack[qiv->nb_stack - 1].h) {
> -g_hash_table_iter_init(&iter, qiv->stack[qiv->nb_stack - 1].h);
> -if (g_hash_table_iter_next(&iter, &key, NULL)) {
> -error_set(errp, QERR_QMP_EXTRA_MEMBER, (char *) key);
> +if (qiv->strict) {
> +GHashTable * const top_ht = qiv->stack[qiv->nb_stack - 1].h;
> +if (top_ht) {
> +if (g_hash_table_size(top_ht)) {
> +const char *key;
> +g_hash_table_find(top_ht, always_true, &key);
> +error_set(errp, QERR_QMP_EXTRA_MEMBER, key);
> +}
> +g_hash_table_unref(top_ht);
>  }
> -g_hash_table_unref(qiv->stack[qiv->nb_stack - 1].h);
>  }
>  
> -assert(qiv->nb_stack > 0);
>  qiv->nb_stack--;
>  }
>  




Re: [Qemu-devel] [PATCH] tci: Use uintptr_t for the GETPC() macro

2012-04-17 Thread Stefan Weil

Am 17.04.2012 19:02, schrieb Eric Blake:

On 04/17/2012 10:49 AM, Stefan Weil wrote:

Am 17.04.2012 18:40, schrieb Eric Blake:

On 04/17/2012 10:36 AM, Stefan Weil wrote:

This completes commit 2050396801ca0c8359364d61eaadece951006057
and fixes builds with TCI.

Signed-off-by: Stefan Weil
---
exec-all.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/exec-all.h b/exec-all.h
index a963fd4..e766f9d 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -284,7 +284,7 @@ extern int tb_invalidated_flag;
For all others, GETPC remains undefined (which makes TCI a
little faster. */
# if defined(CONFIG_SOFTMMU) || defined(TARGET_ALPHA) ||
defined(TARGET_SH4)
extern void *tci_tb_ptr;
-# define GETPC() tci_tb_ptr
+# define GETPC() (uintptr_t)tci_tb_ptr

Missing (). Any time you add a cast to a macro, you need to fully
parenthesize the entire expression.


This is a good rule in general, but not needed here, because
tci_tb_ptr is not a macro argument nor a macro!


Not true. You _don't_ know how GETPC() will be used in context in
future code, and there are operators with higher precedence than
casting. Consider:

GETPC()++ is silently accepted by your code, but with proper parentheses:

#define GETPC() ((uintptr_t)tci_tb_ptr)

then GETPC()++ will be a compilation error.


tci_tb_ptr is a void pointer. Therefore the ++ operator will either
raise a compilation error or gcc will handle it as if it were a
char pointer. In that case the code would even work :-)

Are there other operators with higher precedence which might
be critical?

Nevertheless I see your point, so I'll send a new patch which
works without any doubt.





Re: [Qemu-devel] [PATCH 35/46] qemu-iotests: qcow2.py

2012-04-17 Thread Philipp Hahn
Hello,

On Thursday 05 April 2012 17:52:13 Kevin Wolf wrote:
> This adds a tool that is meant to inspect and edit qcow2 files in a
> low-level way, that wouldn't be possible with qemu-img/io, for example
> by adding yet unknown extensions or flags. This way we can test whether
> qemu deals properly with future backwards compatible extensions.

Cool. I wrote a very similar tool to dump qcow2 data some time ago when I 
tried to debug a ref-count corruption bug. I'll attach it just FYI.

BYtE
Philipp
-- 
Philipp Hahn   Open Source Software Engineer  h...@univention.de
Univention GmbHbe open.   fon: +49 421 22 232- 0
Mary-Somerville-Str.1  D-28359 Bremen fax: +49 421 22 232-99
   http://www.univention.de/


qcow2.py
Description: application/python


signature.asc
Description: This is a digitally signed message part.


[Qemu-devel] [PATCH 1/2] qemu-ga: generate missing stubs for fsfreeze

2012-04-17 Thread Michael Roth
When linux-specific commands (including guest-fsfreeze-*) were consolidated
under defined(__linux__), we forgot to account for the case where
defined(__linux__) && !defined(FIFREEZE). As a result stubs are no longer
being generated on linux hosts that don't have FIFREEZE support. Fix
this.

Tested-by: Andreas Färber 
Signed-off-by: Michael Roth 
---
 qga/commands-posix.c |   36 
 1 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index faf970d..087c3af 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -881,46 +881,50 @@ error:
 
 #else /* defined(__linux__) */
 
-GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
+void qmp_guest_suspend_disk(Error **err)
 {
 error_set(err, QERR_UNSUPPORTED);
-
-return 0;
 }
 
-int64_t qmp_guest_fsfreeze_freeze(Error **err)
+void qmp_guest_suspend_ram(Error **err)
 {
 error_set(err, QERR_UNSUPPORTED);
-
-return 0;
 }
 
-int64_t qmp_guest_fsfreeze_thaw(Error **err)
+void qmp_guest_suspend_hybrid(Error **err)
 {
 error_set(err, QERR_UNSUPPORTED);
-
-return 0;
 }
 
-void qmp_guest_suspend_disk(Error **err)
+GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
 {
-error_set(err, QERR_UNSUPPORTED);
+error_set(errp, QERR_UNSUPPORTED);
+return NULL;
 }
 
-void qmp_guest_suspend_ram(Error **err)
+#endif
+
+#if !defined(CONFIG_FSFREEZE)
+
+GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
 {
 error_set(err, QERR_UNSUPPORTED);
+
+return 0;
 }
 
-void qmp_guest_suspend_hybrid(Error **err)
+int64_t qmp_guest_fsfreeze_freeze(Error **err)
 {
 error_set(err, QERR_UNSUPPORTED);
+
+return 0;
 }
 
-GuestNetworkInterfaceList *qmp_guest_network_get_interfaces(Error **errp)
+int64_t qmp_guest_fsfreeze_thaw(Error **err)
 {
-error_set(errp, QERR_UNSUPPORTED);
-return NULL;
+error_set(err, QERR_UNSUPPORTED);
+
+return 0;
 }
 
 #endif
-- 
1.7.4.1




[Qemu-devel] [PATCH 2/2] qemu-ga: fix help output

2012-04-17 Thread Michael Roth

Signed-off-by: Michael Roth 
---
 qemu-ga.c |7 ---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/qemu-ga.c b/qemu-ga.c
index d6f786e..74a1b02 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -117,12 +117,13 @@ static gboolean register_signal_handlers(void)
 static void usage(const char *cmd)
 {
 printf(
-"Usage: %s -c \n"
+"Usage: %s [-m  -p ] []\n"
 "QEMU Guest Agent %s\n"
 "\n"
 "  -m, --method  transport method: one of unix-listen, virtio-serial, or\n"
 "isa-serial (virtio-serial is the default)\n"
-"  -p, --pathdevice/socket path (%s is the default for 
virtio-serial)\n"
+"  -p, --pathdevice/socket path (the default for virtio-serial is:\n"
+"%s)\n"
 "  -l, --logfile set logfile path, logs to stderr by default\n"
 "  -f, --pidfile specify pidfile (default is %s)\n"
 "  -v, --verbose log extra debugging information\n"
@@ -131,7 +132,7 @@ static void usage(const char *cmd)
 #ifdef _WIN32
 "  -s, --service service commands: install, uninstall\n"
 #endif
-"  -b, --blacklist   comma-separated list of RPCs to disable (no spaces, \"?\""
+"  -b, --blacklist   comma-separated list of RPCs to disable (no spaces, 
\"?\"\n"
 "to list available RPCs)\n"
 "  -h, --helpdisplay this help and exit\n"
 "\n"
-- 
1.7.4.1




Re: [Qemu-devel] [PATCH] tci: Use uintptr_t for the GETPC() macro

2012-04-17 Thread Eric Blake
On 04/17/2012 10:49 AM, Stefan Weil wrote:
> Am 17.04.2012 18:40, schrieb Eric Blake:
>> On 04/17/2012 10:36 AM, Stefan Weil wrote:
>>> This completes commit 2050396801ca0c8359364d61eaadece951006057
>>> and fixes builds with TCI.
>>>
>>> Signed-off-by: Stefan Weil
>>> ---
>>>   exec-all.h |2 +-
>>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/exec-all.h b/exec-all.h
>>> index a963fd4..e766f9d 100644
>>> --- a/exec-all.h
>>> +++ b/exec-all.h
>>> @@ -284,7 +284,7 @@ extern int tb_invalidated_flag;
>>>  For all others, GETPC remains undefined (which makes TCI a
>>> little faster. */
>>>   # if defined(CONFIG_SOFTMMU) || defined(TARGET_ALPHA) ||
>>> defined(TARGET_SH4)
>>>   extern void *tci_tb_ptr;
>>> -#  define GETPC() tci_tb_ptr
>>> +#  define GETPC() (uintptr_t)tci_tb_ptr
>> Missing ().  Any time you add a cast to a macro, you need to fully
>> parenthesize the entire expression.
> 
> This is a good rule in general, but not needed here, because
> tci_tb_ptr is not a macro argument nor a macro!

Not true.  You _don't_ know how GETPC() will be used in context in
future code, and there are operators with higher precedence than
casting.  Consider:

GETPC()++ is silently accepted by your code, but with proper parentheses:

#define GETPC() ((uintptr_t)tci_tb_ptr)

then GETPC()++ will be a compilation error.

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



signature.asc
Description: OpenPGP digital signature


  1   2   3   >