Re: [Qemu-devel] [PATCH v2] configure: preserve various environment variables in config.status

[Eric Blake]

On 11/18/2015 03:31 AM, Daniel P. Berrange wrote:
> The config.status script is auto-generated by configure upon
> completion. The intention is that config.status can be later
> invoked by the developer to re-detect the same environment
> that configure originally used. The current config.status
> script, however, only contains a record of the command line
> arguments to configure. Various environment variables have
> an effect on what configure will find. In particular the
> PKG_CONFIG_LIBDIR & PKG_CONFIG_PATH vars will affect what
> libraries pkg-config finds. The PATH var will affect what
> toolchain binaries and -config scripts are found. The
> LD_LIBRARY_PATH var will affect what libraries are found.
> Most commands have env variables that will override the
> name/path of the default version configure finds. All
> these key env variables should be recorded in the
> config.status script.

Would be worth mentioning in the commit message that we are explicitly
deviating from autoconf's practice of preserving CFLAGS.

> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  configure | 38 ++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/configure b/configure
> index d7472d7..09a503c 100755
> --- a/configure
> +++ b/configure
> @@ -5925,6 +5925,44 @@ cat   # Compiler output produced by configure, useful for debugging
>  # configure, is in config.log if it exists.
>  EOD
> +
> +preserve_env() {
> +envname=$1
> +
> +eval envval=\$$envname
> +
> +if test -n "$envval"
> +then
> + echo "$envname='$envval'" >> config.status

Works as long as $envval doesn't contain '; I'm not sure if it is worth
worrying about someone compiling in /path/to'evil/location.

> + echo "export $envname" >> config.status
> +fi

You missed Stefan Weil's suggestion of an else clause to unset variables
that were added after the fact but not present during the original
configure.  I'm not sure if that matters.

> +}
> +
> +# Preserve various env variables that influence what
> +# features/build target configure will detect
> +preserve_env AR
> +preserve_env AS
> +preserve_env CC
> +preserve_env CPP
> +preserve_env CXX
> +preserve_env INSTALL
> +preserve_env LD
> +preserve_env LD_LIBRARY_PATH
> +preserve_env LIBTOOL
> +preserve_env MAKE
> +preserve_env NM
> +preserve_env OBJCOPY
> +preserve_env PATH
> +preserve_env PKG_CONFIG
> +preserve_env PKG_CONFIG_LIBDIR
> +preserve_env PKG_CONFIG_PATH
> +preserve_env PYTHON
> +preserve_env SDL_CONFIG
> +preserve_env SDL2_CONFIG
> +preserve_env SMBD
> +preserve_env STRIP
> +preserve_env WINDRES
> +
>  printf "exec" >>config.status
>  printf " '%s'" "$0" "$@" >>config.status
>  echo >>config.status
> 

It's better than before, even if we could think of further tweaks.  If
you enhance the commit message, I can live with:
Reviewed-by: Eric Blake 


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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] vfio/pci-quirks: Only quirk to size of PCI config space

[Alex Williamson]

On Wed, 2015-11-18 at 10:36 +0100, Laszlo Ersek wrote:
> On 11/18/15 00:41, Alex Williamson wrote:
> > For quirks that support the full PCIe extended config space, limit the
> > quirk to only the size of config space available through vfio.  This
> > allows host systems with broken MMCONFIG regions to still make use of
> > these quirks without generating bad address faults trying to access
> > beyond the end of config space exposed through vfio.  This may expose
> > direct access to parts of extended config space that we'd prefer not
> > to expose, but that's why we have an IOMMU.
> > 
> > Signed-off-by: Alex Williamson 
> > Reported-by: Ronnie Swanink 
> > ---
> >  hw/vfio/pci-quirks.c |6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> > index 30c68a1..e117c41 100644
> > --- a/hw/vfio/pci-quirks.c
> > +++ b/hw/vfio/pci-quirks.c
> > @@ -328,7 +328,7 @@ static void vfio_probe_ati_bar4_quirk(VFIOPCIDevice 
> > *vdev, int nr)
> >  window->data_offset = 4;
> >  window->nr_matches = 1;
> >  window->matches[0].match = 0x4000;
> > -window->matches[0].mask = PCIE_CONFIG_SPACE_SIZE - 1;
> > +window->matches[0].mask = vdev->config_size - 1;
> >  window->bar = nr;
> >  window->addr_mem = >mem[0];
> >  window->data_mem = >mem[1];
> > @@ -674,7 +674,7 @@ static void vfio_probe_nvidia_bar5_quirk(VFIOPCIDevice 
> > *vdev, int nr)
> >  window->matches[0].match = 0x1800;
> >  window->matches[0].mask = PCI_CONFIG_SPACE_SIZE - 1;
> >  window->matches[1].match = 0x88000;
> > -window->matches[1].mask = PCIE_CONFIG_SPACE_SIZE - 1;
> > +window->matches[1].mask = vdev->config_size - 1;
> >  window->bar = nr;
> >  window->addr_mem = bar5->addr_mem = >mem[0];
> >  window->data_mem = bar5->data_mem = >mem[1];
> > @@ -765,7 +765,7 @@ static void vfio_probe_nvidia_bar0_quirk(VFIOPCIDevice 
> > *vdev, int nr)
> >  memory_region_init_io(mirror->mem, OBJECT(vdev),
> >_nvidia_mirror_quirk, mirror,
> >"vfio-nvidia-bar0-88000-mirror-quirk",
> > -  PCIE_CONFIG_SPACE_SIZE);
> > +  vdev->config_size);
> >  memory_region_add_subregion_overlap(>bars[nr].region.mem,
> >  mirror->offset, mirror->mem, 1);
> >  
> > 
> > 
> 
> $ git log -- hw/vfio/pci-quirks.c | grep Reviewed-by
> 
> 
> Okay, I guess my review won't be deemed immediately unnecessary then. :)

Reviews are very much appreciated.

> (1) Please add to the commit message:
> 
> Link: https://www.redhat.com/archives/vfio-users/2015-November/msg00192.html
> 
> With that reference:
> 
> Reviewed-by: Laszlo Ersek 

Done

> (2) Also, one question just to make sure I understand right: the last
> sentence of the commit message means that the left-inclusive,
> right-exclusive config space offset range
> 
>   [vdev->config_size, PCIE_CONFIG_SPACE_SIZE)
> 
> is now directly available to the guest, without QEMU noticing; but,
> we're not worried about that, because the guest can't abuse that freedom
> anyway for doing arbitrary DMA. Is that right?
> 
> If so, then I propose another update to the commit message (replacing
> the last sentence):
> 
> This may grant the guest direct access to trailing parts of
> extended config space that we'd prefer not to expose, but that's
> why we have an IOMMU (the guest can't abuse said config space access
> for arbitrary DMA).
> 
> Perhaps the above is trivial for you (assuming it is correct at all...);
> it may not be trivial for others.

How's this?:

commit b27c4f5da92a1732a77ddbe98571583a4eac1e14
Author: Alex Williamson 
Date:   Tue Nov 17 16:37:38 2015 -0700

vfio/pci-quirks: Only quirk to size of PCI config space

For quirks that support the full PCIe extended config space, limit the
quirk to only the size of config space available through vfio.  This
allows host systems with broken MMCONFIG regions to still make use of
these quirks without generating bad address faults trying to access
beyond the end of config space exposed through vfio.  This may expose
direct access to the mirror of extended config space, only trapping
the sub-range of standard config space, but allowing this makes the
quirk, and thus the device, functional.  We expect that only device
specific accesses make use of the mirror, not general extended PCI
capability accesses, so any virtualization in this space is likely
unnecessary anyway, and the device is still IOMMU isolated, so it
should only be able to hurt itself through any bogus configurations
enabled by this space.

Link: https://www.redhat.com/archives/vfio-users/2015-November/msg00192.html
Reported-by: Ronnie Swanink 

Re: [Qemu-devel] [PATCH v12 17/36] qapi: Fix c_name() munging

[Eric Blake]

On 11/18/2015 03:18 AM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> *** WARNING: THE ATTACHED DOCUMENT(S) CONTAIN MACROS ***
>> *** MACROS MAY CONTAIN MALICIOUS CODE ***
>> *** Open only if you can verify and trust the sender ***
>> *** Please contact info...@redhat.com if you have questions or concerns **
> 
> Another one.
> 
>> The method c_name() is supposed to do two different actions: munge
>> '-' into '_', and add a 'q_' prefix to ticklish names.  But it did
>> these steps out of order, making it possible to submit input that
>> is not ticklish until after munging, where the output then lacked
>> the desired prefix.
>>
>> The failure is exposed easily if you have a compiler that recognizes
>> C11 keywords, and try to name a member '_Thread-local', as it would
>> result in trying to compile the declaration 'uint64_t _Thread_local;'
>> which is not valid.  However, this name violates our conventions
>> (ultimately, want to enforce that no qapi names start with single
>> underscore), so the test is slightly weaker by instead testing
>> 'wchar-t'; the declaration 'uint64_t wchar_t;' is valid in C (where
>> wchar_t is only a typedef) but fails with a C++ compiler (where it
>> is a keyword).
> 
> Do we support compiling it with a C++ compiler?  To sidestep the
> question, I'd say "but would fail".

I know we require a C++ compiler for qga on Windows, and qga uses qapi,
so I think the problem is real; but as I do not have a working setup for
compiling qga for windows, I can only speculate.  Changing s/fails/but
would fail/ is a nice hedge.

> 
> In my private opinion, the one sane way to compile C code with a C++
> compiler is wrapping it in extern "C" { ... }.

True - but I don't think that changes the set of C++ keywords inside the
extern block, does it?  (The thought of context-sensitive keywords makes
me cringe for how one would write a sane parser for that language).

> 
>> Fix things by reversing the order of actions within c_name().
>>
>> Signed-off-by: Eric Blake 
> 
> Again, just commit message polish, can do on merge.
> 

Don't know why you got it on some messages and not others; I also got a
round of those pollutions on other threads.  It looks like the
responsible party has cleaned up their false positives in the meantime,
so hopefully we aren't hit by more of the annoyance.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v12 19/36] blkdebug: Merge hand-rolled and qapi BlkdebugEvent enum

[Eric Blake]

On 11/18/2015 05:08 AM, Kevin Wolf wrote:
> Am 18.11.2015 um 11:30 hat Markus Armbruster geschrieben:
>> Eric Blake  writes:
>>
>>> No need to keep two separate enums, where editing one is likely
>>> to forget the other.  Now that we can specify a qapi enum prefix,
>>> we don't even have to change the bulk of the uses.
>>>

>>>  ##
>>> -{ 'enum': 'BlkdebugEvent',
>>> +{ 'enum': 'BlkdebugEvent', 'prefix': 'BLKDBG',
>>>'data': [ 'l1_update', 'l1_grow.alloc_table', 'l1_grow.write_table',
>>>  'l1_grow.activate_table', 'l2_load', 'l2_update',
>>>  'l2_update_compressed', 'l2_alloc.cow_read', 'l2_alloc.write',
>>
>> I'm no friend of the 'prefix' feature.  You could avoid it here by
>> renaming BlkdebugEvent to Blkdbg.  No additional churn, because you
>> already replace hand-written BlkDebugEvent by generated BlkdebugEvent.
>>
>> Alternatively, you could reduce churn by renaming it to BlkDebugEvent.
>>
>> Matter of taste.  Perhaps Kevin has a preference.
> 
> Wouldn't that mean that we end up with a C type called Blkdbg? In my
> opinion that's a bit unspecific, so if you ask me, I would paint my
> bikeshed in a different colour.

Most of the existing qapi names are Blkdebug, it was only the internal
enum being replaced that used BlkDebug.  Also, qapi would munge BlkDebug
into BLK_DEBUG, so I think we want to stick with the lowercase 'd' so
that the munging (without 'prefix') would stick to BLKDEBUG.

I'm also fine if you want me to do a followup patch that renames all
enums from BLKDBG_L1_UPDATE to BLKDEBUG_EVENT_L1_UPDATE, at which point
we could drop the 'prefix' (I don't know how many lines it would make
long enough to need different wrapping, but modulo wrapping it would all
be a mechanically scripted change).

I don't have a favorite color in this painting match, so let me know
yours :)

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 1/4] qemu-iotests: refine common.config

[Max Reitz]

On 04.11.2015 03:26, Bo Tu wrote:
> Replacing sed with awk, then it's easier to read.

I think you meant "awk with sed".

> Replacing "[ ! -z "$default_alias_machine" ]" with
> "[[ $default_alias_machine ]]", then it's slightly shorter.
> 
> Suggested-By: Sascha Silbe 
> Reviewed-by: Sascha Silbe 
> Reviewed-by: Eric Blake   
> Signed-off-by: Bo Tu 
> ---
>  tests/qemu-iotests/common.config | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/qemu-iotests/common.config 
> b/tests/qemu-iotests/common.config
> index 596bb2b..08f4b4e 100644
> --- a/tests/qemu-iotests/common.config
> +++ b/tests/qemu-iotests/common.config
> @@ -128,11 +128,10 @@ export QEMU_IMG=_qemu_img_wrapper
>  export QEMU_IO=_qemu_io_wrapper
>  export QEMU_NBD=_qemu_nbd_wrapper
>  
> -default_machine=$($QEMU -machine \? | awk '/(default)/{print $1}')
> -default_alias_machine=$($QEMU -machine \? |\
> -awk -v var_default_machine="$default_machine"\)\
> -'{if 
> ($(NF-2)=="(alias"&&$(NF-1)=="of"&&$(NF)==var_default_machine){print $1}}')
> -if [ ! -z "$default_alias_machine" ]; then
> +default_machine=$($QEMU -machine help | sed -n '/(default)/ s/ .*//p')
> +default_alias_machine=$($QEMU -machine help | \
> +   sed -n "/(alias of $default_machine)"/' { s/ .*//p; q; }')

Could be shortened to "/(alias of $default_machine)/ { s/ .*//p; q; }"
(superfluous quotation marks), but that doesn't make it less correct.


With the commit message fixed:

Reviewed-by: Max Reitz 

> +if [[ "$default_alias_machine" ]]; then
>  default_machine="$default_alias_machine"
>  fi
>  
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] vhost-user: fix log size

[Marc-André Lureau]

Reviewed-by: Marc-André Lureau 

On Wed, Nov 18, 2015 at 3:16 PM, Michael S. Tsirkin  wrote:
> commit 2b8819c6eee517c1582983773f8555bb3f9ed645
> ("vhost-user: modify SET_LOG_BASE to pass mmap size and offset")
> passes log size in units of 4 byte chunks instead of the
> expected size in bytes.
>
> Fix this up.
>
> Signed-off-by: Michael S. Tsirkin 
> ---
>  hw/virtio/vhost-user.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 71c3e16..1b6c5ac 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -206,7 +206,7 @@ static int vhost_user_set_log_base(struct vhost_dev *dev, 
> uint64_t base,
>  VhostUserMsg msg = {
>  .request = VHOST_USER_SET_LOG_BASE,
>  .flags = VHOST_USER_VERSION,
> -.payload.log.mmap_size = log->size,
> +.payload.log.mmap_size = log->size * sizeof(*(log->log)),
>  .payload.log.mmap_offset = 0,
>  .size = sizeof(msg.payload.log),
>  };
> --
> MST
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH for-2.5 44/77] pci-bridge: Set a supported devfn_min for bridge

[Michael S. Tsirkin]

On Wed, Nov 18, 2015 at 03:25:25PM +0100, Paolo Bonzini wrote:
> 
> 
> On 18/11/2015 15:21, Michael S. Tsirkin wrote:
> > This depends on the devfn_min thing, right?
> > I'm yet to review it, generally I'd prefer to
> > avoid changing device allocation rules since
> > that makes it so easy to break compatibility.
> > 
> > Assuming addresses are all explicitly stated,
> > is there even a problem?
> 
> It makes it a bit easier to use pci-bridge; it is a bug, albeit one that
> only affects direct usage through the command line of PCI bridges.
> 
> Paolo

Yes but not a regression ... I'll look at how
invasive the change it depends on is.

Re: [Qemu-devel] CPU Cache simulation

[Hao Bai]

Sorry about the ambiguity.
I am using x86-64 architecture in user mode. Basically, I am trying to log
all the cache activities when I run a guest program with QEMU. That's why I
asked whether QEMU simulated CPU caches. I was assuming if QEMU did not
simulate CPU caches then there would be no way to do this (Correct me if I
am wrong). Is there a way to do this?
Pointer to the 2009 thread:
https://lists.nongnu.org/archive/html/qemu-devel/2009-06/msg01353.html

Cheers

On Wed, Nov 18, 2015 at 10:15 AM, Eduardo Habkost 
wrote:

> On Wed, Nov 18, 2015 at 03:04:09AM -0500, Hao Bai wrote:
> > Hi all,
> >
> > I read from an earlier thread in 2009 that QEMU did not simulate CPU
> > caches. Is this still the case now?
>
> Could you clarify what you need? In which architecture? What
> exactly you mean by "simulating CPU caches"? Do you just want the
> guest to think the machine has a specific cache topology, or do
> you want to emulate other cache behavior? Do you need support to
> specific cache control instructions?
>
> Do you have a pointer to the 2009 thread?
>
> --
> Eduardo
>

Re: [Qemu-devel] [PATCH v2 2/4] qemu-iotests: s390x: fix test 051

[Max Reitz]

On 04.11.2015 03:26, Bo Tu wrote:
> The tests for device type "ide_cd" should only be tested for the pc
> platform.
> The default device id of hard disk on the s390 platform differs to that
> of the x86 platform. A new variable device_id is defined and "virtio0"
> set for the s390 platform. A x86 platform specific output file is also
> needed.
> Warning message expected for s390x when drive without device.
> 
> Reviewed-by: Sascha Silbe 
> Signed-off-by: Bo Tu 
> ---
>  tests/qemu-iotests/051|  99 ++
>  tests/qemu-iotests/051.out| 143 +++---
>  tests/qemu-iotests/051.pc.out | 422 
> ++
>  3 files changed, 515 insertions(+), 149 deletions(-)
>  create mode 100644 tests/qemu-iotests/051.pc.out
> 

[...]

> diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
> index 7765aa0..d17c969 100644
> --- a/tests/qemu-iotests/051.out
> +++ b/tests/qemu-iotests/051.out

[...]

> @@ -109,20 +109,23 @@ QEMU X.Y.Z monitor - type 'help' for more information
>  
>  Testing: -drive if=floppy
>  QEMU X.Y.Z monitor - type 'help' for more information
> -(qemu) qququiquit
> +(qemu) Warning: Orphaned drive without device: 
> id=floppy0,file=,if=floppy,bus=0,unit=0
> +qququiquit

I'd still like these warnings to be filtered out (as I wrote in my reply
to the original RFC's v4, and as was done in later versions of that RFC
(the _filter_orphan function e.g. in
http://lists.nongnu.org/archive/html/qemu-devel/2015-05/msg04816.html).

Looks good other than that.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL v1 (for 2.5) 0/4] Fix misc memory leaks & bugs in crypto code

[Peter Maydell]

On 18 November 2015 at 15:47, Daniel P. Berrange <berra...@redhat.com> wrote:
> Running test-crypto-tlscredsx509 & test-crypto-tlssession under
> valgrind identified 3 memory leaks and one other bug. This short
> series fixes them.
>
> The following changes since commit c27e9014d56fa4880e7d741275d887c3a5949997:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/pull-vnc-20151116-1' into 
> staging (2015-11-17 12:34:07 +)
>
> are available in the git repository at:
>
>   git://github.com/berrange/qemu.git tags/qcrypto-fixes-20151118-1
>
> for you to fetch changes up to 08cb175a24d642a40e41db2fef2892b0a1ab504e:
>
>   crypto: avoid passing NULL to access() syscall (2015-11-18 15:42:26 +)
>
> 
> Pull qcrypto fixes 2015/11/18 v1
>
> 
>
> Daniel P. Berrange (4):
>   crypto: fix leak of gnutls_dh_params_t data on credential unload
>   crypto: fix mistaken setting of Error in success code path
>   crypto: fix leaks in TLS x509 helper functions
>   crypto: avoid passing NULL to access() syscall
>
>  crypto/tlscredsx509.c   | 7 ++-
>  crypto/tlssession.c | 4 ++--
>  tests/crypto-tls-x509-helpers.c | 2 ++
>  3 files changed, 10 insertions(+), 3 deletions(-)

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v12 27/36] qapi: Forbid case-insensitive clashes

[Markus Armbruster]

Eric Blake  writes:

> In general, designing user interfaces that rely on case
> distinction is poor practice.  Another benefit of enforcing
> a restriction against case-insensitive clashes is that we
> no longer have to worry about the situation of enum values
> that could be distinguished by case if mapped by c_name(),
> but which cannot be distinguished when mapped to C as
> ALL_CAPS by c_enum_const() [via c_name(name, False).upper()].
> Thus, having the generator look for case collisions up front
> will prevent developers from worrying whether different
> munging rules for member names compared to enum values as a
> discriminator will cause any problems in qapi unions.
>
> We do not have to care about c_name(name, True) vs.
> c_name(name, False), as long as any pair of munged names being
> compared were munged using the same flag value to c_name().
> This is because commit 9fb081e already reserved names munging
> to 'q_', and this patch extends the reservation to 'Q_'; and
> because recent patches fixed things to ensure the only thing
> done by the flag is adding the prefix 'q_', that the only use
> of '.' (which also munges to '_') is in downstream extension
> prefixes, and that enum munging does not add underscores to
> any CamelCase values.
>
> However, we DO have to care about the fact that we have a
> command 'stop' and an event 'STOP' (currently the only case
> collision of any of our .json entities).  To solve that, use
> some tricks in the lookup map for entities.  With some careful
> choice of keys, we can bisect the map into two collision pools
> (name.upper() for events, name.lower() for all else).  Since
> we already document that no two entities can have the exact
> same spelling, an entity can only occur under one of the two
> partitions of the map.  We could go further and enforce that
> events are named with 'ALL_CAPS' and that nothing else is
> named in that manner; but that can be done as a followup if
> desired.  We could also separate commands from type names,
> but then we no longer have a convenient upper vs. lower
> partitioning allowing us to share a single dictionary.
>
> In order to keep things stable, the visit() method has to
> ensure that it visits entities sorted by their real name, and
> not by the new munged keys of the dictionary; Python's
> attrgetter is a lifesaver for this task.
>
> There is also the possibility that we may want to add a future
> extension to QMP of teaching it to be case-insensitive (the
> user could request command 'Quit' instead of 'quit', or could
> spell a struct field as 'CPU' instead of 'cpu').  But for that
> to be a practical extension, we cannot break backwards
> compatibility with any existing struct that was already
> relying on case sensitivity.  Fortunately, after a recent
> patch cleaned up CpuInfo, there are no such existing qapi
> structs.  Of course, the idea of a future extension is not
> as strong of a reason to make the change.
>
> At any rate, it is easier to be strict now, and relax things
> later if we find a reason to need case-sensitive QMP members,
> than it would be to wish we had the restriction in place.
>
> Add new tests args-case-clash.json and command-type-case-clash.json
> to demonstrate that the collision detection works.
>
> Signed-off-by: Eric Blake 
>
> ---
> v12: add in entity case collisions (required sharing two maps),
> improve commit message
> v11: rebase to latest, don't focus so hard on future case-insensitive
> extensions, adjust commit message
> v10: new patch
> ---
>  docs/qapi-code-gen.txt |  7 +
>  scripts/qapi.py| 41 
> +-
>  tests/Makefile |  2 ++
>  tests/qapi-schema/args-case-clash.err  |  1 +
>  tests/qapi-schema/args-case-clash.exit |  1 +
>  tests/qapi-schema/args-case-clash.json |  4 +++
>  tests/qapi-schema/args-case-clash.out  |  0
>  tests/qapi-schema/command-type-case-clash.err  |  1 +
>  tests/qapi-schema/command-type-case-clash.exit |  1 +
>  tests/qapi-schema/command-type-case-clash.json |  3 ++
>  tests/qapi-schema/command-type-case-clash.out  |  0
>  11 files changed, 53 insertions(+), 8 deletions(-)
>  create mode 100644 tests/qapi-schema/args-case-clash.err
>  create mode 100644 tests/qapi-schema/args-case-clash.exit
>  create mode 100644 tests/qapi-schema/args-case-clash.json
>  create mode 100644 tests/qapi-schema/args-case-clash.out
>  create mode 100644 tests/qapi-schema/command-type-case-clash.err
>  create mode 100644 tests/qapi-schema/command-type-case-clash.exit
>  create mode 100644 tests/qapi-schema/command-type-case-clash.json
>  create mode 100644 tests/qapi-schema/command-type-case-clash.out
>
> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index b01a691..1f6cb16 100644
> --- a/docs/qapi-code-gen.txt
> +++ b/docs/qapi-code-gen.txt
> @@ -102,6 +102,13 @@ single-dimension 

[Qemu-devel] [Bug 1465935] Re: kvm_irqchip_commit_routes: Assertion `ret == 0' failed

[Serge Hallyn]

@chengyuanli

could you please verify this?

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

Title:
  kvm_irqchip_commit_routes: Assertion `ret == 0' failed

Status in QEMU:
  New
Status in qemu package in Ubuntu:
  Fix Released
Status in qemu source package in Trusty:
  Fix Committed
Status in qemu source package in Utopic:
  Won't Fix
Status in qemu source package in Vivid:
  Fix Committed

Bug description:
  Several my QEMU instances crashed, and in the  qemu log, I can see
  this assertion failure,

 qemu-system-x86_64: /build/buildd/qemu-2.0.0+dfsg/kvm-all.c:984:
  kvm_irqchip_commit_routes: Assertion `ret == 0' failed.

  The QEMU version is 2.0.0, HV OS is ubuntu 12.04, kernel 3.2.0-38.
  Guest OS is RHEL 6.3.

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

Re: [Qemu-devel] [PATCH v12 21/36] qapi: Tighten the regex on valid names

[Eric Blake]

On 11/18/2015 04:51 AM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> We already documented that qapi names should match specific
>> patterns (such as starting with a letter unless it was an enum
>> value or a downstream extension).  Tighten that from a suggestion
>> into a hard requirement, which frees up names beginning with a
>> single underscore for qapi internal usage.
> 
> If we care enough about naming conventions to document them, enforcing
> them makes only sense.
> 
>> Also restrict things
>> to avoid 'a__b' or 'a--b' (that is, dash and underscore must not
>> occur consecutively).
> 
> I feel this is entering "foolish names" territory, where things like
> "IcAnFiNdNaMeSeVeNlEsSrEaDaBlEtHaNStudlyCaps" live.  Catching fools

IhAdToOmUcHfUnReAdInGtHaT

[It's amazing that scholars can ever manage to correctly interpret old
written languages, thanks to omitted word breaks (scriptio continua) or
omitted vowels (such as in Hebrew)]

> automatically is generally futile, they're too creative :)
> 
> Let's drop this part.

Sure, I can do that. I'll post a fixup patch, as it will affect docs too.

> 
>> Add a new test, reserved-member-underscore, to demonstrate the
>> tighter checking.
>>
>> Signed-off-by: Eric Blake 
>>
>> ---
>> v12: new patch
>> ---

>> +incompatibly in a future release.  All names must begin with a letter,
>> +and contain only ASCII letters, digits, dash, and underscore, where
>> +dash and underscore cannot occur consecutively.  There are two

Namely, right here.

>> +# Names must be letters, numbers, -, and _.  They must start with letter,
>> +# except for downstream extensions which must start with __RFQDN_.
>> +# Dots are only valid in the downstream extension prefix.
>> +valid_name = re.compile('^(__[a-zA-Z][a-zA-Z0-9.]*_)?'
>> +'[a-zA-Z][a-zA-Z0-9]*([_-][a-zA-Z0-9]+)*$')
> 
> This regexp consists of two parts: optional __RFQDN_ prefix and the name
> proper.
> 
> The latter stays simpler if we don't attempt to catch adjacent [-_].
> 
> The former isn't quite right.  According to RFC 822 Appendix 1 - Domain
> Name Syntax Specification:
> 
> * We must accept '-' in addition to digits.
> 
> * We require RFQDN to start with a letter.  This is still a loose match.
>   The tight match is "labels separated by dots", where labels consist of
>   letters, digits and '-', starting with a letter.  I wouldn't bother
>   checking first characters are letters at all.
> 
> Recommend
> 
>valid_name = re.compile('^(__[a-zA-Z0-9.-]+_)?'
>'[a-zA-Z][a-zA-Z0-9_-]*$')

Sure, that works for me. It's tighter than what we had before, but
looser than what I proposed so that it allows more RFQDN.  It
potentially lets users confuse us by abusing 'foo__max' or similar, but
I'm less worried about that abuse - it's okay to stop the blatantly
obvious mistakes without worrying about the corner cases, if it makes
the maintenance easier for the cases we do catch.

> 
>>
>>
>>  def check_name(expr_info, source, name, allow_optional=False,
>> @@ -374,8 +376,8 @@ def check_name(expr_info, source, name, 
>> allow_optional=False,
>>  % (source, name))
>>  # Enum members can start with a digit, because the generated C
>>  # code always prefixes it with the enum name
>> -if enum_member:
>> -membername = '_' + membername
>> +if enum_member and membername[0].isdigit():
> 
> What's wrong with the old condition?
> 
>> +membername = 'D' + membername

The old code prepended a lone '_', which doesn't work any more with the
tighter valid_name regex, so we have to use some other prefix (I picked
'D').

>>  # Reserve the entire 'q_' namespace for c_name()
>>  if not valid_name.match(membername) or \
>> c_name(membername, False).startswith('q_'):

The other thing is that I realized that an enum value of 'q-int' was
getting munged to '_q-int' which no longer gets flagged by the c_name()
filter.  But neither would 'Dq-int' get flagged.  So limiting the
munging to just enum values that start with a digit (where we know it
doesn't start with q) means we don't weaken the second condition.

I guess I need to call that out in the commit message; all the more
reason for me to send a fixup.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 4/4] qemu-iotests: disable VNC server for test 120

[Max Reitz]

On 04.11.2015 03:26, Bo Tu wrote:
> Ever since qemu-iotest 120 was introduced, its expected output didn't
> include the output from the built-in VNC server:
> 
>  QA output created by 120
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
>  QMP_VERSION
> +VNC server running on `::1:5900'
>  {"return": {}}
>  wrote 65536/65536 bytes at offset 0
>  64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> 
> Since some architectures do not even have a graphical console, we just
> pass -nographic to avoid the output.
> 
> Fixes: a68197ff5b11 ("iotests: Add tests for overriding
> BDRV_O_PROTOCOL")
> 
> Reviewed-by: Sascha Silbe 
> Signed-off-by: Bo Tu 
> ---
>  tests/qemu-iotests/120 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/120 b/tests/qemu-iotests/120
> index 9f13078..fb69efd 100755
> --- a/tests/qemu-iotests/120
> +++ b/tests/qemu-iotests/120
> @@ -49,7 +49,7 @@ echo "{'execute': 'qmp_capabilities'}
>{'execute': 'human-monitor-command',
> 'arguments': {'command-line': 'qemu-io drv \"write -P 42 0 64k\"'}}
>{'execute': 'quit'}" \
> -| $QEMU -qmp stdio -nodefaults \
> +| $QEMU -qmp stdio -nodefaults -nographic \
>  -drive 
> id=drv,if=none,file="$TEST_IMG",driver=raw,file.driver=$IMGFMT \
>  | _filter_qmp | _filter_qemu_io
>  $QEMU_IO -c 'read -P 42 0 64k' "$TEST_IMG" | _filter_qemu_io

Looks good, but I think we need the same for 119.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 3/4] qemu-iotests: s390x: fix test 068

