Re: [Xen-devel] [PATCH] tools/xenstore: Do not abort xenstore-ls if a node disappears while iterating

[David Woodhouse <dwmw2@xxxxxxxxxxxxx>]

On Mon, 2019-03-04 at 15:51 +0100, Juergen Gross wrote:
> On 04/03/2019 15:31, David Woodhouse wrote:
> > On Mon, 2019-03-04 at 14:18 +0000, Wei Liu wrote:
> > > CC Ian as well.
> > > 
> > > It would be better if you run ./scripts/get_maintainers.pl on
> > > your
> > > patches in the future to CC the correct people.
> > 
> > Will do; thanks.
> > 
> > > On Fri, Mar 01, 2019 at 12:16:56PM +0000, David Woodhouse wrote:
> > > > From: David Woodhouse <dwmw@xxxxxxxxxxxx>
> > > > 
> > > > When recursing, a node sometimes disappears. Deal with it and
> > > > move on
> > > > instead of aborting and failing to print the rest of what was
> > > > requested.
> > > >     
> > > > Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
> > > > ---
> > > > And thus did an extremely sporadic "not going to delete that
> > > > device
> > > > because it already doesn't exist" failure mode become painfully
> > > > obvious
> > > > in retrospect...
> > > > 
> > > > diff --git a/tools/xenstore/xenstore_client.c
> > > > b/tools/xenstore/xenstore_client.c
> > > > index 3afc630ab8..c089d60a2a 100644
> > > > --- a/tools/xenstore/xenstore_client.c
> > > > +++ b/tools/xenstore/xenstore_client.c
> > > > @@ -153,8 +153,13 @@ static void do_ls(struct xs_handle *h,
> > > > char
> > > > *path,
> > > > int cur_depth, int show_perms
> > > >        err(1, "malloc in do_ls");
> > > >  
> > > >      e = xs_directory(h, XBT_NULL, path, &num);
> > > > -    if (e == NULL)
> > > > -        err(1, "xs_directory (%s)", path);
> > > > +    if (e == NULL) {
> > > > +        if (!cur_depth)
> > > > +            err(1, "xs_directory (%s)", path);
> > > > +
> > > > +        /* If a node disappears while recursing, silently move
> > > > on.
> > > > */
> > > > +        num = 0;
> > > > +    }
> > > 
> > > Can you check if the errno is ENOENT? I would rather not ignore
> > > other
> > > types of errors if possible.
> > 
> > Under what circumstances is it correct for xenstore-ls to abort
> > immediately and not attempt to print anything more?
> > 
> > I'm sure there are circumstances where the world is so hosed that
> > it'll
> > keep getting errors and *fail* to print anything more. But to
> > deliberately bail out? Should that really be restricted to ENOENT?
> 
> EACCES seems to be another candidate for trying to continue.
> 
> EINVAL, ENOMEM and EIO should never occur, so aborting the operation
> would be okay IMO.

If you get one of those errors for a given path, but then everything
else afterwards works fine.... do you really *want* to abort and not
print the later ones? I agree it would be OK — at least tolerable — to
abort. But is it really the most desired outcome?

Attachment: smime.p7s
Description: S/MIME cryptographic signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel