Re: [PATCH v2 26/29] tools/xenstored: mount 9pfs device in stubdom

[Juergen Gross <jgross@xxxxxxxx>]

On 13.11.23 23:09, Julien Grall wrote:
> Hi Juergen,
>
> On 10/11/2023 16:08, Juergen Gross wrote:
> > diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
> > index 162b87b460..4263c1360f 100644
> > --- a/tools/xenstored/domain.c
> > +++ b/tools/xenstored/domain.c
> > @@ -1236,6 +1236,8 @@ void stubdom_init(void)
> >           barf_perror("Failed to initialize stubdom");
> >       xenevtchn_notify(xce_handle, stubdom->port);
> > +
> > +    mount_9pfs();
> >   #endif
> >   }
> > diff --git a/tools/xenstored/minios.c b/tools/xenstored/minios.c
> > index 202d70387a..fddbede869 100644
> > --- a/tools/xenstored/minios.c
> > +++ b/tools/xenstored/minios.c
> > @@ -19,8 +19,16 @@
> >   #include <sys/mman.h>
> >   #include "core.h"
> >   #include <xen/grant_table.h>
> > +#include <mini-os/9pfront.h>
> >   #include <mini-os/events.h>
> >   #include <mini-os/gnttab.h>
> > +#include <mini-os/sched.h>
> > +#include <mini-os/xenbus.h>
> > +#include <mini-os/xmalloc.h>
> > +
> > +#define P9_STATE_PATH    "device/9pfs/0/state"
> > +
> > +static void *p9_device;
> >   void write_pidfile(const char *pidfile)
> >   {
> > @@ -62,3 +70,31 @@ void early_init(void)
> >       if (stub_domid == DOMID_INVALID)
> >           barf("could not get own domid");
> >   }
> > +
> > +static void mount_thread(void *p)
> > +{
> > +    xenbus_event_queue events = NULL;
> > +    char *err;
> > +    char *dummy;
> > +
> > +    free(xenbus_watch_path_token(XBT_NIL, P9_STATE_PATH, "9pfs", &events));
> AFAICT, xenbus_watch_path_token() can fail. I agree this is unlikely, but if it 
> fails, then it would be useful to get some logs. Otherwise...
> > +
> > +    for (;;) {
> ... this loop would be infinite.
Okay, will add logging.

> > +        xenbus_wait_for_watch(&events);
> > +        err = xenbus_read(XBT_NIL, P9_STATE_PATH, &dummy);
> Can you explain why don't care about the value of the node?
I only care about the presence of the "state" node. All real state changes
will be handled in init_9pfront().

> > +        if (!err)
> > +            break;
> > +        free(err);
> > +    }
> > +
> > +    free(dummy);
> > +
> > +    free(xenbus_unwatch_path_token(XBT_NIL, P9_STATE_PATH, "9pfs"));
> xenbus_unwatch_path_token() could technically fails. It would be helpful to 
> print a message.
I can add that, but do we really care? This is a common pattern in Mini-OS.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature