All of lore.kernel.org
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: qemu-devel@nongnu.org, virtio-fs@redhat.com
Cc: "Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Max Reitz <mreitz@redhat.com>
Subject: [PATCH v2 1/9] virtiofsd: Add TempFd structure
Date: Wed,  9 Jun 2021 17:55:43 +0200	[thread overview]
Message-ID: <20210609155551.44437-2-mreitz@redhat.com> (raw)
In-Reply-To: <20210609155551.44437-1-mreitz@redhat.com>

We are planning to add file handles to lo_inode objects as an
alternative to lo_inode.fd.  That means that everywhere where we
currently reference lo_inode.fd, we will have to open a temporary file
descriptor that needs to be closed after use.

So instead of directly accessing lo_inode.fd, there will be a helper
function (lo_inode_fd()) that either returns lo_inode.fd, or opens a new
file descriptor with open_by_handle_at().  It encapsulates this result
in a TempFd structure to let the caller know whether the FD needs to be
closed after use (opened from the handle) or not (copied from
lo_inode.fd).

By using g_auto(TempFd) to store this result, callers will not even have
to care about closing a temporary FD after use.  It will be done
automatically once the object goes out of scope.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
---
 tools/virtiofsd/passthrough_ll.c | 49 ++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 49c21fd855..a4674aba80 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -174,6 +174,28 @@ struct lo_data {
     int user_killpriv_v2, killpriv_v2;
 };
 
