[Qemu-devel] [PATCH v2 0/1] vvfat label option updated

[Qemu-devel] [PATCH v2 0/1] vvfat label option updated

[Wolfgang Bumiller]

* Till/Until changed
* Added (since 2.4) to json file
* Added the 11-byte limit description. Without mentioning the trailing
  spaces in the json file since the guest OS doesn't expose them to
  the user. However, I mentioned it in the commit message.

The limit comes from the FAT structure, but for completeness' sake I
looked up the FAT descriptions a little more.  Turns out at least
FAT32 labels are "supposed" to be stored with uppercase letters, and
seem to have some theoretical restrictions on the used characters, but
neither linux nor windows guests seem to care on the OS side. And at
least mkfs.vfat's -n option only warns about lowercase letters, but
doesn't error. Theoretically you're also not allowed to start a label
with a space, while mkfs.vfat neither warns nor errors about this.
So I decided to not add any additional checks and only mentioned it in
the json's doc string.

Wolfgang Bumiller (1):
  vvfat: add a label option

 block/vvfat.c        | 25 ++++++++++++++++++++++---
 qapi/block-core.json |  6 +++++-
 2 files changed, 27 insertions(+), 4 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH v2 1/1] vvfat: add a label option

[Wolfgang Bumiller]

Until now the vvfat volume label was hardcoded to be
"QEMU VVFAT", now you can pass a file.label=labelname option
to the -drive to change it.

The FAT structure defines the volume label to be limited to
11 bytes and is filled up spaces when shorter than that. The
trailing spaces however aren't exposed to the user by
operating systems.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 block/vvfat.c        | 25 ++++++++++++++++++++++---
 qapi/block-core.json |  6 +++++-
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index e803589..1f8a240 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -322,6 +322,7 @@ typedef struct BDRVVVFATState {
 
     int fat_type; /* 16 or 32 */
     array_t fat,directory,mapping;
+    char volume_label[11];
 
     unsigned int cluster_size;
     unsigned int sectors_per_cluster;
@@ -859,7 +860,7 @@ static int init_directories(BDRVVVFATState* s,
     {
 	direntry_t* entry=array_get_next(&(s->directory));
 	entry->attributes=0x28; /* archive | volume label */
-        memcpy(entry->name, "QEMU VVFAT ", sizeof(entry->name));
+        memcpy(entry->name, s->volume_label, sizeof(entry->name));
     }
 
     /* Now build FAT, and write back information into directory */
@@ -968,7 +969,8 @@ static int init_directories(BDRVVVFATState* s,
     bootsector->u.fat16.signature=0x29;
     bootsector->u.fat16.id=cpu_to_le32(0xfabe1afd);
 
-    memcpy(bootsector->u.fat16.volume_label,"QEMU VVFAT ",11);
+    memcpy(bootsector->u.fat16.volume_label, s->volume_label,
+           sizeof(bootsector->u.fat16.volume_label));
     memcpy(bootsector->fat_type,(s->fat_type==12?"FAT12   ":s->fat_type==16?"FAT16   ":"FAT32   "),8);
     bootsector->magic[0]=0x55; bootsector->magic[1]=0xaa;
 
@@ -1008,6 +1010,11 @@ static QemuOptsList runtime_opts = {
             .help = "Create a floppy rather than a hard disk image",
         },
         {
+            .name = "label",
+            .type = QEMU_OPT_STRING,
+            .help = "Use a volume label other than QEMU VVFAT",
+        },
+        {
             .name = "rw",
             .type = QEMU_OPT_BOOL,
             .help = "Make the image writable",
@@ -1069,7 +1076,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
     BDRVVVFATState *s = bs->opaque;
     int cyls, heads, secs;
     bool floppy;
-    const char *dirname;
+    const char *dirname, *label;
     QemuOpts *opts;
     Error *local_err = NULL;
     int ret;
@@ -1096,6 +1103,18 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
     s->fat_type = qemu_opt_get_number(opts, "fat-type", 0);
     floppy = qemu_opt_get_bool(opts, "floppy", false);
 
+    memset(s->volume_label, ' ', sizeof(s->volume_label));
+    label = qemu_opt_get(opts, "label");
+    if (label) {
+        size_t label_length = strlen(label);
+        if (label_length > 11) {
+            error_setg(errp, "vvfat label cannot be longer than 11 bytes");
+            ret = -EINVAL;
+            goto fail;
+        }
+        memcpy(s->volume_label, label, label_length);
+    }
+
     if (floppy) {
         /* 1.44MB or 2.88MB floppy.  2.88MB can be FAT12 (default) or FAT16. */
         if (!s->fat_type) {
diff --git a/qapi/block-core.json b/qapi/block-core.json
index afa9d3d..feb9e4f 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1453,13 +1453,17 @@
 # @fat-type:    #optional FAT type: 12, 16 or 32
 # @floppy:      #optional whether to export a floppy image (true) or
 #               partitioned hard disk (false; default)
+# @label:       #optional set the volume label, limited to 11 bytes. FAT16 and
+                FAT32 traditionally have some restrictions on labels, which are
+                ignored by most operating systems. Defaults to "QEMU VVFAT".
+                (since 2.4)
 # @rw:          #optional whether to allow write operations (default: false)
 #
 # Since: 1.7
 ##
 { 'struct': 'BlockdevOptionsVVFAT',
   'data': { 'dir': 'str', '*fat-type': 'int', '*floppy': 'bool',
-            '*rw': 'bool' } }
+            '*label': 'str', '*rw': 'bool' } }
 
 ##
 # @BlockdevOptionsGenericFormat
-- 
2.1.4

Re: [Qemu-devel] [PATCH v2 1/1] vvfat: add a label option

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1423 bytes --]

On 06/19/2015 03:35 AM, Wolfgang Bumiller wrote:
> Until now the vvfat volume label was hardcoded to be
> "QEMU VVFAT", now you can pass a file.label=labelname option
> to the -drive to change it.
> 
> The FAT structure defines the volume label to be limited to
> 11 bytes and is filled up spaces when shorter than that. The
> trailing spaces however aren't exposed to the user by
> operating systems.
> 
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
>  block/vvfat.c        | 25 ++++++++++++++++++++++---
>  qapi/block-core.json |  6 +++++-
>  2 files changed, 27 insertions(+), 4 deletions(-)
> 

> +++ b/qapi/block-core.json
> @@ -1453,13 +1453,17 @@
>  # @fat-type:    #optional FAT type: 12, 16 or 32
>  # @floppy:      #optional whether to export a floppy image (true) or
>  #               partitioned hard disk (false; default)
> +# @label:       #optional set the volume label, limited to 11 bytes. FAT16 and
> +                FAT32 traditionally have some restrictions on labels, which are
> +                ignored by most operating systems. Defaults to "QEMU VVFAT".
> +                (since 2.4)

Nicer; thanks.  (Still doesn't mention what those restrictions are, but
I'm not as worried about that)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH v2 0/1] vvfat label option updated

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1351 bytes --]

On Fri, Jun 19, 2015 at 11:35:28AM +0200, Wolfgang Bumiller wrote:
> * Till/Until changed
> * Added (since 2.4) to json file
> * Added the 11-byte limit description. Without mentioning the trailing
>   spaces in the json file since the guest OS doesn't expose them to
>   the user. However, I mentioned it in the commit message.
> 
> The limit comes from the FAT structure, but for completeness' sake I
> looked up the FAT descriptions a little more.  Turns out at least
> FAT32 labels are "supposed" to be stored with uppercase letters, and
> seem to have some theoretical restrictions on the used characters, but
> neither linux nor windows guests seem to care on the OS side. And at
> least mkfs.vfat's -n option only warns about lowercase letters, but
> doesn't error. Theoretically you're also not allowed to start a label
> with a space, while mkfs.vfat neither warns nor errors about this.
> So I decided to not add any additional checks and only mentioned it in
> the json's doc string.
> 
> Wolfgang Bumiller (1):
>   vvfat: add a label option
> 
>  block/vvfat.c        | 25 ++++++++++++++++++++++---
>  qapi/block-core.json |  6 +++++-
>  2 files changed, 27 insertions(+), 4 deletions(-)
> 
> -- 
> 2.1.4
> 
> 
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]