Re: [Qemu-devel] [PATCH 09/17] memory: iommu support

[David Gibson]

On Thu, May 02, 2013 at 07:24:54AM +0200, Paolo Bonzini wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
> 
> Il 02/05/2013 05:05, David Gibson ha scritto:
> >>> I think the problem is that we do not have reference counting,
> >>> and this makes it simpler to manage the lifetime.  It can be
> >>> changed later.
> > I don't really follow this logic.  In the existing case, the iommu 
> > target is always system memory, and there's already 
> > address_space_memory which always exists.
> 
> But does the tce->iommu address space always exist?  Can you have
> multiple domains and so on?

I'm not sure exactly what you mean here.  We have multiple
"partitionable endpoints" that are somewhat equivalent to iommu
domains on intel - each corresponds to a different DMA address space.
By convention, though, these are configured by firmware and never
changed during runtime.  For simplicity in the qemu guest case, we
only ever have one partitionable endpoint per (guest) PCI host
bridge.  That's not really a limitation, because pseries can happily
support multiple host bridges (even dozens or hundreds).

> I can surely change it if you prefer, after all you're the only user
> right now (address_space_memory always exists).  But I'm not 100% sure
> it will create more problems than it solves.

Well, we're talking here about the *target* address space of the
iommu.  So that's address_space_memory for us too.  In fact I don't
know of any realistic cases where the target AS won't be
address_space_memory, although it's certainly theoretically possible.

-- 
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


signature.asc
Description: Digital signature

[Qemu-devel] [PULL] Trivial patches for 2013-05-02

[Michael Tokarev]

Hello.

As Stefan Hajnoczi mentioned yesterday, I'll try to maintain qemu-trivial
patch queue.  So here goes the first pull request, with just 3 really
trivial patches.  For now I push the changes into my repository, available
also as 
http://git.corpit.ru/?p=qemu.git;a=shortlog;h=refs/heads/trivial-patches .

The patches included this week are really trivial.  There are a few other
patches pending on the qemu-trivial mailing list, but due to several reasons
I weren't able to do any good testing/review of thes - will do that for the
next pull request.  There are a series of holidays here in Russia, first is
1st-2nd May "International workers day", plus a weekend, and second is the
Victory day (9th may, end of World War II), also with a weekend, and everyone
here goes out of city.  That's why I'm sending pull request at Thursday
instead of usual Friday.

For the other patches already submitted to -trivial, I've the following plans:
 "translate: remove redundantly included qemu/timer.h" by liguang --
   pending Signed-off-by.
 "m25p80.c: Sync Flash chip list with Linux" by Ed Maste --
   I want to actually take a look and compare the resulting thing with the
   list in linux kernel.  Will hopefully do for the next pull request.
 "bsd-user: OS-agnostic 64-bit SYSCTL types" by Ed Maste --
   also I want to verify that it works, for that I'll have to install some
   FreeBSD guest or maybe find the changes in their #include's to see what's
   it all about.
 "SMBUS module update" by Maksim_Ratnikov --
   I don't think this qualifies for -trivial, and the submission is done in
   html form.  Pending resubmission in correct form to qemu-devel and the
   appropriate maintainer.

So even if I included all changes so far, the pull requests weren't much
larger anyway :)

Also, I'm still trying to figure out how qemu-trivial is, and should be,
maintained, and doing stuff without any automation, so things may be a
bit rough for the first time.  This is, ofcourse, fixable later ;)

As mentioned above, due to holiday I won't be available until Monday.

Below is the actual pull request, with whole diff appended (since it is
rather small), for easier review.  Please pull :)

/mjt

The following changes since commit e9016ee2bda1b7757072b856b2196f691aee3388:

  virtio-net: count VIRTIO_NET_F_MAC when calculating config_len (2013-04-30 
16:04:24 -0500)

are available in the git repository at:

  git://git.corpit.ru/qemu.git trivial-patches

for you to fetch changes up to 6e860b5db4c76c66d7e02f93c9e22e0384bd3c6c:

  pvscsi: fix compilation on 32 bit hosts (2013-05-01 21:00:20 +0400)


Andreas Färber (1):
  configure: Pick up libseccomp include path

Hervé Poussineau (1):
  pvscsi: fix compilation on 32 bit hosts

Stefan Weil (1):
  Trivial grammar and spelling fixes

 configure|1 +
 hw/moxie/moxiesim.c  |2 +-
 include/hw/stream.h  |6 +++---
 include/sysemu/rng.h |2 +-
 qmp-commands.hx  |2 +-
 target-s390x/translate.c |2 +-
 trace-events |2 +-
 7 files changed, 9 insertions(+), 8 deletions(-)
mjt@gandalf:/build/kvm/git$ git request-pull --help
mjt@gandalf:/build/kvm/git$ git request-pull -p qemu/master 
git://git.corpit.ru/qemu.git
The following changes since commit e9016ee2bda1b7757072b856b2196f691aee3388:

  virtio-net: count VIRTIO_NET_F_MAC when calculating config_len (2013-04-30 
16:04:24 -0500)

are available in the git repository at:

  git://git.corpit.ru/qemu.git trivial-patches

for you to fetch changes up to 6e860b5db4c76c66d7e02f93c9e22e0384bd3c6c:

  pvscsi: fix compilation on 32 bit hosts (2013-05-01 21:00:20 +0400)


Andreas Färber (1):
  configure: Pick up libseccomp include path

Hervé Poussineau (1):
  pvscsi: fix compilation on 32 bit hosts

Stefan Weil (1):
  Trivial grammar and spelling fixes

 configure|1 +
 hw/moxie/moxiesim.c  |2 +-
 include/hw/stream.h  |6 +++---
 include/sysemu/rng.h |2 +-
 qmp-commands.hx  |2 +-
 target-s390x/translate.c |2 +-
 trace-events |2 +-
 7 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/configure b/configure
index d91b141..c4d85ba 100755
--- a/configure
+++ b/configure
@@ -1499,6 +1499,7 @@ libs_softmmu="$libs_softmmu -lz"
 if test "$seccomp" != "no" ; then
 if $pkg_config --atleast-version=1.0.0 libseccomp --modversion >/dev/null 
2>&1; then
 libs_softmmu="$libs_softmmu `$pkg_config --libs libseccomp`"
+QEMU_CFLAGS="$QEMU_CFLAGS `$pkg_config --cflags libseccomp`"
seccomp="yes"
 else
if test "$seccomp" = "yes"; then
diff --git a/hw/moxie/moxiesim.c b/hw/moxie/moxiesim.c
index 70bf28f..649f9a7 100644
--- a/hw/moxie/moxiesim.c
+++ b/hw/moxie/moxiesim.c
@@ -1,7 +1,7 @@
 /*
  * QEMU/moxiesim emulation
  *
- * Emulates a very simpl

Re: [Qemu-devel] [PATCH V18 1/6] docs: document for add-cow file format

[Dong Xu Wang]

On 2013/4/27 6:45, Eric Blake wrote:

On 04/10/2013 02:11 AM, Dong Xu Wang wrote:

Document for add-cow format, the usage and spec of add-cow are
introduced.

Signed-off-by: Dong Xu Wang 
---
V17->V18:
1) remove version field.
2) header size is maximum value and cluster size value.
3) fix type.
  docs/specs/add-cow.txt | 165 +
  1 file changed, 165 insertions(+)
  create mode 100644 docs/specs/add-cow.txt

diff --git a/docs/specs/add-cow.txt b/docs/specs/add-cow.txt
new file mode 100644
index 000..151028b
--- /dev/null
+++ b/docs/specs/add-cow.txt
@@ -0,0 +1,165 @@
+== General ==


No copyright notice?  Not necessarily your fault, since many other files
in this directory suffer from the same problem.


Yep, all documents in docs/specs have no copyright, so I omit it.

+
+The raw file format does not support backing files or copy on write
+feature. The add-cow image format makes it possible to use backing
+files with a image by keeping a separate .add-cow metadata file.
+Once all clusters have been written into the image it is safe to
+discard the .add-cow and backing files, then we can use the image
+directly.
+
+An example usage of add-cow would look like:
+(ubuntu.img is a disk image which has an installed OS.)
+1)  Create a image, such as raw format, with the same size of
+ubuntu.img:
+qemu-img create -f raw test.raw 8G
+2)  Create an add-cow image which will store dirty bitmap
+qemu-img create -f add-cow test.add-cow \
+-o backing_file=ubuntu.img,image_file=test.raw
+3)  Run qemu with add-cow image
+qemu -drive if=virtio,file=test.add-cow
+
+test.raw may be larger than ubuntu.img, in that case, the size of
+test.add-cow will be calculated from the size of test.raw.
+
+image_fmt can be omitted, in that case image_fmt is assumed to be
+"raw". backing_fmt can also be omitted, add-cow should do a probe
+operation and determine what the backing file's format is.


In general, probing a raw file is a security hole (we just plugged a CVE
with NBD probing); you probably ought to mention that it is recommended
to always specify the format for any raw file, so that probing doesn't
misinterpret the contents of the file as some other format.


Okay, will mention.

+
+=Specification=
+
+The file format looks like this:
+
+ +---+---+
+ | Header|   COW bitmap  |
+ +---+---+
+
+All numbers in add-cow are stored in Little Endian byte order.
+
+== Header ==
+
+The Header is included in the first bytes:
+(HEADER_SIZE is defined in 40-43 bytes.)
+Byte0  -  3:magic
+add-cow magic string ("ACOW").


Probably ought to mention that this magic string is in the ASCII
encoding (those characters map to different bytes on EBCDIC, although I
doubt qemu will ever really been ported to EBCDIC)


Okay.

+
+4  -  7:backing file name offset
+Offset in the add-cow file at which the backing
+file name is stored (NB: The string is not
+lNUL-terminated).


s/lNUL/NUL/


Okay.

+If backing file name does NOT exist, this field
+will be 0. Must be between 76 and [HEADER_SIZE
+- 2](a file name must be at least 1 byte).
+



+
+40 - 43:HEADER_SIZE
+The header field is variable-sized. This field
+indicates how many bytes will be used to store
+add-cow header. By default, it is maximum value
+of 4096 and cluster size value.


Should it be required to be a multiple of 4096, for efficient alignment
of clusters?


I think I can make cluster size be multiple of 4096..

+
+44 - 59:backing file format
+Format of backing file. It will be filled with
+0 if backing file name offset is 0. If backing
+file name offset is non-empty, it must be
+non-empty. It is coded in free-form ASCII, and
+is not NUL-terminated. Zero padded on the right.


Requiring this to be non-empty if a backing file is named contradicts
your earlier statement that backing format is probed (I actually like
mandating the backing format, though).


I think logic can be:
1) if "-o backing_fmt = raw" then do not probe.
2) else even if "-o backing_fmt = $fmt" is used, also perform a probe 
operation and write backing_fmt in add_cow headers.



+
+60 - 75:image file format
+Format of image file. It must be non-empty. It
+is coded in free-form ASCII, and is not
+NUL-terminated. Zero padded on the right.


Again, requiring a format contradicts the 

Re: [Qemu-devel] [PATCH 09/17] memory: iommu support

[Paolo Bonzini]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Il 02/05/2013 05:05, David Gibson ha scritto:
>>> I think the problem is that we do not have reference counting,
>>> and this makes it simpler to manage the lifetime.  It can be
>>> changed later.
> I don't really follow this logic.  In the existing case, the iommu 
> target is always system memory, and there's already 
> address_space_memory which always exists.

But does the tce->iommu address space always exist?  Can you have
multiple domains and so on?

I can surely change it if you prefer, after all you're the only user
right now (address_space_memory always exists).  But I'm not 100% sure
it will create more problems than it solves.

Paolo
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRgfimAAoJEBvWZb6bTYby4LgP/0B+I7Hql/TYBputor6VwH8S
xC2EK8uMpmmTr5RknsaQRQQTPmaXzb1YEXNRN7GKbsEoP5ibSfoFFhMTkUurILWS
KdEk+qE3bKnkVqKbyhS3t+zhae7x4fEVO2Cvob3DS/eiaSPuFex3BmHInGyoH7IK
ZrTmJwfhWZV0xOKQ96Wyz0tGDTSEhbgn4kIaZANQXbf6Em1dmKsqAyeegotu+oje
R6rwJ4lDKlKNAhFOs0lJYHOIsh+EH09QNZiAVWbp8Bx5B0m02bpFBwFZUJWxt3gh
1XspLS+TYHqOqiq//WVIGEJ6U6hepyoU9vFUVfeaW02PFU4+D9VuC6Yd5r4sRYzJ
BmwebxeNSrNnCsrDz/R/w07+VDgCgNmhSLDfOx3JEpt/uuw3oxKODpb5+bMMZodO
rz8Qv6RilozMK9b9j9O4+n6KFRLMoaqmyS4qWRiSmeF3myD0m8NaKynsutB8efQ9
Zm0/vinWhyFrl3zR/daRY3RY25IEtV2EtGcVCGVrtIhvgaI02enSMoo41wO2bdWQ
W+JmX1Y6+2NaCzhx6K7Fc2CpAIynGd3jC0d/XG4OL7Ik6SF0eA/3fRDTF/2/d1HZ
7EeaiG7OF6YznA68qAGcizyYCUNY8uRjv8rSFL842xGetvHQW5bZ4LNGJbLfPVpn
qzGnzdS7tqVa1w48qrNv
=RGlp
-END PGP SIGNATURE-

Re: [Qemu-devel] [PATCH 1/1 V4] virtio-net: dynamic network offloads configuration

[Michael S. Tsirkin]

On Mon, Apr 22, 2013 at 12:58:26PM -0500, Anthony Liguori wrote:
> "Michael S. Tsirkin"  writes:
> 
> > On Sun, Apr 07, 2013 at 09:34:08AM +0300, Dmitry Fleytman wrote:
> >> From: Dmitry Fleytman 
> >> 
> >> Virtio-net driver currently negotiates network offloads
> >> on startup via features mechanism and have no ability to
> >> disable and re-enable offloads later.
> >> This patch introduced a new control command that allows
> >> to configure device network offloads state dynamically.
> >> The patch also introduces a new feature flag
> >> VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.
> >> 
> >> Signed-off-by: Dmitry Fleytman 
> >
> > Acked-by: Michael S. Tsirkin 
> 
> I was looking for a Reviewed-by, not just an Acked-by.
> 
> Regards,
> 
> Anthony Liguori

Ping. We can't pass WHQL without this support -
could this be merged for 1.5 pretty please?

> >
> > Pls pick this up for 1.5.
> >
> >> ---
> >>  hw/pc.h |  4 +++
> >>  hw/virtio-net.c | 97 
> >> -
> >>  hw/virtio-net.h | 13 
> >>  3 files changed, 99 insertions(+), 15 deletions(-)
> >> 
> >> diff --git a/hw/pc.h b/hw/pc.h
> >> index 8e1dd4c..7ca4698 100644
> >> --- a/hw/pc.h
> >> +++ b/hw/pc.h
> >> @@ -221,6 +221,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
> >>  .property = "vectors",\
> >>  /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\
> >>  .value= stringify(0x),\
> >> +},{ \
> >> +.driver   = "virtio-net-pci", \
> >> +.property = "ctrl_guest_offloads", \
> >> +.value= "off", \
> >>  },{\
> >>  .driver   = "e1000",\
> >>  .property = "romfile",\
> >> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> >> index 5917740..a92d061 100644
> >> --- a/hw/virtio-net.c
> >> +++ b/hw/virtio-net.c
> >> @@ -360,6 +360,33 @@ static uint32_t virtio_net_bad_features(VirtIODevice 
> >> *vdev)
> >>  return features;
> >>  }
> >>  
> >> +static void virtio_net_apply_guest_offloads(VirtIONet *n)
> >> +{
> >> +tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
> >> +!!(n->curr_guest_offloads & (1ULL << 
> >> VIRTIO_NET_F_GUEST_CSUM)),
> >> +!!(n->curr_guest_offloads & (1ULL << 
> >> VIRTIO_NET_F_GUEST_TSO4)),
> >> +!!(n->curr_guest_offloads & (1ULL << 
> >> VIRTIO_NET_F_GUEST_TSO6)),
> >> +!!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_ECN)),
> >> +!!(n->curr_guest_offloads & (1ULL << 
> >> VIRTIO_NET_F_GUEST_UFO)));
> >> +}
> >> +
> >> +static uint64_t virtio_net_guest_offloads_by_features(uint32_t features)
> >> +{
> >> +static const uint64_t guest_offloads_mask =
> >> +(1ULL << VIRTIO_NET_F_GUEST_CSUM) |
> >> +(1ULL << VIRTIO_NET_F_GUEST_TSO4) |
> >> +(1ULL << VIRTIO_NET_F_GUEST_TSO6) |
> >> +(1ULL << VIRTIO_NET_F_GUEST_ECN)  |
> >> +(1ULL << VIRTIO_NET_F_GUEST_UFO);
> >> +
> >> +return guest_offloads_mask & features;
> >> +}
> >> +
> >> +static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n)
> >> +{
> >> +return virtio_net_guest_offloads_by_features(n->vdev.guest_features);
> >> +}
> >> +
> >>  static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features)
> >>  {
> >>  VirtIONet *n = to_virtio_net(vdev);
> >> @@ -371,12 +398,9 @@ static void virtio_net_set_features(VirtIODevice 
> >> *vdev, uint32_t features)
> >>  virtio_net_set_mrg_rx_bufs(n, !!(features & (1 << 
> >> VIRTIO_NET_F_MRG_RXBUF)));
> >>  
> >>  if (n->has_vnet_hdr) {
> >> -tap_set_offload(qemu_get_subqueue(n->nic, 0)->peer,
> >> -(features >> VIRTIO_NET_F_GUEST_CSUM) & 1,
> >> -(features >> VIRTIO_NET_F_GUEST_TSO4) & 1,
> >> -(features >> VIRTIO_NET_F_GUEST_TSO6) & 1,
> >> -(features >> VIRTIO_NET_F_GUEST_ECN)  & 1,
> >> -(features >> VIRTIO_NET_F_GUEST_UFO)  & 1);
> >> +n->curr_guest_offloads =
> >> +virtio_net_guest_offloads_by_features(features);
> >> +virtio_net_apply_guest_offloads(n);
> >>  }
> >>  
> >>  for (i = 0;  i < n->max_queues; i++) {
> >> @@ -422,6 +446,42 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, 
> >> uint8_t cmd,
> >>  return VIRTIO_NET_OK;
> >>  }
> >>  
> >> +static int virtio_net_handle_offloads(VirtIONet *n, uint8_t cmd,
> >> + struct iovec *iov, unsigned int 
> >> iov_cnt)
> >> +{
> >> +uint64_t offloads;
> >> +size_t s;
> >> +
> >> +if (!((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & 
> >> n->vdev.guest_features)) {
> >> +return VIRTIO_NET_ERR;
> >> +}
> >> +
> >> +s = iov_to_buf(iov, iov_cnt, 0, &offloads, sizeof(offloads));
> >> +if (s != sizeof(offloads)) {
> >> +return VIRTIO_NET_ERR;
> >> +}
> >> +
> >> +if (cmd == VIRTIO_NET_CTRL_GUEST_OFFLOADS_

[Qemu-devel] Debug init on Qemu using gdb!

[Muhammad Nouman]

Hi ! i want to debug init code on Qemu using gdb. I want to see that which
instructions are running in userpace while the init is executing its
code.Currently the gdb is showing only the instructions running in kernel
space .
Can anyone please suggest what should i do?


Thanks


Nouman

Re: [Qemu-devel] [PATCH 09/17] memory: iommu support

[David Gibson]

On Wed, May 01, 2013 at 06:10:47PM +0200, Paolo Bonzini wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
> 
> Il 01/05/2013 06:35, David Gibson ha scritto:
> >> From: Avi Kivity 
> >> 
> >> Add a new memory region type that translates addresses it is
> >> given, then forwards them to a target address space.  This is
> >> similar to an alias, except that the mapping is more flexible
> >> than a linear translation and trucation, and also less efficient
> >> since the translation happens at runtime.
> >> 
> >> The implementation uses an AddressSpace mapping the target region
> >> to avoid hierarchical dispatch all the way to the resolved
> >> region; only iommu regions are looked up dynamically.
> >> 
> >> Signed-off-by: Avi Kivity  [Modified to put
> >> translation in address_space_translate - Paolo] Signed-off-by:
> >> Paolo Bonzini  --- exec.c|
> >> 35 +-- include/exec/memory.h |
> >> 44  memory.c
> >> |   28  3 files changed, 101
> >> insertions(+), 6 deletions(-)
> > 
> > [snip]
> >> +void memory_region_init_iommu(MemoryRegion *mr, +
> >> MemoryRegionIOMMUOps *ops, +
> >> MemoryRegion *target, +  const char
> >> *name, +  uint64_t size) +{ +
> >> memory_region_init(mr, name, size); +mr->ops = NULL; +
> >> mr->iommu_ops = ops, +mr->opaque = mr; +mr->terminates =
> >> true;  /* then re-forwards */ +mr->destructor =
> >> memory_region_destructor_iommu; +mr->iommu_target_as =
> >> g_new(AddressSpace, 1); +
> >> address_space_init(mr->iommu_target_as, target);
> > 
> > Since IOMMUs are very likely to share a target AS (in fact, it
> > will nearly always be system memory), it seems odd to me to
> > construct new AddressSpace objects for each one, rather than just
> > giving the AddressSpace as the parameter to
> > memory_region_init_iommu.
> > 
> 
> I think the problem is that we do not have reference counting, and
> this makes it simpler to manage the lifetime.  It can be changed later.

I don't really follow this logic.  In the existing case, the iommu
target is always system memory, and there's already
address_space_memory which always exists.

-- 
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


signature.asc
Description: Digital signature

Re: [Qemu-devel] [PATCH 16/17] spapr_vio: take care of creating our own AddressSpace/DMAContext

[David Gibson]

On Wed, May 01, 2013 at 06:09:21PM +0200, Paolo Bonzini wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
> 
> Il 01/05/2013 07:16, David Gibson ha scritto:
> > Lack of atomicity makes me a little nervous there, although I
> > guess its ok since qemu is single-threaded.
> 
> Yes.  The original plan was to add a boolean return value to
> address_space_rw, but I left this for later since I wasn't sure of the
> semantics for multipage writes.  What happens if the second half of
> the destination buffer has an invalid translation?  Right now it's
> atomic, but it sounds weird for real hardware.

So, in this regard I don't think real hardware would be atomic.  It
would write a certain amount, then generate some sort of bus error
when it hits the bad translation.  So in general I expect the (guest)
OS would need to treat the target of in-flight device to host DMAs as
having undefined contents if there's a bus error like that.  It would
depend on bus and possibly individual device conventions what it could
assume about which DMAs are interrupted, and which might still be
in-flight.

That is, in this sense, we don't expect the hardware to behave
atomically at all.  The atomicity I was concerned about was atomicity
of checking permissions and returning an error based on that check.

So, case, I think an error return value from address_space_rw() is
appropriate.  Semantics would be that if an error is returned you
can't tell if the operation has not started, completed, or somewhere
in the middle.

-- 
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


signature.asc
Description: Digital signature

[Qemu-devel] [PATCH V4 5/5] block: make all steps in qmp_transaction() as callback

[Wenchao Xia]

Make it easier to add other operations to qmp_transaction() by using
callbacks, with external snapshots serving as an example implementation
of the callbacks.

