* [PATCH RESEND] Monitor: fix for regression with container devices @ 2015-02-09 10:13 Artur Paszkiewicz 2015-02-11 4:38 ` NeilBrown 0 siblings, 1 reply; 3+ messages in thread From: Artur Paszkiewicz @ 2015-02-09 10:13 UTC (permalink / raw) To: neilb; +Cc: linux-raid, pawel.baldysiak, Artur Paszkiewicz This patch fixes 2 problems introduced by commit 9a518d8: not closing a file descriptor and ignoring container devices. Array state is always "inactive" for containers, so we make sure that the device is not a container by reading also the "level" sysfs entry. Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com> Reviewed-by: Pawel Baldysiak <pawel.baldysiak@intel.com> --- Monitor.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/Monitor.c b/Monitor.c index 971d2ec..66d67ba 100644 --- a/Monitor.c +++ b/Monitor.c @@ -483,11 +483,17 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat, strncmp(buf,"inact",5) == 0) { if (fd >= 0) close(fd); - if (!st->err) - alert("DeviceDisappeared", dev, NULL, ainfo); - st->err++; - return 0; + fd = sysfs_open(st->devnm, NULL, "level"); + if (fd < 0 || read(fd, buf, 10) != 0) { + if (fd >= 0) + close(fd); + if (!st->err) + alert("DeviceDisappeared", dev, NULL, ainfo); + st->err++; + return 0; + } } + close(fd); } fd = open(dev, O_RDONLY); if (fd < 0) { -- 2.1.4 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH RESEND] Monitor: fix for regression with container devices 2015-02-09 10:13 [PATCH RESEND] Monitor: fix for regression with container devices Artur Paszkiewicz @ 2015-02-11 4:38 ` NeilBrown 2015-02-13 15:29 ` Artur Paszkiewicz 0 siblings, 1 reply; 3+ messages in thread From: NeilBrown @ 2015-02-11 4:38 UTC (permalink / raw) To: Artur Paszkiewicz; +Cc: linux-raid, pawel.baldysiak [-- Attachment #1: Type: text/plain, Size: 2426 bytes --] On Mon, 9 Feb 2015 11:13:50 +0100 Artur Paszkiewicz <artur.paszkiewicz@intel.com> wrote: > This patch fixes 2 problems introduced by commit 9a518d8: not closing a > file descriptor and ignoring container devices. Array state is always > "inactive" for containers, so we make sure that the device is not a > container by reading also the "level" sysfs entry. > > Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com> > Reviewed-by: Pawel Baldysiak <pawel.baldysiak@intel.com> > --- > Monitor.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/Monitor.c b/Monitor.c > index 971d2ec..66d67ba 100644 > --- a/Monitor.c > +++ b/Monitor.c > @@ -483,11 +483,17 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat, > strncmp(buf,"inact",5) == 0) { > if (fd >= 0) > close(fd); > - if (!st->err) > - alert("DeviceDisappeared", dev, NULL, ainfo); > - st->err++; > - return 0; > + fd = sysfs_open(st->devnm, NULL, "level"); > + if (fd < 0 || read(fd, buf, 10) != 0) { > + if (fd >= 0) > + close(fd); > + if (!st->err) > + alert("DeviceDisappeared", dev, NULL, ainfo); > + st->err++; > + return 0; > + } > } > + close(fd); > } > fd = open(dev, O_RDONLY); > if (fd < 0) { Thanks for the patch. I don't think I agree with the logic of using 'level' though. For the sort of arrays that I need to ignore here, 'level' will be empty. It would make sense to test 'metadata' though. If that starts 'external:', then we don't want to ignore the array. Could you confirm that this works please? Thanks, NeilBrown diff --git a/Monitor.c b/Monitor.c index 971d2ecbea72..6e085cb24993 100644 --- a/Monitor.c +++ b/Monitor.c @@ -483,11 +483,18 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat, strncmp(buf,"inact",5) == 0) { if (fd >= 0) close(fd); - if (!st->err) - alert("DeviceDisappeared", dev, NULL, ainfo); - st->err++; - return 0; + fd = sysfs_open(st->devnm, NULL, "metadata"); + if (fd < 0 || read(fd, buf, 9) != 9 || + strncmp(buf, "external:", 9) != 0) { + if (fd >= 0) + close(fd); + if (!st->err) + alert("DeviceDisappeared", dev, NULL, ainfo); + st->err++; + return 0; + } } + close(fd); } fd = open(dev, O_RDONLY); if (fd < 0) { [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 811 bytes --] ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH RESEND] Monitor: fix for regression with container devices 2015-02-11 4:38 ` NeilBrown @ 2015-02-13 15:29 ` Artur Paszkiewicz 0 siblings, 0 replies; 3+ messages in thread From: Artur Paszkiewicz @ 2015-02-13 15:29 UTC (permalink / raw) To: NeilBrown; +Cc: linux-raid, pawel.baldysiak On 02/11/2015 05:38 AM, NeilBrown wrote: > On Mon, 9 Feb 2015 11:13:50 +0100 Artur Paszkiewicz > <artur.paszkiewicz@intel.com> wrote: > > > This patch fixes 2 problems introduced by commit 9a518d8: not closing a > > file descriptor and ignoring container devices. Array state is always > > "inactive" for containers, so we make sure that the device is not a > > container by reading also the "level" sysfs entry. > > > > Signed-off-by: Artur Paszkiewicz <artur.paszkiewicz@intel.com> > > Reviewed-by: Pawel Baldysiak <pawel.baldysiak@intel.com> > > --- > > Monitor.c | 14 ++++++++++---- > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/Monitor.c b/Monitor.c > > index 971d2ec..66d67ba 100644 > > --- a/Monitor.c > > +++ b/Monitor.c > > @@ -483,11 +483,17 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat, > > strncmp(buf,"inact",5) == 0) { > > if (fd >= 0) > > close(fd); > > - if (!st->err) > > - alert("DeviceDisappeared", dev, NULL, ainfo); > > - st->err++; > > - return 0; > > + fd = sysfs_open(st->devnm, NULL, "level"); > > + if (fd < 0 || read(fd, buf, 10) != 0) { > > + if (fd >= 0) > > + close(fd); > > + if (!st->err) > > + alert("DeviceDisappeared", dev, NULL, ainfo); > > + st->err++; > > + return 0; > > + } > > } > > + close(fd); > > } > > fd = open(dev, O_RDONLY); > > if (fd < 0) { > > Thanks for the patch. > > I don't think I agree with the logic of using 'level' though. > For the sort of arrays that I need to ignore here, 'level' will be empty. > > It would make sense to test 'metadata' though. If that starts 'external:', > then we don't want to ignore the array. > > Could you confirm that this works please? > Hi Neil, I tested your patch. I assume you wanted to use 'metadata_version', because there is no 'metadata' attribute, right? I had also thought about that, but simply looking for 'external:' is not enough to determine that the array is a container - for volumes inside the container it looks like this: 'external:/md127/0'. But I think that the arrays you want to ignore will just have 'none' there, so maybe it can be done like this? diff --git a/Monitor.c b/Monitor.c index 971d2ec..83daf3b 100644 --- a/Monitor.c +++ b/Monitor.c @@ -483,11 +483,18 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat, strncmp(buf,"inact",5) == 0) { if (fd >= 0) close(fd); - if (!st->err) - alert("DeviceDisappeared", dev, NULL, ainfo); - st->err++; - return 0; + fd = sysfs_open(st->devnm, NULL, "metadata_version"); + if (fd < 0 || read(fd, buf, 4) < 0 || + strncmp(buf, "none", 4) == 0) { + if (fd >= 0) + close(fd); + if (!st->err) + alert("DeviceDisappeared", dev, NULL, ainfo); + st->err++; + return 0; + } } + close(fd); } fd = open(dev, O_RDONLY); if (fd < 0) { Thanks, Artur > Thanks, > NeilBrown > > diff --git a/Monitor.c b/Monitor.c > index 971d2ecbea72..6e085cb24993 100644 > --- a/Monitor.c > +++ b/Monitor.c > @@ -483,11 +483,18 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat, > strncmp(buf,"inact",5) == 0) { > if (fd >= 0) > close(fd); > - if (!st->err) > - alert("DeviceDisappeared", dev, NULL, ainfo); > - st->err++; > - return 0; > + fd = sysfs_open(st->devnm, NULL, "metadata"); > + if (fd < 0 || read(fd, buf, 9) != 9 || > + strncmp(buf, "external:", 9) != 0) { > + if (fd >= 0) > + close(fd); > + if (!st->err) > + alert("DeviceDisappeared", dev, NULL, ainfo); > + st->err++; > + return 0; > + } > } > + close(fd); > } > fd = open(dev, O_RDONLY); > if (fd < 0) { > ^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-02-13 15:29 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-02-09 10:13 [PATCH RESEND] Monitor: fix for regression with container devices Artur Paszkiewicz 2015-02-11 4:38 ` NeilBrown 2015-02-13 15:29 ` Artur Paszkiewicz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).