Re: [PATCH v3 16/33] tools/libs/light: add backend type for 9pfs PV devices

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, Jan 31, 2024 at 04:18:43PM +0100, Jürgen Groß wrote:
> On 12.01.24 17:55, Anthony PERARD wrote:
> > On Thu, Jan 04, 2024 at 10:00:38AM +0100, Juergen Gross wrote:
> > > +static int xen9pfsd_spawn(libxl__egc *egc, uint32_t domid, 
> > > libxl_device_p9 *p9,
> > > +                         libxl__ao_device *aodev)
> > > +{
> > > +    STATE_AO_GC(aodev->ao);
> > > +    struct libxl__aop9_state *aop9;
> > > +    int rc;
> > > +    char *args[] = { "xen-9pfsd", NULL };
> > > +    char *path = GCSPRINTF("/local/domain/%u/libxl/xen-9pfs",
> > > +                           p9->backend_domid);
> > > +
> > > +    if (p9->type != LIBXL_P9_TYPE_XEN_9PFSD ||
> > > +        libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/state", path)))
> > 
> > I feel like this check and this function might not work as expected.
> > What happen if we try to add more than one 9pfs "device"? libxl I think
> > is going to try to start several xen-9pfs daemon before the first one
> > have had time to write the "*/state" path.
> 
> I don't think so. The path is specific for the _backend_ domid.
> 
> > What about two different libxl process trying to spawn that daemon? Is
> > xen-9pfs going to behave well and have one giveup? But that would
> > probably mean that libxl is going to have an error due to the process
> > exiting early, maybe.
> 
> I think I need to handle this case gracefully in the daemon by exiting with
> a 0 exit code.

As long as the scenario is handle somehow, I'm happy.

> > Could you reorder the file, to make it easier to follow the code of
> > the async style? "xen9pfsd_spawn()" should be first, followed by
> > _confirm() _failed and _detached() and finally xen9pfsd_spawn_outcome().
> 
> This would need to add some forward declarations. If you really are fine with
> that, I can do the reordering.

What's wrong with forward declarations?

When you write a bunch of function that are supposed to be called one by
one, but the next one to be called is above the current function in the
source file, isn't that a bit like top-posting?
Anyway, writing function in the source code in a chronological order,
with forward declaration, is part of the libxl CODING_STYLE.

Cheers,

-- 
Anthony PERARD