* [mdadm PATCH 1/1] Fix a build error
@ 2017-09-20 3:48 Xiao Ni
2017-09-29 21:47 ` Jes Sorensen
0 siblings, 1 reply; 7+ messages in thread
From: Xiao Ni @ 2017-09-20 3:48 UTC (permalink / raw)
To: linux-raid; +Cc: jsorensen, ncroxon, pmenzel, antlists
On the s390 platform the build fails with the error below.
Manage.c: In function 'Manage_subdevs':
Manage.c:1502:5: error: passing argument 3 of 'fstat_is_blkdev' from incompatible pointer type [-Werror]
fstat_is_blkdev(tfd, dv->devname, &rdev);
^
In file included from Manage.c:25:0:
mdadm.h:1446:12: note: expected 'dev_t *' but argument is of type 'long unsigned int *'
It was introduced by commit 0a6bff09 (mdadm/util: unify fstat
checking blkdev into function). It needs to pass a type 'dev_t'
argument to fstat_is_blkdev, but it passes a type 'unsigned
long' argument. So use a temporary variable to fix this.
Signed-off-by: Xiao Ni <xni@redhat.com>
Suggested-by: Paul Menzel <pmenzel@molgen.mpg.de>
Suggested-by: Wols Lists <antlists@youngman.org.uk>
---
Manage.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/Manage.c b/Manage.c
index 871d342..6de1fc2 100644
--- a/Manage.c
+++ b/Manage.c
@@ -1497,13 +1497,14 @@ int Manage_subdevs(char *devname, int fd,
*/
rdev = makedev(mj, mn);
} else {
+ dev_t device_id;
tfd = dev_open(dv->devname, O_RDONLY);
if (tfd >= 0) {
- fstat_is_blkdev(tfd, dv->devname, &rdev);
+ fstat_is_blkdev(tfd, dv->devname, &device_id);
close(tfd);
} else {
int open_err = errno;
- if (!stat_is_blkdev(dv->devname, &rdev)) {
+ if (!stat_is_blkdev(dv->devname, &device_id)) {
if (dv->disposition == 'M')
/* non-fatal. Also improbable */
continue;
@@ -1523,6 +1524,7 @@ int Manage_subdevs(char *devname, int fd,
goto abort;
}
}
+ rdev = (unsigned long)device_id;
}
switch(dv->disposition){
default:
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [mdadm PATCH 1/1] Fix a build error
2017-09-20 3:48 [mdadm PATCH 1/1] Fix a build error Xiao Ni
@ 2017-09-29 21:47 ` Jes Sorensen
2017-09-30 1:19 ` Xiao Ni
0 siblings, 1 reply; 7+ messages in thread
From: Jes Sorensen @ 2017-09-29 21:47 UTC (permalink / raw)
To: Xiao Ni, linux-raid; +Cc: ncroxon, pmenzel, antlists
On 09/19/2017 11:48 PM, Xiao Ni wrote:
> On the s390 platform the build fails with the error below.
> Manage.c: In function 'Manage_subdevs':
> Manage.c:1502:5: error: passing argument 3 of 'fstat_is_blkdev' from incompatible pointer type [-Werror]
> fstat_is_blkdev(tfd, dv->devname, &rdev);
> ^
> In file included from Manage.c:25:0:
> mdadm.h:1446:12: note: expected 'dev_t *' but argument is of type 'long unsigned int *'
>
> It was introduced by commit 0a6bff09 (mdadm/util: unify fstat
> checking blkdev into function). It needs to pass a type 'dev_t'
> argument to fstat_is_blkdev, but it passes a type 'unsigned
> long' argument. So use a temporary variable to fix this.
>
> Signed-off-by: Xiao Ni <xni@redhat.com>
> Suggested-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Suggested-by: Wols Lists <antlists@youngman.org.uk>
> ---
> Manage.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
So having a quick look at this, I have to say I don't like the casting
back and forth. The fact that we carry rdev in an unsigned long in
Manage_subdevs() seems dubious to me.
Did you look into what the implications would be to change it to a dev_t?
Jes
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [mdadm PATCH 1/1] Fix a build error
2017-09-29 21:47 ` Jes Sorensen
@ 2017-09-30 1:19 ` Xiao Ni
2017-09-30 13:08 ` Jes Sorensen
0 siblings, 1 reply; 7+ messages in thread
From: Xiao Ni @ 2017-09-30 1:19 UTC (permalink / raw)
To: Jes Sorensen, linux-raid; +Cc: ncroxon, pmenzel, antlists
On 09/30/2017 05:47 AM, Jes Sorensen wrote:
> On 09/19/2017 11:48 PM, Xiao Ni wrote:
>> On the s390 platform the build fails with the error below.
>> Manage.c: In function 'Manage_subdevs':
>> Manage.c:1502:5: error: passing argument 3 of 'fstat_is_blkdev' from
>> incompatible pointer type [-Werror]
>> fstat_is_blkdev(tfd, dv->devname, &rdev);
>> ^
>> In file included from Manage.c:25:0:
>> mdadm.h:1446:12: note: expected 'dev_t *' but argument is of type
>> 'long unsigned int *'
>>
>> It was introduced by commit 0a6bff09 (mdadm/util: unify fstat
>> checking blkdev into function). It needs to pass a type 'dev_t'
>> argument to fstat_is_blkdev, but it passes a type 'unsigned
>> long' argument. So use a temporary variable to fix this.
>>
>> Signed-off-by: Xiao Ni <xni@redhat.com>
>> Suggested-by: Paul Menzel <pmenzel@molgen.mpg.de>
>> Suggested-by: Wols Lists <antlists@youngman.org.uk>
>> ---
>> Manage.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> So having a quick look at this, I have to say I don't like the casting
> back and forth. The fact that we carry rdev in an unsigned long in
> Manage_subdevs() seems dubious to me.
>
> Did you look into what the implications would be to change it to a dev_t?
>
> Jes
Hi Jes
Do you mean define rdev as dev_t at first? It will change a lot if we do
so. The argument rdev is passed
to many functions now. Such as Manage_add, Manage_remove,
hot_remove_disk and so on. So I think
it's the reason why we carry rdev in an unsigned long in Manage_subddevs().
Do you have a better solution?
Best Regards
Xiao
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [mdadm PATCH 1/1] Fix a build error
2017-09-30 1:19 ` Xiao Ni
@ 2017-09-30 13:08 ` Jes Sorensen
2017-09-30 14:12 ` Xiao Ni
0 siblings, 1 reply; 7+ messages in thread
From: Jes Sorensen @ 2017-09-30 13:08 UTC (permalink / raw)
To: Xiao Ni, linux-raid; +Cc: ncroxon, pmenzel, antlists
On 09/29/2017 09:19 PM, Xiao Ni wrote:
>
>
> On 09/30/2017 05:47 AM, Jes Sorensen wrote:
>> On 09/19/2017 11:48 PM, Xiao Ni wrote:
>>> On the s390 platform the build fails with the error below.
>>> Manage.c: In function 'Manage_subdevs':
>>> Manage.c:1502:5: error: passing argument 3 of 'fstat_is_blkdev' from
>>> incompatible pointer type [-Werror]
>>> fstat_is_blkdev(tfd, dv->devname, &rdev);
>>> ^
>>> In file included from Manage.c:25:0:
>>> mdadm.h:1446:12: note: expected 'dev_t *' but argument is of type
>>> 'long unsigned int *'
>>>
>>> It was introduced by commit 0a6bff09 (mdadm/util: unify fstat
>>> checking blkdev into function). It needs to pass a type 'dev_t'
>>> argument to fstat_is_blkdev, but it passes a type 'unsigned
>>> long' argument. So use a temporary variable to fix this.
>>>
>>> Signed-off-by: Xiao Ni <xni@redhat.com>
>>> Suggested-by: Paul Menzel <pmenzel@molgen.mpg.de>
>>> Suggested-by: Wols Lists <antlists@youngman.org.uk>
>>> ---
>>> Manage.c | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> So having a quick look at this, I have to say I don't like the casting
>> back and forth. The fact that we carry rdev in an unsigned long in
>> Manage_subdevs() seems dubious to me.
>>
>> Did you look into what the implications would be to change it to a dev_t?
>>
>> Jes
>
> Hi Jes
>
> Do you mean define rdev as dev_t at first? It will change a lot if we do
> so. The argument rdev is passed
> to many functions now. Such as Manage_add, Manage_remove,
> hot_remove_disk and so on. So I think
> it's the reason why we carry rdev in an unsigned long in Manage_subddevs().
>
> Do you have a better solution?
I looked at the other functions and they all carry rdev as a dev_t so I
made it the same in Manage_subdevs(). We may have to clean up some more
functions to take the right input argument, but it looks like the
correct solution to me.
Cheers,
Jes
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [mdadm PATCH 1/1] Fix a build error
2017-09-30 13:08 ` Jes Sorensen
@ 2017-09-30 14:12 ` Xiao Ni
2017-10-01 22:08 ` Jes Sorensen
0 siblings, 1 reply; 7+ messages in thread
From: Xiao Ni @ 2017-09-30 14:12 UTC (permalink / raw)
To: Jes Sorensen, linux-raid; +Cc: ncroxon, pmenzel, antlists
On 09/30/2017 09:08 PM, Jes Sorensen wrote:
> On 09/29/2017 09:19 PM, Xiao Ni wrote:
>>
>>
>> On 09/30/2017 05:47 AM, Jes Sorensen wrote:
>>> On 09/19/2017 11:48 PM, Xiao Ni wrote:
>>>> On the s390 platform the build fails with the error below.
>>>> Manage.c: In function 'Manage_subdevs':
>>>> Manage.c:1502:5: error: passing argument 3 of 'fstat_is_blkdev'
>>>> from incompatible pointer type [-Werror]
>>>> fstat_is_blkdev(tfd, dv->devname, &rdev);
>>>> ^
>>>> In file included from Manage.c:25:0:
>>>> mdadm.h:1446:12: note: expected 'dev_t *' but argument is of type
>>>> 'long unsigned int *'
>>>>
>>>> It was introduced by commit 0a6bff09 (mdadm/util: unify fstat
>>>> checking blkdev into function). It needs to pass a type 'dev_t'
>>>> argument to fstat_is_blkdev, but it passes a type 'unsigned
>>>> long' argument. So use a temporary variable to fix this.
>>>>
>>>> Signed-off-by: Xiao Ni <xni@redhat.com>
>>>> Suggested-by: Paul Menzel <pmenzel@molgen.mpg.de>
>>>> Suggested-by: Wols Lists <antlists@youngman.org.uk>
>>>> ---
>>>> Manage.c | 6 ++++--
>>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> So having a quick look at this, I have to say I don't like the
>>> casting back and forth. The fact that we carry rdev in an unsigned
>>> long in Manage_subdevs() seems dubious to me.
>>>
>>> Did you look into what the implications would be to change it to a
>>> dev_t?
>>>
>>> Jes
>>
>> Hi Jes
>>
>> Do you mean define rdev as dev_t at first? It will change a lot if we
>> do so. The argument rdev is passed
>> to many functions now. Such as Manage_add, Manage_remove,
>> hot_remove_disk and so on. So I think
>> it's the reason why we carry rdev in an unsigned long in
>> Manage_subddevs().
>>
>> Do you have a better solution?
>
> I looked at the other functions and they all carry rdev as a dev_t so
> I made it the same in Manage_subdevs(). We may have to clean up some
> more functions to take the right input argument, but it looks like the
> correct solution to me.
Do you mean some changes like this:
diff --git a/Manage.c b/Manage.c
index 871d342..21536f5 100644
--- a/Manage.c
+++ b/Manage.c
@@ -1367,7 +1367,7 @@ int Manage_subdevs(char *devname, int fd,
}
for (dv = devlist; dv; dv = dv->next) {
- unsigned long rdev = 0; /* device to add/remove etc */
+ dev_t rdev = 0; /* device to add/remove etc */
int rv;
int mj,mn;
diff --git a/sysfs.c b/sysfs.c
index 78d2b52..dc44e38 100644
--- a/sysfs.c
+++ b/sysfs.c
@@ -78,7 +78,7 @@ int sysfs_open(char *devnm, char *devname, char *attr)
return fd;
}
-void sysfs_init_dev(struct mdinfo *mdi, unsigned long devid)
+void sysfs_init_dev(struct mdinfo *mdi, dev_t devid)
{
snprintf(mdi->sys_name,
sizeof(mdi->sys_name), "dev-%s", devid2kname(devid));
We carry rdev as a dev_t and then clean up other functions. If it's ok,
I'll send a new patch.
Best Regards
Xiao
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [mdadm PATCH 1/1] Fix a build error
2017-09-30 14:12 ` Xiao Ni
@ 2017-10-01 22:08 ` Jes Sorensen
2017-10-02 9:12 ` Xiao Ni
0 siblings, 1 reply; 7+ messages in thread
From: Jes Sorensen @ 2017-10-01 22:08 UTC (permalink / raw)
To: Xiao Ni, linux-raid; +Cc: ncroxon, pmenzel, antlists
On 09/30/2017 10:12 AM, Xiao Ni wrote:
>
>
> On 09/30/2017 09:08 PM, Jes Sorensen wrote:
>> On 09/29/2017 09:19 PM, Xiao Ni wrote:
>>> Do you have a better solution?
>>
>> I looked at the other functions and they all carry rdev as a dev_t so
>> I made it the same in Manage_subdevs(). We may have to clean up some
>> more functions to take the right input argument, but it looks like the
>> correct solution to me.
>
> Do you mean some changes like this:
>
> diff --git a/Manage.c b/Manage.c
> index 871d342..21536f5 100644
> --- a/Manage.c
> +++ b/Manage.c
> @@ -1367,7 +1367,7 @@ int Manage_subdevs(char *devname, int fd,
> }
>
> for (dv = devlist; dv; dv = dv->next) {
> - unsigned long rdev = 0; /* device to add/remove etc */
> + dev_t rdev = 0; /* device to add/remove etc */
> int rv;
> int mj,mn;
>
> diff --git a/sysfs.c b/sysfs.c
> index 78d2b52..dc44e38 100644
> --- a/sysfs.c
> +++ b/sysfs.c
> @@ -78,7 +78,7 @@ int sysfs_open(char *devnm, char *devname, char *attr)
> return fd;
> }
>
> -void sysfs_init_dev(struct mdinfo *mdi, unsigned long devid)
> +void sysfs_init_dev(struct mdinfo *mdi, dev_t devid)
> {
> snprintf(mdi->sys_name,
> sizeof(mdi->sys_name), "dev-%s", devid2kname(devid));
>
>
> We carry rdev as a dev_t and then clean up other functions. If it's ok,
> I'll send a new patch.
>
> Best Regards
> Xiao
Yes,
But check latest git first, I already pushed in some of the changes.
Cheers,
Jes
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [mdadm PATCH 1/1] Fix a build error
2017-10-01 22:08 ` Jes Sorensen
@ 2017-10-02 9:12 ` Xiao Ni
0 siblings, 0 replies; 7+ messages in thread
From: Xiao Ni @ 2017-10-02 9:12 UTC (permalink / raw)
To: Jes Sorensen, linux-raid; +Cc: ncroxon, pmenzel, antlists
On 10/02/2017 06:08 AM, Jes Sorensen wrote:
> On 09/30/2017 10:12 AM, Xiao Ni wrote:
>>
>>
>> On 09/30/2017 09:08 PM, Jes Sorensen wrote:
>>> On 09/29/2017 09:19 PM, Xiao Ni wrote:
>>>> Do you have a better solution?
>>>
>>> I looked at the other functions and they all carry rdev as a dev_t
>>> so I made it the same in Manage_subdevs(). We may have to clean up
>>> some more functions to take the right input argument, but it looks
>>> like the correct solution to me.
>>
>> Do you mean some changes like this:
>>
>> diff --git a/Manage.c b/Manage.c
>> index 871d342..21536f5 100644
>> --- a/Manage.c
>> +++ b/Manage.c
>> @@ -1367,7 +1367,7 @@ int Manage_subdevs(char *devname, int fd,
>> }
>>
>> for (dv = devlist; dv; dv = dv->next) {
>> - unsigned long rdev = 0; /* device to add/remove etc */
>> + dev_t rdev = 0; /* device to add/remove etc */
>> int rv;
>> int mj,mn;
>>
>> diff --git a/sysfs.c b/sysfs.c
>> index 78d2b52..dc44e38 100644
>> --- a/sysfs.c
>> +++ b/sysfs.c
>> @@ -78,7 +78,7 @@ int sysfs_open(char *devnm, char *devname, char *attr)
>> return fd;
>> }
>>
>> -void sysfs_init_dev(struct mdinfo *mdi, unsigned long devid)
>> +void sysfs_init_dev(struct mdinfo *mdi, dev_t devid)
>> {
>> snprintf(mdi->sys_name,
>> sizeof(mdi->sys_name), "dev-%s", devid2kname(devid));
>>
>>
>> We carry rdev as a dev_t and then clean up other functions. If it's ok,
>> I'll send a new patch.
>>
>> Best Regards
>> Xiao
>
> Yes,
>
> But check latest git first, I already pushed in some of the changes.
>
> Cheers,
> Jes
>
Hi Jes
I checked the patches you pushed recent. I backport them and the problem
can be fixed already.
Thanks for the help.
Best Regards
Xiao
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-10-02 9:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-20 3:48 [mdadm PATCH 1/1] Fix a build error Xiao Ni
2017-09-29 21:47 ` Jes Sorensen
2017-09-30 1:19 ` Xiao Ni
2017-09-30 13:08 ` Jes Sorensen
2017-09-30 14:12 ` Xiao Ni
2017-10-01 22:08 ` Jes Sorensen
2017-10-02 9:12 ` Xiao Ni
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.