* [PULL 01/12] virtiofsd: Fix side-effect in assert()
2021-05-06 18:56 [PULL 00/12] virtiofs queue Dr. David Alan Gilbert (git)
@ 2021-05-06 18:56 ` Dr. David Alan Gilbert (git)
2021-05-06 19:11 ` [Virtio-fs] " Edward McClanahan
2021-05-06 18:56 ` [PULL 02/12] virtiofsd: Allow use "-o xattrmap" without "-o xattr" Dr. David Alan Gilbert (git)
` (11 subsequent siblings)
12 siblings, 1 reply; 15+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2021-05-06 18:56 UTC (permalink / raw)
To: qemu-devel, groug, jose.carlos.venegas.munoz, ma.mandourr
Cc: virtio-fs, vgoyal, stefanha
From: Greg Kurz <groug@kaod.org>
It is bad practice to put an expression with a side-effect in
assert() because the side-effect won't happen if the code is
compiled with -DNDEBUG.
Use an intermediate variable. Consolidate this in an macro to
have proper line numbers when the assertion is hit.
virtiofsd: ../../tools/virtiofsd/passthrough_ll.c:2797: lo_getxattr:
Assertion `fchdir_res == 0' failed.
Aborted
2796 /* fchdir should not fail here */
=>2797 FCHDIR_NOFAIL(lo->proc_self_fd);
2798 ret = getxattr(procname, name, value, size);
2799 FCHDIR_NOFAIL(lo->root.fd);
Fixes: bdfd66788349 ("virtiofsd: Fix xattr operations")
Cc: misono.tomohiro@jp.fujitsu.com
Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <20210409100627.451573-1-groug@kaod.org>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tools/virtiofsd/passthrough_ll.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 1553d2ef45..6592f96f68 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2723,6 +2723,11 @@ static int xattr_map_server(const struct lo_data *lo, const char *server_name,
return -ENODATA;
}
+#define FCHDIR_NOFAIL(fd) do { \
+ int fchdir_res = fchdir(fd); \
+ assert(fchdir_res == 0); \
+ } while (0)
+
static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name,
size_t size)
{
@@ -2789,9 +2794,9 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name,
ret = fgetxattr(fd, name, value, size);
} else {
/* fchdir should not fail here */
- assert(fchdir(lo->proc_self_fd) == 0);
+ FCHDIR_NOFAIL(lo->proc_self_fd);
ret = getxattr(procname, name, value, size);
- assert(fchdir(lo->root.fd) == 0);
+ FCHDIR_NOFAIL(lo->root.fd);
}
if (ret == -1) {
@@ -2864,9 +2869,9 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size)
ret = flistxattr(fd, value, size);
} else {
/* fchdir should not fail here */
- assert(fchdir(lo->proc_self_fd) == 0);
+ FCHDIR_NOFAIL(lo->proc_self_fd);
ret = listxattr(procname, value, size);
- assert(fchdir(lo->root.fd) == 0);
+ FCHDIR_NOFAIL(lo->root.fd);
}
if (ret == -1) {
@@ -3000,9 +3005,9 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name,
ret = fsetxattr(fd, name, value, size, flags);
} else {
/* fchdir should not fail here */
- assert(fchdir(lo->proc_self_fd) == 0);
+ FCHDIR_NOFAIL(lo->proc_self_fd);
ret = setxattr(procname, name, value, size, flags);
- assert(fchdir(lo->root.fd) == 0);
+ FCHDIR_NOFAIL(lo->root.fd);
}
saverr = ret == -1 ? errno : 0;
@@ -3066,9 +3071,9 @@ static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *in_name)
ret = fremovexattr(fd, name);
} else {
/* fchdir should not fail here */
- assert(fchdir(lo->proc_self_fd) == 0);
+ FCHDIR_NOFAIL(lo->proc_self_fd);
ret = removexattr(procname, name);
- assert(fchdir(lo->root.fd) == 0);
+ FCHDIR_NOFAIL(lo->root.fd);
}
saverr = ret == -1 ? errno : 0;
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Virtio-fs] [PULL 01/12] virtiofsd: Fix side-effect in assert()
2021-05-06 18:56 ` [PULL 01/12] virtiofsd: Fix side-effect in assert() Dr. David Alan Gilbert (git)
@ 2021-05-06 19:11 ` Edward McClanahan
0 siblings, 0 replies; 15+ messages in thread
From: Edward McClanahan @ 2021-05-06 19:11 UTC (permalink / raw)
To: qemu-devel, groug, jose.carlos.venegas.munoz, ma.mandourr,
Dr. David Alan Gilbert (git)
Cc: virtio-fs, vgoyal
[-- Attachment #1: Type: text/plain, Size: 4533 bytes --]
Very nice catch David... Countless times I've gotten bit by this one!
Get Outlook for Android<https://aka.ms/AAb9ysg>
________________________________
From: virtio-fs-bounces@redhat.com <virtio-fs-bounces@redhat.com> on behalf of Dr. David Alan Gilbert (git) <dgilbert@redhat.com>
Sent: Thursday, May 6, 2021 1:56:30 PM
To: qemu-devel@nongnu.org <qemu-devel@nongnu.org>; groug@kaod.org <groug@kaod.org>; jose.carlos.venegas.munoz@intel.com <jose.carlos.venegas.munoz@intel.com>; ma.mandourr@gmail.com <ma.mandourr@gmail.com>
Cc: virtio-fs@redhat.com <virtio-fs@redhat.com>; vgoyal@redhat.com <vgoyal@redhat.com>
Subject: [Virtio-fs] [PULL 01/12] virtiofsd: Fix side-effect in assert()
External email: Use caution opening links or attachments
From: Greg Kurz <groug@kaod.org>
It is bad practice to put an expression with a side-effect in
assert() because the side-effect won't happen if the code is
compiled with -DNDEBUG.
Use an intermediate variable. Consolidate this in an macro to
have proper line numbers when the assertion is hit.
virtiofsd: ../../tools/virtiofsd/passthrough_ll.c:2797: lo_getxattr:
Assertion `fchdir_res == 0' failed.
Aborted
2796 /* fchdir should not fail here */
=>2797 FCHDIR_NOFAIL(lo->proc_self_fd);
2798 ret = getxattr(procname, name, value, size);
2799 FCHDIR_NOFAIL(lo->root.fd);
Fixes: bdfd66788349 ("virtiofsd: Fix xattr operations")
Cc: misono.tomohiro@jp.fujitsu.com
Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <20210409100627.451573-1-groug@kaod.org>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tools/virtiofsd/passthrough_ll.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 1553d2ef45..6592f96f68 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2723,6 +2723,11 @@ static int xattr_map_server(const struct lo_data *lo, const char *server_name,
return -ENODATA;
}
+#define FCHDIR_NOFAIL(fd) do { \
+ int fchdir_res = fchdir(fd); \
+ assert(fchdir_res == 0); \
+ } while (0)
+
static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name,
size_t size)
{
@@ -2789,9 +2794,9 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name,
ret = fgetxattr(fd, name, value, size);
} else {
/* fchdir should not fail here */
- assert(fchdir(lo->proc_self_fd) == 0);
+ FCHDIR_NOFAIL(lo->proc_self_fd);
ret = getxattr(procname, name, value, size);
- assert(fchdir(lo->root.fd) == 0);
+ FCHDIR_NOFAIL(lo->root.fd);
}
if (ret == -1) {
@@ -2864,9 +2869,9 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size)
ret = flistxattr(fd, value, size);
} else {
/* fchdir should not fail here */
- assert(fchdir(lo->proc_self_fd) == 0);
+ FCHDIR_NOFAIL(lo->proc_self_fd);
ret = listxattr(procname, value, size);
- assert(fchdir(lo->root.fd) == 0);
+ FCHDIR_NOFAIL(lo->root.fd);
}
if (ret == -1) {
@@ -3000,9 +3005,9 @@ static void lo_setxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name,
ret = fsetxattr(fd, name, value, size, flags);
} else {
/* fchdir should not fail here */
- assert(fchdir(lo->proc_self_fd) == 0);
+ FCHDIR_NOFAIL(lo->proc_self_fd);
ret = setxattr(procname, name, value, size, flags);
- assert(fchdir(lo->root.fd) == 0);
+ FCHDIR_NOFAIL(lo->root.fd);
}
saverr = ret == -1 ? errno : 0;
@@ -3066,9 +3071,9 @@ static void lo_removexattr(fuse_req_t req, fuse_ino_t ino, const char *in_name)
ret = fremovexattr(fd, name);
} else {
/* fchdir should not fail here */
- assert(fchdir(lo->proc_self_fd) == 0);
+ FCHDIR_NOFAIL(lo->proc_self_fd);
ret = removexattr(procname, name);
- assert(fchdir(lo->root.fd) == 0);
+ FCHDIR_NOFAIL(lo->root.fd);
}
saverr = ret == -1 ? errno : 0;
--
2.31.1
_______________________________________________
Virtio-fs mailing list
Virtio-fs@redhat.com
https://listman.redhat.com/mailman/listinfo/virtio-fs
[-- Attachment #2: Type: text/html, Size: 8163 bytes --]
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PULL 02/12] virtiofsd: Allow use "-o xattrmap" without "-o xattr"
2021-05-06 18:56 [PULL 00/12] virtiofs queue Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 01/12] virtiofsd: Fix side-effect in assert() Dr. David Alan Gilbert (git)
@ 2021-05-06 18:56 ` Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 03/12] virtiofsd: Add help for -o xattr-mapping Dr. David Alan Gilbert (git)
` (10 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2021-05-06 18:56 UTC (permalink / raw)
To: qemu-devel, groug, jose.carlos.venegas.munoz, ma.mandourr
Cc: virtio-fs, vgoyal, stefanha
From: Carlos Venegas <jose.carlos.venegas.munoz@intel.com>
When -o xattrmap is used, it will not work unless xattr is enabled.
This patch enables xattr when -o xattrmap is used.
Signed-off-by: Carlos Venegas <jose.carlos.venegas.munoz@intel.com>
Message-Id: <20210414201207.3612432-2-jose.carlos.venegas.munoz@intel.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
---
tools/virtiofsd/passthrough_ll.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 6592f96f68..2c36f4ec46 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -3831,6 +3831,7 @@ int main(int argc, char *argv[])
}
if (lo.xattrmap) {
+ lo.xattr = 1;
parse_xattrmap(&lo);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 03/12] virtiofsd: Add help for -o xattr-mapping
2021-05-06 18:56 [PULL 00/12] virtiofs queue Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 01/12] virtiofsd: Fix side-effect in assert() Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 02/12] virtiofsd: Allow use "-o xattrmap" without "-o xattr" Dr. David Alan Gilbert (git)
@ 2021-05-06 18:56 ` Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 04/12] virtiofs: Fixup printf args Dr. David Alan Gilbert (git)
` (9 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2021-05-06 18:56 UTC (permalink / raw)
To: qemu-devel, groug, jose.carlos.venegas.munoz, ma.mandourr
Cc: virtio-fs, vgoyal, stefanha
From: Carlos Venegas <jose.carlos.venegas.munoz@intel.com>
The option is not documented in help.
Add small help about the option.
Signed-off-by: Carlos Venegas <jose.carlos.venegas.munoz@intel.com>
Message-Id: <20210414201207.3612432-3-jose.carlos.venegas.munoz@intel.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Connor Kuehl <ckuehl@redhat.com>
---
tools/virtiofsd/helper.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
index 28243b51b2..5e98ed702b 100644
--- a/tools/virtiofsd/helper.c
+++ b/tools/virtiofsd/helper.c
@@ -172,6 +172,9 @@ void fuse_cmdline_help(void)
" default: no_writeback\n"
" -o xattr|no_xattr enable/disable xattr\n"
" default: no_xattr\n"
+ " -o xattrmap=<mapping> Enable xattr mapping (enables xattr)\n"
+ " <mapping> is a string consists of a series of rules\n"
+ " e.g. -o xattrmap=:map::user.virtiofs.:\n"
" -o modcaps=CAPLIST Modify the list of capabilities\n"
" e.g. -o modcaps=+sys_admin:-chown\n"
" --rlimit-nofile=<num> set maximum number of file descriptors\n"
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 04/12] virtiofs: Fixup printf args
2021-05-06 18:56 [PULL 00/12] virtiofs queue Dr. David Alan Gilbert (git)
` (2 preceding siblings ...)
2021-05-06 18:56 ` [PULL 03/12] virtiofsd: Add help for -o xattr-mapping Dr. David Alan Gilbert (git)
@ 2021-05-06 18:56 ` Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 05/12] virtiofsd: Don't assume header layout Dr. David Alan Gilbert (git)
` (8 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2021-05-06 18:56 UTC (permalink / raw)
To: qemu-devel, groug, jose.carlos.venegas.munoz, ma.mandourr
Cc: virtio-fs, vgoyal, stefanha
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Fixup some fuse_log printf args for 32bit compatibility.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20210428110100.27757-2-dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tools/virtiofsd/passthrough_ll.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 2c36f4ec46..93a49db3cd 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2011,10 +2011,10 @@ static void lo_getlk(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi,
fuse_log(FUSE_LOG_DEBUG,
"lo_getlk(ino=%" PRIu64 ", flags=%d)"
- " owner=0x%lx, l_type=%d l_start=0x%lx"
- " l_len=0x%lx\n",
- ino, fi->flags, fi->lock_owner, lock->l_type, lock->l_start,
- lock->l_len);
+ " owner=0x%" PRIx64 ", l_type=%d l_start=0x%" PRIx64
+ " l_len=0x%" PRIx64 "\n",
+ ino, fi->flags, fi->lock_owner, lock->l_type,
+ (uint64_t)lock->l_start, (uint64_t)lock->l_len);
if (!lo->posix_lock) {
fuse_reply_err(req, ENOSYS);
@@ -2061,10 +2061,10 @@ static void lo_setlk(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi,
fuse_log(FUSE_LOG_DEBUG,
"lo_setlk(ino=%" PRIu64 ", flags=%d)"
- " cmd=%d pid=%d owner=0x%lx sleep=%d l_whence=%d"
- " l_start=0x%lx l_len=0x%lx\n",
+ " cmd=%d pid=%d owner=0x%" PRIx64 " sleep=%d l_whence=%d"
+ " l_start=0x%" PRIx64 " l_len=0x%" PRIx64 "\n",
ino, fi->flags, lock->l_type, lock->l_pid, fi->lock_owner, sleep,
- lock->l_whence, lock->l_start, lock->l_len);
+ lock->l_whence, (uint64_t)lock->l_start, (uint64_t)lock->l_len);
if (!lo->posix_lock) {
fuse_reply_err(req, ENOSYS);
@@ -3102,9 +3102,10 @@ static void lo_copy_file_range(fuse_req_t req, fuse_ino_t ino_in, off_t off_in,
fuse_log(FUSE_LOG_DEBUG,
"lo_copy_file_range(ino=%" PRIu64 "/fd=%d, "
- "off=%lu, ino=%" PRIu64 "/fd=%d, "
- "off=%lu, size=%zd, flags=0x%x)\n",
- ino_in, in_fd, off_in, ino_out, out_fd, off_out, len, flags);
+ "off=%ju, ino=%" PRIu64 "/fd=%d, "
+ "off=%ju, size=%zd, flags=0x%x)\n",
+ ino_in, in_fd, (intmax_t)off_in,
+ ino_out, out_fd, (intmax_t)off_out, len, flags);
res = copy_file_range(in_fd, &off_in, out_fd, &off_out, len, flags);
if (res < 0) {
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 05/12] virtiofsd: Don't assume header layout
2021-05-06 18:56 [PULL 00/12] virtiofs queue Dr. David Alan Gilbert (git)
` (3 preceding siblings ...)
2021-05-06 18:56 ` [PULL 04/12] virtiofs: Fixup printf args Dr. David Alan Gilbert (git)
@ 2021-05-06 18:56 ` Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 06/12] virtiofsd: Changed allocations of fuse_req to GLib functions Dr. David Alan Gilbert (git)
` (7 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2021-05-06 18:56 UTC (permalink / raw)
To: qemu-devel, groug, jose.carlos.venegas.munoz, ma.mandourr
Cc: virtio-fs, vgoyal, stefanha
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
virtiofsd incorrectly assumed a fixed set of header layout in the virt
queue; assuming that the fuse and write headers were conveniently
separated from the data; the spec doesn't allow us to take that
convenience, so fix it up to deal with it the hard way.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20210428110100.27757-3-dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
tools/virtiofsd/fuse_virtio.c | 94 +++++++++++++++++++++++++++--------
1 file changed, 73 insertions(+), 21 deletions(-)
diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
index 3e13997406..6dd73c9b72 100644
--- a/tools/virtiofsd/fuse_virtio.c
+++ b/tools/virtiofsd/fuse_virtio.c
@@ -129,18 +129,55 @@ static void fv_panic(VuDev *dev, const char *err)
* Copy from an iovec into a fuse_buf (memory only)
* Caller must ensure there is space
*/
-static void copy_from_iov(struct fuse_buf *buf, size_t out_num,
- const struct iovec *out_sg)
+static size_t copy_from_iov(struct fuse_buf *buf, size_t out_num,
+ const struct iovec *out_sg,
+ size_t max)
{
void *dest = buf->mem;
+ size_t copied = 0;
- while (out_num) {
+ while (out_num && max) {
size_t onelen = out_sg->iov_len;
+ onelen = MIN(onelen, max);
memcpy(dest, out_sg->iov_base, onelen);
dest += onelen;
+ copied += onelen;
out_sg++;
out_num--;
+ max -= onelen;
}
+
+ return copied;
+}
+
+/*
+ * Skip 'skip' bytes in the iov; 'sg_1stindex' is set as
+ * the index for the 1st iovec to read data from, and
+ * 'sg_1stskip' is the number of bytes to skip in that entry.
+ *
+ * Returns True if there are at least 'skip' bytes in the iovec
+ *
+ */
+static bool skip_iov(const struct iovec *sg, size_t sg_size,
+ size_t skip,
+ size_t *sg_1stindex, size_t *sg_1stskip)
+{
+ size_t vec;
+
+ for (vec = 0; vec < sg_size; vec++) {
+ if (sg[vec].iov_len > skip) {
+ *sg_1stskip = skip;
+ *sg_1stindex = vec;
+
+ return true;
+ }
+
+ skip -= sg[vec].iov_len;
+ }
+
+ *sg_1stindex = vec;
+ *sg_1stskip = 0;
+ return skip == 0;
}
/*
@@ -457,6 +494,7 @@ static void fv_queue_worker(gpointer data, gpointer user_data)
bool allocated_bufv = false;
struct fuse_bufvec bufv;
struct fuse_bufvec *pbufv;
+ struct fuse_in_header inh;
assert(se->bufsize > sizeof(struct fuse_in_header));
@@ -505,14 +543,15 @@ static void fv_queue_worker(gpointer data, gpointer user_data)
elem->index);
assert(0); /* TODO */
}
- /* Copy just the first element and look at it */
- copy_from_iov(&fbuf, 1, out_sg);
+ /* Copy just the fuse_in_header and look at it */
+ copy_from_iov(&fbuf, out_num, out_sg,
+ sizeof(struct fuse_in_header));
+ memcpy(&inh, fbuf.mem, sizeof(struct fuse_in_header));
pbufv = NULL; /* Compiler thinks an unitialised path */
- if (out_num > 2 &&
- out_sg[0].iov_len == sizeof(struct fuse_in_header) &&
- ((struct fuse_in_header *)fbuf.mem)->opcode == FUSE_WRITE &&
- out_sg[1].iov_len == sizeof(struct fuse_write_in)) {
+ if (inh.opcode == FUSE_WRITE &&
+ out_len >= (sizeof(struct fuse_in_header) +
+ sizeof(struct fuse_write_in))) {
/*
* For a write we don't actually need to copy the
* data, we can just do it straight out of guest memory
@@ -521,15 +560,15 @@ static void fv_queue_worker(gpointer data, gpointer user_data)
*/
fuse_log(FUSE_LOG_DEBUG, "%s: Write special case\n", __func__);
- /* copy the fuse_write_in header afte rthe fuse_in_header */
- fbuf.mem += out_sg->iov_len;
- copy_from_iov(&fbuf, 1, out_sg + 1);
- fbuf.mem -= out_sg->iov_len;
- fbuf.size = out_sg[0].iov_len + out_sg[1].iov_len;
+ fbuf.size = copy_from_iov(&fbuf, out_num, out_sg,
+ sizeof(struct fuse_in_header) +
+ sizeof(struct fuse_write_in));
+ /* That copy reread the in_header, make sure we use the original */
+ memcpy(fbuf.mem, &inh, sizeof(struct fuse_in_header));
/* Allocate the bufv, with space for the rest of the iov */
pbufv = malloc(sizeof(struct fuse_bufvec) +
- sizeof(struct fuse_buf) * (out_num - 2));
+ sizeof(struct fuse_buf) * out_num);
if (!pbufv) {
fuse_log(FUSE_LOG_ERR, "%s: pbufv malloc failed\n",
__func__);
@@ -540,24 +579,37 @@ static void fv_queue_worker(gpointer data, gpointer user_data)
pbufv->count = 1;
pbufv->buf[0] = fbuf;
- size_t iovindex, pbufvindex;
- iovindex = 2; /* 2 headers, separate iovs */
+ size_t iovindex, pbufvindex, iov_bytes_skip;
pbufvindex = 1; /* 2 headers, 1 fusebuf */
+ if (!skip_iov(out_sg, out_num,
+ sizeof(struct fuse_in_header) +
+ sizeof(struct fuse_write_in),
+ &iovindex, &iov_bytes_skip)) {
+ fuse_log(FUSE_LOG_ERR, "%s: skip failed\n",
+ __func__);
+ goto out;
+ }
+
for (; iovindex < out_num; iovindex++, pbufvindex++) {
pbufv->count++;
pbufv->buf[pbufvindex].pos = ~0; /* Dummy */
pbufv->buf[pbufvindex].flags = 0;
pbufv->buf[pbufvindex].mem = out_sg[iovindex].iov_base;
pbufv->buf[pbufvindex].size = out_sg[iovindex].iov_len;
+
+ if (iov_bytes_skip) {
+ pbufv->buf[pbufvindex].mem += iov_bytes_skip;
+ pbufv->buf[pbufvindex].size -= iov_bytes_skip;
+ iov_bytes_skip = 0;
+ }
}
} else {
/* Normal (non fast write) path */
- /* Copy the rest of the buffer */
- fbuf.mem += out_sg->iov_len;
- copy_from_iov(&fbuf, out_num - 1, out_sg + 1);
- fbuf.mem -= out_sg->iov_len;
+ copy_from_iov(&fbuf, out_num, out_sg, se->bufsize);
+ /* That copy reread the in_header, make sure we use the original */
+ memcpy(fbuf.mem, &inh, sizeof(struct fuse_in_header));
fbuf.size = out_len;
/* TODO! Endianness of header */
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 06/12] virtiofsd: Changed allocations of fuse_req to GLib functions
2021-05-06 18:56 [PULL 00/12] virtiofs queue Dr. David Alan Gilbert (git)
` (4 preceding siblings ...)
2021-05-06 18:56 ` [PULL 05/12] virtiofsd: Don't assume header layout Dr. David Alan Gilbert (git)
@ 2021-05-06 18:56 ` Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 07/12] virtiofsd: Changed allocations of iovec to GLib's functions Dr. David Alan Gilbert (git)
` (6 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2021-05-06 18:56 UTC (permalink / raw)
To: qemu-devel, groug, jose.carlos.venegas.munoz, ma.mandourr
Cc: virtio-fs, vgoyal, stefanha
From: Mahmoud Mandour <ma.mandourr@gmail.com>
Replaced the allocation and deallocation of fuse_req structs
using calloc()/free() call pairs to a GLib's g_try_new0()
and g_free().
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210420154643.58439-2-ma.mandourr@gmail.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
tools/virtiofsd/fuse_lowlevel.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index 58e32fc963..812cef6ef6 100644
--- a/tools/virtiofsd/fuse_lowlevel.c
+++ b/tools/virtiofsd/fuse_lowlevel.c
@@ -106,7 +106,7 @@ static void list_add_req(struct fuse_req *req, struct fuse_req *next)
static void destroy_req(fuse_req_t req)
{
pthread_mutex_destroy(&req->lock);
- free(req);
+ g_free(req);
}
void fuse_free_req(fuse_req_t req)
@@ -130,7 +130,7 @@ static struct fuse_req *fuse_ll_alloc_req(struct fuse_session *se)
{
struct fuse_req *req;
- req = (struct fuse_req *)calloc(1, sizeof(struct fuse_req));
+ req = g_try_new0(struct fuse_req, 1);
if (req == NULL) {
fuse_log(FUSE_LOG_ERR, "fuse: failed to allocate request\n");
} else {
@@ -1684,7 +1684,7 @@ static struct fuse_req *check_interrupt(struct fuse_session *se,
if (curr->u.i.unique == req->unique) {
req->interrupted = 1;
list_del_req(curr);
- free(curr);
+ g_free(curr);
return NULL;
}
}
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 07/12] virtiofsd: Changed allocations of iovec to GLib's functions
2021-05-06 18:56 [PULL 00/12] virtiofs queue Dr. David Alan Gilbert (git)
` (5 preceding siblings ...)
2021-05-06 18:56 ` [PULL 06/12] virtiofsd: Changed allocations of fuse_req to GLib functions Dr. David Alan Gilbert (git)
@ 2021-05-06 18:56 ` Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 08/12] virtiofsd: Changed allocations of fuse_session " Dr. David Alan Gilbert (git)
` (5 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2021-05-06 18:56 UTC (permalink / raw)
To: qemu-devel, groug, jose.carlos.venegas.munoz, ma.mandourr
Cc: virtio-fs, vgoyal, stefanha
From: Mahmoud Mandour <ma.mandourr@gmail.com>
Replaced the calls to malloc()/calloc() and their respective
calls to free() of iovec structs with GLib's allocation and
deallocation functions and used g_autofree when appropriate.
Replaced the allocation of in_sg_cpy to g_new() instead of a call
to calloc() and a null-checking assertion. Not g_new0()
because the buffer is immediately overwritten using memcpy.
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
Message-Id: <20210427181333.148176-1-ma.mandourr@gmail.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
tools/virtiofsd/fuse_lowlevel.c | 31 ++++++++++++-------------------
tools/virtiofsd/fuse_virtio.c | 7 ++-----
2 files changed, 14 insertions(+), 24 deletions(-)
diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index 812cef6ef6..88496f9560 100644
--- a/tools/virtiofsd/fuse_lowlevel.c
+++ b/tools/virtiofsd/fuse_lowlevel.c
@@ -217,9 +217,9 @@ static int send_reply(fuse_req_t req, int error, const void *arg,
int fuse_reply_iov(fuse_req_t req, const struct iovec *iov, int count)
{
int res;
- struct iovec *padded_iov;
+ g_autofree struct iovec *padded_iov = NULL;
- padded_iov = malloc((count + 1) * sizeof(struct iovec));
+ padded_iov = g_try_new(struct iovec, count + 1);
if (padded_iov == NULL) {
return fuse_reply_err(req, ENOMEM);
}
@@ -228,7 +228,6 @@ int fuse_reply_iov(fuse_req_t req, const struct iovec *iov, int count)
count++;
res = send_reply_iov(req, 0, padded_iov, count);
- free(padded_iov);
return res;
}
@@ -568,7 +567,7 @@ static struct fuse_ioctl_iovec *fuse_ioctl_iovec_copy(const struct iovec *iov,
struct fuse_ioctl_iovec *fiov;
size_t i;
- fiov = malloc(sizeof(fiov[0]) * count);
+ fiov = g_try_new(struct fuse_ioctl_iovec, count);
if (!fiov) {
return NULL;
}
@@ -586,8 +585,8 @@ int fuse_reply_ioctl_retry(fuse_req_t req, const struct iovec *in_iov,
size_t out_count)
{
struct fuse_ioctl_out arg;
- struct fuse_ioctl_iovec *in_fiov = NULL;
- struct fuse_ioctl_iovec *out_fiov = NULL;
+ g_autofree struct fuse_ioctl_iovec *in_fiov = NULL;
+ g_autofree struct fuse_ioctl_iovec *out_fiov = NULL;
struct iovec iov[4];
size_t count = 1;
int res;
@@ -603,13 +602,14 @@ int fuse_reply_ioctl_retry(fuse_req_t req, const struct iovec *in_iov,
/* Can't handle non-compat 64bit ioctls on 32bit */
if (sizeof(void *) == 4 && req->ioctl_64bit) {
res = fuse_reply_err(req, EINVAL);
- goto out;
+ return res;
}
if (in_count) {
in_fiov = fuse_ioctl_iovec_copy(in_iov, in_count);
if (!in_fiov) {
- goto enomem;
+ res = fuse_reply_err(req, ENOMEM);
+ return res;
}
iov[count].iov_base = (void *)in_fiov;
@@ -619,7 +619,8 @@ int fuse_reply_ioctl_retry(fuse_req_t req, const struct iovec *in_iov,
if (out_count) {
out_fiov = fuse_ioctl_iovec_copy(out_iov, out_count);
if (!out_fiov) {
- goto enomem;
+ res = fuse_reply_err(req, ENOMEM);
+ return res;
}
iov[count].iov_base = (void *)out_fiov;
@@ -628,15 +629,8 @@ int fuse_reply_ioctl_retry(fuse_req_t req, const struct iovec *in_iov,
}
res = send_reply_iov(req, 0, iov, count);
-out:
- free(in_fiov);
- free(out_fiov);
return res;
-
-enomem:
- res = fuse_reply_err(req, ENOMEM);
- goto out;
}
int fuse_reply_ioctl(fuse_req_t req, int result, const void *buf, size_t size)
@@ -663,11 +657,11 @@ int fuse_reply_ioctl(fuse_req_t req, int result, const void *buf, size_t size)
int fuse_reply_ioctl_iov(fuse_req_t req, int result, const struct iovec *iov,
int count)
{
- struct iovec *padded_iov;
+ g_autofree struct iovec *padded_iov = NULL;
struct fuse_ioctl_out arg;
int res;
- padded_iov = malloc((count + 2) * sizeof(struct iovec));
+ padded_iov = g_try_new(struct iovec, count + 2);
if (padded_iov == NULL) {
return fuse_reply_err(req, ENOMEM);
}
@@ -680,7 +674,6 @@ int fuse_reply_ioctl_iov(fuse_req_t req, int result, const struct iovec *iov,
memcpy(&padded_iov[2], iov, count * sizeof(struct iovec));
res = send_reply_iov(req, 0, padded_iov, count + 2);
- free(padded_iov);
return res;
}
diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
index 6dd73c9b72..a3d37ab696 100644
--- a/tools/virtiofsd/fuse_virtio.c
+++ b/tools/virtiofsd/fuse_virtio.c
@@ -331,6 +331,7 @@ int virtio_send_data_iov(struct fuse_session *se, struct fuse_chan *ch,
VuVirtq *q = vu_get_queue(dev, qi->qidx);
VuVirtqElement *elem = &req->elem;
int ret = 0;
+ g_autofree struct iovec *in_sg_cpy = NULL;
assert(count >= 1);
assert(iov[0].iov_len >= sizeof(struct fuse_out_header));
@@ -384,8 +385,7 @@ int virtio_send_data_iov(struct fuse_session *se, struct fuse_chan *ch,
* Build a copy of the the in_sg iov so we can skip bits in it,
* including changing the offsets
*/
- struct iovec *in_sg_cpy = calloc(sizeof(struct iovec), in_num);
- assert(in_sg_cpy);
+ in_sg_cpy = g_new(struct iovec, in_num);
memcpy(in_sg_cpy, in_sg, sizeof(struct iovec) * in_num);
/* These get updated as we skip */
struct iovec *in_sg_ptr = in_sg_cpy;
@@ -423,7 +423,6 @@ int virtio_send_data_iov(struct fuse_session *se, struct fuse_chan *ch,
ret = errno;
fuse_log(FUSE_LOG_DEBUG, "%s: preadv failed (%m) len=%zd\n",
__func__, len);
- free(in_sg_cpy);
goto err;
}
fuse_log(FUSE_LOG_DEBUG, "%s: preadv ret=%d len=%zd\n", __func__,
@@ -447,13 +446,11 @@ int virtio_send_data_iov(struct fuse_session *se, struct fuse_chan *ch,
if (ret != len) {
fuse_log(FUSE_LOG_DEBUG, "%s: ret!=len\n", __func__);
ret = EIO;
- free(in_sg_cpy);
goto err;
}
in_sg_left -= ret;
len -= ret;
} while (in_sg_left);
- free(in_sg_cpy);
/* Need to fix out->len on EOF */
if (len) {
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 08/12] virtiofsd: Changed allocations of fuse_session to GLib's functions
2021-05-06 18:56 [PULL 00/12] virtiofs queue Dr. David Alan Gilbert (git)
` (6 preceding siblings ...)
2021-05-06 18:56 ` [PULL 07/12] virtiofsd: Changed allocations of iovec to GLib's functions Dr. David Alan Gilbert (git)
@ 2021-05-06 18:56 ` Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 09/12] virtiofsd: Changed allocation of lo_map_elems " Dr. David Alan Gilbert (git)
` (4 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2021-05-06 18:56 UTC (permalink / raw)
To: qemu-devel, groug, jose.carlos.venegas.munoz, ma.mandourr
Cc: virtio-fs, vgoyal, stefanha
From: Mahmoud Mandour <ma.mandourr@gmail.com>
Replaced the allocation and deallocation of fuse_session structs
from calloc() and free() calls to g_try_new0() and g_free().
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210420154643.58439-4-ma.mandourr@gmail.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
tools/virtiofsd/fuse_lowlevel.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index 88496f9560..7fe2cef1eb 100644
--- a/tools/virtiofsd/fuse_lowlevel.c
+++ b/tools/virtiofsd/fuse_lowlevel.c
@@ -2470,7 +2470,7 @@ void fuse_session_destroy(struct fuse_session *se)
free(se->vu_socket_path);
se->vu_socket_path = NULL;
- free(se);
+ g_free(se);
}
@@ -2493,7 +2493,7 @@ struct fuse_session *fuse_session_new(struct fuse_args *args,
return NULL;
}
- se = (struct fuse_session *)calloc(1, sizeof(struct fuse_session));
+ se = g_try_new0(struct fuse_session, 1);
if (se == NULL) {
fuse_log(FUSE_LOG_ERR, "fuse: failed to allocate fuse object\n");
goto out1;
@@ -2553,7 +2553,7 @@ struct fuse_session *fuse_session_new(struct fuse_args *args,
out4:
fuse_opt_free_args(args);
out2:
- free(se);
+ g_free(se);
out1:
return NULL;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 09/12] virtiofsd: Changed allocation of lo_map_elems to GLib's functions
2021-05-06 18:56 [PULL 00/12] virtiofs queue Dr. David Alan Gilbert (git)
` (7 preceding siblings ...)
2021-05-06 18:56 ` [PULL 08/12] virtiofsd: Changed allocations of fuse_session " Dr. David Alan Gilbert (git)
@ 2021-05-06 18:56 ` Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 10/12] virtiofsd: Changed allocations of fv_VuDev & its internals to GLib functions Dr. David Alan Gilbert (git)
` (3 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2021-05-06 18:56 UTC (permalink / raw)
To: qemu-devel, groug, jose.carlos.venegas.munoz, ma.mandourr
Cc: virtio-fs, vgoyal, stefanha
From: Mahmoud Mandour <ma.mandourr@gmail.com>
Replaced (re)allocation of lo_map_elem structs from realloc() to
GLib's g_try_realloc_n() and replaced the respective free() call
with a g_free().
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210420154643.58439-5-ma.mandourr@gmail.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
tools/virtiofsd/passthrough_ll.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 93a49db3cd..406b5bd10e 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -406,7 +406,7 @@ static void lo_map_init(struct lo_map *map)
static void lo_map_destroy(struct lo_map *map)
{
- free(map->elems);
+ g_free(map->elems);
}
static int lo_map_grow(struct lo_map *map, size_t new_nelems)
@@ -418,7 +418,7 @@ static int lo_map_grow(struct lo_map *map, size_t new_nelems)
return 1;
}
- new_elems = realloc(map->elems, sizeof(map->elems[0]) * new_nelems);
+ new_elems = g_try_realloc_n(map->elems, new_nelems, sizeof(map->elems[0]));
if (!new_elems) {
return 0;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 10/12] virtiofsd: Changed allocations of fv_VuDev & its internals to GLib functions
2021-05-06 18:56 [PULL 00/12] virtiofs queue Dr. David Alan Gilbert (git)
` (8 preceding siblings ...)
2021-05-06 18:56 ` [PULL 09/12] virtiofsd: Changed allocation of lo_map_elems " Dr. David Alan Gilbert (git)
@ 2021-05-06 18:56 ` Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 11/12] virtiofsd/passthrough_ll.c: Changed local allocations " Dr. David Alan Gilbert (git)
` (2 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2021-05-06 18:56 UTC (permalink / raw)
To: qemu-devel, groug, jose.carlos.venegas.munoz, ma.mandourr
Cc: virtio-fs, vgoyal, stefanha
From: Mahmoud Mandour <ma.mandourr@gmail.com>
Changed the allocations of fv_VuDev structs, VuDev structs, and
fv_QueueInfo strcuts from using calloc()/realloc() & free() to using
the equivalent functions from GLib.
In instances, removed the pair of allocation and assertion for
non-NULL checking with a GLib function that aborts on error.
Removed NULL-checking for fv_VuDev struct allocation and used
a GLib function that crashes on error; namely, g_new0(). This
is because allocating one struct should not be a problem on an
healthy system. Also following the pattern of aborting-on-null
behaviour that is taken with allocating VuDev structs and
fv_QueueInfo structs.
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210420154643.58439-6-ma.mandourr@gmail.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
tools/virtiofsd/fuse_virtio.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
index a3d37ab696..828f0fa590 100644
--- a/tools/virtiofsd/fuse_virtio.c
+++ b/tools/virtiofsd/fuse_virtio.c
@@ -782,7 +782,7 @@ static void fv_queue_cleanup_thread(struct fv_VuDev *vud, int qidx)
pthread_mutex_destroy(&ourqi->vq_lock);
close(ourqi->kill_fd);
ourqi->kick_fd = -1;
- free(vud->qi[qidx]);
+ g_free(vud->qi[qidx]);
vud->qi[qidx] = NULL;
}
@@ -813,15 +813,13 @@ static void fv_queue_set_started(VuDev *dev, int qidx, bool started)
if (started) {
/* Fire up a thread to watch this queue */
if (qidx >= vud->nqueues) {
- vud->qi = realloc(vud->qi, (qidx + 1) * sizeof(vud->qi[0]));
- assert(vud->qi);
+ vud->qi = g_realloc_n(vud->qi, qidx + 1, sizeof(vud->qi[0]));
memset(vud->qi + vud->nqueues, 0,
sizeof(vud->qi[0]) * (1 + (qidx - vud->nqueues)));
vud->nqueues = qidx + 1;
}
if (!vud->qi[qidx]) {
- vud->qi[qidx] = calloc(sizeof(struct fv_QueueInfo), 1);
- assert(vud->qi[qidx]);
+ vud->qi[qidx] = g_new0(struct fv_QueueInfo, 1);
vud->qi[qidx]->virtio_dev = vud;
vud->qi[qidx]->qidx = qidx;
} else {
@@ -1087,12 +1085,7 @@ int virtio_session_mount(struct fuse_session *se)
__func__);
/* TODO: Some cleanup/deallocation! */
- se->virtio_dev = calloc(sizeof(struct fv_VuDev), 1);
- if (!se->virtio_dev) {
- fuse_log(FUSE_LOG_ERR, "%s: virtio_dev calloc failed\n", __func__);
- close(data_sock);
- return -1;
- }
+ se->virtio_dev = g_new0(struct fv_VuDev, 1);
se->vu_socketfd = data_sock;
se->virtio_dev->se = se;
@@ -1114,8 +1107,8 @@ void virtio_session_close(struct fuse_session *se)
return;
}
- free(se->virtio_dev->qi);
+ g_free(se->virtio_dev->qi);
pthread_rwlock_destroy(&se->virtio_dev->vu_dispatch_rwlock);
- free(se->virtio_dev);
+ g_free(se->virtio_dev);
se->virtio_dev = NULL;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 11/12] virtiofsd/passthrough_ll.c: Changed local allocations to GLib functions
2021-05-06 18:56 [PULL 00/12] virtiofs queue Dr. David Alan Gilbert (git)
` (9 preceding siblings ...)
2021-05-06 18:56 ` [PULL 10/12] virtiofsd: Changed allocations of fv_VuDev & its internals to GLib functions Dr. David Alan Gilbert (git)
@ 2021-05-06 18:56 ` Dr. David Alan Gilbert (git)
2021-05-06 18:56 ` [PULL 12/12] virtiofsd/fuse_virtio.c: Changed allocations of locals to GLib Dr. David Alan Gilbert (git)
2021-05-11 15:05 ` [PULL 00/12] virtiofs queue Peter Maydell
12 siblings, 0 replies; 15+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2021-05-06 18:56 UTC (permalink / raw)
To: qemu-devel, groug, jose.carlos.venegas.munoz, ma.mandourr
Cc: virtio-fs, vgoyal, stefanha
From: Mahmoud Mandour <ma.mandourr@gmail.com>
Changed the allocations of some local variables to GLib's allocation
functions, such as g_try_malloc0(), and annotated those variables
as g_autofree. Subsequently, I was able to remove the calls to free().
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210420154643.58439-7-ma.mandourr@gmail.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
tools/virtiofsd/passthrough_ll.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 406b5bd10e..49c21fd855 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -1653,7 +1653,7 @@ static void lo_do_readdir(fuse_req_t req, fuse_ino_t ino, size_t size,
struct lo_data *lo = lo_data(req);
struct lo_dirp *d = NULL;
struct lo_inode *dinode;
- char *buf = NULL;
+ g_autofree char *buf = NULL;
char *p;
size_t rem = size;
int err = EBADF;
@@ -1669,7 +1669,7 @@ static void lo_do_readdir(fuse_req_t req, fuse_ino_t ino, size_t size,
}
err = ENOMEM;
- buf = calloc(1, size);
+ buf = g_try_malloc0(size);
if (!buf) {
goto error;
}
@@ -1755,7 +1755,6 @@ error:
} else {
fuse_reply_buf(req, buf, size - rem);
}
- free(buf);
}
static void lo_readdir(fuse_req_t req, fuse_ino_t ino, size_t size,
@@ -2732,7 +2731,7 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name,
size_t size)
{
struct lo_data *lo = lo_data(req);
- char *value = NULL;
+ g_autofree char *value = NULL;
char procname[64];
const char *name;
char *mapped_name;
@@ -2773,7 +2772,7 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name,
ino, name, size);
if (size) {
- value = malloc(size);
+ value = g_try_malloc(size);
if (!value) {
goto out_err;
}
@@ -2812,8 +2811,6 @@ static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name,
fuse_reply_xattr(req, ret);
}
out_free:
- free(value);
-
if (fd >= 0) {
close(fd);
}
@@ -2832,7 +2829,7 @@ out:
static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size)
{
struct lo_data *lo = lo_data(req);
- char *value = NULL;
+ g_autofree char *value = NULL;
char procname[64];
struct lo_inode *inode;
ssize_t ret;
@@ -2854,7 +2851,7 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size)
size);
if (size) {
- value = malloc(size);
+ value = g_try_malloc(size);
if (!value) {
goto out_err;
}
@@ -2939,8 +2936,6 @@ static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size)
fuse_reply_xattr(req, ret);
}
out_free:
- free(value);
-
if (fd >= 0) {
close(fd);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 12/12] virtiofsd/fuse_virtio.c: Changed allocations of locals to GLib
2021-05-06 18:56 [PULL 00/12] virtiofs queue Dr. David Alan Gilbert (git)
` (10 preceding siblings ...)
2021-05-06 18:56 ` [PULL 11/12] virtiofsd/passthrough_ll.c: Changed local allocations " Dr. David Alan Gilbert (git)
@ 2021-05-06 18:56 ` Dr. David Alan Gilbert (git)
2021-05-11 15:05 ` [PULL 00/12] virtiofs queue Peter Maydell
12 siblings, 0 replies; 15+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2021-05-06 18:56 UTC (permalink / raw)
To: qemu-devel, groug, jose.carlos.venegas.munoz, ma.mandourr
Cc: virtio-fs, vgoyal, stefanha
From: Mahmoud Mandour <ma.mandourr@gmail.com>
Replaced the allocation of local variables from malloc() to
GLib allocation functions.
In one instance, dropped the usage to an assert after a malloc()
call and used g_malloc() instead.
Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210420154643.58439-8-ma.mandourr@gmail.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
tools/virtiofsd/fuse_virtio.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
index 828f0fa590..1170f375a5 100644
--- a/tools/virtiofsd/fuse_virtio.c
+++ b/tools/virtiofsd/fuse_virtio.c
@@ -511,8 +511,7 @@ static void fv_queue_worker(gpointer data, gpointer user_data)
* They're spread over multiple descriptors in a scatter/gather set
* and we can't trust the guest to keep them still; so copy in/out.
*/
- fbuf.mem = malloc(se->bufsize);
- assert(fbuf.mem);
+ fbuf.mem = g_malloc(se->bufsize);
fuse_mutex_init(&req->ch.lock);
req->ch.fd = -1;
@@ -564,8 +563,8 @@ static void fv_queue_worker(gpointer data, gpointer user_data)
memcpy(fbuf.mem, &inh, sizeof(struct fuse_in_header));
/* Allocate the bufv, with space for the rest of the iov */
- pbufv = malloc(sizeof(struct fuse_bufvec) +
- sizeof(struct fuse_buf) * out_num);
+ pbufv = g_try_malloc(sizeof(struct fuse_bufvec) +
+ sizeof(struct fuse_buf) * out_num);
if (!pbufv) {
fuse_log(FUSE_LOG_ERR, "%s: pbufv malloc failed\n",
__func__);
@@ -622,7 +621,7 @@ static void fv_queue_worker(gpointer data, gpointer user_data)
out:
if (allocated_bufv) {
- free(pbufv);
+ g_free(pbufv);
}
/* If the request has no reply, still recycle the virtqueue element */
@@ -641,7 +640,7 @@ out:
}
pthread_mutex_destroy(&req->ch.lock);
- free(fbuf.mem);
+ g_free(fbuf.mem);
free(req);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PULL 00/12] virtiofs queue
2021-05-06 18:56 [PULL 00/12] virtiofs queue Dr. David Alan Gilbert (git)
` (11 preceding siblings ...)
2021-05-06 18:56 ` [PULL 12/12] virtiofsd/fuse_virtio.c: Changed allocations of locals to GLib Dr. David Alan Gilbert (git)
@ 2021-05-11 15:05 ` Peter Maydell
12 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2021-05-11 15:05 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: jose.carlos.venegas.munoz, QEMU Developers, Greg Kurz, virtio-fs,
Stefan Hajnoczi, Mahmoud Mandour, vgoyal
On Thu, 6 May 2021 at 20:05, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
>
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The following changes since commit d90f154867ec0ec22fd719164b88716e8fd48672:
>
> Merge remote-tracking branch 'remotes/dg-gitlab/tags/ppc-for-6.1-20210504' into staging (2021-05-05 20:29:14 +0100)
>
> are available in the Git repository at:
>
> https://gitlab.com/dagrh/qemu.git tags/pull-virtiofs-20210506
>
> for you to fetch changes up to 67a010f64cc9e33ba19ab389dedaa52013a9de8a:
>
> virtiofsd/fuse_virtio.c: Changed allocations of locals to GLib (2021-05-06 19:47:44 +0100)
>
> ----------------------------------------------------------------
> virtiofsd pull 2021-05-06
>
> A pile of cleanups:
>
> Use of glib allocators from Mahmoud
> Virtio spec compliance and printf cleanup from me.
> Sugar to turn on xattr when defining xattr mapping from Carlos
> an assert cleanup from Greg
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/6.1
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread