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

["Jürgen Groß" <jgross@suse.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 3016 bytes --]

On 03.12.20 16:44, Jan Beulich wrote:
> 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?

I can add a macro replacing the double snprintf().

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

Fine with me.


Juergen

[-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]