[Qemu-devel] [PATCH v2] ide: Adds "model=s" option, allowing the user to override the default disk model name "QEMU HARDDISK"

[Qemu-devel] [PATCH v2] ide: Adds "model=s" option, allowing the user to override the default disk model name "QEMU HARDDISK"

[Floris Bos]

Some Linux distributions use the /dev/disk/by-id/scsi-SATA_name-of-disk-model_serial addressing scheme
when refering to partitions in /etc/fstab and elsewhere.
This causes problems when starting a disk image taken from an existing physical server under qemu,
because when running under qemu name-of-disk-model is always "QEMU HARDDISK"
This patch introduces a model=s option which in combination with the existing serial=s option can be used to
fake the disk the operating system was previously on, allowing the OS to boot properly.

Cc: kwolf@redhat.com
Signed-off-by: Floris Bos <dev@noc-ps.com>
---
 blockdev.c        |    5 +++++
 blockdev.h        |    2 ++
 hw/ide/core.c     |   27 ++++++++++++++++++++++-----
 hw/ide/internal.h |    4 +++-
 hw/ide/qdev.c     |   18 ++++++++++++++++--
 qemu-config.c     |    4 ++++
 qemu-options.hx   |    4 +++-
 7 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index d78aa51..3df9cb9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -277,6 +277,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
     const char *file = NULL;
     char devname[128];
     const char *serial;
+    const char *model;
     const char *mediastr = "";
     BlockInterfaceType type;
     enum { MEDIA_DISK, MEDIA_CDROM } media;
@@ -313,6 +314,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
 
     file = qemu_opt_get(opts, "file");
     serial = qemu_opt_get(opts, "serial");
+    model = qemu_opt_get(opts, "model");
 
     if ((buf = qemu_opt_get(opts, "if")) != NULL) {
         pstrcpy(devname, sizeof(devname), buf);
@@ -534,6 +536,9 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
     dinfo->refcount = 1;
     if (serial)
         strncpy(dinfo->serial, serial, sizeof(dinfo->serial) - 1);
+    if (model) {
+        pstrcpy(dinfo->model, sizeof(dinfo->model), model);
+    }
     QTAILQ_INSERT_TAIL(&drives, dinfo, next);
 
     bdrv_set_on_error(dinfo->bdrv, on_read_error, on_write_error);
diff --git a/blockdev.h b/blockdev.h
index 260e16b..21eb4b5 100644
--- a/blockdev.h
+++ b/blockdev.h
@@ -18,6 +18,7 @@ void blockdev_mark_auto_del(BlockDriverState *bs);
 void blockdev_auto_del(BlockDriverState *bs);
 
 #define BLOCK_SERIAL_STRLEN 20
+#define BLOCK_MODEL_STRLEN 40
 
 typedef enum {
     IF_DEFAULT = -1,            /* for use with drive_add() only */
@@ -37,6 +38,7 @@ struct DriveInfo {
     int media_cd;
     QemuOpts *opts;
     char serial[BLOCK_SERIAL_STRLEN + 1];
+    char model[BLOCK_MODEL_STRLEN + 1];
     QTAILQ_ENTRY(DriveInfo) next;
     int refcount;
 };
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 4d568ac..7bd2810 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -101,7 +101,7 @@ static void ide_identify(IDEState *s)
     put_le16(p + 21, 512); /* cache size in sectors */
     put_le16(p + 22, 4); /* ecc bytes */
     padstr((char *)(p + 23), s->version, 8); /* firmware version */
-    padstr((char *)(p + 27), "QEMU HARDDISK", 40); /* model */
+    padstr((char *)(p + 27), s->drive_model_str, 40); /* model */
 #if MAX_MULT_SECTORS > 1
     put_le16(p + 47, 0x8000 | MAX_MULT_SECTORS);
 #endif
@@ -189,7 +189,7 @@ static void ide_atapi_identify(IDEState *s)
     put_le16(p + 21, 512); /* cache size in sectors */
     put_le16(p + 22, 4); /* ecc bytes */
     padstr((char *)(p + 23), s->version, 8); /* firmware version */
-    padstr((char *)(p + 27), "QEMU DVD-ROM", 40); /* model */
+    padstr((char *)(p + 27), s->drive_model_str, 40); /* model */
     put_le16(p + 48, 1); /* dword I/O (XXX: should not be set on CDROM) */
 #ifdef USE_DMA_CDROM
     put_le16(p + 49, 1 << 9 | 1 << 8); /* DMA and LBA supported */
@@ -246,7 +246,7 @@ static void ide_cfata_identify(IDEState *s)
     padstr((char *)(p + 10), s->drive_serial_str, 20); /* serial number */
     put_le16(p + 22, 0x0004);			/* ECC bytes */
     padstr((char *) (p + 23), s->version, 8);	/* Firmware Revision */
-    padstr((char *) (p + 27), "QEMU MICRODRIVE", 40);/* Model number */
+    padstr((char *) (p + 27), s->drive_model_str, 40);/* Model number */
 #if MAX_MULT_SECTORS > 1
     put_le16(p + 47, 0x8000 | MAX_MULT_SECTORS);
 #else
@@ -1834,7 +1834,7 @@ static const BlockDevOps ide_cd_block_ops = {
 };
 
 int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
-                   const char *version, const char *serial)
+                   const char *version, const char *serial, const char *model)
 {
     int cylinders, heads, secs;
     uint64_t nb_sectors;
@@ -1885,6 +1885,22 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
         snprintf(s->drive_serial_str, sizeof(s->drive_serial_str),
                  "QM%05d", s->drive_serial);
     }
