On Wed, Nov 08, 2023 at 05:56:42PM +0000, Martin Wilck wrote:
> On Thu, 2023-11-02 at 18:15 -0400, Benjamin Marzinski wrote:
> > When resizing a multipath device, make sure that all the paths have
> > been updated to the new size first.
> >
> > Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
> > ---
> > multipathd/cli_handlers.c | 30 +++++++++++++++++-------------
> > 1 file changed, 17 insertions(+), 13 deletions(-)
> >
> > diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
> > index c9addfbb..ffea193d 100644
> > --- a/multipathd/cli_handlers.c
> > +++ b/multipathd/cli_handlers.c
> > @@ -845,9 +845,11 @@ cli_resize(void *v, struct strbuf *reply, void
> > *data)
> > char * mapname = get_keyparam(v, KEY_MAP);
> > struct multipath *mpp;
> > int minor;
> > - unsigned long long size;
> > + unsigned long long size = 0;
> > struct pathgroup *pgp;
> > struct path *pp;
> > + unsigned int i, j;
> > + bool mismatch = false;
> >
> > mapname = convert_dev(mapname, 0);
> > condlog(2, "%s: resize map (operator)", mapname);
> > @@ -867,21 +869,23 @@ cli_resize(void *v, struct strbuf *reply, void
> > *data)
> > return 1;
> > }
> >
> > - pgp = VECTOR_SLOT(mpp->pg, 0);
> > -
> > - if (!pgp){
> > - condlog(0, "%s: couldn't get path group. cannot
> > resize",
> > - mapname);
> > - return 1;
> > + vector_foreach_slot(mpp->pg, pgp, i) {
> > + vector_foreach_slot (pgp->paths, pp, j) {
> > + if (sysfs_get_size(pp, &pp->size) != 0)
> > + continue;
> > + if (!size)
> > + size = pp->size;
>
> In the previous patch, you assume that if sysfs_get_size() fails, the
> size is unchanged. The "continue" doesn't make much sense to me: you
> should make the same assumption here, i.e. set size = pp->size even in
> the failure case. Otherwise you might miss a size mismatch.
>
> Altenatively, you could error out immediately if sysfs_get_size() fails
> for any path.
I figured that paths for which we can't get a size shouldn't stop a
resize if all the other paths have changed size, but I admit that isn't
really consistent. I'll change it.
-Ben
> Martin
>
>
> > + else if (pp->size != size)
> > + mismatch = true;
> > + }
> > }
> > - pp = VECTOR_SLOT(pgp->paths, 0);
> > -
> > - if (!pp){
> > - condlog(0, "%s: couldn't get path. cannot resize",
> > mapname);
> > + if (!size) {
> > + condlog(0, "%s: couldn't get size from sysfs. cannot
> > resize",
> > + mapname);
> > return 1;
> > }
> > - if (!pp->udev || sysfs_get_size(pp, &size)) {
> > - condlog(0, "%s: couldn't get size for sysfs. cannot
> > resize",
> > + if (mismatch) {
> > + condlog(0, "%s: path size not consistent. cannot
> > resize",
> > mapname);
> > return 1;
> > }