[Max Reitz]

On 04.11.2015 03:26, Bo Tu wrote:
> Now, s390-virtio-ccw is default machine and s390-ccw.img is default boot
> loader. If the s390-virtio-ccw machine finds no device to load from and
> errors out, then emits a panic and exits the vm. This breaks test cases
> 068 for s390x.
> Adding the parameter of "-no-shutdown" for s390-ccw-virtio will pause VM
> before shutdown.
> 
> Reviewed-by: Sascha Silbe 
> Signed-off-by: Bo Tu 
> ---
>  tests/qemu-iotests/068 | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)

Acked-by: Max Reitz 

(I have no way of verifying that this patch fixes this test for said
machine type, so I cannot give an R-b.)



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v12 22/36] qapi: Don't let implicit enum MAX member collide

[Eric Blake]

On 11/18/2015 06:12 AM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> Now that we guarantee the user doesn't have any enum values
>> beginning with a single underscore, we can use that for our
>> own purposes.  Renaming ENUM_MAX to ENUM__MAX makes it obvious
>> that the sentinel is generated.
> 
> The ENUM__MAX are mildly ugly.  If we cared, we could get rid of most or
> all uses.  Right now, I don't.
> 
> Hmm, perhaps these ENUM__MAX are your motivation for rejecting adjacent
> [-_] in the previous patch, to catch clashes like (foolishly named) type
> 'FOO--MAX' with enumeration type 'Foo'.  I acknowledge the clash.

Yes, and that was part of my reasoning on 21/36 - but you've convinced
me to relax that one, so it no longer applies as an argument here.

> However, there are many more clashes we don't attempt to catch,
> e.g. type 'Foo-lookup' with enumeration type 'Foo'.  Perhaps we want to
> build a real fence later, but right now I don't want us to ram in more
> scattered fence laths.
> 
> Might want to say something like "Rejecting enum members named 'MAX' is
> now useless, and will be dropped in the next patch".

Yes, that helps the commit message. Can you handle it, or do I need to
send a fixup?

> 
>> This patch was mostly generated by applying a temporary patch:
>>
>> |diff --git a/scripts/qapi.py b/scripts/qapi.py
>> |index e6d014b..b862ec9 100644
>> |--- a/scripts/qapi.py
>> |+++ b/scripts/qapi.py
>> |@@ -1570,6 +1570,7 @@ const char *const %(c_name)s_lookup[] = {
>> | max_index = c_enum_const(name, 'MAX', prefix)
>> | ret += mcgen('''
>> | [%(max_index)s] = NULL,
>> |+// %(max_index)s
>> | };
>> | ''',
>> |max_index=max_index)
>>
>> then running:
>>
>> $ cat qapi-{types,event}.c tests/test-qapi-types.c |
>> sed -n 's,^// \(.*\)MAX,s|\1MAX|\1_MAX|g,p' > list
>> $ git grep -l _MAX | xargs sed -i -f list
>>
>> The only things not generated are the changes in scripts/qapi.py.
>>
>> Signed-off-by: Eric Blake 
> 
> Patch looks good.

The fun part was coming up with the automation, and then making sure it
was reproducible. :)

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v14 00/13] vfio-pci: pass the aer error to guest

[Michael S. Tsirkin]

Focusing on 2.5 now.
Pls post after 2.5.

On Tue, Nov 17, 2015 at 04:41:51PM +0800, Cao jin wrote:
> From: Chen Fan 
> 
> For now, for vfio pci passthough devices when qemu receives
> an error from host aer report, currentlly just terminate the guest,
> but usually user want to know what error occurred but stopping the
> guest, so this patches add aer capability support for vfio device,
> and pass the error to guest, and have guest driver to recover
> from the error.
> 
> v13-v14:
>1. for multifunction device, requiring all functions enable AER.(9/13)
>2. due to all affected functions receive error signal, ignore no
>   error occurred function. (12/13)
> 
> v12-v13:
>1. since support multifuncion hotplug, here add callback to enable aer.
>2. add pci device pre+post reset for aer host reset.
> 
> Chen Fan (13):
>   vfio: extract vfio_get_hot_reset_info as a single function
>   vfio: squeeze out vfio_pci_do_hot_reset for support bus reset
>   pcie: modify the capability size assert
>   vfio: make the 4 bytes aligned for capability size
>   vfio: add pcie extanded capability support
>   aer: impove pcie_aer_init to support vfio device
>   vfio: add aer support for vfio device
>   vfio: add check host bus reset is support or not
>   add check reset mechanism when hotplug vfio device
>   pci: add pci device pre-post reset callbacks for host bus reset
>   pcie_aer: expose pcie_aer_msg() interface
>   vfio-pci: pass the aer error to guest
>   vfio: add 'aer' property to expose aercap
> 
>  hw/pci-bridge/ioh3420.c|   2 +-
>  hw/pci-bridge/xio3130_downstream.c |   2 +-
>  hw/pci-bridge/xio3130_upstream.c   |   2 +-
>  hw/pci/pci.c   |  47 +++
>  hw/pci/pci_bridge.c|   9 +
>  hw/pci/pcie.c  |   2 +-
>  hw/pci/pcie_aer.c  |   6 +-
>  hw/vfio/pci.c  | 625 
> +
>  hw/vfio/pci.h  |   8 +
>  include/hw/pci/pci.h   |   7 +
>  include/hw/pci/pci_bus.h   |   5 +
>  include/hw/pci/pcie_aer.h  |   3 +-
>  12 files changed, 646 insertions(+), 72 deletions(-)
> 
> -- 
> 1.9.3

Re: [Qemu-devel] [PATCH] vfio/pci-quirks: Only quirk to size of PCI config space

[Laszlo Ersek]

On 11/18/15 17:03, Alex Williamson wrote:
> On Wed, 2015-11-18 at 10:36 +0100, Laszlo Ersek wrote:
>> On 11/18/15 00:41, Alex Williamson wrote:
>>> For quirks that support the full PCIe extended config space, limit the
>>> quirk to only the size of config space available through vfio.  This
>>> allows host systems with broken MMCONFIG regions to still make use of
>>> these quirks without generating bad address faults trying to access
>>> beyond the end of config space exposed through vfio.  This may expose
>>> direct access to parts of extended config space that we'd prefer not
>>> to expose, but that's why we have an IOMMU.
>>>
>>> Signed-off-by: Alex Williamson 
>>> Reported-by: Ronnie Swanink 
>>> ---
>>>  hw/vfio/pci-quirks.c |6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
>>> index 30c68a1..e117c41 100644
>>> --- a/hw/vfio/pci-quirks.c
>>> +++ b/hw/vfio/pci-quirks.c
>>> @@ -328,7 +328,7 @@ static void vfio_probe_ati_bar4_quirk(VFIOPCIDevice 
>>> *vdev, int nr)
>>>  window->data_offset = 4;
>>>  window->nr_matches = 1;
>>>  window->matches[0].match = 0x4000;
>>> -window->matches[0].mask = PCIE_CONFIG_SPACE_SIZE - 1;
>>> +window->matches[0].mask = vdev->config_size - 1;
>>>  window->bar = nr;
>>>  window->addr_mem = >mem[0];
>>>  window->data_mem = >mem[1];
>>> @@ -674,7 +674,7 @@ static void vfio_probe_nvidia_bar5_quirk(VFIOPCIDevice 
>>> *vdev, int nr)
>>>  window->matches[0].match = 0x1800;
>>>  window->matches[0].mask = PCI_CONFIG_SPACE_SIZE - 1;
>>>  window->matches[1].match = 0x88000;
>>> -window->matches[1].mask = PCIE_CONFIG_SPACE_SIZE - 1;
>>> +window->matches[1].mask = vdev->config_size - 1;
>>>  window->bar = nr;
>>>  window->addr_mem = bar5->addr_mem = >mem[0];
>>>  window->data_mem = bar5->data_mem = >mem[1];
>>> @@ -765,7 +765,7 @@ static void vfio_probe_nvidia_bar0_quirk(VFIOPCIDevice 
>>> *vdev, int nr)
>>>  memory_region_init_io(mirror->mem, OBJECT(vdev),
>>>_nvidia_mirror_quirk, mirror,
>>>"vfio-nvidia-bar0-88000-mirror-quirk",
>>> -  PCIE_CONFIG_SPACE_SIZE);
>>> +  vdev->config_size);
>>>  memory_region_add_subregion_overlap(>bars[nr].region.mem,
>>>  mirror->offset, mirror->mem, 1);
>>>  
>>>
>>>
>>
>> $ git log -- hw/vfio/pci-quirks.c | grep Reviewed-by
>> 
>>
>> Okay, I guess my review won't be deemed immediately unnecessary then. :)
> 
> Reviews are very much appreciated.
> 
>> (1) Please add to the commit message:
>>
>> Link: https://www.redhat.com/archives/vfio-users/2015-November/msg00192.html
>>
>> With that reference:
>>
>> Reviewed-by: Laszlo Ersek 
> 
> Done
> 
>> (2) Also, one question just to make sure I understand right: the last
>> sentence of the commit message means that the left-inclusive,
>> right-exclusive config space offset range
>>
>>   [vdev->config_size, PCIE_CONFIG_SPACE_SIZE)
>>
>> is now directly available to the guest, without QEMU noticing; but,
>> we're not worried about that, because the guest can't abuse that freedom
>> anyway for doing arbitrary DMA. Is that right?
>>
>> If so, then I propose another update to the commit message (replacing
>> the last sentence):
>>
>> This may grant the guest direct access to trailing parts of
>> extended config space that we'd prefer not to expose, but that's
>> why we have an IOMMU (the guest can't abuse said config space access
>> for arbitrary DMA).
>>
>> Perhaps the above is trivial for you (assuming it is correct at all...);
>> it may not be trivial for others.
> 
> How's this?:
> 
> commit b27c4f5da92a1732a77ddbe98571583a4eac1e14
> Author: Alex Williamson 
> Date:   Tue Nov 17 16:37:38 2015 -0700
> 
> vfio/pci-quirks: Only quirk to size of PCI config space
> 
> For quirks that support the full PCIe extended config space, limit the
> quirk to only the size of config space available through vfio.  This
> allows host systems with broken MMCONFIG regions to still make use of
> these quirks without generating bad address faults trying to access
> beyond the end of config space exposed through vfio.  This may expose
> direct access to the mirror of extended config space, only trapping
> the sub-range of standard config space, but allowing this makes the
> quirk, and thus the device, functional.  We expect that only device
> specific accesses make use of the mirror, not general extended PCI
> capability accesses, so any virtualization in this space is likely
> unnecessary anyway, and the device is still IOMMU isolated, so it
> should only be able to hurt itself through any bogus configurations
> enabled by this space.
> 
> Link: 
> 

Re: [Qemu-devel] [PATCH for 2.9 v8 0/10] dataplane snapshot fixes

[Denis V. Lunev]

On 11/18/2015 06:24 PM, Juan Quintela wrote:

"Denis V. Lunev"  wrote:

On 11/18/2015 05:31 PM, Juan Quintela wrote:

Greg Kurz  wrote:

On Tue, 17 Nov 2015 12:08:20 +0300
"Denis V. Lunev"  wrote:


with test
  while /bin/true ; do
  virsh snapshot-create rhel7
  sleep 10
  virsh snapshot-delete rhel7 --current
  done
with enabled iothreads on a running VM leads to a lot of troubles: hangs,
asserts, errors.


In my case, when using a virtio-blk-dataplane device, calling savevm *always*
result in a QEMU hang.

Oops


With this series (plus the s/bs/bs_vm_state/ change in patch 11), savevm/loadvm
now works like a charm.

Nice, thanks for the testing.


I saw that Juan does not like aio_context being used in migration code, but
in case this series gets applied anyway:

Tested-by: Greg Kurz 

I *think* that we should get better API's exported from block layer, but
*at least* we will get this series in.

Thanks, Juan.

that is good to me. Current block level API is terrible and unclear.

Greg is correct there.

Should I resubmit the last patch or you will change this yourself?

Please, test and resend.

I am not have lots of experience testing that.

Thanks, Juan.

perfect! Give me 2-3 hours pls.

Re: [Qemu-devel] [PATCH v2 2/5] sockets: remove use of QemuOpts from socket_listen

[Eric Blake]

On 11/18/2015 03:08 AM, Daniel P. Berrange wrote:
> On Tue, Nov 17, 2015 at 03:22:04PM -0700, Eric Blake wrote:
>> On 11/17/2015 10:00 AM, Daniel P. Berrange wrote:
>>> The socket_listen method accepts a QAPI SocketAddress object
>>> which it then turns into QemuOpts before calling the
>>> inet_listen_opts/unix_listen_opts helper methods. By
>>> converting the latter to use QAPI SocketAddress directly,
>>> the QemuOpts conversion step can be eliminated
>>>
>>> This also fixes the problem where ipv4=off && ipv6=off
>>> would be treated the same as ipv4=on && ipv6=on
>>>

>>> + *  ipv4  ipv6   family
>>> + *   - -   PF_UNSPEC

This says I have no preference, so pick the one that works.

>>> + *   - f   PF_INET
>>> + *   - t   PF_INET6
>>> + *   f -   PF_INET6
>>> + *   f f   
>>> + *   f t   PF_INET6
>>> + *   t -   PF_INET
>>> + *   t f   PF_INET
>>
>> These I understand,

Generally to mean "I specifically requested this" or "I specifically
don't want that", where there is no collision.

>>
>>> + *   t t   PF_INET6
>>
>> but why is this one PF_INET6 instead of PF_UNSPEC?
> 
> If you use PF_UNSPEC, then the addresses we listen on will be automatically
> deteremined by results of the DNS lookup. ie if DNS only returns an IPv4
> address, it won't listen on IPv6.  When the user has said ipv6=on, then
> they expect to get an error if it was not possible to listen on IPv6. So
> we must use PF_INET6 here to ensure that error, otherwise ipv6=on & ipv4=on
> would be no different from ipv6=- & ipv4=-.

But if I'm specifically requesting that both families be used, either
that should be an error (we can't honor two families at once) or it
should be allowed (use the family that makes sense), not a synonym for
ipv6-only.

>>> @@ -219,13 +251,15 @@ listen:
>>>  freeaddrinfo(res);
>>>  return -1;
>>>  }
>>> -qemu_opt_set(opts, "host", uaddr, _abort);
>>> -qemu_opt_set_number(opts, "port", inet_getport(e) - port_offset,
>>> -_abort);
>>> -qemu_opt_set_bool(opts, "ipv6", e->ai_family == PF_INET6,
>>> -  _abort);
>>> -qemu_opt_set_bool(opts, "ipv4", e->ai_family != PF_INET6,
>>> -  _abort);
>>> +if (update_addr) {
>>> +g_free(saddr->host);
>>> +saddr->host = g_strdup(uaddr);
>>> +g_free(saddr->port);
>>> +saddr->port = g_strdup_printf("%d",
>>> +  inet_getport(e) - port_offset);
>>> +saddr->has_ipv6 = saddr->ipv6 = e->ai_family == PF_INET6;
>>> +saddr->has_ipv4 = saddr->ipv4 = e->ai_family != PF_INET6;
>>
>> Should we handle PF_UNSPEC specifically, maybe by having the has_ipv6
>> assignment based on e->ai_family != PF_INET?
> 
> The returne e->ai_family from getaddrinfo will never have a value
> of PF_UNSPEC - that's an input only value. So we're OK to assume
> we'll have PF_INET6 and PF_INET only.  Well at least until someone
> invents IPv7 but I'll let my great grandchildren deal with that
> problem ;-)

Okay, but maybe worth a comment somewhere.  Or put another way, on
input, we can request one or both families, on output, we want to
guarantee which family was selected.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] Closing Bitmaps (Was: Re: [PATCH v7 03/24] block: Release dirty bitmaps in bdrv_close())

[John Snow]

On 11/18/2015 10:03 AM, Kevin Wolf wrote:
> Am 17.11.2015 um 18:05 hat John Snow geschrieben:
>> Still the subject of debate on-list, but the thought is roughly this:
>>
>> Bitmaps will be able to flush-to-file on close. (If they have no
>> persistence data, it's a no-op (maybe.)) This might mean being flushed
>> to their own BDS -- the one they are describing -- as a qcow2 extension.
>> Or, it could be to an arbitrary new standalone file format designed for
>> the sole purpose of containing bitmap data.
>>
>> The discussion hasn't progressed beyond "Max and Kevin do not think
>> storing arbitrary bitmaps in .qcow2 files is a good idea." The logical
>> conclusion is "We need a new standalone format, then" but we haven't
>> decided what that format will look like or how it will be used.
> 
> I think the actual logical conclusion is that you use qcow2 images in
> order to use the feature.
> 
> Kevin
> 

It's fine to say "To hell with raw," but for networked filesystems and
other configurations that aren't using the qcow2 driver, I think it
won't be a sufficient answer.

qcow2 support is my first priority, but I think it can't be my only one.

[Qemu-devel] [PULL 1/4] nand: fix address overflow

[Kevin Wolf]

From: Rabin Vincent 

The shifts of the address mask and value shift beyond 32 bits when there
are 5 address cycles.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Rabin Vincent 
Reviewed-by: Paolo Bonzini 
Reviewed-by: Peter Crosthwaite 
Signed-off-by: Kevin Wolf 
---
 hw/block/nand.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/block/nand.c b/hw/block/nand.c
index 61d2cec..a68266f 100644
--- a/hw/block/nand.c
+++ b/hw/block/nand.c
@@ -522,8 +522,8 @@ void nand_setio(DeviceState *dev, uint32_t value)
 
 if (s->ale) {
 unsigned int shift = s->addrlen * 8;
-unsigned int mask = ~(0xff << shift);
-unsigned int v = value << shift;
+uint64_t mask = ~(0xffull << shift);
+uint64_t v = (uint64_t)value << shift;
 
 s->addr = (s->addr & mask) | v;
 s->addrlen ++;
-- 
1.8.3.1

[Qemu-devel] [PULL 0/4] Block patches for 2.5.0-rc1

[Kevin Wolf]

The following changes since commit ab9b872ab3147faf3c04e91d525815b9139dd996:

  Merge remote-tracking branch 'remotes/mdroth/tags/qga-pull-2015-11-13-v2-tag' 
into staging (2015-11-18 12:47:29 +)

are available in the git repository at:


  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to ca4fa82fe66076284f702adcfe7c319ebbf909ec:

  Merge remote-tracking branch 'mreitz/tags/pull-block-for-kevin-2015-11-18' 
into queue-block (2015-11-18 16:27:44 +0100)



Block layer patches


Alberto Garcia (1):
  block: Call external_snapshot_clean after blockdev-snapshot

John Snow (1):
  iotests: fix race in 030

Kevin Wolf (1):
  Merge remote-tracking branch 
'mreitz/tags/pull-block-for-kevin-2015-11-18' into queue-block

Max Reitz (1):
  blockdev: Add missing bdrv_unref() in drive-backup

Rabin Vincent (1):
  nand: fix address overflow

 blockdev.c | 2 ++
 hw/block/nand.c| 4 ++--
 tests/qemu-iotests/030 | 5 -
 3 files changed, 8 insertions(+), 3 deletions(-)

[Qemu-devel] [PULL 3/4] blockdev: Add missing bdrv_unref() in drive-backup

[Kevin Wolf]

From: Max Reitz 

All error paths after a successful bdrv_open() of target_bs should
contain a bdrv_unref(target_bs). This one did not yet, so add it.

Signed-off-by: Max Reitz 
Reviewed-by: Alberto Garcia 
Reviewed-by: Kevin Wolf 
Reviewed-by: Fam Zheng 
Signed-off-by: Kevin Wolf 
---
 blockdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/blockdev.c b/blockdev.c
index 917ae06..07c1741 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3171,6 +3171,7 @@ static void do_drive_backup(const char *device, const 
char *target,
 bmap = bdrv_find_dirty_bitmap(bs, bitmap);
 if (!bmap) {
 error_setg(errp, "Bitmap '%s' could not be found", bitmap);
+bdrv_unref(target_bs);
 goto out;
 }
 }
-- 
1.8.3.1

[Qemu-devel] [PULL 2/4] iotests: fix race in 030

[Kevin Wolf]

From: John Snow 

the stop_test case tests that we can resume a block-stream
command after it has stopped/paused due to error. We cannot
always reliably query it before it finishes after resume, though,
so make this a conditional.

The important thing is that we are still testing that it has stopped,
and that it finishes successfully after we send a resume command.

Signed-off-by: John Snow 
Reviewed-by: Fam Zheng 
Reviewed-by: Alberto Garcia 
Reviewed-by: Jeff Cody 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/030 | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index 952a524..32469ef 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -245,6 +245,7 @@ class TestEIO(TestErrors):
 while not completed:
 for event in self.vm.get_qmp_events(wait=True):
 if event['event'] == 'BLOCK_JOB_ERROR':
+error = True
 self.assert_qmp(event, 'data/device', 'drive0')
 self.assert_qmp(event, 'data/operation', 'read')
 
@@ -257,9 +258,11 @@ class TestEIO(TestErrors):
 self.assert_qmp(result, 'return', {})
 
 result = self.vm.qmp('query-block-jobs')
+if result == {'return': []}:
+# Race; likely already finished. Check.
+continue
 self.assert_qmp(result, 'return[0]/paused', False)
 self.assert_qmp(result, 'return[0]/io-status', 'ok')
-error = True
 elif event['event'] == 'BLOCK_JOB_COMPLETED':
 self.assertTrue(error, 'job completed unexpectedly')
 self.assert_qmp(event, 'data/type', 'stream')
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 1/3] virtio-net: use the backend cross-endian capabilities

[Greg Kurz]

When running a fully emulated device in cross-endian conditions, including
a virtio 1.0 device offered to a big endian guest, we need to fix the vnet
headers. This is currently handled by the virtio_net_hdr_swap() function
in the core virtio-net code but it should actually be handled by the net
backend.

With this patch, virtio-net now tries to configure the backend to do the
endian fixing when the device starts. If the backend cannot support the
requested endiannes, we have to fall back on virtio_net_hdr_swap(): this
is recorded in the needs_vnet_hdr_swap flag, to be used in the TX and RX
paths.

The current vhost-net code also tries to configure net backends. This will
be no more needed and will be addressed in a subsequent patch.

Signed-off-by: Greg Kurz 
---
v3: - dropped extraneous peer_has_vnet_hdr() declaration
- only rely on the flag to decide if swap is needed
---
 hw/net/virtio-net.c|   33 +++--
 include/hw/virtio/virtio-net.h |1 +
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index a877614e3e7a..d4cc94ea5e55 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -152,6 +152,31 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t 
status)
 }
 }
 
