Firstly the function misses dmaas checking. This patch adds it.
Secondly the function uses s390_pci_find_dev_by_fh() to look up the
zpci device. This may fail if the guest provides a valid and disabled
fh but fh of the associated zpci device is enabled. Thus we use
s390_pci_find_dev_by_idx() instead.
Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Because of the refactor of s390_pci_find_dev_by_idx(), list_pci()
should be updated. We introduce a new function to get the next
available zpci device. It simplifies the code of looking up zpci
devices.
Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Acked-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
s390_find_dev_by_idx() only indexes usable zpci devices. It implies
that the index value of each zpci device is dynamic and may change if
a new zpci device is plugged. So we have to use a constant index to
look up the device.
Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
The code in CLP_SET_PCI_FN case misses some checkings. Let's add
them.
Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
We need to support hot-plug/hot-unplug for the new zpci devices as
well. This patch enables the present hot-plug/hot-unplug handlers
to support not only generic pci devices but also zpci devices.
Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
The uid-checking facility guarantees uniqueness of the uid within the
vm and exposes the real uid to the guest when listing pci devices.
Let's always enable it and present it to the guest in the response to
the list pci clp command.
Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
To support definitions of s390 pci attributes in Qemu cmdline, we have
to make current S390PCIBusDevice struct inherit DeviceState and add
three properties for it. Currently we only support definitions of uid
and fid.
'uid' is optionally defined by users, identifies a zpci device and
must be defined with a 16-bit and non-zero unique value.
'fid' ranges from 0x0 to 0xFFFFFFFF. For fid property, we introduce a
new PropertyInfo by the name of s390_pci_fid_propinfo with our special
setter and getter. As 'fid' is optional, introduce 'fid_defined' to
track whether the user specified a fid.
'target' field is to direct qemu to find the corresponding generic PCI
device. It is equal to the 'id' value of one of generic pci devices.
If the user doesn't specify 'id' parameter for a generic pci device,
its 'id' value will be generated automatically and use this value as
'target' to create an associated zpci device.
If the user did not specify 'uid' or 'fid', values are generated
automatically. 'target' is required.
In addition, if a pci device has no associated zpci device, the code
will generate a zpci device automatically for it.
Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Currently each zpci device holds its own DMA address space and memory
region. At the same time, all instances of zpci device are stored in
S390pciState. So duirng the initialization of S390pciState, all zpci
devices are created and then all DMA address spaces are created. Thus,
when initializing pci devices, their corresponding DMA address spaces
could be found.
But zpci qdev will be introduced later. Zpci device may be initialized
and plugged afterwards generic pci device. So we should initialize all
DMA address spaces and memory regions before initializing zpci devices.
We introduce a new struct named S390PCIIOMMU. And a new field of
S390pciState, which is an array to store all instances of S390PCIIOMMU,
is added so that qemu pci code could find the corresponding DMA
address space when initializing a generic pci device. And this should
be done before the connection of a zpci device and a generic pci
device is built.
Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Acked-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
To enable S390PCIBusDevice as qdev, there should be a new bus to
plug and manage all instances of S390PCIBusDevice. Due to this,
S390PCIBus is introduced.
Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Current code uses some fields combinatorially to indicate the state of
a s390 pci device. This patch introduces device states in order to make
the code more readable and more logical.
Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Because this function is called very frequently, we should use a more
effective way to find the zpci device. So we use the FH's index to get
the device directly.
Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Present code uses some macros to structure PCI Function Handle. But
their names don't have a uniform format. Let's use FH_MASK_ as the
unified prefix.
While we're at it, differentiate the SHM bits: use different bits for
vfio and emulated devices.
Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
We forgot to write the fid; fix that.
Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Acked-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
There are a number of places where the code needs to get the instance
of S390pciState. It calls object_resolve_path() every time. This
wastes a lot of time and leads to low performance. Thus we add
s390_get_phb() to improve it.
Because we always have a phb, we remove all return checkings in the
callers and add an assert in s390_get_phb() to make sure that phb is
getted successfully.
Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
In commit d78c19b5cf, vfio code stores
the IOMMU's offset_within_address_space and adjusts the IOVA before
calling vfio_dma_map/vfio_dma_unmap. But s390_translate_iommu already
considers the base address of an IOMMU memory region.
Thus we use pal as the size and 0x0 as the base address to initialize
IOMMU memory subregion.
Signed-off-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
The previous patch moved virtual css bridge and bus out from
virtio-ccw, but kept the direct reference of virtio-ccw specific
unplug function inside css-bridge.c.
To make the virtual css bus and bridge useful for non-virtio devices,
this introduces a common unplug function pointer "unplug" to call
specific virtio-ccw unplug parts. Thus, the tight coupling to
virtio-ccw can be removed.
This unplug pointer is a member of CCWDeviceClass, which is introduced
as an abstract device layer called "ccw-device". This layer is between
DeviceState and specific devices which are plugged in virtual css bus,
like virtio-ccw device. The specific unplug handlers should be assigned
to "unplug" during initialization.
Signed-off-by: Jing Liu <liujbjl@linux.vnet.ibm.com>
Reviewed-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
Reviewed-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Currently, common base layers virtual css bridge and bus are
defined in hw/s390x/virtio-ccw.c(h). In order to support
multiple types of devices in the virtual channel subsystem,
especially non virtio-ccw, refactoring work needs to be done.
This work is just a pure code move without any functional change
except dropping an empty function virtual_css_bridge_init() and
virtio_ccw_busdev_unplug() changing. virtio_ccw_busdev_unplug()
is specific to virtio-ccw but gets referenced from the common
virtual css bridge code. To keep the functional changes to a
minimum we export this function from virtio-ccw.c and continue
to reference it inside virtual_css_bridge_class_init()
(now living in hw/s390x/css-bridge.c). A follow-up patch will
clean this up.
Signed-off-by: Jing Liu <liujbjl@linux.vnet.ibm.com>
Reviewed-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Introduce a TYPE_* define (like we already use for a couple of other
QOM types) for the name of the virtual CSS bridge QOM type instead of
sprinkling the same string literal over several source files.
Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
A lot of what virtio_ccw_device_realize() does isn't specific to
virtio; it would apply to emulated CCW as well. Factor it out to make
it easier to implement emulated CCW devices later on.
Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
Reviewed-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
When migrating from a different QEMU version, the start_address and
bios_start_address may differ. During migration these values are migrated
and overwrite the values that were detected by QEMU itself.
On a reboot, QEMU will reload its own BIOS, but use the migrated start
addresses, which does not work if the values differ.
Fix this by not relying on the migrated values anymore, but still
provide them during migration, so existing QEMUs continue to work.
Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
If bootindex is specified for a device, we need to IPL from
it. Currently it works for ccw devices, but not for SCSI. To be able to
IPL from the specific device, pc-bios needs to know its address.
For this reason we add special QEMU_SCSI IPL type into the IPLB
structure, that contains the scsi device address.
We enhance the ipl block with a currently qemu-only parameter block
that allows us to specify a concrete scsi device.
Signed-off-by: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
Reviewed-by: Eric Farman <farman@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
There is ,bootindex=%d argument to specify the lookup order of
boot devices.
If a bootindex assigned to the device, then IPL Parameter Info Block
is created for that device when it is IPLed from.
If it is a mere SCSI device (not FCP), then IPIB is created with a
special SCSI type and its fields are used to store SCSI address of the
device. This new ipl block is private to qemu for now.
If the device to IPL from is specified this way, then SCSI bus lookup
is bypassed and prescribed devices uses the address specified.
Signed-off-by: Eugene (jno) Dvurechenski <jno@linux.vnet.ibm.com>
Signed-off-by: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
* fix a wrong variable type for A64 SYS_HEAPINFO semihosting call
* xlnx_dp: fix iffy xlnx_dp_aux_push_tx_fifo
* aux: fix break that wanted to break two levels out
* aux: Rename aux.[ch] to auxbus.[ch] for the benefit of Windows
* hw/block/m25p80: fix resource leak
* i.MX: split the GPT timer implementation into per SOC definitions
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQIcBAABCAAGBQJXfl2ZAAoJEDwlJe0UNgzeQx0P/15MNaL0oWCEAqowiqr5dNb1
NTFE41NZprRmUcLIGzL28xekWLKZyZRawAoQJbFFTFhm6dkYdQuq1p9Y12nM+SS4
rUPe3CjUNaK0JC1A2ObKuuP7891C5/gBP1m67p0rVkhmsxZpPhmftuDSPRNT3XaQ
2y4uyEr+YCoCA684VkOMgqx+Nw9v1mZaDcbjPoQ97U00+wJ9I9lh5Z3jITV/XpkD
YWESCC6E9g158tZmcmiaCOe+OCfLqM4mEHK/b6pq82SfMG5m0MRuDw0LTo6oOO2f
T8UW+WPVhaOUQARYQls93jkrxlVbmf3xZySLUPasEMdUQSdCmy0nVdAxh/w78oxV
sNGG+TC9XPPGRZiAfp96ccYSqFNzYUT5GYqpycJ+dyc2FzHHfQdez/sYtGsv75cF
4V/OHHeO+msNj8J5wYhKZTbrBnCPpB/CFxh5tl12YYZDDba/voTgeZt6D259gzVa
xtvab7dGjbCfs5T5+tbaPhI155DBzhwvTOEfl4JKMRSJeyK3F75iOR/TFXJbzFgO
NBZpkKWeJaX5zrPFPgwYtI8KyLZtzSgy8c1xYmAKMiqY6f2Hl/E2NMKfIFOUb8h8
apFG2KAdCkSW9qft2KUExfEdhOmn2ZWrjBxqxLCRbbZK5RnL8+UTEytmZymbmEoi
QSNtyWgD2hIXILlUfh+S
=uKU5
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20160707' into staging
target-arm queue:
* fix a wrong variable type for A64 SYS_HEAPINFO semihosting call
* xlnx_dp: fix iffy xlnx_dp_aux_push_tx_fifo
* aux: fix break that wanted to break two levels out
* aux: Rename aux.[ch] to auxbus.[ch] for the benefit of Windows
* hw/block/m25p80: fix resource leak
* i.MX: split the GPT timer implementation into per SOC definitions
# gpg: Signature made Thu 07 Jul 2016 14:48:09 BST
# gpg: using RSA key 0x3C2525ED14360CDE
# gpg: Good signature from "Peter Maydell <peter.maydell@linaro.org>"
# gpg: aka "Peter Maydell <pmaydell@gmail.com>"
# gpg: aka "Peter Maydell <pmaydell@chiark.greenend.org.uk>"
# Primary key fingerprint: E1A5 C593 CD41 9DE2 8E83 15CF 3C25 25ED 1436 0CDE
* remotes/pmaydell/tags/pull-target-arm-20160707:
i.MX: split the GPT timer implementation into per SOC definitions
hw/block/m25p80: fix resource leak
aux: Rename aux.[ch] to auxbus.[ch] for the benefit of Windows
aux: fix break that wanted to break two levels out
xlnx_dp: fix iffy xlnx_dp_aux_push_tx_fifo
target-arm/arm-semi.c: In SYS_HEAPINFO use correct type for 'limit'
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
In various Freescale SOCs, the GPT timers can be configured to select
its input clock.
Depending on the SOC the set of available input clocks may vary.
The actual single GPT definition was no good enough and because of it
booting the sabrelite board with a i.MX6DL device tree would fail
because of an incorrect input clock definition for the i.MX6DL SOC.
This patch fixes the i.MX6DL boot failure by adding the ability to
define a different set of input clocks depending on the considered SOC.
A different class has been defined for i.MX25, i.MX31 and i.MX6 each with
its specific set of input clocks.
The patch has been tested by booting KZM, i.MX25 PDK, i.MX6Q sabrelite
and i.MX6DL sabrelite.
Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
Message-id: 1467325619-8374-1-git-send-email-jcd@tribudubois.net
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
[PMM: fixed spacing round '/' operator]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
These two are spot by Coverity 1357232 and 1357233.
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Message-id: 1467684998-12076-1-git-send-email-zhaoshenglong@huawei.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
On Windows 'aux.*' is a reserved name and cannot be used for
filenames; see
https://msdn.microsoft.com/en-gb/library/windows/desktop/aa365247(v=vs.85).aspx
This prevents cloning the QEMU git repo on Windows:
C:\Java\sources\kvm> git clone https://github.com/qemu/qemu.git
Cloning into 'qemu'...
remote: Counting objects: 279563, done.
remote: Total 279563 (delta 0), reused 0 (delta 0), pack-reused 279563R
Receiving objects: 100% (279563/279563), 122.45 MiB | 3.52 MiB/s, done.
Resolving deltas: 100% (221942/221942), done.
Checking connectivity... done.
error: unable to create file hw/misc/aux.c (No such file or directory)
error: unable to create file include/hw/misc/aux.h (No such file or directory)
Checking out files: 100% (4795/4795), done.
fatal: unable to checkout working tree
warning: Clone succeeded, but checkout failed.
You can inspect what was checked out with 'git status'
and retry the checkout with 'git checkout -f HEAD'
(bug https://bugs.launchpad.net/bugs/1595240)
Rename the offending files for the benefit of Windows.
Reported-by: Алексей Курган <akurgan@yandex.ru>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Wei Huang <wei@redhat.com>
Tested-by: KONRAD Frederic <fred.konrad@greensocs.com>
Message-id: 1467377145-32385-1-git-send-email-peter.maydell@linaro.org
The last "ret = AUX_I2C_NACK;" is dead, because it is always overridden
by AUX_I2C_ACK. What really the code wants is to jump out of the switch
statement, and a "return" will not cut it because it would omit a debug
printf.
Change the logic so that we can break out of the while loop. For clarity,
hoist the bus->last_* assignments up, right after i2c_start_transfer.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
xlnx_dp_aux_push_tx_fifo takes an immediate uint8_t and a buffer length,
which must be 1 because that is how many uint8_t's fit in a uint8_t.
Sure enough, that is what xlnx_dp_write passes to it, but the function
is just weird. Therefore, make xlnx_dp_aux_push_tx_fifo look like
xlnx_dp_aux_push_rx_fifo, taking a pointer to the buffer.
Reported by Coverity.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
In commit f5666418c4 most of the SYS_HEAPINFO implementation was
fixed to use target_ulong rather than uint32_t, but the 'limit'
variable was not changed.
Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1467650942-28706-1-git-send-email-peter.maydell@linaro.org
Version: GnuPG v1
iQEcBAABAgAGBQJXffbYAAoJEO8Ells5jWIRZnAH/2bleryOC4i6Pwz7cNpX07MQ
YmJ97K6RueVcYjj7pSi4l8nzv18K0k43x04ST6WIpHbRf07RIJIBOVn/f8xUnNo/
EonhPgYWZXZX7h9EWnjUWccKU8ReQO06lUi2H7XX6f6Ba3maG2A4HCvhraE+noIr
pkM4Qj6niD+i6hZsznmhuLWUE+ynt2JdmBIxxG+e12t/Hq5vW7IYPj6Y1IQia9r9
vkz2wWmmiVTty6g5d/pu8yyWt+b2ob8cQ0OoRrqr1uNF/h4uWcUlgUZEe/DJER3M
u+s5xVKLM4FK+w0grBKMCDzmRsjixV42tcS0H8LABpdRce8PINOmpsO3QI+Ul5g=
=YU37
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging
# gpg: Signature made Thu 07 Jul 2016 07:29:44 BST
# gpg: using RSA key 0xEF04965B398D6211
# gpg: Good signature from "Jason Wang (Jason Wang on RedHat) <jasowang@redhat.com>"
# gpg: WARNING: This key is not certified with sufficiently trusted signatures!
# gpg: It is not certain that the signature belongs to the owner.
# Primary key fingerprint: 215D 46F4 8246 689E C77F 3562 EF04 965B 398D 6211
* remotes/jasowang/tags/net-pull-request:
tap: vhost busy polling support
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This patch add the capability of basic vhost net busy polling which is
supported by recent kernel. User could configure the maximum number of
us that could be spent on busy polling through a new property of tap
"poll-us".
Cc: Greg Kurz <groug@kaod.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQIcBAABAgAGBQJXfMjDAAoJEDhwtADrkYZTyKwP/i5Lva3qhDk9AJhvPNSbnps/
sxVGVkUTeUM0UoXdKmDChkk2zScv2AOVxL3oibb22PmU1MvDU+XoKYokae/VucVS
WL16t/Z77QxSJ1yKhXT3i0P7+9sR9Gq2eDHchSSITLreVO8/jNb7u1JjnV4nPyQP
Scg6sCCnfgSw8W4EOz0wDhwXHbRv7obSY0lGUD1R6FifETHQ98rTpII0BS1AyvbF
HIJMcGpirAe62BGNQeJv933mrD1EgRfBytQAkEL/l6DDlz/xk9AE26bsjdP5Xa9A
MwEBje9CPm0ICleqq1ZVKNQQqng2PVfXXXdJYKQKVnIHL+REzRDKbTGTlS4njdVi
v2i4E7MwDPIf86zRUXJ6eUkWDHgld4of2WZmoxNPZmWWmHMb31iqGcb0//mYmP06
QOPd/uxpxg9pze4JeQMhzHCbFum2/s43cyqaH24lrwhTw3EsI45t4sgJQTG1NIFt
ZdPXVwNBfa6ur50dsRWHYonQimpofW94YYupWLggiVGKTzvfSWVQJGc9Ho65P23y
ZJrkbN3bWNnwVFzIbWFI5TKnE/TsnWvTPwk+9cGSP4MKf1sy6t5nIycoTiP3vC9x
PesENYYKy5ZkOkKQpC6lGwjkMl24ii3lVl5g4vWXXjtZHwGNmq/7/UJ7Dm4htqIk
B5gm5gQ/2Tul4Dihlteb
=BgpV
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2016-07-06' into staging
QAPI patches for 2016-07-06
# gpg: Signature made Wed 06 Jul 2016 10:00:51 BST
# gpg: using RSA key 0x3870B400EB918653
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>"
# gpg: aka "Markus Armbruster <armbru@pond.sub.org>"
# Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867 4E5F 3870 B400 EB91 8653
* remotes/armbru/tags/pull-qapi-2016-07-06:
replay: Use new QAPI cloning
sockets: Use new QAPI cloning
qapi: Add new clone visitor
qapi: Add new visit_complete() function
tests: Factor out common code in qapi output tests
tests: Clean up test-string-output-visitor
qmp-output-visitor: Favor new visit_free() function
string-output-visitor: Favor new visit_free() function
qmp-input-visitor: Favor new visit_free() function
string-input-visitor: Favor new visit_free() function
opts-visitor: Favor new visit_free() function
qapi: Add new visit_free() function
qapi: Add parameter to visit_end_*
qemu-img: Don't leak errors when outputting JSON
qapi: Improve use of qmp/types.h
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Rather than rolling our own clone via an expensive conversion
in and back out of QObject, use the new clone visitor.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-16-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Rather than rolling our own clone via an expensive conversion
in and back out of QObject, use the new clone visitor.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-15-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We have a couple places in the code base that want to deep-clone
one QAPI object into another, and they were resorting to serializing
the struct out to QObject then reparsing it. A much more efficient
version can be done by adding a new clone visitor.
Since cloning is still relatively uncommon, expose the use of the
new visitor via a QAPI_CLONE() macro that takes care of type-punning
the underlying function pointer, rather than generating lots of
unused functions for types that won't be cloned. And yes, we're
relying on the compiler treating all pointers equally, even though
a strict C program cannot portably do so - but we're not the first
one in the qemu code base to expect it to work (hello, glib!).
The choice of adding a fourth visitor type deserves some explanation.
On the surface, the clone visitor is mostly an input visitor (it
takes arbitrary input - in this case, another QAPI object - and
creates a new QAPI object during the course of the visit). But
ever since commit da72ab0 consolidated enum visits based on the
visitor type, using VISITOR_INPUT would cause us to run
visit_type_str(), even though for cloning there is nothing to do
(we just copy the enum value across, without regards to its mapping
to strings). Also, since our input happens to be a QAPI object,
we can also satisfy the internal checks for VISITOR_OUTPUT. So in
the end, I settled with a new VISITOR_CLONE, and chose its value
such that many internal checks can use 'v->type & mask', sticking
to 'v->type == value' where the difference matters.
Note that we can only clone objects (including alternates) and lists,
not built-ins or enums. The visitor core hides integer width from
the actual visitor (since commit 04e070d), and as long as that's the
case, we can't clone top-level integers. Then again, those can
always be cloned by direct copy, since they are not objects with
deep pointers, so it's no real loss. And restricting cloning to
just objects and lists is cleaner than restricting it to non-integers.
As such, I documented that the clone visitor is for direct use only
by code internal to QAPI, and should not be used on incomplete objects
(other than a hack to work around the fact that we allow NULL in place
of "" in visit_type_str() in other output visitors). Note that as
written, the clone visitor will never fail on a complete object.
Scalars (including enums) not at the root of the clone copy just fine
with no additional effort while visiting the scalar, by virtue of a
g_memdup() each time we push another struct onto the stack. Cloning
a string requires deduplication of a pointer, which means it can also
provide the guarantee of an input visitor of never producing NULL
even when still accepting NULL in place of "" the way the QMP output
visitor does.
Cloning an 'any' type could be possible by incrementing the QObject
refcnt, but it's not obvious whether that is better than implementing
a QObject deep clone. So for now, we document it as unsupported,
and intentionally omit the .type_any() callback to let a developer
know their usage needs implementation.
Add testsuite coverage for several different clone situations, to
ensure that the code is working. I also tested that valgrind was
happy with the test.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-14-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Making each output visitor provide its own output collection
function was the only remaining reason for exposing visitor
sub-types to the rest of the code base. Add a polymorphic
visit_complete() function which is a no-op for input visitors,
and which populates an opaque pointer for output visitors. For
maximum type-safety, also add a parameter to the output visitor
constructors with a type-correct version of the output pointer,
and assert that the two uses match.
This approach was considered superior to either passing the
output parameter only during construction (action at a distance
during visit_free() feels awkward) or only during visit_complete()
(defeating type safety makes it easier to use incorrectly).
Most callers were function-local, and therefore a mechanical
conversion; the testsuite was a bit trickier, but the previous
cleanup patch minimized the churn here.
The visit_complete() function may be called at most once; doing
so lets us use transfer semantics rather than duplication or
ref-count semantics to get the just-built output back to the
caller, even though it means our behavior is not idempotent.
Generated code is simplified as follows for events:
|@@ -26,7 +26,7 @@ void qapi_event_send_acpi_device_ost(ACP
| QDict *qmp;
| Error *err = NULL;
| QMPEventFuncEmit emit;
|- QmpOutputVisitor *qov;
|+ QObject *obj;
| Visitor *v;
| q_obj_ACPI_DEVICE_OST_arg param = {
| info
|@@ -39,8 +39,7 @@ void qapi_event_send_acpi_device_ost(ACP
|
| qmp = qmp_event_build_dict("ACPI_DEVICE_OST");
|
|- qov = qmp_output_visitor_new();
|- v = qmp_output_get_visitor(qov);
|+ v = qmp_output_visitor_new(&obj);
|
| visit_start_struct(v, "ACPI_DEVICE_OST", NULL, 0, &err);
| if (err) {
|@@ -55,7 +54,8 @@ void qapi_event_send_acpi_device_ost(ACP
| goto out;
| }
|
|- qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov));
|+ visit_complete(v, &obj);
|+ qdict_put_obj(qmp, "data", obj);
| emit(QAPI_EVENT_ACPI_DEVICE_OST, qmp, &err);
and for commands:
| {
| Error *err = NULL;
|- QmpOutputVisitor *qov = qmp_output_visitor_new();
| Visitor *v;
|
|- v = qmp_output_get_visitor(qov);
|+ v = qmp_output_visitor_new(ret_out);
| visit_type_AddfdInfo(v, "unused", &ret_in, &err);
|- if (err) {
|- goto out;
|+ if (!err) {
|+ visit_complete(v, ret_out);
| }
|- *ret_out = qmp_output_get_qobject(qov);
|-
|-out:
| error_propagate(errp, err);
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-13-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Create a new visitor_get() function to capture common
actions taken in collecting output from an output visitor,
to make it easier to refactor the output visitors in a
later patch.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-12-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Use &error_abort and error_free_or_abort() in more places, use
the generated qapi_free_intList() instead of open-coding it,
reduce the scope of some variables, avoid code duplication
during test setup with visitor_output_setup_internal(), and
copy the visitor_reset() concept from the qmp-output test to
the string-output test.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-11-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Now that we have a polymorphic visit_free(), we no longer need
qmp_output_visitor_cleanup(); however, we still need to
expose the subtype for qmp_output_get_qobject().
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-10-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Now that we have a polymorphic visit_free(), we no longer need
string_output_visitor_cleanup(); however, we still need to
expose the subtype for string_output_get_string().
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-9-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Now that we have a polymorphic visit_free(), we no longer need
qmp_input_visitor_cleanup(); which in turn means we no longer
need to return a subtype from qmp_input_visitor_new() nor a
public upcast function.
Generated code changes to qmp-marshal.c look like:
|@@ -52,11 +52,10 @@ void qmp_marshal_add_fd(QDict *args, QOb
| {
| Error *err = NULL;
| AddfdInfo *retval;
|- QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true);
| Visitor *v;
| q_obj_add_fd_arg arg = {0};
|
|- v = qmp_input_get_visitor(qiv);
|+ v = qmp_input_visitor_new(QOBJECT(args), true);
| visit_start_struct(v, NULL, NULL, 0, &err);
| if (err) {
| goto out;
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-8-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Now that we have a polymorphic visit_free(), we no longer need
string_input_visitor_cleanup(); which in turn means we no longer
need to return a subtype from string_input_visitor_new() nor a
public upcast function.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-7-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Now that we have a polymorphic visit_free(), we no longer need
opts_visitor_cleanup(); which in turn means we no longer need
to return a subtype from opts_visitor_new() nor a public upcast
function.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-6-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Making each visitor provide its own (awkwardly-named) FOO_cleanup()
is unusual, when we can instead have a polymorphic visit_free()
interface. Over the next few patches, we can use the polymorphic
functions to eliminate the need for a FOO_get_visitor() function
for accessing specific visitor functionality, once everything can
be accessed directly through the Visitor* interfaces.
The dealloc visitor is the first one converted to completely use
the new entry point, since qapi_dealloc_visitor_cleanup() was the
only reason that qapi_dealloc_get_visitor() existed, and only
generated and testsuite code was even using it. With the new
visit_free() entry point in place, we no longer need to expose
the QapiDeallocVisitor subtype through qapi_dealloc_visitor_new(),
and can get by with less generated code, with diffs that look like:
| void qapi_free_ACPIOSTInfo(ACPIOSTInfo *obj)
| {
|- QapiDeallocVisitor *qdv;
| Visitor *v;
|
| if (!obj) {
| return;
| }
|
|- qdv = qapi_dealloc_visitor_new();
|- v = qapi_dealloc_get_visitor(qdv);
|+ v = qapi_dealloc_visitor_new();
| visit_type_ACPIOSTInfo(v, NULL, &obj, NULL);
|- qapi_dealloc_visitor_cleanup(qdv);
|+ visit_free(v);
|}
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-5-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Rather than making the dealloc visitor track of stack of pointers
remembered during visit_start_* in order to free them during
visit_end_*, it's a lot easier to just make all callers pass the
same pointer to visit_end_*. The generated code has access to the
same pointer, while all other users are doing virtual walks and
can pass NULL. The dealloc visitor is then greatly simplified.
All three visit_end_*() functions intentionally take a void**,
even though the visit_start_*() functions differ between void**,
GenericList**, and GenericAlternate**. This is done for several
reasons: when doing a virtual walk, passing NULL doesn't care
what the type is, but when doing a generated walk, we already
have to cast the caller's specific FOO* to call visit_start,
while using void** lets us use visit_end without a cast. Also,
an upcoming patch will add a clone visitor that wants to use
the same implementation for all three visit_end callbacks,
which is made easier if all three share the same signature.
For visitors with already track per-object state (the QMP visitors
via a stack, and the string visitors which do not allow nesting),
add an assertion that the caller is indeed passing the same
pointer to paired calls.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-4-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
If our JSON output ever encounters an error, we would just silently
leak the error object. Instead, assert that our usage won't fail.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1465490926-28625-3-git-send-email-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Yi Min Zhao