The QEMU_VFIO_DEBUG definition is only modifiable at build-time.
Trace events can be enabled at run-time. As we prefer the latter,
convert qemu_vfio_dump_mappings() to use trace events instead
of fprintf().
Reviewed-by: Fam Zheng <fam@euphon.net>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201103020733.2303148-7-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
For debugging purpose, trace where DMA regions are mapped.
Reviewed-by: Fam Zheng <fam@euphon.net>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201103020733.2303148-6-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
For debugging purpose, trace where a BAR is mapped.
Reviewed-by: Fam Zheng <fam@euphon.net>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201103020733.2303148-5-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
For debug purpose, trace BAR regions info.
Reviewed-by: Fam Zheng <fam@euphon.net>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201103020733.2303148-4-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
We sometime get kernel panic with some devices on Aarch64
hosts. Alex Williamson suggests it might be broken PCIe
root complex. Add trace event to record the latest I/O
access before crashing. In case, assert our accesses are
aligned.
Reviewed-by: Fam Zheng <fam@euphon.net>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201103020733.2303148-3-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Change the confuse "VFIO IOMMU check failed" error message by
the explicit "VFIO IOMMU Type1 is not supported" once.
Example on POWER:
$ qemu-system-ppc64 -drive if=none,id=nvme0,file=nvme://0001:01:00.0/1,format=raw
qemu-system-ppc64: -drive if=none,id=nvme0,file=nvme://0001:01:00.0/1,format=raw: VFIO IOMMU Type1 is not supported
Suggested-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Fam Zheng <fam@euphon.net>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201103020733.2303148-2-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
The Completion Queue Command Identifier is a 16-bit value,
so nvme_submit_command() is unlikely to work on big-endian
hosts, as the relevant bits are truncated.
Fix by using the correct byte-swap function.
Fixes: bdd6a90a9e ("block: Add VFIO based NVMe driver")
Reported-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20201029093306.1063879-25-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
qemu_vfio_pci_map_bar() calls mmap(), and mmap(2) states:
'offset' must be a multiple of the page size as returned
by sysconf(_SC_PAGE_SIZE).
In commit f68453237b we started to use an offset of 4K which
broke this contract on Aarch64 arch.
Fix by mapping at offset 0, and and accessing doorbells at offset=4K.
Fixes: f68453237b ("block/nvme: Map doorbells pages write-only")
Reported-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-24-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Make sure iov's va and size are properly aligned on the
host page size.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-23-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
In preparation of 64kB host page support, let's change the size
and alignment of the prp_list_pages so that the VFIO DMA MAP succeeds
with 64kB host page size. We align on the host page size.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-22-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
In preparation of 64kB host page support, let's change the size
and alignment of the queue so that the VFIO DMA MAP succeeds.
We align on the host page size.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-21-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
In preparation of 64kB host page support, let's change the size
and alignment of the IDENTIFY command response buffer so that
the VFIO DMA MAP succeeds. We align on the host page size.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-20-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
While trying to simplify the code using a macro, we forgot
the 12-bit shift... Correct that.
Fixes: fad1eb6886 ("block/nvme: Use register definitions from 'block/nvme.h'")
Reported-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-19-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Commit bdd6a90a9e ("block: Add VFIO based NVMe driver")
sets the request_alignment in nvme_refresh_limits().
For consistency, also set it during initialization.
Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-18-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
As all commands use the ADMIN queue, it is pointless to pass
it as argument each time. Remove the argument, and rename the
function as nvme_admin_cmd_sync() to make this new behavior
clearer.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20201029093306.1063879-17-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
We don't need to dereference from BDRVNVMeState each time.
Use a NVMeQueuePair pointer on the admin queue.
The nvme_init() becomes easier to review, matching the style
of nvme_add_io_queue().
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-16-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
From the specification chapter 3.1.8 "AQA - Admin Queue Attributes"
the Admin Submission Queue Size field is a 0’s based value:
Admin Submission Queue Size (ASQS):
Defines the size of the Admin Submission Queue in entries.
Enabling a controller while this field is cleared to 00h
produces undefined results. The minimum size of the Admin
Submission Queue is two entries. The maximum size of the
Admin Submission Queue is 4096 entries.
This is a 0’s based value.
This bug has never been hit because the device initialization
uses a single command synchronously :)
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-15-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Replace magic values by definitions, and simplifiy since the
number of queues will never reach 64K.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-14-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Rename Submission Queue flags with 'Sq' to differentiate
submission queue flags from command queue flags, and introduce
Completion Queue flag definitions.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20201029093306.1063879-13-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Just for consistency, following the example documented since
commit e3fe3988d7 ("error: Document Error API usage rules"),
return a boolean value indicating an error is set or not.
Directly pass errp as the local_err is not requested in our
case. This simplifies a bit nvme_create_queue_pair().
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-12-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Just for consistency, following the example documented since
commit e3fe3988d7 ("error: Document Error API usage rules"),
return a boolean value indicating an error is set or not.
Directly pass errp as the local_err is not requested in our
case.
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20201029093306.1063879-11-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
We can not have negative queue count/size/index, use unsigned type.
Rename 'nr_queues' as 'queue_count' to match the spec naming.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-10-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
To be able to use some definitions in structure declarations,
move them earlier. No logical change.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-9-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-8-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
What we want to trace is the block driver state and the queue index.
Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-7-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
As we want to enable multiple queues, report the event
in each nvme_poll_queue() call, rather than once in
the callback calling nvme_poll_queues().
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-6-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Controllers have different capabilities and report them in the
CAP register. We are particularly interested by the page size
limits.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-5-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Instead of displaying warning on stderr, use warn_report()
which also displays it on the monitor.
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-4-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Use the same format used for the hw/vfio/ trace events.
Suggested-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201029093306.1063879-3-philmd@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
The "block/nvme.h" header is shared by both the NVMe block
driver and the NVMe emulated device. Add the 'F:' entry on
both sections, so all maintainers/reviewers are notified
when it is changed.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Message-Id: <20200701140634.25994-1-philmd@redhat.com>
Eventfd can be registered with a zero length when fast_mmio is true.
Handle this case properly when dispatching through QEMU.
Signed-off-by: Elena Afanasova <eafanasova@gmail.com>
Message-id: cf71a62eb04e61932ff8ffdd02e0b2aab4f495a0.camel@gmail.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Elena Afanasova <eafanasova@gmail.com>
Message-Id: <20201017210102.26036-1-eafanasova@gmail.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
We should use printf format specifier "%u" instead of "%d" for
argument of type "unsigned int".
In addition, fix two error format problems found by checkpatch.pl:
ERROR: space required after that ',' (ctx:VxV)
+ fprintf(stderr,"%s attributes=0x%02x begin=%u size=%d\n",
^
ERROR: line over 90 characters
+ fprintf(stderr, "%d, %s (%u, %d)\n", i, commit->path ? commit->path : "(null)", commit->param.rename.cluster, commit->action);
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Alex Chen <alex.chen@huawei.com>
Message-Id: <5FA12620.6030705@huawei.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
pylint complains about the use of super with the current class and
instance as arguments in VM.__init__():
iotests.py:546:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
No reason not to follow the advice and make it happy, so let's do this.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20201027163806.290960-4-kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
When run with Python 3.9, pylint incorrectly warns about things like
Optional[foo] because it doesn't recognise Optional as unsubscriptable.
This is a known pylint bug:
https://github.com/PyCQA/pylint/issues/3882
Just disable this check to get rid of the warnings.
Disabling this shouldn't make us miss any real bug because mypy also
has a similar check ("... is not indexable").
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20201027163806.290960-3-kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit 1847a4a8c2 clarified that event_wait() can return None (though
only with timeout=0) and commit f12a282ff4 annotated it as returning
Optional[QMPMessage].
Type checks in wait_migration() fail because of the unexpected optional
return type:
iotests.py:750: error: Value of type variable "Msg" of "log" cannot be "Optional[Dict[str, Any]]"
iotests.py:751: error: Value of type "Optional[Dict[str, Any]]" is not indexable
iotests.py:754: error: Value of type "Optional[Dict[str, Any]]" is not indexable
Fortunately, the non-zero default timeout is used in the event_wait()
call, so we can make mypy happy by just asserting this.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20201027163806.290960-2-kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
@sn_opts is initialized at the beginning, so it should be deleted
after jumping to the lable 'fail_getopt'
Signed-off-by: Guoyi Tu <tu.guoyi@h3c.com>
Message-Id: <6ff1c5d372944494be3932274f75485d@h3c.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit 9ce44e2ce2 "qmp: Move dispatcher to a coroutine" modified
aio_poll() in util/aio-posix.c to avoid an assertion failure. This
change is missing in util/aio-win32.c.
Apply the changes to util/aio-posix.c to util/aio-win32.c too.
This fixes an assertion failure on Windows whenever QEMU exits.
$ ./qemu-system-x86_64.exe -machine pc,accel=tcg -display gtk
**
ERROR:../qemu/util/aio-win32.c:337:aio_poll: assertion failed:
(in_aio_context_home_thread(ctx))
Bail out! ERROR:../qemu/util/aio-win32.c:337:aio_poll: assertion
failed: (in_aio_context_home_thread(ctx))
Fixes: 9ce44e2ce2 ("qmp: Move dispatcher to a coroutine")
Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
Message-Id: <20201021064033.8600-1-vr_qemu@t-online.de>
Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
- Only report abstract socket support in QAPI on Linux hosts
- Expand test coverage
- Misc other code cleanups
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEE2vOm/bJrYpEtDo4/vobrtBUQT98FAl+hYpUACgkQvobrtBUQ
T998sw//d3Cd4Ta+zwkMaiziUMmMcO/vhs0X5VLp7HsHZdfAlZKlebI6kmNr8Ucy
n10bmsIWfrdNMvYpOPyEehbens6BPHELwfw+gkA1rf+7GWS1fafUnz/liRCbITFs
TglLFtZ9pQfUBOOE42uEkFZrq3Kw4gPn1XYn28PHSLOFQn2Wzy0ww1De/X/EAxUm
QPcjICB+pPwcavpRC+0Kdz5oxNVXNirc+mPwETZobJ5tGByM5th5O0hT5qmTHcxp
aWIgllIanGlfJA4DPI5ee92D7voSaen6470kfKY9dcUAopHPZWih0SNv++X9pOFz
3iSCUOSEkx39ItyXmObeoBUxMXrQxRqyPg3HRGet3TqlpNjsc8t2UiiLWa2OijT5
wAc2btLoSghtzJvKSYcE9uLJBQq7VbntDvuKO2RZLrraTSUvzlPO57yPIcvrsM3p
c6Qj5QNwdvsurS+Nqz2a5ATDjLPeZyOcePeVoIYu60wLcL38fEFK8OjHZIqWWlrY
GEDv+krTa4DhFV5E5OSgQPnZQmWUHSCmCFzF9vyfZpYSUh2OrB05hSi4GhPbAwA1
n8MZHIG6LHw53Xy9J16l1bInNPfl/9LEiFB4GwjATcZeNEeKp1BKHtu7yFmgwoAh
DTvKBrVstAGxNMDQd+uVwHiYo2EcWCK4ZHAcofuhhfagpRs7vTc=
=UBCe
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/berrange-gitlab/tags/sock-next-pull-request' into staging
- Fix inverted logic in abstract socket QAPI support
- Only report abstract socket support in QAPI on Linux hosts
- Expand test coverage
- Misc other code cleanups
# gpg: Signature made Tue 03 Nov 2020 14:00:53 GMT
# gpg: using RSA key DAF3A6FDB26B62912D0E8E3FBE86EBB415104FDF
# gpg: Good signature from "Daniel P. Berrange <dan@berrange.com>" [full]
# gpg: aka "Daniel P. Berrange <berrange@redhat.com>" [full]
# Primary key fingerprint: DAF3 A6FD B26B 6291 2D0E 8E3F BE86 EBB4 1510 4FDF
* remotes/berrange-gitlab/tags/sock-next-pull-request:
sockets: Make abstract UnixSocketAddress depend on CONFIG_LINUX
sockets: Bypass "replace empty @path" for abstract unix sockets
char-socket: Fix qemu_chr_socket_address() for abstract sockets
sockets: Fix socket_sockaddr_to_address_unix() for abstract sockets
sockets: Fix default of UnixSocketAddress member @tight
test-util-sockets: Test the complete abstract socket matrix
test-util-sockets: Synchronize properly, don't sleep(1)
test-util-sockets: Factor out test_socket_unix_abstract_one()
test-util-sockets: Clean up SocketAddress construction
test-util-sockets: Correct to set has_abstract, has_tight
test-util-sockets: Plug file descriptor leak
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
The abstract socket namespace is a non-portable Linux extension. An
attempt to use it elsewhere should fail with ENOENT (the abstract
address looks like a "" pathname, which does not resolve). We report
this failure like
Failed to connect socket abc: No such file or directory
Tolerable, although ENOTSUP would be better.
However, introspection lies: it has @abstract regardless of host
support. Easy enough to fix: since Linux provides them since 2.2,
'if': 'defined(CONFIG_LINUX)' should do.
The above failure becomes
Parameter 'backend.data.addr.data.abstract' is unexpected
I consider this an improvement.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
unix_listen_saddr() replaces empty @path by unique value. It obtains
the value by creating and deleting a unique temporary file with
mkstemp(). This is racy, as the comment explains. It's also entirely
undocumented as far as I can tell. Goes back to commit d247d25f18
"sockets: helper functions for qemu (Gerd Hoffman)", v0.10.0.
Since abstract socket addresses have no connection with filesystem
pathnames, making them up with mkstemp() seems inappropriate. Bypass
the replacement of empty @path.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Commit 776b97d360 "qemu-sockets: add abstract UNIX domain socket
support" neglected to update qemu_chr_socket_address(). It shows
shows neither @abstract nor @tight. Fix that.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Commit 776b97d360 "qemu-sockets: add abstract UNIX domain socket
support" neglected to update socket_sockaddr_to_address_unix(). The
function returns a non-abstract socket address for abstract
sockets (wrong) with a null @path (also wrong; a non-optional QAPI str
member must never be null).
The null @path is due to confused code going back all the way to
commit 17c55decec "sockets: add helpers for creating SocketAddress
from a socket".
Add the required special case, and simplify the confused code.
Fixes: 776b97d360
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
An optional bool member of a QAPI struct can be false, true, or absent.
The previous commit demonstrated that socket_listen() and
socket_connect() are broken for absent @tight, and indeed QMP chardev-
add also defaults absent member @tight to false instead of true.
In C, QAPI members are represented by two fields, has_MEMBER and MEMBER.
We have:
has_MEMBER MEMBER
false true false
true true true
absent false false/ignore
When has_MEMBER is false, MEMBER should be set to false on write, and
ignored on read.
For QMP, the QAPI visitors handle absent @tight by setting both
@has_tight and @tight to false. unix_listen_saddr() and
unix_connect_saddr() however use @tight only, disregarding @has_tight.
This is wrong and means that absent @tight defaults to false whereas it
should default to true.
The same is true for @has_abstract, though @abstract defaults to
false and therefore has the same behavior for all of QMP, HMP and CLI.
Fix unix_listen_saddr() and unix_connect_saddr() to check
@has_abstract/@has_tight, and to default absent @tight to true.
However, this is only half of the story. HMP chardev-add and CLI
-chardev so far correctly defaulted @tight to true, but defaults to
false again with the above fix for HMP and CLI. In fact, the "tight"
and "abstract" options now break completely.
Digging deeper, we find that qemu_chr_parse_socket() also ignores
@has_tight, leaving it false when it sets @tight. That is also wrong,
but the two wrongs cancelled out. Fix qemu_chr_parse_socket() to set
@has_tight and @has_abstract; writing testcases for HMP and CLI is left
for another day.
Fixes: 776b97d360
Reported-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The test covers only two out of nine combinations. Test all nine.
Four turn out to be broken. Marked /* BUG */.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The abstract sockets test spawns a thread to listen and accept, and a
second one to connect, with a sleep(1) in between to "ensure" the
former is listening when the latter tries to connect. Review fail.
Risks spurious test failure, say when a heavily loaded machine doesn't
schedule the first thread quickly enough. It's also slow.
Listen and accept in the main thread, and start the connect thread in
between. Look ma, no sleep! Run time drops from 2s wall clock to a
few milliseconds.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The thread functions build the SocketAddress from global variable
@abstract_sock_name and the tight flag passed as pointer
argument (either NULL or (gpointer)1). There is no need for such
hackery; simply pass the SocketAddress instead.
While there, dumb down g_rand_int_range() to g_random_int().
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The code tested doesn't care, which is a bug I will fix shortly.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