Signed-off-by: Wenchao Xia 
---
 blockdev.c |   92 ++-
 1 files changed, 71 insertions(+), 21 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 77adec8..87ed99e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -779,14 +779,43 @@ void qmp_blockdev_snapshot_sync(const char *device, const 
char *snapshot_file,
 
 
 /* New and old BlockDriverState structs for group snapshots */
-typedef struct BlkTransactionStates {
+
+typedef struct BlkTransactionStates BlkTransactionStates;
+
+/* Only prepare() may fail. In a single transaction, only one of commit() or
+   rollback() will be called, clean() will always be called if it present. */
+typedef struct BdrvActionOps {
+/* Size of state struct, in bytes. */
+size_t instance_size;
+/* Prepare the work, must NOT be NULL. */
+void (*prepare)(BlkTransactionStates *common, Error **errp);
+/* Commit the changes, must NOT be NULL. */
+void (*commit)(BlkTransactionStates *common);
+/* Rollback the changes on fail, can be NULL. */
+void (*rollback)(BlkTransactionStates *common);
+/* Clean up resource in the end, can be NULL. */
+void (*clean)(BlkTransactionStates *common);
+} BdrvActionOps;
+
+/*
+ * This structure must be arranged as first member in parent type, assuming
+ * that compiler will also arrange it to the same address with parent instance.
+ * Later it will be used in free().
+ */
+struct BlkTransactionStates {
+BlockdevAction *action;
+const BdrvActionOps *ops;
+QSIMPLEQ_ENTRY(BlkTransactionStates) entry;
+};
+
+/* external snapshot private data */
+typedef struct ExternalSnapshotStates {
+BlkTransactionStates common;
 BlockDriverState *old_bs;
 BlockDriverState *new_bs;
-QSIMPLEQ_ENTRY(BlkTransactionStates) entry;
-} BlkTransactionStates;
+} ExternalSnapshotStates;
 
-static void external_snapshot_prepare(BlockdevAction *action,
-  BlkTransactionStates *states,
+static void external_snapshot_prepare(BlkTransactionStates *common,
   Error **errp)
 {
 BlockDriver *proto_drv;
@@ -797,6 +826,9 @@ static void external_snapshot_prepare(BlockdevAction 
*action,
 const char *new_image_file;
 const char *format = "qcow2";
 enum NewImageMode mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
+ExternalSnapshotStates *states =
+ DO_UPCAST(ExternalSnapshotStates, common, common);
+BlockdevAction *action = common->action;
 
 /* get parameters */
 g_assert(action->kind == BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC);
@@ -871,8 +903,11 @@ static void external_snapshot_prepare(BlockdevAction 
*action,
 }
 }
 
-static void external_snapshot_commit(BlkTransactionStates *states)
+static void external_snapshot_commit(BlkTransactionStates *common)
 {
+ExternalSnapshotStates *states =
+ DO_UPCAST(ExternalSnapshotStates, common, common);
+
 /* This removes our old bs from the bdrv_states, and adds the new bs */
 bdrv_append(states->new_bs, states->old_bs);
 /* We don't need (or want) to use the transactional
@@ -882,13 +917,24 @@ static void external_snapshot_commit(BlkTransactionStates 
*states)
 NULL);
 }
 
-static void external_snapshot_rollback(BlkTransactionStates *states)
+static void external_snapshot_rollback(BlkTransactionStates *common)
 {
+ExternalSnapshotStates *states =
+ DO_UPCAST(ExternalSnapshotStates, common, common);
 if (states->new_bs) {
 bdrv_delete(states->new_bs);
 }
 }
 
+static const BdrvActionOps actions[] = {
+[BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC] = {
+.instance_size = sizeof(ExternalSnapshotStates),
+.prepare  = external_snapshot_prepare,
+.commit   = external_snapshot_commit,
+.rollback = external_snapshot_rollback,
+},
+};
+
 /*
  * 'Atomic' group snapshots.  The snapshots are taken as a set, and if any fail
  *  then we do not pivot any of the devices in the group, and abandon the
@@ -909,32 +955,31 @@ void qmp_transaction(BlockdevActionList *dev_list, Error 
**errp)
 /* We don't do anything in this loop that commits us to the snapshot */
 while (NULL != dev_entry) {
 BlockdevAction *dev_info = NULL;
+const BdrvActionOps *ops;
 
 dev_info = dev_entry->value;
 dev_entry = dev_entry->next;
 
-states = g_malloc0(sizeof(BlkTransactionStates));
+assert(dev_info->kind < ARRAY_SIZE(actions));
+
+ops = &actions[dev_info->kind];
+states = g_malloc0(ops->instance_size);
+states->ops = ops;
+states->action = dev_info;
 QSIMPLEQ_INSERT_TAIL(&snap_bdrv_states, states, entry);
 
-switch (dev_info->kind) {
- 

[Qemu-devel] [PATCH V4 4/5] block: package rollback code in qmp_transaction()

[Wenchao Xia]

Signed-off-by: Wenchao Xia 
---
 blockdev.c |   11 ---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 26bc78e..77adec8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -882,6 +882,13 @@ static void external_snapshot_commit(BlkTransactionStates 
*states)
 NULL);
 }
 
+static void external_snapshot_rollback(BlkTransactionStates *states)
+{
+if (states->new_bs) {
+bdrv_delete(states->new_bs);
+}
+}
+
 /*
  * 'Atomic' group snapshots.  The snapshots are taken as a set, and if any fail
  *  then we do not pivot any of the devices in the group, and abandon the
@@ -939,9 +946,7 @@ delete_and_fail:
 * the original bs for all images
 */
 QSIMPLEQ_FOREACH(states, &snap_bdrv_states, entry) {
-if (states->new_bs) {
- bdrv_delete(states->new_bs);
-}
+external_snapshot_rollback(states);
 }
 exit:
 QSIMPLEQ_FOREACH_SAFE(states, &snap_bdrv_states, entry, next) {
-- 
1.7.1

[Qemu-devel] [PATCH V4 2/5] block: move input parsing code in qmp_transaction()

[Wenchao Xia]

The code is moved into preparation function, and changed
a bit to tip more clearly what it is doing.

Signed-off-by: Wenchao Xia 
Reviewed-by: Eric Blake 
---
 blockdev.c |   38 +++---
 1 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 14784eb..06100d7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -785,10 +785,7 @@ typedef struct BlkTransactionStates {
 QSIMPLEQ_ENTRY(BlkTransactionStates) entry;
 } BlkTransactionStates;
 
-static void external_snapshot_prepare(const char *device,
-  const char *format,
-  const char *new_image_file,
-  enum NewImageMode mode,
+static void external_snapshot_prepare(BlockdevAction *action,
   BlkTransactionStates *states,
   Error **errp)
 {
@@ -796,7 +793,24 @@ static void external_snapshot_prepare(const char *device,
 BlockDriver *drv;
 int flags, ret;
 Error *local_err = NULL;
+const char *device;
+const char *new_image_file;
+const char *format = "qcow2";
+enum NewImageMode mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
 
+/* get parameters */
+g_assert(action->kind == BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC);
+
+device = action->blockdev_snapshot_sync->device;
+new_image_file = action->blockdev_snapshot_sync->snapshot_file;
+if (action->blockdev_snapshot_sync->has_format) {
+format = action->blockdev_snapshot_sync->format;
+}
+if (action->blockdev_snapshot_sync->has_mode) {
+mode = action->blockdev_snapshot_sync->mode;
+}
+
+/* start processing */
 drv = bdrv_find_format(format);
 if (!drv) {
 error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
@@ -877,10 +891,6 @@ void qmp_transaction(BlockdevActionList *dev_list, Error 
**errp)
 /* We don't do anything in this loop that commits us to the snapshot */
 while (NULL != dev_entry) {
 BlockdevAction *dev_info = NULL;
-enum NewImageMode mode;
-const char *new_image_file;
-const char *device;
-const char *format = "qcow2";
 
 dev_info = dev_entry->value;
 dev_entry = dev_entry->next;
@@ -890,17 +900,7 @@ void qmp_transaction(BlockdevActionList *dev_list, Error 
**errp)
 
 switch (dev_info->kind) {
 case BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC:
-device = dev_info->blockdev_snapshot_sync->device;
-if (!dev_info->blockdev_snapshot_sync->has_mode) {
-dev_info->blockdev_snapshot_sync->mode = 
NEW_IMAGE_MODE_ABSOLUTE_PATHS;
-}
-new_image_file = dev_info->blockdev_snapshot_sync->snapshot_file;
-if (dev_info->blockdev_snapshot_sync->has_format) {
-format = dev_info->blockdev_snapshot_sync->format;
-}
-mode = dev_info->blockdev_snapshot_sync->mode;
-external_snapshot_prepare(device, format, new_image_file,
-  mode, states, &local_err);
+external_snapshot_prepare(dev_info, states, errp);
 if (error_is_set(&local_err)) {
 error_propagate(errp, local_err);
 goto delete_and_fail;
-- 
1.7.1

[Qemu-devel] [PATCH V4 1/5] block: package preparation code in qmp_transaction()

[Wenchao Xia]

The code before really committing is moved into a function. Most
code are simply moved from qmp_transaction(), except that on fail it
just return now. Other code such as input parsing is not touched,
to make it easier in review.

Signed-off-by: Wenchao Xia 
---
 blockdev.c |  139 +---
 1 files changed, 77 insertions(+), 62 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 6e293e9..14784eb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -785,6 +785,78 @@ typedef struct BlkTransactionStates {
 QSIMPLEQ_ENTRY(BlkTransactionStates) entry;
 } BlkTransactionStates;
 
+static void external_snapshot_prepare(const char *device,
+  const char *format,
+  const char *new_image_file,
+  enum NewImageMode mode,
+  BlkTransactionStates *states,
+  Error **errp)
+{
+BlockDriver *proto_drv;
+BlockDriver *drv;
+int flags, ret;
+Error *local_err = NULL;
+
+drv = bdrv_find_format(format);
+if (!drv) {
+error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
+return;
+}
+
+states->old_bs = bdrv_find(device);
+if (!states->old_bs) {
+error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+return;
+}
+
+if (!bdrv_is_inserted(states->old_bs)) {
+error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
+return;
+}
+
+if (bdrv_in_use(states->old_bs)) {
+error_set(errp, QERR_DEVICE_IN_USE, device);
+return;
+}
+
+if (!bdrv_is_read_only(states->old_bs)) {
+if (bdrv_flush(states->old_bs)) {
+error_set(errp, QERR_IO_ERROR);
+return;
+}
+}
+
+flags = states->old_bs->open_flags;
+
+proto_drv = bdrv_find_protocol(new_image_file);
+if (!proto_drv) {
+error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
+return;
+}
+
+/* create new image w/backing file */
+if (mode != NEW_IMAGE_MODE_EXISTING) {
+bdrv_img_create(new_image_file, format,
+states->old_bs->filename,
+states->old_bs->drv->format_name,
+NULL, -1, flags, &local_err, false);
+if (error_is_set(&local_err)) {
+error_propagate(errp, local_err);
+return;
+}
+}
+
+/* We will manually add the backing_hd field to the bs later */
+states->new_bs = bdrv_new("");
+/* TODO Inherit bs->options or only take explicit options with an
+ * extended QMP command? */
+ret = bdrv_open(states->new_bs, new_image_file, NULL,
+flags | BDRV_O_NO_BACKING, drv);
+if (ret != 0) {
+error_set(errp, QERR_OPEN_FILE_FAILED, new_image_file);
+}
+}
+
 /*
  * 'Atomic' group snapshots.  The snapshots are taken as a set, and if any fail
  *  then we do not pivot any of the devices in the group, and abandon the
@@ -792,7 +864,6 @@ typedef struct BlkTransactionStates {
  */
 void qmp_transaction(BlockdevActionList *dev_list, Error **errp)
 {
-int ret = 0;
 BlockdevActionList *dev_entry = dev_list;
 BlkTransactionStates *states, *next;
 Error *local_err = NULL;
@@ -806,9 +877,6 @@ void qmp_transaction(BlockdevActionList *dev_list, Error 
**errp)
 /* We don't do anything in this loop that commits us to the snapshot */
 while (NULL != dev_entry) {
 BlockdevAction *dev_info = NULL;
-BlockDriver *proto_drv;
-BlockDriver *drv;
-int flags;
 enum NewImageMode mode;
 const char *new_image_file;
 const char *device;
@@ -831,70 +899,17 @@ void qmp_transaction(BlockdevActionList *dev_list, Error 
**errp)
 format = dev_info->blockdev_snapshot_sync->format;
 }
 mode = dev_info->blockdev_snapshot_sync->mode;
-break;
-default:
-abort();
-}
-
-drv = bdrv_find_format(format);
-if (!drv) {
-error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
-goto delete_and_fail;
-}
-
-states->old_bs = bdrv_find(device);
-if (!states->old_bs) {
-error_set(errp, QERR_DEVICE_NOT_FOUND, device);
-goto delete_and_fail;
-}
-
-if (!bdrv_is_inserted(states->old_bs)) {
-error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
-goto delete_and_fail;
-}
-
-if (bdrv_in_use(states->old_bs)) {
-error_set(errp, QERR_DEVICE_IN_USE, device);
-goto delete_and_fail;
-}
-
-if (!bdrv_is_read_only(states->old_bs)) {
-if (bdrv_flush(states->old_bs)) {
-error_set(errp, QERR_IO_ERROR);
-goto delete_and_fail;
-}
-}
-
-flags = states->old_bs->open_flags;
-
-proto_drv =

[Qemu-devel] [PATCH V4 3/5] block: package committing code in qmp_transaction()

[Wenchao Xia]

The code is simply moved into a separate function.

Signed-off-by: Wenchao Xia 
---
 blockdev.c |   19 ---
 1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 06100d7..26bc78e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -871,6 +871,17 @@ static void external_snapshot_prepare(BlockdevAction 
*action,
 }
 }
 
+static void external_snapshot_commit(BlkTransactionStates *states)
+{
+/* This removes our old bs from the bdrv_states, and adds the new bs */
+bdrv_append(states->new_bs, states->old_bs);
+/* We don't need (or want) to use the transactional
+ * bdrv_reopen_multiple() across all the entries at once, because we
+ * don't want to abort all of them if one of them fails the reopen */
+bdrv_reopen(states->new_bs, states->new_bs->open_flags & ~BDRV_O_RDWR,
+NULL);
+}
+
 /*
  * 'Atomic' group snapshots.  The snapshots are taken as a set, and if any fail
  *  then we do not pivot any of the devices in the group, and abandon the
@@ -916,13 +927,7 @@ void qmp_transaction(BlockdevActionList *dev_list, Error 
**errp)
 /* Now we are going to do the actual pivot.  Everything up to this point
  * is reversible, but we are committed at this point */
 QSIMPLEQ_FOREACH(states, &snap_bdrv_states, entry) {
-/* This removes our old bs from the bdrv_states, and adds the new bs */
-bdrv_append(states->new_bs, states->old_bs);
-/* We don't need (or want) to use the transactional
- * bdrv_reopen_multiple() across all the entries at once, because we
- * don't want to abort all of them if one of them fails the reopen */
-bdrv_reopen(states->new_bs, states->new_bs->open_flags & ~BDRV_O_RDWR,
-NULL);
+external_snapshot_commit(states);
 }
 
 /* success */
-- 
1.7.1

[Qemu-devel] [PATCH V4 0/5] block: make qmp_transaction extendable

[Wenchao Xia]

  This serial will package backing chain snapshot code as one case, to make it
possible adding more operations later.

v2:
  Address Kevin's comments:
  Use the same prototype prepare, commit, rollback model in original code,
commit should never fail.

v3:
  Address Stefan's comments:
  3/5, 4/5: remove *action parameter since later only BlkTransactionStates* is
needed.
  5/5: embbed BlkTransactionStates in ExternalSnapshotStates, *opaque is
removed, related call back function format change for external snapshot.
  Address Kevin's comments:
  removed all indention in commit message.
  1/5: return void for prepare() function, *errp plays the role as error
checker.
  5/5: mark *commit callback must exist, *rollback callback can be NULL. Align
"callback =" in "const BdrvActionOps external_snapshot_ops" to the same colum.
  Address Eric's comments:
  1/5: better commit message.
  5/5: better commit message and comments in code that only one of rollback()
or commit() will be called.

v4:
  5/5: document clean() callback will always be called if it present, declare
static for global variable "actions", use array plus .instance_size to remove
"switch" checking code according to caller input.

Wenchao Xia (5):
  1 block: package preparation code in qmp_transaction()
  2 block: move input parsing code in qmp_transaction()
  3 block: package committing code in qmp_transaction()
  4 block: package rollback code in qmp_transaction()
  5 block: make all steps in qmp_transaction() as callback

 blockdev.c |  263 ++-
 1 files changed, 169 insertions(+), 94 deletions(-)

[Qemu-devel] [PATCH v5 5/6] vmdk: store fields of VmdkMetaData in cpu endian

[Fam Zheng]

Previously VmdkMetaData.offset is stored little endian while other
fields are cpu endian. This changes offset to cpu endian and convert
before writing to image.
Signed-off-by: Fam Zheng 
---
 block/vmdk.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 0463d3b..d98f304 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -813,14 +813,15 @@ static int get_whole_cluster(BlockDriverState *bs,
 
 static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData *m_data)
 {
+uint32_t offset;
+QEMU_BUILD_BUG_ON(sizeof(offset) != sizeof(m_data->offset));
+offset = cpu_to_le32(m_data->offset);
 /* update L2 table */
 if (bdrv_pwrite_sync(
 extent->file,
 ((int64_t)m_data->l2_offset * 512)
 + (m_data->l2_index * sizeof(m_data->offset)),
-&(m_data->offset),
-sizeof(m_data->offset)
-) < 0) {
+&offset, sizeof(offset)) < 0) {
 return VMDK_ERROR;
 }
 /* update backup L2 table */
@@ -830,8 +831,7 @@ static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData 
*m_data)
 extent->file,
 ((int64_t)m_data->l2_offset * 512)
 + (m_data->l2_index * sizeof(m_data->offset)),
-&(m_data->offset), sizeof(m_data->offset)
-) < 0) {
+&offset, sizeof(offset)) < 0) {
 return VMDK_ERROR;
 }
 }
@@ -848,7 +848,7 @@ static int get_cluster_offset(BlockDriverState *bs,
 {
 unsigned int l1_index, l2_offset, l2_index;
 int min_index, i, j;
-uint32_t min_count, *l2_table, tmp = 0;
+uint32_t min_count, *l2_table;
 bool zeroed = false;
 
 if (m_data) {
@@ -924,8 +924,7 @@ static int get_cluster_offset(BlockDriverState *bs,
 }
 
 *cluster_offset >>= 9;
-tmp = cpu_to_le32(*cluster_offset);
-l2_table[l2_index] = tmp;
+l2_table[l2_index] = cpu_to_le32(*cluster_offset);
 
 /* First of all we write grain itself, to avoid race condition
  * that may to corrupt the image.
@@ -938,7 +937,7 @@ static int get_cluster_offset(BlockDriverState *bs,
 }
 
 if (m_data) {
-m_data->offset = tmp;
+m_data->offset = *cluster_offset;
 m_data->l1_index = l1_index;
 m_data->l2_index = l2_index;
 m_data->l2_offset = l2_offset;
-- 
1.8.1.4

[Qemu-devel] [PATCH v5 6/6] vmdk: add bdrv_co_write_zeroes

[Fam Zheng]

Use special offset to write zeroes efficiently, when zeroed-grain GTE is
available. If zero-write an allocated cluster, cluster is leaked because
its offset pointer is overwritten by "0x1".

Signed-off-by: Fam Zheng 
---
 block/vmdk.c | 86 +++-
 1 file changed, 68 insertions(+), 18 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index d98f304..608daaf 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -124,6 +124,7 @@ typedef struct VmdkMetaData {
 unsigned int l2_index;
 unsigned int l2_offset;
 int valid;
+uint32_t *l2_cache_entry;
 } VmdkMetaData;
 
 typedef struct VmdkGrainMarker {
@@ -835,6 +836,9 @@ static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData 
*m_data)
 return VMDK_ERROR;
 }
 }
+if (m_data->l2_cache_entry) {
+*m_data->l2_cache_entry = offset;
+}
 
 return VMDK_OK;
 }
@@ -905,6 +909,14 @@ static int get_cluster_offset(BlockDriverState *bs,
 l2_index = ((offset >> 9) / extent->cluster_sectors) % extent->l2_size;
 *cluster_offset = le32_to_cpu(l2_table[l2_index]);
 
+if (m_data) {
+m_data->valid = 1;
+m_data->l1_index = l1_index;
+m_data->l2_index = l2_index;
+m_data->offset = *cluster_offset;
+m_data->l2_offset = l2_offset;
+m_data->l2_cache_entry = &l2_table[l2_index];
+}
 if (extent->has_zero_grain && *cluster_offset == VMDK_GTE_ZEROED) {
 zeroed = true;
 }
@@ -938,10 +950,6 @@ static int get_cluster_offset(BlockDriverState *bs,
 
 if (m_data) {
 m_data->offset = *cluster_offset;
-m_data->l1_index = l1_index;
-m_data->l2_index = l2_index;
-m_data->l2_offset = l2_offset;
-m_data->valid = 1;
 }
 }
 *cluster_offset <<= 9;
@@ -1164,8 +1172,17 @@ static coroutine_fn int vmdk_co_read(BlockDriverState 
*bs, int64_t sector_num,
 return ret;
 }
 
+/**
+ * vmdk_write:
+ * @zeroed:   buf is ignored (data is zero), use zeroed_grain GTE feature
+ * if possible, otherwise return -ENOTSUP.
+ * @zero_dry_run: used for zeroed == true only, don't update L2 table, just
+ *
+ * Returns: error code with 0 for success.
+ */
 static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
- const uint8_t *buf, int nb_sectors)
+  const uint8_t *buf, int nb_sectors,
+  bool zeroed, bool zero_dry_run)
 {
 BDRVVmdkState *s = bs->opaque;
 VmdkExtent *extent = NULL;
@@ -1211,7 +1228,7 @@ static int vmdk_write(BlockDriverState *bs, int64_t 
sector_num,
 &cluster_offset);
 }
 }
-if (ret) {
+if (ret == VMDK_ERROR) {
 return -EINVAL;
 }
 extent_begin_sector = extent->end_sector - extent->sectors;
@@ -1221,17 +1238,34 @@ static int vmdk_write(BlockDriverState *bs, int64_t 
sector_num,
 if (n > nb_sectors) {
 n = nb_sectors;
 }
-
-ret = vmdk_write_extent(extent,
-cluster_offset, index_in_cluster * 512,
-buf, n, sector_num);
-if (ret) {
-return ret;
-}
-if (m_data.valid) {
-/* update L2 tables */
-if (vmdk_L2update(extent, &m_data) == -1) {
-return -EIO;
+if (zeroed) {
+/* Do zeroed write, buf is ignored */
+if (extent->has_zero_grain &&
+index_in_cluster == 0 &&
+n >= extent->cluster_sectors) {
+n = extent->cluster_sectors;
+if (!zero_dry_run) {
+m_data.offset = VMDK_GTE_ZEROED;
+/* update L2 tables */
+if (vmdk_L2update(extent, &m_data) != VMDK_OK) {
+return -EIO;
+}
+}
+} else {
+return -ENOTSUP;
+}
+} else {
+ret = vmdk_write_extent(extent,
+cluster_offset, index_in_cluster * 512,
+buf, n, sector_num);
+if (ret) {
+return ret;
+}
+if (m_data.valid) {
+/* update L2 tables */
+if (vmdk_L2update(extent, &m_data) != VMDK_OK) {
+return -EIO;
+}
 }
 }
 nb_sectors -= n;
@@ -1257,7 +1291,22 @@ static coroutine_fn int vmdk_co_write(BlockDriverState 
*bs, int64_t sector_num,
 int ret;
 BDRVVmdkState *s = bs->opaque;
 qemu_co_mutex_lock(&s->lock);
-ret = vmdk_write(bs, sector_num, buf, nb_sectors);
+ret = vmdk_write(bs, sector_num, buf, nb_sectors, false, false);
+qemu_co_mutex_unlock(&s->lock);
+return ret;
+}
+
+static int coroutine_fn vmdk_co_write_zeroes(BlockDriverState *bs,
+  

[Qemu-devel] [PATCH v5 3/6] vmdk: Add option to create zeroed-grain image

[Fam Zheng]

Add image create option "zeroed-grain" to enable zeroed-grain GTE
feature of vmdk sparse extents. When this option is on, header version
of newly created extent will be 2 and VMDK4_FLAG_ZERO_GRAIN flag bit
will be set.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c | 22 +-
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 7e07c0f..cc19e20 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -48,6 +48,8 @@
 #define VMDK_UNALLOC (-2)
 #define VMDK_ZEROED  (-3)
 
+#define BLOCK_OPT_ZEROED_GRAIN "zeroed_grain"
+
 typedef struct {
 uint32_t version;
 uint32_t flags;
@@ -1262,7 +1264,7 @@ static coroutine_fn int vmdk_co_write(BlockDriverState 
*bs, int64_t sector_num,
 
 
 static int vmdk_create_extent(const char *filename, int64_t filesize,
-  bool flat, bool compress)
+  bool flat, bool compress, bool zeroed_grain)
 {
 int ret, i;
 int fd = 0;
@@ -1284,9 +1286,10 @@ static int vmdk_create_extent(const char *filename, 
int64_t filesize,
 }
 magic = cpu_to_be32(VMDK4_MAGIC);
 memset(&header, 0, sizeof(header));
-header.version = 1;
-header.flags =
-3 | (compress ? VMDK4_FLAG_COMPRESS | VMDK4_FLAG_MARKER : 0);
+header.version = zeroed_grain ? 2 : 1;
+header.flags = 3
+   | (compress ? VMDK4_FLAG_COMPRESS | VMDK4_FLAG_MARKER : 0)
+   | (zeroed_grain ? VMDK4_FLAG_ZERO_GRAIN : 0);
 header.compressAlgorithm = compress ? VMDK4_COMPRESSION_DEFLATE : 0;
 header.capacity = filesize / 512;
 header.granularity = 128;
@@ -1467,6 +1470,7 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options)
 char parent_desc_line[BUF_SIZE] = "";
 uint32_t parent_cid = 0x;
 uint32_t number_heads = 16;
+bool zeroed_grain = false;
 const char desc_template[] =
 "# Disk DescriptorFile\n"
 "version=1\n"
@@ -1502,6 +1506,8 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options)
 flags |= options->value.n ? BLOCK_FLAG_COMPAT6 : 0;
 } else if (!strcmp(options->name, BLOCK_OPT_SUBFMT)) {
 fmt = options->value.s;
+} else if (!strcmp(options->name, BLOCK_OPT_ZEROED_GRAIN)) {
+zeroed_grain |= options->value.n;
 }
 options++;
 }
@@ -1588,7 +1594,8 @@ static int vmdk_create(const char *filename, 
QEMUOptionParameter *options)
 snprintf(ext_filename, sizeof(ext_filename), "%s%s",
 path, desc_filename);
 
-if (vmdk_create_extent(ext_filename, size, flat, compress)) {
+if (vmdk_create_extent(ext_filename, size,
+   flat, compress, zeroed_grain)) {
 return -EINVAL;
 }
 filesize -= size;
@@ -1714,6 +1721,11 @@ static QEMUOptionParameter vmdk_create_options[] = {
 "VMDK flat extent format, can be one of "
 "{monolithicSparse (default) | monolithicFlat | 
twoGbMaxExtentSparse | twoGbMaxExtentFlat | streamOptimized} "
 },
+{
+.name = BLOCK_OPT_ZEROED_GRAIN,
+.type = OPT_FLAG,
+.help = "Enable efficient zero writes using the zeroed-grain GTE 
feature"
+},
 { NULL }
 };
 
-- 
1.8.1.4

[Qemu-devel] [PATCH v5 4/6] vmdk: change magic number to macro

[Fam Zheng]

Two hard coded flag bits are changed to macros.
Signed-off-by: Fam Zheng 
---
 block/vmdk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index cc19e20..0463d3b 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -32,6 +32,7 @@
 #define VMDK3_MAGIC (('C' << 24) | ('O' << 16) | ('W' << 8) | 'D')
 #define VMDK4_MAGIC (('K' << 24) | ('D' << 16) | ('M' << 8) | 'V')
 #define VMDK4_COMPRESSION_DEFLATE 1
+#define VMDK4_FLAG_NL_DETECT (1 << 0)
 #define VMDK4_FLAG_RGD (1 << 1)
 /* Zeroed-grain enable bit */
 #define VMDK4_FLAG_ZERO_GRAIN   (1 << 2)
@@ -1287,7 +1288,7 @@ static int vmdk_create_extent(const char *filename, 
int64_t filesize,
 magic = cpu_to_be32(VMDK4_MAGIC);
 memset(&header, 0, sizeof(header));
 header.version = zeroed_grain ? 2 : 1;
-header.flags = 3
+header.flags = VMDK4_FLAG_RGD | VMDK4_FLAG_NL_DETECT
| (compress ? VMDK4_FLAG_COMPRESS | VMDK4_FLAG_MARKER : 0)
| (zeroed_grain ? VMDK4_FLAG_ZERO_GRAIN : 0);
 header.compressAlgorithm = compress ? VMDK4_COMPRESSION_DEFLATE : 0;
-- 
1.8.1.4

[Qemu-devel] [PATCH v5 2/6] vmdk: add support for “zeroed‐grain” GTE

[Fam Zheng]

Introduced support for zeroed-grain GTE, as specified in Virtual Disk
Format 5.0[1].

Recent VMware hosted platform products support a new “zeroed‐grain”
grain table entry (GTE). The zeroed‐grain GTE returns all zeros on
read.  In other words, the zeroed‐grain GTE indicates that a grain
in the child disk is zero‐filled but does not actually occupy space
in storage.  A sparse extent with zeroed‐grain GTE has the following
in its header:

 * SparseExtentHeader.version = 2
 * SparseExtentHeader.flags has bit 2 set

Other than the new flag and the possibly zeroed‐grain GTE, version 2
sparse extents are identical to version 1.  Also, a zeroed‐grain GTE
has value 0x1 in the GT table.

[1] Virtual Disk Format 5.0, 
http://www.vmware.com/support/developer/vddk/vmdk_50_technote.pdf?src=vmdk
Signed-off-by: Fam Zheng 
---
 block/vmdk.c | 24 ++--
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 16aa29c..7e07c0f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -33,10 +33,13 @@
 #define VMDK4_MAGIC (('K' << 24) | ('D' << 16) | ('M' << 8) | 'V')
 #define VMDK4_COMPRESSION_DEFLATE 1
 #define VMDK4_FLAG_RGD (1 << 1)
+/* Zeroed-grain enable bit */
+#define VMDK4_FLAG_ZERO_GRAIN   (1 << 2)
 #define VMDK4_FLAG_COMPRESS (1 << 16)
 #define VMDK4_FLAG_MARKER (1 << 17)
 #define VMDK4_GD_AT_END 0xULL
 
+#define VMDK_GTE_ZEROED 0x1
 
 /* VMDK internal error codes */
 #define VMDK_OK  0
@@ -81,6 +84,8 @@ typedef struct VmdkExtent {
 bool flat;
 bool compressed;
 bool has_marker;
+bool has_zero_grain;
+int version;
 int64_t sectors;
 int64_t end_sector;
 int64_t flat_start_offset;
@@ -569,6 +574,8 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
 extent->compressed =
 le16_to_cpu(header.compressAlgorithm) == VMDK4_COMPRESSION_DEFLATE;
 extent->has_marker = le32_to_cpu(header.flags) & VMDK4_FLAG_MARKER;
+extent->version = le32_to_cpu(header.version);
+extent->has_zero_grain = le32_to_cpu(header.flags) & VMDK4_FLAG_ZERO_GRAIN;
 ret = vmdk_init_tables(bs, extent);
 if (ret) {
 /* free extent allocated by vmdk_add_extent */
@@ -839,6 +846,7 @@ static int get_cluster_offset(BlockDriverState *bs,
 unsigned int l1_index, l2_offset, l2_index;
 int min_index, i, j;
 uint32_t min_count, *l2_table, tmp = 0;
+bool zeroed = false;
 
 if (m_data) {
 m_data->valid = 0;
@@ -894,9 +902,13 @@ static int get_cluster_offset(BlockDriverState *bs,
 l2_index = ((offset >> 9) / extent->cluster_sectors) % extent->l2_size;
 *cluster_offset = le32_to_cpu(l2_table[l2_index]);
 
-if (!*cluster_offset) {
+if (extent->has_zero_grain && *cluster_offset == VMDK_GTE_ZEROED) {
+zeroed = true;
+}
+
+if (!*cluster_offset || zeroed) {
 if (!allocate) {
-return VMDK_UNALLOC;
+return zeroed ? VMDK_ZEROED : VMDK_UNALLOC;
 }
 
 /* Avoid the L2 tables update for the images that have snapshots. */
@@ -967,8 +979,8 @@ static int coroutine_fn 
vmdk_co_is_allocated(BlockDriverState *bs,
 ret = get_cluster_offset(bs, extent, NULL,
 sector_num * 512, 0, &offset);
 qemu_co_mutex_unlock(&s->lock);
-/* get_cluster_offset returning 0 means success */
-ret = !ret;
+
+ret = (ret == VMDK_OK || ret == VMDK_ZEROED);
 
 index_in_cluster = sector_num % extent->cluster_sectors;
 n = extent->cluster_sectors - index_in_cluster;
@@ -,9 +1123,9 @@ static int vmdk_read(BlockDriverState *bs, int64_t 
sector_num,
 if (n > nb_sectors) {
 n = nb_sectors;
 }
-if (ret) {
+if (ret != VMDK_OK) {
 /* if not allocated, try to read from parent image, if exist */
-if (bs->backing_hd) {
+if (bs->backing_hd && ret != VMDK_ZEROED) {
 if (!vmdk_is_cid_valid(bs)) {
 return -EINVAL;
 }
-- 
1.8.1.4

[Qemu-devel] [PATCH v5 1/6] vmdk: named return code.

[Fam Zheng]

Internal routines in vmdk.c previously return -1 on error and 0 on
success. More return values are useful for future changes such as
zeroed-grain GTE. Change all the magic `return 0` and `return -1` to
macro names:

 * VMDK_OK  0
 * VMDK_ERROR   (-1)
 * VMDK_UNALLOC (-2)
 * VMDK_ZEROED  (-3)

Signed-off-by: Fam Zheng 
---
 block/vmdk.c | 60 ++--
 1 file changed, 34 insertions(+), 26 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 7bad757..16aa29c 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -37,6 +37,14 @@
 #define VMDK4_FLAG_MARKER (1 << 17)
 #define VMDK4_GD_AT_END 0xULL
 
+
+/* VMDK internal error codes */
+#define VMDK_OK  0
+#define VMDK_ERROR   (-1)
+/* Cluster not allocated */
+#define VMDK_UNALLOC (-2)
+#define VMDK_ZEROED  (-3)
+
 typedef struct {
 uint32_t version;
 uint32_t flags;
@@ -578,22 +586,22 @@ static int vmdk_parse_description(const char *desc, const 
char *opt_name,
 
 opt_pos = strstr(desc, opt_name);
 if (!opt_pos) {
-return -1;
+return VMDK_ERROR;
 }
 /* Skip "=\"" following opt_name */
 opt_pos += strlen(opt_name) + 2;
 if (opt_pos >= end) {
-return -1;
+return VMDK_ERROR;
 }
 opt_end = opt_pos;
 while (opt_end < end && *opt_end != '"') {
 opt_end++;
 }
 if (opt_end == end || buf_size < opt_end - opt_pos + 1) {
-return -1;
+return VMDK_ERROR;
 }
 pstrcpy(buf, opt_end - opt_pos + 1, opt_pos);
-return 0;
+return VMDK_OK;
 }
 
 /* Open an extent file and append to bs array */
@@ -772,7 +780,7 @@ static int get_whole_cluster(BlockDriverState *bs,
 int ret;
 
 if (!vmdk_is_cid_valid(bs)) {
-return -1;
+return VMDK_ERROR;
 }
 
 /* floor offset to cluster */
@@ -780,17 +788,17 @@ static int get_whole_cluster(BlockDriverState *bs,
 ret = bdrv_read(bs->backing_hd, offset >> 9, whole_grain,
 extent->cluster_sectors);
 if (ret < 0) {
-return -1;
+return VMDK_ERROR;
 }
 
 /* Write grain only into the active image */
 ret = bdrv_write(extent->file, cluster_offset, whole_grain,
 extent->cluster_sectors);
 if (ret < 0) {
-return -1;
+return VMDK_ERROR;
 }
 }
-return 0;
+return VMDK_OK;
 }
 
 static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData *m_data)
@@ -803,7 +811,7 @@ static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData 
*m_data)
 &(m_data->offset),
 sizeof(m_data->offset)
 ) < 0) {
-return -1;
+return VMDK_ERROR;
 }
 /* update backup L2 table */
 if (extent->l1_backup_table_offset != 0) {
@@ -814,11 +822,11 @@ static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData 
*m_data)
 + (m_data->l2_index * sizeof(m_data->offset)),
 &(m_data->offset), sizeof(m_data->offset)
 ) < 0) {
-return -1;
+return VMDK_ERROR;
 }
 }
 
-return 0;
+return VMDK_OK;
 }
 
 static int get_cluster_offset(BlockDriverState *bs,
@@ -837,17 +845,17 @@ static int get_cluster_offset(BlockDriverState *bs,
 }
 if (extent->flat) {
 *cluster_offset = extent->flat_start_offset;
-return 0;
+return VMDK_OK;
 }
 
 offset -= (extent->end_sector - extent->sectors) * SECTOR_SIZE;
 l1_index = (offset >> 9) / extent->l1_entry_sectors;
 if (l1_index >= extent->l1_size) {
-return -1;
+return VMDK_ERROR;
 }
 l2_offset = extent->l1_table[l1_index];
 if (!l2_offset) {
-return -1;
+return VMDK_UNALLOC;
 }
 for (i = 0; i < L2_CACHE_SIZE; i++) {
 if (l2_offset == extent->l2_cache_offsets[i]) {
@@ -877,7 +885,7 @@ static int get_cluster_offset(BlockDriverState *bs,
 l2_table,
 extent->l2_size * sizeof(uint32_t)
 ) != extent->l2_size * sizeof(uint32_t)) {
-return -1;
+return VMDK_ERROR;
 }
 
 extent->l2_cache_offsets[min_index] = l2_offset;
@@ -888,7 +896,7 @@ static int get_cluster_offset(BlockDriverState *bs,
 
 if (!*cluster_offset) {
 if (!allocate) {
-return -1;
+return VMDK_UNALLOC;
 }
 
 /* Avoid the L2 tables update for the images that have snapshots. */
@@ -911,7 +919,7 @@ static int get_cluster_offset(BlockDriverState *bs,
  */
 if (get_whole_cluster(
 bs, extent, *cluster_offset, offset, allocate) == -1) {
-return -1;
+return VMDK_ERROR;
 }
 
 if (m_data) {
@@ -923,7 +931,7 @@ static int get_cluster_offset(BlockDriverState *bs,
 }
 }
 *cluster_offset <<= 9;
-return 0;
+return VMDK_OK;
 }
 
 static VmdkExtent *find_exten

[Qemu-devel] [PATCH v5 0/6] vmdk: zeroed-grain GTE support

[Fam Zheng]

Added support for zeroed-grain GTE to VMDK according to VMDK Spec 5.0[1].

[1] Virtual Disk Format 5.0 - VMware,
http://www.vmware.com/support/developer/vddk/vmdk_50_technote.pdf?src=vmdk

Changes since v4:
 - 6/6: Correct endianess: *m_data->l2_cache_entry = offset;


Changes since v3:
 - 5/6: Remove tmp
 - 6/6: Update L2 Cache
Remove redundant assignment to m_data
Simplify l2_offset assignment to m_data
Fix m_data.offset endian.

Changes since v2:
 - all: Added 5/6 (vmdk: store fields of VmdkMetaData in cpu endian)
 - 6/6: Avoid side-effect of vmdk_L2update.
Change function comment to gtkdoc stype.
Fix VMDK4_FLAG_ZG.

Changes since v1:
 - all: fix From: field
 - 1/5: squash one line of ret code macro change from 2/5
 - 2/5: change VMDK4_FLAG_ZG to VMDK4_FLAG_ZERO_GRAIN
 - 3/5: move BLOCK_OPT_ZEROED_GRAIN defination from block_int.h to vmdk.c
 - 5/5: fix metadata update issue, unit test with cases 033 034

Fam Zheng (6):
  vmdk: named return code.
  vmdk: add support for “zeroed‐grain” GTE
  vmdk: Add option to create zeroed-grain image
  vmdk: change magic number to macro
  vmdk: store fields of VmdkMetaData in cpu endian
  vmdk: add bdrv_co_write_zeroes

 block/vmdk.c | 208 +--
 1 file changed, 145 insertions(+), 63 deletions(-)

-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 7/7] block: dump to monitor for bdrv_snapshot_dump() and bdrv_image_info_dump()

[Wenchao Xia]

于 2013-4-30 3:05, Luiz Capitulino 写道:

On Fri, 26 Apr 2013 16:46:57 +0200
Stefan Hajnoczi  wrote:


On Fri, Apr 26, 2013 at 05:31:15PM +0800, Wenchao Xia wrote:

@@ -2586,10 +2585,12 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict)
  }

  if (total > 0) {
-monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), 
NULL));
+bdrv_snapshot_dump(NULL);
+monitor_printf(mon, "\n");


Luiz: any issue with mixing monitor_printf(mon) and
monitor_vprintf(cur_mon) calls?  I guess there was a reason for
explicitly passing mon instead of relying on cur_mon.


where are they being mixed?


  bdrv_snapshot_dump() used a global variable "cur_mon" inside, instead
of let caller pass in a explicit montior* "mon", I guess that is the
question.

--
Best Regards

Wenchao Xia

Re: [Qemu-devel] [PATCH 4/7] block: distinguish id and name in bdrv_find_snapshot()

[Wenchao Xia]

于 2013-5-1 2:16, Eric Blake 写道:

On 04/26/2013 03:31 AM, Wenchao Xia wrote:

To make it clear about id and name in searching, the API is changed
a bit to distinguish them, and caller can choose to search by id or name.
If not found, *errp will be set to tip why.

Note that the caller logic is changed a bit:
1) In del_existing_snapshots() called by do_savevm(), it travers twice
to find the snapshot, instead once, so matching sequence may change
if there are unwisely chosen, mixed id and names.
2) In do_savevm(), same with del_existing_snapshot(), when it tries to
find the snapshot to overwrite, matching sequence may change for same
reason.
3) In load_vmstate(), first when it tries to find the snapshot to be loaded,
sequence may change for the same reason of above. Later in validation, the
logic is changed to be more strict to require both id and name matching.
4) In do_info_snapshot(), in validation, the logic is changed to be more
strict to require both id and name matching.

Savevm, loadvm logic may need to be improved later, to avoid mixing of them.

Some code is borrowed from Pavel's patch.

Signed-off-by: Wenchao Xia 
Signed-off-by: Pavel Hrdina 
---
  block/snapshot.c |   72 +++---
  include/block/snapshot.h |5 ++-
  savevm.c |   35 --
  3 files changed, 83 insertions(+), 29 deletions(-)



+ *
+ * Returns: true when a snapshot is found and @sn_info will be filled, false
+ * when error or not found with @errp filled if errp != NULL.
+ */
+bool bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
+const char *id, const char *name, Error **errp)


Unusual convention to have (input, output, input, input, output)
parameters; as long as you are changing the signature, I'd consider
putting all input parameters (bs, id, name) firs, then output parameters
last (sn_info, errp).


  {
  QEMUSnapshotInfo *sn_tab, *sn;
-int nb_sns, i, ret;
+int nb_sns, i;
+bool ret = false;

-ret = -ENOENT;
  nb_sns = bdrv_snapshot_list(bs, &sn_tab);
  if (nb_sns < 0) {
-return ret;
+error_setg_errno(errp, -nb_sns, "Failed to get a snapshot list");
+return false;
+} else if (nb_sns == 0) {
+error_setg(errp, "Device has no snapshots");
+return false;
  }
-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;
+


No assertion that at least one of id or name is provided,...


+
+if (id && name) {
+for (i = 0; i < nb_sns; i++) {
+sn = &sn_tab[i];
+if (!strcmp(sn->id_str, id) && !strcmp(sn->name, name)) {
+*sn_info = *sn;
+ret = true;
+break;
+}
+}
+} else if (id) {
+for (i = 0; i < nb_sns; i++) {
+sn = &sn_tab[i];
+if (!strcmp(sn->id_str, id)) {
+*sn_info = *sn;
+ret = true;
+break;
+}
+}
+} else if (name) {
+for (i = 0; i < nb_sns; i++) {
+sn = &sn_tab[i];
+if (!strcmp(sn->name, name)) {
+*sn_info = *sn;
+ret = true;
+break;
+}
  }
  }
+
+if (!ret) {
+error_setg(errp, "Device have no matching snapshot");
+}


...therefore, if I call bdrv_snapshot_find(bs, &info, NULL, NULL, errp),
I'll get this error.  Seems okay.


+++ b/savevm.c
@@ -2286,8 +2286,8 @@ static int del_existing_snapshots(Monitor *mon, const 
char *name)
  bs = NULL;
  while ((bs = bdrv_next(bs))) {
  if (bdrv_can_snapshot(bs) &&
-bdrv_snapshot_find(bs, snapshot, name) >= 0)
-{
+(bdrv_snapshot_find(bs, snapshot, name, NULL, NULL) ||
+ bdrv_snapshot_find(bs, snapshot, NULL, name, NULL))) {


This does an id lookup first, and falls back to a name lookup.  Is that
what we want?  Consider an image with the following snapshots:

id 1 name 2
id 2 name 3
id 3 name 1
id 4 name 5

Pre-patch, find(1) gives id 1, find(2) gives id 1, find(3) gives id 2,
find(4) gives id 4, find(5) gives id 4; no way to get id 3.  Post-patch,
find(1,NULL) gives id 1, find(2,NULL) gives id 2, find(3,NULL) gives id
3, find(4,NULL) gives id 4, find(5,NULL) fails and you fall back to
find(NULL,5) to give id 4.  Thus, it only makes a difference for
snapshots whose name is a numeric string that also matches an id, where
your change now favors the id lookup over the entire set instead of the
first name or id match while doing a single pass over the set.

Pavel's series on top of this would change the code to favor a name-only
lookup, or an explicit HMP option to do an id-only lookup, instead of
this code's double lookup.

At this point, I'm okay with the semantics of this patc

[Qemu-devel] [v2][Qemu-ppc][PATCH 1/1] PPC: e500: correct params->ram_size with ram_size

[Tiejun Chen]

We should sync params->ram_size after we fixup memory size on
a alignment boundary. Otherwise Guest would exceed the actual
memory region.

Signed-off-by: Tiejun Chen 
---
v2: 

eliminate that original comment in v1.

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

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index c1bdb6b..3978f84 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -523,6 +523,7 @@ void ppce500_init(PPCE500Params *params)
 
 /* Fixup Memory size on a alignment boundary */
 ram_size &= ~(RAM_SIZES_ALIGN - 1);
+params->ram_size = ram_size;
 
 /* Register Memory */
 memory_region_init_ram(ram, "mpc8544ds.ram", ram_size);
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH qom-cpu 4/9] target-i386: Replace cpuid_*features fields with a feature word array

[Andreas Färber]

Am 22.04.2013 21:00, schrieb Eduardo Habkost:
> This replaces the feature-bit fields on both X86CPU and x86_def_t
> structs with an array.
> 
> With this, we will be able to simplify code that simply does the same
> operation on all feature words (e.g. kvm_check_features_against_host(),
> filter_features_for_kvm(), add_flagname_to_bitmaps(), CPU feature-bit
> property lookup/registration, and the proposed "feature-words" property)
> 
> The following field replacements were made on X86CPU and x86_def_t:
> 
>   (cpuid_)features -> features[FEAT_1_EDX]
>   (cpuid_)ext_features -> features[FEAT_1_ECX]
>   (cpuid_)ext2_features-> features[FEAT_8000_0001_EDX]
>   (cpuid_)ext3_features-> features[FEAT_8000_0001_ECX]
>   (cpuid_)ext4_features-> features[FEAT_C000_0001_EDX]
>   (cpuid_)kvm_features -> features[FEAT_KVM]
>   (cpuid_)svm_features -> features[FEAT_SVM]
>   (cpuid_)7_0_ebx_features -> features[FEAT_7_0_EBX]
> 
> Signed-off-by: Eduardo Habkost 
> Reviewed-By: Igor Mammedov 
[...]
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 73ae2ef..110ef98 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
[...]
> @@ -1490,22 +1485,22 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, 
> char *features, Error **errp)
>  }
>  featurestr = strtok(NULL, ",");
>  }
> -env->cpuid_features |= plus_features[FEAT_1_EDX];
> -env->cpuid_ext_features |= plus_features[FEAT_1_ECX];
> -env->cpuid_ext2_features |= plus_features[FEAT_8000_0001_EDX];
> -env->cpuid_ext3_features |= plus_features[FEAT_8000_0001_ECX];
> -env->cpuid_ext4_features |= plus_features[FEAT_C000_0001_EDX];
> -env->cpuid_kvm_features |= plus_features[FEAT_KVM];
> -env->cpuid_svm_features |= plus_features[FEAT_SVM];
> -env->cpuid_7_0_ebx_features |= plus_features[FEAT_7_0_EBX];
> -env->cpuid_features &= ~minus_features[FEAT_1_EDX];
> -env->cpuid_ext_features &= ~minus_features[FEAT_1_ECX];
> -env->cpuid_ext2_features &= ~minus_features[FEAT_8000_0001_EDX];
> -env->cpuid_ext3_features &= ~minus_features[FEAT_8000_0001_ECX];
> -env->cpuid_ext4_features &= ~minus_features[FEAT_C000_0001_EDX];
> -env->cpuid_kvm_features &= ~minus_features[FEAT_KVM];
> -env->cpuid_svm_features &= ~minus_features[FEAT_SVM];
> -env->cpuid_7_0_ebx_features &= ~minus_features[FEAT_7_0_EBX];
> +env->features[FEAT_1_EDX] |= plus_features[FEAT_1_EDX];
> +env->features[FEAT_1_ECX] |= plus_features[FEAT_1_ECX];
> +env->features[FEAT_8000_0001_EDX] |= plus_features[FEAT_8000_0001_EDX];
> +env->features[FEAT_8000_0001_ECX] |= plus_features[FEAT_8000_0001_ECX];
> +env->features[FEAT_C000_0001_EDX] |= plus_features[FEAT_C000_0001_EDX];
> +env->features[FEAT_KVM] |= plus_features[FEAT_KVM];
> +env->features[FEAT_SVM] |= plus_features[FEAT_SVM];
> +env->features[FEAT_7_0_EBX] |= plus_features[FEAT_7_0_EBX];
> +env->features[FEAT_1_EDX] &= ~minus_features[FEAT_1_EDX];
> +env->features[FEAT_1_ECX] &= ~minus_features[FEAT_1_ECX];
> +env->features[FEAT_8000_0001_EDX] &= ~minus_features[FEAT_8000_0001_EDX];
> +env->features[FEAT_8000_0001_ECX] &= ~minus_features[FEAT_8000_0001_ECX];
> +env->features[FEAT_C000_0001_EDX] &= ~minus_features[FEAT_C000_0001_EDX];
> +env->features[FEAT_KVM] &= ~minus_features[FEAT_KVM];
> +env->features[FEAT_SVM] &= ~minus_features[FEAT_SVM];
> +env->features[FEAT_7_0_EBX] &= ~minus_features[FEAT_7_0_EBX];
>  
>  out:
>  return;

Can this be done in a loop as a follow-up?

[...]
> @@ -1630,24 +1625,24 @@ static void cpu_x86_register(X86CPU *cpu, const char 
> *name, Error **errp)
>  }
>  
>  if (kvm_enabled()) {
> -def->kvm_features |= kvm_default_features;
> +def->features[FEAT_KVM] |= kvm_default_features;
>  }
> -def->ext_features |= CPUID_EXT_HYPERVISOR;
> +def->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
>  
>  object_property_set_str(OBJECT(cpu), def->vendor, "vendor", errp);
>  object_property_set_int(OBJECT(cpu), def->level, "level", errp);
>  object_property_set_int(OBJECT(cpu), def->family, "family", errp);
>  object_property_set_int(OBJECT(cpu), def->model, "model", errp);
>  object_property_set_int(OBJECT(cpu), def->stepping, "stepping", errp);
> -env->cpuid_features = def->features;
> -env->cpuid_ext_features = def->ext_features;
> -env->cpuid_ext2_features = def->ext2_features;
> -env->cpuid_ext3_features = def->ext3_features;
> +env->features[FEAT_1_EDX] = def->features[FEAT_1_EDX];
> +env->features[FEAT_1_ECX] = def->features[FEAT_1_ECX];
> +env->features[FEAT_8000_0001_EDX] = def->features[FEAT_8000_0001_EDX];
> +env->features[FEAT_8000_0001_ECX] = def->features[FEAT_8000_0001_ECX];
>  object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", errp);
> -env->cpuid_kvm_features = def->kvm_features;
> -env->cpuid_svm_features = def->svm_features;
> -en

Re: [Qemu-devel] [PATCH 1/2] target-ppc: Fix invalid SPR read/write warnings

[Alexander Graf]

On 01.05.2013, at 12:43, Anton Blanchard wrote:

> 
> Invalid and privileged SPR warnings currently print the wrong
> address. While fixing that, also make it clear that we are
> printing both the decimal and hexadecimal SPR number.
> 
> Before:
> 
>  Trying to read invalid spr 896 380 at 0714
> 
> After:
> 
>  Trying to read invalid spr 896 (0x380) at 0710
> 
> Signed-off-by: Anton Blanchard 

Thanks, applied both to ppc-next. Welcome to the wonderful world of QEMU ;).


Alex

[Qemu-devel] [Bug 1100843] Re: Live Migration Causes Performance Issues

[Jonathan Jefferson]

I used this handy tool to run system call preliminary benchmarks: 
http://code.google.com/p/byte-unixbench/

 In a nutshell, what I found is a confirmation that live migration does indeed 
degrade performance on precise KVM. 
 I hope the below results help narrow down this critical problem to eventually 
have it resolved in 12.04 LTS version.

detail results:
Compiled the benchmarking tool and then:
root@sample-vm:~/UnixBench# ./Run syscall

Output:

** before live-migration **

Benchmark Run: Wed May 01 2013 20:29:54 - 20:32:04
1 CPU in system; running 1 parallel copy of tests
System Call Overhead4177612.4 lps   (10.0 s, 7 samples)
System Benchmarks Partial Index  BASELINE   RESULTINDEX
System Call Overhead  15000.04177612.4   2785.1
   
System Benchmarks Index Score (Partial Only) 2785.1


** after live-migration **

Benchmark Run: Wed May 01 2013 20:35:16 - 20:37:26
1 CPU in system; running 1 parallel copy of tests
System Call Overhead3065118.3 lps   (10.0 s, 7 samples)
System Benchmarks Partial Index  BASELINE   RESULTINDEX
System Call Overhead  15000.03065118.3   2043.4
   
System Benchmarks Index Score (Partial Only) 2043.4


XML domain dump:

  1048576
  1048576
  1
  
1024
  
  
hvm

  
  

  
  
  destroy
  restart
  destroy
  
/usr/bin/kvm

  
  
  
  
  


  
  
  
  
  
  


  
  


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

Title:
  Live Migration Causes Performance Issues

Status in QEMU:
  New
Status in “qemu-kvm” package in Ubuntu:
  Triaged

Bug description:
  I have 2 physical hosts running Ubuntu Precise.  With 1.0+noroms-
  0ubuntu14.7 and qemu-kvm 1.2.0+noroms-0ubuntu7 (source from quantal,
  built for Precise with pbuilder.) I attempted to build qemu-1.3.0 debs
  from source to test, but libvirt seems to have an issue with it that I
  haven't been able to track down yet.

   I'm seeing a performance degradation after live migration on Precise,
  but not Lucid.  These hosts are managed by libvirt (tested both
  0.9.8-2ubuntu17 and 1.0.0-0ubuntu4) in conjunction with OpenNebula.  I
  don't seem to have this problem with lucid guests (running a number of
  standard kernels, 3.2.5 mainline and backported linux-
  image-3.2.0-35-generic as well.)

  I first noticed this problem with phoronix doing compilation tests,
  and then tried lmbench where even simple calls experience performance
  degradation.

  I've attempted to post to the kvm mailing list, but so far the only
  suggestion was it may be related to transparent hugepages not being
  used after migration, but this didn't pan out.  Someone else has a
  similar problem here -
  http://thread.gmane.org/gmane.comp.emulators.kvm.devel/100592

  qemu command line example: /usr/bin/kvm -name one-2 -S -M pc-1.2 -cpu
  Westmere -enable-kvm -m 73728 -smp 16,sockets=2,cores=8,threads=1
  -uuid f89e31a4-4945-c12c-6544-149ba0746c2f -no-user-config -nodefaults
  -chardev
  socket,id=charmonitor,path=/var/lib/libvirt/qemu/one-2.monitor,server,nowait
  -mon chardev=charmonitor,id=monitor,mode=control -rtc
  base=utc,driftfix=slew -no-kvm-pit-reinjection -no-shutdown -device
  piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -drive
  file=/var/lib/one//datastores/0/2/disk.0,if=none,id=drive-virtio-
  disk0,format=raw,cache=none -device virtio-blk-
  pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-
  disk0,bootindex=1 -drive
  file=/var/lib/one//datastores/0/2/disk.1,if=none,id=drive-
  ide0-0-0,readonly=on,format=raw -device ide-cd,bus=ide.0,unit=0,drive
  =drive-ide0-0-0,id=ide0-0-0 -netdev
  tap,fd=23,id=hostnet0,vhost=on,vhostfd=25 -device virtio-net-
  pci,netdev=hostnet0,id=net0,mac=02:00:0a:64:02:fe,bus=pci.0,addr=0x3
  -vnc 0.0.0.0:2,password -vga cirrus -incoming tcp:0.0.0.0:49155
  -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5

  Disk backend is LVM running on SAN via FC connection (using symlink
  from /var/lib/one/datastores/0/2/disk.0 above)

  
  ubuntu-12.04 - first boot
  ==
  Simple syscall: 0.0527 microseconds
  Simple read: 0.1143 microseconds
  Simple write: 0.0953 microseconds
  Simple open/close: 1.0432 microseconds

  Using phoronix pts/compuation

Re: [Qemu-devel] [PATCH qom-cpu 2/9] target-i386/kvm.c: Code formatting changes

[Andreas Färber]

Am 22.04.2013 21:00, schrieb Eduardo Habkost:
> Add appropriate spaces around operators, and break line where it needs
> to be broken to allow feature-words array to be introduced without
> having too-long lines.
> 
> Signed-off-by: Eduardo Habkost 
> Reviewed-By: Igor Mammedov 
> ---
> Changes v9:
>  * 1-char alignment change: keep the opening parenthesis of both sides
>of the "==" operator starting at the same column
> ---
>  target-i386/kvm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 0e7cc81..b5bff33 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -613,7 +613,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
>  cpuid_data.cpuid.nent = cpuid_i;
>  
>  if (((env->cpuid_version >> 8)&0xF) >= 6
> -&& (env->cpuid_features&(CPUID_MCE|CPUID_MCA)) == 
> (CPUID_MCE|CPUID_MCA)
> +&& (env->cpuid_features & (CPUID_MCE|CPUID_MCA)) ==
> +   (CPUID_MCE|CPUID_MCA)

I've added spaces around the | operator as well. :)

Andreas

>  && kvm_check_extension(cs->kvm_state, KVM_CAP_MCE) > 0) {
>  uint64_t mcg_cap;
>  int banks;
> 


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

Re: [Qemu-devel] [PATCH qom-cpu 0/9] x86: feature words array (v11) + "feature-words" property

[Andreas Färber]

Am 22.04.2013 21:00, schrieb Eduardo Habkost:
> This series includes the previous "replace cpuid_*features fields with a 
> feature
> word array" series.
> 
> The first 4 patches already have a Reviewed-by from Igor, they correspond to 
> v10
> plus a small indent fix requested by him.
> 
> As the cpuid_*features series was holding my 
> "feature-words"/"filtered-features"
> series (previously sent as RFC), I am now sending both as a single series, to
> try to get "feature-words"/"filtered-features" some attention and try to get 
> it
> included in 1.5.
> 
> The "feature-words"/"filtered-features" mechanism is very important for 
> libvirt,
> to allow it to ensure the guest is getting the required set of CPU features, 
> as
> configured by the user.
> 
> 
> Eduardo Habkost (9):
>   target-i386: cleanup: Group together level, xlevel, xlevel2 fields
>   target-i386/kvm.c: Code formatting changes
>   target-i386/cpu.c: Break lines so they don't get too long
>   target-i386: Replace cpuid_*features fields with a feature word array

Thanks, applied these to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

These had been around for quite some time and I have reviewed the first
three line by line; for the fourth I have looked at the script sources
and trust Igor's review. Thanks for repeatedly rebasing this, Eduardo!


>   target-i386: Add ECX information to FeatureWordInfo

This one is lacking Reviewed-by / Acked-by ...

>   target-i386: Add "feature-words" property
>   target-i386: Use FeatureWord loop on filter_features_for_kvm()

... and this one seems to depend on it.

>   target-i386: Introduce X86CPU.filtered_features field
>   target-i386: Add "filtered-features" property to X86CPU

As mentioned earlier I'd prefer to defer the property design rather than
putting it lightly reviewed into 1.5 and living with some ABI.
If libvirt urgently needs this info, this series needs to be reviewed
and sorted out until the weekend (Hard Freeze on Monday).

Regards,
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] [Bug 1175089] [NEW] Crash why dragon fly 3.4.1

[Venkatesh Srinivas]

On Wed, May 1, 2013 at 1:29 AM, BRULE Herman <1175...@bugs.launchpad.net> wrote:
> Public bug reported:
>
> Hello, all is here (kernel 3.8, qemu 1.2.2-r3):
> /usr/bin/qemu-system-x86_64 -k fr -alt-grab -m 2048 -vga vmware -net 
> nic,vlan=0,model=virtio -net user -rtc base=localtime -smp 
> 4,cores=4,sockets=1 -boot once=d -cdrom dfly-x86_64-gui-3.4.1_REL.iso
> KVM internal error. Suberror: 1
> emulation failure
> EAX=0010 EBX=9338 ECX= EDX=
> ESI=17fc EDI=17c8 EBP=000364a0 ESP=17b8
> EIP=9318 EFL=3002 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0
> ES =0010   00c09300
> CS =0018   9b00
> SS =0010   00c09300
> DS =0010   00c09300
> FS =0033 a000  00c0f300
> GS =0033 a000  00c0f300
> LDT=   8200
> TR =0038 5f98 2067 8b00
> GDT= 9590 003f
> IDT= 5e00 0197
> CR0=0010 CR2= CR3= CR4=
> DR0= DR1= DR2= 
> DR3=
> DR6=0ff0 DR7=0400
> EFER=
> Code=00 a3 ea 5d 00 00 66 ea 10 93 18 00 0f 20 c0 fe c8 0f 22 c0  1d 93 
> 00 00 31 c0 8e d8 8e d0 0f 01 1e dc 95 66 07 66 1f 66 0f a1 66 0f a9 66 61 bc 
> ea
>
> ** Affects: qemu
>  Importance: Undecided
>  Status: New
>

While I have nothing to offer on the bug, DragonFly BSD 3.4 doesn't
have guest drivers for virtio-net; you may want to use e1000 as your
paravirtualized NIC. (re: virtio-net -- I worked on Tim Bisson's port
of FreeBSD's vtnet driver, but when I last worked on it, it was slower
in both TCP_STREAM and TCP_RR than e1000, when connected to TAP; so
there was work left to do...)

-- vs;

[Qemu-devel] [PATCH] PPC: Add MMU type for 2.06 with AMR but no TB pages

[Alexander Graf]

When running -cpu on a POWER7 system with PR KVM, we mask out the 1TB
MMU capability from the MMU type mask, but not the AMR bit.

This leads to us having a new MMU type that we don't check for in our
MMU management functions.

Add the new type, so that we don't have to worry about breakage there.
We're not going to use the TCG MMU management in that case anyway.

The long term fix for this will be to move all these MMU management
functions to class callbacks.

Signed-off-by: Alexander Graf 
---
 target-ppc/cpu.h| 3 +++
 target-ppc/mmu_helper.c | 4 
 2 files changed, 7 insertions(+)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 7cacb56..aa1d013 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -119,6 +119,9 @@ enum powerpc_mmu_t {
 /* Architecture 2.06 variant   */
 POWERPC_MMU_2_06   = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
  | POWERPC_MMU_AMR | 0x0003,
+/* Architecture 2.06 "degraded" (no 1T segments)   */
+POWERPC_MMU_2_06a  = POWERPC_MMU_64 | POWERPC_MMU_AMR
+ | 0x0003,
 /* Architecture 2.06 "degraded" (no 1T segments or AMR)*/
 POWERPC_MMU_2_06d  = POWERPC_MMU_64 | 0x0003,
 #endif /* defined(TARGET_PPC64) */
diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
index acf0133..68d5415 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -1188,6 +1188,7 @@ void dump_mmu(FILE *f, fprintf_function cpu_fprintf, 
CPUPPCState *env)
 #if defined(TARGET_PPC64)
 case POWERPC_MMU_64B:
 case POWERPC_MMU_2_06:
+case POWERPC_MMU_2_06a:
 case POWERPC_MMU_2_06d:
 dump_slb(f, cpu_fprintf, env);
 break;
@@ -1324,6 +1325,7 @@ hwaddr cpu_get_phys_page_debug(CPUPPCState *env, 
target_ulong addr)
 #if defined(TARGET_PPC64)
 case POWERPC_MMU_64B:
 case POWERPC_MMU_2_06:
+case POWERPC_MMU_2_06a:
 case POWERPC_MMU_2_06d:
 return ppc_hash64_get_phys_page_debug(env, addr);
 #endif
@@ -1815,6 +1817,7 @@ void ppc_tlb_invalidate_all(CPUPPCState *env)
 #if defined(TARGET_PPC64)
 case POWERPC_MMU_64B:
 case POWERPC_MMU_2_06:
+case POWERPC_MMU_2_06a:
 case POWERPC_MMU_2_06d:
 #endif /* defined(TARGET_PPC64) */
 tlb_flush(env, 1);
@@ -1884,6 +1887,7 @@ void ppc_tlb_invalidate_one(CPUPPCState *env, 
target_ulong addr)
 #if defined(TARGET_PPC64)
 case POWERPC_MMU_64B:
 case POWERPC_MMU_2_06:
+case POWERPC_MMU_2_06a:
 case POWERPC_MMU_2_06d:
 /* tlbie invalidate TLBs for all segments */
 /* XXX: given the fact that there are too many segments to invalidate,
-- 
1.8.1.4

[Qemu-devel] [Bug 1175089] [NEW] Crash why dragon fly 3.4.1

[BRULE Herman]

Public bug reported:

Hello, all is here (kernel 3.8, qemu 1.2.2-r3):
/usr/bin/qemu-system-x86_64 -k fr -alt-grab -m 2048 -vga vmware -net 
nic,vlan=0,model=virtio -net user -rtc base=localtime -smp 4,cores=4,sockets=1 
-boot once=d -cdrom dfly-x86_64-gui-3.4.1_REL.iso 
KVM internal error. Suberror: 1
emulation failure
EAX=0010 EBX=9338 ECX= EDX=
ESI=17fc EDI=17c8 EBP=000364a0 ESP=17b8
EIP=9318 EFL=3002 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =0010   00c09300
CS =0018   9b00
SS =0010   00c09300
DS =0010   00c09300
FS =0033 a000  00c0f300
GS =0033 a000  00c0f300
LDT=   8200
TR =0038 5f98 2067 8b00
GDT= 9590 003f
IDT= 5e00 0197
CR0=0010 CR2= CR3= CR4=
DR0= DR1= DR2= 
DR3= 
DR6=0ff0 DR7=0400
EFER=
Code=00 a3 ea 5d 00 00 66 ea 10 93 18 00 0f 20 c0 fe c8 0f 22 c0  1d 93 00 
00 31 c0 8e d8 8e d0 0f 01 1e dc 95 66 07 66 1f 66 0f a1 66 0f a9 66 61 bc ea

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  Crash why dragon fly 3.4.1

Status in QEMU:
  New

Bug description:
  Hello, all is here (kernel 3.8, qemu 1.2.2-r3):
  /usr/bin/qemu-system-x86_64 -k fr -alt-grab -m 2048 -vga vmware -net 
nic,vlan=0,model=virtio -net user -rtc base=localtime -smp 4,cores=4,sockets=1 
-boot once=d -cdrom dfly-x86_64-gui-3.4.1_REL.iso 
  KVM internal error. Suberror: 1
  emulation failure
  EAX=0010 EBX=9338 ECX= EDX=
  ESI=17fc EDI=17c8 EBP=000364a0 ESP=17b8
  EIP=9318 EFL=3002 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0
  ES =0010   00c09300
  CS =0018   9b00
  SS =0010   00c09300
  DS =0010   00c09300
  FS =0033 a000  00c0f300
  GS =0033 a000  00c0f300
  LDT=   8200
  TR =0038 5f98 2067 8b00
  GDT= 9590 003f
  IDT= 5e00 0197
  CR0=0010 CR2= CR3= CR4=
  DR0= DR1= DR2= 
DR3= 
  DR6=0ff0 DR7=0400
  EFER=
  Code=00 a3 ea 5d 00 00 66 ea 10 93 18 00 0f 20 c0 fe c8 0f 22 c0  1d 93 
00 00 31 c0 8e d8 8e d0 0f 01 1e dc 95 66 07 66 1f 66 0f a1 66 0f a9 66 61 bc ea

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

Re: [Qemu-devel] [PATCH 2/2] Add i.MX25 3DS evaluation board support

[Jean-Christophe DUBOIS]

Peter,

I have submitted a few patch to add support to the i.MX25 processor and 
the 3DS Freescale evaluation platform.


Most of the i.MX devices present on i.MX25 can be reused on other 
Freescale processors including the i.MX6.


So far I did not get that much feedback on this.

Is this of any interest to somebody?

If so, what it the process to get it adopted in an official tree.

Thanks

JC


On 04/26/2013 04:02 PM, Jean-Christophe DUBOIS wrote:

This is an initial port of the Freescale i.MX25 processor.

This allow a minimally configured linux kernel to boot on Qemu.

It also handle the newly added FEC ethernet device.

Signed-off-by: Jean-Christophe DUBOIS 
---
  hw/arm/Makefile.objs |   1 +
  hw/arm/imx25_3ds.c   | 218 +++
  2 files changed, 219 insertions(+)
  create mode 100644 hw/arm/imx25_3ds.c

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 9e3a06f..2f4280d 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -2,6 +2,7 @@ obj-y += boot.o collie.o exynos4_boards.o gumstix.o highbank.o
  obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
  obj-y += omap_sx1.o palm.o pic_cpu.o realview.o spitz.o stellaris.o
  obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o
+obj-y += imx25_3ds.o
  
  obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o

  obj-y += omap1.o omap2.o strongarm.o
diff --git a/hw/arm/imx25_3ds.c b/hw/arm/imx25_3ds.c
new file mode 100644
index 000..9de4941
--- /dev/null
+++ b/hw/arm/imx25_3ds.c
@@ -0,0 +1,218 @@
+/*
+ * Copyright (c) 2013 Jean-Christophe Dubois
+ *
+ * 3Dstack Board System emulation.
+ *
+ * Based on hw/arm/kzm.c
+ *
+ * Copyright (c) 2008 OKL and 2011 NICTA
+ * Written by Hans at OK-Labs
+ * Updated by Peter Chubb.
+ *
+ * This code is licensed under the GPL, version 2 or later.
+ * See the file `COPYING' in the top level directory.
+ *
+ * It (partially) emulates a i.MX25 3D-Stack PDK board
+ */
+
+#include "hw/sysbus.h"
+#include "exec/address-spaces.h"
+#include "hw/hw.h"
+#include "hw/arm/arm.h"
+#include "hw/devices.h"
+#include "net/net.h"
+#include "sysemu/sysemu.h"
+#include "hw/boards.h"
+#include "hw/char/serial.h"
+#include "hw/arm/imx.h"
+
+/* Memory map for 3D-Stack Emulation Baseboard:
+ * 0x-0x3fff 16k ROM  IGNORED
+ * 0x4000-0x00403fff Reserved IGNORED
+ * 0x00404000-0x00408fff 20k ROM  IGNORED
+ * 0x00409000-0x0fff Reserved IGNORED
+ * 0x1000-0x1fff Reserved IGNORED
+ * 0x2000-0x2fff Reserved IGNORED
+ * 0x3000-0x3fff Reserved IGNORED
+ * 0x4000-0x43ef Reserved IGNORED
+ * 0x43f0-0x6fff I.MX25 Internal Register Space
+ *   0x43f0 IO_AREA0
+ *   0x43f9 UART1 EMULATED
+ *   0x43f94000 UART2 EMULATED
+ *   0x43fb UART4 IGNORED
+ *   0x43fb4000 UART5 IGNORED
+ *   0x5000c000 UART3 IGNORED
+ *   0x53f8 CCM   EMULATED
+ *   0x53f84000 GPT 4 EMULATED
+ *   0x53f88000 GPT 3 EMULATED
+ *   0x53f8c000 GPT 2 EMULATED
+ *   0x53f9 GPT 1 EMULATED
+ *   0x53f94000 PIT 1 EMULATED
+ *   0x53f98000 PIT 2 EMULATED
+ *   0x6800 ASIC  EMULATED
+ * 0x7800-0x7801 SRAM EMULATED
+ * 0x7802-0x7fff SRAM AliasingEMULATED
+ * 0x8000-0x87ff RAM + Alias  EMULATED
+ * 0x9000-0x9fff RAM + Alias  EMULATED
+ * 0xa000-0xa7ff FlashIGNORED
+ * 0xa800-0xafff FlashIGNORED
+ * 0xb000-0xb1ff SRAM IGNORED
+ * 0xb200-0xb3ff SRAM IGNORED
+ * 0xb400-0xb5ff CS4  IGNORED
+ * 0xb600-0xb8000fff Reserved IGNORED
+ * 0xb8001000-0xb8001fff SDRAM CTRL reg   IGNORED
+ * 0xb8002000-0xb8002fff WEIM CTRL regIGNORED
+ * 0xb8003000-0xb8003fff M3IF CTRL regIGNORED
+ * 0xb8004000-0xb8004fff EMI CTRL reg IGNORED
+ * 0xb8005000-0xbaff Reserved IGNORED
+ * 0xbb00-0xbb000fff NAND flash area buf  IGNORED
+ * 0xbb001000-0xbb0011ff NAND flash reserved  IGNORED
+ * 0xbb001200-0xbb001dff Reserved IGNORED
+ * 0xbb001e00-0xbb001fff NAN flash CTRL reg   IGNORED
+ * 0xbb012000-0xbfff Reserved IGNORED
+ * 0xc000-0x Reserved IGNORED
+ */
+
+#define IMX25_SRAM_ADDRESS  (0x7800)
+#define IMX25_SRAMSIZE  (128*1024)
+#define IMX25_CS_SRAMSIZE   (128*1024*1024)
+#define IMX25_3DS_ADDRESS   (0x8000)
+#define IMX25_CS_RAMSIZE(256*1024*1024)
+
+static struct arm_boot_info imx25_3ds_binfo = {
+.loader_start = IMX25_3

[Qemu-devel] [PATCH 2/2] Add i.MX device to i.MX25 3DS platform.

[Jean-Christophe DUBOIS]

Signed-off-by: Jean-Christophe DUBOIS 
---
 hw/arm/imx25_3ds.c | 48 
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/hw/arm/imx25_3ds.c b/hw/arm/imx25_3ds.c
index 9de4941..aba9279 100644
--- a/hw/arm/imx25_3ds.c
+++ b/hw/arm/imx25_3ds.c
@@ -25,6 +25,7 @@
 #include "hw/boards.h"
 #include "hw/char/serial.h"
 #include "hw/arm/imx.h"
+#include "hw/i2c/i2c.h"
 
 /* Memory map for 3D-Stack Emulation Baseboard:
  * 0x-0x3fff 16k ROM  IGNORED
@@ -37,6 +38,9 @@
  * 0x4000-0x43ef Reserved IGNORED
  * 0x43f0-0x6fff I.MX25 Internal Register Space
  *   0x43f0 IO_AREA0
+ *   0x43f8 I2C0  EMULATED
+ *   0x43f84000 I2C2  EMULATED
+ *   0x43f98000 I2C1  EMULATED
  *   0x43f9 UART1 EMULATED
  *   0x43f94000 UART2 EMULATED
  *   0x43fb UART4 IGNORED
@@ -95,12 +99,11 @@ static void imx25_3ds_init(QEMUMachineInitArgs *args)
 ARMCPU *cpu;
 MemoryRegion *address_space_mem = get_system_memory();
 qemu_irq *cpu_pic;
-DeviceState *dev;
+DeviceState *pic_dev;
 DeviceState *ccm;
 MemoryRegion *sram = g_new(MemoryRegion, 1);
 MemoryRegion *sram_alias = g_new(MemoryRegion, 1);
 
-
 if (!cpu_model) {
 cpu_model = "arm926";
 }
@@ -169,31 +172,44 @@ static void imx25_3ds_init(QEMUMachineInitArgs *args)
 
 /* add the PIC */
 cpu_pic = arm_pic_init_cpu(cpu);
-dev = sysbus_create_varargs("imx_avic", 0x6800,
+pic_dev = sysbus_create_varargs("imx_avic", 0x6800,
 cpu_pic[ARM_PIC_CPU_IRQ],
 cpu_pic[ARM_PIC_CPU_FIQ], NULL);
 
 /* add some serial lines */
-imx_serial_create(0, 0x43f9, qdev_get_gpio_in(dev, 45));
-imx_serial_create(1, 0x43f94000, qdev_get_gpio_in(dev, 32));
+imx_serial_create(0, 0x43f9, qdev_get_gpio_in(pic_dev, 45));
+imx_serial_create(1, 0x43f94000, qdev_get_gpio_in(pic_dev, 32));
 // Qemu does not support more than 4 serial ports. Too bad.
-//imx_serial_create(3, 0x5000c000, qdev_get_gpio_in(dev, 18));
-//imx_serial_create(4, 0x43fb, qdev_get_gpio_in(dev, 46));
-//imx_serial_create(5, 0x43fb4000, qdev_get_gpio_in(dev, 47));
+//imx_serial_create(3, 0x5000c000, qdev_get_gpio_in(pic_dev, 18));
+//imx_serial_create(4, 0x43fb, qdev_get_gpio_in(pic_dev, 46));
+//imx_serial_create(5, 0x43fb4000, qdev_get_gpio_in(pic_dev, 47));
 
 ccm = sysbus_create_simple("imx_ccm", 0x53f8, NULL);
 
 /* add gpt timers */
-imx_timerg_create(0x53f84000, qdev_get_gpio_in(dev, 1), ccm);
-imx_timerg_create(0x53f88000, qdev_get_gpio_in(dev, 29), ccm);
-imx_timerg_create(0x53f8c000, qdev_get_gpio_in(dev, 53), ccm);
-imx_timerg_create(0x53f9, qdev_get_gpio_in(dev, 54), ccm);
+imx_timerg_create(0x53f84000, qdev_get_gpio_in(pic_dev, 1), ccm);
+imx_timerg_create(0x53f88000, qdev_get_gpio_in(pic_dev, 29), ccm);
+imx_timerg_create(0x53f8c000, qdev_get_gpio_in(pic_dev, 53), ccm);
+imx_timerg_create(0x53f9, qdev_get_gpio_in(pic_dev, 54), ccm);
 
 /* add epit timers */
-imx_timerp_create(0x53f94000, qdev_get_gpio_in(dev, 28), ccm);
-imx_timerp_create(0x53f98000, qdev_get_gpio_in(dev, 27), ccm);
-
-imx_fec_create(0, 0x50038000, qdev_get_gpio_in(dev, 57));
+imx_timerp_create(0x53f94000, qdev_get_gpio_in(pic_dev, 28), ccm);
+imx_timerp_create(0x53f98000, qdev_get_gpio_in(pic_dev, 27), ccm);
+
+imx_fec_create(0, 0x50038000, qdev_get_gpio_in(pic_dev, 57));
+
+/*** I2C ***/
+for (i = 0; i < 3; i++) {
+static uint32_t addr[] = {0x43F8, 0x43F98000, 0x43F84000};
+static uint32_t irq[]  = {3, 4, 10};
+SysBusDevice *busdev;
+DeviceState *i2c_dev = qdev_create(NULL, "imx.i2c");
+
+qdev_init_nofail(i2c_dev);
+busdev = SYS_BUS_DEVICE(i2c_dev);
+sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(pic_dev, irq[i]));
+sysbus_mmio_map(busdev, 0, addr[i]);
+}
 
 imx25_3ds_binfo.ram_size = ram_size;
 imx25_3ds_binfo.kernel_filename = kernel_filename;
-- 
1.8.1.2

[Qemu-devel] [PATCH 1/2] Add i.MX I2C device emulator.

[Jean-Christophe DUBOIS]

Signed-off-by: Jean-Christophe DUBOIS 
---
 default-configs/arm-softmmu.mak |   2 +
 hw/i2c/Makefile.objs|   1 +
 hw/i2c/imx_i2c.c| 374 
 3 files changed, 377 insertions(+)
 create mode 100644 hw/i2c/imx_i2c.c

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index b3a0207..a20f112 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -81,3 +81,5 @@ CONFIG_VERSATILE_PCI=y
 CONFIG_VERSATILE_I2C=y
 
 CONFIG_SDHCI=y
+
+CONFIG_IMX_I2C=y
diff --git a/hw/i2c/Makefile.objs b/hw/i2c/Makefile.objs
index 648278e..d27bbaa 100644
--- a/hw/i2c/Makefile.objs
+++ b/hw/i2c/Makefile.objs
@@ -4,4 +4,5 @@ common-obj-$(CONFIG_ACPI) += smbus_ich9.o
 common-obj-$(CONFIG_APM) += pm_smbus.o
 common-obj-$(CONFIG_BITBANG_I2C) += bitbang_i2c.o
 common-obj-$(CONFIG_EXYNOS4) += exynos4210_i2c.o
+common-obj-$(CONFIG_IMX_I2C) += imx_i2c.o
 obj-$(CONFIG_OMAP) += omap_i2c.o
diff --git a/hw/i2c/imx_i2c.c b/hw/i2c/imx_i2c.c
new file mode 100644
index 000..30d3f5c
--- /dev/null
+++ b/hw/i2c/imx_i2c.c
@@ -0,0 +1,374 @@
+/*
+ *  i.MX I2C Bus Serial Interface Emulation
+ *
+ *  Copyright (C) 2013 Jean-Christophe Dubois.
+ *
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under the terms of the GNU General Public License as published by the
+ *  Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful, but WITHOUT
+ *  ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ *  FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ *  for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, see .
+ *
+ */
+
+#include "hw/sysbus.h"
+#include "hw/i2c/i2c.h"
+
+#ifndef IMX_I2C_DEBUG
+#define IMX_I2C_DEBUG 0
+#endif
+
+#define TYPE_IMX_I2C  "imx.i2c"
+#define IMX_I2C(obj)  \
+OBJECT_CHECK(imx_i2c_state, (obj), TYPE_IMX_I2C)
+
+/* i.MX I2C memory map */
+#define IMX_I2C_MEM_SIZE   0x14
+#define IADR_ADDR  0x00  /* address register */
+#define IFDR_ADDR  0x04  /* frequency divider register */
+#define I2CR_ADDR  0x08  /* control register */
+#define I2SR_ADDR  0x0c  /* status register */
+#define I2DR_ADDR  0x10  /* data register */
+
+#define IADR_MASK  0xFE
+#define IADR_RESET 0
+
+#define IFDR_MASK  0x3F
+#define IFDR_RESET 0
+
+#define I2CR_IEN   (1 << 7)
+#define I2CR_IIEN  (1 << 6)
+#define I2CR_MSTA  (1 << 5)
+#define I2CR_MTX   (1 << 4)
+#define I2CR_TXAK  (1 << 3)
+#define I2CR_RSTA  (1 << 2)
+#define I2CR_MASK  0xFC
+#define I2CR_RESET 0
+
+#define I2SR_ICF   (1 << 7)
+#define I2SR_IAAF  (1 << 6)
+#define I2SR_IBB   (1 << 5)
+#define I2SR_IAL   (1 << 4)
+#define I2SR_SRW   (1 << 2)
+#define I2SR_IIF   (1 << 1)
+#define I2SR_RXAK  (1 << 0)
+#define I2SR_MASK  0xE9
+#define I2SR_RESET 0x81
+
+#define I2DR_MASK  0xFF
+#define I2DR_RESET 0
+
+#define ADDR_RESET 0xFF00
+
+#if IMX_I2C_DEBUG
+#define DPRINT(fmt, args...)  \
+do { fprintf(stderr, "%s: "fmt, __func__, ## args); } while (0)
+
+static const char *imx_i2c_get_regname(unsigned offset)
+{
+switch (offset) {
+case IADR_ADDR:
+return "IADR";
+case IFDR_ADDR:
+return "IFDR";
+case I2CR_ADDR:
+return "I2CR";
+case I2SR_ADDR:
+return "I2SR";
+case I2DR_ADDR:
+return "I2DR";
+default:
+return "[?]";
+}
+}
+
+#else
+#define DPRINT(fmt, args...)  do { } while (0)
+#endif
+
+typedef struct imx_i2c_state {
+SysBusDevice busdev;
+MemoryRegion iomem;
+i2c_bus *bus;
+qemu_irq irq;
+
+uint16_t  address;
+
+uint16_t iadr;
+uint16_t ifdr;
+uint16_t i2cr;
+uint16_t i2sr;
+uint16_t i2dr_read;
+uint16_t i2dr_write;
+} imx_i2c_state;
+
+static inline bool imx_i2c_is_enabled(imx_i2c_state *s)
+{
+return (s->i2cr & I2CR_IEN);
+}
+
+static inline bool imx_i2c_interrupt_is_enabled(imx_i2c_state *s)
+{
+return (s->i2cr & I2CR_IIEN);
+}
+
+static inline bool imx_i2c_is_master(imx_i2c_state *s)
+{
+return (s->i2cr & I2CR_MSTA);
+}
+
+static inline bool imx_i2c_direction_is_tx(imx_i2c_state *s)
+{
+return (s->i2cr & I2CR_MTX);
+}
+
+static void imx_i2c_reset(DeviceState *d)
+{
+imx_i2c_state *s = FROM_SYSBUS(imx_i2c_stat

Re: [Qemu-devel] [PATCH 5/5] target-i386: implement machine->hot_add_cpu hook

[Andreas Färber]

Am 30.04.2013 18:00, schrieb Igor Mammedov:
> Signed-off-by: Igor Mammedov 
> ---
> v3:
>   * use local static variable for saving cpu_model
> 
> v2:
>   * override .hot_add_cpu statically starting with 1.5 machine

Thanks, applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

I verified it's working as expected in Windows 2012 Datacenter, but
neither in openSUSE 12.3 nor in Fedora 18 Live DVD did I see any change
in /proc/cpuinfo or GNOME System Monitor.

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] Continuous work on sandboxing

[Corey Bryant]

On 05/01/2013 01:25 PM, Eduardo Otubo wrote:



On 04/30/2013 12:24 PM, Paul Moore wrote:

On Monday, April 29, 2013 05:52:10 PM Corey Bryant wrote:

On 04/26/2013 05:07 PM, Paul Moore wrote:

[snip]


3. Debugging and/or learning mode - third party libraries still
have the
problem of interfering in the Qemu's signal mask. According to some
previous discussions, perhaps patch all external libraries that
mass up
with this mask (spice, for example) is a way to solve it. But not
sure
if it worth the time spent. Would like to hear you guys.


I think patching all the libraries is a losing battle, I think we
need to
pursue alternate debugging techniques.


I agree.  It would be nice to have some sort of learning mode that
reported all denied syscalls on a single run, but signal handlers
doesn't seem like the right way.  Maybe we could improve on this
approach, since it never gained traction:
https://lkml.org/lkml/2013/1/7/313

At least we can get a single denied syscall at a time today via the
audit log that the kernel issues.  Eduardo, you may want to see if
there's a good place to document that for QEMU so that people know where
to look.


Lately I've been using the fact that the seccomp BPF filter result
generates
an audit log; it either dumps to syslog or the audit log (depending on
your
configuration) and seems to accomplish most of what we wanted with
SECCOMP_RET_INFO.

I'm always open to new/better ideas, but this has been working
reasonably well
for me for the past few months.


I think this feature would fits well on Qemu if we could have a "normal"
signal handling. But external libraries interfere a lot on this matter.

Paolo, am I the first one to complain about signal handling on Qemu
(being interfered by other libraries)? I believe this may cause some
trouble in other parts of the project as well. Wouldn't be this a good
time to, perhaps, just think about a signal handling refactoring?



You don't need signal handling to use what Paul was talking about above. 
 I think that should be enough for -sandbox purposes, but perhaps you 
could document it somewhere for QEMU.


--
Regards,
Corey Bryant

Re: [Qemu-devel] [PATCH] softfloat: rebase to version 2a

[Blue Swirl]

On Wed, May 1, 2013 at 5:57 PM, Peter Maydell  wrote:
> On 1 May 2013 18:53, Blue Swirl  wrote:
>> On Mon, Apr 29, 2013 at 6:05 PM, Anthony Liguori  wrote:
>>> d07cca0 Add native softfloat fpu functions (Christoph Egger)
>
>> d07cca0 was supplied by Christoph Egger (cc'd):
>> http://lists.nongnu.org/archive/html/qemu-devel/2008-11/msg00939.html
>
> As it happens the only fpu file that patch touches is
> the now-deleted softfloat-native.h, so I think it's ok
> anyway?

Right, that should be fine too.

>
> -- PMM

Re: [Qemu-devel] [PATCH] softfloat: rebase to version 2a

[Peter Maydell]

On 1 May 2013 18:53, Blue Swirl  wrote:
> On Mon, Apr 29, 2013 at 6:05 PM, Anthony Liguori  wrote:
>> d07cca0 Add native softfloat fpu functions (Christoph Egger)

> d07cca0 was supplied by Christoph Egger (cc'd):
> http://lists.nongnu.org/archive/html/qemu-devel/2008-11/msg00939.html

As it happens the only fpu file that patch touches is
the now-deleted softfloat-native.h, so I think it's ok
anyway?

-- PMM

Re: [Qemu-devel] [PATCH] translate: remove redundantly included qemu/timer.h

[Peter Maydell]

On 1 May 2013 18:09, Michael Tokarev  wrote:
> Also, Peter, does your followup email mean your Rewieved-by
> should be added too?

As Andreas says, you don't add R-b: tags unless they
were explicitly given. However in this case I think I
just forgot to write it, so here you are:

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [Qemu-devel] [RFC] Moving Hard Freeze to Monday, May 6th.

[Peter Maydell]

On 30 April 2013 19:36, Anthony Liguori  wrote:
>
> The current release schedule has hard freeze happening tomorrow.  There
> are a few things still outstanding including cpu hotplug and updating
> SeaBIOS.  We still need to resolve how to handle the softfloat code
> too.  I am particularly interested in this last one for the 1.5 release.
>
> Given this, I'd like to delay the Hard Freeze until Monday and slip the
> 1.5 release by two days to the 17th to ensure we have enough -rcs..

Fine by me, since I, er, forgot hardfreeze would be today :-)

-- PMM

Re: [Qemu-devel] [PATCH] translate: remove redundantly included qemu/timer.h

[Andreas Färber]

Am 01.05.2013 19:09, schrieb Michael Tokarev:
> 30.04.2013 06:59, liguang wrote:
>> Signed-off-by: liguang 
>> ---
>>  translate-all.c |1 -
>>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> Hmm.  Does we require S-o-b line even for such a trivial (but nevertheless 
> important) stuff?

Yes: http://wiki.qemu.org/Contribute/SubmitAPatch

If it's missing, author can either reply with Signed-off-by - if you're
willing to fix it up - or resend.

And whenever you pick up patches, you sign them off as well.

> Also, Peter, does your followup email mean your Rewieved-by should be added 
> too?

No, generally not if it was not explicitly given. Exception from the
rule: We have allowed to turn an informal "ack" into an Acked-by.

Thanks for taking over qemu-trivial!

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] Continuous work on sandboxing

[Eduardo Otubo]

On 04/30/2013 12:24 PM, Paul Moore wrote:

On Monday, April 29, 2013 05:52:10 PM Corey Bryant wrote:

On 04/26/2013 05:07 PM, Paul Moore wrote:

[snip]


3. Debugging and/or learning mode - third party libraries still have the
problem of interfering in the Qemu's signal mask. According to some
previous discussions, perhaps patch all external libraries that mass up
with this mask (spice, for example) is a way to solve it. But not sure
if it worth the time spent. Would like to hear you guys.


I think patching all the libraries is a losing battle, I think we need to
pursue alternate debugging techniques.


I agree.  It would be nice to have some sort of learning mode that
reported all denied syscalls on a single run, but signal handlers
doesn't seem like the right way.  Maybe we could improve on this
approach, since it never gained traction: https://lkml.org/lkml/2013/1/7/313

At least we can get a single denied syscall at a time today via the
audit log that the kernel issues.  Eduardo, you may want to see if
there's a good place to document that for QEMU so that people know where
to look.


Lately I've been using the fact that the seccomp BPF filter result generates
an audit log; it either dumps to syslog or the audit log (depending on your
configuration) and seems to accomplish most of what we wanted with
SECCOMP_RET_INFO.

I'm always open to new/better ideas, but this has been working reasonably well
for me for the past few months.


I think this feature would fits well on Qemu if we could have a "normal" 
signal handling. But external libraries interfere a lot on this matter.


Paolo, am I the first one to complain about signal handling on Qemu 
(being interfered by other libraries)? I believe this may cause some 
trouble in other parts of the project as well. Wouldn't be this a good 
time to, perhaps, just think about a signal handling refactoring?


Regards,

--
Eduardo Otubo
IBM Linux Technology Center

Re: [Qemu-devel] [PATCH] softfloat: rebase to version 2a

[Andreas Färber]

Am 29.04.2013 22:18, schrieb Peter Maydell:
> On 29 April 2013 19:53, Anthony Liguori  wrote:
>> Anthony Liguori  writes:
>>
>>> Thiemo Seufer 
[...]
>>>   5fafdf2 find -type f | xargs sed -i 's/[\t ]$//g' # on most files
>>>   63a654b trunc() for Solaris 9 / SPARC, by Juergen Keil.
>>>   fc81ba5 Check that HOST_SOLARIS is defined before relying on its
>>>   value. Spotted by Joachim Henke.
> 
> These three are all changes to files that have subsequently been
> deleted (the softfloat-native support was dropped altogether).

Further, ", by ..." used to indicate that the committer applied a patch
authored by someone else in pre-Git days. :) CC'ing Juergen Keil.

Regards,
Andreas

Full patch: http://patchwork.ozlabs.org/patch/240431/

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

Re: [Qemu-devel] [PATCH] translate: remove redundantly included qemu/timer.h

[Michael Tokarev]

30.04.2013 06:59, liguang wrote:
> Signed-off-by: liguang 
> ---
>  translate-all.c |1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)

Hmm.  Does we require S-o-b line even for such a trivial (but nevertheless 
important) stuff?
Also, Peter, does your followup email mean your Rewieved-by should be added too?

Confused,

/mjt

> diff --git a/translate-all.c b/translate-all.c
> index da93608..d04a116 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -55,7 +55,6 @@
>  #else
>  #include "exec/address-spaces.h"
>  #endif
> -#include "qemu/timer.h"
>  
>  #include "exec/cputlb.h"
>  #include "translate-all.h"
> 

Re: [Qemu-devel] [PATCH] softfloat: rebase to version 2a

[Andreas Färber]

Am 29.04.2013 20:05, schrieb Anthony Liguori:
> N.B. If you are on CC, see after the '---' for a requested action!
> 
> The license of SoftFloat-2b is claimed to be GPLv2 incompatible by
> the FSF due to an indemnification clause.  The previous release,
> SoftFloat-2a, did not contain this clause.  The only changes between
> these two versions as far as QEMU is concerned is the license change
> and a global modification of the comment structure.  This patch rebases
> our softfloat code to SoftFloat-2a in order to have a GPLv2 compatible
> license.
> 
> Please note, this is a comment-only change.  The resulting binary should
> be the same.
> 
> I created this patch using the following strategy:
> 
> 1) Create a branch using the original import of softfloat code:
>$ git checkout 158142c2c2df728cfa3b5320c65534921a764f26
> 
> 2) Remove carriage returns from Softfloat-2b
> 
> 3) Compare each of the softfloat files against Softfloat-2b using the
>following mapping to generate Fabrice's original softfloat changes:
> 
>- fpu/softfloat.c -> softfloat/bits64/softfloat.c
>- fpu/softfloat.h -> softfloat/bits64/386-Win32-gcc/softfloat.h
>- fpu/softfloat-macros.h -> softfloat/bits64/softfloat-macros
>- fpu/softfloat-specialize.h -> 
> softfloat/bits64/386-Win32-gcc/softfloat-specialize
> 
> 4) Replace our softfloat files with the corresponding files from Softfloat-2a
> 
> 5) Apply the diffs from (3) to (4) and commit
> 
> 6) Create a diff between (5) and 158142c2c2df728cfa3b5320c65534921a764f26
>- This diff consists 100% of licensing change + comment reformating
> 
> 7) Checkout the latest master branch, apply the diff from (6)
>- There were a lot of comment rejects, confirmed this was only comments
>  and then used an emacs macro to rewrite the comments to the Softfloat-2a
>  form.
> 
> Cc: Andreas Färber 
> Cc: Aurelien Jarno 
> Cc: Avi Kivity 
> Cc: Ben Taylor 
> Cc: Blue Swirl 
> Cc: Christophe Lyon 
> Cc: Fabrice Bellard 
> Cc: Guan Xuetao 
> Cc: Jocelyn Mayer 
> Cc: Juan Quintela 
> Cc: malc 
> Cc: Max Filippov 
> Cc: Paolo Bonzini 
> Cc: Paul Brook 
> Cc: Peter Maydell 
> Cc: Richard Henderson 
> Cc: Richard Sandiford 
> Cc: Stefan Weil 
> Cc: Thiemo Seufer 
> Signed-off-by: Anthony Liguori 
> ---
> In order to make this change, we need to relicense all contributions
> from initial import of the SoftFloat code to match the license of
> SoftFloat-2a (instead of the implied SoftFloat-2b license).
> 
> If you are on CC, it is because you have contributed to the softfloat
> code in QEMU.  Please response to this note with:
> 
> Acked-by: Your Name 
> 
> To significant that you are able and willing to relicense your changes
> to the SoftFloat-1a license (or a GPL compatible license).

Including my pre-SUSE contributions,

Acked-by: Andreas Färber 

for changing to SoftFloat-2a license.

Thanks for looking into this,

Andreas

> Please respond no later than May 6th, 2013.  If we are unable to confirm
> relicense from an author, changes from that author will be reverted.
> ---
> For completeness, here is the full listing of contributions:
> 
> Andreas Färber 
>   be45f06 Silence softfloat warnings on OpenSolaris
>   5aea4c5 softfloat: Replace uint16 type with uint_fast16_t
>   94a49d8 softfloat: Replace int16 type with int_fast16_t
>   c969654 softfloat: Fix mixups of int and int16
>   38641f8 softfloat: Use uint16 consistently
>   87b8cc3 softfloat: Resolve type mismatches between declaration and 
> implementation
>   8d725fa softfloat: Prepend QEMU-style header with derivation notice
>   9f8d2a0 softfloat: Use uint32 consistently
>   bb98fe4 softfloat: Drop [s]bits{8, 16, 32, 64} types in favor of 
> [u]int{8, 16, 32, 64}_t
> 
[snip]

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

Re: [Qemu-devel] [PATCH] softfloat: rebase to version 2a

[Anthony Liguori]

Aurelien Jarno  writes:

> On Mon, Apr 29, 2013 at 01:05:03PM -0500, Anthony Liguori wrote:
>> 7) Checkout the latest master branch, apply the diff from (6)
>>- There were a lot of comment rejects, confirmed this was only comments
>>  and then used an emacs macro to rewrite the comments to the Softfloat-2a
>>  form.
>
> I guess this last step is the reason why comments added long after the
> original code are also modified by your patch. Right?

