qemu-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-commits] [qemu/qemu] 5bb832: virtiofsd: Release vu_dispatch_lock w


From: Peter Maydell
Subject: [Qemu-commits] [qemu/qemu] 5bb832: virtiofsd: Release vu_dispatch_lock when stopping ...
Date: Wed, 17 Mar 2021 03:38:40 -0700

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 5bb8327b655dbce10a91ef809acb0875dd0ee0ed
      
https://github.com/qemu/qemu/commit/5bb8327b655dbce10a91ef809acb0875dd0ee0ed
  Author: Greg Kurz <groug@kaod.org>
  Date:   2021-03-15 (Mon, 15 Mar 2021)

  Changed paths:
    M tools/virtiofsd/fuse_virtio.c

  Log Message:
  -----------
  virtiofsd: Release vu_dispatch_lock when stopping queue

QEMU can stop a virtqueue by sending a VHOST_USER_GET_VRING_BASE request
to virtiofsd. As with all other vhost-user protocol messages, the thread
that runs the main event loop in virtiofsd takes the vu_dispatch lock in
write mode. This ensures that no other thread can access virtqueues or
memory tables at the same time.

In the case of VHOST_USER_GET_VRING_BASE, the main thread basically
notifies the queue thread that it should terminate and waits for its
termination:

main()
 virtio_loop()
  vu_dispatch_wrlock()
  vu_dispatch()
   vu_process_message()
    vu_get_vring_base_exec()
     fv_queue_cleanup_thread()
      pthread_join()

Unfortunately, the queue thread ends up calling virtio_send_msg()
at some point, which itself needs to grab the lock:

fv_queue_thread()
 g_list_foreach()
  fv_queue_worker()
   fuse_session_process_buf_int()
    do_release()
     lo_release()
      fuse_reply_err()
       send_reply()
        send_reply_iov()
         fuse_send_reply_iov_nofree()
          fuse_send_msg()
           virtio_send_msg()
            vu_dispatch_rdlock() <-- Deadlock !

Simply have the main thread to release the lock before going to
sleep and take it back afterwards. A very similar patch was already
sent by Vivek Goyal sometime back:

https://listman.redhat.com/archives/virtio-fs/2021-January/msg00073.html

The only difference here is that this done in fv_queue_set_started()
because fv_queue_cleanup_thread() can also be called from virtio_loop()
without the lock being held.

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Vivek Goyal <vgoyal@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210312092212.782255-8-groug@kaod.org>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


  Commit: 6d118c4349966a1890d00bbbdc42001f173c6e4d
      
https://github.com/qemu/qemu/commit/6d118c4349966a1890d00bbbdc42001f173c6e4d
  Author: Vivek Goyal <vgoyal@redhat.com>
  Date:   2021-03-15 (Mon, 15 Mar 2021)

  Changed paths:
    M tools/virtiofsd/passthrough_ll.c

  Log Message:
  -----------
  virtiofsd: Add qemu version and copyright info

Option "-V" currently displays the fuse protocol version virtiofsd is
using. For example, I see this.

$ ./virtiofsd -V
"using FUSE kernel interface version 7.33"

People also want to know software version of virtiofsd so that they can
figure out if a certain fix is part of currently running virtiofsd or
not. Eric Ernst ran into this issue.

David Gilbert thinks that it probably is best that we simply carry the
qemu version and display that information given we are part of qemu
tree.

So this patch enhances version information and also adds qemu version
and copyright info. Not sure if copyright information is supposed
to be displayed along with version info. Given qemu-storage-daemon
and other utilities are doing it, so I continued with same pattern.
This is how now output looks like.

$ ./virtiofsd -V
virtiofsd version 5.2.50 (v5.2.0-2357-gcbcf09872a-dirty)
Copyright (c) 2003-2020 Fabrice Bellard and the QEMU Project developers
using FUSE kernel interface version 7.33

Reported-by: Eric Ernst <eric.g.ernst@gmail.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Message-Id: <20210303195339.GB3793@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Sergio Lopez <slp@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


  Commit: 28d1ad0ea41342472afda15b515d95671eac4030
      
