* [PATCH v2 1/3] block/export/fuse: Extract fuse_fallocate_punch_hole()
2022-01-24 22:03 [PATCH v2 0/3] block/export/fuse: Fix build failure on FreeBSD Philippe Mathieu-Daudé via
@ 2022-01-24 22:03 ` Philippe Mathieu-Daudé via
2022-01-25 11:10 ` Thomas Huth
2022-01-24 22:03 ` [PATCH v2 2/3] block/export/fuse: Extract fuse_fallocate_zero_range() Philippe Mathieu-Daudé via
2022-01-24 22:03 ` [PATCH v2 3/3] block/export/fuse: Fix build failure on FreeBSD Philippe Mathieu-Daudé via
2 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-01-24 22:03 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Li-Wen Hsu, Ed Maste, Kyle Evans, Fabrice Fontaine,
Philippe Mathieu-Daudé
Extract fuse_fallocate_punch_hole() to avoid #ifdef'ry
mixed within if/else statement.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
block/export/fuse.c | 59 +++++++++++++++++++++++++++------------------
1 file changed, 35 insertions(+), 24 deletions(-)
diff --git a/block/export/fuse.c b/block/export/fuse.c
index 6710d8aed86..31cb0503adc 100644
--- a/block/export/fuse.c
+++ b/block/export/fuse.c
@@ -603,6 +603,38 @@ static void fuse_write(fuse_req_t req, fuse_ino_t inode, const char *buf,
}
}
+static bool fuse_fallocate_zero_range(fuse_req_t req, fuse_ino_t inode,
+ int mode, int64_t blk_len,
+ off_t offset, off_t *length)
+{
+#ifdef CONFIG_FALLOCATE_ZERO_RANGE
+ FuseExport *exp = fuse_req_userdata(req);
+
+ if (mode & FALLOC_FL_ZERO_RANGE) {
+ int ret;
+
+ if (!(mode & FALLOC_FL_KEEP_SIZE) && offset + *length > blk_len) {
+ /* No need for zeroes, we are going to write them ourselves */
+ ret = fuse_do_truncate(exp, offset + *length, false,
+ PREALLOC_MODE_OFF);
+ if (ret < 0) {
+ fuse_reply_err(req, -ret);
+ return false;
+ }
+ }
+
+ do {
+ int size = MIN(*length, BDRV_REQUEST_MAX_BYTES);
+
+ ret = blk_pwrite_zeroes(exp->common.blk, offset, size, 0);
+ offset += size;
+ *length -= size;
+ } while (ret == 0 && *length > 0);
+ }
+#endif /* CONFIG_FALLOCATE_ZERO_RANGE */
+ return true;
+}
+
/**
* Let clients perform various fallocate() operations.
*/
@@ -642,30 +674,9 @@ static void fuse_fallocate(fuse_req_t req, fuse_ino_t inode, int mode,
offset += size;
length -= size;
} while (ret == 0 && length > 0);
- }
-#ifdef CONFIG_FALLOCATE_ZERO_RANGE
- else if (mode & FALLOC_FL_ZERO_RANGE) {
- if (!(mode & FALLOC_FL_KEEP_SIZE) && offset + length > blk_len) {
- /* No need for zeroes, we are going to write them ourselves */
- ret = fuse_do_truncate(exp, offset + length, false,
- PREALLOC_MODE_OFF);
- if (ret < 0) {
- fuse_reply_err(req, -ret);
- return;
- }
- }
-
- do {
- int size = MIN(length, BDRV_REQUEST_MAX_BYTES);
-
- ret = blk_pwrite_zeroes(exp->common.blk,
- offset, size, 0);
- offset += size;
- length -= size;
- } while (ret == 0 && length > 0);
- }
-#endif /* CONFIG_FALLOCATE_ZERO_RANGE */
- else if (!mode) {
+ } else if (!fuse_fallocate_zero_range(req, inode, blk_len, mode, offset, &length)) {
+ return;
+ } else if (!mode) {
/* We can only fallocate at the EOF with a truncate */
if (offset < blk_len) {
fuse_reply_err(req, EOPNOTSUPP);
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 1/3] block/export/fuse: Extract fuse_fallocate_punch_hole()
2022-01-24 22:03 ` [PATCH v2 1/3] block/export/fuse: Extract fuse_fallocate_punch_hole() Philippe Mathieu-Daudé via
@ 2022-01-25 11:10 ` Thomas Huth
2022-01-25 16:02 ` Philippe Mathieu-Daudé via
0 siblings, 1 reply; 7+ messages in thread
From: Thomas Huth @ 2022-01-25 11:10 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Kyle Evans, Ed Maste, Li-Wen Hsu, qemu-block, Fabrice Fontaine
On 24/01/2022 23.03, Philippe Mathieu-Daudé via wrote:
> Extract fuse_fallocate_punch_hole() to avoid #ifdef'ry
> mixed within if/else statement.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> block/export/fuse.c | 59 +++++++++++++++++++++++++++------------------
> 1 file changed, 35 insertions(+), 24 deletions(-)
>
> diff --git a/block/export/fuse.c b/block/export/fuse.c
> index 6710d8aed86..31cb0503adc 100644
> --- a/block/export/fuse.c
> +++ b/block/export/fuse.c
> @@ -603,6 +603,38 @@ static void fuse_write(fuse_req_t req, fuse_ino_t inode, const char *buf,
> }
> }
>
> +static bool fuse_fallocate_zero_range(fuse_req_t req, fuse_ino_t inode,
> + int mode, int64_t blk_len,
> + off_t offset, off_t *length)
> +{
> +#ifdef CONFIG_FALLOCATE_ZERO_RANGE
> + FuseExport *exp = fuse_req_userdata(req);
> +
> + if (mode & FALLOC_FL_ZERO_RANGE) {
> + int ret;
> +
> + if (!(mode & FALLOC_FL_KEEP_SIZE) && offset + *length > blk_len) {
> + /* No need for zeroes, we are going to write them ourselves */
> + ret = fuse_do_truncate(exp, offset + *length, false,
> + PREALLOC_MODE_OFF);
> + if (ret < 0) {
> + fuse_reply_err(req, -ret);
> + return false;
> + }
> + }
> +
> + do {
> + int size = MIN(*length, BDRV_REQUEST_MAX_BYTES);
> +
> + ret = blk_pwrite_zeroes(exp->common.blk, offset, size, 0);
> + offset += size;
> + *length -= size;
> + } while (ret == 0 && *length > 0);
> + }
> +#endif /* CONFIG_FALLOCATE_ZERO_RANGE */
> + return true;
> +}
> +
> /**
> * Let clients perform various fallocate() operations.
> */
> @@ -642,30 +674,9 @@ static void fuse_fallocate(fuse_req_t req, fuse_ino_t inode, int mode,
> offset += size;
> length -= size;
> } while (ret == 0 && length > 0);
> - }
> -#ifdef CONFIG_FALLOCATE_ZERO_RANGE
> - else if (mode & FALLOC_FL_ZERO_RANGE) {
> - if (!(mode & FALLOC_FL_KEEP_SIZE) && offset + length > blk_len) {
> - /* No need for zeroes, we are going to write them ourselves */
> - ret = fuse_do_truncate(exp, offset + length, false,
> - PREALLOC_MODE_OFF);
> - if (ret < 0) {
> - fuse_reply_err(req, -ret);
> - return;
> - }
> - }
> -
> - do {
> - int size = MIN(length, BDRV_REQUEST_MAX_BYTES);
> -
> - ret = blk_pwrite_zeroes(exp->common.blk,
> - offset, size, 0);
> - offset += size;
> - length -= size;
> - } while (ret == 0 && length > 0);
I might not have enough coffee today yet, but I think your patch is wrong:
If the code executed this do-while loop/if-statement-branch, the other
else-statements below were never called. Now with your patch, if the
do-while loop in fuse_fallocate_zero_range() is called, the function will
return with "true" at the end, causing the other else-statements below to be
called, so that ret finally gets set to -EOPNOTSUPP. Or did I miss something?
Thomas
> - }
> -#endif /* CONFIG_FALLOCATE_ZERO_RANGE */
> - else if (!mode) {
> + } else if (!fuse_fallocate_zero_range(req, inode, blk_len, mode, offset, &length)) {
> + return;
> + } else if (!mode) {
> /* We can only fallocate at the EOF with a truncate */
> if (offset < blk_len) {
> fuse_reply_err(req, EOPNOTSUPP);
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 1/3] block/export/fuse: Extract fuse_fallocate_punch_hole()
2022-01-25 11:10 ` Thomas Huth
@ 2022-01-25 16:02 ` Philippe Mathieu-Daudé via
0 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-01-25 16:02 UTC (permalink / raw)
To: Thomas Huth, qemu-devel
Cc: qemu-block, Li-Wen Hsu, Ed Maste, Kyle Evans, Fabrice Fontaine
On 1/25/22 12:10, Thomas Huth wrote:
> On 24/01/2022 23.03, Philippe Mathieu-Daudé via wrote:
>> Extract fuse_fallocate_punch_hole() to avoid #ifdef'ry
>> mixed within if/else statement.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> block/export/fuse.c | 59 +++++++++++++++++++++++++++------------------
>> 1 file changed, 35 insertions(+), 24 deletions(-)
>>
>> diff --git a/block/export/fuse.c b/block/export/fuse.c
>> index 6710d8aed86..31cb0503adc 100644
>> --- a/block/export/fuse.c
>> +++ b/block/export/fuse.c
>> @@ -603,6 +603,38 @@ static void fuse_write(fuse_req_t req, fuse_ino_t
>> inode, const char *buf,
>> }
>> }
>> +static bool fuse_fallocate_zero_range(fuse_req_t req, fuse_ino_t
>> inode,
>> + int mode, int64_t blk_len,
>> + off_t offset, off_t *length)
>> +{
>> +#ifdef CONFIG_FALLOCATE_ZERO_RANGE
>> + FuseExport *exp = fuse_req_userdata(req);
>> +
>> + if (mode & FALLOC_FL_ZERO_RANGE) {
>> + int ret;
>> +
>> + if (!(mode & FALLOC_FL_KEEP_SIZE) && offset + *length >
>> blk_len) {
>> + /* No need for zeroes, we are going to write them
>> ourselves */
>> + ret = fuse_do_truncate(exp, offset + *length, false,
>> + PREALLOC_MODE_OFF);
>> + if (ret < 0) {
>> + fuse_reply_err(req, -ret);
>> + return false;
>> + }
>> + }
>> +
>> + do {
>> + int size = MIN(*length, BDRV_REQUEST_MAX_BYTES);
>> +
>> + ret = blk_pwrite_zeroes(exp->common.blk, offset, size, 0);
>> + offset += size;
>> + *length -= size;
>> + } while (ret == 0 && *length > 0);
>> + }
>> +#endif /* CONFIG_FALLOCATE_ZERO_RANGE */
>> + return true;
>> +}
>> +
>> /**
>> * Let clients perform various fallocate() operations.
>> */
>> @@ -642,30 +674,9 @@ static void fuse_fallocate(fuse_req_t req,
>> fuse_ino_t inode, int mode,
>> offset += size;
>> length -= size;
>> } while (ret == 0 && length > 0);
>> - }
>> -#ifdef CONFIG_FALLOCATE_ZERO_RANGE
>> - else if (mode & FALLOC_FL_ZERO_RANGE) {
>> - if (!(mode & FALLOC_FL_KEEP_SIZE) && offset + length >
>> blk_len) {
>> - /* No need for zeroes, we are going to write them
>> ourselves */
>> - ret = fuse_do_truncate(exp, offset + length, false,
>> - PREALLOC_MODE_OFF);
>> - if (ret < 0) {
>> - fuse_reply_err(req, -ret);
>> - return;
>> - }
>> - }
>> -
>> - do {
>> - int size = MIN(length, BDRV_REQUEST_MAX_BYTES);
>> -
>> - ret = blk_pwrite_zeroes(exp->common.blk,
>> - offset, size, 0);
>> - offset += size;
>> - length -= size;
>> - } while (ret == 0 && length > 0);
>
> I might not have enough coffee today yet, but I think your patch is
> wrong: If the code executed this do-while loop/if-statement-branch, the
> other else-statements below were never called. Now with your patch, if
> the do-while loop in fuse_fallocate_zero_range() is called, the function
> will return with "true" at the end, causing the other else-statements
> below to be called, so that ret finally gets set to -EOPNOTSUPP. Or did
> I miss something?
You are right, this patch is crap, sorry.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/3] block/export/fuse: Extract fuse_fallocate_zero_range()
2022-01-24 22:03 [PATCH v2 0/3] block/export/fuse: Fix build failure on FreeBSD Philippe Mathieu-Daudé via
2022-01-24 22:03 ` [PATCH v2 1/3] block/export/fuse: Extract fuse_fallocate_punch_hole() Philippe Mathieu-Daudé via
@ 2022-01-24 22:03 ` Philippe Mathieu-Daudé via
2022-01-25 11:28 ` Thomas Huth
2022-01-24 22:03 ` [PATCH v2 3/3] block/export/fuse: Fix build failure on FreeBSD Philippe Mathieu-Daudé via
2 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-01-24 22:03 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Li-Wen Hsu, Ed Maste, Kyle Evans, Fabrice Fontaine,
Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
block/export/fuse.c | 48 +++++++++++++++++++++++++++++----------------
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/block/export/fuse.c b/block/export/fuse.c
index 31cb0503adc..3a158342c75 100644
--- a/block/export/fuse.c
+++ b/block/export/fuse.c
@@ -603,6 +603,35 @@ static void fuse_write(fuse_req_t req, fuse_ino_t inode, const char *buf,
}
}
+static bool fuse_fallocate_punch_hole(fuse_req_t req, fuse_ino_t inode,
+ int mode, int64_t blk_len,
+ off_t offset, off_t *length)
+{
+ FuseExport *exp = fuse_req_userdata(req);
+
+ if (mode & FALLOC_FL_KEEP_SIZE) {
+ *length = MIN(*length, blk_len - offset);
+ }
+
+ if (mode & FALLOC_FL_PUNCH_HOLE) {
+ int ret;
+
+ if (!(mode & FALLOC_FL_KEEP_SIZE)) {
+ fuse_reply_err(req, EINVAL);
+ return false;
+ }
+
+ do {
+ int size = MIN(*length, BDRV_REQUEST_MAX_BYTES);
+
+ ret = blk_pdiscard(exp->common.blk, offset, size);
+ offset += size;
+ *length -= size;
+ } while (ret == 0 && *length > 0);
+ }
+ return true;
+}
+
static bool fuse_fallocate_zero_range(fuse_req_t req, fuse_ino_t inode,
int mode, int64_t blk_len,
off_t offset, off_t *length)
@@ -657,23 +686,8 @@ static void fuse_fallocate(fuse_req_t req, fuse_ino_t inode, int mode,
return;
}
- if (mode & FALLOC_FL_KEEP_SIZE) {
- length = MIN(length, blk_len - offset);
- }
-
- if (mode & FALLOC_FL_PUNCH_HOLE) {
- if (!(mode & FALLOC_FL_KEEP_SIZE)) {
- fuse_reply_err(req, EINVAL);
- return;
- }
-
- do {
- int size = MIN(length, BDRV_REQUEST_MAX_BYTES);
-
- ret = blk_pdiscard(exp->common.blk, offset, size);
- offset += size;
- length -= size;
- } while (ret == 0 && length > 0);
+ if (!fuse_fallocate_punch_hole(req, inode, mode, blk_len, offset, &length)) {
+ return;
} else if (!fuse_fallocate_zero_range(req, inode, blk_len, mode, offset, &length)) {
return;
} else if (!mode) {
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 2/3] block/export/fuse: Extract fuse_fallocate_zero_range()
2022-01-24 22:03 ` [PATCH v2 2/3] block/export/fuse: Extract fuse_fallocate_zero_range() Philippe Mathieu-Daudé via
@ 2022-01-25 11:28 ` Thomas Huth
0 siblings, 0 replies; 7+ messages in thread
From: Thomas Huth @ 2022-01-25 11:28 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Kyle Evans, Ed Maste, Li-Wen Hsu, qemu-block, Fabrice Fontaine
On 24/01/2022 23.03, Philippe Mathieu-Daudé via wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> block/export/fuse.c | 48 +++++++++++++++++++++++++++++----------------
> 1 file changed, 31 insertions(+), 17 deletions(-)
>
> diff --git a/block/export/fuse.c b/block/export/fuse.c
> index 31cb0503adc..3a158342c75 100644
> --- a/block/export/fuse.c
> +++ b/block/export/fuse.c
> @@ -603,6 +603,35 @@ static void fuse_write(fuse_req_t req, fuse_ino_t inode, const char *buf,
> }
> }
>
> +static bool fuse_fallocate_punch_hole(fuse_req_t req, fuse_ino_t inode,
> + int mode, int64_t blk_len,
> + off_t offset, off_t *length)
> +{
> + FuseExport *exp = fuse_req_userdata(req);
> +
> + if (mode & FALLOC_FL_KEEP_SIZE) {
> + *length = MIN(*length, blk_len - offset);
> + }
> +
> + if (mode & FALLOC_FL_PUNCH_HOLE) {
> + int ret;
> +
> + if (!(mode & FALLOC_FL_KEEP_SIZE)) {
> + fuse_reply_err(req, EINVAL);
> + return false;
> + }
> +
> + do {
> + int size = MIN(*length, BDRV_REQUEST_MAX_BYTES);
> +
> + ret = blk_pdiscard(exp->common.blk, offset, size);
> + offset += size;
> + *length -= size;
> + } while (ret == 0 && *length > 0);
> + }
> + return true;
> +}
> +
> static bool fuse_fallocate_zero_range(fuse_req_t req, fuse_ino_t inode,
> int mode, int64_t blk_len,
> off_t offset, off_t *length)
> @@ -657,23 +686,8 @@ static void fuse_fallocate(fuse_req_t req, fuse_ino_t inode, int mode,
> return;
> }
>
> - if (mode & FALLOC_FL_KEEP_SIZE) {
> - length = MIN(length, blk_len - offset);
> - }
> -
> - if (mode & FALLOC_FL_PUNCH_HOLE) {
> - if (!(mode & FALLOC_FL_KEEP_SIZE)) {
> - fuse_reply_err(req, EINVAL);
> - return;
> - }
> -
> - do {
> - int size = MIN(length, BDRV_REQUEST_MAX_BYTES);
> -
> - ret = blk_pdiscard(exp->common.blk, offset, size);
> - offset += size;
> - length -= size;
> - } while (ret == 0 && length > 0);
> + if (!fuse_fallocate_punch_hole(req, inode, mode, blk_len, offset, &length)) {
> + return;
Same issue as with the previous patch? If the do-while loop has been
executed, the else branches below should not be called...
Thomas
> } else if (!fuse_fallocate_zero_range(req, inode, blk_len, mode, offset, &length)) {
> return;
> } else if (!mode) {
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 3/3] block/export/fuse: Fix build failure on FreeBSD
2022-01-24 22:03 [PATCH v2 0/3] block/export/fuse: Fix build failure on FreeBSD Philippe Mathieu-Daudé via
2022-01-24 22:03 ` [PATCH v2 1/3] block/export/fuse: Extract fuse_fallocate_punch_hole() Philippe Mathieu-Daudé via
2022-01-24 22:03 ` [PATCH v2 2/3] block/export/fuse: Extract fuse_fallocate_zero_range() Philippe Mathieu-Daudé via
@ 2022-01-24 22:03 ` Philippe Mathieu-Daudé via
2 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-01-24 22:03 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Li-Wen Hsu, Ed Maste, Kyle Evans, Fabrice Fontaine,
Philippe Mathieu-Daudé
When building on FreeBSD we get:
[816/6851] Compiling C object libblockdev.fa.p/block_export_fuse.c.o
../block/export/fuse.c:612:16: error: use of undeclared identifier 'FALLOC_FL_KEEP_SIZE'
if (mode & FALLOC_FL_KEEP_SIZE) {
^
../block/export/fuse.c:616:16: error: use of undeclared identifier 'FALLOC_FL_PUNCH_HOLE'
if (mode & FALLOC_FL_PUNCH_HOLE) {
^
../block/export/fuse.c:619:22: error: use of undeclared identifier 'FALLOC_FL_KEEP_SIZE'
if (!(mode & FALLOC_FL_KEEP_SIZE)) {
^
3 errors generated.
FAILED: libblockdev.fa.p/block_export_fuse.c.o
Meson indeed reported FALLOC_FL_PUNCH_HOLE is not available:
C compiler for the host machine: cc (clang 10.0.1 "FreeBSD clang version 10.0.1")
Checking for function "fallocate" : NO
Checking for function "posix_fallocate" : YES
Header <linux/falloc.h> has symbol "FALLOC_FL_PUNCH_HOLE" : NO
Header <linux/falloc.h> has symbol "FALLOC_FL_ZERO_RANGE" : NO
...
Similarly to commit 304332039 ("block/export/fuse.c: fix musl build"),
guard the code requiring FALLOC_FL_KEEP_SIZE / FALLOC_FL_PUNCH_HOLE
definitions under CONFIG_FALLOCATE_PUNCH_HOLE #ifdef'ry.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
block/export/fuse.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/block/export/fuse.c b/block/export/fuse.c
index 3a158342c75..4c5d662be87 100644
--- a/block/export/fuse.c
+++ b/block/export/fuse.c
@@ -607,6 +607,7 @@ static bool fuse_fallocate_punch_hole(fuse_req_t req, fuse_ino_t inode,
int mode, int64_t blk_len,
off_t offset, off_t *length)
{
+#ifdef CONFIG_FALLOCATE_PUNCH_HOLE
FuseExport *exp = fuse_req_userdata(req);
if (mode & FALLOC_FL_KEEP_SIZE) {
@@ -629,6 +630,7 @@ static bool fuse_fallocate_punch_hole(fuse_req_t req, fuse_ino_t inode,
*length -= size;
} while (ret == 0 && *length > 0);
}
+#endif /* CONFIG_FALLOCATE_PUNCH_HOLE */
return true;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 7+ messages in thread