| Message ID | 1552226873-3668-1-git-send-email-hndksztwj@163.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | target:tcmu:add '\n' when return user space | expand |
Hi, On Sun, 10 Mar 2019 22:07:53 +0800, tangwenji wrote: > In function tcmu_get_global_max_data_area return string should include '\n'. Why? Please describe the motivation for your changes. > Signed-off-by: tangwenji <tang.wenji@zte.com.cn> > --- > drivers/target/target_core_user.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c > index 5831e0e..50229c1 100644 > --- a/drivers/target/target_core_user.c > +++ b/drivers/target/target_core_user.c > @@ -244,7 +244,7 @@ static int tcmu_set_global_max_data_area(const char *str, > static int tcmu_get_global_max_data_area(char *buffer, > const struct kernel_param *kp) > { > - return sprintf(buffer, "%d", TCMU_BLOCKS_TO_MBS(tcmu_global_max_blocks)); > + return sprintf(buffer, "%d\n", TCMU_BLOCKS_TO_MBS(tcmu_global_max_blocks)); Although minor, this is still a modification to a released API so probably can't be done without potentially breaking user-space. Cheers, David
On 03/10/2019 09:07 AM, tangwenji wrote: > From: tangwenji <tang.wenji@zte.com.cn> > > In function tcmu_get_global_max_data_area return string should include '\n'. > > Signed-off-by: tangwenji <tang.wenji@zte.com.cn> > --- > drivers/target/target_core_user.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c > index 5831e0e..50229c1 100644 > --- a/drivers/target/target_core_user.c > +++ b/drivers/target/target_core_user.c > @@ -244,7 +244,7 @@ static int tcmu_set_global_max_data_area(const char *str, > static int tcmu_get_global_max_data_area(char *buffer, > const struct kernel_param *kp) > { > - return sprintf(buffer, "%d", TCMU_BLOCKS_TO_MBS(tcmu_global_max_blocks)); > + return sprintf(buffer, "%d\n", TCMU_BLOCKS_TO_MBS(tcmu_global_max_blocks)); > } > > static const struct kernel_param_ops tcmu_global_max_data_area_op = { > Acked-by: Mike Christie <mchristi@redhat.com>
On 03/10/2019 09:41 AM, David Disseldorp wrote: > Hi, > > On Sun, 10 Mar 2019 22:07:53 +0800, tangwenji wrote: > >> In function tcmu_get_global_max_data_area return string should include '\n'. > > Why? Please describe the motivation for your changes. > >> Signed-off-by: tangwenji <tang.wenji@zte.com.cn> >> --- >> drivers/target/target_core_user.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c >> index 5831e0e..50229c1 100644 >> --- a/drivers/target/target_core_user.c >> +++ b/drivers/target/target_core_user.c >> @@ -244,7 +244,7 @@ static int tcmu_set_global_max_data_area(const char *str, >> static int tcmu_get_global_max_data_area(char *buffer, >> const struct kernel_param *kp) >> { >> - return sprintf(buffer, "%d", TCMU_BLOCKS_TO_MBS(tcmu_global_max_blocks)); >> + return sprintf(buffer, "%d\n", TCMU_BLOCKS_TO_MBS(tcmu_global_max_blocks)); > > Although minor, this is still a modification to a released API so > probably can't be done without potentially breaking user-space. > I think the lack of newline is breaking some tools, because they were looking for newlines due to the other files having it.
On 03/11/2019 11:05 AM, Mike Christie wrote: > On 03/10/2019 09:41 AM, David Disseldorp wrote: >> Hi, >> >> On Sun, 10 Mar 2019 22:07:53 +0800, tangwenji wrote: >> >>> In function tcmu_get_global_max_data_area return string should include '\n'. >> >> Why? Please describe the motivation for your changes. >> >>> Signed-off-by: tangwenji <tang.wenji@zte.com.cn> >>> --- >>> drivers/target/target_core_user.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c >>> index 5831e0e..50229c1 100644 >>> --- a/drivers/target/target_core_user.c >>> +++ b/drivers/target/target_core_user.c >>> @@ -244,7 +244,7 @@ static int tcmu_set_global_max_data_area(const char *str, >>> static int tcmu_get_global_max_data_area(char *buffer, >>> const struct kernel_param *kp) >>> { >>> - return sprintf(buffer, "%d", TCMU_BLOCKS_TO_MBS(tcmu_global_max_blocks)); >>> + return sprintf(buffer, "%d\n", TCMU_BLOCKS_TO_MBS(tcmu_global_max_blocks)); >> >> Although minor, this is still a modification to a released API so >> probably can't be done without potentially breaking user-space. >> > > I think the lack of newline is breaking some tools, because they were > looking for newlines due to the other files having it. > I guess that does not make sense. It will be easier to update userspace and we would have to carry the userspcae patch for older kernels, so the patch is not needed.
On Mon, 11 Mar 2019 11:15:53 -0500, Mike Christie wrote: > >> Although minor, this is still a modification to a released API so > >> probably can't be done without potentially breaking user-space. > >> > > > > I think the lack of newline is breaking some tools, because they were > > looking for newlines due to the other files having it. > > > > I guess that does not make sense. It will be easier to update userspace > and we would have to carry the userspcae patch for older kernels, so the > patch is not needed. Agreed. After getting burnt by 6baca7601bde I'm hesitant to make any changes to existing configFS attributes. Cheers, David
diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c index 5831e0e..50229c1 100644 --- a/drivers/target/target_core_user.c +++ b/drivers/target/target_core_user.c @@ -244,7 +244,7 @@ static int tcmu_set_global_max_data_area(const char *str, static int tcmu_get_global_max_data_area(char *buffer, const struct kernel_param *kp) { - return sprintf(buffer, "%d", TCMU_BLOCKS_TO_MBS(tcmu_global_max_blocks)); + return sprintf(buffer, "%d\n", TCMU_BLOCKS_TO_MBS(tcmu_global_max_blocks)); } static const struct kernel_param_ops tcmu_global_max_data_area_op = {