aboutsummaryrefslogtreecommitdiff
path: root/hw/9pfs
AgeCommit message (Collapse)AuthorFilesLines
2020-11-05hw/9pfs: Fix Kconfig dependency problem between 9pfs and XenPhilippe Mathieu-Daudé2-5/+1
Commit b2c00bce54c ("meson: convert hw/9pfs, cleanup") introduced CONFIG_9PFS (probably a wrong conflict resolution). This config is not used anywhere. Backends depend on CONFIG_FSDEV_9P which itself depends on CONFIG_VIRTFS. Remove the invalid CONFIG_9PFS and use CONFIG_FSDEV_9P instead, to fix the './configure --without-default-devices --enable-xen' build: /usr/bin/ld: libcommon.fa.p/hw_xen_xen-legacy-backend.c.o: in function `xen_be_register_common': hw/xen/xen-legacy-backend.c:754: undefined reference to `xen_9pfs_ops' /usr/bin/ld: libcommon.fa.p/fsdev_qemu-fsdev.c.o:(.data.rel+0x8): undefined reference to `local_ops' /usr/bin/ld: libcommon.fa.p/fsdev_qemu-fsdev.c.o:(.data.rel+0x20): undefined reference to `synth_ops' /usr/bin/ld: libcommon.fa.p/fsdev_qemu-fsdev.c.o:(.data.rel+0x38): undefined reference to `proxy_ops' collect2: error: ld returned 1 exit status Fixes: b2c00bce54c ("meson: convert hw/9pfs, cleanup") Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Acked-by: Greg Kurz <groug@kaod.org> Tested-by: Greg Kurz <groug@kaod.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Acked-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Message-Id: <20201104115706.3101190-3-philmd@redhat.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
2020-11-05hw/9pfs : add space before the open parenthesis '('Xinhao Zhang1-1/+1
Fix code style. Space required before the open parenthesis '('. Signed-off-by: Xinhao Zhang <zhangxinhao1@huawei.com> Signed-off-by: Kai Deng <dengkai1@huawei.com> Reported-by: Euler Robot <euler.robot@huawei.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <20201030043515.1030223-3-zhangxinhao1@huawei.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
2020-11-05hw/9pfs : open brace '{' following struct go on the same lineXinhao Zhang1-6/+3
Fix code style. Open braces for struct should go on the same line. Signed-off-by: Xinhao Zhang <zhangxinhao1@huawei.com> Signed-off-by: Kai Deng <dengkai1@huawei.com> Reported-by: Euler Robot <euler.robot@huawei.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <20201030043515.1030223-2-zhangxinhao1@huawei.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
2020-11-05hw/9pfs : add spaces around operatorXinhao Zhang2-13/+13
Fix code style. Operator needs spaces both sides. Signed-off-by: Xinhao Zhang <zhangxinhao1@huawei.com> Signed-off-by: Kai Deng <dengkai1@huawei.com> Reported-by: Euler Robot <euler.robot@huawei.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <20201030043515.1030223-1-zhangxinhao1@huawei.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
2020-10-199pfs: suppress performance warnings on qtest runsChristian Schoenebeck2-2/+6
Don't trigger any performance warning if we're just running test cases, because tests intentionally run for edge cases. So far performance warnings were suppressed for the 'synth' fs driver backend only. This patch suppresses them for all 9p fs driver backends. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <a2d2ff2163f8853ea782a7a1d4e6f2afd7c29ffe.1603106145.git.qemu_oss@crudebyte.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
2020-09-18Use OBJECT_DECLARE_SIMPLE_TYPE when possibleEduardo Habkost1-3/+1
This converts existing DECLARE_INSTANCE_CHECKER usage to OBJECT_DECLARE_SIMPLE_TYPE when possible. $ ./scripts/codeconverter/converter.py -i \ --pattern=AddObjectDeclareSimpleType $(git grep -l '' -- '*.[ch]') Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Acked-by: Paul Durrant <paul@xen.org> Message-Id: <20200916182519.415636-6-ehabkost@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
2020-09-159pfs: disable msize warning for synth driverChristian Schoenebeck2-1/+3
Previous patch introduced a performance warning being logged on host side if client connected with an 'msize' <= 8192. Disable this performance warning for the synth driver to prevent that warning from being printed whenever the 9pfs (qtest) test cases are running. Introduce a new export flag V9FS_NO_PERF_WARN for that purpose, which might also be used to disable such warnings from the CLI in future. We could have also prevented the warning by simply raising P9_MAX_SIZE in virtio-9p-test.c to any value larger than 8192, however in the context of test cases it makes sense running for edge cases, which includes the lowest 'msize' value supported by the server which is 4096, hence we want to preserve an msize of 4096 for the test client. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <E1kEyDy-0006nN-5A@lizzy.crudebyte.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
2020-09-159pfs: log warning if msize <= 8192Christian Schoenebeck1-0/+9
It is essential to choose a reasonable high value for 'msize' to avoid severely degraded file I/O performance. This parameter can only be chosen on client/guest side, and a Linux client defaults to an 'msize' of only 8192 if the user did not explicitly specify a value for 'msize', which results in very poor file I/O performance. Unfortunately many users are not aware that they should specify an appropriate value for 'msize' to avoid severe performance issues, so log a performance warning (with a QEMU wiki link explaining this issue in detail) on host side in that case to make it more clear. Currently a client cannot automatically pick a reasonable value for 'msize', because a good value for 'msize' depends on the file I/O potential of the underlying storage on host side, i.e. a feature invisible to the client, and even then a user would still need to trade off between performance profit and additional RAM costs, i.e. with growing 'msize' (RAM occupation), performance still increases, but performance delta will shrink continuously. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <e6fc84845c95816ad5baecb0abd6bfefdcf7ec9f.1599144062.git.qemu_oss@crudebyte.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
2020-09-09Use DECLARE_*CHECKER* macrosEduardo Habkost1-2/+2
Generated using: $ ./scripts/codeconverter/converter.py -i \ --pattern=TypeCheckMacro $(git grep -l '' -- '*.[ch]') Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Message-Id: <20200831210740.126168-12-ehabkost@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Message-Id: <20200831210740.126168-13-ehabkost@redhat.com> Message-Id: <20200831210740.126168-14-ehabkost@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
2020-09-09Move QOM typedefs and add missing includesEduardo Habkost1-3/+4
Some typedefs and macros are defined after the type check macros. This makes it difficult to automatically replace their definitions with OBJECT_DECLARE_TYPE. Patch generated using: $ ./scripts/codeconverter/converter.py -i \ --pattern=QOMStructTypedefSplit $(git grep -l '' -- '*.[ch]') which will split "typdef struct { ... } TypedefName" declarations. Followed by: $ ./scripts/codeconverter/converter.py -i --pattern=MoveSymbols \ $(git grep -l '' -- '*.[ch]') which will: - move the typedefs and #defines above the type check macros - add missing #include "qom/object.h" lines if necessary Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Message-Id: <20200831210740.126168-9-ehabkost@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Message-Id: <20200831210740.126168-10-ehabkost@redhat.com> Message-Id: <20200831210740.126168-11-ehabkost@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
2020-08-24Merge remote-tracking branch 'remotes/cschoenebeck/tags/pull-9p-20200812' ↵Peter Maydell4-94/+333
into staging 9pfs: Fix severe performance issue of Treaddir requests. # gpg: Signature made Wed 12 Aug 2020 11:06:21 BST # gpg: using RSA key 96D8D110CF7AF8084F88590134C2B58765A47395 # gpg: issuer "qemu_oss@crudebyte.com" # gpg: Good signature from "Christian Schoenebeck <qemu_oss@crudebyte.com>" [unknown] # gpg: WARNING: This key is not certified with a trusted signature! # gpg: There is no indication that the signature belongs to the owner. # Primary key fingerprint: ECAB 1A45 4014 1413 BA38 4926 30DB 47C3 A012 D5F4 # Subkey fingerprint: 96D8 D110 CF7A F808 4F88 5901 34C2 B587 65A4 7395 * remotes/cschoenebeck/tags/pull-9p-20200812: 9pfs: clarify latency of v9fs_co_run_in_worker() 9pfs: differentiate readdir lock between 9P2000.u vs. 9P2000.L 9pfs: T_readdir latency optimization 9pfs: add new function v9fs_co_readdir_many() 9pfs: split out fs driver core of v9fs_co_readdir() 9pfs: make v9fs_readdir_response_size() public tests/virtio-9p: added split readdir tests Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-08-21meson: convert hw/9pfs, cleanupMarc-André Lureau3-9/+24
hw/Makefile.objs is gone so there is more code that can be removed. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-08-21trace: switch position of headers to what Meson requiresPaolo Bonzini1-0/+1
Meson doesn't enjoy the same flexibility we have with Make in choosing the include path. In particular the tracing headers are using $(build_root)/$(<D). In order to keep the include directives unchanged, the simplest solution is to generate headers with patterns like "trace/trace-audio.h" and place forwarding headers in the source tree such that for example "audio/trace.h" includes "trace/trace-audio.h". This patch is too ugly to be applied to the Makefiles now. It's only a way to separate the changes to the tracing header files from the Meson rewrite of the tracing logic. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-08-129pfs: clarify latency of v9fs_co_run_in_worker()Christian Schoenebeck1-1/+11
As we just fixed a severe performance issue with Treaddir request handling, clarify this overall issue as a comment on v9fs_co_run_in_worker() with the intention to hopefully prevent such performance mistakes in future (and fixing other yet outstanding ones). Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Message-Id: <4d34d332e1aaa8a2cf8dc0b5da4fd7727f2a86e8.1596012787.git.qemu_oss@crudebyte.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
2020-08-129pfs: differentiate readdir lock between 9P2000.u vs. 9P2000.LChristian Schoenebeck2-8/+40
Previous patch suggests that it might make sense to use a different mutex type now while handling readdir requests, depending on the precise protocol variant, as v9fs_do_readdir_with_stat() (used by 9P2000.u) uses a CoMutex to avoid deadlocks that might happen with QemuMutex otherwise, whereas do_readdir_many() (used by 9P2000.L) should better use a QemuMutex, as the precise behaviour of a failed CoMutex lock on fs driver side would not be clear. And to avoid the wrong lock type being used, be now strict and error out if a 9P2000.L client sends a Tread on a directory, and likeweise error out if a 9P2000.u client sends a Treaddir request. This patch is just intended as transitional measure, as currently 9P2000.u vs. 9P2000.L implementations currently differ where the main logic of fetching directory entries is located at (9P2000.u still being more top half focused, while 9P2000.L already being bottom half focused in regards to fetching directory entries that is). Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Message-Id: <9a2ddc347e533b0d801866afd9dfac853d2d4106.1596012787.git.qemu_oss@crudebyte.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
2020-08-129pfs: T_readdir latency optimizationChristian Schoenebeck1-74/+58
Make top half really top half and bottom half really bottom half: Each T_readdir request handling is hopping between threads (main I/O thread and background I/O driver threads) several times for every individual directory entry, which sums up to huge latencies for handling just a single T_readdir request. Instead of doing that, collect now all required directory entries (including all potentially required stat buffers for each entry) in one rush on a background I/O thread from fs driver by calling the previously added function v9fs_co_readdir_many() instead of v9fs_co_readdir(), then assemble the entire resulting network response message for the readdir request on main I/O thread. The fs driver is still aborting the directory entry retrieval loop (on the background I/O thread inside of v9fs_co_readdir_many()) as soon as it would exceed the client's requested maximum R_readdir response size. So this will not introduce a performance penalty on another end. Also: No longer seek initial directory position in v9fs_readdir(), as this is now handled (more consistently) by v9fs_co_readdir_many() instead. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Message-Id: <c7c3d1cf4e86611538cef44897842819d9359d7a.1596012787.git.qemu_oss@crudebyte.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
2020-08-129pfs: add new function v9fs_co_readdir_many()Christian Schoenebeck3-0/+196
The newly added function v9fs_co_readdir_many() retrieves multiple directory entries with a single fs driver request. It is intended to replace uses of v9fs_co_readdir(), the latter only retrieves a single directory entry per fs driver request instead. The reason for this planned replacement is that for every fs driver request the coroutine is dispatched from main I/O thread to a background I/O thread and eventually dispatched back to main I/O thread. Hopping between threads adds latency. So if a 9pfs Treaddir request reads a large amount of directory entries, this currently sums up to huge latencies of several hundred ms or even more. So using v9fs_co_readdir_many() instead of v9fs_co_readdir() will provide significant performance improvements. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Message-Id: <73dc827a12ef577ae7e644dcf34a5c0e443ab42f.1596012787.git.qemu_oss@crudebyte.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
2020-08-129pfs: split out fs driver core of v9fs_co_readdir()Christian Schoenebeck1-14/+24
The implementation of v9fs_co_readdir() has two parts: the outer part is executed by main I/O thread, whereas the inner part is executed by fs driver on a background I/O thread. Move the inner part to its own new, private function do_readdir(), so it can be shared by another upcoming new function. This is just a preparatory patch for the subsequent patch, with the purpose to avoid the next patch to clutter the overall diff. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <a426ee06e77584fa2d8253ce5d8bea519eb3ffd4.1596012787.git.qemu_oss@crudebyte.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
2020-08-129pfs: make v9fs_readdir_response_size() publicChristian Schoenebeck2-2/+9
Rename function v9fs_readdir_data_size() -> v9fs_readdir_response_size() and make it callable from other units. So far this function is only used by 9p.c, however subsequent patches require the function to be callable from another 9pfs unit. And as we're at it; also make it clear for what this function is used for. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <3668ebc7d5b929a0e4f1357457060d96f50f76f4.1596012787.git.qemu_oss@crudebyte.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
2020-07-10virtio-9p: Use ERRP_GUARD()Vladimir Sementsov-Ogievskiy2-7/+6
If we want to check error after errp-function call, we need to introduce local_err and then propagate it to errp. Instead, use the ERRP_GUARD() macro, benefits are: 1. No need of explicit error_propagate call 2. No need of explicit local_err variable: use errp directly 3. ERRP_GUARD() leaves errp as is if it's not NULL or &error_fatal, this means that we don't break error_abort (we'll abort on error_set, not on error_propagate) If we want to add some info to errp (by error_prepend() or error_append_hint()), we must use the ERRP_GUARD() macro. Otherwise, this info will not be added when errp == &error_fatal (the program will exit prior to the error_append_hint() or error_prepend() call). Fix such a case in v9fs_device_realize_common(). This commit is generated by command sed -n '/^virtio-9p$/,/^$/{s/^F: //p}' MAINTAINERS | \ xargs git ls-files | grep '\.[hc]$' | \ xargs spatch \ --sp-file scripts/coccinelle/errp-guard.cocci \ --macro-file scripts/cocci-macro-file.h \ --in-place --no-show-diff --max-width 80 Reported-by: Kevin Wolf <kwolf@redhat.com> Reported-by: Greg Kurz <groug@kaod.org> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Acked-by: Greg Kurz <groug@kaod.org> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> [Commit message tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200707165037.1026246-7-armbru@redhat.com> [ERRP_AUTO_PROPAGATE() renamed to ERRP_GUARD(), and auto-propagated-errp.cocci to errp-guard.cocci. Commit message tweaked again.]
2020-07-02Clean up some calls to ignore Error objects the right wayMarkus Armbruster1-4/+2
Receiving the error in a local variable only to free it is less clear (and also less efficient) than passing NULL. Clean up. Cc: Daniel P. Berrange <berrange@redhat.com> Cc: Jerome Forissier <jerome@forissier.org> CC: Greg Kurz <groug@kaod.org> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <20200630090351.1247703-4-armbru@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2020-05-25xen/9pfs: increase max ring order to 9Stefano Stabellini1-1/+1
The max order allowed by the protocol is 9. Increase the max order supported by QEMU to 9 to increase performance. Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Message-Id: <20200521192627.15259-3-sstabellini@kernel.org> Signed-off-by: Greg Kurz <groug@kaod.org>
2020-05-25xen/9pfs: yield when there isn't enough room on the ringStefano Stabellini1-6/+25
Instead of truncating replies, which is problematic, wait until the client reads more data and frees bytes on the reply ring. Do that by calling qemu_coroutine_yield(). The corresponding qemu_coroutine_enter_if_inactive() is called from xen_9pfs_bh upon receiving the next notification from the client. We need to be careful to avoid races in case xen_9pfs_bh and the coroutine are both active at the same time. In xen_9pfs_bh, wait until either the critical section is over (ring->co == NULL) or until the coroutine becomes inactive (qemu_coroutine_yield() was called) before continuing. Then, simply wake up the coroutine if it is inactive. Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Message-Id: <20200521192627.15259-2-sstabellini@kernel.org> Signed-off-by: Greg Kurz <groug@kaod.org>
2020-05-25Revert "9p: init_in_iov_from_pdu can truncate the size"Stefano Stabellini4-39/+22
This reverts commit 16724a173049ac29c7b5ade741da93a0f46edff7. It causes https://bugs.launchpad.net/bugs/1877688. Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Message-Id: <20200521192627.15259-1-sstabellini@kernel.org> Signed-off-by: Greg Kurz <groug@kaod.org>
2020-05-259p: Lock directory streams with a CoMutexGreg Kurz1-4/+4
Locking was introduced in QEMU 2.7 to address the deprecation of readdir_r(3) in glibc 2.24. It turns out that the frontend code is the worst place to handle a critical section with a pthread mutex: the code runs in a coroutine on behalf of the QEMU mainloop and then yields control, waiting for the fsdev backend to process the request in a worker thread. If the client resends another readdir request for the same fid before the previous one finally unlocked the mutex, we're deadlocked. This never bit us because the linux client serializes readdir requests for the same fid, but it is quite easy to demonstrate with a custom client. A good solution could be to narrow the critical section in the worker thread code and to return a copy of the dirent to the frontend, but this causes quite some changes in both 9p.c and codir.c. So, instead of that, in order for people to easily backport the fix to older QEMU versions, let's simply use a CoMutex since all the users for this sit in coroutines. Fixes: 7cde47d4a89d ("9p: add locking to V9fsDir") Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Message-Id: <158981894794.109297.3530035833368944254.stgit@bahia.lan> Signed-off-by: Greg Kurz <groug@kaod.org>
2020-05-259pfs: include linux/limits.h for XATTR_SIZE_MAXDan Robertson1-0/+1
linux/limits.h should be included for the XATTR_SIZE_MAX definition used by v9fs_xattrcreate. Fixes: 3b79ef2cf488 ("9pfs: limit xattr size in xattrcreate") Signed-off-by: Dan Robertson <dan@dlrobertson.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Message-Id: <20200515203015.7090-2-dan@dlrobertson.com> Signed-off-by: Greg Kurz <groug@kaod.org>
2020-05-15qdev: Unrealize must not failMarkus Armbruster3-5/+5
Devices may have component devices and buses. Device realization may fail. Realization is recursive: a device's realize() method realizes its components, and device_set_realized() realizes its buses (which should in turn realize the devices on that bus, except bus_set_realized() doesn't implement that, yet). When realization of a component or bus fails, we need to roll back: unrealize everything we realized so far. If any of these unrealizes failed, the device would be left in an inconsistent state. Must not happen. device_set_realized() lets it happen: it ignores errors in the roll back code starting at label child_realize_fail. Since realization is recursive, unrealization must be recursive, too. But how could a partly failed unrealize be rolled back? We'd have to re-realize, which can fail. This design is fundamentally broken. device_set_realized() does not roll back at all. Instead, it keeps unrealizing, ignoring further errors. It can screw up even for a device with no buses: if the lone dc->unrealize() fails, it still unregisters vmstate, and calls listeners' unrealize() callback. bus_set_realized() does not roll back either. Instead, it stops unrealizing. Fortunately, no unrealize method can fail, as we'll see below. To fix the design error, drop parameter @errp from all the unrealize methods. Any unrealize method that uses @errp now needs an update. This leads us to unrealize() methods that can fail. Merely passing it to another unrealize method cannot cause failure, though. Here are the ones that do other things with @errp: * virtio_serial_device_unrealize() Fails when qbus_set_hotplug_handler() fails, but still does all the other work. On failure, the device would stay realized with its resources completely gone. Oops. Can't happen, because qbus_set_hotplug_handler() can't actually fail here. Pass &error_abort to qbus_set_hotplug_handler() instead. * hw/ppc/spapr_drc.c's unrealize() Fails when object_property_del() fails, but all the other work is already done. On failure, the device would stay realized with its vmstate registration gone. Oops. Can't happen, because object_property_del() can't actually fail here. Pass &error_abort to object_property_del() instead. * spapr_phb_unrealize() Fails and bails out when remove_drcs() fails, but other work is already done. On failure, the device would stay realized with some of its resources gone. Oops. remove_drcs() fails only when chassis_from_bus()'s object_property_get_uint() fails, and it can't here. Pass &error_abort to remove_drcs() instead. Therefore, no unrealize method can fail before this patch. device_set_realized()'s recursive unrealization via bus uses object_property_set_bool(). Can't drop @errp there, so pass &error_abort. We similarly unrealize with object_property_set_bool() elsewhere, always ignoring errors. Pass &error_abort instead. Several unrealize methods no longer handle errors from other unrealize methods: virtio_9p_device_unrealize(), virtio_input_device_unrealize(), scsi_qdev_unrealize(), ... Much of the deleted error handling looks wrong anyway. One unrealize methods no longer ignore such errors: usb_ehci_pci_exit(). Several realize methods no longer ignore errors when rolling back: v9fs_device_realize_common(), pci_qdev_unrealize(), spapr_phb_realize(), usb_qdev_realize(), vfio_ccw_realize(), virtio_device_realize(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200505152926.18877-17-armbru@redhat.com>
2020-05-14xen-9pfs: Fix log messages of reply errorsChristian Schoenebeck1-4/+5
If delivery of some 9pfs response fails for some reason, log the error message by mentioning the 9P protocol reply type, not by client's request type. The latter could be misleading that the error occurred already when handling the request input. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> Message-Id: <ad0e5a9b6abde52502aa40b30661d29aebe1590a.1589132512.git.qemu_oss@crudebyte.com> Signed-off-by: Greg Kurz <groug@kaod.org>
2020-05-149pfs: local: ignore O_NOATIME if we don't have permissionsOmar Sandoval1-0/+13
QEMU's local 9pfs server passes through O_NOATIME from the client. If the QEMU process doesn't have permissions to use O_NOATIME (namely, it does not own the file nor have the CAP_FOWNER capability), the open will fail. This causes issues when from the client's point of view, it believes it has permissions to use O_NOATIME (e.g., a process running as root in the virtual machine). Additionally, overlayfs on Linux opens files on the lower layer using O_NOATIME, so in this case a 9pfs mount can't be used as a lower layer for overlayfs (cf. https://github.com/osandov/drgn/blob/dabfe1971951701da13863dbe6d8a1d172ad9650/vmtest/onoatimehack.c and https://github.com/NixOS/nixpkgs/issues/54509). Luckily, O_NOATIME is effectively a hint, and is often ignored by, e.g., network filesystems. open(2) notes that O_NOATIME "may not be effective on all filesystems. One example is NFS, where the server maintains the access time." This means that we can honor it when possible but fall back to ignoring it. Acked-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Signed-off-by: Omar Sandoval <osandov@fb.com> Message-Id: <e9bee604e8df528584693a4ec474ded6295ce8ad.1587149256.git.osandov@fb.com> Signed-off-by: Greg Kurz <groug@kaod.org>
2020-03-109p/proxy: Fix export_flagsGreg Kurz1-2/+2
The common fsdev options are set by qemu_fsdev_add() before it calls the backend specific option parsing code. In the case of "proxy" this means "writeout" or "readonly" were simply ignored. This has been broken from the beginning. Reported-by: Stéphane Graber <stgraber@ubuntu.com> Signed-off-by: Greg Kurz <groug@kaod.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Message-Id: <158349633705.1237488.8895481990204796135.stgit@bahia.lan>
2020-02-08hw/9pfs/9p-synth: added directory for readdir testChristian Schoenebeck2-0/+24
This will provide the following virtual files by the 9pfs synth driver: - /ReadDirDir/ReadDirFile99 - /ReadDirDir/ReadDirFile98 ... - /ReadDirDir/ReadDirFile1 - /ReadDirDir/ReadDirFile0 This virtual directory and its virtual 100 files will be used by the upcoming 9pfs readdir tests. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <5408c28c8de25dd575b745cef63bf785305ccef2.1579567020.git.qemu_oss@crudebyte.com> Signed-off-by: Greg Kurz <groug@kaod.org>
2020-02-089pfs: validate count sent by client with T_readdirChristian Schoenebeck1-0/+9
A good 9p client sends T_readdir with "count" parameter that's sufficiently smaller than client's initially negotiated msize (maximum message size). We perform a check for that though to avoid the server to be interrupted with a "Failed to encode VirtFS reply type 41" transport error message by bad clients. This count value constraint uses msize - 11, because 11 is the header size of R_readdir. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <3990d3891e8ae2074709b56449e96ab4b4b93b7d.1579567020.git.qemu_oss@crudebyte.com> [groug: added comment ] Signed-off-by: Greg Kurz <groug@kaod.org>
2020-02-089pfs: require msize >= 4096Christian Schoenebeck2-0/+23
A client establishes a session by sending a Tversion request along with a 'msize' parameter which client uses to suggest server a maximum message size ever to be used for communication (for both requests and replies) between client and server during that session. If client suggests a 'msize' smaller than 4096 then deny session by server immediately with an error response (Rlerror for "9P2000.L" clients or Rerror for "9P2000.u" clients) instead of replying with Rversion. So far any msize submitted by client with Tversion was simply accepted by server without any check. Introduction of some minimum msize makes sense, because e.g. a msize < 7 would not allow any subsequent 9p operation at all, because 7 is the size of the header section common by all 9p message types. A substantial higher value of 4096 was chosen though to prevent potential issues with some message types. E.g. Rreadlink may yield up to a size of PATH_MAX which is usually 4096, and like almost all 9p message types, Rreadlink is not allowed to be truncated by the 9p protocol. This chosen size also prevents a similar issue with Rreaddir responses (provided client always sends adequate 'count' parameter with Treaddir), because even though directory entries retrieval may be split up over several T/Rreaddir messages; a Rreaddir response must not truncate individual directory entries though. So msize should be large enough to return at least one directory entry with the longest possible file name supported by host. Most file systems support a max. file name length of 255. Largest known file name lenght limit would be currently ReiserFS with max. 4032 bytes, which is also covered by this min. msize value because 4032 + 35 < 4096. Furthermore 4096 is already the minimum msize of the Linux kernel's 9pfs client. Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <8ceecb7fb9fdbeabbe55c04339349a36929fb8e3.1579567019.git.qemu_oss@crudebyte.com> Signed-off-by: Greg Kurz <groug@kaod.org>
2020-01-27Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into stagingPeter Maydell1-1/+1
* Register qdev properties as class properties (Marc-André) * Cleanups (Philippe) * virtio-scsi fix (Pan Nengyuan) * Tweak Skylake-v3 model id (Kashyap) * x86 UCODE_REV support and nested live migration fix (myself) * Advisory mode for pvpanic (Zhenwei) # gpg: Signature made Fri 24 Jan 2020 20:16:23 GMT # gpg: using RSA key BFFBD25F78C7AE83 # gpg: Good signature from "Paolo Bonzini <bonzini@gnu.org>" [full] # gpg: aka "Paolo Bonzini <pbonzini@redhat.com>" [full] # Primary key fingerprint: 46F5 9FBD 57D6 12E7 BFD4 E2F7 7E15 100C CD36 69B1 # Subkey fingerprint: F133 3857 4B66 2389 866C 7682 BFFB D25F 78C7 AE83 * remotes/bonzini/tags/for-upstream: (58 commits) build-sys: clean up flags included in the linker command line target/i386: Add the 'model-id' for Skylake -v3 CPU models qdev: use object_property_help() qapi/qmp: add ObjectPropertyInfo.default-value qom: introduce object_property_help() qom: simplify qmp_device_list_properties() vl: print default value in object help qdev: register properties as class properties qdev: move instance properties to class properties qdev: rename DeviceClass.props qdev: set properties with device_class_set_props() object: return self in object_ref() object: release all props object: add object_class_property_add_link() object: express const link with link property object: add direct link flag object: rename link "child" to "target" object: check strong flag with & object: do not free class properties object: add object_property_set_default ... Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-01-24qdev: set properties with device_class_set_props()Marc-André Lureau1-1/+1
The following patch will need to handle properties registration during class_init time. Let's use a device_class_set_props() setter. spatch --macro-file scripts/cocci-macro-file.h --sp-file ./scripts/coccinelle/qdev-set-props.cocci --keep-comments --in-place --dir . @@ typedef DeviceClass; DeviceClass *d; expression val; @@ - d->props = val + device_class_set_props(d, val) Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <20200110153039.1379601-20-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-01-22virtio-9p-device: convert to new virtio_delete_queuePan Nengyuan1-1/+1
Use virtio_delete_queue to make it more clear. Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> Message-Id: <20200117060927.51996-3-pannengyuan@huawei.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Acked-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
2020-01-22virtio-9p-device: fix memleak in virtio_9p_device_unrealizePan Nengyuan1-0/+1
v->vq forgot to cleanup in virtio_9p_device_unrealize, the memory leak stack is as follow: Direct leak of 14336 byte(s) in 2 object(s) allocated from: #0 0x7f819ae43970 (/lib64/libasan.so.5+0xef970) ??:? #1 0x7f819872f49d (/lib64/libglib-2.0.so.0+0x5249d) ??:? #2 0x55a3a58da624 (./x86_64-softmmu/qemu-system-x86_64+0x2c14624) /mnt/sdb/qemu/hw/virtio/virtio.c:2327 #3 0x55a3a571bac7 (./x86_64-softmmu/qemu-system-x86_64+0x2a55ac7) /mnt/sdb/qemu/hw/9pfs/virtio-9p-device.c:209 #4 0x55a3a58e7bc6 (./x86_64-softmmu/qemu-system-x86_64+0x2c21bc6) /mnt/sdb/qemu/hw/virtio/virtio.c:3504 #5 0x55a3a5ebfb37 (./x86_64-softmmu/qemu-system-x86_64+0x31f9b37) /mnt/sdb/qemu/hw/core/qdev.c:876 Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> Message-Id: <20200117060927.51996-2-pannengyuan@huawei.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Acked-by: Greg Kurz <groug@kaod.org>
2020-01-209pfs/9p.c: remove unneeded labelsDaniel Henrique Barboza1-6/+3
'out' label in v9fs_xattr_write() and 'out_nofid' label in v9fs_complete_rename() can be replaced by appropriate return calls. CC: Greg Kurz <groug@kaod.org> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Acked-by: Greg Kurz <groug@kaod.org> Signed-off-by: Greg Kurz <groug@kaod.org>
2020-01-209p: init_in_iov_from_pdu can truncate the sizeGreg Kurz4-21/+38
init_in_iov_from_pdu might not be able to allocate the full buffer size requested, which comes from the client and could be larger than the transport has available at the time of the request. Specifically, this can happen with read operations, with the client requesting a read up to the max allowed, which might be more than the transport has available at the time. Today the implementation of init_in_iov_from_pdu throws an error, both Xen and Virtio. Instead, change the V9fsTransport interface so that the size becomes a pointer and can be limited by the implementation of init_in_iov_from_pdu. Change both the Xen and Virtio implementations to set the size to the size of the buffer they managed to allocate, instead of throwing an error. However, if the allocated buffer size is less than P9_IOHDRSZ (the size of the header) still throw an error as the case is unhandable. Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com> CC: groug@kaod.org CC: anthony.perard@citrix.com CC: roman@zededa.com CC: qemu_oss@crudebyte.com [groug: fix 32-bit build] Signed-off-by: Greg Kurz <groug@kaod.org>
2020-01-209p: local: always return -1 on error in local_unlinkat_commonDaniel Henrique Barboza1-8/+6
local_unlinkat_common() is supposed to always return -1 on error. This is being done by jumps to the 'err_out' label, which is a 'return ret' call, and 'ret' is initialized with -1. Unfortunately there is a condition in which the function will return 0 on error: in a case where flags == AT_REMOVEDIR, 'ret' will be 0 when reaching map_dirfd = openat_dir(...) And, if map_dirfd == -1 and errno != ENOENT, the existing 'err_out' jump will execute 'return ret', when ret is still set to zero at that point. This patch fixes it by changing all 'err_out' labels by 'return -1' calls, ensuring that the function will always return -1 on error conditions. 'ret' can be left unintialized since it's now being used just to store the result of 'unlinkat' calls. CC: Greg Kurz <groug@kaod.org> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> [groug: changed prefix in title to be "9p: local:"] Signed-off-by: Greg Kurz <groug@kaod.org>
2020-01-209pfs: local: Fix possible memory leak in local_link()Jiajun Chen1-1/+1
There is a possible memory leak while local_link return -1 without free odirpath and oname. Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Jaijun Chen <chenjiajun8@huawei.com> Signed-off-by: Xiang Zheng <zhengxiang9@huawei.com> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Greg Kurz <groug@kaod.org>
2019-12-189pfs: make Error **errp const where it is appropriateVladimir Sementsov-Ogievskiy2-2/+2
Mostly, Error ** is for returning error from the function, so the callee sets it. However error_append_security_model_hint and error_append_socket_sockfd_hint get already filled errp parameter. They don't change the pointer itself, only change the internal state of referenced Error object. So we can make it Error *const * errp, to stress the behavior. It will also help coccinelle script (in future) to distinguish such cases from common errp usage. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Acked-by: Greg Kurz <groug@kaod.org> Message-Id: <20191205174635.18758-9-vsementsov@virtuozzo.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [Commit message replaced] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2019-11-239pfs: Fix divide by zero bugDan Schatzberg1-2/+4
Some filesystems may return 0s in statfs (trivially, a FUSE filesystem can do so). QEMU should handle this gracefully and just behave the same as if statfs failed. Signed-off-by: Dan Schatzberg <dschatzberg@fb.com> Acked-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Signed-off-by: Greg Kurz <groug@kaod.org>
2019-10-109p: Use variable length suffixes for inode remappingChristian Schoenebeck2-33/+279
Use variable length suffixes for inode remapping instead of the fixed 16 bit size prefixes before. With this change the inode numbers on guest will typically be much smaller (e.g. around >2^1 .. >2^7 instead of >2^48 with the previous fixed size inode remapping. Additionally this solution is more efficient, since inode numbers in practice can take almost their entire 64 bit range on guest as well, so there is less likely a need for generating and tracking additional suffixes, which might also be beneficial for nested virtualization where each level of virtualization would shift up the inode bits and increase the chance of expensive remapping actions. The "Exponential Golomb" algorithm is used as basis for generating the variable length suffixes. The algorithm has a parameter k which controls the distribution of bits on increasing indeces (minimum bits at low index vs. maximum bits at high index). With k=0 the generated suffixes look like: Index Dec/Bin -> Generated Suffix Bin 1 [1] -> [1] (1 bits) 2 [10] -> [010] (3 bits) 3 [11] -> [110] (3 bits) 4 [100] -> [00100] (5 bits) 5 [101] -> [10100] (5 bits) 6 [110] -> [01100] (5 bits) 7 [111] -> [11100] (5 bits) 8 [1000] -> [0001000] (7 bits) 9 [1001] -> [1001000] (7 bits) 10 [1010] -> [0101000] (7 bits) 11 [1011] -> [1101000] (7 bits) 12 [1100] -> [0011000] (7 bits) ... 65533 [1111111111111101] -> [1011111111111111000000000000000] (31 bits) 65534 [1111111111111110] -> [0111111111111111000000000000000] (31 bits) 65535 [1111111111111111] -> [1111111111111111000000000000000] (31 bits) Hence minBits=1 maxBits=31 And with k=5 they would look like: Index Dec/Bin -> Generated Suffix Bin 1 [1] -> [000001] (6 bits) 2 [10] -> [100001] (6 bits) 3 [11] -> [010001] (6 bits) 4 [100] -> [110001] (6 bits) 5 [101] -> [001001] (6 bits) 6 [110] -> [101001] (6 bits) 7 [111] -> [011001] (6 bits) 8 [1000] -> [111001] (6 bits) 9 [1001] -> [000101] (6 bits) 10 [1010] -> [100101] (6 bits) 11 [1011] -> [010101] (6 bits) 12 [1100] -> [110101] (6 bits) ... 65533 [1111111111111101] -> [0011100000000000100000000000] (28 bits) 65534 [1111111111111110] -> [1011100000000000100000000000] (28 bits) 65535 [1111111111111111] -> [0111100000000000100000000000] (28 bits) Hence minBits=6 maxBits=28 Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Signed-off-by: Greg Kurz <groug@kaod.org>
2019-10-109p: stat_to_qid: implement slow pathAntonios Motakis2-7/+76
stat_to_qid attempts via qid_path_prefixmap to map unique files (which are identified by 64 bit inode nr and 32 bit device id) to a 64 QID path value. However this implementation makes some assumptions about inode number generation on the host. If qid_path_prefixmap fails, we still have 48 bits available in the QID path to fall back to a less memory efficient full mapping. Signed-off-by: Antonios Motakis <antonios.motakis@huawei.com> [CS: - Rebased to https://github.com/gkurz/qemu/commits/9p-next (SHA1 7fc4c49e91). - Updated hash calls to new xxhash API. - Removed unnecessary parantheses in qpf_lookup_func(). - Removed unnecessary g_malloc0() result checks. - Log error message when running out of prefixes in qid_path_fullmap(). - Log warning message about potential degraded performance in qid_path_prefixmap(). - Wrapped qpf_table initialization to dedicated qpf_table_init() function. - Fixed typo in comment. ] Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Signed-off-by: Greg Kurz <groug@kaod.org>
2019-10-109p: Added virtfs option 'multidevs=remap|forbid|warn'Antonios Motakis3-21/+200
'warn' (default): Only log an error message (once) on host if more than one device is shared by same export, except of that just ignore this config error though. This is the default behaviour for not breaking existing installations implying that they really know what they are doing. 'forbid': Like 'warn', but except of just logging an error this also denies access of guest to additional devices. 'remap': Allows to share more than one device per export by remapping inodes from host to guest appropriately. To support multiple devices on the 9p share, and avoid qid path collisions we take the device id as input to generate a unique QID path. The lowest 48 bits of the path will be set equal to the file inode, and the top bits will be uniquely assigned based on the top 16 bits of the inode and the device id. Signed-off-by: Antonios Motakis <antonios.motakis@huawei.com> [CS: - Rebased to https://github.com/gkurz/qemu/commits/9p-next (SHA1 7fc4c49e91). - Added virtfs option 'multidevs', original patch simply did the inode remapping without being asked. - Updated hash calls to new xxhash API. - Updated docs for new option 'multidevs'. - Fixed v9fs_do_readdir() not having remapped inodes. - Log error message when running out of prefixes in qid_path_prefixmap(). - Fixed definition of QPATH_INO_MASK. - Wrapped qpp_table initialization to dedicated qpp_table_init() function. - Dropped unnecessary parantheses in qpp_lookup_func(). - Dropped unnecessary g_malloc0() result checks. ] Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> [groug: - Moved "multidevs" parsing to the local backend. - Added hint to invalid multidevs option error. - Turn "remap" into "x-remap". ] Signed-off-by: Greg Kurz <groug@kaod.org>
2019-10-109p: Treat multiple devices on one export as an errorAntonios Motakis2-14/+57
The QID path should uniquely identify a file. However, the inode of a file is currently used as the QID path, which on its own only uniquely identifies files within a device. Here we track the device hosting the 9pfs share, in order to prevent security issues with QID path collisions from other devices. We only print a warning for now but a subsequent patch will allow users to have finer control over the desired behaviour. Failing the I/O will be one the proposed behaviour, so we also change stat_to_qid() to return an error here in order to keep other patches simpler. Signed-off-by: Antonios Motakis <antonios.motakis@huawei.com> [CS: - Assign dev_id to export root's device already in v9fs_device_realize_common(), not postponed in stat_to_qid(). - error_report_once() if more than one device was shared by export. - Return -ENODEV instead of -ENOSYS in stat_to_qid(). - Fixed typo in log comment. ] Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> [groug, changed to warning, updated message and changelog] Signed-off-by: Greg Kurz <groug@kaod.org>
2019-10-10fsdev: Add return value to fsdev_throttle_parse_opts()Greg Kurz1-2/+1
It is more convenient to use the return value of the function to notify errors, rather than to be tied up setting up the &local_err boilerplate. Signed-off-by: Greg Kurz <groug@kaod.org>
2019-10-109p: Simplify error path of v9fs_device_realize_common()Greg Kurz3-10/+14
Make v9fs_device_unrealize_common() idempotent and use it for rollback, in order to reduce code duplication. Signed-off-by: Greg Kurz <groug@kaod.org>
2019-10-109p: unsigned type for type, version, pathAntonios Motakis2-10/+10
There is no need for signedness on these QID fields for 9p. Signed-off-by: Antonios Motakis <antonios.motakis@huawei.com> [CS: - Also make QID type unsigned. - Adjust donttouch_stat() to new types. - Adjust trace-events to new types. ] Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Signed-off-by: Greg Kurz <groug@kaod.org>
generated by cgit v1.1 at 2024-12-29 11:57:10 +0000