[Qemu-devel] [PATCH] configure: Earlier pkg-config probe

2012-12-16 Thread Stefan Weil
Probe pkg-config before it is used for the first time (libseccomp check).

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

diff --git a/configure b/configure
index ecdb33a..728caca 100755
--- a/configure
+++ b/configure
@@ -1360,6 +1360,14 @@ esac
 fi
 
 ##
+# pkg-config probe
+
+if ! has $pkg_config_exe; then
+  echo Error: pkg-config binary '$pkg_config_exe' not found
+  exit 1
+fi
+
+##
 # NPTL probe
 
 if test $nptl != no ; then
@@ -1590,14 +1598,6 @@ if test $xen_pci_passthrough != no; then
 fi
 
 ##
-# pkg-config probe
-
-if ! has $pkg_config_exe; then
-  echo Error: pkg-config binary '$pkg_config_exe' not found
-  exit 1
-fi
-
-##
 # libtool probe
 
 if ! has $libtool; then
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH v2] build: Use separate makefile for trace/

2012-12-16 Thread Paolo Bonzini
Il 14/12/2012 20:13, Lluís Vilanova ha scritto:
 Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu
 --
 Changes in v2:
 
 * Do not depend on qemu-timer-common.o.
 * Use $(obj) in rules to refer to the build sub-directory.
 * Remove dependencies against $(GENERATED_HEADERS).
 
 Cc: Paolo Bonzini pbonz...@redhat.com
 ---
  .gitignore  |8 ++--
  Makefile|   15 +---
  Makefile.objs   |   64 ++--
  scripts/tracetool/backend/dtrace.py |2 +
  scripts/tracetool/format/h.py   |6 ++-
  trace.h |6 +++
  trace/Makefile.objs |   70 
 +++
  7 files changed, 96 insertions(+), 75 deletions(-)
  create mode 100644 trace.h
  create mode 100644 trace/Makefile.objs
 
 diff --git a/.gitignore b/.gitignore
 index bd6ba1c..0b75915 100644
 --- a/.gitignore
 +++ b/.gitignore
 @@ -2,10 +2,10 @@ config-devices.*
  config-all-devices.*
  config-host.*
  config-target.*
 -trace.h
 -trace.c
 -trace-dtrace.h
 -trace-dtrace.dtrace
 +trace/generated-tracers.h
 +trace/generated-tracers.c
 +trace/generated-tracers-dtrace.h
 +trace/generated-tracers-dtrace.dtrace
  *-timestamp
  *-softmmu
  *-darwin-user
 diff --git a/Makefile b/Makefile
 index e9d6848..21a7912 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -31,12 +31,15 @@ ifneq ($(filter-out %clean,$(MAKECMDGOALS)),$(if 
 $(MAKECMDGOALS),,fail))
  endif
  endif
  
 -GENERATED_HEADERS = config-host.h trace.h qemu-options.def
 +GENERATED_HEADERS = config-host.h qemu-options.def
 +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
 +GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c
 +
 +GENERATED_HEADERS += trace/generated-tracers.h
  ifeq ($(TRACE_BACKEND),dtrace)
 -GENERATED_HEADERS += trace-dtrace.h
 +GENERATED_HEADERS += trace/generated-tracers-dtrace.h
  endif
 -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
 -GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c trace.c
 +GENERATED_SOURCES += trace/generated-tracers.c
  
  # Don't try to regenerate Makefile or configure
  # We don't generate any of them
 @@ -252,9 +255,9 @@ clean:
   rm -f *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~
   rm -Rf .libs
   rm -f qemu-img-cmds.h
 - rm -f trace-dtrace.dtrace trace-dtrace.dtrace-timestamp
   @# May not be present in GENERATED_HEADERS
 - rm -f trace-dtrace.h trace-dtrace.h-timestamp
 + rm -f trace/generated-tracers-dtrace.dtrace*
 + rm -f trace/generated-tracers-dtrace.h*
   rm -f $(foreach f,$(GENERATED_HEADERS),$(f) $(f)-timestamp)
   rm -f $(foreach f,$(GENERATED_SOURCES),$(f) $(f)-timestamp)
   rm -rf qapi-generated
 diff --git a/Makefile.objs b/Makefile.objs
 index 3c7abca..24832a2 100644
 --- a/Makefile.objs
 +++ b/Makefile.objs
 @@ -147,66 +147,7 @@ libdis-$(CONFIG_LM32_DIS) += lm32-dis.o
  ##
  # trace
  
 -ifeq ($(TRACE_BACKEND),dtrace)
 -TRACE_H_EXTRA_DEPS=trace-dtrace.h
 -endif
 -trace.h: trace.h-timestamp $(TRACE_H_EXTRA_DEPS)
 -trace.h-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
 - $(call quiet-command,$(TRACETOOL) \
 - --format=h \
 - --backend=$(TRACE_BACKEND) \
 -  $  $@,  GEN   trace.h)
 - @cmp -s $@ trace.h || cp $@ trace.h
 -
 -trace.c: trace.c-timestamp
 -trace.c-timestamp: $(SRC_PATH)/trace-events $(BUILD_DIR)/config-host.mak
 - $(call quiet-command,$(TRACETOOL) \
 - --format=c \
 - --backend=$(TRACE_BACKEND) \
 -  $  $@,  GEN   trace.c)
 - @cmp -s $@ trace.c || cp $@ trace.c
 -
 -trace.o: trace.c $(GENERATED_HEADERS)
 -
 -trace-dtrace.h: trace-dtrace.dtrace
 - $(call quiet-command,dtrace -o $@ -h -s $,   GEN   trace-dtrace.h)
 -
 -# Normal practice is to name DTrace probe file with a '.d' extension
 -# but that gets picked up by QEMU's Makefile as an external dependency
 -# rule file. So we use '.dtrace' instead
 -trace-dtrace.dtrace: trace-dtrace.dtrace-timestamp
 -trace-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events 
 $(BUILD_DIR)/config-host.mak
 - $(call quiet-command,$(TRACETOOL) \
 - --format=d \
 - --backend=$(TRACE_BACKEND) \
 -  $  $@,  GEN   trace-dtrace.dtrace)
 - @cmp -s $@ trace-dtrace.dtrace || cp $@ trace-dtrace.dtrace
 -
 -trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS)
 - $(call quiet-command,dtrace -o $@ -G -s $,   GEN   trace-dtrace.o)
 -
 -ifeq ($(LIBTOOL),)
 -trace-dtrace.lo: trace-dtrace.dtrace
 - @echo missing libtool. please install and rerun configure.; exit 1
 -else
 -trace-dtrace.lo: trace-dtrace.dtrace
 - $(call quiet-command,$(LIBTOOL) --mode=compile --tag=CC dtrace -o $@ -G 
 -s $,   lt GEN trace-dtrace.o)
 -endif
 -
 -trace/simple.o: trace/simple.c $(GENERATED_HEADERS)
 -
 

Re: [Qemu-devel] big wait check in ram_save_iterate()

2012-12-16 Thread Paolo Bonzini
Il 15/12/2012 12:20, Peter Lieven ha scritto:
 Am 15.12.2012 10:02, schrieb Paolo Bonzini:

 is the check for spending  50ms in the loop still necessary in qemu
 1.3.0?

 Yes, it helps finding the available bandwidth and tuning the downtime of
 migration.
 
 Aha, okay. But then it should also be added to block migration, shoudn't it?

The main problem with block migration is that it uses the same strategy
for RAM and storage migration, but the two are very different.

In RAM migration, you are basically fighting an uphill battle.  You
cannot do RAM migration without some amount of downtime, because CPUs
are always going to be faster than the migration process and latency is
zero.

But in storage migration you have the same constraints as the guest.
Disk is slow for the guest too (and as slow as network, or slower).  The
guest needs to flush to disk, while the migration process does not need
to do that.  And migration can proceed asynchronously, just like the
guest, with multiple in-flight operations at the same time.

drive-mirror can do all this much better than migration, and without
slowing down RAM migration (which occurs on a separate socket).  See the
drive-mirror patches that I posted recently for 1.4.  With those
patches, I can sync the destination to the source every 100 ms during a
kernel compilation, and every 1-2 seconds during hibernation (the
absolute worst case) using iSCSI for both the source and the target and
a middle-range SAS disk.  This is a far cry from what RAM migration can
achieve on any reasonable workload, and it's the reason why I'm pushing
for a separate mechanism than the migrate command.

libvirt has had patches posted to support this too.

Paolo

 I tested it in qemu-kvm 1.2.0 and it help to increase responsiveness if the
 I/O latency on the storage is high.




[Qemu-devel] [Bug 1071236] Re: creating qcow2 image with preallocation fails if size =4G

2012-12-16 Thread Michael Tokarev
This is fixed by commit  a3548077062dd9dc2701ebffd931ba6eaef40bec which
is included in 1.3 release of qemu.

** Changed in: qemu
   Status: Confirmed = Fix Released

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

Title:
  creating qcow2 image with preallocation fails if size =4G

Status in QEMU:
  Fix Released

Bug description:
  Steps to reproduce:
1. run qemu-img create -f qcow2 -o cluster_size=512,preallocation=metadata 
disk.img 4G

  Reproducible:
Always

  Configuration:
Gentoo Linux 3.4.9, 64b
latest qemu available from portage (afaik this is pulled from the git repo)

  Possible workarounds:
1. ommit preallocation=
2. increase cluster_size
3. reduce the image size

  Although workarounds are available, it is still a bug that could (and
  should) be fixed.

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



[Qemu-devel] [Bug 1090837] Re: Error in building Qemu-1.3.0 on Solaris 10

2012-12-16 Thread Paolo Bonzini
Are you using /bin/sh (broken) or /usr/xpg4/bin/sh (should work)?

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

Title:
  Error in building Qemu-1.3.0 on Solaris 10

Status in QEMU:
  New

Bug description:
  While trying to build Qemu on Oracle Solaris 10 (SPARC processor), I
  encountered the following error in the configure step:

  ./configure --prefix=/usr/local/ --install=/usr/ucb/install
  ./configure: bad substitution
  ./configure: !: not found
  ./configure: !: not found
  ./configure: !: not found
  ./configure: !: not found
  ./configure: !: not found
  ./configure: curl-config: not found
  ./configure: curl-config: not found

  As the following bug report says:
  https://bugs.launchpad.net/qemu/+bug/636315, sh is hard-coded in the
  script. Can't the script be modified to accept a $SHELL argument to
  make use of bash or other shell during configure and make step?

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



Re: [Qemu-devel] [PULL] pci,net,misc infrastructure

2012-12-16 Thread Michael S. Tsirkin
On Thu, Dec 13, 2012 at 02:31:53PM -0600, Anthony Liguori wrote:
 Michael S. Tsirkin m...@redhat.com writes:
 
  The following changes since commit 1c97e303d4ea80a2691334b0febe87a50660f99d:
 
Merge remote-tracking branch 'afaerber/qom-cpu' into staging (2012-12-10 
  08:35:15 -0600)
 
  are available in the git repository at:
 
 
git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_anthony
 
  for you to fetch changes up to 5c1ad98d71923b83a530e5db4c2110564b84e11d:
 
pci_bus.h: tweak include guards (2012-12-12 23:41:04 +0200)
 
 Doesn't build:
 
   CChw/apm.o
 /home/anthony/git/qemu/hw/apm.c:25:17: fatal error: pci.h: No such file or 
 directory
 compilation terminated.
 
 And there's really no good reason for this.  apm's part of target-i386
 so you couldn't have built this prior to doing a pull request.
 
 Please setup your tree in buildbot and wait for a full run before
 sending a pull requests in the future.
 
 Regards,
 
 Anthony Liguori

Ugh.
I'm not sure but I think I see how this happened: I re-run make after
each commit, before the last one apm.c did find the header it depends
on, the last commit only removes the -I flag to find the header
but neither the header itself not the C file changed so it didn't
need to rebuild.

-- 
MST



Re: [Qemu-devel] [PULL] pci,net,misc infrastructure

2012-12-16 Thread Michael S. Tsirkin
On Fri, Dec 14, 2012 at 09:37:17PM +, Blue Swirl wrote:
 Perhaps the use of '-' vs. '_' in file names could be unified while
 renaming. I think most new files use dash.

I can do this in a follow up patch but first let's put this rule in
coding style file. As it is more files use _ than -.
And I'm not sure what's the point of using dash, as opposed
to underscore: underscore seems more consistent.

-- 
MST



Re: [Qemu-devel] [PATCH 2/3] s390: Add CPU reset handler

2012-12-16 Thread Andreas Färber
Am 14.12.2012 17:46, schrieb Jens Freimann:
 Add a CPU reset handler to have all CPUs in a PoP compliant
 state.
 
 Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com

The logic looks okay now. Some comments inline.

 ---
 v2 - v3: 
 * explain in comment which code sets cpu 0 to running during IPL
 
 v1 - v2:
 * move setting of control registers and psa to s390_cpu_reset
   and call it from the new s390_machine_cpu_reset_cb()
   This makes it more similar to how it is done on x86
 * in s390_cpu_reset() set env-halted state of cpu after
   the memset. This is needed to keep our s390_cpu_running
   counter in sync when s390_cpu_reset is called via the
   qemu_devices_reset path
 * set env-halted state in s390_cpu_initfn to 1 to avoid
   decrementing the cpu counter during first reset
 ---
  target-s390x/cpu.c | 32 ++--
  target-s390x/kvm.c |  9 -
  2 files changed, 38 insertions(+), 3 deletions(-)
 
 diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
 index 619b202..75d4036 100644
 --- a/target-s390x/cpu.c
 +++ b/target-s390x/cpu.c
 @@ -4,6 +4,7 @@
   * Copyright (c) 2009 Ulrich Hecht
   * Copyright (c) 2011 Alexander Graf
   * Copyright (c) 2012 SUSE LINUX Products GmbH
 + * Copyright (c) 2012 IBM Corp.
   *
   * This library is free software; you can redistribute it and/or
   * modify it under the terms of the GNU Lesser General Public
 @@ -18,9 +19,13 @@
   * You should have received a copy of the GNU Lesser General Public
   * License along with this library; if not, see
   * http://www.gnu.org/licenses/lgpl-2.1.html
 + * Contributions after 2012-12-11 are licensed under the terms of the
 + * GNU GPL, version 2 or (at your option) any later version.
 + *
   */
  
  #include cpu.h
 +#include hw/hw.h
  #include qemu-common.h
  #include qemu-timer.h
  
 @@ -37,12 +42,30 @@ static void s390_cpu_reset(CPUState *s)
  log_cpu_state(env, 0);
  }
  
 -scc-parent_reset(s);
 +s390_del_running_cpu(env);
  
 +scc-parent_reset(s);

If this gets respun, a white line separating the parent reset from the
local reset would be nice. :)

  memset(env, 0, offsetof(CPUS390XState, breakpoints));
 +
 +/* architectured initial values for CR 0 and 14 */
 +env-cregs[0] = 0xE0UL;
 +env-cregs[14] = 0xC200UL;
 +/* set to z/Architecture mode */
 +env-psw.mask = 0x00018000ULL;
 +env-psa = 0;
 +/* set halted to 1 to make sure we can add the cpu in
 + * s390_ipl_cpu code, where env-halted is set back to 0
 + * after incrementing the cpu counter */
 +env-halted = 1;
  /* FIXME: reset vector? */

Do the above added cregs/psw/psa reset values resolve this FIXME? Or
does that refer to something different?

  tlb_flush(env, 1);
 -s390_add_running_cpu(env);
 +}
 +
 +static void s390_cpu_machine_reset_cb(void *opaque)
 +{
 +S390CPU *cpu = opaque;
 +
 +cpu_reset(CPU(cpu));
  }
  
  static void s390_cpu_initfn(Object *obj)
 @@ -66,7 +89,12 @@ static void s390_cpu_initfn(Object *obj)
  env-cpu_num = cpu_num++;
  env-ext_index = -1;
  
 +/* set env-halted state to 1 to avoid decrementing the running
 + * cpu counter in s390_cpu_reset to a negative number at 
 + * initial ipl */
 +env-halted = 1;
  cpu_reset(CPU(cpu));
 +qemu_register_reset(s390_cpu_machine_reset_cb, cpu);

Since we register the reset handler in instance_init, we should
unregister it in a instance_finalize callback (uninitfn?). Since we do
not hot-unplug s390 CPUs yet to my knowledge, that could be done in a
follow-up.

(For x86 it it registered in the provisional realize function and not
unregistered lacking a matching unrealization mechanism today; elsewhere
reset registration is done in the machine.)

  }
  
  static void s390_cpu_class_init(ObjectClass *oc, void *data)
 diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
 index 94de764..fda9f1f 100644
 --- a/target-s390x/kvm.c
 +++ b/target-s390x/kvm.c
 @@ -85,7 +85,14 @@ int kvm_arch_init_vcpu(CPUS390XState *env)
  
  void kvm_arch_reset_vcpu(CPUS390XState *env)
  {

Note to Alex: In my upcoming KVM CPUState series, this argument type
changes to CPUState ...

 -/* FIXME: add code to reset vcpu. */
 +   /* The initial reset call is needed here to reset in-kernel
 +* vcpu data that we can't access directly from QEMU
 +* (i.e. with older kernels which don't support sync_regs/ONE_REG).
 +* Before this ioctl cpu_synchronize_state() is called in common kvm
 +* code (kvm-all) */
 +if (kvm_vcpu_ioctl(env, KVM_S390_INITIAL_RESET, NULL)) {

... but so does the argument for kvm_vcpu_ioctl(), so merging becomes a
trivial env - cpu change.

 +perror(Can't reset vcpu\n);
 +}
  }
  
  int kvm_arch_put_registers(CPUS390XState *env, int level)

Andreas

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



Re: [Qemu-devel] [PULL] pci,net,misc infrastructure

2012-12-16 Thread Michael S. Tsirkin
 Please setup your tree in buildbot and wait for a full run before
 sending a pull requests in the future.

I actually waited 24 hours which is normally enough for
buildbot to run the tree, and assumed pass means OK.
Turns out buildbot was passing, but only because it was building
some old commits:

http://buildbot.b1-systems.de/qemu/builders/pci_x86_64_debian_6_0/builds/480

Built f1219091edd20e3b92544025c2b6dd5e4d98b61b from May on Dec 15.
I don't understand why: both the pci branch and the for_anthony
tag on my kernel.org tree are updated.



Re: [Qemu-devel] [PATCH 3/3] S390: Enable -cpu help and QMP query-cpu-definitions

2012-12-16 Thread Andreas Färber
Am 14.12.2012 17:46, schrieb Jens Freimann:
 From: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com
 
 This enables qemu -cpu help to return a list of supported CPU models
 on s390 and also to query for cpu definitions in the monitor.
 Initially only cpu model = host is returned. This needs to be reworked
 into a full-fledged CPU model handling later on.
 This change is needed to allow libvirt exploiters (like OpenStack)
 to specify a CPU model.
 
 Signed-off-by: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com
 Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
 Reviewed-by: Christian Borntraeger borntrae...@de.ibm.com

I guess we can live with this solution for now,

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

Can you follow-up with a strcmp(cpu_model, host) check for
!kvm_enabled() in cpu_s390x_init() please?

Thanks,
Andreas

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



Re: [Qemu-devel] [RFC PATCH v7 7/8] virtio-pci-blk : Switch to new API.

2012-12-16 Thread Michael S. Tsirkin
On Thu, Dec 13, 2012 at 08:51:30AM -0600, Anthony Liguori wrote:
 Paolo Bonzini pbonz...@redhat.com writes:
 
  Il 12/12/2012 18:58, Peter Maydell ha scritto:
  It should be equally
  valid to just use the PCI transport plugged into a VirtioDevice,
  both of which were created by the user with -device [and for
  new transports, separate transport and backend should be the
  standard]. That means the virtio-bus interface needs a way for
  the backend to announce to the transport what it is so that
  the PCI transport can set the right PCI IDs.
 
  There is such an interface (the device_id, aka VIRTIO_ID_*).  Then
  virtio-pci needs a mapping from the device_id to the (default)
  vendor_id/device_id/class tuple.
 
 Why?
 
 I think it's perfectly fine to have to specify a device ID for
 virtio-pci.
 
 The way virtio-pci is designed is such that every device that uses
 virtio-pci ends up looking like an independent PCI device.
 
 We should always have virtio-blk-pci, virtio-net-pci, etc.  The goal of
 this refactoring should not be to eliminate that.
 
 But these devices should be trivial to implement and modelled in a sane
 way.
 
 I don't think we should be trying to solve the problem of making
 virtio-pci easy to use.  Why would you say:
 
   -device virtio-pci,id=foo -device virtio-blk,bus=foo
 
 When you can just say:
 
   -device virtio-pci-blk
 
 I think we're optimizing for the wrong thing here...
 
 Regards,
 
 Anthony Liguori
 
 
  Paolo


I agree. What I mean is this: virtio has device IDs.
These are not pci specific and are defined in linux/virtio_ids.h
Now it looks like pci device IDs of virtio devices are
pci id = 0x1000 + virtio device ID; so let's pull virtio_ids.h
from linux and write a macro that does + 0x1000 instead of hard-coding
values.

Makes sense?

-- 
MST



Re: [Qemu-devel] [PATCH v6 12/12] virtio-blk: add x-data-plane=on|off performance feature

2012-12-16 Thread Michael S. Tsirkin
On Mon, Dec 10, 2012 at 02:09:45PM +0100, Stefan Hajnoczi wrote:
 @@ -33,6 +34,7 @@ typedef struct VirtIOBlock
  VirtIOBlkConf *blk;
  unsigned short sector_mask;
  DeviceState *qdev;
 +VirtIOBlockDataPlane *dataplane;
  } VirtIOBlock;
  
  static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
 @@ -407,6 +409,14 @@ static void virtio_blk_handle_output(VirtIODevice *vdev, 
 VirtQueue *vq)
  .num_writes = 0,
  };
  
 +/* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
 + * dataplane here instead of waiting for .set_status().
 + */

By the way which guests are these?

 +if (s-dataplane) {
 +virtio_blk_data_plane_start(s-dataplane);
 +return;
 +}
 +

By the way it's chunk such as this that I meant: it's not
compiled out even if dataplane is disabled by configure.
Naither is the extra field in the struct.


  while ((req = virtio_blk_get_request(s))) {
  virtio_blk_handle_request(req, mrb);
  }



[Qemu-devel] [Bug 1090837] Re: Error in building Qemu-1.3.0 on Solaris 10

2012-12-16 Thread Dipanjan Das
If I invoke /usr/xpg4/bin/sh from bash and then start the build process,
will it be OK/ Or do I need to add /usr/xpg4/bin/sh to PATH? Does the
patch mentioned in the referred bug need to be applied?

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

Title:
  Error in building Qemu-1.3.0 on Solaris 10

Status in QEMU:
  New

Bug description:
  While trying to build Qemu on Oracle Solaris 10 (SPARC processor), I
  encountered the following error in the configure step:

  ./configure --prefix=/usr/local/ --install=/usr/ucb/install
  ./configure: bad substitution
  ./configure: !: not found
  ./configure: !: not found
  ./configure: !: not found
  ./configure: !: not found
  ./configure: !: not found
  ./configure: curl-config: not found
  ./configure: curl-config: not found

  As the following bug report says:
  https://bugs.launchpad.net/qemu/+bug/636315, sh is hard-coded in the
  script. Can't the script be modified to accept a $SHELL argument to
  make use of bash or other shell during configure and make step?

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



Re: [Qemu-devel] [PATCH v6 03/12] dataplane: add host memory mapping code

2012-12-16 Thread Michael S. Tsirkin
On Fri, Dec 14, 2012 at 12:45:16PM +0100, Stefan Hajnoczi wrote:
 On Wed, Dec 12, 2012 at 4:49 PM, Michael S. Tsirkin m...@redhat.com wrote:
  On Wed, Dec 12, 2012 at 04:34:21PM +0100, Stefan Hajnoczi wrote:
  On Tue, Dec 11, 2012 at 08:09:56PM +0200, Michael S. Tsirkin wrote:
   On Tue, Dec 11, 2012 at 10:32:28AM -0600, Anthony Liguori wrote:
Michael S. Tsirkin m...@redhat.com writes:
   
 On Tue, Dec 11, 2012 at 04:27:49PM +0100, Stefan Hajnoczi wrote:
 On Tue, Dec 11, 2012 at 3:13 PM, Michael S. Tsirkin 
 m...@redhat.com wrote:
  On Mon, Dec 10, 2012 at 02:09:36PM +0100, Stefan Hajnoczi wrote:
  The data plane thread needs to map guest physical addresses to 
  host
  pointers.  Normally this is done with cpu_physical_memory_map() 
  but the
  function assumes the global mutex is held.  The data plane 
  thread does
  not touch the global mutex and therefore needs a thread-safe 
  memory
  mapping mechanism.
 
  Hostmem registers a MemoryListener similar to how vhost collects 
  and
  pushes memory region information into the kernel.  There is a
  fine-grained lock on the regions list which is held during 
  lookup and
  when installing a new regions list.
 
  Can we export and reuse the vhost code for this?
  I think you will find this advantageous when you add migration
  support down the line.
  And if you find it necessary to use MemoryListener e.g. for 
  performance
  reasons, then vhost will likely benefit too.

 It's technically possible and not hard to do but it prevents
 integrating deeper with core QEMU as the memory API becomes
 thread-safe.

 There are two ways to implement dirty logging:
 1. The vhost log approach which syncs dirty information 
 periodically.
 2. A cheap thread-safe way to mark dirty outside the global mutex,
 i.e. a thread-safe memory_region_set_dirty().

 You don't normally want to dirty the whole region,
 you want to do this to individual pages.

 If we can get thread-safe guest memory load/store in QEMU then #2 is
 included.  We can switch to using hw/virtio.c instead of
 hw/dataplane/vring.c, we get dirty logging for free, we can drop
 hostmem.c completely, etc.

 Stefan

 So why not reuse existing code? If you drop it later it won't
 matter what you used ...
   