https://github.com/qemu/qemu/commit/28d1ad0ea41342472afda15b515d95671eac4030
  Author: Greg Kurz <groug@kaod.org>
  Date:   2021-03-15 (Mon, 15 Mar 2021)

  Changed paths:
    M tools/virtiofsd/passthrough_ll.c

  Log Message:
  -----------
  virtiofsd: Don't allow empty filenames

POSIX.1-2017 clearly stipulates that empty filenames aren't
allowed ([1] and [2]). Since virtiofsd is supposed to mirror
the host file system hierarchy and the host can be assumed to
be linux, we don't really expect clients to pass requests with
an empty path in it. If they do so anyway, this would eventually
cause an error when trying to create/lookup the actual inode
on the underlying POSIX filesystem. But this could still confuse
some code that wouldn't be ready to cope with this.

Filter out empty names coming from the client at the top level,
so that the rest doesn't have to care about it. This is done
everywhere we already call is_safe_path_component(), but
in a separate helper since the usual error for empty path
names is ENOENT instead of EINVAL.

[1] 
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_170
[2] 
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_13

Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <20210312141003.819108-4-groug@kaod.org>
Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


  Commit: 20afcc23b3212784c84fb06062f66d9d2ce6865d
      
https://github.com/qemu/qemu/commit/20afcc23b3212784c84fb06062f66d9d2ce6865d
  Author: Greg Kurz <groug@kaod.org>
  Date:   2021-03-15 (Mon, 15 Mar 2021)

  Changed paths:
    M tools/virtiofsd/passthrough_ll.c

  Log Message:
  -----------
  virtiofsd: Don't allow empty paths in lookup_name()

When passed an empty filename, lookup_name() returns the inode of
the parent directory, unless the parent is the root in which case
the st_dev doesn't match and lo_find() returns NULL. This is
because lookup_name() passes AT_EMPTY_PATH down to fstatat() or
statx().

This behavior doesn't quite make sense because users of lookup_name()
then pass the name to unlinkat(), renameat() or renameat2(), all of
which will always fail on empty names.

Drop AT_EMPTY_PATH from the flags in lookup_name() so that it has
the consistent behavior of "returning an existing child inode or
NULL" for all directories.

Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <20210312141003.819108-2-groug@kaod.org>
Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
Reviewed-by: Vivek Goyal <vgoyal@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


  Commit: 03ccaaae48fe1bd3ee0842717008fe74d7745680
      
https://github.com/qemu/qemu/commit/03ccaaae48fe1bd3ee0842717008fe74d7745680
  Author: Greg Kurz <groug@kaod.org>
  Date:   2021-03-15 (Mon, 15 Mar 2021)

  Changed paths:
    M tools/virtiofsd/passthrough_ll.c

  Log Message:
  -----------
  virtiofsd: Convert some functions to return bool

Both currently only return 0 or 1.

Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <20210312141003.819108-3-groug@kaod.org>
Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
Reviewed-by: Vivek Goyal <vgoyal@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


  Commit: a339149afa50578380bf8a7c1ed5ae7061431db4
      
https://github.com/qemu/qemu/commit/a339149afa50578380bf8a7c1ed5ae7061431db4
  Author: Hao Wang <wanghao232@huawei.com>
  Date:   2021-03-15 (Mon, 15 Mar 2021)

  Changed paths:
    M migration/multifd.c

  Log Message:
  -----------
  migration/tls: fix inverted semantics in multifd_channel_connect

Function multifd_channel_connect() return "true" to indicate failure,
which is rather confusing. Fix that.

Signed-off-by: Hao Wang <wanghao232@huawei.com>
Message-Id: <20210209104237.2250941-2-wanghao232@huawei.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Chuan Zheng <zhengchuan@huawei.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


  Commit: fca676429ca7f309b5d492c7675d35fec484197c
      
https://github.com/qemu/qemu/commit/fca676429ca7f309b5d492c7675d35fec484197c
  Author: Hao Wang <wanghao232@huawei.com>
  Date:   2021-03-15 (Mon, 15 Mar 2021)

  Changed paths:
    M migration/multifd.c

  Log Message:
  -----------
  migration/tls: add error handling in multifd_tls_handshake_thread

