Re: [PULL 05/27] hw/xen: Watches on XenStore transactions

[Peter Maydell]

On Tue, 2 May 2023 at 18:08, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 7 Mar 2023 at 18:27, David Woodhouse <dwmw2@infradead.org> wrote:
> >
> > From: David Woodhouse <dwmw@amazon.co.uk>
> >
> > Firing watches on the nodes that still exist is relatively easy; just
> > walk the tree and look at the nodes with refcount of one.
> >
> > Firing watches on *deleted* nodes is more fun. We add 'modified_in_tx'
> > and 'deleted_in_tx' flags to each node. Nodes with those flags cannot
> > be shared, as they will always be unique to the transaction in which
> > they were created.
> >
> > When xs_node_walk would need to *create* a node as scaffolding and it
> > encounters a deleted_in_tx node, it can resurrect it simply by clearing
> > its deleted_in_tx flag. If that node originally had any *data*, they're
> > gone, and the modified_in_tx flag will have been set when it was first
> > deleted.
> >
> > We then attempt to send appropriate watches when the transaction is
> > committed, properly delete the deleted_in_tx nodes, and remove the
> > modified_in_tx flag from the others.
> >
> > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> > Reviewed-by: Paul Durrant <paul@xen.org>
>
> Hi; Coverity's "is there missing error handling?"
> heuristic fired for a change in this code (CID 1508359):
>
> >  static int transaction_commit(XenstoreImplState *s, XsTransaction *tx)
> >  {
> > +    struct walk_op op;
> > +    XsNode **n;
> > +
> >      if (s->root_tx != tx->base_tx) {
> >          return EAGAIN;
> >      }
> > @@ -720,10 +861,18 @@ static int transaction_commit(XenstoreImplState *s, 
> > XsTransaction *tx)
> >      s->root_tx = tx->tx_id;
> >      s->nr_nodes = tx->nr_nodes;
> >
> > +    init_walk_op(s, &op, XBT_NULL, tx->dom_id, "/", &n);
>
> This is the only call to init_walk_op() which ignores its
> return value. Intentional, or missing error handling?

Hi -- I was going through the unclassified Coverity issues
again today, and this one's still on the list. Is this a
bug, or intentional?

thanks
-- PMM