+    if (model) {
+        pstrcpy(s->drive_model_str, sizeof(s->drive_model_str), model);
+    } else {
+        switch (kind) {
+        case IDE_CD:
+            strcpy(s->drive_model_str, "QEMU DVD-ROM");
+            break;
+        case IDE_CFATA:
+            strcpy(s->drive_model_str, "QEMU MICRODRIVE");
+            break;
+        default:
+            strcpy(s->drive_model_str, "QEMU HARDDISK");
+            break;
+        }
+    }
+
     if (version) {
         pstrcpy(s->version, sizeof(s->version), version);
     } else {
@@ -1977,7 +1993,8 @@ void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
         if (dinfo) {
             if (ide_init_drive(&bus->ifs[i], dinfo->bdrv,
                                dinfo->media_cd ? IDE_CD : IDE_HD, NULL,
-                               *dinfo->serial ? dinfo->serial : NULL) < 0) {
+                               *dinfo->serial ? dinfo->serial : NULL,
+                               *dinfo->model ? dinfo->model : NULL) < 0) {
                 error_report("Can't set up IDE drive %s", dinfo->id);
                 exit(1);
             }
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index c808a0d..b1319dc 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -348,6 +348,7 @@ struct IDEState {
     uint8_t identify_data[512];
     int drive_serial;
     char drive_serial_str[21];
+    char drive_model_str[41];
     /* ide regs */
     uint8_t feature;
     uint8_t error;
@@ -468,6 +469,7 @@ struct IDEDevice {
     BlockConf conf;
     char *version;
     char *serial;
+    char *model;
 };
 
 #define BM_STATUS_DMAING 0x01
@@ -534,7 +536,7 @@ void ide_data_writel(void *opaque, uint32_t addr, uint32_t val);
 uint32_t ide_data_readl(void *opaque, uint32_t addr);
 
 int ide_init_drive(IDEState *s, BlockDriverState *bs, IDEDriveKind kind,
-                   const char *version, const char *serial);
+                   const char *version, const char *serial, const char *model);
 void ide_init2(IDEBus *bus, qemu_irq irq);
 void ide_init2_with_non_qdev_drives(IDEBus *bus, DriveInfo *hd0,
                                     DriveInfo *hd1, qemu_irq irq);
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index f6a4896..3854c20 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -120,6 +120,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
     IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
     IDEState *s = bus->ifs + dev->unit;
     const char *serial;
+    const char *model;
     DriveInfo *dinfo;
 
     if (dev->conf.discard_granularity && dev->conf.discard_granularity != 512) {
@@ -135,8 +136,17 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
             serial = dinfo->serial;
         }
     }
+    model = dev->model;
+    if (!model) {
+        /* try to fall back to value set with legacy -drive model=... */
+        dinfo = drive_get_by_blockdev(dev->conf.bs);
+        if (*dinfo->model) {
+            model = dinfo->model;
+        }
+    }
 
-    if (ide_init_drive(s, dev->conf.bs, kind, dev->version, serial) < 0) {
+    if (ide_init_drive(s, dev->conf.bs, kind,
+                       dev->version, serial, model) < 0) {
         return -1;
     }
 
@@ -146,6 +156,9 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
     if (!dev->serial) {
         dev->serial = g_strdup(s->drive_serial_str);
     }
+    if (!dev->model) {
+        dev->model = g_strdup(s->drive_model_str);
+    }
 
     add_boot_device_path(dev->conf.bootindex, &dev->qdev,
                          dev->unit ? "/disk@1" : "/disk@0");
@@ -173,7 +186,8 @@ static int ide_drive_initfn(IDEDevice *dev)
 #define DEFINE_IDE_DEV_PROPERTIES()                     \
     DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),        \
     DEFINE_PROP_STRING("ver",  IDEDrive, dev.version),  \
-    DEFINE_PROP_STRING("serial",  IDEDrive, dev.serial)
+    DEFINE_PROP_STRING("serial",  IDEDrive, dev.serial),\
+    DEFINE_PROP_STRING("model", IDEDrive, dev.model)
 
 static Property ide_hd_properties[] = {
     DEFINE_IDE_DEV_PROPERTIES(),
diff --git a/qemu-config.c b/qemu-config.c
index be84a03..9e7459f 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -70,6 +70,10 @@ static QemuOptsList qemu_drive_opts = {
             .type = QEMU_OPT_STRING,
             .help = "disk serial number",
         },{
+            .name = "model",
+            .type = QEMU_OPT_STRING,
+            .help = "disk model name",
+        },{
             .name = "rerror",
             .type = QEMU_OPT_STRING,
             .help = "read error action",
diff --git a/qemu-options.hx b/qemu-options.hx
index daefce3..70b5cf6 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -140,7 +140,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
     "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
     "       [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
     "       [,cache=writethrough|writeback|none|directsync|unsafe][,format=f]\n"
-    "       [,serial=s][,addr=A][,id=name][,aio=threads|native]\n"
+    "       [,serial=s][,model=s][,addr=A][,id=name][,aio=threads|native]\n"
     "       [,readonly=on|off][,copy-on-read=on|off]\n"
     "       [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]][[,iops=i]|[[,iops_rd=r][,iops_wr=w]]\n"
     "                use 'file' as a drive image\n", QEMU_ARCH_ALL)
@@ -183,6 +183,8 @@ the format.  Can be used to specifiy format=raw to avoid interpreting
 an untrusted format header.
 @item serial=@var{serial}
 This option specifies the serial number to assign to the device.
+@item model=@var{model}
+This option specifies the model name to assign to the device. Default for IDE disks is: QEMU HARDDISK
 @item addr=@var{addr}
 Specify the controller's PCI address (if=virtio only).
 @item werror=@var{action},rerror=@var{action}
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH v2] ide: Adds "model=s" option, allowing the user to override the default disk model name "QEMU HARDDISK"

[Markus Armbruster]

Floris Bos <bos@je-eigen-domein.nl> writes:

> Some Linux distributions use the /dev/disk/by-id/scsi-SATA_name-of-disk-model_serial addressing scheme
> when refering to partitions in /etc/fstab and elsewhere.
> This causes problems when starting a disk image taken from an existing physical server under qemu,
> because when running under qemu name-of-disk-model is always "QEMU HARDDISK"
> This patch introduces a model=s option which in combination with the existing serial=s option can be used to
> fake the disk the operating system was previously on, allowing the OS to boot properly.
>
> Cc: kwolf@redhat.com
> Signed-off-by: Floris Bos <dev@noc-ps.com>

