Re: [Libguestfs] [PATCH nbdkit 4/4] Add floppy plugin.

Eric Blake
2018-10-30T14:12:55Z

On 10/28/18 5:13 AM, Richard W.M. Jones wrote:
> Create a virtual FAT-formatted floppy disk from a directory of files.
>
> For example:
>
>    nbdkit floppy /path/to/directory
>
> The implementation of this is quite different from nbdkit-iso-plugin
> since we cannot use an external program.  Instead this plugin
> synthesizes the MBR partition and FAT32 structures that are required.
>
> To do: Implement bootable virtual floppy using syslinux.  This is not
> straightforward as you have to either run the syslinux command on the
> virtual floppy, or else you have to synthesize the same set of
> operations that syslinux performs.  Also you have to deal with CHS
> geometry.
> ---
>   configure.ac                            |   2 +
>   plugins/floppy/Makefile.am              |  68 ++
>   plugins/floppy/directory-lfn.c          | 623 ++++++++++++++++++
>   plugins/floppy/floppy.c                 | 198 ++++++
>   plugins/floppy/nbdkit-floppy-plugin.pod |  87 +++
>   plugins/floppy/virtual-floppy.c         | 797 ++++++++++++++++++++++++
>   plugins/floppy/virtual-floppy.h         | 233 +++++++
>   plugins/iso/nbdkit-iso-plugin.pod       |   1 +
>   tests/Makefile.am                       |   6 +
>   tests/test-floppy.sh                    |  67 ++
>   10 files changed, 2082 insertions(+)
>