If any error happens during multifd send thread creating (e.g. channel broke
because new domain is destroyed by the dst), multifd_tls_handshake_thread
may exit silently, leaving main migration thread hanging (ram_save_setup ->
multifd_send_sync_main -> qemu_sem_wait(&p->sem_sync)).
Fix that by adding error handling in multifd_tls_handshake_thread.

Signed-off-by: Hao Wang <wanghao232@huawei.com>
Message-Id: <20210209104237.2250941-3-wanghao232@huawei.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Chuan Zheng <zhengchuan@huawei.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


  Commit: a8e2ab5db2181b68f371ee794e1a0fe7ca6f5e24
      
https://github.com/qemu/qemu/commit/a8e2ab5db2181b68f371ee794e1a0fe7ca6f5e24
  Author: Mahmoud Mandour <ma.mandourr@gmail.com>
  Date:   2021-03-15 (Mon, 15 Mar 2021)

  Changed paths:
    M monitor/monitor.c
    M monitor/qmp.c

  Log Message:
  -----------
  monitor: Replaced qemu_mutex_lock calls with QEMU_LOCK_GUARD

Removed various qemu_mutex_lock and their respective qemu_mutex_unlock
calls and used lock guard macros (QEMU_LOCK_GUARD and
WITH_QEMU_LOCK_GUARD). This simplifies the code by
eliminating qemu_mutex_unlock calls.

Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
Message-Id: <20210311031538.5325-6-ma.mandourr@gmail.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


  Commit: 373969507a3dc7de2d291da7e1bd03acf46ec643
      
https://github.com/qemu/qemu/commit/373969507a3dc7de2d291da7e1bd03acf46ec643
  Author: Mahmoud Mandour <ma.mandourr@gmail.com>
  Date:   2021-03-15 (Mon, 15 Mar 2021)

  Changed paths:
    M migration/migration.c
    M migration/ram.c

  Log Message:
  -----------
  migration: Replaced qemu_mutex_lock calls with QEMU_LOCK_GUARD

Replaced various qemu_mutex_lock calls and their respective
qemu_mutex_unlock calls with QEMU_LOCK_GUARD macro. This simplifies
the code by eliminating the respective qemu_mutex_unlock calls.

Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
Message-Id: <20210311031538.5325-7-ma.mandourr@gmail.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


  Commit: 5d1428d6c43942cfb40a909e4c30a5cbb81bda8f
      
https://github.com/qemu/qemu/commit/5d1428d6c43942cfb40a909e4c30a5cbb81bda8f
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   2021-03-17 (Wed, 17 Mar 2021)

  Changed paths:
    M migration/migration.c
    M migration/multifd.c
    M migration/ram.c
    M monitor/monitor.c
    M monitor/qmp.c
    M tools/virtiofsd/fuse_virtio.c
    M tools/virtiofsd/passthrough_ll.c

  Log Message:
  -----------
  Merge remote-tracking branch 
'remotes/dgilbert-gitlab/tags/pull-virtiofs-20210315' into staging

virtiofs and migration pull 2021-03-15

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

# gpg: Signature made Mon 15 Mar 2021 20:03:03 GMT
# gpg:                using RSA key 45F5C71B4A0CB7FB977A9FA90516331EBC5BFDE7
# gpg: Good signature from "Dr. David Alan Gilbert (RH2) <dgilbert@redhat.com>" 
[full]
# Primary key fingerprint: 45F5 C71B 4A0C B7FB 977A  9FA9 0516 331E BC5B FDE7

* remotes/dgilbert-gitlab/tags/pull-virtiofs-20210315:
  migration: Replaced qemu_mutex_lock calls with QEMU_LOCK_GUARD
  monitor: Replaced qemu_mutex_lock calls with QEMU_LOCK_GUARD
  migration/tls: add error handling in multifd_tls_handshake_thread
  migration/tls: fix inverted semantics in multifd_channel_connect
  virtiofsd: Convert some functions to return bool
  virtiofsd: Don't allow empty paths in lookup_name()
  virtiofsd: Don't allow empty filenames
  virtiofsd: Add qemu version and copyright info
  virtiofsd: Release vu_dispatch_lock when stopping queue

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Compare: https://github.com/qemu/qemu/compare/0693602a2327...5d1428d6c439



reply via email to

[Prev in Thread] Current Thread [Next in Thread]