Indeed.  This is also why I provided a detailed explanation of the
methodology :-)

>> In order to make this change, we need to relicense all contributions
>> from initial import of the SoftFloat code to match the license of
>> SoftFloat-2a (instead of the implied SoftFloat-2b license).
>> 
>> If you are on CC, it is because you have contributed to the softfloat
>> code in QEMU.  Please response to this note with:
>> 
>> Acked-by: Your Name 
>> 
>> To significant that you are able and willing to relicense your changes
>> to the SoftFloat-1a license (or a GPL compatible license).
>> 
>> Please respond no later than May 6th, 2013.  If we are unable to confirm
>> relicense from an author, changes from that author will be reverted.
>
> Acked-by: Aurelien Jarno 

Thanks to everyone that has responded so far!

Regards,

Anthony Liguori

>
> -- 
> Aurelien Jarno  GPG: 1024D/F1BCDB73
> aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH v3] Throttle-down guest when live migration does not converge.

[Paolo Bonzini]

Il 01/05/2013 18:34, Chegu Vinod ha scritto:
> On 5/1/2013 8:40 AM, Paolo Bonzini wrote:
>>> I shall make the suggested changes.
>>> Appreciate your review feedback on this part of the change.
> Hi Paolo.,
> 
> Thanks for taking a look (BTW, I accidentally left out the "RFC"  in the
> patch subject line...my bad!).
>> Hi Vinod,
>>
>> I think unfortunately it is not acceptable to make this patch work only
>> for KVM.  (It cannot work for Xen, but that's not a problem since Xen
>> uses a different migration mechanism; but it should work for TCG).
> 
> Ok. I hadn't yet looked at TCG aspects etc. Will follow up offline...

If we do it right with run_on_cpu, it should just work with TCG.

>> Unfortunately, as you noted the run_on_cpu callbacks currently run
>> under the big QEMU lock.  We need to fix that first.  We have time
>> for that during 1.6.
> 
> Ok.  Was under the impression that anytime a vcpu thread enters to do
> anything in qemu the BQL had to be held. So choose to go with
> run_on_cpu()  .  Will follow up offline on alternatives

run_on_cpu() is fine, but the problem is: 1) that run_on_cpu() is
synchronous, so the migration thread is paused too; 2) doing the usleep
directly in kvm_cpu_exec.

> "Holding" the vcpus in the host context (i.e. kvm module) itself is
> perhaps another way. Would need some handshakes (i.e. new ioctls ) with
> the kernel. Would that be acceptable way to proceed?

No, I think it's better to make the wait in userspace.  KVM could help
finding the CPUs to be stunned, since it is where the dirty bits are
computed.

Paolo

Re: [Qemu-devel] [Qemu-trivial] [PATCH for 1.5] pvscsi: fix compilation on 32 bit hosts

[Michael Tokarev]

01.05.2013 09:41, Hervé Poussineau wrote:
> This fixes the following error:
> In file included from qemu/include/trace.h:4:0,
>  from trace/generated-events.c:3:
> ./trace/generated-tracers.h: In function ‘trace_pvscsi_get_sg_list’:
> ./trace/generated-tracers.h:4271:9: error: format ‘%lu’ expects argument of
> type ‘long unsigned int’, but argument 4 has type ‘size_t’ [-Werror=format]
> 
> Signed-off-by: Hervé Poussineau 

Thanks, applied to qemu-trivial tree (6e860b5db4c76c66d7e02f93c9e22e0384bd3c6c).

/mjt

Re: [Qemu-devel] [Qemu-trivial] [PATCH] Trivial grammar and spelling fixes

[Michael Tokarev]

28.04.2013 13:49, Stefan Weil wrote:
> similiar -> similar
> recieve -> receive
> transfered -> transferred
> preperation -> preparation
> 
> Most changes are in comments, one modifies a parameter name in a function
> prototype.
> 
> The spelling fixes were made using codespell.
> 
> Signed-off-by: Stefan Weil 

Thanks, applied to qemu-trivial tree (805a25050272fe7cbbc2e9ce911612fc86c7b9a9)

/mjt

Re: [Qemu-devel] [Qemu-trivial] [PATCH for-1.5] configure: Pick up libseccomp include path

[Michael Tokarev]

28.04.2013 18:27, Andreas Färber wrote:
> openSUSE 12.3 has seccomp.h in /usr/include/libseccomp-1.0.1,
> so add `pkg-config --cflags libseccomp` output to QEMU_CFLAGS.
> 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Andreas Färber 

Thanks, applied to qemu-trivial tree (372e47e9b5e31c493823d7f512716644fb02d0fd).

/mjt

>  configure | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/configure b/configure
> index 21438d4..643153f 100755
> --- a/configure
> +++ b/configure
> @@ -1505,6 +1505,7 @@ libs_softmmu="$libs_softmmu -lz"
>  if test "$seccomp" != "no" ; then
>  if $pkg_config --atleast-version=1.0.0 libseccomp --modversion 
> >/dev/null 2>&1; then
>  libs_softmmu="$libs_softmmu `$pkg_config --libs libseccomp`"
> +QEMU_CFLAGS="$QEMU_CFLAGS `$pkg_config --cflags libseccomp`"
>   seccomp="yes"
>  else
>   if test "$seccomp" = "yes"; then

Re: [Qemu-devel] [PATCH v3] Throttle-down guest when live migration does not converge.

[Chegu Vinod]

On 5/1/2013 8:40 AM, Paolo Bonzini wrote:

I shall make the suggested changes.
Appreciate your review feedback on this part of the change.

Hi Paolo.,

Thanks for taking a look (BTW, I accidentally left out the "RFC"  in the 
patch subject line...my bad!).

Hi Vinod,

I think unfortunately it is not acceptable to make this patch work only
for KVM.  (It cannot work for Xen, but that's not a problem since Xen
uses a different migration mechanism; but it should work for TCG).


Ok. I hadn't yet looked at TCG aspects etc. Will follow up offline...



Unfortunately, as you noted the run_on_cpu callbacks currently run
under the big QEMU lock.  We need to fix that first.  We have time
for that during 1.6.


Ok.  Was under the impression that anytime a vcpu thread enters to do 
anything in qemu the BQL had to be held. So choose to go with 
run_on_cpu()  .  Will follow up offline on alternatives


"Holding" the vcpus in the host context (i.e. kvm module) itself is 
perhaps another way. Would need some handshakes (i.e. new ioctls ) with 
the kernel. Would that be acceptable way to proceed?


Thanks
Vinod



Paolo
.

Re: [Qemu-devel] [PATCH] softfloat: rebase to version 2a

[Aurelien Jarno]

On Mon, Apr 29, 2013 at 01:05:03PM -0500, Anthony Liguori wrote:
> N.B. If you are on CC, see after the '---' for a requested action!
> 
> The license of SoftFloat-2b is claimed to be GPLv2 incompatible by
> the FSF due to an indemnification clause.  The previous release,
> SoftFloat-2a, did not contain this clause.  The only changes between
> these two versions as far as QEMU is concerned is the license change
> and a global modification of the comment structure.  This patch rebases
> our softfloat code to SoftFloat-2a in order to have a GPLv2 compatible
> license.

Thanks for your work!

> Please note, this is a comment-only change.  The resulting binary should
> be the same.
> 
> I created this patch using the following strategy:
> 
> 1) Create a branch using the original import of softfloat code:
>$ git checkout 158142c2c2df728cfa3b5320c65534921a764f26
> 
> 2) Remove carriage returns from Softfloat-2b
> 
> 3) Compare each of the softfloat files against Softfloat-2b using the
>following mapping to generate Fabrice's original softfloat changes:
> 
>- fpu/softfloat.c -> softfloat/bits64/softfloat.c
>- fpu/softfloat.h -> softfloat/bits64/386-Win32-gcc/softfloat.h
>- fpu/softfloat-macros.h -> softfloat/bits64/softfloat-macros
>- fpu/softfloat-specialize.h -> 
> softfloat/bits64/386-Win32-gcc/softfloat-specialize
> 
> 4) Replace our softfloat files with the corresponding files from Softfloat-2a
> 
> 5) Apply the diffs from (3) to (4) and commit
> 
> 6) Create a diff between (5) and 158142c2c2df728cfa3b5320c65534921a764f26
>- This diff consists 100% of licensing change + comment reformating
> 
> 7) Checkout the latest master branch, apply the diff from (6)
>- There were a lot of comment rejects, confirmed this was only comments
>  and then used an emacs macro to rewrite the comments to the Softfloat-2a
>  form.

I guess this last step is the reason why comments added long after the
original code are also modified by your patch. Right?

> Cc: Andreas Färber 
> Cc: Aurelien Jarno 
> Cc: Avi Kivity 
> Cc: Ben Taylor 
> Cc: Blue Swirl 
> Cc: Christophe Lyon 
> Cc: Fabrice Bellard 
> Cc: Guan Xuetao 
> Cc: Jocelyn Mayer 
> Cc: Juan Quintela 
> Cc: malc 
> Cc: Max Filippov 
> Cc: Paolo Bonzini 
> Cc: Paul Brook 
> Cc: Peter Maydell 
> Cc: Richard Henderson 
> Cc: Richard Sandiford 
> Cc: Stefan Weil 
> Cc: Thiemo Seufer 
> Signed-off-by: Anthony Liguori 
> ---
> In order to make this change, we need to relicense all contributions
> from initial import of the SoftFloat code to match the license of
> SoftFloat-2a (instead of the implied SoftFloat-2b license).
> 
> If you are on CC, it is because you have contributed to the softfloat
> code in QEMU.  Please response to this note with:
> 
> Acked-by: Your Name 
> 
> To significant that you are able and willing to relicense your changes
> to the SoftFloat-1a license (or a GPL compatible license).
> 
> Please respond no later than May 6th, 2013.  If we are unable to confirm
> relicense from an author, changes from that author will be reverted.