> +/* Used for dealing with VFAT LFNs when creating a directory. */
> +struct lfn {
> +  const char *name;             /* Original Unix filename. */
> +  char short_base[8];           /* Short basename. */
> +  char short_ext[3];            /* Short file extension. */
> +  char *lfn;                    /* Long filename for MS-DOS as UTF16-LE.
*/
> +  size_t lfn_size;              /* Size *in bytes* of lfn. */

Including or excluding the trailing 2 bytes for a terminating NUL character?


> +/* Create the on disk directory table for dirs[di]. */
> +int
> +create_directory (size_t di, const char *label,
> +                  struct virtual_floppy *floppy)
> +{

> +  /* Add files. */
> +  attributes = DIR_ENTRY_ARCHIVE; /* Same as set by Linux kernel. */
> +  for (i = 0; i < nr_files; ++i) {
> +    const size_t fi = floppy->dirs[di].files[i];
> +    assert (fi < floppy->nr_files);
> +
> +    lfn = &lfns[nr_subdirs+i];

Spacing around +

> +/* Either truncate or pad a string (with spaces). */
> +void
> +pad_string (const char *label, size_t n, uint8_t *out)
> +{
> +  const size_t len = strlen (label);
> +
> +  memcpy (out, label, len <= n ? len : n);
> +  if (len < n)
> +    memset (out+len, ' ', n-len);

More operator spacing

> +}
> +
> +/* Add a directory entry to dirs[di].table. */
> +static int
> +add_directory_entry (const struct lfn *lfn,
> +                     uint8_t attributes, uint32_t file_size,
> +                     struct stat *statbuf,
> +                     size_t di, struct virtual_floppy *floppy)
> +{
> +  uint8_t seq, checksum;
> +  ssize_t i;
> +  size_t j;
> +  struct lfn_entry lfn_entry;
> +  struct dir_entry entry;
> +  int last_seq = 1;
> +
> +  /* Compute a checksum for the shortname.  In real LFN filesystems
> +   * this is used to check whether a non-LFN-aware operating system
> +   * (ie. MS-DOS) has edited the directory.
> +   */
> +  checksum = 0;
> +  for (j = 0; j < 8; ++j)
> +    checksum = ((checksum & 1) << 7) + (checksum >> 1) +
lfn->short_base[j];
> +  for (j = 0; j < 3; ++j)
> +    checksum = ((checksum & 1) << 7) + (checksum >> 1) +
lfn->short_ext[j];
> +
> +  /* LFN support.
> +   *
> +   * Iterate in reverse over the sequence numbers.  If the filename is:
> +   *
> +   *   "ABCDEFGHIJKLMNO"
> +   *
> +   * assuming those are UCS-2 codepoints, so lfn_size = 15*2 = 30,
> +   * then we generate these LFN sequences:
> +   *
> +   *   seq   byte_offset   s[13]
> +   *   0x42  26            "NO           "
> +   *   0x01  0             "ABCDEFGHIJKLM"
> +   */
> +  for (seq = 1 + lfn->lfn_size/2/13; seq >= 1; --seq) {
> +    size_t byte_offset = (seq-1)*2*13, r;

And again

> +    uint16_t s[13];
> +
> +    /* Copy the portion of the LFN into s.
> +     * r = Number of bytes from the original string to copy.
> +     */
> +    r = lfn->lfn_size - byte_offset;
> +    if (r > 26)
> +      memcpy (s, &lfn->lfn[byte_offset], 26);
> +    else {
> +      memcpy (s, &lfn->lfn[byte_offset], r);
> +      /* Pad remaining filename with 0. */
> +      for (j = r/2; j < 13; ++j)

And here

> +        s[j] = htole16 (0);
> +    }
> +
> +    memset (&lfn_entry, 0, sizeof lfn_entry);
> +    lfn_entry.seq = seq;
> +    if (last_seq) {
> +      lfn_entry.seq |= 0x40;
> +      last_seq = 0;
> +    }
> +    lfn_entry.attributes = 0xf;
> +    lfn_entry.checksum = checksum;
> +
> +    /* Copy the name portion to the fields in the LFN entry. */
> +    memcpy (lfn_entry.name1, &s[0], 5*2);
> +    memcpy (lfn_entry.name2, &s[5], 6*2);
> +    memcpy (lfn_entry.name3, &s[11], 2*2);

And again.

> +
> +    i = extend_dir_table (di, floppy);
> +    if (i == -1)
> +      return -1;
> +    memcpy (&floppy->dirs[di].table[i], &lfn_entry, sizeof
(struct dir_entry));
> +  }
> +
> +  /* Create the 8.3 (short name / DOS-compatible) entry. */
> +  memset (&entry, 0, sizeof entry);
> +  memcpy (entry.name, lfn->short_base, 8);
> +  memcpy (entry.name+8, lfn->short_ext, 3);

here too

> +  entry.attributes = attributes;
> +  set_times (statbuf, &entry);
> +  entry.size = htole32 (file_size);
> +  /* Note that entry.cluster_hi and .cluster_lo are set later on in
> +   * update_directory_first_cluster.
> +   */
> +
> +  i = extend_dir_table (di, floppy);
> +  if (i == -1)
> +    return -1;
> +  floppy->dirs[di].table[i] = entry;
> +
> +  return 0;
> +}
> +

> +static int
> +convert_long_file_names (struct lfn *lfns, size_t n)
> +{

> +
> +    /* Convert the original filename to UTF16-LE.  Maximum LFN length
> +     * is 0x3f * 13 = 819 UCS-2 characters.
> +     */
> +    if (convert_to_utf16le (lfn->name, &lfn->lfn,
&lfn->lfn_size) == -1)
> +      return -1;
> +    if (lfn->lfn_size > 2*819) {
> +      nbdkit_error ("%s: filename is too long", lfn->name);

And here. An error unlikely to happen since most file systems tend to
limit things to 256 bytes, but don't remove the safety check.

> +      return -1;
> +    }
> +  }
> +
> +  /* Now we must see if some short filenames are duplicates and
> +   * rename them.  XXX Unfortunately O(n^2).
> +   */
> +  for (i = 1; i < n; ++i) {
> +    for (j = 0; j < i; ++j) {
> +      if (memcmp (lfns[i].short_base, lfns[j].short_base, 8) == 0
&&
> +          memcmp (lfns[i].short_ext, lfns[j].short_ext, 3) == 0) {
> +        char s[9];
> +        ssize_t k;
> +
> +        /* Entry i is a duplicate of j (j < i).  So we will rename i.
*/
> +        lfn = &lfns[i];
> +
> +        len = snprintf (s, sizeof s, "~%zu", i);
> +        assert (len >= 2 && len <= 8);
> +
> +        k = 8-len;
> +        while (k > 0 && lfn->short_base[k] == ' ')
> +          k--;
> +        memcpy (&lfn->short_base[k], s, len);
> +      }
> +    }
> +  }

Does this properly handle filenames with leading or trailing '.' but no
other reason to rename a short name into LFN format?  (I didn't actually
test, I just know that such filenames tend to have interesting effects
on FAT).  Or was that an issue with FAT16, but not FAT32?


> +/* Extend dirs[di].table by 1 directory entry. */
> +static ssize_t
> +extend_dir_table (size_t di, struct virtual_floppy *floppy)
> +{
> +  struct dir_entry *p;
> +  size_t i;
> +
> +  i = floppy->dirs[di].table_entries;
> +  p = realloc (floppy->dirs[di].table, sizeof (struct dir_entry) *
(i+1));

More spacing

> +++ b/plugins/floppy/floppy.c

> +static int
> +floppy_config (const char *key, const char *value)
> +{
> +  if (strcmp (key, "dir") == 0) {
> +    dir = nbdkit_realpath (value);
> +    if (dir == NULL)
> +      return -1;
> +  }

Should you error if the user passes more than one dir= instead of
leaking the earlier one?

> +++ b/plugins/floppy/nbdkit-floppy-plugin.pod
> @@ -0,0 +1,87 @@
> +=head1 NAME
> +
> +nbdkit-floppy-plugin - create virtual floppy disk from directory
> +
> +=head1 SYNOPSIS
> +
> + nbdkit floppy [dir=]DIRECTORY
> +               [label=LABEL]
> +
> +=head1 DESCRIPTION
> +
> +C<nbdkit-floppy-plugin> is a plugin for L<nbdkit(1)> which
creates a
> +virtual FAT-formatted floppy disk image from a directory on the fly.
> +The files in the specified directory (and subdirectories) appear in
> +the virtual floppy, which is served read-only over the NBD protocol.
> +
> +The virtual floppy disk will have a single partition (using an MBR
> +partition table).  In that partition will be a virtual FAT32
> +filesystem containing the files.  Long filenames are supported.
> +
> +=head1 EXAMPLE
> +
> +Create a virtual floppy disk:
> +
> + nbdkit floppy /path/to/directory
> +
> +=head1 PARAMETERS
> +
> +=over 4
> +
> +=item B<dir=>DIRECTORY
> +
> +Specify the directory containing files and subdirectories which will
> +be added to the virtual floppy disk.  Files inside this directory will
> +appear in the root directory of the virtual floppy.
> +
> +This parameter is required.
> +
> +In nbdkit E<ge> 1.8, C<dir=> may be omitted.  To ensure that
the

Do we still need the >= 1.8 disclaimer, given that this plugin is newer
than the point where we added the feature? (I guess so, since you are
still numbering things 1.7.x, and it's easier to rely on new features at
the bump to 1.8)

> +directory name does not end up being parsed accidentally as
> +C<key=value>, prefix relative paths with C<./> (absolute paths
do not
> +need modification).
> +
> +=item B<label=>LABEL
> +
> +The optional volume label for the filesystem.  This may be up to 11
> +ASCII characters.  If omitted, C<NBDKITFLOPY> is used.
> +
> +=back
> +
> +=head1 LIMITATIONS
> +
> +The maximum size of the disk is around 2TB.  The maximum size of a
> +single file is 4GB.  Non-regular files (such as block special,
> +symbolic links, sockets) are not supported and will be ignored.

If I recall, there is also a limitation on the top-level directory
having a maximum number of children (was it something like 256 or 512
entries), since it does not get to continue on to secondary pages,
unlike subdirectories.

Should symlinks pointing to somewhere within the directory be supported
by expanding it into the contents visible through the symlink? (But if
we ever add that, we have to be careful of symlink-to-directory loops,
as well as race conditions where TOCTTOU could result in a symlink
escaping the directory)

> +
> +The plugin does not support writes.
> +
> +The plugin does not save a temporary copy of the files, so you must
> +leave the directory alone while nbdkit is running, else you may get an
> +error for example if the plugin tries to open one of the files which
> +you have moved or deleted.  (This is different from how
> +L<nbdkit-iso-plugin(1)> works).

Is there anything we can do on Linux (such as mounting an overlayfs) to
ensure that the user can't change the contents we are serving from
underneath us?  But for now, I'm fine with the current restriction of
just documenting that the user really shouldn't be modifying things
behind nbdkit's back.

> +
> +The virtual floppy will not be bootable.  This could be added in
> +future (using SYSLINUX) but requires considerable work.  As a
> +workaround use L<nbdkit-iso-plugin(1)> instead.
> +
> +FAT32 is always used, even for small disks (where dosfstools, for
> +example, would choose FAT12 or FAT16).  This results in extra wasted
> +space, but since it is only I<virtual> wasted space it isn't
really
> +important, and it simplifies the implementation greatly.
> +

> +++ b/plugins/floppy/virtual-floppy.c


> +static ssize_t
> +visit (const char *dir, struct virtual_floppy *floppy)
> +{
> +  void *np;
> +  size_t di;
> +  char *origdir;
> +  DIR *DIR;
> +  struct dirent *d;
> +  int err;
> +  struct stat statbuf;
> +
> +  /* Allocate a new index in the directory array.  Note that the root
> +   * directory will always be at dirs[0].
> +   */
> +  di = floppy->nr_dirs;
> +  np = realloc (floppy->dirs, sizeof (struct dir) * (di+1));

More operator spacing.

> +  if (np == NULL) {
> +    nbdkit_error ("realloc: %m");
> +    goto error0;
> +  }
> +  floppy->dirs = np;
> +  floppy->nr_dirs++;
> +  memset (&floppy->dirs[di], 0, sizeof (struct dir));
> +
> +  /* Because this is called from config_complete, before nbdkit
> +   * daemonizes or starts any threads, it's safe to use chdir here and
> +   * greatly simplifies the code.  However we must chdir back to the
> +   * original directory at the end.
> +   */
> +  origdir = get_current_dir_name ();
> +  if (origdir == NULL) {
> +    nbdkit_error ("get_current_dir_name: %m");
> +    goto error0;
> +  }
> +  if (chdir (dir) == -1) {
> +    nbdkit_error ("chdir: %s: %m", dir);
> +    goto error1;
> +  }
> +
> +  DIR = opendir (".");
> +  if (DIR == NULL) {
> +    nbdkit_error ("opendir: %s: %m", dir);
> +    goto error1;
> +  }
> +
> +  errno = 0;
> +  while ((d = readdir (DIR)) != NULL) {
> +    if (strcmp (d->d_name, ".") == 0 ||
> +        strcmp (d->d_name, "..") == 0)
> +      continue;
> +
> +    if (lstat (d->d_name, &statbuf) == -1) {
> +      nbdkit_error ("stat: %s/%s: %m", dir, d->d_name);
> +      goto error2;
> +    }
> +
> +    /* Directory. */
> +    if (S_ISDIR (statbuf.st_mode)) {
> +      if (visit_subdirectory (dir, d->d_name, &statbuf, di, floppy)
== -1)
> +        goto error2;
> +    }
> +    /* Regular file. */
> +    else if (S_ISREG (statbuf.st_mode)) {
> +      if (visit_file (dir, d->d_name, &statbuf, di, floppy) == -1)
> +        goto error2;
> +    }
> +    /* else ALL other file types are ignored - see documentation. */
> +  }

Need to reset errno back to 0 before looping back to the next readdir()
call.

> +
> +  /* Did readdir fail? */
> +  if (errno != 0) {
> +    nbdkit_error ("readdir: %s: %m", dir);
> +    goto error2;
> +  }
> +
> +  if (closedir (DIR) == -1) {
> +    nbdkit_error ("closedir: %s: %m", dir);
> +    goto error1;
> +  }
> +
> +  if (chdir (origdir) == -1) {
> +    nbdkit_error ("chdir: %s: %m", origdir);
> +    goto error0;
> +  }
> +  free (origdir);
> +  return di;
> +
> + error2:
> +  closedir (DIR);
> + error1:
> +  err = errno;
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wunused-result"
> +  chdir (origdir);
> +#pragma GCC diagnostic pop
> +  errno = err;
> +  free (origdir);
> + error0:
> +  return -1;
> +}
> +
> +/* This is called to visit a subdirectory in a directory.  It
> + * recursively calls the visit() function, and then adds the
> + * subdirectory to the list of subdirectories in the parent.
> + */
> +static int
> +visit_subdirectory (const char *dir, const char *name,
> +                    const struct stat *statbuf, size_t di,
> +                    struct virtual_floppy *floppy)
> +{
> +  void *np;
> +  char *subdir;
> +  ssize_t sdi;                  /* subdirectory index */
> +  size_t i;
> +
> +  if (asprintf (&subdir, "%s/%s", dir, name) == -1) {
> +    nbdkit_error ("asprintf: %m");
> +    return -1;
> +  }
> +  /* Recursively visit this directory.  As a side effect this adds the
> +   * new subdirectory to the global list of directories, and returns
> +   * the index in that list (sdi).
> +   */
> +  sdi = visit (subdir, floppy);
> +  if (sdi == -1) {
> +    free (subdir);
> +    return -1;
> +  }
> +  free (subdir);
> +
> +  /* We must set sdi->name because visit() cannot set it. */
> +  floppy->dirs[sdi].name = strdup (name);
> +  if (floppy->dirs[sdi].name == NULL) {
> +    nbdkit_error ("strdup: %m");
> +    return -1;
> +  }
> +  floppy->dirs[sdi].statbuf = *statbuf;
> +  floppy->dirs[sdi].pdi = di;
> +
> +  /* Add to the list of subdirs in the parent directory (di). */
> +  i = floppy->dirs[di].nr_subdirs;
> +  np = realloc (floppy->dirs[di].subdirs, sizeof (size_t) * (i+1));

more spacing

> +  if (np == NULL) {
> +    nbdkit_error ("realloc: %m");
> +    return -1;
> +  }
> +  floppy->dirs[di].subdirs = np;
> +  floppy->dirs[di].nr_subdirs++;
> +  floppy->dirs[di].subdirs[i] = sdi;
> +
> +  return 0;
> +}
> +
> +/* This is called to visit a file in a directory.  It performs some
> + * checks and then adds the file to the global list of files, and also
> + * adds the file to the list of files in the parent directory.
> + */
> +static int
> +visit_file (const char *dir, const char *name,
> +            const struct stat *statbuf, size_t di,
> +            struct virtual_floppy *floppy)
> +{
> +  void *np;
> +  char *host_path;
> +  size_t fi, i;
> +
> +  if (asprintf (&host_path, "%s/%s", dir, name) == -1) {
> +    nbdkit_error ("asprintf: %m");
> +    return -1;
> +  }
> +
> +  if (statbuf->st_size >= UINT32_MAX) {
> +    nbdkit_error ("%s: file is larger than maximum supported by
VFAT",
> +                  host_path);
> +    free (host_path);
> +    return -1;
> +  }
> +
> +  /* Add to global list of files. */
> +  fi = floppy->nr_files;
> +  np = realloc (floppy->files, sizeof (struct file) * (fi+1));

and again

> +  if (np == NULL) {
> +    nbdkit_error ("realloc: %m");
> +    free (host_path);
> +    return -1;
> +  }
> +  floppy->files = np;
> +  floppy->nr_files++;
> +  floppy->files[fi].name = strdup (name);
> +  if (floppy->files[fi].name == NULL) {
> +    nbdkit_error ("strdup: %m");
> +    free (host_path);
> +    return -1;
> +  }
> +  floppy->files[fi].host_path = host_path;
> +  floppy->files[fi].statbuf = *statbuf;
> +
> +  /* Add to the list of files in the parent directory (di). */
> +  i = floppy->dirs[di].nr_files;
> +  np = realloc (floppy->dirs[di].files, sizeof (size_t) * (i+1));

Recurring issue, isn't it :)


> +static int
> +create_regions (struct virtual_floppy *floppy)
> +{
> +  struct region region;
> +  size_t i;
> +
> +  /* MBR. */
> +  region.start = 0;
> +  region.end = SECTOR_SIZE-1;

and again

Overall, looks like a useful plugin!

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org