Re: [PATCH] meson: set colorout to auto

[Marc-André Lureau]

Hi

On Tue, Aug 25, 2020 at 10:44 AM Gerd Hoffmann  wrote:

> Dunno why the default is set to "always".  IMHO it should be "auto",
> i.e. only colorize in case stdout goes to a terminal.  Cluttering
> logfiles and confusing compiler message parsers with terminal control
> sequences is not nice ...
>
> Signed-off-by: Gerd Hoffmann 
>

"Enable colored output with GCC. Ninja redirects stdout/stderr so by
default GCC thinks it is not talking to a terminal"

https://github.com/mesonbuild/meson/commit/4f63fe498314c385de2d3b6a3a953d15985914d2

Since we use make, I don't know if it's any better.

Perhaps meson should set compiler/tools colors = always/never based on what
it is connected to at configure time instead?

-- 
Marc-André Lureau

Re: [PATCH v2 1/2] linux-user: Add support for ppoll_time64() and pselect6_time64()

[Laurent Vivier]

Le 25/08/2020 à 00:30, Filip Bozuta a écrit :
> This patch introduces functionality for following time64 syscalls:
> 
> *ppoll_time64
> 
> This is a year 2038 safe variant of:
> 
> int poll(struct pollfd *fds, nfds_t nfds, int timeout)
> -- wait for some event on a file descriptor --
> man page: https://man7.org/linux/man-pages/man2/ppoll.2.html
> 
> *pselect6_time64
> 
> This is a year 2038 safe variant of:
> 
> int pselect6(int nfds, fd_set *readfds, fd_set *writefds,
>  fd_set *exceptfds, const struct timespec *timeout,
>  const sigset_t *sigmask);
> -- synchronous I/O multiplexing --
> man page: https://man7.org/linux/man-pages/man2/pselect6.2.html
> 
> Implementation notes:
> 
> Year 2038 safe syscalls in this patch were implemented
> with the same code as their regular variants (ppoll() and pselect()).
> This code was moved to new functions ('do_ppoll()' and 'do_pselect6()')
> that take a 'bool time64' from which a right 'struct timespec' converting
> function is called.
> (target_to_host/host_to_target_timespec() for regular and
>  target_to_host/host_to_target_timespec64() for time64 variants)
> 
> Signed-off-by: Filip Bozuta 
> ---
>  linux-user/syscall.c | 462 +++
>  1 file changed, 251 insertions(+), 211 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1211e759c2..fc6a6e32e4 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -397,7 +397,7 @@ static int sys_getcwd1(char *buf, size_t size)
>return strlen(buf)+1;
>  }
>  
> -#ifdef TARGET_NR_utimensat
> +#if defined(TARGET_NR_utimensat)
>  #if defined(__NR_utimensat)
>  #define __NR_sys_utimensat __NR_utimensat
>  _syscall4(int,sys_utimensat,int,dirfd,const char *,pathname,
> @@ -763,11 +763,11 @@ safe_syscall5(int, waitid, idtype_t, idtype, id_t, id, 
> siginfo_t *, infop, \
>int, options, struct rusage *, rusage)
>  safe_syscall3(int, execve, const char *, filename, char **, argv, char **, 
> envp)
>  #if defined(TARGET_NR_select) || defined(TARGET_NR__newselect) || \
> -defined(TARGET_NR_pselect6)
> +defined(TARGET_NR_pselect6) || defined(TARGET_NR_pselect6_time64)
>  safe_syscall6(int, pselect6, int, nfds, fd_set *, readfds, fd_set *, 
> writefds, \
>fd_set *, exceptfds, struct timespec *, timeout, void *, sig)
>  #endif
> -#if defined(TARGET_NR_ppoll) || defined(TARGET_NR_poll)
> +#if defined(TARGET_NR_ppoll) || defined(TARGET_NR_ppoll_time64)
>  safe_syscall5(int, ppoll, struct pollfd *, ufds, unsigned int, nfds,
>struct timespec *, tsp, const sigset_t *, sigmask,
>size_t, sigsetsize)
> @@ -984,7 +984,7 @@ abi_long do_brk(abi_ulong new_brk)
>  }
>  
>  #if defined(TARGET_NR_select) || defined(TARGET_NR__newselect) || \
> -defined(TARGET_NR_pselect6)
> +defined(TARGET_NR_pselect6) || defined(TARGET_NR_pselect6_time64)
>  static inline abi_long copy_from_user_fdset(fd_set *fds,
>  abi_ulong target_fds_addr,
>  int n)
> @@ -1252,7 +1252,8 @@ static inline abi_long target_to_host_timespec(struct 
> timespec *host_ts,
>  }
>  #endif
>  
> -#if defined(TARGET_NR_clock_settime64) || defined(TARGET_NR_futex_time64)
> +#if defined(TARGET_NR_clock_settime64) || defined(TARGET_NR_futex_time64) || 
> \
> +defined(TARGET_NR_pselect6_time64) || defined(TARGET_NR_ppoll_time64)
>  static inline abi_long target_to_host_timespec64(struct timespec *host_ts,
>   abi_ulong target_addr)
>  {
> @@ -1458,6 +1459,237 @@ static abi_long do_old_select(abi_ulong arg1)
>  #endif
>  #endif
>  
> +#if defined(TARGET_NR_pselect6) || defined(TARGET_NR_pselect6_time64)
> +static abi_long do_pselect6(abi_long arg1, abi_long arg2, abi_long arg3,
> +abi_long arg4, abi_long arg5, abi_long arg6,
> +bool time64)
> +{
> +abi_long rfd_addr, wfd_addr, efd_addr, n, ts_addr;
> +fd_set rfds, wfds, efds;
> +fd_set *rfds_ptr, *wfds_ptr, *efds_ptr;
> +struct timespec ts, *ts_ptr;
> +abi_long ret;
> +
> +/*
> + * The 6th arg is actually two args smashed together,
> + * so we cannot use the C library.
> + */
> +sigset_t set;
> +struct {
> +sigset_t *set;
> +size_t size;
> +} sig, *sig_ptr;
> +
> +abi_ulong arg_sigset, arg_sigsize, *arg7;
> +target_sigset_t *target_sigset;
> +
> +n = arg1;
> +rfd_addr = arg2;
> +wfd_addr = arg3;
> +efd_addr = arg4;
> +ts_addr = arg5;
> +
> +ret = copy_from_user_fdset_ptr(&rfds, &rfds_ptr, rfd_addr, n);
> +if (ret) {
> +return ret;
> +}
> +ret = copy_from_user_fdset_ptr(&wfds, &wfds_ptr, wfd_addr, n);
> +if (ret) {
> +return ret;
> +}
> +ret = copy_from_user_fdset_ptr(&ef

Re: [PATCH v2 1/2] linux-user: Add support for ppoll_time64() and pselect6_time64()

[Laurent Vivier]

Le 25/08/2020 à 00:30, Filip Bozuta a écrit :
> This patch introduces functionality for following time64 syscalls:
> 
> *ppoll_time64
> 
> This is a year 2038 safe variant of:
> 
> int poll(struct pollfd *fds, nfds_t nfds, int timeout)
> -- wait for some event on a file descriptor --
> man page: https://man7.org/linux/man-pages/man2/ppoll.2.html
> 
> *pselect6_time64
> 
> This is a year 2038 safe variant of:
> 
> int pselect6(int nfds, fd_set *readfds, fd_set *writefds,
>  fd_set *exceptfds, const struct timespec *timeout,
>  const sigset_t *sigmask);
> -- synchronous I/O multiplexing --
> man page: https://man7.org/linux/man-pages/man2/pselect6.2.html
> 
> Implementation notes:
> 
> Year 2038 safe syscalls in this patch were implemented
> with the same code as their regular variants (ppoll() and pselect()).
> This code was moved to new functions ('do_ppoll()' and 'do_pselect6()')
> that take a 'bool time64' from which a right 'struct timespec' converting
> function is called.
> (target_to_host/host_to_target_timespec() for regular and
>  target_to_host/host_to_target_timespec64() for time64 variants)
> 
> Signed-off-by: Filip Bozuta 
> ---
>  linux-user/syscall.c | 462 +++
>  1 file changed, 251 insertions(+), 211 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 1211e759c2..fc6a6e32e4 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -397,7 +397,7 @@ static int sys_getcwd1(char *buf, size_t size)
>return strlen(buf)+1;
>  }
>  
> -#ifdef TARGET_NR_utimensat
> +#if defined(TARGET_NR_utimensat)
>  #if defined(__NR_utimensat)
>  #define __NR_sys_utimensat __NR_utimensat
>  _syscall4(int,sys_utimensat,int,dirfd,const char *,pathname,
> @@ -763,11 +763,11 @@ safe_syscall5(int, waitid, idtype_t, idtype, id_t, id, 
> siginfo_t *, infop, \
>int, options, struct rusage *, rusage)
>  safe_syscall3(int, execve, const char *, filename, char **, argv, char **, 
> envp)
>  #if defined(TARGET_NR_select) || defined(TARGET_NR__newselect) || \
> -defined(TARGET_NR_pselect6)
> +defined(TARGET_NR_pselect6) || defined(TARGET_NR_pselect6_time64)
>  safe_syscall6(int, pselect6, int, nfds, fd_set *, readfds, fd_set *, 
> writefds, \
>fd_set *, exceptfds, struct timespec *, timeout, void *, sig)
>  #endif
> -#if defined(TARGET_NR_ppoll) || defined(TARGET_NR_poll)
> +#if defined(TARGET_NR_ppoll) || defined(TARGET_NR_ppoll_time64)
>  safe_syscall5(int, ppoll, struct pollfd *, ufds, unsigned int, nfds,
>struct timespec *, tsp, const sigset_t *, sigmask,
>size_t, sigsetsize)
> @@ -984,7 +984,7 @@ abi_long do_brk(abi_ulong new_brk)
>  }
>  
>  #if defined(TARGET_NR_select) || defined(TARGET_NR__newselect) || \
> -defined(TARGET_NR_pselect6)
> +defined(TARGET_NR_pselect6) || defined(TARGET_NR_pselect6_time64)
>  static inline abi_long copy_from_user_fdset(fd_set *fds,
>  abi_ulong target_fds_addr,
>  int n)
> @@ -1252,7 +1252,8 @@ static inline abi_long target_to_host_timespec(struct 
> timespec *host_ts,
>  }
>  #endif
>  
> -#if defined(TARGET_NR_clock_settime64) || defined(TARGET_NR_futex_time64)
> +#if defined(TARGET_NR_clock_settime64) || defined(TARGET_NR_futex_time64) || 
> \
> +defined(TARGET_NR_pselect6_time64) || defined(TARGET_NR_ppoll_time64)
>  static inline abi_long target_to_host_timespec64(struct timespec *host_ts,
>   abi_ulong target_addr)
>  {
> @@ -1458,6 +1459,237 @@ static abi_long do_old_select(abi_ulong arg1)
>  #endif
>  #endif
>  
> +#if defined(TARGET_NR_pselect6) || defined(TARGET_NR_pselect6_time64)
> +static abi_long do_pselect6(abi_long arg1, abi_long arg2, abi_long arg3,
> +abi_long arg4, abi_long arg5, abi_long arg6,
> +bool time64)
> +{
> +abi_long rfd_addr, wfd_addr, efd_addr, n, ts_addr;
> +fd_set rfds, wfds, efds;
> +fd_set *rfds_ptr, *wfds_ptr, *efds_ptr;
> +struct timespec ts, *ts_ptr;
> +abi_long ret;
> +
> +/*
> + * The 6th arg is actually two args smashed together,
> + * so we cannot use the C library.
> + */
> +sigset_t set;
> +struct {
> +sigset_t *set;
> +size_t size;
> +} sig, *sig_ptr;
> +
> +abi_ulong arg_sigset, arg_sigsize, *arg7;
> +target_sigset_t *target_sigset;
> +
> +n = arg1;
> +rfd_addr = arg2;
> +wfd_addr = arg3;
> +efd_addr = arg4;
> +ts_addr = arg5;
> +
> +ret = copy_from_user_fdset_ptr(&rfds, &rfds_ptr, rfd_addr, n);
> +if (ret) {
> +return ret;
> +}
> +ret = copy_from_user_fdset_ptr(&wfds, &wfds_ptr, wfd_addr, n);
> +if (ret) {
> +return ret;
> +}
> +ret = copy_from_user_fdset_ptr(&ef

Re: [PATCH v2 2/2] linux-user: Add support for utimensat_time64() and semtimedop_time64()

[Laurent Vivier]

Le 25/08/2020 à 00:30, Filip Bozuta a écrit :
> This patch introduces functionality for following time64 syscalls:
> 
> *utimensat_time64()
> 
> int utimensat(int dirfd, const char *pathname,
>   const struct timespec times[2], int flags);
> -- change file timestamps with nanosecond precision --
> man page: https://man7.org/linux/man-pages/man2/utimensat.2.html
> 
> *semtimedop_time64()
> 
> int semtimedop(int semid, struct sembuf *sops, size_t nsops,
>const struct timespec *timeout);
> -- System V semaphore operations --
> man page: https://www.man7.org/linux/man-pages/man2/semtimedop.2.html
> 
> Implementation notes:
> 
>Syscall 'utimensat_time64()' is implemented in similar way as its
>regular variants only difference being that time64 converting function
>is used to convert values of 'struct timespec' between host and target
>('target_to_host_timespec64()').
> 
>For syscall 'semtimedop_time64()' and additional argument is added
>in function 'do_semtimedop()' through which the aproppriate 'struct 
> timespec'
>converting function is called (false for regular target_to_host_timespec()
>and true for target_to_host_timespec64()). For 'do_ipc()' a
>check was added as that additional argument: 'TARGET_ABI_BITS == 64'.
> 
> Signed-off-by: Filip Bozuta 
> Reviewed-by: Laurent Vivier 
> ---
>  linux-user/syscall.c | 60 
>  1 file changed, 50 insertions(+), 10 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index fc6a6e32e4..4d460af744 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1253,7 +1253,8 @@ static inline abi_long target_to_host_timespec(struct 
> timespec *host_ts,
>  #endif
>  
>  #if defined(TARGET_NR_clock_settime64) || defined(TARGET_NR_futex_time64) || 
> \
> -defined(TARGET_NR_pselect6_time64) || defined(TARGET_NR_ppoll_time64)
> +defined(TARGET_NR_pselect6_time64) || defined(TARGET_NR_ppoll_time64) || 
> \
> +defined(TARGET_NR_utimensat_time64) || 
> defined(TARGET_NR_semtimedop_time64)
>  static inline abi_long target_to_host_timespec64(struct timespec *host_ts,
>   abi_ulong target_addr)
>  {
> @@ -4117,7 +4118,7 @@ static inline abi_long target_to_host_sembuf(struct 
> sembuf *host_sembuf,
>  }
>  
>  #if defined(TARGET_NR_ipc) || defined(TARGET_NR_semop) || \
> -defined(TARGET_NR_semtimedop)
> +defined(TARGET_NR_semtimedop) || defined(TARGET_NR_semtimedop_time64)
>  
>  /*
>   * This macro is required to handle the s390 variants, which passes the
> @@ -4134,7 +4135,7 @@ static inline abi_long target_to_host_sembuf(struct 
> sembuf *host_sembuf,
>  static inline abi_long do_semtimedop(int semid,
>   abi_long ptr,
>   unsigned nsops,
> - abi_long timeout)
> + abi_long timeout, bool time64)
>  {
>  struct sembuf sops[nsops];
>  struct timespec ts, *pts = NULL;
> @@ -4142,8 +4143,14 @@ static inline abi_long do_semtimedop(int semid,
>  
>  if (timeout) {
>  pts = &ts;
> -if (target_to_host_timespec(pts, timeout)) {
> -return -TARGET_EFAULT;
> +if (time64) {
> +if (target_to_host_timespec64(pts, timeout)) {
> +return -TARGET_EFAULT;
> +}
> +} else {
> +if (target_to_host_timespec(pts, timeout)) {
> +return -TARGET_EFAULT;
> +}
>  }
>  }
>  
> @@ -4657,7 +4664,7 @@ static abi_long do_ipc(CPUArchState *cpu_env,
>  
>  switch (call) {
>  case IPCOP_semop:
> -ret = do_semtimedop(first, ptr, second, 0);
> +ret = do_semtimedop(first, ptr, second, 0, false);
>  break;
>  case IPCOP_semtimedop:
>  /*
> @@ -4667,9 +4674,9 @@ static abi_long do_ipc(CPUArchState *cpu_env,
>   * to a struct timespec where the generic variant uses fifth parameter.
>   */
>  #if defined(TARGET_S390X)
> -ret = do_semtimedop(first, ptr, second, third);
> +ret = do_semtimedop(first, ptr, second, third, TARGET_ABI_BITS == 
> 64);
>  #else
> -ret = do_semtimedop(first, ptr, second, fifth);
> +ret = do_semtimedop(first, ptr, second, fifth, TARGET_ABI_BITS == 
> 64);
>  #endif
>  break;
>  
> @@ -9887,11 +9894,15 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>  #endif
>  #ifdef TARGET_NR_semop
>  case TARGET_NR_semop:
> -return do_semtimedop(arg1, arg2, arg3, 0);
> +return do_semtimedop(arg1, arg2, arg3, 0, false);
>  #endif
>  #ifdef TARGET_NR_semtimedop
>  case TARGET_NR_semtimedop:
> -return do_semtimedop(arg1, arg2, arg3, arg4);
> +return do_semtimedop(arg1, arg2, arg3, arg4, false);
> +#endif
> +#ifdef TARGET_NR_semtimedop_time64
> +  

Re: [PATCH v2 00/21] aspeed: cleanups and some extensions

[Cédric Le Goater]

On 8/25/20 8:01 AM, Joel Stanley wrote:
> On Wed, 19 Aug 2020 at 10:10, Cédric Le Goater  wrote:
>>
>> Hello,
>>
>> This series includes various fixes improving the support of Aspeed
>> machines. Extra attention was given to the robustness of the ftgmac100
>> model. A small kernel module tester was created for this purpose :
>>
>>https://github.com/legoater/ftgmac100-test/
> 
> I gave this a test and it successfully broke the machine for me
> without your fixes.

The network stack is busted. yes. Sometimes it survives or does a reset.

HW always survives which is surprising.

Thanks, 

C.

> I tried to test this series but the new build system stopped me from
> being able to complete a build. It failed with:
> 
> Found ninjatool-1.8 at qemu/upstream/build/ninjatool
> ./ninjatool -t ninja2make --omit clean dist uninstall < build.ninja >
> Makefile.ninja
> /bin/sh: build.ninja: No such file or directory
> 
> :(
> 
>>
>> Changes in v2 :
>>
>>  - definitions for some new flash models in m25p80 by Igor
>>  - All Joel's comments should have been addressed
>>  - A better fix of the integer overflow in ftgmac100_do_tx suggested
>>by Peter.
>>
>>
>> This needs a couple more reviewed-by before I can send a PR.
> 
> I have read through all the patches and I have no objections.
> 
> Cheers,
> 
> Joel
> 
>>
>> Thanks,
>>
>> C.
>>
>> Cédric Le Goater (16):
>>   m25p80: Return the JEDEC ID twice for mx25l25635e
>>   m25p80: Add support for mx25l25635f
>>   m25p80: Add support for n25q512ax3
>>   aspeed/scu: Fix valid access size on AST2400
>>   aspeed/smc: Fix MemoryRegionOps definition
>>   aspeed/smc: Fix max_slaves of the legacy SMC device
>>   aspeed/sdhci: Fix reset sequence
>>   ftgmac100: Fix registers that can be read
>>   ftgmac100: Fix interrupt status "Packet transmitted on ethernet"
>>   ftgmac100: Fix interrupt status "Packet moved to RX FIFO"
>>   ftgmac100: Change interrupt status when a DMA error occurs
>>   ftgmac100: Check for invalid len and address before doing a DMA
>> transfer
>>   ftgmac100: Fix integer overflow in ftgmac100_do_tx()
>>   ftgmac100: Improve software reset
>>   aspeed/sdmc: Simplify calculation of RAM bits
>>   aspeed/smc: Open AHB window of the second chip of the AST2600 FMC
>> controller
>>
>> Igor Kononenko (2):
>>   arm: aspeed: add strap define `25HZ` of AST2500
>>   hw: add a number of SPI-flash's of m25p80 family
>>
>> Joel Stanley (2):
>>   aspeed/sdmc: Perform memory training
>>   aspeed/sdmc: Allow writes to unprotected registers
>>
>> erik-smit (1):
>>   hw/arm/aspeed: Add board model for Supermicro X11 BMC
>>
>>  include/hw/misc/aspeed_scu.h  |   1 +
>>  include/hw/misc/aspeed_sdmc.h |  13 +++-
>>  hw/arm/aspeed.c   |  35 ++
>>  hw/block/m25p80.c |   6 +-
>>  hw/misc/aspeed_scu.c  |   9 +--
>>  hw/misc/aspeed_sdmc.c | 125 +++---
>>  hw/net/ftgmac100.c|  95 ++
>>  hw/sd/aspeed_sdhci.c  |  14 +++-
>>  hw/ssi/aspeed_smc.c   |   6 +-
>>  9 files changed, 209 insertions(+), 95 deletions(-)
>>
>> --
>> 2.25.4
>>

Re: meson: problems building under msys2/mingw-w64 native

[Paolo Bonzini]

Il lun 24 ago 2020, 22:27 Mark Cave-Ayland 
ha scritto:

>
> With this I can get all the way to the link phase so I think it's fairly
> close. I'm
> not sure whether these escaping/quoting problems are with meson or the way
> in which
> configure is using it?
>

You can try building with ninja to find whose fault it is.

We can get rid of ninjatool as soon as tests have been converted, but I
will look into fixing the quoting issue as well. Can you send me off list
the build.ninja file to see if the isystem problem is in Meson or ninjatool?

Paolo


>
> ATB,
>
> Mark.
>
>

Re: [PATCH] configure: default to PIE disabled on Windows platforms

[Paolo Bonzini]

Queued, thanks.

Paolo

Il lun 24 ago 2020, 18:31 Daniel P. Berrangé  ha
scritto:

> If Windows EXE files are built with -pie/-fpie they will fail to
> launch. Historically QEMU defaulted to disabling PIE for Windows,
> but this setting was accidentally lost when the configure summary
> text was removed in
>
>   commit f9332757898a764d85e19d339ec421236e885b68
>   Author: Paolo Bonzini 
>   Date:   Mon Feb 3 13:28:38 2020 +0100
>
> meson: move summary to meson.build
>
> Signed-off-by: Paolo Bonzini 
>
> Fixes: f9332757898a764d85e19d339ec421236e885b68
> Signed-off-by: Daniel P. Berrangé 
> ---
>  configure | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/configure b/configure
> index 67832e3bab..b6f3b6e191 100755
> --- a/configure
> +++ b/configure
> @@ -857,6 +857,7 @@ MINGW32*)
>  audio_drv_list=""
>fi
>supported_os="yes"
> +  pie="no"
>  ;;
>  GNU/kFreeBSD)
>bsd="yes"
> --
> 2.26.2
>
>

Re: [PATCH] meson: Fix meson build with --enable-libdaxctl

[Paolo Bonzini]

Queued, thanks.

Paolo

Il lun 24 ago 2020, 17:52 Bruce Rogers  ha scritto:

> The daxctl library needs to be linked against when daxctl is asked for
> in configure.
>
> Signed-off-by: Bruce Rogers 
> ---
>  configure   | 1 +
>  meson.build | 6 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 67832e3bab..15de7d10de 100755
> --- a/configure
> +++ b/configure
> @@ -7488,6 +7488,7 @@ fi
>
>  if test "$libdaxctl" = "yes" ; then
>echo "CONFIG_LIBDAXCTL=y" >> $config_host_mak
> +  echo "LIBDAXCTL_LIBS=$libdaxctl_libs" >> $config_host_mak
>  fi
>
>  if test "$bochs" = "yes" ; then
> diff --git a/meson.build b/meson.build
> index df5bf728b5..0a24e5c31a 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -380,6 +380,10 @@ if 'CONFIG_LIBPMEM' in config_host
>libpmem = declare_dependency(compile_args:
> config_host['LIBPMEM_CFLAGS'].split(),
> link_args:
> config_host['LIBPMEM_LIBS'].split())
>  endif
> +libdaxctl = not_found
> +if 'CONFIG_LIBDAXCTL' in config_host
> +  libdaxctl = declare_dependency(link_args:
> config_host['LIBDAXCTL_LIBS'].split())
> +endif
>
>  # Create config-host.h
>
> @@ -786,7 +790,7 @@ common_ss.add(files('cpus-common.c'))
>
>  subdir('softmmu')
>
> -specific_ss.add(files('disas.c', 'exec.c', 'gdbstub.c'), capstone,
> libpmem)
> +specific_ss.add(files('disas.c', 'exec.c', 'gdbstub.c'), capstone,
> libpmem, libdaxctl)
>  specific_ss.add(files('exec-vary.c'))
>  specific_ss.add(when: 'CONFIG_TCG', if_true: files(
>'fpu/softfloat.c',
> --
> 2.28.0
>
>

Re: [PATCH] meson: Fix chardev-baum.so name

[Paolo Bonzini]

Queued, thanks.

Paolo

Il lun 24 ago 2020, 17:52 Bruce Rogers  ha scritto:

> Somehow in the conversion to meson, the module named chardev-baum got
> renamed to chardev-brlapi. Change it back.
>
> Signed-off-by: Bruce Rogers 
> ---
>  chardev/meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/chardev/meson.build b/chardev/meson.build
> index a46a6237be..7726837e34 100644
> --- a/chardev/meson.build
> +++ b/chardev/meson.build
> @@ -39,7 +39,7 @@ chardev_modules = {}
>  if config_host.has_key('CONFIG_BRLAPI') and sdl.found()
>module_ss = ss.source_set()
>module_ss.add(when: [sdl, brlapi], if_true: files('baum.c'))
> -  chardev_modules += { 'brlapi': module_ss }
> +  chardev_modules += { 'baum': module_ss }
>  endif
>
>  modules += { 'chardev': chardev_modules }
> --
> 2.28.0
>
>

Re: meson: problems building under msys2/mingw-w64 native

[Mark Cave-Ayland]

On 24/08/2020 12:37, Gerd Hoffmann wrote:

>> 2) GTK UI now depends on CONFIG_VTE
>>
>> This one I spotted on my local Linux setup as I didn't have the libvte-dev 
>> package
>> installed and couldn't understand why I couldn't run QEMU with the GTK UI as 
>> I always
>> do, even though configure reported that it found the GTK library and headers.
>>
>> A quick search showed that the GTK UI was being guarded by "if
>> config_host.has_key('CONFIG_GTK') and config_host.has_key('CONFIG_VTE')" in
>> ui/meson.build.
> 
> That is not correct.  vte is intentionally not a hard dependency ...
> 
>> For me the easy solution was to install libvte-dev, but since there are no 
>> VTE
>> packages for Windows my guess is this will now make the GTK UI unavailable 
>> for
>> Windows users.
> 
> .. because we don't have that on windows.
> 
> I think simply dropping the "and config_host.has_key('CONFIG_VTE')"
> should work, can you try that?

Hi Gerd,

I can't get the native Windows build to complete yet, however I've removed the
libvte-dev headers again on my Linux setup and confirmed that GTK works once 
again
with the below diff:

diff --git a/ui/meson.build b/ui/meson.build
index 81fd393432..cc71f51f37 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -42,7 +42,7 @@ if config_host.has_key('CONFIG_CURSES')
   ui_modules += {'curses' : curses_ss}
 endif

-if config_host.has_key('CONFIG_GTK') and config_host.has_key('CONFIG_VTE')
+if config_host.has_key('CONFIG_GTK')
   softmmu_ss.add(when: 'CONFIG_WIN32', if_true: files('win32-kbd-hook.c'))

   gtk_ss = ss.source_set()


ATB,

Mark.

Re: [PATCH v2 0/2] meson: avoid compiling qemu-keymap by default

[Paolo Bonzini]

Queued, thanks.

Paolo

Il lun 24 ago 2020, 17:24 Laurent Vivier  ha scritto:

> We don't need it with linux-user only build, and if xkbcommon dynamic
> library is detected it can break the build of static only binaries.
>
> So disable it if it is no explicitly asked by the user when neither
> system or tools are built.
>
> build qemu-keymap:
>   configure --disable-system --disable-tools --disable-user
> --enable-xkbcommon
>   configure --disable-system --enable-tools --disable-user
>   configure --enable-system --disable-tools --disable-user
>
> don't build qemu-keymap:
>   configure --disable-system --disable-tools --disable-user
>   configure --disable-system --disable-tools --enable-user
>
> Laurent Vivier (2):
>   meson: move xkbcommon to meson
>   meson: avoid compiling qemu-keymap by default
>
>  configure | 29 -
>  meson.build   | 16 +++-
>  meson_options.txt |  1 +
>  ui/meson.build|  2 +-
>  4 files changed, 17 insertions(+), 31 deletions(-)
>
> --
> 2.26.2
>
>
>

Re: meson: problems building under msys2/mingw-w64 native

[Paolo Bonzini]

Great, thanks! Can you send it as a patch? I am collecting Meson fixes and
I should be able to send a pull request this week.

Also if you can please test the msys fixes that were sent on the list that
would be great.

Paolo

Il mar 25 ago 2020, 09:52 Mark Cave-Ayland 
ha scritto:

> On 24/08/2020 12:37, Gerd Hoffmann wrote:
>
> >> 2) GTK UI now depends on CONFIG_VTE
> >>
> >> This one I spotted on my local Linux setup as I didn't have the
> libvte-dev package
> >> installed and couldn't understand why I couldn't run QEMU with the GTK
> UI as I always
> >> do, even though configure reported that it found the GTK library and
> headers.
> >>
> >> A quick search showed that the GTK UI was being guarded by "if
> >> config_host.has_key('CONFIG_GTK') and
> config_host.has_key('CONFIG_VTE')" in
> >> ui/meson.build.
> >
> > That is not correct.  vte is intentionally not a hard dependency ...
> >
> >> For me the easy solution was to install libvte-dev, but since there are
> no VTE
> >> packages for Windows my guess is this will now make the GTK UI
> unavailable for
> >> Windows users.
> >
> > .. because we don't have that on windows.
> >
> > I think simply dropping the "and config_host.has_key('CONFIG_VTE')"
> > should work, can you try that?
>
> Hi Gerd,
>
> I can't get the native Windows build to complete yet, however I've removed
> the
> libvte-dev headers again on my Linux setup and confirmed that GTK works
> once again
> with the below diff:
>
> diff --git a/ui/meson.build b/ui/meson.build
> index 81fd393432..cc71f51f37 100644
> --- a/ui/meson.build
> +++ b/ui/meson.build
> @@ -42,7 +42,7 @@ if config_host.has_key('CONFIG_CURSES')
>ui_modules += {'curses' : curses_ss}
>  endif
>
> -if config_host.has_key('CONFIG_GTK') and config_host.has_key('CONFIG_VTE')
> +if config_host.has_key('CONFIG_GTK')
>softmmu_ss.add(when: 'CONFIG_WIN32', if_true: files('win32-kbd-hook.c'))
>
>gtk_ss = ss.source_set()
>
>
> ATB,
>
> Mark.
>
>

Re: [Fwd] Issue 25164 in oss-fuzz: qemu: Fuzzing build failure

[Paolo Bonzini]

Il lun 24 ago 2020, 00:58 Alexander Bulekov  ha scritto:

> Hi Paolo,
> Our oss-fuzz builds started failing, after the meson merge. I think I
> tracked down the issues:
> 1.) Looking at the build-log here:
>
> https://oss-fuzz-build-logs.storage.googleapis.com/log-d43d402c-1ce5-4422-b3db-ccbf83a862a0.txt
> The error happens at link-time. Re-running the build with V=1:
> "/usr/bin/ld" ...
> --whole-archive /usr/local/lib/clang/12.0.0/.../libclang_rt.asan-x86_64.a \
> --start-group . -T /src/qemu/tests/qtest/fuzz/fork_fuzz.ld  \
> -wrap qtest_inb -wrap qtest_inw . --end-group .
>

I think you can put everything into a response for and include it with
@fuzz.cmd in the command line.


> 2.) 77afc75f69 ("oss-fuzz/build: remove LIB_FUZZING_ENGINE")
>
> On oss-fuzz, we cannot explicitly specify fsanitize=fuzzer: We have to
> leverage the $CC $CXX $CFLAGS $CXXFLAGS $LIB_FUZZING_ENGINE from
> oss-fuzz. That was the reason for the "make CONFIG_FUZZ CFLAGS" trickery
> in the original build script.
>
> Details:
>
> https://google.github.io/oss-fuzz/getting-started/new-project-guide/#Requirements
>
> To work around this, I think we can create separate configure options
> --enable-oss-fuzz and --oss-fuzz-cflags and CONFIG_OSS_FUZZ. In meson we
> create a new "source-set" specific_oss_fuzz_ss which is identical to
> specific_fuzz_ss, except it does not depend on "-fsanitize=fuzzer",
> which is specified in tests/qtest/fuzz/meson.build
>
> I've been working on patches to do (2)


Great, I can review them. Should you use $LIB_FUZZING_ENGINE directly
instead of a separate command line option?

Paolo

but I don't know how to fix (1).
> Do you have any ideas?
>
> -Alex
>
> - Forwarded message from ClusterFuzz-External via monorail <
> monorail+v2.382749...@chromium.org> -
>
> Date: Sun, 23 Aug 2020 03:10:14 -0700
> From: ClusterFuzz-External via monorail <
> monorail+v2.382749...@chromium.org>
> To: alx...@bu.edu
> Subject: Issue 25164 in oss-fuzz: qemu: Fuzzing build failure
>
> Status: New
> Owner: 
> CC: b...@redhat.com, stefa...@redhat.com, alx...@bu.edu,
> pbonz...@redhat.com, darren.k...@oracle.com
> Labels: Proj-qemu
> Type: Build-Failure
>
> New issue 25164 by ClusterFuzz-External: qemu: Fuzzing build failure
> https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=25164
>
> The last 2 builds for qemu have been failing.
> Build log:
> https://oss-fuzz-build-logs.storage.googleapis.com/log-d43d402c-1ce5-4422-b3db-ccbf83a862a0.txt
> Build type: fuzzing
>
> To reproduce locally, please see:
> https://google.github.io/oss-fuzz/advanced-topics/reproducing#reproducing-build-failures
>
> This bug tracker is not being monitored by OSS-Fuzz team. If you have any
> questions, please create an issue at
> https://github.com/google/oss-fuzz/issues/new.
>
> **This bug will be automatically closed within a day once it is fixed.**
>
> --
> You received this message because:
>   1. You were specifically CC'd on the issue
>
> You may adjust your notification preferences at:
> https://bugs.chromium.org/hosting/settings
>
> Reply to this email to add a comment.
>
> - End forwarded message -
>
>

Re: [PATCH v2 0/7] Run cross-compilation build tests in the gitlab-CI

[Paolo Bonzini]

Build system parts

Acked-by: Paolo Bonzini 

Il dom 23 ago 2020, 13:18 Thomas Huth  ha scritto:

> Now that we can use all our QEMU build containers in the gitlab-CI,
> we can also run the cross-compilation jobs there. Of course, some
> problems have to be fixed first, so this is taken care of in the first
> four patches.
>
> The following two patches make sure that we can also enable WHPX builds
> with
> our debian-win64-cross container, so that we can compile-test this
> accelerator
> code now, too.
>
> The last patch then finally enables the cross-compilation jobs in the CI.
>
> v2:
>  - Dropped patches that are not necessary anymore
>  - Added the first two patches to fix problems with the new meson build
>system
>
> Thomas Huth (7):
>   configure: Add system = 'linux' for meson when cross-compiling
>   tests/docker: Install python3-setuptools in the debian9-mxe containers
>   tests/Makefile: test-image-locking needs CONFIG_POSIX
>   tests/Makefile: test-replication needs CONFIG_POSIX
>   dockerfiles/debian-win64-cross: Download WHPX MinGW headers
>   configure: Allow automatic WHPX detection
>   gitlab-ci: Add cross-compiling build tests
>
>  .gitlab-ci.d/crossbuilds.yml  | 113 ++
>  .gitlab-ci.yml|   1 +
>  MAINTAINERS   |   1 +
>  configure |   4 +
>  tests/Makefile.include|   6 +-
>  .../dockerfiles/debian-win64-cross.docker |   9 +-
>  tests/docker/dockerfiles/debian9-mxe.docker   |   2 +-
>  7 files changed, 133 insertions(+), 3 deletions(-)
>  create mode 100644 .gitlab-ci.d/crossbuilds.yml
>
> --
> 2.18.2
>
>

Re: [PATCH] scripts/qemu-version.sh: Add missing space before ']'

[Paolo Bonzini]

Queued, thanks.

Paolo

Il dom 23 ago 2020, 12:26 Thomas Huth  ha scritto:

> When configure has been run with --with-pkgversion=xyz, the shell complains
> about a missing ']' in this script.
>
> Fixes: 2c273f32d3 ("meson: generate qemu-version.h")
> Signed-off-by: Thomas Huth 
> ---
>  scripts/qemu-version.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/qemu-version.sh b/scripts/qemu-version.sh
> index 4847385e42..03128c56a2 100755
> --- a/scripts/qemu-version.sh
> +++ b/scripts/qemu-version.sh
> @@ -6,7 +6,7 @@ dir="$1"
>  pkgversion="$2"
>  version="$3"
>
> -if [ -z "$pkgversion"]; then
> +if [ -z "$pkgversion" ]; then
>  cd "$dir"
>  if [ -e .git ]; then
>  pkgversion=$(git describe --match 'v*' --dirty | echo "")
> --
> 2.18.2
>
>

Re: [PATCH] meson: set colorout to auto

[Gerd Hoffmann]

On Tue, Aug 25, 2020 at 10:59:42AM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Aug 25, 2020 at 10:44 AM Gerd Hoffmann  wrote:
> 
> > Dunno why the default is set to "always".  IMHO it should be "auto",
> > i.e. only colorize in case stdout goes to a terminal.  Cluttering
> > logfiles and confusing compiler message parsers with terminal control
> > sequences is not nice ...
> >
> > Signed-off-by: Gerd Hoffmann 
> >
> 
> "Enable colored output with GCC. Ninja redirects stdout/stderr so by
> default GCC thinks it is not talking to a terminal"
> 
> https://github.com/mesonbuild/meson/commit/4f63fe498314c385de2d3b6a3a953d15985914d2

Hmm, maybe ninja handles this then, by stripping the terminal sequences
in case stdout isn't a terminal.

With ninja being the default backend the default kind-of makes sense
(for meson upstream) ...

> Since we use make, I don't know if it's any better.

... but given qemu uses make not ninja we might prefer something else ;)

As far I know make doesn't redirect output.  Or maybe it redirects using
a pty (instead of a pipe) in case stdout is a terminal.  At least auto
mode for colored gcc warnings works fine with make.  It is colored when
started in a terminal, it isn't when started in emacs (and piped through
the emacs message parser).

> Perhaps meson should set compiler/tools colors = always/never based on what
> it is connected to at configure time instead?

Why?  Even when running configure in a terminal I might use emacs for
builds later on.

take care,
  Gerd

Re: [PATCH] meson: set colorout to auto

[Marc-André Lureau]

Hi

On Tue, Aug 25, 2020 at 12:07 PM Gerd Hoffmann  wrote:

> On Tue, Aug 25, 2020 at 10:59:42AM +0400, Marc-André Lureau wrote:
> > Hi
> >
> > On Tue, Aug 25, 2020 at 10:44 AM Gerd Hoffmann 
> wrote:
> >
> > > Dunno why the default is set to "always".  IMHO it should be "auto",
> > > i.e. only colorize in case stdout goes to a terminal.  Cluttering
> > > logfiles and confusing compiler message parsers with terminal control
> > > sequences is not nice ...
> > >
> > > Signed-off-by: Gerd Hoffmann 
> > >
> >
> > "Enable colored output with GCC. Ninja redirects stdout/stderr so by
> > default GCC thinks it is not talking to a terminal"
> >
> >
> https://github.com/mesonbuild/meson/commit/4f63fe498314c385de2d3b6a3a953d15985914d2
>
> Hmm, maybe ninja handles this then, by stripping the terminal sequences
> in case stdout isn't a terminal.
>
> With ninja being the default backend the default kind-of makes sense
> (for meson upstream) ...
>
> > Since we use make, I don't know if it's any better.
>
> ... but given qemu uses make not ninja we might prefer something else ;)
>
> As far I know make doesn't redirect output.  Or maybe it redirects using
> a pty (instead of a pipe) in case stdout is a terminal.  At least auto
> mode for colored gcc warnings works fine with make.  It is colored when
> started in a terminal, it isn't when started in emacs (and piped through
> the emacs message parser).
>
>
Then it looks like you have a legit patch. We can revert it when we switch
back to ninja some day.

