Patch 6a7e2bbee5 docs: add virtiofsd(1) man page introduced new man
page virtiofsd.1. Unfortunately, wrong file location is used as
source for install command. This cause installation of docs fail.
Fixing wrong location so installation is successful.
Fix warning reported by Clang static code analyzer:
CC tools/virtiofsd/passthrough_ll.o
tools/virtiofsd/passthrough_ll.c:925:9: warning: Value stored to 'newfd' is never read
newfd = -1;
^ ~~
tools/virtiofsd/passthrough_ll.c:942:9: warning: Value stored to 'newfd' is never read
newfd = -1;
^ ~~
Fixes: 7c6b66027 Reported-by: Clang Static Analyzer Signed-off-by: Philippe Mathieu-Daudé <[email protected]> Reviewed-by: Ján Tomko <[email protected]> Signed-off-by: Dr. David Alan Gilbert <[email protected]>
Peter Maydell [Thu, 20 Feb 2020 17:35:42 +0000 (17:35 +0000)]
Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-5.0-pull-request' into staging
Implement membarrier, SO_RCVTIMEO and SO_SNDTIMEO
Disable by default build of fdt, slirp and tools with linux-user
Improve strace and use qemu_log to send trace to a file
Add partial ALSA ioctl supports
* remotes/vivier2/tags/linux-user-for-5.0-pull-request:
linux-user: Add support for selected alsa timer instructions using ioctls
linux-user: Add support for getting/setting selected alsa timer parameters using ioctls
linux-user: Add support for selecting alsa timer using ioctl
linux-user: Add support for getting/setting specified alsa timer parameters using ioctls
linux-user: Add support for getting alsa timer version and id
linux-user: remove gemu_log from the linux-user tree
linux-user: Use `qemu_log' for strace
linux-user: Use `qemu_log' for non-strace logging
configure: Avoid compiling system tools on user build by default
linux-user/strace: Improve output of various syscalls
configure: linux-user doesn't need neither fdt nor slirp
linux-user: implement getsockopt SO_RCVTIMEO and SO_SNDTIMEO
linux-user: Implement membarrier syscall
Peter Maydell [Thu, 20 Feb 2020 14:04:16 +0000 (14:04 +0000)]
Merge remote-tracking branch 'remotes/rth/tags/pull-pa-20200218' into staging
Fixes for Dino and Artist.
# gpg: Signature made Tue 18 Feb 2020 19:35:09 GMT
# gpg: using RSA key 7A481E78868B4DB6A85A05C064DF38E8AF7E215F
# gpg: issuer "[email protected]"
# gpg: Good signature from "Richard Henderson <[email protected]>" [full]
# Primary key fingerprint: 7A48 1E78 868B 4DB6 A85A 05C0 64DF 38E8 AF7E 215F
* remotes/rth/tags/pull-pa-20200218:
hw/hppa/dino: Do not accept accesses to registers 0x818 and 0x82c
hw/hppa/dino: Fix bitmask for the PCIROR register
hw/hppa/dino: Fix reg800_keep_bits overrun (CID 141938714193931419394)
hw/hppa/dino: Add comments with register name
hw/display/artist: Remove dead code (CID 1419388 & 1419389)
hw/display/artist: Avoid drawing line when nothing to display
hw/display/artist: Delay some variables initialization
hw/display/artist: Remove pointless initialization
hw/display/artist: Move trace event to draw_line()
Starts the timer device that is selected. The third ioctl's argument is
ignored. Before calling this ioctl, the ioctl "SNDRV_TIMER_IOCTL_SELECT"
should be called first to select the timer that is to be started. If no
timer is selected, the error EBADFD ("File descriptor in bad shape")
is returned.
SNDRV_TIMER_IOCTL_STOP - Stop selected alsa timer
Stops the timer device that is selected. The third ioctl's argument is
ignored. Before calling this ioctl, the ioctl "SNDRV_TIMER_IOCTL_SELECT"
should be called first to select the timer that is to be stopped. If no
timer is selected, the error EBADFD ("File descriptor in bad shape")
is returned.
Continues the timer device that is selected. The third ioctl's argument is
ignored. Before calling this ioctl, the ioctl "SNDRV_TIMER_IOCTL_SELECT"
should be called first to select the timer that is to be continued. If no
timer is selected, the error EBADFD ("File descriptor in bad shape")
is returned.
Pauses the timer device that is selected. The third ioctl's argument is
ignored. Before calling this ioctl, the ioctl "SNDRV_TIMER_IOCTL_SELECT"
should be called first to select the timer that is to be paused. If no
timer is selected, the error EBADFD ("File descriptor in bad shape")
is returned.
Implementation notes:
Since all of the implemented ioctls have NULL as their third argument,
their implementation was straightforward.
Filip Bozuta [Wed, 15 Jan 2020 19:36:45 +0000 (20:36 +0100)]
linux-user: Add support for getting/setting selected alsa timer parameters using ioctls
This patch implements functionalities of following ioctls:
SNDRV_TIMER_IOCTL_INFO - Getting information about selected timer
Read information about the selected timer. The information is returned in
the following structure:
struct snd_timer_info {
unsigned int flags; /* timer flags - SNDRV_TIMER_FLG_* */
int card; /* card number */
unsigned char id[64]; /* timer identificator */
unsigned char name[80]; /* timer name */
unsigned long reserved0; /* reserved for future use */
unsigned long resolution; /* average period resolution in ns */
unsigned char reserved[64]; /* reserved for future use */
};
A pointer to this structure should be passed as the third ioctl's argument.
Before calling this ioctl, the ioctl "SNDRV_TIMER_IOCTL_SELECT" should be
called first to select the timer which information is to be obtained. If no
timer is selected, the error EBADFD ("File descriptor in bad shape") is
returned.
SNDRV_TIMER_IOCTL_PARAMS - Setting parameters for selected timer
Sets parameters for the selected timer. The paramaters are set in the
following structure:
struct snd_timer_params {
unsigned int flags; /* flags - SNDRV_TIMER_PSFLG_* */
unsigned int ticks; /* requested resolution in ticks */
unsigned int queue_size; /* total size of queue (32-1024) */
unsigned int reserved0; /* reserved, was: failure locations */
unsigned int filter; /* event filter */
unsigned char reserved[60]; /* reserved */
};
A pointer to this structure should be passed as the third ioctl's argument.
Before calling this ioctl, the ioctl "SNDRV_TIMER_IOCTL_SELECT" should be
called first to select the timer which parameters are to be set. If no
timer is selected, the error EBADFD ("File descriptor in bad shape") is
returned.
SNDRV_TIMER_IOCTL_STATUS - Getting status of selected timer
Read status of the selected timer. The status of the timer is returned in
the following structure:
struct snd_timer_status {
struct timespec tstamp; /* Timestamp - last update */
unsigned int resolution; /* current period resolution in ns */
unsigned int lost; /* counter of master tick lost */
unsigned int overrun; /* count of read queue overruns */
unsigned int queue; /* used queue size */
unsigned char reserved[64]; /* reserved */
};
A pointer to this structure should be passed as the third ioctl's argument.
Before calling this ioctl, the ioctl "SNDRV_TIMER_IOCTL_SELECT" should be
called first to select the timer which status is to be obtained. If no
timer is selected, the error EBADFD ("File descriptor in bad shape") is
returned.
Implementation notes:
All ioctls in this patch have pointer to some kind of a structure
as their third argument. That is the reason why corresponding
definitions were added in 'linux-user/syscall_types.h'. Structure
'snd_timer_status' has field of type 'struct timespec' which is why
a corresponding definition of that structure was also added in
'linux-user/syscall_types.h'. All of these strucutures have some
fields that are of type 'unsigned long'. That is the reason why
separate target structures were defined in 'linux-user/syscall_defs.h'.
Structure 'struct timespec' already had a separate target definition
so that definition was used to define a target structure for
'snd_timer_status'. The rest of the implementation was straightforward.
A pointer to this structure should be passed as the third ioctl's argument.
Before calling the ioctl, the field "tid" should be initialized with the id
information for the timer which is to be selected. If there is no timer
device with the specified id, the error ENODEV ("No such device") is
returned.
Implementation notes:
Ioctl implemented in this patch has a pointer to a
'struct snd_timer_select' as its third argument.
That is the reason why a corresponding definition
was added in 'linux-user/syscall_types.h'. The rest
of the implementation was straightforward.
Filip Bozuta [Wed, 15 Jan 2020 19:36:43 +0000 (20:36 +0100)]
linux-user: Add support for getting/setting specified alsa timer parameters using ioctls
This patch implements functionalities of following ioctls:
SNDRV_TIMER_IOCTL_GINFO - Getting information about specified timer
Read information about the specified timer. The information about the
timer is returned in the following structure:
struct snd_timer_ginfo {
struct snd_timer_id tid; /* requested timer ID */
unsigned int flags; /* timer flags - SNDRV_TIMER_FLG_* */
int card; /* card number */
unsigned char id[64]; /* timer identification */
unsigned char name[80]; /* timer name */
unsigned long reserved0; /* reserved for future use */
unsigned long resolution; /* average period resolution in ns */
unsigned long resolution_min; /* minimal period resolution in ns */
unsigned long resolution_max; /* maximal period resolution in ns */
unsigned int clients; /* active timer clients */
unsigned char reserved[32]; /* reserved */
};
A pointer to this structure should be passed as the third ioctl's argument.
Before calling the ioctl, the field "tid" should be initialized with the id
information for the timer which information is to be obtained. After the
ioctl call, the rest of the structure fields are filled with values from
the timer device with the specified id. If there is no device with the
specified id, the error ENODEV ("No such device") is returned.
SNDRV_TIMER_IOCTL_GPARAMS - Setting precise period duration
Sets timer precise period duration numerator and denominator in seconds. The
period duration is set in the following structure:
struct snd_timer_gparams {
struct snd_timer_id tid; /* requested timer ID */
unsigned long period_num; /* period duration - numerator */
unsigned long period_den; /* period duration - denominator */
unsigned char reserved[32]; /* reserved */
};
A pointer to this structure should be passed as the third ioctl's argument.
Before calling the ioctl, the field "tid" should be initialized with the id
information for the timer which period duration is to be set. Also, the
fileds "period_num" and "period_den" should be filled with the period
duration numerator and denominator values that are to be set respectively.
If there is no device with the specified id, the error ENODEV ("No such
device") is returned.
SNDRV_TIMER_IOCTL_GSTATUS - Getting current period resolution
Read timer current period resolution in nanoseconds and period resolution
numerator and denominator in seconds. The period resolution information is
returned in the following structure:
struct snd_timer_gstatus {
struct snd_timer_id tid; /* requested timer ID */
unsigned long resolution; /* current period resolution in ns */
unsigned long resolution_num; /* period resolution - numerator */
unsigned long resolution_den; /* period resolution - denominator */
unsigned char reserved[32]; /* reserved for future use */
};
A pointer to this structure should be passed as the third ioctl's argument.
Before calling the ioctl, the field "tid" should be initialized with the id
information for the timer which period resolution is to be obtained. After
the ioctl call, the rest of the structure fields are filled with values
from the timer device with the specified id. If there is no device with the
specified id, the error ENODEV ("No such device") is returned.
Implementation notes:
All ioctls in this patch have pointer to some kind of a structure as their
third argument. That is the reason why corresponding definitions were added
in 'linux-user/syscall_types.h'. All of these strcutures have some fields
that are of type 'unsigned long'. That is the reason why separate target
structures were defined in 'linux-user/syscall_defs.h'. Also, all of the
structures have a field with type 'struct snd_timer_id' which is the reason
why a separate target structure 'struct target_snd_timer_id' was also
defined. The rest of the implementation was straightforward.
Filip Bozuta [Wed, 15 Jan 2020 19:36:41 +0000 (20:36 +0100)]
linux-user: Add support for getting alsa timer version and id
This patch implements functionalities of following ioctls:
SNDRV_TIMER_IOCTL_PVERSION - Getting the sound timer version
Read the sound timer version. The third ioctl's argument is
a pointer to an int in which the specified timers version
is returned.
SNDRV_TIMER_IOCTL_NEXT_DEVICE - Getting id information about next timer
Read id information about the next timer device from the sound timer
device list. The id infomration is returned in the following structure:
struct snd_timer_id {
int dev_class; /* timer device class number */
int dev_sclass; /* slave device class number (unused) */
int card; /* card number */
int device; /* device number */
int subdevice; /* sub-device number */
};
The devices in the sound timer device list are arranged by the fields
of this structure respectively (first by dev_class number, then by
card number, ...). A pointer to this structure should be passed as
the third ioctl's argument. Before calling the ioctl, the parameters
of this structure should be initialized in relation to the next timer
device which information is to be obtained. For example, if a wanted
timer device has the device class number equal to or bigger then 2,
the field dev_class should be initialized to 2. After the ioctl call,
the structure fields are filled with values from the next device in
the sound timer device list. If there is no next device in the list,
the structure is filled with "zero" id values (in that case all
fields are filled with value -1).
Implementation notes:
The ioctl 'SNDRV_TIMER_IOCTL_NEXT_DEVICE' has a pointer to a
'struct snd_timer_id' as its third argument. That is the reason why
corresponding definition is added in 'linux-user/syscall_types.h'.
Since all elements of this structure are of type 'int', the rest of
the implementation was straightforward.
The line '#include <linux/rtc.h>' was added to recognize
preprocessor definitions for these ioctls. This needs to be
done only once in this series of commits. Also, the content
of this file (with respect to ioctl definitions) remained
unchanged for a long time, therefore there is no need to
worry about supporting older Linux kernel version.
Josh Kunz [Tue, 4 Feb 2020 02:54:14 +0000 (18:54 -0800)]
linux-user: Use `qemu_log' for strace
This change switches linux-user strace logging to use the newer `qemu_log`
logging subsystem rather than the older `gemu_log` (notice the "g")
logger. `qemu_log` has several advantages, namely that it allows logging
to a file, and provides a more unified interface for configuration
of logging (via the QEMU_LOG environment variable or options).
This change introduces a new log mask: `LOG_STRACE` which is used for
logging of user-mode strace messages.
Josh Kunz [Tue, 4 Feb 2020 02:54:13 +0000 (18:54 -0800)]
linux-user: Use `qemu_log' for non-strace logging
Since most calls to `gemu_log` are actually logging unimplemented features,
this change replaces most non-strace calls to `gemu_log` with calls to
`qemu_log_mask(LOG_UNIMP, ...)`. This allows the user to easily log to
a file, and to mask out these log messages if they desire.
Note: This change is slightly backwards incompatible, since now these
"unimplemented" log messages will not be logged by default.
*** CID 1419387: Memory - illegal accesses (OVERRUN)
/hw/hppa/dino.c: 267 in dino_chip_read_with_attrs()
261 val = s->ilr & s->imr & s->icr;
262 break;
263 case DINO_TOC_ADDR:
264 val = s->toc_addr;
265 break;
266 case DINO_GMASK ... DINO_TLTIM:
>>> CID 1419387: Memory - illegal accesses (OVERRUN)
>>> Overrunning array "s->reg800" of 12 4-byte elements at element index 12 (byte offset 48) using index "(addr - 2048UL) / 4UL" (which evaluates to 12).
267 val = s->reg800[(addr - DINO_GMASK) / 4];
268 if (addr == DINO_PAMR) {
269 val &= ~0x01; /* LSB is hardwired to 0 */
270 }
271 if (addr == DINO_MLTIM) {
272 val &= ~0x07; /* 3 LSB are hardwired to 0 */
*** CID 1419393: Memory - corruptions (OVERRUN)
/hw/hppa/dino.c: 363 in dino_chip_write_with_attrs()
357 /* These registers are read-only. */
358 break;
359
360 case DINO_GMASK ... DINO_TLTIM:
361 i = (addr - DINO_GMASK) / 4;
362 val &= reg800_keep_bits[i];
>>> CID 1419393: Memory - corruptions (OVERRUN)
>>> Overrunning array "s->reg800" of 12 4-byte elements at element index 12 (byte offset 48) using index "i" (which evaluates to 12).
363 s->reg800[i] = val;
364 break;
365
366 default:
367 /* Controlled by dino_chip_mem_valid above. */
368 g_assert_not_reached();
*** CID 1419394: Memory - illegal accesses (OVERRUN)
/hw/hppa/dino.c: 362 in dino_chip_write_with_attrs()
356 case DINO_IRR1:
357 /* These registers are read-only. */
358 break;
359
360 case DINO_GMASK ... DINO_TLTIM:
361 i = (addr - DINO_GMASK) / 4;
>>> CID 1419394: Memory - illegal accesses (OVERRUN)
>>> Overrunning array "reg800_keep_bits" of 12 4-byte elements at element index 12 (byte offset 48) using index "i" (which evaluates to 12).
362 val &= reg800_keep_bits[i];
363 s->reg800[i] = val;
364 break;
365
366 default:
367 /* Controlled by dino_chip_mem_valid above. */
Indeed the array should contain 13 entries, the undocumented
register 0x82c is missing. Fix by increasing the array size
and adding the missing register.
$ echo x 0xfff80830 | hppa-softmmu/qemu-system-hppa -S -monitor stdio -display none
QEMU 4.2.50 monitor - type 'help' for more information
(qemu) x 0xfff80830
qemu/hw/hppa/dino.c:267:15: runtime error: index 12 out of bounds for type 'uint32_t [12]'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/phil/source/qemu/hw/hppa/dino.c:267:15 in 00000000fff80830: 0x00000000
$ echo writeb 0xfff80830 0x69 \
| hppa-softmmu/qemu-system-hppa -S -accel qtest -qtest stdio -display none
[I 1581634452.654113] OPENED
[R +4.105415] writeb 0xfff80830 0x69
qemu/hw/hppa/dino.c:362:16: runtime error: index 12 out of bounds for type 'const uint32_t [12]'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior qemu/hw/hppa/dino.c:362:16 in
=================================================================
==29607==ERROR: AddressSanitizer: global-buffer-overflow on address 0x5577dae32f30 at pc 0x5577d93f2463 bp 0x7ffd97ea11b0 sp 0x7ffd97ea11a8
READ of size 4 at 0x5577dae32f30 thread T0
#0 0x5577d93f2462 in dino_chip_write_with_attrs qemu/hw/hppa/dino.c:362:16
#1 0x5577d9025664 in memory_region_write_with_attrs_accessor qemu/memory.c:503:12
#2 0x5577d9024920 in access_with_adjusted_size qemu/memory.c:539:18
#3 0x5577d9023608 in memory_region_dispatch_write qemu/memory.c:1482:13
#4 0x5577d8e3177a in flatview_write_continue qemu/exec.c:3166:23
#5 0x5577d8e20357 in flatview_write qemu/exec.c:3206:14
#6 0x5577d8e1fef4 in address_space_write qemu/exec.c:3296:18
#7 0x5577d8e20693 in address_space_rw qemu/exec.c:3306:16
#8 0x5577d9011595 in qtest_process_command qemu/qtest.c:432:13
#9 0x5577d900d19f in qtest_process_inbuf qemu/qtest.c:705:9
#10 0x5577d900ca22 in qtest_read qemu/qtest.c:717:5
#11 0x5577da8c4254 in qemu_chr_be_write_impl qemu/chardev/char.c:183:9
#12 0x5577da8c430c in qemu_chr_be_write qemu/chardev/char.c:195:9
#13 0x5577da8cf587 in fd_chr_read qemu/chardev/char-fd.c:68:9
#14 0x5577da9836cd in qio_channel_fd_source_dispatch qemu/io/channel-watch.c:84:12
#15 0x7faf44509ecc in g_main_context_dispatch (/lib64/libglib-2.0.so.0+0x4fecc)
#16 0x5577dab75f96 in glib_pollfds_poll qemu/util/main-loop.c:219:9
#17 0x5577dab74797 in os_host_main_loop_wait qemu/util/main-loop.c:242:5
#18 0x5577dab7435a in main_loop_wait qemu/util/main-loop.c:518:11
#19 0x5577d9514eb3 in main_loop qemu/vl.c:1682:9
#20 0x5577d950699d in main qemu/vl.c:4450:5
#21 0x7faf41a87f42 in __libc_start_main (/lib64/libc.so.6+0x23f42)
#22 0x5577d8cd4d4d in _start (qemu/build/sanitizer/hppa-softmmu/qemu-system-hppa+0x1256d4d)
0x5577dae32f30 is located 0 bytes to the right of global variable 'reg800_keep_bits' defined in 'qemu/hw/hppa/dino.c:87:23' (0x5577dae32f00) of size 48
SUMMARY: AddressSanitizer: global-buffer-overflow qemu/hw/hppa/dino.c:362:16 in dino_chip_write_with_attrs
Shadow bytes around the buggy address:
0x0aaf7b5be590: 00 f9 f9 f9 f9 f9 f9 f9 00 02 f9 f9 f9 f9 f9 f9
0x0aaf7b5be5a0: 07 f9 f9 f9 f9 f9 f9 f9 07 f9 f9 f9 f9 f9 f9 f9
0x0aaf7b5be5b0: 07 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
0x0aaf7b5be5c0: 00 00 00 02 f9 f9 f9 f9 00 00 00 00 00 00 00 00
0x0aaf7b5be5d0: 00 00 00 00 00 00 00 00 00 00 00 03 f9 f9 f9 f9
=>0x0aaf7b5be5e0: 00 00 00 00 00 00[f9]f9 f9 f9 f9 f9 00 00 00 00
0x0aaf7b5be5f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0aaf7b5be600: 00 00 01 f9 f9 f9 f9 f9 00 00 00 00 07 f9 f9 f9
0x0aaf7b5be610: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
0x0aaf7b5be620: 00 00 00 05 f9 f9 f9 f9 00 00 00 00 07 f9 f9 f9
0x0aaf7b5be630: f9 f9 f9 f9 00 00 f9 f9 f9 f9 f9 f9 07 f9 f9 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
Shadow gap: cc
==29607==ABORTING
hw/net/rocker: Report unimplemented feature with qemu_log_mask(UNIMP)
Fix warnings reported by Clang static code analyzer:
CC hw/net/rocker/rocker.o
hw/net/rocker/rocker.c:213:9: warning: Value stored to 'tx_tso_mss' is never read
tx_tso_mss = rocker_tlv_get_le16(tlvs[ROCKER_TLV_TX_TSO_MSS]);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
hw/net/rocker/rocker.c:217:9: warning: Value stored to 'tx_tso_hdr_len' is never read
tx_tso_hdr_len = rocker_tlv_get_le16(tlvs[ROCKER_TLV_TX_TSO_HDR_LEN]);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
hw/net/rocker/rocker.c:255:9: warning: Value stored to 'tx_l3_csum_off' is never read
tx_l3_csum_off += tx_tso_mss = tx_tso_hdr_len = 0;
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Fix warning reported by Clang static code analyzer:
hw/display/qxl.c:1634:14: warning: Value stored to 'orig_io_port' during its initialization is never read
uint32_t orig_io_port = io_port;
^~~~~~~~~~~~ ~~~~~~~
Michal Privoznik [Fri, 14 Feb 2020 09:55:19 +0000 (10:55 +0100)]
Report stringified errno in VFIO related errors
In a few places we report errno formatted as a negative integer.
This is not as user friendly as it can be. Use strerror() and/or
error_setg_errno() instead.
Peter Maydell [Tue, 18 Feb 2020 14:23:43 +0000 (14:23 +0000)]
Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging
Block layer patches:
- Fix check_to_replace_node()
- commit: Expose on-error option in QMP
- qcow2: Fix qcow2_alloc_cluster_abort() for external data file
- mirror: Fix deadlock
- vvfat: Fix segfault while closing read-write node
- Code cleanups
# gpg: Signature made Tue 18 Feb 2020 14:04:43 GMT
# gpg: using RSA key 7F09B272C88F2FD6
# gpg: Good signature from "Kevin Wolf <[email protected]>" [full]
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6
* remotes/kevin/tags/for-upstream: (36 commits)
iotests: Check that @replaces can replace filters
iotests: Add tests for invalid Quorum @replaces
iotests: Use self.image_len in TestRepairQuorum
iotests: Resolve TODOs in 041
iotests/041: Drop superfluous shutdowns
iotests: Add VM.assert_block_path()
iotests: Use complete_and_wait() in 155
quorum: Stop marking it as a filter
mirror: Double-check immediately before replacing
block: Remove bdrv_recurse_is_first_non_filter()
block: Use bdrv_recurse_can_replace()
quorum: Implement .bdrv_recurse_can_replace()
blkverify: Implement .bdrv_recurse_can_replace()
block: Add bdrv_recurse_can_replace()
quorum: Fix child permissions
iotests: Let 041 use -blockdev for quorum children
block: Drop bdrv_is_first_non_filter()
blockdev: Allow resizing everywhere
blockdev: Allow external snapshots everywhere
block/io_uring: Remove superfluous semicolon
...
Max Reitz [Tue, 18 Feb 2020 10:34:52 +0000 (11:34 +0100)]
iotests: Use self.image_len in TestRepairQuorum
041's TestRepairQuorum has its own image_len, no need to refer to
TestSingleDrive. (This patch allows commenting out TestSingleDrive to
speed up 041 during test testing.)
Max Reitz [Tue, 18 Feb 2020 10:34:50 +0000 (11:34 +0100)]
iotests/041: Drop superfluous shutdowns
All tearDowns in 041 shutdown the VM. Thus, test cases do not need to
do it themselves (unless they need the VM to be down for some
post-operation check).
Max Reitz [Tue, 18 Feb 2020 10:34:47 +0000 (11:34 +0100)]
quorum: Stop marking it as a filter
Quorum is not a filter, for example because it cannot guarantee which of
its children will serve the next request. Thus, any of its children may
differ from the data visible to quorum's parents.
We have other filters with multiple children, but they differ in this
aspect:
- blkverify quits the whole qemu process if its children differ. As
such, we can always skip it when we want to skip it (as a filter node)
by going to any of its children. Both have the same data.
- replication generally serves requests from bs->file, so this is its
only actually filtered child.
- Block job filters currently only have one child, but they will
probably get more children in the future. Still, they will always
have only one actually filtered child.
Having "filters" as a dedicated node category only makes sense if you
can skip them by going to a one fixed child that always shows the same
data as the filter node. Quorum cannot fulfill this, so it is not a
filter.
Max Reitz [Tue, 18 Feb 2020 10:34:46 +0000 (11:34 +0100)]
mirror: Double-check immediately before replacing
There is no guarantee that we can still replace the node we want to
replace at the end of the mirror job. Double-check by calling
bdrv_recurse_can_replace().
Max Reitz [Tue, 18 Feb 2020 10:34:44 +0000 (11:34 +0100)]
block: Use bdrv_recurse_can_replace()
Let check_to_replace_node() use the more specialized
bdrv_recurse_can_replace() instead of
bdrv_recurse_is_first_non_filter(), which is too restrictive (or, in the
case of quorum, sometimes not restrictive enough).
Max Reitz [Tue, 18 Feb 2020 10:34:41 +0000 (11:34 +0100)]
block: Add bdrv_recurse_can_replace()
After a couple of follow-up patches, this function will replace
bdrv_recurse_is_first_non_filter() in check_to_replace_node().
bdrv_recurse_is_first_non_filter() is both not sufficiently specific for
check_to_replace_node() (it allows cases that should not be allowed,
like replacing child nodes of quorum with dissenting data that have more
parents than just quorum), and it is too restrictive (it is perfectly
fine to replace filters).
Max Reitz [Tue, 18 Feb 2020 10:34:40 +0000 (11:34 +0100)]
quorum: Fix child permissions
Quorum cannot share WRITE or RESIZE on its children. Presumably, it
only does so because as a filter, it seemed intuitively correct to point
its .bdrv_child_perm to bdrv_filter_default_perm().
However, it is not really a filter, and bdrv_filter_default_perm() does
not work for it, so we have to provide a custom .bdrv_child_perm
implementation.
Max Reitz [Tue, 18 Feb 2020 10:34:39 +0000 (11:34 +0100)]
iotests: Let 041 use -blockdev for quorum children
Using -drive with default options means that a virtio-blk drive will be
created that has write access to the to-be quorum children. Quorum
should have exclusive write access to them, so we should use -blockdev
instead.
Max Reitz [Tue, 18 Feb 2020 10:34:37 +0000 (11:34 +0100)]
blockdev: Allow resizing everywhere
Block nodes that do not allow resizing should not share BLK_PERM_RESIZE.
It does not matter whether they are the first non-filter in their chain
or not.
Max Reitz [Tue, 18 Feb 2020 10:34:36 +0000 (11:34 +0100)]
blockdev: Allow external snapshots everywhere
There is no good reason why we would allow external snapshots only on
the first non-filter node in a chain. Parent BDSs should not care
whether their child is replaced by a snapshot. (If they do care, they
should announce that via freezing the chain, which is checked in
bdrv_append() through bdrv_set_backing_hd().)
Before we had bdrv_is_first_non_filter() here (since 212a5a8f095), there
was a special function bdrv_check_ext_snapshot() that allowed snapshots
by default, but block drivers could override this. Only blkverify did
so, however.
It is not clear to me why blkverify would do so; maybe just so that the
testee block driver would not be replaced. The introducing commit f6186f49e2c does not explain why. Maybe because 08b24cfe376 would have
been the correct solution? (Which adds a .supports_backing check.)
Kevin Wolf [Fri, 14 Feb 2020 20:08:12 +0000 (21:08 +0100)]
iotests: Test error handling policies with block-commit
This tests both read failure (from the top node) and write failure (to
the base node) for on-error=report/stop/ignore.
As block-commit actually starts two different types of block jobs
(mirror.c for committing the active later, commit.c for intermediate
layers), all tests are run for both cases.
Kevin Wolf [Fri, 14 Feb 2020 20:08:11 +0000 (21:08 +0100)]
commit: Expose on-error option in QMP
Now that the error handling in the common block job is fixed, we can
expose the on-error option in QMP instead of hard-coding it as 'report'
in qmp_block_commit().
This fulfills the promise that the old comment in that function made,
even if a bit later than expected: "This will be part of the QMP
command, if/when the BlockdevOnError change for blkmirror makes it in".
Kevin Wolf [Fri, 14 Feb 2020 20:08:10 +0000 (21:08 +0100)]
commit: Fix is_read for block_job_error_action()
block_job_error_action() needs to know if reading from the top node or
writing to the base node failed so that it can set the right 'operation'
in the BLOCK_JOB_ERROR QMP event.
Kevin Wolf [Fri, 14 Feb 2020 20:08:09 +0000 (21:08 +0100)]
commit: Inline commit_populate()
commit_populate() is a very short function and only called in a single
place. Its return value doesn't tell us whether an error happened while
reading or writing, which would be necessary for sending the right data
in the BLOCK_JOB_ERROR QMP event.
Kevin Wolf [Fri, 14 Feb 2020 20:08:07 +0000 (21:08 +0100)]
commit: Remove unused bytes_written
The bytes_written variable is only ever written to, it serves no
purpose. This has actually been the case since the commit job was first
introduced in commit 747ff602636.
Kevin Wolf [Fri, 14 Feb 2020 20:08:06 +0000 (21:08 +0100)]
qapi: Document meaning of 'ignore' BlockdevOnError for jobs
It is not obvious what 'ignore' actually means for block jobs: It could
be continuing the job and returning success in the end despite the error
(no block job does this). It could also mean continuing and returning
failure in the end (this is what stream does). And it can mean retrying
the failed request later (this is what backup, commit and mirror do).
This (somewhat inconsistent) behaviour was introduced and described for
stream and mirror in commit 32c81a4a6ec. backup and commit were
introduced later and use the same model as mirror.
Kevin Wolf [Tue, 11 Feb 2020 09:49:00 +0000 (10:49 +0100)]
iotests: Test copy offloading with external data file
This adds a test for 'qemu-img convert' with copy offloading where the
target image has an external data file. If the test hosts supports it,
it tests both the case where copy offloading is supported and the case
where it isn't (otherwise we just test unsupported twice).
More specifically, the case with unsupported copy offloading tests
qcow2_alloc_cluster_abort() with external data files.
Kevin Wolf [Tue, 11 Feb 2020 09:48:59 +0000 (10:48 +0100)]
qcow2: Fix qcow2_alloc_cluster_abort() for external data file
For external data file, cluster allocations return an offset in the data
file and are not refcounted. In this case, there is nothing to do for
qcow2_alloc_cluster_abort(). Freeing the same offset in the qcow2 file
is wrong and causes crashes in the better case or image corruption in
the worse case.
Kevin Wolf [Tue, 11 Feb 2020 09:48:58 +0000 (10:48 +0100)]
qcow2: update_refcount(): Reset old_table_index after qcow2_cache_put()
In the case that update_refcount() frees a refcount block, it evicts it
from the metadata cache. Before doing so, however, it returns the
currently used refcount block to the cache because it might be the same.
Returning the refcount block early means that we need to reset
old_table_index so that we reload the refcount block in the next
iteration if it is actually still in use.
Hikaru Nishida [Sun, 9 Feb 2020 17:51:56 +0000 (02:51 +0900)]
block/vvfat: Do not unref qcow on closing backing bdrv
Before this commit, BDRVVVFATState.qcow is unrefed in write_target_close
on closing backing bdrv of vvfat. However, qcow bdrv is opend as a child
of vvfat in enable_write_target() so it will be also unrefed on closing
vvfat itself. This causes use-after-free of qcow on freeing vvfat which
has backing bdrv and qcow bdrv as children in this order because
bdrv_close(vvfat) tries to free qcow bdrv after freeing backing bdrv
as QLIST_FOREACH_SAFE() loop keeps next pointer, but BdrvChild of qcow
is already freed in bdrv_close(backing bdrv).
Alberto Garcia [Thu, 13 Feb 2020 17:16:46 +0000 (18:16 +0100)]
qcow2: Fix alignment checks in encrypted images
I/O requests to encrypted media should be aligned to the sector size
used by the underlying encryption method, not to BDRV_SECTOR_SIZE.
Fortunately this doesn't break anything at the moment because
both existing QCRYPTO_BLOCK_*_SECTOR_SIZE have the same value as
BDRV_SECTOR_SIZE.
The checks in qcow2_co_preadv_encrypted() are also unnecessary because
they are repeated immediately afterwards in qcow2_co_encdec().
Kevin Wolf [Tue, 28 Jan 2020 15:09:28 +0000 (16:09 +0100)]
mirror: Don't let an operation wait for itself
mirror_wait_for_free_in_flight_slot() just picks a random operation to
wait for. However, when mirror_co_read() waits for free slots, its
MirrorOp is already in s->ops_in_flight, so if not enough slots are
immediately available, an operation can end up waiting for itself to
complete, which results in a hang.
Fix this by passing the current MirrorOp and skipping this operation
when picking an operation to wait for.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1794692 Signed-off-by: Kevin Wolf <[email protected]> Reviewed-by: Eric Blake <[email protected]>
Kevin Wolf [Tue, 28 Jan 2020 15:06:41 +0000 (16:06 +0100)]
mirror: Store MirrorOp.co for debuggability
If a coroutine is launched, but the coroutine pointer isn't stored
anywhere, debugging any problems inside the coroutine is quite hard.
Let's store the coroutine pointer of a mirror operation in MirrorOp to
have it available in the debugger.
Kevin Wolf [Wed, 29 Jan 2020 10:22:39 +0000 (11:22 +0100)]
monitor: Move qmp_query_qmp_schema to qmp-cmds-control.c
monitor/misc.c contains code that works only in the system emulator, so
it can't be linked to tools like a storage daemon. In order to make
schema introspection available for tools, move the function to
monitor/qmp-cmds-control.c, which can be linked into the storage daemon.
Kevin Wolf [Wed, 29 Jan 2020 10:22:38 +0000 (11:22 +0100)]
monitor: Collect "control" command handlers in qmp-cmds.control.c
Move all of the QMP commands handlers to implement the 'control' module
(qapi/control.json) that can be shared between the system emulator and
tools such as a storage daemon to a new file monitor/qmp-cmds-control.c.
Kevin Wolf [Wed, 29 Jan 2020 10:22:37 +0000 (11:22 +0100)]
qapi: Split control.json off misc.json
misc.json contains definitions that are related to the system emulator,
so it can't be used for other tools like the storage daemon. This patch
moves basic functionality that is shared between all tools (and mostly
related to the monitor itself) into a new control.json, which could be
used in tools as well.
Kevin Wolf [Wed, 29 Jan 2020 10:22:36 +0000 (11:22 +0100)]
monitor: Move monitor option parsing to monitor/monitor.c
Both the system emulators and tools with QMP support (specifically, the
planned storage daemon) will need to parse monitor options, so move that
code to monitor/monitor.c, which can be linked into binaries that aren't
a system emulator.
Currently, there is no usage of TARGET_NR_syscall_count for target
xtensa, and there is no obvious indication if there is some planned
usage in future.