Let's not lose sight of the forest for the trees here...
   
This whole series is not reusing existing code.  That's really the 
whole
point.
   
The point is to take the code (duplication and all) and then do all of
the refactoring to use common code in the tree itself.
   
If we want to put this in a hw/staging/ directory, that's fine by me
too.
   
Regards,
   
Anthony Liguori
  
   Yes I agree. I think lack of handling for cross regin descriptors
   bothers me a bit more.
 
  The two things you've mentioned both aren't handled by hw/virtio.c:
 
  1. Issue: Indirect descriptors have no alignment restrictions and can
 cross regions.
 
 hw/virtio.c uses vring_desc_flags() and other accessor functions,
 which do lduw_phys() - there is no memory region boundary checking
 here.
 
  Since addresses are aligned this one is fine I think.
 
  2. Issue: Virtio buffers can cross memory region boundaries.
 
 hw/virtio.c maps buffers 1:1 using virtqueue_map_sg() and exits if
 mapping fails.  It does not split buffers if they cross a memory
 region.
 
  These are definitely ugly corner cases but hw/virtio.c is proof that
  we're not hitting them in practice.
 
  Stefan
 
  Yes, this one seems ugly. Maybe add a TODO?
 
  OK let's assume we want to put it in staging/
  I worry about the virtio-blk changes being isolated.
  Can you put ifdef CONFIG_VIRTIO_BLK_DATA_PLANE around
  them all to avoid dependency on that header completely
  if configured out?
 
 Okay, I'll move the #ifdefs.  I like the stubs in the header file
 because it reduces the amount of #ifdefs, but this is easy to change.
 
 Stefan

Okay.
Another option (if you prefer stubs) is to add a stub for access to
s-dataplane field, and surround just the field with ifdefs.
As it is, this code:
if (s-dataplane) {
return;
}
can't be compiled out since compiler is not smart enough to
figure out dataplane is never set.
-- 
MST



Re: [Qemu-devel] [PATCHv2] virtio: verify that all outstanding buffers are flushed

2012-12-16 Thread Michael S. Tsirkin
On Thu, Dec 13, 2012 at 11:48:17AM +0100, Kevin Wolf wrote:
 Am 12.12.2012 17:37, schrieb Michael S. Tsirkin:
  On Wed, Dec 12, 2012 at 04:52:53PM +0100, Paolo Bonzini wrote:
  Il 12/12/2012 16:25, Michael S. Tsirkin ha scritto:
  On Wed, Dec 12, 2012 at 04:01:27PM +0100, Paolo Bonzini wrote:
  Il 12/12/2012 15:47, Michael S. Tsirkin ha scritto:
  As step 1, I think we should just complete all outstanding
  requests when VM stops.
 
  Yes it means you can't do the retry hack after migration
  but this is hardly common scenario.
 
  I disagree...
 
  Disagree with what? You are saying it's common?
 
  It's not common, but you cannot block migration because you have an I/O
  error.  Solving the error may involve migrating the guests away from
  that host.
 
  Paolo
  
  No, you should complete with error.
 
 Just to confirm that this isn't possible: rerror/werror=stop is a
 supported feature, and I do get bug reports when it breaks (including
 migration while the VM is stopped for an I/O error).
 
 Makes it common enough that breaking it is definitely not an option, right?
 
 Kevin

Nod. So we need to fix inuse which is currently broken.
We still can have an assert but it needs to be done
more carefully, counting outstanding requests and
checking it matches inuse.

-- 
MST



Re: [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device

2012-12-16 Thread Andreas Färber
Am 14.12.2012 17:46, schrieb Jens Freimann:
 From: Christian Borntraeger borntrae...@de.ibm.com
 
 Lets move the code to setup IPL for external kernel
 or via the zipl rom into a separate file. This allows to
 
 - define a reboot handler, setting up the PSW appropriately

Careful with the ordering then: Since patch 2/3 adds another reset
handler in the CPU instance_init, the ipl device must be created after
the CPU - I'm guessing this is the case here but will also need to be
assured in the ccw machine.

 - enhance the boot code to IPL disks that contain a bootmap that
   was created with zipl under LPAR or z/VM (future patch)
 - reuse that code for several machines (e.g. virtio-ccw and virtio-s390)
 - allow different machines to provide different defaults
 
 Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
 Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
 ---
 v1 - v2:
  * get rid of ipl.h
  * move defines to ipl.c and make s390_ipl_cpu static
 
 ---
  hw/s390-virtio.c   |  98 ---
  hw/s390x/Makefile.objs |   1 +
  hw/s390x/ipl.c | 153 
 +
  3 files changed, 164 insertions(+), 88 deletions(-)
  create mode 100644 hw/s390x/ipl.c
 
 diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
 index ca1bb09..a350430 100644
 --- a/hw/s390-virtio.c
 +++ b/hw/s390-virtio.c
[...]
 @@ -185,6 +168,15 @@ static void s390_init(QEMUMachineInitArgs *args)
  /* get a BUS */
  s390_bus = s390_virtio_bus_init(my_ram_size);
  s390_sclp_init();
 +dev  = qdev_create(NULL, s390-ipl);
 +if (args-kernel_filename) {
 +qdev_prop_set_string(dev, kernel, args-kernel_filename);
 +}
 +if (args-initrd_filename) {
 +qdev_prop_set_string(dev, initrd, args-initrd_filename);
 +}
 +qdev_prop_set_string(dev, cmdline, args-kernel_cmdline);

Why NULL checks for 2 out of 3 string properties?

 +qdev_init_nofail(dev);
  
  /* allocate RAM */
  memory_region_init_ram(ram, s390.ram, my_ram_size);
[...]
 diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
 new file mode 100644
 index 000..945a9ba
 --- /dev/null
 +++ b/hw/s390x/ipl.c

Nice location. :)

 @@ -0,0 +1,153 @@
 +/*
 + * bootloader support
 + *
 + * Copyright IBM, Corp. 2012
 + *
 + * Authors:
 + *  Christian Borntraeger borntrae...@de.ibm.com
 + *
 + * This work is licensed under the terms of the GNU GPL, version 2 or (at 
 your
 + * option) any later version.  See the COPYING file in the top-level 
 directory.
 + *
 + */
 +
 +#include sysemu.h

sysemu.h?

 +#include cpu.h
 +#include elf.h
 +#include hw/loader.h
 +#include hw/sysbus.h
 +
 +#define KERN_IMAGE_START0x01UL
 +#define KERN_PARM_AREA  0x010480UL
 +#define INITRD_START0x80UL
 +#define INITRD_PARM_START   0x010408UL
 +#define INITRD_PARM_SIZE0x010410UL
 +#define PARMFILE_START  0x001000UL
 +#define ZIPL_FILENAME   s390-zipl.rom
 +#define ZIPL_IMAGE_START0x009000UL
 +#define IPL_PSW_MASK0x00018000ULL
 +
 +typedef struct {

Anonymous structs are discouraged (not sure where that makes a
difference, maybe gdb?), i.e. typedef struct S390IPLState {

 +SysBusDevice dev;

Please adopt the following QOM convention:

SysBusDevice parent_obj; // this field is then referenced nowhere
// white line; in header files /* private/public */ gtk-doc annotation
...
 +char *kernel;
 +char *initrd;
 +char *cmdline;
 +} S390IPLState;

I read that you got rid of an ipl.h; since you are using this device
from a machine that seems okay - if used from another object, header
files are encouraged. Or if memory address constants are to be shared
with a qtest test case (don't think that makes sense for a bootloader).

 +
 +static void s390_ipl_cpu(uint64_t pswaddr)
 +{
 +CPUS390XState *env = qemu_get_cpu(0);
 +env-psw.addr = pswaddr;
 +env-psw.mask = IPL_PSW_MASK;
 +s390_add_running_cpu(env);
 +}
 +
 +static int s390_ipl_init(SysBusDevice *dev)
 +{
 +S390IPLState *ipl = DO_UPCAST(S390IPLState, dev, dev);

Please use a QOM cast macro S390_IPL(dev) instead of DO_UPCAST().

You'll find many examples in
https://lists.gnu.org/archive/html/qemu-devel/2012-11/msg02746.html

 +ram_addr_t kernel_size = 0;
 +
 +if (!ipl-kernel) {
 +ram_addr_t bios_size = 0;
 +char *bios_filename;
 +
 +/* Load zipl bootloader */
 +if (bios_name == NULL) {
 +bios_name = ZIPL_FILENAME;
 +}
 +
 +bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
 +bios_size = load_image_targphys(bios_filename, ZIPL_IMAGE_START, 
 4096);
 +g_free(bios_filename);
 +
 +if ((long)bios_size  0) {
 +hw_error(could not load bootloader '%s'\n, bios_name);
 +}
 +
 +if (bios_size  4096) {
 +hw_error(stage1 bootloader is  

Re: [Qemu-devel] [RFC PATCH v7 7/8] virtio-pci-blk : Switch to new API.

2012-12-16 Thread Andreas Färber
Am 12.12.2012 18:53, schrieb Andreas Färber:
 Finally supplying a public device_initialize() or so would be helpful
 for the latter since VMState cannot cross pointers IIRC. I'll look into
 that part since inlining the old qdev functions cripples my Tegra work.

Investigating this, it turned out that my branch was outdated in still
calling qdev_prop_set_globals(), which some good soul (Paolo?) inlined
into device initfn. So the only step taken by qdev_try_create() that
remains after object_initialize() is qdev_set_parent_bus(), which I
think does not warrant a

device_post_initialize:
- qdev_set_parent_bus()

qdev_try_create:
- object_new()
- device_post_initialize()

qdev_create:
- qdev_try_create()

device_initialize:
- object_initialize()
- device_post_initialize()

construction.

So, given that virtio device states are in (or moved to) a header, they
can already be embedded in a has-a virtio-bus has-a Virtio*Device device
today and initialized with object_initialize(s-the_device) and
qdev_set_parent_bus(s-the_device, s-the_virtio_bus).

Andreas

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



Re: [Qemu-devel] target-i386: Allow tsc-frequency to be larger then 2.147G

2012-12-16 Thread Michael Tokarev
This is a follow-up to a more-or-less trivial commit,
2e84849aa2cc7f220d3b3668f5f7e3c57bb1b590 .  I'm adding
some more context - the whole function in question.


commit 2e84849aa2cc7f220d3b3668f5f7e3c57bb1b590
Author: Don Slutz d...@cloudswitch.com
Date:   Fri Sep 21 20:13:13 2012 -0400

target-i386: Allow tsc-frequency to be larger then 2.147G

The check using INT_MAX (2147483647) is wrong in this case.

Signed-off-by: Fred Oliveira folive...@cloudswitch.com
Signed-off-by: Don Slutz d...@cloudswitch.com
Signed-off-by: Stefan Hajnoczi stefa...@gmail.com

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 423e009..cbc172e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -846,7 +846,7 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, 
void *opaque,
 {
 X86CPU *cpu = X86_CPU(obj);
 const int64_t min = 0;
-const int64_t max = INT_MAX;
+const int64_t max = INT64_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;
 }

The patch makes the second test (if value  max)
to be a no-op, since value is of type int64_t,
and max is now INT64_MAX, so value can never be
larger than max.  Overflow can be catched by the
first test (value  0).

Note this function has another defect: the tsc
frequency is truncated to KHz.  It's okay when
it is called from the default cpu init function,
where the initial value is in khz and is multiplied
by 1000 when calling x86_cpuid_set_tsc_freq(),
but not okay when called as a handler for user-
defined option, like -cpu foo,tsc_frequency=bar.

I'm not sure how often this option is used, however.

Thanks,

/mjt



Re: [Qemu-devel] [PATCH 0/2] virtio: reset all qbuses too when writing to the status field

2012-12-16 Thread Michael S. Tsirkin
On Thu, Dec 13, 2012 at 09:54:23AM +0100, Paolo Bonzini wrote:
 Il 12/12/2012 22:27, Michael S. Tsirkin ha scritto:
  Maybe it's obvious to you that qdev_reset_all(x)
  does a soft reset and what soft reset means
  for each bus type
 
  We can define soft reset to be *independent* of the bus type.  As you
  said, you access it with a device register.
  
  I think qemu has one type of reset ATM which is the hard reset.
 
 A hard reset would kill BARs and configuration space too.
 qdev_reset_all doesn't.  Ergo, it is not a hard reset.
 
 But hey, I'm not wed to names.  Let's call it device-level reset and
 bus-level reset.  Whatever.

It's not a question of a name.

ATM qemu supports one kind of reset because it's a kind of reset all
hardware supports.  The moment we start inventing
our own one we need to document exactly what it means.


  Any other kind is device specific now.
  The fact that qdev has two APIs which are kind of but not
  exactly the same is a bug not a feature.
 
 BusState reset and DeviceState reset are not the same because they are
 triggered differently, one by bus-level functionality (e.g. FLR) and the
 other by device-level functionality (e.g. a register).
 
 That's neither a bug nor a feature.  It's just obvious.
 
  And relying on it in generic virtio is just going to
  confuse things.
 
 It is going to confuse you perhaps, but it will not confuse whoever will
 write the next virtio device with a bus.  Who knows, virtio-i2c.
 
  Further it does not follow that all backends are children
  of the frontend.
 
 Backends are not children of the frontend.  Seriously, this is qdev 101.
 
 virtio-scsi has no backend.  Frontends (disks) are children of
 virtio-scsi.  Each backend (host block device) is connected to a child
 of virtio-scsi.
 
 virtio-scsi does not need to reset back-ends.  It needs to reset front-ends.
 
 virtio-serial does not know it needs to reset back-ends.  It knows it
 needs to reset front-ends.  Each reset of the front-end will also
 propagate to a back-end, but virtio-serial need not know that.
 
  So please just fix the virtio-scsi bug for now and we can
  address the bigger issue if any later.
 
 I'm refusing to fix the bug in virtio-scsi.  It is not a virtio-scsi
 bug, as proved by the virtio-serial bus having to do the same things.
 Two wrongs do not make a right, and here we have three wrongs already:
 virtio-pci, virtio-s390, virtio-serial all reinventing the wheel.
 
 Paolo

You have a point about a problem.

My problem is with the solution, this solution depends on the exact
modeling for correctness and that I don't want to do since we seem to be
re-shuffling what inherits what so often.

For example I could not figure out how the reset function for virtio pci
(which clears pending msix vectors so is required) was called by this.

Another thing that bothers me is that during regular PCI bus reset
virtio does not invoke qdev_reset_all but with this reset, it does.
Inconsistent.

-- 
MST



[Qemu-devel] [Bug 1090837] Re: Error in building Qemu-1.3.0 on Solaris 10

2012-12-16 Thread Dipanjan Das
Until tomorrow, I can't report back as the Solaris 10 system is in my
workplace. What I tried was to trigger ./configure from bash shell which
produced the log above and aborted. As the title already says, I tried
to build Qemu 1.3.0.

P.S. If I already use bash shell to invoke the script, do I need to
`bash ./configure ...` any more?

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

Title:
  Error in building Qemu-1.3.0 on Solaris 10

Status in QEMU:
  New

Bug description:
  While trying to build Qemu on Oracle Solaris 10 (SPARC processor), I
  encountered the following error in the configure step:

  ./configure --prefix=/usr/local/ --install=/usr/ucb/install
  ./configure: bad substitution
  ./configure: !: not found
  ./configure: !: not found
  ./configure: !: not found
  ./configure: !: not found
  ./configure: !: not found
  ./configure: curl-config: not found
  ./configure: curl-config: not found

  As the following bug report says:
  https://bugs.launchpad.net/qemu/+bug/636315, sh is hard-coded in the
  script. Can't the script be modified to accept a $SHELL argument to
  make use of bash or other shell during configure and make step?

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



Re: [Qemu-devel] [PATCH 0/2] virtio: reset all qbuses too when writing to the status field

2012-12-16 Thread Paolo Bonzini
Il 16/12/2012 18:04, Michael S. Tsirkin ha scritto:
 On Thu, Dec 13, 2012 at 09:54:23AM +0100, Paolo Bonzini wrote:
 Il 12/12/2012 22:27, Michael S. Tsirkin ha scritto:
 Maybe it's obvious to you that qdev_reset_all(x)
 does a soft reset and what soft reset means
 for each bus type

 We can define soft reset to be *independent* of the bus type.  As you
 said, you access it with a device register.

 I think qemu has one type of reset ATM which is the hard reset.

 A hard reset would kill BARs and configuration space too.
 qdev_reset_all doesn't.  Ergo, it is not a hard reset.

 But hey, I'm not wed to names.  Let's call it device-level reset and
 bus-level reset.  Whatever.
 
 It's not a question of a name.
 
 ATM qemu supports one kind of reset because it's a kind of reset all
 hardware supports.  The moment we start inventing
 our own one we need to document exactly what it means.

We have two, DeviceClass's and BusClass's reset members.

I'll make a patch to document them.

 You have a point about a problem.
 
 My problem is with the solution, this solution depends on the exact
 modeling for correctness and that I don't want to do since we seem to be
 re-shuffling what inherits what so often.
 
 For example I could not figure out how the reset function for virtio pci
 (which clears pending msix vectors so is required) was called by this.

The same way a PCI bus reset clears pending MSIX vectors.

qdev_reset_all(pci_dev)
   - qdev_walk_children(pci_dev, qdev_reset_one, qbus_reset_one, NULL);
   - qdev_reset_one(pci_dev, NULL);
   - device_reset(pci_dev);
   - calls dc-reset member set for virtio-*-pci, i.e. virtio_pci_reset

 Another thing that bothers me is that during regular PCI bus reset
 virtio does not invoke qdev_reset_all but with this reset, it does.
 Inconsistent.

It does.

A PCI bus reset (or FLR) calls pci_device_reset which does this

void pci_device_reset(PCIDevice *dev)
{
int r;

qdev_reset_all(dev-qdev);
...
}

This is exactly how a PCI bus reset clears pending MSIX vectors.

Paolo



Re: [Qemu-devel] [PATCHv2] virtio: verify that all outstanding buffers are flushed

2012-12-16 Thread Anthony Liguori
Paolo Bonzini pbonz...@redhat.com writes:

  We technically should save the addresses and sizes too.  It makes
  it a heck of a lot safer then re-reading guest memory since we do some
  validation on the size of the sg elements.
 
 Not really.
 
 The guest puts the descriptors in the ring and leaves them there until
 the device acks.  If it changes them once they're exposed but before
 they're acked, it can get either before or after version, and always
 could.

 The problems start when the guest tries to race against QEMU and defy
 the validation.  Always using the validated version is a bit easier
 than redoing the validation after migration.

Exactly.

Regards,

Anthony Liguori


 Paolo




Re: [Qemu-devel] [PULL] pci,net,misc infrastructure

2012-12-16 Thread Michael S. Tsirkin
On Thu, Dec 13, 2012 at 02:31:53PM -0600, Anthony Liguori wrote:
 Michael S. Tsirkin m...@redhat.com writes:
 
  The following changes since commit 1c97e303d4ea80a2691334b0febe87a50660f99d:
 
Merge remote-tracking branch 'afaerber/qom-cpu' into staging (2012-12-10 
  08:35:15 -0600)
 
  are available in the git repository at:
 
 
git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_anthony
 
  for you to fetch changes up to 5c1ad98d71923b83a530e5db4c2110564b84e11d:
 
pci_bus.h: tweak include guards (2012-12-12 23:41:04 +0200)
 
 Doesn't build:
 
   CChw/apm.o
 /home/anthony/git/qemu/hw/apm.c:25:17: fatal error: pci.h: No such file or 
 directory
 compilation terminated.
 
 And there's really no good reason for this.  apm's part of target-i386
 so you couldn't have built this prior to doing a pull request.
 
 Please setup your tree in buildbot and wait for a full run before
 sending a pull requests in the future.
 
 Regards,
 
 Anthony Liguori

I've pushed a new version out will resend a pull request:
I did make clean this time so it should be fine.
Will look into fixing buildbot too.

-- 
MST



[Qemu-devel] [PULLv2] pci,net,misc infrastructure

2012-12-16 Thread Michael S. Tsirkin
The following changes since commit 1c97e303d4ea80a2691334b0febe87a50660f99d:

  Merge remote-tracking branch 'afaerber/qom-cpu' into staging (2012-12-10 
08:35:15 -0600)

are available in the git repository at:


  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_anthony

for you to fetch changes up to e2911ce3819502edc2d3694c96abf37de43a592a:

  kvm: do not flush after deleting gsi (2012-12-16 16:49:08 +0200)


pci,net,misc infrastructure

This rearranges pci core into hw/pci/
so it's separate from actual devices.
Also included are a couple of misc fixes:
- a fix for tap bug reported by alex on kvm forum
- a couple of misc updates: get_maintainer.pl,
  and license update for the q35 files
- a minor optimization to path updates

I've addressed the comments Blue Swirl had
for the rename patches - would like to merge ASAP
to avoid any conflicts, and we can do code style
fixups later - note this is style of existing files.

Signed-off-by: Michael S. Tsirkin m...@redhat.com


Jason Baron (1):
  Fixup q35/ich9 Licenses

Michael S. Tsirkin (12):
  tap: reset vnet header size on open
  get_maintainer.pl: update from linix 3.8
  pci: prepare makefiles for pci code reorganization
  MAINTAINERS: add hw/pci/ to list of PCI files
  pci: move pci core code to hw/pci
  pci: update all users to look in pci/
  pci: fix path for local includes
  Revert pci: prepare makefiles for pci code reorganization
  pci: rename pci_internals.h pci_bus.h
  pci_bus: update comment
  pci_bus.h: tweak include guards
  kvm: do not flush after deleting gsi

 MAINTAINERS   |  1 +
 arch_init.c   |  2 +-
 hw/Makefile.objs  | 10 ++---
 hw/ac97.c |  2 +-
 hw/acpi_ich9.c| 15 +++--
 hw/acpi_piix4.c   |  2 +-
 hw/alpha_sys.h|  4 ++--
 hw/apb_pci.c  |  8 +++
 hw/apic.c |  2 +-
 hw/apm.c  |  2 +-
 hw/bonito.c   |  4 ++--
 hw/cirrus_vga.c   |  2 +-
 hw/dec_pci.c  |  8 +++
 hw/e1000.c|  2 +-
 hw/eepro100.c |  2 +-
 hw/es1370.c   |  2 +-
 hw/esp-pci.c  |  2 +-
 hw/grackle_pci.c  |  4 ++--
 hw/gt64xxx.c  |  4 ++--
 hw/hda-audio.c|  2 +-
 hw/i386/Makefile.objs |  2 +-
 hw/i82378.c   |  2 +-
 hw/i82801b11.c|  2 +-
 hw/ich9.h |  8 +++
 hw/ide.h  |  2 +-
 hw/ide/ahci.c |  4 ++--
 hw/ide/cmd646.c   |  2 +-
 hw/ide/core.c |  2 +-
 hw/ide/ich.c  |  4 ++--
 hw/ide/pci.c  |  2 +-
 hw/ide/piix.c |  2 +-
 hw/ide/via.c  |  2 +-
 hw/intel-hda.c|  4 ++--
 hw/ioh3420.c  |  6 +++---
 hw/ioh3420.h  |  2 +-
 hw/ivshmem.c  |  4 ++--
 hw/kvm/apic.c |  2 +-
 hw/kvm/pci-assign.c   |  4 ++--
 hw/lpc_ich9.c | 40 +++
 hw/lsi53c895a.c   |  2 +-
 hw/macio.c|  2 +-
 hw/megasas.c  |  4 ++--
 hw/mips_fulong2e.c|  2 +-
 hw/mips_malta.c   |  2 +-
 hw/ne2000.c   |  2 +-
 hw/openpic.c  |  2 +-
 hw/pc.c   |  4 ++--
 hw/pc_piix.c  |  4 ++--
 hw/pci/Makefile.objs  |  6 ++
 hw/{ = pci}/msi.c|  2 +-
 hw/{ = pci}/msi.h|  2 +-
 hw/{ = pci}/msix.c   |  8 +++
 hw/{ = pci}/msix.h   |  2 +-
 hw/{ = pci}/pci-hotplug.c| 12 +--
 hw/{ = pci}/pci-stub.c   |  2 +-
 hw/{ = pci}/pci.c| 14 ++--
 hw/{ = pci}/pci.h| 10 -
 hw/{ = pci}/pci_bridge.c |  4 ++--
 hw/{ = pci}/pci_bridge.h |  2 +-
 hw/{pci_internals.h = pci/pci_bus.h} | 16 ++
 hw/{ = pci}/pci_host.c   |  4 ++--
 hw/{ = pci}/pci_host.h   |  2 +-
 hw/{ = pci}/pci_ids.h|  0
 hw/{ = pci}/pci_regs.h   |  0
 hw/{ = pci}/pcie.c   | 12 +--
 hw/{ = pci}/pcie.h   |  8 +++
 hw/{ 

Re: [Qemu-devel] [PATCH 0/2] virtio: reset all qbuses too when writing to the status field

2012-12-16 Thread Michael S. Tsirkin
On Sun, Dec 16, 2012 at 08:31:16PM +0100, Paolo Bonzini wrote:
 Il 16/12/2012 18:04, Michael S. Tsirkin ha scritto:
  On Thu, Dec 13, 2012 at 09:54:23AM +0100, Paolo Bonzini wrote:
  Il 12/12/2012 22:27, Michael S. Tsirkin ha scritto:
  Maybe it's obvious to you that qdev_reset_all(x)
  does a soft reset and what soft reset means
  for each bus type
 
  We can define soft reset to be *independent* of the bus type.  As you
  said, you access it with a device register.
 
  I think qemu has one type of reset ATM which is the hard reset.
 
  A hard reset would kill BARs and configuration space too.
  qdev_reset_all doesn't.  Ergo, it is not a hard reset.
 
  But hey, I'm not wed to names.  Let's call it device-level reset and
  bus-level reset.  Whatever.
  
  It's not a question of a name.
  
  ATM qemu supports one kind of reset because it's a kind of reset all
  hardware supports.  The moment we start inventing
  our own one we need to document exactly what it means.
 
 We have two, DeviceClass's and BusClass's reset members.
 
 I'll make a patch to document them.
 
  You have a point about a problem.
  
  My problem is with the solution, this solution depends on the exact
  modeling for correctness and that I don't want to do since we seem to be
  re-shuffling what inherits what so often.
  
  For example I could not figure out how the reset function for virtio pci
  (which clears pending msix vectors so is required) was called by this.
 
 The same way a PCI bus reset clears pending MSIX vectors.
 
 qdev_reset_all(pci_dev)
- qdev_walk_children(pci_dev, qdev_reset_one, qbus_reset_one, NULL);
- qdev_reset_one(pci_dev, NULL);
- device_reset(pci_dev);
- calls dc-reset member set for virtio-*-pci, i.e. virtio_pci_reset
 
  Another thing that bothers me is that during regular PCI bus reset
  virtio does not invoke qdev_reset_all but with this reset, it does.
  Inconsistent.
 
 It does.
 
 A PCI bus reset (or FLR) calls pci_device_reset which does this
 
 void pci_device_reset(PCIDevice *dev)
 {
 int r;
 
 qdev_reset_all(dev-qdev);
 ...
 }
 
 This is exactly how a PCI bus reset clears pending MSIX vectors.
 
 Paolo

Yes but I mean virtio-pci device is not a bus.
virtio could be a bus.
Our modeling seems incorrect and relying on it for correctness,
looks a bit wrong.

-- 
MST



Re: [Qemu-devel] [PATCH 1/3] s390: Move IPL code into a separate device

2012-12-16 Thread Christian Borntraeger
On 16/12/12 17:26, Andreas Färber wrote:
 Am 14.12.2012 17:46, schrieb Jens Freimann:
 From: Christian Borntraeger borntrae...@de.ibm.com

 Lets move the code to setup IPL for external kernel
 or via the zipl rom into a separate file. This allows to

 - define a reboot handler, setting up the PSW appropriately
 
 Careful with the ordering then: Since patch 2/3 adds another reset
 handler in the CPU instance_init, the ipl device must be created after
 the CPU - I'm guessing this is the case here but will also need to be
 assured in the ccw machine.
 
 - enhance the boot code to IPL disks that contain a bootmap that
   was created with zipl under LPAR or z/VM (future patch)
 - reuse that code for several machines (e.g. virtio-ccw and virtio-s390)
 - allow different machines to provide different defaults

 Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
 Signed-off-by: Jens Freimann jf...@linux.vnet.ibm.com
 ---
 v1 - v2:
  * get rid of ipl.h
  * move defines to ipl.c and make s390_ipl_cpu static

 ---
  hw/s390-virtio.c   |  98 ---
  hw/s390x/Makefile.objs |   1 +
  hw/s390x/ipl.c | 153 
 +
  3 files changed, 164 insertions(+), 88 deletions(-)
  create mode 100644 hw/s390x/ipl.c

 diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c
 index ca1bb09..a350430 100644
 --- a/hw/s390-virtio.c
 +++ b/hw/s390-virtio.c
 [...]
 @@ -185,6 +168,15 @@ static void s390_init(QEMUMachineInitArgs *args)
  /* get a BUS */
  s390_bus = s390_virtio_bus_init(my_ram_size);
  s390_sclp_init();
 +dev  = qdev_create(NULL, s390-ipl);
 +if (args-kernel_filename) {
 +qdev_prop_set_string(dev, kernel, args-kernel_filename);
 +}
 +if (args-initrd_filename) {
 +qdev_prop_set_string(dev, initrd, args-initrd_filename);
 +}
 +qdev_prop_set_string(dev, cmdline, args-kernel_cmdline);
 
 Why NULL checks for 2 out of 3 string properties?

cmdline is always a valid string, (never NULL), but kernel and initrd can
be NULL, which kills qdev_prop_set_string.


 + * Authors:
 + *  Christian Borntraeger borntrae...@de.ibm.com
 + *
 + * This work is licensed under the terms of the GNU GPL, version 2 or (at 
 your
 + * option) any later version.  See the COPYING file in the top-level 
 directory.
 + *
 + */
 +
 +#include sysemu.h
 
 sysemu.h?

bios_name. 
I could use another property which is modified/set by the machine init.

 
 +#include cpu.h
 +#include elf.h
 +#include hw/loader.h
 +#include hw/sysbus.h
 +
 +#define KERN_IMAGE_START0x01UL
 +#define KERN_PARM_AREA  0x010480UL
 +#define INITRD_START0x80UL
 +#define INITRD_PARM_START   0x010408UL
 +#define INITRD_PARM_SIZE0x010410UL
 +#define PARMFILE_START  0x001000UL
 +#define ZIPL_FILENAME   s390-zipl.rom
 +#define ZIPL_IMAGE_START0x009000UL
 +#define IPL_PSW_MASK0x00018000ULL
 +
 +typedef struct {
 
 Anonymous structs are discouraged (not sure where that makes a
 difference, maybe gdb?), i.e. typedef struct S390IPLState {
 
 +SysBusDevice dev;
 
 Please adopt the following QOM convention:
 
 SysBusDevice parent_obj; // this field is then referenced nowhere


ok

 +
 +static void s390_ipl_cpu(uint64_t pswaddr)
 +{
 +CPUS390XState *env = qemu_get_cpu(0);
 +env-psw.addr = pswaddr;
 +env-psw.mask = IPL_PSW_MASK;
 +s390_add_running_cpu(env);
 +}
 +
 +static int s390_ipl_init(SysBusDevice *dev)
 +{
 +S390IPLState *ipl = DO_UPCAST(S390IPLState, dev, dev);
 
 Please use a QOM cast macro S390_IPL(dev) instead of DO_UPCAST().
 
 You'll find many examples in
 https://lists.gnu.org/archive/html/qemu-devel/2012-11/msg02746.html

OK.

[..]
 
 +static TypeInfo s390_ipl_info = {
 
 static const

ok
 
 +.class_init = s390_ipl_class_init,
 +.parent = TYPE_SYS_BUS_DEVICE,
 +.name  = s390-ipl,
 +.instance_size  = sizeof(S390IPLState),
 +};
 +
 +static void s390_register_ipl(void)
 
 s390_ipl_register_types?

makes sense.
 
 +{
 +type_register_static(s390_ipl_info);
 +}
 +
 +type_init(s390_register_ipl)
 +
 
 Trailing white line.
ok




[Qemu-devel] [Bug 1031698] Re: Missing PPC TRACE exception on a branch

2012-12-16 Thread Michael Tokarev
This is fixed by qemu commit f0cc4aa8450376ca2aee3ebb09db71f9f2ff333b
which went into qemu-1.3 release.

** Changed in: qemu
   Status: New = Fix Committed

** Changed in: qemu
   Status: Fix Committed = Fix Released

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

Title:
  Missing PPC TRACE exception on a branch

Status in QEMU:
  Fix Released

Bug description:
  I am using qemu-1.1.1-1 to emulate a PPC PREP machine on intel host
  linux ubuntu 11-10

  In the following example, i will show that the POWERPC TRACE exception
  at vector 0xD00 is not taken when executing a specific kind of branch.

  
  I have the following stand-alone assembly source code for powerpc in file 
SE_TEST.s

  
.text
.global _start

  
  _start:

  /* copy vector handler at address 0xD00 */
addis   3,0,(0xD00)@ha
addi3,3,(0xD00)@l
addis   4,0,(vector_handler)@ha
addi4,4,(vector_handler)@l
addi3,3,-4
addi4,4,-4
lwzu7,4(4)
stwu7,4(3)
lwzu7,4(4)
stwu7,4(3)
lwzu7,4(4)
stwu7,4(3)
lwzu7,4(4)
stwu7,4(3)

  
  /* set branch address in SRR0 register */ 
addis   3,0,(branch)@ha
addi3,3,(branch)@l
mtspr   26,3

  /* Read MSR */
  /* Set SE bit and clear IP bit then set value in SSR1 */

mfmsr   4
ori 4,4,0x0400
andi.   4,4,0xFFBF
mtspr   27,4
  /* Set CR condition to execute not taken branch after rfi */  
addi5,0,0
cmpi0,5,0
rfi
nop
nop
nop

  branch:   
bne down   - branch where the error is.
  branch_plus_1:
nop
  branch_plus_2:
nop
  down: nop
nop
nop
nop

  vector_handler:   
mfspr   6,26
nop
nop
nop


  It compiles with powerpc-eabi-gcc SE_TEST.s -o SE_TEST.elf

  
  Then I run Qemu with the command : ./qemu-system-ppc -M prep -s -S

  Then i run a cross gdb with the command: powerpc-eabi-gdb --nx

  On gdb prompt i execute the following gdb command :

  file SE_TEST.elf
  target remote :1234
  load SE_TEST.elf
  set $pc =_start
  b *0xD08
  c

  echo srr0 value in trace handler  
  p/x $r6
  echo address of branch_plus_1   
  p/x branch_plus_1
  echo address of branch_plus_2   
  p/x branch_plus_2

  The gdb command windows display the result:

  (gdb) so co
  0xfffc in ?? ()
  Loading section .init, size 0x24 lma 0x1800074
  Loading section .text, size 0x26c lma 0x1800098
  Loading section .fini, size 0x20 lma 0x1800304
  Loading section .eh_frame, size 0x8 lma 0x1810324
  Loading section .ctors, size 0x8 lma 0x181032c
  Loading section .dtors, size 0x8 lma 0x1810334
  Loading section .jcr, size 0x4 lma 0x181033c
  Loading section .data, size 0x4 lma 0x1810340
  Start address 0x1800200, load size 720
  Transfer rate: 5760 bits in 1 sec, 90 bytes/write.
  Breakpoint 1 at 0xd08

  Breakpoint 1, 0x0d08 in ?? ()
  srr0 value in trace handler  $1 = 0x1800274
  address of branch_plus_1   $2 = 0x1800270
  address of branch_plus_2   $3 = 0x1800274

  
  In trace exception handler, the SRR0 register value should be the value of 
address just following the branch id 0x1800270.
  But it is not the case, the SRR0 value is the address of the next next 
instruction after the branch instruction.

  It seems that the singlestep exception was not taken after executing
  the bne down instruction but was taken after executing the first
  following nop instruction.

  I have make some other test with other kind of branch, the behaviour
  is correct and only for a conditionnal branch when it is not taken
  with issue appears.

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



[Qemu-devel] [Bug 1090837] Re: Error in building Qemu-1.3.0 on Solaris 10

2012-12-16 Thread Andreas Färber
Just using $SHELL inside configure would be insufficient if run via Solaris' sh.
Using `bash ./configure ...` should've worked for v1.2 without patches, didn't 
test v1.3 on Sol10 yet.

Whether the DTrace support scripts fully work on Solaris 10 is another
issue.

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

Title:
  Error in building Qemu-1.3.0 on Solaris 10

Status in QEMU:
  New

Bug description:
  While trying to build Qemu on Oracle Solaris 10 (SPARC processor), I
  encountered the following error in the configure step:

  ./configure --prefix=/usr/local/ --install=/usr/ucb/install
  ./configure: bad substitution
  ./configure: !: not found
  ./configure: !: not found
  ./configure: !: not found
  ./configure: !: not found
  ./configure: !: not found
  ./configure: curl-config: not found
  ./configure: curl-config: not found

  As the following bug report says:
  https://bugs.launchpad.net/qemu/+bug/636315, sh is hard-coded in the
  script. Can't the script be modified to accept a $SHELL argument to
  make use of bash or other shell during configure and make step?

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



[Qemu-devel] [Bug 1090837] Re: Error in building Qemu-1.3.0 on Solaris 10

2012-12-16 Thread Andreas Färber
Yes, if you execute `./configure ...` the shell will execute the shebang
line inside the script, which says /bin/sh and happens to be a really
ancient version for backwards compatibility on Solaris.

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

Title:
  Error in building Qemu-1.3.0 on Solaris 10

Status in QEMU:
  New

Bug description:
  While trying to build Qemu on Oracle Solaris 10 (SPARC processor), I
  encountered the following error in the configure step:

  ./configure --prefix=/usr/local/ --install=/usr/ucb/install
  ./configure: bad substitution
  ./configure: !: not found
  ./configure: !: not found
  ./configure: !: not found
  ./configure: !: not found
  ./configure: !: not found
  ./configure: curl-config: not found
  ./configure: curl-config: not found

  As the following bug report says:
  https://bugs.launchpad.net/qemu/+bug/636315, sh is hard-coded in the
  script. Can't the script be modified to accept a $SHELL argument to
  make use of bash or other shell during configure and make step?

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



[Qemu-devel] [PATCH v8 01/10] fix some debug printf format strings

2012-12-16 Thread Matthew Ogilvie
These are normally ifdefed out and don't matter.  But if you enable
them, they ought to be correct.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/cirrus_vga.c | 4 ++--
 hw/i8259.c  | 3 ++-
 hw/ide/cmd646.c | 5 +++--
 hw/ide/via.c| 5 +++--
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 40efa8a..84f2f16 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2053,8 +2053,8 @@ static void cirrus_vga_mem_write(void *opaque,
}
 } else {
 #ifdef DEBUG_CIRRUS
-printf(cirrus: mem_writeb  TARGET_FMT_plx  value %02x\n, addr,
-   mem_value);
+printf(cirrus: mem_writeb  TARGET_FMT_plx  value %02 PRIx64 \n,
+   addr, mem_value);
 #endif
 }
 }
diff --git a/hw/i8259.c b/hw/i8259.c
index af0ba4d..60c25ba 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -355,7 +355,8 @@ static uint64_t pic_ioport_read(void *opaque, hwaddr addr,
 ret = s-imr;
 }
 }
-DPRINTF(read: addr=0x%02x val=0x%02x\n, addr, ret);
+DPRINTF(read: addr=0x%02 TARGET_PRIxPHYS  val=0x%02x\n,
+addr, ret);
 return ret;
 }
 
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 804db60..72ceaaf 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -154,7 +154,7 @@ static uint64_t bmdma_read(void *opaque, hwaddr addr,
 break;
 }
 #ifdef DEBUG_IDE
-printf(bmdma: readb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: readb 0x%02 TARGET_PRIxPHYS  : 0x%02x\n, addr, val);
 #endif
 return val;
 }
@@ -170,7 +170,8 @@ static void bmdma_write(void *opaque, hwaddr addr,
 }
 
 #ifdef DEBUG_IDE
-printf(bmdma: writeb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: writeb 0x%02 TARGET_PRIxPHYS  : 0x%02 PRIx64 \n,
+   addr, val);
 #endif
 switch(addr  3) {
 case 0:
diff --git a/hw/ide/via.c b/hw/ide/via.c
index efda173..10ba9b5 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -55,7 +55,7 @@ static uint64_t bmdma_read(void *opaque, hwaddr addr,
 break;
 }
 #ifdef DEBUG_IDE
-printf(bmdma: readb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: readb 0x%02 TARGET_PRIxPHYS  : 0x%02x\n, addr, val);
 #endif
 return val;
 }
@@ -70,7 +70,8 @@ static void bmdma_write(void *opaque, hwaddr addr,
 }
 
 #ifdef DEBUG_IDE
-printf(bmdma: writeb 0x%02x : 0x%02x\n, addr, val);
+printf(bmdma: writeb 0x%02 TARGET_PRIxPHYS  : 0x%02 PRIx64 \n,
+   addr, val);
 #endif
 switch (addr  3) {
 case 0:
-- 
1.7.10.2.484.gcd07cc5




[Qemu-devel] [PATCH v8 07/10] i8259: refactor pic_set_irq level logic

2012-12-16 Thread Matthew Ogilvie
No change in functionality.

Clarify that the only difference between level triggered and
edge triggered interrupts is on the leading edge.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/i8259.c | 26 +-
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/hw/i8259.c b/hw/i8259.c
index 95587cd..9b2ec40 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -140,26 +140,18 @@ static void pic_set_irq(void *opaque, int irq, int level)
 }
 #endif
 
-if (s-elcr  mask) {
-/* level triggered */
-if (level) {
+if (level) {
+if ((s-last_irr  mask) == 0 ||  /* edge for edge triggered */
+(s-elcr  mask)) {   /* or level triggered */
 s-irr |= mask;
-s-last_irr |= mask;
-} else {
-s-irr = ~mask;
-s-last_irr = ~mask;
 }
+s-last_irr |= mask;
 } else {
-/* edge triggered */
-if (level) {
-if ((s-last_irr  mask) == 0) {
-s-irr |= mask;
-}
-s-last_irr |= mask;
-} else {
-s-irr = ~mask;
-s-last_irr = ~mask;
-}
+/* Dropping level clears the interrupt regardless of edge trigger
+ * vs level trigger.
+ */
+s-irr = ~mask;
+s-last_irr = ~mask;
 }
 pic_update_irq(s);
 }
-- 
1.7.10.2.484.gcd07cc5




Re: [Qemu-devel] [PATCH] fix dma.c MemoryRegion convertion

2012-12-16 Thread Marcelo Tosatti
On Sat, Dec 15, 2012 at 01:45:38PM +, Julien Grall wrote:
 On Fri, Dec 14, 2012 at 11:39 PM, Marcelo Tosatti mtosa...@redhat.com wrote:
 
 
  The high byte of the ioport address is necessary to compute
  the register address, see 82371AB PCI ISA IDE Xcelerator (PIIX4)
  document, eg:
 
  4.2.1.1. DCOM—DMA Command Register (IO)
  I/O Address: Channels 0–3—08h; Channels 4–7—0D0h
 
 
 I sent a patch on the mailing to resolve the problem yesterday
 (http://www.mail-archive.com/qemu-devel@nongnu.org/msg145242.html).
 Did it resolve your WinXp-32 installation? I just need to rework the comment.

Yes it does.

 
  Also the size of the region is wrong.
 
 I don't think the size of the region is wrong. For the second DMA
 controller, the ioports are 0xD0 - 0xDE for the control registers.
 So the size is 16 not 8. It's the same for the channel registers.
 
 Thanks,

Correct, my patch is broken.

Thanks




[Qemu-devel] [PATCH v8 05/10] fix i8254 output logic to match the spec

2012-12-16 Thread Matthew Ogilvie
This patch fixes i8254 output line (IRQ0) logic to match the spec.
Basically, IRQ0 line should normally be high, and only occasionally
go low to cause an edge.  This is an important prerequisite to
fixing the i8259 interrupt controller to cancel an unhandled interrupt
when the IRQ line goes low.

More details:
  * Fix high-vs-low counting logic to match the timing diagrams
and descriptions in Intel's documentation.
  * Improve reading back the count in mode 3.
  * Handle GATE input properly with respect to the OUT line, and add
a FIXME comment for GATE-paused counting and reading
back the counter.  GATE is hard wired high for channel 0 (IRQ0), but
it can be software controlled on channel 2 (PC speaker).

See http://bochs.sourceforge.net/techspec/intel-82c54-timer.pdf.gz
or search the net for 23124406.pdf.

Risks:

There is a risk that migrating between versions of qemu
with and without this patch will lose or gain a single timer
interrupt during the migration process.  The only case where
this is likely to be serious is probably losing a single-shot (mode 4)
interrupt, but if my understanding of how things work is good, then
that should only be possible if a whole slew of conditions are
all met:

 1. The guest is configured to run in a tickless mode (like
modern Linux).
 2. The guest is for some reason still using the i8254 rather
than something more modern like an HPET.  (The combination
of 1 and 2 should be rare.)
 3. The migration is going from a fixed version back to the
old version.  (Not sure how common this is, but it should
be rarer than migrating from old to new.)
 4. There are not going to be any timely events/interrupts
(keyboard, network, process sleeps, etc) that cause the guest
to reset the PIT mode 4 one-shot counter soon enough.

This combination should be rare enough that more complicated
solutions are not worth the effort.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/i8254.c| 24 ++--
 hw/i8254_common.c | 18 ++
 2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/hw/i8254.c b/hw/i8254.c
index bea5f92..edb5b7a 100644
--- a/hw/i8254.c
+++ b/hw/i8254.c
@@ -39,6 +39,15 @@ static void pit_irq_timer_update(PITChannelState *s, int64_t 
current_time);
 
 static int pit_get_count(PITChannelState *s)
 {
+/* FIXME: Add some way to represent a paused timer and return
+ *   the paused-at counter value, to better model:
+ * - gate-triggered modes (1 and 5),
+ * - gate-pausable modes,
+ * - [maybe] the wait until next CLK pulse to load counter logic,
+ * - [maybe/not clear] half-loaded counter logic for mode 0, and
+ *   the null count status bit,
+ * - etc.
+ */
 uint64_t d;
 int counter;
 
@@ -52,8 +61,7 @@ static int pit_get_count(PITChannelState *s)
 counter = (s-count - d)  0x;
 break;
 case 3:
-/* XXX: may be incorrect for odd counts */
-counter = s-count - ((2 * d) % s-count);
+counter = (s-count - ((2 * d) % s-count))  0xfffe;
 break;
 default:
 counter = s-count - (d % s-count);
@@ -98,6 +106,18 @@ static inline void pit_load_count(PITChannelState *s, int 
val)
 if (val == 0)
 val = 0x1;
 s-count_load_time = qemu_get_clock_ns(vm_clock);
+
+/* In gate-triggered one-shot modes, indirectly model some pit_get_out()
+ * cases by setting the load time way back until gate-triggered.
+ * (Generally only affects reading status from channel 2 speaker,
+ * due to hard-wired gates on other channels.)
+ *
+ * FIXME: This might be redesigned if a paused timer state is added
+ * for pic_get_count().
+ */
+if (s-mode == 1 || s-mode == 5)
+s-count_load_time -= muldiv64(val+2, get_ticks_per_sec(), PIT_FREQ);
+
 s-count = val;
 pit_irq_timer_update(s, s-count_load_time);
 }
diff --git a/hw/i8254_common.c b/hw/i8254_common.c
index a03d7cd..dc13c99 100644
--- a/hw/i8254_common.c
+++ b/hw/i8254_common.c
@@ -50,24 +50,18 @@ int pit_get_out(PITChannelState *s, int64_t current_time)
 switch (s-mode) {
 default:
 case 0:
-out = (d = s-count);
-break;
 case 1:
-out = (d  s-count);
+out = (d = s-count);
 break;
 case 2:
-if ((d % s-count) == 0  d != 0) {
-out = 1;
-} else {
-out = 0;
-}
+out = (d % s-count) != (s-count - 1) || s-gate == 0;
 break;
 case 3:
-out = (d % s-count)  ((s-count + 1)  1);
+out = (d % s-count)  ((s-count + 1)  1) || s-gate == 0;
 break;
 case 4:
 case 5:
-out = (d == s-count);
+out = (d != s-count);
 break;
 }
 return out;
@@ -93,10 +87,10 @@ int64_t pit_get_next_transition_time(PITChannelState *s, 
int64_t current_time)
 break;
 case 2:
 base = (d / s-count) * 

[Qemu-devel] [PATCH v8 00/10] i8254, i8259 and running Microport UNIX (ca 1987)

2012-12-16 Thread Matthew Ogilvie
This series makes a series of mostly-unrelated fixes to allow
running an old Microport UNIX (ca 1987) guest under qemu.

The most intrusive of the fixes is modifying how the i8259 handles
the trailing edge of an interrupt request (the trailing edge ought
to cancel interrupt, even when the line is edge-triggered).

Changes since version 7:
   * Abandon attempts to be fancy about handling cross version migration
 in the i8254 model.  Just fix it directly.  Analysis suggests
 migration issues should essentially always be minor - see comments
 for patch 5.
   * Added qtest-based test/pic-test.c (and associated infrastructure)
 to test the i8259's handling of the trailing edge of an interrupt
 request, to prevent future regressions.

I'm still wondering if maybe the non-i825[49]-related patches should be
split off from the rest of this series?  Any chance some of them (at
least the trivial ones) could just be included immediately?

I still need to refine the KVM patches I posted back in September
to match these latest changes.  Maybe I'll get to that next weekend.


Potential issue with this:


I'm not sure if it is a problem or not, but while figuring out
how to hookup pic-test.c, I noticed the existence of the

qemu_irq_pulse()

function in hw/irq.h.  It is raising an IRQ line only
to immediately lower it again, which for a real i8259
(at least) effectively cancels the interrupt request before
it could ever be serviced.  It is used in the following files:

hw/bonito.c
hw/dma.c
hw/grlib_apbuart.c
hw/grlib_gptimer.c
hw/hpet.c
hw/milkymist-ac97.c
hw/milkymist-minimac2.c
hw/milkymist-pfpu.c
hw/milkymist-softusb.c
hw/milkymist-sysctl.c
hw/milkymist-tmu2.c
hw/omap1.c
hw/omap_gptimer.c
hw/onenand.c
hw/spapr_events.c
hw/spapr_llan.c
hw/spapr_pci.c
hw/spapr_vio.c
hw/spapr_vty.c
hw/stellaris.c
hw/xilinx_ethlite.c

If any of these calls are ever routed into the i8259, then I doubt
they will behave as desired once the i8259 is fixed to handle the
trailing edge of an interrupt request as indicated in the
spec (patch 6).  (That is, the devices use over-simplified interrupt logic
that relies on the currently broken behavior of the i8259 model.)

On the other hand, maybe some or all of these devices are only used
in situations where the the IRQ line is fed into something else (like
an ioapic configured to invert the polarity of the line, or perhaps
some other non-intel interrupt chip that doesn't have this
canceling behavior), in which case they should be fine.

Can anyone shed some light on this?  I don't really know much
of anything about any of the devices in the above list (or any
other devices that might manually do something similar without using
qemu_irq_pulse()), so I'm not sure if there are real problems here
or not.

If more than a couple of these devices are wrong, then I'm tempted to
only fix the cascade interrupt (IRQ2) in the i8259 (leaving others
as-is), until such time as all these device models are improved.

-

Matthew Ogilvie (10):
  fix some debug printf format strings
  vl: fix -hdachs/-hda argument order parsing issues
  qemu-options.hx: mention retrace= VGA option
  vga: add some optional CGA compatibility hacks
  fix i8254 output logic to match the spec
  i8259: fix so that dropping IRQ level always clears the interrupt
request
  i8259: refactor pic_set_irq level logic
  qtest: fix qemu_irq_intercept_out()
  qtest: add set_irq_{in,out} infrastructure for testing interrupt
controllers
  add test/pic-test.c

 hw/cirrus_vga.c   |   4 +-
 hw/i8254.c|  24 ++-
 hw/i8254_common.c |  18 +++-
 hw/i8259.c|  34 +++
 hw/ide/cmd646.c   |   5 ++-
 hw/ide/via.c  |   5 ++-
 hw/irq.c  |  11 +++--
 hw/irq.h  |   2 +-
 hw/pc.h   |   4 ++
 hw/vga.c  |  37 
 qemu-options.hx   |  27 +++-
 qtest.c   |  53 ++-
 tests/Makefile|   2 +
 tests/libqtest.c  |  12 ++
 tests/libqtest.h  |  48 +
 tests/pic-test.c  | 127 ++
 vl.c  |  62 +-
 17 files changed, 403 insertions(+), 72 deletions(-)
 create mode 100644 tests/pic-test.c

-- 
1.7.10.2.484.gcd07cc5




[Qemu-devel] [PATCH v8 03/10] qemu-options.hx: mention retrace= VGA option

2012-12-16 Thread Matthew Ogilvie
The feature was added in commit cb5a7aa8c32141bb Sep 2008.
My description is based on Better VGA retrace emulation (needed
for some DOS games/demos) from
http://www.boblycat.org/~malc/code/patches/qemu/index.html

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 qemu-options.hx | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 231cc4f..c50f737 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1007,7 +1007,7 @@ DEF(vga, HAS_ARG, QEMU_OPTION_vga,
 -vga [std|cirrus|vmware|qxl|xenfb|none]\n
 select video card type\n, QEMU_ARCH_ALL)
 STEXI
-@item -vga @var{type}
+@item -vga @var{type}[,@var{prop}=@var{value}[,...]]
 @findex -vga
 Select type of VGA card to emulate. Valid values for @var{type} are
 @table @option
@@ -1032,6 +1032,12 @@ Recommended choice when using the spice protocol.
 @item none
 Disable VGA card.
 @end table
+Valid optional properties are
+@table @option
+@item retrace=dumb|precise
+Select dumb (default) or precise VGA retrace logic, useful for some
+DOS games/demos.
+@end table
 ETEXI
 
 DEF(full-screen, 0, QEMU_OPTION_full_screen,
-- 
1.7.10.2.484.gcd07cc5




[Qemu-devel] buildbot failure in qemu on block_mingw32

2012-12-16 Thread qemu
The Buildbot has detected a new failure on builder block_mingw32 while building 
qemu.
Full details are available at:
 http://buildbot.b1-systems.de/qemu/builders/block_mingw32/builds/417

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

Buildslave for this Build: kraxel_rhel61

Build Reason: The Nightly scheduler named 'nightly_block' triggered this build
Build Source Stamp: [branch block] HEAD
Blamelist: 

BUILD FAILED: failed git

sincerely,
 -The Buildbot



[Qemu-devel] [PATCH v8 08/10] qtest: fix qemu_irq_intercept_out()

2012-12-16 Thread Matthew Ogilvie
For the 8259 (at least), we need to modify the entries in gpio_out
(which is pointing at PICCommonState::int_out) in-place rather than
change it to point to a totally different table.  The 8259 sends its
output to int_out even if gpio_out is a different table.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/irq.c | 11 ---
 hw/irq.h |  2 +-
 qtest.c  |  2 +-
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/irq.c b/hw/irq.c
index f4e2a78..60fa152 100644
--- a/hw/irq.c
+++ b/hw/irq.c
@@ -129,8 +129,13 @@ void qemu_irq_intercept_in(qemu_irq *gpio_in, 
qemu_irq_handler handler, int n)
 }
 }
 
-void qemu_irq_intercept_out(qemu_irq **gpio_out, qemu_irq_handler handler, int 
n)
+void qemu_irq_intercept_out(qemu_irq *gpio_out, qemu_irq_handler handler, int 
n)
 {
-qemu_irq *old_irqs = *gpio_out;
-*gpio_out = qemu_allocate_irqs(handler, old_irqs, n);
+int i;
+qemu_irq *old_irqs = qemu_allocate_irqs(NULL, NULL, n);
+for (i = 0; i  n; i++) {
+*old_irqs[i] = *gpio_out[i];
+gpio_out[i]-handler = handler;
+gpio_out[i]-opaque = old_irqs;
+}
 }
diff --git a/hw/irq.h b/hw/irq.h
index 610e6b7..8dc26cf 100644
--- a/hw/irq.h
+++ b/hw/irq.h
@@ -52,6 +52,6 @@ qemu_irq *qemu_irq_proxy(qemu_irq **target, int n);
 /* For internal use in qtest.  Similar to qemu_irq_split, but operating
on an existing vector of qemu_irq.  */
 void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n);
-void qemu_irq_intercept_out(qemu_irq **gpio_out, qemu_irq_handler handler, int 
n);
+void qemu_irq_intercept_out(qemu_irq *gpio_out, qemu_irq_handler handler, int 
n);
 
 #endif
diff --git a/qtest.c b/qtest.c
index fbfab4e..6965910 100644
--- a/qtest.c
+++ b/qtest.c
@@ -232,7 +232,7 @@ static void qtest_process_command(CharDriverState *chr, 
gchar **words)
 }
 
 if (words[0][14] == 'o') {
-qemu_irq_intercept_out(dev-gpio_out, qtest_irq_handler, 
dev-num_gpio_out);
+qemu_irq_intercept_out(dev-gpio_out, qtest_irq_handler, 
dev-num_gpio_out);
 } else {
 qemu_irq_intercept_in(dev-gpio_in, qtest_irq_handler, 
dev-num_gpio_in);
 }
-- 
1.7.10.2.484.gcd07cc5




Re: [Qemu-devel] [PATCH 2/2] qemu-img:report size overflow error message

2012-12-16 Thread li guang
在 2012-12-14五的 13:19 +0100,Markus Armbruster写道:
 liguang lig.f...@cn.fujitsu.com writes:
 
  qemu-img will complain when qcow or qcow2
  size overflow for 64 bits, report the right
  message in this condition.
 
  before change:
  qemu-img: Invalid image size specified! You may use k, M, G or T suffixes 
  for
  qemu-img: kilobytes, megabytes, gigabytes and terabytes.
 
  after change:
  qemu-img: Image size must be less than 8 exabytes!
 
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   qemu-img.c |7 ++-
   1 files changed, 6 insertions(+), 1 deletions(-)
 
  diff --git a/qemu-img.c b/qemu-img.c
  index e29e01b..1c3af67 100644
  --- a/qemu-img.c
  +++ b/qemu-img.c
  @@ -346,13 +346,18 @@ static int img_create(int argc, char **argv)
   int64_t sval;
   char *end;
   sval = strtosz_suffix(argv[optind++], end, STRTOSZ_DEFSUFFIX_B);
  -if (sval  0 || *end) {
  +if (sval == EINVAL || *end) {
   error_report(Invalid image size specified! You may use k, M, 
  G or 
 T suffixes for );
   error_report(kilobytes, megabytes, gigabytes and terabytes.);
   ret = -1;
   goto out;
   }
  +if (sval == ERANGE) {
  +error_report(Image size must be less than 8 exabytes!);
  +ret = -1;
  +goto out;
  +}
   img_size = (uint64_t)sval;
   }
 
 If strtosz_suffix() ever acquires additional error codes, this caller
 will fail to detect the failure.  Try something like
 
 if (sval  0 || *end) {
 if (sval == -ERANGE) {
 error_report(Image size must be less than 8 exabytes!);
 } else {
 error_report(Invalid image size specified!
You may use k, M, G or T suffixes for);
 error_report(kilobytes, megabytes, gigabytes and 
 terabytes.);
 }
 ret = -1;
 goto out;
 }
 
 To be pedantically correct, 8 exabytes should be 8 Exbibytes or 8
 EiB.

yes, it's better.
Thanks!

-- 
regards!
li guang




Re: [Qemu-devel] [PATCH 1/2] cutils:change strtosz_suffix_unit function

2012-12-16 Thread li guang
在 2012-12-14五的 13:09 +0100,Markus Armbruster写道:
 liguang lig.f...@cn.fujitsu.com writes:
 
  if value to be translated is larger than INT64_MAX,
  this function will not be convenient for caller to
  be aware of it, so change a little for this.
 
  Signed-off-by: liguang lig.f...@cn.fujitsu.com
  ---
   cutils.c |3 ++-
   1 files changed, 2 insertions(+), 1 deletions(-)
 
  diff --git a/cutils.c b/cutils.c
  index 4f0692f..da05c9e 100644
  --- a/cutils.c
  +++ b/cutils.c
/*
 * Convert string to bytes, allowing either B/b for bytes, K/k for KB,
 * M/m for MB, G/g for GB or T/t for TB. End pointer will be returned
 * in *end, if not NULL. Return -1 on error.
 */
  @@ -219,7 +219,7 @@ static int64_t suffix_mul(char suffix, int64_t unit)
   int64_t strtosz_suffix_unit(const char *nptr, char **end,
   const char default_suffix, int64_t unit)
   {
  -int64_t retval = -1;
  +int64_t retval = EINVAL;
   char *endptr;
   unsigned char c;
   int mul_required = 0;
  @@ -246,6 +246,7 @@ int64_t strtosz_suffix_unit(const char *nptr, char 
  **end,
   goto fail;
   }
   if ((val * mul = INT64_MAX) || val  0) {
  +retval = ERANGE;
   goto fail;
   }
   retval = val * mul;
 
 Your error codes aren't negative, and you failed to update the function
 comment!
OK, will fix.
-- 
regards!
li guang




[Qemu-devel] buildbot failure in qemu on block_openbsd_4.9

2012-12-16 Thread qemu
The Buildbot has detected a new failure on builder block_openbsd_4.9 while 
building qemu.
Full details are available at:
 http://buildbot.b1-systems.de/qemu/builders/block_openbsd_4.9/builds/417

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

Buildslave for this Build: kraxel_openbsd49

Build Reason: The Nightly scheduler named 'nightly_block' triggered this build
Build Source Stamp: [branch block] HEAD
Blamelist: 

BUILD FAILED: failed git

sincerely,
 -The Buildbot



[Qemu-devel] [PATCH v8 10/10] add test/pic-test.c

2012-12-16 Thread Matthew Ogilvie
Microport UNIX System V/386 v2.1 (ca. 1987) does something similar
to the slave_imr() test, and doesn't run if the canceling behavior
of IRQ2 doesn't work.

I don't know anything that depends on the canceling behavior of
other interrupts (master_basic() or slave_basic() tests), but
it seems best to fix the issue generically if possible.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 tests/Makefile   |   2 +
 tests/pic-test.c | 127 +++
 2 files changed, 129 insertions(+)
 create mode 100644 tests/pic-test.c

diff --git a/tests/Makefile b/tests/Makefile
index b60f0fb..d21b0d5 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -25,6 +25,7 @@ check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
 check-qtest-i386-y = tests/fdc-test$(EXESUF)
 check-qtest-i386-y += tests/hd-geo-test$(EXESUF)
 check-qtest-i386-y += tests/rtc-test$(EXESUF)
+check-qtest-i386-y += tests/pic-test$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 check-qtest-sparc-y = tests/m48t59-test$(EXESUF)
 check-qtest-sparc64-y = tests/m48t59-test$(EXESUF)
@@ -75,6 +76,7 @@ tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o 
tests/test-qmp-marsh
 tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o 
$(test-qapi-obj-y)
 
 tests/rtc-test$(EXESUF): tests/rtc-test.o $(trace-obj-y)
+tests/pic-test$(EXESUF): tests/pic-test.o $(trace-obj-y)
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o $(trace-obj-y)
 tests/fdc-test$(EXESUF): tests/fdc-test.o tests/libqtest.o $(trace-obj-y)
 tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o tests/libqtest.o $(trace-obj-y)
diff --git a/tests/pic-test.c b/tests/pic-test.c
new file mode 100644
index 000..57b8bee
--- /dev/null
+++ b/tests/pic-test.c
@@ -0,0 +1,127 @@
+/*
+ * QTest testcase for the 8259 interrupt controller
+ *
+ * Copyright (c) 2012 Matthew Ogilvie
+ *
+ * Authors:
+ *  Matthew Ogilvie mmogilvi_q...@miniinfo.net
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#include libqtest.h
+
+#include glib.h
+
+static void init_i8259(void)
+{
+/* master: */
+outb(0x20,0x13); /* ICW1: edge-triggered, multiple 8259s, ICW4 needed */
+outb(0x21,0x08); /* ICW2: map to int08-int0f */
+outb(0x21,0x04); /* ICW3: IR2 connected to slave */
+outb(0x21,0x01); /* ICW4: not-SFNM, not-buffered, manual EOI, 8086 mode */
+
+outb(0x21,0xfa); /* OCW1/IMR: mask off all except IRQ2 and IRQ0 */
+
+/* slave: */
+outb(0xa0,0x13); /* ICW1: edge-triggered, multiple 8259s, ICW4 needed */
+outb(0xa1,0x70); /* ICW2: map to int70-int77 */
+outb(0xa1,0x02); /* ICW3: connected to master IR2 */
+outb(0xa1,0x01); /* ICW4: not-SFNM, not-buffered, manual EOI, 8086 mode */
+
+outb(0xa1,0x3f); /* OCW1/IMR: mask off all except IRQ14 and IRQ15 */
+}
+
+static void set_irq(int num, int level)
+{
+if(num  8) {
+set_irq_in(i8259-slave, num - 8, level);
+} else {
+set_irq_in(i8259-master, num, level);
+}
+}
+
+static void slave_imr(void)
+{
+init_i8259();
+
+g_assert_cmpint(get_irq(0), ==, 0);
+set_irq(14, 1);
+g_assert_cmpint(get_irq(0), ==, 1);
+
+outb(0xa1,0xff); /* OCW1/IMR: mask off all slave IRQs */
+g_assert_cmpint(get_irq(0), ==, 0);
+
+outb(0xa0,0x0a); /* slave OCW3: read IRR */
+g_assert_cmpint(inb(0xa0)0x40, ==, 0x40); /* IRR still has IRQ14 */
+outb(0x20,0x0a); /* master OCW3: read IRR */
+g_assert_cmpint(inb(0x20)0x04, ==, 0x00); /* IRR does not have IRQ2 */
+
+outb(0xa1,0x3f); /* OCW1/IMR: unmask IRQ14 and IRQ15 */
+g_assert_cmpint(get_irq(0), ==, 1);
+
+outb(0xa0,0x0a); /* slave OCW3: read IRR */
+g_assert_cmpint(inb(0xa0)0x40, ==, 0x40); /* IRR has IRQ14 */
+outb(0x20,0x0a); /* master OCW3: read IRR */
+g_assert_cmpint(inb(0x20)0x04, ==, 0x04); /* IRR has IRQ2 */
+}
+
+static void master_cancel(void)
+{
+init_i8259();
+
+g_assert_cmpint(get_irq(0), ==, 0);
+set_irq(0, 1);
+g_assert_cmpint(get_irq(0), ==, 1);
+set_irq(0, 0);
+g_assert_cmpint(get_irq(0), ==, 0);
+}
+
+static void slave_cancel(void)
+{
+init_i8259();
+
+g_assert_cmpint(get_irq(0), ==, 0);
+set_irq(14, 1);
+g_assert_cmpint(get_irq(0), ==, 1);
+set_irq(14, 0);
+g_assert_cmpint(get_irq(0), ==, 0);
+}
+
+int main(int argc, char **argv)
+{
+QTestState *s = NULL;
+int ret;
+
+g_test_init(argc, argv, NULL);
+
+s = qtest_start(/*-qtest-log /dev/tty */ -display none);
+qtest_irq_intercept_out(s, i8259-master);
+
+/* FUTURE: Before testing obscure cases, first test basic
+ *  interrupt delivery to the CPU and EOI functionality.
+ *  But that requires some kind of abstracted hook into
+ *  cpu_get_pic_interrupt() vs other architecture equivalents,
+ *  and the abstraction doesn't currently exist.
+ */
+
+/* I know of at least one (admittedly 

[Qemu-devel] [PATCH v8 02/10] vl: fix -hdachs/-hda argument order parsing issues

2012-12-16 Thread Matthew Ogilvie
Without this patch, the -hdachs argument had to occur either
BEFORE the corresponding -hda option, or AFTER the plain
disk image name (if neither -hda nor -drive is used).  Otherwise
it would effectively be ignored.

Option -hdachs still has no effect on -drive, but that seems best.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 vl.c | 39 ++-
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/vl.c b/vl.c
index 3ebf01f..12786f0 100644
--- a/vl.c
+++ b/vl.c
@@ -2528,8 +2528,9 @@ int main(int argc, char **argv, char **envp)
 const char *kernel_filename, *kernel_cmdline;
 char boot_devices[33] = cad; /* default to HD-floppy-CD-ROM */
 DisplayState *ds;
-int cyls, heads, secs, translation;
-QemuOpts *hda_opts = NULL, *opts, *machine_opts;
+char hdachs_params[512];  /* save -hdachs to apply to later -hda */
+QemuOpts *hda_opts = NULL; /* save -hda to be modified by later -hdachs */
+QemuOpts *opts, *machine_opts;
 QemuOptsList *olist;
 int optind;
 const char *optarg;
@@ -2584,8 +2585,7 @@ int main(int argc, char **argv, char **envp)
 cpu_model = NULL;
 ram_size = 0;
 snapshot = 0;
-cyls = heads = secs = 0;
-translation = BIOS_ATA_TRANSLATION_AUTO;
+snprintf(hdachs_params, sizeof(hdachs_params), %s, HD_OPTS);
 
 for (i = 0; i  MAX_NODES; i++) {
 node_mem[i] = 0;
@@ -2633,7 +2633,7 @@ int main(int argc, char **argv, char **envp)
 if (optind = argc)
 break;
 if (argv[optind][0] != '-') {
-   hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], HD_OPTS);
+hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], hdachs_params);
 } else {
 const QEMUOption *popt;
 
@@ -2656,21 +2656,8 @@ int main(int argc, char **argv, char **envp)
 cpu_model = optarg;
 break;
 case QEMU_OPTION_hda:
-{
-char buf[256];
-if (cyls == 0)
-snprintf(buf, sizeof(buf), %s, HD_OPTS);
-else
-snprintf(buf, sizeof(buf),
- %s,cyls=%d,heads=%d,secs=%d%s,
- HD_OPTS , cyls, heads, secs,
- translation == BIOS_ATA_TRANSLATION_LBA ?
- ,trans=lba :
- translation == BIOS_ATA_TRANSLATION_NONE ?
- ,trans=none : );
-drive_add(IF_DEFAULT, 0, optarg, buf);
-break;
-}
+hda_opts = drive_add(IF_DEFAULT, 0, optarg, hdachs_params);
+break;
 case QEMU_OPTION_hdb:
 case QEMU_OPTION_hdc:
 case QEMU_OPTION_hdd:
@@ -2704,7 +2691,10 @@ int main(int argc, char **argv, char **envp)
 break;
 case QEMU_OPTION_hdachs:
 {
+int cyls, heads, secs, translation;
 const char *p;
+cyls = heads = secs = 0;
+translation = BIOS_ATA_TRANSLATION_AUTO;
 p = optarg;
 cyls = strtol(p, (char **)p, 0);
 if (cyls  1 || cyls  16383)
@@ -2736,7 +2726,14 @@ int main(int argc, char **argv, char **envp)
 fprintf(stderr, qemu: invalid physical CHS format\n);
 exit(1);
 }
-   if (hda_opts != NULL) {
+snprintf(hdachs_params, sizeof(hdachs_params),
+ %s,cyls=%d,heads=%d,secs=%d%s,
+ HD_OPTS , cyls, heads, secs,
+ translation == BIOS_ATA_TRANSLATION_LBA ?
+ ,trans=lba :
+ translation == BIOS_ATA_TRANSLATION_NONE ?
+ ,trans=none : );
+if (hda_opts != NULL) {
 char num[16];
 snprintf(num, sizeof(num), %d, cyls);
 qemu_opt_set(hda_opts, cyls, num);
-- 
1.7.10.2.484.gcd07cc5




Re: [Qemu-devel] [PATCH 1/2] cutils:change strtosz_suffix_unit function

2012-12-16 Thread li guang
在 2012-12-14五的 14:45 +0100,Igor Mammedov写道:
 On Fri, 14 Dec 2012 13:09:17 +0100
 Markus Armbruster arm...@redhat.com wrote:
 
  liguang lig.f...@cn.fujitsu.com writes:
  
   if value to be translated is larger than INT64_MAX,
   this function will not be convenient for caller to
   be aware of it, so change a little for this.
  
   Signed-off-by: liguang lig.f...@cn.fujitsu.com
   ---
cutils.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
  
   diff --git a/cutils.c b/cutils.c
   index 4f0692f..da05c9e 100644
   --- a/cutils.c
   +++ b/cutils.c
 /*
  * Convert string to bytes, allowing either B/b for bytes, K/k for KB,
  * M/m for MB, G/g for GB or T/t for TB. End pointer will be returned
  * in *end, if not NULL. Return -1 on error.
  */
 Size is not the only user of it, this function is used to convert KHz,... in
 target-i386/cpu.c, i.e. unit might be something else than 1024. That's why
 there is question about generalizing strtosz_suffix_unit() in future instead 
 of
 duplicating suffixed int parsing. And specialization for size with
 unit=1024 and enforcing limits could be made in strtosz_suffix() which is
 used by current size users.
 
 
   @@ -219,7 +219,7 @@ static int64_t suffix_mul(char suffix, int64_t unit)
int64_t strtosz_suffix_unit(const char *nptr, char **end,
const char default_suffix, int64_t unit)
{
   -int64_t retval = -1;
   +int64_t retval = EINVAL;
char *endptr;
unsigned char c;
int mul_required = 0;
   @@ -246,6 +246,7 @@ int64_t strtosz_suffix_unit(const char *nptr, char 
   **end,
goto fail;
}
if ((val * mul = INT64_MAX) || val  0) {
   +retval = ERANGE;
goto fail;
}
retval = val * mul;
  
  Your error codes aren't negative, and you failed to update the function
  comment!
 Returning negative here is fine for now form target-i386/cpu.c pov.

OK


-- 
regards!
li guang




[Qemu-devel] [PATCH v8 04/10] vga: add some optional CGA compatibility hacks

2012-12-16 Thread Matthew Ogilvie
This patch adds some optional compatibility hacks (default
disabled) to allow Microport UNIX to function under qemu.

I've tried to structure it to be easy to add more hacks for other
old CGA programs, if anyone ever needs them.

Microport UNIX System V/386 v 2.1 (ca 1987) tries to program
the CGA registers directly with neither the assistance of BIOS, nor
with proper handling of EGA/VGA-only registers.  Note that it didn't
work on real VGA hardware, either (although in that case, the most
obvious problems seemed to be out-of-range hsync and/or vsync
signalling, rather than the issues in this patch).

Eventually real MDA and/or CGA support might provide an alternative to
this patch, although a hybrid approach like this patch might still
be useful in marginal cases.

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/pc.h |  4 
 hw/vga.c| 37 +
 qemu-options.hx | 19 +++
 vl.c| 23 +++
 4 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/hw/pc.h b/hw/pc.h
index 2237e86..9b9538b 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -152,6 +152,10 @@ enum vga_retrace_method {
 
 extern enum vga_retrace_method vga_retrace_method;
 
+#define VGA_CGA_HACK_PALETTE_BLANKING  (10)
+#define VGA_CGA_HACK_FONT_HEIGHT   (11)
+extern int vga_cga_hacks;
+
 int isa_vga_mm_init(hwaddr vram_base,
 hwaddr ctrl_base, int it_shift,
 MemoryRegion *address_space);
diff --git a/hw/vga.c b/hw/vga.c
index c266161..2ddd09d 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -552,14 +552,31 @@ void vga_ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 printf(vga: write CR%x = 0x%02x\n, s-cr_index, val);
 #endif
 /* handle CR0-7 protection */
-if ((s-cr[VGA_CRTC_V_SYNC_END]  VGA_CR11_LOCK_CR0_CR7) 
-s-cr_index = VGA_CRTC_OVERFLOW) {
-/* can always write bit 4 of CR7 */
-if (s-cr_index == VGA_CRTC_OVERFLOW) {
-s-cr[VGA_CRTC_OVERFLOW] = (s-cr[VGA_CRTC_OVERFLOW]  ~0x10) |
-(val  0x10);
+if (s-cr[VGA_CRTC_V_SYNC_END]  VGA_CR11_LOCK_CR0_CR7) {
+if (s-cr_index = VGA_CRTC_OVERFLOW) {
+/* can always write bit 4 of CR7 */
+if (s-cr_index == VGA_CRTC_OVERFLOW) {
+s-cr[VGA_CRTC_OVERFLOW] =
+(s-cr[VGA_CRTC_OVERFLOW]  ~0x10) | (val  0x10);
+}
+return;
+} else if ((vga_cga_hacks  VGA_CGA_HACK_FONT_HEIGHT) 
+   !(s-sr[VGA_SEQ_CLOCK_MODE]  VGA_SR01_CHAR_CLK_8DOTS)) 
{
+/* extra CGA compatibility hacks (not in standard VGA) */
+if (s-cr_index == VGA_CRTC_MAX_SCAN 
+val == 7 
+(s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+return;
+} else if (s-cr_index == VGA_CRTC_CURSOR_START 
+   val == 6 
+   (s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+val = 0xd;
+} else if (s-cr_index == VGA_CRTC_CURSOR_END 
+   val == 7 
+   (s-cr[VGA_CRTC_MAX_SCAN]  0xf) == 0xf) {
+val = 0xe;
+}
 }
-return;
 }
 s-cr[s-cr_index] = val;
 
@@ -1896,7 +1913,11 @@ static void vga_update_display(void *opaque)
 /* nothing to do */
 } else {
 full_update = 0;
-if (!(s-ar_index  0x20)) {
+if (!(s-ar_index  0x20) 
+/* extra CGA compatibility hacks (not in standard VGA) */
+(!(vga_cga_hacks  VGA_CGA_HACK_PALETTE_BLANKING) ||
+ s-ar_index != 0 ||
+ !s-ar_flip_flop)) {
 graphic_mode = GMODE_BLANK;
 } else {
 graphic_mode = s-gr[VGA_GFX_MISC]  VGA_GR06_GRAPHICS_MODE;
diff --git a/qemu-options.hx b/qemu-options.hx
index c50f737..3f1e122 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1037,6 +1037,25 @@ Valid optional properties are
 @item retrace=dumb|precise
 Select dumb (default) or precise VGA retrace logic, useful for some
 DOS games/demos.
+@item cga_hacks=@var{hack1}[+@var{hack2}[+...]]
+Enable various extra CGA compatibility hacks for programs that are
+trying to directly set CGA modes without BIOS assistance nor
+real knowledge of EGA/VGA.  These might only work with -vga std.
+Valid hacks are
+@table @option
+@item palette_blanking
+Wait to blank the screen until palette registers seem to actually be
+modified, instead of blanking it as soon as the palette address bit (0x10)
+of the attribute address register (0x3c0) is cleared.
+@item font_height
+Ignore attempts to change the VGA font height (index 9),
+cursor start (index 10), and cursor end (index 11) of the CRTC control
+registers (0x3d5) if trying to set them to the default for CGA fonts
+instead of VGA 

[Qemu-devel] [PATCH v8 06/10] i8259: fix so that dropping IRQ level always clears the interrupt request

2012-12-16 Thread Matthew Ogilvie
Intel's definition of edge triggered means: asserted with a
low-to-high transition at the time an interrupt is registered and
then kept high until the interrupt is served via one of the
EOI mechanisms or goes away unhandled.

So the only difference between edge triggered and level triggered
is in the leading edge, with no difference in the trailing edge.

This bug manifested itself when the guest was Microport UNIX
System V/386 v2.1 (ca. 1987), because it would sometimes mask
off IRQ14 in the slave IMR after it had already been asserted.
The master would still try to deliver an interrupt even though
IRQ2 had dropped again, resulting in a spurious interupt
(IRQ15) and a panicked kernel.

Output from a test program:
---
Real hardware [Pentium 4]:
  cmdRead unmask IRR=4005 mask IRR=4001 sti unmask irq14 IRR=0001 DONE
[I also see a final IRR= occasionally.  Probably just happened to
ask it while the timer (IRQ0) line is low.  It masks off most IRQ's, including
timer.]
---
Unpatched qemu:
  cmdRead unmask IRR=4015 mask IRR=4015 sti irq15 unmask IRR=4015 DONE
[Presumably IRQ4 (0x10 - qemu's serial device model?) had a transient
edge during initialization, but had been masked off even before I
masked it off?]
---
Patched qemu:
  cmdRead unmask IRR=4005 mask IRR=4001 sti unmask irq14 IRR=0001 DONE
---

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---

There is a risk that some other device models depend on the broken
behavior.  See my question about qemu_irq_pulse() in the cover
letter.

 hw/i8259.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i8259.c b/hw/i8259.c
index 60c25ba..95587cd 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -157,6 +157,7 @@ static void pic_set_irq(void *opaque, int irq, int level)
 }
 s-last_irr |= mask;
 } else {
+s-irr = ~mask;
 s-last_irr = ~mask;
 }
 }
-- 
1.7.10.2.484.gcd07cc5




[Qemu-devel] [PATCH v8 09/10] qtest: add set_irq_{in, out} infrastructure for testing interrupt controllers

2012-12-16 Thread Matthew Ogilvie

Signed-off-by: Matthew Ogilvie mmogilvi_q...@miniinfo.net
---
 hw/i8259.c   |  6 ++
 qtest.c  | 51 +++
 tests/libqtest.c | 12 
 tests/libqtest.h | 48 
 4 files changed, 117 insertions(+)

diff --git a/hw/i8259.c b/hw/i8259.c
index 9b2ec40..5f09f2f 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -453,6 +453,9 @@ qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq)
 }
 
 isa_pic = dev-qdev;
+object_property_add_link(OBJECT(bus),
+ i8259-master, TYPE_DEVICE,
+ (Object **)isa_pic, NULL);
 
 dev = i8259_init_chip(isa-i8259, bus, false);
 
@@ -462,6 +465,9 @@ qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq)
 }
 
 slave_pic = DO_UPCAST(PICCommonState, dev, dev);
+object_property_add_link(OBJECT(bus),
+ i8259-slave, TYPE_DEVICE,
+ (Object **)slave_pic, NULL);
 
 return irq_set;
 }
diff --git a/qtest.c b/qtest.c
index 6965910..d4c2dd7 100644
--- a/qtest.c
+++ b/qtest.c
@@ -117,6 +117,21 @@ static bool qtest_opened;
  * where NUM is an IRQ number.  For the PC, interrupts can be intercepted
  * simply with irq_intercept_in ioapic (note that IRQ0 comes out with
  * NUM=0 even though it is remapped to GSI 2).
+ *
+ * Testing interrupt handler chips like the i8259:
+ *
+ *   set_irq_in QOM-PATH IRQ LEVEL
+ *   OK
+ *
+ *   set_irq_out QOM-PATH IRQ LEVEL
+ *   OK
+ *
+ * Forcibly set the given interrupt pin to the given level (from the
+ * consumer's point of view).
+ *
+ * FUTURE: Some abstracted mechanism to initiate delivery of an
+ *   interrupt to the CPU, returning the interrupt vector number
+ *   that would be delivered to that CPU.
  */
 
 static int hex2nib(char ch)
@@ -239,7 +254,43 @@ static void qtest_process_command(CharDriverState *chr, 
gchar **words)
 irq_intercept_dev = dev;
 qtest_send_prefix(chr);
 qtest_send(chr, OK\n);
+} else if (strcmp(words[0], set_irq_in) == 0 ||
+   strcmp(words[0], set_irq_out) == 0) {
+DeviceState *dev;
+qemu_irq *irqs;
+int num_irqs;
+int num;
+int level;
+
+g_assert(words[1]  words[2]  words[3]);
+
+dev = DEVICE(object_resolve_path(words[1], NULL));
+if (!dev) {
+qtest_send_prefix(chr);
+qtest_send(chr, FAIL Unknown device\n);
+return;
+}
 
+if (words[0][8] == 'o') {
+irqs = dev-gpio_out;
+num_irqs = dev-num_gpio_out;
+} else {
+irqs = dev-gpio_in;
+num_irqs = dev-num_gpio_in;
+}
+
+num = strtol(words[2], NULL, 0);
+if (num  0 || num = num_irqs) {
+qtest_send_prefix(chr);
+qtest_send(chr, FAIL Bad IRQ number\n);
+return;
+}
+
+level = strtol(words[3], NULL, 0);
+
+qemu_set_irq(irqs[num], level != 0);
+qtest_send_prefix(chr);
+qtest_send(chr, OK\n);
 } else if (strcmp(words[0], outb) == 0 ||
strcmp(words[0], outw) == 0 ||
strcmp(words[0], outl) == 0) {
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 71b84c1..f6160ad 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -379,6 +379,18 @@ void qtest_irq_intercept_in(QTestState *s, const char 
*qom_path)
 qtest_rsp(s, 0);
 }
 
+void qtest_set_irq_in(QTestState *s, const char *qom_path, int num, int level)
+{
+qtest_sendf(s, set_irq_in %s %d %d\n, qom_path, num, level);
+qtest_rsp(s, 0);
+}
+
+void qtest_set_irq_out(QTestState *s, const char *qom_path, int num, int level)
+{
+qtest_sendf(s, set_irq_out %s %d %d\n, qom_path, num, level);
+qtest_rsp(s, 0);
+}
+
 static void qtest_out(QTestState *s, const char *cmd, uint16_t addr, uint32_t 
value)
 {
 qtest_sendf(s, %s 0x%x 0x%x\n, cmd, addr, value);
diff --git a/tests/libqtest.h b/tests/libqtest.h
index c8ade85..a045fc7 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -76,6 +76,30 @@ void qtest_irq_intercept_in(QTestState *s, const char 
*string);
 void qtest_irq_intercept_out(QTestState *s, const char *string);
 
 /**
+ * qtest_set_irq_in:
+ * @s: QTestState instance to operate on.
+ * @string: QOM path of a device
+ * @irq: IRQ number
+ * @level: level to set it to (0 or 1)
+ *
+ * Force given device/irq GPIO-in pin to the given level.
+ * Mostly useful for testing interrupt controllers, rather than other devices.
+ */
+void qtest_set_irq_in(QTestState *s, const char *string, int irq, int level);
+
+/**
+ * qtest_set_irq_out:
+ * @s: QTestState instance to operate on.
+ * @string: QOM path of a device
+ * @irq: IRQ number
+ * @level: level to set it to (0 or 1)
+ *
+ * Force given device/irq GPIO-out pin to the given level.
+ * Mostly useful for testing interrupt controllers, rather than other devices.
+ */
+void 

[Qemu-devel] [PATCH v3 2/2] qemu-img:report size overflow error message

2012-12-16 Thread liguang
qemu-img will complain when qcow or qcow2
size overflow for 64 bits, report the right
message in this condition.

$./qemu-img create -f qcow2 /tmp/foo 0x1
before change:
qemu-img: Invalid image size specified! You may use k, M, G or T suffixes for
qemu-img: kilobytes, megabytes, gigabytes and terabytes.

after change:
qemu-img: Image size must be less than 8 EiB!

Signed-off-by: liguang lig.f...@cn.fujitsu.com
---
 qemu-img.c |   16 +++-
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index e29e01b..203e3cb 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -347,11 +347,17 @@ static int img_create(int argc, char **argv)
 char *end;
 sval = strtosz_suffix(argv[optind++], end, STRTOSZ_DEFSUFFIX_B);
 if (sval  0 || *end) {
-error_report(Invalid image size specified! You may use k, M, G or 

-  T suffixes for );
-error_report(kilobytes, megabytes, gigabytes and terabytes.);
-ret = -1;
-goto out;
+if (sval == -ERANGE) {
+error_report(Image size must be less than 8 EiB!);
+ret = -1;
+goto out;
+} else {
+error_report(Invalid image size specified! You may use k, M, 
G or 
+ T suffixes for );
+error_report(kilobytes, megabytes, gigabytes and terabytes.);
+ret = -1;
+goto out;
+}
 }
 img_size = (uint64_t)sval;
 }
-- 
1.7.2.5




[Qemu-devel] [PATCH v3 1/2] cutils:change strtosz_suffix_unit function

2012-12-16 Thread liguang
if value to be translated is larger than INT64_MAX,
this function will not be convenient for caller to
be aware of it, so change a little for this.

Signed-off-by: liguang lig.f...@cn.fujitsu.com
---
 cutils.c |6 --
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/cutils.c b/cutils.c
index 4f0692f..0433e05 100644
--- a/cutils.c
+++ b/cutils.c
@@ -214,12 +214,13 @@ static int64_t suffix_mul(char suffix, int64_t unit)
 /*
  * Convert string to bytes, allowing either B/b for bytes, K/k for KB,
  * M/m for MB, G/g for GB or T/t for TB. End pointer will be returned
- * in *end, if not NULL. Return -1 on error.
+ * in *end, if not NULL. Return -ERANGE on overflow, Return -EINVAL on
+ * other error.
  */
 int64_t strtosz_suffix_unit(const char *nptr, char **end,
 const char default_suffix, int64_t unit)
 {
-int64_t retval = -1;
+int64_t retval = -EINVAL;
 char *endptr;
 unsigned char c;
 int mul_required = 0;
@@ -246,6 +247,7 @@ int64_t strtosz_suffix_unit(const char *nptr, char **end,
 goto fail;
 }
 if ((val * mul = INT64_MAX) || val  0) {
+retval = -ERANGE;
 goto fail;
 }
 retval = val * mul;
-- 
1.7.2.5




Re: [Qemu-devel] [PATCH 13/13] target-ppc: Give a meaningful error if too many threads are specified

2012-12-16 Thread David Gibson
On Thu, Dec 13, 2012 at 02:18:15PM +, Peter Maydell wrote:
 On 13 December 2012 14:17, Peter Maydell peter.mayd...@linaro.org wrote:
  On 13 December 2012 13:19, Alexander Graf ag...@suse.de wrote:
  So because we don't call kvm_arch_init, the cap is 0 and thus we return 1?
 
  No, if we aren't CONFIG_KVM then that source file and version of
  the function aren't compiled at all.
 
 ...and 5 seconds after sending that I realised what you actually
 meant. ignore me :-)

And a bit later, so did I.  I'll revise.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson




Re: [Qemu-devel] [Qemu-ppc] [PATCH 11/13] pseries: Fixes and enhancements to L1 cache properties

2012-12-16 Thread David Gibson
On Thu, Dec 13, 2012 at 01:50:25PM +0100, Alexander Graf wrote:
 
 On 04.12.2012, at 03:42, David Gibson wrote:
 
  PAPR requires that the device tree's CPU nodes have several properties
  with information about the L1 cache.  We created two of these
  properties, but with incorrect names - [id]cache-block-size instead
  of [id]-cache-block-size (note the extra hyphen).
  
  We were also missing some of the required cache properties.  This
  patch adds the [id]-cache-line-size properties (which have the same
  values as the block size properties in all current cases).  We also
  add the [id]-cache-size properties.  The latter requires some extra
  infrastructure in the general target-ppc code to (optionally) set the
  cache sizes for various CPUs.  We obtain the published values either
  from there, or from the host when KVM is in use.
  
  Signed-off-by: David Gibson da...@gibson.dropbear.id.au
  ---
  hw/spapr.c  |   20 ++--
  target-ppc/cpu.h|1 +
  target-ppc/kvm.c|   10 ++
  target-ppc/kvm_ppc.h|   12 
  target-ppc/translate_init.c |4 
  5 files changed, 45 insertions(+), 2 deletions(-)
  
  diff --git a/hw/spapr.c b/hw/spapr.c
  index d23aa9d..3bacf2f 100644
  --- a/hw/spapr.c
  +++ b/hw/spapr.c
  @@ -315,6 +315,10 @@ static void *spapr_create_fdt_skel(const char 
  *cpu_model,
 0x, 0x};
  uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : 
  TIMEBASE_FREQ;
  uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 
  10;
  +int dcache_size = kvm_enabled() ? kvmppc_get_dcache_size()
  +: env-l1_dcache_size;
  +int icache_size = kvm_enabled() ? kvmppc_get_icache_size()
  +: env-l1_icache_size;
 
 By default with KVM we use -cpu host, right? So we already should
 get the correct cache sizes for the CPU you're on.

Um.. sort of.  The first problem with that is that I only just added
the cache size information to qemu, so only a few CPUs currently
populate that information.  Using the host info means we can get the
right information even for CPUs that don't yet have cache info in
qemu.

 Imagine we would support the compatibility feature where you could
 run with -cpu POWER6 on a POWER7 machine. Would exposing the POWER6
 cache size rather than the host's make any real difference to the
 guest? Or would it work nevertheless?

The second problem is that there may be circumstances where the
cache size is altered from the normal size for the cpu.  Running in
POWER6 compat mode is one example, but I'm not sure there aren't
others.  e.g. the hypervisor limiting the amount of cache available to
a partition, or portions of the cache disabled due to a chicken switch
or the like.  In short, when we have the cache size information
directly available from the host, I'd really prefer to use that, over
getting the host PVR and assuming what we have in our table is
correct.

 If it would still work just fine, I'd say ditch the ask the host
 bit and always use the sizes from the cpu definition.

So, if the cache size is wrong it will probably work for most
purposes.  In fact as far as I know, it will always work from a strict
functionality point of view.  But if something in the guest uses the
cache size information to optimize its cache blocking (I'm thinking of
BLAS) it could have a big effect on performance.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson



Re: [Qemu-devel] [PATCH Update] sheepdog: don't update inode when create_and_write fails

2012-12-16 Thread MORITA Kazutaka
At Sat, 15 Dec 2012 17:59:12 +0800,
Liu Yuan wrote:
 
 From: Liu Yuan tailai...@taobao.com
 
 For the error case such as SD_RES_NO_SPACE, we shouldn't update the inode 
 bitmap
 to avoid the scenario that the object is allocated but wasn't created at the
 server side. This will result in VM's IO error on the failed object.
 
 Cc: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp
 Cc: Kevin Wolf kw...@redhat.com
 Signed-off-by: Liu Yuan tailai...@taobao.com
 ---
  Update
   - update the coding style and passed checkpath
 
  block/sheepdog.c |   12 +++-
  1 file changed, 7 insertions(+), 5 deletions(-)
 
 diff --git a/block/sheepdog.c b/block/sheepdog.c
 index a48f58c..6116316 100644
 --- a/block/sheepdog.c
 +++ b/block/sheepdog.c
 @@ -697,6 +697,12 @@ static void coroutine_fn aio_read_response(void *opaque)
  
  acb = aio_req-aiocb;
  
 +if (rsp.result != SD_RES_SUCCESS) {
 +acb-ret = -EIO;
 +error_report(%s, sd_strerror(rsp.result));
 +goto err;
 +}
 +
  switch (acb-aiocb_type) {
  case AIOCB_WRITE_UDATA:
  /* this coroutine context is no longer suitable for co_recv
 @@ -736,11 +742,7 @@ static void coroutine_fn aio_read_response(void *opaque)
  break;
  }
  
 -if (rsp.result != SD_RES_SUCCESS) {
 -acb-ret = -EIO;
 -error_report(%s, sd_strerror(rsp.result));
 -}
 -
 +err:
  free_aio_req(s, aio_req);
  if (!acb-nr_pending) {
  /*

send_pending_req() needs to be called even in error case.  Rather than
moving the error check, I think it looks better to update
s-inode.data_vdi_id only when rsp.result is SD_RES_SUCCESS.

Thanks,

Kazutaka



[Qemu-devel] [PATCH] Added uapi directory into linux-header

2012-12-16 Thread Bharat Bhushan
Linux ARCH specific header files are now in uapi/ directory also.
These header files (epapr_hcalls.h on powerpc) are needed in qemu
in linux-headers/uapi/asm/ directory as these are referenced by
other header files in linux-headers/asm/.

This patch is about changing the scripts for same.

Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
 configure   |1 +
 scripts/update-linux-headers.sh |   10 ++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 38b1cc6..51cce6b 100755
--- a/configure
+++ b/configure
@@ -3724,6 +3724,7 @@ if test $linux = yes ; then
 # For non-KVM architectures we will not have asm headers
 if [ -e $source_path/linux-headers/asm-$linux_arch ]; then
   symlink $source_path/linux-headers/asm-$linux_arch linux-headers/asm
+  symlink $source_path/linux-headers/uapi/asm-$linux_arch 
linux-headers/uapi/asm
 fi
 fi
 
diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
index 4c7b566..d40f9c4 100755
--- a/scripts/update-linux-headers.sh
+++ b/scripts/update-linux-headers.sh
@@ -46,14 +46,24 @@ for arch in $ARCHLIST; do
 
 make -C $linux INSTALL_HDR_PATH=$tmpdir SRCARCH=$arch headers_install
 
+if ! [ -e $output/linux-headers/uapi ] ; then
+mkdir $output/linux-headers/uapi
+fi
+
 rm -rf $output/linux-headers/asm-$arch
 mkdir -p $output/linux-headers/asm-$arch
+rm -rf $output/linux-headers/uapi/asm-$arch
+mkdir -p $output/linux-headers/uapi/asm-$arch
+
 for header in kvm.h kvm_para.h; do
 cp $tmpdir/include/asm/$header $output/linux-headers/asm-$arch
 done
 if [ $arch = x86 ]; then
 cp $tmpdir/include/asm/hyperv.h $output/linux-headers/asm-x86
 fi
+if [ $arch = powerpc ]; then
+cp $linux/arch/$arch/include/uapi/asm/epapr_hcalls.h 
$output/linux-headers/uapi/asm-$arch/
+fi
 done
 
 rm -rf $output/linux-headers/linux
-- 
1.7.0.4





Re: [Qemu-devel] main-loop.c: About Select handling

2012-12-16 Thread Furukawa, Eiji

Sorry,I was misunderstanding it. 

If The main loop will iterate again, it is unquestionable.

--
E.Furukawa

 On Thu, Dec 06, 2012 at 12:30:19AM +, Furukawa, Eiji wrote:
 
  The select() do not masked signal in os_host_main_loop_wait.
 
  For example, qemu-timer.c gives SIGALRM regularly.
  Even if a buffer of select is empty,
 
 What does a buffer of select is empty mean?
 
  I think that it is a problem that this select() does exit by this SIGALRM.
 
 The main loop will iterate again.  Why is it a problem if select() returns?
 
 Stefan




[Qemu-devel] [PATCH] pci: Rework handling of multiple host bridges (pci domains)

2012-12-16 Thread David Gibson
Currently the qemu core PCI code doesn't support a machine having
multiple independent PCI host bridges (aka PCI domains).  It does have
some incomplete stubs for such support, but unfortunately what's there
is based on a bogus premise.

PCI domains are often referenced by a PCI domain number, and the
current code stores a domain number for each root-level PCI bus
(although it's always set to 0 so far).  But by definition, the domain
number never appears on the PCI bus itself, so domain numbers are
assigned purely by convention.  Some platforms have a well defined
convention, for example on PC the domain numbers are assigned by
ACPI.  However other platforms have no strong convention, and so
domain numbering is performed entirely by the guest and we have no
reasonable way of knowing what it will pick.

Fixing this is complicated by the fact that the domain numbers are
exposed in migration streams via pcibus_get_dev_path().  We also can't
easily switch to using the qbus name as our identifier, because
libvirt and other tools often expect a bus named simply pci to exist
- including on platforms that would need the identifier  to
match existing device paths.

This patch replaces the domain number in the PCIHostBus structure with
a string root bus id.  The idea is that platforms which do have a
strong domain convention can derive this from the domain number, other
platforms can use whatever technique is appropriate to generate a
stable, clear id for each PCI host bridge.  The functions for creating
new PCI buses / host bridges now take an id.  If this is NULL, it is
treated as  which makes the device paths backwards compatible
with existing migration streams.

Platforms are updated to supply root bus ids as follows:
 * Machines I believe support migration (x86, ARM), use NULL so that
the device paths don't change.

 * Machines that appear only to support a single PCI host bridge, but
which as far as I know don't (yet) support migration (i.e. most
things), use pci as the root bus ID.

 * PPC4xx can in theory support multiple host bridges in real
hardware, although our current models never construct that.  In this
case we use pci@ for the root bus ID, where  is the host
bridge's base IO address (on the system bus).  This changes device
paths, but migration is so broken on PPC at present it doesn't matter.

 * pseries (aka sPAPR) has more extensive existing support for
multiple host bridges, since multiple (indeed, numerous) PCI host
bridges are routine on real hardware (pseries is emulating an
existing defined para-virtualized environment).  Here we use
pci@ as the root bus id, where  is the BUID
(firmware supplied ID for the host bridge, which is used in hypervisor
calls).

This patch leaves pci_parse_devaddr() (and therefore its callers)
broken for machines that don't use the backwards compat  id.
I'm looking at fixing that in a later version of this patch.

Cc: Michael S. Tsirkin m...@redhat.com

Signed-off-by: David Gibson da...@gibson.dropbear.id.au
---
 hw/alpha_typhoon.c |2 +-
 hw/apb_pci.c   |2 +-
 hw/bonito.c|2 +-
 hw/grackle_pci.c   |2 +-
 hw/gt64xxx.c   |2 +-
 hw/pci-hotplug.c   |   22 +++
 hw/pci.c   |  104 
 hw/pci.h   |9 +++--
 hw/pcie_aer.c  |4 +-
 hw/piix_pci.c  |2 +-
 hw/ppc4xx_pci.c|5 ++-
 hw/ppce500_pci.c   |2 +-
 hw/prep_pci.c  |2 +-
 hw/q35.c   |2 +-
 hw/sh_pci.c|2 +-
 hw/spapr_pci.c |2 +-
 hw/unin_pci.c  |4 +-
 hw/versatile_pci.c |2 +-
 18 files changed, 103 insertions(+), 69 deletions(-)

Michael,

I know this is pretty ugly, but I haven't spotted any better way of
doing things, given th constraint of maintaining migration stream
compatibility.  Does something along these lines seem reasonable, or
is there another approach you'd prefer for fixing the domain problem.

diff --git a/hw/alpha_typhoon.c b/hw/alpha_typhoon.c
index 9b16d96..0736c8c 100644
--- a/hw/alpha_typhoon.c
+++ b/hw/alpha_typhoon.c
@@ -769,7 +769,7 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
 memory_region_add_subregion(addr_space, 0x801fc00ULL,
 s-pchip.reg_io);
 
-b = pci_register_bus(dev, pci,
+b = pci_register_bus(dev, pci, pci,
  typhoon_set_irq, sys_map_irq, s,
  s-pchip.reg_mem, addr_space_io, 0, 64);
 phb-bus = b;
diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index 054814f..893864f 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -377,7 +377,7 @@ PCIBus *pci_apb_init(hwaddr special_base,
 memory_region_init(d-pci_mmio, pci-mmio, 0x1ULL);
 memory_region_add_subregion(get_system_memory(), mem_base, d-pci_mmio);
 
-d-bus = pci_register_bus(d-busdev.qdev, pci,
+d-bus = pci_register_bus(d-busdev.qdev, pci, pci,
   pci_apb_set_irq, 

Re: [Qemu-devel] [PATCH Update] sheepdog: don't update inode when create_and_write fails

2012-12-16 Thread Liu Yuan
On 12/17/2012 11:43 AM, MORITA Kazutaka wrote:
 send_pending_req() needs to be called even in error case.  Rather than
 moving the error check, I think it looks better to update
 s-inode.data_vdi_id only when rsp.result is SD_RES_SUCCESS.

Why can't we check the rsp.result in the first place? double check
rsp.result inside one function looks a little bit complicated to me.

How about
+if (rsp.result != SD_RES_SUCCESS) {
+acb-ret = -EIO;
+error_report(%s, sd_strerror(rsp.result));
+send_pending_req(s, aio_req-oid);
+goto err;
+
+}

By the way, seems that
  send_pending_req(s, vid_to_data_oid(s-inode.vdi_id, idx));
can be reduced to
  send_pending_req(s, aio_req-oid);

If yes, I can send another patch to fix.

Thanks,
Yuan



Re: [Qemu-devel] [PATCH V8 3/4] Use QemuOpts support in block layer

2012-12-16 Thread Bharata B Rao
On Fri, Dec 14, 2012 at 7:34 PM, Dong Xu Wang
wdon...@linux.vnet.ibm.com wrote:
 diff --git a/block/gluster.c b/block/gluster.c
 index 1c90174..daabb61 100644
 --- a/block/gluster.c
 +++ b/block/gluster.c
 @@ -335,8 +335,7 @@ out:
  return ret;
  }

 -static int qemu_gluster_create(const char *filename,
 -QEMUOptionParameter *options)
 +static int qemu_gluster_create(const char *filename, QemuOpts* opts)
  {
  struct glfs *glfs;
  struct glfs_fd *fd;
 @@ -350,11 +349,9 @@ static int qemu_gluster_create(const char *filename,
  goto out;
  }

 -while (options  options-name) {
 -if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
 -total_size = options-value.n / BDRV_SECTOR_SIZE;
 -}
 -options++;
 +if (opts) {
 +total_size =
 +qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) /
 BDRV_SECTOR_SIZE;
  }

  fd = glfs_creat(glfs, gconf-image,
 @@ -544,13 +541,17 @@ static void qemu_gluster_close(BlockDriverState *bs)
  glfs_fini(s-glfs);
  }

 -static QEMUOptionParameter qemu_gluster_create_options[] = {
 -{
 -.name = BLOCK_OPT_SIZE,
 -.type = OPT_SIZE,
 -.help = Virtual disk size
 -},
 -{ NULL }
 +static QemuOptsList gluster_create_opts = {
 +.name = gluster-create-opts,
 +.head = QTAILQ_HEAD_INITIALIZER(gluster_create_opts.head),
 +.desc = {
 +{
 +.name = BLOCK_OPT_SIZE,
 +.type = QEMU_OPT_NUMBER,
 +.help = Virtual disk size
 +},
 +{ /* end of list */ }
 +}
  };

  static BlockDriver bdrv_gluster = {
 @@ -565,7 +566,7 @@ static BlockDriver bdrv_gluster = {
  .bdrv_aio_readv   = qemu_gluster_aio_readv,
  .bdrv_aio_writev  = qemu_gluster_aio_writev,
  .bdrv_aio_flush   = qemu_gluster_aio_flush,
 -.create_options   = qemu_gluster_create_options,
 +.create_options   = gluster_create_opts,

While other elements have qemu_ prefix, create_options doesn't after
your change.

  };

  static BlockDriver bdrv_gluster_tcp = {
 @@ -580,7 +581,7 @@ static BlockDriver bdrv_gluster_tcp = {
  .bdrv_aio_readv   = qemu_gluster_aio_readv,
  .bdrv_aio_writev  = qemu_gluster_aio_writev,
  .bdrv_aio_flush   = qemu_gluster_aio_flush,
 -.create_options   = qemu_gluster_create_options,
 +.create_options   = gluster_create_opts,
  };

  static BlockDriver bdrv_gluster_unix = {

Missed doing the same change for bdrv_gluster_unix and bdrv_gluster_rdma ?

Regards,
Bharata.



[Qemu-devel] QMP command implementation help

2012-12-16 Thread Shraddha Kamat
I am implementing couple of QMP commands to Qemu - so trying a 
hello-world command first as shown here
https://github.com/qemu/qemu/blob/master/docs/writing-qmp-commands.txt

Step 1 :

Added to qapi-schema.json file this :
{ 'command': 'hello-world' }

Step 2:
===
Added to qmp.c :

void qmp_hello_world(Error **errp)
{
printf(Hello, world!\n);
}

Step 3 :

Added to qmp-commands.hx :
   {
.name = hello-world,
.args_type = ,
.mhandler.cmd_new = qmp_marshal_input_hello_world,
   },

and build upstream Qemu-kvm : 


Then tried to test the command hello-world :

# telnet localhost 
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
{
QMP: {
version: {
qemu: {
micro: 50, 
minor: 2, 
major: 1
}, 
package:  (qemu-kvm-devel)
}, 
capabilities: [
]
}
}
{ execute: qmp_capabilities }
{
return: {
}
}
{ execute: hello-world }
{
error: {
class: CommandNotFound, 
desc: The command hello-world has not been found
}
}
--

I debugged a bit - 

qemu-kvm/x86_64-softmmu/qmp-commands-old.h
has this entry :
{
.name   = hello-world,
.args_type  = ,
.mhandler.cmd_new = qmp_marshal_input_hello_world,
},

but in monitor.c/qmp_find_cmd() - qmp_cmds doesn't have 
this entry and hence the error !!

Note that qemu_cmds is defined as 

static const mon_cmd_t qmp_cmds[] = {
#include qmp-commands-old.h
{ /* NULL */ },
};

in monitor.c. 

What might be causing qmp-commands-old.h not to load qmp_cmds[] with
hello-world command entry ? Please help.

-- Shraddha




Re: [Qemu-devel] [PATCH Update] sheepdog: don't update inode when create_and_write fails

2012-12-16 Thread MORITA Kazutaka
At Mon, 17 Dec 2012 13:22:31 +0800,
Liu Yuan wrote:
 
 On 12/17/2012 11:43 AM, MORITA Kazutaka wrote:
  send_pending_req() needs to be called even in error case.  Rather than
  moving the error check, I think it looks better to update
  s-inode.data_vdi_id only when rsp.result is SD_RES_SUCCESS.
 
 Why can't we check the rsp.result in the first place? double check
 rsp.result inside one function looks a little bit complicated to me.
 
 How about
 +if (rsp.result != SD_RES_SUCCESS) {
 +acb-ret = -EIO;
 +error_report(%s, sd_strerror(rsp.result));
 +send_pending_req(s, aio_req-oid);
 +goto err;
 +
 +}

Either is okay, but s-co_recv = NULL is necessary before
send_pending_req() because we can receive another response while
sending pending requests.  In addition, it is better to call
send_pending_req() only when we update inode; in most cases, we don't
need to traverse pending list.

 
 By the way, seems that
   send_pending_req(s, vid_to_data_oid(s-inode.vdi_id, idx));
 can be reduced to
   send_pending_req(s, aio_req-oid);
 
 If yes, I can send another patch to fix.

Looks correct.

Thanks,

Kazutaka



[Qemu-devel] [PATCH 1/3] Synchronized the linux headers

2012-12-16 Thread Bharat Bhushan
This is needed for the watchdog patches (following up this one)

Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
 linux-headers/asm-generic/kvm_para.h  |4 +
 linux-headers/asm-powerpc/kvm.h   |   86 ++
 linux-headers/asm-powerpc/kvm_para.h  |7 +-
 linux-headers/linux/kvm.h |   21 -
 linux-headers/uapi/asm-powerpc/epapr_hcalls.h |   98 +
 5 files changed, 208 insertions(+), 8 deletions(-)
 create mode 100644 linux-headers/asm-generic/kvm_para.h
 create mode 100644 linux-headers/uapi/asm-powerpc/epapr_hcalls.h

diff --git a/linux-headers/asm-generic/kvm_para.h 
b/linux-headers/asm-generic/kvm_para.h
new file mode 100644
index 000..486f0af
--- /dev/null
+++ b/linux-headers/asm-generic/kvm_para.h
@@ -0,0 +1,4 @@
+/*
+ * There isn't anything here, but the file must not be empty or patch
+ * will delete it.
+ */
diff --git a/linux-headers/asm-powerpc/kvm.h b/linux-headers/asm-powerpc/kvm.h
index 1bea4d8..2fba8a6 100644
--- a/linux-headers/asm-powerpc/kvm.h
+++ b/linux-headers/asm-powerpc/kvm.h
@@ -221,6 +221,12 @@ struct kvm_sregs {
 
__u32 dbsr; /* KVM_SREGS_E_UPDATE_DBSR */
__u32 dbcr[3];
+   /*
+* iac/dac registers are 64bit wide, while this API
+* interface provides only lower 32 bits on 64 bit
+* processors. ONE_REG interface is added for 64bit
+* iac/dac registers.
+*/
__u32 iac[4];
__u32 dac[2];
__u32 dvc[2];
@@ -325,6 +331,86 @@ struct kvm_book3e_206_tlb_params {
__u32 reserved[8];
 };
 
+/* For KVM_PPC_GET_HTAB_FD */
+struct kvm_get_htab_fd {
+   __u64   flags;
+   __u64   start_index;
+   __u64   reserved[2];
+};
+
+/* Values for kvm_get_htab_fd.flags */
+#define KVM_GET_HTAB_BOLTED_ONLY   ((__u64)0x1)
+#define KVM_GET_HTAB_WRITE ((__u64)0x2)
+
+/*
+ * Data read on the file descriptor is formatted as a series of
+ * records, each consisting of a header followed by a series of
+ * `n_valid' HPTEs (16 bytes each), which are all valid.  Following
+ * those valid HPTEs there are `n_invalid' invalid HPTEs, which
+ * are not represented explicitly in the stream.  The same format
+ * is used for writing.
+ */
+struct kvm_get_htab_header {
+   __u32   index;
+   __u16   n_valid;
+   __u16   n_invalid;
+};
+
 #define KVM_REG_PPC_HIOR   (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x1)
+#define KVM_REG_PPC_IAC1   (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x2)
+#define KVM_REG_PPC_IAC2   (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x3)
+#define KVM_REG_PPC_IAC3   (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x4)
+#define KVM_REG_PPC_IAC4   (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x5)
+#define KVM_REG_PPC_DAC1   (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x6)
+#define KVM_REG_PPC_DAC2   (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x7)
+#define KVM_REG_PPC_DABR   (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x8)
+#define KVM_REG_PPC_DSCR   (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x9)
+#define KVM_REG_PPC_PURR   (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xa)
+#define KVM_REG_PPC_SPURR  (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xb)
+#define KVM_REG_PPC_DAR(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xc)
+#define KVM_REG_PPC_DSISR  (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0xd)
+#define KVM_REG_PPC_AMR(KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xe)
+#define KVM_REG_PPC_UAMOR  (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0xf)
+
+#define KVM_REG_PPC_MMCR0  (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x10)
+#define KVM_REG_PPC_MMCR1  (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x11)
+#define KVM_REG_PPC_MMCRA  (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x12)
+
+#define KVM_REG_PPC_PMC1   (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x18)
+#define KVM_REG_PPC_PMC2   (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x19)
+#define KVM_REG_PPC_PMC3   (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1a)
+#define KVM_REG_PPC_PMC4   (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1b)
+#define KVM_REG_PPC_PMC5   (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1c)
+#define KVM_REG_PPC_PMC6   (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1d)
+#define KVM_REG_PPC_PMC7   (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1e)
+#define KVM_REG_PPC_PMC8   (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x1f)
+
+/* 32 floating-point registers */
+#define KVM_REG_PPC_FPR0   (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x20)
+#define KVM_REG_PPC_FPR(n) (KVM_REG_PPC_FPR0 + (n))
+#define KVM_REG_PPC_FPR31  (KVM_REG_PPC | KVM_REG_SIZE_U64 | 0x3f)
+
+/* 32 VMX/Altivec vector registers */
+#define KVM_REG_PPC_VR0(KVM_REG_PPC | KVM_REG_SIZE_U128 | 0x40)
+#define KVM_REG_PPC_VR(n)  (KVM_REG_PPC_VR0 + (n))
+#define KVM_REG_PPC_VR31   (KVM_REG_PPC | KVM_REG_SIZE_U128 | 0x5f)
+
+/* 32 double-width FP registers for VSX */
+/* 

[Qemu-devel] [PATCH 3/3] Enable kvm emulated watchdog

2012-12-16 Thread Bharat Bhushan
Enable the KVM emulated watchdog if KVM supports (use the
capability enablement in watchdog handler). Also watchdog exit
(KVM_EXIT_WATCHDOG) handling is added.
Watchdog state machine is cleared whenever VM state changes to running.
This is to handle the cases like return from debug halt etc.

Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
 hw/ppc.h |2 +
 hw/ppc_booke.c   |   71 ++
 target-ppc/kvm.c |   13 +-
 3 files changed, 85 insertions(+), 1 deletions(-)

diff --git a/hw/ppc.h b/hw/ppc.h
index 2f3ea27..3672fe8 100644
--- a/hw/ppc.h
+++ b/hw/ppc.h
@@ -44,6 +44,8 @@ struct ppc_tb_t {
 
 uint64_t cpu_ppc_get_tb(ppc_tb_t *tb_env, uint64_t vmclk, int64_t tb_offset);
 clk_setup_cb cpu_ppc_tb_init (CPUPPCState *env, uint32_t freq);
+extern int cap_ppc_watchdog;
+extern int cap_booke_sregs;
 /* Embedded PowerPC DCR management */
 typedef uint32_t (*dcr_read_cb)(void *opaque, int dcrn);
 typedef void (*dcr_write_cb)(void *opaque, int dcrn, uint32_t val);
diff --git a/hw/ppc_booke.c b/hw/ppc_booke.c
index 837a5b6..f18df74 100644
--- a/hw/ppc_booke.c
+++ b/hw/ppc_booke.c
@@ -21,6 +21,8 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+#include sysemu.h
+#include kvm.h
 #include hw.h
 #include ppc.h
 #include qemu-timer.h
@@ -203,6 +205,11 @@ static void booke_wdt_cb(void *opaque)
  booke_timer-wdt_timer);
 }
 
+static void ppc_booke_watchdog_clear_tsr(CPUPPCState *env, target_ulong tsr)
+{
+env-spr[SPR_BOOKE_TSR] = tsr  ~(TSR_ENW | TSR_WIS | TSR_WRS_MASK);
+}
+
 void store_booke_tsr(CPUPPCState *env, target_ulong val)
 {
 env-spr[SPR_BOOKE_TSR] = ~val;
@@ -241,6 +248,64 @@ static void ppc_booke_timer_reset_handle(void *opaque)
 booke_update_irq(env);
 }
 
+static void cpu_state_change_handler(void *opaque, int running, RunState state)
+{
+CPUPPCState *env = opaque;
+
+struct kvm_sregs sregs;
+
+if (!running) {
+return;
+}
+
+/*
+ * Clear watchdog interrupt condition by clearing TSR.
+ * Similar logic needed to be implemented for watchdog
+ * emulation in qemu.
+ */
+
+if (!kvm_enabled()) {
+/* FIXME: add handling for qemu emulated case */
+return;
+}
+
+if (cap_booke_sregs  cap_ppc_watchdog) {
+kvm_vcpu_ioctl(env, KVM_GET_SREGS, sregs);
+
+/* Clear TSR.ENW, TSR.WIS and TSR.WRS */
+ppc_booke_watchdog_clear_tsr(env, sregs.u.e.tsr);
+sregs.u.e.tsr = env-spr[SPR_BOOKE_TSR];
+sregs.u.e.update_special = KVM_SREGS_E_UPDATE_TSR;
+
+kvm_vcpu_ioctl(env, KVM_SET_SREGS, sregs);
+}
+}
+
+static int kvm_booke_watchdog_enable(CPUPPCState *env)
+{
+int ret;
+struct kvm_enable_cap encap = {};
+
+if (!kvm_enabled()) {
+return 0;
+}
+
+if (!cap_ppc_watchdog) {
+printf(warning: KVM does not support watchdog);
+return 0;
+}
+
+encap.cap = KVM_CAP_PPC_BOOKE_WATCHDOG;
+ret = kvm_vcpu_ioctl(env, KVM_ENABLE_CAP, encap);
+if (ret  0) {
+fprintf(stderr, %s: couldn't enable KVM_CAP_PPC_BOOKE_WATCHDOG: %s\n,
+__func__, strerror(-ret));
+return ret;
+}
+
+return ret;
+}
+
 void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t flags)
 {
 ppc_tb_t *tb_env;
@@ -262,5 +327,11 @@ void ppc_booke_timers_init(CPUPPCState *env, uint32_t 
freq, uint32_t flags)
 booke_timer-wdt_timer =
 qemu_new_timer_ns(vm_clock, booke_wdt_cb, env);
 
+if (kvm_enabled()) {
+kvm_booke_watchdog_enable(env);
+}
+
+qemu_add_vm_change_state_handler(cpu_state_change_handler, env);
+
 qemu_register_reset(ppc_booke_timer_reset_handle, env);
 }
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 3f5df57..3d5f86a 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -32,6 +32,7 @@
 #include device_tree.h
 #include hw/sysbus.h
 #include hw/spapr.h
+#include hw/watchdog.h
 
 #include hw/sysbus.h
 #include hw/spapr.h
@@ -56,11 +57,12 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 static int cap_interrupt_unset = false;
 static int cap_interrupt_level = false;
 static int cap_segstate;
-static int cap_booke_sregs;
+int cap_booke_sregs;
 static int cap_ppc_smt;
 static int cap_ppc_rma;
 static int cap_spapr_tce;
 static int cap_hior;
+int cap_ppc_watchdog;
 
 /* XXX We have a race condition where we actually have a level triggered
  * interrupt, but the infrastructure can't expose that yet, so the guest
@@ -90,6 +92,7 @@ int kvm_arch_init(KVMState *s)
 cap_ppc_rma = kvm_check_extension(s, KVM_CAP_PPC_RMA);
 cap_spapr_tce = kvm_check_extension(s, KVM_CAP_SPAPR_TCE);
 cap_hior = kvm_check_extension(s, KVM_CAP_PPC_HIOR);
+cap_ppc_watchdog = kvm_check_extension(s, KVM_CAP_PPC_BOOKE_WATCHDOG);
 
 if (!cap_interrupt_level) {
 fprintf(stderr, KVM: Couldn't find level irq capability. 

[Qemu-devel] [PATCH 2/3] Reset qemu timers when guest reset

2012-12-16 Thread Bharat Bhushan
This patch install the timer reset handler. This will be called when
the guest is reset.

Signed-off-by: Bharat Bhushan bharat.bhus...@freescale.com
---
 hw/ppc_booke.c |   12 
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/hw/ppc_booke.c b/hw/ppc_booke.c
index d51e7fa..837a5b6 100644
--- a/hw/ppc_booke.c
+++ b/hw/ppc_booke.c
@@ -231,6 +231,16 @@ void store_booke_tcr(CPUPPCState *env, target_ulong val)
 
 }
 
+static void ppc_booke_timer_reset_handle(void *opaque)
+{
+CPUPPCState *env = opaque;
+
+env-spr[SPR_BOOKE_TSR] = 0;
+env-spr[SPR_BOOKE_TCR] = 0;
+
+booke_update_irq(env);
+}
+
 void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, uint32_t flags)
 {
 ppc_tb_t *tb_env;
@@ -251,4 +261,6 @@ void ppc_booke_timers_init(CPUPPCState *env, uint32_t freq, 
uint32_t flags)
 qemu_new_timer_ns(vm_clock, booke_fit_cb, env);
 booke_timer-wdt_timer =
 qemu_new_timer_ns(vm_clock, booke_wdt_cb, env);
+
+qemu_register_reset(ppc_booke_timer_reset_handle, env);
 }
-- 
1.7.0.4





[Qemu-devel] [PATCH 2/6] snapshot: add error set function

2012-12-16 Thread Wenchao Xia
  Added two function which will try replace the error if it is
already set, so only last error is reported.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
 error.c |   23 +++
 error.h |9 +
 2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/error.c b/error.c
index 128d88c..5a82f8e 100644
--- a/error.c
+++ b/error.c
@@ -43,6 +43,29 @@ void error_set(Error **errp, ErrorClass err_class, const 
char *fmt, ...)
 *errp = err;
 }
 
+void error_set_replace(Error **errp, ErrorClass err_class, const char *fmt, 
...)
+{
+Error *err;
+va_list ap;
+
+if (errp == NULL) {
+return;
+}
+if (*errp != NULL) {
+error_free(*errp);
+*errp = NULL;
+}
+
+err = g_malloc0(sizeof(*err));
+
+va_start(ap, fmt);
+err-msg = g_strdup_vprintf(fmt, ap);
+va_end(ap);
+err-err_class = err_class;
+
+*errp = err;
+}
+
 void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
  const char *fmt, ...)
 {
diff --git a/error.h b/error.h
index 4d52e73..8039408 100644
--- a/error.h
+++ b/error.h
@@ -29,6 +29,9 @@ typedef struct Error Error;
  */
 void error_set(Error **err, ErrorClass err_class, const char *fmt, ...) 
GCC_FMT_ATTR(3, 4);
 
+void error_set_replace(Error **err, ErrorClass err_class, const char *fmt, ...)
+GCC_FMT_ATTR(3, 4);
+
 /**
  * Set an indirect pointer to an error given a ErrorClass value and a
  * printf-style human message, followed by a strerror() string if
@@ -43,6 +46,12 @@ void error_set_errno(Error **err, int os_error, ErrorClass 
err_class, const char
 error_set(err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__)
 #define error_setg_errno(err, os_error, fmt, ...) \
 error_set_errno(err, os_error, ERROR_CLASS_GENERIC_ERROR, fmt, ## 
__VA_ARGS__)
+#define error_setg_replace(err, fmt, ...) do { \
+if (*err != NULL) { \
+error_free(*err); \
+ } \
+error_set(err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__); \
+} while (/*CONSTCOND*/0)
 
 /**
  * Returns true if an indirect pointer to an error is pointing to a valid
-- 
1.7.1





[Qemu-devel] [PATCH 5/6] snapshot: qmp interface

2012-12-16 Thread Wenchao Xia
  This patch changes the implemtion of external block snapshot
to use internal unified interface, now qmp handler just do
a translation of request and submit.
  Also internal block snapshot qmp interface was added.
  Now add external snapshot, add/delete internal snapshot
can be started in their own qmp interface or a group of
BlockAction in qmp transaction interface.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
 blockdev.c   |  352 +++---
 qapi-schema.json |  102 ++--
 2 files changed, 292 insertions(+), 162 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 1c38c67..d1bbe23 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1175,6 +1175,195 @@ delete_and_fail:
 return -1;
 }
 
+/* translation from qmp commands */
+static int fill_blk_trsact_create_sync(BlockdevSnapshot *create_sync,
+   BlkTransactionStatesSync *st_sync,
+   SNTime *time,
+   const char *time_str,
+   Error **errp)
+{
+const char *format = qcow2;
+enum NewImageMode mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
+enum SnapshotType type = SNAPSHOT_TYPE_EXTERNAL;
+BlockDriverState *bs;
+
+const char *device = create_sync-device;
+const char *name = create_sync-snapshot_file;
+if (create_sync-has_mode) {
+mode = create_sync-mode;
+}
+if (create_sync-has_format) {
+format = create_sync-format;
+}
+if (create_sync-has_type) {
+type = create_sync-type;
+}
+
+/* find the target bs */
+bs = bdrv_find(device);
+if (!bs) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+return -1;
+}
+
+switch (type) {
+case SNAPSHOT_TYPE_INTERNAL:
+st_sync-type = BLK_SNAPSHOT_INTERNAL;
+break;
+case SNAPSHOT_TYPE_EXTERNAL:
+st_sync-type = BLK_SNAPSHOT_EXTERNAL;
+break;
+default:
+st_sync-type = BLK_SNAPSHOT_NOSUPPORT;
+error_setg(errp, Device %s requested invalid snapshot
+  type %d., device, type);
+return -1;
+}
+
+switch (mode) {
+case NEW_IMAGE_MODE_EXISTING:
+st_sync-use_existing = TRUE;
+break;
+case NEW_IMAGE_MODE_ABSOLUTE_PATHS:
+st_sync-use_existing = FALSE;
+break;
+default:
+error_setg(errp, Device %s requested invalid snapshot
+  mode %d., device, mode);
+return -1;
+}
+
+if (st_sync-type == BLK_SNAPSHOT_INTERNAL) {
+/* internal case, if caller need create new one with default string */
+if (((name == NULL) || (name[0] == '\0')) 
+   (!st_sync-use_existing)) {
+st_sync-internal.sn_name = time_str;
+} else {
+st_sync-internal.sn_name = name;
+}
+st_sync-internal.bs = bs;
+st_sync-internal.time = *time;
+} else if (st_sync-type == BLK_SNAPSHOT_EXTERNAL) {
+st_sync-external.new_image_file = name;
+st_sync-external.format = format;
+st_sync-external.old_bs = bs;
+}
+
+return 0;
+}
+static int fill_blk_trsact_delete_sync(BlockdevSnapshotDelete *delete_sync,
+   BlkTransactionStatesSync *st_sync,
+   Error **errp)
+{
+enum SnapshotType type = SNAPSHOT_TYPE_EXTERNAL;
+BlockDriverState *bs;
+
+const char *device = delete_sync-device;
+const char *name = delete_sync-snapshot_file;
+if (delete_sync-has_type) {
+type = delete_sync-type;
+}
+
+/* find the target bs */
+bs = bdrv_find(device);
+if (!bs) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+return -1;
+}
+
+switch (type) {
+case SNAPSHOT_TYPE_INTERNAL:
+st_sync-type = BLK_SNAPSHOT_INTERNAL;
+break;
+case SNAPSHOT_TYPE_EXTERNAL:
+st_sync-type = BLK_SNAPSHOT_EXTERNAL;
+break;
+default:
+st_sync-type = BLK_SNAPSHOT_NOSUPPORT;
+error_setg(errp, Device %s requested invalid snapshot
+  type %d., device, type);
+return -1;
+}
+
+if (st_sync-type == BLK_SNAPSHOT_INTERNAL) {
+st_sync-internal.sn_name = name;
+st_sync-internal.bs = bs;
+} else if (st_sync-type == BLK_SNAPSHOT_EXTERNAL) {
+st_sync-external.new_image_file = name;
+st_sync-external.old_bs = bs;
+}
+
+return 0;
+}
+
+static int fill_blk_trsact(BlockdevAction *dev_info,
+   BlkTransactionStates *states,
+   SNTime *time,
+   const char *time_str,
+   Error **errp)
+{
+switch (dev_info-kind) {
+case BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC:
+states-st_sync.op = BLK_SN_SYNC_CREATE;
+states-async = FALSE;
+return 

Re: [Qemu-devel] [PATCH V8 3/4] Use QemuOpts support in block layer

2012-12-16 Thread Dong Xu Wang
On Mon, Dec 17, 2012 at 1:44 PM, Bharata B Rao bharata@gmail.com wrote:
 On Fri, Dec 14, 2012 at 7:34 PM, Dong Xu Wang
 wdon...@linux.vnet.ibm.com wrote:
 diff --git a/block/gluster.c b/block/gluster.c
 index 1c90174..daabb61 100644
 --- a/block/gluster.c
 +++ b/block/gluster.c
 @@ -335,8 +335,7 @@ out:
  return ret;
  }

 -static int qemu_gluster_create(const char *filename,
 -QEMUOptionParameter *options)
 +static int qemu_gluster_create(const char *filename, QemuOpts* opts)
  {
  struct glfs *glfs;
  struct glfs_fd *fd;
 @@ -350,11 +349,9 @@ static int qemu_gluster_create(const char *filename,
  goto out;
  }

 -while (options  options-name) {
 -if (!strcmp(options-name, BLOCK_OPT_SIZE)) {
 -total_size = options-value.n / BDRV_SECTOR_SIZE;
 -}
 -options++;
 +if (opts) {
 +total_size =
 +qemu_opt_get_number(opts, BLOCK_OPT_SIZE, 0) /
 BDRV_SECTOR_SIZE;
  }

  fd = glfs_creat(glfs, gconf-image,
 @@ -544,13 +541,17 @@ static void qemu_gluster_close(BlockDriverState *bs)
  glfs_fini(s-glfs);
  }

 -static QEMUOptionParameter qemu_gluster_create_options[] = {
 -{
 -.name = BLOCK_OPT_SIZE,
 -.type = OPT_SIZE,
 -.help = Virtual disk size
 -},
 -{ NULL }
 +static QemuOptsList gluster_create_opts = {
 +.name = gluster-create-opts,
 +.head = QTAILQ_HEAD_INITIALIZER(gluster_create_opts.head),
 +.desc = {
 +{
 +.name = BLOCK_OPT_SIZE,
 +.type = QEMU_OPT_NUMBER,
 +.help = Virtual disk size
 +},
 +{ /* end of list */ }
 +}
  };

  static BlockDriver bdrv_gluster = {
 @@ -565,7 +566,7 @@ static BlockDriver bdrv_gluster = {
  .bdrv_aio_readv   = qemu_gluster_aio_readv,
  .bdrv_aio_writev  = qemu_gluster_aio_writev,
  .bdrv_aio_flush   = qemu_gluster_aio_flush,
 -.create_options   = qemu_gluster_create_options,
 +.create_options   = gluster_create_opts,

 While other elements have qemu_ prefix, create_options doesn't after
 your change.

Okay, will rename in next version.

  };

  static BlockDriver bdrv_gluster_tcp = {
 @@ -580,7 +581,7 @@ static BlockDriver bdrv_gluster_tcp = {
  .bdrv_aio_readv   = qemu_gluster_aio_readv,
  .bdrv_aio_writev  = qemu_gluster_aio_writev,
  .bdrv_aio_flush   = qemu_gluster_aio_flush,
 -.create_options   = qemu_gluster_create_options,
 +.create_options   = gluster_create_opts,
  };

  static BlockDriver bdrv_gluster_unix = {

 Missed doing the same change for bdrv_gluster_unix and bdrv_gluster_rdma ?


Yep, I miss them. Will add them in next verson.

Thank you  Bharata.
 Regards,
 Bharata.




[Qemu-devel] [PATCH 1/6] snapshot: export function in block.c

2012-12-16 Thread Wenchao Xia
  This patch moves bdrv_snapshotfind from savevm.c to block.c and export
it, also added bdrv_deappend in block.c.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
 block.c  |   30 ++
 block.h  |3 +++
 savevm.c |   22 --
 3 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/block.c b/block.c
index 0668c4b..61c7c6a 100644
--- a/block.c
+++ b/block.c
@@ -1376,6 +1376,13 @@ void bdrv_append(BlockDriverState *bs_new, 
BlockDriverState *bs_top)
 bs_new-drv ? bs_new-drv-format_name : );
 }
 
+/* revert the action */
+void bdrv_deappend(BlockDriverState *bs_new, BlockDriverState *bs_top)
+{
+bdrv_swap(bs_new, bs_top);
+/* this is enough? */
+}
+
 void bdrv_delete(BlockDriverState *bs)
 {
 assert(!bs-dev);
@@ -3210,6 +3217,29 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
 return -ENOTSUP;
 }
 
+int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
+   const char *name)
+{
+QEMUSnapshotInfo *sn_tab, *sn;
+int nb_sns, i, ret;
+
+ret = -ENOENT;
+nb_sns = bdrv_snapshot_list(bs, sn_tab);
+if (nb_sns  0) {
+return ret;
+}
+for (i = 0; i  nb_sns; i++) {
+sn = sn_tab[i];
+if (!strcmp(sn-id_str, name) || !strcmp(sn-name, name)) {
+*sn_info = *sn;
+ret = 0;
+break;
+}
+}
+g_free(sn_tab);
+return ret;
+}
+
 /* backing_file can either be relative, or absolute, or a protocol.  If it is
  * relative, it must be relative to the chain.  So, passing in bs-filename
  * from a BDS as backing_file should not be done, as that may be relative to
diff --git a/block.h b/block.h
index 893448a..2322b13 100644
--- a/block.h
+++ b/block.h
@@ -130,6 +130,7 @@ BlockDriverState *bdrv_new(const char *device_name);
 void bdrv_make_anon(BlockDriverState *bs);
 void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
 void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
+void bdrv_deappend(BlockDriverState *bs_new, BlockDriverState *bs_top);
 void bdrv_delete(BlockDriverState *bs);
 int bdrv_parse_cache_flags(const char *mode, int *flags);
 int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags);
@@ -330,6 +331,8 @@ int bdrv_snapshot_list(BlockDriverState *bs,
 int bdrv_snapshot_load_tmp(BlockDriverState *bs,
const char *snapshot_name);
 char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn);
+int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
+   const char *name);
 
 char *get_human_readable_size(char *buf, int buf_size, int64_t size);
 int path_is_absolute(const char *path);
diff --git a/savevm.c b/savevm.c
index 5d04d59..c027529 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2061,28 +2061,6 @@ out:
 return ret;
 }
 
-static int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
-  const char *name)
-{
-QEMUSnapshotInfo *sn_tab, *sn;
-int nb_sns, i, ret;
-
-ret = -ENOENT;
-nb_sns = bdrv_snapshot_list(bs, sn_tab);
-if (nb_sns  0)
-return ret;
-for(i = 0; i  nb_sns; i++) {
-sn = sn_tab[i];
-if (!strcmp(sn-id_str, name) || !strcmp(sn-name, name)) {
-*sn_info = *sn;
-ret = 0;
-break;
-}
-}
-g_free(sn_tab);
-return ret;
-}
-
 /*
  * Deletes snapshots of a given name in all opened images.
  */
-- 
1.7.1





[Qemu-devel] [PATCH v2 1/2] sheepdog: don't update inode when create_and_write fails

2012-12-16 Thread Liu Yuan
From: Liu Yuan tailai...@taobao.com

For the error case such as SD_RES_NO_SPACE, we shouldn't update the inode bitmap
to avoid the scenario that the object is allocated but wasn't created at the
server side. This will result in VM's IO error on the failed object.

Cc: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp
Cc: Kevin Wolf kw...@redhat.com
Signed-off-by: Liu Yuan tailai...@taobao.com
---
 block/sheepdog.c |9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index a48f58c..ef7bc81 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -714,10 +714,11 @@ static void coroutine_fn aio_read_response(void *opaque)
  * and max_dirty_data_idx are changed to include updated
  * index between them.
  */
-s-inode.data_vdi_id[idx] = s-inode.vdi_id;
-s-max_dirty_data_idx = MAX(idx, s-max_dirty_data_idx);
-s-min_dirty_data_idx = MIN(idx, s-min_dirty_data_idx);
-
+if (rsp.result == SD_RES_SUCCESS) {
+s-inode.data_vdi_id[idx] = s-inode.vdi_id;
+s-max_dirty_data_idx = MAX(idx, s-max_dirty_data_idx);
+s-min_dirty_data_idx = MIN(idx, s-min_dirty_data_idx);
+}
 /*
  * Some requests may be blocked because simultaneous
  * create requests are not allowed, so we search the
-- 
1.7.9.5




[Qemu-devel] [PATCH v2 2/2] sheepdog: pass oid directly to send_pending_req()

2012-12-16 Thread Liu Yuan
From: Liu Yuan tailai...@taobao.com

Cc: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp
Cc: Kevin Wolf kw...@redhat.com
Signed-off-by: Liu Yuan tailai...@taobao.com
---
 block/sheepdog.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index ef7bc81..ceabc00 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -724,7 +724,7 @@ static void coroutine_fn aio_read_response(void *opaque)
  * create requests are not allowed, so we search the
  * pending requests here.
  */
-send_pending_req(s, vid_to_data_oid(s-inode.vdi_id, idx));
+send_pending_req(s, aio_req-oid);
 }
 break;
 case AIOCB_READ_UDATA:
-- 
1.7.9.5




[Qemu-devel] [PATCH 4/6] snapshot: implemention of common API to take snapshots

2012-12-16 Thread Wenchao Xia
  This patch is the implemention of transaction based snapshot
internal API. Internal snapshot for specified block device
is added, which can be used as part of functionality in one way
to live full back up vm seperating internal block snapshot and
vmstate(vmstate goes to another file, not implemented in this
patch).
  Every request's handler need to have three function:
execution, updating, cancelling. Also another check function
could be provided to check if request is valid before execition.
  internal snapshot implemention was based on the code from
diet...@proxmox.com.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
Signed-off-by: Dietmar Maurer diet...@proxmox.com
---
 blockdev.c |  515 
 1 files changed, 515 insertions(+), 0 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 9a05e57..1c38c67 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -660,6 +660,521 @@ void do_commit(Monitor *mon, const QDict *qdict)
 }
 }
 
+/*   snapshot functions.
+ *   following are implemention around core struct BlkTransactionStates.
+ * to start, invalidate, cancel the action.
+ */
+
+/* Block internal snapshot(qcow2, sheepdog image...) support.
+   checking and execution was splitted to enable a check for every device
+before execution in group case. */
+
+SNTime get_sn_time(void)
+{
+SNTime time;
+/* is uint32_t big enough to get time_t value on other machine ? */
+#ifdef _WIN32
+struct _timeb tb;
+_ftime(tb);
+time.date_sec = tb.time;
+time.date_nsec = tb.millitm * 100;
+#else
+struct timeval tv;
+gettimeofday(tv, NULL);
+time.date_sec = tv.tv_sec;
+time.date_nsec = tv.tv_usec * 1000;
+#endif
+time.vm_clock_nsec = qemu_get_clock_ns(vm_clock);
+return time;
+}
+
+void generate_sn_name_from_time(SNTime *time, char *time_str, int size)
+{
+#ifdef _WIN32
+time_t t = time-date_sec;
+struct tm *ptm = localtime(t);
+strftime(time_str, size, vm-%Y%m%d%H%M%S, ptm);
+#else
+/* cast below needed for OpenBSD where tv_sec is still 'long' */
+time_t second = time-date_sec;
+struct tm tm;
+localtime_r((const time_t *)second, tm);
+strftime(time_str, size, vm-%Y%m%d%H%M%S, tm);
+#endif
+}
+
+static int blk_sn_check_create_internal_sync(BlkTransactionStates *states,
+ Error **errp)
+{
+BlkTransactionStatesSync *st_sync = states-st_sync;
+BlockDriverState *bs = st_sync-internal.bs;
+const char *device = bdrv_get_device_name(bs);
+QEMUSnapshotInfo *old_sn = st_sync-internal.old_sn;
+const char *name = st_sync-internal.sn_name;
+bool use_existing = st_sync-use_existing;
+int ret;
+
+if (!bdrv_is_inserted(bs)) {
+error_set_replace(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
+return -1;
+}
+
+if (bdrv_is_read_only(bs)) {
+error_set_replace(errp, QERR_DEVICE_IS_READ_ONLY, device);
+return -1;
+}
+
+if (!bdrv_can_snapshot(bs)) {
+error_set_replace(errp, QERR_NOT_SUPPORTED);
+return -1;
+}
+
+ret = bdrv_snapshot_find(bs, old_sn, name);
+if (ret = 0) {
+st_sync-internal.old_sn_exist = TRUE;
+} else {
+if (use_existing) {
+/* caller require use existing one */
+error_set_replace(errp, ERROR_CLASS_GENERIC_ERROR,
+  snapshot '%s' not exists on '%s' but caller 
+  want to use it.,
+  name, device);
+return -1;
+}
+}
+
+st_sync-internal.step = BLK_SNAPSHOT_INT_START;
+return 0;
+}
+
+static int blk_sn_create_internal_sync(BlkTransactionStates *states,
+   Error **errp)
+{
+BlkTransactionStatesSync *st_sync = states-st_sync;
+BlockDriverState *bs = st_sync-internal.bs;
+const char *name = st_sync-internal.sn_name;
+QEMUSnapshotInfo *sn = st_sync-internal.sn;
+int ret = 0;
+const char *device = bdrv_get_device_name(bs);
+QEMUSnapshotInfo *old_sn = st_sync-internal.old_sn;
+
+SNTime *time = st_sync-internal.time;
+if (time-vm_clock_nsec == 0) {
+/* not preset, it need to be set */
+error_setg_replace(errp,
+   Invalid timestamp was set on for '%s' on '%s'\n,
+   name, device);
+return -1;
+}
+
+sn-date_sec = time-date_sec;
+sn-date_nsec = time-date_nsec;
+sn-vm_clock_nsec = time-vm_clock_nsec;
+sn-vm_state_size = st_sync-internal.vm_state_size;
+
+if (st_sync-internal.old_sn_exist) {
+ret = bdrv_snapshot_delete(bs, old_sn-id_str);
+if (ret  0) {
+error_setg_replace(errp,
+   Error in deleting existing snapshot %s on '%s'\n,
+   name, device);
+return -1;
+}
+pstrcpy(sn-name, sizeof(sn-name), old_sn-name);
+pstrcpy(sn-id_str, 

[Qemu-devel] [PATCH 3/6] snapshot: design of common API to take snapshots

2012-12-16 Thread Wenchao Xia
  This patch added API to take snapshots in unified style for
both internal or external type. The core structure is based
on transaction, for that there is a qmp interface need to support
, qmp_transaction, so all operations are packed as requests.
  In this way a sperate internal layer for snapshot is splitted
out from qmp layer, and now qmp can just translate the user request
and fill in internal API. Internal API use params defined inside
qemu, so other component inside qemu can use it without considering
the qmp's parameter format.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
 blockdev.h |  129 
 1 files changed, 129 insertions(+), 0 deletions(-)

diff --git a/blockdev.h b/blockdev.h
index d73d552..4a1b508 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -66,4 +66,133 @@ void qmp_change_blockdev(const char *device, const char 
*filename,
  bool has_format, const char *format, Error **errp);
 void do_commit(Monitor *mon, const QDict *qdict);
 int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
+
+
+/*   snapshot transaction API.
+ *   Split out a layer around core struct BlkTransactionStates, so other
+ *  component in qemu can fill the request and simply use the API to submit,
+ *  QMP may just use part of the API's function, no need to expose all internal
+ *  function to user.
+ */
+
+/* sync snapshot */
+
+typedef enum BlkTransactionOperationSync {
+BLK_SN_SYNC_CREATE = 0,
+BLK_SN_SYNC_DELETE,
+} BlkTransactionOperationSync;
+
+/* internal snapshot */
+
+typedef struct SNTime {
+uint32_t date_sec; /* UTC date of the snapshot */
+uint32_t date_nsec;
+uint64_t vm_clock_nsec; /* VM clock relative to boot */
+} SNTime;
+
+typedef enum BlkSnapshotIntStep {
+BLK_SNAPSHOT_INT_START = 0,
+BLK_SNAPSHOT_INT_CREATED,
+BLK_SNAPSHOT_INT_CANCELED,
+} BlkSnapshotIntStep;
+
+typedef struct BlkSnapshotInternal {
+/* caller input */
+const char *sn_name; /* must be set in create/delete. */
+BlockDriverState *bs; /* must be set in create/delete */
+SNTime time; /* must be set in create. */
+uint64_t vm_state_size; /* optional, default is 0, only valid in create. */
+/* following were used internal */
+QEMUSnapshotInfo sn;
+QEMUSnapshotInfo old_sn;
+bool old_sn_exist;
+BlkSnapshotIntStep step;
+} BlkSnapshotInternal;
+
+/* external snapshot */
+
+typedef enum BlkSnapshotExtStep {
+BLK_SNAPSHOT_EXT_START = 0,
+BLK_SNAPSHOT_EXT_CREATED,
+BLK_SNAPSHOT_EXT_INVALIDATED,
+BLK_SNAPSHOT_EXT_CANCELED,
+} BlkSnapshotExtStep;
+
+typedef struct BlkSnapshotExternal {
+/* caller input */
+const char *new_image_file; /* must be set in create/delete. */
+BlockDriverState *old_bs; /* must be set in create/delete. */
+const char *format; /* must be set in create. */
+/* following were used internal */
+BlockDriverState *new_bs;
+BlockDriver *format_drv;
+BlkSnapshotExtStep step;
+} BlkSnapshotExternal;
+
+typedef enum BlkSnapshotType {
+BLK_SNAPSHOT_INTERNAL = 0,
+BLK_SNAPSHOT_EXTERNAL,
+BLK_SNAPSHOT_NOSUPPORT,
+} BlkSnapshotType;
+
+/* for simple sync type params were all put here ignoring the difference of
+   different operation type as create/delete. */
+typedef struct BlkTransactionStatesSync {
+/* caller input */
+BlkSnapshotType type;
+union {
+BlkSnapshotInternal internal;
+BlkSnapshotExternal external;
+};
+bool use_existing;
+BlkTransactionOperationSync op;
+} BlkTransactionStatesSync;
+
+/* async snapshot, not supported now */
+typedef struct BlkTransactionStatesAsync {
+int reserved;
+} BlkTransactionStatesAsync;
+
+/* Core structure for group snapshots, fill in it and then call the API. */
+typedef struct BlkTransactionStates BlkTransactionStates;
+
+struct BlkTransactionStates {
+/* caller input */
+bool async;
+union {
+BlkTransactionStatesSync st_sync;
+BlkTransactionStatesSync st_async;
+};
+/* following were used internal */
+Error *err;
+int (*blk_trans_do)(BlkTransactionStates *states, Error **errp);
+int (*blk_trans_invalid)(BlkTransactionStates *states, Error **errp);
+int (*blk_trans_cancel)(BlkTransactionStates *states, Error **errp);
+QSIMPLEQ_ENTRY(BlkTransactionStates) entry;
+};
+
+typedef QSIMPLEQ_HEAD(snap_bdrv_states, BlkTransactionStates) \
+  BlkTransactionStatesList;
+
+/* API */
+BlkTransactionStates *blk_trans_st_new(void);
+void blk_trans_st_delete(BlkTransactionStates **p_st);
+BlkTransactionStatesList *blk_trans_st_list_new(void);
+void blk_trans_st_list_delete(BlkTransactionStatesList **p_list);
+
+/* add a request to list, request would be checked to see if it is valid,
+   return -1 when met error and states would not be queued. */
+int add_transaction(BlkTransactionStatesList *list,
+BlkTransactionStates *states,
+  

Re: [Qemu-devel] [PATCH 4/6] snapshot: implemention of common API to take snapshots

2012-12-16 Thread Dietmar Maurer
   This patch is the implemention of transaction based snapshot internal API.

What exactly is the advantage of using qmp transactions (as opposed to the pve 
snapshot patches)?
Seems this makes it impossible to use external commands to make snapshots? 




[Qemu-devel] [PATCH 0/3] Enable and Handle in-kernel watchdog emulation

2012-12-16 Thread Bharat Bhushan
The Patchset enables and handle the in-kernel watchdog emulation
if KVM supports.
The first Patch is synchronizes the Linux-headers as this is needed
for enabling the in-kernel watchdog emulation.
The Second Patch installs the timer reset handler and the Third patch
enables and handles the watchdog exit.

Bharat Bhushan (3):
  Synchronized the linux headers
  Reset qemu timers when guest reset
  Enable kvm emulated watchdog

 hw/ppc.h  |2 +
 hw/ppc_booke.c|   83 +
 linux-headers/asm-generic/kvm_para.h  |4 +
 linux-headers/asm-powerpc/kvm.h   |   86 ++
 linux-headers/asm-powerpc/kvm_para.h  |7 +-
 linux-headers/linux/kvm.h |   21 -
 linux-headers/uapi/asm-powerpc/epapr_hcalls.h |   98 +
 target-ppc/kvm.c  |   13 +++-
 8 files changed, 305 insertions(+), 9 deletions(-)
 create mode 100644 linux-headers/asm-generic/kvm_para.h
 create mode 100644 linux-headers/uapi/asm-powerpc/epapr_hcalls.h





[Qemu-devel] [PATCH V9 2/4] Create four opts list related functions

2012-12-16 Thread Dong Xu Wang
This patch will create 4 functions, count_opts_list, append_opts_list,
free_opts_list and print_opts_list, they will used in following commits.

v6-v7):
1) Fix typo.

v5-v6):
1) allocate enough space in append_opts_list function.

Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
---
 qemu-option.c |   90 +
 qemu-option.h |4 ++
 2 files changed, 94 insertions(+), 0 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 139ecd6..4a577e7 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -1149,3 +1149,93 @@ int qemu_opts_foreach(QemuOptsList *list, 
qemu_opts_loopfunc func, void *opaque,
 loc_pop(loc);
 return rc;
 }
+
+static size_t count_opts_list(QemuOptsList *list)
+{
+size_t i = 0;
+
+while (list  list-desc[i].name) {
+i++;
+}
+
+return i;
+}
+
+/* Create a new QemuOptsList and make its desc to the merge of first and 
second.
+ * It will allocate space for one new QemuOptsList plus enough space for
+ * QemuOptDesc in first and second QemuOptsList. First argument's QemuOptDesc
+ * members take precedence over second's.
+ */
+QemuOptsList *append_opts_list(QemuOptsList *first,
+   QemuOptsList *second)
+{
+size_t num_first_options, num_second_options;
+QemuOptsList *dest = NULL;
+int i = 0;
+int index = 0;
+
+num_first_options = count_opts_list(first);
+num_second_options = count_opts_list(second);
+if (num_first_options + num_second_options == 0) {
+return NULL;
+}
+
+dest = g_malloc0(sizeof(QemuOptsList)
++ (num_first_options + num_second_options + 1) * sizeof(QemuOptDesc));
+
+dest-name = append_opts_list;
+dest-implied_opt_name = NULL;
+dest-merge_lists = false;
+QTAILQ_INIT(dest-head);
+while (first  (first-desc[i].name)) {
+if (!find_desc_by_name(dest-desc, first-desc[i].name)) {
+dest-desc[index].name = g_strdup(first-desc[i].name);
+dest-desc[index].help = g_strdup(first-desc[i].help);
+dest-desc[index].type = first-desc[i].type;
+dest-desc[index].def_value_str =
+g_strdup(first-desc[i].def_value_str);
+++index;
+   }
+i++;
+}
+i = 0;
+while (second  (second-desc[i].name)) {
+if (!find_desc_by_name(dest-desc, second-desc[i].name)) {
+dest-desc[index].name = g_strdup(first-desc[i].name);
+dest-desc[index].help = g_strdup(first-desc[i].help);
+dest-desc[index].type = second-desc[i].type;
+dest-desc[index].def_value_str =
+g_strdup(second-desc[i].def_value_str);
+++index;
+}
+i++;
+}
+dest-desc[index].name = NULL;
+return dest;
+}
+
+void free_opts_list(QemuOptsList *list)
+{
+int i = 0;
+
+while (list  list-desc[i].name) {
+g_free((char *)list-desc[i].name);
+g_free((char *)list-desc[i].help);
+g_free((char *)list-desc[i].def_value_str);
+i++;
+}
+
+g_free(list);
+}
+
+void print_opts_list(QemuOptsList *list)
+{
+int i = 0;
+printf(Supported options:\n);
+while (list  list-desc[i].name) {
+printf(%-16s %s\n, list-desc[i].name,
+list-desc[i].help ?
+list-desc[i].help : No description available);
+i++;
+}
+}
diff --git a/qemu-option.h b/qemu-option.h
index 2f8b9f0..76f2b4d 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -156,4 +156,8 @@ int qemu_opts_print(QemuOpts *opts, void *dummy);
 int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void 
*opaque,
   int abort_on_failure);
 
+QemuOptsList *append_opts_list(QemuOptsList *dest,
+   QemuOptsList *list);
+void free_opts_list(QemuOptsList *list);
+void print_opts_list(QemuOptsList *list);
 #endif
-- 
1.7.1




[Qemu-devel] [PATCH V9 0/4] replace QEMUOptionParameter with QemuOpts parser

2012-12-16 Thread Dong Xu Wang
Patch 1-3 are from Luiz, added Markus's comments, discussion could be found
here:
http://lists.nongnu.org/archive/html/qemu-devel/2012-07/msg02716.html
Patch 3 was changed according Paolo's comments.

Patch 4-5: because qemu_opts_create can not fail while id is null, so create
function qemu_opts_create_nofail and use it.

Patch 6: create function qemu_opt_set_number, like qemu_opt_set_bool.

Patch 7: add def_value and use it in qemu_opts_print.

Patch 8: Create functions to pair with QEMUOptionParameter parser.

Patch 9: Use QemuOpts parser in Block.

Patch 10: Remove QEMUOptionParameter parser related code.

Patches 1 - 6 have been merged into block branch, so only patches 8 to 10
are included.

v8-v9)
1) add qemu_ prefix to gluster_create_opts.
2) fix bug: bdrv_gluster_unix and bdrv_gluster_rdma should also be
   converted.

