Re: [Qemu-devel] [PATCH] opts: fix NULL pointer derefernce in get_opt_value

2018-07-16 Thread Daniel P. Berrange
On Mon, Jul 16, 2018 at 06:41:46PM +0100, Mike Krinkin wrote:
> The value argument can be NULL, for example, in hw/i386/multiboot.c
> in the load_multiboot function get_opt_value is explicitly called
> with NULL as the second argument.
> 
> The problem was introduced in commit 950c4e6c94b1 ("opts: don't
> silently truncate long option values"). This change fixes the
> problem by adding a check whether the value is NULL or not.
> 
> Signed-off-by: Mike Krinkin 
> ---
>  util/qemu-option.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

This problem is fixed in this:

  https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg01490.html

but it is still waiting for i386 maintainers to respond

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|



Re: [Qemu-devel] [PATCH v12 28/28] tests/qmp-test: blacklist sev specific qmp commands

2018-03-08 Thread Daniel P. Berrange
On Thu, Mar 08, 2018 at 06:45:04PM -0300, Eduardo Habkost wrote:
> On Thu, Mar 08, 2018 at 02:18:55PM -0600, Brijesh Singh wrote:
> > 
> > 
> > On 3/8/18 11:08 AM, Daniel P. Berrangé wrote:
> > > On Thu, Mar 08, 2018 at 06:49:01AM -0600, Brijesh Singh wrote:
> > >> Blacklist the following commands to fix the 'make check' failure.
> > >>
> > >> query-sev-launch-measure: it returns meaninful data only when we launch
> > >> SEV guest otherwise the command returns an error.
> > >>
> > >> query-sev: it return an error when SEV is not available on host (e.g non
> > >> X86 platform or KVM is disabled at the build time)
> > >>
> > >> query-sev-capabilities: it returns an error when SEV feature is not
> > >> available on host machine.
> > > We generally expect 'make check' to succeed on every single patch
> > > in a series, so that 'git bisect' doesn't break.
> > >
> > > So you should add each command to the blacklist in the same commit
> > > that introduced the failure in the first place.
> > 
> > 
> > Sure, I can quickly send the updated patch series to address your this
> > concern, but before spamming everyone's inbox I was wondering if I can
> > get some indication whether this series will make into 2.12 merge.
> > 
> > Paolo, Eduardo and Richard,
> > 
> > Most of the changes are in x86 directory hence any thought if you are
> > considering this series for 2.12 ? I have been testing the series with
> > and without SEV support and so far have not ran into any issue. if you
> > are not planning to pull this series in 2.12 then I will wait a bit
> > longer to get more feedback before sending the updates to address
> > Daniel's comment. thanks
> 
> Trying to merge it before 2.12 soft freeze (next Tuesday) still
> looks like a reasonable goal to me.  What do others think?

I've only really looked at the QAPI / QMP bits and they seem fine from
pov of libvirt's needs - just very minor comments. So not objection from
me on that area of the code.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|



Re: [Qemu-devel] [PATCH v1 2/2] make: fix help message reference to bogus V=0 variable

2018-01-24 Thread Daniel P. Berrange
On Tue, Jan 23, 2018 at 01:05:31PM -0600, Eric Blake wrote:
> On 01/23/2018 10:47 AM, Daniel P. Berrange wrote:
> > The make rules for building QEMU are mostly silent by default. They can
> > be made verbose by setting the variable V=1. The default state does not
> > however correspond to a V=0 setting - $(V) must be undefined / empty to
> > get the default quiet build.
> 
> Makefiles generated by automake support V=0; how hard would it be to
> instead tweak things so that 'V=' and 'V=0' have the same effect?

Well it would make the various conmditionals a bit more complex because
we'd have to check three states instead of two. I'm not really convinced
it is worth it because explicitly passing V=0 doesn't do anything useful
given that we are unconditionally silent by default.

It makes more sense with autoconf, because the default behaviour may vary
based on configure setup and/or configure args, so you can't assume that
omitting 'V=' gives you silent build like we do with QEMU.

> 
> > 
> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> > ---
> >  Makefile | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index c263190b8d..554ba69ced 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -940,4 +940,5 @@ ifdef QEMU_GA_MSI_ENABLED
> >  endif
> > @echo  ''
> >  endif
> > -   @echo  '  $(MAKE) V=0|1 [targets] 0 => quiet build (default), 1 => 
> > verbose build'
> > +   @echo  '  $(MAKE) [targets]  (quiet build, default)'
> > +   @echo  '  $(MAKE) V=1 [targets]  (verbose build)'
> > 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 




Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PULL 10/51] build-sys: silence make by default or V=0

2018-01-23 Thread Daniel P. Berrange
On Tue, Jan 23, 2018 at 05:08:08PM +0100, Marc-Andre Lureau wrote:
> Hi
> 
> On Tue, Jan 23, 2018 at 4:38 PM, Daniel P. Berrange <berra...@redhat.com> 
> wrote:
> > On Tue, Jan 16, 2018 at 03:16:52PM +0100, Paolo Bonzini wrote:
> >> From: Marc-André Lureau <marcandre.lur...@redhat.com>
> >>
> >> Move generic make flags in MAKEFLAGS (SUBDIR_MAKEFLAGS is more qemu 
> >> specific).
> >>
> >> Use --quiet to silence make 'is up to date' message.
> >>
> >> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> >> Tested-by: Eric Blake <ebl...@redhat.com>
> >> Reviewed-by: Paolo Bonzini <pbonz...@redhat.com>
> >> Message-Id: <20180104160523.22995-3-marcandre.lur...@redhat.com>
> >> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> >> ---
> >>  Makefile  | 2 +-
> >>  rules.mak | 2 ++
> >>  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > After applying it when you run 'make install' absolutely nothing is
> > displayed, but it none the less does work. This is very misleading
> > to devs who thing nothing is being installed...
> 
> Right, you would need V=1 now
> 
> > Either this needs reverting, or we need to re-write the 'install' target
> > so that it generates messages of whats being installed. Perhaps something
> > like this
> >
> 
> Make sense to me, could you send a former patch for review?

When I looked at this more, I became concerned that I would inevitably miss
places which need updating, as our makefiles as huge & have many targets
potentially affected by this. So I took the former approach, with a small
tweak to silence "is up to date" messages


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



[Qemu-devel] [PATCH v1 2/2] make: fix help message reference to bogus V=0 variable

2018-01-23 Thread Daniel P. Berrange
The make rules for building QEMU are mostly silent by default. They can
be made verbose by setting the variable V=1. The default state does not
however correspond to a V=0 setting - $(V) must be undefined / empty to
get the default quiet build.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index c263190b8d..554ba69ced 100644
--- a/Makefile
+++ b/Makefile
@@ -940,4 +940,5 @@ ifdef QEMU_GA_MSI_ENABLED
 endif
@echo  ''
 endif
-   @echo  '  $(MAKE) V=0|1 [targets] 0 => quiet build (default), 1 => 
verbose build'
+   @echo  '  $(MAKE) [targets]  (quiet build, default)'
+   @echo  '  $(MAKE) V=1 [targets]  (verbose build)'
-- 
2.14.3




[Qemu-devel] [PATCH v1 0/2] Two fixes to make rules

2018-01-23 Thread Daniel P. Berrange
The primary goal of this was to fix the recent regression that made
everything done by make completely silent, causing things like
"make install" to emit no output. In doing so I noticed a small mistake
in the help text.

Daniel P. Berrange (2):
  Revert "build-sys: silence make by default or V=0"
  make: fix help message reference to bogus V=0 variable

 Makefile  | 5 +++--
 rules.mak | 2 --
 2 files changed, 3 insertions(+), 4 deletions(-)

-- 
2.14.3




[Qemu-devel] [PATCH v1 1/2] Revert "build-sys: silence make by default or V=0"

2018-01-23 Thread Daniel P. Berrange
This reverts commit 42a77f1ce4934b243df003f95bda88530631387a.

The primary intention of this change was to silence messages
like

  make[1]: '/home/berrange/src/virt/qemu/capstone/libcapstone.a' is up to date.

which we get when calling make recursively with explicit
targets.

The problem is that this change affected every make target,
not merely the targets that triggered these "is up to date"
messages. As a result any targets that were not invoking
commands via "$(call quiet-command ...)" suddenly become
silent. This is particularly bad for "make install" which
now appears todo nothing.

Rather than go through every make rule and try to identify
places where we now need to explicitly print a message to
show work taking place, just revert the change.

To address the original problem of silencing "is up to date"
messages, we simply add --quiet to the SUBDIR_MAKEVARS
variable, so it only affects us on recursive make calls.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 Makefile  | 2 +-
 rules.mak | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index f26ef1b1df..c263190b8d 100644
--- a/Makefile
+++ b/Makefile
@@ -280,7 +280,7 @@ else
 DOCS=
 endif
 
-SUBDIR_MAKEFLAGS=BUILD_DIR=$(BUILD_DIR)
+SUBDIR_MAKEFLAGS=$(if $(V),,--no-print-directory --quiet) 
BUILD_DIR=$(BUILD_DIR)
 SUBDIR_DEVICES_MAK=$(patsubst %, %/config-devices.mak, $(TARGET_DIRS))
 SUBDIR_DEVICES_MAK_DEP=$(patsubst %, %-config-devices.mak.d, $(TARGET_DIRS))
 
diff --git a/rules.mak b/rules.mak
index 5fb4951561..6e943335f3 100644
--- a/rules.mak
+++ b/rules.mak
@@ -131,8 +131,6 @@ modules:
 # If called with only a single argument, will print nothing in quiet mode.
 quiet-command = $(if $(V),$1,$(if $(2),@printf "  %-7s %s\n" $2 $3 && $1, @$1))
 
-MAKEFLAGS += $(if $(V),,--no-print-directory --quiet)
-
 # cc-option
 # Usage: CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)
 
-- 
2.14.3




Re: [Qemu-devel] [PULL 10/51] build-sys: silence make by default or V=0

2018-01-23 Thread Daniel P. Berrange
On Tue, Jan 16, 2018 at 03:16:52PM +0100, Paolo Bonzini wrote:
> From: Marc-André Lureau 
> 
> Move generic make flags in MAKEFLAGS (SUBDIR_MAKEFLAGS is more qemu specific).
> 
> Use --quiet to silence make 'is up to date' message.
> 
> Signed-off-by: Marc-André Lureau 
> Tested-by: Eric Blake 
> Reviewed-by: Paolo Bonzini 
> Message-Id: <20180104160523.22995-3-marcandre.lur...@redhat.com>
> Signed-off-by: Paolo Bonzini 
> ---
>  Makefile  | 2 +-
>  rules.mak | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)

After applying it when you run 'make install' absolutely nothing is
displayed, but it none the less does work. This is very misleading
to devs who thing nothing is being installed...

Either this needs reverting, or we need to re-write the 'install' target
so that it generates messages of whats being installed. Perhaps something
like this

diff --git a/Makefile b/Makefile
index f26ef1b1df..8ef195a0df 100644
--- a/Makefile
+++ b/Makefile
@@ -697,28 +697,33 @@ ifneq ($(TOOLS),)
 endif
 ifneq ($(CONFIG_MODULES),)
$(INSTALL_DIR) "$(DESTDIR)$(qemu_moddir)"
+   $(call quiet-command,\
for s in $(modules-m:.mo=$(DSOSUF)); do \
t="$(DESTDIR)$(qemu_moddir)/$$(echo $$s | tr / -)"; \
$(INSTALL_LIB) $$s "$$t"; \
test -z "$(STRIP)" || $(STRIP) "$$t"; \
-   done
+   done, "INSTALL", "$(modules-m)")
 endif
 ifneq ($(HELPERS-y),)
$(call install-prog,$(HELPERS-y),$(DESTDIR)$(libexecdir))
 endif
 ifneq ($(BLOBS),)
+   $(call quiet-command,\
set -e; for x in $(BLOBS); do \
$(INSTALL_DATA) $(SRC_PATH)/pc-bios/$$x 
"$(DESTDIR)$(qemu_datadir)"; \
-   done
+   done, "INSTALL", "$(BLOBS)")
 endif
 ifeq ($(CONFIG_GTK),y)
$(MAKE) -C po $@
 endif
$(INSTALL_DIR) "$(DESTDIR)$(qemu_datadir)/keymaps"
+   $(call quiet-command,\
set -e; for x in $(KEYMAPS); do \
$(INSTALL_DATA) $(SRC_PATH)/pc-bios/keymaps/$$x 
"$(DESTDIR)$(qemu_datadir)/keymaps"; \
-   done
-   $(INSTALL_DATA) $(BUILD_DIR)/trace-events-all 
"$(DESTDIR)$(qemu_datadir)/trace-events-all"
+   done, "INSTALL", "$(KEYMAPS)")
+   $(call quiet-command,\
+   $(INSTALL_DATA) $(BUILD_DIR)/trace-events-all 
"$(DESTDIR)$(qemu_datadir)/trace-events-all",\
+   "INSTALL", "trace-events-all")
for d in $(TARGET_DIRS); do \
$(MAKE) $(SUBDIR_MAKEFLAGS) TARGET_DIR=$$d/ -C $$d $@ || exit 1 ; \
 done
diff --git a/rules.mak b/rules.mak
index 5fb4951561..cd669833bf 100644
--- a/rules.mak
+++ b/rules.mak
@@ -147,7 +147,8 @@ set-vpath = $(if $1,$(foreach 
PATTERN,$(VPATH_SUFFIXES),$(eval vpath $(PATTERN)
 # install-prog list, dir
 define install-prog
$(INSTALL_DIR) "$2"
-   $(INSTALL_PROG) $1 "$2"
+   $(call quiet-command,\
+   $(INSTALL_PROG) $1 "$2", "INSTALL", "$1")
$(if $(STRIP),$(STRIP) $(foreach T,$1,"$2/$(notdir $T)"),)
 endef
 



> 
> diff --git a/Makefile b/Makefile
> index d86ecd2..1671db3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -277,7 +277,7 @@ else
>  DOCS=
>  endif
>  
> -SUBDIR_MAKEFLAGS=$(if $(V),,--no-print-directory) BUILD_DIR=$(BUILD_DIR)
> +SUBDIR_MAKEFLAGS=BUILD_DIR=$(BUILD_DIR)
>  SUBDIR_DEVICES_MAK=$(patsubst %, %/config-devices.mak, $(TARGET_DIRS))
>  SUBDIR_DEVICES_MAK_DEP=$(patsubst %, %-config-devices.mak.d, $(TARGET_DIRS))
>  
> diff --git a/rules.mak b/rules.mak
> index 6e94333..5fb4951 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -131,6 +131,8 @@ modules:
>  # If called with only a single argument, will print nothing in quiet mode.
>  quiet-command = $(if $(V),$1,$(if $(2),@printf "  %-7s %s\n" $2 $3 && $1, 
> @$1))
>  
> +MAKEFLAGS += $(if $(V),,--no-print-directory --quiet)
> +
>  # cc-option
>  # Usage: CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)
>  

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup

2018-01-22 Thread Daniel P. Berrange
On Fri, Jan 19, 2018 at 04:18:07PM -0800, Jack Schwartz wrote:
> Hi Anatol, Daniel and Kevin.
> 
> On 01/19/18 10:36, Anatol Pomozov wrote:
> > Hello Jack
> > 
> > On Wed, Jan 17, 2018 at 12:06 PM, Jack Schwartz
> >  wrote:
> > > Hi Kevin and Anatol.
> > > 
> > > Kevin, thanks for your review.
> > > 
> > > More inline below...
> > > 
> > > On 01/15/18 07:54, Kevin Wolf wrote:
> > > > Am 21.12.2017 um 18:25 hat Jack Schwartz geschrieben:
> > > > > Properly account for the possibility of multiboot kernels with a zero
> > > > > bss_end_addr.  The Multiboot Specification, section 3.1.3 allows for
> > > > > kernels without a bss section, by allowing a zeroed bss_end_addr
> > > > > multiboot
> > > > > header field.
> > > > > 
> > > > > Do some cleanup to multiboot.c as well:
> > > > > - Remove some unused variables.
> > > > > - Use more intuitive header names when displaying fields in messages.
> > > > > - Change fprintf(stderr...) to error_report
> > > > There are some conflicts with Anatol's (CCed) multiboot series:
> > > > https://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg03003.html
> > > > 
> > > > None if these should be hard to resolve, but it would be good if you
> > > > could agree with each other whose patch series should come first, and
> > > > then the other one should be rebased on top of that.
> > > Anatol,
> > > 
> > > from my side, there are pros and cons to either patch set going in first,
> > > but advantages to either are pretty negligible.  Pro for you going first: 
> > > I
> > > can use the constants you will define in header files.  Pro for me going
> > > first: your merge should be about the same as if you went first (since my
> > > changes are small, more localized and affect only multiboot.c) and my 
> > > merge
> > > will be easier.
> > > 
> > > What are your thoughts?
> > Please move ahead with your patches. I'll rebase my changes on top of yours.
> OK.  I'm consulting with my company's legal department and waiting for their
> approvals for delivery of a test "kernel".  I'll get in touch will everyone
> once I have an answer about that.  I anticipate about a week before taking
> next steps to deliver.
> 
> Kevin and Daniel, thanks for your inputs on this issue (different
> subthread), which I have forwarded to our legal department for review.

FWIW, I don't think it needs to be your responsibility to decide this. I
think the QEMU community / maintainer taking the patches needs to decide
whether it is acceptable / desirable.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH V4 2/7] CAN bus support to connect bust to Linux host SocketCAN interface.

2018-01-19 Thread Daniel P. Berrange
On Mon, Jan 15, 2018 at 09:12:09PM -0300, Philippe Mathieu-Daudé wrote:
> On 01/15/2018 06:29 PM, Pavel Pisa wrote:
> >>> +/* open socket */
> >>> +s = socket(PF_CAN, SOCK_RAW, CAN_RAW);
> >>
> >> I never used it, but I think QEMU uses his socket API: "qemu/sockets.h"
> > 
> > The SocketCAN host connection code is Linux specific,
> > but I can switch to qemu_socket() if it is preferred.
> > But address family has to be from Linux header file anyway.
> 
> qemu_socket() sockets are heavily tested and already solve many things,
> like async I/O and error handling.

NB that's just the low level system call wrapper. All it really does is
ensure O_CLOSEXEC is set for all sockets. It should defintely be used
just for that reason alone though. 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH] allow to build with older sed

2018-01-19 Thread Daniel P. Berrange
On Fri, Jan 19, 2018 at 12:52:27AM -0700, Jan Beulich wrote:
> sed's -E option may not be supported by older distros. As there's no
> point using sed here at all, use just shell mechanisms to establish the
> variable values, starting from the stem instead of the full target.
> 
> Signed-off-by: Jan Beulich <jbeul...@suse.com>

Reviewed-by: Daniel P. Berrange <berra...@redhat.com>

> --- a/Makefile
> +++ b/Makefile
> @@ -242,8 +242,7 @@ GENERATED_FILES += $(KEYCODEMAP_FILES)
>  
>  ui/input-keymap-%.c: $(KEYCODEMAP_GEN) $(KEYCODEMAP_CSV) 
> $(SRC_PATH)/ui/Makefile.objs
>   $(call quiet-command,\
> - src=$$(echo $@ | sed -E -e 
> "s,^ui/input-keymap-(.+)-to-(.+)\.c$$,\1,") && \
> - dst=$$(echo $@ | sed -E -e 
> "s,^ui/input-keymap-(.+)-to-(.+)\.c$$,\2,") && \
> + stem=$* && src=$${stem%-to-*} dst=$${stem#*-to-} && \
>   test -e $(KEYCODEMAP_GEN) && \
>   $(PYTHON) $(KEYCODEMAP_GEN) \
> --lang glib2 \

Clever ! I was hoping someone might suggest a cleaner approach one day

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH] block: implement the bdrv_reopen_prepare helper for LUKS driver

2018-01-19 Thread Daniel P. Berrange
On Thu, Jan 18, 2018 at 01:51:36PM -0600, Eric Blake wrote:
> On 01/18/2018 04:31 AM, Daniel P. Berrange wrote:
> > If the bdrv_reopen_prepare helper isn't provided, the qemu-img commit
> > command fails to re-open the base layer after committing changes into
> > it. Provide a no-op implementation for the LUKS driver, since there
> > is not any custom work that needs doing to re-open it.
> > 
> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> > ---
> >  block/crypto.c | 7 +++
> >  1 file changed, 7 insertions(+)
> 
> I'm hoping another block-layer expert chimes in, as I'm not quite sure
> what the full reopen rules are; but the idea makes sense to me.

Yeah, likewise - it is hard to understand what is required, but I see
lots of other block drivers just put a no-op impl here. I'm relying on
Kevin/Max to tell me if i'm wrong here 

> > diff --git a/block/crypto.c b/block/crypto.c
> > index 60ddf8623e..bb9a8f5376 100644
> > --- a/block/crypto.c
> > +++ b/block/crypto.c
> > @@ -382,6 +382,12 @@ static void block_crypto_close(BlockDriverState *bs)
> >  qcrypto_block_free(crypto->block);
> >  }
> >  
> > +static int block_crypto_reopen_prepare(BDRVReopenState *state,
> > +   BlockReopenQueue *queue, Error 
> > **errp)
> > +{
> > +/* nothing needs checking */
> 
> Are we sure that even changes such as moving from read-only to
> read-write need no checking?

LUKS doesn't do anything differently with ro vs rw images, so I'm assuming
any such checks are handled by the layer below.



> 
> > +return 0;
> > +}
> >  
> >  /*
> >   * 1 MB bounce buffer gives good performance / memory tradeoff
> > @@ -620,6 +626,7 @@ BlockDriver bdrv_crypto_luks = {
> >  .bdrv_truncate  = block_crypto_truncate,
> >  .create_opts= _crypto_create_opts_luks,
> >  
> > +.bdrv_reopen_prepare = block_crypto_reopen_prepare,
> >  .bdrv_refresh_limits = block_crypto_refresh_limits,
> >  .bdrv_co_preadv = block_crypto_co_preadv,
> >  .bdrv_co_pwritev= block_crypto_co_pwritev,

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



[Qemu-devel] [PATCH] ui: avoid sign extension using client width/height

2018-01-18 Thread Daniel P. Berrange
Pixman returns a signed int for the image width/height, but the VNC
protocol only permits a unsigned int16. Effective framebuffer size
is determined by the guest, limited by the video RAM size, so the
dimensions are unlikely to exceed the range of an unsigned int16,
but this is not currently validated.

With the current use of 'int' for client width/height, the calculation
of offsets in vnc_update_throttle_offset() suffers from integer size
promotion and sign extension, causing coverity warnings

*** CID 1385147:  Integer handling issues  (SIGN_EXTENSION)
/ui/vnc.c: 979 in vnc_update_throttle_offset()
973  * than that the client would already suffering awful audio
974  * glitches, so dropping samples is no worse really).
975  */
976 static void vnc_update_throttle_offset(VncState *vs)
977 {
978 size_t offset =
>>> CID 1385147:  Integer handling issues  (SIGN_EXTENSION)
>>> Suspicious implicit sign extension:
"vs->client_pf.bytes_per_pixel" with type "unsigned char" (8 bits,
unsigned) is promoted in "vs->client_width * vs->client_height *
vs->client_pf.bytes_per_pixel" to type "int" (32 bits, signed), then
sign-extended to type "unsigned long" (64 bits, unsigned).  If
"vs->client_width * vs->client_height * vs->client_pf.bytes_per_pixel"
is greater than 0x7FFF, the upper bits of the result will all be 1.
979 vs->client_width * vs->client_height * 
vs->client_pf.bytes_per_pixel;

Change client_width / client_height to be a size_t to avoid sign
extension and integer promotion. Then validate that dimensions are in
range wrt the RFB protocol u16 limits.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 ui/vnc.c | 9 +
 ui/vnc.h | 4 ++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 665a143578..33b087221f 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -672,6 +672,11 @@ static void vnc_desktop_resize(VncState *vs)
 vs->client_height == pixman_image_get_height(vs->vd->server)) {
 return;
 }
+
+assert(pixman_image_get_width(vs->vd->server) < 65536 &&
+   pixman_image_get_width(vs->vd->server) >= 0);
+assert(pixman_image_get_height(vs->vd->server) < 65536 &&
+   pixman_image_get_height(vs->vd->server) >= 0);
 vs->client_width = pixman_image_get_width(vs->vd->server);
 vs->client_height = pixman_image_get_height(vs->vd->server);
 vnc_lock_output(vs);
@@ -2490,6 +2495,10 @@ static int protocol_client_init(VncState *vs, uint8_t 
*data, size_t len)
 return 0;
 }
 
+assert(pixman_image_get_width(vs->vd->server) < 65536 &&
+   pixman_image_get_width(vs->vd->server) >= 0);
+assert(pixman_image_get_height(vs->vd->server) < 65536 &&
+   pixman_image_get_height(vs->vd->server) >= 0);
 vs->client_width = pixman_image_get_width(vs->vd->server);
 vs->client_height = pixman_image_get_height(vs->vd->server);
 vnc_write_u16(vs, vs->client_width);
diff --git a/ui/vnc.h b/ui/vnc.h
index 0c33a5f7fe..bbda0540a7 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -278,8 +278,8 @@ struct VncState
 int last_x;
 int last_y;
 uint32_t last_bmask;
-int client_width;
-int client_height;
+size_t client_width; /* limited to u16 by RFB proto */
+size_t client_height; /* limited to u16 by RFB proto */
 VncShareMode share_mode;
 
 uint32_t vnc_encoding;
-- 
2.14.3




Re: [Qemu-devel] [PULL 10/14] ui: fix VNC client throttling when audio capture is active