+/**
+ * Represents a file descriptor that may either be owned by this
+ * TempFd, or only referenced (i.e. the ownership belongs to some
+ * other object, and the value has just been copied into this TempFd).
+ *
+ * The purpose of this encapsulation is to be used as g_auto(TempFd)
+ * to automatically clean up owned file descriptors when this object
+ * goes out of scope.
+ *
+ * Use temp_fd_steal() to get an owned file descriptor that will not
+ * be closed when the TempFd goes out of scope.
+ */
+typedef struct {
+    int fd;
+    bool owned; /* fd owned by this object? */
+} TempFd;
+
+#define TEMP_FD_INIT ((TempFd) { .fd = -1, .owned = false })
+
+static void temp_fd_clear(TempFd *temp_fd);
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(TempFd, temp_fd_clear);
+
 static const struct fuse_opt lo_opts[] = {
     { "sandbox=namespace",
       offsetof(struct lo_data, sandbox),
@@ -249,6 +271,33 @@ static struct lo_data *lo_data(fuse_req_t req)
     return (struct lo_data *)fuse_req_userdata(req);
 }
 
+/**
+ * Clean-up function for TempFds
+ */
+static void temp_fd_clear(TempFd *temp_fd)
+{
+    if (temp_fd->owned) {
+        close(temp_fd->fd);
+        *temp_fd = TEMP_FD_INIT;
+    }
+}
+
+/**
+ * Return an owned fd from *temp_fd that will not be closed when
+ * *temp_fd goes out of scope.
+ *
+ * (TODO: Remove __attribute__ once this is used.)
+ */
+static __attribute__((unused)) int temp_fd_steal(TempFd *temp_fd)
+{
+    if (temp_fd->owned) {
+        temp_fd->owned = false;
+        return temp_fd->fd;
+    } else {
+        return dup(temp_fd->fd);
+    }
+}
+
 /*
  * Load capng's state from our saved state if the current thread
  * hadn't previously been loaded.
-- 
2.31.1



WARNING: multiple messages have this Message-ID (diff)
From: Max Reitz <mreitz@redhat.com>
To: qemu-devel@nongnu.org, virtio-fs@redhat.com
Cc: Max Reitz <mreitz@redhat.com>
Subject: [Virtio-fs] [PATCH v2 1/9] virtiofsd: Add TempFd structure
Date: Wed,  9 Jun 2021 17:55:43 +0200	[thread overview]
Message-ID: <20210609155551.44437-2-mreitz@redhat.com> (raw)
In-Reply-To: <20210609155551.44437-1-mreitz@redhat.com>

We are planning to add file handles to lo_inode objects as an
alternative to lo_inode.fd.  That means that everywhere where we
currently reference lo_inode.fd, we will have to open a temporary file
descriptor that needs to be closed after use.

So instead of directly accessing lo_inode.fd, there will be a helper
function (lo_inode_fd()) that either returns lo_inode.fd, or opens a new
file descriptor with open_by_handle_at().  It encapsulates this result
in a TempFd structure to let the caller know whether the FD needs to be
closed after use (opened from the handle) or not (copied from
lo_inode.fd).

By using g_auto(TempFd) to store this result, callers will not even have
to care about closing a temporary FD after use.  It will be done
automatically once the object goes out of scope.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
---
 tools/virtiofsd/passthrough_ll.c | 49 ++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 49c21fd855..a4674aba80 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -174,6 +174,28 @@ struct lo_data {
     int user_killpriv_v2, killpriv_v2;
 };
 
+/**
+ * Represents a file descriptor that may either be owned by this
+ * TempFd, or only referenced (i.e. the ownership belongs to some
+ * other object, and the value has just been copied into this TempFd).
+ *
+ * The purpose of this encapsulation is to be used as g_auto(TempFd)
+ * to automatically clean up owned file descriptors when this object
+ * goes out of scope.
+ *
+ * Use temp_fd_steal() to get an owned file descriptor that will not
+ * be closed when the TempFd goes out of scope.
+ */
+typedef struct {
+    int fd;
+    bool owned; /* fd owned by this object? */
+} TempFd;
+
+#define TEMP_FD_INIT ((TempFd) { .fd = -1, .owned = false })
+
+static void temp_fd_clear(TempFd *temp_fd);
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(TempFd, temp_fd_clear);
+
 static const struct fuse_opt lo_opts[] = {
     { "sandbox=namespace",
       offsetof(struct lo_data, sandbox),
@@ -249,6 +271,33 @@ static struct lo_data *lo_data(fuse_req_t req)
     return (struct lo_data *)fuse_req_userdata(req);
 }
 
+/**
+ * Clean-up function for TempFds
+ */
+static void temp_fd_clear(TempFd *temp_fd)
+{
+    if (temp_fd->owned) {
+        close(temp_fd->fd);
+        *temp_fd = TEMP_FD_INIT;
+    }
+}
+
+/**
+ * Return an owned fd from *temp_fd that will not be closed when
+ * *temp_fd goes out of scope.
+ *
+ * (TODO: Remove __attribute__ once this is used.)
+ */
+static __attribute__((unused)) int temp_fd_steal(TempFd *temp_fd)
+{
+    if (temp_fd->owned) {
+        temp_fd->owned = false;
+        return temp_fd->fd;
+    } else {
+        return dup(temp_fd->fd);
+    }
+}
+
 /*
  * Load capng's state from our saved state if the current thread
  * hadn't previously been loaded.
-- 
2.31.1


  reply	other threads:[~2021-06-09 15:58 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-09 15:55 [PATCH v2 0/9] virtiofsd: Allow using file handles instead of O_PATH FDs Max Reitz
2021-06-09 15:55 ` [Virtio-fs] " Max Reitz
2021-06-09 15:55 ` Max Reitz [this message]
2021-06-09 15:55   ` [Virtio-fs] [PATCH v2 1/9] virtiofsd: Add TempFd structure Max Reitz
2021-06-09 15:55 ` [PATCH v2 2/9] virtiofsd: Use lo_inode_open() instead of openat() Max Reitz
2021-06-09 15:55   ` [Virtio-fs] " Max Reitz
2021-06-09 15:55 ` [PATCH v2 3/9] virtiofsd: Add lo_inode_fd() helper Max Reitz
2021-06-09 15:55   ` [Virtio-fs] " Max Reitz
2021-06-09 15:55 ` [PATCH v2 4/9] virtiofsd: Let lo_fd() return a TempFd Max Reitz
2021-06-09 15:55   ` [Virtio-fs] " Max Reitz
2021-06-09 15:55 ` [PATCH v2 5/9] virtiofsd: Let lo_inode_open() " Max Reitz
2021-06-09 15:55   ` [Virtio-fs] " Max Reitz
2021-06-09 15:55 ` [PATCH v2 6/9] virtiofsd: Add lo_inode.fhandle Max Reitz
2021-06-09 15:55   ` [Virtio-fs] " Max Reitz
2021-06-09 15:55 ` [PATCH v2 7/9] virtiofsd: Add inodes_by_handle hash table Max Reitz
2021-06-09 15:55   ` [Virtio-fs] " Max Reitz
2021-06-11 20:04   ` Vivek Goyal
2021-06-16 13:38     ` Max Reitz
2021-06-17 21:21       ` Vivek Goyal
2021-06-18  8:28         ` Max Reitz
2021-06-18 18:29           ` Vivek Goyal
2021-06-21  9:02             ` Max Reitz
2021-06-21 15:51               ` Vivek Goyal
2021-06-21 17:07                 ` Max Reitz
2021-06-21 21:27                   ` Vivek Goyal
2021-07-13 15:07               ` Max Reitz
2021-07-20 14:50                 ` Vivek Goyal
2021-07-21  8:29                   ` Max Reitz
2021-06-18  8:30   ` Max Reitz
2021-06-18  8:30     ` [Virtio-fs] " Max Reitz
2021-06-09 15:55 ` [PATCH v2 8/9] virtiofsd: Optionally fill lo_inode.fhandle Max Reitz
2021-06-09 15:55   ` [Virtio-fs] " Max Reitz
2021-06-09 15:55 ` [PATCH v2 9/9] virtiofsd: Add lazy lo_do_find() Max Reitz
2021-06-09 15:55   ` [Virtio-fs] " Max Reitz
2021-06-11 19:19 ` [PATCH v2 0/9] virtiofsd: Allow using file handles instead of O_PATH FDs Vivek Goyal
2021-06-11 19:19   ` [Virtio-fs] " Vivek Goyal
2021-06-16 13:41   ` Max Reitz
2021-06-16 13:41     ` [Virtio-fs] " Max Reitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210609155551.44437-2-mreitz@redhat.com \
    --to=mreitz@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=virtio-fs@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.