Re: [PATCH v2 14/17] xen/hypfs: add support for id-based dynamic directories

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.12.2020 09:21, Juergen Gross wrote:
> --- a/xen/common/hypfs.c
> +++ b/xen/common/hypfs.c
> @@ -355,6 +355,81 @@ unsigned int hypfs_getsize(const struct hypfs_entry 
> *entry)
>      return entry->size;
>  }
>  
> +int hypfs_read_dyndir_id_entry(const struct hypfs_entry_dir *template,
> +                               unsigned int id, bool is_last,
> +                               XEN_GUEST_HANDLE_PARAM(void) *uaddr)
> +{
> +    struct xen_hypfs_dirlistentry direntry;
> +    char name[HYPFS_DYNDIR_ID_NAMELEN];
> +    unsigned int e_namelen, e_len;
> +
> +    e_namelen = snprintf(name, sizeof(name), template->e.name, id);
> +    e_len = DIRENTRY_SIZE(e_namelen);
> +    direntry.e.pad = 0;
> +    direntry.e.type = template->e.type;
> +    direntry.e.encoding = template->e.encoding;
> +    direntry.e.content_len = template->e.funcs->getsize(&template->e);
> +    direntry.e.max_write_len = template->e.max_size;
> +    direntry.off_next = is_last ? 0 : e_len;
> +    if ( copy_to_guest(*uaddr, &direntry, 1) )
> +        return -EFAULT;
> +    if ( copy_to_guest_offset(*uaddr, DIRENTRY_NAME_OFF, name,
> +                              e_namelen + 1) )
> +        return -EFAULT;
> +
> +    guest_handle_add_offset(*uaddr, e_len);
> +
> +    return 0;
> +}
> +
> +static struct hypfs_entry *hypfs_dyndir_findentry(
> +    const struct hypfs_entry_dir *dir, const char *name, unsigned int 
> name_len)
> +{
> +    const struct hypfs_dyndir_id *data;
> +
> +    data = hypfs_get_dyndata();
> +
> +    /* Use template with original findentry function. */
> +    return data->template->e.funcs->findentry(data->template, name, 
> name_len);
> +}
> +
> +static int hypfs_read_dyndir(const struct hypfs_entry *entry,
> +                             XEN_GUEST_HANDLE_PARAM(void) uaddr)
> +{
> +    const struct hypfs_dyndir_id *data;
> +
> +    data = hypfs_get_dyndata();
> +
> +    /* Use template with original read function. */
> +    return data->template->e.funcs->read(&data->template->e, uaddr);
> +}
> +
> +struct hypfs_entry *hypfs_gen_dyndir_entry_id(
> +    const struct hypfs_entry_dir *template, unsigned int id)
> +{
> +    struct hypfs_dyndir_id *data;
> +
> +    data = hypfs_get_dyndata();
> +
> +    data->template = template;
> +    data->id = id;
> +    snprintf(data->name, sizeof(data->name), template->e.name, id);
> +    data->dir = *template;
> +    data->dir.e.name = data->name;

I'm somewhat puzzled, if not confused, by the apparent redundancy
of this name generation with that in hypfs_read_dyndir_id_entry().
Wasn't the idea to be able to use generic functions on these
generated entries?

Also, seeing that other function's name, wouldn't the one here
want to be named hypfs_gen_dyndir_id_entry()?

Jan