Reviewed-by: Marc-André Lureau 

> Perhaps meson should set compiler/tools colors = always/never based on
> what
> > it is connected to at configure time instead?
>
> Why?  Even when running configure in a terminal I might use emacs for
> builds later on.
>
> take care,
>   Gerd
>
>

-- 
Marc-André Lureau

Re: [PATCH v5 0/8] Remove EPYC mode apicid decode and use generic decode

[Dr. David Alan Gilbert]

* Babu Moger (babu.mo...@amd.com) wrote:
> Hi Dave,
> 
> On 8/24/20 1:41 PM, Dr. David Alan Gilbert wrote:
> > * Babu Moger (babu.mo...@amd.com) wrote:
> >> To support some of the complex topology, we introduced EPYC mode apicid 
> >> decode.
> >> But, EPYC mode decode is running into problems. Also it can become quite a
> >> maintenance problem in the future. So, it was decided to remove that code 
> >> and
> >> use the generic decode which works for majority of the topology. Most of 
> >> the
> >> SPECed configuration would work just fine. With some non-SPECed user 
> >> inputs,
> >> it will create some sub-optimal configuration.
> >> Here is the discussion thread.
> >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fqemu-devel%2Fc0bcc1a6-1d84-a6e7-e468-d5b437c1b254%40amd.com%2F&data=02%7C01%7Cbabu.moger%40amd.com%7C74d90724af9c4adcc75008d8485d4d16%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637338912853492167&sdata=GTsMKcpeYXAA0CvpLTirPHKdNSdlJE3RuPjCtSyWtGQ%3D&reserved=0
> >>
> >> This series removes all the EPYC mode specific apicid changes and use the 
> >> generic
> >> apicid decode.
> > 
> > Hi Babu,
> >   This does simplify things a lot!
> > One worry, what happens about a live migration of a VM from an old qemu
> > that was using the node-id to a qemu with this new scheme?
> 
> The node_id which we introduced was only used internally. This wasn't
> exposed outside. I don't think live migration will be an issue.

Didn't it become part of the APIC ID visible to the guest?

Dave

-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: meson: problems building under msys2/mingw-w64 native

[Yonggang Luo]

Hi Paolo Bonzini,
I've already sent a series of fixes for msys, do you have a look at that?


On Tue, Aug 25, 2020 at 3:55 PM Paolo Bonzini  wrote:

> Great, thanks! Can you send it as a patch? I am collecting Meson fixes and
> I should be able to send a pull request this week.
>
> Also if you can please test the msys fixes that were sent on the list that
> would be great.
>
> Paolo
>
> Il mar 25 ago 2020, 09:52 Mark Cave-Ayland 
> ha scritto:
>
>> On 24/08/2020 12:37, Gerd Hoffmann wrote:
>>
>> >> 2) GTK UI now depends on CONFIG_VTE
>> >>
>> >> This one I spotted on my local Linux setup as I didn't have the
>> libvte-dev package
>> >> installed and couldn't understand why I couldn't run QEMU with the GTK
>> UI as I always
>> >> do, even though configure reported that it found the GTK library and
>> headers.
>> >>
>> >> A quick search showed that the GTK UI was being guarded by "if
>> >> config_host.has_key('CONFIG_GTK') and
>> config_host.has_key('CONFIG_VTE')" in
>> >> ui/meson.build.
>> >
>> > That is not correct.  vte is intentionally not a hard dependency ...
>> >
>> >> For me the easy solution was to install libvte-dev, but since there
>> are no VTE
>> >> packages for Windows my guess is this will now make the GTK UI
>> unavailable for
>> >> Windows users.
>> >
>> > .. because we don't have that on windows.
>> >
>> > I think simply dropping the "and config_host.has_key('CONFIG_VTE')"
>> > should work, can you try that?
>>
>> Hi Gerd,
>>
>> I can't get the native Windows build to complete yet, however I've
>> removed the
>> libvte-dev headers again on my Linux setup and confirmed that GTK works
>> once again
>> with the below diff:
>>
>> diff --git a/ui/meson.build b/ui/meson.build
>> index 81fd393432..cc71f51f37 100644
>> --- a/ui/meson.build
>> +++ b/ui/meson.build
>> @@ -42,7 +42,7 @@ if config_host.has_key('CONFIG_CURSES')
>>ui_modules += {'curses' : curses_ss}
>>  endif
>>
>> -if config_host.has_key('CONFIG_GTK') and
>> config_host.has_key('CONFIG_VTE')
>> +if config_host.has_key('CONFIG_GTK')
>>softmmu_ss.add(when: 'CONFIG_WIN32', if_true:
>> files('win32-kbd-hook.c'))
>>
>>gtk_ss = ss.source_set()
>>
>>
>> ATB,
>>
>> Mark.
>>
>>

-- 
 此致
礼
罗勇刚
Yours
sincerely,
Yonggang Luo

Re: [RFC v4 00/70] support vector extension v1.0

[Frank Chang]

On Mon, Aug 17, 2020 at 4:50 PM  wrote:

> From: Frank Chang 
>
> This patchset implements the vector extension v1.0 for RISC-V on QEMU.
>
> This patchset is sent as RFC because RVV v1.0 is still in draft state.
> v2 patchset was sent for RVV v0.9 and bumped to RVV v1.0 since v3 patchset.
>
> The port is available here:
> https://github.com/sifive/qemu/tree/rvv-1.0-upstream-v4
>
> You can change the cpu argument: vext_spec to v1.0 (i.e. vext_spec=v1.0)
> to run with RVV v1.0 instructions.
>
> Note: This patchset depends on two other patchsets listed in Based-on
>   section below so it might not able to be built unless those two
>   patchsets are applied.
>
> Changelog:
>
> v4
>   * remove explicit float flmul variable in DisasContext.
>   * replace floating-point calculations with shift operations to
> improve performance.
>   * relax RV_VLEN_MAX to 512-bits.
>
> v3
>   * apply nan-box helpers from Richard Henderson.
>   * remove fp16 api changes as they are sent independently in another
> pathcset by Chih-Min Chao.
>   * remove all tail elements clear functions as tail elements can
> retain unchanged for either VTA set to undisturbed or agnostic.
>   * add fp16 nan-box check generator function.
>   * add floating-point rounding mode enum.
>   * replace flmul arithmetic with shifts to avoid floating-point
> conversions.
>   * add Zvqmac extension.
>   * replace gdbstub vector register xml files with dynamic generator.
>   * bumped to RVV v1.0.
>   * RVV v1.0 related changes:
> * add vlre.v and vsr.v vector whole register
>   load/store instructions
> * add vrgatherei16 instruction.
> * rearranged bits in vtype to make vlmul bits into a contiguous
>   field.
>
> v2
>   * drop v0.7.1 support.
>   * replace invisible return check macros with functions.
>   * move mark_vs_dirty() to translators.
>   * add SSTATUS_VS flag for s-mode.
>   * nan-box scalar fp register for floating-point operations.
>   * add gdbstub files for vector registers to allow system-mode
> debugging with GDB.
>
> Based-on: <20200724002807.441147-1-richard.hender...@linaro.org/>
> Based-on: <1596102747-20226-1-git-send-email-chihmin.c...@sifive.com/>
>
> Frank Chang (62):
>   target/riscv: drop vector 0.7.1 and add 1.0 support
>   target/riscv: Use FIELD_EX32() to extract wd field
>   target/riscv: rvv-1.0: introduce writable misa.v field
>   target/riscv: rvv-1.0: remove rvv related codes from fcsr registers
>   target/riscv: rvv-1.0: check MSTATUS_VS when accessing vector csr
> registers
>   target/riscv: rvv-1.0: remove MLEN calculations
>   target/riscv: rvv-1.0: add fractional LMUL
>   target/riscv: rvv-1.0: add VMA and VTA
>   target/riscv: rvv-1.0: update check functions
>   target/riscv: introduce more imm value modes in translator functions
>   target/riscv: rvv:1.0: add translation-time nan-box helper function
>   target/riscv: rvv-1.0: configure instructions
>   target/riscv: rvv-1.0: stride load and store instructions
>   target/riscv: rvv-1.0: index load and store instructions
>   target/riscv: rvv-1.0: fix address index overflow bug of indexed
> load/store insns
>   target/riscv: rvv-1.0: fault-only-first unit stride load
>   target/riscv: rvv-1.0: amo operations
>   target/riscv: rvv-1.0: load/store whole register instructions
>   target/riscv: rvv-1.0: update vext_max_elems() for load/store insns
>   target/riscv: rvv-1.0: take fractional LMUL into vector max elements
> calculation
>   target/riscv: rvv-1.0: floating-point square-root instruction
>   target/riscv: rvv-1.0: floating-point classify instructions
>   target/riscv: rvv-1.0: mask population count instruction
>   target/riscv: rvv-1.0: find-first-set mask bit instruction
>   target/riscv: rvv-1.0: set-X-first mask bit instructions
>   target/riscv: rvv-1.0: iota instruction
>   target/riscv: rvv-1.0: element index instruction
>   target/riscv: rvv-1.0: allow load element with sign-extended
>   target/riscv: rvv-1.0: register gather instructions
>   target/riscv: rvv-1.0: integer scalar move instructions
>   target/riscv: rvv-1.0: floating-point move instruction
>   target/riscv: rvv-1.0: floating-point scalar move instructions
>   target/riscv: rvv-1.0: whole register move instructions
>   target/riscv: rvv-1.0: integer extension instructions
>   target/riscv: rvv-1.0: single-width averaging add and subtract
> instructions
>   target/riscv: rvv-1.0: single-width bit shift instructions
>   target/riscv: rvv-1.0: integer add-with-carry/subtract-with-borrow
>   target/riscv: rvv-1.0: narrowing integer right shift instructions
>   target/riscv: rvv-1.0: widening integer multiply-add instructions
>   target/riscv: rvv-1.0: add Zvqmac extension
>   target/riscv: rvv-1.0: quad-widening integer multiply-add instructions
>   target/riscv: rvv-1.0: single-width saturating add and subtract
> instructions
>   target/riscv: rvv-1.0: integer comparison instructions
>   target/riscv: use softfloat lib float16 c

[PULL 03/34] qcow2: Add calculate_l2_meta()

[Max Reitz]

From: Alberto Garcia 

handle_alloc() creates a QCowL2Meta structure in order to update the
image metadata and perform the necessary copy-on-write operations.

This patch moves that code to a separate function so it can be used
from other places.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Message-Id: 

Signed-off-by: Max Reitz 
---
 block/qcow2-cluster.c | 77 +--
 1 file changed, 53 insertions(+), 24 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 71ddb0c462..6447222ba0 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1038,6 +1038,56 @@ void qcow2_alloc_cluster_abort(BlockDriverState *bs, 
QCowL2Meta *m)
 }
 }
 
+/*
+ * For a given write request, create a new QCowL2Meta structure, add
+ * it to @m and the BDRVQcow2State.cluster_allocs list.
+ *
+ * @host_cluster_offset points to the beginning of the first cluster.
+ *
+ * @guest_offset and @bytes indicate the offset and length of the
+ * request.
+ *
+ * If @keep_old is true it means that the clusters were already
+ * allocated and will be overwritten. If false then the clusters are
+ * new and we have to decrease the reference count of the old ones.
+ */
+static void calculate_l2_meta(BlockDriverState *bs,
+  uint64_t host_cluster_offset,
+  uint64_t guest_offset, unsigned bytes,
+  QCowL2Meta **m, bool keep_old)
+{
+BDRVQcow2State *s = bs->opaque;
+unsigned cow_start_from = 0;
+unsigned cow_start_to = offset_into_cluster(s, guest_offset);
+unsigned cow_end_from = cow_start_to + bytes;
+unsigned cow_end_to = ROUND_UP(cow_end_from, s->cluster_size);
+unsigned nb_clusters = size_to_clusters(s, cow_end_from);
+QCowL2Meta *old_m = *m;
+
+*m = g_malloc0(sizeof(**m));
+**m = (QCowL2Meta) {
+.next   = old_m,
+
+.alloc_offset   = host_cluster_offset,
+.offset = start_of_cluster(s, guest_offset),
+.nb_clusters= nb_clusters,
+
+.keep_old_clusters = keep_old,
+
+.cow_start = {
+.offset = cow_start_from,
+.nb_bytes   = cow_start_to - cow_start_from,
+},
+.cow_end = {
+.offset = cow_end_from,
+.nb_bytes   = cow_end_to - cow_end_from,
+},
+};
+
+qemu_co_queue_init(&(*m)->dependent_requests);
+QLIST_INSERT_HEAD(&s->cluster_allocs, *m, next_in_flight);
+}
+
 /*
  * Returns the number of contiguous clusters that can be used for an allocating
  * write, but require COW to be performed (this includes yet unallocated space,
@@ -1436,35 +1486,14 @@ static int handle_alloc(BlockDriverState *bs, uint64_t 
guest_offset,
 uint64_t requested_bytes = *bytes + offset_into_cluster(s, guest_offset);
 int avail_bytes = nb_clusters << s->cluster_bits;
 int nb_bytes = MIN(requested_bytes, avail_bytes);
-QCowL2Meta *old_m = *m;
-
-*m = g_malloc0(sizeof(**m));
-
-**m = (QCowL2Meta) {
-.next   = old_m,
-
-.alloc_offset   = alloc_cluster_offset,
-.offset = start_of_cluster(s, guest_offset),
-.nb_clusters= nb_clusters,
-
-.keep_old_clusters  = keep_old_clusters,
-
-.cow_start = {
-.offset = 0,
-.nb_bytes   = offset_into_cluster(s, guest_offset),
-},
-.cow_end = {
-.offset = nb_bytes,
-.nb_bytes   = avail_bytes - nb_bytes,
-},
-};
-qemu_co_queue_init(&(*m)->dependent_requests);
-QLIST_INSERT_HEAD(&s->cluster_allocs, *m, next_in_flight);
 
 *host_offset = alloc_cluster_offset + offset_into_cluster(s, guest_offset);
 *bytes = MIN(*bytes, nb_bytes - offset_into_cluster(s, guest_offset));
 assert(*bytes != 0);
 
+calculate_l2_meta(bs, alloc_cluster_offset, guest_offset, *bytes,
+  m, keep_old_clusters);
+
 return 1;
 
 fail:
-- 
2.26.2

[PULL 00/34] Block patches

[Max Reitz]

The following changes since commit 30aa19446d82358a30eac3b556b4d6641e00b7c1:

  Merge remote-tracking branch 'remotes/cschoenebeck/tags/pull-9p-20200812' 
into staging (2020-08-24 16:39:53 +0100)

are available in the Git repository at:

  https://github.com/XanClic/qemu.git tags/pull-block-2020-08-25

for you to fetch changes up to c576fd97d4ca77b5a1a27728df11a61083dbfa98:

  iotests: Add tests for qcow2 images with extended L2 entries (2020-08-25 
10:20:18 +0200)


Block patches:
- qcow2 subclusters (extended L2 entries)


Alberto Garcia (34):
  qcow2: Make Qcow2AioTask store the full host offset
  qcow2: Convert qcow2_get_cluster_offset() into qcow2_get_host_offset()
  qcow2: Add calculate_l2_meta()
  qcow2: Split cluster_needs_cow() out of count_cow_clusters()
  qcow2: Process QCOW2_CLUSTER_ZERO_ALLOC clusters in handle_copied()
  qcow2: Add get_l2_entry() and set_l2_entry()
  qcow2: Document the Extended L2 Entries feature
  qcow2: Add dummy has_subclusters() function
  qcow2: Add subcluster-related fields to BDRVQcow2State
  qcow2: Add offset_to_sc_index()
  qcow2: Add offset_into_subcluster() and size_to_subclusters()
  qcow2: Add l2_entry_size()
  qcow2: Update get/set_l2_entry() and add get/set_l2_bitmap()
  qcow2: Add QCow2SubclusterType and qcow2_get_subcluster_type()
  qcow2: Add qcow2_get_subcluster_range_type()
  qcow2: Add qcow2_cluster_is_allocated()
  qcow2: Add cluster type parameter to qcow2_get_host_offset()
  qcow2: Replace QCOW2_CLUSTER_* with QCOW2_SUBCLUSTER_*
  qcow2: Handle QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC
  qcow2: Add subcluster support to calculate_l2_meta()
  qcow2: Add subcluster support to qcow2_get_host_offset()
  qcow2: Add subcluster support to zero_in_l2_slice()
  qcow2: Add subcluster support to discard_in_l2_slice()
  qcow2: Add subcluster support to check_refcounts_l2()
  qcow2: Update L2 bitmap in qcow2_alloc_cluster_link_l2()
  qcow2: Clear the L2 bitmap when allocating a compressed cluster
  qcow2: Add subcluster support to handle_alloc_space()
  qcow2: Add subcluster support to qcow2_co_pwrite_zeroes()
  qcow2: Add subcluster support to qcow2_measure()
  qcow2: Add prealloc field to QCowL2Meta
  qcow2: Add the 'extended_l2' option and the QCOW2_INCOMPAT_EXTL2 bit
  qcow2: Allow preallocation and backing files if extended_l2 is set
  qcow2: Assert that expand_zero_clusters_in_l1() does not support
subclusters
  iotests: Add tests for qcow2 images with extended L2 entries

 docs/interop/qcow2.txt   |  68 ++-
 docs/qcow2-cache.txt |  19 +-
 qapi/block-core.json |   7 +
 block/qcow2.h| 211 ++-
 include/block/block_int.h|   1 +
 block/qcow2-cluster.c| 906 +--
 block/qcow2-refcount.c   |  47 +-
 block/qcow2.c| 302 +++
 block/trace-events   |   2 +-
 tests/qemu-iotests/031.out   |   8 +-
 tests/qemu-iotests/036.out   |   4 +-
 tests/qemu-iotests/049.out   | 102 ++--
 tests/qemu-iotests/060.out   |   3 +-
 tests/qemu-iotests/061   |   6 +
 tests/qemu-iotests/061.out   |  25 +-
 tests/qemu-iotests/065   |  12 +-
 tests/qemu-iotests/082.out   |  39 +-
 tests/qemu-iotests/085.out   |  38 +-
 tests/qemu-iotests/144.out   |   4 +-
 tests/qemu-iotests/182.out   |   2 +-
 tests/qemu-iotests/185.out   |   8 +-
 tests/qemu-iotests/198   |   2 +
 tests/qemu-iotests/206.out   |   6 +-
 tests/qemu-iotests/242.out   |   5 +
 tests/qemu-iotests/255.out   |   8 +-
 tests/qemu-iotests/271   | 901 ++
 tests/qemu-iotests/271.out   | 726 +
 tests/qemu-iotests/274.out   |  49 +-
 tests/qemu-iotests/280.out   |   2 +-
 tests/qemu-iotests/291.out   |   2 +
 tests/qemu-iotests/302.out   |   1 +
 tests/qemu-iotests/303.out   |   4 +-
 tests/qemu-iotests/common.filter |   1 +
 tests/qemu-iotests/group |   1 +
 34 files changed, 2952 insertions(+), 570 deletions(-)
 create mode 100755 tests/qemu-iotests/271
 create mode 100644 tests/qemu-iotests/271.out

-- 
2.26.2

[PULL 07/34] qcow2: Document the Extended L2 Entries feature

[Max Reitz]

From: Alberto Garcia 

Subcluster allocation in qcow2 is implemented by extending the
existing L2 table entries and adding additional information to
indicate the allocation status of each subcluster.

This patch documents the changes to the qcow2 format and how they
affect the calculation of the L2 cache size.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Reviewed-by: Eric Blake 
Message-Id: 
<5199f2e1c717bcaa58b48142c9062b803145ff7f.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 docs/interop/qcow2.txt | 68 --
 docs/qcow2-cache.txt   | 19 +++-
 2 files changed, 83 insertions(+), 4 deletions(-)

diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index f072e27900..7da0d81df8 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -42,6 +42,9 @@ The first cluster of a qcow2 image contains the file header:
 as the maximum cluster size and won't be able to open 
images
 with larger cluster sizes.
 
+Note: if the image has Extended L2 Entries then 
cluster_bits
+must be at least 14 (i.e. 16384 byte clusters).
+
  24 - 31:   size
 Virtual disk size in bytes.
 
@@ -117,7 +120,12 @@ the next fields through header_length.
 clusters. The compression_type field must be
 present and not zero.
 
-Bits 4-63:  Reserved (set to 0)
+Bit 4:  Extended L2 Entries.  If this bit is set then
+L2 table entries use an extended format that
+allows subcluster-based allocation. See the
+Extended L2 Entries section for more details.
+
+Bits 5-63:  Reserved (set to 0)
 
  80 -  87:  compatible_features
 Bitmask of compatible features. An implementation can
@@ -498,7 +506,7 @@ cannot be relaxed without an incompatible layout change).
 Given an offset into the virtual disk, the offset into the image file can be
 obtained as follows:
 
-l2_entries = (cluster_size / sizeof(uint64_t))
+l2_entries = (cluster_size / sizeof(uint64_t))[*]
 
 l2_index = (offset / cluster_size) % l2_entries
 l1_index = (offset / cluster_size) / l2_entries
@@ -508,6 +516,8 @@ obtained as follows:
 
 return cluster_offset + (offset % cluster_size)
 
+[*] this changes if Extended L2 Entries are enabled, see next section
+
 L1 table entry:
 
 Bit  0 -  8:Reserved (set to 0)
@@ -548,7 +558,8 @@ Standard Cluster Descriptor:
 nor is data read from the backing file if the cluster is
 unallocated.
 
-With version 2, this is always 0.
+With version 2 or with extended L2 entries (see the next
+section), this is always 0.
 
  1 -  8:Reserved (set to 0)
 
@@ -585,6 +596,57 @@ file (except if bit 0 in the Standard Cluster Descriptor 
is set). If there is
 no backing file or the backing file is smaller than the image, they shall read
 zeros for all parts that are not covered by the backing file.
 
+== Extended L2 Entries ==
+
+An image uses Extended L2 Entries if bit 4 is set on the incompatible_features
+field of the header.
+
+In these images standard data clusters are divided into 32 subclusters of the
+same size. They are contiguous and start from the beginning of the cluster.
+Subclusters can be allocated independently and the L2 entry contains 
information
+indicating the status of each one of them. Compressed data clusters don't have
+subclusters so they are treated the same as in images without this feature.
+
+The size of an extended L2 entry is 128 bits so the number of entries per table
+is calculated using this formula:
+
+l2_entries = (cluster_size / (2 * sizeof(uint64_t)))
+
+The first 64 bits have the same format as the standard L2 table entry described
+in the previous section, with the exception of bit 0 of the standard cluster
+descriptor.
+
+The last 64 bits contain a subcluster allocation bitmap with this format:
+
+Subcluster Allocation Bitmap (for standard clusters):
+
+Bit  0 - 31:Allocation status (one bit per subcluster)
+
+1: the subcluster is allocated. In this case the
+   host cluster offset field must contain a valid
+   offset.
+0: the subcluster is not allocated. In this case
+   read requests shall go to the backing file or
+   return zeros if there is no backing file data.
+
+Bits are assigned starting from the least significant
+one (i.e. bit x is used for subcluster x).
+
+32 - 63 Subcluster reads as zeros (one bit per subcluster)
+
+1: the subcl

[PULL 04/34] qcow2: Split cluster_needs_cow() out of count_cow_clusters()

[Max Reitz]

From: Alberto Garcia 

We are going to need it in other places.

Signed-off-by: Alberto Garcia 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Max Reitz 
Message-Id: 
<65e5d9627ca2ebe7e62deaeddf60949c33067d9d.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2-cluster.c | 34 +++---
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 6447222ba0..369689b27c 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1088,6 +1088,24 @@ static void calculate_l2_meta(BlockDriverState *bs,
 QLIST_INSERT_HEAD(&s->cluster_allocs, *m, next_in_flight);
 }
 
+/* Returns true if writing to a cluster requires COW */
+static bool cluster_needs_cow(BlockDriverState *bs, uint64_t l2_entry)
+{
+switch (qcow2_get_cluster_type(bs, l2_entry)) {
+case QCOW2_CLUSTER_NORMAL:
+if (l2_entry & QCOW_OFLAG_COPIED) {
+return false;
+}
+case QCOW2_CLUSTER_UNALLOCATED:
+case QCOW2_CLUSTER_COMPRESSED:
+case QCOW2_CLUSTER_ZERO_PLAIN:
+case QCOW2_CLUSTER_ZERO_ALLOC:
+return true;
+default:
+abort();
+}
+}
+
 /*
  * Returns the number of contiguous clusters that can be used for an allocating
  * write, but require COW to be performed (this includes yet unallocated space,
@@ -1100,25 +1118,11 @@ static int count_cow_clusters(BlockDriverState *bs, int 
nb_clusters,
 
 for (i = 0; i < nb_clusters; i++) {
 uint64_t l2_entry = be64_to_cpu(l2_slice[l2_index + i]);
-QCow2ClusterType cluster_type = qcow2_get_cluster_type(bs, l2_entry);
-
-switch(cluster_type) {
-case QCOW2_CLUSTER_NORMAL:
-if (l2_entry & QCOW_OFLAG_COPIED) {
-goto out;
-}
+if (!cluster_needs_cow(bs, l2_entry)) {
 break;
-case QCOW2_CLUSTER_UNALLOCATED:
-case QCOW2_CLUSTER_COMPRESSED:
-case QCOW2_CLUSTER_ZERO_PLAIN:
-case QCOW2_CLUSTER_ZERO_ALLOC:
-break;
-default:
-abort();
 }
 }
 
-out:
 assert(i <= nb_clusters);
 return i;
 }
-- 
2.26.2

[PULL 01/34] qcow2: Make Qcow2AioTask store the full host offset

[Max Reitz]

From: Alberto Garcia 

The file_cluster_offset field of Qcow2AioTask stores a cluster-aligned
host offset. In practice this is not very useful because all users(*)
of this structure need the final host offset into the cluster, which
they calculate using

   host_offset = file_cluster_offset + offset_into_cluster(s, offset)

There is no reason why Qcow2AioTask cannot store host_offset directly
and that is what this patch does.

(*) compressed clusters are the exception: in this case what
file_cluster_offset was storing was the full compressed cluster
descriptor (offset + size). This does not change with this patch
but it is documented now.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Message-Id: 
<07c4b15c644dcf06c9459f98846ac1c4ea96e26f.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2.c  | 69 ++
 block/trace-events |  2 +-
 2 files changed, 34 insertions(+), 37 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 6ad6bdc166..5ab1f45fe2 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -74,7 +74,7 @@ typedef struct {
 
 static int coroutine_fn
 qcow2_co_preadv_compressed(BlockDriverState *bs,
-   uint64_t file_cluster_offset,
+   uint64_t cluster_descriptor,
uint64_t offset,
uint64_t bytes,
QEMUIOVector *qiov,
@@ -2113,7 +2113,7 @@ out:
 
 static coroutine_fn int
 qcow2_co_preadv_encrypted(BlockDriverState *bs,
-   uint64_t file_cluster_offset,
+   uint64_t host_offset,
uint64_t offset,
uint64_t bytes,
QEMUIOVector *qiov,
@@ -2140,16 +2140,12 @@ qcow2_co_preadv_encrypted(BlockDriverState *bs,
 }
 
 BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
-ret = bdrv_co_pread(s->data_file,
-file_cluster_offset + offset_into_cluster(s, offset),
-bytes, buf, 0);
+ret = bdrv_co_pread(s->data_file, host_offset, bytes, buf, 0);
 if (ret < 0) {
 goto fail;
 }
 
-if (qcow2_co_decrypt(bs,
- file_cluster_offset + offset_into_cluster(s, offset),
- offset, buf, bytes) < 0)
+if (qcow2_co_decrypt(bs, host_offset, offset, buf, bytes) < 0)
 {
 ret = -EIO;
 goto fail;
@@ -2167,7 +2163,7 @@ typedef struct Qcow2AioTask {
 
 BlockDriverState *bs;
 QCow2ClusterType cluster_type; /* only for read */
-uint64_t file_cluster_offset;
+uint64_t host_offset; /* or full descriptor in compressed clusters */
 uint64_t offset;
 uint64_t bytes;
 QEMUIOVector *qiov;
@@ -2180,7 +2176,7 @@ static coroutine_fn int qcow2_add_task(BlockDriverState 
*bs,
AioTaskPool *pool,
AioTaskFunc func,
QCow2ClusterType cluster_type,
-   uint64_t file_cluster_offset,
+   uint64_t host_offset,
uint64_t offset,
uint64_t bytes,
QEMUIOVector *qiov,
@@ -2195,7 +2191,7 @@ static coroutine_fn int qcow2_add_task(BlockDriverState 
*bs,
 .bs = bs,
 .cluster_type = cluster_type,
 .qiov = qiov,
-.file_cluster_offset = file_cluster_offset,
+.host_offset = host_offset,
 .offset = offset,
 .bytes = bytes,
 .qiov_offset = qiov_offset,
@@ -2204,7 +2200,7 @@ static coroutine_fn int qcow2_add_task(BlockDriverState 
*bs,
 
 trace_qcow2_add_task(qemu_coroutine_self(), bs, pool,
  func == qcow2_co_preadv_task_entry ? "read" : "write",
- cluster_type, file_cluster_offset, offset, bytes,
+ cluster_type, host_offset, offset, bytes,
  qiov, qiov_offset);
 
 if (!pool) {
@@ -2218,13 +2214,12 @@ static coroutine_fn int qcow2_add_task(BlockDriverState 
*bs,
 
 static coroutine_fn int qcow2_co_preadv_task(BlockDriverState *bs,
  QCow2ClusterType cluster_type,
- uint64_t file_cluster_offset,
+ uint64_t host_offset,
  uint64_t offset, uint64_t bytes,
  QEMUIOVector *qiov,
  size_t qiov_offset)
 {
 BDRVQcow2State *s = bs->opaque;
-int offset_in_cluster = offset_into_cluster(s, offset);
 
 switch (cluster_type) {
 case QCOW2_CLUSTER_ZERO_PLAIN:
@@ -22

[PULL 14/34] qcow2: Add QCow2SubclusterType and qcow2_get_subcluster_type()

[Max Reitz]

From: Alberto Garcia 

This patch adds QCow2SubclusterType, which is the subcluster-level
version of QCow2ClusterType. All QCOW2_SUBCLUSTER_* values have the
the same meaning as their QCOW2_CLUSTER_* equivalents (when they
exist). See below for details and caveats.

In images without extended L2 entries clusters are treated as having
exactly one subcluster so it is possible to replace one data type with
the other while keeping the exact same semantics.

With extended L2 entries there are new possible values, and every
subcluster in the same cluster can obviously have a different
QCow2SubclusterType so functions need to be adapted to work on the
subcluster level.

There are several things that have to be taken into account:

  a) QCOW2_SUBCLUSTER_COMPRESSED means that the whole cluster is
 compressed. We do not support compression at the subcluster
 level.

  b) There are two different values for unallocated subclusters:
 QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN which means that the whole
 cluster is unallocated, and QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC
 which means that the cluster is allocated but the subcluster is
 not. The latter can only happen in images with extended L2
 entries.

  c) QCOW2_SUBCLUSTER_INVALID is used to detect the cases where an L2
 entry has a value that violates the specification. The caller is
 responsible for handling these situations.

 To prevent compatibility problems with images that have invalid
 values but are currently being read by QEMU without causing side
 effects, QCOW2_SUBCLUSTER_INVALID is only returned for images
 with extended L2 entries.

qcow2_cluster_to_subcluster_type() is added as a separate function
from qcow2_get_subcluster_type(), but this is only temporary and both
will be merged in a subsequent patch.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Message-Id: 
<26ef38e270f25851c98b51278852b4c4a7f97e69.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2.h | 126 +-
 1 file changed, 125 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 82b86f6cec..3aec6f452a 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -80,6 +80,21 @@
 
 #define QCOW_EXTL2_SUBCLUSTERS_PER_CLUSTER 32
 
+/* The subcluster X [0..31] is allocated */
+#define QCOW_OFLAG_SUB_ALLOC(X)   (1ULL << (X))
+/* The subcluster X [0..31] reads as zeroes */
+#define QCOW_OFLAG_SUB_ZERO(X)(QCOW_OFLAG_SUB_ALLOC(X) << 32)
+/* Subclusters [X, Y) (0 <= X <= Y <= 32) are allocated */
+#define QCOW_OFLAG_SUB_ALLOC_RANGE(X, Y) \
+(QCOW_OFLAG_SUB_ALLOC(Y) - QCOW_OFLAG_SUB_ALLOC(X))
+/* Subclusters [X, Y) (0 <= X <= Y <= 32) read as zeroes */
+#define QCOW_OFLAG_SUB_ZERO_RANGE(X, Y) \
+(QCOW_OFLAG_SUB_ALLOC_RANGE(X, Y) << 32)
+/* L2 entry bitmap with all allocation bits set */
+#define QCOW_L2_BITMAP_ALL_ALLOC  (QCOW_OFLAG_SUB_ALLOC_RANGE(0, 32))
+/* L2 entry bitmap with all "read as zeroes" bits set */
+#define QCOW_L2_BITMAP_ALL_ZEROES (QCOW_OFLAG_SUB_ZERO_RANGE(0, 32))
+
 /* Size of normal and extended L2 entries */
 #define L2E_SIZE_NORMAL   (sizeof(uint64_t))
 #define L2E_SIZE_EXTENDED (sizeof(uint64_t) * 2)
@@ -462,6 +477,33 @@ typedef struct QCowL2Meta
 QLIST_ENTRY(QCowL2Meta) next_in_flight;
 } QCowL2Meta;
 
+/*
+ * In images with standard L2 entries all clusters are treated as if
+ * they had one subcluster so QCow2ClusterType and QCow2SubclusterType
+ * can be mapped to each other and have the exact same meaning
+ * (QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC cannot happen in these images).
+ *
+ * In images with extended L2 entries QCow2ClusterType refers to the
+ * complete cluster and QCow2SubclusterType to each of the individual
+ * subclusters, so there are several possible combinations:
+ *
+ * |--+---|
+ * | Cluster type | Possible subcluster types |
+ * |--+---|
+ * | UNALLOCATED  | UNALLOCATED_PLAIN |
+ * |  |ZERO_PLAIN |
+ * |--+---|
+ * | NORMAL   | UNALLOCATED_ALLOC |
+ * |  |ZERO_ALLOC |
+ * |  |NORMAL |
+ * |--+---|
+ * | COMPRESSED   |COMPRESSED |
+ * |--+---|
+ *
+ * QCOW2_SUBCLUSTER_INVALID means that the L2 entry is incorrect and
+ * the image should be marked corrupt.
+ */
+
 typedef enum QCow2ClusterType {
 QCOW2_CLUSTER_UNALLOCATED,
 QCOW2_CLUSTER_ZERO_PLAIN,
@@ -470,6 +512,16 @@ typedef enum QCow2ClusterType {
 QCOW2_CLUSTER_COMPRESSED,
 } QCow2ClusterType;
 
+typedef enum QCow2SubclusterType {
+QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN,
+QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC,
+QCOW2_SUBCLUSTER_ZERO_PLAIN,
+QCOW2_SUBCLUSTER_ZERO_ALLOC,

[PULL 05/34] qcow2: Process QCOW2_CLUSTER_ZERO_ALLOC clusters in handle_copied()

[Max Reitz]

From: Alberto Garcia 

When writing to a qcow2 file there are two functions that take a
virtual offset and return a host offset, possibly allocating new
clusters if necessary:

   - handle_copied() looks for normal data clusters that are already
 allocated and have a reference count of 1. In those clusters we
 can simply write the data and there is no need to perform any
 copy-on-write.

   - handle_alloc() looks for clusters that do need copy-on-write,
 either because they haven't been allocated yet, because their
 reference count is != 1 or because they are ZERO_ALLOC clusters.

The ZERO_ALLOC case is a bit special because those are clusters that
are already allocated and they could perfectly be dealt with in
handle_copied() (as long as copy-on-write is performed when required).

In fact, there is extra code specifically for them in handle_alloc()
that tries to reuse the existing allocation if possible and frees them
otherwise.

This patch changes the handling of ZERO_ALLOC clusters so the
semantics of these two functions are now like this:

   - handle_copied() looks for clusters that are already allocated and
 which we can overwrite (NORMAL and ZERO_ALLOC clusters with a
 reference count of 1).

   - handle_alloc() looks for clusters for which we need a new
 allocation (all other cases).

One important difference after this change is that clusters found
in handle_copied() may now require copy-on-write, but this will be
necessary anyway once we add support for subclusters.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Message-Id: 

Signed-off-by: Max Reitz 
---
 block/qcow2-cluster.c | 252 +++---
 1 file changed, 139 insertions(+), 113 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 369689b27c..b7b7b37062 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1040,13 +1040,18 @@ void qcow2_alloc_cluster_abort(BlockDriverState *bs, 
QCowL2Meta *m)
 
 /*
  * For a given write request, create a new QCowL2Meta structure, add
- * it to @m and the BDRVQcow2State.cluster_allocs list.
+ * it to @m and the BDRVQcow2State.cluster_allocs list. If the write
+ * request does not need copy-on-write or changes to the L2 metadata
+ * then this function does nothing.
  *
  * @host_cluster_offset points to the beginning of the first cluster.
  *
  * @guest_offset and @bytes indicate the offset and length of the
  * request.
  *
+ * @l2_slice contains the L2 entries of all clusters involved in this
+ * write request.
+ *
  * If @keep_old is true it means that the clusters were already
  * allocated and will be overwritten. If false then the clusters are
  * new and we have to decrease the reference count of the old ones.
@@ -1054,15 +1059,53 @@ void qcow2_alloc_cluster_abort(BlockDriverState *bs, 
QCowL2Meta *m)
 static void calculate_l2_meta(BlockDriverState *bs,
   uint64_t host_cluster_offset,
   uint64_t guest_offset, unsigned bytes,
-  QCowL2Meta **m, bool keep_old)
+  uint64_t *l2_slice, QCowL2Meta **m, bool 
keep_old)
 {
 BDRVQcow2State *s = bs->opaque;
-unsigned cow_start_from = 0;
+int l2_index = offset_to_l2_slice_index(s, guest_offset);
+uint64_t l2_entry;
+unsigned cow_start_from, cow_end_to;
 unsigned cow_start_to = offset_into_cluster(s, guest_offset);
 unsigned cow_end_from = cow_start_to + bytes;
-unsigned cow_end_to = ROUND_UP(cow_end_from, s->cluster_size);
 unsigned nb_clusters = size_to_clusters(s, cow_end_from);
 QCowL2Meta *old_m = *m;
+QCow2ClusterType type;
+
+assert(nb_clusters <= s->l2_slice_size - l2_index);
+
+/* Return if there's no COW (all clusters are normal and we keep them) */
+if (keep_old) {
+int i;
+for (i = 0; i < nb_clusters; i++) {
+l2_entry = be64_to_cpu(l2_slice[l2_index + i]);
+if (qcow2_get_cluster_type(bs, l2_entry) != QCOW2_CLUSTER_NORMAL) {
+break;
+}
+}
+if (i == nb_clusters) {
+return;
+}
+}
+
+/* Get the L2 entry of the first cluster */
+l2_entry = be64_to_cpu(l2_slice[l2_index]);
+type = qcow2_get_cluster_type(bs, l2_entry);
+
+if (type == QCOW2_CLUSTER_NORMAL && keep_old) {
+cow_start_from = cow_start_to;
+} else {
+cow_start_from = 0;
+}
+
+/* Get the L2 entry of the last cluster */
+l2_entry = be64_to_cpu(l2_slice[l2_index + nb_clusters - 1]);
+type = qcow2_get_cluster_type(bs, l2_entry);
+
+if (type == QCOW2_CLUSTER_NORMAL && keep_old) {
+cow_end_to = cow_end_from;
+} else {
+cow_end_to = ROUND_UP(cow_end_from, s->cluster_size);
+}
 
 *m = g_malloc0(sizeof(**m));
 **m = (QCowL2Meta) {
@@ -1088,18 +1131,22 @@ static void calculate_l2_meta(BlockDriverState *bs,
 QLI

[PULL 13/34] qcow2: Update get/set_l2_entry() and add get/set_l2_bitmap()

[Max Reitz]

From: Alberto Garcia 

Extended L2 entries are 128-bit wide: 64 bits for the entry itself and
64 bits for the subcluster allocation bitmap.

In order to support them correctly get/set_l2_entry() need to be
updated so they take the entry width into account in order to
calculate the correct offset.

This patch also adds the get/set_l2_bitmap() functions that are
used to access the bitmaps. For convenience we allow calling
get_l2_bitmap() on images without subclusters. In this case the
returned value is always 0 and has no meaning.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Message-Id: 
<6ee0f81ae3329c991de125618b3675e1e46acdbb.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2.h | 21 +
 1 file changed, 21 insertions(+)

diff --git a/block/qcow2.h b/block/qcow2.h
index 46b351229a..82b86f6cec 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -533,15 +533,36 @@ static inline size_t l2_entry_size(BDRVQcow2State *s)
 static inline uint64_t get_l2_entry(BDRVQcow2State *s, uint64_t *l2_slice,
 int idx)
 {
+idx *= l2_entry_size(s) / sizeof(uint64_t);
 return be64_to_cpu(l2_slice[idx]);
 }
 
+static inline uint64_t get_l2_bitmap(BDRVQcow2State *s, uint64_t *l2_slice,
+ int idx)
+{
+if (has_subclusters(s)) {
+idx *= l2_entry_size(s) / sizeof(uint64_t);
+return be64_to_cpu(l2_slice[idx + 1]);
+} else {
+return 0; /* For convenience only; this value has no meaning. */
+}
+}
+
 static inline void set_l2_entry(BDRVQcow2State *s, uint64_t *l2_slice,
 int idx, uint64_t entry)
 {
+idx *= l2_entry_size(s) / sizeof(uint64_t);
 l2_slice[idx] = cpu_to_be64(entry);
 }
 
+static inline void set_l2_bitmap(BDRVQcow2State *s, uint64_t *l2_slice,
+ int idx, uint64_t bitmap)
+{
+assert(has_subclusters(s));
+idx *= l2_entry_size(s) / sizeof(uint64_t);
+l2_slice[idx + 1] = cpu_to_be64(bitmap);
+}
+
 static inline bool has_data_file(BlockDriverState *bs)
 {
 BDRVQcow2State *s = bs->opaque;
-- 
2.26.2

[PULL 02/34] qcow2: Convert qcow2_get_cluster_offset() into qcow2_get_host_offset()

[Max Reitz]

From: Alberto Garcia 

qcow2_get_cluster_offset() takes an (unaligned) guest offset and
returns the (aligned) offset of the corresponding cluster in the qcow2
image.

In practice none of the callers need to know where the cluster starts
so this patch makes the function calculate and return the final host
offset directly. The function is also renamed accordingly.

There is a pre-existing exception with compressed clusters: in this
case the function returns the complete cluster descriptor (containing
the offset and size of the compressed data). This does not change with
this patch but it is now documented.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Message-Id: 

Signed-off-by: Max Reitz 
---
 block/qcow2.h |  4 ++--
 block/qcow2-cluster.c | 41 +++--
 block/qcow2.c | 24 +++-
 3 files changed, 32 insertions(+), 37 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 7ce2c23bdb..06475e0849 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -694,8 +694,8 @@ int qcow2_write_l1_entry(BlockDriverState *bs, int 
l1_index);
 int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t sector_num,
   uint8_t *buf, int nb_sectors, bool enc, Error 
**errp);
 
-int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
- unsigned int *bytes, uint64_t *cluster_offset);
+int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
+  unsigned int *bytes, uint64_t *host_offset);
 int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
unsigned int *bytes, uint64_t *host_offset,
QCowL2Meta **m);
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 550850b264..71ddb0c462 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -496,10 +496,15 @@ static int coroutine_fn 
do_perform_cow_write(BlockDriverState *bs,
 
 
 /*
- * get_cluster_offset
+ * get_host_offset
  *
- * For a given offset of the virtual disk, find the cluster type and offset in
- * the qcow2 file. The offset is stored in *cluster_offset.
+ * For a given offset of the virtual disk find the equivalent host
+ * offset in the qcow2 file and store it in *host_offset. Neither
+ * offset needs to be aligned to a cluster boundary.
+ *
+ * If the cluster is unallocated then *host_offset will be 0.
+ * If the cluster is compressed then *host_offset will contain the
+ * complete compressed cluster descriptor.
  *
  * On entry, *bytes is the maximum number of contiguous bytes starting at
  * offset that we are interested in.
@@ -511,12 +516,12 @@ static int coroutine_fn 
do_perform_cow_write(BlockDriverState *bs,
  * Returns the cluster type (QCOW2_CLUSTER_*) on success, -errno in error
  * cases.
  */
-int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
- unsigned int *bytes, uint64_t *cluster_offset)
+int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
+  unsigned int *bytes, uint64_t *host_offset)
 {
 BDRVQcow2State *s = bs->opaque;
 unsigned int l2_index;
-uint64_t l1_index, l2_offset, *l2_slice;
+uint64_t l1_index, l2_offset, *l2_slice, l2_entry;
 int c;
 unsigned int offset_in_cluster;
 uint64_t bytes_available, bytes_needed, nb_clusters;
@@ -537,7 +542,7 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t 
offset,
 bytes_needed = bytes_available;
 }
 
-*cluster_offset = 0;
+*host_offset = 0;
 
 /* seek to the l2 offset in the l1 table */
 
@@ -570,7 +575,7 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t 
offset,
 /* find the cluster offset for the given disk offset */
 
 l2_index = offset_to_l2_slice_index(s, offset);
-*cluster_offset = be64_to_cpu(l2_slice[l2_index]);
+l2_entry = be64_to_cpu(l2_slice[l2_index]);
 
 nb_clusters = size_to_clusters(s, bytes_needed);
 /* bytes_needed <= *bytes + offset_in_cluster, both of which are unsigned
@@ -578,7 +583,7 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t 
offset,
  * true */
 assert(nb_clusters <= INT_MAX);
 
-type = qcow2_get_cluster_type(bs, *cluster_offset);
+type = qcow2_get_cluster_type(bs, l2_entry);
 if (s->qcow_version < 3 && (type == QCOW2_CLUSTER_ZERO_PLAIN ||
 type == QCOW2_CLUSTER_ZERO_ALLOC)) {
 qcow2_signal_corruption(bs, true, -1, -1, "Zero cluster entry found"
@@ -599,42 +604,42 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
uint64_t offset,
 }
 /* Compressed clusters can only be processed one by one */
 c = 1;
-*cluster_offset &= L2E_COMPRESSED_OFFSET_SIZE_MASK;
+*host_offset = l2_entry & L2E_COMPRESSED_OFFSET_SIZE_MASK;
 break;
 case QCOW2_CLUSTER_ZERO_PLAIN:
 case QCOW2_CLUSTER_UNALLOCATED:
 /* how many em

[PULL 08/34] qcow2: Add dummy has_subclusters() function

[Max Reitz]

From: Alberto Garcia 

This function will be used by the qcow2 code to check if an image has
subclusters or not.

At the moment this simply returns false. Once all patches needed for
subcluster support are ready then QEMU will be able to create and
read images with subclusters and this function will return the actual
value.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Message-Id: 
<905526221083581a1b7057bca1585487661c5c13.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/block/qcow2.h b/block/qcow2.h
index eecbadc4cb..2064dd3d85 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -510,6 +510,12 @@ typedef enum QCow2MetadataOverlap {
 
 #define INV_OFFSET (-1ULL)
 
+static inline bool has_subclusters(BDRVQcow2State *s)
+{
+/* FIXME: Return false until this feature is complete */
+return false;
+}
+
 static inline uint64_t get_l2_entry(BDRVQcow2State *s, uint64_t *l2_slice,
 int idx)
 {
-- 
2.26.2

[PULL 19/34] qcow2: Handle QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC

[Max Reitz]

From: Alberto Garcia 

When dealing with subcluster types there is a new value called
QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC that has no equivalent in
QCow2ClusterType.

This patch handles that value in all places where subcluster types
are processed.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Message-Id: 

Signed-off-by: Max Reitz 
---
 block/qcow2.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 477d60dd71..60192f1be3 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2064,7 +2064,8 @@ static int coroutine_fn 
qcow2_co_block_status(BlockDriverState *bs,
 *pnum = bytes;
 
 if ((type == QCOW2_SUBCLUSTER_NORMAL ||
- type == QCOW2_SUBCLUSTER_ZERO_ALLOC) && !s->crypto) {
+ type == QCOW2_SUBCLUSTER_ZERO_ALLOC ||
+ type == QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC) && !s->crypto) {
 *map = host_offset;
 *file = s->data_file->bs;
 status |= BDRV_BLOCK_OFFSET_VALID;
@@ -2072,7 +2073,8 @@ static int coroutine_fn 
qcow2_co_block_status(BlockDriverState *bs,
 if (type == QCOW2_SUBCLUSTER_ZERO_PLAIN ||
 type == QCOW2_SUBCLUSTER_ZERO_ALLOC) {
 status |= BDRV_BLOCK_ZERO;
-} else if (type != QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN) {
+} else if (type != QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN &&
+   type != QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC) {
 status |= BDRV_BLOCK_DATA;
 }
 if (s->metadata_preallocation && (status & BDRV_BLOCK_DATA) &&
@@ -2235,6 +2237,7 @@ static coroutine_fn int 
qcow2_co_preadv_task(BlockDriverState *bs,
 g_assert_not_reached();
 
 case QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN:
+case QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC:
 assert(bs->backing); /* otherwise handled in qcow2_co_preadv_part */
 
 BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
@@ -2303,7 +2306,8 @@ static coroutine_fn int 
qcow2_co_preadv_part(BlockDriverState *bs,
 
 if (type == QCOW2_SUBCLUSTER_ZERO_PLAIN ||
 type == QCOW2_SUBCLUSTER_ZERO_ALLOC ||
-(type == QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN && !bs->backing))
+(type == QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN && !bs->backing) ||
+(type == QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC && !bs->backing))
 {
 qemu_iovec_memset(qiov, qiov_offset, 0, cur_bytes);
 } else {
@@ -3858,6 +3862,7 @@ static coroutine_fn int 
qcow2_co_pwrite_zeroes(BlockDriverState *bs,
 ret = qcow2_get_host_offset(bs, offset, &nr, &off, &type);
 if (ret < 0 ||
 (type != QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN &&
+ type != QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC &&
  type != QCOW2_SUBCLUSTER_ZERO_PLAIN &&
  type != QCOW2_SUBCLUSTER_ZERO_ALLOC)) {
 qemu_co_mutex_unlock(&s->lock);
@@ -3936,6 +3941,7 @@ qcow2_co_copy_range_from(BlockDriverState *bs,
 
 switch (type) {
 case QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN:
+case QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC:
 if (bs->backing && bs->backing->bs) {
 int64_t backing_length = bdrv_getlength(bs->backing->bs);
 if (src_offset >= backing_length) {
-- 
2.26.2

[PULL 15/34] qcow2: Add qcow2_get_subcluster_range_type()

[Max Reitz]

From: Alberto Garcia 

There are situations in which we want to know how many contiguous
subclusters of the same type there are in a given cluster. This can be
done by simply iterating over the subclusters and repeatedly calling
qcow2_get_subcluster_type() for each one of them.

However once we determined the type of a subcluster we can check the
rest efficiently by counting the number of adjacent ones (or zeroes)
in the bitmap. This is what this function does.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Message-Id: 

Signed-off-by: Max Reitz 
---
 block/qcow2-cluster.c | 51 +++
 1 file changed, 51 insertions(+)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 0b762502f6..2fe7a0f79c 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -375,6 +375,57 @@ fail:
 return ret;
 }
 
+/*
+ * For a given L2 entry, count the number of contiguous subclusters of
+ * the same type starting from @sc_from. Compressed clusters are
+ * treated as if they were divided into subclusters of size
+ * s->subcluster_size.
+ *
+ * Return the number of contiguous subclusters and set @type to the
+ * subcluster type.
+ *
+ * If the L2 entry is invalid return -errno and set @type to
+ * QCOW2_SUBCLUSTER_INVALID.
+ */
+G_GNUC_UNUSED
+static int qcow2_get_subcluster_range_type(BlockDriverState *bs,
+   uint64_t l2_entry,
+   uint64_t l2_bitmap,
+   unsigned sc_from,
+   QCow2SubclusterType *type)
+{
+BDRVQcow2State *s = bs->opaque;
+uint32_t val;
+
+*type = qcow2_get_subcluster_type(bs, l2_entry, l2_bitmap, sc_from);
+
+if (*type == QCOW2_SUBCLUSTER_INVALID) {
+return -EINVAL;
+} else if (!has_subclusters(s) || *type == QCOW2_SUBCLUSTER_COMPRESSED) {
+return s->subclusters_per_cluster - sc_from;
+}
+
+switch (*type) {
+case QCOW2_SUBCLUSTER_NORMAL:
+val = l2_bitmap | QCOW_OFLAG_SUB_ALLOC_RANGE(0, sc_from);
+return cto32(val) - sc_from;
+
+case QCOW2_SUBCLUSTER_ZERO_PLAIN:
+case QCOW2_SUBCLUSTER_ZERO_ALLOC:
+val = (l2_bitmap | QCOW_OFLAG_SUB_ZERO_RANGE(0, sc_from)) >> 32;
+return cto32(val) - sc_from;
+
+case QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN:
+case QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC:
+val = ((l2_bitmap >> 32) | l2_bitmap)
+& ~QCOW_OFLAG_SUB_ALLOC_RANGE(0, sc_from);
+return ctz32(val) - sc_from;
+
+default:
+g_assert_not_reached();
+}
+}
+
 /*
  * Checks how many clusters in a given L2 slice are contiguous in the image
  * file. As soon as one of the flags in the bitmask stop_flags changes compared
-- 
2.26.2

[PULL 06/34] qcow2: Add get_l2_entry() and set_l2_entry()

[Max Reitz]

From: Alberto Garcia 

The size of an L2 entry is 64 bits, but if we want to have subclusters
we need extended L2 entries. This means that we have to access L2
tables and slices differently depending on whether an image has
extended L2 entries or not.

This patch replaces all l2_slice[] accesses with calls to
get_l2_entry() and set_l2_entry().

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Message-Id: 
<9586363531fec125ba1386e561762d3e4224e9fc.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2.h  | 12 
 block/qcow2-cluster.c  | 63 ++
 block/qcow2-refcount.c | 17 ++--
 3 files changed, 54 insertions(+), 38 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 06475e0849..eecbadc4cb 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -510,6 +510,18 @@ typedef enum QCow2MetadataOverlap {
 
 #define INV_OFFSET (-1ULL)
 
+static inline uint64_t get_l2_entry(BDRVQcow2State *s, uint64_t *l2_slice,
+int idx)
+{
+return be64_to_cpu(l2_slice[idx]);
+}
+
+static inline void set_l2_entry(BDRVQcow2State *s, uint64_t *l2_slice,
+int idx, uint64_t entry)
+{
+l2_slice[idx] = cpu_to_be64(entry);
+}
+
 static inline bool has_data_file(BlockDriverState *bs)
 {
 BDRVQcow2State *s = bs->opaque;
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index b7b7b37062..5bd1e1feb8 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -383,12 +383,13 @@ fail:
  * cluster which may require a different handling)
  */
 static int count_contiguous_clusters(BlockDriverState *bs, int nb_clusters,
-int cluster_size, uint64_t *l2_slice, uint64_t stop_flags)
+int cluster_size, uint64_t *l2_slice, int l2_index, uint64_t 
stop_flags)
 {
+BDRVQcow2State *s = bs->opaque;
 int i;
 QCow2ClusterType first_cluster_type;
 uint64_t mask = stop_flags | L2E_OFFSET_MASK | QCOW_OFLAG_COMPRESSED;
-uint64_t first_entry = be64_to_cpu(l2_slice[0]);
+uint64_t first_entry = get_l2_entry(s, l2_slice, l2_index);
 uint64_t offset = first_entry & mask;
 
 first_cluster_type = qcow2_get_cluster_type(bs, first_entry);
@@ -401,7 +402,7 @@ static int count_contiguous_clusters(BlockDriverState *bs, 
int nb_clusters,
first_cluster_type == QCOW2_CLUSTER_ZERO_ALLOC);
 
 for (i = 0; i < nb_clusters; i++) {
-uint64_t l2_entry = be64_to_cpu(l2_slice[i]) & mask;
+uint64_t l2_entry = get_l2_entry(s, l2_slice, l2_index + i) & mask;
 if (offset + (uint64_t) i * cluster_size != l2_entry) {
 break;
 }
@@ -417,14 +418,16 @@ static int count_contiguous_clusters(BlockDriverState 
*bs, int nb_clusters,
 static int count_contiguous_clusters_unallocated(BlockDriverState *bs,
  int nb_clusters,
  uint64_t *l2_slice,
+ int l2_index,
  QCow2ClusterType wanted_type)
 {
+BDRVQcow2State *s = bs->opaque;
 int i;
 
 assert(wanted_type == QCOW2_CLUSTER_ZERO_PLAIN ||
wanted_type == QCOW2_CLUSTER_UNALLOCATED);
 for (i = 0; i < nb_clusters; i++) {
-uint64_t entry = be64_to_cpu(l2_slice[i]);
+uint64_t entry = get_l2_entry(s, l2_slice, l2_index + i);
 QCow2ClusterType type = qcow2_get_cluster_type(bs, entry);
 
 if (type != wanted_type) {
@@ -575,7 +578,7 @@ int qcow2_get_host_offset(BlockDriverState *bs, uint64_t 
offset,
 /* find the cluster offset for the given disk offset */
 
 l2_index = offset_to_l2_slice_index(s, offset);
-l2_entry = be64_to_cpu(l2_slice[l2_index]);
+l2_entry = get_l2_entry(s, l2_slice, l2_index);
 
 nb_clusters = size_to_clusters(s, bytes_needed);
 /* bytes_needed <= *bytes + offset_in_cluster, both of which are unsigned
@@ -610,7 +613,7 @@ int qcow2_get_host_offset(BlockDriverState *bs, uint64_t 
offset,
 case QCOW2_CLUSTER_UNALLOCATED:
 /* how many empty clusters ? */
 c = count_contiguous_clusters_unallocated(bs, nb_clusters,
-  &l2_slice[l2_index], type);
+  l2_slice, l2_index, type);
 break;
 case QCOW2_CLUSTER_ZERO_ALLOC:
 case QCOW2_CLUSTER_NORMAL: {
@@ -618,7 +621,7 @@ int qcow2_get_host_offset(BlockDriverState *bs, uint64_t 
offset,
 *host_offset = host_cluster_offset + offset_in_cluster;
 /* how many allocated clusters ? */
 c = count_contiguous_clusters(bs, nb_clusters, s->cluster_size,
-  &l2_slice[l2_index], QCOW_OFLAG_ZERO);
+  l2_slice, l2_index, QCOW_OFLAG_ZERO);
 if (offset_into_cluster(

[PULL 10/34] qcow2: Add offset_to_sc_index()

[Max Reitz]

From: Alberto Garcia 

For a given offset, return the subcluster number within its cluster
(i.e. with 32 subclusters per cluster it returns a number between 0
and 31).

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Message-Id: 
<56e3e4ac0d827c6a2f5f259106c5ddb7c4ca2653.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/block/qcow2.h b/block/qcow2.h
index eee4c8de9c..2503374677 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -581,6 +581,11 @@ static inline int offset_to_l2_slice_index(BDRVQcow2State 
*s, int64_t offset)
 return (offset >> s->cluster_bits) & (s->l2_slice_size - 1);
 }
 
+static inline int offset_to_sc_index(BDRVQcow2State *s, int64_t offset)
+{
+return (offset >> s->subcluster_bits) & (s->subclusters_per_cluster - 1);
+}
+
 static inline int64_t qcow2_vm_state_offset(BDRVQcow2State *s)
 {
 return (int64_t)s->l1_vm_state_index << (s->cluster_bits + s->l2_bits);
-- 
2.26.2

[PULL 20/34] qcow2: Add subcluster support to calculate_l2_meta()

[Max Reitz]

From: Alberto Garcia 

If an image has subclusters then there are more copy-on-write
scenarios that we need to consider. Let's say we have a write request
from the middle of subcluster #3 until the end of the cluster:

1) If we are writing to a newly allocated cluster then we need
   copy-on-write. The previous contents of subclusters #0 to #3 must
   be copied to the new cluster. We can optimize this process by
   skipping all leading unallocated or zero subclusters (the status of
   those skipped subclusters will be reflected in the new L2 bitmap).

2) If we are overwriting an existing cluster:

   2.1) If subcluster #3 is unallocated or has the all-zeroes bit set
then we need copy-on-write (on subcluster #3 only).

   2.2) If subcluster #3 was already allocated then there is no need
for any copy-on-write. However we still need to update the L2
bitmap to reflect possible changes in the allocation status of
subclusters #4 to #31. Because of this, this function checks
if all the overwritten subclusters are already allocated and
in this case it returns without creating a new QCowL2Meta
structure.

After all these changes l2meta_cow_start() and l2meta_cow_end()
are not necessarily cluster-aligned anymore. We need to update the
calculation of old_start and old_end in handle_dependencies() to
guarantee that no two requests try to write on the same cluster.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Message-Id: 
<4292dd56e4446d386a2fe307311737a711c00708.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2-cluster.c | 167 +-
 1 file changed, 133 insertions(+), 34 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 5937937596..5e7ae0843d 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -387,7 +387,6 @@ fail:
  * If the L2 entry is invalid return -errno and set @type to
  * QCOW2_SUBCLUSTER_INVALID.
  */
-G_GNUC_UNUSED
 static int qcow2_get_subcluster_range_type(BlockDriverState *bs,
uint64_t l2_entry,
uint64_t l2_bitmap,
@@ -,56 +1110,148 @@ void qcow2_alloc_cluster_abort(BlockDriverState *bs, 
QCowL2Meta *m)
  * If @keep_old is true it means that the clusters were already
  * allocated and will be overwritten. If false then the clusters are
  * new and we have to decrease the reference count of the old ones.
+ *
+ * Returns 0 on success, -errno on failure.
  */
-static void calculate_l2_meta(BlockDriverState *bs,
-  uint64_t host_cluster_offset,
-  uint64_t guest_offset, unsigned bytes,
-  uint64_t *l2_slice, QCowL2Meta **m, bool 
keep_old)
+static int calculate_l2_meta(BlockDriverState *bs, uint64_t 
host_cluster_offset,
+ uint64_t guest_offset, unsigned bytes,
+ uint64_t *l2_slice, QCowL2Meta **m, bool keep_old)
 {
 BDRVQcow2State *s = bs->opaque;
-int l2_index = offset_to_l2_slice_index(s, guest_offset);
-uint64_t l2_entry;
+int sc_index, l2_index = offset_to_l2_slice_index(s, guest_offset);
+uint64_t l2_entry, l2_bitmap;
 unsigned cow_start_from, cow_end_to;
 unsigned cow_start_to = offset_into_cluster(s, guest_offset);
 unsigned cow_end_from = cow_start_to + bytes;
 unsigned nb_clusters = size_to_clusters(s, cow_end_from);
 QCowL2Meta *old_m = *m;
-QCow2ClusterType type;
+QCow2SubclusterType type;
+int i;
+bool skip_cow = keep_old;
 
 assert(nb_clusters <= s->l2_slice_size - l2_index);
 
-/* Return if there's no COW (all clusters are normal and we keep them) */
-if (keep_old) {
-int i;
-for (i = 0; i < nb_clusters; i++) {
-l2_entry = get_l2_entry(s, l2_slice, l2_index + i);
-if (qcow2_get_cluster_type(bs, l2_entry) != QCOW2_CLUSTER_NORMAL) {
-break;
+/* Check the type of all affected subclusters */
+for (i = 0; i < nb_clusters; i++) {
+l2_entry = get_l2_entry(s, l2_slice, l2_index + i);
+l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index + i);
+if (skip_cow) {
+unsigned write_from = MAX(cow_start_to, i << s->cluster_bits);
+unsigned write_to = MIN(cow_end_from, (i + 1) << s->cluster_bits);
+int first_sc = offset_to_sc_index(s, write_from);
+int last_sc = offset_to_sc_index(s, write_to - 1);
+int cnt = qcow2_get_subcluster_range_type(bs, l2_entry, l2_bitmap,
+  first_sc, &type);
+/* Is any of the subclusters of type != QCOW2_SUBCLUSTER_NORMAL ? 
*/
+if (type != QCOW2_SUBCLUSTER_NORMAL || first_sc + cnt <= last_sc) {
+skip_cow = false;
 }
+} else {
+/* I

[PULL 17/34] qcow2: Add cluster type parameter to qcow2_get_host_offset()

[Max Reitz]

From: Alberto Garcia 

This function returns an integer that can be either an error code or a
cluster type (a value from the QCow2ClusterType enum).

We are going to start using subcluster types instead of cluster types
in some functions so it's better to use the exact data types instead
of integers for clarity and in order to detect errors more easily.

This patch makes qcow2_get_host_offset() return 0 on success and
puts the returned cluster type in a separate parameter. There are no
semantic changes.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Message-Id: 
<396b6eab1859a271551dcd7dcba77f8934aa3c3f.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2.h |  3 ++-
 block/qcow2-cluster.c | 11 +++
 block/qcow2.c | 37 ++---
 3 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index ea647c8bb5..74f65793bd 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -893,7 +893,8 @@ int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t 
sector_num,
   uint8_t *buf, int nb_sectors, bool enc, Error 
**errp);
 
 int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
-  unsigned int *bytes, uint64_t *host_offset);
+  unsigned int *bytes, uint64_t *host_offset,
+  QCow2ClusterType *cluster_type);
 int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
unsigned int *bytes, uint64_t *host_offset,
QCowL2Meta **m);
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 2fe7a0f79c..ec0fe0e13b 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -565,13 +565,14 @@ static int coroutine_fn 
do_perform_cow_write(BlockDriverState *bs,
  *
  * On exit, *bytes is the number of bytes starting at offset that have the same
  * cluster type and (if applicable) are stored contiguously in the image file.
+ * The cluster type is stored in *cluster_type.
  * Compressed clusters are always returned one by one.
  *
- * Returns the cluster type (QCOW2_CLUSTER_*) on success, -errno in error
- * cases.
+ * Returns 0 on success, -errno in error cases.
  */
 int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
-  unsigned int *bytes, uint64_t *host_offset)
+  unsigned int *bytes, uint64_t *host_offset,
+  QCow2ClusterType *cluster_type)
 {
 BDRVQcow2State *s = bs->opaque;
 unsigned int l2_index;
@@ -713,7 +714,9 @@ out:
 assert(bytes_available - offset_in_cluster <= UINT_MAX);
 *bytes = bytes_available - offset_in_cluster;
 
-return type;
+*cluster_type = type;
+
+return 0;
 
 fail:
 qcow2_cache_put(s->l2_table_cache, (void **)&l2_slice);
diff --git a/block/qcow2.c b/block/qcow2.c
index edbf9fbd0a..070f89c700 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2043,6 +2043,7 @@ static int coroutine_fn 
qcow2_co_block_status(BlockDriverState *bs,
 BDRVQcow2State *s = bs->opaque;
 uint64_t host_offset;
 unsigned int bytes;
+QCow2ClusterType type;
 int ret, status = 0;
 
 qemu_co_mutex_lock(&s->lock);
@@ -2054,7 +2055,7 @@ static int coroutine_fn 
qcow2_co_block_status(BlockDriverState *bs,
 }
 
 bytes = MIN(INT_MAX, count);
-ret = qcow2_get_host_offset(bs, offset, &bytes, &host_offset);
+ret = qcow2_get_host_offset(bs, offset, &bytes, &host_offset, &type);
 qemu_co_mutex_unlock(&s->lock);
 if (ret < 0) {
 return ret;
@@ -2062,15 +2063,15 @@ static int coroutine_fn 
qcow2_co_block_status(BlockDriverState *bs,
 
 *pnum = bytes;
 
-if ((ret == QCOW2_CLUSTER_NORMAL || ret == QCOW2_CLUSTER_ZERO_ALLOC) &&
+if ((type == QCOW2_CLUSTER_NORMAL || type == QCOW2_CLUSTER_ZERO_ALLOC) &&
 !s->crypto) {
 *map = host_offset;
 *file = s->data_file->bs;
 status |= BDRV_BLOCK_OFFSET_VALID;
 }
-if (ret == QCOW2_CLUSTER_ZERO_PLAIN || ret == QCOW2_CLUSTER_ZERO_ALLOC) {
+if (type == QCOW2_CLUSTER_ZERO_PLAIN || type == QCOW2_CLUSTER_ZERO_ALLOC) {
 status |= BDRV_BLOCK_ZERO;
-} else if (ret != QCOW2_CLUSTER_UNALLOCATED) {
+} else if (type != QCOW2_CLUSTER_UNALLOCATED) {
 status |= BDRV_BLOCK_DATA;
 }
 if (s->metadata_preallocation && (status & BDRV_BLOCK_DATA) &&
@@ -2279,6 +2280,7 @@ static coroutine_fn int 
qcow2_co_preadv_part(BlockDriverState *bs,
 int ret = 0;
 unsigned int cur_bytes; /* number of bytes in current iteration */
 uint64_t host_offset = 0;
+QCow2ClusterType type;
 AioTaskPool *aio = NULL;
 
 while (bytes != 0 && aio_task_pool_status(aio) == 0) {
@@ -2290,22 +2292,23 @@ static coroutine_fn int 
qcow2_co_preadv_part(BlockDriverState *bs,
 }
 
 qemu_co_mutex_lock(&s->lock);
-ret = qcow2_g

[PULL 30/34] qcow2: Add prealloc field to QCowL2Meta

[Max Reitz]

From: Alberto Garcia 

This field allows us to indicate that the L2 metadata update does not
come from a write request with actual data but from a preallocation
request.

For traditional images this does not make any difference, but for
images with extended L2 entries this means that the clusters are
allocated normally in the L2 table but individual subclusters are
marked as unallocated.

This will allow preallocating images that have a backing file.

There is one special case: when we resize an existing image we can
also request that the new clusters are preallocated. If the image
already had a backing file then we have to hide any possible stale
data and zero out the new clusters (see commit 955c7d6687 for more
details).

In this case the subclusters cannot be left as unallocated so the L2
bitmap must be updated.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Message-Id: 
<960d4c444a4f5a870e2b47e5da322a73cd9a2f5a.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2.h | 8 
 block/qcow2-cluster.c | 2 +-
 block/qcow2.c | 6 ++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 4ef4ae4ab0..f3499e53bf 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -463,6 +463,14 @@ typedef struct QCowL2Meta
  */
 bool skip_cow;
 
+/**
+ * Indicates that this is not a normal write request but a preallocation.
+ * If the image has extended L2 entries this means that no new individual
+ * subclusters will be marked as allocated in the L2 bitmap (but any
+ * existing contents of that bitmap will be kept).
+ */
+bool prealloc;
+
 /**
  * The I/O vector with the data from the actual guest write request.
  * If non-NULL, this is meant to be merged together with the data
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 9d349d61c6..fd506b4cb3 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1067,7 +1067,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, 
QCowL2Meta *m)
 set_l2_entry(s, l2_slice, l2_index + i, offset | QCOW_OFLAG_COPIED);
 
 /* Update bitmap with the subclusters that were just written */
-if (has_subclusters(s)) {
+if (has_subclusters(s) && !m->prealloc) {
 uint64_t l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index + i);
 unsigned written_from = m->cow_start.offset;
 unsigned written_to = m->cow_end.offset + m->cow_end.nb_bytes ?:
diff --git a/block/qcow2.c b/block/qcow2.c
index 54c9b7c119..7c03d41170 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2096,6 +2096,7 @@ static coroutine_fn int 
qcow2_handle_l2meta(BlockDriverState *bs,
 QCowL2Meta *next;
 
 if (link_l2) {
+assert(!l2meta->prealloc);
 ret = qcow2_alloc_cluster_link_l2(bs, l2meta);
 if (ret) {
 goto out;
@@ -3130,6 +3131,7 @@ static int coroutine_fn preallocate_co(BlockDriverState 
*bs, uint64_t offset,
 
 while (meta) {
 QCowL2Meta *next = meta->next;
+meta->prealloc = true;
 
 ret = qcow2_alloc_cluster_link_l2(bs, meta);
 if (ret < 0) {
@@ -4217,6 +4219,7 @@ static int coroutine_fn 
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
 int64_t clusters_allocated;
 int64_t old_file_size, last_cluster, new_file_size;
 uint64_t nb_new_data_clusters, nb_new_l2_tables;
+bool subclusters_need_allocation = false;
 
 /* With a data file, preallocation means just allocating the metadata
  * and forwarding the truncate request to the data file */
@@ -4298,6 +4301,8 @@ static int coroutine_fn 
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
BDRV_REQ_ZERO_WRITE, NULL);
 if (ret >= 0) {
 flags &= ~BDRV_REQ_ZERO_WRITE;
+/* Ensure that we read zeroes and not backing file data */
+subclusters_need_allocation = true;
 }
 } else {
 ret = -1;
@@ -4336,6 +4341,7 @@ static int coroutine_fn 
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
 .offset   = nb_clusters << s->cluster_bits,
 .nb_bytes = 0,
 },
+.prealloc = !subclusters_need_allocation,
 };
 qemu_co_queue_init(&allocation.dependent_requests);
 
-- 
2.26.2

[PULL 18/34] qcow2: Replace QCOW2_CLUSTER_* with QCOW2_SUBCLUSTER_*

[Max Reitz]

From: Alberto Garcia 

In order to support extended L2 entries some functions of the qcow2
driver need to start dealing with subclusters instead of clusters.

qcow2_get_host_offset() is modified to return the subcluster type
instead of the cluster type, and all callers are updated to replace
all values of QCow2ClusterType with their QCow2SubclusterType
equivalents.

This patch only changes the data types, there are no semantic changes.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Message-Id: 

Signed-off-by: Max Reitz 
---
 block/qcow2.h |  2 +-
 block/qcow2-cluster.c | 10 +++
 block/qcow2.c | 70 ++-
 3 files changed, 42 insertions(+), 40 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 74f65793bd..5df761edc3 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -894,7 +894,7 @@ int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t 
sector_num,
 
 int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
   unsigned int *bytes, uint64_t *host_offset,
-  QCow2ClusterType *cluster_type);
+  QCow2SubclusterType *subcluster_type);
 int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
unsigned int *bytes, uint64_t *host_offset,
QCowL2Meta **m);
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index ec0fe0e13b..5937937596 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -564,15 +564,15 @@ static int coroutine_fn 
do_perform_cow_write(BlockDriverState *bs,
  * offset that we are interested in.
  *
  * On exit, *bytes is the number of bytes starting at offset that have the same
- * cluster type and (if applicable) are stored contiguously in the image file.
- * The cluster type is stored in *cluster_type.
- * Compressed clusters are always returned one by one.
+ * subcluster type and (if applicable) are stored contiguously in the image
+ * file. The subcluster type is stored in *subcluster_type.
+ * Compressed clusters are always processed one by one.
  *
  * Returns 0 on success, -errno in error cases.
  */
 int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
   unsigned int *bytes, uint64_t *host_offset,
-  QCow2ClusterType *cluster_type)
+  QCow2SubclusterType *subcluster_type)
 {
 BDRVQcow2State *s = bs->opaque;
 unsigned int l2_index;
@@ -714,7 +714,7 @@ out:
 assert(bytes_available - offset_in_cluster <= UINT_MAX);
 *bytes = bytes_available - offset_in_cluster;
 
-*cluster_type = type;
+*subcluster_type = qcow2_cluster_to_subcluster_type(type);
 
 return 0;
 
diff --git a/block/qcow2.c b/block/qcow2.c
index 070f89c700..477d60dd71 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2043,7 +2043,7 @@ static int coroutine_fn 
qcow2_co_block_status(BlockDriverState *bs,
 BDRVQcow2State *s = bs->opaque;
 uint64_t host_offset;
 unsigned int bytes;
-QCow2ClusterType type;
+QCow2SubclusterType type;
 int ret, status = 0;
 
 qemu_co_mutex_lock(&s->lock);
@@ -2063,15 +2063,16 @@ static int coroutine_fn 
qcow2_co_block_status(BlockDriverState *bs,
 
 *pnum = bytes;
 
-if ((type == QCOW2_CLUSTER_NORMAL || type == QCOW2_CLUSTER_ZERO_ALLOC) &&
-!s->crypto) {
+if ((type == QCOW2_SUBCLUSTER_NORMAL ||
+ type == QCOW2_SUBCLUSTER_ZERO_ALLOC) && !s->crypto) {
 *map = host_offset;
 *file = s->data_file->bs;
 status |= BDRV_BLOCK_OFFSET_VALID;
 }
-if (type == QCOW2_CLUSTER_ZERO_PLAIN || type == QCOW2_CLUSTER_ZERO_ALLOC) {
+if (type == QCOW2_SUBCLUSTER_ZERO_PLAIN ||
+type == QCOW2_SUBCLUSTER_ZERO_ALLOC) {
 status |= BDRV_BLOCK_ZERO;
-} else if (type != QCOW2_CLUSTER_UNALLOCATED) {
+} else if (type != QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN) {
 status |= BDRV_BLOCK_DATA;
 }
 if (s->metadata_preallocation && (status & BDRV_BLOCK_DATA) &&
@@ -2168,7 +2169,7 @@ typedef struct Qcow2AioTask {
 AioTask task;
 
 BlockDriverState *bs;
-QCow2ClusterType cluster_type; /* only for read */
+QCow2SubclusterType subcluster_type; /* only for read */
 uint64_t host_offset; /* or full descriptor in compressed clusters */
 uint64_t offset;
 uint64_t bytes;
@@ -2181,7 +2182,7 @@ static coroutine_fn int 
qcow2_co_preadv_task_entry(AioTask *task);
 static coroutine_fn int qcow2_add_task(BlockDriverState *bs,
AioTaskPool *pool,
AioTaskFunc func,
-   QCow2ClusterType cluster_type,
+   QCow2SubclusterType subcluster_type,
uint64_t host_offset,
uint64_t offset,
  

[PULL 09/34] qcow2: Add subcluster-related fields to BDRVQcow2State

[Max Reitz]

From: Alberto Garcia 

This patch adds the following new fields to BDRVQcow2State:

- subclusters_per_cluster: Number of subclusters in a cluster
- subcluster_size: The size of each subcluster, in bytes
- subcluster_bits: No. of bits so 1 << subcluster_bits = subcluster_size

Images without subclusters are treated as if they had exactly one
subcluster per cluster (i.e. subcluster_size = cluster_size).

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Message-Id: 
<55bfeac86b092fa2c9d182a95cbeb479ff7eca4f.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2.h | 5 +
 block/qcow2.c | 5 +
 2 files changed, 10 insertions(+)

diff --git a/block/qcow2.h b/block/qcow2.h
index 2064dd3d85..eee4c8de9c 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -78,6 +78,8 @@
 /* The cluster reads as all zeros */
 #define QCOW_OFLAG_ZERO (1ULL << 0)
 
+#define QCOW_EXTL2_SUBCLUSTERS_PER_CLUSTER 32
+
 #define MIN_CLUSTER_BITS 9
 #define MAX_CLUSTER_BITS 21
 
@@ -295,6 +297,9 @@ typedef struct BDRVQcow2State {
 int cluster_bits;
 int cluster_size;
 int l2_slice_size;
+int subcluster_bits;
+int subcluster_size;
+int subclusters_per_cluster;
 int l2_bits;
 int l2_size;
 int l1_size;
diff --git a/block/qcow2.c b/block/qcow2.c
index 6738daa247..fb4584d3ee 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1444,6 +1444,11 @@ static int coroutine_fn qcow2_do_open(BlockDriverState 
*bs, QDict *options,
 }
 }
 
+s->subclusters_per_cluster =
+has_subclusters(s) ? QCOW_EXTL2_SUBCLUSTERS_PER_CLUSTER : 1;
+s->subcluster_size = s->cluster_size / s->subclusters_per_cluster;
+s->subcluster_bits = ctz32(s->subcluster_size);
+
 /* Check support for various header values */
 if (header.refcount_order > 6) {
 error_setg(errp, "Reference count entry width too large; may not "
-- 
2.26.2

[PULL 24/34] qcow2: Add subcluster support to check_refcounts_l2()

[Max Reitz]

From: Alberto Garcia 

The offset field of an uncompressed cluster's L2 entry must be aligned
to the cluster size, otherwise it is invalid. If the cluster has no
data then it means that the offset points to a preallocation, so we
can clear the offset field without affecting the guest-visible data.
This is what 'qemu-img check' does when run in repair mode.

On traditional qcow2 images this can only happen when QCOW_OFLAG_ZERO
is set, and repairing such entries turns the clusters from ZERO_ALLOC
into ZERO_PLAIN.

Extended L2 entries have no ZERO_ALLOC clusters and no QCOW_OFLAG_ZERO
but the idea is the same: if none of the subclusters are allocated
then we can clear the offset field and leave the bitmap untouched.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Message-Id: 
<9f4ed1d0a34b0a545b032c31ecd8c14734065342.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2-refcount.c | 16 +++-
 tests/qemu-iotests/060.out |  2 +-
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 770c5dbc83..aae52607eb 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1669,12 +1669,18 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 
 /* Correct offsets are cluster aligned */
 if (offset_into_cluster(s, offset)) {
+bool contains_data;
 res->corruptions++;
 
-if (qcow2_get_cluster_type(bs, l2_entry) ==
-QCOW2_CLUSTER_ZERO_ALLOC)
-{
-fprintf(stderr, "%s offset=%" PRIx64 ": Preallocated zero "
+if (has_subclusters(s)) {
+uint64_t l2_bitmap = get_l2_bitmap(s, l2_table, i);
+contains_data = (l2_bitmap & QCOW_L2_BITMAP_ALL_ALLOC);
+} else {
+contains_data = !(l2_entry & QCOW_OFLAG_ZERO);
+}
+
+if (!contains_data) {
+fprintf(stderr, "%s offset=%" PRIx64 ": Preallocated "
 "cluster is not properly aligned; L2 entry "
 "corrupted.\n",
 fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR",
@@ -1686,7 +1692,7 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 int ign = active ? QCOW2_OL_ACTIVE_L2 :
QCOW2_OL_INACTIVE_L2;
 
-l2_entry = QCOW_OFLAG_ZERO;
+l2_entry = has_subclusters(s) ? 0 : QCOW_OFLAG_ZERO;
 set_l2_entry(s, l2_table, i, l2_entry);
 ret = qcow2_pre_write_overlap_check(bs, ign,
 l2e_offset, l2_entry_size(s), false);
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index e574c38797..b2804a0d07 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -320,7 +320,7 @@ discard 65536/65536 bytes at offset 0
 qcow2: Marking image as corrupt: Preallocated zero cluster offset 0x2a00 
unaligned (guest offset: 0); further corruption events will be suppressed
 write failed: Input/output error
 --- Repairing ---
-Repairing offset=2a00: Preallocated zero cluster is not properly aligned; L2 
entry corrupted.
+Repairing offset=2a00: Preallocated cluster is not properly aligned; L2 entry 
corrupted.
 The following inconsistencies were found and repaired:
 
 0 leaked clusters
-- 
2.26.2

[PULL 11/34] qcow2: Add offset_into_subcluster() and size_to_subclusters()

[Max Reitz]

From: Alberto Garcia 

Like offset_into_cluster() and size_to_clusters(), but for
subclusters.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Message-Id: 
<3cc2390dcdef3d234d47c741b708bd8734490862.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/block/qcow2.h b/block/qcow2.h
index 2503374677..4fe31adfd3 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -555,11 +555,21 @@ static inline int64_t offset_into_cluster(BDRVQcow2State 
*s, int64_t offset)
 return offset & (s->cluster_size - 1);
 }
 
+static inline int64_t offset_into_subcluster(BDRVQcow2State *s, int64_t offset)
+{
+return offset & (s->subcluster_size - 1);
+}
+
 static inline uint64_t size_to_clusters(BDRVQcow2State *s, uint64_t size)
 {
 return (size + (s->cluster_size - 1)) >> s->cluster_bits;
 }
 
+static inline uint64_t size_to_subclusters(BDRVQcow2State *s, uint64_t size)
+{
+return (size + (s->subcluster_size - 1)) >> s->subcluster_bits;
+}
+
 static inline int64_t size_to_l1(BDRVQcow2State *s, int64_t size)
 {
 int shift = s->cluster_bits + s->l2_bits;
-- 
2.26.2

[PULL 29/34] qcow2: Add subcluster support to qcow2_measure()

[Max Reitz]

From: Alberto Garcia 

Extended L2 entries are bigger than normal L2 entries so this has an
impact on the amount of metadata needed for a qcow2 file.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Message-Id: 
<7efae2efd5e36b42d2570743a12576d68ce53685.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2.c | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 0cf0b0a9fb..54c9b7c119 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3232,28 +3232,31 @@ int64_t qcow2_refcount_metadata_size(int64_t clusters, 
size_t cluster_size,
  * @total_size: virtual disk size in bytes
  * @cluster_size: cluster size in bytes
  * @refcount_order: refcount bits power-of-2 exponent
+ * @extended_l2: true if the image has extended L2 entries
  *
  * Returns: Total number of bytes required for the fully allocated image
  * (including metadata).
  */
 static int64_t qcow2_calc_prealloc_size(int64_t total_size,
 size_t cluster_size,
-int refcount_order)
+int refcount_order,
+bool extended_l2)
 {
 int64_t meta_size = 0;
 uint64_t nl1e, nl2e;
 int64_t aligned_total_size = ROUND_UP(total_size, cluster_size);
+size_t l2e_size = extended_l2 ? L2E_SIZE_EXTENDED : L2E_SIZE_NORMAL;
 
 /* header: 1 cluster */
 meta_size += cluster_size;
 
 /* total size of L2 tables */
 nl2e = aligned_total_size / cluster_size;
-nl2e = ROUND_UP(nl2e, cluster_size / sizeof(uint64_t));
-meta_size += nl2e * sizeof(uint64_t);
+nl2e = ROUND_UP(nl2e, cluster_size / l2e_size);
+meta_size += nl2e * l2e_size;
 
 /* total size of L1 tables */
-nl1e = nl2e * sizeof(uint64_t) / cluster_size;
+nl1e = nl2e * l2e_size / cluster_size;
 nl1e = ROUND_UP(nl1e, cluster_size / sizeof(uint64_t));
 meta_size += nl1e * sizeof(uint64_t);
 
@@ -4838,6 +4841,8 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, 
BlockDriverState *in_bs,
 PreallocMode prealloc;
 bool has_backing_file;
 bool has_luks;
+bool extended_l2 = false; /* Set to false until the option is added */
+size_t l2e_size;
 
 /* Parse image creation options */
 cluster_size = qcow2_opt_get_cluster_size_del(opts, &local_err);
@@ -4896,8 +4901,9 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, 
BlockDriverState *in_bs,
 virtual_size = ROUND_UP(virtual_size, cluster_size);
 
 /* Check that virtual disk size is valid */
+l2e_size = extended_l2 ? L2E_SIZE_EXTENDED : L2E_SIZE_NORMAL;
 l2_tables = DIV_ROUND_UP(virtual_size / cluster_size,
- cluster_size / sizeof(uint64_t));
+ cluster_size / l2e_size);
 if (l2_tables * sizeof(uint64_t) > QCOW_MAX_L1_SIZE) {
 error_setg(&local_err, "The image size is too large "
"(try using a larger cluster size)");
@@ -4960,9 +4966,9 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, 
BlockDriverState *in_bs,
 }
 
 info = g_new0(BlockMeasureInfo, 1);
-info->fully_allocated =
+info->fully_allocated = luks_payload_size +
 qcow2_calc_prealloc_size(virtual_size, cluster_size,
- ctz32(refcount_bits)) + luks_payload_size;
+ ctz32(refcount_bits), extended_l2);
 
 /*
  * Remove data clusters that are not required.  This overestimates the
-- 
2.26.2

[PULL 22/34] qcow2: Add subcluster support to zero_in_l2_slice()

[Max Reitz]

From: Alberto Garcia 

The QCOW_OFLAG_ZERO bit that indicates that a cluster reads as
zeroes is only used in standard L2 entries. Extended L2 entries use
individual 'all zeroes' bits for each subcluster.

This must be taken into account when updating the L2 entry and also
when deciding that an existing entry does not need to be updated.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Message-Id: 

Signed-off-by: Max Reitz 
---
 block/qcow2-cluster.c | 38 --
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 08ecb4ca0c..5afcd72f5a 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1957,7 +1957,6 @@ static int zero_in_l2_slice(BlockDriverState *bs, 
uint64_t offset,
 int l2_index;
 int ret;
 int i;
-bool unmap = !!(flags & BDRV_REQ_MAY_UNMAP);
 
 ret = get_cluster_table(bs, offset, &l2_slice, &l2_index);
 if (ret < 0) {
@@ -1969,28 +1968,31 @@ static int zero_in_l2_slice(BlockDriverState *bs, 
uint64_t offset,
 assert(nb_clusters <= INT_MAX);
 
 for (i = 0; i < nb_clusters; i++) {
-uint64_t old_offset;
-QCow2ClusterType cluster_type;
-
-old_offset = get_l2_entry(s, l2_slice, l2_index + i);
+uint64_t old_l2_entry = get_l2_entry(s, l2_slice, l2_index + i);
+uint64_t old_l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index + i);
+QCow2ClusterType type = qcow2_get_cluster_type(bs, old_l2_entry);
+bool unmap = (type == QCOW2_CLUSTER_COMPRESSED) ||
+((flags & BDRV_REQ_MAY_UNMAP) && qcow2_cluster_is_allocated(type));
+uint64_t new_l2_entry = unmap ? 0 : old_l2_entry;
+uint64_t new_l2_bitmap = old_l2_bitmap;
+
+if (has_subclusters(s)) {
+new_l2_bitmap = QCOW_L2_BITMAP_ALL_ZEROES;
+} else {
+new_l2_entry |= QCOW_OFLAG_ZERO;
+}
 
-/*
- * Minimize L2 changes if the cluster already reads back as
- * zeroes with correct allocation.
- */
-cluster_type = qcow2_get_cluster_type(bs, old_offset);
-if (cluster_type == QCOW2_CLUSTER_ZERO_PLAIN ||
-(cluster_type == QCOW2_CLUSTER_ZERO_ALLOC && !unmap)) {
+if (old_l2_entry == new_l2_entry && old_l2_bitmap == new_l2_bitmap) {
 continue;
 }
 
 qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
-if (cluster_type == QCOW2_CLUSTER_COMPRESSED || unmap) {
-set_l2_entry(s, l2_slice, l2_index + i, QCOW_OFLAG_ZERO);
-qcow2_free_any_clusters(bs, old_offset, 1, QCOW2_DISCARD_REQUEST);
-} else {
-uint64_t entry = get_l2_entry(s, l2_slice, l2_index + i);
-set_l2_entry(s, l2_slice, l2_index + i, entry | QCOW_OFLAG_ZERO);
+if (unmap) {
+qcow2_free_any_clusters(bs, old_l2_entry, 1, 
QCOW2_DISCARD_REQUEST);
+}
+set_l2_entry(s, l2_slice, l2_index + i, new_l2_entry);
+if (has_subclusters(s)) {
+set_l2_bitmap(s, l2_slice, l2_index + i, new_l2_bitmap);
 }
 }
 
-- 
2.26.2

[PULL 27/34] qcow2: Add subcluster support to handle_alloc_space()

[Max Reitz]

From: Alberto Garcia 

The bdrv_co_pwrite_zeroes() call here fills complete clusters with
zeroes, but it can happen that some subclusters are not part of the
write request or the copy-on-write. This patch makes sure that only
the affected subclusters are overwritten.

A potential improvement would be to also fill with zeroes the other
subclusters if we can guarantee that we are not overwriting existing
data. However this would waste more disk space, so we should first
evaluate if it's really worth doing.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Message-Id: 

Signed-off-by: Max Reitz 
---
 block/qcow2.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 60192f1be3..9990535c46 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2421,6 +2421,9 @@ static int handle_alloc_space(BlockDriverState *bs, 
QCowL2Meta *l2meta)
 
 for (m = l2meta; m != NULL; m = m->next) {
 int ret;
+uint64_t start_offset = m->alloc_offset + m->cow_start.offset;
+unsigned nb_bytes = m->cow_end.offset + m->cow_end.nb_bytes -
+m->cow_start.offset;
 
 if (!m->cow_start.nb_bytes && !m->cow_end.nb_bytes) {
 continue;
@@ -2435,16 +2438,14 @@ static int handle_alloc_space(BlockDriverState *bs, 
QCowL2Meta *l2meta)
  * efficiently zero out the whole clusters
  */
 
-ret = qcow2_pre_write_overlap_check(bs, 0, m->alloc_offset,
-m->nb_clusters * s->cluster_size,
+ret = qcow2_pre_write_overlap_check(bs, 0, start_offset, nb_bytes,
 true);
 if (ret < 0) {
 return ret;
 }
 
 BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_ALLOC_SPACE);
-ret = bdrv_co_pwrite_zeroes(s->data_file, m->alloc_offset,
-m->nb_clusters * s->cluster_size,
+ret = bdrv_co_pwrite_zeroes(s->data_file, start_offset, nb_bytes,
 BDRV_REQ_NO_FALLBACK);
 if (ret < 0) {
 if (ret != -ENOTSUP && ret != -EAGAIN) {
-- 
2.26.2

[PULL 12/34] qcow2: Add l2_entry_size()

[Max Reitz]

From: Alberto Garcia 

qcow2 images with subclusters have 128-bit L2 entries. The first 64
bits contain the same information as traditional images and the last
64 bits form a bitmap with the status of each individual subcluster.

Because of that we cannot assume that L2 entries are sizeof(uint64_t)
anymore. This function returns the proper value for the image.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Message-Id: 

Signed-off-by: Max Reitz 
---
 block/qcow2.h  |  9 +
 block/qcow2-cluster.c  | 12 ++--
 block/qcow2-refcount.c | 14 --
 block/qcow2.c  |  8 
 4 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 4fe31adfd3..46b351229a 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -80,6 +80,10 @@
 
 #define QCOW_EXTL2_SUBCLUSTERS_PER_CLUSTER 32
 
+/* Size of normal and extended L2 entries */
+#define L2E_SIZE_NORMAL   (sizeof(uint64_t))
+#define L2E_SIZE_EXTENDED (sizeof(uint64_t) * 2)
+
 #define MIN_CLUSTER_BITS 9
 #define MAX_CLUSTER_BITS 21
 
@@ -521,6 +525,11 @@ static inline bool has_subclusters(BDRVQcow2State *s)
 return false;
 }
 
+static inline size_t l2_entry_size(BDRVQcow2State *s)
+{
+return has_subclusters(s) ? L2E_SIZE_EXTENDED : L2E_SIZE_NORMAL;
+}
+
 static inline uint64_t get_l2_entry(BDRVQcow2State *s, uint64_t *l2_slice,
 int idx)
 {
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 5bd1e1feb8..0b762502f6 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -208,7 +208,7 @@ static int l2_load(BlockDriverState *bs, uint64_t offset,
uint64_t l2_offset, uint64_t **l2_slice)
 {
 BDRVQcow2State *s = bs->opaque;
-int start_of_slice = sizeof(uint64_t) *
+int start_of_slice = l2_entry_size(s) *
 (offset_to_l2_index(s, offset) - offset_to_l2_slice_index(s, offset));
 
 return qcow2_cache_get(bs, s->l2_table_cache, l2_offset + start_of_slice,
@@ -281,7 +281,7 @@ static int l2_allocate(BlockDriverState *bs, int l1_index)
 
 /* allocate a new l2 entry */
 
-l2_offset = qcow2_alloc_clusters(bs, s->l2_size * sizeof(uint64_t));
+l2_offset = qcow2_alloc_clusters(bs, s->l2_size * l2_entry_size(s));
 if (l2_offset < 0) {
 ret = l2_offset;
 goto fail;
@@ -305,7 +305,7 @@ static int l2_allocate(BlockDriverState *bs, int l1_index)
 
 /* allocate a new entry in the l2 cache */
 
-slice_size2 = s->l2_slice_size * sizeof(uint64_t);
+slice_size2 = s->l2_slice_size * l2_entry_size(s);
 n_slices = s->cluster_size / slice_size2;
 
 trace_qcow2_l2_allocate_get_empty(bs, l1_index);
@@ -369,7 +369,7 @@ fail:
 }
 s->l1_table[l1_index] = old_l2_offset;
 if (l2_offset > 0) {
-qcow2_free_clusters(bs, l2_offset, s->l2_size * sizeof(uint64_t),
+qcow2_free_clusters(bs, l2_offset, s->l2_size * l2_entry_size(s),
 QCOW2_DISCARD_ALWAYS);
 }
 return ret;
@@ -717,7 +717,7 @@ static int get_cluster_table(BlockDriverState *bs, uint64_t 
offset,
 
 /* Then decrease the refcount of the old table */
 if (l2_offset) {
-qcow2_free_clusters(bs, l2_offset, s->l2_size * sizeof(uint64_t),
+qcow2_free_clusters(bs, l2_offset, s->l2_size * l2_entry_size(s),
 QCOW2_DISCARD_OTHER);
 }
 
@@ -1921,7 +1921,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState 
*bs, uint64_t *l1_table,
 int ret;
 int i, j;
 
-slice_size2 = s->l2_slice_size * sizeof(uint64_t);
+slice_size2 = s->l2_slice_size * l2_entry_size(s);
 n_slices = s->cluster_size / slice_size2;
 
 if (!is_active_l1) {
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 04546838e8..770c5dbc83 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1254,7 +1254,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
 l2_slice = NULL;
 l1_table = NULL;
 l1_size2 = l1_size * sizeof(uint64_t);
-slice_size2 = s->l2_slice_size * sizeof(uint64_t);
+slice_size2 = s->l2_slice_size * l2_entry_size(s);
 n_slices = s->cluster_size / slice_size2;
 
 s->cache_discards = true;
@@ -1605,7 +1605,7 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 int i, l2_size, nb_csectors, ret;
 
 /* Read L2 table from disk */
-l2_size = s->l2_size * sizeof(uint64_t);
+l2_size = s->l2_size * l2_entry_size(s);
 l2_table = g_malloc(l2_size);
 
 ret = bdrv_pread(bs->file, l2_offset, l2_table, l2_size);
@@ -1680,15 +1680,16 @@ static int check_refcounts_l2(BlockDriverState *bs, 
BdrvCheckResult *res,
 fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR",
 offset);
 if (fix & BDRV_FIX_ERRORS) {
+int idx = i * (l2_entry_size(s) / sizeof(uint64_t));
 uint64_t 

[PATCH 2/5] meson: fixes relpath may fail on win32. for example C:/msys64/mingw64/x.exe relative to E:/path/qemu-build would fail.

[luoyonggang]

From: Yonggang Luo 

---
 scripts/mtest2make.py | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
index bdb257bbd9..d7a51bf97e 100644
--- a/scripts/mtest2make.py
+++ b/scripts/mtest2make.py
@@ -53,9 +53,16 @@ i = 0
 for test in json.load(sys.stdin):
 env = ' '.join(('%s=%s' % (shlex.quote(k), shlex.quote(v))
 for k, v in test['env'].items()))
-executable = os.path.relpath(test['cmd'][0])
+executable = test['cmd'][0]
+try:
+executable = os.path.relpath(executable)
+except:
+pass
 if test['workdir'] is not None:
-test['cmd'][0] = os.path.relpath(test['cmd'][0], test['workdir'])
+try:
+test['cmd'][0] = os.path.relpath(executable, test['workdir'])
+except:
+test['cmd'][0] = executable
 else:
 test['cmd'][0] = executable
 cmd = '$(.test.env) %s %s' % (env, ' '.join((shlex.quote(x) for x in 
test['cmd'])))
-- 
2.27.0.windows.1

[PULL 21/34] qcow2: Add subcluster support to qcow2_get_host_offset()

[Max Reitz]

From: Alberto Garcia 

The logic of this function remains pretty much the same, except that
it uses count_contiguous_subclusters(), which combines the logic of
count_contiguous_clusters() / count_contiguous_clusters_unallocated()
and checks individual subclusters.

qcow2_cluster_to_subcluster_type() is not necessary as a separate
function anymore so it's inlined into its caller.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Message-Id: 

[mreitz: Initialize expected_type to anything]
Signed-off-by: Max Reitz 
---
 block/qcow2.h |  38 ---
 block/qcow2-cluster.c | 150 ++
 2 files changed, 92 insertions(+), 96 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 5df761edc3..4fad40b96b 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -710,29 +710,6 @@ static inline QCow2ClusterType 
qcow2_get_cluster_type(BlockDriverState *bs,
 }
 }
 
-/*
- * For an image without extended L2 entries, return the
- * QCow2SubclusterType equivalent of a given QCow2ClusterType.
- */
-static inline
-QCow2SubclusterType qcow2_cluster_to_subcluster_type(QCow2ClusterType type)
-{
-switch (type) {
-case QCOW2_CLUSTER_COMPRESSED:
-return QCOW2_SUBCLUSTER_COMPRESSED;
-case QCOW2_CLUSTER_ZERO_PLAIN:
-return QCOW2_SUBCLUSTER_ZERO_PLAIN;
-case QCOW2_CLUSTER_ZERO_ALLOC:
-return QCOW2_SUBCLUSTER_ZERO_ALLOC;
-case QCOW2_CLUSTER_NORMAL:
-return QCOW2_SUBCLUSTER_NORMAL;
-case QCOW2_CLUSTER_UNALLOCATED:
-return QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN;
-default:
-g_assert_not_reached();
-}
-}
-
 /*
  * In an image without subsclusters @l2_bitmap is ignored and
  * @sc_index must be 0.
@@ -776,7 +753,20 @@ QCow2SubclusterType 
qcow2_get_subcluster_type(BlockDriverState *bs,
 g_assert_not_reached();
 }
 } else {
-return qcow2_cluster_to_subcluster_type(type);
+switch (type) {
+case QCOW2_CLUSTER_COMPRESSED:
+return QCOW2_SUBCLUSTER_COMPRESSED;
+case QCOW2_CLUSTER_ZERO_PLAIN:
+return QCOW2_SUBCLUSTER_ZERO_PLAIN;
+case QCOW2_CLUSTER_ZERO_ALLOC:
+return QCOW2_SUBCLUSTER_ZERO_ALLOC;
+case QCOW2_CLUSTER_NORMAL:
+return QCOW2_SUBCLUSTER_NORMAL;
+case QCOW2_CLUSTER_UNALLOCATED:
+return QCOW2_SUBCLUSTER_UNALLOCATED_PLAIN;
+default:
+g_assert_not_reached();
+}
 }
 }
 
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 5e7ae0843d..08ecb4ca0c 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -426,66 +426,66 @@ static int 
qcow2_get_subcluster_range_type(BlockDriverState *bs,
 }
 
 /*
- * Checks how many clusters in a given L2 slice are contiguous in the image
- * file. As soon as one of the flags in the bitmask stop_flags changes compared
- * to the first cluster, the search is stopped and the cluster is not counted
- * as contiguous. (This allows it, for example, to stop at the first compressed
- * cluster which may require a different handling)
+ * Return the number of contiguous subclusters of the exact same type
+ * in a given L2 slice, starting from cluster @l2_index, subcluster
+ * @sc_index. Allocated subclusters are required to be contiguous in
+ * the image file.
+ * At most @nb_clusters are checked (note that this means clusters,
+ * not subclusters).
+ * Compressed clusters are always processed one by one but for the
+ * purpose of this count they are treated as if they were divided into
+ * subclusters of size s->subcluster_size.
+ * On failure return -errno and update @l2_index to point to the
+ * invalid entry.
  */
-static int count_contiguous_clusters(BlockDriverState *bs, int nb_clusters,
-int cluster_size, uint64_t *l2_slice, int l2_index, uint64_t 
stop_flags)
+static int count_contiguous_subclusters(BlockDriverState *bs, int nb_clusters,
+unsigned sc_index, uint64_t *l2_slice,
+unsigned *l2_index)
 {
 BDRVQcow2State *s = bs->opaque;
-int i;
-QCow2ClusterType first_cluster_type;
-uint64_t mask = stop_flags | L2E_OFFSET_MASK | QCOW_OFLAG_COMPRESSED;
-uint64_t first_entry = get_l2_entry(s, l2_slice, l2_index);
-uint64_t offset = first_entry & mask;
+int i, count = 0;
+bool check_offset = false;
+uint64_t expected_offset = 0;
+QCow2SubclusterType expected_type = QCOW2_SUBCLUSTER_NORMAL, type;
 
-first_cluster_type = qcow2_get_cluster_type(bs, first_entry);
-if (first_cluster_type == QCOW2_CLUSTER_UNALLOCATED) {
-return 0;
-}
-
-/* must be allocated */
-assert(first_cluster_type == QCOW2_CLUSTER_NORMAL ||
-   first_cluster_type == QCOW2_CLUSTER_ZERO_ALLOC);
+assert(*l2_index + nb_clusters <= s->l2_slice_size);
 
 for (i = 0; i < nb_clusters; i++) {
-uint64_t l2_entry = get_l2_entry(s, l2_slice, l2_index + i) & mask;
-

[PULL 25/34] qcow2: Update L2 bitmap in qcow2_alloc_cluster_link_l2()

[Max Reitz]

From: Alberto Garcia 

The L2 bitmap needs to be updated after each write to indicate what
new subclusters are now allocated. This needs to happen even if the
cluster was already allocated and the L2 entry was otherwise valid.

In some cases however a write operation doesn't need change the L2
bitmap (because all affected subclusters were already allocated). This
is detected in calculate_l2_meta(), and qcow2_alloc_cluster_link_l2()
is never called in those cases.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Message-Id: 
<0875620d49f44320334b6a91c73b3f301f975f38.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2-cluster.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index a41351aba5..816ddc7639 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1062,6 +1062,24 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, 
QCowL2Meta *m)
 assert((offset & L2E_OFFSET_MASK) == offset);
 
 set_l2_entry(s, l2_slice, l2_index + i, offset | QCOW_OFLAG_COPIED);
+
+/* Update bitmap with the subclusters that were just written */
+if (has_subclusters(s)) {
+uint64_t l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index + i);
+unsigned written_from = m->cow_start.offset;
+unsigned written_to = m->cow_end.offset + m->cow_end.nb_bytes ?:
+m->nb_clusters << s->cluster_bits;
+int first_sc, last_sc;
+/* Narrow written_from and written_to down to the current cluster 
*/
+written_from = MAX(written_from, i << s->cluster_bits);
+written_to   = MIN(written_to, (i + 1) << s->cluster_bits);
+assert(written_from < written_to);
+first_sc = offset_to_sc_index(s, written_from);
+last_sc  = offset_to_sc_index(s, written_to - 1);
+l2_bitmap |= QCOW_OFLAG_SUB_ALLOC_RANGE(first_sc, last_sc + 1);
+l2_bitmap &= ~QCOW_OFLAG_SUB_ZERO_RANGE(first_sc, last_sc + 1);
+set_l2_bitmap(s, l2_slice, l2_index + i, l2_bitmap);
+}
  }
 
 
-- 
2.26.2

[PULL 16/34] qcow2: Add qcow2_cluster_is_allocated()

[Max Reitz]

From: Alberto Garcia 

This helper function tells us if a cluster is allocated (that is,
there is an associated host offset for it).

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Message-Id: 
<6d8771c5c79cbdc6c519875a5078e1cc85856d63.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/block/qcow2.h b/block/qcow2.h
index 3aec6f452a..ea647c8bb5 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -780,6 +780,12 @@ QCow2SubclusterType 
qcow2_get_subcluster_type(BlockDriverState *bs,
 }
 }
 
+static inline bool qcow2_cluster_is_allocated(QCow2ClusterType type)
+{
+return (type == QCOW2_CLUSTER_COMPRESSED || type == QCOW2_CLUSTER_NORMAL ||
+type == QCOW2_CLUSTER_ZERO_ALLOC);
+}
+
 /* Check whether refcounts are eager or lazy */
 static inline bool qcow2_need_accurate_refcounts(BDRVQcow2State *s)
 {
-- 
2.26.2

[PATCH 1/5] meson: SIMPLE_PATH_RE should match the full path token. Or the $ and : contained in path would not matched. if the path are start with C:/ and E:/

[luoyonggang]

From: Yonggang Luo 

---
 scripts/ninjatool.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/ninjatool.py b/scripts/ninjatool.py
index cc77d51aa8..6ca8be6f10 100755
--- a/scripts/ninjatool.py
+++ b/scripts/ninjatool.py
@@ -55,7 +55,7 @@ else:
 
 PATH_RE = r"[^$\s:|]+|\$[$ :]|\$[a-zA-Z0-9_-]+|\$\{[a-zA-Z0-9_.-]+\}"
 
-SIMPLE_PATH_RE = re.compile(r"[^$\s:|]+")
+SIMPLE_PATH_RE = re.compile(r"^[^$\s:|]+$")
 IDENT_RE = re.compile(r"[a-zA-Z0-9_.-]+$")
 STRING_RE = re.compile(r"(" + PATH_RE + r"|[\s:|])(?:\r?\n)?|.")
 TOPLEVEL_RE = re.compile(r"([=:#]|\|\|?|^ +|(?:" + PATH_RE + r")+)\s*|.")
-- 
2.27.0.windows.1

Re: meson: problems building under msys2/mingw-w64 native

[Yonggang Luo]

OK, I resend the patches in consecutive ways.
and for througfully fixes msys2 build, we need upstream meson pull request
https://github.com/mesonbuild/meson/pull/7637

On Tue, Aug 25, 2020 at 4:34 PM Paolo Bonzini  wrote:

> I saw it, thanks. I would like to have someone else (like Mark) test it
> and then I will include it.
>
> Paolo
>
> Il mar 25 ago 2020, 10:24 罗勇刚(Yonggang Luo)  ha
> scritto:
>
>> Hi Paolo Bonzini,
>> I've already sent a series of fixes for msys, do you have a look at that?
>>
>>
>> On Tue, Aug 25, 2020 at 3:55 PM Paolo Bonzini 
>> wrote:
>>
>>> Great, thanks! Can you send it as a patch? I am collecting Meson fixes
>>> and I should be able to send a pull request this week.
>>>
>>> Also if you can please test the msys fixes that were sent on the list
>>> that would be great.
>>>
>>> Paolo
>>>
>>> Il mar 25 ago 2020, 09:52 Mark Cave-Ayland <
>>> mark.cave-ayl...@ilande.co.uk> ha scritto:
>>>
 On 24/08/2020 12:37, Gerd Hoffmann wrote:

 >> 2) GTK UI now depends on CONFIG_VTE
 >>
 >> This one I spotted on my local Linux setup as I didn't have the
 libvte-dev package
 >> installed and couldn't understand why I couldn't run QEMU with the
 GTK UI as I always
 >> do, even though configure reported that it found the GTK library and
 headers.
 >>
 >> A quick search showed that the GTK UI was being guarded by "if
 >> config_host.has_key('CONFIG_GTK') and
 config_host.has_key('CONFIG_VTE')" in
 >> ui/meson.build.
 >
 > That is not correct.  vte is intentionally not a hard dependency ...
 >
 >> For me the easy solution was to install libvte-dev, but since there
 are no VTE
 >> packages for Windows my guess is this will now make the GTK UI
 unavailable for
 >> Windows users.
 >
 > .. because we don't have that on windows.
 >
 > I think simply dropping the "and config_host.has_key('CONFIG_VTE')"
 > should work, can you try that?

 Hi Gerd,

 I can't get the native Windows build to complete yet, however I've
 removed the
 libvte-dev headers again on my Linux setup and confirmed that GTK works
 once again
 with the below diff:

 diff --git a/ui/meson.build b/ui/meson.build
 index 81fd393432..cc71f51f37 100644
 --- a/ui/meson.build
 +++ b/ui/meson.build
 @@ -42,7 +42,7 @@ if config_host.has_key('CONFIG_CURSES')
ui_modules += {'curses' : curses_ss}
  endif

 -if config_host.has_key('CONFIG_GTK') and
 config_host.has_key('CONFIG_VTE')
 +if config_host.has_key('CONFIG_GTK')
softmmu_ss.add(when: 'CONFIG_WIN32', if_true:
 files('win32-kbd-hook.c'))

gtk_ss = ss.source_set()


 ATB,

 Mark.


>>
>> --
>>  此致
>> 礼
>> 罗勇刚
>> Yours
>> sincerely,
>> Yonggang Luo
>>
>

-- 
 此致
礼
罗勇刚
Yours
sincerely,
Yonggang Luo

[PULL 23/34] qcow2: Add subcluster support to discard_in_l2_slice()

[Max Reitz]

From: Alberto Garcia 

Two things need to be taken into account here:

1) With full_discard == true the L2 entry must be cleared completely.
   This also includes the L2 bitmap if the image has extended L2
   entries.

2) With full_discard == false we have to make the discarded cluster
   read back as zeroes. With normal L2 entries this is done with the
   QCOW_OFLAG_ZERO bit, whereas with extended L2 entries this is done
   with the individual 'all zeroes' bits for each subcluster.

   Note however that QCOW_OFLAG_ZERO is not supported in v2 qcow2
   images so, if there is a backing file, discard cannot guarantee
   that the image will read back as zeroes. If this is important for
   the caller it should forbid it as qcow2_co_pdiscard() does (see
   80f5c01183 for more details).

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Message-Id: 
<5ef8274e628aa3ab559bfac467abf488534f2b76.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2-cluster.c | 52 +++
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 5afcd72f5a..a41351aba5 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1848,11 +1848,17 @@ static int discard_in_l2_slice(BlockDriverState *bs, 
uint64_t offset,
 assert(nb_clusters <= INT_MAX);
 
 for (i = 0; i < nb_clusters; i++) {
-uint64_t old_l2_entry;
-
-old_l2_entry = get_l2_entry(s, l2_slice, l2_index + i);
+uint64_t old_l2_entry = get_l2_entry(s, l2_slice, l2_index + i);
+uint64_t old_l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index + i);
+uint64_t new_l2_entry = old_l2_entry;
+uint64_t new_l2_bitmap = old_l2_bitmap;
+QCow2ClusterType cluster_type =
+qcow2_get_cluster_type(bs, old_l2_entry);
 
 /*
+ * If full_discard is true, the cluster should not read back as zeroes,
+ * but rather fall through to the backing file.
+ *
  * If full_discard is false, make sure that a discarded area reads back
  * as zeroes for v3 images (we cannot do it for v2 without actually
  * writing a zero-filled buffer). We can skip the operation if the
@@ -1861,40 +1867,28 @@ static int discard_in_l2_slice(BlockDriverState *bs, 
uint64_t offset,
  *
  * TODO We might want to use bdrv_block_status(bs) here, but we're
  * holding s->lock, so that doesn't work today.
- *
- * If full_discard is true, the sector should not read back as zeroes,
- * but rather fall through to the backing file.
  */
-switch (qcow2_get_cluster_type(bs, old_l2_entry)) {
-case QCOW2_CLUSTER_UNALLOCATED:
-if (full_discard || !bs->backing) {
-continue;
-}
-break;
-
-case QCOW2_CLUSTER_ZERO_PLAIN:
-if (!full_discard) {
-continue;
+if (full_discard) {
+new_l2_entry = new_l2_bitmap = 0;
+} else if (bs->backing || qcow2_cluster_is_allocated(cluster_type)) {
+if (has_subclusters(s)) {
+new_l2_entry = 0;
+new_l2_bitmap = QCOW_L2_BITMAP_ALL_ZEROES;
+} else {
+new_l2_entry = s->qcow_version >= 3 ? QCOW_OFLAG_ZERO : 0;
 }
-break;
-
-case QCOW2_CLUSTER_ZERO_ALLOC:
-case QCOW2_CLUSTER_NORMAL:
-case QCOW2_CLUSTER_COMPRESSED:
-break;
+}
 
-default:
-abort();
+if (old_l2_entry == new_l2_entry && old_l2_bitmap == new_l2_bitmap) {
+continue;
 }
 
 /* First remove L2 entries */
 qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
-if (!full_discard && s->qcow_version >= 3) {
-set_l2_entry(s, l2_slice, l2_index + i, QCOW_OFLAG_ZERO);
-} else {
-set_l2_entry(s, l2_slice, l2_index + i, 0);
+set_l2_entry(s, l2_slice, l2_index + i, new_l2_entry);
+if (has_subclusters(s)) {
+set_l2_bitmap(s, l2_slice, l2_index + i, new_l2_bitmap);
 }
-
 /* Then decrease the refcount */
 qcow2_free_any_clusters(bs, old_l2_entry, 1, type);
 }
-- 
2.26.2

[PATCH 3/5] meson: Mingw64 gcc doesn't recognize system include_type for sdl2

[luoyonggang]

From: Yonggang Luo 

---
 meson.build | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index df5bf728b5..a3585881e1 100644
--- a/meson.build
+++ b/meson.build
@@ -224,8 +224,7 @@ if 'CONFIG_BRLAPI' in config_host
   brlapi = declare_dependency(link_args: config_host['BRLAPI_LIBS'].split())
 endif
 
-sdl = dependency('sdl2', required: get_option('sdl'), static: enable_static,
- include_type: 'system')
+sdl = dependency('sdl2', required: get_option('sdl'), static: enable_static)
 sdl_image = not_found
 if sdl.found()
   # work around 2.0.8 bug
-- 
2.27.0.windows.1

[PULL 31/34] qcow2: Add the 'extended_l2' option and the QCOW2_INCOMPAT_EXTL2 bit

[Max Reitz]

From: Alberto Garcia 

Now that the implementation of subclusters is complete we can finally
add the necessary options to create and read images with this feature,
which we call "extended L2 entries".

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Message-Id: 
<6476caaa73216bd05b7bb2d504a20415e1665176.1594396418.git.be...@igalia.com>
[mreitz: %s/5\.1/5.2/; fixed 302's and 303's reference output]
Signed-off-by: Max Reitz 
---
 qapi/block-core.json |   7 +++
 block/qcow2.h|   8 ++-
 include/block/block_int.h|   1 +
 block/qcow2.c|  66 ++--
 tests/qemu-iotests/031.out   |   8 +--
 tests/qemu-iotests/036.out   |   4 +-
 tests/qemu-iotests/049.out   | 102 +++
 tests/qemu-iotests/060.out   |   1 +
 tests/qemu-iotests/061.out   |  20 +++---
 tests/qemu-iotests/065   |  12 ++--
 tests/qemu-iotests/082.out   |  39 +---
 tests/qemu-iotests/085.out   |  38 ++--
 tests/qemu-iotests/144.out   |   4 +-
 tests/qemu-iotests/182.out   |   2 +-
 tests/qemu-iotests/185.out   |   8 +--
 tests/qemu-iotests/198   |   2 +
 tests/qemu-iotests/206.out   |   4 ++
 tests/qemu-iotests/242.out   |   5 ++
 tests/qemu-iotests/255.out   |   8 +--
 tests/qemu-iotests/274.out   |  49 ---
 tests/qemu-iotests/280.out   |   2 +-
 tests/qemu-iotests/291.out   |   2 +
 tests/qemu-iotests/302.out   |   1 +
 tests/qemu-iotests/303.out   |   4 +-
 tests/qemu-iotests/common.filter |   1 +
 25 files changed, 256 insertions(+), 142 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 197bdc1c36..db08c58d78 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -67,6 +67,9 @@
 # standalone (read-only) raw image without looking at qcow2
 # metadata (since: 4.0)
 #
+# @extended-l2: true if the image has extended L2 entries; only valid for
+#   compat >= 1.1 (since 5.2)
+#
 # @lazy-refcounts: on or off; only valid for compat >= 1.1
 #
 # @corrupt: true if the image has been marked corrupt; only valid for
@@ -88,6 +91,7 @@
   'compat': 'str',
   '*data-file': 'str',
   '*data-file-raw': 'bool',
+  '*extended-l2': 'bool',
   '*lazy-refcounts': 'bool',
   '*corrupt': 'bool',
   'refcount-bits': 'int',
@@ -4304,6 +4308,8 @@
 # @data-file-raw: True if the external data file must stay valid as a
 # standalone (read-only) raw image without looking at qcow2
 # metadata (default: false; since: 4.0)
+# @extended-l2  True to make the image have extended L2 entries
+#   (default: false; since 5.2)
 # @size: Size of the virtual disk in bytes
 # @version: Compatibility level (default: v3)
 # @backing-file: File name of the backing file if a backing file
@@ -4324,6 +4330,7 @@
   'data': { 'file': 'BlockdevRef',
 '*data-file':   'BlockdevRef',
 '*data-file-raw':   'bool',
+'*extended-l2': 'bool',
 'size': 'size',
 '*version': 'BlockdevQcow2Version',
 '*backing-file':'str',
diff --git a/block/qcow2.h b/block/qcow2.h
index f3499e53bf..065ec3df0b 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -246,15 +246,18 @@ enum {
 QCOW2_INCOMPAT_CORRUPT_BITNR= 1,
 QCOW2_INCOMPAT_DATA_FILE_BITNR  = 2,
 QCOW2_INCOMPAT_COMPRESSION_BITNR = 3,
+QCOW2_INCOMPAT_EXTL2_BITNR  = 4,
 QCOW2_INCOMPAT_DIRTY= 1 << QCOW2_INCOMPAT_DIRTY_BITNR,
 QCOW2_INCOMPAT_CORRUPT  = 1 << QCOW2_INCOMPAT_CORRUPT_BITNR,
 QCOW2_INCOMPAT_DATA_FILE= 1 << QCOW2_INCOMPAT_DATA_FILE_BITNR,
 QCOW2_INCOMPAT_COMPRESSION  = 1 << QCOW2_INCOMPAT_COMPRESSION_BITNR,
+QCOW2_INCOMPAT_EXTL2= 1 << QCOW2_INCOMPAT_EXTL2_BITNR,
 
 QCOW2_INCOMPAT_MASK = QCOW2_INCOMPAT_DIRTY
 | QCOW2_INCOMPAT_CORRUPT
 | QCOW2_INCOMPAT_DATA_FILE
-| QCOW2_INCOMPAT_COMPRESSION,
+| QCOW2_INCOMPAT_COMPRESSION
+| QCOW2_INCOMPAT_EXTL2,
 };
 
 /* Compatible feature bits */
@@ -581,8 +584,7 @@ typedef enum QCow2MetadataOverlap {
 
 static inline bool has_subclusters(BDRVQcow2State *s)
 {
-/* FIXME: Return false until this feature is complete */
-return false;
+return s->incompatible_features & QCOW2_INCOMPAT_EXTL2;
 }
 
 static inline size_t l2_entry_size(BDRVQcow2State *s)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 38dec0275b..9da7a42927 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -59,6 +59,7 @@
 #define BLOCK_OPT_DATA_FILE "data_file"
 #define BLOCK_OPT_DATA_FILE_RAW "data_file_raw"

[PULL 28/34] qcow2: Add subcluster support to qcow2_co_pwrite_zeroes()

[Max Reitz]

From: Alberto Garcia 

This works now at the subcluster level and pwrite_zeroes_alignment is
updated accordingly.

qcow2_cluster_zeroize() is turned into qcow2_subcluster_zeroize() with
the following changes:

   - The request can now be subcluster-aligned.

   - The cluster-aligned body of the request is still zeroized using
 zero_in_l2_slice() as before.

   - The subcluster-aligned head and tail of the request are zeroized
 with the new zero_l2_subclusters() function.

There is just one thing to take into account for a possible future
improvement: compressed clusters cannot be partially zeroized so
zero_l2_subclusters() on the head or the tail can return -ENOTSUP.
This makes the caller repeat the *complete* request and write actual
zeroes to disk. This is sub-optimal because

   1) if the head area was compressed we would still be able to use
  the fast path for the body and possibly the tail.

   2) if the tail area was compressed we are writing zeroes to the
  head and the body areas, which are already zeroized.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Message-Id: 
<17e05e2ee7e12f10dcf012da81e83ebe27eb3bef.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2.h |  4 +--
 block/qcow2-cluster.c | 81 +++
 block/qcow2.c | 33 +-
 3 files changed, 94 insertions(+), 24 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 4fad40b96b..4ef4ae4ab0 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -898,8 +898,8 @@ void qcow2_alloc_cluster_abort(BlockDriverState *bs, 
QCowL2Meta *m);
 int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
   uint64_t bytes, enum qcow2_discard_type type,
   bool full_discard);
-int qcow2_cluster_zeroize(BlockDriverState *bs, uint64_t offset,
-  uint64_t bytes, int flags);
+int qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset,
+ uint64_t bytes, int flags);
 
 int qcow2_expand_zero_clusters(BlockDriverState *bs,
BlockDriverAmendStatusCB *status_cb,
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 1e84bd8e2e..9d349d61c6 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -2016,12 +2016,59 @@ static int zero_in_l2_slice(BlockDriverState *bs, 
uint64_t offset,
 return nb_clusters;
 }
 
-int qcow2_cluster_zeroize(BlockDriverState *bs, uint64_t offset,
-  uint64_t bytes, int flags)
+static int zero_l2_subclusters(BlockDriverState *bs, uint64_t offset,
+   unsigned nb_subclusters)
+{
+BDRVQcow2State *s = bs->opaque;
+uint64_t *l2_slice;
+uint64_t old_l2_bitmap, l2_bitmap;
+int l2_index, ret, sc = offset_to_sc_index(s, offset);
+
+/* For full clusters use zero_in_l2_slice() instead */
+assert(nb_subclusters > 0 && nb_subclusters < s->subclusters_per_cluster);
+assert(sc + nb_subclusters <= s->subclusters_per_cluster);
+assert(offset_into_subcluster(s, offset) == 0);
+
+ret = get_cluster_table(bs, offset, &l2_slice, &l2_index);
+if (ret < 0) {
+return ret;
+}
+
+switch (qcow2_get_cluster_type(bs, get_l2_entry(s, l2_slice, l2_index))) {
+case QCOW2_CLUSTER_COMPRESSED:
+ret = -ENOTSUP; /* We cannot partially zeroize compressed clusters */
+goto out;
+case QCOW2_CLUSTER_NORMAL:
+case QCOW2_CLUSTER_UNALLOCATED:
+break;
+default:
+g_assert_not_reached();
+}
+
+old_l2_bitmap = l2_bitmap = get_l2_bitmap(s, l2_slice, l2_index);
+
+l2_bitmap |=  QCOW_OFLAG_SUB_ZERO_RANGE(sc, sc + nb_subclusters);
+l2_bitmap &= ~QCOW_OFLAG_SUB_ALLOC_RANGE(sc, sc + nb_subclusters);
+
+if (old_l2_bitmap != l2_bitmap) {
+set_l2_bitmap(s, l2_slice, l2_index, l2_bitmap);
+qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
+}
+
+ret = 0;
+out:
+qcow2_cache_put(s->l2_table_cache, (void **) &l2_slice);
+
+return ret;
+}
+
+int qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset,
+ uint64_t bytes, int flags)
 {
 BDRVQcow2State *s = bs->opaque;
 uint64_t end_offset = offset + bytes;
 uint64_t nb_clusters;
+unsigned head, tail;
 int64_t cleared;
 int ret;
 
@@ -2036,8 +2083,8 @@ int qcow2_cluster_zeroize(BlockDriverState *bs, uint64_t 
offset,
 }
 
 /* Caller must pass aligned values, except at image end */
-assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
-assert(QEMU_IS_ALIGNED(end_offset, s->cluster_size) ||
+assert(offset_into_subcluster(s, offset) == 0);
+assert(offset_into_subcluster(s, end_offset) == 0 ||
end_offset >= bs->total_sectors << BDRV_SECTOR_BITS);
 
 /*
@@ -2052,11 +2099,26 @@ int qcow2_cluster_zeroize(BlockDriverState *bs, 
uint64_t offset,
 return -ENOTSUP;
 }
 
- 

Re: meson: problems building under msys2/mingw-w64 native

[Paolo Bonzini]

I saw it, thanks. I would like to have someone else (like Mark) test it and
then I will include it.

Paolo

Il mar 25 ago 2020, 10:24 罗勇刚(Yonggang Luo)  ha
scritto:

> Hi Paolo Bonzini,
> I've already sent a series of fixes for msys, do you have a look at that?
>
>
> On Tue, Aug 25, 2020 at 3:55 PM Paolo Bonzini  wrote:
>
>> Great, thanks! Can you send it as a patch? I am collecting Meson fixes
>> and I should be able to send a pull request this week.
>>
>> Also if you can please test the msys fixes that were sent on the list
>> that would be great.
>>
>> Paolo
>>
>> Il mar 25 ago 2020, 09:52 Mark Cave-Ayland 
>> ha scritto:
>>
>>> On 24/08/2020 12:37, Gerd Hoffmann wrote:
>>>
>>> >> 2) GTK UI now depends on CONFIG_VTE
>>> >>
>>> >> This one I spotted on my local Linux setup as I didn't have the
>>> libvte-dev package
>>> >> installed and couldn't understand why I couldn't run QEMU with the
>>> GTK UI as I always
>>> >> do, even though configure reported that it found the GTK library and
>>> headers.
>>> >>
>>> >> A quick search showed that the GTK UI was being guarded by "if
>>> >> config_host.has_key('CONFIG_GTK') and
>>> config_host.has_key('CONFIG_VTE')" in
>>> >> ui/meson.build.
>>> >
>>> > That is not correct.  vte is intentionally not a hard dependency ...
>>> >
>>> >> For me the easy solution was to install libvte-dev, but since there
>>> are no VTE
>>> >> packages for Windows my guess is this will now make the GTK UI
>>> unavailable for
>>> >> Windows users.
>>> >
>>> > .. because we don't have that on windows.
>>> >
>>> > I think simply dropping the "and config_host.has_key('CONFIG_VTE')"
>>> > should work, can you try that?
>>>
>>> Hi Gerd,
>>>
>>> I can't get the native Windows build to complete yet, however I've
>>> removed the
>>> libvte-dev headers again on my Linux setup and confirmed that GTK works
>>> once again
>>> with the below diff:
>>>
>>> diff --git a/ui/meson.build b/ui/meson.build
>>> index 81fd393432..cc71f51f37 100644
>>> --- a/ui/meson.build
>>> +++ b/ui/meson.build
>>> @@ -42,7 +42,7 @@ if config_host.has_key('CONFIG_CURSES')
>>>ui_modules += {'curses' : curses_ss}
>>>  endif
>>>
>>> -if config_host.has_key('CONFIG_GTK') and
>>> config_host.has_key('CONFIG_VTE')
>>> +if config_host.has_key('CONFIG_GTK')
>>>softmmu_ss.add(when: 'CONFIG_WIN32', if_true:
>>> files('win32-kbd-hook.c'))
>>>
>>>gtk_ss = ss.source_set()
>>>
>>>
>>> ATB,
>>>
>>> Mark.
>>>
>>>
>
> --
>  此致
> 礼
> 罗勇刚
> Yours
> sincerely,
> Yonggang Luo
>

[PULL 34/34] iotests: Add tests for qcow2 images with extended L2 entries

[Max Reitz]

From: Alberto Garcia 

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Message-Id: 

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/271 | 901 +
 tests/qemu-iotests/271.out | 726 ++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 1628 insertions(+)
 create mode 100755 tests/qemu-iotests/271
 create mode 100644 tests/qemu-iotests/271.out

diff --git a/tests/qemu-iotests/271 b/tests/qemu-iotests/271
new file mode 100755
index 00..39ff462328
--- /dev/null
+++ b/tests/qemu-iotests/271
@@ -0,0 +1,901 @@
+#!/bin/bash
+#
+# Test qcow2 images with extended L2 entries
+#
+# Copyright (C) 2019-2020 Igalia, S.L.
+# Author: Alberto Garcia 
+#
+# 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 .
+#
+
+# creator
+owner=be...@igalia.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+status=1   # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+rm -f "$TEST_IMG.raw"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file nfs
+_supported_os Linux
+_unsupported_imgopts extended_l2 compat=0.10 cluster_size data_file 
refcount_bits=1[^0-9]
+
+l2_offset=$((0x4))
+
+_verify_img()
+{
+$QEMU_IMG compare "$TEST_IMG" "$TEST_IMG.raw" | grep -v 'Images are 
identical'
+$QEMU_IMG check "$TEST_IMG" | _filter_qemu_img_check | \
+grep -v 'No errors were found on the image'
+}
+
+# Compare the bitmap of an extended L2 entry against an expected value
+_verify_l2_bitmap()
+{
+entry_no="$1"# L2 entry number, starting from 0
+expected_alloc="$alloc"  # Space-separated list of allocated subcluster 
indexes
+expected_zero="$zero"# Space-separated list of zero subcluster indexes
+
+offset=$(($l2_offset + $entry_no * 16))
+entry=$(peek_file_be "$TEST_IMG" $offset 8)
+offset=$(($offset + 8))
+bitmap=$(peek_file_be "$TEST_IMG" $offset 8)
+
+expected_bitmap=0
+for bit in $expected_alloc; do
+expected_bitmap=$(($expected_bitmap | (1 << $bit)))
+done
+for bit in $expected_zero; do
+expected_bitmap=$(($expected_bitmap | (1 << (32 + $bit
+done
+printf -v expected_bitmap "%u" $expected_bitmap # Convert to unsigned
+
+printf "L2 entry #%d: 0x%016x %016x\n" "$entry_no" "$entry" "$bitmap"
+if [ "$bitmap" != "$expected_bitmap" ]; then
+printf "ERROR: expecting bitmap   0x%016x\n" "$expected_bitmap"
+fi
+}
+
+# This should be called as _run_test c=XXX sc=XXX off=XXX len=XXX cmd=XXX
+# c:   cluster number (0 if unset)
+# sc:  subcluster number inside cluster @c (0 if unset)
+# off: offset inside subcluster @sc, in kilobytes (0 if unset)
+# len: request length, passed directly to qemu-io (e.g: 256, 4k, 1M, ...)
+# cmd: the command to pass to qemu-io, must be one of
+#  write-> write
+#  zero -> write -z
+#  unmap-> write -z -u
+#  compress -> write -c
+#  discard  -> discard
+_run_test()
+{
+unset c sc off len cmd
+for var in "$@"; do eval "$var"; done
+case "${cmd:-write}" in
+zero)
+cmd="write -q -z";;
+unmap)
+cmd="write -q -z -u";;
+compress)
+pat=$((${pat:-0} + 1))
+cmd="write -q -c -P ${pat}";;
+write)
+pat=$((${pat:-0} + 1))
+cmd="write -q -P ${pat}";;
+discard)
+cmd="discard -q";;
+*)
+echo "Unknown option $cmd"
+exit 1;;
+esac
+c="${c:-0}"
+sc="${sc:-0}"
+off="${off:-0}"
+offset="$(($c * 64 + $sc * 2 + $off))"
+[ "$offset" != 0 ] && offset="${offset}k"
+cmd="$cmd ${offset} ${len}"
+raw_cmd=$(echo $cmd | sed s/-c//) # Raw images don't support -c
+echo $cmd | sed 's/-P [0-9][0-9]\?/-P PATTERN/'
+$QEMU_IO -c "$cmd" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "$raw_cmd" -f raw "$TEST_IMG.raw" | _filter_qemu_io
+_verify_img
+_verify_l2_bitmap "$c"
+}
+
+_reset_img()
+{
+size="$1"
+$QEMU_IMG create -f raw "$TEST_IMG.raw" "$size" | _filter_img_create
+if [ "$use_backing_file" = "yes" ]; then
+$QEMU_IMG create -f raw "$TEST_IMG.base" "$size" | _filter_img_create
+$QEMU_IO -c "write -q -P 0xFF 0 $size" -f raw "

[PULL 33/34] qcow2: Assert that expand_zero_clusters_in_l1() does not support subclusters

[Max Reitz]

From: Alberto Garcia 

This function is only used by qcow2_expand_zero_clusters() to
downgrade a qcow2 image to a previous version. This would require
transforming all extended L2 entries into normal L2 entries but this
is not a simple task and there are no plans to implement this at the
moment.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Message-Id: 
<15e65112b4144381b4d8c0bdf8fb76b0d813e3d1.1594396418.git.be...@igalia.com>
[mreitz: Fixed comment style]
Signed-off-by: Max Reitz 
---
 block/qcow2-cluster.c  | 14 --
 tests/qemu-iotests/061 |  6 ++
 tests/qemu-iotests/061.out |  5 +
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index fd506b4cb3..996b3314f4 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -2166,6 +2166,9 @@ static int expand_zero_clusters_in_l1(BlockDriverState 
*bs, uint64_t *l1_table,
 int ret;
 int i, j;
 
+/* qcow2_downgrade() is not allowed in images with subclusters */
+assert(!has_subclusters(s));
+
 slice_size2 = s->l2_slice_size * l2_entry_size(s);
 n_slices = s->cluster_size / slice_size2;
 
@@ -2233,8 +2236,11 @@ static int expand_zero_clusters_in_l1(BlockDriverState 
*bs, uint64_t *l1_table,
 
 if (cluster_type == QCOW2_CLUSTER_ZERO_PLAIN) {
 if (!bs->backing) {
-/* not backed; therefore we can simply deallocate the
- * cluster */
+/*
+ * not backed; therefore we can simply deallocate the
+ * cluster. No need to call set_l2_bitmap(), this
+ * function doesn't support images with subclusters.
+ */
 set_l2_entry(s, l2_slice, j, 0);
 l2_dirty = true;
 continue;
@@ -2305,6 +2311,10 @@ static int expand_zero_clusters_in_l1(BlockDriverState 
*bs, uint64_t *l1_table,
 } else {
 set_l2_entry(s, l2_slice, j, offset);
 }
+/*
+ * No need to call set_l2_bitmap() after set_l2_entry() because
+ * this function doesn't support images with subclusters.
+ */
 l2_dirty = true;
 }
 
diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
index 08ddbdd10c..5747beb7ed 100755
--- a/tests/qemu-iotests/061
+++ b/tests/qemu-iotests/061
@@ -303,6 +303,12 @@ $QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
 _img_info --format-specific
 _check_test_img
 
+echo
+echo "=== Testing version downgrade with extended L2 entries ==="
+echo
+_make_test_img -o "compat=1.1,extended_l2=on" 64M
+$QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
+
 echo
 echo "=== Try changing the external data file ==="
 echo
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index b0a1382046..ee30da2665 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -533,6 +533,11 @@ Format specific information:
 extended l2: false
 No errors were found on the image.
 
+=== Testing version downgrade with extended L2 entries ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+qemu-img: Cannot downgrade an image with incompatible features 0x10 set
+
 === Try changing the external data file ===
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
-- 
2.26.2

[PULL 32/34] qcow2: Allow preallocation and backing files if extended_l2 is set

[Max Reitz]

From: Alberto Garcia 

Traditional qcow2 images don't allow preallocation if a backing file
is set. This is because once a cluster is allocated there is no way to
tell that its data should be read from the backing file.

Extended L2 entries have individual allocation bits for each
subcluster, and therefore it is perfectly possible to have an
allocated cluster with all its subclusters unallocated.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Message-Id: 
<6d5b0f38e7dc5f2f31d8cab1cb92044e9909aece.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2.c  | 7 ---
 tests/qemu-iotests/206.out | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 00cda5696b..da56b1a4df 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3449,10 +3449,11 @@ qcow2_co_create(BlockdevCreateOptions *create_options, 
Error **errp)
 qcow2_opts->preallocation = PREALLOC_MODE_OFF;
 }
 if (qcow2_opts->has_backing_file &&
-qcow2_opts->preallocation != PREALLOC_MODE_OFF)
+qcow2_opts->preallocation != PREALLOC_MODE_OFF &&
+!qcow2_opts->extended_l2)
 {
-error_setg(errp, "Backing file and preallocation cannot be used at "
-   "the same time");
+error_setg(errp, "Backing file and preallocation can only be used at "
+   "the same time if extended_l2 is on");
 ret = -EINVAL;
 goto out;
 }
diff --git a/tests/qemu-iotests/206.out b/tests/qemu-iotests/206.out
index 363c5abe35..a100849fcb 100644
--- a/tests/qemu-iotests/206.out
+++ b/tests/qemu-iotests/206.out
@@ -203,7 +203,7 @@ Job failed: Different refcount widths than 16 bits require 
compatibility level 1
 === Invalid backing file options ===
 {"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": 
{"backing-file": "/dev/null", "driver": "qcow2", "file": "node0", 
"preallocation": "full", "size": 67108864}}}
 {"return": {}}
-Job failed: Backing file and preallocation cannot be used at the same time
+Job failed: Backing file and preallocation can only be used at the same time 
if extended_l2 is on
 {"execute": "job-dismiss", "arguments": {"id": "job0"}}
 {"return": {}}
 
-- 
2.26.2

[PULL 26/34] qcow2: Clear the L2 bitmap when allocating a compressed cluster

[Max Reitz]

From: Alberto Garcia 

Compressed clusters always have the bitmap part of the extended L2
entry set to 0.

Signed-off-by: Alberto Garcia 
Reviewed-by: Max Reitz 
Message-Id: 
<04455b3de5dfeb9d1cfe1fc7b02d7060a6e09710.1594396418.git.be...@igalia.com>
Signed-off-by: Max Reitz 
---
 block/qcow2-cluster.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 816ddc7639..1e84bd8e2e 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -862,6 +862,9 @@ int qcow2_alloc_compressed_cluster_offset(BlockDriverState 
*bs,
 BLKDBG_EVENT(bs->file, BLKDBG_L2_UPDATE_COMPRESSED);
 qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice);
 set_l2_entry(s, l2_slice, l2_index, cluster_offset);
+if (has_subclusters(s)) {
+set_l2_bitmap(s, l2_slice, l2_index, 0);
+}
 qcow2_cache_put(s->l2_table_cache, (void **) &l2_slice);
 
 *host_offset = cluster_offset & s->cluster_offset_mask;
-- 
2.26.2

[PATCH 4/5] meson: !/bin/sh are msys2 friendly.

[luoyonggang]

From: Yonggang Luo 

---
 scripts/undefsym.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/undefsym.sh b/scripts/undefsym.sh
index b9ec332e95..8189308b2c 100755
--- a/scripts/undefsym.sh
+++ b/scripts/undefsym.sh
@@ -1,4 +1,4 @@
-#! /usr/bin/env bash
+#!/bin/sh
 
 # Before a shared module's DSO is produced, a static library is built for it
 # and passed to this script.  The script generates -Wl,-u options to force
-- 
2.27.0.windows.1

[PATCH 5/5] configure: replace all $PWD with $build_path that can handling msys2 properly

[luoyonggang]

From: Yonggang Luo 

---
 configure | 34 ++
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/configure b/configure
index b8f5b81a67..a0e2b20877 100755
--- a/configure
+++ b/configure
@@ -13,8 +13,13 @@ export CCACHE_RECACHE=yes
 
 # make source path absolute
 source_path=$(cd "$(dirname -- "$0")"; pwd)
+build_path=$PWD
+if [ "$MSYSTEM" = "MINGW64" -o  "$MSYSTEM" = "MINGW32" ]; then
+source_path=$(cd "$(dirname -- "$0")"; pwd -W)
+build_path=`pwd -W`
+fi
 
-if test "$PWD" = "$source_path"
+if test "$build_path" = "$source_path"
 then
 echo "Using './build' as the directory for build output"
 
@@ -346,7 +351,12 @@ ld_has() {
 $ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
 }
 
-if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
+check_valid_build_path="[[:space:]:]"
+if [ "$MSYSTEM" = "MINGW64" -o  "$MSYSTEM" = "MINGW32" ]; then
+check_valid_build_path="[[:space:]]"
+fi
+
+if printf %s\\n "$source_path" "$build_path" | grep -q 
"$check_valid_build_path";
 then
   error_exit "main directory cannot contain spaces nor colons"
 fi
@@ -942,7 +952,7 @@ Linux)
   linux="yes"
   linux_user="yes"
   kvm="yes"
-  QEMU_INCLUDES="-isystem ${source_path}/linux-headers -I$PWD/linux-headers 
$QEMU_INCLUDES"
+  QEMU_INCLUDES="-isystem ${source_path}/linux-headers 
-I${build_path}/linux-headers $QEMU_INCLUDES"
   libudev="yes"
 ;;
 esac
@@ -4299,7 +4309,7 @@ EOF
   symlink "$source_path/dtc/Makefile" "dtc/Makefile"
   fi
   fdt_cflags="-I${source_path}/dtc/libfdt"
-  fdt_ldflags="-L$PWD/dtc/libfdt"
+  fdt_ldflags="-L${build_path}/dtc/libfdt"
   fdt_libs="$fdt_libs"
   elif test "$fdt" = "yes" ; then
   # Not a git build & no libfdt found, prompt for system install
@@ -5284,7 +5294,7 @@ case "$capstone" in
 else
   LIBCAPSTONE=libcapstone.a
 fi
-capstone_libs="-L$PWD/capstone -lcapstone"
+capstone_libs="-L${build_path}/capstone -lcapstone"
 capstone_cflags="-I${source_path}/capstone/include"
 ;;
 
@@ -6284,8 +6294,8 @@ case "$slirp" in
   git_submodules="${git_submodules} slirp"
 fi
 mkdir -p slirp
-slirp_cflags="-I${source_path}/slirp/src -I$PWD/slirp/src"
-slirp_libs="-L$PWD/slirp -lslirp"
+slirp_cflags="-I${source_path}/slirp/src -I${build_path}/slirp/src"
+slirp_libs="-L${build_path}/slirp -lslirp"
 if test "$mingw32" = "yes" ; then
   slirp_libs="$slirp_libs -lws2_32 -liphlpapi"
 fi
@@ -8233,7 +8243,7 @@ fi
 mv $cross config-meson.cross
 
 rm -rf meson-private meson-info meson-logs
-NINJA=$PWD/ninjatool $meson setup \
+NINJA="${build_path}/ninjatool" $meson setup \
 --prefix "${pre_prefix}$prefix" \
 --libdir "${pre_prefix}$libdir" \
 --libexecdir "${pre_prefix}$libexecdir" \
@@ -8249,11 +8259,11 @@ NINJA=$PWD/ninjatool $meson setup \
 -Dstrip=$(if test "$strip_opt" = yes; then echo true; else echo false; 
fi) \
 -Db_pie=$(if test "$pie" = yes; then echo true; else echo false; fi) \
 -Db_coverage=$(if test "$gcov" = yes; then echo true; else echo false; 
fi) \
-   -Dsdl=$sdl -Dsdl_image=$sdl_image \
-   -Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png 
\
-   -Dgettext=$gettext \
+-Dsdl=$sdl -Dsdl_image=$sdl_image \
+-Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg 
-Dvnc_png=$vnc_png \
+-Dgettext=$gettext \
 $cross_arg \
-"$PWD" "$source_path"
+"$build_path" "$source_path"
 
 if test "$?" -ne 0 ; then
 error_exit "meson setup failed"
-- 
2.27.0.windows.1

Re: [PATCH 4/5] meson: !/bin/sh are msys2 friendly.

[Daniel P . Berrangé]

On Tue, Aug 25, 2020 at 04:34:59PM +0800, luoyongg...@gmail.com wrote:
> From: Yonggang Luo 
> 
> ---
>  scripts/undefsym.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/undefsym.sh b/scripts/undefsym.sh
> index b9ec332e95..8189308b2c 100755
> --- a/scripts/undefsym.sh
> +++ b/scripts/undefsym.sh
> @@ -1,4 +1,4 @@
> -#! /usr/bin/env bash
> +#!/bin/sh

Does this script actually work on non-bash shells ?  If not, then this
change will likely break on plaforms where /bin/sh is not bash.

>  
>  # Before a shared module's DSO is produced, a static library is built for it
>  # and passed to this script.  The script generates -Wl,-u options to force
> -- 
> 2.27.0.windows.1
> 
> 

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

Re: [PATCH 5/5] configure: replace all $PWD with $build_path that can handling msys2 properly

[Daniel P . Berrangé]

On Tue, Aug 25, 2020 at 04:35:00PM +0800, luoyongg...@gmail.com wrote:

Please explain *why* the change is needed in the commit message.
ie explain what's broken with current code.

> From: Yonggang Luo 
> 
> ---
>  configure | 34 ++
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/configure b/configure
> index b8f5b81a67..a0e2b20877 100755
> --- a/configure
> +++ b/configure
> @@ -13,8 +13,13 @@ export CCACHE_RECACHE=yes
>  
>  # make source path absolute
>  source_path=$(cd "$(dirname -- "$0")"; pwd)
> +build_path=$PWD
> +if [ "$MSYSTEM" = "MINGW64" -o  "$MSYSTEM" = "MINGW32" ]; then
> +source_path=$(cd "$(dirname -- "$0")"; pwd -W)
> +build_path=`pwd -W`

What is the -W arg - that seems to not exist in pwd that I
see, so I guess its something msys custom.

> +fi
>  
> -if test "$PWD" = "$source_path"
> +if test "$build_path" = "$source_path"
>  then
>  echo "Using './build' as the directory for build output"
>  
> @@ -346,7 +351,12 @@ ld_has() {
>  $ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
>  }
>  
> -if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
> +check_valid_build_path="[[:space:]:]"
> +if [ "$MSYSTEM" = "MINGW64" -o  "$MSYSTEM" = "MINGW32" ]; then
> +check_valid_build_path="[[:space:]]"
> +fi
> +
> +if printf %s\\n "$source_path" "$build_path" | grep -q 
> "$check_valid_build_path";
>  then
>error_exit "main directory cannot contain spaces nor colons"
>  fi
> @@ -942,7 +952,7 @@ Linux)
>linux="yes"
>linux_user="yes"
>kvm="yes"
> -  QEMU_INCLUDES="-isystem ${source_path}/linux-headers -I$PWD/linux-headers 
> $QEMU_INCLUDES"
> +  QEMU_INCLUDES="-isystem ${source_path}/linux-headers 
> -I${build_path}/linux-headers $QEMU_INCLUDES"
>libudev="yes"
>  ;;
>  esac
> @@ -4299,7 +4309,7 @@ EOF
>symlink "$source_path/dtc/Makefile" "dtc/Makefile"
>fi
>fdt_cflags="-I${source_path}/dtc/libfdt"
> -  fdt_ldflags="-L$PWD/dtc/libfdt"
> +  fdt_ldflags="-L${build_path}/dtc/libfdt"
>fdt_libs="$fdt_libs"
>elif test "$fdt" = "yes" ; then
># Not a git build & no libfdt found, prompt for system install
> @@ -5284,7 +5294,7 @@ case "$capstone" in
>  else
>LIBCAPSTONE=libcapstone.a
>  fi
> -capstone_libs="-L$PWD/capstone -lcapstone"
> +capstone_libs="-L${build_path}/capstone -lcapstone"
>  capstone_cflags="-I${source_path}/capstone/include"
>  ;;
>  
> @@ -6284,8 +6294,8 @@ case "$slirp" in
>git_submodules="${git_submodules} slirp"
>  fi
>  mkdir -p slirp
> -slirp_cflags="-I${source_path}/slirp/src -I$PWD/slirp/src"
> -slirp_libs="-L$PWD/slirp -lslirp"
> +slirp_cflags="-I${source_path}/slirp/src -I${build_path}/slirp/src"
> +slirp_libs="-L${build_path}/slirp -lslirp"
>  if test "$mingw32" = "yes" ; then
>slirp_libs="$slirp_libs -lws2_32 -liphlpapi"
>  fi
> @@ -8233,7 +8243,7 @@ fi
>  mv $cross config-meson.cross
>  
>  rm -rf meson-private meson-info meson-logs
> -NINJA=$PWD/ninjatool $meson setup \
> +NINJA="${build_path}/ninjatool" $meson setup \
>  --prefix "${pre_prefix}$prefix" \
>  --libdir "${pre_prefix}$libdir" \
>  --libexecdir "${pre_prefix}$libexecdir" \
> @@ -8249,11 +8259,11 @@ NINJA=$PWD/ninjatool $meson setup \
>  -Dstrip=$(if test "$strip_opt" = yes; then echo true; else echo 
> false; fi) \
>  -Db_pie=$(if test "$pie" = yes; then echo true; else echo false; fi) 
> \
>  -Db_coverage=$(if test "$gcov" = yes; then echo true; else echo 
> false; fi) \
> - -Dsdl=$sdl -Dsdl_image=$sdl_image \
> - -Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png 
> \
> - -Dgettext=$gettext \
> +-Dsdl=$sdl -Dsdl_image=$sdl_image \
> +-Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg 
> -Dvnc_png=$vnc_png \
> +-Dgettext=$gettext \
>  $cross_arg \
> -"$PWD" "$source_path"
> +"$build_path" "$source_path"
>  
>  if test "$?" -ne 0 ; then
>  error_exit "meson setup failed"
> -- 
> 2.27.0.windows.1
> 
> 

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

Re: [PATCH] meson: Don't make object files for dtrace on macOS

[Roman Bolshakov]

On Mon, Aug 24, 2020 at 03:24:31PM +0100, Daniel P. Berrangé wrote:
> On Sun, Aug 23, 2020 at 12:05:47PM +0300, Roman Bolshakov wrote:
> > dtrace on macOS uses unresolved symbols with a special prefix to define
> > probes [1], only headers should be generated for USDT (dtrace(1)). But
> > it doesn't support backwards compatible no-op -G flag [2] and implicit
> > build rules fail.
> > 
> > 1. https://markmail.org/message/6grq2ygr5nwdwsnb
> > 2. https://markmail.org/message/5xrxt2w5m42nojkz
> > 
> > Cc: Daniel P. Berrangé 
> > Cc: Cameron Esfahani 
> > Signed-off-by: Roman Bolshakov 
> > ---
> >  trace/meson.build | 13 -
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> Reviewed-by: Daniel P. Berrangé 
> 

Paolo, should it go through your meson-fixes tree?

Regards,
Roman

Re: [PATCH 2/5] meson: fixes relpath may fail on win32. for example C:/msys64/mingw64/x.exe relative to E:/path/qemu-build would fail.

[Daniel P . Berrangé]

Subject line is way too long here.  The explanation should be
in the commit message body, not the subject.

On Tue, Aug 25, 2020 at 04:34:57PM +0800, luoyongg...@gmail.com wrote:
> From: Yonggang Luo 
> 
> ---
>  scripts/mtest2make.py | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
> index bdb257bbd9..d7a51bf97e 100644
> --- a/scripts/mtest2make.py
> +++ b/scripts/mtest2make.py
> @@ -53,9 +53,16 @@ i = 0
>  for test in json.load(sys.stdin):
>  env = ' '.join(('%s=%s' % (shlex.quote(k), shlex.quote(v))
>  for k, v in test['env'].items()))
> -executable = os.path.relpath(test['cmd'][0])
> +executable = test['cmd'][0]
> +try:
> +executable = os.path.relpath(executable)
> +except:
> +pass
>  if test['workdir'] is not None:
> -test['cmd'][0] = os.path.relpath(test['cmd'][0], test['workdir'])
> +try:
> +test['cmd'][0] = os.path.relpath(executable, test['workdir'])
> +except:
> +test['cmd'][0] = executable
>  else:
>  test['cmd'][0] = executable
>  cmd = '$(.test.env) %s %s' % (env, ' '.join((shlex.quote(x) for x in 
> test['cmd'])))
> -- 
> 2.27.0.windows.1
> 
> 

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

Re: [PATCH 1/5] meson: SIMPLE_PATH_RE should match the full path token. Or the $ and : contained in path would not matched. if the path are start with C:/ and E:/

[Daniel P . Berrangé]

Commit message subject is way  too long. Put the explanation into
commit message body, not the subject line.

On Tue, Aug 25, 2020 at 04:34:56PM +0800, luoyongg...@gmail.com wrote:
> From: Yonggang Luo 
> 
> ---
>  scripts/ninjatool.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/ninjatool.py b/scripts/ninjatool.py
> index cc77d51aa8..6ca8be6f10 100755
> --- a/scripts/ninjatool.py
> +++ b/scripts/ninjatool.py
> @@ -55,7 +55,7 @@ else:
>  
>  PATH_RE = r"[^$\s:|]+|\$[$ :]|\$[a-zA-Z0-9_-]+|\$\{[a-zA-Z0-9_.-]+\}"
>  
> -SIMPLE_PATH_RE = re.compile(r"[^$\s:|]+")
> +SIMPLE_PATH_RE = re.compile(r"^[^$\s:|]+$")
>  IDENT_RE = re.compile(r"[a-zA-Z0-9_.-]+$")
>  STRING_RE = re.compile(r"(" + PATH_RE + r"|[\s:|])(?:\r?\n)?|.")
>  TOPLEVEL_RE = re.compile(r"([=:#]|\|\|?|^ +|(?:" + PATH_RE + r")+)\s*|.")
> -- 
> 2.27.0.windows.1
> 
> 

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

Re: [PATCH 4/5] meson: !/bin/sh are msys2 friendly.

[Yonggang Luo]

Hi, works, msys2 sh compiled and run

On Tue, Aug 25, 2020 at 4:59 PM Daniel P. Berrangé 
wrote:

> On Tue, Aug 25, 2020 at 04:34:59PM +0800, luoyongg...@gmail.com wrote:
> > From: Yonggang Luo 
> >
> > ---
> >  scripts/undefsym.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/scripts/undefsym.sh b/scripts/undefsym.sh
> > index b9ec332e95..8189308b2c 100755
> > --- a/scripts/undefsym.sh
> > +++ b/scripts/undefsym.sh
> > @@ -1,4 +1,4 @@
> > -#! /usr/bin/env bash
> > +#!/bin/sh
>
> Does this script actually work on non-bash shells ?  If not, then this
> change will likely break on plaforms where /bin/sh is not bash.
>
> >
> >  # Before a shared module's DSO is produced, a static library is built
> for it
> >  # and passed to this script.  The script generates -Wl,-u options to
> force
> > --
> > 2.27.0.windows.1
> >
> >
>
> Regards,
> Daniel
> --
> |: https://berrange.com  -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-
> https://www.instagram.com/dberrange :|
>
>

-- 
 此致
礼
罗勇刚
Yours
sincerely,
Yonggang Luo

Re: [PATCH] meson: Don't make object files for dtrace on macOS

[Paolo Bonzini]

Yes, I have already queued it.

Paolo

Il mar 25 ago 2020, 11:04 Roman Bolshakov  ha
scritto:

> On Mon, Aug 24, 2020 at 03:24:31PM +0100, Daniel P. Berrangé wrote:
> > On Sun, Aug 23, 2020 at 12:05:47PM +0300, Roman Bolshakov wrote:
> > > dtrace on macOS uses unresolved symbols with a special prefix to define
> > > probes [1], only headers should be generated for USDT (dtrace(1)). But
> > > it doesn't support backwards compatible no-op -G flag [2] and implicit
> > > build rules fail.
> > >
> > > 1. https://markmail.org/message/6grq2ygr5nwdwsnb
> > > 2. https://markmail.org/message/5xrxt2w5m42nojkz
> > >
> > > Cc: Daniel P. Berrangé 
> > > Cc: Cameron Esfahani 
> > > Signed-off-by: Roman Bolshakov 
> > > ---
> > >  trace/meson.build | 13 -
> > >  1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > Reviewed-by: Daniel P. Berrangé 
> >
>
> Paolo, should it go through your meson-fixes tree?
>
> Regards,
> Roman
>
>

Re: [PATCH 3/4] configure: Prefer gmake on darwin

[Roman Bolshakov]

On Mon, Aug 24, 2020 at 04:57:31PM +0100, Peter Maydell wrote:
> On Mon, 24 Aug 2020 at 15:51, Eric Blake  wrote:
> >
> > On 8/22/20 4:21 PM, Roman Bolshakov wrote:
> > > New meson/make build requires GNU make 3.82+ but macOS ships 3.81 even
> > > on Big Sur while homebrew provides GNU make 4.3 as 'gmake' in $PATH.
> >
> > Does this line up with our development policies on supported platforms?
> > Should we be fixing the creation of Makefile.ninja to avoid constructs
> > not understood by older GNU make, if that is what is shipped out of the
> > box on MacOS as one of our supported platforms?  Or is MacOS on the
> > fringe for what counts as supported, where we are okay mandating that
> > users must install a separate newer GNU make than what comes by default?
> 
> If it's easy to add back support for make 3.81 that would be the
> nicest thing, I think. But we already require the user to install
> a non-system python, for instance, so asking them to also install
> make from homebrew isn't a completely new thing. (The only awkward
> thing is that homebrew doesn't actually put the new make on the
> path as 'make', only as 'gmake', so you have to then manually
> fiddle the PATH.) At some point requiring some tools from homebrew
> or similar for QEMU compilation is just inevitable given
> Apple's apparent policy of never moving the system versions of
> tools beyond the last GPLv2 version.
> 

Never thought of that, but perhaps it's similar to what happened with
bash. Apple shipped an old GPLv2 version of bash (3.2) for quite a while
even after 4.x release. Then they suddenly switched default shell to
zsh. Following the approach, we're more likely to see meson and ninja in
Apple Command Line Tools than GNU Make 3.82+ 🙂

As for alias, Homebrew also provides GNU coreutils and sed with g
prefix and a special gnubin prefix is provided to simplify bulk addition
of GNU tools to PATH, so it's consistent in some sense :)

Here's a related homebrew discussion about system binary shadowing:
https://discourse.brew.sh/t/why-was-with-default-names-removed/4405/14

-Roman

Re: [PULL v2 00/24] target/xtensa updates for 5.2

[Peter Maydell]

On Mon, 24 Aug 2020 at 22:54, Max Filippov  wrote:
>
> On Mon, Aug 24, 2020 at 2:33 PM Peter Maydell  
> wrote:
> > On Sat, 22 Aug 2020 at 20:48, Max Filippov  wrote:
> > > On Sat, Aug 22, 2020 at 3:20 AM Philippe Mathieu-Daudé  
> > > wrote:
> > > >
> > > > Where does that come from?
> > >
> > > Generated by xtensa processor generator, as one of many output artifacts.
> >
> > Is there anything different with the source for these cores
> > compared to the ones we already have in the tree, or are
> > these just "more cores, generated the same way as the old ones" ?
>
> They are generated the same way as the old ones, but they have different
> configurations: they support FPU2000 and DFPU opcodes implemented
> earlier in this series. I've added them to enable testing of these opcodes.
> de233_fpu is supposed to be a platform for further xtensa gcc development.

OK, in that case I don't see any reason why we should keep them
out of the tree. Which isn't to shut down the point Philippe has
raised about generated sources, just that there's not much need
to block this pullreq while we have that conversation since it's
not a change from the status-quo.


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM

Re: meson.build files are missing copyright/license headers

[Peter Maydell]

On Tue, 25 Aug 2020 at 06:57, Thomas Huth  wrote:
>
> On 24/08/2020 20.46, Peter Maydell wrote:
> > We don't mandate SPDX[*],
> > but it will do the job if you want to use it.
> >
> > [*] Mostly because nobody's cared enough to say "we should
> > standardize on this and convert existing files and add the
> > check to checkpatch that new files have an SPDX line".
>
> We should standardize on this and convert existing files and add the
> check to checkpatch that new files have an SPDX line! :-)
>
> Ok, now sombody said it loud. Would there be any objections to enforce
> this via checkpatch on new files?

I wouldn't object, indeed I think checkpatch-enforcement is an
important part of the process. I think that if we're going
to move to SPDX we should:
 * look at the SPDX spec and identify what we need to do beyond
   just adding SPDX lines (eg do we need a LICENSES/ subdir
   like the kernel has with a file per license?)
 * document (in docs/devel?) that we require SPDX
 * decide what our plan is for 3rd-party code (libvixl, etc)
   [where any SPDX line we add will be lost again next time
   we resync our copy of the code]
 * put in a checkpatch check for new files (presumably the
   Linux kernel has one we can borrow)
 * update existing files (I think the kernel folks probably
   have scripted stuff for the easy parts of this; multi-license
   files like fpu/softfloat.c likely need by-hand conversion)

Mostly I think if we're going to do this we should find somebody
who wants to put in the work to push it forwards so we don't
have a half-and-half interminably extended transition between
old-style license notices and SPDX.

thanks
-- PMM

Re: [PATCH 02/10] numa: introduce MachineClass::forbid_asymmetrical_numa

[Daniel Henrique Barboza]

On 8/24/20 8:49 PM, David Gibson wrote:

On Mon, Aug 24, 2020 at 08:45:12AM -0300, Daniel Henrique Barboza wrote:





[...]


LOPAPR support a somewhat asymmetrical NUMA setup in its current
form,


Huh, I didn't even realize that.  What's the mechanism?


LOPAPR mentions that a single resource/node can have multiple associativity
arrays. The idea is to contemplate the situations where the node has
more than one connection with the board.

I say "somewhat" because, right after mentioning that, the spec also says that
the OS should consider that the distance between two nodes must always be
the shortest one of all available arrays. I'll copy/paste the except here
(end of section 15.2, "Numa Resource Associativity":


Ah.  I didn't think that's what "asymmetric NUMA" meant... but come to
think of it, I'm not very sure about that.



This was a poor attempt of my part to cut PAPR some slack.

TBH, even if current PAPR allows for some form of NUMA asymmetry, I don't think
it's worth implementing at all. It'll be more complexity on top of what I
already added here, and the best case scenario will be the kernel ignoring it
(worst case - kernel blowing it up because we're adding more associativity
arrays in each CPU and so on).



Thanks,


DHB




-

The reason that the “ibm,associativity” property may contain multiple 
associativity
lists is that a resource may be multiply connected into the platform. This 
resource
then has a different associativity characteristics relative to its multiple 
connections.
To determine the associativity between any two resources, the OS scans down the 
two
resources associativity lists in all pair wise combinations counting how many 
domains
are the same until the first domain where the two list do not agree. The 
highest such
count is the associativity between the two resources.




DHB





but
the Linux kernel doesn't support it. The effort to implement it in the current
spapr machine code, given that Linux wouldn't mind it, is not worth it. This
is why I chose to invalidate it for pseries.


Igor,

It's kind of difficult to answer that question - PAPR doesn't
specifically describe limitations, it's just that the representation
it uses is inherently limited.  Instead of the obvious, simple and
pretty much universal method (used in the generic kernel and qemu) of
having a matrix of distance between all the nodes, it instead
describes the hierarchy of components that give rise to the different
distances.

So, for each NUMA relevant object (cpu, memory block, host bridge,
etc.) there is a vector of IDs.  Each number in the vector gives one
level of the objects location in the heirarchy.

So, for example the first number might be the physical chip/socket.
the second one which group of cores & memory interfaces sharing an Ln
cache, the third one the specific core number.  So to work out how far
objects are from each other you essentially look at how long a prefix
of their vector they share, which tells you how far above in the
hierarchy you have to go to reach it.

There's a bunch of complicating details, but that's the gist of it.


Perhaps a warning would be better in this case?

In either case, it sounds like this won't be a common constraint
and I now agree with your original suggestion of doing this in
machine initialization code.

Agreed, if it goes to spapr specific machine code I will not object much.
(it will burden just spapr maintainers, so it's about convincing
David in the end)


I believe he's ok with it given that he suggested it in his first reply.

I'll move this verification to spapr machine_init in the next version.



Thanks,

DHB

Re: [RFC PATCH 0/9] block/curl: Add caching of data downloaded from the remote server

[Max Reitz]

On 19.08.20 16:19, David Edmondson wrote:
> On Wednesday, 2020-08-19 at 15:11:37 +01, Stefan Hajnoczi wrote:
> 
>> On Tue, Aug 18, 2020 at 12:08:36PM +0100, David Edmondson wrote:
>>> When using qemu-img to convert an image that is hosted on an HTTP
>>> server to some faster local (or pseudo-local) storage, the overall
>>> performance can be improved by reading data from the HTTP server in
>>> larger blocks and by caching and re-using blocks already read. This
>>> set of patches implements both of these, and adds a further patch
>>> allowing an offset to be added to all of the HTTP requests.
>>
>> Hi David,
>> Thanks for posting this! Kevin and Max are the maintainers in this area,
>> but I wanted to ask an initial question:
>>
>> Is caching curl-specific or could this be implemented as a block filter
>> driver so that it can be stacked on top of other network protocols too?
> 
> This implementation is curl specific, as you probably surmised. I will
> look at implementing something similar as a block filter.

I think from an implementation standpoint the best would be if we could
just use such a generic caching block filter above all curl nodes so we
can drop all caching from curl.

However, I suppose then we’d at least have the problem of how to put
this cache node on top of all curl nodes without breaking compatibility,
which may be impossible.

OTOH, maybe it would be fine to leave the new cache optional, and just
leave the curl driver itself as it is.  Which would also mean that
wouldn’t need readahead support in the cache driver.

But if we do need this full cache directly in the curl driver, is it at
least possible to share most of the caching code between it and a
(potential future) dedicated cache block driver?

Max



signature.asc
Description: OpenPGP digital signature

Re: [EXTERNAL] Re: [PATCH] hw/arm/sbsa-ref.c : Add a fake embedded controller

[Graeme Gregory]

On Mon, Aug 24, 2020 at 03:59:38PM +0100, Peter Maydell wrote:
> On Thu, 20 Aug 2020 at 14:32, Graeme Gregory  wrote:
> >
> > A difference between sbsa platform and the virt platform is PSCI is
> > handled by ARM-TF in the sbsa platform. This means that the PSCI code
> > there needs to communicate some of the platform power changes down
> > to the qemu code for things like shutdown/reset control.
> >
> > Space has been left to extend the EC if we find other use cases in
> > future where ARM-TF and qemu need to communicate.
> >
> > Signed-off-by: Graeme Gregory 
> > ---
> >  hw/arm/sbsa-ref.c | 95 +++
> >  1 file changed, 95 insertions(+)
> >
> > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> > index f030a416fd..c8743fc1d0 100644
> > --- a/hw/arm/sbsa-ref.c
> > +++ b/hw/arm/sbsa-ref.c
> > @@ -41,6 +41,7 @@
> >  #include "hw/usb.h"
> >  #include "hw/char/pl011.h"
> >  #include "net/net.h"
> > +#include "migration/vmstate.h"
> >
> >  #define RAMLIMIT_GB 8192
> >  #define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
> > @@ -62,6 +63,7 @@ enum {
> >  SBSA_CPUPERIPHS,
> >  SBSA_GIC_DIST,
> >  SBSA_GIC_REDIST,
> > +SBSA_SECURE_EC,
> >  SBSA_SMMU,
> >  SBSA_UART,
> >  SBSA_RTC,
> > @@ -98,6 +100,14 @@ typedef struct {
> >  #define SBSA_MACHINE(obj) \
> >  OBJECT_CHECK(SBSAMachineState, (obj), TYPE_SBSA_MACHINE)
> >
> > +typedef struct {
> > +SysBusDevice parent_obj;
> > +MemoryRegion iomem;
> > +} SECUREECState;
> 
> Could you put the definition of the device in its own .c file,
> please (hw/misc/ seems like the right place). We generally
> prefer to keep the board and individual device models
> separate. (Some older code in the tree still has devices
> lurking in source files, but new stuff generally follows
> the rules.)
> 
Yes, certainly!

> > +enum sbsa_secure_ec_powerstates {
> > +SBSA_SECURE_EC_CMD_NULL,
> > +SBSA_SECURE_EC_CMD_POWEROFF,
> > +SBSA_SECURE_EC_CMD_REBOOT,
> 
> The last two are clear enough, but what's a null power state for?
> 
My personal dislike of sending 0 to hardware as its harder to spot
when using scopes/logic analyzers. I can remove it if you wish and
explicitly number the states.

> > +static uint64_t secure_ec_read(void *opaque, hwaddr offset, unsigned size)
> > +{
> > +/* No use for this currently */
> > +return 0;
> > +}
> > +
> > +static void secure_ec_write(void *opaque, hwaddr offset,
> > + uint64_t value, unsigned size)
> > +{
> > +if (offset == 0) { /* PSCI machine power command register */
> > +switch (value) {
> > +case SBSA_SECURE_EC_CMD_NULL:
> > +break;
> > +case SBSA_SECURE_EC_CMD_POWEROFF:
> > +qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> > +break;
> > +case SBSA_SECURE_EC_CMD_REBOOT:
> > +qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> > +break;
> > +default:
> > +error_report("sbsa-ref: ERROR Unkown power command");
> 
> "unknown".
> 
> We prefer qemu_log_mask(LOG_GUEST_ERROR, ...) for logging this
> kind of "guest code did something wrong" situation.
> (you could also do that for attempting to read this w/o register
> if you liked).
> 
Excellent that is much better, Ill make that change.

> > +}
> > +} else {
> > +error_report("sbsa-ref: unknown EC register");
> 
> similarly here
> 
> > +}
> > +}
> > +
> > +static const MemoryRegionOps secure_ec_ops = {
> > +.read = secure_ec_read,
> > +.write = secure_ec_write,
> > +.endianness = DEVICE_NATIVE_ENDIAN,
> 
> Consider explicitly specifying the permitted access size
> by setting .valid.min_access_size and .valid.max_access_size
> (restricting guests to only doing 32-bit writes will make
> life easier when you come to add registers later...)
> 
That was my original intent so i will do this.

> > +};
> > +
> > +static void secure_ec_init(Object *obj)
> > +{
> > +SECUREECState *s = SECURE_EC(obj);
> > +SysBusDevice *dev = SYS_BUS_DEVICE(obj);
> > +
> > +memory_region_init_io(&s->iomem, obj, &secure_ec_ops, s, "secure-ec",
> > +0x1000);
> 
> Minor indent error here.
> 
Will fix, just FYI checkpatch does not pick this up currently.

> > +sysbus_init_mmio(dev, &s->iomem);
> > +}
> > +
> > +static void create_secure_ec(MemoryRegion *mem)
> > +{
> > +hwaddr base = sbsa_ref_memmap[SBSA_SECURE_EC].base;
> > +DeviceState *dev = qdev_create(NULL, TYPE_SECURE_EC);
> > +SysBusDevice *s = SYS_BUS_DEVICE(dev);
> > +
> > +memory_region_add_subregion(mem, base,
> > +sysbus_mmio_get_region(s, 0));
> > +}
> > +
> >  static void sbsa_ref_init(MachineState *machine)
> >  {
> >  unsigned int smp_cpus = machine->smp.cpus;
> > @@ -708,6 +778,8 @@ static void sbsa_ref_init(MachineState *machine)
> >
> >  create_pcie(sms);
> >
> > +create_secure_ec(sec

Re: [EXTERNAL] Re: [PATCH] hw/arm/sbsa-ref.c : Add a fake embedded controller

[Graeme Gregory]

On Fri, Aug 21, 2020 at 03:49:11PM +0200, Philippe Mathieu-Daudé wrote:
> On 8/20/20 3:32 PM, Graeme Gregory wrote:
> > A difference between sbsa platform and the virt platform is PSCI is
> > handled by ARM-TF in the sbsa platform. This means that the PSCI code
> > there needs to communicate some of the platform power changes down
> > to the qemu code for things like shutdown/reset control.
> 
> No need to explicit 'fake' in patch subject, almost all emulated
> devices are fake.
> 
Noted, will change.

> > 
> > Space has been left to extend the EC if we find other use cases in
> > future where ARM-TF and qemu need to communicate.
> > 
> > Signed-off-by: Graeme Gregory 
> > ---
> >  hw/arm/sbsa-ref.c | 95 +++
> >  1 file changed, 95 insertions(+)
> > 
> > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> > index f030a416fd..c8743fc1d0 100644
> > --- a/hw/arm/sbsa-ref.c
> > +++ b/hw/arm/sbsa-ref.c
> > @@ -41,6 +41,7 @@
> >  #include "hw/usb.h"
> >  #include "hw/char/pl011.h"
> >  #include "net/net.h"
> > +#include "migration/vmstate.h"
> >  
> >  #define RAMLIMIT_GB 8192
> >  #define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
> > @@ -62,6 +63,7 @@ enum {
> >  SBSA_CPUPERIPHS,
> >  SBSA_GIC_DIST,
> >  SBSA_GIC_REDIST,
> > +SBSA_SECURE_EC,
> >  SBSA_SMMU,
> >  SBSA_UART,
> >  SBSA_RTC,
> > @@ -98,6 +100,14 @@ typedef struct {
> >  #define SBSA_MACHINE(obj) \
> >  OBJECT_CHECK(SBSAMachineState, (obj), TYPE_SBSA_MACHINE)
> >  
> > +typedef struct {
> > +SysBusDevice parent_obj;
> > +MemoryRegion iomem;
> > +} SECUREECState;
> > +
> > +#define TYPE_SECURE_EC  "sbsa-secure-ec"
> > +#define SECURE_EC(obj) OBJECT_CHECK(SECUREECState, (obj), TYPE_SECURE_EC)
> > +
> >  static const MemMapEntry sbsa_ref_memmap[] = {
> >  /* 512M boot ROM */
> >  [SBSA_FLASH] =  {  0, 0x2000 },
> > @@ -107,6 +117,7 @@ static const MemMapEntry sbsa_ref_memmap[] = {
> >  [SBSA_CPUPERIPHS] = { 0x4000, 0x0004 },
> >  [SBSA_GIC_DIST] =   { 0x4006, 0x0001 },
> >  [SBSA_GIC_REDIST] = { 0x4008, 0x0400 },
> > +[SBSA_SECURE_EC] =  { 0x5000, 0x1000 },
> >  [SBSA_UART] =   { 0x6000, 0x1000 },
> >  [SBSA_RTC] ={ 0x6001, 0x1000 },
> >  [SBSA_GPIO] =   { 0x6002, 0x1000 },
> > @@ -585,6 +596,65 @@ static void *sbsa_ref_dtb(const struct arm_boot_info 
> > *binfo, int *fdt_size)
> >  return board->fdt;
> >  }
> >  
> > +enum sbsa_secure_ec_powerstates {
> > +SBSA_SECURE_EC_CMD_NULL,
> > +SBSA_SECURE_EC_CMD_POWEROFF,
> > +SBSA_SECURE_EC_CMD_REBOOT,
> > +};
> > +
> > +static uint64_t secure_ec_read(void *opaque, hwaddr offset, unsigned size)
> > +{
> > +/* No use for this currently */
> > +return 0;
> > +}
> > +
> > +static void secure_ec_write(void *opaque, hwaddr offset,
> > + uint64_t value, unsigned size)
> > +{
> > +if (offset == 0) { /* PSCI machine power command register */
> > +switch (value) {
> > +case SBSA_SECURE_EC_CMD_NULL:
> > +break;
> > +case SBSA_SECURE_EC_CMD_POWEROFF:
> > +qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> > +break;
> > +case SBSA_SECURE_EC_CMD_REBOOT:
> > +qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> > +break;
> > +default:
> > +error_report("sbsa-ref: ERROR Unkown power command");
> > +}
> > +} else {
> > +error_report("sbsa-ref: unknown EC register");
> > +}
> > +}
> > +
> > +static const MemoryRegionOps secure_ec_ops = {
> > +.read = secure_ec_read,
> > +.write = secure_ec_write,
> > +.endianness = DEVICE_NATIVE_ENDIAN,
> > +};
> > +
> > +static void secure_ec_init(Object *obj)
> > +{
> > +SECUREECState *s = SECURE_EC(obj);
> > +SysBusDevice *dev = SYS_BUS_DEVICE(obj);
> > +
> > +memory_region_init_io(&s->iomem, obj, &secure_ec_ops, s, "secure-ec",
> > +0x1000);
> > +sysbus_init_mmio(dev, &s->iomem);
> > +}
> > +
> > +static void create_secure_ec(MemoryRegion *mem)
> > +{
> > +hwaddr base = sbsa_ref_memmap[SBSA_SECURE_EC].base;
> > +DeviceState *dev = qdev_create(NULL, TYPE_SECURE_EC);
> 
> This API has been removed in 2194abd6231 ("qdev: qdev_create(),
> qdev_try_create() are now unused, drop"), can you rebase please?
> 
Yes, I accidently sent from a work branch not top of tree, noticed
1ms after sending!

> > +SysBusDevice *s = SYS_BUS_DEVICE(dev);
> > +
> > +memory_region_add_subregion(mem, base,
> > +sysbus_mmio_get_region(s, 0));
> > +}
> > +
> >  static void sbsa_ref_init(MachineState *machine)
> >  {
> >  unsigned int smp_cpus = machine->smp.cpus;
> > @@ -708,6 +778,8 @@ static void sbsa_ref_init(MachineState *machine)
> >  
> >  

Re: [PATCH 1/4] configure: Use discovered make for in-source build

[Roman Bolshakov]

On Tue, Aug 25, 2020 at 01:07:55AM +0300, Roman Bolshakov wrote:
> On Mon, Aug 24, 2020 at 09:37:07AM -0500, Eric Blake wrote:
> > On 8/22/20 4:21 PM, Roman Bolshakov wrote:
> > > @@ -38,6 +38,8 @@ then
> > >   # This file is auto-generated by configure to support in-source tree
> > >   # 'make' command invocation
> > > +include build/config-host.mak
> > 
> > Should this use '-include' (also spelled 'sinclude'), to avoid halting the
> > build if build/config-host.mak doesn't exist for whatever reason?
> > 
> 
> Sure I can do (and thanks for the noticed typos) but I tested that if
> the build is interrupted too early (before Makefile is symlinked to
> build directory but after GNUmakefile is created) it would fail even if
> -include is used:
> 
> $ make
> changing dir to build for /Library/Developer/CommandLineTools/usr/bin/make 
> ""...
> make[1]: Makefile: No such file or directory
> make[1]: *** No rule to make target `Makefile'.  Stop.
> changing dir to build for /Library/Developer/CommandLineTools/usr/bin/make 
> ""...
> make[1]: Makefile: No such file or directory
> make[1]: *** No rule to make target `Makefile'.  Stop.
> make: *** [all] Error 2
> 
> I'm also curious why the switch happens twice... According to the debug
> trace, it tries to remake build/config-host.mak using the implicit force
> rule:
> 
> GNUmakefile:12: update target 'build/config-host.mak' due to: force
> 
> Then there should be an explicit empty rule for build/config-host.mak. I
> will send a fix for that in v2. Then it would fail like this:
> 
> $ make
> changing dir to build for /Library/Developer/CommandLineTools/usr/bin/make 
> ""...
> make[1]: Makefile: No such file or directory
> make[1]: *** No rule to make target `Makefile'.  Stop.
> make: *** [all] Error 2
> 
> > > +
> > >   ifeq ($(MAKECMDGOALS),)
> > >   recurse: all
> > >   endif
> > > 
> > 

Hi Eric,

What if we just print an error if build/config-host.mak can't be found?

ifeq ($(wildcard build/config-host.mak),)
$(error "Incomplete configuration. Please run ./configure")
endif

IMO this is more sane approach than proceeding with partially-configured
not working build without a Makefile in proper place.

Regards,
Roman

Re: [PATCH] meson: Mingw64 gcc doesn't recognize system include_type for sdl2

[Marc-André Lureau]

Hi

On Tue, Aug 25, 2020 at 4:17 AM  wrote:

> From: Yonggang Luo 
>
> ---
>  meson.build | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index df5bf728b5..a3585881e1 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -224,8 +224,7 @@ if 'CONFIG_BRLAPI' in config_host
>brlapi = declare_dependency(link_args:
> config_host['BRLAPI_LIBS'].split())
>  endif
>
> -sdl = dependency('sdl2', required: get_option('sdl'), static:
> enable_static,
> - include_type: 'system')
> +sdl = dependency('sdl2', required: get_option('sdl'), static:
> enable_static)
>  sdl_image = not_found
>  if sdl.found()
># work around 2.0.8 bug
> --
> 2.27.0.windows.1
>
>
>
Which version of gcc are you using?

Paolo, why did you you -isystem includes here? (if it's on purpose, it's
worth to document it in build-system.rst since it takes sdl as an example
and doesn't mention this)


-- 
Marc-André Lureau

Re: [PATCH v2 1/5] qemu-iotests: Fix FilePaths cleanup

[Max Reitz]

On 21.08.20 01:54, Nir Soffer wrote:
> If os.remove() fails to remove one of the paths, for example if the file
> was removed by the test, the cleanup loop would exit silently, without
> removing the rest of the files.
> 
> Fixes: de263986b5dc
> Signed-off-by: Nir Soffer 
> ---
>  tests/qemu-iotests/iotests.py | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [EXTERNAL] Re: [PATCH] hw/arm/sbsa-ref.c : Add a fake embedded controller

[Peter Maydell]

On Tue, 25 Aug 2020 at 11:08, Graeme Gregory  wrote:
>
> On Mon, Aug 24, 2020 at 03:59:38PM +0100, Peter Maydell wrote:
> > > +enum sbsa_secure_ec_powerstates {
> > > +SBSA_SECURE_EC_CMD_NULL,
> > > +SBSA_SECURE_EC_CMD_POWEROFF,
> > > +SBSA_SECURE_EC_CMD_REBOOT,
> >
> > The last two are clear enough, but what's a null power state for?
> >
> My personal dislike of sending 0 to hardware as its harder to spot
> when using scopes/logic analyzers. I can remove it if you wish and
> explicitly number the states.

Yeah, just number the states 1 and 2 rather than having an
unused 0 named state, I think.

PS: when you respin make sure you're on top-of-git -- we just
landed the meson build system conversion, so the way you add
new source files is different (no more Makefile.objs, it goes
in the meson.build file in the relevant directory instead).

thanks
-- PMM

Re: [PATCH 2/2] nbd: disable signals and forking on Windows builds

[Daniel P . Berrangé]

On Mon, Aug 24, 2020 at 12:12:53PM -0500, Eric Blake wrote:
> On 8/24/20 12:02 PM, Daniel P. Berrangé wrote:
> > Disabling these parts are sufficient to get the qemu-nbd program
> > compiling in a Windows build.
> > 
> > Signed-off-by: Daniel P. Berrangé 
> > ---
> >   meson.build |  7 ++-
> >   qemu-nbd.c  | 10 +-
> >   2 files changed, 11 insertions(+), 6 deletions(-)
> 
> Feels a bit hacky at what it supports, but certainly better than nothing ;)
> 
> > 
> > diff --git a/meson.build b/meson.build
> > index df5bf728b5..1071871605 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -1074,12 +1074,9 @@ if have_tools
> >dependencies: [authz, block, crypto, io, qom, qemuutil], 
> > install: true)
> > qemu_io = executable('qemu-io', files('qemu-io.c'),
> >dependencies: [block, qemuutil], install: true)
> > -  qemu_block_tools += [qemu_img, qemu_io]
> > -  if targetos == 'linux' or targetos == 'sunos' or targetos.endswith('bsd')
> > -qemu_nbd = executable('qemu-nbd', files('qemu-nbd.c'),
> > +  qemu_nbd = executable('qemu-nbd', files('qemu-nbd.c'),
> >  dependencies: [block, qemuutil], install: true)
> 
> Conflicts with this patch:
> https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg05546.html
> 
> but this one gets rid of the need for that one.
> 
> > -qemu_block_tools += [qemu_nbd]
> > -  endif
> > +  qemu_block_tools += [qemu_img, qemu_io, qemu_nbd]
> > subdir('storage-daemon')
> > subdir('contrib/rdmacm-mux')
> > diff --git a/qemu-nbd.c b/qemu-nbd.c
> > index b102874f0f..c6fd6524d3 100644
> > --- a/qemu-nbd.c
> > +++ b/qemu-nbd.c
> > @@ -155,12 +155,13 @@ QEMU_COPYRIGHT "\n"
> >   , name);
> >   }
> > +#ifndef WIN32
> >   static void termsig_handler(int signum)
> >   {
> >   atomic_cmpxchg(&state, RUNNING, TERMINATE);
> >   qemu_notify_event();
> >   }
> > -
> > +#endif
> 
> How does one terminate a long-running server on Windows if there is no
> SIGTERM handler?  I guess Ctrl-C does something, but without the state
> notification from a signal handler, you are getting less-clean shutdowns,
> which may explain the hangs you were seeing in testing?  But incremental
> progress is fine, and I see no reason to not take this patch as-is.

The hangs occurred even with windows client/ native server, just less
frequently so don't think it is related.

Re-reading the code I notice this SIGTERM handler is only there for
the NBD device client thread, so we should skip it if that is not
installed.

Ctrl-C does SIGINT, so that's unrelated to the SIGTERM handler.

> 
> Reviewed-by: Eric Blake 
> 
> I'm happy to queue this series through my NBD tree.

I'll post a v2

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

[PATCH v2 0/3] nbd: build qemu-nbd on Windows

[Daniel P . Berrangé]

We are already building the NBD client and server on Windows when it is
used via the main system emulator binaries. This demonstrates there is
no fundamental blocker to buildig the qemu-nbd binary too.

Changed in v2:

 - Split second patch into two parts
 - Use  HAVE_NBD_DEVICE condition to disable SIGTERM handler not WIN32

Daniel P. Berrangé (3):
  block: add missing socket_init() calls to tools
  nbd: skip SIGTERM handler if NBD device support is not built
  nbd: disable signals and forking on Windows builds

 meson.build |  7 ++-
 qemu-img.c  |  2 ++
 qemu-io.c   |  2 ++
 qemu-nbd.c  | 11 ++-
 4 files changed, 16 insertions(+), 6 deletions(-)

-- 
2.26.2

[PATCH v2 2/3] nbd: skip SIGTERM handler if NBD device support is not built

[Daniel P . Berrangé]

The termsig_handler function is used by the client thread handling the
host NBD device connection to do a graceful shutdown. IOW, if we have
disabled NBD device support at compile time, we don't need the SIGTERM
handler. This fixes a build issue for Windows.

Signed-off-by: Daniel P. Berrangé 
---
 qemu-nbd.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index b102874f0f..dc6ef089af 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -155,12 +155,13 @@ QEMU_COPYRIGHT "\n"
 , name);
 }
 
+#if HAVE_NBD_DEVICE
 static void termsig_handler(int signum)
 {
 atomic_cmpxchg(&state, RUNNING, TERMINATE);
 qemu_notify_event();
 }
-
+#endif /* HAVE_NBD_DEVICE */
 
 static int qemu_nbd_client_list(SocketAddress *saddr, QCryptoTLSCreds *tls,
 const char *hostname)
@@ -587,6 +588,7 @@ int main(int argc, char **argv)
 unsigned socket_activation;
 const char *pid_file_name = NULL;
 
+#if HAVE_NBD_DEVICE
 /* The client thread uses SIGTERM to interrupt the server.  A signal
  * handler ensures that "qemu-nbd -v -c" exits with a nice status code.
  */
@@ -594,6 +596,7 @@ int main(int argc, char **argv)
 memset(&sa_sigterm, 0, sizeof(sa_sigterm));
 sa_sigterm.sa_handler = termsig_handler;
 sigaction(SIGTERM, &sa_sigterm, NULL);
+#endif /* HAVE_NBD_DEVICE */
 
 #ifdef CONFIG_POSIX
 signal(SIGPIPE, SIG_IGN);
-- 
2.26.2

[PATCH v2 3/3] nbd: disable signals and forking on Windows builds

[Daniel P . Berrangé]

Disabling these parts are sufficient to get the qemu-nbd program
compiling in a Windows build.

Signed-off-by: Daniel P. Berrangé 
---
 meson.build | 7 ++-
 qemu-nbd.c  | 5 +
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/meson.build b/meson.build
index df5bf728b5..1071871605 100644
--- a/meson.build
+++ b/meson.build
@@ -1074,12 +1074,9 @@ if have_tools
  dependencies: [authz, block, crypto, io, qom, qemuutil], install: 
true)
   qemu_io = executable('qemu-io', files('qemu-io.c'),
  dependencies: [block, qemuutil], install: true)
-  qemu_block_tools += [qemu_img, qemu_io]
-  if targetos == 'linux' or targetos == 'sunos' or targetos.endswith('bsd')
-qemu_nbd = executable('qemu-nbd', files('qemu-nbd.c'),
+  qemu_nbd = executable('qemu-nbd', files('qemu-nbd.c'),
dependencies: [block, qemuutil], install: true)
-qemu_block_tools += [qemu_nbd]
-  endif
+  qemu_block_tools += [qemu_img, qemu_io, qemu_nbd]
 
   subdir('storage-daemon')
   subdir('contrib/rdmacm-mux')
diff --git a/qemu-nbd.c b/qemu-nbd.c
index dc6ef089af..33476a1000 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -899,6 +899,7 @@ int main(int argc, char **argv)
 #endif
 
 if ((device && !verbose) || fork_process) {
+#ifndef WIN32
 int stderr_fd[2];
 pid_t pid;
 int ret;
@@ -962,6 +963,10 @@ int main(int argc, char **argv)
  */
 exit(errors);
 }
+#else /* WIN32 */
+error_report("Unable to fork into background on Windows hosts");
+exit(EXIT_FAILURE);
+#endif /* WIN32 */
 }
 
 if (device != NULL && sockpath == NULL) {
-- 
2.26.2

[PATCH v2 1/3] block: add missing socket_init() calls to tools

[Daniel P . Berrangé]

Any tool that uses sockets needs to call socket_init() in order to work
on the Windows platform.

Reviewed-by: Eric Blake 
Signed-off-by: Daniel P. Berrangé 
---
 qemu-img.c | 2 ++
 qemu-io.c  | 2 ++
 qemu-nbd.c | 1 +
 3 files changed, 5 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c
index 5308773811..eb2fc1f862 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -41,6 +41,7 @@
 #include "qemu/log.h"
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
+#include "qemu/sockets.h"
 #include "qemu/units.h"
 #include "qom/object_interfaces.h"
 #include "sysemu/block-backend.h"
@@ -5410,6 +5411,7 @@ int main(int argc, char **argv)
 signal(SIGPIPE, SIG_IGN);
 #endif
 
+socket_init();
 error_init(argv[0]);
 module_call_init(MODULE_INIT_TRACE);
 qemu_init_exec_dir(argv[0]);
diff --git a/qemu-io.c b/qemu-io.c
index 3adc5a7d0d..7cc832b3d6 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -25,6 +25,7 @@
 #include "qemu/config-file.h"
 #include "qemu/readline.h"
 #include "qemu/log.h"
+#include "qemu/sockets.h"
 #include "qapi/qmp/qstring.h"
 #include "qapi/qmp/qdict.h"
 #include "qom/object_interfaces.h"
@@ -542,6 +543,7 @@ int main(int argc, char **argv)
 signal(SIGPIPE, SIG_IGN);
 #endif
 
+socket_init();
 error_init(argv[0]);
 module_call_init(MODULE_INIT_TRACE);
 qemu_init_exec_dir(argv[0]);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index d2657b8db5..b102874f0f 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -599,6 +599,7 @@ int main(int argc, char **argv)
 signal(SIGPIPE, SIG_IGN);
 #endif
 
+socket_init();
 error_init(argv[0]);
 module_call_init(MODULE_INIT_TRACE);
 qcrypto_init(&error_fatal);
-- 
2.26.2

Re: [PATCH v2 2/5] qemu-iotests: Fix FilePaths docstring

[Max Reitz]

On 21.08.20 01:54, Nir Soffer wrote:
> When this class was extracted from FilePath, the docstring was not
> updated for generating multiple files, and the example usage was
> referencing unrelated file.
> 
> Fixes: de263986b5dc
> Signed-off-by: Nir Soffer 
> ---
>  tests/qemu-iotests/iotests.py | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2 2/5] qemu-iotests: Fix FilePaths docstring

[Max Reitz]

On 21.08.20 01:54, Nir Soffer wrote:
> When this class was extracted from FilePath, the docstring was not
> updated for generating multiple files, and the example usage was
> referencing unrelated file.
> 
> Fixes: de263986b5dc
> Signed-off-by: Nir Soffer 
> ---
>  tests/qemu-iotests/iotests.py | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 16a04df8a3..f34a1d7ef1 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -450,14 +450,16 @@ def file_pattern(name):
>  
>  class FilePaths:
>  """
> -FilePaths is an auto-generated filename that cleans itself up.
> +Context manager generating multiple file names. The generated files are
> +removed when exiting the context.
>  
> -Use this context manager to generate filenames and ensure that the file
> -gets deleted::
> +Example usage:
> +
> +with FilePaths(['test.img', 'test.sock']) as (img_path, sock_path):

On second thought, this isn’t a great example because image files and
sockets should go into different base directories.

Max

> +# Use img_path and sock_path here...
> +
> +# img_path and sock_path are automatically removed here.
>  
> -with FilePaths(['test.img']) as img_path:
> -qemu_img('create', img_path, '1G')
> -# migration_sock_path is automatically deleted
>  """
>  def __init__(self, names, base_dir=test_dir):
>  self.paths = []
> 




signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2 3/5] qemu-iotests: Support varargs syntax in FilePaths

[Max Reitz]

On 21.08.20 01:54, Nir Soffer wrote:
> Accept variable number of names instead of a sequence:
> 
> with FilePaths("a", "b", "c") as (a, b, c):
> 
> The disadvantage is that base_dir must be used as kwarg:
> 
> with FilePaths("a", "b", base_dir=soc_dir) as (sock1, sock2):
> 
> But this is more clear and calling optional argument as positional
> arguments is bad idea anyway.
> 
> Signed-off-by: Nir Soffer 
> ---
>  tests/qemu-iotests/194|  4 ++--
>  tests/qemu-iotests/257| 10 --
>  tests/qemu-iotests/iotests.py |  6 +++---
>  3 files changed, 9 insertions(+), 11 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

[PULL 1/1] tests: fix a memory in test_socket_unix_abstract_good

[Daniel P . Berrangé]

From: Li Qiang 

After build qemu with '-fsanitize=address' extra-cflags,
'make check' show following leak:

=
==44580==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 2500 byte(s) in 1 object(s) allocated from:
#0 0x7f1b5a8b8d28 in __interceptor_calloc 
(/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
#1 0x7f1b5a514b10 in g_malloc0 
(/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x51b10)
#2 0xd79ea4e4c0ad31c3  ()

SUMMARY: AddressSanitizer: 2500 byte(s) leaked in 1 allocation(s).

Call 'g_rand_free' in the end of function to avoid this.

Fixes: 4d3a329af59("tests/util-sockets: add abstract unix socket cases")
Signed-off-by: Li Qiang 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by:  xiaoqiang zhao 
Signed-off-by: Daniel P. Berrangé 
---
 tests/test-util-sockets.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c
index 261dc48c03..af9f5c0c70 100644
--- a/tests/test-util-sockets.c
+++ b/tests/test-util-sockets.c
@@ -313,6 +313,7 @@ static void test_socket_unix_abstract_good(void)
 g_thread_join(serv);
 
 g_free(abstract_sock_name);
+g_rand_free(r);
 }
 #endif
 
-- 
2.26.2

[PULL 0/1] Socket next patches

[Daniel P . Berrangé]

The following changes since commit 44423107e7b5731ef40c5c8632a5bad8b49d0838:

  Merge remote-tracking branch 'remotes/xtensa/tags/20200821-xtensa' into 
staging (2020-08-24 19:55:23 +0100)

are available in the Git repository at:

  https://github.com/berrange/qemu tags/socket-next-pull-request

for you to fetch changes up to 74a57ddc02c41f8f4bb549cedb107c1086daba58:

  tests: fix a memory in test_socket_unix_abstract_good (2020-08-25 11:49:49 
+0100)


Add support for UNIX sockets in the abstract namespace



Li Qiang (1):
  tests: fix a memory in test_socket_unix_abstract_good

 tests/test-util-sockets.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.26.2

Re: [PATCH v2 4/5] qemu-iotests: Merge FilePaths and FilePath

[Max Reitz]

On 21.08.20 01:54, Nir Soffer wrote:
> FilePath creates now one temporary file:
> 
> with FilePath("a") as a:
> 
> Or more:
> 
> with FilePath("a", "b", "c") as (a, b, c):
> 
> This is also the behavior of the file_path() helper, used by some of the
> tests. Now we have only 2 helpers for creating temporary files instead
> of 3.
> 
> Signed-off-by: Nir Soffer 
> ---
>  tests/qemu-iotests/194|  2 +-
>  tests/qemu-iotests/208|  2 +-
>  tests/qemu-iotests/222|  2 +-
>  tests/qemu-iotests/257|  4 ++--
>  tests/qemu-iotests/iotests.py | 23 +++
>  5 files changed, 16 insertions(+), 17 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [PATCH 1/1] spapr_vscsi: do not allow device hotplug

[David Gibson]

On Tue, Aug 25, 2020 at 08:27:28AM +0200, Markus Armbruster wrote:
> Philippe Mathieu-Daudé  writes:
> 
> > Cc'ing Markus
> >
> > On 8/20/20 9:06 PM, Daniel Henrique Barboza wrote:
> >> We do not implement hotplug in the vscsi bus, but we forgot to
> >> tell qdev about it. The result is that users are able to hotplug
> >> devices in the vscsi bus, the devices appear in qdev, but they
> >> aren't usable by the guest OS unless the user reboots it first.
> >> 
> >> Setting qbus hotplug_handler to NULL will tell qdev-monitor, via
> >> qbus_is_hotpluggable(), that we do not support hotplug operations
> >> in spapr_vscsi.
> >> 
> >> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1862059
> >> 
> >> Signed-off-by: Daniel Henrique Barboza 
> >> ---
> >>  hw/scsi/spapr_vscsi.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >> 
> >> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
> >> index d17dc03c73..57f0a1336f 100644
> >> --- a/hw/scsi/spapr_vscsi.c
> >> +++ b/hw/scsi/spapr_vscsi.c
> >> @@ -1219,6 +1219,9 @@ static void spapr_vscsi_realize(SpaprVioDevice *dev, 
> >> Error **errp)
> >>  
> >>  scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(dev),
> >>   &vscsi_scsi_info, NULL);
> >> +
> >> +/* ibmvscsi SCSI bus does not allow hotplug. */
> >> +qbus_set_hotplug_handler(BUS(&s->bus), NULL);
> >
> > Can't this be a problem later in DeviceClass::unrealize()?
> 
> Can't say offhand.  Cc'ing QOM maintainers for actual expertise.
> 
> A quick grep coughs up virtio_serial_device_realize() /
> virtio_serial_device_unrealize(), which set and unset the hotplug
> handler of a bus their device provides.  Not the same as the code above,

Hm, it almost is, actually.  Only difference is that virtio serial is
setting the hotplug handler to something, rather than disabling it.

> just a clue that messing with a bus's hotplug handler in a realize
> method can work.
> 
> qbus_set_hotplug_handler() is entirely undocumented.  It sets a link.
> Might be intended for use by the device that provides the bus.

Heh.

> > I was expecting something like, overwriting the parent bus type:
> >
> > -- >8 --
> > @@ -1271,6 +1271,7 @@ static void spapr_vscsi_class_init(ObjectClass
> > *klass, void *data)
> >  DeviceClass *dc = DEVICE_CLASS(klass);
> >  SpaprVioDeviceClass *k = VIO_SPAPR_DEVICE_CLASS(klass);
> >
> > +k->bus_type = NULL; /* ibmvscsi SCSI bus does not allow hotplug. */
> >  k->realize = spapr_vscsi_realize;
> >  k->reset = spapr_vscsi_reset;
> >  k->devnode = spapr_vscsi_devnode;
> 
> k->bus_type does not exist.  Do you mean dc->bus_type?

Yeah, I'm pretty sure changing the bus type is going to be worse than
setting the hotplug handler.

> TYPE_VIO_SPAPR_VSCSI_DEVICE is a subtype of TYPE_VIO_SPAPR_DEVICE is a
> subtype of TYPE_DEVICE.
> 
> TYPE_DEVICE is bus-less, i.e. (concrete) instances do not plug into a
> bus.
> 
> TYPE_VIO_SPAPR_DEVICE has bus_type TYPE_SPAPR_VIO_BUS, i.e. (concrete)
> instances plug into a TYPE_SPAPR_VIO_BUS.  Both hot and cold plug.
> 
> Zapping TYPE_VIO_SPAPR_VSCSI_DEVICE's bus_type makes it bus-less.  I
> doubt that's okay.  Also, it's not just about hot plug.  What are you
> trying to accomplish?

I'm pretty sure all of the above is not relevant.  It's the
subordinate bus of TYPE_VIO_SPAPR_VSCSI_DEVICE on which we want to
disable hotplugging, not the parent bus.  We can't hotplug spapr-vscsi
controllers, but that's fine.  What we're trying to address here is
that the controller supports a TYPE_SCSI_BUS, onto which disks can
normally be hotplugged, but not in this case.

-- 
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: [PATCH v2 5/5] qemu-iotests: Simplify FilePath __init__

[Max Reitz]

On 21.08.20 01:54, Nir Soffer wrote:
> Use list comprehension instead of append loop.
> 
> Signed-off-by: Nir Soffer 
> ---
>  tests/qemu-iotests/iotests.py | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v7 3/4] qapi: add filter-node-name to block-stream

[Andrey Shinkevich]

On 25.08.2020 09:37, Markus Armbruster wrote:

Andrey Shinkevich  writes:


Provide the possibility to pass the 'filter-node-name' parameter to the
block-stream job as it is done for the commit block job.

Signed-off-by: Andrey Shinkevich 
Reviewed-by: Vladimir Sementsov-Ogievskiy 

[...]

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0b8ccd3..1db6ce1 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2524,6 +2524,11 @@
  #'stop' and 'enospc' can only be used if the block device
  #supports io-status (see BlockInfo).  Since 1.3.
  #
+# @filter-node-name: the node name that should be assigned to the
+#filter driver that the stream job inserts into the graph
+#above @device. If this option is not given, a node name is
+#autogenerated. (Since: 5.1)

It's (Since: 5.2) now.



Well noted, Markus, thank you.

Andrey





+#
  # @auto-finalize: When false, this job will wait in a PENDING state after it 
has
  # finished its work, waiting for @block-job-finalize before
  # making any block graph changes.
@@ -2554,6 +2559,7 @@
'data': { '*job-id': 'str', 'device': 'str', '*base': 'str',
  '*base-node': 'str', '*backing-file': 'str', '*speed': 'int',
  '*on-error': 'BlockdevOnError',
+'*filter-node-name': 'str',
  '*auto-finalize': 'bool', '*auto-dismiss': 'bool' } }
  
  ##

Re: [PATCH v2 04/58] pl110: Rename pl110_version enum values

[Daniel P . Berrangé]

On Wed, Aug 19, 2020 at 08:11:42PM -0400, Eduardo Habkost wrote:
> The PL110 enum value name will conflict with the PL110 type cast
> checker, when we replace the existing macro with an inline
> function.  Add a VERSION_ prefix to all pl110_version enum
> values, to avoid conflicts.
> 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v1 -> v2:
> * Fixed typo on commit message
> * Rename all enum values to VERSION_* (Philippe Mathieu-Daudé)

Reviewed-by: Daniel P. Berrangé 


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

Re: [PATCH v2 05/58] allwinner-h3: Rename memmap enum constants

[Daniel P . Berrangé]

On Wed, Aug 19, 2020 at 08:11:43PM -0400, Eduardo Habkost wrote:
> Some of the enum constant names conflict with the QOM type check
> macros (AW_H3_CCU, AW_H3_SYSCTRL).  This needs to be addressed to
> allow us to transform the QOM type check macros into functions
> generated by OBJECT_DECLARE_TYPE().
> 
> Rename all the constants to AW_H3_DEV_*, to avoid conflicts.
> 
> Reviewed-by: Daniel P. Berrangé 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v1 -> v2:
> * Added more details to commit message
> 
> ---
> Cc: Beniamino Galvani 
> Cc: Peter Maydell 
> Cc: Niek Linnenbank 
> Cc: qemu-...@nongnu.org
> Cc: qemu-devel@nongnu.org
> ---
>  include/hw/arm/allwinner-h3.h |  62 -
>  hw/arm/allwinner-h3.c | 124 +-
>  hw/arm/orangepi.c |   6 +-
>  3 files changed, 96 insertions(+), 96 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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

Re: [PATCH v2 06/58] aspeed_soc: Rename memmap/irqmap enum constants

[Daniel P . Berrangé]

On Wed, Aug 19, 2020 at 08:11:44PM -0400, Eduardo Habkost wrote:
> Some of the enum constant names conflict with the QOM type check
> macros:
> 
> ASPEED_GPIO
> ASPEED_I2C
> ASPEED_RTC
> ASPEED_SCU
> ASPEED_SDHCI
> ASPEED_SDMC
> ASPEED_VIC
> ASPEED_WDT
> ASPEED_XDMA
> 
> This needs to be addressed to allow us to transform the QOM type
> check macros into functions generated by OBJECT_DECLARE_TYPE().
> 
> Rename all the constants to ASPEED_DEV_*, to avoid conflicts.
> 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v1 -> v2:
> * Added more details to commit message
> 
> ---
> Cc: "Cédric Le Goater" 
> Cc: Peter Maydell 
> Cc: Andrew Jeffery 
> Cc: Joel Stanley 
> Cc: qemu-...@nongnu.org
> Cc: qemu-devel@nongnu.org
> ---
>  include/hw/arm/aspeed_soc.h |  92 +++
>  hw/arm/aspeed.c |   4 +-
>  hw/arm/aspeed_ast2600.c | 208 
>  hw/arm/aspeed_soc.c | 228 ++--
>  4 files changed, 266 insertions(+), 266 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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

Re: [PATCH v2 07/58] opentitan: Rename memmap enum constants

[Daniel P . Berrangé]

On Wed, Aug 19, 2020 at 08:11:45PM -0400, Eduardo Habkost wrote:
> Some of the enum constant names conflict with the QOM type check
> macros (IBEX_PLIC, IBEX_UART).  This needs to be addressed to
> allow us to transform the QOM type check macros into functions
> generated by OBJECT_DECLARE_TYPE().
> 
> Rename all the constants to IBEX_DEV_*, to avoid conflicts.
> 
> Reviewed-by: Alistair Francis 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v1 -> v2:
> * Added more details to commit message
> 
> ---
> Cc: Alistair Francis 
> Cc: Palmer Dabbelt 
> Cc: Sagar Karandikar 
> Cc: Bastian Koppelmann 
> Cc: qemu-ri...@nongnu.org
> Cc: qemu-devel@nongnu.org
> ---
>  include/hw/riscv/opentitan.h | 38 
>  hw/riscv/opentitan.c | 84 ++--
>  2 files changed, 61 insertions(+), 61 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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

Re: [PATCH v2 08/58] sifive_e: Rename memmap enum constants

[Daniel P . Berrangé]

On Wed, Aug 19, 2020 at 08:11:46PM -0400, Eduardo Habkost wrote:
> Some of the enum constant names conflict with a QOM type check
> macro (SIFIVE_E_PRCI).  This needs to be addressed to allow us to
> transform the QOM type check macros into functions generated by
> OBJECT_DECLARE_TYPE().
> 
> Rename all the constants to SIFIVE_E_DEV_*, to avoid conflicts.
> 
> Reviewed-by: Alistair Francis 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v1 -> v2:
> * Added more details to commit message
> 
> ---
> Cc: Palmer Dabbelt 
> Cc: Alistair Francis 
> Cc: Sagar Karandikar 
> Cc: Bastian Koppelmann 
> Cc: qemu-ri...@nongnu.org
> Cc: qemu-devel@nongnu.org
> ---
>  include/hw/riscv/sifive_e.h | 38 -
>  hw/riscv/sifive_e.c | 82 ++---
>  2 files changed, 60 insertions(+), 60 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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

Re: [PATCH v2 09/58] sifive_u: Rename memmap enum constants

[Daniel P . Berrangé]

On Wed, Aug 19, 2020 at 08:11:47PM -0400, Eduardo Habkost wrote:
> Some of the enum constant names conflict with the QOM type check
> macros (SIFIVE_U_OTP, SIFIVE_U_PRCI).  This needs to be addressed
> to allow us to transform the QOM type check macros into functions
> generated by OBJECT_DECLARE_TYPE().
> 
> Rename all the constants to SIFIVE_U_DEV_*, to avoid conflicts.
> 
> Reviewed-by: Alistair Francis 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v1 -> v2:
> * Added more details to commit message
> 
> ---
> Cc: Palmer Dabbelt 
> Cc: Alistair Francis 
> Cc: Sagar Karandikar 
> Cc: Bastian Koppelmann 
> Cc: qemu-ri...@nongnu.org
> Cc: qemu-devel@nongnu.org
> ---
>  include/hw/riscv/sifive_u.h |  30 
>  hw/riscv/sifive_u.c | 136 ++--
>  2 files changed, 83 insertions(+), 83 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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