Paolo Bonzini [Sun, 15 Jul 2018 16:21:56 +0000 (18:21 +0200)]
qga: fix file descriptor leak
The file descriptor for /sys/power/state was never closed. Reported
by Coverity.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Peter Maydell [Mon, 16 Jul 2018 16:43:23 +0000 (17:43 +0100)]
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20180716' into staging
target-arm queue:
* accel/tcg: Use correct test when looking in victim TLB for code
* bcm2835_aux: Swap RX and TX interrupt assignments
* hw/arm/bcm2836: Mark the bcm2836 / bcm2837 devices with user_creatable = false
* hw/intc/arm_gic: Fix handling of GICD_ITARGETSR
* hw/intc/arm_gic: Check interrupt number in gic_deactivate_irq()
* aspeed: Implement write-1-{set, clear} for AST2500 strapping
* target/arm: Fix LD1W and LDFF1W (scalar plus vector)
* remotes/pmaydell/tags/pull-target-arm-20180716:
accel/tcg: Assert that tlb fill gave us a valid TLB entry
accel/tcg: Use correct test when looking in victim TLB for code
bcm2835_aux: Swap RX and TX interrupt assignments
hw/arm/bcm2836: Mark the bcm2836 / bcm2837 devices with user_creatable = false
hw/intc/arm_gic: Fix handling of GICD_ITARGETSR
hw/intc/arm_gic: Check interrupt number in gic_deactivate_irq()
aspeed: Implement write-1-{set, clear} for AST2500 strapping
target/arm: Fix LD1W and LDFF1W (scalar plus vector)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Fri, 13 Jul 2018 14:16:36 +0000 (15:16 +0100)]
accel/tcg: Assert that tlb fill gave us a valid TLB entry
In commit 4b1a3e1e34ad97 we added a check for whether the TLB entry
we had following a tlb_fill had the INVALID bit set. This could
happen in some circumstances because a stale or wrong TLB entry was
pulled out of the victim cache. However, after commit 68fea038553039e (which prevents stale entries being in the victim
cache) and the previous commit (which ensures we don't incorrectly
hit in the victim cache)) this should never be possible.
Drop the check on TLB_INVALID_MASK from the "is this a TLB_RECHECK?"
condition, and instead assert that the tlb fill procedure has given
us a valid TLB entry (or longjumped out with a guest exception).
Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20180713141636.18665-3-peter.maydell@linaro.org
Peter Maydell [Fri, 13 Jul 2018 14:16:35 +0000 (15:16 +0100)]
accel/tcg: Use correct test when looking in victim TLB for code
In get_page_addr_code(), we were incorrectly looking in the victim
TLB for an entry which matched the target address for reads, not
for code accesses. This meant that we could hit on a victim TLB
entry that indicated that the address was readable but not
executable, and incorrectly bypass the call to tlb_fill() which
should generate the guest MMU exception. Fix this bug.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20180713141636.18665-2-peter.maydell@linaro.org
RX and TX interrupt bits were reversed, resulting in an endless sequence
of serial interupts in the emulated system and the following repeated
error message when booting Linux.
This is with arm64:defconfig. The root file system was generated using
buildroot.
NB that this error likely arises from an erratum in the
BCM2835 datasheet where the TX and RX bits were swapped
in the AU_MU_IER_REG description (but correct for IIR):
https://elinux.org/BCM2835_datasheet_errata#p12
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Message-id: 1529355846-25102-1-git-send-email-linux@roeck-us.net Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
[PMM: added NB about datasheet] Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Thomas Huth [Mon, 16 Jul 2018 16:18:41 +0000 (17:18 +0100)]
hw/arm/bcm2836: Mark the bcm2836 / bcm2837 devices with user_creatable = false
These devices are currently causing some problems when a user is trying
to hot-plug or introspect them during runtime. Since these devices can
not be instantiated by the user at all (they need to be wired up in code
instead), we should mark them with user_creatable = false anyway, then we
avoid at least the crashes with the hot-plugging. The introspection problem
will be handled by a separate patch.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 1531415537-26037-1-git-send-email-thuth@redhat.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Mon, 16 Jul 2018 16:18:41 +0000 (17:18 +0100)]
hw/intc/arm_gic: Fix handling of GICD_ITARGETSR
The GICD_ITARGETSR implementation still has some 11MPCore behaviour
that we were incorrectly using in our GICv1 and GICv2 implementations
for the case where the interrupt number is less than GIC_INTERNAL.
The desired behaviour here is:
* for 11MPCore: RAZ/WI for irqs 0..28; read a number matching the
CPU doing the read for irqs 29..31
* for GICv1 and v2: RAZ/WI if uniprocessor; otherwise read a
number matching the CPU doing the read for all irqs < 32
Stop squashing GICD_ITARGETSR to 0 for IRQs 0..28 unless this
is an 11MPCore GIC.
Reported-by: Jan Kiszka <jan.kiszka@web.de> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Message-id: 20180712154152.32183-3-peter.maydell@linaro.org
Peter Maydell [Mon, 16 Jul 2018 16:18:41 +0000 (17:18 +0100)]
hw/intc/arm_gic: Check interrupt number in gic_deactivate_irq()
In gic_deactivate_irq() the interrupt number comes from the guest
(on a write to the GICC_DIR register), so we need to sanity check
that it isn't out of range before we use it as an array index.
Handle this in a similar manner to the check we do in
gic_complete_irq() for the GICC_EOI register.
The array overrun is not disastrous because the calling code
uses (value & 0x3ff) to extract the interrupt field, so the
only out-of-range values possible are 1020..1023, which allow
overrunning only from irq_state[] into the following
irq_target[] array which the guest can already manipulate.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Message-id: 20180712154152.32183-2-peter.maydell@linaro.org
Andrew Jeffery [Mon, 16 Jul 2018 16:18:41 +0000 (17:18 +0100)]
aspeed: Implement write-1-{set, clear} for AST2500 strapping
The AST2500 SoC family changes the runtime behaviour of the hardware
strapping register (SCU70) to write-1-set/write-1-clear, with
write-1-clear implemented on the "read-only" SoC revision register
(SCU7C). For the the AST2400, the hardware strapping is
runtime-configured with read-modify-write semantics.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Reviewed-by: Joel Stanley <joel@jms.id.au>
Message-id: 20180709143524.17480-1-andrew@aj.id.au Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
* remotes/armbru/tags/pull-misc-2018-07-16:
monitor: Fix tracepoint crash on JSON syntax error
MAINTAINERS: New section "Incompatible changes", copy libvir-list
qemu-doc: Move appendix "Deprecated features" to its own file
cli qmp: Mark --preconfig, exit-preconfig experimental
qapi: Do not expose "allow-preconfig" in query-qmp-schema
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
monitor: Fix tracepoint crash on JSON syntax error
When tracepoint handle_qmp_command is enabled, we crash on JSON syntax
errors. Broken in commit 1cc37471525. Fix by skipping the tracepoint
on JSON syntax error. Before the flawed commit, we skipped it by
returning early.
Fixes: CID 1394216 Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180716091012.29510-1-armbru@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com>
MAINTAINERS: New section "Incompatible changes", copy libvir-list
Libvirt developers would like to be copied on patches to qemu-doc
appendix "Deprecated features". Do them the favor.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180716073226.21127-3-armbru@redhat.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
qemu-doc: Move appendix "Deprecated features" to its own file
Consumers of QEMU need to track feature deprecation. Keeping
deprecation documentation in its own file helps in two small ways:
* You can track changes the easy and obvious way, with git-log.
Before, you had to resort to more complex gittery like "git-log
--oneline -L '/@node Deprecated features/,/@node Supported build
platforms/:qemu-doc.texi'"
* It lets us use MAINTAINERS to copy interested parties on deprecation
patches, so they can advise or object before they're a done deal.
The next commit will do that for libvirt.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180716073226.21127-2-armbru@redhat.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
cli qmp: Mark --preconfig, exit-preconfig experimental
Committing to the current --preconfig / exit-preconfig interface
before it has seen any use is premature. Mark both as experimental,
the former in documentation, the latter by renaming it to
x-exit-preconfig.
See the previous commit for more detailed rationale.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180705091402.26244-3-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Acked-by: Eduardo Habkost <ehabkost@redhat.com> Acked-by: Igor Mammedov <imammedo@redhat.com>
[Straightforward conflict with commit 514337c142f resolved]
qapi: Do not expose "allow-preconfig" in query-qmp-schema
According to commit 047f7038f58, option --preconfig
[...] allows pausing QEMU in the new RUN_STATE_PRECONFIG state,
allowing the configuration of QEMU from QMP before the machine
jumps into board initialization code of machine_run_board_init()
The intent is to allow management to query machine state and
additionally configure it using previous query results within one
QEMU instance (i.e. eliminate the need to start QEMU twice, 1st to
query board specific parameters and 2nd for actual VM start using
query results for additional parameters).
The implementation is a bit of a hack: it splices in an additional
main loop before machine creation, in special runstate preconfig. New
command exit-preconfig exits that main loop. QEMU continues
initializing, creates the machine, and runs the good old main loop.
The replacement of the main loop is transparent to monitors.
Sadly, some commands expect initialization to be complete. Running
them in --preconfig's main loop violates their preconditions. Since
we don't really know which commands are safe, we use a whitelist.
This drags the concept of run state into the QMP core.
The whitelist is done as a command flag in the QAPI schema (commit d6fe3d02e9a). Drags the concept of run state further into the QAPI
language.
The command flag is exposed in query-qmp-schema (also commit d6fe3d02e9a). This makes it ABI.
I consider the whole thing an offensively ugly hack, but sometimes an
ugly hack is the best we can do to solve a problem people have.
The need described by the commit message quote above is genuine. The
proper solution would be a main loop that permits complete
configuration via QMP. This is out of reach, thus the hack.
However, even though the need is genuine, it isn't urgent: libvirt is
not going to use this anytime soon. Baking a hack into ABI before it
has any users is a bad idea.
This commit reverts the parts of commit d6fe3d02e9a that affect ABI
via query-qmp-schema. The commit did the following:
(1) Add command flag 'allow-preconfig' to the QAPI schema language
(2) Pass it to code generators
(3) Have the commands.py code generator pass it to the command
registry (so commit 047f7038f58 can use it as whitelist)
(4) Add 'allow-preconfig' to SchemaInfoCommand (neglecting to update
qapi-code-gen.txt section "Client JSON Protocol introspection")
(5) Set 'allow-preconfig': true for commands qmp_capabilities,
query-commands, query-command-line-options, query-status
Revert exactly (4), plus a bit of documentation added to
qemu-tech.info in commit 047f7038f58.
Shrinks query-qmp-schema's output from 126.5KiB to 121.8KiB for me.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180705091402.26244-2-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Acked-by: Eduardo Habkost <ehabkost@redhat.com> Acked-by: Igor Mammedov <imammedo@redhat.com>
[Straightforward conflict with commit d626b6c1ae7 resolved]
Peter Maydell [Mon, 16 Jul 2018 10:04:24 +0000 (11:04 +0100)]
Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-3.0-20180716' into staging
ppc patch queue 2018-07-16
Here's my first hard freeze pull request for qemu-3.0. This contains
an assortment of bugfixes. Several are for regressions, others are for
bugs that I think are significant enough to address during hard freeze.
Peter Maydell [Mon, 16 Jul 2018 09:06:23 +0000 (10:06 +0100)]
Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-3.0-pull-request' into staging
Some fixes for linux-user:
- workaround for CMSG_NXTHDR bug
- two patches for ppc64/ppc64le host:
fix fcntl() with *LK64 commands
(seen when dpkg wants to lock the DB)
fix reserved_va alignment (ppc64 needs
a 64kB alignment)
- convert a forgotten fcntl() to safe_fcntl()
* remotes/vivier2/tags/linux-user-for-3.0-pull-request:
Zero out the host's `msg_control` buffer
linux-user: fix mmap_find_vma_reserved()
linux-user: convert remaining fcntl() to safe_fcntl()
linux-user: ppc64: use the correct values for F_*LK64s
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Coverity warned that the false arm of conditional expression is
unreachable when it is inside an if with the same condition.
Remove the unreachable code to avoid the warning.
Fixes: CID 1394215 Reported-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Interrupt conditions occurring while masked are not being
signaled when later unmasked.
The fix is to raise/lower IRQs when IMASK is changed.
To avoid problems like this in future, consolidate
IRQ pin update logic in one function.
Also fix probable typo "IEVENT_TXF | IEVENT_TXF",
and update IRQ pins on reset.
Signed-off-by: Michael Davidsaver <mdavidsaver@gmail.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Greg Kurz [Thu, 12 Jul 2018 10:01:49 +0000 (12:01 +0200)]
ppc/xics: fix ICP reset path
Recent cleanup in commit a028dd423ee6 dropped the ICPStateClass::reset
handler. It is now up to child ICP classes to call the DeviceClass::reset
handler of the parent class, thanks to device_class_set_parent_reset().
This is a better object programming pattern, but unfortunately it causes
QEMU to crash during CPU hotplug:
but dc->reset, ie, DeviceClass::reset for the TYPE_ICP type, is
itself NULL.
This patch hence sets DeviceClass::reset for the TYPE_ICP type to
point to icp_reset(). It then registers a reset handler that calls
DeviceClass::reset. If the ICP subtype has configured its own reset
handler with device_class_set_parent_reset(), this ensures it will
be called first and it can then call ICPStateClass::parent_reset
safely. This fixes the reset path for the TYPE_KVM_ICP type, which
is the only subtype that defines its own reset function.
Reported-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> Suggested-by: David Gibson <david@gibson.dropbear.id.au> Fixes: a028dd423ee6dfd091a8c63028240832bf10f671 Signed-off-by: Greg Kurz <groug@kaod.org> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
David Gibson [Tue, 10 Jul 2018 01:40:52 +0000 (11:40 +1000)]
spapr: Correct inverted test in spapr_pc_dimm_node()
This function was introduced between v2.11 and v2.12 to replace obsolete
ways of specifying the NUMA nodes for DIMMs. It's used to find the correct
node for an LMB, by locating which DIMM object it lies within.
Unfortunately, one of the checks is inverted, so we check whether the
address is less than two different things, rather than actually checking
a range. This introduced a regression, meaning that after a reboot qemu
will advertise incorrect node information for memory to the guest.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Greg Kurz <groug@kaod.org> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
sm501: Update screen on frame buffer address change
When the guest changes the address of the frame buffer we need to
refresh the screen to correctly display the new content. This fixes
display update problems when changing between screens on AmigaOS.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Jonas Schievink [Wed, 11 Jul 2018 22:12:44 +0000 (00:12 +0200)]
Zero out the host's `msg_control` buffer
If this is not done, qemu would drop any control message after the first
one.
This is because glibc's `CMSG_NXTHDR` macro accesses the uninitialized
cmsghdr's length field in order to find out if the message fits into the
`msg_control` buffer, wrongly assuming that it doesn't because the
length field contains garbage. Accessing the length field is fine for
completed messages we receive from the kernel, but is - as far as I know
- not needed since the kernel won't return such an invalid cmsghdr in
the first place.
This is tracked as this glibc bug:
https://sourceware.org/bugzilla/show_bug.cgi?id=13500
It's probably also a good idea to bail with an error if `CMSG_NXTHDR`
returns NULL but `TARGET_CMSG_NXTHDR` doesn't (ie. we still expect
cmsgs).
linux-user: ppc64: use the correct values for F_*LK64s
Qemu includes the glibc headers for the host defines and target headers are
part of the qemu source themselves. The glibc has the F_GETLK64, F_SETLK64
and F_SETLKW64 defined to 12, 13 and 14 for all archs in
sysdeps/unix/sysv/linux/bits/fcntl-linux.h. The linux kernel generic
definition for F_*LK is 5, 6 & 7 and F_*LK64* is 12,13, and 14 as seen in
include/uapi/asm-generic/fcntl.h. On 64bit machine, by default the kernel
assumes all F_*LK to 64bit calls and doesnt support use of F_*LK64* as
can be seen in include/linux/fcntl.h in linux source.
On x86_64 host, the values for F_*LK64* are set to 5, 6 and 7
explicitly in /usr/include/x86_64-linux-gnu/bits/fcntl.h by the glibc.
Whereas, a PPC64 host doesn't have such a definition in
/usr/include/powerpc64le-linux-gnu/bits/fcntl.h by the glibc. So,
the sources on PPC64 host sees the default value of F_*LK64*
as 12, 13 & 14(fcntl-linux.h).
Since the 64bit kernel doesnt support 12, 13 & 14; the glibc fcntl syscall
implementation(__libc_fcntl*(), __fcntl64_nocancel) does the F_*LK64* value
convertion back to F_*LK* values on PPC64 as seen in
sysdeps/unix/sysv/linux/powerpc/powerpc64/sysdep.h with FCNTL_ADJUST_CMD()
macro. Whereas on x86_64 host the values for F_*LK64* are set to 5, 6 and 7
and no adjustments are needed.
Since qemu doesnt use the glibc fcntl, but makes the safe_syscall* on its
own, the PPC64 qemu is calling the syscall with 12, 13, and 14(without
adjustment) and they all fail. The fcntl calls to F_GETLK/F_SETLK|W all
fail by all pplications run on PPC64 host user emulation.
The fix here could be to see why on PPC64 the glibc is still keeping
F_*LK64* different from F_*LK and why adjusting them to 5, 6 and 7 before
the syscall for PPC only. See if we can make the
/usr/include/powerpc64le-linux-gnu/bits/fcntl.h to have the values
5, 6 & 7 just like x86_64 and remove the adjustment code in glibc. That
way, qemu sources see the kernel supported values in glibc headers.
OR
On PPC64 host, qemu sources see both F_*LK & F_*LK64* as same and set to
12, 13 and 14 because __USE_FILE_OFFSET64 is defined in qemu
sources(also refer sysdeps/unix/sysv/linux/bits/fcntl-linux.h).
Do the value adjustment just like it is done by glibc source by using
F_GETLK value of 5. That way, we make the syscalls with the actual
supported values in Qemu. The patch is taking this approach.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <153148521235.87746.14142430397318741182.stgit@lep8c.aus.stglabs.ibm.com> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Ville Skyttä [Tue, 12 Jun 2018 06:51:50 +0000 (09:51 +0300)]
docs: Grammar and spelling fixes
Signed-off-by: Ville Skyttä <ville.skytta@iki.fi> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180612065150.21110-1-ville.skytta@iki.fi Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Thu, 12 Jul 2018 16:37:49 +0000 (17:37 +0100)]
Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging
Block layer patches:
- file-posix: Check correct file type (regular file for 'file',
character or block device for 'host_device'/'host_cdrom')
- scsi-disk: Block Device Characteristics emulation fix
- qemu-img: Consider required alignment for sparse area detection
- Documentation and test improvements
# gpg: Signature made Thu 12 Jul 2018 17:29:17 BST
# gpg: using RSA key 7F09B272C88F2FD6
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>"
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6
* remotes/kevin/tags/for-upstream:
qemu-img: align result of is_allocated_sectors
scsi-disk: Block Device Characteristics emulation fix
iotests: add test 226 for file driver types
file-posix: specify expected filetypes
qemu-img: Document copy offloading implications with -S and -c
iotests: nbd: Stop qemu-nbd before remaking image
iotests: 153: Fix dead code
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Lieven [Thu, 12 Jul 2018 13:00:10 +0000 (15:00 +0200)]
qemu-img: align result of is_allocated_sectors
We currently don't enforce that the sparse segments we detect during convert are
aligned. This leads to unnecessary and costly read-modify-write cycles either
internally in Qemu or in the background on the storage device as nearly all
modern filesystems or hardware have a 4k alignment internally.
This patch modifies is_allocated_sectors so that its *pnum result will always
end at an alignment boundary. This way all requests will end at an alignment
boundary. The start of all requests will also be aligned as long as the results
of get_block_status do not lead to an unaligned offset.
The number of RMW cycles when converting an example image [1] to a raw device that
has 4k sector size is about 4600 4k read requests to perform a total of about 15000
write requests. With this path the additional 4600 read requests are eliminated while
the number of total write requests stays constant.
By the SCSI spec, the expected size of the SBC page is 0x40.
There is no telling how the guest will behave with a shorter
message - it can ignore it, or worse, make (wrong)
assumptions.
This patch fixes the emulation by setting the size to 0x40.
This is the output of the previous sg_vpd command after
applying it:
$ sg_vpd --page=bdc /dev/sda -v
inquiry cdb: 12 01 b1 00 fc 00
Block device characteristics VPD page (SBC):
[PQual=0 Peripheral device type: disk]
Medium rotation rate is not reported
Product type: Not specified
WABEREQ=0
WACEREQ=0
Nominal form factor not reported
FUAB=0
VBULS=0
To improve readability, this patch also adds the VBULS value
explictly and add comments on the existing fields we're
setting.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
John Snow [Tue, 10 Jul 2018 17:00:41 +0000 (13:00 -0400)]
iotests: add test 226 for file driver types
Test that we're rejecting what we ought to for file,
host_driver and host_cdrom drivers. Test that we're
seeing the deprecated message for block and chardevs
on the file driver.
Signed-off-by: John Snow <jsnow@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
John Snow [Tue, 10 Jul 2018 17:00:40 +0000 (13:00 -0400)]
file-posix: specify expected filetypes
Adjust each caller of raw_open_common to specify if they are expecting
host and character devices or not. Tighten expectations of file types upon
open in the common code and refuse types that are not expected.
This has two effects:
(1) Character and block devices are now considered deprecated for the
'file' driver, which expects only S_IFREG, and
(2) no file-posix driver (file, host_cdrom, or host_device) can open
directories now.
I don't think there's a legitimate reason to open directories as if
they were files. This prevents QEMU from opening and attempting to probe
a directory inode, which can break in exciting ways. One of those ways
is lseek on ext4/xfs, which will return 0x7fffffffffffffff as the file
size instead of EISDIR. This can coax QEMU into responding with a
confusing "file too big" instead of "Hey, that's not a file".
See: https://bugs.launchpad.net/qemu/+bug/1739304/ Signed-off-by: John Snow <jsnow@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
qemu-img: Document copy offloading implications with -S and -c
Explicitly enabling zero detection or compression suppresses copy
offloading during convert. Document it.
Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
197 is one example where _make_test_img is used twice without stopping
the NBD server in between. An error will occur like this:
@@ -26,9 +26,13 @@
=== Partial final cluster ===
+qemu-img: TEST_DIR/t.IMGFMT: Failed to get "resize" lock
+Is another process using the image?
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1024
+Failed to find an available port: Address already in use
read 1024/1024 bytes at offset 0
Patch _make_test_img to stop the old qemu-nbd before starting a new one,
which fixes this problem, and similarly 215.
Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This step was left behind my mistake. As suggested by the echoed text,
the intention was to test two devices with the same image, with
different options. The behavior should be the same as two QEMU
processes. Complete it.
Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
John Arbuckle [Mon, 9 Jul 2018 15:02:35 +0000 (11:02 -0400)]
ui/cocoa.m: replace scrollingDeltaY with deltaY
The NSEvent class method scrollingDeltaY is available
for Mac OS 10.7 and newer. Since QEMU supports Mac OS
10.5 and up, we need to be using a method that is
available on these version of Mac OS X. The deltaY
method is a method that does almost the same thing as
scrollingDeltaY and is available on Mac OS 10.5 and
up. So we can replace scrollingDeltaY with deltaY.
We only check deltaY's value if it is not zero
because zero means that the scrolling increment was
sufficiently fine that it was only reported in scrollingDeltaY,
or that the scrolling was horizontal.
Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
Message-id: 20180709150235.7573-1-programmingkidx@gmail.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
[PMM: tweak commit message and comment a little] Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
seccomp: allow sched_setscheduler() with SCHED_IDLE policy
Current and upcoming mesa releases rely on a shader disk cash. It uses
a thread job queue with low priority, set with
sched_setscheduler(SCHED_IDLE). However, that syscall is rejected by
the "resourcecontrol" seccomp qemu filter.
Since it should be safe to allow lowering thread priority, let's allow
scheduling thread to idle policy.
Related to:
https://bugzilla.redhat.com/show_bug.cgi?id=1594456
vfio/pci: do not set the PCIDevice 'has_rom' attribute
PCI devices needing a ROM allocate an optional MemoryRegion with
pci_add_option_rom(). pci_del_option_rom() does the cleanup when the
device is destroyed. The only action taken by this routine is to call
vmstate_unregister_ram() which clears the id string of the optional
ROM RAMBlock and now, also flags the RAMBlock as non-migratable. This
was recently added by commit b895de502717 ("migration: discard
non-migratable RAMBlocks"), .
VFIO devices do their own loading of the PCI option ROM in
vfio_pci_size_rom(). The memory region is switched to an I/O region
and the PCI attribute 'has_rom' is set but the RAMBlock of the ROM
region is not allocated. When the associated PCI device is deleted,
pci_del_option_rom() calls vmstate_unregister_ram() which tries to
flag a NULL RAMBlock, leading to a SEGV.
It seems that 'has_rom' was set to have memory_region_destroy()
called, but since commit 469b046ead06 ("memory: remove
memory_region_destroy") this is not necessary anymore as the
MemoryRegion is freed automagically.
Remove the PCIDevice 'has_rom' attribute setting in vfio.
Fixes: b895de502717 ("migration: discard non-migratable RAMBlocks") Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: David Hildenbrand <david@redhat.com> Acked-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20180705155811.20366-3-f4bug@amsat.org> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
The macro CMMA_BLOCK_SIZE was defined but not used, and a hardcoded
value was instead used in the code.
This patch fixes the value of CMMA_BLOCK_SIZE and uses it in the
appropriate place in the code, and fixes another case of hardcoded
value in the KVM backend, replacing it with the more appropriate
constant KVM_S390_CMMA_SIZE_MAX.
Peter Maydell [Tue, 10 Jul 2018 16:28:29 +0000 (17:28 +0100)]
Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging
Block layer patches:
- Copy offloading fixes for when the copy increases the image size
- Temporary revert of the removal of deprecated -drive options
- Fix request serialisation in the image fleecing scenario
- Fix copy-on-read crash with unaligned image size
- Fix another drain crash
# gpg: Signature made Tue 10 Jul 2018 16:37:52 BST
# gpg: using RSA key 7F09B272C88F2FD6
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>"
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6
* remotes/kevin/tags/for-upstream: (24 commits)
block: Use common write req handling in truncate
block: Fix bdrv_co_truncate overlap check
block: Use common req handling in copy offloading
block: Use common req handling for discard
block: Fix handling of image enlarging write
block: Extract common write req handling
block: Use uint64_t for BdrvTrackedRequest byte fields
block: Use BdrvChild to discard
block: Add copy offloading trace points
block: Prefix file driver trace points with "file_"
Revert "block: Remove deprecated -drive geometry options"
Revert "block: Remove deprecated -drive option addr"
Revert "block: Remove deprecated -drive option serial"
Revert "block: Remove dead deprecation warning code"
block/blklogwrites: Make sure the log sector size is not too small
qapi/block-core.json: Add missing documentation for blklogwrites log-append option
block/backup: fix fleecing scheme: use serialized writes
block: add BDRV_REQ_SERIALISING flag
block: split flags in copy_range
block/io: fix copy_range
...
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Tue, 10 Jul 2018 15:41:20 +0000 (16:41 +0100)]
Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20180710a' into staging
Migration pull 2018-07-10 (for 3.0)
Migration fixes and migration test fixes, mostly
around postcopy and postcopy recovery
# gpg: Signature made Tue 10 Jul 2018 16:27:19 BST
# gpg: using RSA key 0516331EBC5BFDE7
# gpg: Good signature from "Dr. David Alan Gilbert (RH2) <dgilbert@redhat.com>"
# Primary key fingerprint: 45F5 C71B 4A0C B7FB 977A 9FA9 0516 331E BC5B FDE7
* remotes/dgilbert/tags/pull-migration-20180710a:
migration: reorder MIG_CMD_POSTCOPY_RESUME
tests: hide stderr for postcopy recovery test
tests: add postcopy recovery test
tests: introduce wait_for_migration_status()
tests: introduce migrate_query*() helpers
tests: allow migrate() to take extra flags
tests: introduce migrate_postcopy_* helpers
migration: show pause/recover state on dst host
migration: fix incorrect bitmap size calculation
migration: loosen recovery check when load vm
migration: simplify check to use qemu file buffer
migration: unify incoming processing
migration: unbreak postcopy recovery
migration: move income process out of multifd
migration: delay postcopy paused state
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Truncation is the last to convert from open coded req handling to
reusing helpers. This time the permission check in prepare has to adapt
to the new caller: it checks a different permission bit, and doesn't
trigger the before write notifier.
Also, truncation should always trigger a bs->total_sectors update and in
turn call parent resize_cb. Update the condition in finish helper, too.
It's intended to do a duplicated bs->read_only check before calling
bdrv_co_write_req_prepare() so that we can be more informative with the
error message, as bdrv_co_write_req_prepare() doesn't have Error
parameter.
Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
If we are growing the image and potentially using preallocation for the
new area, we need to make sure that no write requests are made to the
"preallocated" area which is [@old_size, @offset), not
[@offset, offset * 2 - @old_size).
Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This brings the request handling logic inline with write and discard,
fixing write_gen, resize_cb, dirty bitmaps and image size refreshing.
The last of these issues broke iotest case 222, which is now fixed.
Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reuse the new bdrv_co_write_req_prepare/finish helpers. The variation
here is that discard requests don't affect bs->wr_highest_offset, and it
cannot extend the image.
Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Peter Xu [Tue, 10 Jul 2018 09:44:24 +0000 (17:44 +0800)]
migration: reorder MIG_CMD_POSTCOPY_RESUME
It was accidently added before MIG_CMD_PACKAGED so it might break
command compatibility when we run postcopy migration between old/new
QEMUs. Fix that up quickly before the QEMU 3.0 release.
Reported-by: Lukáš Doktor <ldoktor@redhat.com> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180710094424.30754-1-peterx@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Peter Xu [Tue, 10 Jul 2018 09:19:02 +0000 (17:19 +0800)]
tests: hide stderr for postcopy recovery test
We dumped something when network failure happens. We should avoid those
messages to be dumped when running the tests:
$ ./tests/migration-test -p /x86_64/migration/postcopy/recovery
/x86_64/migration/postcopy/recovery: qemu-system-x86_64: check_section_footer: Read section footer failed: -5
qemu-system-x86_64: Detected IO failure for postcopy. Migration paused.
qemu-system-x86_64: Detected IO failure for postcopy. Migration paused.
OK
After the patch:
$ ./tests/migration-test -p /x86_64/migration/postcopy/recovery
/x86_64/migration/postcopy/recovery: OK
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180710091902.28780-11-peterx@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Peter Xu [Tue, 10 Jul 2018 09:19:01 +0000 (17:19 +0800)]
tests: add postcopy recovery test
Test the postcopy recovery procedure by emulating a network failure
using migrate-pause command.
Tested-by: Balamuruhan S <bala24@linux.vnet.ibm.com> Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180710091902.28780-10-peterx@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Peter Xu [Tue, 10 Jul 2018 09:19:00 +0000 (17:19 +0800)]
tests: introduce wait_for_migration_status()
It's generalized from wait_for_migration_complete() to allow us to wait
for any migration status besides failure.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Reviewed-by: Balamuruhan S <bala24@linux.vnet.ibm.com> Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180710091902.28780-9-peterx@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Peter Xu [Tue, 10 Jul 2018 09:18:59 +0000 (17:18 +0800)]
tests: introduce migrate_query*() helpers
Introduce helpers to query migration states and use it.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Reviewed-by: Balamuruhan S <bala24@linux.vnet.ibm.com> Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180710091902.28780-8-peterx@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Peter Xu [Tue, 10 Jul 2018 09:18:58 +0000 (17:18 +0800)]
tests: allow migrate() to take extra flags
For example, we can pass in '"resume": true' to resume a migration.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Reviewed-by: Balamuruhan S <bala24@linux.vnet.ibm.com> Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180710091902.28780-7-peterx@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Peter Xu [Tue, 10 Jul 2018 09:18:57 +0000 (17:18 +0800)]
tests: introduce migrate_postcopy_* helpers
Separate the old postcopy UNIX socket test into three steps, provide a
helper for each step. With these helpers, we can do more compliated
tests like postcopy recovery, while keep the codes shared.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Reviewed-by: Balamuruhan S <bala24@linux.vnet.ibm.com> Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180710091902.28780-6-peterx@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Fix up merge with 2e295789 / Skip tests for ppc tcg
As a mechanical refactoring patch, this is the first step towards
unified and more correct write code paths. This is helpful because
multiple BlockDriverState fields need to be updated after modifying
image data, and it's hard to maintain in multiple places such as copy
offload, discard and truncate.
Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
block: Use uint64_t for BdrvTrackedRequest byte fields
This matches the types used for bytes in the rest parts of block layer.
In the case of bdrv_co_truncate, new_bytes can be the image size which
probably doesn't fit in a 32 bit int.
Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Other I/O functions are already using a BdrvChild pointer in the API, so
make discard do the same. It makes it possible to initiate the same
permission checks before doing I/O, and much easier to share the
helper functions for this, which will be added and used by write,
truncate and copy range paths.
Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
block: Prefix file driver trace points with "file_"
With in one module, trace points usually have a common prefix named
after the module name. paio_submit and paio_submit_co are the only two
trace points so far in the two file protocol drivers. As we are adding
more, having a common prefix here is better so that trace points can be
enabled with a glob. Rename them.
Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Some deprecated -drive options were still in use by libvirt, only
fixed with libvirt commit b340c6c614 ("qemu: format serial and geometry
on frontend disk device"), which is not yet in any released version
of libvirt.
So let's hold off removing the deprecated options for one more QEMU
release.
Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> Signed-off-by: Cornelia Huck <cohuck@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Peter Xu [Tue, 10 Jul 2018 09:18:56 +0000 (17:18 +0800)]
migration: show pause/recover state on dst host
These two states will be missing when doing "query-migrate" on
destination VM. Add these states so that we can get the query results
as expected.
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180710091902.28780-5-peterx@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Peter Xu [Tue, 10 Jul 2018 09:18:55 +0000 (17:18 +0800)]
migration: fix incorrect bitmap size calculation
The calculation on size of received bitmap is incorrect for postcopy
recovery. Here we wanted to let the size to cover all the valid bits in
the bitmap, we should use DIV_ROUND_UP() instead of a division.
For example, a RAMBlock with size=4K (which contains only one single 4K
page) will have nbits=1, then nbits/8=0, then the real bitmap won't be
sent to source at all.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180710091902.28780-4-peterx@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Peter Xu [Tue, 10 Jul 2018 09:18:54 +0000 (17:18 +0800)]
migration: loosen recovery check when load vm
We were checking against -EIO, assuming that it will cover all IO
failures. But actually it is not. One example is that in
qemu_loadvm_section_start_full() we can have tons of places that will
return -EINVAL even if the error is caused by IO failures on the
network.
Let's loosen the recovery check logic here to cover all the error cases
happened by removing the explicit check against -EIO. After all we
won't lose anything here if any other failure happened.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180710091902.28780-3-peterx@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Peter Xu [Tue, 10 Jul 2018 09:18:53 +0000 (17:18 +0800)]
migration: simplify check to use qemu file buffer
Firstly, renaming the old matching_page_sizes variable to
matches_target_page_size, which suites more to what it did (it only
checks against target page size rather than multiple page sizes).
Meanwhile, simplify the check logic a bit, and enhance the comments.
Should have no functional change.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180710091902.28780-2-peterx@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Peter Xu [Wed, 27 Jun 2018 13:22:46 +0000 (21:22 +0800)]
migration: unify incoming processing
This is the 2nd patch to unbreak postcopy recovery.
Let's unify the migration_incoming_process() call at a single place
rather than calling it in connection setup codes. This fixes a problem
that we will go into incoming migration procedure even if we are trying
to recovery from a paused postcopy migration.
Fixes: 36c2f8be2c ("migration: Delay start of migration main routines") Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180627132246.5576-5-peterx@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Peter Xu [Wed, 27 Jun 2018 13:22:45 +0000 (21:22 +0800)]
migration: unbreak postcopy recovery
The whole postcopy recovery logic was accidentally broken. We need to
fix it in two steps.
This is the first step that we should do the recovery when needed. It
was bypassed before after commit 36c2f8be2c.
Introduce postcopy_try_recovery() helper for the postcopy recovery
logic. Call it both in migration_fd_process_incoming() and
migration_ioc_process_incoming().
Fixes: 36c2f8be2c ("migration: Delay start of migration main routines") Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180627132246.5576-4-peterx@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Peter Xu [Wed, 27 Jun 2018 13:22:44 +0000 (21:22 +0800)]
migration: move income process out of multifd
Move the call to migration_incoming_process() out of multifd code. It's
a bit strange that we can migration generic calls in multifd code.
Instead, let multifd_recv_new_channel() return a boolean showing whether
it's ready to continue the incoming migration.
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180627132246.5576-3-peterx@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Peter Xu [Wed, 27 Jun 2018 13:22:43 +0000 (21:22 +0800)]
migration: delay postcopy paused state
Before this patch we firstly setup the postcopy-paused state then we
clean up the QEMUFile handles. That can be racy if there is a very fast
"migrate-recover" command running in parallel. Fix that up.
Reported-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180627132246.5576-2-peterx@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Ari Sundholm [Fri, 6 Jul 2018 12:00:38 +0000 (15:00 +0300)]
block/blklogwrites: Make sure the log sector size is not too small
The sector size needs to be large enough to accommodate the data
structures for the log super block and log write entries. This was
previously not properly checked, which made it possible to cause
QEMU to badly misbehave.
Signed-off-by: Ari Sundholm <ari@tuxera.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
block/backup: fix fleecing scheme: use serialized writes
Fleecing scheme works as follows: we want a kind of temporary snapshot
of active drive A. We create temporary image B, with B->backing = A.
Then we start backup(sync=none) from A to B. From this point, B reads
as point-in-time snapshot of A (A continues to be active drive,
accepting guest IO).
This scheme needs some additional synchronization between reads from B
and backup COW operations, otherwise, the following situation is
theoretically possible:
(assume B is qcow2, client is NBD client, reading from B)
1. client starts reading and take qcow2 mutex in qcow2_co_preadv, and
goes up to l2 table loading (assume cache miss)
2) guest write => backup COW => qcow2 write =>
try to take qcow2 mutex => waiting
3. l2 table loaded, we see that cluster is UNALLOCATED, go to
"case QCOW2_CLUSTER_UNALLOCATED" and unlock mutex before
bdrv_co_preadv(bs->backing, ...)
4) aha, mutex unlocked, backup COW continues, and we finally finish
guest write and change cluster in our active disk A
5. actually, do bdrv_co_preadv(bs->backing, ...) and read
_new updated_ data.
To avoid this, let's make backup writes serializing, to not intersect
with reads from B.
Note: we expand range of handled cases from (sync=none and
B->backing = A) to just (A in backing chain of B), to finally allow
safe reading from B during backup for all cases when A in backing chain
of B, i.e. B formally looks like point-in-time snapshot of A.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Serialized writes should be used in copy-on-write of backup(sync=none)
for image fleecing scheme.
We need to change an assert in bdrv_aligned_pwritev, added in 28de2dcd88de. The assert may fail now, because call to
wait_serialising_requests here may become first call to it for this
request with serializing flag set. It occurs if the request is aligned
(otherwise, we should already set serializing flag before calling
bdrv_aligned_pwritev and correspondingly waited for all intersecting
requests). However, for aligned requests, we should not care about
outdating of previously read data, as there no such data. Therefore,
let's just update an assert to not care about aligned requests.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Pass read flags and write flags separately. This is needed to handle
coming BDRV_REQ_NO_SERIALISING clearly in following patches.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
On each recursion step, we go to the child of src or dst, only for one
of them. So, it's wrong to create tracked requests for both on each
step. It leads to tracked requests duplication.
2. Wait for serializing requests on write path independently of
BDRV_REQ_NO_SERIALISING
Before commit 9ded4a01149 "backup: Use copy offloading",
BDRV_REQ_NO_SERIALISING was used for only one case: read in
copy-on-write operation during backup. Also, the flag was handled only
on read path (in bdrv_co_preadv and bdrv_aligned_preadv).
After 9ded4a01149, flag is used for not waiting serializing operations
on backup target (in same case of copy-on-write operation). This
behavior change is unsubstantiated and potentially dangerous, let's
drop it and add additional asserts and documentation.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Peter Maydell [Mon, 9 Jul 2018 12:45:35 +0000 (13:45 +0100)]
target/arm: Use correct mmu_idx for exception-return unstacking
For M-profile exception returns, the mmu index to use for exception
return unstacking is supposed to be that of wherever we are returning to:
* if returning to handler mode, privileged
* if returning to thread mode, privileged or unprivileged depending on
CONTROL.nPRIV for the destination security state
We were passing the wrong thing as the 'priv' argument to
arm_v7m_mmu_idx_for_secstate_and_priv(). The effect was that guests
which programmed the MPU to behave differently for privileged and
unprivileged code could get spurious MemManage Unstack exceptions.
Reported-by: Adithya Baglody <adithya.nagaraj.baglody@intel.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20180709124535.1116-1-peter.maydell@linaro.org
* remotes/vivier2/tags/linux-user-for-3.0-pull-request:
linux-user: Report error message on stderr, rather than stdout
linux-user: Do not report "syscall not implemented" by default
linux-user: Do not report "Unsupported syscall" by default
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Kevin Wolf [Fri, 6 Jul 2018 16:41:07 +0000 (18:41 +0200)]
block: Fix copy-on-read crash with partial final cluster
If the virtual disk size isn't aligned to full clusters,
bdrv_co_do_copy_on_readv() may get pnum == 0 before having the full
cluster completed, which will let it run into an assertion failure:
Check for EOF, assert that we read at least as much as the read request
originally wanted to have (which is true at EOF because otherwise
bdrv_check_byte_request() would already have returned an error) and
return success early even though we couldn't copy the full cluster.
Kevin Wolf [Fri, 29 Jun 2018 16:01:31 +0000 (18:01 +0200)]
block: Poll after drain on attaching a node
Commit dcf94a23b1 ('block: Don't poll in parent drain callbacks')
removed polling in bdrv_child_cb_drained_begin() on the grounds that the
original bdrv_drain() already will poll and BdrvChildRole.drained_begin
calls must not cause graph changes (and therefore must not call
aio_poll() or the recursion through the graph will break.
This reasoning is correct for calls through bdrv_do_drained_begin().
However, BdrvChildRole.drained_begin is also called when a node that is
already in a drained section (i.e. bdrv_do_drained_begin() has already
returned and therefore can't poll any more) is attached to a new parent.
In this case, we must explicitly poll to have all requests completed
before the drained new child can be attached to the parent.
In bdrv_replace_child_noperm(), we know that we're not inside the
recursion of bdrv_do_drained_begin() because graph changes are not
allowed there, and bdrv_replace_child_noperm() is a graph change. The
call of BdrvChildRole.drained_begin() must therefore be followed by a
BDRV_POLL_WHILE() that waits for the completion of requests.
Reported-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Peter Maydell [Mon, 9 Jul 2018 20:31:40 +0000 (21:31 +0100)]
Merge remote-tracking branch 'remotes/ehabkost/tags/machine-next-pull-request' into staging
Machine/NUMA fixes for -rc0
* Properly free device_memory at machine_finalize()
* Fix implicit NUMA initialization regression (for machines with
auto_enable_numa_with_memhp=true)
# gpg: Signature made Mon 09 Jul 2018 18:40:38 BST
# gpg: using RSA key 2807936F984DC5A6
# gpg: Good signature from "Eduardo Habkost <ehabkost@redhat.com>"
# Primary key fingerprint: 5A32 2FD5 ABC4 D3DB ACCF D1AA 2807 936F 984D C5A6
* remotes/ehabkost/tags/machine-next-pull-request:
hw/machine: Remove the Zero check of nb_numa_nodes for numa_complete_configuration()
machine: properly free device_memory
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20180705151515.779173-1-vsementsov@virtuozzo.com Signed-off-by: Max Reitz <mreitz@redhat.com>
Max Reitz [Mon, 2 Jul 2018 21:07:20 +0000 (23:07 +0200)]
vmdk: Fix possible segfault with non-VMDK backing
VMDK performs a probing check in vmdk_co_create_opts() to prevent the
user from assigning non-VMDK files as a backing file, because it only
supports VMDK backing files. However, with the @backing runtime option,
it is possible to assign arbitrary nodes as backing nodes, regardless of
what the image header says. Therefore, VMDK may not just access backing
nodes assuming they are VMDK nodes -- which it does, because it needs to
compare the backing file's CID with the overlay's parentCID value, and
naturally the backing file only has a CID when it's a VMDK file.
Instead, it should report the CID of non-VMDK backing files not to match
the overlay because clearly a non-present CID does not match.
Without this change, vmdk_read_cid() reads from the backing file's
bs->file, which may be NULL (in which case we get a segfault). Also, it
interprets bs->opaque as a BDRVVmdkState and then reads from the
.desc_offset field, which usually will just return some arbitrary value
which then results in either garbage to be read, or bdrv_pread() to
return an error, both of which result in a non-matching CID to be
reported.
(In a very unlikely case, we could read something that looks like a
VMDK descriptor, and then get a CID which might actually match. But
that is highly unlikely, and the only result would be that VMDK accepts
the backing file which is not too bad (albeit unintentional).)
((And in theory, the seek to .desc_offset might leak data from another
block driver's opaque object. But then again, the user should realize
very quickly that a non-VMDK backing file does not work (because the
read will very likely fail, due to the reasons given above), so this
should not be exploitable.))
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20180702210721.4847-2-mreitz@redhat.com Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
projects / mirror_qemu.git / log