2018-01-18 Thread Daniel P. Berrange
On Thu, Jan 18, 2018 at 02:54:48PM +0100, Paolo Bonzini wrote:
> On 18/01/2018 14:36, Daniel P. Berrange wrote:
> >>> +/*
> >>> + * Figure out how much pending data we should allow in the output
> >>> + * buffer before we throttle incremental display updates, and/or
> >>> + * drop audio samples.
> >>> + *
> >>> + * We allow for equiv of 1 full display's worth of FB updates,
> >>> + * and 1 second of audio samples. If audio backlog was larger
> >>> + * than that the client would already suffering awful audio
> >>> + * glitches, so dropping samples is no worse really).
> >>> + */
> >>> +static void vnc_update_throttle_offset(VncState *vs)
> >>> +{
> >>> +size_t offset =
> >>> +vs->client_width * vs->client_height * 
> >>> vs->client_pf.bytes_per_pixel;
> >> because the multiply is done with the "int" type, and then may
> >> be sign-extended when converted to the probably-64-bit unsigned
> >> size_t, resulting in the high bits all being set if the
> >> multiply ended up with a 1 in bit 31.
> > I guess we can usefully change client_width/client_height to be an unsigned
> > int, since there's no valid scenario for them to be negative.
> 
> In addition to that, do we support a >= 2 GiB framebuffer at all? (Even
> with unsigned ints, Coverity would rightly complain about a truncated
> 32-bit multiplication being assigned to a 64-bit value).

client_width/client_height are values that are initialized from the
graphics card frontend config, and thus limited by amount of video
RAM QEMU allows.   bytes_per_pixel is limited to 8/16/32.

So I think we're safe from 2GB overflow in any normal case.

That said, VGA RAM size is configurable, so I'm curious what would happen
if someone configured an insanely large VGA RAM and asked for a big frame
buffer in guest.

VNC is protocol limited to uint16 for width/height size, and so is X11
so I imagine some exploding behavour would follow :-)

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v2 0/4] QIOChannelFile bug fixes

2018-01-18 Thread Daniel P. Berrange
On Wed, Nov 01, 2017 at 02:25:22PM +, Ross Lagerwall wrote:
> Hi,
> 
> Here is a bug fix with the use of QIOChannelFile and 2 bug fixes and an
> improvement to implementation of QIOChannelFile.
> 
> Regards,
> Ross Lagerwall
> 
> Ross Lagerwall (4):
>   migration: Don't leak IO channels
>   io: Fix QIOChannelFile when creating and opening read-write
>   io: Don't call close multiple times in QIOChannelFile
>   io: Add /dev/fdset/ support to QIOChannelFile

I've queued the 3 io patches, but will leave the migration one for
the migration maintainer's queue.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v2 4/4] io: Add /dev/fdset/ support to QIOChannelFile

2018-01-18 Thread Daniel P. Berrange
On Wed, Nov 01, 2017 at 02:25:26PM +, Ross Lagerwall wrote:
> Add /dev/fdset/ support to QIOChannelFile by calling qemu_open() instead
> of open() and qemu_close() instead of close(). There is a subtle
> semantic change since qemu_open() automatically sets O_CLOEXEC, but this
> doesn't affect any of the users of the function.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerw...@citrix.com>
> ---
> Changed in v2:
> * Split into separate patch.
> 
>  io/channel-file.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrange <berra...@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v2 3/4] io: Don't call close multiple times in QIOChannelFile

2018-01-18 Thread Daniel P. Berrange
On Wed, Nov 01, 2017 at 02:25:25PM +, Ross Lagerwall wrote:
> If the file descriptor underlying QIOChannelFile is closed in the
> io_close() method, don't close it again in the finalize() method since
> the file descriptor number may have been reused in the meantime.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerw...@citrix.com>
> ---
> New in v2.
> 
>  io/channel-file.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Daniel P. Berrange <berra...@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v2 2/4] io: Fix QIOChannelFile when creating and opening read-write

2018-01-18 Thread Daniel P. Berrange
On Wed, Nov 01, 2017 at 02:25:24PM +, Ross Lagerwall wrote:
> The code wrongly passes the mode to open() only if O_WRONLY is set.
> Instead, the mode should be passed when O_CREAT is set (or O_TMPFILE on
> Linux). Fix this by always passing the mode since open() will correctly
> ignore the mode if it is not needed. Add a testcase which exercises this
> bug and also change the existing testcase to check that the mode of the
> created file is correct.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerw...@citrix.com>
> ---
> Changed in v2:
> * Separated from qemu_open() change.
> 
>  include/io/channel-file.h|  2 +-
>  io/channel-file.c|  6 +-
>  tests/test-io-channel-file.c | 29 +
>  3 files changed, 27 insertions(+), 10 deletions(-)

Reviewed-by: Daniel P. Berrange <berra...@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v2 0/4] QIOChannelFile bug fixes

2018-01-18 Thread Daniel P. Berrange
On Thu, Jan 18, 2018 at 01:40:30PM +, Ross Lagerwall wrote:
> On 11/01/2017 02:25 PM, Ross Lagerwall wrote:
> > Hi,
> > 
> > Here is a bug fix with the use of QIOChannelFile and 2 bug fixes and an
> > improvement to implementation of QIOChannelFile.
> > 
> > Regards,
> > Ross Lagerwall
> > 
> > Ross Lagerwall (4):
> >migration: Don't leak IO channels
> >io: Fix QIOChannelFile when creating and opening read-write
> >io: Don't call close multiple times in QIOChannelFile
> >io: Add /dev/fdset/ support to QIOChannelFile
> > 
> >   include/io/channel-file.h|  2 +-
> >   io/channel-file.c| 11 ---
> >   migration/savevm.c   |  2 ++
> >   tests/test-io-channel-file.c | 29 +
> >   4 files changed, 32 insertions(+), 12 deletions(-)
> > 
> 
> Ping for reviews...
> 
> v1:
> Got feedback from Daniel P. Berrange and Marc-André Lureau.
> 
> v2:
> Patch 1: Unreviewed
> Patch 2: Unreviewed
> Patch 3: Reviewed by Marc-André Lureau
> Patch 4: Reviewed by Marc-André Lureau
> 
> The patch series still applies cleanly on top of master.

Sorry my bad for forgetting this. I'll review it now.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v2 1/4] migration: Don't leak IO channels

2018-01-18 Thread Daniel P. Berrange
On Wed, Nov 01, 2017 at 02:25:23PM +, Ross Lagerwall wrote:
> Since qemu_fopen_channel_{in,out}put take references on the underlying
> IO channels, make sure to release our references to them.
> 
> Signed-off-by: Ross Lagerwall <ross.lagerw...@citrix.com>
> ---
> New in v2.
> 
>  migration/savevm.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Daniel P. Berrange <berra...@redhat.com>


> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 4a88228..87557dd 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2259,6 +2259,7 @@ void qmp_xen_save_devices_state(const char *filename, 
> Error **errp)
>  }
>  qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-save-state");
>  f = qemu_fopen_channel_output(QIO_CHANNEL(ioc));
> +object_unref(OBJECT(ioc));
>  ret = qemu_save_device_state(f);
>  qemu_fclose(f);
>  if (ret < 0) {
> @@ -2292,6 +2293,7 @@ void qmp_xen_load_devices_state(const char *filename, 
> Error **errp)
>  }
>  qio_channel_set_name(QIO_CHANNEL(ioc), "migration-xen-load-state");
>  f = qemu_fopen_channel_input(QIO_CHANNEL(ioc));
> +object_unref(OBJECT(ioc));
>  
>  ret = qemu_loadvm_state(f);
>  qemu_fclose(f);
> -- 
> 2.9.5
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PULL 10/14] ui: fix VNC client throttling when audio capture is active

2018-01-18 Thread Daniel P. Berrange
On Thu, Jan 18, 2018 at 01:29:35PM +, Peter Maydell wrote:
> On 12 January 2018 at 12:58, Gerd Hoffmann <kra...@redhat.com> wrote:
> > From: "Daniel P. Berrange" <berra...@redhat.com>
> >
> > The VNC server must throttle data sent to the client to prevent the 'output'
> > buffer size growing without bound, if the client stops reading data off the
> > socket (either maliciously or due to stalled/slow network connection).
> 
> Hi. Coverity (CID 1385147) complains about a suspicious sign extension
> in this patch:
> 
> > +/*
> > + * Figure out how much pending data we should allow in the output
> > + * buffer before we throttle incremental display updates, and/or
> > + * drop audio samples.
> > + *
> > + * We allow for equiv of 1 full display's worth of FB updates,
> > + * and 1 second of audio samples. If audio backlog was larger
> > + * than that the client would already suffering awful audio
> > + * glitches, so dropping samples is no worse really).
> > + */
> > +static void vnc_update_throttle_offset(VncState *vs)
> > +{
> > +size_t offset =
> > +vs->client_width * vs->client_height * 
> > vs->client_pf.bytes_per_pixel;
> 
> because the multiply is done with the "int" type, and then may
> be sign-extended when converted to the probably-64-bit unsigned
> size_t, resulting in the high bits all being set if the
> multiply ended up with a 1 in bit 31.

I guess we can usefully change client_width/client_height to be an unsigned
int, since there's no valid scenario for them to be negative.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH QEMU v1 0/4] multiboot: bss_end_addr can be zero / cleanup

2018-01-18 Thread Daniel P. Berrange
On Thu, Jan 18, 2018 at 12:35:00PM +0100, Kevin Wolf wrote:
> Am 17.01.2018 um 21:06 hat Jack Schwartz geschrieben:
> > Before I proceed with adding my multiboot test file, I'll clarify here that
> > I started with a version from the grub2 tree.  In that file I expanded a
> > header file, also from the same tree.  Neither file had any license header,
> > though the tree I got them from (Dated October 2017) contains the GNU GPLv3
> > license file.
> 
> I see. QEMU as a whole is GPLv2, so this might be a problem. It's
> probably not as bad as merging GPLv3 code into QEMU proper because it's
> a standalone test kernel that I suppose could have a different license.
> But IANAL and maybe it's safer not to go there.

As long as the GPLv3 code is not linked / otherwise combined with any
of the GPLv2 code in QEMU that will be ok. Since it is a self-contained
test kernel, that would not be an issue in this case - it only interfaces
to QEMU via the x86 machine ABI.

It just means that the QEMU source tar.gz will be under a conjunction
of licenses  "GPLv2 and GPLv2+ and GPLv3 and ...all our other licenss..."

The resulting binaries for emulators/tools will still be GPLv2 as they're
only linking in the GPLv2 + GPLv2+ source, never linking the GPlv3 test
image.

For simplicity of understanding though, it could be desirable to avoid
this if not an unreasonable amount of extra work

> Maybe it would be less hassle to just reimplement the tests, based on
> the MIT licensed tests that are already in tests/multiboot/.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] Do I need update the microcode of virtual machine

2018-01-18 Thread Daniel P. Berrange
On Thu, Jan 18, 2018 at 06:38:57PM +0800, Li Qiang wrote:
> Hi Paolo, all,
> 
> I have a question about the intel microcode update for spectre variant#2.
> From my understanding, there is no need to update the microcode of VMs
> because the kvm has expose the SPEC_CTL and PRED_CMD to the guest.
> Also, if we need to update the micorcode in guest, who is the vendor for
> this.
> From the hyper-v, I think I'm right.
> -->
> https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/CVE-2017-5715-and-hyper-v-vms
> 
> But upon I update the centos guest, the host kvm/qemu has been updated.
> The IBPB_ENABLED and IBRS_ENABLED are both zero if I don't update the
> microcode in the guest.
> If I update the guest micorcode, the are both 1.
>
> So I want to know, if I should update the microcode in guest.
> If the answer is Yes, then what about the Windows guest, how to update the
> microcode?

Microcode updates are only applicable to the physical CPUs seen by the
host. There is no concept of microcde for virtual CPUs in the guest. The
guest merely sees whatever CPU feature the hypervisor has permitted it to
see. IOW, as described in that microsoft link, you need to

 - Update microcode and/or firmware in host
 - Update host hypervisor software
 - Change hypervisor config for each guest to enable new CPU features
 - Update guest software (kernel)
 - Cold boot (ie fully shutoff, and then power on) the guest

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



[Qemu-devel] [PATCH] block: implement the bdrv_reopen_prepare helper for LUKS driver

2018-01-18 Thread Daniel P. Berrange
If the bdrv_reopen_prepare helper isn't provided, the qemu-img commit
command fails to re-open the base layer after committing changes into
it. Provide a no-op implementation for the LUKS driver, since there
is not any custom work that needs doing to re-open it.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 block/crypto.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/block/crypto.c b/block/crypto.c
index 60ddf8623e..bb9a8f5376 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -382,6 +382,12 @@ static void block_crypto_close(BlockDriverState *bs)
 qcrypto_block_free(crypto->block);
 }
 
+static int block_crypto_reopen_prepare(BDRVReopenState *state,
+   BlockReopenQueue *queue, Error **errp)
+{
+/* nothing needs checking */
+return 0;
+}
 
 /*
  * 1 MB bounce buffer gives good performance / memory tradeoff
@@ -620,6 +626,7 @@ BlockDriver bdrv_crypto_luks = {
 .bdrv_truncate  = block_crypto_truncate,
 .create_opts= _crypto_create_opts_luks,
 
+.bdrv_reopen_prepare = block_crypto_reopen_prepare,
 .bdrv_refresh_limits = block_crypto_refresh_limits,
 .bdrv_co_preadv = block_crypto_co_preadv,
 .bdrv_co_pwritev= block_crypto_co_pwritev,
-- 
2.14.3




Re: [Qemu-devel] [PATCH] chardev/char-socket: add POLLHUP handler

2018-01-18 Thread Daniel P. Berrange
On Thu, Jan 18, 2018 at 12:41:08PM +0300, klim wrote:
> On 01/16/2018 08:25 PM, Paolo Bonzini wrote:
> > On 10/01/2018 14:18, Klim Kireev wrote:
> > > The following behavior was observed for QEMU configured by libvirt
> > > to use guest agent as usual for the guests without virtio-serial
> > > driver (Windows or the guest remaining in BIOS stage).
> > > 
> > > In QEMU on first connect to listen character device socket
> > > the listen socket is removed from poll just after the accept().
> > > virtio_serial_guest_ready() returns 0 and the descriptor
> > > of the connected Unix socket is removed from poll and it will
> > > not be present in poll() until the guest will initialize the driver
> > > and change the state of the serial to "guest connected".
> > > 
> > > In libvirt connect() to guest agent is performed on restart and
> > > is run under VM state lock. Connect() is blocking and can
> > > wait forever.
> > > In this case libvirt can not perform ANY operation on that VM.
> > > 
> > > The bug can be easily reproduced this way:
> > > 
> > > Terminal 1:
> > > qemu-system-x86_64 -m 512 -device pci-serial,chardev=serial1 -chardev 
> > > socket,id=serial1,path=/tmp/console.sock,server,nowait
> > > (virtio-serial and isa-serial also fit)
> > > 
> > > Terminal 2:
> > > minicom -D unix\#/tmp/console.sock
> > > (type something and pres enter)
> > > C-a x (to exit)
> > > 
> > > Do 3 times:
> > > minicom -D unix\#/tmp/console.sock
> > > C-a x
> > > 
> > > It needs 4 connections, because the first one is accepted by QEMU, then 
> > > two are queued by
> > > the kernel, and the 4th blocks.
> > > 
> > > The problem is that QEMU doesn't add a read watcher after succesful read
> > > until the guest device wants to acquire recieved data, so
> > > I propose to install a separate pullhup watcher regardless of
> > > whether the device waits for data or not. After closing the connection,
> > > the guest has a capability to read the data within timeout.
> > I don't understand the timeout part.
> The timeout is important, because of following situation:
> client sends to the guest big bunch of data and closes his end of socket.
> if we just destroy the connection on the qemu's side, the guest can't read
> remaining data in channel.

Why is that a problem that needs solving ?  IMHO if the clients wants any
kind of assurance that the guest has read all the data, it should keep the
socket open and wait for a reply from the guest agent. It is totally
reasonable for the request to be dropped/partially sent if the client closes
the socket without waiting. The guest should be robust to seeing such
partial messages.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



[Qemu-devel] [PATCH v7 3/4] ui: fix alphabetical ordering of keymaps

2018-01-17 Thread Daniel P. Berrange
The qcode-to-linux keymaps was accidentally added in the wrong place
by

  commit de80d78594b4c3767a12d8d42debcf12cbf85a5b
  Author: Owen Smith <owen.sm...@citrix.com>
  Date:   Fri Nov 3 11:56:28 2017 +

ui: generate qcode to linux mappings

breaking the alphabetical ordering of keymaps

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 Makefile   | 2 +-
 include/ui/input.h | 6 +++---
 ui/input-keymap.c  | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Makefile b/Makefile
index c62e96b6c7..e3f960905f 100644
--- a/Makefile
+++ b/Makefile
@@ -236,10 +236,10 @@ KEYCODEMAP_FILES = \
 ui/input-keymap-qcode-to-atset1.c \
 ui/input-keymap-qcode-to-atset2.c \
 ui/input-keymap-qcode-to-atset3.c \
+ui/input-keymap-qcode-to-linux.c \
 ui/input-keymap-qcode-to-qnum.c \
 ui/input-keymap-qcode-to-sun.c \
 ui/input-keymap-qnum-to-qcode.c \
-ui/input-keymap-qcode-to-linux.c \
 $(NULL)
 
 GENERATED_FILES += $(KEYCODEMAP_FILES)
diff --git a/include/ui/input.h b/include/ui/input.h
index e6c9b483b0..bf3d0d1060 100644
--- a/include/ui/input.h
+++ b/include/ui/input.h
@@ -80,6 +80,9 @@ extern const guint16 qemu_input_map_qcode_to_atset2[];
 extern const guint qemu_input_map_qcode_to_atset3_len;
 extern const guint16 qemu_input_map_qcode_to_atset3[];
 
+extern const guint qemu_input_map_qcode_to_linux_len;
+extern const guint16 qemu_input_map_qcode_to_linux[];
+
 extern const guint qemu_input_map_qcode_to_qnum_len;
 extern const guint16 qemu_input_map_qcode_to_qnum[];
 
@@ -89,7 +92,4 @@ extern const guint16 qemu_input_map_qcode_to_sun[];
 extern const guint qemu_input_map_qnum_to_qcode_len;
 extern const guint16 qemu_input_map_qnum_to_qcode[];
 
-extern const guint qemu_input_map_qcode_to_linux_len;
-extern const guint16 qemu_input_map_qcode_to_linux[];
-
 #endif /* INPUT_H */
diff --git a/ui/input-keymap.c b/ui/input-keymap.c
index 32cc224e39..1f60caf314 100644
--- a/ui/input-keymap.c
+++ b/ui/input-keymap.c
@@ -9,10 +9,10 @@
 #include "ui/input-keymap-qcode-to-atset1.c"
 #include "ui/input-keymap-qcode-to-atset2.c"
 #include "ui/input-keymap-qcode-to-atset3.c"
+#include "ui/input-keymap-qcode-to-linux.c"
 #include "ui/input-keymap-qcode-to-qnum.c"
 #include "ui/input-keymap-qcode-to-sun.c"
 #include "ui/input-keymap-qnum-to-qcode.c"
-#include "ui/input-keymap-qcode-to-linux.c"
 
 int qemu_input_linux_to_qcode(unsigned int lnx)
 {
-- 
2.14.3




[Qemu-devel] [PATCH v7 2/4] ui: convert GTK and SDL1 frontends to keycodemapdb

2018-01-17 Thread Daniel P. Berrange
The x_keycode_to_pc_keycode and evdev_keycode_to_pc_keycode
tables are replaced with automatically generated tables.
In addition the X11 heuristics are improved to detect running
on XQuartz and XWin X11 servers, to activate the correct OS-X
and Win32 keycode maps.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 Makefile   |   7 ++
 include/ui/input.h |  21 +
 ui/Makefile.objs   |   5 +-
 ui/gtk.c   | 205 +--
 ui/input-keymap.c  |   7 ++
 ui/sdl.c   | 105 +++---
 ui/trace-events|   9 +-
 ui/x_keymap.c  | 250 -
 ui/x_keymap.h  |   8 +-
 9 files changed, 300 insertions(+), 317 deletions(-)

diff --git a/Makefile b/Makefile
index 7361aa46b9..182ba47bf3 100644
--- a/Makefile
+++ b/Makefile
@@ -232,11 +232,18 @@ KEYCODEMAP_GEN = 
$(SRC_PATH)/ui/keycodemapdb/tools/keymap-gen
 KEYCODEMAP_CSV = $(SRC_PATH)/ui/keycodemapdb/data/keymaps.csv
 
 KEYCODEMAP_FILES = \
+ui/input-keymap-atset1-to-qcode.c \
 ui/input-keymap-linux-to-qcode.c \
 ui/input-keymap-qcode-to-qnum.c \
 ui/input-keymap-qnum-to-qcode.c \
 ui/input-keymap-qcode-to-linux.c \
 ui/input-keymap-usb-to-qcode.c \
+ui/input-keymap-win32-to-qcode.c \
+ui/input-keymap-x11-to-qcode.c \
+ui/input-keymap-xorgevdev-to-qcode.c \
+ui/input-keymap-xorgkbd-to-qcode.c \
+ui/input-keymap-xorgxquartz-to-qcode.c \
+ui/input-keymap-xorgxwin-to-qcode.c \
 $(NULL)
 
 GENERATED_FILES += $(KEYCODEMAP_FILES)
diff --git a/include/ui/input.h b/include/ui/input.h
index 0d289e4142..05aab2db5c 100644
--- a/include/ui/input.h
+++ b/include/ui/input.h
@@ -68,6 +68,9 @@ 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);
 
+extern const guint qemu_input_map_atset1_to_qcode_len;
+extern const guint16 qemu_input_map_atset1_to_qcode[];
+
 extern const guint qemu_input_map_linux_to_qcode_len;
 extern const guint16 qemu_input_map_linux_to_qcode[];
 
@@ -83,4 +86,22 @@ extern const guint16 qemu_input_map_qcode_to_linux[];
 extern const guint qemu_input_map_usb_to_qcode_len;
 extern const guint16 qemu_input_map_usb_to_qcode[];
 
+extern const guint qemu_input_map_win32_to_qcode_len;
+extern const guint16 qemu_input_map_win32_to_qcode[];
+
+extern const guint qemu_input_map_x11_to_qcode_len;
+extern const guint16 qemu_input_map_x11_to_qcode[];
+
+extern const guint qemu_input_map_xorgevdev_to_qcode_len;
+extern const guint16 qemu_input_map_xorgevdev_to_qcode[];
+
+extern const guint qemu_input_map_xorgkbd_to_qcode_len;
+extern const guint16 qemu_input_map_xorgkbd_to_qcode[];
+
+extern const guint qemu_input_map_xorgxquartz_to_qcode_len;
+extern const guint16 qemu_input_map_xorgxquartz_to_qcode[];
+
+extern const guint qemu_input_map_xorgxwin_to_qcode_len;
+extern const guint16 qemu_input_map_xorgxwin_to_qcode[];
+
 #endif /* INPUT_H */
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index ec8533d6d9..99195884b0 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -11,11 +11,12 @@ 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_SDL) += sdl.mo
 common-obj-$(CONFIG_COCOA) += cocoa.o
 common-obj-$(CONFIG_CURSES) += curses.o
 common-obj-$(CONFIG_VNC) += $(vnc-obj-y)
-common-obj-$(CONFIG_GTK) += gtk.o x_keymap.o
+common-obj-$(CONFIG_GTK) += gtk.o
+common-obj-$(if $(CONFIG_WIN32),n,$(if $(CONFIG_SDL),y,$(CONFIG_GTK))) += 
x_keymap.o
 
 ifeq ($(CONFIG_SDLABI),1.2)
 sdl.mo-objs := sdl.o sdl_zoom.o
diff --git a/ui/gtk.c b/ui/gtk.c
index f3b7567984..1217160724 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -52,7 +52,6 @@
 #include "ui/input.h"
 #include "sysemu/sysemu.h"
 #include "qmp-commands.h"
-#include "x_keymap.h"
 #include "keymaps.h"
 #include "chardev/char.h"
 #include "qom/object.h"
@@ -65,6 +64,48 @@
 #define VC_SCALE_MIN0.25
 #define VC_SCALE_STEP   0.25
 
+#ifdef GDK_WINDOWING_X11
+#include "ui/x_keymap.h"
+
+/* Gtk2 compat */
+#ifndef GDK_IS_X11_DISPLAY
+#define GDK_IS_X11_DISPLAY(dpy) (dpy != NULL)
+#endif
+#endif
+
+
+#ifdef GDK_WINDOWING_WAYLAND
+/* Gtk2 compat */
+#ifndef GDK_IS_WAYLAND_DISPLAY
+#define GDK_IS_WAYLAND_DISPLAY(dpy) (dpy != NULL)
+#endif
+#endif
+
+
+#ifdef GDK_WINDOWING_WIN32
+/* Gtk2 compat */
+#ifndef GDK_IS_WIN32_DISPLAY
+#define GDK_IS_WIN32_DISPLAY(dpy) (dpy != NULL)
+#endif
+#endif
+
+
+#ifdef GDK_WINDOWING_BROADWAY
+/* Gtk2 compat */
+#ifndef GDK_I

[Qemu-devel] [PATCH v7 3/4] ui: add fix for GTK Pause key handling on Win32

2018-01-17 Thread Daniel P. Berrange
Versions of GTK prior to 3.22 did not correctly set the keyval
field when VK_PAUSE was received on Windows.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 ui/gtk.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 1217160724..188c40eef5 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1206,7 +1206,14 @@ static gboolean gd_key_event(GtkWidget *widget, 
GdkEventKey *key, void *opaque)
 return TRUE;
 }
 