v7-v8)
1) print elements = accept any params while opts_accepts_any() ==
true.
2) since def_print_str is the default value if an option isn't set,
so rename it to def_value_str.
3) rebase to upstream source tree.
4) add gluster.c, raw-win32.c, and rbd.c.

v6-v7:
1) Fix typo: enouth-enough.
2) use osdep.h:stringify(), not redefining new macro.
3) preserve TODO comment.
4) fix typo: BLOCK_OPT_ENCRYPT-BLOCK_OPT_STATIC.
5) initialize disk_type even when opts is NULL.

v5-v6:
1) allocate enough space in append_opts_list function.
2) judge if opts == NULL in block layer create functions.
3) use bdrv_create_file(filename, NULL) in qcow_create funtion.
4) made more readable while using qemu_opt_get_number funtion.

v4-v5:
1) Rewrite qemu_opts_create_nofail function based on Peter Maydell's comments.
2) Use g_strdup_printf in qemu_opt_set_number.
3) Rewrite qemu_opts_print.
4) .bdrv_create_options returns pointer directly. Fix a bug about encryption.
5) Check qemu_opt_get_number in raw-posix.c.

v3-v4:
1) Rebased to the newest source tree.
2) Remove redundant #include block-cache.h
3) Other small changes.

v2-v3:
1) rewrite qemu_opt_set_bool and qemu_opt_set_number according Paolo's coments.
2) split patches to make review easier.

v1-v2:
1) add Luiz's patches.
2) create qemu_opt_set_number() and qemu_opts_create_nofail() functions.
3) add QemuOptsList map to drivers.
4) use original opts parser, not creating new ones.
5) fix other bugs.

Dong Xu Wang (4):
  add def_print_str and use it in qemu_opts_print.
  Create four opts list related functions
  Use QemuOpts support in block layer
  remove QEMUOptionParameter related functions and struct

 block.c   |   91 ++---
 block.h   |4 +-
 block/cow.c   |   46 +++---
 block/gluster.c   |   37 +++---
 block/qcow.c  |   60 
 block/qcow2.c |  171 ---
 block/qed.c   |   86 ++--
 block/raw-posix.c |   65 -
 block/raw-win32.c |   30 ++--
 block/raw.c   |   30 +++--
 block/rbd.c   |   63 
 block/sheepdog.c  |   75 +-
 block/vdi.c   |   69 +-
 block/vmdk.c  |   74 +-
 block/vpc.c   |   67 +
 block/vvfat.c |   11 +-
 block_int.h   |6 +-
 qemu-img.c|   61 
 qemu-option.c |  406 +++--
 qemu-option.h |   37 +
 20 files changed, 641 insertions(+), 848 deletions(-)




[Qemu-devel] [PATCH V9 4/4] remove QEMUOptionParameter related functions and struct