Acked-by: Aurelien Jarno 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net

[Qemu-devel] [PATCH 1.5] win32: fix compilation again

[Paolo Bonzini]

While commit c02817e5bfbb27955cac970019e6670dc427bc41 fixed compilation
without an installed libtool, moving the dependencies to rules.mak does
not work because the version-*-y variables are not defined yet.  Building
in a clean tree thus fails.

Revert the commit and remove the dummy /bin/false assignment to LIBTOOL.
This makes the build work, at the price of slightly worse errors when
there are Makefile bugs.

Signed-off-by: Paolo Bonzini 
---
 Makefile  | 1 +
 rules.mak | 6 +-
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index f91f3b0..7dc0204 100644
--- a/Makefile
+++ b/Makefile
@@ -172,6 +172,7 @@ version.lo: $(SRC_PATH)/version.rc config-host.h
 version-obj-$(CONFIG_WIN32) += version.o
 version-lobj-$(CONFIG_WIN32) += version.lo
 
+Makefile: $(version-obj-y) $(version-lobj-y)
 
 ##
 # Build libraries
diff --git a/rules.mak b/rules.mak
index 197a9d7..d3145b4 100644
--- a/rules.mak
+++ b/rules.mak
@@ -22,15 +22,11 @@ QEMU_CFLAGS += -I$(/dev/null 2>&1 && echo OK), $2, $3)
 
-VPATH_SUFFIXES = %.c %.h %.S %.m %.mak %.texi %.sh
+VPATH_SUFFIXES = %.c %.h %.S %.m %.mak %.texi %.sh %.rc
 set-vpath = $(if $1,$(foreach PATTERN,$(VPATH_SUFFIXES),$(eval vpath 
$(PATTERN) $1)))
 
 # find-in-path
-- 
1.8.2

Re: [Qemu-devel] [RFC] Moving Hard Freeze to Monday, May 6th.

[Rob Landley]

On 04/30/2013 01:36:39 PM, Anthony Liguori wrote:


The current release schedule has hard freeze happening tomorrow.   
There

are a few things still outstanding including cpu hotplug and updating
SeaBIOS.  We still need to resolve how to handle the softfloat code
too.  I am particularly interested in this last one for the 1.5  
release.


Sparc's OpenBios also needs a refresh to handle longer command lines:

http://lists.gnu.org/archive/html/qemu-devel/2013-04/msg03880.html

Rob

Re: [Qemu-devel] [PATCH 09/17] memory: iommu support

[Paolo Bonzini]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Il 01/05/2013 06:35, David Gibson ha scritto:
>> From: Avi Kivity 
>> 
>> Add a new memory region type that translates addresses it is
>> given, then forwards them to a target address space.  This is
>> similar to an alias, except that the mapping is more flexible
>> than a linear translation and trucation, and also less efficient
>> since the translation happens at runtime.
>> 
>> The implementation uses an AddressSpace mapping the target region
>> to avoid hierarchical dispatch all the way to the resolved
>> region; only iommu regions are looked up dynamically.
>> 
>> Signed-off-by: Avi Kivity  [Modified to put
>> translation in address_space_translate - Paolo] Signed-off-by:
>> Paolo Bonzini  --- exec.c|
>> 35 +-- include/exec/memory.h |
>> 44  memory.c
>> |   28  3 files changed, 101
>> insertions(+), 6 deletions(-)
> 
> [snip]
>> +void memory_region_init_iommu(MemoryRegion *mr, +
>> MemoryRegionIOMMUOps *ops, +
>> MemoryRegion *target, +  const char
>> *name, +  uint64_t size) +{ +
>> memory_region_init(mr, name, size); +mr->ops = NULL; +
>> mr->iommu_ops = ops, +mr->opaque = mr; +mr->terminates =
>> true;  /* then re-forwards */ +mr->destructor =
>> memory_region_destructor_iommu; +mr->iommu_target_as =
>> g_new(AddressSpace, 1); +
>> address_space_init(mr->iommu_target_as, target);
> 
> Since IOMMUs are very likely to share a target AS (in fact, it
> will nearly always be system memory), it seems odd to me to
> construct new AddressSpace objects for each one, rather than just
> giving the AddressSpace as the parameter to
> memory_region_init_iommu.
> 

I think the problem is that we do not have reference counting, and
this makes it simpler to manage the lifetime.  It can be changed later.

Paolo
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRgT6GAAoJEBvWZb6bTYbyk/cP/RCHjFrtXYas+TLti8vcbCS1
zCUGnURgNqKxI4529E9VGF4PlO6/uxDP/mpNgDx7FCkMyA6bNL6fxw7RhvWyKFjU
xvZt6l7HYVOxbuNVAraZ4JmQ+fnvgyxJn1djMKNdGKNuRdpXj6b2NC+xyw8rSSc2
bIR6FT0piAlFP+kXiNFyf3cIZzT84qVX1tqphCRov7jvWyYG8Zv5VHpEEjaY7fJS
LEJFQ4ZfjM94RAekhZ7GkGmmlh9vfTplKz0G+0/EOYcaWBNwd7bKjGf6hWNtLb3z
3chtMKyODe8JRR/FuWuLx0GDZzvKZOE9yRGHerhL0aDCMviNKILUTnES3twe82j9
F8ywYO4CnkdKRIzKXXDTd8hZkgzF60fWzMvtvjOg6oXHJFyNoXRLhD23sZsTFaMi
q+mk7JiPfXrvjrZcUlQ4YwN7gv9gkZ+2JO1di/R9Xq62aqYBLke9Fwke7fdX+qbR
RB7coXgfExheRJdyWB56Lqr9m2KAtnW7T6ISR1G/nV0yr+ye1N9LifUkCAg4MHa8
ANsMMA6NbwmwtqJ9DDqzH+RkK7ZzCzq/IkYGeEial/sUnxXLggu6LanyahTMun49
ZmJDbEUWVAbrA4bkbQ1XHM6B2EFLg//wen7uSdIR+UZKgjkhxpB+PKiLppvPyXW1
5LuT1XZmghaeE3nD/i3A
=GcNo
-END PGP SIGNATURE-

Re: [Qemu-devel] [PATCH 15/17] pci: use memory core for iommu support

[Paolo Bonzini]

Il 01/05/2013 07:06, David Gibson ha scritto:
>>> + +/* FIXME: inherit memory region from bus creator */ +
>>> memory_region_init_alias(mr, "iommu-nop", get_system_memory(),
>>> 0, INT64_MAX); +return mr;
> I don't see the reason for creating a new alias for each PCI
> device. Can't pci_dev->iommu just point directly to
> get_system_memory() in the normal case?
> 
> In addition to creating additional objects, having these aliases
> makes it much les obvious how to tell if two PCI devices share an
> IOMMU address space.  We have to be able to determine that for
> VFIO, since devices which share an address space in the host
> clearly can't be assigned to different address spaces in the
> guest.

Right, the alias that is needed to enable/disable bus-mastering is
created already below, so pci_dev->iommu can be shared (as it is in the
spapr case).

Paolo

Re: [Qemu-devel] [PATCH 16/17] spapr_vio: take care of creating our own AddressSpace/DMAContext

[Paolo Bonzini]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Il 01/05/2013 07:16, David Gibson ha scritto:
> Lack of atomicity makes me a little nervous there, although I
> guess its ok since qemu is single-threaded.

Yes.  The original plan was to add a boolean return value to
address_space_rw, but I left this for later since I wasn't sure of the
semantics for multipage writes.  What happens if the second half of
the destination buffer has an invalid translation?  Right now it's
atomic, but it sounds weird for real hardware.

Paolo

> It does mean we do the translations twice, effectively, which isn't
> ideal, although emulated devices are already so slow it probably
> doesn't matter.
> 


-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRgT4xAAoJEBvWZb6bTYbyePoP/3BC7W/3LC/YfEJ16CbSsnhN
tJ2kLdjuI9+OAzAfb++TKoxkAAGtPSCFpBiS7s1T2r9gTSYMQX15enppU+lBFxwq
qfK7UF11carjnsDB3rXhUnZTGYPWYwA17mR4U2PBFy4EeZZ1p57m1QmJsrgI9UI7
u7QyC+NrNPM3fA0R/CCSn5ld3hh14GK3gEv+SK9IIsnwmoyQ6NKsDi3AAzdnSbMZ
pnMpFCb2Vu2MT/8M80r7J3aRUMGsVwlQgVyqxDpWJ4YJSDO6SaoIj7U+0gv867sa
gJwg94N6umurU49esGGjOzvTxlwdgQQDkG/sDUeMPsGS/cqb4RYeJKRK6ieFqdTY
SPUOz0ssUQhqzRX9zoUQE8aA0rt/CLL+E9+8MYmqAkKMLPpMmLsqrZxl11d19MTN
WdZnYmJvVLAVn+YUae4N0ehhgXSMv2MTabb39V7VG2QziEdcIn8wQIWGTTTw6o/p
GBFu0P05mXL8jvSQ5CQp3k36OqwKGlgjaHB4MOsbCKTahLBQ9VoarKB6Xh/4xWxj
3+kYjYdydnJMogOe4FE5mld6LJnlo66e/GJNdZyHzsdyB9cK9qMg+i3yXQElNSp9
H34sMKfpFgJ/6z/cwbrNrSWesgKDTqhhXKzocgvZkQUbvJCF5I2xigi8aKB3vC+N
wjEs5RUYBzb3ARKWCwy/
=Jr5p
-END PGP SIGNATURE-

[Qemu-devel] [PATCH qom-cpu for-1.5 4/4] target-i386: Change CPUID model of 486 to 8

[Andreas Färber]

This changes the model number of 486 to 8 (DX4) which matches the
feature set presented, and actually has the CPUID instruction.

This adds a compatibility property, to keep model=0 on pc-*-1.4 and older.

Signed-off-by: H. Peter Anvin 
Cc: Eduardo Habkost 
[AF: Add compat_props entry]
Signed-off-by: Andreas Färber 
---
 include/hw/i386/pc.h | 4 
 target-i386/cpu.c| 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 41869e5..417afe4 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -242,6 +242,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
 .driver   = "pc-sysfw",\
 .property = "rom_only",\
 .value= stringify(0),\
+},{\
+.driver   = "486-" TYPE_X86_CPU,\
+.property = "model",\
+.value= stringify(0),\
 }
 
 #endif
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8a9563b..f7d4d9b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -550,7 +550,7 @@ static x86_def_t builtin_x86_defs[] = {
 .level = 1,
 .vendor = CPUID_VENDOR_INTEL,
 .family = 4,
-.model = 0,
+.model = 8,
 .stepping = 0,
 .features = I486_FEATURES,
 .xlevel = 0,
-- 
1.8.1.4

[Qemu-devel] [PATCH qom-cpu for-1.5 1/4] qdev: Let qdev_prop_parse() pass through Error

[Andreas Färber]

Move error reporting to callers.

Signed-off-by: Andreas Färber 
---
 hw/core/qdev-properties.c| 25 +++--
 hw/core/qdev.c   |  7 ++-
 include/hw/qdev-properties.h |  5 +++--
 qdev-monitor.c   |  6 +-
 4 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index ca1739e..716ba19 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -986,25 +986,18 @@ void error_set_from_qdev_prop_error(Error **errp, int 
ret, DeviceState *dev,
 }
 }
 
-int qdev_prop_parse(DeviceState *dev, const char *name, const char *value)
+void qdev_prop_parse(DeviceState *dev, const char *name, const char *value,
+ Error **errp)
 {
 char *legacy_name;
-Error *err = NULL;
 
 legacy_name = g_strdup_printf("legacy-%s", name);
 if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) {
-object_property_parse(OBJECT(dev), value, legacy_name, &err);
+object_property_parse(OBJECT(dev), value, legacy_name, errp);
 } else {
-object_property_parse(OBJECT(dev), value, name, &err);
+object_property_parse(OBJECT(dev), value, name, errp);
 }
 g_free(legacy_name);
-
-if (err) {
-qerror_report_err(err);
-error_free(err);
-return -1;
-}
-return 0;
 }
 
 void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value)
@@ -1106,18 +1099,22 @@ void qdev_prop_register_global_list(GlobalProperty 
*props)
 }
 }
 
-void qdev_prop_set_globals(DeviceState *dev)
+void qdev_prop_set_globals(DeviceState *dev, Error **errp)
 {
 ObjectClass *class = object_get_class(OBJECT(dev));
 
 do {
 GlobalProperty *prop;
 QTAILQ_FOREACH(prop, &global_props, next) {
+Error *err = NULL;
+
 if (strcmp(object_class_get_name(class), prop->driver) != 0) {
 continue;
 }
-if (qdev_prop_parse(dev, prop->property, prop->value) != 0) {
-exit(1);
+qdev_prop_parse(dev, prop->property, prop->value, &err);
+if (err != NULL) {
+error_propagate(errp, err);
+return;
 }
 }
 class = object_class_get_parent(class);
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index ab1d8f5..7167c90 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -752,7 +752,12 @@ static void device_initfn(Object *obj)
 }
 class = object_class_get_parent(class);
 } while (class != object_class_by_name(TYPE_DEVICE));
-qdev_prop_set_globals(dev);
+qdev_prop_set_globals(dev, &err);
+if (err != NULL) {
+qerror_report_err(err);
+error_free(err);
+exit(1);
+}
 
 object_property_add_link(OBJECT(dev), "parent_bus", TYPE_BUS,
  (Object **)&dev->parent_bus, &err);
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 25dd1bb..38469d4 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -148,7 +148,8 @@ extern PropertyInfo qdev_prop_arraylen;
 
 /* Set properties between creation and init.  */
 void *qdev_get_prop_ptr(DeviceState *dev, Property *prop);
-int qdev_prop_parse(DeviceState *dev, const char *name, const char *value);
+void qdev_prop_parse(DeviceState *dev, const char *name, const char *value,
+ Error **errp);
 void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value);
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value);
 void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value);
@@ -167,7 +168,7 @@ void qdev_prop_set_ptr(DeviceState *dev, const char *name, 
void *value);
 
 void qdev_prop_register_global(GlobalProperty *prop);
 void qdev_prop_register_global_list(GlobalProperty *props);
-void qdev_prop_set_globals(DeviceState *dev);
+void qdev_prop_set_globals(DeviceState *dev, Error **errp);
 void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
 Property *prop, const char *value);
 
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 2cb5600..e54dbc2 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -105,13 +105,17 @@ static void qdev_print_devinfo(ObjectClass *klass, void 
*opaque)
 static int set_property(const char *name, const char *value, void *opaque)
 {
 DeviceState *dev = opaque;
+Error *err = NULL;
 
 if (strcmp(name, "driver") == 0)
 return 0;
 if (strcmp(name, "bus") == 0)
 return 0;
 
-if (qdev_prop_parse(dev, name, value) == -1) {
+qdev_prop_parse(dev, name, value, &err);
+if (err != NULL) {
+qerror_report_err(err);
+error_free(err);
 return -1;
 }
 return 0;
-- 
1.8.1.4

[Qemu-devel] [PATCH qom-cpu for-1.5 3/4] target-i386: Emulate X86CPU subclasses for global properties

[Andreas Färber]

After initializing the object from its x86_def_t and before setting any
additional -cpu arguments, set any global properties for the designated
subclass -{i386,x86_64}-cpu.

Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index bba41fe..8a9563b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1626,6 +1626,7 @@ X86CPU *cpu_x86_create(const char *cpu_model, DeviceState 
*icc_bridge,
 CPUX86State *env;
 gchar **model_pieces;
 char *name, *features;
+char *typename;
 Error *error = NULL;
 
 model_pieces = g_strsplit(cpu_model, ",", 2);
@@ -1653,6 +1654,14 @@ X86CPU *cpu_x86_create(const char *cpu_model, 
DeviceState *icc_bridge,
 goto out;
 }
 
+/* Emulate per-model subclasses for global properties */
+typename = g_strdup_printf("%s-" TYPE_X86_CPU, name);
+qdev_prop_set_custom_globals(DEVICE(cpu), typename, &error);
+g_free(typename);
+if (error) {
+goto out;
+}
+
 cpu_x86_parse_featurestr(cpu, features, &error);
 if (error) {
 goto out;
-- 
1.8.1.4

[Qemu-devel] [PATCH qom-cpu for-1.5 2/4] qdev: Introduce qdev_prop_set_custom_globals()

[Andreas Färber]

Reuse it in qdev_prop_set_globals().

Signed-off-by: Andreas Färber 
---
 hw/core/qdev-properties.c| 35 ---
 include/hw/qdev-properties.h |  2 ++
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 716ba19..68d1bff 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1099,23 +1099,36 @@ void qdev_prop_register_global_list(GlobalProperty 
*props)
 }
 }
 
+void qdev_prop_set_custom_globals(DeviceState *dev, const char *driver,
+  Error **errp)
+{
+GlobalProperty *prop;
+
+QTAILQ_FOREACH(prop, &global_props, next) {
+Error *err = NULL;
+
+if (strcmp(driver, prop->driver) != 0) {
+continue;
+}
+qdev_prop_parse(dev, prop->property, prop->value, &err);
+if (err != NULL) {
+error_propagate(errp, err);
+return;
+}
+}
+}
+
 void qdev_prop_set_globals(DeviceState *dev, Error **errp)
 {
 ObjectClass *class = object_get_class(OBJECT(dev));
 
 do {
-GlobalProperty *prop;
-QTAILQ_FOREACH(prop, &global_props, next) {
-Error *err = NULL;
+Error *err = NULL;
 
-if (strcmp(object_class_get_name(class), prop->driver) != 0) {
-continue;
-}
-qdev_prop_parse(dev, prop->property, prop->value, &err);
-if (err != NULL) {
-error_propagate(errp, err);
-return;
-}
+qdev_prop_set_custom_globals(dev, object_class_get_name(class), &err);
+if (err != NULL) {
+error_propagate(errp, err);
+return;
 }
 class = object_class_get_parent(class);
 } while (class);
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 38469d4..833300c 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -169,6 +169,8 @@ void qdev_prop_set_ptr(DeviceState *dev, const char *name, 
void *value);
 void qdev_prop_register_global(GlobalProperty *prop);
 void qdev_prop_register_global_list(GlobalProperty *props);
 void qdev_prop_set_globals(DeviceState *dev, Error **errp);
+void qdev_prop_set_custom_globals(DeviceState *dev, const char *driver,
+  Error **errp);
 void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
 Property *prop, const char *value);
 
-- 
1.8.1.4

[Qemu-devel] [PATCH qom-cpu for-1.5 0/4] target-i386: X86CPU compatibility properties

[Andreas Färber]

Hello,

It's easier adapting the infrastructure to our needs than working around it:
X86CPU already has QOM properties today. What's lacking is model subclasses,
and with the one X86CPU type its global properties are overwritten by models.
But we already know the designated naming scheme for the models!

So let's simply prepare compat_props for CPU models and make sure they are
already picked up today.

This works just fine for changing the 486 CPUID model value and avoids to
redo the PC part once we have X86CPU subclasses.
Tested using: ./QMP/qom-get /machine/icc-bridge/icc/child[0].model

For changing n270 CPUID flags we'll still need to resort to Eduardo's proposed
helper functions for now.

Regards,
Andreas

Cc: Eduardo Habkost 
Cc: Igor Mammedov 
Cc: H. Peter Anvin 
Cc: Borislav Petkov 
Cc: Anthony Liguori 
Cc: Paolo Bonzini 
Cc: Michael S. Tsirkin 

Andreas Färber (4):
  qdev: Let qdev_prop_parse() pass through Error
  qdev: Introduce qdev_prop_set_custom_globals()
  target-i386: Emulate X86CPU subclasses for global properties
  target-i386: Change CPUID model of 486 to 8

 hw/core/qdev-properties.c| 50 ++--
 hw/core/qdev.c   |  7 ++-
 include/hw/i386/pc.h |  4 
 include/hw/qdev-properties.h |  7 +--
 qdev-monitor.c   |  6 +-
 target-i386/cpu.c| 11 +-
 6 files changed, 60 insertions(+), 25 deletions(-)

-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v3] Throttle-down guest when live migration does not converge.

[Paolo Bonzini]

> I shall make the suggested changes.
> Appreciate your review feedback on this part of the change.

Hi Vinod,

I think unfortunately it is not acceptable to make this patch work only
for KVM.  (It cannot work for Xen, but that's not a problem since Xen
uses a different migration mechanism; but it should work for TCG).

Unfortunately, as you noted the run_on_cpu callbacks currently run
under the big QEMU lock.  We need to fix that first.  We have time
for that during 1.6.

Paolo

Re: [Qemu-devel] [RFC] Continuous work on sandboxing

[Corey Bryant]

On 05/01/2013 10:13 AM, Paul Moore wrote:

On Tuesday, April 30, 2013 04:28:54 PM Corey Bryant wrote:

Just to be clear, I'm thinking you could launch guests in one of two
different seccomp sandboxed environments:

1) Using the existing and more permissive whitelist where every QEMU
feature works:

qemu-kvm -sandbox on,default


In general, I like the comma delimited list of sandbox filters/methods/etc.
but I'm not sure we need to explicitly specify "default", it seems like "on"
would be sufficient.  It also preserved compatibility with what we have now.



Yes, I agree.  This should definitely remain backward compatible.

--
Regards,
Corey Bryant

Re: [Qemu-devel] [PATCH v2] memory: give name every AddressSpace

[Paolo Bonzini]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Il 01/05/2013 05:25, David Gibson ha scritto:
>>> Applied to iommu branch, thanks.
> I don't see this patch at git://github.com/bonzini/qemu.git yet.
> Did you need to do something to push it out?

I just applied it locally for now.

Paolo
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRgS/NAAoJEBvWZb6bTYbyr2wP/0SE8WDVpcteMf0oO+W/o1jg
nav1QqSiVWs6KbFR5uQsiLK3l+Rv1BKKf5FZoGiOSvHxyr7/tYpv6MmOAMYyKqVF
+KsYZ5J8snL9OyZviSkUPvmYF/v8+MZxBAMJHB5ryjpZoDk/4y5aT0LrPl6NLeqy
fkXToabzPFWHEWVwa6JkHgyS1IFxYM0zgobCv82TGypSAebded02BvrIzUV6jSI0
iEjIAKdZiSK2pdgHnZFRHqd0I8+NHjeSaXXX5YxhtE0R3uCMNoRBiam9t5aGVIYb
HLwGq94/ikf4gDnntkE6/J9zAwVcHltp97InGEhOQRP/71xgGjTbRVDn7v5kadwC
UMZzGA+fBVhI80Zjnb280hQVPHihgicO3koaaEkW6zBDGuCZZtr2yocQFnVYRZhH
CnQ5yUG6SZCgH+RlsIZ8mh/bEknH21+IF4xdTv5j3gSH8R5Y3I+l3KShTMDCcEfc
+ItrdQQLJXMlPXXRCeG6N1+Hkg1VpcnhGBpfIXqUTgfJtvGqi+TzEMqt8KGeIMEf
7YLfpHm7vd5kseBo6u85XScAcxPazfoSn8vTb0yFiU+YeSUstZzoAwoQg/7lzarB
1ouWvNrT9plNywg2aMGO34tCQCSCDwMlsQ47HMGxKiCTlVEczz758Xki1NAURCTy
uza3CdJb9OrYDQ5qeDOi
=jcUq
-END PGP SIGNATURE-

Re: [Qemu-devel] [PATCH v3 0/4] Initial VHDX support

[Jeff Cody]

On Wed, May 01, 2013 at 02:43:04PM +0200, Stefan Hajnoczi wrote:
> On Mon, Apr 29, 2013 at 02:48:15PM -0400, Jeff Cody wrote:
> > 
> > Differences from v2:
> > 
> > 
> > Patch 2/4:  changed 2 uint8_t[16] to MSGUID (Kevin)
> > renamed all strucs to proper style (Kevin)
> > indentations / style (Kevin)
> > Removed/fixed outdated comments (Stefan)
> > dropped vhdx_header_padded (Kevin)
> > 
> > Patch 3/4:  removed hunk that was meant for old patch 5/5 (Fam)
> > check VHDX header version field (Fam)
> > validate file type id in open and not just probe (Kevin)
> > allow headers with seq # of 0 if other header is invalid (Kevin)
> > fixed masking typo for parent meta present (Kevin)
> > removed error check for 512-byte sectors (Kevin)
> > verify region and metadata table guid are unique (Kevin)
> > removed signed/unsigned comparisons (Stefan)
> > sanity checking on rt.entry_count (Stefan)
> > use clz_() instead of inline code (Stefan)
> > bat_rt.length sanity check - compute it rather
> >than trust the entry length (Stefan)
> > 
> > Patch 5 (v2): Dropped
> > 
> > This adds the initial support for VHDX image files.  
> > 
> > It currently only supports read operations of VHDX, for fixed and dynamic 
> > files.
> > 
> > Notably, the following is not yet supported:
> > * Differencing files
> > * Log replay (so we will refuse to open any images that are not 'clean')
> > * .bdrv_create()
> > * write operations other than to the header
> > 
> > 
> > Jeff Cody (4):
> >   qemu: add castagnoli crc32c checksum algorithm
> >   block: vhdx header for the QEMU support of VHDX images
> >   block: initial VHDX driver support framework - supports open and probe
> >   block: add read-only support to VHDX image format.
> > 
> >  block/Makefile.objs   |   1 +
> >  block/vhdx.c  | 972 
> > ++
> >  block/vhdx.h  | 325 +
> >  include/qemu/crc32c.h |  35 ++
> >  util/Makefile.objs|   1 +
> >  util/crc32c.c | 115 ++
> >  6 files changed, 1449 insertions(+)
> >  create mode 100644 block/vhdx.c
> >  create mode 100644 block/vhdx.h
> >  create mode 100644 include/qemu/crc32c.h
> >  create mode 100644 util/crc32c.c
> 
> Do you have links to vhdx files I can test?
>

I created a new VHDX file, because the ones I've been testing with
have grown too large to share easily.  It is a 32GB dynamic image that
contains a single EXT4 partition:

https://www.dropbox.com/s/52q84zy2n59ix6h/TestDisk32GBDynamic-ext4.vhdx.7z

I'll keep this link around for a while, but it will eventually go
away.

Inside the ext4 partition there are 10 image files generated from
/dev/urandom, and corresponding SHA1 hashes in a single SHA1SUM file,
for verification.

Also, if you wish to check the SHA1SUM of the entire image from the
view of the guest (e.g.  sha1sum /dev/vdb), the resulting hash should
be:

  f897e5cadd3a0b1e776625caf79ed0eaf4baa3d8  /dev/vdb

(The above has was calculated from a Linux guest under Hyper-V 2012,
and verified with these patches and QEMU).


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

Thanks!

Jeff

Re: [Qemu-devel] [PATCH] audio: Enable all cards

[Paolo Bonzini]

Il 01/05/2013 16:14, Jan Kiszka ha scritto:
> From: Jan Kiszka 
> 
> ...or they will bitrot to death.
> 
> Signed-off-by: Jan Kiszka 
> ---
>  default-configs/sound.mak |6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)

I agree.  I didn't want to change the default without hearing from malc,
but it's a good idea.

Paolo

> diff --git a/default-configs/sound.mak b/default-configs/sound.mak
> index ff69c4d..4f22c34 100644
> --- a/default-configs/sound.mak
> +++ b/default-configs/sound.mak
> @@ -1,4 +1,4 @@
>  CONFIG_SB16=y
> -#CONFIG_ADLIB=y
> -#CONFIG_GUS=y
> -#CONFIG_CS4231A=y
> +CONFIG_ADLIB=y
> +CONFIG_GUS=y
> +CONFIG_CS4231A=y
> 

[Qemu-devel] [PATCH] m25p80.c: Sync Flash chip list with Linux

[Ed Maste]

Add new devices for various manufacturers, and re-sort Spansion list to
match the order in Linux, which requires chips with a non-zero extended ID
to come first.

With this commit the outstanding differences to Linux rev 55bf75b are:

- Erase size flag differences in s25sl032p, s25sl064p, s25fl016k, s25fl064k
  (These devices have only some blocks that support small erase sizes.)
- Linux lacks n25q128
- Devices without a Jedec ID have been excluded

Signed-off-by: Ed Maste 
---
 hw/block/m25p80.c | 31 ++-
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index b3ca19a..759c84d 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -91,18 +91,27 @@ static const FlashPartInfo known_devices[] = {
 { INFO("at26df161a",  0x1f4601,  0,  64 << 10,  32, ER_4K) },
 { INFO("at26df321",   0x1f4700,  0,  64 << 10,  64, ER_4K) },
 
+{ INFO("at45db081d",  0x1f2500,  0,  64 << 10,  16, ER_4K) },
+
 /* EON -- en25xxx */
 { INFO("en25f32", 0x1c3116,  0,  64 << 10,  64, ER_4K) },
 { INFO("en25p32", 0x1c2016,  0,  64 << 10,  64, 0) },
 { INFO("en25q32b",0x1c3016,  0,  64 << 10,  64, 0) },
 { INFO("en25p64", 0x1c2017,  0,  64 << 10, 128, 0) },
+{ INFO("en25q64", 0x1c3017,  0,  64 << 10, 128, ER_4K) },
+
+/* GigaDevice */
+{ INFO("gd25q32", 0xc84016,  0,  64 << 10,  64, ER_4K) },
+{ INFO("gd25q64", 0xc84017,  0,  64 << 10, 128, ER_4K) },
 
 /* Intel/Numonyx -- xxxs33b */
 { INFO("160s33b", 0x898911,  0,  64 << 10,  32, 0) },
 { INFO("320s33b", 0x898912,  0,  64 << 10,  64, 0) },
 { INFO("640s33b", 0x898913,  0,  64 << 10, 128, 0) },
+{ INFO("n25q064", 0x20ba17,  0,  64 << 10, 128, 0) },
 
 /* Macronix */
+{ INFO("mx25l2005a",  0xc22012,  0,  64 << 10,   4, ER_4K) },
 { INFO("mx25l4005a",  0xc22013,  0,  64 << 10,   8, ER_4K) },
 { INFO("mx25l8005",   0xc22014,  0,  64 << 10,  16, 0) },
 { INFO("mx25l1606e",  0xc22015,  0,  64 << 10,  32, ER_4K) },
@@ -113,15 +122,16 @@ static const FlashPartInfo known_devices[] = {
 { INFO("mx25l25635e", 0xc22019,  0,  64 << 10, 512, 0) },
 { INFO("mx25l25655e", 0xc22619,  0,  64 << 10, 512, 0) },
 
+/* Micron */
+{ INFO("n25q128a11",  0x20bb18,  0,  64 << 10, 256, 0) },
+{ INFO("n25q128a13",  0x20ba18,  0,  64 << 10, 256, 0) },
+{ INFO("n25q256a",0x20ba19,  0,  64 << 10, 512, ER_4K) },
+
 /* Spansion -- single (large) sector size only, at least
  * for the chips listed here (without boot sectors).
  */
-{ INFO("s25sl004a",   0x010212,  0,  64 << 10,   8, 0) },
-{ INFO("s25sl008a",   0x010213,  0,  64 << 10,  16, 0) },
-{ INFO("s25sl016a",   0x010214,  0,  64 << 10,  32, 0) },
-{ INFO("s25sl032a",   0x010215,  0,  64 << 10,  64, 0) },
 { INFO("s25sl032p",   0x010215, 0x4d00,  64 << 10,  64, ER_4K) },
-{ INFO("s25sl064a",   0x010216,  0,  64 << 10, 128, 0) },
+{ INFO("s25sl064p",   0x010216, 0x4d00,  64 << 10, 128, ER_4K) },
 { INFO("s25fl256s0",  0x010219, 0x4d00, 256 << 10, 128, 0) },
 { INFO("s25fl256s1",  0x010219, 0x4d01,  64 << 10, 512, 0) },
 { INFO("s25fl512s",   0x010220, 0x4d00, 256 << 10, 256, 0) },
@@ -130,6 +140,11 @@ static const FlashPartInfo known_devices[] = {
 { INFO("s25sl12801",  0x012018, 0x0301,  64 << 10, 256, 0) },
 { INFO("s25fl129p0",  0x012018, 0x4d00, 256 << 10,  64, 0) },
 { INFO("s25fl129p1",  0x012018, 0x4d01,  64 << 10, 256, 0) },
+{ INFO("s25sl004a",   0x010212,  0,  64 << 10,   8, 0) },
+{ INFO("s25sl008a",   0x010213,  0,  64 << 10,  16, 0) },
+{ INFO("s25sl016a",   0x010214,  0,  64 << 10,  32, 0) },
+{ INFO("s25sl032a",   0x010215,  0,  64 << 10,  64, 0) },
+{ INFO("s25sl064a",   0x010216,  0,  64 << 10, 128, 0) },
 { INFO("s25fl016k",   0xef4015,  0,  64 << 10,  32, ER_4K | ER_32K) },
 { INFO("s25fl064k",   0xef4017,  0,  64 << 10, 128, ER_4K | ER_32K) },
 
@@ -153,11 +168,13 @@ static const FlashPartInfo known_devices[] = {
 { INFO("m25p32",  0x202016,  0,  64 << 10,  64, 0) },
 { INFO("m25p64",  0x202017,  0,  64 << 10, 128, 0) },
 { INFO("m25p128", 0x202018,  0, 256 << 10,  64, 0) },
+{ INFO("n25q032", 0x20ba16,  0,  64 << 10,  64, 0) },
 
 { INFO("m45pe10", 0x204011,  0,  64 << 10,   2, 0) },
 { INFO("m45pe80", 0x204014,  0,  64 << 10,  16, 0) },
 { INFO("m45pe16", 0x204015,  0,  64 << 10,  32, 0) },
 
+{ INFO("m25pe20", 0x208012,  0,  64 << 10,   4, 0) },
 { INFO("m25pe80", 0x208014,  0,  64 << 10,  16, 0) },
 { INFO("m25pe16", 0x208015,  0,  64 << 10,  32, ER_4K) },
 
@@ -174,8 +191,12 @@ static const FlashPartInfo known_devices[] = {
 { INFO("w25x16",  0xef30

Re: [Qemu-devel] [RFC][PATCH v3 00/24] instrument: Let the user wrap/override specific event tracing routines

[Lluís Vilanova]

Stefan Hajnoczi writes:

> On Sun, Apr 28, 2013 at 09:25:15PM +0200, Lluís Vilanova wrote:
>> Stefan Hajnoczi writes:
>> 
>> > On Fri, Apr 26, 2013 at 02:15:46PM +0200, Lluís Vilanova wrote:
>> > Advanced users will have to modify the QEMU source.
>> 
>> >> > Basically I think putting a stable API in place here isn't going to fly.
>> >> > In terms of the tracing subsystem I don't mind, but I think it's a
>> >> > question for the larger QEMU community.
>> >> 
>> >> What are you referring to as an API? The tracing events and their 
>> >> signatures?
>> 
>> > The interface that dynamic instrumentation libraries use to interact
>> > with QEMU.  That means "stable" tracing events plus any operations that
>> > libraries can perform (stop/stop vcpu, peek/poke memory, dirty bitmaps,
>> > MMU, interrupts, etc).
>> 
>> Not all events require being "stable", just a few that I established as
>> "abstract" and relate to some very generic guest state (which in their most
>> basic form can be boiled down to 5). As for the rest of the API and more
>> controversial events, as I said I have no problem in maintaining a separate
>> branch.

> Can you split the work into two patch series:
> 1. Target-independent TCG events (e.g. vbbl_begin)
> 2. "custom" trace backend that links custom C trace event handler
>functions - but without dynamic libraries or stable API.

> It loses the dynamic library and stable API features but would be easy
> to merge.

Yes, I was thinking about sending a mail along those lines too. This basically
entails two series (2nd and 4th in my current branch):

* Support for tracing events at TCG code generation time (basically
  auto-generating per-event TCG helpers - trace__tcg -, which call the
  actual tracing event - trace_ -).

* The events themselves (calls to trace__tcg).

I can then maintain instrumentation support and its public API on a separate
branch.

Still, I'm not sure when I'll have time for this, as it will require moving a
few patches among the series I have now (and readjusting their contents).


Thanks,
   Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

Re: [Qemu-devel] [RFC][PATCH v2]Add timestamp to error message

[Anthony Liguori]

"Daniel P. Berrange"  writes:

> On Wed, May 01, 2013 at 06:16:33AM -0600, Eric Blake wrote:
>> On 05/01/2013 06:05 AM, Stefan Hajnoczi wrote:
>> 
>> >> +error_printf(
>> >> +   "%4d-%02d-%02d %02d:%02d:%02d.%03lld+ ",
>> >> +   fields.tm_year + 1900, fields.tm_mon + 1, fields.tm_mday,
>> >> +   fields.tm_hour, fields.tm_min, fields.tm_sec,
>> >> +   now % 1000);
>> > 
>> > Can strftime(3) be used instead of copying code from glibc?
>> 
>> No, because glibc's strftime() is not async-signal safe, and therefore
>> is not safe to call between fork() and exec() (libvirt hit actual
>> deadlocks[1] where a child was blocked on a mutex associated with
>> time-related functions that happened to be held by another parent
>> thread, but where the fork nuked that other thread so the mutex would
>> never clear).  This code is copied from libvirt, which copied the
>> no-lock-needed safe portions of glibc for a reason.
>> 
>> [1] https://www.redhat.com/archives/libvir-list/2011-November/msg01609.html
>
> NB the original libvirt code had this & other related functions in
> a standalone file, along with a significant test suite. I think it
> is better from a maintenence POV to keep this functionality in a
> file that's separate from the qemu error file, so it can be reused
> elsewhere in QEMU if needed. It should also copy the test suite,
> since it is bad practice to throw away tests which already exist.

I also don't buy that we can't use strftime.  There are very few places
where we use fork() in QEMU (it doesn't exist on Windows so it can't be
used without protection).  None of those places use the error reporting
infrastructure.

This code is also extremely naive.  It doesn't take into account leap
seconds and makes bad assumptions about leap years.

Regards,

Anthony Liguori

>
> Daniel
> -- 
> |: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org  -o- http://virt-manager.org :|
> |: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v4] Add GDB qAttached support

[Jan Kiszka]

On 2013-03-14 20:51, Jan Kiszka wrote:
> With this patch QEMU handles qAttached request from gdb. When QEMU
> replies 1, GDB sends a "detach" command at the end of a debugging
> session otherwise GDB sends "kill".
> 
> The default value for qAttached is 1 on system emulation and 0 on user
> emulation.
> 
> Based on original version by Fabien Chouteau.
> 

Ping for this and the revert of "gdbstub: Do not kill target in system
emulation mode".

Jan

> Signed-off-by: Jan Kiszka 
> ---
> 
> As Fabien dropped his attempt to make this configurable, let's
> preserve the value of exposing this feature to gdb statically.
> 
>  gdbstub.c |   10 ++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/gdbstub.c b/gdbstub.c
> index e414ad9..9daee86 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -42,6 +42,12 @@
>  #include "sysemu/kvm.h"
>  #include "qemu/bitops.h"
>  
> +#ifdef CONFIG_USER_ONLY
> +#define GDB_ATTACHED "0"
> +#else
> +#define GDB_ATTACHED "1"
> +#endif
> +
>  #ifndef TARGET_CPU_MEMORY_RW_DEBUG
>  static inline int target_memory_rw_debug(CPUArchState *env, target_ulong 
> addr,
>   uint8_t *buf, int len, int is_write)
> @@ -2491,6 +2497,10 @@ static int gdb_handle_packet(GDBState *s, const char 
> *line_buf)
>  break;
>  }
>  #endif
> +if (strncmp(p, "Attached", 8) == 0) {
> +put_packet(s, GDB_ATTACHED);
> +break;
> +}
>  /* Unrecognised 'q' command.  */
>  goto unknown_command;
>  
> 




signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] audio: Enable all cards

[Jan Kiszka]

From: Jan Kiszka 

...or they will bitrot to death.

Signed-off-by: Jan Kiszka 
---
 default-configs/sound.mak |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/default-configs/sound.mak b/default-configs/sound.mak
index ff69c4d..4f22c34 100644
--- a/default-configs/sound.mak
+++ b/default-configs/sound.mak
@@ -1,4 +1,4 @@
 CONFIG_SB16=y
-#CONFIG_ADLIB=y
-#CONFIG_GUS=y
-#CONFIG_CS4231A=y
+CONFIG_ADLIB=y
+CONFIG_GUS=y
+CONFIG_CS4231A=y
-- 
1.7.3.4

Re: [Qemu-devel] [RFC] Continuous work on sandboxing

[Paul Moore]

On Tuesday, April 30, 2013 04:28:54 PM Corey Bryant wrote:
> Just to be clear, I'm thinking you could launch guests in one of two
> different seccomp sandboxed environments:
> 
> 1) Using the existing and more permissive whitelist where every QEMU
> feature works:
> 
> qemu-kvm -sandbox on,default

In general, I like the comma delimited list of sandbox filters/methods/etc. 
but I'm not sure we need to explicitly specify "default", it seems like "on" 
would be sufficient.  It also preserved compatibility with what we have now.

> 2) A more restricted whitelist environment that doesn't allow all QEMU
> features to work.  It would be limited to the whitelist in 1 and it
> would also deny things like execve(), open(), socket(), certain ioctl()
> parameters, and may only allow reads/writes to specifc fds, and/or block
> anything else that could be dangerous:
> 
> qemu-kvm -sandbox on,restricted
> 
> I'm just throwing these command line options and syscalls out there.
> And maybe it makes more sense for libvirt to pass the syscalls and
> parameters to QEMU so that libvirt can determine the parameters to
> restrict, like fd's the guest is allowed to read/write.
> 
> Here's another thread where this was discussed:
> http://www.redhat.com/archives/libvir-list/2013-April/msg01501.html

Seems reasonable to me.

-- 
paul moore
security and virtualization @ redhat

Re: [Qemu-devel] [PATCH for 1.5] Revert "target-i386: Use movcond to implement rotate flags."

[Hervé Poussineau]

Richard Henderson a écrit :

On 2013-05-01 07:06, Hervé Poussineau wrote:

This commit breaks boot of MS-DOS 6.22, which stops at:
MODE CON CODEPAGE PREPARE=((850) C:\DOS\EGA.CPI)

This reverts part of commit 34d80a55ff8517fd37bcfea5063b9797e2bd9132.

Signed-off-by: Hervé Poussineau 


I strongly suspect the bug is now fixed by 
089305ac0a273e64c9a5655d26da7fe19ecee66f and there's no need for a revert.


No, the bug is still present in e9016ee2bda1b7757072b856b2196f691aee3388.

Hervé

Re: [Qemu-devel] pause_all_vcpus() TCG bug?

[Peter Maydell]

On 1 May 2013 14:33, Andreas Färber  wrote:
> Hello,
>
> This is today's function, with annotations and question inline:
>
> void pause_all_vcpus(void)
> {
> CPUArchState *penv = first_cpu;
>
> qemu_clock_enable(vm_clock, false);
> while (penv) {
> CPUState *pcpu = ENV_GET_CPU(penv);
> pcpu->stop = true;
> qemu_cpu_kick(pcpu);
> penv = penv->next_cpu;
> }
>
> /* So, at this point penv == NULL. */
>
> if (qemu_in_vcpu_thread()) {
> cpu_stop_current();
> if (!kvm_enabled()) {
> while (penv) {
>
> /* Looks like this can never be true then? */
> /* Is penv = first_cpu; missing? */
>
> CPUState *pcpu = ENV_GET_CPU(penv);
> pcpu->stop = 0;
>
> /* 0 instead of false may hint at a mismerge... */
>
> pcpu->stopped = true;
> penv = penv->next_cpu;
> }
> return;
> }
> }

This certainly looks odd. This bug seems to have
been present since this patch was first committed
(d798e9745, January last year, by Jan.)

-- PMM

[Qemu-devel] pause_all_vcpus() TCG bug?

[Andreas Färber]

Hello,

This is today's function, with annotations and question inline:

void pause_all_vcpus(void)
{
CPUArchState *penv = first_cpu;

qemu_clock_enable(vm_clock, false);
while (penv) {
CPUState *pcpu = ENV_GET_CPU(penv);
pcpu->stop = true;
qemu_cpu_kick(pcpu);
penv = penv->next_cpu;
}

/* So, at this point penv == NULL. */

if (qemu_in_vcpu_thread()) {
cpu_stop_current();
if (!kvm_enabled()) {
while (penv) {

/* Looks like this can never be true then? */
/* Is penv = first_cpu; missing? */

CPUState *pcpu = ENV_GET_CPU(penv);
pcpu->stop = 0;

/* 0 instead of false may hint at a mismerge... */

pcpu->stopped = true;
penv = penv->next_cpu;
}
return;
}
}

while (!all_vcpus_paused()) {
qemu_cond_wait(&qemu_pause_cond, &qemu_global_mutex);
penv = first_cpu;
while (penv) {
qemu_cpu_kick(ENV_GET_CPU(penv));
penv = penv->next_cpu;
}
}
}

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] [PATCH for 1.5] Revert "target-i386: Use movcond to implement rotate flags."

[Richard Henderson]

On 2013-05-01 07:06, Hervé Poussineau wrote:

This commit breaks boot of MS-DOS 6.22, which stops at:
MODE CON CODEPAGE PREPARE=((850) C:\DOS\EGA.CPI)

This reverts part of commit 34d80a55ff8517fd37bcfea5063b9797e2bd9132.

Signed-off-by: Hervé Poussineau 


I strongly suspect the bug is now fixed by 
089305ac0a273e64c9a5655d26da7fe19ecee66f and there's no need for a revert.




r~

Re: [Qemu-devel] [PATCH v3] Throttle-down guest when live migration does not converge.

[Chegu Vinod]

On 5/1/2013 5:38 AM, Eric Blake wrote:

On 05/01/2013 06:22 AM, Chegu Vinod wrote:

Busy enterprise workloads hosted on large sized VM's tend to dirty
memory faster than the transfer rate achieved via live guest migration.
Despite some good recent improvements (& using dedicated 10Gig NICs
between hosts) the live migration does NOT converge.
---

Changes from v2:
- incorporated feedback from Orit, Juan and Eric
- stop the throttling thread at the start of stage 3
- rebased to latest qemu.git

+++ b/qapi-schema.json
@@ -600,9 +600,14 @@
  #  loads, by sending compressed difference of the pages
  #
  # Since: 1.2
+#
+# @auto-converge: Migration supports automatic throttling down of guest
+#  to force convergence. Disabled by default.
+#
+# Since: 1.6
  ##

I've already argued that ALL new migration capabilities should be
disabled by default (see the thread on 'x-rdma-pin-all', which will be a
merge conflict if it gets applied before your patch).  So I don't think
that last sentence adds anything, and can be dropped.

I think this works, although it's the first instance of having two
top-level Since: tags on a single JSON entity.  I was envisioning:

@xbzrle: yadda... pages

@auto-convert: Migration supports... convergence (since 1.6)

Since: 1.2

to match the conventions elsewhere that the overall JSON entity (the
enum MigrationCapability) exists since 1.2, but the addition of
auto-convert happened in 1.6.

However, as nothing parses the .json file to turn it into formal docs
(yet), I'm not going to insist on a respin if this is the only problem
with your patch.  I'm not comfortable enough with my skills in reviewing
the rest of the patch, or I'd offer a reviewed-by.


I shall make the suggested changes.
Appreciate your review feedback on this part of the change.

Thanks
Vinod

Re: [Qemu-devel] [PATCH v3 0/4] Initial VHDX support

[Stefan Hajnoczi]

On Mon, Apr 29, 2013 at 02:48:15PM -0400, Jeff Cody wrote:
> 
> Differences from v2:
> 
> 
> Patch 2/4:  changed 2 uint8_t[16] to MSGUID (Kevin)
> renamed all strucs to proper style (Kevin)
> indentations / style (Kevin)
> Removed/fixed outdated comments (Stefan)
> dropped vhdx_header_padded (Kevin)
> 
> Patch 3/4:  removed hunk that was meant for old patch 5/5 (Fam)
> check VHDX header version field (Fam)
> validate file type id in open and not just probe (Kevin)
> allow headers with seq # of 0 if other header is invalid (Kevin)
> fixed masking typo for parent meta present (Kevin)
> removed error check for 512-byte sectors (Kevin)
> verify region and metadata table guid are unique (Kevin)
> removed signed/unsigned comparisons (Stefan)
> sanity checking on rt.entry_count (Stefan)
> use clz_() instead of inline code (Stefan)
> bat_rt.length sanity check - compute it rather
>than trust the entry length (Stefan)
> 
> Patch 5 (v2): Dropped
> 
> This adds the initial support for VHDX image files.  
> 
> It currently only supports read operations of VHDX, for fixed and dynamic 
> files.
> 
> Notably, the following is not yet supported:
> * Differencing files
> * Log replay (so we will refuse to open any images that are not 'clean')
> * .bdrv_create()
> * write operations other than to the header
> 
> 
> Jeff Cody (4):
>   qemu: add castagnoli crc32c checksum algorithm
>   block: vhdx header for the QEMU support of VHDX images
>   block: initial VHDX driver support framework - supports open and probe
>   block: add read-only support to VHDX image format.
> 
>  block/Makefile.objs   |   1 +
>  block/vhdx.c  | 972 
> ++
>  block/vhdx.h  | 325 +
>  include/qemu/crc32c.h |  35 ++
>  util/Makefile.objs|   1 +
>  util/crc32c.c | 115 ++
>  6 files changed, 1449 insertions(+)
>  create mode 100644 block/vhdx.c
>  create mode 100644 block/vhdx.h
>  create mode 100644 include/qemu/crc32c.h
>  create mode 100644 util/crc32c.c

Do you have links to vhdx files I can test?

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

Stefan

Re: [Qemu-devel] [PATCH v3] Throttle-down guest when live migration does not converge.

[Eric Blake]

On 05/01/2013 06:22 AM, Chegu Vinod wrote:
> Busy enterprise workloads hosted on large sized VM's tend to dirty
> memory faster than the transfer rate achieved via live guest migration.
> Despite some good recent improvements (& using dedicated 10Gig NICs
> between hosts) the live migration does NOT converge.

> 
> ---
> 
> Changes from v2:
> - incorporated feedback from Orit, Juan and Eric
> - stop the throttling thread at the start of stage 3
> - rebased to latest qemu.git
> 

> +++ b/qapi-schema.json
> @@ -600,9 +600,14 @@
>  #  loads, by sending compressed difference of the pages
>  #
>  # Since: 1.2
> +#
> +# @auto-converge: Migration supports automatic throttling down of guest
> +#  to force convergence. Disabled by default.
> +#
> +# Since: 1.6
>  ##

I've already argued that ALL new migration capabilities should be
disabled by default (see the thread on 'x-rdma-pin-all', which will be a
merge conflict if it gets applied before your patch).  So I don't think
that last sentence adds anything, and can be dropped.

I think this works, although it's the first instance of having two
top-level Since: tags on a single JSON entity.  I was envisioning:

@xbzrle: yadda... pages

@auto-convert: Migration supports... convergence (since 1.6)

Since: 1.2

to match the conventions elsewhere that the overall JSON entity (the
enum MigrationCapability) exists since 1.2, but the addition of
auto-convert happened in 1.6.

However, as nothing parses the .json file to turn it into formal docs
(yet), I'm not going to insist on a respin if this is the only problem
with your patch.  I'm not comfortable enough with my skills in reviewing
the rest of the patch, or I'd offer a reviewed-by.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC][PATCH v2]Add timestamp to error message

[Daniel P. Berrange]

On Wed, May 01, 2013 at 06:16:33AM -0600, Eric Blake wrote:
> On 05/01/2013 06:05 AM, Stefan Hajnoczi wrote:
> 
> >> +error_printf(
> >> +   "%4d-%02d-%02d %02d:%02d:%02d.%03lld+ ",
> >> +   fields.tm_year + 1900, fields.tm_mon + 1, fields.tm_mday,
> >> +   fields.tm_hour, fields.tm_min, fields.tm_sec,
> >> +   now % 1000);
> > 
> > Can strftime(3) be used instead of copying code from glibc?
> 
> No, because glibc's strftime() is not async-signal safe, and therefore
> is not safe to call between fork() and exec() (libvirt hit actual
> deadlocks[1] where a child was blocked on a mutex associated with
> time-related functions that happened to be held by another parent
> thread, but where the fork nuked that other thread so the mutex would
> never clear).  This code is copied from libvirt, which copied the
> no-lock-needed safe portions of glibc for a reason.
> 
> [1] https://www.redhat.com/archives/libvir-list/2011-November/msg01609.html

NB the original libvirt code had this & other related functions in
a standalone file, along with a significant test suite. I think it
is better from a maintenence POV to keep this functionality in a
file that's separate from the qemu error file, so it can be reused
elsewhere in QEMU if needed. It should also copy the test suite,
since it is bad practice to throw away tests which already exist.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] Michael Tokarev taking over trivial-patches queue

[Anthony Liguori]

Stefan Hajnoczi  writes:

> Michael Tokarev is taking over maintainership of the trivial-patches
> queue starting today.  Michael maintains KVM in Debian and
> participates on IRC and the mailing lists.

Thanks Michael!

>
> The trivial patches queue is a dedicated patch queue for small patches
> which may or may not fall into an actively maintained QEMU subsystem.
> It was created because too many patches were falling through the
> cracks even though they were simple and suitable for merge.
>
> Pull requests have been sent roughly weekly since April 7th, 2011.
>
> For more info on the trivial-patches queue, see:
> http://qemu-project.org/Contribute/TrivialPatches
>
> (This page will be updated when mjt chooses a public repo for his pull
> requests.)
>
> Thank you Michael!

Thanks for maintaining trivial for so long Stefan!

Regards,

Anthony Liguori

>
> Stefan

[Qemu-devel] [PATCH v3] Throttle-down guest when live migration does not converge.

[Chegu Vinod]

Busy enterprise workloads hosted on large sized VM's tend to dirty
memory faster than the transfer rate achieved via live guest migration.
Despite some good recent improvements (& using dedicated 10Gig NICs
between hosts) the live migration does NOT converge.

If a user chooses to force convergence of their migration via a new
migration capability "auto-converge" then this change will auto-detect
lack of convergence scenario and trigger a slow down of the workload
by explicitly disallowing the VCPUs from spending much time in the VM
context.

The migration thread tries to catchup and this eventually leads
to convergence in some "deterministic" amount of time. Yes it does
impact the performance of all the VCPUs but in my observation that
lasts only for a short duration of time. i.e. we end up entering
stage 3 (downtime phase) soon after that. No external trigger is
required.

Thanks to Juan and Paolo for their useful suggestions.

Verified the convergence using the following :
- SpecJbb2005 workload running on a 20VCPU/256G guest(~80% busy)
- OLTP like workload running on a 80VCPU/512G guest (~80% busy)

Sample results with SpecJbb2005 workload : (migrate speed set to 20Gb and
migrate downtime set to 4seconds).

(qemu) info migrate
capabilities: xbzrle: off auto-converge: off  <
Migration status: active
total time: 1487503 milliseconds
expected downtime: 519 milliseconds
transferred ram: 383749347 kbytes
remaining ram: 2753372 kbytes
total ram: 268444224 kbytes
duplicate: 65461532 pages
skipped: 64901568 pages
normal: 95750218 pages
normal bytes: 383000872 kbytes
dirty pages rate: 67551 pages

---

(qemu) info migrate
capabilities: xbzrle: off auto-converge: on   <
Migration status: completed
total time: 241161 milliseconds
downtime: 6373 milliseconds
transferred ram: 28235307 kbytes
remaining ram: 0 kbytes
total ram: 268444224 kbytes
duplicate: 64946416 pages
skipped: 64903523 pages
normal: 7044971 pages
normal bytes: 28179884 kbytes

---

Changes from v2:
- incorporated feedback from Orit, Juan and Eric
- stop the throttling thread at the start of stage 3
- rebased to latest qemu.git

Changes from v1:
- rebased to latest qemu.git
- added auto-converge capability(default off) - suggested by Anthony Liguori &
Eric Blake.

Signed-off-by: Chegu Vinod 
---
 arch_init.c   |   61 +
 cpus.c|   12 
 include/migration/migration.h |7 +
 include/qemu/main-loop.h  |3 ++
 kvm-all.c |   46 +++
 migration.c   |   18 
 qapi-schema.json  |7 -
 7 files changed, 153 insertions(+), 1 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 49c5dc2..7e03b2c 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -104,6 +104,7 @@ int graphic_depth = 15;
 #endif
 
 const uint32_t arch_type = QEMU_ARCH;
+static bool mig_throttle_on;
 
 /***/
 /* ram save/restore */
@@ -379,7 +380,14 @@ static void migration_bitmap_sync(void)
 MigrationState *s = migrate_get_current();
 static int64_t start_time;
 static int64_t num_dirty_pages_period;
+static int64_t bytes_xfer_prev;
 int64_t end_time;
+int64_t bytes_xfer_now;
+static int dirty_rate_high_cnt;
+
+if (!bytes_xfer_prev) {
+bytes_xfer_prev = ram_bytes_transferred();
+}
 
 if (!start_time) {
 start_time = qemu_get_clock_ms(rt_clock);
@@ -404,6 +412,27 @@ static void migration_bitmap_sync(void)
 
 /* more than 1 second = 1000 millisecons */
 if (end_time > start_time + 1000) {
+if (migrate_auto_converge()) {
+/* The following detection logic can be refined later. For now:
+   Check to see if the dirtied bytes is 50% more than the approx.
+   amount of bytes that just got transferred since the last time we
+   were in this routine. If that happens N times (for now N==5)
+   we turn on the throttle down logic */
+bytes_xfer_now = ram_bytes_transferred();
+if (s->dirty_pages_rate &&
+((num_dirty_pages_period*TARGET_PAGE_SIZE) >
+((bytes_xfer_now - bytes_xfer_prev)/2))) {
+if (dirty_rate_high_cnt++ > 5) {
+DPRINTF("Unable to converge. Throtting down guest\n");
+qemu_mutex_lock_mig_throttle();
+if (!mig_throttle_on) {
+mig_throttle_on = true;
+}
+qemu_mutex_unlock_mig_throttle();
+}
+ }
+ bytes_xfer_prev = bytes_xfer_now;
+}
 s->dirty_pages_rate = num_dirty_pages_period * 1000
 / (end_time - start_time);
 s->dirty_bytes_rate = s->dirty_pages_rate * TARGET_PAGE_SIZE;
@@ -496,6 +525,33 @@ static in

Re: [Qemu-devel] [RFC][PATCH v2]Add timestamp to error message

[Eric Blake]

On 05/01/2013 06:05 AM, Stefan Hajnoczi wrote:

>> +error_printf(
>> +   "%4d-%02d-%02d %02d:%02d:%02d.%03lld+ ",
>> +   fields.tm_year + 1900, fields.tm_mon + 1, fields.tm_mday,
>> +   fields.tm_hour, fields.tm_min, fields.tm_sec,
>> +   now % 1000);
> 
> Can strftime(3) be used instead of copying code from glibc?

No, because glibc's strftime() is not async-signal safe, and therefore
is not safe to call between fork() and exec() (libvirt hit actual
deadlocks[1] where a child was blocked on a mutex associated with
time-related functions that happened to be held by another parent
thread, but where the fork nuked that other thread so the mutex would
never clear).  This code is copied from libvirt, which copied the
no-lock-needed safe portions of glibc for a reason.

[1] https://www.redhat.com/archives/libvir-list/2011-November/msg01609.html

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC][PATCH v2]Add timestamp to error message

[Stefan Hajnoczi]

On Mon, Apr 29, 2013 at 03:57:25PM -0400, Seiji Aguchi wrote:
> diff --git a/util/qemu-error.c b/util/qemu-error.c
> index 08a36f4..35ef9ab 100644
> --- a/util/qemu-error.c
> +++ b/util/qemu-error.c
> @@ -196,6 +196,96 @@ void error_print_loc(void)
>  }
>  }
>  
> +
> +#define SECS_PER_HOUR   (60 * 60)
> +#define SECS_PER_DAY(SECS_PER_HOUR * 24)
> +#define DIV(a, b) ((a) / (b) - ((a) % (b) < 0))
> +#define LEAPS_THRU_END_OF(y) (DIV(y, 4) - DIV(y, 100) + DIV(y, 400))
> +
> +const unsigned short int __mon_yday[2][13] = {
> +/* Normal years.  */
> +{ 0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365 },
> +/* Leap years.  */
> +{ 0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366 }
> +};
> +
> +#define is_leap_year(y) \
> +((y) % 4 == 0 && ((y) % 100 != 0 || (y) % 400 == 0))
> +
> +static void error_print_timestamp(void)
> +{
> +struct timespec ts;
> +unsigned long long now;
> +
> +struct tm fields;
> +long int days, rem, y;
> +const unsigned short int *ip;
> +unsigned long long whenSecs;
> +unsigned int offset = 0; /* We hardcoded GMT */
> +
> +if (clock_gettime(CLOCK_REALTIME, &ts) < 0) {
> +return;
> +}
> +
> +now = (ts.tv_sec * 1000ull) + (ts.tv_nsec / (1000ull * 1000ull));
> +/* This code is taken from GLibC under terms of LGPLv2+ */
> +
> +whenSecs = now / 1000ull;
> +
> +days = whenSecs / SECS_PER_DAY;
> +rem = whenSecs % SECS_PER_DAY;
> +rem += offset;
> +while (rem < 0) {
> +rem += SECS_PER_DAY;
> +--days;
> +}
> +while (rem >= SECS_PER_DAY) {
> +rem -= SECS_PER_DAY;
> +++days;
> +}
> +fields.tm_hour = rem / SECS_PER_HOUR;
> +rem %= SECS_PER_HOUR;
> +fields.tm_min = rem / 60;
> +fields.tm_sec = rem % 60;
> +/* January 1, 1970 was a Thursday.  */
> +fields.tm_wday = (4 + days) % 7;
> +if (fields.tm_wday < 0) {
> +fields.tm_wday += 7;
> +}
> +y = 1970;
> +
> +while (days < 0 || days >= (is_leap_year(y) ? 366 : 365)) {
> +/* Guess a corrected year, assuming 365 days per year.  */
> +long int yg = y + days / 365 - (days % 365 < 0);
> +
> +  /* Adjust DAYS and Y to match the guessed year.  */
> +  days -= ((yg - y) * 365
> +   + LEAPS_THRU_END_OF(yg - 1)
> +   - LEAPS_THRU_END_OF(y - 1));
> +  y = yg;
> +}
> +fields.tm_year = y - 1900;
> +
> +fields.tm_yday = days;
> +ip = __mon_yday[is_leap_year(y)];
> +for (y = 11; days < (long int) ip[y]; --y) {
> +continue;
> +}
> +
> +days -= ip[y];
> +fields.tm_mon = y;
> +fields.tm_mday = days + 1;
> +
> +error_printf(
> +   "%4d-%02d-%02d %02d:%02d:%02d.%03lld+ ",
> +   fields.tm_year + 1900, fields.tm_mon + 1, fields.tm_mday,
> +   fields.tm_hour, fields.tm_min, fields.tm_sec,
> +   now % 1000);

Can strftime(3) be used instead of copying code from glibc?

Re: [Qemu-devel] [PATCH 2/3] block: add block_backup QMP command

[Stefan Hajnoczi]

On Mon, Apr 29, 2013 at 09:51:47AM -0600, Eric Blake wrote:
> On 04/29/2013 03:27 AM, Paolo Bonzini wrote:
> > Il 29/04/2013 09:21, Stefan Hajnoczi ha scritto:
> >>> I'd really love to see us change 'BlockJobInfo' to use an enum for
> >>> 'type', instead of its open-coded 'str'.  Likewise, the block-job
> >>> related events in QMP/qmp-events.txt should be updated to refer to the
> >>> enum instead of also being open-coded 'str'.
> >>
> >> Since the block job QMP API has been in released I'm not sure changing
> >> this is worthwhile.  QEMU and libvirt would have to maintain
> >> compatibility so the code will just be duplicated.
> > 
> > I don't think this would change the actual data on the wire.  However,
> > it would let libvirt know the supported block job types by introspecting
> > the enum.
> 
> Until we have introspection, the point is moot.  When we have
> introspection, libvirt would much rather see an enum than a 'str'.  I
> see absolutely no back-compat problem in changing the code to be
> type-safe prior to the point that introspection is added.

Okay, I haven't looked at how QAPI enums are implemented.  If we can
move from custom strings to enum without breaking existing libvirt, then
great!

Stefan

Re: [Qemu-devel] [RFC][PATCH v3 00/24] instrument: Let the user wrap/override specific event tracing routines

[Stefan Hajnoczi]

On Sun, Apr 28, 2013 at 09:25:15PM +0200, Lluís Vilanova wrote:
> Stefan Hajnoczi writes:
> 
> > On Fri, Apr 26, 2013 at 02:15:46PM +0200, Lluís Vilanova wrote:
> > Advanced users will have to modify the QEMU source.
> 
> >> > Basically I think putting a stable API in place here isn't going to fly.
> >> > In terms of the tracing subsystem I don't mind, but I think it's a
> >> > question for the larger QEMU community.
> >> 
> >> What are you referring to as an API? The tracing events and their 
> >> signatures?
> 
> > The interface that dynamic instrumentation libraries use to interact
> > with QEMU.  That means "stable" tracing events plus any operations that
> > libraries can perform (stop/stop vcpu, peek/poke memory, dirty bitmaps,
> > MMU, interrupts, etc).
> 
> Not all events require being "stable", just a few that I established as
> "abstract" and relate to some very generic guest state (which in their most
> basic form can be boiled down to 5). As for the rest of the API and more
> controversial events, as I said I have no problem in maintaining a separate
> branch.

Can you split the work into two patch series:
1. Target-independent TCG events (e.g. vbbl_begin)
2. "custom" trace backend that links custom C trace event handler
   functions - but without dynamic libraries or stable API.

It loses the dynamic library and stable API features but would be easy
to merge.

Stefan

[Qemu-devel] Michael Tokarev taking over trivial-patches queue

[Stefan Hajnoczi]

Michael Tokarev is taking over maintainership of the trivial-patches
queue starting today.  Michael maintains KVM in Debian and
participates on IRC and the mailing lists.

The trivial patches queue is a dedicated patch queue for small patches
which may or may not fall into an actively maintained QEMU subsystem.
It was created because too many patches were falling through the
cracks even though they were simple and suitable for merge.

Pull requests have been sent roughly weekly since April 7th, 2011.

For more info on the trivial-patches queue, see:
http://qemu-project.org/Contribute/TrivialPatches

(This page will be updated when mjt chooses a public repo for his pull
requests.)

Thank you Michael!

Stefan

Re: [Qemu-devel] [PATCH] Fix PReP NIP reset value

[Andreas Färber]

Am 30.04.2013 17:07, schrieb Fabien Chouteau:
> The value was changed by the "PPC: fix hreset_vector..." patch.
> 
> Signed-off-by: Fabien Chouteau 
> ---
>  hw/ppc/prep.c |3 +++
>  1 file changed, 3 insertions(+)

Thanks, applied to prep-up (with enhanced commit message):
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/prep-up

Andreas

> 
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index cceab3e..2d0c4fe 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -427,6 +427,9 @@ static void ppc_prep_reset(void *opaque)
>  PowerPCCPU *cpu = opaque;
>  
>  cpu_reset(CPU(cpu));
> +
> +/* Reset address */
> +cpu->env.nip = 0xfffc;
>  }
>  
>  /* PowerPC PREP hardware initialisation */
> 

