Re: [Qemu-devel] [GIT PULL for qemu-pseries] spapr: Render full FDT on ibm, client-architecture-support

[David Gibson]

On Wed, Aug 28, 2019 at 01:27:35PM +1000, Alexey Kardashevskiy wrote:
> 
> 
> On 28/08/2019 12:12, David Gibson wrote:
> > On Wed, Aug 28, 2019 at 10:46:34AM +1000, David Gibson wrote:
> > > On Tue, Aug 27, 2019 at 04:56:50PM +1000, Alexey Kardashevskiy wrote:
> > > > The following changes since commit 
> > > > d6bb8b27204eaa58f1da948b65454e3a598ab2a4:
> > > > 
> > > >pseries: Update SLOF firmware image (2019-08-27 16:47:04 +1000)
> > > > 
> > > > are available in the Git repository at:
> > > > 
> > > >g...@github.com:aik/qemu.git tags/qemu-slof-20190827
> > > > 
> > > > for you to fetch changes up to da9960a5aaa25d27c9798c3d94c7b4c2d8af31ac:
> > > > 
> > > >spapr: Render full FDT on ibm,client-architecture-support 
> > > > (2019-08-27 16:47:46 +1000)
> > > > 
> > > > 
> > > > Alexey Kardashevskiy (1):
> > > >spapr: Render full FDT on ibm,client-architecture-support
> > > > 
> > > >   hw/ppc/spapr.c | 90 
> > > > +++---
> > > >   1 file changed, 10 insertions(+), 80 deletions(-)
> > > > 
> > > > 
> > > > *** Note: this is not for master, this is for pseries
> > > > 
> > > 
> > > Merged, thanks.
> > 
> > Urgh.  And the qemu change is now un-merged.  Alas, as soon as we had
> > a CAS reboot for XIVE the guest didn't boot on the second attempt.
> > Haven't had a chance to investigate yet.
> 
> QEMU command line, guest kernel version? I'd give it a try.

RHEL8.1 in guest and host, booting via GRUB into a XIVE capable
kernel.

I've now been able to investigate, dumping the dtb at H_UPDATE_DT
time.  Comparing before and after the fully-render-dt change, I get:

@@ -24,13 +24,13 @@
};
 
chosen {
-   bootargs = "BOOT_IMAGE=/vmlinuz-4.18.0-137.el8.ppc64le 
root=/dev/mapper/rhel-root ro crashkernel=auto rd.lvm.lv=rhel/root 
rd.lvm.lv=rhel/swap";
+   bootargs = [00];
bootpath = "/pci@8002000/scsi@0";
cpu = <0xe45ee80>;
ibm,arch-vec-5-platform-support = <0x17801880 0x19001a40>;
ibm,architecture-vec-5 = [19 00 20 00 00 80 05 00 00 00 00 00 
00 00 00 00 00 00 00 00 00 00 80 40 40 00 40];
-   linux,initrd-end = <0x00 0x588ed7d>;
-   linux,initrd-start = <0x00 0x3c0>;
+   linux,initrd-end = <0x00>;
+   linux,initrd-start = <0x00>;
linux,pci-probe-only = <0x00>;
linux,stdout-package = <0xe733a70>;
linux,stdout-path = "/vdevice/vty@7101";
@@ -252,7 +252,7 @@
used-by-rtas;
 
ethernet@1 {
-   assigned-addresses = <0x82000830 0x00 0x8004 0x00 
0x4 0x81000810 0x00 0x100 0x00 0x20 0x82000814 0x00 0x8001 0x00 0x1000 
0xc3000820 0x2100 0x1 0x00 0x4000>;
+   assigned-addresses;
cache-line-size = <0x00>;
class-code = <0x2>;
device-id = <0x1000>;
@@ -274,7 +274,7 @@
};
 
scsi@0 {
-   assigned-addresses = <0x8110 0x00 0x80 0x00 0x80 
0x8214 0x00 0x8000 0x00 0x1000 0xc320 0x2100 0x00 0x00 0x4000>;
+   assigned-addresses;
cache-line-size = <0x00>;
class-code = <0x1>;
device-id = <0x1001>;

So basically the bootargs, initrd params and assigned-addresses have
been lost.

For bootargs and initrd, I think what's happening is that because qemu
isn't setting those (with no -kernel parameter) SLOF is creating them,
but they're now being overwritten by qemu's post-CAS DT.

I'm not sure what's going on with assigned-addresses.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] libvhost-user: undefined reference to MADV_NOHUGEPAGE

[Sandra Loosemore]

On 8/27/19 9:01 PM, Richard Henderson wrote:

On 8/27/19 6:42 PM, Sandra Loosemore wrote:

Sorry if that was not clear.  The target is aarch64-none-elf with the provided
semihosting facilities in QEMU.  The host is x86_64-linux-gnu. We deliberately
link against a pretty old glibc sysroot (looks like version 2.11.1), but we did
that for last year's 3.0 release as well, and haven't made any other changes in
the configure options etc that we use to build QEMU for this target.


Still not clear.

The combination "glibc" and "qemu semihosting" doesn't make sense.  The triplet
"aarch64-none-elf" is a gcc thing and has no referent in qemu.

Are you building qemu-system-aarch64 for x86_64-linux, using an old x86_64 
sysroot?


Yes.  We only use this configuration of QEMU as an instruction-set 
simulator so that we can test cross-compilers and gdb for bare-metal 
aarch64-none-elf target, using newlib as the target C library and the 
GDB semihosting support in QEMU for low-level fileio primitives.


BTW, I did not run into this undefined-symbol error when building the 
equivalent configuration for bare-metal nios2-elf target with the same 
sysroot and host setup.  That target does not build libvhost-user at all.



In any case, glibc 2.11.1 is definitely out of support.  Even CentOS 6 used
2.12 and we don't support that anymore either.  Of the current
long-term-support distros, I believe the oldest version of glibc is CentOS 7
with 2.17.

As recently mentioned in

   https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg04514.html

we may accept a small patch with a large comment, but there are no guarantees
how long we will keep such workarounds.


I wouldn't mind just applying a local patch to fix the build.  What I'm 
really trying to do is just get help in understanding what broke this, 
so I can come up with such a patch to un-break it again.



I encourage you to re-examine why you're carrying around a 10 year old glibc.


We update our host build environment infrequently, and we've still had 
customers requiring CentOS 6 support until quite recently.


-Sandra

Re: [Qemu-devel] QEMU as ISS (Instruction Set Simulator)

[Libo Zhou]

Hi Aleksandar,


Thank you for the link to Loongson2F documentation. It has been very useful:)


I have spent several days immersing myself in the source code, now I think I 
have a more solid understanding about it. Just like Loongson Multimedia 
Instructions, I need to implement some sort of complex vector instructions, and 
I need to write some helper functions (e.g. my_helper.c).


The QEMU wiki website has very thorough explanation on TCG, but I haven't found 
any explanation on the port-specific helpers. Is there any documentation on how 
the helper functions are generated? I think now I *might* know how to write a 
working helper function, but I just don't know how it works.


Cheers,
Libo









-- Original message --
From: "Aleksandar Markovic"; 
Sendtime: Thursday, Aug 22, 2019 6:53 PM
To: "Libo Zhou"; 
Cc: "qemu-devel"; 
Subject: Re: [Qemu-devel] QEMU as ISS (Instruction Set Simulator)



On Thu, Aug 22, 2019 at 12:24 PM ??  wrote:

> Hi Aleksandar,
>
> Thank you very much for your patient explanation in the previous post. And
> thank you for checking.
> Your and Peter's replies in the previous post certainly helped a lot. I am
> now looking at a git commit 7 years ago (
> bd277fa1966bc400f1b411f868f39125cda0c403), it was a Loongson Multimedia
> Instruction implementation done my Richard Henderson.
>

Cool, that commit is a very good staring point - it is definitely not too
simple, and it is not too complex either. And you can discover several
different concepts in the process of exploring the change.

Documentation on instruction set extension related to the commit (found by
Google):
https://files.somniafabularum.com/loongson/docs/Loongson2FUserGuide.pdf

Be persistent, take your time, study the details and handling of individual
instructions, and, of course, let us know if you encounter some major
obstacles or thorny dilemmas.

Yours,
Aleksandar


> I think what he did is exactly what I want to do now. I got a vague view
> of the big picture, but I need more time to figure out the details. I will
> certainly ask more questions about this later, but before that I need to
> look at some other parts of the source code:) Again thank you for checking!
>
> Cheers,
> Libo
>
>
> -- Original message --
> *From:* "Aleksandar Markovic";
> *Sendtime:* Thursday, Aug 22, 2019 4:23 PM
> *To:* "??";
> *Cc:* "qemu-devel";
> *Subject:* Re: [Qemu-devel] QEMU as ISS (Instruction Set Simulator)
>
> On Tue, Aug 20, 2019 at 12:12 PM ??  wrote:
>
> > I am working on a project that requires me to modify the ISA of the MIPS
> > target.
>
>
> L.,
>
> How is it going?
>
> Aleksandar
>
>
>
> > I have been staring at the source code for about a week, but found it
> > really difficult due to me being a young rookie and the sparse comments.
> > Specifically, I need to extend MIPS, by adding some new instructions and
> > new CPU registers to the current architecture, and that sounds really
> easy.
> > I think the place for me to look at should be at the directory
> > ${qemu_root}/target/mips/. With a MIPS Instruction Set Manual Release 6
> > handy, I have difficulty finding the source code where the ISA resides.
> Is
> > it in op_helper.c? Or translate.c? Any guidance would be really
> > appreciated. Thank you very much in advance.
> >
> >
> > Cheers,
> > L.
>

Re: [Qemu-devel] [RFC 3/3] virt: Check KVM_CAP_ARM_IRQ_LINE_LAYOUT_2 for smp_cpus > 256

[Richard Henderson]

On 8/27/19 9:05 AM, Eric Auger wrote:
> +++ b/target/arm/kvm_arm.h
> @@ -233,6 +233,14 @@ bool kvm_arm_pmu_supported(CPUState *cs);
>   */
>  int kvm_arm_get_max_vm_ipa_size(MachineState *ms);
>  
> +/**
> + * kvm_arm_irq_line_layout_2 - Returns whether more than 256
> + * vcpus are supported by KVM_IRQ_LINE
> + *
> + * @ms: Machine state handle
> + */
> +bool kvm_arm_irq_line_layout_2(MachineState *ms);
> +
>  /**
>   * kvm_arm_sync_mpstate_to_kvm
>   * @cpu: ARMCPU
> @@ -280,6 +288,11 @@ static inline int 
> kvm_arm_get_max_vm_ipa_size(MachineState *ms)
>  return -ENOENT;
>  }
>  
> +static inline int kvm_arm_irq_line_layout_2(MachineState *ms)
> +{
> +return -ENOENT;
> +}
> +

These signatures don't match.


r~

Re: [Qemu-devel] [GIT PULL for qemu-pseries] spapr: Render full FDT on ibm, client-architecture-support

[Alexey Kardashevskiy]

On 28/08/2019 12:12, David Gibson wrote:

On Wed, Aug 28, 2019 at 10:46:34AM +1000, David Gibson wrote:

On Tue, Aug 27, 2019 at 04:56:50PM +1000, Alexey Kardashevskiy wrote:

The following changes since commit d6bb8b27204eaa58f1da948b65454e3a598ab2a4:

   pseries: Update SLOF firmware image (2019-08-27 16:47:04 +1000)

are available in the Git repository at:

   g...@github.com:aik/qemu.git tags/qemu-slof-20190827

for you to fetch changes up to da9960a5aaa25d27c9798c3d94c7b4c2d8af31ac:

   spapr: Render full FDT on ibm,client-architecture-support (2019-08-27 
16:47:46 +1000)


Alexey Kardashevskiy (1):
   spapr: Render full FDT on ibm,client-architecture-support

  hw/ppc/spapr.c | 90 +++---
  1 file changed, 10 insertions(+), 80 deletions(-)


*** Note: this is not for master, this is for pseries



Merged, thanks.


Urgh.  And the qemu change is now un-merged.  Alas, as soon as we had
a CAS reboot for XIVE the guest didn't boot on the second attempt.
Haven't had a chance to investigate yet.


QEMU command line, guest kernel version? I'd give it a try.



--
Alexey

Re: [Qemu-devel] [PATCH V3] gdbstub: Fix handler for 'F' packet

[Richard Henderson]

On 8/27/19 3:33 PM, Sandra Loosemore wrote:
> Handling of the 'F' packet has been broken since commit
> 4b20fab101b9e2d0fb47454209637a17fc7a13d5, which converted it to use
> the new packet parsing infrastructure.  Per the GDB RSP specification
> 
> https://sourceware.org/gdb/current/onlinedocs/gdb/The-F-Reply-Packet.html
> 
> the second parameter may be omitted, but the rewritten implementation
> was failing to recognize this case.  The result was that QEMU was
> repeatedly resending the fileio request and ignoring GDB's replies of
> successful completion.  This patch restores the behavior of the
> previous code in allowing the errno parameter to be omitted and
> passing 0 to the callback in that case.
> 
> Signed-off-by: Sandra Loosemore 
> ---
>  gdbstub.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH] target/arm: Free TCG temps in trans_VMOV_64_sp()

[Richard Henderson]

On 8/27/19 5:19 AM, Peter Maydell wrote:
> The function neon_store_reg32() doesn't free the TCG temp that it
> is passed, so the caller must do that. We got this right in most
> places but forgot to free the TCG temps in trans_VMOV_64_sp().
> 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/translate-vfp.inc.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] libvhost-user: undefined reference to MADV_NOHUGEPAGE

[Richard Henderson]

On 8/27/19 6:42 PM, Sandra Loosemore wrote:
> Sorry if that was not clear.  The target is aarch64-none-elf with the provided
> semihosting facilities in QEMU.  The host is x86_64-linux-gnu. We deliberately
> link against a pretty old glibc sysroot (looks like version 2.11.1), but we 
> did
> that for last year's 3.0 release as well, and haven't made any other changes 
> in
> the configure options etc that we use to build QEMU for this target.

Still not clear.

The combination "glibc" and "qemu semihosting" doesn't make sense.  The triplet
"aarch64-none-elf" is a gcc thing and has no referent in qemu.

Are you building qemu-system-aarch64 for x86_64-linux, using an old x86_64 
sysroot?

In any case, glibc 2.11.1 is definitely out of support.  Even CentOS 6 used
2.12 and we don't support that anymore either.  Of the current
long-term-support distros, I believe the oldest version of glibc is CentOS 7
with 2.17.

As recently mentioned in

  https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg04514.html

we may accept a small patch with a large comment, but there are no guarantees
how long we will keep such workarounds.

I encourage you to re-examine why you're carrying around a 10 year old glibc.


r~

Re: [Qemu-devel] [GIT PULL for qemu-pseries] spapr: Render full FDT on ibm, client-architecture-support

[David Gibson]

On Tue, Aug 27, 2019 at 04:56:50PM +1000, Alexey Kardashevskiy wrote:
> The following changes since commit d6bb8b27204eaa58f1da948b65454e3a598ab2a4:
> 
>   pseries: Update SLOF firmware image (2019-08-27 16:47:04 +1000)
> 
> are available in the Git repository at:
> 
>   g...@github.com:aik/qemu.git tags/qemu-slof-20190827
> 
> for you to fetch changes up to da9960a5aaa25d27c9798c3d94c7b4c2d8af31ac:
> 
>   spapr: Render full FDT on ibm,client-architecture-support (2019-08-27 
> 16:47:46 +1000)
> 
> 
> Alexey Kardashevskiy (1):
>   spapr: Render full FDT on ibm,client-architecture-support
> 
>  hw/ppc/spapr.c | 90 
> +++---
>  1 file changed, 10 insertions(+), 80 deletions(-)
> 
> 
> *** Note: this is not for master, this is for pseries
> 

Merged, thanks.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH qemu v3] spapr_pci: Advertise BAR reallocation capability

[David Gibson]

On Fri, Jul 19, 2019 at 02:37:34PM +1000, Alexey Kardashevskiy wrote:
> The pseries guests do not normally allocate PCI resources and rely on
> the system firmware doing so. Furthermore at least at some point in
> the past the pseries guests won't even allowed to change BARs, probably
> it is still the case for phyp. So since the initial commit we have [1]
> which prevents resource reallocation.
> 
> This is not a problem until we want specific BAR alignments, for example,
> PAGE_SIZE==64k to make sure we can still map MMIO BARs directly. For
> the boot time devices we handle this in SLOF [2] but since QEMU's RTAS
> does not allocate BARs, the guest does this instead and does not align
> BARs even if Linux is given pci=resource_alignment=16@pci:0:0 as
> PCI_PROBE_ONLY makes Linux ignore alignment requests.
> 
> ARM folks added a dial to control PCI_PROBE_ONLY via the device tree [3].
> This makes use of the dial to advertise to the guest that we can handle
> BAR reassignments. This limits the change to the latest pseries machine
> to avoid old guests explosion.
> 
> We do not remove the flag from [1] as pseries guests are still supported
> under phyp so having that removed may cause problems.
> 
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/powerpc/platforms/pseries/setup.c?h=v5.1#n773
> [2] 
> https://git.qemu.org/?p=SLOF.git;a=blob;f=board-qemu/slof/pci-phb.fs;h=06729bcf77a0d4e900c527adcd9befe2a269f65d;hb=HEAD#l338
> [3] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f81c11af
> Signed-off-by: Alexey Kardashevskiy 

Applied to ppc-for-4.2, thanks.

> ---
> Changes:
> v3:
> * moved the default setting to spapr_machine_class_init()
> 
> v2:
> * limited the change by a machine version
> ---
>  include/hw/ppc/spapr.h | 1 +
>  hw/ppc/spapr.c | 9 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index c79bc6a1232b..ebbd92673b34 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -121,6 +121,7 @@ struct SpaprMachineClass {
>  bool legacy_irq_allocation;
>  bool broken_host_serial_model; /* present real host info to the guest */
>  bool pre_4_1_migration; /* don't migrate hpt-max-page-size */
> +bool linux_pci_probe;
>  
>  void (*phb_placement)(SpaprMachineState *spapr, uint32_t index,
>uint64_t *buid, hwaddr *pio, 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 2ee671e55e0c..4bff0cf90d4b 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1164,6 +1164,7 @@ static void 
> spapr_dt_ov5_platform_support(SpaprMachineState *spapr, void *fdt,
>  static void spapr_dt_chosen(SpaprMachineState *spapr, void *fdt)
>  {
>  MachineState *machine = MACHINE(spapr);
> +SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
>  int chosen;
>  const char *boot_device = machine->boot_order;
>  char *stdout_path = spapr_vio_stdout_path(spapr->vio_bus);
> @@ -1221,6 +1222,11 @@ static void spapr_dt_chosen(SpaprMachineState *spapr, 
> void *fdt)
>  _FDT(fdt_setprop_string(fdt, chosen, "stdout-path", stdout_path));
>  }
>  
> +/* We can deal with BAR reallocation just fine, advertise it to the 
> guest */
> +if (smc->linux_pci_probe) {
> +_FDT(fdt_setprop_cell(fdt, chosen, "linux,pci-probe-only", 0));
> +}
> +
>  spapr_dt_ov5_platform_support(spapr, fdt, chosen);
>  
>  g_free(stdout_path);
> @@ -4467,6 +4473,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
> void *data)
>  spapr_caps_add_properties(smc, _abort);
>  smc->irq = _irq_dual;
>  smc->dr_phb_enabled = true;
> +smc->linux_pci_probe = true;
>  }
>  
>  static const TypeInfo spapr_machine_info = {
> @@ -4526,12 +4533,14 @@ DEFINE_SPAPR_MACHINE(4_2, "4.2", true);
>   */
>  static void spapr_machine_4_1_class_options(MachineClass *mc)
>  {
> +SpaprMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
>  static GlobalProperty compat[] = {
>  /* Only allow 4kiB and 64kiB IOMMU pagesizes */
>  { TYPE_SPAPR_PCI_HOST_BRIDGE, "pgsz", "0x11000" },
>  };
>  
>  spapr_machine_4_2_class_options(mc);
> +smc->linux_pci_probe = false;
>  compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));
>  }
>  

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [QEMU-PPC] [PATCH V4] powerpc/spapr: Add host threads parameter to ibm, get_system_parameter

[David Gibson]

On Tue, Aug 27, 2019 at 02:57:51PM +1000, Suraj Jitindar Singh wrote:
> The ibm,get_system_parameter rtas call is used by the guest to retrieve
> data relating to certain parameters of the system. The SPLPAR
> characteristics option (token 20) is used to determine characteristics of
> the environment in which the lpar will run.
> 
> It may be useful for a guest to know the number of physical host threads
> present on the underlying system where it is being run. Add the
> characteristic "HostThrs" to the SPLPAR Characteristics
> ibm,get_system_parameter rtas call to expose this information to a
> guest. Add a n_host_threads property to the processor class which is
> then used to retrieve this information and define it for POWER8 and
> POWER9. Other processors will default to 0 and the charateristic won't
> be added.
> 
> Signed-off-by: Suraj Jitindar Singh 

Applied, thanks.

> 
> ---
> 
> V1 -> V2:
> - Take into account that the core may be operating in split core mode
>   meaning a single core may be split into multiple subcores.
> V2 -> V3:
> - Add curly braces for single line if statements
> V3 -> V4;
> - Add a host threads property to the processor class and use this to
>   derive the information rather than the device tree.
> ---
>  hw/ppc/spapr_rtas.c | 15 +++
>  target/ppc/cpu-qom.h|  1 +
>  target/ppc/translate_init.inc.c |  2 ++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 526b489297..bee3835214 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -266,6 +266,7 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
>target_ulong args,
>uint32_t nret, target_ulong rets)
>  {
> +PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
>  MachineState *ms = MACHINE(qdev_get_machine());
>  unsigned int max_cpus = ms->smp.max_cpus;
>  target_ulong parameter = rtas_ld(args, 0);
> @@ -283,6 +284,20 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU 
> *cpu,
>current_machine->ram_size / MiB,
>ms->smp.cpus,
>max_cpus);
> +if (pcc->n_host_threads > 0) {
> +char *hostthr_val, *old = param_val;
> +
> +/*
> + * Add HostThrs property. This property is not present in PAPR 
> but
> + * is expected by some guests to communicate the number of 
> physical
> + * host threads per core on the system so that they can scale
> + * information which varies based on the thread configuration.
> + */
> +hostthr_val = g_strdup_printf(",HostThrs=%d", 
> pcc->n_host_threads);
> +param_val = g_strconcat(param_val, hostthr_val, NULL);
> +g_free(hostthr_val);
> +g_free(old);
> +}
>  ret = sysparm_st(buffer, length, param_val, strlen(param_val) + 1);
>  g_free(param_val);
>  break;
> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> index 7ffdb0a706..e499575dc8 100644
> --- a/target/ppc/cpu-qom.h
> +++ b/target/ppc/cpu-qom.h
> @@ -191,6 +191,7 @@ typedef struct PowerPCCPUClass {
>  const PPCHash64Options *hash64_opts;
>  struct ppc_radix_page_info *radix_page_info;
>  uint32_t lrg_decr_bits;
> +int n_host_threads;
>  void (*init_proc)(CPUPPCState *env);
>  int  (*check_pow)(CPUPPCState *env);
>  int (*handle_mmu_fault)(PowerPCCPU *cpu, vaddr eaddr, int rwx, int 
> mmu_idx);
> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> index 4a21ed7289..41f77b7ef8 100644
> --- a/target/ppc/translate_init.inc.c
> +++ b/target/ppc/translate_init.inc.c
> @@ -8770,6 +8770,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>  pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
>  pcc->hash64_opts = _hash64_opts_POWER7;
>  pcc->lrg_decr_bits = 32;
> +pcc->n_host_threads = 8;
>  #endif
>  pcc->excp_model = POWERPC_EXCP_POWER8;
>  pcc->bus_model = PPC_FLAGS_INPUT_POWER7;
> @@ -8981,6 +8982,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>  pcc->hash64_opts = _hash64_opts_POWER7;
>  pcc->radix_page_info = _radix_page_info;
>  pcc->lrg_decr_bits = 56;
> +pcc->n_host_threads = 4;
>  #endif
>  pcc->excp_model = POWERPC_EXCP_POWER9;
>  pcc->bus_model = PPC_FLAGS_INPUT_POWER9;

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [GIT PULL for qemu-pseries] spapr: Render full FDT on ibm, client-architecture-support

[David Gibson]

On Wed, Aug 28, 2019 at 10:46:34AM +1000, David Gibson wrote:
> On Tue, Aug 27, 2019 at 04:56:50PM +1000, Alexey Kardashevskiy wrote:
> > The following changes since commit d6bb8b27204eaa58f1da948b65454e3a598ab2a4:
> > 
> >   pseries: Update SLOF firmware image (2019-08-27 16:47:04 +1000)
> > 
> > are available in the Git repository at:
> > 
> >   g...@github.com:aik/qemu.git tags/qemu-slof-20190827
> > 
> > for you to fetch changes up to da9960a5aaa25d27c9798c3d94c7b4c2d8af31ac:
> > 
> >   spapr: Render full FDT on ibm,client-architecture-support (2019-08-27 
> > 16:47:46 +1000)
> > 
> > 
> > Alexey Kardashevskiy (1):
> >   spapr: Render full FDT on ibm,client-architecture-support
> > 
> >  hw/ppc/spapr.c | 90 
> > +++---
> >  1 file changed, 10 insertions(+), 80 deletions(-)
> > 
> > 
> > *** Note: this is not for master, this is for pseries
> > 
> 
> Merged, thanks.

Urgh.  And the qemu change is now un-merged.  Alas, as soon as we had
a CAS reboot for XIVE the guest didn't boot on the second attempt.
Haven't had a chance to investigate yet.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v6 3/3] target/ppc: Refactor emulation of vmrgew and vmrgow instructions

[David Gibson]

On Tue, Aug 27, 2019 at 12:19:27PM -0700, Richard Henderson wrote:
> On 8/27/19 2:37 AM, Stefan Brankovic wrote:
> > Since I found this two instructions implemented with tcg, I refactored
> > them so they are consistent with other similar implementations that
> > I introduced in this patch.
> > 
> > Also, a new dual macro GEN_VXFORM_TRANS_DUAL is added. This macro is
> > used if one instruction is realized with direct translation, and second
> > one with a helper.
> > 
> > Signed-off-by: Stefan Brankovic 
> > ---
> >  target/ppc/translate/vmx-impl.inc.c | 66 
> > +
> >  1 file changed, 37 insertions(+), 29 deletions(-)
> 
> Reviewed-by: Richard Henderson 

Applied to ppc-for-4.2, thanks.

> 
> 
> r~
> 

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] libvhost-user: undefined reference to MADV_NOHUGEPAGE

[Sandra Loosemore]

On 8/27/19 6:39 PM, Richard Henderson wrote:

On 8/27/19 5:26 PM, Sandra Loosemore wrote:

Yesterday I tried to build the recent 4.1 release system-mode QEMU for
aarch64-none-elf and ran into a compilation error in
./contrib/libvhost-user/libvhost-user.c.


Why are you attempting to compile qemu for a non-hosted (aka bare metal)
configuration?  That is certainly not a supported thing to do.  We require
POSIX (or Win32) at a minimum.

I can only presume that you're intending a hosted configuration, and using a
cross-compiler that doesn't actually match up.  What is the intended host (and
libc if this is for Linux, since there are at least 3)?


Sorry if that was not clear.  The target is aarch64-none-elf with the 
provided semihosting facilities in QEMU.  The host is x86_64-linux-gnu. 
We deliberately link against a pretty old glibc sysroot (looks like 
version 2.11.1), but we did that for last year's 3.0 release as well, 
and haven't made any other changes in the configure options etc that we 
use to build QEMU for this target.


-Sandra

Re: [Qemu-devel] [PATCH v4] riscv: hmp: Add a command to show virtual memory mappings

[Bin Meng]

Hi Palmer,

On Wed, Aug 28, 2019 at 7:18 AM Palmer Dabbelt  wrote:
>
> On Sun, 18 Aug 2019 22:59:54 PDT (-0700), bmeng...@gmail.com wrote:
> > On Wed, Aug 14, 2019 at 11:33 PM Bin Meng  wrote:
> >>
> >> This adds 'info mem' command for RISC-V, to show virtual memory
> >> mappings that aids debugging.
> >>
> >> Rather than showing every valid PTE, the command compacts the
> >> output by merging all contiguous physical address mappings into
> >> one block and only shows the merged block mapping details.
> >>
> >> Signed-off-by: Bin Meng 
> >> Acked-by: Dr. David Alan Gilbert 
> >> Reviewed-by: Palmer Dabbelt 
> >>
> >> ---
> >>
> >> Changes in v4:
> >> - restore to v2, that does not print all harts's PTE, since we
> >>   should switch to a cpu context via the 'cpu' command
> >>
> >> Changes in v3:
> >> - print PTEs for all harts instead of just current hart
> >>
> >> Changes in v2:
> >> - promote ppn to hwaddr when doing page table address calculation
> >>
> >>  hmp-commands-info.hx   |   2 +-
> >>  target/riscv/Makefile.objs |   4 +
> >>  target/riscv/monitor.c | 229 
> >> +
> >>  3 files changed, 234 insertions(+), 1 deletion(-)
> >>  create mode 100644 target/riscv/monitor.c
> >>
> >
> > Ping?
> >
> > What's the status of this patch?
>
> This is in my patch queue (for-master on github).  I'm still a bit backed up 
> on
> email, but when I get caught back up I'll send a PR.

I double checked your git repo, and found you applied an older version
of this patch.

Please drop that, and apply this v4 one.
http://patchwork.ozlabs.org/patch/1147104/

Regards,
Bin

Re: [Qemu-devel] [PATCH v9 05/11] numa: Extend CLI to provide initiator information for numa nodes

[Tao Xu]

On 8/27/2019 9:12 PM, Igor Mammedov wrote:

On Tue, 20 Aug 2019 16:34:44 +0800
Tao Xu  wrote:


On 8/16/2019 10:57 PM, Igor Mammedov wrote:

On Wed, 14 Aug 2019 19:31:27 -0700
Dan Williams  wrote:
   

On Wed, Aug 14, 2019 at 6:57 PM Tao Xu  wrote:


On 8/15/2019 5:29 AM, Dan Williams wrote:

On Tue, Aug 13, 2019 at 10:14 PM Tao Xu  wrote:


On 8/14/2019 10:39 AM, Dan Williams wrote:

On Tue, Aug 13, 2019 at 8:00 AM Igor Mammedov  wrote:


On Fri,  9 Aug 2019 14:57:25 +0800
Tao  wrote:
 

From: Tao Xu 
 

[...]

+for (i = 0; i < machine->numa_state->num_nodes; i++) {
+if (numa_info[i].initiator_valid &&
+!numa_info[numa_info[i].initiator].has_cpu) {

  ^^ possible out of bounds 
read, see bellow
 

+error_report("The initiator-id %"PRIu16 " of NUMA node %d"
+ " does not exist.", numa_info[i].initiator, i);
+error_printf("\n");
+
+exit(1);
+}

it takes care only about nodes that have cpus or memory-only ones that have
initiator explicitly provided on CLI. And leaves possibility to have
memory-only nodes without initiator mixed with nodes that have initiator.
Is it valid to have mixed configuration?
Should we forbid it?


The spec talks about the "Proximity Domain for the Attached Initiator"
field only being valid if the memory controller for the memory can be
identified by an initiator id in the SRAT. So I expect the only way to
define a memory proximity domain without this local initiator is to
allow specifying a node-id that does not have an entry in the SRAT.
 

Hi Dan,

So there may be a situation for the Attached Initiator field is not
valid? If true, I would allow user to input Initiator invalid.


Yes it's something the OS needs to consider because the platform may
not be able to meet the constraint that a single initiator is
associated with the memory controller for a given memory target. In
retrospect it would have been nice if the spec reserved 0x for
this purpose, but it seems "not in SRAT" is the only way to identify
memory that is not attached to any single initiator.
 

But As far as I konw, QEMU can't emulate a NUMA node "not in SRAT". I am
wondering if it is effective only set Initiator invalid?


You don't need to emulate a NUMA node not in SRAT. Just put a number
in this HMAT entry larger than the largest proximity domain number
found in the SRAT.
 
  


So behavior is really not defined in the spec
(well I wasn't able to convince myself that above behavior is in the spec).

In this case I'd go with a strict check for now not allowing invalid initiator
(we can easily relax check and allow it point to nonsense later but no other 
way around)
   


So let me summarize the solution, in order to avoid misunderstanding, if
there are something wrong, pls tell me:

1)
-machine,hmat=yes
-object memory-backend-ram,size=1G,id=m0 \
-object memory-backend-ram,size=1G,id=m1 \
-object memory-backend-ram,size=1G,id=m2 \
-numa node,nodeid=0,memdev=m0 \
-numa node,nodeid=1,memdev=m1,initiator=0 \
-numa node,nodeid=2,memdev=m2,initiator=0 \
-numa cpu,node-id=0,socket-id=0 \
-numa cpu,node-id=0,socket-id=1

then qemu can use HMAT.

2)
if initiator this case:

-numa node,nodeid=0,memdev=m0 \
-numa node,nodeid=1,memdev=m1,initiator=0 \
-numa node,nodeid=2,memdev=m2

then qemu can't boot and show error message.

3)
if initiator this case:

-numa node,nodeid=0,memdev=m0 \
-numa node,nodeid=1,memdev=m1,initiator=0 \
-numa node,nodeid=2,memdev=m2,initiator=1

then qemu can boot and the initiator of nodeid=2 is invalid.

In this last case I'd error out instead of booting with invalid config.


OK

Re: [Qemu-devel] libvhost-user: undefined reference to MADV_NOHUGEPAGE

[Richard Henderson]

On 8/27/19 5:26 PM, Sandra Loosemore wrote:
> Yesterday I tried to build the recent 4.1 release system-mode QEMU for
> aarch64-none-elf and ran into a compilation error in
> ./contrib/libvhost-user/libvhost-user.c.

Why are you attempting to compile qemu for a non-hosted (aka bare metal)
configuration?  That is certainly not a supported thing to do.  We require
POSIX (or Win32) at a minimum.

I can only presume that you're intending a hosted configuration, and using a
cross-compiler that doesn't actually match up.  What is the intended host (and
libc if this is for Linux, since there are at least 3)?

> It's complaining about
> MADV_NOHUGEPAGE not being defined.  I'm not familiar with that code or even
> what it does; should that bit of logic be made conditional on MADV_NOHUGEPAGE
> being defined, or is this indicative of a configuration error for this 
> target? 

I'm leaning toward the latter, but you're not giving much in the way of a hint.


r~

[Qemu-devel] libvhost-user: undefined reference to MADV_NOHUGEPAGE

[Sandra Loosemore]

Yesterday I tried to build the recent 4.1 release system-mode QEMU for 
aarch64-none-elf and ran into a compilation error in 
./contrib/libvhost-user/libvhost-user.c.  It's complaining about 
MADV_NOHUGEPAGE not being defined.  I'm not familiar with that code or 
even what it does; should that bit of logic be made conditional on 
MADV_NOHUGEPAGE being defined, or is this indicative of a configuration 
error for this target?  I see the unguarded reference was also present 
in the 3.0 release so it seems like whatever changed is somewhere else, 
and I don't know where to look for it or what I should be looking for. 
:-S  Does this sound familiar to anyone else?


-Sandra

Re: [Qemu-devel] RISC-V: Vector && DSP Extension

[Palmer Dabbelt]

On Thu, 22 Aug 2019 15:37:15 PDT (-0700), alistai...@gmail.com wrote:

On Wed, Aug 21, 2019 at 6:56 PM liuzhiwei  wrote:



On 2019/8/22 上午3:31, Palmer Dabbelt wrote:
> On Thu, 15 Aug 2019 14:37:52 PDT (-0700), alistai...@gmail.com wrote:
>> On Thu, Aug 15, 2019 at 2:07 AM Peter Maydell
>>  wrote:
>>>
>>> On Thu, 15 Aug 2019 at 09:53, Aleksandar Markovic
>>>  wrote:
>>> >
>>> > > We can accept draft
>>> > > extensions in QEMU as long as they are disabled by default.
>>>
>>> > Hi, Alistair, Palmer,
>>> >
>>> > Is this an official stance of QEMU community, or perhaps Alistair's
>>> > personal judgement, or maybe a rule within risv subcomunity?
>>>
>>> Alistair asked on a previous thread; my view was:
>>> https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg03364.html
>>> and nobody else spoke up disagreeing (summary: should at least be
>>> disabled-by-default and only enabled by setting an explicit
>>> property whose name should start with the 'x-' prefix).
>>
>> Agreed!
>>
>>>
>>> In general QEMU does sometimes introduce experimental extensions
>>> (we've had them in the block layer, for example) and so the 'x-'
>>> property to enable them is a reasonably established convention.
>>> I think it's a reasonable compromise to allow this sort of work
>>> to start and not have to live out-of-tree for a long time, without
>>> confusing users or getting into a situation where some QEMU
>>> versions behave differently or to obsolete drafts of a spec
>>> without it being clear from the command line that experimental
>>> extensions are being enabled.
>>>
>>> There is also an element of "submaintainer judgement" to be applied
>>> here -- upstream is probably not the place for a draft extension
>>> to be implemented if it is:
>>>  * still fast moving or subject to major changes of design direction
>>>  * major changes to the codebase (especially if it requires
>>>changes to core code) that might later need to be redone
>>>entirely differently
>>>  * still experimental
>>
>> Yep, agreed. For RISC-V I think this would extend to only allowing
>> extensions that have backing from the foundation and are under active
>> discussion.
>
> My general philosophy here is that we'll take anything written down in
> an official RISC-V ISA manual (ie, the ones actually released by the
> foundation).  This provides a single source of truth for what an
> extension name / version means, which is important to avoid
> confusion.  If it's a ratified extension then I see no reason not to
> support it on my end.  For frozen extensions we should probably just
> wait the 45 days until they go up for a ratification vote, but I'd be
> happy to start reviewing patches then (or earlier :)).
>
> If the spec is a draft in the ISA manual then we need to worry about
> the support burden, which I don't have a fixed criteria for --
> generally there shouldn't be issues here, but early drafts can be in a
> state where they're going to change extensively and are unlikely to be
> used by anyone.  There's also the question of "what is an official
> release of a draft specification?".
> That's a bit awkward right now: the current ratified ISA manual
> contains version 0.3 of the hypervisor extension, but I just talked to
> Andrew and the plan is to remove the draft extensions from the
> ratified manuals because these drafts are old and the official manuals
> update slowly.  For now I guess we'll need an an-hoc way of
> determining if a draft extension has been officially versioned or not,
> which is a bit of a headache.
>
> We already have examples of supporting draft extensions, including
> priv-1.9.1.  This does cause some pain for us on the QEMU side (CSR
> bits have different semantics between the specs), but there's 1.9.1
> hardware out there and the port continues to be useful so I'd be in
> favor of keeping it around for now.  I suppose there is an implicit
> risk that draft extensions will be deprecated, but the "x-" prefix,
> draft status, and long deprecation period should be sufficient to
> inform users of the risk.  I wouldn't be opposed to adding a "this is
> a draft ISA" warning, but I feel like it might be a bit overkill.
>
Hi, Palmer

Maybe it is the headache of open source hardware. Everyone cooperates to
build a better architecture.

In my opinion, we should focus on the future. The code in QEMU mainline
should evolve to the  ratified extension step by step, and only support
the best extension at last.

At that time,  even many hardwares just support  the deprecated draft
extension,  the draft codes should be in the wild and maintained by the
hardware manufactures.

But before that,  it is better to  have a draft implementation. So that
We can work step by step to accelerate the coming of the ratified
extension.

Even at last draft extension implementation are deprecated, they are not
meaningless. The manufactures may use  the  history commit to support
their hardwares that

only support drafted extension.


Overall I agree with 

Re: [Qemu-devel] [Qemu-riscv] [PATCH v1 02/28] target/riscv: Add the virtulisation mode

[Alistair Francis]

On Tue, Aug 27, 2019 at 8:44 AM Chih-Min Chao  wrote:
>
>
>
> On Sat, Aug 24, 2019 at 7:41 AM Alistair Francis  
> wrote:
>>
>> Signed-off-by: Alistair Francis 
>> ---
>>  target/riscv/cpu.h|  4 
>>  target/riscv/cpu_bits.h   |  6 ++
>>  target/riscv/cpu_helper.c | 23 +++
>>  3 files changed, 33 insertions(+)
>>
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index 7f54fb8c87..0ef1ecb0e0 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -117,6 +117,8 @@ struct CPURISCVState {
>>
>>  #ifndef CONFIG_USER_ONLY
>>  target_ulong priv;
>> +/* This contains QEMU specific information about the virt state. */
>> +target_ulong virt;
>>  target_ulong resetvec;
>>
>>  target_ulong mhartid;
>> @@ -257,6 +259,8 @@ int riscv_cpu_gdb_read_register(CPUState *cpu, uint8_t 
>> *buf, int reg);
>>  int riscv_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>>  bool riscv_cpu_exec_interrupt(CPUState *cs, int interrupt_request);
>>  bool riscv_cpu_fp_enabled(CPURISCVState *env);
>> +bool riscv_cpu_virt_enabled(CPURISCVState *env);
>> +void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
>>  int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch);
>>  hwaddr riscv_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
>>  void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>> index e99834856c..1fbde516be 100644
>> --- a/target/riscv/cpu_bits.h
>> +++ b/target/riscv/cpu_bits.h
>> @@ -422,6 +422,12 @@
>>  #define PRV_H 2 /* Reserved */
>>  #define PRV_M 3
>>
>> +/* Virtulisation modes */
>> +#define VIRT_OFF0
>> +#define VIRT_ON 1
>> +#define VIRT_MODE_SHIFT 0
>> +#define VIRT_MODE_MASK  (1 << VIRT_MODE_SHIFT)
>> +
>>
>>  /* RV32 satp CSR field masks */
>>  #define SATP32_MODE 0x8000
>>  #define SATP32_ASID 0x7fc0
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index 225e407cff..7b0bb14c01 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -81,6 +81,29 @@ bool riscv_cpu_fp_enabled(CPURISCVState *env)
>>  return false;
>>  }
>>
>> +bool riscv_cpu_virt_enabled(CPURISCVState *env)
>> +{
>> +bool tmp;
>> +
>> +if (!riscv_has_ext(env, RVH)) {
>> +return false;
>> +}
>> +
>> +tmp = (env->virt & VIRT_MODE_MASK) >> VIRT_MODE_SHIFT;
>> +
>> +return tmp == VIRT_ON;
>> +}
>> +
>> +void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable)
>> +{
>> +if (!riscv_has_ext(env, RVH)) {
>> +return;
>> +}
>> +
>> +env->virt &= ~VIRT_MODE_MASK;
>> +env->virt |= enable << VIRT_MODE_SHIFT;
>> +}
>> +
>>  int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint32_t interrupts)
>>  {
>>  CPURISCVState *env = >env;
>> --
>> 2.22.0
>>
> Why not to use get_field and set_field though it is not a real register but 
> an internal state
>
> #define VIRT_ONOFF0x01
> #define VIRT_OFF 0
> #define VIRT_ON 1
>
> access
>get_field(env->virt, VIRT_ONOFF);
>set_field(env->virt, VIRT_ONOFF, enable);

Good idea, I have updated this.

Alistair

>
>
> chihmin

Re: [Qemu-devel] [PATCH v2 66/68] target/arm: Convert T16, long branches

[Richard Henderson]

On 8/27/19 2:34 AM, Peter Maydell wrote:
>> +tcg_gen_andi_i32(tmp, tmp, -4);
> 
> Minor nit, but can we use 0xfffc like the old code did,
> to avoid the reader having to do 2s-complement arithmetic
> in their head to figure out that we're clearing the low 2 bits?

I always preferred "x & -c" for exactly the same reason:
to avoid the reader having to do 2s compliment arithmetic
in their head to figure out that we're aligning to c.

But, sure, if you like.

> This would be a good place to put a comment equivalent to that
> in the old decoder:
> 
> # thumb_insn_is_16bit() ensures we won't be decoding these as
> # T16 instructions for a Thumb2 CPU, so these patterns must be
> # a Thumb1 split BL/BLX.
> 
>> +BLX_suffix  11101 imm:11
>> +BL_BLX_prefix   0 imm:s11   
>> +BL_suffix   1 imm:11

I had placed that with trans_BL_BLX_prefix, but I suppose this
is a better place.


r~

Re: [Qemu-devel] [PATCH v2] riscv: rv32: Root page table address can be larger than 32-bit

[Palmer Dabbelt]

On Sun, 18 Aug 2019 23:00:40 PDT (-0700), bmeng...@gmail.com wrote:

On Wed, Aug 14, 2019 at 5:46 PM Bin Meng  wrote:


Hi Palmer,

On Sat, Aug 10, 2019 at 9:49 AM Alistair Francis  wrote:
>
> On Wed, Aug 7, 2019 at 7:50 PM Bin Meng  wrote:
> >
> > For RV32, the root page table's PPN has 22 bits hence its address
> > bits could be larger than the maximum bits that target_ulong is
> > able to represent. Use hwaddr instead.
> >
> > Signed-off-by: Bin Meng 
>
> Reviewed-by: Alistair Francis 
>

Would you take this one too?



Ping?

What's the status of this patch?


Also in the patch queue.

Re: [Qemu-devel] [PATCH v4] riscv: hmp: Add a command to show virtual memory mappings

[Palmer Dabbelt]

On Sun, 18 Aug 2019 22:59:54 PDT (-0700), bmeng...@gmail.com wrote:

On Wed, Aug 14, 2019 at 11:33 PM Bin Meng  wrote:


This adds 'info mem' command for RISC-V, to show virtual memory
mappings that aids debugging.

Rather than showing every valid PTE, the command compacts the
output by merging all contiguous physical address mappings into
one block and only shows the merged block mapping details.

Signed-off-by: Bin Meng 
Acked-by: Dr. David Alan Gilbert 
Reviewed-by: Palmer Dabbelt 

---

Changes in v4:
- restore to v2, that does not print all harts's PTE, since we
  should switch to a cpu context via the 'cpu' command

Changes in v3:
- print PTEs for all harts instead of just current hart

Changes in v2:
- promote ppn to hwaddr when doing page table address calculation

 hmp-commands-info.hx   |   2 +-
 target/riscv/Makefile.objs |   4 +
 target/riscv/monitor.c | 229 +
 3 files changed, 234 insertions(+), 1 deletion(-)
 create mode 100644 target/riscv/monitor.c



Ping?

What's the status of this patch?


This is in my patch queue (for-master on github).  I'm still a bit backed up on 
email, but when I get caught back up I'll send a PR.

Re: [Qemu-devel] [PATCH v2 4/4] audio: paaudio: ability to specify stream name

[Zoltán Kővágó]

On 2019-08-27 07:42, Gerd Hoffmann wrote:
> On Mon, Aug 26, 2019 at 09:59:04PM +0200, Kővágó, Zoltán wrote:
>> This can be used to identify stream in tools like pavucontrol when one
>> creates multiple -audiodevs or runs multiple qemu instances.
> 
> Hmm, can we create an useful name automatically, without yet another
> config option?
> 
> Useful choices could be the device name (usb-audio, ...) or the device
> id (whatever -device id=xxx was specified on the command line).

I'm afraid this is not going to work with the current architecture: due
to mixeng even if you have multiple devices, they'll be mixed to a
single stream and the audio backend will only see this one mixed stream.
 As a workaround we could do something like concat all device names or
ids, but I don't like that idea.

Alternatively we could use the id of the audiodev instead, and no more
problems with mixeng.  However, with mixeng off (implemented in my next
patch series) suddenly soundcards will have suddenly end up as different
streams.  (This can be worked around by creating multiple audiodevs,
like what you have to use now to get multiple streams from pa, so this
is probably a smaller problem.)

Currently I'm leaning for the audiodev's id option, unless someone
proposes something better.

Regards,
Zoltan

[Qemu-devel] [PATCH V3] gdbstub: Fix handler for 'F' packet

[Sandra Loosemore]

Handling of the 'F' packet has been broken since commit
4b20fab101b9e2d0fb47454209637a17fc7a13d5, which converted it to use
the new packet parsing infrastructure.  Per the GDB RSP specification

https://sourceware.org/gdb/current/onlinedocs/gdb/The-F-Reply-Packet.html

the second parameter may be omitted, but the rewritten implementation
was failing to recognize this case.  The result was that QEMU was
repeatedly resending the fileio request and ignoring GDB's replies of
successful completion.  This patch restores the behavior of the
previous code in allowing the errno parameter to be omitted and
passing 0 to the callback in that case.

Signed-off-by: Sandra Loosemore 
---
 gdbstub.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index b92ba59..3e8bcd0 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1820,11 +1820,15 @@ static void handle_read_all_regs(GdbCmdContext 
*gdb_ctx, void *user_ctx)
 
 static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
-if (gdb_ctx->num_params >= 2 && gdb_ctx->s->current_syscall_cb) {
+if (gdb_ctx->num_params >= 1 && gdb_ctx->s->current_syscall_cb) {
 target_ulong ret, err;
 
 ret = (target_ulong)gdb_ctx->params[0].val_ull;
-err = (target_ulong)gdb_ctx->params[1].val_ull;
+if (gdb_ctx->num_params >= 2) {
+err = (target_ulong)gdb_ctx->params[1].val_ull;
+} else {
+err = 0;
+}
 gdb_ctx->s->current_syscall_cb(gdb_ctx->s->c_cpu, ret, err);
 gdb_ctx->s->current_syscall_cb = NULL;
 }
-- 
2.8.1

Re: [Qemu-devel] [PATCH 2/2] block/nvme: add support for discard

[John Snow]

On 8/25/19 3:15 AM, Maxim Levitsky wrote:
> Signed-off-by: Maxim Levitsky 
> ---
>  block/nvme.c   | 83 ++
>  block/trace-events |  2 ++
>  2 files changed, 85 insertions(+)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index f8bd11e19a..dd041f39c9 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -112,6 +112,7 @@ typedef struct {
>  bool plugged;
>  
>  bool supports_write_zeros;
> +bool supports_discard;
>  
>  CoMutex dma_map_lock;
>  CoQueue dma_flush_queue;
> @@ -463,6 +464,7 @@ static void nvme_identify(BlockDriverState *bs, int 
> namespace, Error **errp)
>  
>  oncs = le16_to_cpu(idctrl->oncs);
>  s->supports_write_zeros = (oncs & NVME_ONCS_WRITE_ZEROS) != 0;
> +s->supports_discard = (oncs & NVME_ONCS_DSM) != 0;

Same comment -- checking !!(register & FIELD) is nicer than the
negative. (I'm actually not sure even the !! is needed, but it seems to
be a QEMU-ism and I've caught myself using it...)

Rest looks good to me on a skim, but I'm not very well-versed in NVME.

>  
>  memset(resp, 0, 4096);
>  
> @@ -1153,6 +1155,86 @@ static coroutine_fn int 
> nvme_co_pwrite_zeroes(BlockDriverState *bs,
>  }
>  
>  
> +static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs,
> + int64_t offset,
> + int bytes)
> +{
> +BDRVNVMeState *s = bs->opaque;
> +NVMeQueuePair *ioq = s->queues[1];
> +NVMeRequest *req;
> +NvmeDsmRange *buf;
> +QEMUIOVector local_qiov;
> +int ret;
> +
> +NvmeCmd cmd = {
> +.opcode = NVME_CMD_DSM,
> +.nsid = cpu_to_le32(s->nsid),
> +.cdw10 = cpu_to_le32(0), /*number of ranges - 0 based*/
> +.cdw11 = cpu_to_le32(1 << 2), /*deallocate bit*/
> +};
> +
> +NVMeCoData data = {
> +.ctx = bdrv_get_aio_context(bs),
> +.ret = -EINPROGRESS,
> +};
> +
> +if (!s->supports_discard) {
> +return -ENOTSUP;
> +}
> +
> +assert(s->nr_queues > 1);
> +
> +buf = qemu_try_blockalign0(bs, s->page_size);
> +if (!buf) {
> +return -ENOMEM;
> +}
> +
> +buf->nlb = cpu_to_le32(bytes >> s->blkshift);
> +buf->slba = cpu_to_le64(offset >> s->blkshift);
> +buf->cattr = 0;
> +
> +qemu_iovec_init(_qiov, 1);
> +qemu_iovec_add(_qiov, buf, 4096);
> +
> +req = nvme_get_free_req(ioq);
> +assert(req);
> +
> +qemu_co_mutex_lock(>dma_map_lock);
> +ret = nvme_cmd_map_qiov(bs, , req, _qiov);
> +qemu_co_mutex_unlock(>dma_map_lock);
> +
> +if (ret) {
> +req->busy = false;
> +goto out;
> +}
> +
> +trace_nvme_dsm(s, offset, bytes);
> +
> +nvme_submit_command(s, ioq, req, , nvme_rw_cb, );
> +
> +data.co = qemu_coroutine_self();
> +while (data.ret == -EINPROGRESS) {
> +qemu_coroutine_yield();
> +}
> +
> +qemu_co_mutex_lock(>dma_map_lock);
> +ret = nvme_cmd_unmap_qiov(bs, _qiov);
> +qemu_co_mutex_unlock(>dma_map_lock);
> +
> +if (ret) {
> +goto out;
> +}
> +
> +ret = data.ret;
> +trace_nvme_dsm_done(s, offset, bytes, ret);
> +out:
> +qemu_iovec_destroy(_qiov);
> +qemu_vfree(buf);
> +return ret;
> +
> +}
> +
> +
>  static int nvme_reopen_prepare(BDRVReopenState *reopen_state,
> BlockReopenQueue *queue, Error **errp)
>  {
> @@ -1259,6 +1341,7 @@ static BlockDriver bdrv_nvme = {
>  .bdrv_co_pwritev  = nvme_co_pwritev,
>  
>  .bdrv_co_pwrite_zeroes= nvme_co_pwrite_zeroes,
> +.bdrv_co_pdiscard = nvme_co_pdiscard,
>  
>  .bdrv_co_flush_to_disk= nvme_co_flush,
>  .bdrv_reopen_prepare  = nvme_reopen_prepare,
> diff --git a/block/trace-events b/block/trace-events
> index 8209fbd0c7..7d1d48b502 100644
> --- a/block/trace-events
> +++ b/block/trace-events
> @@ -153,6 +153,8 @@ nvme_write_zeros(void *s, uint64_t offset, uint64_t 
> bytes, int flags) "s %p offs
>  nvme_qiov_unaligned(const void *qiov, int n, void *base, size_t size, int 
> align) "qiov %p n %d base %p size 0x%zx align 0x%x"
>  nvme_prw_buffered(void *s, uint64_t offset, uint64_t bytes, int niov, int 
> is_write) "s %p offset %"PRId64" bytes %"PRId64" niov %d is_write %d"
>  nvme_rw_done(void *s, int is_write, uint64_t offset, uint64_t bytes, int 
> ret) "s %p is_write %d offset %"PRId64" bytes %"PRId64" ret %d"
> +nvme_dsm(void *s, uint64_t offset, uint64_t bytes) "s %p offset %"PRId64" 
> bytes %"PRId64""
> +nvme_dsm_done(void *s, uint64_t offset, uint64_t bytes, int ret) "s %p 
> offset %"PRId64" bytes %"PRId64" ret %d"
>  nvme_dma_map_flush(void *s) "s %p"
>  nvme_free_req_queue_wait(void *q) "q %p"
>  nvme_cmd_map_qiov(void *s, void *cmd, void *req, void *qiov, int entries) "s 
> %p cmd %p req %p qiov %p entries %d"
> 

Re: [Qemu-devel] [PATCH v2 18/68] target/arm: Convert the rest of A32 Miscelaneous instructions

[Richard Henderson]

On 8/27/19 1:01 PM, Richard Henderson wrote:
> Other constraints, such as rd != 15 or imod != 0, should continue to return
> false so that a (potential) grouped insn can match.

Eh.  This is not the answer that the TT example suggests.

So far we are able to order the grouped insns such that
decoding directives like

if t == 15 then SEE "TT";

are respected.  Since we do not generally do a very good
job of diagnosing all of the UNPREDICTABLE behavior, we
should not rely on getting all of it, e.g. by requiring
that if TT diagnoses some UNPRED that STREX also diagnoses
similar UNPRED.

I'm going to walk through the patch set and fix these.


r~

Re: [Qemu-devel] [PATCH V2] gdbstub: Fix handler for 'F' packet

[Sandra Loosemore]

On 8/27/19 3:30 PM, no-re...@patchew.org wrote:


=== OUTPUT BEGIN ===
ERROR: space prohibited before that close parenthesis ')'
#37: FILE: gdbstub.c:1827:
+if (gdb_ctx->num_params >= 2 ) {


Arggghh, I am sorry.  I fixed this and then screwed up and resent the 
old patch over again.  I'll try again.


-Sandra

Re: [Qemu-devel] [PATCH 1/2] block/nvme: add support for write zeros

[John Snow]

Without a commit message, I have no real hope of reviewing this. I was
CC'd, though, so I'll give it a blind shot.

We want to add write_zeroes support for block/nvme, but I can't really
verify any of that is correct or working without a unit test, a spec, or
some instructions to help me verify any of this does what it looks like
it does.

On 8/25/19 3:15 AM, Maxim Levitsky wrote:
> Signed-off-by: Maxim Levitsky 
> ---
>  block/nvme.c | 72 +++-
>  block/trace-events   |  1 +
>  include/block/nvme.h | 19 +++-
>  3 files changed, 90 insertions(+), 2 deletions(-)
> 
> diff --git a/block/nvme.c b/block/nvme.c
> index 5be3a39b63..f8bd11e19a 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -111,6 +111,8 @@ typedef struct {
>  uint64_t max_transfer;
>  bool plugged;
>  
> +bool supports_write_zeros;
> +

I suppose the spelling of "zeroes" is not as established as I thought it
was. Actually, what's worse is that the NVME writers apparently couldn't
decide what to name it themselves either:

"Write Zeroes" has 23 hits.
"Write Zeros" has just two, in Figure 114 for the Identify NS Data.

Oh, in QEMU we're not much better:

jhuston@probe (review) ~/s/qemu> git grep -i zeros | wc -l
265
jhuston@probe (review) ~/s/qemu> git grep -i zeroes | wc -l
747

I'm going to suggest that we use 'zeroes' as the spelling here, to match
the existing 'pwrite_zeroes', and then otherwise to match the NVME
spec's usual spelling.

>  CoMutex dma_map_lock;
>  CoQueue dma_flush_queue;
>  
> @@ -421,6 +423,7 @@ static void nvme_identify(BlockDriverState *bs, int 
> namespace, Error **errp)
>  NvmeIdNs *idns;
>  NvmeLBAF *lbaf;
>  uint8_t *resp;
> +uint16_t oncs;
>  int r;
>  uint64_t iova;
>  NvmeCmd cmd = {
> @@ -458,6 +461,9 @@ static void nvme_identify(BlockDriverState *bs, int 
> namespace, Error **errp)
>  s->max_transfer = MIN_NON_ZERO(s->max_transfer,
>s->page_size / sizeof(uint64_t) * s->page_size);
>  
> +oncs = le16_to_cpu(idctrl->oncs);
> +s->supports_write_zeros = (oncs & NVME_ONCS_WRITE_ZEROS) != 0;
> +

For other reviewers: oncs is "Optional NVM Command Support".

I think it's better to say `!!(oncs & NVME_ONCS_WRITE_ZEROES)` to remove
doubt over the width of the bitmask.

>  memset(resp, 0, 4096);
>  
>  cmd.cdw10 = 0;
> @@ -470,6 +476,12 @@ static void nvme_identify(BlockDriverState *bs, int 
> namespace, Error **errp)
>  s->nsze = le64_to_cpu(idns->nsze);
>  lbaf = >lbaf[NVME_ID_NS_FLBAS_INDEX(idns->flbas)];
>  
> +if (NVME_ID_NS_DLFEAT_WRITE_ZEROS(idns->dlfeat) &&
> +NVME_ID_NS_DLFEAT_READ_BEHAVIOR(idns->dlfeat) ==
> +NVME_ID_NS_DLFEAT_READ_BEHAVIOR_ZEROS) {
> +bs->supported_write_flags |= BDRV_REQ_MAY_UNMAP;
> +}
> +
>  if (lbaf->ms) {
>  error_setg(errp, "Namespaces with metadata are not yet supported");
>  goto out;
> @@ -764,6 +776,8 @@ static int nvme_file_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  int ret;
>  BDRVNVMeState *s = bs->opaque;
>  
> +bs->supported_write_flags = BDRV_REQ_FUA;
> +

Is this a related change?

>  opts = qemu_opts_create(_opts, NULL, 0, _abort);
>  qemu_opts_absorb_qdict(opts, options, _abort);
>  device = qemu_opt_get(opts, NVME_BLOCK_OPT_DEVICE);
> @@ -792,7 +806,6 @@ static int nvme_file_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  goto fail;
>  }
>  }
> -bs->supported_write_flags = BDRV_REQ_FUA;
>  return 0;
>  fail:
>  nvme_close(bs);
> @@ -1086,6 +1099,60 @@ static coroutine_fn int nvme_co_flush(BlockDriverState 
> *bs)
>  }
>  
>  
> +static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs,
> +  int64_t offset,
> +  int bytes,
> +  BdrvRequestFlags flags)
> +{
> +BDRVNVMeState *s = bs->opaque;
> +NVMeQueuePair *ioq = s->queues[1];

I think it'd be slick to name the queues, but that's not related to this
patch.

> +NVMeRequest *req;
> +
> +uint32_t cdw12 = ((bytes >> s->blkshift) - 1) & 0x;
> +
> +if (!s->supports_write_zeros) {
> +return -ENOTSUP;
> +}
> +
> +NvmeCmd cmd = {
> +.opcode = NVME_CMD_WRITE_ZEROS,
> +.nsid = cpu_to_le32(s->nsid),
> +.cdw10 = cpu_to_le32((offset >> s->blkshift) & 0x),
> +.cdw11 = cpu_to_le32(((offset >> s->blkshift) >> 32) & 0x),
> +};
> +
> +NVMeCoData data = {
> +.ctx = bdrv_get_aio_context(bs),
> +.ret = -EINPROGRESS,
> +};
> +
> +if (flags & BDRV_REQ_MAY_UNMAP) {
> +cdw12 |= (1 << 25);
> +}
> +
> +if (flags & BDRV_REQ_FUA) {
> +cdw12 |= (1 << 30);
> +}
> +
> +cmd.cdw12 = cpu_to_le32(cdw12);
> +
> +trace_nvme_write_zeros(s, offset, bytes, 

Re: [Qemu-devel] [PATCH] gdbstub: Send a reply to the vKill packet.

[Aleksandar Markovic]

14.02.2019. 19.27, "Sandra Loosemore"  је
написао/ла:
>
> On 2/14/19 10:48 AM, Peter Maydell wrote:
>>
>> On Tue, 12 Feb 2019 at 21:52, Sandra Loosemore 
wrote:
>>>
>>>
>>> Per the GDB remote protocol documentation
>>>
>>>
https://sourceware.org/gdb/current/onlinedocs/gdb/Packets.html#index-vKill-packet
>>>
>>> the debug stub is expected to send a reply to the 'vKill' packet.  At
>>> least some versions of GDB crash if the gdb stub simply exits without
>>> sending a reply.  This patch fixes QEMU's gdb stub to conform to the
>>> expected behavior.
>>>
>>> Note that QEMU's existing handling of the legacy 'k' packet is
>>> correct: in that case GDB does not expect a reply, and QEMU does not
>>> send one.
>>>
>>> Signed-off-by: Sandra Loosemore 
>>
>>
>> Thanks, applied to target-arm.next.
>>
>> As an aside, do you know if there is any kind of test suite for
>> the remote protocol that implementors of a debug stub can use to
>> check that they're conforming to it?
>
>
> Well, I discovered this problem by running the GDB testsuite (using QEMU
for nios2-elf target with the other target-specific patches I recently
posted).  I'm not sure if it's designed to exhaustively test the entire
remote protocol, but it does a pretty good job of covering user-visible GDB
features that depend on the remote target doing something reasonable, even
if it's just saying "Huh?  I don't know how to do that."  :-)
>

Debugging using gdb/qemu setups is fairly frequent use case and perhaps we
should have a test module for interoperability of gdb and qemu, and also
"make check-gdb".

Thanks for pinpointing and fixing bugs in this area, Sandra!

Aleksandar

> -Sandra
>

Re: [Qemu-devel] [PATCH V2] gdbstub: Fix handler for 'F' packet

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20190827211753.7936-1-san...@codesourcery.com/



Hi,

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

Message-id: 20190827211753.7936-1-san...@codesourcery.com
Type: series
Subject: [Qemu-devel] [PATCH V2] gdbstub: Fix handler for 'F' packet

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
ae35e0f gdbstub: Fix handler for 'F' packet

=== OUTPUT BEGIN ===
ERROR: space prohibited before that close parenthesis ')'
#37: FILE: gdbstub.c:1827:
+if (gdb_ctx->num_params >= 2 ) {

total: 1 errors, 0 warnings, 17 lines checked

Commit ae35e0f8b9b6 (gdbstub: Fix handler for 'F' packet) has style problems, 
please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190827211753.7936-1-san...@codesourcery.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[Qemu-devel] [PATCH V2] gdbstub: Fix handler for 'F' packet

[Sandra Loosemore]

Handling of the 'F' packet has been broken since commit
4b20fab101b9e2d0fb47454209637a17fc7a13d5, which converted it to use
the new packet parsing infrastructure.  Per the GDB RSP specification

https://sourceware.org/gdb/current/onlinedocs/gdb/The-F-Reply-Packet.html

the second parameter may be omitted, but the rewritten implementation
was failing to recognize this case.  The result was that QEMU was
repeatedly resending the fileio request and ignoring GDB's replies of
successful completion.  This patch restores the behavior of the
previous code in allowing the errno parameter to be omitted and
passing 0 to the callback in that case.

Signed-off-by: Sandra Loosemore 
---
 gdbstub.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index b92ba59..141568a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1820,11 +1820,15 @@ static void handle_read_all_regs(GdbCmdContext 
*gdb_ctx, void *user_ctx)
 
 static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
-if (gdb_ctx->num_params >= 2 && gdb_ctx->s->current_syscall_cb) {
+if (gdb_ctx->num_params >= 1 && gdb_ctx->s->current_syscall_cb) {
 target_ulong ret, err;
 
 ret = (target_ulong)gdb_ctx->params[0].val_ull;
-err = (target_ulong)gdb_ctx->params[1].val_ull;
+if (gdb_ctx->num_params >= 2 ) {
+err = (target_ulong)gdb_ctx->params[1].val_ull;
+} else {
+err = 0;
+}
 gdb_ctx->s->current_syscall_cb(gdb_ctx->s->c_cpu, ret, err);
 gdb_ctx->s->current_syscall_cb = NULL;
 }
-- 
2.8.1

Re: [Qemu-devel] [PATCH 0/2] block: Let blockdev-create return 0 on success

[John Snow]

On 8/23/19 2:47 PM, Max Reitz wrote:
> Jobs are expected to return 0 on success.  .bdrv_co_create() on the
> other hand is a block layer function, and as such returns a non-negative
> value on success.
> 
> blockdev_create_run() should translate between the two (patch 1).
> 
> Without patch 1, blockdev-create is likely to fail for VPC images.
> Hence patch 2.
> 
> 
> Max Reitz (2):
>   block: Let blockdev-create return 0 on success
>   iotests: Test blockdev-create for vpc
> 
>  block/create.c |   3 +-
>  tests/qemu-iotests/266 | 182 +
>  tests/qemu-iotests/266.out | 107 ++
>  tests/qemu-iotests/group   |   1 +
>  4 files changed, 292 insertions(+), 1 deletion(-)
>  create mode 100755 tests/qemu-iotests/266
>  create mode 100644 tests/qemu-iotests/266.out
> 

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH 2/2] iotests: Test blockdev-create for vpc

[John Snow]

On 8/23/19 2:47 PM, Max Reitz wrote:
> Signed-off-by: Max Reitz 
> ---
>  tests/qemu-iotests/266 | 182 +
>  tests/qemu-iotests/266.out | 107 ++
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 290 insertions(+)
>  create mode 100755 tests/qemu-iotests/266
>  create mode 100644 tests/qemu-iotests/266.out
> 
> diff --git a/tests/qemu-iotests/266 b/tests/qemu-iotests/266
> new file mode 100755
> index 00..19b7b29535
> --- /dev/null
> +++ b/tests/qemu-iotests/266
> @@ -0,0 +1,182 @@
> +#!/usr/bin/env python
> +#
> +# Test VPC and file image creation
> +#
> +# Copyright (C) 2019 Red Hat, 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 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see .
> +#
> +
> +import iotests
> +from iotests import imgfmt
> +
> +iotests.verify_image_format(supported_fmts=['vpc'])
> +iotests.verify_protocol(supported=['file'])
> +

Oh, I guess I haven't successfully lobbied for the inclusion of the
other thing yet. oh-kay.

> +def blockdev_create(vm, options):
> +result = vm.qmp_log('blockdev-create', job_id='job0', options=options,
> +filters=[iotests.filter_qmp_testfiles])
> +
> +if 'return' in result:
> +assert result['return'] == {}
> +vm.run_job('job0')
> +iotests.log("")
> +

Probably approaching the time when we want a standard for this in
iotests, but I'm not insisting today.

> +with iotests.FilePath('t.vpc') as disk_path, \
> + iotests.VM() as vm:
> +
> +#
> +# Successful image creation (defaults)
> +#
> +iotests.log("=== Successful image creation (defaults) ===")
> +iotests.log("")
> +
> +# 8 heads, 964 cyls/head, 17 secs/cyl
> +# (Close to 64 MB)
> +size = 8 * 964 * 17 * 512
> +
> +vm.launch()
> +blockdev_create(vm, { 'driver': 'file',
> +  'filename': disk_path,
> +  'size': 0 })
> +
> +vm.qmp_log('blockdev-add', driver='file', filename=disk_path,
> +   node_name='imgfile', filters=[iotests.filter_qmp_testfiles])
> +
> +blockdev_create(vm, { 'driver': imgfmt,
> +  'file': 'imgfile',
> +  'size': size })
> +vm.shutdown()
> +
> +iotests.img_info_log(disk_path)
> +
> +#
> +# Successful image creation (explicit defaults)
> +#
> +iotests.log("=== Successful image creation (explicit defaults) ===")
> +iotests.log("")
> +
> +# 16 heads, 964 cyls/head, 17 secs/cyl
> +# (Close to 128 MB)
> +size = 16 * 964 * 17 * 512
> +
> +vm.launch()
> +blockdev_create(vm, { 'driver': 'file',
> +  'filename': disk_path,
> +  'size': 0 })
> +
> +vm.qmp_log('blockdev-add', driver='file', filename=disk_path,
> +   node_name='imgfile', filters=[iotests.filter_qmp_testfiles])
> +
> +blockdev_create(vm, { 'driver': imgfmt,
> +  'file': 'imgfile',
> +  'size': size,
> +  'subformat': 'dynamic',
> +  'force-size': False })
> +vm.shutdown()
> +
> +iotests.img_info_log(disk_path)
> +
> +#
> +# Successful image creation (non-default options)
> +#
> +iotests.log("=== Successful image creation (non-default options) ===")
> +iotests.log("")
> +
> +# Not representable in CHS (fine with force-size=True)
> +size = 1048576
> +
> +vm.launch()
> +blockdev_create(vm, { 'driver': 'file',
> +  'filename': disk_path,
> +  'size': 0 })
> +
> +vm.qmp_log('blockdev-add', driver='file', filename=disk_path,
> +   node_name='imgfile', filters=[iotests.filter_qmp_testfiles])
> +
> +blockdev_create(vm, { 'driver': imgfmt,
> +  'file': 'imgfile',
> +  'size': size,
> +  'subformat': 'fixed',
> +  'force-size': True })
> +vm.shutdown()
> +
> +iotests.img_info_log(disk_path)
> +
> +#
> +# Size not representable in CHS with force-size=False
> +#
> +iotests.log("=== Size not representable in CHS ===")
> +iotests.log("")
> +
> +# Not representable in CHS (will not work with force-size=False)
> +size = 1048576
> +
> +vm.launch()
> +   

Re: [Qemu-devel] GSoC project: API Documentation Generation links and comments

[John Snow]

On 8/27/19 4:56 PM, Peter Maydell wrote:
> On Tue, 27 Aug 2019 at 21:52, John Snow  wrote:
>> - For theming, I'm a fan of the RTD theme, because I think it makes the
>> TOC tree stand out better and makes for nicer browsing than the default
>> alabaster theme. Maybe when there's a better over-arching TOC laid out
>> with better organization we could see which themes the list likes best.
> 
> FWIW, for the in-tree doc generation I opted for Alabaster
> (though I prefer RTD's look as well) because Alabaster doesn't
> require shipping any fonts, whereas RTD does, and it seemed
> easier to sidestep any questions about whether a font file in the
> docs was mere aggregation or not. For "putting docs up on the

What do you mean by "mere aggregation"?

> website" this doesn't apply so much, I think. Awkwardly, the
> two themes aren't completely drop-in replacements, though:
> I found that some of the tweaks needed for stuff like the
> sidebar to come out the way I wanted were theme-specific.
> 

That's... unfortunate. I've been playing around with RTD and Alabaster
for a hobby project, but hadn't noticed anything too different yet.

Of course, QEMU is gigantic and it is an edge-case magnet.

--js

Re: [Qemu-devel] [PATCH 0/1] Fix cacheline detection on FreeBSD/powerpc

[Justin Hibbits]

On Wed, 21 Aug 2019 10:25:45 +0200
Laurent Vivier  wrote:

> This is the patch originally sent by Justin, modified
> to change the parameter size on FreeBSD only.
> 
> Justin, could you review and test on FreeBSD?
> Peter, could you run "make check" on your MacOS X host?
> 
> Thanks,
> Laurent
> 
> Justin Hibbits (1):
>   Fix cacheline detection on FreeBSD/powerpc.
> 
>  util/cacheinfo.c | 23 +--
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 

Hi,

Sorry for the delay, I'll get to testing it tonight or tomorrow.  The
patch looks good from inspection.

- Justin

Re: [Qemu-devel] GSoC project: API Documentation Generation links and comments

[Peter Maydell]

On Tue, 27 Aug 2019 at 21:52, John Snow  wrote:
> - For theming, I'm a fan of the RTD theme, because I think it makes the
> TOC tree stand out better and makes for nicer browsing than the default
> alabaster theme. Maybe when there's a better over-arching TOC laid out
> with better organization we could see which themes the list likes best.

FWIW, for the in-tree doc generation I opted for Alabaster
(though I prefer RTD's look as well) because Alabaster doesn't
require shipping any fonts, whereas RTD does, and it seemed
easier to sidestep any questions about whether a font file in the
docs was mere aggregation or not. For "putting docs up on the
website" this doesn't apply so much, I think. Awkwardly, the
two themes aren't completely drop-in replacements, though:
I found that some of the tweaks needed for stuff like the
sidebar to come out the way I wanted were theme-specific.

thanks
-- PMM

Re: [Qemu-devel] GSoC project: API Documentation Generation links and comments

[John Snow]

On 8/26/19 2:51 PM, Gabriel Barreto wrote:
> I've uploaded to my github repository¹ the work done so far. Using
> Peter's patches as a starting point, we were able to generate
> kernel-docs documentation for some of QEMU's APIs. After studying the
> available options, we found a nice solution to publish the
> documentation online and keep it updated, using Github Pages and
> Travis CI. The idea is to use QEMU's Github mirror, updating the
> documentation (located on a gh-pages branch) with every push done to
> the master branch. I've implemented this and it's available at a
> Github Page² on a gh-pages branch managed by travis jobs. The default
> theme needs better structure, but a search in existing documentation
> is possible as an out-of-the-box feature. My work is not done yet, as
> I still need to rebase my commits to obtain a proper format for RFCs
> and figure out a better alternative to deal with the massive number of
> warnings that happen when generating the documentation. I'll keep
> working on it and welcome any feedback from you. I'm available to
> answer all questions you might have.
> 
> 
> [1] https://github.com/gsb16/qemu
> [2] https://gsb16.github.io/qemu/
> 
> 
> Kind Regards,
> Gabriel Barreto
> 

Ah, nice!


Some observations / questions:

- Having up-to-date documentation available on every push will be
convenient. I'm not sure if github pages will be the right home for
this, because I am not sure we are committed to maintaining a presence
on that page. ...I'd argue that it's better than not hosting our sphinx
documentation anywhere, ever, though.

- For theming, I'm a fan of the RTD theme, because I think it makes the
TOC tree stand out better and makes for nicer browsing than the default
alabaster theme. Maybe when there's a better over-arching TOC laid out
with better organization we could see which themes the list likes best.

- I imagine it's not too much work to get this to integrate with our
other existing manuals, too? (This looks like JUST kdoc generated
information, right?)

- The headers manual section is not organized into any logical
subsection or manual; it's useful to have for search, but probably isn't
very good reading as-is. We might want to filter these to be near the
features they're related to?

- https://gsb16.github.io/qemu/qapi/qjson.html and similar pages are
just simply empty. Either we want to populate them with stubs or just
omit empty pages, probably.

- https://gsb16.github.io/qemu/qom/cpu.html Do the build logs generate
errors like this in a visible way, or is this an invisible failure?




For now, what's your next steps? We need to get your code out of github
and onto the mailing list for review; do you have a todo list or any
issues you were hoping to tackle before you tried to send it out?

--js

[Qemu-devel] [PULL 1/1] trace: Clarify DTrace/SystemTap help message

[Stefan Hajnoczi]

From: Philippe Mathieu-Daudé 

Most tracing backends are implemented within QEMU, except the
DTrace/SystemTap backends.

One side effect is when running 'qemu -trace help', an incomplete
list of trace events is displayed when using the DTrace/SystemTap
backends.

This is partly due to trace events registered as modules with
trace_init(), and since the events are not used within QEMU,
the linker optimize and remove the unused modules (which is
OK in this particular case).
Currently only the events compiled in trace-root.o and in the
last trace.o member of libqemuutil.a are linked, resulting in
an incomplete list of events.

To avoid confusion, improve the help message, recommending to
use the proper systemtap script to display the events list.

Before:

  $ lm32-softmmu/qemu-system-lm32 -trace help 2>&1 | wc -l
  70

After:

  $ lm32-softmmu/qemu-system-lm32 -trace help
  Run 'qemu-trace-stap list qemu-system-lm32' to print a list
  of names of trace points with the DTrace/SystemTap backends.

  $ qemu-trace-stap list qemu-system-lm32 | wc -l
  1136

Signed-off-by: Philippe Mathieu-Daudé 
Message-id: 20190823142203.5210-1-phi...@redhat.com
Message-Id: <20190823142203.5210-1-phi...@redhat.com>
Signed-off-by: Stefan Hajnoczi 
---
 trace/control.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/trace/control.c b/trace/control.c
index 43fb7868db..d9cafc161b 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -165,6 +165,12 @@ void trace_list_events(void)
 while ((ev = trace_event_iter_next()) != NULL) {
 fprintf(stderr, "%s\n", trace_event_get_name(ev));
 }
+#ifdef CONFIG_TRACE_DTRACE
+fprintf(stderr, "This list of names of trace points may be incomplete "
+"when using the DTrace/SystemTap backends.\n"
+"Run 'qemu-trace-stap list %s' to print the full list.\n",
+error_get_progname());
+#endif
 }
 
 static void do_trace_enable_events(const char *line_buf)
-- 
2.21.0

[Qemu-devel] [PULL 0/1] Tracing patches

[Stefan Hajnoczi]

The following changes since commit dac03af5d5482ec7ee9c23db467bb7230b33c0d9:

  Merge remote-tracking branch 'remotes/rth/tags/pull-axp-20190825' into 
staging (2019-08-27 10:00:51 +0100)

are available in the Git repository at:

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

for you to fetch changes up to 9f591a5d95e1969969632ab44cf35e505c8ddc3b:

  trace: Clarify DTrace/SystemTap help message (2019-08-27 15:12:36 +0100)


Pull request



Philippe Mathieu-Daudé (1):
  trace: Clarify DTrace/SystemTap help message

 trace/control.c | 6 ++
 1 file changed, 6 insertions(+)

-- 
2.21.0

[Qemu-devel] [PULL 10/12] block/qcow2: refactor qcow2_co_preadv to use buffer-based io

[Stefan Hajnoczi]

From: Vladimir Sementsov-Ogievskiy 

Use buffer based io in encrypted case.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Acked-by: Stefan Hajnoczi 
Message-id: 20190604161514.262241-11-vsement...@virtuozzo.com
Message-Id: <20190604161514.262241-11-vsement...@virtuozzo.com>
Signed-off-by: Stefan Hajnoczi 
---
 block/qcow2.c | 28 
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 7c5a4859f7..b2b87d1a8d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2059,19 +2059,15 @@ static coroutine_fn int 
qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
 }
 
 assert(cur_bytes <= QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
-qemu_iovec_reset(_qiov);
-qemu_iovec_add(_qiov, cluster_data, cur_bytes);
-}
 
-BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
-ret = bdrv_co_preadv(s->data_file,
- cluster_offset + offset_in_cluster,
- cur_bytes, _qiov, 0);
-if (ret < 0) {
-goto fail;
-}
-if (bs->encrypted) {
-assert(s->crypto);
+BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
+ret = bdrv_co_pread(s->data_file,
+cluster_offset + offset_in_cluster,
+cur_bytes, cluster_data, 0);
+if (ret < 0) {
+goto fail;
+}
+
 assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
 assert((cur_bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
 if (qcow2_co_decrypt(bs, cluster_offset, offset,
@@ -2080,6 +2076,14 @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState 
*bs, uint64_t offset,
 goto fail;
 }
 qemu_iovec_from_buf(qiov, bytes_done, cluster_data, cur_bytes);
+} else {
+BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
+ret = bdrv_co_preadv(s->data_file,
+ cluster_offset + offset_in_cluster,
+ cur_bytes, _qiov, 0);
+if (ret < 0) {
+goto fail;
+}
 }
 break;
 
-- 
2.21.0

[Qemu-devel] [PULL 07/12] block/io: bdrv_aligned_preadv: use and support qiov_offset

[Stefan Hajnoczi]

From: Vladimir Sementsov-Ogievskiy 

Use and support new API in bdrv_co_do_copy_on_readv.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Acked-by: Stefan Hajnoczi 
Message-id: 20190604161514.262241-8-vsement...@virtuozzo.com
Message-Id: <20190604161514.262241-8-vsement...@virtuozzo.com>
Signed-off-by: Stefan Hajnoczi 
---
 block/io.c | 21 -
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/block/io.c b/block/io.c
index 4c7a7ac7b1..f191a3fa1e 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1376,7 +1376,7 @@ err:
  */
 static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child,
 BdrvTrackedRequest *req, int64_t offset, unsigned int bytes,
-int64_t align, QEMUIOVector *qiov, int flags)
+int64_t align, QEMUIOVector *qiov, size_t qiov_offset, int flags)
 {
 BlockDriverState *bs = child->bs;
 int64_t total_bytes, max_bytes;
@@ -1387,7 +1387,6 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild 
*child,
 assert(is_power_of_2(align));
 assert((offset & (align - 1)) == 0);
 assert((bytes & (align - 1)) == 0);
-assert(!qiov || bytes == qiov->size);
 assert((bs->open_flags & BDRV_O_NO_IO) == 0);
 max_transfer = QEMU_ALIGN_DOWN(MIN_NON_ZERO(bs->bl.max_transfer, INT_MAX),
align);
@@ -1425,7 +1424,8 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild 
*child,
 }
 
 if (!ret || pnum != bytes) {
-ret = bdrv_co_do_copy_on_readv(child, offset, bytes, qiov, 0, 
flags);
+ret = bdrv_co_do_copy_on_readv(child, offset, bytes,
+   qiov, qiov_offset, flags);
 goto out;
 } else if (flags & BDRV_REQ_PREFETCH) {
 goto out;
@@ -1441,7 +1441,7 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild 
*child,
 
 max_bytes = ROUND_UP(MAX(0, total_bytes - offset), align);
 if (bytes <= max_bytes && bytes <= max_transfer) {
-ret = bdrv_driver_preadv(bs, offset, bytes, qiov, 0, 0);
+ret = bdrv_driver_preadv(bs, offset, bytes, qiov, qiov_offset, 0);
 goto out;
 }
 
@@ -1449,17 +1449,12 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild 
*child,
 int num;
 
 if (max_bytes) {
-QEMUIOVector local_qiov;
-
 num = MIN(bytes_remaining, MIN(max_bytes, max_transfer));
 assert(num);
-qemu_iovec_init(_qiov, qiov->niov);
-qemu_iovec_concat(_qiov, qiov, bytes - bytes_remaining, num);
 
 ret = bdrv_driver_preadv(bs, offset + bytes - bytes_remaining,
- num, _qiov, 0, 0);
+ num, qiov, bytes - bytes_remaining, 0);
 max_bytes -= num;
-qemu_iovec_destroy(_qiov);
 } else {
 num = bytes_remaining;
 ret = qemu_iovec_memset(qiov, bytes - bytes_remaining, 0,
@@ -1561,7 +1556,7 @@ static int bdrv_padding_rmw_read(BdrvChild *child,
 bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
 }
 ret = bdrv_aligned_preadv(child, req, req->overlap_offset, bytes,
-  align, _qiov, 0);
+  align, _qiov, 0, 0);
 if (ret < 0) {
 return ret;
 }
@@ -1584,7 +1579,7 @@ static int bdrv_padding_rmw_read(BdrvChild *child,
 ret = bdrv_aligned_preadv(
 child, req,
 req->overlap_offset + req->overlap_bytes - align,
-align, align, _qiov, 0);
+align, align, _qiov, 0, 0);
 if (ret < 0) {
 return ret;
 }
@@ -1665,7 +1660,7 @@ int coroutine_fn bdrv_co_preadv(BdrvChild *child,
 tracked_request_begin(, bs, offset, bytes, BDRV_TRACKED_READ);
 ret = bdrv_aligned_preadv(child, , offset, bytes,
   bs->bl.request_alignment,
-  qiov, flags);
+  qiov, 0, flags);
 tracked_request_end();
 bdrv_dec_in_flight(bs);
 
-- 
2.21.0

[Qemu-devel] [PULL 06/12] block/io: bdrv_co_do_copy_on_readv: lazy allocation

[Stefan Hajnoczi]

From: Vladimir Sementsov-Ogievskiy 

Allocate bounce_buffer only if it is really needed. Also, sub-optimize
allocation size (why not?).

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Acked-by: Stefan Hajnoczi 
Message-id: 20190604161514.262241-7-vsement...@virtuozzo.com
Message-Id: <20190604161514.262241-7-vsement...@virtuozzo.com>
Signed-off-by: Stefan Hajnoczi 
---
 block/io.c | 21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/block/io.c b/block/io.c
index 5817bb9405..4c7a7ac7b1 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1236,7 +1236,7 @@ static int coroutine_fn 
bdrv_co_do_copy_on_readv(BdrvChild *child,
  * modifying the image file.  This is critical for zero-copy guest I/O
  * where anything might happen inside guest memory.
  */
-void *bounce_buffer;
+void *bounce_buffer = NULL;
 
 BlockDriver *drv = bs->drv;
 int64_t cluster_offset;
@@ -1271,14 +1271,6 @@ static int coroutine_fn 
bdrv_co_do_copy_on_readv(BdrvChild *child,
 trace_bdrv_co_do_copy_on_readv(bs, offset, bytes,
cluster_offset, cluster_bytes);
 
-bounce_buffer = qemu_try_blockalign(bs,
-MIN(MIN(max_transfer, cluster_bytes),
-MAX_BOUNCE_BUFFER));
-if (bounce_buffer == NULL) {
-ret = -ENOMEM;
-goto err;
-}
-
 while (cluster_bytes) {
 int64_t pnum;
 
@@ -1305,6 +1297,17 @@ static int coroutine_fn 
bdrv_co_do_copy_on_readv(BdrvChild *child,
 
 /* Must copy-on-read; use the bounce buffer */
 pnum = MIN(pnum, MAX_BOUNCE_BUFFER);
+if (!bounce_buffer) {
+int64_t max_we_need = MAX(pnum, cluster_bytes - pnum);
+int64_t max_allowed = MIN(max_transfer, MAX_BOUNCE_BUFFER);
+int64_t bounce_buffer_len = MIN(max_we_need, max_allowed);
+
+bounce_buffer = qemu_try_blockalign(bs, bounce_buffer_len);
+if (!bounce_buffer) {
+ret = -ENOMEM;
+goto err;
+}
+}
 qemu_iovec_init_buf(_qiov, bounce_buffer, pnum);
 
 ret = bdrv_driver_preadv(bs, cluster_offset, pnum,
-- 
2.21.0

[Qemu-devel] [PULL 12/12] block/qcow2: implement .bdrv_co_pwritev(_compressed)_part

[Stefan Hajnoczi]

From: Vladimir Sementsov-Ogievskiy 

Implement and use new interface to get rid of hd_qiov.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Acked-by: Stefan Hajnoczi 
Message-id: 20190604161514.262241-13-vsement...@virtuozzo.com
Message-Id: <20190604161514.262241-13-vsement...@virtuozzo.com>
Signed-off-by: Stefan Hajnoczi 
---
 block/qcow2.h |  1 +
 include/qemu/iov.h|  1 +
 block/qcow2-cluster.c |  9 ---
 block/qcow2.c | 60 +--
 util/iov.c| 10 
 5 files changed, 48 insertions(+), 33 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index fc1b0d3c1e..998bcdaef1 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -420,6 +420,7 @@ typedef struct QCowL2Meta
  * from @cow_start and @cow_end into one single write operation.
  */
 QEMUIOVector *data_qiov;
+size_t data_qiov_offset;
 
 /** Pointer to next L2Meta of the same write request */
 struct QCowL2Meta *next;
diff --git a/include/qemu/iov.h b/include/qemu/iov.h
index 29957c8a72..bffc151282 100644
--- a/include/qemu/iov.h
+++ b/include/qemu/iov.h
@@ -206,6 +206,7 @@ void qemu_iovec_init_extended(
 void *tail_buf, size_t tail_len);
 void qemu_iovec_init_slice(QEMUIOVector *qiov, QEMUIOVector *source,
size_t offset, size_t len);
+int qemu_iovec_subvec_niov(QEMUIOVector *qiov, size_t offset, size_t len);
 void qemu_iovec_add(QEMUIOVector *qiov, void *base, size_t len);
 void qemu_iovec_concat(QEMUIOVector *dst,
QEMUIOVector *src, size_t soffset, size_t sbytes);
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 0e4524d450..f09cc992af 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -829,7 +829,6 @@ static int perform_cow(BlockDriverState *bs, QCowL2Meta *m)
 assert(start->nb_bytes <= UINT_MAX - end->nb_bytes);
 assert(start->nb_bytes + end->nb_bytes <= UINT_MAX - data_bytes);
 assert(start->offset + start->nb_bytes <= end->offset);
-assert(!m->data_qiov || m->data_qiov->size == data_bytes);
 
 if ((start->nb_bytes == 0 && end->nb_bytes == 0) || m->skip_cow) {
 return 0;
@@ -861,7 +860,11 @@ static int perform_cow(BlockDriverState *bs, QCowL2Meta *m)
 /* The part of the buffer where the end region is located */
 end_buffer = start_buffer + buffer_size - end->nb_bytes;
 
-qemu_iovec_init(, 2 + (m->data_qiov ? m->data_qiov->niov : 0));
+qemu_iovec_init(, 2 + (m->data_qiov ?
+qemu_iovec_subvec_niov(m->data_qiov,
+   m->data_qiov_offset,
+   data_bytes)
+: 0));
 
 qemu_co_mutex_unlock(>lock);
 /* First we read the existing data from both COW regions. We
@@ -904,7 +907,7 @@ static int perform_cow(BlockDriverState *bs, QCowL2Meta *m)
 if (start->nb_bytes) {
 qemu_iovec_add(, start_buffer, start->nb_bytes);
 }
-qemu_iovec_concat(, m->data_qiov, 0, data_bytes);
+qemu_iovec_concat(, m->data_qiov, m->data_qiov_offset, 
data_bytes);
 if (end->nb_bytes) {
 qemu_iovec_add(, end_buffer, end->nb_bytes);
 }
diff --git a/block/qcow2.c b/block/qcow2.c
index ec1fff9dd1..0882ff6e92 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2103,7 +2103,8 @@ fail:
 /* Check if it's possible to merge a write request with the writing of
  * the data from the COW regions */
 static bool merge_cow(uint64_t offset, unsigned bytes,
-  QEMUIOVector *hd_qiov, QCowL2Meta *l2meta)
+  QEMUIOVector *qiov, size_t qiov_offset,
+  QCowL2Meta *l2meta)
 {
 QCowL2Meta *m;
 
@@ -2132,11 +2133,12 @@ static bool merge_cow(uint64_t offset, unsigned bytes,
 
 /* Make sure that adding both COW regions to the QEMUIOVector
  * does not exceed IOV_MAX */
-if (hd_qiov->niov > IOV_MAX - 2) {
+if (qemu_iovec_subvec_niov(qiov, qiov_offset, bytes) > IOV_MAX - 2) {
 continue;
 }
 
-m->data_qiov = hd_qiov;
+m->data_qiov = qiov;
+m->data_qiov_offset = qiov_offset;
 return true;
 }
 
@@ -2218,24 +2220,22 @@ static int handle_alloc_space(BlockDriverState *bs, 
QCowL2Meta *l2meta)
 return 0;
 }
 
-static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *qiov,
- int flags)
+static coroutine_fn int qcow2_co_pwritev_part(
+BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+QEMUIOVector *qiov, size_t qiov_offset, int flags)
 {
 BDRVQcow2State *s = bs->opaque;
 int offset_in_cluster;
 int ret;
 unsigned int cur_bytes; /* number of sectors in current iteration */
 uint64_t cluster_offset;
-QEMUIOVector hd_qiov;
+

[Qemu-devel] [PULL 04/12] block: define .*_part io handlers in BlockDriver

[Stefan Hajnoczi]

From: Vladimir Sementsov-Ogievskiy 

Add handlers supporting qiov_offset parameter:
bdrv_co_preadv_part
bdrv_co_pwritev_part
bdrv_co_pwritev_compressed_part
This is used to reduce need of defining local_qiovs and hd_qiovs in all
corners of block layer code. The following patches will increase usage
of this new API part by part.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Acked-by: Stefan Hajnoczi 
Message-id: 20190604161514.262241-5-vsement...@virtuozzo.com
Message-Id: <20190604161514.262241-5-vsement...@virtuozzo.com>
Signed-off-by: Stefan Hajnoczi 
---
 include/block/block_int.h | 15 ++
 block/backup.c|  2 +-
 block/io.c| 96 +++
 qemu-img.c|  4 +-
 4 files changed, 95 insertions(+), 22 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index ceec8c2f56..79a1fdb258 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -210,6 +210,9 @@ struct BlockDriver {
  */
 int coroutine_fn (*bdrv_co_preadv)(BlockDriverState *bs,
 uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
+int coroutine_fn (*bdrv_co_preadv_part)(BlockDriverState *bs,
+uint64_t offset, uint64_t bytes,
+QEMUIOVector *qiov, size_t qiov_offset, int flags);
 int coroutine_fn (*bdrv_co_writev)(BlockDriverState *bs,
 int64_t sector_num, int nb_sectors, QEMUIOVector *qiov, int flags);
 /**
@@ -229,6 +232,9 @@ struct BlockDriver {
  */
 int coroutine_fn (*bdrv_co_pwritev)(BlockDriverState *bs,
 uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags);
+int coroutine_fn (*bdrv_co_pwritev_part)(BlockDriverState *bs,
+uint64_t offset, uint64_t bytes,
+QEMUIOVector *qiov, size_t qiov_offset, int flags);
 
 /*
  * Efficiently zero a region of the disk image.  Typically an image format
@@ -339,6 +345,9 @@ struct BlockDriver {
 
 int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs,
 uint64_t offset, uint64_t bytes, QEMUIOVector *qiov);
+int coroutine_fn (*bdrv_co_pwritev_compressed_part)(BlockDriverState *bs,
+uint64_t offset, uint64_t bytes, QEMUIOVector *qiov,
+size_t qiov_offset);
 
 int (*bdrv_snapshot_create)(BlockDriverState *bs,
 QEMUSnapshotInfo *sn_info);
@@ -570,6 +579,12 @@ struct BlockDriver {
 const char *const *strong_runtime_opts;
 };
 
+static inline bool block_driver_can_compress(BlockDriver *drv)
+{
+return drv->bdrv_co_pwritev_compressed ||
+   drv->bdrv_co_pwritev_compressed_part;
+}
+
 typedef struct BlockLimits {
 /* Alignment requirement, in bytes, for offset/length of I/O
  * requests. Must be a power of 2 less than INT_MAX; defaults to
diff --git a/block/backup.c b/block/backup.c
index 2baf7bed65..03637aeb11 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -674,7 +674,7 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 return NULL;
 }
 
-if (compress && target->drv->bdrv_co_pwritev_compressed == NULL) {
+if (compress && !block_driver_can_compress(target->drv)) {
 error_setg(errp, "Compression is not supported for this drive %s",
bdrv_get_device_name(target));
 return NULL;
diff --git a/block/io.c b/block/io.c
index 04e69400d8..fd2fc7d5ff 100644
--- a/block/io.c
+++ b/block/io.c
@@ -146,7 +146,8 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)
 
 /* Default alignment based on whether driver has byte interface */
 bs->bl.request_alignment = (drv->bdrv_co_preadv ||
-drv->bdrv_aio_preadv) ? 1 : 512;
+drv->bdrv_aio_preadv ||
+drv->bdrv_co_preadv_part) ? 1 : 512;
 
 /* Take some limits from the children as a default */
 if (bs->file) {
@@ -1044,11 +1045,14 @@ static void bdrv_co_io_em_complete(void *opaque, int 
ret)
 
 static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
uint64_t offset, uint64_t bytes,
-   QEMUIOVector *qiov, int flags)
+   QEMUIOVector *qiov,
+   size_t qiov_offset, int flags)
 {
 BlockDriver *drv = bs->drv;
 int64_t sector_num;
 unsigned int nb_sectors;
+QEMUIOVector local_qiov;
+int ret;
 
 assert(!(flags & ~BDRV_REQ_MASK));
 assert(!(flags & BDRV_REQ_NO_FALLBACK));
@@ -1057,8 +1061,19 @@ static int coroutine_fn 
bdrv_driver_preadv(BlockDriverState *bs,
 return -ENOMEDIUM;
 }
 
+if (drv->bdrv_co_preadv_part) {
+return drv->bdrv_co_preadv_part(bs, offset, bytes, qiov, qiov_offset,
+flags);
+}
+
+if (qiov_offset > 0 || bytes != qiov->size) {
+

[Qemu-devel] [PULL 11/12] block/qcow2: implement .bdrv_co_preadv_part

[Stefan Hajnoczi]

From: Vladimir Sementsov-Ogievskiy 

Implement and use new interface to get rid of hd_qiov.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Acked-by: Stefan Hajnoczi 
Message-id: 20190604161514.262241-12-vsement...@virtuozzo.com
Message-Id: <20190604161514.262241-12-vsement...@virtuozzo.com>
Signed-off-by: Stefan Hajnoczi 
---
 block/qcow2-cluster.c |  5 +++--
 block/qcow2.c | 49 +++
 2 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index cc5609e27a..0e4524d450 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -452,8 +452,9 @@ static int coroutine_fn 
do_perform_cow_read(BlockDriverState *bs,
  * interface.  This avoids double I/O throttling and request tracking,
  * which can lead to deadlock when block layer copy-on-read is enabled.
  */
-ret = bs->drv->bdrv_co_preadv(bs, src_cluster_offset + offset_in_cluster,
-  qiov->size, qiov, 0);
+ret = bs->drv->bdrv_co_preadv_part(bs,
+   src_cluster_offset + offset_in_cluster,
+   qiov->size, qiov, 0, 0);
 if (ret < 0) {
 return ret;
 }
diff --git a/block/qcow2.c b/block/qcow2.c
index b2b87d1a8d..ec1fff9dd1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -76,7 +76,8 @@ qcow2_co_preadv_compressed(BlockDriverState *bs,
uint64_t file_cluster_offset,
uint64_t offset,
uint64_t bytes,
-   QEMUIOVector *qiov);
+   QEMUIOVector *qiov,
+   size_t qiov_offset);
 
 static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
 {
@@ -1967,21 +1968,18 @@ out:
 return ret;
 }
 
-static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
-uint64_t bytes, QEMUIOVector *qiov,
-int flags)
+static coroutine_fn int qcow2_co_preadv_part(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov,
+ size_t qiov_offset, int flags)
 {
 BDRVQcow2State *s = bs->opaque;
 int offset_in_cluster;
 int ret;
 unsigned int cur_bytes; /* number of bytes in current iteration */
 uint64_t cluster_offset = 0;
-uint64_t bytes_done = 0;
-QEMUIOVector hd_qiov;
 uint8_t *cluster_data = NULL;
 
-qemu_iovec_init(_qiov, qiov->niov);
-
 while (bytes != 0) {
 
 /* prepare next request */
@@ -2000,34 +1998,31 @@ static coroutine_fn int 
qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
 
 offset_in_cluster = offset_into_cluster(s, offset);
 
-qemu_iovec_reset(_qiov);
-qemu_iovec_concat(_qiov, qiov, bytes_done, cur_bytes);
-
 switch (ret) {
 case QCOW2_CLUSTER_UNALLOCATED:
 
 if (bs->backing) {
 BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
-ret = bdrv_co_preadv(bs->backing, offset, cur_bytes,
- _qiov, 0);
+ret = bdrv_co_preadv_part(bs->backing, offset, cur_bytes,
+  qiov, qiov_offset, 0);
 if (ret < 0) {
 goto fail;
 }
 } else {
 /* Note: in this case, no need to wait */
-qemu_iovec_memset(_qiov, 0, 0, cur_bytes);
+qemu_iovec_memset(qiov, qiov_offset, 0, cur_bytes);
 }
 break;
 
 case QCOW2_CLUSTER_ZERO_PLAIN:
 case QCOW2_CLUSTER_ZERO_ALLOC:
-qemu_iovec_memset(_qiov, 0, 0, cur_bytes);
+qemu_iovec_memset(qiov, qiov_offset, 0, cur_bytes);
 break;
 
 case QCOW2_CLUSTER_COMPRESSED:
 ret = qcow2_co_preadv_compressed(bs, cluster_offset,
  offset, cur_bytes,
- _qiov);
+ qiov, qiov_offset);
 if (ret < 0) {
 goto fail;
 }
@@ -2075,12 +2070,12 @@ static coroutine_fn int 
qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
 ret = -EIO;
 goto fail;
 }
-qemu_iovec_from_buf(qiov, bytes_done, cluster_data, cur_bytes);
+qemu_iovec_from_buf(qiov, qiov_offset, cluster_data, 
cur_bytes);
 } else {
 BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
-ret = bdrv_co_preadv(s->data_file,
- cluster_offset + offset_in_cluster,
- cur_bytes, _qiov, 0);
+ 

Re: [Qemu-devel] [PATCH] gdbstub: Fix handler for 'F' packet

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20190827194849.7076-1-san...@codesourcery.com/



Hi,

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

Message-id: 20190827194849.7076-1-san...@codesourcery.com
Type: series
Subject: [Qemu-devel] [PATCH] gdbstub: Fix handler for 'F' packet

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
0744b4b gdbstub: Fix handler for 'F' packet

=== OUTPUT BEGIN ===
ERROR: space prohibited before that close parenthesis ')'
#37: FILE: gdbstub.c:1827:
+if (gdb_ctx->num_params >= 2 ) {

total: 1 errors, 0 warnings, 17 lines checked

Commit 0744b4b4b06b (gdbstub: Fix handler for 'F' packet) has style problems, 
please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190827194849.7076-1-san...@codesourcery.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[Qemu-devel] [PULL 09/12] block/io: introduce bdrv_co_p{read, write}v_part

[Stefan Hajnoczi]

From: Vladimir Sementsov-Ogievskiy 

Introduce extended variants of bdrv_co_preadv and bdrv_co_pwritev
with qiov_offset parameter.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Acked-by: Stefan Hajnoczi 
Message-id: 20190604161514.262241-10-vsement...@virtuozzo.com
Message-Id: <20190604161514.262241-10-vsement...@virtuozzo.com>
Signed-off-by: Stefan Hajnoczi 
---
 include/block/block_int.h |  6 ++
 block/io.c| 29 +++--
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 79a1fdb258..0422acdf1c 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -959,9 +959,15 @@ extern BlockDriver bdrv_qcow2;
 int coroutine_fn bdrv_co_preadv(BdrvChild *child,
 int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
 BdrvRequestFlags flags);
+int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
+int64_t offset, unsigned int bytes,
+QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags);
 int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
 int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
 BdrvRequestFlags flags);
+int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
+int64_t offset, unsigned int bytes,
+QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags);
 
 static inline int coroutine_fn bdrv_co_pread(BdrvChild *child,
 int64_t offset, unsigned int bytes, void *buf, BdrvRequestFlags flags)
diff --git a/block/io.c b/block/io.c
index 237d7f40f5..0fa10831ed 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1614,7 +1614,8 @@ static void bdrv_padding_destroy(BdrvRequestPadding *pad)
  *
  * Function always succeeds.
  */
-static bool bdrv_pad_request(BlockDriverState *bs, QEMUIOVector **qiov,
+static bool bdrv_pad_request(BlockDriverState *bs,
+ QEMUIOVector **qiov, size_t *qiov_offset,
  int64_t *offset, unsigned int *bytes,
  BdrvRequestPadding *pad)
 {
@@ -1623,11 +1624,12 @@ static bool bdrv_pad_request(BlockDriverState *bs, 
QEMUIOVector **qiov,
 }
 
 qemu_iovec_init_extended(>local_qiov, pad->buf, pad->head,
- *qiov, 0, *bytes,
+ *qiov, *qiov_offset, *bytes,
  pad->buf + pad->buf_len - pad->tail, pad->tail);
 *bytes += pad->head + pad->tail;
 *offset -= pad->head;
 *qiov = >local_qiov;
+*qiov_offset = 0;
 
 return true;
 }
@@ -1635,6 +1637,14 @@ static bool bdrv_pad_request(BlockDriverState *bs, 
QEMUIOVector **qiov,
 int coroutine_fn bdrv_co_preadv(BdrvChild *child,
 int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
 BdrvRequestFlags flags)
+{
+return bdrv_co_preadv_part(child, offset, bytes, qiov, 0, flags);
+}
+
+int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
+int64_t offset, unsigned int bytes,
+QEMUIOVector *qiov, size_t qiov_offset,
+BdrvRequestFlags flags)
 {
 BlockDriverState *bs = child->bs;
 BdrvTrackedRequest req;
@@ -1655,12 +1665,12 @@ int coroutine_fn bdrv_co_preadv(BdrvChild *child,
 flags |= BDRV_REQ_COPY_ON_READ;
 }
 
-bdrv_pad_request(bs, , , , );
+bdrv_pad_request(bs, , _offset, , , );
 
 tracked_request_begin(, bs, offset, bytes, BDRV_TRACKED_READ);
 ret = bdrv_aligned_preadv(child, , offset, bytes,
   bs->bl.request_alignment,
-  qiov, 0, flags);
+  qiov, qiov_offset, flags);
 tracked_request_end();
 bdrv_dec_in_flight(bs);
 
@@ -2023,6 +2033,13 @@ out:
 int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
 int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
 BdrvRequestFlags flags)
+{
+return bdrv_co_pwritev_part(child, offset, bytes, qiov, 0, flags);
+}
+
+int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
+int64_t offset, unsigned int bytes, QEMUIOVector *qiov, size_t qiov_offset,
+BdrvRequestFlags flags)
 {
 BlockDriverState *bs = child->bs;
 BdrvTrackedRequest req;
@@ -2054,14 +2071,14 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
 goto out;
 }
 
-if (bdrv_pad_request(bs, , , , )) {
+if (bdrv_pad_request(bs, , _offset, , , )) {
 mark_request_serialising(, align);
 wait_serialising_requests();
 bdrv_padding_rmw_read(child, , , false);
 }
 
 ret = bdrv_aligned_pwritev(child, , offset, bytes, align,
-   qiov, 0, flags);
+   qiov, qiov_offset, flags);
 
 bdrv_padding_destroy();
 
-- 
2.21.0

[Qemu-devel] [PULL 08/12] block/io: bdrv_aligned_pwritev: use and support qiov_offset

[Stefan Hajnoczi]

From: Vladimir Sementsov-Ogievskiy 

Use and support new API in bdrv_aligned_pwritev.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Acked-by: Stefan Hajnoczi 
Message-id: 20190604161514.262241-9-vsement...@virtuozzo.com
Message-Id: <20190604161514.262241-9-vsement...@virtuozzo.com>
Signed-off-by: Stefan Hajnoczi 
---
 block/io.c | 27 +--
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/block/io.c b/block/io.c
index f191a3fa1e..237d7f40f5 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1872,7 +1872,7 @@ bdrv_co_write_req_finish(BdrvChild *child, int64_t 
offset, uint64_t bytes,
  */
 static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
 BdrvTrackedRequest *req, int64_t offset, unsigned int bytes,
-int64_t align, QEMUIOVector *qiov, int flags)
+int64_t align, QEMUIOVector *qiov, size_t qiov_offset, int flags)
 {
 BlockDriverState *bs = child->bs;
 BlockDriver *drv = bs->drv;
@@ -1892,7 +1892,7 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild 
*child,
 assert(is_power_of_2(align));
 assert((offset & (align - 1)) == 0);
 assert((bytes & (align - 1)) == 0);
-assert(!qiov || bytes == qiov->size);
+assert(!qiov || qiov_offset + bytes <= qiov->size);
 max_transfer = QEMU_ALIGN_DOWN(MIN_NON_ZERO(bs->bl.max_transfer, INT_MAX),
align);
 
@@ -1900,7 +1900,7 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild 
*child,
 
 if (!ret && bs->detect_zeroes != BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF &&
 !(flags & BDRV_REQ_ZERO_WRITE) && drv->bdrv_co_pwrite_zeroes &&
-qemu_iovec_is_zero(qiov, 0, qiov->size)) {
+qemu_iovec_is_zero(qiov, qiov_offset, bytes)) {
 flags |= BDRV_REQ_ZERO_WRITE;
 if (bs->detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP) {
 flags |= BDRV_REQ_MAY_UNMAP;
@@ -1913,15 +1913,15 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild 
*child,
 bdrv_debug_event(bs, BLKDBG_PWRITEV_ZERO);
 ret = bdrv_co_do_pwrite_zeroes(bs, offset, bytes, flags);
 } else if (flags & BDRV_REQ_WRITE_COMPRESSED) {
-ret = bdrv_driver_pwritev_compressed(bs, offset, bytes, qiov, 0);
+ret = bdrv_driver_pwritev_compressed(bs, offset, bytes,
+ qiov, qiov_offset);
 } else if (bytes <= max_transfer) {
 bdrv_debug_event(bs, BLKDBG_PWRITEV);
-ret = bdrv_driver_pwritev(bs, offset, bytes, qiov, 0, flags);
+ret = bdrv_driver_pwritev(bs, offset, bytes, qiov, qiov_offset, flags);
 } else {
 bdrv_debug_event(bs, BLKDBG_PWRITEV);
 while (bytes_remaining) {
 int num = MIN(bytes_remaining, max_transfer);
-QEMUIOVector local_qiov;
 int local_flags = flags;
 
 assert(num);
@@ -1931,12 +1931,10 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild 
*child,
  * need to flush on the last iteration */
 local_flags &= ~BDRV_REQ_FUA;
 }
-qemu_iovec_init(_qiov, qiov->niov);
-qemu_iovec_concat(_qiov, qiov, bytes - bytes_remaining, num);
 
 ret = bdrv_driver_pwritev(bs, offset + bytes - bytes_remaining,
-  num, _qiov, 0, local_flags);
-qemu_iovec_destroy(_qiov);
+  num, qiov, bytes - bytes_remaining,
+  local_flags);
 if (ret < 0) {
 break;
 }
@@ -1979,7 +1977,7 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild 
*child,
 
 qemu_iovec_init_buf(_qiov, pad.buf, write_bytes);
 ret = bdrv_aligned_pwritev(child, req, aligned_offset, write_bytes,
-   align, _qiov,
+   align, _qiov, 0,
flags & ~BDRV_REQ_ZERO_WRITE);
 if (ret < 0 || pad.merge_reads) {
 /* Error or all work is done */
@@ -1995,7 +1993,7 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild 
*child,
 /* Write the aligned part in the middle. */
 uint64_t aligned_bytes = bytes & ~(align - 1);
 ret = bdrv_aligned_pwritev(child, req, offset, aligned_bytes, align,
-   NULL, flags);
+   NULL, 0, flags);
 if (ret < 0) {
 goto out;
 }
@@ -2009,7 +2007,8 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild 
*child,
 
 qemu_iovec_init_buf(_qiov, pad.tail_buf, align);
 ret = bdrv_aligned_pwritev(child, req, offset, align, align,
-   _qiov, flags & ~BDRV_REQ_ZERO_WRITE);
+   _qiov, 0,
+   flags & ~BDRV_REQ_ZERO_WRITE);
 }
 
 out:
@@ -2062,7 +2061,7 @@ int coroutine_fn 

[Qemu-devel] [PULL 03/12] block/io: refactor padding

[Stefan Hajnoczi]

From: Vladimir Sementsov-Ogievskiy 

We have similar padding code in bdrv_co_pwritev,
bdrv_co_do_pwrite_zeroes and bdrv_co_preadv. Let's combine and unify
it.

[Squashed in Vladimir's qemu-iotests 077 fix
--Stefan]

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Acked-by: Stefan Hajnoczi 
Message-id: 20190604161514.262241-4-vsement...@virtuozzo.com
Message-Id: <20190604161514.262241-4-vsement...@virtuozzo.com>
Signed-off-by: Stefan Hajnoczi 
---
 block/io.c | 365 +
 1 file changed, 200 insertions(+), 165 deletions(-)

diff --git a/block/io.c b/block/io.c
index f656fb2dce..04e69400d8 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1415,28 +1415,177 @@ out:
 }
 
 /*
- * Handle a read request in coroutine context
+ * Request padding
+ *
+ *  |< align ->| |<- align >|
+ *  |<- head ->|<- bytes ->|<-- tail -->|
+ *  |  |   | | ||
+ * -*--$---* ... *-$*---
+ *  |  |   | | ||
+ *  |  offset  | | end  |
+ *  ALIGN_DOWN(offset) ALIGN_UP(offset)  ALIGN_DOWN(end)   ALIGN_UP(end)
+ *  [buf   ... ) [tail_buf  )
+ *
+ * @buf is an aligned allocation needed to store @head and @tail paddings. 
@head
+ * is placed at the beginning of @buf and @tail at the @end.
+ *
+ * @tail_buf is a pointer to sub-buffer, corresponding to align-sized chunk
+ * around tail, if tail exists.
+ *
+ * @merge_reads is true for small requests,
+ * if @buf_len == @head + bytes + @tail. In this case it is possible that both
+ * head and tail exist but @buf_len == align and @tail_buf == @buf.
  */
+typedef struct BdrvRequestPadding {
+uint8_t *buf;
+size_t buf_len;
+uint8_t *tail_buf;
+size_t head;
+size_t tail;
+bool merge_reads;
+QEMUIOVector local_qiov;
+} BdrvRequestPadding;
+
+static bool bdrv_init_padding(BlockDriverState *bs,
+  int64_t offset, int64_t bytes,
+  BdrvRequestPadding *pad)
+{
+uint64_t align = bs->bl.request_alignment;
+size_t sum;
+
+memset(pad, 0, sizeof(*pad));
+
+pad->head = offset & (align - 1);
+pad->tail = ((offset + bytes) & (align - 1));
+if (pad->tail) {
+pad->tail = align - pad->tail;
+}
+
+if ((!pad->head && !pad->tail) || !bytes) {
+return false;
+}
+
+sum = pad->head + bytes + pad->tail;
+pad->buf_len = (sum > align && pad->head && pad->tail) ? 2 * align : align;
+pad->buf = qemu_blockalign(bs, pad->buf_len);
+pad->merge_reads = sum == pad->buf_len;
+if (pad->tail) {
+pad->tail_buf = pad->buf + pad->buf_len - align;
+}
+
+return true;
+}
+
+static int bdrv_padding_rmw_read(BdrvChild *child,
+ BdrvTrackedRequest *req,
+ BdrvRequestPadding *pad,
+ bool zero_middle)
+{
+QEMUIOVector local_qiov;
+BlockDriverState *bs = child->bs;
+uint64_t align = bs->bl.request_alignment;
+int ret;
+
+assert(req->serialising && pad->buf);
+
+if (pad->head || pad->merge_reads) {
+uint64_t bytes = pad->merge_reads ? pad->buf_len : align;
+
+qemu_iovec_init_buf(_qiov, pad->buf, bytes);
+
+if (pad->head) {
+bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD);
+}
+if (pad->merge_reads && pad->tail) {
+bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
+}
+ret = bdrv_aligned_preadv(child, req, req->overlap_offset, bytes,
+  align, _qiov, 0);
+if (ret < 0) {
+return ret;
+}
+if (pad->head) {
+bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD);
+}
+if (pad->merge_reads && pad->tail) {
+bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL);
+}
+
+if (pad->merge_reads) {
+goto zero_mem;
+}
+}
+
+if (pad->tail) {
+qemu_iovec_init_buf(_qiov, pad->tail_buf, align);
+
+bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL);
+ret = bdrv_aligned_preadv(
+child, req,
+req->overlap_offset + req->overlap_bytes - align,
+align, align, _qiov, 0);
+if (ret < 0) {
+return ret;
+}
+bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL);
+}
+
+zero_mem:
+if (zero_middle) {
+memset(pad->buf + pad->head, 0, pad->buf_len - pad->head - pad->tail);
+}
+
+return 0;
+}
+
+static void bdrv_padding_destroy(BdrvRequestPadding *pad)
+{
+if (pad->buf) {
+qemu_vfree(pad->buf);
+qemu_iovec_destroy(>local_qiov);
+}
+}
+
+/*
+ * bdrv_pad_request
+ *
+ * Exchange request parameters with padded request if 

[Qemu-devel] [PULL 05/12] block/io: bdrv_co_do_copy_on_readv: use and support qiov_offset

[Stefan Hajnoczi]

From: Vladimir Sementsov-Ogievskiy 

Use and support new API in bdrv_co_do_copy_on_readv. Note that in case
of allocated-in-top we need to shrink read size to MIN(..) by hand, as
pre-patch this was actually done implicitly by qemu_iovec_concat (and
we used local_qiov.size).

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Acked-by: Stefan Hajnoczi 
Message-id: 20190604161514.262241-6-vsement...@virtuozzo.com
Message-Id: <20190604161514.262241-6-vsement...@virtuozzo.com>
Signed-off-by: Stefan Hajnoczi 
---
 block/io.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/block/io.c b/block/io.c
index fd2fc7d5ff..5817bb9405 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1227,7 +1227,7 @@ bdrv_driver_pwritev_compressed(BlockDriverState *bs, 
uint64_t offset,
 
 static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
 int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
-int flags)
+size_t qiov_offset, int flags)
 {
 BlockDriverState *bs = child->bs;
 
@@ -1239,7 +1239,6 @@ static int coroutine_fn 
bdrv_co_do_copy_on_readv(BdrvChild *child,
 void *bounce_buffer;
 
 BlockDriver *drv = bs->drv;
-QEMUIOVector local_qiov;
 int64_t cluster_offset;
 int64_t cluster_bytes;
 size_t skip_bytes;
@@ -1302,6 +1301,8 @@ static int coroutine_fn 
bdrv_co_do_copy_on_readv(BdrvChild *child,
 assert(skip_bytes < pnum);
 
 if (ret <= 0) {
+QEMUIOVector local_qiov;
+
 /* Must copy-on-read; use the bounce buffer */
 pnum = MIN(pnum, MAX_BOUNCE_BUFFER);
 qemu_iovec_init_buf(_qiov, bounce_buffer, pnum);
@@ -1339,16 +1340,15 @@ static int coroutine_fn 
bdrv_co_do_copy_on_readv(BdrvChild *child,
 }
 
 if (!(flags & BDRV_REQ_PREFETCH)) {
-qemu_iovec_from_buf(qiov, progress, bounce_buffer + skip_bytes,
+qemu_iovec_from_buf(qiov, qiov_offset + progress,
+bounce_buffer + skip_bytes,
 pnum - skip_bytes);
 }
 } else if (!(flags & BDRV_REQ_PREFETCH)) {
 /* Read directly into the destination */
-qemu_iovec_init(_qiov, qiov->niov);
-qemu_iovec_concat(_qiov, qiov, progress, pnum - skip_bytes);
-ret = bdrv_driver_preadv(bs, offset + progress, local_qiov.size,
- _qiov, 0, 0);
-qemu_iovec_destroy(_qiov);
+ret = bdrv_driver_preadv(bs, offset + progress,
+ MIN(pnum - skip_bytes, bytes - progress),
+ qiov, qiov_offset + progress, 0);
 if (ret < 0) {
 goto err;
 }
@@ -1422,7 +1422,7 @@ static int coroutine_fn bdrv_aligned_preadv(BdrvChild 
*child,
 }
 
 if (!ret || pnum != bytes) {
-ret = bdrv_co_do_copy_on_readv(child, offset, bytes, qiov, flags);
+ret = bdrv_co_do_copy_on_readv(child, offset, bytes, qiov, 0, 
flags);
 goto out;
 } else if (flags & BDRV_REQ_PREFETCH) {
 goto out;
-- 
2.21.0

[Qemu-devel] [PULL 02/12] util/iov: improve qemu_iovec_is_zero

[Stefan Hajnoczi]

From: Vladimir Sementsov-Ogievskiy 

We'll need to check a part of qiov soon, so implement it now.

Optimization with align down to 4 * sizeof(long) is dropped due to:
1. It is strange: it aligns length of the buffer, but where is a
   guarantee that buffer pointer is aligned itself?
2. buffer_is_zero() is a better place for optimizations and it has
   them.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Acked-by: Stefan Hajnoczi 
Message-id: 20190604161514.262241-3-vsement...@virtuozzo.com
Message-Id: <20190604161514.262241-3-vsement...@virtuozzo.com>
Signed-off-by: Stefan Hajnoczi 
---
 include/qemu/iov.h |  2 +-
 block/io.c |  2 +-
 util/iov.c | 31 +++
 3 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/include/qemu/iov.h b/include/qemu/iov.h
index f3787a0cf7..29957c8a72 100644
--- a/include/qemu/iov.h
+++ b/include/qemu/iov.h
@@ -212,7 +212,7 @@ void qemu_iovec_concat(QEMUIOVector *dst,
 size_t qemu_iovec_concat_iov(QEMUIOVector *dst,
  struct iovec *src_iov, unsigned int src_cnt,
  size_t soffset, size_t sbytes);
-bool qemu_iovec_is_zero(QEMUIOVector *qiov);
+bool qemu_iovec_is_zero(QEMUIOVector *qiov, size_t qiov_offeset, size_t bytes);
 void qemu_iovec_destroy(QEMUIOVector *qiov);
 void qemu_iovec_reset(QEMUIOVector *qiov);
 size_t qemu_iovec_to_buf(QEMUIOVector *qiov, size_t offset,
diff --git a/block/io.c b/block/io.c
index 56bbf195bb..f656fb2dce 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1722,7 +1722,7 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild 
*child,
 
 if (!ret && bs->detect_zeroes != BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF &&
 !(flags & BDRV_REQ_ZERO_WRITE) && drv->bdrv_co_pwrite_zeroes &&
-qemu_iovec_is_zero(qiov)) {
+qemu_iovec_is_zero(qiov, 0, qiov->size)) {
 flags |= BDRV_REQ_ZERO_WRITE;
 if (bs->detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP) {
 flags |= BDRV_REQ_MAY_UNMAP;
diff --git a/util/iov.c b/util/iov.c
index 366ff9cdd1..9ac0261853 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -451,23 +451,30 @@ void qemu_iovec_init_extended(
 }
 
 /*
- * Check if the contents of the iovecs are all zero
+ * Check if the contents of subrange of qiov data is all zeroes.
  */
-bool qemu_iovec_is_zero(QEMUIOVector *qiov)
+bool qemu_iovec_is_zero(QEMUIOVector *qiov, size_t offset, size_t bytes)
 {
-int i;
-for (i = 0; i < qiov->niov; i++) {
-size_t offs = QEMU_ALIGN_DOWN(qiov->iov[i].iov_len, 4 * sizeof(long));
-uint8_t *ptr = qiov->iov[i].iov_base;
-if (offs && !buffer_is_zero(qiov->iov[i].iov_base, offs)) {
+struct iovec *iov;
+size_t current_offset;
+
+assert(offset + bytes <= qiov->size);
+
+iov = iov_skip_offset(qiov->iov, offset, _offset);
+
+while (bytes) {
+uint8_t *base = (uint8_t *)iov->iov_base + current_offset;
+size_t len = MIN(iov->iov_len - current_offset, bytes);
+
+if (!buffer_is_zero(base, len)) {
 return false;
 }
-for (; offs < qiov->iov[i].iov_len; offs++) {
-if (ptr[offs]) {
-return false;
-}
-}
+
+current_offset = 0;
+bytes -= len;
+iov++;
 }
+
 return true;
 }
 
-- 
2.21.0

[Qemu-devel] [PULL 01/12] util/iov: introduce qemu_iovec_init_extended

[Stefan Hajnoczi]

From: Vladimir Sementsov-Ogievskiy 

Introduce new initialization API, to create requests with padding. Will
be used in the following patch. New API uses qemu_iovec_init_buf if
resulting io vector has only one element, to avoid extra allocations.
So, we need to update qemu_iovec_destroy to support destroying such
QIOVs.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Acked-by: Stefan Hajnoczi 
Message-id: 20190604161514.262241-2-vsement...@virtuozzo.com
Message-Id: <20190604161514.262241-2-vsement...@virtuozzo.com>
Signed-off-by: Stefan Hajnoczi 
---
 include/qemu/iov.h |   7 +++
 util/iov.c | 112 +++--
 2 files changed, 114 insertions(+), 5 deletions(-)

diff --git a/include/qemu/iov.h b/include/qemu/iov.h
index 48b45987b7..f3787a0cf7 100644
--- a/include/qemu/iov.h
+++ b/include/qemu/iov.h
@@ -199,6 +199,13 @@ static inline void *qemu_iovec_buf(QEMUIOVector *qiov)
 
 void qemu_iovec_init(QEMUIOVector *qiov, int alloc_hint);
 void qemu_iovec_init_external(QEMUIOVector *qiov, struct iovec *iov, int niov);
+void qemu_iovec_init_extended(
+QEMUIOVector *qiov,
+void *head_buf, size_t head_len,
+QEMUIOVector *mid_qiov, size_t mid_offset, size_t mid_len,
+void *tail_buf, size_t tail_len);
+void qemu_iovec_init_slice(QEMUIOVector *qiov, QEMUIOVector *source,
+   size_t offset, size_t len);
 void qemu_iovec_add(QEMUIOVector *qiov, void *base, size_t len);
 void qemu_iovec_concat(QEMUIOVector *dst,
QEMUIOVector *src, size_t soffset, size_t sbytes);
diff --git a/util/iov.c b/util/iov.c
index 74e6ca8ed7..366ff9cdd1 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -353,6 +353,103 @@ void qemu_iovec_concat(QEMUIOVector *dst,
 qemu_iovec_concat_iov(dst, src->iov, src->niov, soffset, sbytes);
 }
 
+/*
+ * qiov_find_iov
+ *
+ * Return pointer to iovec structure, where byte at @offset in original vector
+ * @iov exactly is.
+ * Set @remaining_offset to be offset inside that iovec to the same byte.
+ */
+static struct iovec *iov_skip_offset(struct iovec *iov, size_t offset,
+ size_t *remaining_offset)
+{
+while (offset > 0 && offset >= iov->iov_len) {
+offset -= iov->iov_len;
+iov++;
+}
+*remaining_offset = offset;
+
+return iov;
+}
+
+/*
+ * qiov_slice
+ *
+ * Find subarray of iovec's, containing requested range. @head would
+ * be offset in first iov (returned by the function), @tail would be
+ * count of extra bytes in last iovec (returned iov + @niov - 1).
+ */
+static struct iovec *qiov_slice(QEMUIOVector *qiov,
+size_t offset, size_t len,
+size_t *head, size_t *tail, int *niov)
+{
+struct iovec *iov, *end_iov;
+
+assert(offset + len <= qiov->size);
+
+iov = iov_skip_offset(qiov->iov, offset, head);
+end_iov = iov_skip_offset(iov, *head + len, tail);
+
+if (*tail > 0) {
+assert(*tail < end_iov->iov_len);
+*tail = end_iov->iov_len - *tail;
+end_iov++;
+}
+
+*niov = end_iov - iov;
+
+return iov;
+}
+
+/*
+ * Compile new iovec, combining @head_buf buffer, sub-qiov of @mid_qiov,
+ * and @tail_buf buffer into new qiov.
+ */
+void qemu_iovec_init_extended(
+QEMUIOVector *qiov,
+void *head_buf, size_t head_len,
+QEMUIOVector *mid_qiov, size_t mid_offset, size_t mid_len,
+void *tail_buf, size_t tail_len)
+{
+size_t mid_head, mid_tail;
+int total_niov, mid_niov = 0;
+struct iovec *p, *mid_iov;
+
+if (mid_len) {
+mid_iov = qiov_slice(mid_qiov, mid_offset, mid_len,
+ _head, _tail, _niov);
+}
+
+total_niov = !!head_len + mid_niov + !!tail_len;
+if (total_niov == 1) {
+qemu_iovec_init_buf(qiov, NULL, 0);
+p = >local_iov;
+} else {
+qiov->niov = qiov->nalloc = total_niov;
+qiov->size = head_len + mid_len + tail_len;
+p = qiov->iov = g_new(struct iovec, qiov->niov);
+}
+
+if (head_len) {
+p->iov_base = head_buf;
+p->iov_len = head_len;
+p++;
+}
+
+if (mid_len) {
+memcpy(p, mid_iov, mid_niov * sizeof(*p));
+p[0].iov_base = (uint8_t *)p[0].iov_base + mid_head;
+p[0].iov_len -= mid_head;
+p[mid_niov - 1].iov_len -= mid_tail;
+p += mid_niov;
+}
+
+if (tail_len) {
+p->iov_base = tail_buf;
+p->iov_len = tail_len;
+}
+}
+
 /*
  * Check if the contents of the iovecs are all zero
  */
@@ -374,14 +471,19 @@ bool qemu_iovec_is_zero(QEMUIOVector *qiov)
 return true;
 }
 
+void qemu_iovec_init_slice(QEMUIOVector *qiov, QEMUIOVector *source,
+   size_t offset, size_t len)
+{
+qemu_iovec_init_extended(qiov, NULL, 0, source, offset, len, NULL, 0);
+}
+
 void qemu_iovec_destroy(QEMUIOVector *qiov)
 {
-assert(qiov->nalloc != -1);
+if (qiov->nalloc != -1) {
+   

[Qemu-devel] [PULL 00/12] Block patches

[Stefan Hajnoczi]

The following changes since commit dac03af5d5482ec7ee9c23db467bb7230b33c0d9:

  Merge remote-tracking branch 'remotes/rth/tags/pull-axp-20190825' into 
staging (2019-08-27 10:00:51 +0100)

are available in the Git repository at:

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

for you to fetch changes up to 5396234b96a2ac743f48644529771498e036e698:

  block/qcow2: implement .bdrv_co_pwritev(_compressed)_part (2019-08-27 
14:58:42 +0100)


Pull request



Vladimir Sementsov-Ogievskiy (12):
  util/iov: introduce qemu_iovec_init_extended
  util/iov: improve qemu_iovec_is_zero
  block/io: refactor padding
  block: define .*_part io handlers in BlockDriver
  block/io: bdrv_co_do_copy_on_readv: use and support qiov_offset
  block/io: bdrv_co_do_copy_on_readv: lazy allocation
  block/io: bdrv_aligned_preadv: use and support qiov_offset
  block/io: bdrv_aligned_pwritev: use and support qiov_offset
  block/io: introduce bdrv_co_p{read, write}v_part
  block/qcow2: refactor qcow2_co_preadv to use buffer-based io
  block/qcow2: implement .bdrv_co_preadv_part
  block/qcow2: implement .bdrv_co_pwritev(_compressed)_part

 block/qcow2.h |   1 +
 include/block/block_int.h |  21 ++
 include/qemu/iov.h|  10 +-
 block/backup.c|   2 +-
 block/io.c| 541 +++---
 block/qcow2-cluster.c |  14 +-
 block/qcow2.c | 131 +
 qemu-img.c|   4 +-
 util/iov.c| 153 +--
 9 files changed, 568 insertions(+), 309 deletions(-)

-- 
2.21.0

Re: [Qemu-devel] [PATCH 2/2] qapi: deprecate implicit filters

[John Snow]

On 8/23/19 5:22 AM, Vladimir Sementsov-Ogievskiy wrote:
> 14.08.2019 13:07, Vladimir Sementsov-Ogievskiy wrote:
>> To get rid of implicit filters related workarounds in future let's
>> deprecate them now.
> 
> Interesting, could we deprecate implicit filter without deprecation of 
> unnecessity of
> parameter? As actually, it's good when this parameter is not necessary, in 
> most cases
> user is not interested in node-name.
> 

https://en.wiktionary.org/wiki/unnecessity -- I am surprised to learn
that this a real word in the language I speak. :)

I assume you're referring to making the optional argument mandatory.

> Obviously we can do the following:
> 
> 1. In 4.2 we deprecate unnecessity, which implies deprecation of implicit 
> filters
> 2. After some releases in 4.x we can drop deprecated functionality, so we 
> drop it together with
> implicit filters. And, in same release 4.x we return it back (as it's 
> compatible change :)
> but without implicit filters (so, if filter-node-name not specified, we just 
> create
> explicit filter with autogenerated node-name)
> 
> So, effectively we just drop "deprecation mark" together with implicit 
> filters, which is nice
> but actually confusing.
> 
> Instead, we may do
> 1. In 4.2 deprecate
> 2. In 4.x drop optionality together with implicit filters
> 3. In 4.y (y > x of course) return optionality back
> 

Ah, I see what you're digging at here now...

> It's a bit safer, but for users who miss releases [4.x, 4.y) it's no 
> difference..
> 
> Or we just write in spec, that implicit filters are deprecated? But we have 
> nothing about implicit
> filters in spec. More over, we directly write that we have filter, and if 
> parameter is omitted
> it's node-name is autogenerated. So actually, the fact the filter is hidden 
> when filter-node-name is
> unspecified is _undocumented_.
> 
> So, finally, it looks like nothing to deprecated in specification, we can 
> just drop implicit filters :)
> 
> What do you think?
> 

What exactly _IS_ an implicit filter? How does it differ today from an
explicit filter? I assumed the only difference was if it was named or
not; but I think I must be mistaken now if you're proposing leaving the
interface alone entirely.

Are they instantiated differently?

--js

Re: [Qemu-devel] [edk2-rfc] [edk2-devel] CPU hotplug using SMM with QEMU+OVMF

[Laszlo Ersek]

On 08/27/19 18:23, Igor Mammedov wrote:
> On Mon, 26 Aug 2019 17:30:43 +0200
> Laszlo Ersek  wrote:
> 
>> On 08/23/19 17:25, Kinney, Michael D wrote:
>>> Hi Jiewen,
>>>
>>> If a hot add CPU needs to run any code before the
>>> first SMI, I would recommend is only executes code
>>> from a write protected FLASH range without a stack
>>> and then wait for the first SMI.  
>>
>> "without a stack" looks very risky to me. Even if we manage to implement
>> the guest code initially, we'll be trapped without a stack, should we
>> ever need to add more complex stuff there.
> 
> Do we need anything complex in relocation handler, though?
> From what I'd imagine, minimum handler should
>   1: get address of TSEG, possibly read it from chipset

The TSEG base calculation is not trivial in this environment. The 32-bit
RAM size needs to be read from the CMOS (IO port accesses). Then the
extended TSEG size (if any) needs to be detected from PCI config space
(IO port accesses). Both CMOS and PCI config space requires IO port
writes too (not just reads). Even if there are enough registers for the
calculations, can we rely on these unprotected IO ports?

Also, can we switch to 32-bit mode without a stack? I assume it would be
necessary to switch to 32-bit mode for 32-bit arithmetic.

Getting the initial APIC ID needs some CPUID instructions IIUC, which
clobber EAX through EDX, if I understand correctly. Given the register
pressure, CPUID might have to be one of the first instructions to call.

>   2: calculate its new SMBASE offset based on its APIC ID
>   3: save new SMBASE
> 
>>> For this OVMF use case, is any CPU init required
>>> before the first SMI?  
>>
>> I expressed a preference for that too: "I wish we could simply wake the
>> new CPU [...] with an SMI".
>>
>> 398b3327-0820-95af-a34d-1a4a1d50cf35@redhat.com">http://mid.mail-archive.com/398b3327-0820-95af-a34d-1a4a1d50cf35@redhat.com
>>
>>
>>> From Paolo's list of steps are steps (8a) and (8b) 
>>> really required?  
> 
> 07b - implies 08b

I agree about that implication, yes. *If* we send an INIT/SIPI/SIPI to
the new CPU, then the new CPU needs a HLT loop, I think.

>8b could be trivial hlt loop and we most likely could skip 08a and 
> signaling host CPU steps
>but we need INIT/SIPI/SIPI sequence to wake up AP so it could handle 
> pending SMI
>before handling SIPI (so behavior would follow SDM).
> 
> 
>> See again my message linked above -- just after the quoted sentence, I
>> wrote, "IOW, if we could excise steps 07b, 08a, 08b".
>>
>> But, I obviously defer to Paolo and Igor on that.
>>
>> (I do believe we have a dilemma here. In QEMU, we probably prefer to
>> emulate physical hardware as faithfully as possible. However, we do not
>> have Cache-As-RAM (nor do we intend to, IIUC). Does that justify other
>> divergences from physical hardware too, such as waking just by virtue of
>> an SMI?)
> So far we should be able to implement it per spec (at least SDM one),
> but we would still need to invent chipset hardware
> i.e. like adding to Q35 non exiting SMRAM and means to map/unmap it
> to non-SMM address space.
> (and I hope we could avoid adding "parked CPU" thingy)

I think we'll need a separate QEMU tree for this. I'm quite in the dark
-- I can't tell if I'll be able to do something in OVMF without actually
trying it. And for that, we'll need some proposed QEMU code that is
testable, but not upstream yet. (As I might realize that I'm unable to
make it work in OVMF.)

>>> Can the SMI monarch use the Local
>>> APIC to send a directed SMI to the hot added CPU?
>>> The SMI monarch needs to know the APIC ID of the
>>> hot added CPU.  Do we also need to handle the case
>>> where multiple CPUs are added at once?  I think we
>>> would need to serialize the use of 3000:8000 for the
>>> SMM rebase operation on each hot added CPU.  
>>
>> I agree this would be a huge help.
> 
> We can serialize it (for normal hotplug flow) from ACPI handler
> in the guest (i.e. non enforced serialization).
> The only reason for serialization I see is not to allow
> a bunch of new CPU trample over default SMBASE save area
> at the same time.

If the default SMBASE area is corrupted due to concurrent access, could
that lead to invalid relocated SMBASE values? Possibly pointing into
normal RAM?

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v2] configure: more resilient Python version capture

[Eduardo Habkost]

On Mon, Aug 26, 2019 at 11:58:32AM -0400, Cleber Rosa wrote:
> The current approach to capture the Python version is fragile, as it
> was demonstrated by a very specific build of Python 3 on Fedora 29
> that, under non-interactive shells would print multiline version
> information.
> 
> The (badly) stripped version output would be sent to config-host.mak,
> producing bad syntax and rendering the makefiles unusable.  Now, the
> Python versions is printed by configure, but only a simple (and better
> controlled variable) indicating whether the build system is using
> Python 2 is kept on config-host.mak.
> 
> Signed-off-by: Cleber Rosa 

Queued, thanks!

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 18/68] target/arm: Convert the rest of A32 Miscelaneous instructions

[Richard Henderson]

On 8/27/19 3:32 AM, Peter Maydell wrote:
>> +static bool trans_HVC(DisasContext *s, arg_HVC *a)
>> +{
>> +if (!ENABLE_ARCH_7 || IS_USER(s) || arm_dc_feature(s, ARM_FEATURE_M)) {
>> +return false;
>> +}
>> +gen_hvc(s, a->imm);
>> +return true;
>> +}
> 
> I was wondering about for these trans_ functions the
> difference between returning 'false' and calling
> unallocated_encoding() and then returning 'true'.
> If I understand the decodetree right this will only
> make a difference when the pattern is inside a {} group.

Correct.

> So for instance here we have
> 
> {
>   [...]
>   {
> HVC   0111 1110   1000    \
>   imm=%imm16_16_0
> CPS   0011 1010  1000 0 imod:2 M:1 A:1 I:1 F:1 mode:5 \
>  
> UDF   0111    1010   
>   }
>   B_cond_thumb    0. cond:4 .. 10.0    imm=%imm21
> }
> 
> which means that if the HVC returns 'false' we'll end up
> trying the insn as a B_cond_thumb.

Correct.

> In this case the
> trans function for the B_cond_thumb pattern will correctly
> return false itself for a->cond >= 0xe, so it makes no
> difference. But maybe it would be more robust for a pattern
> like HVC to be self-contained so it doesn't fall through
> for cases that really do belong to it but happen to be
> required to UNDEF (like IS_USER() == true) ?

I agree this should be the rule.

E.g. for this IS_USER case, we have successfully decoded HVC and so should not
return false.  The fact that HVC should raise SIGILL if IS_USER should not be
confused with decoding HVC.

Other constraints, such as rd != 15 or imod != 0, should continue to return
false so that a (potential) grouped insn can match.

> OTOH I suppose you could say that when you're writing patterns
> like the B_cond_thumb one you know you've underdecoded and must
> catch all the theoretical overlaps by writing checks in the trans
> function, so as long as you do that correctly you're fine.

Yes.


r~

Re: [Qemu-devel] [Qemu-block] [PATCH v5 1/6] iotests: allow Valgrind checking all QEMU processes

[John Snow]

On 8/25/19 11:26 AM, Andrey Shinkevich wrote:
> 
> 
> On 16/08/2019 01:49, John Snow wrote:
>>
>>
>> On 7/19/19 12:30 PM, Andrey Shinkevich wrote:
>>> With the '-valgrind' option, let all the QEMU processes be run under
>>> the Valgrind tool. The Valgrind own parameters may be set with its
>>> environment variable VALGRIND_OPTS, e.g.
>>> VALGRIND_OPTS="--leak-check=yes" ./check -qcow2 -valgrind 
>>> or they may be listed in the Valgrind checked file ./.valgrindrc or
>>> ~/.valgrindrc like
>>> --memcheck:leak-check=no
>>> --memcheck:track-origins=yes
>>> When QEMU-IO process is being killed, the shell report refers to the
>>> text of the command in _qemu_io_wrapper(), which was modified with this
>>> patch. So, the benchmark output for the tests 039, 061 and 137 is to be
>>> changed also.
>>>
>>
>> Oh, weird. "VALGRIND_QEMU=y" actually has just meant ... valgrind
>> qemu-io. OK.
>>
>>> Signed-off-by: Andrey Shinkevich 
>>> ---
>>>   tests/qemu-iotests/039.out   | 30 ---
>>>   tests/qemu-iotests/061.out   | 12 ++--
>>>   tests/qemu-iotests/137.out   |  6 +---
>>>   tests/qemu-iotests/common.rc | 69 
>>> 
>>>   4 files changed, 59 insertions(+), 58 deletions(-)
>>>
>>> diff --git a/tests/qemu-iotests/039.out b/tests/qemu-iotests/039.out
>>> index 724d7b2..972c6c0 100644
>>> --- a/tests/qemu-iotests/039.out
>>> +++ b/tests/qemu-iotests/039.out
>>> @@ -11,11 +11,7 @@ No errors were found on the image.
>>>   Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
>>>   wrote 512/512 bytes at offset 0
>>>   512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>>> -./common.rc: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; 
>>> then
>>> -exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 
>>> "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -else
>>> -exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -fi )
>>> +./common.rc: Killed  ( _qemu_proc_wrapper 
>>> "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>>   incompatible_features 0x1
>>>   ERROR cluster 5 refcount=0 reference=1
>>>   ERROR OFLAG_COPIED data cluster: l2_entry=8005 refcount=0
>>> @@ -50,11 +46,7 @@ read 512/512 bytes at offset 0
>>>   Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
>>>   wrote 512/512 bytes at offset 0
>>>   512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>>> -./common.rc: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; 
>>> then
>>> -exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 
>>> "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -else
>>> -exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -fi )
>>> +./common.rc: Killed  ( _qemu_proc_wrapper 
>>> "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>>   incompatible_features 0x1
>>>   ERROR cluster 5 refcount=0 reference=1
>>>   Rebuilding refcount structure
>>> @@ -68,11 +60,7 @@ incompatible_features 0x0
>>>   Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
>>>   wrote 512/512 bytes at offset 0
>>>   512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>>> -./common.rc: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; 
>>> then
>>> -exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 
>>> "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -else
>>> -exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -fi )
>>> +./common.rc: Killed  ( _qemu_proc_wrapper 
>>> "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>>   incompatible_features 0x0
>>>   No errors were found on the image.
>>>   
>>> @@ -91,11 +79,7 @@ No errors were found on the image.
>>>   Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
>>>   wrote 512/512 bytes at offset 0
>>>   512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>>> -./common.rc: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; 
>>> then
>>> -exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 
>>> "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -else
>>> -exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -fi )
>>> +./common.rc: Killed  ( _qemu_proc_wrapper 
>>> "${VALGRIND_LOGFILE}" "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@" )
>>>   incompatible_features 0x1
>>>   ERROR cluster 5 refcount=0 reference=1
>>>   ERROR OFLAG_COPIED data cluster: l2_entry=8005 refcount=0
>>> @@ -105,11 +89,7 @@ Data may be corrupted, or further writes to the image 
>>> may corrupt it.
>>>   Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
>>>   wrote 512/512 bytes at offset 0
>>>   512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>>> -./common.rc: Killed  ( if [ "${VALGRIND_QEMU}" == "y" ]; 
>>> then
>>> -exec valgrind --log-file="${VALGRIND_LOGFILE}" --error-exitcode=99 
>>> "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -else
>>> -exec "$QEMU_IO_PROG" $QEMU_IO_ARGS "$@";
>>> -fi )
>>> 

Re: [Qemu-devel] [PATCH] i386/vmmouse: Properly reset state

[Eduardo Habkost]

On Sun, Aug 25, 2019 at 04:58:18PM +0200, Jan Kiszka wrote:
> On 21.07.19 10:58, Jan Kiszka wrote:
> > From: Jan Kiszka 
> > 
> > nb_queue was not zeroed so that we no longer delivered events if a
> > previous guest left the device in an overflow state.
> > 
> > The state of absolute does not matter as the next vmmouse_update_handler
> > call will align it again.
> > 
> > Signed-off-by: Jan Kiszka 
> > ---
> >   hw/i386/vmmouse.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/i386/vmmouse.c b/hw/i386/vmmouse.c
> > index 5d2d278be4..e335bd07da 100644
> > --- a/hw/i386/vmmouse.c
> > +++ b/hw/i386/vmmouse.c
> > @@ -257,6 +257,7 @@ static void vmmouse_reset(DeviceState *d)
> >   VMMouseState *s = VMMOUSE(d);
> > 
> >   s->queue_size = VMMOUSE_QUEUE_SIZE;
> > +s->nb_queue = 0;
> > 
> >   vmmouse_disable(s);
> >   }
> > --
> > 2.16.4
> > 
> > 
> 
> Ping - or who is looking after this?

Despite being in hw/i386, I think we can say vmmouse.c doesn't
have a maintainer.  Last time someone changed vmmouse.c in a
meaningful way (not just adapting to API changes or removing
duplicate code) was in 2012.

But the change makes sense to me, so:

Reviewed-by: Eduardo Habkost 

I'll queue it.

-- 
Eduardo

[Qemu-devel] [PATCH] gdbstub: Fix handler for 'F' packet

[Sandra Loosemore]

Handling of the 'F' packet has been broken since commit
4b20fab101b9e2d0fb47454209637a17fc7a13d5, which converted it to use
the new packet parsing infrastructure.  Per the GDB RSP specification

https://sourceware.org/gdb/current/onlinedocs/gdb/The-F-Reply-Packet.html

the second parameter may be omitted, but the rewritten implementation
was failing to recognize this case.  The result was that QEMU was
repeatedly resending the fileio request and ignoring GDB's replies of
successful completion.  This patch restores the behavior of the
previous code in allowing the errno parameter to be omitted and
passing 0 to the callback in that case.

Signed-off-by: Sandra Loosemore 
---
 gdbstub.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index b92ba59..141568a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1820,11 +1820,15 @@ static void handle_read_all_regs(GdbCmdContext 
*gdb_ctx, void *user_ctx)
 
 static void handle_file_io(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
-if (gdb_ctx->num_params >= 2 && gdb_ctx->s->current_syscall_cb) {
+if (gdb_ctx->num_params >= 1 && gdb_ctx->s->current_syscall_cb) {
 target_ulong ret, err;
 
 ret = (target_ulong)gdb_ctx->params[0].val_ull;
-err = (target_ulong)gdb_ctx->params[1].val_ull;
+if (gdb_ctx->num_params >= 2 ) {
+err = (target_ulong)gdb_ctx->params[1].val_ull;
+} else {
+err = 0;
+}
 gdb_ctx->s->current_syscall_cb(gdb_ctx->s->c_cpu, ret, err);
 gdb_ctx->s->current_syscall_cb = NULL;
 }
-- 
2.8.1

Re: [Qemu-devel] [Qemu-block] [PATCH v5 4/6] iotests: Valgrind fails with nonexistent directory

[John Snow]

On 8/25/19 11:24 AM, Andrey Shinkevich wrote:
> 
> 
> On 16/08/2019 03:55, John Snow wrote:
>>
>>
>> On 7/19/19 12:30 PM, Andrey Shinkevich wrote:
>>> The Valgrind uses the exported variable TMPDIR and fails if the
>>> directory does not exist. Let us exclude such a test case from
>>> being run under the Valgrind and notify the user of it.
>>>
>>> Suggested-by: Kevin Wolf 
>>> Signed-off-by: Andrey Shinkevich 
>>> ---
>>>   tests/qemu-iotests/051 | 4 
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
>>> index ce942a5..f8141ca 100755
>>> --- a/tests/qemu-iotests/051
>>> +++ b/tests/qemu-iotests/051
>>> @@ -377,6 +377,10 @@ printf %b "qemu-io $device_id \"write -P 0x33 0 
>>> 4k\"\ncommit $device_id\n" |
>>>   $QEMU_IO -c "read -P 0x33 0 4k" "$TEST_IMG" | _filter_qemu_io
>>>   
>>>   # Using snapshot=on with a non-existent TMPDIR
>>> +if [ "${VALGRIND_QEMU}" == "y" ]; then
>>> +_casenotrun "Valgrind needs a valid TMPDIR for itself"
>>> +fi
>>> +VALGRIND_QEMU="" \
>>>   TMPDIR=/nonexistent run_qemu -drive driver=null-co,snapshot=on
>>>   
>>>   # Using snapshot=on together with read-only=on
>>>
>>
>> The only other way around this would be a complicated mechanism to set
>> the TMPDIR for valgrind's sub-processes only, with e.g.
>>
>> valgrind ... env TMPDIR=/nonexistent qemu ...
>>
>> ... It's probably not worth trying to concoct such a thing; but I
>> suppose it is possible. You'd have to set up a generic layer for setting
>> environment variables, then in the qemu shim, you could either set them
>> directly (non-valgrind invocation) or set them as part of the valgrind
>> command-line.
>>
>> Or you could just take my R-B:
>>
>> Reviewed-by: John Snow 
>>
> 
> Thanks again John for your review and the advice.
> Probably, it doesn't worth efforts to manage that case because QEMU 
> should fail anyway with the error message "Could not get temporary 
> filename: No such file or directory". So, we would not benefit much from 
> that run. We have other test cases that cover the main functionality. 
> It's just to check the QEMU error path for possible memory issues. Shall we?
> 
> Andrey
> 

Yeah, don't bother with this for now. I just have a personal compulsion
to try to concretely measure how much work it would take to avoid a
"hack" and then make my decision.

You're free to just take the R-B :)

--js

Re: [Qemu-devel] [Qemu-block] [PATCH v5 6/6] iotests: extend sleeping time under Valgrind

[John Snow]

On 8/23/19 11:27 AM, Vladimir Sementsov-Ogievskiy wrote:
> 16.08.2019 4:01, John Snow wrote:
>>
>>
>> On 7/19/19 12:30 PM, Andrey Shinkevich wrote:
>>> To synchronize the time when QEMU is running longer under the Valgrind,
>>> increase the sleeping time in the test 247.
>>>
>>> Signed-off-by: Andrey Shinkevich 
>>> Reviewed-by: Vladimir Sementsov-Ogievskiy 
>>> ---
>>>   tests/qemu-iotests/247 | 6 +-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/qemu-iotests/247 b/tests/qemu-iotests/247
>>> index 546a794..c853b73 100755
>>> --- a/tests/qemu-iotests/247
>>> +++ b/tests/qemu-iotests/247
>>> @@ -57,7 +57,11 @@ TEST_IMG="$TEST_IMG.4" _make_test_img $size
>>>   {"execute":"block-commit",
>>>"arguments":{"device":"format-4", "top-node": "format-2", 
>>> "base-node":"format-0", "job-id":"job0"}}
>>>   EOF
>>> -sleep 1
>>> +if [ "${VALGRIND_QEMU}" == "y" ]; then
>>> +sleep 10
>>> +else
>>> +sleep 1
>>> +fi
>>>   echo '{"execute":"quit"}'
>>>   ) | $QEMU -qmp stdio -nographic -nodefaults \
>>>   -blockdev 
>>> file,node-name=file-0,filename=$TEST_IMG.0,auto-read-only=on \
>>>
>>
>> This makes me nervous, though. Won't this race terribly? (Wait, why
>> doesn't it race already?)
>>
> 
> Hmm, however it works somehow. I'm afraid that everything with "sleep" is 
> definitely racy..
> Or what do you mean?
> 

Right -- anything with a sleep is already at risk for racing.

What I am picking up on here is that with valgrind, there is an even
greater computational overhead that's much harder to predict, so I was
wondering how these values were determined.

(I wouldn't withhold an RB for that alone -- the sleeps are existing
problems.)

What I moved on to wondering in particular is why test 247 doesn't
already have race problems, because it looks quite fragile.

Neither of these are really Andrey's problems; I was just surprised
momentarily that I don't see 247 fail more often already, as-is.

--js

Re: [Qemu-devel] [PATCH v2 21/68] target/arm: Convert Synchronization primitives

[Richard Henderson]

On 8/27/19 4:10 AM, Peter Maydell wrote:
> On Tue, 27 Aug 2019 at 11:46, Peter Maydell  wrote:
>> ...OK, not this specific function, as I just noticed it's the _a32
>> one, but trans_STREXB(), trans_STREXH(), etc are wrong.
> 
> I did a quick grep through for places checking the 6K condition,
> and I think these are the only ones that need changing:
> 
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index b4d53f3d37f..58e50f2d808 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -8874,7 +8874,7 @@ static bool trans_STREXD_t32(DisasContext *s,
> arg_STREX *a)
> 
>  static bool trans_STREXB(DisasContext *s, arg_STREX *a)
>  {
> -if (!ENABLE_ARCH_6K) {
> +if (!ENABLE_ARCH_6K && !arm_dc_feature(s, ARM_FEATURE_M)) {

Looking again, I think the correct test is

if (s->thumb ? !ENABLE_ARCH_7 : !ENABLE_ARCH_6K)

for all of these.


r~

[Qemu-devel] [RFC,Draft] ui: add an embedded Barrier client

[Laurent Vivier]

This allows to receive mouse and keyboard events from
a Barrier server.

This is enabled by adding the following parameter on the
command line

... -object input-barrier,id=$id,name=$name ...

Where $name is the name declared in the screens section of barrier.conf

The barrier server (barriers) must be configured and must run on the
local host.

For instance:

  section: screens
  localhost:
  ...
  VM-1:
  ...
  end

  section: links
  localhost:
  right = VM-1
  VM-1:
  left = localhost
  end

Then on the QEMU command line:

... -object input-barrier,id=barrie0,name=VM-1 ...

When the mouse will move out of the screen of the local host on
the right, the mouse and the keyboard will be grabbed and all
related events will be send to the guest OS.

This is usefull when qemu is configured without emulated graphic card
but with a VFIO attached graphic card.

More information about Barrier can be found at:

  https://github.com/debauchee/barrier

This avoids to install the Barrier server in the guest OS,
for instance when it is not supported or during the installation.

Signed-off-by: Laurent Vivier 
---
 ui/Makefile.objs   |   1 +
 ui/input-barrier.c | 639 +
 2 files changed, 640 insertions(+)
 create mode 100644 ui/input-barrier.c

diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index cc2bf5b180f3..95b69a87444d 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -9,6 +9,7 @@ vnc-obj-y += vnc-jobs.o
 
 common-obj-y += keymaps.o console.o cursor.o qemu-pixman.o
 common-obj-y += input.o input-keymap.o input-legacy.o kbd-state.o
+common-obj-y += input-barrier.o
 common-obj-$(CONFIG_LINUX) += input-linux.o
 common-obj-$(CONFIG_SPICE) += spice-core.o spice-input.o spice-display.o
 common-obj-$(CONFIG_COCOA) += cocoa.o
diff --git a/ui/input-barrier.c b/ui/input-barrier.c
new file mode 100644
index ..3fdcbc194f95
--- /dev/null
+++ b/ui/input-barrier.c
@@ -0,0 +1,639 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/main-loop.h"
+#include "qemu/sockets.h"
+#include "qapi/error.h"
+#include "qom/object_interfaces.h"
+#include "io/channel-socket.h"
+#include "ui/input.h"
+
+#define TYPE_INPUT_BARRIER "input-barrier"
+#define INPUT_BARRIER(obj) \
+OBJECT_CHECK(InputBarrier, (obj), TYPE_INPUT_BARRIER)
+#define INPUT_BARRIER_GET_CLASS(obj) \
+OBJECT_GET_CLASS(InputBarrierClass, (obj), TYPE_INPUT_BARRIER)
+#define INPUT_BARRIER_CLASS(klass) \
+OBJECT_CLASS_CHECK(InputBarrierClass, (klass), TYPE_INPUT_BARRIER)
+
+typedef struct InputBarrier InputBarrier;
+typedef struct InputBarrierClass InputBarrierClass;
+
+#define MAX_HELLO_LENGTH 1024
+
+struct InputBarrier {
+Object parent;
+
+QIOChannelSocket *sioc;
+guint ioc_tag;
+
+/* display properties */
+char *name;
+
+/*
+ * XXX:
+ * add x-origin, y-origin, width and height
+ * add Barrier server IP address and port
+ */
+
+char buffer[MAX_HELLO_LENGTH];
+};
+
+struct InputBarrierClass {
+ObjectClass parent_class;
+};
+
+/* Barrier protocol */
+#define BARRIER_VERSION_MAJOR 1
+#define BARRIER_VERSION_MINOR 6
+
+enum cmdids {
+MSG_CNoop,
+MSG_CClose,
+MSG_CEnter,
+MSG_CLeave,
+MSG_CClipboard,
+MSG_CScreenSaver,
+MSG_CResetOptions,
+MSG_CInfoAck,
+MSG_CKeepAlive,
+MSG_DKeyDown,
+MSG_DKeyRepeat,
+MSG_DKeyUp,
+MSG_DMouseDown,
+MSG_DMouseUp,
+MSG_DMouseMove,
+MSG_DMouseRelMove,
+MSG_DMouseWheel,
+MSG_DClipboard,
+MSG_DInfo,
+MSG_DSetOptions,
+MSG_DFileTransfer,
+MSG_DDragInfo,
+MSG_QInfo,
+MSG_EIncompatible,
+MSG_EBusy,
+MSG_EUnknown,
+MSG_EBad,
+/* connection sequence */
+MSG_Hello,
+MSG_HelloBack,
+};
+
+struct version {
+int16_t major;
+int16_t minor;
+};
+
+struct mousebutton {
+int8_t buttonid;
+};
+
+struct enter {
+int16_t x;
+int16_t y;
+int32_t seqn;
+int16_t modifier;
+};
+
+struct mousepos {
+int16_t x;
+int16_t y;
+};
+
+struct key {
+int16_t keyid;
+int16_t modifier;
+int16_t button;
+};
+
+struct repeat {
+int16_t keyid;
+int16_t modifier;
+int16_t repeat;
+int16_t button;
+};
+
+#define MAX_OPTIONS 32
+struct set {
+int nb;
+struct {
+int id;
+char nul;
+int value;
+} option[MAX_OPTIONS];
+};
+
+struct msg {
+enum cmdids cmd;
+union {
+struct version version;
+struct mousebutton mousebutton;
+struct mousepos mousepos;
+struct enter enter;
+struct key key;
+struct repeat repeat;
+struct set set;
+};
+};
+
+const char *cmd_names[] = {
+[MSG_CNoop]  = "CNOP",
+[MSG_CClose] = "CBYE",
+[MSG_CEnter]  

Re: [Qemu-devel] [PATCH v6 3/3] target/ppc: Refactor emulation of vmrgew and vmrgow instructions

[Richard Henderson]

On 8/27/19 2:37 AM, Stefan Brankovic wrote:
> Since I found this two instructions implemented with tcg, I refactored
> them so they are consistent with other similar implementations that
> I introduced in this patch.
> 
> Also, a new dual macro GEN_VXFORM_TRANS_DUAL is added. This macro is
> used if one instruction is realized with direct translation, and second
> one with a helper.
> 
> Signed-off-by: Stefan Brankovic 
> ---
>  target/ppc/translate/vmx-impl.inc.c | 66 
> +
>  1 file changed, 37 insertions(+), 29 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v6 2/3] target/ppc: Optimize emulation of vclzh and vclzb instructions

[Richard Henderson]

On 8/27/19 2:37 AM, Stefan Brankovic wrote:
> +for (i = 0; i < 2; i++) {
> +if (i == 0) {
> +/* Get high doubleword of vB in avr. */
> +get_avr64(avr, VB, true);
> +} else {
> +/* Get low doubleword of vB in avr. */
> +get_avr64(avr, VB, false);
> +}
> +/*
> + * Perform count for every byte element using tcg_gen_clzi_i64.
> + * Since it counts leading zeros on 64 bit lenght, we have to move
> + * ith byte element to highest 8 bits of tmp, or it with mask(so we 
> get
> + * all ones in lowest 56 bits), then perform tcg_gen_clzi_i64 and 
> move
> + * it's result in appropriate byte element of result.
> + */
> +tcg_gen_shli_i64(tmp, avr, 56);
> +tcg_gen_or_i64(tmp, tmp, mask);
> +tcg_gen_clzi_i64(result, tmp, 64);
> +for (j = 1; j < 7; j++) {
> +tcg_gen_shli_i64(tmp, avr, (7 - j) * 8);
> +tcg_gen_or_i64(tmp, tmp, mask);
> +tcg_gen_clzi_i64(tmp, tmp, 64);
> +tcg_gen_deposit_i64(result, result, tmp, j * 8, 8);
> +}
> +tcg_gen_or_i64(tmp, avr, mask);
> +tcg_gen_clzi_i64(tmp, tmp, 64);
> +tcg_gen_deposit_i64(result, result, tmp, 56, 8);
> +if (i == 0) {
> +/* Place result in high doubleword element of vD. */
> +tcg_gen_mov_i64(result1, result);
> +} else {
> +/* Place result in low doubleword element of vD. */
> +tcg_gen_mov_i64(result2, result);
> +}
> +}

By my count, 60 non-move operations.  This is too many to inline.

Moreover, unlike vpkpx, which I can see being used for graphics
format conversion in old operating systems (who else uses 16-bit
graphics formats now?), I would be very surprised to see vclzb
or vclzh being used frequently.

How did you determine that these instructions needed optimization?

I can see wanting to apply

--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -1817,8 +1817,8 @@ VUPK(lsw, s64, s32, UPKLO)
 }   \
 }

-#define clzb(v) ((v) ? clz32((uint32_t)(v) << 24) : 8)
-#define clzh(v) ((v) ? clz32((uint32_t)(v) << 16) : 16)
+#define clzb(v) clz32(((uint32_t)(v) << 24) | 0x00ffu)
+#define clzh(v) clz32(((uint32_t)(v) << 16) | 0xu)

 VGENERIC_DO(clzb, u8)
 VGENERIC_DO(clzh, u16)

as the cmov instruction required by the current implementation is going to be
quite a bit slower than the OR instruction.

And similarly for ctzb() and ctzh().


r~

Re: [Qemu-devel] [PATCH v6 1/3] target/ppc: Optimize emulation of vpkpx instruction

[BALATON Zoltan]

On Tue, 27 Aug 2019, Richard Henderson wrote:

On 8/27/19 2:37 AM, Stefan Brankovic wrote:

+for (i = 0; i < 4; i++) {
+switch (i) {
+case 0:
+/*
+ * Get high doubleword of vA to perfrom 6-5-5 pack of pixels
+ * 1 and 2.
+ */
+get_avr64(avr, VA, true);
+tcg_gen_movi_i64(result, 0x0ULL);
+break;
+case 1:
+/*
+ * Get low doubleword of vA to perfrom 6-5-5 pack of pixels
+ * 3 and 4.
+ */
+get_avr64(avr, VA, false);
+break;
+case 2:
+/*
+ * Get high doubleword of vB to perfrom 6-5-5 pack of pixels
+ * 5 and 6.
+ */
+get_avr64(avr, VB, true);
+tcg_gen_movi_i64(result, 0x0ULL);
+break;
+case 3:
+/*
+ * Get low doubleword of vB to perfrom 6-5-5 pack of pixels


If this is replaced by Richard's suggested version it does not matter but 
there's a typo in above comments. Probably you've meant perfrom -> perform


Regards,
BALATON Zoltan

[Qemu-devel] [PATCH v2 0/2] Alignment checks cleanup

[Nir Soffer]

While working on 4k support, I noticed that there is lot of code using
BDRV_SECTOR_SIZE (512) for checking alignment. I wonder how this can work with
4k storage.

Lets start by cleaning up to make the code easier to understand:
- Use QEMU_IS_ALIGNED macro to check alignment
- Remove unneeded masks based on BDRV_SECTOR_SIZE

Nir Soffer (2):
  block: Use QEMU_IS_ALIGNED
  block: Remove unused masks

 block/bochs.c | 4 ++--
 block/cloop.c | 4 ++--
 block/dmg.c   | 4 ++--
 block/io.c| 8 
 block/qcow2-cluster.c | 4 ++--
 block/qcow2.c | 4 ++--
 block/vvfat.c | 8 
 include/block/block.h | 2 --
 migration/block.c | 2 +-
 qemu-img.c| 2 +-
 10 files changed, 20 insertions(+), 22 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH v2 1/2] block: Use QEMU_IS_ALIGNED

[Nir Soffer]

Replace instances of:

(n & (BDRV_SECTOR_SIZE - 1)) == 0

And:

   (n & ~BDRV_SECTOR_MASK) == 0

With:

QEMU_IS_ALIGNED(n, BDRV_SECTOR_SIZE)

Which reveals the intent of the code better, and makes it easier to
locate the code checking alignment.

Signed-off-by: Nir Soffer 
---
 block/bochs.c | 4 ++--
 block/cloop.c | 4 ++--
 block/dmg.c   | 4 ++--
 block/io.c| 8 
 block/qcow2-cluster.c | 4 ++--
 block/qcow2.c | 4 ++--
 block/vvfat.c | 8 
 qemu-img.c| 2 +-
 8 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/block/bochs.c b/block/bochs.c
index 962f18592d..32bb83b268 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -248,8 +248,8 @@ bochs_co_preadv(BlockDriverState *bs, uint64_t offset, 
uint64_t bytes,
 QEMUIOVector local_qiov;
 int ret;
 
-assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
-assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE));
+assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE));
 
 qemu_iovec_init(_qiov, qiov->niov);
 qemu_co_mutex_lock(>lock);
diff --git a/block/cloop.c b/block/cloop.c
index 384c9735bb..4de94876d4 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -253,8 +253,8 @@ cloop_co_preadv(BlockDriverState *bs, uint64_t offset, 
uint64_t bytes,
 int nb_sectors = bytes >> BDRV_SECTOR_BITS;
 int ret, i;
 
-assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
-assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE));
+assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE));
 
 qemu_co_mutex_lock(>lock);
 
diff --git a/block/dmg.c b/block/dmg.c
index 45f6b28f17..4a045f2b3e 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -697,8 +697,8 @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, 
uint64_t bytes,
 int nb_sectors = bytes >> BDRV_SECTOR_BITS;
 int ret, i;
 
-assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
-assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE));
+assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE));
 
 qemu_co_mutex_lock(>lock);
 
diff --git a/block/io.c b/block/io.c
index 56bbf195bb..7508703ecd 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1080,8 +1080,8 @@ static int coroutine_fn 
bdrv_driver_preadv(BlockDriverState *bs,
 sector_num = offset >> BDRV_SECTOR_BITS;
 nb_sectors = bytes >> BDRV_SECTOR_BITS;
 
-assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
-assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE));
+assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE));
 assert(bytes <= BDRV_REQUEST_MAX_BYTES);
 assert(drv->bdrv_co_readv);
 
@@ -1133,8 +1133,8 @@ static int coroutine_fn 
bdrv_driver_pwritev(BlockDriverState *bs,
 sector_num = offset >> BDRV_SECTOR_BITS;
 nb_sectors = bytes >> BDRV_SECTOR_BITS;
 
-assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
-assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE));
+assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE));
 assert(bytes <= BDRV_REQUEST_MAX_BYTES);
 
 assert(drv->bdrv_co_writev);
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index cc5609e27a..f2de74690d 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -470,8 +470,8 @@ static bool coroutine_fn 
do_perform_cow_encrypt(BlockDriverState *bs,
 {
 if (bytes && bs->encrypted) {
 BDRVQcow2State *s = bs->opaque;
-assert((offset_in_cluster & ~BDRV_SECTOR_MASK) == 0);
-assert((bytes & ~BDRV_SECTOR_MASK) == 0);
+assert(QEMU_IS_ALIGNED(offset_in_cluster, BDRV_SECTOR_SIZE));
+assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE));
 assert(s->crypto);
 if (qcow2_co_encrypt(bs, cluster_offset,
  src_cluster_offset + offset_in_cluster,
diff --git a/block/qcow2.c b/block/qcow2.c
index 7c5a4859f7..e82961c9cf 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2072,8 +2072,8 @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState 
*bs, uint64_t offset,
 }
 if (bs->encrypted) {
 assert(s->crypto);
-assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
-assert((cur_bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE));
+assert(QEMU_IS_ALIGNED(cur_bytes, BDRV_SECTOR_SIZE));
 if (qcow2_co_decrypt(bs, cluster_offset, offset,
  cluster_data, cur_bytes) < 0) {
 ret = -EIO;
diff --git a/block/vvfat.c b/block/vvfat.c
index f6c28805dd..019b8f1341 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1547,8 +1547,8 @@ vvfat_co_preadv(BlockDriverState *bs, uint64_t offset, 
uint64_t bytes,
 int nb_sectors = bytes >> BDRV_SECTOR_BITS;
 void 

[Qemu-devel] [PATCH v2 2/2] block: Remove unused masks

[Nir Soffer]

Replace confusing usage:

~BDRV_SECTOR_MASK

With more clear:

(BDRV_SECTOR_SIZE - 1)

Remove BDRV_SECTOR_MASK and the unused BDRV_BLOCK_OFFSET_MASK which was
it's last user.

Signed-off-by: Nir Soffer 
---
 include/block/block.h | 2 --
 migration/block.c | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index 124ad40809..37c9de7446 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -143,7 +143,6 @@ typedef struct HDGeometry {
 
 #define BDRV_SECTOR_BITS   9
 #define BDRV_SECTOR_SIZE   (1ULL << BDRV_SECTOR_BITS)
-#define BDRV_SECTOR_MASK   ~(BDRV_SECTOR_SIZE - 1)
 
 #define BDRV_REQUEST_MAX_SECTORS MIN(SIZE_MAX >> BDRV_SECTOR_BITS, \
  INT_MAX >> BDRV_SECTOR_BITS)
@@ -195,7 +194,6 @@ typedef struct HDGeometry {
 #define BDRV_BLOCK_ALLOCATED0x10
 #define BDRV_BLOCK_EOF  0x20
 #define BDRV_BLOCK_RECURSE  0x40
-#define BDRV_BLOCK_OFFSET_MASK  BDRV_SECTOR_MASK
 
 typedef QSIMPLEQ_HEAD(BlockReopenQueue, BlockReopenQueueEntry) 
BlockReopenQueue;
 
diff --git a/migration/block.c b/migration/block.c
index aa747b55fa..92c36b68ec 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -906,7 +906,7 @@ static int block_load(QEMUFile *f, void *opaque, int 
version_id)
 do {
 addr = qemu_get_be64(f);
 
-flags = addr & ~BDRV_SECTOR_MASK;
+flags = addr & (BDRV_SECTOR_SIZE - 1);
 addr >>= BDRV_SECTOR_BITS;
 
 if (flags & BLK_MIG_FLAG_DEVICE_BLOCK) {
-- 
2.20.1

Re: [Qemu-devel] [PATCH v6 1/3] target/ppc: Optimize emulation of vpkpx instruction

[Richard Henderson]

On 8/27/19 2:37 AM, Stefan Brankovic wrote:
> +for (i = 0; i < 4; i++) {
> +switch (i) {
> +case 0:
> +/*
> + * Get high doubleword of vA to perfrom 6-5-5 pack of pixels
> + * 1 and 2.
> + */
> +get_avr64(avr, VA, true);
> +tcg_gen_movi_i64(result, 0x0ULL);
> +break;
> +case 1:
> +/*
> + * Get low doubleword of vA to perfrom 6-5-5 pack of pixels
> + * 3 and 4.
> + */
> +get_avr64(avr, VA, false);
> +break;
> +case 2:
> +/*
> + * Get high doubleword of vB to perfrom 6-5-5 pack of pixels
> + * 5 and 6.
> + */
> +get_avr64(avr, VB, true);
> +tcg_gen_movi_i64(result, 0x0ULL);
> +break;
> +case 3:
> +/*
> + * Get low doubleword of vB to perfrom 6-5-5 pack of pixels
> + * 7 and 8.
> + */
> +get_avr64(avr, VB, false);
> +break;
> +}
> +/* Perform the packing for 2 pixels(each iteration for 1). */
> +tcg_gen_movi_i64(tmp, 0x0ULL);
> +for (j = 0; j < 2; j++) {
> +tcg_gen_shri_i64(shifted, avr, (j * 16 + 3));
> +tcg_gen_andi_i64(shifted, shifted, mask1 << (j * 16));
> +tcg_gen_or_i64(tmp, tmp, shifted);
> +
> +tcg_gen_shri_i64(shifted, avr, (j * 16 + 6));
> +tcg_gen_andi_i64(shifted, shifted, mask2 << (j * 16));
> +tcg_gen_or_i64(tmp, tmp, shifted);
> +
> +tcg_gen_shri_i64(shifted, avr, (j * 16 + 9));
> +tcg_gen_andi_i64(shifted, shifted, mask3 << (j * 16));
> +tcg_gen_or_i64(tmp, tmp, shifted);
> +}
> +if ((i == 0) || (i == 2)) {
> +tcg_gen_shli_i64(tmp, tmp, 32);
> +}
> +tcg_gen_or_i64(result, result, tmp);
> +if (i == 1) {
> +/* Place packed pixels 1:4 to high doubleword of vD. */
> +tcg_gen_mov_i64(result1, result);
> +}
> +if (i == 3) {
> +/* Place packed pixels 5:8 to low doubleword of vD. */
> +tcg_gen_mov_i64(result2, result);
> +}
> +}
> +set_avr64(VT, result1, true);
> +set_avr64(VT, result2, false);

I really have a hard time believing that it is worthwhile to inline all of this
code.  By my count this is 82 non-move opcodes.  That is a *lot* of inline
expansion.

However, I can well imagine that the existing out-of-line helper is less than
optimal.

> -void helper_vpkpx(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
> -{
> -int i, j;
> -ppc_avr_t result;
> -#if defined(HOST_WORDS_BIGENDIAN)
> -const ppc_avr_t *x[2] = { a, b };
> -#else
> -const ppc_avr_t *x[2] = { b, a };
> -#endif
> -
> -VECTOR_FOR_INORDER_I(i, u64) {
> -VECTOR_FOR_INORDER_I(j, u32) {
> -uint32_t e = x[i]->u32[j];

Double indirect loads?

> -
> -result.u16[4 * i + j] = (((e >> 9) & 0xfc00) |
> - ((e >> 6) & 0x3e0) |
> - ((e >> 3) & 0x1f));

Store to temporary ...

> -}
> -}
> -*r = result;

... and then copy?

Try replacing the existing helper with something like the following.


r~



static inline uint64_t pkpx_1(uint64_t a, int shr, int shl)
{
uint64_t r;

r  = ((a >> (shr + 9)) & 0x3f) << shl;
r |= ((a >> (shr + 6)) & 0x1f) << shl;
r |= ((a >> (shr + 3)) & 0x1f) << shl;

return r;
}

static inline uint64_t pkpx_2(uint64_t ah, uint64_t al)
{
return pkpx_1(ah, 32, 48)
 | pkpx_1(ah,  0, 32)
 | pkpx_1(al, 32, 16)
 | pkpx_1(al,  0,  0);
}

void helper_vpkpx(uint64_t *r, uint64_t *a, uint64_t *b)
{
uint64_t rh = pkpx_2(a->VsrD(0), a->VsrD(1));
uint64_t rl = pkpx_2(b->VsrD(0), b->VsrD(1));
r->VsrD(0) = rh;
r->VsrD(1) = rl;
}

Re: [Qemu-devel] [PATCH] iotests: Unify cache mode quoting

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190827173432.7656-1-nsof...@redhat.com/



Hi,

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

Message-id: 20190827173432.7656-1-nsof...@redhat.com
Type: series
Subject: [Qemu-devel] [PATCH] iotests: Unify cache mode quoting

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
f682fe9 iotests: Unify cache mode quoting

=== OUTPUT BEGIN ===
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 38 lines checked

Commit f682fe9cf33c (iotests: Unify cache mode quoting) has style problems, 
please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190827173432.7656-1-nsof...@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-devel] [edk2-rfc] [edk2-devel] CPU hotplug using SMM with QEMU+OVMF

[Igor Mammedov]

On Sat, 24 Aug 2019 01:48:09 +
"Yao, Jiewen"  wrote:

> I give my thought.
> Paolo may add more.
Here are some ideas I have on the topic.

> 
> > -Original Message-
> > From: Kinney, Michael D
> > Sent: Friday, August 23, 2019 11:25 PM
> > To: Yao, Jiewen ; Paolo Bonzini
> > ; Laszlo Ersek ;
> > r...@edk2.groups.io; Kinney, Michael D 
> > Cc: Alex Williamson ; de...@edk2.groups.io;
> > qemu devel list ; Igor Mammedov
> > ; Chen, Yingwen ;
> > Nakajima, Jun ; Boris Ostrovsky
> > ; Joao Marcal Lemos Martins
> > ; Phillip Goerl 
> > Subject: RE: [edk2-rfc] [edk2-devel] CPU hotplug using SMM with
> > QEMU+OVMF
> > 
> > Hi Jiewen,
> > 
> > If a hot add CPU needs to run any code before the
> > first SMI, I would recommend is only executes code
> > from a write protected FLASH range without a stack
> > and then wait for the first SMI.  
> [Jiewen] Right.
> 
> Another option from Paolo, the new CPU will not run until 0x7b.
> To mitigate DMA threat, someone need guarantee the low memory SIPI vector is 
> DMA protected.
> 
> NOTE: The LOW memory *could* be mapped to write protected FLASH AREA via PAM 
> register. The Host CPU may setup that in SMM.
> If that is the case, we don’t need worry DMA.
> 
> I copied the detail step here, because I found it is hard to dig them out 
> again.

*) In light of using dedicated SMRAM at 3 with pre-configured
relocation vector for initial relocation which is not reachable from
non-SMM mode:

> 
> (01a) QEMU: create new CPU.  The CPU already exists, but it does not
>  start running code until unparked by the CPU hotplug controller.
we might not need parked CPU (if we ignore attacker's attempt to send
SMI to several new CPUs, see below for issue it causes)

> (01b) QEMU: trigger SCI
> 
> (02-03) no equivalent
> 
> (04) Host CPU: (OS) execute GPE handler from DSDT
> 
> (05) Host CPU: (OS) Port 0xB2 write, all CPUs enter SMM (NOTE: New CPU
>  will not enter CPU because SMI is disabled)
I think only CPU that does the write will enter SMM
and we might not need to pull in all already initialized CPUs into SMM.

At this step we could also send a directed SMI to a new CPU from host
CPU that entered SMM on write.

> (06) Host CPU: (SMM) Save 38000, Update 38000 -- fill simple SMM
>  rebase code.
could skip this step as well (*)


> (07a) Host CPU: (SMM) Write to CPU hotplug controller to enable
>  new CPU
ditto
 
> (07b) Host CPU: (SMM) Send INIT/SIPI/SIPI to new CPU.
we need to wake up new CPU somehow so it would process (09) pending (05) SMI
before jumping to SIPI vector

> (08a) New CPU: (Low RAM) Enter protected mode.
> 
> (08b) New CPU: (Flash) Signals host CPU to proceed and enter cli;hlt loop.

these both steps could be changed to to just cli;hlt loop or do INIT reset.
if SMI relocation handler and/or host CPU will pull in the new CPU into OVMF,
we actually don't care about SIPI vector as all firmware initialization
for the new CPU is done in SMM mode (07b triggers 10).
Thus eliminating one attack vector to protect from.

> (09) Host CPU: (SMM) Send SMI to the new CPU only.
could be done at (05)
 
> (10) New CPU: (SMM) Run SMM code at 38000, and rebase SMBASE to
>  TSEG.
it could also pull in itself into other OVMF structures
(assuming it can TSEG as stack as that's rather complex) or
just do relocation and let host CPU to fill in OVMF structures for the new CPU 
(12).

> (11) Host CPU: (SMM) Restore 38000.
could skip this step as well (*)

> (12) Host CPU: (SMM) Update located data structure to add the new CPU
>  information. (This step will involve CPU_SERVICE protocol)
> 
> (13) New CPU: (Flash) do whatever other initialization is needed
do we actually need it?

> (14) New CPU: (Flash) Deadloop, and wait for INIT-SIPI-SIPI.
> 
> (15) Host CPU: (OS) Send INIT-SIPI-SIPI to pull new CPU in..
> 
> 
> > 
> > For this OVMF use case, is any CPU init required
> > before the first SMI?  
> [Jiewen] I am sure what is the detail action in 08b.
> And I am not sure what your "init" means here?
> Personally, I don’t think we need too much init work, such as Microcode or 
> MTRR.
> But we need detail info.
Wouldn't it be preferable to do in SMM mode?

> > From Paolo's list of steps are steps (8a) and (8b)
> > really required?  Can the SMI monarch use the Local
> > APIC to send a directed SMI to the hot added CPU?
> > The SMI monarch needs to know the APIC ID of the
> > hot added CPU.
> [Jiewen] I think it depend upon virtual hardware design.
> Leave question to Paolo.

it's not really needed as described in (8x), it could be just
cli;hlt loop so that our SIPI could land at sensible code and stop the new CPU,
it even could be an attacker's code if we do all initialization in SMM mode.

> Do we also need to handle the case
> > where multiple CPUs are added at once?  I think we
> > would need to serialize the use of 3000:8000 for the
> > SMM rebase operation on each hot added CPU.
> > It would be simpler if we can 

[Qemu-devel] [PULL 13/15] iotests: Check for enabled drivers before testing them

[Max Reitz]

From: Thomas Huth 

It is possible to enable only a subset of the block drivers with the
"--block-drv-rw-whitelist" option of the "configure" script. All other
drivers are marked as unusable (or only included as read-only with the
"--block-drv-ro-whitelist" option). If an iotest is now using such a
disabled block driver, it is failing - which is bad, since at least the
tests in the "auto" group should be able to deal with this situation.
Thus let's introduce a "_require_drivers" function that can be used by
the shell tests to check for the availability of certain drivers first,
and marks the test as "not run" if one of the drivers is missing.

This patch mainly targets the test in the "auto" group which should
never fail in such a case, but also improves some of the other tests
along the way. Note that we also assume that the "qcow2" and "file"
drivers are always available - otherwise it does not make sense to
run "make check-block" at all (which only tests with qcow2 by default).

Signed-off-by: Thomas Huth 
Message-id: 20190823133552.11680-1-th...@redhat.com
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/071   |  1 +
 tests/qemu-iotests/081   |  4 +---
 tests/qemu-iotests/099   |  1 +
 tests/qemu-iotests/120   |  1 +
 tests/qemu-iotests/162   |  4 +---
 tests/qemu-iotests/184   |  1 +
 tests/qemu-iotests/186   |  1 +
 tests/qemu-iotests/common.rc | 14 ++
 8 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/071 b/tests/qemu-iotests/071
index 1cca9233d0..fab52b 100755
--- a/tests/qemu-iotests/071
+++ b/tests/qemu-iotests/071
@@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt qcow2
 _supported_proto file
+_require_drivers blkdebug blkverify
 
 do_run_qemu()
 {
diff --git a/tests/qemu-iotests/081 b/tests/qemu-iotests/081
index c418bab093..85acdf76d4 100755
--- a/tests/qemu-iotests/081
+++ b/tests/qemu-iotests/081
@@ -41,6 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt raw
 _supported_proto file
 _supported_os Linux
+_require_drivers quorum
 
 do_run_qemu()
 {
@@ -55,9 +56,6 @@ run_qemu()
   | _filter_qemu_io | _filter_generated_node_ids
 }
 
-test_quorum=$($QEMU_IMG --help|grep quorum)
-[ "$test_quorum" = "" ] && _supported_fmt quorum
-
 quorum="driver=raw,file.driver=quorum,file.vote-threshold=2"
 quorum="$quorum,file.children.0.file.filename=$TEST_DIR/1.raw"
 quorum="$quorum,file.children.1.file.filename=$TEST_DIR/2.raw"
diff --git a/tests/qemu-iotests/099 b/tests/qemu-iotests/099
index ae02f27afe..c3cf66798a 100755
--- a/tests/qemu-iotests/099
+++ b/tests/qemu-iotests/099
@@ -42,6 +42,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow qcow2 qed vdi vhdx vmdk vpc
 _supported_proto file
 _supported_os Linux
+_require_drivers blkdebug blkverify
 _unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat" 
\
 "subformat=twoGbMaxExtentSparse"
 
diff --git a/tests/qemu-iotests/120 b/tests/qemu-iotests/120
index e9b4fbb009..2931a7550f 100755
--- a/tests/qemu-iotests/120
+++ b/tests/qemu-iotests/120
@@ -40,6 +40,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt generic
 _supported_proto file
 _unsupported_fmt luks
+_require_drivers raw
 
 _make_test_img 64M
 
diff --git a/tests/qemu-iotests/162 b/tests/qemu-iotests/162
index 4e5ed74fd5..2d719afbed 100755
--- a/tests/qemu-iotests/162
+++ b/tests/qemu-iotests/162
@@ -39,9 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.filter
 
 _supported_fmt generic
-
-test_ssh=$($QEMU_IMG --help | grep '^Supported formats:.* ssh\( \|$\)')
-[ "$test_ssh" = "" ] && _notrun "ssh support required"
+_require_drivers ssh
 
 echo
 echo '=== NBD ==='
diff --git a/tests/qemu-iotests/184 b/tests/qemu-iotests/184
index cb0c181228..33dd8d2a4f 100755
--- a/tests/qemu-iotests/184
+++ b/tests/qemu-iotests/184
@@ -33,6 +33,7 @@ trap "exit \$status" 0 1 2 3 15
 . ./common.filter
 
 _supported_os Linux
+_require_drivers throttle
 
 do_run_qemu()
 {
diff --git a/tests/qemu-iotests/186 b/tests/qemu-iotests/186
index 5f6b18c150..3ea0442d44 100755
--- a/tests/qemu-iotests/186
+++ b/tests/qemu-iotests/186
@@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt qcow2
 _supported_proto file
+_require_drivers null-co
 
 if [ "$QEMU_DEFAULT_MACHINE" != "pc" ]; then
 _notrun "Requires a PC machine"
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 5502c3da2f..ee20be8920 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -520,5 +520,19 @@ _require_command()
 [ -x "$c" ] || _notrun "$1 utility required, skipped this test"
 }
 
+# Check that a set of drivers has been whitelisted in the QEMU binary
+#
+_require_drivers()
+{
+available=$($QEMU -drive format=help | \
+sed -e '/Supported formats:/!d' -e 's/Supported formats://')
+for driver
+do
+if ! echo "$available" | grep -q 

[Qemu-devel] [PULL 10/15] iotests: Disable 110 for vmdk.twoGbMaxExtentSparse

[Max Reitz]

The error message for the test case where we have a quorum node for
which no directory name can be generated is different: For
twoGbMaxExtentSparse, it complains that it cannot open the extent file.
For other (sub)formats, it just notes that it cannot determine the
backing file path.  Both are fine, but just disable twoGbMaxExtentSparse
for simplicity's sake.

Signed-off-by: Max Reitz 
Reviewed-by: John Snow 
Message-id: 20190815153638.4600-7-mre...@redhat.com
Reviewed-by: John Snow 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/110 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/110 b/tests/qemu-iotests/110
index 2cdc7c8a72..2ef516baf1 100755
--- a/tests/qemu-iotests/110
+++ b/tests/qemu-iotests/110
@@ -40,7 +40,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 # Any format supporting backing files
 _supported_fmt qed qcow qcow2 vmdk
 _supported_proto file
-_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat"
+_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat" 
\
+ "subformat=twoGbMaxExtentSparse"
 
 TEST_IMG_REL=$(basename "$TEST_IMG")
 
-- 
2.21.0

[Qemu-devel] [PULL 15/15] iotests: Unify cache mode quoting

[Max Reitz]

From: Nir Soffer 

Quoting cache mode is not needed, and most tests use unquoted values.
Unify all test to use the same style.

Message-id: 20190827173432.7656-1-nsof...@redhat.com
Signed-off-by: Nir Soffer 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/026 | 4 ++--
 tests/qemu-iotests/039 | 4 ++--
 tests/qemu-iotests/052 | 2 +-
 tests/qemu-iotests/091 | 4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026
index e30243608b..ffb18ab6b5 100755
--- a/tests/qemu-iotests/026
+++ b/tests/qemu-iotests/026
@@ -41,8 +41,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 # Currently only qcow2 supports rebasing
 _supported_fmt qcow2
 _supported_proto file
-_default_cache_mode "writethrough"
-_supported_cache_modes "writethrough" "none"
+_default_cache_mode writethrough
+_supported_cache_modes writethrough none
 # The refcount table tests expect a certain minimum width for refcount entries
 # (so that the refcount table actually needs to grow); that minimum is 16 bits,
 # being the default refcount entry width.
diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
index 0d4e963bd4..7c730d94a7 100755
--- a/tests/qemu-iotests/039
+++ b/tests/qemu-iotests/039
@@ -42,8 +42,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow2
 _supported_proto file
 _supported_os Linux
-_default_cache_mode "writethrough"
-_supported_cache_modes "writethrough"
+_default_cache_mode writethrough
+_supported_cache_modes writethrough
 
 size=128M
 
diff --git a/tests/qemu-iotests/052 b/tests/qemu-iotests/052
index 6e2ecbfe21..45a140910d 100755
--- a/tests/qemu-iotests/052
+++ b/tests/qemu-iotests/052
@@ -40,7 +40,7 @@ _supported_fmt generic
 _supported_proto file
 
 # Don't do O_DIRECT on tmpfs
-_supported_cache_modes "writeback" "writethrough" "unsafe"
+_supported_cache_modes writeback writethrough unsafe
 
 size=128M
 _make_test_img $size
diff --git a/tests/qemu-iotests/091 b/tests/qemu-iotests/091
index d62ef18a02..f4b44659ae 100755
--- a/tests/qemu-iotests/091
+++ b/tests/qemu-iotests/091
@@ -46,8 +46,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow2
 _supported_proto file
 _supported_os Linux
-_default_cache_mode "none"
-_supported_cache_modes "writethrough" "none" "writeback"
+_default_cache_mode none
+_supported_cache_modes writethrough none writeback
 
 size=1G
 
-- 
2.21.0

[Qemu-devel] [PULL 08/15] vmdk: Reject invalid compressed writes

[Max Reitz]

Compressed writes generally have to write full clusters, not just in
theory but also in practice when it comes to vmdk's streamOptimized
subformat.  It currently is just silently broken for writes with
non-zero in-cluster offsets:

$ qemu-img create -f vmdk -o subformat=streamOptimized foo.vmdk 1M
$ qemu-io -c 'write 4k 4k' -c 'read 4k 4k' foo.vmdk
wrote 4096/4096 bytes at offset 4096
4 KiB, 1 ops; 00.01 sec (443.724 KiB/sec and 110.9309 ops/sec)
read failed: Invalid argument

(The technical reason is that vmdk_write_extent() just writes the
incomplete compressed data actually to offset 4k.  When reading the
data, vmdk_read_extent() looks at offset 0 and finds the compressed data
size to be 0, because that is what it reads from there.  This yields an
error.)

For incomplete writes with zero in-cluster offsets, the error path when
reading the rest of the cluster is a bit different, but the result is
the same:

$ qemu-img create -f vmdk -o subformat=streamOptimized foo.vmdk 1M
$ qemu-io -c 'write 0k 4k' -c 'read 4k 4k' foo.vmdk
wrote 4096/4096 bytes at offset 0
4 KiB, 1 ops; 00.01 sec (362.641 KiB/sec and 90.6603 ops/sec)
read failed: Invalid argument

(Here, vmdk_read_extent() finds the data and then sees that the
uncompressed data is short.)

It is better to reject invalid writes than to make the user believe they
might have succeeded and then fail when trying to read it back.

Signed-off-by: Max Reitz 
Reviewed-by: John Snow 
Message-id: 20190815153638.4600-5-mre...@redhat.com
Reviewed-by: John Snow 
Signed-off-by: Max Reitz 
---
 block/vmdk.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/block/vmdk.c b/block/vmdk.c
index a7f82e665e..fed3b50c8a 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1734,6 +1734,16 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t 
cluster_offset,
 if (extent->compressed) {
 void *compressed_data;
 
+/* Only whole clusters */
+if (offset_in_cluster ||
+n_bytes > (extent->cluster_sectors * SECTOR_SIZE) ||
+(n_bytes < (extent->cluster_sectors * SECTOR_SIZE) &&
+ offset + n_bytes != extent->end_sector * SECTOR_SIZE))
+{
+ret = -EINVAL;
+goto out;
+}
+
 if (!extent->has_marker) {
 ret = -EINVAL;
 goto out;
-- 
2.21.0

[Qemu-devel] [PULL 12/15] file-posix: fix request_alignment typo

[Max Reitz]

From: Stefan Hajnoczi 

Fixes: a6b257a08e3d72219f03e461a52152672fec0612
   ("file-posix: Handle undetectable alignment")
Signed-off-by: Stefan Hajnoczi 
Message-id: 20190827101328.4062-1-stefa...@redhat.com
Reviewed-by: Thomas Huth 
Signed-off-by: Max Reitz 
---
 block/file-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 447f937aa1..71f168ee2f 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -380,7 +380,7 @@ static void raw_probe_alignment(BlockDriverState *bs, int 
fd, Error **errp)
 for (i = 0; i < ARRAY_SIZE(alignments); i++) {
 align = alignments[i];
 if (raw_is_io_aligned(fd, buf + align, max_align)) {
-/* Fallback to request_aligment. */
+/* Fallback to request_alignment. */
 s->buf_align = (align != 1) ? align : bs->bl.request_alignment;
 break;
 }
-- 
2.21.0

[Qemu-devel] [PULL 07/15] iotests: Keep testing broken relative extent paths

[Max Reitz]

We had a test for a case where relative extent paths did not work, but
unfortunately we just fixed the underlying problem, so it works now.
This patch adds a new test case that still fails.

Signed-off-by: Max Reitz 
Reviewed-by: John Snow 
Message-id: 20190815153638.4600-4-mre...@redhat.com
Reviewed-by: John Snow 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/059 | 27 +++
 tests/qemu-iotests/059.out |  4 
 2 files changed, 31 insertions(+)

diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index fbed5f9483..10bfbaecec 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -114,6 +114,8 @@ $QEMU_IMG convert -f qcow2 -O vmdk -o 
subformat=streamOptimized "$TEST_IMG.qcow2
 
 echo
 echo "=== Testing monolithicFlat with internally generated JSON file name ==="
+
+echo '--- blkdebug ---'
 # Should work, because bdrv_dirname() works fine with blkdebug
 IMGOPTS="subformat=monolithicFlat" _make_test_img 64M
 $QEMU_IO -c "open -o 
driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TEST_IMG,file.inject-error.0.event=read_aio"
 \
@@ -122,6 +124,31 @@ $QEMU_IO -c "open -o 
driver=$IMGFMT,file.driver=blkdebug,file.image.filename=$TE
 | _filter_testdir | _filter_imgfmt | _filter_img_info
 _cleanup_test_img
 
+echo '--- quorum ---'
+# Should not work, because bdrv_dirname() does not work with quorum
+IMGOPTS="subformat=monolithicFlat" _make_test_img 64M
+cp "$TEST_IMG" "$TEST_IMG.orig"
+
+filename="json:{
+\"driver\": \"$IMGFMT\",
+\"file\": {
+\"driver\": \"quorum\",
+\"children\": [ {
+\"driver\": \"file\",
+\"filename\": \"$TEST_IMG\"
+}, {
+\"driver\": \"file\",
+\"filename\": \"$TEST_IMG.orig\"
+} ],
+\"vote-threshold\": 1
+} }"
+
+filename=$(echo "$filename" | tr '\n' ' ' | sed -e 's/\s\+/ /g')
+$QEMU_IMG info "$filename" 2>&1 \
+| sed -e "s/'json:[^']*'/\$QUORUM_FILE/g" \
+| _filter_testdir | _filter_imgfmt | _filter_img_info
+
+
 echo
 echo "=== Testing version 3 ==="
 _use_sample_img iotest-version3.vmdk.bz2
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index a51b571d27..39bf7e211d 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -2049,10 +2049,14 @@ wrote 512/512 bytes at offset 10240
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Testing monolithicFlat with internally generated JSON file name ===
+--- blkdebug ---
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 format name: IMGFMT
 cluster size: 0 bytes
 vm state offset: 0 bytes
+--- quorum ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+qemu-img: Could not open $QUORUM_FILE: Cannot use relative paths with VMDK 
descriptor file $QUORUM_FILE: Cannot generate a base directory for quorum nodes
 
 === Testing version 3 ===
 image: TEST_DIR/iotest-version3.IMGFMT
-- 
2.21.0

[Qemu-devel] [PULL 14/15] tests/check-block: Skip iotests when sanitizers are enabled

[Max Reitz]

From: Thomas Huth 

The sanitizers (especially the address sanitizer from Clang) are
sometimes printing out warnings or false positives - this spoils
the output of the iotests, causing some of the tests to fail.
Thus let's skip the automatic iotests during "make check" when the
user configured QEMU with --enable-sanitizers.

Signed-off-by: Thomas Huth 
Message-id: 20190823084203.29734-1-th...@redhat.com
Signed-off-by: Max Reitz 
---
 tests/check-block.sh | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tests/check-block.sh b/tests/check-block.sh
index c8b6cec3f6..679aedec50 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -21,6 +21,11 @@ if grep -q "TARGET_GPROF=y" *-softmmu/config-target.mak 
2>/dev/null ; then
 exit 0
 fi
 
+if grep -q "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null ; then
+echo "Sanitizers are enabled ==> Not running the qemu-iotests."
+exit 0
+fi
+
 if [ -z "$(find . -name 'qemu-system-*' -print)" ]; then
 echo "No qemu-system binary available ==> Not running the qemu-iotests."
 exit 0
-- 
2.21.0

[Qemu-devel] [PULL 04/15] iotests: Test allocate_first_block() with O_DIRECT

[Max Reitz]

From: Nir Soffer 

Using block_resize we can test allocate_first_block() with file
descriptor opened with O_DIRECT, ensuring that it works for any size
larger than 4096 bytes.

Testing smaller sizes is tricky as the result depends on the filesystem
used for testing. For example on NFS any size will work since O_DIRECT
does not require any alignment.

Signed-off-by: Nir Soffer 
Reviewed-by: Max Reitz 
Message-id: 20190827010528.8818-3-nsof...@redhat.com
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/175 | 28 
 tests/qemu-iotests/175.out |  8 
 2 files changed, 36 insertions(+)

diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
index 7ba28b3c1b..55db2803ed 100755
--- a/tests/qemu-iotests/175
+++ b/tests/qemu-iotests/175
@@ -49,6 +49,23 @@ _filter_blocks()
 -e "s/blocks=$((extra_blocks + img_size / 512))\\(\$\\|[^0-9]\\)/max 
allocation/"
 }
 
+# Resize image using block_resize.
+# Parameter 1: image path
+# Parameter 2: new size
+_block_resize()
+{
+local path=$1
+local size=$2
+
+$QEMU -qmp stdio -nographic -nodefaults \
+-blockdev file,node-name=file,filename=$path,cache.direct=on \
+<

[Qemu-devel] [PULL 11/15] iotests: Disable 126 for flat vmdk subformats

[Max Reitz]

iotest 126 requires backing file support, which flat vmdks cannot offer.
Skip this test for such subformats.

Signed-off-by: Max Reitz 
Message-id: 20190815153638.4600-8-mre...@redhat.com
Reviewed-by: John Snow 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/126 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qemu-iotests/126 b/tests/qemu-iotests/126
index 9b0dcf9255..b7fce1e59d 100755
--- a/tests/qemu-iotests/126
+++ b/tests/qemu-iotests/126
@@ -33,6 +33,8 @@ status=1  # failure is the default!
 
 # Needs backing file support
 _supported_fmt qcow qcow2 qed vmdk
+_unsupported_imgopts "subformat=monolithicFlat" \
+ "subformat=twoGbMaxExtentFlat"
 # This is the default protocol (and we want to test the difference between
 # colons which separate a protocol prefix from the rest and colons which are
 # just part of the filename, so we cannot test protocols which require a 
prefix)
-- 
2.21.0

[Qemu-devel] [PULL 05/15] iotests: Fix _filter_img_create()

[Max Reitz]

fe646693acc changed qemu-img create's output so that it no longer prints
single quotes around parameter values.  The subformat and adapter_type
filters in _filter_img_create() have never been adapted to that change.

Fixes: fe646693acc13ac48b98435d14149ab04dc597bc
Signed-off-by: Max Reitz 
Reviewed-by: John Snow 
Message-id: 20190815153638.4600-2-mre...@redhat.com
Reviewed-by: John Snow 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/059.out   | 16 
 tests/qemu-iotests/common.filter |  4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index fe3f861f3c..b2e718d29f 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -13,17 +13,17 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 qemu-io: can't open device TEST_DIR/t.vmdk: L1 size too big
 
 === Testing monolithicFlat creation and opening ===
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648 
subformat=monolithicFlat
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648
 image: TEST_DIR/t.IMGFMT
 file format: IMGFMT
 virtual size: 2 GiB (2147483648 bytes)
 
 === Testing monolithicFlat with zeroed_grain ===
 qemu-img: TEST_DIR/t.IMGFMT: Flat image can't enable zeroed grain
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648 
subformat=monolithicFlat
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648
 
 === Testing big twoGbMaxExtentFlat ===
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824000 
subformat=twoGbMaxExtentFlat
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824000
 image: TEST_DIR/t.vmdk
 file format: vmdk
 virtual size: 0.977 TiB (1073741824000 bytes)
@@ -2038,7 +2038,7 @@ Format specific information:
 qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Invalid extent line: RW 12582912 
VMFS "dummy.IMGFMT" 1
 
 === Testing truncated sparse ===
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=107374182400 
subformat=monolithicSparse
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=107374182400
 qemu-img: Could not open 'TEST_DIR/t.IMGFMT': File truncated, expecting at 
least 13172736 bytes
 
 === Converting to streamOptimized from image with small cluster size===
@@ -2049,7 +2049,7 @@ wrote 512/512 bytes at offset 10240
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Testing monolithicFlat with internally generated JSON file name ===
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
subformat=monolithicFlat
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 qemu-io: can't open: Cannot use relative extent paths with VMDK descriptor 
file 'json:{"image": {"driver": "file", "filename": "TEST_DIR/t.IMGFMT"}, 
"driver": "blkdebug", "inject-error.0.event": "read_aio"}'
 
 === Testing version 3 ===
@@ -2259,7 +2259,7 @@ read 512/512 bytes at offset 64931328
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Testing 4TB monolithicFlat creation and IO ===
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4398046511104 
subformat=monolithicFlat
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4398046511104
 image: TEST_DIR/t.IMGFMT
 file format: IMGFMT
 virtual size: 4 TiB (4398046511104 bytes)
@@ -2333,7 +2333,7 @@ read 1024/1024 bytes at offset 966367641600
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Testing qemu-img map on extents ===
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33285996544 
subformat=monolithicSparse
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33285996544
 wrote 1024/1024 bytes at offset 65024
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 wrote 1024/1024 bytes at offset 2147483136
@@ -2344,7 +2344,7 @@ Offset  Length  Mapped to   File
 0   0x2 0x3fTEST_DIR/t.vmdk
 0x7fff  0x2 0x41TEST_DIR/t.vmdk
 0x14000 0x1 0x43TEST_DIR/t.vmdk
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33285996544 
subformat=twoGbMaxExtentSparse
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33285996544
 wrote 1024/1024 bytes at offset 65024
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 wrote 1024/1024 bytes at offset 2147483136
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 8e9235d6fe..445a1c23e0 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -130,8 +130,8 @@ _filter_img_create()
 -e "s# compat6=\\(on\\|off\\)##g" \
 -e "s# static=\\(on\\|off\\)##g" \
 -e "s# zeroed_grain=\\(on\\|off\\)##g" \
--e "s# subformat='[^']*'##g" \
--e "s# adapter_type='[^']*'##g" \
+-e "s# subformat=[^ ]*##g" \
+-e "s# adapter_type=[^ ]*##g" \
 -e "s# hwversion=[^ ]*##g" \
 -e "s# lazy_refcounts=\\(on\\|off\\)##g" \
 -e "s# block_size=[0-9]\\+##g" \
-- 
2.21.0

[Qemu-devel] [PULL 06/15] vmdk: Use bdrv_dirname() for relative extent paths

[Max Reitz]

This makes iotest 033 pass with e.g. subformat=monolithicFlat.  It also
turns a former error in 059 into success.

Signed-off-by: Max Reitz 
Message-id: 20190815153638.4600-3-mre...@redhat.com
Reviewed-by: John Snow 
Signed-off-by: Max Reitz 
---
 block/vmdk.c   | 54 --
 tests/qemu-iotests/059 |  7 +++--
 tests/qemu-iotests/059.out |  4 ++-
 3 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index fd78fd0ccf..a7f82e665e 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1076,8 +1076,7 @@ static const char *next_line(const char *s)
 }
 
 static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
-  const char *desc_file_path, QDict *options,
-  Error **errp)
+  QDict *options, Error **errp)
 {
 int ret;
 int matches;
@@ -1087,6 +1086,7 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
 const char *p, *np;
 int64_t sectors = 0;
 int64_t flat_offset;
+char *desc_file_dir = NULL;
 char *extent_path;
 BdrvChild *extent_file;
 BDRVVmdkState *s = bs->opaque;
@@ -1130,16 +1130,23 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
 continue;
 }
 
-if (!path_is_absolute(fname) && !path_has_protocol(fname) &&
-!desc_file_path[0])
-{
-bdrv_refresh_filename(bs->file->bs);
-error_setg(errp, "Cannot use relative extent paths with VMDK "
-   "descriptor file '%s'", bs->file->bs->filename);
-return -EINVAL;
-}
+if (path_is_absolute(fname)) {
+extent_path = g_strdup(fname);
+} else {
+if (!desc_file_dir) {
+desc_file_dir = bdrv_dirname(bs->file->bs, errp);
+if (!desc_file_dir) {
+bdrv_refresh_filename(bs->file->bs);
+error_prepend(errp, "Cannot use relative paths with VMDK "
+  "descriptor file '%s': ",
+  bs->file->bs->filename);
+ret = -EINVAL;
+goto out;
+}
+}
 
-extent_path = path_combine(desc_file_path, fname);
+extent_path = g_strconcat(desc_file_dir, fname, NULL);
+}
 
 ret = snprintf(extent_opt_prefix, 32, "extents.%d", s->num_extents);
 assert(ret < 32);
@@ -1149,7 +1156,8 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
 g_free(extent_path);
 if (local_err) {
 error_propagate(errp, local_err);
-return -EINVAL;
+ret = -EINVAL;
+goto out;
 }
 
 /* save to extents array */
@@ -1160,7 +1168,7 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
 0, 0, 0, 0, 0, , errp);
 if (ret < 0) {
 bdrv_unref_child(bs, extent_file);
-return ret;
+goto out;
 }
 extent->flat_start_offset = flat_offset << 9;
 } else if (!strcmp(type, "SPARSE") || !strcmp(type, "VMFSSPARSE")) {
@@ -1175,24 +1183,27 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
 g_free(buf);
 if (ret) {
 bdrv_unref_child(bs, extent_file);
-return ret;
+goto out;
 }
 extent = >extents[s->num_extents - 1];
 } else if (!strcmp(type, "SESPARSE")) {
 ret = vmdk_open_se_sparse(bs, extent_file, bs->open_flags, errp);
 if (ret) {
 bdrv_unref_child(bs, extent_file);
-return ret;
+goto out;
 }
 extent = >extents[s->num_extents - 1];
 } else {
 error_setg(errp, "Unsupported extent type '%s'", type);
 bdrv_unref_child(bs, extent_file);
-return -ENOTSUP;
+ret = -ENOTSUP;
+goto out;
 }
 extent->type = g_strdup(type);
 }
-return 0;
+
+ret = 0;
+goto out;
 
 invalid:
 np = next_line(p);
@@ -1201,7 +1212,11 @@ invalid:
 np--;
 }
 error_setg(errp, "Invalid extent line: %.*s", (int)(np - p), p);
-return -EINVAL;
+ret = -EINVAL;
+
+out:
+g_free(desc_file_dir);
+return ret;
 }
 
 static int vmdk_open_desc_file(BlockDriverState *bs, int flags, char *buf,
@@ -1228,8 +1243,7 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int 
flags, char *buf,
 }
 s->create_type = g_strdup(ct);
 s->desc_offset = 0;
-ret = vmdk_parse_extents(buf, bs, bs->file->bs->exact_filename, options,
- errp);
+ret = vmdk_parse_extents(buf, bs, options, errp);
 exit:
 return ret;
 }
diff --git 

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

[Max Reitz]

The following changes since commit 23919ddfd56135cad3cb468a8f54d5a595f024f4:

  Merge remote-tracking branch 'remotes/aperard/tags/pull-xen-20190827' into 
staging (2019-08-27 15:52:36 +0100)

are available in the Git repository at:

  https://github.com/XanClic/qemu.git tags/pull-block-2019-08-27

for you to fetch changes up to bb043c056cffcc2f3ce88bfdaf2e76e455c09e2c:

  iotests: Unify cache mode quoting (2019-08-27 19:48:44 +0200)


Block patches:
- qemu-io now accepts a file to read a write pattern from
- Ensure that raw files have their first block allocated so we can probe
  the O_DIRECT alignment if necessary
- Various fixes


Denis Plotnikov (1):
  qemu-io: add pattern file for write command

Max Reitz (7):
  iotests: Fix _filter_img_create()
  vmdk: Use bdrv_dirname() for relative extent paths
  iotests: Keep testing broken relative extent paths
  vmdk: Reject invalid compressed writes
  iotests: Disable broken streamOptimized tests
  iotests: Disable 110 for vmdk.twoGbMaxExtentSparse
  iotests: Disable 126 for flat vmdk subformats

Nir Soffer (3):
  block: posix: Always allocate the first block
  iotests: Test allocate_first_block() with O_DIRECT
  iotests: Unify cache mode quoting

Stefan Hajnoczi (1):
  file-posix: fix request_alignment typo

Thomas Huth (2):
  iotests: Check for enabled drivers before testing them
  tests/check-block: Skip iotests when sanitizers are enabled

Vladimir Sementsov-Ogievskiy (1):
  block: fix permission update in bdrv_replace_node

 block.c   |  5 +-
 block/file-posix.c| 53 +-
 block/vmdk.c  | 64 
 qemu-io-cmds.c| 99 +--
 tests/check-block.sh  |  5 +
 tests/qemu-iotests/002|  1 +
 tests/qemu-iotests/003|  1 +
 tests/qemu-iotests/005|  3 +-
 tests/qemu-iotests/009|  1 +
 tests/qemu-iotests/010|  1 +
 tests/qemu-iotests/011|  1 +
 tests/qemu-iotests/017|  3 +-
 tests/qemu-iotests/018|  3 +-
 tests/qemu-iotests/019|  3 +-
 tests/qemu-iotests/020|  3 +-
 tests/qemu-iotests/026|  4 +-
 tests/qemu-iotests/027|  1 +
 tests/qemu-iotests/032|  1 +
 tests/qemu-iotests/033|  1 +
 tests/qemu-iotests/034|  3 +-
 tests/qemu-iotests/037|  3 +-
 tests/qemu-iotests/039|  4 +-
 tests/qemu-iotests/052|  2 +-
 tests/qemu-iotests/059| 34 ++-
 tests/qemu-iotests/059.out| 26 +++--
 tests/qemu-iotests/063|  3 +-
 tests/qemu-iotests/071|  1 +
 tests/qemu-iotests/072|  1 +
 tests/qemu-iotests/081|  4 +-
 tests/qemu-iotests/091|  4 +-
 tests/qemu-iotests/099|  1 +
 tests/qemu-iotests/105|  3 +-
 tests/qemu-iotests/110|  3 +-
 tests/qemu-iotests/120|  1 +
 tests/qemu-iotests/126|  2 +
 tests/qemu-iotests/{150.out => 150.out.qcow2} |  0
 tests/qemu-iotests/150.out.raw| 12 +++
 tests/qemu-iotests/162|  4 +-
 tests/qemu-iotests/175| 47 +++--
 tests/qemu-iotests/175.out| 16 ++-
 tests/qemu-iotests/178.out.qcow2  |  4 +-
 tests/qemu-iotests/184|  1 +
 tests/qemu-iotests/186|  1 +
 tests/qemu-iotests/197|  1 +
 tests/qemu-iotests/215|  1 +
 tests/qemu-iotests/221.out| 12 ++-
 tests/qemu-iotests/251|  1 +
 tests/qemu-iotests/253.out| 12 ++-
 tests/qemu-iotests/common.filter  |  4 +-
 tests/qemu-iotests/common.rc  | 14 +++
 50 files changed, 391 insertions(+), 87 deletions(-)
 rename tests/qemu-iotests/{150.out => 150.out.qcow2} (100%)
 create mode 100644 tests/qemu-iotests/150.out.raw

-- 
2.21.0

[Qemu-devel] [PULL 09/15] iotests: Disable broken streamOptimized tests

[Max Reitz]

streamOptimized does not support writes that do not span exactly one
cluster.  Furthermore, it cannot rewrite already allocated clusters.
As such, many iotests do not work with it.  Disable them.

Signed-off-by: Max Reitz 
Message-id: 20190815153638.4600-6-mre...@redhat.com
Reviewed-by: John Snow 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/002 | 1 +
 tests/qemu-iotests/003 | 1 +
 tests/qemu-iotests/005 | 3 ++-
 tests/qemu-iotests/009 | 1 +
 tests/qemu-iotests/010 | 1 +
 tests/qemu-iotests/011 | 1 +
 tests/qemu-iotests/017 | 3 ++-
 tests/qemu-iotests/018 | 3 ++-
 tests/qemu-iotests/019 | 3 ++-
 tests/qemu-iotests/020 | 3 ++-
 tests/qemu-iotests/027 | 1 +
 tests/qemu-iotests/032 | 1 +
 tests/qemu-iotests/033 | 1 +
 tests/qemu-iotests/034 | 3 ++-
 tests/qemu-iotests/037 | 3 ++-
 tests/qemu-iotests/063 | 3 ++-
 tests/qemu-iotests/072 | 1 +
 tests/qemu-iotests/105 | 3 ++-
 tests/qemu-iotests/197 | 1 +
 tests/qemu-iotests/215 | 1 +
 tests/qemu-iotests/251 | 1 +
 21 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/tests/qemu-iotests/002 b/tests/qemu-iotests/002
index fd413bce48..1a0d411df5 100755
--- a/tests/qemu-iotests/002
+++ b/tests/qemu-iotests/002
@@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt generic
 _supported_proto generic
+_unsupported_imgopts "subformat=streamOptimized"
 
 
 size=128M
diff --git a/tests/qemu-iotests/003 b/tests/qemu-iotests/003
index ccd3a39dfb..33eeade0de 100755
--- a/tests/qemu-iotests/003
+++ b/tests/qemu-iotests/003
@@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt generic
 _supported_proto generic
+_unsupported_imgopts "subformat=streamOptimized"
 
 size=128M
 offset=67M
diff --git a/tests/qemu-iotests/005 b/tests/qemu-iotests/005
index 9c7681c19b..58442762fe 100755
--- a/tests/qemu-iotests/005
+++ b/tests/qemu-iotests/005
@@ -43,7 +43,8 @@ _supported_fmt generic
 _supported_proto generic
 _supported_os Linux
 _unsupported_imgopts "subformat=twoGbMaxExtentFlat" \
- "subformat=twoGbMaxExtentSparse"
+ "subformat=twoGbMaxExtentSparse" \
+ "subformat=streamOptimized"
 
 # vpc is limited to 127GB, so we can't test it here
 if [ "$IMGFMT" = "vpc" ]; then
diff --git a/tests/qemu-iotests/009 b/tests/qemu-iotests/009
index 51b200db1d..4dc7d210f9 100755
--- a/tests/qemu-iotests/009
+++ b/tests/qemu-iotests/009
@@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt generic
 _supported_proto generic
+_unsupported_imgopts "subformat=streamOptimized"
 
 
 size=6G
diff --git a/tests/qemu-iotests/010 b/tests/qemu-iotests/010
index 48c533f632..df809b3088 100755
--- a/tests/qemu-iotests/010
+++ b/tests/qemu-iotests/010
@@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt generic
 _supported_proto generic
+_unsupported_imgopts "subformat=streamOptimized"
 
 
 size=6G
diff --git a/tests/qemu-iotests/011 b/tests/qemu-iotests/011
index 56f704b5b9..57b99ae4a9 100755
--- a/tests/qemu-iotests/011
+++ b/tests/qemu-iotests/011
@@ -38,6 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _supported_fmt generic
 _supported_proto generic
+_unsupported_imgopts "subformat=streamOptimized"
 
 
 size=6G
diff --git a/tests/qemu-iotests/017 b/tests/qemu-iotests/017
index 79875de454..0a4b854e65 100755
--- a/tests/qemu-iotests/017
+++ b/tests/qemu-iotests/017
@@ -41,7 +41,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow qcow2 vmdk qed
 _supported_proto generic
 _unsupported_proto vxhs
-_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat"
+_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat" 
\
+ "subformat=streamOptimized"
 
 TEST_OFFSETS="0 4294967296"
 
diff --git a/tests/qemu-iotests/018 b/tests/qemu-iotests/018
index 78169838ba..c69ce09209 100755
--- a/tests/qemu-iotests/018
+++ b/tests/qemu-iotests/018
@@ -41,7 +41,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow qcow2 vmdk qed
 _supported_proto file
 _supported_os Linux
-_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat"
+_unsupported_imgopts "subformat=monolithicFlat" "subformat=twoGbMaxExtentFlat" 
\
+ "streamOptimized"
 
 TEST_OFFSETS="0 4294967296"
 
diff --git a/tests/qemu-iotests/019 b/tests/qemu-iotests/019
index a56dd30bed..b4f5234609 100755
--- a/tests/qemu-iotests/019
+++ b/tests/qemu-iotests/019
@@ -47,7 +47,8 @@ _supported_proto file
 _supported_os Linux
 _unsupported_imgopts "subformat=monolithicFlat" \
  "subformat=twoGbMaxExtentFlat" \
- "subformat=twoGbMaxExtentSparse"
+ "subformat=twoGbMaxExtentSparse" \
+ "subformat=streamOptimized"
 
 TEST_OFFSETS="0 4294967296"
 CLUSTER_SIZE=65536
diff --git a/tests/qemu-iotests/020 b/tests/qemu-iotests/020
index 6b0ebb37d2..f41b92f35f 100755
--- a/tests/qemu-iotests/020
+++ 

[Qemu-devel] [PULL 02/15] block: fix permission update in bdrv_replace_node

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

It's wrong to OR shared permissions. It may lead to crash on further
permission updates.
Also, no needs to consider previously calculated permissions, as at
this point we already bind all new parents and bdrv_get_cumulative_perm
result is enough. So fix the bug by just set permissions by
bdrv_get_cumulative_perm result.

Bug was introduced in long ago 234ac1a9025, in 2.9.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Message-id: 20190824100740.61635-1-vsement...@virtuozzo.com
Signed-off-by: Max Reitz 
---
 block.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 874a29a983..5944124845 100644
--- a/block.c
+++ b/block.c
@@ -4165,7 +4165,6 @@ void bdrv_replace_node(BlockDriverState *from, 
BlockDriverState *to,
 {
 BdrvChild *c, *next;
 GSList *list = NULL, *p;
-uint64_t old_perm, old_shared;
 uint64_t perm = 0, shared = BLK_PERM_ALL;
 int ret;
 
@@ -4211,8 +4210,8 @@ void bdrv_replace_node(BlockDriverState *from, 
BlockDriverState *to,
 bdrv_unref(from);
 }
 
-bdrv_get_cumulative_perm(to, _perm, _shared);
-bdrv_set_perm(to, old_perm | perm, old_shared | shared);
+bdrv_get_cumulative_perm(to, , );
+bdrv_set_perm(to, perm, shared);
 
 out:
 g_slist_free(list);
-- 
2.21.0

[Qemu-devel] [PULL 01/15] qemu-io: add pattern file for write command

[Max Reitz]

From: Denis Plotnikov 

The patch allows to provide a pattern file for write
command. There was no similar ability before.

Signed-off-by: Denis Plotnikov 
Message-id: 20190820164616.4072-1-dplotni...@virtuozzo.com
Reviewed-by: Eric Blake 
[mreitz: Keep optstring in alphabetical order]
Signed-off-by: Max Reitz 
---
 qemu-io-cmds.c | 99 +++---
 1 file changed, 93 insertions(+), 6 deletions(-)

diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 8904733961..d46fa166d3 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -350,6 +350,79 @@ static void qemu_io_free(void *p)
 qemu_vfree(p);
 }
 
+/*
+ * qemu_io_alloc_from_file()
+ *
+ * Allocates the buffer and populates it with the content of the given file
+ * up to @len bytes. If the file length is less than @len, then the buffer
+ * is populated with the file content cyclically.
+ *
+ * @blk - the block backend where the buffer content is going to be written to
+ * @len - the buffer length
+ * @file_name - the file to read the content from
+ *
+ * Returns: the buffer pointer on success
+ *  NULL on error
+ */
+static void *qemu_io_alloc_from_file(BlockBackend *blk, size_t len,
+ const char *file_name)
+{
+char *buf, *buf_origin;
+FILE *f = fopen(file_name, "r");
+int pattern_len;
+
+if (!f) {
+perror(file_name);
+return NULL;
+}
+
+if (qemuio_misalign) {
+len += MISALIGN_OFFSET;
+}
+
+buf_origin = buf = blk_blockalign(blk, len);
+
+if (qemuio_misalign) {
+buf_origin += MISALIGN_OFFSET;
+buf += MISALIGN_OFFSET;
+len -= MISALIGN_OFFSET;
+}
+
+pattern_len = fread(buf_origin, 1, len, f);
+
+if (ferror(f)) {
+perror(file_name);
+goto error;
+}
+
+if (pattern_len == 0) {
+fprintf(stderr, "%s: file is empty\n", file_name);
+goto error;
+}
+
+fclose(f);
+
+if (len > pattern_len) {
+len -= pattern_len;
+buf += pattern_len;
+
+while (len > 0) {
+size_t len_to_copy = MIN(pattern_len, len);
+
+memcpy(buf, buf_origin, len_to_copy);
+
+len -= len_to_copy;
+buf += len_to_copy;
+}
+}
+
+return buf_origin;
+
+error:
+qemu_io_free(buf_origin);
+return NULL;
+}
+
 static void dump_buffer(const void *buffer, int64_t offset, int64_t len)
 {
 uint64_t i;
@@ -948,6 +1021,7 @@ static void write_help(void)
 " -n, -- with -z, don't allow slow fallback\n"
 " -p, -- ignored for backwards compatibility\n"
 " -P, -- use different pattern to fill file\n"
+" -s, -- use a pattern file to fill the write buffer\n"
 " -C, -- report statistics in a machine parsable format\n"
 " -q, -- quiet mode, do not show I/O statistics\n"
 " -u, -- with -z, allow unmapping\n"
@@ -964,7 +1038,7 @@ static const cmdinfo_t write_cmd = {
 .perm   = BLK_PERM_WRITE,
 .argmin = 2,
 .argmax = -1,
-.args   = "[-bcCfnquz] [-P pattern] off len",
+.args   = "[-bcCfnquz] [-P pattern | -s source_file] off len",
 .oneline= "writes a number of bytes at a specified offset",
 .help   = write_help,
 };
@@ -973,7 +1047,7 @@ static int write_f(BlockBackend *blk, int argc, char 
**argv)
 {
 struct timespec t1, t2;
 bool Cflag = false, qflag = false, bflag = false;
-bool Pflag = false, zflag = false, cflag = false;
+bool Pflag = false, zflag = false, cflag = false, sflag = false;
 int flags = 0;
 int c, cnt, ret;
 char *buf = NULL;
@@ -982,8 +1056,9 @@ static int write_f(BlockBackend *blk, int argc, char 
**argv)
 /* Some compilers get confused and warn if this is not initialized.  */
 int64_t total = 0;
 int pattern = 0xcd;
+const char *file_name = NULL;
 
-while ((c = getopt(argc, argv, "bcCfnpP:quz")) != -1) {
+while ((c = getopt(argc, argv, "bcCfnpP:qs:uz")) != -1) {
 switch (c) {
 case 'b':
 bflag = true;
@@ -1013,6 +1088,10 @@ static int write_f(BlockBackend *blk, int argc, char 
**argv)
 case 'q':
 qflag = true;
 break;
+case 's':
+sflag = true;
+file_name = optarg;
+break;
 case 'u':
 flags |= BDRV_REQ_MAY_UNMAP;
 break;
@@ -1050,8 +1129,9 @@ static int write_f(BlockBackend *blk, int argc, char 
**argv)
 return -EINVAL;
 }
 
-if (zflag && Pflag) {
-printf("-z and -P cannot be specified at the same time\n");
+if (zflag + Pflag + sflag > 1) {
+printf("Only one of -z, -P, and -s "
+   "can be specified at the same time\n");
 return -EINVAL;
 }
 
@@ -1087,7 +1167,14 @@ static int write_f(BlockBackend *blk, int argc, char 
**argv)
 }
 
 if (!zflag) {
-buf = qemu_io_alloc(blk, count, pattern);
+if (sflag) {
+buf = qemu_io_alloc_from_file(blk, count, file_name);
+ 

[Qemu-devel] [PULL 03/15] block: posix: Always allocate the first block

[Max Reitz]

From: Nir Soffer 

When creating an image with preallocation "off" or "falloc", the first
block of the image is typically not allocated. When using Gluster
storage backed by XFS filesystem, reading this block using direct I/O
succeeds regardless of request length, fooling alignment detection.

In this case we fallback to a safe value (4096) instead of the optimal
value (512), which may lead to unneeded data copying when aligning
requests.  Allocating the first block avoids the fallback.

Since we allocate the first block even with preallocation=off, we no
longer create images with zero disk size:

$ ./qemu-img create -f raw test.raw 1g
Formatting 'test.raw', fmt=raw size=1073741824

$ ls -lhs test.raw
4.0K -rw-r--r--. 1 nsoffer nsoffer 1.0G Aug 16 23:48 test.raw

And converting the image requires additional cluster:

$ ./qemu-img measure -f raw -O qcow2 test.raw
required size: 458752
fully allocated size: 1074135040

When using format like vmdk with multiple files per image, we allocate
one block per file:

$ ./qemu-img create -f vmdk -o subformat=twoGbMaxExtentFlat test.vmdk 4g
Formatting 'test.vmdk', fmt=vmdk size=4294967296 compat6=off 
hwversion=undefined subformat=twoGbMaxExtentFlat

$ ls -lhs test*.vmdk
4.0K -rw-r--r--. 1 nsoffer nsoffer 2.0G Aug 27 03:23 test-f001.vmdk
4.0K -rw-r--r--. 1 nsoffer nsoffer 2.0G Aug 27 03:23 test-f002.vmdk
4.0K -rw-r--r--. 1 nsoffer nsoffer  353 Aug 27 03:23 test.vmdk

I did quick performance test for copying disks with qemu-img convert to
new raw target image to Gluster storage with sector size of 512 bytes:

for i in $(seq 10); do
rm -f dst.raw
sleep 10
time ./qemu-img convert -f raw -O raw -t none -T none src.raw dst.raw
done

Here is a table comparing the total time spent:

TypeBefore(s)   After(s)Diff(%)
---
real  530.028469.123  -11.4
user   17.204 10.768  -37.4
sys17.881  7.011  -60.7

We can see very clear improvement in CPU usage.

Signed-off-by: Nir Soffer 
Message-id: 20190827010528.8818-2-nsof...@redhat.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 block/file-posix.c| 51 +++
 tests/qemu-iotests/059.out|  2 +-
 tests/qemu-iotests/{150.out => 150.out.qcow2} |  0
 tests/qemu-iotests/150.out.raw| 12 +
 tests/qemu-iotests/175| 19 ---
 tests/qemu-iotests/175.out|  8 +--
 tests/qemu-iotests/178.out.qcow2  |  4 +-
 tests/qemu-iotests/221.out| 12 +++--
 tests/qemu-iotests/253.out| 12 +++--
 9 files changed, 99 insertions(+), 21 deletions(-)
 rename tests/qemu-iotests/{150.out => 150.out.qcow2} (100%)
 create mode 100644 tests/qemu-iotests/150.out.raw

diff --git a/block/file-posix.c b/block/file-posix.c
index fbeb0068db..447f937aa1 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1749,6 +1749,43 @@ static int handle_aiocb_discard(void *opaque)
 return ret;
 }
 
+/*
+ * Help alignment probing by allocating the first block.
+ *
+ * When reading with direct I/O from unallocated area on Gluster backed by XFS,
+ * reading succeeds regardless of request length. In this case we fallback to
+ * safe alignment which is not optimal. Allocating the first block avoids this
+ * fallback.
+ *
+ * fd may be opened with O_DIRECT, but we don't know the buffer alignment or
+ * request alignment, so we use safe values.
+ *
+ * Returns: 0 on success, -errno on failure. Since this is an optimization,
+ * caller may ignore failures.
+ */
+static int allocate_first_block(int fd, size_t max_size)
+{
+size_t write_size = (max_size < MAX_BLOCKSIZE)
+? BDRV_SECTOR_SIZE
+: MAX_BLOCKSIZE;
+size_t max_align = MAX(MAX_BLOCKSIZE, getpagesize());
+void *buf;
+ssize_t n;
+int ret;
+
+buf = qemu_memalign(max_align, write_size);
+memset(buf, 0, write_size);
+
+do {
+n = pwrite(fd, buf, write_size, 0);
+} while (n == -1 && errno == EINTR);
+
+ret = (n == -1) ? -errno : 0;
+
+qemu_vfree(buf);
+return ret;
+}
+
 static int handle_aiocb_truncate(void *opaque)
 {
 RawPosixAIOData *aiocb = opaque;
@@ -1788,6 +1825,17 @@ static int handle_aiocb_truncate(void *opaque)
 /* posix_fallocate() doesn't set errno. */
 error_setg_errno(errp, -result,
  "Could not preallocate new data");
+} else if (current_length == 0) {
+/*
+ * posix_fallocate() uses fallocate() if the filesystem
+ * supports it, or fallback to manually writing zeroes. If
+ * fallocate() was used, unaligned reads from the fallocated
+ * area in raw_probe_alignment() will succeed, hence we need to
+ * allocate the first block.
+ 

Re: [Qemu-devel] [PATCH] iotests: Unify cache mode quoting

[Nir Soffer]

On Tue, Aug 27, 2019 at 8:43 PM Max Reitz  wrote:
>
> On 27.08.19 19:38, Nir Soffer wrote:
> > On Tue, Aug 27, 2019 at 8:36 PM Max Reitz  > > wrote:
> >
> > On 27.08.19 19:34, Nir Soffer wrote:
> > > Quoting cache mode is not needed, and most tests use unquoted values.
> > > Unify all test to use the same style.
> >
> > S-o-b is missing, shall I add it?
> >
> >
> > Thanks!
> >
> > Signed-off-by: Nir Soffer 
>
> Thanks, applied to my block branch:
>
> https://git.xanclic.moe/XanClic/qemu/commits/branch/block
>
> (By the way, did you set your author email address to an @gmail address
> on purpose?)

I configured my gmail address nowhere, I think this is a side affect of using
gmail to send the patches.

>
> Max
>

Re: [Qemu-devel] [PATCH] iotests: Unify cache mode quoting

[Max Reitz]

On 27.08.19 19:38, Nir Soffer wrote:
> On Tue, Aug 27, 2019 at 8:36 PM Max Reitz  > wrote:
> 
> On 27.08.19 19:34, Nir Soffer wrote:
> > Quoting cache mode is not needed, and most tests use unquoted values.
> > Unify all test to use the same style.
> 
> S-o-b is missing, shall I add it?
> 
> 
> Thanks!
> 
> Signed-off-by: Nir Soffer 

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

(By the way, did you set your author email address to an @gmail address
on purpose?)

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] iotests: Unify cache mode quoting

[Nir Soffer]

On Tue, Aug 27, 2019 at 8:36 PM Max Reitz  wrote:

> On 27.08.19 19:34, Nir Soffer wrote:
> > Quoting cache mode is not needed, and most tests use unquoted values.
> > Unify all test to use the same style.
>
> S-o-b is missing, shall I add it?
>

Thanks!

Signed-off-by: Nir Soffer 


>
> Max
>
> > ---
> >  tests/qemu-iotests/026 | 4 ++--
> >  tests/qemu-iotests/039 | 4 ++--
> >  tests/qemu-iotests/052 | 2 +-
> >  tests/qemu-iotests/091 | 4 ++--
> >  4 files changed, 7 insertions(+), 7 deletions(-)
>
>

Re: [Qemu-devel] [PATCH] iotests: Unify cache mode quoting

[Max Reitz]

On 27.08.19 19:34, Nir Soffer wrote:
> Quoting cache mode is not needed, and most tests use unquoted values.
> Unify all test to use the same style.

S-o-b is missing, shall I add it?

Max

> ---
>  tests/qemu-iotests/026 | 4 ++--
>  tests/qemu-iotests/039 | 4 ++--
>  tests/qemu-iotests/052 | 2 +-
>  tests/qemu-iotests/091 | 4 ++--
>  4 files changed, 7 insertions(+), 7 deletions(-)



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] tests/check-block: Skip iotests when sanitizers are enabled

[Max Reitz]

On 23.08.19 10:42, Thomas Huth wrote:
> The sanitizers (especially the address sanitizer from Clang) are
> sometimes printing out warnings or false positives - this spoils
> the output of the iotests, causing some of the tests to fail.
> Thus let's skip the automatic iotests during "make check" when the
> user configured QEMU with --enable-sanitizers.
> 
> Signed-off-by: Thomas Huth 
> ---
>  tests/check-block.sh | 5 +
>  1 file changed, 5 insertions(+)

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] iotests: Unify cache mode quoting

[Nir Soffer]

Quoting cache mode is not needed, and most tests use unquoted values.
Unify all test to use the same style.
---
 tests/qemu-iotests/026 | 4 ++--
 tests/qemu-iotests/039 | 4 ++--
 tests/qemu-iotests/052 | 2 +-
 tests/qemu-iotests/091 | 4 ++--
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026
index e30243608b..ffb18ab6b5 100755
--- a/tests/qemu-iotests/026
+++ b/tests/qemu-iotests/026
@@ -41,8 +41,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 # Currently only qcow2 supports rebasing
 _supported_fmt qcow2
 _supported_proto file
-_default_cache_mode "writethrough"
-_supported_cache_modes "writethrough" "none"
+_default_cache_mode writethrough
+_supported_cache_modes writethrough none
 # The refcount table tests expect a certain minimum width for refcount entries
 # (so that the refcount table actually needs to grow); that minimum is 16 bits,
 # being the default refcount entry width.
diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
index 0d4e963bd4..7c730d94a7 100755
--- a/tests/qemu-iotests/039
+++ b/tests/qemu-iotests/039
@@ -42,8 +42,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow2
 _supported_proto file
 _supported_os Linux
-_default_cache_mode "writethrough"
-_supported_cache_modes "writethrough"
+_default_cache_mode writethrough
+_supported_cache_modes writethrough
 
 size=128M
 
diff --git a/tests/qemu-iotests/052 b/tests/qemu-iotests/052
index 6e2ecbfe21..45a140910d 100755
--- a/tests/qemu-iotests/052
+++ b/tests/qemu-iotests/052
@@ -40,7 +40,7 @@ _supported_fmt generic
 _supported_proto file
 
 # Don't do O_DIRECT on tmpfs
-_supported_cache_modes "writeback" "writethrough" "unsafe"
+_supported_cache_modes writeback writethrough unsafe
 
 size=128M
 _make_test_img $size
diff --git a/tests/qemu-iotests/091 b/tests/qemu-iotests/091
index d62ef18a02..f4b44659ae 100755
--- a/tests/qemu-iotests/091
+++ b/tests/qemu-iotests/091
@@ -46,8 +46,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow2
 _supported_proto file
 _supported_os Linux
-_default_cache_mode "none"
-_supported_cache_modes "writethrough" "none" "writeback"
+_default_cache_mode none
+_supported_cache_modes writethrough none writeback
 
 size=1G
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH v2] iotests: Check for enabled drivers before testing them

[Max Reitz]

On 23.08.19 15:35, Thomas Huth wrote:
> It is possible to enable only a subset of the block drivers with the
> "--block-drv-rw-whitelist" option of the "configure" script. All other
> drivers are marked as unusable (or only included as read-only with the
> "--block-drv-ro-whitelist" option). If an iotest is now using such a
> disabled block driver, it is failing - which is bad, since at least the
> tests in the "auto" group should be able to deal with this situation.
> Thus let's introduce a "_require_drivers" function that can be used by
> the shell tests to check for the availability of certain drivers first,
> and marks the test as "not run" if one of the drivers is missing.
> 
> This patch mainly targets the test in the "auto" group which should
> never fail in such a case, but also improves some of the other tests
> along the way. Note that we also assume that the "qcow2" and "file"
> drivers are always available - otherwise it does not make sense to
> run "make check-block" at all (which only tests with qcow2 by default).
> 
> Signed-off-by: Thomas Huth 
> ---
>  v2:
>  - Update the check in _require_drivers() according to Max' suggestion
>  - Remove superfluous check in test 081
>  - Mark 120 to require "raw"
>  - Replaced the check in 162 to use the new _require_drivers() function
>  - Mark 186 to require "null-co"
> 
>  tests/qemu-iotests/071   |  1 +
>  tests/qemu-iotests/081   |  4 +---
>  tests/qemu-iotests/099   |  1 +
>  tests/qemu-iotests/120   |  1 +
>  tests/qemu-iotests/162   |  4 +---
>  tests/qemu-iotests/184   |  1 +
>  tests/qemu-iotests/186   |  1 +
>  tests/qemu-iotests/common.rc | 14 ++
>  8 files changed, 21 insertions(+), 6 deletions(-)

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] file-posix: fix request_alignment typo

[Max Reitz]

On 27.08.19 12:13, Stefan Hajnoczi wrote:
> Fixes: a6b257a08e3d72219f03e461a52152672fec0612
>("file-posix: Handle undetectable alignment")
> Signed-off-by: Stefan Hajnoczi 
> ---
>  block/file-posix.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 0/7] vmdk: Misc fixes

[Max Reitz]

On 15.08.19 17:36, Max Reitz wrote:
> I made the mistake of trying to run the iotests with all non-default
> subformats our vmdk driver has to offer:
> - monolithicFlat
> - twoGbMaxExtentSparse
> - twoGbMaxExtentFlat
> - streamOptimized
> 
> Many things broke, so this series fixes what I found.  It’s mostly just
> iotest fixes, but there are actually two real fixes in here.

Thanks for the review, applied to my block branch.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 1/2] block: posix: Always allocate the first block

[Max Reitz]

On 27.08.19 18:58, Max Reitz wrote:
> On 27.08.19 03:05, Nir Soffer wrote:
>> When creating an image with preallocation "off" or "falloc", the first
>> block of the image is typically not allocated. When using Gluster
>> storage backed by XFS filesystem, reading this block using direct I/O
>> succeeds regardless of request length, fooling alignment detection.
>>
>> In this case we fallback to a safe value (4096) instead of the optimal
>> value (512), which may lead to unneeded data copying when aligning
>> requests.  Allocating the first block avoids the fallback.
>>
>> Since we allocate the first block even with preallocation=off, we no
>> longer create images with zero disk size:
>>
>> $ ./qemu-img create -f raw test.raw 1g
>> Formatting 'test.raw', fmt=raw size=1073741824
>>
>> $ ls -lhs test.raw
>> 4.0K -rw-r--r--. 1 nsoffer nsoffer 1.0G Aug 16 23:48 test.raw
>>
>> And converting the image requires additional cluster:
>>
>> $ ./qemu-img measure -f raw -O qcow2 test.raw
>> required size: 458752
>> fully allocated size: 1074135040
>>
>> When using format like vmdk with multiple files per image, we allocate
>> one block per file:
>>
>> $ ./qemu-img create -f vmdk -o subformat=twoGbMaxExtentFlat test.vmdk 4g
>> Formatting 'test.vmdk', fmt=vmdk size=4294967296 compat6=off 
>> hwversion=undefined subformat=twoGbMaxExtentFlat
>>
>> $ ls -lhs test*.vmdk
>> 4.0K -rw-r--r--. 1 nsoffer nsoffer 2.0G Aug 27 03:23 test-f001.vmdk
>> 4.0K -rw-r--r--. 1 nsoffer nsoffer 2.0G Aug 27 03:23 test-f002.vmdk
>> 4.0K -rw-r--r--. 1 nsoffer nsoffer  353 Aug 27 03:23 test.vmdk
>>
>> I did quick performance test for copying disks with qemu-img convert to
>> new raw target image to Gluster storage with sector size of 512 bytes:
>>
>> for i in $(seq 10); do
>> rm -f dst.raw
>> sleep 10
>> time ./qemu-img convert -f raw -O raw -t none -T none src.raw dst.raw
>> done
>>
>> Here is a table comparing the total time spent:
>>
>> TypeBefore(s)   After(s)Diff(%)
>> ---
>> real  530.028469.123  -11.4
>> user   17.204 10.768  -37.4
>> sys17.881  7.011  -60.7
>>
>> We can see very clear improvement in CPU usage.
>>
>> Signed-off-by: Nir Soffer 
>> ---
>>  block/file-posix.c| 51 +++
>>  tests/qemu-iotests/059.out|  2 +-
>>  tests/qemu-iotests/{150.out => 150.out.qcow2} |  0
>>  tests/qemu-iotests/150.out.raw| 12 +
>>  tests/qemu-iotests/175| 19 ---
>>  tests/qemu-iotests/175.out|  8 +--
>>  tests/qemu-iotests/178.out.qcow2  |  4 +-
>>  tests/qemu-iotests/221.out| 12 +++--
>>  tests/qemu-iotests/253.out| 12 +++--
>>  9 files changed, 99 insertions(+), 21 deletions(-)
>>  rename tests/qemu-iotests/{150.out => 150.out.qcow2} (100%)
>>  create mode 100644 tests/qemu-iotests/150.out.raw
> 
> Reviewed-by: Max Reitz 
> 
> Maybe it’ll break the vmdk iotests when using a non-default subformat;
> but currently running the iotests for non-default VMDK subformats is
> broken anyway, so it doesn’t matter.

(Good news, 059 really was the only issue for VMDK.)



signature.asc
Description: OpenPGP digital signature