Re: [Qemu-devel] [PATCH for 2.10 v2 0/6] Various docker fixes

[Fam Zheng]

On Tue, 07/25 14:34, Alex Bennée wrote:
> Hi,
> 
> Phillipe pointed out a few anomalies with the travis.docker image
> during review which has led to a couple more patches. As they are bug
> fixes I didn't race to get a pull-req out today for hard-freeze. I
> will roll a pull-req by the end of the week if there are no objections
> as I'll be away for the rc1/2 cycles.
> 
> Alex Bennée (6):
>   docker: ensure NOUSER for travis images
>   docker: fix dirty/stash detection on some systems
>   docker: ignore submodules when checking diff
>   docker: docker.py make --no-cache skip checksum test
>   docker: don't install device-tree-compiler build-deps in travis.docker
>   docker: reduce noise when building travis.docker

Thanks, I've queued patches 1, 4, 5, 6 for 2.11. I intend to drop the code
touched by 2 and 3 with a dedicated script:

https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg06349.html
https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg06358.html

(It would be great if you can give some test/review there :)

Fam

> 
>  tests/docker/Makefile.include  | 3 ++-
>  tests/docker/docker.py | 3 ++-
>  tests/docker/dockerfiles/travis.docker | 6 --
>  3 files changed, 8 insertions(+), 4 deletions(-)
> 
> -- 
> 2.13.0
> 

Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH

[Thomas Huth]

On 31.08.2017 08:38, Cornelia Huck wrote:
> On Thu, 31 Aug 2017 07:51:17 +0200
> Thomas Huth  wrote:
> 
>> On 30.08.2017 18:36, Halil Pasic wrote:
>>> The function ioinst_handle_xsch is presenting cc 2 when it's supposed to
>>> present cc 1 and the other way around, because css_do_xsch has the error
>>> codes mixed up. Fixing the error codes also fixes the priority.
>>>
>>> Let us fix this.  
>>
>> (Nit: In case you respin, I'd suggest to remove the last sentence. You
>> already used "fix" two times in the previous one)
> 
> I can also remove it on applying, if Halil agrees (I have not yet
> reviewed it, though).
> 
>>
>>> Signed-off-by: Halil Pasic 
>>> Reported-by: Pierre Morel  
>>
>> Space missing -^
> 
> And I can also add that space on applying :)
> 
>>
>>> ---
>>>  hw/s390x/css.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
>>> index 1880b1a0ff..a50fb0727e 100644
>>> --- a/hw/s390x/css.c
>>> +++ b/hw/s390x/css.c
>>> @@ -1281,12 +1281,12 @@ int css_do_xsch(SubchDev *sch)
>>>  (!(s->ctrl &
>>> (SCSW_ACTL_RESUME_PEND | SCSW_ACTL_START_PEND | 
>>> SCSW_ACTL_SUSP))) ||
>>>  (s->ctrl & SCSW_ACTL_SUBCH_ACTIVE)) {
>>> -ret = -EINPROGRESS;
>>> +ret = -EBUSY;
>>>  goto out;
>>>  }
>>>  
>>>  if (s->ctrl & SCSW_CTRL_MASK_STCTL) {
>>> -ret = -EBUSY;
>>> +ret = -EINPROGRESS;
>>>  goto out;
>>>  }  
>>
>> Using both, EBUSY and EINPROGRESS as error codes sounds very confusing
>> to me here ... what's the difference between busy and in-progress? So
>> while you're at it, maybe you could replace the code for CC 2 ("CANCEL
>> SUBCHANNEL not applicable") with a different error code?
> 
> IIRC, I used these two as they matched my idea of what happens best
> (there's a difference between "subchannel is busy" and "the I/O is
> already in progress, too late to cancel"). "xsch not applicable" is
> very hard to translate to an Unix error code :/

OK, the codes make more sense with your description ==> Maybe simply add
a proper comment for each of the return codes?

 Thomas

Re: [Qemu-devel] [PATCH v2 02/13] hvf: add code base from Google's QEMU repository

[Stefan Hajnoczi]

On Wed, Aug 30, 2017 at 03:07:38PM +0100, Daniel P. Berrange wrote:
> On Wed, Aug 30, 2017 at 03:26:51AM -0500, Sergio Andres Gomez Del Real wrote:
> > diff --git a/target/i386/hvf-utils/x86.c b/target/i386/hvf-utils/x86.c
> > new file mode 100644
> > index 00..e3db2c9c8b
> > --- /dev/null
> > +++ b/target/i386/hvf-utils/x86.c
> > @@ -0,0 +1,174 @@
> > +/*
> > + * Copyright (C) 2016 Veertu Inc,
> > + * Copyright (C) 2017 Google Inc,
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License as
> > + * published by the Free Software Foundation; either version 2 or
> > + * (at your option) version 3 of the License.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along
> > + * with this program; if not, see .
> > + */
> 
> Again  v2-or-v3-only.
> 
> There's many more files with this same problem but I'll stop pointing
> them out now.
> 
> If this is to be included in QEMU,  Veertu & Google (and any other
> copyright holders) would have to agree to change these files to
> v2-or-later

Sergio: Have you or Paolo had any contact with the Veertu or Google
authors about your Hypervisor.framework project?  If you're already in
contact with them you could raise the issue and ask them to join this
email thread.

There are benefits to having this code upstream.  If they ever want to
rebase on qemu.git there will be less work for them.

Stefan

Re: [Qemu-devel] [PATCH v2 11/17] MAINTAINERS: add missing entries for throttling infra

[Alberto Garcia]

On Wed 30 Aug 2017 11:55:17 PM CEST, Philippe Mathieu-Daudé  
wrote:
> Signed-off-by: Philippe Mathieu-Daudé 
> Reviewed-by: Stefan Hajnoczi 

Reviewed-by: Alberto Garcia 

Berto

Re: [Qemu-devel] [PATCH 3/9] s390x/css: be more consistent if broken beyond repair

[Thomas Huth]

On 31.08.2017 08:10, Thomas Huth wrote:
> On 30.08.2017 18:36, Halil Pasic wrote:
>> If we detect that the internally manged state of the subchannel
>> is broken beyond repair while in do_subchannel_work in case of
>> virtual we just abort the operation and pretend all went well,
>> while in case of pass-through we honor the situation with -ENODEV
>> which results in cc 3 for the instruction whose handler triggered
>> the call.
>>
>> Let's be consistent on this and do the -ENODEV also for virtual
>> subchannels.
>>
>> Signed-off-by: Halil Pasic 
>> Acked-by: Pierre Morel
>> ---
>>  hw/s390x/css.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
>> index 0822538cde..bc47bf5b20 100644
>> --- a/hw/s390x/css.c
>> +++ b/hw/s390x/css.c
>> @@ -1078,7 +1078,7 @@ int do_subchannel_work_virtual(SubchDev *sch)
>>  sch_handle_start_func_virtual(sch);
>>  } else {
>>  /* Cannot happen. */
>> -return 0;
>> +return -ENODEV;
>>  }
>>  css_inject_io_interrupt(sch);
>>  return 0;
> 
> First, I think ENODEV is not really a good choice here since there
> certainly was a device. So maybe use EINVAL or EBADR or something else
> instead?
>
> Second, while return an error code is certainly better than returning 0,
> I think most errors will still go unnoticed here, since most callers of
> do_subchannel_work() seem to ignore the return value ... so I wonder
> whether we rather want to have another way to recognize this condition.
> If the comment is right and this really can not happen, I think you
> should use an g_assert_notreached() here instead. Otherwise the comment
> should be changed and it's maybe a good idea to use a
> qemu_log_mask(LOG_GUEST_ERROR, "subchannel in bad state bla bla...") here.

OK, after reading through patch 4/9 I think I've got the basic idea now
... you'll eventually set sch->iret.cc = 3 here instead, so the exact
error code does not really matter here.
But still - if it "Cannot happen", maybe an assert() or an additional
qemu_log would be appropriate?

 Thomas

Re: [Qemu-devel] [PATCH v2 1/7] block/ssh: don't call libssh2_init() in block_init()

[Richard W.M. Jones]

On Wed, Aug 30, 2017 at 02:40:16PM -0500, Eric Blake wrote:
> On 08/30/2017 11:56 AM, Jeff Cody wrote:
> > We don't need libssh2 failure to be fatal (we could just opt to not
> > register the driver on failure). But, it is probably a good idea to
> > avoid external library calls during the block_init(), and call the
> > libssh2 global init function on the first usage, returning any errors.
> > 
> > Signed-off-by: Jeff Cody 
> > ---
> >  block/ssh.c | 40 +---
> >  1 file changed, 29 insertions(+), 11 deletions(-)
> > 
> 
> > +static int ssh_state_init(BDRVSSHState *s, Error **errp)
> >  {
> > +int ret;
> > +
> > +if (!ssh_libinit_called) {
> > +ret = libssh2_init(0);
> > +if (ret) {
> > +error_setg(errp, "libssh2 initialization failed with %d", ret);
> > +return ret;
> 
> Do we know if this number is always positive or negative?

FWIW documentation says:

  "Returns 0 if succeeded, or a negative value for error."

I was holding off on reviewing this patch in general since there's a
libssh-based alternative driver under development, which should be
better.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

Re: [Qemu-devel] [PATCH 2/9] s390x: fix invalid use of cc 1 for SSCH

[Thomas Huth]

On 30.08.2017 18:36, Halil Pasic wrote:
> According to the POP a start subchannel instruction (SSCH) returning with
> cc 1 implies that the subchannel was status pending when SSCH executed.
> 
> Due to a somewhat confusing error handling, where error codes are mapped
> to cc value, sane looking error codes result in non AR compliant
> behavior.
> 
> Let's fix this! Instead of cc 1 we use cc 3 which means device not
> operational, and is much closer to the truth in the given cases.
> 
> Signed-off-by: Halil Pasic 
> Acked-by: Pierre Morel
> ---
> 
> This patch turned out quite controversial. We did not reach a consensus
> during the internal review.
> 
> The most of the discussion revolved around the ORB flag which
> architecturally must be supported, but are currently not supported by
> vfio-ccw (not yet, or can't be). The idea showing the most promise for
> consensus was to handle this via device status (along the lines better a
> strange acting device than a non-conform machine) but since it's a
> radical change we decided to first discuss upstream and then do whatever
> needs to be done.
> ---
>  hw/s390x/css.c  | 15 ++-
>  hw/s390x/s390-ccw.c |  2 +-
>  2 files changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index a50fb0727e..0822538cde 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -1034,7 +1034,7 @@ static int sch_handle_start_func_passthrough(SubchDev 
> *sch)
>   */
>  if (!(orb->ctrl0 & ORB_CTRL0_MASK_PFCH) ||
>  !(orb->ctrl0 & ORB_CTRL0_MASK_C64)) {
> -return -EINVAL;
> +return -ENODEV;

I don't really like ENODEV in this case (since the device is apparently
there)... but well, since you're later change it again to set cc=3
directly, I guess the temporary ENODEV is ok.

>  }
>  
>  ret = s390_ccw_cmd_request(orb, s, sch->driver_data);
> @@ -1046,16 +1046,13 @@ static int sch_handle_start_func_passthrough(SubchDev 
> *sch)
>  break;
>  case -ENODEV:
>  break;
> +case -EFAULT:
> + break;

I think you should mention this in the patch description. Why is EFAULT
suddenly handled here?

>  case -EACCES:
>  /* Let's reflect an inaccessible host device by cc 3. */
> -ret = -ENODEV;
> -break;
>  default:
> -   /*
> -* All other return codes will trigger a program check,
> -* or set cc to 1.
> -*/
> -   break;
> +/* Let's make all other return codes map to cc 3.  */
> +ret = -ENODEV;
>  };
>  
>  return ret;
> @@ -1115,7 +1112,7 @@ static int do_subchannel_work(SubchDev *sch)
>  if (sch->do_subchannel_work) {
>  return sch->do_subchannel_work(sch);
>  } else {
> -return -EINVAL;
> +return -ENODEV;
>  }
>  }
>  
> diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c
> index 8614dda6f8..2b0741741c 100644
> --- a/hw/s390x/s390-ccw.c
> +++ b/hw/s390x/s390-ccw.c
> @@ -25,7 +25,7 @@ int s390_ccw_cmd_request(ORB *orb, SCSW *scsw, void *data)
>  if (cdc->handle_request) {
>  return cdc->handle_request(orb, scsw, data);
>  } else {
> -return -ENOSYS;
> +return -ENODEV;
>  }
>  }

 Thomas

Re: [Qemu-devel] [PATCH v2 02/13] hvf: add code base from Google's QEMU repository

[Stefan Hajnoczi]

On Wed, Aug 30, 2017 at 03:26:51AM -0500, Sergio Andres Gomez Del Real wrote:
> This file begins tracking the files that will be the code base for HVF
> support in QEMU. This code base is part of Google's QEMU version of
> their Android emulator, and can be found at
> https://android.googlesource.com/platform/external/qemu/+/emu-master-dev
> 
> This code is based on Veertu Inc's vdhh (Veertu Desktop Hosted
> Hypervisor), found at https://github.com/veertuinc/vdhh. Everything is
> appropriately licensed under GPL v2.
> 
> This code base already implements a very great deal of functionality,
> although Google's version removed from Vertuu's the support for APIC
> page and hyperv-related stuff. According to the Android Emulator Release
> Notes, Revision 26.1.3 (August 2017), "Hypervisor.framework is now
> enabled by default on macOS for 32-bit x86 images to improve performance
> and macOS compatibility", although we better use with caution for, as the
> same Revision warns us, "If you experience issues with it specifically,
> please file a bug report...". The code hasn't seen much update in the
> last 5 months, so I think that we can further develop the code with
> occasional visiting Google's repository to see if there has been any
> update.
> 
> The code's style isn't aligned to QEMU's standards; this will be fixed
> in a subsequent patch in this series.
> On top of this code base we are implementing the following features: fix
> the code that passes the cpuid features to the guest; implementing dirty
> page tracking for vga memory region; reimplementing the event injection
> mechanism for exception injection and many other minor
> fixes/refactoring that are documented in their respective patches.
> 
> Signed-off-by: Sergio Andres Gomez Del Real 
> ---

This looks like a reasonable approach to getting Hypervisor.framework
support.  I haven't reviewed the details but it contains all the pieces
one would expect.

Which guest OSes have you booted successfully?

Stefan

Re: [Qemu-devel] Persistent bitmaps for non-qcow2 formats

[Yaniv Lavi (Dary)]

YANIV LAVI (YANIV DARY)

SENIOR TECHNICAL PRODUCT MANAGER

Red Hat Israel Ltd. 

34 Jerusalem Road, Building A, 1st floor

Ra'anana, Israel 4350109

yl...@redhat.comT: +972-9-7692306/8272306 F: +972-9-7692223IM: ylavi
  TRIED. TESTED. TRUSTED. 
@redhatnews    Red Hat
   Red Hat



On Thu, Aug 31, 2017 at 12:25 AM, John Snow  wrote:

>
>
> On 08/30/2017 08:58 AM, Yaniv Lavi (Dary) wrote:
> >
> >
> > We had no reason to switch to anything else so far and I'm sure this
> > option was not available when we started supporting raw format.
> >
> >
>
> Yeah, they don't exist yet...! I've looped you in to see if the proposal
> being discussed would alleviate the need for bitmaps for "other formats"
> if we can offer a "raw mode qcow2."
>

As we our direction is to move a file system/LUN per disk I don't think
adding a new type disk image is the right priority.
The below option of separate file sound like the right direction.


>
> At the moment I am going to still try to add bitmaps to other formats
> (through the use of a qcow2 wrapper) but it sounds like a raw-layout
> qcow2 might provide some benefits too.
>
> -js
>

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 8/8] ppc: remove non implemented cpu models

[Thomas Huth]

On 30.08.2017 15:24, Igor Mammedov wrote:
> Remove cpu models that aren't implemented and are not
> compiled/tested since they are under TODO ifdef
> which isn't defined in sources.
> 
> If someone really needs a removed model he/she should add
> as regular one with corresponding implementation.
> 
> Signed-off-by: Igor Mammedov 
> ---
>  target/ppc/cpu-models.c | 459 
> 
>  1 file changed, 459 deletions(-)
> 
> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
> index cf878a9..611fc1b 100644
> --- a/target/ppc/cpu-models.c
> +++ b/target/ppc/cpu-models.c
[...]
> -#endif
>  #if defined(TODO_USER_ONLY)
>  POWERPC_DEF("440sp", CPU_POWERPC_440SP,  440EP,
>  "PowerPC 440 SP")
> @@ -396,20 +207,6 @@
>  POWERPC_DEF("440spe",CPU_POWERPC_440SPE, 440EP,
>  "PowerPC 440 SPE")
>  #endif
> -/* PowerPC 460 family
> */
> -#if defined(TODO)
> -POWERPC_DEF("464",   CPU_POWERPC_464,460,
> -"Generic PowerPC 464")
> -#endif
> -/* PowerPC 464 microcontrollers  
> */
> -#if defined(TODO)
> -POWERPC_DEF("464h90",CPU_POWERPC_464H90, 460,
> -"PowerPC 464H90")
> -#endif
> -#if defined(TODO)
> -POWERPC_DEF("464h90f",   CPU_POWERPC_464H90F,460F,
> -"PowerPC 464H90F")
> -#endif

By the way, I guess you could also remove the 460 stuff from
translate_init.c since there are no 460 CPUs defined in QEMU
(but I guess that should go into a separate patch instead).

 Thomas

Re: [Qemu-devel] [PATCH v2 04/13] hvf: run hvf code through checkpatch.pl and fix style issues

[Stefan Hajnoczi]

On Wed, Aug 30, 2017 at 03:26:53AM -0500, Sergio Andres Gomez Del Real wrote:
> @@ -900,6 +904,9 @@ void cpu_synchronize_all_states(void)
>  
>  CPU_FOREACH(cpu) {
>  cpu_synchronize_state(cpu);
> +if (hvf_enabled()) {
> +hvf_cpu_synchronize_state(cpu);
> +}

The other accelerators put their code into cpu_synchronize_state():

  static inline void cpu_synchronize_state(CPUState *cpu)
  {
  if (kvm_enabled()) {
  kvm_cpu_synchronize_state(cpu);
  }
  if (hax_enabled()) {
  hax_cpu_synchronize_state(cpu);
  }
  }

Why put the hvf code outside cpu_synchronize_state()?

>  }
>  }
>  
> @@ -909,6 +916,9 @@ void cpu_synchronize_all_post_reset(void)
>  
>  CPU_FOREACH(cpu) {
>  cpu_synchronize_post_reset(cpu);
> +if (hvf_enabled()) {
> +hvf_cpu_synchronize_post_reset(cpu);
> +}

Same here.

>  }
>  }
>  
> @@ -918,6 +928,9 @@ void cpu_synchronize_all_post_init(void)
>  
>  CPU_FOREACH(cpu) {
>  cpu_synchronize_post_init(cpu);
> +if (hvf_enabled()) {
> +hvf_cpu_synchronize_post_init(cpu);
> +}

Same here.

These changes are not checkpatch.pl fixes.  It's okay to have a huge
patch that just fixes checkpatch.pl issues, but please don't include
other changes in the patch.  They should go in separate commits with
proper commit messages/descriptions.

I'll wait for the next revision before reviewing this patch further.

Re: [Qemu-devel] [PATCH v2 07/13] apic: add function to apic that will be used by hvf

[Stefan Hajnoczi]

On Wed, Aug 30, 2017 at 03:26:56AM -0500, Sergio Andres Gomez Del Real wrote:
> This commit adds the function apic_get_highest_priority_irr to
> apic.c and exports it through the interface in apic.h for use by hvf.
> 
> Signed-off-by: Sergio Andres Gomez Del Real 
> ---
>  hw/intc/apic.c | 11 +++
>  include/hw/i386/apic.h |  1 +
>  2 files changed, 12 insertions(+)
> 
> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> index fe15fb6024..3de59d07fd 100644
> --- a/hw/intc/apic.c
> +++ b/hw/intc/apic.c
> @@ -305,6 +305,17 @@ static void apic_set_tpr(APICCommonState *s, uint8_t val)
>  }
>  }
>  
> +int apic_get_highest_priority_irr(DeviceState *dev)

Missing doc comments.  -1 means no interrupts.

Re: [Qemu-devel] [PATCH v2 09/13] hvf: refactor cpuid code

[Stefan Hajnoczi]

On Wed, Aug 30, 2017 at 03:26:58AM -0500, Sergio Andres Gomez Del Real wrote:
> @@ -613,6 +614,15 @@ static uint32_t xsave_area_size(uint64_t mask)
>  return ret;
>  }
>  
> +static inline bool accel_uses_host_cpuid(void)
> +{
> +if (kvm_enabled() || hvf_enabled()) {
> +return true;
> +} else {
> +return false;
> +}

Shorter equivalent:

  return kvm_enabled() || hvf_enabled();

> @@ -2396,6 +2427,7 @@ static void x86_cpu_load_def(X86CPU *cpu, 
> X86CPUDefinition *def, Error **errp)
>  }
>  
>  /* Special cases not set in the X86CPUDefinition structs: */
> +/* TODO: implement for hvf */

This patch has several todos.  What is missing?  What is the impact of
silently skipping this for hvf?

Re: [Qemu-devel] [PATCH v2 10/13] hvf: implement vga dirty page tracking

[Stefan Hajnoczi]

On Wed, Aug 30, 2017 at 03:26:59AM -0500, Sergio Andres Gomez Del Real wrote:
> +static void hvf_log_start(MemoryListener *listener,
> +  MemoryRegionSection *section, int old, int new)
> +{
> +if (old != 0)
> +return;

QEMU coding style uses curly braces even when the if statement body only
has 1 line.

> +
> +hvf_set_dirty_tracking(section, 1);
> +}
> +
> +static void hvf_log_stop(MemoryListener *listener,
> + MemoryRegionSection *section, int old, int new)
> +{
> +if (new != 0)
> +return;

Same here.

[Qemu-devel] [PULL for-2.10 00/15] Block patches

[Stefan Hajnoczi]

The following changes since commit 248b23735645f7cbb503d9be6f5bf825f2a603ab:

  Update version for v2.10.0-rc4 release (2017-08-24 17:34:26 +0100)

are available in the git repository at:

  git://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 3e4c705212abfe8c9882a00beb2d1466a8a53cec:

  qcow2: allocate cluster_cache/cluster_data on demand (2017-08-30 18:02:10 
+0100)





Alberto Garcia (8):
  throttle: Fix wrong variable name in the header documentation
  throttle: Update the throttle_fix_bucket() documentation
  throttle: Make throttle_is_valid() a bit less verbose
  throttle: Remove throttle_fix_bucket() / throttle_unfix_bucket()
  throttle: Make LeakyBucket.avg and LeakyBucket.max integer types
  throttle: Make burst_length 64bit and add range checks
  throttle: Test the valid range of config values
  misc: Remove unused Error variables

Dan Aloni (1):
  nvme: Fix get/set number of queues feature, again

Eduardo Habkost (1):
  oslib-posix: Print errors before aborting on qemu_alloc_stack()

Fred Rolland (1):
  qemu-doc: Add UUID support in initiator name

Stefan Hajnoczi (4):
  scripts: add argparse module for Python 2.6 compatibility
  docker.py: Python 2.6 argparse compatibility
  tests: migration/guestperf Python 2.6 argparse compatibility
  qcow2: allocate cluster_cache/cluster_data on demand

 include/qemu/throttle.h|8 +-
 block/qcow.c   |   12 +-
 block/qcow2-cluster.c  |   17 +
 block/qcow2.c  |   20 +-
 dump.c |4 +-
 hw/block/nvme.c|4 +-
 tests/test-throttle.c  |   80 +-
 util/oslib-posix.c |2 +
 util/throttle.c|   86 +-
 COPYING.PYTHON |  270 
 qemu-doc.texi  |5 +-
 scripts/argparse.py| 2406 
 tests/docker/docker.py |4 +-
 tests/migration/guestperf/shell.py |8 +-
 14 files changed, 2831 insertions(+), 95 deletions(-)
 create mode 100644 COPYING.PYTHON
 create mode 100644 scripts/argparse.py

-- 
2.13.5

[Qemu-devel] [PULL for-2.10 01/15] nvme: Fix get/set number of queues feature, again

[Stefan Hajnoczi]

From: Dan Aloni 

The number of queues that should be return by the admin command should:

  1) Only mention the number of non-admin queues.
  2) It is zero-based, meaning that '0 == one non-admin queue',
 '1 == two non-admin queues', and so forth.

Because our `num_queues` means the number of queues _plus_ the admin
queue, then the right calculation for the number returned from the admin
command is `num_queues - 2`, combining the two requirements mentioned.

The issue was discovered by reducing num_queues from 64 to 8 and running
a Linux VM with an SMP parameter larger than that (e.g. 22). It tries to
utilize all queues, and therefore fails with an invalid queue number
when trying to queue I/Os on the last queue.

Signed-off-by: Dan Aloni 
CC: Alex Friedman 
CC: Keith Busch 
CC: Stefan Hajnoczi 
Reviewed-by: Keith Busch 
Signed-off-by: Stefan Hajnoczi 
---
 hw/block/nvme.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 6071dc12d8..9aa32692a3 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -615,7 +615,7 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, 
NvmeRequest *req)
 result = blk_enable_write_cache(n->conf.blk);
 break;
 case NVME_NUMBER_OF_QUEUES:
-result = cpu_to_le32((n->num_queues - 1) | ((n->num_queues - 1) << 
16));
+result = cpu_to_le32((n->num_queues - 2) | ((n->num_queues - 2) << 
16));
 break;
 default:
 return NVME_INVALID_FIELD | NVME_DNR;
@@ -636,7 +636,7 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, 
NvmeRequest *req)
 break;
 case NVME_NUMBER_OF_QUEUES:
 req->cqe.result =
-cpu_to_le32((n->num_queues - 1) | ((n->num_queues - 1) << 16));
+cpu_to_le32((n->num_queues - 2) | ((n->num_queues - 2) << 16));
 break;
 default:
 return NVME_INVALID_FIELD | NVME_DNR;
-- 
2.13.5

[Qemu-devel] [PULL for-2.10 11/15] scripts: add argparse module for Python 2.6 compatibility

[Stefan Hajnoczi]

The minimum Python version supported by QEMU is 2.6.  The argparse
standard library module was only added in Python 2.7.  Many scripts
would like to use argparse because it supports command-line
sub-commands.

This patch adds argparse.  See the top of argparse.py for details.

Suggested-by: Daniel P. Berrange 
Signed-off-by: Stefan Hajnoczi 
Acked-by: John Snow 
Message-id: 20170825155732.15665-2-stefa...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 COPYING.PYTHON  |  270 ++
 scripts/argparse.py | 2406 +++
 2 files changed, 2676 insertions(+)
 create mode 100644 COPYING.PYTHON
 create mode 100644 scripts/argparse.py

