| Message ID | 20240917155858.959835-1-bmarzins@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | christophe varoqui |
| Headers | show |
| Series | multipathd: checker port_state before setting it. | expand |
Hi Benjamin, Changes looks good . Thanks for the fix. Regards, Muneendra. -----Original Message----- From: Benjamin Marzinski <bmarzins@redhat.com> Sent: Tuesday, September 17, 2024 9:29 PM To: Christophe Varoqui <christophe.varoqui@opensvc.com> Cc: device-mapper development <dm-devel@lists.linux.dev>; Martin Wilck <Martin.Wilck@suse.com>; Muneendra Kumar <muneendra.kumar@broadcom.com> Subject: [PATCH] multipathd: checker port_state before setting it. If the rport port_state is already Marginal, trying to set it to Marginal causes an error like the following: multipathd[365376]: /sys/devices/pci0000:c0/0000:c0:01.1/0000:c4:00.0/host0/rport-0:0-5/fc_rem ote_ports/rport-0:0-5: failed to set port_state to marginal: Invalid argument To avoid causing this confusing error message, check if the port_state is already marginal before trying to set it. Cc: Muneendra Kumar <muneendra.kumar@broadcom.com> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> --- multipathd/fpin_handlers.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/multipathd/fpin_handlers.c b/multipathd/fpin_handlers.c index be087ca0..6b56f9b7 100644 --- a/multipathd/fpin_handlers.c +++ b/multipathd/fpin_handlers.c @@ -169,9 +169,14 @@ fpin_els_add_li_frame(struct fc_nl_event *fc_event) /*Sets the rport port_state to marginal*/ static void fpin_set_rport_marginal(struct udev_device *rport_dev) { + char old_value[20]; /* match kernel show_fc_rport_port_state() size */ static const char marginal[] = "Marginal"; ssize_t ret; + ret = sysfs_attr_get_value(rport_dev, "port_state", + old_value, sizeof(old_value)); + if (ret == sizeof(marginal) - 1 && strcmp(old_value, marginal) == 0) + return; ret = sysfs_attr_set_value(rport_dev, "port_state", marginal, sizeof(marginal) - 1); if (ret != sizeof(marginal) - 1) -- 2.45.0
On Tue, Sep 17, 2024 at 11:58:58AM -0400, Benjamin Marzinski wrote: > If the rport port_state is already Marginal, trying to set it to > Marginal causes an error like the following: > > multipathd[365376]: /sys/devices/pci0000:c0/0000:c0:01.1/0000:c4:00.0/host0/rport-0:0-5/fc_remote_ports/rport-0:0-5: failed to set port_state to marginal: Invalid argument > > To avoid causing this confusing error message, check if the port_state > is already marginal before trying to set it. > > Cc: Muneendra Kumar <muneendra.kumar@broadcom.com> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Ping. My in-kernel fix for this was accepted, but it would be nice to have this work on older kernels as well. -Ben > --- > multipathd/fpin_handlers.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/multipathd/fpin_handlers.c b/multipathd/fpin_handlers.c > index be087ca0..6b56f9b7 100644 > --- a/multipathd/fpin_handlers.c > +++ b/multipathd/fpin_handlers.c > @@ -169,9 +169,14 @@ fpin_els_add_li_frame(struct fc_nl_event *fc_event) > /*Sets the rport port_state to marginal*/ > static void fpin_set_rport_marginal(struct udev_device *rport_dev) > { > + char old_value[20]; /* match kernel show_fc_rport_port_state() size */ > static const char marginal[] = "Marginal"; > ssize_t ret; > > + ret = sysfs_attr_get_value(rport_dev, "port_state", > + old_value, sizeof(old_value)); > + if (ret == sizeof(marginal) - 1 && strcmp(old_value, marginal) == 0) > + return; > ret = sysfs_attr_set_value(rport_dev, "port_state", > marginal, sizeof(marginal) - 1); > if (ret != sizeof(marginal) - 1) > -- > 2.45.0 >
On Tue, 2024-09-17 at 11:58 -0400, Benjamin Marzinski wrote: > If the rport port_state is already Marginal, trying to set it to > Marginal causes an error like the following: > > multipathd[365376]: > /sys/devices/pci0000:c0/0000:c0:01.1/0000:c4:00.0/host0/rport-0:0- > 5/fc_remote_ports/rport-0:0-5: failed to set port_state to marginal: > Invalid argument > > To avoid causing this confusing error message, check if the > port_state > is already marginal before trying to set it. > > Cc: Muneendra Kumar <muneendra.kumar@broadcom.com> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> Reviewed-by: Martin Wilck <mwilck@suse.com>
On Tue, 2024-11-05 at 11:22 -0500, Benjamin Marzinski wrote: > On Tue, Sep 17, 2024 at 11:58:58AM -0400, Benjamin Marzinski wrote: > > If the rport port_state is already Marginal, trying to set it to > > Marginal causes an error like the following: > > > > multipathd[365376]: > > /sys/devices/pci0000:c0/0000:c0:01.1/0000:c4:00.0/host0/rport-0:0- > > 5/fc_remote_ports/rport-0:0-5: failed to set port_state to > > marginal: Invalid argument > > > > To avoid causing this confusing error message, check if the > > port_state > > is already marginal before trying to set it. > > > > Cc: Muneendra Kumar <muneendra.kumar@broadcom.com> > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > > Ping. My in-kernel fix for this was accepted, but it would be nice to > have this work on older kernels as well. Applied to "queue" now. Sorry for the delay. Martin
diff --git a/multipathd/fpin_handlers.c b/multipathd/fpin_handlers.c index be087ca0..6b56f9b7 100644 --- a/multipathd/fpin_handlers.c +++ b/multipathd/fpin_handlers.c @@ -169,9 +169,14 @@ fpin_els_add_li_frame(struct fc_nl_event *fc_event) /*Sets the rport port_state to marginal*/ static void fpin_set_rport_marginal(struct udev_device *rport_dev) { + char old_value[20]; /* match kernel show_fc_rport_port_state() size */ static const char marginal[] = "Marginal"; ssize_t ret; + ret = sysfs_attr_get_value(rport_dev, "port_state", + old_value, sizeof(old_value)); + if (ret == sizeof(marginal) - 1 && strcmp(old_value, marginal) == 0) + return; ret = sysfs_attr_set_value(rport_dev, "port_state", marginal, sizeof(marginal) - 1); if (ret != sizeof(marginal) - 1)
If the rport port_state is already Marginal, trying to set it to Marginal causes an error like the following: multipathd[365376]: /sys/devices/pci0000:c0/0000:c0:01.1/0000:c4:00.0/host0/rport-0:0-5/fc_remote_ports/rport-0:0-5: failed to set port_state to marginal: Invalid argument To avoid causing this confusing error message, check if the port_state is already marginal before trying to set it. Cc: Muneendra Kumar <muneendra.kumar@broadcom.com> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> --- multipathd/fpin_handlers.c | 5 +++++ 1 file changed, 5 insertions(+)