[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC v2 1/8] xen/hypfs: support fo nested dynamic hypfs nodes
On Fri, Feb 11, 2022 at 02:28:49PM +0100, Juergen Gross wrote: > On 11.02.22 09:16, Oleksii Moisieiev wrote: > > Hi Juergen, > > > > On Thu, Feb 10, 2022 at 08:34:08AM +0100, Juergen Gross wrote: > > > On 08.02.22 19:00, Oleksii Moisieiev wrote: > > > > > > > > > Add new api: > > > > - hypfs_read_dyndir_entry > > > > - hypfs_gen_dyndir_entry > > > > which are the extension of the dynamic hypfs nodes support, presented in > > > > 0b3b53be8cf226d947a79c2535a9efbb2dd7bc38. > > > > This allows nested dynamic nodes to be added. Also input parameter is > > > > hypfs_entry, so properties can also be generated dynamically. > > > > > > > > Generating mixed list of dirs and properties is also supported. > > > > Same as to the dynamic hypfs nodes, this is anchored in percpu pointer, > > > > which can be retriewed on any level of the dynamic entries. > > > > This handle should be allocated on enter() callback and released on > > > > exit() callback. When using nested dynamic dirs and properties handle > > > > should be allocated on the first enter() call and released on the last > > > > exit() call. > > > > > > > > Signed-off-by: Oleksii Moisieiev <oleksii_moisieiev@xxxxxxxx> > > ... > > > > > diff --git a/xen/include/xen/hypfs.h b/xen/include/xen/hypfs.h > > > > index e9d4c2555b..5d2728b963 100644 > > > > --- a/xen/include/xen/hypfs.h > > > > +++ b/xen/include/xen/hypfs.h > > > > @@ -79,8 +79,8 @@ struct hypfs_entry_dir { > > > > struct hypfs_dyndir_id { > > > > > > Please rename to struct hypfs_dyndir. > > > > Ok, thanks. > > > > > > > > > struct hypfs_entry_dir dir; /* Modified copy of > > > > template. */ > > > > struct hypfs_funcs funcs; /* Dynamic functions. */ > > > > - const struct hypfs_entry_dir *template; /* Template used. */ > > > > -#define HYPFS_DYNDIR_ID_NAMELEN 12 > > > > + const struct hypfs_entry *template; /* Template used. */ > > > > +#define HYPFS_DYNDIR_ID_NAMELEN 32 > > > > char name[HYPFS_DYNDIR_ID_NAMELEN]; /* Name of hypfs entry. > > > > */ > > > > unsigned int id; /* Numerical id. */ > > > > > > What about the following change instead: > > > > > > - const struct hypfs_entry_dir *template; /* Template used. */ > > > -#define HYPFS_DYNDIR_ID_NAMELEN 12 > > > - char name[HYPFS_DYNDIR_ID_NAMELEN]; /* Name of hypfs entry. */ > > > - > > > - unsigned int id; /* Numerical id. */ > > > - void *data; /* Data associated with id. > > > */ > > > + const struct hypfs_entry *template; /* Template used. */ > > > + union { > > > +#define HYPFS_DYNDIR_NAMELEN 32 > > > + char name[HYPFS_DYNDIR_NAMELEN]; /* Name of hypfs entry. */ > > > + struct { > > > +#define HYPFS_DYNDIR_ID_NAMELEN 12 > > > + char name[HYPFS_DYNDIR_ID_NAMELEN]; /* Name of id entry. */ > > > + unsigned int id; /* Numerical id. */ > > > + } id; > > > + }; > > > + void*data; /* Data associated with entry. */ > > > > > > > I'm not sure I see the benefit from this union. The only one I see is > > that struct hypds_dyndir will be smaller by sizeof(unsigned int). > > Could you explain please? > > My main concern is that it is not obvious to a user that the > numerical id is needed only for a special case. Putting it into > the union makes this much more clear. > This make sense. I'll make this union. Thanks. > > > > Also what do you think about the following change: > > - char name[HYPFS_DYNDIR_ID_NAMELEN]; /* Name of hypfs entry. */ > > - > > - unsigned int id; /* Numerical id. */ > > - void *data; /* Data associated with id. */ > > + char name[HYPFS_DYNDIR_ID_NAMELEN]; /* Name of hypfs entry. */ > > + > > + unsigned int id; /* Numerical id. */ > > + union { > > + const void *content; > > + void *data; /* Data associated with id. > > */ > > + } > > This change is similar to the hypfs_entry_leaf. In this case we can > > store const pointer for read-only entries and use data when write access > > is needed? > > Sure, if you need that. Thanks I will do this as well. Best regards, Oleksii > > > > > PS: I will address all your comments in v3. > > Thanks, > > > Juergen
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |