friendly ping (see also https://patchew.org/QEMU/cover.1725607795.git.mv...@redhat.com/)
Please let me know if there is anything I can do to make this easier to review or if I should split or help otherwise. On Fri, Sep 6, 2024 at 9:39 AM Michael Vogt <michael.v...@gmail.com> wrote: > Hi, > > This is v4 of the openat2 support in linux-user. Thanks again for the > excellent second round of feedback from Richard Henderson. > > The code is identical to the previous v3 and I only fixed two typos in > the commit message. I'm sending v4 because in v3 I forgot to add > "--threaded" when generating the coverletter/patch which makes it a bit > awkward to review and it does not show up properly on > e.g. https://patchew.org/QEMU/. My apologies for this mistake. > > This version tries to be closer to the kernels behavior, i.e. now > do_openat2() uses a new copy_struct_from_user() helper that is very > similar to the kernels. This lead me to also drop incuding openat2.h > (as was originally suggested in the v1 review). It now contains it as > a copy named `struct open_how_ver0` and with that we can LOG_UNIMP if > the struct ever grows and qemu-user needs updating. > > To answer the question why "maybe_do_fake_open()" uses a > "use_returned_fd" bool instead of just returning "-1": I wanted to be > as close as possible to the previous behavior and maybe_fake_open() > could in theory return "-1" for failures in memfd_create() or > mkstemp() or fake_open->fill(). In those cases the old code in > do_guest_openat() failed and returned the error but the new code would > just see a -1 and continue trying to open a special file that should > have been faked. Maybe I did overthink this as it's very > corner-case-y. Advise is welcome here, happy to change back or > simplify in other ways. > > Thanks again, > Michael > > v3 -> v4: > - fix typos in the commit message > > v2 -> v3: > - fix coding style (braches) > - improve argument args/naming in do_openat2() > - merge do_openat2/do_guest_openat2 > - do size checks first in do_openat2 > - add "copy_struct_from_user" and use in "do_openat2()" > - drop using openat2.h and create "struct open_how_v0" > - log if open_how guest struct is bigger than our supported struct > > v1 -> v2: > - do not include <sys/syscall.h> > - drop do_guest_openat2 from qemu.h and make static > - drop "safe" from do_guest_openat2 > - ensure maybe_do_fake_open() is correct about when the result should > be used or not > - Extract do_openat2() helper from do_syscall1() > - Call user_unlock* if a lock call fails > - Fix silly incorrect use of "target_open_how" when "open_how" is required > - Fix coding style comments > - Fix validation of arg4 in openat2 > - Fix missing zero initialization of open_how > - Define target_open_how with abi_* types > - Warn about unimplemented size if "size" of openat2 is bigger than > target_open_how > > > Michael Vogt (1): > linux-user: add openat2 support in linux-user > > linux-user/syscall.c | 116 ++++++++++++++++++++++++++++++++++++-- > linux-user/syscall_defs.h | 7 +++ > 2 files changed, 119 insertions(+), 4 deletions(-) > > -- > 2.45.2 > >
