[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On 04.12.20 10:16, Jan Beulich wrote:
On 04.12.2020 09:52, Jürgen Groß wrote:
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().

That wasn't my point. I'm concerned of there being two name generation
sites in the first place. Is this perhaps simply some form of
optimization, avoiding hypfs_read_dyndir_id_entry() to call
hypfs_gen_dyndir_entry_id() (but risking the two going out of sync)?

Be aware that hypfs_read_dyndir_id_entry() is generating a struct
xen_hypfs_dirlistentry, which is different from the internal
representation of the data produced by hypfs_gen_dyndir_entry_id().

So the main common part is the name generation. What else would you
want apart from making it common via e.g. a macro? Letting
hypfs_read_dyndir_id_entry() call hypfs_gen_dyndir_entry_id() would
just be a more general approach with all the data but the generated
name of hypfs_gen_dyndir_entry_id() dropped or just copied around
a second time.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.