Re: Review request: changes in samba vxfs plugin

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: Review request: changes in samba vxfs plugin

Samba - samba-technical mailing list
On Thu, Jun 22, 2017 at 06:13:40PM +0000, Pooja Mahadik wrote:
> Hi,
>
> Please review attached patch for changes in samba-vxfs plugin.
> We have implemented ‘set_dos_attributes_fn’ & ‘fset_dos_attributes_fn’ VFS operations for vxfs plugin. New implementation of these operations calls the default set dos attribute function(SMB_VFS_NEXT_SET_DOS_ATTRIBUTES) to set the given attribute. In addition to this, we are trapping set/clear of ‘read-only’ attribute to take some additional actions upon set/clear of read-only attribute.
>
> Please revert in case of any queries/issues.

One quick comment - can you change all the DEBUG(10,(...));
statements to the modern form. See README.coding:

DEBUG statements
----------------

Use these following macros instead of DEBUG:

DBG_ERR log level 0             error conditions
DBG_WARNING     log level 1             warning conditions
DBG_NOTICE      log level 3             normal, but significant, condition
DBG_INFO        log level 5             informational message
DBG_DEBUG       log level 10            debug-level message

Example usage:

DBG_ERR("Memory allocation failed\n");
DBG_DEBUG("Received %d bytes\n", count);

The messages from these macros are automatically prefixed with the
function name.

Thanks !

Jeremy.

Reply | Threaded
Open this post in threaded view
|

Re: [EXTERNAL] Re: Review request: changes in samba vxfs plugin

Samba - samba-technical mailing list
Earlier mail to samba-technical lists email failed, sending code diff again.

Thanks,
-pooja



On 6/26/17, 11:30 PM, "Pooja Mahadik" <[hidden email]> wrote:

>Hi Jeremy,
>
>I incorporated previous comment, please find attached updated diff.
>
>Thanks & Regards,
>-pooja
>
>
>
>On 6/23/17, 4:32 PM, "Pooja Mahadik" <[hidden email]> wrote:
>
>>Hi Jeremy,
>>
>>Ok, I’m incorporating below comment.
>>
>>Thanks,
>>-pooja
>>
>>
>>
>>
>>On 6/23/17, 5:55 AM, "Jeremy Allison" <[hidden email]> wrote:
>>
>>>On Thu, Jun 22, 2017 at 06:13:40PM +0000, Pooja Mahadik wrote:
>>>> Hi,
>>>>
>>>> Please review attached patch for changes in samba-vxfs plugin.
>>>> We have implemented ‘set_dos_attributes_fn’ & ‘fset_dos_attributes_fn’ VFS operations for vxfs plugin. New implementation of these operations calls the default set dos attribute function(SMB_VFS_NEXT_SET_DOS_ATTRIBUTES) to set the given attribute. In addition to this, we are trapping set/clear of ‘read-only’ attribute to take some additional actions upon set/clear of read-only attribute.
>>>>
>>>> Please revert in case of any queries/issues.
>>>
>>>One quick comment - can you change all the DEBUG(10,(...));
>>>statements to the modern form. See README.coding:
>>>
>>>DEBUG statements
>>>----------------
>>>
>>>Use these following macros instead of DEBUG:
>>>
>>>DBG_ERR log level 0             error conditions
>>>DBG_WARNING     log level 1             warning conditions
>>>DBG_NOTICE      log level 3             normal, but significant, condition
>>>DBG_INFO        log level 5             informational message
>>>DBG_DEBUG       log level 10            debug-level message
>>>
>>>Example usage:
>>>
>>>DBG_ERR("Memory allocation failed\n");
>>>DBG_DEBUG("Received %d bytes\n", count);
>>>
>>>The messages from these macros are automatically prefixed with the
>>>function name.
>>>
>>>Thanks !
>>>
>>>Jeremy.