diff --git a/COPYING.PYTHON b/COPYING.PYTHON
new file mode 100644
index 00..4d3f1ef276
--- /dev/null
+++ b/COPYING.PYTHON
@@ -0,0 +1,270 @@
+A. HISTORY OF THE SOFTWARE
+==
+
+Python was created in the early 1990s by Guido van Rossum at Stichting
+Mathematisch Centrum (CWI, see http://www.cwi.nl) in the Netherlands
+as a successor of a language called ABC.  Guido remains Python's
+principal author, although it includes many contributions from others.
+
+In 1995, Guido continued his work on Python at the Corporation for
+National Research Initiatives (CNRI, see http://www.cnri.reston.va.us)
+in Reston, Virginia where he released several versions of the
+software.
+
+In May 2000, Guido and the Python core development team moved to
+BeOpen.com to form the BeOpen PythonLabs team.  In October of the same
+year, the PythonLabs team moved to Digital Creations (now Zope
+Corporation, see http://www.zope.com).  In 2001, the Python Software
+Foundation (PSF, see http://www.python.org/psf/) was formed, a
+non-profit organization created specifically to own Python-related
+Intellectual Property.  Zope Corporation is a sponsoring member of
+the PSF.
+
+All Python releases are Open Source (see http://www.opensource.org for
+the Open Source Definition).  Historically, most, but not all, Python
+releases have also been GPL-compatible; the table below summarizes
+the various releases.
+
+Release Derived YearOwner   GPL-
+fromcompatible? (1)
+
+0.9.0 thru 1.2  1991-1995   CWI yes
+1.3 thru 1.5.2  1.2 1995-1999   CNRIyes
+1.6 1.5.2   2000CNRIno
+2.0 1.6 2000BeOpen.com  no
+1.6.1   1.6 2001CNRIyes (2)
+2.1 2.0+1.6.1   2001PSF no
+2.0.1   2.0+1.6.1   2001PSF yes
+2.1.1   2.1+2.0.1   2001PSF yes
+2.2 2.1.1   2001PSF yes
+2.1.2   2.1.1   2002PSF yes
+2.1.3   2.1.2   2002PSF yes
+2.2.1   2.2 2002PSF yes
+2.2.2   2.2.1   2002PSF yes
+2.2.3   2.2.2   2003PSF yes
+2.3 2.2.2   2002-2003   PSF yes
+2.3.1   2.3 2002-2003   PSF yes
+2.3.2   2.3.1   2002-2003   PSF yes
+2.3.3   2.3.2   2002-2003   PSF yes
+2.3.4   2.3.3   2004PSF yes
+2.3.5   2.3.4   2005PSF yes
+2.4 2.3 2004PSF yes
+2.4.1   2.4 2005PSF yes
+2.4.2   2.4.1   2005PSF yes
+2.4.3   2.4.2   2006PSF yes
+2.5 2.4 2006PSF yes
+2.7 2.6 2010PSF yes
+
+Footnotes:
+
+(1) GPL-compatible doesn't mean that we're distributing Python under
+the GPL.  All Python licenses, unlike the GPL, let you distribute
+a modified version without making your changes open source.  The
+GPL-compatible licenses make it possible to combine Python with
+other software that is released under the GPL; the others don't.
+
+(2) According to Richard Stallman, 1.6.1 is not GPL-compatible,
+because its license has a choice of law clause.  According to
+CNRI, however, Stallman's lawyer has told CNRI's lawyer that 1.6.1
+is "not incompatible" with the GPL.
+
+Thanks to the many outside volunteers who have worked under Guido's
+direction to make these releases possible.
+
+
+B. TERMS AND CONDITIONS FOR ACCESSING OR OTHERWISE USING PYTHON
+===
+
+PYTHON SOFTWARE FOUNDATION LICENSE VERSION 2
+
+
+1. This LICENSE AGREEMENT is between the Python Software Foundation
+("PSF"), and the Individual or Organization ("Licensee") accessing and
+otherwise using this software ("Python") in source or binary form a

[Qemu-devel] [PULL for-2.10 03/15] throttle: Update the throttle_fix_bucket() documentation

[Stefan Hajnoczi]

From: Alberto Garcia 

The way the throttling algorithm works is that requests start being
throttled once the bucket level exceeds the burst limit. When we get
there the bucket leaks at the level set by the user (bkt->avg), and
that leak rate is what prevents guest I/O from exceeding the desired
limit.

If we don't allow bursts (i.e. bkt->max == 0) then we can start
throttling requests immediately. The problem with keeping the
threshold at 0 is that it only allows one request at a time, and as
soon as there's a bit of I/O from the guest every other request will
be throttled and performance will suffer considerably. That can even
make the guest unable to reach the throttle limit if that limit is
high enough, and that happens regardless of the block scheduler used
by the guest.

Increasing that threshold gives flexibility to the guest, allowing it
to perform short bursts of I/O before being throttled. Increasing the
threshold too much does not make a difference in the long run (because
it's the leak rate what defines the actual throughput) but it does
allow the guest to perform longer initial bursts and exceed the
throttle limit for a short while.

A burst value of bkt->avg / 10 allows the guest to perform 100ms'
worth of I/O at the target rate without being throttled.

Signed-off-by: Alberto Garcia 
Message-id: 
31aae6645f0d1fbf3860fb2b528b757236f0c0a7.1503580370.git.be...@igalia.com
Signed-off-by: Stefan Hajnoczi 
---
 util/throttle.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/util/throttle.c b/util/throttle.c
index b2a52b8b34..9a6bda813c 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -366,14 +366,9 @@ static void throttle_fix_bucket(LeakyBucket *bkt)
 /* zero bucket level */
 bkt->level = bkt->burst_level = 0;
 
-/* The following is done to cope with the Linux CFQ block scheduler
- * which regroup reads and writes by block of 100ms in the guest.
- * When they are two process one making reads and one making writes cfq
- * make a pattern looking like the following:
- * WWWRRWwR
- * Having a max burst value of 100ms of the average will help smooth the
- * throttling
- */
+/* If bkt->max is 0 we still want to allow short bursts of I/O
+ * from the guest, otherwise every other request will be throttled
+ * and performance will suffer considerably. */
 min = bkt->avg / 10;
 if (bkt->avg && !bkt->max) {
 bkt->max = min;
-- 
2.13.5

[Qemu-devel] [PULL for-2.10 02/15] throttle: Fix wrong variable name in the header documentation

[Stefan Hajnoczi]

From: Alberto Garcia 

The level of the burst bucket is stored in bkt.burst_level, not
bkt.burst_length.

Signed-off-by: Alberto Garcia 
Reviewed-by: Manos Pitsidianakis 
Message-id: 
49aab2711d02f285567f3b3b13a113847af33812.1503580370.git.be...@igalia.com
Signed-off-by: Stefan Hajnoczi 
---
 include/qemu/throttle.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index d056008c18..66a8ac10a4 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -63,7 +63,7 @@ typedef enum {
  * - The bkt.avg rate does not apply until the bucket is full,
  *   allowing the user to do bursts until then. The I/O limit during
  *   bursts is bkt.max. To enforce this limit we keep an additional
- *   bucket in bkt.burst_length that leaks at a rate of bkt.max units
+ *   bucket in bkt.burst_level that leaks at a rate of bkt.max units
  *   per second.
  *
  * - Because of all of the above, the user can perform I/O at a
-- 
2.13.5

[Qemu-devel] [PULL for-2.10 09/15] oslib-posix: Print errors before aborting on qemu_alloc_stack()

[Stefan Hajnoczi]

From: Eduardo Habkost 

If QEMU is running on a system that's out of memory and mmap()
fails, QEMU aborts with no error message at all, making it hard
to debug the reason for the failure.

Add perror() calls that will print error information before
aborting.

Signed-off-by: Eduardo Habkost 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Message-id: 20170829212053.6003-1-ehabk...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 util/oslib-posix.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index cacf0ef5e3..80086c549f 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -530,6 +530,7 @@ void *qemu_alloc_stack(size_t *sz)
 ptr = mmap(NULL, *sz, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
 if (ptr == MAP_FAILED) {
+perror("failed to allocate memory for stack");
 abort();
 }
 
@@ -544,6 +545,7 @@ void *qemu_alloc_stack(size_t *sz)
 guardpage = ptr;
 #endif
 if (mprotect(guardpage, pagesz, PROT_NONE) != 0) {
+perror("failed to set up stack guard page");
 abort();
 }
 
-- 
2.13.5

[Qemu-devel] [PULL for-2.10 04/15] throttle: Make throttle_is_valid() a bit less verbose

[Stefan Hajnoczi]

From: Alberto Garcia 

Use a pointer to the bucket instead of repeating cfg->buckets[i] all
the time. This makes the code more concise and will help us expand the
checks later and save a few line breaks.

Signed-off-by: Alberto Garcia 
Message-id: 
763ffc40a26b17d54cf93f5a999e4656049fcf0c.1503580370.git.be...@igalia.com
Signed-off-by: Stefan Hajnoczi 
---
 util/throttle.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/util/throttle.c b/util/throttle.c
index 9a6bda813c..bde56fe3de 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -324,32 +324,31 @@ bool throttle_is_valid(ThrottleConfig *cfg, Error **errp)
 }
 
 for (i = 0; i < BUCKETS_COUNT; i++) {
-if (cfg->buckets[i].avg < 0 ||
-cfg->buckets[i].max < 0 ||
-cfg->buckets[i].avg > THROTTLE_VALUE_MAX ||
-cfg->buckets[i].max > THROTTLE_VALUE_MAX) {
+LeakyBucket *bkt = &cfg->buckets[i];
+if (bkt->avg < 0 || bkt->max < 0 ||
+bkt->avg > THROTTLE_VALUE_MAX || bkt->max > THROTTLE_VALUE_MAX) {
 error_setg(errp, "bps/iops/max values must be within [0, %lld]",
THROTTLE_VALUE_MAX);
 return false;
 }
 
-if (!cfg->buckets[i].burst_length) {
+if (!bkt->burst_length) {
 error_setg(errp, "the burst length cannot be 0");
 return false;
 }
 
-if (cfg->buckets[i].burst_length > 1 && !cfg->buckets[i].max) {
+if (bkt->burst_length > 1 && !bkt->max) {
 error_setg(errp, "burst length set without burst rate");
 return false;
 }
 
-if (cfg->buckets[i].max && !cfg->buckets[i].avg) {
+if (bkt->max && !bkt->avg) {
 error_setg(errp, "bps_max/iops_max require corresponding"
" bps/iops values");
 return false;
 }
 
-if (cfg->buckets[i].max && cfg->buckets[i].max < cfg->buckets[i].avg) {
+if (bkt->max && bkt->max < bkt->avg) {
 error_setg(errp, "bps_max/iops_max cannot be lower than bps/iops");
 return false;
 }
-- 
2.13.5

[Qemu-devel] [PULL for-2.10 06/15] throttle: Make LeakyBucket.avg and LeakyBucket.max integer types

[Stefan Hajnoczi]

From: Alberto Garcia 

Both the throttling limits set with the throttling.iops-* and
throttling.bps-* options and their QMP equivalents defined in the
BlockIOThrottle struct are integer values.

Those limits are also reported in the BlockDeviceInfo struct and they
are integers there as well.

Therefore there's no reason to store them internally as double and do
the conversion everytime we're setting or querying them, so this patch
uses uint64_t for those types. Let's also use an unsigned type because
we don't allow negative values anyway.

LeakyBucket.level and LeakyBucket.burst_level do however remain double
because their value changes depending on the fraction of time elapsed
since the previous I/O operation.

Signed-off-by: Alberto Garcia 
Message-id: 
f29b840422767b5be2c41c2dfdbbbf6c5f8fedf8.1503580370.git.be...@igalia.com
Signed-off-by: Stefan Hajnoczi 
---
 include/qemu/throttle.h | 4 ++--
 tests/test-throttle.c   | 3 ++-
 util/throttle.c | 7 +++
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index 66a8ac10a4..6e31155fd4 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -77,8 +77,8 @@ typedef enum {
  */
 
 typedef struct LeakyBucket {
-double  avg;  /* average goal in units per second */
-double  max;  /* leaky bucket max burst in units */
+uint64_t avg; /* average goal in units per second */
+uint64_t max; /* leaky bucket max burst in units */
 double  level;/* bucket level in units */
 double  burst_level;  /* bucket level in units (for computing bursts) 
*/
 unsigned burst_length;/* max length of the burst period, in seconds */
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 768f11dfed..41c0dd2529 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -284,13 +284,14 @@ static void test_enabled(void)
 for (i = 0; i < BUCKETS_COUNT; i++) {
 throttle_config_init(&cfg);
 set_cfg_value(false, i, 150);
+g_assert(throttle_is_valid(&cfg, NULL));
 g_assert(throttle_enabled(&cfg));
 }
 
 for (i = 0; i < BUCKETS_COUNT; i++) {
 throttle_config_init(&cfg);
 set_cfg_value(false, i, -150);
-g_assert(!throttle_enabled(&cfg));
+g_assert(!throttle_is_valid(&cfg, NULL));
 }
 }
 
diff --git a/util/throttle.c b/util/throttle.c
index 4e80a7ea54..80660ffd2c 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -106,13 +106,13 @@ int64_t throttle_compute_wait(LeakyBucket *bkt)
 /* If bkt->max is 0 we still want to allow short bursts of I/O
  * from the guest, otherwise every other request will be throttled
  * and performance will suffer considerably. */
-bucket_size = bkt->avg / 10;
+bucket_size = (double) bkt->avg / 10;
 burst_bucket_size = 0;
 } else {
 /* If we have a burst limit then we have to wait until all I/O
  * at burst rate has finished before throttling to bkt->avg */
 bucket_size = bkt->max * bkt->burst_length;
-burst_bucket_size = bkt->max / 10;
+burst_bucket_size = (double) bkt->max / 10;
 }
 
 /* If the main bucket is full then we have to wait */
@@ -338,8 +338,7 @@ bool throttle_is_valid(ThrottleConfig *cfg, Error **errp)
 
 for (i = 0; i < BUCKETS_COUNT; i++) {
 LeakyBucket *bkt = &cfg->buckets[i];
-if (bkt->avg < 0 || bkt->max < 0 ||
-bkt->avg > THROTTLE_VALUE_MAX || bkt->max > THROTTLE_VALUE_MAX) {
+if (bkt->avg > THROTTLE_VALUE_MAX || bkt->max > THROTTLE_VALUE_MAX) {
 error_setg(errp, "bps/iops/max values must be within [0, %lld]",
THROTTLE_VALUE_MAX);
 return false;
-- 
2.13.5

[Qemu-devel] [PULL for-2.10 05/15] throttle: Remove throttle_fix_bucket() / throttle_unfix_bucket()

[Stefan Hajnoczi]

From: Alberto Garcia 

The throttling code can change internally the value of bkt->max if it
hasn't been set by the user. The problem with this is that if we want
to retrieve the original value we have to undo this change first. This
is ugly and unnecessary: this patch removes the throttle_fix_bucket()
and throttle_unfix_bucket() functions completely and moves the logic
to throttle_compute_wait().

Signed-off-by: Alberto Garcia 
Reviewed-by: Manos Pitsidianakis 
Message-id: 
5b0b9e1ac6eb208d709eddc7b09e7669a523bff3.1503580370.git.be...@igalia.com
Signed-off-by: Stefan Hajnoczi 
---
 util/throttle.c | 62 +
 1 file changed, 23 insertions(+), 39 deletions(-)

diff --git a/util/throttle.c b/util/throttle.c
index bde56fe3de..4e80a7ea54 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -95,23 +95,36 @@ static int64_t throttle_do_compute_wait(double limit, 
double extra)
 int64_t throttle_compute_wait(LeakyBucket *bkt)
 {
 double extra; /* the number of extra units blocking the io */
+double bucket_size;   /* I/O before throttling to bkt->avg */
+double burst_bucket_size; /* Before throttling to bkt->max */
 
 if (!bkt->avg) {
 return 0;
 }
 
-/* If the bucket is full then we have to wait */
-extra = bkt->level - bkt->max * bkt->burst_length;
+if (!bkt->max) {
+/* If bkt->max is 0 we still want to allow short bursts of I/O
+ * from the guest, otherwise every other request will be throttled
+ * and performance will suffer considerably. */
+bucket_size = bkt->avg / 10;
+burst_bucket_size = 0;
+} else {
+/* If we have a burst limit then we have to wait until all I/O
+ * at burst rate has finished before throttling to bkt->avg */
+bucket_size = bkt->max * bkt->burst_length;
+burst_bucket_size = bkt->max / 10;
+}
+
+/* If the main bucket is full then we have to wait */
+extra = bkt->level - bucket_size;
 if (extra > 0) {
 return throttle_do_compute_wait(bkt->avg, extra);
 }
 
-/* If the bucket is not full yet we have to make sure that we
- * fulfill the goal of bkt->max units per second. */
+/* If the main bucket is not full yet we still have to check the
+ * burst bucket in order to enforce the burst limit */
 if (bkt->burst_length > 1) {
-/* We use 1/10 of the max value to smooth the throttling.
- * See throttle_fix_bucket() for more details. */
-extra = bkt->burst_level - bkt->max / 10;
+extra = bkt->burst_level - burst_bucket_size;
 if (extra > 0) {
 return throttle_do_compute_wait(bkt->max, extra);
 }
@@ -357,31 +370,6 @@ bool throttle_is_valid(ThrottleConfig *cfg, Error **errp)
 return true;
 }
 
-/* fix bucket parameters */
-static void throttle_fix_bucket(LeakyBucket *bkt)
-{
-double min;
-
-/* zero bucket level */
-bkt->level = bkt->burst_level = 0;
-
-/* If bkt->max is 0 we still want to allow short bursts of I/O
- * from the guest, otherwise every other request will be throttled
- * and performance will suffer considerably. */
-min = bkt->avg / 10;
-if (bkt->avg && !bkt->max) {
-bkt->max = min;
-}
-}
-
-/* undo internal bucket parameter changes (see throttle_fix_bucket()) */
-static void throttle_unfix_bucket(LeakyBucket *bkt)
-{
-if (bkt->max < bkt->avg) {
-bkt->max = 0;
-}
-}
-
 /* Used to configure the throttle
  *
  * @ts: the throttle state we are working on
@@ -396,8 +384,10 @@ void throttle_config(ThrottleState *ts,
 
 ts->cfg = *cfg;
 
+/* Zero bucket level */
 for (i = 0; i < BUCKETS_COUNT; i++) {
-throttle_fix_bucket(&ts->cfg.buckets[i]);
+ts->cfg.buckets[i].level = 0;
+ts->cfg.buckets[i].burst_level = 0;
 }
 
 ts->previous_leak = qemu_clock_get_ns(clock_type);
@@ -410,13 +400,7 @@ void throttle_config(ThrottleState *ts,
  */
 void throttle_get_config(ThrottleState *ts, ThrottleConfig *cfg)
 {
-int i;
-
 *cfg = ts->cfg;
-
-for (i = 0; i < BUCKETS_COUNT; i++) {
-throttle_unfix_bucket(&cfg->buckets[i]);
-}
 }
 
 
-- 
2.13.5

[Qemu-devel] [PULL for-2.10 14/15] qemu-doc: Add UUID support in initiator name

[Stefan Hajnoczi]

From: Fred Rolland 

Update doc with the usage of UUID for initiator name.

Related-To: https://bugzilla.redhat.com/1006468
Signed-off-by: Fred Rolland 
Message-id: 20170823084830.30500-1-froll...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 qemu-doc.texi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/qemu-doc.texi b/qemu-doc.texi
index 9811476ac1..4076226f39 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -1066,10 +1066,11 @@ in a configuration file provided via '-readconfig' or 
directly on the
 command line.
 
 If the initiator-name is not specified qemu will use a default name
-of 'iqn.2008-11.org.linux-kvm[:'] where  is the name of the
+of 'iqn.2008-11.org.linux-kvm[:'] where  is the UUID of the
+virtual machine. If the UUID is not specified qemu will use
+'iqn.2008-11.org.linux-kvm[:'] where  is the name of the
 virtual machine.
 
-
 @example
 Setting a specific initiator name to use when logging in to the target
 -iscsi initiator-name=iqn.qemu.test:my-initiator
-- 
2.13.5

[Qemu-devel] [PULL for-2.10 08/15] throttle: Test the valid range of config values

[Stefan Hajnoczi]

From: Alberto Garcia 

Signed-off-by: Alberto Garcia 
Message-id: 
a57dd6274e1b6dc9c28769fec4c7ea543be5c5e3.1503580370.git.be...@igalia.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/test-throttle.c | 77 +++
 1 file changed, 77 insertions(+)

diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 41c0dd2529..bf7a5a648a 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -378,6 +378,82 @@ static void test_is_valid(void)
 test_is_valid_for_value(1, true);
 }
 
+static void test_ranges(void)
+{
+int i;
+
+for (i = 0; i < BUCKETS_COUNT; i++) {
+LeakyBucket *b = &cfg.buckets[i];
+throttle_config_init(&cfg);
+
+/* avg = 0 means throttling is disabled, but the config is valid */
+b->avg = 0;
+g_assert(throttle_is_valid(&cfg, NULL));
+g_assert(!throttle_enabled(&cfg));
+
+/* These are valid configurations (values <= THROTTLE_VALUE_MAX) */
+b->avg = 1;
+g_assert(throttle_is_valid(&cfg, NULL));
+
+b->avg = THROTTLE_VALUE_MAX;
+g_assert(throttle_is_valid(&cfg, NULL));
+
+b->avg = THROTTLE_VALUE_MAX;
+b->max = THROTTLE_VALUE_MAX;
+g_assert(throttle_is_valid(&cfg, NULL));
+
+/* Values over THROTTLE_VALUE_MAX are not allowed */
+b->avg = THROTTLE_VALUE_MAX + 1;
+g_assert(!throttle_is_valid(&cfg, NULL));
+
+b->avg = THROTTLE_VALUE_MAX;
+b->max = THROTTLE_VALUE_MAX + 1;
+g_assert(!throttle_is_valid(&cfg, NULL));
+
+/* burst_length must be between 1 and THROTTLE_VALUE_MAX */
+b->avg = 1;
+b->max = 1;
+b->burst_length = 0;
+g_assert(!throttle_is_valid(&cfg, NULL));
+
+b->avg = 1;
+b->max = 1;
+b->burst_length = 1;
+g_assert(throttle_is_valid(&cfg, NULL));
+
+b->avg = 1;
+b->max = 1;
+b->burst_length = THROTTLE_VALUE_MAX;
+g_assert(throttle_is_valid(&cfg, NULL));
+
+b->avg = 1;
+b->max = 1;
+b->burst_length = THROTTLE_VALUE_MAX + 1;
+g_assert(!throttle_is_valid(&cfg, NULL));
+
+/* burst_length * max cannot exceed THROTTLE_VALUE_MAX */
+b->avg = 1;
+b->max = 2;
+b->burst_length = THROTTLE_VALUE_MAX / 2;
+g_assert(throttle_is_valid(&cfg, NULL));
+
+b->avg = 1;
+b->max = 3;
+b->burst_length = THROTTLE_VALUE_MAX / 2;
+g_assert(!throttle_is_valid(&cfg, NULL));
+
+b->avg = 1;
+b->max = THROTTLE_VALUE_MAX;
+b->burst_length = 1;
+g_assert(throttle_is_valid(&cfg, NULL));
+
+b->avg = 1;
+b->max = THROTTLE_VALUE_MAX;
+b->burst_length = 2;
+g_assert(!throttle_is_valid(&cfg, NULL));
+}
+}
+
 static void test_max_is_missing_limit(void)
 {
 int i;
@@ -669,6 +745,7 @@ int main(int argc, char **argv)
 g_test_add_func("/throttle/config/enabled", test_enabled);
 g_test_add_func("/throttle/config/conflicting", test_conflicting_config);
 g_test_add_func("/throttle/config/is_valid",test_is_valid);
+g_test_add_func("/throttle/config/ranges",  test_ranges);
 g_test_add_func("/throttle/config/max", test_max_is_missing_limit);
 g_test_add_func("/throttle/config/iops_size",
 test_iops_size_is_missing_limit);
-- 
2.13.5

[Qemu-devel] [PULL for-2.10 15/15] qcow2: allocate cluster_cache/cluster_data on demand

[Stefan Hajnoczi]

Most qcow2 files are uncompressed so it is wasteful to allocate (32 + 1)
* cluster_size + 512 bytes upfront.  Allocate s->cluster_cache and
s->cluster_data when the first read operation is performance on a
compressed cluster.

The buffers are freed in .bdrv_close().  .bdrv_open() no longer has any
code paths that can allocate these buffers, so remove the free functions
in the error code path.

This patch can result in significant memory savings when many qcow2
disks are attached or backing file chains are long:

Before 12.81% (1,023,193,088B)
After   5.36% (393,893,888B)

Reported-by: Alexey Kardashevskiy 
Tested-by: Alexey Kardashevskiy 
Reviewed-by: Eric Blake 
Signed-off-by: Stefan Hajnoczi 
Message-id: 20170821135530.32344-1-stefa...@redhat.com
Cc: Kevin Wolf 
Signed-off-by: Stefan Hajnoczi 
---
 block/qcow2-cluster.c | 17 +
 block/qcow2.c | 12 
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index f06c08f64c..8538533102 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1516,6 +1516,23 @@ int qcow2_decompress_cluster(BlockDriverState *bs, 
uint64_t cluster_offset)
 nb_csectors = ((cluster_offset >> s->csize_shift) & s->csize_mask) + 1;
 sector_offset = coffset & 511;
 csize = nb_csectors * 512 - sector_offset;
+
+/* Allocate buffers on first decompress operation, most images are
+ * uncompressed and the memory overhead can be avoided.  The buffers
+ * are freed in .bdrv_close().
+ */
+if (!s->cluster_data) {
+/* one more sector for decompressed data alignment */
+s->cluster_data = qemu_try_blockalign(bs->file->bs,
+QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size + 512);
+if (!s->cluster_data) {
+return -ENOMEM;
+}
+}
+if (!s->cluster_cache) {
+s->cluster_cache = g_malloc(s->cluster_size);
+}
+
 BLKDBG_EVENT(bs->file, BLKDBG_READ_COMPRESSED);
 ret = bdrv_read(bs->file, coffset >> 9, s->cluster_data,
 nb_csectors);
diff --git a/block/qcow2.c b/block/qcow2.c
index fbfffadc76..a3679c69e8 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1360,16 +1360,6 @@ static int qcow2_do_open(BlockDriverState *bs, QDict 
*options, int flags,
 goto fail;
 }
 
-s->cluster_cache = g_malloc(s->cluster_size);
-/* one more sector for decompressed data alignment */
-s->cluster_data = qemu_try_blockalign(bs->file->bs, QCOW_MAX_CRYPT_CLUSTERS
-* s->cluster_size + 512);
-if (s->cluster_data == NULL) {
-error_setg(errp, "Could not allocate temporary cluster buffer");
-ret = -ENOMEM;
-goto fail;
-}
-
 s->cluster_cache_offset = -1;
 s->flags = flags;
 
@@ -1507,8 +1497,6 @@ static int qcow2_do_open(BlockDriverState *bs, QDict 
*options, int flags,
 if (s->refcount_block_cache) {
 qcow2_cache_destroy(bs, s->refcount_block_cache);
 }
-g_free(s->cluster_cache);
-qemu_vfree(s->cluster_data);
 qcrypto_block_free(s->crypto);
 qapi_free_QCryptoBlockOpenOptions(s->crypto_opts);
 return ret;
-- 
2.13.5

[Qemu-devel] [PULL for-2.10 07/15] throttle: Make burst_length 64bit and add range checks

[Stefan Hajnoczi]

From: Alberto Garcia 

LeakyBucket.burst_length is defined as an unsigned integer but the
code never checks for overflows and it only makes sure that the value
is not 0.

In practice this means that the user can set something like
throttling.iops-total-max-length=4294967300 despite being larger than
UINT_MAX and the final value after casting to unsigned int will be 4.

This patch changes the data type to uint64_t. This does not increase
the storage size of LeakyBucket, and allows us to assign the value
directly from qemu_opt_get_number() or BlockIOThrottle and then do the
checks directly in throttle_is_valid().

The value of burst_length does not have a specific upper limit,
but since the bucket size is defined by max * burst_length we have
to prevent overflows. Instead of going for UINT64_MAX or something
similar this patch reuses THROTTLE_VALUE_MAX, which allows I/O bursts
of 1 GiB/s for 10 days in a row.

Signed-off-by: Alberto Garcia 
Message-id: 
1b2e3049803f71cafb2e1fa1be4fb47147a0d398.1503580370.git.be...@igalia.com
Signed-off-by: Stefan Hajnoczi 
---
 include/qemu/throttle.h | 2 +-
 util/throttle.c | 5 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index 6e31155fd4..8e01885d29 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -81,7 +81,7 @@ typedef struct LeakyBucket {
 uint64_t max; /* leaky bucket max burst in units */
 double  level;/* bucket level in units */
 double  burst_level;  /* bucket level in units (for computing bursts) 
*/
-unsigned burst_length;/* max length of the burst period, in seconds */
+uint64_t burst_length;/* max length of the burst period, in seconds */
 } LeakyBucket;
 
 /* The following structure is used to configure a ThrottleState
diff --git a/util/throttle.c b/util/throttle.c
index 80660ffd2c..b8c524336c 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -354,6 +354,11 @@ bool throttle_is_valid(ThrottleConfig *cfg, Error **errp)
 return false;
 }
 
+if (bkt->max && bkt->burst_length > THROTTLE_VALUE_MAX / bkt->max) {
+error_setg(errp, "burst length too high for this burst rate");
+return false;
+}
+
 if (bkt->max && !bkt->avg) {
 error_setg(errp, "bps_max/iops_max require corresponding"
" bps/iops values");
-- 
2.13.5

[Qemu-devel] [PULL for-2.10 10/15] misc: Remove unused Error variables

[Stefan Hajnoczi]

From: Alberto Garcia 

There's a few cases which we're passing an Error pointer to a function
only to discard it immediately afterwards without checking it. In
these cases we can simply remove the variable and pass NULL instead.

Signed-off-by: Alberto Garcia 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Eric Blake 
Message-id: 20170829120836.16091-1-be...@igalia.com
Signed-off-by: Stefan Hajnoczi 
---
 block/qcow.c  | 12 +++-
 block/qcow2.c |  8 ++--
 dump.c|  4 +---
 3 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index c08cdc4a7b..63904a26ee 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -454,13 +454,11 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
 start_sect = (offset & ~(s->cluster_size - 1)) >> 9;
 for(i = 0; i < s->cluster_sectors; i++) {
 if (i < n_start || i >= n_end) {
-Error *err = NULL;
 memset(s->cluster_data, 0x00, 512);
 if (qcrypto_block_encrypt(s->crypto, start_sect + 
i,
   s->cluster_data,
   BDRV_SECTOR_SIZE,
-  &err) < 0) {
-error_free(err);
+  NULL) < 0) {
 errno = EIO;
 return -1;
 }
@@ -572,7 +570,6 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, 
int64_t sector_num,
 QEMUIOVector hd_qiov;
 uint8_t *buf;
 void *orig_buf;
-Error *err = NULL;
 
 if (qiov->niov > 1) {
 buf = orig_buf = qemu_try_blockalign(bs, qiov->size);
@@ -637,7 +634,7 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, 
int64_t sector_num,
 if (bs->encrypted) {
 assert(s->crypto);
 if (qcrypto_block_decrypt(s->crypto, sector_num, buf,
-  n * BDRV_SECTOR_SIZE, &err) < 0) {
+  n * BDRV_SECTOR_SIZE, NULL) < 0) {
 goto fail;
 }
 }
@@ -660,7 +657,6 @@ done:
 return ret;
 
 fail:
-error_free(err);
 ret = -EIO;
 goto done;
 }
@@ -709,11 +705,9 @@ static coroutine_fn int qcow_co_writev(BlockDriverState 
*bs, int64_t sector_num,
 break;
 }
 if (bs->encrypted) {
-Error *err = NULL;
 assert(s->crypto);
 if (qcrypto_block_encrypt(s->crypto, sector_num, buf,
-  n * BDRV_SECTOR_SIZE, &err) < 0) {
-error_free(err);
+  n * BDRV_SECTOR_SIZE, NULL) < 0) {
 ret = -EIO;
 break;
 }
diff --git a/block/qcow2.c b/block/qcow2.c
index 40ba26c111..fbfffadc76 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1820,15 +1820,13 @@ static coroutine_fn int 
qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
 assert(s->crypto);
 assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
 assert((cur_bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
-Error *err = NULL;
 if (qcrypto_block_decrypt(s->crypto,
   (s->crypt_physical_offset ?
cluster_offset + offset_in_cluster :
offset) >> BDRV_SECTOR_BITS,
   cluster_data,
   cur_bytes,
-  &err) < 0) {
-error_free(err);
+  NULL) < 0) {
 ret = -EIO;
 goto fail;
 }
@@ -1942,7 +1940,6 @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState 
*bs, uint64_t offset,
 qemu_iovec_concat(&hd_qiov, qiov, bytes_done, cur_bytes);
 
 if (bs->encrypted) {
-Error *err = NULL;
 assert(s->crypto);
 if (!cluster_data) {
 cluster_data = qemu_try_blockalign(bs->file->bs,
@@ -1963,8 +1960,7 @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState 
*bs, uint64_t offset,
cluster_offset + offset_in_cluster :
offset) >> BDRV_SECTOR_BITS,
   cluster_data,
-  cur_bytes, &err) < 0) {
-error_free(err);
+  cur_bytes, NULL) < 0) {
 ret = -EIO;
 goto fail;
 }
diff --git a/dump.c b/dump.c
index d9090a24cc..a79773d0f7

[Qemu-devel] [PULL for-2.10 13/15] tests: migration/guestperf Python 2.6 argparse compatibility

[Stefan Hajnoczi]

Add the scripts/ directory to sys.path so Python 2.6 will be able to
import argparse.

Cc: Daniel P. Berrange 
Signed-off-by: Stefan Hajnoczi 
Acked-by: John Snow 
Acked-by: Fam Zheng 
Message-id: 20170825155732.15665-4-stefa...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/migration/guestperf/shell.py | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tests/migration/guestperf/shell.py 
b/tests/migration/guestperf/shell.py
index 185c5697a6..7992459a97 100644
--- a/tests/migration/guestperf/shell.py
+++ b/tests/migration/guestperf/shell.py
@@ -18,12 +18,14 @@
 #
 
 
-import argparse
-import fnmatch
 import os
 import os.path
-import platform
 import sys
+sys.path.append(os.path.join(os.path.dirname(__file__),
+ '..', '..', '..', 'scripts'))
+import argparse
+import fnmatch
+import platform
 
 from guestperf.hardware import Hardware
 from guestperf.engine import Engine
-- 
2.13.5

[Qemu-devel] [PULL for-2.10 12/15] docker.py: Python 2.6 argparse compatibility

[Stefan Hajnoczi]

Add the scripts/ directory to sys.path so Python 2.6 will be able to
import argparse.

Cc: Fam Zheng 
Signed-off-by: Stefan Hajnoczi 
Acked-by: John Snow 
Acked-by: Fam Zheng 
Message-id: 20170825155732.15665-3-stefa...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/docker/docker.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/docker/docker.py b/tests/docker/docker.py
index ee40ca04d9..81c87ee329 100755
--- a/tests/docker/docker.py
+++ b/tests/docker/docker.py
@@ -13,12 +13,14 @@
 
 import os
 import sys
+sys.path.append(os.path.join(os.path.dirname(__file__),
+ '..', '..', 'scripts'))
+import argparse
 import subprocess
 import json
 import hashlib
 import atexit
 import uuid
-import argparse
 import tempfile
 import re
 import signal
-- 
2.13.5

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 8/8] ppc: remove non implemented cpu models

[Igor Mammedov]

On Thu, 31 Aug 2017 09:58:45 +0200
Thomas Huth  wrote:

> On 30.08.2017 15:24, Igor Mammedov wrote:
> > Remove cpu models that aren't implemented and are not
> > compiled/tested since they are under TODO ifdef
> > which isn't defined in sources.
> > 
> > If someone really needs a removed model he/she should add
> > as regular one with corresponding implementation.
> > 
> > Signed-off-by: Igor Mammedov 
> > ---
> >  target/ppc/cpu-models.c | 459 
> > 
> >  1 file changed, 459 deletions(-)
> > 
> > diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
> > index cf878a9..611fc1b 100644
> > --- a/target/ppc/cpu-models.c
> > +++ b/target/ppc/cpu-models.c  
> [...]
> > -#endif
> >  #if defined(TODO_USER_ONLY)
> >  POWERPC_DEF("440sp", CPU_POWERPC_440SP,  440EP,
> >  "PowerPC 440 SP")
> > @@ -396,20 +207,6 @@
> >  POWERPC_DEF("440spe",CPU_POWERPC_440SPE, 440EP,
> >  "PowerPC 440 SPE")
> >  #endif
> > -/* PowerPC 460 family  
> >   */
> > -#if defined(TODO)
> > -POWERPC_DEF("464",   CPU_POWERPC_464,460,
> > -"Generic PowerPC 464")
> > -#endif
> > -/* PowerPC 464 microcontrollers
> >   */
> > -#if defined(TODO)
> > -POWERPC_DEF("464h90",CPU_POWERPC_464H90, 460,
> > -"PowerPC 464H90")
> > -#endif
> > -#if defined(TODO)
> > -POWERPC_DEF("464h90f",   CPU_POWERPC_464H90F,460F,
> > -"PowerPC 464H90F")
> > -#endif  
> 
> By the way, I guess you could also remove the 460 stuff from
> translate_init.c since there are no 460 CPUs defined in QEMU
> (but I guess that should go into a separate patch instead).

I would leave it to someone who knows more about PPC.
Would you like to post one?

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 8/8] ppc: remove non implemented cpu models

[Thomas Huth]

On 31.08.2017 10:35, Igor Mammedov wrote:
> On Thu, 31 Aug 2017 09:58:45 +0200
> Thomas Huth  wrote:
> 
>> On 30.08.2017 15:24, Igor Mammedov wrote:
>>> Remove cpu models that aren't implemented and are not
>>> compiled/tested since they are under TODO ifdef
>>> which isn't defined in sources.
>>>
>>> If someone really needs a removed model he/she should add
>>> as regular one with corresponding implementation.
>>>
>>> Signed-off-by: Igor Mammedov 
>>> ---
>>>  target/ppc/cpu-models.c | 459 
>>> 
>>>  1 file changed, 459 deletions(-)
>>>
>>> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
>>> index cf878a9..611fc1b 100644
>>> --- a/target/ppc/cpu-models.c
>>> +++ b/target/ppc/cpu-models.c  
>> [...]
>>> -#endif
>>>  #if defined(TODO_USER_ONLY)
>>>  POWERPC_DEF("440sp", CPU_POWERPC_440SP,  440EP,
>>>  "PowerPC 440 SP")
>>> @@ -396,20 +207,6 @@
>>>  POWERPC_DEF("440spe",CPU_POWERPC_440SPE, 440EP,
>>>  "PowerPC 440 SPE")
>>>  #endif
>>> -/* PowerPC 460 family  
>>>   */
>>> -#if defined(TODO)
>>> -POWERPC_DEF("464",   CPU_POWERPC_464,460,
>>> -"Generic PowerPC 464")
>>> -#endif
>>> -/* PowerPC 464 microcontrollers
>>>   */
>>> -#if defined(TODO)
>>> -POWERPC_DEF("464h90",CPU_POWERPC_464H90, 460,
>>> -"PowerPC 464H90")
>>> -#endif
>>> -#if defined(TODO)
>>> -POWERPC_DEF("464h90f",   CPU_POWERPC_464H90F,460F,
>>> -"PowerPC 464H90F")
>>> -#endif  
>>
>> By the way, I guess you could also remove the 460 stuff from
>> translate_init.c since there are no 460 CPUs defined in QEMU
>> (but I guess that should go into a separate patch instead).
> 
> I would leave it to someone who knows more about PPC.
> Would you like to post one?

Sure, I'll try to come up with a patch once this one here has been merged.

 Thomas

Re: [Qemu-devel] [PULL for-2.10 00/15] Block patches

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20170831082210.8362-1-stefa...@redhat.com
Subject: [Qemu-devel] [PULL for-2.10 00/15] Block patches
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]patchew/20170830215523.25278-1-f4...@amsat.org -> 
patchew/20170830215523.25278-1-f4...@amsat.org
 * [new tag]   patchew/20170831082210.8362-1-stefa...@redhat.com -> 
patchew/20170831082210.8362-1-stefa...@redhat.com
Switched to a new branch 'test'
eaf80da3e4 qcow2: allocate cluster_cache/cluster_data on demand
fab9c02727 qemu-doc: Add UUID support in initiator name
76e70ed0d9 tests: migration/guestperf Python 2.6 argparse compatibility
316bda7e7b docker.py: Python 2.6 argparse compatibility
0a7f8a625b scripts: add argparse module for Python 2.6 compatibility
3e6e26d514 misc: Remove unused Error variables
f01d195374 oslib-posix: Print errors before aborting on qemu_alloc_stack()
b2f08be812 throttle: Test the valid range of config values
f1cd43f5e4 throttle: Make burst_length 64bit and add range checks
60981818d1 throttle: Make LeakyBucket.avg and LeakyBucket.max integer types
3032f687f4 throttle: Remove throttle_fix_bucket() / throttle_unfix_bucket()
73e3d4f547 throttle: Make throttle_is_valid() a bit less verbose
0ae5ba07ca throttle: Update the throttle_fix_bucket() documentation
9f52f0ea86 throttle: Fix wrong variable name in the header documentation
062842b9f2 nvme: Fix get/set number of queues feature, again

=== OUTPUT BEGIN ===
Checking PATCH 1/15: nvme: Fix get/set number of queues feature, again...
Checking PATCH 2/15: throttle: Fix wrong variable name in the header 
documentation...
Checking PATCH 3/15: throttle: Update the throttle_fix_bucket() documentation...
Checking PATCH 4/15: throttle: Make throttle_is_valid() a bit less verbose...
Checking PATCH 5/15: throttle: Remove throttle_fix_bucket() / 
throttle_unfix_bucket()...
Checking PATCH 6/15: throttle: Make LeakyBucket.avg and LeakyBucket.max integer 
types...
Checking PATCH 7/15: throttle: Make burst_length 64bit and add range checks...
Checking PATCH 8/15: throttle: Test the valid range of config values...
Checking PATCH 9/15: oslib-posix: Print errors before aborting on 
qemu_alloc_stack()...
Checking PATCH 10/15: misc: Remove unused Error variables...
Checking PATCH 11/15: scripts: add argparse module for Python 2.6 
compatibility...
ERROR: trailing whitespace
#118: FILE: COPYING.PYTHON:93:
+Reserved" are retained in Python alone or in any derivative version $

total: 1 errors, 0 warnings, 2676 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 12/15: docker.py: Python 2.6 argparse compatibility...
Checking PATCH 13/15: tests: migration/guestperf Python 2.6 argparse 
compatibility...
Checking PATCH 14/15: qemu-doc: Add UUID support in initiator name...
Checking PATCH 15/15: qcow2: allocate cluster_cache/cluster_data on demand...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH

[Cornelia Huck]

On Thu, 31 Aug 2017 09:32:49 +0200
Thomas Huth  wrote:

> On 31.08.2017 08:38, Cornelia Huck wrote:
> > On Thu, 31 Aug 2017 07:51:17 +0200
> > Thomas Huth  wrote:
> >   
> >> On 30.08.2017 18:36, Halil Pasic wrote:  
> >>> The function ioinst_handle_xsch is presenting cc 2 when it's supposed to
> >>> present cc 1 and the other way around, because css_do_xsch has the error
> >>> codes mixed up. Fixing the error codes also fixes the priority.
> >>>
> >>> Let us fix this.
> >>
> >> (Nit: In case you respin, I'd suggest to remove the last sentence. You
> >> already used "fix" two times in the previous one)  
> > 
> > I can also remove it on applying, if Halil agrees (I have not yet
> > reviewed it, though).
> >   
> >>  
> >>> Signed-off-by: Halil Pasic 
> >>> Reported-by: Pierre Morel
> >>
> >> Space missing -^  
> > 
> > And I can also add that space on applying :)
> >   
> >>  
> >>> ---
> >>>  hw/s390x/css.c | 4 ++--
> >>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> >>> index 1880b1a0ff..a50fb0727e 100644
> >>> --- a/hw/s390x/css.c
> >>> +++ b/hw/s390x/css.c
> >>> @@ -1281,12 +1281,12 @@ int css_do_xsch(SubchDev *sch)
> >>>  (!(s->ctrl &
> >>> (SCSW_ACTL_RESUME_PEND | SCSW_ACTL_START_PEND | 
> >>> SCSW_ACTL_SUSP))) ||
> >>>  (s->ctrl & SCSW_ACTL_SUBCH_ACTIVE)) {
> >>> -ret = -EINPROGRESS;
> >>> +ret = -EBUSY;
> >>>  goto out;
> >>>  }
> >>>  
> >>>  if (s->ctrl & SCSW_CTRL_MASK_STCTL) {
> >>> -ret = -EBUSY;
> >>> +ret = -EINPROGRESS;
> >>>  goto out;
> >>>  }
> >>
> >> Using both, EBUSY and EINPROGRESS as error codes sounds very confusing
> >> to me here ... what's the difference between busy and in-progress? So
> >> while you're at it, maybe you could replace the code for CC 2 ("CANCEL
> >> SUBCHANNEL not applicable") with a different error code?  
> > 
> > IIRC, I used these two as they matched my idea of what happens best
> > (there's a difference between "subchannel is busy" and "the I/O is
> > already in progress, too late to cancel"). "xsch not applicable" is
> > very hard to translate to an Unix error code :/  
> 
> OK, the codes make more sense with your description ==> Maybe simply add
> a proper comment for each of the return codes?

Taking a step back and looking at the other I/O instructions and their
implementation in qemu:

- For those instructions that can set it, cc 1 is set when the
  subchannel is status pending. That's usually the "default" branch in
  ioinst.c.
- cc 2 is set when the subchannel is "busy" (or, in the case of xsch,
  "not applicable for xsch"... sigh) This is supposed to be handled via
  -EBUSY.

So, there are actually two problems with the current implementation of
xsch:

- The return codes are switched around, which this patch fixes.
- But "status pending" should also take precedence over "not
  applicable", if I read the PoP correctly, so the second if needs to
  be moved up.

And yes, it makes sense do add some comments...

Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals

[Markus Armbruster]

Marc-André Lureau  writes:

> Hi
>
> - Original Message -
>> Marc-André Lureau  writes:
>> 
>> > Hi
>> >
>> > - Original Message -
>> >> Marc-André Lureau  writes:
>> >> 
>> >> > They are not considered constant expressions in C, producing an error
>> >> > when compiling a const QLit.
>> >> 
>> >> A const QLit?  Do you mean a non-const one?
>> >
>> > Really a const QLitObject:
>> >
>> >
>> > const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
>> >  QLIT_QNULL,
>> >  {}
>> >  }));
>> >
>> > qmp-introspect.c:17:63: error: initializer element is not constant
>> >   const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
>> > ^
>> > Removing the "compound literals" fixes this error.
>> 
>> Does QLIT_QLIST(((const QLitObject[]) { ... } work?
>
> No. Even if I put "const" all over the place (in member, in compound type 
> etc).
>
> Give it a try, see if you can make it const, I am out of luck.

The commit message's explanation is wrong.  This isn't about const at
all, it's about "constant expressions", which are something else
entirely.

For what it's worth, clang is cool with the compound literals.  On
Fedora 26 with a minimized test case (appended):

$ clang -c -g -O -Wall compound-lit.c
$ gcc -c -g -O -Wall compound-lit.c
compound-lit.c:30:37: error: initializer element is not constant
 .value.qdict = (QLitDictEntry[]){
 ^
compound-lit.c:30:37: note: (near initialization for ‘(anonymous).value’)

GCC bug or not?  A search of the GCC Bugzilla finds
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71713

Copying a few notorious language lawyers^W^W^Wtrusted advisers.

Even if this turns out to be a gcc bug, we'll have to work around it.
But the work-around needs a comment then.

In any case, the commit message needs fixing.



enum {
QTYPE_NONE, QTYPE_QSTRING, QTYPE_QDICT,
};

typedef struct QLitDictEntry QLitDictEntry;
typedef struct QLitObject QLitObject;

struct QLitObject {
int type;
union {
const char *qstr;
QLitDictEntry *qdict;
} value;
};

struct QLitDictEntry {
const char *key;
QLitObject value;
};

QLitObject qlit1 = (QLitObject){
.type = QTYPE_QDICT,
.value.qdict = (QLitDictEntry[]){
{ "foo", {} },
{}
}};

QLitObject qlit2 = (QLitObject){
.type = QTYPE_QDICT,
.value.qdict = (QLitDictEntry[]){
{ "foo", (QLitObject){} },
{}
}};

Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH

[Halil Pasic]

On 08/31/2017 07:51 AM, Thomas Huth wrote:
> On 30.08.2017 18:36, Halil Pasic wrote:
>> The function ioinst_handle_xsch is presenting cc 2 when it's supposed to
>> present cc 1 and the other way around, because css_do_xsch has the error
>> codes mixed up. Fixing the error codes also fixes the priority.
>>
>> Let us fix this.
> 
> (Nit: In case you respin, I'd suggest to remove the last sentence. You
> already used "fix" two times in the previous one)
> 
>> Signed-off-by: Halil Pasic 
>> Reported-by: Pierre Morel
> 
> Space missing -^
> 

copy-paste :(

>> ---
>>  hw/s390x/css.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
>> index 1880b1a0ff..a50fb0727e 100644
>> --- a/hw/s390x/css.c
>> +++ b/hw/s390x/css.c
>> @@ -1281,12 +1281,12 @@ int css_do_xsch(SubchDev *sch)
>>  (!(s->ctrl &
>> (SCSW_ACTL_RESUME_PEND | SCSW_ACTL_START_PEND | 
>> SCSW_ACTL_SUSP))) ||
>>  (s->ctrl & SCSW_ACTL_SUBCH_ACTIVE)) {
>> -ret = -EINPROGRESS;
>> +ret = -EBUSY;
>>  goto out;
>>  }
>>  
>>  if (s->ctrl & SCSW_CTRL_MASK_STCTL) {
>> -ret = -EBUSY;
>> +ret = -EINPROGRESS;
>>  goto out;
>>  }
> 
> Using both, EBUSY and EINPROGRESS as error codes sounds very confusing
> to me here ... what's the difference between busy and in-progress? So
> while you're at it, maybe you could replace the code for CC 2 ("CANCEL
> SUBCHANNEL not applicable") with a different error code?
> 
>  Thomas
> 

Well, the idea of the series is to get rid of these artificial error codes,
so your concern of using EBUSY and EINPROGRESS will be dealt with in patch
5.

The idea was to first do the fixes and then do the transformation without
changing behavior.

Thanks for having a look!

Regards,

Halil

Re: [Qemu-devel] [PATCH v1 04/11] s390x: get rid of s390-virtio.c

[Thomas Huth]

On 30.08.2017 19:05, David Hildenbrand wrote:
> It is a leftover from the days where we had still the !ccw virtio
> machine. As this one is long gone, let's move everything to
> s390-virtio-ccw.c.
> 
> Cornelia Huck 
> Signed-off-by: David Hildenbrand 
> ---
[...]
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index ffd56af834..41a9e976dc 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
[...]
> +static int gtod_load(QEMUFile *f, void *opaque, int version_id)
> +{
> +uint64_t tod_low;
> +uint8_t tod_high;
> +int r;
> +
> +if (qemu_get_byte(f) == S390_TOD_CLOCK_VALUE_MISSING) {
> +fprintf(stderr, "WARNING: Guest clock was not migrated. This could "
> +"cause the guest to hang.\n");
> +return 0;
> +}
> +
> +tod_high = qemu_get_byte(f);
> +tod_low = qemu_get_be64(f);
> +
> +r = s390_set_clock(&tod_high, &tod_low);
> +if (r) {
> +fprintf(stderr, "WARNING: Unable to set guest clock value. "
> +"s390_get_clock returned error %d. This could cause "
> +"the guest to hang.\n", r);
> +}
> +
> +return 0;
> +}
> +
> +

Nit: One empty line should be enough here.

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [RFC v2 0/8] monitor: allow per-monitor thread

[Daniel P. Berrange]

On Thu, Aug 31, 2017 at 11:31:55AM +0800, Peter Xu wrote:
> Before getting to an conclusion, just want to make sure we have got a
> consensus on that at least we should start to move the monitor command
> handling into a separate thread rather than main thread, am I correct?

Certainly agree on that, moving dispatch of monitor commands out of
the main thread is critical IMHO. The main thread should only ever
be doing work that is gauranteed non-blockable and completable in a
short, finite amount of time. This means at most it should do I/O only
for the monitor.

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 04/14] qlit: remove compound literals

[Marc-André Lureau]

Hi

On Thu, Aug 31, 2017 at 10:43 AM Markus Armbruster 
wrote:

> Marc-André Lureau  writes:
>
> > Hi
> >
> > - Original Message -
> >> Marc-André Lureau  writes:
> >>
> >> > Hi
> >> >
> >> > - Original Message -
> >> >> Marc-André Lureau  writes:
> >> >>
> >> >> > They are not considered constant expressions in C, producing an
> error
> >> >> > when compiling a const QLit.
> >> >>
> >> >> A const QLit?  Do you mean a non-const one?
> >> >
> >> > Really a const QLitObject:
> >> >
> >> >
> >> > const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
> >> >  QLIT_QNULL,
> >> >  {}
> >> >  }));
> >> >
> >> > qmp-introspect.c:17:63: error: initializer element is not constant
> >> >   const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
> >> > ^
> >> > Removing the "compound literals" fixes this error.
> >>
> >> Does QLIT_QLIST(((const QLitObject[]) { ... } work?
> >
> > No. Even if I put "const" all over the place (in member, in compound
> type etc).
> >
> > Give it a try, see if you can make it const, I am out of luck.
>
> The commit message's explanation is wrong.  This isn't about const at
> all, it's about "constant expressions", which are something else
> entirely.
>

The point was that declaring a non const QLit with those "compound
literals" worked vs with const.


> For what it's worth, clang is cool with the compound literals.  On
> Fedora 26 with a minimized test case (appended):
>
> $ clang -c -g -O -Wall compound-lit.c
> $ gcc -c -g -O -Wall compound-lit.c
> compound-lit.c:30:37: error: initializer element is not constant
>  .value.qdict = (QLitDictEntry[]){
>  ^
> compound-lit.c:30:37: note: (near initialization for
> ‘(anonymous).value’)
>
> GCC bug or not?  A search of the GCC Bugzilla finds
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71713
>
> Copying a few notorious language lawyers^W^W^Wtrusted advisers.
>
> Even if this turns out to be a gcc bug, we'll have to work around it.
> But the work-around needs a comment then.
>
> In any case, the commit message needs fixing.
>

What about adapting the bug comment:

qlit: remove compound literals

A compound literal (i.e., "(struct Str1){1}"), is not a constant
expression, and so it cannot be used to initialize an object with static
storage duration.

$ gcc -c -g -O -Wall compound-lit.c
compound-lit.c:30:37: error: initializer element is not constant
 .value.qdict = (QLitDictEntry[]){
 ^
compound-lit.c:30:37: note: (near initialization for
‘(anonymous).value’)

clang accepts it. In some cases, gcc accepts compound literals as
initializer, but not in this nested case. There is a gcc bug about it:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71713.

Feel free to adapt on commit

thanks


>
>
> enum {
> QTYPE_NONE, QTYPE_QSTRING, QTYPE_QDICT,
> };
>
> typedef struct QLitDictEntry QLitDictEntry;
> typedef struct QLitObject QLitObject;
>
> struct QLitObject {
> int type;
> union {
> const char *qstr;
> QLitDictEntry *qdict;
> } value;
> };
>
> struct QLitDictEntry {
> const char *key;
> QLitObject value;
> };
>
> QLitObject qlit1 = (QLitObject){
> .type = QTYPE_QDICT,
> .value.qdict = (QLitDictEntry[]){
> { "foo", {} },
> {}
> }};
>
> QLitObject qlit2 = (QLitObject){
> .type = QTYPE_QDICT,
> .value.qdict = (QLitDictEntry[]){
> { "foo", (QLitObject){} },
> {}
> }};
>
> --
Marc-André Lureau

Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH

[Thomas Huth]

On 31.08.2017 11:09, Halil Pasic wrote:
> 
> 
> On 08/31/2017 07:51 AM, Thomas Huth wrote:
>> On 30.08.2017 18:36, Halil Pasic wrote:
>>> The function ioinst_handle_xsch is presenting cc 2 when it's supposed to
>>> present cc 1 and the other way around, because css_do_xsch has the error
>>> codes mixed up. Fixing the error codes also fixes the priority.
>>>
>>> Let us fix this.
>>
>> (Nit: In case you respin, I'd suggest to remove the last sentence. You
>> already used "fix" two times in the previous one)
>>
>>> Signed-off-by: Halil Pasic 
>>> Reported-by: Pierre Morel
>>
>> Space missing -^
>>
> 
> copy-paste :(
> 
>>> ---
>>>  hw/s390x/css.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
>>> index 1880b1a0ff..a50fb0727e 100644
>>> --- a/hw/s390x/css.c
>>> +++ b/hw/s390x/css.c
>>> @@ -1281,12 +1281,12 @@ int css_do_xsch(SubchDev *sch)
>>>  (!(s->ctrl &
>>> (SCSW_ACTL_RESUME_PEND | SCSW_ACTL_START_PEND | 
>>> SCSW_ACTL_SUSP))) ||
>>>  (s->ctrl & SCSW_ACTL_SUBCH_ACTIVE)) {
>>> -ret = -EINPROGRESS;
>>> +ret = -EBUSY;
>>>  goto out;
>>>  }
>>>  
>>>  if (s->ctrl & SCSW_CTRL_MASK_STCTL) {
>>> -ret = -EBUSY;
>>> +ret = -EINPROGRESS;
>>>  goto out;
>>>  }
>>
>> Using both, EBUSY and EINPROGRESS as error codes sounds very confusing
>> to me here ... what's the difference between busy and in-progress? So
>> while you're at it, maybe you could replace the code for CC 2 ("CANCEL
>> SUBCHANNEL not applicable") with a different error code?
>>
>>  Thomas
>>
> 
> Well, the idea of the series is to get rid of these artificial error codes,
> so your concern of using EBUSY and EINPROGRESS will be dealt with in patch
> 5.
> 
> The idea was to first do the fixes and then do the transformation without
> changing behavior.

Yeah, I realized that when I started to look at the later patches ... so
please ignore my comment, it should be OK the way you're doing it.

 Thomas

Re: [Qemu-devel] [PATCH] io: add new qio_channel_{readv, writev, read, write}_all functions

[Daniel P. Berrange]

On Wed, Aug 30, 2017 at 02:34:59PM -0500, Eric Blake wrote:
> On 08/30/2017 08:56 AM, Daniel P. Berrange wrote:
> > These functions wait until they are able to read / write the full
> > requested data buffer(s).
> > 
> 
> Hmm, sounds like the NBD code would benefit from using this in a
> followup patch.
> 
> > Signed-off-by: Daniel P. Berrange 
> > ---
> > 
> > This patch combines these two previous proposals:
> > 
> >  - https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01536.html
> >  - https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg04041.html
> > 
> > and switches the test suite over to use the new APIs so we get
> > coverage by all the tests/test-io-channel-*  test programs
> > 
> 
> >  /**
> > + * qio_channel_readv_all:
> > + * @ioc: the channel object
> > + * @iov: the array of memory regions to read data into
> > + * @niov: the length of the @iov array
> > + * @errp: pointer to a NULL-initialized error object
> > + *
> > + * Read data from the IO channel, storing it in the
> > + * memory regions referenced by @iov. Each element
> > + * in the @iov will be fully populated with data
> > + * before the next one is used. The @niov parameter
> > + * specifies the total number of elements in @iov.
> > + *
> > + * The function will wait for all requested data
> > + * to be read, yielding from the current couroutine
> 
> s/couroutine/coroutine/
> 
> > + * if required.
> > + *
> > + * If end-of-file occurrs before all requested data
> 
> s/occurrs/occurs/
> 
> > + * has been read, an error will be reported.
> > + *
> > + * Returns: 0 if all bytes were read, or -1 on error
> > + */
> 
> > +/**
> > + * qio_channel_writev_all:
> > + * @ioc: the channel object
> > + * @iov: the array of memory regions to write data from
> > + * @niov: the length of the @iov array
> > + * @errp: pointer to a NULL-initialized error object
> > + *
> > + * Write data to the IO channel, reading it from the
> > + * memory regions referenced by @iov. Each element
> > + * in the @iov will be fully sent, before the next
> > + * one is used. The @niov parameter specifies the
> > + * total number of elements in @iov.
> > + *
> > + * The function will wait for all requested data
> > + * to be written, yielding from the current couroutine
> 
> s/couroutine/coroutine/
> 
> > +++ b/io/channel.c
> 
> > +int qio_channel_readv_all(QIOChannel *ioc,
> 
> > +while (nlocal_iov > 0) {
> > +ssize_t len;
> > +len = qio_channel_readv(ioc, local_iov, nlocal_iov, errp);
> > +if (len == QIO_CHANNEL_ERR_BLOCK) {
> > +qio_channel_wait(ioc, G_IO_IN);
> > +continue;
> > +} else if (len < 0) {
> > +error_setg_errno(errp, EIO,
> > + "Channel was not able to read full iov");
> 
> Should we report -len instead of EIO here?

No, QIO methods never return errno values, since channel implementations
are not guaranteed to be calling commands that return errnos. eg the
TLS wrapper calls gnutls which has no errno values reported.

If fact, though, we should just not call error_setg_errno() at all,
since the qio_channel_readv method has already populated 'errp'.

> 
> > +int qio_channel_writev_all(QIOChannel *ioc,
> 
> > +while (nlocal_iov > 0) {
> > +ssize_t len;
> > +len = qio_channel_writev(ioc, local_iov, nlocal_iov, errp);
> > +if (len == QIO_CHANNEL_ERR_BLOCK) {
> > +qio_channel_wait(ioc, G_IO_OUT);
> > +continue;
> > +}
> > +if (len < 0) {
> > +error_setg_errno(errp, EIO,
> > + "Channel was not able to write full iov");
> 
> and again
> 
> With the typos fixed, and either an explanation why EIO is better or
> else a fix to preserve errno,

Again, we should not call error_setg_errno at all, since errp is
already populated.

> 
> Reviewed-by: Eric Blake 




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 11/13] hvf: move fields from CPUState to CPUX86State

[Stefan Hajnoczi]

On Wed, Aug 30, 2017 at 03:27:00AM -0500, Sergio Andres Gomez Del Real wrote:
> @@ -1187,11 +1190,15 @@ typedef struct CPUX86State {
>  int32_t interrupt_injected;
>  uint8_t soft_interrupt;
>  uint8_t has_error_code;
> +uint32_t ins_len;

This field seems unused in this patch?

> @@ -678,15 +687,15 @@ int hvf_init_vcpu(CPUState *cpu)
>  sigdelset(&set, SIG_IPI);
>  
>  int r;
> -init_emu(cpu);
> -init_decoder(cpu);
> +init_emu();
> +init_decoder();
>  init_cpuid(cpu);
>  
>  hvf_state->hvf_caps = (struct hvf_vcpu_caps *)g_malloc0(sizeof(struct 
> hvf_vcpu_caps));
> -cpu->hvf_x86 = (struct hvf_x86_state *)g_malloc0(sizeof(struct 
> hvf_x86_state));
> +env->hvf_emul = (HVFX86EmulatorState 
> *)g_malloc0(sizeof(HVFX86EmulatorState));

Please use g_new0().  The cast isn't necessary because the C compiler
casts void * to any other pointer type without a warning:

  env->hvf_emul = g_new0(HVFX86EmulatorState, 1);

>  
>  r = hv_vcpu_create((hv_vcpuid_t *)&cpu->hvf_fd, HV_VCPU_DEFAULT);
> -cpu->hvf_vcpu_dirty = 1;
> +cpu->vcpu_dirty = 1;

cpu->vcpu_dirty is bool.  Please use true/false consistently instead of
1/0.

> @@ -356,13 +356,14 @@ typedef struct x68_segment_selector {
>  };
>  } __attribute__ ((__packed__)) x68_segment_selector;
>  
> -/* Definition of hvf_x86_state is here */
> -struct hvf_x86_state {
> -int hlt;
> -uint64_t init_tsc;
> +typedef struct lazy_flags {
> +addr_t result;
> +addr_t auxbits;
> +} lazy_flags;
>  
> +/* Definition of hvf_x86_state is here */

hvf_x86_state no longer exists.  This comment is outdated now?

Re: [Qemu-devel] [PATCH v2 15/17] MAINTAINERS: add missing Cryptography entry

[Daniel P. Berrange]

On Wed, Aug 30, 2017 at 06:55:21PM -0300, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0b77590dc8..7a0c00550e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1526,6 +1526,7 @@ S: Maintained
>  F: crypto/
>  F: include/crypto/
>  F: tests/test-crypto-*
> +F: tests/benchmark-crypto-*
>  F: qemu.sasl
>  
>  Coroutines

Acked-by: Daniel P. Berrange 

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 2/9] s390x: fix invalid use of cc 1 for SSCH

[Cornelia Huck]

On Wed, 30 Aug 2017 18:36:02 +0200
Halil Pasic  wrote:

> According to the POP a start subchannel instruction (SSCH) returning with
> cc 1 implies that the subchannel was status pending when SSCH executed.
> 
> Due to a somewhat confusing error handling, where error codes are mapped
> to cc value, sane looking error codes result in non AR compliant
> behavior.
> 
> Let's fix this! Instead of cc 1 we use cc 3 which means device not
> operational, and is much closer to the truth in the given cases.
> 
> Signed-off-by: Halil Pasic 
> Acked-by: Pierre Morel
> ---
> 
> This patch turned out quite controversial. We did not reach a consensus
> during the internal review.
> 
> The most of the discussion revolved around the ORB flag which
> architecturally must be supported, but are currently not supported by
> vfio-ccw (not yet, or can't be). The idea showing the most promise for
> consensus was to handle this via device status (along the lines better a
> strange acting device than a non-conform machine) but since it's a
> radical change we decided to first discuss upstream and then do whatever
> needs to be done.
> ---
>  hw/s390x/css.c  | 15 ++-
>  hw/s390x/s390-ccw.c |  2 +-
>  2 files changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index a50fb0727e..0822538cde 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -1034,7 +1034,7 @@ static int sch_handle_start_func_passthrough(SubchDev 
> *sch)
>   */
>  if (!(orb->ctrl0 & ORB_CTRL0_MASK_PFCH) ||
>  !(orb->ctrl0 & ORB_CTRL0_MASK_C64)) {
> -return -EINVAL;
> +return -ENODEV;

This feels wrong. If we don't support this yet, doing something like a
channel-program check or an operand exception feels closer to the
architecture than indicating a gone device.

>  }
>  
>  ret = s390_ccw_cmd_request(orb, s, sch->driver_data);
> @@ -1046,16 +1046,13 @@ static int sch_handle_start_func_passthrough(SubchDev 
> *sch)
>  break;
>  case -ENODEV:
>  break;
> +case -EFAULT:
> + break;
>  case -EACCES:
>  /* Let's reflect an inaccessible host device by cc 3. */
> -ret = -ENODEV;
> -break;
>  default:
> -   /*
> -* All other return codes will trigger a program check,
> -* or set cc to 1.
> -*/
> -   break;
> +/* Let's make all other return codes map to cc 3.  */
> +ret = -ENODEV;

Why? This feels wrong. For those cases where we want to signal an error
but cc 1 is conceptually wrong, either an operand exception (for very
few cases) or a channel-program check feels more in line with the
architecture.

That's a general problem with doing stuff in the hypervisor: We have
sets of internal problems that obviously don't show up in the
architecture, and we can either handle them internally or use what the
architecture offers for problem signaling. z/VM has probably faced the
same problems :)

>  };
>  
>  return ret;
> @@ -1115,7 +1112,7 @@ static int do_subchannel_work(SubchDev *sch)
>  if (sch->do_subchannel_work) {
>  return sch->do_subchannel_work(sch);
>  } else {
> -return -EINVAL;
> +return -ENODEV;

This rather seems like a job for an assert? If we don't have a function
for the 'asynchronous' handling of the various functions assigned for a
subchannel, that looks like an internal error.

>  }
>  }
>  
> diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c
> index 8614dda6f8..2b0741741c 100644
> --- a/hw/s390x/s390-ccw.c
> +++ b/hw/s390x/s390-ccw.c
> @@ -25,7 +25,7 @@ int s390_ccw_cmd_request(ORB *orb, SCSW *scsw, void *data)
>  if (cdc->handle_request) {
>  return cdc->handle_request(orb, scsw, data);
>  } else {
> -return -ENOSYS;
> +return -ENODEV;

If we get here, it means that we called a request handler (which is
only done for the passthrough variety) without having assigned a
request handler beforehand. This also looks like an internal error to
me...

>  }
>  }
>  

Re: [Qemu-devel] [PATCH v1 05/11] s390x: rename s390-virtio.h to s390-virtio-hcall.h

[Thomas Huth]

On 30.08.2017 19:05, David Hildenbrand wrote:
> The only interface left, so let's properly rename it.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  hw/s390x/s390-virtio-ccw.c  | 2 +-
>  hw/s390x/s390-virtio-hcall.c| 2 +-
>  hw/s390x/{s390-virtio.h => s390-virtio-hcall.h} | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>  rename hw/s390x/{s390-virtio.h => s390-virtio-hcall.h} (92%)
[...]
> diff --git a/hw/s390x/s390-virtio.h b/hw/s390x/s390-virtio-hcall.h
> similarity index 92%
> rename from hw/s390x/s390-virtio.h
> rename to hw/s390x/s390-virtio-hcall.h
> index d984cd4115..64c5bbd827 100644
> --- a/hw/s390x/s390-virtio.h
> +++ b/hw/s390x/s390-virtio-hcall.h
> @@ -1,5 +1,5 @@
>  /*
> - * Virtio interfaces for s390
> + * Support for virtio hypercalls on s390x
>   *
>   * Copyright 2012 IBM Corp.
>   * Author(s): Cornelia Huck 
> 

Maybe also rename the HW_S390_VIRTIO_H header guard? Anyway:

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH v2 00/13] add support for Hypervisor.framework in QEMU

[Stefan Hajnoczi]

On Wed, Aug 30, 2017 at 03:26:49AM -0500, Sergio Andres Gomez Del Real wrote:
> 
> Changes in v2:
>  (1) Removed legacy option "-enable-hvf" in favor of "-M accel=hvf"
>  (2) Added missing copyright headers; replace fprintfs for error_report;
>  improved commit description.
>  (3) Moved patch that adds compilation rules in Makefile.objs right after
>  the patch that adds the new files from Google's repo.
>  (4) Removed conditional macros from cpus.c and cpu.c
>  (5) Moved patch that fixes coding style to patch # 3
>  (6) Fix commit message in apic patch
>  (7) Squash some commits to avoid code churn
> 
> 
> The following patchset adds to QEMU the supporting for macOS's native
> hypervisor, Hypervisor.framework (hvf). The code base is taken from
> Google's Android emulator at
> https://android.googlesource.com/platform/external/qemu/+/emu-master-dev.
> 
> Apart from general code refactoring, some additional features were 
> implemented:
> retrieve the set of features supported by host cpu and hvf (cpuid);
> dirty page tracking for VGA memory area; reimplementation of the event
> injection mechanism to allow injection of exceptions during vmexits, which is
> exemplified by the injection of a GP fault when the guest vmexits due to
> execution of the vmcall instruction; changing the emulator's use of CPUState
> structure in favor of CPUX86State, so as to in the future remove data 
> structures
> that are uselessly specific to hvf and unified some of the state between 
> kvm/tcg
> and hvf.
> Some features initially planned to implement that didn't make it include:
> page fault handling in the emulator and implementing the dummy_signal to 
> handle
> the SIG_IPI signal without race conditions. Hopefully these can be implemented
> in the near future.

I have done a brief review (mainly style issues) of the whole series.

A test case is required.  Maybe the easiest option is to extend
tests/boot-serial-test.c to try hvf (if available).  That way an
automated test will verify that the BIOS executes inside the guest.

Stefan

Re: [Qemu-devel] [PATCH 3/9] s390x/css: be more consistent if broken beyond repair

[Cornelia Huck]

On Wed, 30 Aug 2017 18:36:03 +0200
Halil Pasic  wrote:

> If we detect that the internally manged state of the subchannel
> is broken beyond repair while in do_subchannel_work in case of
> virtual we just abort the operation and pretend all went well,
> while in case of pass-through we honor the situation with -ENODEV
> which results in cc 3 for the instruction whose handler triggered
> the call.
> 
> Let's be consistent on this and do the -ENODEV also for virtual
> subchannels.
> 
> Signed-off-by: Halil Pasic 
> Acked-by: Pierre Morel
> ---
>  hw/s390x/css.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index 0822538cde..bc47bf5b20 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -1078,7 +1078,7 @@ int do_subchannel_work_virtual(SubchDev *sch)
>  sch_handle_start_func_virtual(sch);
>  } else {
>  /* Cannot happen. */
> -return 0;
> +return -ENODEV;

No, this _really_ cannot happen. fctl is a three-bit field, which means
that one of the branches above must have executed. fctl cannot be 0, as
any caller of do_subchannel_work() either sets a bit there or, in the
case of rsch, checks for a bit already set. This is an internal error,
so an assert seems more suitable here. [We might need to keep the
return to keep mingw happy.]

>  }
>  css_inject_io_interrupt(sch);
>  return 0;

Re: [Qemu-devel] [PATCH 1/7] serial: add serial_chr_nonnull() to use the null backend when none provided

[Marc-André Lureau]

Hi

On Thu, Aug 31, 2017 at 7:20 AM Thomas Huth  wrote:

> On 31.08.2017 05:53, Philippe Mathieu-Daudé wrote:
> > Suggested-by: Peter Maydell 
> > Signed-off-by: Philippe Mathieu-Daudé 
> > ---
> >  include/hw/char/serial.h |  1 +
> >  hw/char/serial.c | 13 +
> >  2 files changed, 14 insertions(+)
> >
> > diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
> > index c4daf11a14..96bccb39e1 100644
> > --- a/include/hw/char/serial.h
> > +++ b/include/hw/char/serial.h
> > @@ -93,6 +93,7 @@ SerialState *serial_mm_init(MemoryRegion
> *address_space,
> >  hwaddr base, int it_shift,
> >  qemu_irq irq, int baudbase,
> >  Chardev *chr, enum device_endian end);
> > +Chardev *serial_chr_nonnull(Chardev *chr);
>
> Why do you need the prototype? Please make the function static if
> possible (otherwise please add some rationale in the patch description).
>

It's being used from other units in following patches


> >  /* serial-isa.c */
> >  #define TYPE_ISA_SERIAL "isa-serial"
> > diff --git a/hw/char/serial.c b/hw/char/serial.c
> > index 9aec6c60d8..7a100db107 100644
> > --- a/hw/char/serial.c
> > +++ b/hw/char/serial.c
> > @@ -1025,6 +1025,19 @@ static const MemoryRegionOps serial_mm_ops[3] = {
> >  },
> >  };
> >
> > +Chardev *serial_chr_nonnull(Chardev *chr)
> > +{
> > +static int serial_id;
> > +char *label;
> > +
> > +label = g_strdup_printf("discarding-serial%d", serial_id++);
> > +chr = qemu_chr_new(label, "null");
>
> That looks wrong - you're ignoring the input parameter and always open
> the "null" device? Shouldn't there be a "if (chr) return chr;" in front
> of this?
>

I think its missing too.

The function name and usage isn't obvious. I would rather see the caller
use "chr ?: serial_chr_null()" rather than "serial_chr_nonnull(chr)".


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v1 11/11] target/s390x: use program_interrupt() in per_check_exception()

[Thomas Huth]

On 30.08.2017 19:06, David Hildenbrand wrote:
> I am not sure if we are handling ilen the right way here. ilen should
> always match the instruction triggering the exception. This is relevant
> for per exceptions triggered via EXECUTE instructions. The ilen to be
> indicated has to match the EXECUTE instruction.
> 
> Clean it up for now but leave ilen as is, we can fix that later.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/misc_helper.c | 9 +++--
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
> index eb7accc0ce..ac9657f23f 100644
> --- a/target/s390x/misc_helper.c
> +++ b/target/s390x/misc_helper.c
> @@ -445,14 +445,11 @@ void HELPER(chsc)(CPUS390XState *env, uint64_t inst)
>  #ifndef CONFIG_USER_ONLY
>  void HELPER(per_check_exception)(CPUS390XState *env)
>  {
> -CPUState *cs = CPU(s390_env_get_cpu(env));
> +uint32_t ilen;
>  
>  if (env->per_perc_atmid) {
> -env->int_pgm_code = PGM_PER;
> -env->int_pgm_ilen = get_ilen(cpu_ldub_code(env, env->per_address));
> -
> -cs->exception_index = EXCP_PGM;
> -cpu_loop_exit(cs);
> +ilen = get_ilen(cpu_ldub_code(env, env->per_address));
> +program_interrupt(env, PGM_PER, ilen);
>  }
>  }

The changes basically look fine to me, but may I suggest to
1) Add a comment to the code about your concerns with ilen
2) Change the patch description to focus on the work that is actually
done here instead of only talking about your ilen concerns?

 Thanks,
  Thomas

Re: [Qemu-devel] [PATCH 1/7] serial: add serial_chr_nonnull() to use the null backend when none provided

[Thomas Huth]

On 31.08.2017 11:36, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Aug 31, 2017 at 7:20 AM Thomas Huth  > wrote:
> 
> On 31.08.2017 05:53, Philippe Mathieu-Daudé wrote:
> > Suggested-by: Peter Maydell  >
> > Signed-off-by: Philippe Mathieu-Daudé  >
> > ---
> >  include/hw/char/serial.h |  1 +
> >  hw/char/serial.c         | 13 +
> >  2 files changed, 14 insertions(+)
> >
> > diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
> > index c4daf11a14..96bccb39e1 100644
> > --- a/include/hw/char/serial.h
> > +++ b/include/hw/char/serial.h
> > @@ -93,6 +93,7 @@ SerialState *serial_mm_init(MemoryRegion
> *address_space,
> >                              hwaddr base, int it_shift,
> >                              qemu_irq irq, int baudbase,
> >                              Chardev *chr, enum device_endian end);
> > +Chardev *serial_chr_nonnull(Chardev *chr);
> 
> Why do you need the prototype? Please make the function static if
> possible (otherwise please add some rationale in the patch description).
> 
> It's being used from other units in following patches

Ah, well, right. I was only on CC: in the first two patches, so I missed
the other ones at the first glance. So never mind my comment, the
prototype is fine here.

 Thomas

[Qemu-devel] [PATCH v2] io: add new qio_channel_{readv, writev, read, write}_all functions

[Daniel P. Berrange]

These functions wait until they are able to read / write the full
requested data buffer(s).

Signed-off-by: Daniel P. Berrange 
---

Changed in v2:

 - Remove 'coroutine_fn' annotation (Stefan)
 - Fix docs typos (Eric)
 - Remove bogus error overwriting (Eric)

 include/io/channel.h   |  90 +++
 io/channel.c   |  94 +
 tests/io-channel-helpers.c | 102 -
 3 files changed, 193 insertions(+), 93 deletions(-)

diff --git a/include/io/channel.h b/include/io/channel.h
index 54f3dc252f..8f25893c45 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -269,6 +269,58 @@ ssize_t qio_channel_writev_full(QIOChannel *ioc,
 Error **errp);
 
 /**
+ * qio_channel_readv_all:
+ * @ioc: the channel object
+ * @iov: the array of memory regions to read data into
+ * @niov: the length of the @iov array
+ * @errp: pointer to a NULL-initialized error object
+ *
+ * Read data from the IO channel, storing it in the
+ * memory regions referenced by @iov. Each element
+ * in the @iov will be fully populated with data
+ * before the next one is used. The @niov parameter
+ * specifies the total number of elements in @iov.
+ *
+ * The function will wait for all requested data
+ * to be read, yielding from the current coroutine
+ * if required.
+ *
+ * If end-of-file occurs before all requested data
+ * has been read, an error will be reported.
+ *
+ * Returns: 0 if all bytes were read, or -1 on error
+ */
+int qio_channel_readv_all(QIOChannel *ioc,
+  const struct iovec *iov,
+  size_t niov,
+  Error **errp);
+
+
+/**
+ * qio_channel_writev_all:
+ * @ioc: the channel object
+ * @iov: the array of memory regions to write data from
+ * @niov: the length of the @iov array
+ * @errp: pointer to a NULL-initialized error object
+ *
+ * Write data to the IO channel, reading it from the
+ * memory regions referenced by @iov. Each element
+ * in the @iov will be fully sent, before the next
+ * one is used. The @niov parameter specifies the
+ * total number of elements in @iov.
+ *
+ * The function will wait for all requested data
+ * to be written, yielding from the current coroutine
+ * if required.
+ *
+ * Returns: 0 if all bytes were written, or -1 on error
+ */
+int qio_channel_writev_all(QIOChannel *ioc,
+   const struct iovec *iov,
+   size_t niov,
+   Error **erp);
+
+/**
  * qio_channel_readv:
  * @ioc: the channel object
  * @iov: the array of memory regions to read data into
@@ -331,6 +383,44 @@ ssize_t qio_channel_write(QIOChannel *ioc,
   Error **errp);
 
 /**
+ * qio_channel_read_all:
+ * @ioc: the channel object
+ * @buf: the memory region to read data into
+ * @buflen: the number of bytes to @buf
+ * @errp: pointer to a NULL-initialized error object
+ *
+ * Reads @buflen bytes into @buf, possibly blocking or (if the
+ * channel is non-blocking) yielding from the current coroutine
+ * multiple times until the entire content is read. If end-of-file
+ * occurs it will return an error rather than a short-read. Otherwise
+ * behaves as qio_channel_read().
+ *
+ * Returns: 0 if all bytes were read, or -1 on error
+ */
+int qio_channel_read_all(QIOChannel *ioc,
+ char *buf,
+ size_t buflen,
+ Error **errp);
+/**
+ * qio_channel_write_all:
+ * @ioc: the channel object
+ * @buf: the memory region to write data into
+ * @buflen: the number of bytes to @buf
+ * @errp: pointer to a NULL-initialized error object
+ *
+ * Writes @buflen bytes from @buf, possibly blocking or (if the
+ * channel is non-blocking) yielding from the current coroutine
+ * multiple times until the entire content is written.  Otherwise
+ * behaves as qio_channel_write().
+ *
+ * Returns: 0 if all bytes were written, or -1 on error
+ */
+int qio_channel_write_all(QIOChannel *ioc,
+  const char *buf,
+  size_t buflen,
+  Error **errp);
+
+/**
  * qio_channel_set_blocking:
  * @ioc: the channel object
  * @enabled: the blocking flag state
diff --git a/io/channel.c b/io/channel.c
index 1cfb8b33a2..5e8c2f0a91 100644
--- a/io/channel.c
+++ b/io/channel.c
@@ -22,6 +22,7 @@
 #include "io/channel.h"
 #include "qapi/error.h"
 #include "qemu/main-loop.h"
+#include "qemu/iov.h"
 
 bool qio_channel_has_feature(QIOChannel *ioc,
  QIOChannelFeature feature)
@@ -85,6 +86,79 @@ ssize_t qio_channel_writev_full(QIOChannel *ioc,
 }
 
 
+
+int qio_channel_readv_all(QIOChannel *ioc,
+  const struct iovec *iov,
+  size_t niov,
+  Error **errp)
+{
+int ret = -1;
+struct iovec *local_iov = g_new(struct iovec,

Re: [Qemu-devel] [PATCH v7 07/11] qemu.py: include debug information on launch error

[Lukáš Doktor]

Dne 18.8.2017 v 19:05 Amador Pahim napsal(a):
> When launching a VM, if an exception happens and the VM is not
> initiated, it might be useful to see the qemu command line and
> the qemu command output.
> 
> This patch creates that message. Notice that self._iolog needs to be
> cleaned up in the beginning of the launch() to make sure we will not
> expose the qemu log from a previous launch if the current one fails.
> 
> Signed-off-by: Amador Pahim 
> ---
>  scripts/qemu.py | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index 0bcec4b3b1..29fd2469f9 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -147,6 +147,7 @@ class QEMUMachine(object):
>  
>  def launch(self):
>  '''Launch the VM and establish a QMP connection'''
> +self._iolog = None
>  self._qemu_full_args = None
>  devnull = open(os.path.devnull, 'rb')
>  qemulog = open(self._qemu_log_path, 'wb')
> @@ -162,6 +163,13 @@ class QEMUMachine(object):
>  self._post_launch()
>  except:
>  self.shutdown()
> +
> +LOG.debug('Error launching VM')
> +if self._qemu_full_args:
> +LOG.debug('Command: %r', ' '.join(self._qemu_full_args))
> +if self._iolog:
> +LOG.debug('Output: %r', self._iolog)
> +
>  raise
>  

I don't want to open the cave, but anybody else would actually prefer custom 
Exception with the qemu_full_args, iolog and details about the original 
exception to this? Users who don't care would see/log all the details from the 
`exception.__str__` and those who are interested in the original exception 
would query for it eg. via `exception.original_exception`.

Anyway even this change is IMO step into the right direction and we can (and 
should) refine the use of exceptions in following patches (as Markus spotted, 
pure Exceptions and other ugliness lives in those waters).

As for the debug vs. error I'd actually stick to "debug", because in negative 
testing the logged message is only an expected debug and in positive testing it 
expands the knowledge of the flow, but the actual error message should come 
from the exception. Not a strong opinion, though...

Regards,
Lukáš

>  def shutdown(self):
> 




signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] io: fix typo in docs comment for qio_channel_read

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange 
---
 include/io/channel.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/io/channel.h b/include/io/channel.h
index db9bb022a1..54f3dc252f 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -299,7 +299,7 @@ ssize_t qio_channel_writev(QIOChannel *ioc,
Error **errp);
 
 /**
- * qio_channel_readv:
+ * qio_channel_read:
  * @ioc: the channel object
  * @buf: the memory region to read data into
  * @buflen: the length of @buf
-- 
2.13.5

Re: [Qemu-devel] [PATCH 4/9] s390x: refactor error handling for SSCH and RSCH

[Cornelia Huck]

On Wed, 30 Aug 2017 18:36:04 +0200
Halil Pasic  wrote:

> Simplify the error handling of the SSCH and RSCH handler avoiding
> arbitrary and cryptic error codes being mapped to what a subchannel is
> supposed to do.  Let the code detecting the condition tell how it's to be
> handled in a less ambiguous way.  It's best to handle SSCH and RSCH in
> one go as the emulation of the two shares a lot of code.

So determining the return code at the point in time where we can
instead of trying to map to return codes and back again makes quite a
bit of sense, but I'll have to look at the rest of this. For one thing,
would a better split to introduce the cc-reporting infrastructure first
and then convert the different I/O functions?

> 
> Signed-off-by: Halil Pasic 
> Acked-by: Pierre Morel
> 
> ---
> Notes:
> Funny, we had a different swich-case for SSCH and RSCH. For
> virtual it did not matter, but for passtrough it could. In practice
> -EINVAL from the kernel would have been mapped to cc 2 in case of
> RSCH and to cc 1 in case of SSHC which is absurd. Same goes for
> -EBUSY from kernel which is correctly mapped to cc 2 in case of
> SSCH, but for RSCH it gets mapped to cc 1 which is also absurd.
> ---
>  hw/s390x/css.c  | 86 
> ++---
>  hw/s390x/s390-ccw.c |  8 ++---
>  hw/vfio/ccw.c   | 32 +
>  include/hw/s390x/css.h  | 30 
>  include/hw/s390x/s390-ccw.h |  2 +-
>  target/s390x/ioinst.c   | 61 +---
>  6 files changed, 97 insertions(+), 122 deletions(-)
> 
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index bc47bf5b20..1102642c10 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -1015,12 +1015,11 @@ static void sch_handle_start_func_virtual(SubchDev 
> *sch)
>  
>  }
>  
> -static int sch_handle_start_func_passthrough(SubchDev *sch)
> +static void sch_handle_start_func_passthrough(SubchDev *sch)
>  {
>  
>  PMCW *p = &sch->curr_status.pmcw;
>  SCSW *s = &sch->curr_status.scsw;
> -int ret;
>  
>  ORB *orb = &sch->orb;
>  if (!(s->ctrl & SCSW_ACTL_SUSP)) {
> @@ -1034,28 +1033,10 @@ static int sch_handle_start_func_passthrough(SubchDev 
> *sch)
>   */
>  if (!(orb->ctrl0 & ORB_CTRL0_MASK_PFCH) ||
>  !(orb->ctrl0 & ORB_CTRL0_MASK_C64)) {
> -return -ENODEV;
> +sch->iret.cc = 3;

Same as already commented: I don't think cc 3 is a good match.

>  }
>  
> -ret = s390_ccw_cmd_request(orb, s, sch->driver_data);
> -switch (ret) {
> -/* Currently we don't update control block and just return the cc code. 
> */
> -case 0:
> -break;
> -case -EBUSY:
> -break;
> -case -ENODEV:
> -break;
> -case -EFAULT:
> - break;
> -case -EACCES:
> -/* Let's reflect an inaccessible host device by cc 3. */
> -default:
> -/* Let's make all other return codes map to cc 3.  */
> -ret = -ENODEV;
> -};
> -
> -return ret;
> +s390_ccw_cmd_request(sch);

As you change the handling anyway: Don't change this logic in prior
patches?

>  }
>  
>  /*
> @@ -1064,7 +1045,7 @@ static int sch_handle_start_func_passthrough(SubchDev 
> *sch)
>   * read/writes) asynchronous later on if we start supporting more than
>   * our current very simple devices.
>   */
> -int do_subchannel_work_virtual(SubchDev *sch)
> +void do_subchannel_work_virtual(SubchDev *sch)
>  {
>  
>  SCSW *s = &sch->curr_status.scsw;
> @@ -1078,41 +1059,35 @@ int do_subchannel_work_virtual(SubchDev *sch)
>  sch_handle_start_func_virtual(sch);
>  } else {
>  /* Cannot happen. */
> -return -ENODEV;
> +sch->iret.cc = 3;

See comment for the last patch.

>  }
>  css_inject_io_interrupt(sch);
> -return 0;
>  }
>  
> -int do_subchannel_work_passthrough(SubchDev *sch)
> +void do_subchannel_work_passthrough(SubchDev *sch)
>  {
> -int ret;
>  SCSW *s = &sch->curr_status.scsw;
>  
>  if (s->ctrl & SCSW_FCTL_CLEAR_FUNC) {
>  /* TODO: Clear handling */
>  sch_handle_clear_func(sch);
> -ret = 0;
>  } else if (s->ctrl & SCSW_FCTL_HALT_FUNC) {
>  /* TODO: Halt handling */
>  sch_handle_halt_func(sch);
> -ret = 0;
>  } else if (s->ctrl & SCSW_FCTL_START_FUNC) {
> -ret = sch_handle_start_func_passthrough(sch);
> +sch_handle_start_func_passthrough(sch);
>  } else {
>  /* Cannot happen. */
> -return -ENODEV;
> +sch->iret.cc = 3;

ftcl == 0 should have been rejected already by the function handlers
here as well, shouldn't it?

>  }
> -
> -return ret;
>  }
>  
> -static int do_subchannel_work(SubchDev *sch)
> +static void do_subchannel_work(SubchDev *sch)
>  {
>  if (sch->do_subchannel_work) {
> -return sch->do_subchannel_work(sch);
> +sch->do_subchannel_work(sch);
>  } else {
> -return -ENODEV;
> +sch->ir

Re: [Qemu-devel] [PATCH 0/9]

[Cornelia Huck]

On Wed, 30 Aug 2017 18:36:00 +0200
Halil Pasic  wrote:

> This series has a character of a refactoring, as the initial motivation
> was improving readability and reducing complexity.

But you reduced the cover letter subject too much ;)

> 
> Despite of the original intent the tree first patches buxfixes, and
> according to Dong Jia the patch-set also has a functional value: for ccw
> pass-through, he is planing to pass-through the instruction completion
> information (cc or interruption condition) from the kernel, and this
> patch set can pretty much be seen as a preparation for that.

I think the first one (with the precedence fixed as well) is pretty
uncontroversial, and I'd be happy to queue an updated version.

> 
> The basic idea is: tell how to handle an unusual conditon where it's
> identified, instead of mapping it to an errno (more or less arbitrarily),
> then possibly mapping these errnos around, to finally (mentally) map the
> errno back to the condition and take appropriate action. 
> 
> At the end of the series we also have 125 lines of code less to maintain,
> and the executable got a bit smaller too.

I agree with the basic direction, but I think this needs more hashing
out.

> 
> Halil Pasic (9):
>   s390x/css: fix cc handling for XSCH
>   s390x: fix invalid use of cc 1 for SSCH
>   s390x/css: be more consistent if broken beyond repair
>   s390x: refactor error handling for SSCH and RSCH
>   s390x: refactor error handling for XSCH handler
>   s390x: refactor error handling for CSCH handler
>   s390x: refactor error handling for HSCH handler
>   s390x: refactor error handling for MSCH handler
>   s390x: factor out common ioinst handler logic
> 
>  hw/s390x/css.c  | 158 --
>  hw/s390x/s390-ccw.c |   8 +-
>  hw/vfio/ccw.c   |  32 ++--
>  include/hw/s390x/css.h  |  38 +++---
>  include/hw/s390x/s390-ccw.h |   2 +-
>  target/s390x/ioinst.c   | 181 
> ++--
>  6 files changed, 147 insertions(+), 272 deletions(-)
> 

Re: [Qemu-devel] [PULL 0/1] slirp updates

[Peter Maydell]

On 28 August 2017 at 00:05, Samuel Thibault
 wrote:
> warning: redirection vers https://people.debian.org/~sthibault/qemu.git/
> The following changes since commit 04d74e07b4542aad5aa4ad03951b38b767f5314a:
>
>   slirp: fix clearing ifq_so from pending packets (2017-08-26 01:04:12 +0200)
>
> are available in the git repository at:
>
>   http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
>
> for you to fetch changes up to 04d74e07b4542aad5aa4ad03951b38b767f5314a:
>
>   slirp: fix clearing ifq_so from pending packets (2017-08-26 01:04:12 +0200)
>
> 
> slirp updates
>
> 

Applied to master, thanks (I cherry-picked the patch so I
could add the cc:stable note).

-- PMM

Re: [Qemu-devel] [PATCH 1/9] s390x/css: fix cc handling for XSCH

[Halil Pasic]

On 08/31/2017 10:42 AM, Cornelia Huck wrote:
> On Thu, 31 Aug 2017 09:32:49 +0200
> Thomas Huth  wrote:
> 
>> On 31.08.2017 08:38, Cornelia Huck wrote:
>>> On Thu, 31 Aug 2017 07:51:17 +0200
>>> Thomas Huth  wrote:
>>>   
 On 30.08.2017 18:36, Halil Pasic wrote:  
> The function ioinst_handle_xsch is presenting cc 2 when it's supposed to
> present cc 1 and the other way around, because css_do_xsch has the error
> codes mixed up. Fixing the error codes also fixes the priority.
>
> Let us fix this.

 (Nit: In case you respin, I'd suggest to remove the last sentence. You
 already used "fix" two times in the previous one)  
>>>
>>> I can also remove it on applying, if Halil agrees (I have not yet
>>> reviewed it, though).
>>>   
  
> Signed-off-by: Halil Pasic 
> Reported-by: Pierre Morel

 Space missing -^  
>>>
>>> And I can also add that space on applying :)
>>>   
  
> ---
>  hw/s390x/css.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index 1880b1a0ff..a50fb0727e 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -1281,12 +1281,12 @@ int css_do_xsch(SubchDev *sch)
>  (!(s->ctrl &
> (SCSW_ACTL_RESUME_PEND | SCSW_ACTL_START_PEND | 
> SCSW_ACTL_SUSP))) ||
>  (s->ctrl & SCSW_ACTL_SUBCH_ACTIVE)) {
> -ret = -EINPROGRESS;
> +ret = -EBUSY;
>  goto out;
>  }
>  
>  if (s->ctrl & SCSW_CTRL_MASK_STCTL) {
> -ret = -EBUSY;
> +ret = -EINPROGRESS;
>  goto out;
>  }

 Using both, EBUSY and EINPROGRESS as error codes sounds very confusing
 to me here ... what's the difference between busy and in-progress? So
 while you're at it, maybe you could replace the code for CC 2 ("CANCEL
 SUBCHANNEL not applicable") with a different error code?  
>>>
>>> IIRC, I used these two as they matched my idea of what happens best
>>> (there's a difference between "subchannel is busy" and "the I/O is
>>> already in progress, too late to cancel"). "xsch not applicable" is
>>> very hard to translate to an Unix error code :/  
>>
>> OK, the codes make more sense with your description ==> Maybe simply add
>> a proper comment for each of the return codes?
> 
> Taking a step back and looking at the other I/O instructions and their
> implementation in qemu:
> 
> - For those instructions that can set it, cc 1 is set when the
>   subchannel is status pending. That's usually the "default" branch in
>   ioinst.c.
> - cc 2 is set when the subchannel is "busy" (or, in the case of xsch,
>   "not applicable for xsch"... sigh) This is supposed to be handled via
>   -EBUSY.
> 
> So, there are actually two problems with the current implementation of
> xsch:
> 
> - The return codes are switched around, which this patch fixes.
> - But "status pending" should also take precedence over "not
>   applicable", if I read the PoP correctly, so the second if needs to
>   be moved up.

You are right and I was wrong. "Condition code 1 has precedence over
condition code 2." So it's 3 > 1 > 2 (and I remembered 3 > 2 > 1).

I will fix this for v2.

> 
> And yes, it makes sense do add some comments...
> 

If we apply the series as a whole adding comments would an overkill
IMHO. We will switch this to iret.cc = ? so it should become obvious.

Re: [Qemu-devel] [PATCH 00/79] Patch Round-up for stable 2.9.1, freeze on 2017-09-04

[Peter Maydell]

On 29 August 2017 at 01:13, Michael Roth  wrote:
> Hi everyone,
>
> The following new patches are queued for QEMU stable v2.9.1:
>
>   https://github.com/mdroth/qemu/commits/stable-2.9-staging
>
> The release is planned for 2017-09-07:
>
>   http://wiki.qemu.org/Planning/2.9
>
> Please respond here or CC qemu-sta...@nongnu.org on any patches you
> think should be included in the release.

I would suggest also commit 1201d308519f1e915866d7583d5136d03cc1d384
("slirp: fix clearing ifq_so from pending packets") which I've
just applied to master, as it fixes a use-after-free if the
guest sends suitable bogus packets and the VM is using slirp
networking.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] scripts: Support building with Python 3

[Peter Maydell]

On 31 August 2017 at 07:35, Markus Armbruster  wrote:
> So, first we'll invest in work-arounds to make both 2 and 3 work.  Once
> 2 is gone, we can invest some more to clean them up.  Which probably
> won't happen, so we'll continue to carry work-arounds that no longer
> make sense.
>
> I maintain roughly one fourth of all Python code in qemu, and I'm not
> looking forward to this hoop-jumping at all.
>
> Are we really, really sure we want to go this way?  What exactly are we
> hoping to accomplish by it?

My take is that we have the following goals we want to achieve:

(1) We need to continue to build and run on older (long-term-support)
distros that still ship only Python 2.x (alas even back to 2.6)
(2) We need to be able to build and run on newer distros that
have dropped Python 2 altogether in favour of Python 3
(I don't know if there are any such today, but presumably by
2020 there will be)

Unless we can confidently say that either (1) or (2) is the
empty set, we need to handle both 2 and 3 in the same codebase.
This is a pain, but unfortunately Python upstream have forced
us into it by breaking source code compatibility.

I think (1) is pretty clearly not (yet) an empty set, so the
only alternative I see to "support 2 and 3 now" is "keep supporting
only 2 for the moment and hope that no distro drops 2 support
before all the LTS 2-only distro versions vanish into history".

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/7] serial: add serial_chr_nonnull() to use the null backend when none provided

[Peter Maydell]

On 31 August 2017 at 04:53, Philippe Mathieu-Daudé  wrote:
> Suggested-by: Peter Maydell 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  include/hw/char/serial.h |  1 +
>  hw/char/serial.c | 13 +
>  2 files changed, 14 insertions(+)
>
> diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
> index c4daf11a14..96bccb39e1 100644
> --- a/include/hw/char/serial.h
> +++ b/include/hw/char/serial.h
> @@ -93,6 +93,7 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
>  hwaddr base, int it_shift,
>  qemu_irq irq, int baudbase,
>  Chardev *chr, enum device_endian end);
> +Chardev *serial_chr_nonnull(Chardev *chr);

For new globally-visible function declarations, can we
have a doc-comment form comment that documents the
function, please?

thanks
-- PMM

[Qemu-devel] [PULL 00/29] Code cleanup patches

[Marc-André Lureau]

The following changes since commit 1201d308519f1e915866d7583d5136d03cc1d384:

  slirp: fix clearing ifq_so from pending packets (2017-08-30 23:14:34 +0100)

are available in the git repository at:

  https://github.com/elmarco/qemu.git tags/tidy-pull-request

for you to fetch changes up to e4d67e4f2e06128bc7f07263afe9acc2e2242fdc:

  eepro100: replace g_malloc()+memcpy() with g_memdup() (2017-08-31 12:29:07 
+0200)





Marc-André Lureau (29):
  i386: use ROUND_UP macro
  vnc: use QEMU_ALIGN_DOWN
  vhdx: use QEMU_ALIGN_DOWN
  vhost: use QEMU_ALIGN_DOWN
  i8254: use QEMU_ALIGN_DOWN
  pcspk: use QEMU_ALIGN_DOWN
  dmg: use DIV_ROUND_UP
  qcow2: use DIV_ROUND_UP
  vpc: use DIV_ROUND_UP
  vvfat: use DIV_ROUND_UP
  vnc: use DIV_ROUND_UP
  ui: use DIV_ROUND_UP
  vga: use DIV_ROUND_UP
  virtio-gpu: use DIV_ROUND_UP
  monitor: use DIV_ROUND_UP
  console: use DIV_ROUND_UP
  virtio-serial: use DIV_ROUND_UP
  piix: use DIV_ROUND_UP
  q35: use DIV_ROUND_UP
  usb-hub: use DIV_ROUND_UP
  msix: use DIV_ROUND_UP
  ppc: use DIV_ROUND_UP
  i386/dump: use DIV_ROUND_UP
  kvm: use DIV_ROUND_UP
  decnumber: use DIV_ROUND_UP
  i386: introduce ELF_NOTE_SIZE macro
  i386: replace g_malloc()+memcpy() with g_memdup()
  test-iov: replace g_malloc()+memcpy() with g_memdup()
  eepro100: replace g_malloc()+memcpy() with g_memdup()

 include/ui/console.h|  2 +-
 linux-headers/asm-x86/kvm.h |  2 +-
 block/dmg.c |  2 +-
 block/qcow2-cluster.c   |  2 +-
 block/vhdx-log.c|  2 +-
 block/vpc.c |  4 ++--
 block/vvfat.c   |  4 ++--
 hw/audio/pcspk.c|  2 +-
 hw/char/virtio-serial-bus.c |  8 
 hw/display/vga.c|  2 +-
 hw/display/virtio-gpu.c |  4 ++--
 hw/i386/multiboot.c |  3 +--
 hw/net/eepro100.c   |  3 +--
 hw/pci-host/piix.c  |  2 +-
 hw/pci-host/q35.c   |  2 +-
 hw/pci/msix.c   |  4 ++--
 hw/timer/i8254_common.c |  4 ++--
 hw/usb/dev-hub.c|  8 
 hw/virtio/vhost.c   |  2 +-
 libdecnumber/decNumber.c|  2 +-
 monitor.c   |  4 ++--
 target/i386/arch_dump.c | 40 
 target/ppc/mem_helper.c |  2 +-
 target/ppc/translate.c  |  2 +-
 tests/test-iov.c|  3 +--
 ui/cursor.c |  2 +-
 ui/vnc-enc-tight.c  |  2 +-
 ui/vnc.c| 10 +-
 28 files changed, 63 insertions(+), 66 deletions(-)

-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 01/29] i386: use ROUND_UP macro

[Marc-André Lureau]

I used the clang-tidy qemu-round check (with the option OnlyAlignUp)
to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Juan Quintela 
Acked-by: Eduardo Habkost 
Reviewed-by: Richard Henderson 
---
 target/i386/arch_dump.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/target/i386/arch_dump.c b/target/i386/arch_dump.c
index fe0aa36932..1d51bb5206 100644
--- a/target/i386/arch_dump.c
+++ b/target/i386/arch_dump.c
@@ -84,9 +84,9 @@ static int x86_64_write_elf64_note(WriteCoreDumpFunction f,
 note->n_descsz = cpu_to_le32(descsz);
 note->n_type = cpu_to_le32(NT_PRSTATUS);
 buf = (char *)note;
-buf += ((sizeof(Elf64_Nhdr) + 3) / 4) * 4;
+buf += ROUND_UP(sizeof(Elf64_Nhdr), 4);
 memcpy(buf, name, name_size);
-buf += ((name_size + 3) / 4) * 4;
+buf += ROUND_UP(name_size, 4);
 memcpy(buf + 32, &id, 4); /* pr_pid */
 buf += descsz - sizeof(x86_64_user_regs_struct)-sizeof(target_ulong);
 memcpy(buf, ®s, sizeof(x86_64_user_regs_struct));
@@ -163,9 +163,9 @@ static int x86_write_elf64_note(WriteCoreDumpFunction f, 
CPUX86State *env,
 note->n_descsz = cpu_to_le32(descsz);
 note->n_type = cpu_to_le32(NT_PRSTATUS);
 buf = (char *)note;
-buf += ((sizeof(Elf64_Nhdr) + 3) / 4) * 4;
+buf += ROUND_UP(sizeof(Elf64_Nhdr), 4);
 memcpy(buf, name, name_size);
-buf += ((name_size + 3) / 4) * 4;
+buf += ROUND_UP(name_size, 4);
 memcpy(buf, &prstatus, sizeof(prstatus));
 
 ret = f(note, note_size, opaque);
@@ -218,9 +218,9 @@ int x86_cpu_write_elf32_note(WriteCoreDumpFunction f, 
CPUState *cs,
 note->n_descsz = cpu_to_le32(descsz);
 note->n_type = cpu_to_le32(NT_PRSTATUS);
 buf = (char *)note;
-buf += ((sizeof(Elf32_Nhdr) + 3) / 4) * 4;
+buf += ROUND_UP(sizeof(Elf32_Nhdr), 4);
 memcpy(buf, name, name_size);
-buf += ((name_size + 3) / 4) * 4;
+buf += ROUND_UP(name_size, 4);
 memcpy(buf, &prstatus, sizeof(prstatus));
 
 ret = f(note, note_size, opaque);
@@ -353,9 +353,9 @@ static inline int cpu_write_qemu_note(WriteCoreDumpFunction 
f,
 note64->n_type = 0;
 }
 buf = note;
-buf += ((note_head_size + 3) / 4) * 4;
+buf += ROUND_UP(note_head_size, 4);
 memcpy(buf, name, name_size);
-buf += ((name_size + 3) / 4) * 4;
+buf += ROUND_UP(name_size, 4);
 memcpy(buf, &state, sizeof(state));
 
 ret = f(note, note_size, opaque);
-- 
2.14.1.146.gd35faa819

Re: [Qemu-devel] [Qemu-discuss] Accessing a shared folder

[Mahmood via Qemu-devel]

Hello again,
For the command

    mount -t 9p -o trans=virtio Downloads /media/Downloads

inside the Centos-7 guest, I get this error

mount: unknown filesystem type '9p'

Any thought?

Regards,
Mahmood

[Qemu-devel] [PULL 05/29] i8254: use QEMU_ALIGN_DOWN

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
---
 hw/timer/i8254_common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/timer/i8254_common.c b/hw/timer/i8254_common.c
index 976d5200f1..ee064aa819 100644
--- a/hw/timer/i8254_common.c
+++ b/hw/timer/i8254_common.c
@@ -93,7 +93,7 @@ int64_t pit_get_next_transition_time(PITChannelState *s, 
int64_t current_time)
 }
 break;
 case 2:
-base = (d / s->count) * s->count;
+base = QEMU_ALIGN_DOWN(d, s->count);
 if ((d - base) == 0 && d != 0) {
 next_time = base + s->count;
 } else {
@@ -101,7 +101,7 @@ int64_t pit_get_next_transition_time(PITChannelState *s, 
int64_t current_time)
 }
 break;
 case 3:
-base = (d / s->count) * s->count;
+base = QEMU_ALIGN_DOWN(d, s->count);
 period2 = ((s->count + 1) >> 1);
 if ((d - base) < period2) {
 next_time = base + period2;
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 02/29] vnc: use QEMU_ALIGN_DOWN

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Richard Henderson 
---
 ui/vnc.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 651cbb8606..2c1c0cb5ed 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2624,8 +2624,8 @@ static int vnc_refresh_lossy_rect(VncDisplay *vd, int x, 
int y)
 int stx = x / VNC_STAT_RECT;
 int has_dirty = 0;
 
-y = y / VNC_STAT_RECT * VNC_STAT_RECT;
-x = x / VNC_STAT_RECT * VNC_STAT_RECT;
+y = QEMU_ALIGN_DOWN(y, VNC_STAT_RECT);
+x = QEMU_ALIGN_DOWN(x, VNC_STAT_RECT);
 
 QTAILQ_FOREACH(vs, &vd->clients, next) {
 int j;
@@ -2714,8 +2714,8 @@ double vnc_update_freq(VncState *vs, int x, int y, int w, 
int h)
 double total = 0;
 int num = 0;
 
-x =  (x / VNC_STAT_RECT) * VNC_STAT_RECT;
-y =  (y / VNC_STAT_RECT) * VNC_STAT_RECT;
+x =  QEMU_ALIGN_DOWN(x, VNC_STAT_RECT);
+y =  QEMU_ALIGN_DOWN(y, VNC_STAT_RECT);
 
 for (j = y; j <= y + h; j += VNC_STAT_RECT) {
 for (i = x; i <= x + w; i += VNC_STAT_RECT) {
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 04/29] vhost: use QEMU_ALIGN_DOWN

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Richard Henderson 
---
 hw/virtio/vhost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 6eddb099b0..0049a2c0b3 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -70,7 +70,7 @@ static void vhost_dev_sync_region(struct vhost_dev *dev,
 uint64_t end = MIN(mlast, rlast);
 vhost_log_chunk_t *from = log + start / VHOST_LOG_CHUNK;
 vhost_log_chunk_t *to = log + end / VHOST_LOG_CHUNK + 1;
-uint64_t addr = (start / VHOST_LOG_CHUNK) * VHOST_LOG_CHUNK;
+uint64_t addr = QEMU_ALIGN_DOWN(start, VHOST_LOG_CHUNK);
 
 if (end < start) {
 return;
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 07/29] dmg: use DIV_ROUND_UP

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Richard Henderson 
---
 block/dmg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/dmg.c b/block/dmg.c
index 900ae5a678..6c0711f563 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -111,7 +111,7 @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t 
chunk,
 uncompressed_sectors = s->sectorcounts[chunk];
 break;
 case 1: /* copy */
-uncompressed_sectors = (s->lengths[chunk] + 511) / 512;
+uncompressed_sectors = DIV_ROUND_UP(s->lengths[chunk], 512);
 break;
 case 2: /* zero */
 /* as the all-zeroes block may be large, it is treated specially: the
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 03/29] vhdx: use QEMU_ALIGN_DOWN

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eric Blake 
Reviewed-by: Richard Henderson 
---
 block/vhdx-log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/vhdx-log.c b/block/vhdx-log.c
index 14b724ef7b..0ac4863b25 100644
--- a/block/vhdx-log.c
+++ b/block/vhdx-log.c
@@ -902,7 +902,7 @@ static int vhdx_log_write(BlockDriverState *bs, 
BDRVVHDXState *s,
 }
 
 sector_offset = offset % VHDX_LOG_SECTOR_SIZE;
-file_offset = (offset / VHDX_LOG_SECTOR_SIZE) * VHDX_LOG_SECTOR_SIZE;
+file_offset = QEMU_ALIGN_DOWN(offset, VHDX_LOG_SECTOR_SIZE);
 
 aligned_length = length;
 
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 06/29] pcspk: use QEMU_ALIGN_DOWN

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Richard Henderson 
---
 hw/audio/pcspk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index f643b122bb..0206f7399b 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -69,7 +69,7 @@ static inline void generate_samples(PCSpkState *s)
 const uint32_t n = ((uint64_t)PIT_FREQ << 32) / m;
 
 /* multiple of wavelength for gapless looping */
-s->samples = (PCSPK_BUF_LEN * PIT_FREQ / m * m / (PIT_FREQ >> 1) + 1) 
>> 1;
+s->samples = (QEMU_ALIGN_DOWN(PCSPK_BUF_LEN * PIT_FREQ, m) / (PIT_FREQ 
>> 1) + 1) >> 1;
 for (i = 0; i < s->samples; ++i)
 s->sample_buf[i] = (64 & (n * i >> 25)) - 32;
 } else {
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 12/29] ui: use DIV_ROUND_UP

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
---
 ui/cursor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/cursor.c b/ui/cursor.c
index 5155b392e8..2e2fe13fa6 100644
--- a/ui/cursor.c
+++ b/ui/cursor.c
@@ -118,7 +118,7 @@ void cursor_put(QEMUCursor *c)
 
 int cursor_get_mono_bpl(QEMUCursor *c)
 {
-return (c->width + 7) / 8;
+return DIV_ROUND_UP(c->width, 8);
 }
 
 void cursor_set_mono(QEMUCursor *c,
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 14/29] virtio-gpu: use DIV_ROUND_UP

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
---
 hw/display/virtio-gpu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 6aae147324..f0761cf18b 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -408,7 +408,7 @@ static void virtio_gpu_transfer_to_host_2d(VirtIOGPU *g,
 }
 
 format = pixman_image_get_format(res->image);
-bpp = (PIXMAN_FORMAT_BPP(format) + 7) / 8;
+bpp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(format), 8);
 stride = pixman_image_get_stride(res->image);
 
 if (t2d.offset || t2d.r.x || t2d.r.y ||
@@ -570,7 +570,7 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g,
 scanout = &g->scanout[ss.scanout_id];
 
 format = pixman_image_get_format(res->image);
-bpp = (PIXMAN_FORMAT_BPP(format) + 7) / 8;
+bpp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(format), 8);
 offset = (ss.r.x * bpp) + ss.r.y * pixman_image_get_stride(res->image);
 if (!scanout->ds || surface_data(scanout->ds)
 != ((uint8_t *)pixman_image_get_data(res->image) + offset) ||
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 10/29] vvfat: use DIV_ROUND_UP

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eric Blake 
Reviewed-by: Richard Henderson 
---
 block/vvfat.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index a9e207f7f0..c54fa94651 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -449,7 +449,7 @@ static direntry_t *create_long_filename(BDRVVVFATState *s, 
const char *filename)
 return NULL;
 }
 
-number_of_entries = (length * 2 + 25) / 26;
+number_of_entries = DIV_ROUND_UP(length * 2, 26);
 
 for(i=0;idirectory));
@@ -2554,7 +2554,7 @@ static int commit_one_file(BDRVVVFATState* s,
 (size > offset && c >=2 && !fat_eof(s, c)));
 
 ret = vvfat_read(s->bs, cluster2sector(s, c),
-(uint8_t*)cluster, (rest_size + 0x1ff) / 0x200);
+(uint8_t*)cluster, DIV_ROUND_UP(rest_size, 0x200));
 
 if (ret < 0) {
 qemu_close(fd);
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 08/29] qcow2: use DIV_ROUND_UP

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eric Blake 
Reviewed-by: Richard Henderson 
---
 block/qcow2-cluster.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index f06c08f64c..30db942dde 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -61,7 +61,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t 
min_size,
 new_l1_size = 1;
 }
 while (min_size > new_l1_size) {
-new_l1_size = (new_l1_size * 3 + 1) / 2;
+new_l1_size = DIV_ROUND_UP(new_l1_size * 3, 2);
 }
 }
 
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 15/29] monitor: use DIV_ROUND_UP

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Acked-by: Dr. David Alan Gilbert 
Reviewed-by: Richard Henderson 
---
 monitor.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/monitor.c b/monitor.c
index e0f880107f..135ec2de8f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1349,7 +1349,7 @@ static void memory_dump(Monitor *mon, int count, int 
format, int wsize,
 
 switch(format) {
 case 'o':
-max_digits = (wsize * 8 + 2) / 3;
+max_digits = DIV_ROUND_UP(wsize * 8, 3);
 break;
 default:
 case 'x':
@@ -1357,7 +1357,7 @@ static void memory_dump(Monitor *mon, int count, int 
format, int wsize,
 break;
 case 'u':
 case 'd':
-max_digits = (wsize * 8 * 10 + 32) / 33;
+max_digits = DIV_ROUND_UP(wsize * 8 * 10, 33);
 break;
 case 'c':
 wsize = 1;
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 11/29] vnc: use DIV_ROUND_UP

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Richard Henderson 
---
 ui/vnc-enc-tight.c | 2 +-
 ui/vnc.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 89ab12c0d8..f38aceb4da 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -979,7 +979,7 @@ static int send_mono_rect(VncState *vs, int x, int y,
 }
 #endif
 
-bytes = ((w + 7) / 8) * h;
+bytes = (DIV_ROUND_UP(w, 8)) * h;
 
 vnc_write_u8(vs, (stream | VNC_TIGHT_EXPLICIT_FILTER) << 4);
 vnc_write_u8(vs, VNC_TIGHT_FILTER_PALETTE);
diff --git a/ui/vnc.c b/ui/vnc.c
index 2c1c0cb5ed..fd43f9b983 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2781,7 +2781,7 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
 PIXMAN_FORMAT_BPP(pixman_image_get_format(vd->guest.fb));
 guest_row0 = (uint8_t *)pixman_image_get_data(vd->guest.fb);
 guest_stride = pixman_image_get_stride(vd->guest.fb);
-guest_ll = pixman_image_get_width(vd->guest.fb) * ((guest_bpp + 7) / 
8);
+guest_ll = pixman_image_get_width(vd->guest.fb) * 
(DIV_ROUND_UP(guest_bpp, 8));
 }
 line_bytes = MIN(server_stride, guest_ll);
 
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 19/29] q35: use DIV_ROUND_UP

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Richard Henderson 
---
 hw/pci-host/q35.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 0e472f2ed4..1ff648e80c 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -315,7 +315,7 @@ static void mch_update_pam(MCHPCIState *mch)
 memory_region_transaction_begin();
 for (i = 0; i < 13; i++) {
 pam_update(&mch->pam_regions[i], i,
-   pd->config[MCH_HOST_BRIDGE_PAM0 + ((i + 1) / 2)]);
+   pd->config[MCH_HOST_BRIDGE_PAM0 + (DIV_ROUND_UP(i, 2))]);
 }
 memory_region_transaction_commit();
 }
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 09/29] vpc: use DIV_ROUND_UP

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eric Blake 
Reviewed-by: Richard Henderson 
---
 block/vpc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index 82911ebead..1576d7b595 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -783,7 +783,7 @@ static int calculate_geometry(int64_t total_sectors, 
uint16_t* cyls,
 } else {
 *secs_per_cyl = 17;
 cyls_times_heads = total_sectors / *secs_per_cyl;
-*heads = (cyls_times_heads + 1023) / 1024;
+*heads = DIV_ROUND_UP(cyls_times_heads, 1024);
 
 if (*heads < 4) {
 *heads = 4;
@@ -836,7 +836,7 @@ static int create_dynamic_disk(BlockBackend *blk, uint8_t 
*buf,
 offset = 3 * 512;
 
 memset(buf, 0xFF, 512);
-for (i = 0; i < (num_bat_entries * 4 + 511) / 512; i++) {
+for (i = 0; i < DIV_ROUND_UP(num_bat_entries * 4, 512); i++) {
 ret = blk_pwrite(blk, offset, buf, 512, 0);
 if (ret < 0) {
 goto fail;
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 17/29] virtio-serial: use DIV_ROUND_UP

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Reviewed-by: Amit Shah 
---
 hw/char/virtio-serial-bus.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index f5bc173844..17a1bb008a 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -663,7 +663,7 @@ static void virtio_serial_save_device(VirtIODevice *vdev, 
QEMUFile *f)
 
 /* The ports map */
 max_nr_ports = s->serial.max_virtserial_ports;
-for (i = 0; i < (max_nr_ports + 31) / 32; i++) {
+for (i = 0; i < DIV_ROUND_UP(max_nr_ports, 32); i++) {
 qemu_put_be32s(f, &s->ports_map[i]);
 }
 
@@ -798,7 +798,7 @@ static int virtio_serial_load_device(VirtIODevice *vdev, 
QEMUFile *f,
 qemu_get_be32s(f, &tmp);
 
 max_nr_ports = s->serial.max_virtserial_ports;
-for (i = 0; i < (max_nr_ports + 31) / 32; i++) {
+for (i = 0; i < DIV_ROUND_UP(max_nr_ports, 32); i++) {
 qemu_get_be32s(f, &ports_map);
 
 if (ports_map != s->ports_map[i]) {
@@ -863,7 +863,7 @@ static uint32_t find_free_port_id(VirtIOSerial *vser)
 unsigned int i, max_nr_ports;
 
 max_nr_ports = vser->serial.max_virtserial_ports;
-for (i = 0; i < (max_nr_ports + 31) / 32; i++) {
+for (i = 0; i < DIV_ROUND_UP(max_nr_ports, 32); i++) {
 uint32_t map, zeroes;
 
 map = vser->ports_map[i];
@@ -1075,7 +1075,7 @@ static void virtio_serial_device_realize(DeviceState 
*dev, Error **errp)
 vser->ovqs[i] = virtio_add_queue(vdev, 128, handle_output);
 }
 
-vser->ports_map = g_malloc0(((vser->serial.max_virtserial_ports + 31) / 32)
+vser->ports_map = 
g_malloc0((DIV_ROUND_UP(vser->serial.max_virtserial_ports, 32))
 * sizeof(vser->ports_map[0]));
 /*
  * Reserve location 0 for a console port for backward compat
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 25/29] decnumber: use DIV_ROUND_UP

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Richard Henderson 
---
 libdecnumber/decNumber.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libdecnumber/decNumber.c b/libdecnumber/decNumber.c
index c9e7807f87..8c197023f4 100644
--- a/libdecnumber/decNumber.c
+++ b/libdecnumber/decNumber.c
@@ -4775,7 +4775,7 @@ static decNumber * decDivideOp(decNumber *res,
half=*up & 0x01;
*up/=2;/* [shift] */
if (!half) continue;
-   *(up-1)+=(DECDPUNMAX+1)/2;
+   *(up-1)+=DIV_ROUND_UP(DECDPUNMAX, 2);
}
  /* [accunits still describes the original remainder length] */
 
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 18/29] piix: use DIV_ROUND_UP

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
---
 hw/pci-host/piix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 072a04e318..894e131c00 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -140,7 +140,7 @@ static void i440fx_update_memory_mappings(PCII440FXState *d)
 memory_region_transaction_begin();
 for (i = 0; i < 13; i++) {
 pam_update(&d->pam_regions[i], i,
-   pd->config[I440FX_PAM + ((i + 1) / 2)]);
+   pd->config[I440FX_PAM + (DIV_ROUND_UP(i, 2))]);
 }
 memory_region_set_enabled(&d->smram_region,
   !(pd->config[I440FX_SMRAM] & SMRAM_D_OPEN));
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 13/29] vga: use DIV_ROUND_UP

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Richard Henderson 
---
 hw/display/vga.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/vga.c b/hw/display/vga.c
index 63421f9ee8..3433102ef3 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1621,7 +1621,7 @@ static void vga_draw_graphic(VGACommonState *s, int 
full_update)
s->line_compare, sr(s, VGA_SEQ_CLOCK_MODE));
 #endif
 addr1 = (s->start_addr * 4);
-bwidth = (width * bits + 7) / 8;
+bwidth = DIV_ROUND_UP(width * bits, 8);
 y_start = -1;
 d = surface_data(surface);
 linesize = surface_stride(surface);
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 21/29] msix: use DIV_ROUND_UP

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
---
 hw/pci/msix.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 5078d3dd19..c944c02135 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -438,7 +438,7 @@ void msix_save(PCIDevice *dev, QEMUFile *f)
 }
 
 qemu_put_buffer(f, dev->msix_table, n * PCI_MSIX_ENTRY_SIZE);
-qemu_put_buffer(f, dev->msix_pba, (n + 7) / 8);
+qemu_put_buffer(f, dev->msix_pba, DIV_ROUND_UP(n, 8));
 }
 
 /* Should be called after restoring the config space. */
@@ -453,7 +453,7 @@ void msix_load(PCIDevice *dev, QEMUFile *f)
 
 msix_clear_all_vectors(dev);
 qemu_get_buffer(f, dev->msix_table, n * PCI_MSIX_ENTRY_SIZE);
-qemu_get_buffer(f, dev->msix_pba, (n + 7) / 8);
+qemu_get_buffer(f, dev->msix_pba, DIV_ROUND_UP(n, 8));
 msix_update_function_masked(dev);
 
 for (vector = 0; vector < n; vector++) {
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PULL 22/29] ppc: use DIV_ROUND_UP

[Marc-André Lureau]

I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra

Signed-off-by: Marc-André Lureau 
Acked-by: David Gibson 
Reviewed-by: Richard Henderson 
---
 target/ppc/mem_helper.c | 2 +-
 target/ppc/translate.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
index e6383c6bfa..a34e604db3 100644
--- a/target/ppc/mem_helper.c
+++ b/target/ppc/mem_helper.c
@@ -111,7 +111,7 @@ void helper_lswx(CPUPPCState *env, target_ulong addr, 
uint32_t reg,
  uint32_t ra, uint32_t rb)
 {
 if (likely(xer_bc != 0)) {
-int num_used_regs = (xer_bc + 3) / 4;
+int num_used_regs = DIV_ROUND_UP(xer_bc, 4);
 if (unlikely((ra != 0 && lsw_reg_in_range(reg, num_used_regs, ra)) ||
  lsw_reg_in_range(reg, num_used_regs, rb))) {
 raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 01233e8b6d..606b605ba0 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -2880,7 +2880,7 @@ static void gen_lswi(DisasContext *ctx)
 }
 if (nb == 0)
 nb = 32;
-nr = (nb + 3) / 4;
+nr = DIV_ROUND_UP(nb, 4);
 if (unlikely(lsw_reg_in_range(start, nr, ra))) {
 gen_inval_exception(ctx, POWERPC_EXCP_INVAL_LSWX);
 return;
-- 
2.14.1.146.gd35faa819

Re: [Qemu-devel] [PATCH] scripts: Support building with Python 3

[Daniel P. Berrange]

On Thu, Aug 31, 2017 at 11:27:15AM +0100, Peter Maydell wrote:
> On 31 August 2017 at 07:35, Markus Armbruster  wrote:
> > So, first we'll invest in work-arounds to make both 2 and 3 work.  Once
> > 2 is gone, we can invest some more to clean them up.  Which probably
> > won't happen, so we'll continue to carry work-arounds that no longer
> > make sense.
> >
> > I maintain roughly one fourth of all Python code in qemu, and I'm not
> > looking forward to this hoop-jumping at all.
> >
> > Are we really, really sure we want to go this way?  What exactly are we
> > hoping to accomplish by it?
> 
> My take is that we have the following goals we want to achieve:
> 
> (1) We need to continue to build and run on older (long-term-support)
> distros that still ship only Python 2.x (alas even back to 2.6)
> (2) We need to be able to build and run on newer distros that
> have dropped Python 2 altogether in favour of Python 3
> (I don't know if there are any such today, but presumably by
> 2020 there will be)

Fedora has dropped Python 2 in the default install now, so needs to
be manually pulled in. While python 2 won't be dropped entirely in
the near future, its days are definitely numbered and so once
upstream officially dedclares it dead, I'd expect Fedora to follow.
 
> Unless we can confidently say that either (1) or (2) is the
> empty set, we need to handle both 2 and 3 in the same codebase.
> This is a pain, but unfortunately Python upstream have forced
> us into it by breaking source code compatibility.
> 
> I think (1) is pretty clearly not (yet) an empty set, so the
> only alternative I see to "support 2 and 3 now" is "keep supporting
> only 2 for the moment and hope that no distro drops 2 support
> before all the LTS 2-only distro versions vanish into history".

If we can update to python 2.7 as our minimum, then supporting py2
and py3 gets simpler, avoiding some of the nastier hacks, even
without that though it isn't too hard.


>From QEMU pov, I don't think supporting py2 and py3 at the same time
is actually as bad as you might expect from reading about the pain
in the broader community. We're lucky that pretty much all our python
code is self-contained not relying on 3rd party modules, and it
doesn't use particularly advanced features of python either.

I worked on OpenStack Nova during the conversion to support py2 & py3
in parallel, and the resulting code was not really any harder to
maintain than the original code. The hard bit was Nova's use of 3rd
party modules whose maintainers hadn't converted.

So, IMHO, provided we make sure our CI system is giving us test coverage
across the various py2 & py3 versions, the burden on us will not be
significant. Writing code that's py2 & py3 compatible is not hard as
long as you have good testing. So we just have a lump for the initial
conversion, then CI will take care of avoiding regressions going forward.

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] [PULL 26/29] i386: introduce ELF_NOTE_SIZE macro

[Marc-André Lureau]

Factour out a common pattern to compute the ELF note size.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eduardo Habkost 
Reviewed-by: Richard Henderson 
---
 target/i386/arch_dump.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/target/i386/arch_dump.c b/target/i386/arch_dump.c
index e081f677fa..e682904052 100644
--- a/target/i386/arch_dump.c
+++ b/target/i386/arch_dump.c
@@ -18,6 +18,11 @@
 #include "elf.h"
 #include "sysemu/memory_mapping.h"
 
+#define ELF_NOTE_SIZE(hdr_size, name_size, desc_size)   \
+((DIV_ROUND_UP((hdr_size), 4)   \
+  + DIV_ROUND_UP((name_size), 4)\
+  + DIV_ROUND_UP((desc_size), 4)) * 4)
+
 #ifdef TARGET_X86_64
 typedef struct {
 target_ulong r15, r14, r13, r12, rbp, rbx, r11, r10;
@@ -77,8 +82,7 @@ static int x86_64_write_elf64_note(WriteCoreDumpFunction f,
 regs.gs = env->segs[R_GS].selector;
 
 descsz = sizeof(x86_64_elf_prstatus);
-note_size = (DIV_ROUND_UP(sizeof(Elf64_Nhdr), 4) + DIV_ROUND_UP(name_size, 
4) +
-DIV_ROUND_UP(descsz, 4)) * 4;
+note_size = ELF_NOTE_SIZE(sizeof(Elf64_Nhdr), name_size, descsz);
 note = g_malloc0(note_size);
 note->n_namesz = cpu_to_le32(name_size);
 note->n_descsz = cpu_to_le32(descsz);
@@ -156,8 +160,7 @@ static int x86_write_elf64_note(WriteCoreDumpFunction f, 
CPUX86State *env,
 
 x86_fill_elf_prstatus(&prstatus, env, id);
 descsz = sizeof(x86_elf_prstatus);
-note_size = (DIV_ROUND_UP(sizeof(Elf64_Nhdr), 4) + DIV_ROUND_UP(name_size, 
4) +
-DIV_ROUND_UP(descsz, 4)) * 4;
+note_size = ELF_NOTE_SIZE(sizeof(Elf64_Nhdr), name_size, descsz);
 note = g_malloc0(note_size);
 note->n_namesz = cpu_to_le32(name_size);
 note->n_descsz = cpu_to_le32(descsz);
@@ -211,8 +214,7 @@ int x86_cpu_write_elf32_note(WriteCoreDumpFunction f, 
CPUState *cs,
 
 x86_fill_elf_prstatus(&prstatus, &cpu->env, cpuid);
 descsz = sizeof(x86_elf_prstatus);
-note_size = (DIV_ROUND_UP(sizeof(Elf32_Nhdr), 4) + DIV_ROUND_UP(name_size, 
4) +
-DIV_ROUND_UP(descsz, 4)) * 4;
+note_size = ELF_NOTE_SIZE(sizeof(Elf32_Nhdr), name_size, descsz);
 note = g_malloc0(note_size);
 note->n_namesz = cpu_to_le32(name_size);
 note->n_descsz = cpu_to_le32(descsz);
@@ -443,10 +445,8 @@ ssize_t cpu_get_note_size(int class, int machine, int 
nr_cpus)
 #endif
 qemu_desc_size = sizeof(QEMUCPUState);
 
-elf_note_size = (DIV_ROUND_UP(note_head_size, 4) + DIV_ROUND_UP(name_size, 
4) +
- DIV_ROUND_UP(elf_desc_size, 4)) * 4;
-qemu_note_size = (DIV_ROUND_UP(note_head_size, 4) + 
DIV_ROUND_UP(name_size, 4) +
-  DIV_ROUND_UP(qemu_desc_size, 4)) * 4;
+elf_note_size = ELF_NOTE_SIZE(note_head_size, name_size, elf_desc_size);
+qemu_note_size = ELF_NOTE_SIZE(note_head_size, name_size, qemu_desc_size);
 
 return (elf_note_size + qemu_note_size) * nr_cpus;
 }
-- 
2.14.1.146.gd35faa819

Re: [Qemu-devel] [PATCH 0/9]

[Halil Pasic]

On 08/31/2017 12:04 PM, Cornelia Huck wrote:
> On Wed, 30 Aug 2017 18:36:00 +0200
> Halil Pasic  wrote:
> 
>> This series has a character of a refactoring, as the initial motivation
>> was improving readability and reducing complexity.
> 
> But you reduced the cover letter subject too much ;)

Noticed that after sending. The plan was to copy paste
it form the internal version, but I'm no good at copy-pasting.

> 
>>
>> Despite of the original intent the tree first patches buxfixes, and
>> according to Dong Jia the patch-set also has a functional value: for ccw
>> pass-through, he is planing to pass-through the instruction completion
>> information (cc or interruption condition) from the kernel, and this
>> patch set can pretty much be seen as a preparation for that.
> 
> I think the first one (with the precedence fixed as well) is pretty
> uncontroversial, and I'd be happy to queue an updated version.
> 

I will send it as a stand-alone patch (probably today).

>>
>> The basic idea is: tell how to handle an unusual conditon where it's
>> identified, instead of mapping it to an errno (more or less arbitrarily),
>> then possibly mapping these errnos around, to finally (mentally) map the
>> errno back to the condition and take appropriate action. 
>>
>> At the end of the series we also have 125 lines of code less to maintain,
>> and the executable got a bit smaller too.
> 
> I agree with the basic direction, but I think this needs more hashing
> out.
> 

I'm sure we will sort the details out.

>>
>> Halil Pasic (9):
>>   s390x/css: fix cc handling for XSCH
>>   s390x: fix invalid use of cc 1 for SSCH
>>   s390x/css: be more consistent if broken beyond repair
>>   s390x: refactor error handling for SSCH and RSCH
>>   s390x: refactor error handling for XSCH handler
>>   s390x: refactor error handling for CSCH handler
>>   s390x: refactor error handling for HSCH handler
>>   s390x: refactor error handling for MSCH handler
>>   s390x: factor out common ioinst handler logic
>>
>>  hw/s390x/css.c  | 158 --
>>  hw/s390x/s390-ccw.c |   8 +-
>>  hw/vfio/ccw.c   |  32 ++--
>>  include/hw/s390x/css.h  |  38 +++---
>>  include/hw/s390x/s390-ccw.h |   2 +-
>>  target/s390x/ioinst.c   | 181 
>> ++--
>>  6 files changed, 147 insertions(+), 272 deletions(-)
>>
> 

[Qemu-devel] [PULL 28/29] test-iov: replace g_malloc()+memcpy() with g_memdup()

[Marc-André Lureau]

I found these pattern via grepping the source tree. I don't have a
coccinelle script for it!

Signed-off-by: Marc-André Lureau 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
---
 tests/test-iov.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/test-iov.c b/tests/test-iov.c
index a22d71fd2c..fa3d75aee1 100644
--- a/tests/test-iov.c
+++ b/tests/test-iov.c
@@ -167,8 +167,7 @@ static void test_io(void)
 }
 iov_from_buf(iov, niov, 0, buf, sz);
 
-siov = g_malloc(sizeof(*iov) * niov);
-memcpy(siov, iov, sizeof(*iov) * niov);
+siov = g_memdup(iov, sizeof(*iov) * niov);
 
 if (socketpair(PF_UNIX, SOCK_STREAM, 0, sv) < 0) {
perror("socketpair");
-- 
2.14.1.146.gd35faa819

Re: [Qemu-devel] [Qemu-discuss] Accessing a shared folder

[Peter Maydell]

On 31 August 2017 at 11:34, Mahmood via Qemu-devel
 wrote:
> Hello again,
> For the command
>
> mount -t 9p -o trans=virtio Downloads /media/Downloads
>
> inside the Centos-7 guest, I get this error
>
> mount: unknown filesystem type '9p'
>
> Any thought?

That's an issue with your guest OS (probably it doesn't
have 9p support enabled in the kernel).

thanks
-- PMM

[Qemu-devel] [PULL 29/29] eepro100: replace g_malloc()+memcpy() with g_memdup()

[Marc-André Lureau]

I found these pattern via grepping the source tree. I don't have a
coccinelle script for it!

Signed-off-by: Marc-André Lureau 
Reviewed-by: Stefan Weil 
Reviewed-by: Jason Wang 
Reviewed-by: Richard Henderson 
---
 hw/net/eepro100.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index 5a4774aab4..a7b9f77519 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -1904,8 +1904,7 @@ static void e100_nic_realize(PCIDevice *pci_dev, Error 
**errp)
 
 qemu_register_reset(nic_reset, s);
 
-s->vmstate = g_malloc(sizeof(vmstate_eepro100));
-memcpy(s->vmstate, &vmstate_eepro100, sizeof(vmstate_eepro100));
+s->vmstate = g_memdup(&vmstate_eepro100, sizeof(vmstate_eepro100));
 s->vmstate->name = qemu_get_queue(s->nic)->model;
 vmstate_register(&pci_dev->qdev, -1, s->vmstate, s);
 }
-- 
2.14.1.146.gd35faa819

Re: [Qemu-devel] [PATCH] scripts: Support building with Python 3

[Daniel P. Berrange]

On Thu, Aug 31, 2017 at 11:55:18AM +0100, Peter Maydell wrote:
> On 31 August 2017 at 11:47, Daniel P. Berrange  wrote:
> > If we can update to python 2.7 as our minimum, then supporting py2
> > and py3 gets simpler, avoiding some of the nastier hacks, even
> > without that though it isn't too hard.
> 
> Unfortunately RHEL6 is what's holding us to retaining 2.6
> support, and googling suggests that doesn't go EOL until 2020...

Who is actually requiring that we support RHEL6 until EOL though ?

AFAIK, from Red Hat side, we don't need QEMU git master to be buildable
on RHEL-6 machines anymore, since QEMU there is long since in bug-fix
only mode, no rebases or feature backports.

I assume there's probably some community interest in building git master
on RHEL-6, but I think it is reasonable for us to say no at some point,
rather than waiting until final EOL (2020 for normal subscribers, 2024
for people who paid for extended lifetime).

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] [PULL 27/29] i386: replace g_malloc()+memcpy() with g_memdup()

[Marc-André Lureau]

I found these pattern via grepping the source tree. I don't have a
coccinelle script for it!

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eduardo Habkost 
Reviewed-by: Richard Henderson 
---
 hw/i386/multiboot.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index f13e23139b..6001f4caa2 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -352,8 +352,7 @@ int load_multiboot(FWCfgState *fw_cfg,
 mb_debug("   mb_mods_count = %d\n", mbs.mb_mods_count);
 
 /* save bootinfo off the stack */
-mb_bootinfo_data = g_malloc(sizeof(bootinfo));
-memcpy(mb_bootinfo_data, bootinfo, sizeof(bootinfo));
+mb_bootinfo_data = g_memdup(bootinfo, sizeof(bootinfo));
 
 /* Pass variables to option rom */
 fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, mh_entry_addr);
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PATCH v2 2/4] block: use BDRV_SECTOR_SIZE in crypto driver

[Daniel P. Berrange]

Signed-off-by: Daniel P. Berrange 
---
 block/crypto.c | 32 
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index cc8afe0e0d..e63f094379 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -392,16 +392,16 @@ block_crypto_co_readv(BlockDriverState *bs, int64_t 
sector_num,
 QEMUIOVector hd_qiov;
 int ret = 0;
 size_t payload_offset =
-qcrypto_block_get_payload_offset(crypto->block) / 512;
+qcrypto_block_get_payload_offset(crypto->block) / BDRV_SECTOR_SIZE;
 
 qemu_iovec_init(&hd_qiov, qiov->niov);
 
 /* Bounce buffer because we don't wish to expose cipher text
  * in qiov which points to guest memory.
  */
-cipher_data =
-qemu_try_blockalign(bs->file->bs, MIN(BLOCK_CRYPTO_MAX_SECTORS * 512,
-  qiov->size));
+cipher_data = qemu_try_blockalign(
+bs->file->bs, MIN(BLOCK_CRYPTO_MAX_SECTORS * BDRV_SECTOR_SIZE,
+  qiov->size));
 if (cipher_data == NULL) {
 ret = -ENOMEM;
 goto cleanup;
@@ -415,7 +415,7 @@ block_crypto_co_readv(BlockDriverState *bs, int64_t 
sector_num,
 }
 
 qemu_iovec_reset(&hd_qiov);
-qemu_iovec_add(&hd_qiov, cipher_data, cur_nr_sectors * 512);
+qemu_iovec_add(&hd_qiov, cipher_data, cur_nr_sectors * 
BDRV_SECTOR_SIZE);
 
 ret = bdrv_co_readv(bs->file,
 payload_offset + sector_num,
@@ -426,18 +426,18 @@ block_crypto_co_readv(BlockDriverState *bs, int64_t 
sector_num,
 
 if (qcrypto_block_decrypt(crypto->block,
   sector_num,
-  cipher_data, cur_nr_sectors * 512,
+  cipher_data, cur_nr_sectors * 
BDRV_SECTOR_SIZE,
   NULL) < 0) {
 ret = -EIO;
 goto cleanup;
 }
 
 qemu_iovec_from_buf(qiov, bytes_done,
-cipher_data, cur_nr_sectors * 512);
+cipher_data, cur_nr_sectors * BDRV_SECTOR_SIZE);
 
 remaining_sectors -= cur_nr_sectors;
 sector_num += cur_nr_sectors;
-bytes_done += cur_nr_sectors * 512;
+bytes_done += cur_nr_sectors * BDRV_SECTOR_SIZE;
 }
 
  cleanup:
@@ -459,16 +459,16 @@ block_crypto_co_writev(BlockDriverState *bs, int64_t 
sector_num,
 QEMUIOVector hd_qiov;
 int ret = 0;
 size_t payload_offset =
-qcrypto_block_get_payload_offset(crypto->block) / 512;
+qcrypto_block_get_payload_offset(crypto->block) / BDRV_SECTOR_SIZE;
 
 qemu_iovec_init(&hd_qiov, qiov->niov);
 
 /* Bounce buffer because we're not permitted to touch
  * contents of qiov - it points to guest memory.
  */
-cipher_data =
-qemu_try_blockalign(bs->file->bs, MIN(BLOCK_CRYPTO_MAX_SECTORS * 512,
-  qiov->size));
+cipher_data = qemu_try_blockalign(
+bs->file->bs, MIN(BLOCK_CRYPTO_MAX_SECTORS * BDRV_SECTOR_SIZE,
+  qiov->size));
 if (cipher_data == NULL) {
 ret = -ENOMEM;
 goto cleanup;
@@ -482,18 +482,18 @@ block_crypto_co_writev(BlockDriverState *bs, int64_t 
sector_num,
 }
 
 qemu_iovec_to_buf(qiov, bytes_done,
-  cipher_data, cur_nr_sectors * 512);
+  cipher_data, cur_nr_sectors * BDRV_SECTOR_SIZE);
 
 if (qcrypto_block_encrypt(crypto->block,
   sector_num,
-  cipher_data, cur_nr_sectors * 512,
+  cipher_data, cur_nr_sectors * 
BDRV_SECTOR_SIZE,
   NULL) < 0) {
 ret = -EIO;
 goto cleanup;
 }
 
 qemu_iovec_reset(&hd_qiov);
-qemu_iovec_add(&hd_qiov, cipher_data, cur_nr_sectors * 512);
+qemu_iovec_add(&hd_qiov, cipher_data, cur_nr_sectors * 
BDRV_SECTOR_SIZE);
 
 ret = bdrv_co_writev(bs->file,
  payload_offset + sector_num,
@@ -504,7 +504,7 @@ block_crypto_co_writev(BlockDriverState *bs, int64_t 
sector_num,
 
 remaining_sectors -= cur_nr_sectors;
 sector_num += cur_nr_sectors;
-bytes_done += cur_nr_sectors * 512;
+bytes_done += cur_nr_sectors * BDRV_SECTOR_SIZE;
 }
 
  cleanup:
-- 
2.13.5

[Qemu-devel] [PATCH v2 1/4] block: use 1 MB bounce buffers for crypto instead of 16KB

[Daniel P. Berrange]

Using 16KB bounce buffers creates a significant performance
penalty for I/O to encrypted volumes on storage which high
I/O latency (rotating rust & network drives), because it
triggers lots of fairly small I/O operations.

On tests with rotating rust, and cache=none|directsync,
write speed increased from 2MiB/s to 32MiB/s, on a par
with that achieved by the in-kernel luks driver. With
other cache modes the in-kernel driver is still notably
faster because it is able to report completion of the
I/O request before any encryption is done, while the
in-QEMU driver must encrypt the data before completion.

Reviewed-by: Eric Blake 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Daniel P. Berrange 
---
 block/crypto.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index 58ef6f2f52..cc8afe0e0d 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -379,7 +379,7 @@ static void block_crypto_close(BlockDriverState *bs)
 }
 
 
-#define BLOCK_CRYPTO_MAX_SECTORS 32
+#define BLOCK_CRYPTO_MAX_SECTORS 2048
 
 static coroutine_fn int
 block_crypto_co_readv(BlockDriverState *bs, int64_t sector_num,
@@ -396,9 +396,8 @@ block_crypto_co_readv(BlockDriverState *bs, int64_t 
sector_num,
 
 qemu_iovec_init(&hd_qiov, qiov->niov);
 
-/* Bounce buffer so we have a linear mem region for
- * entire sector. XXX optimize so we avoid bounce
- * buffer in case that qiov->niov == 1
+/* Bounce buffer because we don't wish to expose cipher text
+ * in qiov which points to guest memory.
  */
 cipher_data =
 qemu_try_blockalign(bs->file->bs, MIN(BLOCK_CRYPTO_MAX_SECTORS * 512,
@@ -464,9 +463,8 @@ block_crypto_co_writev(BlockDriverState *bs, int64_t 
sector_num,
 
 qemu_iovec_init(&hd_qiov, qiov->niov);
 
-/* Bounce buffer so we have a linear mem region for
- * entire sector. XXX optimize so we avoid bounce
- * buffer in case that qiov->niov == 1
+/* Bounce buffer because we're not permitted to touch
+ * contents of qiov - it points to guest memory.
  */
 cipher_data =
 qemu_try_blockalign(bs->file->bs, MIN(BLOCK_CRYPTO_MAX_SECTORS * 512,
-- 
2.13.5

[Qemu-devel] [PATCH v2 4/4] block: convert qcrypto_block_encrypt|decrypt to take bytes offset

[Daniel P. Berrange]

Instead of sector offset, take the bytes offset when encrypting
or decrypting data.

Signed-off-by: Daniel P. Berrange 
---
 block/crypto.c |  8 
 block/qcow.c   |  7 +--
 block/qcow2-cluster.c  |  8 +++-
 block/qcow2.c  |  4 ++--
 crypto/block-luks.c| 12 
 crypto/block-qcow.c| 12 
 crypto/block.c | 14 --
 crypto/blockpriv.h |  4 ++--
 include/crypto/block.h | 14 --
 9 files changed, 48 insertions(+), 35 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index 40daa77188..6b8d88efbc 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -426,8 +426,8 @@ block_crypto_co_preadv(BlockDriverState *bs, uint64_t 
offset, uint64_t bytes,
 goto cleanup;
 }
 
-if (qcrypto_block_decrypt(crypto->block, sector_num, cipher_data,
-  cur_bytes, NULL) < 0) {
+if (qcrypto_block_decrypt(crypto->block, offset + bytes_done,
+  cipher_data, cur_bytes, NULL) < 0) {
 ret = -EIO;
 goto cleanup;
 }
@@ -484,8 +484,8 @@ block_crypto_co_pwritev(BlockDriverState *bs, uint64_t 
offset, uint64_t bytes,
 
 qemu_iovec_to_buf(qiov, bytes_done, cipher_data, cur_bytes);
 
-if (qcrypto_block_encrypt(crypto->block, sector_num, cipher_data,
-  cur_bytes, NULL) < 0) {
+if (qcrypto_block_encrypt(crypto->block, offset + bytes_done,
+  cipher_data, cur_bytes, NULL) < 0) {
 ret = -EIO;
 goto cleanup;
 }
diff --git a/block/qcow.c b/block/qcow.c
index c08cdc4a7b..c2a446e824 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -456,7 +456,9 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
 if (i < n_start || i >= n_end) {
 Error *err = NULL;
 memset(s->cluster_data, 0x00, 512);
-if (qcrypto_block_encrypt(s->crypto, start_sect + 
i,
+if (qcrypto_block_encrypt(s->crypto,
+  start_sect + i *
+  BDRV_SECTOR_SIZE,
   s->cluster_data,
   BDRV_SECTOR_SIZE,
   &err) < 0) {
@@ -636,7 +638,8 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, 
int64_t sector_num,
 }
 if (bs->encrypted) {
 assert(s->crypto);
-if (qcrypto_block_decrypt(s->crypto, sector_num, buf,
+if (qcrypto_block_decrypt(s->crypto,
+  sector_num * BDRV_SECTOR_SIZE, buf,
   n * BDRV_SECTOR_SIZE, &err) < 0) {
 goto fail;
 }
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index f06c08f64c..85ffc33c2c 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -396,15 +396,13 @@ static bool coroutine_fn 
do_perform_cow_encrypt(BlockDriverState *bs,
 {
 if (bytes && bs->encrypted) {
 BDRVQcow2State *s = bs->opaque;
-int64_t sector = (s->crypt_physical_offset ?
+int64_t offset = (s->crypt_physical_offset ?
   (cluster_offset + offset_in_cluster) :
-  (src_cluster_offset + offset_in_cluster))
- >> BDRV_SECTOR_BITS;
+  (src_cluster_offset + offset_in_cluster));
 assert((offset_in_cluster & ~BDRV_SECTOR_MASK) == 0);
 assert((bytes & ~BDRV_SECTOR_MASK) == 0);
 assert(s->crypto);
-if (qcrypto_block_encrypt(s->crypto, sector, buffer,
-  bytes, NULL) < 0) {
+if (qcrypto_block_encrypt(s->crypto, offset, buffer, bytes, NULL) < 0) 
{
 return false;
 }
 }
diff --git a/block/qcow2.c b/block/qcow2.c
index 40ba26c111..1c9f6c8f7d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1824,7 +1824,7 @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState 
*bs, uint64_t offset,
 if (qcrypto_block_decrypt(s->crypto,
   (s->crypt_physical_offset ?
cluster_offset + offset_in_cluster :
-   offset) >> BDRV_SECTOR_BITS,
+   offset),
   cluster_data,
   cur_bytes,
   &err) < 0) {
@@ -1961,7 +1961,7 @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState 
*bs, uint64_t offset,
 if (qcrypto_block_encrypt(s->crypto,
 

Re: [Qemu-devel] [PATCH] scripts: Support building with Python 3

[Peter Maydell]

On 31 August 2017 at 11:47, Daniel P. Berrange  wrote:
> If we can update to python 2.7 as our minimum, then supporting py2
> and py3 gets simpler, avoiding some of the nastier hacks, even
> without that though it isn't too hard.

Unfortunately RHEL6 is what's holding us to retaining 2.6
support, and googling suggests that doesn't go EOL until 2020...

thanks
-- PMM

[Qemu-devel] [PATCH v2 0/4] block: improve luks driver perf & switch to byte APIs

[Daniel P. Berrange]

This series includes a previously posted patch that improves performance
of the luks crypto driver:

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

And then adds three patches that switch over to use byte based APIs for
I/O, rather than the legacy sector based APIs.

Daniel P. Berrange (4):
  block: use 1 MB bounce buffers for crypto instead of 16KB
  block: use BDRV_SECTOR_SIZE in crypto driver
  block: convert crypto driver to bdrv_co_preadv|pwritev
  block: convert qcrypto_block_encrypt|decrypt to take bytes offset

 block/crypto.c | 119 +
 block/qcow.c   |   7 ++-
 block/qcow2-cluster.c  |   8 ++--
 block/qcow2.c  |   4 +-
 crypto/block-luks.c|  12 +++--
 crypto/block-qcow.c|  12 +++--
 crypto/block.c |  14 +++---
 crypto/blockpriv.h |   4 +-
 include/crypto/block.h |  14 +++---
 9 files changed, 104 insertions(+), 90 deletions(-)

-- 
2.13.5

[Qemu-devel] [PATCH v2 3/4] block: convert crypto driver to bdrv_co_preadv|pwritev

[Daniel P. Berrange]

Make the crypto driver implement the bdrv_co_preadv|pwritev
callbacks,  and also use bdrv_co_preadv|pwritev for I/O
with the protocol driver beneath.

Signed-off-by: Daniel P. Berrange 
---
 block/crypto.c | 103 +
 1 file changed, 53 insertions(+), 50 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index e63f094379..40daa77188 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -380,19 +380,23 @@ static void block_crypto_close(BlockDriverState *bs)
 
 
 #define BLOCK_CRYPTO_MAX_SECTORS 2048
+#define BLOCK_CRYPTO_MAX_LENGTH (BLOCK_CRYPTO_MAX_SECTORS * BDRV_SECTOR_SIZE)
 
 static coroutine_fn int
-block_crypto_co_readv(BlockDriverState *bs, int64_t sector_num,
-  int remaining_sectors, QEMUIOVector *qiov)
+block_crypto_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+   QEMUIOVector *qiov, int flags)
 {
 BlockCrypto *crypto = bs->opaque;
-int cur_nr_sectors; /* number of sectors in current iteration */
+uint64_t cur_bytes; /* number of bytes in current iteration */
 uint64_t bytes_done = 0;
 uint8_t *cipher_data = NULL;
 QEMUIOVector hd_qiov;
 int ret = 0;
-size_t payload_offset =
-qcrypto_block_get_payload_offset(crypto->block) / BDRV_SECTOR_SIZE;
+size_t payload_offset = qcrypto_block_get_payload_offset(crypto->block);
+uint64_t sector_num = offset / BDRV_SECTOR_SIZE;
+
+assert((offset % BDRV_SECTOR_SIZE) == 0);
+assert((bytes % BDRV_SECTOR_SIZE) == 0);
 
 qemu_iovec_init(&hd_qiov, qiov->niov);
 
@@ -400,44 +404,39 @@ block_crypto_co_readv(BlockDriverState *bs, int64_t 
sector_num,
  * in qiov which points to guest memory.
  */
 cipher_data = qemu_try_blockalign(
-bs->file->bs, MIN(BLOCK_CRYPTO_MAX_SECTORS * BDRV_SECTOR_SIZE,
-  qiov->size));
+bs->file->bs, MIN(BLOCK_CRYPTO_MAX_LENGTH, qiov->size));
 if (cipher_data == NULL) {
 ret = -ENOMEM;
 goto cleanup;
 }
 
-while (remaining_sectors) {
-cur_nr_sectors = remaining_sectors;
+while (bytes) {
+cur_bytes = bytes;
 
-if (cur_nr_sectors > BLOCK_CRYPTO_MAX_SECTORS) {
-cur_nr_sectors = BLOCK_CRYPTO_MAX_SECTORS;
+if (cur_bytes > BLOCK_CRYPTO_MAX_LENGTH) {
+cur_bytes = BLOCK_CRYPTO_MAX_LENGTH;
 }
 
 qemu_iovec_reset(&hd_qiov);
-qemu_iovec_add(&hd_qiov, cipher_data, cur_nr_sectors * 
BDRV_SECTOR_SIZE);
+qemu_iovec_add(&hd_qiov, cipher_data, cur_bytes);
 
-ret = bdrv_co_readv(bs->file,
-payload_offset + sector_num,
-cur_nr_sectors, &hd_qiov);
+ret = bdrv_co_preadv(bs->file, payload_offset + offset + bytes_done,
+ cur_bytes, &hd_qiov, 0);
 if (ret < 0) {
 goto cleanup;
 }
 
-if (qcrypto_block_decrypt(crypto->block,
-  sector_num,
-  cipher_data, cur_nr_sectors * 
BDRV_SECTOR_SIZE,
-  NULL) < 0) {
+if (qcrypto_block_decrypt(crypto->block, sector_num, cipher_data,
+  cur_bytes, NULL) < 0) {
 ret = -EIO;
 goto cleanup;
 }
 
-qemu_iovec_from_buf(qiov, bytes_done,
-cipher_data, cur_nr_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_from_buf(qiov, bytes_done, cipher_data, cur_bytes);
 
-remaining_sectors -= cur_nr_sectors;
-sector_num += cur_nr_sectors;
-bytes_done += cur_nr_sectors * BDRV_SECTOR_SIZE;
+sector_num += cur_bytes / BDRV_SECTOR_SIZE;
+bytes -= cur_bytes;
+bytes_done += cur_bytes;
 }
 
  cleanup:
@@ -449,17 +448,20 @@ block_crypto_co_readv(BlockDriverState *bs, int64_t 
sector_num,
 
 
 static coroutine_fn int
-block_crypto_co_writev(BlockDriverState *bs, int64_t sector_num,
-   int remaining_sectors, QEMUIOVector *qiov)
+block_crypto_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+QEMUIOVector *qiov, int flags)
 {
 BlockCrypto *crypto = bs->opaque;
-int cur_nr_sectors; /* number of sectors in current iteration */
+uint64_t cur_bytes; /* number of bytes in current iteration */
 uint64_t bytes_done = 0;
 uint8_t *cipher_data = NULL;
 QEMUIOVector hd_qiov;
 int ret = 0;
-size_t payload_offset =
-qcrypto_block_get_payload_offset(crypto->block) / BDRV_SECTOR_SIZE;
+size_t payload_offset = qcrypto_block_get_payload_offset(crypto->block);
+uint64_t sector_num = offset / BDRV_SECTOR_SIZE;
+
+assert((offset % BDRV_SECTOR_SIZE) == 0);
+assert((bytes % BDRV_SECTOR_SIZE) == 0);
 
 qemu_iovec_init(&hd_qiov, qiov->niov);
 
@@ -467,44 +469,39 @@ block_crypto_co_writev(BlockDriverState *bs, int64_t 
sector_num,
  *

[Qemu-devel] [PATCH v2 1/3] docker: Use unconfined security profile

[Fam Zheng]

Some by default blocked syscalls are required to run tests for example
userfaultfd.

Signed-off-by: Fam Zheng 
---
 tests/docker/Makefile.include | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index aaab1a4208..8ded838b20 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -145,6 +145,7 @@ docker-run: docker-qemu-src
$(call quiet-command,   \
$(SRC_PATH)/tests/docker/docker.py run  \
$(if $(NOUSER),,-u $(shell id -u)) -t   \
+   --security-opt seccomp=unconfined   \
$(if $V,,--rm)  \
$(if $(DEBUG),-i,)  \
$(if $(NETWORK),$(if $(subst 
$(NETWORK),,1),--net=$(NETWORK)),--net=none) \
-- 
2.13.5

[Qemu-devel] [PATCH v2 2/3] docker: Add nettle-devel to fedora image

[Fam Zheng]

The LUKS cases in qemu-iotests requires this.

Signed-off-by: Fam Zheng 
---
 tests/docker/dockerfiles/fedora.docker | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/docker/dockerfiles/fedora.docker 
b/tests/docker/dockerfiles/fedora.docker
index 4eaa8ed2a5..27e8201c54 100644
--- a/tests/docker/dockerfiles/fedora.docker
+++ b/tests/docker/dockerfiles/fedora.docker
@@ -3,6 +3,7 @@ ENV PACKAGES \
 ccache git tar PyYAML sparse flex bison python2 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 \
 mingw32-pixman mingw32-glib2 mingw32-gmp mingw32-SDL mingw32-pkg-config \
 mingw32-gtk2 mingw32-gtk3 mingw32-gnutls mingw32-nettle mingw32-libtasn1 \
 mingw32-libjpeg-turbo mingw32-libpng mingw32-curl mingw32-libssh2 \
-- 
2.13.5

[Qemu-devel] [PATCH v2 3/3] docker: Add test-block

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 tests/docker/test-block | 21 +
 1 file changed, 21 insertions(+)
 create mode 100755 tests/docker/test-block

diff --git a/tests/docker/test-block b/tests/docker/test-block
new file mode 100755
index 00..20ef70538f
--- /dev/null
+++ b/tests/docker/test-block
@@ -0,0 +1,21 @@
+#!/bin/bash -e
+#
+# Run block test cases
+#
+# Copyright (c) 2017 Red Hat Inc.
+#
+# Authors:
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2
+# or (at your option) any later version. See the COPYING file in
+# the top-level directory.
+
+. common.rc
+
+cd "$BUILD_DIR"
+
+build_qemu --target-list=x86_64-softmmu
+cd tests/qemu-iotests
+./check -raw
+./check -qcow2
-- 
2.13.5