[PATCH 0/4] hw/nvme: make model & firmware version cnofigurable

[PATCH 0/4] hw/nvme: make model & firmware version cnofigurable

[Daniel P. Berrangé]

These are both currently set to QEMU specific hardcoded defaults,
and in common with other block devices, should allow user overrides.

The pre-existing serial field should also be length checked to
detect & report invalid user input instead of silently truncating.

Daniel P. Berrangé (4):
  include/block: define constants for NVME string fields
  hw/nvme: report error for oversized 'serial' parameter
  hw/nvme: add user controlled 'model' property
  hw/nvme: add user controlled 'firmware-version' property

 docs/system/devices/nvme.rst | 10 ++++++++++
 hw/nvme/ctrl.c               | 31 ++++++++++++++++++++++++++++---
 hw/nvme/nvme.h               |  2 ++
 include/block/nvme.h         | 10 +++++++---
 4 files changed, 47 insertions(+), 6 deletions(-)

-- 
2.53.0

[PATCH 1/4] include/block: define constants for NVME string fields

[Daniel P. Berrangé]

The version, model and serial fields accept fixed length strings.
Add constants to enable user supplied strings to be validated.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/block/nvme.h | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/include/block/nvme.h b/include/block/nvme.h
index 9d7159ed7a..e4e7be5120 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -1134,12 +1134,16 @@ enum NvmeIdCns {
     NVME_ID_CNS_CS_IND_NS_ALLOCATED   = 0x1f,
 };
 
+#define NVME_ID_CTRL_SN_MAX_LEN 20
+#define NVME_ID_CTRL_MN_MAX_LEN 40
+#define NVME_ID_CTRL_FR_MAX_LEN 8
+
 typedef struct QEMU_PACKED NvmeIdCtrl {
     uint16_t    vid;
     uint16_t    ssvid;
-    uint8_t     sn[20];
-    uint8_t     mn[40];
-    uint8_t     fr[8];
+    uint8_t     sn[NVME_ID_CTRL_SN_MAX_LEN];
+    uint8_t     mn[NVME_ID_CTRL_MN_MAX_LEN];
+    uint8_t     fr[NVME_ID_CTRL_FR_MAX_LEN];
     uint8_t     rab;
     uint8_t     ieee[3];
     uint8_t     cmic;
-- 
2.53.0

[PATCH 2/4] hw/nvme: report error for oversized 'serial' parameter

[Daniel P. Berrangé]

The 'serial' accepted by the NVME device is at most 20 characters
long. An over-sized user supplied value should be reported rather
than silently truncated.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 hw/nvme/ctrl.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index cc4593cd42..e6b2e3b70a 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -8600,6 +8600,11 @@ static bool nvme_check_params(NvmeCtrl *n, Error **errp)
         error_setg(errp, "serial property not set");
         return false;
     }
+    if (strlen(params->serial) > NVME_ID_CTRL_SN_MAX_LEN) {
+        error_setg(errp, "'serial' parameter '%s' can be at most '%d' characters",
+                   params->serial, NVME_ID_CTRL_SN_MAX_LEN);
+        return false;
+    }
 
     if (params->mqes < 1) {
         error_setg(errp, "mqes property cannot be less than 1");
-- 
2.53.0

[PATCH 3/4] hw/nvme: add user controlled 'model' property

[Daniel P. Berrangé]

This enables overriding the built in default "QEMU NVMe Ctrl" string
with a user specified string. The value can be at most 40 characters
in length.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 docs/system/devices/nvme.rst |  5 +++++
 hw/nvme/ctrl.c               | 14 ++++++++++++--
 hw/nvme/nvme.h               |  1 +
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/docs/system/devices/nvme.rst b/docs/system/devices/nvme.rst
index 6509b35fcb..109c01a70d 100644
--- a/docs/system/devices/nvme.rst
+++ b/docs/system/devices/nvme.rst
@@ -60,6 +60,11 @@ parameters.
   the SMART / Health information extended log become available in the
   controller. We emulate version 5 of this log page.
 
+``model`` (default: ``QEMU NVMe Ctrl``)
+  Override the default reported model, which can be used when needing
+  to more closely impersonate a particular device type. The model name
+  can be a maximum of 40 characters in length.
+
 Additional Namespaces
 ---------------------
 
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index e6b2e3b70a..4067443309 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -43,7 +43,8 @@
  *              atomic.dn=<on|off[optional]>, \
  *              atomic.awun<N[optional]>, \
  *              atomic.awupf<N[optional]>, \
- *              subsys=<subsys_id>
+ *              subsys=<subsys_id>, \
+ *              model=<model-str>
  *      -device nvme-ns,drive=<drive_id>,bus=<bus_name>,nsid=<nsid>,\
  *              zoned=<true|false[optional]>, \
  *              subsys=<subsys_id>,shared=<true|false[optional]>, \
@@ -8606,6 +8607,13 @@ static bool nvme_check_params(NvmeCtrl *n, Error **errp)
         return false;
     }
 
+    if (params->model &&
+        strlen(params->model) > NVME_ID_CTRL_MN_MAX_LEN) {
+        error_setg(errp, "'model' parameter '%s' can be at most '%d' characters",
+                   params->model, NVME_ID_CTRL_MN_MAX_LEN);
+        return false;
+    }
+
     if (params->mqes < 1) {
         error_setg(errp, "mqes property cannot be less than 1");
         return false;
@@ -9099,7 +9107,8 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
 
     id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
     id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
-    strpadcpy((char *)id->mn, sizeof(id->mn), "QEMU NVMe Ctrl", ' ');
+    strpadcpy((char *)id->mn, sizeof(id->mn),
+              n->params.model ? n->params.model : "QEMU NVMe Ctrl", ' ');
     strpadcpy((char *)id->fr, sizeof(id->fr), QEMU_VERSION, ' ');
     strpadcpy((char *)id->sn, sizeof(id->sn), n->params.serial, ' ');
 
@@ -9379,6 +9388,7 @@ static const Property nvme_props[] = {
     DEFINE_PROP_LINK("subsys", NvmeCtrl, subsys, TYPE_NVME_SUBSYS,
                      NvmeSubsystem *),
     DEFINE_PROP_STRING("serial", NvmeCtrl, params.serial),
+    DEFINE_PROP_STRING("model", NvmeCtrl, params.model),
     DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, params.cmb_size_mb, 0),
     DEFINE_PROP_UINT32("num_queues", NvmeCtrl, params.num_queues, 0),
     DEFINE_PROP_UINT32("max_ioqpairs", NvmeCtrl, params.max_ioqpairs, 64),
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index d66f7dc82d..ebf1fcfdcd 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -542,6 +542,7 @@ typedef struct NvmeCQueue {
 
 typedef struct NvmeParams {
     char     *serial;
+    char     *model;
     uint32_t num_queues; /* deprecated since 5.1 */
     uint32_t max_ioqpairs;
     uint16_t msix_qsize;
-- 
2.53.0

[PATCH 4/4] hw/nvme: add user controlled 'firmware-version' property

[Daniel P. Berrangé]

This enables overriding the built in default QEMU project version string
with a user specified string. The value can be at most 8 characters
in length.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 docs/system/devices/nvme.rst |  5 +++++
 hw/nvme/ctrl.c               | 14 ++++++++++++--
 hw/nvme/nvme.h               |  1 +
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/docs/system/devices/nvme.rst b/docs/system/devices/nvme.rst
index 109c01a70d..98a4401043 100644
--- a/docs/system/devices/nvme.rst
+++ b/docs/system/devices/nvme.rst
@@ -65,6 +65,11 @@ parameters.
   to more closely impersonate a particular device type. The model name
   can be a maximum of 40 characters in length.
 
+``firmware-version`` (default: current QEMU version number)
+  Override the default reported firmware version, which can be used when
+  needing to more closely impersonate a particular device type. The version
+  can be a maximum of 8 characters in length.
+
 Additional Namespaces
 ---------------------
 
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 4067443309..b392e053ab 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -44,7 +44,8 @@
  *              atomic.awun<N[optional]>, \
  *              atomic.awupf<N[optional]>, \
  *              subsys=<subsys_id>, \
- *              model=<model-str>
+ *              model=<model-str>, \
+ *              firmware-version=<version-str>
  *      -device nvme-ns,drive=<drive_id>,bus=<bus_name>,nsid=<nsid>,\
  *              zoned=<true|false[optional]>, \
  *              subsys=<subsys_id>,shared=<true|false[optional]>, \
@@ -8614,6 +8615,13 @@ static bool nvme_check_params(NvmeCtrl *n, Error **errp)
         return false;
     }
 
+    if (params->firmware_version &&
+        strlen(params->firmware_version) > NVME_ID_CTRL_FR_MAX_LEN) {
+        error_setg(errp, "'firmware-version' parameter '%s' can be at most '%d' characters",
+                   params->firmware_version, NVME_ID_CTRL_FR_MAX_LEN);
+        return false;
+    }
+
     if (params->mqes < 1) {
         error_setg(errp, "mqes property cannot be less than 1");
         return false;
@@ -9109,7 +9117,8 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
     id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
     strpadcpy((char *)id->mn, sizeof(id->mn),
               n->params.model ? n->params.model : "QEMU NVMe Ctrl", ' ');
-    strpadcpy((char *)id->fr, sizeof(id->fr), QEMU_VERSION, ' ');
+    strpadcpy((char *)id->fr, sizeof(id->fr),
+              n->params.firmware_version ? n->params.firmware_version : QEMU_VERSION, ' ');
     strpadcpy((char *)id->sn, sizeof(id->sn), n->params.serial, ' ');
 
     id->cntlid = cpu_to_le16(n->cntlid);
@@ -9389,6 +9398,7 @@ static const Property nvme_props[] = {
                      NvmeSubsystem *),
     DEFINE_PROP_STRING("serial", NvmeCtrl, params.serial),
     DEFINE_PROP_STRING("model", NvmeCtrl, params.model),
+    DEFINE_PROP_STRING("firmware-version", NvmeCtrl, params.firmware_version),
     DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, params.cmb_size_mb, 0),
     DEFINE_PROP_UINT32("num_queues", NvmeCtrl, params.num_queues, 0),
     DEFINE_PROP_UINT32("max_ioqpairs", NvmeCtrl, params.max_ioqpairs, 64),
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index ebf1fcfdcd..5ef3ebee29 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -543,6 +543,7 @@ typedef struct NvmeCQueue {
 typedef struct NvmeParams {
     char     *serial;
     char     *model;
+    char     *firmware_version;
     uint32_t num_queues; /* deprecated since 5.1 */
     uint32_t max_ioqpairs;
     uint16_t msix_qsize;
-- 
2.53.0

Re: [PATCH 0/4] hw/nvme: make model & firmware version cnofigurable

[Keith Busch]

On Fri, Mar 06, 2026 at 04:57:13PM +0000, Daniel P. Berrangé wrote:
> The pre-existing serial field should also be length checked to
> detect & report invalid user input instead of silently truncating.

Generally fine with that, but worried about user regressions from
unknowingly relying on the truncation behavior. Could these overflow
conditions just emit a truncation warn_report instead of failing to
start?

Re: [PATCH 0/4] hw/nvme: make model & firmware version cnofigurable

[Daniel P. Berrangé]

On Fri, Mar 06, 2026 at 10:05:37AM -0700, Keith Busch wrote:
> On Fri, Mar 06, 2026 at 04:57:13PM +0000, Daniel P. Berrangé wrote:
> > The pre-existing serial field should also be length checked to
> > detect & report invalid user input instead of silently truncating.
> 
> Generally fine with that, but worried about user regressions from
> unknowingly relying on the truncation behavior. Could these overflow
> conditions just emit a truncation warn_report instead of failing to
> start?

There is always that possibility that someone is unknowingly having
their data truncated, however, we have generally taken the view that
it is fair game to introduce error checking for invalid input in
these kind of cases. As prior art, we introduced error checking to
the SCSI serial to remove truncation:

  commit 75997e182b695f2e3f0a2d649734952af5caf3ee
  Author: Kevin Wolf <kwolf@redhat.com>
  Date:   Tue Jun 4 18:17:55 2024 +0200

    scsi-disk: Don't silently truncate serial number

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH 0/4] hw/nvme: make model & firmware version cnofigurable

[Avi Kivity]

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

On Fri, Mar 6, 2026 at 6:57 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> These are both currently set to QEMU specific hardcoded defaults,
> and in common with other block devices, should allow user overrides.
>
> The pre-existing serial field should also be length checked to
> detect & report invalid user input instead of silently truncating.
>
> Daniel P. Berrangé (4):
>   include/block: define constants for NVME string fields
>   hw/nvme: report error for oversized 'serial' parameter
>   hw/nvme: add user controlled 'model' property
>   hw/nvme: add user controlled 'firmware-version' property
>
>
Ping on this, I'd really like to see it merged.

[-- Attachment #2: Type: text/html, Size: 1103 bytes --]

Re: [PATCH 0/4] hw/nvme: make model & firmware version cnofigurable

[Klaus Jensen]

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

On Mar  6 16:57, Daniel P. Berrangé wrote:
> These are both currently set to QEMU specific hardcoded defaults,
> and in common with other block devices, should allow user overrides.
> 
> The pre-existing serial field should also be length checked to
> detect & report invalid user input instead of silently truncating.
> 
> Daniel P. Berrangé (4):
>   include/block: define constants for NVME string fields
>   hw/nvme: report error for oversized 'serial' parameter
>   hw/nvme: add user controlled 'model' property
>   hw/nvme: add user controlled 'firmware-version' property
> 
>  docs/system/devices/nvme.rst | 10 ++++++++++
>  hw/nvme/ctrl.c               | 31 ++++++++++++++++++++++++++++---
>  hw/nvme/nvme.h               |  2 ++
>  include/block/nvme.h         | 10 +++++++---
>  4 files changed, 47 insertions(+), 6 deletions(-)
> 
> -- 
> 2.53.0
> 
> 

Given the reference to prior art on introducing error checking in these
cases, this LGTM.

Applied to nvme.for-11.1. Thanks!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/4] hw/nvme: make model & firmware version cnofigurable

[Avi Kivity]

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

On Mon, Apr 13, 2026 at 8:34 PM Klaus Jensen <its@irrelevant.dk> wrote:

> On Mar  6 16:57, Daniel P. Berrangé wrote:
> > These are both currently set to QEMU specific hardcoded defaults,
> > and in common with other block devices, should allow user overrides.
> >
> > The pre-existing serial field should also be length checked to
> > detect & report invalid user input instead of silently truncating.
> >
> > Daniel P. Berrangé (4):
> >   include/block: define constants for NVME string fields
> >   hw/nvme: report error for oversized 'serial' parameter
> >   hw/nvme: add user controlled 'model' property
> >   hw/nvme: add user controlled 'firmware-version' property
> >
> >  docs/system/devices/nvme.rst | 10 ++++++++++
> >  hw/nvme/ctrl.c               | 31 ++++++++++++++++++++++++++++---
> >  hw/nvme/nvme.h               |  2 ++
> >  include/block/nvme.h         | 10 +++++++---
> >  4 files changed, 47 insertions(+), 6 deletions(-)
> >
> > --
> > 2.53.0
> >
> >
>
> Given the reference to prior art on introducing error checking in these
> cases, this LGTM.
>
> Applied to nvme.for-11.1. Thanks!
>

I don't see it in master. What's the schedule for upstreaming it?

[-- Attachment #2: Type: text/html, Size: 1708 bytes --]

Re: [PATCH 0/4] hw/nvme: make model & firmware version cnofigurable

[Avi Kivity]

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

On Mon, May 18, 2026 at 2:44 PM Avi Kivity <avi@scylladb.com> wrote:

> On Mon, Apr 13, 2026 at 8:34 PM Klaus Jensen <its@irrelevant.dk> wrote:
>
>> On Mar  6 16:57, Daniel P. Berrangé wrote:
>> > These are both currently set to QEMU specific hardcoded defaults,
>> > and in common with other block devices, should allow user overrides.
>> >
>> > The pre-existing serial field should also be length checked to
>> > detect & report invalid user input instead of silently truncating.
>> >
>> > Daniel P. Berrangé (4):
>> >   include/block: define constants for NVME string fields
>> >   hw/nvme: report error for oversized 'serial' parameter
>> >   hw/nvme: add user controlled 'model' property
>> >   hw/nvme: add user controlled 'firmware-version' property
>> >
>> >  docs/system/devices/nvme.rst | 10 ++++++++++
>> >  hw/nvme/ctrl.c               | 31 ++++++++++++++++++++++++++++---
>> >  hw/nvme/nvme.h               |  2 ++
>> >  include/block/nvme.h         | 10 +++++++---
>> >  4 files changed, 47 insertions(+), 6 deletions(-)
>> >
>> > --
>> > 2.53.0
>> >
>> >
>>
>> Given the reference to prior art on introducing error checking in these
>> cases, this LGTM.
>>
>> Applied to nvme.for-11.1. Thanks!
>>
>
> I don't see it in master. What's the schedule for upstreaming it?
>


I see it's upstream. Thanks, silent maintainers! Looking forward to 11.1.

[-- Attachment #2: Type: text/html, Size: 2217 bytes --]