2012-12-16 Thread Dong Xu Wang
Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
---
 qemu-option.c |  285 -
 qemu-option.h |   32 ---
 2 files changed, 0 insertions(+), 317 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 4a577e7..75b93ff 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -153,22 +153,6 @@ int check_params(char *buf, int buf_size,
 return 0;
 }
 
-/*
- * Searches an option list for an option with the given name
- */
-QEMUOptionParameter *get_option_parameter(QEMUOptionParameter *list,
-const char *name)
-{
-while (list  list-name) {
-if (!strcmp(list-name, name)) {
-return list;
-}
-list++;
-}
-
-return NULL;
-}
-
 static void parse_option_bool(const char *name, const char *value, bool *ret,
   Error **errp)
 {
@@ -240,275 +224,6 @@ static void parse_option_size(const char *name, const 
char *value,
 }
 }
 
-/*
- * Sets the value of a parameter in a given option list. The parsing of the
- * value depends on the type of option:
- *
- * OPT_FLAG (uses value.n):
- *  If no value is given, the flag is set to 1.
- *  Otherwise the value must be on (set to 1) or off (set to 0)
- *
- * OPT_STRING (uses value.s):
- *  value is strdup()ed and assigned as option value
- *
- * OPT_SIZE (uses value.n):
- *  The value is converted to an integer. Suffixes for kilobytes etc. are
- *  allowed (powers of 1024).
- *
- * Returns 0 on succes, -1 in error cases
- */
-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);
-if (list == NULL) {
-fprintf(stderr, Unknown option '%s'\n, name);
-return -1;
-}
-
-// Process parameter
-switch (list-type) {
-case OPT_FLAG:
-parse_option_bool(name, value, flag, local_err);
-if (!error_is_set(local_err)) {
-list-value.n = flag;
-}
-break;
-
-case OPT_STRING:
-if (value != NULL) {
-list-value.s = g_strdup(value);
-} else {
-fprintf(stderr, Option '%s' needs a parameter\n, name);
-return -1;
-}
-break;
-
-case OPT_SIZE:
-parse_option_size(name, value, list-value.n, local_err);
-break;
-
-default:
-fprintf(stderr, Bug: Option '%s' has an unknown type\n, name);
-return -1;
-}
-
-if (error_is_set(local_err)) {
-qerror_report_err(local_err);
-error_free(local_err);
-return -1;
-}
-
-return 0;
-}
-
-/*
- * Sets the given parameter to an integer instead of a string.
- * This function cannot be used to set string options.
- *
- * Returns 0 on success, -1 in error cases
- */
-int set_option_parameter_int(QEMUOptionParameter *list, const char *name,
-uint64_t value)
-{
-// Find a matching parameter
-list = get_option_parameter(list, name);
-if (list == NULL) {
-fprintf(stderr, Unknown option '%s'\n, name);
-return -1;
-}
-
-// Process parameter
-switch (list-type) {
-case OPT_FLAG:
-case OPT_NUMBER:
-case OPT_SIZE:
-list-value.n = value;
-break;
-
-default:
-return -1;
-}
-
-return 0;
-}
-
-/*
- * Frees a option list. If it contains strings, the strings are freed as well.
- */
-void free_option_parameters(QEMUOptionParameter *list)
-{
-QEMUOptionParameter *cur = list;
-
-while (cur  cur-name) {
-if (cur-type == OPT_STRING) {
-g_free(cur-value.s);
-}
-cur++;
-}
-
-g_free(list);
-}
-
-/*
- * Count valid options in list
- */
-static size_t count_option_parameters(QEMUOptionParameter *list)
-{
-size_t num_options = 0;
-
-while (list  list-name) {
-num_options++;
-list++;
-}
-
-return num_options;
-}
-
-/*
- * Append an option list (list) to an option list (dest).
- *
- * If dest is NULL, a new copy of list is created.
- *
- * Returns a pointer to the first element of dest (or the newly allocated copy)
- */
-QEMUOptionParameter *append_option_parameters(QEMUOptionParameter *dest,
-QEMUOptionParameter *list)
-{
-size_t num_options, num_dest_options;
-
-num_options = count_option_parameters(dest);
-num_dest_options = num_options;
-
-num_options += count_option_parameters(list);
-
-dest = g_realloc(dest, (num_options + 1) * sizeof(QEMUOptionParameter));
-dest[num_dest_options].name = NULL;
-
-while (list  list-name) {
-if (get_option_parameter(dest, list-name) == NULL) {
-dest[num_dest_options++] = *list;
-dest[num_dest_options].name = NULL;
-}
-list++;
-}
-
-return dest;
-}
-
-/*
- * Parses a parameter string (param) into an option list (dest).
- *
- * list is the template option 

[Qemu-devel] [PATCH V9 1/4] add def_print_str and use it in qemu_opts_print.

2012-12-16 Thread Dong Xu Wang
qemu_opts_print has no user now, so can re-write the function safely.

qemu_opts_print will be used while using qemu-img create, it will
produce the same output as previous code.

The behavior of this function has changed:

1. Print every possible option, whether a value has been set or not.
2. Option descriptors may provide a default value.
3. Print to stdout instead of stderr.

Previously the behavior was to print every option that has been set.
Options that have not been set would be skipped.

v7-v8:
1) print elements = accept any params while opts_accepts_any() ==
true.
2) since def_print_str is the default value if an option isn't set,
so rename it to def_value_str.

Signed-off-by: Dong Xu Wang wdon...@linux.vnet.ibm.com
---
 qemu-option.c |   31 ---
 qemu-option.h |1 +
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 94557cf..139ecd6 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -862,15 +862,32 @@ void qemu_opts_del(QemuOpts *opts)
 
 int qemu_opts_print(QemuOpts *opts, void *dummy)
 {
-QemuOpt *opt;
+QemuOptDesc *desc = opts-list-desc;
 
-fprintf(stderr, %s: %s:, opts-list-name,
-opts-id ? opts-id : noid);
-QTAILQ_FOREACH(opt, opts-head, next) {
-fprintf(stderr,  %s=\%s\, opt-name, opt-str);
+if (desc[0].name == NULL) {
+printf(no elements = accept any params);
+return 0;
 }
-fprintf(stderr, \n);
-return 0;
+for (; desc  desc-name; desc++) {
+const char *value = desc-def_value_str;
+QemuOpt *opt;
+
+opt = qemu_opt_find(opts, desc-name);
+if (opt) {
+value = opt-str;
+}
+
+if (!value) {
+continue;
+}
+
+if (desc-type == QEMU_OPT_STRING) {
+printf(%s='%s' , desc-name, value);
+} else {
+printf(%s=%s , desc-name, value);
+}
+}
+ return 0;
 }
 
 static int opts_do_parse(QemuOpts *opts, const char *params,
diff --git a/qemu-option.h b/qemu-option.h
index 002dd07..2f8b9f0 100644
--- a/qemu-option.h
+++ b/qemu-option.h
@@ -96,6 +96,7 @@ typedef struct QemuOptDesc {
 const char *name;
 enum QemuOptType type;
 const char *help;
+const char *def_value_str;
 } QemuOptDesc;
 
 struct QemuOptsList {
-- 
1.7.1




[Qemu-devel] [PATCH 0/6] snapshot: take snapshots in unified way

2012-12-16 Thread Wenchao Xia
  These patch added a seperated layer to take internal or external snapshots
in a unified way, the granularity is block device, so other functions can
just combine the request and submit, such as group snapshot, savevm.

Total goal are:
  Live back up vm in external or internal image, which need three function:
1 live snapshot block device internal/external.
2 live save vmstate internal/external.
3 combination of the function unit.

  This patch basically provide function one in unified style with granularity
of device.

Wenchao Xia (6):
  snapshot: export function in block.c
  snapshot: add error set function
  snapshot: design of common API to take snapshots
  snapshot: implemention of common API to take snapshots
  snapshot: qmp interface
  snapshot: human monitor interface

 block.c  |   30 ++
 block.h  |3 +
 blockdev.c   |  833 +-
 blockdev.h   |  129 +
 error.c  |   23 ++
 error.h  |9 +
 hmp-commands.hx  |   50 +++-
 hmp.c|   30 ++-
 hmp.h|1 +
 monitor.c|   21 +-
 qapi-schema.json |  102 ++-
 savevm.c |   77 --
 sysemu.h |2 +-
 13 files changed, 1117 insertions(+), 193 deletions(-)





[Qemu-devel] [PATCH 6/6] snapshot: human monitor interface

2012-12-16 Thread Wenchao Xia
  This patch add support in human monitor to create/delete/check
internal snapshot on a single blk device.
  To make info command get parameter, added a new info handler
type which can take QDict as parameter.

Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com
---
 hmp-commands.hx |   50 +-
 hmp.c   |   30 +-
 hmp.h   |1 +
 monitor.c   |   21 +++--
 savevm.c|   55 ++-
 sysemu.h|2 +-
 6 files changed, 133 insertions(+), 26 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 010b8c9..ee74723 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -983,17 +983,22 @@ ETEXI
 
 {
 .name   = snapshot_blkdev,
-.args_type  = reuse:-n,device:B,snapshot-file:s?,format:s?,
-.params = [-n] device [new-image-file] [format],
-.help   = initiates a live snapshot\n\t\t\t
-  of device. If a new image file is specified, 
the\n\t\t\t
-  new image file will become the new root image.\n\t\t\t
-  If format is specified, the snapshot file will\n\t\t\t
-  be created in that format. Otherwise the\n\t\t\t
-  snapshot will be internal! (currently 
unsupported).\n\t\t\t
-  The default format is qcow2.  The -n flag requests 
QEMU\n\t\t\t
-  to reuse the image found in new-image-file, instead 
of\n\t\t\t
-  recreating it from scratch.,
+.args_type  = 
internal:-i,reuse:-n,device:B,snapshot-file:s?,format:s?,
+.params = [-i] [-n] device [new-image-file] [format],
+.help   = initiates a live snapshot of device.\n\t\t\t
+The -i flag requests QEMU to create internal 
snapshot\n\t\t\t
+  instead of external one.\n\t\t\t
+The -n flag requests QEMU to use existing 
snapshot\n\t\t\t
+  instead of creating new snapshot, which would fails 
if\n\t\t\t
+  snapshot does not exist ahead.\n\t\t\t
+new-image-file is the snapshot's name, in external 
case\n\t\t\t
+  it is the new image's name which will become the new 
root\n\t\t\t
+  image and must be specified, in internal case it is 
the\n\t\t\t
+  record's name and if not specified QEMU will 
create\n\t\t\t
+  internal snapshot with name generated according to 
time.\n\t\t\t
+format is only valid in external case, which is the 
new\n\t\t\t
+  snapshot image's format. If not sepcified default 
format\n\t\t\t
+  qcow2 will be used.,
 .mhandler.cmd = hmp_snapshot_blkdev,
 },
 
@@ -1004,6 +1009,25 @@ Snapshot device, using snapshot file as target if 
provided
 ETEXI
 
 {
+.name   = snapshot_delete_blkdev,
+.args_type  = internal:-i,device:B,snapshot-file:s,
+.params = [-i] device new-image-file,
+.help   = delete a snapshot  synchronous.\n\t\t\t
+The -i flag requests QEMU to delete internal 
snapshot\n\t\t\t
+  instead of external one.\n\t\t\t
+new-image-file is the snapshot's name, in external 
case\n\t\t\t
+  it is the image's name which is not supported 
now.\n\t\t\t
+  in internal case it is the snapshot record's id or 
name.,
+.mhandler.cmd = hmp_snapshot_delete_blkdev,
+},
+
+STEXI
+@item snapshot_delete_blkdev
+@findex snapshot_delete_blkdev
+Delete a snapshot on a block device.
+ETEXI
+
+{
 .name   = drive_mirror,
 .args_type  = reuse:-n,full:-f,device:B,target:s,format:s?,
 .params = [-n] [-f] device target [format],
@@ -1486,8 +1510,8 @@ ETEXI
 
 {
 .name   = info,
-.args_type  = item:s?,
-.params = [subcommand],
+.args_type  = item:s?,params:s?,
+.params = [subcommand] [params],
 .help   = show various information about the system state,
 .mhandler.cmd = do_info,
 },
diff --git a/hmp.c b/hmp.c
index 180ba2b..f247f51 100644
--- a/hmp.c
+++ b/hmp.c
@@ -806,20 +806,40 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict)
 const char *filename = qdict_get_try_str(qdict, snapshot-file);
 const char *format = qdict_get_try_str(qdict, format);
 int reuse = qdict_get_try_bool(qdict, reuse, 0);
+int internal = qdict_get_try_bool(qdict, internal, 0);
 enum NewImageMode mode;
+enum SnapshotType type;
 Error *errp = NULL;
 
-if (!filename) {
-/* In the future, if 'snapshot-file' is not specified, the snapshot
-   will be taken internally. Today it's 

Re: [Qemu-devel] [PATCH 4/6] snapshot: implemention of common API to take snapshots

2012-12-16 Thread Wenchao Xia

于 2012-12-17 14:36, Dietmar Maurer 写道:

   This patch is the implemention of transaction based snapshot internal API.


What exactly is the advantage of using qmp transactions (as opposed to the pve 
snapshot patches)?

  Which pve snapshot patches you referring? In group case a check should
be done for all snapshot first, and I need to support qmp transaction
interface which is already there, so I build an internal transaction
layer below to make the interface unified and relative easier to add in
qmp transaction.


Seems this makes it impossible to use external commands to make snapshots?


  what command? can you tip more about the case?


--
Best Regards

Wenchao Xia




Re: [Qemu-devel] [PATCH v2 2/2] sheepdog: pass oid directly to send_pending_req()

2012-12-16 Thread MORITA Kazutaka
At Mon, 17 Dec 2012 14:17:27 +0800,
Liu Yuan wrote:
 
 From: Liu Yuan tailai...@taobao.com
 
 Cc: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp
 Cc: Kevin Wolf kw...@redhat.com
 Signed-off-by: Liu Yuan tailai...@taobao.com
 ---
  block/sheepdog.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp



Re: [Qemu-devel] [PATCH v2 1/2] sheepdog: don't update inode when create_and_write fails

2012-12-16 Thread MORITA Kazutaka
At Mon, 17 Dec 2012 14:17:26 +0800,
Liu Yuan wrote:
 
 From: Liu Yuan tailai...@taobao.com
 
 For the error case such as SD_RES_NO_SPACE, we shouldn't update the inode 
 bitmap
 to avoid the scenario that the object is allocated but wasn't created at the
 server side. This will result in VM's IO error on the failed object.
 
 Cc: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp
 Cc: Kevin Wolf kw...@redhat.com
 Signed-off-by: Liu Yuan tailai...@taobao.com
 ---
  block/sheepdog.c |9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

Reviewed-by: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp



Re: [Qemu-devel] [PATCH 4/6] snapshot: implemention of common API to take snapshots

2012-12-16 Thread Dietmar Maurer
 于 2012-12-17 14:36, Dietmar Maurer 写道:
 This patch is the implemention of transaction based snapshot internal
 API.
 
  What exactly is the advantage of using qmp transactions (as opposed to the
 pve snapshot patches)?
Which pve snapshot patches you referring? 

I refer to:  

https://git.proxmox.com/?p=pve-qemu-kvm.git;a=blob;f=debian/patches/internal-snapshot-async.patch;h=6c86de3a6160c58d77baa41a7774c4a80e63639e;hb=HEAD

 In group case a check should be
 done for all snapshot first, and I need to support qmp transaction interface
 which is already there, so I build an internal transaction layer below to make
 the interface unified and relative easier to add in qmp transaction.

IMHO qmp transactions are a bit clumsy and inflexible. And as you can see with 
my patches,
there is no real need for them.

  Seems this makes it impossible to use external commands to make
 snapshots?
 
what command? can you tip more about the case?

For example, nexenta storage provides an API to create snapshots. We want to use
that. Another example would be to use lvcreate to create lvm snapshots.

- Dietmar