Re: [RFC PATCH v1 03/25] hw/xen: Implement XenStore watches

[David Woodhouse <dwmw2@xxxxxxxxxxxxx>]

On Tue, 2023-03-07 at 11:29 +0000, Paul Durrant wrote:
> 
> I think you could stash a tail pointer here...
> 
> > +    }
> > +
> > +    if (dom_id && s->nr_domu_watches >= XS_MAX_WATCHES) {
> > +        return E2BIG;
> > +    }
> > +
> > +    w = g_new0(XsWatch, 1);
> > +    w->token = g_strdup(token);
> > +    w->cb = fn;
> > +    w->cb_opaque = opaque;
> > +    w->dom_id = dom_id;
> > +    w->rel_prefix = strlen(abspath) - strlen(path);
> > +
> > +    l = g_hash_table_lookup(s->watches, abspath);
> 
> ... to avoid the duplicate hash lookup here.

Good point. The EEXIST check was added later as I was reviewing and
comparing with the real xenstored, so I didn't spot that. Thanks.

--- a/hw/i386/kvm/xenstore_impl.c
+++ b/hw/i386/kvm/xenstore_impl.c
@@ -673,7 +673,7 @@ int xs_impl_watch(XenstoreImplState *s, unsigned int 
dom_id, const char *path,
     }
 
     /* Check for duplicates */
-    w = g_hash_table_lookup(s->watches, abspath);
+    l = w = g_hash_table_lookup(s->watches, abspath);
     while (w) {
         if (!g_strcmp0(token, w->token) &&  opaque == w->cb_opaque &&
             fn == w->cb && dom_id == w->dom_id) {
@@ -693,7 +693,7 @@ int xs_impl_watch(XenstoreImplState *s, unsigned int 
dom_id, const char *path,
     w->dom_id = dom_id;
     w->rel_prefix = strlen(abspath) - strlen(path);
 
-    l = g_hash_table_lookup(s->watches, abspath);
+    /* l was looked up above when checking for duplicates */
     if (l) {
         w->next = l->next;
         l->next = w;

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