subject:"\[Devel\] \[PATCH 2\/2\] fs\/fuse kio_pcs\: check fuse

Re: [Devel] [PATCH 2/2] fs/fuse kio_pcs: check fuse_conn args

2018-05-17 Thread Kirill Tkhai

On 17.05.2018 14:31, Pavel Butsykin wrote:
> On 17.05.2018 13:05, Kirill Tkhai wrote:
>> Hi, Pasha,
>>
>> On 17.05.2018 12:40, Pavel Butsykin wrote:
>>> Allow initialization of kdirect only for vstorage/pstorage.
>>>
>>> Signed-off-by: Pavel Butsykin 
>>> ---
>>>   drivers/block/ploop/dev.c  |  4 
>>>   fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 23 +--
>>>   include/linux/fs.h |  5 +
>>>   3 files changed, 22 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c
>>> index 0e72656ec8f9..c6094144c7a5 100644
>>> --- a/drivers/block/ploop/dev.c
>>> +++ b/drivers/block/ploop/dev.c
>>> @@ -3880,10 +3880,6 @@ static int ploop_truncate(struct ploop_device * plo, 
>>> unsigned long arg)
>>>   return err;
>>>   }
>>>   -#define IS_PSTORAGE(sb) (sb->s_magic == FUSE_SUPER_MAGIC && \
>>> - (!strcmp(sb->s_subtype, "pstorage") || \
>>> -  !strcmp(sb->s_subtype, "vstorage")))
>>> -
>>>   static int ploop_bd_full(struct backing_dev_info *bdi, long long nr, int 
>>> root)
>>>   {
>>>   struct ploop_device *plo  = bdi->congested_data;
>>> diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c 
>>> b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>>> index b8503d55e246..4b9c4a304571 100644
>>> --- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>>> +++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>>> @@ -144,13 +144,24 @@ void kpcs_conn_abort(struct fuse_conn *fc)
>>>   }
>>>     static int kpcs_probe(struct fuse_conn *fc, char *name)
>>> -
>>>   {
>>> -    printk("%s TODO IMPLEMENT check fuse_conn args here!\n", __FUNCTION__);
>>> -    if (!strncmp(name, kio_pcs_ops.name, FUSE_KIO_NAME))
>>> -    return 1;
>>> +    if (strncmp(name, kio_pcs_ops.name, FUSE_KIO_NAME)) {
>>> +    printk(KERN_ERR "FUSE: kio_pcs: invalid kio_ops name: %s\n", name);
>>
>> Despite there was printk() without error level modifier (which is not 
>> correct),
>> and printk(KERN_ERR) is better, let's follow the modern Linux kernel style,
>> which anyway will be requested in case of submitting patches to mainstream.
>> pr_err() is more likely to use for such the printing (see the rest of pr_* 
>> in printk.h).
>>
> 
> ok, thanks.
> 
>>> +    return 0;
>>> +    }
>>>   -    return 0;
>>> +    if (!(fc->flags & FUSE_KDIRECT_IO)) {
>>> +    printk(KERN_ERR "FUSE: kio_pcs: kdirect is not enabled\n");
>>> +    return 0;
>>> +    }
>>
>> I don't think this is needed, since we already check for this bit in 
>> fuse_fill_super()
>> before call of fuse_kio_get(). But I'm not insist on this.
>>
> 
> And what is the meaning of this check:
> "!strncmp(name, kio_pcs_ops.name, FUSE_KIO_NAME)" ?

FUSE_KIO_NAME looks like a define of string, but it's a size,
and it was confusing for me.

Possible, we can drop this too. Less code is better for the attention.

> We also already have this check. I assumed this can protect against
> possible mistakes in the future.

I think, we fastly find the root cause in case of something is wrong
on mount. Duplications confuse... But I'm not insist on this.

>>> +
>>> +    if (!IS_PSTORAGE(fc->sb)) {
>>> +    printk(KERN_ERR "FUSE: kio_pcs: kdirect is only available for"
>>> +    "pstorage/vstorage fuse mount\n");
>>> +    return 0;
>>> +    }
>>> +
>>> +    return 1;
>>>   }
>>>     @@ -1202,7 +1213,7 @@ err:
>>>   static struct fuse_kio_ops kio_pcs_ops = {
>>>   .name    = "pcs",
>>>   .owner    = THIS_MODULE,
>>> -    .probe    = kpcs_probe, /*TODO: check sb->dev name */
>>> +    .probe    = kpcs_probe,
>>>     .conn_init    = kpcs_conn_init,
>>>   .conn_fini    = kpcs_conn_fini,
>>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>>> index a1dc3521f979..21770550b6c1 100644
>>> --- a/include/linux/fs.h
>>> +++ b/include/linux/fs.h
>>> @@ -1713,6 +1713,11 @@ struct super_block {
>>>   struct list_lru    s_inode_lru cacheline_aligned_in_smp;
>>>   };
>>>   +#define IS_PSTORAGE(sb) ((sb)->s_magic == FUSE_SUPER_MAGIC && \
>>> + (sb)->s_subtype && \
>>> + (!strcmp((sb)->s_subtype, "pstorage") || \
>>> +  !strcmp((sb)->s_subtype, "vstorage")))
>>> +
>>>   extern const unsigned super_block_wrapper_version;
>>>   struct super_block_wrapper {
>>>   struct super_block sb;
>>>
>>
>> Thanks,
>> Kirill
>>
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH 2/2] fs/fuse kio_pcs: check fuse_conn args

2018-05-17 Thread Pavel Butsykin


On 17.05.2018 13:05, Kirill Tkhai wrote:

Hi, Pasha,

On 17.05.2018 12:40, Pavel Butsykin wrote:

Allow initialization of kdirect only for vstorage/pstorage.

Signed-off-by: Pavel Butsykin 
---
  drivers/block/ploop/dev.c  |  4 
  fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 23 +--
  include/linux/fs.h |  5 +
  3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c
index 0e72656ec8f9..c6094144c7a5 100644
--- a/drivers/block/ploop/dev.c
+++ b/drivers/block/ploop/dev.c
@@ -3880,10 +3880,6 @@ static int ploop_truncate(struct ploop_device * plo, 
unsigned long arg)
return err;
  }
  
-#define IS_PSTORAGE(sb) (sb->s_magic == FUSE_SUPER_MAGIC && \

-(!strcmp(sb->s_subtype, "pstorage") || \
- !strcmp(sb->s_subtype, "vstorage")))
-
  static int ploop_bd_full(struct backing_dev_info *bdi, long long nr, int root)
  {
struct ploop_device *plo  = bdi->congested_data;
diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c 
b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
index b8503d55e246..4b9c4a304571 100644
--- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
+++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
@@ -144,13 +144,24 @@ void kpcs_conn_abort(struct fuse_conn *fc)
  }
  
  static int kpcs_probe(struct fuse_conn *fc, char *name)

-
  {
-   printk("%s TODO IMPLEMENT check fuse_conn args here!\n", __FUNCTION__);
-   if (!strncmp(name, kio_pcs_ops.name, FUSE_KIO_NAME))
-   return 1;
+   if (strncmp(name, kio_pcs_ops.name, FUSE_KIO_NAME)) {
+   printk(KERN_ERR "FUSE: kio_pcs: invalid kio_ops name: %s\n", 
name);


Despite there was printk() without error level modifier (which is not correct),
and printk(KERN_ERR) is better, let's follow the modern Linux kernel style,
which anyway will be requested in case of submitting patches to mainstream.
pr_err() is more likely to use for such the printing (see the rest of pr_* in 
printk.h).



ok, thanks.


+   return 0;
+   }
  
-	return 0;

+   if (!(fc->flags & FUSE_KDIRECT_IO)) {
+   printk(KERN_ERR "FUSE: kio_pcs: kdirect is not enabled\n");
+   return 0;
+   }


I don't think this is needed, since we already check for this bit in 
fuse_fill_super()
before call of fuse_kio_get(). But I'm not insist on this.



And what is the meaning of this check:
"!strncmp(name, kio_pcs_ops.name, FUSE_KIO_NAME)" ?

We also already have this check. I assumed this can protect against
possible mistakes in the future.


+
+   if (!IS_PSTORAGE(fc->sb)) {
+   printk(KERN_ERR "FUSE: kio_pcs: kdirect is only available for"
+   "pstorage/vstorage fuse mount\n");
+   return 0;
+   }
+
+   return 1;
  }
  
  
@@ -1202,7 +1213,7 @@ err:

  static struct fuse_kio_ops kio_pcs_ops = {
.name   = "pcs",
.owner  = THIS_MODULE,
-   .probe  = kpcs_probe, /*TODO: check sb->dev name */
+   .probe  = kpcs_probe,
  
  	.conn_init	= kpcs_conn_init,

.conn_fini  = kpcs_conn_fini,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index a1dc3521f979..21770550b6c1 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1713,6 +1713,11 @@ struct super_block {
struct list_lru s_inode_lru cacheline_aligned_in_smp;
  };
  
+#define IS_PSTORAGE(sb) ((sb)->s_magic == FUSE_SUPER_MAGIC && \

+(sb)->s_subtype && \
+(!strcmp((sb)->s_subtype, "pstorage") || \
+ !strcmp((sb)->s_subtype, "vstorage")))
+
  extern const unsigned super_block_wrapper_version;
  struct super_block_wrapper {
struct super_block sb;



Thanks,
Kirill


___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH 2/2] fs/fuse kio_pcs: check fuse_conn args

2018-05-17 Thread Kirill Tkhai

Hi, Pasha,

On 17.05.2018 12:40, Pavel Butsykin wrote:
> Allow initialization of kdirect only for vstorage/pstorage.
> 
> Signed-off-by: Pavel Butsykin 
> ---
>  drivers/block/ploop/dev.c  |  4 
>  fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 23 +--
>  include/linux/fs.h |  5 +
>  3 files changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c
> index 0e72656ec8f9..c6094144c7a5 100644
> --- a/drivers/block/ploop/dev.c
> +++ b/drivers/block/ploop/dev.c
> @@ -3880,10 +3880,6 @@ static int ploop_truncate(struct ploop_device * plo, 
> unsigned long arg)
>   return err;
>  }
>  
> -#define IS_PSTORAGE(sb) (sb->s_magic == FUSE_SUPER_MAGIC && \
> -  (!strcmp(sb->s_subtype, "pstorage") || \
> -   !strcmp(sb->s_subtype, "vstorage")))
> -
>  static int ploop_bd_full(struct backing_dev_info *bdi, long long nr, int 
> root)
>  {
>   struct ploop_device *plo  = bdi->congested_data;
> diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c 
> b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> index b8503d55e246..4b9c4a304571 100644
> --- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> +++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> @@ -144,13 +144,24 @@ void kpcs_conn_abort(struct fuse_conn *fc)
>  }
>  
>  static int kpcs_probe(struct fuse_conn *fc, char *name)
> -
>  {
> - printk("%s TODO IMPLEMENT check fuse_conn args here!\n", __FUNCTION__);
> - if (!strncmp(name, kio_pcs_ops.name, FUSE_KIO_NAME))
> - return 1;
> + if (strncmp(name, kio_pcs_ops.name, FUSE_KIO_NAME)) {
> + printk(KERN_ERR "FUSE: kio_pcs: invalid kio_ops name: %s\n", 
> name);

Despite there was printk() without error level modifier (which is not correct),
and printk(KERN_ERR) is better, let's follow the modern Linux kernel style,
which anyway will be requested in case of submitting patches to mainstream.
pr_err() is more likely to use for such the printing (see the rest of pr_* in 
printk.h).

> + return 0;
> + }
>  
> - return 0;
> + if (!(fc->flags & FUSE_KDIRECT_IO)) {
> + printk(KERN_ERR "FUSE: kio_pcs: kdirect is not enabled\n");
> + return 0;
> + }

I don't think this is needed, since we already check for this bit in 
fuse_fill_super()
before call of fuse_kio_get(). But I'm not insist on this.

> +
> + if (!IS_PSTORAGE(fc->sb)) {
> + printk(KERN_ERR "FUSE: kio_pcs: kdirect is only available for"
> + "pstorage/vstorage fuse mount\n");
> + return 0;
> + }
> +
> + return 1;
>  }
>  
>  
> @@ -1202,7 +1213,7 @@ err:
>  static struct fuse_kio_ops kio_pcs_ops = {
>   .name   = "pcs",
>   .owner  = THIS_MODULE,
> - .probe  = kpcs_probe, /*TODO: check sb->dev name */
> + .probe  = kpcs_probe,
>  
>   .conn_init  = kpcs_conn_init,
>   .conn_fini  = kpcs_conn_fini,
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index a1dc3521f979..21770550b6c1 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1713,6 +1713,11 @@ struct super_block {
>   struct list_lru s_inode_lru cacheline_aligned_in_smp;
>  };
>  
> +#define IS_PSTORAGE(sb) ((sb)->s_magic == FUSE_SUPER_MAGIC && \
> +  (sb)->s_subtype && \
> +  (!strcmp((sb)->s_subtype, "pstorage") || \
> +   !strcmp((sb)->s_subtype, "vstorage")))
> +
>  extern const unsigned super_block_wrapper_version;
>  struct super_block_wrapper {
>   struct super_block sb;
> 

Thanks,
Kirill
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH 2/2] fs/fuse kio_pcs: check fuse_conn args

2018-05-17 Thread Pavel Butsykin

Allow initialization of kdirect only for vstorage/pstorage.

Signed-off-by: Pavel Butsykin 
---
 drivers/block/ploop/dev.c  |  4 
 fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 23 +--
 include/linux/fs.h |  5 +
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/block/ploop/dev.c b/drivers/block/ploop/dev.c
index 0e72656ec8f9..c6094144c7a5 100644
--- a/drivers/block/ploop/dev.c
+++ b/drivers/block/ploop/dev.c
@@ -3880,10 +3880,6 @@ static int ploop_truncate(struct ploop_device * plo, 
unsigned long arg)
return err;
 }
 
-#define IS_PSTORAGE(sb) (sb->s_magic == FUSE_SUPER_MAGIC && \
-(!strcmp(sb->s_subtype, "pstorage") || \
- !strcmp(sb->s_subtype, "vstorage")))
-
 static int ploop_bd_full(struct backing_dev_info *bdi, long long nr, int root)
 {
struct ploop_device *plo  = bdi->congested_data;
diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c 
b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
index b8503d55e246..4b9c4a304571 100644
--- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
+++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
@@ -144,13 +144,24 @@ void kpcs_conn_abort(struct fuse_conn *fc)
 }
 
 static int kpcs_probe(struct fuse_conn *fc, char *name)
-
 {
-   printk("%s TODO IMPLEMENT check fuse_conn args here!\n", __FUNCTION__);
-   if (!strncmp(name, kio_pcs_ops.name, FUSE_KIO_NAME))
-   return 1;
+   if (strncmp(name, kio_pcs_ops.name, FUSE_KIO_NAME)) {
+   printk(KERN_ERR "FUSE: kio_pcs: invalid kio_ops name: %s\n", 
name);
+   return 0;
+   }
 
-   return 0;
+   if (!(fc->flags & FUSE_KDIRECT_IO)) {
+   printk(KERN_ERR "FUSE: kio_pcs: kdirect is not enabled\n");
+   return 0;
+   }
+
+   if (!IS_PSTORAGE(fc->sb)) {
+   printk(KERN_ERR "FUSE: kio_pcs: kdirect is only available for"
+   "pstorage/vstorage fuse mount\n");
+   return 0;
+   }
+
+   return 1;
 }
 
 
@@ -1202,7 +1213,7 @@ err:
 static struct fuse_kio_ops kio_pcs_ops = {
.name   = "pcs",
.owner  = THIS_MODULE,
-   .probe  = kpcs_probe, /*TODO: check sb->dev name */
+   .probe  = kpcs_probe,
 
.conn_init  = kpcs_conn_init,
.conn_fini  = kpcs_conn_fini,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index a1dc3521f979..21770550b6c1 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1713,6 +1713,11 @@ struct super_block {
struct list_lru s_inode_lru cacheline_aligned_in_smp;
 };
 
+#define IS_PSTORAGE(sb) ((sb)->s_magic == FUSE_SUPER_MAGIC && \
+(sb)->s_subtype && \
+(!strcmp((sb)->s_subtype, "pstorage") || \
+ !strcmp((sb)->s_subtype, "vstorage")))
+
 extern const unsigned super_block_wrapper_version;
 struct super_block_wrapper {
struct super_block sb;
-- 
2.15.1

___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH 2/2] fs/fuse kio_pcs: check fuse_conn args

Re: [Devel] [PATCH 2/2] fs/fuse kio_pcs: check fuse_conn args

Re: [Devel] [PATCH 2/2] fs/fuse kio_pcs: check fuse_conn args

[Devel] [PATCH 2/2] fs/fuse kio_pcs: check fuse_conn args

4 matches

Site Navigation

Mail list logo

Footer information