At first glance:

1. IDE only, scsi-disk is missing.

2. "model" needs to be a device property, just like "serial".

3. I'd rather not add "model" to -drive.  This should simplify your
   patch quite a bit.

   -drive mixes up frontend (a.k.a. device model) and backend
   configuration.  It provides only common device model configuration
   knobs.  For full device model control, use -device.

   "model" isn't common.  It applies only to some device models, and is
   silently ignored for all others.

   "serial" is in -drive only for backward compatibility.

   Should you insist on adding "model" to -drive, too, please split the
   patch: first part adds the property, second part adds the -drive
   parameter.

Re: [Qemu-devel] [PATCH v2] ide: Adds "model=s" option, allowing the user to override the default disk model name "QEMU HARDDISK"

[Floris Bos / Maxnet]

On 03/12/2012 02:48 PM, Markus Armbruster wrote:
> Floris Bos<bos@je-eigen-domein.nl>  writes:
>
>> Some Linux distributions use the /dev/disk/by-id/scsi-SATA_name-of-disk-model_serial addressing scheme
>> when refering to partitions in /etc/fstab and elsewhere.
>> This causes problems when starting a disk image taken from an existing physical server under qemu,
>> because when running under qemu name-of-disk-model is always "QEMU HARDDISK"
>> This patch introduces a model=s option which in combination with the existing serial=s option can be used to
>> fake the disk the operating system was previously on, allowing the OS to boot properly.
>>
>> Cc: kwolf@redhat.com
>> Signed-off-by: Floris Bos<dev@noc-ps.com>
> At first glance:
>
> 1. IDE only, scsi-disk is missing.

Correct.
IDE support is easy.

Adding SCSI support is more complicated, because "model" and "serial" 
are not the only factors that can make up a  /dev/disk/by-id link
Could be a combination of 3 fields: vendor, model and serial.
But could also be based on another unique ascii or binary identifier 
provided by the drive through Vital Product Data page 83h.

-- 
Yours sincerely,

Floris Bos

Re: [Qemu-devel] [PATCH v2] ide: Adds "model=s" option, allowing the user to override the default disk model name "QEMU HARDDISK"

[Paolo Bonzini]

Il 12/03/2012 16:27, Floris Bos / Maxnet ha scritto:
>>
>>
>> 1. IDE only, scsi-disk is missing.
> 
> Correct.
> IDE support is easy.
> 
> Adding SCSI support is more complicated, because "model" and "serial"
> are not the only factors that can make up a  /dev/disk/by-id link
> Could be a combination of 3 fields: vendor, model and serial.
> But could also be based on another unique ascii or binary identifier
> provided by the drive through Vital Product Data page 83h.

I am going to add customization of VPD page 83h soonish (though only
limited to another ASCII identifier), and Dmitry Fleytman told me
offlist that he's going to add vendor/model support too.

That said, I agree with the other comments from Markus.

Paolo

Re: [Qemu-devel] [PATCH v2] ide: Adds "model=s" option, allowing the user to override the default disk model name "QEMU HARDDISK"

[Floris Bos / Maxnet]

On 03/12/2012 05:08 PM, Paolo Bonzini wrote:
> Il 12/03/2012 16:27, Floris Bos / Maxnet ha scritto:
>>>
>>> 1. IDE only, scsi-disk is missing.
>> Correct.
>> IDE support is easy.
>>
>> Adding SCSI support is more complicated, because "model" and "serial"
>> are not the only factors that can make up a  /dev/disk/by-id link
>> Could be a combination of 3 fields: vendor, model and serial.
>> But could also be based on another unique ascii or binary identifier
>> provided by the drive through Vital Product Data page 83h.
> I am going to add customization of VPD page 83h soonish (though only
> limited to another ASCII identifier), and Dmitry Fleytman told me
> offlist that he's going to add vendor/model support too.

Nice

> That said, I agree with the other comments from Markus.

Fair enough.
You guys outnumber me, so I will resubmit my IDE patch without the 
legacy -drive support.


Yours sincerely,

Floris Bos