-if (key->keyval == GDK_KEY_Pause) {
+if (key->keyval == GDK_KEY_Pause
+#ifdef G_OS_WIN32
+/* for some reason GDK does not fill keyval for VK_PAUSE
+ * See https://bugzilla.gnome.org/show_bug.cgi?id=769214
+ */
+|| key->hardware_keycode == VK_PAUSE
+#endif
+) {
 qemu_input_event_send_key_qcode(vc->gfx.dcl.con, Q_KEY_CODE_PAUSE,
 key->type == GDK_KEY_PRESS);
 return TRUE;
-- 
2.14.3




[Qemu-devel] [PATCH v7 0/4] Convert frontends to use keycodemapdb

2018-01-17 Thread Daniel P. Berrange
This is a followup to

  v1: https://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg02047.html
  v2: https://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg02471.html
  v3: https://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg02517.html
  v4: https://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg02708.html
  v5: https://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg02950.html
  v6: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg01988.html

This new series contains just the patches for converting the ui frontends
for SDL1, SDL2, and GTK to use keycodemapdb.

The particularly notable benefit is that this includes fixes such that SDL1
and GTK2/3 can actually work when using a non-Linux host Xorg/X11 server.
Previously it would display but key handling was complete garbage, making it
unusable in practice.

Changed in v7:

 - Fix syntax error in win32 hack

Daniel P. Berrange (4):
  ui: convert the SDL2 frontend to keycodemapdb
  ui: convert GTK and SDL1 frontends to keycodemapdb
  ui: add fix for GTK Pause key handling on Win32
  ui: ignore hardware keycode 255 on win32

 Makefile   |   8 ++
 include/ui/input.h |  24 +
 ui/Makefile.objs   |   5 +-
 ui/gtk.c   | 220 ++-
 ui/input-keymap.c  |   8 ++
 ui/sdl.c   | 105 +++--
 ui/sdl2-input.c|  16 +++-
 ui/sdl2-keymap.h   | 267 -
 ui/trace-events|   9 +-
 ui/x_keymap.c  | 250 +++--
 ui/x_keymap.h  |   8 +-
 11 files changed, 330 insertions(+), 590 deletions(-)
 delete mode 100644 ui/sdl2-keymap.h

-- 
2.14.3




[Qemu-devel] [PATCH v7 4/4] hw: convert virtio-input-hid device to keycodemapdb

2018-01-17 Thread Daniel P. Berrange
Replace the keymap_qcode table with automatically generated
tables.

Missing entries in keymap_qcode now fixed:

  Q_KEY_CODE_ASTERISK -> KEY_KPASTERISK
  Q_KEY_CODE_KP_MULTIPLY -> KEY_KPASTERISK
  Q_KEY_CODE_STOP -> KEY_STOP
  Q_KEY_CODE_AGAIN -> KEY_AGAIN
  Q_KEY_CODE_PROPS -> KEY_PROPS
  Q_KEY_CODE_UNDO -> KEY_UNDO
  Q_KEY_CODE_FRONT -> KEY_FRONT
  Q_KEY_CODE_COPY -> KEY_COPY
  Q_KEY_CODE_OPEN -> KEY_OPEN
  Q_KEY_CODE_PASTE -> KEY_PASTE
  Q_KEY_CODE_FIND -> KEY_FIND
  Q_KEY_CODE_CUT -> KEY_CUT
  Q_KEY_CODE_LF -> KEY_LINEFEED
  Q_KEY_CODE_HELP -> KEY_HELP
  Q_KEY_CODE_COMPOSE -> KEY_COMPOSE
  Q_KEY_CODE_RO -> KEY_RO
  Q_KEY_CODE_HIRAGANA -> KEY_HIRAGANA
  Q_KEY_CODE_HENKAN -> KEY_HENKAN
  Q_KEY_CODE_YEN -> KEY_YEN
  Q_KEY_CODE_KP_COMMA -> KEY_KPCOMMA
  Q_KEY_CODE_KP_EQUALS -> KEY_KPEQUAL
  Q_KEY_CODE_POWER -> KEY_POWER
  Q_KEY_CODE_SLEEP -> KEY_SLEEP
  Q_KEY_CODE_WAKE -> KEY_WAKEUP
  Q_KEY_CODE_AUDIONEXT -> KEY_NEXTSONG
  Q_KEY_CODE_AUDIOPREV -> KEY_PREVIOUSSONG
  Q_KEY_CODE_AUDIOSTOP -> KEY_STOPCD
  Q_KEY_CODE_AUDIOPLAY -> KEY_PLAYPAUSE
  Q_KEY_CODE_AUDIOMUTE -> KEY_MUTE
  Q_KEY_CODE_VOLUMEUP -> KEY_VOLUMEUP
  Q_KEY_CODE_VOLUMEDOWN -> KEY_VOLUMEDOWN
  Q_KEY_CODE_MEDIASELECT -> KEY_MEDIA
  Q_KEY_CODE_MAIL -> KEY_MAIL
  Q_KEY_CODE_CALCULATOR -> KEY_CALC
  Q_KEY_CODE_COMPUTER -> KEY_COMPUTER
  Q_KEY_CODE_AC_HOME -> KEY_HOMEPAGE
  Q_KEY_CODE_AC_BACK -> KEY_BACK
  Q_KEY_CODE_AC_FORWARD -> KEY_FORWARD
  Q_KEY_CODE_AC_REFRESH -> KEY_REFRESH
  Q_KEY_CODE_AC_BOOKMARKS -> KEY_BOOKMARKS

NB, the virtio-input device reports a bitmask to the guest driver that
has a bit set for each Linux keycode that the host is able to send to
the guest.

Thus by adding these extra key mappings we are technically changing the
host<->guest ABI. This would also happen any time we defined new mappings
for QEMU keycodes in future.

When a keycode is removed from the list of possible keycodes that host can
send to the guest, it means that the guest OS will think it is possible
to receive a key that in pratice can never be generated, which is harmless.

When a keycode is added to the list of possible keycodes that the host can
send to the guest, it means that the guest OS can see an unexpected event.
The Linux virtio_input.c driver code simply forwards this event to the
input_event() method in the Linux input subsystem. This in turn calls
input_handle_event(), which then calls input_get_disposition(). This method
checks if the input event is present in the permitted keys bitmap, and if
not returns INPUT_IGNORE_EVENT. Thus the unexpected event will get dropped,
which is harmless.

If the guest OS reboots, or otherwise re-initializes the virt-input device,
it will read the new keycode bitmap. No matter how many keys are defined,
the config space has a fixed 128 byte bitmap. There is, however, a size
field defiend which says how many bytes in the bitmap are used. So the guest
OS reads the size of the bitmap, and then it reads the data from bitmap upto
the designated size. So if the guest OS re-initializes at precisely the time
that QEMU is migrated across versions, in the worst case, it could conceivably
read the old size field, but then get the newly updated bitmap.  If a key were
added this is harmless, since it simply means it may not process the newly
added key. If a key were removed, then it could be readnig a byte from the
bitmap that was not initialized. Fortunately QEMU always memsets() the entire
bitmap to 0, prior to setting keybits. Thus the guest OS will simply read
zeros, which is again harmless.

Based on this analysis, it is believed that there is no need to preserve the
virtio-input-hid keymaps across migration, as the host<->guest ABI change is
harmless and self-resolving at time of guest reboot.

NB, this behaviour should perhaps be formalized in the virtio-input spec
to declare how guest OS drivers should be written to be robust in their
handling of the potentially changable key bitmaps.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 hw/input/virtio-input-hid.c | 136 +++-
 1 file changed, 9 insertions(+), 127 deletions(-)

diff --git a/hw/input/virtio-input-hid.c b/hw/input/virtio-input-hid.c
index e78faec0b1..bd89c3e6ae 100644
--- a/hw/input/virtio-input-hid.c
+++ b/hw/input/virtio-input-hid.c
@@ -22,126 +22,7 @@
 
 /* - */
 
-static const unsigned int keymap_qcode[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,
-

[Qemu-devel] [PATCH v7 1/4] hw: convert ps2 device to keycodemapdb

2018-01-17 Thread Daniel P. Berrange
Replace the qcode_to_keycode_set1, qcode_to_keycode_set2,
and qcode_to_keycode_set3 tables with automatically
generated tables.

Missing entries in qcode_to_keycode_set1 now fixed:

 - Q_KEY_CODE_SYSRQ -> 0x54
 - Q_KEY_CODE_PRINT -> 0x54 (NB ignored due to special case)
 - Q_KEY_CODE_AGAIN -> 0xe005
 - Q_KEY_CODE_PROPS -> 0xe006
 - Q_KEY_CODE_UNDO -> 0xe007
 - Q_KEY_CODE_FRONT -> 0xe00c
 - Q_KEY_CODE_COPY -> 0xe078
 - Q_KEY_CODE_OPEN -> 0x64
 - Q_KEY_CODE_PASTE -> 0x65
 - Q_KEY_CODE_CUT -> 0xe03c
 - Q_KEY_CODE_LF -> 0x5b
 - Q_KEY_CODE_HELP -> 0xe075
 - Q_KEY_CODE_COMPOSE -> 0xe05d
 - Q_KEY_CODE_PAUSE -> 0xe046
 - Q_KEY_CODE_KP_EQUALS -> 0x59

And some mistakes corrected:

 - Q_KEY_CODE_HIRAGANA was mapped to 0x70 (Katakanahiragana)
   instead of of 0x77 (Hirigana)
 - Q_KEY_CODE_MENU was incorrectly mapped to the compose
   scancode (0xe05d) and is now mapped to 0xe01e
 - Q_KEY_CODE_FIND was mapped to 0xe065 (Search) instead
   of to 0xe041 (Find)
 - Q_KEY_CODE_POWER, SLEEP & WAKE had 0x0e instead of 0xe0
   as the prefix

Missing entries in qcode_to_keycode_set2 now fixed:

 - Q_KEY_CODE_PRINT -> 0x7f (NB ignored due to special case)
 - Q_KEY_CODE_COMPOSE -> 0xe02f
 - Q_KEY_CODE_PAUSE -> 0xe077
 - Q_KEY_CODE_KP_EQUALS -> 0x0f

And some mistakes corrected:

 - Q_KEY_CODE_HIRAGANA was mapped to 0x13 (Katakanahiragana)
   instead of of 0x62 (Hirigana)
 - Q_KEY_CODE_MENU was incorrectly mapped to the compose
   scancode (0xe02f) and is now not mapped
 - Q_KEY_CODE_FIND was mapped to 0xe010 (Search) and is now
   not mapped.
 - Q_KEY_CODE_POWER, SLEEP & WAKE had 0x0e instead of 0xe0
   as the prefix

Missing entries in qcode_to_keycode_set3 now fixed:

 - Q_KEY_CODE_ASTERISK -> 0x7e
 - Q_KEY_CODE_SYSRQ -> 0x57
 - Q_KEY_CODE_LESS -> 0x13
 - Q_KEY_CODE_STOP -> 0x0a
 - Q_KEY_CODE_AGAIN -> 0x0b
 - Q_KEY_CODE_PROPS -> 0x0c
 - Q_KEY_CODE_UNDO -> 0x10
 - Q_KEY_CODE_COPY -> 0x18
 - Q_KEY_CODE_OPEN -> 0x20
 - Q_KEY_CODE_PASTE -> 0x28
 - Q_KEY_CODE_FIND -> 0x30
 - Q_KEY_CODE_CUT -> 0x38
 - Q_KEY_CODE_HELP -> 0x09
 - Q_KEY_CODE_COMPOSE -> 0x8d
 - Q_KEY_CODE_AUDIONEXT -> 0x93
 - Q_KEY_CODE_AUDIOPREV -> 0x94
 - Q_KEY_CODE_AUDIOSTOP -> 0x98
 - Q_KEY_CODE_AUDIOMUTE -> 0x9c
 - Q_KEY_CODE_VOLUMEUP -> 0x95
 - Q_KEY_CODE_VOLUMEDOWN -> 0x9d
 - Q_KEY_CODE_CALCULATOR -> 0xa3
 - Q_KEY_CODE_AC_HOME -> 0x97

And some mistakes corrected:

 - Q_KEY_CODE_MENU was incorrectly mapped to the compose
   scancode (0x8d) and is now 0x91

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 Makefile   |   3 +
 hw/input/ps2.c | 406 +
 include/ui/input.h |   9 ++
 ui/input-keymap.c  |   3 +
 4 files changed, 22 insertions(+), 399 deletions(-)

diff --git a/Makefile b/Makefile
index f26ef1b1df..2ed9db770c 100644
--- a/Makefile
+++ b/Makefile
@@ -233,6 +233,9 @@ KEYCODEMAP_CSV = 
$(SRC_PATH)/ui/keycodemapdb/data/keymaps.csv
 
 KEYCODEMAP_FILES = \
 ui/input-keymap-linux-to-qcode.c \
+ui/input-keymap-qcode-to-atset1.c \
+ui/input-keymap-qcode-to-atset2.c \
+ui/input-keymap-qcode-to-atset3.c \
 ui/input-keymap-qcode-to-qnum.c \
 ui/input-keymap-qnum-to-qcode.c \
 ui/input-keymap-qcode-to-linux.c \
diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index f388a23c8e..9eca6a4356 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -124,401 +124,6 @@ typedef struct {
 uint8_t mouse_buttons;
 } PS2MouseState;
 
-/* Table to convert from QEMU codes to scancodes.  */
-static const uint16_t qcode_to_keycode_set1[Q_KEY_CODE__MAX] = {
-[0 ... Q_KEY_CODE__MAX - 1] = 0,
-
-[Q_KEY_CODE_A] = 0x1e,
-[Q_KEY_CODE_B] = 0x30,
-[Q_KEY_CODE_C] = 0x2e,
-[Q_KEY_CODE_D] = 0x20,
-[Q_KEY_CODE_E] = 0x12,
-[Q_KEY_CODE_F] = 0x21,
-[Q_KEY_CODE_G] = 0x22,
-[Q_KEY_CODE_H] = 0x23,
-[Q_KEY_CODE_I] = 0x17,
-[Q_KEY_CODE_J] = 0x24,
-[Q_KEY_CODE_K] = 0x25,
-[Q_KEY_CODE_L] = 0x26,
-[Q_KEY_CODE_M] = 0x32,
-[Q_KEY_CODE_N] = 0x31,
-[Q_KEY_CODE_O] = 0x18,
-[Q_KEY_CODE_P] = 0x19,
-[Q_KEY_CODE_Q] = 0x10,
-[Q_KEY_CODE_R] = 0x13,
-[Q_KEY_CODE_S] = 0x1f,
-[Q_KEY_CODE_T] = 0x14,
-[Q_KEY_CODE_U] = 0x16,
-[Q_KEY_CODE_V] = 0x2f,
-[Q_KEY_CODE_W] = 0x11,
-[Q_KEY_CODE_X] = 0x2d,
-[Q_KEY_CODE_Y] = 0x15,
-[Q_KEY_CODE_Z] = 0x2c,
-[Q_KEY_CODE_0] = 0x0b,
-[Q_KEY_CODE_1] = 0x02,
-[Q_KEY_CODE_2] = 0x03,
-[Q_KEY_CODE_3] = 0x04,
-[Q_KEY_CODE_4] = 0x05,
-[Q_KEY_CODE_5] = 0x06,
-[Q_KEY_CODE_6] = 0x07,
-[Q_KEY_CODE_7] = 0x08,
-[Q_KEY_CODE_8] = 0x09,
-[Q_KEY_CODE_9] = 0x0a,
-[Q_KEY_CODE_GRAVE_ACCENT] = 0x29,
-[Q_KEY_CODE_MINUS] = 0x0c,
-[Q_KEY_CODE_EQUAL] = 0x0d,
-[Q_KEY_CODE_BACKSLASH] = 0x2b,
-[Q_KEY_CODE_BACKSPACE] = 0x0e,
-[Q_KEY_

[Qemu-devel] [PATCH v7 0/4] Convert hw backends to use keycodemapdb

2018-01-17 Thread Daniel P. Berrange
This is a followup to

  v1: https://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg02047.html
  v2: https://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg02471.html
  v3: https://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg02517.html
  v4: https://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg02708.html
  v5: https://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg02950.html
  v6: https://lists.nongnu.org/archive/html/qemu-devel/2017-12/msg01995.html

This new series contains just the patches for converting the hardware
backends to use keycodemapdb.

The main benefit is adding various missing key mappings that were previously
accidentally left out

Changed in v7:

  - Drop Xen patch as equiv already merged
  - Add patch to fix keycodemap ordering in source

Daniel P. Berrange (4):
  hw: convert ps2 device to keycodemapdb
  hw: convert the escc device to keycodemapdb
  ui: fix alphabetical ordering of keymaps
  hw: convert virtio-input-hid device to keycodemapdb

 Makefile|   6 +-
 hw/char/escc.c  | 126 +-
 hw/input/ps2.c  | 406 +---
 hw/input/virtio-input-hid.c | 136 +--
 include/ui/input.h  |  18 +-
 ui/input-keymap.c   |   6 +-
 6 files changed, 46 insertions(+), 652 deletions(-)

-- 
2.14.3




[Qemu-devel] [PATCH v7 1/4] ui: convert the SDL2 frontend to keycodemapdb

2018-01-17 Thread Daniel P. Berrange
The SDL2 scancodes are conveniently identical to the USB
scancodes. Replace the sdl2_scancode_to_qcode table with
an automatically generated table.

Missing entries in sdl2_scancode_to_qcode now fixed:

  - 0x32 -> Q_KEY_CODE_BACKSLASH
  - 0x66 -> Q_KEY_CODE_POWER
  - 0x67 -> Q_KEY_CODE_KP_EQUALS
  - 0x74 -> Q_KEY_CODE_OPEN
  - 0x77 -> Q_KEY_CODE_FRONT
  - 0x7f -> Q_KEY_CODE_AUDIOMUTE
  - 0x80 -> Q_KEY_CODE_VOLUMEUP
  - 0x81 -> Q_KEY_CODE_VOLUMEDOWN
  - 0x85 -> Q_KEY_CODE_KP_COMMA
  - 0x87 -> Q_KEY_CODE_RO
  - 0x89 -> Q_KEY_CODE_YEN
  - 0x8a -> Q_KEY_CODE_HENKAN
  - 0x93 -> Q_KEY_CODE_HIRAGANA
  - 0xe8 -> Q_KEY_CODE_AUDIOPLAY
  - 0xe9 -> Q_KEY_CODE_AUDIOSTOP
  - 0xea -> Q_KEY_CODE_AUDIOPREV
  - 0xeb -> Q_KEY_CODE_AUDIONEXT
  - 0xed -> Q_KEY_CODE_VOLUMEUP
  - 0xee -> Q_KEY_CODE_VOLUMEDOWN
  - 0xef -> Q_KEY_CODE_AUDIOMUTE
  - 0xf1 -> Q_KEY_CODE_AC_BACK
  - 0xf2 -> Q_KEY_CODE_AC_FORWARD
  - 0xf3 -> Q_KEY_CODE_STOP
  - 0xf4 -> Q_KEY_CODE_FIND
  - 0xf8 -> Q_KEY_CODE_SLEEP
  - 0xfa -> Q_KEY_CODE_AC_REFRESH
  - 0xfb -> Q_KEY_CODE_CALCULATOR

And some mistakes corrected:

  - 0x65 -> Q_KEY_CODE_COMPOSE, not duplicating Q_KEY_CODE_MENU

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 Makefile   |   1 +
 include/ui/input.h |   3 +
 ui/input-keymap.c  |   1 +
 ui/sdl2-input.c|  16 +++-
 ui/sdl2-keymap.h   | 267 -
 5 files changed, 16 insertions(+), 272 deletions(-)
 delete mode 100644 ui/sdl2-keymap.h

diff --git a/Makefile b/Makefile
index f26ef1b1df..7361aa46b9 100644
--- a/Makefile
+++ b/Makefile
@@ -236,6 +236,7 @@ KEYCODEMAP_FILES = \
 ui/input-keymap-qcode-to-qnum.c \
 ui/input-keymap-qnum-to-qcode.c \
 ui/input-keymap-qcode-to-linux.c \
+ui/input-keymap-usb-to-qcode.c \
 $(NULL)
 
 GENERATED_FILES += $(KEYCODEMAP_FILES)
diff --git a/include/ui/input.h b/include/ui/input.h
index 5cc76d6e41..0d289e4142 100644
--- a/include/ui/input.h
+++ b/include/ui/input.h
@@ -80,4 +80,7 @@ extern const guint16 qemu_input_map_qnum_to_qcode[];
 extern const guint qemu_input_map_qcode_to_linux_len;
 extern const guint16 qemu_input_map_qcode_to_linux[];
 
+extern const guint qemu_input_map_usb_to_qcode_len;
+extern const guint16 qemu_input_map_usb_to_qcode[];
+
 #endif /* INPUT_H */
diff --git a/ui/input-keymap.c b/ui/input-keymap.c
index 663986a17b..3be6dedea5 100644
--- a/ui/input-keymap.c
+++ b/ui/input-keymap.c
@@ -9,6 +9,7 @@
 #include "ui/input-keymap-qcode-to-qnum.c"
 #include "ui/input-keymap-qnum-to-qcode.c"
 #include "ui/input-keymap-qcode-to-linux.c"
+#include "ui/input-keymap-usb-to-qcode.c"
 
 int qemu_input_linux_to_qcode(unsigned int lnx)
 {
diff --git a/ui/sdl2-input.c b/ui/sdl2-input.c
index 6e315ae800..605d781971 100644
--- a/ui/sdl2-input.c
+++ b/ui/sdl2-input.c
@@ -30,8 +30,6 @@
 #include "ui/sdl2.h"
 #include "sysemu/sysemu.h"
 
-#include "sdl2-keymap.h"
-
 static uint8_t modifiers_state[SDL_NUM_SCANCODES];
 
 void sdl2_reset_keys(struct sdl2_console *scon)
@@ -39,9 +37,11 @@ void sdl2_reset_keys(struct sdl2_console *scon)
 QemuConsole *con = scon ? scon->dcl.con : NULL;
 int i;
 
-for (i = 0; i < SDL_NUM_SCANCODES; i++) {
+for (i = 0 ;
+ i < SDL_NUM_SCANCODES && i < qemu_input_map_usb_to_qcode_len ;
+ i++) {
 if (modifiers_state[i]) {
-int qcode = sdl2_scancode_to_qcode[i];
+int qcode = qemu_input_map_usb_to_qcode[i];
 qemu_input_event_send_key_qcode(con, qcode, false);
 modifiers_state[i] = 0;
 }
@@ -51,9 +51,15 @@ void sdl2_reset_keys(struct sdl2_console *scon)
 void sdl2_process_key(struct sdl2_console *scon,
   SDL_KeyboardEvent *ev)
 {
-int qcode = sdl2_scancode_to_qcode[ev->keysym.scancode];
+int qcode;
 QemuConsole *con = scon ? scon->dcl.con : NULL;
 
+if (ev->keysym.scancode >= qemu_input_map_usb_to_qcode_len) {
+return;
+}
+
+qcode = qemu_input_map_usb_to_qcode[ev->keysym.scancode];
+
 if (!qemu_console_is_graphic(con)) {
 if (ev->type == SDL_KEYDOWN) {
 switch (ev->keysym.scancode) {
diff --git a/ui/sdl2-keymap.h b/ui/sdl2-keymap.h
deleted file mode 100644
index cbedaa477d..00
--- a/ui/sdl2-keymap.h
+++ /dev/null
@@ -1,267 +0,0 @@
-
-/* map SDL2 scancodes to QKeyCode */
-
-static const int sdl2_scancode_to_qcode[SDL_NUM_SCANCODES] = {
-[SDL_SCANCODE_A] = Q_KEY_CODE_A,
-[SDL_SCANCODE_B] = Q_KEY_CODE_B,
-[SDL_SCANCODE_C] = Q_KEY_CODE_C,
-[SDL_SCANCODE_D] = Q_KEY_CODE_D,
-[SDL_SCANCODE_E] = Q_KEY_CODE_E,
-[SDL_SCANCODE_F] = Q_KEY_CODE_F,
-[SDL_SCA

[Qemu-devel] [PATCH v7 4/4] ui: ignore hardware keycode 255 on win32

2018-01-17 Thread Daniel P. Berrange
It is a reserved value and doesn't have a corresponding
valid scancode.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 ui/gtk.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index 188c40eef5..f0ad63e431 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1206,6 +1206,12 @@ static gboolean gd_key_event(GtkWidget *widget, 
GdkEventKey *key, void *opaque)
 return TRUE;
 }
 
+#ifdef WIN32
+/* on windows, we ought to ignore the reserved key event? */
+if (key->hardware_keycode == 0xff)
+return false;
+#endif
+
 if (key->keyval == GDK_KEY_Pause
 #ifdef G_OS_WIN32
 /* for some reason GDK does not fill keyval for VK_PAUSE
-- 
2.14.3




[Qemu-devel] [PATCH v7 2/4] hw: convert the escc device to keycodemapdb

2018-01-17 Thread Daniel P. Berrange
Replace the qcode_to_keycode table with automatically
generated tables.

Missing entries in qcode_to_keycode now fixed:

 - Q_KEY_CODE_KP_COMMA -> 0x2d

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 Makefile   |   1 +
 hw/char/escc.c | 126 +++--
 include/ui/input.h |   3 ++
 ui/input-keymap.c  |   1 +
 4 files changed, 10 insertions(+), 121 deletions(-)

diff --git a/Makefile b/Makefile
index 2ed9db770c..c62e96b6c7 100644
--- a/Makefile
+++ b/Makefile
@@ -237,6 +237,7 @@ KEYCODEMAP_FILES = \
 ui/input-keymap-qcode-to-atset2.c \
 ui/input-keymap-qcode-to-atset3.c \
 ui/input-keymap-qcode-to-qnum.c \
+ui/input-keymap-qcode-to-sun.c \
 ui/input-keymap-qnum-to-qcode.c \
 ui/input-keymap-qcode-to-linux.c \
 $(NULL)
diff --git a/hw/char/escc.c b/hw/char/escc.c
index 3ab831a6a7..449bf2fc63 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -717,126 +717,6 @@ MemoryRegion *escc_init(hwaddr base, qemu_irq irqA, 
qemu_irq irqB,
 return >mmio;
 }
 
-static const uint8_t qcode_to_keycode[Q_KEY_CODE__MAX] = {
-[Q_KEY_CODE_SHIFT] = 99,
-[Q_KEY_CODE_SHIFT_R]   = 110,
-[Q_KEY_CODE_ALT]   = 19,
-[Q_KEY_CODE_ALT_R] = 13,
-[Q_KEY_CODE_CTRL]  = 76,
-[Q_KEY_CODE_CTRL_R]= 76,
-[Q_KEY_CODE_ESC]   = 29,
-[Q_KEY_CODE_1] = 30,
-[Q_KEY_CODE_2] = 31,
-[Q_KEY_CODE_3] = 32,
-[Q_KEY_CODE_4] = 33,
-[Q_KEY_CODE_5] = 34,
-[Q_KEY_CODE_6] = 35,
-[Q_KEY_CODE_7] = 36,
-[Q_KEY_CODE_8] = 37,
-[Q_KEY_CODE_9] = 38,
-[Q_KEY_CODE_0] = 39,
-[Q_KEY_CODE_MINUS] = 40,
-[Q_KEY_CODE_EQUAL] = 41,
-[Q_KEY_CODE_BACKSPACE] = 43,
-[Q_KEY_CODE_TAB]   = 53,
-[Q_KEY_CODE_Q] = 54,
-[Q_KEY_CODE_W] = 55,
-[Q_KEY_CODE_E] = 56,
-[Q_KEY_CODE_R] = 57,
-[Q_KEY_CODE_T] = 58,
-[Q_KEY_CODE_Y] = 59,
-[Q_KEY_CODE_U] = 60,
-[Q_KEY_CODE_I] = 61,
-[Q_KEY_CODE_O] = 62,
-[Q_KEY_CODE_P] = 63,
-[Q_KEY_CODE_BRACKET_LEFT]  = 64,
-[Q_KEY_CODE_BRACKET_RIGHT] = 65,
-[Q_KEY_CODE_RET]   = 89,
-[Q_KEY_CODE_A] = 77,
-[Q_KEY_CODE_S] = 78,
-[Q_KEY_CODE_D] = 79,
-[Q_KEY_CODE_F] = 80,
-[Q_KEY_CODE_G] = 81,
-[Q_KEY_CODE_H] = 82,
-[Q_KEY_CODE_J] = 83,
-[Q_KEY_CODE_K] = 84,
-[Q_KEY_CODE_L] = 85,
-[Q_KEY_CODE_SEMICOLON] = 86,
-[Q_KEY_CODE_APOSTROPHE]= 87,
-[Q_KEY_CODE_GRAVE_ACCENT]  = 42,
-[Q_KEY_CODE_BACKSLASH] = 88,
-[Q_KEY_CODE_Z] = 100,
-[Q_KEY_CODE_X] = 101,
-[Q_KEY_CODE_C] = 102,
-[Q_KEY_CODE_V] = 103,
-[Q_KEY_CODE_B] = 104,
-[Q_KEY_CODE_N] = 105,
-[Q_KEY_CODE_M] = 106,
-[Q_KEY_CODE_COMMA] = 107,
-[Q_KEY_CODE_DOT]   = 108,
-[Q_KEY_CODE_SLASH] = 109,
-[Q_KEY_CODE_ASTERISK]  = 47,
-[Q_KEY_CODE_SPC]   = 121,
-[Q_KEY_CODE_CAPS_LOCK] = 119,
-[Q_KEY_CODE_F1]= 5,
-[Q_KEY_CODE_F2]= 6,
-[Q_KEY_CODE_F3]= 8,
-[Q_KEY_CODE_F4]= 10,
-[Q_KEY_CODE_F5]= 12,
-[Q_KEY_CODE_F6]= 14,
-[Q_KEY_CODE_F7]= 16,
-[Q_KEY_CODE_F8]= 17,
-[Q_KEY_CODE_F9]= 18,
-[Q_KEY_CODE_F10]   = 7,
-[Q_KEY_CODE_NUM_LOCK]  = 98,
-[Q_KEY_CODE_SCROLL_LOCK]   = 23,
-[Q_KEY_CODE_KP_DIVIDE] = 46,
-[Q_KEY_CODE_KP_MULTIPLY]   = 47,
-[Q_KEY_CODE_KP_SUBTRACT]   = 71,
-[Q_KEY_CODE_KP_ADD]= 125,
-[Q_KEY_CODE_KP_ENTER]  = 90,
-[Q_KEY_CODE_KP_DECIMAL]= 50,
-[Q_KEY_CODE_KP_0]  = 94,
-[Q_KEY_CODE_KP_1]  = 112,
-[Q_KEY_CODE_KP_2]  = 113,
-[Q_KEY_CODE_KP_3]  = 114,
-[Q_KEY_CODE_KP_4]  = 91,
-[Q_KEY_CODE_KP_5]  = 92,
-[Q_KEY_CODE_KP_6]  = 93,
-[Q_KEY_CODE_KP_7]  = 68,
-[Q_KEY_CODE_KP_8]  = 69,
-[Q_KEY_CODE_KP_9]  = 70,
-[Q_KEY_CODE_LESS]  = 124,
-[Q_KEY_CODE_F11]   = 9,
-[Q_KEY_CODE_F12]   = 11,
-[Q_KEY_CODE_HOME]  = 52,
-[Q_KEY_CODE_PGUP]  = 96,
-[Q_KEY_CODE_PGDN]  = 123,
-[Q_KEY_CODE_END]   = 74,
-[Q_KEY_CODE_LEFT]  = 24,
-[Q_KEY_CODE_UP]= 20,
-[Q_KEY_CODE_DOWN]  = 27,
-[Q_KEY_CODE_RIGHT] 

Re: [Qemu-devel] [RFC PATCH 1/3] compiler: add QEMU_WARN_NONNULL_ARGS()

2018-01-17 Thread Daniel P. Berrange
On Wed, Jan 17, 2018 at 11:33:34AM -0300, Philippe Mathieu-Daudé wrote:
> On 01/17/2018 10:32 AM, Daniel P. Berrange wrote:
> > On Wed, Jan 17, 2018 at 10:18:19AM -0300, Philippe Mathieu-Daudé wrote:
> >> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> >> ---
> >>  include/qemu/compiler.h | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> >> index 340e5fdc09..d9b2489391 100644
> >> --- a/include/qemu/compiler.h
> >> +++ b/include/qemu/compiler.h
> >> @@ -26,6 +26,8 @@
> >>  
> >>  #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
> >>  
> >> +#define QEMU_WARN_NONNULL_ARGS(args...) __attribute__((nonnull(args)))
> > 
> > If we take this, it should come with a warning attached to it, because
> > it has really nasty behaviour with GCC. Consider code like:
> > 
> >   void foo(void *bar) __attribute__((nonnull(1)));
> > 
> >   ...
> > 
> >   void foo(void *bar) { if (!bar) return; }
> > 
> > GCC may or may not warn you about passing NULL for the 'bar'
> > parameter, but it will none the less assume nothing passes
> > NULL, and thus remove the 'if (!bar)' conditional during
> > optimization. IOW, adding nonnull annotations can actually
> > make your code less robust :-(
> 
> TIL!
> 
> > After having a number of crashes in libvirt caused by gcc
> > optimizing out checks for NULL, we now only define nonnull
> > when running under static analysis (coverity) and not when
> > compiling normally.
> > 
> > https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/internal.h;h=5895030415968d72200599e8a59bbf01ffc2d5a3;hb=HEAD#l162
> 
> Why do you use __attribute__(()) ? Isn't this enough:

No idea offhand - Eric wrote this so perhaps he had a reason for that
else branch style.

> 
> #if defined __clang_analyzer__ || defined __COVERITY__
> #define QEMU_STATIC_ANALYSIS 1
> +#define QEMU_WARN_NONNULL_ARGS(args...) __attribute__((nonnull(args)))
> +#else
> +#define QEMU_WARN_NONNULL_ARGS(args...)
> #endif
> 
> > The 2 functions you've added nonnull attrs to look safe enough,
> > but people might unwittingly use this elsewhere in QEMU in future
> > not realizing the side-effect it has.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH 0/7] docker: update Ubuntu and Fedora images, deprecate old ones

2018-01-17 Thread Daniel P. Berrange
On Wed, Jan 17, 2018 at 10:58:21AM -0300, Philippe Mathieu-Daudé wrote:
> On 01/17/2018 07:25 AM, Daniel P. Berrange wrote:
> > On Wed, Jan 17, 2018 at 10:26:36AM +0800, Fam Zheng wrote:
> >> On 01/12/2018 08:49 PM, Philippe Mathieu-Daudé wrote:
> >>> Hi,
> >>>
> >>> This series is to be clearer about which upstream version we are using.
> >>>
> >>> All "FROM distrib:latest" entries have now been removed and replaced by
> >>> explicit "FROM distrib:version" ones.
> >>>
> >>> To keep backward compatibility, a warning is displayed to the user,
> >>> suggesting which correct base image to use.
> >>>
> >>> To be consistent, we remove the deprecated images of the "make docker" 
> >>> output.
> >>
> >> Changing image names when a new release comes out is not maintainable for
> >> Fedora and Ubuntu because of the fast pace. Therefore I prefer Paolo's
> >> simple patch.
> > 
> > The flipside is that if we release QEMU 2.12 with fedora:27, then for as  
> > long
> > as the 2.12 stable branch exists, dockers tests run on that branch will 
> > have a
> > consistent target and will be unlikely to break.  If we use fedora:latest,
> > then every time a new Fedora comes out there's non-trivial chance that 
> > docker
> > builds on all the stable branches will all break due to some new glibc 
> > change.
> > Sticking with explicit named versions is a big win in this respect IMHO.
> 
> What happened to me twice is checking an older QEMU tag while bisecting
> and being unable to run 'make docker-test' because the older QEMU tag
> use 'latest' which is not the current 'latest' from the Docker library,
> and I had to manually change the tag to the version 'latest' was
> pointing to at the time of the QEMU tag release.
> 
> On each stable release we need to freeze the current tag in a new
> Dockerfile, and keep the generic Dockerfile following the 'latest' tag.
> 
> Once tagged different that 'latest' the Dockerfile isn't suppose to
> change. So we only maintain the 'latest' version.
> 
> Fam/Alex/Daniel do you agree with this approach?

I don't see a benefit to that as opposed to just updating git master
to the most recent Fedora stable release every 6 months. In fact it is
more work, because it'll require special action by whomever actually
makes the releases.

Even in git master the use of 'latest' is causing pain. A patch series
of mine passed when i run it in docker because it pulled down fresh
f26 image to my machine, but it failed in patchew because patchew had
latest resolving to a previously cached f25 image. Using explicit
versions all the time will avoid this kind of caching problem.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [RFC PATCH 1/3] compiler: add QEMU_WARN_NONNULL_ARGS()

2018-01-17 Thread Daniel P. Berrange
On Wed, Jan 17, 2018 at 10:18:19AM -0300, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  include/qemu/compiler.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> index 340e5fdc09..d9b2489391 100644
> --- a/include/qemu/compiler.h
> +++ b/include/qemu/compiler.h
> @@ -26,6 +26,8 @@
>  
>  #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
>  
> +#define QEMU_WARN_NONNULL_ARGS(args...) __attribute__((nonnull(args)))

If we take this, it should come with a warning attached to it, because
it has really nasty behaviour with GCC. Consider code like:

  void foo(void *bar) __attribute__((nonnull(1)));

  ...

  void foo(void *bar) { if (!bar) return; }

GCC may or may not warn you about passing NULL for the 'bar'
parameter, but it will none the less assume nothing passes
NULL, and thus remove the 'if (!bar)' conditional during
optimization. IOW, adding nonnull annotations can actually
make your code less robust :-(

After having a number of crashes in libvirt caused by gcc
optimizing out checks for NULL, we now only define nonnull
when running under static analysis (coverity) and not when
compiling normally.

https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/internal.h;h=5895030415968d72200599e8a59bbf01ffc2d5a3;hb=HEAD#l162

The 2 functions you've added nonnull attrs to look safe enough,
but people might unwittingly use this elsewhere in QEMU in future
not realizing the side-effect it has.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH] Add ability to provide ifname when using netdev bridge or tap helper

2018-01-17 Thread Daniel P. Berrange
On Wed, Jan 17, 2018 at 06:53:30PM +0800, Jason Wang wrote:
> 
> 
> On 2018年01月17日 18:31, Daniel P. Berrange wrote:
> > On Tue, Jan 16, 2018 at 03:18:24PM -0800, Shaun Reitan wrote:
> > > This patch replaces the patch I sent yesturday. This one fixes
> > > a bug in my original code as well as corrects a few styling
> > > issues. Hopfully this one comes out correct!  Sorry for the
> > > inconvienece.
> > > When currently using -netdev bridge or -netdev tap with a helper
> > > you are unable to set an ifname. This patch adds that ability so
> > > that you can now specify an ifname.
> > I really don't think users should be allowed to override the
> > ifname when using the setuid bridge helper. This allows an
> > unprivileged users to create tap devices that may confuse the
> > sysadmin, and/or network mgmt scripts.
> 
> Ok, I drop it from my queue.
> 
> > 
> > eg consider the user asks for a tap device called  eth1. To the
> > sysadmin the user's tap device now looks like a physical NIC.
> > This can be even worse if the host does physical NIC hotplug,
> > or uses SRIOV. eg consider the host as eth0 -> eth7 for SRIOV
> > NICs, and eth3 is given to a guest. Now a user uses the setuid
> > helper to ask for a TAP called eth3. When the SRIOV device is
> > later released by the guest it will end up called eth8, as the
> > TAP device occupies eth3. In bad cases this could even cause
> > the host mgmt layer to configure bogus addresses on the eth3
> > TAP device instead of the SRIOV device.
> 
> It looks to me that mgmt should not assume the type or location of device
> just from its name. Ethtool should be used to do this.

Yes, a well written tool, or a prudent administrator should be careful
and validate the devices they use, but most have assumptions they make
which could easily be violated here.

> > If we want to allow ifname to be set via the setuid helper, then IMHO,
> > the config file for the helper *must* whitelist the various permitted
> > naming patterns.
> 
> Good point but this only work for e.g default helper. Qemu allows 3rd helper
> which can do anything they want.

If anyone writes a custom setuid helper, they are responsible for writing
to in a way that doesn't open security holes, or expose the admin to
intentionally misleading setup. IOW, that is somebody else's problem.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH] Add ability to provide ifname when using netdev bridge or tap helper

2018-01-17 Thread Daniel P. Berrange
On Tue, Jan 16, 2018 at 03:18:24PM -0800, Shaun Reitan wrote:
> This patch replaces the patch I sent yesturday. This one fixes
> a bug in my original code as well as corrects a few styling
> issues. Hopfully this one comes out correct!  Sorry for the
> inconvienece.
>  
> When currently using -netdev bridge or -netdev tap with a helper
> you are unable to set an ifname. This patch adds that ability so
> that you can now specify an ifname.

I really don't think users should be allowed to override the
ifname when using the setuid bridge helper. This allows an
unprivileged users to create tap devices that may confuse the
sysadmin, and/or network mgmt scripts.

eg consider the user asks for a tap device called  eth1. To the
sysadmin the user's tap device now looks like a physical NIC.
This can be even worse if the host does physical NIC hotplug,
or uses SRIOV. eg consider the host as eth0 -> eth7 for SRIOV
NICs, and eth3 is given to a guest. Now a user uses the setuid
helper to ask for a TAP called eth3. When the SRIOV device is
later released by the guest it will end up called eth8, as the
TAP device occupies eth3. In bad cases this could even cause
the host mgmt layer to configure bogus addresses on the eth3
TAP device instead of the SRIOV device.

If we want to allow ifname to be set via the setuid helper, then IMHO,
the config file for the helper *must* whitelist the various permitted
naming patterns.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH 0/7] docker: update Ubuntu and Fedora images, deprecate old ones

2018-01-17 Thread Daniel P. Berrange
On Wed, Jan 17, 2018 at 10:26:36AM +0800, Fam Zheng wrote:
> 
> 
> On 01/12/2018 08:49 PM, Philippe Mathieu-Daudé wrote:
> > Hi,
> > 
> > This series is to be clearer about which upstream version we are using.
> > 
> > All "FROM distrib:latest" entries have now been removed and replaced by
> > explicit "FROM distrib:version" ones.
> > 
> > To keep backward compatibility, a warning is displayed to the user,
> > suggesting which correct base image to use.
> > 
> > To be consistent, we remove the deprecated images of the "make docker" 
> > output.
> 
> Changing image names when a new release comes out is not maintainable for
> Fedora and Ubuntu because of the fast pace. Therefore I prefer Paolo's
> simple patch.

The flipside is that if we release QEMU 2.12 with fedora:27, then for as  long
as the 2.12 stable branch exists, dockers tests run on that branch will have a
consistent target and will be unlikely to break.  If we use fedora:latest,
then every time a new Fedora comes out there's non-trivial chance that docker
builds on all the stable branches will all break due to some new glibc change.
Sticking with explicit named versions is a big win in this respect IMHO.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH] chardev/char-socket: add POLLHUP handler

2018-01-16 Thread Daniel P. Berrange
On Tue, Jan 16, 2018 at 06:56:20PM +0100, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Jan 10, 2018 at 2:18 PM, Klim Kireev  
> wrote:
> > The following behavior was observed for QEMU configured by libvirt
> > to use guest agent as usual for the guests without virtio-serial
> > driver (Windows or the guest remaining in BIOS stage).
> >
> > In QEMU on first connect to listen character device socket
> > the listen socket is removed from poll just after the accept().
> > virtio_serial_guest_ready() returns 0 and the descriptor
> > of the connected Unix socket is removed from poll and it will
> > not be present in poll() until the guest will initialize the driver
> > and change the state of the serial to "guest connected".
> >
> > In libvirt connect() to guest agent is performed on restart and
> > is run under VM state lock. Connect() is blocking and can
> > wait forever.
> > In this case libvirt can not perform ANY operation on that VM.
> 
> Adding Daniel in CC for comments about libvirt behaviour.

This is a libvirt bug - libvirt should put a finite timeout on connecting
to the guest agent socket to avoid this.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH x86-next v2] target-i386: add PCID flag to Westmere, Sandy Bridge and Ivy Bridge

2018-01-16 Thread Daniel P. Berrange
On Tue, Jan 16, 2018 at 03:08:15PM -0200, Eduardo Habkost wrote:
> [CCing Daniel]
> 
> On Tue, Jan 16, 2018 at 04:33:00PM +0100, Kashyap Chamarthy wrote:
> > On Tue, Jan 16, 2018 at 01:55:22PM +0100, Vincent Bernat wrote:
> > >  ❦ 16 janvier 2018 10:41 -0200, Eduardo Habkost  :
> > > 
> > > >> > Adding Westmere-PCID would require adding a Westmere-PCID-IBRS
> > > >> > CPU model too, so this is starting to look a bit ridiculous.
> > > >> > Sane VM management systems would know how to use
> > > >> > "-cpu Westmere,+pcid" without requiring new CPU model entries in
> > > >> > QEMU.  What's missing in existing management stacks to allow that
> > > >> > to happen?
> > > >> 
> > > >> That's what I actually do. So, I am fine with the solution of doing
> > > >> nothing. However, it would be nice for unaware people to get the 
> > > >> speedup
> > > >> of pcid without knowing about it. Maybe we can just forget about
> > > >> Westmere and still apply it to Sandy Bridge and Ivy Bridge.
> > > >
> > > > If management stacks today don't let the user choose
> > > > "Westmere,+pcid", we probably have no other choice than adding a
> > > > Westmere-PCID CPU model.  But our management stacks need to be
> > > > fixed so we won't need similar hacks in the future.
> > 
> > True;  I'm aware of the limitation here in Nova.
> > 
> > > With libvirt:
> > > 
> > >   
> > > Westmere
> > > 
> > >   
> > 
> > Yep, libvirt upstream allows it.
> > 
> > > We are using CloudStack on top of that and it's also an available
> > > option. However, looking at OpenStack, it doesn't seem possible:
> > >  
> > > https://github.com/openstack/nova/blob/6b248518da794a4c82665c22abf7bee5aa527a47/nova/conf/libvirt.py#L506
> > 
> > That's correct, upstream OpenStack Nova doesn't yet have facility to
> > specify granular CPU feature names.  Nova just ought to wire up the
> > facility libvirt already provides.
> 
> I still don't understand why OpenStack doesn't let users add or
> modify elements on the domain XML.  This isn't the first time I
> see this preventing users from fixing problems or optimizing
> their systems.
> 
> Is there a summary of the reasons behind this limitation
> somewhere?

Exposing ability to control every aspect of Libvirt XML is a non-goal of
Nova. A great many of the features require different modelling and/or
explicit handling by Nova to work well in the context of OpenStack's
architecture. The domain XML is automatically generated on the fly by
Nova based on the info it gets from various inputs, so there's nothing
that can be editted directly to add custom elements. The only way that
would allow modification is for Nova to send the XML it generates to
an external plugin script and read back modified XML. Historically Nova
did have alot of plugin points that allowed arbitrary admin hacks like
this, but they end up being a support burden in themselves, as they
end up being black boxes which change Nova behaviour in unpredictable
ways. Thus Nova has actually worked to remove as many of the plugins
as possible.

In this case there is a clear benefit to being able to add extra CPU
features, over the base named model. It is easy for Nova to wire this
up and it should do so as a priority.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PULL 08/51] chardev: introduce qemu_chr_timeout_add_ms()

2018-01-16 Thread Daniel P. Berrange
On Tue, Jan 16, 2018 at 03:16:50PM +0100, Paolo Bonzini wrote:
> From: Peter Xu 
> 
> It's a replacement of g_timeout_add[_seconds]() for chardevs.  Chardevs
> now can have dedicated gcontext, we should always bind chardev tasks
> onto those gcontext rather than the default main context.  Since there
> are quite a few of g_timeout_add[_seconds]() callers, a new function
> qemu_chr_timeout_add_ms() is introduced.

FYI the point of using g_timeout_add_seconds() is that it allow the
glib event loop to be more efficient. It ensures that all timers
which second granularity are dispatched on the same iteration of
the main loop. IOW, if you have 10 timers registered with
g_timeout_add_seconds() the main loop wakes up once a second and
runs all 10 of them. If you have 10 timers registered with g_timeout_add
the main loop wakes up 10 times a second, at a different time for each
timer.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



[Qemu-devel] [PATCH v5 12/14] ui: update keycodemapdb to get py3 fixes

2018-01-16 Thread Daniel P. Berrange
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 ui/keycodemapdb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/keycodemapdb b/ui/keycodemapdb
index 05dad417e9..6b3d716e2b 16
--- a/ui/keycodemapdb
+++ b/ui/keycodemapdb
@@ -1 +1 @@
-Subproject commit 05dad417e9d0b37ee1fba33056d91a6b734b3357
+Subproject commit 6b3d716e2b6472eb7189d3220552280ef3d832ce
-- 
2.14.3




[Qemu-devel] [PATCH v5 11/14] input: add missing JIS keys to virtio input

2018-01-16 Thread Daniel P. Berrange
From: Miika S 

keycodemapdb updated to add the QKeyCodes muhenkan and katakanahiragana

Signed-off-by: Miika S 
---
 hw/input/virtio-input-hid.c | 7 +++
 qapi/ui.json| 5 -
 ui/keycodemapdb | 2 +-
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/input/virtio-input-hid.c b/hw/input/virtio-input-hid.c
index e78faec0b1..9628d289f9 100644
--- a/hw/input/virtio-input-hid.c
+++ b/hw/input/virtio-input-hid.c
@@ -139,6 +139,13 @@ static const unsigned int keymap_qcode[Q_KEY_CODE__MAX] = {
 [Q_KEY_CODE_META_L]  = KEY_LEFTMETA,
 [Q_KEY_CODE_META_R]  = KEY_RIGHTMETA,
 [Q_KEY_CODE_MENU]= KEY_MENU,
+
+[Q_KEY_CODE_MUHENKAN]= KEY_MUHENKAN,
+[Q_KEY_CODE_HENKAN]  = KEY_HENKAN,
+[Q_KEY_CODE_KATAKANAHIRAGANA]= KEY_KATAKANAHIRAGANA,
+[Q_KEY_CODE_COMPOSE] = KEY_COMPOSE,
+[Q_KEY_CODE_RO]  = KEY_RO,
+[Q_KEY_CODE_YEN] = KEY_YEN,
 };
 
 static const unsigned int keymap_button[INPUT_BUTTON__MAX] = {
diff --git a/qapi/ui.json b/qapi/ui.json
index 07b468f625..d6679aa8f5 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -748,6 +748,9 @@
 # @ac_bookmarks: since 2.10
 # altgr, altgr_r: dropped in 2.10
 #
+# @muhenkan: since 2.12
+# @katakanahiragana: since 2.12
+#
 # 'sysrq' was mistakenly added to hack around the fact that
 # the ps2 driver was not generating correct scancodes sequences
 # when 'alt+print' was pressed. This flaw is now fixed and the
@@ -775,7 +778,7 @@
 'left', 'up', 'down', 'right', 'insert', 'delete', 'stop', 'again',
 'props', 'undo', 'front', 'copy', 'open', 'paste', 'find', 'cut',
 'lf', 'help', 'meta_l', 'meta_r', 'compose', 'pause',
-'ro', 'hiragana', 'henkan', 'yen',
+'ro', 'hiragana', 'henkan', 'yen', 'muhenkan', 'katakanahiragana',
 'kp_comma', 'kp_equals', 'power', 'sleep', 'wake',
 'audionext', 'audioprev', 'audiostop', 'audioplay', 'audiomute',
 'volumeup', 'volumedown', 'mediaselect',
diff --git a/ui/keycodemapdb b/ui/keycodemapdb
index 10739aa260..05dad417e9 16
--- a/ui/keycodemapdb
+++ b/ui/keycodemapdb
@@ -1 +1 @@
-Subproject commit 10739aa26051a5d49d88132604539d3ed085e72e
+Subproject commit 05dad417e9d0b37ee1fba33056d91a6b734b3357
-- 
2.14.3




[Qemu-devel] [PATCH v5 09/14] scripts: ensure signrom treats data as bytes

2018-01-16 Thread Daniel P. Berrange
Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/signrom.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/signrom.py b/scripts/signrom.py
index d1dabe0240..0497a1c32e 100644
--- a/scripts/signrom.py
+++ b/scripts/signrom.py
@@ -18,7 +18,7 @@ fin = open(sys.argv[1], 'rb')
 fout = open(sys.argv[2], 'wb')
 
 magic = fin.read(2)
-if magic != '\x55\xaa':
+if magic != b'\x55\xaa':
 sys.exit("%s: option ROM does not begin with magic 55 aa" % sys.argv[1])
 
 size_byte = ord(fin.read(1))
@@ -33,7 +33,7 @@ elif len(data) < size:
 # Add padding if necessary, rounding the whole input to a multiple of
 # 512 bytes according to the third byte of the input.
 # size-1 because a final byte is added below to store the checksum.
-data = data.ljust(size-1, '\0')
+data = data.ljust(size-1, b'\0')
 else:
 if ord(data[-1:]) != 0:
 sys.stderr.write('WARNING: ROM includes nonzero checksum\n')
-- 
2.14.3




[Qemu-devel] [PATCH v5 04/14] qapi: adapt to moved location of StringIO module in py3

2018-01-16 Thread Daniel P. Berrange
Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/qapi.py | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 514b7bb5a4..514cca44bf 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -22,6 +22,10 @@ try:
 from collections import OrderedDict
 except:
 from ordereddict import OrderedDict
+try:
+from StringIO import StringIO
+except ImportError:
+from io import StringIO
 
 builtin_types = {
 'null': 'QTYPE_QNULL',
@@ -1995,8 +1999,7 @@ def open_output(output_dir, do_c, do_h, prefix, c_file, 
h_file,
 if really:
 return open(name, opt)
 else:
-import StringIO
-return StringIO.StringIO()
+return StringIO()
 
 fdef = maybe_open(do_c, c_file, 'w')
 fdecl = maybe_open(do_h, h_file, 'w')
-- 
2.14.3




[Qemu-devel] [PATCH v5 06/14] qapi: remove '-q' arg to diff when comparing QAPI output

2018-01-16 Thread Daniel P. Berrange
When the qapi schema tests fail they merely print that the expected
output didn't match the actual output. This is largely useless when
trying diagnose what went wrong. Removing the '-q' arg to diff
means that it is still silent on successful tests, but when it
fails we'll see details of the incorrect output.

Reviewed-by: Eric Blake <ebl...@redhat.com>
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 tests/Makefile.include | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 39a4b5359d..d65fb4e1b3 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -908,10 +908,10 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): 
check-%.json: $(SRC_PATH)/%.json
$^ >$*.test.out 2>$*.test.err; \
echo $$? >$*.test.exit, \
"TEST","$*.out")
-   @diff -q $(SRC_PATH)/$*.out $*.test.out
+   @diff $(SRC_PATH)/$*.out $*.test.out
@# Sanitize error messages (make them independent of build directory)
-   @perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff -q 
$(SRC_PATH)/$*.err -
-   @diff -q $(SRC_PATH)/$*.exit $*.test.exit
+   @perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff 
$(SRC_PATH)/$*.err -
+   @diff $(SRC_PATH)/$*.exit $*.test.exit
 
 .PHONY: check-tests/qapi-schema/doc-good.texi
 check-tests/qapi-schema/doc-good.texi: tests/qapi-schema/doc-good.test.texi
-- 
2.14.3




[Qemu-devel] [PATCH v5 01/14] qapi: convert to use python print function instead of statement

2018-01-16 Thread Daniel P. Berrange
Python 3 no longer supports the bare "print" statement, it must be
called as a normal function with round brackets. It is possible to
opt-in to this new syntax with Python 2.6 onwards by importing the
"print_function" from the "__future__" module, making it easy to
support Python 2 and 3 in parallel.

Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/qapi.py| 12 ++--
 scripts/qapi2texi.py   |  9 +
 tests/qapi-schema/test-qapi.py | 41 +
 3 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 43a54bf40f..64fde4b6c5 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -11,6 +11,7 @@
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
+from __future__ import print_function
 import errno
 import getopt
 import os
@@ -1467,7 +1468,7 @@ class QAPISchema(object):
 self._def_exprs()
 self.check()
 except QAPIError as err:
-print >>sys.stderr, err
+print(err, file=sys.stderr)
 exit(1)
 
 def _def_entity(self, ent):
@@ -1931,7 +1932,7 @@ def parse_command_line(extra_options='', 
extra_long_options=[]):
['source', 'header', 'prefix=',
 'output-dir='] + extra_long_options)
 except getopt.GetoptError as err:
-print >>sys.stderr, "%s: %s" % (sys.argv[0], str(err))
+print("%s: %s" % (sys.argv[0], str(err)), file=sys.stderr)
 sys.exit(1)
 
 output_dir = ''
@@ -1945,9 +1946,8 @@ def parse_command_line(extra_options='', 
extra_long_options=[]):
 if o in ('-p', '--prefix'):
 match = re.match(r'([A-Za-z_.-][A-Za-z0-9_.-]*)?', a)
 if match.end() != len(a):
-print >>sys.stderr, \
-"%s: 'funny character '%s' in argument of --prefix" \
-% (sys.argv[0], a[match.end()])
+print("%s: 'funny character '%s' in argument of --prefix" \
+  % (sys.argv[0], a[match.end()]), file=sys.stderr)
 sys.exit(1)
 prefix = a
 elif o in ('-o', '--output-dir'):
@@ -1964,7 +1964,7 @@ def parse_command_line(extra_options='', 
extra_long_options=[]):
 do_h = True
 
 if len(args) != 1:
-print >>sys.stderr, "%s: need exactly one argument" % sys.argv[0]
+print("%s: need exactly one argument" % sys.argv[0], file=sys.stderr)
 sys.exit(1)
 fname = args[0]
 
diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 92e2af2cd6..70e1fe76ef 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -4,6 +4,7 @@
 # This work is licensed under the terms of the GNU LGPL, version 2+.
 # See the COPYING file in the top-level directory.
 """This script produces the documentation of a qapi schema in texinfo format"""
+from __future__ import print_function
 import re
 import sys
 
@@ -274,15 +275,15 @@ def texi_schema(schema):
 def main(argv):
 """Takes schema argument, prints result to stdout"""
 if len(argv) != 2:
-print >>sys.stderr, "%s: need exactly 1 argument: SCHEMA" % argv[0]
+print("%s: need exactly 1 argument: SCHEMA" % argv[0], file=sys.stderr)
 sys.exit(1)
 
 schema = qapi.QAPISchema(argv[1])
 if not qapi.doc_required:
-print >>sys.stderr, ("%s: need pragma 'doc-required' "
- "to generate documentation" % argv[0])
+print("%s: need pragma 'doc-required' "
+   "to generate documentation" % argv[0], file=sys.stderr)
 sys.exit(1)
-print texi_schema(schema)
+print(texi_schema(schema))
 
 
 if __name__ == '__main__':
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index fe0ca08d78..a43fa873e1 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -10,6 +10,7 @@
 # See the COPYING file in the top-level directory.
 #
 
+from __future__ import print_function
 from qapi import *
 from pprint import pprint
 import os
@@ -18,51 +19,51 @@ import sys
 
 class QAPISchemaTestVisitor(QAPISchemaVisitor):
 def visit_enum_type(self, name, info, values, prefix):
-print 'enum %s %s' % (name, values)
+print('enum %s %s' % (name, values))
 if prefix:
-print 'prefix %s' % prefix
+print('prefix %s' % prefix)
 
 def visit_object_type(self, name, info, base, members, variants):
-print 'object %s' % name
+print('object %s' % name)
  

[Qemu-devel] [PATCH v5 14/14] docker: change Fedora images to run with python3

2018-01-16 Thread Daniel P. Berrange
Fedora has switched to Python 3 by default, so it makes sense to use that
for testing QEMU builds, so we get testing of Python 3 compatibility.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 tests/docker/dockerfiles/fedora.docker | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/docker/dockerfiles/fedora.docker 
b/tests/docker/dockerfiles/fedora.docker
index 4b26c3aded..a22fe16157 100644
--- a/tests/docker/dockerfiles/fedora.docker
+++ b/tests/docker/dockerfiles/fedora.docker
@@ -1,6 +1,6 @@
 FROM fedora:latest
 ENV PACKAGES \
-ccache gettext git tar PyYAML sparse flex bison python2 bzip2 hostname \
+ccache gettext git tar PyYAML sparse flex bison python3 bzip2 hostname \
 glib2-devel pixman-devel zlib-devel SDL-devel libfdt-devel \
 gcc gcc-c++ clang make perl which bc findutils libaio-devel \
 nettle-devel \
@@ -12,6 +12,7 @@ ENV PACKAGES \
 mingw64-gtk2 mingw64-gtk3 mingw64-gnutls mingw64-nettle mingw64-libtasn1 \
 mingw64-libjpeg-turbo mingw64-libpng mingw64-curl mingw64-libssh2 \
 mingw64-bzip2
+ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3
 
 RUN dnf install -y $PACKAGES
 RUN rpm -q $PACKAGES | sort > /packages.txt
-- 
2.14.3




[Qemu-devel] [PATCH v5 02/14] qapi: use items()/values() intead of iteritems()/itervalues()

2018-01-16 Thread Daniel P. Berrange
The iteritems()/itervalues() methods are gone in py3, but the
items()/values() methods are still around. The latter are less
efficient than the former in py2, but this has unmeasurably
small impact on QEMU build time, so taking portability over
efficiency is a net win.

Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/qapi.py| 12 ++--
 scripts/qapi2texi.py   |  2 +-
 tests/qapi-schema/test-qapi.py |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 64fde4b6c5..98d7123d27 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -245,7 +245,7 @@ class QAPIDoc(object):
"'Returns:' is only valid for commands")
 
 def check(self):
-bogus = [name for name, section in self.args.iteritems()
+bogus = [name for name, section in self.args.items()
  if not section.member]
 if bogus:
 raise QAPISemError(
@@ -300,7 +300,7 @@ class QAPISchemaParser(object):
 if not isinstance(pragma, dict):
 raise QAPISemError(
 info, "Value of 'pragma' must be a dictionary")
-for name, value in pragma.iteritems():
+for name, value in pragma.items():
 self._pragma(name, value, info)
 else:
 expr_elem = {'expr': expr,
@@ -1566,7 +1566,7 @@ class QAPISchema(object):
 
 def _make_members(self, data, info):
 return [self._make_member(key, value, info)
-for (key, value) in data.iteritems()]
+for (key, value) in data.items()]
 
 def _def_struct_type(self, expr, info, doc):
 name = expr['struct']
@@ -1598,11 +1598,11 @@ class QAPISchema(object):
 name, info, doc, 'base', self._make_members(base, info)))
 if tag_name:
 variants = [self._make_variant(key, value)
-for (key, value) in data.iteritems()]
+for (key, value) in data.items()]
 members = []
 else:
 variants = [self._make_simple_variant(key, value, info)
-for (key, value) in data.iteritems()]
+for (key, value) in data.items()]
 typ = self._make_implicit_enum_type(name, info,
 [v.name for v in variants])
 tag_member = QAPISchemaObjectTypeMember('type', typ, False)
@@ -1617,7 +1617,7 @@ class QAPISchema(object):
 name = expr['alternate']
 data = expr['data']
 variants = [self._make_variant(key, value)
-for (key, value) in data.iteritems()]
+for (key, value) in data.items()]
 tag_member = QAPISchemaObjectTypeMember('type', 'QType', False)
 self._def_entity(
 QAPISchemaAlternateType(name, info, doc,
diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 70e1fe76ef..bf1c57b2e2 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -146,7 +146,7 @@ def texi_member(member, suffix=''):
 def texi_members(doc, what, base, variants, member_func):
 """Format the table of members"""
 items = ''
-for section in doc.args.itervalues():
+for section in doc.args.values():
 # TODO Drop fallbacks when undocumented members are outlawed
 if section.text:
 desc = texi_format(section.text)
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index a43fa873e1..ac43d3458e 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -63,7 +63,7 @@ for doc in schema.docs:
 else:
 print('doc freeform')
 print('body=\n%s' % doc.body.text)
-for arg, section in doc.args.iteritems():
+for arg, section in doc.args.items():
 print('arg=%s\n%s' % (arg, section.text))
 for section in doc.sections:
 print('section=%s\n%s' % (section.name, section.text))
-- 
2.14.3




[Qemu-devel] [PATCH v5 00/14] Support building with py2 or py3

2018-01-16 Thread Daniel P. Berrange
This is an update for my previously posted series:

 v2: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg06528.html
 v3: https://lists.gnu.org/archive/html/qemu-devel/2018-01/msg02978.html
 v4: https://lists.gnu.org/archive/html/qemu-devel/2018-01/msg03150.html

This series enables some level of CI testing for py3 so that our CI jobs will
get coverage of both py2 and py3 builds to avoid bitrot.

I did a test travis build with py 3.0 and py 3.6 and got success:

  https://travis-ci.org/berrange/qemu/builds/328223261

The goal was to achieve the following

  ./configure --python=/usr/bin/python3
  make
  make check

This still requires passing python path to configure explicitly. A
further improvement would be for configure to automatically detect
a pythjon 3 binary and use it preferentially to python 2.

I have not attempted to fix/validate the block I/O tests. I would expect
them to be broken, but easily fixable with the similar kind of scope
changes as seen here. I felt it better to tackle that separately to
avoid this initial series getting too large.

Although the Python 2 EOL date is 2020, we already have distros which
are not shipping Python 2 by default (Fedora >= 26 has dropped Py2 from
the default install). Any new releases of long life and/or enterprise
distros may well not ship Python 2 given that it would go EOL long
before the EOL of the distro itself. IOW QEMU does have a fairly pressing
need to be able to support Python 3 for building.

A request for py3 is tracked here:

   https://bugs.launchpad.net/qemu/+bug/1708462

If, rather than supporting py2+py3 in parallel, we wish to entirely drop
py2 support, this series would not change significantly

 - The "from __future__ import print_function" line can be removed
   from patch 1.
 - The code in patches 2, 3, 4 to deal with changed module names
   for a few functions can be simpified to only try the py3 location
 - The travis + docker jobs would be fully updated to install py3,
   or delete jobs which can't support py3

Given how little code is removed should we drop py2 support, I don't
believe it is in our immediate interests to do this. It would create
extra pain for consumers of QEMU, with little benefit to QEMU code
maintainance. The key thing is ensuring our travis+docker jobs provide
satisfactory automated test coverage for the variety of python versions
in the distros we care about targetting.

NB, Patch 11 here is not related to python 3 work - it was just a
temporary pre-requisite of pulling in the keycodemapdb update.

Changes since v4:

 - Fix broken rebase which accidentally squashed first two
   patches together
 - Unset LC_ALL, and set LANG + LC_CTYPE, instead of only LANG (Eric)

Changes since v3:

 - Remove space before '(' in print() function calls (Phillippe)
 - Force use of en_US.UTF-8 for QAPI code generation (Patchew)

Changes since v2:

 - Pull in fix for keycodemapdb
 - Enable testing with Travis
 - Enable testing with Fedora Docker images
 - Fix for sort ordering to fix 'make check-qapi-schema'
 - Fix for signrom data

Daniel P. Berrange (13):
  qapi: convert to use python print function instead of statement
  qapi: use items()/values() intead of iteritems()/itervalues()
  qapi: Use OrderedDict from standard library if available
  qapi: adapt to moved location of StringIO module in py3
  qapi: Adapt to moved location of 'maketrans' function in py3
  qapi: remove '-q' arg to diff when comparing QAPI output
  qapi: ensure stable sort ordering when checking QAPI entities
  qapi: force a UTF-8 locale for running Python
  scripts: ensure signrom treats data as bytes
  configure: allow use of python 3
  ui: update keycodemapdb to get py3 fixes
  travis: improve python version test coverage
  docker: change Fedora images to run with python3

Miika S (1):
  input: add missing JIS keys to virtio input

 .travis.yml| 14 +++
 Makefile   | 22 +
 configure  |  5 ++--
 hw/input/virtio-input-hid.c|  7 ++
 qapi/ui.json   |  5 +++-
 scripts/qapi.py| 43 --
 scripts/qapi2texi.py   | 11 +
 scripts/signrom.py |  4 ++--
 tests/Makefile.include |  6 ++---
 tests/docker/dockerfiles/fedora.docker |  3 ++-
 tests/qapi-schema/test-qapi.py | 43 +-
 ui/keycodemapdb|  2 +-
 12 files changed, 96 insertions(+), 69 deletions(-)

-- 
2.14.3




[Qemu-devel] [PATCH v5 13/14] travis: improve python version test coverage

2018-01-16 Thread Daniel P. Berrange
Currently travis declares ancient python 2.4 is desired. Update that to
2.6 which is the oldest version any targetted distros still needs. If we
just list a python 3 version at the top level this will double the
number of travis jobs we run which is unreasonable.

So arbitrarily pick the clang test matrix entries to build with python
3.0 and 3.6, to extend coverage of python versions, without increasing
job count or build time.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 .travis.yml | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index f583839755..708c886017 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,7 +1,7 @@
 sudo: false
 language: c
 python:
-  - "2.4"
+  - "2.6"
 compiler:
   - gcc
 cache: ccache
@@ -115,15 +115,17 @@ matrix:
 - sudo apt-get build-dep -qq qemu
 - wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
 - git submodule update --init --recursive
-# Trusty System build with latest stable clang
+# Trusty System build with latest stable clang & python 3.0
 - sudo: required
   addons:
   dist: trusty
   language: generic
   compiler: none
+  python:
+- "3.0"
   env:
 - COMPILER_NAME=clang CXX=clang++-3.9 CC=clang-3.9
-- CONFIG="--disable-linux-user --cc=clang-3.9 --cxx=clang++-3.9"
+- CONFIG="--disable-linux-user --cc=clang-3.9 --cxx=clang++-3.9 
--python=/usr/bin/python3"
   before_install:
 - wget -nv -O - http://llvm.org/apt/llvm-snapshot.gpg.key | sudo 
apt-key add -
 - sudo apt-add-repository -y 'deb http://llvm.org/apt/trusty 
llvm-toolchain-trusty-3.9 main'
@@ -134,15 +136,17 @@ matrix:
 - git submodule update --init --recursive
   before_script:
 - ./configure ${CONFIG} || cat config.log
-# Trusty Linux User build with latest stable clang
+# Trusty Linux User build with latest stable clang & python 3.6
 - sudo: required
   addons:
   dist: trusty
   language: generic
   compiler: none
+  python:
+- "3.6"
   env:
 - COMPILER_NAME=clang CXX=clang++-3.9 CC=clang-3.9
-- CONFIG="--disable-system --cc=clang-3.9 --cxx=clang++-3.9"
+- CONFIG="--disable-system --cc=clang-3.9 --cxx=clang++-3.9 
--python=/usr/bin/python3"
   before_install:
 - wget -nv -O - http://llvm.org/apt/llvm-snapshot.gpg.key | sudo 
apt-key add -
 - sudo apt-add-repository -y 'deb http://llvm.org/apt/trusty 
llvm-toolchain-trusty-3.9 main'
-- 
2.14.3




[Qemu-devel] [PATCH v5 10/14] configure: allow use of python 3

2018-01-16 Thread Daniel P. Berrange
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 configure | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index b272a0336b..60b99f45f6 100755
--- a/configure
+++ b/configure
@@ -1598,9 +1598,8 @@ fi
 
 # Note that if the Python conditional here evaluates True we will exit
 # with status 1 which is a shell 'false' value.
-if ! $python -c 'import sys; sys.exit(sys.version_info < (2,6) or 
sys.version_info >= (3,))'; then
-  error_exit "Cannot use '$python', Python 2.6 or later is required." \
-  "Note that Python 3 or later is not yet supported." \
+if ! $python -c 'import sys; sys.exit(sys.version_info < (2,6))'; then
+  error_exit "Cannot use '$python', Python 2 >= 2.6 or Python 3 is required." \
   "Use --python=/path/to/python to specify a supported Python."
 fi
 
-- 
2.14.3




[Qemu-devel] [PATCH v5 08/14] qapi: force a UTF-8 locale for running Python

2018-01-16 Thread Daniel P. Berrange
Python2 did not validate locale correctness when reading input data, so
would happily read UTF-8 data in non-UTF-8 locales. Python3 is strict so
if you try to read UTF-8 data in the C locale, it will raise an error
for any UTF-8 bytes that aren't representable in 7-bit ascii encoding.
e.g.

UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 54: 
ordinal not in range(128)
Traceback (most recent call last):
  File "/tmp/qemu-test/src/scripts/qapi-commands.py", line 317, in 
schema = QAPISchema(input_file)
  File "/tmp/qemu-test/src/scripts/qapi.py", line 1468, in __init__
parser = QAPISchemaParser(open(fname, 'r'))
  File "/tmp/qemu-test/src/scripts/qapi.py", line 301, in __init__
previously_included)
  File "/tmp/qemu-test/src/scripts/qapi.py", line 348, in _include
exprs_include = QAPISchemaParser(fobj, previously_included, info)
  File "/tmp/qemu-test/src/scripts/qapi.py", line 271, in __init__
self.src = fp.read()
  File "/usr/lib64/python3.5/encodings/ascii.py", line 26, in decode
return codecs.ascii_decode(input, self.errors)[0]

More background on this can be seen in

  https://www.python.org/dev/peps/pep-0538/

Many distros support a new C.UTF-8 locale that is like the C locale,
but with UTF-8 instead of 7-bit ASCII. That is not entirely portable
though. This patch thus sets the LANG to "C", but overrides LC_CTYPE
to be en_US.UTF-8 locale. This gets us pretty close to C.UTF-8, but
in a way that should be portable to everywhere QEMU builds.

This patch only forces UTF-8 for QAPI scripts, since that is the one
showing the immediate error under Python3 with C locale, but potentially
we ought to force this for all python scripts used in the build process.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 Makefile | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index d86ecd2dd4..63767bb11f 100644
--- a/Makefile
+++ b/Makefile
@@ -17,6 +17,8 @@ ifneq ($(wildcard config-host.mak),)
 all:
 include config-host.mak
 
+PYTHON_UTF8 = LC_ALL= LANG=C LC_CTYPE=en_US.UTF-8 $(PYTHON)
+
 git-submodule-update:
 
 .PHONY: git-submodule-update
@@ -471,17 +473,17 @@ qapi-py = $(SRC_PATH)/scripts/qapi.py 
$(SRC_PATH)/scripts/ordereddict.py
 
 qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\
 $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
+   $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-types.py \
$(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \
"GEN","$@")
 qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\
 $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
+   $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-visit.py \
$(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \
"GEN","$@")
 qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\
 $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py 
$(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
+   $(call quiet-command,$(PYTHON_UTF8) 
$(SRC_PATH)/scripts/qapi-commands.py \
$(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \
"GEN","$@")
 
@@ -502,27 +504,27 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json 
$(SRC_PATH)/qapi/common.json \
 
 qapi-types.c qapi-types.h :\
 $(qapi-modules) $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
+   $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-types.py \
$(gen-out-type) -o "." -b $<, \
"GEN","$@")
 qapi-visit.c qapi-visit.h :\
 $(qapi-modules) $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
+   $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-visit.py \
$(gen-out-type) -o "." -b $<, \
"GEN","$@")
 qapi-event.c qapi-event.h :\
 $(qapi-modules) $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py \
+   $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-event.py \
$(gen-out-type) -o "." $<, \
"GEN","$@")
 qmp-commands.h qmp-marshal.c :\
 $(qapi-modules) $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
-   $(call quiet-command,$(PYTH

[Qemu-devel] [PATCH v5 03/14] qapi: Use OrderedDict from standard library if available

2018-01-16 Thread Daniel P. Berrange
The OrderedDict class appeared in the 'collections' module
from python 2.7 onwards, so use that in preference to our
local backport if available.

Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/qapi.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 98d7123d27..514b7bb5a4 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -18,7 +18,10 @@ import os
 import re
 import string
 import sys
-from ordereddict import OrderedDict
+try:
+from collections import OrderedDict
+except:
+from ordereddict import OrderedDict
 
 builtin_types = {
 'null': 'QTYPE_QNULL',
-- 
2.14.3




[Qemu-devel] [PATCH v5 07/14] qapi: ensure stable sort ordering when checking QAPI entities

2018-01-16 Thread Daniel P. Berrange
Some early python 3.x versions will have different default
ordering when calling the 'values()' method on a dict, compared
to python 2.x and later 3.x versions. Explicitly sort the items
to get a stable ordering.

Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/qapi.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 1fdd189c0d..58f995b07f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1678,7 +1678,7 @@ class QAPISchema(object):
 assert False
 
 def check(self):
-for ent in self._entity_dict.values():
+for (name, ent) in sorted(self._entity_dict.items()):
 ent.check(self)
 
 def visit(self, visitor):
-- 
2.14.3




[Qemu-devel] [PATCH v5 05/14] qapi: Adapt to moved location of 'maketrans' function in py3

2018-01-16 Thread Daniel P. Berrange
Reviewed-by: Eric Blake <ebl...@redhat.com>
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/qapi.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 514cca44bf..1fdd189c0d 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1734,7 +1734,10 @@ def c_enum_const(type_name, const_name, prefix=None):
 type_name = prefix
 return camel_to_upper(type_name) + '_' + c_name(const_name, False).upper()
 
-c_name_trans = string.maketrans('.-', '__')
+if hasattr(str, 'maketrans'):
+c_name_trans = str.maketrans('.-', '__')
+else:
+c_name_trans = string.maketrans('.-', '__')
 
 
 # Map @name to a valid C identifier.
-- 
2.14.3




Re: [Qemu-devel] RFE for patchew docker result reporting

2018-01-16 Thread Daniel P. Berrange
On Tue, Jan 16, 2018 at 10:16:28AM +0800, Fam Zheng wrote:
> On Mon, Jan 15, 2018 at 8:26 PM, Cornelia Huck <coh...@redhat.com> wrote:
> > On Mon, 15 Jan 2018 11:48:41 +0000
> > "Daniel P. Berrange" <berra...@redhat.com> wrote:
> >
> >> Currently if I look at the patchew website for build logs, the 'docker'
> >> job results are listed as a single expandable item.
> >>
> >> Patchew runs 3 separate docker builds, however, and there's a tonne of
> >> output to scroll through to find which one actually failed.
> >>
> >> Thus, my RFE is to split the build log up so that instead of a single
> >> "docker" result, it shows  "docker quick", "docker biuld" and
> >> "docker mingw" as separate build logs / results. This would make it
> >> easier to see at a glance which scenario failed.
> >
> > +1. Splitting the output into smaller units makes sense to me.
> 
> Made the proposed change on patchew.org. Thanks.

That's great, thanks for figuring this out so quickly !

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v4 10/13] input: add missing JIS keys to virtio input

2018-01-15 Thread Daniel P. Berrange
On Mon, Jan 15, 2018 at 11:17:15AM -0600, Eric Blake wrote:
> On 01/15/2018 11:02 AM, Daniel P. Berrange wrote:
> > From: Miika S <miika9...@gmail.com>
> > 
> > keycodemapdb updated to add the QKeyCodes muhenkan and katakanahiragana
> > 
> > Signed-off-by: Miika S <miika9...@gmail.com>
> > ---
> >  hw/input/virtio-input-hid.c | 7 +++
> >  qapi/ui.json| 5 -
> >  ui/keycodemapdb | 2 +-
> >  3 files changed, 12 insertions(+), 2 deletions(-)
> 
> Why are you bumping the submodule here and again in 11/13? Can those two
> patches be squashed together?

This patch is just temporarily pulled from another patch series, which
I expect to get merged before this one does.

> > +++ b/ui/keycodemapdb
> > @@ -1 +1 @@
> > -Subproject commit 10739aa26051a5d49d88132604539d3ed085e72e
> > +Subproject commit 05dad417e9d0b37ee1fba33056d91a6b734b3357


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v4 07/13] qapi: force a UTF-8 locale for running Python

2018-01-15 Thread Daniel P. Berrange
On Mon, Jan 15, 2018 at 11:15:01AM -0600, Eric Blake wrote:
> On 01/15/2018 11:02 AM, Daniel P. Berrange wrote:
> > Python2 did not validate locale correctness when reading input data, so
> > would happily read UTF-8 data in non-UTF-8 locales. Python3 is strict so
> > if you try to read UTF-8 data in the C locale, it will raise an error
> > for any UTF-8 bytes that aren't representable in 7-bit ascii encoding.
> 
> Urgh, that sounds like a Python bug. The C locale is defined by POSIX to
> be 8-bit clean (ie. a superset of ascii with 256 characters, not strict
> ascii with only 128 characters and 128 bytes that form encoding errors).
>  But that doesn't change the fact that we have to work around python's
> braindead misinterpretation of reality.

FYI there is some background on this behaviour here:

 https://www.python.org/dev/peps/pep-0538/

NB that doc says the new C-is-UTF-8 assumpion is for Python 3.7 or later,
but Fedora backported it to F27's  Python 3.6 :-)

The failure can be seen on Fedora with 3.0 -> 3.5 only. (BTW you can
install many Python 3.x versions concurrently on Fedora which is handy
for testing)

> > e.g.
> > 
> > UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 54: 
> > ordinal not in range(128)
> > Traceback (most recent call last):
> >   File "/tmp/qemu-test/src/scripts/qapi-commands.py", line 317, in 
> > schema = QAPISchema(input_file)
> >   File "/tmp/qemu-test/src/scripts/qapi.py", line 1468, in __init__
> > parser = QAPISchemaParser(open(fname, 'r'))
> >   File "/tmp/qemu-test/src/scripts/qapi.py", line 301, in __init__
> > previously_included)
> >   File "/tmp/qemu-test/src/scripts/qapi.py", line 348, in _include
> > exprs_include = QAPISchemaParser(fobj, previously_included, info)
> >   File "/tmp/qemu-test/src/scripts/qapi.py", line 271, in __init__
> > self.src = fp.read()
> >   File "/usr/lib64/python3.5/encodings/ascii.py", line 26, in decode
> > return codecs.ascii_decode(input, self.errors)[0]
> > 
> > Many distros support a new C.UTF-8 locale that is like the C locale,
> > but with UTF-8 instead of 7-bit ASCII. That is not entirely portable
> > though, so this patch instead forces the en_US.UTF-8 locale, which
> > is pretty similar but more widely available.
> > 
> > We set LANG, rather than only LC_CTYPE, since generated source ought
> > to be independant of all of the user's locale settings.
> 
> s/independant/independent/
> 
> LANG is the lowest-priority setting - if the user has explicitly set
> LC_CTYPE or LC_ALL, their settings override what is in LANG.
> 
> > 
> > This patch only forces UTF-8 for QAPI scripts, since that is the one
> > showing the immediate error under Python3 with C locale, but potentially
> > we ought to force this for all python scripts used in the build process.
> > 
> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> > ---
> >  Makefile | 22 --
> >  1 file changed, 12 insertions(+), 10 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index d86ecd2dd4..fde91cc42d 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -17,6 +17,8 @@ ifneq ($(wildcard config-host.mak),)
> >  all:
> >  include config-host.mak
> >  
> > +PYTHON_UTF8 = LANG=en_US.UTF-8 $(PYTHON)
> 
> I'm worried that this is not reproducible in the face of a user that
> explicitly sets different locale env-vars with higher priority than LANG.

You might remember a similar issue affecting libvirt-glib/libosinfo when
glib-mkenums was rewritten to use Python instead of Perl. For that I ended
up doing

   LC_ALL= LANG=C LC_CTYPE=en_US.UTF-8 


> > +
> >  git-submodule-update:
> >  
> >  .PHONY: git-submodule-update
> > @@ -471,17 +473,17 @@ qapi-py = $(SRC_PATH)/scripts/qapi.py 
> > $(SRC_PATH)/scripts/ordereddict.py
> >  
> >  qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\
> >  $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py 
> > $(qapi-py)
> > -   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
> > +   $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-types.py \
> 
> But once we agree on the right override to stuff into PYTHON_UTF8, the
> rest of the patch converting invocations to PYTHON_UTF8 makes sense.

Any thoughts on whether we should apply this more widely to our build
to make its output predictable regardless of user's locale ?

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



[Qemu-devel] [PATCH v4 13/13] docker: change Fedora images to run with python3

2018-01-15 Thread Daniel P. Berrange
Fedora has switched to Python 3 by default, so it makes sense to use that
for testing QEMU builds, so we get testing of Python 3 compatibility.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 tests/docker/dockerfiles/fedora.docker | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/docker/dockerfiles/fedora.docker 
b/tests/docker/dockerfiles/fedora.docker
index 4b26c3aded..a22fe16157 100644
--- a/tests/docker/dockerfiles/fedora.docker
+++ b/tests/docker/dockerfiles/fedora.docker
@@ -1,6 +1,6 @@
 FROM fedora:latest
 ENV PACKAGES \
-ccache gettext git tar PyYAML sparse flex bison python2 bzip2 hostname \
+ccache gettext git tar PyYAML sparse flex bison python3 bzip2 hostname \
 glib2-devel pixman-devel zlib-devel SDL-devel libfdt-devel \
 gcc gcc-c++ clang make perl which bc findutils libaio-devel \
 nettle-devel \
@@ -12,6 +12,7 @@ ENV PACKAGES \
 mingw64-gtk2 mingw64-gtk3 mingw64-gnutls mingw64-nettle mingw64-libtasn1 \
 mingw64-libjpeg-turbo mingw64-libpng mingw64-curl mingw64-libssh2 \
 mingw64-bzip2
+ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3
 
 RUN dnf install -y $PACKAGES
 RUN rpm -q $PACKAGES | sort > /packages.txt
-- 
2.14.3




[Qemu-devel] [PATCH v4 11/13] ui: update keycodemapdb to get py3 fixes

2018-01-15 Thread Daniel P. Berrange
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 ui/keycodemapdb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/keycodemapdb b/ui/keycodemapdb
index 05dad417e9..6b3d716e2b 16
--- a/ui/keycodemapdb
+++ b/ui/keycodemapdb
@@ -1 +1 @@
-Subproject commit 05dad417e9d0b37ee1fba33056d91a6b734b3357
+Subproject commit 6b3d716e2b6472eb7189d3220552280ef3d832ce
-- 
2.14.3




[Qemu-devel] [PATCH v4 09/13] configure: allow use of python 3

2018-01-15 Thread Daniel P. Berrange
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 configure | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index b272a0336b..60b99f45f6 100755
--- a/configure
+++ b/configure
@@ -1598,9 +1598,8 @@ fi
 
 # Note that if the Python conditional here evaluates True we will exit
 # with status 1 which is a shell 'false' value.
-if ! $python -c 'import sys; sys.exit(sys.version_info < (2,6) or 
sys.version_info >= (3,))'; then
-  error_exit "Cannot use '$python', Python 2.6 or later is required." \
-  "Note that Python 3 or later is not yet supported." \
+if ! $python -c 'import sys; sys.exit(sys.version_info < (2,6))'; then
+  error_exit "Cannot use '$python', Python 2 >= 2.6 or Python 3 is required." \
   "Use --python=/path/to/python to specify a supported Python."
 fi
 
-- 
2.14.3




[Qemu-devel] [PATCH v4 07/13] qapi: force a UTF-8 locale for running Python

2018-01-15 Thread Daniel P. Berrange
Python2 did not validate locale correctness when reading input data, so
would happily read UTF-8 data in non-UTF-8 locales. Python3 is strict so
if you try to read UTF-8 data in the C locale, it will raise an error
for any UTF-8 bytes that aren't representable in 7-bit ascii encoding.
e.g.

UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 54: 
ordinal not in range(128)
Traceback (most recent call last):
  File "/tmp/qemu-test/src/scripts/qapi-commands.py", line 317, in 
schema = QAPISchema(input_file)
  File "/tmp/qemu-test/src/scripts/qapi.py", line 1468, in __init__
parser = QAPISchemaParser(open(fname, 'r'))
  File "/tmp/qemu-test/src/scripts/qapi.py", line 301, in __init__
previously_included)
  File "/tmp/qemu-test/src/scripts/qapi.py", line 348, in _include
exprs_include = QAPISchemaParser(fobj, previously_included, info)
  File "/tmp/qemu-test/src/scripts/qapi.py", line 271, in __init__
self.src = fp.read()
  File "/usr/lib64/python3.5/encodings/ascii.py", line 26, in decode
return codecs.ascii_decode(input, self.errors)[0]

Many distros support a new C.UTF-8 locale that is like the C locale,
but with UTF-8 instead of 7-bit ASCII. That is not entirely portable
though, so this patch instead forces the en_US.UTF-8 locale, which
is pretty similar but more widely available.

We set LANG, rather than only LC_CTYPE, since generated source ought
to be independant of all of the user's locale settings.

This patch only forces UTF-8 for QAPI scripts, since that is the one
showing the immediate error under Python3 with C locale, but potentially
we ought to force this for all python scripts used in the build process.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 Makefile | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index d86ecd2dd4..fde91cc42d 100644
--- a/Makefile
+++ b/Makefile
@@ -17,6 +17,8 @@ ifneq ($(wildcard config-host.mak),)
 all:
 include config-host.mak
 
+PYTHON_UTF8 = LANG=en_US.UTF-8 $(PYTHON)
+
 git-submodule-update:
 
 .PHONY: git-submodule-update
@@ -471,17 +473,17 @@ qapi-py = $(SRC_PATH)/scripts/qapi.py 
$(SRC_PATH)/scripts/ordereddict.py
 
 qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\
 $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
+   $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-types.py \
$(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \
"GEN","$@")
 qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\
 $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
+   $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-visit.py \
$(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \
"GEN","$@")
 qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\
 $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py 
$(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
+   $(call quiet-command,$(PYTHON_UTF8) 
$(SRC_PATH)/scripts/qapi-commands.py \
$(gen-out-type) -o qga/qapi-generated -p "qga-" $<, \
"GEN","$@")
 
@@ -502,27 +504,27 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json 
$(SRC_PATH)/qapi/common.json \
 
 qapi-types.c qapi-types.h :\
 $(qapi-modules) $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
+   $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-types.py \
$(gen-out-type) -o "." -b $<, \
"GEN","$@")
 qapi-visit.c qapi-visit.h :\
 $(qapi-modules) $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
+   $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-visit.py \
$(gen-out-type) -o "." -b $<, \
"GEN","$@")
 qapi-event.c qapi-event.h :\
 $(qapi-modules) $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py \
+   $(call quiet-command,$(PYTHON_UTF8) $(SRC_PATH)/scripts/qapi-event.py \
$(gen-out-type) -o "." $<, \
"GEN","$@")
 qmp-commands.h qmp-marshal.c :\
 $(qapi-modules) $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
-   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
+   $(call quiet-comma

[Qemu-devel] [PATCH v4 05/13] qapi: remove '-q' arg to diff when comparing QAPI output

2018-01-15 Thread Daniel P. Berrange
When the qapi schema tests fail they merely print that the expected
output didn't match the actual output. This is largely useless when
trying diagnose what went wrong. Removing the '-q' arg to diff
means that it is still silent on successful tests, but when it
fails we'll see details of the incorrect output.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 tests/Makefile.include | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 39a4b5359d..d65fb4e1b3 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -908,10 +908,10 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): 
check-%.json: $(SRC_PATH)/%.json
$^ >$*.test.out 2>$*.test.err; \
echo $$? >$*.test.exit, \
"TEST","$*.out")
-   @diff -q $(SRC_PATH)/$*.out $*.test.out
+   @diff $(SRC_PATH)/$*.out $*.test.out
@# Sanitize error messages (make them independent of build directory)
-   @perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff -q 
$(SRC_PATH)/$*.err -
-   @diff -q $(SRC_PATH)/$*.exit $*.test.exit
+   @perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff 
$(SRC_PATH)/$*.err -
+   @diff $(SRC_PATH)/$*.exit $*.test.exit
 
 .PHONY: check-tests/qapi-schema/doc-good.texi
 check-tests/qapi-schema/doc-good.texi: tests/qapi-schema/doc-good.test.texi
-- 
2.14.3




[Qemu-devel] [PATCH v4 10/13] input: add missing JIS keys to virtio input

2018-01-15 Thread Daniel P. Berrange
From: Miika S 

keycodemapdb updated to add the QKeyCodes muhenkan and katakanahiragana

Signed-off-by: Miika S 
---
 hw/input/virtio-input-hid.c | 7 +++
 qapi/ui.json| 5 -
 ui/keycodemapdb | 2 +-
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/input/virtio-input-hid.c b/hw/input/virtio-input-hid.c
index e78faec0b1..9628d289f9 100644
--- a/hw/input/virtio-input-hid.c
+++ b/hw/input/virtio-input-hid.c
@@ -139,6 +139,13 @@ static const unsigned int keymap_qcode[Q_KEY_CODE__MAX] = {
 [Q_KEY_CODE_META_L]  = KEY_LEFTMETA,
 [Q_KEY_CODE_META_R]  = KEY_RIGHTMETA,
 [Q_KEY_CODE_MENU]= KEY_MENU,
+
+[Q_KEY_CODE_MUHENKAN]= KEY_MUHENKAN,
+[Q_KEY_CODE_HENKAN]  = KEY_HENKAN,
+[Q_KEY_CODE_KATAKANAHIRAGANA]= KEY_KATAKANAHIRAGANA,
+[Q_KEY_CODE_COMPOSE] = KEY_COMPOSE,
+[Q_KEY_CODE_RO]  = KEY_RO,
+[Q_KEY_CODE_YEN] = KEY_YEN,
 };
 
 static const unsigned int keymap_button[INPUT_BUTTON__MAX] = {
diff --git a/qapi/ui.json b/qapi/ui.json
index 07b468f625..d6679aa8f5 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -748,6 +748,9 @@
 # @ac_bookmarks: since 2.10
 # altgr, altgr_r: dropped in 2.10
 #
+# @muhenkan: since 2.12
+# @katakanahiragana: since 2.12
+#
 # 'sysrq' was mistakenly added to hack around the fact that
 # the ps2 driver was not generating correct scancodes sequences
 # when 'alt+print' was pressed. This flaw is now fixed and the
@@ -775,7 +778,7 @@
 'left', 'up', 'down', 'right', 'insert', 'delete', 'stop', 'again',
 'props', 'undo', 'front', 'copy', 'open', 'paste', 'find', 'cut',
 'lf', 'help', 'meta_l', 'meta_r', 'compose', 'pause',
-'ro', 'hiragana', 'henkan', 'yen',
+'ro', 'hiragana', 'henkan', 'yen', 'muhenkan', 'katakanahiragana',
 'kp_comma', 'kp_equals', 'power', 'sleep', 'wake',
 'audionext', 'audioprev', 'audiostop', 'audioplay', 'audiomute',
 'volumeup', 'volumedown', 'mediaselect',
diff --git a/ui/keycodemapdb b/ui/keycodemapdb
index 10739aa260..05dad417e9 16
--- a/ui/keycodemapdb
+++ b/ui/keycodemapdb
@@ -1 +1 @@
-Subproject commit 10739aa26051a5d49d88132604539d3ed085e72e
+Subproject commit 05dad417e9d0b37ee1fba33056d91a6b734b3357
-- 
2.14.3




[Qemu-devel] [PATCH v4 04/13] qapi: Adapt to moved location of 'maketrans' function in py3

2018-01-15 Thread Daniel P. Berrange
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/qapi.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 514cca44bf..1fdd189c0d 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1734,7 +1734,10 @@ def c_enum_const(type_name, const_name, prefix=None):
 type_name = prefix
 return camel_to_upper(type_name) + '_' + c_name(const_name, False).upper()
 
-c_name_trans = string.maketrans('.-', '__')
+if hasattr(str, 'maketrans'):
+c_name_trans = str.maketrans('.-', '__')
+else:
+c_name_trans = string.maketrans('.-', '__')
 
 
 # Map @name to a valid C identifier.
-- 
2.14.3




[Qemu-devel] [PATCH v4 08/13] scripts: ensure signrom treats data as bytes

2018-01-15 Thread Daniel P. Berrange
Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/signrom.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/signrom.py b/scripts/signrom.py
index d1dabe0240..0497a1c32e 100644
--- a/scripts/signrom.py
+++ b/scripts/signrom.py
@@ -18,7 +18,7 @@ fin = open(sys.argv[1], 'rb')
 fout = open(sys.argv[2], 'wb')
 
 magic = fin.read(2)
-if magic != '\x55\xaa':
+if magic != b'\x55\xaa':
 sys.exit("%s: option ROM does not begin with magic 55 aa" % sys.argv[1])
 
 size_byte = ord(fin.read(1))
@@ -33,7 +33,7 @@ elif len(data) < size:
 # Add padding if necessary, rounding the whole input to a multiple of
 # 512 bytes according to the third byte of the input.
 # size-1 because a final byte is added below to store the checksum.
-data = data.ljust(size-1, '\0')
+data = data.ljust(size-1, b'\0')
 else:
 if ord(data[-1:]) != 0:
 sys.stderr.write('WARNING: ROM includes nonzero checksum\n')
-- 
2.14.3




[Qemu-devel] [PATCH v4 03/13] qapi: adapt to moved location of StringIO module in py3

2018-01-15 Thread Daniel P. Berrange
Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/qapi.py | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 514b7bb5a4..514cca44bf 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -22,6 +22,10 @@ try:
 from collections import OrderedDict
 except:
 from ordereddict import OrderedDict
+try:
+from StringIO import StringIO
+except ImportError:
+from io import StringIO
 
 builtin_types = {
 'null': 'QTYPE_QNULL',
@@ -1995,8 +1999,7 @@ def open_output(output_dir, do_c, do_h, prefix, c_file, 
h_file,
 if really:
 return open(name, opt)
 else:
-import StringIO
-return StringIO.StringIO()
+return StringIO()
 
 fdef = maybe_open(do_c, c_file, 'w')
 fdecl = maybe_open(do_h, h_file, 'w')
-- 
2.14.3




[Qemu-devel] [PATCH v4 06/13] qapi: ensure stable sort ordering when checking QAPI entities

2018-01-15 Thread Daniel P. Berrange
Some early python 3.x versions will have different default
ordering when calling the 'values()' method on a dict, compared
to python 2.x and later 3.x versions. Explicitly sort the items
to get a stable ordering.

Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/qapi.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 1fdd189c0d..58f995b07f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1678,7 +1678,7 @@ class QAPISchema(object):
 assert False
 
 def check(self):
-for ent in self._entity_dict.values():
+for (name, ent) in sorted(self._entity_dict.items()):
 ent.check(self)
 
 def visit(self, visitor):
-- 
2.14.3




[Qemu-devel] [PATCH v4 12/13] travis: improve python version test coverage

2018-01-15 Thread Daniel P. Berrange
Currently travis declares ancient python 2.4 is desired. Update that to
2.6 which is the oldest version any targetted distros still needs. If we
just list a python 3 version at the top level this will double the
number of travis jobs we run which is unreasonable.

So arbitrarily pick the clang test matrix entries to build with python
3.0 and 3.6, to extend coverage of python versions, without increasing
job count or build time.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 .travis.yml | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index f583839755..708c886017 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,7 +1,7 @@
 sudo: false
 language: c
 python:
-  - "2.4"
+  - "2.6"
 compiler:
   - gcc
 cache: ccache
@@ -115,15 +115,17 @@ matrix:
 - sudo apt-get build-dep -qq qemu
 - wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
 - git submodule update --init --recursive
-# Trusty System build with latest stable clang
+# Trusty System build with latest stable clang & python 3.0
 - sudo: required
   addons:
   dist: trusty
   language: generic
   compiler: none
+  python:
+- "3.0"
   env:
 - COMPILER_NAME=clang CXX=clang++-3.9 CC=clang-3.9
-- CONFIG="--disable-linux-user --cc=clang-3.9 --cxx=clang++-3.9"
+- CONFIG="--disable-linux-user --cc=clang-3.9 --cxx=clang++-3.9 
--python=/usr/bin/python3"
   before_install:
 - wget -nv -O - http://llvm.org/apt/llvm-snapshot.gpg.key | sudo 
apt-key add -
 - sudo apt-add-repository -y 'deb http://llvm.org/apt/trusty 
llvm-toolchain-trusty-3.9 main'
@@ -134,15 +136,17 @@ matrix:
 - git submodule update --init --recursive
   before_script:
 - ./configure ${CONFIG} || cat config.log
-# Trusty Linux User build with latest stable clang
+# Trusty Linux User build with latest stable clang & python 3.6
 - sudo: required
   addons:
   dist: trusty
   language: generic
   compiler: none
+  python:
+- "3.6"
   env:
 - COMPILER_NAME=clang CXX=clang++-3.9 CC=clang-3.9
-- CONFIG="--disable-system --cc=clang-3.9 --cxx=clang++-3.9"
+- CONFIG="--disable-system --cc=clang-3.9 --cxx=clang++-3.9 
--python=/usr/bin/python3"
   before_install:
 - wget -nv -O - http://llvm.org/apt/llvm-snapshot.gpg.key | sudo 
apt-key add -
 - sudo apt-add-repository -y 'deb http://llvm.org/apt/trusty 
llvm-toolchain-trusty-3.9 main'
-- 
2.14.3




[Qemu-devel] [PATCH v4 01/13] qapi: use items()/values() intead of iteritems()/itervalues()

2018-01-15 Thread Daniel P. Berrange
The iteritems()/itervalues() methods are gone in py3, but the
items()/values() methods are still around. The latter are less
efficient than the former in py2, but this has unmeasurably
small impact on QEMU build time, so taking portability over
efficiency is a net win.

Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/qapi.py| 24 +++
 scripts/qapi2texi.py   | 11 ++-
 tests/qapi-schema/test-qapi.py | 43 +-
 3 files changed, 40 insertions(+), 38 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 43a54bf40f..98d7123d27 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -11,6 +11,7 @@
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
+from __future__ import print_function
 import errno
 import getopt
 import os
@@ -244,7 +245,7 @@ class QAPIDoc(object):
"'Returns:' is only valid for commands")
 
 def check(self):
-bogus = [name for name, section in self.args.iteritems()
+bogus = [name for name, section in self.args.items()
  if not section.member]
 if bogus:
 raise QAPISemError(
@@ -299,7 +300,7 @@ class QAPISchemaParser(object):
 if not isinstance(pragma, dict):
 raise QAPISemError(
 info, "Value of 'pragma' must be a dictionary")
-for name, value in pragma.iteritems():
+for name, value in pragma.items():
 self._pragma(name, value, info)
 else:
 expr_elem = {'expr': expr,
@@ -1467,7 +1468,7 @@ class QAPISchema(object):
 self._def_exprs()
 self.check()
 except QAPIError as err:
-print >>sys.stderr, err
+print(err, file=sys.stderr)
 exit(1)
 
 def _def_entity(self, ent):
@@ -1565,7 +1566,7 @@ class QAPISchema(object):
 
 def _make_members(self, data, info):
 return [self._make_member(key, value, info)
-for (key, value) in data.iteritems()]
+for (key, value) in data.items()]
 
 def _def_struct_type(self, expr, info, doc):
 name = expr['struct']
@@ -1597,11 +1598,11 @@ class QAPISchema(object):
 name, info, doc, 'base', self._make_members(base, info)))
 if tag_name:
 variants = [self._make_variant(key, value)
-for (key, value) in data.iteritems()]
+for (key, value) in data.items()]
 members = []
 else:
 variants = [self._make_simple_variant(key, value, info)
-for (key, value) in data.iteritems()]
+for (key, value) in data.items()]
 typ = self._make_implicit_enum_type(name, info,
 [v.name for v in variants])
 tag_member = QAPISchemaObjectTypeMember('type', typ, False)
@@ -1616,7 +1617,7 @@ class QAPISchema(object):
 name = expr['alternate']
 data = expr['data']
 variants = [self._make_variant(key, value)
-for (key, value) in data.iteritems()]
+for (key, value) in data.items()]
 tag_member = QAPISchemaObjectTypeMember('type', 'QType', False)
 self._def_entity(
 QAPISchemaAlternateType(name, info, doc,
@@ -1931,7 +1932,7 @@ def parse_command_line(extra_options='', 
extra_long_options=[]):
['source', 'header', 'prefix=',
 'output-dir='] + extra_long_options)
 except getopt.GetoptError as err:
-print >>sys.stderr, "%s: %s" % (sys.argv[0], str(err))
+print("%s: %s" % (sys.argv[0], str(err)), file=sys.stderr)
 sys.exit(1)
 
 output_dir = ''
@@ -1945,9 +1946,8 @@ def parse_command_line(extra_options='', 
extra_long_options=[]):
 if o in ('-p', '--prefix'):
 match = re.match(r'([A-Za-z_.-][A-Za-z0-9_.-]*)?', a)
 if match.end() != len(a):
-print >>sys.stderr, \
-"%s: 'funny character '%s' in argument of --prefix" \
-% (sys.argv[0], a[match.end()])
+print("%s: 'funny character '%s' in argument of --prefix" \
+  % (sys.argv[0], a[match.end()]), file=sys.stderr)
 sys.exit(1)
 prefix = a
 elif o in ('-o', '--output-dir'):
@@ -1964,7 +1964,7 @@ def parse_command_line(extra_options='', 
extra_long_options=[]):
 do_h = True
 
 if len(args) != 1:
-print >>sys.stderr, "%s: need exactly one argument" % sys.argv[0]
+   

[Qemu-devel] [PATCH v4 02/13] qapi: Use OrderedDict from standard library if available

2018-01-15 Thread Daniel P. Berrange
The OrderedDict class appeared in the 'collections' module
from python 2.7 onwards, so use that in preference to our
local backport if available.

Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/qapi.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 98d7123d27..514b7bb5a4 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -18,7 +18,10 @@ import os
 import re
 import string
 import sys
-from ordereddict import OrderedDict
+try:
+from collections import OrderedDict
+except:
+from ordereddict import OrderedDict
 
 builtin_types = {
 'null': 'QTYPE_QNULL',
-- 
2.14.3




[Qemu-devel] [PATCH v4 00/13] Support building with py2 or py3

2018-01-15 Thread Daniel P. Berrange
This is an update for my previously posted series:

 v2: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg06528.html

This series enables some level of CI testing for py3 so that our CI jobs will
get coverage of both py2 and py3 builds to avoid bitrot.

I did a test travis build with py 3.0 and py 3.6 and got success:

  https://travis-ci.org/berrange/qemu/builds/328223261

The goal was to achieve the following

  ./configure --python=/usr/bin/python3
  make
  make check

This still requires passing python path to configure explicitly. A
further improvement would be for configure to automatically detect
a pythjon 3 binary and use it preferentially to python 2.

I have not attempted to fix/validate the block I/O tests. I would expect
them to be broken, but easily fixable with the similar kind of scope
changes as seen here. I felt it better to tackle that separately to
avoid this initial series getting too large.

Although the Python 2 EOL date is 2020, we already have distros which
are not shipping Python 2 by default (Fedora >= 26 has dropped Py2 from
the default install). Any new releases of long life and/or enterprise
distros may well not ship Python 2 given that it would go EOL long
before the EOL of the distro itself. IOW QEMU does have a fairly pressing
need to be able to support Python 3 for building.

A request for py3 is tracked here:

   https://bugs.launchpad.net/qemu/+bug/1708462

NB, Patch 10 here is not related to python 3 work - it was just a
temporary pre-requisite of pulling in the keycodemapdb update.

Changes since v3:

 - Remove space before '(' in print() function calls (Phillippe)
 - Force use of en_US.UTF-8 for QAPI code generation (Patchew)

Changes since v2:

 - Pull in fix for keycodemapdb
 - Enable testing with Travis
 - Enable testing with Fedora Docker images
 - Fix for sort ordering to fix 'make check-qapi-schema'
 - Fix for signrom data

Daniel P. Berrange (12):
  qapi: use items()/values() intead of iteritems()/itervalues()
  qapi: Use OrderedDict from standard library if available
  qapi: adapt to moved location of StringIO module in py3
  qapi: Adapt to moved location of 'maketrans' function in py3
  qapi: remove '-q' arg to diff when comparing QAPI output
  qapi: ensure stable sort ordering when checking QAPI entities
  qapi: force a UTF-8 locale for running Python
  scripts: ensure signrom treats data as bytes
  configure: allow use of python 3
  ui: update keycodemapdb to get py3 fixes
  travis: improve python version test coverage
  docker: change Fedora images to run with python3

Miika S (1):
  input: add missing JIS keys to virtio input

 .travis.yml| 14 +++
 Makefile   | 22 +
 configure  |  5 ++--
 hw/input/virtio-input-hid.c|  7 ++
 qapi/ui.json   |  5 +++-
 scripts/qapi.py| 43 --
 scripts/qapi2texi.py   | 11 +
 scripts/signrom.py |  4 ++--
 tests/Makefile.include |  6 ++---
 tests/docker/dockerfiles/fedora.docker |  3 ++-
 tests/qapi-schema/test-qapi.py | 43 +-
 ui/keycodemapdb|  2 +-
 12 files changed, 96 insertions(+), 69 deletions(-)

-- 
2.14.3




Re: [Qemu-devel] [PATCH v2 6/8] docker: add the Fedora 27 base image

2018-01-15 Thread Daniel P. Berrange
On Mon, Jan 15, 2018 at 11:34:57AM -0300, Philippe Mathieu-Daudé wrote:
> straight copy on Fedora 25 base.
> 
> Suggested-by: Paolo Bonzini 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/docker/dockerfiles/fedora27.docker | 18 ++
>  1 file changed, 18 insertions(+)
>  create mode 100644 tests/docker/dockerfiles/fedora27.docker
> 
> diff --git a/tests/docker/dockerfiles/fedora27.docker 
> b/tests/docker/dockerfiles/fedora27.docker
> new file mode 100644
> index 00..7cba4dcc2c
> --- /dev/null
> +++ b/tests/docker/dockerfiles/fedora27.docker
> @@ -0,0 +1,18 @@
> +FROM fedora:27
> +ENV PACKAGES \
> +ccache gettext git tar PyYAML sparse flex bison python2 bzip2 hostname \
> +glib2-devel pixman-devel zlib-devel SDL-devel libfdt-devel \

Perhaps SDL2 here too

> +gcc gcc-c++ clang make perl which bc findutils libaio-devel \
> +nettle-devel \
> +mingw32-pixman mingw32-glib2 mingw32-gmp mingw32-SDL mingw32-pkg-config \
> +mingw32-gtk2 mingw32-gtk3 mingw32-gnutls mingw32-nettle mingw32-libtasn1 
> \

Is there a reason we have both GTK2 and GTK3 installed ? We've deprecated GTK2
and configure will pick GTK3 by default. IIUC there's no way to override
that in the docker builds to force GTK2, so it can possibly be dropped.

> +mingw32-libjpeg-turbo mingw32-libpng mingw32-curl mingw32-libssh2 \
> +mingw32-bzip2 \
> +mingw64-pixman mingw64-glib2 mingw64-gmp mingw64-SDL mingw64-pkg-config \
> +mingw64-gtk2 mingw64-gtk3 mingw64-gnutls mingw64-nettle mingw64-libtasn1 
> \

Repeated here.

> +mingw64-libjpeg-turbo mingw64-libpng mingw64-curl mingw64-libssh2 \
> +mingw64-bzip2
> +
> +RUN dnf install -y $PACKAGES
> +RUN rpm -q $PACKAGES | sort > /packages.txt
> +ENV FEATURES mingw clang pyyaml

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v2 3/8] docker: add the Ubuntu Trusty base image

2018-01-15 Thread Daniel P. Berrange
On Mon, Jan 15, 2018 at 11:34:54AM -0300, Philippe Mathieu-Daudé wrote:
> based on QEMU v2.10 ubuntu.docker (ca853f0c76e3 and 2346b12fc52d)
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/docker/dockerfiles/ubuntu14.04.docker | 17 +
>  1 file changed, 17 insertions(+)
>  create mode 100644 tests/docker/dockerfiles/ubuntu14.04.docker
> 
> diff --git a/tests/docker/dockerfiles/ubuntu14.04.docker 
> b/tests/docker/dockerfiles/ubuntu14.04.docker
> new file mode 100644
> index 00..cd24e759a7
> --- /dev/null
> +++ b/tests/docker/dockerfiles/ubuntu14.04.docker
> @@ -0,0 +1,17 @@
> +FROM ubuntu:14.04
> +RUN apt-get update
> +ENV PACKAGES flex bison \
> +libusb-1.0-0-dev libiscsi-dev librados-dev libncurses5-dev 
> libncursesw5-dev \
> +libseccomp-dev libgnutls-dev libssh2-1-dev  libspice-server-dev \
> +libspice-protocol-dev libnss3-dev libfdt-dev \
> +libgtk-3-dev libvte-2.90-dev libsdl1.2-dev libpng12-dev libpixman-1-dev \

Perhaps use SDL 2, instead of 1.2 series, since we're deprecating SDL 1.2
in this release...

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH 0/3] input: add keys and mouse buttons to virtio input

2018-01-15 Thread Daniel P. Berrange
On Mon, Jan 15, 2018 at 04:18:53PM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > Miika S (3):
> >   input: add mouse side buttons to virtio input
> >   input: virtio: don't send mouse wheel event twice
> 
> Cherry-picked these two ad the kbd update depends on the not-yet merged
> keycodemapdb update for virtio-keyboard.

It is merged actually in, so you can include the keycode fix too:

https://git.qemu.org/gitweb.cgi?p=keycodemapdb.git;a=commit;h=05dad417e9d0b37ee1fba33056d91a6b734b3357

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



[Qemu-devel] [PATCH] ui: deprecate use of SDL 1.2 in favour of 2.0 series

2018-01-15 Thread Daniel P. Berrange
The SDL 2.0 release was made in Aug, 2013:

  https://www.libsdl.org/release/

That will soon be 4 + 1/2 years ago, which is enough time to consider
the 2.0 series widely supported.

Thus we deprecate the SDL 1.2 support, which will allow us to delete it
in the last release of 2018. By this time, SDL 2.0 will be more than 5
years old.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 configure | 6 ++
 qemu-doc.texi | 7 +++
 ui/sdl.c  | 3 +++
 3 files changed, 16 insertions(+)

diff --git a/configure b/configure
index b272a0336b..dc2615a0cb 100755
--- a/configure
+++ b/configure
@@ -5635,6 +5635,12 @@ if test "$gtkabi" = "2.0"; then
 echo "WARNING: future releases. Please switch to using GTK 3.0"
 fi
 
+if test "$sdlabi" = "1.2"; then
+echo
+echo "WARNING: Use of SDL 1.2 is deprecated and will be removed in"
+echo "WARNING: future releases. Please switch to using SDL 2.0"
+fi
+
 if test "$supported_cpu" = "no"; then
 echo
 echo "WARNING: SUPPORT FOR THIS HOST CPU WILL GO AWAY IN FUTURE RELEASES!"
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 3e9eb819a6..3d2b3ff1ea 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2596,6 +2596,13 @@ and 3.x series APIs. Support for the GTK 2.x builds will 
be
 discontinued, so maintainers should switch to using GTK 3.x,
 which is the default.
 
+@subsection SDL 1.2
+
+Previously QEMU has supported building against both SDL 1.2
+and 2.0 series APIs. Support for the SDL 1.2 builds will be
+discontinued, so maintainers should switch to using SDL 2.0,
+which is the default.
+
 @section System emulator command line arguments
 
 @subsection -tdf (since 1.3.0)
diff --git a/ui/sdl.c b/ui/sdl.c
index 7b71a9ac58..8a93054fd4 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -963,6 +963,9 @@ void sdl_display_init(DisplayState *ds, int full_screen, 
int no_frame)
 exit(1);
 }
 
+g_printerr("Running QEMU with SDL 1.2 is deprecated, and will be removed\n"
+   "in a future release. Please switch to SDL 2.0 instead\n");
+
 if (no_frame)
 gui_noframe = 1;
 
-- 
2.14.3




Re: [Qemu-devel] [RFC PATCH 02/10] block/qapi: Add qcow2 create options to schema

2018-01-15 Thread Daniel P. Berrange
On Mon, Jan 15, 2018 at 03:07:15PM +0100, Kevin Wolf wrote:
> Am 15.01.2018 um 14:51 hat Daniel P. Berrange geschrieben:
> > On Mon, Jan 15, 2018 at 02:38:48PM +0100, Kevin Wolf wrote:
> > > Am 12.01.2018 um 11:53 hat Daniel P. Berrange geschrieben:
> > > > On Thu, Jan 11, 2018 at 08:52:17PM +0100, Kevin Wolf wrote:
> > > > > Signed-off-by: Kevin Wolf <kw...@redhat.com>
> > > > > ---
> > > > >  qapi/block-core.json | 33 -
> > > > >  1 file changed, 32 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > > > index 1749376c61..9341f6708d 100644
> > > > > --- a/qapi/block-core.json
> > > > > +++ b/qapi/block-core.json
> > > > > @@ -3320,6 +3320,37 @@
> > > > >  { 'command': 'blockdev-del', 'data': { 'node-name': 'str' } }
> > > > >  
> > > > >  ##
> > > > > +# @BlockdevQcow2CompatLevel:
> > > > > +# @0_10:The original QCOW2 format as introduced in qemu 0.10 
> > > > > (version 2)
> > > > > +# @1_1: The extended QCOW2 format as introduced in qemu 1.1 
> > > > > (version 3)
> > > > > +#
> > > > > +# Since: 2.10
> > > > > +##
> > > > > +{ 'enum': 'BlockdevQcow2CompatLevel',
> > > > > +  'data': [ '0_10', '1_1' ] }
> > > > > +
> > > > > +
> > > > > +##
> > > > > +# @BlockdevCreateOptionsQcow2:
> > > > > +#
> > > > > +# Driver specific image creation options for qcow2.
> > > > > +#
> > > > > +# TODO Describe fields
> > > > > +#
> > > > > +# Since: 2.12
> > > > > +##
> > > > > +{ 'struct': 'BlockdevCreateOptionsQcow2',
> > > > > +  'data': { 'size': 'size',
> > > > > +'*compat':  'BlockdevQcow2CompatLevel',
> > > > > +'*backing-file':'str',
> > > > > +'*backing-fmt': 'BlockdevDriver',
> > > > 
> > > > For anything non-trivial, the caller is going to have to stuff a
> > > > JSON string into 'backing-file' value. It feels like we should
> > > > be referencing 'BlockdevOptions' here in some manner.
> > > 
> > > Hm, that's an interesting question. For the image creation, this is
> > > really treated as a string that is directly written into the image file,
> > > without being parsed, so 'str' is the more correct type in this context.
> > > However, when the backing file gets loaded, that string is in fact
> > > parsed and we expect it to describe the same thing as BlockdevOptions.
> > > 
> > > If we get BlockdevOptions here, qemu would have to convert them into a
> > > json:{...} string before writing the header of the new image.
> > > Compatibility code would become a bit more complex because we'd have to
> > > convert the existing string into BlockdevOptions, only to convert it
> > > back to a string before we write it to the image file. And finally, the
> > > 1023 character limit of qcow2 becomes kind of unpredicatble when you
> > > don't pass the string yourself.
> > > 
> > > So considering all of that, I still think that 'str' is the better
> > > option here.
> > 
> > Hmm, when we write the backing chain into the qcow2 header, we only
> > want to write the 1st level of the backing chain.
> 
> That's a good point, too. References in BlockdevOptions are often
> mandatory, which conflicts with this.
> 
> > When we are creating the new qcow2 image, we could be pointing to a backing
> > chain that goes many levels deep. So the actual creation process potentially
> > needs to be given the full arbitrarily deep backing file eg in order that
> > we can set 'encrypt.secret' for any encrypted images at at arbitrary level.
> 
> But we don't even access the images in the backing chain during image
> creation. Why would we need a secret for them?

Oh, i forgot that when qcow2 opens the just created image, it uses the
O_NO_IO and O_NO_BACKING flags. So yeah, we're probably ok in actual
fact.

> > IOW, I think we need to be able to pass BlockdevOptions here to specify the
> > full deep chain, but then only the 1st level of these BlockdevOptions should
> > get written into the qcow2 file header.
> 
> But what's the point of even passing the full chain if only the first
> layer is actually used?


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [RFC PATCH 02/10] block/qapi: Add qcow2 create options to schema

2018-01-15 Thread Daniel P. Berrange
On Mon, Jan 15, 2018 at 02:38:48PM +0100, Kevin Wolf wrote:
> Am 12.01.2018 um 11:53 hat Daniel P. Berrange geschrieben:
> > On Thu, Jan 11, 2018 at 08:52:17PM +0100, Kevin Wolf wrote:
> > > Signed-off-by: Kevin Wolf <kw...@redhat.com>
> > > ---
> > >  qapi/block-core.json | 33 -
> > >  1 file changed, 32 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > index 1749376c61..9341f6708d 100644
> > > --- a/qapi/block-core.json
> > > +++ b/qapi/block-core.json
> > > @@ -3320,6 +3320,37 @@
> > >  { 'command': 'blockdev-del', 'data': { 'node-name': 'str' } }
> > >  
> > >  ##
> > > +# @BlockdevQcow2CompatLevel:
> > > +# @0_10:The original QCOW2 format as introduced in qemu 0.10 
> > > (version 2)
> > > +# @1_1: The extended QCOW2 format as introduced in qemu 1.1 (version 
> > > 3)
> > > +#
> > > +# Since: 2.10
> > > +##
> > > +{ 'enum': 'BlockdevQcow2CompatLevel',
> > > +  'data': [ '0_10', '1_1' ] }
> > > +
> > > +
> > > +##
> > > +# @BlockdevCreateOptionsQcow2:
> > > +#
> > > +# Driver specific image creation options for qcow2.
> > > +#
> > > +# TODO Describe fields
> > > +#
> > > +# Since: 2.12
> > > +##
> > > +{ 'struct': 'BlockdevCreateOptionsQcow2',
> > > +  'data': { 'size': 'size',
> > > +'*compat':  'BlockdevQcow2CompatLevel',
> > > +'*backing-file':'str',
> > > +'*backing-fmt': 'BlockdevDriver',
> > 
> > For anything non-trivial, the caller is going to have to stuff a
> > JSON string into 'backing-file' value. It feels like we should
> > be referencing 'BlockdevOptions' here in some manner.
> 
> Hm, that's an interesting question. For the image creation, this is
> really treated as a string that is directly written into the image file,
> without being parsed, so 'str' is the more correct type in this context.
> However, when the backing file gets loaded, that string is in fact
> parsed and we expect it to describe the same thing as BlockdevOptions.
> 
> If we get BlockdevOptions here, qemu would have to convert them into a
> json:{...} string before writing the header of the new image.
> Compatibility code would become a bit more complex because we'd have to
> convert the existing string into BlockdevOptions, only to convert it
> back to a string before we write it to the image file. And finally, the
> 1023 character limit of qcow2 becomes kind of unpredicatble when you
> don't pass the string yourself.
> 
> So considering all of that, I still think that 'str' is the better
> option here.

Hmm, when we write the backing chain into the qcow2 header, we only want to
write the 1st level of the backing chain.

When we are creating the new qcow2 image, we could be pointing to a backing
chain that goes many levels deep. So the actual creation process potentially
needs to be given the full arbitrarily deep backing file eg in order that
we can set 'encrypt.secret' for any encrypted images at at arbitrary level.

IOW, I think we need to be able to pass BlockdevOptions here to specify the
full deep chain, but then only the 1st level of these BlockdevOptions should
get written into the qcow2 file header.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



[Qemu-devel] RFE for patchew docker result reporting

2018-01-15 Thread Daniel P. Berrange
Currently if I look at the patchew website for build logs, the 'docker'
job results are listed as a single expandable item.

Patchew runs 3 separate docker builds, however, and there's a tonne of
output to scroll through to find which one actually failed.

Thus, my RFE is to split the build log up so that instead of a single
"docker" result, it shows  "docker quick", "docker biuld" and
"docker mingw" as separate build logs / results. This would make it
easier to see at a glance which scenario failed.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH v3 13/13] docker: change Fedora images to run with python3

2018-01-15 Thread Daniel P. Berrange
On Mon, Jan 15, 2018 at 07:47:29AM -0300, Philippe Mathieu-Daudé wrote:
> Hi Daniel,
> 
> On 01/15/2018 07:26 AM, Daniel P. Berrange wrote:
> > Fedora has switched to Python 3 by default, so it makes sense to use that
> > for testing QEMU builds, so we get testing of Python 3 compatibility.
> 
> I'd rather keep Fedora 25 with Python 2, and use Python 3 in Fedora 27.

FYI, Fedora 25 went end of life 2 weeks ago, so I'm not sure it is worth
keeping that around in QEMU CI all that much much longer.

To your general point though - I just picked Fedora since all non-EOL
versions of Fedora now ship with Python3 only by default. I'm happy to
see any arbitrary combo of Docker images changed, as long as we get a
nice mixture of python 2/3 versions covered across the set. So feel free
to suggest anything that gets us good coverage of 2+3.

> If you agree, I can apply this patch once your series is merged, on top
> of http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg02569.html
> 
> relevant patches:
> http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg02574.html
> http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg02575.html
> 
> > 
> > Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> > ---
> >  tests/docker/dockerfiles/fedora.docker | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/docker/dockerfiles/fedora.docker 
> > b/tests/docker/dockerfiles/fedora.docker
> > index 4b26c3aded..a22fe16157 100644
> > --- a/tests/docker/dockerfiles/fedora.docker
> > +++ b/tests/docker/dockerfiles/fedora.docker
> > @@ -1,6 +1,6 @@
> >  FROM fedora:latest
> >  ENV PACKAGES \
> > -ccache gettext git tar PyYAML sparse flex bison python2 bzip2 hostname 
> > \
> > +ccache gettext git tar PyYAML sparse flex bison python3 bzip2 hostname 
> > \
> >  glib2-devel pixman-devel zlib-devel SDL-devel libfdt-devel \
> >  gcc gcc-c++ clang make perl which bc findutils libaio-devel \
> >  nettle-devel \
> > @@ -12,6 +12,7 @@ ENV PACKAGES \
> >  mingw64-gtk2 mingw64-gtk3 mingw64-gnutls mingw64-nettle 
> > mingw64-libtasn1 \
> >  mingw64-libjpeg-turbo mingw64-libpng mingw64-curl mingw64-libssh2 \
> >  mingw64-bzip2
> > +ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3
> >  
> >  RUN dnf install -y $PACKAGES
> >  RUN rpm -q $PACKAGES | sort > /packages.txt
> > 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [Qemu-devel] [PATCH 1/3] input: add missing JIS keys to virtio input

2018-01-15 Thread Daniel P. Berrange
On Fri, Dec 22, 2017 at 05:25:29PM +0200, Miika S wrote:
> keycodemapdb updated to add the QKeyCodes muhenkan and katakanahiragana
> 
> Signed-off-by: Miika S <miika9...@gmail.com>
> ---
>  hw/input/virtio-input-hid.c | 7 +++
>  qapi/ui.json| 5 -
>  ui/keycodemapdb | 2 +-
>  3 files changed, 12 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrange <berra...@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



[Qemu-devel] [PATCH v3 11/13] ui: update keycodemapdb to get py3 fixes

2018-01-15 Thread Daniel P. Berrange
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 ui/keycodemapdb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/keycodemapdb b/ui/keycodemapdb
index 05dad417e9..6b3d716e2b 16
--- a/ui/keycodemapdb
+++ b/ui/keycodemapdb
@@ -1 +1 @@
-Subproject commit 05dad417e9d0b37ee1fba33056d91a6b734b3357
+Subproject commit 6b3d716e2b6472eb7189d3220552280ef3d832ce
-- 
2.14.3




[Qemu-devel] [PATCH v3 09/13] configure: allow use of python 3

2018-01-15 Thread Daniel P. Berrange
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 configure | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index b272a0336b..60b99f45f6 100755
--- a/configure
+++ b/configure
@@ -1598,9 +1598,8 @@ fi
 
 # Note that if the Python conditional here evaluates True we will exit
 # with status 1 which is a shell 'false' value.
-if ! $python -c 'import sys; sys.exit(sys.version_info < (2,6) or 
sys.version_info >= (3,))'; then
-  error_exit "Cannot use '$python', Python 2.6 or later is required." \
-  "Note that Python 3 or later is not yet supported." \
+if ! $python -c 'import sys; sys.exit(sys.version_info < (2,6))'; then
+  error_exit "Cannot use '$python', Python 2 >= 2.6 or Python 3 is required." \
   "Use --python=/path/to/python to specify a supported Python."
 fi
 
-- 
2.14.3




[Qemu-devel] [PATCH v3 08/13] scripts: ensure signrom treats data as bytes

2018-01-15 Thread Daniel P. Berrange
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/signrom.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/signrom.py b/scripts/signrom.py
index d1dabe0240..0497a1c32e 100644
--- a/scripts/signrom.py
+++ b/scripts/signrom.py
@@ -18,7 +18,7 @@ fin = open(sys.argv[1], 'rb')
 fout = open(sys.argv[2], 'wb')
 
 magic = fin.read(2)
-if magic != '\x55\xaa':
+if magic != b'\x55\xaa':
 sys.exit("%s: option ROM does not begin with magic 55 aa" % sys.argv[1])
 
 size_byte = ord(fin.read(1))
@@ -33,7 +33,7 @@ elif len(data) < size:
 # Add padding if necessary, rounding the whole input to a multiple of
 # 512 bytes according to the third byte of the input.
 # size-1 because a final byte is added below to store the checksum.
-data = data.ljust(size-1, '\0')
+data = data.ljust(size-1, b'\0')
 else:
 if ord(data[-1:]) != 0:
 sys.stderr.write('WARNING: ROM includes nonzero checksum\n')
-- 
2.14.3




[Qemu-devel] [PATCH v3 13/13] docker: change Fedora images to run with python3

2018-01-15 Thread Daniel P. Berrange
Fedora has switched to Python 3 by default, so it makes sense to use that
for testing QEMU builds, so we get testing of Python 3 compatibility.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 tests/docker/dockerfiles/fedora.docker | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/docker/dockerfiles/fedora.docker 
b/tests/docker/dockerfiles/fedora.docker
index 4b26c3aded..a22fe16157 100644
--- a/tests/docker/dockerfiles/fedora.docker
+++ b/tests/docker/dockerfiles/fedora.docker
@@ -1,6 +1,6 @@
 FROM fedora:latest
 ENV PACKAGES \
-ccache gettext git tar PyYAML sparse flex bison python2 bzip2 hostname \
+ccache gettext git tar PyYAML sparse flex bison python3 bzip2 hostname \
 glib2-devel pixman-devel zlib-devel SDL-devel libfdt-devel \
 gcc gcc-c++ clang make perl which bc findutils libaio-devel \
 nettle-devel \
@@ -12,6 +12,7 @@ ENV PACKAGES \
 mingw64-gtk2 mingw64-gtk3 mingw64-gnutls mingw64-nettle mingw64-libtasn1 \
 mingw64-libjpeg-turbo mingw64-libpng mingw64-curl mingw64-libssh2 \
 mingw64-bzip2
+ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3
 
 RUN dnf install -y $PACKAGES
 RUN rpm -q $PACKAGES | sort > /packages.txt
-- 
2.14.3




[Qemu-devel] [PATCH v3 12/13] travis: improve python version test coverage

2018-01-15 Thread Daniel P. Berrange
Currently travis declares ancient python 2.4 is desired. Update that to
2.6 which is the oldest version any targetted distros still needs. If we
just list a python 3 version at the top level this will double the
number of travis jobs we run which is unreasonable.

So arbitrarily pick the clang test matrix entries to build with python
3.0 and 3.6, to extend coverage of python versions, without increasing
job count or build time.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 .travis.yml | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index f583839755..708c886017 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,7 +1,7 @@
 sudo: false
 language: c
 python:
-  - "2.4"
+  - "2.6"
 compiler:
   - gcc
 cache: ccache
@@ -115,15 +115,17 @@ matrix:
 - sudo apt-get build-dep -qq qemu
 - wget -O - 
http://people.linaro.org/~alex.bennee/qemu-submodule-git-seed.tar.xz | tar -xvJ
 - git submodule update --init --recursive
-# Trusty System build with latest stable clang
+# Trusty System build with latest stable clang & python 3.0
 - sudo: required
   addons:
   dist: trusty
   language: generic
   compiler: none
+  python:
+- "3.0"
   env:
 - COMPILER_NAME=clang CXX=clang++-3.9 CC=clang-3.9
-- CONFIG="--disable-linux-user --cc=clang-3.9 --cxx=clang++-3.9"
+- CONFIG="--disable-linux-user --cc=clang-3.9 --cxx=clang++-3.9 
--python=/usr/bin/python3"
   before_install:
 - wget -nv -O - http://llvm.org/apt/llvm-snapshot.gpg.key | sudo 
apt-key add -
 - sudo apt-add-repository -y 'deb http://llvm.org/apt/trusty 
llvm-toolchain-trusty-3.9 main'
@@ -134,15 +136,17 @@ matrix:
 - git submodule update --init --recursive
   before_script:
 - ./configure ${CONFIG} || cat config.log
-# Trusty Linux User build with latest stable clang
+# Trusty Linux User build with latest stable clang & python 3.6
 - sudo: required
   addons:
   dist: trusty
   language: generic
   compiler: none
+  python:
+- "3.6"
   env:
 - COMPILER_NAME=clang CXX=clang++-3.9 CC=clang-3.9
-- CONFIG="--disable-system --cc=clang-3.9 --cxx=clang++-3.9"
+- CONFIG="--disable-system --cc=clang-3.9 --cxx=clang++-3.9 
--python=/usr/bin/python3"
   before_install:
 - wget -nv -O - http://llvm.org/apt/llvm-snapshot.gpg.key | sudo 
apt-key add -
 - sudo apt-add-repository -y 'deb http://llvm.org/apt/trusty 
llvm-toolchain-trusty-3.9 main'
-- 
2.14.3




[Qemu-devel] [PATCH v3 05/13] qapi: Adapt to moved location of 'maketrans' function in py3

2018-01-15 Thread Daniel P. Berrange
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/qapi.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index eaa63a58be..b6a7b5139f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1734,7 +1734,10 @@ def c_enum_const(type_name, const_name, prefix=None):
 type_name = prefix
 return camel_to_upper(type_name) + '_' + c_name(const_name, False).upper()
 
-c_name_trans = string.maketrans('.-', '__')
+if hasattr(str, 'maketrans'):
+c_name_trans = str.maketrans('.-', '__')
+else:
+c_name_trans = string.maketrans('.-', '__')
 
 
 # Map @name to a valid C identifier.
-- 
2.14.3




[Qemu-devel] [PATCH v3 06/13] qapi: remove '-q' arg to diff when comparing QAPI output

2018-01-15 Thread Daniel P. Berrange
When the qapi schema tests fail they merely print that the expected
output didn't match the actual output. This is largely useless when
trying diagnose what went wrong. Removing the '-q' arg to diff
means that it is still silent on successful tests, but when it
fails we'll see details of the incorrect output.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 tests/Makefile.include | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 39a4b5359d..d65fb4e1b3 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -908,10 +908,10 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): 
check-%.json: $(SRC_PATH)/%.json
$^ >$*.test.out 2>$*.test.err; \
echo $$? >$*.test.exit, \
"TEST","$*.out")
-   @diff -q $(SRC_PATH)/$*.out $*.test.out
+   @diff $(SRC_PATH)/$*.out $*.test.out
@# Sanitize error messages (make them independent of build directory)
-   @perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff -q 
$(SRC_PATH)/$*.err -
-   @diff -q $(SRC_PATH)/$*.exit $*.test.exit
+   @perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff 
$(SRC_PATH)/$*.err -
+   @diff $(SRC_PATH)/$*.exit $*.test.exit
 
 .PHONY: check-tests/qapi-schema/doc-good.texi
 check-tests/qapi-schema/doc-good.texi: tests/qapi-schema/doc-good.test.texi
-- 
2.14.3




[Qemu-devel] [PATCH v3 07/13] qapi: ensure stable sort ordering when checking QAPI entities

2018-01-15 Thread Daniel P. Berrange
Some early python 3.x versions will have different default
ordering when calling the 'values()' method on a dict, compared
to python 2.x and later 3.x versions. Explicitly sort the items
to get a stable ordering.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/qapi.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index b6a7b5139f..6266447eb0 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1678,7 +1678,7 @@ class QAPISchema(object):
 assert False
 
 def check(self):
-for ent in self._entity_dict.values():
+for (name, ent) in sorted(self._entity_dict.items()):
 ent.check(self)
 
 def visit(self, visitor):
-- 
2.14.3




[Qemu-devel] [PATCH v3 04/13] qapi: adapt to moved location of StringIO module in py3

2018-01-15 Thread Daniel P. Berrange
Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/qapi.py | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 7ec2e00b2c..eaa63a58be 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -22,6 +22,10 @@ try:
 from collections import OrderedDict
 except:
 from ordereddict import OrderedDict
+try:
+from StringIO import StringIO
+except ImportError:
+from io import StringIO
 
 builtin_types = {
 'null': 'QTYPE_QNULL',
@@ -1995,8 +1999,7 @@ def open_output(output_dir, do_c, do_h, prefix, c_file, 
h_file,
 if really:
 return open(name, opt)
 else:
-import StringIO
-return StringIO.StringIO()
+return StringIO()
 
 fdef = maybe_open(do_c, c_file, 'w')
 fdecl = maybe_open(do_h, h_file, 'w')
-- 
2.14.3




[Qemu-devel] [PATCH v3 10/13] input: add missing JIS keys to virtio input

2018-01-15 Thread Daniel P. Berrange
From: Miika S 

keycodemapdb updated to add the QKeyCodes muhenkan and katakanahiragana

Signed-off-by: Miika S 
---
 hw/input/virtio-input-hid.c | 7 +++
 qapi/ui.json| 5 -
 ui/keycodemapdb | 2 +-
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/input/virtio-input-hid.c b/hw/input/virtio-input-hid.c
index e78faec0b1..9628d289f9 100644
--- a/hw/input/virtio-input-hid.c
+++ b/hw/input/virtio-input-hid.c
@@ -139,6 +139,13 @@ static const unsigned int keymap_qcode[Q_KEY_CODE__MAX] = {
 [Q_KEY_CODE_META_L]  = KEY_LEFTMETA,
 [Q_KEY_CODE_META_R]  = KEY_RIGHTMETA,
 [Q_KEY_CODE_MENU]= KEY_MENU,
+
+[Q_KEY_CODE_MUHENKAN]= KEY_MUHENKAN,
+[Q_KEY_CODE_HENKAN]  = KEY_HENKAN,
+[Q_KEY_CODE_KATAKANAHIRAGANA]= KEY_KATAKANAHIRAGANA,
+[Q_KEY_CODE_COMPOSE] = KEY_COMPOSE,
+[Q_KEY_CODE_RO]  = KEY_RO,
+[Q_KEY_CODE_YEN] = KEY_YEN,
 };
 
 static const unsigned int keymap_button[INPUT_BUTTON__MAX] = {
diff --git a/qapi/ui.json b/qapi/ui.json
index 07b468f625..d6679aa8f5 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -748,6 +748,9 @@
 # @ac_bookmarks: since 2.10
 # altgr, altgr_r: dropped in 2.10
 #
+# @muhenkan: since 2.12
+# @katakanahiragana: since 2.12
+#
 # 'sysrq' was mistakenly added to hack around the fact that
 # the ps2 driver was not generating correct scancodes sequences
 # when 'alt+print' was pressed. This flaw is now fixed and the
@@ -775,7 +778,7 @@
 'left', 'up', 'down', 'right', 'insert', 'delete', 'stop', 'again',
 'props', 'undo', 'front', 'copy', 'open', 'paste', 'find', 'cut',
 'lf', 'help', 'meta_l', 'meta_r', 'compose', 'pause',
-'ro', 'hiragana', 'henkan', 'yen',
+'ro', 'hiragana', 'henkan', 'yen', 'muhenkan', 'katakanahiragana',
 'kp_comma', 'kp_equals', 'power', 'sleep', 'wake',
 'audionext', 'audioprev', 'audiostop', 'audioplay', 'audiomute',
 'volumeup', 'volumedown', 'mediaselect',
diff --git a/ui/keycodemapdb b/ui/keycodemapdb
index 10739aa260..05dad417e9 16
--- a/ui/keycodemapdb
+++ b/ui/keycodemapdb
@@ -1 +1 @@
-Subproject commit 10739aa26051a5d49d88132604539d3ed085e72e
+Subproject commit 05dad417e9d0b37ee1fba33056d91a6b734b3357
-- 
2.14.3




[Qemu-devel] [PATCH v3 00/13] Support building with py2 or py3

2018-01-15 Thread Daniel P. Berrange
This is an update for my previously posted series:

 v2: https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg06528.html

This series enables some level of CI testing for py3 so that our CI jobs will
get coverage of both py2 and py3 builds to avoid bitrot.

I did a test travis build with py 3.0 and py 3.6 and got success:

  https://travis-ci.org/berrange/qemu/builds/328223261

The goal was to achieve the following

  ./configure --python=/usr/bin/python3
  make
  make check

This still requires passing python path to configure explicitly. A
further improvement would be for configure to automatically detect
a pythjon 3 binary and use it preferentially to python 2.

I have not attempted to fix/validate the block I/O tests. I would expect
them to be broken, but easily fixable with the similar kind of scope
changes as seen here. I felt it better to tackle that separately to
avoid this initial series getting too large.

Although the Python 2 EOL date is 2020, we already have distros which
are not shipping Python 2 by default (Fedora >= 26 has dropped Py2 from
the default install). Any new releases of long life and/or enterprise
distros may well not ship Python 2 given that it would go EOL long
before the EOL of the distro itself. IOW QEMU does have a fairly pressing
need to be able to support Python 3 for building.

A request for py3 is tracked here:

  https://bugs.launchpad.net/qemu/+bug/1708462

NB, Patch 10 here is not related to python 3 work - it was just a
temporary pre-requisite of pulling in the keycodemapdb update.

Changes since v2:

 - Pull in fix for keycodemapdb
 - Enable testing with Travis
 - Enable testing with Fedora Docker images
 - Fix for sort ordering to fix 'make check-qapi-schema'
 - Fix for signrom data

Daniel P. Berrange (12):
  qapi: convert to use python print function instead of statement
  qapi: use items()/values() intead of iteritems()/itervalues()
  qapi: Use OrderedDict from standard library if available
  qapi: adapt to moved location of StringIO module in py3
  qapi: Adapt to moved location of 'maketrans' function in py3
  qapi: remove '-q' arg to diff when comparing QAPI output
  qapi: ensure stable sort ordering when checking QAPI entities
  scripts: ensure signrom treats data as bytes
  configure: allow use of python 3
  ui: update keycodemapdb to get py3 fixes
  travis: improve python version test coverage
  docker: change Fedora images to run with python3

Miika S (1):
  input: add missing JIS keys to virtio input

 .travis.yml| 14 +++
 configure  |  5 ++--
 hw/input/virtio-input-hid.c|  7 ++
 qapi/ui.json   |  5 +++-
 scripts/qapi.py| 43 --
 scripts/qapi2texi.py   | 11 +
 scripts/signrom.py |  4 ++--
 tests/Makefile.include |  6 ++---
 tests/docker/dockerfiles/fedora.docker |  3 ++-
 tests/qapi-schema/test-qapi.py | 43 +-
 ui/keycodemapdb|  2 +-
 11 files changed, 84 insertions(+), 59 deletions(-)

-- 
2.14.3




[Qemu-devel] [PATCH v3 02/13] qapi: use items()/values() intead of iteritems()/itervalues()

2018-01-15 Thread Daniel P. Berrange
The iteritems()/itervalues() methods are gone in py3, but the
items()/values() methods are still around. The latter are less
efficient than the former in py2, but this has unmeasurably
small impact on QEMU build time, so taking portability over
efficiency is a net win

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/qapi.py| 12 ++--
 scripts/qapi2texi.py   |  2 +-
 tests/qapi-schema/test-qapi.py |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 924c762381..5ef50317ca 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -245,7 +245,7 @@ class QAPIDoc(object):
"'Returns:' is only valid for commands")
 
 def check(self):
-bogus = [name for name, section in self.args.iteritems()
+bogus = [name for name, section in self.args.items()
  if not section.member]
 if bogus:
 raise QAPISemError(
@@ -300,7 +300,7 @@ class QAPISchemaParser(object):
 if not isinstance(pragma, dict):
 raise QAPISemError(
 info, "Value of 'pragma' must be a dictionary")
-for name, value in pragma.iteritems():
+for name, value in pragma.items():
 self._pragma(name, value, info)
 else:
 expr_elem = {'expr': expr,
@@ -1566,7 +1566,7 @@ class QAPISchema(object):
 
 def _make_members(self, data, info):
 return [self._make_member(key, value, info)
-for (key, value) in data.iteritems()]
+for (key, value) in data.items()]
 
 def _def_struct_type(self, expr, info, doc):
 name = expr['struct']
@@ -1598,11 +1598,11 @@ class QAPISchema(object):
 name, info, doc, 'base', self._make_members(base, info)))
 if tag_name:
 variants = [self._make_variant(key, value)
-for (key, value) in data.iteritems()]
+for (key, value) in data.items()]
 members = []
 else:
 variants = [self._make_simple_variant(key, value, info)
-for (key, value) in data.iteritems()]
+for (key, value) in data.items()]
 typ = self._make_implicit_enum_type(name, info,
 [v.name for v in variants])
 tag_member = QAPISchemaObjectTypeMember('type', typ, False)
@@ -1617,7 +1617,7 @@ class QAPISchema(object):
 name = expr['alternate']
 data = expr['data']
 variants = [self._make_variant(key, value)
-for (key, value) in data.iteritems()]
+for (key, value) in data.items()]
 tag_member = QAPISchemaObjectTypeMember('type', 'QType', False)
 self._def_entity(
 QAPISchemaAlternateType(name, info, doc,
diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 6630138192..d155cf099e 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -146,7 +146,7 @@ def texi_member(member, suffix=''):
 def texi_members(doc, what, base, variants, member_func):
 """Format the table of members"""
 items = ''
-for section in doc.args.itervalues():
+for section in doc.args.values():
 # TODO Drop fallbacks when undocumented members are outlawed
 if section.text:
 desc = texi_format(section.text)
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index aad407e0df..f535bc1c0c 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -63,7 +63,7 @@ for doc in schema.docs:
 else:
 print ('doc freeform')
 print ('body=\n%s' % doc.body.text)
-for arg, section in doc.args.iteritems():
+for arg, section in doc.args.items():
 print ('arg=%s\n%s' % (arg, section.text))
 for section in doc.sections:
 print ('section=%s\n%s' % (section.name, section.text))
-- 
2.14.3




[Qemu-devel] [PATCH v3 03/13] qapi: Use OrderedDict from standard library if available

2018-01-15 Thread Daniel P. Berrange
The OrderedDict class appeared in the 'collections' module
from python 2.7 onwards, so use that in preference to our
local backport if available.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/qapi.py | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 5ef50317ca..7ec2e00b2c 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -18,7 +18,10 @@ import os
 import re
 import string
 import sys
-from ordereddict import OrderedDict
+try:
+from collections import OrderedDict
+except:
+from ordereddict import OrderedDict
 
 builtin_types = {
 'null': 'QTYPE_QNULL',
-- 
2.14.3




[Qemu-devel] [PATCH v3 01/13] qapi: convert to use python print function instead of statement

2018-01-15 Thread Daniel P. Berrange
Get Py2 + 3 compatibility by using the print function
instead of print statement. This works for 2.6 onwards.

Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
---
 scripts/qapi.py| 12 ++--
 scripts/qapi2texi.py   |  9 +
 tests/qapi-schema/test-qapi.py | 41 +
 3 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 43a54bf40f..924c762381 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -11,6 +11,7 @@
 # This work is licensed under the terms of the GNU GPL, version 2.
 # See the COPYING file in the top-level directory.
 
+from __future__ import print_function
 import errno
 import getopt
 import os
@@ -1467,7 +1468,7 @@ class QAPISchema(object):
 self._def_exprs()
 self.check()
 except QAPIError as err:
-print >>sys.stderr, err
+print (err, file=sys.stderr)
 exit(1)
 
 def _def_entity(self, ent):
@@ -1931,7 +1932,7 @@ def parse_command_line(extra_options='', 
extra_long_options=[]):
['source', 'header', 'prefix=',
 'output-dir='] + extra_long_options)
 except getopt.GetoptError as err:
-print >>sys.stderr, "%s: %s" % (sys.argv[0], str(err))
+print ("%s: %s" % (sys.argv[0], str(err)), file=sys.stderr)
 sys.exit(1)
 
 output_dir = ''
@@ -1945,9 +1946,8 @@ def parse_command_line(extra_options='', 
extra_long_options=[]):
 if o in ('-p', '--prefix'):
 match = re.match(r'([A-Za-z_.-][A-Za-z0-9_.-]*)?', a)
 if match.end() != len(a):
-print >>sys.stderr, \
-"%s: 'funny character '%s' in argument of --prefix" \
-% (sys.argv[0], a[match.end()])
+print ("%s: 'funny character '%s' in argument of --prefix" \
+   % (sys.argv[0], a[match.end()]), file=sys.stderr)
 sys.exit(1)
 prefix = a
 elif o in ('-o', '--output-dir'):
@@ -1964,7 +1964,7 @@ def parse_command_line(extra_options='', 
extra_long_options=[]):
 do_h = True
 
 if len(args) != 1:
-print >>sys.stderr, "%s: need exactly one argument" % sys.argv[0]
+print ("%s: need exactly one argument" % sys.argv[0], file=sys.stderr)
 sys.exit(1)
 fname = args[0]
 
diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
index 92e2af2cd6..6630138192 100755
--- a/scripts/qapi2texi.py
+++ b/scripts/qapi2texi.py
@@ -4,6 +4,7 @@
 # This work is licensed under the terms of the GNU LGPL, version 2+.
 # See the COPYING file in the top-level directory.
 """This script produces the documentation of a qapi schema in texinfo format"""
+from __future__ import print_function
 import re
 import sys
 
@@ -274,15 +275,15 @@ def texi_schema(schema):
 def main(argv):
 """Takes schema argument, prints result to stdout"""
 if len(argv) != 2:
-print >>sys.stderr, "%s: need exactly 1 argument: SCHEMA" % argv[0]
+print ("%s: need exactly 1 argument: SCHEMA" % argv[0], 
file=sys.stderr)
 sys.exit(1)
 
 schema = qapi.QAPISchema(argv[1])
 if not qapi.doc_required:
-print >>sys.stderr, ("%s: need pragma 'doc-required' "
- "to generate documentation" % argv[0])
+print ("%s: need pragma 'doc-required' "
+   "to generate documentation" % argv[0], file=sys.stderr)
 sys.exit(1)
-print texi_schema(schema)
+print (texi_schema(schema))
 
 
 if __name__ == '__main__':
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index fe0ca08d78..aad407e0df 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -10,6 +10,7 @@
 # See the COPYING file in the top-level directory.
 #
 
+from __future__ import print_function
 from qapi import *
 from pprint import pprint
 import os
@@ -18,51 +19,51 @@ import sys
 
 class QAPISchemaTestVisitor(QAPISchemaVisitor):
 def visit_enum_type(self, name, info, values, prefix):
-print 'enum %s %s' % (name, values)
+print ('enum %s %s' % (name, values))
 if prefix:
-print 'prefix %s' % prefix
+print ('prefix %s' % prefix)
 
 def visit_object_type(self, name, info, base, members, variants):
-print 'object %s' % name
+print ('object %s' % name)
 if base:
-print 'base %s' % base.name
+print ('base %s' % base.name)
 for m in members:
-print 'member %s: %s optional=%s' % \
-(m.name, m.type.name, m.optional)
+print ('  

Re: [Qemu-devel] [PATCH v4] Add ability for user to specify mouse ungrab key

2018-01-15 Thread Daniel P. Berrange
On Fri, Jan 12, 2018 at 10:15:24PM -0500, Programmingkid wrote:
> 
> > On Jan 10, 2018, at 11:14 AM, Daniel P. Berrange <berra...@redhat.com> 
> > wrote:
> > 
> > On Tue, Dec 26, 2017 at 08:14:28PM -0500, John Arbuckle wrote:
> >> Currently the ungrab keys for the Cocoa and GTK interface are 
> >> Control-Alt-g.
> >> This combination may not be very fun for the user to have to enter, so we
> >> now enable the user to specify their own key(s) as the ungrab key(s). The
> >> list of keys that can be used is found in the file qapi/ui.json under 
> >> QKeyCode.
> >> The max number of keys that can be used is three. The original ungrab keys
> >> still remain usable because there is a real risk of the user forgetting 
> >> the keys he or she specified as the ungrab keys. They remain as a plan B
> >> if plan A is forgotten.
> > 
> > This is a bad idea. A key reason to give a custom ungrab sequence, is 
> > because
> > the user wants to be able to use the default ungrab sequence inside the
> > guest. Always having the default ungrab sequence active prevents this.
> 
> Sounds like a great idea.
> 
> > 
> >> Syntax: -ungrab 
> >> 
> >> Example usage:  -ungrab home
> >>-ungrab shift-ctrl
> >>-ungrab ctrl-x
> >>-ungrab pgup-pgdn
> >>-ungrab kp_5-kp_6
> >>-ungrab kp_4-kp_5-kp_6
> > 
> > I'm in two minds about using '-' as a separator.  Perhaps '+' is a better
> > choice ?
> 
> I think '-' is better because the user isn't required to push the shift key 
> or order to see the symbol unlike '+'.
> 
> > 
> >> 
> >> Signed-off-by: John Arbuckle <programmingk...@gmail.com>
> >> ---
> >> v4 changes:
> >> - Removed initialization code for key_value_array.
> >> - Added void keyword to console_ungrab_key_sequence(),
> >>  and console_ungrab_key_string() functions.
> >> 
> >> v3 changes:
> >> - Added the ability for any "sendkey supported" key to be used.
> >> - Added ability for one to three key sequences to be used.
> >> 
> >> v2 changes:
> >> - Removed the "int i" code from the for loops. 
> >> 
> >> include/ui/console.h |  6 ++
> >> qemu-options.hx  |  2 ++
> >> ui/cocoa.m   | 48 +++--
> >> ui/console.c | 60 
> >> 
> >> vl.c |  3 +++
> >> 5 files changed, 117 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/include/ui/console.h b/include/ui/console.h
> >> index 580dfc57ee..37dc150268 100644
> >> --- a/include/ui/console.h
> >> +++ b/include/ui/console.h
> >> @@ -534,4 +534,10 @@ static inline void early_gtk_display_init(int opengl)
> >> /* egl-headless.c */
> >> void egl_headless_init(void);
> >> 
> >> +/* console.c */
> >> +void set_ungrab_seq(const char *new_seq);
> >> +int *console_ungrab_key_sequence(void);
> >> +const char *console_ungrab_key_string(void);
> >> +int console_ungrab_sequence_length(void);
> > 
> > We don't need to have a console_ungrab_sequence_length() method if
> > we make the array returned by console_ungrab_key_sequence() be a
> > zero terminated array.
> 
> I kind of liked having it but calculating it once in the front-end isn't a 
> problem.
> 
> > 
> >> diff --git a/ui/cocoa.m b/ui/cocoa.m
> >> index 330ccebf90..412a5fc02d 100644
> >> --- a/ui/cocoa.m
> >> +++ b/ui/cocoa.m
> >> @@ -303,6 +303,7 @@ - (float) cdx;
> >> - (float) cdy;
> >> - (QEMUScreen) gscreen;
> >> - (void) raiseAllKeys;
> >> +- (bool) user_ungrab_seq;
> >> @end
> >> 
> >> QemuCocoaView *cocoaView;
> >> @@ -674,9 +675,24 @@ - (void) handleEvent:(NSEvent *)event
> >> }
> >> }
> >> 
> >> +/*
> >> + * This code has to be here because the user might use a 
> >> modifier
> >> + * key like shift as an ungrab key.
> >> + */
> >> +if ([self user_ungrab_seq] == true) {
> >> +[self ungrabMouse];
> >> +return;
> >> +}
> >> break;
> >> case NSEventTypeKeyDown:
> >> keycode = cocoa_keyco

  1   2   3   4   5   6   7   8   9   10   >