0001-Changes-in-samba-vxfs-plugin.patch (9K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [EXTERNAL] Re: Review request: changes in samba vxfs plugin

Samba - samba-technical mailing list
Hi,

This is a kind reminder for this review request. I have incorporated initial comment from Jeremy. Also looking for review from 2nd reviewer.

Thanks & Regards,
-pooja

On 6/26/17, 11:38 PM, "Pooja Mahadik" <[hidden email]> wrote:

    Earlier mail to samba-technical lists email failed, sending code diff again.
   
    Thanks,
    -pooja
   
   
   
    On 6/26/17, 11:30 PM, "Pooja Mahadik" <[hidden email]> wrote:
   
    >Hi Jeremy,
    >
    >I incorporated previous comment, please find attached updated diff.
    >
    >Thanks & Regards,
    >-pooja
    >
    >
    >
    >On 6/23/17, 4:32 PM, "Pooja Mahadik" <[hidden email]> wrote:
    >
    >>Hi Jeremy,
    >>
    >>Ok, I’m incorporating below comment.
    >>
    >>Thanks,
    >>-pooja
    >>
    >>
    >>
    >>
    >>On 6/23/17, 5:55 AM, "Jeremy Allison" <[hidden email]> wrote:
    >>
    >>>On Thu, Jun 22, 2017 at 06:13:40PM +0000, Pooja Mahadik wrote:
    >>>> Hi,
    >>>>
    >>>> Please review attached patch for changes in samba-vxfs plugin.
    >>>> We have implemented ‘set_dos_attributes_fn’ & ‘fset_dos_attributes_fn’ VFS operations for vxfs plugin. New implementation of these operations calls the default set dos attribute function(SMB_VFS_NEXT_SET_DOS_ATTRIBUTES) to set the given attribute. In addition to this, we are trapping set/clear of ‘read-only’ attribute to take some additional actions upon set/clear of read-only attribute.
    >>>>
    >>>> Please revert in case of any queries/issues.
    >>>
    >>>One quick comment - can you change all the DEBUG(10,(...));
    >>>statements to the modern form. See README.coding:
    >>>
    >>>DEBUG statements
    >>>----------------
    >>>
    >>>Use these following macros instead of DEBUG:
    >>>
    >>>DBG_ERR log level 0             error conditions
    >>>DBG_WARNING     log level 1             warning conditions
    >>>DBG_NOTICE      log level 3             normal, but significant, condition
    >>>DBG_INFO        log level 5             informational message
    >>>DBG_DEBUG       log level 10            debug-level message
    >>>
    >>>Example usage:
    >>>
    >>>DBG_ERR("Memory allocation failed\n");
    >>>DBG_DEBUG("Received %d bytes\n", count);
    >>>
    >>>The messages from these macros are automatically prefixed with the
    >>>function name.
    >>>
    >>>Thanks !
    >>>
    >>>Jeremy.
   

Reply | Threaded
Open this post in threaded view
|

Re: [EXTERNAL] Re: Review request: changes in samba vxfs plugin

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Mon, Jun 26, 2017 at 06:00:32PM +0000, Pooja Mahadik wrote:
> Hi Jeremy,
>
> I incorporated previous comment, please find attached updated diff.
>
> Thanks & Regards,
> -pooja

Hi Pooja,

These changes look OK. 'Reviewed-by: Jeremy Allison <[hidden email]>'

Can I get a second Team reviewer ?

Cheers,

Jeremy.

Reply | Threaded
Open this post in threaded view
|

Re: [EXTERNAL] Re: Review request: changes in samba vxfs plugin

Samba - samba-technical mailing list
Hi Jeremy,

Thanks a lot for the review.

Can someone please do the second review for this patch?

Regards,
-pooja

On 7/20/17, 3:45 AM, "Jeremy Allison" <[hidden email]> wrote:

    On Mon, Jun 26, 2017 at 06:00:32PM +0000, Pooja Mahadik wrote:
    > Hi Jeremy,
    >
    > I incorporated previous comment, please find attached updated diff.
    >
    > Thanks & Regards,
    > -pooja
   
    Hi Pooja,
   
    These changes look OK. 'Reviewed-by: Jeremy Allison <[hidden email]>'
   
    Can I get a second Team reviewer ?
   
    Cheers,
   
    Jeremy.
   

Reply | Threaded
Open this post in threaded view
|

Re: [EXTERNAL] Re: Review request: changes in samba vxfs plugin

Samba - samba-technical mailing list
Hi Pooja,

On Sat, Aug 05, 2017 at 04:26:00PM +0000, Pooja Mahadik wrote:
> Hi Jeremy,
>
> Thanks a lot for the review.
>
> Can someone please do the second review for this patch?

sorry, but nack.

I don't think this would actually build...

Can you please check the attached suggested fixes?

Cheerio!
-slow
Reply | Threaded
Open this post in threaded view
|

Re: [EXTERNAL] Re: Review request: changes in samba vxfs plugin

Samba - samba-technical mailing list
Hi Ralph,

Thank You for the review.
Actually, I tried build after incorporating latest comments but probably did mistake while sending the diff.
I’m working on your review comments.

Best Regards,
-pooja

On 8/9/17, 6:38 PM, "Ralph Böhme" <[hidden email]> wrote:

    Hi Pooja,
   
    On Sat, Aug 05, 2017 at 04:26:00PM +0000, Pooja Mahadik wrote:
    > Hi Jeremy,
    >
    > Thanks a lot for the review.
    >
    > Can someone please do the second review for this patch?
   
    sorry, but nack.
   
    I don't think this would actually build...
   
    Can you please check the attached suggested fixes?
   
    Cheerio!
    -slow
   

Reply | Threaded
Open this post in threaded view
|

Re: [EXTERNAL] Re: Review request: changes in samba vxfs plugin

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Wed, Aug 09, 2017 at 03:08:21PM +0200, Ralph Böhme wrote:

> Hi Pooja,
>
> On Sat, Aug 05, 2017 at 04:26:00PM +0000, Pooja Mahadik wrote:
> > Hi Jeremy,
> >
> > Thanks a lot for the review.
> >
> > Can someone please do the second review for this patch?
>
> sorry, but nack.
>
> I don't think this would actually build...
>
> Can you please check the attached suggested fixes?

Ralph, thanks for the more careful review :-). We
probably need to get a build env for this somewhere...

Reply | Threaded
Open this post in threaded view
|

Re: [EXTERNAL] Re: Review request: changes in samba vxfs plugin

Samba - samba-technical mailing list
Hi Ralph, Jeremy,

I have incorporated review comments, please review attached updated diff.

Thanks & Regards,
-pooja

On 8/14/17, 10:40 PM, "Jeremy Allison" <[hidden email]> wrote:

    On Wed, Aug 09, 2017 at 03:08:21PM +0200, Ralph Böhme wrote:
    > Hi Pooja,
    >
    > On Sat, Aug 05, 2017 at 04:26:00PM +0000, Pooja Mahadik wrote:
    > > Hi Jeremy,
    > >
    > > Thanks a lot for the review.
    > >
    > > Can someone please do the second review for this patch?
    >
    > sorry, but nack.
    >
    > I don't think this would actually build...
    >
    > Can you please check the attached suggested fixes?
   
    Ralph, thanks for the more careful review :-). We
    probably need to get a build env for this somewhere...
   


0001-Changes-in-samba-vxfs-plugin.patch (9K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [EXTERNAL] Re: Review request: changes in samba vxfs plugin

Samba - samba-technical mailing list
On Sun, Aug 27, 2017 at 12:20:06PM +0000, Pooja Mahadik wrote:
> I have incorporated review comments, please review attached updated diff.

lgtm. Can I get a second team member review please?

Cheerio!
-slow

Reply | Threaded
Open this post in threaded view
|

Re: [EXTERNAL] Re: Review request: changes in samba vxfs plugin

Samba - samba-technical mailing list
On Wed, Sep 27, 2017 at 03:09:05PM +0200, Ralph Böhme via samba-technical wrote:
> On Sun, Aug 27, 2017 at 12:20:06PM +0000, Pooja Mahadik wrote:
> > I have incorporated review comments, please review attached updated diff.
>
> lgtm. Can I get a second team member review please?

LGTM. Pushed !

Reply | Threaded
Open this post in threaded view
|

Re: [EXTERNAL] Re: Review request: changes in samba vxfs plugin

Samba - samba-technical mailing list
Hi Jeremy, Ralph,

Thanks a lot for review, all help.

Best Regards,
-pooja

On 9/30/17, 5:31 AM, "Jeremy Allison" <[hidden email]> wrote:

    On Wed, Sep 27, 2017 at 03:09:05PM +0200, Ralph Böhme via samba-technical wrote:
    > On Sun, Aug 27, 2017 at 12:20:06PM +0000, Pooja Mahadik wrote:
    > > I have incorporated review comments, please review attached updated diff.
    >
    > lgtm. Can I get a second team member review please?
   
    LGTM. Pushed !