| Message ID | 1648610498-28440-1-git-send-email-bmarzins@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | christophe varoqui |
| Headers | show |
| Series | multipath: return failure on an invalid remove command | expand |
On Tue, 2022-03-29 at 22:21 -0500, Benjamin Marzinski wrote: > When "multipath -f" is run on a device that doesn't exist or isn't a > multipath device, the command will not remove any device, but it will > still return success. Multiple functions rely on _dm_flush_map() > returning success when called with name that doesn't match any > multipath device. So before calling _dm_flush_map(), call > dm_is_mpath(), > to check if the device exists and is a multipath device, and return > failure if it's not. > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > --- > multipath/main.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/multipath/main.c b/multipath/main.c > index d09f62db..e865309d 100644 > --- a/multipath/main.c > +++ b/multipath/main.c > @@ -1060,6 +1060,11 @@ main (int argc, char *argv[]) > if (retries < 0) > retries = conf->remove_retries; > if (cmd == CMD_FLUSH_ONE) { > + if (dm_is_mpath(dev) != 1) { > + condlog(0, "failed removing \"%s\"", dev); Could we provide a more meaningful error message here? (As you're the native speaker - shouldn't it rather be "failed to remove"?) Otherwise, ack. Martin > + r = RTVL_FAIL; > + goto out; > + } > r = dm_suspend_and_flush_map(dev, retries) ? > RTVL_FAIL : RTVL_OK; > goto out; -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
On Wed, Mar 30, 2022 at 12:50:00PM +0000, Martin Wilck wrote: > On Tue, 2022-03-29 at 22:21 -0500, Benjamin Marzinski wrote: > > When "multipath -f" is run on a device that doesn't exist or isn't a > > multipath device, the command will not remove any device, but it will > > still return success. Multiple functions rely on _dm_flush_map() > > returning success when called with name that doesn't match any > > multipath device. So before calling _dm_flush_map(), call > > dm_is_mpath(), > > to check if the device exists and is a multipath device, and return > > failure if it's not. > > > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> > > --- > > multipath/main.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/multipath/main.c b/multipath/main.c > > index d09f62db..e865309d 100644 > > --- a/multipath/main.c > > +++ b/multipath/main.c > > @@ -1060,6 +1060,11 @@ main (int argc, char *argv[]) > > if (retries < 0) > > retries = conf->remove_retries; > > if (cmd == CMD_FLUSH_ONE) { > > + if (dm_is_mpath(dev) != 1) { > > + condlog(0, "failed removing \"%s\"", dev); > > Could we provide a more meaningful error message here? > (As you're the native speaker - shouldn't it rather be "failed to > remove"?) Sure. I'll clean that up. -Ben > > Otherwise, ack. > > Martin > > > > + r = RTVL_FAIL; > > + goto out; > > + } > > r = dm_suspend_and_flush_map(dev, retries) ? > > RTVL_FAIL : RTVL_OK; > > goto out; -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
diff --git a/multipath/main.c b/multipath/main.c index d09f62db..e865309d 100644 --- a/multipath/main.c +++ b/multipath/main.c @@ -1060,6 +1060,11 @@ main (int argc, char *argv[]) if (retries < 0) retries = conf->remove_retries; if (cmd == CMD_FLUSH_ONE) { + if (dm_is_mpath(dev) != 1) { + condlog(0, "failed removing \"%s\"", dev); + r = RTVL_FAIL; + goto out; + } r = dm_suspend_and_flush_map(dev, retries) ? RTVL_FAIL : RTVL_OK; goto out;
When "multipath -f" is run on a device that doesn't exist or isn't a multipath device, the command will not remove any device, but it will still return success. Multiple functions rely on _dm_flush_map() returning success when called with name that doesn't match any multipath device. So before calling _dm_flush_map(), call dm_is_mpath(), to check if the device exists and is a multipath device, and return failure if it's not. Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com> --- multipath/main.c | 5 +++++ 1 file changed, 5 insertions(+)