Re: [Qemu-devel] [RFC 7/7] target-i386: Disable direct passthrough of PMU CPUID leaf by default

[Andreas Färber]

Am 30.04.2013 21:57, schrieb Eduardo Habkost:
> On Tue, Apr 30, 2013 at 07:04:23PM +0200, Igor Mammedov wrote:
>> It's impossible to implement "n270  movbe" fixup with compat props currently,
>> so 5/7 + 1/7 looks ok. We can drop them once features converted to static 
>> properties
>> + sub-classes.
> 
> True. But why this doesn't apply to 6/7 (486 model=8) as well?

Eduardo, this series is "RFC". Are you going to grant permission to pick
individual agreed-on patches from it or are you planning to resend?

Andreas

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

Re: [Qemu-devel] [PATCH v4 0/6] vmdk: zeroed-grain GTE support

[Stefan Hajnoczi]

On Sun, Apr 28, 2013 at 11:27:57AM +0800, Fam Zheng wrote:
> Added support for zeroed-grain GTE to VMDK according to VMDK Spec 5.0[1].
> 
> [1] Virtual Disk Format 5.0 - VMware,
> http://www.vmware.com/support/developer/vddk/vmdk_50_technote.pdf?src=vmdk
> 
> Changes since v3:
>  - 5/6: Remove tmp
>  - 6/6: Update L2 Cache
> Remove redundant assignment to m_data
> Simplify l2_offset assignment to m_data
> Fix m_data.offset endian.
> 
> Changes since v2:
>  - all: Added 5/6 (vmdk: store fields of VmdkMetaData in cpu endian)
>  - 6/6: Avoid side-effect of vmdk_L2update.
> Change function comment to gtkdoc stype.
> Fix VMDK4_FLAG_ZG.
> 
> Changes since v1:
>  - all: fix From: field
>  - 1/5: squash one line of ret code macro change from 2/5
>  - 2/5: change VMDK4_FLAG_ZG to VMDK4_FLAG_ZERO_GRAIN
>  - 3/5: move BLOCK_OPT_ZEROED_GRAIN defination from block_int.h to vmdk.c
>  - 5/5: fix metadata update issue, unit test with cases 033 034
> 
> Fam Zheng (6):
>   vmdk: named return code.
>   vmdk: add support for “zeroed‐grain” GTE
>   vmdk: Add option to create zeroed-grain image
>   vmdk: change magic number to macro
>   vmdk: store fields of VmdkMetaData in cpu endian
>   vmdk: add bdrv_co_write_zeroes
> 
>  block/vmdk.c | 208 
> +--
>  1 file changed, 145 insertions(+), 63 deletions(-)

One issue with Patch 6.

Please send full patch series in future revisions.  This makes it easier
to review and apply without having to search for the previous thread.

Stefan

Re: [Qemu-devel] [PATCH v4 6/6] vmdk: add bdrv_co_write_zeroes

[Stefan Hajnoczi]

On Sun, Apr 28, 2013 at 11:27:59AM +0800, Fam Zheng wrote:
> @@ -835,6 +836,9 @@ static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData 
> *m_data)
>  return VMDK_ERROR;
>  }
>  }
> +if (m_data->l2_cache_entry) {
> +*m_data->l2_cache_entry = m_data->offset;
> +}

l2_table is little-endian, use the local variable 'offset' instead of
m_data->offset.

Re: [Qemu-devel] [PATCH] linux-user: Fix MIPS16/microMIPS signal handling

[Peter Maydell]

On 30 April 2013 19:09, Kwok Cheung Yeung  wrote:
> Signal handlers written using a compressed MIPS instruction
> set will segfault when invoked.  This patch fixes this.
>
> Switch the ISA mode on cores supporting the MIPS16/microMIPS
> ISAs according to bit 0 of the signal handler address.  Clear
> bit 0 of the address assigned to the PC.

Don't you also need to handle bit-0-set in restore_sigcontext
when returning from the signal? (I guess that might cause
a crash if you have a non-compressed-instruction-set signal
handler invoked while running compressed-instruction--set code.)

>
> Signed-off-by: Kwok Cheung Yeung 
> ---
>  linux-user/signal.c | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 1055507..abfb382 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -2662,6 +2662,11 @@ static void setup_frame(int sig, struct 
> target_sigaction * ka,
>  * since it returns to userland using eret
>  * we cannot do this here, and we must set PC directly */
>  regs->active_tc.PC = regs->active_tc.gpr[25] = ka->_sa_handler;
> +if (regs->insn_flags & (ASE_MIPS16 | ASE_MICROMIPS)) {
> +regs->hflags &= ~MIPS_HFLAG_M16;
> +regs->hflags |= (ka->_sa_handler & 1) << MIPS_HFLAG_M16_SHIFT;
> +regs->active_tc.PC &= ~(target_ulong) 1;
> +}
>  unlock_user_struct(frame, frame_addr, 1);
>  return;
>
> @@ -2771,6 +2776,11 @@ static void setup_rt_frame(int sig, struct 
> target_sigaction *ka,
>  * since it returns to userland using eret
>  * we cannot do this here, and we must set PC directly */
>  env->active_tc.PC = env->active_tc.gpr[25] = ka->_sa_handler;
> +if (env->insn_flags & (ASE_MIPS16 | ASE_MICROMIPS)) {
> +env->hflags &= ~MIPS_HFLAG_M16;
> +env->hflags |= (ka->_sa_handler & 1) << MIPS_HFLAG_M16_SHIFT;
> +env->active_tc.PC &= ~(target_ulong) 1;
> +}
>  unlock_user_struct(frame, frame_addr, 1);
>  return;
>
> --
> 1.8.2.2
>
>

-- PMM

[Qemu-devel] [PATCH 2/2] target-ppc: Add read and write of PPR SPR

[Anton Blanchard]

Recent Linux kernels save and restore the PPR across exceptions
so we need to handle it.

Signed-off-by: Anton Blanchard 
---

Index: b/target-ppc/translate_init.c
===
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7010,6 +7010,10 @@ static void init_proc_POWER7 (CPUPPCStat
  &spr_read_generic, &spr_write_generic,
  &spr_read_generic, &spr_write_generic,
  0x);
+spr_register(env, SPR_PPR, "PPR",
+ &spr_read_generic, &spr_write_generic,
+ &spr_read_generic, &spr_write_generic,
+ 0x);
 #if !defined(CONFIG_USER_ONLY)
 env->slb_nr = 32;
 #endif

[Qemu-devel] [PATCH 1/2] target-ppc: Fix invalid SPR read/write warnings

[Anton Blanchard]

Invalid and privileged SPR warnings currently print the wrong
address. While fixing that, also make it clear that we are
printing both the decimal and hexadecimal SPR number.

Before:

  Trying to read invalid spr 896 380 at 0714

After:

  Trying to read invalid spr 896 (0x380) at 0710

Signed-off-by: Anton Blanchard 
---

Index: b/target-ppc/translate.c
===
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -4005,19 +4005,19 @@ static inline void gen_op_mfspr(DisasCon
  * allowing userland application to read the PVR
  */
 if (sprn != SPR_PVR) {
-qemu_log("Trying to read privileged spr %d %03x at "
- TARGET_FMT_lx "\n", sprn, sprn, ctx->nip);
-printf("Trying to read privileged spr %d %03x at "
-   TARGET_FMT_lx "\n", sprn, sprn, ctx->nip);
+qemu_log("Trying to read privileged spr %d (0x%03x) at "
+ TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
+printf("Trying to read privileged spr %d (0x%03x) at "
+   TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
 }
 gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
 }
 } else {
 /* Not defined */
-qemu_log("Trying to read invalid spr %d %03x at "
-TARGET_FMT_lx "\n", sprn, sprn, ctx->nip);
-printf("Trying to read invalid spr %d %03x at " TARGET_FMT_lx "\n",
-   sprn, sprn, ctx->nip);
+qemu_log("Trying to read invalid spr %d (0x%03x) at "
+ TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
+printf("Trying to read invalid spr %d (0x%03x) at "
+   TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
 gen_inval_exception(ctx, POWERPC_EXCP_INVAL_SPR);
 }
 }
@@ -4150,18 +4150,18 @@ static void gen_mtspr(DisasContext *ctx)
 (*write_cb)(ctx, sprn, rS(ctx->opcode));
 } else {
 /* Privilege exception */
-qemu_log("Trying to write privileged spr %d %03x at "
- TARGET_FMT_lx "\n", sprn, sprn, ctx->nip);
-printf("Trying to write privileged spr %d %03x at " TARGET_FMT_lx
-   "\n", sprn, sprn, ctx->nip);
+qemu_log("Trying to write privileged spr %d (0x%03x) at "
+ TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
+printf("Trying to write privileged spr %d (0x%03x) at "
+   TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
 gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
 }
 } else {
 /* Not defined */
-qemu_log("Trying to write invalid spr %d %03x at "
- TARGET_FMT_lx "\n", sprn, sprn, ctx->nip);
-printf("Trying to write invalid spr %d %03x at " TARGET_FMT_lx "\n",
-   sprn, sprn, ctx->nip);
+qemu_log("Trying to write invalid spr %d (0x%03x) at "
+ TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
+printf("Trying to write invalid spr %d (0x%03x) at "
+   TARGET_FMT_lx "\n", sprn, sprn, ctx->nip - 4);
 gen_inval_exception(ctx, POWERPC_EXCP_INVAL_SPR);
 }
 }

Re: [Qemu-devel] [PATCH v2 02/12] ARM: Prepare translation for AArch64 code

[Peter Maydell]

On 1 May 2013 11:19, Laurent Desnogues  wrote:
> On Wednesday, May 1, 2013, Richard Henderson  wrote:
>> On 2013-04-30 07:36, John Rigby wrote:
>>>
>>>   uint32_t regs[16];
>>> +
>>> +/* Regs for A64 mode.  */
>>> +uint64_t xregs[31];
>>> +uint64_t pc;
>>> +uint64_t sp;
>>> +uint32_t pstate;
>>> +uint32_t aarch64_state; /* 1 if CPU is in aarch64 state */
>>> +
>>
>> How do these registers overlap (or not) in real hardware?
>> Is it possible to union these with the 32-bit state?
>
> There is an overlap between 32- and 64-bit state registers,
> but it's against the set of 32 (?) 32-bit registers that exist
> across all modes, not against the 16 registers as used here.
>
> IMHO it doesn't make sense to union them, the mapping
> can be done when switching from 32- to 64-bit modes.

Agreed -- the 32/64 switch only ever happens when taking
or returning from an exception, so it is easier to get
the overlap semantics right with explicit code there
rather than try to change all the existing 32 bit code
to implicitly get things right. Plus as you say the
interaction with aarch32 register banking would make
unioning tricky.

-- PMM

Re: [Qemu-devel] [PATCH v2 02/12] ARM: Prepare translation for AArch64 code

[Laurent Desnogues]

On Wednesday, May 1, 2013, Richard Henderson  wrote:
> On 2013-04-30 07:36, John Rigby wrote:
>>
>>   uint32_t regs[16];
>> +
>> +/* Regs for A64 mode.  */
>> +uint64_t xregs[31];
>> +uint64_t pc;
>> +uint64_t sp;
>> +uint32_t pstate;
>> +uint32_t aarch64_state; /* 1 if CPU is in aarch64 state */
>> +
>
> How do these registers overlap (or not) in real hardware?
> Is it possible to union these with the 32-bit state?

There is an overlap between 32- and 64-bit state registers,
but it's against the set of 32 (?) 32-bit registers that exist
across all modes, not against the 16 registers as used here.

IMHO it doesn't make sense to union them, the mapping
can be done when switching from 32- to 64-bit modes.


Laurent

Re: [Qemu-devel] [PATCH v2 00/12] AArch64 preparation patch set

[Richard Henderson]

On 2013-04-30 14:15, Peter Maydell wrote:

(add --suppress-cc, --from, --cc, etc to taste, and you can
set these defaults in your .gitconfig rather than using command
line arguments. --dry-run is also a useful send-email option.)


Also helpful is setting some defaults in your qemu/.git/config.

E.g. I have the --to=qemu-devel set there.  All that's required
is that you run the git-send-email from within the qemu source tree.


r~

Re: [Qemu-devel] [PATCH v2 02/12] ARM: Prepare translation for AArch64 code

[Richard Henderson]

On 2013-04-30 07:36, John Rigby wrote:

  uint32_t regs[16];
+
+/* Regs for A64 mode.  */
+uint64_t xregs[31];
+uint64_t pc;
+uint64_t sp;
+uint32_t pstate;
+uint32_t aarch64_state; /* 1 if CPU is in aarch64 state */
+


How do these registers overlap (or not) in real hardware?
Is it possible to union these with the 32-bit state?


r~