Re: [PATCH 11/15] limpathpersist: Handle changing key corner case

[Benjamin Marzinski <bmarzins@redhat.com>]

On Fri, Jul 11, 2025 at 04:11:46PM +0200, Martin Wilck wrote:
> On Fri, 2025-07-11 at 14:15 +0200, Martin Wilck wrote:
> > On Thu, 2025-07-10 at 14:10 -0400, Benjamin Marzinski wrote:
> > > When you change the reservation key of a registered multipath
> > > device,
> > > some of paths might be down or even deleted since you originally
> > 
> > [...]
> > > 
> > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > 
> > This is not a full review, just feedback about a failing CI pipeline.
> > 
> > > ---
> > >  libmpathpersist/mpath_persist_int.c | 124 ++++++++++++++++++++----
> > > --
> > > --
> > >  1 file changed, 90 insertions(+), 34 deletions(-)
> > > 
> > > diff --git a/libmpathpersist/mpath_persist_int.c
> > > b/libmpathpersist/mpath_persist_int.c
> > > index ad5a4ee7..ca972c2b 100644
> > > --- a/libmpathpersist/mpath_persist_int.c
> > > +++ b/libmpathpersist/mpath_persist_int.c
> > > 
> > > +void preempt_missing_path(struct multipath *mpp, uint8_t *key,
> > > uint8_t *sa_key,
> > > +			  int noisy)
> > > +{
> > > +	uint8_t zero[8] = {0};
> > > +	struct prin_resp resp = {0};
> > 
> > gcc 4.8 (Debian Jessie) dislikes this syntax.
> > 
> > > mpath_persist_int.c:272:9: error: missing braces around initializer
> > > [-Werror=missing-braces]
> > 
> > The same issue appears in patch 7/15 and patch 15/15.
> > It seems to be a gcc bug
> > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119).
> > So we could use #pragma to silence the warning, but I'd like to avoid
> > that, especially because it occurs multiple times.
> > 
> > I don't think we should drop Jessie support just yet.
> > 
> > This works:
> > 
> > 	struct prin_resp resp = {{{0}}};
> 
> It does, but it causes an error with Jessie's clang compiler (clang
> 3.5.0).
>                                   ^
> > mpath_persist_int.c:673:30: error: missing field 'additional_length' initializer [-Werror,-Wmissing-field-initializers]
> >         struct prin_resp resp = {{{0}}};
>  
> Both compilers seem to accept this:
> 
> 	struct prin_resp resp = {{{.prgeneration = 0}}};
> 
> It's getting so awkward that we might as well just use memset.

Yeah. picking a member from one of union types to set to 0 is pretty
awkward. I can repost the patches with memsets.

-Ben

> 
> Martin
>