+static void virtio_net_vnet_status(VirtIONet *n, uint8_t status)
+{
+VirtIODevice *vdev = VIRTIO_DEVICE(n);
+NetClientState *peer = qemu_get_queue(n->nic)->peer;
+
+if (virtio_net_started(n, status)) {
+int r;
+
+if (virtio_is_big_endian(vdev)) {
+r = qemu_set_vnet_be(peer, true);
+} else {
+r = qemu_set_vnet_le(peer, true);
+}
+
+n->needs_vnet_hdr_swap = !!r;
+} else if (virtio_net_started(n, vdev->status) &&
+   !virtio_net_started(n, status)) {
+if (virtio_is_big_endian(vdev)) {
+qemu_set_vnet_be(peer, false);
+} else {
+qemu_set_vnet_le(peer, false);
+}
+}
+}
+
 static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
 {
 VirtIONet *n = VIRTIO_NET(vdev);
@@ -159,6 +184,7 @@ static void virtio_net_set_status(struct VirtIODevice 
*vdev, uint8_t status)
 int i;
 uint8_t queue_status;
 
+virtio_net_vnet_status(n, status);
 virtio_net_vhost_status(n, status);
 
 for (i = 0; i < n->max_queues; i++) {
@@ -957,7 +983,10 @@ static void receive_header(VirtIONet *n, const struct 
iovec *iov, int iov_cnt,
 void *wbuf = (void *)buf;
 work_around_broken_dhclient(wbuf, wbuf + n->host_hdr_len,
 size - n->host_hdr_len);
-virtio_net_hdr_swap(VIRTIO_DEVICE(n), wbuf);
+
+if (n->needs_vnet_hdr_swap) {
+virtio_net_hdr_swap(VIRTIO_DEVICE(n), wbuf);
+}
 iov_from_buf(iov, iov_cnt, 0, buf, sizeof(struct virtio_net_hdr));
 } else {
 struct virtio_net_hdr hdr = {
@@ -1167,7 +1196,7 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
 error_report("virtio-net header incorrect");
 exit(1);
 }
-if (virtio_needs_swap(vdev)) {
+if (n->needs_vnet_hdr_swap) {
 virtio_net_hdr_swap(vdev, (void *) );
 sg2[0].iov_base = 
 sg2[0].iov_len = n->guest_hdr_len;
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index f3cc25feca2b..27bc868fbc7d 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -94,6 +94,7 @@ typedef struct VirtIONet {
 uint64_t curr_guest_offloads;
 QEMUTimer *announce_timer;
 int announce_counter;
+bool needs_vnet_hdr_swap;
 } VirtIONet;
 
 void virtio_net_set_netclient_name(VirtIONet *n, const char *name,

[Qemu-devel] [PATCH v3 2/3] Revert "vhost-net: tell tap backend about the vnet endianness"

[Greg Kurz]

This reverts commit 5be7d9f1b1452613b95c6ba70b8d7ad3d0797991.

Cross-endian is now configured by the core virtio-net code. We simply
fall back on full emulation if the net backend cannot support the
requested endianness for vnet headers.

Signed-off-by: Greg Kurz 
---
 hw/net/vhost_net.c  |   33 +
 hw/net/virtio-net.c |7 +++
 2 files changed, 8 insertions(+), 32 deletions(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index d91b7b155ea7..fd7ea47fd38d 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -38,7 +38,6 @@
 #include "standard-headers/linux/virtio_ring.h"
 #include "hw/virtio/vhost.h"
 #include "hw/virtio/virtio-bus.h"
-#include "hw/virtio/virtio-access.h"
 
 struct vhost_net {
 struct vhost_dev dev;
@@ -205,27 +204,6 @@ static void vhost_net_set_vq_index(struct vhost_net *net, 
int vq_index)
 net->dev.vq_index = vq_index;
 }
 
-static int vhost_net_set_vnet_endian(VirtIODevice *dev, NetClientState *peer,
- bool set)
-{
-int r = 0;
-
-if (virtio_vdev_has_feature(dev, VIRTIO_F_VERSION_1) ||
-(virtio_legacy_is_cross_endian(dev) && !virtio_is_big_endian(dev))) {
-r = qemu_set_vnet_le(peer, set);
-if (r) {
-error_report("backend does not support LE vnet headers");
-}
-} else if (virtio_legacy_is_cross_endian(dev)) {
-r = qemu_set_vnet_be(peer, set);
-if (r) {
-error_report("backend does not support BE vnet headers");
-}
-}
-
-return r;
-}
-
 static int vhost_net_start_one(struct vhost_net *net,
VirtIODevice *dev)
 {
@@ -320,11 +298,6 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
 goto err;
 }
 
-r = vhost_net_set_vnet_endian(dev, ncs[0].peer, true);
-if (r < 0) {
-goto err;
-}
-
 for (i = 0; i < total_queues; i++) {
 vhost_net_set_vq_index(get_vhost_net(ncs[i].peer), i * 2);
 }
@@ -332,7 +305,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
 r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true);
 if (r < 0) {
 error_report("Error binding guest notifier: %d", -r);
-goto err_endian;
+goto err;
 }
 
 for (i = 0; i < total_queues; i++) {
@@ -354,8 +327,6 @@ err_start:
 fprintf(stderr, "vhost guest notifier cleanup failed: %d\n", e);
 fflush(stderr);
 }
-err_endian:
-vhost_net_set_vnet_endian(dev, ncs[0].peer, false);
 err:
 return r;
 }
@@ -378,8 +349,6 @@ void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs,
 fflush(stderr);
 }
 assert(r >= 0);
-
-assert(vhost_net_set_vnet_endian(dev, ncs[0].peer, false) >= 0);
 }
 
 void vhost_net_cleanup(struct vhost_net *net)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index d4cc94ea5e55..5a0ab6ad5bb5 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -128,6 +128,13 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t 
status)
 if (!n->vhost_started) {
 int r, i;
 
+if (n->needs_vnet_hdr_swap) {
+error_report("backend does not support %s vnet headers."
+ "falling back on userspace virtio",
+ virtio_is_big_endian(vdev) ? "BE" : "LE");
+return;
+}
+
 /* Any packets outstanding? Purge them to avoid touching rings
  * when vhost is running.
  */

Re: [Qemu-devel] Closing Bitmaps (Was: Re: [PATCH v7 03/24] block: Release dirty bitmaps in bdrv_close())

[John Snow]

On 11/17/2015 09:29 PM, Fam Zheng wrote:
> On Tue, 11/17 12:05, John Snow wrote:
>> Still the subject of debate on-list, but the thought is roughly this:
> 
> Thanks for summerizing this!
> 
>>
>> Bitmaps will be able to flush-to-file on close. (If they have no
>> persistence data, it's a no-op (maybe.)) This might mean being flushed
>> to their own BDS -- the one they are describing -- as a qcow2 extension.
>> Or, it could be to an arbitrary new standalone file format designed for
>> the sole purpose of containing bitmap data.
>>
>> The discussion hasn't progressed beyond "Max and Kevin do not think
>> storing arbitrary bitmaps in .qcow2 files is a good idea." The logical
>> conclusion is "We need a new standalone format, then" but we haven't
>> decided what that format will look like or how it will be used.
> 
> I second that. While storing dirty bitmap in the image itself sounds fine, it
> is unlikely the best way for any other formats. Given that we will have a
> standalone format for raw or other formats, it's probably not worth to extend
> qcow2 specifically.
> 

We do already have the code, though, and for simple desktop use cases
it's very, very convenient, though it does leave us with two usage cases
for bitmaps.

I believe Denis and Vladimir plan to allow for Parallels to support the
inline bitmap load/store primitive, too.

I think the model of "Store in the image if you can, use an external
otherwise" is perfectly fine -- if not slightly more complicated than
what the bare minimum feature requires. I think the usability boost it
provides to users is worth the maintenance and review burden.

For now, I think Vladimir is working on the qcow2 extension first, and
we'll visit the generic file format later.

Is it possible to create a block driver that *only* supports the bitmap
load/store primitives? (I.e. -- no read/write support at all?)

>>
>> Then, Through CLI arguments or QMP arguments, you can modify the
>> persistence attributes of bitmaps -- choosing where to store them.
>> Bitmaps for qcow2 nodes can be stored in their own node, bitmaps
>> describing raw files will need to be stored in an external file.
>>
>> (Is it possible to create a block driver that doesn't implement
>> read/write primitives, and only implements theoretical bitmap load/save
>> primitives? We could create a block driver for a standalone bitmap
>> container in this way...)
> 
> What about implementing it as a filter? It would work similar to blkdebug, 
> with
> .bdrv_co_writev to mark the in memory dirty bits (or even manage a
> sophisticated cache to support large images efficiently), and .bdrv_close
> to flush data to disk.
> 
> But that depends on the dynamic reconfigiration feature of block nodes, for
> hot add/remove of dirty bitmap to work.
> 
> Fam
> 

Filters I think would be the ideal path, but if the QMP and CLI features
around dynamic reconfiguration are not quite ready, that route may not
be open to us... or not in a timely fashion, anyway.

Re: [Qemu-devel] CPU Cache simulation

[Eduardo Habkost]

On Wed, Nov 18, 2015 at 03:04:09AM -0500, Hao Bai wrote:
> Hi all,
> 
> I read from an earlier thread in 2009 that QEMU did not simulate CPU
> caches. Is this still the case now?

Could you clarify what you need? In which architecture? What
exactly you mean by "simulating CPU caches"? Do you just want the
guest to think the machine has a specific cache topology, or do
you want to emulate other cache behavior? Do you need support to
specific cache control instructions?

Do you have a pointer to the 2009 thread?

-- 
Eduardo

Re: [Qemu-devel] [PATCH v7 02/24] blockjob: Call bdrv_unref() on creation error

[Kevin Wolf]

Am 09.11.2015 um 23:39 hat Max Reitz geschrieben:
> If block_job_create() fails, it should release its reference to the
> job's BDS. Normally, this is done in the callback provided by the
> caller, but that callback will not be invoked if the block job failed to
> be created.
> 
> Signed-off-by: Max Reitz 
> Reviewed-by: Alberto Garcia 
> Reviewed-by: Kevin Wolf 

Commit 18930ba3 already fixed this. Now adding another bdrv_unref()
would be wrong, so I guess we have to simply drop the patch.

NACK

Re: [Qemu-devel] [PATCH v2 2/5] sockets: remove use of QemuOpts from socket_listen

[Daniel P. Berrange]

On Wed, Nov 18, 2015 at 08:44:28AM -0700, Eric Blake wrote:
> On 11/18/2015 03:08 AM, Daniel P. Berrange wrote:
> > On Tue, Nov 17, 2015 at 03:22:04PM -0700, Eric Blake wrote:
> >> On 11/17/2015 10:00 AM, Daniel P. Berrange wrote:
> >>> The socket_listen method accepts a QAPI SocketAddress object
> >>> which it then turns into QemuOpts before calling the
> >>> inet_listen_opts/unix_listen_opts helper methods. By
> >>> converting the latter to use QAPI SocketAddress directly,
> >>> the QemuOpts conversion step can be eliminated
> >>>
> >>> This also fixes the problem where ipv4=off && ipv6=off
> >>> would be treated the same as ipv4=on && ipv6=on
> >>>
> 
> >>> + *  ipv4  ipv6   family
> >>> + *   - -   PF_UNSPEC
> 
> This says I have no preference, so pick the one that works.
> 
> >>> + *   - f   PF_INET
> >>> + *   - t   PF_INET6
> >>> + *   f -   PF_INET6
> >>> + *   f f   
> >>> + *   f t   PF_INET6
> >>> + *   t -   PF_INET
> >>> + *   t f   PF_INET
> >>
> >> These I understand,
> 
> Generally to mean "I specifically requested this" or "I specifically
> don't want that", where there is no collision.
> 
> >>
> >>> + *   t t   PF_INET6
> >>
> >> but why is this one PF_INET6 instead of PF_UNSPEC?
> > 
> > If you use PF_UNSPEC, then the addresses we listen on will be automatically
> > deteremined by results of the DNS lookup. ie if DNS only returns an IPv4
> > address, it won't listen on IPv6.  When the user has said ipv6=on, then
> > they expect to get an error if it was not possible to listen on IPv6. So
> > we must use PF_INET6 here to ensure that error, otherwise ipv6=on & ipv4=on
> > would be no different from ipv6=- & ipv4=-.
> 
> But if I'm specifically requesting that both families be used, either
> that should be an error (we can't honor two families at once) or it
> should be allowed (use the family that makes sense), not a synonym for
> ipv6-only.

Yes, you are right that this code means  ipv6=t & ipv4=t is
essentially equivalent to ipv6=t & ipv4=-, but that is a
limitation of getaddrinfo().

To address this semantic flaw, when ipv6=t, then we need
better handling of the IPV6_V6ONLY flag to take account
of ipv4= setting, and then actually verify whether ipv4
really was enabled when we asked for it.  This is a
pre-existing bug that my patch is not making worse. I
will have a think about fixing it separately.

And yes, we so badly need a unit test to validate all
this logic, which I also want to look into...

Regards,
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] configure: preserve various environment variables in config.status

[Eric Blake]

On 11/18/2015 03:03 AM, Daniel P. Berrange wrote:

>>
>> Autoconf preserves CC, CFLAGS, LDFLAGS, LIBS, CPPFLAGS, and CPP by
>> default.  Also, PKG_CONFIG is typically preserved.  If you run libvirt's
>> './configure --help', you'll also notice a bunch of *_CFLAGS and *_LIBS
>> in the precious list starting under the label "Some influential
>> environment variables".
> 
> I'll add in env vars for all the commands like CC, CPP, MAKE, etc that
> QEMU's configure uses.  I've tried preserving various *FLAGS vars but
> the problem here is that configure will modify/augment those variables
> while it is running, so when we get to preserve the original flags we
> only have the munged version. In any case recommendation is to use
> --extra-cflags rather than CFLAGS, so I figure its not a big deal to
> skip preserving CFLAGS/LDFLAGS

Just one more case where we (perhaps needlessly) diverge from autotools
and make distro's life harder.  Oh well, it's not your fault that we
aren't using CFLAGS like other projects, and not preserving CFLAGS does
not change that.

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PULL v1 (for 2.5) 0/4] Fix misc memory leaks & bugs in crypto code

[Daniel P. Berrange]

Running test-crypto-tlscredsx509 & test-crypto-tlssession under
valgrind identified 3 memory leaks and one other bug. This short
series fixes them.

The following changes since commit c27e9014d56fa4880e7d741275d887c3a5949997:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-vnc-20151116-1' into 
staging (2015-11-17 12:34:07 +)

are available in the git repository at:

  git://github.com/berrange/qemu.git tags/qcrypto-fixes-20151118-1

for you to fetch changes up to 08cb175a24d642a40e41db2fef2892b0a1ab504e:

  crypto: avoid passing NULL to access() syscall (2015-11-18 15:42:26 +)


Pull qcrypto fixes 2015/11/18 v1



Daniel P. Berrange (4):
  crypto: fix leak of gnutls_dh_params_t data on credential unload
  crypto: fix mistaken setting of Error in success code path
  crypto: fix leaks in TLS x509 helper functions
  crypto: avoid passing NULL to access() syscall

 crypto/tlscredsx509.c   | 7 ++-
 crypto/tlssession.c | 4 ++--
 tests/crypto-tls-x509-helpers.c | 2 ++
 3 files changed, 10 insertions(+), 3 deletions(-)

-- 
2.5.0

[Qemu-devel] [PULL v1 (for 2.5) 1/4] crypto: fix leak of gnutls_dh_params_t data on credential unload

[Daniel P. Berrange]

The QCryptoTLSCredsX509 object was not free'ing the allocated
gnutls_dh_params_t data when unloading the credentials

Signed-off-by: Daniel P. Berrange 
---
 crypto/tlscredsx509.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index dc46bc4..c5d1a0d 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -654,6 +654,10 @@ qcrypto_tls_creds_x509_unload(QCryptoTLSCredsX509 *creds)
 gnutls_certificate_free_credentials(creds->data);
 creds->data = NULL;
 }
+if (creds->parent_obj.dh_params) {
+gnutls_dh_params_deinit(creds->parent_obj.dh_params);
+creds->parent_obj.dh_params = NULL;
+}
 }
 
 
-- 
2.5.0

[Qemu-devel] [PULL v1 (for 2.5) 2/4] crypto: fix mistaken setting of Error in success code path

[Daniel P. Berrange]

The qcrypto_tls_session_check_certificate() method was setting
an Error even when the ACL check suceeded. This didn't affect
the callers detection of errors because they relied on the
function return status, but this did cause a memory leak since
the caller would not free an Error they did not expect to be
set.

Signed-off-by: Daniel P. Berrange 
---
 crypto/tlssession.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crypto/tlssession.c b/crypto/tlssession.c
index ffc5c47..3735529 100644
--- a/crypto/tlssession.c
+++ b/crypto/tlssession.c
@@ -304,9 +304,9 @@ qcrypto_tls_session_check_certificate(QCryptoTLSSession 
*session,
 
 allow = qemu_acl_party_is_allowed(acl, session->peername);
 
-error_setg(errp, "TLS x509 ACL check for %s is %s",
-   session->peername, allow ? "allowed" : "denied");
 if (!allow) {
+error_setg(errp, "TLS x509 ACL check for %s is denied",
+   session->peername);
 goto error;
 }
 }
-- 
2.5.0

[Qemu-devel] [PULL v1 (for 2.5) 3/4] crypto: fix leaks in TLS x509 helper functions

[Daniel P. Berrange]

The test_tls_get_ipaddr() method forgot to free the returned data
from getaddrinfo().

The test_tls_write_cert_chain() method forgot to free the allocated
buffer holding the certificate data after writing it out to a file.

Signed-off-by: Daniel P. Berrange 
---
 tests/crypto-tls-x509-helpers.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/crypto-tls-x509-helpers.c b/tests/crypto-tls-x509-helpers.c
index c5de67b..47b4c7b 100644
--- a/tests/crypto-tls-x509-helpers.c
+++ b/tests/crypto-tls-x509-helpers.c
@@ -153,6 +153,7 @@ test_tls_get_ipaddr(const char *addrstr,
 *datalen = res->ai_addrlen;
 *data = g_new(char, *datalen);
 memcpy(*data, res->ai_addr, *datalen);
+freeaddrinfo(res);
 }
 
 /*
@@ -465,6 +466,7 @@ void test_tls_write_cert_chain(const char *filename,
 if (!g_file_set_contents(filename, buffer, offset, NULL)) {
 abort();
 }
+g_free(buffer);
 }
 
 
-- 
2.5.0

[Qemu-devel] [PULL 4/4] block: Call external_snapshot_clean after blockdev-snapshot

[Kevin Wolf]

From: Alberto Garcia 

Otherwise the AioContext will never be released.

Signed-off-by: Alberto Garcia 
Message-id: 1447419624-21918-1-git-send-email-be...@igalia.com
Reviewed-by: Fam Zheng 
Signed-off-by: Max Reitz 
---
 blockdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/blockdev.c b/blockdev.c
index 07c1741..313841b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2098,6 +2098,7 @@ static const BlkActionOps actions[] = {
 .prepare  = external_snapshot_prepare,
 .commit   = external_snapshot_commit,
 .abort = external_snapshot_abort,
+.clean = external_snapshot_clean,
 },
 [TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC] = {
 .instance_size = sizeof(ExternalSnapshotState),
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 0/3] virtio-net/vhost-net: share cross-endian enablement

[Greg Kurz]

Since QEMU 2.4.0, vhost-net uses the cross-endian support of TAP devices to
fix vnet headers. In fact, virtio-net can do the same instead of hackily
patching headers in virtio_net_hdr_swap().

This series moves the enablement of cross-endian support from vhost-net to
virtio-net: both vhost and full emulation can now benefit from it. Of course
we keep the current hack to fall back on when the backend doesn't support
cross-endian.

---

Greg Kurz (3):
  virtio-net: use the backend cross-endian capabilities
  Revert "vhost-net: tell tap backend about the vnet endianness"
  virtio: drop the virtio_needs_swap() helper


 hw/net/vhost_net.c|   33 +--
 hw/net/virtio-net.c   |   40 +++--
 include/hw/virtio/virtio-access.h |9 
 include/hw/virtio/virtio-net.h|1 +
 4 files changed, 40 insertions(+), 43 deletions(-)

[Qemu-devel] [PATCH v3 3/3] virtio: drop the virtio_needs_swap() helper

[Greg Kurz]

It is not used anymore.

Signed-off-by: Greg Kurz 
---
 include/hw/virtio/virtio-access.h |9 -
 1 file changed, 9 deletions(-)

diff --git a/include/hw/virtio/virtio-access.h 
b/include/hw/virtio/virtio-access.h
index 8aec843c8ff3..a01fff2e51d7 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -143,15 +143,6 @@ static inline uint64_t virtio_ldq_p(VirtIODevice *vdev, 
const void *ptr)
 }
 }
 
-static inline bool virtio_needs_swap(VirtIODevice *vdev)
-{
-#ifdef HOST_WORDS_BIGENDIAN
-return virtio_access_is_big_endian(vdev) ? false : true;
-#else
-return virtio_access_is_big_endian(vdev) ? true : false;
-#endif
-}
-
 static inline uint16_t virtio_tswap16(VirtIODevice *vdev, uint16_t s)
 {
 #ifdef HOST_WORDS_BIGENDIAN

Re: [Qemu-devel] [PATCH for 2.9 v8 0/10] dataplane snapshot fixes

[Juan Quintela]

"Denis V. Lunev"  wrote:
> On 11/18/2015 05:31 PM, Juan Quintela wrote:
>> Greg Kurz  wrote:
>>> On Tue, 17 Nov 2015 12:08:20 +0300
>>> "Denis V. Lunev"  wrote:
>>>
 with test
  while /bin/true ; do
  virsh snapshot-create rhel7
  sleep 10
  virsh snapshot-delete rhel7 --current
  done
 with enabled iothreads on a running VM leads to a lot of troubles: hangs,
 asserts, errors.

>>> In my case, when using a virtio-blk-dataplane device, calling savevm 
>>> *always*
>>> result in a QEMU hang.
>> Oops
>>
>>> With this series (plus the s/bs/bs_vm_state/ change in patch 11), 
>>> savevm/loadvm
>>> now works like a charm.
>> Nice, thanks for the testing.
>>
>>> I saw that Juan does not like aio_context being used in migration code, but
>>> in case this series gets applied anyway:
>>>
>>> Tested-by: Greg Kurz 
>> I *think* that we should get better API's exported from block layer, but
>> *at least* we will get this series in.
>>
>> Thanks, Juan.
>
> that is good to me. Current block level API is terrible and unclear.
>
> Greg is correct there.
>
> Should I resubmit the last patch or you will change this yourself?

Please, test and resend.

I am not have lots of experience testing that.

Thanks, Juan.

Re: [Qemu-devel] [PATCH v7 01/24] blockdev: Add missing bdrv_unref() in drive-backup

[Kevin Wolf]

Am 09.11.2015 um 23:39 hat Max Reitz geschrieben:
> All error paths after a successful bdrv_open() of target_bs should
> contain a bdrv_unref(target_bs). This one did not yet, so add it.
> 
> Signed-off-by: Max Reitz 
> Reviewed-by: Alberto Garcia 
> Reviewed-by: Kevin Wolf 

Thanks, applied for 2.5.

Kevin

Re: [Qemu-devel] [PATCH] configure: preserve various environment variables in config.status

[Stefan Weil]

Am 17.11.2015 um 20:06 schrieb Eric Blake:
> On 11/17/2015 10:59 AM, Daniel P. Berrange wrote:
[...]
>> +++ b/configure
>> @@ -5925,6 +5925,24 @@ cat >  # Compiler output produced by configure, useful for debugging
>>  # configure, is in config.log if it exists.
>>  EOD
>> +
>> +preserve_env() {
>> +envname=$1
>> +
>> +if test -n "${!envname}"
> Bashism, but configure is /bin/sh.  This won't work on dash :(
>
> I think you'll have to use eval, and we'll just have to audit that
> preserve_env can never be called with suspicious text where eval would
> open a security hole.

You could also call the function with two arguments ...
preserve_env PATH "$PATH"
... and use the 2nd argument in the test.

[Qemu-devel] [PULL v1 (for 2.5) 4/4] crypto: avoid passing NULL to access() syscall

[Daniel P. Berrange]

The qcrypto_tls_creds_x509_sanity_check() checks whether
certs exist by calling access(). It is valid for this
method to be invoked with certfile==NULL though, since
for client credentials the cert is optional. This caused
it to call access(NULL), which happens to be harmless on
current Linux, but should none the less be avoided.

Signed-off-by: Daniel P. Berrange 
---
 crypto/tlscredsx509.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index c5d1a0d..d080deb 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -485,7 +485,8 @@ qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 
*creds,
 int ret = -1;
 
 memset(cacerts, 0, sizeof(cacerts));
-if (access(certFile, R_OK) == 0) {
+if (certFile &&
+access(certFile, R_OK) == 0) {
 cert = qcrypto_tls_creds_load_cert(creds,
certFile, isServer,
errp);
-- 
2.5.0

Re: [Qemu-devel] [PATCH v3 1/3] target-i386: add pkeys support for cpuid handling

[Eduardo Habkost]

On Wed, Nov 18, 2015 at 10:20:15AM +0800, Huaitong Han wrote:
[...]
> @@ -408,6 +420,13 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] 
> = {
>  .cpuid_reg = R_EBX,
>  .tcg_features = TCG_7_0_EBX_FEATURES,
>  },
> +[FEAT_7_0_ECX] = {
> +.feat_names = cpuid_7_0_ecx_feature_name,
> +.cpuid_eax = 7,
> +.cpuid_needs_ecx = true, .cpuid_ecx = 0,
> +.cpuid_reg = R_ECX,
> +.tcg_features = TCG_7_0_ECX_FEATURES,
> +},

The patch looks good, but when we add the feature names to
cpuid_7_0_ecx_feature_name, QEMU will consider them as
migratable, but they are truly migratable only after we add the
ext_save_areas entry.

We can fix this by moving cpuid_7_0_ecx_feature_name to patch 2/3
(or to a separate patch, to be applied after 2/3).

-- 
Eduardo

Re: [Qemu-devel] [PATCH v12 19/36] blkdebug: Merge hand-rolled and qapi BlkdebugEvent enum

[Kevin Wolf]

Am 18.11.2015 um 17:26 hat Eric Blake geschrieben:
> On 11/18/2015 05:08 AM, Kevin Wolf wrote:
> > Am 18.11.2015 um 11:30 hat Markus Armbruster geschrieben:
> >> Eric Blake  writes:
> >>
> >>> No need to keep two separate enums, where editing one is likely
> >>> to forget the other.  Now that we can specify a qapi enum prefix,
> >>> we don't even have to change the bulk of the uses.
> >>>
> 
> >>>  ##
> >>> -{ 'enum': 'BlkdebugEvent',
> >>> +{ 'enum': 'BlkdebugEvent', 'prefix': 'BLKDBG',
> >>>'data': [ 'l1_update', 'l1_grow.alloc_table', 'l1_grow.write_table',
> >>>  'l1_grow.activate_table', 'l2_load', 'l2_update',
> >>>  'l2_update_compressed', 'l2_alloc.cow_read', 
> >>> 'l2_alloc.write',
> >>
> >> I'm no friend of the 'prefix' feature.  You could avoid it here by
> >> renaming BlkdebugEvent to Blkdbg.  No additional churn, because you
> >> already replace hand-written BlkDebugEvent by generated BlkdebugEvent.
> >>
> >> Alternatively, you could reduce churn by renaming it to BlkDebugEvent.
> >>
> >> Matter of taste.  Perhaps Kevin has a preference.
> > 
> > Wouldn't that mean that we end up with a C type called Blkdbg? In my
> > opinion that's a bit unspecific, so if you ask me, I would paint my
> > bikeshed in a different colour.
> 
> Most of the existing qapi names are Blkdebug, it was only the internal
> enum being replaced that used BlkDebug.  Also, qapi would munge BlkDebug
> into BLK_DEBUG, so I think we want to stick with the lowercase 'd' so
> that the munging (without 'prefix') would stick to BLKDEBUG.

Oh, I don't really have an opinion whether BlockDebug, BlkDebug or
Blkdebug is the best spelling. I think Blkdebug makes sense.

But I do have a strong opinion on whether the enum should be called
BlkdebugEvent or just Blkdbg (without any mention of "Event"). The
latter doesn't describe any more what the type is about.

> I'm also fine if you want me to do a followup patch that renames all
> enums from BLKDBG_L1_UPDATE to BLKDEBUG_EVENT_L1_UPDATE, at which point
> we could drop the 'prefix' (I don't know how many lines it would make
> long enough to need different wrapping, but modulo wrapping it would all
> be a mechanically scripted change).

I don't mind the 'prefix' feature. I think it's nice to have short, but
still unique enough constant names, but if Markus is bothered enough to
send a patch, I can live with BLKDEBUG_EVENT_*.

Kevin


pgpIXIZ6TfEN_.pgp
Description: PGP signature

[Qemu-devel] [PATCH 2/2] input: linux evdev support

[Gerd Hoffmann]

This patch adds support for reading input events directly from linux
evdev devices and forward them to the guest.  Unlike virtio-input-host
which simply passes on all events to the guest without looking at them
this will interpret the events and feed them into the qemu input
subsystem.

Therefore this is limited to what the qemu input subsystem and the
emulated input devices are able to handle.  Also there is no support for
absolute coordinates (tablet/touchscreen).  So we are talking here about
basic mouse and keyboard support.

The advantage is that it'll work without virtio-input drivers in the
guest, the events are delivered to the usual ps/2 or usb input devices
(depending on what the machine happens to have).  And for keyboards
qemu is able to switch the keyboard between guest and host on hotkey.
The hotkey is hard-coded for now (both control keys), initialy the
guest owns the keyboard.

Probably most useful when assigning vga devices with vfio and using a
physical monitor instead of vnc/spice/gtk as guest display.

Usage:  Add '-input-linux /dev/input/event' to the qemu command
line.  Note that udev has rules which populate /dev/input/by-{id,path}
with static names, which might be more convinient to use.

Signed-off-by: Gerd Hoffmann 
---
 include/ui/input.h |   2 +
 qemu-options.hx|   9 +++
 ui/Makefile.objs   |   1 +
 ui/input-linux.c   | 194 +
 vl.c   |  11 +++
 5 files changed, 217 insertions(+)
 create mode 100644 ui/input-linux.c

diff --git a/include/ui/input.h b/include/ui/input.h
index d7afd80..443742e 100644
--- a/include/ui/input.h
+++ b/include/ui/input.h
@@ -68,4 +68,6 @@ void qemu_input_check_mode_change(void);
 void qemu_add_mouse_mode_change_notifier(Notifier *notify);
 void qemu_remove_mouse_mode_change_notifier(Notifier *notify);
 
+int input_linux_init(void *opaque, QemuOpts *opts, Error **errp);
+
 #endif /* INPUT_H */
diff --git a/qemu-options.hx b/qemu-options.hx
index 0eea4ee..a2d7338 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1165,6 +1165,15 @@ STEXI
 Set the initial graphical resolution and depth (PPC, SPARC only).
 ETEXI
 
+DEF("input-linux", 1, QEMU_OPTION_input_linux,
+"-input-linux \n"
+"Use input device.\n", QEMU_ARCH_ALL)
+STEXI
+@item -input-linux @var{dev}
+@findex -input-linux
+Use input device.
+ETEXI
+
 DEF("vnc", HAS_ARG, QEMU_OPTION_vnc ,
 "-vnc displaystart a VNC server on display\n", QEMU_ARCH_ALL)
 STEXI
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 728393c..dc936f1 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -9,6 +9,7 @@ vnc-obj-y += vnc-jobs.o
 
 common-obj-y += keymaps.o console.o cursor.o qemu-pixman.o
 common-obj-y += input.o input-keymap.o input-legacy.o
+common-obj-$(CONFIG_LINUX) += input-linux.o
 common-obj-$(CONFIG_SPICE) += spice-core.o spice-input.o spice-display.o
 common-obj-$(CONFIG_SDL) += sdl.mo x_keymap.o
 common-obj-$(CONFIG_COCOA) += cocoa.o
diff --git a/ui/input-linux.c b/ui/input-linux.c
new file mode 100644
index 000..3c8d587
--- /dev/null
+++ b/ui/input-linux.c
@@ -0,0 +1,194 @@
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#include "qemu-common.h"
+#include "qemu/config-file.h"
+#include "qemu/sockets.h"
+#include "sysemu/sysemu.h"
+#include "ui/input.h"
+
+#include 
+#include "standard-headers/linux/input.h"
+
+typedef struct InputLinux InputLinux;
+
+struct InputLinux {
+const char  *evdev;
+int fd;
+boolgrab_request;
+boolgrab_active;
+boolkeydown[KEY_CNT];
+int keycount;
+};
+
+static void input_linux_toggle_grab(InputLinux *il)
+{
+intptr_t request = !il->grab_active;
+int rc;
+
+rc = ioctl(il->fd, EVIOCGRAB, request);
+if (rc < 0) {
+return;
+}
+il->grab_active = !il->grab_active;
+}
+
+static void input_linux_event_keyboard(void *opaque)
+{
+InputLinux *il = opaque;
+struct input_event event;
+int rc;
+
+for (;;) {
+rc = read(il->fd, , sizeof(event));
+if (rc != sizeof(event)) {
+break;
+}
+
+switch (event.type) {
+case EV_KEY:
+/* keep track of key state */
+if (!il->keydown[event.code] && event.value) {
+il->keydown[event.code] = true;
+il->keycount++;
+}
+if (il->keydown[event.code] && !event.value) {
+il->keydown[event.code] = false;
+il->keycount--;
+}
+
+/* send event to guest when grab is active */
+if (il->grab_active) {
+int qcode = qemu_input_linux_to_qcode(event.code);
+qemu_input_event_send_key_qcode(NULL, qcode, event.value);
+}
+
+/* hotkey -> record switch request ... 

[Qemu-devel] [PATCH 1/2] input: add qemu_input_qcode_to_linux + qemu_input_linux_to_qcode

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 include/ui/input.h |   3 ++
 ui/input-keymap.c  | 145 +
 2 files changed, 148 insertions(+)

diff --git a/include/ui/input.h b/include/ui/input.h
index d06a12d..d7afd80 100644
--- a/include/ui/input.h
+++ b/include/ui/input.h
@@ -48,6 +48,9 @@ int qemu_input_key_value_to_qcode(const KeyValue *value);
 int qemu_input_key_value_to_scancode(const KeyValue *value, bool down,
  int *codes);
 
+int qemu_input_qcode_to_linux(unsigned int qcode);
+int qemu_input_linux_to_qcode(unsigned int lnx);
+
 InputEvent *qemu_input_event_new_btn(InputButton btn, bool down);
 void qemu_input_queue_btn(QemuConsole *src, InputButton btn, bool down);
 void qemu_input_update_buttons(QemuConsole *src, uint32_t *button_map,
diff --git a/ui/input-keymap.c b/ui/input-keymap.c
index d36be4b..31f4daa 100644
--- a/ui/input-keymap.c
+++ b/ui/input-keymap.c
@@ -2,6 +2,126 @@
 #include "ui/keymaps.h"
 #include "ui/input.h"
 
+#include "standard-headers/linux/input.h"
+
+/* FIXME: duplicate, see hw/input/virtio-input-hid.c */
+static const unsigned int qcode_to_linux[Q_KEY_CODE_MAX] = {
+[Q_KEY_CODE_ESC] = KEY_ESC,
+[Q_KEY_CODE_1]   = KEY_1,
+[Q_KEY_CODE_2]   = KEY_2,
+[Q_KEY_CODE_3]   = KEY_3,
+[Q_KEY_CODE_4]   = KEY_4,
+[Q_KEY_CODE_5]   = KEY_5,
+[Q_KEY_CODE_6]   = KEY_6,
+[Q_KEY_CODE_7]   = KEY_7,
+[Q_KEY_CODE_8]   = KEY_8,
+[Q_KEY_CODE_9]   = KEY_9,
+[Q_KEY_CODE_0]   = KEY_0,
+[Q_KEY_CODE_MINUS]   = KEY_MINUS,
+[Q_KEY_CODE_EQUAL]   = KEY_EQUAL,
+[Q_KEY_CODE_BACKSPACE]   = KEY_BACKSPACE,
+
+[Q_KEY_CODE_TAB] = KEY_TAB,
+[Q_KEY_CODE_Q]   = KEY_Q,
+[Q_KEY_CODE_W]   = KEY_W,
+[Q_KEY_CODE_E]   = KEY_E,
+[Q_KEY_CODE_R]   = KEY_R,
+[Q_KEY_CODE_T]   = KEY_T,
+[Q_KEY_CODE_Y]   = KEY_Y,
+[Q_KEY_CODE_U]   = KEY_U,
+[Q_KEY_CODE_I]   = KEY_I,
+[Q_KEY_CODE_O]   = KEY_O,
+[Q_KEY_CODE_P]   = KEY_P,
+[Q_KEY_CODE_BRACKET_LEFT]= KEY_LEFTBRACE,
+[Q_KEY_CODE_BRACKET_RIGHT]   = KEY_RIGHTBRACE,
+[Q_KEY_CODE_RET] = KEY_ENTER,
+
+[Q_KEY_CODE_CTRL]= KEY_LEFTCTRL,
+[Q_KEY_CODE_A]   = KEY_A,
+[Q_KEY_CODE_S]   = KEY_S,
+[Q_KEY_CODE_D]   = KEY_D,
+[Q_KEY_CODE_F]   = KEY_F,
+[Q_KEY_CODE_G]   = KEY_G,
+[Q_KEY_CODE_H]   = KEY_H,
+[Q_KEY_CODE_J]   = KEY_J,
+[Q_KEY_CODE_K]   = KEY_K,
+[Q_KEY_CODE_L]   = KEY_L,
+[Q_KEY_CODE_SEMICOLON]   = KEY_SEMICOLON,
+[Q_KEY_CODE_APOSTROPHE]  = KEY_APOSTROPHE,
+[Q_KEY_CODE_GRAVE_ACCENT]= KEY_GRAVE,
+
+[Q_KEY_CODE_SHIFT]   = KEY_LEFTSHIFT,
+[Q_KEY_CODE_BACKSLASH]   = KEY_BACKSLASH,
+[Q_KEY_CODE_LESS]= KEY_102ND,
+[Q_KEY_CODE_Z]   = KEY_Z,
+[Q_KEY_CODE_X]   = KEY_X,
+[Q_KEY_CODE_C]   = KEY_C,
+[Q_KEY_CODE_V]   = KEY_V,
+[Q_KEY_CODE_B]   = KEY_B,
+[Q_KEY_CODE_N]   = KEY_N,
+[Q_KEY_CODE_M]   = KEY_M,
+[Q_KEY_CODE_COMMA]   = KEY_COMMA,
+[Q_KEY_CODE_DOT] = KEY_DOT,
+[Q_KEY_CODE_SLASH]   = KEY_SLASH,
+[Q_KEY_CODE_SHIFT_R] = KEY_RIGHTSHIFT,
+
+[Q_KEY_CODE_ALT] = KEY_LEFTALT,
+[Q_KEY_CODE_SPC] = KEY_SPACE,
+[Q_KEY_CODE_CAPS_LOCK]   = KEY_CAPSLOCK,
+
+[Q_KEY_CODE_F1]  = KEY_F1,
+[Q_KEY_CODE_F2]  = KEY_F2,
+[Q_KEY_CODE_F3]  = KEY_F3,
+[Q_KEY_CODE_F4]  = KEY_F4,
+[Q_KEY_CODE_F5]  = KEY_F5,
+[Q_KEY_CODE_F6]  = KEY_F6,
+[Q_KEY_CODE_F7]  = KEY_F7,
+[Q_KEY_CODE_F8]  = KEY_F8,
+[Q_KEY_CODE_F9]  = KEY_F9,
+[Q_KEY_CODE_F10] = KEY_F10,
+[Q_KEY_CODE_NUM_LOCK]= KEY_NUMLOCK,
+[Q_KEY_CODE_SCROLL_LOCK] = KEY_SCROLLLOCK,
+
+[Q_KEY_CODE_KP_0]= KEY_KP0,
+[Q_KEY_CODE_KP_1]= KEY_KP1,
+[Q_KEY_CODE_KP_2]= KEY_KP2,
+[Q_KEY_CODE_KP_3]= KEY_KP3,
+[Q_KEY_CODE_KP_4]= KEY_KP4,
+[Q_KEY_CODE_KP_5]= KEY_KP5,
+[Q_KEY_CODE_KP_6]= 

Re: [Qemu-devel] [PATCH v12 17/36] qapi: Fix c_name() munging

[Markus Armbruster]

Eric Blake  writes:

> On 11/18/2015 03:18 AM, Markus Armbruster wrote:
>> Eric Blake  writes:
>> 
>>> *** WARNING: THE ATTACHED DOCUMENT(S) CONTAIN MACROS ***
>>> *** MACROS MAY CONTAIN MALICIOUS CODE ***
>>> *** Open only if you can verify and trust the sender ***
>>> *** Please contact info...@redhat.com if you have questions or concerns **
>> 
>> Another one.
>> 
>>> The method c_name() is supposed to do two different actions: munge
>>> '-' into '_', and add a 'q_' prefix to ticklish names.  But it did
>>> these steps out of order, making it possible to submit input that
>>> is not ticklish until after munging, where the output then lacked
>>> the desired prefix.
>>>
>>> The failure is exposed easily if you have a compiler that recognizes
>>> C11 keywords, and try to name a member '_Thread-local', as it would
>>> result in trying to compile the declaration 'uint64_t _Thread_local;'
>>> which is not valid.  However, this name violates our conventions
>>> (ultimately, want to enforce that no qapi names start with single
>>> underscore), so the test is slightly weaker by instead testing
>>> 'wchar-t'; the declaration 'uint64_t wchar_t;' is valid in C (where
>>> wchar_t is only a typedef) but fails with a C++ compiler (where it
>>> is a keyword).
>> 
>> Do we support compiling it with a C++ compiler?  To sidestep the
>> question, I'd say "but would fail".
>
> I know we require a C++ compiler for qga on Windows, and qga uses qapi,
> so I think the problem is real; but as I do not have a working setup for
> compiling qga for windows, I can only speculate.  Changing s/fails/but
> would fail/ is a nice hedge.
>
>> 
>> In my private opinion, the one sane way to compile C code with a C++
>> compiler is wrapping it in extern "C" { ... }.
>
> True - but I don't think that changes the set of C++ keywords inside the
> extern block, does it?  (The thought of context-sensitive keywords makes
> me cringe for how one would write a sane parser for that language).

The truly sane way to compile C with a C++ compiler is of course "not":
compile it with C and link it.

The exception is headers.  Stick to declaring extern symbols and the
types they need.

>>> Fix things by reversing the order of actions within c_name().
>>>
>>> Signed-off-by: Eric Blake 
>> 
>> Again, just commit message polish, can do on merge.
>> 
>
> Don't know why you got it on some messages and not others; I also got a
> round of those pollutions on other threads.  It looks like the
> responsible party has cleaned up their false positives in the meantime,
> so hopefully we aren't hit by more of the annoyance.

I reported it, and was told it's been fixed.

Re: [Qemu-devel] [PATCH v12 19/36] blkdebug: Merge hand-rolled and qapi BlkdebugEvent enum

[Markus Armbruster]

Kevin Wolf  writes:

> Am 18.11.2015 um 17:26 hat Eric Blake geschrieben:
>> On 11/18/2015 05:08 AM, Kevin Wolf wrote:
>> > Am 18.11.2015 um 11:30 hat Markus Armbruster geschrieben:
>> >> Eric Blake  writes:
>> >>
>> >>> No need to keep two separate enums, where editing one is likely
>> >>> to forget the other.  Now that we can specify a qapi enum prefix,
>> >>> we don't even have to change the bulk of the uses.
>> >>>
>> 
>> >>>  ##
>> >>> -{ 'enum': 'BlkdebugEvent',
>> >>> +{ 'enum': 'BlkdebugEvent', 'prefix': 'BLKDBG',
>> >>>'data': [ 'l1_update', 'l1_grow.alloc_table', 'l1_grow.write_table',
>> >>>  'l1_grow.activate_table', 'l2_load', 'l2_update',
>> >>>  'l2_update_compressed', 'l2_alloc.cow_read', 
>> >>> 'l2_alloc.write',
>> >>
>> >> I'm no friend of the 'prefix' feature.  You could avoid it here by
>> >> renaming BlkdebugEvent to Blkdbg.  No additional churn, because you
>> >> already replace hand-written BlkDebugEvent by generated BlkdebugEvent.
>> >>
>> >> Alternatively, you could reduce churn by renaming it to BlkDebugEvent.
>> >>
>> >> Matter of taste.  Perhaps Kevin has a preference.
>> > 
>> > Wouldn't that mean that we end up with a C type called Blkdbg? In my
>> > opinion that's a bit unspecific, so if you ask me, I would paint my
>> > bikeshed in a different colour.
>> 
>> Most of the existing qapi names are Blkdebug, it was only the internal
>> enum being replaced that used BlkDebug.  Also, qapi would munge BlkDebug
>> into BLK_DEBUG, so I think we want to stick with the lowercase 'd' so
>> that the munging (without 'prefix') would stick to BLKDEBUG.
>
> Oh, I don't really have an opinion whether BlockDebug, BlkDebug or
> Blkdebug is the best spelling. I think Blkdebug makes sense.
>
> But I do have a strong opinion on whether the enum should be called
> BlkdebugEvent or just Blkdbg (without any mention of "Event"). The
> latter doesn't describe any more what the type is about.

Okay, we'll take the patch as is then.

>> I'm also fine if you want me to do a followup patch that renames all
>> enums from BLKDBG_L1_UPDATE to BLKDEBUG_EVENT_L1_UPDATE, at which point
>> we could drop the 'prefix' (I don't know how many lines it would make
>> long enough to need different wrapping, but modulo wrapping it would all
>> be a mechanically scripted change).
>
> I don't mind the 'prefix' feature. I think it's nice to have short, but
> still unique enough constant names, but if Markus is bothered enough to
> send a patch, I can live with BLKDEBUG_EVENT_*.

Fair enough :)  For now, let's move the queue forward.

Re: [Qemu-devel] RFC: raspberry pi / pi2 / Windows-on-ARM support

[Andrew Baumann]

Hi Peter,

Thanks for your feedback!

From: Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com]
Sent: Tuesday, 17 November 2015 23:51
> I haven't looked beyond the diffstat yet, but a top level
> architectural comment, I only see the one file in hw/arm. We are
> promoting the split of board and SoC now as two separate objects. Each
> of the patch series linked above demonstrates this. The BCM SoC would
> be an object in its own right, then the board instantiates the SoC as
> a composite device.

I understand the rationale here, but is there much value pursuing this for the 
Pi? As far as I know, the Pi1/2 SOCs (bcm2835 and 2836) are one-off designs 
used only in the Pi. Moreover, the 2836 is basically a copy of the 2835 with 
the addition of a new block for a multi-core interrupt controller and 
mailboxes, so trying to split those out into two separate SOC definition files 
would require either lots of duplication, or a tight coupling between the two. 
Finally, there is basically nothing on the board besides the SoC and RAM (just 
a USB-Ethernet adapter), so the actual board definition files would be almost 
empty. As it is, I've already factored out all the common devices within 
hw/arm/raspi.c -- my feeling is that this is a cleaner way to do it for the Pi.

> It is big. Work of this magnitude is generally better managed as
> multiple series (each of multiple patches). A general approach with
> new ARM boards is the first series is the bare minimum needed to get a
> linux boot over the UART console. This usually means any CPU
> customisations, timer, UART, intc, SoC and boards, maybe the
> system/power controllers (if the kernel touches them in boot) and
> optionally network or emmc. With network you can do an NFS root (this
> was the only option on Allwinner/cubieboard for quite some time) and
> with emmc you can do your root=/dev/mmcblk0p2 style boot as normal. If
> you are using existing lan9118, network first might be a clear win,
> although emmc is more realistic. Can always do both.
> 
> The you can follow up with the USB, DMA, AUX, ... each independently
> or as a follow up series'.
> 
> Any core code and ARM CPU work that is a bugfix or stands in its own
> right, you should send straight up as a separate work.

Ok, understood. Even following this guidance, the first patch will be big -- Pi 
has custom interrupt and DMA controllers, and the GPU gets itself involved in 
the boot process. I think we could probably leave out fb/usb and maybe also 
dma, mphi, vchiq and power, but most of the rest will be needed to boot 
anything.

BTW, I used lan9118 as a kludge for Windows, but it isn't present on a real Pi, 
and the device definition no longer instantiates it. At present, there is no 
usable network device.

> Curious, does rPi kernel support device tree boot? This would be nice
> for testing with a generic kernel.

I believe it does on recent Raspbian images (I see a number of dtb files in 
Raspbian's boot partition), but I haven't tried to figure out how to use them. 
In general, we have limitations where the bootloader is concerned. On Pi HW, 
the boot process is driven by the GPU, which reads a plethora of options from 
config files (config.txt and cmdline.txt) on the boot partition. We don't 
emulate any of that stuff... instead, the previous raspi code did just enough 
to boot Linux using qemu's Linux loader support, and I figured out how to make 
UEFI happy on raspi2 so it can boot Windows, but that's about all.

I will take a look at the patches you referred to, and try to refactor the 
devices accordingly.

BTW, my main goal here is Windows support (surprise! :), so if anyone else is 
willing to help out with Linux that would be much appreciated. The kernel 
currently limps into early userspace before crashing on a SETEND instruction, 
and is clearly not as happy as it was when the original Pi1 support was being 
actively developed ~2 years ago.

Thanks,
Andrew

Re: [Qemu-devel] [PATCH v12 28/36] qapi: Simplify QObject

[Markus Armbruster]

Eric Blake  writes:

> The QObject hierarchy is small enough, and unlikely to grow further
> (since we only use it to map to JSON and already cover all JSON
> types), that we can simplify things by not tracking a separate
> vtable, but just inline all two elements of the vtable QType
> directly into QObject.  We can drop qnull_destroy_obj() in the
> process.
>
> This also has the nice benefit of moving the typename 'QType'
> out of the way, so that the next patch can repurpose it for a
> nicer name for 'qtype_code'.
>
> The various objects are now 8 bytes larger on 64-bit platforms, but
> hopefully this is in the noise.  If size is absolutely critical due
> to cache line speed effects, we could do some bit packing to
> restore the previous size (qtype_code currently only requires three
> distinct bits, so we could assert an assumption that architecture
> ABIs will give function pointers at least an 8-byte alignment, and
> that the three low-order bits of destroy are always zero; or we
> could use a bitfield for refcnt, with a maximum of SIZE_MAX/8, and
> given that a QObject already occupies at least 8 bytes, that won't
> artificially limit us). Or we could expose the currently static
> destroy methods and instead create a table in qobject.h that maps
> QType to destructor.

That's what I'd do.

A type registration function would keep the destructors static.  But why
bother.

>   But let's save any size compression for a
> followup patch, if at all.

Okay.

> On the other hand, by avoiding a separate vtable, there is less
> indirection, so use of QObjects may be slightly more efficient.
>
> However, as we don't have evidence pointing at either QObject size
> or access speed as being a hot spot, I didn't think it was worth
> benchmarking.
>
> Suggested-by: Markus Armbruster 
> Signed-off-by: Eric Blake 

Works for me.

Re: [Qemu-devel] [PATCH 2/2] tap-win32: disable broken async write path

[Andrew Baumann]

From: Stefan Weil [mailto:s...@weilnetz.de]
Sent: Tuesday, 17 November 2015 23:40
> > +#ifdef DEBUG_TAP_WIN32
> > +LPVOID msgbuf;
> 
> Does this also work ...
> 
> char *msgbuf;
> 
> 
> > +error = GetLastError();
> > +
> FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER|FORMAT_MESSA
> GE_FROM_SYSTEM,
> > +  NULL, error, MAKELANGID(LANG_NEUTRAL,
> SUBLANG_DEFAULT),
> > +  (LPTSTR), 0, NULL);
> 
> ... and remove the type cast here? If it works without compiler
> warnings, I'd prefer that variant.

Thanks Stefan. I was able to change the declaration to LPTSTR and avoid the 
cast, but am reticent about changing it to a bare char *, because the 
definition of LPTSTR depends on #ifdef UNICODE. I also fixed a lurking bug I 
had accidentally introduced in the event that WriteFile completes synchronously.

Revised patch incoming shortly...

Andrew

Re: [Qemu-devel] [PATCH v4 02/17] Add a base IPMI interface

[Corey Minyard]

I haven't heard any more comments on this series, should I resubmit with
the one shutdown change?

-corey
On Nov 12, 2015 1:02 PM,  wrote:

> From: Corey Minyard 
>
> Add the basic IPMI types and infrastructure to QEMU.  Low-level
> interfaces and simulation interfaces will register with this; it's
> kind of the go-between to tie them together.
>
> Signed-off-by: Corey Minyard 
> ---
>  default-configs/i386-softmmu.mak   |   1 +
>  default-configs/x86_64-softmmu.mak |   1 +
>  hw/Makefile.objs   |   1 +
>  hw/ipmi/Makefile.objs  |   1 +
>  hw/ipmi/ipmi.c | 125 ++
>  include/hw/ipmi/ipmi.h | 178
> +
>  qemu-doc.texi  |   2 +
>  7 files changed, 309 insertions(+)
>  create mode 100644 hw/ipmi/Makefile.objs
>  create mode 100644 hw/ipmi/ipmi.c
>  create mode 100644 include/hw/ipmi/ipmi.h
>
> diff --git a/default-configs/i386-softmmu.mak
> b/default-configs/i386-softmmu.mak
> index 43c96d1..8fa751a 100644
> --- a/default-configs/i386-softmmu.mak
> +++ b/default-configs/i386-softmmu.mak
> @@ -9,6 +9,7 @@ CONFIG_VGA_CIRRUS=y
>  CONFIG_VMWARE_VGA=y
>  CONFIG_VIRTIO_VGA=y
>  CONFIG_VMMOUSE=y
> +CONFIG_IPMI=y
>  CONFIG_SERIAL=y
>  CONFIG_PARALLEL=y
>  CONFIG_I8254=y
> diff --git a/default-configs/x86_64-softmmu.mak
> b/default-configs/x86_64-softmmu.mak
> index dfb8095..6767f4f 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -9,6 +9,7 @@ CONFIG_VGA_CIRRUS=y
>  CONFIG_VMWARE_VGA=y
>  CONFIG_VIRTIO_VGA=y
>  CONFIG_VMMOUSE=y
> +CONFIG_IPMI=y
>  CONFIG_SERIAL=y
>  CONFIG_PARALLEL=y
>  CONFIG_I8254=y
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index 7e7c241..4a07ed4 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -13,6 +13,7 @@ devices-dirs-$(CONFIG_SOFTMMU) += ide/
>  devices-dirs-$(CONFIG_SOFTMMU) += input/
>  devices-dirs-$(CONFIG_SOFTMMU) += intc/
>  devices-dirs-$(CONFIG_IPACK) += ipack/
> +devices-dirs-$(CONFIG_IPMI) += ipmi/
>  devices-dirs-$(CONFIG_SOFTMMU) += isa/
>  devices-dirs-$(CONFIG_SOFTMMU) += misc/
>  devices-dirs-$(CONFIG_SOFTMMU) += net/
> diff --git a/hw/ipmi/Makefile.objs b/hw/ipmi/Makefile.objs
> new file mode 100644
> index 000..65bde11
> --- /dev/null
> +++ b/hw/ipmi/Makefile.objs
> @@ -0,0 +1 @@
> +common-obj-$(CONFIG_IPMI) += ipmi.o
> diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
> new file mode 100644
> index 000..7d17469
> --- /dev/null
> +++ b/hw/ipmi/ipmi.c
> @@ -0,0 +1,125 @@
> +/*
> + * QEMU IPMI emulation
> + *
> + * Copyright (c) 2015 Corey Minyard, MontaVista Software, LLC
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining
> a copy
> + * of this software and associated documentation files (the "Software"),
> to deal
> + * in the Software without restriction, including without limitation the
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or
> sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> IN
> + * THE SOFTWARE.
> + */
> +
> +#include "hw/hw.h"
> +#include "hw/ipmi/ipmi.h"
> +#include "sysemu/sysemu.h"
> +#include "qmp-commands.h"
> +#include "qom/object_interfaces.h"
> +#include "qapi/visitor.h"
> +
> +static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
> +{
> +switch (op) {
> +case IPMI_RESET_CHASSIS:
> +if (checkonly) {
> +return 0;
> +}
> +qemu_system_reset_request();
> +return 0;
> +
> +case IPMI_POWEROFF_CHASSIS:
> +if (checkonly) {
> +return 0;
> +}
> +qemu_system_powerdown_request();
> +return 0;
> +
> +case IPMI_SEND_NMI:
> +if (checkonly) {
> +return 0;
> +}
> +qemu_mutex_lock_iothread();
> +qmp_inject_nmi(NULL);
> +qemu_mutex_unlock_iothread();
> +return 0;
> +
> +case IPMI_POWERCYCLE_CHASSIS:
> +case IPMI_PULSE_DIAG_IRQ:
> +case IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP:
> +case IPMI_POWERON_CHASSIS:
> +default:
> +return IPMI_CC_COMMAND_NOT_SUPPORTED;
> +}
> +}
> +
> +static void 

[Qemu-devel] [PATCH v3 7/9] io: add QIOChannelWebsock class

[Daniel P. Berrange]

Add a QIOChannel subclass that can run the websocket protocol over
the top of another QIOChannel instance. This initial implementation
is only capable of acting as a websockets server. There is no support
for acting as a websockets client yet.

Signed-off-by: Daniel P. Berrange 
---
 include/io/channel-websock.h | 108 +
 io/Makefile.objs |   1 +
 io/channel-websock.c | 975 +++
 trace-events |   8 +
 4 files changed, 1092 insertions(+)
 create mode 100644 include/io/channel-websock.h
 create mode 100644 io/channel-websock.c

diff --git a/include/io/channel-websock.h b/include/io/channel-websock.h
new file mode 100644
index 000..0dc21cc
--- /dev/null
+++ b/include/io/channel-websock.h
@@ -0,0 +1,108 @@
+/*
+ * QEMU I/O channels driver websockets
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_CHANNEL_WEBSOCK_H__
+#define QIO_CHANNEL_WEBSOCK_H__
+
+#include "io/channel.h"
+#include "qemu/buffer.h"
+#include "io/task.h"
+
+#define TYPE_QIO_CHANNEL_WEBSOCK "qio-channel-websock"
+#define QIO_CHANNEL_WEBSOCK(obj) \
+OBJECT_CHECK(QIOChannelWebsock, (obj), TYPE_QIO_CHANNEL_WEBSOCK)
+
+typedef struct QIOChannelWebsock QIOChannelWebsock;
+typedef union QIOChannelWebsockMask QIOChannelWebsockMask;
+
+union QIOChannelWebsockMask {
+char c[4];
+uint32_t u;
+};
+
+/**
+ * QIOChannelWebsock
+ *
+ * The QIOChannelWebsock class provides a channel wrapper which
+ * can transparently run the HTTP websockets protocol. This is
+ * usually used over a TCP socket, but there is actually no
+ * technical restriction on which type of master channel is
+ * used as the transport.
+ *
+ * This channel object is currently only capable of running as
+ * a websocket server and is a pretty crude implementation
+ * of it, not supporting the full websockets protocol feature
+ * set. It is sufficient to use with a simple websockets
+ * client for encapsulating VNC for noVNC in-browser client.
+ */
+
+struct QIOChannelWebsock {
+QIOChannel parent;
+QIOChannel *master;
+Buffer encinput;
+Buffer encoutput;
+Buffer rawinput;
+Buffer rawoutput;
+size_t payload_remain;
+QIOChannelWebsockMask mask;
+guint io_tag;
+Error *io_err;
+gboolean io_eof;
+};
+
+/**
+ * qio_channel_websock_new_server:
+ * @master: the underlying channel object
+ *
+ * Create a new websockets channel that runs the server
+ * side of the protocol.
+ *
+ * After creating the channel, it is mandatory to call
+ * the qio_channel_websock_handshake() method before attempting
+ * todo any I/O on the channel.
+ *
+ * Once the handshake has completed, all I/O should be done
+ * via the new websocket channel object and not the original
+ * master channel
+ *
+ * Returns: the new websockets channel object
+ */
+QIOChannelWebsock *
+qio_channel_websock_new_server(QIOChannel *master);
+
+/**
+ * qio_channel_websock_handshake:
+ * @ioc: the websocket channel object
+ * @func: the callback to invoke when completed
+ * @opaque: opaque data to pass to @func
+ * @destroy: optional callback to free @opaque
+ *
+ * Perform the websocket handshake. This method
+ * will return immediately and the handshake will
+ * continue in the background, provided the main
+ * loop is running. When the handshake is complete,
+ * or fails, the @func callback will be invoked.
+ */
+void qio_channel_websock_handshake(QIOChannelWebsock *ioc,
+   QIOTaskFunc func,
+   gpointer opaque,
+   GDestroyNotify destroy);
+
+#endif /* QIO_CHANNEL_WEBSOCK_H__ */
diff --git a/io/Makefile.objs b/io/Makefile.objs
index a48011b..e3771b1 100644
--- a/io/Makefile.objs
+++ b/io/Makefile.objs
@@ -3,4 +3,5 @@ io-obj-y += channel-file.o
 io-obj-y += channel-socket.o
 io-obj-y += channel-tls.o
 io-obj-y += channel-watch.o
+io-obj-y += channel-websock.o
 io-obj-y += task.o
diff --git a/io/channel-websock.c b/io/channel-websock.c
new file mode 100644
index 000..348d849
--- /dev/null
+++ b/io/channel-websock.c
@@ -0,0 +1,975 @@
+/*
+ * QEMU I/O channels driver websockets
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can 

[Qemu-devel] [PATCH v3 6/9] io: add QIOChannelTLS class

[Daniel P. Berrange]

Add a QIOChannel subclass that can run the TLS protocol over
the top of another QIOChannel instance. The object provides a
simplified API to perform the handshake when starting the TLS
session. The layering of TLS over the underlying channel does
not have to be setup immediately. It is possible to take an
existing QIOChannel that has done some handshake and then swap
in the QIOChannelTLS layer. This allows for use with protocols
which start TLS right away, and those which start plain text
and then negotiate TLS.

Signed-off-by: Daniel P. Berrange 
---
 include/io/channel-tls.h| 142 
 io/Makefile.objs|   1 +
 io/channel-tls.c| 405 
 tests/.gitignore|   1 +
 tests/Makefile  |   4 +
 tests/test-io-channel-tls.c | 342 +
 trace-events|  10 ++
 7 files changed, 905 insertions(+)
 create mode 100644 include/io/channel-tls.h
 create mode 100644 io/channel-tls.c
 create mode 100644 tests/test-io-channel-tls.c

diff --git a/include/io/channel-tls.h b/include/io/channel-tls.h
new file mode 100644
index 000..0298b17
--- /dev/null
+++ b/include/io/channel-tls.h
@@ -0,0 +1,142 @@
+/*
+ * QEMU I/O channels TLS driver
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_CHANNEL_TLS_H__
+#define QIO_CHANNEL_TLS_H__
+
+#include "io/channel.h"
+#include "io/task.h"
+#include "crypto/tlssession.h"
+
+#define TYPE_QIO_CHANNEL_TLS "qio-channel-tls"
+#define QIO_CHANNEL_TLS(obj) \
+OBJECT_CHECK(QIOChannelTLS, (obj), TYPE_QIO_CHANNEL_TLS)
+
+typedef struct QIOChannelTLS QIOChannelTLS;
+
+/**
+ * QIOChannelTLS
+ *
+ * The QIOChannelTLS class provides a channel wrapper which
+ * can transparently run the TLS encryption protocol. It is
+ * usually used over a TCP socket, but there is actually no
+ * technical restriction on which type of master channel is
+ * used as the transport.
+ *
+ * This channel object is capable of running as either a
+ * TLS server or TLS client.
+ */
+
+struct QIOChannelTLS {
+QIOChannel parent;
+QIOChannel *master;
+QCryptoTLSSession *session;
+};
+
+/**
+ * qio_channel_tls_new_server:
+ * @master: the underlying channel object
+ * @creds: the credentials to use for TLS handshake
+ * @aclname: the access control list for validating clients
+ * @errp: pointer to an uninitialized error object
+ *
+ * Create a new TLS channel that runs the server side of
+ * a TLS session. The TLS session handshake will use the
+ * credentials provided in @creds. If the @aclname parameter
+ * is non-NULL, then the client will have to provide
+ * credentials (ie a x509 client certificate) which will
+ * then be validated against the ACL.
+ *
+ * After creating the channel, it is mandatory to call
+ * the qio_channel_tls_handshake() method before attempting
+ * todo any I/O on the channel.
+ *
+ * Once the handshake has completed, all I/O should be done
+ * via the new TLS channel object and not the original
+ * master channel
+ *
+ * Returns: the new TLS channel object, or NULL
+ */
+QIOChannelTLS *
+qio_channel_tls_new_server(QIOChannel *master,
+   QCryptoTLSCreds *creds,
+   const char *aclname,
+   Error **errp);
+
+/**
+ * qio_channel_tls_new_client:
+ * @master: the underlying channel object
+ * @creds: the credentials to use for TLS handshake
+ * @hostname: the user specified server hostname
+ * @errp: pointer to an uninitialized error object
+ *
+ * Create a new TLS channel that runs the client side of
+ * a TLS session. The TLS session handshake will use the
+ * credentials provided in @creds. The @hostname parameter
+ * should provide the user specified hostname of the server
+ * and will be validated against the server's credentials
+ * (ie CommonName of the x509 certificate)
+ *
+ * After creating the channel, it is mandatory to call
+ * the qio_channel_tls_handshake() method before attempting
+ * todo any I/O on the channel.
+ *
+ * Once the handshake has completed, all I/O should be done
+ * via the new TLS channel object and not the original
+ * master channel
+ *
+ * Returns: the new TLS channel object, or NULL
+ */

[Qemu-devel] [PATCH v3 9/9] io: add QIOChannelBuffer class

[Daniel P. Berrange]

Add a QIOChannel subclass that is capable of performing I/O
to/from a memory buffer. This implementation does not attempt
to support concurrent readers & writers. It is designed for
serialized access where by a single thread at a time may write
data, seek and then read data back out.

Signed-off-by: Daniel P. Berrange 
---
 include/io/channel-buffer.h|  60 ++
 io/Makefile.objs   |   1 +
 io/channel-buffer.c| 261 +
 tests/.gitignore   |   1 +
 tests/Makefile |   3 +
 tests/test-io-channel-buffer.c |  50 
 6 files changed, 376 insertions(+)
 create mode 100644 include/io/channel-buffer.h
 create mode 100644 io/channel-buffer.c
 create mode 100644 tests/test-io-channel-buffer.c

diff --git a/include/io/channel-buffer.h b/include/io/channel-buffer.h
new file mode 100644
index 000..91a52b3
--- /dev/null
+++ b/include/io/channel-buffer.h
@@ -0,0 +1,60 @@
+/*
+ * QEMU I/O channels memory buffer driver
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_CHANNEL_BUFFER_H__
+#define QIO_CHANNEL_BUFFER_H__
+
+#include "io/channel.h"
+
+#define TYPE_QIO_CHANNEL_BUFFER "qio-channel-buffer"
+#define QIO_CHANNEL_BUFFER(obj) \
+OBJECT_CHECK(QIOChannelBuffer, (obj), TYPE_QIO_CHANNEL_BUFFER)
+
+typedef struct QIOChannelBuffer QIOChannelBuffer;
+
+/**
+ * QIOChannelBuffer:
+ *
+ * The QIOChannelBuffer object provides a channel implementation
+ * that is able to perform I/O to/from a memory buffer.
+ *
+ */
+
+struct QIOChannelBuffer {
+QIOChannel parent;
+size_t capacity; /* Total allocated memory */
+size_t usage;/* Current size of data */
+size_t offset;   /* Offset for future I/O ops */
+char *data;
+};
+
+
+/**
+ * qio_channel_buffer_new:
+ * @capacity: the initial buffer capacity to allocate
+ *
+ * Allocate a new buffer which is initially empty
+ *
+ * Returns: the new channel object
+ */
+QIOChannelBuffer *
+qio_channel_buffer_new(size_t capacity);
+
+#endif /* QIO_CHANNEL_BUFFER_H__ */
diff --git a/io/Makefile.objs b/io/Makefile.objs
index 1a58ccb..0e3de31 100644
--- a/io/Makefile.objs
+++ b/io/Makefile.objs
@@ -1,4 +1,5 @@
 io-obj-y = channel.o
+io-obj-y += channel-buffer.o
 io-obj-y += channel-command.o
 io-obj-y += channel-file.o
 io-obj-y += channel-socket.o
diff --git a/io/channel-buffer.c b/io/channel-buffer.c
new file mode 100644
index 000..ece1f14
--- /dev/null
+++ b/io/channel-buffer.c
@@ -0,0 +1,261 @@
+/*
+ * QEMU I/O channels memory buffer driver
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include "io/channel-buffer.h"
+#include "io/channel-watch.h"
+#include "qemu/sockets.h"
+#include "trace.h"
+
+QIOChannelBuffer *
+qio_channel_buffer_new(size_t capacity)
+{
+QIOChannelBuffer *ioc;
+
+ioc = QIO_CHANNEL_BUFFER(object_new(TYPE_QIO_CHANNEL_BUFFER));
+
+if (capacity) {
+ioc->data = g_new0(char, capacity);
+ioc->capacity = capacity;
+}
+
+return ioc;
+}
+
+
+static void qio_channel_buffer_finalize(Object *obj)
+{
+QIOChannelBuffer *ioc = QIO_CHANNEL_BUFFER(obj);
+g_free(ioc->data);
+ioc->capacity = ioc->usage = ioc->offset = 0;
+}
+
+
+static ssize_t qio_channel_buffer_readv(QIOChannel *ioc,
+const struct iovec *iov,
+size_t niov,
+int **fds,
+size_t *nfds,
+Error **errp)
+{
+QIOChannelBuffer *bioc 

[Qemu-devel] [PATCH v1 3/4] char: don't assume telnet initialization will not block

[Daniel P. Berrange]

The current code for doing telnet initialization is writing to
a socket without checking the return status. While it is highly
unlikely to be a problem when writing to a bare socket, as the
buffers are large enough to prevent blocking, this cannot be
assumed safe with TLS sockets. So write the telnet initialization
code into a memory buffer and then use an I/O watch to fully
send the data.

Signed-off-by: Daniel P. Berrange 
---
 qemu-char.c | 85 -
 1 file changed, 67 insertions(+), 18 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index dcb5fa3..f481d3b 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2820,19 +2820,68 @@ static void tcp_chr_update_read_handler(CharDriverState 
*chr)
 }
 }
 
-#define IACSET(x,a,b,c) x[0] = a; x[1] = b; x[2] = c;
-static void tcp_chr_telnet_init(QIOChannel *ioc)
+typedef struct {
+CharDriverState *chr;
+char buf[12];
+size_t buflen;
+} TCPCharDriverTelnetInit;
+
+static gboolean tcp_chr_telnet_init_io(QIOChannel *ioc,
+   GIOCondition cond G_GNUC_UNUSED,
+   gpointer user_data)
 {
-char buf[3];
-/* Send the telnet negotion to put telnet in binary, no echo, single char 
mode */
-IACSET(buf, 0xff, 0xfb, 0x01);  /* IAC WILL ECHO */
-qio_channel_write(ioc, buf, 3, NULL);
-IACSET(buf, 0xff, 0xfb, 0x03);  /* IAC WILL Suppress go ahead */
-qio_channel_write(ioc, buf, 3, NULL);
-IACSET(buf, 0xff, 0xfb, 0x00);  /* IAC WILL Binary */
-qio_channel_write(ioc, buf, 3, NULL);
-IACSET(buf, 0xff, 0xfd, 0x00);  /* IAC DO Binary */
-qio_channel_write(ioc, buf, 3, NULL);
+TCPCharDriverTelnetInit *init = user_data;
+ssize_t ret;
+
+ret = qio_channel_write(ioc, init->buf, init->buflen, NULL);
+if (ret < 0) {
+if (ret == QIO_CHANNEL_ERR_BLOCK) {
+ret = 0;
+} else {
+tcp_chr_disconnect(init->chr);
+return FALSE;
+}
+}
+init->buflen -= ret;
+
+if (init->buflen == 0) {
+tcp_chr_connect(init->chr);
+return FALSE;
+}
+
+return TRUE;
+}
+
+static void tcp_chr_telnet_init(CharDriverState *chr)
+{
+TCPCharDriver *s = chr->opaque;
+TCPCharDriverTelnetInit *init =
+g_new0(TCPCharDriverTelnetInit, 1);
+size_t n = 0;
+
+init->chr = chr;
+init->buflen = 12;
+
+#define IACSET(x, a, b, c)  \
+do {\
+x[n++] = a; \
+x[n++] = b; \
+x[n++] = c; \
+} while (0)
+
+/* Prep the telnet negotion to put telnet in binary,
+ * no echo, single char mode */
+IACSET(init->buf, 0xff, 0xfb, 0x01);  /* IAC WILL ECHO */
+IACSET(init->buf, 0xff, 0xfb, 0x03);  /* IAC WILL Suppress go ahead */
+IACSET(init->buf, 0xff, 0xfb, 0x00);  /* IAC WILL Binary */
+IACSET(init->buf, 0xff, 0xfd, 0x00);  /* IAC DO Binary */
+
+#undef IACSET
+
+qio_channel_add_watch(
+s->ioc, G_IO_OUT,
+tcp_chr_telnet_init_io,
+init, NULL);
 }
 
 static int tcp_chr_new_client(CharDriverState *chr, QIOChannelSocket *sioc)
@@ -2852,7 +2901,12 @@ static int tcp_chr_new_client(CharDriverState *chr, 
QIOChannelSocket *sioc)
 g_source_remove(s->listen_tag);
 s->listen_tag = 0;
 }
-tcp_chr_connect(chr);
+
+if (s->do_telnetopt) {
+tcp_chr_telnet_init(chr);
+} else {
+tcp_chr_connect(chr);
+}
 
 return 0;
 }
@@ -2878,7 +2932,6 @@ static gboolean tcp_chr_accept(QIOChannel *channel,
void *opaque)
 {
 CharDriverState *chr = opaque;
-TCPCharDriver *s = chr->opaque;
 QIOChannelSocket *sioc;
 
 sioc = qio_channel_socket_accept(QIO_CHANNEL_SOCKET(channel),
@@ -2887,10 +2940,6 @@ static gboolean tcp_chr_accept(QIOChannel *channel,
 return TRUE;
 }
 
-if (s->do_telnetopt) {
-tcp_chr_telnet_init(QIO_CHANNEL(sioc));
-}
-
 tcp_chr_new_client(chr, sioc);
 
 object_unref(OBJECT(sioc));
-- 
2.5.0

[Qemu-devel] [PATCH v1 1/4] char: remove fixed length filename allocation

[Daniel P. Berrange]

A variety of places were snprintf()ing into a fixed length
filename buffer. Some of the buffers were stack allocated,
while another was heap allocated with g_malloc(). Switch
them all to heap allocated using g_strdup_printf() avoiding
arbitrary length restrictions.

This also facilitates later patches which will want to
populate the filename by calling external functions
which do not support use of a pre-allocated buffer.

Signed-off-by: Daniel P. Berrange 
---
 qemu-char.c | 86 +++--
 1 file changed, 44 insertions(+), 42 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 5448b0f..59cf52f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -87,39 +87,37 @@
 
 #define READ_BUF_LEN 4096
 #define READ_RETRIES 10
-#define CHR_MAX_FILENAME_SIZE 256
 #define TCP_MAX_FDS 16
 
 /***/
 /* Socket address helpers */
 
-static int SocketAddress_to_str(char *dest, int max_len,
-const char *prefix, SocketAddress *addr,
-bool is_listen, bool is_telnet)
+static char *SocketAddress_to_str(const char *prefix, SocketAddress *addr,
+  bool is_listen, bool is_telnet)
 {
 switch (addr->type) {
 case SOCKET_ADDRESS_KIND_INET:
-return snprintf(dest, max_len, "%s%s:%s:%s%s", prefix,
-is_telnet ? "telnet" : "tcp", addr->u.inet->host,
-addr->u.inet->port, is_listen ? ",server" : "");
+return g_strdup_printf("%s%s:%s:%s%s", prefix,
+   is_telnet ? "telnet" : "tcp", 
addr->u.inet->host,
+   addr->u.inet->port, is_listen ? ",server" : "");
 break;
 case SOCKET_ADDRESS_KIND_UNIX:
-return snprintf(dest, max_len, "%sunix:%s%s", prefix,
-addr->u.q_unix->path, is_listen ? ",server" : "");
+return g_strdup_printf("%sunix:%s%s", prefix,
+   addr->u.q_unix->path,
+   is_listen ? ",server" : "");
 break;
 case SOCKET_ADDRESS_KIND_FD:
-return snprintf(dest, max_len, "%sfd:%s%s", prefix, addr->u.fd->str,
-is_listen ? ",server" : "");
+return g_strdup_printf("%sfd:%s%s", prefix, addr->u.fd->str,
+   is_listen ? ",server" : "");
 break;
 default:
 abort();
 }
 }
 
-static int sockaddr_to_str(char *dest, int max_len,
-   struct sockaddr_storage *ss, socklen_t ss_len,
-   struct sockaddr_storage *ps, socklen_t ps_len,
-   bool is_listen, bool is_telnet)
+static char *sockaddr_to_str(struct sockaddr_storage *ss, socklen_t ss_len,
+ struct sockaddr_storage *ps, socklen_t ps_len,
+ bool is_listen, bool is_telnet)
 {
 char shost[NI_MAXHOST], sserv[NI_MAXSERV];
 char phost[NI_MAXHOST], pserv[NI_MAXSERV];
@@ -128,9 +126,9 @@ static int sockaddr_to_str(char *dest, int max_len,
 switch (ss->ss_family) {
 #ifndef _WIN32
 case AF_UNIX:
-return snprintf(dest, max_len, "unix:%s%s",
-((struct sockaddr_un *)(ss))->sun_path,
-is_listen ? ",server" : "");
+return g_strdup_printf("unix:%s%s",
+   ((struct sockaddr_un *)(ss))->sun_path,
+   is_listen ? ",server" : "");
 #endif
 case AF_INET6:
 left  = "[";
@@ -141,14 +139,14 @@ static int sockaddr_to_str(char *dest, int max_len,
 sserv, sizeof(sserv), NI_NUMERICHOST | NI_NUMERICSERV);
 getnameinfo((struct sockaddr *) ps, ps_len, phost, sizeof(phost),
 pserv, sizeof(pserv), NI_NUMERICHOST | NI_NUMERICSERV);
-return snprintf(dest, max_len, "%s:%s%s%s:%s%s <-> %s%s%s:%s",
-is_telnet ? "telnet" : "tcp",
-left, shost, right, sserv,
-is_listen ? ",server" : "",
-left, phost, right, pserv);
+return g_strdup_printf("%s:%s%s%s:%s%s <-> %s%s%s:%s",
+   is_telnet ? "telnet" : "tcp",
+   left, shost, right, sserv,
+   is_listen ? ",server" : "",
+   left, phost, right, pserv);
 
 default:
-return snprintf(dest, max_len, "unknown");
+return g_strdup_printf("unknown");
 }
 }
 
@@ -1072,14 +1070,17 @@ static CharDriverState *qemu_chr_open_pipe(const char 
*id,
 {
 ChardevHostdev *opts = backend->u.pipe;
 int fd_in, fd_out;
-char filename_in[CHR_MAX_FILENAME_SIZE];
-char filename_out[CHR_MAX_FILENAME_SIZE];
+char *filename_in;
+char *filename_out;
 const char *filename = opts->device;

[Qemu-devel] [Bug 1516203] Re: qemu-system-x86_64 crashed with SIGSEGV in SDL_BlitCopy()

[Chris J Arges]

Can you please test with the 4.2.0-19.23 kernel on Xenial as well?
Thanks

** Changed in: qemu (Ubuntu)
   Status: New => Incomplete

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

Title:
  qemu-system-x86_64 crashed with SIGSEGV in SDL_BlitCopy()

Status in QEMU:
  New
Status in qemu package in Ubuntu:
  Incomplete

Bug description:
  with -device virtio-vga -cdrom ubuntu-15.10-desktop-amd64.iso

  ProblemType: Crash
  DistroRelease: Ubuntu 16.04
  Package: qemu-system-x86 1:2.4+dfsg-4ubuntu2
  ProcVersionSignature: Ubuntu 4.3.0-0.6-generic 4.3.0
  Uname: Linux 4.3.0-0-generic x86_64
  ApportVersion: 2.19.2-0ubuntu6
  Architecture: amd64
  CurrentDesktop: Unity
  Date: Sat Nov 14 05:05:25 2015
  EcryptfsInUse: Yes
  ExecutablePath: /usr/bin/qemu-system-x86_64
  InstallationDate: Installed on 2012-12-22 (1056 days ago)
  InstallationMedia: Ubuntu 12.04.1 LTS "Precise Pangolin" - Release amd64 
(20120823.1)
  KvmCmdLine:
   COMMAND STAT  EUID  RUID   PID  PPID %CPU COMMAND
   kvm-irqfd-clean S<   0 0   497 2  0.0 [kvm-irqfd-clean]
  MachineType: Hewlett-Packard HP Elite 7300 Series MT
  ProcCmdline: qemu-system-x86_64 -machine ubuntu,accel=kvm -m 1024 -device 
virtio-vga -cdrom ubuntu-15.10-desktop-amd64.iso
  ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinuz-4.3.0-0-generic 
root=UUID=8905185c-9d82-498c-970c-6fdb9ee07c45 ro quiet splash zswap.enabled=1 
crashkernel=384M-:128M vt.handoff=7
  SegvAnalysis:
   Segfault happened at: 0x7fecce94624f:movq   (%rax),%mm0
   PC (0x7fecce94624f) ok
   source "(%rax)" (0x7fecb5f0e010) not located in a known VMA region (needed 
readable region)!
   destination "%mm0" ok
  SegvReason: reading unknown VMA
  Signal: 11
  SourcePackage: qemu
  StacktraceTop:
   ?? () from /usr/lib/x86_64-linux-gnu/libSDL-1.2.so.0
   ?? () from /usr/lib/x86_64-linux-gnu/libSDL-1.2.so.0
   SDL_LowerBlit () from /usr/lib/x86_64-linux-gnu/libSDL-1.2.so.0
   SDL_UpperBlit () from /usr/lib/x86_64-linux-gnu/libSDL-1.2.so.0
   ?? ()
  Title: qemu-system-x86_64 crashed with SIGSEGV in SDL_LowerBlit()
  UpgradeStatus: Upgraded to xenial on 2013-06-19 (877 days ago)
  UserGroups: adm cdrom dip gnunet kismet kvm libvirtd lpadmin plugdev 
sambashare sbuild sudo
  dmi.bios.date: 05/18/2011
  dmi.bios.vendor: AMI
  dmi.bios.version: 7.05
  dmi.board.name: 2AB5
  dmi.board.vendor: PEGATRON CORPORATION
  dmi.board.version: 1.01
  dmi.chassis.asset.tag: CZC126149V
  dmi.chassis.type: 3
  dmi.chassis.vendor: Hewlett-Packard
  dmi.modalias: 
dmi:bvnAMI:bvr7.05:bd05/18/2011:svnHewlett-Packard:pnHPElite7300SeriesMT:pvr1.01:rvnPEGATRONCORPORATION:rn2AB5:rvr1.01:cvnHewlett-Packard:ct3:cvr:
  dmi.product.name: HP Elite 7300 Series MT
  dmi.product.version: 1.01
  dmi.sys.vendor: Hewlett-Packard

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

Re: [Qemu-devel] [PATCH v12 22/36] qapi: Don't let implicit enum MAX member collide

[Markus Armbruster]

Eric Blake  writes:

> On 11/18/2015 06:12 AM, Markus Armbruster wrote:
>> Eric Blake  writes:
>> 
>>> Now that we guarantee the user doesn't have any enum values
>>> beginning with a single underscore, we can use that for our
>>> own purposes.  Renaming ENUM_MAX to ENUM__MAX makes it obvious
>>> that the sentinel is generated.
>> 
>> The ENUM__MAX are mildly ugly.  If we cared, we could get rid of most or
>> all uses.  Right now, I don't.
>> 
>> Hmm, perhaps these ENUM__MAX are your motivation for rejecting adjacent
>> [-_] in the previous patch, to catch clashes like (foolishly named) type
>> 'FOO--MAX' with enumeration type 'Foo'.  I acknowledge the clash.
>
> Yes, and that was part of my reasoning on 21/36 - but you've convinced
> me to relax that one, so it no longer applies as an argument here.
>
>> However, there are many more clashes we don't attempt to catch,
>> e.g. type 'Foo-lookup' with enumeration type 'Foo'.  Perhaps we want to
>> build a real fence later, but right now I don't want us to ram in more
>> scattered fence laths.
>> 
>> Might want to say something like "Rejecting enum members named 'MAX' is
>> now useless, and will be dropped in the next patch".
>
> Yes, that helps the commit message. Can you handle it, or do I need to
> send a fixup?

Can do!

>>> This patch was mostly generated by applying a temporary patch:
>>>
>>> |diff --git a/scripts/qapi.py b/scripts/qapi.py
>>> |index e6d014b..b862ec9 100644
>>> |--- a/scripts/qapi.py
>>> |+++ b/scripts/qapi.py
>>> |@@ -1570,6 +1570,7 @@ const char *const %(c_name)s_lookup[] = {
>>> | max_index = c_enum_const(name, 'MAX', prefix)
>>> | ret += mcgen('''
>>> | [%(max_index)s] = NULL,
>>> |+// %(max_index)s
>>> | };
>>> | ''',
>>> |max_index=max_index)
>>>
>>> then running:
>>>
>>> $ cat qapi-{types,event}.c tests/test-qapi-types.c |
>>> sed -n 's,^// \(.*\)MAX,s|\1MAX|\1_MAX|g,p' > list
>>> $ git grep -l _MAX | xargs sed -i -f list
>>>
>>> The only things not generated are the changes in scripts/qapi.py.
>>>
>>> Signed-off-by: Eric Blake 
>> 
>> Patch looks good.
>
> The fun part was coming up with the automation, and then making sure it
> was reproducible. :)

May not always be faster than manual, but it sure is a lot more fun!
More reliable, too, in my experience.

[Qemu-devel] libcacard: forward declaration of VReader

[Alex Roithman]

hi

my project not build complete, because i have:

/usr/bin/c++   -DQT_CORE_LIB -DQT_GUI_LIB -DQT_NO_DEBUG -DQT_SVG_LIB
-DQT_XML_LIB -I/home/Flash/qt-virt-manager-build
-I/home/Flash/qt-virt-manager/src -isystem /usr/include/QtSvg -isystem
/usr/include/QtGui -isystem /usr/include/QtXml -isystem /usr/include/QtCore
-I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include
-I/usr/include/glib-2.0/gobject -I/usr/include/glib-2.0/gio
-I/usr/include/cacard -I/usr/include/spice-1
-I/usr/include/spice-client-glib-2.0 -I/usr/include/qtermwidget4-o
CMakeFiles/qt4-virt-manager.dir/src/vm_viewer/qspice_widgets/qspicesmartcardmanager.cpp.o
-c
/home/Flash/qt-virt-manager/src/vm_viewer/qspice_widgets/qspicesmartcardmanager.cpp
/home/Flash/qt-virt-manager/src/vm_viewer/qspice_widgets/qspicesmartcardmanager.cpp:
В функции-члене «QStringList
QSpiceSmartcardManager::spiceSmartcardManager_get_readers()»:
/home/Flash/qt-virt-manager/src/vm_viewer/qspice_widgets/qspicesmartcardmanager.cpp:89:35:
error: invalid use of incomplete type «VReader {aka struct VReaderStruct}»
 _readerList.append(_reader->name);
   ^
In file included from /usr/include/cacard/eventt.h:8:0,
 from /usr/include/cacard/vreader.h:9,
 from
/home/Flash/qt-virt-manager/src/vm_viewer/qspice_widgets/qspicesmartcardmanager.cpp:2:
/usr/include/cacard/vreadert.h:16:16: warning: forward declaration of
«VReader {aka struct VReaderStruct}»
 typedef struct VReaderStruct VReader;


in system installed
libcacard.x86_64 2:2.3.1-7.fc22
libcacard-devel.x86_64 2:2.3.1-7.fc22

in code i'm use only
#include 
from libcacard

part of code:

VReader *_reader =
static_cast(g_list_nth_data(_list, i));
 _readerList.append(_reader->name);


I accept any advice.

Re: [Qemu-devel] CPU Cache simulation

[Pranith Kumar]

Hi,

On Wed, Nov 18, 2015 at 11:48 AM, Hao Bai  wrote:

> Sorry about the ambiguity.
> I am using x86-64 architecture in user mode. Basically, I am trying to log
> all the cache activities when I run a guest program with QEMU. That's why I
> asked whether QEMU simulated CPU caches. I was assuming if QEMU did not
> simulate CPU caches then there would be no way to do this (Correct me if I
> am wrong). Is there a way to do this?
>
>


You can try qsim: https://github.com/gtcasl/qsim

In particular look at the cache simulator:
https://github.com/gtcasl/qsim/blob/master/examples/x86/cachesim.cpp

Setup instructions: https://github.com/pranith/qsim-setup



-- 
Pranith

[Qemu-devel] [PATCH v3 2/9] io: add helper module for creating watches on FDs

[Daniel P. Berrange]

A number of the channel implementations will require the
ability to create watches on file descriptors / sockets.
To avoid duplicating this code in each channel, provide a
helper API for dealing with file descriptor watches.

There are two watch implementations provided. The first
is useful for bi-directional file descriptors such as
sockets, regular files, character devices, etc. The
second works with a pair of unidirectional file descriptors
such as pipes.

Signed-off-by: Daniel P. Berrange 
---
 include/io/channel-watch.h |  72 
 io/Makefile.objs   |   1 +
 io/channel-watch.c | 200 +
 3 files changed, 273 insertions(+)
 create mode 100644 include/io/channel-watch.h
 create mode 100644 io/channel-watch.c

diff --git a/include/io/channel-watch.h b/include/io/channel-watch.h
new file mode 100644
index 000..656358a
--- /dev/null
+++ b/include/io/channel-watch.h
@@ -0,0 +1,72 @@
+/*
+ * QEMU I/O channels watch helper APIs
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_CHANNEL_WATCH_H__
+#define QIO_CHANNEL_WATCH_H__
+
+#include "io/channel.h"
+
+/*
+ * This module provides helper functions that will be needed by
+ * the various QIOChannel implementations, for creating watches
+ * on file descriptors / sockets
+ */
+
+/**
+ * qio_channel_create_fd_watch:
+ * @ioc: the channel object
+ * @fd: the file descriptor
+ * @condition: the I/O condition
+ *
+ * Create a new main loop source that is able to
+ * monitor the file descriptor @fd for the
+ * I/O conditions in @condition. This is able
+ * monitor block devices, character devices,
+ * sockets, pipes but not plain files.
+ *
+ * Returns: the new main loop source
+ */
+GSource *qio_channel_create_fd_watch(QIOChannel *ioc,
+ int fd,
+ GIOCondition condition);
+
+/**
+ * qio_channel_create_fd_pair_watch:
+ * @ioc: the channel object
+ * @fdread: the file descriptor for reading
+ * @fdwrite: the file descriptor for writing
+ * @condition: the I/O condition
+ *
+ * Create a new main loop source that is able to
+ * monitor the pair of file descriptors @fdread
+ * and @fdwrite for the I/O conditions in @condition.
+ * This is intended for monitoring unidirectional
+ * file descriptors such as pipes, where a pair
+ * of descriptors is required for bidirectional
+ * I/O
+ *
+ * Returns: the new main loop source
+ */
+GSource *qio_channel_create_fd_pair_watch(QIOChannel *ioc,
+  int fdread,
+  int fdwrite,
+  GIOCondition condition);
+
+#endif /* QIO_CHANNEL_WATCH_H__ */
diff --git a/io/Makefile.objs b/io/Makefile.objs
index a6ed361..b02ea90 100644
--- a/io/Makefile.objs
+++ b/io/Makefile.objs
@@ -1 +1,2 @@
 io-obj-y = channel.o
+io-obj-y += channel-watch.o
diff --git a/io/channel-watch.c b/io/channel-watch.c
new file mode 100644
index 000..9564605
--- /dev/null
+++ b/io/channel-watch.c
@@ -0,0 +1,200 @@
+/*
+ * QEMU I/O channels watch helper APIs
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#include "io/channel-watch.h"
+
+typedef struct QIOChannelFDSource QIOChannelFDSource;
+struct QIOChannelFDSource {
+GSource parent;
+GPollFD fd;
+QIOChannel *ioc;
+GIOCondition condition;
+};
+
+
+typedef struct QIOChannelFDPairSource QIOChannelFDPairSource;
+struct QIOChannelFDPairSource {
+GSource parent;
+GPollFD fdread;
+GPollFD fdwrite;
+QIOChannel *ioc;
+GIOCondition condition;
+};
+

[Qemu-devel] [PATCH v3 0/9] Introduce I/O channels framework

[Daniel P. Berrange]

This series of patches is a followup submission for review of another
chunk of my large series supporting TLS across chardevs, VNC and
migration, previously shown here:

 RFC: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00829.html
  v1: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg04853.html
  v2: https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg03439.html

In this series, I have provided only the general I/O channels
framework, which will ultimately be used by VNC, chardev and
migration code, which currently work as follows:

 - VNC: uses raw sockets APIs directly, layering in TLS, SASL
   and websockets where needed. This has resulted in what should
   be fairly generic code for TLS and websockets being entangled
   with the rest of the VNC server code.

 - Chardevs: uses GLib's GIOChannel framework. This only provides
   implementations for sockets and files. Its API lacks support for
   using struct iovec for read/writes, file descriptor passing,
   misc sockets ioctls/fcntls. While you can work around these
   problems by accessing the underling file descriptor directly,
   this breaks the encapsulation, requiring callers to know about
   specific implementations. It also does not have integration
   with QEMU Error reporting framework. So while the GIOChannel
   is extensible, extending it would result in throwing away
   pretty much the entire of the existing implementations

 - Migration: uses QEMUFile framework. The provides an abstract
   interface for I/O channels used during migration. It has
   impls for sockets, files, memory buffers & commands. While
   it has struct iovec support for writes, it does not have
   the same for reads. It also doesn't support file descriptor
   passing or control of the sockets ioctls/fcntls. It also
   does not have any explicit event loop integration, expecting
   the callers to directly access the underling file desctriptor
   and setup events on that. This limits its utility in cases
   where you need channels which are not backed by file
   descriptors. It has no integration with QEMU Error object
   for error reporting, has fixed semantics wrt writes
   ie must always do a full write, no partial writes, and
   most strangely forces either input or output mode, but
   refuses to allow both, so no bi-directional channels!

Out of all of these, the migration code is probably closest
to what is needed, but is still a good way off from being a
generic framework that be can reused outside of the migration
code.

There is also the GLib GIO library which provides a generic
framework, but we can't depend on that due to the minimum
GLib requirement. It also has various missing pieces such as
file descriptor passing, and no support for struct iovec
either.

Hence, this series was born, which tries to take the best of
the designs for the GIOChannel, QIO and QEMUFile code to
provide QIOChannel. Right from the start this new framework
is explicitly isolated from any other QEMU subsystem, so its
implementation will not get mixed up with specific use cases.

The QIOChannel abstract base class defines the overall API
contract

 - qio_channel_{write,writev,write_full} for writing data. The
   underling impl always uses struct iovec, but non-iov
   APIs are provided as simple wrappers for convenience

 - qio_channel_{read,readv,read_full} for reading data. The
   underling impl always uses struct iovec, but non-iov
   APIs are provided as simple wrappers for convenience

 - qio_channel_has_feature to determine which optional
   features the channel supports - eg file descriptor
   passing, nagle, etc

 - qio_channel_set_{blocking,delay,cork} for various fcntl
   and ioctl controls

 - qio_channel_{close,shutdown} for closing the I/O stream
   or individual channels

 - qio_channel_seek for random access channels

 - qio_channel_{add,create}_watch for integration into the
   main loop for event notifications

 - qio_channel_wait for blocking of execution pending an
   event notification

 - qio_channel_yield for switching coroutine until an
   event notification

All the APIs support Error ** object arguments where needed.
The object is built using QOM, in order to get reference
counting and sub-classing with type checking. They are *not*
user creatable/visible objects though - these are internal
infrastructure, so we will be free to adapt APIs/objects at
will over time.

In this series there are a variety of implementations. Some
of them provide an actual base layer data transport, while
others provide a data translation layer:

In this series there are a variety of implementations. Some
of them provide an actual base layer data transport, while
others provide a data translation layer:

 - QIOChannelSocket - for socket based I/O, IPv4, IPV6 & UNIX,
  both stream and datagram.
 - QIOChannelFile - any non-socket file, plain file, char dev,
fifo, pipe, etc
 - QIOChannelTLS - data translation layer to 

[Qemu-devel] [PATCH v3 5/9] io: add QIOChannelFile class

[Daniel P. Berrange]

Add a QIOChannel subclass that is capable of operating on things
that are files, such as plain files, pipes, character/block
devices, but notably not sockets.

Signed-off-by: Daniel P. Berrange 
---
 include/io/channel-file.h|  93 +
 io/Makefile.objs |   1 +
 io/channel-file.c| 237 +++
 tests/.gitignore |   2 +
 tests/Makefile   |   3 +
 tests/test-io-channel-file.c | 100 ++
 trace-events |   4 +
 7 files changed, 440 insertions(+)
 create mode 100644 include/io/channel-file.h
 create mode 100644 io/channel-file.c
 create mode 100644 tests/test-io-channel-file.c

diff --git a/include/io/channel-file.h b/include/io/channel-file.h
new file mode 100644
index 000..308e6d4
--- /dev/null
+++ b/include/io/channel-file.h
@@ -0,0 +1,93 @@
+/*
+ * QEMU I/O channels files driver
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_CHANNEL_FILE_H__
+#define QIO_CHANNEL_FILE_H__
+
+#include "io/channel.h"
+
+#define TYPE_QIO_CHANNEL_FILE "qio-channel-file"
+#define QIO_CHANNEL_FILE(obj) \
+OBJECT_CHECK(QIOChannelFile, (obj), TYPE_QIO_CHANNEL_FILE)
+
+typedef struct QIOChannelFile QIOChannelFile;
+
+/**
+ * QIOChannelFile:
+ *
+ * The QIOChannelFile object provides a channel implementation
+ * that is able to perform I/O on block devices, character
+ * devices, FIFOs, pipes and plain files. While it is technically
+ * able to work on sockets too on the UNIX platform, this is not
+ * portable to Windows and lacks some extra sockets specific
+ * functionality. So the QIOChannelSocket object is recommended
+ * for that use case.
+ *
+ */
+
+struct QIOChannelFile {
+QIOChannel parent;
+int fd;
+};
+
+
+/**
+ * qio_channel_file_new_fd:
+ * @fd: the file descriptor
+ *
+ * Create a new IO channel object for a file represented
+ * by the @fd parameter. @fd can be associated with a
+ * block device, character device, fifo, pipe, or a
+ * regular file. For sockets, the QIOChannelSocket class
+ * should be used instead, as this provides greater
+ * functionality and cross platform portability.
+ *
+ * The channel will own the passed in file descriptor
+ * and will take responsibility for closing it, so the
+ * caller must not close it. If appropriate the caller
+ * should dup() its FD before opening the channel.
+ *
+ * Returns: the new channel object
+ */
+QIOChannelFile *
+qio_channel_file_new_fd(int fd);
+
+/**
+ * qio_channel_file_new_path:
+ * @fd: the file descriptor
+ * @flags: the open flags (O_RDONLY|O_WRONLY|O_RDWR, etc)
+ * @mode: the file creation mode if O_WRONLY is set in @flags
+ * @errp: pointer to initialized error object
+ *
+ * Create a new IO channel object for a file represented
+ * by the @path parameter. @path can point to any
+ * type of file on which sequential I/O can be
+ * performed, whether it be a plain file, character
+ * device or block device.
+ *
+ * Returns: the new channel object
+ */
+QIOChannelFile *
+qio_channel_file_new_path(const char *path,
+  int flags,
+  mode_t mode,
+  Error **errp);
+
+#endif /* QIO_CHANNEL_FILE_H__ */
diff --git a/io/Makefile.objs b/io/Makefile.objs
index e9d77aa..3d2f232 100644
--- a/io/Makefile.objs
+++ b/io/Makefile.objs
@@ -1,4 +1,5 @@
 io-obj-y = channel.o
+io-obj-y += channel-file.o
 io-obj-y += channel-socket.o
 io-obj-y += channel-watch.o
 io-obj-y += task.o
diff --git a/io/channel-file.c b/io/channel-file.c
new file mode 100644
index 000..3e67b22
--- /dev/null
+++ b/io/channel-file.c
@@ -0,0 +1,237 @@
+/*
+ * QEMU I/O channels files driver
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more 

[Qemu-devel] [PATCH v3 8/9] io: add QIOChannelCommand class

[Daniel P. Berrange]

Add a QIOChannel subclass that is capable of performing I/O
to/from a separate process, via a pair of pipes. The command
can be used for unidirectional or bi-directional I/O.

Signed-off-by: Daniel P. Berrange 
---
 include/io/channel-command.h|  91 ++
 io/Makefile.objs|   1 +
 io/channel-command.c| 369 
 tests/.gitignore|   2 +
 tests/Makefile  |   3 +
 tests/test-io-channel-command.c | 129 ++
 trace-events|   6 +
 7 files changed, 601 insertions(+)
 create mode 100644 include/io/channel-command.h
 create mode 100644 io/channel-command.c
 create mode 100644 tests/test-io-channel-command.c

diff --git a/include/io/channel-command.h b/include/io/channel-command.h
new file mode 100644
index 000..bd3c599
--- /dev/null
+++ b/include/io/channel-command.h
@@ -0,0 +1,91 @@
+/*
+ * QEMU I/O channels external command driver
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_CHANNEL_COMMAND_H__
+#define QIO_CHANNEL_COMMAND_H__
+
+#include "io/channel.h"
+
+#define TYPE_QIO_CHANNEL_COMMAND "qio-channel-command"
+#define QIO_CHANNEL_COMMAND(obj) \
+OBJECT_CHECK(QIOChannelCommand, (obj), TYPE_QIO_CHANNEL_COMMAND)
+
+typedef struct QIOChannelCommand QIOChannelCommand;
+
+
+/**
+ * QIOChannelCommand:
+ *
+ * The QIOChannelCommand class provides a channel implementation
+ * that can transport data with an externally running command
+ * via its stdio streams.
+ */
+
+struct QIOChannelCommand {
+QIOChannel parent;
+int writefd;
+int readfd;
+pid_t pid;
+};
+
+
+/**
+ * qio_channel_command_new_pid:
+ * @writefd: the FD connected to the command's stdin
+ * @readfd: the FD connected to the command's stdout
+ * @pid: the PID of the running child command
+ * @errp: pointer to an uninitialized error object
+ *
+ * Create a channel for performing I/O with the
+ * previously spawned command identified by @pid.
+ * The two file descriptors provide the connection
+ * to command's stdio streams, either one or which
+ * may be -1 to indicate that stream is not open.
+ *
+ * The channel will take ownership of the process
+ * @pid and will kill it when closing the channel.
+ * Similarly it will take responsibility for
+ * closing the file descriptors @writefd and @readfd.
+ *
+ * Returns: the command channel object, or NULL on error
+ */
+QIOChannelCommand *
+qio_channel_command_new_pid(int writefd,
+int readfd,
+pid_t pid);
+
+/**
+ * qio_channel_command_new_spawn:
+ * @argv: the NULL terminated list of command arguments
+ * @flags: the I/O mode, one of O_RDONLY, O_WRONLY, O_RDWR
+ * @errp: pointer to an uninitialized error object
+ *
+ * Create a channel for performing I/O with the
+ * command to be spawned with arguments @argv.
+ *
+ * Returns: the command channel object, or NULL on error
+ */
+QIOChannelCommand *
+qio_channel_command_new_spawn(const char *const argv[],
+  int flags,
+  Error **errp);
+
+
+#endif /* QIO_CHANNEL_COMMAND_H__ */
diff --git a/io/Makefile.objs b/io/Makefile.objs
index e3771b1..1a58ccb 100644
--- a/io/Makefile.objs
+++ b/io/Makefile.objs
@@ -1,4 +1,5 @@
 io-obj-y = channel.o
+io-obj-y += channel-command.o
 io-obj-y += channel-file.o
 io-obj-y += channel-socket.o
 io-obj-y += channel-tls.o
diff --git a/io/channel-command.c b/io/channel-command.c
new file mode 100644
index 000..d735efb
--- /dev/null
+++ b/io/channel-command.c
@@ -0,0 +1,369 @@
+/*
+ * QEMU I/O channels external command driver
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy 

[Qemu-devel] [PATCH v3 3/9] io: add QIOTask class for async operations

[Daniel P. Berrange]

A number of I/O operations need to be performed asynchronously
to avoid blocking the main loop. The caller of such APIs need
to provide a callback to be invoked on completion/error and
need access to the error, if any. The small QIOTask provides
a simple framework for dealing with such probes. The API
docs inline provide an outline of how this is to be used.

Some functions don't have the ability to run asynchronously
(eg getaddrinfo always blocks), so to facilitate their use,
the task class provides a mechanism to run a blocking
function in a thread, while triggering the completion
callback in the main event loop thread. This easily allows
any synchronous function to be made asynchronous, albeit
at the cost of spawning a thread.

In this series, the QIOTask class will be used for things like
the TLS handshake, the websockets handshake and TCP connect()
progress.

The concept of QIOTask is inspired by the GAsyncResult
interface / GTask class in the GIO libraries. The min
version requirements on glib don't allow those to be
used from QEMU, so QIOTask provides a facsimilie which
can be easily switched to GTask in the future if the
min version is increased.

Signed-off-by: Daniel P. Berrange 
---
 include/io/task.h| 256 +++
 io/Makefile.objs |   1 +
 io/task.c| 159 +
 tests/.gitignore |   1 +
 tests/Makefile   |   3 +
 tests/test-io-task.c | 276 +++
 trace-events |   9 ++
 7 files changed, 705 insertions(+)
 create mode 100644 include/io/task.h
 create mode 100644 io/task.c
 create mode 100644 tests/test-io-task.c

diff --git a/include/io/task.h b/include/io/task.h
new file mode 100644
index 000..2418714
--- /dev/null
+++ b/include/io/task.h
@@ -0,0 +1,256 @@
+/*
+ * QEMU I/O task
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_TASK_H__
+#define QIO_TASK_H__
+
+#include "qemu-common.h"
+#include "qapi/error.h"
+#include "qom/object.h"
+
+typedef struct QIOTask QIOTask;
+
+typedef void (*QIOTaskFunc)(Object *source,
+Error *err,
+gpointer opaque);
+
+typedef int (*QIOTaskWorker)(QIOTask *task,
+ Error **errp,
+ gpointer opaque);
+
+/**
+ * QIOTask:
+ *
+ * The QIOTask object provides a simple mechanism for reporting
+ * success / failure of long running background operations.
+ *
+ * A object on which the operation is to be performed could have
+ * a public API which accepts a task callback:
+ *
+ * 
+ *   Task callback function signature
+ *   
+ *  void myobject_operation(QMyObject *obj,
+ *  QIOTaskFunc *func,
+ *  gpointer opaque,
+ *  GDestroyNotify *notify);
+ *   
+ * 
+ *
+ * The 'func' parameter is the callback to be invoked, and 'opaque'
+ * is data to pass to it. The optional 'notify' function is used
+ * to free 'opaque' when no longer needed.
+ *
+ * Now, lets say the implementation of this method wants to set
+ * a timer to run once a second checking for completion of some
+ * activity. It would do something like
+ *
+ * 
+ *   Task callback function implementation
+ *   
+ *void myobject_operation(QMyObject *obj,
+ *QIOTaskFunc *func,
+ *gpointer opaque,
+ *GDestroyNotify *notify)
+ *{
+ *  QIOTask *task;
+ *
+ *  task = qio_task_new(OBJECT(obj), func, opaque, notify);
+ *
+ *  g_timeout_add_full(G_PRIORITY_DEFAULT,
+ * 1000,
+ * myobject_operation_timer,
+ * task,
+ * NULL);
+ *}
+ *   
+ * 
+ *
+ * It could equally have setup a watch on a file descriptor or
+ * created a background thread, or something else entirely.
+ * Notice that the source object is passed to the task, and
+ * QIOTask will hold a reference on that. This ensure that
+ * the QMyObject instance cannot be garbage collected while
+ * the async task is still in progress.
+ *
+ * In this case, myobject_operation_timer will fire after
+ * 3 secs and do
+ *
+ * 
+ *   Task timer function

Re: [Qemu-devel] [PATCH qom-next] MAINTAINERS: Add check-qom-proplist to QOM

[Andreas Färber]

Am 18.11.2015 um 19:13 schrieb Andreas Färber:
> Add the QOM unit test to the QOM maintenance area so that maintainers
> get CC'ed on changes and to document QOM test coverage.
> 
> Cc: Daniel P. Berrange 
> Signed-off-by: Andreas Färber 
> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9e1fa72..18250ae 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1164,6 +1164,7 @@ F: include/qom/
>  X: include/qom/cpu.h
>  F: qom/
>  X: qom/cpu.c

Will also add:

F: tests/check-qom-interface.c

> +F: tests/check-qom-proplist.c
>  F: tests/qom-test.c
>  
>  QMP

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

[Qemu-devel] [PATCH 2/2] nvme: bump PCI revision

[Christoph Hellwig]

The broken Identify implementation in earlier Qemu versions means we
need to blacklist it from issueing the NVMe 1.1 Identify Namespace List
command.  As we want to be able to use it in newer Qemu versions we need
a way to identify those.  Bump the PCI revision as a guest visible
indicator of this bug fix.

Signed-off-by: Christoph Hellwig 
---
 hw/block/nvme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 4a6443f..360be71 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -946,7 +946,7 @@ static void nvme_class_init(ObjectClass *oc, void *data)
 pc->class_id = PCI_CLASS_STORAGE_EXPRESS;
 pc->vendor_id = PCI_VENDOR_ID_INTEL;
 pc->device_id = 0x5845;
-pc->revision = 1;
+pc->revision = 2;
 pc->is_express = 1;
 
 set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
-- 
1.9.1

[Qemu-devel] [PATCH 0/2] import nvme fix

[Christoph Hellwig]

First one fixes Identify to behave as mandated by the spec, and the
second bumps the PCI revision so that guest drivers can detect
the fixed version of the device so that only the old version has
to be blacklisted.

Re: [Qemu-devel] libcacard: forward declaration of VReader

[Alex Roithman]

hi

>
> my project not build complete, because i have:
> 
> /usr/bin/c++   -DQT_CORE_LIB -DQT_GUI_LIB -DQT_NO_DEBUG -DQT_SVG_LIB
> -DQT_XML_LIB -I/home/Flash/qt-virt-manager-build
> -I/home/Flash/qt-virt-manager/src -isystem /usr/include/QtSvg -isystem
> /usr/include/QtGui -isystem /usr/include/QtXml -isystem /usr/include/QtCore
> -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include
> -I/usr/include/glib-2.0/gobject -I/usr/include/glib-2.0/gio
> -I/usr/include/cacard -I/usr/include/spice-1
> -I/usr/include/spice-client-glib-2.0 -I/usr/include/qtermwidget4-o
> CMakeFiles/qt4-virt-manager.dir/src/vm_viewer/qspice_widgets/qspicesmartcardmanager.cpp.o
> -c
> /home/Flash/qt-virt-manager/src/vm_viewer/qspice_widgets/qspicesmartcardmanager.cpp
> /home/Flash/qt-virt-manager/src/vm_viewer/qspice_widgets/qspicesmartcardmanager.cpp:
> В функции-члене «QStringList
> QSpiceSmartcardManager::spiceSmartcardManager_get_readers()»:
> /home/Flash/qt-virt-manager/src/vm_viewer/qspice_widgets/qspicesmartcardmanager.cpp:89:35:
> error: invalid use of incomplete type «VReader {aka struct VReaderStruct}»
>  _readerList.append(_reader->name);
>^
> In file included from /usr/include/cacard/eventt.h:8:0,
>  from /usr/include/cacard/vreader.h:9,
>  from
> /home/Flash/qt-virt-manager/src/vm_viewer/qspice_widgets/qspicesmartcardmanager.cpp:2:
> /usr/include/cacard/vreadert.h:16:16: warning: forward declaration of
> «VReader {aka struct VReaderStruct}»
>  typedef struct VReaderStruct VReader;
> 
>


in system installed
> libcacard.x86_64 2:2.3.1-7.fc22
> libcacard-devel.x86_64 2:2.3.1-7.fc22
>
> in code i'm use only
> #include 
> from libcacard
>
> part of code:
> 
> VReader *_reader =
> static_cast(g_list_nth_data(_list, i));
>  _readerList.append(_reader->name);
> 
>
> I accept any advice.
>

Re: [Qemu-devel] [PATCH v3 0/3] target-i386: add memory protection-key support

[Eduardo Habkost]

On Wed, Nov 18, 2015 at 10:20:14AM +0800, Huaitong Han wrote:
> Changes in v3:
> *Fix cpuid_7_0_ecx_feature_name error.
> 
> Changes in v2:
> *Fix memcpy error for xsave state.
> *Fix TCG_7_0_ECX_FEATURES to 0.
> *Make subjects more readable.
> 
> The protection-key feature provides an additional mechanism by which IA-32e
> paging controls access to usermode addresses.
> 
> Hardware support for protection keys for user pages is enumerated with CPUID
> feature flag CPUID.7.0.ECX[3]:PKU. Software support is CPUID.7.0.ECX[4]:OSPKE
> with the setting of CR4.PKE(bit 22).
> 
> The PKRU register is XSAVE-managed state CPUID.D.0.EAX[9], the size of XSAVE
> state component for PKRU is 8 bytes, the offset is 0xa80.

Is every CPU supporting PKU guaranteed to have
CPUID.(EAX=0DH,ECX=9):EBX = 0xa80? Where is the PKRU state
offset/layout documented?

> 
> The specification of Protection Keys can be found at SDM (4.6.2, volume 3)
> http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-manual-325462.pdf.
> 
[...]

-- 
Eduardo

Re: [Qemu-devel] [Qemu-arm] [PATCH] target-arm: Priority masking with basepri on v7m

[Peter Crosthwaite]

On Tue, Nov 17, 2015 at 1:52 PM, Francois Baldassari
 wrote:
> Thanks for the swift reply.
>
> Michael's changes look very good indeed. Thank you for pointing them out.
>

If you have tested them as working and fixing a problem for you, you
should contribute a tested-by tag to the series.

Regards,
Peter

> No need to consider this any further then.
>
> Cheers,
>
> François.
>
> On Tue, Nov 17, 2015 at 1:49 PM, Peter Maydell 
> wrote:
>>
>> On 17 November 2015 at 21:40, François Baldassari
>>  wrote:
>> > On armv7m mcus, the BASEPRI register can be set to mask interrupts
>> > above a certain priority.
>> >
>> > This changeset implements that functionality by way of the NVIC which
>> > ultimately sets the interrupt mask in the GIC.
>> >
>> > Signed-off-by: François Baldassari 
>>
>> There are a lot of problems with our NVIC priority handling
>> right now. You might like to take a look at the patch set that
>> Michael Davidsaver sent out earlier this month:
>> https://lists.nongnu.org/archive/html/qemu-devel/2015-11/msg01542.html
>>
>> That has some problems but I think it's probably the way we're
>> going to go to fix up the NVIC.
>>
>> thanks
>> -- PMM
>
>

Re: [Qemu-devel] CPU Cache simulation

[Eduardo Habkost]

On Wed, Nov 18, 2015 at 12:56:28PM -0500, Pranith Kumar wrote:
> Hi,
> 
> On Wed, Nov 18, 2015 at 11:48 AM, Hao Bai  wrote:
> 
> > Sorry about the ambiguity.
> > I am using x86-64 architecture in user mode. Basically, I am trying to log
> > all the cache activities when I run a guest program with QEMU. That's why I
> > asked whether QEMU simulated CPU caches. I was assuming if QEMU did not
> > simulate CPU caches then there would be no way to do this (Correct me if I
> > am wrong). Is there a way to do this?
> >
> >
> 
> 
> You can try qsim: https://github.com/gtcasl/qsim
> 
> In particular look at the cache simulator:
> https://github.com/gtcasl/qsim/blob/master/examples/x86/cachesim.cpp
> 
> Setup instructions: https://github.com/pranith/qsim-setup

Interesting. How much did you change QEMU to make this work? Have
you been rebasing this to recent QEMU versions often?

-- 
Eduardo

Re: [Qemu-devel] [Intel-gfx] [Announcement] 2015-Q3 release of XenGT - a Mediated Graphics Passthrough Solution from Intel

[Alex Williamson]

[cc +qemu-devel, +paolo, +gerd]

On Tue, 2015-10-27 at 17:25 +0800, Jike Song wrote:
> Hi all,
> 
> We are pleased to announce another update of Intel GVT-g for Xen.
> 
> Intel GVT-g is a full GPU virtualization solution with mediated
> pass-through, starting from 4th generation Intel Core(TM) processors
> with Intel Graphics processors. A virtual GPU instance is maintained
> for each VM, with part of performance critical resources directly
> assigned. The capability of running native graphics driver inside a
> VM, without hypervisor intervention in performance critical paths,
> achieves a good balance among performance, feature, and sharing
> capability. Xen is currently supported on Intel Processor Graphics
> (a.k.a. XenGT); and the core logic can be easily ported to other
> hypervisors.
> 
> 
> Repositories
> 
>  Kernel: https://github.com/01org/igvtg-kernel (2015q3-3.18.0 branch)
>  Xen: https://github.com/01org/igvtg-xen (2015q3-4.5 branch)
>  Qemu: https://github.com/01org/igvtg-qemu (xengt_public2015q3 branch)
> 
> 
> This update consists of:
> 
>  - XenGT is now merged with KVMGT in unified repositories(kernel and 
> qemu), but currently
>different branches for qemu.  XenGT and KVMGT share same iGVT-g core 
> logic.

Hi!

At redhat we've been thinking about how to support vGPUs from multiple
vendors in a common way within QEMU.  We want to enable code sharing
between vendors and give new vendors an easy path to add their own
support.  We also have the complication that not all vGPU vendors are as
open source friendly as Intel, so being able to abstract the device
mediation and access outside of QEMU is a big advantage.

The proposal I'd like to make is that a vGPU, whether it is from Intel
or another vendor, is predominantly a PCI(e) device.  We have an
interface in QEMU already for exposing arbitrary PCI devices, vfio-pci.
Currently vfio-pci uses the VFIO API to interact with "physical" devices
and system IOMMUs.  I highlight /physical/ there because some of these
physical devices are SR-IOV VFs, which is somewhat of a fuzzy concept,
somewhere between fixed hardware and a virtual device implemented in
software.  That software just happens to be running on the physical
endpoint.

vGPUs are similar, with the virtual device created at a different point,
host software.  They also rely on different IOMMU constructs, making use
of the MMU capabilities of the GPU (GTTs and such), but really having
similar requirements.

The proposal is therefore that GPU vendors can expose vGPUs to
userspace, and thus to QEMU, using the VFIO API.  For instance, vfio
supports modular bus drivers and IOMMU drivers.  An intel-vfio-gvt-d
module (or extension of i915) can register as a vfio bus driver, create
a struct device per vGPU, create an IOMMU group for that device, and
register that device with the vfio-core.  Since we don't rely on the
system IOMMU for GVT-d vGPU assignment, another vGPU vendor driver (or
extension of the same module) can register a "type1" compliant IOMMU
driver into vfio-core.  From the perspective of QEMU then, all of the
existing vfio-pci code is re-used, QEMU remains largely unaware of any
specifics of the vGPU being assigned, and the only necessary change so
far is how QEMU traverses sysfs to find the device and thus the IOMMU
group leading to the vfio group.

There are a few areas where we know we'll need to extend the VFIO API to
make this work, but it seems like they can all be done generically.  One
is that PCI BARs are described through the VFIO API as regions and each
region has a single flag describing whether mmap (ie. direct mapping) of
that region is possible.  We expect that vGPUs likely need finer
granularity, enabling some areas within a BAR to be trapped and fowarded
as a read or write access for the vGPU-vfio-device module to emulate,
while other regions, like framebuffers or texture regions, are directly
mapped.  I have prototype code to enable this already.

Another area is that we really don't want to proliferate each vGPU
needing a new IOMMU type within vfio.  The existing type1 IOMMU provides
potentially the most simple mapping and unmapping interface possible.
We'd therefore need to allow multiple "type1" IOMMU drivers for vfio,
making type1 be more of an interface specification rather than a single
implementation.  This is a trivial change to make within vfio and one
that I believe is compatible with the existing API.  Note that
implementing a type1-compliant vfio IOMMU does not imply pinning an
mapping every registered page.  A vGPU, with mediated device access, may
use this only to track the current HVA to GPA mappings for a VM.  Only
when a DMA is enabled for the vGPU instance is that HVA pinned and an
HPA to GPA translation programmed into the GPU MMU.

Another area of extension is how to expose a framebuffer to QEMU for
seamless integration into a SPICE/VNC channel.  For this I believe we
could use a new region, much like we've done to expose 

Re: [Qemu-devel] [PATCH v12 29/36] qobject: Rename qtype_code to QType

[Markus Armbruster]

Eric Blake  writes:

> The name QType is more in line with our conventions for qapi
> types, and matches the fact that each enum member has a prefix
> of QTYPE_.
>
> Signed-off-by: Eric Blake 

At this point, the connection to QAPI is unobvious.

You can either point to CODING_STYLE, which ask for type names in
CamelCase, or to the next patch.

Patch looks good.

Re: [Qemu-devel] [PATCH v12 32/36] qapi: Inline _make_implicit_tag()

[Markus Armbruster]

Eric Blake  writes:

> Now that alternates no longer use an implicit tag, we can
> inline _make_implicit_tag() into its one caller of
> _def_union_type().

Scratch "of".

> No change to generated code.
>
> Suggested-by: Markus Armbruster 
> Signed-off-by: Eric Blake 

Patch looks good.

Re: [Qemu-devel] [PATCH v12 31/36] qapi: Simplify visiting of alternate types

[Markus Armbruster]

Eric Blake  writes:

> Previously, working with alternates required two lookup arrays
> and some indirection: for type Foo, we created Foo_qtypes[]
> which maps each qtype to a value of the generated FooKind enum,
> then look up that value in FooKind_lookup[] like we do for other
> union types.
>
> This has a couple of subtle bugs.  First, the generator was
> creating a call with a parameter '(int *) &(*obj)->type' where
> type is an enum type; this is unsafe if the compiler chooses
> to store the enum type in a different size than int, where
> assigning through the wrong size pointer can corrupt data or
> cause a SIGBUS.  [We still have the casting bug for our enum
> visitors, but that's a topic for a different patch.]

I'm not sure I get the last sentence.

> Second, since the values of the FooKind enum start at zero, all
> entries of the Foo_qtypes[] array that were not explicitly
> initialized will map to the same branch of the union as the
> first member of the alternate, rather than triggering a desired
> failure in visit_get_next_type().  Fortunately, the bug seldom
> bites; the very next thing the input visitor does is try to
> parse the incoming JSON with the wrong parser, which normally
> fails; the output visitor is not used with a C struct in that
> state, and the dealloc visitor has nothing to clean up (so
> there is no leak).
>
> However, the second bug IS observable in one case: parsing an
> integer causes unusual behavior in an alternate that contains
> at least a 'number' member but no 'int' member, because the
> 'number' parser accepts QTYPE_QINT in addition to the expected
> QTYPE_QFLOAT (that is, since 'int' is not a member, the type
> QTYPE_QINT accidentally maps to FooKind 0; if this enum value
> is the 'number' branch the integer parses successfully, but if
> the 'number' branch is not first, some other branch tries to
> parse the integer and rejects it).  A later patch will worry
> about fixing alternates to always parse all inputs that a
> non-alternate 'number' would accept, for now this is still
> marked FIXME in the updated test-qmp-input-visitor.c, to
> merely point out that new undesired behavior of 'ans' matches
> the existing undesired behavior of 'asn'.
>
> This patch fixes the default-initialization bug by deleting the
> indirection, and modifying get_next_type() to directly assign a
> QTypeCode parameter.  This in turn fixes the type-casting bug,
> as we are no longer casting a pointer to enum to a questionable
> size. There is no longer a need to generate an implicit FooKind
> enum associated with the alternate type (since the QMP wire
> format never uses the stringized counterparts of the C union
> member names).  Since the updated visit_get_next_type() does not
> know which qtypes are expected, the generated visitor is
> modified to generate an error statement if an unexpected type is
> encountered.
>
> Callers now have to know the QTYPE_* mapping when looking at the
> discriminator; but so far, only the testsuite was even using the
> C struct of an alternate types.  I considered the possibility of
> keeping the internal enum FooKind, but initialized differently
> than most generated arrays, as in:
>   typedef enum FooKind {
>   FOO_KIND_A = QTYPE_QDICT,
>   FOO_KIND_B = QTYPE_QINT,
>   } FooKind;
> to create nicer aliases for knowing when to use foo->a or foo->b
> when inspecting foo->type; but it turned out to add too much
> complexity, especially without a client.
>
> There is a user-visible side effect to this change, but I
> consider it to be an improvement. Previously,
> the invalid QMP command:
>   {"execute":"blockdev-add", "arguments":{"options":
> {"driver":"raw", "id":"a", "file":true}}}
> failed with:
>   {"error": {"class": "GenericError",
> "desc": "Invalid parameter type for 'file', expected: QDict"}}
> (visit_get_next_type() succeeded, and the error comes from the
> visit_type_BlockdevOptions() expecting {}; there is no mention of
> the fact that a string would also work).  Now it fails with:
>   {"error": {"class": "GenericError",
> "desc": "Invalid parameter type for 'file', expected: BlockdevRef"}}
> (the error when the next type doesn't match any expected types for
> the overall alternate).
>
> Signed-off-by: Eric Blake 

Patch looks good.

Re: [Qemu-devel] [PATCH] qom: add a test case for complex property finalization

[Andreas Färber]

Am 16.11.2015 um 16:37 schrieb Daniel P. Berrange:
> QDev has some quite complex object child/link relationships
> which place some requirements on the object_property_del_all
> method to consider that properties can be modified while
> being iterated over.
> 
> This extends the QOM property test case to replicate the
> QDev like structure and expose any potential bugs in the
> object_property_del_all method.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  tests/check-qom-proplist.c | 159 
> +
>  1 file changed, 159 insertions(+)

Thanks, queued on qom-next and sent a patch to include it in MAINTAINERS.

Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

[Qemu-devel] [PATCH v1 4/4] char: introduce support for TLS encrypted TCP chardev backend

[Daniel P. Berrange]

This integrates support for QIOChannelTLS object in the TCP
chardev backend. If the 'tls-creds=NAME' option is passed with
the '-chardev tcp' argument, then it will setup the chardev
such that the client is required to establish a TLS handshake
when connecting. There is no support for checking the client
certificate against ACLs in this initial patch. This is pending
work to QOM-ify the ACL object code.

A complete invocation to run QEMU as the server for a TLS
encrypted serial dev might be

  $ qemu-system-x86_64 \
  -nodefconfig -nodefaults -device sga -display none \
  -chardev socket,id=s0,host=127.0.0.1,port=9000,tls-creds=tls0,server \
  -device isa-serial,chardev=s0 \
  -object tls-creds-x509,id=tls0,endpoint=server,verify-peer=off,\
 dir=/home/berrange/security/qemutls

To test with the gnutls-cli tool as the client:

  $ gnutls-cli --priority=NORMAL -p 9000 \
   --x509cafile=/home/berrange/security/qemutls/ca-cert.pem \
   127.0.0.1

If QEMU was told to use 'anon' credential type, then use the
priority string 'NORMAL:+ANON-DH' with gnutls-cli

Alternatively, if setting up a chardev to operate as a client,
then the TLS credentials registered must be for the client
endpoint. First a TLS server must be setup, which can be done
with the gnutls-serv tool

  $ gnutls-serv --priority=NORMAL -p 9000 --echo \
   --x509cafile=/home/berrange/security/qemutls/ca-cert.pem \
   --x509certfile=/home/berrange/security/qemutls/server-cert.pem \
   --x509keyfile=/home/berrange/security/qemutls/server-key.pem

Then QEMU can connect with

  $ qemu-system-x86_64 \
  -nodefconfig -nodefaults -device sga -display none \
  -chardev socket,id=s0,host=127.0.0.1,port=9000,tls-creds=tls0 \
  -device isa-serial,chardev=s0 \
  -object tls-creds-x509,id=tls0,endpoint=client,\
dir=/home/berrange/security/qemutls

Signed-off-by: Daniel P. Berrange 
---
 qapi-schema.json |   2 +
 qemu-char.c  | 138 ++-
 qemu-options.hx  |   9 +++-
 3 files changed, 135 insertions(+), 14 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 8b1a423..be6636c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3046,6 +3046,7 @@
 #
 # @addr: socket address to listen on (server=true)
 #or connect to (server=false)
+# @tls-creds: #optional the ID of the TLS credentials object (since 2.4)
 # @server: #optional create server socket (default: true)
 # @wait: #optional wait for incoming connection on server
 #sockets (default: false).
@@ -3060,6 +3061,7 @@
 # Since: 1.4
 ##
 { 'struct': 'ChardevSocket', 'data': { 'addr'   : 'SocketAddress',
+ '*tls-creds'  : 'str',
  '*server': 'bool',
  '*wait'  : 'bool',
  '*nodelay'   : 'bool',
diff --git a/qemu-char.c b/qemu-char.c
index f481d3b..025b9d8 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -34,6 +34,7 @@
 #include "qapi-visit.h"
 #include "io/channel-socket.h"
 #include "io/channel-file.h"
+#include "io/channel-tls.h"
 
 #include 
 #include 
@@ -2482,9 +2483,11 @@ static CharDriverState 
*qemu_chr_open_udp(QIOChannelSocket *sioc)
 /* TCP Net console */
 
 typedef struct {
-QIOChannel *ioc;
+QIOChannel *ioc; /* Client I/O channel */
+QIOChannelSocket *sioc; /* Client master channel */
 QIOChannelSocket *listen_ioc;
 guint listen_tag;
+QCryptoTLSCreds *tls_creds;
 int connected;
 int max_size;
 int do_telnetopt;
@@ -2719,6 +2722,8 @@ static void tcp_chr_disconnect(CharDriverState *chr)
 QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, NULL);
 }
 remove_fd_in_watch(chr);
+object_unref(OBJECT(s->sioc));
+s->sioc = NULL;
 object_unref(OBJECT(s->ioc));
 s->ioc = NULL;
 g_free(chr->filename);
@@ -2792,12 +2797,12 @@ static void tcp_chr_connect(void *opaque)
 {
 CharDriverState *chr = opaque;
 TCPCharDriver *s = chr->opaque;
-QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(s->ioc);
 
 g_free(chr->filename);
-chr->filename = sockaddr_to_str(>localAddr, sioc->localAddrLen,
->remoteAddr, sioc->remoteAddrLen,
-s->is_listen, s->is_telnet);
+chr->filename = sockaddr_to_str(
+>sioc->localAddr, s->sioc->localAddrLen,
+>sioc->remoteAddr, s->sioc->remoteAddrLen,
+s->is_listen, s->is_telnet);
 
 s->connected = 1;
 if (s->ioc) {
@@ -2884,6 +2889,57 @@ static void tcp_chr_telnet_init(CharDriverState *chr)
 init, NULL);
 }
 
+
+static void tcp_chr_tls_handshake(Object *source,
+  Error *err,
+  gpointer user_data)
+{
+CharDriverState *chr = user_data;
+TCPCharDriver *s = chr->opaque;
+
+if (err) {
+

[Qemu-devel] [PATCH v1 0/3] Convert VNC server to QIOChannel

[Daniel P. Berrange]

This is an update of patches previously shown in an RFC posting

  RFC: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00829.html

This series depends on the previously posted series:

  "[PATCH v3 0/9] Introduce I/O channels framework"

This short series converts the VNC server to use the new
QIOChannel framework. This removes all the websocket protocol
code from the VNC server and further simplifies the TLS
handling in the VNC server

The conversion has been tested against the virt-viewer/
remote-viewer programs in the various TLS modes, and also
against the noVNC proxy websockets client with and without
TLS support.

Daniel P. Berrange (3):
  ui: convert VNC server to use QIOChannelSocket
  ui: convert VNC server to use QIOChannelTLS
  ui: convert VNC server to use QIOChannelWebsock

 ui/vnc-auth-sasl.c |  57 -
 ui/vnc-auth-vencrypt.c |  93 +++-
 ui/vnc-jobs.c  |  20 +-
 ui/vnc-ws.c| 400 ++--
 ui/vnc-ws.h|  71 +-
 ui/vnc.c   | 608 ++---
 ui/vnc.h   |  31 ++-
 7 files changed, 448 insertions(+), 832 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCH v1 1/3] ui: convert VNC server to use QIOChannelSocket

[Daniel P. Berrange]

The minimal first step conversion to use QIOChannelSocket
classes instead of directly using POSIX sockets API. This
will later be extended to also cover the TLS, SASL and
websockets code.

Signed-off-by: Daniel P. Berrange 
---
 ui/vnc-auth-sasl.c |  57 +++--
 ui/vnc-auth-vencrypt.c |  27 ++-
 ui/vnc-jobs.c  |  20 +-
 ui/vnc-ws.c|  51 -
 ui/vnc-ws.h|   8 +-
 ui/vnc.c   | 574 +
 ui/vnc.h   |  22 +-
 7 files changed, 433 insertions(+), 326 deletions(-)

diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
index fc732bd..de8abc9 100644
--- a/ui/vnc-auth-sasl.c
+++ b/ui/vnc-auth-sasl.c
@@ -62,7 +62,7 @@ long vnc_client_write_sasl(VncState *vs)
   (const char **)>sasl.encoded,
   >sasl.encodedLength);
 if (err != SASL_OK)
-return vnc_client_io_error(vs, -1, EIO);
+return vnc_client_io_error(vs, -1, NULL);
 
 vs->sasl.encodedOffset = 0;
 }
@@ -86,7 +86,11 @@ long vnc_client_write_sasl(VncState *vs)
  * SASL encoded output
  */
 if (vs->output.offset == 0) {
-qemu_set_fd_handler(vs->csock, vnc_client_read, NULL, vs);
+if (vs->ioc_tag) {
+g_source_remove(vs->ioc_tag);
+}
+vs->ioc_tag = qio_channel_add_watch(
+vs->ioc, G_IO_IN, vnc_client_io, vs, NULL);
 }
 
 return ret;
@@ -110,7 +114,7 @@ long vnc_client_read_sasl(VncState *vs)
   , );
 
 if (err != SASL_OK)
-return vnc_client_io_error(vs, -1, -EIO);
+return vnc_client_io_error(vs, -1, NULL);
 VNC_DEBUG("Read SASL Encoded %p size %ld Decoded %p size %d\n",
   encoded, ret, decoded, decodedLen);
 buffer_reserve(>input, decodedLen);
@@ -255,17 +259,17 @@ static int protocol_client_auth_sasl_step(VncState *vs, 
uint8_t *data, size_t le
 vnc_read_when(vs, protocol_client_auth_sasl_step_len, 4);
 } else {
 if (!vnc_auth_sasl_check_ssf(vs)) {
-VNC_DEBUG("Authentication rejected for weak SSF %d\n", vs->csock);
+VNC_DEBUG("Authentication rejected for weak SSF %p\n", vs->ioc);
 goto authreject;
 }
 
 /* Check username whitelist ACL */
 if (vnc_auth_sasl_check_access(vs) < 0) {
-VNC_DEBUG("Authentication rejected for ACL %d\n", vs->csock);
+VNC_DEBUG("Authentication rejected for ACL %p\n", vs->ioc);
 goto authreject;
 }
 
-VNC_DEBUG("Authentication successful %d\n", vs->csock);
+VNC_DEBUG("Authentication successful %p\n", vs->ioc);
 vnc_write_u32(vs, 0); /* Accept auth */
 /*
  * Delay writing in SSF encoded mode until pending output
@@ -383,17 +387,17 @@ static int protocol_client_auth_sasl_start(VncState *vs, 
uint8_t *data, size_t l
 vnc_read_when(vs, protocol_client_auth_sasl_step_len, 4);
 } else {
 if (!vnc_auth_sasl_check_ssf(vs)) {
-VNC_DEBUG("Authentication rejected for weak SSF %d\n", vs->csock);
+VNC_DEBUG("Authentication rejected for weak SSF %p\n", vs->ioc);
 goto authreject;
 }
 
 /* Check username whitelist ACL */
 if (vnc_auth_sasl_check_access(vs) < 0) {
-VNC_DEBUG("Authentication rejected for ACL %d\n", vs->csock);
+VNC_DEBUG("Authentication rejected for ACL %p\n", vs->ioc);
 goto authreject;
 }
 
-VNC_DEBUG("Authentication successful %d\n", vs->csock);
+VNC_DEBUG("Authentication successful %p\n", vs->ioc);
 vnc_write_u32(vs, 0); /* Accept auth */
 start_client_init(vs);
 }
@@ -487,6 +491,32 @@ static int protocol_client_auth_sasl_mechname_len(VncState 
*vs, uint8_t *data, s
 return 0;
 }
 
+static char *
+vnc_socket_ip_addr_string(QIOChannelSocket *ioc,
+  bool local,
+  Error **errp)
+{
+SocketAddress *addr;
+char *ret;
+
+if (local) {
+addr = qio_channel_socket_get_local_address(ioc, errp);
+} else {
+addr = qio_channel_socket_get_remote_address(ioc, errp);
+}
+if (!addr) {
+return NULL;
+}
+
+if (addr->type != SOCKET_ADDRESS_KIND_INET) {
+error_setg(errp, "Not an inet socket type");
+return NULL;
+}
+ret = g_strdup_printf("%s;%s", addr->u.inet->host, addr->u.inet->port);
+qapi_free_SocketAddress(addr);
+return ret;
+}
+
 void start_auth_sasl(VncState *vs)
 {
 const char *mechlist = NULL;
@@ -495,13 +525,16 @@ void start_auth_sasl(VncState *vs)
 char *localAddr, *remoteAddr;
 int mechlistlen;
 
-VNC_DEBUG("Initialize SASL auth %d\n", vs->csock);
+VNC_DEBUG("Initialize SASL auth %p\n", vs->ioc);
 
 /* Get local & remote client addresses in form  IPADDR;PORT */
-if (!(localAddr = vnc_socket_local_addr("%s;%s", 

[Qemu-devel] [PATCH v1 2/3] ui: convert VNC server to use QIOChannelTLS

[Daniel P. Berrange]

Switch VNC server over to using the QIOChannelTLS object for
the TLS session. This removes all remaining VNC specific code
for dealing with TLS handshakes.

Signed-off-by: Daniel P. Berrange 
---
 ui/vnc-auth-vencrypt.c | 106 ++---
 ui/vnc-ws.c|  95 +---
 ui/vnc.c   |  69 +++-
 ui/vnc.h   |   5 +--
 4 files changed, 73 insertions(+), 202 deletions(-)

diff --git a/ui/vnc-auth-vencrypt.c b/ui/vnc-auth-vencrypt.c
index 95a6823..093dd2f 100644
--- a/ui/vnc-auth-vencrypt.c
+++ b/ui/vnc-auth-vencrypt.c
@@ -63,71 +63,21 @@ static void start_auth_vencrypt_subauth(VncState *vs)
 }
 }
 
-static gboolean vnc_tls_handshake_io(QIOChannel *ioc,
- GIOCondition condition,
- void *opaque);
-
-static int vnc_start_vencrypt_handshake(VncState *vs)
+static void vnc_tls_handshake_done(Object *source,
+   Error *err,
+   gpointer user_data)
 {
-Error *err = NULL;
-
-if (qcrypto_tls_session_handshake(vs->tls, ) < 0) {
-goto error;
-}
+VncState *vs = user_data;
 
-switch (qcrypto_tls_session_get_handshake_status(vs->tls)) {
-case QCRYPTO_TLS_HANDSHAKE_COMPLETE:
-VNC_DEBUG("Handshake done, checking credentials\n");
-if (qcrypto_tls_session_check_credentials(vs->tls, ) < 0) {
-goto error;
-}
-VNC_DEBUG("Client verification passed, starting TLS I/O\n");
-if (vs->ioc_tag) {
-g_source_remove(vs->ioc_tag);
-}
+if (err) {
+VNC_DEBUG("Handshake failed %s\n",
+  error_get_pretty(err));
+vnc_client_error(vs);
+} else {
 vs->ioc_tag = qio_channel_add_watch(
 vs->ioc, G_IO_IN | G_IO_OUT, vnc_client_io, vs, NULL);
-
 start_auth_vencrypt_subauth(vs);
-break;
-
-case QCRYPTO_TLS_HANDSHAKE_RECVING:
-VNC_DEBUG("Handshake interrupted (blocking read)\n");
-if (vs->ioc_tag) {
-g_source_remove(vs->ioc_tag);
-}
-vs->ioc_tag = qio_channel_add_watch(
-vs->ioc, G_IO_IN, vnc_tls_handshake_io, vs, NULL);
-break;
-
-case QCRYPTO_TLS_HANDSHAKE_SENDING:
-VNC_DEBUG("Handshake interrupted (blocking write)\n");
-if (vs->ioc_tag) {
-g_source_remove(vs->ioc_tag);
-}
-vs->ioc_tag = qio_channel_add_watch(
-vs->ioc, G_IO_OUT, vnc_tls_handshake_io, vs, NULL);
-break;
 }
-
-return 0;
-
- error:
-VNC_DEBUG("Handshake failed %s\n", error_get_pretty(err));
-error_free(err);
-vnc_client_error(vs);
-return -1;
-}
-
-static gboolean vnc_tls_handshake_io(QIOChannel *ioc G_GNUC_UNUSED,
- GIOCondition condition G_GNUC_UNUSED,
- void *opaque)
-{
-VncState *vs = (VncState *)opaque;
-
-VNC_DEBUG("Handshake IO continue\n");
-vnc_start_vencrypt_handshake(vs);
-return TRUE;
 }
 
 
@@ -142,33 +92,37 @@ static int protocol_client_vencrypt_auth(VncState *vs, 
uint8_t *data, size_t len
 vnc_client_error(vs);
 } else {
 Error *err = NULL;
+QIOChannelTLS *tls;
 VNC_DEBUG("Accepting auth %d, setting up TLS for handshake\n", auth);
 vnc_write_u8(vs, 1); /* Accept auth */
 vnc_flush(vs);
 
-vs->tls = qcrypto_tls_session_new(vs->vd->tlscreds,
-  NULL,
-  vs->vd->tlsaclname,
-  QCRYPTO_TLS_CREDS_ENDPOINT_SERVER,
-  );
-if (!vs->tls) {
-VNC_DEBUG("Failed to setup TLS %s\n",
-  error_get_pretty(err));
+if (vs->ioc_tag) {
+g_source_remove(vs->ioc_tag);
+vs->ioc_tag = 0;
+}
+
+tls = qio_channel_tls_new_server(
+vs->ioc,
+vs->vd->tlscreds,
+vs->vd->tlsaclname,
+);
+if (!tls) {
+VNC_DEBUG("Failed to setup TLS %s\n", error_get_pretty(err));
 error_free(err);
 vnc_client_error(vs);
 return 0;
 }
 
-qcrypto_tls_session_set_callbacks(vs->tls,
-  vnc_tls_push,
-  vnc_tls_pull,
-  vs);
-
 VNC_DEBUG("Start TLS VeNCrypt handshake process\n");
-if (vnc_start_vencrypt_handshake(vs) < 0) {
-VNC_DEBUG("Failed to start TLS handshake\n");
-return 0;
-}
+object_unref(OBJECT(vs->ioc));
+vs->ioc = QIO_CHANNEL(tls);
+vs->tls = qio_channel_tls_get_session(tls);
+
+

[Qemu-devel] [PATCH 1/2] nvme: fix identify to be NVMe 1.1 compliant

[Christoph Hellwig]

NVMe 1.1 requires devices to implement a Namespace List subcommand of
the identify command.  Qemu not only not implements this features, but
also misinterprets it as an Identify Controller request.  Due to this
any OS trying to use the Namespace List will fail the probe.

Signed-off-by: Christoph Hellwig 
---
 hw/block/nvme.c | 59 ++---
 1 file changed, 52 insertions(+), 7 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 5da41b2..4a6443f 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -462,19 +462,22 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeCmd *cmd)
 return NVME_SUCCESS;
 }
 
-static uint16_t nvme_identify(NvmeCtrl *n, NvmeCmd *cmd)
+static uint16_t nvme_identify_ctrl(NvmeCtrl *n, NvmeIdentify *c)
+{
+uint64_t prp1 = le64_to_cpu(c->prp1);
+uint64_t prp2 = le64_to_cpu(c->prp2);
+
+return nvme_dma_read_prp(n, (uint8_t *)>id_ctrl, sizeof(n->id_ctrl),
+prp1, prp2);
+}
+
+static uint16_t nvme_identify_ns(NvmeCtrl *n, NvmeIdentify *c)
 {
 NvmeNamespace *ns;
-NvmeIdentify *c = (NvmeIdentify *)cmd;
-uint32_t cns  = le32_to_cpu(c->cns);
 uint32_t nsid = le32_to_cpu(c->nsid);
 uint64_t prp1 = le64_to_cpu(c->prp1);
 uint64_t prp2 = le64_to_cpu(c->prp2);
 
-if (cns) {
-return nvme_dma_read_prp(n, (uint8_t *)>id_ctrl, sizeof(n->id_ctrl),
-prp1, prp2);
-}
 if (nsid == 0 || nsid > n->num_namespaces) {
 return NVME_INVALID_NSID | NVME_DNR;
 }
@@ -484,6 +487,48 @@ static uint16_t nvme_identify(NvmeCtrl *n, NvmeCmd *cmd)
 prp1, prp2);
 }
 
+static uint16_t nvme_identify_nslist(NvmeCtrl *n, NvmeIdentify *c)
+{
+static const int data_len = 4096;
+uint32_t min_nsid = le32_to_cpu(c->nsid);
+uint64_t prp1 = le64_to_cpu(c->prp1);
+uint64_t prp2 = le64_to_cpu(c->prp2);
+uint32_t *list;
+uint16_t ret;
+int i, j = 0;
+
+list = g_malloc0(data_len);
+for (i = 0; i < n->num_namespaces; i++) {
+if (i <= min_nsid) {
+continue;
+}
+list[j++] = cpu_to_le32(i);
+if (j == data_len / sizeof(uint32_t)) {
+break;
+}
+}
+ret = nvme_dma_read_prp(n, (uint8_t *)list, data_len, prp1, prp2);
+g_free(list);
+return ret;
+}
+
+
+static uint16_t nvme_identify(NvmeCtrl *n, NvmeCmd *cmd)
+{
+NvmeIdentify *c = (NvmeIdentify *)cmd;
+
+switch (le32_to_cpu(c->cns)) {
+case 0x00:
+return nvme_identify_ns(n, c);
+case 0x01:
+return nvme_identify_ctrl(n, c);
+case 0x02:
+return nvme_identify_nslist(n, c);
+default:
+return NVME_INVALID_FIELD | NVME_DNR;
+}
+}
+
 static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
 {
 uint32_t dw10 = le32_to_cpu(cmd->cdw10);
-- 
1.9.1

Re: [Qemu-devel] [PATCH v12 36/36] qapi: Shorter visits of optional fields

[Markus Armbruster]

Eric Blake  writes:

> For less code, reflect the determined boolean value of an optional
> visit back to the caller instead of making the caller read the
> boolean after the fact.
>
> The resulting generated code has the following diff:
>
> |-visit_optional(v, _fdset_id, "fdset-id");
> |-if (has_fdset_id) {
> |+if (visit_optional(v, _fdset_id, "fdset-id")) {
> | visit_type_int(v, _id, "fdset-id", );
> | if (err) {
> | goto out;
> | }
> | }
>
> Signed-off-by: Eric Blake 

Feels like a wash to me, but I'm willing to take it anyway, in
recognition of the massive amount of work you've been doing in this area
:)

Re: [Qemu-devel] [PATCH v12 21/36] qapi: Tighten the regex on valid names

[Markus Armbruster]

Eric Blake  writes:

> On 11/18/2015 04:51 AM, Markus Armbruster wrote:
>> Eric Blake  writes:
>> 
>>> We already documented that qapi names should match specific
>>> patterns (such as starting with a letter unless it was an enum
>>> value or a downstream extension).  Tighten that from a suggestion
>>> into a hard requirement, which frees up names beginning with a
>>> single underscore for qapi internal usage.
>> 
>> If we care enough about naming conventions to document them, enforcing
>> them makes only sense.
>> 
>>> Also restrict things
>>> to avoid 'a__b' or 'a--b' (that is, dash and underscore must not
>>> occur consecutively).
>> 
>> I feel this is entering "foolish names" territory, where things like
>> "IcAnFiNdNaMeSeVeNlEsSrEaDaBlEtHaNStudlyCaps" live.  Catching fools
>
> IhAdToOmUcHfUnReAdInGtHaT
>
> [It's amazing that scholars can ever manage to correctly interpret old
> written languages, thanks to omitted word breaks (scriptio continua) or
> omitted vowels (such as in Hebrew)]

Indeed.

>> automatically is generally futile, they're too creative :)
>> 
>> Let's drop this part.
>
> Sure, I can do that. I'll post a fixup patch, as it will affect docs too.
>
>> 
>>> Add a new test, reserved-member-underscore, to demonstrate the
>>> tighter checking.
>>>
>>> Signed-off-by: Eric Blake 
>>>
>>> ---
>>> v12: new patch
>>> ---
>
>>> +incompatibly in a future release.  All names must begin with a letter,
>>> +and contain only ASCII letters, digits, dash, and underscore, where
>>> +dash and underscore cannot occur consecutively.  There are two
>
> Namely, right here.
>
>>> +# Names must be letters, numbers, -, and _.  They must start with letter,
>>> +# except for downstream extensions which must start with __RFQDN_.
>>> +# Dots are only valid in the downstream extension prefix.
>>> +valid_name = re.compile('^(__[a-zA-Z][a-zA-Z0-9.]*_)?'
>>> +'[a-zA-Z][a-zA-Z0-9]*([_-][a-zA-Z0-9]+)*$')
>> 
>> This regexp consists of two parts: optional __RFQDN_ prefix and the name
>> proper.
>> 
>> The latter stays simpler if we don't attempt to catch adjacent [-_].
>> 
>> The former isn't quite right.  According to RFC 822 Appendix 1 - Domain
>> Name Syntax Specification:
>> 
>> * We must accept '-' in addition to digits.
>> 
>> * We require RFQDN to start with a letter.  This is still a loose match.
>>   The tight match is "labels separated by dots", where labels consist of
>>   letters, digits and '-', starting with a letter.  I wouldn't bother
>>   checking first characters are letters at all.
>> 
>> Recommend
>> 
>>valid_name = re.compile('^(__[a-zA-Z0-9.-]+_)?'
>>'[a-zA-Z][a-zA-Z0-9_-]*$')
>
> Sure, that works for me. It's tighter than what we had before, but
> looser than what I proposed so that it allows more RFQDN.  It
> potentially lets users confuse us by abusing 'foo__max' or similar, but
> I'm less worried about that abuse - it's okay to stop the blatantly
> obvious mistakes without worrying about the corner cases, if it makes
> the maintenance easier for the cases we do catch.
>
>> 
>>>
>>>
>>>  def check_name(expr_info, source, name, allow_optional=False,
>>> @@ -374,8 +376,8 @@ def check_name(expr_info, source, name, 
>>> allow_optional=False,
>>>  % (source, name))
>>>  # Enum members can start with a digit, because the generated C
>>>  # code always prefixes it with the enum name
>>> -if enum_member:
>>> -membername = '_' + membername
>>> +if enum_member and membername[0].isdigit():
>> 
>> What's wrong with the old condition?
>> 
>>> +membername = 'D' + membername
>
> The old code prepended a lone '_', which doesn't work any more with the
> tighter valid_name regex, so we have to use some other prefix (I picked
> 'D').
>
>>>  # Reserve the entire 'q_' namespace for c_name()
>>>  if not valid_name.match(membername) or \
>>> c_name(membername, False).startswith('q_'):
>
> The other thing is that I realized that an enum value of 'q-int' was
> getting munged to '_q-int' which no longer gets flagged by the c_name()
> filter.  But neither would 'Dq-int' get flagged.  So limiting the
> munging to just enum values that start with a digit (where we know it
> doesn't start with q) means we don't weaken the second condition.

Aha!  Worth a comment, I think.

> I guess I need to call that out in the commit message; all the more
> reason for me to send a fixup.

Yes, please!

[Qemu-devel] [PATCH qom-next] MAINTAINERS: Add check-qom-proplist to QOM

[Andreas Färber]

Add the QOM unit test to the QOM maintenance area so that maintainers
get CC'ed on changes and to document QOM test coverage.

Cc: Daniel P. Berrange 
Signed-off-by: Andreas Färber 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9e1fa72..18250ae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1164,6 +1164,7 @@ F: include/qom/
 X: include/qom/cpu.h
 F: qom/
 X: qom/cpu.c
+F: tests/check-qom-proplist.c
 F: tests/qom-test.c
 
 QMP
-- 
2.6.2

Re: [Qemu-devel] CPU Cache simulation

[Pranith Kumar]

On Wed, Nov 18, 2015 at 1:06 PM, Eduardo Habkost  wrote:
>
>
> Interesting. How much did you change QEMU to make this work? Have
> you been rebasing this to recent QEMU versions often?


The core of qemu is not changed except for one TCG issue I didn't know
how to fix. Rest is just annotating the code to generate appropriate
callbacks. You find the patches here:
https://github.com/pranith/qemu/commits/aaa8b521187e4ecd1d35914e9b119f9d6eaa8633

I try to rebase once a release comes out. The current version is based
on 2.4, so it is pretty current. I will rebase onto 2.5 in the near
future since the rc0 is out.

-- 
Pranith

[Qemu-devel] [PATCH v3 1/9] io: add abstract QIOChannel classes

[Daniel P. Berrange]

Start the new generic I/O channel framework by defining a
QIOChannel abstract base class. This is designed to feel
similar to GLib's GIOChannel, but with the addition of
support for using iovecs, qemu error reporting, file
descriptor passing, coroutine integration and use of
the QOM framework for easier sub-classing.

The intention is that anywhere in QEMU that almost
anywhere that deals with sockets will use this new I/O
infrastructure, so that it becomes trivial to then layer
in support for TLS encryption. This will at least include
the VNC server, char device backend and migration code.

Signed-off-by: Daniel P. Berrange 
---
 MAINTAINERS  |   7 +
 Makefile |   2 +
 Makefile.objs|   5 +
 Makefile.target  |   2 +
 include/io/channel.h | 503 +++
 io/Makefile.objs |   1 +
 io/channel.c | 291 +
 7 files changed, 811 insertions(+)
 create mode 100644 include/io/channel.h
 create mode 100644 io/Makefile.objs
 create mode 100644 io/channel.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9e1fa72..2f9d686 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1231,6 +1231,13 @@ S: Odd fixes
 F: util/buffer.c
 F: include/qemu/buffer.h
 
+I/O Channels
+M: Daniel P. Berrange 
+S: Maintained
+F: io/
+F: include/io/
+F: tests/test-io-*
+
 Usermode Emulation
 --
 Overall
diff --git a/Makefile b/Makefile
index c7fa427..b0049ea 100644
--- a/Makefile
+++ b/Makefile
@@ -159,6 +159,7 @@ dummy := $(call unnest-vars,, \
 crypto-obj-y \
 crypto-aes-obj-y \
 qom-obj-y \
+io-obj-y \
 common-obj-y \
 common-obj-m)
 
@@ -178,6 +179,7 @@ SOFTMMU_SUBDIR_RULES=$(filter %-softmmu,$(SUBDIR_RULES))
 
 $(SOFTMMU_SUBDIR_RULES): $(block-obj-y)
 $(SOFTMMU_SUBDIR_RULES): $(crypto-obj-y)
+$(SOFTMMU_SUBDIR_RULES): $(io-obj-y)
 $(SOFTMMU_SUBDIR_RULES): config-all-devices.mak
 
 subdir-%:
diff --git a/Makefile.objs b/Makefile.objs
index 77be052..dac2c02 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -28,6 +28,11 @@ crypto-aes-obj-y = crypto/
 
 qom-obj-y = qom/
 
+###
+# io-obj-y is code used by both qemu system emulation and qemu-img
+
+io-obj-y = io/
+
 ##
 # Target independent part of system emulation. The long term path is to
 # suppress *all* target specific code in case of system emulation, i.e. a
diff --git a/Makefile.target b/Makefile.target
index 962d004..34ddb7e 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -176,6 +176,7 @@ dummy := $(call unnest-vars,.., \
crypto-obj-y \
crypto-aes-obj-y \
qom-obj-y \
+   io-obj-y \
common-obj-y \
common-obj-m)
 target-obj-y := $(target-obj-y-save)
@@ -185,6 +186,7 @@ all-obj-y += $(qom-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y)
 all-obj-$(CONFIG_USER_ONLY) += $(crypto-aes-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(crypto-obj-y)
+all-obj-$(CONFIG_SOFTMMU) += $(io-obj-y)
 
 $(QEMU_PROG_BUILD): config-devices.mak
 
diff --git a/include/io/channel.h b/include/io/channel.h
new file mode 100644
index 000..f42a6a8
--- /dev/null
+++ b/include/io/channel.h
@@ -0,0 +1,503 @@
+/*
+ * QEMU I/O channels
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_CHANNEL_H__
+#define QIO_CHANNEL_H__
+
+#include "qemu-common.h"
+#include "qapi/error.h"
+#include "qom/object.h"
+
+#define TYPE_QIO_CHANNEL "qio-channel"
+#define QIO_CHANNEL(obj)\
+OBJECT_CHECK(QIOChannel, (obj), TYPE_QIO_CHANNEL)
+#define QIO_CHANNEL_CLASS(klass)\
+OBJECT_CLASS_CHECK(QIOChannelClass, klass, TYPE_QIO_CHANNEL)
+#define QIO_CHANNEL_GET_CLASS(obj)  \
+OBJECT_GET_CLASS(QIOChannelClass, obj, TYPE_QIO_CHANNEL)
+
+typedef struct QIOChannel QIOChannel;
+typedef struct QIOChannelClass QIOChannelClass;
+
+#define QIO_CHANNEL_ERR_BLOCK -2
+
+typedef enum QIOChannelFeature QIOChannelFeature;
+
+enum QIOChannelFeature {
+QIO_CHANNEL_FEATURE_FD_PASS  

[Qemu-devel] Fwd: libcacard: forward declaration of VReader

[Alex Roithman]

hi

my project not build complete, because i have:

/usr/bin/c++  ...
/home/Flash/qt-virt-manager/src/vm_viewer/qspice_widgets/qspicesmartcardmanager.cpp:89:35:
error: invalid use of incomplete type «VReader {aka struct VReaderStruct}»
 _readerList.append(_reader->name);
   ^
In file included from /usr/include/cacard/eventt.h:8:0,
 from /usr/include/cacard/vreader.h:9,
 from
/home/Flash/qt-virt-manager/src/vm_viewer/qspice_widgets/qspicesmartcardmanager.cpp:2:
/usr/include/cacard/vreadert.h:16:16: warning: forward declaration of
«VReader {aka struct VReaderStruct}»
 typedef struct VReaderStruct VReader;


in system installed
libcacard.x86_64 2:2.3.1-7.fc22
libcacard-devel.x86_64 2:2.3.1-7.fc22

in code i'm use only
#include 
from libcacard

part of code:

VReader *_reader =
static_cast(g_list_nth_data(_list, i));
 _readerList.append(_reader->name);


I accept any advice.

[Qemu-devel] [PATCH v3 4/9] io: add QIOChannelSocket class

[Daniel P. Berrange]

Implement a QIOChannel subclass that supports sockets I/O.
The implementation is able to manage a single socket file
descriptor, whether a TCP/UNIX listener, TCP/UNIX connection,
or a UDP datagram. It provides APIs which can listen and
connect either asynchronously or synchronously. Since there
is no asynchronous DNS lookup API available, it uses the
QIOTask helper for spawning a background thread to ensure
non-blocking operation.

Signed-off-by: Daniel P. Berrange 
---
 configure  |  11 +
 include/io/channel-socket.h| 244 +
 include/qemu/sockets.h |  19 ++
 io/Makefile.objs   |   1 +
 io/channel-socket.c| 753 +
 scripts/create_config  |   9 +
 tests/.gitignore   |   1 +
 tests/Makefile |   3 +
 tests/io-channel-helpers.c | 246 ++
 tests/io-channel-helpers.h |  42 +++
 tests/test-io-channel-socket.c | 399 ++
 trace-events   |  19 ++
 util/qemu-sockets.c|   2 +-
 13 files changed, 1748 insertions(+), 1 deletion(-)
 create mode 100644 include/io/channel-socket.h
 create mode 100644 io/channel-socket.c
 create mode 100644 tests/io-channel-helpers.c
 create mode 100644 tests/io-channel-helpers.h
 create mode 100644 tests/test-io-channel-socket.c

diff --git a/configure b/configure
index 0a4c78a..02d77f1 100755
--- a/configure
+++ b/configure
@@ -2395,6 +2395,14 @@ fi
 
 
 ##
+# getifaddrs (for tests/test-io-channel-socket )
+
+have_ifaddrs_h=yes
+if ! check_include "ifaddrs.h" ; then
+  have_ifaddrs_h=no
+fi
+
+##
 # VTE probe
 
 if test "$vte" != "no"; then
@@ -5105,6 +5113,9 @@ fi
 if test "$tasn1" = "yes" ; then
   echo "CONFIG_TASN1=y" >> $config_host_mak
 fi
+if test "$have_ifaddrs_h" = "yes" ; then
+echo "HAVE_IFADDRS_H=y" >> $config_host_mak
+fi
 if test "$vte" = "yes" ; then
   echo "CONFIG_VTE=y" >> $config_host_mak
   echo "VTE_CFLAGS=$vte_cflags" >> $config_host_mak
diff --git a/include/io/channel-socket.h b/include/io/channel-socket.h
new file mode 100644
index 000..0719757
--- /dev/null
+++ b/include/io/channel-socket.h
@@ -0,0 +1,244 @@
+/*
+ * QEMU I/O channels sockets driver
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ *
+ */
+
+#ifndef QIO_CHANNEL_SOCKET_H__
+#define QIO_CHANNEL_SOCKET_H__
+
+#include "io/channel.h"
+#include "io/task.h"
+#include "qemu/sockets.h"
+
+#define TYPE_QIO_CHANNEL_SOCKET "qio-channel-socket"
+#define QIO_CHANNEL_SOCKET(obj) \
+OBJECT_CHECK(QIOChannelSocket, (obj), TYPE_QIO_CHANNEL_SOCKET)
+
+typedef struct QIOChannelSocket QIOChannelSocket;
+
+/**
+ * QIOChannelSocket:
+ *
+ * The QIOChannelSocket class provides a channel implementation
+ * that can transport data over a UNIX socket or TCP socket.
+ * Beyond the core channel API, it also provides functionality
+ * for accepting client connections, tuning some socket
+ * parameters and getting socket address strings.
+ */
+
+struct QIOChannelSocket {
+QIOChannel parent;
+int fd;
+struct sockaddr_storage localAddr;
+socklen_t localAddrLen;
+struct sockaddr_storage remoteAddr;
+socklen_t remoteAddrLen;
+};
+
+
+/**
+ * qio_channel_socket_new:
+ *
+ * Create a channel for performing I/O on a socket
+ * connection, that is initially closed. After
+ * creating the socket, it must be setup as a client
+ * connection or server.
+ *
+ * Returns: the socket channel object
+ */
+QIOChannelSocket *
+qio_channel_socket_new(void);
+
+/**
+ * qio_channel_socket_new_fd:
+ * @fd: the socket file descriptor
+ * @errp: pointer to an uninitialized error object
+ *
+ * Create a channel for performing I/O on the socket
+ * connection represented by the file descriptor @fd.
+ *
+ * Returns: the socket channel object, or NULL on error
+ */
+QIOChannelSocket *
+qio_channel_socket_new_fd(int fd,
+  Error **errp);
+
+
+/**
+ * qio_channel_socket_connect_sync:
+ * @ioc: the socket channel object
+ * @addr: the address to connect to
+ * @errp: pointer to an uninitialized error object
+ *
+ * Attempt to connect to the address @addr. This method
+ * will run in the foreground 

Re: [Qemu-devel] [PATCH v12 30/36] qapi: Convert QType into qapi builtin enum type

[Markus Armbruster]

Eric Blake  writes:

> What's more meta than using qapi to define qapi? :)
>
> Convert QType into a full-fledged[*] builtin qapi enum type, so
> that a subsequent patch can then use it as the discriminator
> type of qapi alternate types.  Fortunately, the judicious use of
> 'prefix' in the qapi definition avoids churn to the spelling of
> the enum constants.
>
> To avoid circular definitions, we have to flip the order of
> inclusion between "qobject.h" vs. "qapi-types.h".  Back in commit
> 28770e0, we had the latter include the former, so that we could
> use 'QObject *' for our implementation of 'any'.  But that usage
> also works with only a forward declaration, whereas the
> definition of QObject requires QType to be a complete type.

I figure putting the typedef in include/qemu/typedefs.h would be
simpler.  Didn't think of it when I reviewed v11.

>
> [*] The type has to be builtin, rather than declared in
> qapi/common.json, because we want to use it for alternates even
> when common.json is not included. But since it is the first
> builtin enum type, we have to add special cases to qapi-types
> and qapi-visit to only emit definitions once, even when two
> qapi files are being compiled into the same binary (the way we
> already handled builtin list types like 'intList').  We may
> need to revisit how multiple qapi files share common types,
> but that's a project for another day.
>
> Signed-off-by: Eric Blake 

Patch looks sane.

[Qemu-devel] [PATCH v1 2/4] char: convert from GIOChannel to QIOChannel

[Daniel P. Berrange]

In preparation for introducing TLS support to the TCP chardev
backend, convert existing chardev code from using GIOChannel
to QIOChannel. This simplifies the chardev code by removing
most of the OS platform conditional code for dealing with
file descriptor passing.

Signed-off-by: Daniel P. Berrange 
---
 qemu-char.c | 649 +++-
 1 file changed, 250 insertions(+), 399 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 59cf52f..dcb5fa3 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -32,6 +32,8 @@
 #include "qapi/qmp-input-visitor.h"
 #include "qapi/qmp-output-visitor.h"
 #include "qapi-visit.h"
+#include "io/channel-socket.h"
+#include "io/channel-file.h"
 
 #include 
 #include 
@@ -768,7 +770,7 @@ typedef struct IOWatchPoll
 {
 GSource parent;
 
-GIOChannel *channel;
+QIOChannel *ioc;
 GSource *src;
 
 IOCanReadHandler *fd_can_read;
@@ -791,8 +793,8 @@ static gboolean io_watch_poll_prepare(GSource *source, gint 
*timeout_)
 }
 
 if (now_active) {
-iwp->src = g_io_create_watch(iwp->channel,
- G_IO_IN | G_IO_ERR | G_IO_HUP | 
G_IO_NVAL);
+iwp->src = qio_channel_create_watch(
+iwp->ioc, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL);
 g_source_set_callback(iwp->src, iwp->fd_read, iwp->opaque, NULL);
 g_source_attach(iwp->src, NULL);
 } else {
@@ -838,9 +840,9 @@ static GSourceFuncs io_watch_poll_funcs = {
 };
 
 /* Can only be used for read */
-static guint io_add_watch_poll(GIOChannel *channel,
+static guint io_add_watch_poll(QIOChannel *ioc,
IOCanReadHandler *fd_can_read,
-   GIOFunc fd_read,
+   QIOChannelFunc fd_read,
gpointer user_data)
 {
 IOWatchPoll *iwp;
@@ -849,7 +851,7 @@ static guint io_add_watch_poll(GIOChannel *channel,
 iwp = (IOWatchPoll *) g_source_new(_watch_poll_funcs, 
sizeof(IOWatchPoll));
 iwp->fd_can_read = fd_can_read;
 iwp->opaque = user_data;
-iwp->channel = channel;
+iwp->ioc = ioc;
 iwp->fd_read = (GSourceFunc) fd_read;
 iwp->src = NULL;
 
@@ -885,79 +887,50 @@ static void remove_fd_in_watch(CharDriverState *chr)
 }
 }
 
-#ifndef _WIN32
-static GIOChannel *io_channel_from_fd(int fd)
-{
-GIOChannel *chan;
-
-if (fd == -1) {
-return NULL;
-}
 
-chan = g_io_channel_unix_new(fd);
-
-g_io_channel_set_encoding(chan, NULL, NULL);
-g_io_channel_set_buffered(chan, FALSE);
-
-return chan;
-}
-#endif
-
-static GIOChannel *io_channel_from_socket(int fd)
+static int io_channel_send_full(QIOChannel *ioc,
+const void *buf, size_t len,
+int *fds, size_t nfds)
 {
-GIOChannel *chan;
+size_t offset = 0;
 
-if (fd == -1) {
-return NULL;
-}
+while (offset < len) {
+ssize_t ret = 0;
+struct iovec iov = { .iov_base = (char *)buf + offset,
+ .iov_len = len - offset };
+
+ret = qio_channel_writev_full(
+ioc, , 1,
+fds, nfds, NULL);
+if (ret == QIO_CHANNEL_ERR_BLOCK) {
+errno = EAGAIN;
+return -1;
+} else if (ret < 0) {
+if (offset) {
+return offset;
+}
 
-#ifdef _WIN32
-chan = g_io_channel_win32_new_socket(fd);
-#else
-chan = g_io_channel_unix_new(fd);
-#endif
+errno = EINVAL;
+return -1;
+}
 
-g_io_channel_set_encoding(chan, NULL, NULL);
-g_io_channel_set_buffered(chan, FALSE);
+offset += ret;
+}
 
-return chan;
+return offset;
 }
 
-static int io_channel_send(GIOChannel *fd, const void *buf, size_t len)
-{
-size_t offset = 0;
-GIOStatus status = G_IO_STATUS_NORMAL;
-
-while (offset < len && status == G_IO_STATUS_NORMAL) {
-gsize bytes_written = 0;
 
-status = g_io_channel_write_chars(fd, buf + offset, len - offset,
-  _written, NULL);
-offset += bytes_written;
-}
-
-if (offset > 0) {
-return offset;
-}
-switch (status) {
-case G_IO_STATUS_NORMAL:
-g_assert(len == 0);
-return 0;
-case G_IO_STATUS_AGAIN:
-errno = EAGAIN;
-return -1;
-default:
-break;
-}
-errno = EINVAL;
-return -1;
+static int io_channel_send(QIOChannel *ioc, const void *buf, size_t len)
+{
+return io_channel_send_full(ioc, buf, len, NULL, 0);
 }
 
 #ifndef _WIN32
 
 typedef struct FDCharDriver {
 CharDriverState *chr;
-GIOChannel *fd_in, *fd_out;
+QIOChannel *ioc_in, *ioc_out;
 int max_size;
 } FDCharDriver;
 
@@ -966,17 +939,16 @@ static int fd_chr_write(CharDriverState *chr, const 
uint8_t *buf, int len)
 {
 FDCharDriver *s = chr->opaque;
 
-return 

[Qemu-devel] [PATCH v1 0/4] Convert chardevs to QIOChannel & add TLS support

[Daniel P. Berrange]

This is an update of patches previously shown in an RFC posting

  RFC: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg00829.html

This series depends on the previously posted series:

  "[PATCH v3 0/9] Introduce I/O channels framework"

This short series converts the chardev backends to use the new
QIOChannel framework. After doing so it then adds support for
TLS encryption of TCP chardevs. The commit message in the last
patch explains the TLS encryption in detail.

The GIOChannel -> QIOChannel conversion has been validated by
running the qtest framework, which indeed found a few bugs
initially which I have since fixed.

The TLS support has been tested for interoperability using
the gnutls-serv and gnutls-client programs which provide
stub TLS endpoints/clients respectively.

Daniel P. Berrange (4):
  char: remove fixed length filename allocation
  char: convert from GIOChannel to QIOChannel
  char: don't assume telnet initialization will not block
  char: introduce support for TLS encrypted TCP chardev backend

 qapi-schema.json |   2 +
 qemu-char.c  | 914 ---
 qemu-options.hx  |   9 +-
 3 files changed, 474 insertions(+), 451 deletions(-)

-- 
2.5.0

Re: [Qemu-devel] [PATCH v12 35/36] qapi: Simplify visits of optional fields

[Markus Armbruster]

Eric Blake  writes:

> None of the visitor callbacks would set an error when testing
> if an optional field was present; make this part of the interface
> contract by eliminating the errp argument.
>
> The resulting generated code has a nice diff:
>
> |-visit_optional(v, _fdset_id, "fdset-id", );
> |-if (err) {
> |-goto out;
> |-}
> |-if (has_fdset_id) {
> |+visit_optional(v, _fdset_id, "fdset-id");
> |+if (has_fdset_id) {
> | visit_type_int(v, _id, "fdset-id", );
> | if (err) {
> | goto out;
> | }
> | }

I think this should be

  |-visit_optional(v, _fdset_id, "fdset-id", );
  |-if (err) {
  |-goto out;
  |-}
  |+visit_optional(v, _fdset_id, "fdset-id");
  | if (has_fdset_id) {
  | visit_type_int(v, _id, "fdset-id", );
  | if (err) {
  | goto out;
  | }
  | }

>
> Signed-off-by: Eric Blake 

Patch looks good.

[Qemu-devel] [PATCH v1 3/3] ui: convert VNC server to use QIOChannelWebsock

[Daniel P. Berrange]

Remove custom websock handling code from the VNC server and use
the QIOChannelWebsock class instead.

Signed-off-by: Daniel P. Berrange 
---
 ui/vnc-ws.c | 328 +---
 ui/vnc-ws.h |  63 
 ui/vnc.c|  27 +
 ui/vnc.h|   4 -
 4 files changed, 30 insertions(+), 392 deletions(-)

diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
index 053beca..8018233 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -19,10 +19,7 @@
  */
 
 #include "vnc.h"
-#include "qemu/main-loop.h"
-#include "crypto/hash.h"
-
-static void vncws_handshake_read(VncState *vs);
+#include "io/channel-websock.h"
 
 static void vncws_tls_handshake_done(Object *source,
  Error *err,
@@ -34,6 +31,7 @@ static void vncws_tls_handshake_done(Object *source,
 VNC_DEBUG("Handshake failed %s\n", error_get_pretty(err));
 vnc_client_error(vs);
 } else {
+VNC_DEBUG("TLS handshake complete, starting websocket handshake\n");
 vs->ioc_tag = qio_channel_add_watch(
 QIO_CHANNEL(vs->ioc), G_IO_IN, vncws_handshake_io, vs, NULL);
 }
@@ -79,38 +77,21 @@ gboolean vncws_tls_handshake_io(QIOChannel *ioc 
G_GNUC_UNUSED,
 return TRUE;
 }
 
-static void vncws_handshake_read(VncState *vs)
-{
-uint8_t *handshake_end;
-long ret;
-/* Typical HTTP headers from novnc are 512 bytes, so limiting
- * total header size to 4096 is easily enough. */
-size_t want = 4096 - vs->ws_input.offset;
-buffer_reserve(>ws_input, want);
-ret = vnc_client_read_buf(vs, buffer_end(>ws_input), want);
 
-if (!ret) {
-if (vs->disconnecting) {
-vnc_disconnect_finish(vs);
-}
-return;
-}
-vs->ws_input.offset += ret;
+static void vncws_handshake_done(Object *source,
+ Error *err,
+ gpointer user_data)
+{
+VncState *vs = user_data;
 
-handshake_end = (uint8_t *)g_strstr_len((char *)vs->ws_input.buffer,
-vs->ws_input.offset, WS_HANDSHAKE_END);
-if (handshake_end) {
-if (vs->ioc_tag) {
-g_source_remove(vs->ioc_tag);
-}
+if (err) {
+VNC_DEBUG("Websock handshake failed %s\n", error_get_pretty(err));
+vnc_client_error(vs);
+} else {
+VNC_DEBUG("Websock handshake complete, starting VNC protocol\n");
+vnc_init_state(vs);
 vs->ioc_tag = qio_channel_add_watch(
 vs->ioc, G_IO_IN, vnc_client_io, vs, NULL);
-vncws_process_handshake(vs, vs->ws_input.buffer, vs->ws_input.offset);
-buffer_advance(>ws_input, handshake_end - vs->ws_input.buffer +
-strlen(WS_HANDSHAKE_END));
-} else if (vs->ws_input.offset >= 4096) {
-VNC_DEBUG("End of headers not found in first 4096 bytes\n");
-vnc_client_error(vs);
 }
 }
 
@@ -120,280 +101,23 @@ gboolean vncws_handshake_io(QIOChannel *ioc 
G_GNUC_UNUSED,
 void *opaque)
 {
 VncState *vs = opaque;
-vncws_handshake_read(vs);
-return TRUE;
-}
-
-long vnc_client_read_ws(VncState *vs)
-{
-int ret, err;
-uint8_t *payload;
-size_t payload_size, header_size;
-VNC_DEBUG("Read websocket %p size %zd offset %zd\n", vs->ws_input.buffer,
-vs->ws_input.capacity, vs->ws_input.offset);
-buffer_reserve(>ws_input, 4096);
-ret = vnc_client_read_buf(vs, buffer_end(>ws_input), 4096);
-if (!ret) {
-return 0;
-}
-vs->ws_input.offset += ret;
-
-ret = 0;
-/* consume as much of ws_input buffer as possible */
-do {
-if (vs->ws_payload_remain == 0) {
-err = vncws_decode_frame_header(>ws_input,
-_size,
->ws_payload_remain,
->ws_payload_mask);
-if (err <= 0) {
-return err;
-}
-
-buffer_advance(>ws_input, header_size);
-}
-if (vs->ws_payload_remain != 0) {
-err = vncws_decode_frame_payload(>ws_input,
- >ws_payload_remain,
- >ws_payload_mask,
- ,
- _size);
-if (err < 0) {
-return err;
-}
-if (err == 0) {
-return ret;
-}
-ret += err;
-
-buffer_reserve(>input, payload_size);
-buffer_append(>input, payload, payload_size);
-
-buffer_advance(>ws_input, payload_size);
-}
-} while (vs->ws_input.offset > 0);
-
-return ret;
-}
-
-long vnc_client_write_ws(VncState *vs)
-{
-long ret;
-VNC_DEBUG("Write WS: Pending output %p size %zd offset %zd\n",
-  vs->output.buffer, vs->output.capacity, 

Re: [Qemu-devel] [PATCH v12 30/36] qapi: Convert QType into qapi builtin enum type

[Markus Armbruster]

Suggest to spell it "QAPI built-in enum type" in the subject.

In general, I prefer "QAPI" when referring to the concept, because it's
actually an acronym.

Re: [Qemu-devel] [PATCH 1/2] nvme: fix identify to be NVMe 1.1 compliant

[Christoph Hellwig]

Meh, this was still missing the uncommited changes for the nsid
off by one vs the array index:

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 360be71..4f768d5 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -499,10 +499,10 @@ static uint16_t nvme_identify_nslist(NvmeCtrl *n, 
NvmeIdentify *c)
 
 list = g_malloc0(data_len);
 for (i = 0; i < n->num_namespaces; i++) {
-if (i <= min_nsid) {
+if (i < min_nsid) {
 continue;
 }
-list[j++] = cpu_to_le32(i);
+list[j++] = cpu_to_le32(i + 1);
 if (j == data_len / sizeof(uint32_t)) {
 break;
 }

[Qemu-devel] [Bug 1465935] Re: kvm_irqchip_commit_routes: Assertion `ret == 0' failed

[Serge Hallyn]

This package is causing a regression in lp:qa-regression-testing's
scripts/test-qemu.py.

I'm running the testcase one more time (after having verified that the
current package did not suffer the same failure), then I'm going to mark this
verification-failed.

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

Title:
  kvm_irqchip_commit_routes: Assertion `ret == 0' failed

Status in QEMU:
  New
Status in qemu package in Ubuntu:
  Fix Released
Status in qemu source package in Trusty:
  Fix Committed
Status in qemu source package in Utopic:
  Won't Fix
Status in qemu source package in Vivid:
  Fix Committed

Bug description:
  Several my QEMU instances crashed, and in the  qemu log, I can see
  this assertion failure,

 qemu-system-x86_64: /build/buildd/qemu-2.0.0+dfsg/kvm-all.c:984:
  kvm_irqchip_commit_routes: Assertion `ret == 0' failed.

  The QEMU version is 2.0.0, HV OS is ubuntu 12.04, kernel 3.2.0-38.
  Guest OS is RHEL 6.3.

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

Re: [Qemu-devel] RFC: raspberry pi / pi2 / Windows-on-ARM support

[Peter Crosthwaite]

On Wed, Nov 18, 2015 at 10:16 AM, Andrew Baumann
 wrote:
> Hi Peter,
>
> Thanks for your feedback!
>
> From: Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com]
> Sent: Tuesday, 17 November 2015 23:51
>> I haven't looked beyond the diffstat yet, but a top level
>> architectural comment, I only see the one file in hw/arm. We are
>> promoting the split of board and SoC now as two separate objects. Each
>> of the patch series linked above demonstrates this. The BCM SoC would
>> be an object in its own right, then the board instantiates the SoC as
>> a composite device.
>
> I understand the rationale here, but is there much value pursuing this for 
> the Pi? As far as I know, the Pi1/2 SOCs (bcm2835 and 2836) are one-off 
> designs used only in the Pi. Moreover, the 2836 is basically a copy of the 
> 2835 with the addition of a new block for a multi-core interrupt controller 
> and mailboxes, so trying to split those out into two separate SOC definition 
> files would require either lots of duplication, or a tight coupling between 
> the two.

Duplication is not right. You can code share by either having a QOM
inheritance between the two SoCs (the QOM object oriented model allows
this) from a common abstraction, or a QOM property that drives the
needed iffery. Even if the board is thin, I would still do it split,
and several of the in-tree machine models are like this. If anyone
wants to model the lower level stuff like the power supplies or play
with the GPIOs etc there some framework there. If the board ever gets
cloned and modded then the SoC is ready to go.

> Finally, there is basically nothing on the board besides the SoC and RAM 
> (just a USB-Ethernet adapter), so the actual board definition files would be 
> almost empty.

USB-Ethernet is non trivial though isn't it? I'm looking at the
schematic and my gut is the lan9152 warrants a board/SoC split.

> As it is, I've already factored out all the common devices within 
> hw/arm/raspi.c -- my feeling is that this is a cleaner way to do it for the 
> Pi.
>

You will probably find that the way to do it is covert your board as
it stands to SoC, rebadging rPi as BCM SoC then put a thin rPI board
around it that sets just the RAM initially.

>> It is big. Work of this magnitude is generally better managed as
>> multiple series (each of multiple patches). A general approach with
>> new ARM boards is the first series is the bare minimum needed to get a
>> linux boot over the UART console. This usually means any CPU
>> customisations, timer, UART, intc, SoC and boards, maybe the
>> system/power controllers (if the kernel touches them in boot) and
>> optionally network or emmc. With network you can do an NFS root (this
>> was the only option on Allwinner/cubieboard for quite some time) and
>> with emmc you can do your root=/dev/mmcblk0p2 style boot as normal. If
>> you are using existing lan9118, network first might be a clear win,
>> although emmc is more realistic. Can always do both.
>>
>> The you can follow up with the USB, DMA, AUX, ... each independently
>> or as a follow up series'.
>>
>> Any core code and ARM CPU work that is a bugfix or stands in its own
>> right, you should send straight up as a separate work.
>
> Ok, understood. Even following this guidance, the first patch will be big -- 
> Pi has custom interrupt and DMA controllers, and the GPU gets itself involved 
> in the boot process. I think we could probably leave out fb/usb and maybe 
> also dma, mphi, vchiq and power, but most of the rest will be needed to boot 
> anything.
>
> BTW, I used lan9118 as a kludge for Windows, but it isn't present on a real 
> Pi, and the device definition no longer instantiates it. At present, there is 
> no usable network device.
>

Ok I guess just start with emmc. If that diff to sd.c is for emmc card
support (currently missing), I would start early and send that
straight away.

>> Curious, does rPi kernel support device tree boot? This would be nice
>> for testing with a generic kernel.
>
> I believe it does on recent Raspbian images (I see a number of dtb files in 
> Raspbian's boot partition), but I haven't tried to figure out how to use 
> them. In general, we have limitations where the bootloader is concerned. On 
> Pi HW, the boot process is driven by the GPU, which reads a plethora of 
> options from config files (config.txt and cmdline.txt) on the boot partition. 
> We don't emulate any of that stuff... instead, the previous raspi code did 
> just enough to boot Linux using qemu's Linux loader support, and I figured 
> out how to make UEFI happy on raspi2 so it can boot Windows, but that's about 
> all.
>
> I will take a look at the patches you referred to, and try to refactor the 
> devices accordingly.
>
> BTW, my main goal here is Windows support (surprise! :), so if anyone else is 
> willing to help out with Linux that would be much appreciated. The kernel 
> currently limps into early userspace before crashing on a